From 94181f96959c215d2fde81a95aea42270f633eba Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 25 Mar 2024 16:17:51 +0100 Subject: [PATCH 1/4] remove storage migration --- substrate/frame/support/src/migrations.rs | 114 +++++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index 2ceab44cb16bd..a24b30d6fa2ee 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -17,7 +17,7 @@ use crate::{ defensive, - storage::transactional::with_transaction_opaque_err, + storage::{storage_prefix, transactional::with_transaction_opaque_err}, traits::{ Defensive, GetStorageVersion, NoStorageVersionSet, PalletInfoAccess, SafeMode, StorageVersion, @@ -369,6 +369,118 @@ impl, DbWeight: Get> frame_support::traits } } +/// `RemoveStorage` is a utility struct used to remove a storage item from a specific pallet. +/// +/// This struct is generic over three parameters: +/// - `P` is a type that implements the [`Get`] trait for a static string, representing the pallet's +/// name. +/// - `S` is a type that implements the [`Get`] trait for a static string, representing the storage +/// name. +/// - `DbWeight` is a type that implements the [`Get`] trait for [`RuntimeDbWeight`], providing the +/// weight for database operations. +/// +/// On runtime upgrade, the `on_runtime_upgrade` function will clear the storage from the specified +/// storage, logging the number of keys removed. If the `try-runtime` feature is enabled, the +/// `pre_upgrade` and `post_upgrade` functions can be used to verify the storage removal before and +/// after the upgrade. +/// +/// # Examples: +/// ```ignore +/// construct_runtime! { +/// pub enum Runtime +/// { +/// System: frame_system = 0, +/// +/// SomePallet: pallet_something = 1, +/// +/// YourOtherPallets... +/// } +/// }; +/// +/// parameter_types! { +/// pub const SomePallet: &'static str = "SomePallet"; +/// pub const StorageAccounts: &'static str = "Accounts"; +/// pub const StorageAccountCount: &'static str = "AccountCount"; +/// } +/// +/// pub type Migrations = ( +/// RemoveStorage, +/// RemoveStorage, +/// AnyOtherMigrations... +/// ); +/// +/// pub type Executive = frame_executive::Executive< +/// Runtime, +/// Block, +/// frame_system::ChainContext, +/// Runtime, +/// Migrations +/// >; +/// ``` +/// +/// WARNING: `RemoveStorage` has no guard rails preventing it from bricking the chain if the +/// operation of removing storage for the given pallet would exceed the block weight limit. +/// +/// If your storage has too many keys to be removed in a single block, it is advised to wait for +/// a multi-block scheduler currently under development which will allow for removal of storage +/// items (and performing other heavy migrations) over multiple blocks +/// (see ). +pub struct RemoveStorage, S: Get<&'static str>, DbWeight: Get>( + PhantomData<(P, S, DbWeight)>, +); +impl, S: Get<&'static str>, DbWeight: Get> + frame_support::traits::OnRuntimeUpgrade for RemoveStorage +{ + fn on_runtime_upgrade() -> frame_support::weights::Weight { + let hashed_prefix = storage_prefix(P::get().as_bytes(), S::get().as_bytes()); + let keys_removed = match clear_prefix(&hashed_prefix, None) { + KillStorageResult::AllRemoved(value) => value, + KillStorageResult::SomeRemaining(value) => { + log::error!( + "`clear_prefix` failed to remove all keys for storage `{}` from pallet `{}`. THIS SHOULD NEVER HAPPEN! ๐Ÿšจ", + S::get(), P::get() + ); + value + }, + } as u64; + + log::info!("Removed `{}` `{}` `{}` keys ๐Ÿงน", keys_removed, P::get(), S::get()); + + DbWeight::get().reads_writes(keys_removed + 1, keys_removed) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + use crate::storage::unhashed::contains_prefixed_key; + + let hashed_prefix = storage_prefix(P::get().as_bytes(), S::get().as_bytes()); + match contains_prefixed_key(&hashed_prefix) { + true => log::info!("Found `{}` `{}` keys pre-removal ๐Ÿ‘€", P::get(), S::get()), + false => log::warn!( + "Migration RemoveStorage<{}, {}> can be removed (no keys found pre-removal).", + P::get(), + S::get() + ), + }; + Ok(sp_std::vec::Vec::new()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: sp_std::vec::Vec) -> Result<(), sp_runtime::TryRuntimeError> { + use crate::storage::unhashed::contains_prefixed_key; + + let hashed_prefix = storage_prefix(P::get().as_bytes(), S::get().as_bytes()); + match contains_prefixed_key(&hashed_prefix) { + true => { + log::error!("`{}` `{}` has keys remaining post-removal โ—", P::get(), S::get()); + return Err("Keys remaining post-removal, this should never happen ๐Ÿšจ".into()) + }, + false => log::info!("No `{}` `{}` keys found post-removal ๐ŸŽ‰", P::get(), S::get()), + }; + Ok(()) + } +} + /// A migration that can proceed in multiple steps. pub trait SteppedMigration { /// The cursor type that stores the progress (aka. state) of this migration. From 72f5c46c5ff4876e8a10200b355c2397100f5676 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 25 Mar 2024 16:26:26 +0100 Subject: [PATCH 2/4] pr doc --- prdoc/pr_3828.prdoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 prdoc/pr_3828.prdoc diff --git a/prdoc/pr_3828.prdoc b/prdoc/pr_3828.prdoc new file mode 100644 index 0000000000000..bf6f75c550451 --- /dev/null +++ b/prdoc/pr_3828.prdoc @@ -0,0 +1,12 @@ +# 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: "[FREAME] Remove storage migration type" + +doc: + - audience: Runtime Dev + description: | + Introduce migration type to remove data associated with a specific storage of a pallet. + +crates: + - name: frame-support From 189c6c07dbf4031ab83d52ea0615a796a7c47cd3 Mon Sep 17 00:00:00 2001 From: Muharem Date: Tue, 26 Mar 2024 16:30:54 +0100 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Liam Aharon Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- prdoc/pr_3828.prdoc | 2 +- substrate/frame/support/src/migrations.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_3828.prdoc b/prdoc/pr_3828.prdoc index bf6f75c550451..e93da425ad58a 100644 --- a/prdoc/pr_3828.prdoc +++ b/prdoc/pr_3828.prdoc @@ -1,7 +1,7 @@ # 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: "[FREAME] Remove storage migration type" +title: "[FRAME] Remove storage migration type" doc: - audience: Runtime Dev diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index a24b30d6fa2ee..8f841f051d0fc 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -462,7 +462,7 @@ impl, S: Get<&'static str>, DbWeight: Get> S::get() ), }; - Ok(sp_std::vec::Vec::new()) + Ok(Default::default()) } #[cfg(feature = "try-runtime")] From dedce62fb9106d667db08bba765edf118e0e2e47 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 25 Jun 2024 15:04:32 +0200 Subject: [PATCH 4/4] pr doc bump --- prdoc/pr_3828.prdoc | 1 + 1 file changed, 1 insertion(+) diff --git a/prdoc/pr_3828.prdoc b/prdoc/pr_3828.prdoc index e93da425ad58a..426625d5f23ef 100644 --- a/prdoc/pr_3828.prdoc +++ b/prdoc/pr_3828.prdoc @@ -10,3 +10,4 @@ doc: crates: - name: frame-support + bump: minor