Don't wait for dispute coordinator (#5157)

* Don't wait for dispute coordinator in backing and approval-voting - we are single threaded there, so this is blocking everything. * Add missing import. * Don't warn on dropped receiver.
paritytech · Mar 18, 2022 · 6daf3b6 · 6daf3b6
1 parent 6eefcc7
commit 6daf3b6
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 26 deletions.
diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs
@@ -35,7 +35,7 @@ use polkadot_node_subsystem::{
 		ApprovalVotingMessage, AssignmentCheckError, AssignmentCheckResult,
 		AvailabilityRecoveryMessage, BlockDescription, CandidateValidationMessage, ChainApiMessage,
 		ChainSelectionMessage, DisputeCoordinatorMessage, HighestApprovedAncestorBlock,
-		ImportStatementsResult, RuntimeApiMessage, RuntimeApiRequest,
+		RuntimeApiMessage, RuntimeApiRequest,
 	},
 	overseer::{self, SubsystemSender as _},
 	FromOverseer, OverseerSignal, SpawnedSubsystem, SubsystemContext, SubsystemError,
@@ -950,7 +950,9 @@ async fn handle_actions(
 				dispute_statement,
 				validator_index,
 			} => {
-				let (pending_confirmation, confirmation_rx) = oneshot::channel();
+				// TODO: Log confirmation results in an efficient way:
+				// https://github.com/paritytech/polkadot/issues/5156
+				let (pending_confirmation, _confirmation_rx) = oneshot::channel();
 				ctx.send_message(DisputeCoordinatorMessage::ImportStatements {
 					candidate_hash,
 					candidate_receipt,
@@ -959,15 +961,6 @@ async fn handle_actions(
 					pending_confirmation,
 				})
 				.await;
-
-				match confirmation_rx.await {
-					Err(oneshot::Canceled) => {
-						gum::debug!(target: LOG_TARGET, "Dispute coordinator confirmation lost",)
-					},
-					Ok(ImportStatementsResult::ValidImport) => {},
-					Ok(ImportStatementsResult::InvalidImport) =>
-						gum::warn!(target: LOG_TARGET, "Failed to import statements of validity",),
-				}
 			},
 			Action::NoteApprovedInChainSelection(block_hash) => {
 				ctx.send_message(ChainSelectionMessage::Approved(block_hash)).await;

diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs
@@ -25,6 +25,7 @@ use polkadot_node_primitives::{
 use polkadot_node_subsystem::{
 	messages::{
 		AllMessages, ApprovalVotingMessage, AssignmentCheckResult, AvailabilityRecoveryMessage,
+		ImportStatementsResult,
 	},
 	ActivatedLeaf, ActiveLeavesUpdate, LeafStatus,
 };

diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs
@@ -50,8 +50,8 @@ use polkadot_subsystem::{
 	messages::{
 		AllMessages, AvailabilityDistributionMessage, AvailabilityStoreMessage,
 		CandidateBackingMessage, CandidateValidationMessage, CollatorProtocolMessage,
-		DisputeCoordinatorMessage, ImportStatementsResult, ProvisionableData, ProvisionerMessage,
-		RuntimeApiRequest, StatementDistributionMessage, ValidationFailed,
+		DisputeCoordinatorMessage, ProvisionableData, ProvisionerMessage, RuntimeApiRequest,
+		StatementDistributionMessage, ValidationFailed,
 	},
 	overseer, ActivatedLeaf, PerLeafSpan, Stage, SubsystemSender,
 };
@@ -891,7 +891,9 @@ impl CandidateBackingJob {
 		if let (Some(candidate_receipt), Some(dispute_statement)) =
 			(maybe_candidate_receipt, maybe_signed_dispute_statement)
 		{
-			let (pending_confirmation, confirmation_rx) = oneshot::channel();
+			// TODO: Log confirmation results in an efficient way:
+			// https://github.com/paritytech/polkadot/issues/5156
+			let (pending_confirmation, _confirmation_rx) = oneshot::channel();
 			sender
 				.send_message(DisputeCoordinatorMessage::ImportStatements {
 					candidate_hash,
@@ -901,16 +903,6 @@ impl CandidateBackingJob {
 					pending_confirmation,
 				})
 				.await;
-
-			match confirmation_rx.await {
-				Err(oneshot::Canceled) => {
-					gum::debug!(target: LOG_TARGET, "Dispute coordinator confirmation lost",)
-				},
-				Ok(ImportStatementsResult::ValidImport) => {},
-				Ok(ImportStatementsResult::InvalidImport) => {
-					gum::warn!(target: LOG_TARGET, "Failed to import statements of validity",)
-				},
-			}
 		}
 
 		Ok(())

diff --git a/node/core/backing/src/tests.rs b/node/core/backing/src/tests.rs
@@ -27,7 +27,9 @@ use polkadot_primitives::v2::{
 	CollatorId, GroupRotationInfo, HeadData, PersistedValidationData, ScheduledCore,
 };
 use polkadot_subsystem::{
-	messages::{CollatorProtocolMessage, RuntimeApiMessage, RuntimeApiRequest},
+	messages::{
+		CollatorProtocolMessage, ImportStatementsResult, RuntimeApiMessage, RuntimeApiRequest,
+	},
 	ActivatedLeaf, ActiveLeavesUpdate, FromOverseer, LeafStatus, OverseerSignal,
 };
 use sp_application_crypto::AppKey;

diff --git a/node/core/dispute-coordinator/src/error.rs b/node/core/dispute-coordinator/src/error.rs
@@ -118,7 +118,7 @@ impl JfyiError {
 	pub fn log(self) {
 		match self {
 			// don't spam the log with spurious errors
-			Self::Runtime(_) | Self::Oneshot(_) => {
+			Self::Runtime(_) | Self::Oneshot(_) | Self::DisputeImportOneshotSend => {
 				gum::debug!(target: LOG_TARGET, error = ?self)
 			},
 			// it's worth reporting otherwise