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

Author: pgherveou

bin/node/runtime/src/lib.rs

@@ -55,8 +55,6 @@ use frame_system::{
 pub use node_primitives::{AccountId, Signature};
 use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Moment, Nonce};
 use pallet_asset_conversion::{NativeOrAssetId, NativeOrAssetIdConverter};
-#[cfg(feature = "runtime-benchmarks")]
-use pallet_contracts::NoopMigration;
 use pallet_election_provider_multi_phase::SolutionAccuracyOf;
 use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
 use pallet_nfts::PalletFeatures;
@@ -1257,7 +1255,7 @@ impl pallet_contracts::Config for Runtime {
 	#[cfg(not(feature = "runtime-benchmarks"))]
 	type Migrations = ();
 	#[cfg(feature = "runtime-benchmarks")]
-	type Migrations = (NoopMigration<1>, NoopMigration<2>);
+	type Migrations = pallet_contracts::migration::codegen::BenchMigrations;
 	type MaxDelegateDependencies = ConstU32<32>;
 	type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
 }

frame/contracts/Cargo.toml

@@ -3,6 +3,7 @@ name = "pallet-contracts"
 version = "4.0.0-dev"
 authors = ["Parity Technologies <[email protected]>"]
 edition = "2021"
+build = "build.rs"
 license = "Apache-2.0"
 homepage = "https://substrate.io"
 repository = "https://github.com/paritytech/substrate/"

frame/contracts/build.rs

@@ -0,0 +1,73 @@
+// This file is part of Substrate.
+
+// Copyright (C) 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 std::io::Write;
+
+/// Get the latest migration version.
+///
+/// Find the highest version number from the available migration files.
+/// Each migration file should follow the naming convention `vXX.rs`, where `XX` is the version
+/// number.
+fn get_latest_version() -> u16 {
+	std::fs::read_dir("src/migration")
+		.expect("Folder `src/migration` not found.")
+		.filter_map(|entry| {
+			let file_name = entry.as_ref().ok()?.file_name();
+			let file_name = file_name.to_str()?;
+			if file_name.starts_with('v') && file_name.ends_with(".rs") {
+				let version = &file_name[1..&file_name.len() - 3];
+				let version = version.parse::<u16>().ok()?;
+
+				// Ensure that the version matches the one defined in the file.
+				let path = entry.unwrap().path();
+				let file_content = std::fs::read_to_string(&path).ok()?;
+				assert!(
+					file_content.contains(&format!("const VERSION: u16 = {}", version)),
+					"Invalid MigrationStep::VERSION in {:?}",
+					path
+				);
+
+				return Some(version)
+			}
+			None
+		})
+		.max()
+		.expect("Failed to find any files matching the 'src/migration/vxx.rs' pattern.")
+}
+
+/// Generates a module that exposes the latest migration version, and the benchmark migrations type.
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+	let out_dir = std::env::var("OUT_DIR")?;
+	let path = std::path::Path::new(&out_dir).join("migration_codegen.rs");
+	let mut f = std::fs::File::create(&path)?;
+	let version = get_latest_version();
+	write!(
+		f,
+		"
+         pub mod codegen {{
+		  use crate::NoopMigration;
+		  /// The latest migration version, pulled from the latest migration file.
+		  pub const LATEST_MIGRATION_VERSION: u16 = {version};
+		  /// The Migration Steps used for benchmarking the migration framework.
+		  pub type BenchMigrations = (NoopMigration<{}>, NoopMigration<{version}>);
+		}}",
+		version - 1,
+	)?;
+
+	println!("cargo:rerun-if-changed=src/migration");
+	Ok(())
+}

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 {

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)]

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);
+		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!(
-					<Pallet<Test>>::on_chain_storage_version(),
-					StorageVersion::new(version)
+					TestMigration::migrate(Weight::MAX).0,
+					MigrateResult::NoMigrationInProgress
 				);
-			}
-
-			assert_eq!(TestMigration::migrate(Weight::MAX).0, MigrateResult::NoMigrationInProgress);
-			assert_eq!(StorageVersion::get::<Pallet<Test>>(), 2);
-		});
+				assert_eq!(StorageVersion::get::<Pallet<Test>>(), LATEST_MIGRATION_VERSION);
+			});
 	}
 }

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