Offence implementations can disable offenders independently from slashing (#10201)

* Offence implementations can disable offenders independently from slashing

* Fix build on CI

* Run cargo fmt

* Fixes based on review comments

* Make parameter naming consistent

* Fix migration and some English

* Fix migration - again

* cargo fmt

* Cover 2 new cases with a test

Author: wigy-opensource-developer

frame/offences/src/lib.rs

@@ -150,6 +150,7 @@ where
 			&concurrent_offenders,
 			&slash_perbill,
 			offence.session_index(),
+			offence.disable_strategy(),
 		);
 
 		// Deposit the event.

frame/offences/src/migration.rs

@@ -19,7 +19,7 @@ use super::{Config, OffenceDetails, Perbill, SessionIndex};
 use frame_support::{
 	generate_storage_alias, pallet_prelude::ValueQuery, traits::Get, weights::Weight,
 };
-use sp_staking::offence::OnOffenceHandler;
+use sp_staking::offence::{DisableStrategy, OnOffenceHandler};
 use sp_std::vec::Vec;
 
 /// Type of data stored as a deferred offence
@@ -41,7 +41,12 @@ pub fn remove_deferred_storage<T: Config>() -> Weight {
 	let deferred = <DeferredOffences<T>>::take();
 	log::info!(target: "runtime::offences", "have {} deferred offences, applying.", deferred.len());
 	for (offences, perbill, session) in deferred.iter() {
-		let consumed = T::OnOffenceHandler::on_offence(&offences, &perbill, *session);
+		let consumed = T::OnOffenceHandler::on_offence(
+			&offences,
+			&perbill,
+			*session,
+			DisableStrategy::WhenSlashed,
+		);
 		weight = weight.saturating_add(consumed);
 	}
 

frame/offences/src/mock.rs

@@ -36,7 +36,7 @@ use sp_runtime::{
 	Perbill,
 };
 use sp_staking::{
-	offence::{self, Kind, OffenceDetails},
+	offence::{self, DisableStrategy, Kind, OffenceDetails},
 	SessionIndex,
 };
 use std::cell::RefCell;
@@ -55,6 +55,7 @@ impl<Reporter, Offender> offence::OnOffenceHandler<Reporter, Offender, Weight>
 		_offenders: &[OffenceDetails<Reporter, Offender>],
 		slash_fraction: &[Perbill],
 		_offence_session: SessionIndex,
+		_disable_strategy: DisableStrategy,
 	) -> Weight {
 		ON_OFFENCE_PERBILL.with(|f| {
 			*f.borrow_mut() = slash_fraction.to_vec();

frame/staking/src/mock.rs

@@ -33,7 +33,7 @@ use sp_runtime::{
 	testing::{Header, TestXt, UintAuthorityId},
 	traits::{IdentityLookup, Zero},
 };
-use sp_staking::offence::{OffenceDetails, OnOffenceHandler};
+use sp_staking::offence::{DisableStrategy, OffenceDetails, OnOffenceHandler};
 use std::cell::RefCell;
 
 pub const INIT_TIMESTAMP: u64 = 30_000;
@@ -765,11 +765,12 @@ pub(crate) fn on_offence_in_era(
 	>],
 	slash_fraction: &[Perbill],
 	era: EraIndex,
+	disable_strategy: DisableStrategy,
 ) {
 	let bonded_eras = crate::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
@@ -781,6 +782,7 @@ pub(crate) fn on_offence_in_era(
 			offenders,
 			slash_fraction,
 			Staking::eras_start_session_index(era).unwrap(),
+			disable_strategy,
 		);
 	} else {
 		panic!("cannot slash in era {}", era);
@@ -795,7 +797,7 @@ pub(crate) fn on_offence_now(
 	slash_fraction: &[Perbill],
 ) {
 	let now = Staking::active_era().unwrap().index;
-	on_offence_in_era(offenders, slash_fraction, now)
+	on_offence_in_era(offenders, slash_fraction, now, DisableStrategy::WhenSlashed)
 }
 
 pub(crate) fn add_slash(who: &AccountId) {

frame/staking/src/pallet/impls.rs

@@ -36,7 +36,7 @@ use sp_runtime::{
 	Perbill,
 };
 use sp_staking::{
-	offence::{OffenceDetails, OnOffenceHandler},
+	offence::{DisableStrategy, OffenceDetails, OnOffenceHandler},
 	SessionIndex,
 };
 use sp_std::{collections::btree_map::BTreeMap, prelude::*};
@@ -1137,6 +1137,7 @@ where
 		>],
 		slash_fraction: &[Perbill],
 		slash_session: SessionIndex,
+		disable_strategy: DisableStrategy,
 	) -> Weight {
 		let reward_proportion = SlashRewardFraction::<T>::get();
 		let mut consumed_weight: Weight = 0;
@@ -1206,6 +1207,7 @@ where
 				window_start,
 				now: active_era,
 				reward_proportion,
+				disable_strategy,
 			});
 
 			if let Some(mut unapplied) = unapplied {

frame/staking/src/slashing.rs

@@ -63,6 +63,7 @@ use sp_runtime::{
 	traits::{Saturating, Zero},
 	DispatchResult, RuntimeDebug,
 };
+use sp_staking::offence::DisableStrategy;
 use sp_std::vec::Vec;
 
 /// The proportion of the slashing reward to be paid out on the first slashing detection.
@@ -213,6 +214,8 @@ pub(crate) 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(crate) reward_proportion: Perbill,
+	/// When to disable offenders.
+	pub(crate) disable_strategy: DisableStrategy,
 }
 
 /// Computes a slash of a validator and nominators. It returns an unapplied
@@ -224,29 +227,30 @@ pub(crate) struct SlashParams<'a, T: 'a + Config> {
 pub(crate) fn compute_slash<T: Config>(
 	params: SlashParams<T>,
 ) -> Option<UnappliedSlash<T::AccountId, BalanceOf<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 = slash * exposure.own;
-	if slash * exposure.total == Zero::zero() {
+	let own_slash = params.slash * params.exposure.own;
+	if params.slash * params.exposure.total == Zero::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.
@@ -261,35 +265,34 @@ pub(crate) 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::new();
-	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::new(),
@@ -316,9 +319,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.
@@ -371,29 +373,27 @@ fn slash_nominators<T: Config>(
 	prior_slash_p: Perbill,
 	nominators_slashed: &mut Vec<(T::AccountId, BalanceOf<T>)>,
 ) -> BalanceOf<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();
 
 		// the era slash of a nominator always grows, if the validator
 		// had a new max slash for the era.
 		let era_slash = {
 			let own_slash_prior = prior_slash_p * nominator.value;
-			let own_slash_by_validator = slash * nominator.value;
+			let own_slash_by_validator = params.slash * nominator.value;
 			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
 		};
@@ -402,18 +402,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);
 			}
 		}