From efd660309f2ee944718a6302627bbb956ada3729 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Fri, 25 Oct 2024 11:31:21 +0200 Subject: [PATCH] Fix migrations for pallet-xcm (#6148) Relates to: https://github.com/paritytech/polkadot-sdk/pull/4826 Relates to: https://github.com/paritytech/polkadot-sdk/issues/3214 ## Description `pallet-xcm` stores some operational data that uses `Versioned*` XCM types. When we add a new XCM version (XV), we deprecate XV-2 and remove XV-3. Without proper migration, this can lead to issues with [undecodable storage](https://github.com/paritytech/polkadot-sdk/actions/runs/11381324568/job/31662577532?pr=6092), as was identified on the XCMv5 branch where XCMv2 was removed. This PR extends the existing `MigrateToLatestXcmVersion` to include migration for the `Queries`, `LockedFungibles`, and `RemoteLockedFungibles` storage types. Additionally, more checks were added to `try_state` for these types. ## TODO - [x] create tracking issue for `polkadot-fellows` https://github.com/polkadot-fellows/runtimes/issues/492 - [x] Add missing `MigrateToLatestXcmVersion` for westend - [x] fix pallet-xcm `Queries` - fails for Westend https://github.com/paritytech/polkadot-sdk/actions/runs/11381324568/job/31662577532?pr=6092 - `V2` was removed from `Versioned*` stuff, but we have a live data with V2 e.g. Queries - e.g. Kusama or Polkadot relay chains ``` VersionNotifier: { origin: { V2: { parents: 0 interior: { X1: { Parachain: 2,124 } } } } isActive: true } ``` ![image](https://github.com/user-attachments/assets/f59f761b-46a7-4def-8aea-45c4e41c0a00) - [x] fix also for `RemoteLockedFungibles` - [x] fix also for `LockedFungibles` ## Follow-ups - [ ] deploy on Westend chains before XCMv5 - [ ] https://github.com/paritytech/polkadot-sdk/issues/6188 --------- Co-authored-by: command-bot <> Co-authored-by: GitHub Action Co-authored-by: Francisco Aguirre --- polkadot/runtime/westend/src/lib.rs | 2 + .../parachain/xcm_config.rs | 2 +- .../relay_chain/xcm_config.rs | 2 +- polkadot/xcm/pallet-xcm/src/lib.rs | 42 +- polkadot/xcm/pallet-xcm/src/migration.rs | 379 +++++++++++++++++- polkadot/xcm/pallet-xcm/src/tests/mod.rs | 172 +++++++- .../single_asset_adapter/mock.rs | 2 +- .../xcm/xcm-builder/src/tests/pay/mock.rs | 3 +- polkadot/xcm/xcm-runtime-apis/tests/mock.rs | 3 +- prdoc/pr_6148.prdoc | 17 + 10 files changed, 610 insertions(+), 14 deletions(-) create mode 100644 prdoc/pr_6148.prdoc diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index a91ca399db41..4941d91df57d 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1809,6 +1809,8 @@ pub mod migrations { >, parachains_shared::migration::MigrateToV1, parachains_scheduler::migration::MigrateV2ToV3, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); } diff --git a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs index 7cb230f6e006..f8a1826b8ab1 100644 --- a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs +++ b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/parachain/xcm_config.rs @@ -168,7 +168,7 @@ impl pallet_xcm::Config for Runtime { type UniversalLocation = UniversalLocation; // No version discovery needed const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 0; - type AdvertisedXcmVersion = frame::traits::ConstU32<3>; + type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion; type AdminOrigin = frame_system::EnsureRoot; // No locking type TrustedLockers = (); diff --git a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs index a31e664d8216..e7b602df7338 100644 --- a/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs +++ b/polkadot/xcm/docs/src/cookbook/relay_token_transactor/relay_chain/xcm_config.rs @@ -142,7 +142,7 @@ impl pallet_xcm::Config for Runtime { type UniversalLocation = UniversalLocation; // No version discovery needed const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 0; - type AdvertisedXcmVersion = frame::traits::ConstU32<3>; + type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion; type AdminOrigin = frame_system::EnsureRoot; // No locking type TrustedLockers = (); diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 254bc2c7b83d..9b8f735b478f 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -2807,6 +2807,44 @@ impl Pallet { /// set. #[cfg(any(feature = "try-runtime", test))] pub fn do_try_state() -> Result<(), TryRuntimeError> { + use migration::data::NeedsMigration; + + // Take the minimum version between `SafeXcmVersion` and `latest - 1` and ensure that the + // operational data is stored at least at that version, for example, to prevent issues when + // removing older XCM versions. + let minimal_allowed_xcm_version = if let Some(safe_xcm_version) = SafeXcmVersion::::get() + { + XCM_VERSION.saturating_sub(1).min(safe_xcm_version) + } else { + XCM_VERSION.saturating_sub(1) + }; + + // check `Queries` + ensure!( + !Queries::::iter_values() + .any(|data| data.needs_migration(minimal_allowed_xcm_version)), + TryRuntimeError::Other("`Queries` data should be migrated to the higher xcm version!") + ); + + // check `LockedFungibles` + ensure!( + !LockedFungibles::::iter_values() + .any(|data| data.needs_migration(minimal_allowed_xcm_version)), + TryRuntimeError::Other( + "`LockedFungibles` data should be migrated to the higher xcm version!" + ) + ); + + // check `RemoteLockedFungibles` + ensure!( + !RemoteLockedFungibles::::iter() + .any(|(key, data)| key.needs_migration(minimal_allowed_xcm_version) || + data.needs_migration(minimal_allowed_xcm_version)), + TryRuntimeError::Other( + "`RemoteLockedFungibles` data should be migrated to the higher xcm version!" + ) + ); + // if migration has been already scheduled, everything is ok and data will be eventually // migrated if CurrentMigration::::exists() { @@ -2887,7 +2925,7 @@ impl xcm_executor::traits::Enact for UnlockTicket { let mut maybe_remove_index = None; let mut locked = BalanceOf::::zero(); let mut found = false; - // We could just as well do with with an into_iter, filter_map and collect, however this way + // We could just as well do with an into_iter, filter_map and collect, however this way // avoids making an allocation. for (i, x) in locks.iter_mut().enumerate() { if x.1.try_as::<_>().defensive() == Ok(&self.unlocker) { @@ -3268,7 +3306,7 @@ impl OnResponse for Pallet { }); return Weight::zero() } - return match maybe_notify { + match maybe_notify { Some((pallet_index, call_index)) => { // This is a bit horrible, but we happen to know that the `Call` will // be built by `(pallet_index: u8, call_index: u8, QueryId, Response)`. diff --git a/polkadot/xcm/pallet-xcm/src/migration.rs b/polkadot/xcm/pallet-xcm/src/migration.rs index 2c5b2620f535..80154f57ddfb 100644 --- a/polkadot/xcm/pallet-xcm/src/migration.rs +++ b/polkadot/xcm/pallet-xcm/src/migration.rs @@ -15,7 +15,8 @@ // along with Polkadot. If not, see . use crate::{ - pallet::CurrentMigration, Config, Pallet, VersionMigrationStage, VersionNotifyTargets, + pallet::CurrentMigration, Config, CurrentXcmVersion, Pallet, VersionMigrationStage, + VersionNotifyTargets, }; use frame_support::{ pallet_prelude::*, @@ -25,6 +26,307 @@ use frame_support::{ const DEFAULT_PROOF_SIZE: u64 = 64 * 1024; +/// Utilities for handling XCM version migration for the relevant data. +pub mod data { + use crate::*; + + /// A trait for handling XCM versioned data migration for the requested `XcmVersion`. + pub(crate) trait NeedsMigration { + type MigratedData; + + /// Returns true if data does not match `minimal_allowed_xcm_version`. + fn needs_migration(&self, minimal_allowed_xcm_version: XcmVersion) -> bool; + + /// Attempts to migrate data. `Ok(None)` means no migration is needed. + /// `Ok(Some(Self::MigratedData))` should contain the migrated data. + fn try_migrate(self, to_xcm_version: XcmVersion) -> Result, ()>; + } + + /// Implementation of `NeedsMigration` for `LockedFungibles` data. + impl NeedsMigration for BoundedVec<(B, VersionedLocation), M> { + type MigratedData = Self; + + fn needs_migration(&self, minimal_allowed_xcm_version: XcmVersion) -> bool { + self.iter() + .any(|(_, unlocker)| unlocker.identify_version() < minimal_allowed_xcm_version) + } + + fn try_migrate( + mut self, + to_xcm_version: XcmVersion, + ) -> Result, ()> { + let mut was_modified = false; + for locked in self.iter_mut() { + if locked.1.identify_version() < to_xcm_version { + let Ok(new_unlocker) = locked.1.clone().into_version(to_xcm_version) else { + return Err(()) + }; + locked.1 = new_unlocker; + was_modified = true; + } + } + + if was_modified { + Ok(Some(self)) + } else { + Ok(None) + } + } + } + + /// Implementation of `NeedsMigration` for `Queries` data. + impl NeedsMigration for QueryStatus { + type MigratedData = Self; + + fn needs_migration(&self, minimal_allowed_xcm_version: XcmVersion) -> bool { + match &self { + QueryStatus::Pending { responder, maybe_match_querier, .. } => + responder.identify_version() < minimal_allowed_xcm_version || + maybe_match_querier + .as_ref() + .map(|v| v.identify_version() < minimal_allowed_xcm_version) + .unwrap_or(false), + QueryStatus::VersionNotifier { origin, .. } => + origin.identify_version() < minimal_allowed_xcm_version, + QueryStatus::Ready { response, .. } => + response.identify_version() < minimal_allowed_xcm_version, + } + } + + fn try_migrate(self, to_xcm_version: XcmVersion) -> Result, ()> { + if !self.needs_migration(to_xcm_version) { + return Ok(None) + } + + // do migration + match self { + QueryStatus::Pending { responder, maybe_match_querier, maybe_notify, timeout } => { + let Ok(responder) = responder.into_version(to_xcm_version) else { + return Err(()) + }; + let Ok(maybe_match_querier) = + maybe_match_querier.map(|mmq| mmq.into_version(to_xcm_version)).transpose() + else { + return Err(()) + }; + Ok(Some(QueryStatus::Pending { + responder, + maybe_match_querier, + maybe_notify, + timeout, + })) + }, + QueryStatus::VersionNotifier { origin, is_active } => origin + .into_version(to_xcm_version) + .map(|origin| Some(QueryStatus::VersionNotifier { origin, is_active })), + QueryStatus::Ready { response, at } => response + .into_version(to_xcm_version) + .map(|response| Some(QueryStatus::Ready { response, at })), + } + } + } + + /// Implementation of `NeedsMigration` for `RemoteLockedFungibles` key type. + impl NeedsMigration for (XcmVersion, A, VersionedAssetId) { + type MigratedData = Self; + + fn needs_migration(&self, minimal_allowed_xcm_version: XcmVersion) -> bool { + self.0 < minimal_allowed_xcm_version || + self.2.identify_version() < minimal_allowed_xcm_version + } + + fn try_migrate(self, to_xcm_version: XcmVersion) -> Result, ()> { + if !self.needs_migration(to_xcm_version) { + return Ok(None) + } + + let Ok(asset_id) = self.2.into_version(to_xcm_version) else { return Err(()) }; + Ok(Some((to_xcm_version, self.1, asset_id))) + } + } + + /// Implementation of `NeedsMigration` for `RemoteLockedFungibles` data. + impl> NeedsMigration + for RemoteLockedFungibleRecord + { + type MigratedData = Self; + + fn needs_migration(&self, minimal_allowed_xcm_version: XcmVersion) -> bool { + self.owner.identify_version() < minimal_allowed_xcm_version || + self.locker.identify_version() < minimal_allowed_xcm_version + } + + fn try_migrate(self, to_xcm_version: XcmVersion) -> Result, ()> { + if !self.needs_migration(to_xcm_version) { + return Ok(None) + } + + let RemoteLockedFungibleRecord { amount, owner, locker, consumers } = self; + + let Ok(owner) = owner.into_version(to_xcm_version) else { return Err(()) }; + let Ok(locker) = locker.into_version(to_xcm_version) else { return Err(()) }; + + Ok(Some(RemoteLockedFungibleRecord { amount, owner, locker, consumers })) + } + } + + impl Pallet { + /// Migrates relevant data to the `required_xcm_version`. + pub(crate) fn migrate_data_to_xcm_version( + weight: &mut Weight, + required_xcm_version: XcmVersion, + ) { + const LOG_TARGET: &str = "runtime::xcm::pallet_xcm::migrate_data_to_xcm_version"; + + // check and migrate `Queries` + let queries_to_migrate = Queries::::iter().filter_map(|(id, data)| { + weight.saturating_add(T::DbWeight::get().reads(1)); + match data.try_migrate(required_xcm_version) { + Ok(Some(new_data)) => Some((id, new_data)), + Ok(None) => None, + Err(_) => { + tracing::error!( + target: LOG_TARGET, + ?id, + ?required_xcm_version, + "`Queries` cannot be migrated!" + ); + None + }, + } + }); + for (id, new_data) in queries_to_migrate { + tracing::info!( + target: LOG_TARGET, + query_id = ?id, + ?new_data, + "Migrating `Queries`" + ); + Queries::::insert(id, new_data); + weight.saturating_add(T::DbWeight::get().writes(1)); + } + + // check and migrate `LockedFungibles` + let locked_fungibles_to_migrate = + LockedFungibles::::iter().filter_map(|(id, data)| { + weight.saturating_add(T::DbWeight::get().reads(1)); + match data.try_migrate(required_xcm_version) { + Ok(Some(new_data)) => Some((id, new_data)), + Ok(None) => None, + Err(_) => { + tracing::error!( + target: LOG_TARGET, + ?id, + ?required_xcm_version, + "`LockedFungibles` cannot be migrated!" + ); + None + }, + } + }); + for (id, new_data) in locked_fungibles_to_migrate { + tracing::info!( + target: LOG_TARGET, + account_id = ?id, + ?new_data, + "Migrating `LockedFungibles`" + ); + LockedFungibles::::insert(id, new_data); + weight.saturating_add(T::DbWeight::get().writes(1)); + } + + // check and migrate `RemoteLockedFungibles` - 1. step - just data + let remote_locked_fungibles_to_migrate = + RemoteLockedFungibles::::iter().filter_map(|(id, data)| { + weight.saturating_add(T::DbWeight::get().reads(1)); + match data.try_migrate(required_xcm_version) { + Ok(Some(new_data)) => Some((id, new_data)), + Ok(None) => None, + Err(_) => { + tracing::error!( + target: LOG_TARGET, + ?id, + ?required_xcm_version, + "`RemoteLockedFungibles` data cannot be migrated!" + ); + None + }, + } + }); + for (id, new_data) in remote_locked_fungibles_to_migrate { + tracing::info!( + target: LOG_TARGET, + key = ?id, + amount = ?new_data.amount, + locker = ?new_data.locker, + owner = ?new_data.owner, + consumers_count = ?new_data.consumers.len(), + "Migrating `RemoteLockedFungibles` data" + ); + RemoteLockedFungibles::::insert(id, new_data); + weight.saturating_add(T::DbWeight::get().writes(1)); + } + + // check and migrate `RemoteLockedFungibles` - 2. step - key + let remote_locked_fungibles_keys_to_migrate = RemoteLockedFungibles::::iter_keys() + .filter_map(|key| { + if key.needs_migration(required_xcm_version) { + let old_key = key.clone(); + match key.try_migrate(required_xcm_version) { + Ok(Some(new_key)) => Some((old_key, new_key)), + Ok(None) => None, + Err(_) => { + tracing::error!( + target: LOG_TARGET, + id = ?old_key, + ?required_xcm_version, + "`RemoteLockedFungibles` key cannot be migrated!" + ); + None + }, + } + } else { + None + } + }); + for (old_key, new_key) in remote_locked_fungibles_keys_to_migrate { + weight.saturating_add(T::DbWeight::get().reads(1)); + // make sure, that we don't override accidentally other data + if RemoteLockedFungibles::::get(&new_key).is_some() { + tracing::error!( + target: LOG_TARGET, + ?old_key, + ?new_key, + "`RemoteLockedFungibles` already contains data for a `new_key`!" + ); + // let's just skip for now, could be potentially caused with missing this + // migration before (manual clean-up?). + continue; + } + + tracing::info!( + target: LOG_TARGET, + ?old_key, + ?new_key, + "Migrating `RemoteLockedFungibles` key" + ); + + // now we can swap the keys + RemoteLockedFungibles::::swap::< + ( + NMapKey, + NMapKey, + NMapKey, + ), + _, + _, + >(&old_key, &new_key); + weight.saturating_add(T::DbWeight::get().writes(1)); + } + } + } +} + pub mod v1 { use super::*; use crate::{CurrentMigration, VersionMigrationStage}; @@ -84,7 +386,80 @@ pub mod v1 { pub struct MigrateToLatestXcmVersion(core::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToLatestXcmVersion { fn on_runtime_upgrade() -> Weight { + let mut weight = T::DbWeight::get().reads(1); + + // trigger expensive/lazy migration (kind of multi-block) CurrentMigration::::put(VersionMigrationStage::default()); - T::DbWeight::get().writes(1) + weight.saturating_accrue(T::DbWeight::get().writes(1)); + + // migrate other operational data to the latest XCM version in-place + let latest = CurrentXcmVersion::get(); + Pallet::::migrate_data_to_xcm_version(&mut weight, latest); + + weight + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_: alloc::vec::Vec) -> Result<(), sp_runtime::TryRuntimeError> { + use data::NeedsMigration; + const LOG_TARGET: &str = "runtime::xcm::pallet_xcm::migrate_to_latest"; + + let latest = CurrentXcmVersion::get(); + + let number_of_queries_to_migrate = crate::Queries::::iter() + .filter(|(id, data)| { + let needs_migration = data.needs_migration(latest); + if needs_migration { + tracing::warn!( + target: LOG_TARGET, + query_id = ?id, + query = ?data, + "Query was not migrated!" + ) + } + needs_migration + }) + .count(); + + let number_of_locked_fungibles_to_migrate = crate::LockedFungibles::::iter() + .filter_map(|(id, data)| { + if data.needs_migration(latest) { + tracing::warn!( + target: LOG_TARGET, + ?id, + ?data, + "LockedFungibles item was not migrated!" + ); + Some(true) + } else { + None + } + }) + .count(); + + let number_of_remote_locked_fungibles_to_migrate = + crate::RemoteLockedFungibles::::iter() + .filter_map(|(key, data)| { + if key.needs_migration(latest) || data.needs_migration(latest) { + tracing::warn!( + target: LOG_TARGET, + ?key, + "RemoteLockedFungibles item was not migrated!" + ); + Some(true) + } else { + None + } + }) + .count(); + + ensure!(number_of_queries_to_migrate == 0, "must migrate all `Queries`."); + ensure!(number_of_locked_fungibles_to_migrate == 0, "must migrate all `LockedFungibles`."); + ensure!( + number_of_remote_locked_fungibles_to_migrate == 0, + "must migrate all `RemoteLockedFungibles`." + ); + + Ok(()) } } diff --git a/polkadot/xcm/pallet-xcm/src/tests/mod.rs b/polkadot/xcm/pallet-xcm/src/tests/mod.rs index c16c1a1ba986..e98a8f8d2ce7 100644 --- a/polkadot/xcm/pallet-xcm/src/tests/mod.rs +++ b/polkadot/xcm/pallet-xcm/src/tests/mod.rs @@ -19,11 +19,15 @@ pub(crate) mod assets_transfer; use crate::{ - mock::*, pallet::SupportedVersion, AssetTraps, Config, CurrentMigration, Error, - ExecuteControllerWeightInfo, LatestVersionedLocation, Pallet, Queries, QueryStatus, - RecordedXcm, ShouldRecordXcm, VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, + migration::data::NeedsMigration, + mock::*, + pallet::{LockedFungibles, RemoteLockedFungibles, SupportedVersion}, + AssetTraps, Config, CurrentMigration, Error, ExecuteControllerWeightInfo, + LatestVersionedLocation, Pallet, Queries, QueryStatus, RecordedXcm, RemoteLockedFungibleRecord, + ShouldRecordXcm, VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, WeightInfo, }; +use bounded_collections::BoundedVec; use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, traits::{Currency, Hooks}, @@ -1258,6 +1262,168 @@ fn multistage_migration_works() { }) } +#[test] +fn migrate_data_to_xcm_version_works() { + new_test_ext_with_balances(vec![]).execute_with(|| { + // check `try-state` + assert!(Pallet::::do_try_state().is_ok()); + + let latest_version = XCM_VERSION; + let previous_version = XCM_VERSION - 1; + + // `Queries` migration + { + let origin = VersionedLocation::from(Location::parent()); + let query_id1 = 0; + let query_id2 = 2; + let query_as_latest = + QueryStatus::VersionNotifier { origin: origin.clone(), is_active: true }; + let query_as_previous = QueryStatus::VersionNotifier { + origin: origin.into_version(previous_version).unwrap(), + is_active: true, + }; + assert_ne!(query_as_latest, query_as_previous); + assert!(!query_as_latest.needs_migration(latest_version)); + assert!(!query_as_latest.needs_migration(previous_version)); + assert!(query_as_previous.needs_migration(latest_version)); + assert!(!query_as_previous.needs_migration(previous_version)); + + // store two queries: migrated and not migrated + Queries::::insert(query_id1, query_as_latest.clone()); + Queries::::insert(query_id2, query_as_previous); + assert!(Pallet::::do_try_state().is_ok()); + + // trigger migration + Pallet::::migrate_data_to_xcm_version(&mut Weight::zero(), latest_version); + + // no change for query_id1 + assert_eq!(Queries::::get(query_id1), Some(query_as_latest.clone())); + // change for query_id2 + assert_eq!(Queries::::get(query_id2), Some(query_as_latest)); + assert!(Pallet::::do_try_state().is_ok()); + } + + // `LockedFungibles` migration + { + let account1 = AccountId::new([13u8; 32]); + let account2 = AccountId::new([58u8; 32]); + let unlocker = VersionedLocation::from(Location::parent()); + let lockeds_as_latest = BoundedVec::truncate_from(vec![(0, unlocker.clone())]); + let lockeds_as_previous = BoundedVec::truncate_from(vec![( + 0, + unlocker.into_version(previous_version).unwrap(), + )]); + assert_ne!(lockeds_as_latest, lockeds_as_previous); + assert!(!lockeds_as_latest.needs_migration(latest_version)); + assert!(!lockeds_as_latest.needs_migration(previous_version)); + assert!(lockeds_as_previous.needs_migration(latest_version)); + assert!(!lockeds_as_previous.needs_migration(previous_version)); + + // store two lockeds: migrated and not migrated + LockedFungibles::::insert(&account1, lockeds_as_latest.clone()); + LockedFungibles::::insert(&account2, lockeds_as_previous); + assert!(Pallet::::do_try_state().is_ok()); + + // trigger migration + Pallet::::migrate_data_to_xcm_version(&mut Weight::zero(), latest_version); + + // no change for account1 + assert_eq!(LockedFungibles::::get(&account1), Some(lockeds_as_latest.clone())); + // change for account2 + assert_eq!(LockedFungibles::::get(&account2), Some(lockeds_as_latest)); + assert!(Pallet::::do_try_state().is_ok()); + } + + // `RemoteLockedFungibles` migration + { + let account1 = AccountId::new([13u8; 32]); + let account2 = AccountId::new([58u8; 32]); + let account3 = AccountId::new([97u8; 32]); + let asset_id = VersionedAssetId::from(AssetId(Location::parent())); + let owner = VersionedLocation::from(Location::parent()); + let locker = VersionedLocation::from(Location::parent()); + let key1_as_latest = (latest_version, account1, asset_id.clone()); + let key2_as_latest = (latest_version, account2, asset_id.clone()); + let key3_as_previous = ( + previous_version, + account3.clone(), + asset_id.clone().into_version(previous_version).unwrap(), + ); + let expected_key3_as_latest = (latest_version, account3, asset_id); + let data_as_latest = RemoteLockedFungibleRecord { + amount: Default::default(), + owner: owner.clone(), + locker: locker.clone(), + consumers: Default::default(), + }; + let data_as_previous = RemoteLockedFungibleRecord { + amount: Default::default(), + owner: owner.into_version(previous_version).unwrap(), + locker: locker.into_version(previous_version).unwrap(), + consumers: Default::default(), + }; + assert_ne!(data_as_latest.owner, data_as_previous.owner); + assert_ne!(data_as_latest.locker, data_as_previous.locker); + assert!(!key1_as_latest.needs_migration(latest_version)); + assert!(!key1_as_latest.needs_migration(previous_version)); + assert!(!key2_as_latest.needs_migration(latest_version)); + assert!(!key2_as_latest.needs_migration(previous_version)); + assert!(key3_as_previous.needs_migration(latest_version)); + assert!(!key3_as_previous.needs_migration(previous_version)); + assert!(!expected_key3_as_latest.needs_migration(latest_version)); + assert!(!expected_key3_as_latest.needs_migration(previous_version)); + assert!(!data_as_latest.needs_migration(latest_version)); + assert!(!data_as_latest.needs_migration(previous_version)); + assert!(data_as_previous.needs_migration(latest_version)); + assert!(!data_as_previous.needs_migration(previous_version)); + + // store three lockeds: + // fully migrated + RemoteLockedFungibles::::insert(&key1_as_latest, data_as_latest.clone()); + // only key migrated + RemoteLockedFungibles::::insert(&key2_as_latest, data_as_previous.clone()); + // neither key nor data migrated + RemoteLockedFungibles::::insert(&key3_as_previous, data_as_previous); + assert!(Pallet::::do_try_state().is_ok()); + + // trigger migration + Pallet::::migrate_data_to_xcm_version(&mut Weight::zero(), latest_version); + + let assert_locked_eq = + |left: Option>, + right: Option>| { + match (left, right) { + (None, Some(_)) | (Some(_), None) => + assert!(false, "Received unexpected message"), + (None, None) => (), + (Some(l), Some(r)) => { + assert_eq!(l.owner, r.owner); + assert_eq!(l.locker, r.locker); + }, + } + }; + + // no change + assert_locked_eq( + RemoteLockedFungibles::::get(&key1_as_latest), + Some(data_as_latest.clone()), + ); + // change - data migrated + assert_locked_eq( + RemoteLockedFungibles::::get(&key2_as_latest), + Some(data_as_latest.clone()), + ); + // fully migrated + assert_locked_eq(RemoteLockedFungibles::::get(&key3_as_previous), None); + assert_locked_eq( + RemoteLockedFungibles::::get(&expected_key3_as_latest), + Some(data_as_latest.clone()), + ); + assert!(Pallet::::do_try_state().is_ok()); + } + }) +} + #[test] fn record_xcm_works() { let balances = vec![(ALICE, INITIAL_BALANCE)]; diff --git a/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/mock.rs b/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/mock.rs index 4d9809e84f88..e6fe8e45c265 100644 --- a/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/mock.rs +++ b/polkadot/xcm/xcm-builder/src/asset_exchange/single_asset_adapter/mock.rs @@ -312,7 +312,7 @@ impl pallet_xcm::Config for Runtime { type UniversalLocation = UniversalLocation; // No version discovery needed const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 0; - type AdvertisedXcmVersion = frame_support::traits::ConstU32<3>; + type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion; type AdminOrigin = frame_system::EnsureRoot; // No locking type TrustedLockers = (); diff --git a/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs b/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs index d76ff21b8597..26ea226313f0 100644 --- a/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs +++ b/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs @@ -126,7 +126,6 @@ parameter_types! { pub const AnyNetwork: Option = None; pub UniversalLocation: InteriorLocation = (ByGenesis([0; 32]), Parachain(42)).into(); pub UnitWeightCost: u64 = 1_000; - pub static AdvertisedXcmVersion: u32 = 3; pub const BaseXcmWeight: Weight = Weight::from_parts(1_000, 1_000); pub CurrencyPerSecondPerByte: (AssetId, u128, u128) = (AssetId(RelayLocation::get()), 1, 1); pub TrustedAssets: (AssetFilter, Location) = (All.into(), Here.into()); @@ -267,7 +266,7 @@ impl pallet_xcm::Config for Test { type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 100; - type AdvertisedXcmVersion = AdvertisedXcmVersion; + type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion; type TrustedLockers = (); type SovereignAccountOf = SovereignAccountOf; type Currency = Balances; diff --git a/polkadot/xcm/xcm-runtime-apis/tests/mock.rs b/polkadot/xcm/xcm-runtime-apis/tests/mock.rs index b3afa23503e3..4a5de8875007 100644 --- a/polkadot/xcm/xcm-runtime-apis/tests/mock.rs +++ b/polkadot/xcm/xcm-runtime-apis/tests/mock.rs @@ -145,7 +145,6 @@ parameter_types! { pub const MaxInstructions: u32 = 100; pub const NativeTokenPerSecondPerByte: (AssetId, u128, u128) = (AssetId(HereLocation::get()), 1, 1); pub UniversalLocation: InteriorLocation = [GlobalConsensus(NetworkId::Westend), Parachain(2000)].into(); - pub static AdvertisedXcmVersion: XcmVersion = 4; pub const HereLocation: Location = Location::here(); pub const RelayLocation: Location = Location::parent(); pub const MaxAssetsIntoHolding: u32 = 64; @@ -349,7 +348,7 @@ impl pallet_xcm::Config for TestRuntime { type RuntimeOrigin = RuntimeOrigin; type RuntimeCall = RuntimeCall; const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 100; - type AdvertisedXcmVersion = AdvertisedXcmVersion; + type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion; type AdminOrigin = EnsureRoot; type TrustedLockers = (); type SovereignAccountOf = (); diff --git a/prdoc/pr_6148.prdoc b/prdoc/pr_6148.prdoc new file mode 100644 index 000000000000..430a58dfefbb --- /dev/null +++ b/prdoc/pr_6148.prdoc @@ -0,0 +1,17 @@ +title: Fix migrations for pallet-xcm +doc: +- audience: Runtime Dev + description: |- + `pallet-xcm` stores some operational data that uses `Versioned*` XCM types. When we add a new XCM version (XV), we deprecate XV-2 and remove XV-3. + This PR extends the existing `MigrateToLatestXcmVersion` to include migration for the `Queries`, `LockedFungibles`, and `RemoteLockedFungibles` storage types. + Additionally, more checks were added to `try_state` for these types. + +crates: +- name: westend-runtime + bump: patch +- name: staging-xcm-builder + bump: none +- name: xcm-runtime-apis + bump: none +- name: pallet-xcm + bump: patch