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: upgrade asset manager #814

Merged
merged 42 commits into from
Nov 8, 2022
Merged

feat: upgrade asset manager #814

merged 42 commits into from
Nov 8, 2022

Conversation

bhgomes
Copy link
Contributor

@bhgomes bhgomes commented Sep 29, 2022

Signed-off-by: Brandon H. Gomes bhgomes@pm.me

Description

closes: #808

  • Switch to u128 for AssetId of Calamari asset-manager pallet and add migration code for asset-manager and pallet-assets storage items that use AssetId. MantaPay AssetId to follow in a different PR. Only switch Calamari not Dolphin, because we don't need the migration before we restart it.

  • The invariants that are checked are that there is no data in the new keys prior to the migration and that the same data is successfully migrated to the new keys. We also check manually that the old keys no longer hold any data.

  • Turn AssetId and Balance of AssetConfig into generics

  • Refactor some of the FungibleLedger functions to have try-versions. Rename some of the api functions.

  • General improvements of related code

  • Main files to review:

Related question to why we can't assert that the old asset-id storage keys are empty in the post-upgrade try-runtime hook - https://substrate.stackexchange.com/questions/5214/potential-storage-issue-with-try-runtime-tool
But this was manually checked on a local testnet that the old storage keys are indeed drained.

Result of try-runtime:

2022-11-07 20:53:45 Connection established to target: Target { sockaddrs: [], host: "ws.calamari.systems", host_header: "ws.calamari.systems:443", _mode: Tls, path_and_query: "/" }
2022-11-07 20:53:45 scraping key-pairs from remote @ 0x089effe921771abf95df0623ee32ead2aab3d2b9e809b45af5300e0f72f0fbf9    
2022-11-07 20:53:45 downloading data for all pallets.    
2022-11-07 20:54:30 adding data for hashed key: 26aa394eea5630e07c48ae0c9558cef7f9cce9c888469bb1a0dceaa129672ef8    
2022-11-07 20:54:30 extending externalities with 1 manually injected key-values    
2022-11-07 20:54:30 👩‍👦 scraping child-tree data from 0 top keys    
2022-11-07 20:54:30 Custom("[backend]: frontend dropped; terminate client")
2022-11-07 20:54:30 injecting a total of 64079 top keys    
2022-11-07 20:54:31 injecting a total of 0 child keys    
2022-11-07 20:54:31 initialized state externalities with storage root 0xddadddff54a674394eeaeeef9d15b8b3571648c977c5a75334fb92a28c1e0e69    
2022-11-07 20:54:32 Connection established to target: Target { sockaddrs: [], host: "ws.calamari.systems", host_header: "ws.calamari.systems:443", _mode: Tls, path_and_query: "/" }
2022-11-07 20:54:32 found matching spec name: "calamari"    
2022-11-07 20:54:32 found matching spec version: 3432    
2022-11-07 20:54:32 Custom("[backend]: frontend dropped; terminate client")
2022-11-07 20:54:32 Starting migration for AssetManager...    
2022-11-07 20:54:32 Storage migration for AssetManager's AssetIdLocation storage item has been executed.    
2022-11-07 20:54:32 Storage migration for AssetManager's LocationAssetId storage item has been executed.    
2022-11-07 20:54:32 Storage migration for AssetManager's AssetIdMetadata storage item has been executed.    
2022-11-07 20:54:32 Storage migration for AssetManager's UnitsPerSecond storage item has been executed.    
2022-11-07 20:54:32 Storage migration for AssetManager's NextAssetId storage item has been executed.    
2022-11-07 20:54:32 ✅ Storage migration for AssetManager has been executed successfully and storage version has been update to: 2.    
2022-11-07 20:54:32 Starting migration for pallet-assets...    
2022-11-07 20:54:32 Storage migration for Assets' Asset storage item has been executed.    
2022-11-07 20:54:32 Storage migration for Assets' Account storage item has been executed.    
2022-11-07 20:54:32 Storage migration for Assets' Metadata storage item has been executed.    
2022-11-07 20:54:32 ✅ Storage migration for Assets has been executed successfully and storage version has been update to: 1.    
2022-11-07 20:54:32 ✅ no migration for System    
2022-11-07 20:54:32 ✅ no migration for CalamariVesting    
2022-11-07 20:54:32 ✅ no migration for AssetManager    
2022-11-07 20:54:32 ✅ no migration for Assets    
2022-11-07 20:54:32 ✅ no migration for Multisig    
2022-11-07 20:54:32 ✅ no migration for Utility    
2022-11-07 20:54:32 ✅ no migration for XTokens    
2022-11-07 20:54:32 ✅ no migration for DmpQueue    
2022-11-07 20:54:32 ✅ no migration for CumulusXcm    
2022-11-07 20:54:32 ⚠️ PolkadotXcm declares internal migrations (which *might* execute). On-chain `StorageVersion(0)` vs current storage version `StorageVersion(0)`    
2022-11-07 20:54:32 ⚠️ XcmpQueue declares internal migrations (which *might* execute). On-chain `StorageVersion(1)` vs current storage version `StorageVersion(1)`    
2022-11-07 20:54:32 ✅ no migration for Scheduler    
2022-11-07 20:54:32 ✅ no migration for Preimage    
2022-11-07 20:54:32 ✅ no migration for Treasury    
2022-11-07 20:54:32 ✅ no migration for Aura    
2022-11-07 20:54:32 ✅ no migration for Session    
2022-11-07 20:54:32 ✅ no migration for CollatorSelection    
2022-11-07 20:54:32 ✅ no migration for Authorship    
2022-11-07 20:54:32 ✅ no migration for AuraAuthorFilter    
2022-11-07 20:54:32 ✅ no migration for AuthorInherent    
2022-11-07 20:54:32 ✅ no migration for ParachainStaking    
2022-11-07 20:54:32 ✅ no migration for TechnicalMembership    
2022-11-07 20:54:32 ✅ no migration for TechnicalCommittee    
2022-11-07 20:54:32 ✅ no migration for CouncilMembership    
2022-11-07 20:54:32 ✅ no migration for Council    
2022-11-07 20:54:32 ✅ no migration for Democracy    
2022-11-07 20:54:32 ✅ no migration for TransactionPayment    
2022-11-07 20:54:32 ✅ no migration for Balances    
2022-11-07 20:54:32 ✅ no migration for TransactionPause    
2022-11-07 20:54:32 ✅ no migration for ParachainInfo    
2022-11-07 20:54:32 ✅ no migration for Timestamp    
2022-11-07 20:54:32 ⚠️ ParachainSystem declares internal migrations (which *might* execute). On-chain `StorageVersion(1)` vs current storage version `StorageVersion(1)`    
2022-11-07 20:54:32 ✅ Storage migration for AssetManager's AssetIdLocation storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for AssetManager's LocationAssetId storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for AssetManager's AssetIdMetadata storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for AssetManager's UnitsPerSecond storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for AssetManager's NextAssetId storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for Assets' Asset storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for Assets' Account storage item has been executed successfully.    
2022-11-07 20:54:32 ✅ Storage migration for Assets' Metadata storage item has been executed successfully.    
2022-11-07 20:54:32 proof: 00000000000000000000000000000000244d6f6f6e7269766572104d4f5652120000000000000000000000000000000000304163616c6120446f6c6c617210415553440c0000000000000000000000000000000000444b6172757261204c6971756964204b534d104c4b534d0c00000000000000000000000000000000004c4b6172757261204e617469766520546f6b656e0c4b41520c002043616c616d6172690c4b4d410c000000e8764817000000000000000000000001244d6f6f6e7269766572104d4f565212000000008a5d78456301000000000000000001250f64652af8a98da6f42e6eec6d15f964ab324d46889d2f9e67eb3ab6afedf07de95ac7304163616c6120446f6c6c617210415553440c000000e40b54020000000000000000000000013f072c12285b5d4551f88e8f6e7eb52b810100000085814a7236281c16b0fda4cfb8703c2d812e4895a67bd0a55cf8a1708235807a3f080436db8bf2d563cb5d2999a82809eded0b0000004b6fe6541b739c2191023bbb5678e77d4668c868fd7506763680f4376c51fb453f080436db8bf2d563cb5d2999a82809eded0b000000b379f4b0c9897881aa84d53a72e71854805da327dd6d45d1d4d8b1ad9338defd3f080436db8bf2d563cb5d2999a82809eded0b000000cb4c9e2e6aa6069223e22ab5a99733c7e4ac72249fd17125776307fad169b8ad3f0805...06aca508a2c0a73c18ff700d5ab9b7a70aa15a76d84d95cb61adfc8bf8b8ffb80c000000200480323ccbf2d2073d768b1dc4546b42999aef966d7b058fc49abb8cbe805b04c8cc80af4f4109ea0c9d14b5c82609227b9f653d04c4c62c1395873a1b1519037798aca70e19bf46066d74446ee15c8c2715f7030d000000280480cd3e3bf5669614bac02cd1179605106e99aaf3f221a73d929e71957054e22f2280419d54780bded1897cff3f854e27db5701302ec42bb8c0bd15d3eae29e3a3355806a7a38a2bf0e9f1c98b6a411f70116427de6a80b123a6a53440fd0cb1d9222ada70e1f3931028cc05c2e18a319e8f64f9e08000000210480191fc2e0dbc7baa0c1b06d2d92d7dde8848ccdfa73e04b9d6392abb411b565ad80d3d816fc1f7cf147f744017f5fab0aa253a527bae0078808968beaa1daedd661808537bdbb3acc2e767192560c45fd15cc48d4ac698528c3609390a7690df92607bf300cd15a3fd6e04e47bee3922dbfa92c8d826869651d2963939b862dcfb8bc6b7d419e3ce659b3c1704b7dea3ec3ee2745de72e8948330af2d400480bb196235f93acd40fb1562587e0964bfc739619fb39d307ead8f03393c1617df805142ad38e5b94dbbf1e1fab52b777a99344a4c7baf611d40f67b8f0c87be0753c10340000080853ad76c7f4a8d11ae49f5ecb01a7a216fa5d149fdb098a600f2dba9f41e392f / 103 nodes    
2022-11-07 20:54:32 proof size: 879.43 KB (900538 bytes)    
2022-11-07 20:54:32 compact proof size: 876.74 KB (897781 bytes)    
2022-11-07 20:54:32 zstd-compressed compact proof 876.77 KB (897808 bytes)    
2022-11-07 20:54:32 TryRuntime_on_runtime_upgrade executed without errors. Consumed weight = 5150000000, total weight = 500000000000 (0.0103)  

Before we can approve this PR for merge, please make sure that all the following items have been checked off:

  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Added one label out of the L- group to this PR
  • Added one or more labels from the A- and C- groups to this PR
  • Explicitly labelled A-calamari, A-dolphin and/or A-manta if your changes are meant for/impact either of these (CI depends on it)
  • This PR is targeted against the current Milestone ( otherwise discuss if it can be added in time)
  • Re-reviewed Files changed in the Github PR explorer.

Situational Notes:

  • If adding functionality, write unit tests!
  • If importing a new pallet, choose a proper module index for it, and allow it in BaseFilter. Ensure every extrinsic works from front-end. If there's corresponding tool, ensure both work for each other.
  • If needed, update our Javascript/Typescript APIs. These APIs are officially used by exchanges or community developers.
  • If modifying existing runtime storage items, make sure to implement storage migrations for the runtime and test them with try-runtime. This includes migrations inherited from upstream changes, and you can search the diffs for modifications of #[pallet::storage] items to check for any.
  • If runtime changes, need to update the version numbers properly:
    • authoring_version: The version of the authorship interface. An authoring node will not attempt to author blocks unless this is equal to its native runtime.
    • spec_version: The version of the runtime specification. A full node will not attempt to use its native runtime in substitute for the on-chain Wasm runtime unless all of spec_name, spec_version, and authoring_version are the same between Wasm and native.
    • impl_version: The version of the implementation of the specification. Nodes are free to ignore this; it serves only as an indication that the code is different; as long as the other two versions are the same then while the actual code may be different, it is nonetheless required to do the same thing. Non-consensus-breaking optimizations are about the only changes that could be made which would result in only the impl_version changing.
    • transaction_version: The version of the extrinsics interface. This number must be updated in the following circumstances: extrinsic parameters (number, order, or types) have been changed; extrinsics or pallets have been removed; or the pallet order in the construct_runtime! macro or extrinsic order in a pallet has been changed. You can run the metadata_diff.yml workflow for help. If this number is updated, then the spec_version must also be updated
  • Verify benchmarks & weights have been updated for any modified runtime logics

Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
primitives/manta/src/assets.rs Outdated Show resolved Hide resolved
primitives/manta/src/assets.rs Outdated Show resolved Hide resolved
pallets/asset-manager/src/lib.rs Show resolved Hide resolved
pallets/asset-manager/src/lib.rs Show resolved Hide resolved
@ghzlatarev ghzlatarev self-assigned this Sep 30, 2022
@ghzlatarev ghzlatarev added L-changed Log: Issues and PRs related to changes A-runtime Area: Issues and PRs related to Runtimes A-migration Area: This PR needs/does a storage migration A-calamari Area: Issues and PRs related to the Calamari Runtime C-enhancement Category: An issue proposing an enhancement or a PR with one labels Sep 30, 2022
@ghzlatarev ghzlatarev added this to the v3.5.0 milestone Sep 30, 2022
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
@ghzlatarev ghzlatarev added the A-dolphin Area: Issues and PRs related to the Dolphin Runtime label Sep 30, 2022
…d invariants checkes, but works.

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
@ghzlatarev ghzlatarev marked this pull request as ready for review October 5, 2022 16:32
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Copy link
Contributor

@Garandor Garandor left a comment

Choose a reason for hiding this comment

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

ok, i'm through. see comments for requested changes.
Most importantly, remove evm_address

@ghzlatarev
Copy link
Contributor

ghzlatarev commented Nov 6, 2022

Result of try-runtime:

The log shows no indication that the assetmanager migration has executed, that seems wrong

That's because I didn't add any logs, will do. And the migration is not part of the pallet code so that's probably the reason for 2022-10-05 20:29:37 ✅ no migration for AssetManager

…nfig impl and #[pallet::generate_store(pub(super) trait Store)] in pallet-asset-manager, remove unnecessary comments, use StandardAssetId in pallet-manta-pay benchmarking

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
:

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Dengjianping
Dengjianping previously approved these changes Nov 7, 2022
@ghzlatarev
Copy link
Contributor

Result of try-runtime:

The log shows no indication that the assetmanager migration has executed, that seems wrong

Updated the try-runtime results with the logs now.

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Copy link
Contributor

@Garandor Garandor left a comment

Choose a reason for hiding this comment

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

LGTM!

@Garandor Garandor merged commit 0365ac7 into manta Nov 8, 2022
@Garandor Garandor deleted the feat/upgrade-asset-manager branch November 8, 2022 03:38
@Garandor Garandor removed this from the v4.0.0 milestone Nov 8, 2022
zqhxuyuan pushed a commit that referenced this pull request Dec 7, 2022
* feat: add asset-manager changes

Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>

* Compiles and tests pass, cleaned warnings

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Split Dolphin and Calamari AssetId types

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Migration code for asset-manager and assets pallets, needs cleanup and invariants checkes, but works.

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Add invariant checks

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Update

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Migrate Accounts storage double map with storage aliasing

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Add equivalence checks before and after migration

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Remove post-upgrade check that old storage is empty

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Clean up and formatting

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Revert default AssetRegistryMetadata. Fix benchmarks

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Clean up and docs

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Cleanup

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix clippy

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Address review comments. Incremenet storage version of both pallets and check for correctness

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Use MaybeSerializeDesirialize from sp-runtime

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Improve asset-manager benchmarking code

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix clippy, rmeove pallet-assets from pallet-asset-manager cargo.toml

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Use more explicitly named trait for asset metadata defaults for testing - TestingDefault

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix docs

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Migrate AssetRegistryMetadata

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Bump AssetManager StorageVersion in pallet

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Use AtLeast32BitUnsigned instead of local impl of CheckedIncrement

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* taplo fmt

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Use One trait

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix tests

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Rename try_ name to more explicit and clean up docs

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* clean up asset-manager benchmarking code, removed redundant GenesisConfig impl and #[pallet::generate_store(pub(super) trait Store)] in pallet-asset-manager, remove unnecessary comments, use StandardAssetId in pallet-manta-pay benchmarking

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix clippy and asset-manager benchamrking tests

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Add missing doccoment in old asset-manager migration code

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Refactor AssetIdMigration generics and get rid of magic numbers'

* Missing doccoments related to xtokents and xcm

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* More missing dcocoments

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Add progress logs to AssetId migration

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Add #[compact] to all AssetId extrinsic params'
:

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Remove evm_address field from metadata

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix clippy

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

* Fix pre-runtime hook storage version check for pallet-assets

Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>

Signed-off-by: Brandon H. Gomes <bhgomes@pm.me>
Signed-off-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Co-authored-by: Georgi Zlatarev <georgi.zlatarev@manta.network>
Signed-off-by: zqhxuyuan <zqhxuyuan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-calamari Area: Issues and PRs related to the Calamari Runtime A-dolphin Area: Issues and PRs related to the Dolphin Runtime A-migration Area: This PR needs/does a storage migration A-runtime Area: Issues and PRs related to Runtimes C-enhancement Category: An issue proposing an enhancement or a PR with one L-changed Log: Issues and PRs related to changes P-high Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AssetManager AssetId migration
6 participants