From 5c1452917a195413ce28137ef9176937865e72da Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 6 Jun 2023 13:13:05 +0200 Subject: [PATCH 01/12] move migrate sequence to config --- bin/node/runtime/src/lib.rs | 4 +++- frame/contracts/src/benchmarking/mod.rs | 10 ++++----- frame/contracts/src/lib.rs | 29 ++++++++++++++----------- frame/contracts/src/migration.rs | 25 +++++++-------------- frame/contracts/src/tests.rs | 5 +++-- 5 files changed, 35 insertions(+), 38 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 6dc9841f6b44f..33ffe6d4f32cc 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -55,6 +55,7 @@ use frame_system::{ pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; use pallet_asset_conversion::{NativeOrAssetId, NativeOrAssetIdConverter}; +use pallet_contracts::migration::{v10, v11, v9}; use pallet_election_provider_multi_phase::SolutionAccuracyOf; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use pallet_nfts::PalletFeatures; @@ -1240,6 +1241,7 @@ impl pallet_contracts::Config for Runtime { type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = ConstBool; type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; + type Migrations = (v9::Migration, v10::Migration, v11::Migration); } impl pallet_sudo::Config for Runtime { @@ -1981,7 +1983,7 @@ pub type Executive = frame_executive::Executive< type Migrations = ( pallet_nomination_pools::migration::v2::MigrateToV2, pallet_alliance::migration::Migration, - pallet_contracts::Migration, + pallet_contracts::Migration::Migrations>, ); type EventRecord = frame_system::EventRecord< diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index b98c05f2503c2..27b9aadeed675 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -272,7 +272,7 @@ benchmarks! { migration_noop { assert_eq!(StorageVersion::get::>(), 2); }: { - Migration::::migrate(Weight::MAX) + Migration::::migrate(Weight::MAX) } verify { assert_eq!(StorageVersion::get::>(), 2); } @@ -281,7 +281,7 @@ benchmarks! { #[pov_mode = Measured] migrate { StorageVersion::new(0).put::>(); - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); let origin: RawOrigin<::AccountId> = RawOrigin::Signed(whitelisted_caller()); }: { >::migrate(origin.into(), Weight::MAX).unwrap() @@ -294,7 +294,7 @@ benchmarks! { on_runtime_upgrade_noop { assert_eq!(StorageVersion::get::>(), 2); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_none()); } @@ -306,7 +306,7 @@ benchmarks! { let v = vec![42u8].try_into().ok(); MigrationInProgress::::set(v.clone()); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_some()); assert_eq!(MigrationInProgress::::get(), v); @@ -317,7 +317,7 @@ benchmarks! { on_runtime_upgrade { StorageVersion::new(0).put::>(); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_some()); } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 779b713795b64..fa0a95d2ffebb 100644 --- a/frame/contracts/src/lib.rs +++ b/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,9 @@ pub mod pallet { /// The maximum length of the debug buffer in bytes. #[pallet::constant] type MaxDebugBufferLen: Get; + + /// The sequence of migrations applied. + type Migrations: MigrateSequence; } #[pallet::hooks] @@ -326,7 +329,7 @@ pub mod pallet { fn on_idle(_block: T::BlockNumber, remaining_weight: Weight) -> Weight { use migration::MigrateResult::*; - let (result, weight) = Migration::::migrate(remaining_weight); + let (result, weight) = Migration::::migrate(remaining_weight); let remaining_weight = remaining_weight.saturating_sub(weight); if !matches!(result, Completed | NoMigrationInProgress) { @@ -338,7 +341,7 @@ pub mod pallet { } fn integrity_test() { - Migration::::integrity_test(); + Migration::::integrity_test(); // Total runtime memory limit let max_runtime_mem: u32 = T::Schedule::get().limits.runtime_memory; @@ -518,7 +521,7 @@ pub mod pallet { storage_deposit_limit: Option< as codec::HasCompact>::Type>, determinism: Determinism, ) -> DispatchResult { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let origin = ensure_signed(origin)?; Self::bare_upload_code(origin, code, storage_deposit_limit.map(Into::into), determinism) .map(|_| ()) @@ -534,7 +537,7 @@ pub mod pallet { origin: OriginFor, code_hash: CodeHash, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let origin = ensure_signed(origin)?; >::remove(&origin, code_hash)?; // we waive the fee because removing unused code is beneficial @@ -558,7 +561,7 @@ pub mod pallet { dest: AccountIdLookupOf, code_hash: CodeHash, ) -> DispatchResult { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; ensure_root(origin)?; let dest = T::Lookup::lookup(dest)?; >::try_mutate(&dest, |contract| { @@ -608,7 +611,7 @@ pub mod pallet { storage_deposit_limit: Option< as codec::HasCompact>::Type>, data: Vec, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let common = CommonInput { origin: Origin::from_runtime_origin(origin)?, value, @@ -668,7 +671,7 @@ pub mod pallet { data: Vec, salt: Vec, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let code_len = code.len() as u32; let data_len = data.len() as u32; let salt_len = salt.len() as u32; @@ -711,7 +714,7 @@ pub mod pallet { data: Vec, salt: Vec, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let data_len = data.len() as u32; let salt_len = salt.len() as u32; let common = CommonInput { @@ -746,7 +749,7 @@ pub mod pallet { ensure_signed(origin)?; let weight_limit = weight_limit.saturating_add(T::WeightInfo::migrate()); - let (result, weight) = Migration::::migrate(weight_limit); + let (result, weight) = Migration::::migrate(weight_limit); match result { Completed => @@ -1272,7 +1275,7 @@ impl Invokable for InstantiateInput { macro_rules! ensure_no_migration_in_progress { () => { - if Migration::::in_progress() { + if Migration::::in_progress() { return ContractResult { gas_consumed: Zero::zero(), gas_required: Zero::zero(), @@ -1412,7 +1415,7 @@ impl Pallet { storage_deposit_limit: Option>, determinism: Determinism, ) -> CodeUploadResult, BalanceOf> { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let schedule = T::Schedule::get(); let module = PrefabWasmModule::from_code( code, @@ -1433,7 +1436,7 @@ impl Pallet { /// Query storage of a specified contract under a specified key. pub fn get_storage(address: T::AccountId, key: Vec) -> GetStorageResult { - if Migration::::in_progress() { + if Migration::::in_progress() { return Err(ContractAccessError::MigrationInProgress) } let contract_info = diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index a75d5cc1a1f4d..c13c5f4681f50 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -18,15 +18,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; )* }; } @@ -58,10 +53,6 @@ fn invalid_version(version: StorageVersion) -> ! { /// The cursor used to store the state of the current migration step. pub type Cursor = BoundedVec>; -// 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 = (v9::Migration, v10::Migration, v11::Migration); - /// IsFinished describes whether a migration is finished or not. pub enum IsFinished { Yes, @@ -206,14 +197,14 @@ pub trait MigrateSequence: private::Sealed { } /// Performs all necessary migrations based on `StorageVersion`. -#[cfg(not(any(feature = "runtime-benchmarks", test)))] -pub struct Migration>(PhantomData<(T, M)>); - -/// Custom migration for running runtime-benchmarks and tests. -#[cfg(any(feature = "runtime-benchmarks", test))] -pub struct Migration, NoopMigration<2>)>( - PhantomData<(T, M)>, -); +// #[cfg(not(any(feature = "runtime-benchmarks", test)))] +pub struct Migration(PhantomData<(T, M)>); + +// /// Custom migration for running runtime-benchmarks and tests. +// #[cfg(any(feature = "runtime-benchmarks", test))] +// pub struct Migration, NoopMigration<2>)>( +// PhantomData<(T, M)>, +// ); #[cfg(feature = "try-runtime")] impl Migration { diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index d23a238dcd158..9425f25adf4b8 100644 --- a/frame/contracts/src/tests.rs +++ b/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]); From e8c493cecdef218476a5a6ce78ce678140996132 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 6 Jun 2023 13:19:31 +0200 Subject: [PATCH 02/12] remove commented out code --- frame/contracts/src/migration.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index c13c5f4681f50..ec8a67c755c48 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -197,15 +197,8 @@ pub trait MigrateSequence: private::Sealed { } /// Performs all necessary migrations based on `StorageVersion`. -// #[cfg(not(any(feature = "runtime-benchmarks", test)))] pub struct Migration(PhantomData<(T, M)>); -// /// Custom migration for running runtime-benchmarks and tests. -// #[cfg(any(feature = "runtime-benchmarks", test))] -// pub struct Migration, NoopMigration<2>)>( -// PhantomData<(T, M)>, -// ); - #[cfg(feature = "try-runtime")] impl Migration { fn run_all_steps() -> Result<(), TryRuntimeError> { From 0e629301500114775c71e7e01cf039ed0c5ebb2e Mon Sep 17 00:00:00 2001 From: Juan Date: Tue, 6 Jun 2023 13:41:50 +0200 Subject: [PATCH 03/12] Update frame/contracts/src/lib.rs Co-authored-by: PG Herveou --- frame/contracts/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index fa0a95d2ffebb..118ccca94206c 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -320,7 +320,7 @@ pub mod pallet { #[pallet::constant] type MaxDebugBufferLen: Get; - /// The sequence of migrations applied. + /// The sequence of migration steps that will be applied during a migration. type Migrations: MigrateSequence; } From 14a71755cfb5546c4e78f5fdd6cf54ac1134d9b7 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 6 Jun 2023 14:01:59 +0200 Subject: [PATCH 04/12] remove Migrations generic --- bin/node/runtime/src/lib.rs | 2 +- frame/contracts/src/benchmarking/mod.rs | 10 +- frame/contracts/src/lib.rs | 24 ++--- frame/contracts/src/migration.rs | 118 ++++++++++++------------ 4 files changed, 79 insertions(+), 75 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 33ffe6d4f32cc..dd2accb086a43 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1983,7 +1983,7 @@ pub type Executive = frame_executive::Executive< type Migrations = ( pallet_nomination_pools::migration::v2::MigrateToV2, pallet_alliance::migration::Migration, - pallet_contracts::Migration::Migrations>, + pallet_contracts::Migration, ); type EventRecord = frame_system::EventRecord< diff --git a/frame/contracts/src/benchmarking/mod.rs b/frame/contracts/src/benchmarking/mod.rs index 27b9aadeed675..b98c05f2503c2 100644 --- a/frame/contracts/src/benchmarking/mod.rs +++ b/frame/contracts/src/benchmarking/mod.rs @@ -272,7 +272,7 @@ benchmarks! { migration_noop { assert_eq!(StorageVersion::get::>(), 2); }: { - Migration::::migrate(Weight::MAX) + Migration::::migrate(Weight::MAX) } verify { assert_eq!(StorageVersion::get::>(), 2); } @@ -281,7 +281,7 @@ benchmarks! { #[pov_mode = Measured] migrate { StorageVersion::new(0).put::>(); - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); let origin: RawOrigin<::AccountId> = RawOrigin::Signed(whitelisted_caller()); }: { >::migrate(origin.into(), Weight::MAX).unwrap() @@ -294,7 +294,7 @@ benchmarks! { on_runtime_upgrade_noop { assert_eq!(StorageVersion::get::>(), 2); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_none()); } @@ -306,7 +306,7 @@ benchmarks! { let v = vec![42u8].try_into().ok(); MigrationInProgress::::set(v.clone()); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_some()); assert_eq!(MigrationInProgress::::get(), v); @@ -317,7 +317,7 @@ benchmarks! { on_runtime_upgrade { StorageVersion::new(0).put::>(); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_some()); } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 118ccca94206c..59c9ea23ed38b 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -329,7 +329,7 @@ pub mod pallet { fn on_idle(_block: T::BlockNumber, remaining_weight: Weight) -> Weight { use migration::MigrateResult::*; - let (result, weight) = Migration::::migrate(remaining_weight); + let (result, weight) = Migration::::migrate(remaining_weight); let remaining_weight = remaining_weight.saturating_sub(weight); if !matches!(result, Completed | NoMigrationInProgress) { @@ -341,7 +341,7 @@ pub mod pallet { } fn integrity_test() { - Migration::::integrity_test(); + Migration::::integrity_test(); // Total runtime memory limit let max_runtime_mem: u32 = T::Schedule::get().limits.runtime_memory; @@ -521,7 +521,7 @@ pub mod pallet { storage_deposit_limit: Option< as codec::HasCompact>::Type>, determinism: Determinism, ) -> DispatchResult { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let origin = ensure_signed(origin)?; Self::bare_upload_code(origin, code, storage_deposit_limit.map(Into::into), determinism) .map(|_| ()) @@ -537,7 +537,7 @@ pub mod pallet { origin: OriginFor, code_hash: CodeHash, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let origin = ensure_signed(origin)?; >::remove(&origin, code_hash)?; // we waive the fee because removing unused code is beneficial @@ -561,7 +561,7 @@ pub mod pallet { dest: AccountIdLookupOf, code_hash: CodeHash, ) -> DispatchResult { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; ensure_root(origin)?; let dest = T::Lookup::lookup(dest)?; >::try_mutate(&dest, |contract| { @@ -611,7 +611,7 @@ pub mod pallet { storage_deposit_limit: Option< as codec::HasCompact>::Type>, data: Vec, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let common = CommonInput { origin: Origin::from_runtime_origin(origin)?, value, @@ -671,7 +671,7 @@ pub mod pallet { data: Vec, salt: Vec, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let code_len = code.len() as u32; let data_len = data.len() as u32; let salt_len = salt.len() as u32; @@ -714,7 +714,7 @@ pub mod pallet { data: Vec, salt: Vec, ) -> DispatchResultWithPostInfo { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let data_len = data.len() as u32; let salt_len = salt.len() as u32; let common = CommonInput { @@ -749,7 +749,7 @@ pub mod pallet { ensure_signed(origin)?; let weight_limit = weight_limit.saturating_add(T::WeightInfo::migrate()); - let (result, weight) = Migration::::migrate(weight_limit); + let (result, weight) = Migration::::migrate(weight_limit); match result { Completed => @@ -1275,7 +1275,7 @@ impl Invokable for InstantiateInput { macro_rules! ensure_no_migration_in_progress { () => { - if Migration::::in_progress() { + if Migration::::in_progress() { return ContractResult { gas_consumed: Zero::zero(), gas_required: Zero::zero(), @@ -1415,7 +1415,7 @@ impl Pallet { storage_deposit_limit: Option>, determinism: Determinism, ) -> CodeUploadResult, BalanceOf> { - Migration::::ensure_migrated()?; + Migration::::ensure_migrated()?; let schedule = T::Schedule::get(); let module = PrefabWasmModule::from_code( code, @@ -1436,7 +1436,7 @@ impl Pallet { /// Query storage of a specified contract under a specified key. pub fn get_storage(address: T::AccountId, key: Vec) -> GetStorageResult { - if Migration::::in_progress() { + if Migration::::in_progress() { return Err(ContractAccessError::MigrationInProgress) } let contract_info = diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index ec8a67c755c48..ad85da0477148 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -17,16 +17,10 @@ //! Migration framework for pallets. -/// Macro to include all migration modules. -macro_rules! use_modules { - ($($module:ident),*) => { - $( - pub 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::{ @@ -197,16 +191,16 @@ pub trait MigrateSequence: private::Sealed { } /// Performs all necessary migrations based on `StorageVersion`. -pub struct Migration(PhantomData<(T, M)>); +pub struct Migration(PhantomData); #[cfg(feature = "try-runtime")] -impl Migration { +impl Migration { fn run_all_steps() -> Result<(), TryRuntimeError> { let mut weight = Weight::zero(); let name = >::name(); loop { let in_progress_version = >::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!( @@ -215,7 +209,7 @@ impl Migration { 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 } @@ -227,7 +221,7 @@ impl Migration { } } -impl OnRuntimeUpgrade for Migration { +impl OnRuntimeUpgrade for Migration { fn on_runtime_upgrade() -> Weight { let name = >::name(); let latest_version = >::current_storage_version(); @@ -258,7 +252,7 @@ impl OnRuntimeUpgrade for Migration { "{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::::set(Some(cursor)); #[cfg(feature = "try-runtime")] @@ -279,11 +273,14 @@ impl OnRuntimeUpgrade for Migration { target: LOG_TARGET, "{}: Range supported {:?}, range requested {:?}", >::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()) } } @@ -308,11 +305,11 @@ pub enum StepResult { Completed { steps_done: u32 }, } -impl Migration { - /// Verify that each migration's step of the MigrateSequence fits into `Cursor`. +impl Migration { + /// Verify that each migration's step of the [`T::Migrations`] sequence fits into `Cursor`. pub(crate) fn integrity_test() { let max_weight = ::BlockWeights::get().max_block; - M::integrity_test(max_weight) + T::Migrations::integrity_test(max_weight) } /// Migrate @@ -341,33 +338,36 @@ impl Migration { 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::>(); + if >::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::>(); - if >::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)) }) @@ -511,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 @@ -524,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 @@ -534,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; + type TestMigration = Migration; ExtBuilder::default().build().execute_with(|| { assert_eq!(StorageVersion::get::>(), 2); @@ -564,8 +569,7 @@ mod test { #[test] fn migration_works() { - type M = (MockMigration<1>, MockMigration<2>); - type TestMigration = Migration; + type TestMigration = Migration; ExtBuilder::default().set_storage_version(0).build().execute_with(|| { assert_eq!(StorageVersion::get::>(), 0); From 2601d79a37c70fde3d81e891957d9b5d0f946a1c Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 6 Jun 2023 17:11:59 +0200 Subject: [PATCH 05/12] make runtime use noop migrations --- bin/node/runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index dd2accb086a43..dd4b14b1889b5 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -55,7 +55,7 @@ use frame_system::{ pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; use pallet_asset_conversion::{NativeOrAssetId, NativeOrAssetIdConverter}; -use pallet_contracts::migration::{v10, v11, v9}; +use pallet_contracts::NoopMigration; use pallet_election_provider_multi_phase::SolutionAccuracyOf; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use pallet_nfts::PalletFeatures; @@ -1241,7 +1241,7 @@ impl pallet_contracts::Config for Runtime { type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = ConstBool; type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; - type Migrations = (v9::Migration, v10::Migration, v11::Migration); + type Migrations = (NoopMigration<1>, NoopMigration<2>); } impl pallet_sudo::Config for Runtime { From e88854a39dbbd133b647b0b5ca9ede0a525b0bbe Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Tue, 6 Jun 2023 17:30:01 +0200 Subject: [PATCH 06/12] restrict is_upgrade_supported --- frame/contracts/src/migration.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index ad85da0477148..ce0e018f0ac3d 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -186,7 +186,7 @@ pub trait MigrateSequence: private::Sealed { return false }; - in_storage >= first_supported && target == high + in_storage == first_supported && target == high } } @@ -513,7 +513,7 @@ mod test { fn is_upgrade_supported_works() { type Migrations = (MockMigration<9>, MockMigration<10>, MockMigration<11>); - [(1, 1), (8, 11), (9, 11)].into_iter().for_each(|(from, to)| { + [(1, 1), (8, 11)].into_iter().for_each(|(from, to)| { assert!( Migrations::is_upgrade_supported( StorageVersion::new(from), @@ -525,7 +525,7 @@ mod test { ) }); - [(1, 0), (0, 3), (7, 11), (8, 10)].into_iter().for_each(|(from, to)| { + [(1, 0), (0, 3), (7, 11), (8, 10), (9, 11)].into_iter().for_each(|(from, to)| { assert!( !Migrations::is_upgrade_supported( StorageVersion::new(from), From 1400ab05eb3452b534d5999e82dd8f4fc528913b Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 7 Jun 2023 11:51:57 +0200 Subject: [PATCH 07/12] undo is_upgrade_supported change --- frame/contracts/src/migration.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index ce0e018f0ac3d..ad85da0477148 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -186,7 +186,7 @@ pub trait MigrateSequence: private::Sealed { return false }; - in_storage == first_supported && target == high + in_storage >= first_supported && target == high } } @@ -513,7 +513,7 @@ mod test { fn is_upgrade_supported_works() { type Migrations = (MockMigration<9>, MockMigration<10>, MockMigration<11>); - [(1, 1), (8, 11)].into_iter().for_each(|(from, to)| { + [(1, 1), (8, 11), (9, 11)].into_iter().for_each(|(from, to)| { assert!( Migrations::is_upgrade_supported( StorageVersion::new(from), @@ -525,7 +525,7 @@ mod test { ) }); - [(1, 0), (0, 3), (7, 11), (8, 10), (9, 11)].into_iter().for_each(|(from, to)| { + [(1, 0), (0, 3), (7, 11), (8, 10)].into_iter().for_each(|(from, to)| { assert!( !Migrations::is_upgrade_supported( StorageVersion::new(from), From cc7609c9275d1fa03fde4d5b139100777bc4b96b Mon Sep 17 00:00:00 2001 From: Juan Date: Wed, 7 Jun 2023 14:14:43 +0200 Subject: [PATCH 08/12] Update bin/node/runtime/src/lib.rs Co-authored-by: PG Herveou --- bin/node/runtime/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index dd4b14b1889b5..a5fe7465a3818 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1241,6 +1241,9 @@ impl pallet_contracts::Config for Runtime { type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = ConstBool; type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; + #[cfg(not(feature = "runtime-benchmarks"))] + type Migrations = (); + #[cfg(feature = "runtime-benchmarks")] type Migrations = (NoopMigration<1>, NoopMigration<2>); } From c173ad7e2751099212aa18c49d7d6794c451e224 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 7 Jun 2023 15:38:06 +0200 Subject: [PATCH 09/12] add rust doc example for `Migrations` --- frame/contracts/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 59c9ea23ed38b..d3bb0aa866a0a 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -321,6 +321,13 @@ pub mod pallet { type MaxDebugBufferLen: Get; /// The sequence of migration steps that will be applied during a migration. + /// + /// # Examples + /// ```ignore + /// impl pallet_contracts::Config for Runtime { + /// // ... + /// type Migrations = (v9::Migration, v10::Migration, v11::Migration); + /// ``` type Migrations: MigrateSequence; } From c9cf9c682be7056c718e524caea6a10424cb98a3 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 7 Jun 2023 15:58:16 +0200 Subject: [PATCH 10/12] feature gate NoopMigration --- bin/node/runtime/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index a5fe7465a3818..aad86b28de32f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -55,6 +55,7 @@ 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; @@ -1241,9 +1242,9 @@ impl pallet_contracts::Config for Runtime { type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = ConstBool; type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; - #[cfg(not(feature = "runtime-benchmarks"))] + #[cfg(not(feature = "runtime-benchmarks"))] type Migrations = (); - #[cfg(feature = "runtime-benchmarks")] + #[cfg(feature = "runtime-benchmarks")] type Migrations = (NoopMigration<1>, NoopMigration<2>); } From de545a3b9ecdd17a191ead0950087ffe8c28554f Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Wed, 7 Jun 2023 16:17:49 +0200 Subject: [PATCH 11/12] fix example code --- frame/contracts/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index d3bb0aa866a0a..3a30b3ce1eeb7 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -327,6 +327,8 @@ pub mod pallet { /// impl pallet_contracts::Config for Runtime { /// // ... /// type Migrations = (v9::Migration, v10::Migration, v11::Migration); + /// // ... + /// } /// ``` type Migrations: MigrateSequence; } From 62bec961fdf9ee09e59d8b481ba9df0fb0df46f3 Mon Sep 17 00:00:00 2001 From: Juan Girini Date: Thu, 8 Jun 2023 09:36:50 +0200 Subject: [PATCH 12/12] improve example --- frame/contracts/src/lib.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 3a30b3ce1eeb7..42691d4b01dc7 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -323,12 +323,10 @@ pub mod pallet { /// The sequence of migration steps that will be applied during a migration. /// /// # Examples - /// ```ignore - /// impl pallet_contracts::Config for Runtime { - /// // ... - /// type Migrations = (v9::Migration, v10::Migration, v11::Migration); - /// // ... - /// } + /// ``` + /// use pallet_contracts::migration::{v9, v10, v11}; + /// # struct Runtime {}; + /// type Migrations = (v9::Migration, v10::Migration, v11::Migration); /// ``` type Migrations: MigrateSequence; }