From bec4168baa5de648a148aed0494cea78d661662d Mon Sep 17 00:00:00 2001
From: eskimor <eskimor@users.noreply.github.com>
Date: Sun, 22 Jan 2023 20:21:37 +0100
Subject: [PATCH] Fix some unjustified disputes (#6103)
* Fix indentation + add warning
on participation errors.
* Don't vote invalid on internal errors.
* Don't dispute on code compression error.
* Remove CodeDecompressionError
* Candidate not invalid if PVF preparation fails.
Instead: Report error.
* Fix malus
* Add clarifying comment.
* cargo fmt
* Fix indentation.
---
.../node/core/candidate-validation/src/lib.rs | 16 +--
.../core/candidate-validation/src/tests.rs | 4 +-
.../dispute-coordinator/src/initialized.rs | 99 ++++++++++---------
.../src/participation/mod.rs | 2 +-
polkadot/node/malus/src/variants/common.rs | 4 -
polkadot/node/primitives/src/lib.rs | 2 -
6 files changed, 64 insertions(+), 63 deletions(-)
diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs
index 58d91946830..ff5073da56a 100644
--- a/polkadot/node/core/candidate-validation/src/lib.rs
+++ b/polkadot/node/core/candidate-validation/src/lib.rs
@@ -527,8 +527,9 @@ async fn validate_candidate_exhaustive(
Err(e) => {
gum::info!(target: LOG_TARGET, ?para_id, err=?e, "Invalid candidate (validation code)");
- // If the validation code is invalid, the candidate certainly is.
- return Ok(ValidationResult::Invalid(InvalidCandidate::CodeDecompressionFailure))
+ // Code already passed pre-checking, if decompression fails now this most likley means
+ // some local corruption happened.
+ return Err(ValidationFailed("Code decompression failed".to_string()))
},
};
@@ -560,7 +561,6 @@ async fn validate_candidate_exhaustive(
match result {
Err(ValidationError::InternalError(e)) => Err(ValidationFailed(e)),
-
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::HardTimeout)) =>
Ok(ValidationResult::Invalid(InvalidCandidate::Timeout)),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::WorkerReportedError(e))) =>
@@ -569,9 +569,13 @@ async fn validate_candidate_exhaustive(
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(
"ambiguous worker death".to_string(),
))),
- Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::PrepareError(e))) =>
- Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(e))),
-
+ Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::PrepareError(e))) => {
+ // In principle if preparation of the `WASM` fails, the current candidate can not be the
+ // reason for that. So we can't say whether it is invalid or not in addition with
+ // pre-checking enabled only valid runtimes should ever get enacted, so we can be
+ // reasonably sure that this is some local problem on the current node.
+ Err(ValidationFailed(e))
+ },
Ok(res) =>
if res.head_data.hash() != candidate_receipt.descriptor.para_head {
gum::info!(target: LOG_TARGET, ?para_id, "Invalid candidate (para_head)");
diff --git a/polkadot/node/core/candidate-validation/src/tests.rs b/polkadot/node/core/candidate-validation/src/tests.rs
index 445ea118f3d..c47df9589e4 100644
--- a/polkadot/node/core/candidate-validation/src/tests.rs
+++ b/polkadot/node/core/candidate-validation/src/tests.rs
@@ -799,7 +799,7 @@ fn compressed_code_works() {
}
#[test]
-fn code_decompression_failure_is_invalid() {
+fn code_decompression_failure_is_error() {
let validation_data = PersistedValidationData { max_pov_size: 1024, ..Default::default() };
let pov = PoV { block_data: BlockData(vec![1; 32]) };
let head_data = HeadData(vec![1, 1, 1]);
@@ -842,7 +842,7 @@ fn code_decompression_failure_is_invalid() {
&Default::default(),
));
- assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::CodeDecompressionFailure)));
+ assert_matches!(v, Err(_));
}
#[test]
diff --git a/polkadot/node/core/dispute-coordinator/src/initialized.rs b/polkadot/node/core/dispute-coordinator/src/initialized.rs
index 9d934c4956c..daab2838d7e 100644
--- a/polkadot/node/core/dispute-coordinator/src/initialized.rs
+++ b/polkadot/node/core/dispute-coordinator/src/initialized.rs
@@ -198,59 +198,62 @@ impl Initialized {
gum::trace!(target: LOG_TARGET, "Waiting for message");
let mut overlay_db = OverlayedBackend::new(backend);
let default_confirm = Box::new(|| Ok(()));
- let confirm_write =
- match MuxedMessage::receive(ctx, &mut self.participation_receiver).await? {
- MuxedMessage::Participation(msg) => {
- gum::trace!(target: LOG_TARGET, "MuxedMessage::Participation");
- let ParticipationStatement {
- session,
+ let confirm_write = match MuxedMessage::receive(ctx, &mut self.participation_receiver)
+ .await?
+ {
+ MuxedMessage::Participation(msg) => {
+ gum::trace!(target: LOG_TARGET, "MuxedMessage::Participation");
+ let ParticipationStatement {
+ session,
+ candidate_hash,
+ candidate_receipt,
+ outcome,
+ } = self.participation.get_participation_result(ctx, msg).await?;
+ if let Some(valid) = outcome.validity() {
+ gum::trace!(
+ target: LOG_TARGET,
+ ?session,
+ ?candidate_hash,
+ ?valid,
+ "Issuing local statement based on participation outcome."
+ );
+ self.issue_local_statement(
+ ctx,
+ &mut overlay_db,
candidate_hash,
candidate_receipt,
- outcome,
- } = self.participation.get_participation_result(ctx, msg).await?;
- if let Some(valid) = outcome.validity() {
- gum::trace!(
- target: LOG_TARGET,
- ?session,
- ?candidate_hash,
- ?valid,
- "Issuing local statement based on participation outcome."
- );
- self.issue_local_statement(
- ctx,
- &mut overlay_db,
- candidate_hash,
- candidate_receipt,
- session,
- valid,
- clock.now(),
- )
- .await?;
- }
+ session,
+ valid,
+ clock.now(),
+ )
+ .await?;
+ } else {
+ gum::warn!(target: LOG_TARGET, ?outcome, "Dispute participation failed");
+ }
+ default_confirm
+ },
+ MuxedMessage::Subsystem(msg) => match msg {
+ FromOrchestra::Signal(OverseerSignal::Conclude) => return Ok(()),
+ FromOrchestra::Signal(OverseerSignal::ActiveLeaves(update)) => {
+ gum::trace!(target: LOG_TARGET, "OverseerSignal::ActiveLeaves");
+ self.process_active_leaves_update(
+ ctx,
+ &mut overlay_db,
+ update,
+ clock.now(),
+ )
+ .await?;
default_confirm
},
- MuxedMessage::Subsystem(msg) => match msg {
- FromOrchestra::Signal(OverseerSignal::Conclude) => return Ok(()),
- FromOrchestra::Signal(OverseerSignal::ActiveLeaves(update)) => {
- gum::trace!(target: LOG_TARGET, "OverseerSignal::ActiveLeaves");
- self.process_active_leaves_update(
- ctx,
- &mut overlay_db,
- update,
- clock.now(),
- )
- .await?;
- default_confirm
- },
- FromOrchestra::Signal(OverseerSignal::BlockFinalized(_, n)) => {
- gum::trace!(target: LOG_TARGET, "OverseerSignal::BlockFinalized");
- self.scraper.process_finalized_block(&n);
- default_confirm
- },
- FromOrchestra::Communication { msg } =>
- self.handle_incoming(ctx, &mut overlay_db, msg, clock.now()).await?,
+ FromOrchestra::Signal(OverseerSignal::BlockFinalized(_, n)) => {
+ gum::trace!(target: LOG_TARGET, "OverseerSignal::BlockFinalized");
+ self.scraper.process_finalized_block(&n);
+ default_confirm
},
- };
+ FromOrchestra::Communication { msg } =>
+ self.handle_incoming(ctx, &mut overlay_db, msg, clock.now()).await?,
+ },
+ };
if !overlay_db.is_empty() {
let ops = overlay_db.into_write_ops();
diff --git a/polkadot/node/core/dispute-coordinator/src/participation/mod.rs b/polkadot/node/core/dispute-coordinator/src/participation/mod.rs
index 78a3a259207..f813b216b6a 100644
--- a/polkadot/node/core/dispute-coordinator/src/participation/mod.rs
+++ b/polkadot/node/core/dispute-coordinator/src/participation/mod.rs
@@ -373,7 +373,7 @@ async fn participate(
err,
);
- send_result(&mut result_sender, req, ParticipationOutcome::Invalid).await;
+ send_result(&mut result_sender, req, ParticipationOutcome::Error).await;
},
Ok(Ok(ValidationResult::Invalid(invalid))) => {
diff --git a/polkadot/node/malus/src/variants/common.rs b/polkadot/node/malus/src/variants/common.rs
index d92a7315467..3a3cc19aef1 100644
--- a/polkadot/node/malus/src/variants/common.rs
+++ b/polkadot/node/malus/src/variants/common.rs
@@ -62,8 +62,6 @@ pub enum FakeCandidateValidationError {
ParamsTooLarge,
/// Code size is over the limit.
CodeTooLarge,
- /// Code does not decompress correctly.
- CodeDecompressionFailure,
/// PoV does not decompress correctly.
POVDecompressionFailure,
/// Validation function returned invalid data.
@@ -89,8 +87,6 @@ impl Into<InvalidCandidate> for FakeCandidateValidationError {
FakeCandidateValidationError::Timeout => InvalidCandidate::Timeout,
FakeCandidateValidationError::ParamsTooLarge => InvalidCandidate::ParamsTooLarge(666),
FakeCandidateValidationError::CodeTooLarge => InvalidCandidate::CodeTooLarge(666),
- FakeCandidateValidationError::CodeDecompressionFailure =>
- InvalidCandidate::CodeDecompressionFailure,
FakeCandidateValidationError::POVDecompressionFailure =>
InvalidCandidate::PoVDecompressionFailure,
FakeCandidateValidationError::BadReturn => InvalidCandidate::BadReturn,
diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs
index 2d76a14c115..4c141e622e0 100644
--- a/polkadot/node/primitives/src/lib.rs
+++ b/polkadot/node/primitives/src/lib.rs
@@ -237,8 +237,6 @@ pub enum InvalidCandidate {
ParamsTooLarge(u64),
/// Code size is over the limit.
CodeTooLarge(u64),
- /// Code does not decompress correctly.
- CodeDecompressionFailure,
/// PoV does not decompress correctly.
PoVDecompressionFailure,
/// Validation function returned invalid data.
--
GitLab