From 833b8d8f136c4f709bfe29a09fe86755ef4320c0 Mon Sep 17 00:00:00 2001 From: John Mela Date: Wed, 28 Sep 2022 16:17:38 -0400 Subject: [PATCH 01/11] Increment spec_version to 2 --- runtime/frequency/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/frequency/src/lib.rs b/runtime/frequency/src/lib.rs index 0245bf67cc..9e74da915d 100644 --- a/runtime/frequency/src/lib.rs +++ b/runtime/frequency/src/lib.rs @@ -208,7 +208,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("frequency"), impl_name: create_runtime_str!("frequency"), authoring_version: 1, - spec_version: 1, + spec_version: 2, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From 42ab88ea44c15e5c2fe0ffb29a1138707007f504 Mon Sep 17 00:00:00 2001 From: John Mela Date: Thu, 29 Sep 2022 17:48:59 -0400 Subject: [PATCH 02/11] Storage Migration WIP --- pallets/msa/Cargo.toml | 1 + pallets/msa/src/lib.rs | 15 +++++++- pallets/msa/src/migrations/mod.rs | 2 + pallets/msa/src/migrations/v1.rs | 64 +++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 pallets/msa/src/migrations/mod.rs create mode 100644 pallets/msa/src/migrations/v1.rs diff --git a/pallets/msa/Cargo.toml b/pallets/msa/Cargo.toml index c2ec8fffd0..0d5f247d65 100644 --- a/pallets/msa/Cargo.toml +++ b/pallets/msa/Cargo.toml @@ -19,6 +19,7 @@ codec = {package = "parity-scale-codec", version = "3.1.5", default-features = f frame-benchmarking = {default-features = false, git = "https://github.com/paritytech/substrate.git", optional = true, branch = "polkadot-v0.9.27" } frame-support = {default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.27" } frame-system = {default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.27" } +log = { version = "0.4.17", default-features = false } scale-info = {version = "2.1.2", default-features = false, features = ["derive"]} sp-core = {default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.27" } sp-runtime = {default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.27" } diff --git a/pallets/msa/src/lib.rs b/pallets/msa/src/lib.rs index c4c28f7447..4267635c27 100644 --- a/pallets/msa/src/lib.rs +++ b/pallets/msa/src/lib.rs @@ -84,6 +84,7 @@ mod mock; #[cfg(test)] mod tests; +pub mod migrations; pub mod weights; pub use weights::*; @@ -97,6 +98,10 @@ use sp_std::prelude::*; #[frame_support::pallet] pub mod pallet { use super::*; + use frame_support::traits::StorageVersion; + + // Upgrade storage version from 0 to 1 + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); #[pallet::config] pub trait Config: frame_system::Config { @@ -123,6 +128,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); /// Storage type for MSA identifier @@ -163,10 +169,17 @@ pub mod pallet { /// - Key: AccountId /// - Value: [`MessageSourceId`] #[pallet::storage] - #[pallet::getter(fn get_msa_by_account_id)] pub type MessageSourceIdOf = StorageMap<_, Twox64Concat, T::AccountId, MessageSourceId, OptionQuery>; + /// Migrated Storage type for key to MSA information + /// - Key: AccountId + /// - Value: [`MessageSourceId`] + #[pallet::storage] + #[pallet::getter(fn get_msa_by_account_id)] + pub type MessageSourceMigratedIdOf = + StorageMap<_, Twox64Concat, T::AccountId, MessageSourceId, OptionQuery>; + /// Storage for MSA keys /// - Key: MSA Id /// - Value: List of Keys diff --git a/pallets/msa/src/migrations/mod.rs b/pallets/msa/src/migrations/mod.rs new file mode 100644 index 0000000000..c3b6446630 --- /dev/null +++ b/pallets/msa/src/migrations/mod.rs @@ -0,0 +1,2 @@ +/// Version 1. +pub mod v1; diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs new file mode 100644 index 0000000000..d802edebe6 --- /dev/null +++ b/pallets/msa/src/migrations/v1.rs @@ -0,0 +1,64 @@ +/// Trivial migration which moves MessageSourceIdOf -> MessageSourceMigratedIdOf +/// +/// Note: The depositor is not optional since he can never change. +use crate::{Config, Pallet}; + +use frame_support::{ + migration::StorageKeyIterator, + traits::{OnRuntimeUpgrade, StorageVersion}, + weights::Weight, +}; +use log; + +pub struct MigrateToV1(sp_std::marker::PhantomData); +pub mod v1 { + use super::*; + impl OnRuntimeUpgrade for MigrateToV1 { + /// Perform a module upgrade. + /// + /// # Warning + /// + /// This function will be called before we initialized any runtime state, aka `on_initialize` + /// wasn't called yet. So, information like the block number and any other + /// block local data are not accessible. + /// + /// Return the non-negotiable weight consumed for runtime upgrade. + fn on_runtime_upgrade() -> Weight { + let current = Pallet::::current_storage_version(); + let onchain = Pallet::::on_chain_storage_version(); + log::info!( + "Running MSA migration with current storage version {:?} / onchain {:?}", + current, + onchain + ); + + // Execute the migration when upgrading MSA storage version from version 0 to version 1 + if current == 1 && onchain == 0 { + // put the current storage version into storage + current.put::>(); + log::info!("Migrated MessageSourceIdOf storage to MessageSourceMigratedIdOf"); + T::DbWeight::get().reads(1 as Weight) + } else if onchain > 0 { + log::info!("MigrateToV1 has already been completed and can be removed."); + T::DbWeight::get().reads(1 as Weight) + } + 0 + } + + /// Execute some pre-checks prior to a runtime upgrade. + /// + /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } + + /// Execute some post-checks after a runtime upgrade. + /// + /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + Ok(()) + } + } +} From 878b18366871a8b71d01225475ecd9e807ec70ec Mon Sep 17 00:00:00 2001 From: John Mela Date: Fri, 30 Sep 2022 13:29:14 -0400 Subject: [PATCH 03/11] Fix imports --- pallets/msa/src/migrations/v1.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs index d802edebe6..8421a8cba9 100644 --- a/pallets/msa/src/migrations/v1.rs +++ b/pallets/msa/src/migrations/v1.rs @@ -1,13 +1,9 @@ /// Trivial migration which moves MessageSourceIdOf -> MessageSourceMigratedIdOf /// /// Note: The depositor is not optional since he can never change. -use crate::{Config, Pallet}; +use crate::*; -use frame_support::{ - migration::StorageKeyIterator, - traits::{OnRuntimeUpgrade, StorageVersion}, - weights::Weight, -}; +use frame_support::{migration::StorageKeyIterator, traits::OnRuntimeUpgrade, weights::Weight}; use log; pub struct MigrateToV1(sp_std::marker::PhantomData); From 469f2d893ade52a31e0ca85aa2b450cbd474a5a4 Mon Sep 17 00:00:00 2001 From: John Mela Date: Fri, 30 Sep 2022 17:29:41 -0400 Subject: [PATCH 04/11] Add documentation --- pallets/msa/src/lib.rs | 2 ++ pallets/msa/src/migrations/v1.rs | 13 +++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pallets/msa/src/lib.rs b/pallets/msa/src/lib.rs index 4267635c27..b573b29f15 100644 --- a/pallets/msa/src/lib.rs +++ b/pallets/msa/src/lib.rs @@ -84,7 +84,9 @@ mod mock; #[cfg(test)] mod tests; +/// Import migrations module which executes migrations when the runtime is upgraded. pub mod migrations; + pub mod weights; pub use weights::*; diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs index 8421a8cba9..864a407aa1 100644 --- a/pallets/msa/src/migrations/v1.rs +++ b/pallets/msa/src/migrations/v1.rs @@ -6,7 +6,10 @@ use crate::*; use frame_support::{migration::StorageKeyIterator, traits::OnRuntimeUpgrade, weights::Weight}; use log; +/// Struct on which to implement OnRuntimeUpgrade trait pub struct MigrateToV1(sp_std::marker::PhantomData); + +/// Version 1 pub mod v1 { use super::*; impl OnRuntimeUpgrade for MigrateToV1 { @@ -30,15 +33,17 @@ pub mod v1 { // Execute the migration when upgrading MSA storage version from version 0 to version 1 if current == 1 && onchain == 0 { + // TODO: Iterate through AccountId in MessageSourceIdOf and + // copy the key/value pair to MessageSourceMigratedIdOf + // put the current storage version into storage current.put::>(); log::info!("Migrated MessageSourceIdOf storage to MessageSourceMigratedIdOf"); - T::DbWeight::get().reads(1 as Weight) - } else if onchain > 0 { + T::DbWeight::get().reads(1) + } else { log::info!("MigrateToV1 has already been completed and can be removed."); - T::DbWeight::get().reads(1 as Weight) + T::DbWeight::get().reads(1) } - 0 } /// Execute some pre-checks prior to a runtime upgrade. From 6f7f4cb30f39ce8b09c40835876af33b3e8e32ec Mon Sep 17 00:00:00 2001 From: John Mela Date: Fri, 30 Sep 2022 17:40:54 -0400 Subject: [PATCH 05/11] Add comments surrounding weights --- pallets/msa/src/migrations/v1.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs index 864a407aa1..e0dd751b43 100644 --- a/pallets/msa/src/migrations/v1.rs +++ b/pallets/msa/src/migrations/v1.rs @@ -39,9 +39,14 @@ pub mod v1 { // put the current storage version into storage current.put::>(); log::info!("Migrated MessageSourceIdOf storage to MessageSourceMigratedIdOf"); + + // TODO: Update the returned weight to reflect the cost of iterating through MessageSourceIdOf + // writing to MessageSourceMigratedIdOf, read the on chain storage version, and update the on chain storage version T::DbWeight::get().reads(1) } else { log::info!("MigrateToV1 has already been completed and can be removed."); + + // The weight cost to read the on chain storage version T::DbWeight::get().reads(1) } } From c7317c71b23cdece2cb773ac2cc5ccbf59a96909 Mon Sep 17 00:00:00 2001 From: John Mela Date: Wed, 5 Oct 2022 09:39:27 -0400 Subject: [PATCH 06/11] Add logger to Cargo.lock --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 20ba70484b..ffef3cb09f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5798,6 +5798,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log", "orml-utilities", "parity-scale-codec", "scale-info", From cea4d032372fb4ed0116ecdb10df071e2a07a0ad Mon Sep 17 00:00:00 2001 From: John Mela Date: Thu, 6 Oct 2022 10:56:02 -0400 Subject: [PATCH 07/11] Remove superfluous mod closure --- pallets/msa/src/migrations/v1.rs | 98 +++++++++++++++----------------- 1 file changed, 46 insertions(+), 52 deletions(-) diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs index e0dd751b43..4f11488ca7 100644 --- a/pallets/msa/src/migrations/v1.rs +++ b/pallets/msa/src/migrations/v1.rs @@ -5,66 +5,60 @@ use crate::*; use frame_support::{migration::StorageKeyIterator, traits::OnRuntimeUpgrade, weights::Weight}; use log; - /// Struct on which to implement OnRuntimeUpgrade trait pub struct MigrateToV1(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for MigrateToV1 { + /// Perform a module upgrade. + /// + /// # Warning + /// + /// This function will be called before we initialized any runtime state, aka `on_initialize` + /// wasn't called yet. So, information like the block number and any other + /// block local data are not accessible. + /// + /// Return the non-negotiable weight consumed for runtime upgrade. + fn on_runtime_upgrade() -> Weight { + let current = Pallet::::current_storage_version(); + let onchain = Pallet::::on_chain_storage_version(); + log::info!( + "Running MSA migration with current storage version {:?} / onchain {:?}", + current, + onchain + ); -/// Version 1 -pub mod v1 { - use super::*; - impl OnRuntimeUpgrade for MigrateToV1 { - /// Perform a module upgrade. - /// - /// # Warning - /// - /// This function will be called before we initialized any runtime state, aka `on_initialize` - /// wasn't called yet. So, information like the block number and any other - /// block local data are not accessible. - /// - /// Return the non-negotiable weight consumed for runtime upgrade. - fn on_runtime_upgrade() -> Weight { - let current = Pallet::::current_storage_version(); - let onchain = Pallet::::on_chain_storage_version(); - log::info!( - "Running MSA migration with current storage version {:?} / onchain {:?}", - current, - onchain - ); - - // Execute the migration when upgrading MSA storage version from version 0 to version 1 - if current == 1 && onchain == 0 { - // TODO: Iterate through AccountId in MessageSourceIdOf and - // copy the key/value pair to MessageSourceMigratedIdOf + // Execute the migration when upgrading MSA storage version from version 0 to version 1 + if current == 1 && onchain == 0 { + // TODO: Iterate through AccountId in MessageSourceIdOf and + // copy the key/value pair to MessageSourceMigratedIdOf - // put the current storage version into storage - current.put::>(); - log::info!("Migrated MessageSourceIdOf storage to MessageSourceMigratedIdOf"); + // put the current storage version into storage + current.put::>(); + log::info!("Migrated MessageSourceIdOf storage to MessageSourceMigratedIdOf"); - // TODO: Update the returned weight to reflect the cost of iterating through MessageSourceIdOf - // writing to MessageSourceMigratedIdOf, read the on chain storage version, and update the on chain storage version - T::DbWeight::get().reads(1) - } else { - log::info!("MigrateToV1 has already been completed and can be removed."); + // TODO: Update the returned weight to reflect the cost of iterating through MessageSourceIdOf + // writing to MessageSourceMigratedIdOf, read the on chain storage version, and update the on chain storage version + T::DbWeight::get().reads(1) + } else { + log::info!("MigrateToV1 has already been completed and can be removed."); - // The weight cost to read the on chain storage version - T::DbWeight::get().reads(1) - } + // The weight cost to read the on chain storage version + T::DbWeight::get().reads(1) } + } - /// Execute some pre-checks prior to a runtime upgrade. - /// - /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<(), &'static str> { - Ok(()) - } + /// Execute some pre-checks prior to a runtime upgrade. + /// + /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + Ok(()) + } - /// Execute some post-checks after a runtime upgrade. - /// - /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. - #[cfg(feature = "try-runtime")] - fn post_upgrade() -> Result<(), &'static str> { - Ok(()) - } + /// Execute some post-checks after a runtime upgrade. + /// + /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + Ok(()) } } From 3904f41721cef2a00b656a0cdda7c53105a0ed2f Mon Sep 17 00:00:00 2001 From: John Mela Date: Thu, 6 Oct 2022 10:56:59 -0400 Subject: [PATCH 08/11] Add migration to frame_executive::Executive to indicate it should be executed in runtime --- runtime/frequency-rococo/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/runtime/frequency-rococo/src/lib.rs b/runtime/frequency-rococo/src/lib.rs index 8b0d419361..be79dcc52c 100644 --- a/runtime/frequency-rococo/src/lib.rs +++ b/runtime/frequency-rococo/src/lib.rs @@ -133,8 +133,13 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, + Migrations, >; +// All migrations executed on runtime upgrade as a nested tuple of types implementing +// `OnRuntimeUpgrade`. +type Migrations = (pallet_msa::migrations::v1::MigrateToV1,); + /// Handles converting a weight scalar to a fee value, based on the scale and granularity of the /// node's balance type. /// From 1fb3574e16b163cb093669eccedf086aab33e3d3 Mon Sep 17 00:00:00 2001 From: John Mela Date: Thu, 6 Oct 2022 16:10:43 -0400 Subject: [PATCH 09/11] Migrate data in the toy migration and return weight to reflect the cost --- pallets/msa/src/migrations/v1.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs index 4f11488ca7..5942d441ea 100644 --- a/pallets/msa/src/migrations/v1.rs +++ b/pallets/msa/src/migrations/v1.rs @@ -30,14 +30,19 @@ impl OnRuntimeUpgrade for MigrateToV1 { if current == 1 && onchain == 0 { // TODO: Iterate through AccountId in MessageSourceIdOf and // copy the key/value pair to MessageSourceMigratedIdOf + let mut count = 0; + for (AccountId, MessageSourceId) in MessageSourceIdOf::::iter() { + MessageSourceMigratedIdOf::::insert(AccountId, MessageSourceId); + count += 1; + } // put the current storage version into storage current.put::>(); log::info!("Migrated MessageSourceIdOf storage to MessageSourceMigratedIdOf"); - // TODO: Update the returned weight to reflect the cost of iterating through MessageSourceIdOf + // Return the weight to reflect the cost of iterating through MessageSourceIdOf // writing to MessageSourceMigratedIdOf, read the on chain storage version, and update the on chain storage version - T::DbWeight::get().reads(1) + T::DbWeight::get().reads_writes(count + 1, count + 1) } else { log::info!("MigrateToV1 has already been completed and can be removed."); From 17654575bc2318d8b38399979010cccd88b72d4b Mon Sep 17 00:00:00 2001 From: John Mela Date: Fri, 7 Oct 2022 11:00:58 -0400 Subject: [PATCH 10/11] Fix linting errors --- pallets/msa/src/migrations/v1.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pallets/msa/src/migrations/v1.rs b/pallets/msa/src/migrations/v1.rs index 5942d441ea..a5c33c9112 100644 --- a/pallets/msa/src/migrations/v1.rs +++ b/pallets/msa/src/migrations/v1.rs @@ -3,7 +3,7 @@ /// Note: The depositor is not optional since he can never change. use crate::*; -use frame_support::{migration::StorageKeyIterator, traits::OnRuntimeUpgrade, weights::Weight}; +use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; use log; /// Struct on which to implement OnRuntimeUpgrade trait pub struct MigrateToV1(sp_std::marker::PhantomData); @@ -28,11 +28,11 @@ impl OnRuntimeUpgrade for MigrateToV1 { // Execute the migration when upgrading MSA storage version from version 0 to version 1 if current == 1 && onchain == 0 { - // TODO: Iterate through AccountId in MessageSourceIdOf and + // Iterate through AccountId in MessageSourceIdOf and // copy the key/value pair to MessageSourceMigratedIdOf let mut count = 0; - for (AccountId, MessageSourceId) in MessageSourceIdOf::::iter() { - MessageSourceMigratedIdOf::::insert(AccountId, MessageSourceId); + for (account_id, msa_id) in MessageSourceIdOf::::iter() { + MessageSourceMigratedIdOf::::insert(account_id, msa_id); count += 1; } From 6ae06a3dd653499652a87d7d535541249a37f8b6 Mon Sep 17 00:00:00 2001 From: John Mela Date: Thu, 13 Oct 2022 14:54:45 -0400 Subject: [PATCH 11/11] Increment spec_version to 2 in order to kick of a runtime upgrade and storage migrations --- runtime/frequency-rococo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/frequency-rococo/src/lib.rs b/runtime/frequency-rococo/src/lib.rs index be79dcc52c..ea8f12155a 100644 --- a/runtime/frequency-rococo/src/lib.rs +++ b/runtime/frequency-rococo/src/lib.rs @@ -195,7 +195,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("frequency-rococo"), impl_name: create_runtime_str!("frequency-rococo"), authoring_version: 1, - spec_version: 1, + spec_version: 2, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1,