[DNM] approval-voting: approvals signature checks optimizations

node/core/approval-voting/src/lib.rs

@@ -1269,14 +1269,15 @@ async fn handle_from_overseer<Context>(
 
 				actions
 			},
-			ApprovalVotingMessage::CheckAndImportApproval(a, res) =>
+			ApprovalVotingMessage::CheckAndImportApproval(a, res, sender_is_originator) =>
 				check_and_import_approval(
 					ctx.sender(),
 					state,
 					db,
 					session_info_provider,
 					metrics,
 					a,
+					sender_is_originator,
 					|r| {
 						let _ = res.send(r);
 					},
@@ -1930,6 +1931,7 @@ async fn check_and_import_approval<T, Sender>(
 	session_info_provider: &mut RuntimeInfo,
 	metrics: &Metrics,
 	approval: IndirectSignedApprovalVote,
+	sender_is_originator: bool,
 	with_response: impl FnOnce(ApprovalCheckResult) -> T,
 ) -> SubsystemResult<(Vec<Action>, T)>
 where
@@ -1997,17 +1999,21 @@ where
 	};
 
 	// Signature check:
-	match DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking).check_signature(
-		&pubkey,
-		approved_candidate_hash,
-		block_entry.session(),
-		&approval.signature,
-	) {
-		Err(_) => respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::InvalidSignature(
-			approval.validator
-		),)),
-		Ok(()) => {},
-	};
+	// We perform signatures checks just for the cases where the approvals weren't received directly from
+	// peer and we received them via gossipping.
+	if !sender_is_originator {
+		match DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking).check_signature(
+			&pubkey,
+			approved_candidate_hash,
+			block_entry.session(),
+			&approval.signature,
+		) {
+			Err(_) => respond_early!(ApprovalCheckResult::Bad(
+				ApprovalCheckError::InvalidSignature(approval.validator),
+			)),
+			Ok(()) => {},
+		};
+	}
 
 	let candidate_entry = match db.load_candidate_entry(&approved_candidate_hash)? {
 		Some(c) => c,

node/core/approval-voting/src/tests.rs

@@ -590,6 +590,7 @@ async fn check_and_import_approval(
 			msg: ApprovalVotingMessage::CheckAndImportApproval(
 				IndirectSignedApprovalVote { block_hash, candidate_index, validator, signature },
 				tx,
+				false,
 			),
 		},
 	)

node/core/dispute-coordinator/src/import.rs

@@ -518,17 +518,26 @@ impl ImportResult {
 		let (mut votes, _) = new_state.into_old_state();
 
 		for (index, sig) in approval_votes.into_iter() {
-			debug_assert!(
-				{
-					let pub_key = &env.session_info().validators.get(index).expect("indices are validated by approval-voting subsystem; qed");
-					let candidate_hash = votes.candidate_receipt.hash();
-					let session_index = env.session_index();
-					DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking)
-						.check_signature(pub_key, candidate_hash, session_index, &sig)
-						.is_ok()
-				},
-				"Signature check for imported approval votes failed! This is a serious bug. Session: {:?}, candidate hash: {:?}, validator index: {:?}", env.session_index(), votes.candidate_receipt.hash(), index
-			);
+			let pub_key = &env
+				.session_info()
+				.validators
+				.get(index)
+				.expect("indices are validated by approval-voting subsystem; qed");
+			let candidate_hash = votes.candidate_receipt.hash();
+			let session_index = env.session_index();
+			// The candidate sent us an invalid signature, so don't import it.
+			// This might happen because in approval-voting we do not checks signatures for votes received from the originator.
+			if !DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking)
+				.check_signature(pub_key, candidate_hash, session_index, &sig)
+				.is_ok()
+			{
+				gum::trace!(
+					target: LOG_TARGET,
+					"Approval checking signature was invalid, so just ignore it"
+				);
+				continue
+			}
+
 			if votes.valid.insert_vote(index, ValidDisputeStatementKind::ApprovalChecking, sig) {
 				imported_valid_votes += 1;
 				imported_approval_votes += 1;

node/malus/Cargo.toml

@@ -24,6 +24,8 @@ polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker" }
 polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker" }
 polkadot-node-primitives = { path = "../primitives" }
 polkadot-primitives = { path = "../../primitives" }
+polkadot-node-network-protocol = { path = "../network/protocol" }
+
 color-eyre = { version = "0.6.1", default-features = false }
 assert_matches = "1.5"
 async-trait = "0.1.57"

node/malus/src/interceptor.rs

@@ -40,7 +40,7 @@ where
 	/// For non-trivial cases, the `sender` can be used to send
 	/// multiple messages after doing some additional processing.
 	fn intercept_incoming(
-		&self,
+		&mut self,
 		_sender: &mut Sender,
 		msg: FromOrchestra<Self::Message>,
 	) -> Option<FromOrchestra<Self::Message>> {

node/malus/src/malus.rs

@@ -36,6 +36,8 @@ enum NemesisVariant {
 	BackGarbageCandidate(BackGarbageCandidateOptions),
 	/// Delayed disputing of ancestors that are perfectly fine.
 	DisputeAncestor(DisputeAncestorOptions),
+	/// Do not distribute approvals to all nodes
+	WitholdApprovalsDistribution(WitholdApprovalsDistributionOptions),
 
 	#[allow(missing_docs)]
 	#[command(name = "prepare-worker", hide = true)]
@@ -59,6 +61,7 @@ impl MalusCli {
 	/// Launch a malus node.
 	fn launch(self) -> eyre::Result<()> {
 		let finality_delay = self.finality_delay;
+
 		match self.variant {
 			NemesisVariant::BackGarbageCandidate(opts) => {
 				let BackGarbageCandidateOptions { percentage, cli } = opts;
@@ -88,6 +91,15 @@ impl MalusCli {
 					finality_delay,
 				)?
 			},
+			NemesisVariant::WitholdApprovalsDistribution(WitholdApprovalsDistributionOptions {
+				num_network_groups,
+				assigned_network_group,
+				cli,
+			}) => polkadot_cli::run_node(
+				cli,
+				WitholdApprovalsDistribution { num_network_groups, assigned_network_group },
+				finality_delay,
+			)?,
 			NemesisVariant::PvfPrepareWorker(cmd) => {
 				#[cfg(target_os = "android")]
 				{

node/malus/src/variants/common.rs

@@ -205,7 +205,7 @@ where
 
 	// Capture all (approval and backing) candidate validation requests and depending on configuration fail them.
 	fn intercept_incoming(
-		&self,
+		&mut self,
 		subsystem_sender: &mut Sender,
 		msg: FromOrchestra<Self::Message>,
 	) -> Option<FromOrchestra<Self::Message>> {

node/malus/src/variants/mod.rs

@@ -20,10 +20,14 @@ mod back_garbage_candidate;
 mod common;
 mod dispute_valid_candidates;
 mod suggest_garbage_candidate;
+mod withold_approvals_distribution;
 
 pub(crate) use self::{
 	back_garbage_candidate::{BackGarbageCandidateOptions, BackGarbageCandidates},
 	dispute_valid_candidates::{DisputeAncestorOptions, DisputeValidCandidates},
 	suggest_garbage_candidate::{SuggestGarbageCandidateOptions, SuggestGarbageCandidates},
+	withold_approvals_distribution::{
+		WitholdApprovalsDistribution, WitholdApprovalsDistributionOptions,
+	},
 };
 pub(crate) use common::*;

node/malus/src/variants/suggest_garbage_candidate.rs

@@ -72,7 +72,7 @@ where
 
 	/// Intercept incoming `Second` requests from the `collator-protocol` subsystem.
 	fn intercept_incoming(
-		&self,
+		&mut self,
 		subsystem_sender: &mut Sender,
 		msg: FromOrchestra<Self::Message>,
 	) -> Option<FromOrchestra<Self::Message>> {