diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index 0c1a229ab2..f6955d2bd0 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -211,6 +211,9 @@ impl_opaque_keys! { // `SessionKeys`. // TODO: Introduce some structure to tie these together to make it a bit less of a footgun. This // should be easy, since OneSessionHandler trait provides the `Key` as an associated type. #2858 +parameter_types! { + pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(17); +} impl session::Trait for Runtime { type OnSessionEnding = Staking; @@ -221,6 +224,7 @@ impl session::Trait for Runtime { type ValidatorId = AccountId; type ValidatorIdOf = staking::StashOf; type SelectInitialValidators = Staking; + type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } impl session::historical::Trait for Runtime { diff --git a/substrate/srml/authority-discovery/src/lib.rs b/substrate/srml/authority-discovery/src/lib.rs index 372a48cc7f..8032b46be4 100644 --- a/substrate/srml/authority-discovery/src/lib.rs +++ b/substrate/srml/authority-discovery/src/lib.rs @@ -155,6 +155,10 @@ mod tests { } } + parameter_types! { + pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); + } + impl session::Trait for Test { type OnSessionEnding = TestOnSessionEnding; type Keys = UintAuthorityId; @@ -164,6 +168,7 @@ mod tests { type ValidatorId = AuthorityId; type ValidatorIdOf = ConvertInto; type SelectInitialValidators = (); + type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } impl session::historical::Trait for Test { diff --git a/substrate/srml/im-online/src/mock.rs b/substrate/srml/im-online/src/mock.rs index ba1a7a7d0a..a7b669ddb8 100644 --- a/substrate/srml/im-online/src/mock.rs +++ b/substrate/srml/im-online/src/mock.rs @@ -125,6 +125,10 @@ parameter_types! { pub const Offset: u64 = 0; } +parameter_types! { + pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); +} + impl session::Trait for Runtime { type ShouldEndSession = session::PeriodicSessions; type OnSessionEnding = session::historical::NoteHistoricalRoot; @@ -134,6 +138,7 @@ impl session::Trait for Runtime { type Keys = UintAuthorityId; type Event = (); type SelectInitialValidators = (); + type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } impl session::historical::Trait for Runtime { diff --git a/substrate/srml/session/src/lib.rs b/substrate/srml/session/src/lib.rs index 8edb80e8cb..1494f88413 100644 --- a/substrate/srml/session/src/lib.rs +++ b/substrate/srml/session/src/lib.rs @@ -121,7 +121,7 @@ use rstd::{prelude::*, marker::PhantomData, ops::{Sub, Rem}}; use codec::Decode; -use sr_primitives::{KeyTypeId, RuntimeAppPublic}; +use sr_primitives::{KeyTypeId, Perbill, RuntimeAppPublic}; use sr_primitives::weights::SimpleDispatchInfo; use sr_primitives::traits::{Convert, Zero, Member, OpaqueKeys}; use sr_staking_primitives::SessionIndex; @@ -334,6 +334,12 @@ pub trait Trait: system::Trait { /// The keys. type Keys: OpaqueKeys + Member + Parameter + Default; + /// The fraction of validators set that is safe to be disabled. + /// + /// After the threshold is reached `disabled` method starts to return true, + /// which in combination with `srml_staking` forces a new era. + type DisabledValidatorsThreshold: Get; + /// Select initial validators. type SelectInitialValidators: SelectInitialValidators; } @@ -356,6 +362,11 @@ decl_storage! { /// will be used to determine the validator's session keys. QueuedKeys get(queued_keys): Vec<(T::ValidatorId, T::Keys)>; + /// Indices of disabled validators. + /// + /// The set is cleared when `on_session_ending` returns a new set of identities. + DisabledValidators get(disabled_validators): Vec; + /// The next session keys for a validator. /// /// The first key is always `DEDUP_KEY_PREFIX` to have all the data in the same branch of @@ -475,6 +486,11 @@ impl Module { .collect::>(); >::put(&validators); + if changed { + // reset disabled validators + DisabledValidators::take(); + } + let applied_at = session_index + 2; // Get next validator set. @@ -539,13 +555,36 @@ impl Module { } /// Disable the validator of index `i`. - pub fn disable_index(i: usize) { - T::SessionHandler::on_disabled(i); + /// + /// Returns `true` if this causes a `DisabledValidatorsThreshold` of validators + /// to be already disabled. + pub fn disable_index(i: usize) -> bool { + let (fire_event, threshold_reached) = DisabledValidators::mutate(|disabled| { + let i = i as u32; + if let Err(index) = disabled.binary_search(&i) { + let count = >::decode_len().unwrap_or(0) as u32; + let threshold = T::DisabledValidatorsThreshold::get() * count; + disabled.insert(index, i); + (true, disabled.len() as u32 > threshold) + } else { + (false, false) + } + }); + + if fire_event { + T::SessionHandler::on_disabled(i); + } + + threshold_reached } - /// Disable the validator identified by `c`. (If using with the staking module, this would be - /// their *stash* account.) - pub fn disable(c: &T::ValidatorId) -> rstd::result::Result<(), ()> { + /// Disable the validator identified by `c`. (If using with the staking module, + /// this would be their *stash* account.) + /// + /// Returns `Ok(true)` if more than `DisabledValidatorsThreshold` validators in current + /// session is already disabled. + /// If used with the staking module it allows to force a new era in such case. + pub fn disable(c: &T::ValidatorId) -> rstd::result::Result { Self::validators().iter().position(|i| i == c).map(Self::disable_index).ok_or(()) } @@ -924,4 +963,22 @@ mod tests { ); }); } + + #[test] + fn return_true_if_more_than_third_is_disabled() { + with_externalities(&mut new_test_ext(), || { + set_next_validators(vec![1, 2, 3, 4, 5, 6, 7]); + force_new_session(); + initialize_block(1); + // apply the new validator set + force_new_session(); + initialize_block(2); + + assert_eq!(Session::disable_index(0), false); + assert_eq!(Session::disable_index(1), false); + assert_eq!(Session::disable_index(2), true); + assert_eq!(Session::disable_index(3), true); + }); + + } } diff --git a/substrate/srml/session/src/mock.rs b/substrate/srml/session/src/mock.rs index fe9a4ee936..736d3fa4da 100644 --- a/substrate/srml/session/src/mock.rs +++ b/substrate/srml/session/src/mock.rs @@ -183,6 +183,10 @@ impl timestamp::Trait for Test { type MinimumPeriod = MinimumPeriod; } +parameter_types! { + pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); +} + impl Trait for Test { type ShouldEndSession = TestShouldEndSession; #[cfg(feature = "historical")] @@ -195,6 +199,7 @@ impl Trait for Test { type Keys = MockSessionKeys; type Event = (); type SelectInitialValidators = (); + type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } #[cfg(feature = "historical")] diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index 831629f5f2..3f88b67875 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -454,7 +454,11 @@ type MomentOf= <::Time as Time>::Moment; /// This is needed because `Staking` sets the `ValidatorIdOf` of the `session::Trait` pub trait SessionInterface: system::Trait { /// Disable a given validator by stash ID. - fn disable_validator(validator: &AccountId) -> Result<(), ()>; + /// + /// 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; /// Get the validators from session. fn validators() -> Vec; /// Prune historical session tries up to but not including the given index. @@ -472,7 +476,7 @@ impl SessionInterface<::AccountId> for T where T::SelectInitialValidators: session::SelectInitialValidators<::AccountId>, T::ValidatorIdOf: Convert<::AccountId, Option<::AccountId>> { - fn disable_validator(validator: &::AccountId) -> Result<(), ()> { + fn disable_validator(validator: &::AccountId) -> Result { >::disable(validator) } @@ -1531,10 +1535,11 @@ impl OnOffenceHandler; @@ -160,6 +161,7 @@ impl session::Trait for Test { type ValidatorId = AccountId; type ValidatorIdOf = crate::StashOf; type SelectInitialValidators = Staking; + type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } impl session::historical::Trait for Test {