Validator Re-Enabling (#5724)

Aims to implement Stage 3 of Validator Disbling as outlined here: #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 <[email protected]> Co-authored-by: Ankan <[email protected]> Co-authored-by: Tsvetomir Dimitrov <[email protected]>
paritytech · Nov 19, 2024 · 8d4138f · 8d4138f
1 parent 0449b21
commit 8d4138f
Show file tree

Hide file tree

Showing 20 changed files with 864 additions and 87 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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