diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs index 035541fb17b1..36efd3decf77 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs @@ -85,7 +85,10 @@ impl CacheProvider { } impl TrieCacheProvider for CacheProvider { - type Cache<'a> = TrieCache<'a, H> where H: 'a; + type Cache<'a> + = TrieCache<'a, H> + where + H: 'a; fn as_trie_db_cache(&self, storage_root: ::Out) -> Self::Cache<'_> { TrieCache { diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index 4a478d047f1b..8dc2f20dd390 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -115,7 +115,10 @@ impl SizeOnlyRecorderProvider { } impl sp_trie::TrieRecorderProvider for SizeOnlyRecorderProvider { - type Recorder<'a> = SizeOnlyRecorder<'a, H> where H: 'a; + type Recorder<'a> + = SizeOnlyRecorder<'a, H> + where + H: 'a; fn drain_storage_proof(self) -> Option { None diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index d2ed5abb6ed1..69ce187dce40 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -395,7 +395,7 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = polkadot_runtime_common::StakingBenchmarkingConfig; type EventListeners = (); type WeightInfo = (); - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } parameter_types! { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 993010cbce66..7a5562cc98c1 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -755,7 +755,7 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = polkadot_runtime_common::StakingBenchmarkingConfig; type EventListeners = (NominationPools, DelegatedStaking); type WeightInfo = weights::pallet_staking::WeightInfo; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } impl pallet_fast_unstake::Config for Runtime { @@ -1836,6 +1836,7 @@ pub mod migrations { >, parachains_shared::migration::MigrateToV1, parachains_scheduler::migration::MigrateV2ToV3, + pallet_staking::migrations::v16::MigrateV15ToV16, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, ); diff --git a/prdoc/pr_5724.prdoc b/prdoc/pr_5724.prdoc new file mode 100644 index 000000000000..be9d21c214a8 --- /dev/null +++ b/prdoc/pr_5724.prdoc @@ -0,0 +1,37 @@ +# 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 Re-Enabling (master PR) + +doc: + - audience: Runtime Dev + description: | + Implementation of the Stage 3 for the New Disabling Strategy: https://github.com/paritytech/polkadot-sdk/issues/4359 + + This PR changes when an active validator node gets disabled for comitting offences. + When Byzantine Threshold Validators (1/3) are already disabled instead of no longer + disabling the highest offenders will be disabled potentially re-enabling low offenders. + + - audience: Node Operator + description: | + Implementation of the Stage 3 for the New Disabling Strategy: https://github.com/paritytech/polkadot-sdk/issues/4359 + + This PR changes when an active validator node gets disabled within parachain consensus (reduced responsibilities and + reduced rewards) for comitting offences. This should not affect active validators on a day-to-day basis and will only + be relevant when the network is under attack or there is a wide spread malfunction causing slashes. In that case + lowest offenders might get eventually re-enabled (back to normal responsibilities and normal rewards). + +migrations: + db: [] + runtime: + - reference: pallet-staking + description: | + Migrating `DisabledValidators` from `Vec` to `Vec<(u32, PerBill)>` where the PerBill represents the severity + of the offence in terms of the % slash. + +crates: + - name: pallet-staking + bump: minor + + - name: pallet-session + bump: minor diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 914b51fb5621..e68e04840776 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -742,7 +742,7 @@ impl pallet_staking::Config for Runtime { type EventListeners = NominationPools; type WeightInfo = pallet_staking::weights::SubstrateWeight; type BenchmarkingConfig = StakingBenchmarkingConfig; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } impl pallet_fast_unstake::Config for Runtime { diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs index 41928905ed95..26a6345e145f 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs @@ -147,30 +147,35 @@ fn mass_slash_doesnt_enter_emergency_phase() { let active_set_size_before_slash = Session::validators().len(); - // Slash more than 1/3 of the active validators - let mut slashed = slash_half_the_active_set(); + // assuming half is above the disabling limit (default 1/3), otherwise test will break + let slashed = slash_half_the_active_set(); let active_set_size_after_slash = Session::validators().len(); // active set should stay the same before and after the slash assert_eq!(active_set_size_before_slash, active_set_size_after_slash); - // Slashed validators are disabled up to a limit - slashed.truncate( - pallet_staking::UpToLimitDisablingStrategy::::disable_limit( - active_set_size_after_slash, - ), - ); - // Find the indices of the disabled validators let active_set = Session::validators(); - let expected_disabled = slashed + let potentially_disabled = slashed .into_iter() .map(|d| active_set.iter().position(|a| *a == d).unwrap() as u32) .collect::>(); + // Ensure that every actually disabled validator is also in the potentially disabled set + // (not necessarily the other way around) + let disabled = Session::disabled_validators(); + for d in disabled.iter() { + assert!(potentially_disabled.contains(d)); + } + + // Ensure no more than disabling limit of validators (default 1/3) is disabled + let disabling_limit = pallet_staking::UpToLimitWithReEnablingDisablingStrategy::< + SLASHING_DISABLING_FACTOR, + >::disable_limit(active_set_size_before_slash); + assert!(disabled.len() == disabling_limit); + assert_eq!(pallet_staking::ForceEra::::get(), pallet_staking::Forcing::NotForcing); - assert_eq!(Session::disabled_validators(), expected_disabled); }); } diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index b182ddec77ab..eaab848c1694 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -304,7 +304,8 @@ impl pallet_staking::Config for Runtime { type MaxUnlockingChunks = MaxUnlockingChunks; type EventListeners = Pools; type WeightInfo = pallet_staking::weights::SubstrateWeight; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = + pallet_staking::UpToLimitWithReEnablingDisablingStrategy; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; } diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index 325758d54dd8..e8b4a355f49a 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -127,8 +127,8 @@ use frame_support::{ dispatch::DispatchResult, ensure, traits::{ - EstimateNextNewSession, EstimateNextSessionRotation, FindAuthor, Get, OneSessionHandler, - ValidatorRegistration, ValidatorSet, + Defensive, EstimateNextNewSession, EstimateNextSessionRotation, FindAuthor, Get, + OneSessionHandler, ValidatorRegistration, ValidatorSet, }, weights::Weight, Parameter, @@ -735,6 +735,23 @@ impl Pallet { }) } + /// Re-enable the validator of index `i`, returns `false` if the validator was already enabled. + pub fn enable_index(i: u32) -> bool { + if i >= Validators::::decode_len().defensive_unwrap_or(0) as u32 { + return false + } + + // If the validator is not disabled, return false. + DisabledValidators::::mutate(|disabled| { + if let Ok(index) = disabled.binary_search(&i) { + disabled.remove(index); + true + } else { + false + } + }) + } + /// Disable the validator identified by `c`. (If using with the staking pallet, /// this would be their *stash* account.) /// diff --git a/substrate/frame/staking/CHANGELOG.md b/substrate/frame/staking/CHANGELOG.md index 113b7a6200b6..064a7d4a48f4 100644 --- a/substrate/frame/staking/CHANGELOG.md +++ b/substrate/frame/staking/CHANGELOG.md @@ -7,6 +7,18 @@ on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). We maintain a single integer version number for staking pallet to keep track of all storage migrations. +## [v16] + + +### Added + +- New default implementation of `DisablingStrategy` - `UpToLimitWithReEnablingDisablingStrategy`. + Same as `UpToLimitDisablingStrategy` except when a limit (1/3 default) is reached. When limit is + reached the offender is only disabled if his offence is greater or equal than some other already + disabled offender. The smallest possible offender is re-enabled to make space for the new greater + offender. A limit should thus always be respected. +- `DisabledValidators` changed format to include severity of the offence. + ## [v15] ### Added diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index a4a6e71af0df..6361663b2b1c 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -324,7 +324,7 @@ use sp_runtime::{ Perbill, Perquintill, Rounding, RuntimeDebug, Saturating, }; use sp_staking::{ - offence::{Offence, OffenceError, ReportOffence}, + offence::{Offence, OffenceError, OffenceSeverity, ReportOffence}, EraIndex, ExposurePage, OnStakingUpdate, Page, PagedExposureMetadata, SessionIndex, StakingAccount, }; @@ -849,6 +849,9 @@ pub trait SessionInterface { /// Disable the validator at the given index, returns `false` if the validator was already /// disabled or the index is out of bounds. fn disable_validator(validator_index: u32) -> bool; + /// Re-enable a validator that was previously disabled. Returns `false` if the validator was + /// already enabled or the index is out of bounds. + fn enable_validator(validator_index: u32) -> bool; /// Get the validators from session. fn validators() -> Vec; /// Prune historical session tries up to but not including the given index. @@ -873,6 +876,10 @@ where >::disable_index(validator_index) } + fn enable_validator(validator_index: u32) -> bool { + >::enable_index(validator_index) + } + fn validators() -> Vec<::AccountId> { >::validators() } @@ -886,6 +893,9 @@ impl SessionInterface for () { fn disable_validator(_: u32) -> bool { true } + fn enable_validator(_: u32) -> bool { + true + } fn validators() -> Vec { Vec::new() } @@ -1271,19 +1281,47 @@ impl BenchmarkingConfig for TestBenchmarkingConfig { /// Controls validator disabling pub trait DisablingStrategy { - /// Make a disabling decision. Returns the index of the validator to disable or `None` if no new - /// validator should be disabled. + /// Make a disabling decision. Returning a [`DisablingDecision`] fn decision( offender_stash: &T::AccountId, + offender_slash_severity: OffenceSeverity, slash_era: EraIndex, - currently_disabled: &Vec, - ) -> Option; + currently_disabled: &Vec<(u32, OffenceSeverity)>, + ) -> DisablingDecision; } -/// Implementation of [`DisablingStrategy`] which disables validators from the active set up to a -/// threshold. `DISABLING_LIMIT_FACTOR` is the factor of the maximum disabled validators in the -/// active set. E.g. setting this value to `3` means no more than 1/3 of the validators in the -/// active set can be disabled in an era. +/// Helper struct representing a decision coming from a given [`DisablingStrategy`] implementing +/// `decision` +/// +/// `disable` is the index of the validator to disable, +/// `reenable` is the index of the validator to re-enable. +#[derive(Debug)] +pub struct DisablingDecision { + pub disable: Option, + pub reenable: Option, +} + +/// Calculate the disabling limit based on the number of validators and the disabling limit factor. +/// +/// This is a sensible default implementation for the disabling limit factor for most disabling +/// strategies. +/// +/// Disabling limit factor n=2 -> 1/n = 1/2 = 50% of validators can be disabled +fn factor_based_disable_limit(validators_len: usize, disabling_limit_factor: usize) -> usize { + validators_len + .saturating_sub(1) + .checked_div(disabling_limit_factor) + .unwrap_or_else(|| { + defensive!("DISABLING_LIMIT_FACTOR should not be 0"); + 0 + }) +} + +/// Implementation of [`DisablingStrategy`] using factor_based_disable_limit which disables +/// validators from the active set up to a threshold. `DISABLING_LIMIT_FACTOR` is the factor of the +/// maximum disabled validators in the active set. E.g. setting this value to `3` means no more than +/// 1/3 of the validators in the active set can be disabled in an era. +/// /// By default a factor of 3 is used which is the byzantine threshold. pub struct UpToLimitDisablingStrategy; @@ -1291,13 +1329,7 @@ impl UpToLimitDisablingStrategy usize { - validators_len - .saturating_sub(1) - .checked_div(DISABLING_LIMIT_FACTOR) - .unwrap_or_else(|| { - defensive!("DISABLING_LIMIT_FACTOR should not be 0"); - 0 - }) + factor_based_disable_limit(validators_len, DISABLING_LIMIT_FACTOR) } } @@ -1306,9 +1338,10 @@ impl DisablingStrategy { fn decision( offender_stash: &T::AccountId, + _offender_slash_severity: OffenceSeverity, slash_era: EraIndex, - currently_disabled: &Vec, - ) -> Option { + currently_disabled: &Vec<(u32, OffenceSeverity)>, + ) -> DisablingDecision { let active_set = T::SessionInterface::validators(); // We don't disable more than the limit @@ -1318,7 +1351,7 @@ impl DisablingStrategy "Won't disable: reached disabling limit {:?}", Self::disable_limit(active_set.len()) ); - return None + return DisablingDecision { disable: None, reenable: None } } // We don't disable for offences in previous eras @@ -1329,18 +1362,116 @@ impl DisablingStrategy CurrentEra::::get().unwrap_or_default(), slash_era ); - return None + return DisablingDecision { disable: None, reenable: None } } let offender_idx = if let Some(idx) = active_set.iter().position(|i| i == offender_stash) { idx as u32 } else { log!(debug, "Won't disable: offender not in active set",); - return None + return DisablingDecision { disable: None, reenable: None } }; log!(debug, "Will disable {:?}", offender_idx); - Some(offender_idx) + DisablingDecision { disable: Some(offender_idx), reenable: None } + } +} + +/// Implementation of [`DisablingStrategy`] which disables validators from the active set up to a +/// limit (factor_based_disable_limit) and if the limit is reached and the new offender is higher +/// (bigger punishment/severity) then it re-enables the lowest offender to free up space for the new +/// offender. +/// +/// This strategy is not based on cumulative severity of offences but only on the severity of the +/// highest offence. Offender first committing a 25% offence and then a 50% offence will be treated +/// the same as an offender committing 50% offence. +/// +/// An extension of [`UpToLimitDisablingStrategy`]. +pub struct UpToLimitWithReEnablingDisablingStrategy; + +impl + UpToLimitWithReEnablingDisablingStrategy +{ + /// Disabling limit calculated from the total number of validators in the active set. When + /// reached re-enabling logic might kick in. + pub fn disable_limit(validators_len: usize) -> usize { + factor_based_disable_limit(validators_len, DISABLING_LIMIT_FACTOR) + } +} + +impl DisablingStrategy + for UpToLimitWithReEnablingDisablingStrategy +{ + fn decision( + offender_stash: &T::AccountId, + offender_slash_severity: OffenceSeverity, + slash_era: EraIndex, + currently_disabled: &Vec<(u32, OffenceSeverity)>, + ) -> DisablingDecision { + let active_set = T::SessionInterface::validators(); + + // We don't disable for offences in previous eras + if ActiveEra::::get().map(|e| e.index).unwrap_or_default() > slash_era { + log!( + debug, + "Won't disable: current_era {:?} > slash_era {:?}", + Pallet::::current_era().unwrap_or_default(), + slash_era + ); + return DisablingDecision { disable: None, reenable: None } + } + + // We don't disable validators that are not in the active set + let offender_idx = if let Some(idx) = active_set.iter().position(|i| i == offender_stash) { + idx as u32 + } else { + log!(debug, "Won't disable: offender not in active set",); + return DisablingDecision { disable: None, reenable: None } + }; + + // Check if offender is already disabled + if let Some((_, old_severity)) = + currently_disabled.iter().find(|(idx, _)| *idx == offender_idx) + { + if offender_slash_severity > *old_severity { + log!(debug, "Offender already disabled but with lower severity, will disable again to refresh severity of {:?}", offender_idx); + return DisablingDecision { disable: Some(offender_idx), reenable: None }; + } else { + log!(debug, "Offender already disabled with higher or equal severity"); + return DisablingDecision { disable: None, reenable: None }; + } + } + + // We don't disable more than the limit (but we can re-enable a smaller offender to make + // space) + if currently_disabled.len() >= Self::disable_limit(active_set.len()) { + log!( + debug, + "Reached disabling limit {:?}, checking for re-enabling", + Self::disable_limit(active_set.len()) + ); + + // Find the smallest offender to re-enable that is not higher than + // offender_slash_severity + if let Some((smallest_idx, _)) = currently_disabled + .iter() + .filter(|(_, severity)| *severity <= offender_slash_severity) + .min_by_key(|(_, severity)| *severity) + { + log!(debug, "Will disable {:?} and re-enable {:?}", offender_idx, smallest_idx); + return DisablingDecision { + disable: Some(offender_idx), + reenable: Some(*smallest_idx), + } + } else { + log!(debug, "No smaller offender found to re-enable"); + return DisablingDecision { disable: None, reenable: None } + } + } else { + // If we are not at the limit, just disable the new offender and dont re-enable anyone + log!(debug, "Will disable {:?}", offender_idx); + return DisablingDecision { disable: Some(offender_idx), reenable: None } + } } } diff --git a/substrate/frame/staking/src/migrations.rs b/substrate/frame/staking/src/migrations.rs index 5c9cf8613213..9dfa93c70b32 100644 --- a/substrate/frame/staking/src/migrations.rs +++ b/substrate/frame/staking/src/migrations.rs @@ -60,6 +60,79 @@ impl Default for ObsoleteReleases { #[storage_alias] type StorageVersion = StorageValue, ObsoleteReleases, ValueQuery>; +/// Migrating `DisabledValidators` from `Vec` to `Vec<(u32, OffenceSeverity)>` to track offense +/// severity for re-enabling purposes. +pub mod v16 { + use super::*; + use sp_staking::offence::OffenceSeverity; + + pub struct VersionUncheckedMigrateV15ToV16(core::marker::PhantomData); + impl UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV15ToV16 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + let old_disabled_validators = v15::DisabledValidators::::get(); + Ok(old_disabled_validators.encode()) + } + + fn on_runtime_upgrade() -> Weight { + // Migrating `DisabledValidators` from `Vec` to `Vec<(u32, OffenceSeverity)>`. + // Using max severity (PerBill 100%) for the migration which effectively makes it so + // offenders before the migration will not be re-enabled this era unless there are + // other 100% offenders. + let max_offence = OffenceSeverity(Perbill::from_percent(100)); + // Inject severity + let migrated = v15::DisabledValidators::::take() + .into_iter() + .map(|v| (v, max_offence)) + .collect::>(); + + DisabledValidators::::set(migrated); + + log!(info, "v16 applied successfully."); + T::DbWeight::get().reads_writes(1, 1) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { + // Decode state to get old_disabled_validators in a format of Vec + let old_disabled_validators = + Vec::::decode(&mut state.as_slice()).expect("Failed to decode state"); + let new_disabled_validators = DisabledValidators::::get(); + + // Compare lengths + frame_support::ensure!( + old_disabled_validators.len() == new_disabled_validators.len(), + "DisabledValidators length mismatch" + ); + + // Compare contents + let new_disabled_validators = + new_disabled_validators.into_iter().map(|(v, _)| v).collect::>(); + frame_support::ensure!( + old_disabled_validators == new_disabled_validators, + "DisabledValidator ids mismatch" + ); + + // Verify severity + let max_severity = OffenceSeverity(Perbill::from_percent(100)); + let new_disabled_validators = DisabledValidators::::get(); + for (_, severity) in new_disabled_validators { + frame_support::ensure!(severity == max_severity, "Severity mismatch"); + } + + Ok(()) + } + } + + pub type MigrateV15ToV16 = VersionedMigration< + 15, + 16, + VersionUncheckedMigrateV15ToV16, + Pallet, + ::DbWeight, + >; +} + /// Migrating `OffendingValidators` from `Vec<(u32, bool)>` to `Vec` pub mod v15 { use super::*; @@ -67,6 +140,9 @@ pub mod v15 { // The disabling strategy used by staking pallet type DefaultDisablingStrategy = UpToLimitDisablingStrategy; + #[storage_alias] + pub(crate) type DisabledValidators = StorageValue, Vec, ValueQuery>; + pub struct VersionUncheckedMigrateV14ToV15(core::marker::PhantomData); impl UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV14ToV15 { fn on_runtime_upgrade() -> Weight { diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 2d3446d2dabc..df8cb38e8b37 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -258,7 +258,8 @@ impl OnStakingUpdate for EventListenerMock { } } -// Disabling threshold for `UpToLimitDisablingStrategy` +// Disabling threshold for `UpToLimitDisablingStrategy` and +// `UpToLimitWithReEnablingDisablingStrategy`` pub(crate) const DISABLING_LIMIT_FACTOR: usize = 3; #[derive_impl(crate::config_preludes::TestDefaultConfig)] @@ -284,7 +285,8 @@ impl crate::pallet::pallet::Config for Test { type HistoryDepth = HistoryDepth; type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; type EventListeners = EventListenerMock; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = + pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } pub struct WeightedNominationsQuota; diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 972d0f3d47b9..2ae925d03643 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -510,7 +510,7 @@ impl Pallet { } // disable all offending validators that have been disabled for the whole era - for index in >::get() { + for (index, _) in >::get() { T::SessionInterface::disable_validator(index); } } @@ -1497,6 +1497,12 @@ where continue } + Self::deposit_event(Event::::SlashReported { + validator: stash.clone(), + fraction: *slash_fraction, + slash_era, + }); + let unapplied = slashing::compute_slash::(slashing::SlashParams { stash, slash: *slash_fraction, @@ -1507,12 +1513,6 @@ where reward_proportion, }); - Self::deposit_event(Event::::SlashReported { - validator: stash.clone(), - fraction: *slash_fraction, - slash_era, - }); - if let Some(mut unapplied) = unapplied { let nominators_len = unapplied.others.len() as u64; let reporters_len = details.reporters.len() as u64; @@ -2303,9 +2303,10 @@ impl Pallet { Ok(()) } + // Sorted by index fn ensure_disabled_validators_sorted() -> Result<(), TryRuntimeError> { ensure!( - DisabledValidators::::get().windows(2).all(|pair| pair[0] <= pair[1]), + DisabledValidators::::get().windows(2).all(|pair| pair[0].0 <= pair[1].0), "DisabledValidators is not sorted" ); Ok(()) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index d33b863a521a..b3f8c18f704c 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -38,6 +38,7 @@ use sp_runtime::{ }; use sp_staking::{ + offence::OffenceSeverity, EraIndex, Page, SessionIndex, StakingAccount::{self, Controller, Stash}, StakingInterface, @@ -69,7 +70,7 @@ pub mod pallet { use super::*; /// The in-code storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(15); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(16); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -704,11 +705,15 @@ pub mod pallet { /// implementor of [`DisablingStrategy`] defines if a validator should be disabled which /// implicitly means that the implementor also controls the max number of disabled validators. /// - /// The vec is always kept sorted so that we can find whether a given validator has previously - /// offended using binary search. + /// The vec is always kept sorted based on the u32 index so that we can find whether a given + /// validator has previously offended using binary search. + /// + /// Additionally, each disabled validator is associated with an `OffenceSeverity` which + /// represents how severe is the offence that got the validator disabled. #[pallet::storage] #[pallet::unbounded] - pub type DisabledValidators = StorageValue<_, Vec, ValueQuery>; + pub type DisabledValidators = + StorageValue<_, Vec<(u32, OffenceSeverity)>, 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 @@ -849,6 +854,10 @@ pub mod pallet { ForceEra { mode: Forcing }, /// Report of a controller batch deprecation. ControllerBatchDeprecated { failures: u32 }, + /// Validator has been disabled. + ValidatorDisabled { stash: T::AccountId }, + /// Validator has been re-enabled. + ValidatorReenabled { stash: T::AccountId }, } #[pallet::error] diff --git a/substrate/frame/staking/src/slashing.rs b/substrate/frame/staking/src/slashing.rs index 9fb782265b8b..ae76b0707dcb 100644 --- a/substrate/frame/staking/src/slashing.rs +++ b/substrate/frame/staking/src/slashing.rs @@ -65,7 +65,7 @@ use sp_runtime::{ traits::{Saturating, Zero}, DispatchResult, RuntimeDebug, }; -use sp_staking::{EraIndex, StakingInterface}; +use sp_staking::{offence::OffenceSeverity, EraIndex, StakingInterface}; /// The proportion of the slashing reward to be paid out on the first slashing detection. /// This is f_1 in the paper. @@ -321,17 +321,48 @@ fn kick_out_if_recent(params: SlashParams) { } /// Inform the [`DisablingStrategy`] implementation about the new offender and disable the list of -/// validators provided by [`make_disabling_decision`]. +/// validators provided by [`decision`]. fn add_offending_validator(params: &SlashParams) { DisabledValidators::::mutate(|disabled| { - if let Some(offender) = - T::DisablingStrategy::decision(params.stash, params.slash_era, &disabled) - { - // Add the validator to `DisabledValidators` and disable it. Do nothing if it is - // already disabled. - if let Err(index) = disabled.binary_search_by_key(&offender, |index| *index) { - disabled.insert(index, offender); - T::SessionInterface::disable_validator(offender); + let new_severity = OffenceSeverity(params.slash); + let decision = + T::DisablingStrategy::decision(params.stash, new_severity, params.slash_era, &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 new_severity > *old_severity { + *old_severity = new_severity; + } + }, + Err(index) => { + // Offender is not disabled, add to `DisabledValidators` and disable it + disabled.insert(index, (offender_idx, new_severity)); + // Propagate disablement to session level + T::SessionInterface::disable_validator(offender_idx); + // Emit event that a validator got disabled + >::deposit_event(super::Event::::ValidatorDisabled { + stash: params.stash.clone(), + }); + }, + } + } + + 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); + // Propagate re-enablement to session level + T::SessionInterface::enable_validator(reenable_idx); + // Emit event that a validator got re-enabled + let reenabled_stash = + T::SessionInterface::validators()[reenable_idx as usize].clone(); + >::deposit_event(super::Event::::ValidatorReenabled { + stash: reenabled_stash, + }); } } }); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index ffa317618f1f..6c2335e1aac8 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -3402,6 +3402,7 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid fraction: Perbill::from_percent(10), slash_era: 1 }, + Event::ValidatorDisabled { stash: 11 }, Event::Slashed { staker: 11, amount: 100 }, Event::Slashed { staker: 101, amount: 12 }, ] @@ -3474,11 +3475,13 @@ fn non_slashable_offence_disables_validator() { fraction: Perbill::from_percent(0), slash_era: 1 }, + Event::ValidatorDisabled { stash: 11 }, Event::SlashReported { validator: 21, fraction: Perbill::from_percent(25), slash_era: 1 }, + Event::ValidatorDisabled { stash: 21 }, Event::Slashed { staker: 21, amount: 250 }, Event::Slashed { staker: 101, amount: 94 } ] @@ -3506,6 +3509,7 @@ fn slashing_independent_of_disabling_validator() { let now = ActiveEra::::get().unwrap().index; + // --- Disable without a slash --- // offence with no slash associated on_offence_in_era( &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], @@ -3516,7 +3520,18 @@ fn slashing_independent_of_disabling_validator() { // nomination remains untouched. assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - // offence that slashes 25% of the bond + // 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( + &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + now, + ); + + // offence that slashes 25% of the bond but does not disable on_offence_in_era( &[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }], &[Perbill::from_percent(25)], @@ -3526,6 +3541,10 @@ fn slashing_independent_of_disabling_validator() { // 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![ @@ -3536,6 +3555,14 @@ fn slashing_independent_of_disabling_validator() { fraction: Perbill::from_percent(0), slash_era: 1 }, + Event::ValidatorDisabled { stash: 11 }, + Event::SlashReported { + validator: 11, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::Slashed { staker: 11, amount: 500 }, + Event::Slashed { staker: 101, amount: 62 }, Event::SlashReported { validator: 21, fraction: Perbill::from_percent(25), @@ -3545,11 +3572,6 @@ fn slashing_independent_of_disabling_validator() { Event::Slashed { staker: 101, amount: 94 } ] ); - - // first validator is disabled but not slashed - assert!(is_disabled(11)); - // second validator is slashed but not disabled - assert!(!is_disabled(21)); }); } @@ -3563,7 +3585,7 @@ fn offence_threshold_doesnt_trigger_new_era() { assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41]); assert_eq!( - UpToLimitDisablingStrategy::::disable_limit( + UpToLimitWithReEnablingDisablingStrategy::::disable_limit( Session::validators().len() ), 1 @@ -3578,7 +3600,7 @@ fn offence_threshold_doesnt_trigger_new_era() { on_offence_now( &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], - &[Perbill::zero()], + &[Perbill::from_percent(50)], ); // 11 should be disabled because the byzantine threshold is 1 @@ -8277,11 +8299,14 @@ mod byzantine_threshold_disabling_strategy { use crate::{ tests::Test, ActiveEra, ActiveEraInfo, DisablingStrategy, UpToLimitDisablingStrategy, }; - use sp_staking::EraIndex; + use sp_runtime::Perbill; + use sp_staking::{offence::OffenceSeverity, EraIndex}; // 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 SLASH_ERA: EraIndex = 1; 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 @@ -8293,48 +8318,431 @@ mod byzantine_threshold_disabling_strategy { pallet_session::Validators::::put(ACTIVE_SET.to_vec()); ActiveEra::::put(ActiveEraInfo { index: 2, start: None }); - let disable_offender = + let disabling_decision = >::decision( &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, SLASH_ERA, &initially_disabled, ); - assert!(disable_offender.is_none()); + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); }); } #[test] fn dont_disable_beyond_byzantine_threshold() { sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![1, 2]; + let initially_disabled = vec![(1, MIN_OFFENDER_SEVERITY), (2, MAX_OFFENDER_SEVERITY)]; pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - let disable_offender = + let disabling_decision = >::decision( &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, SLASH_ERA, &initially_disabled, ); - assert!(disable_offender.is_none()); + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); }); } #[test] fn disable_when_below_byzantine_threshold() { sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![1]; + let initially_disabled = vec![(1, MAX_OFFENDER_SEVERITY)]; pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - let disable_offender = + let disabling_decision = >::decision( &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + }); + } +} + +mod disabling_strategy_with_reenabling { + use crate::{ + tests::Test, ActiveEra, ActiveEraInfo, DisablingStrategy, + UpToLimitWithReEnablingDisablingStrategy, + }; + use sp_runtime::Perbill; + use sp_staking::{offence::OffenceSeverity, EraIndex}; + + // 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 SLASH_ERA: EraIndex = 1; + 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 dont_disable_for_ancient_offence() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + ActiveEra::::put(ActiveEraInfo { index: 2, start: None }); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } + + #[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, + SLASH_ERA, + &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)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &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)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + LOW_OFFENDER_SEVERITY, SLASH_ERA, &initially_disabled, ); - assert_eq!(disable_offender, Some(OFFENDER_VALIDATOR_IDX)); + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } + + #[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, + SLASH_ERA, + &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 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, + SLASH_ERA, + &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)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &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)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + LOW_OFFENDER_SEVERITY, + SLASH_ERA, + &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)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } +} + +#[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]); + + 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 exposure_31 = Staking::eras_stakers(Staking::active_era().unwrap().index, &31); + + // offence with a low slash + on_offence_now( + &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], + &[Perbill::from_percent(10)], + ); + on_offence_now( + &[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }], + &[Perbill::from_percent(20)], + ); + + // it does NOT affect the nominator. + assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); + + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); + + // offence with a higher slash + on_offence_now( + &[OffenceDetails { offender: (31, exposure_31.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + ); + + // 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!( + staking_events_since_last_call(), + vec![ + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::SlashReported { + validator: 11, + fraction: Perbill::from_percent(10), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 11 }, + Event::Slashed { staker: 11, amount: 100 }, + Event::Slashed { staker: 101, amount: 12 }, + Event::SlashReported { + validator: 21, + fraction: Perbill::from_percent(20), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 21 }, + Event::Slashed { staker: 21, amount: 200 }, + Event::Slashed { staker: 101, amount: 75 }, + Event::SlashReported { + validator: 31, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 31 }, + Event::ValidatorReenabled { stash: 11 }, + Event::Slashed { staker: 31, amount: 250 }, + ] + ); + }); +} + +#[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]); + + 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 exposure_31 = Staking::eras_stakers(Staking::active_era().unwrap().index, &31); + + // offence with a major slash + on_offence_now( + &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + ); + on_offence_now( + &[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + ); + + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); + + // offence with a minor slash + on_offence_now( + &[OffenceDetails { offender: (31, exposure_31.clone()), reporters: vec![] }], + &[Perbill::from_percent(10)], + ); + + // First and second offenders are still disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); + // 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::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::SlashReported { + validator: 11, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 11 }, + Event::Slashed { staker: 11, amount: 500 }, + Event::Slashed { staker: 101, amount: 62 }, + Event::SlashReported { + validator: 21, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 21 }, + Event::Slashed { staker: 21, amount: 500 }, + Event::Slashed { staker: 101, amount: 187 }, + Event::SlashReported { + validator: 31, + fraction: Perbill::from_percent(10), + slash_era: 1 + }, + Event::Slashed { staker: 31, amount: 50 }, + ] + ); + }); +} + +#[cfg(all(feature = "try-runtime", test))] +mod migration_tests { + use super::*; + use frame_support::traits::UncheckedOnRuntimeUpgrade; + use migrations::{v15, v16}; + + #[test] + fn migrate_v15_to_v16_with_try_runtime() { + ExtBuilder::default().validator_count(7).build_and_execute(|| { + // Initial setup: Create old `DisabledValidators` in the form of `Vec` + let old_disabled_validators = vec![1u32, 2u32]; + v15::DisabledValidators::::put(old_disabled_validators.clone()); + + // Run pre-upgrade checks + let pre_upgrade_result = v16::VersionUncheckedMigrateV15ToV16::::pre_upgrade(); + assert!(pre_upgrade_result.is_ok()); + let pre_upgrade_state = pre_upgrade_result.unwrap(); + + // Run the migration + v16::VersionUncheckedMigrateV15ToV16::::on_runtime_upgrade(); + + // Run post-upgrade checks + let post_upgrade_result = + v16::VersionUncheckedMigrateV15ToV16::::post_upgrade(pre_upgrade_state); + assert!(post_upgrade_result.is_ok()); }); } } diff --git a/substrate/primitives/staking/src/offence.rs b/substrate/primitives/staking/src/offence.rs index 2c2ebc1fc971..e73e8efe5839 100644 --- a/substrate/primitives/staking/src/offence.rs +++ b/substrate/primitives/staking/src/offence.rs @@ -242,3 +242,28 @@ impl OffenceReportSystem for () { Ok(()) } } + +/// Wrapper type representing the severity of an offence. +/// +/// As of now the only meaningful value taken into account +/// when deciding the severity of an offence is the associated +/// slash amount `Perbill`. +/// +/// For instance used for the purposes of distinguishing who should be +/// prioritized for disablement. +#[derive( + Clone, Copy, PartialEq, Eq, Encode, Decode, sp_runtime::RuntimeDebug, scale_info::TypeInfo, +)] +pub struct OffenceSeverity(pub Perbill); + +impl PartialOrd for OffenceSeverity { + fn partial_cmp(&self, other: &Self) -> Option { + self.0.partial_cmp(&other.0) + } +} + +impl Ord for OffenceSeverity { + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + self.0.cmp(&other.0) + } +} diff --git a/substrate/primitives/state-machine/src/trie_backend.rs b/substrate/primitives/state-machine/src/trie_backend.rs index f91ce5d2e52f..8d4dfd34240d 100644 --- a/substrate/primitives/state-machine/src/trie_backend.rs +++ b/substrate/primitives/state-machine/src/trie_backend.rs @@ -73,7 +73,10 @@ pub trait TrieCacheProvider { #[cfg(feature = "std")] impl TrieCacheProvider for LocalTrieCache { - type Cache<'a> = TrieCache<'a, H> where H: 'a; + type Cache<'a> + = TrieCache<'a, H> + where + H: 'a; fn as_trie_db_cache(&self, storage_root: H::Out) -> Self::Cache<'_> { self.as_trie_db_cache(storage_root) @@ -90,7 +93,10 @@ impl TrieCacheProvider for LocalTrieCache { #[cfg(feature = "std")] impl TrieCacheProvider for &LocalTrieCache { - type Cache<'a> = TrieCache<'a, H> where Self: 'a; + type Cache<'a> + = TrieCache<'a, H> + where + Self: 'a; fn as_trie_db_cache(&self, storage_root: H::Out) -> Self::Cache<'_> { (*self).as_trie_db_cache(storage_root) @@ -139,7 +145,10 @@ impl trie_db::TrieCache> for UnimplementedCacheProvider< #[cfg(not(feature = "std"))] impl TrieCacheProvider for UnimplementedCacheProvider { - type Cache<'a> = UnimplementedCacheProvider where H: 'a; + type Cache<'a> + = UnimplementedCacheProvider + where + H: 'a; fn as_trie_db_cache(&self, _storage_root: ::Out) -> Self::Cache<'_> { unimplemented!() @@ -176,7 +185,10 @@ impl trie_db::TrieRecorder for UnimplementedRecorderProvider< #[cfg(not(feature = "std"))] impl TrieRecorderProvider for UnimplementedRecorderProvider { - type Recorder<'a> = UnimplementedRecorderProvider where H: 'a; + type Recorder<'a> + = UnimplementedRecorderProvider + where + H: 'a; fn drain_storage_proof(self) -> Option { unimplemented!() diff --git a/substrate/primitives/trie/src/recorder.rs b/substrate/primitives/trie/src/recorder.rs index 2886577eddc6..4ec13066ded7 100644 --- a/substrate/primitives/trie/src/recorder.rs +++ b/substrate/primitives/trie/src/recorder.rs @@ -252,7 +252,10 @@ pub struct TrieRecorder<'a, H: Hasher> { } impl crate::TrieRecorderProvider for Recorder { - type Recorder<'a> = TrieRecorder<'a, H> where H: 'a; + type Recorder<'a> + = TrieRecorder<'a, H> + where + H: 'a; fn drain_storage_proof(self) -> Option { Some(Recorder::drain_storage_proof(self))