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 9 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
15 changes: 8 additions & 7 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,9 +96,9 @@ 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 {
fn unchecked_on_runtime_upgrade() -> Weight {
let translate = |pre: v1::QueueConfigData| -> v2::QueueConfigData {
v2::QueueConfigData {
suspend_threshold: pre.suspend_threshold,
Expand Down Expand Up @@ -187,8 +187,8 @@ pub mod v3 {
/// Migrates the pallet storage to v3.
pub struct UncheckedMigrationToV3<T: Config>(PhantomData<T>);

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

impl<T: Config> OnRuntimeUpgrade for UncheckedMigrationToV4<T> {
fn on_runtime_upgrade() -> Weight {
impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrationToV4<T> {
fn unchecked_on_runtime_upgrade() -> Weight {
let translate = |pre: v2::QueueConfigData| -> QueueConfigData {
let pre_default = v2::QueueConfigData::default();
// If the previous values are the default ones, let's replace them with the new
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
7 changes: 3 additions & 4 deletions polkadot/runtime/common/src/assigned_slots/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,25 @@
// 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();
ensure!(on_chain_version < 1, "assigned_slots::MigrateToV1 migration can be deleted");
Ok(Default::default())
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
fn unchecked_on_runtime_upgrade() -> frame_support::weights::Weight {
let on_chain_version = Pallet::<T>::on_chain_storage_version();
if on_chain_version < 1 {
const MAX_PERMANENT_SLOTS: u32 = 100;
Expand Down
6 changes: 3 additions & 3 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,10 +27,10 @@ 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 {
fn unchecked_on_runtime_upgrade() -> Weight {
let mut count = 0u64;
Paras::<T>::translate::<ParaInfoV1<T::AccountId, BalanceOf<T>>, _>(|key, v1| {
count.saturating_inc();
Expand Down
10 changes: 5 additions & 5 deletions polkadot/runtime/parachains/src/assigner_on_demand/migration.rs
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,8 +51,8 @@ mod v1 {

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

// Migrate the current traffic value
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>::unchecked_on_runtime_upgrade(),
Weight::zero()
);

Expand Down
12 changes: 7 additions & 5 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::{
vstaging::NodeFeatures, AsyncBackingParams, Balance, ExecutorParams, 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,14 +165,14 @@ 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");
Ok(Vec::new())
}

fn on_runtime_upgrade() -> Weight {
fn unchecked_on_runtime_upgrade() -> Weight {
migrate_to_v10::<T>()
}

Expand Down
10 changes: 6 additions & 4 deletions polkadot/runtime/parachains/src/configuration/migration/v11.rs
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 primitives::vstaging::NodeFeatures;
use sp_arithmetic::Perbill;
Expand Down Expand Up @@ -177,14 +179,14 @@ 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");
Ok(Vec::new())
}

fn on_runtime_upgrade() -> Weight {
fn unchecked_on_runtime_upgrade() -> Weight {
log::info!(target: configuration::LOG_TARGET, "HostConfiguration MigrateToV11 started");
let weight_consumed = migrate_to_v11::<T>();

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,14 +70,14 @@ 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");
Ok(Vec::new())
}

fn on_runtime_upgrade() -> Weight {
fn unchecked_on_runtime_upgrade() -> Weight {
log::info!(target: configuration::LOG_TARGET, "HostConfiguration MigrateToV12 started");
let weight_consumed = migrate_to_v12::<T>();

Expand Down
12 changes: 6 additions & 6 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 All @@ -106,7 +106,7 @@ mod v1 {
Ok((candidates_before_upgrade as u32).encode())
}

fn on_runtime_upgrade() -> Weight {
fn unchecked_on_runtime_upgrade() -> Weight {
let mut weight: Weight = Weight::zero();

let v0_candidates: Vec<_> = V0PendingAvailability::<T>::drain().collect();
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>::unchecked_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>::unchecked_on_runtime_upgrade(),
Weight::zero()
);

Expand Down
13 changes: 7 additions & 6 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,8 +165,8 @@ mod v1 {

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

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

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

let old = v1::ClaimQueue::<T>::take();
Expand Down
6 changes: 3 additions & 3 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,8 +35,8 @@ pub mod v1 {
///
/// Use experimental [`MigrateToV1`] instead.
pub struct VersionUncheckedMigrateToV1<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
fn on_runtime_upgrade() -> Weight {
impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateToV1<T> {
fn unchecked_on_runtime_upgrade() -> Weight {
let mut weight = T::DbWeight::get().reads(1);

if StorageVersion::get::<Pallet<T>>() != 0 {
Expand Down
24 changes: 24 additions & 0 deletions prdoc/pr_3835.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 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

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