Skip to content

Commit 03d0b02

Browse files
[stable2506] Backport #9451 (#9623)
Backport #9451 into `stable2506` from RomarQ. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Co-authored-by: Rodrigo Quelhas <[email protected]>
1 parent ed54223 commit 03d0b02

File tree

7 files changed

+71
-25
lines changed

7 files changed

+71
-25
lines changed

prdoc/pr_9451.prdoc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
title: 'Call `SingleBlockMigrations` from `frame_system::Config` on `try_on_runtime_upgrade`.'
2+
doc:
3+
- audience: Runtime Dev
4+
description: |-
5+
Fixes a small bug in `try-runtime` code, where `SingleBlockMigrations` from `frame_system::Config` was not called in `try_on_runtime_upgrade`.
6+
crates:
7+
- name: frame-executive
8+
bump: patch

substrate/frame/executive/src/lib.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,10 +424,15 @@ where
424424
pub fn try_runtime_upgrade(checks: UpgradeCheckSelect) -> Result<Weight, TryRuntimeError> {
425425
let before_all_weight =
426426
<AllPalletsWithSystem as BeforeAllRuntimeMigrations>::before_all_runtime_migrations();
427+
427428
let try_on_runtime_upgrade_weight =
428-
<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade(
429-
checks.pre_and_post(),
430-
)?;
429+
<(
430+
COnRuntimeUpgrade,
431+
<System as frame_system::Config>::SingleBlockMigrations,
432+
// We want to run the migrations before we call into the pallets as they may
433+
// access any state that would then not be migrated.
434+
AllPalletsWithSystem,
435+
) as OnRuntimeUpgrade>::try_on_runtime_upgrade(checks.pre_and_post())?;
431436

432437
frame_system::LastRuntimeUpgrade::<System>::put(
433438
frame_system::LastRuntimeUpgradeInfo::from(

substrate/frame/executive/src/tests.rs

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ use sp_runtime::{
4242
};
4343

4444
const TEST_KEY: &[u8] = b":test:key:";
45+
const TEST_KEY_2: &[u8] = b":test:key_2:";
4546

4647
#[frame_support::pallet(dev_mode)]
4748
mod custom {
@@ -354,6 +355,7 @@ impl frame_system::Config for Runtime {
354355
type PostTransactions = MockedSystemCallbacks;
355356
type MultiBlockMigrator = MockedModeGetter;
356357
type ExtensionsWeightInfo = MockExtensionsWeights;
358+
type SingleBlockMigrations = CustomOnRuntimeUpgrade;
357359
}
358360

359361
#[derive(
@@ -479,17 +481,24 @@ type TestBlock = Block<UncheckedXt>;
479481
// Will contain `true` when the custom runtime logic was called.
480482
const CUSTOM_ON_RUNTIME_KEY: &[u8] = b":custom:on_runtime";
481483

482-
struct CustomOnRuntimeUpgrade;
484+
pub struct CustomOnRuntimeUpgrade;
483485
impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade {
484486
fn on_runtime_upgrade() -> Weight {
485487
sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes());
488+
sp_io::storage::set(TEST_KEY_2, "try_runtime_upgrade_works".as_bytes());
486489
sp_io::storage::set(CUSTOM_ON_RUNTIME_KEY, &true.encode());
487490
System::deposit_event(frame_system::Event::CodeUpdated);
488491

489492
assert_eq!(0, System::last_runtime_upgrade_spec_version());
490493

491494
Weight::from_parts(100, 0)
492495
}
496+
497+
#[cfg(feature = "try-runtime")]
498+
fn post_upgrade(_state: Vec<u8>) -> Result<(), TryRuntimeError> {
499+
assert_eq!(&sp_io::storage::get(TEST_KEY_2).unwrap()[..], *b"try_runtime_upgrade_works");
500+
Ok(())
501+
}
493502
}
494503

495504
type Executive = super::Executive<
@@ -1074,20 +1083,15 @@ fn all_weights_are_recorded_correctly() {
10741083
MockedSystemCallbacks::reset();
10751084

10761085
// All weights that show up in the `initialize_block_impl`
1077-
let custom_runtime_upgrade_weight = CustomOnRuntimeUpgrade::on_runtime_upgrade();
1078-
let runtime_upgrade_weight =
1079-
<AllPalletsWithSystem as OnRuntimeUpgrade>::on_runtime_upgrade();
1086+
let runtime_upgrade_weight = Executive::execute_on_runtime_upgrade();
10801087
let on_initialize_weight =
10811088
<AllPalletsWithSystem as OnInitialize<u64>>::on_initialize(block_number);
10821089
let base_block_weight = <Runtime as frame_system::Config>::BlockWeights::get().base_block;
10831090

10841091
// Weights are recorded correctly
10851092
assert_eq!(
10861093
frame_system::Pallet::<Runtime>::block_weight().total(),
1087-
custom_runtime_upgrade_weight +
1088-
runtime_upgrade_weight +
1089-
on_initialize_weight +
1090-
base_block_weight,
1094+
runtime_upgrade_weight + on_initialize_weight + base_block_weight,
10911095
);
10921096
});
10931097
}
@@ -1294,6 +1298,35 @@ fn try_execute_block_works() {
12941298
});
12951299
}
12961300

1301+
#[test]
1302+
#[cfg(feature = "try-runtime")]
1303+
fn try_runtime_upgrade_works() {
1304+
use frame_support::traits::OnGenesis;
1305+
1306+
sp_tracing::init_for_tests();
1307+
1308+
type ExecutiveWithoutMigrations = super::Executive<
1309+
Runtime,
1310+
Block<UncheckedXt>,
1311+
ChainContext<Runtime>,
1312+
Runtime,
1313+
AllPalletsWithSystem,
1314+
>;
1315+
1316+
new_test_ext(1).execute_with(|| {
1317+
// Call `on_genesis` to reset the storage version of all pallets.
1318+
AllPalletsWithSystem::on_genesis();
1319+
1320+
// Make sure the test storages are un-set
1321+
assert!(&sp_io::storage::get(TEST_KEY_2).is_none());
1322+
1323+
ExecutiveWithoutMigrations::try_runtime_upgrade(UpgradeCheckSelect::All).unwrap();
1324+
1325+
// Make sure the test storages were set
1326+
assert_eq!(&sp_io::storage::get(TEST_KEY_2).unwrap()[..], *b"try_runtime_upgrade_works");
1327+
});
1328+
}
1329+
12971330
/// Same as `extrinsic_while_exts_forbidden_errors` but using the try-runtime function.
12981331
#[test]
12991332
#[cfg(feature = "try-runtime")]

templates/parachain/runtime/src/configs/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ parameter_types! {
9797
pub const SS58Prefix: u16 = 42;
9898
}
9999

100+
/// All migrations of the runtime, aside from the ones declared in the pallets.
101+
///
102+
/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`.
103+
#[allow(unused_parens)]
104+
type SingleBlockMigrations = ();
105+
100106
/// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from
101107
/// [`ParaChainDefaultConfig`](`struct@frame_system::config_preludes::ParaChainDefaultConfig`),
102108
/// but overridden as needed.
@@ -127,6 +133,7 @@ impl frame_system::Config for Runtime {
127133
/// The action to take on a Runtime Upgrade
128134
type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode<Self>;
129135
type MaxConsumers = frame_support::traits::ConstU32<16>;
136+
type SingleBlockMigrations = SingleBlockMigrations;
130137
}
131138

132139
/// Configure the palelt weight reclaim tx.

templates/parachain/runtime/src/lib.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,20 +95,13 @@ pub type TxExtension = cumulus_pallet_weight_reclaim::StorageWeightReclaim<
9595
pub type UncheckedExtrinsic =
9696
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, TxExtension>;
9797

98-
/// All migrations of the runtime, aside from the ones declared in the pallets.
99-
///
100-
/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`.
101-
#[allow(unused_parens)]
102-
type Migrations = ();
103-
10498
/// Executive: handles dispatch to the various modules.
10599
pub type Executive = frame_executive::Executive<
106100
Runtime,
107101
Block,
108102
frame_system::ChainContext<Runtime>,
109103
Runtime,
110104
AllPalletsWithSystem,
111-
Migrations,
112105
>;
113106

114107
/// Handles converting a weight scalar to a fee value, based on the scale and granularity of the

templates/solochain/runtime/src/configs/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ parameter_types! {
6060
pub const SS58Prefix: u8 = 42;
6161
}
6262

63+
/// All migrations of the runtime, aside from the ones declared in the pallets.
64+
///
65+
/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`.
66+
#[allow(unused_parens)]
67+
type SingleBlockMigrations = ();
68+
6369
/// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from
6470
/// [`SoloChainDefaultConfig`](`struct@frame_system::config_preludes::SolochainDefaultConfig`),
6571
/// but overridden as needed.
@@ -88,6 +94,7 @@ impl frame_system::Config for Runtime {
8894
/// This is used as an identifier of the chain. 42 is the generic substrate prefix.
8995
type SS58Prefix = SS58Prefix;
9096
type MaxConsumers = frame_support::traits::ConstU32<16>;
97+
type SingleBlockMigrations = SingleBlockMigrations;
9198
}
9299

93100
impl pallet_aura::Config for Runtime {

templates/solochain/runtime/src/lib.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,20 +168,13 @@ pub type UncheckedExtrinsic =
168168
/// The payload being signed in transactions.
169169
pub type SignedPayload = generic::SignedPayload<RuntimeCall, TxExtension>;
170170

171-
/// All migrations of the runtime, aside from the ones declared in the pallets.
172-
///
173-
/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`.
174-
#[allow(unused_parens)]
175-
type Migrations = ();
176-
177171
/// Executive: handles dispatch to the various modules.
178172
pub type Executive = frame_executive::Executive<
179173
Runtime,
180174
Block,
181175
frame_system::ChainContext<Runtime>,
182176
Runtime,
183177
AllPalletsWithSystem,
184-
Migrations,
185178
>;
186179

187180
// Create the runtime by composing the FRAME pallets that were previously configured.

0 commit comments

Comments
 (0)