Companion of paritytech/substrate#10201

Author: aurexav

frame/staking/src/impls.rs

@@ -1359,6 +1359,7 @@ where
 		>],
 		slash_fraction: &[Perbill],
 		slash_session: SessionIndex,
+		disable_strategy: DisableStrategy,
 	) -> Weight {
 		let reward_proportion = <SlashRewardFraction<T>>::get();
 		let mut consumed_weight: Weight = 0;
@@ -1428,6 +1429,7 @@ where
 				window_start,
 				now: active_era,
 				reward_proportion,
+				disable_strategy,
 			});
 
 			if let Some(mut unapplied) = unapplied {

frame/staking/src/lib.rs

@@ -2168,17 +2168,15 @@ pub mod pallet {
 			let total_kton = T::KtonCurrency::total_balance(&stash);
 			let ed_ring = T::RingCurrency::minimum_balance();
 			let ed_kton = T::KtonCurrency::minimum_balance();
-			let mut reapable = false;
-
-			if let Some(ledger) =
+			let reapable = if let Some(ledger) =
 				Self::ledger(Self::bonded(stash.clone()).ok_or(Error::<T>::NotStash)?)
 			{
-				reapable = ((total_ring.is_zero() || total_ring < ed_ring)
+				((total_ring.is_zero() || total_ring < ed_ring)
 					&& (total_kton.is_zero() || total_kton < ed_kton))
-					|| (ledger.active < ed_ring && ledger.active_kton < ed_kton);
+					|| (ledger.active < ed_ring && ledger.active_kton < ed_kton)
 			} else {
-				reapable = true;
-			}
+				true
+			};
 
 			ensure!(reapable, <Error<T>>::FundedTarget);
 

frame/staking/src/mock.rs

@@ -862,11 +862,12 @@ pub fn on_offence_in_era(
 	>],
 	slash_fraction: &[Perbill],
 	era: EraIndex,
+	disable_strategy: DisableStrategy,
 ) {
 	let bonded_eras = <BondedEras<Test>>::get();
 	for &(bonded_era, start_session) in bonded_eras.iter() {
 		if bonded_era == era {
-			let _ = Staking::on_offence(offenders, slash_fraction, start_session);
+			let _ = Staking::on_offence(offenders, slash_fraction, start_session, disable_strategy);
 			return;
 		} else if bonded_era > era {
 			break;
@@ -878,6 +879,7 @@ pub fn on_offence_in_era(
 			offenders,
 			slash_fraction,
 			Staking::eras_start_session_index(era).unwrap(),
+			disable_strategy,
 		);
 	} else {
 		panic!("cannot slash in era {}", era);
@@ -892,7 +894,7 @@ pub fn on_offence_now(
 	slash_fraction: &[Perbill],
 ) {
 	let now = active_era();
-	on_offence_in_era(offenders, slash_fraction, now)
+	on_offence_in_era(offenders, slash_fraction, now, DisableStrategy::WhenSlashed)
 }
 
 pub fn add_slash(who: &AccountId) {

frame/staking/src/slashing.rs

@@ -62,6 +62,7 @@ use sp_runtime::{
 	traits::{Saturating, Zero},
 	DispatchResult, Perbill, RuntimeDebug,
 };
+use sp_staking::offence::DisableStrategy;
 use sp_std::{
 	ops::{Add, AddAssign, Sub},
 	prelude::*,
@@ -298,6 +299,8 @@ pub struct SlashParams<'a, T: 'a + Config> {
 	/// The maximum percentage of a slash that ever gets paid out.
 	/// This is f_inf in the paper.
 	pub reward_proportion: Perbill,
+	/// When to disable offenders.
+	pub disable_strategy: DisableStrategy,
 }
 
 /// Computes a slash of a validator and nominators. It returns an unapplied
@@ -309,30 +312,33 @@ pub struct SlashParams<'a, T: 'a + Config> {
 pub fn compute_slash<T: Config>(
 	params: SlashParams<T>,
 ) -> Option<UnappliedSlash<T::AccountId, RingBalance<T>, KtonBalance<T>>> {
-	let SlashParams { stash, slash, exposure, slash_era, window_start, now, reward_proportion } =
-		params.clone();
-
 	let mut reward_payout = Zero::zero();
 	let mut val_slashed = Zero::zero();
 
 	// is the slash amount here a maximum for the era?
-	let own_slash =
-		RK { r: slash * exposure.own_ring_balance, k: slash * exposure.own_kton_balance };
-	if (slash * exposure.total_power).is_zero() {
+	let own_slash = RK {
+		r: params.slash * params.exposure.own_ring_balance,
+		k: params.slash * params.exposure.own_kton_balance,
+	};
+	if (params.slash * params.exposure.total_power).is_zero() {
 		// kick out the validator even if they won't be slashed,
 		// as long as the misbehavior is from their most recent slashing span.
 		kick_out_if_recent::<T>(params);
 		return None;
 	}
 
 	let (prior_slash_p, _era_slash) =
-		<Pallet<T> as Store>::ValidatorSlashInEra::get(&slash_era, stash)
+		<Pallet<T> as Store>::ValidatorSlashInEra::get(&params.slash_era, params.stash)
 			.unwrap_or((Perbill::zero(), Zero::zero()));
 
 	// compare slash proportions rather than slash values to avoid issues due to rounding
 	// error.
-	if slash.deconstruct() > prior_slash_p.deconstruct() {
-		<Pallet<T> as Store>::ValidatorSlashInEra::insert(&slash_era, stash, &(slash, own_slash));
+	if params.slash.deconstruct() > prior_slash_p.deconstruct() {
+		<Pallet<T> as Store>::ValidatorSlashInEra::insert(
+			&params.slash_era,
+			params.stash,
+			&(params.slash, own_slash),
+		);
 	} else {
 		// we slash based on the max in era - this new event is not the max,
 		// so neither the validator or any nominators will need an update.
@@ -347,35 +353,34 @@ pub fn compute_slash<T: Config>(
 	// apply slash to validator.
 	{
 		let mut spans = fetch_spans::<T>(
-			stash,
-			window_start,
+			params.stash,
+			params.window_start,
 			&mut reward_payout,
 			&mut val_slashed,
-			reward_proportion,
+			params.reward_proportion,
 		);
 
-		let target_span = spans.compare_and_update_span_slash(slash_era, own_slash);
+		let target_span = spans.compare_and_update_span_slash(params.slash_era, own_slash);
 
 		if target_span == Some(spans.span_index()) {
 			// misbehavior occurred within the current slashing span - take appropriate
 			// actions.
 
 			// chill the validator - it misbehaved in the current span and should
 			// not continue in the next election. also end the slashing span.
-			spans.end_span(now);
-			<Pallet<T>>::chill_stash(stash);
+			spans.end_span(params.now);
+			<Pallet<T>>::chill_stash(params.stash);
 		}
 	}
 
-	// add the validator to the offenders list and make sure it is disabled for
-	// the duration of the era
-	add_offending_validator::<T>(params.stash, true);
+	let disable_when_slashed = params.disable_strategy != DisableStrategy::Never;
+	add_offending_validator::<T>(params.stash, disable_when_slashed);
 
 	let mut nominators_slashed = vec![];
-	reward_payout += slash_nominators::<T>(params, prior_slash_p, &mut nominators_slashed);
+	reward_payout += slash_nominators::<T>(params.clone(), prior_slash_p, &mut nominators_slashed);
 
 	Some(UnappliedSlash {
-		validator: stash.clone(),
+		validator: params.stash.clone(),
 		own: val_slashed,
 		others: nominators_slashed,
 		reporters: vec![],
@@ -402,9 +407,8 @@ fn kick_out_if_recent<T: Config>(params: SlashParams<T>) {
 		<Pallet<T>>::chill_stash(params.stash);
 	}
 
-	// add the validator to the offenders list but since there's no slash being
-	// applied there's no need to disable the validator
-	add_offending_validator::<T>(params.stash, false);
+	let disable_without_slash = params.disable_strategy == DisableStrategy::Always;
+	add_offending_validator::<T>(params.stash, disable_without_slash);
 }
 
 /// Add the given validator to the offenders list and optionally disable it.
@@ -457,13 +461,10 @@ fn slash_nominators<T: Config>(
 	prior_slash_p: Perbill,
 	nominators_slashed: &mut Vec<(T::AccountId, RKT<T>)>,
 ) -> RKT<T> {
-	let SlashParams { stash: _, slash, exposure, slash_era, window_start, now, reward_proportion } =
-		params;
-
 	let mut reward_payout = Zero::zero();
 
-	nominators_slashed.reserve(exposure.others.len());
-	for nominator in &exposure.others {
+	nominators_slashed.reserve(params.exposure.others.len());
+	for nominator in &params.exposure.others {
 		let stash = &nominator.who;
 		let mut nom_slashed = Zero::zero();
 
@@ -474,16 +475,19 @@ fn slash_nominators<T: Config>(
 				r: prior_slash_p * nominator.ring_balance,
 				k: prior_slash_p * nominator.kton_balance,
 			};
-			let own_slash_by_validator =
-				RK { r: slash * nominator.ring_balance, k: slash * nominator.kton_balance };
+			let own_slash_by_validator = RK {
+				r: params.slash * nominator.ring_balance,
+				k: params.slash * nominator.kton_balance,
+			};
 			let own_slash_difference = own_slash_by_validator.saturating_sub(own_slash_prior);
 
-			let mut era_slash = <Pallet<T> as Store>::NominatorSlashInEra::get(&slash_era, stash)
-				.unwrap_or_else(|| Zero::zero());
+			let mut era_slash =
+				<Pallet<T> as Store>::NominatorSlashInEra::get(&params.slash_era, stash)
+					.unwrap_or_else(|| Zero::zero());
 
 			era_slash += own_slash_difference;
 
-			<Pallet<T> as Store>::NominatorSlashInEra::insert(&slash_era, stash, &era_slash);
+			<Pallet<T> as Store>::NominatorSlashInEra::insert(&params.slash_era, stash, &era_slash);
 
 			era_slash
 		};
@@ -492,18 +496,18 @@ fn slash_nominators<T: Config>(
 		{
 			let mut spans = fetch_spans::<T>(
 				stash,
-				window_start,
+				params.window_start,
 				&mut reward_payout,
 				&mut nom_slashed,
-				reward_proportion,
+				params.reward_proportion,
 			);
 
-			let target_span = spans.compare_and_update_span_slash(slash_era, era_slash);
+			let target_span = spans.compare_and_update_span_slash(params.slash_era, era_slash);
 
 			if target_span == Some(spans.span_index()) {
 				// End the span, but don't chill the nominator. its nomination
 				// on this validator will be ignored in the future.
-				spans.end_span(now);
+				spans.end_span(params.now);
 			}
 		}
 

frame/staking/src/substrate_tests.rs

@@ -2633,6 +2633,7 @@ fn slash_in_old_span_does_not_deselect() {
 			}],
 			&[Perbill::from_percent(0)],
 			1,
+			DisableStrategy::WhenSlashed,
 		);
 
 		// the validator doesn't get chilled again
@@ -2649,6 +2650,7 @@ fn slash_in_old_span_does_not_deselect() {
 			// NOTE: A 100% slash here would clean up the account, causing de-registration.
 			&[Perbill::from_percent(95)],
 			1,
+			DisableStrategy::WhenSlashed,
 		);
 
 		// the validator doesn't get chilled again
@@ -2955,6 +2957,7 @@ fn slashing_nominators_by_span_max() {
 			}],
 			&[Perbill::from_percent(10)],
 			2,
+			DisableStrategy::WhenSlashed,
 		);
 
 		assert_eq!(Ring::free_balance(11), 900);
@@ -2981,6 +2984,7 @@ fn slashing_nominators_by_span_max() {
 			}],
 			&[Perbill::from_percent(30)],
 			3,
+			DisableStrategy::WhenSlashed,
 		);
 
 		// 11 was not further slashed, but 21 and 101 were.
@@ -3002,6 +3006,7 @@ fn slashing_nominators_by_span_max() {
 			}],
 			&[Perbill::from_percent(20)],
 			2,
+			DisableStrategy::WhenSlashed,
 		);
 
 		// 11 was further slashed, but 21 and 101 were not.
@@ -3137,6 +3142,7 @@ fn remove_deferred() {
 			&[OffenceDetails { offender: (11, exposure.clone()), reporters: vec![] }],
 			&[Perbill::from_percent(15)],
 			1,
+			DisableStrategy::WhenSlashed,
 		);
 
 		// fails if empty
@@ -3330,6 +3336,40 @@ fn non_slashable_offence_doesnt_disable_validator() {
 	});
 }
 
+#[test]
+fn slashing_independent_of_disabling_validator() {
+	ExtBuilder::default().build_and_execute(|| {
+		mock::start_active_era(1);
+		assert_eq_uvec!(Session::validators(), vec![11, 21]);
+
+		let exposure_11 = Staking::eras_stakers(Staking::active_era().unwrap().index, &11);
+		let exposure_21 = Staking::eras_stakers(Staking::active_era().unwrap().index, &21);
+
+		let now = Staking::active_era().unwrap().index;
+
+		// offence with no slash associated, BUT disabling
+		on_offence_in_era(
+			&[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }],
+			&[Perbill::zero()],
+			now,
+			DisableStrategy::Always,
+		);
+
+		// offence that slashes 25% of the bond, BUT not disabling
+		on_offence_in_era(
+			&[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }],
+			&[Perbill::from_percent(25)],
+			now,
+			DisableStrategy::Never,
+		);
+
+		// the offence for validator 10 was explicitly disabled
+		assert!(is_disabled(10));
+		// whereas validator 20 is explicitly not disabled
+		assert!(!is_disabled(20));
+	});
+}
+
 #[test]
 fn offence_threshold_triggers_new_era() {
 	ExtBuilder::default()
@@ -4032,7 +4072,7 @@ fn offences_weight_calculated_correctly() {
 		// On offence with zero offenders: 4 Reads, 1 Write
 		let zero_offence_weight = <Test as frame_system::Config>::DbWeight::get().reads_writes(4, 1);
 		assert_eq!(
-			Staking::on_offence(&[], &[Perbill::from_percent(50)], 0),
+			Staking::on_offence(&[], &[Perbill::from_percent(50)], 0, DisableStrategy::WhenSlashed),
 			zero_offence_weight
 		);
 
@@ -4056,7 +4096,7 @@ fn offences_weight_calculated_correctly() {
 			})
 			.collect();
 		assert_eq!(
-			Staking::on_offence(&offenders, &[Perbill::from_percent(50)], 0),
+			Staking::on_offence(&offenders, &[Perbill::from_percent(50)], 0, DisableStrategy::WhenSlashed),
 			n_offence_unapplied_weight
 		);
 
@@ -4081,7 +4121,7 @@ fn offences_weight_calculated_correctly() {
 			+ <Test as frame_system::Config>::DbWeight::get().reads_writes(2, 2);
 
 		assert_eq!(
-			Staking::on_offence(&one_offender, &[Perbill::from_percent(50)], 0),
+			Staking::on_offence(&one_offender, &[Perbill::from_percent(50)], 0, DisableStrategy::WhenSlashed),
 			one_offence_unapplied_weight
 		);
 	});