[Fix|NominationPools] Only allow apply slash to be executed if the sl…

…ash amount is atleast ED (#6540)

This change prevents `pools::apply_slash` from being executed when the
pending slash amount of the member is lower than the ED.

The issue came to light with the failing [benchmark
test](https://github.com/polkadot-fellows/runtimes/actions/runs/11879471717/job/33101445269?pr=490#step:11:765)
in Kusama. The problem arises from the inexact conversion between points
and balance. Specifically, when points are converted to balance and then
back to points, rounding can introduce a small discrepancy between the
input and the resulting value. This issue surfaced in Kusama due to its
ED being different from Westend and Polkadot (1 UNIT/300), making the
rounding issue noticeable.

This fix is also significant because applying a slash is feeless and
permissionless. Allowing super small slash amounts to be applied without
a fee is undesirable. With this change, such small slashes will still be
applied but only when member funds are withdrawn.

---------

Co-authored-by: Dónal Murray <[email protected]>

prdoc/pr_6540.prdoc

@@ -0,0 +1,16 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Only allow apply slash to be executed if the slash amount is atleast ED
+
+doc:
+  - audience: Runtime User
+    description: |
+      This change prevents `pools::apply_slash` from being executed when the pending slash amount of the member is lower
+      than the ED. With this change, such small slashes will still be applied but only when member funds are withdrawn.
+
+crates:
+- name: pallet-nomination-pools-runtime-api
+  bump: patch
+- name: pallet-nomination-pools
+  bump: major

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

@@ -43,6 +43,9 @@ sp_api::decl_runtime_apis! {
 		fn pool_pending_slash(pool_id: PoolId) -> Balance;
 
 		/// Returns the pending slash for a given pool member.
+		///
+		/// If pending slash of the member exceeds `ExistentialDeposit`, it can be reported on
+		/// chain.
 		fn member_pending_slash(member: AccountId) -> Balance;
 
 		/// Returns true if the pool with `pool_id` needs migration.

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

@@ -1944,6 +1944,8 @@ pub mod pallet {
 		NothingToAdjust,
 		/// No slash pending that can be applied to the member.
 		NothingToSlash,
+		/// The slash amount is too low to be applied.
+		SlashTooLow,
 		/// The pool or member delegation has already migrated to delegate stake.
 		AlreadyMigrated,
 		/// The pool or member delegation has not migrated yet to delegate stake.
@@ -2300,7 +2302,7 @@ pub mod pallet {
 
 			let slash_weight =
 				// apply slash if any before withdraw.
-				match Self::do_apply_slash(&member_account, None) {
+				match Self::do_apply_slash(&member_account, None, false) {
 					Ok(_) => T::WeightInfo::apply_slash(),
 					Err(e) => {
 						let no_pending_slash: DispatchResult = Err(Error::<T>::NothingToSlash.into());
@@ -2974,8 +2976,10 @@ pub mod pallet {
 		/// Fails unless [`crate::pallet::Config::StakeAdapter`] is of strategy type:
 		/// [`adapter::StakeStrategyType::Delegate`].
 		///
-		/// This call can be dispatched permissionlessly (i.e. by any account). If the member has
-		/// slash to be applied, caller may be rewarded with the part of the slash.
+		/// The pending slash amount of the member must be equal or more than `ExistentialDeposit`.
+		/// This call can be dispatched permissionlessly (i.e. by any account). If the execution
+		/// is successful, fee is refunded and caller may be rewarded with a part of the slash
+		/// based on the [`crate::pallet::Config::StakeAdapter`] configuration.
 		#[pallet::call_index(23)]
 		#[pallet::weight(T::WeightInfo::apply_slash())]
 		pub fn apply_slash(
@@ -2989,7 +2993,7 @@ pub mod pallet {
 
 			let who = ensure_signed(origin)?;
 			let member_account = T::Lookup::lookup(member_account)?;
-			Self::do_apply_slash(&member_account, Some(who))?;
+			Self::do_apply_slash(&member_account, Some(who), true)?;
 
 			// If successful, refund the fees.
 			Ok(Pays::No.into())
@@ -3574,15 +3578,21 @@ impl<T: Config> Pallet<T> {
 	fn do_apply_slash(
 		member_account: &T::AccountId,
 		reporter: Option<T::AccountId>,
+		enforce_min_slash: bool,
 	) -> DispatchResult {
 		let member = PoolMembers::<T>::get(member_account).ok_or(Error::<T>::PoolMemberNotFound)?;
 
 		let pending_slash =
 			Self::member_pending_slash(Member::from(member_account.clone()), member.clone())?;
 
-		// if nothing to slash, return error.
+		// ensure there is something to slash.
 		ensure!(!pending_slash.is_zero(), Error::<T>::NothingToSlash);
 
+		if enforce_min_slash {
+			// ensure slashed amount is at least the minimum balance.
+			ensure!(pending_slash >= T::Currency::minimum_balance(), Error::<T>::SlashTooLow);
+		}
+
 		T::StakeAdapter::member_slash(
 			Member::from(member_account.clone()),
 			Pool::from(Pallet::<T>::generate_bonded_account(member.pool_id)),
@@ -3946,6 +3956,9 @@ impl<T: Config> Pallet<T> {
 	/// Returns the unapplied slash of a member.
 	///
 	/// Pending slash is only applicable with [`adapter::DelegateStake`] strategy.
+	///
+	/// If pending slash of the member exceeds `ExistentialDeposit`, it can be reported on
+	/// chain via [`Call::apply_slash`].
 	pub fn api_member_pending_slash(who: T::AccountId) -> BalanceOf<T> {
 		PoolMembers::<T>::get(who.clone())
 			.map(|pool_member| {

substrate/frame/nomination-pools/test-delegate-stake/Cargo.toml

@@ -26,7 +26,7 @@ sp-staking = { workspace = true, default-features = true }
 sp-core = { workspace = true, default-features = true }
 
 frame-system = { workspace = true, default-features = true }
-frame-support = { workspace = true, default-features = true }
+frame-support = { features = ["experimental"], workspace = true, default-features = true }
 frame-election-provider-support = { workspace = true, default-features = true }
 
 pallet-timestamp = { workspace = true, default-features = true }

substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs

@@ -20,7 +20,7 @@
 mod mock;
 
 use frame_support::{
-	assert_noop, assert_ok,
+	assert_noop, assert_ok, hypothetically,
 	traits::{fungible::InspectHold, Currency},
 };
 use mock::*;
@@ -537,10 +537,10 @@ fn pool_slash_proportional() {
 	// a typical example where 3 pool members unbond in era 99, 100, and 101, and a slash that
 	// happened in era 100 should only affect the latter two.
 	new_test_ext().execute_with(|| {
-		ExistentialDeposit::set(1);
+		ExistentialDeposit::set(2);
 		BondingDuration::set(28);
-		assert_eq!(Balances::minimum_balance(), 1);
-		assert_eq!(CurrentEra::<T>::get(), None);
+		assert_eq!(Balances::minimum_balance(), 2);
+		assert_eq!(Staking::current_era(), None);
 
 		// create the pool, we know this has id 1.
 		assert_ok!(Pools::create(RuntimeOrigin::signed(10), 40, 10, 10, 10));
@@ -670,6 +670,34 @@ fn pool_slash_proportional() {
 
 		// no pending slash yet.
 		assert_eq!(Pools::api_pool_pending_slash(1), 0);
+		// and therefore applying slash fails
+		assert_noop!(
+			Pools::apply_slash(RuntimeOrigin::signed(10), 21),
+			PoolsError::<Runtime>::NothingToSlash
+		);
+
+		hypothetically!({
+			// a very small amount is slashed
+			pallet_staking::slashing::do_slash::<Runtime>(
+				&POOL1_BONDED,
+				3,
+				&mut Default::default(),
+				&mut Default::default(),
+				100,
+			);
+
+			// ensure correct amount is pending to be slashed
+			assert_eq!(Pools::api_pool_pending_slash(1), 3);
+
+			// 21 has pending slash lower than ED (2)
+			assert_eq!(Pools::api_member_pending_slash(21), 1);
+
+			// slash fails as minimum pending slash amount not met.
+			assert_noop!(
+				Pools::apply_slash(RuntimeOrigin::signed(10), 21),
+				PoolsError::<Runtime>::SlashTooLow
+			);
+		});
 
 		pallet_staking::slashing::do_slash::<Runtime>(
 			&POOL1_BONDED,
@@ -909,6 +937,7 @@ fn pool_slash_non_proportional_bonded_pool_and_chunks() {
 		);
 	});
 }
+
 #[test]
 fn pool_migration_e2e() {
 	new_test_ext().execute_with(|| {