From 7b73696f0730fa5d7076ea8789abb6d8941b360e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Tue, 13 Dec 2022 13:06:54 +0100 Subject: [PATCH 1/2] Do not write old state to storage in pre-migration check --- pallets/aleph/src/lib.rs | 2 + pallets/aleph/src/migrations/v0_to_v1.rs | 46 ++++++++++------ pallets/aleph/src/migrations/v1_to_v2.rs | 12 +---- pallets/elections/src/lib.rs | 2 + pallets/elections/src/migrations/v0_to_v1.rs | 28 ++++------ pallets/elections/src/migrations/v1_to_v2.rs | 50 +++++++++-------- pallets/elections/src/migrations/v2_to_v3.rs | 48 ++++++++++------- pallets/support/src/migration.rs | 57 ++------------------ 8 files changed, 107 insertions(+), 138 deletions(-) diff --git a/pallets/aleph/src/lib.rs b/pallets/aleph/src/lib.rs index a7198039b9..b358f092aa 100644 --- a/pallets/aleph/src/lib.rs +++ b/pallets/aleph/src/lib.rs @@ -16,6 +16,8 @@ #![cfg_attr(not(feature = "std"), no_std)] +extern crate core; + #[cfg(test)] mod mock; #[cfg(test)] diff --git a/pallets/aleph/src/migrations/v0_to_v1.rs b/pallets/aleph/src/migrations/v0_to_v1.rs index 3fe634e69e..79daed3678 100644 --- a/pallets/aleph/src/migrations/v0_to_v1.rs +++ b/pallets/aleph/src/migrations/v0_to_v1.rs @@ -1,15 +1,16 @@ -#[cfg(feature = "try-runtime")] -use frame_support::ensure; use frame_support::{ log, storage_alias, traits::{Get, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion}, weights::Weight, }; -#[cfg(feature = "try-runtime")] -use pallets_support::ensure_storage_version; -use pallets_support::StorageMigration; use primitives::SessionIndex; use sp_std::vec::Vec; +#[cfg(feature = "try-runtime")] +use { + codec::{Decode, Encode}, + frame_support::ensure, + pallets_support::ensure_storage_version, +}; use crate::Config; @@ -24,9 +25,11 @@ type Validators = StorageValue>; /// Flattening double `Option<>` storage. pub struct Migration(sp_std::marker::PhantomData<(T, P)>); -impl StorageMigration for Migration { - #[cfg(feature = "try-runtime")] - const MIGRATION_STORAGE_PREFIX: &'static [u8] = b"PALLET_ALEPH::V0_TO_V1_MIGRATION"; +#[cfg(feature = "try-runtime")] +#[derive(Decode, Encode)] +struct MigrationChecksState { + session: Option>, + validators: Option>>, } impl OnRuntimeUpgrade for Migration { @@ -87,18 +90,27 @@ impl OnRuntimeUpgrade for Migration { ensure_storage_version::

(0)?; - Self::store_temp("session", SessionForValidatorsChange::get()); - Self::store_temp("validators", Validators::::get()); + let session = SessionForValidatorsChange::get(); + let validators = Validators::::get(); - Ok(Vec::new()) + Ok(MigrationChecksState:: { + session, + validators, + } + .encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> Result<(), &'static str> { ensure_storage_version::

(1)?; + let MigrationChecksState { + session: old_session, + validators: old_validators, + } = >::decode(&mut &*state) + .map_err(|_| "Failed to decode old state")?; + let new_session = SessionForValidatorsChange::get(); - let old_session = Self::read_temp::>>("session"); match old_session { Some(Some(session)) => ensure!( @@ -106,20 +118,22 @@ impl OnRuntimeUpgrade for Migration { "Mismatch on `SessionForValidatorsChange`", ), _ => ensure!( - None == new_session, + new_session.is_none(), "New `SessionForValidatorsChange` should be `None`" ), }; let new_validators = Validators::::get(); - let old_validators = Self::read_temp::>>>("validators"); match old_validators { Some(Some(validators)) => ensure!( Some(validators) == new_validators, "Mismatch on `Validators`", ), - _ => ensure!(None == new_validators, "New `Validators` should be `None`"), + _ => ensure!( + new_validators.is_none(), + "New `Validators` should be `None`" + ), }; Ok(()) diff --git a/pallets/aleph/src/migrations/v1_to_v2.rs b/pallets/aleph/src/migrations/v1_to_v2.rs index 8db71da49e..0a8e9d31c4 100644 --- a/pallets/aleph/src/migrations/v1_to_v2.rs +++ b/pallets/aleph/src/migrations/v1_to_v2.rs @@ -1,15 +1,10 @@ -#[cfg(feature = "try-runtime")] -use frame_support::ensure; use frame_support::{ log, storage_alias, traits::{Get, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion}, weights::Weight, }; #[cfg(feature = "try-runtime")] -use pallets_support::ensure_storage_version; -use pallets_support::StorageMigration; -#[cfg(feature = "try-runtime")] -use sp_std::vec::Vec; +use {frame_support::ensure, pallets_support::ensure_storage_version, sp_std::vec::Vec}; use crate::Config; @@ -32,11 +27,6 @@ type Validators = StorageValue; /// - Validators pub struct Migration(sp_std::marker::PhantomData<(T, P)>); -impl StorageMigration for Migration { - #[cfg(feature = "try-runtime")] - const MIGRATION_STORAGE_PREFIX: &'static [u8] = b"PALLET_ALEPH::V1_TO_V2_MIGRATION"; -} - impl OnRuntimeUpgrade for Migration { fn on_runtime_upgrade() -> Weight { let mut writes = 0; diff --git a/pallets/elections/src/lib.rs b/pallets/elections/src/lib.rs index 747da8cad9..c1453d0b18 100644 --- a/pallets/elections/src/lib.rs +++ b/pallets/elections/src/lib.rs @@ -34,6 +34,8 @@ #![cfg_attr(not(feature = "std"), no_std)] +extern crate core; + mod impls; mod migrations; #[cfg(test)] diff --git a/pallets/elections/src/migrations/v0_to_v1.rs b/pallets/elections/src/migrations/v0_to_v1.rs index 589b87188d..949ece52eb 100644 --- a/pallets/elections/src/migrations/v0_to_v1.rs +++ b/pallets/elections/src/migrations/v0_to_v1.rs @@ -1,14 +1,15 @@ -#[cfg(feature = "try-runtime")] -use frame_support::ensure; use frame_support::{ log, storage_alias, traits::{Get, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion}, weights::Weight, }; -#[cfg(feature = "try-runtime")] -use pallets_support::ensure_storage_version; -use pallets_support::StorageMigration; use sp_std::vec::Vec; +#[cfg(feature = "try-runtime")] +use { + codec::{Decode, Encode}, + frame_support::ensure, + pallets_support::ensure_storage_version, +}; use crate::{ compute_validator_scaled_total_rewards, @@ -39,11 +40,6 @@ type ErasMembers = StorageValue, Validators)>; /// - `ErasMembers` contain tuple of (content of `Members`, empty vector). pub struct Migration(sp_std::marker::PhantomData<(T, P)>); -impl StorageMigration for Migration { - #[cfg(feature = "try-runtime")] - const MIGRATION_STORAGE_PREFIX: &'static [u8] = b"PALLET_ELECTIONS::V0_TO_V1_MIGRATION"; -} - impl OnRuntimeUpgrade for Migration { fn on_runtime_upgrade() -> Weight { log::info!(target: "pallet_elections", "Running migration from STORAGE_VERSION 0 to 1 for pallet elections"); @@ -85,15 +81,12 @@ impl OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { ensure_storage_version::

(0)?; - - let members = Members::::get().ok_or("No `Members` storage")?; - Self::store_temp("members", members); - - Ok(Vec::new()) + let members: Validators = Members::::get().ok_or("No `Members` storage")?; + Ok(members.encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(old_members: Vec) -> Result<(), &'static str> { ensure_storage_version::

(1)?; let mps = MembersPerSession::get().ok_or("No `MembersPerSession` in the storage")?; @@ -103,7 +96,8 @@ impl OnRuntimeUpgrade for Migration { NonReservedMembers::::get().ok_or("No `NonReservedMembers` in the storage")?; let eras_members = ErasMembers::::get().ok_or("No `ErasMembers` in the storage")?; - let old_members = Self::read_temp::>("members"); + let old_members = >::decode(&mut &*old_members) + .map_err(|_| "Failed to decode old members set")?; ensure!( reserved_members == old_members, diff --git a/pallets/elections/src/migrations/v1_to_v2.rs b/pallets/elections/src/migrations/v1_to_v2.rs index b6e141e758..42c8ade61e 100644 --- a/pallets/elections/src/migrations/v1_to_v2.rs +++ b/pallets/elections/src/migrations/v1_to_v2.rs @@ -1,15 +1,15 @@ -#[cfg(feature = "try-runtime")] -use frame_support::ensure; use frame_support::{ log, storage_alias, traits::{Get, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion}, weights::Weight, }; #[cfg(feature = "try-runtime")] -use pallets_support::ensure_storage_version; -use pallets_support::StorageMigration; -#[cfg(feature = "try-runtime")] -use sp_std::vec::Vec; +use { + codec::{Decode, Encode}, + frame_support::ensure, + pallets_support::ensure_storage_version, + sp_std::vec::Vec, +}; use crate::{migrations::Validators, Config, EraValidators}; @@ -42,9 +42,13 @@ type CurrentEraValidators = /// - `ErasMembers` `(reserved, non_reserved)` -> `CurrentEraValidators` `ErasValidators { reserved, non_reserved}` pub struct Migration(sp_std::marker::PhantomData<(T, P)>); -impl StorageMigration for Migration { - #[cfg(feature = "try-runtime")] - const MIGRATION_STORAGE_PREFIX: &'static [u8] = b"PALLET_ELECTIONS::V1_TO_V2_MIGRATION"; +#[cfg(feature = "try-runtime")] +#[derive(Decode, Encode)] +struct MigrationChecksState { + members_per_session: u32, + reserved_members: Validators, + non_reserved_members: Validators, + eras_members: (Validators, Validators), } impl OnRuntimeUpgrade for Migration { @@ -89,24 +93,23 @@ impl OnRuntimeUpgrade for Migration { let members_per_session = MembersPerSession::get().ok_or("No `MembersPerSession` in the storage")?; - Self::store_temp("members_per_session", members_per_session); - let reserved_members = ReservedMembers::::get().ok_or("No `ReservedMembers` in the storage")?; - Self::store_temp("reserved_members", reserved_members); - let non_reserved_members = NonReservedMembers::::get().ok_or("No `NonReservedMembers` in the storage")?; - Self::store_temp("non_reserved_members", non_reserved_members); - let eras_members = ErasMembers::::get().ok_or("No `ErasMembers` in the storage")?; - Self::store_temp("eras_members", eras_members); - Ok(Vec::new()) + Ok(MigrationChecksState:: { + members_per_session, + reserved_members, + non_reserved_members, + eras_members, + } + .encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> Result<(), &'static str> { ensure_storage_version::

(2)?; let committee_size = CommitteeSize::get().ok_or("No `CommitteeSize` in the storage")?; @@ -117,10 +120,13 @@ impl OnRuntimeUpgrade for Migration { let current_era_validators = CurrentEraValidators::::get().ok_or("No `CurrentEraValidators` in the storage")?; - let members_per_session = Self::read_temp::("members_per_session"); - let reserved_members = Self::read_temp::>("reserved_members"); - let non_reserved_members = Self::read_temp::>("non_reserved_members"); - let eras_members = Self::read_temp::<(Validators, Validators)>("eras_members"); + let MigrationChecksState { + members_per_session, + reserved_members, + non_reserved_members, + eras_members, + } = >::decode(&mut &*state) + .map_err(|_| "Failed to decode old state")?; ensure!( committee_size == members_per_session, diff --git a/pallets/elections/src/migrations/v2_to_v3.rs b/pallets/elections/src/migrations/v2_to_v3.rs index 27e29bd75b..b57306ab85 100644 --- a/pallets/elections/src/migrations/v2_to_v3.rs +++ b/pallets/elections/src/migrations/v2_to_v3.rs @@ -1,16 +1,16 @@ -#[cfg(feature = "try-runtime")] -use frame_support::ensure; use frame_support::{ log, storage_alias, traits::{Get, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion}, weights::Weight, }; -#[cfg(feature = "try-runtime")] -use pallets_support::ensure_storage_version; -use pallets_support::StorageMigration; use primitives::CommitteeSeats; #[cfg(feature = "try-runtime")] -use sp_std::vec::Vec; +use { + codec::{Decode, Encode}, + frame_support::ensure, + pallets_support::ensure_storage_version, + sp_std::vec::Vec, +}; use crate::{migrations::Validators, Config, EraValidators}; @@ -31,9 +31,11 @@ type NextEraCommitteeSize = StorageValue; /// `CommitteeSeats`. pub struct Migration(sp_std::marker::PhantomData<(T, P)>); -impl StorageMigration for Migration { - #[cfg(feature = "try-runtime")] - const MIGRATION_STORAGE_PREFIX: &'static [u8] = b"PALLET_ELECTIONS::V2_TO_V3_MIGRATION"; +#[cfg(feature = "try-runtime")] +#[derive(Decode, Encode)] +struct MigrationChecksState { + committee_size: Option, + next_era_committee_size: Option, } impl OnRuntimeUpgrade for Migration { @@ -108,16 +110,17 @@ impl OnRuntimeUpgrade for Migration { ensure_storage_version::

(2)?; let committee_size = CommitteeSize::get(); - Self::store_temp("committee_size", committee_size); - let next_era_committee_size = NextEraCommitteeSize::get(); - Self::store_temp("next_era_committee_size", next_era_committee_size); - Ok(Vec::new()) + Ok(MigrationChecksState { + committee_size, + next_era_committee_size, + } + .encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> Result<(), &'static str> { ensure_storage_version::

(3)?; let new_committee_size = CommitteeSize::get().ok_or("No `CommitteeSize` in the storage")?; @@ -129,10 +132,11 @@ impl OnRuntimeUpgrade for Migration { let next_era_reserved_validators = NextEraReservedValidators::::get() .ok_or("No `NextEraReservedValidators` in the storage")?; - let old_committee_size = - Self::read_temp::>("committee_size").unwrap_or_default(); - let old_next_era_committee_size = - Self::read_temp::>("next_era_committee_size").unwrap_or_default(); + let MigrationChecksState { + committee_size: old_committee_size, + next_era_committee_size: old_next_era_committee_size, + } = ::decode(&mut &*state) + .map_err(|_| "Failed to decode old state")?; let currently_reserved = current_era_validators.reserved.len(); ensure!( @@ -141,7 +145,9 @@ impl OnRuntimeUpgrade for Migration { ); ensure!( new_committee_size.non_reserved_seats - == old_committee_size.saturating_sub(currently_reserved as u32), + == old_committee_size + .unwrap_or_default() + .saturating_sub(currently_reserved as u32), "Mismatch between `CurrentEraValidators` and `CommitteeSize`" ); @@ -152,7 +158,9 @@ impl OnRuntimeUpgrade for Migration { ); ensure!( new_next_era_committee_size.non_reserved_seats - == old_next_era_committee_size.saturating_sub(next_reserved as u32), + == old_next_era_committee_size + .unwrap_or_default() + .saturating_sub(next_reserved as u32), "Mismatch between `NextEraReservedValidators` and `NextEraCommitteeSize`" ); diff --git a/pallets/support/src/migration.rs b/pallets/support/src/migration.rs index 095712ec9a..d43a8d5a3e 100644 --- a/pallets/support/src/migration.rs +++ b/pallets/support/src/migration.rs @@ -1,10 +1,3 @@ -#[cfg(feature = "try-runtime")] -use frame_support::{ - codec::{Decode, Encode}, - sp_io, - sp_std::vec::Vec, - storage::storage_prefix, -}; use frame_support::{ pallet_prelude::{PalletInfoAccess, StorageVersion, Weight}, traits::OnRuntimeUpgrade, @@ -15,63 +8,23 @@ use frame_support::{ /// /// This way, `try-runtime` no longer triggers checks. We do it by hand. pub trait StorageMigration: OnRuntimeUpgrade { - #[cfg(feature = "try-runtime")] - const MIGRATION_STORAGE_PREFIX: &'static [u8]; - #[allow(clippy::let_and_return)] fn migrate() -> Weight { #[cfg(feature = "try-runtime")] - Self::pre_upgrade().expect("Pre upgrade should succeed"); + let state = Self::pre_upgrade().expect("Pre upgrade should succeed"); let weight = Self::on_runtime_upgrade(); #[cfg(feature = "try-runtime")] - Self::post_upgrade(Vec::new()).expect("Post upgrade should succeed"); + Self::post_upgrade(state).expect("Post upgrade should succeed"); weight } - - /// Wrapper for `OnRuntimeUpgradeHelpersExt::set_temp_storage`. - /// - /// Together with the associated const `MIGRATION_STORAGE_PREFIX` they form a shortcut for: - /// ```rust - /// # use frame_support::traits::OnRuntimeUpgradeHelpersExt; - /// # use crate::pallet_elections::Config; - /// # use frame_support::storage::storage_prefix; - /// # use frame_support::pallet_prelude::PalletInfoAccess; - /// # use frame_support::sp_std; - /// - /// #[cfg(feature = "try-runtime")] - /// const MIGRATION_STORAGE_PREFIX: &[u8] = b"..."; - /// - /// # struct Migration(sp_std::marker::PhantomData<(T, P)>); - /// - /// #[cfg(feature = "try-runtime")] - /// impl OnRuntimeUpgradeHelpersExt for Migration { - /// fn storage_key(ident: &str) -> [u8; 32] { - /// storage_prefix(MIGRATION_STORAGE_PREFIX, ident.as_bytes()) - /// } - /// } - /// ``` - /// which would be required for every implementor of `StorageMigration`. - #[cfg(feature = "try-runtime")] - fn store_temp(storage_key: &str, data: T) { - let full_key = storage_prefix(Self::MIGRATION_STORAGE_PREFIX, storage_key.as_bytes()); - sp_io::storage::set(&full_key, &data.encode()); - } - - /// Wrapper for `OnRuntimeUpgradeHelpersExt::get_temp_storage`. - /// - /// Analogous to `Self::store_temp`. - #[cfg(feature = "try-runtime")] - fn read_temp(storage_key: &str) -> T { - let full_key = storage_prefix(Self::MIGRATION_STORAGE_PREFIX, storage_key.as_bytes()); - sp_io::storage::get(&full_key) - .and_then(|bytes| Decode::decode(&mut &*bytes).ok()) - .unwrap_or_else(|| panic!("No `{storage_key}` in the temp storage")) - } } +impl StorageMigration for T {} + +/// Ensure that the current pallet storage version matches `version`. pub fn ensure_storage_version(version: u16) -> Result<(), &'static str> { if StorageVersion::get::

() == StorageVersion::new(version) { Ok(()) From a2713efbaa4d6ae24bb51d092fb322f822a22ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Miko=C5=82ajczyk?= Date: Tue, 13 Dec 2022 13:14:02 +0100 Subject: [PATCH 2/2] O.o --- pallets/aleph/src/lib.rs | 2 -- pallets/elections/src/lib.rs | 2 -- 2 files changed, 4 deletions(-) diff --git a/pallets/aleph/src/lib.rs b/pallets/aleph/src/lib.rs index b358f092aa..a7198039b9 100644 --- a/pallets/aleph/src/lib.rs +++ b/pallets/aleph/src/lib.rs @@ -16,8 +16,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -extern crate core; - #[cfg(test)] mod mock; #[cfg(test)] diff --git a/pallets/elections/src/lib.rs b/pallets/elections/src/lib.rs index c1453d0b18..747da8cad9 100644 --- a/pallets/elections/src/lib.rs +++ b/pallets/elections/src/lib.rs @@ -34,8 +34,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -extern crate core; - mod impls; mod migrations; #[cfg(test)]