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

Use #[pallet::storage_version] for pallet staking #12728

Merged
merged 15 commits into from
Jan 3, 2023
Merged

Use #[pallet::storage_version] for pallet staking #12728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
805ea9a
delete releases
Ank4n Nov 17, 2022
4c71f5a
use standard pallet storage version
Ank4n Nov 17, 2022
59b0689
migrate to standard storage version for staking
Ank4n Nov 17, 2022
2a8dcc1
not compiling
Ank4n Nov 17, 2022
b3b1090
keep old releases enum around for decoding
Ank4n Nov 19, 2022
90f17a9
fix releases
Ank4n Nov 20, 2022
143eecd
Merge branch 'master' into ankan/12041-staking-migration-versioning
Ank4n Nov 20, 2022
e5d1f70
rename old releases
Ank4n Nov 24, 2022
7edb2a0
Merge branch 'master' into ankan/12041-staking-migration-versioning
Ank4n Nov 24, 2022
6ae9b70
Merge remote-tracking branch 'origin/master' into ankan/12041-staking…
Nov 29, 2022
c53a848
retriggering ci
Ank4n Nov 29, 2022
80fa571
Merge branch 'master' into ankan/12041-staking-migration-versioning
Ank4n Dec 19, 2022
93ba64a
fix migration comments
Ank4n Dec 19, 2022
c2f3cbc
doc update
Ank4n Dec 19, 2022
2e3f919
Merge remote-tracking branch 'origin/master' into ankan/12041-staking…
Dec 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions frame/staking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,31 +881,6 @@ impl Default for Forcing {
}
}

// A value placed in storage that represents the current version of the Staking storage. This value
// is used by the `on_runtime_upgrade` logic to determine whether we run storage migration logic.
// This should match directly with the semantic versions of the Rust crate.
#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
enum Releases {
V1_0_0Ancient,
V2_0_0,
V3_0_0,
V4_0_0,
V5_0_0, // blockable validators.
V6_0_0, // removal of all storage associated with offchain phragmen.
V7_0_0, // keep track of number of nominators / validators in map
V8_0_0, // populate `VoterList`.
V9_0_0, // inject validators into `VoterList` as well.
V10_0_0, // remove `EarliestUnappliedSlash`.
V11_0_0, // Move pallet storage prefix, e.g. BagsList -> VoterBagsList
V12_0_0, // remove `HistoryDepth`.
}

impl Default for Releases {
fn default() -> Self {
Releases::V11_0_0
}
}

/// A `Convert` implementation that finds the stash of the given controller account,
/// if any.
pub struct StashOf<T>(sp_std::marker::PhantomData<T>);
Expand Down
138 changes: 110 additions & 28 deletions frame/staking/src/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,87 @@

use super::*;
use frame_election_provider_support::SortedListProvider;
use frame_support::traits::OnRuntimeUpgrade;
use frame_support::{
dispatch::GetStorageVersion, pallet_prelude::ValueQuery, storage_alias,
traits::OnRuntimeUpgrade,
};

/// Used for release versioning upto v12.
///
/// Obsolete from v13. Keeping around to make encoding/decoding of old migration code easier.
#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
enum ObsoleteReleases {
V1_0_0Ancient,
V2_0_0,
V3_0_0,
V4_0_0,
V5_0_0, // blockable validators.
V6_0_0, // removal of all storage associated with offchain phragmen.
V7_0_0, // keep track of number of nominators / validators in map
V8_0_0, // populate `VoterList`.
V9_0_0, // inject validators into `VoterList` as well.
V10_0_0, // remove `EarliestUnappliedSlash`.
V11_0_0, // Move pallet storage prefix, e.g. BagsList -> VoterBagsList
V12_0_0, // remove `HistoryDepth`.
}

impl Default for ObsoleteReleases {
fn default() -> Self {
ObsoleteReleases::V12_0_0
}
}

/// Alias to the old storage item used for release versioning. Obsolete since v13.
#[storage_alias]
type StorageVersion<T: Config> = StorageValue<Pallet<T>, ObsoleteReleases, ValueQuery>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be scoped in v13?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its used in all migrations prior to v13. May be we should get rid of old migrations (unless you see a reason to keep them) and keep it scoped only in v13?


pub mod v13 {
use super::*;

pub struct MigrateToV13<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToV13<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == ObsoleteReleases::V12_0_0,
"Required v12 before upgrading to v13"
);

Ok(Default::default())
}

fn on_runtime_upgrade() -> Weight {
let current = Pallet::<T>::current_storage_version();
let onchain = StorageVersion::<T>::get();

if current == 13 && onchain == ObsoleteReleases::V12_0_0 {
StorageVersion::<T>::kill();
current.put::<Pallet<T>>();

log!(info, "v13 applied successfully");
T::DbWeight::get().reads_writes(1, 2)
} else {
log!(warn, "Skipping v13, should be removed");
melekes marked this conversation as resolved.
Show resolved Hide resolved
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
frame_support::ensure!(
Pallet::<T>::on_chain_storage_version() == 13,
"v13 not applied"
);

frame_support::ensure!(
!StorageVersion::<T>::exists(),
"Storage version not migrated correctly"
);

Ok(())
}
}
}

pub mod v12 {
use super::*;
Expand All @@ -36,7 +116,7 @@ pub mod v12 {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == Releases::V11_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V11_0_0,
"Expected v11 before upgrading to v12"
);

Expand All @@ -53,9 +133,9 @@ pub mod v12 {
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
if StorageVersion::<T>::get() == Releases::V11_0_0 {
if StorageVersion::<T>::get() == ObsoleteReleases::V11_0_0 {
HistoryDepth::<T>::kill();
StorageVersion::<T>::put(Releases::V12_0_0);
StorageVersion::<T>::put(ObsoleteReleases::V12_0_0);

log!(info, "v12 applied successfully");
T::DbWeight::get().reads_writes(1, 2)
Expand All @@ -68,7 +148,7 @@ pub mod v12 {
#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == crate::Releases::V12_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V12_0_0,
"v12 not applied"
);
Ok(())
Expand All @@ -92,7 +172,7 @@ pub mod v11 {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == crate::Releases::V10_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V10_0_0,
"must upgrade linearly"
);
let old_pallet_prefix = twox_128(N::get().as_bytes());
Expand All @@ -117,9 +197,9 @@ pub mod v11 {
let old_pallet_name = N::get();
let new_pallet_name = <P as PalletInfoAccess>::name();

if StorageVersion::<T>::get() == Releases::V10_0_0 {
if StorageVersion::<T>::get() == ObsoleteReleases::V10_0_0 {
// bump version anyway, even if we don't need to move the prefix
StorageVersion::<T>::put(Releases::V11_0_0);
StorageVersion::<T>::put(ObsoleteReleases::V11_0_0);
if new_pallet_name == old_pallet_name {
log!(
warn,
Expand All @@ -139,7 +219,7 @@ pub mod v11 {
#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == crate::Releases::V11_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V11_0_0,
"wrong version after the upgrade"
);

Expand Down Expand Up @@ -184,7 +264,7 @@ pub mod v10 {
pub struct MigrateToV10<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToV10<T> {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
if StorageVersion::<T>::get() == Releases::V9_0_0 {
if StorageVersion::<T>::get() == ObsoleteReleases::V9_0_0 {
let pending_slashes = <Pallet<T> as Store>::UnappliedSlashes::iter().take(512);
for (era, slashes) in pending_slashes {
for slash in slashes {
Expand All @@ -196,7 +276,7 @@ pub mod v10 {
}

EarliestUnappliedSlash::<T>::kill();
StorageVersion::<T>::put(Releases::V10_0_0);
StorageVersion::<T>::put(ObsoleteReleases::V10_0_0);

log!(info, "MigrateToV10 executed successfully");
T::DbWeight::get().reads_writes(1, 1)
Expand All @@ -221,7 +301,7 @@ pub mod v9 {
pub struct InjectValidatorsIntoVoterList<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for InjectValidatorsIntoVoterList<T> {
fn on_runtime_upgrade() -> Weight {
if StorageVersion::<T>::get() == Releases::V8_0_0 {
if StorageVersion::<T>::get() == ObsoleteReleases::V8_0_0 {
let prev_count = T::VoterList::count();
let weight_of_cached = Pallet::<T>::weight_of_fn();
for (v, _) in Validators::<T>::iter() {
Expand All @@ -239,13 +319,13 @@ pub mod v9 {
T::VoterList::count(),
);

StorageVersion::<T>::put(crate::Releases::V9_0_0);
StorageVersion::<T>::put(ObsoleteReleases::V9_0_0);
T::BlockWeights::get().max_block
} else {
log!(
warn,
"InjectValidatorsIntoVoterList being executed on the wrong storage \
version, expected Releases::V8_0_0"
version, expected ObsoleteReleases::V8_0_0"
);
T::DbWeight::get().reads(1)
}
Expand All @@ -254,7 +334,7 @@ pub mod v9 {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == crate::Releases::V8_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V8_0_0,
"must upgrade linearly"
);

Expand All @@ -272,7 +352,7 @@ pub mod v9 {
assert!(post_count == prev_count + validators);

frame_support::ensure!(
StorageVersion::<T>::get() == crate::Releases::V9_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V9_0_0,
"must upgrade "
);
Ok(())
Expand All @@ -281,14 +361,15 @@ pub mod v9 {
}

pub mod v8 {
use crate::{Config, Nominators, Pallet, StorageVersion, Weight};
use super::*;
use crate::{Config, Nominators, Pallet, Weight};
use frame_election_provider_support::SortedListProvider;
use frame_support::traits::Get;

#[cfg(feature = "try-runtime")]
pub fn pre_migrate<T: Config>() -> Result<(), &'static str> {
frame_support::ensure!(
StorageVersion::<T>::get() == crate::Releases::V7_0_0,
StorageVersion::<T>::get() == ObsoleteReleases::V7_0_0,
"must upgrade linearly"
);

Expand All @@ -298,19 +379,19 @@ pub mod v8 {

/// Migration to sorted `VoterList`.
pub fn migrate<T: Config>() -> Weight {
if StorageVersion::<T>::get() == crate::Releases::V7_0_0 {
crate::log!(info, "migrating staking to Releases::V8_0_0");
if StorageVersion::<T>::get() == ObsoleteReleases::V7_0_0 {
crate::log!(info, "migrating staking to ObsoleteReleases::V8_0_0");

let migrated = T::VoterList::unsafe_regenerate(
Nominators::<T>::iter().map(|(id, _)| id),
Pallet::<T>::weight_of_fn(),
);
debug_assert_eq!(T::VoterList::try_state(), Ok(()));

StorageVersion::<T>::put(crate::Releases::V8_0_0);
StorageVersion::<T>::put(ObsoleteReleases::V8_0_0);
crate::log!(
info,
"👜 completed staking migration to Releases::V8_0_0 with {} voters migrated",
"👜 completed staking migration to ObsoleteReleases::V8_0_0 with {} voters migrated",
migrated,
);

Expand Down Expand Up @@ -348,20 +429,20 @@ pub mod v7 {
);
assert!(Validators::<T>::count().is_zero(), "Validators already set.");
assert!(Nominators::<T>::count().is_zero(), "Nominators already set.");
assert!(StorageVersion::<T>::get() == Releases::V6_0_0);
assert!(StorageVersion::<T>::get() == ObsoleteReleases::V6_0_0);
Ok(())
}

pub fn migrate<T: Config>() -> Weight {
log!(info, "Migrating staking to Releases::V7_0_0");
log!(info, "Migrating staking to ObsoleteReleases::V7_0_0");
let validator_count = Validators::<T>::iter().count() as u32;
let nominator_count = Nominators::<T>::iter().count() as u32;

CounterForValidators::<T>::put(validator_count);
CounterForNominators::<T>::put(nominator_count);

StorageVersion::<T>::put(Releases::V7_0_0);
log!(info, "Completed staking migration to Releases::V7_0_0");
StorageVersion::<T>::put(ObsoleteReleases::V7_0_0);
log!(info, "Completed staking migration to ObsoleteReleases::V7_0_0");

T::DbWeight::get().reads_writes(validator_count.saturating_add(nominator_count).into(), 2)
}
Expand Down Expand Up @@ -403,7 +484,7 @@ pub mod v6 {

/// Migrate storage to v6.
pub fn migrate<T: Config>() -> Weight {
log!(info, "Migrating staking to Releases::V6_0_0");
log!(info, "Migrating staking to ObsoleteReleases::V6_0_0");

SnapshotValidators::<T>::kill();
SnapshotNominators::<T>::kill();
Expand All @@ -412,7 +493,8 @@ pub mod v6 {
EraElectionStatus::<T>::kill();
IsCurrentSessionFinal::<T>::kill();

StorageVersion::<T>::put(Releases::V6_0_0);
StorageVersion::<T>::put(ObsoleteReleases::V6_0_0);

log!(info, "Done.");
T::DbWeight::get().writes(6 + 1)
}
Expand Down
14 changes: 5 additions & 9 deletions frame/staking/src/pallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub use impls::*;
use crate::{
slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, EraPayout,
EraRewardPoints, Exposure, Forcing, NegativeImbalanceOf, Nominations, PositiveImbalanceOf,
Releases, RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk,
RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk,
ValidatorPrefs,
};

Expand All @@ -64,8 +64,12 @@ pub mod pallet {

use super::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(13);
Copy link
Contributor

Choose a reason for hiding this comment

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

technically we could have started from 0 as well, but whatever :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically yes :). But may be still nice to convey this is the 13th migration. 😉


#[pallet::pallet]
#[pallet::generate_store(pub(crate) trait Store)]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T>(_);

/// Possible operations on the configuration values of this pallet.
Expand Down Expand Up @@ -561,13 +565,6 @@ pub mod pallet {
#[pallet::getter(fn offending_validators)]
pub type OffendingValidators<T: Config> = StorageValue<_, Vec<(u32, bool)>, ValueQuery>;

/// True if network has been upgraded to this version.
/// Storage version of the pallet.
///
/// This is set to v7.0.0 for new networks.
#[pallet::storage]
pub(crate) type StorageVersion<T: Config> = StorageValue<_, Releases, ValueQuery>;

/// The threshold for when users can start calling `chill_other` for other validators /
/// nominators. The threshold is compared to the actual number of validators / nominators
/// (`CountFor*`) in the system compared to the configured max (`Max*Count`).
Expand Down Expand Up @@ -618,7 +615,6 @@ pub mod pallet {
ForceEra::<T>::put(self.force_era);
CanceledSlashPayout::<T>::put(self.canceled_payout);
SlashRewardFraction::<T>::put(self.slash_reward_fraction);
StorageVersion::<T>::put(Releases::V7_0_0);
MinNominatorBond::<T>::put(self.min_nominator_bond);
MinValidatorBond::<T>::put(self.min_validator_bond);
if let Some(x) = self.max_validator_count {
Expand Down