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 <alexander.popiak@parity.io>

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

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! {
+		#scrate::impl_outer_config! {
-			pub struct GenesisConfig for #runtime {
+			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 );
 	)
 }
 

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::<
+				$crate::crate_to_pallet_version!()
-						<$trait_instance as $system::Trait>::PalletInfo, Self
+					.put_into_storage::<<$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);
 
 				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::<
+				$crate::crate_to_pallet_version!()
-						<$trait_instance as $system::Trait>::PalletInfo, Self
+					.put_into_storage::<<$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);
 
 				<
 					<$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 $(<I>, $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 $(<I>, $instance: $instantiable)?> $crate::dispatch::Clone

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<ModuleOneConfig>,
+/// 	...
+/// }
+/// ```
+#[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,
+			)?;
+	}
+}

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;

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
+/// The block finalization trait.
-/// for your module when the block is ending.
+///
+/// Implementing this lets you express what should happen for your pallet when the block is ending.
 #[impl_for_tuples(30)]
 pub trait OnFinalize<BlockNumber> {
 	/// 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
+/// The block initialization trait.
-/// for your module when the block is beginning (right before the first extrinsic is executed).
+///
+/// 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<BlockNumber> {
 	/// The block is being initialized. Implement to have something happen.
 	///
@@ -1447,6 +1450,17 @@ impl<BlockNumber: Clone> OnInitialize<BlockNumber> 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<PI: PalletInfo, Pallet: 'static>(&self) {
+		let key = Self::storage_key::<PI, Pallet>()
+			.expect("Every active pallet has a name in the runtime; qed");
+
+		crate::storage::unhashed::put(&key, self);
+	}
 }
 
 impl sp_std::cmp::PartialOrd for PalletVersion {

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);
+	});
+}

substrate/primitives/runtime/src/lib.rs

@@ -571,116 +571,6 @@ pub fn verify_encoded_lazy<V: Verify, T: codec::Encode>(
 	)
 }
 
-/// 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<ModuleOneConfig>,
-/// 	...
-/// }
-#[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