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

Malus: improvements in dispute ancestor and suggest garbage candidate implementation #5011

Merged
merged 46 commits into from
Apr 13, 2022
Merged

Malus: improvements in dispute ancestor and suggest garbage candidate implementation #5011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
93656e2
Implement fake validation results
sandreim Feb 14, 2022
f216579
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Feb 28, 2022
2744e75
refactor
sandreim Mar 1, 2022
9941f03
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Mar 1, 2022
a3172b0
cargo lock
sandreim Mar 1, 2022
9100866
spell check
sandreim Mar 1, 2022
efd37ad
spellcheck
sandreim Mar 2, 2022
d4da58d
typos
sandreim Mar 2, 2022
b4719b4
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Mar 4, 2022
b70512b
Review feedback
sandreim Mar 10, 2022
050bab4
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Mar 10, 2022
f786866
move stuff around
sandreim Mar 10, 2022
9e72a42
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Mar 23, 2022
c4a4560
chores
sandreim Mar 23, 2022
e90ab09
Impl valid - still wip
sandreim Mar 24, 2022
8226f4e
fixes
sandreim Mar 26, 2022
35c8c8e
fmt
sandreim Mar 26, 2022
1c279dc
Pull Ladi's implementation:
sandreim Mar 28, 2022
8932494
Fix build
sandreim Mar 28, 2022
fb75de4
Logs and comments
sandreim Mar 28, 2022
9408f45
WIP: suggest garbage candidate + implement validation result caching
sandreim Mar 28, 2022
78ed119
fix
sandreim Mar 28, 2022
efe4e50
Do commitment hash checks in candidate validation
sandreim Mar 30, 2022
1d02b2a
Minor refactor in approval, backing, dispute-coord
sandreim Mar 30, 2022
4719ad3
Working version of suggest garbage candidate
sandreim Mar 30, 2022
519d693
Dedup
sandreim Mar 30, 2022
6724b4a
cleanup #1
sandreim Mar 31, 2022
b6e1bbd
Fix tests
sandreim Apr 1, 2022
1980661
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Apr 1, 2022
5f29b6c
remove debug leftovers
sandreim Apr 1, 2022
ea2b1e5
fmt
sandreim Apr 1, 2022
fca095f
Accidentally commited some local test
sandreim Apr 1, 2022
7d6b472
spellcheck
sandreim Apr 1, 2022
3103ce7
some more fixes
sandreim Apr 4, 2022
bde2403
Refactor and fix it
sandreim Apr 8, 2022
3784972
review feedback
sandreim Apr 8, 2022
991d77f
typo
sandreim Apr 8, 2022
0fa5890
tests review feedback
sandreim Apr 8, 2022
71c35a9
refactor disputer
sandreim Apr 8, 2022
fee4722
fix tests
sandreim Apr 8, 2022
6fb63d2
Fix zombienet disputes test
sandreim Apr 11, 2022
e29433e
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
sandreim Apr 11, 2022
ab22c59
spellcheck
sandreim Apr 11, 2022
f67b522
fix
sandreim Apr 11, 2022
6181157
Fix ui tests
sandreim Apr 11, 2022
155524c
fix typo
sandreim Apr 11, 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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub struct ValidationWorkerCommand {

#[allow(missing_docs)]
#[derive(Debug, Parser)]
#[cfg_attr(feature = "malus", derive(Clone))]
pub struct RunCmd {
#[allow(missing_docs)]
#[clap(flatten)]
Expand Down
2 changes: 1 addition & 1 deletion node/core/approval-voting/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2273,7 +2273,7 @@ async fn launch_approval(
CandidateValidationMessage::ValidateFromExhaustive(
available_data.validation_data,
validation_code,
candidate.descriptor.clone(),
candidate.clone(),
available_data.pov,
APPROVAL_EXECUTION_TIMEOUT,
val_tx,
Expand Down
78 changes: 37 additions & 41 deletions node/core/backing/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use futures::{
};

use polkadot_node_primitives::{
AvailableData, PoV, SignedDisputeStatement, SignedFullStatement, Statement, ValidationResult,
BACKING_EXECUTION_TIMEOUT,
AvailableData, InvalidCandidate, PoV, SignedDisputeStatement, SignedFullStatement, Statement,
ValidationResult, BACKING_EXECUTION_TIMEOUT,
};
use polkadot_node_subsystem_util::{
self as util,
Expand All @@ -41,8 +41,8 @@ use polkadot_node_subsystem_util::{
request_validators, FromJobCommand, JobSender, Validator,
};
use polkadot_primitives::v2::{
BackedCandidate, CandidateCommitments, CandidateDescriptor, CandidateHash, CandidateReceipt,
CollatorId, CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId, SessionIndex,
BackedCandidate, CandidateCommitments, CandidateHash, CandidateReceipt, CollatorId,
CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId, SessionIndex,
SigningContext, ValidatorId, ValidatorIndex, ValidatorSignature, ValidityAttestation,
};
use polkadot_subsystem::{
Expand Down Expand Up @@ -378,14 +378,14 @@ async fn request_pov(

async fn request_candidate_validation(
sender: &mut JobSender<impl SubsystemSender>,
candidate: CandidateDescriptor,
candidate_receipt: CandidateReceipt,
pov: Arc<PoV>,
) -> Result<ValidationResult, Error> {
let (tx, rx) = oneshot::channel();

sender
.send_message(CandidateValidationMessage::ValidateFromChainState(
candidate,
candidate_receipt,
pov,
BACKING_EXECUTION_TIMEOUT,
tx,
Expand Down Expand Up @@ -456,11 +456,9 @@ async fn validate_and_make_available(
.with_pov(&pov)
.with_para_id(candidate.descriptor().para_id)
});
request_candidate_validation(&mut sender, candidate.descriptor.clone(), pov.clone()).await?
request_candidate_validation(&mut sender, candidate.clone(), pov.clone()).await?
};

let expected_commitments_hash = candidate.commitments_hash;

let res = match v {
ValidationResult::Valid(commitments, validation_data) => {
gum::debug!(
Expand All @@ -469,41 +467,39 @@ async fn validate_and_make_available(
"Validation successful",
);

// If validation produces a new set of commitments, we vote the candidate as invalid.
if commitments.hash() != expected_commitments_hash {
sandreim marked this conversation as resolved.
Show resolved Hide resolved
gum::debug!(
target: LOG_TARGET,
candidate_hash = ?candidate.hash(),
actual_commitments = ?commitments,
"Commitments obtained with validation don't match the announced by the candidate receipt",
);
Err(candidate)
} else {
let erasure_valid = make_pov_available(
&mut sender,
n_validators,
pov.clone(),
candidate.hash(),
validation_data,
candidate.descriptor.erasure_root,
span.as_ref(),
)
.await?;
let erasure_valid = make_pov_available(
&mut sender,
n_validators,
pov.clone(),
candidate.hash(),
validation_data,
candidate.descriptor.erasure_root,
span.as_ref(),
)
.await?;

match erasure_valid {
Ok(()) => Ok((candidate, commitments, pov.clone())),
Err(InvalidErasureRoot) => {
gum::debug!(
target: LOG_TARGET,
candidate_hash = ?candidate.hash(),
actual_commitments = ?commitments,
"Erasure root doesn't match the announced by the candidate receipt",
);
Err(candidate)
},
}
match erasure_valid {
Ok(()) => Ok((candidate, commitments, pov.clone())),
Err(InvalidErasureRoot) => {
gum::debug!(
target: LOG_TARGET,
candidate_hash = ?candidate.hash(),
actual_commitments = ?commitments,
"Erasure root doesn't match the announced by the candidate receipt",
);
Err(candidate)
},
}
},
ValidationResult::Invalid(InvalidCandidate::CommitmentsHashMismatch) => {
// If validation produces a new set of commitments, we vote the candidate as invalid.
gum::warn!(
target: LOG_TARGET,
candidate_hash = ?candidate.hash(),
"Validation yielded different commitments",
);
Err(candidate)
},
ValidationResult::Invalid(reason) => {
gum::debug!(
target: LOG_TARGET,
Expand Down
29 changes: 18 additions & 11 deletions node/core/backing/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ use futures::{future, Future};
use polkadot_node_primitives::{BlockData, InvalidCandidate};
use polkadot_node_subsystem_test_helpers as test_helpers;
use polkadot_primitives::v2::{
CollatorId, GroupRotationInfo, HeadData, PersistedValidationData, ScheduledCore,
CandidateDescriptor, CollatorId, GroupRotationInfo, HeadData, PersistedValidationData,
ScheduledCore,
};
use polkadot_subsystem::{
messages::{
Expand Down Expand Up @@ -332,12 +333,12 @@ fn backing_second_works() {
virtual_overseer.recv().await,
AllMessages::CandidateValidation(
CandidateValidationMessage::ValidateFromChainState(
c,
candidate_receipt,
pov,
timeout,
tx,
)
) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && &candidate_receipt.descriptor == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && candidate.commitments.hash() == candidate_receipt.commitments_hash => {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
Expand Down Expand Up @@ -419,6 +420,8 @@ fn backing_works() {
.build();

let candidate_a_hash = candidate_a.hash();
let candidate_a_commitments_hash = candidate_a.commitments.hash();

let public1 = CryptoStore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Expand Down Expand Up @@ -497,7 +500,7 @@ fn backing_works() {
timeout,
tx,
)
) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate_a_commitments_hash=> {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
Expand Down Expand Up @@ -594,6 +597,8 @@ fn backing_works_while_validation_ongoing() {
.build();

let candidate_a_hash = candidate_a.hash();
let candidate_a_commitments_hash = candidate_a.commitments.hash();

let public1 = CryptoStore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Expand Down Expand Up @@ -691,7 +696,7 @@ fn backing_works_while_validation_ongoing() {
timeout,
tx,
)
) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && candidate_a_commitments_hash == c.commitments_hash => {
// we never validate the candidate. our local node
// shouldn't issue any statements.
std::mem::forget(tx);
Expand Down Expand Up @@ -799,6 +804,8 @@ fn backing_misbehavior_works() {
.build();

let candidate_a_hash = candidate_a.hash();
let candidate_a_commitments_hash = candidate_a.commitments.hash();

let public2 = CryptoStore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Expand Down Expand Up @@ -865,7 +872,7 @@ fn backing_misbehavior_works() {
timeout,
tx,
)
) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && candidate_a_commitments_hash == c.commitments_hash => {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
Expand Down Expand Up @@ -1025,7 +1032,7 @@ fn backing_dont_second_invalid() {
timeout,
tx,
)
) if pov == pov && &c == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate_a.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
tx.send(Ok(ValidationResult::Invalid(InvalidCandidate::BadReturn))).unwrap();
}
);
Expand Down Expand Up @@ -1054,7 +1061,7 @@ fn backing_dont_second_invalid() {
timeout,
tx,
)
) if pov == pov && &c == candidate_b.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate_b.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
tx.send(Ok(
ValidationResult::Valid(CandidateCommitments {
head_data: expected_head_data.clone(),
Expand Down Expand Up @@ -1185,7 +1192,7 @@ fn backing_second_after_first_fails_works() {
timeout,
tx,
)
) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate.commitments.hash() => {
tx.send(Ok(ValidationResult::Invalid(InvalidCandidate::BadReturn))).unwrap();
}
);
Expand Down Expand Up @@ -1319,7 +1326,7 @@ fn backing_works_after_failed_validation() {
timeout,
tx,
)
) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT => {
) if pov == pov && c.descriptor() == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate.commitments.hash() => {
tx.send(Err(ValidationFailed("Internal test error".into()))).unwrap();
}
);
Expand Down Expand Up @@ -1696,7 +1703,7 @@ fn retry_works() {
timeout,
_tx,
)
) if pov == pov && &c == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT
) if pov == pov && c.descriptor() == candidate.descriptor() && timeout == BACKING_EXECUTION_TIMEOUT && c.commitments_hash == candidate.commitments.hash()
);
virtual_overseer
});
Expand Down
Loading