Repository: paritytech/substrate
File: frame/contracts/src/migration.rs (Rust)

Code before:
	/// Returns whether migrating from `in_storage` to `target` is supported.
	///
	/// A migration is supported if (in_storage + 1, target) is contained by `VERSION_RANGE`.
	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
	}
}

/// Performs all necessary migrations based on `StorageVersion`.
///
/// If `RUN_ALL_STEPS == true` and `try-runtime` is enabled, this will run all the migrations inside
/// `on_runtime_upgrade`. This should be set to false in tests that want to ensure the step by step
/// migration works.
pub struct Migration<T: Config, const RUN_ALL_STEPS: bool = true>(PhantomData<T>);

#[cfg(feature = "try-runtime")]
impl<T: Config, const RUN_ALL_STEPS: bool> Migration<T, RUN_ALL_STEPS> {
	fn run_all_steps() -> Result<(), TryRuntimeError> {
		let mut weight = Weight::zero();
		let name = <Pallet<T>>::name();
		loop {
			let in_progress_version = <Pallet<T>>::on_chain_storage_version() + 1;
			let state = T::Migrations::pre_upgrade_step(in_progress_version)?;
			let (status, w) = Self::migrate(Weight::MAX);
			weight.saturating_accrue(w);
			log::info!(
				target: LOG_TARGET,
				"{name}: Migration step {:?} weight = {}",
				in_progress_version,
				weight
			);
			T::Migrations::post_upgrade_step(in_progress_version, state)?;
			if matches!(status, MigrateResult::Completed) {
				break
			}
		}

		let name = <Pallet<T>>::name();
		log::info!(target: LOG_TARGET, "{name}: Migration steps weight = {}", weight);

Review comment (suggestion):
question: do we also run the tests in CI without try_runtime? in which case I think it would be fine to just disable the tests using this struct and not add this extra RUN_ALL_STEPS

If not, maybe we change the name to TRY_ALL_STEPS or TEST_ALL_STEPS to make it clear that this is just intended for testing purpose.

Code after:
	/// Returns whether migrating from `in_storage` to `target` is supported.
	///
	/// A migration is supported if (in_storage + 1, target) is contained by `VERSION_RANGE`.
	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
	}
}

/// Performs all necessary migrations based on `StorageVersion`.
///
/// If `TEST_ALL_STEPS == true` and `try-runtime` is enabled, this will run all the migrations
/// inside `on_runtime_upgrade`. This should be set to false in tests that want to ensure the step
/// by step migration works.
pub struct Migration<T: Config, const TEST_ALL_STEPS: bool = true>(PhantomData<T>);

#[cfg(feature = "try-runtime")]
impl<T: Config, const TEST_ALL_STEPS: bool> Migration<T, TEST_ALL_STEPS> {
	fn run_all_steps() -> Result<(), TryRuntimeError> {
		let mut weight = Weight::zero();
		let name = <Pallet<T>>::name();
		loop {
			let in_progress_version = <Pallet<T>>::on_chain_storage_version() + 1;
			let state = T::Migrations::pre_upgrade_step(in_progress_version)?;
			let (status, w) = Self::migrate(Weight::MAX);
			weight.saturating_accrue(w);
			log::info!(
				target: LOG_TARGET,
				"{name}: Migration step {:?} weight = {}",
				in_progress_version,
				weight
			);
			T::Migrations::post_upgrade_step(in_progress_version, state)?;
			if matches!(status, MigrateResult::Completed) {
				break
			}
		}

		let name = <Pallet<T>>::name();
		log::info!(target: LOG_TARGET, "{name}: Migration steps weight = {}", weight);