From db3fd687262c68b115ab6724dfaa6a71d4a48a59 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 3 Oct 2023 02:11:56 +1100 Subject: [PATCH] Init System Parachain storage versions and add migration check jobs to CI (#1344) 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 https://github.com/paritytech/polkadot-sdk/pull/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 https://github.com/paritytech/polkadot-sdk/pull/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). --- .gitlab/pipeline/check.yml | 63 ++++++++++++++++--- .../assets/asset-hub-westend/src/lib.rs | 38 +++++++++++ .../bridge-hubs/bridge-hub-rococo/src/lib.rs | 35 ++++++++++- substrate/frame/multisig/src/migrations.rs | 18 +++--- substrate/frame/nfts/src/migration.rs | 5 +- substrate/frame/referenda/src/migration.rs | 2 - substrate/frame/uniques/src/lib.rs | 3 + 7 files changed, 140 insertions(+), 24 deletions(-) diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index 5cc2337bf409..4f92e6c15d2b 100644 --- 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 index 759cd727f1dc..943332087627 100644 --- 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, // unreleased migrations::NativeAssetParents0ToParents1Migration, + // unreleased + pallet_multisig::migrations::v1::MigrateToV1, + // unreleased + InitStorageVersions, ); +/// Migration to initialize storage versions for pallets added after genesis. +/// +/// Ideally this would be done automatically (see +/// ), 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::(); + writes.saturating_inc(); + } + + if ForeignAssets::on_chain_storage_version() == StorageVersion::new(0) { + ForeignAssets::current_storage_version().put::(); + writes.saturating_inc(); + } + + if PoolAssets::on_chain_storage_version() == StorageVersion::new(0) { + PoolAssets::current_storage_version().put::(); + writes.saturating_inc(); + } + + ::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 index 4c850f92b8de..c4e510ee4095 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 @@ -123,7 +123,40 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (pallet_collator_selection::migration::v1::MigrateToV1,); +pub type Migrations = ( + pallet_collator_selection::migration::v1::MigrateToV1, + pallet_multisig::migrations::v1::MigrateToV1, + InitStorageVersions, +); + +/// Migration to initialize storage versions for pallets added after genesis. +/// +/// Ideally this would be done automatically (see +/// ), 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::(); + writes.saturating_inc(); + } + + if Balances::on_chain_storage_version() == StorageVersion::new(0) { + Balances::current_storage_version().put::(); + writes.saturating_inc(); + } + + ::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 index 3be55080b240..330613bb3dfd 100644 --- a/substrate/frame/multisig/src/migrations.rs +++ b/substrate/frame/multisig/src/migrations.rs @@ -43,16 +43,14 @@ pub mod v1 { impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { - let onchain = Pallet::::on_chain_storage_version(); - - ensure!(onchain < 1, "this migration can be deleted"); - log!(info, "Number of calls to refund and delete: {}", Calls::::iter().count()); Ok(Vec::new()) } fn on_runtime_upgrade() -> Weight { + use sp_runtime::Saturating; + let current = Pallet::::current_storage_version(); let onchain = Pallet::::on_chain_storage_version(); @@ -61,20 +59,24 @@ pub mod v1 { return T::DbWeight::get().reads(1) } + let mut call_count = 0u64; Calls::::drain().for_each(|(_call_hash, (_data, caller, deposit))| { T::Currency::unreserve(&caller, deposit); + call_count.saturating_inc(); }); current.put::>(); - ::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) -> Result<(), sp_runtime::TryRuntimeError> { - let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 2, "this migration needs to be removed"); - ensure!(onchain == 1, "this migration needs to be run"); ensure!( Calls::::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 index f90d332062a2..94635a96aeba 100644 --- 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, TryRuntimeError> { - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); - ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1."); let prev_count = Collection::::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::::on_chain_storage_version() == 1, "wrong storage version"); + ensure!(Pallet::::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 index 281da83d6569..a80897242eec 100644 --- a/substrate/frame/referenda/src/migration.rs +++ b/substrate/frame/referenda/src/migration.rs @@ -99,8 +99,6 @@ pub mod v1 { impl, I: 'static> OnRuntimeUpgrade for MigrateV0ToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { - let onchain_version = Pallet::::on_chain_storage_version(); - ensure!(onchain_version == 0, "migration from version 0 to 1."); let referendum_count = v0::ReferendumInfoFor::::iter().count(); log::info!( target: TARGET, diff --git a/substrate/frame/uniques/src/lib.rs b/substrate/frame/uniques/src/lib.rs index 1b75d0b078ba..8334a8d943e1 100644 --- 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(_); #[cfg(feature = "runtime-benchmarks")]