From 9e030be41b9cf713274e179d27cfdbca07d66a27 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Mon, 24 Apr 2023 17:14:17 +0200 Subject: [PATCH 01/31] Try-state: DispatchResult as return type --- frame/collective/src/lib.rs | 5 +- frame/election-provider-support/src/lib.rs | 2 +- frame/fast-unstake/src/lib.rs | 6 +- frame/nomination-pools/src/lib.rs | 80 +++++++++++++++------- frame/support/src/traits/hooks.rs | 2 +- 5 files changed, 62 insertions(+), 33 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 0ecf008fee80e..f7be5b90351be 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -348,9 +348,8 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { - Self::do_try_state()?; - Ok(()) + fn try_state(_n: BlockNumberFor) -> DispatchResult { + Self::do_try_state() } } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 750ccca46213f..f3cd918fc66ec 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -564,7 +564,7 @@ pub trait SortedListProvider { /// Check internal state of the list. Only meant for debugging. #[cfg(feature = "try-runtime")] - fn try_state() -> Result<(), &'static str>; + fn try_state() -> DispatchResult; /// If `who` changes by the returned amount they are guaranteed to have a worst case change /// in their list position. diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 3e79bf4077d20..946e98714fe6b 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -228,12 +228,12 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> Result<(), &'static str> { + fn try_state(_n: T::BlockNumber) -> DispatchResult { // ensure that the value of `ErasToCheckPerBlock` is less than // `T::MaxErasToCheckPerBlock`. - assert!( + ensure!( ErasToCheckPerBlock::::get() <= T::MaxErasToCheckPerBlock::get(), - "the value of `ErasToCheckPerBlock` is greater than `T::MaxErasToCheckPerBlock`", + DispatchError::Other("the value of `ErasToCheckPerBlock` is greater than `T::MaxErasToCheckPerBlock`"), ); Ok(()) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 78f0c730ce5dd..7a0be23a95ab7 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2627,7 +2627,7 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { + fn try_state(_n: BlockNumberFor) -> DispatchResult { Self::do_try_state(u8::MAX) } @@ -3056,7 +3056,7 @@ impl Pallet { /// multiple `level`s, where the higher the level, the more checks we performs. So, /// `try_state(255)` is the strongest sanity check, and `0` performs no checks. #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] - pub fn do_try_state(level: u8) -> Result<(), &'static str> { + pub fn do_try_state(level: u8) -> DispatchResult { if level.is_zero() { return Ok(()) } @@ -3064,12 +3064,26 @@ impl Pallet { // result in the same set of keys, in the same order. let bonded_pools = BondedPools::::iter_keys().collect::>(); let reward_pools = RewardPools::::iter_keys().collect::>(); - assert_eq!(bonded_pools, reward_pools); + ensure!( + bonded_pools == reward_pools, + DispatchError::Other( + "`BondedPools` and `RewardPools` must all have the EXACT SAME key-set." + ) + ); - assert!(Metadata::::iter_keys().all(|k| bonded_pools.contains(&k))); - assert!(SubPoolsStorage::::iter_keys().all(|k| bonded_pools.contains(&k))); + ensure!( + SubPoolsStorage::::iter_keys().all(|k| bonded_pools.contains(&k)), + DispatchError::Other("`SubPoolsStorage` must be a subset of the above superset.") + ); + ensure!( + Metadata::::iter_keys().all(|k| bonded_pools.contains(&k)), + DispatchError::Other("`Metadata` keys must be a subset of the above superset.") + ); - assert!(MaxPools::::get().map_or(true, |max| bonded_pools.len() <= (max as usize))); + ensure!( + MaxPools::::get().map_or(true, |max| bonded_pools.len() <= (max as usize)), + DispatchError::Other("the count of the above set must be less than `MaxPools`") + ); for id in reward_pools { let account = Self::create_reward_account(id); @@ -3089,9 +3103,12 @@ impl Pallet { let mut pools_members = BTreeMap::::new(); let mut pools_members_pending_rewards = BTreeMap::>::new(); let mut all_members = 0u32; - PoolMembers::::iter().for_each(|(_, d)| { + PoolMembers::::iter().try_for_each(|(_, d)| -> DispatchResult { let bonded_pool = BondedPools::::get(d.pool_id).unwrap(); - assert!(!d.total_points().is_zero(), "no member should have zero points: {d:?}"); + ensure!( + !d.total_points().is_zero(), + DispatchError::Other("No member should have zero points: {d:?}") + ); *pools_members.entry(d.pool_id).or_default() += 1; all_members += 1; @@ -3104,9 +3121,11 @@ impl Pallet { let pending_rewards = d.pending_rewards(current_rc).unwrap(); *pools_members_pending_rewards.entry(d.pool_id).or_default() += pending_rewards; } // else this pool has been heavily slashed and cannot have any rewards anymore. - }); - RewardPools::::iter_keys().for_each(|id| { + Ok(()) + })?; + + RewardPools::::iter_keys().try_for_each(|id| -> DispatchResult { // the sum of the pending rewards must be less than the leftover balance. Since the // reward math rounds down, we might accumulate some dust here. log!( @@ -3116,30 +3135,41 @@ impl Pallet { pools_members_pending_rewards.get(&id), RewardPool::::current_balance(id) ); - assert!( + ensure!( RewardPool::::current_balance(id) >= - pools_members_pending_rewards.get(&id).copied().unwrap_or_default() - ) - }); + pools_members_pending_rewards.get(&id).copied().unwrap_or_default(), + DispatchError::Other( + "The sum of the pending rewards must be less than the leftover balance." + ) + ); + Ok(()) + })?; - BondedPools::::iter().for_each(|(id, inner)| { + BondedPools::::iter().try_for_each(|(id, inner)| -> DispatchResult { let bonded_pool = BondedPool { id, inner }; - assert_eq!( - pools_members.get(&id).copied().unwrap_or_default(), - bonded_pool.member_counter + ensure!( + pools_members.get(&id).copied().unwrap_or_default() == + bonded_pool.member_counter, + DispatchError::Other("Each `BondedPool.member_counter` must be equal to the actual count of members of this pool") + ); + ensure!(MaxPoolMembersPerPool::::get() + .map_or(true, |max| bonded_pool.member_counter <= max), + DispatchError::Other("Each `BondedPool.member_counter` must be less than `MaxPoolMembersPerPool`") ); - assert!(MaxPoolMembersPerPool::::get() - .map_or(true, |max| bonded_pool.member_counter <= max)); let depositor = PoolMembers::::get(&bonded_pool.roles.depositor).unwrap(); - assert!( + ensure!( bonded_pool.is_destroying_and_only_depositor(depositor.active_points()) || depositor.active_points() >= MinCreateBond::::get(), - "depositor must always have MinCreateBond stake in the pool, except for when the \ - pool is being destroyed and the depositor is the last member", + DispatchError::Other("depositor must always have MinCreateBond stake in the pool, except for when the \ + pool is being destroyed and the depositor is the last member"), ); - }); - assert!(MaxPoolMembers::::get().map_or(true, |max| all_members <= max)); + Ok(()) + })?; + ensure!( + MaxPoolMembers::::get().map_or(true, |max| all_members <= max), + DispatchError::Other("There can't be more than `MaxPoolMembers` in a pool.") + ); if level <= 1 { return Ok(()) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 5fea98ee036b5..d10fefe0f0a4b 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -277,7 +277,7 @@ pub trait Hooks { /// /// This hook should not alter any storage. #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumber) -> Result<(), &'static str> { + fn try_state(_n: BlockNumber) -> DispatchResult { Ok(()) } From 1b0155e176c1752ead6037f0ee1930cbbe3cdd56 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Mon, 24 Apr 2023 23:17:54 +0200 Subject: [PATCH 02/31] try_state for the rest of the pallets --- frame/nomination-pools/src/lib.rs | 2 +- frame/staking/src/pallet/impls.rs | 43 +++++++++++-------- .../procedural/src/pallet/expand/hooks.rs | 2 +- frame/support/src/dispatch.rs | 2 +- frame/support/src/traits/try_runtime.rs | 4 +- utils/frame/try-runtime/cli/src/lib.rs | 2 +- 6 files changed, 31 insertions(+), 24 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 7a0be23a95ab7..637629ba74cf3 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -3107,7 +3107,7 @@ impl Pallet { let bonded_pool = BondedPools::::get(d.pool_id).unwrap(); ensure!( !d.total_points().is_zero(), - DispatchError::Other("No member should have zero points: {d:?}") + DispatchError::Other("No member should have zero points") ); *pools_members.entry(d.pool_id).or_default() += 1; all_members += 1; diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 3058d3edc6fd6..a45e3da3507c5 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1688,11 +1688,11 @@ impl StakingInterface for Pallet { #[cfg(any(test, feature = "try-runtime"))] impl Pallet { - pub(crate) fn do_try_state(_: BlockNumberFor) -> Result<(), &'static str> { + pub(crate) fn do_try_state(_: BlockNumberFor) -> DispatchResult { ensure!( T::VoterList::iter() .all(|x| >::contains_key(&x) || >::contains_key(&x)), - "VoterList contains non-staker" + DispatchError::Other("VoterList contains non-staker") ); Self::check_nominators()?; @@ -1701,31 +1701,32 @@ impl Pallet { Self::check_count() } - fn check_count() -> Result<(), &'static str> { + fn check_count() -> DispatchResult { ensure!( ::VoterList::count() == Nominators::::count() + Validators::::count(), - "wrong external count" + DispatchError::Other("wrong external count") ); ensure!( ::TargetList::count() == Validators::::count(), - "wrong external count" + DispatchError::Other("wrong external count") ); ensure!( ValidatorCount::::get() <= ::MaxWinners::get(), - "validator count exceeded election max winners" + DispatchError::Other("validator count exceeded election max winners") ); Ok(()) } - fn check_ledgers() -> Result<(), &'static str> { + fn check_ledgers() -> DispatchResult { Bonded::::iter() .map(|(_, ctrl)| Self::ensure_ledger_consistent(ctrl)) - .collect::>() + .collect::, _>>()?; + Ok(()) } - fn check_exposures() -> Result<(), &'static str> { + fn check_exposures() -> DispatchResult { // a check per validator to ensure the exposure struct is always sane. let era = Self::active_era().unwrap().index; ErasStakers::::iter_prefix_values(era) @@ -1737,14 +1738,14 @@ impl Pallet { .iter() .map(|e| e.value) .fold(Zero::zero(), |acc, x| acc + x), - "wrong total exposure.", + DispatchError::Other("wrong total exposure."), ); Ok(()) }) - .collect::>() + .collect::() } - fn check_nominators() -> Result<(), &'static str> { + fn check_nominators() -> DispatchResult { // a check per nominator to ensure their entire stake is correctly distributed. Will only // kick-in if the nomination was submitted before the current era. let era = Self::active_era().unwrap().index; @@ -1758,27 +1759,33 @@ impl Pallet { } }, ) - .map(|nominator| { + .map(|nominator| -> DispatchResult { // must be bonded. Self::ensure_is_stash(&nominator)?; let mut sum = BalanceOf::::zero(); T::SessionInterface::validators() .iter() .map(|v| Self::eras_stakers(era, v)) - .map(|e| { + .map(|e| -> DispatchResult { let individual = e.others.iter().filter(|e| e.who == nominator).collect::>(); let len = individual.len(); match len { 0 => { /* not supporting this validator at all. */ }, 1 => sum += individual[0].value, - _ => return Err("nominator cannot back a validator more than once."), + _ => + return Err(DispatchError::Other( + "nominator cannot back a validator more than once.", + )), }; Ok(()) }) - .collect::>() + .collect::, _>>()?; + Ok(()) }) - .collect::>() + .collect::, _>>()?; + + Ok(()) } fn ensure_is_stash(who: &T::AccountId) -> Result<(), &'static str> { @@ -1786,7 +1793,7 @@ impl Pallet { Ok(()) } - fn ensure_ledger_consistent(ctrl: T::AccountId) -> Result<(), &'static str> { + fn ensure_ledger_consistent(ctrl: T::AccountId) -> DispatchResult { // ensures ledger.total == ledger.active + sum(ledger.unlocking). let ledger = Self::ledger(ctrl.clone()).ok_or("Not a controller.")?; let real_total: BalanceOf = diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 711b78e642d9f..0ec8dffc566b2 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -215,7 +215,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { fn try_state( n: ::BlockNumber, _s: #frame_support::traits::TryStateSelect - ) -> Result<(), &'static str> { + ) -> DispatchResult { #log_try_state < Self as #frame_support::traits::Hooks< diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 9e949ef6d4a21..e8ddfd12d4a4f 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2095,7 +2095,7 @@ macro_rules! decl_module { fn try_state( _: <$trait_instance as $system::Config>::BlockNumber, _: $crate::traits::TryStateSelect, - ) -> Result<(), &'static str> { + ) -> DispatchResult { let pallet_name = << $trait_instance as diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index bebc248721c99..58e643333be7e 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -129,7 +129,7 @@ impl core::str::FromStr for UpgradeCheckSelect { /// This hook should not alter any storage. pub trait TryState { /// Execute the state checks. - fn try_state(_: BlockNumber, _: Select) -> Result<(), &'static str>; + fn try_state(_: BlockNumber, _: Select) -> DispatchResult; } #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] @@ -139,7 +139,7 @@ impl TryState Result<(), &'static str> { + fn try_state(n: BlockNumber, targets: Select) -> DispatchResult { match targets { Select::None => Ok(()), Select::All => { diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 9268ef2edba8b..70905bfb30e0a 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -141,7 +141,7 @@ //! //! ```ignore //! #[cfg(feature = "try-runtime")] -//! fn try_state(_: BlockNumber) -> Result<(), &'static str> {} +//! fn try_state(_: BlockNumber) -> DispatchResult {} //! ``` //! //! which is called on numerous code paths in the try-runtime tool. These checks should ensure that From c78308bf589751ef764d73e125a9bdb38ce22bf4 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Wed, 26 Apr 2023 15:58:45 +0200 Subject: [PATCH 03/31] pre_upgrade --- frame/assets/src/migration.rs | 4 ++-- frame/bags-list/src/migrations.rs | 8 ++++---- frame/contracts/src/migration.rs | 6 +++--- frame/democracy/src/migrations.rs | 6 +++--- frame/fast-unstake/src/migrations.rs | 4 ++-- frame/multisig/src/migrations.rs | 4 ++-- frame/nfts/src/migration.rs | 4 ++-- frame/nomination-pools/src/migration.rs | 13 +++++++------ frame/preimage/src/migration.rs | 4 ++-- frame/referenda/src/migration.rs | 4 ++-- frame/scheduler/src/migration.rs | 6 +++--- frame/staking/src/migrations.rs | 20 ++++++++++---------- frame/support/src/migrations.rs | 2 +- frame/support/src/traits/hooks.rs | 6 +++--- utils/frame/try-runtime/cli/src/lib.rs | 2 +- 15 files changed, 47 insertions(+), 46 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 71da8823e92ef..623f5396384f2 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -92,10 +92,10 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( Pallet::::on_chain_storage_version() == 0, - "must upgrade linearly" + DispatchError::Other("must upgrade linearly") ); let prev_count = Asset::::iter().count(); Ok((prev_count as u32).encode()) diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 5f9bb8f73ac60..64cccaa062365 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -35,7 +35,7 @@ impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { // The old explicit storage item. #[frame_support::storage_alias] type CounterForListNodes, I: 'static> = @@ -88,14 +88,14 @@ mod old { pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { // The list node data should be corrupt at this point, so this is zero. - ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); + ensure!(crate::ListNodes::::iter().count() == 0, DispatchError::Other("list node data is not corrupt")); // We can use the helper `old::ListNode` to get the existing data. let iter_node_count: u32 = old::ListNodes::::iter().count() as u32; let tracked_node_count: u32 = old::CounterForListNodes::::get(); crate::log!(info, "number of nodes before: {:?} {:?}", iter_node_count, tracked_node_count); - ensure!(iter_node_count == tracked_node_count, "Node count is wrong."); + ensure!(iter_node_count == tracked_node_count, DispatchError::Other("Node count is wrong.")); Ok(iter_node_count.encode()) } diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 96a4c3203474c..108e19d7a6727 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -66,7 +66,7 @@ impl OnRuntimeUpgrade for Migration { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { let version = >::on_chain_storage_version(); if version == 7 { @@ -355,11 +355,11 @@ mod v8 { } #[cfg(feature = "try-runtime")] - pub fn pre_upgrade() -> Result<(), &'static str> { + pub fn pre_upgrade() -> Result<(), DispatchError> { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); - ensure!(reserved >= value.storage_deposit, "Reserved balance out of sync."); + ensure!(reserved >= value.storage_deposit, DispatchError::Other("Reserved balance out of sync.")); } Ok(()) } diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index fe2e445bd02a6..9d0e83e6f613f 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -61,12 +61,12 @@ pub mod v1 { impl> OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { - assert_eq!(StorageVersion::get::>(), 0, "can only upgrade from version 0"); + fn pre_upgrade() -> Result, DispatchError> { + ensure!(StorageVersion::get::>() == 0, DispatchError::Other("can only upgrade from version 0")); let props_count = v0::PublicProps::::get().len(); log::info!(target: TARGET, "{} public proposals will be migrated.", props_count,); - ensure!(props_count <= T::MaxProposals::get() as usize, "too many proposals"); + ensure!(props_count <= T::MaxProposals::get() as usize, DispatchError::Other("too many proposals")); let referenda_count = v0::ReferendumInfoOf::::iter().count(); log::info!(target: TARGET, "{} referenda will be migrated.", referenda_count); diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index e5ef919298a4f..f5cf806a7c85f 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -65,8 +65,8 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { - assert_eq!(Pallet::::on_chain_storage_version(), 0); + fn pre_upgrade() -> Result, DispatchError> { + ensure!(Pallet::::on_chain_storage_version() == 0, DispatchError::Other("The onchain storage version must be zero for the migration to execute.")); Ok(Default::default()) } diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 2a9c858a5552f..da25dae46b0e9 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -43,10 +43,10 @@ pub mod v1 { pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 1, "this migration can be deleted"); + ensure!(onchain < 1, DispatchError::Other("this migration can be deleted")); log!(info, "Number of calls to refund and delete: {}", Calls::::iter().count()); diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index 33ee87e4b9284..e0d035a6c7acf 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -90,10 +90,10 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); - ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1."); + ensure!(onchain_version == 0 && current_version == 1, DispatchError::Other("migration from version 0 to 1.")); let prev_count = Collection::::iter().count(); Ok((prev_count as u32).encode()) } diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 45d6424119900..217f95e599a21 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -352,12 +352,13 @@ pub mod v2 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { // all reward accounts must have more than ED. RewardPools::::iter().for_each(|(id, _)| { - assert!( + ensure!( T::Currency::free_balance(&Pallet::::create_reward_account(id)) >= - T::Currency::minimum_balance() + T::Currency::minimum_balance(), + DispatchError::Other("Reward accounts must have greater balance than ED.") ) }); @@ -435,7 +436,7 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" @@ -535,7 +536,7 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" @@ -620,7 +621,7 @@ pub mod v5 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" diff --git a/frame/preimage/src/migration.rs b/frame/preimage/src/migration.rs index be352201da6cd..d4e1f7f071cdc 100644 --- a/frame/preimage/src/migration.rs +++ b/frame/preimage/src/migration.rs @@ -78,8 +78,8 @@ pub mod v1 { impl OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { - assert_eq!(StorageVersion::get::>(), 0, "can only upgrade from version 0"); + fn pre_upgrade() -> Result, DispatchError> { + ensure!(StorageVersion::get::>() == 0, DispatchError::Other("can only upgrade from version 0")); let images = v0::image_count::().expect("v0 storage corrupted"); log::info!(target: TARGET, "Migrating {} images", &images); diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index c27ab452ac637..ba947f56cec78 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -95,9 +95,9 @@ pub mod v1 { pub struct MigrateV0ToV1(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for MigrateV0ToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { let onchain_version = Pallet::::on_chain_storage_version(); - assert_eq!(onchain_version, 0, "migration from version 0 to 1."); + ensure!(onchain_version, 0, DispatchError::Other("migration from version 0 to 1.")); let referendum_count = v0::ReferendumInfoFor::::iter().count(); log::info!( target: TARGET, diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 5b3a7631eeac9..8bd6da6076fb4 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -97,8 +97,8 @@ pub mod v3 { impl> OnRuntimeUpgrade for MigrateToV4 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { - assert_eq!(StorageVersion::get::>(), 3, "Can only upgrade from version 3"); + fn pre_upgrade() -> Result, DispatchError> { + ensure!(StorageVersion::get::>() == 3, DispatchError::Other("Can only upgrade from version 3")); let agendas = Agenda::::iter_keys().count() as u32; let decodable_agendas = Agenda::::iter_values().count() as u32; @@ -210,7 +210,7 @@ pub mod v4 { impl OnRuntimeUpgrade for CleanupAgendas { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { assert_eq!( StorageVersion::get::>(), 4, diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 23bcfa4398627..0b02957a16bb3 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -58,10 +58,10 @@ pub mod v13 { pub struct MigrateToV13(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV13 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, - "Required v12 before upgrading to v13" + DispatchError::Other("Required v12 before upgrading to v13") ); Ok(Default::default()) @@ -114,16 +114,16 @@ pub mod v12 { pub struct MigrateToV12(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV12 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, - "Expected v11 before upgrading to v12" + DispatchError::Other("Expected v11 before upgrading to v12") ); if HistoryDepth::::exists() { frame_support::ensure!( T::HistoryDepth::get() == HistoryDepth::::get(), - "Provided value of HistoryDepth should be same as the existing storage value" + DispatchError::Other("Provided value of HistoryDepth should be same as the existing storage value") ); } else { log::info!("No HistoryDepth in storage; nothing to remove"); @@ -170,16 +170,16 @@ pub mod v11 { for MigrateToV11 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V10_0_0, - "must upgrade linearly" + DispatchError::Other("must upgrade linearly") ); let old_pallet_prefix = twox_128(N::get().as_bytes()); frame_support::ensure!( sp_io::storage::next_key(&old_pallet_prefix).is_some(), - "no data for the old pallet name has been detected" + DispatchError::Other("no data for the old pallet name has been detected") ); Ok(Default::default()) @@ -332,10 +332,10 @@ pub mod v9 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V8_0_0, - "must upgrade linearly" + DispatchError::Other("must upgrade linearly") ); let prev_count = T::VoterList::count(); diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index f54b73aad8889..ce28292d05e82 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -158,7 +158,7 @@ impl, DbWeight: Get> frame_support::traits } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { let hashed_prefix = twox_128(P::get().as_bytes()); match contains_prefixed_key(&hashed_prefix) { true => log::info!("Found {} keys pre-removal 👀", P::get()), diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index d10fefe0f0a4b..e2abf0bb12dc3 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -167,7 +167,7 @@ pub trait OnRuntimeUpgrade { /// This hook must not write to any state, as it would make the main `on_runtime_upgrade` path /// inaccurate. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { Ok(Vec::new()) } @@ -289,7 +289,7 @@ pub trait Hooks { /// /// 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> { + fn pre_upgrade() -> Result, DispatchError> { Ok(Vec::new()) } @@ -383,7 +383,7 @@ mod tests { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { Pre::mutate(|s| s.push(stringify!($name))); Ok(Vec::new()) } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 70905bfb30e0a..4042c1904ec9d 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -122,7 +122,7 @@ //! //! ```ignore //! #[cfg(feature = "try-runtime")] -//! fn pre_upgrade() -> Result, &'static str> {} +//! fn pre_upgrade() -> Result, DispatchError> {} //! //! #[cfg(feature = "try-runtime")] //! fn post_upgrade(state: Vec) -> Result<(), &'static str> {} From 733d461fdd7cdafc8ae8d25734c5abfeae22eb0c Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Wed, 26 Apr 2023 19:27:15 +0200 Subject: [PATCH 04/31] post_upgrade --- frame/assets/src/migration.rs | 20 +++-- frame/bags-list/src/migrations.rs | 22 ++++-- frame/contracts/src/migration.rs | 27 ++++--- frame/democracy/src/migrations.rs | 24 ++++-- frame/fast-unstake/src/migrations.rs | 14 +++- frame/multisig/src/migrations.rs | 10 ++- frame/nfts/src/migration.rs | 20 +++-- frame/nomination-pools/src/migration.rs | 76 ++++++++++++------- frame/preimage/src/migration.rs | 9 ++- frame/referenda/src/migration.rs | 13 ++-- frame/scheduler/src/migration.rs | 26 +++++-- frame/staking/src/migrations.rs | 30 +++++--- .../procedural/src/pallet/expand/hooks.rs | 2 +- frame/support/src/dispatch.rs | 4 +- frame/support/src/migrations.rs | 2 +- frame/support/src/traits/hooks.rs | 6 +- utils/frame/try-runtime/cli/src/lib.rs | 2 +- 17 files changed, 205 insertions(+), 102 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 623f5396384f2..c7be9c0d98da3 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -102,27 +102,31 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + fn post_upgrade(prev_count: Vec) -> DispatchResult { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); let post_count = Asset::::iter().count() as u32; - assert_eq!( - prev_count, post_count, - "the asset count before and after the migration should be the same" + ensure!( + prev_count == post_count, + DispatchError::Other( + "the asset count before and after the migration should be the same" + ) ); let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); frame_support::ensure!(current_version == 1, "must_upgrade"); - assert_eq!( - current_version, onchain_version, - "after migration, the current_version and onchain_version should be the same" + ensure!( + current_version == onchain_version, + DispatchError::Other( + "after migration, the current_version and onchain_version should be the same" + ) ); Asset::::iter().for_each(|(_id, asset)| { - assert!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state") + ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, DispatchError::Other("assets should only be live or frozen. None should be in destroying status, or undefined state")) }); Ok(()) } diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 64cccaa062365..f5f9402117e4a 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -90,12 +90,18 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { // The list node data should be corrupt at this point, so this is zero. - ensure!(crate::ListNodes::::iter().count() == 0, DispatchError::Other("list node data is not corrupt")); + ensure!( + crate::ListNodes::::iter().count() == 0, + DispatchError::Other("list node data is not corrupt") + ); // We can use the helper `old::ListNode` to get the existing data. let iter_node_count: u32 = old::ListNodes::::iter().count() as u32; let tracked_node_count: u32 = old::CounterForListNodes::::get(); crate::log!(info, "number of nodes before: {:?} {:?}", iter_node_count, tracked_node_count); - ensure!(iter_node_count == tracked_node_count, DispatchError::Other("Node count is wrong.")); + ensure!( + iter_node_count == tracked_node_count, + DispatchError::Other("Node count is wrong.") + ); Ok(iter_node_count.encode()) } @@ -119,7 +125,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { } #[cfg(feature = "try-runtime")] - fn post_upgrade(node_count_before: Vec) -> Result<(), &'static str> { + fn post_upgrade(node_count_before: Vec) -> DispatchResult { let node_count_before: u32 = Decode::decode(&mut node_count_before.as_slice()) .expect("the state parameter should be something that was generated by pre_upgrade"); // Now the list node data is not corrupt anymore. @@ -131,8 +137,14 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { iter_node_count_after, tracked_node_count_after, ); - ensure!(iter_node_count_after == node_count_before, "Not all nodes were migrated."); - ensure!(tracked_node_count_after == iter_node_count_after, "Node count is wrong."); + ensure!( + iter_node_count_after == node_count_before, + DispatchError::Other("Not all nodes were migrated.") + ); + ensure!( + tracked_node_count_after == iter_node_count_after, + DispatchError::Other("Node count is wrong.") + ); Ok(()) } } diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 108e19d7a6727..e876d0c1015cb 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -77,7 +77,7 @@ impl OnRuntimeUpgrade for Migration { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> DispatchResult { let version = Decode::decode(&mut state.as_ref()).map_err(|_| "Cannot decode version")?; post_checks::post_upgrade::(version) } @@ -359,7 +359,10 @@ mod v8 { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); - ensure!(reserved >= value.storage_deposit, DispatchError::Other("Reserved balance out of sync.")); + ensure!( + reserved >= value.storage_deposit, + DispatchError::Other("Reserved balance out of sync.") + ); } Ok(()) } @@ -418,7 +421,7 @@ mod post_checks { type ContractInfoOf = StorageMap, Twox64Concat, ::AccountId, V>; - pub fn post_upgrade(old_version: StorageVersion) -> Result<(), &'static str> { + pub fn post_upgrade(old_version: StorageVersion) -> DispatchResult { if old_version < 7 { return Ok(()) } @@ -434,7 +437,7 @@ mod post_checks { Ok(()) } - fn v8() -> Result<(), &'static str> { + fn v8() -> DispatchResult { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); @@ -442,7 +445,7 @@ mod post_checks { .storage_base_deposit .saturating_add(value.storage_byte_deposit) .saturating_add(value.storage_item_deposit); - ensure!(reserved >= stored, "Reserved balance out of sync."); + ensure!(reserved >= stored, DispatchError::Other("Reserved balance out of sync.")); let mut storage_bytes = 0u32; let mut storage_items = 0u32; @@ -455,17 +458,23 @@ mod post_checks { storage_bytes.saturating_accrue(len); storage_items.saturating_accrue(1); } - ensure!(storage_bytes == value.storage_bytes, "Storage bytes do not match.",); - ensure!(storage_items == value.storage_items, "Storage items do not match.",); + ensure!( + storage_bytes == value.storage_bytes, + DispatchError::Other("Storage bytes do not match.") + ); + ensure!( + storage_items == value.storage_items, + DispatchError::Other("Storage items do not match.") + ); } Ok(()) } - fn v9() -> Result<(), &'static str> { + fn v9() -> DispatchResult { for value in CodeStorage::::iter_values() { ensure!( value.determinism == Determinism::Enforced, - "All pre-existing codes need to be deterministic." + DispatchError::Other("All pre-existing codes need to be deterministic.") ); } Ok(()) diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index 9d0e83e6f613f..66cc0009b664c 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -62,11 +62,17 @@ pub mod v1 { impl> OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!(StorageVersion::get::>() == 0, DispatchError::Other("can only upgrade from version 0")); + ensure!( + StorageVersion::get::>() == 0, + DispatchError::Other("can only upgrade from version 0") + ); let props_count = v0::PublicProps::::get().len(); log::info!(target: TARGET, "{} public proposals will be migrated.", props_count,); - ensure!(props_count <= T::MaxProposals::get() as usize, DispatchError::Other("too many proposals")); + ensure!( + props_count <= T::MaxProposals::get() as usize, + DispatchError::Other("too many proposals") + ); let referenda_count = v0::ReferendumInfoOf::::iter().count(); log::info!(target: TARGET, "{} referenda will be migrated.", referenda_count); @@ -133,15 +139,23 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> DispatchResult { assert_eq!(StorageVersion::get::>(), 1, "must upgrade"); let (old_props_count, old_ref_count): (u32, u32) = Decode::decode(&mut &state[..]).expect("pre_upgrade provides a valid state; qed"); let new_props_count = crate::PublicProps::::get().len() as u32; - assert_eq!(new_props_count, old_props_count, "must migrate all public proposals"); + ensure!( + new_props_count, + old_props_count, + DispatchError::Other("must migrate all public proposals") + ); let new_ref_count = crate::ReferendumInfoOf::::iter().count() as u32; - assert_eq!(new_ref_count, old_ref_count, "must migrate all referenda"); + ensure!( + new_ref_count, + old_ref_count, + DispatchError::Other("must migrate all referenda") + ); log::info!( target: TARGET, diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index f5cf806a7c85f..89d0491e495a6 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -66,13 +66,21 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!(Pallet::::on_chain_storage_version() == 0, DispatchError::Other("The onchain storage version must be zero for the migration to execute.")); + ensure!( + Pallet::::on_chain_storage_version() == 0, + DispatchError::Other( + "The onchain storage version must be zero for the migration to execute." + ) + ); Ok(Default::default()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { - assert_eq!(Pallet::::on_chain_storage_version(), 1); + fn post_upgrade(_: Vec) -> DispatchResult { + ensure!( + Pallet::::on_chain_storage_version() == 1, + DispatchError::Other("The onchain version must be updated after the migration.") + ); Ok(()) } } diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index da25dae46b0e9..8a89c286e5f02 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -72,13 +72,15 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 2, "this migration needs to be removed"); - ensure!(onchain == 1, "this migration needs to be run"); + ensure!(onchain < 2, DispatchError::Other("this migration needs to be removed")); + ensure!(onchain == 1, DispatchError::Other("this migration needs to be run")); ensure!( Calls::::iter().count() == 0, - "there are some dangling calls that need to be destroyed and refunded" + DispatchError::Other( + "there are some dangling calls that need to be destroyed and refunded" + ) ); Ok(()) } diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index e0d035a6c7acf..4ae56b332c853 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -93,23 +93,31 @@ pub mod v1 { fn pre_upgrade() -> Result, DispatchError> { let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); - ensure!(onchain_version == 0 && current_version == 1, DispatchError::Other("migration from version 0 to 1.")); + ensure!( + onchain_version == 0 && current_version == 1, + DispatchError::Other("migration from version 0 to 1.") + ); let prev_count = Collection::::iter().count(); Ok((prev_count as u32).encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + fn post_upgrade(prev_count: Vec) -> DispatchResult { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); let post_count = Collection::::iter().count() as u32; - assert_eq!( - prev_count, post_count, - "the records count before and after the migration should be the same" + ensure!( + prev_count == post_count, + DispatchError::Other( + "the records count before and after the migration should be the same" + ) ); - ensure!(Pallet::::on_chain_storage_version() == 1, "wrong storage version"); + ensure!( + Pallet::::on_chain_storage_version() == 1, + DispatchError::Other("wrong storage version") + ); Ok(()) } diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 217f95e599a21..0824cd9a174eb 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -100,9 +100,12 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> DispatchResult { // new version must be set. - assert_eq!(Pallet::::on_chain_storage_version(), 1); + ensure!( + Pallet::::on_chain_storage_version() == 1, + DispatchError::Other("The onchain version must be updated after the migration.") + ); Pallet::::try_state(frame_system::Pallet::::block_number())?; Ok(()) } @@ -366,23 +369,33 @@ pub mod v2 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> DispatchResult { // new version must be set. - assert_eq!(Pallet::::on_chain_storage_version(), 2); + ensure!( + Pallet::::on_chain_storage_version() == 2, + DispatchError::Other("The onchain version must be updated after the migration.") + ); // no reward or bonded pool has been skipped. - assert_eq!(RewardPools::::iter().count() as u32, RewardPools::::count()); - assert_eq!(BondedPools::::iter().count() as u32, BondedPools::::count()); + ensure!( + RewardPools::::iter().count() as u32 == RewardPools::::count(), + DispatchError::Other( + "The count of reward pools must remain the same after the migration." + ) + ); + ensure!( + BondedPools::::iter().count() as u32 == BondedPools::::count(), + DispatchError::Other( + "The count of reward pools must remain the same after the migration." + ) + ); // all reward pools must have exactly ED in them. This means no reward can be claimed, // and that setting reward counters all over the board to zero will work henceforth. RewardPools::::iter().for_each(|(id, _)| { - assert_eq!( - RewardPool::::current_balance(id), - Zero::zero(), - "reward pool({}) balance is {:?}", - id, - RewardPool::::current_balance(id) + ensure!( + RewardPool::::current_balance(id) == Zero::zero(), + DispatchError::Other("Reward pool balance must be zero."), ); }); @@ -445,12 +458,15 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> DispatchResult { ensure!( Metadata::::iter_keys().all(|id| BondedPools::::contains_key(&id)), - "not all of the stale metadata has been removed" + DispatchError::Other("not all of the stale metadata has been removed") + ); + ensure!( + Pallet::::on_chain_storage_version() == 3, + DispatchError::Other("wrong storage version") ); - ensure!(Pallet::::on_chain_storage_version() == 3, "wrong storage version"); Ok(()) } } @@ -545,20 +561,23 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> DispatchResult { // ensure all BondedPools items now contain an `inner.commission: Commission` field. ensure!( BondedPools::::iter().all(|(_, inner)| inner.commission.current.is_none() && inner.commission.max.is_none() && inner.commission.change_rate.is_none() && inner.commission.throttle_from.is_none()), - "a commission value has been incorrectly set" + DispatchError::Other("a commission value has been incorrectly set") ); ensure!( GlobalMaxCommission::::get() == Some(U::get()), - "global maximum commission error" + DispatchError::Other("global maximum commission error") + ); + ensure!( + Pallet::::on_chain_storage_version() == 4, + DispatchError::Other("wrong storage version") ); - ensure!(Pallet::::on_chain_storage_version() == 4, "wrong storage version"); Ok(()) } } @@ -655,13 +674,13 @@ pub mod v5 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(data: Vec) -> Result<(), &'static str> { + fn post_upgrade(data: Vec) -> DispatchResult { let old_rpool_values: u64 = Decode::decode(&mut &data[..]).unwrap(); let rpool_keys = RewardPools::::iter_keys().count() as u64; let rpool_values = RewardPools::::iter_values().count() as u64; ensure!( rpool_keys == rpool_values, - "There are STILL undecodable RewardPools - migration failed" + DispatchError::Other("There are STILL undecodable RewardPools - migration failed") ); if old_rpool_values != rpool_values { @@ -680,27 +699,30 @@ pub mod v5 { .is_zero() && reward_pool .total_commission_claimed .is_zero()), - "a commission value has been incorrectly set" + DispatchError::Other("a commission value has been incorrectly set") + ); + ensure!( + Pallet::::on_chain_storage_version() == 5, + DispatchError::Other("wrong storage version") ); - ensure!(Pallet::::on_chain_storage_version() == 5, "wrong storage version"); // These should not have been touched - just in case. ensure!( PoolMembers::::iter_keys().count() == PoolMembers::::iter_values().count(), - "There are undecodable PoolMembers in storage." + DispatchError::Other("There are undecodable PoolMembers in storage.") ); ensure!( BondedPools::::iter_keys().count() == BondedPools::::iter_values().count(), - "There are undecodable BondedPools in storage." + DispatchError::Other("There are undecodable BondedPools in storage.") ); ensure!( SubPoolsStorage::::iter_keys().count() == SubPoolsStorage::::iter_values().count(), - "There are undecodable SubPools in storage." + DispatchError::Other("There are undecodable SubPools in storage.") ); ensure!( Metadata::::iter_keys().count() == Metadata::::iter_values().count(), - "There are undecodable Metadata in storage." + DispatchError::Other("There are undecodable Metadata in storage.") ); Ok(()) diff --git a/frame/preimage/src/migration.rs b/frame/preimage/src/migration.rs index d4e1f7f071cdc..385d22817ed20 100644 --- a/frame/preimage/src/migration.rs +++ b/frame/preimage/src/migration.rs @@ -79,7 +79,10 @@ pub mod v1 { impl OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!(StorageVersion::get::>() == 0, DispatchError::Other("can only upgrade from version 0")); + ensure!( + StorageVersion::get::>() == 0, + DispatchError::Other("can only upgrade from version 0") + ); let images = v0::image_count::().expect("v0 storage corrupted"); log::info!(target: TARGET, "Migrating {} images", &images); @@ -148,7 +151,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> DispatchResult { let old_images: u32 = Decode::decode(&mut &state[..]).expect("pre_upgrade provides a valid state; qed"); let new_images = image_count::().expect("V1 storage corrupted"); @@ -161,7 +164,7 @@ pub mod v1 { old_images ); } - assert_eq!(StorageVersion::get::>(), 1, "must upgrade"); + ensure!(StorageVersion::get::>(), 1, DispatchError::Other("must upgrade")); Ok(()) } } diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index ba947f56cec78..2f7a4ba6ea7ba 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -147,15 +147,18 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> DispatchResult { let onchain_version = Pallet::::on_chain_storage_version(); - assert_eq!(onchain_version, 1, "must upgrade from version 0 to 1."); + ensure!( + onchain_version == 1, + DispatchError::Other("must upgrade from version 0 to 1.") + ); let pre_referendum_count: u32 = Decode::decode(&mut &state[..]) .expect("failed to decode the state from pre-upgrade."); let post_referendum_count = ReferendumInfoFor::::iter().count() as u32; - assert_eq!( - post_referendum_count, pre_referendum_count, - "must migrate all referendums." + ensure!( + post_referendum_count == pre_referendum_count, + DispatchError::Other("must migrate all referendums.") ); log::info!(target: TARGET, "migrated all referendums."); Ok(()) diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 8bd6da6076fb4..da2a5441fc061 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -98,7 +98,10 @@ pub mod v3 { impl> OnRuntimeUpgrade for MigrateToV4 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!(StorageVersion::get::>() == 3, DispatchError::Other("Can only upgrade from version 3")); + ensure!( + StorageVersion::get::>() == 3, + DispatchError::Other("Can only upgrade from version 3") + ); let agendas = Agenda::::iter_keys().count() as u32; let decodable_agendas = Agenda::::iter_values().count() as u32; @@ -169,12 +172,15 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { - assert_eq!(StorageVersion::get::>(), 4, "Must upgrade"); + fn post_upgrade(state: Vec) -> DispatchResult { + ensure!(StorageVersion::get::>() == 4, DispatchError::Other("Must upgrade")); // Check that everything decoded fine. for k in crate::Agenda::::iter_keys() { - assert!(crate::Agenda::::try_get(k).is_ok(), "Cannot decode V4 Agenda"); + ensure!( + crate::Agenda::::try_get(k).is_ok(), + DispatchError::Other("Cannot decode V4 Agenda") + ); } let old_agendas: u32 = @@ -285,8 +291,11 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), &'static str> { - assert_eq!(StorageVersion::get::>(), 4, "Version must not change"); + fn post_upgrade(state: Vec) -> DispatchResult { + ensure!( + StorageVersion::get::>() == 4, + DispatchError::Other("Version must not change") + ); let (old_agendas, non_empty_agendas): (u32, u32) = Decode::decode(&mut state.as_ref()).expect("Must decode pre_upgrade state"); @@ -305,7 +314,10 @@ pub mod v4 { old_agendas, new_agendas ), } - assert_eq!(new_agendas, non_empty_agendas, "Expected to keep all non-empty agendas"); + ensure!( + new_agendas == non_empty_agendas, + DispatchError::Other("Expected to keep all non-empty agendas") + ); Ok(()) } diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 0b02957a16bb3..cd9a710523d7b 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -84,15 +84,15 @@ pub mod v13 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { frame_support::ensure!( Pallet::::on_chain_storage_version() == 13, - "v13 not applied" + DispatchError::Other("v13 not applied") ); frame_support::ensure!( !StorageVersion::::exists(), - "Storage version not migrated correctly" + DispatchError::Other("Storage version not migrated correctly") ); Ok(()) @@ -146,10 +146,10 @@ pub mod v12 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, - "v12 not applied" + DispatchError::Other("v12 not applied") ); Ok(()) } @@ -217,10 +217,10 @@ pub mod v11 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, - "wrong version after the upgrade" + DispatchError::Other("wrong version after the upgrade") ); let old_pallet_name = N::get(); @@ -234,14 +234,14 @@ pub mod v11 { let old_pallet_prefix = twox_128(N::get().as_bytes()); frame_support::ensure!( sp_io::storage::next_key(&old_pallet_prefix).is_none(), - "old pallet data hasn't been removed" + DispatchError::Other("old pallet data hasn't been removed") ); let new_pallet_name =

::name(); let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); frame_support::ensure!( sp_io::storage::next_key(&new_pallet_prefix).is_some(), - "new pallet data hasn't been created" + DispatchError::Other("new pallet data hasn't been created") ); Ok(()) @@ -343,17 +343,23 @@ pub mod v9 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + fn post_upgrade(prev_count: Vec) -> DispatchResult { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); let post_count = T::VoterList::count(); let validators = Validators::::count(); - assert!(post_count == prev_count + validators); + ensure!( + post_count == prev_count + validators, + DispatchError::Other( + "`VoterList` count after the migration must equal to the sum of \ + previous count and the current number of validators" + ) + ); frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V9_0_0, - "must upgrade " + DispatchError::Other("must upgrade ") ); Ok(()) } diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 0ec8dffc566b2..40dd34918e892 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -169,7 +169,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> Result<(), &'static str> { + fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> DispatchResult { < Self as diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index e8ddfd12d4a4f..24c3416447148 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2147,7 +2147,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), &'static str> { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> DispatchResult { Ok(()) } } @@ -2185,7 +2185,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), &'static str> { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> DispatchResult { Ok(()) } } diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index ce28292d05e82..16575adbb585a 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -171,7 +171,7 @@ impl, DbWeight: Get> frame_support::traits } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { let hashed_prefix = twox_128(P::get().as_bytes()); match contains_prefixed_key(&hashed_prefix) { true => { diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index e2abf0bb12dc3..b06de33573522 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -182,7 +182,7 @@ pub trait OnRuntimeUpgrade { /// This hook must not write to any state, as it would make the main `on_runtime_upgrade` path /// inaccurate. #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { Ok(()) } } @@ -301,7 +301,7 @@ pub trait Hooks { /// /// 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(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { Ok(()) } @@ -389,7 +389,7 @@ mod tests { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> Result<(), &'static str> { + fn post_upgrade(_: Vec) -> DispatchResult { Post::mutate(|s| s.push(stringify!($name))); Ok(()) } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 4042c1904ec9d..ff5426e3a9bc4 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -125,7 +125,7 @@ //! fn pre_upgrade() -> Result, DispatchError> {} //! //! #[cfg(feature = "try-runtime")] -//! fn post_upgrade(state: Vec) -> Result<(), &'static str> {} +//! fn post_upgrade(state: Vec) -> DispatchResult {} //! ``` //! //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). From 388bf1274cc65bfd303ab723dd4daf0d5a6e2aa8 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Wed, 26 Apr 2023 19:31:45 +0200 Subject: [PATCH 05/31] try_runtime_upgrade --- frame/executive/src/lib.rs | 2 +- frame/support/src/traits/hooks.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index aad1de11d2c7b..f21997d7ba637 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -338,7 +338,7 @@ where /// `true`. Also, if set to `true`, it runs the `pre_upgrade` and `post_upgrade` hooks. pub fn try_runtime_upgrade( checks: frame_try_runtime::UpgradeCheckSelect, - ) -> Result { + ) -> Result { if checks.try_state() { let _guard = frame_support::StorageNoopGuard::default(); >::try_state( diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index b06de33573522..8918ae77c4dd3 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -136,7 +136,7 @@ pub trait OnRuntimeUpgrade { /// Same as `on_runtime_upgrade`, but perform the optional `pre_upgrade` and `post_upgrade` as /// well. #[cfg(feature = "try-runtime")] - fn try_on_runtime_upgrade(checks: bool) -> Result { + fn try_on_runtime_upgrade(checks: bool) -> Result { let maybe_state = if checks { let _guard = frame_support::StorageNoopGuard::default(); let state = Self::pre_upgrade()?; @@ -201,7 +201,7 @@ impl OnRuntimeUpgrade for Tuple { /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade /// hooks for tuples are a noop. #[cfg(feature = "try-runtime")] - fn try_on_runtime_upgrade(checks: bool) -> Result { + fn try_on_runtime_upgrade(checks: bool) -> Result { let mut weight = Weight::zero(); for_tuples!( #( weight = weight.saturating_add(Tuple::try_on_runtime_upgrade(checks)?); )* ); Ok(weight) From 3cd001208c0f39cba9216a6089b4a4a1b9be7ed2 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 15:41:20 +0200 Subject: [PATCH 06/31] fixes --- frame/bags-list/src/lib.rs | 2 +- frame/bags-list/src/list/mod.rs | 28 ++++++++++++++-------------- frame/democracy/src/migrations.rs | 5 +---- frame/nomination-pools/src/lib.rs | 6 +++--- frame/staking/src/pallet/impls.rs | 2 +- 5 files changed, 20 insertions(+), 23 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 87eb2d1b341aa..67a71d4272401 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -355,7 +355,7 @@ impl, I: 'static> SortedListProvider for Pallet } #[cfg(feature = "try-runtime")] - fn try_state() -> Result<(), &'static str> { + fn try_state() -> DispatchResult { Self::do_try_state() } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index f667f4c101ef8..552186fababb6 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -512,18 +512,18 @@ impl, I: 'static> List { /// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure /// all bags and nodes are checked per *any* update to `List`. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - pub(crate) fn do_try_state() -> Result<(), &'static str> { + pub(crate) fn do_try_state() -> DispatchResult { let mut seen_in_list = BTreeSet::new(); ensure!( Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)), - "duplicate identified", + ListError::Duplicate, ); let iter_count = Self::iter().count() as u32; let stored_count = crate::ListNodes::::count(); let nodes_count = crate::ListNodes::::iter().count() as u32; - ensure!(iter_count == stored_count, "iter_count != stored_count"); - ensure!(stored_count == nodes_count, "stored_count != nodes_count"); + ensure!(iter_count == stored_count, DispatchError::Other("iter_count != stored_count")); + ensure!(stored_count == nodes_count, DispatchError::Other("stored_count != nodes_count")); crate::log!(trace, "count of nodes: {}", stored_count); @@ -544,7 +544,10 @@ impl, I: 'static> List { let nodes_in_bags_count = active_bags.clone().fold(0u32, |acc, cur| acc + cur.iter().count() as u32); - ensure!(nodes_count == nodes_in_bags_count, "stored_count != nodes_in_bags_count"); + ensure!( + nodes_count == nodes_in_bags_count, + DispatchError::Other("stored_count != nodes_in_bags_count") + ); crate::log!(trace, "count of active bags {}", active_bags.count()); @@ -757,7 +760,7 @@ impl, I: 'static> Bag { // if there is no head, then there must not be a tail, meaning that the bag is // empty. .unwrap_or_else(|| self.tail.is_none()), - "head has a prev" + DispatchError::Other("head has a prev") ); frame_support::ensure!( @@ -766,7 +769,7 @@ impl, I: 'static> Bag { // if there is no tail, then there must not be a head, meaning that the bag is // empty. .unwrap_or_else(|| self.head.is_none()), - "tail has a next" + DispatchError::Other("tail has a next") ); let mut seen_in_bag = BTreeSet::new(); @@ -775,7 +778,7 @@ impl, I: 'static> Bag { .map(|node| node.id) // each voter is only seen once, thus there is no cycle within a bag .all(|voter| seen_in_bag.insert(voter)), - "duplicate found in bag" + ListError::Duplicate ); Ok(()) @@ -895,15 +898,12 @@ impl, I: 'static> Node { } #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - fn do_try_state(&self) -> Result<(), &'static str> { + fn do_try_state(&self) -> DispatchResult { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; let id = self.id(); - frame_support::ensure!( - expected_bag.contains(id), - "node does not exist in the expected bag" - ); + frame_support::ensure!(expected_bag.contains(id), ListError::NodeNotFound); let non_terminal_check = !self.is_terminal() && expected_bag.head.as_ref() != Some(id) && @@ -912,7 +912,7 @@ impl, I: 'static> Node { expected_bag.head.as_ref() == Some(id) || expected_bag.tail.as_ref() == Some(id); frame_support::ensure!( non_terminal_check || terminal_check, - "a terminal node is neither its bag head or tail" + DispatchError::Other("a terminal node is neither its bag head or tail") ); Ok(()) diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index 66cc0009b664c..6c8c2133210b6 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -69,10 +69,7 @@ pub mod v1 { let props_count = v0::PublicProps::::get().len(); log::info!(target: TARGET, "{} public proposals will be migrated.", props_count,); - ensure!( - props_count <= T::MaxProposals::get() as usize, - DispatchError::Other("too many proposals") - ); + ensure!(props_count <= T::MaxProposals::get() as usize, Error::::TooMany); let referenda_count = v0::ReferendumInfoOf::::iter().count(); log::info!(target: TARGET, "{} referenda will be migrated.", referenda_count); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 637629ba74cf3..789ee7e0db302 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -3082,7 +3082,7 @@ impl Pallet { ensure!( MaxPools::::get().map_or(true, |max| bonded_pools.len() <= (max as usize)), - DispatchError::Other("the count of the above set must be less than `MaxPools`") + Error::::MaxPools ); for id in reward_pools { @@ -3154,7 +3154,7 @@ impl Pallet { ); ensure!(MaxPoolMembersPerPool::::get() .map_or(true, |max| bonded_pool.member_counter <= max), - DispatchError::Other("Each `BondedPool.member_counter` must be less than `MaxPoolMembersPerPool`") + Error::::MaxPoolMembers ); let depositor = PoolMembers::::get(&bonded_pool.roles.depositor).unwrap(); @@ -3168,7 +3168,7 @@ impl Pallet { })?; ensure!( MaxPoolMembers::::get().map_or(true, |max| all_members <= max), - DispatchError::Other("There can't be more than `MaxPoolMembers` in a pool.") + Error::::MaxPoolMembers ); if level <= 1 { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index a45e3da3507c5..f273add9335f8 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1714,7 +1714,7 @@ impl Pallet { ensure!( ValidatorCount::::get() <= ::MaxWinners::get(), - DispatchError::Other("validator count exceeded election max winners") + Error::::TooManyValidators ); Ok(()) } From 87766282579199b0586f547eaf05cdaa2c953f38 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 15:46:28 +0200 Subject: [PATCH 07/31] bags-list fix --- frame/bags-list/src/lib.rs | 2 +- frame/bags-list/src/list/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 67a71d4272401..a8f3864bc0d2f 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -275,7 +275,7 @@ pub mod pallet { #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] impl, I: 'static> Pallet { - pub fn do_try_state() -> Result<(), &'static str> { + pub fn do_try_state() -> DispatchResult { List::::do_try_state() } } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 552186fababb6..635b544e7597f 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -753,7 +753,7 @@ impl, I: 'static> Bag { /// * Ensures tail has no next. /// * Ensures there are no loops, traversal from head to tail is correct. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - fn do_try_state(&self) -> Result<(), &'static str> { + fn do_try_state(&self) -> DispatchResult { frame_support::ensure!( self.head() .map(|head| head.prev().is_none()) From f7aa4d609fe1dabc4e4566bc8a64861cecfef3bb Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 15:52:07 +0200 Subject: [PATCH 08/31] fix --- frame/bags-list/src/lib.rs | 1 + frame/bags-list/src/list/mod.rs | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index a8f3864bc0d2f..97d42eee17d45 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -55,6 +55,7 @@ use codec::FullCodec; use frame_election_provider_support::{ScoreProvider, SortedListProvider}; +use frame_support::dispatch::{DispatchError, DispatchResult}; use frame_system::ensure_signed; use sp_runtime::traits::{AtLeast32BitUnsigned, Bounded, StaticLookup}; use sp_std::prelude::*; diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 635b544e7597f..d1d6221fbd72f 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -28,7 +28,9 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ - defensive, ensure, + defensive, + dispatch::{DispatchError, DispatchResult}, + ensure, traits::{Defensive, DefensiveOption, Get}, DefaultNoBound, PalletError, }; From fd4401373ed33d707086b9c4260ada606165e8c4 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 15:57:31 +0200 Subject: [PATCH 09/31] update test --- frame/bags-list/src/list/tests.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index f5afdc24f2608..01fee5ce3852f 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -359,7 +359,7 @@ mod list { // make sure there are no duplicates. ExtBuilder::default().build_and_execute_no_post_check(|| { Bag::::get(10).unwrap().insert_unchecked(2, 10); - assert_eq!(List::::do_try_state(), Err("duplicate identified")); + assert_eq!(List::::do_try_state(), ListError::Duplicate); }); // ensure count is in sync with `ListNodes::count()`. @@ -373,7 +373,10 @@ mod list { CounterForListNodes::::mutate(|counter| *counter += 1); assert_eq!(crate::ListNodes::::count(), 5); - assert_eq!(List::::do_try_state(), Err("iter_count != stored_count")); + assert_eq!( + List::::do_try_state(), + DispatchError::Other("iter_count != stored_count") + ); }); } From a5693f7fbf41ae69cb38405ab6c74b756ce8c853 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 16:02:07 +0200 Subject: [PATCH 10/31] warning fix --- frame/bags-list/src/list/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index d1d6221fbd72f..635b544e7597f 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -28,9 +28,7 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ - defensive, - dispatch::{DispatchError, DispatchResult}, - ensure, + defensive, ensure, traits::{Defensive, DefensiveOption, Get}, DefaultNoBound, PalletError, }; From f38a40e9874090283b5cc80a1d0a8e72967216d2 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 16:09:19 +0200 Subject: [PATCH 11/31] ... --- frame/bags-list/src/list/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index 01fee5ce3852f..219602e1121b6 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -21,7 +21,10 @@ use crate::{ ListBags, ListNodes, }; use frame_election_provider_support::{SortedListProvider, VoteWeight}; -use frame_support::{assert_ok, assert_storage_noop}; +use frame_support::{ + assert_ok, assert_storage_noop, + dispatch::{DispatchError, DispatchResult}, +}; fn node( id: AccountId, From 113aeac26c0923619f100e81240c30c02b488812 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 16:27:48 +0200 Subject: [PATCH 12/31] =?UTF-8?q?final=20fixes=20=F0=9F=A4=9E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- frame/bags-list/src/lib.rs | 2 +- frame/bags-list/src/list/mod.rs | 13 +++++++++---- frame/bags-list/src/list/tests.rs | 12 ++++++------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 97d42eee17d45..703acb9ffc7e9 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -268,7 +268,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_: BlockNumberFor) -> Result<(), &'static str> { + fn try_state(_: BlockNumberFor) -> DispatchResult { >::try_state() } } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 635b544e7597f..6aeabf14b15f6 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -28,7 +28,9 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ - defensive, ensure, + defensive, + dispatch::{DispatchError, DispatchResult}, + ensure, traits::{Defensive, DefensiveOption, Get}, DefaultNoBound, PalletError, }; @@ -516,7 +518,7 @@ impl, I: 'static> List { let mut seen_in_list = BTreeSet::new(); ensure!( Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)), - ListError::Duplicate, + DispatchError::Other("duplicate identified") ); let iter_count = Self::iter().count() as u32; @@ -778,7 +780,7 @@ impl, I: 'static> Bag { .map(|node| node.id) // each voter is only seen once, thus there is no cycle within a bag .all(|voter| seen_in_bag.insert(voter)), - ListError::Duplicate + DispatchError::Other("duplicate found in bag") ); Ok(()) @@ -903,7 +905,10 @@ impl, I: 'static> Node { let id = self.id(); - frame_support::ensure!(expected_bag.contains(id), ListError::NodeNotFound); + frame_support::ensure!( + expected_bag.contains(id), + DispatchError::Other("node does not exist in the bag") + ); let non_terminal_check = !self.is_terminal() && expected_bag.head.as_ref() != Some(id) && diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index 219602e1121b6..0641012dc6a6e 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -21,10 +21,7 @@ use crate::{ ListBags, ListNodes, }; use frame_election_provider_support::{SortedListProvider, VoteWeight}; -use frame_support::{ - assert_ok, assert_storage_noop, - dispatch::{DispatchError, DispatchResult}, -}; +use frame_support::{assert_ok, assert_storage_noop, dispatch::DispatchError}; fn node( id: AccountId, @@ -362,7 +359,10 @@ mod list { // make sure there are no duplicates. ExtBuilder::default().build_and_execute_no_post_check(|| { Bag::::get(10).unwrap().insert_unchecked(2, 10); - assert_eq!(List::::do_try_state(), ListError::Duplicate); + assert_eq!( + List::::do_try_state(), + DispatchError::Other("duplicate identified").into() + ); }); // ensure count is in sync with `ListNodes::count()`. @@ -378,7 +378,7 @@ mod list { assert_eq!( List::::do_try_state(), - DispatchError::Other("iter_count != stored_count") + DispatchError::Other("iter_count != stored_count").into() ); }); } From 65ffe158d16422978fb64776fcac506237f473bf Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 16:32:51 +0200 Subject: [PATCH 13/31] warning.. --- frame/bags-list/src/list/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 6aeabf14b15f6..4cdbc357170e0 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -28,9 +28,7 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ - defensive, - dispatch::{DispatchError, DispatchResult}, - ensure, + defensive, ensure, traits::{Defensive, DefensiveOption, Get}, DefaultNoBound, PalletError, }; @@ -44,6 +42,9 @@ use sp_std::{ prelude::*, }; +#[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] +use frame_support::dispatch::{DispatchError, DispatchResult}; + #[derive(Debug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, PalletError)] pub enum ListError { /// A duplicate id has been detected. From d0ea89f1fe9ae5c9836eba8e0caf690d433a30b7 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 16:51:40 +0200 Subject: [PATCH 14/31] frame-support --- frame/support/src/dispatch.rs | 8 ++++---- frame/support/src/migrations.rs | 5 ++++- frame/support/src/traits/hooks.rs | 5 ++++- frame/support/src/traits/try_runtime.rs | 13 ++++++------- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 24c3416447148..e028d9378ebfb 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2095,7 +2095,7 @@ macro_rules! decl_module { fn try_state( _: <$trait_instance as $system::Config>::BlockNumber, _: $crate::traits::TryStateSelect, - ) -> DispatchResult { + ) -> crate::dispatch::DispatchResult { let pallet_name = << $trait_instance as @@ -2142,7 +2142,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, &'static str> { + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, DispatchError> { Ok($crate::sp_std::vec::Vec::new()) } @@ -2180,12 +2180,12 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, &'static str> { + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, crate::dispatch::DispatchError> { Ok($crate::sp_std::vec::Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> DispatchResult { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> crate::dispatch::DispatchResult { Ok(()) } } diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index 16575adbb585a..869d375140833 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -18,6 +18,7 @@ #[cfg(feature = "try-runtime")] use crate::storage::unhashed::contains_prefixed_key; use crate::{ + dispatch::{DispatchError, DispatchResult}, traits::{GetStorageVersion, PalletInfoAccess}, weights::{RuntimeDbWeight, Weight}, }; @@ -176,7 +177,9 @@ impl, DbWeight: Get> frame_support::traits match contains_prefixed_key(&hashed_prefix) { true => { log::error!("{} has keys remaining post-removal ❗", P::get()); - return Err("Keys remaining post-removal, this should never happen 🚨") + return Err(DispatchError::Other( + "Keys remaining post-removal, this should never happen 🚨", + )) }, false => log::info!("No {} keys found post-removal 🎉", P::get()), }; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 8918ae77c4dd3..2177d9c41e0ef 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -17,7 +17,10 @@ //! Traits for hooking tasks to events in a blockchain's lifecycle. -use crate::weights::Weight; +use crate::{ + dispatch::{DispatchError, DispatchResult}, + weights::Weight, +}; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index 58e643333be7e..32d5b5b619b72 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -17,6 +17,7 @@ //! Try-runtime specific traits and types. +use crate::dispatch::DispatchResult; use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; @@ -148,7 +149,7 @@ impl TryState { - let functions: &[fn(BlockNumber, Select) -> Result<(), &'static str>] = + let functions: &[fn(BlockNumber, Select) -> DispatchResult] = &[for_tuples!(#( Tuple::try_state ),*)]; let skip = n.clone() % (functions.len() as u32).into(); let skip: u32 = @@ -161,12 +162,10 @@ impl TryState { - let try_state_fns: &[( - &'static str, - fn(BlockNumber, Select) -> Result<(), &'static str>, - )] = &[for_tuples!( - #( (::name(), Tuple::try_state) ),* - )]; + let try_state_fns: &[(&'static str, fn(BlockNumber, Select) -> DispatchResult)] = + &[for_tuples!( + #( (::name(), Tuple::try_state) ),* + )]; let mut result = Ok(()); pallet_names.iter().for_each(|pallet_name| { if let Some((name, try_state_fn)) = From e6a37c70f67d3f4087c8416b59f02af9f9c0ebf3 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Thu, 27 Apr 2023 17:02:44 +0200 Subject: [PATCH 15/31] warnings --- frame/support/src/migrations.rs | 3 ++- frame/support/src/traits/hooks.rs | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index 869d375140833..87a180f3d6c6e 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -15,10 +15,11 @@ // See the License for the specific language governing permissions and // limitations under the License. +#[cfg(feature = "try-runtime")] +use crate::dispatch::{DispatchError, DispatchResult}; #[cfg(feature = "try-runtime")] use crate::storage::unhashed::contains_prefixed_key; use crate::{ - dispatch::{DispatchError, DispatchResult}, traits::{GetStorageVersion, PalletInfoAccess}, weights::{RuntimeDbWeight, Weight}, }; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 2177d9c41e0ef..6000f451a2ff1 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -17,14 +17,14 @@ //! Traits for hooking tasks to events in a blockchain's lifecycle. -use crate::{ - dispatch::{DispatchError, DispatchResult}, - weights::Weight, -}; +use crate::weights::Weight; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; +#[cfg(feature = "try-runtime")] +use crate::dispatch::{DispatchError, DispatchResult}; + /// The block initialization trait. /// /// Implementing this lets you express what should happen for your pallet when the block is From 49ac4db57ced5ae897df0838b18a1de75ca5f482 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Fri, 28 Apr 2023 11:06:35 +0200 Subject: [PATCH 16/31] Update frame/staking/src/migrations.rs Co-authored-by: Liam Aharon --- frame/staking/src/migrations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index cd9a710523d7b..69ee7d7000eae 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -359,7 +359,7 @@ pub mod v9 { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V9_0_0, - DispatchError::Other("must upgrade ") + DispatchError::Other("must upgrade") ); Ok(()) } From 83b5cdf6acb36cbae03bf3e4f2c976b4850ae6bb Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 28 Apr 2023 13:25:56 +0200 Subject: [PATCH 17/31] fix --- frame/assets/src/migration.rs | 7 ++++--- frame/bags-list/src/migrations.rs | 5 ++++- frame/democracy/src/migrations.rs | 10 ++++------ frame/election-provider-support/src/lib.rs | 3 +++ frame/executive/src/lib.rs | 3 +++ frame/fast-unstake/src/migrations.rs | 2 ++ frame/nomination-pools/src/migration.rs | 12 +++++++----- frame/preimage/src/migration.rs | 8 +++++++- frame/referenda/src/migration.rs | 2 +- frame/scheduler/src/migration.rs | 6 ++++-- frame/staking/src/migrations.rs | 6 ++++++ frame/staking/src/pallet/impls.rs | 7 +++++-- frame/staking/src/pallet/mod.rs | 2 +- frame/support/procedural/src/lib.rs | 3 +++ frame/support/procedural/src/pallet/expand/hooks.rs | 5 ++++- frame/support/src/dispatch.rs | 6 +++--- .../transaction-payment/asset-tx-payment/src/lib.rs | 3 +++ frame/transaction-payment/src/lib.rs | 3 +++ 18 files changed, 67 insertions(+), 26 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index c7be9c0d98da3..885dcad633419 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -125,9 +125,10 @@ pub mod v1 { ) ); - Asset::::iter().for_each(|(_id, asset)| { - ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, DispatchError::Other("assets should only be live or frozen. None should be in destroying status, or undefined state")) - }); + Asset::::iter().try_for_each(|(_id, asset)| -> DispatchResult { + ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, DispatchError::Other("assets should only be live or frozen. None should be in destroying status, or undefined state")); + Ok(()) + })?; Ok(()) } } diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index f5f9402117e4a..3dfb277614279 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -23,7 +23,10 @@ use frame_election_provider_support::ScoreProvider; use frame_support::traits::OnRuntimeUpgrade; #[cfg(feature = "try-runtime")] -use frame_support::ensure; +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, +}; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index 6c8c2133210b6..83f2c95e61303 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -136,21 +136,19 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> DispatchResult { - assert_eq!(StorageVersion::get::>(), 1, "must upgrade"); + fn post_upgrade(state: Vec) -> frame_support::dispatch::DispatchResult { + ensure!(StorageVersion::get::>() == 1, DispatchError::Other("must upgrade")); let (old_props_count, old_ref_count): (u32, u32) = Decode::decode(&mut &state[..]).expect("pre_upgrade provides a valid state; qed"); let new_props_count = crate::PublicProps::::get().len() as u32; ensure!( - new_props_count, - old_props_count, + new_props_count == old_props_count, DispatchError::Other("must migrate all public proposals") ); let new_ref_count = crate::ReferendumInfoOf::::iter().count() as u32; ensure!( - new_ref_count, - old_ref_count, + new_ref_count == old_ref_count, DispatchError::Other("must migrate all referenda") ); diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index f3cd918fc66ec..fae945fea8a74 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -189,6 +189,9 @@ pub use sp_npos_elections::{ }; pub use traits::NposSolution; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchResult; + // re-export for the solution macro, with the dependencies of the macro. #[doc(hidden)] pub use codec; diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index f21997d7ba637..30c1f44a5219f 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -137,6 +137,9 @@ use sp_runtime::{ }; use sp_std::{marker::PhantomData, prelude::*}; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchError; + #[allow(dead_code)] const LOG_TARGET: &str = "runtime::executive"; diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index 89d0491e495a6..2225fc53923f0 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -18,6 +18,8 @@ pub mod v1 { use crate::{types::BalanceOf, *}; use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, storage::unhashed, traits::{Defensive, Get, GetStorageVersion, OnRuntimeUpgrade}, weights::Weight, diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 0824cd9a174eb..4a341e3d98e37 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -357,13 +357,14 @@ pub mod v2 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { // all reward accounts must have more than ED. - RewardPools::::iter().for_each(|(id, _)| { + RewardPools::::iter().try_for_each(|(id, _)| -> DispatchResult { ensure!( T::Currency::free_balance(&Pallet::::create_reward_account(id)) >= T::Currency::minimum_balance(), DispatchError::Other("Reward accounts must have greater balance than ED.") - ) - }); + ); + Ok(()) + })?; Ok(Vec::new()) } @@ -392,12 +393,13 @@ pub mod v2 { // all reward pools must have exactly ED in them. This means no reward can be claimed, // and that setting reward counters all over the board to zero will work henceforth. - RewardPools::::iter().for_each(|(id, _)| { + RewardPools::::iter().try_for_each(|(id, _)| -> DispatchResult { ensure!( RewardPool::::current_balance(id) == Zero::zero(), DispatchError::Other("Reward pool balance must be zero."), ); - }); + Ok(()) + })?; log!(info, "post upgrade hook for MigrateToV2 executed."); Ok(()) diff --git a/frame/preimage/src/migration.rs b/frame/preimage/src/migration.rs index 385d22817ed20..b545098a69114 100644 --- a/frame/preimage/src/migration.rs +++ b/frame/preimage/src/migration.rs @@ -24,6 +24,12 @@ use frame_support::{ }; use sp_std::collections::btree_map::BTreeMap; +#[cfg(feature = "try-runtime")] +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, +}; + /// The log target. const TARGET: &'static str = "runtime::preimage::migration::v1"; @@ -164,7 +170,7 @@ pub mod v1 { old_images ); } - ensure!(StorageVersion::get::>(), 1, DispatchError::Other("must upgrade")); + ensure!(StorageVersion::get::>() == 1, DispatchError::Other("must upgrade")); Ok(()) } } diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index 2f7a4ba6ea7ba..f6ca50fdd3c89 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -97,7 +97,7 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { let onchain_version = Pallet::::on_chain_storage_version(); - ensure!(onchain_version, 0, DispatchError::Other("migration from version 0 to 1.")); + ensure!(onchain_version == 0, DispatchError::Other("migration from version 0 to 1.")); let referendum_count = v0::ReferendumInfoFor::::iter().count(); log::info!( target: TARGET, diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index da2a5441fc061..788306080dc4c 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -128,7 +128,9 @@ pub mod v3 { agenda.len(), max_scheduled_per_block, ); - return Err("Agenda would overflow `MaxScheduledPerBlock`.") + return Err(DispatchError::Other( + "Agenda would overflow `MaxScheduledPerBlock`.", + )) } } // Check that bounding the calls will not overflow `MAX_LENGTH`. @@ -145,7 +147,7 @@ pub mod v3 { block_number, l, ); - return Err("Call is too large.") + return Err(DispatchError::Other("Call is too large.")) } }, _ => (), diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 69ee7d7000eae..87d1a2669d977 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -23,6 +23,12 @@ use frame_support::{ traits::OnRuntimeUpgrade, }; +#[cfg(feature = "try-runtime")] +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, +}; + /// Used for release versioning upto v12. /// /// Obsolete from v13. Keeping around to make encoding/decoding of old migration code easier. diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index f273add9335f8..2d302717c42af 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -50,6 +50,9 @@ use crate::{ use super::{pallet::*, STAKING_ID}; +#[cfg(feature = "try-runtime")] +use frame_support::{dispatch::DispatchResult, ensure}; + /// The maximum number of iterations that we do whilst iterating over `T::VoterList` in /// `get_npos_voters`. /// @@ -1466,7 +1469,7 @@ impl SortedListProvider for UseValidatorsMap { 0 } #[cfg(feature = "try-runtime")] - fn try_state() -> Result<(), &'static str> { + fn try_state() -> DispatchResult { Ok(()) } @@ -1543,7 +1546,7 @@ impl SortedListProvider for UseNominatorsAndValidatorsM } #[cfg(feature = "try-runtime")] - fn try_state() -> Result<(), &'static str> { + fn try_state() -> DispatchResult { Ok(()) } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index d8f1855da4bc0..86afd10b5d4d0 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -824,7 +824,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(n: BlockNumberFor) -> Result<(), &'static str> { + fn try_state(n: BlockNumberFor) -> DispatchResult { Self::do_try_state(n) } } diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 33d9ce9473e59..51cf7bebb9f7c 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -41,6 +41,9 @@ use quote::quote; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; +#[cfg(feature = "try-runtime")] +use crate::dispatch::{DispatchError, DispatchResult}; + thread_local! { /// A global counter, can be used to generate a relatively unique identifier. static COUNTER: RefCell = RefCell::new(Counter(0)); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 40dd34918e892..e92f68537deb8 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -17,6 +17,9 @@ use crate::pallet::Def; +#[cfg(feature = "try-runtime")] +use crate::dispatch::{DispatchError, DispatchResult}; + /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let (where_clause, span, has_runtime_upgrade) = match def.hooks.as_ref() { @@ -160,7 +163,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<#frame_support::sp_std::vec::Vec, &'static str> { + fn pre_upgrade() -> Result<#frame_support::sp_std::vec::Vec, DispatchError> { < Self as diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index e028d9378ebfb..3331fff1635b4 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2095,7 +2095,7 @@ macro_rules! decl_module { fn try_state( _: <$trait_instance as $system::Config>::BlockNumber, _: $crate::traits::TryStateSelect, - ) -> crate::dispatch::DispatchResult { + ) -> $crate::dispatch::DispatchResult { let pallet_name = << $trait_instance as @@ -2180,12 +2180,12 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, crate::dispatch::DispatchError> { + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, $crate::dispatch::DispatchError> { Ok($crate::sp_std::vec::Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> crate::dispatch::DispatchResult { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> $crate::dispatch::DispatchResult { Ok(()) } } diff --git a/frame/transaction-payment/asset-tx-payment/src/lib.rs b/frame/transaction-payment/asset-tx-payment/src/lib.rs index 4e83d8b489b70..50e01cd9b9d03 100644 --- a/frame/transaction-payment/asset-tx-payment/src/lib.rs +++ b/frame/transaction-payment/asset-tx-payment/src/lib.rs @@ -59,6 +59,9 @@ use sp_runtime::{ FixedPointOperand, }; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchError; + #[cfg(test)] mod mock; #[cfg(test)] diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 83ff69428d144..b94a187fe112d 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -70,6 +70,9 @@ use frame_support::{ weights::{Weight, WeightToFee}, }; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchError; + #[cfg(test)] mod mock; #[cfg(test)] From 773f4ad93dfc70db80ffaaf95f059b9cc86ee9ac Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 28 Apr 2023 13:34:18 +0200 Subject: [PATCH 18/31] fix warning --- frame/fast-unstake/src/migrations.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index 2225fc53923f0..ba8936bad8c75 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -18,8 +18,6 @@ pub mod v1 { use crate::{types::BalanceOf, *}; use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, storage::unhashed, traits::{Defensive, Get, GetStorageVersion, OnRuntimeUpgrade}, weights::Weight, @@ -27,6 +25,12 @@ pub mod v1 { use sp_staking::EraIndex; use sp_std::prelude::*; + #[cfg(feature = "try-runtime")] + use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, + }; + pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { fn on_runtime_upgrade() -> Weight { From 962ad7f6d2848b7a8326402734d53b0f6a51878f Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 28 Apr 2023 15:51:09 +0200 Subject: [PATCH 19/31] nit fix --- frame/assets/src/migration.rs | 4 +-- frame/bags-list/src/list/mod.rs | 26 ++++++---------- frame/collective/src/lib.rs | 17 +++++----- frame/contracts/src/migration.rs | 19 +++--------- frame/democracy/src/migrations.rs | 17 +++------- frame/fast-unstake/src/lib.rs | 2 +- frame/fast-unstake/src/migrations.rs | 2 +- frame/multisig/src/migrations.rs | 6 ++-- frame/nfts/src/migration.rs | 10 ++---- frame/nomination-pools/src/lib.rs | 11 +++---- frame/nomination-pools/src/migration.rs | 41 ++++++++++--------------- frame/nomination-pools/src/tests.rs | 5 +-- frame/preimage/src/migration.rs | 7 ++--- frame/referenda/src/migration.rs | 12 ++------ frame/scheduler/src/migration.rs | 22 +++---------- frame/staking/src/migrations.rs | 26 ++++++++-------- frame/staking/src/pallet/impls.rs | 8 ++--- 17 files changed, 83 insertions(+), 152 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 885dcad633419..57c42779564cc 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -95,7 +95,7 @@ pub mod v1 { fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( Pallet::::on_chain_storage_version() == 0, - DispatchError::Other("must upgrade linearly") + "must upgrade linearly" ); let prev_count = Asset::::iter().count(); Ok((prev_count as u32).encode()) @@ -126,7 +126,7 @@ pub mod v1 { ); Asset::::iter().try_for_each(|(_id, asset)| -> DispatchResult { - ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, DispatchError::Other("assets should only be live or frozen. None should be in destroying status, or undefined state")); + ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state"); Ok(()) })?; Ok(()) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 4cdbc357170e0..1836786786baa 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -43,7 +43,7 @@ use sp_std::{ }; #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] -use frame_support::dispatch::{DispatchError, DispatchResult}; +use frame_support::dispatch::DispatchResult; #[derive(Debug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, PalletError)] pub enum ListError { @@ -519,14 +519,14 @@ impl, I: 'static> List { let mut seen_in_list = BTreeSet::new(); ensure!( Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)), - DispatchError::Other("duplicate identified") + "duplicate identified" ); let iter_count = Self::iter().count() as u32; let stored_count = crate::ListNodes::::count(); let nodes_count = crate::ListNodes::::iter().count() as u32; - ensure!(iter_count == stored_count, DispatchError::Other("iter_count != stored_count")); - ensure!(stored_count == nodes_count, DispatchError::Other("stored_count != nodes_count")); + ensure!(iter_count == stored_count, "iter_count != stored_count"); + ensure!(stored_count == nodes_count, "stored_count != nodes_count"); crate::log!(trace, "count of nodes: {}", stored_count); @@ -547,10 +547,7 @@ impl, I: 'static> List { let nodes_in_bags_count = active_bags.clone().fold(0u32, |acc, cur| acc + cur.iter().count() as u32); - ensure!( - nodes_count == nodes_in_bags_count, - DispatchError::Other("stored_count != nodes_in_bags_count") - ); + ensure!(nodes_count == nodes_in_bags_count, "stored_count != nodes_in_bags_count"); crate::log!(trace, "count of active bags {}", active_bags.count()); @@ -763,7 +760,7 @@ impl, I: 'static> Bag { // if there is no head, then there must not be a tail, meaning that the bag is // empty. .unwrap_or_else(|| self.tail.is_none()), - DispatchError::Other("head has a prev") + "head has a prev" ); frame_support::ensure!( @@ -772,7 +769,7 @@ impl, I: 'static> Bag { // if there is no tail, then there must not be a head, meaning that the bag is // empty. .unwrap_or_else(|| self.head.is_none()), - DispatchError::Other("tail has a next") + "tail has a next" ); let mut seen_in_bag = BTreeSet::new(); @@ -781,7 +778,7 @@ impl, I: 'static> Bag { .map(|node| node.id) // each voter is only seen once, thus there is no cycle within a bag .all(|voter| seen_in_bag.insert(voter)), - DispatchError::Other("duplicate found in bag") + "duplicate found in bag" ); Ok(()) @@ -906,10 +903,7 @@ impl, I: 'static> Node { let id = self.id(); - frame_support::ensure!( - expected_bag.contains(id), - DispatchError::Other("node does not exist in the bag") - ); + frame_support::ensure!(expected_bag.contains(id), "node does not exist in the bag"); let non_terminal_check = !self.is_terminal() && expected_bag.head.as_ref() != Some(id) && @@ -918,7 +912,7 @@ impl, I: 'static> Node { expected_bag.head.as_ref() == Some(id) || expected_bag.tail.as_ref() == Some(id); frame_support::ensure!( non_terminal_check || terminal_check, - DispatchError::Other("a terminal node is neither its bag head or tail") + "a terminal node is neither its bag head or tail" ); Ok(()) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index f7be5b90351be..55b8c3c6ed39e 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -981,11 +981,11 @@ impl, I: 'static> Pallet { ensure!( Self::proposals().into_iter().count() <= Self::proposal_count() as usize, - DispatchError::Other("The actual number of proposals is greater than `ProposalCount`") + "The actual number of proposals is greater than `ProposalCount`" ); ensure!( Self::proposals().into_iter().count() == >::iter_keys().count(), - DispatchError::Other("Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`") + "Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`" ); Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult { @@ -995,7 +995,7 @@ impl, I: 'static> Pallet { ensure!( ayes.saturating_add(nays) <= T::MaxMembers::get() as usize, - DispatchError::Other("The sum of ayes and nays is greater than `MaxMembers`") + "The sum of ayes and nays is greater than `MaxMembers`" ); } Ok(()) @@ -1007,7 +1007,7 @@ impl, I: 'static> Pallet { let proposal_index = votes.index; ensure!( !proposal_indices.contains(&proposal_index), - DispatchError::Other("The proposal index is not unique.") + "The proposal index is not unique." ); proposal_indices.push(proposal_index); } @@ -1026,19 +1026,16 @@ impl, I: 'static> Pallet { ensure!( Self::members().len() <= T::MaxMembers::get() as usize, - DispatchError::Other("The member count is greater than `MaxMembers`.") + "The member count is greater than `MaxMembers`." ); ensure!( Self::members().windows(2).all(|members| members[0] <= members[1]), - DispatchError::Other("The members are not sorted by value.") + "The members are not sorted by value." ); if let Some(prime) = Self::prime() { - ensure!( - Self::members().contains(&prime), - DispatchError::Other("Prime account is not a member.") - ); + ensure!(Self::members().contains(&prime), "Prime account is not a member."); } Ok(()) diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index e876d0c1015cb..69712777851ed 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -359,10 +359,7 @@ mod v8 { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); - ensure!( - reserved >= value.storage_deposit, - DispatchError::Other("Reserved balance out of sync.") - ); + ensure!(reserved >= value.storage_deposit, "Reserved balance out of sync."); } Ok(()) } @@ -445,7 +442,7 @@ mod post_checks { .storage_base_deposit .saturating_add(value.storage_byte_deposit) .saturating_add(value.storage_item_deposit); - ensure!(reserved >= stored, DispatchError::Other("Reserved balance out of sync.")); + ensure!(reserved >= stored, "Reserved balance out of sync."); let mut storage_bytes = 0u32; let mut storage_items = 0u32; @@ -458,14 +455,8 @@ mod post_checks { storage_bytes.saturating_accrue(len); storage_items.saturating_accrue(1); } - ensure!( - storage_bytes == value.storage_bytes, - DispatchError::Other("Storage bytes do not match.") - ); - ensure!( - storage_items == value.storage_items, - DispatchError::Other("Storage items do not match.") - ); + ensure!(storage_bytes == value.storage_bytes, "Storage bytes do not match."); + ensure!(storage_items == value.storage_items, "Storage items do not match."); } Ok(()) } @@ -474,7 +465,7 @@ mod post_checks { for value in CodeStorage::::iter_values() { ensure!( value.determinism == Determinism::Enforced, - DispatchError::Other("All pre-existing codes need to be deterministic.") + "All pre-existing codes need to be deterministic." ); } Ok(()) diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index 83f2c95e61303..dc55f67b2d81f 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -62,10 +62,7 @@ pub mod v1 { impl> OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!( - StorageVersion::get::>() == 0, - DispatchError::Other("can only upgrade from version 0") - ); + ensure!(StorageVersion::get::>() == 0, "can only upgrade from version 0"); let props_count = v0::PublicProps::::get().len(); log::info!(target: TARGET, "{} public proposals will be migrated.", props_count,); @@ -137,20 +134,14 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> frame_support::dispatch::DispatchResult { - ensure!(StorageVersion::get::>() == 1, DispatchError::Other("must upgrade")); + ensure!(StorageVersion::get::>() == 1, "must upgrade"); let (old_props_count, old_ref_count): (u32, u32) = Decode::decode(&mut &state[..]).expect("pre_upgrade provides a valid state; qed"); let new_props_count = crate::PublicProps::::get().len() as u32; - ensure!( - new_props_count == old_props_count, - DispatchError::Other("must migrate all public proposals") - ); + ensure!(new_props_count == old_props_count, "must migrate all public proposals"); let new_ref_count = crate::ReferendumInfoOf::::iter().count() as u32; - ensure!( - new_ref_count == old_ref_count, - DispatchError::Other("must migrate all referenda") - ); + ensure!(new_ref_count == old_ref_count, "must migrate all referenda"); log::info!( target: TARGET, diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 946e98714fe6b..0d4f1d2f16f2c 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -233,7 +233,7 @@ pub mod pallet { // `T::MaxErasToCheckPerBlock`. ensure!( ErasToCheckPerBlock::::get() <= T::MaxErasToCheckPerBlock::get(), - DispatchError::Other("the value of `ErasToCheckPerBlock` is greater than `T::MaxErasToCheckPerBlock`"), + "the value of `ErasToCheckPerBlock` is greater than `T::MaxErasToCheckPerBlock`", ); Ok(()) diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index ba8936bad8c75..9b135a53f7e8c 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -85,7 +85,7 @@ pub mod v1 { fn post_upgrade(_: Vec) -> DispatchResult { ensure!( Pallet::::on_chain_storage_version() == 1, - DispatchError::Other("The onchain version must be updated after the migration.") + "The onchain version must be updated after the migration." ); Ok(()) } diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 8a89c286e5f02..7dd3c8b10303b 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -46,7 +46,7 @@ pub mod v1 { fn pre_upgrade() -> Result, DispatchError> { let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 1, DispatchError::Other("this migration can be deleted")); + ensure!(onchain < 1, "this migration can be deleted"); log!(info, "Number of calls to refund and delete: {}", Calls::::iter().count()); @@ -74,8 +74,8 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn post_upgrade(_state: Vec) -> DispatchResult { let onchain = Pallet::::on_chain_storage_version(); - ensure!(onchain < 2, DispatchError::Other("this migration needs to be removed")); - ensure!(onchain == 1, DispatchError::Other("this migration needs to be run")); + ensure!(onchain < 2, "this migration needs to be removed"); + ensure!(onchain == 1, "this migration needs to be run"); ensure!( Calls::::iter().count() == 0, DispatchError::Other( diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index 4ae56b332c853..c716d04745776 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -93,10 +93,7 @@ pub mod v1 { fn pre_upgrade() -> Result, DispatchError> { let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); - ensure!( - onchain_version == 0 && current_version == 1, - DispatchError::Other("migration from version 0 to 1.") - ); + ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1."); let prev_count = Collection::::iter().count(); Ok((prev_count as u32).encode()) } @@ -114,10 +111,7 @@ pub mod v1 { ) ); - ensure!( - Pallet::::on_chain_storage_version() == 1, - DispatchError::Other("wrong storage version") - ); + ensure!(Pallet::::on_chain_storage_version() == 1, "wrong storage version"); Ok(()) } diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 789ee7e0db302..1de27b99b3bb1 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -3073,11 +3073,11 @@ impl Pallet { ensure!( SubPoolsStorage::::iter_keys().all(|k| bonded_pools.contains(&k)), - DispatchError::Other("`SubPoolsStorage` must be a subset of the above superset.") + "`SubPoolsStorage` must be a subset of the above superset." ); ensure!( Metadata::::iter_keys().all(|k| bonded_pools.contains(&k)), - DispatchError::Other("`Metadata` keys must be a subset of the above superset.") + "`Metadata` keys must be a subset of the above superset." ); ensure!( @@ -3105,10 +3105,7 @@ impl Pallet { let mut all_members = 0u32; PoolMembers::::iter().try_for_each(|(_, d)| -> DispatchResult { let bonded_pool = BondedPools::::get(d.pool_id).unwrap(); - ensure!( - !d.total_points().is_zero(), - DispatchError::Other("No member should have zero points") - ); + ensure!(!d.total_points().is_zero(), "No member should have zero points"); *pools_members.entry(d.pool_id).or_default() += 1; all_members += 1; @@ -3150,7 +3147,7 @@ impl Pallet { ensure!( pools_members.get(&id).copied().unwrap_or_default() == bonded_pool.member_counter, - DispatchError::Other("Each `BondedPool.member_counter` must be equal to the actual count of members of this pool") + "Each `BondedPool.member_counter` must be equal to the actual count of members of this pool" ); ensure!(MaxPoolMembersPerPool::::get() .map_or(true, |max| bonded_pool.member_counter <= max), diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 4a341e3d98e37..33b07fb262f3b 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -104,7 +104,7 @@ pub mod v1 { // new version must be set. ensure!( Pallet::::on_chain_storage_version() == 1, - DispatchError::Other("The onchain version must be updated after the migration.") + "The onchain version must be updated after the migration." ); Pallet::::try_state(frame_system::Pallet::::block_number())?; Ok(()) @@ -361,7 +361,7 @@ pub mod v2 { ensure!( T::Currency::free_balance(&Pallet::::create_reward_account(id)) >= T::Currency::minimum_balance(), - DispatchError::Other("Reward accounts must have greater balance than ED.") + "Reward accounts must have greater balance than ED." ); Ok(()) })?; @@ -374,7 +374,7 @@ pub mod v2 { // new version must be set. ensure!( Pallet::::on_chain_storage_version() == 2, - DispatchError::Other("The onchain version must be updated after the migration.") + "The onchain version must be updated after the migration." ); // no reward or bonded pool has been skipped. @@ -396,7 +396,7 @@ pub mod v2 { RewardPools::::iter().try_for_each(|(id, _)| -> DispatchResult { ensure!( RewardPool::::current_balance(id) == Zero::zero(), - DispatchError::Other("Reward pool balance must be zero."), + "Reward pool balance must be zero.", ); Ok(()) })?; @@ -463,12 +463,9 @@ pub mod v3 { fn post_upgrade(_: Vec) -> DispatchResult { ensure!( Metadata::::iter_keys().all(|id| BondedPools::::contains_key(&id)), - DispatchError::Other("not all of the stale metadata has been removed") - ); - ensure!( - Pallet::::on_chain_storage_version() == 3, - DispatchError::Other("wrong storage version") + "not all of the stale metadata has been removed" ); + ensure!(Pallet::::on_chain_storage_version() == 3, "wrong storage version"); Ok(()) } } @@ -570,16 +567,13 @@ pub mod v4 { inner.commission.max.is_none() && inner.commission.change_rate.is_none() && inner.commission.throttle_from.is_none()), - DispatchError::Other("a commission value has been incorrectly set") + "a commission value has been incorrectly set" ); ensure!( GlobalMaxCommission::::get() == Some(U::get()), - DispatchError::Other("global maximum commission error") - ); - ensure!( - Pallet::::on_chain_storage_version() == 4, - DispatchError::Other("wrong storage version") + "global maximum commission error" ); + ensure!(Pallet::::on_chain_storage_version() == 4, "wrong storage version"); Ok(()) } } @@ -682,7 +676,7 @@ pub mod v5 { let rpool_values = RewardPools::::iter_values().count() as u64; ensure!( rpool_keys == rpool_values, - DispatchError::Other("There are STILL undecodable RewardPools - migration failed") + "There are STILL undecodable RewardPools - migration failed" ); if old_rpool_values != rpool_values { @@ -701,30 +695,27 @@ pub mod v5 { .is_zero() && reward_pool .total_commission_claimed .is_zero()), - DispatchError::Other("a commission value has been incorrectly set") - ); - ensure!( - Pallet::::on_chain_storage_version() == 5, - DispatchError::Other("wrong storage version") + "a commission value has been incorrectly set" ); + ensure!(Pallet::::on_chain_storage_version() == 5, "wrong storage version"); // These should not have been touched - just in case. ensure!( PoolMembers::::iter_keys().count() == PoolMembers::::iter_values().count(), - DispatchError::Other("There are undecodable PoolMembers in storage.") + "There are undecodable PoolMembers in storage." ); ensure!( BondedPools::::iter_keys().count() == BondedPools::::iter_values().count(), - DispatchError::Other("There are undecodable BondedPools in storage.") + "There are undecodable BondedPools in storage." ); ensure!( SubPoolsStorage::::iter_keys().count() == SubPoolsStorage::::iter_values().count(), - DispatchError::Other("There are undecodable SubPools in storage.") + "There are undecodable SubPools in storage." ); ensure!( Metadata::::iter_keys().count() == Metadata::::iter_values().count(), - DispatchError::Other("There are undecodable Metadata in storage.") + "There are undecodable Metadata in storage." ); Ok(()) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 4cb255e23b4b8..49a0596587706 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4269,10 +4269,7 @@ mod create { assert!(!BondedPools::::contains_key(2)); assert!(!RewardPools::::contains_key(2)); assert!(!PoolMembers::::contains_key(11)); - assert_err!( - StakingMock::active_stake(&next_pool_stash), - DispatchError::Other("balance not found") - ); + assert_err!(StakingMock::active_stake(&next_pool_stash), "balance not found"); Balances::make_free_balance_be(&11, StakingMock::minimum_nominator_bond() + ed); assert_ok!(Pools::create( diff --git a/frame/preimage/src/migration.rs b/frame/preimage/src/migration.rs index b545098a69114..10598368154ca 100644 --- a/frame/preimage/src/migration.rs +++ b/frame/preimage/src/migration.rs @@ -85,10 +85,7 @@ pub mod v1 { impl OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!( - StorageVersion::get::>() == 0, - DispatchError::Other("can only upgrade from version 0") - ); + ensure!(StorageVersion::get::>() == 0, "can only upgrade from version 0"); let images = v0::image_count::().expect("v0 storage corrupted"); log::info!(target: TARGET, "Migrating {} images", &images); @@ -170,7 +167,7 @@ pub mod v1 { old_images ); } - ensure!(StorageVersion::get::>() == 1, DispatchError::Other("must upgrade")); + ensure!(StorageVersion::get::>() == 1, "must upgrade"); Ok(()) } } diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index f6ca50fdd3c89..efc8ec0e89708 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -97,7 +97,7 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { let onchain_version = Pallet::::on_chain_storage_version(); - ensure!(onchain_version == 0, DispatchError::Other("migration from version 0 to 1.")); + ensure!(onchain_version == 0, "migration from version 0 to 1."); let referendum_count = v0::ReferendumInfoFor::::iter().count(); log::info!( target: TARGET, @@ -149,17 +149,11 @@ pub mod v1 { #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> DispatchResult { let onchain_version = Pallet::::on_chain_storage_version(); - ensure!( - onchain_version == 1, - DispatchError::Other("must upgrade from version 0 to 1.") - ); + ensure!(onchain_version == 1, "must upgrade from version 0 to 1."); let pre_referendum_count: u32 = Decode::decode(&mut &state[..]) .expect("failed to decode the state from pre-upgrade."); let post_referendum_count = ReferendumInfoFor::::iter().count() as u32; - ensure!( - post_referendum_count == pre_referendum_count, - DispatchError::Other("must migrate all referendums.") - ); + ensure!(post_referendum_count == pre_referendum_count, "must migrate all referendums."); log::info!(target: TARGET, "migrated all referendums."); Ok(()) } diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 788306080dc4c..372dc74446ab6 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -98,10 +98,7 @@ pub mod v3 { impl> OnRuntimeUpgrade for MigrateToV4 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { - ensure!( - StorageVersion::get::>() == 3, - DispatchError::Other("Can only upgrade from version 3") - ); + ensure!(StorageVersion::get::>() == 3, "Can only upgrade from version 3"); let agendas = Agenda::::iter_keys().count() as u32; let decodable_agendas = Agenda::::iter_values().count() as u32; @@ -175,14 +172,11 @@ pub mod v3 { #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> DispatchResult { - ensure!(StorageVersion::get::>() == 4, DispatchError::Other("Must upgrade")); + ensure!(StorageVersion::get::>() == 4, "Must upgrade"); // Check that everything decoded fine. for k in crate::Agenda::::iter_keys() { - ensure!( - crate::Agenda::::try_get(k).is_ok(), - DispatchError::Other("Cannot decode V4 Agenda") - ); + ensure!(crate::Agenda::::try_get(k).is_ok(), "Cannot decode V4 Agenda"); } let old_agendas: u32 = @@ -294,10 +288,7 @@ pub mod v4 { #[cfg(feature = "try-runtime")] fn post_upgrade(state: Vec) -> DispatchResult { - ensure!( - StorageVersion::get::>() == 4, - DispatchError::Other("Version must not change") - ); + ensure!(StorageVersion::get::>() == 4, "Version must not change"); let (old_agendas, non_empty_agendas): (u32, u32) = Decode::decode(&mut state.as_ref()).expect("Must decode pre_upgrade state"); @@ -316,10 +307,7 @@ pub mod v4 { old_agendas, new_agendas ), } - ensure!( - new_agendas == non_empty_agendas, - DispatchError::Other("Expected to keep all non-empty agendas") - ); + ensure!(new_agendas == non_empty_agendas, "Expected to keep all non-empty agendas"); Ok(()) } diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 87d1a2669d977..8e230c16fc0ff 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -67,7 +67,7 @@ pub mod v13 { fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, - DispatchError::Other("Required v12 before upgrading to v13") + "Required v12 before upgrading to v13" ); Ok(Default::default()) @@ -93,12 +93,12 @@ pub mod v13 { fn post_upgrade(_state: Vec) -> DispatchResult { frame_support::ensure!( Pallet::::on_chain_storage_version() == 13, - DispatchError::Other("v13 not applied") + "v13 not applied" ); frame_support::ensure!( !StorageVersion::::exists(), - DispatchError::Other("Storage version not migrated correctly") + "Storage version not migrated correctly" ); Ok(()) @@ -123,13 +123,13 @@ pub mod v12 { fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, - DispatchError::Other("Expected v11 before upgrading to v12") + "Expected v11 before upgrading to v12" ); if HistoryDepth::::exists() { frame_support::ensure!( T::HistoryDepth::get() == HistoryDepth::::get(), - DispatchError::Other("Provided value of HistoryDepth should be same as the existing storage value") + "Provided value of HistoryDepth should be same as the existing storage value" ); } else { log::info!("No HistoryDepth in storage; nothing to remove"); @@ -155,7 +155,7 @@ pub mod v12 { fn post_upgrade(_state: Vec) -> DispatchResult { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, - DispatchError::Other("v12 not applied") + "v12 not applied" ); Ok(()) } @@ -179,13 +179,13 @@ pub mod v11 { fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V10_0_0, - DispatchError::Other("must upgrade linearly") + "must upgrade linearly" ); let old_pallet_prefix = twox_128(N::get().as_bytes()); frame_support::ensure!( sp_io::storage::next_key(&old_pallet_prefix).is_some(), - DispatchError::Other("no data for the old pallet name has been detected") + "no data for the old pallet name has been detected" ); Ok(Default::default()) @@ -226,7 +226,7 @@ pub mod v11 { fn post_upgrade(_state: Vec) -> DispatchResult { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, - DispatchError::Other("wrong version after the upgrade") + "wrong version after the upgrade" ); let old_pallet_name = N::get(); @@ -240,14 +240,14 @@ pub mod v11 { let old_pallet_prefix = twox_128(N::get().as_bytes()); frame_support::ensure!( sp_io::storage::next_key(&old_pallet_prefix).is_none(), - DispatchError::Other("old pallet data hasn't been removed") + "old pallet data hasn't been removed" ); let new_pallet_name =

::name(); let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); frame_support::ensure!( sp_io::storage::next_key(&new_pallet_prefix).is_some(), - DispatchError::Other("new pallet data hasn't been created") + "new pallet data hasn't been created" ); Ok(()) @@ -341,7 +341,7 @@ pub mod v9 { fn pre_upgrade() -> Result, DispatchError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V8_0_0, - DispatchError::Other("must upgrade linearly") + "must upgrade linearly" ); let prev_count = T::VoterList::count(); @@ -365,7 +365,7 @@ pub mod v9 { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V9_0_0, - DispatchError::Other("must upgrade") + "must upgrade" ); Ok(()) } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 2d302717c42af..000fef6230b76 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1695,7 +1695,7 @@ impl Pallet { ensure!( T::VoterList::iter() .all(|x| >::contains_key(&x) || >::contains_key(&x)), - DispatchError::Other("VoterList contains non-staker") + "VoterList contains non-staker" ); Self::check_nominators()?; @@ -1708,11 +1708,11 @@ impl Pallet { ensure!( ::VoterList::count() == Nominators::::count() + Validators::::count(), - DispatchError::Other("wrong external count") + "wrong external count" ); ensure!( ::TargetList::count() == Validators::::count(), - DispatchError::Other("wrong external count") + "wrong external count" ); ensure!( ValidatorCount::::get() <= @@ -1741,7 +1741,7 @@ impl Pallet { .iter() .map(|e| e.value) .fold(Zero::zero(), |acc, x| acc + x), - DispatchError::Other("wrong total exposure."), + "wrong total exposure.", ); Ok(()) }) From ae24d0ed872d2ac57df45d5f2eaf01704741c27c Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 28 Apr 2023 19:57:25 +0200 Subject: [PATCH 20/31] merge fixes --- .../election-provider-multi-phase/src/lib.rs | 33 +++++++++------ frame/elections-phragmen/src/lib.rs | 41 ++++++++++++------- frame/offences/src/migration.rs | 9 ++-- 3 files changed, 54 insertions(+), 29 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index c5247e3c21c23..fdd63c2b2350a 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -257,6 +257,9 @@ use sp_runtime::{ }; use sp_std::prelude::*; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchResult; + #[cfg(feature = "runtime-benchmarks")] mod benchmarking; #[cfg(test)] @@ -883,7 +886,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> Result<(), &'static str> { + fn try_state(_n: T::BlockNumber) -> DispatchResult { Self::do_try_state() } } @@ -1579,7 +1582,7 @@ impl Pallet { #[cfg(feature = "try-runtime")] impl Pallet { - fn do_try_state() -> Result<(), &'static str> { + fn do_try_state() -> DispatchResult { Self::try_state_snapshot()?; Self::try_state_signed_submissions_map()?; Self::try_state_phase_off() @@ -1588,7 +1591,7 @@ impl Pallet { // [`Snapshot`] state check. Invariants: // - [`DesiredTargets`] exists if and only if [`Snapshot`] is present. // - [`SnapshotMetadata`] exist if and only if [`Snapshot`] is present. - fn try_state_snapshot() -> Result<(), &'static str> { + fn try_state_snapshot() -> DispatchResult { if >::exists() && >::exists() && >::exists() @@ -1600,7 +1603,7 @@ impl Pallet { { Ok(()) } else { - Err("If snapshot exists, metadata and desired targets should be set too. Otherwise, none should be set.") + Err(DispatchError::Other("If snapshot exists, metadata and desired targets should be set too. Otherwise, none should be set.")) } } @@ -1608,37 +1611,43 @@ impl Pallet { // - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more; // - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`]; // - [`SignedSubmissionIndices`] is sorted by election score. - fn try_state_signed_submissions_map() -> Result<(), &'static str> { + fn try_state_signed_submissions_map() -> DispatchResult { let mut last_score: ElectionScore = Default::default(); let indices = >::get(); for (i, indice) in indices.iter().enumerate() { let submission = >::get(indice.2); if submission.is_none() { - return Err("All signed submissions indices must be part of the submissions map") + return Err(DispatchError::Other( + "All signed submissions indices must be part of the submissions map", + )) } if i == 0 { last_score = indice.0 } else { if last_score.strict_threshold_better(indice.0, Perbill::zero()) { - return Err("Signed submission indices vector must be ordered by election score") + return Err(DispatchError::Other( + "Signed submission indices vector must be ordered by election score", + )) } last_score = indice.0; } } if >::iter().nth(indices.len()).is_some() { - return Err("Signed submissions map length should be the same as the indices vec length") + return Err(DispatchError::Other( + "Signed submissions map length should be the same as the indices vec length", + )) } match >::get() { 0 => Ok(()), next => if >::get(next).is_some() { - return Err( + return Err(DispatchError::Other( "The next submissions index should not be in the submissions maps already", - ) + )) } else { Ok(()) }, @@ -1647,12 +1656,12 @@ impl Pallet { // [`Phase::Off`] state check. Invariants: // - If phase is `Phase::Off`, [`Snapshot`] must be none. - fn try_state_phase_off() -> Result<(), &'static str> { + fn try_state_phase_off() -> DispatchResult { match Self::current_phase().is_off() { false => Ok(()), true => if >::get().is_some() { - Err("Snapshot must be none when in Phase::Off") + Err(DispatchError::Other("Snapshot must be none when in Phase::Off")) } else { Ok(()) }, diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 9c40e542e1c2a..0d633216c26a0 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -115,6 +115,9 @@ use sp_runtime::{ }; use sp_std::{cmp::Ordering, prelude::*}; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchResult; + mod benchmarking; pub mod weights; pub use weights::WeightInfo; @@ -327,7 +330,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> Result<(), &'static str> { + fn try_state(_n: T::BlockNumber) -> DispatchResult { Self::do_try_state() } } @@ -1199,7 +1202,7 @@ impl ContainsLengthBound for Pallet { #[cfg(any(feature = "try-runtime", test))] impl Pallet { - fn do_try_state() -> Result<(), &'static str> { + fn do_try_state() -> DispatchResult { Self::try_state_members()?; Self::try_state_runners_up()?; Self::try_state_candidates()?; @@ -1210,20 +1213,22 @@ impl Pallet { /// [`Members`] state checks. Invariants: /// - Members are always sorted based on account ID. - fn try_state_members() -> Result<(), &'static str> { + fn try_state_members() -> DispatchResult { let mut members = Members::::get().clone(); members.sort_by_key(|m| m.who.clone()); if Members::::get() == members { Ok(()) } else { - Err("try_state checks: Members must be always sorted by account ID") + Err(DispatchError::Other( + "try_state checks: Members must be always sorted by account ID", + )) } } // [`RunnersUp`] state checks. Invariants: // - Elements are sorted based on weight (worst to best). - fn try_state_runners_up() -> Result<(), &'static str> { + fn try_state_runners_up() -> DispatchResult { let mut sorted = RunnersUp::::get(); // worst stake first sorted.sort_by(|a, b| a.stake.cmp(&b.stake)); @@ -1231,27 +1236,33 @@ impl Pallet { if RunnersUp::::get() == sorted { Ok(()) } else { - Err("try_state checks: Runners Up must always be sorted by stake (worst to best)") + Err(DispatchError::Other( + "try_state checks: Runners Up must always be sorted by stake (worst to best)", + )) } } // [`Candidates`] state checks. Invariants: // - Always sorted based on account ID. - fn try_state_candidates() -> Result<(), &'static str> { + fn try_state_candidates() -> DispatchResult { let mut candidates = Candidates::::get().clone(); candidates.sort_by_key(|(c, _)| c.clone()); if Candidates::::get() == candidates { Ok(()) } else { - Err("try_state checks: Candidates must be always sorted by account ID") + Err(DispatchError::Other( + "try_state checks: Candidates must be always sorted by account ID", + )) } } // [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Candidates and runners-ups sets are disjoint. - fn try_state_candidates_runners_up_disjoint() -> Result<(), &'static str> { + fn try_state_candidates_runners_up_disjoint() -> DispatchResult { match Self::intersects(&Self::candidates_ids(), &Self::runners_up_ids()) { - true => Err("Candidates and runners up sets should always be disjoint"), + true => Err(DispatchError::Other( + "Candidates and runners up sets should always be disjoint", + )), false => Ok(()), } } @@ -1259,11 +1270,13 @@ impl Pallet { // [`Members`], [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Members and candidates sets are disjoint; // - Members and runners-ups sets are disjoint. - fn try_state_members_disjoint() -> Result<(), &'static str> { + fn try_state_members_disjoint() -> DispatchResult { match Self::intersects(&Pallet::::members_ids(), &Self::candidates_ids()) && Self::intersects(&Pallet::::members_ids(), &Self::runners_up_ids()) { - true => Err("Members set should be disjoint from candidates and runners-up sets"), + true => Err(DispatchError::Other( + "Members set should be disjoint from candidates and runners-up sets", + )), false => Ok(()), } } @@ -1271,14 +1284,14 @@ impl Pallet { // [`Members`], [`RunnersUp`] and approval stake state checks. Invariants: // - Selected members should have approval stake; // - Selected RunnersUp should have approval stake. - fn try_state_members_approval_stake() -> Result<(), &'static str> { + fn try_state_members_approval_stake() -> DispatchResult { match Members::::get() .iter() .chain(RunnersUp::::get().iter()) .all(|s| s.stake != BalanceOf::::zero()) { true => Ok(()), - false => Err("Members and RunnersUp must have approval stake"), + false => Err(DispatchError::Other("Members and RunnersUp must have approval stake")), } } diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index 07bd68407d378..e324b6e8eaa16 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -28,7 +28,10 @@ use sp_staking::offence::{DisableStrategy, OnOffenceHandler}; use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] -use frame_support::ensure; +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + ensure, +}; mod v0 { use super::*; @@ -51,7 +54,7 @@ pub mod v1 { pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, &'static str> { + fn pre_upgrade() -> Result, DispatchError> { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 1, "pallet_offences::MigrateToV1 migration can be deleted"); @@ -81,7 +84,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> DispatchResult { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain == 1, "pallet_offences::MigrateToV1 needs to be run"); ensure!( From f22801c37f78651bac8a18431f2658011913e812 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 28 Apr 2023 21:23:49 +0200 Subject: [PATCH 21/31] small fix --- frame/elections-phragmen/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 0d633216c26a0..545ef138173ad 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -115,7 +115,7 @@ use sp_runtime::{ }; use sp_std::{cmp::Ordering, prelude::*}; -#[cfg(feature = "try-runtime")] +#[cfg(any(feature = "try-runtime", test))] use frame_support::dispatch::DispatchResult; mod benchmarking; From d9942913a23b81d56e10041c9a499fdd1ffdd7b0 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 29 Apr 2023 11:20:45 +0200 Subject: [PATCH 22/31] should be good now --- frame/assets/src/migration.rs | 8 ++------ frame/bags-list/src/migrations.rs | 20 ++++---------------- frame/fast-unstake/src/migrations.rs | 4 +--- frame/multisig/src/migrations.rs | 4 +--- frame/nfts/src/migration.rs | 4 +--- frame/nomination-pools/src/lib.rs | 17 +++++++---------- frame/staking/src/migrations.rs | 6 ++---- 7 files changed, 18 insertions(+), 45 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 57c42779564cc..737489a45c8e2 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -109,9 +109,7 @@ pub mod v1 { let post_count = Asset::::iter().count() as u32; ensure!( prev_count == post_count, - DispatchError::Other( - "the asset count before and after the migration should be the same" - ) + "the asset count before and after the migration should be the same" ); let current_version = Pallet::::current_storage_version(); @@ -120,9 +118,7 @@ pub mod v1 { frame_support::ensure!(current_version == 1, "must_upgrade"); ensure!( current_version == onchain_version, - DispatchError::Other( - "after migration, the current_version and onchain_version should be the same" - ) + "after migration, the current_version and onchain_version should be the same" ); Asset::::iter().try_for_each(|(_id, asset)| -> DispatchResult { diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 3dfb277614279..29c0156539dcb 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -93,18 +93,12 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, DispatchError> { // The list node data should be corrupt at this point, so this is zero. - ensure!( - crate::ListNodes::::iter().count() == 0, - DispatchError::Other("list node data is not corrupt") - ); + ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); // We can use the helper `old::ListNode` to get the existing data. let iter_node_count: u32 = old::ListNodes::::iter().count() as u32; let tracked_node_count: u32 = old::CounterForListNodes::::get(); crate::log!(info, "number of nodes before: {:?} {:?}", iter_node_count, tracked_node_count); - ensure!( - iter_node_count == tracked_node_count, - DispatchError::Other("Node count is wrong.") - ); + ensure!(iter_node_count == tracked_node_count, "Node count is wrong."); Ok(iter_node_count.encode()) } @@ -140,14 +134,8 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { iter_node_count_after, tracked_node_count_after, ); - ensure!( - iter_node_count_after == node_count_before, - DispatchError::Other("Not all nodes were migrated.") - ); - ensure!( - tracked_node_count_after == iter_node_count_after, - DispatchError::Other("Node count is wrong.") - ); + ensure!(iter_node_count_after == node_count_before, "Not all nodes were migrated."); + ensure!(tracked_node_count_after == iter_node_count_after, "Node count is wrong."); Ok(()) } } diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index 9b135a53f7e8c..8ccea25ea03b6 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -74,9 +74,7 @@ pub mod v1 { fn pre_upgrade() -> Result, DispatchError> { ensure!( Pallet::::on_chain_storage_version() == 0, - DispatchError::Other( - "The onchain storage version must be zero for the migration to execute." - ) + "The onchain storage version must be zero for the migration to execute." ); Ok(Default::default()) } diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 7dd3c8b10303b..2f5f229deb99d 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -78,9 +78,7 @@ pub mod v1 { ensure!(onchain == 1, "this migration needs to be run"); ensure!( Calls::::iter().count() == 0, - DispatchError::Other( - "there are some dangling calls that need to be destroyed and refunded" - ) + "there are some dangling calls that need to be destroyed and refunded" ); Ok(()) } diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index c716d04745776..8a9a229042513 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -106,9 +106,7 @@ pub mod v1 { let post_count = Collection::::iter().count() as u32; ensure!( prev_count == post_count, - DispatchError::Other( - "the records count before and after the migration should be the same" - ) + "the records count before and after the migration should be the same" ); ensure!(Pallet::::on_chain_storage_version() == 1, "wrong storage version"); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 1de27b99b3bb1..d008cf69f2f7e 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -3066,9 +3066,7 @@ impl Pallet { let reward_pools = RewardPools::::iter_keys().collect::>(); ensure!( bonded_pools == reward_pools, - DispatchError::Other( - "`BondedPools` and `RewardPools` must all have the EXACT SAME key-set." - ) + "`BondedPools` and `RewardPools` must all have the EXACT SAME key-set." ); ensure!( @@ -3135,9 +3133,7 @@ impl Pallet { ensure!( RewardPool::::current_balance(id) >= pools_members_pending_rewards.get(&id).copied().unwrap_or_default(), - DispatchError::Other( - "The sum of the pending rewards must be less than the leftover balance." - ) + "The sum of the pending rewards must be less than the leftover balance." ); Ok(()) })?; @@ -3149,8 +3145,9 @@ impl Pallet { bonded_pool.member_counter, "Each `BondedPool.member_counter` must be equal to the actual count of members of this pool" ); - ensure!(MaxPoolMembersPerPool::::get() - .map_or(true, |max| bonded_pool.member_counter <= max), + ensure!( + MaxPoolMembersPerPool::::get() + .map_or(true, |max| bonded_pool.member_counter <= max), Error::::MaxPoolMembers ); @@ -3158,8 +3155,8 @@ impl Pallet { ensure!( bonded_pool.is_destroying_and_only_depositor(depositor.active_points()) || depositor.active_points() >= MinCreateBond::::get(), - DispatchError::Other("depositor must always have MinCreateBond stake in the pool, except for when the \ - pool is being destroyed and the depositor is the last member"), + "depositor must always have MinCreateBond stake in the pool, except for when the \ + pool is being destroyed and the depositor is the last member", ); Ok(()) })?; diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 8e230c16fc0ff..ca9fea6c0a215 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -357,10 +357,8 @@ pub mod v9 { let validators = Validators::::count(); ensure!( post_count == prev_count + validators, - DispatchError::Other( - "`VoterList` count after the migration must equal to the sum of \ - previous count and the current number of validators" - ) + "`VoterList` count after the migration must equal to the sum of \ + previous count and the current number of validators" ); frame_support::ensure!( From 5e7110cebc4c0d5f210a7000dc1a90a7e2090a69 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 29 Apr 2023 11:25:22 +0200 Subject: [PATCH 23/31] missed these ones --- frame/nomination-pools/src/migration.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index 33b07fb262f3b..cfc142c25255c 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -380,15 +380,11 @@ pub mod v2 { // no reward or bonded pool has been skipped. ensure!( RewardPools::::iter().count() as u32 == RewardPools::::count(), - DispatchError::Other( - "The count of reward pools must remain the same after the migration." - ) + "The count of reward pools must remain the same after the migration." ); ensure!( BondedPools::::iter().count() as u32 == BondedPools::::count(), - DispatchError::Other( - "The count of reward pools must remain the same after the migration." - ) + "The count of reward pools must remain the same after the migration." ); // all reward pools must have exactly ED in them. This means no reward can be claimed, From 0a3267a766b2eed9dc972077d9406c673d416868 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Tue, 2 May 2023 10:49:15 +0200 Subject: [PATCH 24/31] introduce TryRuntimeError and TryRuntimeResult --- frame/assets/src/migration.rs | 7 ++-- frame/bags-list/src/lib.rs | 9 ++-- frame/bags-list/src/list/mod.rs | 8 ++-- frame/bags-list/src/list/tests.rs | 7 ++-- frame/bags-list/src/migrations.rs | 14 +++---- frame/collective/src/lib.rs | 23 +++++------ frame/contracts/src/migration.rs | 14 +++---- frame/democracy/src/migrations.rs | 5 ++- .../election-provider-multi-phase/src/lib.rs | 41 ++++++++++--------- frame/election-provider-support/src/lib.rs | 4 +- frame/elections-phragmen/src/lib.rs | 18 ++++---- frame/executive/src/lib.rs | 4 +- frame/fast-unstake/src/lib.rs | 4 +- frame/fast-unstake/src/migrations.rs | 11 +++-- frame/multisig/src/migrations.rs | 6 ++- frame/nfts/src/migration.rs | 5 ++- frame/nomination-pools/src/lib.rs | 13 +++--- frame/nomination-pools/src/migration.rs | 25 ++++++----- frame/offences/src/migration.rs | 11 +++-- frame/preimage/src/migration.rs | 9 ++-- frame/referenda/src/migration.rs | 6 ++- frame/scheduler/src/migration.rs | 15 ++++--- frame/staking/src/migrations.rs | 23 +++++------ frame/staking/src/pallet/impls.rs | 26 ++++++------ frame/staking/src/pallet/mod.rs | 2 +- frame/support/procedural/src/lib.rs | 2 +- .../procedural/src/pallet/expand/hooks.rs | 2 +- frame/support/src/dispatch.rs | 10 ++--- frame/support/src/migrations.rs | 10 ++--- frame/support/src/traits/hooks.rs | 20 ++++----- frame/support/src/traits/try_runtime.rs | 18 ++++---- primitives/runtime/src/lib.rs | 6 +++ utils/frame/try-runtime/cli/src/lib.rs | 6 +-- 33 files changed, 201 insertions(+), 183 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 737489a45c8e2..6ec8c290e890f 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -17,6 +17,7 @@ use super::*; use frame_support::{log, traits::OnRuntimeUpgrade}; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; pub mod v1 { use frame_support::{pallet_prelude::*, weights::Weight}; @@ -92,7 +93,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { frame_support::ensure!( Pallet::::on_chain_storage_version() == 0, "must upgrade linearly" @@ -102,7 +103,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> DispatchResult { + fn post_upgrade(prev_count: Vec) -> TryRuntimeResult { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); @@ -121,7 +122,7 @@ pub mod v1 { "after migration, the current_version and onchain_version should be the same" ); - Asset::::iter().try_for_each(|(_id, asset)| -> DispatchResult { + Asset::::iter().try_for_each(|(_id, asset)| -> TryRuntimeResult { ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state"); Ok(()) })?; diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 703acb9ffc7e9..71faf79f34c1c 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -60,6 +60,9 @@ use frame_system::ensure_signed; use sp_runtime::traits::{AtLeast32BitUnsigned, Bounded, StaticLookup}; use sp_std::prelude::*; +#[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] +use sp_runtime::TryRuntimeResult; + #[cfg(any(feature = "runtime-benchmarks", test))] mod benchmarks; @@ -268,7 +271,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_: BlockNumberFor) -> DispatchResult { + fn try_state(_: BlockNumberFor) -> TryRuntimeResult { >::try_state() } } @@ -276,7 +279,7 @@ pub mod pallet { #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] impl, I: 'static> Pallet { - pub fn do_try_state() -> DispatchResult { + pub fn do_try_state() -> TryRuntimeResult { List::::do_try_state() } } @@ -356,7 +359,7 @@ impl, I: 'static> SortedListProvider for Pallet } #[cfg(feature = "try-runtime")] - fn try_state() -> DispatchResult { + fn try_state() -> TryRuntimeResult { Self::do_try_state() } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 1836786786baa..bb6734a927c57 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -43,7 +43,7 @@ use sp_std::{ }; #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] -use frame_support::dispatch::DispatchResult; +use sp_runtime::TryRuntimeResult; #[derive(Debug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, PalletError)] pub enum ListError { @@ -515,7 +515,7 @@ impl, I: 'static> List { /// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure /// all bags and nodes are checked per *any* update to `List`. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - pub(crate) fn do_try_state() -> DispatchResult { + pub(crate) fn do_try_state() -> TryRuntimeResult { let mut seen_in_list = BTreeSet::new(); ensure!( Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)), @@ -753,7 +753,7 @@ impl, I: 'static> Bag { /// * Ensures tail has no next. /// * Ensures there are no loops, traversal from head to tail is correct. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - fn do_try_state(&self) -> DispatchResult { + fn do_try_state(&self) -> TryRuntimeResult { frame_support::ensure!( self.head() .map(|head| head.prev().is_none()) @@ -898,7 +898,7 @@ impl, I: 'static> Node { } #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - fn do_try_state(&self) -> DispatchResult { + fn do_try_state(&self) -> TryRuntimeResult { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; let id = self.id(); diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index 0641012dc6a6e..fd4ad8f893af3 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -21,7 +21,8 @@ use crate::{ ListBags, ListNodes, }; use frame_election_provider_support::{SortedListProvider, VoteWeight}; -use frame_support::{assert_ok, assert_storage_noop, dispatch::DispatchError}; +use frame_support::{assert_ok, assert_storage_noop}; +use sp_runtime::TryRuntimeError; fn node( id: AccountId, @@ -361,7 +362,7 @@ mod list { Bag::::get(10).unwrap().insert_unchecked(2, 10); assert_eq!( List::::do_try_state(), - DispatchError::Other("duplicate identified").into() + TryRuntimeError::Other("duplicate identified").into() ); }); @@ -378,7 +379,7 @@ mod list { assert_eq!( List::::do_try_state(), - DispatchError::Other("iter_count != stored_count").into() + TryRuntimeError::Other("iter_count != stored_count").into() ); }); } diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index 29c0156539dcb..ae50ea257f4df 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -23,10 +23,10 @@ use frame_election_provider_support::ScoreProvider; use frame_support::traits::OnRuntimeUpgrade; #[cfg(feature = "try-runtime")] -use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, -}; +use frame_support::ensure; +#[cfg(feature = "try-runtime")] +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; + #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; @@ -38,7 +38,7 @@ impl, I: 'static> OnRuntimeUpgrade for CheckCounterPrefix Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { // The old explicit storage item. #[frame_support::storage_alias] type CounterForListNodes, I: 'static> = @@ -91,7 +91,7 @@ mod old { pub struct AddScore, I: 'static = ()>(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for AddScore { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { // The list node data should be corrupt at this point, so this is zero. ensure!(crate::ListNodes::::iter().count() == 0, "list node data is not corrupt"); // We can use the helper `old::ListNode` to get the existing data. @@ -122,7 +122,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { } #[cfg(feature = "try-runtime")] - fn post_upgrade(node_count_before: Vec) -> DispatchResult { + fn post_upgrade(node_count_before: Vec) -> TryRuntimeResult { let node_count_before: u32 = Decode::decode(&mut node_count_before.as_slice()) .expect("the state parameter should be something that was generated by pre_upgrade"); // Now the list node data is not corrupt anymore. diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 60393f1aaf069..64802193b3ab6 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -60,6 +60,9 @@ use frame_support::{ weights::Weight, }; +#[cfg(any(feature = "try-runtime", test))] +use sp_runtime::TryRuntimeResult; + #[cfg(test)] mod tests; @@ -352,7 +355,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> DispatchResult { + fn try_state(_n: BlockNumberFor) -> TryRuntimeResult { Self::do_try_state() } } @@ -972,13 +975,11 @@ impl, I: 'static> Pallet { /// Looking at prime account: /// * The prime account must be a member of the collective. #[cfg(any(feature = "try-runtime", test))] - fn do_try_state() -> DispatchResult { - Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult { + fn do_try_state() -> TryRuntimeResult { + Self::proposals().into_iter().try_for_each(|proposal| -> TryRuntimeResult { ensure!( Self::proposal_of(proposal).is_some(), - DispatchError::Other( - "Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping." - ) + "Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping." ); Ok(()) })?; @@ -992,7 +993,7 @@ impl, I: 'static> Pallet { "Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`" ); - Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult { + Self::proposals().into_iter().try_for_each(|proposal| -> TryRuntimeResult { if let Some(votes) = Self::voting(proposal) { let ayes = votes.ayes.len(); let nays = votes.nays.len(); @@ -1006,7 +1007,7 @@ impl, I: 'static> Pallet { })?; let mut proposal_indices = vec![]; - Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult { + Self::proposals().into_iter().try_for_each(|proposal| -> TryRuntimeResult { if let Some(votes) = Self::voting(proposal) { let proposal_index = votes.index; ensure!( @@ -1018,12 +1019,10 @@ impl, I: 'static> Pallet { Ok(()) })?; - >::iter_keys().try_for_each(|proposal_hash| -> DispatchResult { + >::iter_keys().try_for_each(|proposal_hash| -> TryRuntimeResult { ensure!( Self::proposals().contains(&proposal_hash), - DispatchError::Other( - "`Proposals` doesn't contain the proposal hash from the `Voting` storage map." - ) + "`Proposals` doesn't contain the proposal hash from the `Voting` storage map." ); Ok(()) })?; diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 69712777851ed..45b4721cc2343 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -25,7 +25,7 @@ use frame_support::{ traits::{Get, OnRuntimeUpgrade}, Identity, Twox64Concat, }; -use sp_runtime::traits::Saturating; +use sp_runtime::{traits::Saturating, TryRuntimeError, TryRuntimeResult}; use sp_std::{marker::PhantomData, prelude::*}; /// Performs all necessary migrations based on `StorageVersion`. @@ -66,7 +66,7 @@ impl OnRuntimeUpgrade for Migration { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { let version = >::on_chain_storage_version(); if version == 7 { @@ -77,7 +77,7 @@ impl OnRuntimeUpgrade for Migration { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> DispatchResult { + fn post_upgrade(state: Vec) -> TryRuntimeResult { let version = Decode::decode(&mut state.as_ref()).map_err(|_| "Cannot decode version")?; post_checks::post_upgrade::(version) } @@ -355,7 +355,7 @@ mod v8 { } #[cfg(feature = "try-runtime")] - pub fn pre_upgrade() -> Result<(), DispatchError> { + pub fn pre_upgrade() -> Result<(), TryRuntimeError> { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); @@ -418,7 +418,7 @@ mod post_checks { type ContractInfoOf = StorageMap, Twox64Concat, ::AccountId, V>; - pub fn post_upgrade(old_version: StorageVersion) -> DispatchResult { + pub fn post_upgrade(old_version: StorageVersion) -> TryRuntimeResult { if old_version < 7 { return Ok(()) } @@ -434,7 +434,7 @@ mod post_checks { Ok(()) } - fn v8() -> DispatchResult { + fn v8() -> TryRuntimeResult { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); @@ -461,7 +461,7 @@ mod post_checks { Ok(()) } - fn v9() -> DispatchResult { + fn v9() -> TryRuntimeResult { for value in CodeStorage::::iter_values() { ensure!( value.determinism == Determinism::Enforced, diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index dc55f67b2d81f..4b3584ef199b0 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -20,6 +20,7 @@ use super::*; use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade, BoundedVec}; use sp_core::H256; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// The log target. const TARGET: &'static str = "runtime::democracy::migration::v1"; @@ -61,7 +62,7 @@ pub mod v1 { impl> OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!(StorageVersion::get::>() == 0, "can only upgrade from version 0"); let props_count = v0::PublicProps::::get().len(); @@ -133,7 +134,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> frame_support::dispatch::DispatchResult { + fn post_upgrade(state: Vec) -> TryRuntimeResult { ensure!(StorageVersion::get::>() == 1, "must upgrade"); let (old_props_count, old_ref_count): (u32, u32) = diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index fdd63c2b2350a..9a960ee338eda 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -258,7 +258,7 @@ use sp_runtime::{ use sp_std::prelude::*; #[cfg(feature = "try-runtime")] -use frame_support::dispatch::DispatchResult; +use sp_runtime::TryRuntimeResult; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; @@ -886,7 +886,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> DispatchResult { + fn try_state(_n: T::BlockNumber) -> TryRuntimeResult { Self::do_try_state() } } @@ -1582,7 +1582,7 @@ impl Pallet { #[cfg(feature = "try-runtime")] impl Pallet { - fn do_try_state() -> DispatchResult { + fn do_try_state() -> TryRuntimeResult { Self::try_state_snapshot()?; Self::try_state_signed_submissions_map()?; Self::try_state_phase_off() @@ -1591,7 +1591,7 @@ impl Pallet { // [`Snapshot`] state check. Invariants: // - [`DesiredTargets`] exists if and only if [`Snapshot`] is present. // - [`SnapshotMetadata`] exist if and only if [`Snapshot`] is present. - fn try_state_snapshot() -> DispatchResult { + fn try_state_snapshot() -> TryRuntimeResult { if >::exists() && >::exists() && >::exists() @@ -1603,7 +1603,7 @@ impl Pallet { { Ok(()) } else { - Err(DispatchError::Other("If snapshot exists, metadata and desired targets should be set too. Otherwise, none should be set.")) + Err("If snapshot exists, metadata and desired targets should be set too. Otherwise, none should be set.".into()) } } @@ -1611,43 +1611,44 @@ impl Pallet { // - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more; // - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`]; // - [`SignedSubmissionIndices`] is sorted by election score. - fn try_state_signed_submissions_map() -> DispatchResult { + fn try_state_signed_submissions_map() -> TryRuntimeResult { let mut last_score: ElectionScore = Default::default(); let indices = >::get(); for (i, indice) in indices.iter().enumerate() { let submission = >::get(indice.2); if submission.is_none() { - return Err(DispatchError::Other( - "All signed submissions indices must be part of the submissions map", - )) + return Err( + "All signed submissions indices must be part of the submissions map".into() + ) } if i == 0 { last_score = indice.0 } else { if last_score.strict_threshold_better(indice.0, Perbill::zero()) { - return Err(DispatchError::Other( - "Signed submission indices vector must be ordered by election score", - )) + return Err( + "Signed submission indices vector must be ordered by election score".into() + ) } last_score = indice.0; } } if >::iter().nth(indices.len()).is_some() { - return Err(DispatchError::Other( - "Signed submissions map length should be the same as the indices vec length", - )) + return Err( + "Signed submissions map length should be the same as the indices vec length".into() + ) } match >::get() { 0 => Ok(()), next => if >::get(next).is_some() { - return Err(DispatchError::Other( - "The next submissions index should not be in the submissions maps already", - )) + return Err( + "The next submissions index should not be in the submissions maps already" + .into(), + ) } else { Ok(()) }, @@ -1656,12 +1657,12 @@ impl Pallet { // [`Phase::Off`] state check. Invariants: // - If phase is `Phase::Off`, [`Snapshot`] must be none. - fn try_state_phase_off() -> DispatchResult { + fn try_state_phase_off() -> TryRuntimeResult { match Self::current_phase().is_off() { false => Ok(()), true => if >::get().is_some() { - Err(DispatchError::Other("Snapshot must be none when in Phase::Off")) + Err("Snapshot must be none when in Phase::Off".into()) } else { Ok(()) }, diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index fae945fea8a74..14de843895f3b 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -190,7 +190,7 @@ pub use sp_npos_elections::{ pub use traits::NposSolution; #[cfg(feature = "try-runtime")] -use frame_support::dispatch::DispatchResult; +use sp_runtime::TryRuntimeResult; // re-export for the solution macro, with the dependencies of the macro. #[doc(hidden)] @@ -567,7 +567,7 @@ pub trait SortedListProvider { /// Check internal state of the list. Only meant for debugging. #[cfg(feature = "try-runtime")] - fn try_state() -> DispatchResult; + fn try_state() -> TryRuntimeResult; /// If `who` changes by the returned amount they are guaranteed to have a worst case change /// in their list position. diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 545ef138173ad..7fe851d769ec8 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -116,7 +116,7 @@ use sp_runtime::{ use sp_std::{cmp::Ordering, prelude::*}; #[cfg(any(feature = "try-runtime", test))] -use frame_support::dispatch::DispatchResult; +use sp_runtime::TryRuntimeResult; mod benchmarking; pub mod weights; @@ -330,7 +330,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> DispatchResult { + fn try_state(_n: T::BlockNumber) -> TryRuntimeResult { Self::do_try_state() } } @@ -1202,7 +1202,7 @@ impl ContainsLengthBound for Pallet { #[cfg(any(feature = "try-runtime", test))] impl Pallet { - fn do_try_state() -> DispatchResult { + fn do_try_state() -> TryRuntimeResult { Self::try_state_members()?; Self::try_state_runners_up()?; Self::try_state_candidates()?; @@ -1213,7 +1213,7 @@ impl Pallet { /// [`Members`] state checks. Invariants: /// - Members are always sorted based on account ID. - fn try_state_members() -> DispatchResult { + fn try_state_members() -> TryRuntimeResult { let mut members = Members::::get().clone(); members.sort_by_key(|m| m.who.clone()); @@ -1228,7 +1228,7 @@ impl Pallet { // [`RunnersUp`] state checks. Invariants: // - Elements are sorted based on weight (worst to best). - fn try_state_runners_up() -> DispatchResult { + fn try_state_runners_up() -> TryRuntimeResult { let mut sorted = RunnersUp::::get(); // worst stake first sorted.sort_by(|a, b| a.stake.cmp(&b.stake)); @@ -1244,7 +1244,7 @@ impl Pallet { // [`Candidates`] state checks. Invariants: // - Always sorted based on account ID. - fn try_state_candidates() -> DispatchResult { + fn try_state_candidates() -> TryRuntimeResult { let mut candidates = Candidates::::get().clone(); candidates.sort_by_key(|(c, _)| c.clone()); @@ -1258,7 +1258,7 @@ impl Pallet { } // [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Candidates and runners-ups sets are disjoint. - fn try_state_candidates_runners_up_disjoint() -> DispatchResult { + fn try_state_candidates_runners_up_disjoint() -> TryRuntimeResult { match Self::intersects(&Self::candidates_ids(), &Self::runners_up_ids()) { true => Err(DispatchError::Other( "Candidates and runners up sets should always be disjoint", @@ -1270,7 +1270,7 @@ impl Pallet { // [`Members`], [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Members and candidates sets are disjoint; // - Members and runners-ups sets are disjoint. - fn try_state_members_disjoint() -> DispatchResult { + fn try_state_members_disjoint() -> TryRuntimeResult { match Self::intersects(&Pallet::::members_ids(), &Self::candidates_ids()) && Self::intersects(&Pallet::::members_ids(), &Self::runners_up_ids()) { @@ -1284,7 +1284,7 @@ impl Pallet { // [`Members`], [`RunnersUp`] and approval stake state checks. Invariants: // - Selected members should have approval stake; // - Selected RunnersUp should have approval stake. - fn try_state_members_approval_stake() -> DispatchResult { + fn try_state_members_approval_stake() -> TryRuntimeResult { match Members::::get() .iter() .chain(RunnersUp::::get().iter()) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 30c1f44a5219f..19d30755d6350 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -138,7 +138,7 @@ use sp_runtime::{ use sp_std::{marker::PhantomData, prelude::*}; #[cfg(feature = "try-runtime")] -use frame_support::dispatch::DispatchError; +use sp_runtime::TryRuntimeError; #[allow(dead_code)] const LOG_TARGET: &str = "runtime::executive"; @@ -341,7 +341,7 @@ where /// `true`. Also, if set to `true`, it runs the `pre_upgrade` and `post_upgrade` hooks. pub fn try_runtime_upgrade( checks: frame_try_runtime::UpgradeCheckSelect, - ) -> Result { + ) -> Result { if checks.try_state() { let _guard = frame_support::StorageNoopGuard::default(); >::try_state( diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 0d4f1d2f16f2c..cedae115541aa 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -86,7 +86,7 @@ pub mod pallet { traits::{Defensive, ReservableCurrency, StorageVersion}, }; use frame_system::pallet_prelude::*; - use sp_runtime::{traits::Zero, DispatchResult}; + use sp_runtime::{traits::Zero, DispatchResult, TryRuntimeResult}; use sp_staking::{EraIndex, StakingInterface}; use sp_std::{prelude::*, vec::Vec}; pub use weights::WeightInfo; @@ -228,7 +228,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> DispatchResult { + fn try_state(_n: T::BlockNumber) -> TryRuntimeResult { // ensure that the value of `ErasToCheckPerBlock` is less than // `T::MaxErasToCheckPerBlock`. ensure!( diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index 8ccea25ea03b6..9ae0c172b0169 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -26,10 +26,9 @@ pub mod v1 { use sp_std::prelude::*; #[cfg(feature = "try-runtime")] - use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, - }; + use frame_support::ensure; + #[cfg(feature = "try-runtime")] + use sp_runtime::{TryRuntimeError, TryRuntimeResult}; pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { @@ -71,7 +70,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!( Pallet::::on_chain_storage_version() == 0, "The onchain storage version must be zero for the migration to execute." @@ -80,7 +79,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> DispatchResult { + fn post_upgrade(_: Vec) -> TryRuntimeResult { ensure!( Pallet::::on_chain_storage_version() == 1, "The onchain version must be updated after the migration." diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 2f5f229deb99d..980892edf2388 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -24,6 +24,8 @@ use frame_support::{ Identity, }; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; + #[cfg(feature = "try-runtime")] use frame_support::ensure; @@ -43,7 +45,7 @@ pub mod v1 { pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 1, "this migration can be deleted"); @@ -72,7 +74,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 2, "this migration needs to be removed"); ensure!(onchain == 1, "this migration needs to be run"); diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index 8a9a229042513..6123ff3430024 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -17,6 +17,7 @@ use super::*; use frame_support::{log, traits::OnRuntimeUpgrade}; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; pub mod v1 { use frame_support::{pallet_prelude::*, weights::Weight}; @@ -90,7 +91,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1."); @@ -99,7 +100,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> DispatchResult { + fn post_upgrade(prev_count: Vec) -> TryRuntimeResult { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index d008cf69f2f7e..752dd22f2fe30 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -378,6 +378,9 @@ use sp_runtime::{ use sp_staking::{EraIndex, OnStakerSlash, StakingInterface}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, ops::Div, vec::Vec}; +#[cfg(feature = "try-runtime")] +use sp_runtime::TryRuntimeResult; + /// The log target of this pallet. pub const LOG_TARGET: &str = "runtime::nomination-pools"; @@ -2627,7 +2630,7 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> DispatchResult { + fn try_state(_n: BlockNumberFor) -> TryRuntimeResult { Self::do_try_state(u8::MAX) } @@ -3056,7 +3059,7 @@ impl Pallet { /// multiple `level`s, where the higher the level, the more checks we performs. So, /// `try_state(255)` is the strongest sanity check, and `0` performs no checks. #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] - pub fn do_try_state(level: u8) -> DispatchResult { + pub fn do_try_state(level: u8) -> TryRuntimeResult { if level.is_zero() { return Ok(()) } @@ -3101,7 +3104,7 @@ impl Pallet { let mut pools_members = BTreeMap::::new(); let mut pools_members_pending_rewards = BTreeMap::>::new(); let mut all_members = 0u32; - PoolMembers::::iter().try_for_each(|(_, d)| -> DispatchResult { + PoolMembers::::iter().try_for_each(|(_, d)| -> TryRuntimeResult { let bonded_pool = BondedPools::::get(d.pool_id).unwrap(); ensure!(!d.total_points().is_zero(), "No member should have zero points"); *pools_members.entry(d.pool_id).or_default() += 1; @@ -3120,7 +3123,7 @@ impl Pallet { Ok(()) })?; - RewardPools::::iter_keys().try_for_each(|id| -> DispatchResult { + RewardPools::::iter_keys().try_for_each(|id| -> TryRuntimeResult { // the sum of the pending rewards must be less than the leftover balance. Since the // reward math rounds down, we might accumulate some dust here. log!( @@ -3138,7 +3141,7 @@ impl Pallet { Ok(()) })?; - BondedPools::::iter().try_for_each(|(id, inner)| -> DispatchResult { + BondedPools::::iter().try_for_each(|(id, inner)| -> TryRuntimeResult { let bonded_pool = BondedPool { id, inner }; ensure!( pools_members.get(&id).copied().unwrap_or_default() == diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index cfc142c25255c..cde7fb09f6558 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -20,6 +20,9 @@ use crate::log; use frame_support::traits::OnRuntimeUpgrade; use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; +#[cfg(feature = "try-runtime")] +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; + pub mod v1 { use super::*; @@ -100,7 +103,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> DispatchResult { + fn post_upgrade(_: Vec) -> TryRuntimeResult { // new version must be set. ensure!( Pallet::::on_chain_storage_version() == 1, @@ -355,9 +358,9 @@ pub mod v2 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { // all reward accounts must have more than ED. - RewardPools::::iter().try_for_each(|(id, _)| -> DispatchResult { + RewardPools::::iter().try_for_each(|(id, _)| -> TryRuntimeResult { ensure!( T::Currency::free_balance(&Pallet::::create_reward_account(id)) >= T::Currency::minimum_balance(), @@ -370,7 +373,7 @@ pub mod v2 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> DispatchResult { + fn post_upgrade(_: Vec) -> TryRuntimeResult { // new version must be set. ensure!( Pallet::::on_chain_storage_version() == 2, @@ -389,7 +392,7 @@ pub mod v2 { // all reward pools must have exactly ED in them. This means no reward can be claimed, // and that setting reward counters all over the board to zero will work henceforth. - RewardPools::::iter().try_for_each(|(id, _)| -> DispatchResult { + RewardPools::::iter().try_for_each(|(id, _)| -> TryRuntimeResult { ensure!( RewardPool::::current_balance(id) == Zero::zero(), "Reward pool balance must be zero.", @@ -447,7 +450,7 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" @@ -456,7 +459,7 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> DispatchResult { + fn post_upgrade(_: Vec) -> TryRuntimeResult { ensure!( Metadata::::iter_keys().all(|id| BondedPools::::contains_key(&id)), "not all of the stale metadata has been removed" @@ -547,7 +550,7 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" @@ -556,7 +559,7 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> DispatchResult { + fn post_upgrade(_: Vec) -> TryRuntimeResult { // ensure all BondedPools items now contain an `inner.commission: Commission` field. ensure!( BondedPools::::iter().all(|(_, inner)| inner.commission.current.is_none() && @@ -632,7 +635,7 @@ pub mod v5 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!( Pallet::::current_storage_version() > Pallet::::on_chain_storage_version(), "the on_chain version is equal or more than the current one" @@ -666,7 +669,7 @@ pub mod v5 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(data: Vec) -> DispatchResult { + fn post_upgrade(data: Vec) -> TryRuntimeResult { let old_rpool_values: u64 = Decode::decode(&mut &data[..]).unwrap(); let rpool_keys = RewardPools::::iter_keys().count() as u64; let rpool_values = RewardPools::::iter_values().count() as u64; diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index e324b6e8eaa16..6cc72c220b7a8 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -28,10 +28,9 @@ use sp_staking::offence::{DisableStrategy, OnOffenceHandler}; use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] -use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, -}; +use frame_support::ensure; +#[cfg(feature = "try-runtime")] +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; mod v0 { use super::*; @@ -54,7 +53,7 @@ pub mod v1 { pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 1, "pallet_offences::MigrateToV1 migration can be deleted"); @@ -84,7 +83,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain == 1, "pallet_offences::MigrateToV1 needs to be run"); ensure!( diff --git a/frame/preimage/src/migration.rs b/frame/preimage/src/migration.rs index 10598368154ca..46e555498cd2d 100644 --- a/frame/preimage/src/migration.rs +++ b/frame/preimage/src/migration.rs @@ -25,10 +25,9 @@ use frame_support::{ use sp_std::collections::btree_map::BTreeMap; #[cfg(feature = "try-runtime")] -use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, -}; +use frame_support::ensure; +#[cfg(feature = "try-runtime")] +use sp_runtime::TryRuntimeError; /// The log target. const TARGET: &'static str = "runtime::preimage::migration::v1"; @@ -84,7 +83,7 @@ pub mod v1 { impl OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!(StorageVersion::get::>() == 0, "can only upgrade from version 0"); let images = v0::image_count::().expect("v0 storage corrupted"); diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index efc8ec0e89708..ddbaf9dc1ceef 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -22,6 +22,8 @@ use codec::{Decode, Encode, EncodeLike, MaxEncodedLen}; use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade}; use log; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; + /// Initial version of storage types. pub mod v0 { use super::*; @@ -95,7 +97,7 @@ pub mod v1 { pub struct MigrateV0ToV1(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for MigrateV0ToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { let onchain_version = Pallet::::on_chain_storage_version(); ensure!(onchain_version == 0, "migration from version 0 to 1."); let referendum_count = v0::ReferendumInfoFor::::iter().count(); @@ -147,7 +149,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> DispatchResult { + fn post_upgrade(state: Vec) -> TryRuntimeResult { let onchain_version = Pallet::::on_chain_storage_version(); ensure!(onchain_version == 1, "must upgrade from version 0 to 1."); let pre_referendum_count: u32 = Decode::decode(&mut &state[..]) diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 372dc74446ab6..3f49a0e4821de 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -19,6 +19,7 @@ use super::*; use frame_support::traits::OnRuntimeUpgrade; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// The log target. const TARGET: &'static str = "runtime::scheduler::migration"; @@ -97,7 +98,7 @@ pub mod v3 { impl> OnRuntimeUpgrade for MigrateToV4 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { ensure!(StorageVersion::get::>() == 3, "Can only upgrade from version 3"); let agendas = Agenda::::iter_keys().count() as u32; @@ -125,9 +126,7 @@ pub mod v3 { agenda.len(), max_scheduled_per_block, ); - return Err(DispatchError::Other( - "Agenda would overflow `MaxScheduledPerBlock`.", - )) + return Err("Agenda would overflow `MaxScheduledPerBlock`.".into()) } } // Check that bounding the calls will not overflow `MAX_LENGTH`. @@ -144,7 +143,7 @@ pub mod v3 { block_number, l, ); - return Err(DispatchError::Other("Call is too large.")) + return Err("Call is too large.".into()) } }, _ => (), @@ -171,7 +170,7 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> DispatchResult { + fn post_upgrade(state: Vec) -> TryRuntimeResult { ensure!(StorageVersion::get::>() == 4, "Must upgrade"); // Check that everything decoded fine. @@ -212,7 +211,7 @@ pub mod v4 { impl OnRuntimeUpgrade for CleanupAgendas { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { assert_eq!( StorageVersion::get::>(), 4, @@ -287,7 +286,7 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> DispatchResult { + fn post_upgrade(state: Vec) -> TryRuntimeResult { ensure!(StorageVersion::get::>() == 4, "Version must not change"); let (old_agendas, non_empty_agendas): (u32, u32) = diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index ca9fea6c0a215..282ee9a6e30e4 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -24,10 +24,9 @@ use frame_support::{ }; #[cfg(feature = "try-runtime")] -use frame_support::{ - dispatch::{DispatchError, DispatchResult}, - ensure, -}; +use frame_support::ensure; +#[cfg(feature = "try-runtime")] +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// Used for release versioning upto v12. /// @@ -64,7 +63,7 @@ pub mod v13 { pub struct MigrateToV13(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV13 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, "Required v12 before upgrading to v13" @@ -90,7 +89,7 @@ pub mod v13 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { frame_support::ensure!( Pallet::::on_chain_storage_version() == 13, "v13 not applied" @@ -120,7 +119,7 @@ pub mod v12 { pub struct MigrateToV12(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV12 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, "Expected v11 before upgrading to v12" @@ -152,7 +151,7 @@ pub mod v12 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, "v12 not applied" @@ -176,7 +175,7 @@ pub mod v11 { for MigrateToV11 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V10_0_0, "must upgrade linearly" @@ -223,7 +222,7 @@ pub mod v11 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, "wrong version after the upgrade" @@ -338,7 +337,7 @@ pub mod v9 { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V8_0_0, "must upgrade linearly" @@ -349,7 +348,7 @@ pub mod v9 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> DispatchResult { + fn post_upgrade(prev_count: Vec) -> TryRuntimeResult { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index cbc507e07ee11..c996baedb0830 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -52,7 +52,9 @@ use crate::{ use super::{pallet::*, STAKING_ID}; #[cfg(feature = "try-runtime")] -use frame_support::{dispatch::DispatchResult, ensure}; +use frame_support::ensure; +#[cfg(feature = "try-runtime")] +use sp_runtime::TryRuntimeResult; /// The maximum number of iterations that we do whilst iterating over `T::VoterList` in /// `get_npos_voters`. @@ -1470,7 +1472,7 @@ impl SortedListProvider for UseValidatorsMap { 0 } #[cfg(feature = "try-runtime")] - fn try_state() -> DispatchResult { + fn try_state() -> TryRuntimeResult { Ok(()) } @@ -1547,7 +1549,7 @@ impl SortedListProvider for UseNominatorsAndValidatorsM } #[cfg(feature = "try-runtime")] - fn try_state() -> DispatchResult { + fn try_state() -> TryRuntimeResult { Ok(()) } @@ -1717,7 +1719,7 @@ impl StakingInterface for Pallet { #[cfg(any(test, feature = "try-runtime"))] impl Pallet { - pub(crate) fn do_try_state(_: BlockNumberFor) -> DispatchResult { + pub(crate) fn do_try_state(_: BlockNumberFor) -> TryRuntimeResult { ensure!( T::VoterList::iter() .all(|x| >::contains_key(&x) || >::contains_key(&x)), @@ -1730,7 +1732,7 @@ impl Pallet { Self::check_count() } - fn check_count() -> DispatchResult { + fn check_count() -> TryRuntimeResult { ensure!( ::VoterList::count() == Nominators::::count() + Validators::::count(), @@ -1748,14 +1750,14 @@ impl Pallet { Ok(()) } - fn check_ledgers() -> DispatchResult { + fn check_ledgers() -> TryRuntimeResult { Bonded::::iter() .map(|(_, ctrl)| Self::ensure_ledger_consistent(ctrl)) .collect::, _>>()?; Ok(()) } - fn check_exposures() -> DispatchResult { + fn check_exposures() -> TryRuntimeResult { // a check per validator to ensure the exposure struct is always sane. let era = Self::active_era().unwrap().index; ErasStakers::::iter_prefix_values(era) @@ -1771,10 +1773,10 @@ impl Pallet { ); Ok(()) }) - .collect::() + .collect::() } - fn check_nominators() -> DispatchResult { + fn check_nominators() -> TryRuntimeResult { // a check per nominator to ensure their entire stake is correctly distributed. Will only // kick-in if the nomination was submitted before the current era. let era = Self::active_era().unwrap().index; @@ -1788,14 +1790,14 @@ impl Pallet { } }, ) - .map(|nominator| -> DispatchResult { + .map(|nominator| -> TryRuntimeResult { // must be bonded. Self::ensure_is_stash(&nominator)?; let mut sum = BalanceOf::::zero(); T::SessionInterface::validators() .iter() .map(|v| Self::eras_stakers(era, v)) - .map(|e| -> DispatchResult { + .map(|e| -> TryRuntimeResult { let individual = e.others.iter().filter(|e| e.who == nominator).collect::>(); let len = individual.len(); @@ -1822,7 +1824,7 @@ impl Pallet { Ok(()) } - fn ensure_ledger_consistent(ctrl: T::AccountId) -> DispatchResult { + fn ensure_ledger_consistent(ctrl: T::AccountId) -> TryRuntimeResult { // ensures ledger.total == ledger.active + sum(ledger.unlocking). let ledger = Self::ledger(ctrl.clone()).ok_or("Not a controller.")?; let real_total: BalanceOf = diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 86afd10b5d4d0..85d1f500713f4 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -824,7 +824,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(n: BlockNumberFor) -> DispatchResult { + fn try_state(n: BlockNumberFor) -> sp_runtime::TryRuntimeResult { Self::do_try_state(n) } } diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 51cf7bebb9f7c..7744abe607ce5 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -42,7 +42,7 @@ use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; #[cfg(feature = "try-runtime")] -use crate::dispatch::{DispatchError, DispatchResult}; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index e92f68537deb8..54968c5af41bb 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -18,7 +18,7 @@ use crate::pallet::Def; #[cfg(feature = "try-runtime")] -use crate::dispatch::{DispatchError, DispatchResult}; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 5b77857dbb75b..e7f03172f2346 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2095,7 +2095,7 @@ macro_rules! decl_module { fn try_state( _: <$trait_instance as $system::Config>::BlockNumber, _: $crate::traits::TryStateSelect, - ) -> $crate::dispatch::DispatchResult { + ) -> $crate::sp_runtime::TryRuntimeResult { let pallet_name = << $trait_instance as @@ -2142,12 +2142,12 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, DispatchError> { + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, TryRuntimeError> { Ok($crate::sp_std::vec::Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> DispatchResult { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> TryRuntimeResult { Ok(()) } } @@ -2180,12 +2180,12 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, $crate::dispatch::DispatchError> { + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, $crate::sp_runtime::TryRuntimeError> { Ok($crate::sp_std::vec::Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> $crate::dispatch::DispatchResult { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> $crate::sp_runtime::TryRuntimeResult { Ok(()) } } diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index 87a180f3d6c6e..a7d12975ac8f1 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -15,8 +15,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[cfg(feature = "try-runtime")] -use crate::dispatch::{DispatchError, DispatchResult}; #[cfg(feature = "try-runtime")] use crate::storage::unhashed::contains_prefixed_key; use crate::{ @@ -160,7 +158,7 @@ impl, DbWeight: Get> frame_support::traits } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { let hashed_prefix = twox_128(P::get().as_bytes()); match contains_prefixed_key(&hashed_prefix) { true => log::info!("Found {} keys pre-removal 👀", P::get()), @@ -173,14 +171,12 @@ impl, DbWeight: Get> frame_support::traits } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> sp_runtime::TryRuntimeResult { let hashed_prefix = twox_128(P::get().as_bytes()); match contains_prefixed_key(&hashed_prefix) { true => { log::error!("{} has keys remaining post-removal ❗", P::get()); - return Err(DispatchError::Other( - "Keys remaining post-removal, this should never happen 🚨", - )) + return Err("Keys remaining post-removal, this should never happen 🚨".into()) }, false => log::info!("No {} keys found post-removal 🎉", P::get()), }; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 6000f451a2ff1..b2bb060e4ee68 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -23,7 +23,7 @@ use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; #[cfg(feature = "try-runtime")] -use crate::dispatch::{DispatchError, DispatchResult}; +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// The block initialization trait. /// @@ -139,7 +139,7 @@ pub trait OnRuntimeUpgrade { /// Same as `on_runtime_upgrade`, but perform the optional `pre_upgrade` and `post_upgrade` as /// well. #[cfg(feature = "try-runtime")] - fn try_on_runtime_upgrade(checks: bool) -> Result { + fn try_on_runtime_upgrade(checks: bool) -> Result { let maybe_state = if checks { let _guard = frame_support::StorageNoopGuard::default(); let state = Self::pre_upgrade()?; @@ -170,7 +170,7 @@ pub trait OnRuntimeUpgrade { /// This hook must not write to any state, as it would make the main `on_runtime_upgrade` path /// inaccurate. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { Ok(Vec::new()) } @@ -185,7 +185,7 @@ pub trait OnRuntimeUpgrade { /// This hook must not write to any state, as it would make the main `on_runtime_upgrade` path /// inaccurate. #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { Ok(()) } } @@ -204,7 +204,7 @@ impl OnRuntimeUpgrade for Tuple { /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade /// hooks for tuples are a noop. #[cfg(feature = "try-runtime")] - fn try_on_runtime_upgrade(checks: bool) -> Result { + fn try_on_runtime_upgrade(checks: bool) -> Result { let mut weight = Weight::zero(); for_tuples!( #( weight = weight.saturating_add(Tuple::try_on_runtime_upgrade(checks)?); )* ); Ok(weight) @@ -280,7 +280,7 @@ pub trait Hooks { /// /// This hook should not alter any storage. #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumber) -> DispatchResult { + fn try_state(_n: BlockNumber) -> TryRuntimeResult { Ok(()) } @@ -292,7 +292,7 @@ pub trait Hooks { /// /// 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, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { Ok(Vec::new()) } @@ -304,7 +304,7 @@ pub trait Hooks { /// /// 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(_state: Vec) -> DispatchResult { + fn post_upgrade(_state: Vec) -> TryRuntimeResult { Ok(()) } @@ -386,13 +386,13 @@ mod tests { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, DispatchError> { + fn pre_upgrade() -> Result, TryRuntimeError> { Pre::mutate(|s| s.push(stringify!($name))); Ok(Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> DispatchResult { + fn post_upgrade(_: Vec) -> TryRuntimeResult { Post::mutate(|s| s.push(stringify!($name))); Ok(()) } diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index 32d5b5b619b72..d3779061b8a56 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -17,9 +17,9 @@ //! Try-runtime specific traits and types. -use crate::dispatch::DispatchResult; use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::AtLeast32BitUnsigned; +use sp_runtime::TryRuntimeResult; use sp_std::prelude::*; /// Which state tests to execute. @@ -130,7 +130,7 @@ impl core::str::FromStr for UpgradeCheckSelect { /// This hook should not alter any storage. pub trait TryState { /// Execute the state checks. - fn try_state(_: BlockNumber, _: Select) -> DispatchResult; + fn try_state(_: BlockNumber, _: Select) -> TryRuntimeResult; } #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] @@ -140,7 +140,7 @@ impl TryState DispatchResult { + fn try_state(n: BlockNumber, targets: Select) -> TryRuntimeResult { match targets { Select::None => Ok(()), Select::All => { @@ -149,7 +149,7 @@ impl TryState { - let functions: &[fn(BlockNumber, Select) -> DispatchResult] = + let functions: &[fn(BlockNumber, Select) -> TryRuntimeResult] = &[for_tuples!(#( Tuple::try_state ),*)]; let skip = n.clone() % (functions.len() as u32).into(); let skip: u32 = @@ -162,10 +162,12 @@ impl TryState { - let try_state_fns: &[(&'static str, fn(BlockNumber, Select) -> DispatchResult)] = - &[for_tuples!( - #( (::name(), Tuple::try_state) ),* - )]; + let try_state_fns: &[( + &'static str, + fn(BlockNumber, Select) -> TryRuntimeResult, + )] = &[for_tuples!( + #( (::name(), Tuple::try_state) ),* + )]; let mut result = Ok(()); pallet_names.iter().for_each(|pallet_name| { if let Some((name, try_state_fn)) = diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 0994dc21b31dd..49e302bac9dbd 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -775,6 +775,12 @@ pub type ApplyExtrinsicResult = pub type ApplyExtrinsicResultWithInfo = Result, transaction_validity::TransactionValidityError>; +/// The error type used as return type in try runtime hooks. +pub type TryRuntimeError = DispatchError; + +/// Used in try runtime hooks whenever there is no return value. +pub type TryRuntimeResult = Result<(), TryRuntimeError>; + /// Verify a signature on an encoded value in a lazy manner. This can be /// an optimization if the signature scheme has an "unsigned" escape hash. pub fn verify_encoded_lazy( diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index ff5426e3a9bc4..759015bf79ab5 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -122,10 +122,10 @@ //! //! ```ignore //! #[cfg(feature = "try-runtime")] -//! fn pre_upgrade() -> Result, DispatchError> {} +//! fn pre_upgrade() -> Result, TryRuntimeError> {} //! //! #[cfg(feature = "try-runtime")] -//! fn post_upgrade(state: Vec) -> DispatchResult {} +//! fn post_upgrade(state: Vec) -> TryRuntimeResult {} //! ``` //! //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). @@ -141,7 +141,7 @@ //! //! ```ignore //! #[cfg(feature = "try-runtime")] -//! fn try_state(_: BlockNumber) -> DispatchResult {} +//! fn try_state(_: BlockNumber) -> TryRuntimeResult {} //! ``` //! //! which is called on numerous code paths in the try-runtime tool. These checks should ensure that From bf4663c3724280c673d492b6341714be432809aa Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Tue, 2 May 2023 11:20:08 +0200 Subject: [PATCH 25/31] fixes --- frame/assets/src/migration.rs | 2 ++ frame/bags-list/src/lib.rs | 1 - frame/contracts/src/migration.rs | 5 +++- frame/democracy/src/migrations.rs | 5 ++-- frame/elections-phragmen/src/lib.rs | 24 +++++++------------ frame/fast-unstake/src/lib.rs | 5 +++- frame/multisig/src/migrations.rs | 6 ++--- frame/nfts/src/migration.rs | 2 ++ frame/nomination-pools/src/lib.rs | 2 +- frame/referenda/src/migration.rs | 1 + frame/scheduler/src/migration.rs | 2 ++ frame/staking/src/pallet/impls.rs | 6 ++--- .../procedural/src/pallet/expand/hooks.rs | 3 --- 13 files changed, 31 insertions(+), 33 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 6ec8c290e890f..f82a254fa3349 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -17,6 +17,8 @@ use super::*; use frame_support::{log, traits::OnRuntimeUpgrade}; + +#[cfg(feature = "try-runtime")] use sp_runtime::{TryRuntimeError, TryRuntimeResult}; pub mod v1 { diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 71faf79f34c1c..441ab77558cc4 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -55,7 +55,6 @@ use codec::FullCodec; use frame_election_provider_support::{ScoreProvider, SortedListProvider}; -use frame_support::dispatch::{DispatchError, DispatchResult}; use frame_system::ensure_signed; use sp_runtime::traits::{AtLeast32BitUnsigned, Bounded, StaticLookup}; use sp_std::prelude::*; diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index 45b4721cc2343..a058412168988 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -25,9 +25,12 @@ use frame_support::{ traits::{Get, OnRuntimeUpgrade}, Identity, Twox64Concat, }; -use sp_runtime::{traits::Saturating, TryRuntimeError, TryRuntimeResult}; +use sp_runtime::traits::Saturating; use sp_std::{marker::PhantomData, prelude::*}; +#[cfg(feature = "try-runtime")] +use sp_runtime::{TryRuntimeError, TryRuntimeResult}; + /// Performs all necessary migrations based on `StorageVersion`. pub struct Migration(PhantomData); impl OnRuntimeUpgrade for Migration { diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index 4b3584ef199b0..8171f7f7edc76 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -20,7 +20,6 @@ use super::*; use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade, BoundedVec}; use sp_core::H256; -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// The log target. const TARGET: &'static str = "runtime::democracy::migration::v1"; @@ -62,7 +61,7 @@ pub mod v1 { impl> OnRuntimeUpgrade for Migration { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, TryRuntimeError> { + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { ensure!(StorageVersion::get::>() == 0, "can only upgrade from version 0"); let props_count = v0::PublicProps::::get().len(); @@ -134,7 +133,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> TryRuntimeResult { + fn post_upgrade(state: Vec) -> sp_runtime::TryRuntimeResult { ensure!(StorageVersion::get::>() == 1, "must upgrade"); let (old_props_count, old_ref_count): (u32, u32) = diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 7fe851d769ec8..962177f55c05c 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1220,9 +1220,7 @@ impl Pallet { if Members::::get() == members { Ok(()) } else { - Err(DispatchError::Other( - "try_state checks: Members must be always sorted by account ID", - )) + Err("try_state checks: Members must be always sorted by account ID".into()) } } @@ -1236,9 +1234,8 @@ impl Pallet { if RunnersUp::::get() == sorted { Ok(()) } else { - Err(DispatchError::Other( - "try_state checks: Runners Up must always be sorted by stake (worst to best)", - )) + Err("try_state checks: Runners Up must always be sorted by stake (worst to best)" + .into()) } } @@ -1251,18 +1248,14 @@ impl Pallet { if Candidates::::get() == candidates { Ok(()) } else { - Err(DispatchError::Other( - "try_state checks: Candidates must be always sorted by account ID", - )) + Err("try_state checks: Candidates must be always sorted by account ID".into()) } } // [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Candidates and runners-ups sets are disjoint. fn try_state_candidates_runners_up_disjoint() -> TryRuntimeResult { match Self::intersects(&Self::candidates_ids(), &Self::runners_up_ids()) { - true => Err(DispatchError::Other( - "Candidates and runners up sets should always be disjoint", - )), + true => Err("Candidates and runners up sets should always be disjoint".into()), false => Ok(()), } } @@ -1274,9 +1267,8 @@ impl Pallet { match Self::intersects(&Pallet::::members_ids(), &Self::candidates_ids()) && Self::intersects(&Pallet::::members_ids(), &Self::runners_up_ids()) { - true => Err(DispatchError::Other( - "Members set should be disjoint from candidates and runners-up sets", - )), + true => + Err("Members set should be disjoint from candidates and runners-up sets".into()), false => Ok(()), } } @@ -1291,7 +1283,7 @@ impl Pallet { .all(|s| s.stake != BalanceOf::::zero()) { true => Ok(()), - false => Err(DispatchError::Other("Members and RunnersUp must have approval stake")), + false => Err("Members and RunnersUp must have approval stake".into()), } } diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index cedae115541aa..4fc56fa17b134 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -86,11 +86,14 @@ pub mod pallet { traits::{Defensive, ReservableCurrency, StorageVersion}, }; use frame_system::pallet_prelude::*; - use sp_runtime::{traits::Zero, DispatchResult, TryRuntimeResult}; + use sp_runtime::{traits::Zero, DispatchResult}; use sp_staking::{EraIndex, StakingInterface}; use sp_std::{prelude::*, vec::Vec}; pub use weights::WeightInfo; + #[cfg(feature = "try-runtime")] + use sp_runtime::TryRuntimeResult; + #[derive(scale_info::TypeInfo, codec::Encode, codec::Decode, codec::MaxEncodedLen)] #[codec(mel_bound(T: Config))] #[scale_info(skip_type_params(T))] diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index 980892edf2388..dfb52edc9713c 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -24,8 +24,6 @@ use frame_support::{ Identity, }; -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; - #[cfg(feature = "try-runtime")] use frame_support::ensure; @@ -45,7 +43,7 @@ pub mod v1 { pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, TryRuntimeError> { + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 1, "this migration can be deleted"); @@ -74,7 +72,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> sp_runtime::TryRuntimeResult { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 2, "this migration needs to be removed"); ensure!(onchain == 1, "this migration needs to be run"); diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index 6123ff3430024..986584157d2a3 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -17,6 +17,8 @@ use super::*; use frame_support::{log, traits::OnRuntimeUpgrade}; + +#[cfg(feature = "try-runtime")] use sp_runtime::{TryRuntimeError, TryRuntimeResult}; pub mod v1 { diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 752dd22f2fe30..6a6710af6d43f 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -378,7 +378,7 @@ use sp_runtime::{ use sp_staking::{EraIndex, OnStakerSlash, StakingInterface}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, ops::Div, vec::Vec}; -#[cfg(feature = "try-runtime")] +#[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] use sp_runtime::TryRuntimeResult; /// The log target of this pallet. diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index ddbaf9dc1ceef..07c42fa6cfda7 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -22,6 +22,7 @@ use codec::{Decode, Encode, EncodeLike, MaxEncodedLen}; use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade}; use log; +#[cfg(feature = "try-runtime")] use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// Initial version of storage types. diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 3f49a0e4821de..1482cd0238a9f 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -19,6 +19,8 @@ use super::*; use frame_support::traits::OnRuntimeUpgrade; + +#[cfg(feature = "try-runtime")] use sp_runtime::{TryRuntimeError, TryRuntimeResult}; /// The log target. diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index c996baedb0830..38817fd619121 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1805,9 +1805,9 @@ impl Pallet { 0 => { /* not supporting this validator at all. */ }, 1 => sum += individual[0].value, _ => - return Err(DispatchError::Other( - "nominator cannot back a validator more than once.", - )), + return Err( + "nominator cannot back a validator more than once.".into() + ), }; Ok(()) }) diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 54968c5af41bb..1e96b0aea93e5 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -17,9 +17,6 @@ use crate::pallet::Def; -#[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; - /// * implement the individual traits using the Hooks trait pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let (where_clause, span, has_runtime_upgrade) = match def.hooks.as_ref() { From f63ee5957c010566bfa70ce04c122df8f20268d2 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Tue, 2 May 2023 11:30:02 +0200 Subject: [PATCH 26/31] fix --- frame/staking/src/pallet/impls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 38817fd619121..89b87918b4da5 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -53,7 +53,7 @@ use super::{pallet::*, STAKING_ID}; #[cfg(feature = "try-runtime")] use frame_support::ensure; -#[cfg(feature = "try-runtime")] +#[cfg(any(test, feature = "try-runtime"))] use sp_runtime::TryRuntimeResult; /// The maximum number of iterations that we do whilst iterating over `T::VoterList` in From c0a042002288585dd2708bde26ab13764ed5f7a1 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 13 May 2023 13:19:05 +0200 Subject: [PATCH 27/31] removed TryRuntimeResult & made some fixes --- frame/assets/src/migration.rs | 6 +- frame/bags-list/src/lib.rs | 8 +- frame/bags-list/src/list/mod.rs | 8 +- frame/bags-list/src/migrations.rs | 4 +- frame/collective/src/lib.rs | 86 ++++++++++--------- frame/contracts/src/migration.rs | 10 +-- frame/democracy/src/migrations.rs | 2 +- .../election-provider-multi-phase/src/lib.rs | 12 +-- frame/election-provider-support/src/lib.rs | 4 +- frame/elections-phragmen/src/lib.rs | 18 ++-- frame/fast-unstake/src/lib.rs | 4 +- frame/fast-unstake/src/migrations.rs | 4 +- frame/multisig/src/migrations.rs | 2 +- frame/nfts/src/migration.rs | 4 +- frame/nomination-pools/src/lib.rs | 12 +-- frame/nomination-pools/src/migration.rs | 16 ++-- frame/offences/src/migration.rs | 4 +- frame/referenda/src/migration.rs | 4 +- frame/scheduler/src/migration.rs | 6 +- frame/staking/src/migrations.rs | 10 +-- frame/staking/src/pallet/impls.rs | 24 +++--- frame/staking/src/pallet/mod.rs | 2 +- frame/support/procedural/src/lib.rs | 2 +- .../procedural/src/pallet/expand/hooks.rs | 4 +- frame/support/src/dispatch.rs | 8 +- frame/support/src/migrations.rs | 2 +- frame/support/src/traits/hooks.rs | 14 +-- frame/support/src/traits/try_runtime.rs | 10 +-- primitives/runtime/src/lib.rs | 3 - utils/frame/try-runtime/cli/src/lib.rs | 4 +- 30 files changed, 151 insertions(+), 146 deletions(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index f82a254fa3349..796c8ba37fcbb 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -19,7 +19,7 @@ use super::*; use frame_support::{log, traits::OnRuntimeUpgrade}; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; pub mod v1 { use frame_support::{pallet_prelude::*, weights::Weight}; @@ -105,7 +105,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> TryRuntimeResult { + fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); @@ -124,7 +124,7 @@ pub mod v1 { "after migration, the current_version and onchain_version should be the same" ); - Asset::::iter().try_for_each(|(_id, asset)| -> TryRuntimeResult { + Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state"); Ok(()) })?; diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 8f86cb3f216a0..156c52cc87c45 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -60,7 +60,7 @@ use sp_runtime::traits::{AtLeast32BitUnsigned, Bounded, StaticLookup}; use sp_std::prelude::*; #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; #[cfg(any(feature = "runtime-benchmarks", test))] mod benchmarks; @@ -270,7 +270,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_: BlockNumberFor) -> TryRuntimeResult { + fn try_state(_: BlockNumberFor) -> Result<(), TryRuntimeError> { >::try_state() } } @@ -278,7 +278,7 @@ pub mod pallet { #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] impl, I: 'static> Pallet { - pub fn do_try_state() -> TryRuntimeResult { + pub fn do_try_state() -> Result<(), TryRuntimeError> { List::::do_try_state() } } @@ -358,7 +358,7 @@ impl, I: 'static> SortedListProvider for Pallet } #[cfg(feature = "try-runtime")] - fn try_state() -> TryRuntimeResult { + fn try_state() -> Result<(), TryRuntimeError> { Self::do_try_state() } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index bb6734a927c57..d8626080e2523 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -43,7 +43,7 @@ use sp_std::{ }; #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; #[derive(Debug, PartialEq, Eq, Encode, Decode, MaxEncodedLen, TypeInfo, PalletError)] pub enum ListError { @@ -515,7 +515,7 @@ impl, I: 'static> List { /// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure /// all bags and nodes are checked per *any* update to `List`. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - pub(crate) fn do_try_state() -> TryRuntimeResult { + pub(crate) fn do_try_state() -> Result<(), TryRuntimeError> { let mut seen_in_list = BTreeSet::new(); ensure!( Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)), @@ -753,7 +753,7 @@ impl, I: 'static> Bag { /// * Ensures tail has no next. /// * Ensures there are no loops, traversal from head to tail is correct. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - fn do_try_state(&self) -> TryRuntimeResult { + fn do_try_state(&self) -> Result<(), TryRuntimeError> { frame_support::ensure!( self.head() .map(|head| head.prev().is_none()) @@ -898,7 +898,7 @@ impl, I: 'static> Node { } #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - fn do_try_state(&self) -> TryRuntimeResult { + fn do_try_state(&self) -> Result<(), TryRuntimeError> { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; let id = self.id(); diff --git a/frame/bags-list/src/migrations.rs b/frame/bags-list/src/migrations.rs index ae50ea257f4df..7df63a6a44c54 100644 --- a/frame/bags-list/src/migrations.rs +++ b/frame/bags-list/src/migrations.rs @@ -25,7 +25,7 @@ use frame_support::traits::OnRuntimeUpgrade; #[cfg(feature = "try-runtime")] use frame_support::ensure; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; @@ -122,7 +122,7 @@ impl, I: 'static> OnRuntimeUpgrade for AddScore { } #[cfg(feature = "try-runtime")] - fn post_upgrade(node_count_before: Vec) -> TryRuntimeResult { + fn post_upgrade(node_count_before: Vec) -> Result<(), TryRuntimeError> { let node_count_before: u32 = Decode::decode(&mut node_count_before.as_slice()) .expect("the state parameter should be something that was generated by pre_upgrade"); // Now the list node data is not corrupt anymore. diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 64802193b3ab6..68b4f8091caf1 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -61,7 +61,7 @@ use frame_support::{ }; #[cfg(any(feature = "try-runtime", test))] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; #[cfg(test)] mod tests; @@ -355,7 +355,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> TryRuntimeResult { + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -975,14 +975,16 @@ impl, I: 'static> Pallet { /// Looking at prime account: /// * The prime account must be a member of the collective. #[cfg(any(feature = "try-runtime", test))] - fn do_try_state() -> TryRuntimeResult { - Self::proposals().into_iter().try_for_each(|proposal| -> TryRuntimeResult { - ensure!( - Self::proposal_of(proposal).is_some(), - "Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping." - ); - Ok(()) - })?; + fn do_try_state() -> Result<(), TryRuntimeError> { + Self::proposals() + .into_iter() + .try_for_each(|proposal| -> Result<(), TryRuntimeError> { + ensure!( + Self::proposal_of(proposal).is_some(), + "Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping." + ); + Ok(()) + })?; ensure!( Self::proposals().into_iter().count() <= Self::proposal_count() as usize, @@ -993,39 +995,45 @@ impl, I: 'static> Pallet { "Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`" ); - Self::proposals().into_iter().try_for_each(|proposal| -> TryRuntimeResult { - if let Some(votes) = Self::voting(proposal) { - let ayes = votes.ayes.len(); - let nays = votes.nays.len(); - - ensure!( - ayes.saturating_add(nays) <= T::MaxMembers::get() as usize, - "The sum of ayes and nays is greater than `MaxMembers`" - ); - } - Ok(()) - })?; + Self::proposals() + .into_iter() + .try_for_each(|proposal| -> Result<(), TryRuntimeError> { + if let Some(votes) = Self::voting(proposal) { + let ayes = votes.ayes.len(); + let nays = votes.nays.len(); + + ensure!( + ayes.saturating_add(nays) <= T::MaxMembers::get() as usize, + "The sum of ayes and nays is greater than `MaxMembers`" + ); + } + Ok(()) + })?; let mut proposal_indices = vec![]; - Self::proposals().into_iter().try_for_each(|proposal| -> TryRuntimeResult { - if let Some(votes) = Self::voting(proposal) { - let proposal_index = votes.index; + Self::proposals() + .into_iter() + .try_for_each(|proposal| -> Result<(), TryRuntimeError> { + if let Some(votes) = Self::voting(proposal) { + let proposal_index = votes.index; + ensure!( + !proposal_indices.contains(&proposal_index), + "The proposal index is not unique." + ); + proposal_indices.push(proposal_index); + } + Ok(()) + })?; + + >::iter_keys().try_for_each( + |proposal_hash| -> Result<(), TryRuntimeError> { ensure!( - !proposal_indices.contains(&proposal_index), - "The proposal index is not unique." + Self::proposals().contains(&proposal_hash), + "`Proposals` doesn't contain the proposal hash from the `Voting` storage map." ); - proposal_indices.push(proposal_index); - } - Ok(()) - })?; - - >::iter_keys().try_for_each(|proposal_hash| -> TryRuntimeResult { - ensure!( - Self::proposals().contains(&proposal_hash), - "`Proposals` doesn't contain the proposal hash from the `Voting` storage map." - ); - Ok(()) - })?; + Ok(()) + }, + )?; ensure!( Self::members().len() <= T::MaxMembers::get() as usize, diff --git a/frame/contracts/src/migration.rs b/frame/contracts/src/migration.rs index a058412168988..0b7c094ddb176 100644 --- a/frame/contracts/src/migration.rs +++ b/frame/contracts/src/migration.rs @@ -29,7 +29,7 @@ use sp_runtime::traits::Saturating; use sp_std::{marker::PhantomData, prelude::*}; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; /// Performs all necessary migrations based on `StorageVersion`. pub struct Migration(PhantomData); @@ -80,7 +80,7 @@ impl OnRuntimeUpgrade for Migration { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> TryRuntimeResult { + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { let version = Decode::decode(&mut state.as_ref()).map_err(|_| "Cannot decode version")?; post_checks::post_upgrade::(version) } @@ -421,7 +421,7 @@ mod post_checks { type ContractInfoOf = StorageMap, Twox64Concat, ::AccountId, V>; - pub fn post_upgrade(old_version: StorageVersion) -> TryRuntimeResult { + pub fn post_upgrade(old_version: StorageVersion) -> Result<(), TryRuntimeError> { if old_version < 7 { return Ok(()) } @@ -437,7 +437,7 @@ mod post_checks { Ok(()) } - fn v8() -> TryRuntimeResult { + fn v8() -> Result<(), TryRuntimeError> { use frame_support::traits::ReservableCurrency; for (key, value) in ContractInfoOf::>::iter() { let reserved = T::Currency::reserved_balance(&key); @@ -464,7 +464,7 @@ mod post_checks { Ok(()) } - fn v9() -> TryRuntimeResult { + fn v9() -> Result<(), TryRuntimeError> { for value in CodeStorage::::iter_values() { ensure!( value.determinism == Determinism::Enforced, diff --git a/frame/democracy/src/migrations.rs b/frame/democracy/src/migrations.rs index 8171f7f7edc76..397f42dce7210 100644 --- a/frame/democracy/src/migrations.rs +++ b/frame/democracy/src/migrations.rs @@ -133,7 +133,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> sp_runtime::TryRuntimeResult { + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { ensure!(StorageVersion::get::>() == 1, "must upgrade"); let (old_props_count, old_ref_count): (u32, u32) = diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 9a960ee338eda..98fa880349eeb 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -258,7 +258,7 @@ use sp_runtime::{ use sp_std::prelude::*; #[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; @@ -886,7 +886,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> TryRuntimeResult { + fn try_state(_n: T::BlockNumber) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -1582,7 +1582,7 @@ impl Pallet { #[cfg(feature = "try-runtime")] impl Pallet { - fn do_try_state() -> TryRuntimeResult { + fn do_try_state() -> Result<(), TryRuntimeError> { Self::try_state_snapshot()?; Self::try_state_signed_submissions_map()?; Self::try_state_phase_off() @@ -1591,7 +1591,7 @@ impl Pallet { // [`Snapshot`] state check. Invariants: // - [`DesiredTargets`] exists if and only if [`Snapshot`] is present. // - [`SnapshotMetadata`] exist if and only if [`Snapshot`] is present. - fn try_state_snapshot() -> TryRuntimeResult { + fn try_state_snapshot() -> Result<(), TryRuntimeError> { if >::exists() && >::exists() && >::exists() @@ -1611,7 +1611,7 @@ impl Pallet { // - All [`SignedSubmissionIndices`] are present in [`SignedSubmissionsMap`], and no more; // - [`SignedSubmissionNextIndex`] is not present in [`SignedSubmissionsMap`]; // - [`SignedSubmissionIndices`] is sorted by election score. - fn try_state_signed_submissions_map() -> TryRuntimeResult { + fn try_state_signed_submissions_map() -> Result<(), TryRuntimeError> { let mut last_score: ElectionScore = Default::default(); let indices = >::get(); @@ -1657,7 +1657,7 @@ impl Pallet { // [`Phase::Off`] state check. Invariants: // - If phase is `Phase::Off`, [`Snapshot`] must be none. - fn try_state_phase_off() -> TryRuntimeResult { + fn try_state_phase_off() -> Result<(), TryRuntimeError> { match Self::current_phase().is_off() { false => Ok(()), true => diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index ad904d58c6234..6e6d3341bfc8a 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -190,7 +190,7 @@ pub use sp_npos_elections::{ pub use traits::NposSolution; #[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; // re-export for the solution macro, with the dependencies of the macro. #[doc(hidden)] @@ -567,7 +567,7 @@ pub trait SortedListProvider { /// Check internal state of the list. Only meant for debugging. #[cfg(feature = "try-runtime")] - fn try_state() -> TryRuntimeResult; + fn try_state() -> Result<(), TryRuntimeError>; /// If `who` changes by the returned amount they are guaranteed to have a worst case change /// in their list position. diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 962177f55c05c..1939cdbb6bc25 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -116,7 +116,7 @@ use sp_runtime::{ use sp_std::{cmp::Ordering, prelude::*}; #[cfg(any(feature = "try-runtime", test))] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; mod benchmarking; pub mod weights; @@ -330,7 +330,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> TryRuntimeResult { + fn try_state(_n: T::BlockNumber) -> Result<(), TryRuntimeError> { Self::do_try_state() } } @@ -1202,7 +1202,7 @@ impl ContainsLengthBound for Pallet { #[cfg(any(feature = "try-runtime", test))] impl Pallet { - fn do_try_state() -> TryRuntimeResult { + fn do_try_state() -> Result<(), TryRuntimeError> { Self::try_state_members()?; Self::try_state_runners_up()?; Self::try_state_candidates()?; @@ -1213,7 +1213,7 @@ impl Pallet { /// [`Members`] state checks. Invariants: /// - Members are always sorted based on account ID. - fn try_state_members() -> TryRuntimeResult { + fn try_state_members() -> Result<(), TryRuntimeError> { let mut members = Members::::get().clone(); members.sort_by_key(|m| m.who.clone()); @@ -1226,7 +1226,7 @@ impl Pallet { // [`RunnersUp`] state checks. Invariants: // - Elements are sorted based on weight (worst to best). - fn try_state_runners_up() -> TryRuntimeResult { + fn try_state_runners_up() -> Result<(), TryRuntimeError> { let mut sorted = RunnersUp::::get(); // worst stake first sorted.sort_by(|a, b| a.stake.cmp(&b.stake)); @@ -1241,7 +1241,7 @@ impl Pallet { // [`Candidates`] state checks. Invariants: // - Always sorted based on account ID. - fn try_state_candidates() -> TryRuntimeResult { + fn try_state_candidates() -> Result<(), TryRuntimeError> { let mut candidates = Candidates::::get().clone(); candidates.sort_by_key(|(c, _)| c.clone()); @@ -1253,7 +1253,7 @@ impl Pallet { } // [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Candidates and runners-ups sets are disjoint. - fn try_state_candidates_runners_up_disjoint() -> TryRuntimeResult { + fn try_state_candidates_runners_up_disjoint() -> Result<(), TryRuntimeError> { match Self::intersects(&Self::candidates_ids(), &Self::runners_up_ids()) { true => Err("Candidates and runners up sets should always be disjoint".into()), false => Ok(()), @@ -1263,7 +1263,7 @@ impl Pallet { // [`Members`], [`Candidates`] and [`RunnersUp`] state checks. Invariants: // - Members and candidates sets are disjoint; // - Members and runners-ups sets are disjoint. - fn try_state_members_disjoint() -> TryRuntimeResult { + fn try_state_members_disjoint() -> Result<(), TryRuntimeError> { match Self::intersects(&Pallet::::members_ids(), &Self::candidates_ids()) && Self::intersects(&Pallet::::members_ids(), &Self::runners_up_ids()) { @@ -1276,7 +1276,7 @@ impl Pallet { // [`Members`], [`RunnersUp`] and approval stake state checks. Invariants: // - Selected members should have approval stake; // - Selected RunnersUp should have approval stake. - fn try_state_members_approval_stake() -> TryRuntimeResult { + fn try_state_members_approval_stake() -> Result<(), TryRuntimeError> { match Members::::get() .iter() .chain(RunnersUp::::get().iter()) diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 4fc56fa17b134..f8a8e00346a34 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -92,7 +92,7 @@ pub mod pallet { pub use weights::WeightInfo; #[cfg(feature = "try-runtime")] - use sp_runtime::TryRuntimeResult; + use sp_runtime::TryRuntimeError; #[derive(scale_info::TypeInfo, codec::Encode, codec::Decode, codec::MaxEncodedLen)] #[codec(mel_bound(T: Config))] @@ -231,7 +231,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(_n: T::BlockNumber) -> TryRuntimeResult { + fn try_state(_n: T::BlockNumber) -> Result<(), TryRuntimeError> { // ensure that the value of `ErasToCheckPerBlock` is less than // `T::MaxErasToCheckPerBlock`. ensure!( diff --git a/frame/fast-unstake/src/migrations.rs b/frame/fast-unstake/src/migrations.rs index 9ae0c172b0169..564388407045e 100644 --- a/frame/fast-unstake/src/migrations.rs +++ b/frame/fast-unstake/src/migrations.rs @@ -28,7 +28,7 @@ pub mod v1 { #[cfg(feature = "try-runtime")] use frame_support::ensure; #[cfg(feature = "try-runtime")] - use sp_runtime::{TryRuntimeError, TryRuntimeResult}; + use sp_runtime::TryRuntimeError; pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { @@ -79,7 +79,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> TryRuntimeResult { + fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { ensure!( Pallet::::on_chain_storage_version() == 1, "The onchain version must be updated after the migration." diff --git a/frame/multisig/src/migrations.rs b/frame/multisig/src/migrations.rs index dfb52edc9713c..298e73c5d7576 100644 --- a/frame/multisig/src/migrations.rs +++ b/frame/multisig/src/migrations.rs @@ -72,7 +72,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> sp_runtime::TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain < 2, "this migration needs to be removed"); ensure!(onchain == 1, "this migration needs to be run"); diff --git a/frame/nfts/src/migration.rs b/frame/nfts/src/migration.rs index 986584157d2a3..ba14492be84ca 100644 --- a/frame/nfts/src/migration.rs +++ b/frame/nfts/src/migration.rs @@ -19,7 +19,7 @@ use super::*; use frame_support::{log, traits::OnRuntimeUpgrade}; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; pub mod v1 { use frame_support::{pallet_prelude::*, weights::Weight}; @@ -102,7 +102,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> TryRuntimeResult { + fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 6a6710af6d43f..ecba0b12fa619 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -379,7 +379,7 @@ use sp_staking::{EraIndex, OnStakerSlash, StakingInterface}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, ops::Div, vec::Vec}; #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; /// The log target of this pallet. pub const LOG_TARGET: &str = "runtime::nomination-pools"; @@ -2630,7 +2630,7 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumberFor) -> TryRuntimeResult { + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { Self::do_try_state(u8::MAX) } @@ -3059,7 +3059,7 @@ impl Pallet { /// multiple `level`s, where the higher the level, the more checks we performs. So, /// `try_state(255)` is the strongest sanity check, and `0` performs no checks. #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] - pub fn do_try_state(level: u8) -> TryRuntimeResult { + pub fn do_try_state(level: u8) -> Result<(), TryRuntimeError> { if level.is_zero() { return Ok(()) } @@ -3104,7 +3104,7 @@ impl Pallet { let mut pools_members = BTreeMap::::new(); let mut pools_members_pending_rewards = BTreeMap::>::new(); let mut all_members = 0u32; - PoolMembers::::iter().try_for_each(|(_, d)| -> TryRuntimeResult { + PoolMembers::::iter().try_for_each(|(_, d)| -> Result<(), TryRuntimeError> { let bonded_pool = BondedPools::::get(d.pool_id).unwrap(); ensure!(!d.total_points().is_zero(), "No member should have zero points"); *pools_members.entry(d.pool_id).or_default() += 1; @@ -3123,7 +3123,7 @@ impl Pallet { Ok(()) })?; - RewardPools::::iter_keys().try_for_each(|id| -> TryRuntimeResult { + RewardPools::::iter_keys().try_for_each(|id| -> Result<(), TryRuntimeError> { // the sum of the pending rewards must be less than the leftover balance. Since the // reward math rounds down, we might accumulate some dust here. log!( @@ -3141,7 +3141,7 @@ impl Pallet { Ok(()) })?; - BondedPools::::iter().try_for_each(|(id, inner)| -> TryRuntimeResult { + BondedPools::::iter().try_for_each(|(id, inner)| -> Result<(), TryRuntimeError> { let bonded_pool = BondedPool { id, inner }; ensure!( pools_members.get(&id).copied().unwrap_or_default() == diff --git a/frame/nomination-pools/src/migration.rs b/frame/nomination-pools/src/migration.rs index cde7fb09f6558..2c84e4e6d3c8e 100644 --- a/frame/nomination-pools/src/migration.rs +++ b/frame/nomination-pools/src/migration.rs @@ -21,7 +21,7 @@ use frame_support::traits::OnRuntimeUpgrade; use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; pub mod v1 { use super::*; @@ -103,7 +103,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> TryRuntimeResult { + fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { // new version must be set. ensure!( Pallet::::on_chain_storage_version() == 1, @@ -360,7 +360,7 @@ pub mod v2 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { // all reward accounts must have more than ED. - RewardPools::::iter().try_for_each(|(id, _)| -> TryRuntimeResult { + RewardPools::::iter().try_for_each(|(id, _)| -> Result<(), TryRuntimeError> { ensure!( T::Currency::free_balance(&Pallet::::create_reward_account(id)) >= T::Currency::minimum_balance(), @@ -373,7 +373,7 @@ pub mod v2 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> TryRuntimeResult { + fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { // new version must be set. ensure!( Pallet::::on_chain_storage_version() == 2, @@ -392,7 +392,7 @@ pub mod v2 { // all reward pools must have exactly ED in them. This means no reward can be claimed, // and that setting reward counters all over the board to zero will work henceforth. - RewardPools::::iter().try_for_each(|(id, _)| -> TryRuntimeResult { + RewardPools::::iter().try_for_each(|(id, _)| -> Result<(), TryRuntimeError> { ensure!( RewardPool::::current_balance(id) == Zero::zero(), "Reward pool balance must be zero.", @@ -459,7 +459,7 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> TryRuntimeResult { + fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { ensure!( Metadata::::iter_keys().all(|id| BondedPools::::contains_key(&id)), "not all of the stale metadata has been removed" @@ -559,7 +559,7 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> TryRuntimeResult { + fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { // ensure all BondedPools items now contain an `inner.commission: Commission` field. ensure!( BondedPools::::iter().all(|(_, inner)| inner.commission.current.is_none() && @@ -669,7 +669,7 @@ pub mod v5 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(data: Vec) -> TryRuntimeResult { + fn post_upgrade(data: Vec) -> Result<(), TryRuntimeError> { let old_rpool_values: u64 = Decode::decode(&mut &data[..]).unwrap(); let rpool_keys = RewardPools::::iter_keys().count() as u64; let rpool_values = RewardPools::::iter_values().count() as u64; diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index 6cc72c220b7a8..14dbd606dfe87 100644 --- a/frame/offences/src/migration.rs +++ b/frame/offences/src/migration.rs @@ -30,7 +30,7 @@ use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] use frame_support::ensure; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; mod v0 { use super::*; @@ -83,7 +83,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { let onchain = Pallet::::on_chain_storage_version(); ensure!(onchain == 1, "pallet_offences::MigrateToV1 needs to be run"); ensure!( diff --git a/frame/referenda/src/migration.rs b/frame/referenda/src/migration.rs index 07c42fa6cfda7..6f796ca40d9be 100644 --- a/frame/referenda/src/migration.rs +++ b/frame/referenda/src/migration.rs @@ -23,7 +23,7 @@ use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade}; use log; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; /// Initial version of storage types. pub mod v0 { @@ -150,7 +150,7 @@ pub mod v1 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> TryRuntimeResult { + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { let onchain_version = Pallet::::on_chain_storage_version(); ensure!(onchain_version == 1, "must upgrade from version 0 to 1."); let pre_referendum_count: u32 = Decode::decode(&mut &state[..]) diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 1482cd0238a9f..52820d6a32e83 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -21,7 +21,7 @@ use super::*; use frame_support::traits::OnRuntimeUpgrade; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; /// The log target. const TARGET: &'static str = "runtime::scheduler::migration"; @@ -172,7 +172,7 @@ pub mod v3 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> TryRuntimeResult { + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { ensure!(StorageVersion::get::>() == 4, "Must upgrade"); // Check that everything decoded fine. @@ -288,7 +288,7 @@ pub mod v4 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> TryRuntimeResult { + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { ensure!(StorageVersion::get::>() == 4, "Version must not change"); let (old_agendas, non_empty_agendas): (u32, u32) = diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 282ee9a6e30e4..0a27290daacd8 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -26,7 +26,7 @@ use frame_support::{ #[cfg(feature = "try-runtime")] use frame_support::ensure; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; /// Used for release versioning upto v12. /// @@ -89,7 +89,7 @@ pub mod v13 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { frame_support::ensure!( Pallet::::on_chain_storage_version() == 13, "v13 not applied" @@ -151,7 +151,7 @@ pub mod v12 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V12_0_0, "v12 not applied" @@ -222,7 +222,7 @@ pub mod v11 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { frame_support::ensure!( StorageVersion::::get() == ObsoleteReleases::V11_0_0, "wrong version after the upgrade" @@ -348,7 +348,7 @@ pub mod v9 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> TryRuntimeResult { + fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( "the state parameter should be something that was generated by pre_upgrade", ); diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index b788bb9516de6..540d7d603d078 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -54,7 +54,7 @@ use super::{pallet::*, STAKING_ID}; #[cfg(feature = "try-runtime")] use frame_support::ensure; #[cfg(any(test, feature = "try-runtime"))] -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; /// The maximum number of iterations that we do whilst iterating over `T::VoterList` in /// `get_npos_voters`. @@ -1472,7 +1472,7 @@ impl SortedListProvider for UseValidatorsMap { 0 } #[cfg(feature = "try-runtime")] - fn try_state() -> TryRuntimeResult { + fn try_state() -> Result<(), TryRuntimeError> { Ok(()) } @@ -1549,7 +1549,7 @@ impl SortedListProvider for UseNominatorsAndValidatorsM } #[cfg(feature = "try-runtime")] - fn try_state() -> TryRuntimeResult { + fn try_state() -> Result<(), TryRuntimeError> { Ok(()) } @@ -1718,7 +1718,7 @@ impl StakingInterface for Pallet { #[cfg(any(test, feature = "try-runtime"))] impl Pallet { - pub(crate) fn do_try_state(_: BlockNumberFor) -> TryRuntimeResult { + pub(crate) fn do_try_state(_: BlockNumberFor) -> Result<(), TryRuntimeError> { ensure!( T::VoterList::iter() .all(|x| >::contains_key(&x) || >::contains_key(&x)), @@ -1731,7 +1731,7 @@ impl Pallet { Self::check_count() } - fn check_count() -> TryRuntimeResult { + fn check_count() -> Result<(), TryRuntimeError> { ensure!( ::VoterList::count() == Nominators::::count() + Validators::::count(), @@ -1749,14 +1749,14 @@ impl Pallet { Ok(()) } - fn check_ledgers() -> TryRuntimeResult { + fn check_ledgers() -> Result<(), TryRuntimeError> { Bonded::::iter() .map(|(_, ctrl)| Self::ensure_ledger_consistent(ctrl)) .collect::, _>>()?; Ok(()) } - fn check_exposures() -> TryRuntimeResult { + fn check_exposures() -> Result<(), TryRuntimeError> { // a check per validator to ensure the exposure struct is always sane. let era = Self::active_era().unwrap().index; ErasStakers::::iter_prefix_values(era) @@ -1772,10 +1772,10 @@ impl Pallet { ); Ok(()) }) - .collect::() + .collect::>() } - fn check_nominators() -> TryRuntimeResult { + fn check_nominators() -> Result<(), TryRuntimeError> { // a check per nominator to ensure their entire stake is correctly distributed. Will only // kick-in if the nomination was submitted before the current era. let era = Self::active_era().unwrap().index; @@ -1789,14 +1789,14 @@ impl Pallet { } }, ) - .map(|nominator| -> TryRuntimeResult { + .map(|nominator| -> Result<(), TryRuntimeError> { // must be bonded. Self::ensure_is_stash(&nominator)?; let mut sum = BalanceOf::::zero(); T::SessionInterface::validators() .iter() .map(|v| Self::eras_stakers(era, v)) - .map(|e| -> TryRuntimeResult { + .map(|e| -> Result<(), TryRuntimeError> { let individual = e.others.iter().filter(|e| e.who == nominator).collect::>(); let len = individual.len(); @@ -1823,7 +1823,7 @@ impl Pallet { Ok(()) } - fn ensure_ledger_consistent(ctrl: T::AccountId) -> TryRuntimeResult { + fn ensure_ledger_consistent(ctrl: T::AccountId) -> Result<(), TryRuntimeError> { // ensures ledger.total == ledger.active + sum(ledger.unlocking). let ledger = Self::ledger(ctrl.clone()).ok_or("Not a controller.")?; let real_total: BalanceOf = diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 8f8e5560602c7..b048e21939139 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -823,7 +823,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn try_state(n: BlockNumberFor) -> sp_runtime::TryRuntimeResult { + fn try_state(n: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { Self::do_try_state(n) } } diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index f8a8a5bffc279..2fa986f8b8ad1 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -42,7 +42,7 @@ use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 3995882fa6241..efe51e181e055 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -105,7 +105,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { current_version, ); - return Err("On chain and current storage version do not match. Missing runtime upgrade?"); + return Err("On chain and current storage version do not match. Missing runtime upgrade?".into()); } } } else { @@ -128,7 +128,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); return Err("On chain storage version set, while the pallet doesn't \ - have the `#[pallet::storage_version(VERSION)]` attribute."); + have the `#[pallet::storage_version(VERSION)]` attribute.".into()); } } }; diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index b3cf84bb00c7f..47c7b22f5bf15 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2097,7 +2097,7 @@ macro_rules! decl_module { fn try_state( _: <$trait_instance as $system::Config>::BlockNumber, _: $crate::traits::TryStateSelect, - ) -> $crate::sp_runtime::TryRuntimeResult { + ) -> Result<(), $crate::sp_runtime::TryRuntimeError> { let pallet_name = << $trait_instance as @@ -2144,12 +2144,12 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, TryRuntimeError> { + fn pre_upgrade() -> Result<$crate::sp_std::vec::Vec, $crate::sp_runtime::TryRuntimeError> { Ok($crate::sp_std::vec::Vec::new()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> TryRuntimeResult { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), $crate::sp_runtime::TryRuntimeError> { Ok(()) } } @@ -2187,7 +2187,7 @@ macro_rules! decl_module { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: $crate::sp_std::vec::Vec) -> $crate::sp_runtime::TryRuntimeResult { + fn post_upgrade(_: $crate::sp_std::vec::Vec) -> Result<(), $crate::sp_runtime::TryRuntimeError> { Ok(()) } } diff --git a/frame/support/src/migrations.rs b/frame/support/src/migrations.rs index 8b2e9b76524ed..8bda2662a237e 100644 --- a/frame/support/src/migrations.rs +++ b/frame/support/src/migrations.rs @@ -197,7 +197,7 @@ impl, DbWeight: Get> frame_support::traits } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> sp_runtime::TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { let hashed_prefix = twox_128(P::get().as_bytes()); match contains_prefixed_key(&hashed_prefix) { true => { diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 261559047cc2e..12dcd6af0f791 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -23,7 +23,7 @@ use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; #[cfg(feature = "try-runtime")] -use sp_runtime::{TryRuntimeError, TryRuntimeResult}; +use sp_runtime::TryRuntimeError; /// The block initialization trait. /// @@ -185,7 +185,7 @@ pub trait OnRuntimeUpgrade { /// This hook must not write to any state, as it would make the main `on_runtime_upgrade` path /// inaccurate. #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { Ok(()) } } @@ -227,12 +227,12 @@ impl OnRuntimeUpgrade for Tuple { errors.iter().for_each(|err| { log::error!( target: "try-runtime", - "{}", + "{:?}", err ); }); - return Err("Detected multiple errors while executing `try_on_runtime_upgrade`, check the logs!") + return Err("Detected multiple errors while executing `try_on_runtime_upgrade`, check the logs!".into()) } Ok(weight) @@ -308,7 +308,7 @@ pub trait Hooks { /// /// This hook should not alter any storage. #[cfg(feature = "try-runtime")] - fn try_state(_n: BlockNumber) -> TryRuntimeResult { + fn try_state(_n: BlockNumber) -> Result<(), TryRuntimeError> { Ok(()) } @@ -332,7 +332,7 @@ pub trait Hooks { /// /// 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(_state: Vec) -> TryRuntimeResult { + fn post_upgrade(_state: Vec) -> Result<(), TryRuntimeError> { Ok(()) } @@ -420,7 +420,7 @@ mod tests { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_: Vec) -> TryRuntimeResult { + fn post_upgrade(_: Vec) -> Result<(), TryRuntimeError> { Post::mutate(|s| s.push(stringify!($name))); Ok(()) } diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index d3779061b8a56..cb18f9d5b71c0 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -19,7 +19,7 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::AtLeast32BitUnsigned; -use sp_runtime::TryRuntimeResult; +use sp_runtime::TryRuntimeError; use sp_std::prelude::*; /// Which state tests to execute. @@ -130,7 +130,7 @@ impl core::str::FromStr for UpgradeCheckSelect { /// This hook should not alter any storage. pub trait TryState { /// Execute the state checks. - fn try_state(_: BlockNumber, _: Select) -> TryRuntimeResult; + fn try_state(_: BlockNumber, _: Select) -> Result<(), TryRuntimeError>; } #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] @@ -140,7 +140,7 @@ impl TryState TryRuntimeResult { + fn try_state(n: BlockNumber, targets: Select) -> Result<(), TryRuntimeError> { match targets { Select::None => Ok(()), Select::All => { @@ -149,7 +149,7 @@ impl TryState { - let functions: &[fn(BlockNumber, Select) -> TryRuntimeResult] = + let functions: &[fn(BlockNumber, Select) -> Result<(), TryRuntimeError>] = &[for_tuples!(#( Tuple::try_state ),*)]; let skip = n.clone() % (functions.len() as u32).into(); let skip: u32 = @@ -164,7 +164,7 @@ impl TryState { let try_state_fns: &[( &'static str, - fn(BlockNumber, Select) -> TryRuntimeResult, + fn(BlockNumber, Select) -> Result<(), TryRuntimeError>, )] = &[for_tuples!( #( (::name(), Tuple::try_state) ),* )]; diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 8fee251541713..25e2731c1cd7d 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -785,9 +785,6 @@ pub type ApplyExtrinsicResultWithInfo = /// The error type used as return type in try runtime hooks. pub type TryRuntimeError = DispatchError; -/// Used in try runtime hooks whenever there is no return value. -pub type TryRuntimeResult = Result<(), TryRuntimeError>; - /// Verify a signature on an encoded value in a lazy manner. This can be /// an optimization if the signature scheme has an "unsigned" escape hash. pub fn verify_encoded_lazy( diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 759015bf79ab5..b9d97f8c0e802 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -125,7 +125,7 @@ //! fn pre_upgrade() -> Result, TryRuntimeError> {} //! //! #[cfg(feature = "try-runtime")] -//! fn post_upgrade(state: Vec) -> TryRuntimeResult {} +//! fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> {} //! ``` //! //! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`). @@ -141,7 +141,7 @@ //! //! ```ignore //! #[cfg(feature = "try-runtime")] -//! fn try_state(_: BlockNumber) -> TryRuntimeResult {} +//! fn try_state(_: BlockNumber) -> Result<(), TryRuntimeError> {} //! ``` //! //! which is called on numerous code paths in the try-runtime tool. These checks should ensure that From a778238f2319939b5920ad3da188c5ec22978eb1 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 13 May 2023 14:05:15 +0200 Subject: [PATCH 28/31] fix testsg --- frame/scheduler/src/migration.rs | 2 +- .../support/procedural/src/pallet/expand/hooks.rs | 6 +++--- frame/support/test/tests/pallet.rs | 14 ++++++++------ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/frame/scheduler/src/migration.rs b/frame/scheduler/src/migration.rs index 52820d6a32e83..7f23d962063bc 100644 --- a/frame/scheduler/src/migration.rs +++ b/frame/scheduler/src/migration.rs @@ -499,7 +499,7 @@ mod test { // The pre_upgrade hook fails: let err = v3::MigrateToV4::::pre_upgrade().unwrap_err(); - assert!(err.contains("Call is too large")); + assert!(err == "Call is too large".into()); // But the migration itself works: let _w = v3::MigrateToV4::::on_runtime_upgrade(); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index efe51e181e055..ef22f5a3af5be 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -211,7 +211,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<#frame_support::sp_std::vec::Vec, DispatchError> { + fn pre_upgrade() -> Result<#frame_support::sp_std::vec::Vec, #frame_support::sp_runtime::TryRuntimeError> { < Self as @@ -220,7 +220,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> DispatchResult { + fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { #post_storage_version_check < @@ -268,7 +268,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { fn try_state( n: ::BlockNumber, _s: #frame_support::traits::TryStateSelect - ) -> DispatchResult { + ) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { #log_try_state < Self as #frame_support::traits::Hooks< diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 7f15ad1f9298a..2ceec6b2dad5b 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -2110,9 +2110,10 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // Call `on_genesis` to put the storage version of `Example` into the storage. Example::on_genesis(); // The version isn't changed, we should detect it. - assert!(Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) - .unwrap_err() - .contains("On chain and current storage version do not match")); + assert!( + Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap_err() == + "On chain and current storage version do not match".into() + ); }); TestExternalities::default().execute_with(|| { @@ -2138,9 +2139,10 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // `CustomUpgradePallet4` will set a storage version for `Example4` while this doesn't has // any storage version "enabled". - assert!(ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) - .unwrap_err() - .contains("On chain storage version set, while the pallet doesn't")); + assert!( + ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) + .unwrap_err() == "On chain storage version set, while the pallet doesn't".into() + ); }); } From 3271ab2f9888cbce34b2eee78d244723c7c875cb Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 13 May 2023 14:44:20 +0200 Subject: [PATCH 29/31] tests passing --- frame/support/test/tests/pallet.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 2ceec6b2dad5b..f6b5858f113fa 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -2112,7 +2112,8 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // The version isn't changed, we should detect it. assert!( Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap_err() == - "On chain and current storage version do not match".into() + "On chain and current storage version do not match. Missing runtime upgrade?" + .into() ); }); @@ -2141,7 +2142,9 @@ fn post_runtime_upgrade_detects_storage_version_issues() { // any storage version "enabled". assert!( ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) - .unwrap_err() == "On chain storage version set, while the pallet doesn't".into() + .unwrap_err() == "On chain storage version set, while the pallet \ + doesn't have the `#[pallet::storage_version(VERSION)]` attribute." + .into() ); }); } From ac204866e529d908bfe2dc19b3d2f48da1579e10 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 13 May 2023 14:54:19 +0200 Subject: [PATCH 30/31] unnecessary imports --- frame/support/procedural/src/lib.rs | 3 --- frame/transaction-payment/asset-tx-payment/src/lib.rs | 3 --- frame/transaction-payment/src/lib.rs | 3 --- 3 files changed, 9 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 2fa986f8b8ad1..895b09a17e083 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -41,9 +41,6 @@ use quote::quote; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; -#[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeError; - thread_local! { /// A global counter, can be used to generate a relatively unique identifier. static COUNTER: RefCell = RefCell::new(Counter(0)); diff --git a/frame/transaction-payment/asset-tx-payment/src/lib.rs b/frame/transaction-payment/asset-tx-payment/src/lib.rs index 50e01cd9b9d03..4e83d8b489b70 100644 --- a/frame/transaction-payment/asset-tx-payment/src/lib.rs +++ b/frame/transaction-payment/asset-tx-payment/src/lib.rs @@ -59,9 +59,6 @@ use sp_runtime::{ FixedPointOperand, }; -#[cfg(feature = "try-runtime")] -use frame_support::dispatch::DispatchError; - #[cfg(test)] mod mock; #[cfg(test)] diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index b94a187fe112d..83ff69428d144 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -70,9 +70,6 @@ use frame_support::{ weights::{Weight, WeightToFee}, }; -#[cfg(feature = "try-runtime")] -use frame_support::dispatch::DispatchError; - #[cfg(test)] mod mock; #[cfg(test)] From 0fed96eb4f35153dad1863d71fe2fc671a4e45c9 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Tue, 16 May 2023 20:52:34 +0200 Subject: [PATCH 31/31] Update frame/assets/src/migration.rs Co-authored-by: Keith Yeung --- frame/assets/src/migration.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/assets/src/migration.rs b/frame/assets/src/migration.rs index 796c8ba37fcbb..d854a64afb57f 100644 --- a/frame/assets/src/migration.rs +++ b/frame/assets/src/migration.rs @@ -125,7 +125,10 @@ pub mod v1 { ); Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { - ensure!(asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state"); + ensure!( + asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, + "assets should only be live or frozen. None should be in destroying status, or undefined state" + ); Ok(()) })?; Ok(())