Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Disarm OnRuntimeUpgrade::pre/post_upgrade Tuple footgun #14759

23 changes: 23 additions & 0 deletions frame/support/src/traits/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,24 @@ impl OnRuntimeUpgrade for Tuple {

Ok(weight)
}

/// `Executive` used to directly call AllPalletsWithSystem::post_upgrade() which is now a noop.
/// If a runtime is running an old version of `Exeuctive`, return an error instead of silently
/// doing nothing which can lead to confusion. See
/// <https://github.com/paritytech/substrate/issues/13681>.
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
Err("If you see this error, the implementation of `pub fn try_runtime_upgrade` in your `Executive` pallet is probably old and `pre_upgrade` checks will *not* be executed. Please update your implementation of `Executive` `pub fn try_runtime_upgrade` to match Substrate master (https://github.com/paritytech/substrate/blob/master/frame/executive/src/lib.rs).".into())
}

/// `Executive` used to directly call AllPalletsWithSystem::post_upgrade() which is now a noop.
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
/// If a runtime is running an old version of `Exeuctive`, return an error instead of silently
/// doing nothing which can lead to confusion. See
/// <https://github.com/paritytech/substrate/issues/13681>.
#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), TryRuntimeError> {
Err("If you see this error, the implementation of `pub fn try_runtime_upgrade` in your `Executive` pallet is probably old and `post_upgrade` checks will *not* be executed. Please update your implementation of `Executive` `pub fn try_runtime_upgrade` to match Substrate master (https://github.com/paritytech/substrate/blob/master/frame/executive/src/lib.rs).".into())
}
}

/// See [`Hooks::integrity_test`].
Expand Down Expand Up @@ -492,6 +510,7 @@ mod tests {
impl_test_type!(Baz);

TestExternalities::default().execute_with(|| {
// try_on_runtime_upgrade works
Foo::try_on_runtime_upgrade(true).unwrap();
assert_eq!(Pre::take(), vec!["Foo"]);
assert_eq!(Post::take(), vec!["Foo"]);
Expand All @@ -507,6 +526,10 @@ mod tests {
<(Foo, (Bar, Baz))>::try_on_runtime_upgrade(true).unwrap();
assert_eq!(Pre::take(), vec!["Foo", "Bar", "Baz"]);
assert_eq!(Post::take(), vec!["Foo", "Bar", "Baz"]);

// calling pre_upgrade and post_upgrade directly on tuple of pallets fails
assert!(<(Foo, (Bar, Baz))>::pre_upgrade().is_err());
assert!(<(Foo, (Bar, Baz))>::post_upgrade(vec![]).is_err());
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
});
}

Expand Down