Adding try_state hook for Core Fellowship

prdoc/pr_3276.prdoc

@@ -0,0 +1,11 @@
+title: Adding `try-state` hook to core-fellowship pallet
+
+doc:
+  - audience: Runtime User
+    description: |
+        Ranked members with zero demotion periods should not submit an evidence with a wish to retain, subsequently no promotion available for T::MaxRank members.
+        Candidate evidence should only be submitted with a Wish:Promotion.
+
+crates:
+- name: pallet-core-fellowship
+  bump: minor

substrate/frame/core-fellowship/src/lib.rs

@@ -66,6 +66,9 @@ use scale_info::TypeInfo;
 use sp_arithmetic::traits::{Saturating, Zero};
 use sp_runtime::RuntimeDebug;
 
+#[cfg(any(feature = "try-runtime", test))]
+use sp_runtime::TryRuntimeError;
+
 use frame_support::{
 	defensive,
 	dispatch::DispatchResultWithPostInfo,
@@ -316,6 +319,14 @@ pub mod pallet {
 		TooSoon,
 	}
 
+	#[pallet::hooks]
+	impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
+		#[cfg(feature = "try-runtime")]
+		fn try_state(_n: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
+			Self::do_try_state()
+		}
+	}
+
 	#[pallet::call]
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		/// Bump the state of a member.
@@ -341,7 +352,7 @@ pub mod pallet {
 			};
 
 			if demotion_period.is_zero() {
-				return Err(Error::<T, I>::NothingDoing.into())
+				return Err(Error::<T, I>::NothingDoing.into());
 			}
 
 			let demotion_block = member.last_proof.saturating_add(demotion_period);
@@ -361,7 +372,7 @@ pub mod pallet {
 					Event::<T, I>::Offboarded { who }
 				};
 				Self::deposit_event(event);
-				return Ok(Pays::No.into())
+				return Ok(Pays::No.into());
 			}
 
 			Err(Error::<T, I>::NothingDoing.into())
@@ -678,6 +689,83 @@ pub mod pallet {
 				Self::deposit_event(e);
 			}
 		}
+
+		/// Ensure the correctness of the state of this pallet
+		/// The following expectations must always apply.
+		///
+		/// ## Expectations:
+		///
+		/// `ranked_member`:
+		/// * members with zero demotion period should not submit an evidence with a wish to retain,
+		///   subsequently no promotion
+		/// available for max_rank() members.
+		///
+		/// `unranked_member`:
+		/// * evidence should only be submitted with a `Wish:Promotion`.
+		#[cfg(any(feature = "try-runtime", test))]
+		pub fn do_try_state() -> Result<(), TryRuntimeError> {
+			// Check invariants for each member tracked by the pallet
+			let params = Params::<T, I>::get();
+
+			for who in Member::<T, I>::iter_keys() {
+				if let Some(_) = Member::<T, I>::get(who.clone()) {
+					let rank =
+						T::Members::rank_of(&who).ok_or("Member not found in rank registry")?;
+
+					if rank > T::Members::min_rank() {
+						Self::check_ranked_member(rank, &who, params.clone())?;
+					} else {
+						Self::check_unranked_member(&who)?;
+					}
+				} else {
+					return Err(TryRuntimeError::Other("Member should exist, inconsistent mapping"));
+				}
+			}
+			Ok(())
+		}
+		#[cfg(any(feature = "try-runtime", test))]
+		fn check_ranked_member(
+			rank: RankOf<T, I>,
+			who: &T::AccountId,
+			params: ParamsOf<T, I>,
+		) -> Result<(), TryRuntimeError> {
+			let rank_index = Self::rank_to_index(rank).ok_or(Error::<T, I>::InvalidRank)?;
+			let demotion_period = params.demotion_period[rank_index];
+
+			// Determine if a demotion period is applicable
+			let demotion_period_applicable = demotion_period != Zero::zero();
+
+			// If member has evidence submitted, ensure it aligns with their rank and wish
+			if let Some((wish, _)) = MemberEvidence::<T, I>::get(&who) {
+				match wish {
+					Wish::Retention => {
+						ensure!(
+							demotion_period_applicable,
+							TryRuntimeError::Other(
+								"Member with no demotion period can not wish to retain"
+							)
+						);
+					},
+					Wish::Promotion => {
+						ensure!(
+							rank < <T as Config<I>>::MaxRank::get() as u16,
+							TryRuntimeError::Other("Member at max rank cannot be promoted")
+						);
+					},
+				}
+			}
+			Ok(())
+		}
+
+		#[cfg(any(feature = "try-runtime", test))]
+		fn check_unranked_member(who: &T::AccountId) -> Result<(), TryRuntimeError> {
+			if let Some((wish, _evidence)) = MemberEvidence::<T, I>::get(&who) {
+				if wish == Wish::Retention {
+					return Err(TryRuntimeError::Other("Rentention disallowed for Rank < 1"));
+				}
+			}
+			Ok(())
+		}
 	}
 
 	impl<T: Config<I>, I: 'static> GetSalary<RankOf<T, I>, T::AccountId, T::Balance> for Pallet<T, I> {

substrate/frame/core-fellowship/src/tests/unit.rs

@@ -139,6 +139,14 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	ext
 }
 
+/// Run the function pointer inside externalities and asserts the try_state hook at the end.
+pub fn build_and_execute(test: impl FnOnce() -> ()) {
+	new_test_ext().execute_with(|| {
+		test();
+		CoreFellowship::do_try_state().expect("All invariants must hold after a test");
+	});
+}
+
 fn next_block() {
 	System::set_block_number(System::block_number() + 1);
 }
@@ -159,9 +167,20 @@ fn next_demotion(who: u64) -> u64 {
 	member.last_proof + demotion_period[TestClub::rank_of(&who).unwrap() as usize - 1]
 }
 
+/// Generate mocked evidence from some seed.
+fn evidence(seed: u32) -> Evidence<Test, ()> {
+	seed.encode()
+		.into_iter()
+		.cycle()
+		.take(1024)
+		.collect::<Vec<_>>()
+		.try_into()
+		.expect("Static length matches")
+}
+
 #[test]
 fn basic_stuff() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		assert_eq!(CoreFellowship::rank_to_index(0), None);
 		assert_eq!(CoreFellowship::rank_to_index(1), Some(0));
 		assert_eq!(CoreFellowship::rank_to_index(9), Some(8));
@@ -172,7 +191,7 @@ fn basic_stuff() {
 
 #[test]
 fn set_params_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		let params = ParamsType {
 			active_salary: bounded_vec![10, 20, 30, 40, 50, 60, 70, 80, 90],
 			passive_salary: bounded_vec![1, 2, 3, 4, 5, 6, 7, 8, 9],
@@ -224,7 +243,7 @@ fn set_partial_params_works() {
 
 #[test]
 fn induct_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(0, 0);
 		assert_ok!(CoreFellowship::import(signed(0)));
 		set_rank(1, 1);
@@ -239,16 +258,16 @@ fn induct_works() {
 
 #[test]
 fn promote_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(1, 1);
+		set_rank(2, 2);
 		assert_ok!(CoreFellowship::import(signed(1)));
 		assert_noop!(CoreFellowship::promote(signed(1), 10, 1), Error::<Test>::Unranked);
 
 		assert_ok!(CoreFellowship::induct(signed(1), 10));
 		assert_noop!(CoreFellowship::promote(signed(10), 10, 1), DispatchError::BadOrigin);
 		assert_noop!(CoreFellowship::promote(signed(0), 10, 1), Error::<Test>::NoPermission);
 		assert_noop!(CoreFellowship::promote(signed(3), 10, 2), Error::<Test>::UnexpectedRank);
-		run_to(3);
 		assert_noop!(CoreFellowship::promote(signed(1), 10, 1), Error::<Test>::TooSoon);
 		run_to(4);
 		assert_ok!(CoreFellowship::promote(signed(1), 10, 1));
@@ -352,7 +371,7 @@ fn promote_fast_identical_to_promote() {
 
 #[test]
 fn sync_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(10, 5);
 		assert_noop!(CoreFellowship::approve(signed(4), 10, 5), Error::<Test>::NoPermission);
 		assert_noop!(CoreFellowship::approve(signed(6), 10, 6), Error::<Test>::UnexpectedRank);
@@ -364,7 +383,7 @@ fn sync_works() {
 
 #[test]
 fn auto_demote_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(10, 5);
 		assert_ok!(CoreFellowship::import(signed(10)));
 
@@ -380,7 +399,7 @@ fn auto_demote_works() {
 
 #[test]
 fn auto_demote_offboard_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(10, 1);
 		assert_ok!(CoreFellowship::import(signed(10)));
 
@@ -396,7 +415,7 @@ fn auto_demote_offboard_works() {
 
 #[test]
 fn offboard_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		assert_noop!(CoreFellowship::offboard(signed(0), 10), Error::<Test>::NotTracked);
 		set_rank(10, 0);
 		assert_noop!(CoreFellowship::offboard(signed(0), 10), Error::<Test>::Ranked);
@@ -413,7 +432,7 @@ fn offboard_works() {
 
 #[test]
 fn infinite_demotion_period_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		let params = ParamsType {
 			active_salary: bounded_vec![10, 10, 10, 10, 10, 10, 10, 10, 10],
 			passive_salary: bounded_vec![10, 10, 10, 10, 10, 10, 10, 10, 10],
@@ -426,6 +445,7 @@ fn infinite_demotion_period_works() {
 		set_rank(0, 0);
 		assert_ok!(CoreFellowship::import(signed(0)));
 		set_rank(1, 1);
+
 		assert_ok!(CoreFellowship::import(signed(1)));
 
 		assert_noop!(CoreFellowship::bump(signed(0), 0), Error::<Test>::NothingDoing);
@@ -435,7 +455,7 @@ fn infinite_demotion_period_works() {
 
 #[test]
 fn proof_postpones_auto_demote() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(10, 5);
 		assert_ok!(CoreFellowship::import(signed(10)));
 
@@ -448,7 +468,7 @@ fn proof_postpones_auto_demote() {
 
 #[test]
 fn promote_postpones_auto_demote() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		set_rank(10, 5);
 		assert_ok!(CoreFellowship::import(signed(10)));
 
@@ -461,7 +481,7 @@ fn promote_postpones_auto_demote() {
 
 #[test]
 fn get_salary_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		for i in 1..=9u64 {
 			set_rank(10 + i, i as u16);
 			assert_ok!(CoreFellowship::import(signed(10 + i)));
@@ -472,7 +492,7 @@ fn get_salary_works() {
 
 #[test]
 fn active_changing_get_salary_works() {
-	new_test_ext().execute_with(|| {
+	build_and_execute(|| {
 		for i in 1..=9u64 {
 			set_rank(10 + i, i as u16);
 			assert_ok!(CoreFellowship::import(signed(10 + i)));
@@ -483,3 +503,29 @@ fn active_changing_get_salary_works() {
 		}
 	});
 }
+
+#[test]
+fn submit_evidence_candidate_retention_invariant() {
+	new_test_ext().execute_with(|| {
+		use frame_support::pallet_prelude::DispatchError::Other;
+
+		let params = ParamsType {
+			active_salary: bounded_vec![10, 20, 30, 40, 50, 60, 70, 80, 90],
+			passive_salary: bounded_vec![1, 2, 3, 4, 5, 6, 7, 8, 9],
+			demotion_period: bounded_vec![1, 2, 0, 4, 5, 6, 0, 0, 9],
+			min_promotion_period: bounded_vec![1, 2, 3, 4, 5, 10, 15, 20, 30],
+			offboard_timeout: 1,
+		};
+		assert_ok!(CoreFellowship::set_params(signed(1), Box::new(params)));
+
+		set_rank(10, 0);
+		assert_ok!(CoreFellowship::import(signed(10)));
+		CoreFellowship::submit_evidence(signed(10), Wish::Retention, evidence(1)).unwrap();
+
+		// Invariant violated
+		assert_eq!(
+			CoreFellowship::do_try_state(),
+			Err(Other("Rentention disallowed for Rank < 1"))
+		);
+	})
+}