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

Automatic EVM revert code registration for XC20 #1001

Merged
merged 8 commits into from
Aug 11, 2023
Merged

Conversation

Dinonard
Copy link
Member

@Dinonard Dinonard commented Aug 10, 2023

Pull Request Summary

Adds support for automatic EVM revert code registration for XC20.
Each time a new asset is created, an attempt will be made to insert the revert code.
If it succeeds, smart contracts will be able to immediately access the asset.
In case of failure, whole asset registration process is reverted.

Docs update: AstarNetwork/astar-docs#411

Removed some deprecated & unused code.

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • benchmarks for assets

@Dinonard Dinonard added shiden related to shiden runtime astar Related to Astar shibuya related to shibuya runtime This PR/Issue is related to the topic “runtime”. labels Aug 10, 2023
@Dinonard Dinonard marked this pull request as ready for review August 10, 2023 09:39
primitives/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

Looks good!

primitives/src/lib.rs Outdated Show resolved Hide resolved
pallets/xc-asset-config/src/lib.rs Outdated Show resolved Hide resolved
pallets/xc-asset-config/src/lib.rs Outdated Show resolved Hide resolved
Comment on lines 82 to 101
pub struct EvmRevertCodeHandler<A, R>(PhantomData<(A, R)>);
impl<A, R> AssetsCallback<AssetId, AccountId> for EvmRevertCodeHandler<A, R>
where
A: AddressToAssetId<AssetId>,
R: pallet_evm::Config,
{
fn created(id: &AssetId, _: &AccountId) -> Result<(), ()> {
let address = A::asset_id_to_address(*id);
// In case of collision, we need to cancel the asset creation.
ensure!(!pallet_evm::AccountCodes::<R>::contains_key(&address), ());
pallet_evm::AccountCodes::<R>::insert(address, EVM_REVERT_CODE.to_vec());
Ok(())
}

fn destroyed(id: &AssetId) -> Result<(), ()> {
let address = A::asset_id_to_address(*id);
pallet_evm::AccountCodes::<R>::remove(address);
Ok(())
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how the callback weights are taken into account inside pallet_assets (I couldn't find anything in code), but since we are doing DB writes who will be paying for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, and reminder!
I had written down to redo the benchmarks and just didn't 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

Solved.

@Dinonard
Copy link
Member Author

/bench astar-dev pallet_assets

@github-actions
Copy link

Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/5830181829.
Please wait for a while.
Branch: feat/xc20-revert-code
SHA: b9523a2

@github-actions
Copy link

Benchmarks have been finished.
You can download artifacts if exists https://github.com/AstarNetwork/Astar/actions/runs/5830181829.

@github-actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/block-reward/src 85% 0%
chain-extensions/types/dapps-staking/src 0% 0%
pallets/collator-selection/src 69% 0%
precompiles/substrate-ecdsa/src 78% 0%
precompiles/utils/macro/src 0% 0%
chain-extensions/pallet-assets/src 0% 0%
primitives/src 68% 0%
pallets/xc-asset-config/src 53% 0%
pallets/xvm/src 46% 0%
precompiles/xvm/src 83% 0%
precompiles/utils/src 71% 0%
precompiles/assets-erc20/src 76% 0%
precompiles/sr25519/src 79% 0%
pallets/pallet-xcm/src 53% 0%
chain-extensions/dapps-staking/src 0% 0%
precompiles/xcm/src 84% 0%
chain-extensions/types/assets/src 0% 0%
pallets/contracts-migration/src 0% 0%
pallets/ethereum-checked/src 48% 0%
chain-extensions/types/xvm/src 0% 0%
primitives/src/xcm 71% 0%
pallets/custom-signatures/src 51% 0%
pallets/dapps-staking/src/pallet 85% 0%
pallets/dapps-staking/src 81% 0%
chain-extensions/xvm/src 0% 0%
precompiles/dapps-staking/src 93% 0%
Summary 56% (2321 / 4115) 0% (0 / 0)

Minimum allowed line rate is 50%

@Dinonard Dinonard merged commit 4a2c3af into master Aug 11, 2023
@Dinonard Dinonard deleted the feat/xc20-revert-code branch August 11, 2023 13:14
PierreOssun pushed a commit that referenced this pull request Aug 15, 2023
* Automatic EVM revert code registration for XC20

* Add negative test

* Fmt fix

* Add EVM module

* Asset benchmarks

* Cleanup xc asset config

* Update asset benchmarks

* Fix tests features
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astar Related to Astar runtime This PR/Issue is related to the topic “runtime”. shibuya related to shibuya shiden related to shiden runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants