[Fix] Try-state feature-gated for BagsList

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" ]

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());
 		})
 	});
 }

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" }

frame/bags-list/src/lib.rs

@@ -297,6 +297,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	pub fn list_bags_get(score: T::Score) -> Option<list::Bag<T, I>> {
 		ListBags::get(score)
 	}
+
+	fn do_try_state() -> Result<(), &'static str> {
+		List::<T, I>::do_try_state()
+	}
 }
 
 impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I> {
@@ -348,8 +352,9 @@ impl<T: Config<I>, I: 'static> SortedListProvider<T::AccountId> for Pallet<T, I>
 		List::<T, I>::unsafe_regenerate(all, score_of)
 	}
 
+	#[cfg(feature = "try-runtime")]
 	fn try_state() -> Result<(), &'static str> {
-		List::<T, I>::try_state()
+		Self::do_try_state()
 	}
 
 	fn unsafe_clear() {

frame/bags-list/src/list/mod.rs

@@ -220,9 +220,6 @@ impl<T: Config<I>, I: 'static> List<T, I> {
 			crate::ListBags::<T, I>::remove(removed_bag);
 		}
 
-		#[cfg(feature = "std")]
-		debug_assert_eq!(Self::try_state(), Ok(()));
-
 		num_affected
 	}
 
@@ -514,7 +511,7 @@ impl<T: Config<I>, I: 'static> List<T, I> {
 	/// * 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<T: Config<I>, I: 'static> List<T, I> {
 			thresholds.into_iter().filter_map(|t| Bag::<T, I>::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<T: Config<I>, I: 'static> List<T, I> {
 		// 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::<T, I>::iter() {
-			node.try_state()?
+			node.do_try_state()?
 		}
 
 		Ok(())
@@ -751,7 +748,7 @@ impl<T: Config<I>, I: 'static> Bag<T, I> {
 	/// * 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<T: Config<I>, I: 'static> Node<T, I> {
 		self.bag_upper
 	}
 
-	fn try_state(&self) -> Result<(), &'static str> {
+	fn do_try_state(&self) -> Result<(), &'static str> {
 		let expected_bag = Bag::<T, I>::get(self.bag_upper).ok_or("bag not found for node")?;
 
 		let id = self.id();

frame/bags-list/src/list/tests.rs

@@ -137,6 +137,7 @@ fn migrate_works() {
 			BagThresholds::set(NEW_THRESHOLDS);
 			// and we call
 			List::<Runtime>::migrate(old_thresholds);
+			assert_eq!(List::<Runtime>::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::<Runtime>::try_state());
+			assert_ok!(List::<Runtime>::do_try_state());
 		});
 
 		// make sure there are no duplicates.
 		ExtBuilder::default().build_and_execute_no_post_check(|| {
 			Bag::<Runtime>::get(10).unwrap().insert_unchecked(2, 10);
-			assert_eq!(List::<Runtime>::try_state(), Err("duplicate identified"));
+			assert_eq!(List::<Runtime>::do_try_state(), Err("duplicate identified"));
 		});
 
 		// ensure count is in sync with `ListNodes::count()`.
@@ -372,7 +373,7 @@ mod list {
 			CounterForListNodes::<Runtime>::mutate(|counter| *counter += 1);
 			assert_eq!(crate::ListNodes::<Runtime>::count(), 5);
 
-			assert_eq!(List::<Runtime>::try_state(), Err("iter_count != stored_count"));
+			assert_eq!(List::<Runtime>::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,23 +815,23 @@ 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::<Runtime>::get(&14).unwrap();
 				bag_1000.remove_node_unchecked(&node_14);
 
 				// 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::<Runtime>::get(&13).unwrap();
 				bag_1000.remove_node_unchecked(&node_13);
 
 				// 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::<Runtime>::get(&3).unwrap();
@@ -846,15 +847,15 @@ 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::<Runtime>::get(&1).unwrap();
 				bag_10.remove_node_unchecked(&node_1);
 
 				// 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,15 +866,15 @@ 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::<Runtime>::get(&18).unwrap();
 				bag_2000.remove_node_unchecked(&node_18);
 
 				// 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::<Runtime>::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));

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::<Runtime>::try_state().expect("Try-state post condition failed")
+			List::<Runtime>::do_try_state().expect("Try-state post condition failed")
 		})
 	}
 

frame/election-provider-support/Cargo.toml

@@ -43,3 +43,4 @@ std = [
 	"sp-std/std",
 ]
 runtime-benchmarks = []
+try-runtime = []

frame/election-provider-support/src/lib.rs

@@ -563,6 +563,7 @@ pub trait SortedListProvider<AccountId> {
 	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