Contracts: Ensure latest migration match pallet_version (#14676)

* Add version check

* fix format

* simplify

* restructure imports

* add version check

* Fix benchmarking

* Rename migrations -> BenchMigrations

* doc

* add more docs

* fix format string

* Update frame/contracts/build.rs

* fix

* add cargo:rerun-if-changed

---------

Co-authored-by: parity-processbot <>

substrate/frame/contracts/src/benchmarking/mod.rs

@@ -30,7 +30,7 @@ use self::{
 };
 use crate::{
 	exec::{AccountIdOf, Key},
-	migration::{v09, v10, v11, v12, v13, MigrationStep},
+	migration::{codegen::LATEST_MIGRATION_VERSION, v09, v10, v11, v12, v13, MigrationStep},
 	wasm::CallFlags,
 	Pallet as Contracts, *,
 };
@@ -273,29 +273,32 @@ benchmarks! {
 	// This benchmarks the weight of executing Migration::migrate to execute a noop migration.
 	#[pov_mode = Measured]
 	migration_noop {
-		assert_eq!(StorageVersion::get::<Pallet<T>>(), 2);
+		let version = LATEST_MIGRATION_VERSION;
+		assert_eq!(StorageVersion::get::<Pallet<T>>(), version);
 	}:  {
 		Migration::<T>::migrate(Weight::MAX)
 	} verify {
-		assert_eq!(StorageVersion::get::<Pallet<T>>(), 2);
+		assert_eq!(StorageVersion::get::<Pallet<T>>(), version);
 	}
 
 	// This benchmarks the weight of dispatching migrate to execute 1 `NoopMigraton`
 	#[pov_mode = Measured]
 	migrate {
-		StorageVersion::new(0).put::<Pallet<T>>();
+		let latest_version = LATEST_MIGRATION_VERSION;
+		StorageVersion::new(latest_version - 2).put::<Pallet<T>>();
 		<Migration::<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
 		let caller: T::AccountId = whitelisted_caller();
 		let origin = RawOrigin::Signed(caller.clone());
 	}: _(origin, Weight::MAX)
 	verify {
-		assert_eq!(StorageVersion::get::<Pallet<T>>(), 1);
+		assert_eq!(StorageVersion::get::<Pallet<T>>(), latest_version - 1);
 	}
 
 	// This benchmarks the weight of running on_runtime_upgrade when there are no migration in progress.
 	#[pov_mode = Measured]
 	on_runtime_upgrade_noop {
-		assert_eq!(StorageVersion::get::<Pallet<T>>(), 2);
+		let latest_version = LATEST_MIGRATION_VERSION;
+		assert_eq!(StorageVersion::get::<Pallet<T>>(), latest_version);
 	}:  {
 		<Migration::<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade()
 	} verify {
@@ -305,7 +308,8 @@ benchmarks! {
 	// This benchmarks the weight of running on_runtime_upgrade when there is a migration in progress.
 	#[pov_mode = Measured]
 	on_runtime_upgrade_in_progress {
-		StorageVersion::new(0).put::<Pallet<T>>();
+		let latest_version = LATEST_MIGRATION_VERSION;
+		StorageVersion::new(latest_version - 2).put::<Pallet<T>>();
 		let v = vec![42u8].try_into().ok();
 		MigrationInProgress::<T>::set(v.clone());
 	}:  {
@@ -318,7 +322,8 @@ benchmarks! {
 	// This benchmarks the weight of running on_runtime_upgrade when there is a migration to process.
 	#[pov_mode = Measured]
 	on_runtime_upgrade {
-		StorageVersion::new(0).put::<Pallet<T>>();
+		let latest_version = LATEST_MIGRATION_VERSION;
+		StorageVersion::new(latest_version - 2).put::<Pallet<T>>();
 	}:  {
 		<Migration::<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade()
 	} verify {

substrate/frame/contracts/src/lib.rs

@@ -186,12 +186,7 @@ pub mod pallet {
 	use sp_runtime::Perbill;
 
 	/// The current storage version.
-	#[cfg(not(any(test, feature = "runtime-benchmarks")))]
-	const STORAGE_VERSION: StorageVersion = StorageVersion::new(13);
-
-	/// Hard coded storage version for running tests that depend on the current storage version.
-	#[cfg(any(test, feature = "runtime-benchmarks"))]
-	const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
+	pub(crate) const STORAGE_VERSION: StorageVersion = StorageVersion::new(13);
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]

substrate/frame/contracts/src/migration.rs

@@ -61,6 +61,7 @@ pub mod v10;
 pub mod v11;
 pub mod v12;
 pub mod v13;
+include!(concat!(env!("OUT_DIR"), "/migration_codegen.rs"));
 
 use crate::{weights::WeightInfo, Config, Error, MigrationInProgress, Pallet, Weight, LOG_TARGET};
 use codec::{Codec, Decode};
@@ -522,7 +523,10 @@ impl MigrateSequence for Tuple {
 #[cfg(test)]
 mod test {
 	use super::*;
-	use crate::tests::{ExtBuilder, Test};
+	use crate::{
+		migration::codegen::LATEST_MIGRATION_VERSION,
+		tests::{ExtBuilder, Test},
+	};
 
 	#[derive(Default, Encode, Decode, MaxEncodedLen)]
 	struct MockMigration<const N: u16> {
@@ -546,6 +550,11 @@ mod test {
 		}
 	}
 
+	#[test]
+	fn test_storage_version_matches_last_migration_file() {
+		assert_eq!(StorageVersion::new(LATEST_MIGRATION_VERSION), crate::pallet::STORAGE_VERSION);
+	}
+
 	#[test]
 	fn version_range_works() {
 		let range = <(MockMigration<1>, MockMigration<2>)>::VERSION_RANGE;
@@ -584,7 +593,7 @@ mod test {
 		type TestMigration = Migration<Test>;
 
 		ExtBuilder::default().build().execute_with(|| {
-			assert_eq!(StorageVersion::get::<Pallet<Test>>(), 2);
+			assert_eq!(StorageVersion::get::<Pallet<Test>>(), LATEST_MIGRATION_VERSION);
 			assert_eq!(TestMigration::migrate(Weight::MAX).0, MigrateResult::NoMigrationInProgress)
 		});
 	}
@@ -593,21 +602,28 @@ mod test {
 	fn migration_works() {
 		type TestMigration = Migration<Test, false>;
 
-		ExtBuilder::default().set_storage_version(0).build().execute_with(|| {
-			assert_eq!(StorageVersion::get::<Pallet<Test>>(), 0);
-			TestMigration::on_runtime_upgrade();
-			for (version, status) in
-				[(1, MigrateResult::InProgress { steps_done: 1 }), (2, MigrateResult::Completed)]
-			{
-				assert_eq!(TestMigration::migrate(Weight::MAX).0, status);
-				assert_eq!(
-					<Pallet<Test>>::on_chain_storage_version(),
-					StorageVersion::new(version)
-				);
-			}
+		ExtBuilder::default()
+			.set_storage_version(LATEST_MIGRATION_VERSION - 2)
+			.build()
+			.execute_with(|| {
+				assert_eq!(StorageVersion::get::<Pallet<Test>>(), LATEST_MIGRATION_VERSION - 2);
+				TestMigration::on_runtime_upgrade();
+				for (version, status) in [
+					(LATEST_MIGRATION_VERSION - 1, MigrateResult::InProgress { steps_done: 1 }),
+					(LATEST_MIGRATION_VERSION, MigrateResult::Completed),
+				] {
+					assert_eq!(TestMigration::migrate(Weight::MAX).0, status);
+					assert_eq!(
+						<Pallet<Test>>::on_chain_storage_version(),
+						StorageVersion::new(version)
+					);
+				}
 
-			assert_eq!(TestMigration::migrate(Weight::MAX).0, MigrateResult::NoMigrationInProgress);
-			assert_eq!(StorageVersion::get::<Pallet<Test>>(), 2);
-		});
+				assert_eq!(
+					TestMigration::migrate(Weight::MAX).0,
+					MigrateResult::NoMigrationInProgress
+				);
+				assert_eq!(StorageVersion::get::<Pallet<Test>>(), LATEST_MIGRATION_VERSION);
+			});
 	}
 }

substrate/frame/contracts/src/tests.rs

@@ -1,5 +1,4 @@
 // This file is part of Substrate.
-mod pallet_dummy;
 
 // Copyright (C) Parity Technologies (UK) Ltd.
 // SPDX-License-Identifier: Apache-2.0
@@ -16,6 +15,8 @@ mod pallet_dummy;
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+mod pallet_dummy;
+
 use self::test_utils::{ensure_stored, expected_deposit, hash};
 use crate::{
 	self as pallet_contracts,
@@ -24,13 +25,14 @@ use crate::{
 		Result as ExtensionResult, RetVal, ReturnFlags, SysConfig,
 	},
 	exec::{Frame, Key},
+	migration::codegen::LATEST_MIGRATION_VERSION,
 	storage::DeletionQueueManager,
 	tests::test_utils::{get_contract, get_contract_checked},
 	wasm::{Determinism, ReturnCode as RuntimeReturnCode},
 	weights::WeightInfo,
 	BalanceOf, Code, CodeHash, CodeInfoOf, CollectEvents, Config, ContractInfo, ContractInfoOf,
-	DebugInfo, DefaultAddressGenerator, DeletionQueueCounter, Error, MigrationInProgress,
-	NoopMigration, Origin, Pallet, PristineCode, Schedule,
+	DebugInfo, DefaultAddressGenerator, DeletionQueueCounter, Error, MigrationInProgress, Origin,
+	Pallet, PristineCode, Schedule,
 };
 use assert_matches::assert_matches;
 use codec::Encode;
@@ -457,7 +459,7 @@ impl Config for Test {
 	type MaxStorageKeyLen = ConstU32<128>;
 	type UnsafeUnstableInterface = UnstableInterface;
 	type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
-	type Migrations = (NoopMigration<1>, NoopMigration<2>);
+	type Migrations = crate::migration::codegen::BenchMigrations;
 	type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
 	type MaxDelegateDependencies = MaxDelegateDependencies;
 }
@@ -610,17 +612,20 @@ fn calling_plain_account_fails() {
 fn migration_on_idle_hooks_works() {
 	// Defines expectations of how many migration steps can be done given the weight limit.
 	let tests = [
-		(Weight::zero(), 0),
-		(<Test as Config>::WeightInfo::migrate() + 1.into(), 1),
-		(Weight::MAX, 2),
+		(Weight::zero(), LATEST_MIGRATION_VERSION - 2),
+		(<Test as Config>::WeightInfo::migrate() + 1.into(), LATEST_MIGRATION_VERSION - 1),
+		(Weight::MAX, LATEST_MIGRATION_VERSION),
 	];
 
 	for (weight, expected_version) in tests {
-		ExtBuilder::default().set_storage_version(0).build().execute_with(|| {
-			MigrationInProgress::<Test>::set(Some(Default::default()));
-			Contracts::on_idle(System::block_number(), weight);
-			assert_eq!(StorageVersion::get::<Pallet<Test>>(), expected_version);
-		});
+		ExtBuilder::default()
+			.set_storage_version(LATEST_MIGRATION_VERSION - 2)
+			.build()
+			.execute_with(|| {
+				MigrationInProgress::<Test>::set(Some(Default::default()));
+				Contracts::on_idle(System::block_number(), weight);
+				assert_eq!(StorageVersion::get::<Pallet<Test>>(), expected_version);
+			});
 	}
 }