[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;