Handling of disabled validators in backing subsystem

cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs

@@ -346,6 +346,13 @@ impl RuntimeApiSubsystemClient for BlockChainRpcClient {
 		Ok(self.rpc_client.parachain_host_minimum_backing_votes(at, session_index).await?)
 	}
 
+	async fn disabled_validators(
+		&self,
+		at: Hash,
+	) -> Result<Vec<polkadot_primitives::ValidatorIndex>, ApiError> {
+		Ok(self.rpc_client.parachain_host_disabled_validators(at).await?)
+	}
+
 	async fn staging_async_backing_params(&self, at: Hash) -> Result<AsyncBackingParams, ApiError> {
 		Ok(self.rpc_client.parachain_host_staging_async_backing_params(at).await?)
 	}

cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs

@@ -598,6 +598,14 @@ impl RelayChainRpcClient {
 			.await
 	}
 
+	pub async fn parachain_host_disabled_validators(
+		&self,
+		at: RelayHash,
+	) -> Result<Vec<ValidatorIndex>, RelayChainError> {
+		self.call_remote_runtime_function("ParachainHost_disabled_validators", at, None::<()>)
+			.await
+	}
+
 	#[allow(missing_docs)]
 	pub async fn parachain_host_staging_async_backing_params(
 		&self,

cumulus/parachains/integration-tests/emulated/common/src/lib.rs

@@ -23,6 +23,7 @@ use constants::{
 	bridge_hub_polkadot, bridge_hub_rococo, collectives, kusama, penpal, polkadot, rococo, westend,
 };
 use impls::{RococoWococoMessageHandler, WococoRococoMessageHandler};
+use polkadot_primitives::runtime_api::runtime_decl_for_parachain_host::ParachainHostV6;
 
 // Substrate
 use frame_support::traits::OnInitialize;

polkadot/node/core/backing/src/lib.rs

@@ -383,6 +383,19 @@ struct TableContext {
 	validator: Option<Validator>,
 	groups: HashMap<ParaId, Vec<ValidatorIndex>>,
 	validators: Vec<ValidatorId>,
+	disabled_validators: Vec<ValidatorIndex>,
+}
+
+impl TableContext {
+	pub fn validator_is_disabled(&self, validator_idx: &ValidatorIndex) -> bool {
+		self.disabled_validators
+			.iter()
+			.any(|disabled_val_idx| *disabled_val_idx == *validator_idx)
+	}
+
+	pub fn local_validator_is_disabled(&self) -> Option<bool> {
+		self.validator.as_ref().map(|v| v.disabled())
+	}
 }
 
 impl TableContextTrait for TableContext {
@@ -992,14 +1005,18 @@ async fn construct_per_relay_parent_state<Context>(
 
 	let parent = relay_parent;
 
-	let (session_index, validators, groups, cores) = futures::try_join!(
+	let (session_index, validators, groups, cores, disabled_validators) = futures::try_join!(
 		request_session_index_for_child(parent, ctx.sender()).await,
 		request_validators(parent, ctx.sender()).await,
 		request_validator_groups(parent, ctx.sender()).await,
 		request_from_runtime(parent, ctx.sender(), |tx| {
 			RuntimeApiRequest::AvailabilityCores(tx)
 		},)
 		.await,
+		request_from_runtime(parent, ctx.sender(), |tx| {
+			RuntimeApiRequest::DisabledValidators(tx)
+		},)
+		.await,
 	)
 	.map_err(Error::JoinMultiple)?;
 
@@ -1009,22 +1026,27 @@ async fn construct_per_relay_parent_state<Context>(
 	let cores = try_runtime_api!(cores);
 	let minimum_backing_votes =
 		try_runtime_api!(request_min_backing_votes(parent, session_index, ctx.sender()).await);
+	let disabled_validators = try_runtime_api!(disabled_validators);
 
 	let signing_context = SigningContext { parent_hash: parent, session_index };
-	let validator =
-		match Validator::construct(&validators, signing_context.clone(), keystore.clone()) {
-			Ok(v) => Some(v),
-			Err(util::Error::NotAValidator) => None,
-			Err(e) => {
-				gum::warn!(
-					target: LOG_TARGET,
-					err = ?e,
-					"Cannot participate in candidate backing",
-				);
+	let validator = match Validator::construct(
+		&validators,
+		&disabled_validators,
+		signing_context.clone(),
+		keystore.clone(),
+	) {
+		Ok(v) => Some(v),
+		Err(util::Error::NotAValidator) => None,
+		Err(e) => {
+			gum::warn!(
+				target: LOG_TARGET,
+				err = ?e,
+				"Cannot participate in candidate backing",
+			);
 
-				return Ok(None)
-			},
-		};
+			return Ok(None)
+		},
+	};
 
 	let mut groups = HashMap::new();
 	let n_cores = cores.len();
@@ -1054,7 +1076,7 @@ async fn construct_per_relay_parent_state<Context>(
 		}
 	}
 
-	let table_context = TableContext { groups, validators, validator };
+	let table_context = TableContext { validator, groups, validators, disabled_validators };
 	let table_config = TableConfig {
 		allow_multiple_seconded: match mode {
 			ProspectiveParachainsMode::Enabled { .. } => true,
@@ -1564,7 +1586,17 @@ async fn import_statement<Context>(
 
 	let stmt = primitive_statement_to_table(statement);
 
-	Ok(rp_state.table.import_statement(&rp_state.table_context, stmt))
+	// Don't import statement if the sender is disabled
+	if rp_state.table_context.validator_is_disabled(&stmt.sender) {
+		gum::debug!(
+			target: LOG_TARGET,
+			sender_validator_idx = ?stmt.sender,
+			"Not importing statement because the sender is disabled"
+		);
+		return Ok(None)
+	} else {
+		Ok(rp_state.table.import_statement(&rp_state.table_context, stmt))
+	}
 }
 
 /// Handles a summary received from [`import_statement`] and dispatches `Backed` notifications and
@@ -1958,6 +1990,13 @@ async fn handle_second_message<Context>(
 		return Ok(())
 	}
 
+	// Just return if the local validator is disabled. If we are here the local node should be a
+	// validator but defensively use `unwrap_or(false)`) to continue processing in this case.
+	if rp_state.table_context.local_validator_is_disabled().unwrap_or(false) {
+		gum::warn!(target: LOG_TARGET, "Local validator is disabled. Don't validate and second");
+		return Ok(())
+	}
+
 	// If the message is a `CandidateBackingMessage::Second`, sign and dispatch a
 	// Seconded statement only if we have not signed a Valid statement for the requested candidate.
 	//

polkadot/node/core/backing/src/tests/mod.rs

@@ -41,7 +41,7 @@ use sp_keyring::Sr25519Keyring;
 use sp_keystore::Keystore;
 use sp_tracing as _;
 use statement_table::v2::Misbehavior;
-use std::collections::HashMap;
+use std::{collections::HashMap, time::Duration};
 
 mod prospective_parachains;
 
@@ -81,6 +81,7 @@ struct TestState {
 	signing_context: SigningContext,
 	relay_parent: Hash,
 	minimum_backing_votes: u32,
+	disabled_validators: Vec<ValidatorIndex>,
 }
 
 impl TestState {
@@ -152,6 +153,7 @@ impl Default for TestState {
 			signing_context,
 			relay_parent,
 			minimum_backing_votes: LEGACY_MIN_BACKING_VOTES,
+			disabled_validators: Vec::new(),
 		}
 	}
 }
@@ -287,6 +289,16 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS
 		}
 	);
 
+	// Check that subsystem job issues a request for the disabled validators.
+	assert_matches!(
+		virtual_overseer.recv().await,
+		AllMessages::RuntimeApi(
+			RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx))
+		) if parent == test_state.relay_parent => {
+			tx.send(Ok(test_state.disabled_validators.clone())).unwrap();
+		}
+	);
+
 	// Check if subsystem job issues a request for the minimum backing votes.
 	assert_matches!(
 		virtual_overseer.recv().await,
@@ -1596,6 +1608,7 @@ fn candidate_backing_reorders_votes() {
 
 	let table_context = TableContext {
 		validator: None,
+		disabled_validators: Vec::new(),
 		groups: validator_groups,
 		validators: validator_public.clone(),
 	};
@@ -2256,3 +2269,51 @@ fn new_leaf_view_doesnt_clobber_old() {
 		virtual_overseer
 	});
 }
+
+// Test that a disabled local validator doesn't do any work on `CandidateBackingMessage::Second`
+#[test]
+fn disabled_validator_doesnt_distribute_statement() {
+	let mut test_state = TestState::default();
+	test_state.disabled_validators.push(ValidatorIndex(0));
+
+	test_harness(test_state.keystore.clone(), |mut virtual_overseer| async move {
+		test_startup(&mut virtual_overseer, &test_state).await;
+
+		let pov = PoV { block_data: BlockData(vec![42, 43, 44]) };
+		let pvd = dummy_pvd();
+		let validation_code = ValidationCode(vec![1, 2, 3]);
+
+		let expected_head_data = test_state.head_data.get(&test_state.chain_ids[0]).unwrap();
+
+		let pov_hash = pov.hash();
+		let candidate = TestCandidateBuilder {
+			para_id: test_state.chain_ids[0],
+			relay_parent: test_state.relay_parent,
+			pov_hash,
+			head_data: expected_head_data.clone(),
+			erasure_root: make_erasure_root(&test_state, pov.clone(), pvd.clone()),
+			persisted_validation_data_hash: pvd.hash(),
+			validation_code: validation_code.0.clone(),
+		}
+		.build();
+
+		let second = CandidateBackingMessage::Second(
+			test_state.relay_parent,
+			candidate.to_plain(),
+			pvd.clone(),
+			pov.clone(),
+		);
+
+		virtual_overseer.send(FromOrchestra::Communication { msg: second }).await;
+
+		// Ensure backing subsystem is not doing any work
+		assert_matches!(virtual_overseer.recv().timeout(Duration::from_secs(1)).await, None);
+
+		virtual_overseer
+			.send(FromOrchestra::Signal(OverseerSignal::ActiveLeaves(
+				ActiveLeavesUpdate::stop_work(test_state.relay_parent),
+			)))
+			.await;
+		virtual_overseer
+	});
+}

polkadot/node/core/backing/src/tests/prospective_parachains.rs

@@ -185,6 +185,16 @@ async fn activate_leaf(
 			}
 		);
 
+		// Check that the subsystem job issues a request for the disabled validators.
+		assert_matches!(
+			virtual_overseer.recv().await,
+			AllMessages::RuntimeApi(
+				RuntimeApiMessage::Request(parent, RuntimeApiRequest::DisabledValidators(tx))
+			) if parent == hash => {
+				tx.send(Ok(Vec::new())).unwrap();
+			}
+		);
+
 		// Check if subsystem job issues a request for the minimum backing votes.
 		assert_matches!(
 			virtual_overseer.recv().await,

polkadot/node/core/runtime-api/src/cache.rs

@@ -66,6 +66,7 @@ pub(crate) struct RequestResultCache {
 	key_ownership_proof:
 		LruMap<(Hash, ValidatorId), Option<vstaging::slashing::OpaqueKeyOwnershipProof>>,
 	minimum_backing_votes: LruMap<SessionIndex, u32>,
+	disabled_validators: LruMap<Hash, Vec<ValidatorIndex>>,
 
 	staging_para_backing_state: LruMap<(Hash, ParaId), Option<vstaging::BackingState>>,
 	staging_async_backing_params: LruMap<Hash, vstaging::AsyncBackingParams>,
@@ -99,6 +100,7 @@ impl Default for RequestResultCache {
 			unapplied_slashes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 			key_ownership_proof: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 			minimum_backing_votes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
+			disabled_validators: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 
 			staging_para_backing_state: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 			staging_async_backing_params: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
@@ -448,6 +450,21 @@ impl RequestResultCache {
 		self.minimum_backing_votes.insert(session_index, minimum_backing_votes);
 	}
 
+	pub(crate) fn disabled_validators(
+		&mut self,
+		relay_parent: &Hash,
+	) -> Option<&Vec<ValidatorIndex>> {
+		self.disabled_validators.get(relay_parent).map(|v| &*v)
+	}
+
+	pub(crate) fn cache_disabled_validators(
+		&mut self,
+		relay_parent: Hash,
+		disabled_validators: Vec<ValidatorIndex>,
+	) {
+		self.disabled_validators.insert(relay_parent, disabled_validators);
+	}
+
 	pub(crate) fn staging_para_backing_state(
 		&mut self,
 		key: (Hash, ParaId),
@@ -524,6 +541,7 @@ pub(crate) enum RequestResult {
 		vstaging::slashing::OpaqueKeyOwnershipProof,
 		Option<()>,
 	),
+	DisabledValidators(Hash, Vec<ValidatorIndex>),
 
 	StagingParaBackingState(Hash, ParaId, Option<vstaging::BackingState>),
 	StagingAsyncBackingParams(Hash, vstaging::AsyncBackingParams),

polkadot/node/core/runtime-api/src/lib.rs

@@ -166,6 +166,8 @@ where
 				.requests_cache
 				.cache_key_ownership_proof((relay_parent, validator_id), key_ownership_proof),
 			SubmitReportDisputeLost(_, _, _, _) => {},
+			DisabledValidators(relay_parent, disabled_validators) =>
+				self.requests_cache.cache_disabled_validators(relay_parent, disabled_validators),
 
 			StagingParaBackingState(relay_parent, para_id, constraints) => self
 				.requests_cache
@@ -297,6 +299,8 @@ where
 						Request::SubmitReportDisputeLost(dispute_proof, key_ownership_proof, sender)
 					},
 				),
+			Request::DisabledValidators(sender) => query!(disabled_validators(), sender)
+				.map(|sender| Request::DisabledValidators(sender)),
 
 			Request::StagingParaBackingState(para, sender) =>
 				query!(staging_para_backing_state(para), sender)
@@ -569,6 +573,12 @@ where
 			ver = Request::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT,
 			sender
 		),
+		Request::DisabledValidators(sender) => query!(
+			DisabledValidators,
+			disabled_validators(),
+			ver = Request::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT,
+			sender
+		),
 
 		Request::StagingParaBackingState(para, sender) => {
 			query!(

polkadot/node/core/runtime-api/src/tests.rs

@@ -250,6 +250,10 @@ impl RuntimeApiSubsystemClient for MockSubsystemClient {
 		Ok(self.authorities.clone())
 	}
 
+	async fn disabled_validators(&self, _: Hash) -> Result<Vec<ValidatorIndex>, ApiError> {
+		todo!("Not required for tests")
+	}
+
 	async fn staging_async_backing_params(
 		&self,
 		_: Hash,

polkadot/node/subsystem-types/src/messages.rs

@@ -695,6 +695,9 @@ pub enum RuntimeApiRequest {
 	),
 	/// Get the minimum required backing votes.
 	MinimumBackingVotes(SessionIndex, RuntimeApiSender<u32>),
+	/// Returns all disabled validators at a given block height.
+	/// `V6`
+	DisabledValidators(RuntimeApiSender<Vec<ValidatorIndex>>),
 
 	/// Get the backing state of the given para.
 	/// This is a staging API that will not be available on production runtimes.
@@ -726,6 +729,9 @@ impl RuntimeApiRequest {
 	/// `MinimumBackingVotes`
 	pub const MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT: u32 = 6;
 
+	/// `DisabledValidators`
+	pub const DISABLED_VALIDATORS_RUNTIME_REQUIREMENT: u32 = 6;
+
 	/// Minimum version for backing state, required for async backing.
 	///
 	/// 99 for now, should be adjusted to VSTAGING/actual runtime version once released.