Skip to content
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/au precompile #1080

Merged
merged 15 commits into from
Dec 6, 2023
Merged

Feat/au precompile #1080

merged 15 commits into from
Dec 6, 2023

Conversation

gitofdeepanshu
Copy link
Contributor

@gitofdeepanshu gitofdeepanshu commented Nov 7, 2023

Closes #1055

Pull Request Summary

This PR adds the Precompile for pallet-unified-account.

Precompile Architecture

the return value of the precompile functions is designed as (H160/ss58 address,bool) where bool value indicated if the mapping is default or mapped.

true indicates that the mapping exists and precompile is returning the mapped value and false indicates that mapping doesn't exists and the precompile is returning the default value for that address.

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade hook for precompile revert code registration
  • updated spec version
  • updated semver

@gitofdeepanshu gitofdeepanshu added shiden related to shiden runtime astar Related to Astar shibuya related to shibuya runtime This PR/Issue is related to the topic “runtime”. AU Account Unification labels Nov 7, 2023
@gitofdeepanshu gitofdeepanshu marked this pull request as ready for review November 7, 2023 11:34
@gitofdeepanshu gitofdeepanshu removed shiden related to shiden runtime astar Related to Astar labels Nov 8, 2023
precompiles/unified-accounts/UnifiedAccounts.sol Outdated Show resolved Hide resolved
precompiles/unified-accounts/src/lib.rs Outdated Show resolved Hide resolved
precompiles/unified-accounts/src/lib.rs Outdated Show resolved Hide resolved
Dinonard
Dinonard previously approved these changes Dec 5, 2023
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some minor comments in .sol file

* @title UA interface.
*/

/// Interface to the precompiled contract on Shibuya
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to drop Shibuya from the comment.

And the address too, I guess. It's not tied to the interface.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll drop Shibuya.
For address, it's kind of tied to interface as we have same address in all networks and we have this in every precompile interface, also easier for people to just use it right from here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not tied to the .sol file as an interface.

It's tied to the implementation of this interface in our precompiles - there it's said to be the number mentioned above. But it could just as well change in the implementation without any change being required in the .sol file.

It's not a big deal, and you are right when you say it's also mentioned in other files.
I consider it to be wrong - however it is convenient.

No need to change it, it was more of a minor comment.

Copy link

github-actions bot commented Dec 5, 2023

Code Coverage

Package Line Rate Branch Rate Health
pallets/contracts-migration/src 0% 0%
chain-extensions/types/xvm/src 0% 0%
pallets/block-reward/src 85% 0%
pallets/dynamic-evm-base-fee/src 81% 0%
precompiles/sr25519/src 79% 0%
chain-extensions/types/dapps-staking/src 0% 0%
pallets/xc-asset-config/src 53% 0%
precompiles/unified-accounts/src 100% 0%
primitives/src/xcm 66% 0%
chain-extensions/types/unified-accounts/src 0% 0%
pallets/ethereum-checked/src 48% 0%
pallets/xvm/src 40% 0%
chain-extensions/dapps-staking/src 0% 0%
pallets/dapps-staking/src 81% 0%
precompiles/utils/src 55% 0%
chain-extensions/unified-accounts/src 0% 0%
primitives/src 65% 0%
chain-extensions/xvm/src 0% 0%
precompiles/utils/src/testing 38% 0%
pallets/unified-accounts/src 84% 0%
chain-extensions/pallet-assets/src 0% 0%
pallets/dapps-staking/src/pallet 85% 0%
precompiles/dapps-staking/src 93% 0%
precompiles/assets-erc20/src 77% 0%
pallets/collator-selection/src 69% 0%
pallets/block-rewards-hybrid/src 87% 0%
precompiles/utils/macro/src 0% 0%
precompiles/xcm/src 75% 0%
precompiles/substrate-ecdsa/src 78% 0%
precompiles/xvm/src 75% 0%
chain-extensions/types/assets/src 0% 0%
Summary 57% (2303 / 4040) 0% (0 / 0)

Minimum allowed line rate is 50%

Copy link
Member

@PierreOssun PierreOssun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! LGTM

@ashutoshvarma ashutoshvarma merged commit 2a8df8d into master Dec 6, 2023
8 checks passed
@ashutoshvarma ashutoshvarma deleted the feat/au-precompile branch December 6, 2023 05:04
@PierreOssun PierreOssun mentioned this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AU Account Unification runtime This PR/Issue is related to the topic “runtime”. shibuya related to shibuya
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Account Unification CE and Precompile
5 participants