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

Foreign Assets Migration #3020

Merged
merged 35 commits into from
Dec 4, 2024
Merged

Foreign Assets Migration #3020

merged 35 commits into from
Dec 4, 2024

Conversation

ahmadkaouk
Copy link
Contributor

@ahmadkaouk ahmadkaouk commented Oct 21, 2024

What does it do?

This PR is a follow up of #2869 , it manage the migration of existing foreign assets to the new design. The PR introduces five extrinsics in pallet-moonbeam-lazy-migration to handle the migration of a foreign asset.

approve_assets_to_migrate

  • Sets the list of asset id for assets approved for migration
  • Allowed origins: FastGeneralAdmin, 5/9 of OpenTechCommittee, Root

start_foreign_asset_migration

  • Initiates migration for a specific foreign asset
  • Freezes the original asset to prevent further transactions
  • Creates new EVM smart contract via the new moonbeam-foreign-assets pallet
  • Stores migration state including remaining balances and approvals count

migrate_foreign_asset_balances

  • Migrates asset balances in batches (controlled by limit parameter)
  • Drains balances from old assets pallet
  • Mints equivalent amounts in new foreign assets system
  • Updates remaining balances counter

migrate_foreign_asset_approvals

  • Migrates asset approvals in batches (controlled by limit parameter)
  • Drains approvals from old assets pallet
  • Creates equivalent approvals in new foreign assets system
  • Unreserves deposits from old approval system
  • Updates remaining approvals counter

finish_foreign_asset_migration

  • Completes migration after all balances and approvals are migrated
  • Performs final cleanup of old asset storage
  • Unreserves deposits from asset creation and metadata

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@noandrea noandrea added D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes labels Nov 12, 2024
@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Nov 15, 2024
@ahmadkaouk ahmadkaouk added the breaking Needs to be mentioned in breaking changes label Nov 22, 2024
Copy link
Contributor

github-actions bot commented Nov 29, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2268 KB (no changes) ✅

Moonbeam runtime: 2248 KB (no changes) ✅

Moonriver runtime: 2252 KB (no changes) ✅

Compared to latest release (runtime-3300)

Moonbase runtime: 2268 KB (+240 KB compared to latest release) ⚠️

Moonbeam runtime: 2248 KB (+252 KB compared to latest release) ⚠️

Moonriver runtime: 2252 KB (+260 KB compared to latest release) ⚠️

@ahmadkaouk ahmadkaouk marked this pull request as ready for review November 29, 2024 11:31
Copy link
Contributor

github-actions bot commented Nov 29, 2024

Coverage Report

@@                        Coverage Diff                         @@
##           master   ahmad-foreign-assets-migration      +/-   ##
==================================================================
- Coverage   74.70%                           74.44%   -0.26%     
+ Files         369                              375       +6     
+ Lines       94308                            95766    +1458     
==================================================================
+ Hits        70446                            71284     +838     
+ Misses      23862                            24482     +620     
Files Changed Coverage
/client/rpc/dev/src/lib.rs 49.40% (-39.76%) 🔽
/client/rpc/finality/src/lib.rs 12.50% (-68.75%) 🔽
/client/rpc-core/txpool/src/types/content.rs 68.52% (-1.85%) 🔽
/client/rpc-core/txpool/src/types/inspect.rs 88.46% (-3.85%) 🔽
/pallets/moonbeam-foreign-assets/src/evm.rs 74.02% (+3.24%) 🔼
/pallets/moonbeam-foreign-assets/src/lib.rs 74.82% (+3.97%) 🔼
/pallets/moonbeam-lazy-migrations/src/lib.rs 80.65% (-1.39%) 🔽
/pallets/moonbeam-lazy-migrations/src/mock.rs 67.86% (-7.90%) 🔽
/pallets/moonbeam-lazy-migrations/src/tests.rs 98.15% (-1.12%) 🔽

Coverage generated Wed Dec 4 09:10:07 UTC 2024

Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

Added a few remarks, but overall looks good. Great work 🚀

gonzamontiel
gonzamontiel previously approved these changes Dec 3, 2024
Copy link
Contributor

@gonzamontiel gonzamontiel left a comment

Choose a reason for hiding this comment

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

With the addition of being able to migrate several foreign assets at the same time, It looks good to me, great work!

@ahmadkaouk ahmadkaouk merged commit 317f49c into master Dec 4, 2024
41 checks passed
@ahmadkaouk ahmadkaouk deleted the ahmad-foreign-assets-migration branch December 4, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A8-mergeoncegreen Pull request is reviewed well. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited evm-native-foreign-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants