aura, babe: don't allow disabled validators to build blocks (#9414)

* frame-support: add trait for checking disabled validators

* pallet-session: implement DisabledValidators trait

* pallet-babe: check for disabled validators

* pallet-babe: add test for disabled validators

* pallet-aura: check for disabled validators

* pallet-aura: add test for disabled validators

* runtime: fix missing DisableValidator

* test-runtime: add missing DisabledValidators

* frame-support: clean up doc

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

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 {

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;
 

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::<T>::put(new_slot);
 
+				if let Some(n_authorities) = <Authorities<T>>::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)

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<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
@@ -83,8 +87,32 @@ impl pallet_timestamp::Config for Test {
 	type WeightInfo = ();
 }
 
+thread_local! {
+	static DISABLED_VALIDATORS: RefCell<Vec<AuthorityIndex>> = 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<u64>) -> sp_io::TestExternalities {

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);
+	});
+}

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<T: Config> Pallet<T> {
 
 			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.

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;
 

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()

substrate/frame/session/src/lib.rs

@@ -892,3 +892,9 @@ impl<T: Config> EstimateNextNewSession<T::BlockNumber> for Module<T> {
 		T::NextSessionRotation::estimate_next_session_rotation(now)
 	}
 }
+
+impl<T: Config> frame_support::traits::DisabledValidators for Module<T> {
+	fn is_disabled(index: u32) -> bool {
+		<Module<T>>::disabled_validators().binary_search(&index).is_ok()
+	}
+}

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;

substrate/frame/support/src/traits/validation.rs

@@ -244,3 +244,16 @@ pub trait ValidatorRegistration<ValidatorId> {
 	/// 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
+	}
+}

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 = ();