From 112a0e73528fdba01638412e7ec56ca13c52a321 Mon Sep 17 00:00:00 2001
From: Juan <juangirini@gmail.com>
Date: Fri, 9 Jun 2023 13:04:31 +0200
Subject: [PATCH] Move `type Migrations` to `Config` (#14309)
* move migrate sequence to config
* remove commented out code
* Update frame/contracts/src/lib.rs
Co-authored-by: PG Herveou <pgherveou@gmail.com>
* remove Migrations generic
* make runtime use noop migrations
* restrict is_upgrade_supported
* undo is_upgrade_supported change
* Update bin/node/runtime/src/lib.rs
Co-authored-by: PG Herveou <pgherveou@gmail.com>
* add rust doc example for `Migrations`
* feature gate NoopMigration
* fix example code
* improve example
---------
Co-authored-by: PG Herveou <pgherveou@gmail.com>
---
substrate/bin/node/runtime/src/lib.rs | 6 +
substrate/frame/contracts/src/lib.rs | 12 +-
substrate/frame/contracts/src/migration.rs | 134 ++++++++++-----------
substrate/frame/contracts/src/tests.rs | 5 +-
4 files changed, 81 insertions(+), 76 deletions(-)
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 6dc9841f6b4..aad86b28de3 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -55,6 +55,8 @@ use frame_system::{
pub use node_primitives::{AccountId, Signature};
use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment};
use pallet_asset_conversion::{NativeOrAssetId, NativeOrAssetIdConverter};
+#[cfg(feature = "runtime-benchmarks")]
+use pallet_contracts::NoopMigration;
use pallet_election_provider_multi_phase::SolutionAccuracyOf;
use pallet_im_online::sr25519::AuthorityId as ImOnlineId;
use pallet_nfts::PalletFeatures;
@@ -1240,6 +1242,10 @@ impl pallet_contracts::Config for Runtime {
type MaxStorageKeyLen = ConstU32<128>;
type UnsafeUnstableInterface = ConstBool<false>;
type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
+ #[cfg(not(feature = "runtime-benchmarks"))]
+ type Migrations = ();
+ #[cfg(feature = "runtime-benchmarks")]
+ type Migrations = (NoopMigration<1>, NoopMigration<2>);
}
impl pallet_sudo::Config for Runtime {
diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs
index 779b713795b..42691d4b01d 100644
--- a/substrate/frame/contracts/src/lib.rs
+++ b/substrate/frame/contracts/src/lib.rs
@@ -87,12 +87,12 @@ mod address;
mod benchmarking;
mod exec;
mod gas;
-mod migration;
mod schedule;
mod storage;
mod wasm;
pub mod chain_extension;
+pub mod migration;
pub mod weights;
#[cfg(test)]
@@ -319,6 +319,16 @@ pub mod pallet {
/// The maximum length of the debug buffer in bytes.
#[pallet::constant]
type MaxDebugBufferLen: Get<u32>;
+
+ /// The sequence of migration steps that will be applied during a migration.
+ ///
+ /// # Examples
+ /// ```
+ /// use pallet_contracts::migration::{v9, v10, v11};
+ /// # struct Runtime {};
+ /// type Migrations = (v9::Migration<Runtime>, v10::Migration<Runtime>, v11::Migration<Runtime>);
+ /// ```
+ type Migrations: MigrateSequence;
}
#[pallet::hooks]
diff --git a/substrate/frame/contracts/src/migration.rs b/substrate/frame/contracts/src/migration.rs
index a75d5cc1a1f..ad85da04771 100644
--- a/substrate/frame/contracts/src/migration.rs
+++ b/substrate/frame/contracts/src/migration.rs
@@ -17,21 +17,10 @@
//! Migration framework for pallets.
-/// Macro to include all migration modules.
-/// We only want to make these modules public when `runtime-benchmarks` is
-/// enabled, so we can access migration code in benchmarks.
-macro_rules! use_modules {
- ($($module:ident),*) => {
- $(
- #[cfg(feature = "runtime-benchmarks")]
- pub mod $module;
- #[cfg(not(feature = "runtime-benchmarks"))]
- mod $module;
- )*
- };
-}
+pub mod v10;
+pub mod v11;
+pub mod v9;
-use_modules!(v9, v10, v11);
use crate::{weights::WeightInfo, Config, Error, MigrationInProgress, Pallet, Weight, LOG_TARGET};
use codec::{Codec, Decode};
use frame_support::{
@@ -58,10 +47,6 @@ fn invalid_version(version: StorageVersion) -> ! {
/// The cursor used to store the state of the current migration step.
pub type Cursor = BoundedVec<u8, ConstU32<1024>>;
-// In benchmark and tests we use noop migrations, to test and bench the migration framework itself.
-#[cfg(not(any(feature = "runtime-benchmarks", test)))]
-type Migrations<T> = (v9::Migration<T>, v10::Migration<T>, v11::Migration<T>);
-
/// IsFinished describes whether a migration is finished or not.
pub enum IsFinished {
Yes,
@@ -206,23 +191,16 @@ pub trait MigrateSequence: private::Sealed {
}
/// Performs all necessary migrations based on `StorageVersion`.
-#[cfg(not(any(feature = "runtime-benchmarks", test)))]
-pub struct Migration<T: Config, M: MigrateSequence = Migrations<T>>(PhantomData<(T, M)>);
-
-/// Custom migration for running runtime-benchmarks and tests.
-#[cfg(any(feature = "runtime-benchmarks", test))]
-pub struct Migration<T: Config, M: MigrateSequence = (NoopMigration<1>, NoopMigration<2>)>(
- PhantomData<(T, M)>,
-);
+pub struct Migration<T: Config>(PhantomData<T>);
#[cfg(feature = "try-runtime")]
-impl<T: Config, M: MigrateSequence> Migration<T, M> {
+impl<T: Config> Migration<T> {
fn run_all_steps() -> Result<(), TryRuntimeError> {
let mut weight = Weight::zero();
let name = <Pallet<T>>::name();
loop {
let in_progress_version = <Pallet<T>>::on_chain_storage_version() + 1;
- let state = M::pre_upgrade_step(in_progress_version)?;
+ let state = T::Migrations::pre_upgrade_step(in_progress_version)?;
let (status, w) = Self::migrate(Weight::MAX);
weight.saturating_accrue(w);
log::info!(
@@ -231,7 +209,7 @@ impl<T: Config, M: MigrateSequence> Migration<T, M> {
in_progress_version,
weight
);
- M::post_upgrade_step(in_progress_version, state)?;
+ T::Migrations::post_upgrade_step(in_progress_version, state)?;
if matches!(status, MigrateResult::Completed) {
break
}
@@ -243,7 +221,7 @@ impl<T: Config, M: MigrateSequence> Migration<T, M> {
}
}
-impl<T: Config, M: MigrateSequence> OnRuntimeUpgrade for Migration<T, M> {
+impl<T: Config> OnRuntimeUpgrade for Migration<T> {
fn on_runtime_upgrade() -> Weight {
let name = <Pallet<T>>::name();
let latest_version = <Pallet<T>>::current_storage_version();
@@ -274,7 +252,7 @@ impl<T: Config, M: MigrateSequence> OnRuntimeUpgrade for Migration<T, M> {
"{name}: Upgrading storage from {storage_version:?} to {latest_version:?}.",
);
- let cursor = M::new(storage_version + 1);
+ let cursor = T::Migrations::new(storage_version + 1);
MigrationInProgress::<T>::set(Some(cursor));
#[cfg(feature = "try-runtime")]
@@ -295,11 +273,14 @@ impl<T: Config, M: MigrateSequence> OnRuntimeUpgrade for Migration<T, M> {
target: LOG_TARGET,
"{}: Range supported {:?}, range requested {:?}",
<Pallet<T>>::name(),
- M::VERSION_RANGE,
+ T::Migrations::VERSION_RANGE,
(storage_version, target_version)
);
- ensure!(M::is_upgrade_supported(storage_version, target_version), "Unsupported upgrade");
+ ensure!(
+ T::Migrations::is_upgrade_supported(storage_version, target_version),
+ "Unsupported upgrade"
+ );
Ok(Default::default())
}
}
@@ -324,11 +305,11 @@ pub enum StepResult {
Completed { steps_done: u32 },
}
-impl<T: Config, M: MigrateSequence> Migration<T, M> {
- /// Verify that each migration's step of the MigrateSequence fits into `Cursor`.
+impl<T: Config> Migration<T> {
+ /// Verify that each migration's step of the [`T::Migrations`] sequence fits into `Cursor`.
pub(crate) fn integrity_test() {
let max_weight = <T as frame_system::Config>::BlockWeights::get().max_block;
- M::integrity_test(max_weight)
+ T::Migrations::integrity_test(max_weight)
}
/// Migrate
@@ -357,33 +338,36 @@ impl<T: Config, M: MigrateSequence> Migration<T, M> {
in_progress_version,
);
- let result =
- match M::steps(in_progress_version, cursor_before.as_ref(), &mut weight_left) {
- StepResult::InProgress { cursor, steps_done } => {
- *progress = Some(cursor);
+ let result = match T::Migrations::steps(
+ in_progress_version,
+ cursor_before.as_ref(),
+ &mut weight_left,
+ ) {
+ StepResult::InProgress { cursor, steps_done } => {
+ *progress = Some(cursor);
+ MigrateResult::InProgress { steps_done }
+ },
+ StepResult::Completed { steps_done } => {
+ in_progress_version.put::<Pallet<T>>();
+ if <Pallet<T>>::current_storage_version() != in_progress_version {
+ log::info!(
+ target: LOG_TARGET,
+ "{name}: Next migration is {:?},",
+ in_progress_version + 1
+ );
+ *progress = Some(T::Migrations::new(in_progress_version + 1));
MigrateResult::InProgress { steps_done }
- },
- StepResult::Completed { steps_done } => {
- in_progress_version.put::<Pallet<T>>();
- if <Pallet<T>>::current_storage_version() != in_progress_version {
- log::info!(
- target: LOG_TARGET,
- "{name}: Next migration is {:?},",
- in_progress_version + 1
- );
- *progress = Some(M::new(in_progress_version + 1));
- MigrateResult::InProgress { steps_done }
- } else {
- log::info!(
- target: LOG_TARGET,
- "{name}: All migrations done. At version {:?},",
- in_progress_version
- );
- *progress = None;
- MigrateResult::Completed
- }
- },
- };
+ } else {
+ log::info!(
+ target: LOG_TARGET,
+ "{name}: All migrations done. At version {:?},",
+ in_progress_version
+ );
+ *progress = None;
+ MigrateResult::Completed
+ }
+ },
+ };
(result, weight_limit.saturating_sub(weight_left))
})
@@ -527,11 +511,14 @@ mod test {
#[test]
fn is_upgrade_supported_works() {
- type M = (MockMigration<9>, MockMigration<10>, MockMigration<11>);
+ type Migrations = (MockMigration<9>, MockMigration<10>, MockMigration<11>);
[(1, 1), (8, 11), (9, 11)].into_iter().for_each(|(from, to)| {
assert!(
- M::is_upgrade_supported(StorageVersion::new(from), StorageVersion::new(to)),
+ Migrations::is_upgrade_supported(
+ StorageVersion::new(from),
+ StorageVersion::new(to)
+ ),
"{} -> {} is supported",
from,
to
@@ -540,7 +527,10 @@ mod test {
[(1, 0), (0, 3), (7, 11), (8, 10)].into_iter().for_each(|(from, to)| {
assert!(
- !M::is_upgrade_supported(StorageVersion::new(from), StorageVersion::new(to)),
+ !Migrations::is_upgrade_supported(
+ StorageVersion::new(from),
+ StorageVersion::new(to)
+ ),
"{} -> {} is not supported",
from,
to
@@ -550,27 +540,26 @@ mod test {
#[test]
fn steps_works() {
- type M = (MockMigration<2>, MockMigration<3>);
+ type Migrations = (MockMigration<2>, MockMigration<3>);
let version = StorageVersion::new(2);
- let mut cursor = M::new(version);
+ let mut cursor = Migrations::new(version);
let mut weight = Weight::from_all(2);
- let result = M::steps(version, &cursor, &mut weight);
+ let result = Migrations::steps(version, &cursor, &mut weight);
cursor = vec![1u8, 0].try_into().unwrap();
assert_eq!(result, StepResult::InProgress { cursor: cursor.clone(), steps_done: 1 });
assert_eq!(weight, Weight::from_all(1));
let mut weight = Weight::from_all(2);
assert_eq!(
- M::steps(version, &cursor, &mut weight),
+ Migrations::steps(version, &cursor, &mut weight),
StepResult::Completed { steps_done: 1 }
);
}
#[test]
fn no_migration_in_progress_works() {
- type M = (MockMigration<1>, MockMigration<2>);
- type TestMigration = Migration<Test, M>;
+ type TestMigration = Migration<Test>;
ExtBuilder::default().build().execute_with(|| {
assert_eq!(StorageVersion::get::<Pallet<Test>>(), 2);
@@ -580,8 +569,7 @@ mod test {
#[test]
fn migration_works() {
- type M = (MockMigration<1>, MockMigration<2>);
- type TestMigration = Migration<Test, M>;
+ type TestMigration = Migration<Test>;
ExtBuilder::default().set_storage_version(0).build().execute_with(|| {
assert_eq!(StorageVersion::get::<Pallet<Test>>(), 0);
diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs
index d23a238dcd1..9425f25adf4 100644
--- a/substrate/frame/contracts/src/tests.rs
+++ b/substrate/frame/contracts/src/tests.rs
@@ -28,8 +28,8 @@ use crate::{
wasm::{Determinism, PrefabWasmModule, ReturnCode as RuntimeReturnCode},
weights::WeightInfo,
BalanceOf, Code, CodeStorage, CollectEvents, Config, ContractInfo, ContractInfoOf, DebugInfo,
- DefaultAddressGenerator, DeletionQueueCounter, Error, MigrationInProgress, Origin, Pallet,
- Schedule,
+ DefaultAddressGenerator, DeletionQueueCounter, Error, MigrationInProgress, NoopMigration,
+ Origin, Pallet, Schedule,
};
use assert_matches::assert_matches;
use codec::Encode;
@@ -428,6 +428,7 @@ impl Config for Test {
type MaxStorageKeyLen = ConstU32<128>;
type UnsafeUnstableInterface = UnstableInterface;
type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
+ type Migrations = (NoopMigration<1>, NoopMigration<2>);
}
pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);
--
GitLab