From 7ec7cc947c8cee72ae10333ad45fa1c82204aee1 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 23 Apr 2024 11:37:57 +0400 Subject: [PATCH 01/29] wip mbm try-runtime --- Cargo.lock | 26 ++++ .../assets/asset-hub-rococo/src/lib.rs | 2 +- .../assets/asset-hub-westend/src/lib.rs | 2 +- .../bridge-hubs/bridge-hub-rococo/Cargo.toml | 10 ++ .../bridge-hubs/bridge-hub-rococo/src/lib.rs | 23 +++- .../bridge-hubs/bridge-hub-westend/src/lib.rs | 2 +- .../collectives-westend/src/lib.rs | 2 +- .../contracts/contracts-rococo/src/lib.rs | 2 +- .../coretime/coretime-rococo/src/lib.rs | 2 +- .../coretime/coretime-westend/src/lib.rs | 2 +- .../runtimes/people/people-rococo/src/lib.rs | 2 +- .../runtimes/people/people-westend/src/lib.rs | 2 +- .../runtimes/testing/penpal/src/lib.rs | 2 +- .../asset_hub_polkadot_aura.rs | 2 +- .../src/fake_runtime_api/aura.rs | 2 +- polkadot/runtime/rococo/src/lib.rs | 8 +- polkadot/runtime/westend/src/lib.rs | 8 +- substrate/bin/node/runtime/src/lib.rs | 2 +- .../multi-block-migrations/Cargo.toml | 2 + .../src/migrations/v1/mod.rs | 28 ++++ substrate/frame/executive/src/lib.rs | 19 ++- substrate/frame/executive/src/tests.rs | 5 + substrate/frame/migrations/src/lib.rs | 106 ++++++++++++++- .../frame/migrations/src/mock_helpers.rs | 30 +++++ substrate/frame/migrations/src/tests.rs | 124 ++++++++++++++++++ substrate/frame/support/Cargo.toml | 1 + substrate/frame/support/src/migrations.rs | 93 +++++++++++++ substrate/frame/support/src/traits.rs | 4 +- .../support/src/traits/try_runtime/mod.rs | 56 ++------ substrate/frame/support/test/tests/pallet.rs | 19 ++- substrate/frame/system/src/mock.rs | 5 + substrate/frame/try-runtime/src/inner.rs | 8 +- templates/parachain/runtime/src/apis.rs | 2 +- templates/solochain/runtime/src/lib.rs | 4 +- 34 files changed, 517 insertions(+), 90 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7bf5215b6dec..2270bf6c520d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2103,7 +2103,9 @@ dependencies = [ "pallet-bridge-parachains", "pallet-bridge-relayers", "pallet-collator-selection", + "pallet-example-mbm", "pallet-message-queue", + "pallet-migrations", "pallet-multisig", "pallet-session", "pallet-timestamp", @@ -2387,6 +2389,28 @@ dependencies = [ "semver 0.6.0", ] +[[package]] +name = "builder-pattern" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d85376b93d8efe18dd819f56505e33e7a9c0f93fb02bd761f8690026178ed6e5" +dependencies = [ + "builder-pattern-macro", + "futures", +] + +[[package]] +name = "builder-pattern-macro" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47d624ef88b39588d113f807ffb38ee968aafc388ca57dd7a9a7b82d3de1f5f4" +dependencies = [ + "bitflags 1.3.2", + "proc-macro2 1.0.75", + "quote 1.0.35", + "syn 1.0.109", +] + [[package]] name = "bumpalo" version = "3.13.0" @@ -5953,6 +5977,7 @@ dependencies = [ "array-bytes 6.1.0", "assert_matches", "bitflags 1.3.2", + "builder-pattern", "docify", "environmental", "frame-metadata", @@ -10429,6 +10454,7 @@ dependencies = [ "parity-scale-codec", "scale-info", "sp-io", + "sp-std 14.0.0", ] [[package]] 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 5cb29343a1cf..8c93733d7255 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1286,7 +1286,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } 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 366fb91723ae..83531dee00e1 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1377,7 +1377,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml index f5a75aa03acd..2478750a9128 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml @@ -41,6 +41,8 @@ pallet-timestamp = { path = "../../../../../substrate/frame/timestamp", default- pallet-transaction-payment = { path = "../../../../../substrate/frame/transaction-payment", default-features = false } pallet-transaction-payment-rpc-runtime-api = { path = "../../../../../substrate/frame/transaction-payment/rpc/runtime-api", default-features = false } pallet-utility = { path = "../../../../../substrate/frame/utility", default-features = false } +pallet-example-mbm = { path = "../../../../../substrate/frame/examples/multi-block-migrations", default-features = false } +pallet-migrations = { path = "../../../../../substrate/frame/migrations", default-features = false } sp-api = { path = "../../../../../substrate/primitives/api", default-features = false } sp-block-builder = { path = "../../../../../substrate/primitives/block-builder", default-features = false } sp-consensus-aura = { path = "../../../../../substrate/primitives/consensus/aura", default-features = false } @@ -121,6 +123,8 @@ snowbridge-runtime-common = { path = "../../../../../bridges/snowbridge/runtime/ bridge-hub-common = { path = "../common", default-features = false } + + [dev-dependencies] static_assertions = "1.1" bridge-hub-test-utils = { path = "../test-utils" } @@ -221,6 +225,8 @@ std = [ "xcm-builder/std", "xcm-executor/std", "xcm/std", + "pallet-example-mbm/std", + "pallet-migrations/std" ] runtime-benchmarks = [ @@ -262,6 +268,8 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", "xcm-builder/runtime-benchmarks", "xcm-executor/runtime-benchmarks", + "pallet-example-mbm/runtime-benchmarks", + "pallet-migrations/runtime-benchmarks" ] try-runtime = [ @@ -296,6 +304,8 @@ try-runtime = [ "snowbridge-pallet-outbound-queue/try-runtime", "snowbridge-pallet-system/try-runtime", "sp-runtime/try-runtime", + "pallet-example-mbm/try-runtime", + "pallet-migrations/try-runtime" ] fast-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 1eac813b10ce..1d212f69aa65 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 @@ -62,6 +62,7 @@ use frame_support::{ construct_runtime, derive_impl, dispatch::DispatchClass, genesis_builder_helper::{build_state, get_preset}, + migrations::FreezeChainOnFailedMigration, parameter_types, traits::{ConstBool, ConstU32, ConstU64, ConstU8, TransformOrigin}, weights::{ConstantMultiplier, Weight}, @@ -272,6 +273,7 @@ impl frame_system::Config for Runtime { /// The action to take on a Runtime Upgrade type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode; type MaxConsumers = frame_support::traits::ConstU32<16>; + type MultiBlockMigrator = PalletMigrations; } impl pallet_timestamp::Config for Runtime { @@ -662,6 +664,8 @@ impl snowbridge_pallet_system::Config for Runtime { type InboundDeliveryCost = EthereumInboundQueue; } +impl pallet_example_mbm::Config for Runtime {} + // Create the runtime by composing the FRAME pallets that were previously configured. construct_runtime!( pub enum Runtime @@ -727,9 +731,26 @@ construct_runtime!( // Message Queue. Importantly, is registered last so that messages are processed after // the `on_initialize` hooks of bridging pallets. MessageQueue: pallet_message_queue = 175, + + PalletMigrations: pallet_migrations = 200, } ); +frame_support::parameter_types! { + pub MbmServiceWeight: Weight = Perbill::from_percent(80) * RuntimeBlockWeights::get().max_block; +} + +impl pallet_migrations::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type MaxServiceWeight = MbmServiceWeight; + type WeightInfo = (); + type Migrations = (); + type CursorMaxLen = ConstU32<65_536>; + type IdentifierMaxLen = ConstU32<256>; + type MigrationStatusHandler = (); + type FailedMigrationHandler = FreezeChainOnFailedMigration; +} + /// Proper alias for bridge GRANDPA pallet used to bridge with the bulletin chain. pub type BridgeRococoBulletinGrandpa = BridgePolkadotBulletinGrandpa; /// Proper alias for bridge messages pallet used to bridge with the bulletin chain. @@ -1033,7 +1054,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } 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 b4ea2c79f64f..a19b904d905e 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 @@ -733,7 +733,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index c599ba37f128..c8b4c9575f00 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -927,7 +927,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index efa26fcbc22d..6897ba1642ef 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -658,7 +658,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index ad065ee34774..779b9a7ebb97 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -652,7 +652,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 0f0742268618..9fd821318c73 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -644,7 +644,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs index 3cd085fec632..e07baa1e5845 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs @@ -619,7 +619,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs index 307ab90a4772..b9f6561e53e3 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs @@ -619,7 +619,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs index 89885d77378b..816db7a09cf1 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs @@ -837,7 +837,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs b/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs index 82c02943c5fc..ac5bc57ff440 100644 --- a/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs +++ b/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs @@ -152,7 +152,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(_: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(_: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { unimplemented!() } diff --git a/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs b/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs index 6b718e912164..1368d66ef2fe 100644 --- a/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs +++ b/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs @@ -152,7 +152,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(_: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(_: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { unimplemented!() } diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index ba80fa6942c7..e411fcc33d39 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -2229,7 +2229,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { log::info!("try-runtime::on_runtime_upgrade rococo."); let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, BlockWeights::get().max_block) @@ -2500,7 +2500,7 @@ sp_api::impl_runtime_apis! { #[cfg(all(test, feature = "try-runtime"))] mod remote_tests { use super::*; - use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect}; + use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, TryOnRuntimeUpgradeOpts}; use remote_externalities::{ Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport, }; @@ -2532,6 +2532,8 @@ mod remote_tests { .build() .await .unwrap(); - ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost)); + ext.execute_with(|| { + Runtime::on_runtime_upgrade(TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build()) + }); } } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index a06a1e1f7fc8..11ca20f04040 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -2281,7 +2281,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { log::info!("try-runtime::on_runtime_upgrade westend."); let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, BlockWeights::get().max_block) @@ -2564,7 +2564,7 @@ sp_api::impl_runtime_apis! { #[cfg(all(test, feature = "try-runtime"))] mod remote_tests { use super::*; - use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect}; + use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, TryOnRuntimeUpgradeOpts}; use remote_externalities::{ Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport, }; @@ -2596,7 +2596,9 @@ mod remote_tests { .build() .await .unwrap(); - ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost)); + ext.execute_with(|| { + Runtime::on_runtime_upgrade(TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build()) + }); } } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 43c617023bcb..58bbfe41250c 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -3157,7 +3157,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to // have a backtrace here. If any of the pre/post migration checks fail, we shall stop // right here and right now. diff --git a/substrate/frame/examples/multi-block-migrations/Cargo.toml b/substrate/frame/examples/multi-block-migrations/Cargo.toml index 28eca8577154..f77fbf87dd29 100644 --- a/substrate/frame/examples/multi-block-migrations/Cargo.toml +++ b/substrate/frame/examples/multi-block-migrations/Cargo.toml @@ -21,6 +21,7 @@ frame-benchmarking = { path = "../../benchmarking", default-features = false, op log = { version = "0.4.20", default-features = false } scale-info = { version = "2.10.0", default-features = false } sp-io = { path = "../../../primitives/io", default-features = false } +sp-std = { path = "../../../primitives/std", default-features = false } [features] default = ["std"] @@ -33,6 +34,7 @@ std = [ "pallet-migrations/std", "scale-info/std", "sp-io/std", + "sp-std/std" ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index 2016b03de45e..a8afa4a0b085 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -29,6 +29,9 @@ use frame_support::{ weights::WeightMeter, }; +#[cfg(feature = "try-runtime")] +use sp_std::collections::btree_map::BTreeMap; + mod benchmarks; mod tests; pub mod weights; @@ -115,4 +118,29 @@ impl SteppedMigration for LazyMigrationV1 Result, frame_support::sp_runtime::TryRuntimeError> { + use codec::Encode; + + // Return the state of the storage before the migration. + Ok(v0::MyMap::::iter().collect::>().encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev: Vec) -> Result<(), frame_support::sp_runtime::TryRuntimeError> { + use codec::Decode; + + // Check the state of the storage after the migration. + let prev_map = BTreeMap::::decode(&mut &prev[..]) + .expect("Failed to decode the previous storage state"); + + for (key, value) in MyMap::::iter() { + let prev_value = + prev_map.get(&key).expect("Key not found in the previous storage state"); + assert_eq!(value as u32, *prev_value, "Migration failed for key {}", key); + } + + Ok(()) + } } diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 3028eaf318e0..0614f52d8319 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -179,10 +179,12 @@ use sp_std::{marker::PhantomData, prelude::*}; #[cfg(feature = "try-runtime")] use ::{ frame_support::{ - traits::{TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState}, + traits::{ + TryDecodeEntireStorage, TryDecodeEntireStorageError, TryOnRuntimeUpgradeOpts, TryState, + }, StorageNoopGuard, }, - frame_try_runtime::{TryStateSelect, UpgradeCheckSelect}, + frame_try_runtime::TryStateSelect, log, sp_runtime::TryRuntimeError, }; @@ -431,12 +433,12 @@ where /// [`frame_system::LastRuntimeUpgrade`] is set to the current runtime version after /// migrations execute. This is important for idempotency checks, because some migrations use /// this value to determine whether or not they should execute. - pub fn try_runtime_upgrade(checks: UpgradeCheckSelect) -> Result { + pub fn try_runtime_upgrade(opts: TryOnRuntimeUpgradeOpts) -> Result { let before_all_weight = ::before_all_runtime_migrations(); let try_on_runtime_upgrade_weight = <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade( - checks.pre_and_post(), + opts.pre_and_post, )?; frame_system::LastRuntimeUpgrade::::put( @@ -449,19 +451,24 @@ where let _guard = StorageNoopGuard::default(); // The state must be decodable: - if checks.any() { + if opts.decode_entire_state { let res = AllPalletsWithSystem::try_decode_entire_state(); Self::log_decode_result(res)?; } // Check all storage invariants: - if checks.try_state() { + if opts.try_state { AllPalletsWithSystem::try_state( frame_system::Pallet::::block_number(), TryStateSelect::All, )?; } + // Finally, try to run MBMs to completion. + if opts.mbms { + ::MultiBlockMigrator::try_mbms()?; + } + Ok(before_all_weight.saturating_add(try_on_runtime_upgrade_weight)) } diff --git a/substrate/frame/executive/src/tests.rs b/substrate/frame/executive/src/tests.rs index e3721f7b6dcb..b1171a04353a 100644 --- a/substrate/frame/executive/src/tests.rs +++ b/substrate/frame/executive/src/tests.rs @@ -460,6 +460,11 @@ impl MultiStepMigrator for MockedModeGetter { fn step() -> Weight { Weight::zero() } + + #[cfg(feature = "try-runtime")] + fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { + Ok(()) + } } fn extra(nonce: u64, fee: Balance) -> SignedExtra { diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 649bc314a12b..8bb76ce1f345 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -69,22 +69,22 @@ //! either be [`MigrationCursor::Active`] or [`MigrationCursor::Stuck`]. In the active case it //! points to the currently active migration and stores its inner cursor. The inner cursor can then //! be used by the migration to store its inner state and advance. Each time when the migration -//! returns `Some(cursor)`, it signals the pallet that it is not done yet. +//! returns `Some(cursor)`, it signals the pallet that it is not done yet. //! The cursor is reset on each runtime upgrade. This ensures that it starts to execute at the //! first migration in the vector. The pallets cursor is only ever incremented or set to `Stuck` //! once it encounters an error (Goal 4). Once in the stuck state, the pallet will stay stuck until -//! it is fixed through manual governance intervention. +//! it is fixed through manual governance intervention. //! As soon as the cursor of the pallet becomes `Some(_)`; [`MultiStepMigrator::ongoing`] returns //! `true` (Goal 2). This can be used by upstream code to possibly pause transactions. //! In `on_initialize` the pallet will load the current migration and check whether it was already //! executed in the past by checking for membership of its ID in the [`Historic`] set. Historic //! migrations are skipped without causing an error. Each successfully executed migration is added -//! to this set (Goal 5). +//! to this set (Goal 5). //! This proceeds until no more migrations remain. At that point, the event `UpgradeCompleted` is -//! emitted (Goal 1). +//! emitted (Goal 1). //! The execution of each migration happens by calling [`SteppedMigration::transactional_step`]. //! This function wraps the inner `step` function into a transactional layer to allow rollback in -//! the error case (Goal 6). +//! the error case (Goal 6). //! Weight limits must be checked by the migration itself. The pallet provides a [`WeightMeter`] for //! that purpose. The pallet may return [`SteppedMigrationError::InsufficientWeight`] at any point. //! In that scenario, one of two things will happen: if that migration was exclusively executed @@ -785,4 +785,100 @@ impl MultiStepMigrator for Pallet { fn step() -> Weight { Self::progress_mbms(System::::block_number()) } + + #[cfg(feature = "try-runtime")] + fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { + use sp_std::collections::btree_map::BTreeMap; + + // Check if migrations were initiated. + if !Self::ongoing() { + return Ok(()) + } + + // Store pre_upgrade state for all migrations. + let mut pre_upgrade_state = BTreeMap::>::new(); + let mut errors = Vec::new(); + for i in 0..T::Migrations::len() { + match T::Migrations::nth_pre_upgrade(i) { + Ok(state) => { + pre_upgrade_state.insert(i, state); + }, + Err(e) => { + errors.push((i, "pre-upgrade", e)); + }, + } + } + + // Run migrations to completion. + match Self::try_step_until_completion() { + Ok(blocks) => { + log::info!("ℹ️ MBMs executed in {} blocks", blocks); + }, + Err((i, e)) => errors.push((i, "step", e)), + } + + // Run post_upgrade for all migrations. + for i in 0..T::Migrations::len() { + if let Err(e) = T::Migrations::nth_post_upgrade( + i, + pre_upgrade_state.get(&i).unwrap_or(&vec![]).clone(), + ) { + errors.push((i, "post-upgrade", e)); + } + } + + if !errors.is_empty() { + log::error!("❌ MBM errors occured:"); + for (i, p, e) in errors { + log::error!( + " - Migration at index {} errored during phase '{}' error: {:?}", + i, + p, + e + ); + } + return Err("MBM failure/s. See error logs for info.".into()) + } + + Ok(()) + } +} + +impl Pallet { + #[cfg(feature = "try-runtime")] + fn try_step_until_completion() -> Result, (u32, sp_runtime::TryRuntimeError)> + { + use frame_support::traits::Hooks; + + let start_block = System::::block_number(); + loop { + let migration_index = match Cursor::::get() { + Some(MigrationCursor::Active(active)) => active.index, + _ => unreachable!("migration must be active"), + }; + + // Simulate a step. + log::debug!("Block {}", System::::block_number()); + System::::set_block_number(System::::block_number() + 1u32.into()); + System::::on_initialize(System::::block_number()); + crate::Pallet::::on_initialize(System::::block_number()); + Self::step(); + >::on_finalize(System::::block_number()); + >::on_finalize(System::::block_number()); + + // Check events for `UpgradeCompleted` or `UpgradeFailed`. + for e in System::::events().iter() { + let e_success: ::RuntimeEvent = + Event::::UpgradeCompleted.into(); + let e_failed: ::RuntimeEvent = Event::::UpgradeFailed.into(); + if e.event == e_success.into() { + let end_block = System::::block_number(); + return Ok(end_block - start_block) + } + if e.event == e_failed.into() { + return Err((migration_index, "UpgradeFailed event emitted".into())) + } + } + } + } } diff --git a/substrate/frame/migrations/src/mock_helpers.rs b/substrate/frame/migrations/src/mock_helpers.rs index d230417d12e6..5377e513e59c 100644 --- a/substrate/frame/migrations/src/mock_helpers.rs +++ b/substrate/frame/migrations/src/mock_helpers.rs @@ -43,6 +43,12 @@ pub enum MockedMigrationKind { /// Cause an [`SteppedMigrationError::InsufficientWeight`] error after its number of steps /// elapsed. HighWeightAfter(Weight), + /// PreUpgrade should fail. + #[cfg(feature = "try-runtime")] + PreUpgradeFail, + /// PostUpgrade should fail. + #[cfg(feature = "try-runtime")] + PostUpgradeFail, } use MockedMigrationKind::*; // C style @@ -99,6 +105,8 @@ impl SteppedMigrations for MockedMigrations { Err(SteppedMigrationError::Failed) }, TimeoutAfter => unreachable!(), + #[cfg(feature = "try-runtime")] + PreUpgradeFail | PostUpgradeFail => Ok(None), }) } @@ -115,6 +123,28 @@ impl SteppedMigrations for MockedMigrations { MIGRATIONS::get().get(n as usize).map(|(_, s)| Some(*s)) } + #[cfg(feature = "try-runtime")] + fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError> { + let (kind, _) = MIGRATIONS::get()[n as usize]; + + if let PreUpgradeFail = kind { + return Err("Some pre-upgrade error".into()) + } + + Ok(vec![]) + } + + #[cfg(feature = "try-runtime")] + fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let (kind, _) = MIGRATIONS::get()[n as usize]; + + if let PostUpgradeFail = kind { + return Err("Some post-upgrade error".into()) + } + + Ok(()) + } + fn cursor_max_encoded_len() -> usize { 65_536 } diff --git a/substrate/frame/migrations/src/tests.rs b/substrate/frame/migrations/src/tests.rs index 9c9043d37a62..3df380017ebe 100644 --- a/substrate/frame/migrations/src/tests.rs +++ b/substrate/frame/migrations/src/tests.rs @@ -333,3 +333,127 @@ fn migration_timeout_errors() { assert_eq!(upgrades_started_completed_failed(), (0, 0, 1)); }); } + +#[cfg(feature = "try-runtime")] +#[test] +fn try_mbms_success_case() { + use frame_support::migrations::MultiStepMigrator; + use Event::*; + test_closure(|| { + // Add three migrations, each taking one block longer than the previous. + MockedMigrations::set(vec![(SucceedAfter, 0), (SucceedAfter, 1), (SucceedAfter, 2)]); + + System::set_block_number(1); + Migrations::on_runtime_upgrade(); + crate::Pallet::::try_mbms().unwrap(); + + // Check that we got all events. + assert_events(vec![ + UpgradeStarted { migrations: 3 }, + MigrationCompleted { index: 0, took: 1 }, + MigrationAdvanced { index: 1, took: 0 }, + MigrationCompleted { index: 1, took: 1 }, + MigrationAdvanced { index: 2, took: 0 }, + MigrationAdvanced { index: 2, took: 1 }, + MigrationCompleted { index: 2, took: 2 }, + UpgradeCompleted, + ]); + }); +} + +#[cfg(feature = "try-runtime")] +#[test] +fn try_mbms_pre_upgrade_failure() { + use frame_support::migrations::MultiStepMigrator; + use Event::*; + test_closure(|| { + // Add three migrations, it should fail after the second one. + MockedMigrations::set(vec![(SucceedAfter, 0), (PreUpgradeFail, 1), (SucceedAfter, 2)]); + + System::set_block_number(1); + Migrations::on_runtime_upgrade(); + + // try_mbms should fail, but all events should still be emitted. + let _ = crate::Pallet::::try_mbms().unwrap_err(); + assert_events(vec![ + UpgradeStarted { migrations: 3 }, + MigrationCompleted { index: 0, took: 1 }, + MigrationAdvanced { index: 1, took: 0 }, + MigrationCompleted { index: 1, took: 1 }, + MigrationAdvanced { index: 2, took: 0 }, + MigrationAdvanced { index: 2, took: 1 }, + MigrationCompleted { index: 2, took: 2 }, + UpgradeCompleted, + ]); + }); +} + +#[cfg(feature = "try-runtime")] +#[test] +fn try_mbms_post_upgrade_failure() { + use frame_support::migrations::MultiStepMigrator; + use Event::*; + test_closure(|| { + // Add three migrations, it should fail after the second one. + MockedMigrations::set(vec![(SucceedAfter, 0), (PostUpgradeFail, 1), (SucceedAfter, 2)]); + + System::set_block_number(1); + Migrations::on_runtime_upgrade(); + + // try_mbms should fail, but all events should still be emitted. + let _ = crate::Pallet::::try_mbms().unwrap_err(); + assert_events(vec![ + UpgradeStarted { migrations: 3 }, + MigrationCompleted { index: 0, took: 1 }, + MigrationAdvanced { index: 1, took: 0 }, + MigrationCompleted { index: 1, took: 1 }, + MigrationAdvanced { index: 2, took: 0 }, + MigrationAdvanced { index: 2, took: 1 }, + MigrationCompleted { index: 2, took: 2 }, + UpgradeCompleted, + ]); + }); +} + +#[cfg(feature = "try-runtime")] +#[test] +fn try_mbms_migration_failure() { + use frame_support::migrations::MultiStepMigrator; + use Event::*; + test_closure(|| { + // Add three migrations, it should fail after the second one. + MockedMigrations::set(vec![(SucceedAfter, 0), (FailAfter, 5), (SucceedAfter, 10)]); + + System::set_block_number(1); + Migrations::on_runtime_upgrade(); + let _ = crate::Pallet::::try_mbms().unwrap_err(); + + // Check that we got all events. + assert_events(vec![ + UpgradeStarted { migrations: 3 }, + MigrationCompleted { index: 0, took: 1 }, + MigrationAdvanced { index: 1, took: 0 }, + MigrationAdvanced { index: 1, took: 1 }, + MigrationAdvanced { index: 1, took: 2 }, + MigrationAdvanced { index: 1, took: 3 }, + MigrationAdvanced { index: 1, took: 4 }, + MigrationFailed { index: 1, took: 5 }, + UpgradeFailed, + ]); + }); +} + +#[cfg(feature = "try-runtime")] +#[test] +fn try_step_no_migrations() { + use frame_support::migrations::MultiStepMigrator; + test_closure(|| { + MockedMigrations::set(vec![]); + + System::set_block_number(1); + Migrations::on_runtime_upgrade(); + let _ = crate::Pallet::::try_mbms(); + + assert_eq!(System::events().len(), 0); + }); +} diff --git a/substrate/frame/support/Cargo.toml b/substrate/frame/support/Cargo.toml index ecdd93826327..eacaa2a4d307 100644 --- a/substrate/frame/support/Cargo.toml +++ b/substrate/frame/support/Cargo.toml @@ -62,6 +62,7 @@ docify = "0.2.8" static_assertions = "1.1.0" aquamarine = { version = "0.5.0" } +builder-pattern = "0.4.2" [dev-dependencies] assert_matches = "1.3.0" diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index 968639e02d35..a3d7ffe1653b 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -416,6 +416,23 @@ pub trait SteppedMigration { }) .map_err(|()| SteppedMigrationError::Failed)? } + + /// Executed prior to the migration starting. + /// + /// Returns some bytes which are passed into `post_upgrade` after the migration is completed. + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + Ok(Vec::new()) + } + + /// Executed after the migration is completed. + /// + /// Should be used to verify the state of the chain after the migration. The `state` parameter + /// is the return value from `pre_upgrade`. + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + Ok(()) + } } /// Error that can occur during a [`SteppedMigration`]. @@ -541,6 +558,12 @@ pub trait MultiStepMigrator { /// /// Must gracefully handle the case that it is currently not upgrading. fn step() -> Weight; + + #[cfg(feature = "try-runtime")] + /// Repeatedly call [`Self::step`] until the migration is completed or in a fail state. + /// + /// Runs pre/post upgrade hooks if they exist. + fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError>; } impl MultiStepMigrator for () { @@ -551,6 +574,11 @@ impl MultiStepMigrator for () { fn step() -> Weight { Weight::zero() } + + #[cfg(feature = "try-runtime")] + fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { + Ok(()) + } } /// Multiple [`SteppedMigration`]. @@ -586,6 +614,14 @@ pub trait SteppedMigrations { meter: &mut WeightMeter, ) -> Option>, SteppedMigrationError>>; + #[cfg(feature = "try-runtime")] + /// Call the pre-upgrade hooks of the `n`th migration. + fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError>; + + #[cfg(feature = "try-runtime")] + /// Call the pre-upgrade hooks of the `n`th migration. + fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError>; + /// The maximal encoded length across all cursors. fn cursor_max_encoded_len() -> usize; @@ -649,6 +685,16 @@ impl SteppedMigrations for () { None } + #[cfg(feature = "try-runtime")] + fn nth_pre_upgrade(_n: u32) -> Result, sp_runtime::TryRuntimeError> { + Ok(Vec::new()) + } + + #[cfg(feature = "try-runtime")] + fn nth_post_upgrade(_n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + Ok(()) + } + fn cursor_max_encoded_len() -> usize { 0 } @@ -719,6 +765,23 @@ impl SteppedMigrations for T { ) } + #[cfg(feature = "try-runtime")] + fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError> { + if n != 0 { + defensive!("nth_transactional_step should only be called with n==0"); + } + + T::pre_upgrade() + } + + #[cfg(feature = "try-runtime")] + fn nth_post_upgrade(n: u32, state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + if n != 0 { + defensive!("nth_transactional_step should only be called with n==0"); + } + T::post_upgrade(state) + } + fn cursor_max_encoded_len() -> usize { T::Cursor::max_encoded_len() } @@ -784,6 +847,36 @@ impl SteppedMigrations for Tuple { None } + #[cfg(feature = "try-runtime")] + fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError> { + let mut i = 0; + + for_tuples! ( #( + if (i + Tuple::len()) > n { + return Tuple::nth_pre_upgrade(n - i) + } + + i += Tuple::len(); + )* ); + + panic!("n greater than number of migrations") + } + + #[cfg(feature = "try-runtime")] + fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let mut i = 0; + + for_tuples! ( #( + if (i + Tuple::len()) > n { + return Tuple::nth_post_upgrade(n - i, _state) + } + + i += Tuple::len(); + )* ); + + panic!("n greater than number of migrations") + } + fn nth_max_steps(n: u32) -> Option> { let mut i = 0; diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index a423656c394f..fa80538ab829 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -134,6 +134,6 @@ pub use tasks::Task; mod try_runtime; #[cfg(feature = "try-runtime")] pub use try_runtime::{ - Select as TryStateSelect, TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState, - UpgradeCheckSelect, + Select as TryStateSelect, TryDecodeEntireStorage, TryDecodeEntireStorageError, + TryOnRuntimeUpgradeOpts, TryState, }; diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs index bec2dbf549a1..a204bb017868 100644 --- a/substrate/frame/support/src/traits/try_runtime/mod.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -94,49 +94,19 @@ impl sp_std::str::FromStr for Select { } } -/// Select which checks should be run when trying a runtime upgrade upgrade. -#[derive(codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo)] -pub enum UpgradeCheckSelect { - /// Run no checks. - None, - /// Run the `try_state`, `pre_upgrade` and `post_upgrade` checks. - All, - /// Run the `pre_upgrade` and `post_upgrade` checks. - PreAndPost, - /// Run the `try_state` checks. - TryState, -} - -impl UpgradeCheckSelect { - /// Whether the pre- and post-upgrade checks are selected. - pub fn pre_and_post(&self) -> bool { - matches!(self, Self::All | Self::PreAndPost) - } - - /// Whether the try-state checks are selected. - pub fn try_state(&self) -> bool { - matches!(self, Self::All | Self::TryState) - } - - /// Whether to run any checks at all. - pub fn any(&self) -> bool { - !matches!(self, Self::None) - } -} - -#[cfg(feature = "std")] -impl core::str::FromStr for UpgradeCheckSelect { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - match s.to_lowercase().as_str() { - "none" => Ok(Self::None), - "all" => Ok(Self::All), - "pre-and-post" => Ok(Self::PreAndPost), - "try-state" => Ok(Self::TryState), - _ => Err("Invalid CheckSelector"), - } - } +/// Allows selectively deciding which runtime upgrade checks to execute. +#[derive( + codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo, builder_pattern::Builder, +)] +pub struct TryOnRuntimeUpgradeOpts { + #[default(false)] + pub pre_and_post: bool, + #[default(false)] + pub try_state: bool, + #[default(false)] + pub decode_entire_state: bool, + #[default(false)] + pub mbms: bool, } /// Execute some checks to ensure the internal state of a pallet is consistent. diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index f41e606ad7c3..bd1156314d3f 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -2336,7 +2336,7 @@ fn pallet_on_chain_storage_version_initializes_correctly() { #[cfg(feature = "try-runtime")] #[test] fn post_runtime_upgrade_detects_storage_version_issues() { - use frame_support::traits::UpgradeCheckSelect; + use frame_support::traits::TryOnRuntimeUpgradeOpts; struct CustomUpgrade; @@ -2391,7 +2391,8 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // The version isn't changed, we should detect it. assert!( - Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap_err() == + Executive::try_runtime_upgrade( + TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build()).unwrap_err() == "On chain and in-code storage version do not match. Missing runtime upgrade?" .into() ); @@ -2402,14 +2403,18 @@ fn post_runtime_upgrade_detects_storage_version_issues() { Example::on_genesis(); // We set the new storage version in the pallet and that should be detected. UpdateStorageVersion::set(&true); - Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap(); + Executive::try_runtime_upgrade( + TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build() +).unwrap(); }); TestExternalities::default().execute_with(|| { // Call `on_genesis` to put the storage version of `Example` into the storage. Example::on_genesis(); // We set the new storage version in the custom upgrade and that should be detected. - ExecutiveWithUpgrade::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap(); + ExecutiveWithUpgrade::try_runtime_upgrade( + TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build() +).unwrap(); }); TestExternalities::default().execute_with(|| { @@ -2421,8 +2426,10 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // `CustomUpgradePallet4` will set a storage version for `Example4` while this doesn't has // any storage version "enabled". assert!( - ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) - .unwrap_err() == "On chain storage version set, while the pallet \ + ExecutiveWithUpgradePallet4::try_runtime_upgrade( + TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build() + ) + .unwrap_err() == "On chain storage version set, while the pallet \ doesn't have the `#[pallet::storage_version(VERSION)]` attribute." .into() ); diff --git a/substrate/frame/system/src/mock.rs b/substrate/frame/system/src/mock.rs index e1959e572e99..af4a53dbcdca 100644 --- a/substrate/frame/system/src/mock.rs +++ b/substrate/frame/system/src/mock.rs @@ -102,6 +102,11 @@ impl frame_support::migrations::MultiStepMigrator for MockedMigrator { fn step() -> Weight { Weight::zero() } + + #[cfg(feature = "try-runtime")] + fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { + Ok(()) + } } pub type SysEvent = frame_system::Event; diff --git a/substrate/frame/try-runtime/src/inner.rs b/substrate/frame/try-runtime/src/inner.rs index 591124e2ad99..fa6cc5b868ed 100644 --- a/substrate/frame/try-runtime/src/inner.rs +++ b/substrate/frame/try-runtime/src/inner.rs @@ -17,7 +17,7 @@ //! Supporting types for try-runtime, testing and dry-running commands. -pub use frame_support::traits::{TryStateSelect, UpgradeCheckSelect}; +pub use frame_support::traits::{TryOnRuntimeUpgradeOpts, TryStateSelect}; use frame_support::weights::Weight; sp_api::decl_runtime_apis! { @@ -31,10 +31,8 @@ sp_api::decl_runtime_apis! { /// Returns the consumed weight of the migration in case of a successful one, combined with /// the total allowed block weight of the runtime. /// - /// If `checks` is `true`, `pre_migrate` and `post_migrate` of each migration and - /// `try_state` of all pallets will be executed. Else, no. If checks are executed, the PoV - /// tracking is likely inaccurate. - fn on_runtime_upgrade(checks: UpgradeCheckSelect) -> (Weight, Weight); + /// [`TryOnRuntimeUpgradeOpts`] allows configuration of which checks to run post-upgrade. + fn on_runtime_upgrade(opts: TryOnRuntimeUpgradeOpts) -> (Weight, Weight); /// Execute the given block, but optionally disable state-root and signature checks. /// diff --git a/templates/parachain/runtime/src/apis.rs b/templates/parachain/runtime/src/apis.rs index b13ba278fae6..fe752eb017ca 100644 --- a/templates/parachain/runtime/src/apis.rs +++ b/templates/parachain/runtime/src/apis.rs @@ -192,7 +192,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { use super::configs::RuntimeBlockWeights; let weight = Executive::try_runtime_upgrade(checks).unwrap(); diff --git a/templates/solochain/runtime/src/lib.rs b/templates/solochain/runtime/src/lib.rs index 93a56fb0ad78..d13fa72bb776 100644 --- a/templates/solochain/runtime/src/lib.rs +++ b/templates/solochain/runtime/src/lib.rs @@ -554,11 +554,11 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(opts: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to // have a backtrace here. If any of the pre/post migration checks fail, we shall stop // right here and right now. - let weight = Executive::try_runtime_upgrade(checks).unwrap(); + let weight = Executive::try_runtime_upgrade(opts).unwrap(); (weight, BlockWeights::get().max_block) } From 675338a8405080eb07ca275bf9790f84881edc7a Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 21 May 2024 19:38:19 +0400 Subject: [PATCH 02/29] update cargo.lock --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index d1f388a4bfd5..6443da6f752a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2350,7 +2350,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47d624ef88b39588d113f807ffb38ee968aafc388ca57dd7a9a7b82d3de1f5f4" dependencies = [ "bitflags 1.3.2", - "proc-macro2 1.0.75", + "proc-macro2 1.0.82", "quote 1.0.35", "syn 1.0.109", ] From d36ce7121c41aceae164dcb2a3defb8786d3b80a Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Mon, 27 May 2024 18:39:08 +0400 Subject: [PATCH 03/29] refactor try-runtime logic --- substrate/frame/executive/src/lib.rs | 5 - substrate/frame/executive/src/tests.rs | 5 - substrate/frame/migrations/src/lib.rs | 141 +++++++--------------- substrate/frame/migrations/src/tests.rs | 90 ++++++-------- substrate/frame/support/src/migrations.rs | 13 +- substrate/frame/system/src/mock.rs | 5 - 6 files changed, 83 insertions(+), 176 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 0614f52d8319..684579fb6e6b 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -464,11 +464,6 @@ where )?; } - // Finally, try to run MBMs to completion. - if opts.mbms { - ::MultiBlockMigrator::try_mbms()?; - } - Ok(before_all_weight.saturating_add(try_on_runtime_upgrade_weight)) } diff --git a/substrate/frame/executive/src/tests.rs b/substrate/frame/executive/src/tests.rs index b1171a04353a..e3721f7b6dcb 100644 --- a/substrate/frame/executive/src/tests.rs +++ b/substrate/frame/executive/src/tests.rs @@ -460,11 +460,6 @@ impl MultiStepMigrator for MockedModeGetter { fn step() -> Weight { Weight::zero() } - - #[cfg(feature = "try-runtime")] - fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { - Ok(()) - } } fn extra(nonce: u64, fee: Balance) -> SignedExtra { diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 8bb76ce1f345..8d9e785457fb 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -267,6 +267,18 @@ pub trait MockedMigrations: SteppedMigrations { fn set_success_after(n: u32); } +#[cfg(feature = "try-runtime")] +/// Wrapper for pre-upgrade bytes, allowing us to impl MEL on it. +#[derive(Debug, Clone, Eq, PartialEq, Encode, Decode, scale_info::TypeInfo, Default)] +pub struct PreUpgradeBytesWrapper(pub Vec); + +#[cfg(feature = "try-runtime")] +impl MaxEncodedLen for PreUpgradeBytesWrapper { + fn max_encoded_len() -> usize { + 0 + } +} + #[frame_support::pallet] pub mod pallet { use super::*; @@ -378,6 +390,12 @@ pub mod pallet { #[pallet::storage] pub type Historic = StorageMap<_, Twox64Concat, IdentifierOf, (), OptionQuery>; + /// Data stored by the pre-upgrade hook of the MBM. + #[cfg(feature = "try-runtime")] + #[pallet::storage] + pub type PreUpgradeBytes = + StorageMap<_, Twox64Concat, IdentifierOf, PreUpgradeBytesWrapper, ValueQuery>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { @@ -698,6 +716,14 @@ impl Pallet { } let max_steps = T::Migrations::nth_max_steps(cursor.index); + + // If this is the first time running this migration, exec the pre-upgrade hook. + #[cfg(feature = "try-runtime")] + if !PreUpgradeBytes::::contains_key(&bounded_id) { + let bytes = T::Migrations::nth_pre_upgrade(cursor.index).expect("Pre-upgrade failed"); + PreUpgradeBytes::::insert(&bounded_id, PreUpgradeBytesWrapper(bytes)); + } + let next_cursor = T::Migrations::nth_transactional_step( cursor.index, cursor.inner_cursor.clone().map(|c| c.into_inner()), @@ -732,6 +758,15 @@ impl Pallet { }, Ok(None) => { // A migration is done when it returns cursor `None`. + + // Run post-upgrade checks. + #[cfg(feature = "try-runtime")] + T::Migrations::nth_post_upgrade( + cursor.index, + PreUpgradeBytes::::get(&bounded_id).0, + ) + .expect("Post-upgrade failed."); + Self::deposit_event(Event::MigrationCompleted { index: cursor.index, took }); Historic::::insert(&bounded_id, ()); cursor.goto_next_migration(System::::block_number()); @@ -756,10 +791,20 @@ impl Pallet { } /// Fail the current runtime upgrade, caused by `migration`. + /// + /// When the `try-runtime` feature is enabled, this function will panic. + // Allow unreachable code so it can compile without warnings when `try-runtime` is enabled. + #[allow(unreachable_code)] fn upgrade_failed(migration: Option) { use FailedMigrationHandling::*; Self::deposit_event(Event::UpgradeFailed); + #[cfg(feature = "try-runtime")] + { + log::error!("Migration at index {:?} failed.", migration); + panic!("A multi-block migration failed. Run with RUST_LOG=trace for all logs."); + } + match T::FailedMigrationHandler::failed(migration) { KeepStuck => Cursor::::set(Some(MigrationCursor::Stuck)), ForceUnstuck => Cursor::::kill(), @@ -785,100 +830,4 @@ impl MultiStepMigrator for Pallet { fn step() -> Weight { Self::progress_mbms(System::::block_number()) } - - #[cfg(feature = "try-runtime")] - fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { - use sp_std::collections::btree_map::BTreeMap; - - // Check if migrations were initiated. - if !Self::ongoing() { - return Ok(()) - } - - // Store pre_upgrade state for all migrations. - let mut pre_upgrade_state = BTreeMap::>::new(); - let mut errors = Vec::new(); - for i in 0..T::Migrations::len() { - match T::Migrations::nth_pre_upgrade(i) { - Ok(state) => { - pre_upgrade_state.insert(i, state); - }, - Err(e) => { - errors.push((i, "pre-upgrade", e)); - }, - } - } - - // Run migrations to completion. - match Self::try_step_until_completion() { - Ok(blocks) => { - log::info!("ℹ️ MBMs executed in {} blocks", blocks); - }, - Err((i, e)) => errors.push((i, "step", e)), - } - - // Run post_upgrade for all migrations. - for i in 0..T::Migrations::len() { - if let Err(e) = T::Migrations::nth_post_upgrade( - i, - pre_upgrade_state.get(&i).unwrap_or(&vec![]).clone(), - ) { - errors.push((i, "post-upgrade", e)); - } - } - - if !errors.is_empty() { - log::error!("❌ MBM errors occured:"); - for (i, p, e) in errors { - log::error!( - " - Migration at index {} errored during phase '{}' error: {:?}", - i, - p, - e - ); - } - return Err("MBM failure/s. See error logs for info.".into()) - } - - Ok(()) - } -} - -impl Pallet { - #[cfg(feature = "try-runtime")] - fn try_step_until_completion() -> Result, (u32, sp_runtime::TryRuntimeError)> - { - use frame_support::traits::Hooks; - - let start_block = System::::block_number(); - loop { - let migration_index = match Cursor::::get() { - Some(MigrationCursor::Active(active)) => active.index, - _ => unreachable!("migration must be active"), - }; - - // Simulate a step. - log::debug!("Block {}", System::::block_number()); - System::::set_block_number(System::::block_number() + 1u32.into()); - System::::on_initialize(System::::block_number()); - crate::Pallet::::on_initialize(System::::block_number()); - Self::step(); - >::on_finalize(System::::block_number()); - >::on_finalize(System::::block_number()); - - // Check events for `UpgradeCompleted` or `UpgradeFailed`. - for e in System::::events().iter() { - let e_success: ::RuntimeEvent = - Event::::UpgradeCompleted.into(); - let e_failed: ::RuntimeEvent = Event::::UpgradeFailed.into(); - if e.event == e_success.into() { - let end_block = System::::block_number(); - return Ok(end_block - start_block) - } - if e.event == e_failed.into() { - return Err((migration_index, "UpgradeFailed event emitted".into())) - } - } - } - } } diff --git a/substrate/frame/migrations/src/tests.rs b/substrate/frame/migrations/src/tests.rs index 3df380017ebe..df5420f820dc 100644 --- a/substrate/frame/migrations/src/tests.rs +++ b/substrate/frame/migrations/src/tests.rs @@ -17,12 +17,24 @@ #![cfg(test)] +use frame_support::traits::OnRuntimeUpgrade; + +#[cfg(not(feature = "try-runtime"))] use crate::{ mock::{Test as T, *}, mock_helpers::{MockedMigrationKind::*, *}, Cursor, Event, FailedMigrationHandling, MigrationCursor, }; -use frame_support::{pallet_prelude::Weight, traits::OnRuntimeUpgrade}; + +#[cfg(feature = "try-runtime")] +use crate::{ + mock::*, + mock_helpers::{MockedMigrationKind::*, *}, + Event, +}; + +#[cfg(not(feature = "try-runtime"))] +use frame_support::pallet_prelude::Weight; #[docify::export] #[test] @@ -60,6 +72,7 @@ fn simple_works() { }); } +#[cfg(not(feature = "try-runtime"))] #[test] fn failing_migration_sets_cursor_to_stuck() { test_closure(|| { @@ -90,6 +103,7 @@ fn failing_migration_sets_cursor_to_stuck() { }); } +#[cfg(not(feature = "try-runtime"))] #[test] fn failing_migration_force_unstuck_works() { test_closure(|| { @@ -122,6 +136,7 @@ fn failing_migration_force_unstuck_works() { /// A migration that reports not getting enough weight errors if it is the first one to run in that /// block. +#[cfg(not(feature = "try-runtime"))] #[test] fn high_weight_migration_singular_fails() { test_closure(|| { @@ -150,6 +165,7 @@ fn high_weight_migration_singular_fails() { /// A migration that reports of not getting enough weight is retried once, if it is not the first /// one to run in a block. +#[cfg(not(feature = "try-runtime"))] #[test] fn high_weight_migration_retries_once() { test_closure(|| { @@ -179,6 +195,7 @@ fn high_weight_migration_retries_once() { /// not the first one in the block. // Note: Same as `high_weight_migration_retries_once` but with different required weight for the // migration. +#[cfg(not(feature = "try-runtime"))] #[test] fn high_weight_migration_permanently_overweight_fails() { test_closure(|| { @@ -274,6 +291,7 @@ fn historic_skipping_works() { /// When another upgrade happens while a migration is still running, it should set the cursor to /// stuck. +#[cfg(not(feature = "try-runtime"))] #[test] fn upgrade_fails_when_migration_active() { test_closure(|| { @@ -300,6 +318,7 @@ fn upgrade_fails_when_migration_active() { }); } +#[cfg(not(feature = "try-runtime"))] #[test] fn migration_timeout_errors() { test_closure(|| { @@ -336,8 +355,7 @@ fn migration_timeout_errors() { #[cfg(feature = "try-runtime")] #[test] -fn try_mbms_success_case() { - use frame_support::migrations::MultiStepMigrator; +fn try_runtime_success_case() { use Event::*; test_closure(|| { // Add three migrations, each taking one block longer than the previous. @@ -345,7 +363,7 @@ fn try_mbms_success_case() { System::set_block_number(1); Migrations::on_runtime_upgrade(); - crate::Pallet::::try_mbms().unwrap(); + run_to_block(10); // Check that we got all events. assert_events(vec![ @@ -363,9 +381,8 @@ fn try_mbms_success_case() { #[cfg(feature = "try-runtime")] #[test] -fn try_mbms_pre_upgrade_failure() { - use frame_support::migrations::MultiStepMigrator; - use Event::*; +#[should_panic] +fn try_runtime_pre_upgrade_failure() { test_closure(|| { // Add three migrations, it should fail after the second one. MockedMigrations::set(vec![(SucceedAfter, 0), (PreUpgradeFail, 1), (SucceedAfter, 2)]); @@ -373,26 +390,15 @@ fn try_mbms_pre_upgrade_failure() { System::set_block_number(1); Migrations::on_runtime_upgrade(); - // try_mbms should fail, but all events should still be emitted. - let _ = crate::Pallet::::try_mbms().unwrap_err(); - assert_events(vec![ - UpgradeStarted { migrations: 3 }, - MigrationCompleted { index: 0, took: 1 }, - MigrationAdvanced { index: 1, took: 0 }, - MigrationCompleted { index: 1, took: 1 }, - MigrationAdvanced { index: 2, took: 0 }, - MigrationAdvanced { index: 2, took: 1 }, - MigrationCompleted { index: 2, took: 2 }, - UpgradeCompleted, - ]); + // should panic + run_to_block(10); }); } #[cfg(feature = "try-runtime")] #[test] -fn try_mbms_post_upgrade_failure() { - use frame_support::migrations::MultiStepMigrator; - use Event::*; +#[should_panic] +fn try_runtime_post_upgrade_failure() { test_closure(|| { // Add three migrations, it should fail after the second one. MockedMigrations::set(vec![(SucceedAfter, 0), (PostUpgradeFail, 1), (SucceedAfter, 2)]); @@ -400,59 +406,37 @@ fn try_mbms_post_upgrade_failure() { System::set_block_number(1); Migrations::on_runtime_upgrade(); - // try_mbms should fail, but all events should still be emitted. - let _ = crate::Pallet::::try_mbms().unwrap_err(); - assert_events(vec![ - UpgradeStarted { migrations: 3 }, - MigrationCompleted { index: 0, took: 1 }, - MigrationAdvanced { index: 1, took: 0 }, - MigrationCompleted { index: 1, took: 1 }, - MigrationAdvanced { index: 2, took: 0 }, - MigrationAdvanced { index: 2, took: 1 }, - MigrationCompleted { index: 2, took: 2 }, - UpgradeCompleted, - ]); + // should panic + run_to_block(10); }); } #[cfg(feature = "try-runtime")] #[test] -fn try_mbms_migration_failure() { - use frame_support::migrations::MultiStepMigrator; - use Event::*; +#[should_panic] +fn try_runtime_migration_failure() { test_closure(|| { // Add three migrations, it should fail after the second one. MockedMigrations::set(vec![(SucceedAfter, 0), (FailAfter, 5), (SucceedAfter, 10)]); System::set_block_number(1); Migrations::on_runtime_upgrade(); - let _ = crate::Pallet::::try_mbms().unwrap_err(); - // Check that we got all events. - assert_events(vec![ - UpgradeStarted { migrations: 3 }, - MigrationCompleted { index: 0, took: 1 }, - MigrationAdvanced { index: 1, took: 0 }, - MigrationAdvanced { index: 1, took: 1 }, - MigrationAdvanced { index: 1, took: 2 }, - MigrationAdvanced { index: 1, took: 3 }, - MigrationAdvanced { index: 1, took: 4 }, - MigrationFailed { index: 1, took: 5 }, - UpgradeFailed, - ]); + // should panic + run_to_block(10); }); } #[cfg(feature = "try-runtime")] #[test] -fn try_step_no_migrations() { - use frame_support::migrations::MultiStepMigrator; +fn try_runtime_no_migrations() { test_closure(|| { MockedMigrations::set(vec![]); System::set_block_number(1); Migrations::on_runtime_upgrade(); - let _ = crate::Pallet::::try_mbms(); + + run_to_block(10); assert_eq!(System::events().len(), 0); }); diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index a3d7ffe1653b..0c392b37a129 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -558,12 +558,6 @@ pub trait MultiStepMigrator { /// /// Must gracefully handle the case that it is currently not upgrading. fn step() -> Weight; - - #[cfg(feature = "try-runtime")] - /// Repeatedly call [`Self::step`] until the migration is completed or in a fail state. - /// - /// Runs pre/post upgrade hooks if they exist. - fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError>; } impl MultiStepMigrator for () { @@ -574,11 +568,6 @@ impl MultiStepMigrator for () { fn step() -> Weight { Weight::zero() } - - #[cfg(feature = "try-runtime")] - fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { - Ok(()) - } } /// Multiple [`SteppedMigration`]. @@ -619,7 +608,7 @@ pub trait SteppedMigrations { fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError>; #[cfg(feature = "try-runtime")] - /// Call the pre-upgrade hooks of the `n`th migration. + /// Call the post-upgrade hooks of the `n`th migration. fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError>; /// The maximal encoded length across all cursors. diff --git a/substrate/frame/system/src/mock.rs b/substrate/frame/system/src/mock.rs index accc492f45a3..fff848b3b0e5 100644 --- a/substrate/frame/system/src/mock.rs +++ b/substrate/frame/system/src/mock.rs @@ -111,11 +111,6 @@ impl frame_support::migrations::MultiStepMigrator for MockedMigrator { fn step() -> Weight { Weight::zero() } - - #[cfg(feature = "try-runtime")] - fn try_mbms() -> Result<(), sp_runtime::TryRuntimeError> { - Ok(()) - } } pub type SysEvent = frame_system::Event; From 1403cbcf5b12abe7324bb5d8e8aa6812b5432ff4 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Mon, 27 May 2024 18:57:59 +0400 Subject: [PATCH 04/29] taplo --- .../bridge-hubs/bridge-hub-rococo/Cargo.toml | 13 ++++++------- .../examples/multi-block-migrations/Cargo.toml | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml index 7290b82cfa16..60e24dcc16c6 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml @@ -123,7 +123,6 @@ snowbridge-runtime-common = { path = "../../../../../bridges/snowbridge/runtime/ bridge-hub-common = { path = "../common", default-features = false } - [dev-dependencies] static_assertions = "1.1" bridge-hub-test-utils = { path = "../test-utils" } @@ -178,7 +177,9 @@ std = [ "pallet-bridge-parachains/std", "pallet-bridge-relayers/std", "pallet-collator-selection/std", + "pallet-example-mbm/std", "pallet-message-queue/std", + "pallet-migrations/std", "pallet-multisig/std", "pallet-session/std", "pallet-timestamp/std", @@ -225,8 +226,6 @@ std = [ "xcm-builder/std", "xcm-executor/std", "xcm/std", - "pallet-example-mbm/std", - "pallet-migrations/std" ] runtime-benchmarks = [ @@ -247,7 +246,9 @@ runtime-benchmarks = [ "pallet-bridge-parachains/runtime-benchmarks", "pallet-bridge-relayers/runtime-benchmarks", "pallet-collator-selection/runtime-benchmarks", + "pallet-example-mbm/runtime-benchmarks", "pallet-message-queue/runtime-benchmarks", + "pallet-migrations/runtime-benchmarks", "pallet-multisig/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", "pallet-utility/runtime-benchmarks", @@ -268,8 +269,6 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", "xcm-builder/runtime-benchmarks", "xcm-executor/runtime-benchmarks", - "pallet-example-mbm/runtime-benchmarks", - "pallet-migrations/runtime-benchmarks" ] try-runtime = [ @@ -289,7 +288,9 @@ try-runtime = [ "pallet-bridge-parachains/try-runtime", "pallet-bridge-relayers/try-runtime", "pallet-collator-selection/try-runtime", + "pallet-example-mbm/try-runtime", "pallet-message-queue/try-runtime", + "pallet-migrations/try-runtime", "pallet-multisig/try-runtime", "pallet-session/try-runtime", "pallet-timestamp/try-runtime", @@ -304,8 +305,6 @@ try-runtime = [ "snowbridge-pallet-outbound-queue/try-runtime", "snowbridge-pallet-system/try-runtime", "sp-runtime/try-runtime", - "pallet-example-mbm/try-runtime", - "pallet-migrations/try-runtime" ] fast-runtime = [] diff --git a/substrate/frame/examples/multi-block-migrations/Cargo.toml b/substrate/frame/examples/multi-block-migrations/Cargo.toml index 290b62e9465a..ec4fb183919d 100644 --- a/substrate/frame/examples/multi-block-migrations/Cargo.toml +++ b/substrate/frame/examples/multi-block-migrations/Cargo.toml @@ -34,7 +34,7 @@ std = [ "pallet-migrations/std", "scale-info/std", "sp-io/std", - "sp-std/std" + "sp-std/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", From 92053e0b10e7e58d75aeae820e5903175b221832 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 28 May 2024 19:27:37 +0400 Subject: [PATCH 05/29] wip upgrade check select --- cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs | 2 +- cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs | 2 +- .../runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs | 2 +- .../runtimes/collectives/collectives-westend/src/lib.rs | 2 +- .../parachains/runtimes/contracts/contracts-rococo/src/lib.rs | 2 +- cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs | 2 +- .../parachains/runtimes/coretime/coretime-westend/src/lib.rs | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) 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 7053b665dced..4705d12e60c8 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1404,7 +1404,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } 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 42ad616fdbb6..a82094d6f8a6 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1503,7 +1503,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } 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 cffa24b919bb..90190da82dd1 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 @@ -771,7 +771,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index d88010ccaa93..29ba88df1045 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -944,7 +944,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index 0c37dbe4e6e4..1222e11e9a68 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -661,7 +661,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 215bc9ad4a3a..f43bb1c1e41b 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -663,7 +663,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 57a4e9bfd534..ff2456dc1772 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -654,7 +654,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } From e0a81527f732868479a61355e3c61c8eb77efe38 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 28 May 2024 19:28:27 +0400 Subject: [PATCH 06/29] impl_for_tuples MockedMigrations --- substrate/frame/migrations/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 8d9e785457fb..26174f385404 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -260,6 +260,7 @@ pub type IdentifierOf = BoundedVec::IdentifierMaxLen>; pub type ActiveCursorOf = ActiveCursor, BlockNumberFor>; /// Trait for a tuple of No-OP migrations with one element. +#[impl_trait_for_tuples::impl_for_tuples(30)] pub trait MockedMigrations: SteppedMigrations { /// The migration should fail after `n` steps. fn set_fail_after(n: u32); From bf3fbfb82bab6c83dad470a4261fb8eda22b899c Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Wed, 29 May 2024 13:52:58 +0400 Subject: [PATCH 07/29] wip revert upgradecheckselect changes --- .../bridge-hubs/bridge-hub-rococo/src/lib.rs | 2 +- .../runtimes/people/people-rococo/src/lib.rs | 2 +- .../runtimes/people/people-westend/src/lib.rs | 2 +- .../runtimes/testing/penpal/src/lib.rs | 2 +- .../asset_hub_polkadot_aura.rs | 2 +- .../src/fake_runtime_api/aura.rs | 2 +- polkadot/runtime/rococo/src/lib.rs | 6 +++--- polkadot/runtime/westend/src/lib.rs | 6 +++--- substrate/bin/node/runtime/src/lib.rs | 2 +- substrate/frame/executive/src/lib.rs | 4 ++-- substrate/frame/support/src/traits.rs | 4 ++-- .../support/src/traits/try_runtime/mod.rs | 2 +- substrate/frame/support/test/tests/pallet.rs | 18 +++++++++--------- substrate/frame/try-runtime/src/inner.rs | 6 +++--- templates/parachain/runtime/src/apis.rs | 2 +- templates/solochain/runtime/src/lib.rs | 2 +- 16 files changed, 32 insertions(+), 32 deletions(-) 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 7a56d0c7b312..fc6d8d1f4996 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 @@ -1103,7 +1103,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs index 927f49086d5b..5cd8aa357c37 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs @@ -629,7 +629,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs index cb2b5684bbf9..af6b5be44695 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs @@ -629,7 +629,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs index 67157f29a489..8afe56cddefa 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs @@ -952,7 +952,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, RuntimeBlockWeights::get().max_block) } diff --git a/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs b/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs index ac5bc57ff440..82c02943c5fc 100644 --- a/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs +++ b/cumulus/polkadot-parachain/src/fake_runtime_api/asset_hub_polkadot_aura.rs @@ -152,7 +152,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(_: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(_: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { unimplemented!() } diff --git a/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs b/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs index 1368d66ef2fe..6b718e912164 100644 --- a/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs +++ b/cumulus/polkadot-parachain/src/fake_runtime_api/aura.rs @@ -152,7 +152,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(_: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(_: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { unimplemented!() } diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 7dcac6aaddc1..9f96a6e39b67 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -2310,7 +2310,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { log::info!("try-runtime::on_runtime_upgrade rococo."); let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, BlockWeights::get().max_block) @@ -2581,7 +2581,7 @@ sp_api::impl_runtime_apis! { #[cfg(all(test, feature = "try-runtime"))] mod remote_tests { use super::*; - use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, TryOnRuntimeUpgradeOpts}; + use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect}; use remote_externalities::{ Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport, }; @@ -2614,7 +2614,7 @@ mod remote_tests { .await .unwrap(); ext.execute_with(|| { - Runtime::on_runtime_upgrade(TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build()) + Runtime::on_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) }); } } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index c55af54494a3..31ef4ab3c8db 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -2365,7 +2365,7 @@ sp_api::impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { log::info!("try-runtime::on_runtime_upgrade westend."); let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, BlockWeights::get().max_block) @@ -2648,7 +2648,7 @@ sp_api::impl_runtime_apis! { #[cfg(all(test, feature = "try-runtime"))] mod remote_tests { use super::*; - use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, TryOnRuntimeUpgradeOpts}; + use frame_try_runtime::{runtime_decl_for_try_runtime::TryRuntime, UpgradeCheckSelect}; use remote_externalities::{ Builder, Mode, OfflineConfig, OnlineConfig, SnapshotConfig, Transport, }; @@ -2681,7 +2681,7 @@ mod remote_tests { .await .unwrap(); ext.execute_with(|| { - Runtime::on_runtime_upgrade(TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build()) + Runtime::on_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) }); } } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f36b0d233ec1..7d9128bb940a 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -3153,7 +3153,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to // have a backtrace here. If any of the pre/post migration checks fail, we shall stop // right here and right now. diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 684579fb6e6b..5784dcd24285 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -180,7 +180,7 @@ use sp_std::{marker::PhantomData, prelude::*}; use ::{ frame_support::{ traits::{ - TryDecodeEntireStorage, TryDecodeEntireStorageError, TryOnRuntimeUpgradeOpts, TryState, + TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState, UpgradeCheckSelect, }, StorageNoopGuard, }, @@ -433,7 +433,7 @@ where /// [`frame_system::LastRuntimeUpgrade`] is set to the current runtime version after /// migrations execute. This is important for idempotency checks, because some migrations use /// this value to determine whether or not they should execute. - pub fn try_runtime_upgrade(opts: TryOnRuntimeUpgradeOpts) -> Result { + pub fn try_runtime_upgrade(opts: UpgradeCheckSelect) -> Result { let before_all_weight = ::before_all_runtime_migrations(); let try_on_runtime_upgrade_weight = diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index fa80538ab829..a423656c394f 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -134,6 +134,6 @@ pub use tasks::Task; mod try_runtime; #[cfg(feature = "try-runtime")] pub use try_runtime::{ - Select as TryStateSelect, TryDecodeEntireStorage, TryDecodeEntireStorageError, - TryOnRuntimeUpgradeOpts, TryState, + Select as TryStateSelect, TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState, + UpgradeCheckSelect, }; diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs index 52551d84473a..33a47b8f4047 100644 --- a/substrate/frame/support/src/traits/try_runtime/mod.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -98,7 +98,7 @@ impl sp_std::str::FromStr for Select { #[derive( codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo, builder_pattern::Builder, )] -pub struct TryOnRuntimeUpgradeOpts { +pub struct UpgradeCheckSelect { #[default(false)] pub pre_and_post: bool, #[default(false)] diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index 33cb681b1e94..feb889c73c2f 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -2335,7 +2335,7 @@ fn pallet_on_chain_storage_version_initializes_correctly() { #[cfg(feature = "try-runtime")] #[test] fn post_runtime_upgrade_detects_storage_version_issues() { - use frame_support::traits::TryOnRuntimeUpgradeOpts; + use frame_support::traits::UpgradeCheckSelect; struct CustomUpgrade; @@ -2390,8 +2390,8 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // The version isn't changed, we should detect it. assert!( - Executive::try_runtime_upgrade( - TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build()).unwrap_err() == + Executive::try_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) + .unwrap_err() == "On chain and in-code storage version do not match. Missing runtime upgrade?" .into() ); @@ -2402,9 +2402,8 @@ fn post_runtime_upgrade_detects_storage_version_issues() { Example::on_genesis(); // We set the new storage version in the pallet and that should be detected. UpdateStorageVersion::set(&true); - Executive::try_runtime_upgrade( - TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build() -).unwrap(); + Executive::try_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) + .unwrap(); }); TestExternalities::default().execute_with(|| { @@ -2412,8 +2411,9 @@ fn post_runtime_upgrade_detects_storage_version_issues() { Example::on_genesis(); // We set the new storage version in the custom upgrade and that should be detected. ExecutiveWithUpgrade::try_runtime_upgrade( - TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build() -).unwrap(); + UpgradeCheckSelect::new().pre_and_post(true).build(), + ) + .unwrap(); }); TestExternalities::default().execute_with(|| { @@ -2426,7 +2426,7 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // any storage version "enabled". assert!( ExecutiveWithUpgradePallet4::try_runtime_upgrade( - TryOnRuntimeUpgradeOpts::new().pre_and_post(true).build() + UpgradeCheckSelect::new().pre_and_post(true).build() ) .unwrap_err() == "On chain storage version set, while the pallet \ doesn't have the `#[pallet::storage_version(VERSION)]` attribute." diff --git a/substrate/frame/try-runtime/src/inner.rs b/substrate/frame/try-runtime/src/inner.rs index fa6cc5b868ed..37cf792b6ee5 100644 --- a/substrate/frame/try-runtime/src/inner.rs +++ b/substrate/frame/try-runtime/src/inner.rs @@ -17,7 +17,7 @@ //! Supporting types for try-runtime, testing and dry-running commands. -pub use frame_support::traits::{TryOnRuntimeUpgradeOpts, TryStateSelect}; +pub use frame_support::traits::{TryStateSelect, UpgradeCheckSelect}; use frame_support::weights::Weight; sp_api::decl_runtime_apis! { @@ -31,8 +31,8 @@ sp_api::decl_runtime_apis! { /// Returns the consumed weight of the migration in case of a successful one, combined with /// the total allowed block weight of the runtime. /// - /// [`TryOnRuntimeUpgradeOpts`] allows configuration of which checks to run post-upgrade. - fn on_runtime_upgrade(opts: TryOnRuntimeUpgradeOpts) -> (Weight, Weight); + /// [`UpgradeCheckSelect`] allows configuration of which checks to run post-upgrade. + fn on_runtime_upgrade(opts: UpgradeCheckSelect) -> (Weight, Weight); /// Execute the given block, but optionally disable state-root and signature checks. /// diff --git a/templates/parachain/runtime/src/apis.rs b/templates/parachain/runtime/src/apis.rs index 633e42c9330e..107956ded410 100644 --- a/templates/parachain/runtime/src/apis.rs +++ b/templates/parachain/runtime/src/apis.rs @@ -202,7 +202,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(checks: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { use super::configs::RuntimeBlockWeights; let weight = Executive::try_runtime_upgrade(checks).unwrap(); diff --git a/templates/solochain/runtime/src/lib.rs b/templates/solochain/runtime/src/lib.rs index d13fa72bb776..e583f05bebca 100644 --- a/templates/solochain/runtime/src/lib.rs +++ b/templates/solochain/runtime/src/lib.rs @@ -554,7 +554,7 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(opts: frame_try_runtime::TryOnRuntimeUpgradeOpts) -> (Weight, Weight) { + fn on_runtime_upgrade(opts: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to // have a backtrace here. If any of the pre/post migration checks fail, we shall stop // right here and right now. From 279e4cf615fe5a544949992de9295c408d935179 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Wed, 29 May 2024 14:04:17 +0400 Subject: [PATCH 08/29] revert upgradecheckselect changes --- Cargo.lock | 23 -------- polkadot/runtime/rococo/src/lib.rs | 4 +- polkadot/runtime/westend/src/lib.rs | 4 +- substrate/frame/executive/src/lib.rs | 14 ++--- substrate/frame/support/Cargo.toml | 1 - .../support/src/traits/try_runtime/mod.rs | 56 ++++++++++++++----- substrate/frame/support/test/tests/pallet.rs | 17 ++---- 7 files changed, 56 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e3c9ef94244..a448fcf5b0d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2336,28 +2336,6 @@ dependencies = [ "semver 0.6.0", ] -[[package]] -name = "builder-pattern" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d85376b93d8efe18dd819f56505e33e7a9c0f93fb02bd761f8690026178ed6e5" -dependencies = [ - "builder-pattern-macro", - "futures", -] - -[[package]] -name = "builder-pattern-macro" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47d624ef88b39588d113f807ffb38ee968aafc388ca57dd7a9a7b82d3de1f5f4" -dependencies = [ - "bitflags 1.3.2", - "proc-macro2 1.0.82", - "quote 1.0.35", - "syn 1.0.109", -] - [[package]] name = "bumpalo" version = "3.13.0" @@ -5865,7 +5843,6 @@ dependencies = [ "array-bytes", "assert_matches", "bitflags 1.3.2", - "builder-pattern", "docify", "environmental", "frame-metadata", diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 9f96a6e39b67..f0cc7e046f29 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -2613,8 +2613,6 @@ mod remote_tests { .build() .await .unwrap(); - ext.execute_with(|| { - Runtime::on_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) - }); + ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost)); } } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 31ef4ab3c8db..4bf132d82c96 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -2680,9 +2680,7 @@ mod remote_tests { .build() .await .unwrap(); - ext.execute_with(|| { - Runtime::on_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) - }); + ext.execute_with(|| Runtime::on_runtime_upgrade(UpgradeCheckSelect::PreAndPost)); } } diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 5784dcd24285..3028eaf318e0 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -179,12 +179,10 @@ use sp_std::{marker::PhantomData, prelude::*}; #[cfg(feature = "try-runtime")] use ::{ frame_support::{ - traits::{ - TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState, UpgradeCheckSelect, - }, + traits::{TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState}, StorageNoopGuard, }, - frame_try_runtime::TryStateSelect, + frame_try_runtime::{TryStateSelect, UpgradeCheckSelect}, log, sp_runtime::TryRuntimeError, }; @@ -433,12 +431,12 @@ where /// [`frame_system::LastRuntimeUpgrade`] is set to the current runtime version after /// migrations execute. This is important for idempotency checks, because some migrations use /// this value to determine whether or not they should execute. - pub fn try_runtime_upgrade(opts: UpgradeCheckSelect) -> Result { + pub fn try_runtime_upgrade(checks: UpgradeCheckSelect) -> Result { let before_all_weight = ::before_all_runtime_migrations(); let try_on_runtime_upgrade_weight = <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade( - opts.pre_and_post, + checks.pre_and_post(), )?; frame_system::LastRuntimeUpgrade::::put( @@ -451,13 +449,13 @@ where let _guard = StorageNoopGuard::default(); // The state must be decodable: - if opts.decode_entire_state { + if checks.any() { let res = AllPalletsWithSystem::try_decode_entire_state(); Self::log_decode_result(res)?; } // Check all storage invariants: - if opts.try_state { + if checks.try_state() { AllPalletsWithSystem::try_state( frame_system::Pallet::::block_number(), TryStateSelect::All, diff --git a/substrate/frame/support/Cargo.toml b/substrate/frame/support/Cargo.toml index 999452928c4e..a6c4fd6ee309 100644 --- a/substrate/frame/support/Cargo.toml +++ b/substrate/frame/support/Cargo.toml @@ -62,7 +62,6 @@ docify = "0.2.8" static_assertions = "1.1.0" aquamarine = { version = "0.5.0" } -builder-pattern = "0.4.2" [dev-dependencies] assert_matches = "1.3.0" diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs index 33a47b8f4047..c1bf1feb19e5 100644 --- a/substrate/frame/support/src/traits/try_runtime/mod.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -94,19 +94,49 @@ impl sp_std::str::FromStr for Select { } } -/// Allows selectively deciding which runtime upgrade checks to execute. -#[derive( - codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo, builder_pattern::Builder, -)] -pub struct UpgradeCheckSelect { - #[default(false)] - pub pre_and_post: bool, - #[default(false)] - pub try_state: bool, - #[default(false)] - pub decode_entire_state: bool, - #[default(false)] - pub mbms: bool, +/// Select which checks should be run when trying a runtime upgrade upgrade. +#[derive(codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo)] +pub enum UpgradeCheckSelect { + /// Run no checks. + None, + /// Run the `try_state`, `pre_upgrade` and `post_upgrade` checks. + All, + /// Run the `pre_upgrade` and `post_upgrade` checks. + PreAndPost, + /// Run the `try_state` checks. + TryState, +} + +impl UpgradeCheckSelect { + /// Whether the pre- and post-upgrade checks are selected. + pub fn pre_and_post(&self) -> bool { + matches!(self, Self::All | Self::PreAndPost) + } + + /// Whether the try-state checks are selected. + pub fn try_state(&self) -> bool { + matches!(self, Self::All | Self::TryState) + } + + /// Whether to run any checks at all. + pub fn any(&self) -> bool { + !matches!(self, Self::None) + } +} + +#[cfg(feature = "std")] +impl core::str::FromStr for UpgradeCheckSelect { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "none" => Ok(Self::None), + "all" => Ok(Self::All), + "pre-and-post" => Ok(Self::PreAndPost), + "try-state" => Ok(Self::TryState), + _ => Err("Invalid CheckSelector"), + } + } } /// Execute some checks to ensure the internal state of a pallet is consistent. diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index feb889c73c2f..c441d4c371af 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -2390,8 +2390,7 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // The version isn't changed, we should detect it. assert!( - Executive::try_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) - .unwrap_err() == + Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap_err() == "On chain and in-code storage version do not match. Missing runtime upgrade?" .into() ); @@ -2402,18 +2401,14 @@ fn post_runtime_upgrade_detects_storage_version_issues() { Example::on_genesis(); // We set the new storage version in the pallet and that should be detected. UpdateStorageVersion::set(&true); - Executive::try_runtime_upgrade(UpgradeCheckSelect::new().pre_and_post(true).build()) - .unwrap(); + Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap(); }); TestExternalities::default().execute_with(|| { // Call `on_genesis` to put the storage version of `Example` into the storage. Example::on_genesis(); // We set the new storage version in the custom upgrade and that should be detected. - ExecutiveWithUpgrade::try_runtime_upgrade( - UpgradeCheckSelect::new().pre_and_post(true).build(), - ) - .unwrap(); + ExecutiveWithUpgrade::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap(); }); TestExternalities::default().execute_with(|| { @@ -2425,10 +2420,8 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // `CustomUpgradePallet4` will set a storage version for `Example4` while this doesn't has // any storage version "enabled". assert!( - ExecutiveWithUpgradePallet4::try_runtime_upgrade( - UpgradeCheckSelect::new().pre_and_post(true).build() - ) - .unwrap_err() == "On chain storage version set, while the pallet \ + ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) + .unwrap_err() == "On chain storage version set, while the pallet \ doesn't have the `#[pallet::storage_version(VERSION)]` attribute." .into() ); From 1a8e86ca8d3bd06292ddb9445ce88886728ed119 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Wed, 29 May 2024 18:48:13 +0400 Subject: [PATCH 09/29] use Vec --- .../examples/multi-block-migrations/src/migrations/v1/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index a8afa4a0b085..a056ee055bd9 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -32,6 +32,9 @@ use frame_support::{ #[cfg(feature = "try-runtime")] use sp_std::collections::btree_map::BTreeMap; +#[cfg(feature = "try-runtime")] +use sp_std::vec::Vec; + mod benchmarks; mod tests; pub mod weights; From 30121c5e230a8e2f6d166050373458516b39ed2e Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Wed, 29 May 2024 20:43:12 +0400 Subject: [PATCH 10/29] prdoc --- Cargo.lock | 2 -- .../bridge-hubs/bridge-hub-rococo/Cargo.toml | 9 -------- .../bridge-hubs/bridge-hub-rococo/src/lib.rs | 21 ------------------- prdoc/pr_4251.prdoc | 21 +++++++++++++++++++ substrate/frame/try-runtime/src/inner.rs | 6 ++++-- templates/solochain/runtime/src/lib.rs | 4 ++-- 6 files changed, 27 insertions(+), 36 deletions(-) create mode 100644 prdoc/pr_4251.prdoc diff --git a/Cargo.lock b/Cargo.lock index 48de1933fb80..1b568f2ca890 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2048,9 +2048,7 @@ dependencies = [ "pallet-bridge-parachains", "pallet-bridge-relayers", "pallet-collator-selection", - "pallet-example-mbm", "pallet-message-queue", - "pallet-migrations", "pallet-multisig", "pallet-session", "pallet-timestamp", diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml index 60e24dcc16c6..af243998d43a 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml @@ -42,8 +42,6 @@ pallet-timestamp = { path = "../../../../../substrate/frame/timestamp", default- pallet-transaction-payment = { path = "../../../../../substrate/frame/transaction-payment", default-features = false } pallet-transaction-payment-rpc-runtime-api = { path = "../../../../../substrate/frame/transaction-payment/rpc/runtime-api", default-features = false } pallet-utility = { path = "../../../../../substrate/frame/utility", default-features = false } -pallet-example-mbm = { path = "../../../../../substrate/frame/examples/multi-block-migrations", default-features = false } -pallet-migrations = { path = "../../../../../substrate/frame/migrations", default-features = false } sp-api = { path = "../../../../../substrate/primitives/api", default-features = false } sp-block-builder = { path = "../../../../../substrate/primitives/block-builder", default-features = false } sp-consensus-aura = { path = "../../../../../substrate/primitives/consensus/aura", default-features = false } @@ -122,7 +120,6 @@ snowbridge-runtime-common = { path = "../../../../../bridges/snowbridge/runtime/ bridge-hub-common = { path = "../common", default-features = false } - [dev-dependencies] static_assertions = "1.1" bridge-hub-test-utils = { path = "../test-utils" } @@ -177,9 +174,7 @@ std = [ "pallet-bridge-parachains/std", "pallet-bridge-relayers/std", "pallet-collator-selection/std", - "pallet-example-mbm/std", "pallet-message-queue/std", - "pallet-migrations/std", "pallet-multisig/std", "pallet-session/std", "pallet-timestamp/std", @@ -246,9 +241,7 @@ runtime-benchmarks = [ "pallet-bridge-parachains/runtime-benchmarks", "pallet-bridge-relayers/runtime-benchmarks", "pallet-collator-selection/runtime-benchmarks", - "pallet-example-mbm/runtime-benchmarks", "pallet-message-queue/runtime-benchmarks", - "pallet-migrations/runtime-benchmarks", "pallet-multisig/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", "pallet-utility/runtime-benchmarks", @@ -288,9 +281,7 @@ try-runtime = [ "pallet-bridge-parachains/try-runtime", "pallet-bridge-relayers/try-runtime", "pallet-collator-selection/try-runtime", - "pallet-example-mbm/try-runtime", "pallet-message-queue/try-runtime", - "pallet-migrations/try-runtime", "pallet-multisig/try-runtime", "pallet-session/try-runtime", "pallet-timestamp/try-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 fc6d8d1f4996..0c72b000c2a0 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 @@ -68,7 +68,6 @@ use frame_support::{ construct_runtime, derive_impl, dispatch::DispatchClass, genesis_builder_helper::{build_state, get_preset}, - migrations::FreezeChainOnFailedMigration, parameter_types, traits::{ConstBool, ConstU32, ConstU64, ConstU8, Get, TransformOrigin}, weights::{ConstantMultiplier, Weight}, @@ -280,7 +279,6 @@ impl frame_system::Config for Runtime { /// The action to take on a Runtime Upgrade type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode; type MaxConsumers = frame_support::traits::ConstU32<16>; - type MultiBlockMigrator = PalletMigrations; } impl pallet_timestamp::Config for Runtime { @@ -680,8 +678,6 @@ impl snowbridge_pallet_system::Config for Runtime { type InboundDeliveryCost = EthereumInboundQueue; } -impl pallet_example_mbm::Config for Runtime {} - // Create the runtime by composing the FRAME pallets that were previously configured. construct_runtime!( pub enum Runtime @@ -747,26 +743,9 @@ construct_runtime!( // Message Queue. Importantly, is registered last so that messages are processed after // the `on_initialize` hooks of bridging pallets. MessageQueue: pallet_message_queue = 175, - - PalletMigrations: pallet_migrations = 200, } ); -frame_support::parameter_types! { - pub MbmServiceWeight: Weight = Perbill::from_percent(80) * RuntimeBlockWeights::get().max_block; -} - -impl pallet_migrations::Config for Runtime { - type RuntimeEvent = RuntimeEvent; - type MaxServiceWeight = MbmServiceWeight; - type WeightInfo = (); - type Migrations = (); - type CursorMaxLen = ConstU32<65_536>; - type IdentifierMaxLen = ConstU32<256>; - type MigrationStatusHandler = (); - type FailedMigrationHandler = FreezeChainOnFailedMigration; -} - /// Proper alias for bridge GRANDPA pallet used to bridge with the bulletin chain. pub type BridgeRococoBulletinGrandpa = BridgePolkadotBulletinGrandpa; /// Proper alias for bridge messages pallet used to bridge with the bulletin chain. diff --git a/prdoc/pr_4251.prdoc b/prdoc/pr_4251.prdoc new file mode 100644 index 000000000000..c5399afacab3 --- /dev/null +++ b/prdoc/pr_4251.prdoc @@ -0,0 +1,21 @@ +# 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: MBM try-runtime support + +doc: + - audience: Runtime Dev + description: | + - Adds pre_upgrade post_upgrade hook usage to MBM example + - Adds try-runtime gated methods pre_upgrade and post_upgrade on SteppedMigration + - Adds try-runtime gated methods nth_pre_upgrade and nth_post_upgrade on SteppedMigrations + - Modifies pallet_migrations implementation to run pre_upgrade and post_upgrade steps at the appropriate times, and panic in the event of migration failure + - Derives `impl_for_tuples` for `MockedMigrations` + +crates: + - name: pallet-example-mbm + bump: minor + - name: pallet-migrations + bump: minor + - name: frame-support + bump: minor diff --git a/substrate/frame/try-runtime/src/inner.rs b/substrate/frame/try-runtime/src/inner.rs index 37cf792b6ee5..591124e2ad99 100644 --- a/substrate/frame/try-runtime/src/inner.rs +++ b/substrate/frame/try-runtime/src/inner.rs @@ -31,8 +31,10 @@ sp_api::decl_runtime_apis! { /// Returns the consumed weight of the migration in case of a successful one, combined with /// the total allowed block weight of the runtime. /// - /// [`UpgradeCheckSelect`] allows configuration of which checks to run post-upgrade. - fn on_runtime_upgrade(opts: UpgradeCheckSelect) -> (Weight, Weight); + /// If `checks` is `true`, `pre_migrate` and `post_migrate` of each migration and + /// `try_state` of all pallets will be executed. Else, no. If checks are executed, the PoV + /// tracking is likely inaccurate. + fn on_runtime_upgrade(checks: UpgradeCheckSelect) -> (Weight, Weight); /// Execute the given block, but optionally disable state-root and signature checks. /// diff --git a/templates/solochain/runtime/src/lib.rs b/templates/solochain/runtime/src/lib.rs index e583f05bebca..93a56fb0ad78 100644 --- a/templates/solochain/runtime/src/lib.rs +++ b/templates/solochain/runtime/src/lib.rs @@ -554,11 +554,11 @@ impl_runtime_apis! { #[cfg(feature = "try-runtime")] impl frame_try_runtime::TryRuntime for Runtime { - fn on_runtime_upgrade(opts: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { + fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to // have a backtrace here. If any of the pre/post migration checks fail, we shall stop // right here and right now. - let weight = Executive::try_runtime_upgrade(opts).unwrap(); + let weight = Executive::try_runtime_upgrade(checks).unwrap(); (weight, BlockWeights::get().max_block) } From b1ccef132171cc1443c42a04c8fe5753d035026b Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 30 May 2024 14:12:44 +0400 Subject: [PATCH 11/29] address comments --- substrate/frame/migrations/src/lib.rs | 5 +- .../frame/migrations/src/mock_helpers.rs | 15 +++--- substrate/frame/support/src/migrations.rs | 46 +++++++++++-------- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 26174f385404..e1aa140309f6 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -721,7 +721,9 @@ impl Pallet { // If this is the first time running this migration, exec the pre-upgrade hook. #[cfg(feature = "try-runtime")] if !PreUpgradeBytes::::contains_key(&bounded_id) { - let bytes = T::Migrations::nth_pre_upgrade(cursor.index).expect("Pre-upgrade failed"); + let bytes = T::Migrations::nth_pre_upgrade(cursor.index) + .expect("Invalid cursor.index") + .expect("Pre-upgrade failed"); PreUpgradeBytes::::insert(&bounded_id, PreUpgradeBytesWrapper(bytes)); } @@ -766,6 +768,7 @@ impl Pallet { cursor.index, PreUpgradeBytes::::get(&bounded_id).0, ) + .expect("Invalid cursor.index.") .expect("Post-upgrade failed."); Self::deposit_event(Event::MigrationCompleted { index: cursor.index, took }); diff --git a/substrate/frame/migrations/src/mock_helpers.rs b/substrate/frame/migrations/src/mock_helpers.rs index 5377e513e59c..ef9d14c8518d 100644 --- a/substrate/frame/migrations/src/mock_helpers.rs +++ b/substrate/frame/migrations/src/mock_helpers.rs @@ -124,25 +124,28 @@ impl SteppedMigrations for MockedMigrations { } #[cfg(feature = "try-runtime")] - fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError> { + fn nth_pre_upgrade(n: u32) -> Option, sp_runtime::TryRuntimeError>> { let (kind, _) = MIGRATIONS::get()[n as usize]; if let PreUpgradeFail = kind { - return Err("Some pre-upgrade error".into()) + return Some(Err("Some pre-upgrade error".into())) } - Ok(vec![]) + Some(Ok(vec![])) } #[cfg(feature = "try-runtime")] - fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + fn nth_post_upgrade( + n: u32, + _state: Vec, + ) -> Option> { let (kind, _) = MIGRATIONS::get()[n as usize]; if let PostUpgradeFail = kind { - return Err("Some post-upgrade error".into()) + return Some(Err("Some post-upgrade error".into())) } - Ok(()) + Some(Ok(())) } fn cursor_max_encoded_len() -> usize { diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index 0c392b37a129..b6920d8fdf66 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -605,11 +605,16 @@ pub trait SteppedMigrations { #[cfg(feature = "try-runtime")] /// Call the pre-upgrade hooks of the `n`th migration. - fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError>; + /// + /// Returns `None` if the index is out of bounds. + fn nth_pre_upgrade(n: u32) -> Option, sp_runtime::TryRuntimeError>>; #[cfg(feature = "try-runtime")] /// Call the post-upgrade hooks of the `n`th migration. - fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError>; + /// + /// Returns `None` if the index is out of bounds. + fn nth_post_upgrade(n: u32, _state: Vec) + -> Option>; /// The maximal encoded length across all cursors. fn cursor_max_encoded_len() -> usize; @@ -675,13 +680,16 @@ impl SteppedMigrations for () { } #[cfg(feature = "try-runtime")] - fn nth_pre_upgrade(_n: u32) -> Result, sp_runtime::TryRuntimeError> { - Ok(Vec::new()) + fn nth_pre_upgrade(_n: u32) -> Option, sp_runtime::TryRuntimeError>> { + Some(Ok(Vec::new())) } #[cfg(feature = "try-runtime")] - fn nth_post_upgrade(_n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { - Ok(()) + fn nth_post_upgrade( + _n: u32, + _state: Vec, + ) -> Option> { + Some(Ok(())) } fn cursor_max_encoded_len() -> usize { @@ -711,11 +719,11 @@ impl SteppedMigrations for T { } fn nth_step( - _n: u32, + n: u32, cursor: Option>, meter: &mut WeightMeter, ) -> Option>, SteppedMigrationError>> { - if !_n.is_zero() { + if !n.is_zero() { defensive!("nth_step should only be called with n==0"); return None } @@ -755,20 +763,20 @@ impl SteppedMigrations for T { } #[cfg(feature = "try-runtime")] - fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError> { + fn nth_pre_upgrade(n: u32) -> Option, sp_runtime::TryRuntimeError>> { if n != 0 { - defensive!("nth_transactional_step should only be called with n==0"); + defensive!("nth_pre_upgrade should only be called with n==0"); } - T::pre_upgrade() + Some(T::pre_upgrade()) } #[cfg(feature = "try-runtime")] - fn nth_post_upgrade(n: u32, state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + fn nth_post_upgrade(n: u32, state: Vec) -> Option> { if n != 0 { - defensive!("nth_transactional_step should only be called with n==0"); + defensive!("nth_post_upgrade should only be called with n==0"); } - T::post_upgrade(state) + Some(T::post_upgrade(state)) } fn cursor_max_encoded_len() -> usize { @@ -837,7 +845,7 @@ impl SteppedMigrations for Tuple { } #[cfg(feature = "try-runtime")] - fn nth_pre_upgrade(n: u32) -> Result, sp_runtime::TryRuntimeError> { + fn nth_pre_upgrade(n: u32) -> Option, sp_runtime::TryRuntimeError>> { let mut i = 0; for_tuples! ( #( @@ -848,22 +856,22 @@ impl SteppedMigrations for Tuple { i += Tuple::len(); )* ); - panic!("n greater than number of migrations") + None } #[cfg(feature = "try-runtime")] - fn nth_post_upgrade(n: u32, _state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + fn nth_post_upgrade(n: u32, state: Vec) -> Option> { let mut i = 0; for_tuples! ( #( if (i + Tuple::len()) > n { - return Tuple::nth_post_upgrade(n - i, _state) + return Tuple::nth_post_upgrade(n - i, state) } i += Tuple::len(); )* ); - panic!("n greater than number of migrations") + None } fn nth_max_steps(n: u32) -> Option> { From 6fbe84390bea1ea6242f14efafcf4f42787af128 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 30 May 2024 18:14:11 +0400 Subject: [PATCH 12/29] address comment --- substrate/frame/migrations/src/lib.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index e1aa140309f6..1c701ac1b911 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -153,11 +153,16 @@ use core::ops::ControlFlow; use frame_support::{ defensive, defensive_assert, migrations::*, + pallet_prelude::*, + storage_alias, traits::Get, weights::{Weight, WeightMeter}, BoundedVec, }; -use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System}; +use frame_system::{ + pallet_prelude::{BlockNumberFor, *}, + Pallet as System, +}; use sp_runtime::Saturating; use sp_std::vec::Vec; @@ -280,11 +285,17 @@ impl MaxEncodedLen for PreUpgradeBytesWrapper { } } +/// Data stored by the pre-upgrade hook of the MBMs. Only used for `try-runtime` testing. +/// +/// Define this outside of the pallet so it is not confused with actual storage. +#[cfg(feature = "try-runtime")] +#[storage_alias] +type PreUpgradeBytes = + StorageMap, Twox64Concat, IdentifierOf, PreUpgradeBytesWrapper, ValueQuery>; + #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::*; #[pallet::pallet] pub struct Pallet(_); @@ -391,12 +402,6 @@ pub mod pallet { #[pallet::storage] pub type Historic = StorageMap<_, Twox64Concat, IdentifierOf, (), OptionQuery>; - /// Data stored by the pre-upgrade hook of the MBM. - #[cfg(feature = "try-runtime")] - #[pallet::storage] - pub type PreUpgradeBytes = - StorageMap<_, Twox64Concat, IdentifierOf, PreUpgradeBytesWrapper, ValueQuery>; - #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { From 1677cd664aee857a62c32de7f5c31c4ac78a522b Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 30 May 2024 18:17:51 +0400 Subject: [PATCH 13/29] address comment --- .../multi-block-migrations/src/migrations/v1/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index a056ee055bd9..2b24c921cc34 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -138,6 +138,13 @@ impl SteppedMigration for LazyMigrationV1::decode(&mut &prev[..]) .expect("Failed to decode the previous storage state"); + // Check the len of prev and post are the same. + assert_eq!( + MyMap::::iter().count(), + prev_map.len(), + "Migration failed: the number of items in the storage after the migration is not the same as before" + ); + for (key, value) in MyMap::::iter() { let prev_value = prev_map.get(&key).expect("Key not found in the previous storage state"); From 1a61af312632a410134851ba0119cc5bf90aa3fd Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 30 May 2024 18:19:01 +0400 Subject: [PATCH 14/29] fix newlines --- substrate/frame/migrations/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 1c701ac1b911..543c25e8318b 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -70,21 +70,26 @@ //! points to the currently active migration and stores its inner cursor. The inner cursor can then //! be used by the migration to store its inner state and advance. Each time when the migration //! returns `Some(cursor)`, it signals the pallet that it is not done yet. +//! //! The cursor is reset on each runtime upgrade. This ensures that it starts to execute at the //! first migration in the vector. The pallets cursor is only ever incremented or set to `Stuck` //! once it encounters an error (Goal 4). Once in the stuck state, the pallet will stay stuck until //! it is fixed through manual governance intervention. +//! //! As soon as the cursor of the pallet becomes `Some(_)`; [`MultiStepMigrator::ongoing`] returns //! `true` (Goal 2). This can be used by upstream code to possibly pause transactions. //! In `on_initialize` the pallet will load the current migration and check whether it was already //! executed in the past by checking for membership of its ID in the [`Historic`] set. Historic //! migrations are skipped without causing an error. Each successfully executed migration is added //! to this set (Goal 5). +//! //! This proceeds until no more migrations remain. At that point, the event `UpgradeCompleted` is //! emitted (Goal 1). +//! //! The execution of each migration happens by calling [`SteppedMigration::transactional_step`]. //! This function wraps the inner `step` function into a transactional layer to allow rollback in //! the error case (Goal 6). +//! //! Weight limits must be checked by the migration itself. The pallet provides a [`WeightMeter`] for //! that purpose. The pallet may return [`SteppedMigrationError::InsufficientWeight`] at any point. //! In that scenario, one of two things will happen: if that migration was exclusively executed From f57b50a68a49b884f14ad6d7aa1ccce3671d1a79 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 30 May 2024 18:25:50 +0400 Subject: [PATCH 15/29] clippy --- substrate/frame/migrations/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 543c25e8318b..bc2511c1a4fa 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -159,7 +159,6 @@ use frame_support::{ defensive, defensive_assert, migrations::*, pallet_prelude::*, - storage_alias, traits::Get, weights::{Weight, WeightMeter}, BoundedVec, @@ -280,8 +279,10 @@ pub trait MockedMigrations: SteppedMigrations { #[cfg(feature = "try-runtime")] /// Wrapper for pre-upgrade bytes, allowing us to impl MEL on it. +/// +/// For `try-runtime` testing only. #[derive(Debug, Clone, Eq, PartialEq, Encode, Decode, scale_info::TypeInfo, Default)] -pub struct PreUpgradeBytesWrapper(pub Vec); +struct PreUpgradeBytesWrapper(pub Vec); #[cfg(feature = "try-runtime")] impl MaxEncodedLen for PreUpgradeBytesWrapper { @@ -294,7 +295,7 @@ impl MaxEncodedLen for PreUpgradeBytesWrapper { /// /// Define this outside of the pallet so it is not confused with actual storage. #[cfg(feature = "try-runtime")] -#[storage_alias] +#[frame_support::storage_alias] type PreUpgradeBytes = StorageMap, Twox64Concat, IdentifierOf, PreUpgradeBytesWrapper, ValueQuery>; From 028aa60ed26e6687e39527765fca6f081a49cc85 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Fri, 7 Jun 2024 13:15:13 +0800 Subject: [PATCH 16/29] ci: test without try-runtime feature --- .gitlab/pipeline/test.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index d171a8a19426..70a56f324f52 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -176,6 +176,29 @@ test-linux-stable-runtime-benchmarks: script: - time cargo nextest run --workspace --features runtime-benchmarks benchmark --locked --cargo-profile testnet +# some tests do not run with `try-runtime` feature enabled +# https://github.com/paritytech/polkadot-sdk/pull/4251#discussion_r1624282143 +test-linux-stable-no-try-runtime: + stage: test + extends: + - .docker-env + - .common-refs + - .run-immediately + - .pipeline-stopper-artifacts + variables: + RUST_TOOLCHAIN: stable + # Enable debug assertions since we are running optimized builds for testing + # but still want to have debug assertions. + RUSTFLAGS: "-Cdebug-assertions=y -Dwarnings" + script: + - > + time cargo nextest run \ + --workspace \ + --locked \ + --release \ + --no-fail-fast \ + --features experimental,riscv,ci-only-tests \ + # can be used to run all tests # test-linux-stable-all: # stage: test From 06eb24744156d399c5f983d0f30e1556d20476cf Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Fri, 7 Jun 2024 18:58:18 +0800 Subject: [PATCH 17/29] cfg_attr --- substrate/frame/migrations/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index bc2511c1a4fa..e5159c473b95 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -809,7 +809,7 @@ impl Pallet { /// /// When the `try-runtime` feature is enabled, this function will panic. // Allow unreachable code so it can compile without warnings when `try-runtime` is enabled. - #[allow(unreachable_code)] + #[cfg_attr(feature = "try-runtime", allow(unreachable_code))] fn upgrade_failed(migration: Option) { use FailedMigrationHandling::*; Self::deposit_event(Event::UpgradeFailed); From ba768dcb629bdde2c2a64735aefea318f01f46b8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 12 Sep 2024 11:40:02 +0200 Subject: [PATCH 18/29] Cleanup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 + .../src/migrations/v1/mod.rs | 10 ++++++---- substrate/frame/migrations/Cargo.toml | 1 + substrate/frame/migrations/src/lib.rs | 19 ++++++++----------- substrate/frame/support/src/migrations.rs | 12 +++++++----- 5 files changed, 23 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0a089390450..80bf9adbfe4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10944,6 +10944,7 @@ dependencies = [ name = "pallet-migrations" version = "1.0.0" dependencies = [ + "cfg-if", "docify", "frame-benchmarking", "frame-executive", diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index 2b24c921cc34..aee097bb726b 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -145,10 +145,12 @@ impl SteppedMigration for LazyMigrationV1::iter() { - let prev_value = - prev_map.get(&key).expect("Key not found in the previous storage state"); - assert_eq!(value as u32, *prev_value, "Migration failed for key {}", key); + for (key, value) in prev_map { + let value = MyMap::::get(key).expect("Failed to get the value after the migration"); + assert_eq!( + value, value as u64, + "Migration failed: the value after the migration is not the same as before" + ); } Ok(()) diff --git a/substrate/frame/migrations/Cargo.toml b/substrate/frame/migrations/Cargo.toml index 5fbed74a4400..a32e48e65280 100644 --- a/substrate/frame/migrations/Cargo.toml +++ b/substrate/frame/migrations/Cargo.toml @@ -12,6 +12,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { features = ["derive"], workspace = true } +cfg-if = { workspace = true } docify = { workspace = true } impl-trait-for-tuples = { workspace = true } log = { workspace = true, default-features = true } diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index ccc09c6c099c..8327266fd849 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -811,21 +811,18 @@ impl Pallet { /// /// When the `try-runtime` feature is enabled, this function will panic. // Allow unreachable code so it can compile without warnings when `try-runtime` is enabled. - #[cfg_attr(feature = "try-runtime", allow(unreachable_code))] fn upgrade_failed(migration: Option) { use FailedMigrationHandling::*; Self::deposit_event(Event::UpgradeFailed); - #[cfg(feature = "try-runtime")] - { - log::error!("Migration at index {:?} failed.", migration); - panic!("A multi-block migration failed. Run with RUST_LOG=trace for all logs."); - } - - match T::FailedMigrationHandler::failed(migration) { - KeepStuck => Cursor::::set(Some(MigrationCursor::Stuck)), - ForceUnstuck => Cursor::::kill(), - Ignore => {}, + if cfg!(feature = "try-runtime") { + panic!("Migration with index {:?} failed.", migration); + } else { + match T::FailedMigrationHandler::failed(migration) { + KeepStuck => Cursor::::set(Some(MigrationCursor::Stuck)), + ForceUnstuck => Cursor::::kill(), + Ignore => {}, + } } } diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index 8d8dcf660ca4..722a3d83ee54 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -530,18 +530,20 @@ pub trait SteppedMigration { .map_err(|()| SteppedMigrationError::Failed)? } - /// Executed prior to the migration starting. + /// Hook for testing that is run before the migration is started. /// /// Returns some bytes which are passed into `post_upgrade` after the migration is completed. + /// This is not run for the real migration, so panicking is not an issue here. #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { Ok(Vec::new()) } - /// Executed after the migration is completed. + /// Hook for testing that is run after the migration is completed. /// /// Should be used to verify the state of the chain after the migration. The `state` parameter - /// is the return value from `pre_upgrade`. + /// is the return value from `pre_upgrade`. This is not run for the real migration, so panicking + /// is not an issue here. #[cfg(feature = "try-runtime")] fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { Ok(()) @@ -716,16 +718,16 @@ pub trait SteppedMigrations { meter: &mut WeightMeter, ) -> Option>, SteppedMigrationError>>; - #[cfg(feature = "try-runtime")] /// Call the pre-upgrade hooks of the `n`th migration. /// /// Returns `None` if the index is out of bounds. + #[cfg(feature = "try-runtime")] fn nth_pre_upgrade(n: u32) -> Option, sp_runtime::TryRuntimeError>>; - #[cfg(feature = "try-runtime")] /// Call the post-upgrade hooks of the `n`th migration. /// /// Returns `None` if the index is out of bounds. + #[cfg(feature = "try-runtime")] fn nth_post_upgrade(n: u32, _state: Vec) -> Option>; From 97a1cbd863cecb603b1fefd405e979ad5e412b3a Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 12 Sep 2024 10:32:34 +0200 Subject: [PATCH 19/29] replace sp-std --- Cargo.lock | 1 - substrate/frame/examples/multi-block-migrations/Cargo.toml | 1 - .../examples/multi-block-migrations/src/migrations/v1/mod.rs | 4 ++-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 80bf9adbfe4e..86797a291eec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10666,7 +10666,6 @@ dependencies = [ "parity-scale-codec", "scale-info", "sp-io", - "sp-std 14.0.0", ] [[package]] diff --git a/substrate/frame/examples/multi-block-migrations/Cargo.toml b/substrate/frame/examples/multi-block-migrations/Cargo.toml index b59464eda1c4..6af0f700e6c9 100644 --- a/substrate/frame/examples/multi-block-migrations/Cargo.toml +++ b/substrate/frame/examples/multi-block-migrations/Cargo.toml @@ -34,7 +34,6 @@ std = [ "pallet-migrations/std", "scale-info/std", "sp-io/std", - "sp-std/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index aee097bb726b..dd676175ed1c 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -30,10 +30,10 @@ use frame_support::{ }; #[cfg(feature = "try-runtime")] -use sp_std::collections::btree_map::BTreeMap; +use alloc::collections::btree_map::BTreeMap; #[cfg(feature = "try-runtime")] -use sp_std::vec::Vec; +use alloc::vec::Vec; mod benchmarks; mod tests; From f610140219b4858acedf009812d4ac5d6942e9d0 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Thu, 12 Sep 2024 10:42:14 +0200 Subject: [PATCH 20/29] adds external crate alloc --- substrate/frame/examples/multi-block-migrations/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/substrate/frame/examples/multi-block-migrations/src/lib.rs b/substrate/frame/examples/multi-block-migrations/src/lib.rs index 657c42b1662c..c9c78c3dbe90 100644 --- a/substrate/frame/examples/multi-block-migrations/src/lib.rs +++ b/substrate/frame/examples/multi-block-migrations/src/lib.rs @@ -69,6 +69,8 @@ pub mod migrations; mod mock; +extern crate alloc; + pub use pallet::*; #[frame_support::pallet] From 1c969d88c6df431a4709c31f3175320507cdd9d7 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 12 Sep 2024 11:47:44 +0200 Subject: [PATCH 21/29] Cleanup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 + substrate/frame/migrations/src/lib.rs | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 86797a291eec..80bf9adbfe4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10666,6 +10666,7 @@ dependencies = [ "parity-scale-codec", "scale-info", "sp-io", + "sp-std 14.0.0", ] [[package]] diff --git a/substrate/frame/migrations/src/lib.rs b/substrate/frame/migrations/src/lib.rs index 8327266fd849..f1265f10ed07 100644 --- a/substrate/frame/migrations/src/lib.rs +++ b/substrate/frame/migrations/src/lib.rs @@ -286,13 +286,6 @@ pub trait MockedMigrations: SteppedMigrations { #[derive(Debug, Clone, Eq, PartialEq, Encode, Decode, scale_info::TypeInfo, Default)] struct PreUpgradeBytesWrapper(pub Vec); -#[cfg(feature = "try-runtime")] -impl MaxEncodedLen for PreUpgradeBytesWrapper { - fn max_encoded_len() -> usize { - 0 - } -} - /// Data stored by the pre-upgrade hook of the MBMs. Only used for `try-runtime` testing. /// /// Define this outside of the pallet so it is not confused with actual storage. From ad1811a25f7d7a7161e514b8025d9fcb87e71a54 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 12 Sep 2024 12:01:52 +0200 Subject: [PATCH 22/29] cleanup Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/migrations/src/tests.rs | 34 ++++++++----------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/substrate/frame/migrations/src/tests.rs b/substrate/frame/migrations/src/tests.rs index df5420f820dc..f5bb974a20f6 100644 --- a/substrate/frame/migrations/src/tests.rs +++ b/substrate/frame/migrations/src/tests.rs @@ -17,25 +17,14 @@ #![cfg(test)] -use frame_support::traits::OnRuntimeUpgrade; +use frame_support::{pallet_prelude::Weight, traits::OnRuntimeUpgrade}; -#[cfg(not(feature = "try-runtime"))] use crate::{ mock::{Test as T, *}, mock_helpers::{MockedMigrationKind::*, *}, Cursor, Event, FailedMigrationHandling, MigrationCursor, }; -#[cfg(feature = "try-runtime")] -use crate::{ - mock::*, - mock_helpers::{MockedMigrationKind::*, *}, - Event, -}; - -#[cfg(not(feature = "try-runtime"))] -use frame_support::pallet_prelude::Weight; - #[docify::export] #[test] fn simple_works() { @@ -72,8 +61,8 @@ fn simple_works() { }); } -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn failing_migration_sets_cursor_to_stuck() { test_closure(|| { FailedUpgradeResponse::set(FailedMigrationHandling::KeepStuck); @@ -103,8 +92,8 @@ fn failing_migration_sets_cursor_to_stuck() { }); } -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn failing_migration_force_unstuck_works() { test_closure(|| { FailedUpgradeResponse::set(FailedMigrationHandling::ForceUnstuck); @@ -136,8 +125,8 @@ fn failing_migration_force_unstuck_works() { /// A migration that reports not getting enough weight errors if it is the first one to run in that /// block. -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn high_weight_migration_singular_fails() { test_closure(|| { MockedMigrations::set(vec![(HighWeightAfter(Weight::zero()), 2)]); @@ -165,8 +154,8 @@ fn high_weight_migration_singular_fails() { /// A migration that reports of not getting enough weight is retried once, if it is not the first /// one to run in a block. -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn high_weight_migration_retries_once() { test_closure(|| { MockedMigrations::set(vec![(SucceedAfter, 0), (HighWeightAfter(Weight::zero()), 0)]); @@ -195,8 +184,8 @@ fn high_weight_migration_retries_once() { /// not the first one in the block. // Note: Same as `high_weight_migration_retries_once` but with different required weight for the // migration. -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn high_weight_migration_permanently_overweight_fails() { test_closure(|| { MockedMigrations::set(vec![(SucceedAfter, 0), (HighWeightAfter(Weight::MAX), 0)]); @@ -291,8 +280,8 @@ fn historic_skipping_works() { /// When another upgrade happens while a migration is still running, it should set the cursor to /// stuck. -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn upgrade_fails_when_migration_active() { test_closure(|| { MockedMigrations::set(vec![(SucceedAfter, 10)]); @@ -318,8 +307,8 @@ fn upgrade_fails_when_migration_active() { }); } -#[cfg(not(feature = "try-runtime"))] #[test] +#[cfg_attr(feature = "try-runtime", should_panic)] fn migration_timeout_errors() { test_closure(|| { MockedMigrations::set(vec![(TimeoutAfter, 3)]); @@ -379,8 +368,8 @@ fn try_runtime_success_case() { }); } -#[cfg(feature = "try-runtime")] #[test] +#[cfg(feature = "try-runtime")] #[should_panic] fn try_runtime_pre_upgrade_failure() { test_closure(|| { @@ -395,8 +384,8 @@ fn try_runtime_pre_upgrade_failure() { }); } -#[cfg(feature = "try-runtime")] #[test] +#[cfg(feature = "try-runtime")] #[should_panic] fn try_runtime_post_upgrade_failure() { test_closure(|| { @@ -411,8 +400,8 @@ fn try_runtime_post_upgrade_failure() { }); } -#[cfg(feature = "try-runtime")] #[test] +#[cfg(feature = "try-runtime")] #[should_panic] fn try_runtime_migration_failure() { test_closure(|| { @@ -427,7 +416,6 @@ fn try_runtime_migration_failure() { }); } -#[cfg(feature = "try-runtime")] #[test] fn try_runtime_no_migrations() { test_closure(|| { From 666db60dc2300fbbea7ef3ddb9b327dcd5a23f0e Mon Sep 17 00:00:00 2001 From: ggwpez Date: Thu, 12 Sep 2024 10:08:55 +0000 Subject: [PATCH 23/29] Add PrDoc (auto generated) --- prdoc/pr_4251.prdoc | 76 +++++++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/prdoc/pr_4251.prdoc b/prdoc/pr_4251.prdoc index c5399afacab3..77e890fe0357 100644 --- a/prdoc/pr_4251.prdoc +++ b/prdoc/pr_4251.prdoc @@ -1,21 +1,59 @@ -# 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: MBM try-runtime support - +title: MBM `try-runtime` support doc: - - audience: Runtime Dev - description: | - - Adds pre_upgrade post_upgrade hook usage to MBM example - - Adds try-runtime gated methods pre_upgrade and post_upgrade on SteppedMigration - - Adds try-runtime gated methods nth_pre_upgrade and nth_post_upgrade on SteppedMigrations - - Modifies pallet_migrations implementation to run pre_upgrade and post_upgrade steps at the appropriate times, and panic in the event of migration failure - - Derives `impl_for_tuples` for `MockedMigrations` - +- audience: Runtime Dev + description: "# MBM try-runtime support\n\nThis MR adds support to the try-runtime\ + \ trait such that the try-runtime-CLI will be able to support MBM testing [here](https://github.com/paritytech/try-runtime-cli/pull/90).\ + \ It mainly adds two feature-gated hooks to the `SteppedMigration` hook to facilitate\ + \ testing. These hooks are named `pre_upgrade` and `post_upgrade` and have the\ + \ same signature and implications as for single-block migrations.\n\n## Integration\n\ + \nTo make use of this in your Multi-Block-Migration, just implement the two new\ + \ hooks and test pre- and post-conditions in them:\n\n```rust\n#[cfg(feature =\ + \ \"try-runtime\")]\nfn pre_upgrade() -> Result, frame_support::sp_runtime::TryRuntimeError>\ + \ {\n\t// ...\n}\n\n#[cfg(feature = \"try-runtime\")]\nfn post_upgrade(prev: Vec)\ + \ -> Result<(), frame_support::sp_runtime::TryRuntimeError> {\n // ...\n}\n\ + ```\n\nYou may return an error or panic in these functions to indicate failure.\ + \ This will then show up in the try-runtime-CLI and can be used in CI for testing.\n\ + \nChanges:\n- Adds `try-runtime` gated methods `pre_upgrade` and `post_upgrade`\ + \ on `SteppedMigration`\n- Adds `try-runtime` gated methods `nth_pre_upgrade`\ + \ and `nth_post_upgrade` on `SteppedMigrations`\n- Modifies `pallet_migrations`\ + \ implementation to run pre_upgrade and post_upgrade steps at the appropriate\ + \ times, and panic in the event of migration failure." crates: - - name: pallet-example-mbm - bump: minor - - name: pallet-migrations - bump: minor - - name: frame-support - bump: minor +- name: asset-hub-rococo-runtime + bump: minor +- name: asset-hub-westend-runtime + bump: minor +- name: bridge-hub-rococo-runtime + bump: minor +- name: bridge-hub-westend-runtime + bump: minor +- name: collectives-westend-runtime + bump: minor +- name: contracts-rococo-runtime + bump: minor +- name: coretime-rococo-runtime + bump: minor +- name: coretime-westend-runtime + bump: minor +- name: people-rococo-runtime + bump: minor +- name: people-westend-runtime + bump: minor +- name: penpal-runtime + bump: minor +- name: polkadot-parachain-bin + bump: minor +- name: rococo-runtime + bump: minor +- name: westend-runtime + bump: minor +- name: frame-executive + bump: minor +- name: pallet-migrations + bump: minor +- name: frame-support + bump: minor +- name: frame-system + bump: minor +- name: frame-try-runtime + bump: minor From e437426f588c43ca3cd2dd45f48d4a82c52ed0d3 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 12 Sep 2024 12:59:31 +0200 Subject: [PATCH 24/29] review Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 - substrate/frame/examples/multi-block-migrations/Cargo.toml | 1 - substrate/frame/examples/multi-block-migrations/src/lib.rs | 2 -- .../examples/multi-block-migrations/src/migrations/v1/mod.rs | 2 ++ 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 80bf9adbfe4e..86797a291eec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10666,7 +10666,6 @@ dependencies = [ "parity-scale-codec", "scale-info", "sp-io", - "sp-std 14.0.0", ] [[package]] diff --git a/substrate/frame/examples/multi-block-migrations/Cargo.toml b/substrate/frame/examples/multi-block-migrations/Cargo.toml index 6af0f700e6c9..98569964a9c9 100644 --- a/substrate/frame/examples/multi-block-migrations/Cargo.toml +++ b/substrate/frame/examples/multi-block-migrations/Cargo.toml @@ -21,7 +21,6 @@ frame-benchmarking = { optional = true, workspace = true } log = { workspace = true } scale-info = { workspace = true } sp-io = { workspace = true } -sp-std = { workspace = true } [features] default = ["std"] diff --git a/substrate/frame/examples/multi-block-migrations/src/lib.rs b/substrate/frame/examples/multi-block-migrations/src/lib.rs index c9c78c3dbe90..657c42b1662c 100644 --- a/substrate/frame/examples/multi-block-migrations/src/lib.rs +++ b/substrate/frame/examples/multi-block-migrations/src/lib.rs @@ -69,8 +69,6 @@ pub mod migrations; mod mock; -extern crate alloc; - pub use pallet::*; #[frame_support::pallet] diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index dd676175ed1c..5e1ea75fd5c4 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -21,6 +21,8 @@ //! [`v0::MyMap`](`crate::migrations::v1::v0::MyMap`) storage map, transforms them, //! and inserts them into the [`MyMap`](`crate::pallet::MyMap`) storage map. +extern crate alloc; + use super::PALLET_MIGRATIONS_ID; use crate::pallet::{Config, MyMap}; use frame_support::{ From 00de16a82f766de0651c0d9bbd5ee9e491b71254 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 12 Sep 2024 13:04:33 +0200 Subject: [PATCH 25/29] prdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_4251.prdoc | 60 ++++++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/prdoc/pr_4251.prdoc b/prdoc/pr_4251.prdoc index 77e890fe0357..255ce681951f 100644 --- a/prdoc/pr_4251.prdoc +++ b/prdoc/pr_4251.prdoc @@ -1,23 +1,49 @@ title: MBM `try-runtime` support doc: - audience: Runtime Dev - description: "# MBM try-runtime support\n\nThis MR adds support to the try-runtime\ - \ trait such that the try-runtime-CLI will be able to support MBM testing [here](https://github.com/paritytech/try-runtime-cli/pull/90).\ - \ It mainly adds two feature-gated hooks to the `SteppedMigration` hook to facilitate\ - \ testing. These hooks are named `pre_upgrade` and `post_upgrade` and have the\ - \ same signature and implications as for single-block migrations.\n\n## Integration\n\ - \nTo make use of this in your Multi-Block-Migration, just implement the two new\ - \ hooks and test pre- and post-conditions in them:\n\n```rust\n#[cfg(feature =\ - \ \"try-runtime\")]\nfn pre_upgrade() -> Result, frame_support::sp_runtime::TryRuntimeError>\ - \ {\n\t// ...\n}\n\n#[cfg(feature = \"try-runtime\")]\nfn post_upgrade(prev: Vec)\ - \ -> Result<(), frame_support::sp_runtime::TryRuntimeError> {\n // ...\n}\n\ - ```\n\nYou may return an error or panic in these functions to indicate failure.\ - \ This will then show up in the try-runtime-CLI and can be used in CI for testing.\n\ - \nChanges:\n- Adds `try-runtime` gated methods `pre_upgrade` and `post_upgrade`\ - \ on `SteppedMigration`\n- Adds `try-runtime` gated methods `nth_pre_upgrade`\ - \ and `nth_post_upgrade` on `SteppedMigrations`\n- Modifies `pallet_migrations`\ - \ implementation to run pre_upgrade and post_upgrade steps at the appropriate\ - \ times, and panic in the event of migration failure." + description: | + # MBM try-runtime support + + This MR adds support to the try-runtime + trait such that the try-runtime-CLI will be able to support MBM testing [here](https://github.com/paritytech/try-runtime-cli/pull/90). + It mainly adds two feature-gated hooks to the `SteppedMigration` hook to facilitate + testing. These hooks are named `pre_upgrade` and `post_upgrade` and have the + same signature and implications as for single-block migrations. + + ## Integration + + + To make use of this in your Multi-Block-Migration, just implement the two new + hooks and test pre- and post-conditions in them: + + ```rust + #[cfg(feature = + "try-runtime")] + fn pre_upgrade() -> Result, frame_support::sp_runtime::TryRuntimeError> + { + t// ... + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev: Vec) + -> Result<(), frame_support::sp_runtime::TryRuntimeError> { + // ... + } + + ``` + + You may return an error or panic in these functions to indicate failure. + This will then show up in the try-runtime-CLI and can be used in CI for testing. + + + Changes: + - Adds `try-runtime` gated methods `pre_upgrade` and `post_upgrade` + on `SteppedMigration` + - Adds `try-runtime` gated methods `nth_pre_upgrade` + and `nth_post_upgrade` on `SteppedMigrations` + - Modifies `pallet_migrations` + implementation to run pre_upgrade and post_upgrade steps at the appropriate + times, and panic in the event of migration failure. crates: - name: asset-hub-rococo-runtime bump: minor From 6e0539f94165517eeb3813d4ff24031bf10c4dcf Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 12 Sep 2024 13:07:30 +0200 Subject: [PATCH 26/29] prdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_4251.prdoc | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/prdoc/pr_4251.prdoc b/prdoc/pr_4251.prdoc index 255ce681951f..4d4fcd734692 100644 --- a/prdoc/pr_4251.prdoc +++ b/prdoc/pr_4251.prdoc @@ -11,39 +11,33 @@ doc: same signature and implications as for single-block migrations. ## Integration - - To make use of this in your Multi-Block-Migration, just implement the two new - hooks and test pre- and post-conditions in them: + To make use of this in your Multi-Block-Migration, just implement the two new hooks and test pre- and post-conditions in them: ```rust - #[cfg(feature = - "try-runtime")] + #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, frame_support::sp_runtime::TryRuntimeError> - { - t// ... + { + // ... } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev: Vec) - -> Result<(), frame_support::sp_runtime::TryRuntimeError> { + fn post_upgrade(prev: Vec) -> Result<(), frame_support::sp_runtime::TryRuntimeError> { // ... } - - ``` + ``` You may return an error or panic in these functions to indicate failure. - This will then show up in the try-runtime-CLI and can be used in CI for testing. + This will then show up in the try-runtime-CLI and can be used in CI for testing. Changes: - Adds `try-runtime` gated methods `pre_upgrade` and `post_upgrade` - on `SteppedMigration` + on `SteppedMigration` - Adds `try-runtime` gated methods `nth_pre_upgrade` - and `nth_post_upgrade` on `SteppedMigrations` + and `nth_post_upgrade` on `SteppedMigrations` - Modifies `pallet_migrations` - implementation to run pre_upgrade and post_upgrade steps at the appropriate - times, and panic in the event of migration failure. + implementation to run pre_upgrade and post_upgrade steps at the appropriate times, and panic in the event of migration failure. crates: - name: asset-hub-rococo-runtime bump: minor From f26c07b84eb24d5fb652f80a88c10734237d6841 Mon Sep 17 00:00:00 2001 From: georgepisaltu Date: Thu, 12 Sep 2024 15:21:21 +0300 Subject: [PATCH 27/29] Fix clippy Signed-off-by: georgepisaltu --- .../examples/multi-block-migrations/src/migrations/v1/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs index 5e1ea75fd5c4..6243846d86b0 100644 --- a/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs +++ b/substrate/frame/examples/multi-block-migrations/src/migrations/v1/mod.rs @@ -148,9 +148,10 @@ impl SteppedMigration for LazyMigrationV1::get(key).expect("Failed to get the value after the migration"); + let new_value = + MyMap::::get(key).expect("Failed to get the value after the migration"); assert_eq!( - value, value as u64, + value as u64, new_value, "Migration failed: the value after the migration is not the same as before" ); } From 475a9760d6cef59426818796bb651ddc18205704 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 16 Sep 2024 22:10:00 +0200 Subject: [PATCH 28/29] Add missing migration to people-rococo Signed-off-by: Oliver Tale-Yazdi --- cumulus/parachains/runtimes/people/people-rococo/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs index cb9177d0c23b..d21c9839f553 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs @@ -109,6 +109,7 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( pallet_collator_selection::migration::v2::MigrationToV2, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, ); From ea00e350d239d4ea7b7add55626e660548aeca54 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 16 Sep 2024 21:04:53 +0200 Subject: [PATCH 29/29] Derive PartialEq on some enums Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/support/src/traits/try_runtime/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs index 09c33c014406..284ba3d7422d 100644 --- a/substrate/frame/support/src/traits/try_runtime/mod.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -28,7 +28,7 @@ use sp_arithmetic::traits::AtLeast32BitUnsigned; use sp_runtime::TryRuntimeError; /// Which state tests to execute. -#[derive(codec::Encode, codec::Decode, Clone, scale_info::TypeInfo)] +#[derive(codec::Encode, codec::Decode, Clone, scale_info::TypeInfo, PartialEq)] pub enum Select { /// None of them. None, @@ -95,7 +95,7 @@ impl std::str::FromStr for Select { } /// Select which checks should be run when trying a runtime upgrade upgrade. -#[derive(codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo)] +#[derive(codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo, PartialEq)] pub enum UpgradeCheckSelect { /// Run no checks. None,