diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 2c111c0eface..553c70bc5549 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -968,6 +968,8 @@ pub type Migrations = ( InitStorageVersions, // unreleased cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Migration to initialize storage versions for pallets added after genesis. diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index aa319d9f8920..409a5f1177d8 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -949,6 +949,8 @@ pub type Migrations = ( DeleteUndecodableStorage, // unreleased cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Asset Hub Westend has some undecodable storage, delete it. diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs index 2be5652c4347..0d4c1defa37b 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs @@ -152,6 +152,8 @@ pub type Migrations = ( ConstU32, ConstU32, >, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Migration to initialize storage versions for pallets added after genesis. diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs index edfb08f163bf..6188b0acd7d1 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs @@ -124,6 +124,8 @@ pub type Migrations = ( InitStorageVersions, // unreleased cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Migration to initialize storage versions for pallets added after genesis. diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 6016955221c2..d245576494f8 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -727,6 +727,8 @@ type Migrations = ( pallet_collator_selection::migration::v1::MigrateToV1, // unreleased cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index b8203701d1a7..7d0dd43ea889 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -103,6 +103,8 @@ pub type Migrations = ( pallet_contracts::Migration, // unreleased cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); type EventRecord = frame_system::EventRecord< diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 1e880cf6cce6..8b952cfba229 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -106,7 +106,11 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4,); +pub type Migrations = ( + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 55fe3ba9fbf7..d8087c05b736 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -97,7 +97,10 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (); +pub type Migrations = ( + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs index 790be31da030..652485a69fdf 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs @@ -100,7 +100,10 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (); +pub type Migrations = ( + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< diff --git a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs index 24e203c9f013..b24f6767a717 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs @@ -100,7 +100,10 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (); +pub type Migrations = ( + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, +); /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 79c848221fbf..184d0338eb06 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1662,6 +1662,9 @@ pub mod migrations { parachains_configuration::migration::v11::MigrateToV11, // This needs to come after the `parachains_configuration` above as we are reading the configuration. coretime::migration::MigrateToCoretime, + + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index cd9e57a9bdf2..7305684a0d98 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1656,6 +1656,8 @@ pub mod migrations { // Migrate Identity pallet for Usernames pallet_identity::migration::versioned::V0ToV1, parachains_configuration::migration::v11::MigrateToV11, + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, ); } diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 55154198a9b2..586c275ae980 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -62,6 +62,9 @@ use xcm_executor::{ AssetsInHolding, }; +#[cfg(any(feature = "try-runtime", test))] +use sp_runtime::TryRuntimeError; + pub trait WeightInfo { fn send() -> Weight; fn teleport_assets() -> Weight; @@ -464,6 +467,8 @@ pub mod pallet { FeesPaid { paying: Location, fees: Assets }, /// Some assets have been claimed from an asset trap AssetsClaimed { hash: H256, origin: Location, assets: VersionedAssets }, + /// A XCM version migration finished. + VersionMigrationFinished { version: XcmVersion }, } #[pallet::origin] @@ -765,6 +770,9 @@ pub mod pallet { // Consume 10% of block at most let max_weight = T::BlockWeights::get().max_block / 10; let (w, maybe_migration) = Self::check_xcm_version_change(migration, max_weight); + if maybe_migration.is_none() { + Self::deposit_event(Event::VersionMigrationFinished { version: XCM_VERSION }); + } CurrentMigration::::set(maybe_migration); weight_used.saturating_accrue(w); } @@ -791,6 +799,11 @@ pub mod pallet { } weight_used } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { + Self::do_try_state() + } } pub mod migrations { @@ -2387,6 +2400,48 @@ impl Pallet { Self::deposit_event(Event::FeesPaid { paying: location, fees: assets }); Ok(()) } + + /// Ensure the correctness of the state of this pallet. + /// + /// This should be valid before and after each state transition of this pallet. + /// + /// ## Invariants + /// + /// All entries stored in the `SupportedVersion` / `VersionNotifiers` / `VersionNotifyTargets` + /// need to be migrated to the `XCM_VERSION`. If they are not, then `CurrentMigration` has to be + /// set. + #[cfg(any(feature = "try-runtime", test))] + pub fn do_try_state() -> Result<(), TryRuntimeError> { + // if migration has been already scheduled, everything is ok and data will be eventually + // migrated + if CurrentMigration::::exists() { + return Ok(()) + } + + // if migration has NOT been scheduled yet, we need to check all operational data + for v in 0..XCM_VERSION { + ensure!( + SupportedVersion::::iter_prefix(v).next().is_none(), + TryRuntimeError::Other( + "`SupportedVersion` data should be migrated to the `XCM_VERSION`!`" + ) + ); + ensure!( + VersionNotifiers::::iter_prefix(v).next().is_none(), + TryRuntimeError::Other( + "`VersionNotifiers` data should be migrated to the `XCM_VERSION`!`" + ) + ); + ensure!( + VersionNotifyTargets::::iter_prefix(v).next().is_none(), + TryRuntimeError::Other( + "`VersionNotifyTargets` data should be migrated to the `XCM_VERSION`!`" + ) + ); + } + + Ok(()) + } } pub struct LockTicket { diff --git a/polkadot/xcm/pallet-xcm/src/migration.rs b/polkadot/xcm/pallet-xcm/src/migration.rs index 2793afcc9104..018436aa3c93 100644 --- a/polkadot/xcm/pallet-xcm/src/migration.rs +++ b/polkadot/xcm/pallet-xcm/src/migration.rs @@ -14,7 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use crate::{Config, Pallet, VersionNotifyTargets}; +use crate::{ + pallet::CurrentMigration, Config, Pallet, VersionMigrationStage, VersionNotifyTargets, +}; use frame_support::{ pallet_prelude::*, traits::{OnRuntimeUpgrade, StorageVersion}, @@ -73,3 +75,16 @@ pub mod v1 { ::DbWeight, >; } + +/// When adding a new XCM version, we need to run this migration for `pallet_xcm` to ensure that all +/// previously stored data with subkey prefix `XCM_VERSION-1` (and below) are migrated to the +/// `XCM_VERSION`. +/// +/// NOTE: This migration can be permanently added to the runtime migrations. +pub struct MigrateToLatestXcmVersion(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for MigrateToLatestXcmVersion { + fn on_runtime_upgrade() -> Weight { + CurrentMigration::::put(VersionMigrationStage::default()); + T::DbWeight::get().writes(1) + } +} diff --git a/polkadot/xcm/pallet-xcm/src/tests/mod.rs b/polkadot/xcm/pallet-xcm/src/tests/mod.rs index 5f9c86ed7b3f..e5ac80c3a142 100644 --- a/polkadot/xcm/pallet-xcm/src/tests/mod.rs +++ b/polkadot/xcm/pallet-xcm/src/tests/mod.rs @@ -19,8 +19,9 @@ pub(crate) mod assets_transfer; use crate::{ - mock::*, AssetTraps, CurrentMigration, Error, LatestVersionedLocation, Queries, QueryStatus, - VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, + mock::*, pallet::SupportedVersion, AssetTraps, Config, CurrentMigration, Error, + LatestVersionedLocation, Pallet, Queries, QueryStatus, VersionDiscoveryQueue, + VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, WeightInfo, }; use frame_support::{ assert_noop, assert_ok, @@ -1113,3 +1114,83 @@ fn get_and_wrap_version_works() { assert_eq!(VersionDiscoveryQueue::::get().into_inner(), vec![(remote_b.into(), 2)]); }) } + +#[test] +fn multistage_migration_works() { + new_test_ext_with_balances(vec![]).execute_with(|| { + // An entry from a previous runtime with v3 XCM. + let v3_location = VersionedLocation::V3(xcm::v3::Junction::Parachain(1001).into()); + let v3_version = xcm::v3::VERSION; + SupportedVersion::::insert(v3_version, v3_location.clone(), v3_version); + VersionNotifiers::::insert(v3_version, v3_location.clone(), 1); + VersionNotifyTargets::::insert( + v3_version, + v3_location, + (70, Weight::zero(), v3_version), + ); + // A version to advertise. + AdvertisedXcmVersion::set(4); + + // check `try-state` + assert!(Pallet::::do_try_state().is_err()); + + // closure simulates a multistage migration process + let migrate = |expected_cycle_count| { + // A runtime upgrade which alters the version does send notifications. + CurrentMigration::::put(VersionMigrationStage::default()); + let mut maybe_migration = CurrentMigration::::take(); + let mut counter = 0; + let mut weight_used = Weight::zero(); + while let Some(migration) = maybe_migration.take() { + counter += 1; + let (w, m) = XcmPallet::check_xcm_version_change(migration, Weight::zero()); + maybe_migration = m; + weight_used.saturating_accrue(w); + } + assert_eq!(counter, expected_cycle_count); + weight_used + }; + + // run migration for the first time + let _ = migrate(4); + + // check xcm sent + assert_eq!( + take_sent_xcm(), + vec![( + Parachain(1001).into(), + Xcm(vec![QueryResponse { + query_id: 70, + max_weight: Weight::zero(), + response: Response::Version(AdvertisedXcmVersion::get()), + querier: None, + }]) + ),] + ); + + // check migrated data + assert_eq!( + SupportedVersion::::iter().collect::>(), + vec![(XCM_VERSION, Parachain(1001).into_versioned(), v3_version),] + ); + assert_eq!( + VersionNotifiers::::iter().collect::>(), + vec![(XCM_VERSION, Parachain(1001).into_versioned(), 1),] + ); + assert_eq!( + VersionNotifyTargets::::iter().collect::>(), + vec![(XCM_VERSION, Parachain(1001).into_versioned(), (70, Weight::zero(), 4)),] + ); + + // run migration again to check it can run multiple time without any harm or double sending + // messages. + let weight_used = migrate(1); + assert_eq!(weight_used, 1_u8 * ::WeightInfo::already_notified_target()); + + // check no xcm sent + assert_eq!(take_sent_xcm(), vec![]); + + // check `try-state` + assert!(Pallet::::do_try_state().is_ok()); + }) +} diff --git a/prdoc/pr_3228.prdoc b/prdoc/pr_3228.prdoc new file mode 100644 index 000000000000..42a4893af362 --- /dev/null +++ b/prdoc/pr_3228.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "[pallet_xcm] Forgotten migration to XCMv4 + added `try-state` to the `pallet_xcm`" + +doc: + - audience: Runtime Dev + description: | + The current release includes the new `XCMv4`, so the runtimes must incorporate + a migration `pallet_xcm::migration::MigrateToLatestXcmVersion` to ensure + proper data migration in `pallet_xcm`. + +crates: + - name: pallet-xcm