From 241a5e1aeaf41ce2630078dd6dd5b13ee9c4b005 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 2 Feb 2023 12:15:13 +0100 Subject: [PATCH 01/15] [Fix] Try-state feature-gated for BagsList --- frame/bags-list/Cargo.toml | 2 +- frame/bags-list/fuzzer/src/main.rs | 2 +- frame/bags-list/remote-tests/Cargo.toml | 2 +- frame/bags-list/src/lib.rs | 7 +++++- frame/bags-list/src/list/mod.rs | 13 +++++------ frame/bags-list/src/list/tests.rs | 25 +++++++++++----------- frame/bags-list/src/mock.rs | 2 +- frame/election-provider-support/Cargo.toml | 1 + frame/election-provider-support/src/lib.rs | 1 + 9 files changed, 30 insertions(+), 25 deletions(-) diff --git a/frame/bags-list/Cargo.toml b/frame/bags-list/Cargo.toml index e3a4965f6c086..379698b1d39e1 100644 --- a/frame/bags-list/Cargo.toml +++ b/frame/bags-list/Cargo.toml @@ -75,4 +75,4 @@ fuzz = [ "sp-tracing", "frame-election-provider-support/fuzz", ] -try-runtime = [ "frame-support/try-runtime" ] +try-runtime = [ "frame-support/try-runtime", "frame-election-provider-support/try-runtime" ] diff --git a/frame/bags-list/fuzzer/src/main.rs b/frame/bags-list/fuzzer/src/main.rs index 9f7ca464cc2b8..c78e2a13076d5 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::try_state().is_ok()); + assert!(BagsList::do_try_state().is_ok()); }) }); } diff --git a/frame/bags-list/remote-tests/Cargo.toml b/frame/bags-list/remote-tests/Cargo.toml index 9fb6d0154b302..eebf7fb883469 100644 --- a/frame/bags-list/remote-tests/Cargo.toml +++ b/frame/bags-list/remote-tests/Cargo.toml @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] # frame pallet-staking = { path = "../../staking", version = "4.0.0-dev" } -pallet-bags-list = { path = "../../bags-list", version = "4.0.0-dev" } +pallet-bags-list = { path = "../../bags-list", version = "4.0.0-dev", features = ["try-runtime", "default"] } 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" } diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 14f8a613eb798..9070c68eade40 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -297,6 +297,10 @@ impl, I: 'static> Pallet { pub fn list_bags_get(score: T::Score) -> Option> { ListBags::get(score) } + + fn do_try_state() -> Result<(), &'static str> { + List::::do_try_state() + } } impl, I: 'static> SortedListProvider for Pallet { @@ -348,8 +352,9 @@ impl, I: 'static> SortedListProvider for Pallet List::::unsafe_regenerate(all, score_of) } + #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str> { - List::::try_state() + Self::do_try_state() } fn unsafe_clear() { diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 272526ad1a636..b0e2545b6b616 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -220,9 +220,6 @@ impl, I: 'static> List { crate::ListBags::::remove(removed_bag); } - #[cfg(feature = "std")] - debug_assert_eq!(Self::try_state(), Ok(())); - num_affected } @@ -514,7 +511,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 try_state() -> Result<(), &'static str> { + pub(crate) fn do_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)), @@ -542,7 +539,7 @@ impl, I: 'static> List { thresholds.into_iter().filter_map(|t| Bag::::get(t)) }; - let _ = active_bags.clone().try_for_each(|b| b.try_state())?; + let _ = active_bags.clone().try_for_each(|b| b.do_try_state())?; let nodes_in_bags_count = active_bags.clone().fold(0u32, |acc, cur| acc + cur.iter().count() as u32); @@ -553,7 +550,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.try_state()? + node.do_try_state()? } Ok(()) @@ -751,7 +748,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 try_state(&self) -> Result<(), &'static str> { + fn do_try_state(&self) -> Result<(), &'static str> { frame_support::ensure!( self.head() .map(|head| head.prev().is_none()) @@ -894,7 +891,7 @@ impl, I: 'static> Node { self.bag_upper } - fn try_state(&self) -> Result<(), &'static str> { + fn do_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 966ea1a74c71c..3c4aa7c86634d 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -137,6 +137,7 @@ fn migrate_works() { BagThresholds::set(NEW_THRESHOLDS); // and we call List::::migrate(old_thresholds); + assert_eq!(List::::do_try_state(), Ok(())); // then assert_eq!( @@ -352,13 +353,13 @@ mod list { #[test] fn try_state_works() { ExtBuilder::default().build_and_execute_no_post_check(|| { - assert_ok!(List::::try_state()); + assert_ok!(List::::do_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::::try_state(), Err("duplicate identified")); + assert_eq!(List::::do_try_state(), Err("duplicate identified")); }); // ensure count is in sync with `ListNodes::count()`. @@ -372,7 +373,7 @@ mod list { CounterForListNodes::::mutate(|counter| *counter += 1); assert_eq!(crate::ListNodes::::count(), 5); - assert_eq!(List::::try_state(), Err("iter_count != stored_count")); + assert_eq!(List::::do_try_state(), Err("iter_count != stored_count")); }); } @@ -804,7 +805,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![2, 3, 13, 14]); - assert_ok!(bag_1000.try_state()); + assert_ok!(bag_1000.do_try_state()); // and the node isn't mutated when its removed assert_eq!(node_4, node_4_pre_remove); @@ -814,7 +815,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![3, 13, 14]); - assert_ok!(bag_1000.try_state()); + assert_ok!(bag_1000.do_try_state()); // when removing a tail that is not pointing at the head let node_14 = Node::::get(&14).unwrap(); @@ -822,7 +823,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![3, 13]); - assert_ok!(bag_1000.try_state()); + assert_ok!(bag_1000.do_try_state()); // when removing a tail that is pointing at the head let node_13 = Node::::get(&13).unwrap(); @@ -830,7 +831,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_1000), vec![3]); - assert_ok!(bag_1000.try_state()); + assert_ok!(bag_1000.do_try_state()); // when removing a node that is both the head & tail let node_3 = Node::::get(&3).unwrap(); @@ -846,7 +847,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_10), vec![1, 12]); - assert_ok!(bag_10.try_state()); + assert_ok!(bag_10.do_try_state()); // when removing a head that is pointing at the tail let node_1 = Node::::get(&1).unwrap(); @@ -854,7 +855,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_10), vec![12]); - assert_ok!(bag_10.try_state()); + assert_ok!(bag_10.do_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 +866,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_2000), vec![15, 17, 18, 19]); - assert_ok!(bag_2000.try_state()); + assert_ok!(bag_2000.do_try_state()); // when removing a node that is pointing at tail, but not head let node_18 = Node::::get(&18).unwrap(); @@ -873,7 +874,7 @@ mod bags { // then assert_eq!(bag_as_ids(&bag_2000), vec![15, 17, 19]); - assert_ok!(bag_2000.try_state()); + assert_ok!(bag_2000.do_try_state()); // finally, when reading from storage, the state of all bags is as expected assert_eq!( @@ -905,7 +906,7 @@ mod bags { // .. and the bag it was removed from let bag_1000 = Bag::::get(1_000).unwrap(); // is sane - assert_ok!(bag_1000.try_state()); + assert_ok!(bag_1000.do_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 8cc96a988e72a..4095cee888748 100644 --- a/frame/bags-list/src/mock.rs +++ b/frame/bags-list/src/mock.rs @@ -148,7 +148,7 @@ impl ExtBuilder { pub fn build_and_execute(self, test: impl FnOnce() -> ()) { self.build().execute_with(|| { test(); - List::::try_state().expect("Try-state post condition failed") + List::::do_try_state().expect("Try-state post condition failed") }) } diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index c7f47e721d1aa..114caee793f1a 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -43,3 +43,4 @@ std = [ "sp-std/std", ] runtime-benchmarks = [] +try-runtime = [] diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 9d5d6c018e5e1..30d4112edd20b 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -563,6 +563,7 @@ pub trait SortedListProvider { fn unsafe_clear(); /// Check internal state of list. Only meant for debugging. + #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str>; /// If `who` changes by the returned amount they are guaranteed to have a worst case change From 260c12cd10c988404ef8c509dafcc2a17d26c853 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 15:38:16 +0100 Subject: [PATCH 02/15] fix comment --- frame/election-provider-support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 30d4112edd20b..9e60eb3be1a6f 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -562,7 +562,7 @@ pub trait SortedListProvider { /// unbounded amount of storage accesses. fn unsafe_clear(); - /// Check internal state of list. Only meant for debugging. + /// Check internal state of the list. Only meant for debugging. #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str>; From a973fb2de91c264ecc34d9c06e4db306f3de740e Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 16:12:17 +0100 Subject: [PATCH 03/15] fix try_state remote-tests --- frame/bags-list/remote-tests/Cargo.toml | 2 +- frame/bags-list/remote-tests/src/try_state.rs | 2 +- frame/bags-list/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/bags-list/remote-tests/Cargo.toml b/frame/bags-list/remote-tests/Cargo.toml index eebf7fb883469..9fb6d0154b302 100644 --- a/frame/bags-list/remote-tests/Cargo.toml +++ b/frame/bags-list/remote-tests/Cargo.toml @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] # frame pallet-staking = { path = "../../staking", version = "4.0.0-dev" } -pallet-bags-list = { path = "../../bags-list", version = "4.0.0-dev", features = ["try-runtime", "default"] } +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" } diff --git a/frame/bags-list/remote-tests/src/try_state.rs b/frame/bags-list/remote-tests/src/try_state.rs index 514c80d72ab67..d27bb8b8974fb 100644 --- a/frame/bags-list/remote-tests/src/try_state.rs +++ b/frame/bags-list/remote-tests/src/try_state.rs @@ -51,7 +51,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::::try_state().unwrap(); + pallet_bags_list::Pallet::::do_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 9070c68eade40..3fe159443e902 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -298,7 +298,7 @@ impl, I: 'static> Pallet { ListBags::get(score) } - fn do_try_state() -> Result<(), &'static str> { + pub fn do_try_state() -> Result<(), &'static str> { List::::do_try_state() } } From adf84ab1ba5c53cf24391bf398da582cbc340918 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 16:15:32 +0100 Subject: [PATCH 04/15] feature-gate try-state remote test for bags-list --- frame/bags-list/remote-tests/src/try_state.rs | 5 ++++- frame/bags-list/src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/bags-list/remote-tests/src/try_state.rs b/frame/bags-list/remote-tests/src/try_state.rs index d27bb8b8974fb..44c40a8ae66bd 100644 --- a/frame/bags-list/remote-tests/src/try_state.rs +++ b/frame/bags-list/remote-tests/src/try_state.rs @@ -51,7 +51,10 @@ pub async fn execute( ext.execute_with(|| { sp_core::crypto::set_default_ss58_version(Runtime::SS58Prefix::get().try_into().unwrap()); - pallet_bags_list::Pallet::::do_try_state().unwrap(); + + #[cfg(feature = "try-runtime")] + 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 3fe159443e902..9070c68eade40 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -298,7 +298,7 @@ impl, I: 'static> Pallet { ListBags::get(score) } - pub fn do_try_state() -> Result<(), &'static str> { + fn do_try_state() -> Result<(), &'static str> { List::::do_try_state() } } From 4cbf5a5669ca23449e889dbd7badcbe5a8f1b3d4 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 16:24:49 +0100 Subject: [PATCH 05/15] remove try-state from a migration --- frame/staking/src/migrations.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 6253c3feed17d..a8d360c098137 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -386,7 +386,6 @@ pub mod v8 { Nominators::::iter().map(|(id, _)| id), Pallet::::weight_of_fn(), ); - debug_assert_eq!(T::VoterList::try_state(), Ok(())); StorageVersion::::put(ObsoleteReleases::V8_0_0); crate::log!( From 3cecdf1e7f04e9738bddac4ff7269f4b9ef6f1c6 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 16:33:15 +0100 Subject: [PATCH 06/15] more SortedListProvider fixes --- frame/staking/Cargo.toml | 2 +- frame/staking/src/pallet/impls.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/frame/staking/Cargo.toml b/frame/staking/Cargo.toml index 3d5cf1161e8e5..79c0bb5c2a32d 100644 --- a/frame/staking/Cargo.toml +++ b/frame/staking/Cargo.toml @@ -74,4 +74,4 @@ runtime-benchmarks = [ "rand_chacha", "sp-staking/runtime-benchmarks", ] -try-runtime = ["frame-support/try-runtime"] +try-runtime = ["frame-support/try-runtime", "frame-election-provider-support/try-runtime"] diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index e59c2c5a0620e..6a35e2b861565 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1451,9 +1451,11 @@ impl SortedListProvider for UseValidatorsMap { // nothing to do upon regenerate. 0 } + #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str> { Ok(()) } + fn unsafe_clear() { #[allow(deprecated)] Validators::::remove_all(); @@ -1525,6 +1527,8 @@ impl SortedListProvider for UseNominatorsAndValidatorsM // nothing to do upon regenerate. 0 } + + #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str> { Ok(()) } From 04ef619e69b774119d2a22bd52a0822a4d2ee076 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 16:48:40 +0100 Subject: [PATCH 07/15] more fixes --- frame/bags-list/src/lib.rs | 6 +----- frame/bags-list/src/list/mod.rs | 5 +++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 9070c68eade40..16b96e19dee76 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -297,10 +297,6 @@ impl, I: 'static> Pallet { pub fn list_bags_get(score: T::Score) -> Option> { ListBags::get(score) } - - fn do_try_state() -> Result<(), &'static str> { - List::::do_try_state() - } } impl, I: 'static> SortedListProvider for Pallet { @@ -354,7 +350,7 @@ impl, I: 'static> SortedListProvider for Pallet #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str> { - Self::do_try_state() + List::::do_try_state() } fn unsafe_clear() { diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index b0e2545b6b616..5b45ebd368eff 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -511,6 +511,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(any(test, feature = "try-runtime"))] pub(crate) fn do_try_state() -> Result<(), &'static str> { let mut seen_in_list = BTreeSet::new(); ensure!( @@ -748,6 +749,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(any(test, feature = "try-runtime"))] fn do_try_state(&self) -> Result<(), &'static str> { frame_support::ensure!( self.head() @@ -787,6 +789,8 @@ impl, I: 'static> Bag { } /// Check if the bag contains a node with `id`. + #[cfg(feature = "std")] + #[allow(dead_code)] fn contains(&self, id: &T::AccountId) -> bool { self.iter().any(|n| n.id() == id) } @@ -891,6 +895,7 @@ impl, I: 'static> Node { self.bag_upper } + #[cfg(any(test, feature = "try-runtime"))] fn do_try_state(&self) -> Result<(), &'static str> { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; From c0b1676615beadcff1517236a98be5bceed53801 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 17:01:48 +0100 Subject: [PATCH 08/15] more fixes to allow do_try_state usage in other crates --- frame/bags-list/src/list/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 5b45ebd368eff..bcb1142175a4c 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -511,7 +511,8 @@ 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(test, feature = "try-runtime"))] + #[cfg(feature = "std")] + #[allow(dead_code)] pub(crate) fn do_try_state() -> Result<(), &'static str> { let mut seen_in_list = BTreeSet::new(); ensure!( @@ -749,7 +750,8 @@ 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(test, feature = "try-runtime"))] + #[cfg(feature = "std")] + #[allow(dead_code)] fn do_try_state(&self) -> Result<(), &'static str> { frame_support::ensure!( self.head() @@ -895,7 +897,8 @@ impl, I: 'static> Node { self.bag_upper } - #[cfg(any(test, feature = "try-runtime"))] + #[cfg(feature = "std")] + #[allow(dead_code)] fn do_try_state(&self) -> Result<(), &'static str> { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; From 9c0f681f81e3a79d477553f7e97b7274beda6aef Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 17:52:50 +0100 Subject: [PATCH 09/15] do-try-state for fuzz --- frame/bags-list/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 16b96e19dee76..f04c685ae386c 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -274,6 +274,13 @@ pub mod pallet { } } +#[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] +impl, I: 'static> Pallet { + pub fn do_try_state() -> Result<(), &'static str> { + List::::do_try_state() + } +} + impl, I: 'static> Pallet { /// Move an account from one bag to another, depositing an event on success. /// @@ -350,7 +357,7 @@ impl, I: 'static> SortedListProvider for Pallet #[cfg(feature = "try-runtime")] fn try_state() -> Result<(), &'static str> { - List::::do_try_state() + Self::do_try_state() } fn unsafe_clear() { From ee955889f18c14d2d863f33d32dfd2368ccbf5b3 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 18:00:18 +0100 Subject: [PATCH 10/15] more fixes --- frame/bags-list/src/list/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index bcb1142175a4c..498f46e8d722a 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -511,7 +511,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")] + #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] #[allow(dead_code)] pub(crate) fn do_try_state() -> Result<(), &'static str> { let mut seen_in_list = BTreeSet::new(); From e0af072f7000013e6a582e9663642fa41e468de1 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 18:14:29 +0100 Subject: [PATCH 11/15] more fixes --- frame/bags-list/src/list/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 498f46e8d722a..ddda1268d0eed 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -750,8 +750,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")] - #[allow(dead_code)] + #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] fn do_try_state(&self) -> Result<(), &'static str> { frame_support::ensure!( self.head() @@ -897,8 +896,7 @@ impl, I: 'static> Node { self.bag_upper } - #[cfg(feature = "std")] - #[allow(dead_code)] + #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] fn do_try_state(&self) -> Result<(), &'static str> { let expected_bag = Bag::::get(self.bag_upper).ok_or("bag not found for node")?; From 79a9d903eff53c3698dc2bc098991b752f69b574 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 18:22:38 +0100 Subject: [PATCH 12/15] remove feature-flag --- frame/bags-list/src/list/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index ddda1268d0eed..c3502eb9cc5fa 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -790,7 +790,6 @@ impl, I: 'static> Bag { } /// Check if the bag contains a node with `id`. - #[cfg(feature = "std")] #[allow(dead_code)] fn contains(&self, id: &T::AccountId) -> bool { self.iter().any(|n| n.id() == id) From 13f879689f357a2b32a7499c68b33c09e6eabeed Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sun, 5 Feb 2023 18:36:49 +0100 Subject: [PATCH 13/15] do-try-state --- frame/bags-list/remote-tests/Cargo.toml | 2 +- frame/bags-list/remote-tests/src/try_state.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/bags-list/remote-tests/Cargo.toml b/frame/bags-list/remote-tests/Cargo.toml index 9fb6d0154b302..6e951b43a4aeb 100644 --- a/frame/bags-list/remote-tests/Cargo.toml +++ b/frame/bags-list/remote-tests/Cargo.toml @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] # frame pallet-staking = { path = "../../staking", version = "4.0.0-dev" } -pallet-bags-list = { path = "../../bags-list", version = "4.0.0-dev" } +pallet-bags-list = { path = "../../bags-list", version = "4.0.0-dev", features = ["fuzz"] } 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" } diff --git a/frame/bags-list/remote-tests/src/try_state.rs b/frame/bags-list/remote-tests/src/try_state.rs index 44c40a8ae66bd..9ed877a43afe1 100644 --- a/frame/bags-list/remote-tests/src/try_state.rs +++ b/frame/bags-list/remote-tests/src/try_state.rs @@ -16,7 +16,6 @@ //! Test to execute the sanity-check of the voter bag. -use frame_election_provider_support::SortedListProvider; use frame_support::{ storage::generator::StorageMap, traits::{Get, PalletInfoAccess}, @@ -52,8 +51,7 @@ pub async fn execute( ext.execute_with(|| { sp_core::crypto::set_default_ss58_version(Runtime::SS58Prefix::get().try_into().unwrap()); - #[cfg(feature = "try-runtime")] - pallet_bags_list::Pallet::::try_state().unwrap(); + pallet_bags_list::Pallet::::do_try_state().unwrap(); log::info!(target: crate::LOG_TARGET, "executed bags-list sanity check with no errors."); From f97283ce64e7edf1bb8a2e63859ff3552aef2753 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 8 Feb 2023 15:34:01 +0100 Subject: [PATCH 14/15] fix review comments --- frame/bags-list/src/list/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index c3502eb9cc5fa..4082a5324091b 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -512,7 +512,6 @@ impl, I: 'static> List { /// * and sanity-checks all bags and nodes. This will cascade down all the checks and makes sure /// all bags and nodes are checked per *any* update to `List`. #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] - #[allow(dead_code)] pub(crate) fn do_try_state() -> Result<(), &'static str> { let mut seen_in_list = BTreeSet::new(); ensure!( @@ -790,7 +789,7 @@ impl, I: 'static> Bag { } /// Check if the bag contains a node with `id`. - #[allow(dead_code)] + #[cfg(any(test, feature = "try-runtime", feature = "fuzz"))] fn contains(&self, id: &T::AccountId) -> bool { self.iter().any(|n| n.id() == id) } From 3bf424f5e29477b6db1a2f053f5a482c985ed028 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 9 Feb 2023 12:16:27 +0100 Subject: [PATCH 15/15] Update frame/bags-list/src/mock.rs Co-authored-by: Anton --- frame/bags-list/src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bags-list/src/mock.rs b/frame/bags-list/src/mock.rs index 4095cee888748..32f6bb09e4772 100644 --- a/frame/bags-list/src/mock.rs +++ b/frame/bags-list/src/mock.rs @@ -148,7 +148,7 @@ impl ExtBuilder { pub fn build_and_execute(self, test: impl FnOnce() -> ()) { self.build().execute_with(|| { test(); - List::::do_try_state().expect("Try-state post condition failed") + List::::do_try_state().expect("do_try_state post condition failed") }) }