From ad5163ba93a7e142fd6fb2b9ed975bff9c29c479 Mon Sep 17 00:00:00 2001
From: Liam Aharon <liam.aharon@hotmail.com>
Date: Mon, 30 Oct 2023 21:55:05 +1100
Subject: [PATCH] contracts migration: remove unnecessary panics (#2079)

Runtime migration CI is currently failing
(https://gitlab.parity.io/parity/mirrors/polkadot-sdk/builds/4122083)
for the contracts testnet due to unnecessary panicing in a `pre_upgrade`
hook.

Soon idempotency will be enforced
https://github.com/paritytech/try-runtime-cli/issues/42, in the mean
time we need to manually fix these issues as they arise.

---

also removes backticks from the string in `echo`, which caused a
'command not found' error in ci output
---
 .gitlab/pipeline/check.yml                 |  2 +-
 substrate/frame/contracts/src/migration.rs | 37 +++++++++++-----------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml
index cd26003d88..ce3a615579 100644
--- a/.gitlab/pipeline/check.yml
+++ b/.gitlab/pipeline/check.yml
@@ -107,7 +107,7 @@ check-rust-feature-propagation:
       echo "---------- Building ${PACKAGE} runtime ----------"
       time cargo build --release --locked -p "$PACKAGE" --features try-runtime
 
-      echo "---------- Executing `on-runtime-upgrade` for ${NETWORK} ----------"
+      echo "---------- Executing on-runtime-upgrade for ${NETWORK} ----------"
       time ./try-runtime \
           --runtime ./target/release/wbuild/"$PACKAGE"/"$WASM" \
           on-runtime-upgrade --checks=pre-and-post ${EXTRA_ARGS} live --uri ${URI}
diff --git a/substrate/frame/contracts/src/migration.rs b/substrate/frame/contracts/src/migration.rs
index 2714620731..1873ef2765 100644
--- a/substrate/frame/contracts/src/migration.rs
+++ b/substrate/frame/contracts/src/migration.rs
@@ -263,14 +263,14 @@ impl<T: Config, const TEST_ALL_STEPS: bool> Migration<T, TEST_ALL_STEPS> {
 impl<T: Config, const TEST_ALL_STEPS: bool> OnRuntimeUpgrade for Migration<T, TEST_ALL_STEPS> {
 	fn on_runtime_upgrade() -> Weight {
 		let name = <Pallet<T>>::name();
-		let latest_version = <Pallet<T>>::current_storage_version();
-		let storage_version = <Pallet<T>>::on_chain_storage_version();
+		let current_version = <Pallet<T>>::current_storage_version();
+		let on_chain_version = <Pallet<T>>::on_chain_storage_version();
 
-		if storage_version == latest_version {
+		if on_chain_version == current_version {
 			log::warn!(
 				target: LOG_TARGET,
 				"{name}: No Migration performed storage_version = latest_version = {:?}",
-				&storage_version
+				&on_chain_version
 			);
 			return T::WeightInfo::on_runtime_upgrade_noop()
 		}
@@ -281,7 +281,7 @@ impl<T: Config, const TEST_ALL_STEPS: bool> OnRuntimeUpgrade for Migration<T, TE
 			log::warn!(
 				target: LOG_TARGET,
 				"{name}: Migration already in progress {:?}",
-				&storage_version
+				&on_chain_version
 			);
 
 			return T::WeightInfo::on_runtime_upgrade_in_progress()
@@ -289,10 +289,10 @@ impl<T: Config, const TEST_ALL_STEPS: bool> OnRuntimeUpgrade for Migration<T, TE
 
 		log::info!(
 			target: LOG_TARGET,
-			"{name}: Upgrading storage from {storage_version:?} to {latest_version:?}.",
+			"{name}: Upgrading storage from {on_chain_version:?} to {current_version:?}.",
 		);
 
-		let cursor = T::Migrations::new(storage_version + 1);
+		let cursor = T::Migrations::new(on_chain_version + 1);
 		MigrationInProgress::<T>::set(Some(cursor));
 
 		#[cfg(feature = "try-runtime")]
@@ -308,24 +308,25 @@ impl<T: Config, const TEST_ALL_STEPS: bool> OnRuntimeUpgrade for Migration<T, TE
 		// We can't really do much here as our migrations do not happen during the runtime upgrade.
 		// Instead, we call the migrations `pre_upgrade` and `post_upgrade` hooks when we iterate
 		// over our migrations.
-		let storage_version = <Pallet<T>>::on_chain_storage_version();
-		let target_version = <Pallet<T>>::current_storage_version();
+		let on_chain_version = <Pallet<T>>::on_chain_storage_version();
+		let current_version = <Pallet<T>>::current_storage_version();
 
-		ensure!(
-			storage_version != target_version,
-			"No upgrade: Please remove this migration from your runtime upgrade configuration."
-		);
+		if on_chain_version == current_version {
+			log::warn!(
+				target: LOG_TARGET,
+				"No upgrade: Please remove this migration from your Migrations tuple"
+			)
+		}
 
 		log::debug!(
 			target: LOG_TARGET,
 			"Requested migration of {} from {:?}(on-chain storage version) to {:?}(current storage version)",
-			<Pallet<T>>::name(), storage_version, target_version
+			<Pallet<T>>::name(), on_chain_version, current_version
 		);
 
-		ensure!(
-			T::Migrations::is_upgrade_supported(storage_version, target_version),
-			"Unsupported upgrade: VERSION_RANGE should be (on-chain storage version + 1, current storage version)"
-		);
+		if !T::Migrations::is_upgrade_supported(on_chain_version, current_version) {
+			log::warn!(target: LOG_TARGET, "Unsupported upgrade: VERSION_RANGE should be (on-chain storage version + 1, current storage version)")
+		}
 		Ok(Default::default())
 	}