From 8d4138f77106a6af49920ad84f3283f696f3f905 Mon Sep 17 00:00:00 2001
From: Maciej <maciej.zyszkiewicz@parity.io>
Date: Tue, 19 Nov 2024 14:40:25 +0000
Subject: [PATCH] Validator Re-Enabling (#5724)
Aims to implement Stage 3 of Validator Disbling as outlined here:
https://github.com/paritytech/polkadot-sdk/issues/4359
Features:
- [x] New Disabling Strategy (Staking level)
- [x] Re-enabling logic (Session level)
- [x] More generic disabling decision output
- [x] New Disabling Events
Testing & Security:
- [x] Unit tests
- [x] Mock tests
- [x] Try-runtime checks
- [x] Try-runtime tested on westend snap
- [x] Try-runtime CI tests
- [ ] Re-enabling Zombienet Test (?)
- [ ] SRLabs Audit
Closes #4745
Closes #2418
---------
Co-authored-by: ordian <write@reusable.software>
Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
Co-authored-by: Tsvetomir Dimitrov <tsvetomir@parity.io>
---
.../src/validate_block/trie_cache.rs | 5 +-
.../src/validate_block/trie_recorder.rs | 5 +-
polkadot/runtime/test-runtime/src/lib.rs | 2 +-
polkadot/runtime/westend/src/lib.rs | 3 +-
prdoc/pr_5724.prdoc | 37 ++
substrate/bin/node/runtime/src/lib.rs | 2 +-
.../test-staking-e2e/src/lib.rs | 27 +-
.../test-staking-e2e/src/mock.rs | 3 +-
substrate/frame/session/src/lib.rs | 21 +-
substrate/frame/staking/CHANGELOG.md | 12 +
substrate/frame/staking/src/lib.rs | 175 ++++++-
substrate/frame/staking/src/migrations.rs | 76 +++
substrate/frame/staking/src/mock.rs | 6 +-
substrate/frame/staking/src/pallet/impls.rs | 17 +-
substrate/frame/staking/src/pallet/mod.rs | 17 +-
substrate/frame/staking/src/slashing.rs | 51 +-
substrate/frame/staking/src/tests.rs | 442 +++++++++++++++++-
substrate/primitives/staking/src/offence.rs | 25 +
.../state-machine/src/trie_backend.rs | 20 +-
substrate/primitives/trie/src/recorder.rs | 5 +-
20 files changed, 864 insertions(+), 87 deletions(-)
create mode 100644 prdoc/pr_5724.prdoc
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 035541fb17b..36efd3decf7 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<H: Hasher> CacheProvider<H> {
}
impl<H: Hasher> TrieCacheProvider<H> for CacheProvider<H> {
- 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 as Hasher>::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 4a478d047f1..8dc2f20dd39 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<H: Hasher> SizeOnlyRecorderProvider<H> {
}
impl<H: trie_db::Hasher> sp_trie::TrieRecorderProvider<H> for SizeOnlyRecorderProvider<H> {
- type Recorder<'a> = SizeOnlyRecorder<'a, H> where H: 'a;
+ type Recorder<'a>
+ = SizeOnlyRecorder<'a, H>
+ where
+ H: 'a;
fn drain_storage_proof(self) -> Option<StorageProof> {
None
diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs
index d2ed5abb6ed..69ce187dce4 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 993010cbce6..7a5562cc98c 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<Runtime>;
- 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<Runtime>,
parachains_scheduler::migration::MigrateV2ToV3<Runtime>,
+ pallet_staking::migrations::v16::MigrateV15ToV16<Runtime>,
// permanent
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
);
diff --git a/prdoc/pr_5724.prdoc b/prdoc/pr_5724.prdoc
new file mode 100644
index 00000000000..be9d21c214a
--- /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<u32>` 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 914b51fb562..e68e0484077 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<Runtime>;
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 41928905ed9..26a6345e145 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::<SLASHING_DISABLING_FACTOR>::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::<Vec<_>>();
+ // 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::<Runtime>::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 b182ddec77a..eaab848c169 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<Runtime>;
- type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy<SLASHING_DISABLING_FACTOR>;
+ type DisablingStrategy =
+ pallet_staking::UpToLimitWithReEnablingDisablingStrategy<SLASHING_DISABLING_FACTOR>;
type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
}
diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs
index 325758d54dd..e8b4a355f49 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<T: Config> Pallet<T> {
})
}
+ /// 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::<T>::decode_len().defensive_unwrap_or(0) as u32 {
+ return false
+ }
+
+ // If the validator is not disabled, return false.
+ DisabledValidators::<T>::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 113b7a6200b..064a7d4a48f 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 a4a6e71af0d..6361663b2b1 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<AccountId> {
/// 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<AccountId>;
/// Prune historical session tries up to but not including the given index.
@@ -873,6 +876,10 @@ where
<pallet_session::Pallet<T>>::disable_index(validator_index)
}
+ fn enable_validator(validator_index: u32) -> bool {
+ <pallet_session::Pallet<T>>::enable_index(validator_index)
+ }
+
fn validators() -> Vec<<T as frame_system::Config>::AccountId> {
<pallet_session::Pallet<T>>::validators()
}
@@ -886,6 +893,9 @@ impl<AccountId> SessionInterface<AccountId> for () {
fn disable_validator(_: u32) -> bool {
true
}
+ fn enable_validator(_: u32) -> bool {
+ true
+ }
fn validators() -> Vec<AccountId> {
Vec::new()
}
@@ -1271,19 +1281,47 @@ impl BenchmarkingConfig for TestBenchmarkingConfig {
/// Controls validator disabling
pub trait DisablingStrategy<T: Config> {
- /// 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<u32>,
- ) -> Option<u32>;
+ 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<u32>,
+ pub reenable: Option<u32>,
+}
+
+/// 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<const DISABLING_LIMIT_FACTOR: usize = 3>;
@@ -1291,13 +1329,7 @@ impl<const DISABLING_LIMIT_FACTOR: usize> UpToLimitDisablingStrategy<DISABLING_L
/// Disabling limit calculated from the total number of validators in the active set. When
/// reached no more validators will be disabled.
pub fn disable_limit(validators_len: usize) -> 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<T: Config, const DISABLING_LIMIT_FACTOR: usize> DisablingStrategy<T>
{
fn decision(
offender_stash: &T::AccountId,
+ _offender_slash_severity: OffenceSeverity,
slash_era: EraIndex,
- currently_disabled: &Vec<u32>,
- ) -> Option<u32> {
+ 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<T: Config, const DISABLING_LIMIT_FACTOR: usize> DisablingStrategy<T>
"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<T: Config, const DISABLING_LIMIT_FACTOR: usize> DisablingStrategy<T>
CurrentEra::<T>::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<const DISABLING_LIMIT_FACTOR: usize = 3>;
+
+impl<const DISABLING_LIMIT_FACTOR: usize>
+ UpToLimitWithReEnablingDisablingStrategy<DISABLING_LIMIT_FACTOR>
+{
+ /// 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<T: Config, const DISABLING_LIMIT_FACTOR: usize> DisablingStrategy<T>
+ for UpToLimitWithReEnablingDisablingStrategy<DISABLING_LIMIT_FACTOR>
+{
+ 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::<T>::get().map(|e| e.index).unwrap_or_default() > slash_era {
+ log!(
+ debug,
+ "Won't disable: current_era {:?} > slash_era {:?}",
+ Pallet::<T>::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 5c9cf861321..9dfa93c70b3 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<T: Config> = StorageValue<Pallet<T>, ObsoleteReleases, ValueQuery>;
+/// Migrating `DisabledValidators` from `Vec<u32>` 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<T>(core::marker::PhantomData<T>);
+ impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV15ToV16<T> {
+ #[cfg(feature = "try-runtime")]
+ fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
+ let old_disabled_validators = v15::DisabledValidators::<T>::get();
+ Ok(old_disabled_validators.encode())
+ }
+
+ fn on_runtime_upgrade() -> Weight {
+ // Migrating `DisabledValidators` from `Vec<u32>` 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::<T>::take()
+ .into_iter()
+ .map(|v| (v, max_offence))
+ .collect::<Vec<_>>();
+
+ DisabledValidators::<T>::set(migrated);
+
+ log!(info, "v16 applied successfully.");
+ T::DbWeight::get().reads_writes(1, 1)
+ }
+
+ #[cfg(feature = "try-runtime")]
+ fn post_upgrade(state: Vec<u8>) -> Result<(), TryRuntimeError> {
+ // Decode state to get old_disabled_validators in a format of Vec<u32>
+ let old_disabled_validators =
+ Vec::<u32>::decode(&mut state.as_slice()).expect("Failed to decode state");
+ let new_disabled_validators = DisabledValidators::<T>::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::<Vec<_>>();
+ 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::<T>::get();
+ for (_, severity) in new_disabled_validators {
+ frame_support::ensure!(severity == max_severity, "Severity mismatch");
+ }
+
+ Ok(())
+ }
+ }
+
+ pub type MigrateV15ToV16<T> = VersionedMigration<
+ 15,
+ 16,
+ VersionUncheckedMigrateV15ToV16<T>,
+ Pallet<T>,
+ <T as frame_system::Config>::DbWeight,
+ >;
+}
+
/// Migrating `OffendingValidators` from `Vec<(u32, bool)>` to `Vec<u32>`
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<T: Config> = StorageValue<Pallet<T>, Vec<u32>, ValueQuery>;
+
pub struct VersionUncheckedMigrateV14ToV15<T>(core::marker::PhantomData<T>);
impl<T: Config> UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV14ToV15<T> {
fn on_runtime_upgrade() -> Weight {
diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs
index 2d3446d2dab..df8cb38e8b3 100644
--- a/substrate/frame/staking/src/mock.rs
+++ b/substrate/frame/staking/src/mock.rs
@@ -258,7 +258,8 @@ impl OnStakingUpdate<AccountId, Balance> 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<DISABLING_LIMIT_FACTOR>;
+ type DisablingStrategy =
+ pallet_staking::UpToLimitWithReEnablingDisablingStrategy<DISABLING_LIMIT_FACTOR>;
}
pub struct WeightedNominationsQuota<const MAX: u32>;
diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs
index 972d0f3d47b..2ae925d0364 100644
--- a/substrate/frame/staking/src/pallet/impls.rs
+++ b/substrate/frame/staking/src/pallet/impls.rs
@@ -510,7 +510,7 @@ impl<T: Config> Pallet<T> {
}
// disable all offending validators that have been disabled for the whole era
- for index in <DisabledValidators<T>>::get() {
+ for (index, _) in <DisabledValidators<T>>::get() {
T::SessionInterface::disable_validator(index);
}
}
@@ -1497,6 +1497,12 @@ where
continue
}
+ Self::deposit_event(Event::<T>::SlashReported {
+ validator: stash.clone(),
+ fraction: *slash_fraction,
+ slash_era,
+ });
+
let unapplied = slashing::compute_slash::<T>(slashing::SlashParams {
stash,
slash: *slash_fraction,
@@ -1507,12 +1513,6 @@ where
reward_proportion,
});
- Self::deposit_event(Event::<T>::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<T: Config> Pallet<T> {
Ok(())
}
+ // Sorted by index
fn ensure_disabled_validators_sorted() -> Result<(), TryRuntimeError> {
ensure!(
- DisabledValidators::<T>::get().windows(2).all(|pair| pair[0] <= pair[1]),
+ DisabledValidators::<T>::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 d33b863a521..b3f8c18f704 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<T: Config> = StorageValue<_, Vec<u32>, ValueQuery>;
+ pub type DisabledValidators<T: Config> =
+ 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 9fb782265b8..ae76b0707dc 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<T: Config>(params: SlashParams<T>) {
}
/// 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<T: Config>(params: &SlashParams<T>) {
DisabledValidators::<T>::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
+ <Pallet<T>>::deposit_event(super::Event::<T>::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();
+ <Pallet<T>>::deposit_event(super::Event::<T>::ValidatorReenabled {
+ stash: reenabled_stash,
+ });
}
}
});
diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs
index ffa317618f1..6c2335e1aac 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::<Test>::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::<Test>::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::<Test>::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::<DISABLING_LIMIT_FACTOR>::disable_limit(
+ UpToLimitWithReEnablingDisablingStrategy::<DISABLING_LIMIT_FACTOR>::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: <Test as frame_system::Config>::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: [<Test as pallet_session::Config>::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::<Test>::put(ACTIVE_SET.to_vec());
ActiveEra::<Test>::put(ActiveEraInfo { index: 2, start: None });
- let disable_offender =
+ let disabling_decision =
<UpToLimitDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
- let disable_offender =
+ let disabling_decision =
<UpToLimitDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
- let disable_offender =
+ let disabling_decision =
<UpToLimitDisablingStrategy as DisablingStrategy<Test>>::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: <Test as frame_system::Config>::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: [<Test as pallet_session::Config>::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::<Test>::put(ACTIVE_SET.to_vec());
+ ActiveEra::<Test>::put(ActiveEraInfo { index: 2, start: None });
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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::<Test>::put(ACTIVE_SET.to_vec());
+
+ let disabling_decision =
+ <UpToLimitWithReEnablingDisablingStrategy as DisablingStrategy<Test>>::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<u32>`
+ let old_disabled_validators = vec![1u32, 2u32];
+ v15::DisabledValidators::<Test>::put(old_disabled_validators.clone());
+
+ // Run pre-upgrade checks
+ let pre_upgrade_result = v16::VersionUncheckedMigrateV15ToV16::<Test>::pre_upgrade();
+ assert!(pre_upgrade_result.is_ok());
+ let pre_upgrade_state = pre_upgrade_result.unwrap();
+
+ // Run the migration
+ v16::VersionUncheckedMigrateV15ToV16::<Test>::on_runtime_upgrade();
+
+ // Run post-upgrade checks
+ let post_upgrade_result =
+ v16::VersionUncheckedMigrateV15ToV16::<Test>::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 2c2ebc1fc97..e73e8efe583 100644
--- a/substrate/primitives/staking/src/offence.rs
+++ b/substrate/primitives/staking/src/offence.rs
@@ -242,3 +242,28 @@ impl<Reporter, Evidence> OffenceReportSystem<Reporter, Evidence> 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<core::cmp::Ordering> {
+ 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 f91ce5d2e52..8d4dfd34240 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<H: Hasher> {
#[cfg(feature = "std")]
impl<H: Hasher> TrieCacheProvider<H> for LocalTrieCache<H> {
- 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<H: Hasher> TrieCacheProvider<H> for LocalTrieCache<H> {
#[cfg(feature = "std")]
impl<H: Hasher> TrieCacheProvider<H> for &LocalTrieCache<H> {
- 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<H: Hasher> trie_db::TrieCache<NodeCodec<H>> for UnimplementedCacheProvider<
#[cfg(not(feature = "std"))]
impl<H: Hasher> TrieCacheProvider<H> for UnimplementedCacheProvider<H> {
- type Cache<'a> = UnimplementedCacheProvider<H> where H: 'a;
+ type Cache<'a>
+ = UnimplementedCacheProvider<H>
+ where
+ H: 'a;
fn as_trie_db_cache(&self, _storage_root: <H as Hasher>::Out) -> Self::Cache<'_> {
unimplemented!()
@@ -176,7 +185,10 @@ impl<H: Hasher> trie_db::TrieRecorder<H::Out> for UnimplementedRecorderProvider<
#[cfg(not(feature = "std"))]
impl<H: Hasher> TrieRecorderProvider<H> for UnimplementedRecorderProvider<H> {
- type Recorder<'a> = UnimplementedRecorderProvider<H> where H: 'a;
+ type Recorder<'a>
+ = UnimplementedRecorderProvider<H>
+ where
+ H: 'a;
fn drain_storage_proof(self) -> Option<StorageProof> {
unimplemented!()
diff --git a/substrate/primitives/trie/src/recorder.rs b/substrate/primitives/trie/src/recorder.rs
index 2886577eddc..4ec13066ded 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<H: Hasher> crate::TrieRecorderProvider<H> for Recorder<H> {
- type Recorder<'a> = TrieRecorder<'a, H> where H: 'a;
+ type Recorder<'a>
+ = TrieRecorder<'a, H>
+ where
+ H: 'a;
fn drain_storage_proof(self) -> Option<StorageProof> {
Some(Recorder::drain_storage_proof(self))
--
GitLab