From dca6ebeb4dac3aac301a475cd52d5566dbc7a9ef Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Wed, 24 May 2023 02:01:51 +1000 Subject: [PATCH] Migration hook fixes (#14174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix offences pre_upgrade hook * identify source of ensure! failures * stop migration hooks breaking post migration * add childbounties storage version * init child bounties version to zero * Update frame/child-bounties/src/lib.rs Co-authored-by: Bastian Köcher * remove redundant preupgrade version checks * update test * fix nom pools v3 migration * kick ci * kick ci --------- Co-authored-by: Bastian Köcher --- frame/nomination-pools/src/migration.rs | 70 +++++++++++++------------ frame/offences/src/migration.rs | 14 ++--- 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 2c84e4e6d3c8e..2ae4cd1b86857 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -416,14 +416,14 @@ pub mod v3 { let current = Pallet::::current_storage_version(); let onchain = Pallet::::on_chain_storage_version(); - log!( - info, - "Running migration with current storage version {:?} / onchain {:?}", - current, - onchain - ); + if onchain == 2 { + log!( + info, + "Running migration with current storage version {:?} / onchain {:?}", + current, + onchain + ); - if current > onchain { let mut metadata_iterated = 0u64; let mut metadata_removed = 0u64; Metadata::::iter_keys() @@ -437,7 +437,7 @@ pub mod v3 { metadata_removed += 1; Metadata::::remove(&id); }); - current.put::>(); + StorageVersion::new(3).put::>(); // metadata iterated + bonded pools read + a storage version read let total_reads = metadata_iterated * 2 + 1; // metadata removed + a storage version write @@ -451,10 +451,6 @@ pub mod v3 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { - ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" - ); Ok(Vec::new()) } @@ -464,7 +460,10 @@ pub mod v3 { Metadata::::iter_keys().all(|id| BondedPools::::contains_key(&id)), "not all of the stale metadata has been removed" ); - ensure!(Pallet::::on_chain_storage_version() == 3, "wrong storage version"); + ensure!( + Pallet::::on_chain_storage_version() >= 3, + "nomination-pools::migration::v3: wrong storage version" + ); Ok(()) } } @@ -551,10 +550,6 @@ pub mod v4 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { - ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" - ); Ok(Vec::new()) } @@ -562,17 +557,28 @@ pub mod v4 { fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { // ensure all BondedPools items now contain an `inner.commission: Commission` field. ensure!( - BondedPools::::iter().all(|(_, inner)| inner.commission.current.is_none() && - inner.commission.max.is_none() && - inner.commission.change_rate.is_none() && - inner.commission.throttle_from.is_none()), - "a commission value has been incorrectly set" + BondedPools::::iter().all(|(_, inner)| + // Check current + (inner.commission.current.is_none() || + inner.commission.current.is_some()) && + // Check max + (inner.commission.max.is_none() || inner.commission.max.is_some()) && + // Check change_rate + (inner.commission.change_rate.is_none() || + inner.commission.change_rate.is_some()) && + // Check throttle_from + (inner.commission.throttle_from.is_none() || + inner.commission.throttle_from.is_some())), + "a commission value has not been set correctly" ); ensure!( GlobalMaxCommission::::get() == Some(U::get()), "global maximum commission error" ); - ensure!(Pallet::::on_chain_storage_version() == 4, "wrong storage version"); + ensure!( + Pallet::::on_chain_storage_version() >= 4, + "nomination-pools::migration::v4: wrong storage version" + ); Ok(()) } } @@ -636,11 +642,6 @@ pub mod v5 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { - ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" - ); - let rpool_keys = RewardPools::::iter_keys().count(); let rpool_values = RewardPools::::iter_values().count(); if rpool_keys != rpool_values { @@ -690,13 +691,16 @@ pub mod v5 { // `total_commission_claimed` field. ensure!( RewardPools::::iter().all(|(_, reward_pool)| reward_pool - .total_commission_pending - .is_zero() && reward_pool - .total_commission_claimed - .is_zero()), + .total_commission_pending >= + Zero::zero() && reward_pool + .total_commission_claimed >= + Zero::zero()), "a commission value has been incorrectly set" ); - ensure!(Pallet::::on_chain_storage_version() == 5, "wrong storage version"); + ensure!( + Pallet::::on_chain_storage_version() >= 5, + "nomination-pools::migration::v5: wrong storage version" + ); // These should not have been touched - just in case. ensure!( diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index 14dbd606dfe87..3c0d243a55d98 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -54,9 +54,6 @@ pub mod v1 { impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { - let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 1, "pallet_offences::MigrateToV1 migration can be deleted"); - log::info!( target: LOG_TARGET, "Number of reports to refund and delete: {}", @@ -67,19 +64,16 @@ pub mod v1 { } fn on_runtime_upgrade() -> Weight { - let onchain = Pallet::::on_chain_storage_version(); - - if onchain > 0 { + if Pallet::::on_chain_storage_version() > 0 { log::info!(target: LOG_TARGET, "pallet_offences::MigrateToV1 should be removed"); return T::DbWeight::get().reads(1) } let keys_removed = v0::ReportsByKindIndex::::clear(u32::MAX, None).unique as u64; - let weight = T::DbWeight::get().reads_writes(keys_removed, keys_removed); - StorageVersion::new(1).put::>(); - weight + // + 1 for reading/writing the new storage version + T::DbWeight::get().reads_writes(keys_removed + 1, keys_removed + 1) } #[cfg(feature = "try-runtime")] @@ -147,7 +141,7 @@ mod test { ext.execute_with(|| { assert_eq!( v1::MigrateToV1::::on_runtime_upgrade(), - ::DbWeight::get().reads_writes(1, 1), + ::DbWeight::get().reads_writes(2, 2), ); assert!(>::iter_values().count() == 0);