From 324a18e3c5cbf333672c54f9367f530ea976928d Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 1 Sep 2022 11:33:22 +0100 Subject: [PATCH] Runtime State Test + Integration with `try-runtime` (#10174) * add missing version to dependencies * Huh * add features more * more fixing * last touches * it all finally works * remove some feature gates * remove unused * fix old macro * make it work again * fmt * remove unused import * ".git/.scripts/fmt.sh" 1 * Cleanup more * fix and rename everything * a few clippy fixes * Add try-runtime feature Signed-off-by: Oliver Tale-Yazdi * small fixes * fmt * Update bin/node-template/runtime/src/lib.rs * fix build * Update utils/frame/try-runtime/cli/src/lib.rs Co-authored-by: David * Update utils/frame/try-runtime/cli/src/commands/execute_block.rs Co-authored-by: David * address all review comments * fix typos * revert spec change * last touches * update docs * fmt * remove some debug_assertions * fmt Signed-off-by: Oliver Tale-Yazdi Co-authored-by: command-bot <> Co-authored-by: Oliver Tale-Yazdi Co-authored-by: David --- Cargo.lock | 3 + bin/node-template/runtime/Cargo.toml | 4 +- bin/node-template/runtime/src/lib.rs | 10 +- bin/node/runtime/Cargo.toml | 20 +- bin/node/runtime/src/lib.rs | 17 +- 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 | 15 +- frame/bags-list/src/list/mod.rs | 43 ++--- frame/bags-list/src/list/tests.rs | 26 +-- frame/bags-list/src/mock.rs | 2 +- frame/beefy-mmr/Cargo.toml | 1 + frame/beefy/Cargo.toml | 1 + frame/election-provider-support/src/lib.rs | 4 +- frame/executive/Cargo.toml | 3 +- frame/executive/src/lib.rs | 77 ++++++-- frame/nomination-pools/Cargo.toml | 2 +- .../nomination-pools/benchmarking/src/lib.rs | 4 +- frame/nomination-pools/src/lib.rs | 16 +- frame/nomination-pools/src/mock.rs | 2 +- frame/remark/Cargo.toml | 1 + frame/staking/src/migrations.rs | 4 +- frame/staking/src/pallet/impls.rs | 107 ++++++++++- frame/staking/src/pallet/mod.rs | 6 +- .../procedural/src/pallet/expand/hooks.rs | 32 +++- frame/support/src/dispatch.rs | 36 ++++ frame/support/src/traits.rs | 7 +- frame/support/src/traits/hooks.rs | 55 ++---- frame/support/src/traits/try_runtime.rs | 174 ++++++++++++++++++ frame/system/src/lib.rs | 5 +- frame/transaction-storage/Cargo.toml | 1 + frame/try-runtime/Cargo.toml | 4 +- frame/try-runtime/src/lib.rs | 3 +- utils/frame/remote-externalities/src/lib.rs | 2 +- utils/frame/try-runtime/cli/Cargo.toml | 1 + .../cli/src/commands/execute_block.rs | 43 ++++- .../cli/src/commands/follow_chain.rs | 22 ++- utils/frame/try-runtime/cli/src/lib.rs | 41 +++-- 39 files changed, 621 insertions(+), 179 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/Cargo.lock b/Cargo.lock index b0d0206a35300..ae225184c5b28 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2228,6 +2228,7 @@ version = "4.0.0-dev" dependencies = [ "frame-support", "frame-system", + "frame-try-runtime", "hex-literal", "pallet-balances", "pallet-transaction-payment", @@ -2414,6 +2415,7 @@ name = "frame-try-runtime" version = "0.10.0-dev" dependencies = [ "frame-support", + "parity-scale-codec", "sp-api", "sp-runtime", "sp-std", @@ -11103,6 +11105,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 b43fbde52dcdc..7280336cada3d 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -545,8 +545,14 @@ impl_runtime_apis! { (weight, BlockWeights::get().max_block) } - fn execute_block_no_check(block: Block) -> Weight { - Executive::execute_block_no_check(block) + fn execute_block( + block: Block, + 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/Cargo.toml b/bin/node/runtime/Cargo.toml index 10b15b6ec554d..ebec9b8b755f6 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -245,14 +245,16 @@ 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", "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-child-bounties/try-runtime", @@ -264,32 +266,36 @@ try-runtime = [ "pallet-elections-phragmen/try-runtime", "pallet-gilt/try-runtime", "pallet-grandpa/try-runtime", - "pallet-identity/try-runtime", "pallet-im-online/try-runtime", "pallet-indices/try-runtime", + "pallet-identity/try-runtime", "pallet-lottery/try-runtime", "pallet-membership/try-runtime", "pallet-mmr/try-runtime", "pallet-multisig/try-runtime", + "pallet-nomination-pools/try-runtime", "pallet-offences/try-runtime", "pallet-preimage/try-runtime", "pallet-proxy/try-runtime", - "pallet-ranked-collective/try-runtime", "pallet-randomness-collective-flip/try-runtime", + "pallet-ranked-collective/try-runtime", "pallet-recovery/try-runtime", "pallet-referenda/try-runtime", - "pallet-scheduler/try-runtime", + "pallet-remark/try-runtime", "pallet-session/try-runtime", - "pallet-society/try-runtime", "pallet-staking/try-runtime", "pallet-state-trie-migration/try-runtime", + "pallet-scheduler/try-runtime", + "pallet-society/try-runtime", "pallet-sudo/try-runtime", "pallet-timestamp/try-runtime", "pallet-tips/try-runtime", - "pallet-transaction-payment/try-runtime", "pallet-treasury/try-runtime", - "pallet-uniques/try-runtime", "pallet-utility/try-runtime", + "pallet-transaction-payment/try-runtime", + "pallet-asset-tx-payment/try-runtime", + "pallet-transaction-storage/try-runtime", + "pallet-uniques/try-runtime", "pallet-vesting/try-runtime", "pallet-whitelist/try-runtime", ] diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5d4a21cea09b2..8ef7e7852ad02 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2079,8 +2079,21 @@ impl_runtime_apis! { (weight, RuntimeBlockWeights::get().max_block) } - fn execute_block_no_check(block: Block) -> Weight { - Executive::execute_block_no_check(block) + 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: {:?} / try-state-select: {:?}", + block.header.hash(), + 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/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 5163a579c6f43..70f8b2a32f817 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -263,6 +263,11 @@ pub mod pallet { "thresholds must strictly increase, and have no duplicates", ); } + + #[cfg(feature = "try-runtime")] + fn try_state(_: BlockNumberFor) -> Result<(), &'static str> { + >::try_state() + } } } @@ -340,14 +345,8 @@ 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> { - Ok(()) + 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 b4f852685842d..cfa7daf198937 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -28,8 +28,8 @@ use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::ScoreProvider; use frame_support::{ - ensure, - traits::{Defensive, Get}, + defensive, ensure, + traits::{Defensive, DefensiveOption, Get}, DefaultNoBound, PalletError, }; use scale_info::TypeInfo; @@ -220,7 +220,8 @@ impl, I: 'static> List { crate::ListBags::::remove(removed_bag); } - debug_assert_eq!(Self::sanity_check(), Ok(())); + #[cfg(feature = "std")] + debug_assert_eq!(Self::try_state(), Ok(())); num_affected } @@ -325,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, @@ -457,11 +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).ok_or_else(|| { - debug_assert!(false, "id that should exist cannot be found"); - crate::log!(warn, "id that should exist cannot be found"); - 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. @@ -508,7 +505,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: @@ -517,8 +514,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`. - #[cfg(feature = "std")] - 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 +542,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,17 +553,12 @@ 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(()) } - #[cfg(not(feature = "std"))] - pub(crate) fn sanity_check() -> Result<(), &'static str> { - Ok(()) - } - /// Returns the nodes of all non-empty bags. For testing and benchmarks. #[cfg(any(feature = "std", feature = "runtime-benchmarks"))] #[allow(dead_code)] @@ -701,8 +692,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 }; } @@ -753,7 +743,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`. @@ -761,8 +751,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. - #[cfg(feature = "std")] - 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()) @@ -801,7 +790,6 @@ impl, I: 'static> Bag { } /// Check if the bag contains a node with `id`. - #[cfg(feature = "std")] fn contains(&self, id: &T::AccountId) -> bool { self.iter().any(|n| n.id() == id) } @@ -906,8 +894,7 @@ impl, I: 'static> Node { self.bag_upper } - #[cfg(feature = "std")] - 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/beefy-mmr/Cargo.toml b/frame/beefy-mmr/Cargo.toml index 8da182f1c29fc..e8366943c85b0 100644 --- a/frame/beefy-mmr/Cargo.toml +++ b/frame/beefy-mmr/Cargo.toml @@ -50,3 +50,4 @@ std = [ "sp-runtime/std", "sp-std/std", ] +try-runtime = ["frame-support/try-runtime"] diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index eecce963d19f0..84aa8c7757c45 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -37,3 +37,4 @@ std = [ "sp-runtime/std", "sp-std/std", ] +try-runtime = ["frame-support/try-runtime"] 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/Cargo.toml b/frame/executive/Cargo.toml index b67f3313e612b..1ae22ddbb0260 100644 --- a/frame/executive/Cargo.toml +++ b/frame/executive/Cargo.toml @@ -19,6 +19,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features = scale-info = { version = "2.1.1", default-features = false, features = ["derive"] } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } +frame-try-runtime = { version = "0.10.0-dev", default-features = false, path = "../try-runtime", optional = true } sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } @@ -48,4 +49,4 @@ std = [ "sp-std/std", "sp-tracing/std", ] -try-runtime = ["frame-support/try-runtime"] +try-runtime = ["frame-support/try-runtime", "frame-try-runtime" ] diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 45361084f2f42..d7cd1da7910a4 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -202,6 +202,7 @@ where } } +#[cfg(feature = "try-runtime")] impl< System: frame_system::Config + EnsureInherentsAreFirst, Block: traits::Block
, @@ -211,7 +212,8 @@ impl< + OnInitialize + OnIdle + OnFinalize - + OffchainWorker, + + OffchainWorker + + frame_support::traits::TryState, COnRuntimeUpgrade: OnRuntimeUpgrade, > Executive where @@ -222,16 +224,20 @@ where OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { - /// Execute all `OnRuntimeUpgrade` of this runtime, and return the aggregate weight. - pub fn execute_on_runtime_upgrade() -> frame_support::weights::Weight { - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::on_runtime_upgrade() - } - - /// 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. - #[cfg(feature = "try-runtime")] - pub fn execute_block_no_check(block: Block) -> frame_support::weights::Weight { + /// Some consensus related checks such as the state root check can be switched off via + /// `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, + try_state_root: bool, + select: frame_try_runtime::TryStateSelect, + ) -> Result { + use frame_support::traits::TryState; + Self::initialize_block(block.header()); Self::initial_checks(&block); @@ -239,7 +245,17 @@ where Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number()); - // do some of the checks that would normally happen in `final_checks`, but definitely skip + // run the try-state checks of all pallets. + >::try_state( + *header.number(), + select, + ) + .map_err(|e| { + frame_support::log::error!(target: "runtime::executive", "failure: {:?}", e); + e + })?; + + // do some of the checks that would normally happen in `final_checks`, but perhaps skip // the state root check. { let new_header = >::finalize(); @@ -249,27 +265,60 @@ where assert!(header_item == computed_item, "Digest item must match that calculated."); } + if try_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.", ); } - 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. /// /// This should only be used for testing. - #[cfg(feature = "try-runtime")] 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) } +} + +impl< + System: frame_system::Config + EnsureInherentsAreFirst, + Block: traits::Block
, + Context: Default, + UnsignedValidator, + AllPalletsWithSystem: OnRuntimeUpgrade + + OnInitialize + + OnIdle + + OnFinalize + + OffchainWorker, + COnRuntimeUpgrade: OnRuntimeUpgrade, + > Executive +where + Block::Extrinsic: Checkable + Codec, + CheckedOf: Applyable + GetDispatchInfo, + CallOf: + Dispatchable, + OriginOf: From>, + UnsignedValidator: ValidateUnsigned>, +{ + /// Execute all `OnRuntimeUpgrade` of this runtime, and return the aggregate weight. + pub fn execute_on_runtime_upgrade() -> frame_support::weights::Weight { + <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::on_runtime_upgrade() + } /// Start the execution of a particular block. pub fn initialize_block(header: &System::Header) { 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", diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index f4ecb4a9b0ff1..07d5c63493ef8 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -201,11 +201,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 40db4ac64851a..3809a70440d27 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -1110,7 +1110,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 @@ -2138,6 +2138,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, @@ -2389,9 +2394,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(()) } @@ -2401,7 +2406,8 @@ 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))); + // 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))); 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/remark/Cargo.toml b/frame/remark/Cargo.toml index 71a65ce554975..fe20365b7c904 100644 --- a/frame/remark/Cargo.toml +++ b/frame/remark/Cargo.toml @@ -41,3 +41,4 @@ std = [ "sp-runtime/std", "sp-std/std", ] +try-runtime = [ "frame-support/try-runtime" ] 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 2a55d3baea2e6..386c413132f6c 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::sanity_check(), Ok(())); } /// This function will remove a nominator from the `Nominators` storage map, @@ -809,7 +808,6 @@ impl Pallet { false }; - debug_assert_eq!(T::VoterList::sanity_check(), 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::sanity_check(), Ok(())); } /// This function will remove a validator from the `Validators` storage map. @@ -856,7 +853,6 @@ impl Pallet { false }; - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); debug_assert_eq!( Nominators::::count() + Validators::::count(), T::VoterList::count() @@ -1369,7 +1365,7 @@ impl SortedListProvider for UseNominatorsAndValidatorsM // nothing to do upon regenerate. 0 } - fn sanity_check() -> Result<(), &'static str> { + fn try_state() -> Result<(), &'static str> { Ok(()) } @@ -1452,3 +1448,104 @@ impl StakingInterface for Pallet { Nominators::::get(who).map(|n| n.targets.into_inner()) } } + +#[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()?; + Self::check_count() + } + + fn check_count() -> Result<(), &'static str> { + ensure!( + ::VoterList::count() == + Nominators::::count() + Validators::::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.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"); + + 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 1f11f0ff00ac1..74374cc586a23 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -744,6 +744,11 @@ pub mod pallet { ); } } + + #[cfg(feature = "try-runtime")] + fn try_state(n: BlockNumberFor) -> Result<(), &'static str> { + Self::do_try_state(n) + } } #[pallet::call] @@ -856,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::sanity_check(), 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 7a1a94cf46d31..03878f3f186df 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_try_state = quote::quote! { + let pallet_name = < + ::PalletInfo + as + #frame_support::traits::PalletInfo + >::name::().expect("Every active pallet has a name in the runtime; qed"); + #frame_support::log::debug!( + target: #frame_support::LOG_TARGET, + "🩺 try-state pallet {:?}", + pallet_name, + ); + }; + let hooks_impl = if def.hooks.is_none() { let frame_system = &def.frame_system; quote::quote! { @@ -191,5 +203,23 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::integrity_test() } } + + #[cfg(feature = "try-runtime")] + impl<#type_impl_gen> + #frame_support::traits::TryState<::BlockNumber> + for #pallet_ident<#type_use_gen> #where_clause + { + fn try_state( + n: ::BlockNumber, + _s: #frame_support::traits::TryStateSelect + ) -> Result<(), &'static str> { + #log_try_state + < + Self as #frame_support::traits::Hooks< + ::BlockNumber + > + >::try_state(n) + } + } ) } diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 97312ac3476b5..94974888c1d5c 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1549,6 +1549,35 @@ macro_rules! decl_module { {} }; + (@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::TryState<<$trait_instance as $system::Config>::BlockNumber> + for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* + { + fn try_state( + _: <$trait_instance as $system::Config>::BlockNumber, + _: $crate::traits::TryStateSelect, + ) -> 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 try-state because it is using decl_module!", + pallet_name, + ); + Ok(()) + } + } + }; + (@impl_on_runtime_upgrade { $system:ident } $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; @@ -2026,6 +2055,13 @@ macro_rules! decl_module { $( $on_initialize )* } + $crate::decl_module! { + @impl_try_state_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 16504beb16907..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}; 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 f8e91886edf0c..e8cd87823f3c3 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -20,6 +20,7 @@ use crate::weights::Weight; use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; +use sp_std::prelude::*; /// The block initialization trait. /// @@ -93,9 +94,9 @@ 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 } @@ -114,47 +115,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`]. - #[cfg(feature = "try-runtime")] - 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. - #[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()) - } - - /// 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. - #[cfg(feature = "try-runtime")] - 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, @@ -272,6 +232,15 @@ pub trait Hooks { Weight::new() } + /// Execute the sanity checks of this pallet, per block. + /// + /// 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(()) + } + /// 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/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs new file mode 100644 index 0000000000000..c5ddd1cae42be --- /dev/null +++ b/frame/support/src/traits/try_runtime.rs @@ -0,0 +1,174 @@ +// This file is part of Substrate. + +// Copyright (C) 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.iter() + .map(|x| sp_std::str::from_utf8(x).unwrap_or("")) + .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 { + "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 to ensure the internal state of a pallet is consistent. +/// +/// 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. + fn try_state(_: BlockNumber, _: Select) -> Result<(), &'static str>; +} + +#[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> { + 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() % (functions.len() as u32).into(); + 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) + { + result = result.and(try_state_fn(n.clone(), targets.clone())); + } + result + }, + Select::Only(ref pallet_names) => { + 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 try_state_fns { + if pallet_names.iter().any(|n| n == name.as_bytes()) { + result = result.and(try_state_fn(n.clone(), targets.clone())); + } + } + result + }, + } + } +} diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index d9afa17eec959..ba076d963bda7 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(), diff --git a/frame/transaction-storage/Cargo.toml b/frame/transaction-storage/Cargo.toml index f001ef6acd468..8ed34cb50a652 100644 --- a/frame/transaction-storage/Cargo.toml +++ b/frame/transaction-storage/Cargo.toml @@ -48,3 +48,4 @@ std = [ "sp-std/std", "sp-transaction-storage-proof/std", ] +try-runtime = ["frame-support/try-runtime"] diff --git a/frame/try-runtime/Cargo.toml b/frame/try-runtime/Cargo.toml index 075de318c2a05..dd77c0438d71f 100644 --- a/frame/try-runtime/Cargo.toml +++ b/frame/try-runtime/Cargo.toml @@ -13,7 +13,8 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"]} +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" } @@ -21,6 +22,7 @@ sp-std = { version = "4.0.0", default-features = false, path = "../../primitives [features] default = [ "std" ] std = [ + "codec/std", "frame-support/std", "sp-api/std", "sp-runtime/std", diff --git a/frame/try-runtime/src/lib.rs b/frame/try-runtime/src/lib.rs index e4f01d40c9d42..7fec92712cd19 100644 --- a/frame/try-runtime/src/lib.rs +++ b/frame/try-runtime/src/lib.rs @@ -19,6 +19,7 @@ #![cfg_attr(not(feature = "std"), no_std)] +pub use frame_support::traits::TryStateSelect; use frame_support::weights::Weight; sp_api::decl_runtime_apis! { @@ -37,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_no_check(block: Block) -> Weight; + fn execute_block(block: Block, state_root_check: bool, try_state: TryStateSelect) -> Weight; } } 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/Cargo.toml b/utils/frame/try-runtime/cli/Cargo.toml index 4b4f9bdb2809a..dd98854e84d75 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 aee3c34a1e0e8..6a3ef24ff3771 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,17 +27,30 @@ 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 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 { /// 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_no_check` instead of - /// `Core_execute_block`. + /// If set the state root check is disabled. #[clap(long)] - no_check: bool, + no_state_root_check: bool, + + /// Which try-state targets to execute when running this command. + /// + /// 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, /// The block hash at which to fetch the block. /// @@ -70,7 +84,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, @@ -81,6 +95,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``"); @@ -123,13 +146,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 ); @@ -162,6 +186,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).encode(); let (expected_spec_name, expected_spec_version, _) = local_spec::(&ext, &executor); @@ -177,8 +202,8 @@ where &ext, &executor, execution, - if command.no_check { "TryRuntime_execute_block_no_check" } else { "Core_execute_block" }, - block.encode().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 b6a11699a3d72..01fc1dae15a05 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,22 @@ 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, + + /// Which try-state targets to execute when running this command. + /// + /// 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, } pub(crate) async fn follow_chain( @@ -141,8 +157,8 @@ where state_ext, &executor, execution, - "TryRuntime_execute_block_no_check", - block.encode().as_ref(), + "TryRuntime_execute_block", + (block, command.state_root_check, command.try_state.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 9013da95f1722..76679c43f7f14 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -335,17 +335,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_no_check`. + /// 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. ExecuteBlock(commands::execute_block::ExecuteBlockCmd), /// Executes *the offchain worker hooks* of a given block against some state. @@ -656,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); + } }, } } @@ -801,15 +808,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()),