Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Migration hook fixes #14174

Merged
merged 13 commits into from
May 23, 2023
4 changes: 4 additions & 0 deletions frame/child-bounties/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@ pub mod pallet {

use super::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(0);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
pub struct Pallet<T>(_);

#[pallet::config]
Expand Down
53 changes: 35 additions & 18 deletions frame/nomination-pools/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,8 @@ pub mod v3 {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
"the on_chain version is equal or more than the current one"
Pallet::<T>::current_storage_version() >= Pallet::<T>::on_chain_storage_version(),
"nomination-pools::migration::v3: on_chain version is greater than current version"
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
);
Ok(Vec::new())
}
Expand All @@ -449,7 +449,10 @@ pub mod v3 {
Metadata::<T>::iter_keys().all(|id| BondedPools::<T>::contains_key(&id)),
"not all of the stale metadata has been removed"
);
ensure!(Pallet::<T>::on_chain_storage_version() == 3, "wrong storage version");
ensure!(
Pallet::<T>::on_chain_storage_version() >= 3,
"nomination-pools::migration::v3: wrong storage version"
);
Ok(())
}
}
Expand Down Expand Up @@ -537,8 +540,8 @@ pub mod v4 {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
"the on_chain version is equal or more than the current one"
Pallet::<T>::current_storage_version() >= Pallet::<T>::on_chain_storage_version(),
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
"nomination-pools::migration::v3tov5: on_chain version is greater than current version"
);
Ok(Vec::new())
}
Expand All @@ -547,17 +550,28 @@ pub mod v4 {
fn post_upgrade(_: Vec<u8>) -> Result<(), &'static str> {
// ensure all BondedPools items now contain an `inner.commission: Commission` field.
ensure!(
BondedPools::<T>::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::<T>::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"
);
Comment on lines 559 to 573
Copy link
Contributor Author

@liamaharon liamaharon May 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensure! was failing on Rococo I guess because commission became no longer None for everything after people started using the pallet.

But the checks seem kind of like a noop (checking is_some || is_none). Can we just remove them?

ensure!(
GlobalMaxCommission::<T>::get() == Some(U::get()),
"global maximum commission error"
);
ensure!(Pallet::<T>::on_chain_storage_version() == 4, "wrong storage version");
ensure!(
Pallet::<T>::on_chain_storage_version() >= 4,
"nomination-pools::migration::v4: wrong storage version"
);
Ok(())
}
}
Expand Down Expand Up @@ -622,8 +636,8 @@ pub mod v5 {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
"the on_chain version is equal or more than the current one"
Pallet::<T>::current_storage_version() >= Pallet::<T>::on_chain_storage_version(),
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
"nomination-pools::migration::v5: on_chain version is greater than current version"
);

let rpool_keys = RewardPools::<T>::iter_keys().count();
Expand Down Expand Up @@ -675,13 +689,16 @@ pub mod v5 {
// `total_commission_claimed` field.
ensure!(
RewardPools::<T>::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::<T>::on_chain_storage_version() == 5, "wrong storage version");
ensure!(
Pallet::<T>::on_chain_storage_version() >= 5,
"nomination-pools::migration::v5: wrong storage version"
);

// These should not have been touched - just in case.
ensure!(
Expand Down
15 changes: 7 additions & 8 deletions frame/offences/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ pub mod v1 {
impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
let onchain = Pallet::<T>::on_chain_storage_version();
ensure!(onchain < 1, "pallet_offences::MigrateToV1 migration can be deleted");
ensure!(
Pallet::<T>::current_storage_version() >= Pallet::<T>::on_chain_storage_version(),
"offences::migration::v1: on_chain version is greater than current version"
);
liamaharon marked this conversation as resolved.
Show resolved Hide resolved

log::info!(
target: LOG_TARGET,
Expand All @@ -65,19 +67,16 @@ pub mod v1 {
}

fn on_runtime_upgrade() -> Weight {
let onchain = Pallet::<T>::on_chain_storage_version();

if onchain > 0 {
if Pallet::<T>::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::<T>::clear(u32::MAX, None).unique as u64;
let weight = T::DbWeight::get().reads_writes(keys_removed, keys_removed);

StorageVersion::new(1).put::<Pallet<T>>();

weight
// + 1 for reading/writing the new storage version
T::DbWeight::get().reads_writes(keys_removed + 1, keys_removed + 1)
}

#[cfg(feature = "try-runtime")]
Expand Down