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

Ensure all StorageVersions on Polkadot/Kusama are correct #7199

Merged
merged 5 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion runtime/common/src/crowdloan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ pub mod pallet {
use frame_system::{ensure_root, ensure_signed, pallet_prelude::*};

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

#[pallet::pallet]
#[pallet::without_storage_info]
Expand Down
25 changes: 24 additions & 1 deletion runtime/kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,7 @@ pub type Migrations =
#[allow(deprecated, missing_docs)]
pub mod migrations {
use super::*;
use frame_support::traits::{GetStorageVersion, OnRuntimeUpgrade, StorageVersion};

pub type V0940 = (
pallet_nomination_pools::migration::v4::MigrateToV4<
Expand All @@ -1484,7 +1485,29 @@ pub mod migrations {
);

/// Unreleased migrations. Add new ones here:
pub type Unreleased = ();
pub type Unreleased = SetStorageVersions;

/// Migrations that set `StorageVersion`s we missed to set.
pub struct SetStorageVersions;

impl OnRuntimeUpgrade for SetStorageVersions {
fn on_runtime_upgrade() -> Weight {
// The `NisCounterpartBalances` pallet was added to the chain after/with the migration.
// So, the migration never needed to be executed, but we also did not set the proper `StorageVersion`.
let storage_version = NisCounterpartBalances::on_chain_storage_version();
if storage_version < 1 {
StorageVersion::new(1).put::<NisCounterpartBalances>();
}

// Was missed as part of: `runtime_common::session::migration::ClearOldSessionStorage<Runtime>`.
let storage_version = Historical::on_chain_storage_version();
if storage_version < 1 {
StorageVersion::new(1).put::<Historical>();
}

RocksDbWeight::get().reads_writes(2, 2)
}
}
}

/// Unchecked extrinsic type as expected by this runtime.
Expand Down
4 changes: 4 additions & 0 deletions runtime/parachains/src/disputes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,12 @@ pub mod pallet {
type WeightInfo: WeightInfo;
}

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

#[pallet::pallet]
#[pallet::without_storage_info]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T>(_);

/// The last pruned session, if any. All data stored by this module
Expand Down
9 changes: 3 additions & 6 deletions runtime/parachains/src/disputes/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@

use frame_support::traits::StorageVersion;

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

pub mod v1 {
use super::*;
use crate::disputes::{Config, Pallet};
Expand All @@ -38,10 +35,10 @@ pub mod v1 {
fn on_runtime_upgrade() -> Weight {
let mut weight: Weight = Weight::zero();

if StorageVersion::get::<Pallet<T>>() < STORAGE_VERSION {
if StorageVersion::get::<Pallet<T>>() < 1 {
log::info!(target: crate::disputes::LOG_TARGET, "Migrating disputes storage to v1");
weight += migrate_to_v1::<T>();
STORAGE_VERSION.put::<Pallet<T>>();
StorageVersion::new(1).put::<Pallet<T>>();
weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1));
} else {
log::info!(
Expand Down Expand Up @@ -71,7 +68,7 @@ pub mod v1 {
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
log::trace!(target: crate::disputes::LOG_TARGET, "Running post_upgrade()");
ensure!(
StorageVersion::get::<Pallet<T>>() == STORAGE_VERSION,
StorageVersion::get::<Pallet<T>>() >= 1,
"Storage version should be `1` after the migration"
);
ensure!(
Expand Down
4 changes: 3 additions & 1 deletion runtime/parachains/src/ump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,11 @@ impl WeightInfo for TestWeightInfo {
pub mod pallet {
use super::*;

const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
#[pallet::without_storage_info]
#[pallet::storage_version(migration::STORAGE_VERSION)]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T>(_);

#[pallet::config]
Expand Down
2 changes: 0 additions & 2 deletions runtime/parachains/src/ump/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ use frame_support::{
weights::Weight,
};

pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

pub mod v1 {
use super::*;

Expand Down
39 changes: 36 additions & 3 deletions runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1437,13 +1437,24 @@ impl Get<Perbill> for NominationPoolsMigrationV4OldPallet {
///
/// This contains the combined migrations of the last 10 releases. It allows to skip runtime
/// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT.
pub type Migrations =
(migrations::V0940, migrations::V0941, migrations::V0942, migrations::Unreleased);
pub type Migrations = (
migrations::V0938,
migrations::V0940,
migrations::V0941,
migrations::V0942,
migrations::Unreleased,
);

/// The runtime migrations per release.
#[allow(deprecated, missing_docs)]
pub mod migrations {
use super::*;
use frame_support::traits::{GetStorageVersion, OnRuntimeUpgrade, StorageVersion};

pub type V0938 = (
pallet_xcm::migration::v1::MigrateToV1<Runtime>,
parachains_ump::migration::v1::MigrateToV1<Runtime>,
);

pub type V0940 = (
pallet_nomination_pools::migration::v4::MigrateToV4<
Expand All @@ -1460,7 +1471,29 @@ pub mod migrations {
);

/// Unreleased migrations. Add new ones here:
pub type Unreleased = ();
pub type Unreleased = SetStorageVersions;

/// Migrations that set `StorageVersion`s we missed to set.
pub struct SetStorageVersions;

impl OnRuntimeUpgrade for SetStorageVersions {
fn on_runtime_upgrade() -> Weight {
// `Referenda` pallet was added on chain after the migration to version `1` was added.
// Thus, it never required the migration and we just missed to set the correct `StorageVersion`.
let storage_version = Referenda::on_chain_storage_version();
if storage_version < 1 {
StorageVersion::new(1).put::<Referenda>();
}

// Was missed as part of: `runtime_common::session::migration::ClearOldSessionStorage<Runtime>`.
let storage_version = Historical::on_chain_storage_version();
if storage_version < 1 {
StorageVersion::new(1).put::<Historical>();
}

RocksDbWeight::get().reads_writes(2, 2)
}
}
}

/// Unchecked extrinsic type as expected by this runtime.
Expand Down
4 changes: 3 additions & 1 deletion xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,10 @@ pub mod pallet {
pub const CurrentXcmVersion: u32 = XCM_VERSION;
}

const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
#[pallet::storage_version(migration::STORAGE_VERSION)]
#[pallet::storage_version(STORAGE_VERSION)]
#[pallet::without_storage_info]
pub struct Pallet<T>(_);

Expand Down
4 changes: 1 addition & 3 deletions xcm/pallet-xcm/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ use frame_support::{
weights::Weight,
};

pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

const DEFAULT_PROOF_SIZE: u64 = 64 * 1024;

pub mod v1 {
Expand Down Expand Up @@ -51,7 +49,7 @@ pub mod v1 {
VersionNotifyTargets::<T>::translate_values(translate);

log::info!("v1 applied successfully");
STORAGE_VERSION.put::<Pallet<T>>();
StorageVersion::new(1).put::<Pallet<T>>();
Copy link
Member

Choose a reason for hiding this comment

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

I think we should pay attention in general to never use STORAGE_VERSION.put.
There is one more case in runtime/parachains/src/configuration/migration.rs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should pay attention in general to never use STORAGE_VERSION.put.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because when you change STORAGE_VERSION, you will change the version the migration is writing to storage.


weight.saturating_add(T::DbWeight::get().writes(1))
} else {
Expand Down