Skip to content

Commit

Permalink
contracts migration: remove unnecessary panics (#2079)
Browse files Browse the repository at this point in the history
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
paritytech/try-runtime-cli#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
  • Loading branch information
liamaharon authored Oct 30, 2023
1 parent 0aeab38 commit ad5163b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .gitlab/pipeline/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
37 changes: 19 additions & 18 deletions substrate/frame/contracts/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -281,18 +281,18 @@ 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()
}

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")]
Expand All @@ -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())
}

Expand Down

0 comments on commit ad5163b

Please sign in to comment.