From 20157241a847b17cc63643a8f327453123e13f2c Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 15 Mar 2023 12:52:32 +0100 Subject: [PATCH] fix: asset-registry metadata migration --- asset-registry/src/lib.rs | 2 +- asset-registry/src/migrations.rs | 174 ++++++++++++++++++++++++++++++- asset-registry/src/tests.rs | 101 ++++++++++++++++-- 3 files changed, 263 insertions(+), 14 deletions(-) diff --git a/asset-registry/src/lib.rs b/asset-registry/src/lib.rs index e2d5cea20..60a43bbac 100644 --- a/asset-registry/src/lib.rs +++ b/asset-registry/src/lib.rs @@ -29,7 +29,7 @@ mod mock; mod tests; mod migrations; -pub use migrations::Migration; +pub use migrations::MigrateV1Only_V0NotSupported; #[frame_support::pallet] pub mod module { diff --git a/asset-registry/src/migrations.rs b/asset-registry/src/migrations.rs index 75320e289..555774097 100644 --- a/asset-registry/src/migrations.rs +++ b/asset-registry/src/migrations.rs @@ -1,11 +1,146 @@ -use crate::{Config, LocationToAssetId, Pallet, Weight}; +use crate::{Config, LocationToAssetId, Metadata, Pallet, Weight}; use frame_support::pallet_prelude::*; use frame_support::{migration::storage_key_iter, traits::OnRuntimeUpgrade, StoragePrefixedMap}; use xcm::v3::prelude::*; -pub struct Migration(PhantomData); -impl OnRuntimeUpgrade for Migration { +pub mod v0 { + use codec::{Decode, Encode, MaxEncodedLen}; + use frame_support::traits::ConstU32; + use frame_support::WeakBoundedVec; + use scale_info::TypeInfo; + + // these imports are unchanged from v0, see: + // v2 reimport from v1: https://github.com/paritytech/polkadot/blob/645723987cf9662244be8faf4e9b63e8b9a1b3a3/xcm/src/v2/mod.rs#L65-L69 + // v1 reimport from v0: https://github.com/paritytech/polkadot/blob/645723987cf9662244be8faf4e9b63e8b9a1b3a3/xcm/src/v1/mod.rs#L92 + use xcm::v2::{BodyId, BodyPart, NetworkId}; + + // copied from https://github.com/paritytech/polkadot/blob/645723987cf9662244be8faf4e9b63e8b9a1b3a3/xcm/src/v0/multi_location.rs#L46-L65 + #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, Debug, scale_info::TypeInfo)] + pub enum MultiLocation { + /// The interpreting consensus system. + Null, + /// A relative path comprising 1 junction. + X1(Junction), + /// A relative path comprising 2 junctions. + X2(Junction, Junction), + /// A relative path comprising 3 junctions. + X3(Junction, Junction, Junction), + /// A relative path comprising 4 junctions. + X4(Junction, Junction, Junction, Junction), + /// A relative path comprising 5 junctions. + X5(Junction, Junction, Junction, Junction, Junction), + /// A relative path comprising 6 junctions. + X6(Junction, Junction, Junction, Junction, Junction, Junction), + /// A relative path comprising 7 junctions. + X7(Junction, Junction, Junction, Junction, Junction, Junction, Junction), + /// A relative path comprising 8 junctions. + X8( + Junction, + Junction, + Junction, + Junction, + Junction, + Junction, + Junction, + Junction, + ), + } + + // copied from https://github.com/paritytech/polkadot/blob/645723987cf9662244be8faf4e9b63e8b9a1b3a3/xcm/src/v0/junction.rs#L115-L169 + #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)] + pub enum Junction { + /// The consensus system of which the context is a member and state-wise + /// super-set. + /// + /// NOTE: This item is *not* a sub-consensus item: a consensus system + /// may not identify itself trustlessly as a location that includes this + /// junction. + Parent, + /// An indexed parachain belonging to and operated by the context. + /// + /// Generally used when the context is a Polkadot Relay-chain. + Parachain(#[codec(compact)] u32), + /// A 32-byte identifier for an account of a specific network that is + /// respected as a sovereign endpoint within the context. + /// + /// Generally used when the context is a Substrate-based chain. + AccountId32 { network: NetworkId, id: [u8; 32] }, + /// An 8-byte index for an account of a specific network that is + /// respected as a sovereign endpoint within the context. + /// + /// May be used when the context is a Frame-based chain and includes + /// e.g. an indices pallet. + AccountIndex64 { + network: NetworkId, + #[codec(compact)] + index: u64, + }, + /// A 20-byte identifier for an account of a specific network that is + /// respected as a sovereign endpoint within the context. + /// + /// May be used when the context is an Ethereum or Bitcoin chain or + /// smart-contract. + AccountKey20 { network: NetworkId, key: [u8; 20] }, + /// An instanced, indexed pallet that forms a constituent part of the + /// context. + /// + /// Generally used when the context is a Frame-based chain. + PalletInstance(u8), + /// A non-descript index within the context location. + /// + /// Usage will vary widely owing to its generality. + /// + /// NOTE: Try to avoid using this and instead use a more specific item. + GeneralIndex(#[codec(compact)] u128), + /// A nondescript datum acting as a key within the context location. + /// + /// Usage will vary widely owing to its generality. + /// + /// NOTE: Try to avoid using this and instead use a more specific item. + GeneralKey(WeakBoundedVec>), + /// The unambiguous child. + /// + /// Not currently used except as a fallback when deriving ancestry. + OnlyChild, + /// A pluralistic body existing within consensus. + /// + /// Typical to be used to represent a governance origin of a chain, but + /// could in principle be used to represent things such as multisigs + /// also. + Plurality { id: BodyId, part: BodyPart }, + } +} + +pub mod pre_polkadot_0_9_38 { + use codec::{Decode, Encode}; + use frame_support::{pallet_prelude::Member, Parameter, RuntimeDebug}; + use scale_info::TypeInfo; + use sp_std::vec::Vec; + + #[derive(scale_info::TypeInfo, Encode, Decode, Clone, Eq, PartialEq, RuntimeDebug)] + pub enum VersionedMultiLocation { + V0(super::v0::MultiLocation), + V1(xcm::v2::MultiLocation), /* v2::multilocation is identical to v1::multilocation. See https://github.com/paritytech/polkadot/blob/645723987cf9662244be8faf4e9b63e8b9a1b3a3/xcm/src/v2/mod.rs#L68 */ + } + + #[derive(scale_info::TypeInfo, Encode, Decode, Clone, Eq, PartialEq, RuntimeDebug)] + pub struct AssetMetadata { + pub decimals: u32, + pub name: Vec, + pub symbol: Vec, + pub existential_deposit: Balance, + pub location: Option, + pub additional: CustomMetadata, + } +} + +/// Migration that only works if Metadata.location is None or +/// Some(VersionedMultiLocation::V1(_)). Any metadata items that are +/// Some(VersionedMultiLocation::V0(_)) will be set to None. +#[allow(non_camel_case_types)] +pub struct MigrateV1Only_V0NotSupported(PhantomData); +impl OnRuntimeUpgrade for MigrateV1Only_V0NotSupported { fn on_runtime_upgrade() -> Weight { let mut weight: Weight = Weight::zero(); let onchain_version = Pallet::::on_chain_storage_version(); @@ -19,7 +154,9 @@ impl OnRuntimeUpgrade for Migration { mod v2 { use super::*; - + use frame_support::log; + use orml_traits::asset_registry::AssetMetadata; + use xcm::VersionedMultiLocation; pub(crate) fn migrate() -> Weight { let mut weight: Weight = Weight::zero(); let module_prefix = LocationToAssetId::::module_prefix(); @@ -37,6 +174,35 @@ mod v2 { LocationToAssetId::::insert(new_key, value); } + Metadata::::translate( + |_, old: pre_polkadot_0_9_38::AssetMetadata| { + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + + let new_location = match old.location { + Some(pre_polkadot_0_9_38::VersionedMultiLocation::V1(x)) => { + // v2::multilocation is identical to v1::multilocation. See https://github.com/paritytech/polkadot/blob/645723987cf9662244be8faf4e9b63e8b9a1b3a3/xcm/src/v2/mod.rs#L68 + Some(VersionedMultiLocation::V2(x)) + } + Some(pre_polkadot_0_9_38::VersionedMultiLocation::V0(_)) => { + log::error!("Migration for an item in metadata.location failed because V0 is not supported"); + None + } + None => None, + }; + + let new_metadata = AssetMetadata { + additional: old.additional, + decimals: old.decimals, + existential_deposit: old.existential_deposit, + name: old.name, + symbol: old.symbol, + location: new_location, + }; + + Some(new_metadata) + }, + ); + StorageVersion::new(2).put::>(); weight.saturating_accrue(T::DbWeight::get().writes(1)); weight diff --git a/asset-registry/src/tests.rs b/asset-registry/src/tests.rs index d43e34dc1..bebe23992 100644 --- a/asset-registry/src/tests.rs +++ b/asset-registry/src/tests.rs @@ -592,11 +592,15 @@ fn test_v2_to_v3_incompatible_multilocation() { #[test] fn from_unversioned_to_v2_storage() { + use crate::migrations::pre_polkadot_0_9_38; + use xcm::VersionedMultiLocation; + TestNet::reset(); ParaA::execute_with(|| { let module_prefix = b"AssetRegistry"; - let storage_prefix = b"LocationToAssetId"; + let location_storage_prefix = b"LocationToAssetId"; + let metadata_storage_prefix = b"Metadata"; // StorageVersion is 0 before migration assert_eq!(StorageVersion::get::>(), 0); @@ -628,19 +632,19 @@ fn from_unversioned_to_v2_storage() { // Store raw xcm::v2 data put_storage_value( module_prefix, - storage_prefix, + location_storage_prefix, &Twox64Concat::hash(&old_multilocation_0.encode()), asset_id_0, ); put_storage_value( module_prefix, - storage_prefix, + location_storage_prefix, &Twox64Concat::hash(&old_multilocation_1.encode()), asset_id_1, ); put_storage_value( module_prefix, - storage_prefix, + location_storage_prefix, &Twox64Concat::hash(&old_multilocation_2.encode()), asset_id_2, ); @@ -667,8 +671,58 @@ fn from_unversioned_to_v2_storage() { assert_eq!(AssetRegistry::location_to_asset_id(new_multilocation_1), None); assert_eq!(AssetRegistry::location_to_asset_id(new_multilocation_2), None); + let old_metadata_with_v0_location = pre_polkadot_0_9_38::AssetMetadata { + additional: CustomMetadata { fee_per_second: 5 }, + decimals: 1, + existential_deposit: 1234, + location: Some(pre_polkadot_0_9_38::VersionedMultiLocation::V0( + crate::migrations::v0::MultiLocation::Null, + )), + name: b"ghi".to_vec(), + symbol: b"jkl".to_vec(), + }; + + let old_metadata_with_v1_location = pre_polkadot_0_9_38::AssetMetadata { + additional: CustomMetadata { fee_per_second: 6 }, + decimals: 12, + existential_deposit: 123, + location: Some(pre_polkadot_0_9_38::VersionedMultiLocation::V1( + old_multilocation_1.clone(), + )), + name: b"abc".to_vec(), + symbol: b"def".to_vec(), + }; + + let old_metadata_without_location = pre_polkadot_0_9_38::AssetMetadata { + additional: CustomMetadata { fee_per_second: 7 }, + decimals: 3, + existential_deposit: 12345, + location: None, + name: b"mno".to_vec(), + symbol: b"pqr".to_vec(), + }; + + put_storage_value( + module_prefix, + metadata_storage_prefix, + &Twox64Concat::hash(&asset_id_0.encode()), + old_metadata_with_v0_location.clone(), + ); + put_storage_value( + module_prefix, + metadata_storage_prefix, + &Twox64Concat::hash(&asset_id_1.encode()), + old_metadata_with_v1_location.clone(), + ); + put_storage_value( + module_prefix, + metadata_storage_prefix, + &Twox64Concat::hash(&asset_id_2.encode()), + old_metadata_without_location.clone(), + ); + // Run StorageKey migration - crate::Migration::::on_runtime_upgrade(); + crate::MigrateV1Only_V0NotSupported::::on_runtime_upgrade(); // StorageVersion is 2 after migration assert_eq!(StorageVersion::get::>(), 2); @@ -690,24 +744,53 @@ fn from_unversioned_to_v2_storage() { // Assert the old key does not exist anymore assert!(get_storage_value::( module_prefix, - storage_prefix, + location_storage_prefix, &Twox64Concat::hash(&old_multilocation_0.encode()), ) .is_none()); assert!(get_storage_value::( module_prefix, - storage_prefix, + location_storage_prefix, &Twox64Concat::hash(&old_multilocation_1.encode()), ) .is_none()); assert!(get_storage_value::( module_prefix, - storage_prefix, + location_storage_prefix, &Twox64Concat::hash(&old_multilocation_2.encode()), ) .is_none()); + let new_metadata_with_from_v0_location = AssetRegistry::metadata(asset_id_0).unwrap(); + let new_metadata_with_from_v1_location = AssetRegistry::metadata(asset_id_1).unwrap(); + let new_metadata_without_location = AssetRegistry::metadata(asset_id_2).unwrap(); + + assert!(new_metadata_with_from_v0_location.location.is_none()); + assert!(new_metadata_without_location.location.is_none()); + // check the case where a migration actually happened + assert_eq!( + new_metadata_with_from_v1_location.location, + Some(VersionedMultiLocation::V2(old_multilocation_1.clone())) + ); + + let assert_other_fields_unchanged = + |old: pre_polkadot_0_9_38::AssetMetadata, + new: AssetMetadata| { + assert_eq!(old.additional, new.additional); + assert_eq!(old.decimals, new.decimals); + assert_eq!(old.existential_deposit, new.existential_deposit); + assert_eq!(old.name, new.name); + assert_eq!(old.symbol, new.symbol); + }; + + assert_other_fields_unchanged(old_metadata_with_v0_location, new_metadata_with_from_v0_location); + assert_other_fields_unchanged(old_metadata_with_v1_location, new_metadata_with_from_v1_location); + assert_other_fields_unchanged(old_metadata_without_location, new_metadata_without_location); + // Assert further calls are no-op - assert_eq!(crate::Migration::::on_runtime_upgrade(), Weight::zero()); + assert_eq!( + crate::MigrateV1Only_V0NotSupported::::on_runtime_upgrade(), + Weight::zero() + ); }); }