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

[Feature] Sequential migration execution for try-runtime #12319

Merged
merged 24 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,15 +287,7 @@ where
///
/// This should only be used for testing.
pub fn try_runtime_upgrade() -> Result<frame_support::weights::Weight, &'static str> {
// ensure both `pre_upgrade` and `post_upgrade` won't change the storage root
let state = {
let _guard = frame_support::StorageNoopGuard::default();
<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap()
};
let weight = Self::execute_on_runtime_upgrade();
let _guard = frame_support::StorageNoopGuard::default();
<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(state)
.unwrap();
Ok(weight)
}
}
Expand Down
31 changes: 22 additions & 9 deletions frame/support/src/traits/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use impl_trait_for_tuples::impl_for_tuples;
use sp_runtime::traits::AtLeast32BitUnsigned;
use sp_std::prelude::*;

#[cfg(test)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you want to achieve here? That this is also enabled when we are compiling the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be enabled only when it's both test and try-runtime and is exactly what it currently does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To give some context:

#[cfg(feature = "try-runtime")]
use codec::{Decode, Encode};

Will give an unused import error, because this import is only used within tests, so when we are building with try-runtime - there's a warning emitted and build fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To give a bit more context cargo remote -- test --features=try-runtime this is the only time this import has to be enabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[cfg(all(feature = "try-runtime", test))]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I was looking for that :)

#[cfg(feature = "try-runtime")]
use codec::{Decode, Encode};

Expand Down Expand Up @@ -165,27 +166,39 @@ pub trait OnRuntimeUpgrade {
#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
impl OnRuntimeUpgrade for Tuple {
#[cfg(not(feature = "try-runtime"))]
fn on_runtime_upgrade() -> Weight {
let mut weight = Weight::zero();
for_tuples!( #( weight = weight.saturating_add(Tuple::on_runtime_upgrade()); )* );
weight
}

#[cfg(feature = "try-runtime")]
/// We are executing pre and post checks sequentially in order to be able to test seveal
ruseinov marked this conversation as resolved.
Show resolved Hide resolved
/// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade
/// hooks for tuples are a noop.
fn on_runtime_upgrade() -> Weight {
let mut state: Vec<u8> = Vec::default();
let mut weight = Weight::zero();
for_tuples!( #(
let _guard = frame_support::StorageNoopGuard::default();
state = Tuple::pre_upgrade().unwrap();
ruseinov marked this conversation as resolved.
Show resolved Hide resolved
weight = weight.saturating_add(Tuple::on_runtime_upgrade());
let _guard = frame_support::StorageNoopGuard::default();
Tuple::post_upgrade(state).unwrap();
ruseinov marked this conversation as resolved.
Show resolved Hide resolved
)* );
weight
}

#[cfg(feature = "try-runtime")]
/// noop
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
let mut state: Vec<Vec<u8>> = Vec::default();
for_tuples!( #( state.push(Tuple::pre_upgrade()?); )* );
Ok(state.encode())
Ok(Vec::new())
}

#[cfg(feature = "try-runtime")]
/// noop
fn post_upgrade(state: Vec<u8>) -> Result<(), &'static str> {
ruseinov marked this conversation as resolved.
Show resolved Hide resolved
let state: Vec<Vec<u8>> = Decode::decode(&mut state.as_slice())
.expect("the state parameter should be the same as pre_upgrade generated");
let mut state_iter = state.into_iter();
for_tuples!( #( Tuple::post_upgrade(
state_iter.next().expect("the state parameter should be the same as pre_upgrade generated")
)?; )* );
Ok(())
}
}
Expand Down