diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 908c5ea455..afd31b8f7b 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -197,6 +197,7 @@ impl pallet_randomness_collective_flip::Config for Runtime {} impl pallet_aura::Config for Runtime { type AuthorityId = AuraId; + type DisabledValidators = (); } impl pallet_grandpa::Config for Runtime { diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 37b4b24fa6..0a8d258495 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -346,6 +346,7 @@ impl pallet_babe::Config for Runtime { type EpochDuration = EpochDuration; type ExpectedBlockTime = ExpectedBlockTime; type EpochChangeTrigger = pallet_babe::ExternalTrigger; + type DisabledValidators = Session; type KeyOwnerProofSystem = Historical; diff --git a/substrate/frame/aura/src/lib.rs b/substrate/frame/aura/src/lib.rs index 41fb69dfb5..ebb869194a 100644 --- a/substrate/frame/aura/src/lib.rs +++ b/substrate/frame/aura/src/lib.rs @@ -39,7 +39,7 @@ use codec::{Decode, Encode}; use frame_support::{ - traits::{FindAuthor, Get, OnTimestampSet, OneSessionHandler}, + traits::{DisabledValidators, FindAuthor, Get, OnTimestampSet, OneSessionHandler}, ConsensusEngineId, Parameter, }; use sp_consensus_aura::{AuthorityIndex, ConsensusLog, Slot, AURA_ENGINE_ID}; @@ -70,6 +70,11 @@ pub mod pallet { + RuntimeAppPublic + Default + MaybeSerializeDeserialize; + + /// A way to check whether a given validator is disabled and should not be authoring blocks. + /// Blocks authored by a disabled validator will lead to a panic as part of this module's + /// initialization. + type DisabledValidators: DisabledValidators; } #[pallet::pallet] @@ -84,6 +89,16 @@ pub mod pallet { assert!(current_slot < new_slot, "Slot must increase"); CurrentSlot::::put(new_slot); + if let Some(n_authorities) = >::decode_len() { + let authority_index = *new_slot % n_authorities as u64; + if T::DisabledValidators::is_disabled(authority_index as u32) { + panic!( + "Validator with index {:?} is disabled and should not be attempting to author blocks.", + authority_index, + ); + } + } + // TODO [#3398] Generate offence report for all authorities that skipped their slots. T::DbWeight::get().reads_writes(2, 1) diff --git a/substrate/frame/aura/src/mock.rs b/substrate/frame/aura/src/mock.rs index 72d457165d..8d604e527c 100644 --- a/substrate/frame/aura/src/mock.rs +++ b/substrate/frame/aura/src/mock.rs @@ -20,13 +20,17 @@ #![cfg(test)] use crate as pallet_aura; -use frame_support::{parameter_types, traits::GenesisBuild}; -use sp_consensus_aura::ed25519::AuthorityId; +use frame_support::{ + parameter_types, + traits::{DisabledValidators, GenesisBuild}, +}; +use sp_consensus_aura::{ed25519::AuthorityId, AuthorityIndex}; use sp_core::H256; use sp_runtime::{ testing::{Header, UintAuthorityId}, traits::IdentityLookup, }; +use sp_std::cell::RefCell; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -83,8 +87,32 @@ impl pallet_timestamp::Config for Test { type WeightInfo = (); } +thread_local! { + static DISABLED_VALIDATORS: RefCell> = RefCell::new(Default::default()); +} + +pub struct MockDisabledValidators; + +impl MockDisabledValidators { + pub fn disable_validator(index: AuthorityIndex) { + DISABLED_VALIDATORS.with(|v| { + let mut disabled = v.borrow_mut(); + if let Err(i) = disabled.binary_search(&index) { + disabled.insert(i, index); + } + }) + } +} + +impl DisabledValidators for MockDisabledValidators { + fn is_disabled(index: AuthorityIndex) -> bool { + DISABLED_VALIDATORS.with(|v| v.borrow().binary_search(&index).is_ok()) + } +} + impl pallet_aura::Config for Test { type AuthorityId = AuthorityId; + type DisabledValidators = MockDisabledValidators; } pub fn new_test_ext(authorities: Vec) -> sp_io::TestExternalities { diff --git a/substrate/frame/aura/src/tests.rs b/substrate/frame/aura/src/tests.rs index 14e79ab547..596858aac7 100644 --- a/substrate/frame/aura/src/tests.rs +++ b/substrate/frame/aura/src/tests.rs @@ -19,7 +19,12 @@ #![cfg(test)] -use crate::mock::{new_test_ext, Aura}; +use crate::mock::{new_test_ext, Aura, MockDisabledValidators, System}; +use codec::Encode; +use frame_support::traits::OnInitialize; +use frame_system::InitKind; +use sp_consensus_aura::{Slot, AURA_ENGINE_ID}; +use sp_runtime::{Digest, DigestItem}; #[test] fn initial_values() { @@ -28,3 +33,24 @@ fn initial_values() { assert_eq!(Aura::authorities().len(), 4); }); } + +#[test] +#[should_panic( + expected = "Validator with index 1 is disabled and should not be attempting to author blocks." +)] +fn disabled_validators_cannot_author_blocks() { + new_test_ext(vec![0, 1, 2, 3]).execute_with(|| { + // slot 1 should be authored by validator at index 1 + let slot = Slot::from(1); + let pre_digest = + Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())] }; + + System::initialize(&42, &System::parent_hash(), &pre_digest, InitKind::Full); + + // let's disable the validator + MockDisabledValidators::disable_validator(1); + + // and we should not be able to initialize the block + Aura::on_initialize(42); + }); +} diff --git a/substrate/frame/babe/src/lib.rs b/substrate/frame/babe/src/lib.rs index 949f55720b..e9c5d2e8d9 100644 --- a/substrate/frame/babe/src/lib.rs +++ b/substrate/frame/babe/src/lib.rs @@ -24,7 +24,9 @@ use codec::{Decode, Encode}; use frame_support::{ dispatch::DispatchResultWithPostInfo, - traits::{FindAuthor, Get, KeyOwnerProofSystem, OnTimestampSet, OneSessionHandler}, + traits::{ + DisabledValidators, FindAuthor, Get, KeyOwnerProofSystem, OnTimestampSet, OneSessionHandler, + }, weights::{Pays, Weight}, }; use sp_application_crypto::Public; @@ -137,6 +139,11 @@ pub mod pallet { /// when no other module is responsible for changing authority set. type EpochChangeTrigger: EpochChangeTrigger; + /// A way to check whether a given validator is disabled and should not be authoring blocks. + /// Blocks authored by a disabled validator will lead to a panic as part of this module's + /// initialization. + type DisabledValidators: DisabledValidators; + /// The proof of key ownership, used for validating equivocation reports. /// The proof must include the session index and validator count of the /// session at which the equivocation occurred. @@ -678,6 +685,13 @@ impl Pallet { let authority_index = digest.authority_index(); + if T::DisabledValidators::is_disabled(authority_index) { + panic!( + "Validator with index {:?} is disabled and should not be attempting to author blocks.", + authority_index, + ); + } + // Extract out the VRF output if we have it digest.vrf_output().and_then(|vrf_output| { // Reconstruct the bytes of VRFInOut using the authority id. diff --git a/substrate/frame/babe/src/mock.rs b/substrate/frame/babe/src/mock.rs index 795d51e587..a034360c3f 100644 --- a/substrate/frame/babe/src/mock.rs +++ b/substrate/frame/babe/src/mock.rs @@ -238,6 +238,7 @@ impl Config for Test { type EpochDuration = EpochDuration; type ExpectedBlockTime = ExpectedBlockTime; type EpochChangeTrigger = crate::ExternalTrigger; + type DisabledValidators = Session; type KeyOwnerProofSystem = Historical; diff --git a/substrate/frame/babe/src/tests.rs b/substrate/frame/babe/src/tests.rs index 5e72e14877..1cf4b0aac1 100644 --- a/substrate/frame/babe/src/tests.rs +++ b/substrate/frame/babe/src/tests.rs @@ -373,6 +373,31 @@ fn tracks_block_numbers_when_current_and_previous_epoch_started() { }); } +#[test] +#[should_panic( + expected = "Validator with index 0 is disabled and should not be attempting to author blocks." +)] +fn disabled_validators_cannot_author_blocks() { + new_test_ext(4).execute_with(|| { + start_era(1); + + // let's disable the validator at index 1 + Session::disable_index(1); + + // the mocking infrastructure always authors all blocks using authority index 0, + // so we should still be able to author blocks + start_era(2); + + assert_eq!(Staking::current_era().unwrap(), 2); + + // let's disable the validator at index 0 + Session::disable_index(0); + + // this should now panic as the validator authoring blocks is disabled + start_era(3); + }); +} + #[test] fn report_equivocation_current_session_works() { let (pairs, mut ext) = new_test_ext_with_pairs(3); @@ -394,8 +419,8 @@ fn report_equivocation_current_session_works() { ); } - // we will use the validator at index 0 as the offending authority - let offending_validator_index = 0; + // we will use the validator at index 1 as the offending authority + let offending_validator_index = 1; let offending_validator_id = Session::validators()[offending_validator_index]; let offending_authority_pair = pairs .into_iter() @@ -456,7 +481,7 @@ fn report_equivocation_old_session_works() { let authorities = Babe::authorities(); // we will use the validator at index 0 as the offending authority - let offending_validator_index = 0; + let offending_validator_index = 1; let offending_validator_id = Session::validators()[offending_validator_index]; let offending_authority_pair = pairs .into_iter() diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index cdeceb1ef5..5f6c05e650 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -892,3 +892,9 @@ impl EstimateNextNewSession for Module { T::NextSessionRotation::estimate_next_session_rotation(now) } } + +impl frame_support::traits::DisabledValidators for Module { + fn is_disabled(index: u32) -> bool { + >::disabled_validators().binary_search(&index).is_ok() + } +} diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 024e7e6c69..fbb21de7eb 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -38,9 +38,9 @@ pub use members::{ mod validation; pub use validation::{ - EstimateNextNewSession, EstimateNextSessionRotation, FindAuthor, KeyOwnerProofSystem, Lateness, - OneSessionHandler, ValidatorRegistration, ValidatorSet, ValidatorSetWithIdentification, - VerifySeal, + DisabledValidators, EstimateNextNewSession, EstimateNextSessionRotation, FindAuthor, + KeyOwnerProofSystem, Lateness, OneSessionHandler, ValidatorRegistration, ValidatorSet, + ValidatorSetWithIdentification, VerifySeal, }; mod filter; diff --git a/substrate/frame/support/src/traits/validation.rs b/substrate/frame/support/src/traits/validation.rs index 5a68f289df..a473e332a8 100644 --- a/substrate/frame/support/src/traits/validation.rs +++ b/substrate/frame/support/src/traits/validation.rs @@ -244,3 +244,16 @@ pub trait ValidatorRegistration { /// module fn is_registered(id: &ValidatorId) -> bool; } + +/// Trait used to check whether a given validator is currently disabled and should not be +/// participating in consensus (e.g. because they equivocated). +pub trait DisabledValidators { + /// Returns true if the given validator is disabled. + fn is_disabled(index: u32) -> bool; +} + +impl DisabledValidators for () { + fn is_disabled(_index: u32) -> bool { + false + } +} diff --git a/substrate/test-utils/runtime/src/lib.rs b/substrate/test-utils/runtime/src/lib.rs index a148ce5cb7..e7f25ad336 100644 --- a/substrate/test-utils/runtime/src/lib.rs +++ b/substrate/test-utils/runtime/src/lib.rs @@ -582,6 +582,7 @@ impl pallet_babe::Config for Runtime { // are manually adding the digests. normally in this situation you'd use // pallet_babe::SameAuthoritiesForever. type EpochChangeTrigger = pallet_babe::ExternalTrigger; + type DisabledValidators = (); type KeyOwnerProofSystem = ();