From e1b56f8dd3a070ddae01a2daf222e7f9b4480d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 29 Oct 2020 20:20:08 +0100 Subject: [PATCH] Make sure pallet versions are set at genesis (#7451) * Make sure pallet versions are set at genesis This pr ensures that pallet versions are also set at genesis. It does this by hooking into the runtime `GenesisConfig` which means that it will only work when the storage is setup using this genesis config. So, the version will not be set in pallet local tests. However, I think this isn't such a problem. The genesis config will call `on_genesis` on all pallets. This function comes from the new trait `OnGenesis`. Currently the user is not able to provide any custom implementation of this trait. Besides that it also implements `Clone` and `Copy` for the pallet version struct. This pr also moves the macro for generating the runtime genesis config to `frame-support` as most of the other FRAME related macros. * Reduce line width * Update frame/support/src/traits.rs Co-authored-by: Alexander Popiak Co-authored-by: Alexander Popiak --- .../procedural/src/construct_runtime/mod.rs | 8 +- substrate/frame/support/src/dispatch.rs | 24 +-- substrate/frame/support/src/genesis_config.rs | 141 ++++++++++++++++++ substrate/frame/support/src/lib.rs | 2 + substrate/frame/support/src/traits.rs | 43 +++++- .../support/test/tests/pallet_version.rs | 19 ++- substrate/primitives/runtime/src/lib.rs | 110 -------------- 7 files changed, 218 insertions(+), 129 deletions(-) create mode 100644 substrate/frame/support/src/genesis_config.rs diff --git a/substrate/frame/support/procedural/src/construct_runtime/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/mod.rs index f355593def..15f0935f38 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/mod.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/mod.rs @@ -280,8 +280,8 @@ fn decl_outer_config<'a>( ) }); quote!( - #scrate::sp_runtime::impl_outer_config! { - pub struct GenesisConfig for #runtime { + #scrate::impl_outer_config! { + pub struct GenesisConfig for #runtime where AllModulesWithSystem = AllModulesWithSystem { #(#modules_tokens)* } } @@ -462,9 +462,13 @@ fn decl_all_modules<'a>( .filter(|n| **n != SYSTEM_MODULE_NAME) .fold(TokenStream2::default(), |combined, name| quote!((#name, #combined))); + let all_modules_with_system = names.iter() + .fold(TokenStream2::default(), |combined, name| quote!((#name, #combined))); + quote!( #types type AllModules = ( #all_modules ); + type AllModulesWithSystem = ( #all_modules_with_system ); ) } diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index b96d6194eb..d55faa28d1 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -1325,11 +1325,8 @@ macro_rules! decl_module { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); let result: $return = (|| { $( $impl )* })(); - let key = $crate::traits::PalletVersion::storage_key::< - <$trait_instance as $system::Trait>::PalletInfo, Self - >().expect("Every active pallet has a name in the runtime; qed"); - let version = $crate::crate_to_pallet_version!(); - $crate::storage::unhashed::put(&key, &version); + $crate::crate_to_pallet_version!() + .put_into_storage::<<$trait_instance as $system::Trait>::PalletInfo, Self>(); let additional_write = < <$trait_instance as $system::Trait>::DbWeight as $crate::traits::Get<_> @@ -1352,11 +1349,8 @@ macro_rules! decl_module { fn on_runtime_upgrade() -> $crate::dispatch::Weight { $crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); - let key = $crate::traits::PalletVersion::storage_key::< - <$trait_instance as $system::Trait>::PalletInfo, Self - >().expect("Every active pallet has a name in the runtime; qed"); - let version = $crate::crate_to_pallet_version!(); - $crate::storage::unhashed::put(&key, &version); + $crate::crate_to_pallet_version!() + .put_into_storage::<<$trait_instance as $system::Trait>::PalletInfo, Self>(); < <$trait_instance as $system::Trait>::DbWeight as $crate::traits::Get<_> @@ -1837,6 +1831,16 @@ macro_rules! decl_module { } } + // Implement `OnGenesis` for `Module` + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::OnGenesis + for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* + { + fn on_genesis() { + $crate::crate_to_pallet_version!() + .put_into_storage::<<$trait_instance as $system::Trait>::PalletInfo, Self>(); + } + } + // manual implementation of clone/eq/partialeq because using derive erroneously requires // clone/eq/partialeq from T. impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::Clone diff --git a/substrate/frame/support/src/genesis_config.rs b/substrate/frame/support/src/genesis_config.rs new file mode 100644 index 0000000000..99f8ad886d --- /dev/null +++ b/substrate/frame/support/src/genesis_config.rs @@ -0,0 +1,141 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Macros for generating the runtime genesis config. + +/// Helper macro for `impl_outer_config` +#[macro_export] +macro_rules! __impl_outer_config_types { + // Generic + Instance + ( + $concrete:ident $config:ident $snake:ident { $instance:ident } < $ignore:ident >; + $( $rest:tt )* + ) => { + #[cfg(any(feature = "std", test))] + pub type $config = $snake::GenesisConfig<$concrete, $snake::$instance>; + $crate::__impl_outer_config_types! { $concrete $( $rest )* } + }; + // Generic + ( + $concrete:ident $config:ident $snake:ident < $ignore:ident >; + $( $rest:tt )* + ) => { + #[cfg(any(feature = "std", test))] + pub type $config = $snake::GenesisConfig<$concrete>; + $crate::__impl_outer_config_types! { $concrete $( $rest )* } + }; + // No Generic and maybe Instance + ( + $concrete:ident $config:ident $snake:ident $( { $instance:ident } )?; + $( $rest:tt )* + ) => { + #[cfg(any(feature = "std", test))] + pub type $config = $snake::GenesisConfig; + $crate::__impl_outer_config_types! { $concrete $( $rest )* } + }; + ($concrete:ident) => () +} + +/// Implement the runtime genesis configuration. +/// +/// This combines all pallet genesis configurations into one runtime +/// specific genesis configuration. +/// +/// ```ignore +/// pub struct GenesisConfig for Runtime where AllModulesWithSystem = AllModulesWithSystem { +/// rust_module_one: Option, +/// ... +/// } +/// ``` +#[macro_export] +macro_rules! impl_outer_config { + ( + pub struct $main:ident for $concrete:ident where + AllModulesWithSystem = $all_modules_with_system:ident + { + $( $config:ident => + $snake:ident $( $instance:ident )? $( <$generic:ident> )*, )* + } + ) => { + $crate::__impl_outer_config_types! { + $concrete $( $config $snake $( { $instance } )? $( <$generic> )*; )* + } + + $crate::paste::item! { + #[cfg(any(feature = "std", test))] + #[derive($crate::serde::Serialize, $crate::serde::Deserialize, Default)] + #[serde(rename_all = "camelCase")] + #[serde(deny_unknown_fields)] + pub struct $main { + $( + pub [< $snake $(_ $instance )? >]: Option<$config>, + )* + } + #[cfg(any(feature = "std", test))] + impl $crate::sp_runtime::BuildStorage for $main { + fn assimilate_storage( + &self, + storage: &mut $crate::sp_runtime::Storage, + ) -> std::result::Result<(), String> { + $( + if let Some(ref extra) = self.[< $snake $(_ $instance )? >] { + $crate::impl_outer_config! { + @CALL_FN + $concrete; + $snake; + $( $instance )?; + extra; + storage; + } + } + )* + + $crate::BasicExternalities::execute_with_storage(storage, || { + <$all_modules_with_system as $crate::traits::OnGenesis>::on_genesis(); + }); + + Ok(()) + } + } + } + }; + (@CALL_FN + $runtime:ident; + $module:ident; + $instance:ident; + $extra:ident; + $storage:ident; + ) => { + $crate::sp_runtime::BuildModuleGenesisStorage::<$runtime, $module::$instance>::build_module_genesis_storage( + $extra, + $storage, + )?; + }; + (@CALL_FN + $runtime:ident; + $module:ident; + ; + $extra:ident; + $storage:ident; + ) => { + $crate::sp_runtime::BuildModuleGenesisStorage:: + <$runtime, $module::__InherentHiddenInstance>::build_module_genesis_storage( + $extra, + $storage, + )?; + } +} diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 99cfcb66b3..a132b787fd 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -58,6 +58,8 @@ pub mod event; #[macro_use] pub mod metadata; #[macro_use] +pub mod genesis_config; +#[macro_use] pub mod inherent; #[macro_use] pub mod unsigned; diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 96d7244efe..1fadb079e5 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -1421,16 +1421,19 @@ pub trait GetCallMetadata { fn get_call_metadata(&self) -> CallMetadata; } -/// The block finalization trait. Implementing this lets you express what should happen -/// for your module when the block is ending. +/// The block finalization trait. +/// +/// Implementing this lets you express what should happen for your pallet when the block is ending. #[impl_for_tuples(30)] pub trait OnFinalize { /// The block is being finalized. Implement to have something happen. fn on_finalize(_n: BlockNumber) {} } -/// The block initialization trait. Implementing this lets you express what should happen -/// for your module when the block is beginning (right before the first extrinsic is executed). +/// The block initialization trait. +/// +/// Implementing this lets you express what should happen for your pallet when the block is +/// beginning (right before the first extrinsic is executed). pub trait OnInitialize { /// The block is being initialized. Implement to have something happen. /// @@ -1447,6 +1450,17 @@ impl OnInitialize for Tuple { } } +/// A trait that will be called at genesis. +/// +/// Implementing this trait for a pallet let's you express operations that should +/// happen at genesis. It will be called in an externalities provided environment and +/// will see the genesis state after all pallets have written their genesis state. +#[impl_for_tuples(30)] +pub trait OnGenesis { + /// Something that should happen at genesis. + fn on_genesis() {} +} + /// The runtime upgrade trait. /// /// Implementing this lets you express what should happen when the runtime upgrades, @@ -1834,7 +1848,7 @@ pub const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; /// /// Each pallet version is stored in the state under a fixed key. See /// [`PALLET_VERSION_STORAGE_KEY_POSTFIX`] for how this key is built. -#[derive(RuntimeDebug, Eq, PartialEq, Encode, Decode, Ord)] +#[derive(RuntimeDebug, Eq, PartialEq, Encode, Decode, Ord, Clone, Copy)] pub struct PalletVersion { /// The major version of the pallet. pub major: u16, @@ -1872,6 +1886,25 @@ impl PalletVersion { Some(final_key) } + + /// Put this pallet version into the storage. + /// + /// It will use the storage key that is associated with the given `Pallet`. + /// + /// # Panics + /// + /// This function will panic iff `Pallet` can not be found by `PalletInfo`. + /// In a runtime that is put together using + /// [`construct_runtime!`](crate::construct_runtime) this should never happen. + /// + /// It will also panic if this function isn't executed in an externalities + /// provided environment. + pub fn put_into_storage(&self) { + let key = Self::storage_key::() + .expect("Every active pallet has a name in the runtime; qed"); + + crate::storage::unhashed::put(&key, self); + } } impl sp_std::cmp::PartialOrd for PalletVersion { diff --git a/substrate/frame/support/test/tests/pallet_version.rs b/substrate/frame/support/test/tests/pallet_version.rs index f3f4029b0d..d6293ac6a3 100644 --- a/substrate/frame/support/test/tests/pallet_version.rs +++ b/substrate/frame/support/test/tests/pallet_version.rs @@ -20,9 +20,9 @@ #![recursion_limit="128"] use codec::{Decode, Encode}; -use sp_runtime::{generic, traits::{BlakeTwo256, Block as _, Verify}}; +use sp_runtime::{generic, traits::{BlakeTwo256, Block as _, Verify}, BuildStorage}; use frame_support::{ - traits::{PALLET_VERSION_STORAGE_KEY_POSTFIX, PalletVersion, OnRuntimeUpgrade}, + traits::{PALLET_VERSION_STORAGE_KEY_POSTFIX, PalletVersion, OnRuntimeUpgrade, GetPalletVersion}, crate_to_pallet_version, weights::Weight, }; use sp_core::{H256, sr25519}; @@ -173,3 +173,18 @@ fn on_runtime_upgrade_overwrites_old_version() { check_pallet_version("Module2_2"); }); } + +#[test] +fn genesis_init_puts_pallet_version_into_storage() { + let storage = GenesisConfig::default().build_storage().expect("Builds genesis storage"); + + sp_io::TestExternalities::new(storage).execute_with(|| { + check_pallet_version("Module1"); + check_pallet_version("Module2"); + check_pallet_version("Module2_1"); + check_pallet_version("Module2_2"); + + let system_version = System::storage_version().expect("System version should be set"); + assert_eq!(System::current_version(), system_version); + }); +} diff --git a/substrate/primitives/runtime/src/lib.rs b/substrate/primitives/runtime/src/lib.rs index 47081e9115..eb20418203 100644 --- a/substrate/primitives/runtime/src/lib.rs +++ b/substrate/primitives/runtime/src/lib.rs @@ -571,116 +571,6 @@ pub fn verify_encoded_lazy( ) } -/// Helper macro for `impl_outer_config` -#[macro_export] -macro_rules! __impl_outer_config_types { - // Generic + Instance - ( - $concrete:ident $config:ident $snake:ident { $instance:ident } < $ignore:ident >; - $( $rest:tt )* - ) => { - #[cfg(any(feature = "std", test))] - pub type $config = $snake::GenesisConfig<$concrete, $snake::$instance>; - $crate::__impl_outer_config_types! { $concrete $( $rest )* } - }; - // Generic - ( - $concrete:ident $config:ident $snake:ident < $ignore:ident >; - $( $rest:tt )* - ) => { - #[cfg(any(feature = "std", test))] - pub type $config = $snake::GenesisConfig<$concrete>; - $crate::__impl_outer_config_types! { $concrete $( $rest )* } - }; - // No Generic and maybe Instance - ( - $concrete:ident $config:ident $snake:ident $( { $instance:ident } )?; - $( $rest:tt )* - ) => { - #[cfg(any(feature = "std", test))] - pub type $config = $snake::GenesisConfig; - $crate::__impl_outer_config_types! { $concrete $( $rest )* } - }; - ($concrete:ident) => () -} - -/// Implement the output "meta" module configuration struct, -/// which is basically: -/// pub struct GenesisConfig { -/// rust_module_one: Option, -/// ... -/// } -#[macro_export] -macro_rules! impl_outer_config { - ( - pub struct $main:ident for $concrete:ident { - $( $config:ident => - $snake:ident $( $instance:ident )? $( <$generic:ident> )*, )* - } - ) => { - $crate::__impl_outer_config_types! { - $concrete $( $config $snake $( { $instance } )? $( <$generic> )*; )* - } - - $crate::paste::item! { - #[cfg(any(feature = "std", test))] - #[derive($crate::serde::Serialize, $crate::serde::Deserialize)] - #[serde(rename_all = "camelCase")] - #[serde(deny_unknown_fields)] - pub struct $main { - $( - pub [< $snake $(_ $instance )? >]: Option<$config>, - )* - } - #[cfg(any(feature = "std", test))] - impl $crate::BuildStorage for $main { - fn assimilate_storage( - &self, - storage: &mut $crate::Storage, - ) -> std::result::Result<(), String> { - $( - if let Some(ref extra) = self.[< $snake $(_ $instance )? >] { - $crate::impl_outer_config! { - @CALL_FN - $concrete; - $snake; - $( $instance )?; - extra; - storage; - } - } - )* - Ok(()) - } - } - } - }; - (@CALL_FN - $runtime:ident; - $module:ident; - $instance:ident; - $extra:ident; - $storage:ident; - ) => { - $crate::BuildModuleGenesisStorage::<$runtime, $module::$instance>::build_module_genesis_storage( - $extra, - $storage, - )?; - }; - (@CALL_FN - $runtime:ident; - $module:ident; - ; - $extra:ident; - $storage:ident; - ) => { - $crate::BuildModuleGenesisStorage::<$runtime, $module::__InherentHiddenInstance>::build_module_genesis_storage( - $extra, - $storage, - )?; - } -} - /// Checks that `$x` is equal to `$y` with an error rate of `$error`. /// /// # Example