From 5c354546c729f006f14d1331f29f75fffb80e353 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 18 May 2023 13:43:15 +0400 Subject: [PATCH 01/11] fix offences pre_upgrade hook --- frame/offences/src/migration.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index 07bd68407d378..ad37ca4240cbc 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -52,8 +52,10 @@ pub mod v1 { impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 1, "pallet_offences::MigrateToV1 migration can be deleted"); + ensure!( + Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), + "Offences: the on_chain version is equal or more than the current one" + ); log::info!( target: LOG_TARGET, From 7224dfa852a8eee2b396c5a6049394e9f9a77d4e Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 18 May 2023 13:51:51 +0400 Subject: [PATCH 02/11] identify source of ensure! failures --- frame/nomination-pools/src/migration.rs | 6 +++--- frame/offences/src/migration.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 45d6424119900..f53d269ee0afe 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -438,7 +438,7 @@ pub mod v3 { fn pre_upgrade() -> Result, &'static str> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" + "nomination-pools::migration::v3: the on_chain version is equal or more than the current one" ); Ok(Vec::new()) } @@ -538,7 +538,7 @@ pub mod v4 { fn pre_upgrade() -> Result, &'static str> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" + "nomination-pools::migration::v3tov5: the on_chain version is equal or more than the current one" ); Ok(Vec::new()) } @@ -623,7 +623,7 @@ pub mod v5 { fn pre_upgrade() -> Result, &'static str> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "the on_chain version is equal or more than the current one" + "nomination-pools::migration::v5: the on_chain version is equal or more than the current one" ); let rpool_keys = RewardPools::::iter_keys().count(); diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index ad37ca4240cbc..1212c4badae3b 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -54,7 +54,7 @@ pub mod v1 { fn pre_upgrade() -> Result, &'static str> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "Offences: the on_chain version is equal or more than the current one" + "offences::migration::v1: the on_chain version is equal or more than the current one" ); log::info!( From 4f9f99a023d01ade13e4867762a1a209cf604ffb Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 18 May 2023 14:47:34 +0400 Subject: [PATCH 03/11] stop migration hooks breaking post migration --- frame/nomination-pools/src/migration.rs | 53 ++++++++++++++++--------- frame/offences/src/migration.rs | 13 +++--- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index f53d269ee0afe..0c76de8ec2fb0 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -437,8 +437,8 @@ pub mod v3 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "nomination-pools::migration::v3: the on_chain version is equal or more than the current one" + Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), + "nomination-pools::migration::v3: on_chain version is greater than current version" ); Ok(Vec::new()) } @@ -449,7 +449,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(()) } } @@ -537,8 +540,8 @@ pub mod v4 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "nomination-pools::migration::v3tov5: the on_chain version is equal or more than the current one" + Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), + "nomination-pools::migration::v3tov5: on_chain version is greater than current version" ); Ok(Vec::new()) } @@ -547,17 +550,28 @@ pub mod v4 { fn post_upgrade(_: Vec) -> Result<(), &'static str> { // 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(()) } } @@ -622,8 +636,8 @@ pub mod v5 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "nomination-pools::migration::v5: the on_chain version is equal or more than the current one" + Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), + "nomination-pools::migration::v5: on_chain version is greater than current version" ); let rpool_keys = RewardPools::::iter_keys().count(); @@ -675,13 +689,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 1212c4badae3b..2ee0a381d0bc4 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -53,8 +53,8 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { ensure!( - Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), - "offences::migration::v1: the on_chain version is equal or more than the current one" + Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), + "offences::migration::v1: on_chain version is greater than current version" ); log::info!( @@ -67,19 +67,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")] From 42fbea2b6b974946c78b2c5846a67e604ce6fd48 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 18 May 2023 16:36:03 +0400 Subject: [PATCH 04/11] add childbounties storage version --- frame/child-bounties/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frame/child-bounties/src/lib.rs b/frame/child-bounties/src/lib.rs index 094b41822c433..a3e5140efe79d 100644 --- a/frame/child-bounties/src/lib.rs +++ b/frame/child-bounties/src/lib.rs @@ -129,7 +129,11 @@ pub mod pallet { use super::*; + /// The current storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::config] From 25c79215a2381e09697009c1b8f6774ea707943b Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 18 May 2023 16:56:11 +0400 Subject: [PATCH 05/11] init child bounties version to zero --- frame/child-bounties/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/child-bounties/src/lib.rs b/frame/child-bounties/src/lib.rs index a3e5140efe79d..45a423347a632 100644 --- a/frame/child-bounties/src/lib.rs +++ b/frame/child-bounties/src/lib.rs @@ -130,7 +130,7 @@ pub mod pallet { use super::*; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From 716f36eb85b5190f4564944c45de7d41daf2c6b4 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Mon, 22 May 2023 18:17:15 +1000 Subject: [PATCH 06/11] Update frame/child-bounties/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/child-bounties/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frame/child-bounties/src/lib.rs b/frame/child-bounties/src/lib.rs index 45a423347a632..094b41822c433 100644 --- a/frame/child-bounties/src/lib.rs +++ b/frame/child-bounties/src/lib.rs @@ -129,11 +129,7 @@ pub mod pallet { use super::*; - /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); - #[pallet::pallet] - #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::config] From c6cee45d9cf6c12ea0ea76e384621b7f5ff4afdb Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Mon, 22 May 2023 12:50:32 +0400 Subject: [PATCH 07/11] remove redundant preupgrade version checks --- frame/nomination-pools/src/migration.rs | 13 ------------- frame/offences/src/migration.rs | 5 ----- 2 files changed, 18 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 0c76de8ec2fb0..c3ca6819bd8a3 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -436,10 +436,6 @@ pub mod v3 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - ensure!( - Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), - "nomination-pools::migration::v3: on_chain version is greater than current version" - ); Ok(Vec::new()) } @@ -539,10 +535,6 @@ pub mod v4 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - ensure!( - Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), - "nomination-pools::migration::v3tov5: on_chain version is greater than current version" - ); Ok(Vec::new()) } @@ -635,11 +627,6 @@ pub mod v5 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - ensure!( - Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), - "nomination-pools::migration::v5: on_chain version is greater than current version" - ); - let rpool_keys = RewardPools::::iter_keys().count(); let rpool_values = RewardPools::::iter_values().count(); if rpool_keys != rpool_values { diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index 2ee0a381d0bc4..febe9aaa82887 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -52,11 +52,6 @@ pub mod v1 { impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { - ensure!( - Pallet::::current_storage_version() >= Pallet::::on_chain_storage_version(), - "offences::migration::v1: on_chain version is greater than current version" - ); - log::info!( target: LOG_TARGET, "Number of reports to refund and delete: {}", From e9d2ae83fc6195fe855b7a16009109ac3ca885de Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Mon, 22 May 2023 13:14:37 +0400 Subject: [PATCH 08/11] update test --- frame/offences/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index febe9aaa82887..e6e9c2dec48ec 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -139,7 +139,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); From 226450ec7269a160e0aa8e6a549a71b39dd6106c Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Mon, 22 May 2023 17:45:25 +0400 Subject: [PATCH 09/11] fix nom pools v3 migration --- frame/nomination-pools/src/migration.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index c3ca6819bd8a3..04d2941635bb0 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -401,14 +401,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() @@ -422,7 +422,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 From 4108d95572a65f87a43d1b7e9cb251ff528b0b4e Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 23 May 2023 12:01:56 +0400 Subject: [PATCH 10/11] kick ci --- frame/multisig/src/migrations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 298e73c5d7576..96d4d64e53176 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -48,7 +48,7 @@ pub mod v1 { ensure!(onchain < 1, "this migration can be deleted"); - log!(info, "Number of calls to refund and delete: {}", Calls::::iter().count()); + log!(info, "Numberof calls to refund and delete: {}", Calls::::iter().count()); Ok(Vec::new()) } From cd536b6ae3bfafa23bb1bb5199d58dcfd8cb9d78 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 23 May 2023 12:02:30 +0400 Subject: [PATCH 11/11] kick ci --- frame/multisig/src/migrations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 96d4d64e53176..298e73c5d7576 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -48,7 +48,7 @@ pub mod v1 { ensure!(onchain < 1, "this migration can be deleted"); - log!(info, "Numberof calls to refund and delete: {}", Calls::::iter().count()); + log!(info, "Number of calls to refund and delete: {}", Calls::::iter().count()); Ok(Vec::new()) }