From 0c4bc5bb1dada540203ab2a6015567a4389513b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 30 Jan 2022 12:33:27 +0100 Subject: [PATCH 1/4] pallet-scheduler: Fix migrations V2 to V3 V2 already supported origins, so we need to move them over instead of setting it to `Root`. Besides that it also removes the custom `Releases` enum and moves it over to `StorageVersion`. --- frame/scheduler/src/lib.rs | 134 ++++++++++++++--------------------- frame/scheduler/src/tests.rs | 8 +-- 2 files changed, 57 insertions(+), 85 deletions(-) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 7ecd9024e9f96..4cd41ec9a6fed 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -63,7 +63,7 @@ use frame_support::{ dispatch::{DispatchError, DispatchResult, Dispatchable, Parameter}, traits::{ schedule::{self, DispatchTime, MaybeHashed}, - EnsureOrigin, Get, IsType, OriginTrait, PrivilegeCmp, + EnsureOrigin, Get, IsType, OriginTrait, PalletInfoAccess, PrivilegeCmp, StorageVersion, }, weights::{GetDispatchInfo, Weight}, }; @@ -132,22 +132,6 @@ pub type ScheduledOf = ScheduledV3Of; pub type Scheduled = ScheduledV2; -// A value placed in storage that represents the current version of the Scheduler storage. -// This value is used by the `on_runtime_upgrade` logic to determine whether we run -// storage migration logic. -#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, RuntimeDebug, TypeInfo)] -enum Releases { - V1, - V2, - V3, -} - -impl Default for Releases { - fn default() -> Self { - Releases::V1 - } -} - #[cfg(feature = "runtime-benchmarks")] mod preimage_provider { use frame_support::traits::PreimageRecipient; @@ -201,8 +185,12 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; + /// The current storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(3); + #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] #[pallet::without_storage_info] pub struct Pallet(_); @@ -268,12 +256,6 @@ pub mod pallet { pub(crate) type Lookup = StorageMap<_, Twox64Concat, Vec, TaskAddress>; - /// Storage version of the pallet. - /// - /// New networks start with last version. - #[pallet::storage] - pub(crate) type StorageVersion = StorageValue<_, Releases, ValueQuery>; - /// Events type. #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -308,23 +290,6 @@ pub mod pallet { RescheduleNoChange, } - #[pallet::genesis_config] - pub struct GenesisConfig; - - #[cfg(feature = "std")] - impl Default for GenesisConfig { - fn default() -> Self { - Self - } - } - - #[pallet::genesis_build] - impl GenesisBuild for GenesisConfig { - fn build(&self) { - StorageVersion::::put(Releases::V3); - } - } - #[pallet::hooks] impl Hooks> for Pallet { /// Execute the scheduled calls @@ -573,19 +538,19 @@ pub mod pallet { impl Pallet { /// Migrate storage format from V1 to V3. - /// Return true if migration is performed. - pub fn migrate_v1_to_v3() -> bool { - if StorageVersion::::get() == Releases::V1 { - StorageVersion::::put(Releases::V3); - - Agenda::::translate::< - Vec::Call, T::BlockNumber>>>, - _, - >(|_, agenda| { + /// + /// Returns the weight consumed by this migration. + pub fn migrate_v1_to_v3() -> Weight { + let mut weight = T::DbWeight::get().reads_writes(1, 1); + + Agenda::::translate::::Call, T::BlockNumber>>>, _>( + |_, agenda| { Some( agenda .into_iter() .map(|schedule| { + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + schedule.map(|schedule| ScheduledV3 { maybe_id: schedule.maybe_id, priority: schedule.priority, @@ -597,45 +562,54 @@ impl Pallet { }) .collect::>(), ) - }); + }, + ); - true - } else { - false - } + frame_support::storage::migration::remove_storage_prefix( + Self::name().as_bytes(), + b"StorageVersion", + &[], + ); + + StorageVersion::new(3).put::(); + + weight + T::DbWeight::get().writes(2) } /// Migrate storage format from V2 to V3. - /// Return true if migration is performed. + /// + /// Returns the weight consumed by this migration. pub fn migrate_v2_to_v3() -> Weight { - if StorageVersion::::get() == Releases::V2 { - StorageVersion::::put(Releases::V3); + let mut weight = T::DbWeight::get().reads_writes(1, 1); - let mut weight = T::DbWeight::get().reads_writes(1, 1); - - Agenda::::translate::>>, _>(|_, agenda| { - Some( - agenda - .into_iter() - .map(|schedule| { - weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - schedule.map(|schedule| ScheduledV3 { - maybe_id: schedule.maybe_id, - priority: schedule.priority, - call: schedule.call.into(), - maybe_periodic: schedule.maybe_periodic, - origin: system::RawOrigin::Root.into(), - _phantom: Default::default(), - }) + Agenda::::translate::>>, _>(|_, agenda| { + Some( + agenda + .into_iter() + .map(|schedule| { + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + schedule.map(|schedule| ScheduledV3 { + maybe_id: schedule.maybe_id, + priority: schedule.priority, + call: schedule.call.into(), + maybe_periodic: schedule.maybe_periodic, + origin: schedule.origin, + _phantom: Default::default(), }) - .collect::>(), - ) - }); + }) + .collect::>(), + ) + }); - weight - } else { - 0 - } + frame_support::storage::migration::remove_storage_prefix( + Self::name().as_bytes(), + b"StorageVersion", + &[], + ); + + StorageVersion::new(3).put::(); + + weight + T::DbWeight::get().writes(2) } #[cfg(feature = "try-runtime")] diff --git a/frame/scheduler/src/tests.rs b/frame/scheduler/src/tests.rs index 7681ede136d97..d2a795cb19fa4 100644 --- a/frame/scheduler/src/tests.rs +++ b/frame/scheduler/src/tests.rs @@ -21,7 +21,7 @@ use super::*; use crate::mock::{logger, new_test_ext, root, run_to_block, Call, LoggerCall, Scheduler, Test, *}; use frame_support::{ assert_err, assert_noop, assert_ok, - traits::{Contains, OnInitialize, PreimageProvider}, + traits::{Contains, GetStorageVersion, OnInitialize, PreimageProvider}, Hashable, }; use sp_runtime::traits::Hash; @@ -707,9 +707,7 @@ fn migration_to_v3_works() { frame_support::migration::put_storage_value(b"Scheduler", b"Agenda", &k, old); } - assert_eq!(StorageVersion::::get(), Releases::V1); - - assert!(Scheduler::migrate_v1_to_v3()); + Scheduler::migrate_v1_to_v3(); assert_eq_uvec!( Agenda::::iter().collect::>(), @@ -783,7 +781,7 @@ fn migration_to_v3_works() { ] ); - assert_eq!(StorageVersion::::get(), Releases::V3); + assert_eq!(Scheduler::current_storage_version(), 3); }); } From aee9612897edfd0f54073812250f344639b38298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 30 Jan 2022 13:25:27 +0100 Subject: [PATCH 2/4] Fixes --- frame/democracy/src/tests.rs | 2 +- frame/scheduler/src/lib.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/frame/democracy/src/tests.rs b/frame/democracy/src/tests.rs index 641014923c233..0fe83a07610d1 100644 --- a/frame/democracy/src/tests.rs +++ b/frame/democracy/src/tests.rs @@ -63,7 +63,7 @@ frame_support::construct_runtime!( { System: frame_system::{Pallet, Call, Config, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Scheduler: pallet_scheduler::{Pallet, Call, Storage, Config, Event}, + Scheduler: pallet_scheduler::{Pallet, Call, Storage, Event}, Democracy: pallet_democracy::{Pallet, Call, Storage, Config, Event}, } ); diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 4cd41ec9a6fed..98424638d32e5 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -614,13 +614,12 @@ impl Pallet { #[cfg(feature = "try-runtime")] pub fn pre_migrate_to_v3() -> Result<(), &'static str> { - assert!(StorageVersion::::get() < Releases::V3); Ok(()) } #[cfg(feature = "try-runtime")] pub fn post_migrate_to_v3() -> Result<(), &'static str> { - assert!(StorageVersion::::get() == Releases::V3); + assert!(Self::current_storage_version() == 3); for k in Agenda::::iter_keys() { let _ = Agenda::::try_get(k).map_err(|()| "Invalid item in Agenda")?; } From 7cb619077726cf795b9819890a41147178b202dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 30 Jan 2022 13:33:13 +0100 Subject: [PATCH 3/4] Fixes --- bin/node/cli/src/chain_spec.rs | 1 - bin/node/testing/src/genesis.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 6fd57e31e466e..11516f964903a 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -362,7 +362,6 @@ pub fn testnet_genesis( assets: Default::default(), gilt: Default::default(), transaction_storage: Default::default(), - scheduler: Default::default(), transaction_payment: Default::default(), } } diff --git a/bin/node/testing/src/genesis.rs b/bin/node/testing/src/genesis.rs index 2e2f3f9a5a80a..8d2b53b0b7210 100644 --- a/bin/node/testing/src/genesis.rs +++ b/bin/node/testing/src/genesis.rs @@ -91,7 +91,6 @@ pub fn config_endowed(code: Option<&[u8]>, extra_endowed: Vec) -> Gen assets: Default::default(), gilt: Default::default(), transaction_storage: Default::default(), - scheduler: Default::default(), transaction_payment: Default::default(), } } From 9e55ff60f7930fe55e8490623f8c6716ac5fec95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 31 Jan 2022 11:37:08 +0100 Subject: [PATCH 4/4] :facepalm: --- frame/scheduler/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/scheduler/src/lib.rs b/frame/scheduler/src/lib.rs index 98424638d32e5..f9077d4c8c8fa 100644 --- a/frame/scheduler/src/lib.rs +++ b/frame/scheduler/src/lib.rs @@ -619,6 +619,8 @@ impl Pallet { #[cfg(feature = "try-runtime")] pub fn post_migrate_to_v3() -> Result<(), &'static str> { + use frame_support::dispatch::GetStorageVersion; + assert!(Self::current_storage_version() == 3); for k in Agenda::::iter_keys() { let _ = Agenda::::try_get(k).map_err(|()| "Invalid item in Agenda")?;