[pallet_xcm] Forgotten migration to XCMv4 + added try-state to the …

…`pallet_xcm` (paritytech#3228)

Relates to: paritytech#3214

## TODO

- [ ] backport to the `1.7.0` release

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<Runtime>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
 
 /// Migration to initialize storage versions for pallets added after genesis.

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<Runtime>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
 
 /// Asset Hub Westend has some undecodable storage, delete it.

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs

@@ -152,6 +152,8 @@ pub type Migrations = (
 		ConstU32<BRIDGE_HUB_ID>,
 		ConstU32<ASSET_HUB_ID>,
 	>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
 
 /// Migration to initialize storage versions for pallets added after genesis.

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<Runtime>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
 
 /// Migration to initialize storage versions for pallets added after genesis.

cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs

@@ -727,6 +727,8 @@ type Migrations = (
 	pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
 	// unreleased
 	cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
 
 /// Executive: handles dispatch to the various modules.

cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs

@@ -103,6 +103,8 @@ pub type Migrations = (
 	pallet_contracts::Migration<Runtime>,
 	// unreleased
 	cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 );
 
 type EventRecord = frame_system::EventRecord<

cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs

@@ -106,7 +106,11 @@ pub type UncheckedExtrinsic =
 	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
 
 /// Migrations to apply on runtime upgrade.
-pub type Migrations = (cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,);
+pub type Migrations = (
+	cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
+);
 
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<

cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs

@@ -97,7 +97,10 @@ pub type UncheckedExtrinsic =
 	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
 
 /// Migrations to apply on runtime upgrade.
-pub type Migrations = ();
+pub type Migrations = (
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
+);
 
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<

cumulus/parachains/runtimes/people/people-rococo/src/lib.rs

@@ -100,7 +100,10 @@ pub type UncheckedExtrinsic =
 	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
 
 /// Migrations to apply on runtime upgrade.
-pub type Migrations = ();
+pub type Migrations = (
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
+);
 
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<

cumulus/parachains/runtimes/people/people-westend/src/lib.rs

@@ -100,7 +100,10 @@ pub type UncheckedExtrinsic =
 	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
 
 /// Migrations to apply on runtime upgrade.
-pub type Migrations = ();
+pub type Migrations = (
+	// permanent
+	pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
+);
 
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<

polkadot/runtime/rococo/src/lib.rs

@@ -1662,6 +1662,9 @@ pub mod migrations {
 		parachains_configuration::migration::v11::MigrateToV11<Runtime>,
 		// This needs to come after the `parachains_configuration` above as we are reading the configuration.
 		coretime::migration::MigrateToCoretime<Runtime, crate::xcm_config::XcmRouter, GetLegacyLeaseImpl>,
+
+		// permanent
+		pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 	);
 }
 

polkadot/runtime/westend/src/lib.rs

@@ -1656,6 +1656,8 @@ pub mod migrations {
 		// Migrate Identity pallet for Usernames
 		pallet_identity::migration::versioned::V0ToV1<Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
 		parachains_configuration::migration::v11::MigrateToV11<Runtime>,
+		// permanent
+		pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
 	);
 }
 

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::<T>::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<T>) -> Result<(), TryRuntimeError> {
+			Self::do_try_state()
+		}
 	}
 
 	pub mod migrations {
@@ -2387,6 +2400,48 @@ impl<T: Config> Pallet<T> {
 		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::<T>::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::<T>::iter_prefix(v).next().is_none(),
+				TryRuntimeError::Other(
+					"`SupportedVersion` data should be migrated to the `XCM_VERSION`!`"
+				)
+			);
+			ensure!(
+				VersionNotifiers::<T>::iter_prefix(v).next().is_none(),
+				TryRuntimeError::Other(
+					"`VersionNotifiers` data should be migrated to the `XCM_VERSION`!`"
+				)
+			);
+			ensure!(
+				VersionNotifyTargets::<T>::iter_prefix(v).next().is_none(),
+				TryRuntimeError::Other(
+					"`VersionNotifyTargets` data should be migrated to the `XCM_VERSION`!`"
+				)
+			);
+		}
+
+		Ok(())
+	}
 }
 
 pub struct LockTicket<T: Config> {

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 <http://www.gnu.org/licenses/>.
 
-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 {
 		<T as frame_system::Config>::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<T>(sp_std::marker::PhantomData<T>);
+impl<T: Config> OnRuntimeUpgrade for MigrateToLatestXcmVersion<T> {
+	fn on_runtime_upgrade() -> Weight {
+		CurrentMigration::<T>::put(VersionMigrationStage::default());
+		T::DbWeight::get().writes(1)
+	}
+}

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::<Test>::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::<Test>::insert(v3_version, v3_location.clone(), v3_version);
+		VersionNotifiers::<Test>::insert(v3_version, v3_location.clone(), 1);
+		VersionNotifyTargets::<Test>::insert(
+			v3_version,
+			v3_location,
+			(70, Weight::zero(), v3_version),
+		);
+		// A version to advertise.
+		AdvertisedXcmVersion::set(4);
+
+		// check `try-state`
+		assert!(Pallet::<Test>::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::<Test>::put(VersionMigrationStage::default());
+			let mut maybe_migration = CurrentMigration::<Test>::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::<Test>::iter().collect::<Vec<_>>(),
+			vec![(XCM_VERSION, Parachain(1001).into_versioned(), v3_version),]
+		);
+		assert_eq!(
+			VersionNotifiers::<Test>::iter().collect::<Vec<_>>(),
+			vec![(XCM_VERSION, Parachain(1001).into_versioned(), 1),]
+		);
+		assert_eq!(
+			VersionNotifyTargets::<Test>::iter().collect::<Vec<_>>(),
+			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 * <Test as Config>::WeightInfo::already_notified_target());
+
+		// check no xcm sent
+		assert_eq!(take_sent_xcm(), vec![]);
+
+		// check `try-state`
+		assert!(Pallet::<Test>::do_try_state().is_ok());
+	})
+}

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<Runtime>` to ensure
+      proper data migration in `pallet_xcm`.
+
+crates: 
+  - name: pallet-xcm