From 9071b2516d5586c8db6550d6eca7b367bc122597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 21 Jun 2021 22:43:23 +0200 Subject: [PATCH 01/11] Move `PalletVersion` away from the crate version Before this pr, `PalletVersion` was referring to the crate version that hosted the pallet. This pr introduces a custom `package.metadata.frame` section in the `Cargo.toml` that can contain a `pallet-version` key value pair. While the value is expected to be a valid u16. If this key/value pair isn't given, the version is set to 1. It also changes the `PalletVersion` declaration. We now only have one `u16` that represents the version. Not a major/minor/patch version. As the old `PalletVersion` was starting with the `u16` major, decoding the old values will work. --- Cargo.lock | 1 + frame/contracts/Cargo.toml | 3 + frame/contracts/src/migration.rs | 7 +- frame/elections-phragmen/Cargo.toml | 3 + frame/elections-phragmen/src/migrations/v3.rs | 7 +- frame/elections-phragmen/src/migrations/v4.rs | 11 ++- frame/grandpa/Cargo.toml | 3 + frame/grandpa/src/migrations/v3_1.rs | 18 ++--- frame/support/procedural/Cargo.toml | 1 + frame/support/procedural/src/lib.rs | 4 +- .../procedural/src/pallet/expand/hooks.rs | 2 +- .../src/pallet/expand/pallet_struct.rs | 4 +- .../support/procedural/src/pallet_version.rs | 67 +++++++++++-------- frame/support/src/dispatch.rs | 8 +-- frame/support/src/lib.rs | 15 ++--- frame/support/src/traits/hooks.rs | 19 +++--- frame/support/src/traits/metadata.rs | 39 ++++------- frame/support/test/Cargo.toml | 3 + frame/support/test/src/pallet_version.rs | 10 +-- frame/support/test/tests/pallet_version.rs | 8 +-- 20 files changed, 111 insertions(+), 122 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a33cb02f7f0d4..2cf5e2b7f35a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1866,6 +1866,7 @@ name = "frame-support-procedural" version = "3.0.0" dependencies = [ "Inflector", + "cargo_metadata", "frame-support-procedural-tools", "proc-macro2", "quote", diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index 9d344fb6866d7..d03c611f1187a 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -77,3 +77,6 @@ try-runtime = ["frame-support/try-runtime"] # Make contract callable functions marked as __unstable__ available. Do not enable # on live chains as those are subject to change. unstable-interface = [] + +[package.metadata.frame] +pallet-version = 4 diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 8c5c06fde7ab1..c383d23995b99 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -16,16 +16,13 @@ // limitations under the License. use crate::{Config, Weight, Pallet}; -use frame_support::{ - storage::migration, - traits::{GetPalletVersion, PalletVersion, PalletInfoAccess, Get}, -}; +use frame_support::{storage::migration, traits::{GetPalletVersion, PalletInfoAccess, Get}}; pub fn migrate() -> Weight { let mut weight: Weight = 0; match >::storage_version() { - Some(version) if version == PalletVersion::new(3, 0, 0) => { + Some(version) if version == 3 => { weight = weight.saturating_add(T::DbWeight::get().writes(1)); migration::remove_storage_prefix( >::name().as_bytes(), diff --git a/frame/elections-phragmen/Cargo.toml b/frame/elections-phragmen/Cargo.toml index aa2b564f73f24..64806acd842cc 100644 --- a/frame/elections-phragmen/Cargo.toml +++ b/frame/elections-phragmen/Cargo.toml @@ -49,3 +49,6 @@ runtime-benchmarks = [ "frame-system/runtime-benchmarks", ] try-runtime = ["frame-support/try-runtime"] + +[package.metadata.frame] +pallet-version = 4 diff --git a/frame/elections-phragmen/src/migrations/v3.rs b/frame/elections-phragmen/src/migrations/v3.rs index 8afc9ed66920b..99e7c77eb970c 100644 --- a/frame/elections-phragmen/src/migrations/v3.rs +++ b/frame/elections-phragmen/src/migrations/v3.rs @@ -19,10 +19,7 @@ use codec::{Encode, Decode, FullCodec}; use sp_std::prelude::*; -use frame_support::{ - RuntimeDebug, weights::Weight, Twox64Concat, - traits::{GetPalletVersion, PalletVersion}, -}; +use frame_support::{RuntimeDebug, weights::Weight, Twox64Concat, traits::GetPalletVersion}; #[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] struct SeatHolder { @@ -83,7 +80,7 @@ pub fn apply(old_voter_bond: T::Balance, old_candidacy_bond: T::Balan maybe_storage_version, ); match maybe_storage_version { - Some(storage_version) if storage_version <= PalletVersion::new(2, 0, 0) => { + Some(storage_version) if storage_version <= 2 => { migrate_voters_to_recorded_deposit::(old_voter_bond); migrate_candidates_to_recorded_deposit::(old_candidacy_bond); migrate_runners_up_to_recorded_deposit::(old_candidacy_bond); diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index f704b203d34cf..dee5df3d879a3 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -17,10 +17,7 @@ //! Migrations to version [`4.0.0`], as denoted by the changelog. -use frame_support::{ - weights::Weight, - traits::{GetPalletVersion, PalletVersion, Get}, -}; +use frame_support::{weights::Weight, traits::{GetPalletVersion, Get}}; /// The old prefix. pub const OLD_PREFIX: &[u8] = b"PhragmenElection"; @@ -52,7 +49,7 @@ pub fn migrate< ); match maybe_storage_version { - Some(storage_version) if storage_version <= PalletVersion::new(3, 0, 0) => { + Some(storage_version) if storage_version <= 3 => { log::info!("new prefix: {}", new_pallet_name.as_ref()); frame_support::storage::migration::move_pallet( OLD_PREFIX, @@ -96,7 +93,7 @@ pub fn pre_migration>(new: N) { sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_bytes()).unwrap()) ); // ensure storage version is 3. - assert!(

::storage_version().unwrap().major == 3); + assert!(

::storage_version().unwrap() == 3); } /// Some checks for after migration. This can be linked to @@ -106,5 +103,5 @@ pub fn pre_migration>(new: N) { pub fn post_migration

() { log::info!("post-migration elections-phragmen"); // ensure we've been updated to v4 by the automatic write of crate version -> storage version. - assert!(

::storage_version().unwrap().major == 4); + assert!(

::storage_version().unwrap() == 4); } diff --git a/frame/grandpa/Cargo.toml b/frame/grandpa/Cargo.toml index 5c3cac8f82182..3350c13fb4f1d 100644 --- a/frame/grandpa/Cargo.toml +++ b/frame/grandpa/Cargo.toml @@ -60,3 +60,6 @@ std = [ ] runtime-benchmarks = ["frame-benchmarking"] try-runtime = ["frame-support/try-runtime"] + +[package.metadata.frame] +pallet-version = 4 diff --git a/frame/grandpa/src/migrations/v3_1.rs b/frame/grandpa/src/migrations/v3_1.rs index fc626578098da..3e8e24b6cbaa1 100644 --- a/frame/grandpa/src/migrations/v3_1.rs +++ b/frame/grandpa/src/migrations/v3_1.rs @@ -15,10 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{ - weights::Weight, - traits::{GetPalletVersion, PalletVersion, Get}, -}; +use frame_support::{weights::Weight, traits::{GetPalletVersion, Get, PalletVersion}}; use sp_io::hashing::twox_128; /// The old prefix. @@ -52,7 +49,7 @@ pub fn migrate< ); match maybe_storage_version { - Some(storage_version) if storage_version <= PalletVersion::new(3, 0, 0) => { + Some(storage_version) if storage_version <= 3 => { log::info!("new prefix: {}", new_pallet_name.as_ref()); frame_support::storage::migration::move_pallet( OLD_PREFIX, @@ -60,14 +57,7 @@ pub fn migrate< ); ::BlockWeights::get().max_block } - _ => { - log::warn!( - target: "runtime::afg", - "Attempted to apply migration to v3.1 but cancelled because storage version is {:?}", - maybe_storage_version, - ); - 0 - }, + _ => 0, } } @@ -108,7 +98,7 @@ pub fn pre_migration< ), ); // ensure storage version is 3. - assert!(

::storage_version().unwrap().major == 3); + assert!(

::storage_version().unwrap() == 3); } /// Some checks for after migration. This can be linked to diff --git a/frame/support/procedural/Cargo.toml b/frame/support/procedural/Cargo.toml index 4a00a24e3849d..338949adda767 100644 --- a/frame/support/procedural/Cargo.toml +++ b/frame/support/procedural/Cargo.toml @@ -20,6 +20,7 @@ proc-macro2 = "1.0.6" quote = "1.0.3" Inflector = "0.11.4" syn = { version = "1.0.58", features = ["full"] } +cargo_metadata = "0.13.1" [features] default = ["std"] diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 2768608cb6f5b..e3ac377f34eee 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -459,8 +459,8 @@ pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStre } #[proc_macro] -pub fn crate_to_pallet_version(input: TokenStream) -> TokenStream { - pallet_version::crate_to_pallet_version(input).unwrap_or_else(|e| e.to_compile_error()).into() +pub fn pallet_version(input: TokenStream) -> TokenStream { + pallet_version::pallet_version(input).unwrap_or_else(|e| e.to_compile_error()).into() } /// The number of module instances supported by the runtime, starting at index 1, diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 6e21c892d8ebb..ea101604b28e9 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -131,7 +131,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); // log info about the upgrade. - let new_storage_version = #frame_support::crate_to_pallet_version!(); + let new_storage_version = #frame_support::pallet_version!(); let pallet_name = < ::PalletInfo as diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index b655227cfc10d..4c9f6d9f2ea2a 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -153,7 +153,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn current_version() -> #frame_support::traits::PalletVersion { - #frame_support::crate_to_pallet_version!() + #frame_support::pallet_version!() } fn storage_version() -> Option<#frame_support::traits::PalletVersion> { @@ -171,7 +171,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn on_genesis() { - #frame_support::crate_to_pallet_version!() + #frame_support::pallet_version!() .put_into_storage::<::PalletInfo, Self>(); } } diff --git a/frame/support/procedural/src/pallet_version.rs b/frame/support/procedural/src/pallet_version.rs index 0f3c478d4977a..737718b1d19a0 100644 --- a/frame/support/procedural/src/pallet_version.rs +++ b/frame/support/procedural/src/pallet_version.rs @@ -19,46 +19,59 @@ use proc_macro2::{TokenStream, Span}; use syn::{Result, Error}; -use std::{env, str::FromStr}; +use std::{convert::TryInto, env, fmt::Display, path::PathBuf}; use frame_support_procedural_tools::generate_crate_access_2018; - -/// Get the version from the given version environment variable. -/// -/// The version is parsed into the requested destination type. -fn get_version(version_env: &str) -> std::result::Result { - let version = env::var(version_env) - .unwrap_or_else(|_| panic!("`{}` is always set by cargo; qed", version_env)); - - T::from_str(&version).map_err(drop) -} +use cargo_metadata::MetadataCommand; /// Create an error that will be shown by rustc at the call site of the macro. -fn create_error(message: &str) -> Error { +fn create_error(message: impl Display) -> Error { Error::new(Span::call_site(), message) } -/// Implementation of the `crate_to_pallet_version!` macro. -pub fn crate_to_pallet_version(input: proc_macro::TokenStream) -> Result { +/// Implementation of the `pallet_version!` macro. +pub fn pallet_version(input: proc_macro::TokenStream) -> Result { if !input.is_empty() { return Err(create_error("No arguments expected!")) } - let major_version = get_version::("CARGO_PKG_VERSION_MAJOR") - .map_err(|_| create_error("Major version needs to fit into `u16`"))?; + let cargo_toml = PathBuf::from( + env::var("CARGO_MANIFEST_DIR").expect("`CARGO_MANIFEST_DIR` is set by cargo"), + ).join("Cargo.toml"); - let minor_version = get_version::("CARGO_PKG_VERSION_MINOR") - .map_err(|_| create_error("Minor version needs to fit into `u8`"))?; + let metadata = MetadataCommand::new() + .manifest_path(cargo_toml) + .exec() + .expect("`cargo metadata` can not fail on project `Cargo.toml`; qed"); - let patch_version = get_version::("CARGO_PKG_VERSION_PATCH") - .map_err(|_| create_error("Patch version needs to fit into `u8`"))?; + let package = metadata.root_package().expect("We specified a manifest path; qed"); - let crate_ = generate_crate_access_2018("frame-support")?; + let version: u16 = if let Some(frame) = package.metadata.get("frame").and_then(|f| f.as_object()) { + if let Some(key) = frame.keys().find(|k| *k != "pallet-version") { + return Err(create_error(format!("Unknown key in package.metadata.frame: {}", key))) + } - Ok(quote::quote! { - #crate_::traits::PalletVersion { - major: #major_version, - minor: #minor_version, - patch: #patch_version, + if let Some(pallet_version) = frame.get("pallet-version") { + if let Some(pallet_version) = pallet_version.as_u64() { + pallet_version + .try_into() + .map_err(|_| + create_error( + "package.metadata.frame.pallet-version supports in maximum u16." + ) + )? + } else { + return Err( + create_error("package.metadata.frame.pallet-version is required to be a number"), + ) + } + } else { + 1 } - }) + } else { + 1 + }; + + let crate_ = generate_crate_access_2018("frame-support")?; + + Ok(quote::quote! { #crate_::traits::PalletVersion::new(#version) }) } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index ee290a31d5a41..7d08d142c6246 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1446,7 +1446,7 @@ macro_rules! decl_module { as $system::Config >::PalletInfo as $crate::traits::PalletInfo>::name::().unwrap_or(""); - let new_storage_version = $crate::crate_to_pallet_version!(); + let new_storage_version = $crate::pallet_version!(); $crate::log::info!( target: $crate::LOG_TARGET, @@ -1495,7 +1495,7 @@ macro_rules! decl_module { as $system::Config >::PalletInfo as $crate::traits::PalletInfo>::name::().unwrap_or(""); - let new_storage_version = $crate::crate_to_pallet_version!(); + let new_storage_version = $crate::pallet_version!(); $crate::log::info!( target: $crate::LOG_TARGET, @@ -2025,7 +2025,7 @@ macro_rules! decl_module { for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { fn current_version() -> $crate::traits::PalletVersion { - $crate::crate_to_pallet_version!() + $crate::pallet_version!() } fn storage_version() -> Option<$crate::traits::PalletVersion> { @@ -2042,7 +2042,7 @@ macro_rules! decl_module { for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { fn on_genesis() { - $crate::crate_to_pallet_version!() + $crate::pallet_version!() .put_into_storage::<<$trait_instance as $system::Config>::PalletInfo, Self>(); } } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 45988c1c7372b..84782be83f60e 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -630,20 +630,19 @@ pub use frame_support_procedural::DefaultNoBound; /// ``` pub use frame_support_procedural::require_transactional; -/// Convert the current crate version into a [`PalletVersion`](crate::traits::PalletVersion). +/// Extracts the [`PalletVersion`](crate::traits::PalletVersion) as defined in the `Cargo.toml`. /// -/// It uses the `CARGO_PKG_VERSION_MAJOR`, `CARGO_PKG_VERSION_MINOR` and -/// `CARGO_PKG_VERSION_PATCH` environment variables to fetch the crate version. -/// This means that the [`PalletVersion`](crate::traits::PalletVersion) -/// object will correspond to the version of the crate the macro is called in! +/// The pallet version can be stored under `package.metadata.frame.pallet_version` in the +/// `Cargo.toml` file. If no pallet version is specified in the `Cargo.toml`, this will return `1` +/// as pallet version. /// /// # Example /// /// ``` -/// # use frame_support::{traits::PalletVersion, crate_to_pallet_version}; -/// const Version: PalletVersion = crate_to_pallet_version!(); +/// # use frame_support::{traits::PalletVersion, pallet_version}; +/// const Version: PalletVersion = pallet_version!(); /// ``` -pub use frame_support_procedural::crate_to_pallet_version; +pub use frame_support_procedural::pallet_version; /// Return Err of the expression: `return Err($expression);`. /// diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 5f7b35a9ad25c..a8a6192179c2d 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -335,15 +335,14 @@ mod tests { #[test] fn check_pallet_version_ordering() { - let version = PalletVersion::new(1, 0, 0); - assert!(version > PalletVersion::new(0, 1, 2)); - assert!(version == PalletVersion::new(1, 0, 0)); - assert!(version < PalletVersion::new(1, 0, 1)); - assert!(version < PalletVersion::new(1, 1, 0)); - - let version = PalletVersion::new(2, 50, 50); - assert!(version < PalletVersion::new(2, 50, 51)); - assert!(version > PalletVersion::new(2, 49, 51)); - assert!(version < PalletVersion::new(3, 49, 51)); + let version = PalletVersion::new(1); + assert!(version == PalletVersion::new(1)); + assert!(version < PalletVersion::new(2)); + assert!(version < PalletVersion::new(3)); + + let version = PalletVersion::new(2); + assert!(version < PalletVersion::new(3)); + assert!(version > PalletVersion::new(1)); + assert!(version < PalletVersion::new(5)); } } diff --git a/frame/support/src/traits/metadata.rs b/frame/support/src/traits/metadata.rs index b13a0464b30c0..bfc25c90e3724 100644 --- a/frame/support/src/traits/metadata.rs +++ b/frame/support/src/traits/metadata.rs @@ -78,24 +78,13 @@ 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, Clone, Copy)] -pub struct PalletVersion { - /// The major version of the pallet. - pub major: u16, - /// The minor version of the pallet. - pub minor: u8, - /// The patch version of the pallet. - pub patch: u8, -} +#[derive(RuntimeDebug, Eq, PartialEq, Encode, Decode, Ord, Clone, Copy, PartialOrd)] +pub struct PalletVersion(u16); impl PalletVersion { /// Creates a new instance of `Self`. - pub fn new(major: u16, minor: u8, patch: u8) -> Self { - Self { - major, - minor, - patch, - } + pub const fn new(version: u16) -> Self { + Self(version) } /// Returns the storage key for a pallet version. @@ -137,17 +126,15 @@ impl PalletVersion { } } -impl sp_std::cmp::PartialOrd for PalletVersion { - fn partial_cmp(&self, other: &Self) -> Option { - let res = self.major - .cmp(&other.major) - .then_with(|| - self.minor - .cmp(&other.minor) - .then_with(|| self.patch.cmp(&other.patch) - )); - - Some(res) +impl PartialEq for PalletVersion { + fn eq(&self, other: &u16) -> bool { + self.0 == *other + } +} + +impl PartialOrd for PalletVersion { + fn partial_cmp(&self, other: &u16) -> Option { + Some(self.0.cmp(other)) } } diff --git a/frame/support/test/Cargo.toml b/frame/support/test/Cargo.toml index ce5c8ea7de1fb..990d72b4b285f 100644 --- a/frame/support/test/Cargo.toml +++ b/frame/support/test/Cargo.toml @@ -43,3 +43,6 @@ try-runtime = ["frame-support/try-runtime"] # WARNING: CI only execute pallet test with this feature, # if the feature intended to be used outside, CI and this message need to be updated. conditional-storage = [] + +[package.metadata.frame] +pallet-version = 10 diff --git a/frame/support/test/src/pallet_version.rs b/frame/support/test/src/pallet_version.rs index aaa46c3ef2c60..02528185f251e 100644 --- a/frame/support/test/src/pallet_version.rs +++ b/frame/support/test/src/pallet_version.rs @@ -15,18 +15,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{crate_to_pallet_version, traits::PalletVersion}; +use frame_support::{pallet_version, traits::PalletVersion}; #[test] fn ensure_that_current_pallet_version_is_correct() { - let expected = PalletVersion { - major: env!("CARGO_PKG_VERSION_MAJOR").parse().unwrap(), - minor: env!("CARGO_PKG_VERSION_MINOR").parse().unwrap(), - patch: env!("CARGO_PKG_VERSION_PATCH").parse().unwrap(), - }; + let expected = PalletVersion::new(10); assert_eq!( expected, - crate_to_pallet_version!(), + pallet_version!(), ) } diff --git a/frame/support/test/tests/pallet_version.rs b/frame/support/test/tests/pallet_version.rs index 5c33d45aea644..2db1dc6a2e5bf 100644 --- a/frame/support/test/tests/pallet_version.rs +++ b/frame/support/test/tests/pallet_version.rs @@ -23,12 +23,12 @@ use codec::{Decode, Encode}; use sp_runtime::{generic, traits::{BlakeTwo256, Verify}, BuildStorage}; use frame_support::{ traits::{PALLET_VERSION_STORAGE_KEY_POSTFIX, PalletVersion, OnRuntimeUpgrade, GetPalletVersion}, - crate_to_pallet_version, weights::Weight, + pallet_version, weights::Weight, }; use sp_core::{H256, sr25519}; /// A version that we will check for in the tests -const SOME_TEST_VERSION: PalletVersion = PalletVersion { major: 3000, minor: 30, patch: 13 }; +const SOME_TEST_VERSION: PalletVersion = PalletVersion::new(3000); /// Checks that `on_runtime_upgrade` sets the latest pallet version when being called without /// being provided by the user. @@ -54,7 +54,7 @@ mod module2 { origin: ::Origin, { fn on_runtime_upgrade() -> Weight { - assert_eq!(crate_to_pallet_version!(), Self::current_version()); + assert_eq!(pallet_version!(), Self::current_version()); let version_key = PalletVersion::storage_key::().unwrap(); let version_value = sp_io::storage::get(&version_key); @@ -213,7 +213,7 @@ fn check_pallet_version(pallet: &str) { let version = PalletVersion::decode(&mut &value[..]) .expect("Pallet version is encoded correctly"); - assert_eq!(crate_to_pallet_version!(), version); + assert_eq!(pallet_version!(), version); } #[test] From 731b243b8163840fb0fd05f9fa73d7bf1a430145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Jul 2021 12:14:41 +0200 Subject: [PATCH 02/11] Overhaul the entire implementation - Drop PalletVersion - Introduce StorageVersion - StorageVersion needs to be set in the crate and set for the macros - Added migration --- frame/contracts/Cargo.toml | 3 - frame/contracts/src/lib.rs | 6 +- frame/contracts/src/migration.rs | 21 +- frame/elections-phragmen/Cargo.toml | 3 - frame/elections-phragmen/src/lib.rs | 8 +- frame/elections-phragmen/src/migrations/v3.rs | 50 ++-- frame/elections-phragmen/src/migrations/v4.rs | 51 ++-- frame/grandpa/Cargo.toml | 3 - frame/grandpa/src/lib.rs | 7 +- frame/grandpa/src/migrations.rs | 4 +- .../grandpa/src/migrations/{v3_1.rs => v4.rs} | 41 ++- frame/offences/src/migration.rs | 2 +- frame/support/procedural/src/lib.rs | 6 - .../procedural/src/pallet/expand/hooks.rs | 19 +- .../src/pallet/expand/pallet_struct.rs | 28 +- .../src/pallet/parse/pallet_struct.rs | 33 ++- .../support/procedural/src/pallet_version.rs | 77 ----- frame/support/src/dispatch.rs | 52 +--- frame/support/src/lib.rs | 32 +-- frame/support/src/migrations.rs | 60 ++++ frame/support/src/traits.rs | 4 +- frame/support/src/traits/hooks.rs | 14 - frame/support/src/traits/metadata.rs | 118 +++++--- frame/support/test/Cargo.toml | 3 - frame/support/test/src/lib.rs | 3 - frame/support/test/src/pallet_version.rs | 28 -- frame/support/test/tests/pallet.rs | 63 +++- frame/support/test/tests/pallet_instance.rs | 22 +- frame/support/test/tests/pallet_version.rs | 271 ------------------ 29 files changed, 360 insertions(+), 672 deletions(-) rename frame/grandpa/src/migrations/{v3_1.rs => v4.rs} (79%) delete mode 100644 frame/support/procedural/src/pallet_version.rs create mode 100644 frame/support/src/migrations.rs delete mode 100644 frame/support/test/src/pallet_version.rs delete mode 100644 frame/support/test/tests/pallet_version.rs diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index 1ab17391a9d8c..e9f7236629ab0 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -77,6 +77,3 @@ try-runtime = ["frame-support/try-runtime"] # Make contract callable functions marked as __unstable__ available. Do not enable # on live chains as those are subject to change. unstable-interface = [] - -[package.metadata.frame] -pallet-version = 4 diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 3ac56d8980cb2..9cc4e2e29aca0 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -118,7 +118,7 @@ use sp_runtime::{ Perbill, }; use frame_support::{ - traits::{OnUnbalanced, Currency, Get, Time, Randomness}, + traits::{OnUnbalanced, Currency, Get, Time, Randomness, StorageVersion}, weights::{Weight, PostDispatchInfo, WithPostDispatchInfo}, }; use frame_system::Pallet as System; @@ -134,6 +134,9 @@ type BalanceOf = type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; +/// The current storage version. +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; @@ -239,6 +242,7 @@ pub mod pallet { } #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(PhantomData); #[pallet::hooks] diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index c383d23995b99..919fd99bfe2f3 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -16,21 +16,20 @@ // limitations under the License. use crate::{Config, Weight, Pallet}; -use frame_support::{storage::migration, traits::{GetPalletVersion, PalletInfoAccess, Get}}; +use frame_support::{storage::migration, traits::{StorageVersion, PalletInfoAccess, Get}}; pub fn migrate() -> Weight { let mut weight: Weight = 0; - match >::storage_version() { - Some(version) if version == 3 => { - weight = weight.saturating_add(T::DbWeight::get().writes(1)); - migration::remove_storage_prefix( - >::name().as_bytes(), - b"CurrentSchedule", - b"", - ); - } - _ => (), + if StorageVersion::get::>() == 3 { + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + migration::remove_storage_prefix( + >::name().as_bytes(), + b"CurrentSchedule", + b"", + ); + + StorageVersion::new(4).put::>(); } weight diff --git a/frame/elections-phragmen/Cargo.toml b/frame/elections-phragmen/Cargo.toml index 64806acd842cc..aa2b564f73f24 100644 --- a/frame/elections-phragmen/Cargo.toml +++ b/frame/elections-phragmen/Cargo.toml @@ -49,6 +49,3 @@ runtime-benchmarks = [ "frame-system/runtime-benchmarks", ] try-runtime = ["frame-support/try-runtime"] - -[package.metadata.frame] -pallet-version = 4 diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 8a1680633ef7b..5c9bb47fc0922 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -100,11 +100,11 @@ use codec::{Decode, Encode}; use frame_support::{ - dispatch::{WithPostDispatchInfo}, + dispatch::WithPostDispatchInfo, traits::{ ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, - WithdrawReasons, SortedMembers, + WithdrawReasons, SortedMembers, StorageVersion, }, weights::Weight, }; @@ -122,6 +122,9 @@ pub use weights::WeightInfo; /// All migrations. pub mod migrations; +/// The current storage version. +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + /// The maximum votes allowed per voter. pub const MAXIMUM_VOTE: usize = 16; @@ -239,6 +242,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(PhantomData); #[pallet::hooks] diff --git a/frame/elections-phragmen/src/migrations/v3.rs b/frame/elections-phragmen/src/migrations/v3.rs index 99e7c77eb970c..148956ae2f606 100644 --- a/frame/elections-phragmen/src/migrations/v3.rs +++ b/frame/elections-phragmen/src/migrations/v3.rs @@ -19,7 +19,9 @@ use codec::{Encode, Decode, FullCodec}; use sp_std::prelude::*; -use frame_support::{RuntimeDebug, weights::Weight, Twox64Concat, traits::GetPalletVersion}; +use frame_support::{ + RuntimeDebug, Twox64Concat, traits::{PalletInfo, StorageVersion}, weights::Weight, +}; #[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] struct SeatHolder { @@ -37,8 +39,11 @@ struct Voter { /// Trait to implement to give information about types used for migration pub trait V2ToV3 { - /// elections-phragmen module, used to check storage version. - type Module: GetPalletVersion; + /// The elections-phragmen pallet. + type Pallet: 'static; + + /// Information about the pallets in the runtime. + type PalletInfo: PalletInfo; /// System config account id type AccountId: 'static + FullCodec; @@ -63,7 +68,7 @@ frame_support::generate_storage_alias!( > ); -/// Apply all of the migrations from 2_0_0 to 3_0_0. +/// Apply all of the migrations from 2 to 3. /// /// ### Warning /// @@ -73,28 +78,29 @@ frame_support::generate_storage_alias!( /// Be aware that this migration is intended to be used only for the mentioned versions. Use /// with care and run at your own risk. pub fn apply(old_voter_bond: T::Balance, old_candidacy_bond: T::Balance) -> Weight { - let maybe_storage_version = ::storage_version(); + let storage_version = StorageVersion::get::(); log::info!( target: "runtime::elections-phragmen", "Running migration for elections-phragmen with storage version {:?}", - maybe_storage_version, + storage_version, ); - match maybe_storage_version { - Some(storage_version) if storage_version <= 2 => { - migrate_voters_to_recorded_deposit::(old_voter_bond); - migrate_candidates_to_recorded_deposit::(old_candidacy_bond); - migrate_runners_up_to_recorded_deposit::(old_candidacy_bond); - migrate_members_to_recorded_deposit::(old_candidacy_bond); - Weight::max_value() - } - _ => { - log::warn!( - target: "runtime::elections-phragmen", - "Attempted to apply migration to V3 but failed because storage version is {:?}", - maybe_storage_version, - ); - 0 - }, + + if storage_version <= 2 { + migrate_voters_to_recorded_deposit::(old_voter_bond); + migrate_candidates_to_recorded_deposit::(old_candidacy_bond); + migrate_runners_up_to_recorded_deposit::(old_candidacy_bond); + migrate_members_to_recorded_deposit::(old_candidacy_bond); + + StorageVersion::new(3).put::(); + + Weight::max_value() + } else { + log::warn!( + target: "runtime::elections-phragmen", + "Attempted to apply migration to V3 but failed because storage version is {:?}", + storage_version, + ); + 0 } } diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index dee5df3d879a3..6bf786a8acc90 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -17,7 +17,7 @@ //! Migrations to version [`4.0.0`], as denoted by the changelog. -use frame_support::{weights::Weight, traits::{GetPalletVersion, Get}}; +use frame_support::{weights::Weight, traits::{StorageVersion, Get}}; /// The old prefix. pub const OLD_PREFIX: &[u8] = b"PhragmenElection"; @@ -30,8 +30,7 @@ pub const OLD_PREFIX: &[u8] = b"PhragmenElection"; /// /// The old storage prefix, `PhragmenElection` is hardcoded in the migration code. pub fn migrate< - T: frame_system::Config, - P: GetPalletVersion, + T: crate::Config, N: AsRef, >(new_pallet_name: N) -> Weight { if new_pallet_name.as_ref().as_bytes() == OLD_PREFIX { @@ -41,30 +40,30 @@ pub fn migrate< ); return 0; } - let maybe_storage_version =

::storage_version(); + let storage_version = StorageVersion::get::>(); log::info!( target: "runtime::elections-phragmen", "Running migration to v4 for elections-phragmen with storage version {:?}", - maybe_storage_version, + storage_version, ); - match maybe_storage_version { - Some(storage_version) if storage_version <= 3 => { - log::info!("new prefix: {}", new_pallet_name.as_ref()); - frame_support::storage::migration::move_pallet( - OLD_PREFIX, - new_pallet_name.as_ref().as_bytes(), - ); - ::BlockWeights::get().max_block - } - _ => { - log::warn!( - target: "runtime::elections-phragmen", - "Attempted to apply migration to v4 but failed because storage version is {:?}", - maybe_storage_version, - ); - 0 - }, + if storage_version <= 3 { + log::info!("new prefix: {}", new_pallet_name.as_ref()); + frame_support::storage::migration::move_pallet( + OLD_PREFIX, + new_pallet_name.as_ref().as_bytes(), + ); + + StorageVersion::new(4).put::>(); + + ::BlockWeights::get().max_block + } else { + log::warn!( + target: "runtime::elections-phragmen", + "Attempted to apply migration to v4 but failed because storage version is {:?}", + storage_version, + ); + 0 } } @@ -72,7 +71,7 @@ pub fn migrate< /// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. /// /// Panics if anything goes wrong. -pub fn pre_migration>(new: N) { +pub fn pre_migration>(new: N) { let new = new.as_ref(); log::info!("pre-migration elections-phragmen test with new = {}", new); @@ -93,15 +92,15 @@ pub fn pre_migration>(new: N) { sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_bytes()).unwrap()) ); // ensure storage version is 3. - assert!(

::storage_version().unwrap() == 3); + assert_eq!(StorageVersion::get::>(), 3); } /// Some checks for after migration. This can be linked to /// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing. /// /// Panics if anything goes wrong. -pub fn post_migration

() { +pub fn post_migration() { log::info!("post-migration elections-phragmen"); // ensure we've been updated to v4 by the automatic write of crate version -> storage version. - assert!(

::storage_version().unwrap() == 4); + assert_eq!(StorageVersion::get::>(), 4); } diff --git a/frame/grandpa/Cargo.toml b/frame/grandpa/Cargo.toml index 3350c13fb4f1d..5c3cac8f82182 100644 --- a/frame/grandpa/Cargo.toml +++ b/frame/grandpa/Cargo.toml @@ -60,6 +60,3 @@ std = [ ] runtime-benchmarks = ["frame-benchmarking"] try-runtime = ["frame-support/try-runtime"] - -[package.metadata.frame] -pallet-version = 4 diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 2d10e3c96b14d..e3fcc7b587ba0 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -41,7 +41,8 @@ use fg_primitives::{ }; use frame_support::{ dispatch::DispatchResultWithPostInfo, - storage, traits::{OneSessionHandler, KeyOwnerProofSystem}, weights::{Pays, Weight}, + storage, traits::{OneSessionHandler, KeyOwnerProofSystem, StorageVersion}, + weights::{Pays, Weight}, }; use sp_runtime::{ generic::DigestItem, @@ -69,6 +70,9 @@ pub use equivocation::{ pub use pallet::*; +/// The current storage version. +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; @@ -77,6 +81,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::config] diff --git a/frame/grandpa/src/migrations.rs b/frame/grandpa/src/migrations.rs index b0c8578c33e07..05c24e11b3939 100644 --- a/frame/grandpa/src/migrations.rs +++ b/frame/grandpa/src/migrations.rs @@ -15,5 +15,5 @@ // See the License for the specific language governing permissions and // limitations under the License. -/// Version 3.1. -pub mod v3_1; +/// Version 4. +pub mod v4; diff --git a/frame/grandpa/src/migrations/v3_1.rs b/frame/grandpa/src/migrations/v4.rs similarity index 79% rename from frame/grandpa/src/migrations/v3_1.rs rename to frame/grandpa/src/migrations/v4.rs index 3e8e24b6cbaa1..e82428a3ced81 100644 --- a/frame/grandpa/src/migrations/v3_1.rs +++ b/frame/grandpa/src/migrations/v4.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{weights::Weight, traits::{GetPalletVersion, Get, PalletVersion}}; +use frame_support::{weights::Weight, traits::{Get, StorageVersion}}; use sp_io::hashing::twox_128; /// The old prefix. @@ -29,11 +29,9 @@ pub const OLD_PREFIX: &[u8] = b"GrandpaFinality"; /// /// The old storage prefix, `GrandpaFinality` is hardcoded in the migration code. pub fn migrate< - T: frame_system::Config, - P: GetPalletVersion, + T: crate::Config, N: AsRef, >(new_pallet_name: N) -> Weight { - if new_pallet_name.as_ref().as_bytes() == OLD_PREFIX { log::info!( target: "runtime::afg", @@ -41,23 +39,25 @@ pub fn migrate< ); return 0; } - let maybe_storage_version =

::storage_version(); + let storage_version = StorageVersion::get::>(); log::info!( target: "runtime::afg", "Running migration to v3.1 for grandpa with storage version {:?}", - maybe_storage_version, + storage_version, ); - match maybe_storage_version { - Some(storage_version) if storage_version <= 3 => { - log::info!("new prefix: {}", new_pallet_name.as_ref()); - frame_support::storage::migration::move_pallet( - OLD_PREFIX, - new_pallet_name.as_ref().as_bytes(), - ); - ::BlockWeights::get().max_block - } - _ => 0, + if storage_version <= 3 { + log::info!("new prefix: {}", new_pallet_name.as_ref()); + frame_support::storage::migration::move_pallet( + OLD_PREFIX, + new_pallet_name.as_ref().as_bytes(), + ); + + StorageVersion::new(4).put::>(); + + ::BlockWeights::get().max_block + } else { + 0 } } @@ -66,8 +66,7 @@ pub fn migrate< /// /// Panics if anything goes wrong. pub fn pre_migration< - T: frame_system::Config, - P: GetPalletVersion + 'static, + T: crate::Config, N: AsRef, >(new: N) { let new = new.as_ref(); @@ -78,7 +77,7 @@ pub fn pre_migration< assert!(next_key.starts_with(&twox_128(OLD_PREFIX))); // The pallet version is already stored using the pallet name - let storage_key = PalletVersion::storage_key::().unwrap(); + let storage_key = StorageVersion::storage_key::>().unwrap(); // ensure nothing is stored in the new prefix. assert!( @@ -98,14 +97,14 @@ pub fn pre_migration< ), ); // ensure storage version is 3. - assert!(

::storage_version().unwrap() == 3); + assert_eq!(StorageVersion::get::>(), 3); } /// Some checks for after migration. This can be linked to /// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing. /// /// Panics if anything goes wrong. -pub fn post_migration() { +pub fn post_migration() { log::info!("post-migration grandpa"); // Assert that nothing remains at the old prefix diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index ce8a125e7e1a1..57e09125fa9d2 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -86,7 +86,7 @@ mod test { // when assert_eq!( Offences::on_runtime_upgrade(), - ::DbWeight::get().reads_writes(1, 2), + ::DbWeight::get().reads_writes(1, 1), ); // then diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 59c7d70d5629b..3bfbd1ab1063c 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -22,7 +22,6 @@ mod storage; mod construct_runtime; mod pallet; -mod pallet_version; mod transactional; mod debug_no_bound; mod clone_no_bound; @@ -458,11 +457,6 @@ pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStre transactional::require_transactional(attr, input).unwrap_or_else(|e| e.to_compile_error().into()) } -#[proc_macro] -pub fn pallet_version(input: TokenStream) -> TokenStream { - pallet_version::pallet_version(input).unwrap_or_else(|e| e.to_compile_error()).into() -} - /// The number of module instances supported by the runtime, starting at index 1, /// and up to `NUMBER_OF_INSTANCE`. pub(crate) const NUMBER_OF_INSTANCE: u8 = 16; diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index ea101604b28e9..2fbe6cd98ae43 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -40,9 +40,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::log::info!( target: #frame_support::LOG_TARGET, - "⚠️ {} declares internal migrations (which *might* execute), setting storage version to {:?}", + "⚠️ {} declares internal migrations (which *might* execute)", pallet_name, - new_storage_version, ); } } else { @@ -50,9 +49,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::log::info!( target: #frame_support::LOG_TARGET, - "✅ no migration for {}, setting storage version to {:?}", + "✅ no migration for {}", pallet_name, - new_storage_version, ); } }; @@ -131,7 +129,6 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); // log info about the upgrade. - let new_storage_version = #frame_support::pallet_version!(); let pallet_name = < ::PalletInfo as @@ -139,19 +136,11 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::name::().unwrap_or(""); #log_runtime_upgrade - let result = < + < Self as #frame_support::traits::Hooks< ::BlockNumber > - >::on_runtime_upgrade(); - - new_storage_version.put_into_storage::<::PalletInfo, Self>(); - - let additional_write = < - ::DbWeight as #frame_support::traits::Get<_> - >::get().writes(1); - - result.saturating_add(additional_write) + >::on_runtime_upgrade() } #[cfg(feature = "try-runtime")] diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index cfb472a0da385..2520ff7559c54 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -18,7 +18,7 @@ use crate::pallet::{Def, expand::merge_where_clauses, parse::helper::get_doc_literals}; /// * Add derive trait on Pallet -/// * Implement GetPalletVersion on Pallet +/// * Implement GetStorageVersion on Pallet /// * Implement OnGenesis on Pallet /// * Implement ModuleErrorMetadata on Pallet /// * declare Module type alias for construct_runtime @@ -155,6 +155,12 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } ); + let storage_version = if let Some(v) = def.pallet_struct.storage_version.as_ref() { + quote::quote! { #v } + } else { + quote::quote! { #frame_support::traits::StorageVersion::default() } + }; + quote::quote_spanned!(def.pallet_struct.attr_span => #module_error_metadata @@ -165,21 +171,17 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { #[allow(dead_code)] pub type Module<#type_decl_gen> = #pallet_ident<#type_use_gen>; - // Implement `GetPalletVersion` for `Pallet` - impl<#type_impl_gen> #frame_support::traits::GetPalletVersion + // Implement `GetStorageVersion` for `Pallet` + impl<#type_impl_gen> #frame_support::traits::GetStorageVersion for #pallet_ident<#type_use_gen> #config_where_clause { - fn current_version() -> #frame_support::traits::PalletVersion { - #frame_support::pallet_version!() + fn current_storage_version() -> #frame_support::traits::StorageVersion { + #storage_version } - fn storage_version() -> Option<#frame_support::traits::PalletVersion> { - let key = #frame_support::traits::PalletVersion::storage_key::< - ::PalletInfo, Self - >().expect("Every active pallet has a name in the runtime; qed"); - - #frame_support::storage::unhashed::get(&key) + fn active_storage_version() -> #frame_support::traits::StorageVersion { + #frame_support::traits::StorageVersion::get::() } } @@ -189,8 +191,8 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn on_genesis() { - #frame_support::pallet_version!() - .put_into_storage::<::PalletInfo, Self>(); + let storage_version = #storage_version; + storage_version.put::(); } } diff --git a/frame/support/procedural/src/pallet/parse/pallet_struct.rs b/frame/support/procedural/src/pallet/parse/pallet_struct.rs index ba85da2d9e684..801907b3ed05d 100644 --- a/frame/support/procedural/src/pallet/parse/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/parse/pallet_struct.rs @@ -25,6 +25,7 @@ mod keyword { syn::custom_keyword!(Pallet); syn::custom_keyword!(generate_store); syn::custom_keyword!(generate_storage_info); + syn::custom_keyword!(storage_version); syn::custom_keyword!(Store); } @@ -43,11 +44,14 @@ pub struct PalletStructDef { /// Whether to specify the storages max encoded len when implementing `StorageInfoTrait`. /// Contains the span of the attribute. pub generate_storage_info: Option, + /// The current storage version of the pallet. + pub storage_version: Option, } /// Parse for one variant of: /// * `#[pallet::generate_store($vis trait Store)]` /// * `#[pallet::generate_storage_info]` +/// * `#[pallet::storage_version(STORAGE_VERSION)]` pub enum PalletStructAttr { GenerateStore { span: proc_macro2::Span, @@ -55,6 +59,10 @@ pub enum PalletStructAttr { keyword: keyword::Store, }, GenerateStorageInfoTrait(proc_macro2::Span), + StorageVersion { + storage_version: syn::Path, + span: proc_macro2::Span, + } } impl PalletStructAttr { @@ -62,6 +70,7 @@ impl PalletStructAttr { match self { Self::GenerateStore { span, .. } => *span, Self::GenerateStorageInfoTrait(span) => *span, + Self::StorageVersion { span, .. } => *span, } } } @@ -87,6 +96,14 @@ impl syn::parse::Parse for PalletStructAttr { } else if lookahead.peek(keyword::generate_storage_info) { let span = content.parse::()?.span(); Ok(Self::GenerateStorageInfoTrait(span)) + } else if lookahead.peek(keyword::storage_version) { + let span = content.parse::()?.span(); + + let version_content; + syn::parenthesized!(version_content in content); + let storage_version = version_content.parse::()?; + + Ok(Self::StorageVersion { storage_version, span }) } else { Err(lookahead.error()) } @@ -108,6 +125,7 @@ impl PalletStructDef { let mut store = None; let mut generate_storage_info = None; + let mut storage_version_found = None; let struct_attrs: Vec = helper::take_item_pallet_attrs(&mut item.attrs)?; for attr in struct_attrs { @@ -118,6 +136,11 @@ impl PalletStructDef { PalletStructAttr::GenerateStorageInfoTrait(span) if generate_storage_info.is_none() => { generate_storage_info = Some(span); }, + PalletStructAttr::StorageVersion { storage_version, .. } + if storage_version_found.is_none() => + { + storage_version_found = Some(storage_version); + }, attr => { let msg = "Unexpected duplicated attribute"; return Err(syn::Error::new(attr.span(), msg)); @@ -140,6 +163,14 @@ impl PalletStructDef { let mut instances = vec![]; instances.push(helper::check_type_def_gen_no_bounds(&item.generics, item.ident.span())?); - Ok(Self { index, instances, pallet, store, attr_span, generate_storage_info }) + Ok(Self { + index, + instances, + pallet, + store, + attr_span, + generate_storage_info, + storage_version: storage_version_found, + }) } } diff --git a/frame/support/procedural/src/pallet_version.rs b/frame/support/procedural/src/pallet_version.rs deleted file mode 100644 index 737718b1d19a0..0000000000000 --- a/frame/support/procedural/src/pallet_version.rs +++ /dev/null @@ -1,77 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2020-2021 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. - -//! Implementation of macros related to pallet versioning. - -use proc_macro2::{TokenStream, Span}; -use syn::{Result, Error}; -use std::{convert::TryInto, env, fmt::Display, path::PathBuf}; -use frame_support_procedural_tools::generate_crate_access_2018; -use cargo_metadata::MetadataCommand; - -/// Create an error that will be shown by rustc at the call site of the macro. -fn create_error(message: impl Display) -> Error { - Error::new(Span::call_site(), message) -} - -/// Implementation of the `pallet_version!` macro. -pub fn pallet_version(input: proc_macro::TokenStream) -> Result { - if !input.is_empty() { - return Err(create_error("No arguments expected!")) - } - - let cargo_toml = PathBuf::from( - env::var("CARGO_MANIFEST_DIR").expect("`CARGO_MANIFEST_DIR` is set by cargo"), - ).join("Cargo.toml"); - - let metadata = MetadataCommand::new() - .manifest_path(cargo_toml) - .exec() - .expect("`cargo metadata` can not fail on project `Cargo.toml`; qed"); - - let package = metadata.root_package().expect("We specified a manifest path; qed"); - - let version: u16 = if let Some(frame) = package.metadata.get("frame").and_then(|f| f.as_object()) { - if let Some(key) = frame.keys().find(|k| *k != "pallet-version") { - return Err(create_error(format!("Unknown key in package.metadata.frame: {}", key))) - } - - if let Some(pallet_version) = frame.get("pallet-version") { - if let Some(pallet_version) = pallet_version.as_u64() { - pallet_version - .try_into() - .map_err(|_| - create_error( - "package.metadata.frame.pallet-version supports in maximum u16." - ) - )? - } else { - return Err( - create_error("package.metadata.frame.pallet-version is required to be a number"), - ) - } - } else { - 1 - } - } else { - 1 - }; - - let crate_ = generate_crate_access_2018("frame-support")?; - - Ok(quote::quote! { #crate_::traits::PalletVersion::new(#version) }) -} diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 7d08d142c6246..98f7d3ce64fdc 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -29,9 +29,7 @@ pub use crate::weights::{ PaysFee, PostDispatchInfo, WithPostDispatchInfo, }; pub use sp_runtime::{traits::Dispatchable, DispatchError}; -pub use crate::traits::{ - CallMetadata, GetCallMetadata, GetCallName, UnfilteredDispatchable, GetPalletVersion, -}; +pub use crate::traits::{CallMetadata, GetCallMetadata, GetCallName, UnfilteredDispatchable}; /// The return typ of a `Dispatchable` in frame. When returned explicitly from /// a dispatchable function it allows overriding the default `PostDispatchInfo` @@ -1446,25 +1444,14 @@ macro_rules! decl_module { as $system::Config >::PalletInfo as $crate::traits::PalletInfo>::name::().unwrap_or(""); - let new_storage_version = $crate::pallet_version!(); $crate::log::info!( target: $crate::LOG_TARGET, - "⚠️ {} declares internal migrations (which *might* execute), setting storage version to {:?}", + "⚠️ {} declares internal migrations (which *might* execute)", pallet_name, - new_storage_version, ); - let result: $return = (|| { $( $impl )* })(); - - new_storage_version - .put_into_storage::<<$trait_instance as $system::Config>::PalletInfo, Self>(); - - let additional_write = < - <$trait_instance as $system::Config>::DbWeight as $crate::traits::Get<_> - >::get().writes(1); - - result.saturating_add(additional_write) + (|| { $( $impl )* })() } #[cfg(feature = "try-runtime")] @@ -1495,19 +1482,14 @@ macro_rules! decl_module { as $system::Config >::PalletInfo as $crate::traits::PalletInfo>::name::().unwrap_or(""); - let new_storage_version = $crate::pallet_version!(); $crate::log::info!( target: $crate::LOG_TARGET, - "✅ no migration for {}, setting storage version to {:?}", + "✅ no migration for {}", pallet_name, - new_storage_version, ); - new_storage_version - .put_into_storage::<<$trait_instance as $system::Config>::PalletInfo, Self>(); - - <<$trait_instance as $system::Config>::DbWeight as $crate::traits::Get<_>>::get().writes(1) + 0 } #[cfg(feature = "try-runtime")] @@ -2018,33 +2000,11 @@ macro_rules! decl_module { } } - // Bring `GetPalletVersion` into scope to make it easily usable. - pub use $crate::traits::GetPalletVersion as _; - // Implement `GetPalletVersion` for `Module` - impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::GetPalletVersion - for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* - { - fn current_version() -> $crate::traits::PalletVersion { - $crate::pallet_version!() - } - - fn storage_version() -> Option<$crate::traits::PalletVersion> { - let key = $crate::traits::PalletVersion::storage_key::< - <$trait_instance as $system::Config>::PalletInfo, Self - >().expect("Every active pallet has a name in the runtime; qed"); - - $crate::storage::unhashed::get(&key) - } - } - // 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::pallet_version!() - .put_into_storage::<<$trait_instance as $system::Config>::PalletInfo, Self>(); - } + fn on_genesis() {} } // manual implementation of clone/eq/partialeq because using derive erroneously requires diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 0208e6373b722..4d8a8cc039c0e 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -68,6 +68,7 @@ pub mod error; pub mod traits; pub mod weights; pub mod instances; +pub mod migrations; pub use self::hash::{ Twox256, Twox128, Blake2_256, Blake2_128, Identity, Twox64Concat, Blake2_128Concat, Hashable, @@ -659,20 +660,6 @@ pub use frame_support_procedural::DefaultNoBound; /// ``` pub use frame_support_procedural::require_transactional; -/// Extracts the [`PalletVersion`](crate::traits::PalletVersion) as defined in the `Cargo.toml`. -/// -/// The pallet version can be stored under `package.metadata.frame.pallet_version` in the -/// `Cargo.toml` file. If no pallet version is specified in the `Cargo.toml`, this will return `1` -/// as pallet version. -/// -/// # Example -/// -/// ``` -/// # use frame_support::{traits::PalletVersion, pallet_version}; -/// const Version: PalletVersion = pallet_version!(); -/// ``` -pub use frame_support_procedural::pallet_version; - /// Return Err of the expression: `return Err($expression);`. /// /// Used as `fail!(expression)`. @@ -1272,7 +1259,7 @@ pub mod pallet_prelude { Twox128, Blake2_256, Blake2_128, Identity, Twox64Concat, Blake2_128Concat, ensure, RuntimeDebug, storage, traits::{ - Get, Hooks, IsType, GetPalletVersion, EnsureOrigin, PalletInfoAccess, StorageInfoTrait, + Get, Hooks, IsType, EnsureOrigin, PalletInfoAccess, StorageInfoTrait, ConstU32, GetDefault, }, dispatch::{DispatchResultWithPostInfo, Parameter, DispatchError, DispatchResult}, @@ -1397,6 +1384,19 @@ pub mod pallet_prelude { /// This require all storage to implement the trait [`traits::StorageInfoTrait`], thus all keys /// and value types must bound [`traits::MaxEncodedLen`]. /// +/// As the macro implements [`traits::GetStorageVersion`], the current storage version needs to be +/// communicated to the macro. This can be done by using the `storage_version` attribute: +/// +/// ```ignore +/// const STORAGE_VERSION: StorageVersion = StorageVersion::new(5); +/// +/// #[pallet::pallet] +/// #[pallet::storage_version(STORAGE_VERSION)] +/// pub struct Pallet(_); +/// ``` +/// +/// If not present, the current storage version is set to the default value. +/// /// ### Macro expansion: /// /// The macro add this attribute to the struct definition: @@ -1411,7 +1411,7 @@ pub mod pallet_prelude { /// and replace the type `_` by `PhantomData`. /// /// It implements on pallet: -/// * [`traits::GetPalletVersion`] +/// * [`traits::GetStorageVersion`] /// * [`traits::OnGenesis`]: contains some logic to write pallet version into storage. /// * `ModuleErrorMetadata`: using error declared or no metadata. /// diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs new file mode 100644 index 0000000000000..cd5b3c00d4766 --- /dev/null +++ b/frame/support/src/migrations.rs @@ -0,0 +1,60 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +use crate::traits::{GetStorageVersion, PalletInfoAccess}; + +/// Trait used by [`migrate_from_pallet_version_to_storage_version`] to do the actual migration. +pub trait PalletVersionToStorageVersionHelper { + fn migrate(); +} + +impl PalletVersionToStorageVersionHelper for T { + fn migrate() { + const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; + + fn pallet_version_key(name: &str) -> [u8; 32] { + let pallet_name = sp_io::hashing::twox_128(name.as_bytes()); + let postfix = sp_io::hashing::twox_128(PALLET_VERSION_STORAGE_KEY_POSTFIX); + + let mut final_key = [0u8; 32]; + final_key[..16].copy_from_slice(&pallet_name); + final_key[16..].copy_from_slice(&postfix); + + final_key + } + + sp_io::storage::clear(&pallet_version_key(::name())); + + let version = ::current_storage_version(); + version.put::(); + } +} + +#[impl_trait_for_tuples::impl_for_tuples(30)] +impl PalletVersionToStorageVersionHelper for T { + fn migrate() { + for_tuples!( #( T::migrate(); )* ) + } +} + +/// Migrate from the `PalletVersion` struct to the new +/// [`StorageVersion`](crate::traits::StorageVersion) struct. +/// +/// This will remove all `PalletVersion's` from the state and insert the current storage version. +pub fn migrate_from_pallet_version_to_storage_version() { + AllPallets::migrate() +} diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index e8ce07528c8a0..864df98a20329 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -61,8 +61,8 @@ pub use randomness::Randomness; mod metadata; pub use metadata::{ - CallMetadata, GetCallMetadata, GetCallName, PalletInfo, PalletVersion, GetPalletVersion, - PALLET_VERSION_STORAGE_KEY_POSTFIX, PalletInfoAccess, + CallMetadata, GetCallMetadata, GetCallName, PalletInfo, StorageVersion, + STORAGE_VERSION_STORAGE_KEY_POSTFIX, PalletInfoAccess, GetStorageVersion, }; mod hooks; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index a8a6192179c2d..f2166935c7fd7 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -313,7 +313,6 @@ pub trait OnTimestampSet { #[cfg(test)] mod tests { use super::*; - use crate::traits::metadata::PalletVersion; #[test] fn on_initialize_and_on_runtime_upgrade_weight_merge_works() { @@ -332,17 +331,4 @@ mod tests { assert_eq!(<(Test, Test)>::on_initialize(0), 20); assert_eq!(<(Test, Test)>::on_runtime_upgrade(), 40); } - - #[test] - fn check_pallet_version_ordering() { - let version = PalletVersion::new(1); - assert!(version == PalletVersion::new(1)); - assert!(version < PalletVersion::new(2)); - assert!(version < PalletVersion::new(3)); - - let version = PalletVersion::new(2); - assert!(version < PalletVersion::new(3)); - assert!(version > PalletVersion::new(1)); - assert!(version < PalletVersion::new(5)); - } } diff --git a/frame/support/src/traits/metadata.rs b/frame/support/src/traits/metadata.rs index bfc25c90e3724..9c0f3cf8cb839 100644 --- a/frame/support/src/traits/metadata.rs +++ b/frame/support/src/traits/metadata.rs @@ -68,45 +68,42 @@ pub trait GetCallMetadata { fn get_call_metadata(&self) -> CallMetadata; } -/// The storage key postfix that is used to store the [`PalletVersion`] per pallet. +/// The storage key postfix that is used to store the [`StorageVersion`] per pallet. /// /// The full storage key is built by using: -/// Twox128([`PalletInfo::name`]) ++ Twox128([`PALLET_VERSION_STORAGE_KEY_POSTFIX`]) -pub const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; +/// Twox128([`PalletInfo::name`]) ++ Twox128([`STORAGE_VERSION_STORAGE_KEY_POSTFIX`]) +pub const STORAGE_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__STORAGE_VERSION__:"; -/// The version of a pallet. +/// The storage version of a pallet. /// -/// 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, Clone, Copy, PartialOrd)] -pub struct PalletVersion(u16); +/// Each storage version of a pallet is stored in the state under a fixed key. See +/// [`STORAGE_VERSION_STORAGE_KEY_POSTFIX`] for how this key is built. +#[derive(RuntimeDebug, Eq, PartialEq, Encode, Decode, Ord, Clone, Copy, PartialOrd, Default)] +pub struct StorageVersion(u16); -impl PalletVersion { +impl StorageVersion { /// Creates a new instance of `Self`. pub const fn new(version: u16) -> Self { Self(version) } - /// Returns the storage key for a pallet version. + /// Returns the storage key for a storage version. /// - /// See [`PALLET_VERSION_STORAGE_KEY_POSTFIX`] on how this key is built. - /// - /// Returns `None` if the given `PI` returned a `None` as name for the given - /// `Pallet`. - pub fn storage_key() -> Option<[u8; 32]> { - let pallet_name = PI::name::()?; + /// See [`STORAGE_VERSION_STORAGE_KEY_POSTFIX`] on how this key is built. + pub fn storage_key() -> [u8; 32] { + let pallet_name = P::name(); let pallet_name = sp_io::hashing::twox_128(pallet_name.as_bytes()); - let postfix = sp_io::hashing::twox_128(PALLET_VERSION_STORAGE_KEY_POSTFIX); + let postfix = sp_io::hashing::twox_128(STORAGE_VERSION_STORAGE_KEY_POSTFIX); let mut final_key = [0u8; 32]; final_key[..16].copy_from_slice(&pallet_name); final_key[16..].copy_from_slice(&postfix); - Some(final_key) + final_key } - /// Put this pallet version into the storage. + /// Put this storage version for the given pallet into the storage. /// /// It will use the storage key that is associated with the given `Pallet`. /// @@ -118,48 +115,75 @@ impl PalletVersion { /// /// 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"); + pub fn put(&self) { + let key = Self::storage_key::

(); crate::storage::unhashed::put(&key, self); } + + /// Get the storage version of the given pallet from 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 get() -> Self { + let key = Self::storage_key::

(); + + crate::storage::unhashed::get_or_default(&key) + } } -impl PartialEq for PalletVersion { +impl PartialEq for StorageVersion { fn eq(&self, other: &u16) -> bool { self.0 == *other } } -impl PartialOrd for PalletVersion { +impl PartialOrd for StorageVersion { fn partial_cmp(&self, other: &u16) -> Option { Some(self.0.cmp(other)) } } -/// Provides version information about a pallet. +/// Provides information about the storage version of a pallet. /// -/// This trait provides two functions for returning the version of a -/// pallet. There is a state where both functions can return distinct versions. -/// See [`GetPalletVersion::storage_version`] for more information about this. -pub trait GetPalletVersion { - /// Returns the current version of the pallet. - fn current_version() -> PalletVersion; - - /// Returns the version of the pallet that is stored in storage. - /// - /// Most of the time this will return the exact same version as - /// [`GetPalletVersion::current_version`]. Only when being in - /// a state after a runtime upgrade happened and the pallet did - /// not yet updated its version in storage, this will return a - /// different(the previous, seen from the time of calling) version. - /// - /// See [`PalletVersion`] for more information. - /// - /// # Note - /// - /// If there was no previous version of the pallet stored in the state, - /// this function returns `None`. - fn storage_version() -> Option; +/// It differentiates between current and active storage version. Both should be only out of sync +/// when a new runtime upgrade was applied and the runtime migrations did not yet executed. +/// Otherwise it means that the pallet works with an unsupported storage version and unforeseen +/// stuff can happen. +/// +/// The current storage version is the version of the pallet as supported at runtime. The active +/// storage version is the version of the pallet in the storage. +/// +/// It is required to update the active storage version manually when a migration was applied. +pub trait GetStorageVersion { + /// Returns the current storage version as supported by the pallet. + fn current_storage_version() -> StorageVersion; + /// Returns the active storage version of the pallet as stored in the storage. + fn active_storage_version() -> StorageVersion; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn check_storage_version_ordering() { + let version = StorageVersion::new(1); + assert!(version == StorageVersion::new(1)); + assert!(version < StorageVersion::new(2)); + assert!(version < StorageVersion::new(3)); + + let version = StorageVersion::new(2); + assert!(version < StorageVersion::new(3)); + assert!(version > StorageVersion::new(1)); + assert!(version < StorageVersion::new(5)); + } } diff --git a/frame/support/test/Cargo.toml b/frame/support/test/Cargo.toml index 990d72b4b285f..ce5c8ea7de1fb 100644 --- a/frame/support/test/Cargo.toml +++ b/frame/support/test/Cargo.toml @@ -43,6 +43,3 @@ try-runtime = ["frame-support/try-runtime"] # WARNING: CI only execute pallet test with this feature, # if the feature intended to be used outside, CI and this message need to be updated. conditional-storage = [] - -[package.metadata.frame] -pallet-version = 10 diff --git a/frame/support/test/src/lib.rs b/frame/support/test/src/lib.rs index d40031c149d90..ffda500f96ad6 100644 --- a/frame/support/test/src/lib.rs +++ b/frame/support/test/src/lib.rs @@ -22,9 +22,6 @@ #![warn(missing_docs)] #![deny(warnings)] -#[cfg(test)] -mod pallet_version; - /// The configuration trait pub trait Config: 'static { /// The runtime origin type. diff --git a/frame/support/test/src/pallet_version.rs b/frame/support/test/src/pallet_version.rs deleted file mode 100644 index 02528185f251e..0000000000000 --- a/frame/support/test/src/pallet_version.rs +++ /dev/null @@ -1,28 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2020-2021 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. - -use frame_support::{pallet_version, traits::PalletVersion}; - -#[test] -fn ensure_that_current_pallet_version_is_correct() { - let expected = PalletVersion::new(10); - - assert_eq!( - expected, - pallet_version!(), - ) -} diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index f204de69b84bf..8f233c40284a9 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -16,12 +16,12 @@ // limitations under the License. use frame_support::{ - weights::{DispatchInfo, DispatchClass, Pays, GetDispatchInfo}, + dispatch::{UnfilteredDispatchable, Parameter}, storage::unhashed, traits::{ - GetCallName, OnInitialize, OnFinalize, OnRuntimeUpgrade, GetPalletVersion, OnGenesis, + GetCallName, GetStorageVersion, OnFinalize, OnGenesis, OnInitialize, OnRuntimeUpgrade, + StorageVersion, PalletInfoAccess }, - dispatch::{UnfilteredDispatchable, Parameter}, - storage::unhashed, + weights::{DispatchInfo, DispatchClass, Pays, GetDispatchInfo} }; use sp_runtime::DispatchError; use sp_io::{TestExternalities, hashing::{twox_64, twox_128, blake2_128}}; @@ -57,13 +57,15 @@ impl SomeAssociation2 for u64 { type _2 = u64; } pub mod pallet { use super::{ SomeType1, SomeType2, SomeType3, SomeType4, SomeType5, SomeType6, SomeType7, - SomeAssociation1, SomeAssociation2, + SomeAssociation1, SomeAssociation2, StorageVersion, }; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; type BalanceOf = ::Balance; + pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(10); + #[pallet::config] pub trait Config: frame_system::Config where ::AccountId: From + SomeAssociation1, @@ -101,6 +103,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(crate) trait Store)] #[pallet::generate_storage_info] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::hooks] @@ -814,12 +817,7 @@ fn pallet_hooks_expand() { assert_eq!(AllPallets::on_initialize(1), 10); AllPallets::on_finalize(1); - assert_eq!(pallet::Pallet::::storage_version(), None); assert_eq!(AllPallets::on_runtime_upgrade(), 30); - assert_eq!( - pallet::Pallet::::storage_version(), - Some(pallet::Pallet::::current_version()), - ); assert_eq!( frame_system::Pallet::::events()[0].event, @@ -839,15 +837,54 @@ fn pallet_hooks_expand() { #[test] fn pallet_on_genesis() { TestExternalities::default().execute_with(|| { - assert_eq!(pallet::Pallet::::storage_version(), None); + assert_eq!(pallet::Pallet::::active_storage_version(), StorageVersion::new(0)); pallet::Pallet::::on_genesis(); assert_eq!( - pallet::Pallet::::storage_version(), - Some(pallet::Pallet::::current_version()), + pallet::Pallet::::current_storage_version(), + pallet::Pallet::::active_storage_version(), ); }) } +#[test] +fn migrate_from_pallet_version_to_storage_version() { + const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; + + fn pallet_version_key(name: &str) -> [u8; 32] { + let pallet_name = sp_io::hashing::twox_128(name.as_bytes()); + let postfix = sp_io::hashing::twox_128(PALLET_VERSION_STORAGE_KEY_POSTFIX); + + let mut final_key = [0u8; 32]; + final_key[..16].copy_from_slice(&pallet_name); + final_key[16..].copy_from_slice(&postfix); + + final_key + } + + TestExternalities::default().execute_with(|| { + // Insert some fake pallet versions + sp_io::storage::set(&pallet_version_key(Example::name()), &[1, 2, 3]); + sp_io::storage::set(&pallet_version_key(Example2::name()), &[1, 2, 3]); + sp_io::storage::set(&pallet_version_key(System::name()), &[1, 2, 3]); + + // Check that everyone currently is at version 0 + assert_eq!(Example::active_storage_version(), StorageVersion::new(0)); + assert_eq!(Example2::active_storage_version(), StorageVersion::new(0)); + assert_eq!(System::active_storage_version(), StorageVersion::new(0)); + + frame_support::migrations::migrate_from_pallet_version_to_storage_version::(); + + // All pallet versions should be removed + assert!(sp_io::storage::get(&pallet_version_key(Example::name())).is_none()); + assert!(sp_io::storage::get(&pallet_version_key(Example2::name())).is_none()); + assert!(sp_io::storage::get(&pallet_version_key(System::name())).is_none()); + + assert_eq!(Example::active_storage_version(), pallet::STORAGE_VERSION); + assert_eq!(Example2::active_storage_version(), StorageVersion::new(0)); + assert_eq!(System::active_storage_version(), StorageVersion::new(0)); + }); +} + #[test] fn metadata() { use frame_metadata::*; diff --git a/frame/support/test/tests/pallet_instance.rs b/frame/support/test/tests/pallet_instance.rs index ccac97100a4be..a010c0f9134c1 100644 --- a/frame/support/test/tests/pallet_instance.rs +++ b/frame/support/test/tests/pallet_instance.rs @@ -18,7 +18,7 @@ use frame_support::{ weights::{DispatchInfo, DispatchClass, Pays, GetDispatchInfo}, traits::{ - GetCallName, GetPalletVersion, OnInitialize, OnFinalize, OnRuntimeUpgrade, OnGenesis, + GetCallName, OnInitialize, OnFinalize, OnRuntimeUpgrade, OnGenesis, }, dispatch::UnfilteredDispatchable, storage::unhashed, @@ -524,17 +524,7 @@ fn pallet_hooks_expand() { assert_eq!(AllPallets::on_initialize(1), 21); AllPallets::on_finalize(1); - assert_eq!(pallet::Pallet::::storage_version(), None); - assert_eq!(pallet::Pallet::::storage_version(), None); assert_eq!(AllPallets::on_runtime_upgrade(), 61); - assert_eq!( - pallet::Pallet::::storage_version(), - Some(pallet::Pallet::::current_version()), - ); - assert_eq!( - pallet::Pallet::::storage_version(), - Some(pallet::Pallet::::current_version()), - ); // The order is indeed reversed due to https://github.com/paritytech/substrate/issues/6280 assert_eq!( @@ -567,19 +557,9 @@ fn pallet_hooks_expand() { #[test] fn pallet_on_genesis() { TestExternalities::default().execute_with(|| { - assert_eq!(pallet::Pallet::::storage_version(), None); pallet::Pallet::::on_genesis(); - assert_eq!( - pallet::Pallet::::storage_version(), - Some(pallet::Pallet::::current_version()), - ); - assert_eq!(pallet::Pallet::::storage_version(), None); pallet::Pallet::::on_genesis(); - assert_eq!( - pallet::Pallet::::storage_version(), - Some(pallet::Pallet::::current_version()), - ); }) } diff --git a/frame/support/test/tests/pallet_version.rs b/frame/support/test/tests/pallet_version.rs deleted file mode 100644 index 2db1dc6a2e5bf..0000000000000 --- a/frame/support/test/tests/pallet_version.rs +++ /dev/null @@ -1,271 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2020-2021 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. - -//! Tests related to the pallet version. - -#![recursion_limit="128"] - -use codec::{Decode, Encode}; -use sp_runtime::{generic, traits::{BlakeTwo256, Verify}, BuildStorage}; -use frame_support::{ - traits::{PALLET_VERSION_STORAGE_KEY_POSTFIX, PalletVersion, OnRuntimeUpgrade, GetPalletVersion}, - pallet_version, weights::Weight, -}; -use sp_core::{H256, sr25519}; - -/// A version that we will check for in the tests -const SOME_TEST_VERSION: PalletVersion = PalletVersion::new(3000); - -/// Checks that `on_runtime_upgrade` sets the latest pallet version when being called without -/// being provided by the user. -mod module1 { - pub trait Config: frame_system::Config {} - - frame_support::decl_module! { - pub struct Module for enum Call where - origin: ::Origin, - {} - } -} - -/// Checks that `on_runtime_upgrade` sets the latest pallet version when being called and also -/// being provided by the user. -mod module2 { - use super::*; - - pub trait Config: frame_system::Config {} - - frame_support::decl_module! { - pub struct Module, I: Instance=DefaultInstance> for enum Call where - origin: ::Origin, - { - fn on_runtime_upgrade() -> Weight { - assert_eq!(pallet_version!(), Self::current_version()); - - let version_key = PalletVersion::storage_key::().unwrap(); - let version_value = sp_io::storage::get(&version_key); - - if version_value.is_some() { - assert_eq!(SOME_TEST_VERSION, Self::storage_version().unwrap()); - } else { - // As the storage version does not exist yet, it should be `None`. - assert!(Self::storage_version().is_none()); - } - - 0 - } - } - } - - frame_support::decl_storage! { - trait Store for Module, I: Instance=DefaultInstance> as Module2 {} - } -} - -#[frame_support::pallet] -mod pallet3 { - use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::*; - - #[pallet::config] - pub trait Config: frame_system::Config { - } - - #[pallet::pallet] - pub struct Pallet(_); - - #[pallet::hooks] - impl Hooks> for Pallet { - fn on_runtime_upgrade() -> Weight { - return 3; - } - } - - #[pallet::call] - impl Pallet { - } -} - -#[frame_support::pallet] -mod pallet4 { - use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::*; - - #[pallet::config] - pub trait Config: frame_system::Config { - } - - #[pallet::pallet] - pub struct Pallet(PhantomData<(T, I)>); - - #[pallet::hooks] - impl, I: 'static> Hooks> for Pallet { - fn on_runtime_upgrade() -> Weight { - return 3; - } - } - - #[pallet::call] - impl, I: 'static> Pallet { - } -} - -impl module1::Config for Runtime {} -impl module2::Config for Runtime {} -impl module2::Config for Runtime {} -impl module2::Config for Runtime {} - -impl pallet3::Config for Runtime {} -impl pallet4::Config for Runtime {} -impl pallet4::Config for Runtime {} -impl pallet4::Config for Runtime {} - -pub type Signature = sr25519::Signature; -pub type AccountId = ::Signer; -pub type BlockNumber = u64; -pub type Index = u64; - -frame_support::parameter_types!( - pub const BlockHashCount: u32 = 250; -); - -impl frame_system::Config for Runtime { - type BaseCallFilter = (); - type Origin = Origin; - type Index = u64; - type BlockNumber = BlockNumber; - type Call = Call; - type Hash = H256; - type Hashing = sp_runtime::traits::BlakeTwo256; - type AccountId = AccountId; - type Lookup = sp_runtime::traits::IdentityLookup; - type Header = Header; - type Event = Event; - type BlockHashCount = BlockHashCount; - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = (); - type OnSetCode = (); -} - -frame_support::construct_runtime!( - pub enum Runtime where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - System: frame_system::{Pallet, Call, Event}, - Module1: module1::{Pallet, Call}, - Module2: module2::{Pallet, Call}, - Module2_1: module2::::{Pallet, Call}, - Module2_2: module2::::{Pallet, Call}, - Pallet3: pallet3::{Pallet, Call}, - Pallet4: pallet4::{Pallet, Call}, - Pallet4_1: pallet4::::{Pallet, Call}, - Pallet4_2: pallet4::::{Pallet, Call}, - } -); - -pub type Header = generic::Header; -pub type Block = generic::Block; -pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; - -/// Returns the storage key for `PalletVersion` for the given `pallet`. -fn get_pallet_version_storage_key_for_pallet(pallet: &str) -> [u8; 32] { - let pallet_name = sp_io::hashing::twox_128(pallet.as_bytes()); - let postfix = sp_io::hashing::twox_128(PALLET_VERSION_STORAGE_KEY_POSTFIX); - - let mut final_key = [0u8; 32]; - final_key[..16].copy_from_slice(&pallet_name); - final_key[16..].copy_from_slice(&postfix); - - final_key -} - -/// Checks the version of the given `pallet`. -/// -/// It is expected that the pallet version can be found in the storage and equals the -/// current crate version. -fn check_pallet_version(pallet: &str) { - let key = get_pallet_version_storage_key_for_pallet(pallet); - let value = sp_io::storage::get(&key).expect("Pallet version exists"); - let version = PalletVersion::decode(&mut &value[..]) - .expect("Pallet version is encoded correctly"); - - assert_eq!(pallet_version!(), version); -} - -#[test] -fn on_runtime_upgrade_sets_the_pallet_versions_in_storage() { - sp_io::TestExternalities::new_empty().execute_with(|| { - AllPallets::on_runtime_upgrade(); - - check_pallet_version("Module1"); - check_pallet_version("Module2"); - check_pallet_version("Module2_1"); - check_pallet_version("Module2_2"); - check_pallet_version("Pallet3"); - check_pallet_version("Pallet4"); - check_pallet_version("Pallet4_1"); - check_pallet_version("Pallet4_2"); - }); -} - -#[test] -fn on_runtime_upgrade_overwrites_old_version() { - sp_io::TestExternalities::new_empty().execute_with(|| { - let key = get_pallet_version_storage_key_for_pallet("Module2"); - sp_io::storage::set(&key, &SOME_TEST_VERSION.encode()); - - AllPallets::on_runtime_upgrade(); - - check_pallet_version("Module1"); - check_pallet_version("Module2"); - check_pallet_version("Module2_1"); - check_pallet_version("Module2_2"); - check_pallet_version("Pallet3"); - check_pallet_version("Pallet4"); - check_pallet_version("Pallet4_1"); - check_pallet_version("Pallet4_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"); - check_pallet_version("Pallet3"); - check_pallet_version("Pallet4"); - check_pallet_version("Pallet4_1"); - check_pallet_version("Pallet4_2"); - - let system_version = System::storage_version().expect("System version should be set"); - assert_eq!(System::current_version(), system_version); - }); -} From 84db252f93c4a2e8120b161deb14a212f313c2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Jul 2021 12:23:47 +0200 Subject: [PATCH 03/11] Fix migrations --- frame/contracts/src/migration.rs | 4 ++-- frame/elections-phragmen/src/migrations/v3.rs | 11 ++++------- frame/elections-phragmen/src/migrations/v4.rs | 8 ++++---- frame/grandpa/src/migrations/v4.rs | 8 ++++---- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 919fd99bfe2f3..407b7628f0779 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -21,7 +21,7 @@ use frame_support::{storage::migration, traits::{StorageVersion, PalletInfoAcces pub fn migrate() -> Weight { let mut weight: Weight = 0; - if StorageVersion::get::>() == 3 { + if StorageVersion::get::>() == 3 { weight = weight.saturating_add(T::DbWeight::get().writes(1)); migration::remove_storage_prefix( >::name().as_bytes(), @@ -29,7 +29,7 @@ pub fn migrate() -> Weight { b"", ); - StorageVersion::new(4).put::>(); + StorageVersion::new(4).put::>(); } weight diff --git a/frame/elections-phragmen/src/migrations/v3.rs b/frame/elections-phragmen/src/migrations/v3.rs index 148956ae2f606..ca72acc6c4b8d 100644 --- a/frame/elections-phragmen/src/migrations/v3.rs +++ b/frame/elections-phragmen/src/migrations/v3.rs @@ -20,7 +20,7 @@ use codec::{Encode, Decode, FullCodec}; use sp_std::prelude::*; use frame_support::{ - RuntimeDebug, Twox64Concat, traits::{PalletInfo, StorageVersion}, weights::Weight, + RuntimeDebug, Twox64Concat, traits::{PalletInfoAccess, StorageVersion}, weights::Weight, }; #[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] @@ -40,10 +40,7 @@ struct Voter { /// Trait to implement to give information about types used for migration pub trait V2ToV3 { /// The elections-phragmen pallet. - type Pallet: 'static; - - /// Information about the pallets in the runtime. - type PalletInfo: PalletInfo; + type Pallet: 'static + PalletInfoAccess; /// System config account id type AccountId: 'static + FullCodec; @@ -78,7 +75,7 @@ frame_support::generate_storage_alias!( /// Be aware that this migration is intended to be used only for the mentioned versions. Use /// with care and run at your own risk. pub fn apply(old_voter_bond: T::Balance, old_candidacy_bond: T::Balance) -> Weight { - let storage_version = StorageVersion::get::(); + let storage_version = StorageVersion::get::(); log::info!( target: "runtime::elections-phragmen", "Running migration for elections-phragmen with storage version {:?}", @@ -91,7 +88,7 @@ pub fn apply(old_voter_bond: T::Balance, old_candidacy_bond: T::Balan migrate_runners_up_to_recorded_deposit::(old_candidacy_bond); migrate_members_to_recorded_deposit::(old_candidacy_bond); - StorageVersion::new(3).put::(); + StorageVersion::new(3).put::(); Weight::max_value() } else { diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 6bf786a8acc90..7ef081128b50f 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -40,7 +40,7 @@ pub fn migrate< ); return 0; } - let storage_version = StorageVersion::get::>(); + let storage_version = StorageVersion::get::>(); log::info!( target: "runtime::elections-phragmen", "Running migration to v4 for elections-phragmen with storage version {:?}", @@ -54,7 +54,7 @@ pub fn migrate< new_pallet_name.as_ref().as_bytes(), ); - StorageVersion::new(4).put::>(); + StorageVersion::new(4).put::>(); ::BlockWeights::get().max_block } else { @@ -92,7 +92,7 @@ pub fn pre_migration>(new: N) { sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_bytes()).unwrap()) ); // ensure storage version is 3. - assert_eq!(StorageVersion::get::>(), 3); + assert_eq!(StorageVersion::get::>(), 3); } /// Some checks for after migration. This can be linked to @@ -102,5 +102,5 @@ pub fn pre_migration>(new: N) { pub fn post_migration() { log::info!("post-migration elections-phragmen"); // ensure we've been updated to v4 by the automatic write of crate version -> storage version. - assert_eq!(StorageVersion::get::>(), 4); + assert_eq!(StorageVersion::get::>(), 4); } diff --git a/frame/grandpa/src/migrations/v4.rs b/frame/grandpa/src/migrations/v4.rs index e82428a3ced81..d3268f321c765 100644 --- a/frame/grandpa/src/migrations/v4.rs +++ b/frame/grandpa/src/migrations/v4.rs @@ -39,7 +39,7 @@ pub fn migrate< ); return 0; } - let storage_version = StorageVersion::get::>(); + let storage_version = StorageVersion::get::>(); log::info!( target: "runtime::afg", "Running migration to v3.1 for grandpa with storage version {:?}", @@ -53,7 +53,7 @@ pub fn migrate< new_pallet_name.as_ref().as_bytes(), ); - StorageVersion::new(4).put::>(); + StorageVersion::new(4).put::>(); ::BlockWeights::get().max_block } else { @@ -77,7 +77,7 @@ pub fn pre_migration< assert!(next_key.starts_with(&twox_128(OLD_PREFIX))); // The pallet version is already stored using the pallet name - let storage_key = StorageVersion::storage_key::>().unwrap(); + let storage_key = StorageVersion::storage_key::>(); // ensure nothing is stored in the new prefix. assert!( @@ -97,7 +97,7 @@ pub fn pre_migration< ), ); // ensure storage version is 3. - assert_eq!(StorageVersion::get::>(), 3); + assert_eq!(StorageVersion::get::>(), 3); } /// Some checks for after migration. This can be linked to From 62e7ab7be01add5889b76e8897a7c16d6ac9a874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 9 Jul 2021 00:04:06 +0200 Subject: [PATCH 04/11] Review feedback --- .../src/pallet/expand/pallet_struct.rs | 2 +- frame/support/src/traits/metadata.rs | 4 ++-- frame/support/test/tests/pallet.rs | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 2520ff7559c54..f1057e7673f73 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -180,7 +180,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { #storage_version } - fn active_storage_version() -> #frame_support::traits::StorageVersion { + fn on_chain_storage_version() -> #frame_support::traits::StorageVersion { #frame_support::traits::StorageVersion::get::() } } diff --git a/frame/support/src/traits/metadata.rs b/frame/support/src/traits/metadata.rs index 9c0f3cf8cb839..c947c9f6cecc1 100644 --- a/frame/support/src/traits/metadata.rs +++ b/frame/support/src/traits/metadata.rs @@ -166,8 +166,8 @@ impl PartialOrd for StorageVersion { pub trait GetStorageVersion { /// Returns the current storage version as supported by the pallet. fn current_storage_version() -> StorageVersion; - /// Returns the active storage version of the pallet as stored in the storage. - fn active_storage_version() -> StorageVersion; + /// Returns the on-chain storage version of the pallet as stored in the storage. + fn on_chain_storage_version() -> StorageVersion; } #[cfg(test)] diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 8f233c40284a9..d0c378df85707 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -837,11 +837,11 @@ fn pallet_hooks_expand() { #[test] fn pallet_on_genesis() { TestExternalities::default().execute_with(|| { - assert_eq!(pallet::Pallet::::active_storage_version(), StorageVersion::new(0)); + assert_eq!(pallet::Pallet::::on_chain_storage_version(), StorageVersion::new(0)); pallet::Pallet::::on_genesis(); assert_eq!( pallet::Pallet::::current_storage_version(), - pallet::Pallet::::active_storage_version(), + pallet::Pallet::::on_chain_storage_version(), ); }) } @@ -868,9 +868,9 @@ fn migrate_from_pallet_version_to_storage_version() { sp_io::storage::set(&pallet_version_key(System::name()), &[1, 2, 3]); // Check that everyone currently is at version 0 - assert_eq!(Example::active_storage_version(), StorageVersion::new(0)); - assert_eq!(Example2::active_storage_version(), StorageVersion::new(0)); - assert_eq!(System::active_storage_version(), StorageVersion::new(0)); + assert_eq!(Example::on_chain_storage_version(), StorageVersion::new(0)); + assert_eq!(Example2::on_chain_storage_version(), StorageVersion::new(0)); + assert_eq!(System::on_chain_storage_version(), StorageVersion::new(0)); frame_support::migrations::migrate_from_pallet_version_to_storage_version::(); @@ -879,9 +879,9 @@ fn migrate_from_pallet_version_to_storage_version() { assert!(sp_io::storage::get(&pallet_version_key(Example2::name())).is_none()); assert!(sp_io::storage::get(&pallet_version_key(System::name())).is_none()); - assert_eq!(Example::active_storage_version(), pallet::STORAGE_VERSION); - assert_eq!(Example2::active_storage_version(), StorageVersion::new(0)); - assert_eq!(System::active_storage_version(), StorageVersion::new(0)); + assert_eq!(Example::on_chain_storage_version(), pallet::STORAGE_VERSION); + assert_eq!(Example2::on_chain_storage_version(), StorageVersion::new(0)); + assert_eq!(System::on_chain_storage_version(), StorageVersion::new(0)); }); } From 2c8db3351f849282758aaddca0eeb36e004a8f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 19 Jul 2021 09:47:36 +0200 Subject: [PATCH 05/11] Remove unneeded dep --- frame/support/procedural/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/support/procedural/Cargo.toml b/frame/support/procedural/Cargo.toml index f2072384cffb8..ba71a7d12c62f 100644 --- a/frame/support/procedural/Cargo.toml +++ b/frame/support/procedural/Cargo.toml @@ -20,7 +20,6 @@ proc-macro2 = "1.0.6" quote = "1.0.3" Inflector = "0.11.4" syn = { version = "1.0.58", features = ["full"] } -cargo_metadata = "0.13.1" [features] default = ["std"] From 394ba9f76d82283e612b8fadc93b65555b4163d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 19 Jul 2021 17:29:17 +0200 Subject: [PATCH 06/11] remove pub consts --- Cargo.lock | 1 - frame/contracts/src/lib.rs | 2 +- frame/elections-phragmen/src/lib.rs | 2 +- frame/grandpa/src/lib.rs | 2 +- frame/support/test/tests/pallet.rs | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a80f2ad8d123..bd6bcff83fd57 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1890,7 +1890,6 @@ name = "frame-support-procedural" version = "4.0.0-dev" dependencies = [ "Inflector", - "cargo_metadata", "frame-support-procedural-tools", "proc-macro2", "quote", diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 48453596985ee..ee9b95638e7fc 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -136,7 +136,7 @@ type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; /// The current storage version. -pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); +const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); #[frame_support::pallet] pub mod pallet { diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 2fdad28168143..66d0dd1d6141a 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -123,7 +123,7 @@ pub use weights::WeightInfo; pub mod migrations; /// The current storage version. -pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); +const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); /// The maximum votes allowed per voter. pub const MAXIMUM_VOTE: usize = 16; diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index e3fcc7b587ba0..c671a36c5bba7 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -71,7 +71,7 @@ pub use equivocation::{ pub use pallet::*; /// The current storage version. -pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); +const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); #[frame_support::pallet] pub mod pallet { diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index ff80b2f1b2631..abab4f1e39fe0 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -64,7 +64,7 @@ pub mod pallet { type BalanceOf = ::Balance; - pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(10); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(10); #[pallet::config] pub trait Config: frame_system::Config From 45b39eac5597502fb30428df628e1db086a3b64e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 26 Jul 2021 12:29:55 +0200 Subject: [PATCH 07/11] Brings back logging and implements `GetStorageVersion` --- .../procedural/src/pallet/expand/hooks.rs | 5 +- frame/support/src/dispatch.rs | 185 ++++++++++++++++-- 2 files changed, 177 insertions(+), 13 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index c6ebdd07ad779..314f982c5aad9 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -40,8 +40,11 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::log::info!( target: #frame_support::LOG_TARGET, - "⚠️ {} declares internal migrations (which *might* execute)", + "⚠️ {} declares internal migrations (which *might* execute). \ + On-chain `{:?}` vs current storage version `{:?}`", pallet_name, + ::on_chain_storage_version(), + ::current_storage_version(), ); } } else { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 7eebab74aa29f..ebed0ec02b063 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -352,6 +352,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -388,6 +389,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -408,6 +410,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event() = default; @@ -426,7 +429,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -445,6 +449,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event @@ -471,6 +476,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $vis:vis fn deposit_event() = default; @@ -493,6 +499,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_finalize( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -513,7 +520,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -533,6 +541,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -561,6 +570,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -585,6 +595,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_idle($param_name1:ident : $param1:ty, $param_name2:ident: $param2:ty $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -605,7 +616,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -626,6 +638,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -652,6 +665,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -678,6 +692,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -706,6 +721,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -726,7 +742,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -748,6 +765,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -772,6 +790,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } {} + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn integrity_test() { $( $impl:tt )* } @@ -794,6 +813,7 @@ macro_rules! decl_module { $(#[doc = $doc_attr])* fn integrity_test() { $( $impl)* } } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -815,6 +835,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )+ } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn integrity_test() { $( $impl:tt )* } @@ -839,6 +860,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_initialize( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -865,6 +887,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -893,6 +916,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_initialize( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -913,7 +937,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -935,6 +960,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn on_initialize( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -959,6 +985,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn offchain_worker( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } @@ -979,7 +1006,8 @@ macro_rules! decl_module { { fn offchain_worker( $( $param_name : $param ),* ) { $( $impl )* } } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -1001,6 +1029,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* fn offchain_worker( $( $param_name:ident : $param:ty ),* $(,)? ) -> $return:ty { $( $impl:tt )* } @@ -1026,6 +1055,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $( #[doc = $doc_attr:tt] )* const $name:ident: $ty:ty = $value:expr; @@ -1051,7 +1081,8 @@ macro_rules! decl_module { $name: $ty = $value; } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -1075,6 +1106,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* type Error = $error_type:ty; @@ -1095,7 +1127,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $error_type } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* ] $($rest)* ); @@ -1118,6 +1151,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $($t:tt)* ] $($rest:tt)* ) => { @@ -1136,12 +1170,59 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { &'static str } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $($t)* ] $($rest)* ); }; + // Parse storage version + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: + $trait_name:ident$(, $instance:ident: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } + { $( $on_idle:tt )* } + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + { $( $integrity_test:tt )* } + { } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + type StorageVersion = $storage_version:path; + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type< + $trait_instance: $trait_name$(, $instance: $instantiable $(= $module_default_instance)?)? + > + for enum $call_type where origin: $origin_type, system = $system + { $( $other_where_bounds )* } + { $( $deposit_event )* } + { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } + { $( $on_idle )* } + { $( $on_finalize )* } + { $( $offchain )* } + { $( $constants )* } + { $( $error_type )* } + { $( $integrity_test)* } + { $storage_version } + [ $( $dispatchables )* ] + $($rest)* + ); + }; + // This puts the function statement into the [], decreasing `$rest` and moving toward finishing the parse. (@normalize $(#[$attr:meta])* @@ -1160,6 +1241,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $error_type:ty } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* #[weight = $weight:expr] @@ -1184,7 +1266,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $error_type } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } [ $( $dispatchables )* $(#[doc = $doc_attr])* @@ -1216,6 +1299,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[$fn_attr:meta])* @@ -1245,6 +1329,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -1274,6 +1359,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -1303,6 +1389,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] $(#[doc = $doc_attr:tt])* $(#[weight = $weight:expr])? @@ -1333,6 +1420,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $( $error_type:tt )* } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } [ $( $dispatchables:tt )* ] ) => { $crate::decl_module!(@imp @@ -1350,7 +1438,8 @@ macro_rules! decl_module { { $( $offchain )* } { $( $constants )* } { $( $error_type )* } - { $( $integrity_test)* } + { $( $integrity_test )* } + { $( $storage_version )* } ); }; @@ -1454,8 +1543,11 @@ macro_rules! decl_module { $crate::log::info!( target: $crate::LOG_TARGET, - "⚠️ {} declares internal migrations (which *might* execute)", + "⚠️ {} declares internal migrations (which *might* execute). \ + On-chain `{:?}` vs current storage version `{:?}`", pallet_name, + ::on_chain_storage_version(), + ::current_storage_version(), ); (|| { $( $impl )* })() @@ -1807,6 +1899,45 @@ macro_rules! decl_module { } }; + // Implementation for `GetStorageVersion`. + (@impl_get_storage_version + $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + $( $storage_version:tt )+ + ) => { + // Implement `GetStorageVersion` for `Pallet` + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::GetStorageVersion + for $module<$trait_instance $(, $instance)?> where $( $other_where_bounds )* + { + fn current_storage_version() -> $crate::traits::StorageVersion { + $( $storage_version )* + } + + fn on_chain_storage_version() -> $crate::traits::StorageVersion { + $crate::traits::StorageVersion::get::() + } + } + }; + + // Implementation for `GetStorageVersion` when no storage version is passed. + (@impl_get_storage_version + $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + ) => { + // Implement `GetStorageVersion` for `Pallet` + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::GetStorageVersion + for $module<$trait_instance $(, $instance)?> where $( $other_where_bounds )* + { + fn current_storage_version() -> $crate::traits::StorageVersion { + Default::default() + } + + fn on_chain_storage_version() -> $crate::traits::StorageVersion { + $crate::traits::StorageVersion::get::() + } + } + }; + // The main macro expansion that actually renders the module code. (@imp @@ -1836,6 +1967,7 @@ macro_rules! decl_module { { $( $constants:tt )* } { $error_type:ty } { $( $integrity_test:tt )* } + { $( $storage_version:tt )* } ) => { $crate::__check_reserved_fn_name! { $( $fn_name )* } @@ -1892,6 +2024,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } $( $offchain )* } + $crate::decl_module! { @impl_deposit_event $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; @@ -1948,6 +2081,13 @@ macro_rules! decl_module { )* } + $crate::decl_module! { + @impl_get_storage_version + $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; + { $( $other_where_bounds )* } + $( $storage_version )* + } + // Implement weight calculation function for Call impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::GetDispatchInfo for $call_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* @@ -1981,6 +2121,27 @@ macro_rules! decl_module { } } + // Implement PalletInfoAccess for the module. + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::PalletInfoAccess + for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* + { + fn index() -> usize { + < + <$trait_instance as $system::Config>::PalletInfo as $crate::traits::PalletInfo + >::index::() + .expect("Pallet is part of the runtime because pallet `Config` trait is \ + implemented by the runtime") + } + + fn name() -> &'static str { + < + <$trait_instance as $system::Config>::PalletInfo as $crate::traits::PalletInfo + >::name::() + .expect("Pallet is part of the runtime because pallet `Config` trait is \ + implemented by the runtime") + } + } + // Implement GetCallName for the Call. impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::GetCallName for $call_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* From ccf49bb67b2f28a75ce7ef19375a264cde95ca72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 26 Jul 2021 13:34:59 +0200 Subject: [PATCH 08/11] Return weight from migration --- frame/support/src/migrations.rs | 23 ++++++++++++++++------- frame/support/test/tests/pallet.rs | 18 +++++++++--------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index a1418ebe3b247..ef56f16c3ee7a 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -15,15 +15,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::traits::{GetStorageVersion, PalletInfoAccess}; +use crate::{ + traits::{GetStorageVersion, PalletInfoAccess}, + weights::{RuntimeDbWeight, Weight}, +}; /// Trait used by [`migrate_from_pallet_version_to_storage_version`] to do the actual migration. pub trait PalletVersionToStorageVersionHelper { - fn migrate(); + fn migrate(db_weight: &RuntimeDbWeight) -> Weight; } impl PalletVersionToStorageVersionHelper for T { - fn migrate() { + fn migrate(db_weight: &RuntimeDbWeight) -> Weight { const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; fn pallet_version_key(name: &str) -> [u8; 32] { @@ -41,13 +44,19 @@ impl PalletVersionToStorageVersionHelpe let version = ::current_storage_version(); version.put::(); + + db_weight.writes(2) } } #[impl_trait_for_tuples::impl_for_tuples(30)] impl PalletVersionToStorageVersionHelper for T { - fn migrate() { - for_tuples!( #( T::migrate(); )* ) + fn migrate(db_weight: &RuntimeDbWeight) -> Weight { + let mut weight: Weight = 0; + + for_tuples!( #( weight = weight.saturating_add(T::migrate(db_weight)); )* ); + + weight } } @@ -57,6 +66,6 @@ impl PalletVersionToStorageVersionHelper for T { /// This will remove all `PalletVersion's` from the state and insert the current storage version. pub fn migrate_from_pallet_version_to_storage_version< AllPallets: PalletVersionToStorageVersionHelper, ->() { - AllPallets::migrate() +>(db_weight: &RuntimeDbWeight) -> Weight { + AllPallets::migrate(db_weight) } diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index cadd2c3b10f6b..e376f79396f08 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -15,15 +15,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{ - dispatch::{Parameter, UnfilteredDispatchable}, - storage::unhashed, - traits::{ +use frame_support::{dispatch::{Parameter, UnfilteredDispatchable}, storage::unhashed, traits::{ GetCallName, GetStorageVersion, OnFinalize, OnGenesis, OnInitialize, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion, - }, - weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays}, -}; + }, weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays, RuntimeDbWeight}}; +use frame_system::WeightInfo; use sp_io::{ hashing::{blake2_128, twox_128, twox_64}, TestExternalities, @@ -955,9 +951,13 @@ fn migrate_from_pallet_version_to_storage_version() { assert_eq!(Example2::on_chain_storage_version(), StorageVersion::new(0)); assert_eq!(System::on_chain_storage_version(), StorageVersion::new(0)); - frame_support::migrations::migrate_from_pallet_version_to_storage_version::< + let db_weight = RuntimeDbWeight { read: 0, write: 5 }; + let weight = frame_support::migrations::migrate_from_pallet_version_to_storage_version::< AllPalletsWithSystem, - >(); + >(&db_weight); + + // 3 pallets, 2 writes and every write costs 5 weight. + assert_eq!(3 * 2 * 5, weight); // All pallet versions should be removed assert!(sp_io::storage::get(&pallet_version_key(Example::name())).is_none()); From d43e66dc835592ddd0fb32f83023dc9c52a8567e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 26 Jul 2021 14:44:28 +0200 Subject: [PATCH 09/11] Fmt and remove unused import --- frame/support/src/migrations.rs | 4 +++- frame/support/test/tests/pallet.rs | 10 +++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index ef56f16c3ee7a..cf1ba81982424 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -66,6 +66,8 @@ impl PalletVersionToStorageVersionHelper for T { /// This will remove all `PalletVersion's` from the state and insert the current storage version. pub fn migrate_from_pallet_version_to_storage_version< AllPallets: PalletVersionToStorageVersionHelper, ->(db_weight: &RuntimeDbWeight) -> Weight { +>( + db_weight: &RuntimeDbWeight, +) -> Weight { AllPallets::migrate(db_weight) } diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index e376f79396f08..c21808dfa8f24 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -15,11 +15,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{dispatch::{Parameter, UnfilteredDispatchable}, storage::unhashed, traits::{ +use frame_support::{ + dispatch::{Parameter, UnfilteredDispatchable}, + storage::unhashed, + traits::{ GetCallName, GetStorageVersion, OnFinalize, OnGenesis, OnInitialize, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion, - }, weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays, RuntimeDbWeight}}; -use frame_system::WeightInfo; + }, + weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays, RuntimeDbWeight}, +}; use sp_io::{ hashing::{blake2_128, twox_128, twox_64}, TestExternalities, From 4aba0a8ba8c8814c84a21476c8fb5f210edfdd82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 27 Jul 2021 22:49:30 +0200 Subject: [PATCH 10/11] Update frame/support/src/dispatch.rs Co-authored-by: Guillaume Thiolliere --- frame/support/src/dispatch.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index ebed0ec02b063..da9d6adff6f3f 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2172,7 +2172,10 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::OnGenesis for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { - fn on_genesis() {} + fn on_genesis() { + let storage_version = ::current_storage_version(); + storage_version.put::(); + } } // manual implementation of clone/eq/partialeq because using derive erroneously requires From 37d9280b2803de850240049d5b1e794703079070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 27 Jul 2021 22:49:40 +0200 Subject: [PATCH 11/11] Update frame/support/src/traits/metadata.rs Co-authored-by: Guillaume Thiolliere --- frame/support/src/traits/metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits/metadata.rs b/frame/support/src/traits/metadata.rs index 523a8ee95177a..8b1707855f7b0 100644 --- a/frame/support/src/traits/metadata.rs +++ b/frame/support/src/traits/metadata.rs @@ -154,7 +154,7 @@ impl PartialOrd for StorageVersion { /// Provides information about the storage version of a pallet. /// -/// It differentiates between current and active storage version. Both should be only out of sync +/// It differentiates between current and on-chain storage version. Both should be only out of sync /// when a new runtime upgrade was applied and the runtime migrations did not yet executed. /// Otherwise it means that the pallet works with an unsupported storage version and unforeseen /// stuff can happen. @@ -162,7 +162,7 @@ impl PartialOrd for StorageVersion { /// The current storage version is the version of the pallet as supported at runtime. The active /// storage version is the version of the pallet in the storage. /// -/// It is required to update the active storage version manually when a migration was applied. +/// It is required to update the on-chain storage version manually when a migration was applied. pub trait GetStorageVersion { /// Returns the current storage version as supported by the pallet. fn current_storage_version() -> StorageVersion;