Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrations: prevent accidentally using unversioned migrations instead of VersionedMigration #3835

Merged
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
9 changes: 5 additions & 4 deletions cumulus/pallets/xcmp-queue/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{Config, OverweightIndex, Pallet, QueueConfig, QueueConfigData, DEFAU
use cumulus_primitives_core::XcmpMessageFormat;
use frame_support::{
pallet_prelude::*,
traits::{EnqueueMessage, OnRuntimeUpgrade, StorageVersion},
traits::{EnqueueMessage, StorageVersion, UncheckedOnRuntimeUpgrade},
weights::{constants::WEIGHT_REF_TIME_PER_MILLIS, Weight},
};

Expand Down Expand Up @@ -96,7 +96,7 @@ pub mod v2 {
/// 2D weights).
pub struct UncheckedMigrationToV2<T: Config>(PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for UncheckedMigrationToV2<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrationToV2<T> {
#[allow(deprecated)]
fn on_runtime_upgrade() -> Weight {
let translate = |pre: v1::QueueConfigData| -> v2::QueueConfigData {
Expand Down Expand Up @@ -187,7 +187,7 @@ pub mod v3 {
/// Migrates the pallet storage to v3.
pub struct UncheckedMigrationToV3<T: Config>(PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for UncheckedMigrationToV3<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrationToV3<T> {
fn on_runtime_upgrade() -> Weight {
#[frame_support::storage_alias]
type Overweight<T: Config> =
Expand Down Expand Up @@ -266,7 +266,7 @@ pub mod v4 {
/// thresholds to at least the default values.
pub struct UncheckedMigrationToV4<T: Config>(PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for UncheckedMigrationToV4<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrationToV4<T> {
fn on_runtime_upgrade() -> Weight {
let translate = |pre: v2::QueueConfigData| -> QueueConfigData {
let pre_default = v2::QueueConfigData::default();
Expand Down Expand Up @@ -315,6 +315,7 @@ pub mod v4 {
mod tests {
use super::*;
use crate::mock::{new_test_ext, Test};
use frame_support::traits::OnRuntimeUpgrade;

#[test]
#[allow(deprecated)]
Expand Down
5 changes: 2 additions & 3 deletions polkadot/runtime/common/src/assigned_slots/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,17 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use super::{Config, MaxPermanentSlots, MaxTemporarySlots, Pallet, LOG_TARGET};
use frame_support::traits::{Get, GetStorageVersion, OnRuntimeUpgrade};
use frame_support::traits::{Get, GetStorageVersion, UncheckedOnRuntimeUpgrade};

#[cfg(feature = "try-runtime")]
use frame_support::ensure;
#[cfg(feature = "try-runtime")]
use sp_std::vec::Vec;

pub mod v1 {

use super::*;
pub struct VersionUncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
let on_chain_version = Pallet::<T>::on_chain_storage_version();
Expand Down
4 changes: 2 additions & 2 deletions polkadot/runtime/common/src/paras_registrar/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use super::*;
use frame_support::traits::{Contains, OnRuntimeUpgrade};
use frame_support::traits::{Contains, UncheckedOnRuntimeUpgrade};

#[derive(Encode, Decode)]
pub struct ParaInfoV1<Account, Balance> {
Expand All @@ -27,7 +27,7 @@ pub struct ParaInfoV1<Account, Balance> {
pub struct VersionUncheckedMigrateToV1<T, UnlockParaIds>(
sp_std::marker::PhantomData<(T, UnlockParaIds)>,
);
impl<T: Config, UnlockParaIds: Contains<ParaId>> OnRuntimeUpgrade
impl<T: Config, UnlockParaIds: Contains<ParaId>> UncheckedOnRuntimeUpgrade
for VersionUncheckedMigrateToV1<T, UnlockParaIds>
{
fn on_runtime_upgrade() -> Weight {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use super::*;
use frame_support::{
migrations::VersionedMigration, pallet_prelude::ValueQuery, storage_alias,
traits::OnRuntimeUpgrade, weights::Weight,
traits::UncheckedOnRuntimeUpgrade, weights::Weight,
};

mod v0 {
Expand Down Expand Up @@ -51,7 +51,7 @@ mod v1 {

/// Migration to V1
pub struct UncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for UncheckedMigrateToV1<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrateToV1<T> {
fn on_runtime_upgrade() -> Weight {
let mut weight: Weight = Weight::zero();

Expand Down Expand Up @@ -141,7 +141,7 @@ pub type MigrateV0ToV1<T> = VersionedMigration<

#[cfg(test)]
mod tests {
use super::{v0, v1, OnRuntimeUpgrade, Weight};
use super::{v0, v1, UncheckedOnRuntimeUpgrade, Weight};
use crate::mock::{new_test_ext, MockGenesisConfig, OnDemandAssigner, Test};
use primitives::Id as ParaId;

Expand All @@ -163,7 +163,7 @@ mod tests {

// For tests, db weight is zero.
assert_eq!(
<v1::UncheckedMigrateToV1<Test> as OnRuntimeUpgrade>::on_runtime_upgrade(),
<v1::UncheckedMigrateToV1<Test> as UncheckedOnRuntimeUpgrade>::on_runtime_upgrade(),
Weight::zero()
);

Expand Down
10 changes: 6 additions & 4 deletions polkadot/runtime/parachains/src/configuration/migration/v10.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
//! A module that is responsible for migration of storage.

use crate::configuration::{Config, Pallet};
use frame_support::{pallet_prelude::*, traits::Defensive, weights::Weight};
use frame_support::{
pallet_prelude::*,
traits::{Defensive, UncheckedOnRuntimeUpgrade},
weights::Weight,
};
use frame_system::pallet_prelude::BlockNumberFor;
use primitives::{
AsyncBackingParams, Balance, ExecutorParams, NodeFeatures, SessionIndex,
Expand All @@ -26,8 +30,6 @@ use primitives::{
use sp_runtime::Perbill;
use sp_std::vec::Vec;

use frame_support::traits::OnRuntimeUpgrade;

use super::v9::V9HostConfiguration;
// All configuration of the runtime with respect to paras.
#[derive(Clone, Encode, PartialEq, Decode, Debug)]
Expand Down Expand Up @@ -163,7 +165,7 @@ mod v10 {
}

pub struct VersionUncheckedMigrateToV10<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for VersionUncheckedMigrateToV10<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateToV10<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
log::trace!(target: crate::configuration::LOG_TARGET, "Running pre_upgrade() for HostConfiguration MigrateToV10");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@

use crate::configuration::{self, Config, Pallet};
use frame_support::{
migrations::VersionedMigration, pallet_prelude::*, traits::Defensive, weights::Weight,
migrations::VersionedMigration,
pallet_prelude::*,
traits::{Defensive, UncheckedOnRuntimeUpgrade},
weights::Weight,
};
use frame_system::pallet_prelude::BlockNumberFor;
use primitives::{
Expand All @@ -27,7 +30,6 @@ use primitives::{
};
use sp_std::vec::Vec;

use frame_support::traits::OnRuntimeUpgrade;
use polkadot_core_primitives::Balance;
use sp_arithmetic::Perbill;

Expand Down Expand Up @@ -176,7 +178,7 @@ pub type MigrateToV11<T> = VersionedMigration<
>;

pub struct UncheckedMigrateToV11<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for UncheckedMigrateToV11<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrateToV11<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
log::trace!(target: crate::configuration::LOG_TARGET, "Running pre_upgrade() for HostConfiguration MigrateToV11");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::configuration::{self, migration::v11::V11HostConfiguration, Config, P
use frame_support::{
migrations::VersionedMigration,
pallet_prelude::*,
traits::{Defensive, OnRuntimeUpgrade},
traits::{Defensive, UncheckedOnRuntimeUpgrade},
};
use frame_system::pallet_prelude::BlockNumberFor;
use primitives::vstaging::SchedulerParams;
Expand Down Expand Up @@ -70,7 +70,7 @@ pub type MigrateToV12<T> = VersionedMigration<

pub struct UncheckedMigrateToV12<T>(sp_std::marker::PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for UncheckedMigrateToV12<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrateToV12<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
log::trace!(target: crate::configuration::LOG_TARGET, "Running pre_upgrade() for HostConfiguration MigrateToV12");
Expand Down
10 changes: 5 additions & 5 deletions polkadot/runtime/parachains/src/inclusion/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ mod v1 {
CandidatePendingAvailability as V1CandidatePendingAvailability, Config, Pallet,
PendingAvailability as V1PendingAvailability,
};
use frame_support::{traits::OnRuntimeUpgrade, weights::Weight};
use frame_support::{traits::UncheckedOnRuntimeUpgrade, weights::Weight};
use sp_core::Get;
use sp_std::{collections::vec_deque::VecDeque, vec::Vec};

Expand All @@ -87,7 +87,7 @@ mod v1 {

pub struct VersionUncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
log::trace!(target: crate::inclusion::LOG_TARGET, "Running pre_upgrade() for inclusion MigrateToV1");
Expand Down Expand Up @@ -216,7 +216,7 @@ mod tests {
},
mock::{new_test_ext, MockGenesisConfig, Test},
};
use frame_support::traits::OnRuntimeUpgrade;
use frame_support::traits::UncheckedOnRuntimeUpgrade;
use primitives::{AvailabilityBitfield, Id as ParaId};
use test_helpers::{dummy_candidate_commitments, dummy_candidate_descriptor, dummy_hash};

Expand All @@ -225,7 +225,7 @@ mod tests {
new_test_ext(MockGenesisConfig::default()).execute_with(|| {
// No data to migrate.
assert_eq!(
<VersionUncheckedMigrateToV1<Test> as OnRuntimeUpgrade>::on_runtime_upgrade(),
<VersionUncheckedMigrateToV1<Test> as UncheckedOnRuntimeUpgrade>::on_runtime_upgrade(),
Weight::zero()
);
assert!(V1PendingAvailability::<Test>::iter().next().is_none());
Expand Down Expand Up @@ -299,7 +299,7 @@ mod tests {

// For tests, db weight is zero.
assert_eq!(
<VersionUncheckedMigrateToV1<Test> as OnRuntimeUpgrade>::on_runtime_upgrade(),
<VersionUncheckedMigrateToV1<Test> as UncheckedOnRuntimeUpgrade>::on_runtime_upgrade(),
Weight::zero()
);

Expand Down
9 changes: 5 additions & 4 deletions polkadot/runtime/parachains/src/scheduler/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use super::*;
use frame_support::{
migrations::VersionedMigration, pallet_prelude::ValueQuery, storage_alias,
traits::OnRuntimeUpgrade, weights::Weight,
traits::UncheckedOnRuntimeUpgrade, weights::Weight,
};

/// Old/legacy assignment representation (v0).
Expand Down Expand Up @@ -105,7 +105,8 @@ mod v0 {
// - Assignments only consist of `ParaId`, `Assignment` is a concrete type (Same as V0Assignment).
mod v1 {
use frame_support::{
pallet_prelude::ValueQuery, storage_alias, traits::OnRuntimeUpgrade, weights::Weight,
pallet_prelude::ValueQuery, storage_alias, traits::UncheckedOnRuntimeUpgrade,
weights::Weight,
};
use frame_system::pallet_prelude::BlockNumberFor;

Expand Down Expand Up @@ -164,7 +165,7 @@ mod v1 {

/// Migration to V1
pub struct UncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for UncheckedMigrateToV1<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrateToV1<T> {
fn on_runtime_upgrade() -> Weight {
let mut weight: Weight = Weight::zero();

Expand Down Expand Up @@ -302,7 +303,7 @@ mod v2 {
/// Migration to V2
pub struct UncheckedMigrateToV2<T>(sp_std::marker::PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for UncheckedMigrateToV2<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrateToV2<T> {
fn on_runtime_upgrade() -> Weight {
let mut weight: Weight = Weight::zero();

Expand Down
4 changes: 2 additions & 2 deletions polkadot/xcm/pallet-xcm/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
};
use frame_support::{
pallet_prelude::*,
traits::{OnRuntimeUpgrade, StorageVersion},
traits::{OnRuntimeUpgrade, StorageVersion, UncheckedOnRuntimeUpgrade},
weights::Weight,
};

Expand All @@ -35,7 +35,7 @@ pub mod v1 {
///
/// Use experimental [`MigrateToV1`] instead.
pub struct VersionUncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
fn on_runtime_upgrade() -> Weight {
let mut weight = T::DbWeight::get().reads(1);

Expand Down
54 changes: 54 additions & 0 deletions prdoc/pr_3835.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "migrations: prevent accidentally using inner unversioned migration instead of `VersionedMigration`"

doc:
- audience: Runtime Dev
description: |
Currently, it is possible to accidentally use inner unversioned migration instead of `VersionedMigration`
since both implement `OnRuntimeUpgrade`. With this change, we make it clear that `Inner` is not intended
to be used directly. It is achieved by bounding `Inner` to new trait `UncheckedOnRuntimeUpgrade`, which
has the same interface as `OnRuntimeUpgrade`, but can not be used directly for runtime upgrade migrations.
dastansam marked this conversation as resolved.
Show resolved Hide resolved

This change will break all existing migrations passed to `VersionedMigration`. Developers should simply change
those migrations to implement `UncheckedOnRuntimeUpgrade` instead of `OnRuntimeUpgrade`.

Example:

```
--- a/path/to/migration.rs
+++ b/path/to/migration.rs
@@ -1,7 +1,7 @@
-impl<T: Config> OnRuntimeUpgrade for MigrateVNToVM<T> {
+impl<T: Config> UncheckedOnRuntimeUpgrade for MigrateVNToVM<T> {
fn on_runtime_upgrade() -> Weight {
// Migration logic here
// Adjust the migration logic if necessary to align with the expectations
// of new `UncheckedOnRuntimeUpgrade` trait.
0
}
}
```

crates:
- name: "pallet-example-single-block-migrations"
dastansam marked this conversation as resolved.
Show resolved Hide resolved
bump: "major"
- name: "pallet-xcm"
bump: "major"
- name: "pallet-grandpa"
bump: "major"
- name: "pallet-identity"
bump: "major"
- name: "pallet-nomination-pools"
bump: "major"
- name: "pallet-society"
bump: "major"
- name: "frame-support"
bump: "major"
- name: "pallet-uniques"
bump: "major"
- name: "polkadot-runtime-parachains"
bump: "major"
- name: "polkadot-runtime-common"
bump: "major"
Loading
Loading