Update contract multi-block migration

frame/contracts/src/benchmarking/mod.rs

@@ -30,7 +30,7 @@ use self::{
 };
 use crate::{
 	exec::{AccountIdOf, Key},
-	migration::{v10, v11, v9, Migrate},
+	migration::{v10, v11, v9, MigrationStep},
 	wasm::CallFlags,
 	Pallet as Contracts, *,
 };
@@ -237,7 +237,7 @@ benchmarks! {
 	// This benchmarks the v9 migration step. (update codeStorage)
 	#[pov_mode = Measured]
 	v9_migration_step {
-		let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
+		let c in 0 .. T::MaxCodeLen::get();
 		v9::store_old_dummy_code::<T>(c as usize);
 		let mut m = v9::Migration::<T>::default();
 	}: {
@@ -251,7 +251,7 @@ benchmarks! {
 			whitelisted_caller(), WasmModule::dummy(), vec![],
 		)?;
 
-		v10::store_old_contrat_info::<T>(contract.account_id.clone(), contract.info()?);
+		v10::store_old_contract_info::<T>(contract.account_id.clone(), contract.info()?);
 		let mut m = v10::Migration::<T>::default();
 	}: {
 		m.step();
@@ -277,15 +277,15 @@ benchmarks! {
 		assert_eq!(StorageVersion::get::<Pallet<T>>(), 2);
 	}
 
-	// This benchmarks the weight of executing Migration::migrate when there are no migration in progress.
+	// This benchmarks the weight of dispatching migrate to executing 1 `NoopMigraton`
 	#[pov_mode = Measured]
 	migrate {
 		StorageVersion::new(0).put::<Pallet<T>>();
 		<Migration::<T> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
-		let origin: RawOrigin<<T as frame_system::Config>::AccountId> = RawOrigin::Signed(whitelisted_caller());
-	}: {
-		<Contracts<T>>::migrate(origin.into(), Weight::MAX).unwrap()
-	} verify {
+		let caller: T::AccountId = whitelisted_caller();
+		let origin = RawOrigin::Signed(caller.clone());
+	}: _(origin, Weight::MAX)
+	verify {
 		assert_eq!(StorageVersion::get::<Pallet<T>>(), 1);
 	}
 

frame/contracts/src/lib.rs

@@ -80,6 +80,7 @@
 //! an [`eDSL`](https://wiki.haskell.org/Embedded_domain_specific_language) that enables writing
 //! WebAssembly based smart contracts in the Rust programming language.
 
+#![allow(rustdoc::private_intra_doc_links)]
 #![cfg_attr(not(feature = "std"), no_std)]
 #![cfg_attr(feature = "runtime-benchmarks", recursion_limit = "1024")]
 
@@ -328,19 +329,36 @@ pub mod pallet {
 		/// # struct Runtime {};
 		/// type Migrations = (v9::Migration<Runtime>, v10::Migration<Runtime>, v11::Migration<Runtime>);
 		/// ```
+		///
+		/// If you have a single migration step, you can use a tuple with a single element:
+		/// ```
+		/// use pallet_contracts::migration::v9;
+		/// # struct Runtime {};
+		/// type Migrations = (v9::Migration<Runtime>,);
+		/// ```
 		type Migrations: MigrateSequence;
 	}
 
 	#[pallet::hooks]
 	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
 		fn on_idle(_block: T::BlockNumber, remaining_weight: Weight) -> Weight {
 			use migration::MigrateResult::*;
-
-			let (result, weight) = Migration::<T>::migrate(remaining_weight);
-			let remaining_weight = remaining_weight.saturating_sub(weight);
-
-			if !matches!(result, Completed | NoMigrationInProgress) {
-				return weight
+			let mut remaining_weight = remaining_weight;
+
+			loop {
+				let (result, weight) = Migration::<T>::migrate(remaining_weight);
+				remaining_weight.saturating_reduce(weight);
+
+				match result {
+					// There is not enough weight to perform a migration, or make any progress, we
+					// just return the remaining weight.
+					NoMigrationPerformed | InProgress { steps_done: 0 } => return remaining_weight,
+					// Migration is still in progress, we can start the next step.
+					InProgress { .. } => continue,
+					// Either no migration is in progress, or we are done with all migrations, we
+					// can do some more other work with the remaining weight.
+					Completed | NoMigrationInProgress => break,
+				}
 			}
 
 			ContractInfo::<T>::process_deletion_queue_batch(remaining_weight)
@@ -987,6 +1005,8 @@ pub mod pallet {
 	pub(crate) type DeletionQueueCounter<T: Config> =
 		StorageValue<_, DeletionQueueManager<T>, ValueQuery>;
 
+	/// A migration can span across multiple blocks. This storage defines a cursor to track the
+	/// progress of the migration, enabling us to resume from the last completed position.
 	#[pallet::storage]
 	pub(crate) type MigrationInProgress<T: Config> =
 		StorageValue<_, migration::Cursor, OptionQuery>;

frame/contracts/src/migration.rs

@@ -15,7 +15,46 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! Migration framework for pallets.
+//! Multi-block Migration framework for pallet-contracts.
+//!
+//! This module allows us to define a migration as a sequence of [`MigrationStep`]s that can be
+//! executed across multiple blocks.
+//!
+//! # Usage
+//!
+//! A migration step is defined under `src/migration/vX.rs`, where `X` is the version number.
+//! For example, `vX.rs` defines a migration from version `X - 1` to version `X`.
+//!
+//! ## Example:
+//!
+//! To configure a migration to `v11` for a runtime using `v8` of pallet-contracts on the chain, you
+//! would set the `Migrations` type as follows:
+//!
+//! ```
+//! use pallet_contracts::migration::{v9, v10, v11};
+//! # pub enum Runtime {};
+//! type Migrations = (v9::Migration<Runtime>, v10::Migration<Runtime>, v11::Migration<Runtime>);
+//! ```
+//!
+//! ## Notes:
+//!
+//! - Migrations should always be tested with `try-runtime` before being deployed.
+//! - By testing with `try-runtime` against a live network, you ensure that all migration steps work
+//!   and that you have included the required steps.
+//!
+//! ## Low Level / Implementation Details
+//!
+//! When a migration starts and [`OnRuntimeUpgrade::on_runtime_upgrade`] is called, instead of
+//! performing the actual migration, we set a custom storage item [`MigrationInProgress`].
+//! This storage item defines a [`Cursor`] for the current migration step.
+//!
+//! If the [`MigrationInProgress`] storage item exists, it means a migration is in progress, and its
+//! value holds a cursor for the current migration step. These migration steps are executed during
+//! [`Hooks<BlockNumber>::on_idle`] or when the [`Pallet::migrate`] dispatchable is
+//! called.
+//!
+//! While the migration is in progress, all dispatchables except `migrate`, are blocked, and returns
+//! a `MigrationInProgress` error.
 
 pub mod v10;
 pub mod v11;
@@ -57,7 +96,7 @@ pub enum IsFinished {
 ///
 /// The migration is done in steps. The migration is finished when
 /// `step()` returns `IsFinished::Yes`.
-pub trait Migrate: Codec + MaxEncodedLen + Default {
+pub trait MigrationStep: Codec + MaxEncodedLen + Default {
 	/// Returns the version of the migration.
 	const VERSION: u16;
 
@@ -110,7 +149,7 @@ pub trait Migrate: Codec + MaxEncodedLen + Default {
 #[derive(frame_support::DefaultNoBound, Encode, Decode, MaxEncodedLen)]
 pub struct NoopMigration<const N: u16>;
 
-impl<const N: u16> Migrate for NoopMigration<N> {
+impl<const N: u16> MigrationStep for NoopMigration<N> {
 	const VERSION: u16 = N;
 	fn max_step_weight() -> Weight {
 		Weight::zero()
@@ -122,10 +161,10 @@ impl<const N: u16> Migrate for NoopMigration<N> {
 }
 
 mod private {
-	use crate::migration::Migrate;
+	use crate::migration::MigrationStep;
 	pub trait Sealed {}
 	#[impl_trait_for_tuples::impl_for_tuples(10)]
-	#[tuple_types_custom_trait_bound(Migrate)]
+	#[tuple_types_custom_trait_bound(MigrationStep)]
 	impl Sealed for Tuple {}
 }
 
@@ -136,9 +175,9 @@ mod private {
 pub trait MigrateSequence: private::Sealed {
 	/// Returns the range of versions that this migration can handle.
 	/// Migrations must be ordered by their versions with no gaps.
-	/// The following code will fail to compile:
 	///
 	/// The following code will fail to compile:
+	///
 	/// ```compile_fail
 	///     # use pallet_contracts::{NoopMigration, MigrateSequence};
 	/// 	let _ = <(NoopMigration<1>, NoopMigration<3>)>::VERSION_RANGE;
@@ -172,21 +211,10 @@ pub trait MigrateSequence: private::Sealed {
 
 	/// Returns whether migrating from `in_storage` to `target` is supported.
 	///
-	/// A migration is supported if (in_storage + 1, target) is contained by `VERSION_RANGE`.
+	/// A migration is supported if `VERSION_RANGE` is (in_storage + 1, target).
 	fn is_upgrade_supported(in_storage: StorageVersion, target: StorageVersion) -> bool {
-		if in_storage == target {
-			return true
-		}
-		if in_storage > target {
-			return false
-		}
-
 		let (low, high) = Self::VERSION_RANGE;
-		let Some(first_supported) = low.checked_sub(1) else {
-			return false
-		};
-
-		in_storage >= first_supported && target == high
+		target == high && in_storage + 1 == low
 	}
 }
 
@@ -271,15 +299,13 @@ impl<T: Config> OnRuntimeUpgrade for Migration<T> {
 
 		log::debug!(
 			target: LOG_TARGET,
-			"{}: Range supported {:?}, range requested {:?}",
-			<Pallet<T>>::name(),
-			T::Migrations::VERSION_RANGE,
-			(storage_version, target_version)
+			"Requested migration of {} from {:?}(on-chain storage version) to {:?}(current storage version)",
+			<Pallet<T>>::name(), storage_version, target_version
 		);
 
 		ensure!(
 			T::Migrations::is_upgrade_supported(storage_version, target_version),
-			"Unsupported upgrade"
+			"Unsupported upgrade: VERSION_RANGE should be (on-chain storage version + 1, current storage version)"
 		);
 		Ok(Default::default())
 	}
@@ -306,7 +332,8 @@ pub enum StepResult {
 }
 
 impl<T: Config> Migration<T> {
-	/// Verify that each migration's step of the [`T::Migrations`] sequence fits into `Cursor`.
+	/// Verify that each migration's step of the [`Config::Migrations`] sequence fits into
+	/// `Cursor`.
 	pub(crate) fn integrity_test() {
 		let max_weight = <T as frame_system::Config>::BlockWeights::get().max_block;
 		T::Migrations::integrity_test(max_weight)
@@ -387,7 +414,7 @@ impl<T: Config> Migration<T> {
 }
 
 #[impl_trait_for_tuples::impl_for_tuples(10)]
-#[tuple_types_custom_trait_bound(Migrate)]
+#[tuple_types_custom_trait_bound(MigrationStep)]
 impl MigrateSequence for Tuple {
 	const VERSION_RANGE: (u16, u16) = {
 		let mut versions: (u16, u16) = (0, 0);
@@ -487,7 +514,7 @@ mod test {
 		count: u16,
 	}
 
-	impl<const N: u16> Migrate for MockMigration<N> {
+	impl<const N: u16> MigrationStep for MockMigration<N> {
 		const VERSION: u16 = N;
 		fn max_step_weight() -> Weight {
 			Weight::from_all(1)
@@ -512,30 +539,9 @@ mod test {
 	#[test]
 	fn is_upgrade_supported_works() {
 		type Migrations = (MockMigration<9>, MockMigration<10>, MockMigration<11>);
-
-		[(1, 1), (8, 11), (9, 11)].into_iter().for_each(|(from, to)| {
-			assert!(
-				Migrations::is_upgrade_supported(
-					StorageVersion::new(from),
-					StorageVersion::new(to)
-				),
-				"{} -> {} is supported",
-				from,
-				to
-			)
-		});
-
-		[(1, 0), (0, 3), (7, 11), (8, 10)].into_iter().for_each(|(from, to)| {
-			assert!(
-				!Migrations::is_upgrade_supported(
-					StorageVersion::new(from),
-					StorageVersion::new(to)
-				),
-				"{} -> {} is not supported",
-				from,
-				to
-			)
-		});
+		assert!(Migrations::is_upgrade_supported(StorageVersion::new(8), StorageVersion::new(11)));
+		assert!(!Migrations::is_upgrade_supported(StorageVersion::new(9), StorageVersion::new(11)));
+		assert!(!Migrations::is_upgrade_supported(StorageVersion::new(8), StorageVersion::new(12)));
 	}
 
 	#[test]

frame/contracts/src/migration/v10.rs

@@ -16,12 +16,12 @@
 // limitations under the License.
 
 //! Don't rely on reserved balances keeping an account alive
-//! See <https://github.com/paritytech/substrate/pull/13370>.
+//! See <https://github.com/paritytech/substrate/pull/13369>.
 
 use crate::{
 	address::AddressGenerator,
 	exec::AccountIdOf,
-	migration::{IsFinished, Migrate},
+	migration::{IsFinished, MigrationStep},
 	weights::WeightInfo,
 	BalanceOf, CodeHash, Config, Pallet, TrieId, Weight, LOG_TARGET,
 };
@@ -69,7 +69,7 @@ mod old {
 }
 
 #[cfg(feature = "runtime-benchmarks")]
-pub fn store_old_contrat_info<T: Config>(account: T::AccountId, info: crate::ContractInfo<T>) {
+pub fn store_old_contract_info<T: Config>(account: T::AccountId, info: crate::ContractInfo<T>) {
 	let info = old::ContractInfo {
 		trie_id: info.trie_id,
 		code_hash: info.code_hash,
@@ -117,7 +117,7 @@ pub struct Migration<T: Config> {
 type ContractInfoOf<T: Config> =
 	StorageMap<Pallet<T>, Twox64Concat, <T as frame_system::Config>::AccountId, ContractInfo<T>>;
 
-impl<T: Config> Migrate for Migration<T> {
+impl<T: Config> MigrationStep for Migration<T> {
 	const VERSION: u16 = 10;
 
 	fn max_step_weight() -> Weight {
@@ -181,14 +181,14 @@ impl<T: Config> Migrate for Migration<T> {
 			})
 			// If it fails we fallback to minting the ED.
 			.unwrap_or_else(|err| {
-				log::error!(target: LOG_TARGET, "Failed to transfer ED, reason: {:?}", err);
+				log::error!(target: LOG_TARGET, "Failed to transfer the base deposit, reason: {:?}", err);
 				T::Currency::deposit_creating(&deposit_account, min_balance);
 				min_balance
 			});
 
 			// Calculate the new base_deposit to store in the contract:
-			// Ideally: it should be the same as the old one
-			// Ideally, it should be at least 2xED (for the contract and deposit account).
+			// Ideally, it should be the same as the old one
+			// Ideally, it should be at least 2xED (for the contract and deposit accounts).
 			// It can't be more than the `new_deposit`.
 			let new_base_deposit = min(
 				max(contract.storage_base_deposit, min_balance.saturating_add(min_balance)),
@@ -240,8 +240,8 @@ impl<T: Config> Migrate for Migration<T> {
 
 	#[cfg(feature = "try-runtime")]
 	fn post_upgrade_step(state: Vec<u8>) -> Result<(), TryRuntimeError> {
-		let sample =
-			<Vec<(T::AccountId, old::ContractInfo<T>)> as Decode>::decode(&mut &state[..]).unwrap();
+		let sample = <Vec<(T::AccountId, old::ContractInfo<T>)> as Decode>::decode(&mut &state[..])
+			.expect("pre_upgrade_step provides a valid state; qed");
 
 		log::debug!(target: LOG_TARGET, "Validating sample of {} contracts", sample.len());
 		for (account, old_contract) in sample {

frame/contracts/src/migration/v11.rs

@@ -19,7 +19,7 @@
 //! See <https://github.com/paritytech/substrate/pull/13702>.
 
 use crate::{
-	migration::{IsFinished, Migrate},
+	migration::{IsFinished, MigrationStep},
 	weights::WeightInfo,
 	Config, Pallet, TrieId, Weight, LOG_TARGET,
 };
@@ -69,7 +69,7 @@ pub struct Migration<T: Config> {
 	_phantom: PhantomData<T>,
 }
 
-impl<T: Config> Migrate for Migration<T> {
+impl<T: Config> MigrationStep for Migration<T> {
 	const VERSION: u16 = 11;
 
 	// It would be more correct to make our use the now removed [DeletionQueueDepth](https://github.com/paritytech/substrate/pull/13702/files#diff-70e9723e9db62816e35f6f885b6770a8449c75a6c2733e9fa7a245fe52c4656c)
@@ -121,7 +121,8 @@ impl<T: Config> Migrate for Migration<T> {
 
 	#[cfg(feature = "try-runtime")]
 	fn post_upgrade_step(state: Vec<u8>) -> Result<(), TryRuntimeError> {
-		let len = <u32 as Decode>::decode(&mut &state[..]).unwrap();
+		let len = <u32 as Decode>::decode(&mut &state[..])
+			.expect("pre_upgrade_step provides a valid state; qed");
 		let counter = <DeletionQueueCounter<T>>::get();
 		ensure!(counter.insert_counter == len, "invalid insert counter");
 		ensure!(counter.delete_counter == 0, "invalid delete counter");