diff --git a/frame/offences/src/lib.rs b/frame/offences/src/lib.rs index ddae73e280d57..c230eac88dcee 100644 --- a/frame/offences/src/lib.rs +++ b/frame/offences/src/lib.rs @@ -150,6 +150,7 @@ where &concurrent_offenders, &slash_perbill, offence.session_index(), + offence.disable_strategy(), ); // Deposit the event. diff --git a/frame/offences/src/migration.rs b/frame/offences/src/migration.rs index b6e32cbe69e26..d655f2cec539a 100644 --- a/frame/offences/src/migration.rs +++ b/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() -> Weight { let deferred = >::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); } diff --git a/frame/offences/src/mock.rs b/frame/offences/src/mock.rs index 5e4c94944b6fd..bce51f527abc6 100644 --- a/frame/offences/src/mock.rs +++ b/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 offence::OnOffenceHandler _offenders: &[OffenceDetails], slash_fraction: &[Perbill], _offence_session: SessionIndex, + _disable_strategy: DisableStrategy, ) -> Weight { ON_OFFENCE_PERBILL.with(|f| { *f.borrow_mut() = slash_fraction.to_vec(); diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index e5a3e49033934..2b74b0188cff4 100644 --- a/frame/staking/src/mock.rs +++ b/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::::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) { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 7ca1cb1a4a61b..8d86cfbe6b0d6 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/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::::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 { diff --git a/frame/staking/src/slashing.rs b/frame/staking/src/slashing.rs index 414c21aa347f0..066142d8ecc24 100644 --- a/frame/staking/src/slashing.rs +++ b/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,15 +227,12 @@ pub(crate) struct SlashParams<'a, T: 'a + Config> { pub(crate) fn compute_slash( params: SlashParams, ) -> Option>> { - 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::(params); @@ -240,13 +240,17 @@ pub(crate) fn compute_slash( } let (prior_slash_p, _era_slash) = - as Store>::ValidatorSlashInEra::get(&slash_era, stash) + as Store>::ValidatorSlashInEra::get(¶ms.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() { - as Store>::ValidatorSlashInEra::insert(&slash_era, stash, &(slash, own_slash)); + if params.slash.deconstruct() > prior_slash_p.deconstruct() { + as Store>::ValidatorSlashInEra::insert( + ¶ms.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,14 +265,14 @@ pub(crate) fn compute_slash( // apply slash to validator. { let mut spans = fetch_spans::( - 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 @@ -276,20 +280,19 @@ pub(crate) fn compute_slash( // 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); - >::chill_stash(stash); + spans.end_span(params.now); + >::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::(params.stash, true); + let disable_when_slashed = params.disable_strategy != DisableStrategy::Never; + add_offending_validator::(params.stash, disable_when_slashed); let mut nominators_slashed = Vec::new(); - reward_payout += slash_nominators::(params, prior_slash_p, &mut nominators_slashed); + reward_payout += slash_nominators::(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(params: SlashParams) { >::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::(params.stash, false); + let disable_without_slash = params.disable_strategy == DisableStrategy::Always; + add_offending_validator::(params.stash, disable_without_slash); } /// Add the given validator to the offenders list and optionally disable it. @@ -371,13 +373,10 @@ fn slash_nominators( prior_slash_p: Perbill, nominators_slashed: &mut Vec<(T::AccountId, BalanceOf)>, ) -> BalanceOf { - 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 ¶ms.exposure.others { let stash = &nominator.who; let mut nom_slashed = Zero::zero(); @@ -385,15 +384,16 @@ fn slash_nominators( // 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 = as Store>::NominatorSlashInEra::get(&slash_era, stash) - .unwrap_or_else(|| Zero::zero()); + let mut era_slash = + as Store>::NominatorSlashInEra::get(¶ms.slash_era, stash) + .unwrap_or_else(|| Zero::zero()); era_slash += own_slash_difference; - as Store>::NominatorSlashInEra::insert(&slash_era, stash, &era_slash); + as Store>::NominatorSlashInEra::insert(¶ms.slash_era, stash, &era_slash); era_slash }; @@ -402,18 +402,18 @@ fn slash_nominators( { let mut spans = fetch_spans::( 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); } } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 8f13fd7850803..f8f37bed0066c 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -34,7 +34,7 @@ use sp_runtime::{ Perbill, Percent, }; use sp_staking::{ - offence::{OffenceDetails, OnOffenceHandler}, + offence::{DisableStrategy, OffenceDetails, OnOffenceHandler}, SessionIndex, }; use sp_std::prelude::*; @@ -2250,6 +2250,7 @@ fn slash_in_old_span_does_not_deselect() { }], &[Perbill::from_percent(0)], 1, + DisableStrategy::WhenSlashed, ); // the validator doesn't get chilled again @@ -2266,6 +2267,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 @@ -2562,6 +2564,7 @@ fn slashing_nominators_by_span_max() { }], &[Perbill::from_percent(10)], 2, + DisableStrategy::WhenSlashed, ); assert_eq!(Balances::free_balance(11), 900); @@ -2588,6 +2591,7 @@ fn slashing_nominators_by_span_max() { }], &[Perbill::from_percent(30)], 3, + DisableStrategy::WhenSlashed, ); // 11 was not further slashed, but 21 and 101 were. @@ -2609,6 +2613,7 @@ fn slashing_nominators_by_span_max() { }], &[Perbill::from_percent(20)], 2, + DisableStrategy::WhenSlashed, ); // 11 was further slashed, but 21 and 101 were not. @@ -2744,6 +2749,7 @@ fn remove_deferred() { &[OffenceDetails { offender: (11, exposure.clone()), reporters: vec![] }], &[Perbill::from_percent(15)], 1, + DisableStrategy::WhenSlashed, ); // fails if empty @@ -2933,6 +2939,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() @@ -3595,7 +3635,7 @@ fn offences_weight_calculated_correctly() { ExtBuilder::default().nominate(true).build_and_execute(|| { // On offence with zero offenders: 4 Reads, 1 Write let zero_offence_weight = ::DbWeight::get().reads_writes(4, 1); - assert_eq!(Staking::on_offence(&[], &[Perbill::from_percent(50)], 0), zero_offence_weight); + assert_eq!(Staking::on_offence(&[], &[Perbill::from_percent(50)], 0, DisableStrategy::WhenSlashed), zero_offence_weight); // On Offence with N offenders, Unapplied: 4 Reads, 1 Write + 4 Reads, 5 Writes let n_offence_unapplied_weight = ::DbWeight::get().reads_writes(4, 1) @@ -3608,7 +3648,7 @@ fn offences_weight_calculated_correctly() { reporters: vec![], } ).collect(); - assert_eq!(Staking::on_offence(&offenders, &[Perbill::from_percent(50)], 0), n_offence_unapplied_weight); + assert_eq!(Staking::on_offence(&offenders, &[Perbill::from_percent(50)], 0, DisableStrategy::WhenSlashed), n_offence_unapplied_weight); // On Offence with one offenders, Applied let one_offender = [ @@ -3629,7 +3669,7 @@ fn offences_weight_calculated_correctly() { // `reward_cost` * reporters (1) + ::DbWeight::get().reads_writes(2, 2); - assert_eq!(Staking::on_offence(&one_offender, &[Perbill::from_percent(50)], 0), one_offence_unapplied_weight); + assert_eq!(Staking::on_offence(&one_offender, &[Perbill::from_percent(50)], 0, DisableStrategy::WhenSlashed), one_offence_unapplied_weight); }); } diff --git a/primitives/staking/src/offence.rs b/primitives/staking/src/offence.rs index a91cb47c117b6..fdff02d42065e 100644 --- a/primitives/staking/src/offence.rs +++ b/primitives/staking/src/offence.rs @@ -37,6 +37,29 @@ pub type Kind = [u8; 16]; /// so that we can slash it accordingly. pub type OffenceCount = u32; +/// In case of an offence, which conditions get an offending validator disabled. +#[derive( + Clone, + Copy, + PartialEq, + Eq, + Hash, + PartialOrd, + Ord, + Encode, + Decode, + sp_runtime::RuntimeDebug, + scale_info::TypeInfo, +)] +pub enum DisableStrategy { + /// Independently of slashing, this offence will not disable the offender. + Never, + /// Only disable the offender if it is also slashed. + WhenSlashed, + /// Independently of slashing, this offence will always disable the offender. + Always, +} + /// A trait implemented by an offence report. /// /// This trait assumes that the offence is legitimate and was validated already. @@ -79,6 +102,11 @@ pub trait Offence { /// number. Note that for GRANDPA the round number is reset each epoch. fn time_slot(&self) -> Self::TimeSlot; + /// In which cases this offence needs to disable offenders until the next era starts. + fn disable_strategy(&self) -> DisableStrategy { + DisableStrategy::WhenSlashed + } + /// A slash fraction of the total exposure that should be slashed for this /// particular offence kind for the given parameters that happened at a singular `TimeSlot`. /// @@ -150,12 +178,15 @@ pub trait OnOffenceHandler { /// /// The `session` parameter is the session index of the offence. /// + /// The `disable_strategy` parameter decides if the offenders need to be disabled immediately. + /// /// The receiver might decide to not accept this offence. In this case, the call site is /// responsible for queuing the report and re-submitting again. fn on_offence( offenders: &[OffenceDetails], slash_fraction: &[Perbill], session: SessionIndex, + disable_strategy: DisableStrategy, ) -> Res; } @@ -164,6 +195,7 @@ impl OnOffenceHandler _offenders: &[OffenceDetails], _slash_fraction: &[Perbill], _session: SessionIndex, + _disable_strategy: DisableStrategy, ) -> Res { Default::default() }