From 69e92adc5dcfa13452750c2d4847bd445ca56e8d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 2 Nov 2021 09:24:49 +0100 Subject: [PATCH 01/31] add missing version to dependencies --- frame/bags-list/remote-tests/Cargo.toml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/bags-list/remote-tests/Cargo.toml b/frame/bags-list/remote-tests/Cargo.toml index c670178c6188a..ee5b8c7c3f6e7 100644 --- a/frame/bags-list/remote-tests/Cargo.toml +++ b/frame/bags-list/remote-tests/Cargo.toml @@ -14,21 +14,21 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] # frame -pallet-staking = { path = "../../staking" } -pallet-bags-list = { path = "../../bags-list" } -frame-election-provider-support = { path = "../../election-provider-support" } -frame-system = { path = "../../system" } -frame-support = { path = "../../support" } +pallet-staking = { path = "../../staking", version = "4.0.0-dev" } +pallet-bags-list = { path = "../../bags-list", version = "4.0.0-dev" } +frame-election-provider-support = { path = "../../election-provider-support", version = "4.0.0-dev" } +frame-system = { path = "../../system", version = "4.0.0-dev" } +frame-support = { path = "../../support", version = "4.0.0-dev" } # core -sp-storage = { path = "../../../primitives/storage" } -sp-core = { path = "../../../primitives/core" } -sp-tracing = { path = "../../../primitives/tracing" } -sp-runtime = { path = "../../../primitives/runtime" } -sp-std = { path = "../../../primitives/std" } +sp-storage = { path = "../../../primitives/storage", version = "4.0.0-dev" } +sp-core = { path = "../../../primitives/core", version = "4.0.0-dev" } +sp-tracing = { path = "../../../primitives/tracing", version = "4.0.0-dev" } +sp-runtime = { path = "../../../primitives/runtime", version = "4.0.0-dev" } +sp-std = { path = "../../../primitives/std", version = "4.0.0-dev" } # utils -remote-externalities = { path = "../../../utils/frame/remote-externalities" } +remote-externalities = { path = "../../../utils/frame/remote-externalities", version = "0.10.0-dev" } # others tokio = { version = "1", features = ["macros"] } From 9f20cbeca3bd49d0ab6f2c7332342ec88a5f91fe Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 3 Nov 2021 18:32:55 +0100 Subject: [PATCH 02/31] Huh --- bin/node/runtime/Cargo.toml | 69 ++++++------ frame/bags-list/Cargo.toml | 6 +- frame/bags-list/src/lib.rs | 8 +- frame/bags-list/src/list/mod.rs | 6 +- frame/executive/src/lib.rs | 26 ++++- frame/staking/src/pallet/impls.rs | 102 ++++++++++++++++++ frame/staking/src/pallet/mod.rs | 6 ++ .../procedural/src/pallet/expand/hooks.rs | 14 +++ frame/support/src/dispatch.rs | 19 ++++ frame/support/src/traits.rs | 2 +- frame/support/src/traits/hooks.rs | 33 +++++- frame/transaction-storage/Cargo.toml | 1 + 12 files changed, 247 insertions(+), 45 deletions(-) diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 22ff0954e2458..b03153c4757ad 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -105,55 +105,66 @@ substrate-wasm-builder = { version = "5.0.0-dev", path = "../../../utils/wasm-bu default = ["std"] with-tracing = ["frame-executive/with-tracing"] std = [ + "codec/std", + "scale-info/std", + "log/std", + "sp-authority-discovery/std", + "sp-block-builder/std", + "sp-consensus-babe/std", + "sp-inherents/std", + "sp-offchain/std", + "sp-core/std", + "sp-std/std", + "sp-api/std", + "sp-runtime/std", + "sp-staking/std", + "sp-session/std", + "sp-keyring", + "sp-transaction-pool/std", + "sp-version/std", + "sp-io/std", + "sp-npos-elections/std", + + "node-primitives/std", + + "frame-executive/std", + "frame-support/std", + "frame-benchmarking/std", + "frame-system-rpc-runtime-api/std", + "frame-system/std", + "frame-try-runtime/std", + + "pallet-utility/std", "pallet-assets/std", "pallet-authority-discovery/std", "pallet-authorship/std", - "sp-consensus-babe/std", "pallet-babe/std", "pallet-bags-list/std", "pallet-balances/std", "pallet-bounties/std", - "sp-block-builder/std", - "codec/std", - "scale-info/std", "pallet-collective/std", "pallet-contracts/std", "pallet-contracts-primitives/std", "pallet-contracts-rpc-runtime-api/std", "pallet-democracy/std", "pallet-elections-phragmen/std", - "frame-executive/std", "pallet-gilt/std", "pallet-grandpa/std", "pallet-im-online/std", "pallet-indices/std", - "sp-inherents/std", "pallet-lottery/std", "pallet-membership/std", "pallet-mmr/std", "pallet-multisig/std", "pallet-identity/std", "pallet-scheduler/std", - "node-primitives/std", - "sp-offchain/std", "pallet-offences/std", "pallet-proxy/std", - "sp-core/std", "pallet-randomness-collective-flip/std", - "sp-std/std", "pallet-session/std", - "sp-api/std", - "sp-runtime/std", - "sp-staking/std", "pallet-staking/std", - "sp-keyring", - "sp-session/std", "pallet-sudo/std", - "frame-support/std", - "frame-benchmarking/std", - "frame-system-rpc-runtime-api/std", - "frame-system/std", "pallet-election-provider-multi-phase/std", "pallet-timestamp/std", "pallet-tips/std", @@ -161,24 +172,18 @@ std = [ "pallet-transaction-payment/std", "pallet-transaction-storage/std", "pallet-treasury/std", - "sp-transaction-pool/std", - "pallet-utility/std", - "sp-version/std", "pallet-society/std", "pallet-recovery/std", "pallet-uniques/std", "pallet-vesting/std", - "log/std", - "frame-try-runtime/std", - "sp-npos-elections/std", - "sp-io/std" ] runtime-benchmarks = [ + "sp-runtime/runtime-benchmarks", "frame-benchmarking", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", + "pallet-election-provider-multi-phase/runtime-benchmarks", - "sp-runtime/runtime-benchmarks", "pallet-assets/runtime-benchmarks", "pallet-babe/runtime-benchmarks", "pallet-bags-list/runtime-benchmarks", @@ -214,19 +219,24 @@ runtime-benchmarks = [ "hex-literal", ] try-runtime = [ - "frame-executive/try-runtime", "frame-try-runtime", + "frame-executive/try-runtime", "frame-system/try-runtime", + "frame-support/try-runtime", + + "pallet-utility/try-runtime", "pallet-assets/try-runtime", "pallet-authority-discovery/try-runtime", "pallet-authorship/try-runtime", "pallet-babe/try-runtime", + "pallet-bags-list/try-runtime", "pallet-balances/try-runtime", "pallet-bounties/try-runtime", "pallet-collective/try-runtime", "pallet-contracts/try-runtime", "pallet-democracy/try-runtime", "pallet-elections-phragmen/try-runtime", + "pallet-gilt/try-runtime", "pallet-grandpa/try-runtime", "pallet-im-online/try-runtime", "pallet-indices/try-runtime", @@ -246,13 +256,12 @@ try-runtime = [ "pallet-timestamp/try-runtime", "pallet-tips/try-runtime", "pallet-transaction-payment/try-runtime", + "pallet-transaction-storage/try-runtime", "pallet-treasury/try-runtime", - "pallet-utility/try-runtime", "pallet-society/try-runtime", "pallet-recovery/try-runtime", "pallet-uniques/try-runtime", "pallet-vesting/try-runtime", - "pallet-gilt/try-runtime", ] # Make contract callable functions marked as __unstable__ available. Do not enable # on live chains as those are subject to change. diff --git a/frame/bags-list/Cargo.toml b/frame/bags-list/Cargo.toml index 372dc87e212e2..7c6729b1aff90 100644 --- a/frame/bags-list/Cargo.toml +++ b/frame/bags-list/Cargo.toml @@ -64,9 +64,9 @@ runtime-benchmarks = [ "frame-election-provider-support/runtime-benchmarks", ] fuzz = [ - "sp-core", + "sp-core", "sp-io", - "pallet-balances", + "pallet-balances", "sp-tracing", ] - +try-runtime = [ "frame-support/try-runtime" ] diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index b7f96799e459f..e7a446de38404 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -204,6 +204,12 @@ pub mod pallet { "thresholds must strictly increase, and have no duplicates", ); } + + #[cfg(feature = "try-runtime")] + fn sanity_check(_: BlockNumberFor) -> Result<(), &'static str> { + log!(info, "sanity-checking pallet-bags-list"); + List::::sanity_check() + } } } @@ -273,7 +279,7 @@ impl SortedListProvider for Pallet { #[cfg(not(feature = "std"))] fn sanity_check() -> Result<(), &'static str> { - Ok(()) + Err("sanity checks should not be called from wasm") } fn clear(maybe_count: Option) -> u32 { diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 4efc3163816ff..4a24df26ee578 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -205,6 +205,7 @@ impl List { crate::ListBags::::remove(removed_bag); } + #[cfg(feature = "std")] debug_assert_eq!(Self::sanity_check(), Ok(())); num_affected @@ -657,11 +658,6 @@ impl Bag { Ok(()) } - #[cfg(not(feature = "std"))] - fn sanity_check(&self) -> Result<(), &'static str> { - Ok(()) - } - /// Iterate over the nodes in this bag (public for tests). #[cfg(feature = "std")] #[allow(dead_code)] diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index b1bdf357ec07d..f3f49c3a4a4d8 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -160,11 +160,17 @@ impl< Block: traits::Block
, Context: Default, UnsignedValidator, - AllPallets: OnRuntimeUpgrade + #[cfg(not(feature = "try-runtime"))] AllPallets: OnRuntimeUpgrade + OnInitialize + OnIdle + OnFinalize + OffchainWorker, + #[cfg(feature = "try-runtime")] AllPallets: OnRuntimeUpgrade + + OnInitialize + + OnIdle + + OnFinalize + + OffchainWorker + + frame_support::traits::SanityCheck, COnRuntimeUpgrade: OnRuntimeUpgrade, > ExecuteBlock for Executive @@ -193,11 +199,17 @@ impl< Block: traits::Block
, Context: Default, UnsignedValidator, - AllPallets: OnRuntimeUpgrade + #[cfg(not(feature = "try-runtime"))] AllPallets: OnRuntimeUpgrade + OnInitialize + OnIdle + OnFinalize + OffchainWorker, + #[cfg(feature = "try-runtime")] AllPallets: OnRuntimeUpgrade + + OnInitialize + + OnIdle + + OnFinalize + + OffchainWorker + + frame_support::traits::SanityCheck, COnRuntimeUpgrade: OnRuntimeUpgrade, > Executive where @@ -248,6 +260,16 @@ where ); } + // run the sanity-checks of all pallets. + >::sanity_check( + *header.number(), + ) + .map_err(|e| { + sp_runtime::print("failure:"); + sp_runtime::print(e); + }) + .expect("sanity-checks should not fail"); + frame_system::Pallet::::block_weight().total() } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index ec34efe397f5a..e3202ca0dd3cb 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1320,3 +1320,105 @@ impl SortedListProvider for UseNominatorsMap { } } } + +#[cfg(feature = "try-runtime")] +impl Pallet { + pub(crate) fn do_sanity_check(_: BlockNumberFor) -> Result<(), &'static str> { + Self::check_nominators()?; + Self::check_exposures()?; + Self::check_ledgers()?; + Self::check_count() + } + + fn check_count() -> Result<(), &'static str> { + let nominator_count = Nominators::::iter().count() as u32; + let validator_count = Validators::::iter().count() as u32; + ensure!(nominator_count == CounterForNominators::::get(), "wrong CounterForNominators"); + ensure!(validator_count == CounterForValidators::::get(), "wrong CounterForValidators"); + + // the voters that the `SortedListProvider` list is storing for us. + let external_voters = ::SortedListProvider::count(); + ensure!(external_voters == nominator_count, "wrong external count"); + Ok(()) + } + + fn check_ledgers() -> Result<(), &'static str> { + Bonded::::iter() + .map(|(_, ctrl)| Self::ensure_ledger_consistent(ctrl)) + .collect::>() + } + + fn check_exposures() -> Result<(), &'static str> { + // a check per validator to ensure the exposure struct is always sane. + let era = Self::active_era().unwrap().index; + ErasStakers::::iter_prefix_values(era) + .map(|expo| { + ensure!( + expo.total == + expo.own + + expo.others + .iter() + .map(|e| e.value) + .fold(Zero::zero(), |acc, x| acc + x), + "wrong total exposure.", + ); + Ok(()) + }) + .collect::>() + } + + fn check_nominators() -> Result<(), &'static str> { + // 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; + >::iter() + .filter_map( + |(nominator, nomination)| { + if nomination.submitted_in > era { + Some(nominator) + } else { + None + } + }, + ) + .map(|nominator| { + // 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| { + 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."), + }; + Ok(()) + }) + .collect::>() + }) + .collect::>() + } + + fn ensure_is_stash(who: &T::AccountId) -> Result<(), &'static str> { + ensure!(Self::bonded(who).is_some(), "Not a stash."); + Ok(()) + } + + fn ensure_ledger_consistent(ctrl: T::AccountId) -> Result<(), &'static str> { + // ensures ledger.total == ledger.active + sum(ledger.unlocking). + let ledger = Self::ledger(ctrl).ok_or("Not a controller.")?; + let real_total: BalanceOf = + ledger.unlocking.iter().fold(ledger.active, |a, c| a + c.value); + ensure!(real_total == ledger.total, "ledger.total corrupt"); + ensure!( + ledger.active >= T::Currency::minimum_balance() || ledger.active.is_zero(), + "ledger.active corrupt" + ); + Ok(()) + } +} diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 8e97a90e07544..7d176f0d1c0a8 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -694,6 +694,12 @@ pub mod pallet { ); } } + + #[cfg(feature = "try-runtime")] + fn sanity_check(n: BlockNumberFor) -> Result<(), &'static str> { + log!(info, "sanity-checking pallet-staking"); + Self::do_sanity_check(n) + } } #[pallet::call] diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index e0b7e3669da43..03c023f245a3d 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -191,5 +191,19 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::integrity_test() } } + + #[cfg(feature = "try-runtime")] + impl<#type_impl_gen> + #frame_support::traits::SanityCheck<::BlockNumber> + for #pallet_ident<#type_use_gen> #where_clause + { + fn sanity_check(n: ::BlockNumber) -> Result<(), &'static str> { + < + Self as #frame_support::traits::Hooks< + ::BlockNumber + > + >::sanity_check(n) + } + } ) } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index a492bc12f6a38..8d94cf5aacb4c 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1519,6 +1519,18 @@ macro_rules! decl_module { {} }; + (@impl_sanity_check_default + { $system:ident } + $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + ) => { + #[cfg(feature = "try-runtime")] + impl<$trait_instance: $system::Config + $trait_name$(, $instance: $instantiable)?> + $crate::traits::SanityCheck<<$trait_instance as $system::Config>::BlockNumber> + for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* + {} + }; + (@impl_on_runtime_upgrade { $system:ident } $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; @@ -1988,6 +2000,13 @@ macro_rules! decl_module { $( $on_initialize )* } + $crate::decl_module! { + @impl_sanity_check_default + { $system } + $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; + { $( $other_where_bounds )* } + } + $crate::decl_module! { @impl_on_runtime_upgrade { $system } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index bb990e25646db..8bf94ea5f689c 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -75,7 +75,7 @@ pub use hooks::{ Hooks, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, OnTimestampSet, }; #[cfg(feature = "try-runtime")] -pub use hooks::{OnRuntimeUpgradeHelpersExt, ON_RUNTIME_UPGRADE_PREFIX}; +pub use hooks::{OnRuntimeUpgradeHelpersExt, SanityCheck, ON_RUNTIME_UPGRADE_PREFIX}; pub mod schedule; mod storage; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 2a8b0a156247a..256d0a357be49 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -122,7 +122,6 @@ pub trait OnRuntimeUpgradeHelpersExt { /// /// This can be used to communicate data from pre-upgrade to post-upgrade state and check /// them. See [`Self::set_temp_storage`] and [`Self::get_temp_storage`]. - #[cfg(feature = "try-runtime")] fn storage_key(ident: &str) -> [u8; 32] { crate::storage::storage_prefix(ON_RUNTIME_UPGRADE_PREFIX, ident.as_bytes()) } @@ -132,7 +131,6 @@ pub trait OnRuntimeUpgradeHelpersExt { /// Returns `None` if either the data is unavailable or un-decodable. /// /// A `at` storage identifier must be provided to indicate where the storage is being read from. - #[cfg(feature = "try-runtime")] fn get_temp_storage(at: &str) -> Option { sp_io::storage::get(&Self::storage_key(at)) .and_then(|bytes| codec::Decode::decode(&mut &*bytes).ok()) @@ -143,7 +141,6 @@ pub trait OnRuntimeUpgradeHelpersExt { /// /// A `at` storage identifier must be provided to indicate where the storage is being written /// to. - #[cfg(feature = "try-runtime")] fn set_temp_storage(data: T, at: &str) { sp_io::storage::set(&Self::storage_key(at), &data.encode()); } @@ -210,6 +207,27 @@ impl OnRuntimeUpgrade for Tuple { } } +/// Execute the sanity checks of this pallet, per block. +/// +/// It should focus on certain checks to ensure that the state is sensible. Can consume as much +/// weight as it needs. +#[cfg(feature = "try-runtime")] +pub trait SanityCheck { + /// Execute the sanity-checks. + fn sanity_check(_n: BlockNumber) -> Result<(), &'static str> { + Ok(()) + } +} + +#[cfg(feature = "try-runtime")] +#[impl_for_tuples(30)] +impl SanityCheck for Tuple { + fn sanity_check(n: BlockNumber) -> Result<(), &'static str> { + for_tuples!(#( Tuple::sanity_check(n.clone())?; )*); + Ok(()) + } +} + /// The pallet hooks trait. Implementing this lets you express some logic to execute. pub trait Hooks { /// The block is being finalized. Implement to have something happen. @@ -252,6 +270,15 @@ pub trait Hooks { 0 } + /// Execute the sanity checks of this pallet, per block. + /// + /// It should focus on certain checks to ensure that the state is sensible. Can consume as much + /// weight as it needs. + #[cfg(feature = "try-runtime")] + fn sanity_check(_n: BlockNumber) -> Result<(), &'static str> { + Ok(()) + } + /// Execute some pre-checks prior to a runtime upgrade. /// /// This hook is never meant to be executed on-chain but is meant to be used by testing tools. diff --git a/frame/transaction-storage/Cargo.toml b/frame/transaction-storage/Cargo.toml index bcd3fd145f575..10b22f6285dcb 100644 --- a/frame/transaction-storage/Cargo.toml +++ b/frame/transaction-storage/Cargo.toml @@ -46,3 +46,4 @@ std = [ "sp-std/std", "sp-inherents/std", ] +try-runtime = ["frame-support/try-runtime"] From 4586de127718cc3cba53b1773c17adaaa57d6b8d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 3 Nov 2021 18:53:49 +0100 Subject: [PATCH 03/31] add features more --- frame/beefy-mmr/Cargo.toml | 35 ++++++++++++++++++----------------- frame/beefy/Cargo.toml | 1 + 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/frame/beefy-mmr/Cargo.toml b/frame/beefy-mmr/Cargo.toml index 3d4a9a72ddf86..777b82e677652 100644 --- a/frame/beefy-mmr/Cargo.toml +++ b/frame/beefy-mmr/Cargo.toml @@ -36,21 +36,22 @@ hex-literal = "0.3" [features] default = ["std"] std = [ - "beefy-merkle-tree/std", - "beefy-primitives/std", - "codec/std", - "frame-support/std", - "frame-system/std", - "hex", - "libsecp256k1/std", - "log/std", - "pallet-beefy/std", - "pallet-mmr-primitives/std", - "pallet-mmr/std", - "pallet-session/std", - "serde", - "sp-core/std", - "sp-io/std", - "sp-runtime/std", - "sp-std/std", + "beefy-merkle-tree/std", + "beefy-primitives/std", + "codec/std", + "frame-support/std", + "frame-system/std", + "hex", + "libsecp256k1/std", + "log/std", + "pallet-beefy/std", + "pallet-mmr-primitives/std", + "pallet-mmr/std", + "pallet-session/std", + "serde", + "sp-core/std", + "sp-io/std", + "sp-runtime/std", + "sp-std/std", ] +try-runtime = [] diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index e5af666e7ca54..7a6be7cb8ca80 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -38,3 +38,4 @@ std = [ "sp-std/std", "pallet-session/std", ] +try-runtime = [] From c980770cd72b50e7f27548aab66c32640d713545 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 3 Nov 2021 19:00:03 +0100 Subject: [PATCH 04/31] more fixing --- frame/beefy-mmr/Cargo.toml | 2 +- frame/beefy/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/beefy-mmr/Cargo.toml b/frame/beefy-mmr/Cargo.toml index 777b82e677652..91ccd595a05b0 100644 --- a/frame/beefy-mmr/Cargo.toml +++ b/frame/beefy-mmr/Cargo.toml @@ -54,4 +54,4 @@ std = [ "sp-runtime/std", "sp-std/std", ] -try-runtime = [] +try-runtime = ["frame-support/try-runtime"] diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index 7a6be7cb8ca80..4567b7d7dc961 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -38,4 +38,4 @@ std = [ "sp-std/std", "pallet-session/std", ] -try-runtime = [] +try-runtime = ["frame-support/try-runtime"] From 3dad2780bd8d8ac504ceaed477bff9e7cbb2c77f Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 4 Nov 2021 11:42:00 +0100 Subject: [PATCH 05/31] last touches --- frame/staking/src/pallet/impls.rs | 11 ++++++----- frame/staking/src/pallet/mod.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index e3202ca0dd3cb..a5f241f49e73c 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1411,14 +1411,15 @@ impl Pallet { fn ensure_ledger_consistent(ctrl: T::AccountId) -> Result<(), &'static str> { // ensures ledger.total == ledger.active + sum(ledger.unlocking). - let ledger = Self::ledger(ctrl).ok_or("Not a controller.")?; + let ledger = Self::ledger(ctrl.clone()).ok_or("Not a controller.")?; let real_total: BalanceOf = ledger.unlocking.iter().fold(ledger.active, |a, c| a + c.value); ensure!(real_total == ledger.total, "ledger.total corrupt"); - ensure!( - ledger.active >= T::Currency::minimum_balance() || ledger.active.is_zero(), - "ledger.active corrupt" - ); + + if !(ledger.active >= T::Currency::minimum_balance() || ledger.active.is_zero()) { + log!(warn, "ledger.active less than ED: {:?}, {:?}", ctrl, ledger) + } + Ok(()) } } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 7d176f0d1c0a8..e95a05d7b3480 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -845,7 +845,7 @@ pub mod pallet { ) -> DispatchResult { let controller = ensure_signed(origin)?; let mut ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; - ensure!(ledger.unlocking.len() < MAX_UNLOCKING_CHUNKS, Error::::NoMoreChunks,); + ensure!(ledger.unlocking.len() < MAX_UNLOCKING_CHUNKS, Error::::NoMoreChunks); let mut value = value.min(ledger.active); From 844a86f3cbac71b668d7d6ab7b29331e676be440 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 23 Jun 2022 15:00:35 +0200 Subject: [PATCH 06/31] it all finally works --- Cargo.lock | 1 + bin/node-template/runtime/Cargo.toml | 4 +- bin/node-template/runtime/src/lib.rs | 2 +- bin/node/runtime/Cargo.toml | 2 +- bin/node/runtime/src/lib.rs | 8 +- frame/bags-list/src/lib.rs | 12 +- frame/bags-list/src/list/mod.rs | 1 - frame/executive/src/lib.rs | 31 ++-- frame/staking/src/pallet/mod.rs | 6 +- .../procedural/src/construct_runtime/mod.rs | 2 + .../procedural/src/pallet/expand/hooks.rs | 19 +- frame/support/src/traits.rs | 11 +- frame/support/src/traits/hooks.rs | 165 ++++++++++++++---- frame/support/src/traits/misc.rs | 21 --- frame/try-runtime/src/lib.rs | 9 +- utils/frame/try-runtime/cli/Cargo.toml | 1 + .../cli/src/commands/execute_block.rs | 18 +- .../cli/src/commands/follow_chain.rs | 13 +- utils/frame/try-runtime/cli/src/lib.rs | 19 +- 19 files changed, 235 insertions(+), 110 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdd2c9ad1bd6e..d62897463fb7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11296,6 +11296,7 @@ name = "try-runtime-cli" version = "0.10.0-dev" dependencies = [ "clap 3.1.18", + "frame-try-runtime", "jsonrpsee", "log", "parity-scale-codec", diff --git a/bin/node-template/runtime/Cargo.toml b/bin/node-template/runtime/Cargo.toml index 734ed089aa4bd..bbe3e8eef7d3c 100644 --- a/bin/node-template/runtime/Cargo.toml +++ b/bin/node-template/runtime/Cargo.toml @@ -63,6 +63,7 @@ std = [ "frame-support/std", "frame-system-rpc-runtime-api/std", "frame-system/std", + "frame-try-runtime/std", "pallet-aura/std", "pallet-balances/std", "pallet-grandpa/std", @@ -97,9 +98,10 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", ] try-runtime = [ - "frame-executive/try-runtime", "frame-try-runtime", + "frame-executive/try-runtime", "frame-system/try-runtime", + "frame-support/try-runtime", "pallet-aura/try-runtime", "pallet-balances/try-runtime", "pallet-grandpa/try-runtime", diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index de7a36627b3d6..5dcc52424c923 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -314,7 +314,7 @@ pub type Executive = frame_executive::Executive< Block, frame_system::ChainContext, Runtime, - AllPalletsWithSystem, + AllPalletsWithSystemFlat, >; #[cfg(feature = "runtime-benchmarks")] diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index b78f30a46a1f2..59f99f8ea6141 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -243,8 +243,8 @@ runtime-benchmarks = [ "hex-literal", ] try-runtime = [ - "frame-executive/try-runtime", "frame-try-runtime", + "frame-executive/try-runtime", "frame-system/try-runtime", "frame-support/try-runtime", "pallet-alliance/try-runtime", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 30850d1ebc807..ddd56881aa1b4 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1679,7 +1679,7 @@ pub type Executive = frame_executive::Executive< Block, frame_system::ChainContext, Runtime, - AllPalletsWithSystem, + AllPalletsWithSystemFlat, >; /// MMR helper types. @@ -2048,6 +2048,12 @@ impl_runtime_apis! { } fn execute_block(block: Block, state_root_check: bool, sanity_checks: frame_try_runtime::SanityCheckTargets) -> Weight { + log::info!( + target: "node-runtime", "try-runtime: executing block {:?} / root checks: {:?} / sanity-checks: {:?}", + block.header.hash(), + state_root_check, + sanity_checks, + ); Executive::try_execute_block(block, state_root_check, sanity_checks) } } diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index eb5a240a17847..86f057691b911 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -258,8 +258,10 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn sanity_check(_: BlockNumberFor) -> Result<(), &'static str> { - log!(info, "sanity-checking pallet-bags-list"); + fn sanity_check( + _: BlockNumberFor, + _: frame_support::traits::SanityCheckTargets, + ) -> Result<(), &'static str> { >::sanity_check() } } @@ -339,16 +341,10 @@ impl, I: 'static> SortedListProvider for Pallet List::::unsafe_regenerate(all, score_of) } - #[cfg(feature = "std")] fn sanity_check() -> Result<(), &'static str> { List::::sanity_check() } - #[cfg(not(feature = "std"))] - fn sanity_check() -> Result<(), &'static str> { - Err("sanity checks should not be called from wasm") - } - fn unsafe_clear() { // NOTE: This call is unsafe for the same reason as SortedListProvider::unsafe_clear. // I.e. because it can lead to many storage accesses. diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 4d66db0a5defa..869f61b248cab 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -518,7 +518,6 @@ impl, I: 'static> List { /// * length of this list is in sync with `ListNodes::count()`, /// * 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(feature = "std", feature = "try-runtime"))] pub(crate) fn sanity_check() -> Result<(), &'static str> { let mut seen_in_list = BTreeSet::new(); ensure!( diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index ed18ef2c4cd31..07294dac0b0b0 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -239,6 +239,17 @@ where Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number()); + // run the sanity-checks of all pallets. + >::sanity_check( + *header.number(), + sanity_checks, + ) + .map_err(|e| { + sp_runtime::print("failure:"); + sp_runtime::print(e); + }) + .expect("sanity-checks should not fail"); + // do some of the checks that would normally happen in `final_checks`, but definitely skip // the state root check. { @@ -249,22 +260,21 @@ where assert!(header_item == computed_item, "Digest item must match that calculated."); } + if check_state_root { + let storage_root = new_header.state_root(); + header.state_root().check_equal(storage_root); + assert!( + header.state_root() == storage_root, + "Storage root must match that calculated." + ); + } + assert!( header.extrinsics_root() == new_header.extrinsics_root(), "Transaction trie root must be valid.", ); } - // run the sanity-checks of all pallets. - >::sanity_check( - *header.number(), - ) - .map_err(|e| { - sp_runtime::print("failure:"); - sp_runtime::print(e); - }) - .expect("sanity-checks should not fail"); - frame_system::Pallet::::block_weight().total() } @@ -274,7 +284,6 @@ where pub fn try_runtime_upgrade() -> Result { <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap(); let weight = Self::execute_on_runtime_upgrade(); - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade().unwrap(); Ok(weight) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index f4b5747830520..163c45d0fa7ca 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -750,8 +750,10 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn sanity_check(n: BlockNumberFor) -> Result<(), &'static str> { - crate::log!(info, "sanity-checking pallet-staking"); + fn sanity_check( + n: BlockNumberFor, + _: frame_support::traits::SanityCheckTargets, + ) -> Result<(), &'static str> { Self::do_sanity_check(n) } } diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 7b4156a94db58..33da2fe622295 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -366,6 +366,8 @@ fn decl_all_pallets<'a>( /// All pallets included in the runtime as a nested tuple of types. pub type AllPalletsWithSystem = ( #all_pallets_with_system ); + pub type AllPalletsWithSystemFlat = ( #(#names),* ); + /// All pallets included in the runtime as a nested tuple of types. /// Excludes the System pallet. pub type AllPalletsWithoutSystem = ( #all_pallets_without_system ); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index dd63044b8522c..78890d684c79a 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -17,7 +17,6 @@ use crate::pallet::Def; -/// /// * 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() { @@ -59,6 +58,19 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } }; + let log_sanity_check = quote::quote! { + let pallet_name = < + ::PalletInfo + as + #frame_support::traits::PalletInfo + >::name::().unwrap_or(""); + #frame_support::log::debug!( + target: #frame_support::LOG_TARGET, + "🩺 sanity-checking pallet {:?}", + pallet_name, + ); + }; + let hooks_impl = if def.hooks.is_none() { let frame_system = &def.frame_system; quote::quote! { @@ -197,12 +209,13 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::SanityCheck<::BlockNumber> for #pallet_ident<#type_use_gen> #where_clause { - fn sanity_check(n: ::BlockNumber) -> Result<(), &'static str> { + fn sanity_check(n: ::BlockNumber, t: #frame_support::traits::SanityCheckTargets) -> Result<(), &'static str> { + #log_sanity_check < Self as #frame_support::traits::Hooks< ::BlockNumber > - >::sanity_check(n) + >::sanity_check(n, t) } } ) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 237b55514ebdd..19d5e5fa3dc94 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -58,9 +58,9 @@ pub use misc::{ Backing, ConstBool, ConstI128, ConstI16, ConstI32, ConstI64, ConstI8, ConstU128, ConstU16, ConstU32, ConstU64, ConstU8, DefensiveSaturating, EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, - IsSubType, IsType, Len, OffchainWorker, OnKilledAccount, OnNewAccount, PreimageProvider, - PreimageRecipient, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, - WrapperKeepOpaque, WrapperOpaque, + IsSubType, IsType, Len, OnKilledAccount, OnNewAccount, PreimageProvider, PreimageRecipient, + PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, WrapperKeepOpaque, + WrapperOpaque, }; #[doc(hidden)] pub use misc::{DEFENSIVE_OP_INTERNAL_ERROR, DEFENSIVE_OP_PUBLIC_ERROR}; @@ -81,10 +81,11 @@ mod hooks; #[cfg(feature = "std")] pub use hooks::GenesisBuild; pub use hooks::{ - Hooks, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, OnTimestampSet, + Hooks, OffchainWorker, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, + OnTimestampSet, SanityCheckTargets, }; #[cfg(feature = "try-runtime")] -pub use hooks::{OnRuntimeUpgradeHelpersExt, SanityCheck, ON_RUNTIME_UPGRADE_PREFIX}; +pub use hooks::{OnRuntimeUpgradeHelpersExt, SanityCheck, ON_RUNTIME_UPGRADE_PREFIX}; // TODO: sanity-check-targets should also ideally be feature gated here. pub mod schedule; mod storage; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 6d8593dc198f9..4587ff20bc5d7 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -21,6 +21,25 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::Saturating; use sp_runtime::traits::AtLeast32BitUnsigned; +/// Off-chain computation trait. +/// +/// Implementing this trait on a module allows you to perform long-running tasks that make (by +/// default) validators generate transactions that feed results of those long-running computations +/// back on chain. +/// +/// NOTE: This function runs off-chain, so it can access the block state, but cannot preform any +/// alterations. More specifically alterations are not forbidden, but they are not persisted in any +/// way after the worker has finished. +#[impl_for_tuples(100)] +pub trait OffchainWorker { + /// This function is being called after every block import (when fully synced). + /// + /// Implement this and use any of the `Offchain` `sp_io` set of APIs to perform off-chain + /// computations, calls and submit transactions with results to trigger any on-chain changes. + /// Any state alterations are lost and are not persisted. + fn offchain_worker(_n: BlockNumber) {} +} + /// The block initialization trait. /// /// Implementing this lets you express what should happen for your pallet when the block is @@ -38,7 +57,7 @@ pub trait OnInitialize { } } -#[impl_for_tuples(30)] +#[impl_for_tuples(100)] impl OnInitialize for Tuple { fn on_initialize(n: BlockNumber) -> crate::weights::Weight { let mut weight = 0; @@ -50,7 +69,7 @@ impl OnInitialize for Tuple { /// The block finalization trait. /// /// Implementing this lets you express what should happen for your pallet when the block is ending. -#[impl_for_tuples(30)] +#[impl_for_tuples(100)] pub trait OnFinalize { /// The block is being finalized. Implement to have something happen. /// @@ -79,7 +98,7 @@ pub trait OnIdle { } } -#[impl_for_tuples(30)] +#[impl_for_tuples(100)] impl OnIdle for Tuple { fn on_idle(n: BlockNumber, remaining_weight: crate::weights::Weight) -> crate::weights::Weight { let on_idle_functions: &[fn( @@ -92,20 +111,127 @@ impl OnIdle for Tuple { let start_index = start_index.try_into().ok().expect( "`start_index % len` always fits into `usize`, because `len` can be in maximum `usize::MAX`; qed" ); - for on_idle in on_idle_functions.iter().cycle().skip(start_index).take(len) { + for on_idle_fn in on_idle_functions.iter().cycle().skip(start_index).take(len) { let adjusted_remaining_weight = remaining_weight.saturating_sub(weight); - weight = weight.saturating_add(on_idle(n, adjusted_remaining_weight)); + weight = weight.saturating_add(on_idle_fn(n, adjusted_remaining_weight)); } weight } } +use sp_std::prelude::*; + +/// Which sanity checks to execute. +#[derive(codec::Encode, codec::Decode, Clone)] +pub enum SanityCheckTargets { + All, + None, + RoundRobin(u32), + Selected(Vec>), +} + +impl sp_std::fmt::Debug for SanityCheckTargets { + fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + match self { + SanityCheckTargets::RoundRobin(x) => write!(f, "Random({})", x), + SanityCheckTargets::Selected(x) => write!( + f, + "Selected({:?})", + x.into_iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), + ), + SanityCheckTargets::All => write!(f, "All"), + SanityCheckTargets::None => write!(f, "None"), + } + } +} + +#[cfg(feature = "std")] +impl sp_std::str::FromStr for SanityCheckTargets { + type Err = &'static str; + fn from_str(s: &str) -> Result { + match s.as_ref() { + "all" | "All" => Ok(SanityCheckTargets::All), + "none" | "None" => Ok(SanityCheckTargets::None), + _ => + if s.starts_with("rr-") { + let count = s + .split_once("-") + .and_then(|(_, count)| count.parse::().ok()) + .ok_or("failed to parse count")?; + Ok(SanityCheckTargets::RoundRobin(count)) + } else { + let pallets = s.split(",").map(|x| x.as_bytes().to_vec()).collect::>(); + Ok(SanityCheckTargets::Selected(pallets)) + }, + } + } +} + +/// Execute the sanity checks of this pallet, per block. +/// +/// It should focus on certain checks to ensure that the state is sensible. Can consume as much +/// weight as it needs. +#[cfg(feature = "try-runtime")] +pub trait SanityCheck { + /// Execute the sanity-checks. + fn sanity_check(_: BlockNumber, _: SanityCheckTargets) -> Result<(), &'static str>; +} + +#[cfg(feature = "try-runtime")] +use crate::traits::PalletInfoAccess; + +#[cfg(feature = "try-runtime")] +#[impl_for_tuples(100)] +impl SanityCheck for Tuple { + for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); + fn sanity_check(n: BlockNumber, targets: SanityCheckTargets) -> Result<(), &'static str> { + match targets { + SanityCheckTargets::None => Ok(()), + SanityCheckTargets::All => { + let mut result = Ok(()); + for_tuples!( #( result = result.and(Tuple::sanity_check(n.clone(), targets.clone())); )* ); + result + }, + SanityCheckTargets::RoundRobin(len) => { + let functions: &[fn( + BlockNumber, + SanityCheckTargets, + ) -> Result<(), &'static str>] = &[for_tuples!(#( Tuple::sanity_check ),*)]; + let skip = n.clone() % (len as u32).into(); + let skip: u32 = skip.try_into().ok().expect("TODO"); + let mut result = Ok(()); + for sanity_check_fn in + functions.iter().cycle().skip(skip as usize).take(len as usize) + { + result = result.and(sanity_check_fn(n.clone(), targets.clone())); + } + result + }, + SanityCheckTargets::Selected(ref pallet_names) => { + let functions: &[( + &'static str, + fn(BlockNumber, SanityCheckTargets) -> Result<(), &'static str>, + )] = &[for_tuples!( + #( (::name(), Tuple::sanity_check) ),* + )]; + let mut result = Ok(()); + for (name, sanity_check_fn) in functions { + if pallet_names.contains(&name.as_bytes().to_vec()) { + result = result.and(sanity_check_fn(n.clone(), targets.clone())); + } + } + result + }, + } + } +} + /// A trait that will be called at genesis. /// /// Implementing this trait for a pallet let's you express operations that should /// happen at genesis. It will be called in an externalities provided environment and /// will see the genesis state after all pallets have written their genesis state. -#[impl_for_tuples(30)] +#[impl_for_tuples(100)] pub trait OnGenesis { /// Something that should happen at genesis. fn on_genesis() {} @@ -184,7 +310,7 @@ pub trait OnRuntimeUpgrade { } } -#[impl_for_tuples(30)] +#[impl_for_tuples(100)] impl OnRuntimeUpgrade for Tuple { fn on_runtime_upgrade() -> crate::weights::Weight { let mut weight = 0; @@ -207,27 +333,6 @@ impl OnRuntimeUpgrade for Tuple { } } -/// Execute the sanity checks of this pallet, per block. -/// -/// It should focus on certain checks to ensure that the state is sensible. Can consume as much -/// weight as it needs. -#[cfg(feature = "try-runtime")] -pub trait SanityCheck { - /// Execute the sanity-checks. - fn sanity_check(_n: BlockNumber) -> Result<(), &'static str> { - Ok(()) - } -} - -#[cfg(feature = "try-runtime")] -#[impl_for_tuples(30)] -impl SanityCheck for Tuple { - fn sanity_check(n: BlockNumber) -> Result<(), &'static str> { - for_tuples!(#( Tuple::sanity_check(n.clone())?; )*); - Ok(()) - } -} - /// The pallet hooks trait. Implementing this lets you express some logic to execute. pub trait Hooks { /// The block is being finalized. Implement to have something happen. @@ -280,7 +385,7 @@ pub trait Hooks { /// It should focus on certain checks to ensure that the state is sensible. Can consume as much /// weight as it needs. #[cfg(feature = "try-runtime")] - fn sanity_check(_n: BlockNumber) -> Result<(), &'static str> { + fn sanity_check(_n: BlockNumber, _t: SanityCheckTargets) -> Result<(), &'static str> { Ok(()) } @@ -348,7 +453,7 @@ pub trait GenesisBuild: Default + sp_runtime::traits::MaybeSerializeD } /// A trait which is called when the timestamp is set in the runtime. -#[impl_for_tuples(30)] +#[impl_for_tuples(100)] pub trait OnTimestampSet { /// Called when the timestamp is set. fn on_timestamp_set(moment: Moment); diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index 1f0ba1e769c24..217f872479d5f 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -622,27 +622,6 @@ impl PrivilegeCmp for EqualPrivilegeOnly { } } -/// Off-chain computation trait. -/// -/// Implementing this trait on a module allows you to perform long-running tasks -/// that make (by default) validators generate transactions that feed results -/// of those long-running computations back on chain. -/// -/// NOTE: This function runs off-chain, so it can access the block state, -/// but cannot preform any alterations. More specifically alterations are -/// not forbidden, but they are not persisted in any way after the worker -/// has finished. -#[impl_trait_for_tuples::impl_for_tuples(30)] -pub trait OffchainWorker { - /// This function is being called after every block import (when fully synced). - /// - /// Implement this and use any of the `Offchain` `sp_io` set of APIs - /// to perform off-chain computations, calls and submit transactions - /// with results to trigger any on-chain changes. - /// Any state alterations are lost and are not persisted. - fn offchain_worker(_n: BlockNumber) {} -} - /// Some amount of backing from a group. The precise definition of what it means to "back" something /// is left flexible. pub struct Backing { diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index 151f121ffb01a..33526daf1acc6 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -22,14 +22,7 @@ use frame_support::weights::Weight; use sp_std::prelude::*; -/// Which sanity checks to execute. -#[derive(codec::Encode, codec::Decode, Clone, Debug)] -pub enum SanityCheckTargets { - All, - None, - Random(u32), - Selected(Vec>), -} +pub use frame_support::traits::SanityCheckTargets; sp_api::decl_runtime_apis! { /// Runtime api for testing the execution of a runtime upgrade. diff --git a/utils/frame/try-runtime/cli/Cargo.toml b/utils/frame/try-runtime/cli/Cargo.toml index a2d6eba3d8bfc..1666c0992efa4 100644 --- a/utils/frame/try-runtime/cli/Cargo.toml +++ b/utils/frame/try-runtime/cli/Cargo.toml @@ -31,3 +31,4 @@ sp-keystore = { version = "0.12.0", path = "../../../../primitives/keystore" } sp-runtime = { version = "6.0.0", path = "../../../../primitives/runtime" } sp-state-machine = { version = "0.12.0", path = "../../../../primitives/state-machine" } sp-version = { version = "5.0.0", path = "../../../../primitives/version" } +frame-try-runtime = { path = "../../../../frame/try-runtime" } diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index d10eb41a32fe7..07644c135d5af 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -19,6 +19,7 @@ use crate::{ build_executor, ensure_matching_spec, extract_code, full_extensions, hash_of, local_spec, state_machine_call_with_proof, SharedParams, State, LOG_TARGET, }; +use parity_scale_codec::Encode; use remote_externalities::rpc_api; use sc_service::{Configuration, NativeExecutionDispatch}; use sp_core::storage::well_known_keys; @@ -26,16 +27,21 @@ use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor}; use std::{fmt::Debug, str::FromStr}; /// Configurations of the [`Command::ExecuteBlock`]. +/// +/// This will always call into `TryRuntime_execute_block`, which can optionally skip state-root +/// check (useful for trying a unreleased runtime), and can execute runtime sanity checks as well. #[derive(Debug, Clone, clap::Parser)] pub struct ExecuteBlockCmd { /// Overwrite the wasm code in state or not. #[clap(long)] overwrite_wasm_code: bool, - /// If set, then the state root check is disabled by the virtue of calling into - /// `TryRuntime_execute_block` instead of `Core_execute_block`. + /// If set, then the state root check is disabled. #[clap(long)] - no_check: bool, + no_state_root_check: bool, + + #[clap(long, default_value = "all")] + sanity_check_targets: frame_try_runtime::SanityCheckTargets, /// The block hash at which to fetch the block. /// @@ -160,7 +166,7 @@ where let (mut header, extrinsics) = block.deconstruct(); header.digest_mut().pop(); let block = Block::new(header, extrinsics); - let encoded_block = block.encode(); + let payload = (block.clone(), !command.no_state_root_check, command.sanity_check_targets).encode(); let (expected_spec_name, expected_spec_version, _) = local_spec::(&ext, &executor); @@ -176,8 +182,8 @@ where &ext, &executor, execution, - if command.no_check { "TryRuntime_execute_block" } else { "Core_execute_block" }, - if command.no_check { todo!() } else { encoded_block.as_ref() }, + "TryRuntime_execute_block", + &payload, full_extensions(), )?; diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 23f9f6286a3fd..3168e630a72d2 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -23,7 +23,7 @@ use jsonrpsee::{ core::client::{Subscription, SubscriptionClientT}, ws_client::WsClientBuilder, }; -use parity_scale_codec::Decode; +use parity_scale_codec::{Decode, Encode}; use remote_externalities::{rpc_api, Builder, Mode, OnlineConfig}; use sc_executor::NativeExecutionDispatch; use sc_service::Configuration; @@ -40,6 +40,13 @@ pub struct FollowChainCmd { /// The url to connect to. #[clap(short, long, parse(try_from_str = parse::url))] uri: String, + + /// If set, then the state root check is enabled. + #[clap(long)] + state_root_check: bool, + + #[clap(long, default_value = "all")] + sanity_check_targets: frame_try_runtime::SanityCheckTargets, } pub(crate) async fn follow_chain( @@ -140,7 +147,9 @@ where &executor, execution, "TryRuntime_execute_block", - block.encode().as_ref(), + (block, command.state_root_check, command.sanity_check_targets.clone()) + .encode() + .as_ref(), full_extensions(), )?; diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 311b853ab398f..cffec33cb7b27 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -334,17 +334,18 @@ pub enum Command { /// different state transition function. /// /// To make testing slightly more dynamic, you can disable the state root check by enabling - /// `ExecuteBlockCmd::no_check`. If you get signature verification errors, you should - /// manually tweak your local runtime's spec version to fix this. + /// `ExecuteBlockCmd::no_check`. If you get signature verification errors, you should manually + /// tweak your local runtime's spec version to fix this. /// /// A subtle detail of execute block is that if you want to execute block 100 of a live chain /// again, you need to scrape the state of block 99. This is already done automatically if you /// use [`State::Live`], and the parent hash of the target block is used to scrape the state. /// If [`State::Snap`] is being used, then this needs to be manually taken into consideration. /// - /// This executes the same runtime api as normal block import, namely `Core_execute_block`. If - /// `ExecuteBlockCmd::no_check` is set, it uses a custom, try-runtime-only runtime - /// api called `TryRuntime_execute_block` without the state root checks. + /// This does not executes the same runtime api as normal block import, namely + /// `Core_execute_block`. Instead, it uses `TryRuntime_execute_block`, which can optionally + /// skip state-root check (useful for trying a unreleased runtime), and can execute runtime + /// sanity checks as well. ExecuteBlock(commands::execute_block::ExecuteBlockCmd), /// Executes *the offchain worker hooks* of a given block against some state. @@ -791,15 +792,15 @@ pub(crate) fn state_machine_call_with_proof>()), proof_nodes.len() ); - log::info!(target: LOG_TARGET, "proof size: {}", humanize(proof_size)); - log::info!(target: LOG_TARGET, "compact proof size: {}", humanize(compact_proof_size),); - log::info!( + log::debug!(target: LOG_TARGET, "proof size: {}", humanize(proof_size)); + log::debug!(target: LOG_TARGET, "compact proof size: {}", humanize(compact_proof_size),); + log::debug!( target: LOG_TARGET, "zstd-compressed compact proof {}", humanize(compressed_proof.len()), From 0a433452f1645dfd8260ecfaa3d4d3e543eafa03 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 23 Jun 2022 15:11:22 +0200 Subject: [PATCH 07/31] remove some feature gates --- frame/bags-list/src/list/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 869f61b248cab..5252a03ec76d7 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -756,7 +756,6 @@ impl, I: 'static> Bag { /// * Ensures head has no prev. /// * Ensures tail has no next. /// * Ensures there are no loops, traversal from head to tail is correct. - #[cfg(any(feature = "std", feature = "try-runtime"))] fn sanity_check(&self) -> Result<(), &'static str> { frame_support::ensure!( self.head() @@ -796,7 +795,6 @@ impl, I: 'static> Bag { } /// Check if the bag contains a node with `id`. - #[cfg(any(feature = "std", feature = "try-runtime"))] fn contains(&self, id: &T::AccountId) -> bool { self.iter().any(|n| n.id() == id) } @@ -901,7 +899,6 @@ impl, I: 'static> Node { self.bag_upper } - #[cfg(any(feature = "std", feature = "try-runtime"))] fn sanity_check(&self) -> Result<(), &'static str> { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; From d6db0d508b378d15eb7bb0b59275c196dbc41662 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 23 Jun 2022 15:25:01 +0200 Subject: [PATCH 08/31] remove unused --- frame/support/src/traits/hooks.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 4587ff20bc5d7..6445f7fd3056d 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -177,9 +177,6 @@ pub trait SanityCheck { fn sanity_check(_: BlockNumber, _: SanityCheckTargets) -> Result<(), &'static str>; } -#[cfg(feature = "try-runtime")] -use crate::traits::PalletInfoAccess; - #[cfg(feature = "try-runtime")] #[impl_for_tuples(100)] impl SanityCheck for Tuple { From 312749ddb2004046b026e9a4e6f0ab20a0adedbf Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 24 Jun 2022 11:15:41 +0200 Subject: [PATCH 09/31] fix old macro --- frame/support/src/dispatch.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index d4fea160477b8..d3bfdeea11040 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1559,7 +1559,24 @@ macro_rules! decl_module { impl<$trait_instance: $system::Config + $trait_name$(, $instance: $instantiable)?> $crate::traits::SanityCheck<<$trait_instance as $system::Config>::BlockNumber> for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* - {} + { + fn sanity_check( + _: <$trait_instance as $system::Config>::BlockNumber, + _: $crate::traits::SanityCheckTargets + ) -> Result<(), &'static str> { + let pallet_name = << + $trait_instance + as + $system::Config + >::PalletInfo as $crate::traits::PalletInfo>::name::().unwrap_or(""); + $crate::log::debug!( + target: $crate::LOG_TARGET, + "⚠️ pallet {} cannot have sanity-checks because it is using decl_module!", + pallet_name, + ); + Ok(()) + } + } }; (@impl_on_runtime_upgrade From d72ff852e54fd657b96f175853711dcabced4f3c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 24 Jun 2022 11:35:00 +0200 Subject: [PATCH 10/31] make it work again --- frame/bags-list/src/lib.rs | 5 +---- frame/staking/src/pallet/mod.rs | 1 - frame/support/procedural/src/pallet/expand/hooks.rs | 4 ++-- frame/support/src/traits/hooks.rs | 2 +- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 86f057691b911..392083eafdd36 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -258,10 +258,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn sanity_check( - _: BlockNumberFor, - _: frame_support::traits::SanityCheckTargets, - ) -> Result<(), &'static str> { + fn sanity_check(_: BlockNumberFor) -> Result<(), &'static str> { >::sanity_check() } } diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 163c45d0fa7ca..5adca757e0a20 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -752,7 +752,6 @@ pub mod pallet { #[cfg(feature = "try-runtime")] fn sanity_check( n: BlockNumberFor, - _: frame_support::traits::SanityCheckTargets, ) -> Result<(), &'static str> { Self::do_sanity_check(n) } diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 78890d684c79a..0cecbd4034b70 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -209,13 +209,13 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::traits::SanityCheck<::BlockNumber> for #pallet_ident<#type_use_gen> #where_clause { - fn sanity_check(n: ::BlockNumber, t: #frame_support::traits::SanityCheckTargets) -> Result<(), &'static str> { + fn sanity_check(n: ::BlockNumber, _t: #frame_support::traits::SanityCheckTargets) -> Result<(), &'static str> { #log_sanity_check < Self as #frame_support::traits::Hooks< ::BlockNumber > - >::sanity_check(n, t) + >::sanity_check(n) } } ) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 6445f7fd3056d..25dcdbd6da08d 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -382,7 +382,7 @@ pub trait Hooks { /// It should focus on certain checks to ensure that the state is sensible. Can consume as much /// weight as it needs. #[cfg(feature = "try-runtime")] - fn sanity_check(_n: BlockNumber, _t: SanityCheckTargets) -> Result<(), &'static str> { + fn sanity_check(_n: BlockNumber) -> Result<(), &'static str> { Ok(()) } From 123a16c711f3a71ef1992720015045c0348c104e Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 24 Jun 2022 13:21:24 +0200 Subject: [PATCH 11/31] fmt --- frame/staking/src/pallet/mod.rs | 4 +--- utils/frame/try-runtime/cli/src/commands/execute_block.rs | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 5adca757e0a20..bbab26bcb1bd7 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -750,9 +750,7 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn sanity_check( - n: BlockNumberFor, - ) -> Result<(), &'static str> { + fn sanity_check(n: BlockNumberFor) -> Result<(), &'static str> { Self::do_sanity_check(n) } } diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index 07644c135d5af..3999cc018ad75 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -166,7 +166,8 @@ where let (mut header, extrinsics) = block.deconstruct(); header.digest_mut().pop(); let block = Block::new(header, extrinsics); - let payload = (block.clone(), !command.no_state_root_check, command.sanity_check_targets).encode(); + let payload = + (block.clone(), !command.no_state_root_check, command.sanity_check_targets).encode(); let (expected_spec_name, expected_spec_version, _) = local_spec::(&ext, &executor); From f07e714882a2177c64fbceb6fc5a19bd1e429124 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 24 Jun 2022 13:24:12 +0200 Subject: [PATCH 12/31] remove unused import --- frame/try-runtime/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index 33526daf1acc6..3dcf852c37b17 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -20,8 +20,6 @@ #![cfg_attr(not(feature = "std"), no_std)] use frame_support::weights::Weight; -use sp_std::prelude::*; - pub use frame_support::traits::SanityCheckTargets; sp_api::decl_runtime_apis! { From 436afe061f68eeb36c7de7f37c55da4c95db1b49 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Sun, 14 Aug 2022 19:25:54 +0000 Subject: [PATCH 13/31] ".git/.scripts/fmt.sh" 1 --- frame/support/src/traits.rs | 8 ++++---- frame/try-runtime/src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index b2cfa641bfc2b..269fec270ee09 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -58,9 +58,9 @@ pub use misc::{ Backing, ConstBool, ConstI128, ConstI16, ConstI32, ConstI64, ConstI8, ConstU128, ConstU16, ConstU32, ConstU64, ConstU8, DefensiveSaturating, EnsureInherentsAreFirst, EqualPrivilegeOnly, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, - IsSubType, IsType, Len, OnKilledAccount, OnNewAccount, PreimageProvider, PreimageRecipient, - PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, WrapperKeepOpaque, - WrapperOpaque, OffchainWorker, + IsSubType, IsType, Len, OffchainWorker, OnKilledAccount, OnNewAccount, PreimageProvider, + PreimageRecipient, PrivilegeCmp, SameOrOther, Time, TryCollect, TryDrop, TypedGet, UnixTime, + WrapperKeepOpaque, WrapperOpaque, }; #[doc(hidden)] pub use misc::{DEFENSIVE_OP_INTERNAL_ERROR, DEFENSIVE_OP_PUBLIC_ERROR}; @@ -82,7 +82,7 @@ mod hooks; pub use hooks::GenesisBuild; pub use hooks::{ Hooks, IntegrityTest, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, - OnTimestampSet, SanityCheckTargets + OnTimestampSet, SanityCheckTargets, }; #[cfg(feature = "try-runtime")] pub use hooks::{OnRuntimeUpgradeHelpersExt, SanityCheck, ON_RUNTIME_UPGRADE_PREFIX}; // TODO: sanity-check-targets should also ideally be feature gated here. diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index 3dcf852c37b17..21003a8572a9a 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -19,8 +19,8 @@ #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::weights::Weight; pub use frame_support::traits::SanityCheckTargets; +use frame_support::weights::Weight; sp_api::decl_runtime_apis! { /// Runtime api for testing the execution of a runtime upgrade. From de278344eeb4ab46b316751417e99f3f2cfda159 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 16 Aug 2022 18:34:22 +0430 Subject: [PATCH 14/31] Cleanup more --- bin/node-template/runtime/src/lib.rs | 2 +- bin/node/runtime/src/lib.rs | 2 +- frame/executive/src/lib.rs | 18 +- .../procedural/src/construct_runtime/mod.rs | 2 - frame/support/src/traits.rs | 4 +- frame/support/src/traits/hooks.rs | 201 ++++++++++-------- frame/try-runtime/Cargo.toml | 2 +- .../cli/src/commands/execute_block.rs | 3 +- .../cli/src/commands/follow_chain.rs | 3 +- 9 files changed, 132 insertions(+), 105 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 4d693a2277235..9de3e32738b6e 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -318,7 +318,7 @@ pub type Executive = frame_executive::Executive< Block, frame_system::ChainContext, Runtime, - AllPalletsWithSystemFlat, + AllPalletsWithSystem, >; #[cfg(feature = "runtime-benchmarks")] diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 533755875b15a..488362bb358fd 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2074,7 +2074,7 @@ impl_runtime_apis! { state_root_check, sanity_checks, ); - Executive::try_execute_block(block, state_root_check, sanity_checks) + Executive::try_execute_block(block, state_root_check, sanity_checks).unwrap() } } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 08080c761f067..ba4005afe8d7e 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -224,14 +224,18 @@ where OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { - /// Execute given block, but don't do any of the `final_checks`. + /// Execute given block, but don't as strict is the normal block execution. /// - /// Should only be used for testing. + /// Some consensus related checks such as the state root check can be switched off via + /// `check_state_root`. Some additional non-consensus checks can be additionally enabled via + /// `sanity_checks`. + /// + /// Should only be used for testing ONLY. pub fn try_execute_block( block: Block, check_state_root: bool, sanity_checks: frame_try_runtime::SanityCheckTargets, - ) -> frame_support::weights::Weight { + ) -> Result { Self::initialize_block(block.header()); Self::initial_checks(&block); @@ -247,10 +251,10 @@ where .map_err(|e| { sp_runtime::print("failure:"); sp_runtime::print(e); - }) - .expect("sanity-checks should not fail"); + e + })?; - // do some of the checks that would normally happen in `final_checks`, but definitely skip + // do some of the checks that would normally happen in `final_checks`, but perhaps skip // the state root check. { let new_header = >::finalize(); @@ -275,7 +279,7 @@ where ); } - frame_system::Pallet::::block_weight().total() + Ok(frame_system::Pallet::::block_weight().total()) } /// Execute all `OnRuntimeUpgrade` of this runtime, including the pre and post migration checks. diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 5c913813096f5..042af31be6bf4 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -345,8 +345,6 @@ fn decl_all_pallets<'a>( /// All pallets included in the runtime as a nested tuple of types. pub type AllPalletsWithSystem = ( #(#names),* ); - pub type AllPalletsWithSystemFlat = ( #(#names),* ); - /// All pallets included in the runtime as a nested tuple of types. /// Excludes the System pallet. pub type AllPalletsWithoutSystem = ( #(#names_without_system),* ); diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 269fec270ee09..f5658efdc8d77 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -82,10 +82,10 @@ mod hooks; pub use hooks::GenesisBuild; pub use hooks::{ Hooks, IntegrityTest, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, - OnTimestampSet, SanityCheckTargets, + OnTimestampSet, }; #[cfg(feature = "try-runtime")] -pub use hooks::{OnRuntimeUpgradeHelpersExt, SanityCheck, ON_RUNTIME_UPGRADE_PREFIX}; // TODO: sanity-check-targets should also ideally be feature gated here. +pub use hooks::{OnRuntimeUpgradeHelpersExt, ON_RUNTIME_UPGRADE_PREFIX, sanity_checks::*}; pub mod schedule; mod storage; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index c4b8635fe348e..f5708ad0c881c 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -20,6 +20,7 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::Saturating; use sp_runtime::traits::AtLeast32BitUnsigned; +use sp_std::prelude::*; /// The block initialization trait. /// @@ -106,106 +107,128 @@ impl OnIdle for Tuple { } } -use sp_std::prelude::*; +#[cfg(feature = "try-runtime")] +pub mod sanity_checks { + use super::*; -/// Which sanity checks to execute. -#[derive(codec::Encode, codec::Decode, Clone)] -pub enum SanityCheckTargets { - All, - None, - RoundRobin(u32), - Selected(Vec>), -} + // Which sanity checks to execute. + #[derive(codec::Encode, codec::Decode, Clone)] + pub enum SanityCheckTargets { + /// None of them. + None, + /// All of them. + All, + /// Run a fixed number of them in a round robin manner. + RoundRobin(u32), + /// Run only pallets who's name matches the given list. + /// + /// Pallet names are obtained from [`PalletInfoAccess`]. + Selected(Vec>), + } -impl sp_std::fmt::Debug for SanityCheckTargets { - fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - match self { - SanityCheckTargets::RoundRobin(x) => write!(f, "Random({})", x), - SanityCheckTargets::Selected(x) => write!( - f, - "Selected({:?})", - x.into_iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), - ), - SanityCheckTargets::All => write!(f, "All"), - SanityCheckTargets::None => write!(f, "None"), + impl Default for SanityCheckTargets { + fn default() -> Self { + SanityCheckTargets::None } } -} -#[cfg(feature = "std")] -impl sp_std::str::FromStr for SanityCheckTargets { - type Err = &'static str; - fn from_str(s: &str) -> Result { - match s.as_ref() { - "all" | "All" => Ok(SanityCheckTargets::All), - "none" | "None" => Ok(SanityCheckTargets::None), - _ => - if s.starts_with("rr-") { - let count = s - .split_once("-") - .and_then(|(_, count)| count.parse::().ok()) - .ok_or("failed to parse count")?; - Ok(SanityCheckTargets::RoundRobin(count)) - } else { - let pallets = s.split(",").map(|x| x.as_bytes().to_vec()).collect::>(); - Ok(SanityCheckTargets::Selected(pallets)) - }, + impl sp_std::fmt::Debug for SanityCheckTargets { + fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + match self { + SanityCheckTargets::RoundRobin(x) => write!(f, "RoundRobin({})", x), + SanityCheckTargets::Selected(x) => write!( + f, + "Selected({:?})", + x.into_iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), + ), + SanityCheckTargets::All => write!(f, "All"), + SanityCheckTargets::None => write!(f, "None"), + } } } -} -/// Execute the sanity checks of this pallet, per block. -/// -/// It should focus on certain checks to ensure that the state is sensible. Can consume as much -/// weight as it needs. -#[cfg(feature = "try-runtime")] -pub trait SanityCheck { - /// Execute the sanity-checks. - fn sanity_check(_: BlockNumber, _: SanityCheckTargets) -> Result<(), &'static str>; -} + #[cfg(feature = "std")] + impl sp_std::str::FromStr for SanityCheckTargets { + type Err = &'static str; + fn from_str(s: &str) -> Result { + match s.as_ref() { + "all" | "All" => Ok(SanityCheckTargets::All), + "none" | "None" => Ok(SanityCheckTargets::None), + _ => + if s.starts_with("rr-") { + let count = s + .split_once("-") + .and_then(|(_, count)| count.parse::().ok()) + .ok_or("failed to parse count")?; + Ok(SanityCheckTargets::RoundRobin(count)) + } else { + let pallets = + s.split(",").map(|x| x.as_bytes().to_vec()).collect::>(); + Ok(SanityCheckTargets::Selected(pallets)) + }, + } + } + } -#[cfg(feature = "try-runtime")] -#[impl_for_tuples(100)] -impl SanityCheck for Tuple { - for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); - fn sanity_check(n: BlockNumber, targets: SanityCheckTargets) -> Result<(), &'static str> { - match targets { - SanityCheckTargets::None => Ok(()), - SanityCheckTargets::All => { - let mut result = Ok(()); - for_tuples!( #( result = result.and(Tuple::sanity_check(n.clone(), targets.clone())); )* ); - result - }, - SanityCheckTargets::RoundRobin(len) => { - let functions: &[fn( - BlockNumber, - SanityCheckTargets, - ) -> Result<(), &'static str>] = &[for_tuples!(#( Tuple::sanity_check ),*)]; - let skip = n.clone() % (len as u32).into(); - let skip: u32 = skip.try_into().ok().expect("TODO"); - let mut result = Ok(()); - for sanity_check_fn in - functions.iter().cycle().skip(skip as usize).take(len as usize) - { - result = result.and(sanity_check_fn(n.clone(), targets.clone())); - } - result - }, - SanityCheckTargets::Selected(ref pallet_names) => { - let functions: &[( - &'static str, - fn(BlockNumber, SanityCheckTargets) -> Result<(), &'static str>, - )] = &[for_tuples!( - #( (::name(), Tuple::sanity_check) ),* - )]; - let mut result = Ok(()); - for (name, sanity_check_fn) in functions { - if pallet_names.contains(&name.as_bytes().to_vec()) { + /// Execute the sanity checks of this pallet, per block. + /// + /// It should focus on certain checks to ensure that the state is sensible. Can consume as much + /// weight as it needs. + pub trait SanityCheck { + /// Execute the sanity-checks. + fn sanity_check(_: BlockNumber, _: SanityCheckTargets) -> Result<(), &'static str>; + } + + #[cfg_attr( + all(feature = "try-runtime", not(feature = "tuples-96"), not(feature = "tuples-128")), + impl_for_tuples(64) + )] + #[cfg_attr( + all(feature = "try-runtime", feature = "tuples-96", not(feature = "tuples-128")), + impl_for_tuples(96) + )] + #[cfg_attr(all(feature = "try-runtime", feature = "tuples-128"), impl_for_tuples(128))] + impl SanityCheck for Tuple { + for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); + fn sanity_check(n: BlockNumber, targets: SanityCheckTargets) -> Result<(), &'static str> { + match targets { + SanityCheckTargets::None => Ok(()), + SanityCheckTargets::All => { + let mut result = Ok(()); + for_tuples!( #( result = result.and(Tuple::sanity_check(n.clone(), targets.clone())); )* ); + result + }, + SanityCheckTargets::RoundRobin(len) => { + let functions: &[fn( + BlockNumber, + SanityCheckTargets, + ) -> Result<(), &'static str>] = &[for_tuples!(#( Tuple::sanity_check ),*)]; + let skip = n.clone() % (len as u32).into(); + let skip: u32 = skip.try_into().ok().expect("TODO"); + let mut result = Ok(()); + for sanity_check_fn in + functions.iter().cycle().skip(skip as usize).take(len as usize) + { result = result.and(sanity_check_fn(n.clone(), targets.clone())); } - } - result - }, + result + }, + SanityCheckTargets::Selected(ref pallet_names) => { + let functions: &[( + &'static str, + fn(BlockNumber, SanityCheckTargets) -> Result<(), &'static str>, + )] = &[for_tuples!( + #( (::name(), Tuple::sanity_check) ),* + )]; + let mut result = Ok(()); + for (name, sanity_check_fn) in functions { + if pallet_names.contains(&name.as_bytes().to_vec()) { + result = result.and(sanity_check_fn(n.clone(), targets.clone())); + } + } + result + }, + } } } } diff --git a/frame/try-runtime/Cargo.toml b/frame/try-runtime/Cargo.toml index 3ae27f351c33a..dd77c0438d71f 100644 --- a/frame/try-runtime/Cargo.toml +++ b/frame/try-runtime/Cargo.toml @@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"]} -frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } +frame-support = { version = "4.0.0-dev", default-features = false, features = [ "try-runtime" ], path = "../support" } sp-api = { version = "4.0.0-dev", default-features = false, path = "../../primitives/api" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index 3999cc018ad75..f26439fc87567 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -40,7 +40,8 @@ pub struct ExecuteBlockCmd { #[clap(long)] no_state_root_check: bool, - #[clap(long, default_value = "all")] + /// Which sanity check targets to execute when running this command. + #[clap(long, default_value = "none")] sanity_check_targets: frame_try_runtime::SanityCheckTargets, /// The block hash at which to fetch the block. diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 3168e630a72d2..9330c07c13bc9 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -45,7 +45,8 @@ pub struct FollowChainCmd { #[clap(long)] state_root_check: bool, - #[clap(long, default_value = "all")] + /// Which sanity checks to run. + #[clap(long, default_value = "none")] sanity_check_targets: frame_try_runtime::SanityCheckTargets, } From 6a3fdd2daba8eda6a10d3dd64dfaf0436754e52c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 19 Aug 2022 13:54:20 +0430 Subject: [PATCH 15/31] fix and rename everything --- bin/node-template/runtime/src/lib.rs | 8 +- bin/node/runtime/src/lib.rs | 13 +- frame/bags-list/fuzzer/src/main.rs | 2 +- frame/bags-list/remote-tests/src/lib.rs | 2 +- .../src/{sanity_check.rs => try_state.rs} | 2 +- frame/bags-list/src/lib.rs | 8 +- frame/bags-list/src/list/mod.rs | 16 +- frame/bags-list/src/list/tests.rs | 26 +-- frame/bags-list/src/mock.rs | 2 +- frame/election-provider-support/src/lib.rs | 4 +- frame/executive/src/lib.rs | 20 +- .../nomination-pools/benchmarking/src/lib.rs | 4 +- frame/nomination-pools/src/lib.rs | 13 +- frame/nomination-pools/src/mock.rs | 2 +- frame/staking/src/migrations.rs | 4 +- frame/staking/src/pallet/impls.rs | 12 +- frame/staking/src/pallet/mod.rs | 6 +- .../procedural/src/pallet/expand/hooks.rs | 15 +- frame/support/src/dispatch.rs | 12 +- frame/support/src/traits.rs | 7 +- frame/support/src/traits/hooks.rs | 166 +---------------- frame/support/src/traits/try_runtime.rs | 172 ++++++++++++++++++ frame/try-runtime/src/lib.rs | 4 +- .../cli/src/commands/execute_block.rs | 5 +- .../cli/src/commands/follow_chain.rs | 4 +- 25 files changed, 279 insertions(+), 250 deletions(-) rename frame/bags-list/remote-tests/src/{sanity_check.rs => try_state.rs} (96%) create mode 100644 frame/support/src/traits/try_runtime.rs diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 9de3e32738b6e..e9392b811af13 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -545,8 +545,12 @@ impl_runtime_apis! { (weight, BlockWeights::get().max_block) } - fn execute_block(block: Block, state_root_check: bool, sanity_checks: frame_try_runtime::SanityCheckTargets) -> Weight { - Executive::try_execute_block(block, state_root_check, sanity_checks) + fn execute_block( + block: Block, + state_root_check: bool, + select: frame_try_runtime::TryStateSelect + ) -> Weight { + Executive::try_execute_block(block, state_root_check, select).expect("try-state failed") } } } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 488362bb358fd..0007063d0b835 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2067,14 +2067,19 @@ impl_runtime_apis! { (weight, RuntimeBlockWeights::get().max_block) } - fn execute_block(block: Block, state_root_check: bool, sanity_checks: frame_try_runtime::SanityCheckTargets) -> Weight { + fn execute_block( + block: Block, + state_root_check: bool, + select: frame_try_runtime::TryStateSelect + ) -> Weight { log::info!( - target: "node-runtime", "try-runtime: executing block {:?} / root checks: {:?} / sanity-checks: {:?}", + target: "node-runtime", + "try-runtime: executing block {:?} / root checks: {:?} / try-state-select: {:?}", block.header.hash(), state_root_check, - sanity_checks, + select, ); - Executive::try_execute_block(block, state_root_check, sanity_checks).unwrap() + Executive::try_execute_block(block, state_root_check, select).unwrap() } } diff --git a/frame/bags-list/fuzzer/src/main.rs b/frame/bags-list/fuzzer/src/main.rs index c17fbe0a2f77f..9f7ca464cc2b8 100644 --- a/frame/bags-list/fuzzer/src/main.rs +++ b/frame/bags-list/fuzzer/src/main.rs @@ -88,7 +88,7 @@ fn main() { }, } - assert!(BagsList::sanity_check().is_ok()); + assert!(BagsList::try_state().is_ok()); }) }); } diff --git a/frame/bags-list/remote-tests/src/lib.rs b/frame/bags-list/remote-tests/src/lib.rs index 458064cf79f57..927c3dc91cb58 100644 --- a/frame/bags-list/remote-tests/src/lib.rs +++ b/frame/bags-list/remote-tests/src/lib.rs @@ -24,8 +24,8 @@ use sp_std::prelude::*; pub const LOG_TARGET: &str = "runtime::bags-list::remote-tests"; pub mod migration; -pub mod sanity_check; pub mod snapshot; +pub mod try_state; /// A wrapper for a runtime that the functions of this crate expect. /// diff --git a/frame/bags-list/remote-tests/src/sanity_check.rs b/frame/bags-list/remote-tests/src/try_state.rs similarity index 96% rename from frame/bags-list/remote-tests/src/sanity_check.rs rename to frame/bags-list/remote-tests/src/try_state.rs index 1027efb8539ee..11278c20eb8ed 100644 --- a/frame/bags-list/remote-tests/src/sanity_check.rs +++ b/frame/bags-list/remote-tests/src/try_state.rs @@ -44,7 +44,7 @@ pub async fn execute ext.execute_with(|| { sp_core::crypto::set_default_ss58_version(Runtime::SS58Prefix::get().try_into().unwrap()); - pallet_bags_list::Pallet::::sanity_check().unwrap(); + pallet_bags_list::Pallet::::try_state().unwrap(); log::info!(target: crate::LOG_TARGET, "executed bags-list sanity check with no errors."); crate::display_and_check_bags::(currency_unit, currency_name); diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 392083eafdd36..d7cf3db15f20a 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -258,8 +258,8 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn sanity_check(_: BlockNumberFor) -> Result<(), &'static str> { - >::sanity_check() + fn try_state(_: BlockNumberFor) -> Result<(), &'static str> { + >::try_state() } } } @@ -338,8 +338,8 @@ impl, I: 'static> SortedListProvider for Pallet List::::unsafe_regenerate(all, score_of) } - fn sanity_check() -> Result<(), &'static str> { - List::::sanity_check() + fn try_state() -> Result<(), &'static str> { + List::::try_state() } fn unsafe_clear() { diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 5252a03ec76d7..d582cb8ec0f36 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -221,7 +221,7 @@ impl, I: 'static> List { } #[cfg(feature = "std")] - debug_assert_eq!(Self::sanity_check(), Ok(())); + debug_assert_eq!(Self::try_state(), Ok(())); num_affected } @@ -509,7 +509,7 @@ impl, I: 'static> List { node.put(); } - /// Sanity check the list. + /// Check the internal state of the list. /// /// This should be called from the call-site, whenever one of the mutating apis (e.g. `insert`) /// is being used, after all other staking data (such as counter) has been updated. It checks: @@ -518,7 +518,7 @@ impl, I: 'static> List { /// * length of this list is in sync with `ListNodes::count()`, /// * 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`. - pub(crate) fn sanity_check() -> Result<(), &'static str> { + pub(crate) fn try_state() -> Result<(), &'static str> { let mut seen_in_list = BTreeSet::new(); ensure!( Self::iter().map(|node| node.id).all(|id| seen_in_list.insert(id)), @@ -546,7 +546,7 @@ impl, I: 'static> List { thresholds.into_iter().filter_map(|t| Bag::::get(t)) }; - let _ = active_bags.clone().try_for_each(|b| b.sanity_check())?; + let _ = active_bags.clone().try_for_each(|b| b.try_state())?; let nodes_in_bags_count = active_bags.clone().fold(0u32, |acc, cur| acc + cur.iter().count() as u32); @@ -557,7 +557,7 @@ impl, I: 'static> List { // check that all nodes are sane. We check the `ListNodes` storage item directly in case we // have some "stale" nodes that are not in a bag. for (_id, node) in crate::ListNodes::::iter() { - node.sanity_check()? + node.try_state()? } Ok(()) @@ -748,7 +748,7 @@ impl, I: 'static> Bag { } } - /// Sanity check this bag. + /// Check the internal state of the bag. /// /// Should be called by the call-site, after any mutating operation on a bag. The call site of /// this struct is always `List`. @@ -756,7 +756,7 @@ impl, I: 'static> Bag { /// * Ensures head has no prev. /// * Ensures tail has no next. /// * Ensures there are no loops, traversal from head to tail is correct. - fn sanity_check(&self) -> Result<(), &'static str> { + fn try_state(&self) -> Result<(), &'static str> { frame_support::ensure!( self.head() .map(|head| head.prev().is_none()) @@ -899,7 +899,7 @@ impl, I: 'static> Node { self.bag_upper } - fn sanity_check(&self) -> Result<(), &'static str> { + fn try_state(&self) -> Result<(), &'static str> { 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 9bdd54289fd88..966ea1a74c71c 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -350,15 +350,15 @@ mod list { } #[test] - fn sanity_check_works() { + fn try_state_works() { ExtBuilder::default().build_and_execute_no_post_check(|| { - assert_ok!(List::::sanity_check()); + assert_ok!(List::::try_state()); }); // 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::::sanity_check(), Err("duplicate identified")); + assert_eq!(List::::try_state(), Err("duplicate identified")); }); // ensure count is in sync with `ListNodes::count()`. @@ -372,7 +372,7 @@ mod list { CounterForListNodes::::mutate(|counter| *counter += 1); assert_eq!(crate::ListNodes::::count(), 5); - assert_eq!(List::::sanity_check(), Err("iter_count != stored_count")); + assert_eq!(List::::try_state(), Err("iter_count != stored_count")); }); } @@ -804,7 +804,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![2, 3, 13, 14]); - assert_ok!(bag_1000.sanity_check()); + assert_ok!(bag_1000.try_state()); // and the node isn't mutated when its removed assert_eq!(node_4, node_4_pre_remove); @@ -814,7 +814,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![3, 13, 14]); - assert_ok!(bag_1000.sanity_check()); + assert_ok!(bag_1000.try_state()); // when removing a tail that is not pointing at the head let node_14 = Node::::get(&14).unwrap(); @@ -822,7 +822,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![3, 13]); - assert_ok!(bag_1000.sanity_check()); + assert_ok!(bag_1000.try_state()); // when removing a tail that is pointing at the head let node_13 = Node::::get(&13).unwrap(); @@ -830,7 +830,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![3]); - assert_ok!(bag_1000.sanity_check()); + assert_ok!(bag_1000.try_state()); // when removing a node that is both the head & tail let node_3 = Node::::get(&3).unwrap(); @@ -846,7 +846,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_10), vec![1, 12]); - assert_ok!(bag_10.sanity_check()); + assert_ok!(bag_10.try_state()); // when removing a head that is pointing at the tail let node_1 = Node::::get(&1).unwrap(); @@ -854,7 +854,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_10), vec![12]); - assert_ok!(bag_10.sanity_check()); + assert_ok!(bag_10.try_state()); // and since we updated the bag's head/tail, we need to write this storage so we // can correctly `get` it again in later checks bag_10.put(); @@ -865,7 +865,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_2000), vec![15, 17, 18, 19]); - assert_ok!(bag_2000.sanity_check()); + assert_ok!(bag_2000.try_state()); // when removing a node that is pointing at tail, but not head let node_18 = Node::::get(&18).unwrap(); @@ -873,7 +873,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_2000), vec![15, 17, 19]); - assert_ok!(bag_2000.sanity_check()); + assert_ok!(bag_2000.try_state()); // finally, when reading from storage, the state of all bags is as expected assert_eq!( @@ -905,7 +905,7 @@ mod bags { // .. and the bag it was removed from let bag_1000 = Bag::::get(1_000).unwrap(); // is sane - assert_ok!(bag_1000.sanity_check()); + assert_ok!(bag_1000.try_state()); // and has the correct head and tail. assert_eq!(bag_1000.head, Some(3)); assert_eq!(bag_1000.tail, Some(4)); diff --git a/frame/bags-list/src/mock.rs b/frame/bags-list/src/mock.rs index 961bf2b83552f..3bc1b34657262 100644 --- a/frame/bags-list/src/mock.rs +++ b/frame/bags-list/src/mock.rs @@ -147,7 +147,7 @@ impl ExtBuilder { pub fn build_and_execute(self, test: impl FnOnce() -> ()) { self.build().execute_with(|| { test(); - List::::sanity_check().expect("Sanity check post condition failed") + List::::try_state().expect("Try-state post condition failed") }) } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index eee865d0b737b..21a01ce1904ec 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -513,8 +513,8 @@ pub trait SortedListProvider { /// unbounded amount of storage accesses. fn unsafe_clear(); - /// Sanity check internal state of list. Only meant for debug compilation. - fn sanity_check() -> Result<(), &'static str>; + /// Check internal state of list. Only meant for debugging. + fn try_state() -> Result<(), &'static str>; /// If `who` changes by the returned amount they are guaranteed to have a worst case change /// in their list position. diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index ba4005afe8d7e..31e5aad5b0b3b 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -213,7 +213,7 @@ impl< + OnIdle + OnFinalize + OffchainWorker - + frame_support::traits::SanityCheck, + + frame_support::traits::TryState, COnRuntimeUpgrade: OnRuntimeUpgrade, > Executive where @@ -227,15 +227,17 @@ where /// Execute given block, but don't as strict is the normal block execution. /// /// Some consensus related checks such as the state root check can be switched off via - /// `check_state_root`. Some additional non-consensus checks can be additionally enabled via - /// `sanity_checks`. + /// `try_state_root`. Some additional non-consensus checks can be additionally enabled via + /// `try_state`. /// /// Should only be used for testing ONLY. pub fn try_execute_block( block: Block, - check_state_root: bool, - sanity_checks: frame_try_runtime::SanityCheckTargets, + try_state_root: bool, + select: frame_try_runtime::TryStateSelect, ) -> Result { + use frame_support::traits::TryState; + Self::initialize_block(block.header()); Self::initial_checks(&block); @@ -243,10 +245,10 @@ where Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number()); - // run the sanity-checks of all pallets. - >::sanity_check( + // run the try-state checks of all pallets. + >::try_state( *header.number(), - sanity_checks, + select, ) .map_err(|e| { sp_runtime::print("failure:"); @@ -264,7 +266,7 @@ where assert!(header_item == computed_item, "Digest item must match that calculated."); } - if check_state_root { + if try_state_root { let storage_root = new_header.state_root(); header.state_root().check_equal(storage_root); assert!( diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 6cc589ceb08bd..fe9b634bd7735 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -200,11 +200,11 @@ impl ListScenario { Pools::::join(Origin::Signed(joiner.clone()).into(), amount, 1).unwrap(); - // Sanity check that the vote weight is still the same as the original bonded + // check that the vote weight is still the same as the original bonded let weight_of = pallet_staking::Pallet::::weight_of_fn(); assert_eq!(vote_to_balance::(weight_of(&self.origin1)).unwrap(), original_bonded); - // Sanity check the member was added correctly + // check the member was added correctly let member = PoolMembers::::get(&joiner).unwrap(); assert_eq!(member.points, amount); assert_eq!(member.pool_id, 1); diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 4a31048fb8d8b..94cffb3561543 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1105,7 +1105,7 @@ impl SubPools { } /// The sum of all unbonding balance, regardless of whether they are actually unlocked or not. - #[cfg(any(test, debug_assertions))] + #[cfg(any(feature = "try-runtime", test, debug_assertions))] fn sum_unbonding_balance(&self) -> BalanceOf { self.no_era.balance.saturating_add( self.with_era @@ -2128,6 +2128,11 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), &'static str> { + Self::do_try_state(u8::MAX) + } + fn integrity_test() { assert!( T::MaxPointsToBalance::get() > 0, @@ -2375,9 +2380,9 @@ impl Pallet { /// /// To cater for tests that want to escape parts of these checks, this function is split into /// multiple `level`s, where the higher the level, the more checks we performs. So, - /// `sanity_check(255)` is the strongest sanity check, and `0` performs no checks. - #[cfg(any(test, debug_assertions))] - pub fn sanity_checks(level: u8) -> Result<(), &'static str> { + /// `try_state(255)` is the strongest sanity check, and `0` performs no checks. + #[cfg(any(feature = "try-runtime", test, debug_assertions))] + pub fn do_try_state(level: u8) -> Result<(), &'static str> { if level.is_zero() { return Ok(()) } diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 5138c55afccac..3bb260e56f180 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -304,7 +304,7 @@ impl ExtBuilder { pub fn build_and_execute(self, test: impl FnOnce() -> ()) { self.build().execute_with(|| { test(); - Pools::sanity_checks(CheckLevel::get()).unwrap(); + Pools::do_try_state(CheckLevel::get()).unwrap(); }) } } diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 7e3bf6ccb93e1..ff01e1fe3b09b 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -153,7 +153,7 @@ pub mod v8 { Nominators::::iter().map(|(id, _)| id), Pallet::::weight_of_fn(), ); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::try_state(), Ok(())); StorageVersion::::put(crate::Releases::V8_0_0); crate::log!( @@ -170,7 +170,7 @@ pub mod v8 { #[cfg(feature = "try-runtime")] pub fn post_migrate() -> Result<(), &'static str> { - T::VoterList::sanity_check().map_err(|_| "VoterList is not in a sane state.")?; + T::VoterList::try_state().map_err(|_| "VoterList is not in a sane state.")?; crate::log!(info, "👜 staking bags-list migration passes POST migrate checks ✅",); Ok(()) } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index d104a353acdcb..21581f2aa4412 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -789,7 +789,7 @@ impl Pallet { Nominators::::count() + Validators::::count(), T::VoterList::count() ); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::try_state(), Ok(())); } /// This function will remove a nominator from the `Nominators` storage map, @@ -809,7 +809,7 @@ impl Pallet { false }; - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::try_state(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), T::VoterList::count() @@ -837,7 +837,7 @@ impl Pallet { Nominators::::count() + Validators::::count(), T::VoterList::count() ); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::try_state(), Ok(())); } /// This function will remove a validator from the `Validators` storage map. @@ -856,7 +856,7 @@ impl Pallet { false }; - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::try_state(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), T::VoterList::count() @@ -1369,7 +1369,7 @@ impl SortedListProvider for UseNominatorsAndValidatorsM // nothing to do upon regenerate. 0 } - fn sanity_check() -> Result<(), &'static str> { + fn try_state() -> Result<(), &'static str> { Ok(()) } @@ -1455,7 +1455,7 @@ impl StakingInterface for Pallet { #[cfg(feature = "try-runtime")] impl Pallet { - pub(crate) fn do_sanity_check(_: BlockNumberFor) -> Result<(), &'static str> { + pub(crate) fn do_try_state(_: BlockNumberFor) -> Result<(), &'static str> { Self::check_nominators()?; Self::check_exposures()?; Self::check_ledgers()?; diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index d3e8a7b218506..c4ad47969d4d7 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -746,8 +746,8 @@ pub mod pallet { } #[cfg(feature = "try-runtime")] - fn sanity_check(n: BlockNumberFor) -> Result<(), &'static str> { - Self::do_sanity_check(n) + fn try_state(n: BlockNumberFor) -> Result<(), &'static str> { + Self::do_try_state(n) } } @@ -861,7 +861,7 @@ pub mod pallet { if T::VoterList::contains(&stash) { let _ = T::VoterList::on_update(&stash, Self::weight_of(&ledger.stash)).defensive(); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + debug_assert_eq!(T::VoterList::try_state(), Ok(())); } Self::deposit_event(Event::::Bonded(stash, extra)); diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index 0cecbd4034b70..c314cb6c34c92 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -58,7 +58,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } }; - let log_sanity_check = quote::quote! { + let log_try_state = quote::quote! { let pallet_name = < ::PalletInfo as @@ -66,7 +66,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::name::().unwrap_or(""); #frame_support::log::debug!( target: #frame_support::LOG_TARGET, - "🩺 sanity-checking pallet {:?}", + "🩺 try-state pallet {:?}", pallet_name, ); }; @@ -206,16 +206,19 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #[cfg(feature = "try-runtime")] impl<#type_impl_gen> - #frame_support::traits::SanityCheck<::BlockNumber> + #frame_support::traits::TryState<::BlockNumber> for #pallet_ident<#type_use_gen> #where_clause { - fn sanity_check(n: ::BlockNumber, _t: #frame_support::traits::SanityCheckTargets) -> Result<(), &'static str> { - #log_sanity_check + fn try_state( + n: ::BlockNumber, + _s: #frame_support::traits::TryStateSelect + ) -> Result<(), &'static str> { + #log_try_state < Self as #frame_support::traits::Hooks< ::BlockNumber > - >::sanity_check(n) + >::try_state(n) } } ) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 73cec570a15ed..564327c299bc1 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1549,19 +1549,19 @@ macro_rules! decl_module { {} }; - (@impl_sanity_check_default + (@impl_try_state_default { $system:ident } $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; { $( $other_where_bounds:tt )* } ) => { #[cfg(feature = "try-runtime")] impl<$trait_instance: $system::Config + $trait_name$(, $instance: $instantiable)?> - $crate::traits::SanityCheck<<$trait_instance as $system::Config>::BlockNumber> + $crate::traits::TryState<<$trait_instance as $system::Config>::BlockNumber> for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { - fn sanity_check( + fn try_state( _: <$trait_instance as $system::Config>::BlockNumber, - _: $crate::traits::SanityCheckTargets + _: $crate::traits::TryStateSelect, ) -> Result<(), &'static str> { let pallet_name = << $trait_instance @@ -1570,7 +1570,7 @@ macro_rules! decl_module { >::PalletInfo as $crate::traits::PalletInfo>::name::().unwrap_or(""); $crate::log::debug!( target: $crate::LOG_TARGET, - "⚠️ pallet {} cannot have sanity-checks because it is using decl_module!", + "⚠️ pallet {} cannot have try-state because it is using decl_module!", pallet_name, ); Ok(()) @@ -2056,7 +2056,7 @@ macro_rules! decl_module { } $crate::decl_module! { - @impl_sanity_check_default + @impl_try_state_default { $system } $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; { $( $other_where_bounds )* } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index f5658efdc8d77..d4f0e73557c77 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -84,8 +84,6 @@ pub use hooks::{ Hooks, IntegrityTest, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, OnTimestampSet, }; -#[cfg(feature = "try-runtime")] -pub use hooks::{OnRuntimeUpgradeHelpersExt, ON_RUNTIME_UPGRADE_PREFIX, sanity_checks::*}; pub mod schedule; mod storage; @@ -106,3 +104,8 @@ pub use voting::{ ClassCountOf, CurrencyToVote, PollStatus, Polling, SaturatingCurrencyToVote, U128CurrencyToVote, VoteTally, }; + +#[cfg(feature = "try-runtime")] +mod try_runtime; +#[cfg(feature = "try-runtime")] +pub use try_runtime::{OnRuntimeUpgradeHelpersExt, Select as TryStateSelect, TryState}; diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index f5708ad0c881c..c184809f3a39b 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -107,132 +107,6 @@ impl OnIdle for Tuple { } } -#[cfg(feature = "try-runtime")] -pub mod sanity_checks { - use super::*; - - // Which sanity checks to execute. - #[derive(codec::Encode, codec::Decode, Clone)] - pub enum SanityCheckTargets { - /// None of them. - None, - /// All of them. - All, - /// Run a fixed number of them in a round robin manner. - RoundRobin(u32), - /// Run only pallets who's name matches the given list. - /// - /// Pallet names are obtained from [`PalletInfoAccess`]. - Selected(Vec>), - } - - impl Default for SanityCheckTargets { - fn default() -> Self { - SanityCheckTargets::None - } - } - - impl sp_std::fmt::Debug for SanityCheckTargets { - fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - match self { - SanityCheckTargets::RoundRobin(x) => write!(f, "RoundRobin({})", x), - SanityCheckTargets::Selected(x) => write!( - f, - "Selected({:?})", - x.into_iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), - ), - SanityCheckTargets::All => write!(f, "All"), - SanityCheckTargets::None => write!(f, "None"), - } - } - } - - #[cfg(feature = "std")] - impl sp_std::str::FromStr for SanityCheckTargets { - type Err = &'static str; - fn from_str(s: &str) -> Result { - match s.as_ref() { - "all" | "All" => Ok(SanityCheckTargets::All), - "none" | "None" => Ok(SanityCheckTargets::None), - _ => - if s.starts_with("rr-") { - let count = s - .split_once("-") - .and_then(|(_, count)| count.parse::().ok()) - .ok_or("failed to parse count")?; - Ok(SanityCheckTargets::RoundRobin(count)) - } else { - let pallets = - s.split(",").map(|x| x.as_bytes().to_vec()).collect::>(); - Ok(SanityCheckTargets::Selected(pallets)) - }, - } - } - } - - /// Execute the sanity checks of this pallet, per block. - /// - /// It should focus on certain checks to ensure that the state is sensible. Can consume as much - /// weight as it needs. - pub trait SanityCheck { - /// Execute the sanity-checks. - fn sanity_check(_: BlockNumber, _: SanityCheckTargets) -> Result<(), &'static str>; - } - - #[cfg_attr( - all(feature = "try-runtime", not(feature = "tuples-96"), not(feature = "tuples-128")), - impl_for_tuples(64) - )] - #[cfg_attr( - all(feature = "try-runtime", feature = "tuples-96", not(feature = "tuples-128")), - impl_for_tuples(96) - )] - #[cfg_attr(all(feature = "try-runtime", feature = "tuples-128"), impl_for_tuples(128))] - impl SanityCheck for Tuple { - for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); - fn sanity_check(n: BlockNumber, targets: SanityCheckTargets) -> Result<(), &'static str> { - match targets { - SanityCheckTargets::None => Ok(()), - SanityCheckTargets::All => { - let mut result = Ok(()); - for_tuples!( #( result = result.and(Tuple::sanity_check(n.clone(), targets.clone())); )* ); - result - }, - SanityCheckTargets::RoundRobin(len) => { - let functions: &[fn( - BlockNumber, - SanityCheckTargets, - ) -> Result<(), &'static str>] = &[for_tuples!(#( Tuple::sanity_check ),*)]; - let skip = n.clone() % (len as u32).into(); - let skip: u32 = skip.try_into().ok().expect("TODO"); - let mut result = Ok(()); - for sanity_check_fn in - functions.iter().cycle().skip(skip as usize).take(len as usize) - { - result = result.and(sanity_check_fn(n.clone(), targets.clone())); - } - result - }, - SanityCheckTargets::Selected(ref pallet_names) => { - let functions: &[( - &'static str, - fn(BlockNumber, SanityCheckTargets) -> Result<(), &'static str>, - )] = &[for_tuples!( - #( (::name(), Tuple::sanity_check) ),* - )]; - let mut result = Ok(()); - for (name, sanity_check_fn) in functions { - if pallet_names.contains(&name.as_bytes().to_vec()) { - result = result.and(sanity_check_fn(n.clone(), targets.clone())); - } - } - result - }, - } - } - } -} - /// A trait that will be called at genesis. /// /// Implementing this trait for a pallet let's you express operations that should @@ -246,44 +120,6 @@ pub trait OnGenesis { fn on_genesis() {} } -/// Prefix to be used (optionally) for implementing [`OnRuntimeUpgradeHelpersExt::storage_key`]. -#[cfg(feature = "try-runtime")] -pub const ON_RUNTIME_UPGRADE_PREFIX: &[u8] = b"__ON_RUNTIME_UPGRADE__"; - -/// Some helper functions for [`OnRuntimeUpgrade`] during `try-runtime` testing. -#[cfg(feature = "try-runtime")] -pub trait OnRuntimeUpgradeHelpersExt { - /// Generate a storage key unique to this runtime upgrade. - /// - /// This can be used to communicate data from pre-upgrade to post-upgrade state and check - /// them. See [`Self::set_temp_storage`] and [`Self::get_temp_storage`]. - fn storage_key(ident: &str) -> [u8; 32] { - crate::storage::storage_prefix(ON_RUNTIME_UPGRADE_PREFIX, ident.as_bytes()) - } - - /// Get temporary storage data written by [`Self::set_temp_storage`]. - /// - /// Returns `None` if either the data is unavailable or un-decodable. - /// - /// A `at` storage identifier must be provided to indicate where the storage is being read from. - fn get_temp_storage(at: &str) -> Option { - sp_io::storage::get(&Self::storage_key(at)) - .and_then(|bytes| codec::Decode::decode(&mut &*bytes).ok()) - } - - /// Write some temporary data to a specific storage that can be read (potentially in - /// post-upgrade hook) via [`Self::get_temp_storage`]. - /// - /// A `at` storage identifier must be provided to indicate where the storage is being written - /// to. - fn set_temp_storage(data: T, at: &str) { - sp_io::storage::set(&Self::storage_key(at), &data.encode()); - } -} - -#[cfg(feature = "try-runtime")] -impl OnRuntimeUpgradeHelpersExt for U {} - /// The runtime upgrade trait. /// /// Implementing this lets you express what should happen when the runtime upgrades, @@ -409,7 +245,7 @@ pub trait Hooks { /// It should focus on certain checks to ensure that the state is sensible. Can consume as much /// weight as it needs. #[cfg(feature = "try-runtime")] - fn sanity_check(_n: BlockNumber) -> Result<(), &'static str> { + fn try_state(_n: BlockNumber) -> Result<(), &'static str> { Ok(()) } diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs new file mode 100644 index 0000000000000..b462bd7edf199 --- /dev/null +++ b/frame/support/src/traits/try_runtime.rs @@ -0,0 +1,172 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Try-runtime specific traits and types. + +use super::*; +use impl_trait_for_tuples::impl_for_tuples; +use sp_arithmetic::traits::AtLeast32BitUnsigned; +use sp_std::prelude::*; + +/// Prefix to be used (optionally) for implementing [`OnRuntimeUpgradeHelpersExt::storage_key`]. +const ON_RUNTIME_UPGRADE_PREFIX: &[u8] = b"__ON_RUNTIME_UPGRADE__"; + +/// Some helper functions for [`OnRuntimeUpgrade`] during `try-runtime` testing. +pub trait OnRuntimeUpgradeHelpersExt { + /// Generate a storage key unique to this runtime upgrade. + /// + /// This can be used to communicate data from pre-upgrade to post-upgrade state and check + /// them. See [`Self::set_temp_storage`] and [`Self::get_temp_storage`]. + fn storage_key(ident: &str) -> [u8; 32] { + crate::storage::storage_prefix(ON_RUNTIME_UPGRADE_PREFIX, ident.as_bytes()) + } + + /// Get temporary storage data written by [`Self::set_temp_storage`]. + /// + /// Returns `None` if either the data is unavailable or un-decodable. + /// + /// A `at` storage identifier must be provided to indicate where the storage is being read from. + fn get_temp_storage(at: &str) -> Option { + sp_io::storage::get(&Self::storage_key(at)) + .and_then(|bytes| codec::Decode::decode(&mut &*bytes).ok()) + } + + /// Write some temporary data to a specific storage that can be read (potentially in + /// post-upgrade hook) via [`Self::get_temp_storage`]. + /// + /// A `at` storage identifier must be provided to indicate where the storage is being written + /// to. + fn set_temp_storage(data: T, at: &str) { + sp_io::storage::set(&Self::storage_key(at), &data.encode()); + } +} + +impl OnRuntimeUpgradeHelpersExt for U {} + +// Which state tests to execute. +#[derive(codec::Encode, codec::Decode, Clone)] +pub enum Select { + /// None of them. + None, + /// All of them. + All, + /// Run a fixed number of them in a round robin manner. + RoundRobin(u32), + /// Run only pallets who's name matches the given list. + /// + /// Pallet names are obtained from [`PalletInfoAccess`]. + Only(Vec>), +} + +impl Default for Select { + fn default() -> Self { + Select::None + } +} + +impl sp_std::fmt::Debug for Select { + fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + match self { + Select::RoundRobin(x) => write!(f, "RoundRobin({})", x), + Select::Only(x) => write!( + f, + "Only({:?})", + x.into_iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), + ), + Select::All => write!(f, "All"), + Select::None => write!(f, "None"), + } + } +} + +#[cfg(feature = "std")] +impl sp_std::str::FromStr for Select { + type Err = &'static str; + fn from_str(s: &str) -> Result { + match s.as_ref() { + "all" | "All" => Ok(Select::All), + "none" | "None" => Ok(Select::None), + _ => + if s.starts_with("rr-") { + let count = s + .split_once("-") + .and_then(|(_, count)| count.parse::().ok()) + .ok_or("failed to parse count")?; + Ok(Select::RoundRobin(count)) + } else { + let pallets = s.split(",").map(|x| x.as_bytes().to_vec()).collect::>(); + Ok(Select::Only(pallets)) + }, + } + } +} + +/// Execute some checks a pallet to ensure the state is consistent. +pub trait TryState { + /// Execute the state sanity checks. + fn try_state(_: BlockNumber, _: Select) -> Result<(), &'static str>; +} + +#[cfg_attr( + all(feature = "try-runtime", not(feature = "tuples-96"), not(feature = "tuples-128")), + impl_for_tuples(64) +)] +#[cfg_attr( + all(feature = "try-runtime", feature = "tuples-96", not(feature = "tuples-128")), + impl_for_tuples(96) +)] +#[cfg_attr(all(feature = "try-runtime", feature = "tuples-128"), impl_for_tuples(128))] +impl TryState for Tuple { + for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); + fn try_state(n: BlockNumber, targets: Select) -> Result<(), &'static str> { + match targets { + Select::None => Ok(()), + Select::All => { + let mut result = Ok(()); + for_tuples!( #( result = result.and(Tuple::try_state(n.clone(), targets.clone())); )* ); + result + }, + Select::RoundRobin(len) => { + let functions: &[fn(BlockNumber, Select) -> Result<(), &'static str>] = + &[for_tuples!(#( Tuple::try_state ),*)]; + let skip = n.clone() % (len as u32).into(); + let skip: u32 = skip.try_into().ok().expect("TODO"); + let mut result = Ok(()); + for try_state_fn in functions.iter().cycle().skip(skip as usize).take(len as usize) + { + result = result.and(try_state_fn(n.clone(), targets.clone())); + } + result + }, + Select::Only(ref pallet_names) => { + let functions: &[( + &'static str, + fn(BlockNumber, Select) -> Result<(), &'static str>, + )] = &[for_tuples!( + #( (::name(), Tuple::try_state) ),* + )]; + let mut result = Ok(()); + for (name, try_state_fn) in functions { + if pallet_names.contains(&name.as_bytes().to_vec()) { + result = result.and(try_state_fn(n.clone(), targets.clone())); + } + } + result + }, + } + } +} diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index 21003a8572a9a..7fec92712cd19 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -19,7 +19,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -pub use frame_support::traits::SanityCheckTargets; +pub use frame_support::traits::TryStateSelect; use frame_support::weights::Weight; sp_api::decl_runtime_apis! { @@ -38,6 +38,6 @@ sp_api::decl_runtime_apis! { /// /// This is only sensible where the incoming block is from a different network, yet it has /// the same block format as the runtime implementing this API. - fn execute_block(block: Block, state_root_check: bool, sanity_checks: SanityCheckTargets) -> Weight; + fn execute_block(block: Block, state_root_check: bool, try_state: TryStateSelect) -> Weight; } } diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index f26439fc87567..626f3e3e084c2 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -42,7 +42,7 @@ pub struct ExecuteBlockCmd { /// Which sanity check targets to execute when running this command. #[clap(long, default_value = "none")] - sanity_check_targets: frame_try_runtime::SanityCheckTargets, + try_state_targets: frame_try_runtime::TryStateSelect, /// The block hash at which to fetch the block. /// @@ -167,8 +167,7 @@ where let (mut header, extrinsics) = block.deconstruct(); header.digest_mut().pop(); let block = Block::new(header, extrinsics); - let payload = - (block.clone(), !command.no_state_root_check, command.sanity_check_targets).encode(); + let payload = (block.clone(), !command.no_state_root_check, command.try_state_targets).encode(); let (expected_spec_name, expected_spec_version, _) = local_spec::(&ext, &executor); diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 9330c07c13bc9..4d635738201a8 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -47,7 +47,7 @@ pub struct FollowChainCmd { /// Which sanity checks to run. #[clap(long, default_value = "none")] - sanity_check_targets: frame_try_runtime::SanityCheckTargets, + try_state_targets: frame_try_runtime::TryStateSelect, } pub(crate) async fn follow_chain( @@ -148,7 +148,7 @@ where &executor, execution, "TryRuntime_execute_block", - (block, command.state_root_check, command.sanity_check_targets.clone()) + (block, command.state_root_check, command.try_state_targets.clone()) .encode() .as_ref(), full_extensions(), From 6eddd2b2f83bbd7bfb2933f61b88baa62d63831f Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 19 Aug 2022 14:42:52 +0430 Subject: [PATCH 16/31] a few clippy fixes --- frame/support/src/traits/try_runtime.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index b462bd7edf199..7d068d56fc87e 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -85,7 +85,7 @@ impl sp_std::fmt::Debug for Select { Select::Only(x) => write!( f, "Only({:?})", - x.into_iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), + x.iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), ), Select::All => write!(f, "All"), Select::None => write!(f, "None"), @@ -97,18 +97,18 @@ impl sp_std::fmt::Debug for Select { impl sp_std::str::FromStr for Select { type Err = &'static str; fn from_str(s: &str) -> Result { - match s.as_ref() { + match s { "all" | "All" => Ok(Select::All), "none" | "None" => Ok(Select::None), _ => if s.starts_with("rr-") { let count = s - .split_once("-") + .split_once('-') .and_then(|(_, count)| count.parse::().ok()) .ok_or("failed to parse count")?; Ok(Select::RoundRobin(count)) } else { - let pallets = s.split(",").map(|x| x.as_bytes().to_vec()).collect::>(); + let pallets = s.split(',').map(|x| x.as_bytes().to_vec()).collect::>(); Ok(Select::Only(pallets)) }, } From 10b1af7544403765b58bfd9c86e73d7fba171f58 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 19 Aug 2022 13:11:19 +0200 Subject: [PATCH 17/31] Add try-runtime feature Signed-off-by: Oliver Tale-Yazdi --- frame/nomination-pools/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/Cargo.toml b/frame/nomination-pools/Cargo.toml index be5c38d85552c..1d613511eacec 100644 --- a/frame/nomination-pools/Cargo.toml +++ b/frame/nomination-pools/Cargo.toml @@ -32,7 +32,7 @@ sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } [features] runtime-benchmarks = [] -try-runtime = [] +try-runtime = [ "frame-support/try-runtime" ] default = ["std"] std = [ "codec/std", From d79d33bdc23a3093f975276907eb0b47c69998d2 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sat, 20 Aug 2022 15:17:07 +0430 Subject: [PATCH 18/31] small fixes --- frame/nomination-pools/src/lib.rs | 4 +++- utils/frame/remote-externalities/src/lib.rs | 2 +- .../cli/src/commands/execute_block.rs | 20 ++++++++++++++----- .../cli/src/commands/follow_chain.rs | 4 ++-- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index eaea6b9d4701e..c3c227721799e 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2402,7 +2402,9 @@ impl Pallet { let reward_pools = RewardPools::::iter_keys().collect::>(); assert_eq!(bonded_pools, reward_pools); - assert!(Metadata::::iter_keys().all(|k| bonded_pools.contains(&k))); + // all pools have a metadata. Once https://github.com/paritytech/substrate/issues/12077 is + // solved, ensure they are simply the same. + assert!(bonded_pools.iter().all(|b| Metadata::::contains_key(b))); assert!(SubPoolsStorage::::iter_keys().all(|k| bonded_pools.contains(&k))); assert!(MaxPools::::get().map_or(true, |max| bonded_pools.len() <= (max as usize))); diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 202560f18cf84..83481e745f5ee 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -56,7 +56,7 @@ type ChildKeyValues = Vec<(ChildInfo, Vec)>; const LOG_TARGET: &str = "remote-ext"; const DEFAULT_TARGET: &str = "wss://rpc.polkadot.io:443"; const BATCH_SIZE: usize = 1000; -const PAGE: u32 = 512; +const PAGE: u32 = 1000; #[rpc(client)] pub trait RpcApi { diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index 626f3e3e084c2..f35064df9f147 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -42,7 +42,7 @@ pub struct ExecuteBlockCmd { /// Which sanity check targets to execute when running this command. #[clap(long, default_value = "none")] - try_state_targets: frame_try_runtime::TryStateSelect, + try_state: frame_try_runtime::TryStateSelect, /// The block hash at which to fetch the block. /// @@ -76,7 +76,7 @@ pub struct ExecuteBlockCmd { } impl ExecuteBlockCmd { - fn block_at(&self) -> sc_cli::Result + async fn block_at(&self, ws_uri: String) -> sc_cli::Result where Block::Hash: FromStr, ::Err: Debug, @@ -87,6 +87,15 @@ impl ExecuteBlockCmd { log::warn!(target: LOG_TARGET, "--block-at is provided while state type is live. the `Live::at` will be ignored"); hash_of::(block_at) }, + (None, State::Live { at: None, .. }) => { + log::warn!( + target: LOG_TARGET, + "No --block-at or --at provided, using the latest finalized block instead" + ); + remote_externalities::rpc_api::get_finalized_head::(ws_uri) + .await + .map_err(Into::into) + }, (None, State::Live { at: Some(at), .. }) => hash_of::(at), _ => { panic!("either `--block-at` must be provided, or state must be `live with a proper `--at``"); @@ -129,13 +138,14 @@ where let executor = build_executor::(&shared, &config); let execution = shared.execution; - let block_at = command.block_at::()?; let block_ws_uri = command.block_ws_uri::(); + let block_at = command.block_at::(block_ws_uri.clone()).await?; let block: Block = rpc_api::get_block::(block_ws_uri.clone(), block_at).await?; let parent_hash = block.header().parent_hash(); log::info!( target: LOG_TARGET, - "fetched block from {:?}, parent_hash to fetch the state {:?}", + "fetched block #{:?} from {:?}, parent_hash to fetch the state {:?}", + block.header().number(), block_ws_uri, parent_hash ); @@ -167,7 +177,7 @@ where let (mut header, extrinsics) = block.deconstruct(); header.digest_mut().pop(); let block = Block::new(header, extrinsics); - let payload = (block.clone(), !command.no_state_root_check, command.try_state_targets).encode(); + let payload = (block.clone(), !command.no_state_root_check, command.try_state).encode(); let (expected_spec_name, expected_spec_version, _) = local_spec::(&ext, &executor); diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 4d635738201a8..714ad4565be05 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -47,7 +47,7 @@ pub struct FollowChainCmd { /// Which sanity checks to run. #[clap(long, default_value = "none")] - try_state_targets: frame_try_runtime::TryStateSelect, + try_state: frame_try_runtime::TryStateSelect, } pub(crate) async fn follow_chain( @@ -148,7 +148,7 @@ where &executor, execution, "TryRuntime_execute_block", - (block, command.state_root_check, command.try_state_targets.clone()) + (block, command.state_root_check, command.try_state.clone()) .encode() .as_ref(), full_extensions(), From b2a1ff8a531d5ba354f40c0b83b2a063b508e8a4 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sat, 20 Aug 2022 15:36:40 +0430 Subject: [PATCH 19/31] fmt --- utils/frame/try-runtime/cli/src/commands/follow_chain.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 714ad4565be05..b265fca447df3 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -148,9 +148,7 @@ where &executor, execution, "TryRuntime_execute_block", - (block, command.state_root_check, command.try_state.clone()) - .encode() - .as_ref(), + (block, command.state_root_check, command.try_state.clone()).encode().as_ref(), full_extensions(), )?; From 759056698fc1e1bdf0ddbaf0cb78d46e49c5898f Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Sun, 21 Aug 2022 06:31:41 +0100 Subject: [PATCH 20/31] Update bin/node-template/runtime/src/lib.rs --- bin/node-template/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index e9392b811af13..107f9f0069276 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -550,7 +550,7 @@ impl_runtime_apis! { state_root_check: bool, select: frame_try_runtime::TryStateSelect ) -> Weight { - Executive::try_execute_block(block, state_root_check, select).expect("try-state failed") + Executive::try_execute_block(block, state_root_check, select).expect("execute-block failed") } } } From 8c17b9381d4c80089b85addd2e3323a47cdb9677 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 21 Aug 2022 10:20:35 +0430 Subject: [PATCH 21/31] fix build --- frame/nomination-pools/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index c3c227721799e..493c720dde47b 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -2402,9 +2402,8 @@ impl Pallet { let reward_pools = RewardPools::::iter_keys().collect::>(); assert_eq!(bonded_pools, reward_pools); - // all pools have a metadata. Once https://github.com/paritytech/substrate/issues/12077 is - // solved, ensure they are simply the same. - assert!(bonded_pools.iter().all(|b| Metadata::::contains_key(b))); + // TODO: can't check this right now: https://github.com/paritytech/substrate/issues/12077 + // assert!(Metadata::::iter_keys().all(|k| bonded_pools.contains(&k))); assert!(SubPoolsStorage::::iter_keys().all(|k| bonded_pools.contains(&k))); assert!(MaxPools::::get().map_or(true, |max| bonded_pools.len() <= (max as usize))); From 73aa952d5d19ec1ed027250dd754f3046395cfd7 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Sun, 28 Aug 2022 14:20:02 +0100 Subject: [PATCH 22/31] Update utils/frame/try-runtime/cli/src/lib.rs Co-authored-by: David --- utils/frame/try-runtime/cli/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 7d94f28cec131..37303459f0ca0 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -342,7 +342,7 @@ pub enum Command { /// use [`State::Live`], and the parent hash of the target block is used to scrape the state. /// If [`State::Snap`] is being used, then this needs to be manually taken into consideration. /// - /// This does not executes the same runtime api as normal block import, namely + /// This does not execute the same runtime api as normal block import do, namely /// `Core_execute_block`. Instead, it uses `TryRuntime_execute_block`, which can optionally /// skip state-root check (useful for trying a unreleased runtime), and can execute runtime /// sanity checks as well. From ade3b1ca3d974405e5b0ff11b8473a865180ca5e Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Sun, 28 Aug 2022 14:21:09 +0100 Subject: [PATCH 23/31] Update utils/frame/try-runtime/cli/src/commands/execute_block.rs Co-authored-by: David --- utils/frame/try-runtime/cli/src/commands/execute_block.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index f35064df9f147..556bac18fd18d 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -36,7 +36,7 @@ pub struct ExecuteBlockCmd { #[clap(long)] overwrite_wasm_code: bool, - /// If set, then the state root check is disabled. + /// If set the state root check is disabled. #[clap(long)] no_state_root_check: bool, From e53d4eb0a170a803dc4897866197237e84e0121a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 28 Aug 2022 18:14:37 +0430 Subject: [PATCH 24/31] address all review comments --- bin/node-template/runtime/src/lib.rs | 2 ++ bin/node/runtime/src/lib.rs | 2 ++ frame/executive/src/lib.rs | 3 +- .../procedural/src/pallet/expand/hooks.rs | 2 +- frame/support/src/traits/hooks.rs | 4 +-- frame/support/src/traits/try_runtime.rs | 36 ++++++++++--------- .../cli/src/commands/execute_block.rs | 7 ++-- .../cli/src/commands/follow_chain.rs | 5 ++- 8 files changed, 36 insertions(+), 25 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 107f9f0069276..7280336cada3d 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -550,6 +550,8 @@ impl_runtime_apis! { state_root_check: bool, select: frame_try_runtime::TryStateSelect ) -> Weight { + // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to + // have a backtrace here. Executive::try_execute_block(block, state_root_check, select).expect("execute-block failed") } } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 35c8d90a81132..6f8aa6c665961 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2079,6 +2079,8 @@ impl_runtime_apis! { state_root_check, select, ); + // NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to + // have a backtrace here. Executive::try_execute_block(block, state_root_check, select).unwrap() } } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 31e5aad5b0b3b..0366227f97eb7 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -251,8 +251,7 @@ where select, ) .map_err(|e| { - sp_runtime::print("failure:"); - sp_runtime::print(e); + frame_support::log::error!(target: "runtime::executive", "failure: {:?}", e); e })?; diff --git a/frame/support/procedural/src/pallet/expand/hooks.rs b/frame/support/procedural/src/pallet/expand/hooks.rs index c314cb6c34c92..03878f3f186df 100644 --- a/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/frame/support/procedural/src/pallet/expand/hooks.rs @@ -63,7 +63,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ::PalletInfo as #frame_support::traits::PalletInfo - >::name::().unwrap_or(""); + >::name::().expect("Every active pallet has a name in the runtime; qed"); #frame_support::log::debug!( target: #frame_support::LOG_TARGET, "🩺 try-state pallet {:?}", diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index c184809f3a39b..4a4d839772c8a 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -242,8 +242,8 @@ pub trait Hooks { /// Execute the sanity checks of this pallet, per block. /// - /// It should focus on certain checks to ensure that the state is sensible. Can consume as much - /// weight as it needs. + /// It should focus on certain checks to ensure that the state is sensible. This is never + /// executed in a consensus code-path, therefore it can consume as much weight as it needs. #[cfg(feature = "try-runtime")] fn try_state(_n: BlockNumber) -> Result<(), &'static str> { Ok(()) diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index 7d068d56fc87e..0e23b2a2f6673 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -85,7 +85,9 @@ impl sp_std::fmt::Debug for Select { Select::Only(x) => write!( f, "Only({:?})", - x.iter().map(|x| sp_std::str::from_utf8(x).unwrap()).collect::>(), + x.iter() + .map(|x| sp_std::str::from_utf8(x).unwrap_or("")) + .collect::>(), ), Select::All => write!(f, "All"), Select::None => write!(f, "None"), @@ -115,21 +117,20 @@ impl sp_std::str::FromStr for Select { } } -/// Execute some checks a pallet to ensure the state is consistent. +/// Execute some checks to ensure the internal state of a pallet is consistent. +/// +/// Similar +/// +/// Usually, these checks should check all of the invariants that are expected to be help on all of +/// the storage items of your pallet. pub trait TryState { - /// Execute the state sanity checks. + /// Execute the state checks. fn try_state(_: BlockNumber, _: Select) -> Result<(), &'static str>; } -#[cfg_attr( - all(feature = "try-runtime", not(feature = "tuples-96"), not(feature = "tuples-128")), - impl_for_tuples(64) -)] -#[cfg_attr( - all(feature = "try-runtime", feature = "tuples-96", not(feature = "tuples-128")), - impl_for_tuples(96) -)] -#[cfg_attr(all(feature = "try-runtime", feature = "tuples-128"), impl_for_tuples(128))] +#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] +#[cfg_attr(all(feature = "tuples-128"), impl_for_tuples(128))] impl TryState for Tuple { for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); fn try_state(n: BlockNumber, targets: Select) -> Result<(), &'static str> { @@ -144,7 +145,8 @@ impl TryState for Tuple let functions: &[fn(BlockNumber, Select) -> Result<(), &'static str>] = &[for_tuples!(#( Tuple::try_state ),*)]; let skip = n.clone() % (len as u32).into(); - let skip: u32 = skip.try_into().ok().expect("TODO"); + let skip: u32 = + skip.try_into().unwrap_or_else(|_| sp_runtime::traits::Bounded::max_value()); let mut result = Ok(()); for try_state_fn in functions.iter().cycle().skip(skip as usize).take(len as usize) { @@ -153,15 +155,15 @@ impl TryState for Tuple result }, Select::Only(ref pallet_names) => { - let functions: &[( + let try_state_fns: &[( &'static str, fn(BlockNumber, Select) -> Result<(), &'static str>, )] = &[for_tuples!( #( (::name(), Tuple::try_state) ),* )]; let mut result = Ok(()); - for (name, try_state_fn) in functions { - if pallet_names.contains(&name.as_bytes().to_vec()) { + for (name, try_state_fn) in try_state_fns { + if pallet_names.iter().any(|n| n == name.as_bytes()) { result = result.and(try_state_fn(n.clone(), targets.clone())); } } diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index 62cbf8cfc8906..2e8f723f82206 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -28,7 +28,7 @@ use std::{fmt::Debug, str::FromStr}; /// Configurations of the [`Command::ExecuteBlock`]. /// -/// This will always call into `TryRuntime_execute_block`, which can optionally skip state-root +/// This will always call into `TryRuntime_execute_block`, which can optionally skip the state-root /// check (useful for trying a unreleased runtime), and can execute runtime sanity checks as well. #[derive(Debug, Clone, clap::Parser)] pub struct ExecuteBlockCmd { @@ -40,7 +40,10 @@ pub struct ExecuteBlockCmd { #[clap(long)] no_state_root_check: bool, - /// Which sanity check targets to execute when running this command. + /// Which try-state targets to execute when running this command. + /// + /// Expected values: `all`, `none`, or a comma separated list of pallets, as per pallet names + /// in `construct_runtime!()` (e.g. `Staking, System`). #[clap(long, default_value = "none")] try_state: frame_try_runtime::TryStateSelect, diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 67055b0286a7f..4bc29d69080ac 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -45,7 +45,10 @@ pub struct FollowChainCmd { #[clap(long)] state_root_check: bool, - /// Which sanity checks to run. + /// Which try-state targets to execute when running this command. + /// + /// Expected values: `all`, `none`, or a comma separated list of pallets, as per pallet names + /// in `construct_runtime!()` (e.g. `Staking, System`). #[clap(long, default_value = "none")] try_state: frame_try_runtime::TryStateSelect, } From e7057625fb48ca86a7b4ec63dc411157e246f8d9 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 29 Aug 2022 12:17:42 +0430 Subject: [PATCH 25/31] fix typos --- frame/support/src/traits/try_runtime.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index 0e23b2a2f6673..07bd146dfc4ef 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -119,9 +119,7 @@ impl sp_std::str::FromStr for Select { /// Execute some checks to ensure the internal state of a pallet is consistent. /// -/// Similar -/// -/// Usually, these checks should check all of the invariants that are expected to be help on all of +/// Usually, these checks should check all of the invariants that are expected to be held on all of /// the storage items of your pallet. pub trait TryState { /// Execute the state checks. From f733b6fcc329a483d203340ef6e0c6344747d420 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 29 Aug 2022 14:21:24 +0430 Subject: [PATCH 26/31] revert spec change --- utils/frame/try-runtime/cli/src/lib.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 3e6927bd5c783..76679c43f7f14 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -657,21 +657,27 @@ pub(crate) async fn ensure_matching_spec { - log::error!( - target: LOG_TARGET, + let msg = format!( "failed to fetch runtime version from {}: {:?}. Skipping the check", - uri, - why + uri, why ); + if relaxed { + log::error!(target: LOG_TARGET, "{}", msg); + } else { + panic!("{}", msg); + } }, } } From dfe0d8e1c0e65d1701080d93371e2a3e05159da4 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 29 Aug 2022 15:01:46 +0430 Subject: [PATCH 27/31] last touches --- frame/support/src/traits/try_runtime.rs | 6 ++++-- frame/system/src/lib.rs | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs index 07bd146dfc4ef..c5ddd1cae42be 100644 --- a/frame/support/src/traits/try_runtime.rs +++ b/frame/support/src/traits/try_runtime.rs @@ -129,7 +129,9 @@ pub trait TryState { #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] #[cfg_attr(all(feature = "tuples-128"), impl_for_tuples(128))] -impl TryState for Tuple { +impl TryState + for Tuple +{ for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); fn try_state(n: BlockNumber, targets: Select) -> Result<(), &'static str> { match targets { @@ -142,7 +144,7 @@ impl TryState for Tuple Select::RoundRobin(len) => { let functions: &[fn(BlockNumber, Select) -> Result<(), &'static str>] = &[for_tuples!(#( Tuple::try_state ),*)]; - let skip = n.clone() % (len as u32).into(); + let skip = n.clone() % (functions.len() as u32).into(); let skip: u32 = skip.try_into().unwrap_or_else(|_| sp_runtime::traits::Bounded::max_value()); let mut result = Ok(()); diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 990e3cdd14de1..1a1d71bd4b3fe 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -1309,9 +1309,10 @@ impl Pallet { pub fn finalize() -> T::Header { log::debug!( target: "runtime::system", - "[{:?}] length: {} (normal {}%, op: {}%, mandatory {}%) / normal weight: {} ({}%) \ - / op weight {} ({}%) / mandatory weight {} ({}%)", + "[{:?}] {} extrinsics, length: {} (normal {}%, op: {}%, mandatory {}%) / normal weight:\ + {} ({}%) op weight {} ({}%) / mandatory weight {} ({}%)", Self::block_number(), + Self::extrinsic_index().unwrap_or_default(), Self::all_extrinsics_len(), sp_runtime::Percent::from_rational( Self::all_extrinsics_len(), From 73bae7e54ea605fd0cb2e301fb4fba8f1ce9f639 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 29 Aug 2022 15:10:46 +0430 Subject: [PATCH 28/31] update docs --- .../frame/try-runtime/cli/src/commands/execute_block.rs | 9 +++++++-- utils/frame/try-runtime/cli/src/commands/follow_chain.rs | 7 +++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/commands/execute_block.rs b/utils/frame/try-runtime/cli/src/commands/execute_block.rs index c4f0e3d47c6ce..6a3ef24ff3771 100644 --- a/utils/frame/try-runtime/cli/src/commands/execute_block.rs +++ b/utils/frame/try-runtime/cli/src/commands/execute_block.rs @@ -42,8 +42,13 @@ pub struct ExecuteBlockCmd { /// Which try-state targets to execute when running this command. /// - /// Expected values: `all`, `none`, or a comma separated list of pallets, as per pallet names - /// in `construct_runtime!()` (e.g. `Staking, System`). + /// Expected values: + /// - `all` + /// - `none` + /// - A comma separated list of pallets, as per pallet names in `construct_runtime!()` (e.g. + /// `Staking, System`). + /// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a + /// round-robin fashion. #[clap(long, default_value = "none")] try_state: frame_try_runtime::TryStateSelect, diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index 4bc29d69080ac..d54bce86b4dc3 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -47,8 +47,11 @@ pub struct FollowChainCmd { /// Which try-state targets to execute when running this command. /// - /// Expected values: `all`, `none`, or a comma separated list of pallets, as per pallet names - /// in `construct_runtime!()` (e.g. `Staking, System`). + /// Expected values: + /// - `all` + /// - `none` + /// - A comma separated list of pallets, as per pallet names in `construct_runtime!()` (e.g. `Staking, System`). + /// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a round-robin fashion. #[clap(long, default_value = "none")] try_state: frame_try_runtime::TryStateSelect, } From c12202c3d0020b7acfaec11019b323824e4019a4 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 29 Aug 2022 15:18:15 +0430 Subject: [PATCH 29/31] fmt --- utils/frame/try-runtime/cli/src/commands/follow_chain.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs index d54bce86b4dc3..01fc1dae15a05 100644 --- a/utils/frame/try-runtime/cli/src/commands/follow_chain.rs +++ b/utils/frame/try-runtime/cli/src/commands/follow_chain.rs @@ -50,8 +50,10 @@ pub struct FollowChainCmd { /// Expected values: /// - `all` /// - `none` - /// - A comma separated list of pallets, as per pallet names in `construct_runtime!()` (e.g. `Staking, System`). - /// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a round-robin fashion. + /// - A comma separated list of pallets, as per pallet names in `construct_runtime!()` (e.g. + /// `Staking, System`). + /// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a + /// round-robin fashion. #[clap(long, default_value = "none")] try_state: frame_try_runtime::TryStateSelect, } From f102730ca599b52eb12172ddc6ca6cc9f7c03e01 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 30 Aug 2022 16:52:36 +0430 Subject: [PATCH 30/31] remove some debug_assertions --- frame/bags-list/src/list/mod.rs | 14 +++++--------- frame/staking/src/pallet/impls.rs | 5 +---- frame/staking/src/pallet/mod.rs | 1 - 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index d582cb8ec0f36..227143562b47a 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -29,8 +29,8 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ ensure, - traits::{Defensive, Get}, - DefaultNoBound, PalletError, + traits::{Defensive, Get, DefensiveOption}, + DefaultNoBound, PalletError, defensive, }; use scale_info::TypeInfo; use sp_runtime::traits::{Bounded, Zero}; @@ -326,8 +326,7 @@ impl, I: 'static> List { crate::log!( debug, - "inserted {:?} with score {:? - } into bag {:?}, new count is {}", + "inserted {:?} with score {:?} into bag {:?}, new count is {}", id, score, bag_score, @@ -458,9 +457,7 @@ impl, I: 'static> List { // re-fetch `lighter_node` from storage since it may have been updated when `heavier_node` // was removed. - let lighter_node = Node::::get(lighter_id).ok_or_else(|| { - debug_assert!(false, "id that should exist cannot be found"); - crate::log!(warn, "id that should exist cannot be found"); + let lighter_node = Node::::get(lighter_id).defensive_ok_or_else(|| { ListError::NodeNotFound })?; @@ -696,8 +693,7 @@ impl, I: 'static> Bag { if *tail == node.id { // this should never happen, but this check prevents one path to a worst case // infinite loop. - debug_assert!(false, "system logic error: inserting a node who has the id of tail"); - crate::log!(warn, "system logic error: inserting a node who has the id of tail"); + defensive!("system logic error: inserting a node who has the id of tail"); return }; } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 21581f2aa4412..b8d6037bb06c2 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -789,7 +789,6 @@ impl Pallet { Nominators::::count() + Validators::::count(), T::VoterList::count() ); - debug_assert_eq!(T::VoterList::try_state(), Ok(())); } /// This function will remove a nominator from the `Nominators` storage map, @@ -809,7 +808,6 @@ impl Pallet { false }; - debug_assert_eq!(T::VoterList::try_state(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), T::VoterList::count() @@ -837,7 +835,6 @@ impl Pallet { Nominators::::count() + Validators::::count(), T::VoterList::count() ); - debug_assert_eq!(T::VoterList::try_state(), Ok(())); } /// This function will remove a validator from the `Validators` storage map. @@ -856,7 +853,6 @@ impl Pallet { false }; - debug_assert_eq!(T::VoterList::try_state(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), T::VoterList::count() @@ -1456,6 +1452,7 @@ impl StakingInterface for Pallet { #[cfg(feature = "try-runtime")] impl Pallet { pub(crate) fn do_try_state(_: BlockNumberFor) -> Result<(), &'static str> { + T::VoterList::try_state()?; Self::check_nominators()?; Self::check_exposures()?; Self::check_ledgers()?; diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 56a295d3ce211..74374cc586a23 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -861,7 +861,6 @@ pub mod pallet { if T::VoterList::contains(&stash) { let _ = T::VoterList::on_update(&stash, Self::weight_of(&ledger.stash)).defensive(); - debug_assert_eq!(T::VoterList::try_state(), Ok(())); } Self::deposit_event(Event::::Bonded(stash, extra)); From b49179633e623e1ea94667da03c15bf6c408326a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 1 Sep 2022 14:22:11 +0430 Subject: [PATCH 31/31] fmt --- frame/bags-list/src/list/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 227143562b47a..cfa7daf198937 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -28,9 +28,9 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ - ensure, - traits::{Defensive, Get, DefensiveOption}, - DefaultNoBound, PalletError, defensive, + defensive, ensure, + traits::{Defensive, DefensiveOption, Get}, + DefaultNoBound, PalletError, }; use scale_info::TypeInfo; use sp_runtime::traits::{Bounded, Zero}; @@ -457,9 +457,8 @@ impl, I: 'static> List { // re-fetch `lighter_node` from storage since it may have been updated when `heavier_node` // was removed. - let lighter_node = Node::::get(lighter_id).defensive_ok_or_else(|| { - ListError::NodeNotFound - })?; + let lighter_node = + Node::::get(lighter_id).defensive_ok_or_else(|| ListError::NodeNotFound)?; // insert `heavier_node` directly in front of `lighter_node`. This will update both nodes // in storage and update the node counter.