Skip to content

Commit

Permalink
Set frame_system::LastRuntimeUpgrade after running try-runtime mi…
Browse files Browse the repository at this point in the history
…grations (#2515)

Sets `frame_system::LastRuntimeUpgrade` after running try-runtime
migrations to better emulate real behavior.

This fixes an issue where migrations using the spec version to determine
whether to execute can incorrectly fail idempotency checks.

@s0me0ne-unkn0wn noticed this issue with the session key migration
introduced in #2265.
  • Loading branch information
liamaharon authored Nov 28, 2023
1 parent 58a1f9c commit 6dc3e6c
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion substrate/frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,16 +362,27 @@ where
Ok(frame_system::Pallet::<System>::block_weight().total())
}

/// Execute all `OnRuntimeUpgrade` of this runtime.
/// Execute all Migrations of this runtime.
///
/// The `checks` param determines whether to execute `pre/post_upgrade` and `try_state` hooks.
///
/// [`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<Weight, TryRuntimeError> {
let before_all_weight =
<AllPalletsWithSystem as BeforeAllRuntimeMigrations>::before_all_runtime_migrations();
let try_on_runtime_upgrade_weight =
<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade(
checks.pre_and_post(),
)?;

frame_system::LastRuntimeUpgrade::<System>::put(
frame_system::LastRuntimeUpgradeInfo::from(
<System::Version as frame_support::traits::Get<_>>::get(),
),
);

// Nothing should modify the state after the migrations ran:
let _guard = StorageNoopGuard::default();

Expand Down

0 comments on commit 6dc3e6c

Please sign in to comment.