staking: avoid proportional slashing leak dust into chunks that should not be slashed (#12058)

* replace slash ratio with remaining ratio

Signed-off-by: linning <linningde25@gmail.com>

* little refactor

Signed-off-by: linning <linningde25@gmail.com>

* fix test

Signed-off-by: linning <linningde25@gmail.com>

* fix typo

Signed-off-by: linning <linningde25@gmail.com>

* revert refactor

Signed-off-by: linning <linningde25@gmail.com>

* rounding up instead of remaining ratio

Signed-off-by: linning <linningde25@gmail.com>

* address comment

Signed-off-by: linning <linningde25@gmail.com>

* Update frame/nomination-pools/test-staking/src/lib.rs

Signed-off-by: linning <linningde25@gmail.com>

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/nomination-pools/test-staking/src/lib.rs

Signed-off-by: linning <linningde25@gmail.com>

Co-authored-by: Roman Useinov <roman.useinov@gmail.com>

Signed-off-by: linning <linningde25@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Roman Useinov <roman.useinov@gmail.com>
Co-authored-by: parity-processbot <>

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

@@ -490,12 +490,10 @@ fn pool_slash_proportional() {
 		assert_eq!(
 			pool_events_since_last_call(),
 			vec![
-				// This last pool got slashed only the leftover dust. Otherwise in principle, this
-				// chunk/pool should have not been affected.
-				PoolsEvent::UnbondingPoolSlashed { pool_id: 1, era: 127, balance: 19 },
-				// This pool got slashed 12.5, which rounded down to 12.
-				PoolsEvent::UnbondingPoolSlashed { pool_id: 1, era: 128, balance: 8 },
-				// This pool got slashed 12.5, which rounded down to 12.
+				// This era got slashed 12.5, which rounded up to 13.
+				PoolsEvent::UnbondingPoolSlashed { pool_id: 1, era: 128, balance: 7 },
+				// This era got slashed 12 instead of 12.5 because an earlier chunk got 0.5 more
+				// slashed, and 12 is all the remaining slash
 				PoolsEvent::UnbondingPoolSlashed { pool_id: 1, era: 129, balance: 8 },
 				// Bonded pool got slashed for 25, remaining 15 in it.
 				PoolsEvent::PoolSlashed { pool_id: 1, balance: 15 }

substrate/frame/staking/src/lib.rs

@@ -310,7 +310,7 @@ use scale_info::TypeInfo;
 use sp_runtime::{
 	curve::PiecewiseLinear,
 	traits::{AtLeast32BitUnsigned, Convert, Saturating, StaticLookup, Zero},
-	Perbill, Perquintill, RuntimeDebug,
+	Perbill, Perquintill, Rounding, RuntimeDebug,
 };
 use sp_staking::{
 	offence::{Offence, OffenceError, ReportOffence},
@@ -564,6 +564,7 @@ impl<T: Config> StakingLedger<T> {
 			return Zero::zero()
 		}
 
+		use sp_runtime::PerThing as _;
 		use sp_staking::OnStakerSlash as _;
 		let mut remaining_slash = slash_amount;
 		let pre_slash_total = self.total;
@@ -594,7 +595,12 @@ impl<T: Config> StakingLedger<T> {
 						}
 					});
 				let affected_balance = self.active.saturating_add(unbonding_affected_balance);
-				let ratio = Perquintill::from_rational(slash_amount, affected_balance);
+				let ratio = Perquintill::from_rational_with_rounding(
+					slash_amount,
+					affected_balance,
+					Rounding::Up,
+				)
+				.unwrap_or_else(|_| Perquintill::one());
 				(
 					Some(ratio),
 					affected_indices.chain((0..first_slashable_index).rev()).collect::<Vec<_>>(),
@@ -618,7 +624,7 @@ impl<T: Config> StakingLedger<T> {
 
 		let mut slash_out_of = |target: &mut BalanceOf<T>, slash_remaining: &mut BalanceOf<T>| {
 			let mut slash_from_target = if let Some(ratio) = maybe_proportional {
-				ratio * (*target)
+				ratio.mul_ceil(*target)
 			} else {
 				*slash_remaining
 			}

substrate/frame/staking/src/tests.rs

@@ -31,7 +31,7 @@ use pallet_balances::Error as BalancesError;
 use sp_runtime::{
 	assert_eq_error_rate,
 	traits::{BadOrigin, Dispatchable},
-	Perbill, Percent,
+	Perbill, Percent, Rounding,
 };
 use sp_staking::{
 	offence::{DisableStrategy, OffenceDetails, OnOffenceHandler},
@@ -5125,6 +5125,18 @@ fn proportional_ledger_slash_works() {
 	assert_eq!(LedgerSlashPerEra::get().0, 0);
 	assert_eq!(LedgerSlashPerEra::get().1, BTreeMap::from([(6, 30), (7, 30)]));
 
+	// Given
+	ledger.unlocking = bounded_vec![c(4, 100), c(5, 100), c(6, 100), c(7, 100)];
+	ledger.total = 4 * 100;
+	ledger.active = 0;
+	// When the first 2 chunks don't overlap with the affected range of unlock eras.
+	assert_eq!(ledger.slash(15, 0, 3), 15);
+	// Then
+	assert_eq!(ledger.unlocking, vec![c(4, 100), c(5, 100), c(6, 100 - 8), c(7, 100 - 7)]);
+	assert_eq!(ledger.total, 4 * 100 - 15);
+	assert_eq!(LedgerSlashPerEra::get().0, 0);
+	assert_eq!(LedgerSlashPerEra::get().1, BTreeMap::from([(6, 92), (7, 93)]));
+
 	// Given
 	ledger.unlocking = bounded_vec![c(4, 40), c(5, 100), c(6, 10), c(7, 250)];
 	ledger.active = 500;
@@ -5247,6 +5259,7 @@ fn proportional_ledger_slash_works() {
 	assert_eq!(LedgerSlashPerEra::get().1, BTreeMap::from([(4, 0), (5, 0), (6, 0), (7, 0)]));
 
 	// Given
+	use sp_runtime::PerThing as _;
 	let slash = u64::MAX as Balance * 2;
 	let value = u64::MAX as Balance * 2;
 	let unit = 100;
@@ -5259,18 +5272,19 @@ fn proportional_ledger_slash_works() {
 	ledger.active = unit;
 	ledger.total = unit * 4 + value;
 	// When
-	assert_eq!(ledger.slash(slash, 0, 0), slash - 43);
+	assert_eq!(ledger.slash(slash, 0, 0), slash - 5);
 	// Then
 	// The amount slashed out of `unit`
 	let affected_balance = value + unit * 4;
-	let ratio = Perquintill::from_rational(slash, affected_balance);
+	let ratio =
+		Perquintill::from_rational_with_rounding(slash, affected_balance, Rounding::Up).unwrap();
 	// `unit` after the slash is applied
 	let unit_slashed = {
-		let unit_slash = ratio * unit;
+		let unit_slash = ratio.mul_ceil(unit);
 		unit - unit_slash
 	};
 	let value_slashed = {
-		let value_slash = ratio * value;
+		let value_slash = ratio.mul_ceil(value);
 		value - value_slash
 	};
 	assert_eq!(ledger.active, unit_slashed);

substrate/primitives/runtime/src/lib.rs

@@ -97,7 +97,7 @@ pub use sp_arithmetic::helpers_128bit;
 pub use sp_arithmetic::{
 	traits::SaturatedConversion, FixedI128, FixedI64, FixedPointNumber, FixedPointOperand,
 	FixedU128, InnerOf, PerThing, PerU16, Perbill, Percent, Permill, Perquintill, Rational128,
-	UpperOf,
+	Rounding, UpperOf,
 };
 
 pub use either::Either;