Mesh 2176/fix-storage-naming (#1658)

* Deprecate NextCollectionId and NextNFTId * Deprecated AssetMetadataNextLocalKey and AssetMetadataNextGlobalKey * Fix benchmarks * Remove hidden imports; Use set instead of macro --------- Co-authored-by: Adam Dossa <adam.dossa@gmail.com>
PolymeshAssociation · May 7, 2024 · 2b3e32e · 2b3e32e
1 parent d11817f
commit 2b3e32e
Show file tree

Hide file tree

Showing 7 changed files with 312 additions and 28 deletions.
diff --git a/pallets/asset/src/benchmarking.rs b/pallets/asset/src/benchmarking.rs
@@ -90,7 +90,7 @@ fn register_metadata_global_name<T: Config>() -> AssetMetadataKey {
     Module::<T>::register_asset_metadata_global_type(root, name, spec)
         .expect("`register_asset_metadata_global_type` failed");
 
-    let key = Module::<T>::asset_metadata_next_global_key();
+    let key = Module::<T>::current_asset_metadata_global_key().unwrap();
     AssetMetadataKey::Global(key)
 }
 

diff --git a/pallets/asset/src/lib.rs b/pallets/asset/src/lib.rs
@@ -86,6 +86,7 @@ use core::mem;
 use currency::*;
 use frame_support::dispatch::{DispatchError, DispatchResult};
 use frame_support::traits::Get;
+use frame_support::weights::Weight;
 use frame_support::BoundedBTreeSet;
 use frame_support::{decl_module, decl_storage, ensure};
 use frame_system::ensure_root;
@@ -116,9 +117,9 @@ use polymesh_primitives::asset_metadata::{
 use polymesh_primitives::settlement::InstructionId;
 use polymesh_primitives::transfer_compliance::TransferConditionResult;
 use polymesh_primitives::{
-    extract_auth, storage_migration_ver, AssetIdentifier, Balance, Document, DocumentId,
-    IdentityId, Memo, PortfolioId, PortfolioKind, PortfolioUpdateReason, SecondaryKey, Ticker,
-    WeightMeter,
+    extract_auth, storage_migrate_on, storage_migration_ver, AssetIdentifier, Balance, Document,
+    DocumentId, IdentityId, Memo, PortfolioId, PortfolioKind, PortfolioUpdateReason, SecondaryKey,
+    Ticker, WeightMeter,
 };
 
 pub use error::Error;
@@ -133,7 +134,7 @@ type Identity<T> = pallet_identity::Module<T>;
 type Portfolio<T> = pallet_portfolio::Module<T>;
 type Statistics<T> = pallet_statistics::Module<T>;
 
-storage_migration_ver!(3);
+storage_migration_ver!(4);
 
 decl_storage! {
     trait Store for Module<T: Config> as Asset {
@@ -220,9 +221,12 @@ decl_storage! {
             map hasher(twox_64_concat) AssetMetadataGlobalKey => Option<AssetMetadataSpec>;
 
         /// Next Asset Metadata Local Key.
+        #[deprecated]
         pub AssetMetadataNextLocalKey get(fn asset_metadata_next_local_key):
             map hasher(blake2_128_concat) Ticker => AssetMetadataLocalKey;
+
         /// Next Asset Metadata Global Key.
+        #[deprecated]
         pub AssetMetadataNextGlobalKey get(fn asset_metadata_next_global_key): AssetMetadataGlobalKey;
 
         /// A list of tickers that exempt all users from affirming the receivement of the asset.
@@ -238,7 +242,14 @@ decl_storage! {
             map hasher(blake2_128_concat) Ticker => BoundedBTreeSet<IdentityId, T::MaxAssetMediators>;
 
         /// Storage version.
-        StorageVersion get(fn storage_version) build(|_| Version::new(3)): Version;
+        StorageVersion get(fn storage_version) build(|_| Version::new(4)): Version;
+
+        /// The last [`AssetMetadataLocalKey`] used for [`Ticker`].
+        pub CurrentAssetMetadataLocalKey get(fn current_asset_metadata_local_key):
+            map hasher(blake2_128_concat) Ticker => Option<AssetMetadataLocalKey>;
+
+        /// The last [`AssetMetadataGlobalKey`] used for a global key.
+        pub CurrentAssetMetadataGlobalKey get(fn current_asset_metadata_global_key): Option<AssetMetadataGlobalKey>;
     }
     add_extra_genesis {
         config(reserved_country_currency_codes): Vec<Ticker>;
@@ -273,6 +284,13 @@ decl_module! {
         /// initialize the default event for this module
         fn deposit_event() = default;
 
+        fn on_runtime_upgrade() -> Weight {
+            storage_migrate_on!(StorageVersion, 4, {
+                migration::migrate_to_v4::<T>();
+            });
+            Weight::zero()
+        }
+
         const AssetNameMaxLength: u32 = T::AssetNameMaxLength::get();
         const FundingRoundNameMaxLength: u32 = T::FundingRoundNameMaxLength::get();
         const AssetMetadataNameMaxLength: u32 = T::AssetMetadataNameMaxLength::get();
@@ -1362,7 +1380,8 @@ impl<T: Config> Module<T> {
         );
 
         // Next global key.
-        let key = AssetMetadataNextGlobalKey::try_mutate(try_next_pre::<T, _>)?;
+        let key = Self::update_current_asset_metadata_global_key()?;
+        AssetMetadataNextGlobalKey::set(key);
 
         // Store global key <-> name mapping.
         AssetMetadataGlobalNameToKey::insert(&name, key);
@@ -2313,7 +2332,8 @@ impl<T: Config> Module<T> {
         );
 
         // Next local key for asset.
-        let key = AssetMetadataNextLocalKey::try_mutate(ticker, try_next_pre::<T, _>)?;
+        let key = Self::update_current_asset_metadata_local_key(&ticker)?;
+        AssetMetadataNextLocalKey::insert(ticker, key);
 
         // Store local key <-> name mapping.
         AssetMetadataLocalNameToKey::insert(ticker, &name, key);
@@ -2327,6 +2347,43 @@ impl<T: Config> Module<T> {
         ));
         Ok(key.into())
     }
+
+    /// Adds one to `CurrentCollectionId`.
+    fn update_current_asset_metadata_global_key() -> Result<AssetMetadataGlobalKey, DispatchError> {
+        CurrentAssetMetadataGlobalKey::try_mutate(|current_global_key| match current_global_key {
+            Some(current_key) => {
+                let new_key = try_next_pre::<T, _>(current_key)?;
+                *current_global_key = Some(new_key);
+                Ok::<AssetMetadataGlobalKey, DispatchError>(new_key)
+            }
+            None => {
+                let new_key = AssetMetadataGlobalKey(1);
+                *current_global_key = Some(new_key);
+                Ok::<AssetMetadataGlobalKey, DispatchError>(new_key)
+            }
+        })
+    }
+
+    /// Adds one to the `AssetMetadataLocalKey` for the given `ticker`.
+    fn update_current_asset_metadata_local_key(
+        ticker: &Ticker,
+    ) -> Result<AssetMetadataLocalKey, DispatchError> {
+        CurrentAssetMetadataLocalKey::try_mutate(
+            ticker,
+            |current_local_key| match current_local_key {
+                Some(current_key) => {
+                    let new_key = try_next_pre::<T, _>(current_key)?;
+                    *current_local_key = Some(new_key);
+                    Ok::<AssetMetadataLocalKey, DispatchError>(new_key)
+                }
+                None => {
+                    let new_key = AssetMetadataLocalKey(1);
+                    *current_local_key = Some(new_key);
+                    Ok::<AssetMetadataLocalKey, DispatchError>(new_key)
+                }
+            },
+        )
+    }
 }
 
 //==========================================================================
@@ -2526,3 +2583,31 @@ impl<T: Config> AssetFnTrait<T::AccountId, T::RuntimeOrigin> for Module<T> {
         Self::add_mandatory_mediators(origin, ticker, mediators.try_into().unwrap_or_default())
     }
 }
+
+pub mod migration {
+    use frame_support::storage::{IterableStorageMap, StorageMap, StorageValue};
+    use sp_runtime::runtime_logger::RuntimeLogger;
+
+    use crate::{
+        AssetMetadataGlobalKey, AssetMetadataNextGlobalKey, AssetMetadataNextLocalKey, Config,
+        CurrentAssetMetadataGlobalKey, CurrentAssetMetadataLocalKey,
+    };
+
+    pub fn migrate_to_v4<T: Config>() {
+        RuntimeLogger::init();
+        log::info!(">>> Initializing CurrentAssetMetadataLocalKey and CurrentAssetMetadataGlobalKey storage");
+        initialize_storage::<T>();
+        log::info!(">>> Storage has been initialized");
+    }
+
+    fn initialize_storage<T: Config>() {
+        for (ticker, current_local_key) in AssetMetadataNextLocalKey::iter() {
+            CurrentAssetMetadataLocalKey::insert(ticker, current_local_key);
+        }
+
+        let global_key = AssetMetadataNextGlobalKey::get();
+        if AssetMetadataGlobalKey::default() != global_key {
+            CurrentAssetMetadataGlobalKey::put(global_key);
+        }
+    }
+}
diff --git a/pallets/nft/src/benchmarking.rs b/pallets/nft/src/benchmarking.rs
@@ -30,7 +30,7 @@ fn create_collection<T: Config>(
     let collection_keys: NFTCollectionKeys = creates_keys_register_metadata_types::<T>(n);
     Module::<T>::create_nft_collection(origin, ticker, nft_type, collection_keys)
         .expect("failed to create nft collection");
-    Module::<T>::collection_id()
+    Module::<T>::current_collection_id().unwrap()
 }
 
 /// Creates a set of `NFTCollectionKeys` made of `n` global keys and registers `n` global asset metadata types.

diff --git a/pallets/nft/src/lib.rs b/pallets/nft/src/lib.rs
@@ -1,7 +1,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 use codec::{Decode, Encode};
-use frame_support::dispatch::DispatchResult;
+use frame_support::dispatch::{DispatchError, DispatchResult};
 use frame_support::storage::StorageDoubleMap;
 use frame_support::traits::Get;
 use frame_support::weights::Weight;
@@ -34,7 +34,7 @@ type Portfolio<T> = pallet_portfolio::Module<T>;
 #[cfg(feature = "runtime-benchmarks")]
 pub mod benchmarking;
 
-storage_migration_ver!(2);
+storage_migration_ver!(3);
 
 decl_storage!(
     trait Store for Module<T: Config> as NFT {
@@ -54,9 +54,11 @@ decl_storage!(
         pub MetadataValue get(fn metadata_value): double_map hasher(blake2_128_concat) (NFTCollectionId, NFTId), hasher(blake2_128_concat) AssetMetadataKey => AssetMetadataValue;
 
         /// The next available id for an NFT collection.
+        #[deprecated]
         pub NextCollectionId get(fn collection_id): NFTCollectionId;
 
         /// The next available id for an NFT within a collection.
+        #[deprecated]
         pub NextNFTId get(fn nft_id): map hasher(blake2_128_concat) NFTCollectionId => NFTId;
 
         /// The total number of NFTs in a collection
@@ -65,8 +67,14 @@ decl_storage!(
         /// Tracks the owner of an NFT
         pub NFTOwner get(fn nft_owner): double_map hasher(blake2_128_concat) Ticker, hasher(blake2_128_concat) NFTId => Option<PortfolioId>;
 
+        /// The last `NFTId` used for an NFT.
+        pub CurrentNFTId get(fn current_nft_id): map hasher(blake2_128_concat) NFTCollectionId => Option<NFTId>;
+
+        /// The last `NFTCollectionId` used for a collection.
+        pub CurrentCollectionId get(fn current_collection_id): Option<NFTCollectionId>;
+
         /// Storage version.
-        StorageVersion get(fn storage_version) build(|_| Version::new(2)): Version;
+        StorageVersion get(fn storage_version) build(|_| Version::new(3)): Version;
     }
 );
 
@@ -82,8 +90,8 @@ decl_module! {
         fn deposit_event() = default;
 
         fn on_runtime_upgrade() -> Weight {
-            storage_migrate_on!(StorageVersion, 2, {
-                migration::migrate_to_v2::<T>();
+            storage_migrate_on!(StorageVersion, 3, {
+                migration::migrate_to_v3::<T>();
             });
             Weight::zero()
         }
@@ -295,7 +303,8 @@ impl<T: Config> Module<T> {
         }
 
         // Creates the nft collection
-        let collection_id = NextCollectionId::try_mutate(try_next_pre::<T, _>)?;
+        let collection_id = Self::update_current_collection_id()?;
+        NextCollectionId::set(collection_id);
         let nft_collection = NFTCollection::new(collection_id, ticker.clone());
         Collection::insert(&collection_id, nft_collection);
         CollectionKeys::insert(&collection_id, collection_keys);
@@ -360,7 +369,8 @@ impl<T: Config> Module<T> {
         let new_balance = NumberOfNFTs::get(&ticker, &caller_portfolio.did)
             .checked_add(1)
             .ok_or(Error::<T>::BalanceOverflow)?;
-        let nft_id = NextNFTId::try_mutate(&collection_id, try_next_pre::<T, _>)?;
+        let nft_id = Self::update_current_nft_id(&collection_id)?;
+        NextNFTId::insert(&collection_id, nft_id);
         NFTsInCollection::insert(&ticker, new_supply);
         NumberOfNFTs::insert(&ticker, &caller_portfolio.did, new_balance);
         for (metadata_key, metadata_value) in nft_attributes.into_iter() {
@@ -597,6 +607,38 @@ impl<T: Config> Module<T> {
         ));
         Ok(())
     }
+
+    /// Adds one to `CurrentCollectionId`.
+    fn update_current_collection_id() -> Result<NFTCollectionId, DispatchError> {
+        CurrentCollectionId::try_mutate(|current_collection_id| match current_collection_id {
+            Some(current_id) => {
+                let new_id = try_next_pre::<T, _>(current_id)?;
+                *current_collection_id = Some(new_id);
+                Ok::<NFTCollectionId, DispatchError>(new_id)
+            }
+            None => {
+                let new_id = NFTCollectionId(1);
+                *current_collection_id = Some(new_id);
+                Ok::<NFTCollectionId, DispatchError>(new_id)
+            }
+        })
+    }
+
+    /// Adds one to the `NFTId` that belongs to `collection_id`.
+    fn update_current_nft_id(collection_id: &NFTCollectionId) -> Result<NFTId, DispatchError> {
+        CurrentNFTId::try_mutate(collection_id, |current_nft_id| match current_nft_id {
+            Some(current_id) => {
+                let new_nft_id = try_next_pre::<T, _>(current_id)?;
+                *current_nft_id = Some(new_nft_id);
+                Ok::<NFTId, DispatchError>(new_nft_id)
+            }
+            None => {
+                let new_nft_id = NFTId(1);
+                *current_nft_id = Some(new_nft_id);
+                Ok::<NFTId, DispatchError>(new_nft_id)
+            }
+        })
+    }
 }
 
 impl<T: Config> NFTTrait<T::RuntimeOrigin> for Module<T> {
@@ -626,22 +668,28 @@ impl<T: Config> NFTTrait<T::RuntimeOrigin> for Module<T> {
 }
 
 pub mod migration {
-    use crate::sp_api_hidden_includes_decl_storage::hidden_include::IterableStorageDoubleMap;
-    use crate::{Config, NFTOwner};
-    use frame_support::storage::StorageDoubleMap;
-    use pallet_portfolio::PortfolioNFT;
+    use frame_support::storage::{IterableStorageMap, StorageMap, StorageValue};
     use sp_runtime::runtime_logger::RuntimeLogger;
 
-    pub fn migrate_to_v2<T: Config>() {
+    use crate::{
+        Config, CurrentCollectionId, CurrentNFTId, NFTCollectionId, NextCollectionId, NextNFTId,
+    };
+
+    pub fn migrate_to_v3<T: Config>() {
         RuntimeLogger::init();
-        log::info!(">>> Updating NFTOwner Storage");
-        initialize_nft_owner::<T>();
-        log::info!(">>> NFTOwner was successfully updated");
+        log::info!(">>> Initializing CurrentNFTId and CurrentCollectionId storage");
+        initialize_storage::<T>();
+        log::info!(">>> Storage has been initialized");
     }
 
-    fn initialize_nft_owner<T: Config>() {
-        for (portfolio_id, (ticker, nft_id), _) in PortfolioNFT::iter() {
-            NFTOwner::insert(ticker, nft_id, portfolio_id);
+    fn initialize_storage<T: Config>() {
+        for (collection_id, current_nft_id) in NextNFTId::iter() {
+            CurrentNFTId::insert(collection_id, current_nft_id);
+        }
+
+        let collection_id = NextCollectionId::get();
+        if NFTCollectionId::default() != collection_id {
+            CurrentCollectionId::put(collection_id);
         }
     }
 }
diff --git a/pallets/runtime/tests/src/asset_pallet/mod.rs b/pallets/runtime/tests/src/asset_pallet/mod.rs
@@ -2,4 +2,5 @@ mod accept_ticker_transfer;
 mod base_transfer;
 mod controller_transfer;
 mod issue;
+mod register_metadata;
 mod register_ticker;