From 32dae968639cfb97cd99637743ea81e4ab91a42d Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> Date: Tue, 2 Sep 2025 11:47:13 +0100 Subject: [PATCH] Call SingleBlockMigrations from frame_system::Config on try_on_runtime_upgrade (#9451) Recently, when moving the single block migrations from `frame_executive::Executive` to `SingleBlockMigrations` in `frame_system::Config`, I noticed that `try_runtime_upgrade` was ignoring the `SingleBlockMigrations` defined in frame_system. More context at https://github.com/polkadot-fellows/runtimes/pull/844 Based on PR https://github.com/paritytech/polkadot-sdk/pull/1781 and [PRDoc](https://github.com/paritytech/polkadot-sdk/blob/beb9030b249cc078b3955232074a8495e7e0302a/prdoc/1.9.0/pr_1781.prdoc#L29), the new way for providing the single block migrations should be through `SingleBlockMigrations` in `frame_system::Config`. Providing them from `frame_executive::Executive` is still supported, but from what I understood is or will be deprecated. > `SingleBlockMigrations` this is the new way of configuring migrations that run in a single block. Previously they were defined as last generic argument of Executive. This shift is brings all central configuration about migrations closer into view of the developer (migrations that are configured in Executive will still work for now but is deprecated). ## Follow-up Changes Will try to open a pull request tomorrow for deprecating the use of `OnRuntimeUpgrade` in `frame_executive::Executive`. (cherry picked from commit 7753112a1b6aae323af71e8904fbab02fdc73c22) --- prdoc/pr_9451.prdoc | 8 +++ substrate/frame/executive/src/lib.rs | 11 +++-- substrate/frame/executive/src/tests.rs | 49 ++++++++++++++++--- .../parachain/runtime/src/configs/mod.rs | 7 +++ templates/parachain/runtime/src/lib.rs | 7 --- .../solochain/runtime/src/configs/mod.rs | 7 +++ templates/solochain/runtime/src/lib.rs | 7 --- 7 files changed, 71 insertions(+), 25 deletions(-) create mode 100644 prdoc/pr_9451.prdoc diff --git a/prdoc/pr_9451.prdoc b/prdoc/pr_9451.prdoc new file mode 100644 index 0000000000000..2e63d8201aee4 --- /dev/null +++ b/prdoc/pr_9451.prdoc @@ -0,0 +1,8 @@ +title: 'Call `SingleBlockMigrations` from `frame_system::Config` on `try_on_runtime_upgrade`.' +doc: +- audience: Runtime Dev + description: |- + Fixes a small bug in `try-runtime` code, where `SingleBlockMigrations` from `frame_system::Config` was not called in `try_on_runtime_upgrade`. +crates: +- name: frame-executive + bump: patch \ No newline at end of file diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 3ed026ba5606a..ac25bd573c3eb 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -424,10 +424,15 @@ where 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( - checks.pre_and_post(), - )?; + <( + COnRuntimeUpgrade, + ::SingleBlockMigrations, + // We want to run the migrations before we call into the pallets as they may + // access any state that would then not be migrated. + AllPalletsWithSystem, + ) as OnRuntimeUpgrade>::try_on_runtime_upgrade(checks.pre_and_post())?; frame_system::LastRuntimeUpgrade::::put( frame_system::LastRuntimeUpgradeInfo::from( diff --git a/substrate/frame/executive/src/tests.rs b/substrate/frame/executive/src/tests.rs index 8aa0452905c96..6863aa1a4fc1d 100644 --- a/substrate/frame/executive/src/tests.rs +++ b/substrate/frame/executive/src/tests.rs @@ -42,6 +42,7 @@ use sp_runtime::{ }; const TEST_KEY: &[u8] = b":test:key:"; +const TEST_KEY_2: &[u8] = b":test:key_2:"; #[frame_support::pallet(dev_mode)] mod custom { @@ -354,6 +355,7 @@ impl frame_system::Config for Runtime { type PostTransactions = MockedSystemCallbacks; type MultiBlockMigrator = MockedModeGetter; type ExtensionsWeightInfo = MockExtensionsWeights; + type SingleBlockMigrations = CustomOnRuntimeUpgrade; } #[derive( @@ -479,10 +481,11 @@ type TestBlock = Block; // Will contain `true` when the custom runtime logic was called. const CUSTOM_ON_RUNTIME_KEY: &[u8] = b":custom:on_runtime"; -struct CustomOnRuntimeUpgrade; +pub struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { fn on_runtime_upgrade() -> Weight { sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes()); + sp_io::storage::set(TEST_KEY_2, "try_runtime_upgrade_works".as_bytes()); sp_io::storage::set(CUSTOM_ON_RUNTIME_KEY, &true.encode()); System::deposit_event(frame_system::Event::CodeUpdated); @@ -490,6 +493,12 @@ impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { Weight::from_parts(100, 0) } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { + assert_eq!(&sp_io::storage::get(TEST_KEY_2).unwrap()[..], *b"try_runtime_upgrade_works"); + Ok(()) + } } type Executive = super::Executive< @@ -1074,9 +1083,7 @@ fn all_weights_are_recorded_correctly() { MockedSystemCallbacks::reset(); // All weights that show up in the `initialize_block_impl` - let custom_runtime_upgrade_weight = CustomOnRuntimeUpgrade::on_runtime_upgrade(); - let runtime_upgrade_weight = - ::on_runtime_upgrade(); + let runtime_upgrade_weight = Executive::execute_on_runtime_upgrade(); let on_initialize_weight = >::on_initialize(block_number); let base_block_weight = ::BlockWeights::get().base_block; @@ -1084,10 +1091,7 @@ fn all_weights_are_recorded_correctly() { // Weights are recorded correctly assert_eq!( frame_system::Pallet::::block_weight().total(), - custom_runtime_upgrade_weight + - runtime_upgrade_weight + - on_initialize_weight + - base_block_weight, + runtime_upgrade_weight + on_initialize_weight + base_block_weight, ); }); } @@ -1294,6 +1298,35 @@ fn try_execute_block_works() { }); } +#[test] +#[cfg(feature = "try-runtime")] +fn try_runtime_upgrade_works() { + use frame_support::traits::OnGenesis; + + sp_tracing::init_for_tests(); + + type ExecutiveWithoutMigrations = super::Executive< + Runtime, + Block, + ChainContext, + Runtime, + AllPalletsWithSystem, + >; + + new_test_ext(1).execute_with(|| { + // Call `on_genesis` to reset the storage version of all pallets. + AllPalletsWithSystem::on_genesis(); + + // Make sure the test storages are un-set + assert!(&sp_io::storage::get(TEST_KEY_2).is_none()); + + ExecutiveWithoutMigrations::try_runtime_upgrade(UpgradeCheckSelect::All).unwrap(); + + // Make sure the test storages were set + assert_eq!(&sp_io::storage::get(TEST_KEY_2).unwrap()[..], *b"try_runtime_upgrade_works"); + }); +} + /// Same as `extrinsic_while_exts_forbidden_errors` but using the try-runtime function. #[test] #[cfg(feature = "try-runtime")] diff --git a/templates/parachain/runtime/src/configs/mod.rs b/templates/parachain/runtime/src/configs/mod.rs index 70f343a23f458..eb840a410b499 100644 --- a/templates/parachain/runtime/src/configs/mod.rs +++ b/templates/parachain/runtime/src/configs/mod.rs @@ -97,6 +97,12 @@ parameter_types! { pub const SS58Prefix: u16 = 42; } +/// All migrations of the runtime, aside from the ones declared in the pallets. +/// +/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. +#[allow(unused_parens)] +type SingleBlockMigrations = (); + /// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from /// [`ParaChainDefaultConfig`](`struct@frame_system::config_preludes::ParaChainDefaultConfig`), /// but overridden as needed. @@ -127,6 +133,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 SingleBlockMigrations = SingleBlockMigrations; } /// Configure the palelt weight reclaim tx. diff --git a/templates/parachain/runtime/src/lib.rs b/templates/parachain/runtime/src/lib.rs index da6536fd8986a..e70d5cd73c11b 100644 --- a/templates/parachain/runtime/src/lib.rs +++ b/templates/parachain/runtime/src/lib.rs @@ -95,12 +95,6 @@ pub type TxExtension = cumulus_pallet_weight_reclaim::StorageWeightReclaim< pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; -/// All migrations of the runtime, aside from the ones declared in the pallets. -/// -/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. -#[allow(unused_parens)] -type Migrations = (); - /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< Runtime, @@ -108,7 +102,6 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - Migrations, >; /// Handles converting a weight scalar to a fee value, based on the scale and granularity of the diff --git a/templates/solochain/runtime/src/configs/mod.rs b/templates/solochain/runtime/src/configs/mod.rs index e34b3cb821589..b8810a068036c 100644 --- a/templates/solochain/runtime/src/configs/mod.rs +++ b/templates/solochain/runtime/src/configs/mod.rs @@ -60,6 +60,12 @@ parameter_types! { pub const SS58Prefix: u8 = 42; } +/// All migrations of the runtime, aside from the ones declared in the pallets. +/// +/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. +#[allow(unused_parens)] +type SingleBlockMigrations = (); + /// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from /// [`SoloChainDefaultConfig`](`struct@frame_system::config_preludes::SolochainDefaultConfig`), /// but overridden as needed. @@ -88,6 +94,7 @@ impl frame_system::Config for Runtime { /// This is used as an identifier of the chain. 42 is the generic substrate prefix. type SS58Prefix = SS58Prefix; type MaxConsumers = frame_support::traits::ConstU32<16>; + type SingleBlockMigrations = SingleBlockMigrations; } impl pallet_aura::Config for Runtime { diff --git a/templates/solochain/runtime/src/lib.rs b/templates/solochain/runtime/src/lib.rs index 3e81638f3f54e..2fe44c7877462 100644 --- a/templates/solochain/runtime/src/lib.rs +++ b/templates/solochain/runtime/src/lib.rs @@ -168,12 +168,6 @@ pub type UncheckedExtrinsic = /// The payload being signed in transactions. pub type SignedPayload = generic::SignedPayload; -/// All migrations of the runtime, aside from the ones declared in the pallets. -/// -/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. -#[allow(unused_parens)] -type Migrations = (); - /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< Runtime, @@ -181,7 +175,6 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - Migrations, >; // Create the runtime by composing the FRAME pallets that were previously configured.