Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix some unjustified disputes #6103

Merged
merged 10 commits into from
Jan 22, 2023
Merged

Fix some unjustified disputes #6103

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6a5df50
Fix indentation + add warning
eskimor Oct 4, 2022
55d8d01
Don't vote invalid on internal errors.
eskimor Oct 4, 2022
8d078cf
Don't dispute on code compression error.
eskimor Oct 4, 2022
84a135a
Remove CodeDecompressionError
eskimor Oct 4, 2022
f81357b
Candidate not invalid if PVF preparation fails.
eskimor Oct 6, 2022
7051858
Fix malus
eskimor Oct 6, 2022
5159879
Add clarifying comment.
eskimor Oct 6, 2022
a88f697
cargo fmt
eskimor Oct 6, 2022
3198c8d
Fix indentation.
eskimor Oct 11, 2022
a3fa89d
Merge branch 'master' into rk-no-dispute-on-node-restart
eskimor Oct 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions node/core/candidate-validation/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does every network now have pre-checking turned on?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the node side, as soon as people upgrade to 0.9.30 (#5977). In the runtime, Kusama and Polkadot still don't.

pvfCheckingEnabled: false

Someone needs to submit a motion I suppose.

},
};

Expand Down Expand Up @@ -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))) =>
Expand All @@ -570,8 +570,7 @@ async fn validate_candidate_exhaustive(
"ambiguous worker death".to_string(),
))),
Err(ValidationError::InvalidCandidate(WasmInvalidCandidate::PrepareError(e))) =>
Ok(ValidationResult::Invalid(InvalidCandidate::ExecutionError(e))),

Err(ValidationFailed(e)),
eskimor marked this conversation as resolved.
Show resolved Hide resolved
Ok(res) =>
if res.head_data.hash() != candidate_receipt.descriptor.para_head {
gum::info!(target: LOG_TARGET, ?para_id, "Invalid candidate (para_head)");
Expand Down
4 changes: 2 additions & 2 deletions node/core/candidate-validation/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,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]);
Expand Down Expand Up @@ -714,7 +714,7 @@ fn code_decompression_failure_is_invalid() {
&Default::default(),
));

assert_matches!(v, Ok(ValidationResult::Invalid(InvalidCandidate::CodeDecompressionFailure)));
assert_matches!(v, Err(_));
}

#[test]
Expand Down
131 changes: 67 additions & 64 deletions node/core/dispute-coordinator/src/initialized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
eskimor marked this conversation as resolved.
Show resolved Hide resolved
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();
Expand Down Expand Up @@ -391,19 +394,19 @@ impl Initialized {
CompactStatement::Valid(_) =>
ValidDisputeStatementKind::BackingValid(relay_parent),
};
debug_assert!(
SignedDisputeStatement::new_checked(
debug_assert!(
SignedDisputeStatement::new_checked(
DisputeStatement::Valid(valid_statement_kind),
candidate_hash,
session,
validator_public.clone(),
validator_signature.clone(),
).is_ok(),
"Scraped backing votes had invalid signature! candidate: {:?}, session: {:?}, validator_public: {:?}",
candidate_hash,
session,
validator_public,
);
).is_ok(),
"Scraped backing votes had invalid signature! candidate: {:?}, session: {:?}, validator_public: {:?}",
candidate_hash,
session,
validator_public,
);
let signed_dispute_statement =
SignedDisputeStatement::new_unchecked_from_trusted_source(
DisputeStatement::Valid(valid_statement_kind),
Expand Down Expand Up @@ -485,20 +488,20 @@ impl Initialized {
})
.cloned()?;

debug_assert!(
SignedDisputeStatement::new_checked(
debug_assert!(
SignedDisputeStatement::new_checked(
dispute_statement.clone(),
candidate_hash,
session,
validator_public.clone(),
validator_signature.clone(),
).is_ok(),
"Scraped dispute votes had invalid signature! candidate: {:?}, session: {:?}, dispute_statement: {:?}, validator_public: {:?}",
candidate_hash,
session,
).is_ok(),
"Scraped dispute votes had invalid signature! candidate: {:?}, session: {:?}, dispute_statement: {:?}, validator_public: {:?}",
candidate_hash,
session,
dispute_statement,
validator_public,
);
validator_public,
);

Some((
SignedDisputeStatement::new_unchecked_from_trusted_source(
Expand Down
2 changes: 1 addition & 1 deletion node/core/dispute-coordinator/src/participation/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,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))) => {
Expand Down
2 changes: 0 additions & 2 deletions node/primitives/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,6 @@ pub enum InvalidCandidate {
ParamsTooLarge(u64),
/// Code size is over the limit.
CodeTooLarge(u64),
/// Code does not decompress correctly.
eskimor marked this conversation as resolved.
Show resolved Hide resolved
CodeDecompressionFailure,
/// PoV does not decompress correctly.
PoVDecompressionFailure,
/// Validation function returned invalid data.
Expand Down