Remove xcm on_runtime_upgrade pallet hook (#7235)

* move migration stuffs

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* Fix test

* fix

* lint

* fix lint

* rm extra space

* fix lint

* PR review

* fixes

* use saturating_accrue in fn

* fix test

---------

Co-authored-by: Bastian Köcher <git@kchr.de>

polkadot/xcm/pallet-xcm/src/lib.rs

@@ -689,12 +689,6 @@ pub mod pallet {
 			}
 			weight_used
 		}
-		fn on_runtime_upgrade() -> Weight {
-			// Start a migration (this happens before on_initialize so it'll happen later in this
-			// block, which should be good enough)...
-			CurrentMigration::<T>::put(VersionMigrationStage::default());
-			T::DbWeight::get().writes(1)
-		}
 	}
 
 	pub mod migrations {

polkadot/xcm/pallet-xcm/src/migration.rs

@@ -25,6 +25,7 @@ const DEFAULT_PROOF_SIZE: u64 = 64 * 1024;
 
 pub mod v1 {
 	use super::*;
+	use crate::{CurrentMigration, VersionMigrationStage};
 
 	/// Named with the 'VersionUnchecked'-prefix because although this implements some version
 	/// checking, the version checking is not complete as it will begin failing after the upgrade is
@@ -33,19 +34,19 @@ pub mod v1 {
 	/// Use experimental [`VersionCheckedMigrateToV1`] instead.
 	pub struct VersionUncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);
 	impl<T: Config> OnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
-		#[cfg(feature = "try-runtime")]
-		fn pre_upgrade() -> Result<sp_std::vec::Vec<u8>, sp_runtime::TryRuntimeError> {
-			ensure!(StorageVersion::get::<Pallet<T>>() == 0, "must upgrade linearly");
-
-			Ok(sp_std::vec::Vec::new())
-		}
-
 		fn on_runtime_upgrade() -> Weight {
-			if StorageVersion::get::<Pallet<T>>() == 0 {
 			let mut weight = T::DbWeight::get().reads(1);
 
+			if StorageVersion::get::<Pallet<T>>() != 0 {
+				log::warn!("skipping v1, should be removed");
+				return weight
+			}
+
+			weight.saturating_accrue(T::DbWeight::get().writes(1));
+			CurrentMigration::<T>::put(VersionMigrationStage::default());
+
 			let translate = |pre: (u64, u64, u32)| -> Option<(u64, Weight, u32)> {
-					weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
+				weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
 				let translated = (pre.0, Weight::from_parts(pre.1, DEFAULT_PROOF_SIZE), pre.2);
 				log::info!("Migrated VersionNotifyTarget {:?} to {:?}", pre, translated);
 				Some(translated)
@@ -54,13 +55,9 @@ pub mod v1 {
 			VersionNotifyTargets::<T>::translate_values(translate);
 
 			log::info!("v1 applied successfully");
+			weight.saturating_accrue(T::DbWeight::get().writes(1));
 			StorageVersion::new(1).put::<Pallet<T>>();
-
+			weight
-				weight.saturating_add(T::DbWeight::get().writes(1))
-			} else {
-				log::warn!("skipping v1, should be removed");
-				T::DbWeight::get().reads(1)
-			}
 		}
 	}
 

polkadot/xcm/pallet-xcm/src/tests.rs

@@ -16,7 +16,8 @@
 
 use crate::{
 	mock::*, AssetTraps, CurrentMigration, Error, LatestVersionedMultiLocation, Queries,
-	QueryStatus, VersionDiscoveryQueue, VersionNotifiers, VersionNotifyTargets,
+	QueryStatus, VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers,
+	VersionNotifyTargets,
 };
 use frame_support::{
 	assert_noop, assert_ok,
@@ -897,7 +898,7 @@ fn subscription_side_works() {
 		assert_eq!(take_sent_xcm(), vec![(remote.clone(), Xcm(vec![instr]))]);
 
 		// A runtime upgrade which doesn't alter the version sends no notifications.
-		XcmPallet::on_runtime_upgrade();
+		CurrentMigration::<Test>::put(VersionMigrationStage::default());
 		XcmPallet::on_initialize(1);
 		assert_eq!(take_sent_xcm(), vec![]);
 
@@ -905,7 +906,7 @@ fn subscription_side_works() {
 		AdvertisedXcmVersion::set(2);
 
 		// A runtime upgrade which alters the version does send notifications.
-		XcmPallet::on_runtime_upgrade();
+		CurrentMigration::<Test>::put(VersionMigrationStage::default());
 		XcmPallet::on_initialize(2);
 		let instr = QueryResponse {
 			query_id: 0,
@@ -932,7 +933,7 @@ fn subscription_side_upgrades_work_with_notify() {
 		AdvertisedXcmVersion::set(3);
 
 		// A runtime upgrade which alters the version does send notifications.
-		XcmPallet::on_runtime_upgrade();
+		CurrentMigration::<Test>::put(VersionMigrationStage::default());
 		XcmPallet::on_initialize(1);
 
 		let instr1 = QueryResponse {
@@ -982,7 +983,7 @@ fn subscription_side_upgrades_work_without_notify() {
 		VersionNotifyTargets::<Test>::insert(3, v3_location, (72, Weight::zero(), 2));
 
 		// A runtime upgrade which alters the version does send notifications.
-		XcmPallet::on_runtime_upgrade();
+		CurrentMigration::<Test>::put(VersionMigrationStage::default());
 		XcmPallet::on_initialize(1);
 
 		let mut contents = VersionNotifyTargets::<Test>::iter().collect::<Vec<_>>();
@@ -1166,7 +1167,7 @@ fn subscription_side_upgrades_work_with_multistage_notify() {
 		AdvertisedXcmVersion::set(3);
 
 		// A runtime upgrade which alters the version does send notifications.
-		XcmPallet::on_runtime_upgrade();
+		CurrentMigration::<Test>::put(VersionMigrationStage::default());
 		let mut maybe_migration = CurrentMigration::<Test>::take();
 		let mut counter = 0;
 		while let Some(migration) = maybe_migration.take() {