Filter votes from disabled validators in BackedCandidates in process_inherent_data

polkadot/runtime/common/src/assigned_slots/mod.rs

@@ -743,7 +743,9 @@ mod tests {
 		type OnNewHead = ();
 	}
 
-	impl parachains_shared::Config for Test {}
+	impl parachains_shared::Config for Test {
+		type DisabledValidators = ();
+	}
 
 	parameter_types! {
 		pub const LeasePeriod: BlockNumber = 3;

polkadot/runtime/common/src/integration_tests.rs

@@ -191,7 +191,9 @@ impl configuration::Config for Test {
 	type WeightInfo = configuration::TestWeightInfo;
 }
 
-impl shared::Config for Test {}
+impl shared::Config for Test {
+	type DisabledValidators = ();
+}
 
 impl origin::Config for Test {}
 

polkadot/runtime/common/src/paras_registrar/mod.rs

@@ -798,7 +798,9 @@ mod tests {
 		type MaxFreezes = ConstU32<1>;
 	}
 
-	impl shared::Config for Test {}
+	impl shared::Config for Test {
+		type DisabledValidators = ();
+	}
 
 	impl origin::Config for Test {}
 

polkadot/runtime/parachains/src/mock.rs

@@ -183,7 +183,22 @@ impl crate::configuration::Config for Test {
 	type WeightInfo = crate::configuration::TestWeightInfo;
 }
 
-impl crate::shared::Config for Test {}
+pub struct MockDisabledValidators {}
+impl frame_support::traits::DisabledValidators for MockDisabledValidators {
+	/// Returns true if the given validator is disabled.
+	fn is_disabled(index: u32) -> bool {
+		disabled_validators().iter().any(|v| *v == index)
+	}
+
+	/// Returns a hardcoded list (`DISABLED_VALIDATORS`) of disabled validators
+	fn disabled_validators() -> Vec<u32> {
+		disabled_validators()
+	}
+}
+
+impl crate::shared::Config for Test {
+	type DisabledValidators = MockDisabledValidators;
+}
 
 impl origin::Config for Test {}
 
@@ -431,6 +446,8 @@ thread_local! {
 
 	pub static AVAILABILITY_REWARDS: RefCell<HashMap<ValidatorIndex, usize>>
 		= RefCell::new(HashMap::new());
+
+	pub static DISABLED_VALIDATORS: RefCell<Vec<u32>> = RefCell::new(vec![]);
 }
 
 pub fn backing_rewards() -> HashMap<ValidatorIndex, usize> {
@@ -441,6 +458,10 @@ pub fn availability_rewards() -> HashMap<ValidatorIndex, usize> {
 	AVAILABILITY_REWARDS.with(|r| r.borrow().clone())
 }
 
+pub fn disabled_validators() -> Vec<u32> {
+	DISABLED_VALIDATORS.with(|r| r.borrow().clone())
+}
+
 parameter_types! {
 	pub static Processed: Vec<(ParaId, UpwardMessage)> = vec![];
 }
@@ -580,3 +601,7 @@ pub(crate) fn deregister_parachain(id: ParaId) {
 pub(crate) fn try_deregister_parachain(id: ParaId) -> crate::DispatchResult {
 	frame_support::storage::transactional::with_storage_layer(|| Paras::schedule_para_cleanup(id))
 }
+
+pub(crate) fn set_disabled_validators(disabled: Vec<u32>) {
+	DISABLED_VALIDATORS.with(|d| *d.borrow_mut() = disabled)
+}

polkadot/runtime/parachains/src/paras_inherent/mod.rs

@@ -30,7 +30,8 @@ use crate::{
 	metrics::METRICS,
 	paras,
 	scheduler::{self, FreedReason},
-	shared, ParaId,
+	shared::{self, AllowedRelayParentsTracker},
+	ParaId,
 };
 use bitvec::prelude::BitVec;
 use frame_support::{
@@ -42,8 +43,8 @@ use frame_support::{
 use frame_system::pallet_prelude::*;
 use pallet_babe::{self, ParentBlockRandomness};
 use primitives::{
-	BackedCandidate, CandidateHash, CandidateReceipt, CheckedDisputeStatementSet,
-	CheckedMultiDisputeStatementSet, CoreIndex, DisputeStatementSet,
+	effective_minimum_backing_votes, BackedCandidate, CandidateHash, CandidateReceipt,
+	CheckedDisputeStatementSet, CheckedMultiDisputeStatementSet, CoreIndex, DisputeStatementSet,
 	InherentData as ParachainsInherentData, MultiDisputeStatementSet, ScrapedOnChainVotes,
 	SessionIndex, SignedAvailabilityBitfields, SigningContext, UncheckedSignedAvailabilityBitfield,
 	UncheckedSignedAvailabilityBitfields, ValidatorId, ValidatorIndex, ValidityAttestation,
@@ -378,6 +379,7 @@ impl<T: Config> Pallet<T> {
 		let bitfields_weight = signed_bitfields_weight::<T>(&bitfields);
 		let disputes_weight = multi_dispute_statement_sets_weight::<T>(&disputes);
 
+		// Weight before filtering/sanitization
 		let all_weight_before = candidates_weight + bitfields_weight + disputes_weight;
 
 		METRICS.on_before_filter(all_weight_before.ref_time());
@@ -587,17 +589,19 @@ impl<T: Config> Pallet<T> {
 
 		METRICS.on_candidates_processed_total(backed_candidates.len() as u64);
 
-		let backed_candidates = sanitize_backed_candidates::<T, _>(
-			backed_candidates,
-			|candidate_idx: usize,
-			 backed_candidate: &BackedCandidate<<T as frame_system::Config>::Hash>|
-			 -> bool {
-				let para_id = backed_candidate.descriptor().para_id;
-				let prev_context = <paras::Pallet<T>>::para_most_recent_context(para_id);
-				let check_ctx = CandidateCheckContext::<T>::new(prev_context);
-
-				// never include a concluded-invalid candidate
-				current_concluded_invalid_disputes.contains(&backed_candidate.hash()) ||
+		let SanitizedBackedCandidates { backed_candidates, votes_from_disabled_were_dropped } =
+			sanitize_backed_candidates::<T, _>(
+				backed_candidates,
+				&allowed_relay_parents,
+				|candidate_idx: usize,
+				 backed_candidate: &BackedCandidate<<T as frame_system::Config>::Hash>|
+				 -> bool {
+					let para_id = backed_candidate.descriptor().para_id;
+					let prev_context = <paras::Pallet<T>>::para_most_recent_context(para_id);
+					let check_ctx = CandidateCheckContext::<T>::new(prev_context);
+
+					// never include a concluded-invalid candidate
+					current_concluded_invalid_disputes.contains(&backed_candidate.hash()) ||
 					// Instead of checking the candidates with code upgrades twice
 					// move the checking up here and skip it in the training wheels fallback.
 					// That way we avoid possible duplicate checks while assuring all
@@ -607,12 +611,19 @@ impl<T: Config> Pallet<T> {
 					check_ctx
 						.verify_backed_candidate(&allowed_relay_parents, candidate_idx, backed_candidate)
 						.is_err()
-			},
-			&scheduled,
-		);
+				},
+				&scheduled,
+			);
 
 		METRICS.on_candidates_sanitized(backed_candidates.len() as u64);
 
+		// In `Enter` context (invoked during execution) there should be no backing votes from
+		// disabled validators because they should have been filtered out during inherent data
+		// preparation (`ProvideInherent` context). Abort in such cases.
+		if context == ProcessInherentDataContext::Enter {
+			ensure!(!votes_from_disabled_were_dropped, Error::<T>::InherentOverweight);
+		}
+
 		// Process backed candidates according to scheduled cores.
 		let inclusion::ProcessedCandidates::<<HeaderFor<T> as HeaderT>::Hash> {
 			core_indices: occupied,
@@ -900,7 +911,19 @@ pub(crate) fn sanitize_bitfields<T: crate::inclusion::Config>(
 	bitfields
 }
 
-/// Filter out any candidates that have a concluded invalid dispute.
+// Result from `sanitize_backed_candidates`
+#[derive(Debug, PartialEq)]
+struct SanitizedBackedCandidates<Hash> {
+	// Sanitized backed candidates. The `Vec` is sorted according to the occupied core index.
+	backed_candidates: Vec<BackedCandidate<Hash>>,
+	// Set to true if any votes from disabled validators were dropped from the input.
+	votes_from_disabled_were_dropped: bool,
+}
+
+/// Filter out:
+/// 1. any candidates that have a concluded invalid dispute
+/// 2. all backing votes from disabled validators
+/// 3. any candidates that end up with less than `effective_minimum_backing_votes` backing votes
 ///
 /// `scheduled` follows the same naming scheme as provided in the
 /// guide: Currently `free` but might become `occupied`.
@@ -910,15 +933,17 @@ pub(crate) fn sanitize_bitfields<T: crate::inclusion::Config>(
 /// `candidate_has_concluded_invalid_dispute` must return `true` if the candidate
 /// is disputed, false otherwise. The passed `usize` is the candidate index.
 ///
-/// The returned `Vec` is sorted according to the occupied core index.
+/// Returns struct `SanitizedBackedCandidates` where `backed_candidates` are sorted according to the
+/// occupied core index.
 fn sanitize_backed_candidates<
 	T: crate::inclusion::Config,
 	F: FnMut(usize, &BackedCandidate<T::Hash>) -> bool,
 >(
 	mut backed_candidates: Vec<BackedCandidate<T::Hash>>,
+	allowed_relay_parents: &AllowedRelayParentsTracker<T::Hash, BlockNumberFor<T>>,
 	mut candidate_has_concluded_invalid_dispute_or_is_invalid: F,
 	scheduled: &BTreeMap<ParaId, CoreIndex>,
-) -> Vec<BackedCandidate<T::Hash>> {
+) -> SanitizedBackedCandidates<T::Hash> {
 	// Remove any candidates that were concluded invalid.
 	// This does not assume sorting.
 	backed_candidates.indexed_retain(move |candidate_idx, backed_candidate| {
@@ -936,6 +961,13 @@ fn sanitize_backed_candidates<
 		scheduled.get(&desc.para_id).is_some()
 	});
 
+	// Filter out backing statements from disabled validators
+	let dropped_disabled = filter_backed_statements_from_disabled::<T>(
+		&mut backed_candidates,
+		&allowed_relay_parents,
+		scheduled,
+	);
+
 	// Sort the `Vec` last, once there is a guarantee that these
 	// `BackedCandidates` references the expected relay chain parent,
 	// but more importantly are scheduled for a free core.
@@ -946,7 +978,10 @@ fn sanitize_backed_candidates<
 		scheduled[&x.descriptor().para_id].cmp(&scheduled[&y.descriptor().para_id])
 	});
 
-	backed_candidates
+	SanitizedBackedCandidates {
+		backed_candidates,
+		votes_from_disabled_were_dropped: dropped_disabled,
+	}
 }
 
 /// Derive entropy from babe provided per block randomness.
@@ -1029,3 +1064,106 @@ fn limit_and_sanitize_disputes<
 		(checked, checked_disputes_weight)
 	}
 }
+
+// Filters statements from disabled validators in `BackedCandidate`. Returns `true` if at least one
+// statement is removed and `false` otherwise.
+fn filter_backed_statements_from_disabled<T: shared::Config + scheduler::Config>(
+	backed_candidates: &mut Vec<BackedCandidate<<T as frame_system::Config>::Hash>>,
+	allowed_relay_parents: &AllowedRelayParentsTracker<T::Hash, BlockNumberFor<T>>,
+	scheduled: &BTreeMap<ParaId, CoreIndex>,
+) -> bool {
+	let disabled_validators =
+		BTreeSet::<_>::from_iter(shared::Pallet::<T>::disabled_validators().into_iter());
+
+	if disabled_validators.is_empty() {
+		// No disabled validators - nothing to do
+		return false
+	}
+
+	let backed_len_before = backed_candidates.len();
+
+	// Flag which will be returned. Set to `true` if at least one vote is filtered.
+	let mut filtered = false;
+
+	// Process all backed candidates. `validator_indices` in `BackedCandidates` are indices within
+	// the validator group assigned to the parachain. To obtain this group we need:
+	// 1. Core index assigned to the parachain which has produced the candidate
+	// 2. The relay chain block number of the candidate
+	backed_candidates.retain_mut(|bc| {
+		// Get `core_idx` assigned to the `para_id` of the candidate
+		let core_idx = match scheduled.get(&bc.descriptor().para_id) {
+			Some(core_idx) => *core_idx,
+			None => {
+				log::debug!(target: LOG_TARGET, "Can't get core idx of a backed candidate for para id {:?}. Dropping the candidate.", bc.descriptor().para_id);
+				return false
+			}
+		};
+
+		// Get relay parent block number of the candidate. We need this to get the group index assigned to this core at this block number
+		let relay_parent_block_number = match allowed_relay_parents
+			.acquire_info(bc.descriptor().relay_parent, None) {
+				Some((_, block_num)) => block_num,
+				None => {
+					log::debug!(target: LOG_TARGET, "Relay parent {:?} for candidate is not in the allowed relay parents. Dropping the candidate.", bc.descriptor().relay_parent);
+					return false
+				}
+			};
+
+		// Get the group index for the core
+		let group_idx = match <scheduler::Pallet<T>>::group_assigned_to_core(
+			core_idx,
+			relay_parent_block_number + One::one(),
+		) {
+			Some(group_idx) => group_idx,
+			None => {
+				log::debug!(target: LOG_TARGET, "Can't get the group index for core idx {:?}. Dropping the candidate.", core_idx);
+				return false
+			},
+		};
+
+		// And finally get the validator group for this group index
+		let validator_group = match <scheduler::Pallet<T>>::group_validators(group_idx) {
+			Some(validator_group) => validator_group,
+			None => {
+				log::debug!(target: LOG_TARGET, "Can't get the validators from group {:?}. Dropping the candidate.", group_idx);
+				return false
+			}
+		};
+
+		// Bitmask with the disabled indecies within the validator group
+		let disabled_indices = BitVec::<u8, bitvec::order::Lsb0>::from_iter(validator_group.iter().map(|idx| disabled_validators.contains(idx)));
+		// The indices of statements from disabled validators in `BackedCandidate`. We have to drop these.
+		let indices_to_drop = disabled_indices.clone() & &bc.validator_indices;
+		// Apply the bitmask to drop the disabled validator from `validator_indices`
+		bc.validator_indices &= !disabled_indices;
+		// Remove the corresponding votes from `validity_votes`
+		for idx in indices_to_drop.iter_ones().rev() {
+			bc.validity_votes.remove(idx);
+		}
+
+		// If at least one statement was dropped we need to return `true`
+		if indices_to_drop.count_ones() > 0 {
+			filtered = true;
+		}
+
+		// By filtering votes we might render the candidate invalid and cause a failure in
+		// [`process_candidates`]. To avoid this we have to perform a sanity check here. If there
+		// are not enough backing votes after filtering we will remove the whole candidate.
+		if bc.validity_votes.len() < effective_minimum_backing_votes(
+			validator_group.len(),
+			configuration::Pallet::<T>::config().minimum_backing_votes
+
+		) {
+			return false
+		}
+
+		true
+	});
+
+	// Also return `true` if a whole candidate was dropped from the set
+	if !filtered {
+		filtered = backed_len_before > backed_candidates.len();
+	}
+
+	filtered
+}