Cleanup staking try states + fix min bonds

prdoc/pr_9415.prdoc

@@ -0,0 +1,9 @@
+title: Cleanup staking try states + fix min bonds
+doc:
+- audience: Runtime Dev
+  description: ensures staking try-state code passes, and fixes issues of having a mistakenly high min-bond for validators
+crates:
+- name: pallet-nomination-pools
+  bump: patch
+- name: pallet-staking-async
+  bump: patch

substrate/frame/nomination-pools/benchmarking/src/inner.rs

@@ -966,14 +966,14 @@ mod benchmarks {
 		// Remove ed freeze to create a scenario where the ed deposit needs to be adjusted.
 		let _ = Pools::<T>::unfreeze_pool_deposit(&Pools::<T>::generate_reward_account(1));
 
-		assert!(&Pools::<T>::check_ed_imbalance().is_err());
+		assert_eq!(Pools::<T>::check_ed_imbalance().unwrap(), 1);
 
 		whitelist_account!(depositor);
 
 		#[extrinsic_call]
 		_(RuntimeOrigin::Signed(depositor), 1);
 
-		assert!(&Pools::<T>::check_ed_imbalance().is_ok());
+		assert_eq!(Pools::<T>::check_ed_imbalance().unwrap(), 0);
 	}
 
 	#[benchmark]

substrate/frame/nomination-pools/src/lib.rs

@@ -3930,7 +3930,8 @@ impl<T: Config> Pallet<T> {
 			// If this happens, this is most likely due to an old bug and not a recent code change.
 			// We warn about this in try-runtime checks but do not panic.
 			if !pending_rewards_lt_leftover_bal {
-				log::warn!(
+				log!(
+					warn,
 					"pool {:?}, sum pending rewards = {:?}, remaining balance = {:?}",
 					id,
 					pools_members_pending_rewards.get(&id),
@@ -3955,12 +3956,19 @@ impl<T: Config> Pallet<T> {
 			);
 
 			let depositor = PoolMembers::<T>::get(&bonded_pool.roles.depositor).unwrap();
-			ensure!(
-				bonded_pool.is_destroying_and_only_depositor(depositor.active_points()) ||
-					depositor.active_points() >= MinCreateBond::<T>::get(),
-				"depositor must always have MinCreateBond stake in the pool, except for when the \
-				pool is being destroyed and the depositor is the last member",
-			);
+			let depositor_has_enough_stake = bonded_pool
+				.is_destroying_and_only_depositor(depositor.active_points()) ||
+				depositor.active_points() >= MinCreateBond::<T>::get();
+			if !depositor_has_enough_stake {
+				log!(
+					warn,
+					"pool {:?} has depositor {:?} with insufficient stake {:?}, minimum required is {:?}",
+					id,
+					bonded_pool.roles.depositor,
+					depositor.active_points(),
+					MinCreateBond::<T>::get()
+				);
+			}
 
 			ensure!(
 				bonded_pool.points >= bonded_pool.points_to_balance(bonded_pool.points),
@@ -4018,7 +4026,7 @@ impl<T: Config> Pallet<T> {
 
 		// Warn if any pool has incorrect ED frozen. We don't want to fail hard as this could be a
 		// result of an intentional ED change.
-		Self::check_ed_imbalance()?;
+		let _needs_adjust = Self::check_ed_imbalance()?;
 
 		Ok(())
 	}
@@ -4033,27 +4041,27 @@ impl<T: Config> Pallet<T> {
 		test,
 		debug_assertions
 	))]
-	pub fn check_ed_imbalance() -> Result<(), DispatchError> {
-		let mut failed: u32 = 0;
+	pub fn check_ed_imbalance() -> Result<u32, DispatchError> {
+		let mut needs_adjust = 0;
 		BondedPools::<T>::iter_keys().for_each(|id| {
 			let reward_acc = Self::generate_reward_account(id);
 			let frozen_balance =
 				T::Currency::balance_frozen(&FreezeReason::PoolMinBalance.into(), &reward_acc);
 
 			let expected_frozen_balance = T::Currency::minimum_balance();
 			if frozen_balance != expected_frozen_balance {
-				failed += 1;
-				log::warn!(
-					"pool {:?} has incorrect ED frozen that can result from change in ED. Expected  = {:?},  Actual = {:?}",
+				needs_adjust += 1;
+				log!(
+					warn,
+					"pool {:?} has incorrect ED frozen that can result from change in ED. Expected  = {:?},  Actual = {:?}. Use `adjust_pool_deposit` to fix it",
 					id,
 					expected_frozen_balance,
 					frozen_balance,
 				);
 			}
 		});
 
-		ensure!(failed == 0, "Some pools do not have correct ED frozen");
-		Ok(())
+		Ok(needs_adjust)
 	}
 	/// Fully unbond the shares of `member`, when executed from `origin`.
 	///

substrate/frame/nomination-pools/src/migration.rs

@@ -442,7 +442,7 @@ mod v6 {
 		#[cfg(feature = "try-runtime")]
 		fn post_upgrade(_data: Vec<u8>) -> Result<(), TryRuntimeError> {
 			// there should be no ED imbalances anymore..
-			Pallet::<T>::check_ed_imbalance()
+			Pallet::<T>::check_ed_imbalance().map(|_| ())
 		}
 	}
 }

substrate/frame/staking-async/src/pallet/impls.rs

@@ -85,14 +85,12 @@ impl<T: Config> Pallet<T> {
 			.max(asset::existential_deposit::<T>())
 	}
 
-	/// Returns the minimum required bond for validators, defaulting to `MinNominatorBond` if not
-	/// set or less than `MinNominatorBond`.
+	/// Returns the minimum required bond for participation in staking as a validator account.
 	pub(crate) fn min_validator_bond() -> BalanceOf<T> {
-		MinValidatorBond::<T>::get().max(Self::min_nominator_bond())
+		MinValidatorBond::<T>::get().max(asset::existential_deposit::<T>())
 	}
 
-	/// Returns the minimum required bond for nominators, considering the chain’s existential
-	/// deposit.
+	/// Returns the minimum required bond for participation in staking as a nominator account.
 	pub(crate) fn min_nominator_bond() -> BalanceOf<T> {
 		MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>())
 	}
@@ -2086,21 +2084,40 @@ impl<T: Config> Pallet<T> {
 
 		match (is_nominator, is_validator) {
 			(false, false) => {
-				if ledger.active < Self::min_chilled_bond() {
-					log!(warn, "Chilled stash {:?} has less than minimum bond", stash);
+				if ledger.active < Self::min_chilled_bond() && !ledger.active.is_zero() {
+					// chilled accounts allow to go to zero and fully unbond ^^^^^^^^^
+					log!(
+						warn,
+						"Chilled stash {:?} has less stake ({:?}) than minimum role bond ({:?})",
+						stash,
+						ledger.active,
+						Self::min_chilled_bond()
+					);
 				}
 				// is chilled
 			},
 			(true, false) => {
 				// Nominators must have a minimum bond.
 				if ledger.active < Self::min_nominator_bond() {
-					log!(warn, "Nominator {:?} has less than minimum bond", stash);
+					log!(
+						warn,
+						"Nominator {:?} has less stake ({:?}) than minimum role bond ({:?})",
+						stash,
+						ledger.active,
+						Self::min_nominator_bond()
+					);
 				}
 			},
 			(false, true) => {
 				// Validators must have a minimum bond.
 				if ledger.active < Self::min_validator_bond() {
-					log!(warn, "Validator {:?} has less than minimum bond", stash);
+					log!(
+						warn,
+						"Validator {:?} has less stake ({:?}) than minimum role bond ({:?})",
+						stash,
+						ledger.active,
+						Self::min_validator_bond()
+					);
 				}
 			},
 			(true, true) => {

substrate/frame/staking-async/src/pallet/mod.rs

@@ -2026,9 +2026,8 @@ pub mod pallet {
 		/// Remove all data structures concerning a staker/stash once it is at a state where it can
 		/// be considered `dust` in the staking system. The requirements are:
 		///
-		/// 1. the `total_balance` of the stash is below minimum bond.
-		/// 2. or, the `ledger.total` of the stash is below minimum bond.
-		/// 3. or, existential deposit is zero and either `total_balance` or `ledger.total` is zero.
+		/// 1. the `total_balance` of the stash is below `min_chilled_bond` or is zero.
+		/// 2. or, the `ledger.total` of the stash is below `min_chilled_bond` or is zero.
 		///
 		/// The former can happen in cases like a slash; the latter when a fully unbonded account
 		/// is still receiving staking rewards in `RewardDestination::Staked`.

substrate/frame/staking-async/src/tests/bonding.rs

@@ -1387,8 +1387,6 @@ mod reap {
 					Error::<Test>::FundedTarget
 				);
 
-				// Note: Even though the stash is a validator, the threshold to reap is min of
-				// nominator and validator bond
 				// no easy way to cause an account to go below ED, we tweak their staking ledger
 				// instead.
 
@@ -1401,8 +1399,8 @@ mod reap {
 					Error::<Test>::FundedTarget
 				);
 
-				// WHEN: set ledger to below min nominator bond.
-				Ledger::<Test>::insert(11, StakingLedger::<Test>::new(11, 999));
+				// WHEN: set ledger to below ED
+				Ledger::<Test>::insert(11, StakingLedger::<Test>::new(11, 9));
 
 				// THEN: reap-able
 				assert_ok!(Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0));
@@ -1551,9 +1549,9 @@ mod staking_bounds_chill_other {
 			.min_nominator_bond(1_000)
 			.min_validator_bond(1_500)
 			.build_and_execute(|| {
-				// 500 is not enough for any role
+				// 50 is not enough for any role (less than ED)
 				assert_noop!(
-					Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Stash),
+					Staking::bond(RuntimeOrigin::signed(3), 50, RewardDestination::Stash),
 					Error::<Test>::InsufficientBond
 				);
 				// 1000 is enough for nominator but not for validator.