This repository has been archived by the owner on Sep 28, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The account abstraction pallet #130
base: polkadot-v0.9.39
Are you sure you want to change the base?
The account abstraction pallet #130
Changes from 23 commits
9856d9e
8e049e6
a909f8e
3619ac4
2cec263
d810947
6e71c60
c1a740e
6f5aa27
ee7e727
0973c5a
98700fa
f8149af
b0c4bb0
11ec601
eb56aa2
b494247
0307dcd
34b2d24
c829786
564a2e9
83fa23b
ca3bddc
3a1ae8b
7aca6a1
81f533d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the origin derivation algorithm is deterministic, could we skip the origin registering? The derived origin could be derived just in time in a
proxy_call
, like:If we do need to cache the derived origin to reduce gas costs, the cache should be cleaned on kill account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe derivation algorithm is just a sample, it not restricted to be deterministic. Account registration helps to prevent spamming and account address manipulation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akru what is the usecase behind allowing single account to have multiple derived address of the same type?
The way I see it, it just adds unnecessary complexity.
If we only allow one account type to be derived from the origin, then it's much simpler.
If an account wants to control multiple different derived accounts, it can use proxy to achieve that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shaun also commented above(here) about what happens when account is killed. Without any locked or reserved funds, these storage items will remain in the DB even without an owner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still free - so my comment from 2 months ago is still valid.
You should charge rent fee for these two items since you're inflating the database.
Check how
pallet-assets
is doing it, or any other pallet insubstrate
that creates some storage items (e.g.pallet-identity
,pallet-multisig
, etc.).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, you pay transaction fee to cover it (prevent spam), it’s not required at all, just one of ways to do it. I’m not sure it’s required for the prototype.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My personal opinion - better charge fee than just lock token forever. Charged fee much more controllable, it could be burn, redistributed or whatever you want. But locked deposit is just locked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way it's implemented now - sure, that's viable option. It still needs to be added though since it's missing now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure something is missing now, what precisely you are talking about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm talking about payment for the created storage.
Given that our handling of fees is copy/paste of Polkadot, it is how we do things, isn't it?
Don't understand why this pallet should be any different.
Are you proposing we mix it up, some pallets charging for rent fee, while some don't? 🤷♂️
It's been on review for months, and better to do it right immediately then have someone else fix it later, add storage migration logic, etc. Doesn't matter if it's prototype or not, IMO.