From 9940e78525787fecea28e6360a17811ef4bc066e Mon Sep 17 00:00:00 2001 From: linning Date: Sun, 4 Sep 2022 17:20:01 +0800 Subject: [PATCH 01/23] update interfaces of OnRuntimeUpgrade & Hooks Signed-off-by: linning --- frame/alliance/src/migration.rs | 3 + frame/bags-list/src/migrations.rs | 8 +- frame/executive/src/lib.rs | 7 +- frame/nomination-pools/src/migration.rs | 10 ++- frame/staking/src/migrations.rs | 8 +- .../procedural/src/pallet/expand/hooks.rs | 57 +++++++++----- .../procedural/src/pallet/parse/hooks.rs | 21 ++++++ frame/support/src/dispatch.rs | 10 ++- frame/support/src/traits/hooks.rs | 74 ++++++++++++++----- utils/frame/try-runtime/cli/src/lib.rs | 13 ++-- 10 files changed, 159 insertions(+), 52 deletions(-) diff --git a/frame/alliance/src/migration.rs b/frame/alliance/src/migration.rs index 7e3df5219f25b..39ffe30e4fafa 100644 --- a/frame/alliance/src/migration.rs +++ b/frame/alliance/src/migration.rs @@ -41,6 +41,9 @@ pub fn migrate, I: 'static>() -> Weight { pub struct Migration(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> Weight { migrate::() } diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 49b7d136125e2..8a5d14318a474 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -28,6 +28,9 @@ use frame_support::ensure; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> frame_support::weights::Weight { frame_support::weights::Weight::zero() } @@ -90,6 +93,9 @@ mod old { pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + + #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { // The list node data should be corrupt at this point, so this is zero. ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); @@ -122,7 +128,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: ()) -> Result<(), &'static str> { let node_count_before = old::TempStorage::::take(); // Now, the list node data is not corrupt anymore. let iter_node_count_after: u32 = crate::ListNodes::::iter().count() as u32; diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 5d3954ded0998..5ef814550565b 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -287,9 +287,9 @@ where /// /// This should only be used for testing. pub fn try_runtime_upgrade() -> Result { - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap(); + let digest = <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap(); let weight = Self::execute_on_runtime_upgrade(); - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade().unwrap(); + <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(digest).unwrap(); Ok(weight) } @@ -859,6 +859,9 @@ mod tests { struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> Weight { sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes()); sp_io::storage::set(CUSTOM_ON_RUNTIME_KEY, &true.encode()); diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 412c954a2bbf3..05a3b6934173c 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -66,6 +66,9 @@ pub mod v1 { /// Note: The depositor is not optional since he can never change. pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); let onchain = Pallet::::on_chain_storage_version(); @@ -97,7 +100,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: ()) -> Result<(), &'static str> { // new version must be set. assert_eq!(Pallet::::on_chain_storage_version(), 1); Ok(()) @@ -326,6 +329,9 @@ pub mod v2 { } impl OnRuntimeUpgrade for MigrateToV2 { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); let onchain = Pallet::::on_chain_storage_version(); @@ -359,7 +365,7 @@ pub mod v2 { } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: ()) -> Result<(), &'static str> { // new version must be set. assert_eq!(Pallet::::on_chain_storage_version(), 2); diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index ff01e1fe3b09b..735f88c05954e 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -34,6 +34,9 @@ pub mod v10 { /// prevent us from iterating over an arbitrary large number of keys `on_runtime_upgrade`. pub struct MigrateToV10(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV10 { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> frame_support::weights::Weight { if StorageVersion::::get() == Releases::V9_0_0 { let pending_slashes = as Store>::UnappliedSlashes::iter().take(512); @@ -67,6 +70,9 @@ pub mod v9 { /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { let prev_count = T::VoterList::count(); @@ -112,7 +118,7 @@ pub mod v9 { } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: ()) -> Result<(), &'static str> { use frame_support::traits::OnRuntimeUpgradeHelpersExt; let post_count = T::VoterList::count(); let prev_count = Self::get_temp_storage::("prev").unwrap(); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 03878f3f186df..2fc88a2ba027a 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -19,15 +19,20 @@ use crate::pallet::Def; /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { - let (where_clause, span, has_runtime_upgrade) = match def.hooks.as_ref() { + let (where_clause, span, has_runtime_upgrade, pre_state_digest_type) = match def.hooks.as_ref() + { Some(hooks) => { let where_clause = hooks.where_clause.clone(); let span = hooks.attr_span; let has_runtime_upgrade = hooks.has_runtime_upgrade; - (where_clause, span, has_runtime_upgrade) + let pre_state_digest_type = hooks.pre_state_digest_type.clone(); + (where_clause, span, has_runtime_upgrade, pre_state_digest_type) }, - None => (None, def.pallet_struct.attr_span, false), + None => (None, def.pallet_struct.attr_span, false, None), }; + // use `PreStateDigest`'s default type `()` if it is not set explicitly + let pre_state_digest_type = + pre_state_digest_type.map_or(quote::quote! { () }, |t| quote::quote! { #t }); let frame_support = &def.frame_support; let type_impl_gen = &def.type_impl_generics(span); @@ -75,7 +80,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let frame_system = &def.frame_system; quote::quote! { impl<#type_impl_gen> - #frame_support::traits::Hooks<::BlockNumber> + #frame_support::traits::Hooks<::BlockNumber, #pre_state_digest_type> for Pallet<#type_use_gen> {} } } else { @@ -95,7 +100,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::on_finalize(n) } @@ -111,7 +117,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ) -> #frame_support::weights::Weight { < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::on_idle(n, remaining_weight) } @@ -129,7 +136,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::on_initialize(n) } @@ -139,6 +147,9 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause { + #[cfg(feature = "try-runtime")] + type PreStateDigest = #pre_state_digest_type; + fn on_runtime_upgrade() -> #frame_support::weights::Weight { #frame_support::sp_tracing::enter_span!( #frame_support::sp_tracing::trace_span!("on_runtime_update") @@ -154,27 +165,30 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::on_runtime_upgrade() } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { + fn pre_upgrade() -> Result { < - Self - as - #frame_support::traits::Hooks<::BlockNumber> + Self as #frame_support::traits::Hooks< + ::BlockNumber, + #pre_state_digest_type + > >::pre_upgrade() } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(digest: Self::PreStateDigest) -> Result<(), &'static str> { < - Self - as - #frame_support::traits::Hooks<::BlockNumber> - >::post_upgrade() + Self as #frame_support::traits::Hooks< + ::BlockNumber, + #pre_state_digest_type + > + >::post_upgrade(digest) } } @@ -185,7 +199,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { fn offchain_worker(n: ::BlockNumber) { < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::offchain_worker(n) } @@ -198,7 +213,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { fn integrity_test() { < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::integrity_test() } @@ -216,7 +232,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #log_try_state < Self as #frame_support::traits::Hooks< - ::BlockNumber + ::BlockNumber, + #pre_state_digest_type > >::try_state(n) } diff --git a/frame/support/procedural/src/pallet/parse/hooks.rs b/frame/support/procedural/src/pallet/parse/hooks.rs index 2dc8f4da47c5f..59106cdaccc10 100644 --- a/frame/support/procedural/src/pallet/parse/hooks.rs +++ b/frame/support/procedural/src/pallet/parse/hooks.rs @@ -30,6 +30,8 @@ pub struct HooksDef { pub attr_span: proc_macro2::Span, /// Boolean flag, set to true if the `on_runtime_upgrade` method of hooks was implemented. pub has_runtime_upgrade: bool, + /// The `PreStateDigest` type parameter of the trait. + pub pre_state_digest_type: Option, } impl HooksDef { @@ -70,6 +72,24 @@ impl HooksDef { return Err(syn::Error::new(item_trait.span(), msg)) } + let err = || { + let msg = format!( + "Invalid generic type parameters for Hooks, expected Hooks or Hooks" + ); + syn::Error::new(item_trait.span(), msg) + }; + let pre_state_digest_type = match &item_trait.segments[0].arguments { + syn::PathArguments::AngleBracketed(ab) => match ab.args.len() { + 1 => None, + 2 => match &ab.args[1] { + syn::GenericArgument::Type(t) => Some(t.clone()), + _ => return Err(err()), + }, + _ => return Err(err()), + }, + _ => return Err(err()), + }; + let has_runtime_upgrade = item.items.iter().any(|i| match i { syn::ImplItem::Method(method) => method.sig.ident == "on_runtime_upgrade", _ => false, @@ -81,6 +101,7 @@ impl HooksDef { instances, has_runtime_upgrade, where_clause: item.generics.where_clause.clone(), + pre_state_digest_type, }) } } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index fc976b03556a4..4b105bbd00d5c 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1588,6 +1588,9 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> $return { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); let pallet_name = << @@ -1614,7 +1617,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: ()) -> Result<(), &'static str> { Ok(()) } } @@ -1629,6 +1632,9 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); + fn on_runtime_upgrade() -> $crate::dispatch::Weight { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); let pallet_name = << @@ -1652,7 +1658,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: ()) -> Result<(), &'static str> { Ok(()) } } diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 25ec333a7dbe0..43873768daecc 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -120,6 +120,14 @@ pub trait OnGenesis { /// Implementing this lets you express what should happen when the runtime upgrades, /// and changes may need to occur to your module. pub trait OnRuntimeUpgrade { + /// The state digest type, which is generated by `pre_upgrade` based on the pre-updrade + /// state and will be passed to `post_upgrade` after upgrading for post-check. If there + /// is no such need, `()` should be used. + /// + /// TODO: use the `associated_type_defaults` feature once it is stable. + #[cfg(feature = "try-runtime")] + type PreStateDigest: Default; + /// Perform a module upgrade. /// /// # Warning @@ -135,25 +143,41 @@ pub trait OnRuntimeUpgrade { /// Execute some pre-checks prior to a runtime upgrade. /// + /// Return the state digest which will be passed to `post_upgrade` after upgrading for + /// post-check. NOTE: if this function is not implemented, `Self::PreStateDigest` should be set + /// to `()`. + /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) + fn pre_upgrade() -> Result { + Ok(Self::PreStateDigest::default()) } /// Execute some post-checks after a runtime upgrade. /// + /// The state digest generated by `pre_upgrade` before upgrading will be passed in for + /// post-check. NOTE: if `pre_upgrade` is not implemented, the default value of + /// `Self::PreStateDigest` will be used, in such case `Self::PreStateDigest` should be set to + /// `()`. + /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_digest: Self::PreStateDigest) -> Result<(), &'static str> { Ok(()) } } -#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] -#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] -#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] +// Implement `OnRuntimeUpgrade` up to the `(T1, T2)` tuple, it is sufficient for current usage +// because pallets are aggregated in form of `(p1, (p2, (p3, ..)))` where the maximum number of +// pallets supported is limited by the `recursion_limit` attribute (default 128). +// +// Note: `impl_for_tuples` is supported up to 12 here because `PreStateDigest` is bonuded by +// `Default` and rust only implemented `Default` on tuples of arity up to 12. +#[impl_for_tuples(2)] impl OnRuntimeUpgrade for Tuple { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (for_tuples!( #( Tuple::PreStateDigest ),* )); + fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); for_tuples!( #( weight = weight.saturating_add(Tuple::on_runtime_upgrade()); )* ); @@ -161,17 +185,14 @@ impl OnRuntimeUpgrade for Tuple { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { - let mut result = Ok(()); - for_tuples!( #( result = result.and(Tuple::pre_upgrade()); )* ); - result + fn pre_upgrade() -> Result { + Ok((for_tuples!( #( Tuple::pre_upgrade()? ),* ))) } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { - let mut result = Ok(()); - for_tuples!( #( result = result.and(Tuple::post_upgrade()); )* ); - result + fn post_upgrade(digest: Self::PreStateDigest) -> Result<(), &'static str> { + for_tuples!( #( Tuple::post_upgrade(digest.Tuple)?; )* ); + Ok(()) } } @@ -189,7 +210,13 @@ pub trait IntegrityTest { } /// The pallet hooks trait. Implementing this lets you express some logic to execute. -pub trait Hooks { +/// +/// The `PreStateDigest` generic type parameter is used by `pre_upgrade` to generated +/// a state digest based on the pre-updrade state and will be passed to `post_upgrade` +/// after upgrading for post-check. If there is no such need, `()` should be used. +/// +/// TODO: use the `associated_type_defaults` feature once it is stable. +pub trait Hooks { /// The block is being finalized. Implement to have something happen. fn on_finalize(_n: BlockNumber) {} @@ -243,17 +270,26 @@ pub trait Hooks { /// Execute some pre-checks prior to a runtime upgrade. /// + /// Return the state digest which will be passed to `post_upgrade` after upgrading for + /// post-check. NOTE: if this function is not implemented, `PreStateDigest` should be set + /// to `()`. + /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) + fn pre_upgrade() -> Result { + Ok(PreStateDigest::default()) } /// Execute some post-checks after a runtime upgrade. /// + /// The state digest generated by `pre_upgrade` before upgrading will be passed in for + /// post-check. NOTE: if `pre_upgrade` is not implemented, the default value of + /// `PreStateDigest` will be used, in such case `PreStateDigest` should be set to + /// `()`. + /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_digest: PreStateDigest) -> Result<(), &'static str> { Ok(()) } @@ -326,6 +362,8 @@ mod tests { } } impl OnRuntimeUpgrade for Test { + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 76679c43f7f14..fb4f187763d30 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -132,20 +132,21 @@ //! added, given the right flag: //! //! ```ignore +//! #[cfg(feature = "try-runtime")] +//! type PreStateDigest; +//! //! #[cfg(feature = try-runtime)] -//! fn pre_upgrade() -> Result<(), &'static str> {} +//! fn pre_upgrade() -> Result {} //! //! #[cfg(feature = try-runtime)] -//! fn post_upgrade() -> Result<(), &'static str> {} +//! fn post_upgrade(digest: Self::PreStateDigest) -> Result<(), &'static str> {} //! ``` //! //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). //! //! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before -//! and after the migration. If any data needs to be temporarily stored between the pre/post -//! migration hooks, `OnRuntimeUpgradeHelpersExt` can help with that. Note that you should be -//! mindful with any mutable storage ops in the pre/post migration checks, as you almost certainly -//! will not want to mutate any of the storage that is to be migrated. +//! and after the migration. The `Self::PreStateDigest` (default `()`) generated by `pre_upgrade` will +//! be passed to `post_upgrade` after upgrading for post checking. //! //! #### Logging //! From df6c7d3da1f0552e92548326aa996d7ffac73b09 Mon Sep 17 00:00:00 2001 From: linning Date: Sun, 4 Sep 2022 18:11:04 +0800 Subject: [PATCH 02/23] remove try-runtime for PreStateDigest Signed-off-by: linning --- frame/alliance/src/migration.rs | 1 - frame/bags-list/src/migrations.rs | 2 -- frame/executive/src/lib.rs | 1 - frame/nomination-pools/src/migration.rs | 2 -- frame/staking/src/migrations.rs | 2 -- frame/support/procedural/src/pallet/expand/hooks.rs | 1 - frame/support/src/dispatch.rs | 2 -- frame/support/src/traits/hooks.rs | 5 ++--- 8 files changed, 2 insertions(+), 14 deletions(-) diff --git a/frame/alliance/src/migration.rs b/frame/alliance/src/migration.rs index 39ffe30e4fafa..03de40c655a6c 100644 --- a/frame/alliance/src/migration.rs +++ b/frame/alliance/src/migration.rs @@ -41,7 +41,6 @@ pub fn migrate, I: 'static>() -> Weight { pub struct Migration(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 8a5d14318a474..868bce54574ad 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -28,7 +28,6 @@ use frame_support::ensure; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { @@ -92,7 +91,6 @@ mod old { /// A struct that migrates all bags lists to contain a score value. pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); #[cfg(feature = "try-runtime")] diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 5ef814550565b..cab4135953052 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -859,7 +859,6 @@ mod tests { struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 05a3b6934173c..ba697c248da13 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -66,7 +66,6 @@ pub mod v1 { /// Note: The depositor is not optional since he can never change. pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { @@ -329,7 +328,6 @@ pub mod v2 { } impl OnRuntimeUpgrade for MigrateToV2 { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 735f88c05954e..b2fa2fe0337f6 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -34,7 +34,6 @@ pub mod v10 { /// prevent us from iterating over an arbitrary large number of keys `on_runtime_upgrade`. pub struct MigrateToV10(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV10 { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { @@ -70,7 +69,6 @@ pub mod v9 { /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 2fc88a2ba027a..3b249a336c2a4 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -147,7 +147,6 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause { - #[cfg(feature = "try-runtime")] type PreStateDigest = #pre_state_digest_type; fn on_runtime_upgrade() -> #frame_support::weights::Weight { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 4b105bbd00d5c..981d9a76ab185 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1588,7 +1588,6 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> $return { @@ -1632,7 +1631,6 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> $crate::dispatch::Weight { diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 43873768daecc..7502dd1a9f2d5 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -125,7 +125,8 @@ pub trait OnRuntimeUpgrade { /// is no such need, `()` should be used. /// /// TODO: use the `associated_type_defaults` feature once it is stable. - #[cfg(feature = "try-runtime")] + /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` + /// file to add correct features dependencies type PreStateDigest: Default; /// Perform a module upgrade. @@ -175,7 +176,6 @@ pub trait OnRuntimeUpgrade { // `Default` and rust only implemented `Default` on tuples of arity up to 12. #[impl_for_tuples(2)] impl OnRuntimeUpgrade for Tuple { - #[cfg(feature = "try-runtime")] type PreStateDigest = (for_tuples!( #( Tuple::PreStateDigest ),* )); fn on_runtime_upgrade() -> Weight { @@ -362,7 +362,6 @@ mod tests { } } impl OnRuntimeUpgrade for Test { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) From 958f64553346e4a2f6298ada551505d13db92df0 Mon Sep 17 00:00:00 2001 From: linning Date: Sun, 4 Sep 2022 18:52:31 +0800 Subject: [PATCH 03/23] remove the Default bound of PreStateDigest Signed-off-by: linning --- frame/alliance/src/migration.rs | 8 +++++- frame/bags-list/src/migrations.rs | 6 +++-- frame/executive/src/lib.rs | 8 +++++- frame/nomination-pools/src/migration.rs | 11 ++++++-- frame/staking/src/migrations.rs | 11 ++++++-- .../procedural/src/pallet/expand/hooks.rs | 1 + frame/support/src/dispatch.rs | 6 +++-- frame/support/src/traits/hooks.rs | 27 ++++++++++--------- 8 files changed, 55 insertions(+), 23 deletions(-) diff --git a/frame/alliance/src/migration.rs b/frame/alliance/src/migration.rs index 03de40c655a6c..d322f2c9ddbe7 100644 --- a/frame/alliance/src/migration.rs +++ b/frame/alliance/src/migration.rs @@ -41,11 +41,17 @@ pub fn migrate, I: 'static>() -> Weight { pub struct Migration(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { migrate::() } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } } /// v0_to_v1: `UpForKicking` is replaced by a retirement period. diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 868bce54574ad..dc519c1dd4903 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -28,7 +28,8 @@ use frame_support::ensure; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { frame_support::weights::Weight::zero() @@ -91,7 +92,8 @@ mod old { /// A struct that migrates all bags lists to contain a score value. pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index cab4135953052..6290180732767 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -859,7 +859,8 @@ mod tests { struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes()); @@ -867,6 +868,11 @@ mod tests { System::deposit_event(frame_system::Event::CodeUpdated); Weight::from_ref_time(100) } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } } type Executive = super::Executive< diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index ba697c248da13..932a8fcfab3f0 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -66,7 +66,8 @@ pub mod v1 { /// Note: The depositor is not optional since he can never change. pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); @@ -98,6 +99,11 @@ pub mod v1 { } } + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } + #[cfg(feature = "try-runtime")] fn post_upgrade(_: ()) -> Result<(), &'static str> { // new version must be set. @@ -328,7 +334,8 @@ pub mod v2 { } impl OnRuntimeUpgrade for MigrateToV2 { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index b2fa2fe0337f6..f4419c94b0d2c 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -34,7 +34,8 @@ pub mod v10 { /// prevent us from iterating over an arbitrary large number of keys `on_runtime_upgrade`. pub struct MigrateToV10(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV10 { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { if StorageVersion::::get() == Releases::V9_0_0 { @@ -58,6 +59,11 @@ pub mod v10 { T::DbWeight::get().reads(1) } } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } } } @@ -69,7 +75,8 @@ pub mod v9 { /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 3b249a336c2a4..2fc88a2ba027a 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -147,6 +147,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause { + #[cfg(feature = "try-runtime")] type PreStateDigest = #pre_state_digest_type; fn on_runtime_upgrade() -> #frame_support::weights::Weight { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 981d9a76ab185..53b0e2f89075f 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1588,7 +1588,8 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> $return { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); @@ -1631,7 +1632,8 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - type PreStateDigest = (); + #[cfg(feature = "try-runtime")] + type PreStateDigest = (); fn on_runtime_upgrade() -> $crate::dispatch::Weight { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 7502dd1a9f2d5..4625d14adccb6 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -125,9 +125,8 @@ pub trait OnRuntimeUpgrade { /// is no such need, `()` should be used. /// /// TODO: use the `associated_type_defaults` feature once it is stable. - /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` - /// file to add correct features dependencies - type PreStateDigest: Default; + #[cfg(feature = "try-runtime")] + type PreStateDigest; /// Perform a module upgrade. /// @@ -150,9 +149,7 @@ pub trait OnRuntimeUpgrade { /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result { - Ok(Self::PreStateDigest::default()) - } + fn pre_upgrade() -> Result; /// Execute some post-checks after a runtime upgrade. /// @@ -168,14 +165,11 @@ pub trait OnRuntimeUpgrade { } } -// Implement `OnRuntimeUpgrade` up to the `(T1, T2)` tuple, it is sufficient for current usage -// because pallets are aggregated in form of `(p1, (p2, (p3, ..)))` where the maximum number of -// pallets supported is limited by the `recursion_limit` attribute (default 128). -// -// Note: `impl_for_tuples` is supported up to 12 here because `PreStateDigest` is bonuded by -// `Default` and rust only implemented `Default` on tuples of arity up to 12. -#[impl_for_tuples(2)] +#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +#[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(feature = "try-runtime")] type PreStateDigest = (for_tuples!( #( Tuple::PreStateDigest ),* )); fn on_runtime_upgrade() -> Weight { @@ -362,10 +356,17 @@ mod tests { } } impl OnRuntimeUpgrade for Test { + #[cfg(feature = "try-runtime")] type PreStateDigest = (); + fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } } assert_eq!(<(Test, Test)>::on_initialize(0), Weight::from_ref_time(20)); From 4cbfda051f9a63696cdb7deae377d25b4c88d17f Mon Sep 17 00:00:00 2001 From: linning Date: Mon, 5 Sep 2022 01:45:55 +0800 Subject: [PATCH 04/23] remove try-runtime for PreStateDigest & pre_upgrade Signed-off-by: linning --- frame/alliance/src/migration.rs | 2 -- frame/bags-list/src/migrations.rs | 6 ------ frame/executive/src/lib.rs | 2 -- frame/nomination-pools/src/migration.rs | 4 ---- frame/staking/src/migrations.rs | 4 ---- frame/support/procedural/src/pallet/expand/hooks.rs | 6 +++++- frame/support/src/dispatch.rs | 4 ---- frame/support/src/traits/hooks.rs | 10 ++++------ 8 files changed, 9 insertions(+), 29 deletions(-) diff --git a/frame/alliance/src/migration.rs b/frame/alliance/src/migration.rs index d322f2c9ddbe7..bec859c8fcfaf 100644 --- a/frame/alliance/src/migration.rs +++ b/frame/alliance/src/migration.rs @@ -41,14 +41,12 @@ pub fn migrate, I: 'static>() -> Weight { pub struct Migration(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { migrate::() } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index dc519c1dd4903..0e55723f37adb 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -21,21 +21,17 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_election_provider_support::ScoreProvider; use frame_support::traits::OnRuntimeUpgrade; - -#[cfg(feature = "try-runtime")] use frame_support::ensure; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { frame_support::weights::Weight::zero() } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { // The old explicit storage item. #[frame_support::storage_alias] @@ -92,10 +88,8 @@ mod old { /// A struct that migrates all bags lists to contain a score value. pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { // The list node data should be corrupt at this point, so this is zero. ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 6290180732767..a76f75f32e628 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -859,7 +859,6 @@ mod tests { struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { @@ -869,7 +868,6 @@ mod tests { Weight::from_ref_time(100) } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 932a8fcfab3f0..8cf997ae2a085 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -66,7 +66,6 @@ pub mod v1 { /// Note: The depositor is not optional since he can never change. pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { @@ -99,7 +98,6 @@ pub mod v1 { } } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } @@ -334,7 +332,6 @@ pub mod v2 { } impl OnRuntimeUpgrade for MigrateToV2 { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { @@ -356,7 +353,6 @@ pub mod v2 { } } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { // all reward accounts must have more than ED. RewardPools::::iter().for_each(|(id, _)| { diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index f4419c94b0d2c..2254cf5319d1b 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -34,7 +34,6 @@ pub mod v10 { /// prevent us from iterating over an arbitrary large number of keys `on_runtime_upgrade`. pub struct MigrateToV10(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV10 { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { @@ -60,7 +59,6 @@ pub mod v10 { } } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } @@ -75,7 +73,6 @@ pub mod v9 { /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { @@ -109,7 +106,6 @@ pub mod v9 { } } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { use frame_support::traits::OnRuntimeUpgradeHelpersExt; frame_support::ensure!( diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 2fc88a2ba027a..0ba3d93baca85 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -147,7 +147,6 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause { - #[cfg(feature = "try-runtime")] type PreStateDigest = #pre_state_digest_type; fn on_runtime_upgrade() -> #frame_support::weights::Weight { @@ -171,6 +170,11 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::on_runtime_upgrade() } + #[cfg(not(feature = "try-runtime"))] + fn pre_upgrade() -> Result { + Ok(Self::PreStateDigest::default()) + } + #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result { < diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 53b0e2f89075f..10489ea9d90d5 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1588,7 +1588,6 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> $return { @@ -1611,7 +1610,6 @@ macro_rules! decl_module { { $( $impl )* } } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } @@ -1632,7 +1630,6 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> $crate::dispatch::Weight { @@ -1652,7 +1649,6 @@ macro_rules! decl_module { $crate::dispatch::Weight::zero() } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 4625d14adccb6..d62796c4d3c09 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -125,7 +125,8 @@ pub trait OnRuntimeUpgrade { /// is no such need, `()` should be used. /// /// TODO: use the `associated_type_defaults` feature once it is stable. - #[cfg(feature = "try-runtime")] + /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` + /// files to add recorrect features dependencies type PreStateDigest; /// Perform a module upgrade. @@ -148,7 +149,8 @@ pub trait OnRuntimeUpgrade { /// to `()`. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. - #[cfg(feature = "try-runtime")] + /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` + /// files to add recorrect features dependencies fn pre_upgrade() -> Result; /// Execute some post-checks after a runtime upgrade. @@ -169,7 +171,6 @@ 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(feature = "try-runtime")] type PreStateDigest = (for_tuples!( #( Tuple::PreStateDigest ),* )); fn on_runtime_upgrade() -> Weight { @@ -178,7 +179,6 @@ impl OnRuntimeUpgrade for Tuple { weight } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result { Ok((for_tuples!( #( Tuple::pre_upgrade()? ),* ))) } @@ -356,14 +356,12 @@ mod tests { } } impl OnRuntimeUpgrade for Test { - #[cfg(feature = "try-runtime")] type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) } - #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<(), &'static str> { Ok(()) } From 63677918330391dba549d3b4e9b5630e6fa14383 Mon Sep 17 00:00:00 2001 From: linning Date: Mon, 5 Sep 2022 04:35:40 +0800 Subject: [PATCH 05/23] remove tmp storage between upgrade hooks Signed-off-by: linning --- frame/bags-list/src/migrations.rs | 14 ++++---------- frame/staking/src/migrations.rs | 10 ++++------ frame/support/src/traits/hooks.rs | 4 ++-- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 0e55723f37adb..0b4e4d67f439f 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -79,18 +79,14 @@ mod old { #[frame_support::storage_alias] pub type CounterForListNodes, I: 'static> = StorageValue, u32, ValueQuery>; - - #[frame_support::storage_alias] - pub type TempStorage, I: 'static> = - StorageValue, u32, ValueQuery>; } /// A struct that migrates all bags lists to contain a score value. pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { - type PreStateDigest = (); + type PreStateDigest = u32; - fn pre_upgrade() -> Result<(), &'static str> { + fn pre_upgrade() -> Result { // The list node data should be corrupt at this point, so this is zero. ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); // We can use the helper `old::ListNode` to get the existing data. @@ -98,8 +94,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { let tracked_node_count: u32 = old::CounterForListNodes::::get(); crate::log!(info, "number of nodes before: {:?} {:?}", iter_node_count, tracked_node_count); ensure!(iter_node_count == tracked_node_count, "Node count is wrong."); - old::TempStorage::::put(iter_node_count); - Ok(()) + Ok(iter_node_count) } fn on_runtime_upgrade() -> frame_support::weights::Weight { @@ -122,8 +117,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: ()) -> Result<(), &'static str> { - let node_count_before = old::TempStorage::::take(); + fn post_upgrade(node_count_before: u32) -> Result<(), &'static str> { // Now, the list node data is not corrupt anymore. let iter_node_count_after: u32 = crate::ListNodes::::iter().count() as u32; let tracked_node_count_after: u32 = crate::ListNodes::::count(); diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 2254cf5319d1b..300bbb45e5d21 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -73,7 +73,7 @@ pub mod v9 { /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - type PreStateDigest = (); + type PreStateDigest = u32; fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { @@ -106,7 +106,7 @@ pub mod v9 { } } - fn pre_upgrade() -> Result<(), &'static str> { + fn pre_upgrade() -> Result { use frame_support::traits::OnRuntimeUpgradeHelpersExt; frame_support::ensure!( StorageVersion::::get() == crate::Releases::V8_0_0, @@ -114,15 +114,13 @@ pub mod v9 { ); let prev_count = T::VoterList::count(); - Self::set_temp_storage(prev_count, "prev"); - Ok(()) + Ok(prev_count) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: ()) -> Result<(), &'static str> { + fn post_upgrade(prev_count: u32) -> Result<(), &'static str> { use frame_support::traits::OnRuntimeUpgradeHelpersExt; let post_count = T::VoterList::count(); - let prev_count = Self::get_temp_storage::("prev").unwrap(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index d62796c4d3c09..41e1fc9cac5a4 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -126,7 +126,7 @@ pub trait OnRuntimeUpgrade { /// /// TODO: use the `associated_type_defaults` feature once it is stable. /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` - /// files to add recorrect features dependencies + /// files to recorrect features dependencies type PreStateDigest; /// Perform a module upgrade. @@ -150,7 +150,7 @@ pub trait OnRuntimeUpgrade { /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` - /// files to add recorrect features dependencies + /// files to recorrect features dependencies fn pre_upgrade() -> Result; /// Execute some post-checks after a runtime upgrade. From ca1c9e34a537fae4eacbb56b393ade4b5f54c4b6 Mon Sep 17 00:00:00 2001 From: linning Date: Mon, 5 Sep 2022 05:07:59 +0800 Subject: [PATCH 06/23] ensure hooks are storage noop Signed-off-by: linning --- frame/executive/src/lib.rs | 12 +++++++++--- frame/staking/src/migrations.rs | 2 -- frame/support/src/lib.rs | 2 +- frame/support/src/storage/storage_noop_guard.rs | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index a76f75f32e628..79572d07f0634 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -287,10 +287,16 @@ where /// /// This should only be used for testing. pub fn try_runtime_upgrade() -> Result { - let digest = <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap(); + // ensure both `pre_upgrade` and `post_upgrade` won't changing the storage root + let digest = { + let _guard = frame_support::StorageNoopGuard::default(); + <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap() + }; let weight = Self::execute_on_runtime_upgrade(); - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(digest).unwrap(); - + frame_support::assert_storage_noop!( + <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(digest) + .unwrap() + ); Ok(weight) } } diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 300bbb45e5d21..8653916b104a1 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -107,7 +107,6 @@ pub mod v9 { } fn pre_upgrade() -> Result { - use frame_support::traits::OnRuntimeUpgradeHelpersExt; frame_support::ensure!( StorageVersion::::get() == crate::Releases::V8_0_0, "must upgrade linearly" @@ -119,7 +118,6 @@ pub mod v9 { #[cfg(feature = "try-runtime")] fn post_upgrade(prev_count: u32) -> Result<(), &'static str> { - use frame_support::traits::OnRuntimeUpgradeHelpersExt; let post_count = T::VoterList::count(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 3b2a8b3b62fc2..16f812c52de5d 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -93,7 +93,7 @@ pub mod unsigned { }; } -#[cfg(any(feature = "std", feature = "runtime-benchmarks", test))] +#[cfg(any(feature = "std", feature = "runtime-benchmarks", feature = "try-runtime", test))] pub use self::storage::storage_noop_guard::StorageNoopGuard; pub use self::{ dispatch::{Callable, Parameter}, diff --git a/frame/support/src/storage/storage_noop_guard.rs b/frame/support/src/storage/storage_noop_guard.rs index afcc699d91313..7186c3eaf467a 100644 --- a/frame/support/src/storage/storage_noop_guard.rs +++ b/frame/support/src/storage/storage_noop_guard.rs @@ -16,7 +16,7 @@ // limitations under the License. // Feature gated since it can panic. -#![cfg(any(feature = "std", feature = "runtime-benchmarks", test))] +#![cfg(any(feature = "std", feature = "runtime-benchmarks", feature = "try-runtime", test))] //! Contains the [`crate::StorageNoopGuard`] for conveniently asserting //! that no storage mutation has been made by a whole code block. From 46e47c0ba874f540e12c1e64634eb8f9584f42a9 Mon Sep 17 00:00:00 2001 From: linning Date: Mon, 5 Sep 2022 05:10:06 +0800 Subject: [PATCH 07/23] remove OnRuntimeUpgradeHelpersExt Signed-off-by: linning --- frame/support/src/traits.rs | 2 +- frame/support/src/traits/try_runtime.rs | 35 ------------------------- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d4f0e73557c77..208ddc6f95b6f 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -108,4 +108,4 @@ pub use voting::{ #[cfg(feature = "try-runtime")] mod try_runtime; #[cfg(feature = "try-runtime")] -pub use try_runtime::{OnRuntimeUpgradeHelpersExt, Select as TryStateSelect, TryState}; +pub use try_runtime::{Select as TryStateSelect, TryState}; diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index c5ddd1cae42be..f76fdaa0250a5 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -22,41 +22,6 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; -/// Prefix to be used (optionally) for implementing [`OnRuntimeUpgradeHelpersExt::storage_key`]. -const ON_RUNTIME_UPGRADE_PREFIX: &[u8] = b"__ON_RUNTIME_UPGRADE__"; - -/// Some helper functions for [`OnRuntimeUpgrade`] during `try-runtime` testing. -pub trait OnRuntimeUpgradeHelpersExt { - /// Generate a storage key unique to this runtime upgrade. - /// - /// This can be used to communicate data from pre-upgrade to post-upgrade state and check - /// them. See [`Self::set_temp_storage`] and [`Self::get_temp_storage`]. - fn storage_key(ident: &str) -> [u8; 32] { - crate::storage::storage_prefix(ON_RUNTIME_UPGRADE_PREFIX, ident.as_bytes()) - } - - /// Get temporary storage data written by [`Self::set_temp_storage`]. - /// - /// Returns `None` if either the data is unavailable or un-decodable. - /// - /// A `at` storage identifier must be provided to indicate where the storage is being read from. - fn get_temp_storage(at: &str) -> Option { - sp_io::storage::get(&Self::storage_key(at)) - .and_then(|bytes| codec::Decode::decode(&mut &*bytes).ok()) - } - - /// Write some temporary data to a specific storage that can be read (potentially in - /// post-upgrade hook) via [`Self::get_temp_storage`]. - /// - /// A `at` storage identifier must be provided to indicate where the storage is being written - /// to. - fn set_temp_storage(data: T, at: &str) { - sp_io::storage::set(&Self::storage_key(at), &data.encode()); - } -} - -impl OnRuntimeUpgradeHelpersExt for U {} - // Which state tests to execute. #[derive(codec::Encode, codec::Decode, Clone)] pub enum Select { From e80c7fb6ea73cd924b95cc0c655a59bcfd1c0e84 Mon Sep 17 00:00:00 2001 From: linning Date: Mon, 5 Sep 2022 17:51:33 +0800 Subject: [PATCH 08/23] cargo check & fmt Signed-off-by: linning --- frame/bags-list/src/migrations.rs | 3 +-- frame/support/src/traits/hooks.rs | 2 +- frame/support/src/traits/try_runtime.rs | 1 - utils/frame/try-runtime/cli/src/lib.rs | 6 +++--- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 0b4e4d67f439f..8f85e42c58a1f 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -20,8 +20,7 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_election_provider_support::ScoreProvider; -use frame_support::traits::OnRuntimeUpgrade; -use frame_support::ensure; +use frame_support::{ensure, traits::OnRuntimeUpgrade}; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 41e1fc9cac5a4..cf7f48d36c003 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -356,7 +356,7 @@ mod tests { } } impl OnRuntimeUpgrade for Test { - type PreStateDigest = (); + type PreStateDigest = (); fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index f76fdaa0250a5..0683352c72388 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -17,7 +17,6 @@ //! Try-runtime specific traits and types. -use super::*; use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index fb4f187763d30..5c3e51011c853 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -134,7 +134,7 @@ //! ```ignore //! #[cfg(feature = "try-runtime")] //! type PreStateDigest; -//! +//! //! #[cfg(feature = try-runtime)] //! fn pre_upgrade() -> Result {} //! @@ -145,8 +145,8 @@ //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). //! //! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before -//! and after the migration. The `Self::PreStateDigest` (default `()`) generated by `pre_upgrade` will -//! be passed to `post_upgrade` after upgrading for post checking. +//! and after the migration. The `Self::PreStateDigest` (default `()`) generated by `pre_upgrade` +//! will be passed to `post_upgrade` after upgrading for post checking. //! //! #### Logging //! From 49f487c162d6fb27732b74e36356cc26f5f324b1 Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 6 Sep 2022 19:42:22 +0800 Subject: [PATCH 09/23] rename PreStateDigest to PreUpgradeState Signed-off-by: linning --- frame/alliance/src/migration.rs | 2 +- frame/bags-list/src/migrations.rs | 4 +- frame/executive/src/lib.rs | 2 +- frame/nomination-pools/src/migration.rs | 4 +- frame/staking/src/migrations.rs | 4 +- .../procedural/src/pallet/expand/hooks.rs | 42 +++++++++---------- .../procedural/src/pallet/parse/hooks.rs | 8 ++-- frame/support/src/dispatch.rs | 4 +- frame/support/src/traits/hooks.rs | 32 +++++++------- utils/frame/try-runtime/cli/src/lib.rs | 8 ++-- 10 files changed, 55 insertions(+), 55 deletions(-) diff --git a/frame/alliance/src/migration.rs b/frame/alliance/src/migration.rs index afeca25aa168c..26966ede9cf50 100644 --- a/frame/alliance/src/migration.rs +++ b/frame/alliance/src/migration.rs @@ -41,7 +41,7 @@ pub fn migrate, I: 'static>() -> Weight { pub struct Migration(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> Weight { migrate::() diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 8f85e42c58a1f..a718c39d2ccb6 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -25,7 +25,7 @@ use frame_support::{ensure, traits::OnRuntimeUpgrade}; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { frame_support::weights::Weight::zero() @@ -83,7 +83,7 @@ mod old { /// A struct that migrates all bags lists to contain a score value. pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { - type PreStateDigest = u32; + type PreUpgradeState = u32; fn pre_upgrade() -> Result { // The list node data should be corrupt at this point, so this is zero. diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 79572d07f0634..4c6df89d7c0be 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -865,7 +865,7 @@ mod tests { struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> Weight { sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes()); diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 8cf997ae2a085..7e7b7fc81bf15 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -66,7 +66,7 @@ pub mod v1 { /// Note: The depositor is not optional since he can never change. pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); @@ -332,7 +332,7 @@ pub mod v2 { } impl OnRuntimeUpgrade for MigrateToV2 { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 8653916b104a1..a6cb424bf3f12 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -34,7 +34,7 @@ pub mod v10 { /// prevent us from iterating over an arbitrary large number of keys `on_runtime_upgrade`. pub struct MigrateToV10(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV10 { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> frame_support::weights::Weight { if StorageVersion::::get() == Releases::V9_0_0 { @@ -73,7 +73,7 @@ pub mod v9 { /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - type PreStateDigest = u32; + type PreUpgradeState = u32; fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 0ba3d93baca85..c6cc1b611d2ee 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -19,20 +19,20 @@ use crate::pallet::Def; /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { - let (where_clause, span, has_runtime_upgrade, pre_state_digest_type) = match def.hooks.as_ref() + let (where_clause, span, has_runtime_upgrade, pre_upgrade_state) = match def.hooks.as_ref() { Some(hooks) => { let where_clause = hooks.where_clause.clone(); let span = hooks.attr_span; let has_runtime_upgrade = hooks.has_runtime_upgrade; - let pre_state_digest_type = hooks.pre_state_digest_type.clone(); - (where_clause, span, has_runtime_upgrade, pre_state_digest_type) + let pre_upgrade_state = hooks.pre_upgrade_state.clone(); + (where_clause, span, has_runtime_upgrade, pre_upgrade_state) }, None => (None, def.pallet_struct.attr_span, false, None), }; - // use `PreStateDigest`'s default type `()` if it is not set explicitly - let pre_state_digest_type = - pre_state_digest_type.map_or(quote::quote! { () }, |t| quote::quote! { #t }); + // use `PreUpgradeState`'s default type `()` if it is not set explicitly + let pre_upgrade_state = + pre_upgrade_state.map_or(quote::quote! { () }, |t| quote::quote! { #t }); let frame_support = &def.frame_support; let type_impl_gen = &def.type_impl_generics(span); @@ -80,7 +80,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let frame_system = &def.frame_system; quote::quote! { impl<#type_impl_gen> - #frame_support::traits::Hooks<::BlockNumber, #pre_state_digest_type> + #frame_support::traits::Hooks<::BlockNumber, #pre_upgrade_state> for Pallet<#type_use_gen> {} } } else { @@ -101,7 +101,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::on_finalize(n) } @@ -118,7 +118,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::on_idle(n, remaining_weight) } @@ -137,7 +137,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::on_initialize(n) } @@ -147,7 +147,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause { - type PreStateDigest = #pre_state_digest_type; + type PreUpgradeState = #pre_upgrade_state; fn on_runtime_upgrade() -> #frame_support::weights::Weight { #frame_support::sp_tracing::enter_span!( @@ -165,32 +165,32 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::on_runtime_upgrade() } #[cfg(not(feature = "try-runtime"))] - fn pre_upgrade() -> Result { - Ok(Self::PreStateDigest::default()) + fn pre_upgrade() -> Result { + Ok(Self::PreUpgradeState::default()) } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result { + fn pre_upgrade() -> Result { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::pre_upgrade() } #[cfg(feature = "try-runtime")] - fn post_upgrade(digest: Self::PreStateDigest) -> Result<(), &'static str> { + fn post_upgrade(digest: Self::PreUpgradeState) -> Result<(), &'static str> { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::post_upgrade(digest) } @@ -204,7 +204,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::offchain_worker(n) } @@ -218,7 +218,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::integrity_test() } @@ -237,7 +237,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< ::BlockNumber, - #pre_state_digest_type + #pre_upgrade_state > >::try_state(n) } diff --git a/frame/support/procedural/src/pallet/parse/hooks.rs b/frame/support/procedural/src/pallet/parse/hooks.rs index 59106cdaccc10..6d1e1fa3de376 100644 --- a/frame/support/procedural/src/pallet/parse/hooks.rs +++ b/frame/support/procedural/src/pallet/parse/hooks.rs @@ -30,8 +30,8 @@ pub struct HooksDef { pub attr_span: proc_macro2::Span, /// Boolean flag, set to true if the `on_runtime_upgrade` method of hooks was implemented. pub has_runtime_upgrade: bool, - /// The `PreStateDigest` type parameter of the trait. - pub pre_state_digest_type: Option, + /// The `PreUpgradeState` type parameter of the trait. + pub pre_upgrade_state: Option, } impl HooksDef { @@ -78,7 +78,7 @@ impl HooksDef { ); syn::Error::new(item_trait.span(), msg) }; - let pre_state_digest_type = match &item_trait.segments[0].arguments { + let pre_upgrade_state = match &item_trait.segments[0].arguments { syn::PathArguments::AngleBracketed(ab) => match ab.args.len() { 1 => None, 2 => match &ab.args[1] { @@ -101,7 +101,7 @@ impl HooksDef { instances, has_runtime_upgrade, where_clause: item.generics.where_clause.clone(), - pre_state_digest_type, + pre_upgrade_state, }) } } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 10489ea9d90d5..999c632dca13b 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1588,7 +1588,7 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> $return { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); @@ -1630,7 +1630,7 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> $crate::dispatch::Weight { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index cf7f48d36c003..7a23011fd461a 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -127,7 +127,7 @@ pub trait OnRuntimeUpgrade { /// TODO: use the `associated_type_defaults` feature once it is stable. /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` /// files to recorrect features dependencies - type PreStateDigest; + type PreUpgradeState; /// Perform a module upgrade. /// @@ -145,24 +145,24 @@ pub trait OnRuntimeUpgrade { /// Execute some pre-checks prior to a runtime upgrade. /// /// Return the state digest which will be passed to `post_upgrade` after upgrading for - /// post-check. NOTE: if this function is not implemented, `Self::PreStateDigest` should be set + /// post-check. NOTE: if this function is not implemented, `Self::PreUpgradeState` should be set /// to `()`. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` /// files to recorrect features dependencies - fn pre_upgrade() -> Result; + fn pre_upgrade() -> Result; /// Execute some post-checks after a runtime upgrade. /// /// The state digest generated by `pre_upgrade` before upgrading will be passed in for /// post-check. NOTE: if `pre_upgrade` is not implemented, the default value of - /// `Self::PreStateDigest` will be used, in such case `Self::PreStateDigest` should be set to + /// `Self::PreUpgradeState` will be used, in such case `Self::PreUpgradeState` should be set to /// `()`. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade(_digest: Self::PreStateDigest) -> Result<(), &'static str> { + fn post_upgrade(_digest: Self::PreUpgradeState) -> Result<(), &'static str> { Ok(()) } } @@ -171,7 +171,7 @@ 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 { - type PreStateDigest = (for_tuples!( #( Tuple::PreStateDigest ),* )); + type PreUpgradeState = (for_tuples!( #( Tuple::PreUpgradeState ),* )); fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); @@ -179,12 +179,12 @@ impl OnRuntimeUpgrade for Tuple { weight } - fn pre_upgrade() -> Result { + fn pre_upgrade() -> Result { Ok((for_tuples!( #( Tuple::pre_upgrade()? ),* ))) } #[cfg(feature = "try-runtime")] - fn post_upgrade(digest: Self::PreStateDigest) -> Result<(), &'static str> { + fn post_upgrade(digest: Self::PreUpgradeState) -> Result<(), &'static str> { for_tuples!( #( Tuple::post_upgrade(digest.Tuple)?; )* ); Ok(()) } @@ -205,12 +205,12 @@ pub trait IntegrityTest { /// The pallet hooks trait. Implementing this lets you express some logic to execute. /// -/// The `PreStateDigest` generic type parameter is used by `pre_upgrade` to generated +/// The `PreUpgradeState` generic type parameter is used by `pre_upgrade` to generated /// a state digest based on the pre-updrade state and will be passed to `post_upgrade` /// after upgrading for post-check. If there is no such need, `()` should be used. /// /// TODO: use the `associated_type_defaults` feature once it is stable. -pub trait Hooks { +pub trait Hooks { /// The block is being finalized. Implement to have something happen. fn on_finalize(_n: BlockNumber) {} @@ -265,25 +265,25 @@ pub trait Hooks { /// Execute some pre-checks prior to a runtime upgrade. /// /// Return the state digest which will be passed to `post_upgrade` after upgrading for - /// post-check. NOTE: if this function is not implemented, `PreStateDigest` should be set + /// post-check. NOTE: if this function is not implemented, `PreUpgradeState` should be set /// to `()`. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result { - Ok(PreStateDigest::default()) + fn pre_upgrade() -> Result { + Ok(PreUpgradeState::default()) } /// Execute some post-checks after a runtime upgrade. /// /// The state digest generated by `pre_upgrade` before upgrading will be passed in for /// post-check. NOTE: if `pre_upgrade` is not implemented, the default value of - /// `PreStateDigest` will be used, in such case `PreStateDigest` should be set to + /// `PreUpgradeState` will be used, in such case `PreUpgradeState` should be set to /// `()`. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade(_digest: PreStateDigest) -> Result<(), &'static str> { + fn post_upgrade(_digest: PreUpgradeState) -> Result<(), &'static str> { Ok(()) } @@ -356,7 +356,7 @@ mod tests { } } impl OnRuntimeUpgrade for Test { - type PreStateDigest = (); + type PreUpgradeState = (); fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 5c3e51011c853..923da64ad781a 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -133,19 +133,19 @@ //! //! ```ignore //! #[cfg(feature = "try-runtime")] -//! type PreStateDigest; +//! type PreUpgradeState; //! //! #[cfg(feature = try-runtime)] -//! fn pre_upgrade() -> Result {} +//! fn pre_upgrade() -> Result {} //! //! #[cfg(feature = try-runtime)] -//! fn post_upgrade(digest: Self::PreStateDigest) -> Result<(), &'static str> {} +//! fn post_upgrade(digest: Self::PreUpgradeState) -> Result<(), &'static str> {} //! ``` //! //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). //! //! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before -//! and after the migration. The `Self::PreStateDigest` (default `()`) generated by `pre_upgrade` +//! and after the migration. The `Self::PreUpgradeState` (default `()`) generated by `pre_upgrade` //! will be passed to `post_upgrade` after upgrading for post checking. //! //! #### Logging From c95eed3832277cd1f7870e66c95119bee1492d3f Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 6 Sep 2022 22:43:14 +0800 Subject: [PATCH 10/23] replace associate type with codec & vec Signed-off-by: linning --- frame/alliance/src/migration.rs | 6 --- frame/bags-list/src/migrations.rs | 18 +++---- frame/executive/src/lib.rs | 6 --- frame/nomination-pools/src/migration.rs | 17 ++---- frame/staking/src/migrations.rs | 18 +++---- .../procedural/src/pallet/expand/hooks.rs | 23 ++++---- frame/support/src/dispatch.rs | 16 +----- frame/support/src/traits/hooks.rs | 52 ++++++++----------- 8 files changed, 54 insertions(+), 102 deletions(-) diff --git a/frame/alliance/src/migration.rs b/frame/alliance/src/migration.rs index 26966ede9cf50..8f98484240061 100644 --- a/frame/alliance/src/migration.rs +++ b/frame/alliance/src/migration.rs @@ -41,15 +41,9 @@ pub fn migrate, I: 'static>() -> Weight { pub struct Migration(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> Weight { migrate::() } - - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } } /// v0_to_v1: `UpForKicking` is replaced by a retirement period. diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index a718c39d2ccb6..97e04d99afbc7 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -25,13 +25,12 @@ use frame_support::{ensure, traits::OnRuntimeUpgrade}; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> frame_support::weights::Weight { frame_support::weights::Weight::zero() } - fn pre_upgrade() -> Result<(), &'static str> { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { // The old explicit storage item. #[frame_support::storage_alias] type CounterForListNodes, I: 'static> = @@ -49,7 +48,7 @@ impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix::count() ); - Ok(()) + Ok(vec![]) } } @@ -83,9 +82,8 @@ mod old { /// A struct that migrates all bags lists to contain a score value. pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { - type PreUpgradeState = u32; - - fn pre_upgrade() -> Result { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { // The list node data should be corrupt at this point, so this is zero. ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); // We can use the helper `old::ListNode` to get the existing data. @@ -93,7 +91,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { let tracked_node_count: u32 = old::CounterForListNodes::::get(); crate::log!(info, "number of nodes before: {:?} {:?}", iter_node_count, tracked_node_count); ensure!(iter_node_count == tracked_node_count, "Node count is wrong."); - Ok(iter_node_count) + Ok(iter_node_count.encode()) } fn on_runtime_upgrade() -> frame_support::weights::Weight { @@ -116,7 +114,9 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { } #[cfg(feature = "try-runtime")] - fn post_upgrade(node_count_before: u32) -> Result<(), &'static str> { + fn post_upgrade(node_count_before: Vec) -> Result<(), &'static str> { + let node_count_before = Decode::decode(&mut node_count_before.as_slice()) + .expect("the state parameter should be something that generated by pre_upgrade"); // Now, the list node data is not corrupt anymore. let iter_node_count_after: u32 = crate::ListNodes::::iter().count() as u32; let tracked_node_count_after: u32 = crate::ListNodes::::count(); diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 4c6df89d7c0be..d954022bb4b70 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -865,18 +865,12 @@ mod tests { struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> Weight { sp_io::storage::set(TEST_KEY, "custom_upgrade".as_bytes()); sp_io::storage::set(CUSTOM_ON_RUNTIME_KEY, &true.encode()); System::deposit_event(frame_system::Event::CodeUpdated); Weight::from_ref_time(100) } - - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } } type Executive = super::Executive< diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 7e7b7fc81bf15..f00dcb676bb8b 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -66,8 +66,6 @@ pub mod v1 { /// Note: The depositor is not optional since he can never change. pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); let onchain = Pallet::::on_chain_storage_version(); @@ -98,12 +96,8 @@ pub mod v1 { } } - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } - #[cfg(feature = "try-runtime")] - fn post_upgrade(_: ()) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> Result<(), &'static str> { // new version must be set. assert_eq!(Pallet::::on_chain_storage_version(), 1); Ok(()) @@ -332,8 +326,6 @@ pub mod v2 { } impl OnRuntimeUpgrade for MigrateToV2 { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> Weight { let current = Pallet::::current_storage_version(); let onchain = Pallet::::on_chain_storage_version(); @@ -353,7 +345,8 @@ pub mod v2 { } } - fn pre_upgrade() -> Result<(), &'static str> { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { // all reward accounts must have more than ED. RewardPools::::iter().for_each(|(id, _)| { assert!( @@ -362,11 +355,11 @@ pub mod v2 { ) }); - Ok(()) + Ok(vec![]) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: ()) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> Result<(), &'static str> { // new version must be set. assert_eq!(Pallet::::on_chain_storage_version(), 2); diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index a6cb424bf3f12..7ee55ecbaaa2c 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -34,8 +34,6 @@ pub mod v10 { /// prevent us from iterating over an arbitrary large number of keys `on_runtime_upgrade`. pub struct MigrateToV10(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV10 { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> frame_support::weights::Weight { if StorageVersion::::get() == Releases::V9_0_0 { let pending_slashes = as Store>::UnappliedSlashes::iter().take(512); @@ -58,23 +56,18 @@ pub mod v10 { T::DbWeight::get().reads(1) } } - - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } } } pub mod v9 { use super::*; + use frame_support::codec::{Decode, Encode}; /// Migration implementation that injects all validators into sorted list. /// /// This is only useful for chains that started their `VoterList` just based on nominators. pub struct InjectValidatorsIntoVoterList(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for InjectValidatorsIntoVoterList { - type PreUpgradeState = u32; - fn on_runtime_upgrade() -> Weight { if StorageVersion::::get() == Releases::V8_0_0 { let prev_count = T::VoterList::count(); @@ -106,18 +99,21 @@ pub mod v9 { } } - fn pre_upgrade() -> Result { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { frame_support::ensure!( StorageVersion::::get() == crate::Releases::V8_0_0, "must upgrade linearly" ); let prev_count = T::VoterList::count(); - Ok(prev_count) + Ok(prev_count.encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: u32) -> Result<(), &'static str> { + fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + let prev_count = Decode::decode(&mut prev_count) + .expect("the state parameter should be something that generated by pre_upgrade"); let post_count = T::VoterList::count(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index c6cc1b611d2ee..0d6be8d479339 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -19,8 +19,7 @@ use crate::pallet::Def; /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { - let (where_clause, span, has_runtime_upgrade, pre_upgrade_state) = match def.hooks.as_ref() - { + let (where_clause, span, has_runtime_upgrade, pre_upgrade_state) = match def.hooks.as_ref() { Some(hooks) => { let where_clause = hooks.where_clause.clone(); let span = hooks.attr_span; @@ -147,8 +146,6 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause { - type PreUpgradeState = #pre_upgrade_state; - fn on_runtime_upgrade() -> #frame_support::weights::Weight { #frame_support::sp_tracing::enter_span!( #frame_support::sp_tracing::trace_span!("on_runtime_update") @@ -170,29 +167,27 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::on_runtime_upgrade() } - #[cfg(not(feature = "try-runtime"))] - fn pre_upgrade() -> Result { - Ok(Self::PreUpgradeState::default()) - } - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result { - < + fn pre_upgrade() -> Result, &'static str> { + use #frame_support::codec::Encode as _; + Ok(< Self as #frame_support::traits::Hooks< ::BlockNumber, #pre_upgrade_state > - >::pre_upgrade() + >::pre_upgrade().encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(digest: Self::PreUpgradeState) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + let state: #pre_upgrade_state = #frame_support::codec::Decode::decode(&mut state.as_slice()) + .expect("the state parameter should be something that generated by pre_upgrade"); < Self as #frame_support::traits::Hooks< ::BlockNumber, #pre_upgrade_state > - >::post_upgrade(digest) + >::post_upgrade(state) } } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 999c632dca13b..50e9cac80ec1c 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1588,8 +1588,6 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> $return { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); let pallet_name = << @@ -1610,12 +1608,8 @@ macro_rules! decl_module { { $( $impl )* } } - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } - #[cfg(feature = "try-runtime")] - fn post_upgrade(_: ()) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> Result<(), &'static str> { Ok(()) } } @@ -1630,8 +1624,6 @@ macro_rules! decl_module { $crate::traits::OnRuntimeUpgrade for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> $crate::dispatch::Weight { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); let pallet_name = << @@ -1649,12 +1641,8 @@ macro_rules! decl_module { $crate::dispatch::Weight::zero() } - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } - #[cfg(feature = "try-runtime")] - fn post_upgrade(_: ()) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> Result<(), &'static str> { Ok(()) } } diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 7a23011fd461a..97cc5c77546e6 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -18,6 +18,7 @@ //! Traits for hooking tasks to events in a blockchain's lifecycle. use crate::weights::Weight; +use codec::{Decode, Encode}; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; @@ -120,15 +121,6 @@ pub trait OnGenesis { /// Implementing this lets you express what should happen when the runtime upgrades, /// and changes may need to occur to your module. pub trait OnRuntimeUpgrade { - /// The state digest type, which is generated by `pre_upgrade` based on the pre-updrade - /// state and will be passed to `post_upgrade` after upgrading for post-check. If there - /// is no such need, `()` should be used. - /// - /// TODO: use the `associated_type_defaults` feature once it is stable. - /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` - /// files to recorrect features dependencies - type PreUpgradeState; - /// Perform a module upgrade. /// /// # Warning @@ -149,9 +141,10 @@ pub trait OnRuntimeUpgrade { /// to `()`. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. - /// TODO: add #[cfg(feature = "try-runtime")], which required changing a lot of `Cargo.toml` - /// files to recorrect features dependencies - fn pre_upgrade() -> Result; + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + Ok(vec![]) + } /// Execute some post-checks after a runtime upgrade. /// @@ -162,30 +155,35 @@ pub trait OnRuntimeUpgrade { /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade(_digest: Self::PreUpgradeState) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { Ok(()) } } -#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] -#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] -#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] +// #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +// #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] +// #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] +// FIXIME: +#[impl_for_tuples(18)] impl OnRuntimeUpgrade for Tuple { - type PreUpgradeState = (for_tuples!( #( Tuple::PreUpgradeState ),* )); - fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); for_tuples!( #( weight = weight.saturating_add(Tuple::on_runtime_upgrade()); )* ); weight } - fn pre_upgrade() -> Result { - Ok((for_tuples!( #( Tuple::pre_upgrade()? ),* ))) + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + Ok((for_tuples!( #( Tuple::pre_upgrade()? ),* )).encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(digest: Self::PreUpgradeState) -> Result<(), &'static str> { - for_tuples!( #( Tuple::post_upgrade(digest.Tuple)?; )* ); + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + // a type alias to workaround the `for_tuples` macro parsing + type Vecu8 = Vec; + let state: (for_tuples!( #( Vecu8 ),* )) = Decode::decode(&mut state.as_slice()) + .expect("the state parameter should be something that generated by pre_upgrade"); + for_tuples!( #( Tuple::post_upgrade(state.Tuple)?; )* ); Ok(()) } } @@ -210,7 +208,7 @@ pub trait IntegrityTest { /// after upgrading for post-check. If there is no such need, `()` should be used. /// /// TODO: use the `associated_type_defaults` feature once it is stable. -pub trait Hooks { +pub trait Hooks { /// The block is being finalized. Implement to have something happen. fn on_finalize(_n: BlockNumber) {} @@ -283,7 +281,7 @@ pub trait Hooks { /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade(_digest: PreUpgradeState) -> Result<(), &'static str> { + fn post_upgrade(_state: PreUpgradeState) -> Result<(), &'static str> { Ok(()) } @@ -356,15 +354,9 @@ mod tests { } } impl OnRuntimeUpgrade for Test { - type PreUpgradeState = (); - fn on_runtime_upgrade() -> Weight { Weight::from_ref_time(20) } - - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } } assert_eq!(<(Test, Test)>::on_initialize(0), Weight::from_ref_time(20)); From 44226fc222fb8c3513b7a656506ee198966eb8ab Mon Sep 17 00:00:00 2001 From: linning Date: Wed, 7 Sep 2022 05:00:57 +0800 Subject: [PATCH 11/23] add helper strcut to help encode/decode tuple Signed-off-by: linning --- frame/bags-list/src/migrations.rs | 10 +++++-- frame/nomination-pools/src/migration.rs | 4 +-- frame/staking/src/migrations.rs | 5 +++- .../procedural/src/pallet/expand/hooks.rs | 4 +-- frame/support/src/dispatch.rs | 4 +-- frame/support/src/traits/hooks.rs | 29 +++++++++++-------- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 97e04d99afbc7..4db500455ba7b 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -20,7 +20,11 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_election_provider_support::ScoreProvider; -use frame_support::{ensure, traits::OnRuntimeUpgrade}; +#[cfg(feature = "try-runtime")] +use frame_support::ensure; +use frame_support::traits::OnRuntimeUpgrade; +#[cfg(feature = "try-runtime")] +use sp_std::vec::Vec; /// A struct that does not migration, but only checks that the counter prefix exists and is correct. pub struct CheckCounterPrefix, I: 'static>(sp_std::marker::PhantomData<(T, I)>); @@ -48,7 +52,7 @@ impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix::count() ); - Ok(vec![]) + Ok(Vec::new()) } } @@ -115,7 +119,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] fn post_upgrade(node_count_before: Vec) -> Result<(), &'static str> { - let node_count_before = Decode::decode(&mut node_count_before.as_slice()) + let node_count_before: u32 = Decode::decode(&mut node_count_before.as_slice()) .expect("the state parameter should be something that generated by pre_upgrade"); // Now, the list node data is not corrupt anymore. let iter_node_count_after: u32 = crate::ListNodes::::iter().count() as u32; diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index f00dcb676bb8b..cbef549264010 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -18,7 +18,7 @@ use super::*; use crate::log; use frame_support::traits::OnRuntimeUpgrade; -use sp_std::collections::btree_map::BTreeMap; +use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; pub mod v1 { use super::*; @@ -355,7 +355,7 @@ pub mod v2 { ) }); - Ok(vec![]) + Ok(Vec::new()) } #[cfg(feature = "try-runtime")] diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 7ee55ecbaaa2c..d594684c5e8a9 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -61,7 +61,10 @@ pub mod v10 { pub mod v9 { use super::*; + #[cfg(feature = "try-runtime")] use frame_support::codec::{Decode, Encode}; + #[cfg(feature = "try-runtime")] + use sp_std::vec::Vec; /// Migration implementation that injects all validators into sorted list. /// @@ -112,7 +115,7 @@ pub mod v9 { #[cfg(feature = "try-runtime")] fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { - let prev_count = Decode::decode(&mut prev_count) + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) .expect("the state parameter should be something that generated by pre_upgrade"); let post_count = T::VoterList::count(); let validators = Validators::::count(); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 0d6be8d479339..add97e9fd5e84 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -168,7 +168,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result<#frame_support::sp_std::vec::Vec, &'static str> { use #frame_support::codec::Encode as _; Ok(< Self as #frame_support::traits::Hooks< @@ -179,7 +179,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> Result<(), &'static str> { let state: #pre_upgrade_state = #frame_support::codec::Decode::decode(&mut state.as_slice()) .expect("the state parameter should be something that generated by pre_upgrade"); < diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 50e9cac80ec1c..78dc68010e09f 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1609,7 +1609,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), &'static str> { Ok(()) } } @@ -1642,7 +1642,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), &'static str> { Ok(()) } } diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 97cc5c77546e6..e1e893d269904 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -143,7 +143,7 @@ pub trait OnRuntimeUpgrade { /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - Ok(vec![]) + Ok(Vec::new()) } /// Execute some post-checks after a runtime upgrade. @@ -160,11 +160,13 @@ pub trait OnRuntimeUpgrade { } } -// #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] -// #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] -// #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] -// FIXIME: -#[impl_for_tuples(18)] +// A helper struct to help encode/decode tuple +#[derive(Default, Encode, Decode)] +struct TupleCodecHelper(Vec>); + +#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +#[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 { fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); @@ -174,16 +176,19 @@ impl OnRuntimeUpgrade for Tuple { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - Ok((for_tuples!( #( Tuple::pre_upgrade()? ),* )).encode()) + let mut helper = TupleCodecHelper::default(); + for_tuples!( #( helper.0.push(Tuple::pre_upgrade()?); )* ); + Ok(helper.encode()) } #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> Result<(), &'static str> { - // a type alias to workaround the `for_tuples` macro parsing - type Vecu8 = Vec; - let state: (for_tuples!( #( Vecu8 ),* )) = Decode::decode(&mut state.as_slice()) - .expect("the state parameter should be something that generated by pre_upgrade"); - for_tuples!( #( Tuple::post_upgrade(state.Tuple)?; )* ); + let helper: TupleCodecHelper = Decode::decode(&mut state.as_slice()) + .expect("the state parameter should be the same as pre_upgrade generated"); + let mut state_iter = helper.0.into_iter(); + for_tuples!( #( Tuple::post_upgrade( + state_iter.next().expect("the state parameter should be the same as pre_upgrade generated") + )?; )* ); Ok(()) } } From 88e4da503547d1b225df25af69dd8c688a24241c Mon Sep 17 00:00:00 2001 From: linning Date: Wed, 7 Sep 2022 05:12:44 +0800 Subject: [PATCH 12/23] update comment Signed-off-by: linning --- frame/executive/src/lib.rs | 4 ++-- frame/support/src/traits/hooks.rs | 12 +++++------- utils/frame/try-runtime/cli/src/lib.rs | 13 ++++++------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index d954022bb4b70..fc4044c0866b4 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -288,13 +288,13 @@ where /// This should only be used for testing. pub fn try_runtime_upgrade() -> Result { // ensure both `pre_upgrade` and `post_upgrade` won't changing the storage root - let digest = { + let state = { let _guard = frame_support::StorageNoopGuard::default(); <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap() }; let weight = Self::execute_on_runtime_upgrade(); frame_support::assert_storage_noop!( - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(digest) + <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(state) .unwrap() ); Ok(weight) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index e1e893d269904..6fd83f4aa251c 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -136,9 +136,8 @@ pub trait OnRuntimeUpgrade { /// Execute some pre-checks prior to a runtime upgrade. /// - /// Return the state digest which will be passed to `post_upgrade` after upgrading for - /// post-check. NOTE: if this function is not implemented, `Self::PreUpgradeState` should be set - /// to `()`. + /// Return the encoded state digest which will be passed to `post_upgrade` after upgrading for + /// post-check. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] @@ -148,10 +147,9 @@ pub trait OnRuntimeUpgrade { /// Execute some post-checks after a runtime upgrade. /// - /// The state digest generated by `pre_upgrade` before upgrading will be passed in for - /// post-check. NOTE: if `pre_upgrade` is not implemented, the default value of - /// `Self::PreUpgradeState` will be used, in such case `Self::PreUpgradeState` should be set to - /// `()`. + /// The encoded state digest generated by `pre_upgrade` before upgrading will be passed in for + /// post-check. NOTE: if `pre_upgrade` is not implemented an empty vec will be passed in, in + /// such case `post_upgrade` should ignore it. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 923da64ad781a..affdf8f57111a 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -132,21 +132,20 @@ //! added, given the right flag: //! //! ```ignore -//! #[cfg(feature = "try-runtime")] -//! type PreUpgradeState; -//! +//! //! #[cfg(feature = try-runtime)] -//! fn pre_upgrade() -> Result {} +//! fn pre_upgrade() -> Result, &'static str> {} //! //! #[cfg(feature = try-runtime)] -//! fn post_upgrade(digest: Self::PreUpgradeState) -> Result<(), &'static str> {} +//! fn post_upgrade(state: Vec) -> Result<(), &'static str> {} //! ``` //! //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). //! //! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before -//! and after the migration. The `Self::PreUpgradeState` (default `()`) generated by `pre_upgrade` -//! will be passed to `post_upgrade` after upgrading for post checking. +//! and after the migration. The `Vec` generated by `pre_upgrade` is a encoded state digest +//! which will be passed to `post_upgrade` after upgrading for post checking. NOTE: `post_upgrade` +//! should decode it into the same type as `pre_upgrade` encoded. //! //! #### Logging //! From 33a4227ccfb7ddcee5e8b1e82e3852890cd0b1ff Mon Sep 17 00:00:00 2001 From: linning Date: Wed, 7 Sep 2022 05:18:10 +0800 Subject: [PATCH 13/23] fix Signed-off-by: linning --- frame/bags-list/src/migrations.rs | 3 ++- frame/support/src/dispatch.rs | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 4db500455ba7b..df26e7089a9fb 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -20,9 +20,10 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_election_provider_support::ScoreProvider; +use frame_support::traits::OnRuntimeUpgrade; + #[cfg(feature = "try-runtime")] use frame_support::ensure; -use frame_support::traits::OnRuntimeUpgrade; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 78dc68010e09f..272ecc3377710 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1608,6 +1608,11 @@ macro_rules! decl_module { { $( $impl )* } } + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, &'static str> { + Ok($crate::sp_std::vec::Vec::new()) + } + #[cfg(feature = "try-runtime")] fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), &'static str> { Ok(()) @@ -1641,6 +1646,11 @@ macro_rules! decl_module { $crate::dispatch::Weight::zero() } + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, &'static str> { + Ok($crate::sp_std::vec::Vec::new()) + } + #[cfg(feature = "try-runtime")] fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), &'static str> { Ok(()) From ffea86287252cf401859e15f9438d5ec1e11d9bd Mon Sep 17 00:00:00 2001 From: linning Date: Wed, 7 Sep 2022 15:29:55 +0800 Subject: [PATCH 14/23] add test Signed-off-by: linning --- frame/support/src/traits/hooks.rs | 61 +++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 6fd83f4aa251c..eafaed7f07c07 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -421,4 +421,65 @@ mod tests { ON_IDLE_INVOCATION_ORDER.clear(); } } + + #[cfg(feature = "try-runtime")] + #[test] + fn on_runtime_upgrade_tuple() { + struct Test1; + struct Test2; + struct Test3; + + impl OnRuntimeUpgrade for Test1 { + fn pre_upgrade() -> Result, &'static str> { + Ok("Test1".encode()) + } + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + let s: String = Decode::decode(&mut state.as_slice()).unwrap(); + assert_eq!(s, "Test1"); + Ok(()) + } + } + impl OnRuntimeUpgrade for Test2 { + fn pre_upgrade() -> Result, &'static str> { + Ok(100u32.encode()) + } + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + let s: u32 = Decode::decode(&mut state.as_slice()).unwrap(); + assert_eq!(s, 100); + Ok(()) + } + } + impl OnRuntimeUpgrade for Test3 { + fn pre_upgrade() -> Result, &'static str> { + Ok(true.encode()) + } + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + let s: bool = Decode::decode(&mut state.as_slice()).unwrap(); + assert_eq!(s, true); + Ok(()) + } + } + + type Test123 = (Test1, Test2, Test3); + let state = ::pre_upgrade().unwrap(); + let helper: TupleCodecHelper = Decode::decode(&mut state.as_slice()).unwrap(); + assert_eq!( + ::decode(&mut helper.0[0].as_slice()).unwrap(), + "Test1".to_owned() + ); + assert_eq!(::decode(&mut helper.0[1].as_slice()).unwrap(), 100u32); + assert_eq!(::decode(&mut helper.0[2].as_slice()).unwrap(), true); + ::post_upgrade(state).unwrap(); + + type Test321 = (Test3, Test2, Test1); + let state = ::pre_upgrade().unwrap(); + let helper: TupleCodecHelper = Decode::decode(&mut state.as_slice()).unwrap(); + assert_eq!(::decode(&mut helper.0[0].as_slice()).unwrap(), true); + assert_eq!(::decode(&mut helper.0[1].as_slice()).unwrap(), 100u32); + assert_eq!( + ::decode(&mut helper.0[2].as_slice()).unwrap(), + "Test1".to_owned() + ); + ::post_upgrade(state).unwrap(); + } } From 4e19a3fd45b274c4473a408a77448feb977f6083 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 9 Sep 2022 02:30:00 +0800 Subject: [PATCH 15/23] address comment Signed-off-by: linning --- frame/executive/src/lib.rs | 7 ++- .../procedural/src/pallet/expand/hooks.rs | 54 +++++++------------ .../procedural/src/pallet/parse/hooks.rs | 21 -------- frame/support/src/traits/hooks.rs | 28 ++++------ utils/frame/try-runtime/cli/src/lib.rs | 6 +-- 5 files changed, 35 insertions(+), 81 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index fc4044c0866b4..1e67bbb1ccf95 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -293,10 +293,9 @@ where <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap() }; let weight = Self::execute_on_runtime_upgrade(); - frame_support::assert_storage_noop!( - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(state) - .unwrap() - ); + let _guard = frame_support::StorageNoopGuard::default(); + <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(state) + .unwrap(); Ok(weight) } } diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index add97e9fd5e84..48d4aec436d40 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -19,19 +19,15 @@ use crate::pallet::Def; /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { - let (where_clause, span, has_runtime_upgrade, pre_upgrade_state) = match def.hooks.as_ref() { + let (where_clause, span, has_runtime_upgrade) = match def.hooks.as_ref() { Some(hooks) => { let where_clause = hooks.where_clause.clone(); let span = hooks.attr_span; let has_runtime_upgrade = hooks.has_runtime_upgrade; - let pre_upgrade_state = hooks.pre_upgrade_state.clone(); - (where_clause, span, has_runtime_upgrade, pre_upgrade_state) + (where_clause, span, has_runtime_upgrade) }, - None => (None, def.pallet_struct.attr_span, false, None), + None => (None, def.pallet_struct.attr_span, false), }; - // use `PreUpgradeState`'s default type `()` if it is not set explicitly - let pre_upgrade_state = - pre_upgrade_state.map_or(quote::quote! { () }, |t| quote::quote! { #t }); let frame_support = &def.frame_support; let type_impl_gen = &def.type_impl_generics(span); @@ -79,7 +75,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let frame_system = &def.frame_system; quote::quote! { impl<#type_impl_gen> - #frame_support::traits::Hooks<::BlockNumber, #pre_upgrade_state> + #frame_support::traits::Hooks<::BlockNumber> for Pallet<#type_use_gen> {} } } else { @@ -99,8 +95,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::on_finalize(n) } @@ -116,8 +111,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ) -> #frame_support::weights::Weight { < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::on_idle(n, remaining_weight) } @@ -135,8 +129,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::on_initialize(n) } @@ -161,32 +154,26 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::on_runtime_upgrade() } #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result<#frame_support::sp_std::vec::Vec, &'static str> { - use #frame_support::codec::Encode as _; - Ok(< - Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state - > - >::pre_upgrade().encode()) + < + Self + as + #frame_support::traits::Hooks<::BlockNumber> + >::pre_upgrade() } #[cfg(feature = "try-runtime")] fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> Result<(), &'static str> { - let state: #pre_upgrade_state = #frame_support::codec::Decode::decode(&mut state.as_slice()) - .expect("the state parameter should be something that generated by pre_upgrade"); < - Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state - > + Self + as + #frame_support::traits::Hooks<::BlockNumber> >::post_upgrade(state) } } @@ -198,8 +185,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { fn offchain_worker(n: ::BlockNumber) { < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::offchain_worker(n) } @@ -212,8 +198,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { fn integrity_test() { < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::integrity_test() } @@ -231,8 +216,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #log_try_state < Self as #frame_support::traits::Hooks< - ::BlockNumber, - #pre_upgrade_state + ::BlockNumber > >::try_state(n) } diff --git a/frame/support/procedural/src/pallet/parse/hooks.rs b/frame/support/procedural/src/pallet/parse/hooks.rs index 6d1e1fa3de376..2dc8f4da47c5f 100644 --- a/frame/support/procedural/src/pallet/parse/hooks.rs +++ b/frame/support/procedural/src/pallet/parse/hooks.rs @@ -30,8 +30,6 @@ pub struct HooksDef { pub attr_span: proc_macro2::Span, /// Boolean flag, set to true if the `on_runtime_upgrade` method of hooks was implemented. pub has_runtime_upgrade: bool, - /// The `PreUpgradeState` type parameter of the trait. - pub pre_upgrade_state: Option, } impl HooksDef { @@ -72,24 +70,6 @@ impl HooksDef { return Err(syn::Error::new(item_trait.span(), msg)) } - let err = || { - let msg = format!( - "Invalid generic type parameters for Hooks, expected Hooks or Hooks" - ); - syn::Error::new(item_trait.span(), msg) - }; - let pre_upgrade_state = match &item_trait.segments[0].arguments { - syn::PathArguments::AngleBracketed(ab) => match ab.args.len() { - 1 => None, - 2 => match &ab.args[1] { - syn::GenericArgument::Type(t) => Some(t.clone()), - _ => return Err(err()), - }, - _ => return Err(err()), - }, - _ => return Err(err()), - }; - let has_runtime_upgrade = item.items.iter().any(|i| match i { syn::ImplItem::Method(method) => method.sig.ident == "on_runtime_upgrade", _ => false, @@ -101,7 +81,6 @@ impl HooksDef { instances, has_runtime_upgrade, where_clause: item.generics.where_clause.clone(), - pre_upgrade_state, }) } } diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index eafaed7f07c07..3292c6163e901 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -136,7 +136,7 @@ pub trait OnRuntimeUpgrade { /// Execute some pre-checks prior to a runtime upgrade. /// - /// Return the encoded state digest which will be passed to `post_upgrade` after upgrading for + /// Return an encoded state digest which will be passed to `post_upgrade` after upgrading for /// post-check. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. @@ -205,13 +205,7 @@ pub trait IntegrityTest { } /// The pallet hooks trait. Implementing this lets you express some logic to execute. -/// -/// The `PreUpgradeState` generic type parameter is used by `pre_upgrade` to generated -/// a state digest based on the pre-updrade state and will be passed to `post_upgrade` -/// after upgrading for post-check. If there is no such need, `()` should be used. -/// -/// TODO: use the `associated_type_defaults` feature once it is stable. -pub trait Hooks { +pub trait Hooks { /// The block is being finalized. Implement to have something happen. fn on_finalize(_n: BlockNumber) {} @@ -265,26 +259,24 @@ pub trait Hooks { /// Execute some pre-checks prior to a runtime upgrade. /// - /// Return the state digest which will be passed to `post_upgrade` after upgrading for - /// post-check. NOTE: if this function is not implemented, `PreUpgradeState` should be set - /// to `()`. + /// Return an encoded state which will be passed to `post_upgrade` after upgrading for + /// post-check. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result { - Ok(PreUpgradeState::default()) + fn pre_upgrade() -> Result, &'static str> { + Ok(Vec::new()) } /// Execute some post-checks after a runtime upgrade. /// - /// The state digest generated by `pre_upgrade` before upgrading will be passed in for - /// post-check. NOTE: if `pre_upgrade` is not implemented, the default value of - /// `PreUpgradeState` will be used, in such case `PreUpgradeState` should be set to - /// `()`. + /// The encoded state digest generated by `pre_upgrade` before upgrading will be passed in for + /// post-check. NOTE: if `pre_upgrade` is not implemented an empty vec will be passed in, in + /// such case `post_upgrade` should ignore it. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: PreUpgradeState) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { Ok(()) } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index affdf8f57111a..d5259525f08b9 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -143,9 +143,9 @@ //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). //! //! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before -//! and after the migration. The `Vec` generated by `pre_upgrade` is a encoded state digest -//! which will be passed to `post_upgrade` after upgrading for post checking. NOTE: `post_upgrade` -//! should decode it into the same type as `pre_upgrade` encoded. +//! and after the migration. A `Vec` is generated by `pre_upgrade` as a encoded state digest +//! which will be passed to `post_upgrade` after upgrading for post checking. NOTE: `post_upgrade` +//! should decode it into the same type as `pre_upgrade` encoded before using it. //! //! #### Logging //! From 313bc4e59ee0cac165ab35b2b5942d872bf85c59 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 9 Sep 2022 03:27:27 +0800 Subject: [PATCH 16/23] fix doc Signed-off-by: linning --- frame/support/src/traits/try_runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index 0683352c72388..640bb566a65af 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -32,7 +32,7 @@ pub enum Select { RoundRobin(u32), /// Run only pallets who's name matches the given list. /// - /// Pallet names are obtained from [`PalletInfoAccess`]. + /// Pallet names are obtained from [`super::PalletInfoAccess`]. Only(Vec>), } From af17cbf131664a456106c5ee8db1b6db9bafe3d5 Mon Sep 17 00:00:00 2001 From: linning Date: Fri, 9 Sep 2022 16:11:37 +0800 Subject: [PATCH 17/23] fix ci Signed-off-by: linning --- frame/nomination-pools/src/migration.rs | 6 +++--- .../storage_ensure_span_are_ok_on_wrong_gen.stderr | 4 ++-- .../storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index d2ce41f3ee823..4d78f2fa76c96 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -430,16 +430,16 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { + fn pre_upgrade() -> Result, &'static str> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" ); - Ok(()) + Ok(Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> Result<(), &'static str> { ensure!( Metadata::::iter_keys().all(|id| BondedPools::::contains_key(&id)), "not all of the stale metadata has been removed" diff --git a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr index e674e49eddbe5..360db500f9c0d 100644 --- a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr +++ b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr @@ -28,7 +28,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 280 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `PartialStorageInfoTrait` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` @@ -103,7 +103,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 280 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` diff --git a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr index ecdc18263432e..c617e98927bb8 100644 --- a/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr +++ b/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr @@ -28,7 +28,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 280 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `PartialStorageInfoTrait` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` @@ -103,7 +103,7 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied <&[(T,)] as EncodeLike>> <&[(T,)] as EncodeLike>> <&[T] as EncodeLike>> - and 279 others + and 280 others = note: required because of the requirements on the impl of `FullEncode` for `Bar` = note: required because of the requirements on the impl of `FullCodec` for `Bar` = note: required because of the requirements on the impl of `StorageEntryMetadataBuilder` for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` From 90108ecdac2d487642a84bfeef318c7c8ec83aa8 Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 13 Sep 2022 18:29:13 +0800 Subject: [PATCH 18/23] address comment Signed-off-by: linning --- frame/support/src/traits/hooks.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 3292c6163e901..58cfaf0b98e51 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -158,10 +158,6 @@ pub trait OnRuntimeUpgrade { } } -// A helper struct to help encode/decode tuple -#[derive(Default, Encode, Decode)] -struct TupleCodecHelper(Vec>); - #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] @@ -174,16 +170,16 @@ impl OnRuntimeUpgrade for Tuple { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - let mut helper = TupleCodecHelper::default(); - for_tuples!( #( helper.0.push(Tuple::pre_upgrade()?); )* ); - Ok(helper.encode()) + let mut state: Vec> = Vec::default(); + for_tuples!( #( state.push(Tuple::pre_upgrade()?); )* ); + Ok(state.encode()) } #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> Result<(), &'static str> { - let helper: TupleCodecHelper = Decode::decode(&mut state.as_slice()) + let state: Vec> = Decode::decode(&mut state.as_slice()) .expect("the state parameter should be the same as pre_upgrade generated"); - let mut state_iter = helper.0.into_iter(); + 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") )?; )* ); From eb0cb8b5ea93aeeffa8b58c98d62bb417dae7838 Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 13 Sep 2022 18:29:30 +0800 Subject: [PATCH 19/23] add more test cases Signed-off-by: linning --- frame/support/src/traits/hooks.rs | 53 ++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 58cfaf0b98e51..b566d69a133ab 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -448,26 +448,55 @@ mod tests { } } + type TestEmpty = (); + let origin_state = ::pre_upgrade().unwrap(); + let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); + assert!(states.is_empty()); + ::post_upgrade(origin_state).unwrap(); + + type Test1Tuple = (Test1,); + let origin_state = ::pre_upgrade().unwrap(); + let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); + assert_eq!(states.len(), 1); + assert_eq!( + ::decode(&mut states[0].as_slice()).unwrap(), + "Test1".to_owned() + ); + ::post_upgrade(origin_state).unwrap(); + type Test123 = (Test1, Test2, Test3); - let state = ::pre_upgrade().unwrap(); - let helper: TupleCodecHelper = Decode::decode(&mut state.as_slice()).unwrap(); + let origin_state = ::pre_upgrade().unwrap(); + let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); assert_eq!( - ::decode(&mut helper.0[0].as_slice()).unwrap(), + ::decode(&mut states[0].as_slice()).unwrap(), "Test1".to_owned() ); - assert_eq!(::decode(&mut helper.0[1].as_slice()).unwrap(), 100u32); - assert_eq!(::decode(&mut helper.0[2].as_slice()).unwrap(), true); - ::post_upgrade(state).unwrap(); + assert_eq!(::decode(&mut states[1].as_slice()).unwrap(), 100u32); + assert_eq!(::decode(&mut states[2].as_slice()).unwrap(), true); + ::post_upgrade(origin_state).unwrap(); type Test321 = (Test3, Test2, Test1); - let state = ::pre_upgrade().unwrap(); - let helper: TupleCodecHelper = Decode::decode(&mut state.as_slice()).unwrap(); - assert_eq!(::decode(&mut helper.0[0].as_slice()).unwrap(), true); - assert_eq!(::decode(&mut helper.0[1].as_slice()).unwrap(), 100u32); + let origin_state = ::pre_upgrade().unwrap(); + let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); + assert_eq!(::decode(&mut states[0].as_slice()).unwrap(), true); + assert_eq!(::decode(&mut states[1].as_slice()).unwrap(), 100u32); + assert_eq!( + ::decode(&mut states[2].as_slice()).unwrap(), + "Test1".to_owned() + ); + ::post_upgrade(origin_state).unwrap(); + + type TestNested123 = (Test1, (Test2, Test3)); + let origin_state = ::pre_upgrade().unwrap(); + let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); assert_eq!( - ::decode(&mut helper.0[2].as_slice()).unwrap(), + ::decode(&mut states[0].as_slice()).unwrap(), "Test1".to_owned() ); - ::post_upgrade(state).unwrap(); + // nested state for (Test2, Test3) + let nested_states: Vec> = Decode::decode(&mut states[1].as_slice()).unwrap(); + assert_eq!(::decode(&mut nested_states[0].as_slice()).unwrap(), 100u32); + assert_eq!(::decode(&mut nested_states[1].as_slice()).unwrap(), true); + ::post_upgrade(origin_state).unwrap(); } } From 96f3a2129588361ac540da55519a4595a542beef Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 13 Sep 2022 18:40:23 +0800 Subject: [PATCH 20/23] make clippy happy Signed-off-by: linning --- frame/support/src/traits/hooks.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index b566d69a133ab..8fb6b93b3996b 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -18,10 +18,11 @@ //! Traits for hooking tasks to events in a blockchain's lifecycle. use crate::weights::Weight; -use codec::{Decode, Encode}; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; +#[cfg(feature = "try-runtime")] +use codec::{Decode, Encode}; /// The block initialization trait. /// From c6237204ea068255d73c481cd9ff5c68965732d7 Mon Sep 17 00:00:00 2001 From: linning Date: Tue, 13 Sep 2022 19:35:00 +0800 Subject: [PATCH 21/23] fmt Signed-off-by: linning --- frame/support/src/traits/hooks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 8fb6b93b3996b..a2e114fce9fd6 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -21,6 +21,7 @@ use crate::weights::Weight; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; + #[cfg(feature = "try-runtime")] use codec::{Decode, Encode}; From 7df0f563f707fcaea3f3be8f93ce4966eba1ab8e Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 17 Sep 2022 02:24:32 +0800 Subject: [PATCH 22/23] update comment Signed-off-by: linning --- frame/bags-list/src/migrations.rs | 4 ++-- frame/executive/src/lib.rs | 2 +- frame/staking/src/migrations.rs | 2 +- frame/support/src/traits/hooks.rs | 22 ++++++++++++---------- utils/frame/try-runtime/cli/src/lib.rs | 6 +++--- 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index df26e7089a9fb..e1dc9f777e537 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -121,8 +121,8 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] fn post_upgrade(node_count_before: Vec) -> Result<(), &'static str> { let node_count_before: u32 = Decode::decode(&mut node_count_before.as_slice()) - .expect("the state parameter should be something that generated by pre_upgrade"); - // Now, the list node data is not corrupt anymore. + .expect("the state parameter should be something that was generated by pre_upgrade"); + // Now the list node data is not corrupt anymore. let iter_node_count_after: u32 = crate::ListNodes::::iter().count() as u32; let tracked_node_count_after: u32 = crate::ListNodes::::count(); crate::log!( diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 651680018aac7..5aa8387b9c2bf 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -287,7 +287,7 @@ where /// /// This should only be used for testing. pub fn try_runtime_upgrade() -> Result { - // ensure both `pre_upgrade` and `post_upgrade` won't changing the storage root + // 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() diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index d594684c5e8a9..10720ceec27a7 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -116,7 +116,7 @@ pub mod v9 { #[cfg(feature = "try-runtime")] fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) - .expect("the state parameter should be something that generated by pre_upgrade"); + .expect("the state parameter should be something that was generated by pre_upgrade"); let post_count = T::VoterList::count(); let validators = Validators::::count(); assert!(post_count == prev_count + validators); diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index a2e114fce9fd6..eb1106e4c383f 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -138,8 +138,9 @@ pub trait OnRuntimeUpgrade { /// Execute some pre-checks prior to a runtime upgrade. /// - /// Return an encoded state digest which will be passed to `post_upgrade` after upgrading for - /// post-check. + /// Return a `Vec` that can contain arbitrary encoded data (usually some pre-upgrade state), + /// which will be passed to `post_upgrade` after upgrading for post-check. An empty vector + /// should be returned if there is no such need. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] @@ -149,9 +150,9 @@ pub trait OnRuntimeUpgrade { /// Execute some post-checks after a runtime upgrade. /// - /// The encoded state digest generated by `pre_upgrade` before upgrading will be passed in for - /// post-check. NOTE: if `pre_upgrade` is not implemented an empty vec will be passed in, in - /// such case `post_upgrade` should ignore it. + /// The `state` parameter is the `Vec` returned by `pre_upgrade` before upgrading, which + /// can be used for post-check. NOTE: if `pre_upgrade` is not implemented an empty vector will + /// be passed in, in such case `post_upgrade` should ignore it. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] @@ -257,8 +258,9 @@ pub trait Hooks { /// Execute some pre-checks prior to a runtime upgrade. /// - /// Return an encoded state which will be passed to `post_upgrade` after upgrading for - /// post-check. + /// Return a `Vec` that can contain arbitrary encoded data (usually some pre-upgrade state), + /// which will be passed to `post_upgrade` after upgrading for post-check. An empty vector + /// should be returned if there is no such need. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] @@ -268,9 +270,9 @@ pub trait Hooks { /// Execute some post-checks after a runtime upgrade. /// - /// The encoded state digest generated by `pre_upgrade` before upgrading will be passed in for - /// post-check. NOTE: if `pre_upgrade` is not implemented an empty vec will be passed in, in - /// such case `post_upgrade` should ignore it. + /// The `state` parameter is the `Vec` returned by `pre_upgrade` before upgrading, which + /// can be used for post-check. NOTE: if `pre_upgrade` is not implemented an empty vector will + /// be passed in, in such case `post_upgrade` should ignore it. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. #[cfg(feature = "try-runtime")] diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 3dfebe83a8a7e..c3382045add16 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -143,9 +143,9 @@ //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). //! //! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before -//! and after the migration. A `Vec` is generated by `pre_upgrade` as a encoded state digest -//! which will be passed to `post_upgrade` after upgrading for post checking. NOTE: `post_upgrade` -//! should decode it into the same type as `pre_upgrade` encoded before using it. +//! and after the migration. Moreover, `pre_upgrade` can return a `Vec` that contains arbitrary +//! encoded data (usually some pre-upgrade state) which will be passed to `post_upgrade` after +//! upgrading and used for post checking. //! //! #### Logging //! From 87141132fdcc4c466bcf291b5f81abb1e4680a55 Mon Sep 17 00:00:00 2001 From: linning Date: Sat, 17 Sep 2022 02:26:41 +0800 Subject: [PATCH 23/23] fmt Signed-off-by: linning --- frame/staking/src/migrations.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 10720ceec27a7..28bf1b32f697f 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -115,8 +115,9 @@ pub mod v9 { #[cfg(feature = "try-runtime")] fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { - let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) - .expect("the state parameter should be something that was generated by pre_upgrade"); + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( + "the state parameter should be something that was generated by pre_upgrade", + ); let post_count = T::VoterList::count(); let validators = Validators::::count(); assert!(post_count == prev_count + validators);