diff --git a/substrate/frame/offences/src/lib.rs b/substrate/frame/offences/src/lib.rs
index 1c7ffeca7198325374f473cbae9e2f116697530c..a328b2fee4e2e72d3254f0a0a8283951bf5a5fc2 100644
--- a/substrate/frame/offences/src/lib.rs
+++ b/substrate/frame/offences/src/lib.rs
@@ -132,7 +132,6 @@ where
&concurrent_offenders,
&slash_perbill,
offence.session_index(),
- offence.disable_strategy(),
);
// Deposit the event.
diff --git a/substrate/frame/offences/src/migration.rs b/substrate/frame/offences/src/migration.rs
index 3b5cf3ce926952df21248ea5b35271d6596a554b..199f47491369b8281e414076f35e43be823931c8 100644
--- a/substrate/frame/offences/src/migration.rs
+++ b/substrate/frame/offences/src/migration.rs
@@ -23,7 +23,7 @@ use frame_support::{
weights::Weight,
Twox64Concat,
};
-use sp_staking::offence::{DisableStrategy, OnOffenceHandler};
+use sp_staking::offence::OnOffenceHandler;
use sp_std::vec::Vec;
#[cfg(feature = "try-runtime")]
@@ -106,12 +106,7 @@ pub fn remove_deferred_storage<T: Config>() -> Weight {
let deferred = <DeferredOffences<T>>::take();
log::info!(target: LOG_TARGET, "have {} deferred offences, applying.", deferred.len());
for (offences, perbill, session) in deferred.iter() {
- let consumed = T::OnOffenceHandler::on_offence(
- offences,
- perbill,
- *session,
- DisableStrategy::WhenSlashed,
- );
+ let consumed = T::OnOffenceHandler::on_offence(offences, perbill, *session);
weight = weight.saturating_add(consumed);
}
diff --git a/substrate/frame/offences/src/mock.rs b/substrate/frame/offences/src/mock.rs
index 990ceae5ac01e18e779d6cbb9af9c3249c46cb78..2b80668b0d49a0bb0a7574fecf941c22688b6adf 100644
--- a/substrate/frame/offences/src/mock.rs
+++ b/substrate/frame/offences/src/mock.rs
@@ -51,7 +51,6 @@ impl<Reporter, Offender> offence::OnOffenceHandler<Reporter, Offender, Weight>
_offenders: &[OffenceDetails<Reporter, Offender>],
slash_fraction: &[Perbill],
_offence_session: SessionIndex,
- _disable_strategy: DisableStrategy,
) -> Weight {
OnOffencePerbill::mutate(|f| {
*f = slash_fraction.to_vec();
diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs
index d2afd8f26e241853624875dcc87448e2661896a1..c23b1caf3bad6f66f958c9852e6b16bd0c09e1f9 100644
--- a/substrate/frame/staking/src/mock.rs
+++ b/substrate/frame/staking/src/mock.rs
@@ -40,7 +40,7 @@ use sp_runtime::{
BuildStorage,
};
use sp_staking::{
- offence::{DisableStrategy, OffenceDetails, OnOffenceHandler},
+ offence::{OffenceDetails, OnOffenceHandler},
OnStakingUpdate,
};
@@ -723,12 +723,11 @@ 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, disable_strategy);
+ let _ = Staking::on_offence(offenders, slash_fraction, start_session);
return
} else if bonded_era > era {
break
@@ -740,7 +739,6 @@ 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);
@@ -755,7 +753,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, DisableStrategy::WhenSlashed)
+ on_offence_in_era(offenders, slash_fraction, now)
}
pub(crate) fn add_slash(who: &AccountId) {
diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs
index 9c36c94b87b4739d2fda466da586bffa41c9b73a..d0e509b0da0c0813d48b1f3d7f1134ed92d26b3c 100644
--- a/substrate/frame/staking/src/pallet/impls.rs
+++ b/substrate/frame/staking/src/pallet/impls.rs
@@ -40,7 +40,7 @@ use sp_runtime::{
};
use sp_staking::{
currency_to_vote::CurrencyToVote,
- offence::{DisableStrategy, OffenceDetails, OnOffenceHandler},
+ offence::{OffenceDetails, OnOffenceHandler},
EraIndex, Page, SessionIndex, Stake,
StakingAccount::{self, Controller, Stash},
StakingInterface,
@@ -427,10 +427,8 @@ impl<T: Config> Pallet<T> {
}
// disable all offending validators that have been disabled for the whole era
- for (index, disabled) in <OffendingValidators<T>>::get() {
- if disabled {
- T::SessionInterface::disable_validator(index);
- }
+ for index in <OffendingValidators<T>>::get() {
+ T::SessionInterface::disable_validator(index);
}
}
@@ -1354,7 +1352,6 @@ where
>],
slash_fraction: &[Perbill],
slash_session: SessionIndex,
- disable_strategy: DisableStrategy,
) -> Weight {
let reward_proportion = SlashRewardFraction::<T>::get();
let mut consumed_weight = Weight::from_parts(0, 0);
@@ -1419,7 +1416,6 @@ where
window_start,
now: active_era,
reward_proportion,
- disable_strategy,
});
Self::deposit_event(Event::<T>::SlashReported {
diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs
index 18ad3e4a6cf1df1a504a60b42711d90c0fed33e6..667b3859a023a3bac1417d6280c10311546572b1 100644
--- a/substrate/frame/staking/src/pallet/mod.rs
+++ b/substrate/frame/staking/src/pallet/mod.rs
@@ -645,10 +645,11 @@ pub mod pallet {
/// `OffendingValidatorsThreshold` is reached. The vec is always kept sorted so that we can find
/// whether a given validator has previously offended using binary search. It gets cleared when
/// the era ends.
+ // TODO: Fix the comment above
#[pallet::storage]
#[pallet::unbounded]
#[pallet::getter(fn offending_validators)]
- pub type OffendingValidators<T: Config> = StorageValue<_, Vec<(u32, bool)>, ValueQuery>;
+ pub type OffendingValidators<T: Config> = StorageValue<_, Vec<u32>, ValueQuery>;
/// 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
diff --git a/substrate/frame/staking/src/slashing.rs b/substrate/frame/staking/src/slashing.rs
index 0d84d503733e6451feb69f287d69f8655fe0b20e..35b35217e49eb1746a9347a41e47995aae1d3757 100644
--- a/substrate/frame/staking/src/slashing.rs
+++ b/substrate/frame/staking/src/slashing.rs
@@ -64,7 +64,7 @@ use sp_runtime::{
traits::{Saturating, Zero},
DispatchResult, RuntimeDebug,
};
-use sp_staking::{offence::DisableStrategy, EraIndex};
+use sp_staking::EraIndex;
use sp_std::vec::Vec;
/// The proportion of the slashing reward to be paid out on the first slashing detection.
@@ -215,8 +215,6 @@ 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
@@ -285,8 +283,7 @@ pub(crate) fn compute_slash<T: Config>(
}
}
- let disable_when_slashed = params.disable_strategy != DisableStrategy::Never;
- add_offending_validator::<T>(params.stash, disable_when_slashed);
+ add_offending_validator::<T>(params.stash);
let mut nominators_slashed = Vec::new();
reward_payout += slash_nominators::<T>(params.clone(), prior_slash_p, &mut nominators_slashed);
@@ -319,14 +316,13 @@ fn kick_out_if_recent<T: Config>(params: SlashParams<T>) {
<Pallet<T>>::chill_stash(params.stash);
}
- let disable_without_slash = params.disable_strategy == DisableStrategy::Always;
- add_offending_validator::<T>(params.stash, disable_without_slash);
+ add_offending_validator::<T>(params.stash);
}
/// Add the given validator to the offenders list and optionally disable it.
/// If after adding the validator `OffendingValidatorsThreshold` is reached
/// a new era will be forced.
-fn add_offending_validator<T: Config>(stash: &T::AccountId, disable: bool) {
+fn add_offending_validator<T: Config>(stash: &T::AccountId) {
OffendingValidators::<T>::mutate(|offending| {
let validators = T::SessionInterface::validators();
let validator_index = match validators.iter().position(|i| i == stash) {
@@ -336,31 +332,20 @@ fn add_offending_validator<T: Config>(stash: &T::AccountId, disable: bool) {
let validator_index_u32 = validator_index as u32;
- match offending.binary_search_by_key(&validator_index_u32, |(index, _)| *index) {
+ if let Err(index) = offending.binary_search_by_key(&validator_index_u32, |index| *index) {
// this is a new offending validator
- Err(index) => {
- offending.insert(index, (validator_index_u32, disable));
+ offending.insert(index, validator_index_u32);
- let offending_threshold =
- T::OffendingValidatorsThreshold::get() * validators.len() as u32;
+ let offending_threshold =
+ T::OffendingValidatorsThreshold::get() * validators.len() as u32;
- if offending.len() >= offending_threshold as usize {
- // force a new era, to select a new validator set
- <Pallet<T>>::ensure_new_era()
- }
+ // TODO - don't do this
+ if offending.len() >= offending_threshold as usize {
+ // force a new era, to select a new validator set
+ <Pallet<T>>::ensure_new_era()
+ }
- if disable {
- T::SessionInterface::disable_validator(validator_index_u32);
- }
- },
- Ok(index) => {
- if disable && !offending[index].1 {
- // the validator had previously offended without being disabled,
- // let's make sure we disable it now
- offending[index].1 = true;
- T::SessionInterface::disable_validator(validator_index_u32);
- }
- },
+ T::SessionInterface::disable_validator(validator_index_u32);
}
});
}
diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs
index ee6f67adf14c97fe819e692a72915496c02ab759..add212e7279be357d43ee77e4824d28bda763d4f 100644
--- a/substrate/frame/staking/src/tests.rs
+++ b/substrate/frame/staking/src/tests.rs
@@ -38,7 +38,7 @@ use sp_runtime::{
Perbill, Percent, Perquintill, Rounding, TokenError,
};
use sp_staking::{
- offence::{DisableStrategy, OffenceDetails, OnOffenceHandler},
+ offence::{OffenceDetails, OnOffenceHandler},
SessionIndex,
};
use sp_std::prelude::*;
@@ -2429,7 +2429,6 @@ fn slash_in_old_span_does_not_deselect() {
}],
&[Perbill::from_percent(0)],
1,
- DisableStrategy::WhenSlashed,
);
// the validator doesn't get chilled again
@@ -2446,7 +2445,6 @@ 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
@@ -2746,7 +2744,6 @@ fn slashing_nominators_by_span_max() {
}],
&[Perbill::from_percent(10)],
2,
- DisableStrategy::WhenSlashed,
);
assert_eq!(Balances::free_balance(11), 900);
@@ -2773,7 +2770,6 @@ fn slashing_nominators_by_span_max() {
}],
&[Perbill::from_percent(30)],
3,
- DisableStrategy::WhenSlashed,
);
// 11 was not further slashed, but 21 and 101 were.
@@ -2795,7 +2791,6 @@ fn slashing_nominators_by_span_max() {
}],
&[Perbill::from_percent(20)],
2,
- DisableStrategy::WhenSlashed,
);
// 11 was further slashed, but 21 and 101 were not.
@@ -2942,7 +2937,6 @@ fn retroactive_deferred_slashes_two_eras_before() {
&[OffenceDetails { offender: (11, exposure_11_at_era1), reporters: vec![] }],
&[Perbill::from_percent(10)],
1, // should be deferred for two full eras, and applied at the beginning of era 4.
- DisableStrategy::Never,
);
mock::start_active_era(4);
@@ -2980,7 +2974,6 @@ fn retroactive_deferred_slashes_one_before() {
&[OffenceDetails { offender: (11, exposure_11_at_era1), reporters: vec![] }],
&[Perbill::from_percent(10)],
2, // should be deferred for two full eras, and applied at the beginning of era 5.
- DisableStrategy::Never,
);
mock::start_active_era(4);
@@ -3110,7 +3103,6 @@ fn remove_deferred() {
&[OffenceDetails { offender: (11, exposure.clone()), reporters: vec![] }],
&[Perbill::from_percent(15)],
1,
- DisableStrategy::WhenSlashed,
);
// fails if empty
@@ -3290,7 +3282,7 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid
}
#[test]
-fn non_slashable_offence_doesnt_disable_validator() {
+fn non_slashable_offence_disables_validator() {
ExtBuilder::default().build_and_execute(|| {
mock::start_active_era(1);
assert_eq_uvec!(Session::validators(), vec![11, 21]);
@@ -3339,8 +3331,8 @@ fn non_slashable_offence_doesnt_disable_validator() {
]
);
- // the offence for validator 10 wasn't slashable so it wasn't disabled
- assert!(!is_disabled(11));
+ // the offence for validator 10 wasn't slashable but it should have been disabled
+ assert!(is_disabled(11));
// whereas validator 20 gets disabled
assert!(is_disabled(21));
});
@@ -3357,23 +3349,21 @@ fn slashing_independent_of_disabling_validator() {
let now = Staking::active_era().unwrap().index;
- // offence with no slash associated, BUT disabling
+ // offence with no slash associated
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }],
&[Perbill::zero()],
now,
- DisableStrategy::Always,
);
// nomination remains untouched.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
- // offence that slashes 25% of the bond, BUT not disabling
+ // offence that slashes 25% of the bond
on_offence_in_era(
&[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }],
&[Perbill::from_percent(25)],
now,
- DisableStrategy::Never,
);
// nomination remains untouched.
@@ -3402,10 +3392,9 @@ fn slashing_independent_of_disabling_validator() {
]
);
- // the offence for validator 10 was explicitly disabled
+ // both validators should be disabled
assert!(is_disabled(11));
- // whereas validator 21 is explicitly not disabled
- assert!(!is_disabled(21));
+ assert!(is_disabled(21));
});
}
@@ -3466,11 +3455,6 @@ fn disabled_validators_are_kept_disabled_for_whole_era() {
let exposure_11 = Staking::eras_stakers(Staking::active_era().unwrap().index, &11);
let exposure_21 = Staking::eras_stakers(Staking::active_era().unwrap().index, &21);
- on_offence_now(
- &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }],
- &[Perbill::zero()],
- );
-
on_offence_now(
&[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }],
&[Perbill::from_percent(25)],
@@ -3479,18 +3463,15 @@ fn disabled_validators_are_kept_disabled_for_whole_era() {
// nominations are not updated.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
- // validator 11 should not be disabled since the offence wasn't slashable
- assert!(!is_disabled(11));
// validator 21 gets disabled since it got slashed
assert!(is_disabled(21));
advance_session();
// disabled validators should carry-on through all sessions in the era
- assert!(!is_disabled(11));
assert!(is_disabled(21));
- // validator 11 should now get disabled
+ // validator 11 commits an offence
on_offence_now(
&[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }],
&[Perbill::from_percent(25)],
@@ -4623,7 +4604,7 @@ fn offences_weight_calculated_correctly() {
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, DisableStrategy::WhenSlashed),
+ Staking::on_offence(&[], &[Perbill::from_percent(50)], 0),
zero_offence_weight
);
@@ -4648,7 +4629,6 @@ fn offences_weight_calculated_correctly() {
&offenders,
&[Perbill::from_percent(50)],
0,
- DisableStrategy::WhenSlashed
),
n_offence_unapplied_weight
);
@@ -4678,7 +4658,6 @@ fn offences_weight_calculated_correctly() {
&one_offender,
&[Perbill::from_percent(50)],
0,
- DisableStrategy::WhenSlashed{}
),
one_offence_unapplied_weight
);
diff --git a/substrate/primitives/staking/src/offence.rs b/substrate/primitives/staking/src/offence.rs
index 8013166374e064ab1b573e57991449567e3845da..0875e181ee17818efda973eefa88a4a6fcda2367 100644
--- a/substrate/primitives/staking/src/offence.rs
+++ b/substrate/primitives/staking/src/offence.rs
@@ -177,15 +177,15 @@ pub trait OnOffenceHandler<Reporter, Offender, Res> {
///
/// The `session` parameter is the session index of the offence.
///
- /// The `disable_strategy` parameter decides if the offenders need to be disabled immediately.
+ /// NOTE: All offenders are 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.
+ // TODO: check if the comment above is correct
fn on_offence(
offenders: &[OffenceDetails<Reporter, Offender>],
slash_fraction: &[Perbill],
session: SessionIndex,
- disable_strategy: DisableStrategy,
) -> Res;
}
@@ -194,7 +194,6 @@ impl<Reporter, Offender, Res: Default> OnOffenceHandler<Reporter, Offender, Res>
_offenders: &[OffenceDetails<Reporter, Offender>],
_slash_fraction: &[Perbill],
_session: SessionIndex,
- _disable_strategy: DisableStrategy,
) -> Res {
Default::default()
}