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 <[email protected]>

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

Co-authored-by: Guillaume Thiolliere <[email protected]>

Co-authored-by: Guillaume Thiolliere <[email protected]>

frame/contracts/src/lib.rs

@@ -111,7 +111,7 @@ use crate::{
 };
 use frame_support::{
 	dispatch::Dispatchable,
-	traits::{Currency, Filter, Get, OnUnbalanced, Randomness, Time},
+	traits::{Currency, Filter, Get, OnUnbalanced, Randomness, StorageVersion, Time},
 	weights::{GetDispatchInfo, PostDispatchInfo, Weight, WithPostDispatchInfo},
 };
 use frame_system::Pallet as System;
@@ -134,6 +134,9 @@ type NegativeImbalanceOf<T> = <<T as Config>::Currency as Currency<
 	<T as frame_system::Config>::AccountId,
 >>::NegativeImbalance;
 
+/// The current storage version.
+const STORAGE_VERSION: StorageVersion = StorageVersion::new(4);
+
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
@@ -273,6 +276,7 @@ pub mod pallet {
 	}
 
 	#[pallet::pallet]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T>(PhantomData<T>);
 
 	#[pallet::hooks]

frame/contracts/src/migration.rs

@@ -18,22 +18,17 @@
 use crate::{Config, Pallet, Weight};
 use frame_support::{
 	storage::migration,
-	traits::{Get, GetPalletVersion, PalletInfoAccess, PalletVersion},
+	traits::{Get, PalletInfoAccess, StorageVersion},
 };
 
 pub fn migrate<T: Config>() -> Weight {
 	let mut weight: Weight = 0;
 
-	match <Pallet<T>>::storage_version() {
-		Some(version) if version == PalletVersion::new(3, 0, 0) => {
-			weight = weight.saturating_add(T::DbWeight::get().writes(1));
-			migration::remove_storage_prefix(
-				<Pallet<T>>::name().as_bytes(),
-				b"CurrentSchedule",
-				b"",
-			);
-		},
-		_ => (),
+	if StorageVersion::get::<Pallet<T>>() == 3 {
+		weight = weight.saturating_add(T::DbWeight::get().writes(1));
+		migration::remove_storage_prefix(<Pallet<T>>::name().as_bytes(), b"CurrentSchedule", b"");
+
+		StorageVersion::new(4).put::<Pallet<T>>();
 	}
 
 	weight

frame/elections-phragmen/src/lib.rs

@@ -104,7 +104,7 @@ use frame_support::{
 	traits::{
 		ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get,
 		InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency,
-		SortedMembers, WithdrawReasons,
+		SortedMembers, StorageVersion, WithdrawReasons,
 	},
 	weights::Weight,
 };
@@ -122,6 +122,9 @@ pub use weights::WeightInfo;
 /// All migrations.
 pub mod migrations;
 
+/// The current storage version.
+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<T>(PhantomData<T>);
 
 	#[pallet::hooks]

frame/elections-phragmen/src/migrations/v3.rs

@@ -19,7 +19,7 @@
 
 use codec::{Decode, Encode, FullCodec};
 use frame_support::{
-	traits::{GetPalletVersion, PalletVersion},
+	traits::{PalletInfoAccess, StorageVersion},
 	weights::Weight,
 	RuntimeDebug, Twox64Concat,
 };
@@ -41,8 +41,8 @@ struct Voter<AccountId, Balance> {
 
 /// 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 + PalletInfoAccess;
 
 	/// System config account id
 	type AccountId: 'static + FullCodec;
@@ -67,7 +67,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
 ///
@@ -77,28 +77,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<T: V2ToV3>(old_voter_bond: T::Balance, old_candidacy_bond: T::Balance) -> Weight {
-	let maybe_storage_version = <T::Module as GetPalletVersion>::storage_version();
+	let storage_version = StorageVersion::get::<T::Pallet>();
 	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 <= PalletVersion::new(2, 0, 0) => {
-			migrate_voters_to_recorded_deposit::<T>(old_voter_bond);
-			migrate_candidates_to_recorded_deposit::<T>(old_candidacy_bond);
-			migrate_runners_up_to_recorded_deposit::<T>(old_candidacy_bond);
-			migrate_members_to_recorded_deposit::<T>(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::<T>(old_voter_bond);
+		migrate_candidates_to_recorded_deposit::<T>(old_candidacy_bond);
+		migrate_runners_up_to_recorded_deposit::<T>(old_candidacy_bond);
+		migrate_members_to_recorded_deposit::<T>(old_candidacy_bond);
+
+		StorageVersion::new(3).put::<T::Pallet>();
+
+		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
 	}
 }
 

frame/elections-phragmen/src/migrations/v4.rs

@@ -18,7 +18,7 @@
 //! Migrations to version [`4.0.0`], as denoted by the changelog.
 
 use frame_support::{
-	traits::{Get, GetPalletVersion, PalletVersion},
+	traits::{Get, StorageVersion},
 	weights::Weight,
 };
 
@@ -32,48 +32,46 @@ pub const OLD_PREFIX: &[u8] = b"PhragmenElection";
 /// `<Runtime as frame_system::Config>::PalletInfo::name::<ElectionsPhragmenPallet>`.
 ///
 /// The old storage prefix, `PhragmenElection` is hardcoded in the migration code.
-pub fn migrate<T: frame_system::Config, P: GetPalletVersion, N: AsRef<str>>(
-	new_pallet_name: N,
-) -> Weight {
+pub fn migrate<T: crate::Config, N: AsRef<str>>(new_pallet_name: N) -> Weight {
 	if new_pallet_name.as_ref().as_bytes() == OLD_PREFIX {
 		log::info!(
 			target: "runtime::elections-phragmen",
 			"New pallet name is equal to the old prefix. No migration needs to be done.",
 		);
 		return 0
 	}
-	let maybe_storage_version = <P as GetPalletVersion>::storage_version();
+	let storage_version = StorageVersion::get::<crate::Pallet<T>>();
 	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 <= PalletVersion::new(3, 0, 0) => {
-			log::info!("new prefix: {}", new_pallet_name.as_ref());
-			frame_support::storage::migration::move_pallet(
-				OLD_PREFIX,
-				new_pallet_name.as_ref().as_bytes(),
-			);
-			<T as frame_system::Config>::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::<crate::Pallet<T>>();
+
+		<T as frame_system::Config>::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
 	}
 }
 
 /// Some checks prior to migration. This can be linked to
 /// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing.
 ///
 /// Panics if anything goes wrong.
-pub fn pre_migration<P: GetPalletVersion, N: AsRef<str>>(new: N) {
+pub fn pre_migration<T: crate::Config, N: AsRef<str>>(new: N) {
 	let new = new.as_ref();
 	log::info!("pre-migration elections-phragmen test with new = {}", new);
 
@@ -94,15 +92,15 @@ pub fn pre_migration<P: GetPalletVersion, N: AsRef<str>>(new: N) {
 		sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_bytes()).unwrap())
 	);
 	// ensure storage version is 3.
-	assert!(<P as GetPalletVersion>::storage_version().unwrap().major == 3);
+	assert_eq!(StorageVersion::get::<crate::Pallet<T>>(), 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<P: GetPalletVersion>() {
+pub fn post_migration<T: crate::Config>() {
 	log::info!("post-migration elections-phragmen");
 	// ensure we've been updated to v4 by the automatic write of crate version -> storage version.
-	assert!(<P as GetPalletVersion>::storage_version().unwrap().major == 4);
+	assert_eq!(StorageVersion::get::<crate::Pallet<T>>(), 4);
 }

frame/grandpa/src/lib.rs

@@ -42,7 +42,7 @@ use fg_primitives::{
 use frame_support::{
 	dispatch::DispatchResultWithPostInfo,
 	storage,
-	traits::{KeyOwnerProofSystem, OneSessionHandler},
+	traits::{KeyOwnerProofSystem, OneSessionHandler, StorageVersion},
 	weights::{Pays, Weight},
 };
 use sp_runtime::{generic::DigestItem, traits::Zero, DispatchResult, KeyTypeId};
@@ -67,6 +67,9 @@ pub use equivocation::{
 
 pub use pallet::*;
 
+/// The current storage version.
+const STORAGE_VERSION: StorageVersion = StorageVersion::new(4);
+
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
@@ -75,6 +78,7 @@ pub mod pallet {
 
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(super) trait Store)]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T>(_);
 
 	#[pallet::config]

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;

frame/grandpa/src/migrations/v3_1.rs → frame/grandpa/src/migrations/v4.rs

@@ -16,7 +16,7 @@
 // limitations under the License.
 
 use frame_support::{
-	traits::{Get, GetPalletVersion, PalletVersion},
+	traits::{Get, StorageVersion},
 	weights::Weight,
 };
 use sp_io::hashing::twox_128;
@@ -31,50 +31,41 @@ pub const OLD_PREFIX: &[u8] = b"GrandpaFinality";
 /// `<Runtime as frame_system::Config>::PalletInfo::name::<GrandpaPallet>`.
 ///
 /// The old storage prefix, `GrandpaFinality` is hardcoded in the migration code.
-pub fn migrate<T: frame_system::Config, P: GetPalletVersion, N: AsRef<str>>(
-	new_pallet_name: N,
-) -> Weight {
+pub fn migrate<T: crate::Config, N: AsRef<str>>(new_pallet_name: N) -> Weight {
 	if new_pallet_name.as_ref().as_bytes() == OLD_PREFIX {
 		log::info!(
 			target: "runtime::afg",
 			"New pallet name is equal to the old prefix. No migration needs to be done.",
 		);
 		return 0
 	}
-	let maybe_storage_version = <P as GetPalletVersion>::storage_version();
+	let storage_version = StorageVersion::get::<crate::Pallet<T>>();
 	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 <= PalletVersion::new(3, 0, 0) => {
-			log::info!("new prefix: {}", new_pallet_name.as_ref());
-			frame_support::storage::migration::move_pallet(
-				OLD_PREFIX,
-				new_pallet_name.as_ref().as_bytes(),
-			);
-			<T as frame_system::Config>::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
-		},
+	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::<crate::Pallet<T>>();
+
+		<T as frame_system::Config>::BlockWeights::get().max_block
+	} else {
+		0
 	}
 }
 
 /// Some checks prior to migration. This can be linked to
 /// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing.
 ///
 /// Panics if anything goes wrong.
-pub fn pre_migration<T: frame_system::Config, P: GetPalletVersion + 'static, N: AsRef<str>>(
-	new: N,
-) {
+pub fn pre_migration<T: crate::Config, N: AsRef<str>>(new: N) {
 	let new = new.as_ref();
 	log::info!("pre-migration grandpa test with new = {}", new);
 
@@ -83,7 +74,7 @@ pub fn pre_migration<T: frame_system::Config, P: GetPalletVersion + 'static, N:
 	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::<T::PalletInfo, P>().unwrap();
+	let storage_key = StorageVersion::storage_key::<crate::Pallet<T>>();
 
 	// ensure nothing is stored in the new prefix.
 	assert!(
@@ -103,14 +94,14 @@ pub fn pre_migration<T: frame_system::Config, P: GetPalletVersion + 'static, N:
 		),
 	);
 	// ensure storage version is 3.
-	assert!(<P as GetPalletVersion>::storage_version().unwrap().major == 3);
+	assert_eq!(StorageVersion::get::<crate::Pallet<T>>(), 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<P: GetPalletVersion>() {
+pub fn post_migration() {
 	log::info!("post-migration grandpa");
 
 	// Assert that nothing remains at the old prefix