diff --git a/prdoc/pr_7663.prdoc b/prdoc/pr_7663.prdoc new file mode 100644 index 0000000000000..f77e018708dd4 --- /dev/null +++ b/prdoc/pr_7663.prdoc @@ -0,0 +1,19 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Validator disabling in session enhancements + +doc: + - audience: Runtime Dev + description: | + This PR introduces changes to the pallet-session interface. Disabled validators can + still be disabled with just the index but it will default to highest possible severity. + pallet-session also additionally exposes DisabledValidators with their severities. + The staking primitive OffenceSeverity received min, max and default implementations + for ease of use. + +crates: + - name: pallet-staking + bump: major + - name: pallet-session + bump: major diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index a80a2b235757b..e007658aa00d5 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -138,13 +138,16 @@ use frame_support::{ use frame_system::pallet_prelude::BlockNumberFor; use sp_runtime::{ traits::{AtLeast32BitUnsigned, Convert, Member, One, OpaqueKeys, Zero}, - ConsensusEngineId, DispatchError, KeyTypeId, Perbill, Permill, RuntimeAppPublic, + ConsensusEngineId, DispatchError, KeyTypeId, Permill, RuntimeAppPublic, }; use sp_staking::{offence::OffenceSeverity, SessionIndex}; pub use pallet::*; pub use weights::WeightInfo; +#[cfg(any(test, feature = "try-runtime"))] +use sp_runtime::TryRuntimeError; + pub(crate) const LOG_TARGET: &str = "runtime::session"; // syntactic sugar for logging. @@ -590,6 +593,11 @@ pub mod pallet { Weight::zero() } } + + #[cfg(feature = "try-runtime")] + fn try_state(_n: BlockNumberFor) -> Result<(), TryRuntimeError> { + Self::do_try_state() + } } #[pallet::call] @@ -656,6 +664,11 @@ impl Pallet { DisabledValidators::::get().iter().map(|(i, _)| *i).collect() } + /// Public function to access the disabled validators with their severities. + pub fn disabled_validators_with_severities() -> Vec<(u32, OffenceSeverity)> { + DisabledValidators::::get() + } + /// Move on to next session. Register new validator set and session keys. Changes to the /// validator set have a session of delay to take effect. This allows for equivocation /// punishment after a fork. @@ -675,6 +688,7 @@ impl Pallet { Validators::::put(&validators); if changed { + log!(trace, "resetting disabled validators"); // reset disabled validators if active set was changed DisabledValidators::::take(); } @@ -683,12 +697,12 @@ impl Pallet { let session_index = session_index + 1; CurrentIndex::::put(session_index); T::SessionManager::start_session(session_index); - log::trace!(target: "runtime::session", "starting_session {:?}", session_index); + log!(trace, "starting_session {:?}", session_index); // Get next validator set. let maybe_next_validators = T::SessionManager::new_session(session_index + 1); - log::trace!( - target: "runtime::session", + log!( + trace, "planning_session {:?} with {:?} validators", session_index + 1, maybe_next_validators.as_ref().map(|v| v.len()) @@ -745,38 +759,6 @@ impl Pallet { T::SessionHandler::on_new_session::(changed, &session_keys, &queued_amalgamated); } - /// Disable the validator of index `i`, returns `false` if the validator was already disabled. - /// - /// Note: This sets the OffenceSeverity to the lowest value. - pub fn disable_index(i: u32) -> bool { - if i >= Validators::::decode_len().defensive_unwrap_or(0) as u32 { - return false - } - - DisabledValidators::::mutate(|disabled| { - if let Err(index) = disabled.binary_search_by_key(&i, |(index, _)| *index) { - disabled.insert(index, (i, OffenceSeverity(Perbill::zero()))); - T::SessionHandler::on_disabled(i); - return true - } - - false - }) - } - - /// Disable the validator identified by `c`. (If using with the staking pallet, - /// this would be their *stash* account.) - /// - /// Returns `false` either if the validator could not be found or it was already - /// disabled. - pub fn disable(c: &T::ValidatorId) -> bool { - Validators::::get() - .iter() - .position(|i| i == c) - .map(|i| Self::disable_index(i as u32)) - .unwrap_or(false) - } - /// Upgrade the key type from some old type to a new type. Supports adding /// and removing key types. /// @@ -928,45 +910,118 @@ impl Pallet { KeyOwner::::remove((id, key_data)); } - pub fn report_offence(validator: T::ValidatorId, severity: OffenceSeverity) { + /// Disable the validator of index `i` with a specified severity, + /// returns `false` if the validator is not found. + /// + /// Note: If validator is already disabled, the severity will + /// be updated if the new one is higher. + pub fn disable_index_with_severity(i: u32, severity: OffenceSeverity) -> bool { + if i >= Validators::::decode_len().defensive_unwrap_or(0) as u32 { + return false; + } + DisabledValidators::::mutate(|disabled| { - let decision = T::DisablingStrategy::decision(&validator, severity, &disabled); - - if let Some(offender_idx) = decision.disable { - // Check if the offender is already disabled - match disabled.binary_search_by_key(&offender_idx, |(index, _)| *index) { - // Offender is already disabled, update severity if the new one is higher - Ok(index) => { - let (_, old_severity) = &mut disabled[index]; - if severity > *old_severity { - *old_severity = severity; - } - }, - Err(index) => { - // Offender is not disabled, add to `DisabledValidators` and disable it - disabled.insert(index, (offender_idx, severity)); - // let the session handlers know that a validator got disabled - T::SessionHandler::on_disabled(offender_idx); - - // Emit event that a validator got disabled - Self::deposit_event(Event::ValidatorDisabled { - validator: validator.clone(), - }); - }, - } + match disabled.binary_search_by_key(&i, |(index, _)| *index) { + // Validator is already disabled, update severity if the new one is higher + Ok(index) => { + let current_severity = &mut disabled[index].1; + if severity > *current_severity { + log!( + trace, + "updating disablement severity of validator {:?} from {:?} to {:?}", + i, + *current_severity, + severity + ); + *current_severity = severity; + } + true + }, + // Validator is not disabled, add to `DisabledValidators` and disable it + Err(index) => { + log!(trace, "disabling validator {:?}", i); + Self::deposit_event(Event::ValidatorDisabled { + validator: Validators::::get()[index as usize].clone(), + }); + disabled.insert(index, (i, severity)); + T::SessionHandler::on_disabled(i); + true + }, } + }) + } - if let Some(reenable_idx) = decision.reenable { - // Remove the validator from `DisabledValidators` and re-enable it. - if let Ok(index) = disabled.binary_search_by_key(&reenable_idx, |(index, _)| *index) - { - disabled.remove(index); - // Emit event that a validator got re-enabled - let reenabled_stash = Validators::::get()[reenable_idx as usize].clone(); - Self::deposit_event(Event::ValidatorReenabled { validator: reenabled_stash }); - } + /// Disable the validator of index `i` with a default severity (defaults to most severe), + /// returns `false` if the validator is not found. + pub fn disable_index(i: u32) -> bool { + let default_severity = OffenceSeverity::default(); + Self::disable_index_with_severity(i, default_severity) + } + + /// Disable the validator identified by `c`. (If using with the staking pallet, + /// this would be their *stash* account.) + /// + /// Returns `false` either if the validator could not be found or it was already + /// disabled. + pub fn disable(c: &T::ValidatorId) -> bool { + Validators::::get() + .iter() + .position(|i| i == c) + .map(|i| Self::disable_index(i as u32)) + .unwrap_or(false) + } + + /// Re-enable the validator of index `i`, returns `false` if the validator was not disabled. + pub fn reenable_index(i: u32) -> bool { + if i >= Validators::::decode_len().defensive_unwrap_or(0) as u32 { + return false; + } + + DisabledValidators::::mutate(|disabled| { + if let Ok(index) = disabled.binary_search_by_key(&i, |(index, _)| *index) { + log!(trace, "reenabling validator {:?}", i); + Self::deposit_event(Event::ValidatorReenabled { + validator: Validators::::get()[index as usize].clone(), + }); + disabled.remove(index); + return true; } - }); + false + }) + } + + /// Convert a validator ID to an index. + /// (If using with the staking pallet, this would be their *stash* account.) + pub fn validator_id_to_index(id: &T::ValidatorId) -> Option { + Validators::::get().iter().position(|i| i == id).map(|i| i as u32) + } + + /// Report an offence for the given validator and let disabling strategy decide + /// what changes to disabled validators should be made. + pub fn report_offence(validator: T::ValidatorId, severity: OffenceSeverity) { + log!(trace, "reporting offence for {:?} with {:?}", validator, severity); + let decision = + T::DisablingStrategy::decision(&validator, severity, &DisabledValidators::::get()); + + // Disable + if let Some(offender_idx) = decision.disable { + Self::disable_index_with_severity(offender_idx, severity); + } + + // Re-enable + if let Some(reenable_idx) = decision.reenable { + Self::reenable_index(reenable_idx); + } + } + + #[cfg(any(test, feature = "try-runtime"))] + pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { + // Ensure that the validators are sorted + ensure!( + DisabledValidators::::get().windows(2).all(|pair| pair[0].0 <= pair[1].0), + "DisabledValidators is not sorted" + ); + Ok(()) } } @@ -1009,6 +1064,10 @@ impl frame_support::traits::DisabledValidators for Pallet { fn disabled_validators() -> Vec { Self::disabled_validators() } + + fn disabled_validators_with_severities() -> Vec<(u32, OffenceSeverity)> { + Self::disabled_validators_with_severities() + } } /// Wraps the author-scraping logic for consensus engines that can recover diff --git a/substrate/frame/session/src/migrations/v1.rs b/substrate/frame/session/src/migrations/v1.rs index bac0af6fe6b0f..a8c654da327a4 100644 --- a/substrate/frame/session/src/migrations/v1.rs +++ b/substrate/frame/session/src/migrations/v1.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{Config, DisabledValidators as NewDisabledValidators, Pallet, Perbill, Vec}; +use crate::{Config, DisabledValidators as NewDisabledValidators, Pallet, Vec}; use frame_support::{ pallet_prelude::{Get, ValueQuery, Weight}, traits::UncheckedOnRuntimeUpgrade, @@ -49,14 +49,14 @@ impl MigrateDisabledValidators for InitOffenceSeverity { fn peek_disabled() -> Vec<(u32, OffenceSeverity)> { DisabledValidators::::get() .iter() - .map(|v| (*v, OffenceSeverity(Perbill::zero()))) + .map(|v| (*v, OffenceSeverity::max_severity())) .collect::>() } fn take_disabled() -> Vec<(u32, OffenceSeverity)> { DisabledValidators::::take() .iter() - .map(|v| (*v, OffenceSeverity(Perbill::zero()))) + .map(|v| (*v, OffenceSeverity::max_severity())) .collect::>() } } diff --git a/substrate/frame/session/src/tests.rs b/substrate/frame/session/src/tests.rs index 42aeb8e14c364..7aaba65cf7ebb 100644 --- a/substrate/frame/session/src/tests.rs +++ b/substrate/frame/session/src/tests.rs @@ -366,22 +366,6 @@ fn session_keys_generate_output_works_as_set_keys_input() { }); } -#[test] -fn disable_index_returns_false_if_already_disabled() { - new_test_ext().execute_with(|| { - set_next_validators(vec![1, 2, 3, 4, 5, 6, 7]); - force_new_session(); - initialize_block(1); - // apply the new validator set - force_new_session(); - initialize_block(2); - - assert_eq!(Session::disable_index(0), true); - assert_eq!(Session::disable_index(0), false); - assert_eq!(Session::disable_index(1), true); - }); -} - #[test] fn upgrade_keys() { use frame_support::storage; @@ -482,3 +466,255 @@ fn test_migration_v1() { crate::migrations::historical::post_migrate::(); }); } + +mod disabling_byzantine_threshold { + use super::*; + use crate::disabling::{DisablingStrategy, UpToLimitDisablingStrategy}; + use sp_staking::offence::{OffenceError, OffenceSeverity}; + + // Common test data - the stash of the offending validator, the era of the offence and the + // active set + const OFFENDER_ID: ::AccountId = 7; + const MAX_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(100)); + const MIN_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(0)); + const ACTIVE_SET: [::ValidatorId; 7] = [1, 2, 3, 4, 5, 6, 7]; + const OFFENDER_VALIDATOR_IDX: u32 = 6; + + #[test] + fn disable_when_below_byzantine_threshold() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(1, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + }); + } + + #[test] + fn disable_when_below_custom_byzantine_threshold() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(1, MAX_OFFENDER_SEVERITY), (2, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + as DisablingStrategy>::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + }); + } + + #[test] + fn non_slashable_offences_still_disable() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(1, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + OffenceSeverity(Perbill::from_percent(0)), + &initially_disabled, + ); + + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + }); + } + + #[test] + fn dont_disable_beyond_byzantine_threshold() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(1, MIN_OFFENDER_SEVERITY), (2, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } +} + +mod disabling_with_reenabling { + use super::*; + use crate::disabling::{DisablingStrategy, UpToLimitWithReEnablingDisablingStrategy}; + use sp_staking::offence::{OffenceError, OffenceSeverity}; + + // Common test data - the stash of the offending validator, the era of the offence and the + // active set + const OFFENDER_ID: ::AccountId = 7; + const MAX_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(100)); + const LOW_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(0)); + const ACTIVE_SET: [::ValidatorId; 7] = [1, 2, 3, 4, 5, 6, 7]; + const OFFENDER_VALIDATOR_IDX: u32 = 6; // the offender is with index 6 in the active set + + #[test] + fn disable_when_below_byzantine_threshold() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + // Disable Offender and do not re-enable anyone + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + assert_eq!(disabling_decision.reenable, None); + }); + } + + #[test] + fn reenable_arbitrary_on_equal_severity() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); + // Disable 7 and enable 1 + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + assert_eq!(disabling_decision.reenable.unwrap(), 0); + }); + } + + #[test] + fn do_not_reenable_higher_offenders() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + LOW_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + + assert_ok!(Session::do_try_state()); + }); + } + + #[test] + fn reenable_lower_offenders() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, LOW_OFFENDER_SEVERITY), (1, LOW_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); + // Disable 7 and enable 1 + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + assert_eq!(disabling_decision.reenable.unwrap(), 0); + + assert_ok!(Session::do_try_state()); + }); + } + + #[test] + fn reenable_lower_offenders_unordered() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, LOW_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); + // Disable 7 and enable 1 + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + assert_eq!(disabling_decision.reenable.unwrap(), 1); + }); + } + + #[test] + fn update_severity() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = + vec![(OFFENDER_VALIDATOR_IDX, LOW_OFFENDER_SEVERITY), (0, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_none()); + // Disable 7 "again" AKA update their severity + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + }); + } + + #[test] + fn update_cannot_lower_severity() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = + vec![(OFFENDER_VALIDATOR_IDX, MAX_OFFENDER_SEVERITY), (0, MAX_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + LOW_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } + + #[test] + fn no_accidental_reenablement_on_repeated_offence() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = + vec![(OFFENDER_VALIDATOR_IDX, MAX_OFFENDER_SEVERITY), (0, LOW_OFFENDER_SEVERITY)]; + Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } +} diff --git a/substrate/frame/staking/src/migrations.rs b/substrate/frame/staking/src/migrations.rs index 5b0118da67ef7..90c9f5ee5daab 100644 --- a/substrate/frame/staking/src/migrations.rs +++ b/substrate/frame/staking/src/migrations.rs @@ -59,6 +59,8 @@ type StorageVersion = StorageValue, ObsoleteReleases, Value /// Migrates `UnappliedSlashes` to a new storage structure to support paged slashing. /// This ensures that slashing can be processed in batches, preventing large storage operations in a /// single block. +/// +/// Exposes staking disabled validators so they can be migrated from staking to session pallet. pub mod v17 { use super::*; @@ -81,7 +83,7 @@ pub mod v17 { #[frame_support::storage_alias] pub type DisabledValidators = - StorageValue, BoundedVec<(u32, OffenceSeverity), ConstU32<100>>, ValueQuery>; + StorageValue, BoundedVec<(u32, OffenceSeverity), ConstU32<333>>, ValueQuery>; pub struct VersionUncheckedMigrateV16ToV17(core::marker::PhantomData); impl UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV16ToV17 { diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 0d4ec8c16e231..70cacc6ef212a 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -2786,16 +2786,4 @@ impl Pallet { Ok(()) } - - /* todo(ank4n): move to session try runtime - // Sorted by index - fn ensure_disabled_validators_sorted() -> Result<(), TryRuntimeError> { - ensure!( - DisabledValidators::::get().windows(2).all(|pair| pair[0].0 <= pair[1].0), - "DisabledValidators is not sorted" - ); - Ok(()) - } - - */ } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 554c705bfbb81..aea26a7467665 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -786,8 +786,6 @@ fn nominators_also_get_slashed_pro_rata() { balances(&11).0, // free balance validator_balance - validator_share, ); - // Because slashing happened. - assert!(is_disabled(11)); }); } @@ -2561,30 +2559,6 @@ fn offence_ensures_new_era_without_clobbering() { }); } -#[test] -fn offence_deselects_validator_even_when_slash_is_zero() { - ExtBuilder::default() - .validator_count(7) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .set_status(201, StakerStatus::Validator) - .set_status(202, StakerStatus::Validator) - .build_and_execute(|| { - assert!(Session::validators().contains(&11)); - assert!(>::contains_key(11)); - - on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(0)], true); - - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - assert!(is_disabled(11)); - - mock::start_active_era(1); - - // The validator should be reenabled in the new era - assert!(!is_disabled(11)); - }); -} - #[test] fn slashing_performed_according_exposure() { // This test checks that slashing is performed according the exposure (or more precisely, @@ -2600,58 +2574,6 @@ fn slashing_performed_according_exposure() { }); } -#[test] -fn validator_is_not_disabled_for_an_offence_in_previous_era() { - ExtBuilder::default() - .validator_count(4) - .set_status(41, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - - assert!(>::contains_key(11)); - assert!(Session::validators().contains(&11)); - - on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(0)], true); - - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - assert!(is_disabled(11)); - - mock::start_active_era(2); - - // the validator is not disabled in the new era - Staking::validate(RuntimeOrigin::signed(11), Default::default()).unwrap(); - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - assert!(>::contains_key(11)); - assert!(Session::validators().contains(&11)); - - mock::start_active_era(3); - - // an offence committed in era 1 is reported in era 3 - on_offence_in_era(&[offence_from(11, None)], &[Perbill::from_percent(0)], 1, true); - - // the validator doesn't get disabled for an old offence - assert!(Validators::::iter().any(|(stash, _)| stash == 11)); - assert!(!is_disabled(11)); - - // and we are not forcing a new era - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - - on_offence_in_era( - &[offence_from(11, None)], - // NOTE: A 100% slash here would clean up the account, causing de-registration. - &[Perbill::from_percent(95)], - 1, - true, - ); - - // the validator doesn't get disabled again - assert!(Validators::::iter().any(|(stash, _)| stash == 11)); - assert!(!is_disabled(11)); - // and we are still not forcing a new era - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - }); -} - #[test] fn only_first_reporter_receive_the_slice() { // This test verifies that the first reporter of the offence receive their slice from the @@ -3326,308 +3248,6 @@ fn remove_multi_deferred() { }) } -#[test] -fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_validator() { - ExtBuilder::default() - .validator_count(7) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .set_status(201, StakerStatus::Validator) - .set_status(202, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); - - // pre-slash balance - assert_eq!(asset::stakeable_balance::(&11), 1000); - assert_eq!(asset::stakeable_balance::(&101), 2000); - - // 100 has approval for 11 as of now - assert!(Nominators::::get(101).unwrap().targets.contains(&11)); - - // 11 and 21 both have the support of 100 - let exposure_11 = Staking::eras_stakers(active_era(), &11); - let exposure_21 = Staking::eras_stakers(active_era(), &21); - - assert_eq!(exposure_11.total, 1000 + 125); - assert_eq!(exposure_21.total, 1000 + 375); - - on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(10)], true); - - assert_eq!( - staking_events_since_last_call(), - vec![ - Event::PagedElectionProceeded { page: 0, result: Ok(7) }, - Event::StakersElected, - Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, - Event::OffenceReported { - validator: 11, - fraction: Perbill::from_percent(10), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 11, page: 0 }, - Event::Slashed { staker: 11, amount: 100 }, - Event::Slashed { staker: 101, amount: 12 }, - ] - ); - - assert!(matches!( - session_events().as_slice(), - &[.., SessionEvent::ValidatorDisabled { validator: 11 }] - )); - - // post-slash balance - let nominator_slash_amount_11 = 125 / 10; - assert_eq!(asset::stakeable_balance::(&11), 900); - assert_eq!(asset::stakeable_balance::(&101), 2000 - nominator_slash_amount_11); - - // check that validator was disabled. - assert!(is_disabled(11)); - - // actually re-bond the slashed validator - assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); - - mock::start_active_era(2); - let exposure_11 = Staking::eras_stakers(active_era(), &11); - let exposure_21 = Staking::eras_stakers(active_era(), &21); - - // 11's own expo is reduced. sum of support from 11 is less (448), which is 500 - // 900 + 146 - assert!(matches!(exposure_11, Exposure { own: 900, total: 1046, .. })); - // 1000 + 342 - assert!(matches!(exposure_21, Exposure { own: 1000, total: 1342, .. })); - assert_eq!(500 - 146 - 342, nominator_slash_amount_11); - }); -} - -#[test] -fn non_slashable_offence_disables_validator() { - ExtBuilder::default() - .validator_count(7) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .set_status(201, StakerStatus::Validator) - .set_status(202, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); - - // offence with no slash associated - on_offence_now(&[offence_from(11, None)], &[Perbill::zero()], true); - - // it does NOT affect the nominator. - assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - - // offence that slashes 25% of the bond - on_offence_now(&[offence_from(21, None)], &[Perbill::from_percent(25)], true); - - // it DOES NOT affect the nominator. - assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - - assert_eq!( - staking_events_since_last_call(), - vec![ - Event::PagedElectionProceeded { page: 0, result: Ok(7) }, - Event::StakersElected, - Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, - Event::OffenceReported { - validator: 11, - fraction: Perbill::from_percent(0), - offence_era: 1 - }, - Event::OffenceReported { - validator: 21, - fraction: Perbill::from_percent(25), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 21, page: 0 }, - Event::Slashed { staker: 21, amount: 250 }, - Event::Slashed { staker: 101, amount: 94 } - ] - ); - - assert!(matches!( - session_events().as_slice(), - &[ - .., - SessionEvent::ValidatorDisabled { validator: 11 }, - SessionEvent::ValidatorDisabled { validator: 21 }, - ] - )); - - // the offence for validator 11 wasn't slashable but it is disabled - assert!(is_disabled(11)); - // validator 21 gets disabled too - assert!(is_disabled(21)); - }); -} - -#[test] -fn slashing_independent_of_disabling_validator() { - ExtBuilder::default() - .validator_count(5) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51]); - - let now = ActiveEra::::get().unwrap().index; - - // --- Disable without a slash --- - // offence with no slash associated - on_offence_in_era(&[offence_from(11, None)], &[Perbill::zero()], now, true); - - // nomination remains untouched. - assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - - // first validator is disabled but not slashed - assert!(is_disabled(11)); - - // --- Slash without disabling --- - // offence that slashes 50% of the bond (setup for next slash) - on_offence_in_era(&[offence_from(11, None)], &[Perbill::from_percent(50)], now, true); - - // offence that slashes 25% of the bond but does not disable - on_offence_in_era(&[offence_from(21, None)], &[Perbill::from_percent(25)], now, true); - - // nomination remains untouched. - assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - - // second validator is slashed but not disabled - assert!(!is_disabled(21)); - assert!(is_disabled(11)); - - assert_eq!( - staking_events_since_last_call(), - vec![ - Event::PagedElectionProceeded { page: 0, result: Ok(5) }, - Event::StakersElected, - Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, - Event::OffenceReported { - validator: 11, - fraction: Perbill::from_percent(0), - offence_era: 1 - }, - Event::OffenceReported { - validator: 11, - fraction: Perbill::from_percent(50), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 11, page: 0 }, - Event::Slashed { staker: 11, amount: 500 }, - Event::Slashed { staker: 101, amount: 62 }, - Event::OffenceReported { - validator: 21, - fraction: Perbill::from_percent(25), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 21, page: 0 }, - Event::Slashed { staker: 21, amount: 250 }, - Event::Slashed { staker: 101, amount: 94 } - ] - ); - - assert_eq!( - session_events(), - vec![ - SessionEvent::NewSession { session_index: 1 }, - SessionEvent::NewSession { session_index: 2 }, - SessionEvent::NewSession { session_index: 3 }, - SessionEvent::ValidatorDisabled { validator: 11 } - ] - ); - }); -} - -#[test] -fn offence_threshold_doesnt_plan_new_era() { - ExtBuilder::default() - .validator_count(4) - .set_status(41, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41]); - - assert_eq!( - UpToLimitWithReEnablingDisablingStrategy::::disable_limit( - Session::validators().len() - ), - 1 - ); - - // we have 4 validators and an offending validator threshold of 1/3, - // even if the third validator commits an offence a new era should not be forced - on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(50)], true); - - // 11 should be disabled because the byzantine threshold is 1 - assert!(is_disabled(11)); - - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - - on_offence_now(&[offence_from(21, None)], &[Perbill::zero()], true); - - // 21 should not be disabled because the number of disabled validators will be above the - // byzantine threshold - assert!(!is_disabled(21)); - - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - - on_offence_now(&[offence_from(31, None)], &[Perbill::zero()], true); - - // same for 31 - assert!(!is_disabled(31)); - - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - }); -} - -#[test] -fn disabled_validators_are_kept_disabled_for_whole_era() { - ExtBuilder::default() - .validator_count(7) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .set_status(201, StakerStatus::Validator) - .set_status(202, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); - assert_eq!(::SessionsPerEra::get(), 3); - - on_offence_now(&[offence_from(21, None)], &[Perbill::from_percent(25)], true); - - // nominations are not updated. - assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - - // 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(21)); - - // validator 11 commits an offence - on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(25)], true); - - // nominations are not updated. - assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - - advance_session(); - - // and both are disabled in the last session of the era - assert!(is_disabled(11)); - assert!(is_disabled(21)); - - mock::start_active_era(2); - - // when a new era starts disabled validators get cleared - assert!(!is_disabled(11)); - assert!(!is_disabled(21)); - }); -} - #[test] fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { // should check that: @@ -3732,9 +3352,8 @@ fn zero_slash_keeps_nominators() { assert_eq!(asset::stakeable_balance::(&11), 1000); assert_eq!(asset::stakeable_balance::(&101), 2000); - // 11 is not removed but disabled + // 11 is not removed assert!(Validators::::iter().any(|(stash, _)| stash == 11)); - assert!(is_disabled(11)); // and their nominations are kept. assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); }); @@ -8111,368 +7730,483 @@ mod ledger_recovery { } } -mod byzantine_threshold_disabling_strategy { - use crate::tests::{DisablingStrategy, Test, UpToLimitDisablingStrategy}; - use sp_runtime::Perbill; - use sp_staking::offence::OffenceSeverity; - - // Common test data - the stash of the offending validator, the era of the offence and the - // active set - const OFFENDER_ID: ::AccountId = 7; - const MAX_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(100)); - const MIN_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(0)); - const ACTIVE_SET: [::ValidatorId; 7] = [1, 2, 3, 4, 5, 6, 7]; - const OFFENDER_VALIDATOR_IDX: u32 = 6; // the offender is with index 6 in the active set - - // todo(ank4n): Ensure there is a test that for older eras, the disabling strategy does not - // disable the validator. +mod validator_disabling_integration { + use super::*; #[test] - fn dont_disable_beyond_byzantine_threshold() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(1, MIN_OFFENDER_SEVERITY), (2, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, - ); + fn reenable_lower_offenders() { + ExtBuilder::default() + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); - assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); - }); - } + // offence with a low slash + on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(10)], true); + on_offence_now(&[offence_from(21, None)], &[Perbill::from_percent(20)], true); - #[test] - fn disable_when_below_byzantine_threshold() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(1, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, - ); + // it does NOT affect the nominator. + assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); - assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); - }); - } -} + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); -mod disabling_strategy_with_reenabling { - use crate::tests::{DisablingStrategy, Test, UpToLimitWithReEnablingDisablingStrategy}; - use sp_runtime::Perbill; - use sp_staking::offence::OffenceSeverity; + // offence with a higher slash + on_offence_now(&[offence_from(31, None)], &[Perbill::from_percent(50)], true); - // Common test data - the stash of the offending validator, the era of the offence and the - // active set - const OFFENDER_ID: ::AccountId = 7; - const MAX_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(100)); - const LOW_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(0)); - const ACTIVE_SET: [::ValidatorId; 7] = [1, 2, 3, 4, 5, 6, 7]; - const OFFENDER_VALIDATOR_IDX: u32 = 6; // the offender is with index 6 in the active set + // First offender is no longer disabled + assert!(!is_disabled(11)); + // Mid offender is still disabled + assert!(is_disabled(21)); + // New offender is disabled + assert!(is_disabled(31)); - #[test] - fn disable_when_below_byzantine_threshold() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::PagedElectionProceeded { page: 0, result: Ok(7) }, + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::OffenceReported { + validator: 11, + fraction: Perbill::from_percent(10), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 11, + page: 0 + }, + Event::Slashed { staker: 11, amount: 100 }, + Event::Slashed { staker: 101, amount: 12 }, + Event::OffenceReported { + validator: 21, + fraction: Perbill::from_percent(20), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 21, + page: 0 + }, + Event::Slashed { staker: 21, amount: 200 }, + Event::Slashed { staker: 101, amount: 75 }, + Event::OffenceReported { + validator: 31, + fraction: Perbill::from_percent(50), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 31, + page: 0 + }, + Event::Slashed { staker: 31, amount: 250 }, + ] ); - // Disable Offender and do not re-enable anyone - assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); - assert_eq!(disabling_decision.reenable, None); - }); + assert!(matches!( + session_events().as_slice(), + &[ + .., + SessionEvent::ValidatorDisabled { validator: 11 }, + SessionEvent::ValidatorDisabled { validator: 21 }, + SessionEvent::ValidatorDisabled { validator: 31 }, + SessionEvent::ValidatorReenabled { validator: 11 }, + ] + )); + }); } #[test] - fn reenable_arbitrary_on_equal_severity() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, + fn do_not_reenable_higher_offenders() { + ExtBuilder::default() + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + + // offence with a major slash + on_offence_now( + &[offence_from(11, None), offence_from(21, None), offence_from(31, None)], + &[ + Perbill::from_percent(50), + Perbill::from_percent(50), + Perbill::from_percent(10), + ], + true, ); - assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); - // Disable 7 and enable 1 - assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); - assert_eq!(disabling_decision.reenable.unwrap(), 0); - }); - } + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); - #[test] - fn do_not_reenable_higher_offenders() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - LOW_OFFENDER_SEVERITY, - &initially_disabled, + // New offender is not disabled as limit is reached and his prio is lower + assert!(!is_disabled(31)); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::PagedElectionProceeded { page: 0, result: Ok(7) }, + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::OffenceReported { + validator: 11, + fraction: Perbill::from_percent(50), + offence_era: 1 + }, + Event::OffenceReported { + validator: 21, + fraction: Perbill::from_percent(50), + offence_era: 1 + }, + Event::OffenceReported { + validator: 31, + fraction: Perbill::from_percent(10), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 31, + page: 0 + }, + Event::Slashed { staker: 31, amount: 50 }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 21, + page: 0 + }, + Event::Slashed { staker: 21, amount: 500 }, + Event::Slashed { staker: 101, amount: 187 }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 11, + page: 0 + }, + Event::Slashed { staker: 11, amount: 500 }, + Event::Slashed { staker: 101, amount: 62 }, + ] ); - assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); - }); + assert!(matches!( + session_events().as_slice(), + &[ + .., + SessionEvent::ValidatorDisabled { validator: 11 }, + SessionEvent::ValidatorDisabled { validator: 21 }, + ] + )); + }); } #[test] - fn reenable_lower_offenders() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(0, LOW_OFFENDER_SEVERITY), (1, LOW_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, + fn clear_disabled_only_on_era_change() { + ExtBuilder::default() + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .session_per_era(3) + .build_and_execute(|| { + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + + // offence with a major slash + on_offence_now( + &[offence_from(11, None), offence_from(21, None)], + &[Perbill::from_percent(50), Perbill::from_percent(50)], + true, ); - assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); - // Disable 7 and enable 1 - assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); - assert_eq!(disabling_decision.reenable.unwrap(), 0); - }); - } + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); - #[test] - fn reenable_lower_offenders_unordered() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, LOW_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, - ); + // progress session and check if disablement is retained + start_session(2); + assert!(is_disabled(11)); + assert!(is_disabled(21)); - assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); - // Disable 7 and enable 1 - assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); - assert_eq!(disabling_decision.reenable.unwrap(), 1); - }); + // progress era (3 sessions per era) and clear disablement + start_session(3); + assert!(!is_disabled(11)); + assert!(!is_disabled(21)); + }); } #[test] - fn update_severity() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = - vec![(OFFENDER_VALIDATOR_IDX, LOW_OFFENDER_SEVERITY), (0, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, + fn validator_is_not_disabled_for_an_offence_in_previous_era() { + ExtBuilder::default() + .validator_count(4) + .set_status(41, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + + assert!(>::contains_key(11)); + assert!(Session::validators().contains(&11)); + + on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(0)], true); + + assert_eq!(ForceEra::::get(), Forcing::NotForcing); + assert!(is_disabled(11)); + + mock::start_active_era(2); + + // the validator is not disabled in the new era + Staking::validate(RuntimeOrigin::signed(11), Default::default()).unwrap(); + assert_eq!(ForceEra::::get(), Forcing::NotForcing); + assert!(>::contains_key(11)); + assert!(Session::validators().contains(&11)); + + mock::start_active_era(3); + + // an offence committed in era 1 is reported in era 3 + on_offence_in_era(&[offence_from(11, None)], &[Perbill::from_percent(0)], 1, true); + + // the validator doesn't get disabled for an old offence + assert!(Validators::::iter().any(|(stash, _)| stash == 11)); + assert!(!is_disabled(11)); + + // and we are not forcing a new era + assert_eq!(ForceEra::::get(), Forcing::NotForcing); + + on_offence_in_era( + &[offence_from(11, None)], + // NOTE: A 100% slash here would clean up the account, causing de-registration. + &[Perbill::from_percent(95)], + 1, + true, ); - assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_none()); - // Disable 7 "again" AKA update their severity - assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); - }); + // the validator doesn't get disabled again + assert!(Validators::::iter().any(|(stash, _)| stash == 11)); + assert!(!is_disabled(11)); + // and we are still not forcing a new era + assert_eq!(ForceEra::::get(), Forcing::NotForcing); + }); } #[test] - fn update_cannot_lower_severity() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = - vec![(OFFENDER_VALIDATOR_IDX, MAX_OFFENDER_SEVERITY), (0, MAX_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - LOW_OFFENDER_SEVERITY, - &initially_disabled, + fn non_slashable_offence_disables_validator() { + ExtBuilder::default() + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + + // offence with no slash associated + on_offence_now(&[offence_from(11, None)], &[Perbill::zero()], true); + + // it does NOT affect the nominator. + assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); + + // offence that slashes 25% of the bond + on_offence_now(&[offence_from(21, None)], &[Perbill::from_percent(25)], true); + + // it DOES NOT affect the nominator. + assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::PagedElectionProceeded { page: 0, result: Ok(7) }, + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::OffenceReported { + validator: 11, + fraction: Perbill::from_percent(0), + offence_era: 1 + }, + Event::OffenceReported { + validator: 21, + fraction: Perbill::from_percent(25), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 21, + page: 0 + }, + Event::Slashed { staker: 21, amount: 250 }, + Event::Slashed { staker: 101, amount: 94 } + ] ); - assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); - }); + assert!(matches!( + session_events().as_slice(), + &[ + .., + SessionEvent::ValidatorDisabled { validator: 11 }, + SessionEvent::ValidatorDisabled { validator: 21 }, + ] + )); + + // the offence for validator 11 wasn't slashable but it is disabled + assert!(is_disabled(11)); + // validator 21 gets disabled too + assert!(is_disabled(21)); + }); } #[test] - fn no_accidental_reenablement_on_repeated_offence() { - sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = - vec![(OFFENDER_VALIDATOR_IDX, MAX_OFFENDER_SEVERITY), (0, LOW_OFFENDER_SEVERITY)]; - pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - - let disabling_decision = - >::decision( - &OFFENDER_ID, - MAX_OFFENDER_SEVERITY, - &initially_disabled, + fn slashing_independent_of_disabling_validator() { + ExtBuilder::default() + .validator_count(5) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51]); + + let now = ActiveEra::::get().unwrap().index; + + // --- Disable without a slash --- + // offence with no slash associated + on_offence_in_era(&[offence_from(11, None)], &[Perbill::zero()], now, true); + + // nomination remains untouched. + assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); + + // first validator is disabled + assert!(is_disabled(11)); + + // --- Slash without disabling (because limit reached) --- + // offence that slashes 50% of the bond (setup for next slash) + on_offence_in_era( + &[offence_from(11, None)], + &[Perbill::from_percent(50)], + now, + true, ); - assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); - }); - } -} + // offence that slashes 25% of the bond but does not disable + on_offence_in_era( + &[offence_from(21, None)], + &[Perbill::from_percent(25)], + now, + true, + ); -#[test] -fn reenable_lower_offenders_mock() { - ExtBuilder::default() - .validator_count(7) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .set_status(201, StakerStatus::Validator) - .set_status(202, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + // nomination remains untouched. + assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - // offence with a low slash - on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(10)], true); - on_offence_now(&[offence_from(21, None)], &[Perbill::from_percent(20)], true); + // second validator is slashed but not disabled + assert!(!is_disabled(21)); + assert!(is_disabled(11)); - // it does NOT affect the nominator. - assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::PagedElectionProceeded { page: 0, result: Ok(5) }, + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::OffenceReported { + validator: 11, + fraction: Perbill::from_percent(0), + offence_era: 1 + }, + Event::OffenceReported { + validator: 11, + fraction: Perbill::from_percent(50), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 11, + page: 0 + }, + Event::Slashed { staker: 11, amount: 500 }, + Event::Slashed { staker: 101, amount: 62 }, + Event::OffenceReported { + validator: 21, + fraction: Perbill::from_percent(25), + offence_era: 1 + }, + Event::SlashComputed { + offence_era: 1, + slash_era: 1, + offender: 21, + page: 0 + }, + Event::Slashed { staker: 21, amount: 250 }, + Event::Slashed { staker: 101, amount: 94 } + ] + ); - // both validators should be disabled - assert!(is_disabled(11)); - assert!(is_disabled(21)); + assert_eq!( + session_events(), + vec![ + SessionEvent::NewSession { session_index: 1 }, + SessionEvent::NewSession { session_index: 2 }, + SessionEvent::NewSession { session_index: 3 }, + SessionEvent::ValidatorDisabled { validator: 11 } + ] + ); + }); + } - // offence with a higher slash - on_offence_now(&[offence_from(31, None)], &[Perbill::from_percent(50)], true); + #[test] + fn offence_threshold_doesnt_force_new_era() { + ExtBuilder::default() + .validator_count(4) + .set_status(41, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41]); - // First offender is no longer disabled - assert!(!is_disabled(11)); - // Mid offender is still disabled - assert!(is_disabled(21)); - // New offender is disabled - assert!(is_disabled(31)); + assert_eq!( + UpToLimitWithReEnablingDisablingStrategy::::disable_limit( + Session::validators().len() + ), + 1 + ); - assert_eq!( - staking_events_since_last_call(), - vec![ - Event::PagedElectionProceeded { page: 0, result: Ok(7) }, - Event::StakersElected, - Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, - Event::OffenceReported { - validator: 11, - fraction: Perbill::from_percent(10), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 11, page: 0 }, - Event::Slashed { staker: 11, amount: 100 }, - Event::Slashed { staker: 101, amount: 12 }, - Event::OffenceReported { - validator: 21, - fraction: Perbill::from_percent(20), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 21, page: 0 }, - Event::Slashed { staker: 21, amount: 200 }, - Event::Slashed { staker: 101, amount: 75 }, - Event::OffenceReported { - validator: 31, - fraction: Perbill::from_percent(50), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 31, page: 0 }, - Event::Slashed { staker: 31, amount: 250 }, - ] - ); + // we have 4 validators and an offending validator threshold of 1, + // even if two validators commit an offence a new era should not be forced + on_offence_now(&[offence_from(11, None)], &[Perbill::from_percent(50)], true); - assert!(matches!( - session_events().as_slice(), - &[ - .., - SessionEvent::ValidatorDisabled { validator: 11 }, - SessionEvent::ValidatorDisabled { validator: 21 }, - SessionEvent::ValidatorDisabled { validator: 31 }, - SessionEvent::ValidatorReenabled { validator: 11 }, - ] - )); - }); -} + // 11 should be disabled because the byzantine threshold is 1 + assert!(is_disabled(11)); -#[test] -fn do_not_reenable_higher_offenders_mock() { - ExtBuilder::default() - .validator_count(7) - .set_status(41, StakerStatus::Validator) - .set_status(51, StakerStatus::Validator) - .set_status(201, StakerStatus::Validator) - .set_status(202, StakerStatus::Validator) - .build_and_execute(|| { - mock::start_active_era(1); - assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + assert_eq!(ForceEra::::get(), Forcing::NotForcing); - // offence with a major slash - on_offence_now( - &[offence_from(11, None), offence_from(21, None), offence_from(31, None)], - &[Perbill::from_percent(50), Perbill::from_percent(50), Perbill::from_percent(10)], - true, - ); + on_offence_now(&[offence_from(21, None)], &[Perbill::zero()], true); - // both validators should be disabled - assert!(is_disabled(11)); - assert!(is_disabled(21)); + // 21 should not be disabled because the number of disabled validators will be above + // the byzantine threshold + assert!(!is_disabled(21)); - // New offender is not disabled as limit is reached and his prio is lower - assert!(!is_disabled(31)); + assert_eq!(ForceEra::::get(), Forcing::NotForcing); - assert_eq!( - staking_events_since_last_call(), - vec![ - Event::PagedElectionProceeded { page: 0, result: Ok(7) }, - Event::StakersElected, - Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, - Event::OffenceReported { - validator: 11, - fraction: Perbill::from_percent(50), - offence_era: 1 - }, - Event::OffenceReported { - validator: 21, - fraction: Perbill::from_percent(50), - offence_era: 1 - }, - Event::OffenceReported { - validator: 31, - fraction: Perbill::from_percent(10), - offence_era: 1 - }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 31, page: 0 }, - Event::Slashed { staker: 31, amount: 50 }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 21, page: 0 }, - Event::Slashed { staker: 21, amount: 500 }, - Event::Slashed { staker: 101, amount: 187 }, - Event::SlashComputed { offence_era: 1, slash_era: 1, offender: 11, page: 0 }, - Event::Slashed { staker: 11, amount: 500 }, - Event::Slashed { staker: 101, amount: 62 }, - ] - ); + on_offence_now(&[offence_from(31, None)], &[Perbill::zero()], true); - assert!(matches!( - session_events().as_slice(), - &[ - .., - SessionEvent::ValidatorDisabled { validator: 11 }, - SessionEvent::ValidatorDisabled { validator: 21 }, - ] - )); - }); + // same for 31 + assert!(!is_disabled(31)); + + assert_eq!(ForceEra::::get(), Forcing::NotForcing); + }); + } } #[cfg(all(feature = "try-runtime", test))] diff --git a/substrate/frame/support/src/traits/validation.rs b/substrate/frame/support/src/traits/validation.rs index 8a80df75f642c..2bb61b0a60e85 100644 --- a/substrate/frame/support/src/traits/validation.rs +++ b/substrate/frame/support/src/traits/validation.rs @@ -24,7 +24,7 @@ use sp_runtime::{ traits::{Convert, Zero}, BoundToRuntimeAppPublic, ConsensusEngineId, Permill, RuntimeAppPublic, }; -use sp_staking::SessionIndex; +use sp_staking::{offence::OffenceSeverity, SessionIndex}; /// A trait for online node inspection in a session. /// @@ -254,6 +254,8 @@ pub trait DisabledValidators { /// Returns all disabled validators fn disabled_validators() -> Vec; + + fn disabled_validators_with_severities() -> Vec<(u32, OffenceSeverity)>; } impl DisabledValidators for () { @@ -264,4 +266,8 @@ impl DisabledValidators for () { fn disabled_validators() -> Vec { vec![] } + + fn disabled_validators_with_severities() -> Vec<(u32, OffenceSeverity)> { + vec![] + } } diff --git a/substrate/primitives/staking/src/offence.rs b/substrate/primitives/staking/src/offence.rs index 9e3c0e5a1946b..973f5616d18e7 100644 --- a/substrate/primitives/staking/src/offence.rs +++ b/substrate/primitives/staking/src/offence.rs @@ -264,6 +264,26 @@ impl OffenceReportSystem for () { )] pub struct OffenceSeverity(pub Perbill); +impl OffenceSeverity { + /// Returns the maximum severity. + pub fn max_severity() -> Self { + Self(Perbill::from_percent(100)) + } + + /// Returns the minimum severity. + pub fn min_severity() -> Self { + Self(Perbill::from_percent(0)) + } +} + +impl Default for OffenceSeverity { + /// Default is the maximum severity. + /// When severity is unclear it is best to assume the worst. + fn default() -> Self { + Self::max_severity() + } +} + impl PartialOrd for OffenceSeverity { fn partial_cmp(&self, other: &Self) -> Option { self.0.partial_cmp(&other.0)