PR feedback

Author: Ank4n

prdoc/pr_8701.prdoc

@@ -229,7 +229,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup the worst case list scenario.
 
@@ -318,7 +318,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup a worst case list scenario. Note that we don't care about the setup of the
 		// destination position because we are doing a removal from the list but no insert.
@@ -442,7 +442,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup a worst case list scenario. Note we don't care about the destination position,
 		// because we are just doing an insert into the origin position.
@@ -475,7 +475,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup a worst case list scenario. Note that we don't care about the setup of the
 		// destination position because we are doing a removal from the list but no insert.
@@ -633,7 +633,7 @@ mod benchmarks {
 		// Clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup a worst case list scenario. Note that we don't care about the setup of the
 		// destination position because we are doing a removal from the list but no insert.
@@ -734,8 +734,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get()
-			.max(asset::existential_deposit::<T>())
+		let origin_weight = Pallet::<T>::min_nominator_bond()
 			// we use 100 to play friendly with the list threshold values in the mock
 			.max(100u32.into());
 
@@ -781,7 +780,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup a worst case list scenario. Note that we don't care about the setup of the
 		// destination position because we are doing a removal from the list but no insert.
@@ -858,7 +857,7 @@ mod benchmarks {
 		// clean up any existing state.
 		clear_validators_and_nominators::<T>();
 
-		let origin_weight = MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>());
+		let origin_weight = Staking::<T>::min_nominator_bond();
 
 		// setup a worst case list scenario. Note that we don't care about the setup of the
 		// destination position because we are doing a removal from the list but no insert.

substrate/frame/staking-async/src/benchmarking.rs

@@ -72,18 +72,30 @@ use sp_runtime::TryRuntimeError;
 const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2;
 
 impl<T: Config> Pallet<T> {
-	/// Returns the minimum required bond for participation, considering validators, nominators,
+	/// Returns the minimum required bond for participation, considering nominators,
 	/// and the chain’s existential deposit.
 	///
 	/// This function computes the smallest allowed bond among `MinValidatorBond` and
 	/// `MinNominatorBond`, but ensures it is not below the existential deposit required to keep an
 	/// account alive.
-	pub(crate) fn min_bond() -> BalanceOf<T> {
+	pub(crate) fn min_chilled_bond() -> BalanceOf<T> {
 		MinValidatorBond::<T>::get()
 			.min(MinNominatorBond::<T>::get())
 			.max(asset::existential_deposit::<T>())
 	}
 
+	/// Returns the minimum required bond for validators, defaulting to `MinNominatorBond` if not
+	/// set or less than `MinNominatorBond`.
+	pub(crate) fn min_validator_bond() -> BalanceOf<T> {
+		MinValidatorBond::<T>::get().max(Self::min_nominator_bond())
+	}
+
+	/// Returns the minimum required bond for nominators, considering the chain’s existential
+	/// deposit.
+	pub(crate) fn min_nominator_bond() -> BalanceOf<T> {
+		MinNominatorBond::<T>::get().max(asset::existential_deposit::<T>())
+	}
+
 	/// Fetches the ledger associated with a controller or stash account, if any.
 	pub fn ledger(account: StakingAccount<T::AccountId>) -> Result<StakingLedger<T>, Error<T>> {
 		StakingLedger::<T>::get(account)
@@ -182,7 +194,7 @@ impl<T: Config> Pallet<T> {
 		ledger.total = ledger.total.checked_add(&extra).ok_or(ArithmeticError::Overflow)?;
 		ledger.active = ledger.active.checked_add(&extra).ok_or(ArithmeticError::Overflow)?;
 		// last check: the new active amount of ledger must be more than min bond.
-		ensure!(ledger.active >= Self::min_bond(), Error::<T>::InsufficientBond);
+		ensure!(ledger.active >= Self::min_chilled_bond(), Error::<T>::InsufficientBond);
 
 		// NOTE: ledger must be updated prior to calling `Self::weight_of`.
 		ledger.update()?;
@@ -206,7 +218,7 @@ impl<T: Config> Pallet<T> {
 		let new_total = ledger.total;
 
 		let used_weight = if ledger.unlocking.is_empty() &&
-			(ledger.active < Self::min_bond() || ledger.active.is_zero())
+			(ledger.active < Self::min_chilled_bond() || ledger.active.is_zero())
 		{
 			// This account must have called `unbond()` with some value that caused the active
 			// portion to fall below existential deposit + will have no more unlocking chunks
@@ -984,7 +996,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
 
 	#[cfg(feature = "runtime-benchmarks")]
 	fn add_target(target: T::AccountId) {
-		let stake = (MinValidatorBond::<T>::get() + 1u32.into()) * 100u32.into();
+		let stake = (Self::min_validator_bond() + 1u32.into()) * 100u32.into();
 		<Bonded<T>>::insert(target.clone(), target.clone());
 		<Ledger<T>>::insert(target.clone(), StakingLedger::<T>::new(target.clone(), stake));
 		Self::do_add_validator(
@@ -1016,7 +1028,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
 		targets.into_iter().for_each(|v| {
 			let stake: BalanceOf<T> = target_stake
 				.and_then(|w| <BalanceOf<T>>::try_from(w).ok())
-				.unwrap_or_else(|| MinNominatorBond::<T>::get() * 100u32.into());
+				.unwrap_or_else(|| Self::min_nominator_bond() * 100u32.into());
 			<Bonded<T>>::insert(v.clone(), v.clone());
 			<Ledger<T>>::insert(v.clone(), StakingLedger::<T>::new(v.clone(), stake));
 			Self::do_add_validator(
@@ -1437,11 +1449,11 @@ impl<T: Config> StakingInterface for Pallet<T> {
 	type CurrencyToVote = T::CurrencyToVote;
 
 	fn minimum_nominator_bond() -> Self::Balance {
-		MinNominatorBond::<T>::get()
+		Self::min_nominator_bond()
 	}
 
 	fn minimum_validator_bond() -> Self::Balance {
-		MinValidatorBond::<T>::get()
+		Self::min_validator_bond()
 	}
 
 	fn stash_by_ctrl(controller: &Self::AccountId) -> Result<Self::AccountId, DispatchError> {

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

@@ -1150,7 +1150,7 @@ pub mod pallet {
 		DuplicateIndex,
 		/// Slash record not found.
 		InvalidSlashRecord,
-		/// Cannot have a validator or nominator role, with value less than the minimum defined by
+		/// Cannot bond, nominate or validate with value less than the minimum defined by
 		/// governance (see `MinValidatorBond` and `MinNominatorBond`). If unbonding is the
 		/// intention, `chill` first to remove one's role as validator/nominator.
 		InsufficientBond,
@@ -1308,7 +1308,7 @@ pub mod pallet {
 			}
 
 			// Reject a bond which is lower than the minimum bond.
-			if value < Self::min_bond() {
+			if value < Self::min_chilled_bond() {
 				return Err(Error::<T>::InsufficientBond.into());
 			}
 
@@ -1407,9 +1407,9 @@ pub mod pallet {
 				}
 
 				let min_active_bond = if Nominators::<T>::contains_key(&stash) {
-					MinNominatorBond::<T>::get()
+					Self::min_nominator_bond()
 				} else if Validators::<T>::contains_key(&stash) {
-					MinValidatorBond::<T>::get()
+					Self::min_validator_bond()
 				} else {
 					// staker is chilled, no min bond.
 					Zero::zero()
@@ -1493,7 +1493,7 @@ pub mod pallet {
 
 			let ledger = Self::ledger(Controller(controller))?;
 
-			ensure!(ledger.active >= MinValidatorBond::<T>::get(), Error::<T>::InsufficientBond);
+			ensure!(ledger.active >= Self::min_validator_bond(), Error::<T>::InsufficientBond);
 			let stash = &ledger.stash;
 
 			// ensure their commission is correct.
@@ -1534,7 +1534,7 @@ pub mod pallet {
 
 			let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?;
 
-			ensure!(ledger.active >= MinNominatorBond::<T>::get(), Error::<T>::InsufficientBond);
+			ensure!(ledger.active >= Self::min_nominator_bond(), Error::<T>::InsufficientBond);
 			let stash = &ledger.stash;
 
 			// Only check limits if they are not already a nominator.
@@ -1889,7 +1889,7 @@ pub mod pallet {
 			let initial_unlocking = ledger.unlocking.len() as u32;
 			let (ledger, rebonded_value) = ledger.rebond(value);
 			// Last check: the new active amount of ledger must be more than min bond.
-			ensure!(ledger.active >= Self::min_bond(), Error::<T>::InsufficientBond);
+			ensure!(ledger.active >= Self::min_chilled_bond(), Error::<T>::InsufficientBond);
 
 			Self::deposit_event(Event::<T>::Bonded {
 				stash: ledger.stash.clone(),
@@ -1942,13 +1942,13 @@ pub mod pallet {
 			// virtual stakers should not be allowed to be reaped.
 			ensure!(!Self::is_virtual_staker(&stash), Error::<T>::VirtualStakerNotAllowed);
 
-			let min_bond = Self::min_bond();
+			let min_chilled_bond = Self::min_chilled_bond();
 			let origin_balance = asset::total_balance::<T>(&stash);
 			let ledger_total =
 				Self::ledger(Stash(stash.clone())).map(|l| l.total).unwrap_or_default();
-			let reapable = origin_balance < min_bond ||
+			let reapable = origin_balance < min_chilled_bond ||
 				origin_balance.is_zero() ||
-				ledger_total < min_bond ||
+				ledger_total < min_chilled_bond ||
 				ledger_total.is_zero();
 			ensure!(reapable, Error::<T>::FundedTarget);
 
@@ -2123,7 +2123,7 @@ pub mod pallet {
 						threshold * max_nominator_count < current_nominator_count,
 						Error::<T>::CannotChillOther
 					);
-					MinNominatorBond::<T>::get()
+					Self::min_nominator_bond()
 				} else if Validators::<T>::contains_key(&stash) {
 					let max_validator_count =
 						MaxValidatorsCount::<T>::get().ok_or(Error::<T>::CannotChillOther)?;
@@ -2132,7 +2132,7 @@ pub mod pallet {
 						threshold * max_validator_count < current_validator_count,
 						Error::<T>::CannotChillOther
 					);
-					MinValidatorBond::<T>::get()
+					Self::min_validator_bond()
 				} else {
 					Zero::zero()
 				};

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

@@ -513,7 +513,7 @@ impl<T: Config> Rotator<T> {
 
 	#[cfg(any(feature = "try-runtime", feature = "std", feature = "runtime-benchmarks", test))]
 	pub fn assert_election_ongoing() {
-		assert!(Self::planning_era().is_some(), "planning era must exist");
+		assert!(Self::is_planning().is_some(), "planning era must exist");
 		assert!(
 			T::ElectionProvider::status().is_ok(),
 			"Election provider must be in a good state during election"
@@ -526,7 +526,7 @@ impl<T: Config> Rotator<T> {
 	/// is underway, but rather the last era that was planned. If `Self::active_era()` is equal to
 	/// this value, it means that the era is currently active and no new era is planned.
 	///
-	/// See [`Self::planning_era()`] to only get the next index that is planned.
+	/// See [`Self::is_planning()`] to only get the next index if planning in progress.
 	pub fn planned_era() -> EraIndex {
 		CurrentEra::<T>::get().unwrap_or(0)
 	}
@@ -538,7 +538,7 @@ impl<T: Config> Rotator<T> {
 	/// Next era that is planned to be started.
 	///
 	/// Returns None if no era is planned.
-	pub fn planning_era() -> Option<EraIndex> {
+	pub fn is_planning() -> Option<EraIndex> {
 		let (active, planned) = (Self::active_era(), Self::planned_era());
 		if planned.defensive_saturating_sub(active) > 1 {
 			defensive!("planned era must always be equal or one more than active");
@@ -553,7 +553,7 @@ impl<T: Config> Rotator<T> {
 			defensive!("Active era must always be available.");
 			return;
 		};
-		let current_planned_era = Self::planning_era();
+		let current_planned_era = Self::is_planning();
 		let starting = end_index + 1;
 		// the session after the starting session.
 		let planning = starting + 1;
@@ -605,7 +605,7 @@ impl<T: Config> Rotator<T> {
 
 		// Note: we call `planning_era` again, as a new era might have started since we checked
 		// it last.
-		let has_pending_era = Self::planning_era().is_some();
+		let has_pending_era = Self::is_planning().is_some();
 		match (should_plan_era, has_pending_era) {
 			(false, _) => {
 				// nothing to consider