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
Merged

Migration hook fixes #14174

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5c35454
fix offences pre_upgrade hook
liamaharon May 18, 2023
7224dfa
identify source of ensure! failures
liamaharon May 18, 2023
4f9f99a
stop migration hooks breaking post migration
liamaharon May 18, 2023
42fbea2
add childbounties storage version
liamaharon May 18, 2023
25c7921
init child bounties version to zero
liamaharon May 18, 2023
a868c55
Merge branch 'master' of github.com:paritytech/substrate into liam-mi…
liamaharon May 22, 2023
716f36e
Update frame/child-bounties/src/lib.rs
liamaharon May 22, 2023
c6cee45
remove redundant preupgrade version checks
liamaharon May 22, 2023
e9d2ae8
update test
liamaharon May 22, 2023
226450e
fix nom pools v3 migration
liamaharon May 22, 2023
3031bca
Merge branch 'master' of github.com:paritytech/substrate into liam-mi…
liamaharon May 23, 2023
4108d95
kick ci
liamaharon May 23, 2023
cd536b6
kick ci
liamaharon May 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions frame/child-bounties/src/lib.rs
View file
Open in desktop
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
View file
Open in desktop
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
Member Author

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
View file
Open in desktop
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