[Staking] [AHM] Fixes insufficient slashing of nominators (and some other small issues).

cumulus/parachains/runtimes/assets/asset-hub-westend/src/staking.rs

@@ -264,7 +264,6 @@ impl pallet_staking_async::Config for Runtime {
 	type EventListeners = (NominationPools, DelegatedStaking);
 	type WeightInfo = ();
 	type MaxInvulnerables = frame_support::traits::ConstU32<20>;
-	type MaxDisabledValidators = ConstU32<100>;
 	type PlanningEraOffset = PlanningEraOffset;
 	type RcClientInterface = StakingRcClient;
 	type MaxEraDuration = MaxEraDuration;

prdoc/pr_8937.prdoc

@@ -0,0 +1,17 @@
+title: '[Staking] [AHM] Fixes insufficient slashing of nominators and some other small
+  issues.'
+doc:
+- audience: Runtime Dev
+  description: |-
+    ## Removed
+    - Config constant `MaxDisabledValidators`: This constant was removed since validator disabling logic has been moved to pallet-session, making it redundant in staking-async.
+    - Storage DoubleMap `NominatorSlashInEra`: This was used to track per-era maximum slashes for nominators. It’s no longer required — we now only track the highest slash per validator per era.
+    - Call `withdraw_overstake`: This was a temporary extrinsic meant to fix overstake issues, but with fungible migration of staking funds, this is no longer possible and the extrinsic is obsolete.
+
+    ## Changed
+    - Nominator slashing logic: The logic now aggregates slashes from distinct offending validators nominated by a nominator within the same era. For repeated offences by the same validator, only the highest slash fraction is applied. Previously, the pallet applied only the highest slash across all validators, regardless of how many were slashed.
+crates:
+- name: pallet-staking-async
+  bump: major
+- name: pallet-staking-async-parachain-runtime
+  bump: major

substrate/frame/staking-async/ahm-test/src/ah/mock.rs

@@ -345,7 +345,6 @@ impl pallet_staking_async::Config for Runtime {
 	type HistoryDepth = ConstU32<7>;
 	type MaxControllersInDeprecationBatch = ();
 
-	type MaxDisabledValidators = MaxValidators;
 	type MaxValidatorSet = MaxValidators;
 	type MaxExposurePageSize = MaxExposurePageSize;
 	type MaxInvulnerables = MaxValidators;

substrate/frame/staking-async/runtimes/parachain/src/staking.rs

@@ -280,7 +280,6 @@ impl pallet_staking_async::Config for Runtime {
 	type WeightInfo = weights::pallet_staking_async::WeightInfo<Runtime>;
 	type MaxInvulnerables = frame_support::traits::ConstU32<20>;
 	type MaxEraDuration = MaxEraDuration;
-	type MaxDisabledValidators = ConstU32<100>;
 	type PlanningEraOffset =
 		pallet_staking_async::PlanningEraOffsetOf<Self, RelaySessionDuration, ConstU32<10>>;
 	type RcClientInterface = StakingRcClient;

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

@@ -436,7 +436,6 @@ impl crate::pallet::pallet::Config for Test {
 	type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch;
 	type EventListeners = EventListenerMock;
 	type MaxInvulnerables = ConstU32<20>;
-	type MaxDisabledValidators = ConstU32<100>;
 	type MaxEraDuration = MaxEraDuration;
 	type PlanningEraOffset = PlanningEraOffset;
 	type Filter = MockedRestrictList;

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

@@ -308,10 +308,6 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxInvulnerables: Get<u32>;
 
-		/// Maximum number of disabled validators.
-		#[pallet::constant]
-		type MaxDisabledValidators: Get<u32>;
-
 		/// Maximum allowed era duration in milliseconds.
 		///
 		/// This provides a defensive upper bound to cap the effective era duration, preventing
@@ -383,7 +379,6 @@ pub mod pallet {
 			type MaxValidatorSet = ConstU32<100>;
 			type MaxControllersInDeprecationBatch = ConstU32<100>;
 			type MaxInvulnerables = ConstU32<20>;
-			type MaxDisabledValidators = ConstU32<100>;
 			type MaxEraDuration = ();
 			type EventListeners = ();
 			type Filter = Nothing;
@@ -782,11 +777,6 @@ pub mod pallet {
 		(Perbill, BalanceOf<T>),
 	>;
 
-	/// All slashing events on nominators, mapped by era to the highest slash value of the era.
-	#[pallet::storage]
-	pub type NominatorSlashInEra<T: Config> =
-		StorageDoubleMap<_, Twox64Concat, EraIndex, Twox64Concat, T::AccountId, BalanceOf<T>>;
-
 	/// The threshold for when users can start calling `chill_other` for other validators /
 	/// nominators. The threshold is compared to the actual number of validators / nominators
 	/// (`CountFor*`) in the system compared to the configured max (`Max*Count`).
@@ -2488,38 +2478,5 @@ pub mod pallet {
 
 			Ok(Pays::No.into())
 		}
-
-		/// Adjusts the staking ledger by withdrawing any excess staked amount.
-		///
-		/// This function corrects cases where a user's recorded stake in the ledger
-		/// exceeds their actual staked funds. This situation can arise due to cases such as
-		/// external slashing by another pallet, leading to an inconsistency between the ledger
-		/// and the actual stake.
-		#[pallet::call_index(32)]
-		#[pallet::weight(T::DbWeight::get().reads_writes(2, 1))]
-		pub fn withdraw_overstake(origin: OriginFor<T>, stash: T::AccountId) -> DispatchResult {
-			use sp_runtime::Saturating;
-			let _ = ensure_signed(origin)?;
-
-			let ledger = Self::ledger(Stash(stash.clone()))?;
-			let actual_stake = asset::staked::<T>(&stash);
-			let force_withdraw_amount = ledger.total.defensive_saturating_sub(actual_stake);
-
-			// ensure there is something to force unstake.
-			ensure!(!force_withdraw_amount.is_zero(), Error::<T>::BoundNotMet);
-
-			// we ignore if active is 0. It implies the locked amount is not actively staked. The
-			// account can still get away from potential slash, but we can't do much better here.
-			StakingLedger {
-				total: actual_stake,
-				active: ledger.active.saturating_sub(force_withdraw_amount),
-				..ledger
-			}
-			.update()?;
-
-			Self::deposit_event(Event::<T>::Withdrawn { stash, amount: force_withdraw_amount });
-
-			Ok(())
-		}
 	}
 }

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

@@ -41,10 +41,9 @@
 //! Based on research at <https://research.web3.foundation/Polkadot/security/slashing/npos>
 
 use crate::{
-	asset, log, session_rotation::Eras, BalanceOf, Config, NegativeImbalanceOf,
-	NominatorSlashInEra, OffenceQueue, OffenceQueueEras, PagedExposure, Pallet, Perbill,
-	ProcessingOffence, SlashRewardFraction, UnappliedSlash, UnappliedSlashes, ValidatorSlashInEra,
-	WeightInfo,
+	asset, log, session_rotation::Eras, BalanceOf, Config, NegativeImbalanceOf, OffenceQueue,
+	OffenceQueueEras, PagedExposure, Pallet, Perbill, ProcessingOffence, SlashRewardFraction,
+	UnappliedSlash, UnappliedSlashes, ValidatorSlashInEra, WeightInfo,
 };
 use alloc::vec::Vec;
 use codec::{Decode, Encode, MaxEncodedLen};
@@ -178,7 +177,8 @@ fn next_offence<T: Config>() -> Option<(EraIndex, T::AccountId, OffenceRecord<T:
 
 /// Infallible function to process an offence.
 pub(crate) fn process_offence<T: Config>() -> Weight {
-	// We do manual weight racking for early-returns, and use benchmarks for the final two branches.
+	// We do manual weight tracking for early-returns, and use benchmarks for the final two
+	// branches.
 	let mut incomplete_consumed_weight = Weight::from_parts(0, 0);
 	let mut add_db_reads_writes = |reads, writes| {
 		incomplete_consumed_weight += T::DbWeight::get().reads_writes(reads, writes);
@@ -351,36 +351,31 @@ fn slash_nominators<T: Config>(
 	nominators_slashed.reserve(params.exposure.exposure_page.others.len());
 	for nominator in &params.exposure.exposure_page.others {
 		let stash = &nominator.who;
-		let prior_slashed =
-			NominatorSlashInEra::<T>::get(&params.slash_era, stash).unwrap_or_else(Zero::zero);
+		let prior_slashed = params.prior_slash * nominator.value;
 		let new_slash = params.slash * nominator.value;
-		let slash_due = new_slash.saturating_sub(prior_slashed);
+		// this should always be positive since prior slash is always less than the new slash or
+		// filtered out when offence is reported (`Pallet::on_new_offences`).
+		let slash_diff = new_slash.defensive_saturating_sub(prior_slashed);
 
-		if new_slash == Zero::zero() {
+		if slash_diff == Zero::zero() {
 			// nothing to do
 			continue
 		}
 
 		log!(
 			debug,
-			"🦹 slashing nominator {:?} of stake: {:?} for {:?} in era {:?}",
+			"🦹 slashing nominator {:?} of stake: {:?} for {:?} in era {:?}. Prior Slash: {:?}, New Slash: {:?}",
 			stash,
 			nominator.value,
-			slash_due,
+			slash_diff,
 			params.slash_era,
+			params.prior_slash,
+			params.slash,
 		);
 
-		// the era slash of a nominator always grows, if the validator had a new max slash for the
-		// era.
-		NominatorSlashInEra::<T>::insert(
-			&params.slash_era,
-			stash,
-			prior_slashed.saturating_add(slash_due),
-		);
-
-		nominators_slashed.push((stash.clone(), slash_due));
-		total_slashed.saturating_accrue(slash_due);
-		reward_payout.saturating_accrue(params.reward_proportion * slash_due);
+		nominators_slashed.push((stash.clone(), slash_diff));
+		total_slashed.saturating_accrue(slash_diff);
+		reward_payout.saturating_accrue(params.reward_proportion * slash_diff);
 	}
 
 	(total_slashed, reward_payout)
@@ -390,8 +385,6 @@ fn slash_nominators<T: Config>(
 pub(crate) fn clear_era_metadata<T: Config>(obsolete_era: EraIndex) {
 	#[allow(deprecated)]
 	ValidatorSlashInEra::<T>::remove_prefix(&obsolete_era, None);
-	#[allow(deprecated)]
-	NominatorSlashInEra::<T>::remove_prefix(&obsolete_era, None);
 }
 
 // apply the slash to a stash account, deducting any missing funds from the reward

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

@@ -17,7 +17,6 @@
 
 use super::*;
 use frame_support::{hypothetically_ok, traits::Currency};
-use sp_staking::{Stake, StakingInterface};
 
 #[test]
 fn existing_stash_cannot_bond() {
@@ -1073,73 +1072,7 @@ fn restricted_accounts_can_only_withdraw() {
 	})
 }
 
-#[test]
-fn permissionless_withdraw_overstake() {
-	ExtBuilder::default().build_and_execute(|| {
-		// Given Alice, Bob and Charlie with some stake.
-		let alice = 301;
-		let bob = 302;
-		let charlie = 303;
-		let _ = Balances::make_free_balance_be(&alice, 500);
-		let _ = Balances::make_free_balance_be(&bob, 500);
-		let _ = Balances::make_free_balance_be(&charlie, 500);
-		assert_ok!(Staking::bond(RuntimeOrigin::signed(alice), 100, RewardDestination::Staked));
-		assert_ok!(Staking::bond(RuntimeOrigin::signed(bob), 100, RewardDestination::Staked));
-		assert_ok!(Staking::bond(RuntimeOrigin::signed(charlie), 100, RewardDestination::Staked));
-
-		// WHEN: charlie is partially unbonding.
-		assert_ok!(Staking::unbond(RuntimeOrigin::signed(charlie), 90));
-		let charlie_ledger = StakingLedger::<Test>::get(StakingAccount::Stash(charlie)).unwrap();
-
-		// AND: alice and charlie ledger having higher value than actual stake.
-		Ledger::<Test>::insert(alice, StakingLedger::<Test>::new(alice, 200));
-		Ledger::<Test>::insert(
-			charlie,
-			StakingLedger { stash: charlie, total: 200, active: 200 - 90, ..charlie_ledger },
-		);
-
-		// THEN overstake can be permissionlessly withdrawn.
-		let _ = staking_events_since_last_call();
-
-		// Alice stake is corrected.
-		assert_eq!(
-			<Staking as StakingInterface>::stake(&alice).unwrap(),
-			Stake { total: 200, active: 200 }
-		);
-		assert_ok!(Staking::withdraw_overstake(RuntimeOrigin::signed(1), alice));
-		assert_eq!(
-			<Staking as StakingInterface>::stake(&alice).unwrap(),
-			Stake { total: 100, active: 100 }
-		);
-
-		// Charlie who is partially withdrawing also gets their stake corrected.
-		assert_eq!(
-			<Staking as StakingInterface>::stake(&charlie).unwrap(),
-			Stake { total: 200, active: 110 }
-		);
-		assert_ok!(Staking::withdraw_overstake(RuntimeOrigin::signed(1), charlie));
-		assert_eq!(
-			<Staking as StakingInterface>::stake(&charlie).unwrap(),
-			Stake { total: 200 - 100, active: 110 - 100 }
-		);
-
-		assert_eq!(
-			staking_events_since_last_call(),
-			vec![
-				Event::Withdrawn { stash: alice, amount: 200 - 100 },
-				Event::Withdrawn { stash: charlie, amount: 200 - 100 }
-			]
-		);
-
-		// but Bob ledger is fine and that cannot be withdrawn.
-		assert_noop!(
-			Staking::withdraw_overstake(RuntimeOrigin::signed(1), bob),
-			Error::<Test>::BoundNotMet
-		);
-	});
-}
-
-mod rebobd {
+mod rebond {
 	use super::*;
 
 	#[test]