Move PalletVersion away from the crate version (#9165)

* 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.

* Overhaul the entire implementation

- Drop PalletVersion
- Introduce StorageVersion
- StorageVersion needs to be set in the crate and set for the macros
- Added migration

* Fix migrations

* Review feedback

* Remove unneeded dep

* remove pub consts

* Brings back logging and implements `GetStorageVersion`

* Return weight from migration

* Fmt and remove unused import

* Update frame/support/src/dispatch.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Update frame/support/src/traits/metadata.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

substrate/frame/support/procedural/src/lib.rs

@@ -26,7 +26,6 @@ mod default_no_bound;
 mod dummy_part_checker;
 mod key_prefix;
 mod pallet;
-mod pallet_version;
 mod partial_eq_no_bound;
 mod storage;
 mod transactional;
@@ -461,13 +460,6 @@ pub fn require_transactional(attr: TokenStream, input: TokenStream) -> TokenStre
 		.unwrap_or_else(|e| e.to_compile_error().into())
 }
 
-#[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()
-}
-
 /// 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;

substrate/frame/support/procedural/src/pallet/expand/hooks.rs

@@ -40,9 +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), setting storage version to {:?}",
+				"⚠️ {} declares internal migrations (which *might* execute). \
+				 On-chain `{:?}` vs current storage version `{:?}`",
 				pallet_name,
-				new_storage_version,
+				<Self as #frame_support::traits::GetStorageVersion>::on_chain_storage_version(),
+				<Self as #frame_support::traits::GetStorageVersion>::current_storage_version(),
 			);
 		}
 	} else {
@@ -50,9 +52,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 +132,6 @@ 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 pallet_name = <
 					<T as #frame_system::Config>::PalletInfo
 					as
@@ -139,19 +139,11 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
 				>::name::<Self>().unwrap_or("<unknown pallet name>");
 				#log_runtime_upgrade
 
-				let result = <
+				<
 					Self as #frame_support::traits::Hooks<
 						<T as #frame_system::Config>::BlockNumber
 					>
-				>::on_runtime_upgrade();
-
-				new_storage_version.put_into_storage::<<T as #frame_system::Config>::PalletInfo, Self>();
-
-				let additional_write = <
-					<T as #frame_system::Config>::DbWeight as #frame_support::traits::Get<_>
-				>::get().writes(1);
-
-				result.saturating_add(additional_write)
+				>::on_runtime_upgrade()
 			}
 
 			#[cfg(feature = "try-runtime")]

substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs

@@ -18,7 +18,7 @@
 use crate::pallet::{expand::merge_where_clauses, parse::helper::get_doc_literals, Def};
 
 /// * 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
@@ -151,6 +151,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
 
@@ -161,21 +167,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::crate_to_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::<
-						<T as #frame_system::Config>::PalletInfo, Self
-					>().expect("Every active pallet has a name in the runtime; qed");
-
-				#frame_support::storage::unhashed::get(&key)
+			fn on_chain_storage_version() -> #frame_support::traits::StorageVersion {
+				#frame_support::traits::StorageVersion::get::<Self>()
 			}
 		}
 
@@ -185,8 +187,8 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream {
 			#config_where_clause
 		{
 			fn on_genesis() {
-				#frame_support::crate_to_pallet_version!()
-					.put_into_storage::<<T as #frame_system::Config>::PalletInfo, Self>();
+				let storage_version = #storage_version;
+				storage_version.put::<Self>();
 			}
 		}
 

substrate/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,14 +44,18 @@ 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<proc_macro2::Span>,
+	/// The current storage version of the pallet.
+	pub storage_version: Option<syn::Path>,
 }
 
 /// 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, vis: syn::Visibility, keyword: keyword::Store },
 	GenerateStorageInfoTrait(proc_macro2::Span),
+	StorageVersion { storage_version: syn::Path, span: proc_macro2::Span },
 }
 
 impl PalletStructAttr {
@@ -58,6 +63,7 @@ impl PalletStructAttr {
 		match self {
 			Self::GenerateStore { span, .. } => *span,
 			Self::GenerateStorageInfoTrait(span) => *span,
+			Self::StorageVersion { span, .. } => *span,
 		}
 	}
 }
@@ -83,6 +89,14 @@ impl syn::parse::Parse for PalletStructAttr {
 		} else if lookahead.peek(keyword::generate_storage_info) {
 			let span = content.parse::<keyword::generate_storage_info>()?.span();
 			Ok(Self::GenerateStorageInfoTrait(span))
+		} else if lookahead.peek(keyword::storage_version) {
+			let span = content.parse::<keyword::storage_version>()?.span();
+
+			let version_content;
+			syn::parenthesized!(version_content in content);
+			let storage_version = version_content.parse::<syn::Path>()?;
+
+			Ok(Self::StorageVersion { storage_version, span })
 		} else {
 			Err(lookahead.error())
 		}
@@ -104,6 +118,7 @@ impl PalletStructDef {
 
 		let mut store = None;
 		let mut generate_storage_info = None;
+		let mut storage_version_found = None;
 
 		let struct_attrs: Vec<PalletStructAttr> = helper::take_item_pallet_attrs(&mut item.attrs)?;
 		for attr in struct_attrs {
@@ -116,6 +131,11 @@ impl PalletStructDef {
 				{
 					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))
@@ -138,6 +158,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,
+		})
 	}
 }

substrate/frame/support/procedural/src/pallet_version.rs

@@ -1,64 +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 frame_support_procedural_tools::generate_crate_access_2018;
-use proc_macro2::{Span, TokenStream};
-use std::{env, str::FromStr};
-use syn::{Error, Result};
-
-/// Get the version from the given version environment variable.
-///
-/// The version is parsed into the requested destination type.
-fn get_version<T: FromStr>(version_env: &str) -> std::result::Result<T, ()> {
-	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)
-}
-
-/// Create an error that will be shown by rustc at the call site of the macro.
-fn create_error(message: &str) -> 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<TokenStream> {
-	if !input.is_empty() {
-		return Err(create_error("No arguments expected!"))
-	}
-
-	let major_version = get_version::<u16>("CARGO_PKG_VERSION_MAJOR")
-		.map_err(|_| create_error("Major version needs to fit into `u16`"))?;
-
-	let minor_version = get_version::<u8>("CARGO_PKG_VERSION_MINOR")
-		.map_err(|_| create_error("Minor version needs to fit into `u8`"))?;
-
-	let patch_version = get_version::<u8>("CARGO_PKG_VERSION_PATCH")
-		.map_err(|_| create_error("Patch version needs to fit into `u8`"))?;
-
-	let crate_ = generate_crate_access_2018("frame-support")?;
-
-	Ok(quote::quote! {
-		#crate_::traits::PalletVersion {
-			major: #major_version,
-			minor: #minor_version,
-			patch: #patch_version,
-		}
-	})
-}