Init System Parachain storage versions and add migration check jobs to CI (#1344)

liamaharon · web-flow · commit db3fd687262c · 2023-10-03T02:11:56.000+11:00
Makes SPs first class citizens along with the relay chains in the context of our CI runtime upgrade checks. ## Code changes - Sets missing current storage version in `uniques` pallet - Adds multisig V1 migration to run where it was missing - Removes executed migration whos pre/post hooks were failing from collectives runtime - Initializes storage versions for SP pallets added after genesis - Originally I was going to wait for #1297 to be merged so this wouldn't need to be done manually, but it doesn't seem like it'll be merged any time soon so I've decided to set them manually to unblock this ## CI changes - Removed dependency of `westend` runtime upgrades being complete prior to other ones running. I assume it is supposed to cache the `try-runtime` build for a performance benefit, but it seems it wasn't working. Maybe someone from the CI team can look into this or explain why it needs to be there? - Adds check-runtime-migration jobs for Parity asset-hubs, bridge-hubs and contract chains - Updated VARIABLES to accomodate the `kusama-runtime` package being renamed to `staging-kusama-runtime` in #1241 - Added `EXTRA_ARGS` variable to `check-runtime-migration`, and set `--no-weight-warnings` to the relay chain runtime upgrade checks (relay chains don't have weight restrictions).
diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml
@@ -113,13 +113,16 @@ test-rust-feature-propagation:
   script:
     - |
       export RUST_LOG=remote-ext=debug,runtime=debug
-      echo "---------- Running try-runtime for ${NETWORK} ----------"
-      time cargo install --locked --git https://github.com/paritytech/try-runtime-cli --rev a93c9b5abe5d31a4cf1936204f7e5c489184b521
-      time cargo build --release --locked -p "$NETWORK"-runtime --features try-runtime
+      echo "---------- Installing try-runtime-cli ----------"
+      time cargo install --locked --git https://github.com/paritytech/try-runtime-cli --tag v0.3.0
+      echo "---------- Building ${PACKAGE} runtime ----------"
+      time cargo build --release --locked -p "$PACKAGE" --features try-runtime
+      echo "---------- Executing `on-runtime-upgrade` for ${NETWORK} ----------"
       time try-runtime \
-          --runtime ./target/release/wbuild/"$NETWORK"-runtime/target/wasm32-unknown-unknown/release/"$NETWORK"_runtime.wasm \
-          on-runtime-upgrade --checks=pre-and-post live --uri wss://${NETWORK}-try-runtime-node.parity-chains.parity.io:443
+          --runtime ./target/release/wbuild/"$PACKAGE"/"$WASM" \
+          on-runtime-upgrade --checks=pre-and-post ${EXTRA_ARGS} live --uri ${URI}
 
+# Check runtime migrations for Parity managed relay chains
 check-runtime-migration-westend:
   stage: check
   extends:
@@ -128,19 +131,61 @@ check-runtime-migration-westend:
     - .check-runtime-migration
   variables:
     NETWORK: "westend"
+    PACKAGE: "westend-runtime"
+    WASM: "westend_runtime.compact.compressed.wasm"
+    URI: "wss://westend-try-runtime-node.parity-chains.parity.io:443"
+    EXTRA_ARGS: "--no-weight-warnings"
 
 check-runtime-migration-rococo:
   stage: check
-  # DAG
-  needs:
-    - job: check-runtime-migration-westend
-      artifacts: false
   extends:
     - .docker-env
     - .test-pr-refs
     - .check-runtime-migration
   variables:
     NETWORK: "rococo"
+    PACKAGE: "rococo-runtime"
+    WASM: "rococo_runtime.compact.compressed.wasm"
+    URI: "wss://rococo-try-runtime-node.parity-chains.parity.io:443"
+    EXTRA_ARGS: "--no-weight-warnings"
+
+# Check runtime migrations for Parity managed asset hub chains
+check-runtime-migration-asset-hub-westend:
+  stage: check
+  extends:
+    - .docker-env
+    - .test-pr-refs
+    - .check-runtime-migration
+  variables:
+    NETWORK: "asset-hub-westend"
+    PACKAGE: "asset-hub-westend-runtime"
+    WASM: "asset_hub_westend_runtime.compact.compressed.wasm"
+    URI: "wss://westend-asset-hub-rpc.polkadot.io:443"
+
+check-runtime-migration-bridge-hub-rococo:
+  stage: check
+  extends:
+    - .docker-env
+    - .test-pr-refs
+    - .check-runtime-migration
+  variables:
+    NETWORK: "bridge-hub-rococo"
+    PACKAGE: "bridge-hub-rococo-runtime"
+    WASM: "bridge_hub_rococo_runtime.compact.compressed.wasm"
+    URI: "wss://rococo-bridge-hub-rpc.polkadot.io:443"
+
+# Check runtime migrations for Parity managed contract chains
+check-runtime-migration-contracts-rococo:
+  stage: check
+  extends:
+    - .docker-env
+    - .test-pr-refs
+    - .check-runtime-migration
+  variables:
+    NETWORK: "contracts-rococo"
+    PACKAGE: "contracts-rococo-runtime"
+    WASM: "contracts_rococo_runtime.compact.compressed.wasm"
+    URI: "wss://rococo-contracts-rpc.polkadot.io:443"
 
 find-fail-ci-phrase:
   stage: check
diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
@@ -857,8 +857,46 @@ pub type Migrations = (
 	pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
 	// unreleased
 	migrations::NativeAssetParents0ToParents1Migration<Runtime>,
+	// unreleased
+	pallet_multisig::migrations::v1::MigrateToV1<Runtime>,
+	// unreleased
+	InitStorageVersions,
 );
 
+/// Migration to initialize storage versions for pallets added after genesis.
+///
+/// Ideally this would be done automatically (see
+/// <https://github.com/paritytech/polkadot-sdk/pull/1297>), but it probably won't be ready for some
+/// time and it's beneficial to get try-runtime-cli on-runtime-upgrade checks into the CI, so we're
+/// doing it manually.
+pub struct InitStorageVersions;
+
+impl frame_support::traits::OnRuntimeUpgrade for InitStorageVersions {
+	fn on_runtime_upgrade() -> Weight {
+		use frame_support::traits::{GetStorageVersion, StorageVersion};
+		use sp_runtime::traits::Saturating;
+
+		let mut writes = 0;
+
+		if PolkadotXcm::on_chain_storage_version() == StorageVersion::new(0) {
+			PolkadotXcm::current_storage_version().put::<PolkadotXcm>();
+			writes.saturating_inc();
+		}
+
+		if ForeignAssets::on_chain_storage_version() == StorageVersion::new(0) {
+			ForeignAssets::current_storage_version().put::<ForeignAssets>();
+			writes.saturating_inc();
+		}
+
+		if PoolAssets::on_chain_storage_version() == StorageVersion::new(0) {
+			PoolAssets::current_storage_version().put::<PoolAssets>();
+			writes.saturating_inc();
+		}
+
+		<Runtime as frame_system::Config>::DbWeight::get().reads_writes(3, writes)
+	}
+}
+
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<
 	Runtime,
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
@@ -123,7 +123,40 @@ pub type UncheckedExtrinsic =
 	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
 
 /// Migrations to apply on runtime upgrade.
-pub type Migrations = (pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,);
+pub type Migrations = (
+	pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
+	pallet_multisig::migrations::v1::MigrateToV1<Runtime>,
+	InitStorageVersions,
+);
+
+/// Migration to initialize storage versions for pallets added after genesis.
+///
+/// Ideally this would be done automatically (see
+/// <https://github.com/paritytech/polkadot-sdk/pull/1297>), but it probably won't be ready for some
+/// time and it's beneficial to get try-runtime-cli on-runtime-upgrade checks into the CI, so we're
+/// doing it manually.
+pub struct InitStorageVersions;
+
+impl frame_support::traits::OnRuntimeUpgrade for InitStorageVersions {
+	fn on_runtime_upgrade() -> Weight {
+		use frame_support::traits::{GetStorageVersion, StorageVersion};
+		use sp_runtime::traits::Saturating;
+
+		let mut writes = 0;
+
+		if PolkadotXcm::on_chain_storage_version() == StorageVersion::new(0) {
+			PolkadotXcm::current_storage_version().put::<PolkadotXcm>();
+			writes.saturating_inc();
+		}
+
+		if Balances::on_chain_storage_version() == StorageVersion::new(0) {
+			Balances::current_storage_version().put::<Balances>();
+			writes.saturating_inc();
+		}
+
+		<Runtime as frame_system::Config>::DbWeight::get().reads_writes(2, writes)
+	}
+}
 
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<
diff --git a/substrate/frame/multisig/src/migrations.rs b/substrate/frame/multisig/src/migrations.rs
@@ -43,16 +43,14 @@ pub mod v1 {
 	impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
 		#[cfg(feature = "try-runtime")]
 		fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
-			let onchain = Pallet::<T>::on_chain_storage_version();
-
-			ensure!(onchain < 1, "this migration can be deleted");
-
 			log!(info, "Number of calls to refund and delete: {}", Calls::<T>::iter().count());
 
 			Ok(Vec::new())
 		}
 
 		fn on_runtime_upgrade() -> Weight {
+			use sp_runtime::Saturating;
+
 			let current = Pallet::<T>::current_storage_version();
 			let onchain = Pallet::<T>::on_chain_storage_version();
 
@@ -61,20 +59,24 @@ pub mod v1 {
 				return T::DbWeight::get().reads(1)
 			}
 
+			let mut call_count = 0u64;
 			Calls::<T>::drain().for_each(|(_call_hash, (_data, caller, deposit))| {
 				T::Currency::unreserve(&caller, deposit);
+				call_count.saturating_inc();
 			});
 
 			current.put::<Pallet<T>>();
 
-			<T as frame_system::Config>::BlockWeights::get().max_block
+			T::DbWeight::get().reads_writes(
+				// Reads: Get Calls + Get Version
+				call_count.saturating_add(1),
+				// Writes: Drain Calls + Unreserves + Set version
+				call_count.saturating_mul(2).saturating_add(1),
+			)
 		}
 
 		#[cfg(feature = "try-runtime")]
 		fn post_upgrade(_state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
-			let onchain = Pallet::<T>::on_chain_storage_version();
-			ensure!(onchain < 2, "this migration needs to be removed");
-			ensure!(onchain == 1, "this migration needs to be run");
 			ensure!(
 				Calls::<T>::iter().count() == 0,
 				"there are some dangling calls that need to be destroyed and refunded"
diff --git a/substrate/frame/nfts/src/migration.rs b/substrate/frame/nfts/src/migration.rs
@@ -97,9 +97,6 @@ pub mod v1 {
 
 		#[cfg(feature = "try-runtime")]
 		fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
-			let current_version = Pallet::<T>::current_storage_version();
-			let onchain_version = Pallet::<T>::on_chain_storage_version();
-			ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1.");
 			let prev_count = Collection::<T>::iter().count();
 			Ok((prev_count as u32).encode())
 		}
@@ -115,7 +112,7 @@ pub mod v1 {
 				"the records count before and after the migration should be the same"
 			);
 
-			ensure!(Pallet::<T>::on_chain_storage_version() == 1, "wrong storage version");
+			ensure!(Pallet::<T>::on_chain_storage_version() >= 1, "wrong storage version");
 
 			Ok(())
 		}
diff --git a/substrate/frame/referenda/src/migration.rs b/substrate/frame/referenda/src/migration.rs
@@ -99,8 +99,6 @@ pub mod v1 {
 	impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for MigrateV0ToV1<T, I> {
 		#[cfg(feature = "try-runtime")]
 		fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
-			let onchain_version = Pallet::<T, I>::on_chain_storage_version();
-			ensure!(onchain_version == 0, "migration from version 0 to 1.");
 			let referendum_count = v0::ReferendumInfoFor::<T, I>::iter().count();
 			log::info!(
 				target: TARGET,
diff --git a/substrate/frame/uniques/src/lib.rs b/substrate/frame/uniques/src/lib.rs
@@ -69,7 +69,10 @@ pub mod pallet {
 	use frame_support::pallet_prelude::*;
 	use frame_system::pallet_prelude::*;
 
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+
 	#[pallet::pallet]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T, I = ()>(_);
 
 	#[cfg(feature = "runtime-benchmarks")]
