-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: pallet_unified_accounts
implementation
#1019
Conversation
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.
Thanks for the detailed PR and the summary!
One suggestion for the summary is that you don't need to duplicate architecture information if it's also present in the pallet's rustdoc. Better focus on higher level info related to the PR.
Also, try to align naming with what the purpose of struct/trait/function is. It helps a lot when reading, reviewing and maintaining code later on.
Will follow-up PR enable to call evm pallet methods using substrate account? By modifying CallOrigin of pallet_evm config? |
/bench shibuya-dev pallet_unified_accounts |
Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/6313707837. |
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.
Just 2 minor comments, otherwise LGTM.
I only checked the delta since the last review.
Good test coverage! 👍
Benchmarks have been finished. |
/bench shibuya-dev pallet_balances |
pallet_account
implementationpallet_unified_accounts
implementation
Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/6314586382. |
Benchmarks have been finished. |
Minimum allowed line rate is |
Overview
Initial implementation for Account Unification pallet -
pallet_unified_accounts
. The Accounts module provide functionality for native account holders to connect their EVM accounts to have a unified experience across the different VMs. Inspired from Acala's evm-accounts.This PR also introduce a small script that can be used as reference for claim accounts using
ethers.js
-claim.mjs
Assets Handling
In the scenario when EVM address already holds either native balance or XC20 assets then those funds needs to be transferred to the new SS58 account that is being connected to it.
Native Balance
The pallet will handle the transfer of native balance to new connected account since it is not very high maintenance (one dispatch call) and will in-fact improve the overall UX.
If we don't do this on pallet level then, when user connect the accounts, they first need to sign batch TX to transfer the XC20s and then sign another TX to for moving native balance before finally connecting the account.
The last tx can be skipped with this
XC20 (
pallet-assets
)The XC20 (pallet-assets) will be handled by UI purely since it'll unnecessarily increase the complexity in runtime code.
Mainly because it add complex logic to runtime that we would have to maintain for the same things that UI can also handle.
We don't have a easy way to transfer all of user funds from pallet-assets. (no good way to enumerate over user holdings in pallet-assets, no
TransferAll
from orml)EVM DApp Staking
If EVM address participated in DApp Staking via use of precompiles then he/she needs to claim and unstake everything since underlying SS58 would be changed.
This would also be handled purely on UI side as it'll also add complex logic to runtime which can be very well handled by UI.
TODO
Write RPC tests(follow-up task after integration in Shiden & Astar)pallet-balances
benchmark