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

Commit

Permalink
Migration hook fixes (#14174)
Browse files Browse the repository at this point in the history 
* 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 <git@kchr.de>

* remove redundant preupgrade version checks

* update test

* fix nom pools v3 migration

* kick ci

* kick ci

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
  • Loading branch information
@liamaharon @bkchr @gpestana
2 people authored and gpestana committed May 27, 2023
1 parent 0ed6d19 commit f0e53d5
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 43 deletions.
70 changes: 37 additions & 33 deletions frame/nomination-pools/src/migration.rs
View file
Expand Up @@ -416,14 +416,14 @@ pub mod v3 {
let current = Pallet::<T>::current_storage_version();
let onchain = Pallet::<T>::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::<T>::iter_keys()
Expand All @@ -437,7 +437,7 @@ pub mod v3 {
metadata_removed += 1;
Metadata::<T>::remove(&id);
});
current.put::<Pallet<T>>();
StorageVersion::new(3).put::<Pallet<T>>();
// metadata iterated + bonded pools read + a storage version read
let total_reads = metadata_iterated * 2 + 1;
// metadata removed + a storage version write
Expand All @@ -451,10 +451,6 @@ pub mod v3 {

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
ensure!(
Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
"the on_chain version is equal or more than the current one"
);
Ok(Vec::new())
}

Expand All @@ -464,7 +460,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 @@ -551,28 +550,35 @@ pub mod v4 {

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
ensure!(
Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
"the on_chain version is equal or more than the current one"
);
Ok(Vec::new())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_: Vec<u8>) -> Result<(), TryRuntimeError> {
// 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"
);
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 @@ -636,11 +642,6 @@ pub mod v5 {

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
ensure!(
Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
"the on_chain version is equal or more than the current one"
);

let rpool_keys = RewardPools::<T>::iter_keys().count();
let rpool_values = RewardPools::<T>::iter_values().count();
if rpool_keys != rpool_values {
Expand Down Expand Up @@ -690,13 +691,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
14 changes: 4 additions & 10 deletions frame/offences/src/migration.rs
View file
Expand Up @@ -54,9 +54,6 @@ pub mod v1 {
impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
let onchain = Pallet::<T>::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: {}",
Expand All @@ -67,19 +64,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 Expand Up @@ -147,7 +141,7 @@ mod test {
ext.execute_with(|| {
assert_eq!(
v1::MigrateToV1::<T>::on_runtime_upgrade(),
<T as frame_system::Config>::DbWeight::get().reads_writes(1, 1),
<T as frame_system::Config>::DbWeight::get().reads_writes(2, 2),
);

assert!(<v0::ReportsByKindIndex<T>>::iter_values().count() == 0);
Expand Down

0 comments on commit f0e53d5

Please sign in to comment.