Companion of paritytech/substrate#9448

frame/staking/src/impls.rs

@@ -482,6 +482,12 @@ impl<T: Config> Pallet<T> {
 				Self::start_era(start_session);
 			}
 		}
+
+		for (index, disabled) in <OffendingValidators<T>>::get() {
+			if disabled {
+				T::SessionInterface::disable_validator(index);
+			}
+		}
 	}
 
 	/// End a session potentially ending an era.
@@ -565,6 +571,9 @@ impl<T: Config> Pallet<T> {
 			<ErasValidatorReward<T>>::insert(&active_era.index, validator_payout);
 			T::RingCurrency::deposit_creating(&Self::account_id(), validator_payout);
 			T::RingRewardRemainder::on_unbalanced(T::RingCurrency::issue(rest));
+
+			// Clear offending validators.
+			<OffendingValidators<T>>::kill();
 		}
 	}
 
@@ -1541,12 +1550,9 @@ where
 ///
 /// This is needed because `Staking` sets the `ValidatorIdOf` of the `pallet_session::Config`
 pub trait SessionInterface<AccountId>: frame_system::Config {
-	/// Disable a given validator by stash ID.
-	///
-	/// Returns `true` if new era should be forced at the end of this session.
-	/// This allows preventing a situation where there is too many validators
-	/// disabled and block production stalls.
-	fn disable_validator(validator: &AccountId) -> Result<bool, ()>;
+	/// 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;
 	/// Get the validators from session.
 	fn validators() -> Vec<AccountId>;
 	/// Prune historical session tries up to but not including the given index.
@@ -1563,8 +1569,8 @@ where
 	T::SessionManager: pallet_session::SessionManager<AccountId<T>>,
 	T::ValidatorIdOf: Convert<AccountId<T>, Option<AccountId<T>>>,
 {
-	fn disable_validator(validator: &AccountId<T>) -> Result<bool, ()> {
-		<pallet_session::Pallet<T>>::disable(validator)
+	fn disable_validator(validator_index: u32) -> bool {
+		<pallet_session::Pallet<T>>::disable_index(validator_index)
 	}
 
 	fn validators() -> Vec<AccountId<T>> {

frame/staking/src/lib.rs

@@ -388,6 +388,10 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxNominatorRewardedPerValidator: Get<u32>;
 
+		/// The fraction of the validator set that is safe to be offending.
+		/// After the threshold is reached a new era will be forced.
+		type OffendingValidatorsThreshold: Get<Perbill>;
+
 		/// Something that can provide a sorted list of voters in a somewhat sorted way. The
 		/// original use case for this was designed with [`pallet_bags_list::Pallet`] in mind. If
 		/// the bags-list is not desired, [`impls::UseNominatorsMap`] is likely the desired option.
@@ -821,6 +825,19 @@ pub mod pallet {
 	#[pallet::getter(fn current_planned_session)]
 	pub type CurrentPlannedSession<T> = StorageValue<_, SessionIndex, ValueQuery>;
 
+	/// Indices of validators that have offended in the active era and whether they are currently
+	/// disabled.
+	///
+	/// This value should be a superset of disabled validators since not all offences lead to the
+	/// validator being disabled (if there was no slash). This is needed to track the percentage of
+	/// validators that have offended in the current era, ensuring a new era is forced if
+	/// `OffendingValidatorsThreshold` is reached. The vec is always kept sorted so that we can find
+	/// whether a given validator has previously offended using binary search. It gets cleared when
+	/// the era ends.
+	#[pallet::storage]
+	#[pallet::getter(fn offending_validators)]
+	pub type OffendingValidators<T: Config> = StorageValue<_, Vec<(u32, bool)>, ValueQuery>;
+
 	/// True if network has been upgraded to this version.
 	/// Storage version of the pallet.
 	///

frame/staking/src/mock.rs

@@ -90,31 +90,27 @@ impl OneSessionHandler<AccountId> for OtherSessionHandler {
 	{
 	}
 
-	fn on_new_session<'a, I: 'a>(_: bool, validators: I, _: I)
+	fn on_new_session<'a, I: 'a>(_: bool, _: I, _: I)
 	where
 		I: Iterator<Item = (&'a AccountId, Self::Key)>,
 		AccountId: 'a,
 	{
-		SESSION_VALIDATORS.with(|x| {
-			*x.borrow_mut() = (validators.map(|x| x.0.clone()).collect(), HashSet::new())
-		});
 	}
 
-	fn on_disabled(validator_index: usize) {
-		SESSION_VALIDATORS.with(|d| {
-			let mut d = d.borrow_mut();
-			let value = d.0[validator_index];
-			d.1.insert(value);
-		})
-	}
+	fn on_disabled(_: u32) {}
 }
 impl sp_runtime::BoundToRuntimeAppPublic for OtherSessionHandler {
 	type Public = UintAuthorityId;
 }
 
 pub fn is_disabled(controller: AccountId) -> bool {
 	let stash = Staking::ledger(&controller).unwrap().stash;
-	SESSION_VALIDATORS.with(|d| d.borrow().1.contains(&stash))
+	let validator_index = match Session::validators().iter().position(|v| *v == stash) {
+		Some(index) => index as u32,
+		None => return false,
+	};
+
+	Session::disabled_validators().contains(&validator_index)
 }
 
 parameter_types! {
@@ -155,12 +151,10 @@ sp_runtime::impl_opaque_keys! {
 	}
 }
 parameter_types! {
-	pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(25);
 	pub static Period: BlockNumber = 5;
 	pub static Offset: BlockNumber = 0;
 }
 impl pallet_session::Config for Test {
-	type DisabledValidatorsThreshold = DisabledValidatorsThreshold;
 	type Event = Event;
 	type Keys = SessionKeys;
 	type NextSessionRotation = pallet_session::PeriodicSessions<Period, Offset>;
@@ -237,12 +231,12 @@ parameter_types! {
 	pub const StakingPalletId: PalletId = PalletId(*b"da/staki");
 	pub const BondingDurationInEra: EraIndex = 3;
 	pub const MaxNominatorRewardedPerValidator: u32 = 64;
+	pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(75);
 	pub const Cap: Balance = CAP;
 	pub const TotalPower: Power = TOTAL_POWER;
 	pub static SessionsPerEra: SessionIndex = 3;
 	pub static BondingDurationInBlockNumber: BlockNumber = bonding_duration_in_blocks();
 	pub static SlashDeferDuration: EraIndex = 0;
-	pub static SessionValidators: (Vec<AccountId>, HashSet<AccountId>) = Default::default();
 	pub static RingRewardRemainderUnbalanced: Balance = 0;
 }
 impl Config for Test {
@@ -257,6 +251,7 @@ impl Config for Test {
 	type KtonSlash = ();
 	type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator;
 	type NextNewSession = Session;
+	type OffendingValidatorsThreshold = OffendingValidatorsThreshold;
 	type PalletId = StakingPalletId;
 	type RingCurrency = Ring;
 	type RingReward = ();
@@ -537,11 +532,6 @@ impl ExtBuilder {
 		.assimilate_storage(&mut storage);
 		let mut ext = sp_io::TestExternalities::from(storage);
 
-		ext.execute_with(|| {
-			let validators = Session::validators();
-			SESSION_VALIDATORS.with(|x| *x.borrow_mut() = (validators.clone(), HashSet::new()));
-		});
-
 		if self.initialize_first_session {
 			// We consider all test to start after timestamp is initialized This must be ensured by
 			// having `timestamp::on_initialize` called before `staking::on_initialize`. Also, if

frame/staking/src/slashing.rs

@@ -56,7 +56,7 @@ use scale_info::TypeInfo;
 // --- paritytech ---
 use frame_support::{
 	ensure,
-	traits::{Currency, Imbalance, OnUnbalanced, UnixTime},
+	traits::{Currency, Get, Imbalance, OnUnbalanced, UnixTime},
 };
 use sp_runtime::{
 	traits::{Saturating, Zero},
@@ -364,15 +364,13 @@ pub fn compute_slash<T: Config>(
 			// not continue in the next election. also end the slashing span.
 			spans.end_span(now);
 			<Pallet<T>>::chill_stash(stash);
-
-			// make sure to disable validator till the end of this session
-			if T::SessionInterface::disable_validator(stash).unwrap_or(false) {
-				// force a new era, to select a new validator set
-				<Pallet<T>>::ensure_new_era()
-			}
 		}
 	}
 
+	// add the validator to the offenders list and make sure it is disabled for
+	// the duration of the era
+	add_offending_validator::<T>(params.stash, true);
+
 	let mut nominators_slashed = vec![];
 	reward_payout += slash_nominators::<T>(params, prior_slash_p, &mut nominators_slashed);
 
@@ -402,13 +400,53 @@ fn kick_out_if_recent<T: Config>(params: SlashParams<T>) {
 	if spans.era_span(params.slash_era).map(|s| s.index) == Some(spans.span_index()) {
 		spans.end_span(params.now);
 		<Pallet<T>>::chill_stash(params.stash);
+	}
+
+	// add the validator to the offenders list but since there's no slash being
+	// applied there's no need to disable the validator
+	add_offending_validator::<T>(params.stash, false);
+}
+
+/// Add the given validator to the offenders list and optionally disable it.
+/// If after adding the validator `OffendingValidatorsThreshold` is reached
+/// a new era will be forced.
+fn add_offending_validator<T: Config>(stash: &T::AccountId, disable: bool) {
+	<Pallet<T> as Store>::OffendingValidators::mutate(|offending| {
+		let validators = T::SessionInterface::validators();
+		let validator_index = match validators.iter().position(|i| i == stash) {
+			Some(index) => index,
+			None => return,
+		};
 
-		// make sure to disable validator till the end of this session
-		if T::SessionInterface::disable_validator(params.stash).unwrap_or(false) {
-			// force a new era, to select a new validator set
-			<Pallet<T>>::ensure_new_era()
+		let validator_index_u32 = validator_index as u32;
+
+		match offending.binary_search_by_key(&validator_index_u32, |(index, _)| *index) {
+			// this is a new offending validator
+			Err(index) => {
+				offending.insert(index, (validator_index_u32, disable));
+
+				let offending_threshold =
+					T::OffendingValidatorsThreshold::get() * validators.len() as u32;
+
+				if offending.len() >= offending_threshold as usize {
+					// force a new era, to select a new validator set
+					<Pallet<T>>::ensure_new_era()
+				}
+
+				if disable {
+					T::SessionInterface::disable_validator(validator_index_u32);
+				}
+			},
+			Ok(index) => {
+				if disable && !offending[index].1 {
+					// the validator had previously offended without being disabled,
+					// let's make sure we disable it now
+					offending[index].1 = true;
+					T::SessionInterface::disable_validator(validator_index_u32);
+				}
+			},
 		}
-	}
+	});
 }
 
 /// Slash nominators. Accepts general parameters and the prior slash percentage of the validator.