Skip to content
GitLab
Unverified Commit dc39e1c7 authored by asynchronous rob's avatar asynchronous rob Committed by GitHub
Browse files

remove statement::invalid (#2597)

parent 378d3831
Pipeline #127689 passed with stages
in 32 minutes and 40 seconds
Show whitespace changes
Inline Side-by-side
node/core/backing/src/lib.rs
View file @ dc39e1c7
......@@ -197,7 +197,6 @@ fn primitive_statement_to_table(s: &SignedFullStatement) -> TableSignedStatement
let statement = match s.payload() {
Statement::Seconded(c) => TableStatement::Seconded(c.clone()),
Statement::Valid(h) => TableStatement::Valid(h.clone()),
Statement::Invalid(h) => TableStatement::Invalid(h.clone()),
};
TableSignedStatement {
......@@ -555,15 +554,12 @@ impl CandidateBackingJob {
ValidatedCandidateCommand::Attest(res) => {
// sanity check.
if !self.issued_statements.contains(&candidate_hash) {
let statement = if res.is_ok() {
Statement::Valid(candidate_hash)
} else {
Statement::Invalid(candidate_hash)
};
self.issued_statements.insert(candidate_hash);
if res.is_ok() {
let statement = Statement::Valid(candidate_hash);
self.sign_import_and_distribute_statement(statement, &parent_span).await?;
}
self.issued_statements.insert(candidate_hash);
}
}
}
......@@ -1241,7 +1237,6 @@ mod tests {
match statement {
TableStatement::Seconded(committed_candidate_receipt) => Statement::Seconded(committed_candidate_receipt),
TableStatement::Valid(candidate_hash) => Statement::Valid(candidate_hash),
TableStatement::Invalid(candidate_hash) => Statement::Invalid(candidate_hash),
}
}
......@@ -1883,15 +1878,11 @@ mod tests {
}.build();
let candidate_a_hash = candidate_a.hash();
let public0 = CryptoStore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID, Some(&test_state.validators[0].to_seed())
).await.expect("Insert key into keystore");
let public2 = CryptoStore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
).await.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
let seconded_2 = SignedFullStatement::sign(
&test_state.keystore,
Statement::Seconded(candidate_a.clone()),
&test_state.signing_context,
......@@ -1899,25 +1890,17 @@ mod tests {
&public2.into(),
).await.ok().flatten().expect("should be signed");
let signed_b = SignedFullStatement::sign(
let valid_2 = SignedFullStatement::sign(
&test_state.keystore,
Statement::Invalid(candidate_a_hash),
Statement::Valid(candidate_a_hash),
&test_state.signing_context,
ValidatorIndex(2),
&public2.into(),
).await.ok().flatten().expect("should be signed");
let signed_c = SignedFullStatement::sign(
&test_state.keystore,
Statement::Invalid(candidate_a_hash),
&test_state.signing_context,
ValidatorIndex(0),
&public0.into(),
).await.ok().flatten().expect("should be signed");
let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed_a.clone());
let statement = CandidateBackingMessage::Statement(test_state.relay_parent, seconded_2.clone());
virtual_overseer.send(FromOverseer::Communication{ msg: statement }).await;
virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
assert_matches!(
virtual_overseer.recv().await,
......@@ -1976,51 +1959,10 @@ mod tests {
}
);
// This `Invalid` statement contradicts the `Candidate` statement
// sent at first.
let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed_b.clone());
// This `Valid` statement is redundant after the `Seconded` statement already sent.
let statement = CandidateBackingMessage::Statement(test_state.relay_parent, valid_2.clone());
virtual_overseer.send(FromOverseer::Communication{ msg: statement }).await;
assert_matches!(
virtual_overseer.recv().await,
AllMessages::Provisioner(
ProvisionerMessage::ProvisionableData(
_,
ProvisionableData::MisbehaviorReport(
relay_parent,
validator_index,
Misbehavior::ValidityDoubleVote(vdv),
)
)
) if relay_parent == test_state.relay_parent => {
let ((t1, s1), (t2, s2)) = vdv.deconstruct::<TableContext>();
let t1 = table_statement_to_primitive(t1);
let t2 = table_statement_to_primitive(t2);
SignedFullStatement::new(
t1,
validator_index,
s1,
&test_state.signing_context,
&test_state.validator_public[validator_index.0 as usize],
).expect("signature must be valid");
SignedFullStatement::new(
t2,
validator_index,
s2,
&test_state.signing_context,
&test_state.validator_public[validator_index.0 as usize],
).expect("signature must be valid");
}
);
// This `Invalid` statement contradicts the `Valid` statement the subsystem
// should have issued behind the scenes.
let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed_c.clone());
virtual_overseer.send(FromOverseer::Communication{ msg: statement }).await;
virtual_overseer.send(FromOverseer::Communication { msg: statement }).await;
assert_matches!(
virtual_overseer.recv().await,
......@@ -2192,7 +2134,7 @@ mod tests {
// Test that if we have already issued a statement (in this case `Invalid`) about a
// candidate we will not be issuing a `Seconded` statement on it.
#[test]
fn backing_multiple_statements_work() {
fn backing_second_after_first_fails_works() {
let test_state = TestState::default();
test_harness(test_state.keystore.clone(), |test_harness| async move {
let TestHarness { mut virtual_overseer } = test_harness;
......@@ -2213,8 +2155,6 @@ mod tests {
..Default::default()
}.build();
let candidate_hash = candidate.hash();
let validator2 = CryptoStore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
......@@ -2262,24 +2202,6 @@ mod tests {
}
);
// The invalid message is shared.
assert_matches!(
virtual_overseer.recv().await,
AllMessages::StatementDistribution(
StatementDistributionMessage::Share(
relay_parent,
signed_statement,
)
) => {
assert_eq!(relay_parent, test_state.relay_parent);
signed_statement.check_signature(
&test_state.signing_context,
&test_state.validator_public[0],
).unwrap();
assert_eq!(*signed_statement.payload(), Statement::Invalid(candidate_hash));
}
);
// Ask subsystem to `Second` a candidate that already has a statement issued about.
// This should emit no actions from subsystem.
let second = CandidateBackingMessage::Second(
......
node/network/statement-distribution/src/lib.rs
View file @ dc39e1c7
......@@ -179,7 +179,7 @@ impl PeerRelayParentKnowledge {
self.known_candidates.insert(h.clone())
},
CompactStatement::Valid(ref h) | CompactStatement::Invalid(ref h) => {
CompactStatement::Valid(ref h) => {
// The peer can only accept Valid and Invalid statements for which it is aware
// of the corresponding candidate.
if !self.known_candidates.contains(h) {
......@@ -235,7 +235,7 @@ impl PeerRelayParentKnowledge {
h
}
CompactStatement::Valid(ref h)| CompactStatement::Invalid(ref h) => {
CompactStatement::Valid(ref h) => {
if !self.known_candidates.contains(&h) {
return Err(COST_UNEXPECTED_STATEMENT);
}
......@@ -454,7 +454,7 @@ impl ActiveHeadData {
NotedStatement::UsefulButKnown
}
}
CompactStatement::Valid(h) | CompactStatement::Invalid(h) => {
CompactStatement::Valid(h) => {
if !self.candidates.contains(&h) {
return NotedStatement::NotUseful;
}
......
node/primitives/src/lib.rs
View file @ dc39e1c7
......@@ -52,9 +52,6 @@ pub enum Statement {
/// A statement that a validator has deemed a candidate valid.
#[codec(index = 2)]
Valid(CandidateHash),
/// A statement that a validator has deemed a candidate invalid.
#[codec(index = 3)]
Invalid(CandidateHash),
}
impl Statement {
......@@ -64,7 +61,7 @@ impl Statement {
/// for large candidates.
pub fn candidate_hash(&self) -> CandidateHash {
match *self {
Statement::Valid(ref h) | Statement::Invalid(ref h) => *h,
Statement::Valid(ref h) => *h,
Statement::Seconded(ref c) => c.hash(),
}
}
......@@ -75,7 +72,6 @@ impl Statement {
match *self {
Statement::Seconded(ref c) => CompactStatement::Seconded(c.hash()),
Statement::Valid(hash) => CompactStatement::Valid(hash),
Statement::Invalid(hash) => CompactStatement::Invalid(hash),
}
}
}
......
primitives/src/v0.rs
View file @ dc39e1c7
......@@ -688,8 +688,6 @@ pub enum CompactStatement {
Seconded(CandidateHash),
/// State that a parachain candidate is valid.
Valid(CandidateHash),
/// State that a parachain candidate is invalid.
Invalid(CandidateHash),
}
// Inner helper for codec on `CompactStatement`.
......@@ -699,8 +697,6 @@ enum CompactStatementInner {
Seconded(CandidateHash),
#[codec(index = 2)]
Valid(CandidateHash),
#[codec(index = 3)]
Invalid(CandidateHash),
}
impl From<CompactStatement> for CompactStatementInner {
......@@ -708,7 +704,6 @@ impl From<CompactStatement> for CompactStatementInner {
match s {
CompactStatement::Seconded(h) => CompactStatementInner::Seconded(h),
CompactStatement::Valid(h) => CompactStatementInner::Valid(h),
CompactStatement::Invalid(h) => CompactStatementInner::Invalid(h),
}
}
}
......@@ -735,7 +730,6 @@ impl parity_scale_codec::Decode for CompactStatement {
Ok(match CompactStatementInner::decode(input)? {
CompactStatementInner::Seconded(h) => CompactStatement::Seconded(h),
CompactStatementInner::Valid(h) => CompactStatement::Valid(h),
CompactStatementInner::Invalid(h) => CompactStatement::Invalid(h),
})
}
}
......@@ -744,10 +738,7 @@ impl CompactStatement {
/// Get the underlying candidate hash this references.
pub fn candidate_hash(&self) -> &CandidateHash {
match *self {
CompactStatement::Seconded(ref h)
| CompactStatement::Valid(ref h)
| CompactStatement::Invalid(ref h)
=> h
CompactStatement::Seconded(ref h) | CompactStatement::Valid(ref h) => h,
}
}
}
......
statement-table/src/generic.rs
View file @ dc39e1c7
......@@ -70,9 +70,6 @@ pub enum Statement<Candidate, Digest> {
/// Broadcast by a authority to attest that the candidate with given digest is valid.
#[codec(index = 2)]
Valid(Digest),
/// Broadcast by a authority to attest that the candidate with given digest is invalid.
#[codec(index = 3)]
Invalid(Digest),
}
/// A signed statement.
......@@ -94,10 +91,6 @@ pub struct SignedStatement<Candidate, Digest, AuthorityId, Signature> {
pub enum ValidityDoubleVote<Candidate, Digest, Signature> {
/// Implicit vote by issuing and explicitly voting validity.
IssuedAndValidity((Candidate, Signature), (Digest, Signature)),
/// Implicit vote by issuing and explicitly voting invalidity
IssuedAndInvalidity((Candidate, Signature), (Digest, Signature)),
/// Direct votes for validity and invalidity
ValidityAndInvalidity(Candidate, Signature, Signature),
}
impl<Candidate, Digest, Signature> ValidityDoubleVote<Candidate, Digest, Signature> {
......@@ -117,15 +110,6 @@ impl<Candidate, Digest, Signature> ValidityDoubleVote<Candidate, Digest, Signatu
Self::IssuedAndValidity((c, s1), (d, s2)) => {
((Statement::Seconded(c), s1), (Statement::Valid(d), s2))
}
Self::IssuedAndInvalidity((c, s1), (d, s2)) => {
((Statement::Seconded(c), s1), (Statement::Invalid(d), s2))
}
Self::ValidityAndInvalidity(c, s1, s2) => {
(
(Statement::Valid(Ctx::candidate_digest(&c)), s1),
(Statement::Invalid(Ctx::candidate_digest(&c)), s2),
)
}
}
}
}
......@@ -137,8 +121,6 @@ pub enum DoubleSign<Candidate, Digest, Signature> {
Seconded(Candidate, Signature, Signature),
/// On validity.
Validity(Digest, Signature, Signature),
/// On invalidity.
Invalidity(Digest, Signature, Signature),
}
impl<Candidate, Digest, Signature> DoubleSign<Candidate, Digest, Signature> {
......@@ -148,7 +130,6 @@ impl<Candidate, Digest, Signature> DoubleSign<Candidate, Digest, Signature> {
match self {
Self::Seconded(candidate, a, b) => (Statement::Seconded(candidate), a, b),
Self::Validity(digest, a, b) => (Statement::Valid(digest), a, b),
Self::Invalidity(digest, a, b) => (Statement::Invalid(digest), a, b),
}
}
}
......@@ -198,8 +179,6 @@ enum ValidityVote<Signature: Eq + Clone> {
Issued(Signature),
// direct validity vote
Valid(Signature),
// direct invalidity vote
Invalid(Signature),
}
/// A summary of import of a statement.
......@@ -211,8 +190,6 @@ pub struct Summary<Digest, Group> {
pub group_id: Group,
/// How many validity votes are currently witnessed.
pub validity_votes: usize,
/// Whether this has been signalled bad by at least one participant.
pub signalled_bad: bool,
}
/// A validity attestation.
......@@ -251,15 +228,9 @@ pub struct CandidateData<Ctx: Context> {
group_id: Ctx::GroupId,
candidate: Ctx::Candidate,
validity_votes: HashMap<Ctx::AuthorityId, ValidityVote<Ctx::Signature>>,
indicated_bad_by: Vec<Ctx::AuthorityId>,
}
impl<Ctx: Context> CandidateData<Ctx> {
/// whether this has been indicated bad by anyone.
pub fn indicated_bad(&self) -> bool {
!self.indicated_bad_by.is_empty()
}
/// Yield a full attestation for a candidate.
/// If the candidate can be included, it will return `Some`.
pub fn attested(&self, validity_threshold: usize)
......@@ -267,18 +238,17 @@ impl<Ctx: Context> CandidateData<Ctx> {
Ctx::GroupId, Ctx::Candidate, Ctx::AuthorityId, Ctx::Signature,
>>
{
let valid_votes = self.validity_votes.len().saturating_sub(self.indicated_bad_by.len());
let valid_votes = self.validity_votes.len();
if valid_votes < validity_threshold {
return None;
}
let validity_votes = self.validity_votes.iter()
.filter_map(|(a, v)| match *v {
ValidityVote::Invalid(_) => None,
.map(|(a, v)| match *v {
ValidityVote::Valid(ref s) =>
Some((a.clone(), ValidityAttestation::Explicit(s.clone()))),
(a.clone(), ValidityAttestation::Explicit(s.clone())),
ValidityVote::Issued(ref s) =>
Some((a.clone(), ValidityAttestation::Implicit(s.clone()))),
(a.clone(), ValidityAttestation::Implicit(s.clone())),
})
.collect();
......@@ -294,7 +264,6 @@ impl<Ctx: Context> CandidateData<Ctx> {
candidate: digest,
group_id: self.group_id.clone(),
validity_votes: self.validity_votes.len(),
signalled_bad: self.indicated_bad(),
}
}
}
......@@ -377,12 +346,6 @@ impl<Ctx: Context> Table<Ctx> {
digest,
ValidityVote::Valid(signature),
),
Statement::Invalid(digest) => self.validity_vote(
context,
signer.clone(),
digest,
ValidityVote::Invalid(signature),
),
};
match res {
......@@ -483,7 +446,6 @@ impl<Ctx: Context> Table<Ctx> {
group_id: group,
candidate,
validity_votes: HashMap::new(),
indicated_bad_by: Vec::new(),
});
}
......@@ -509,9 +471,8 @@ impl<Ctx: Context> Table<Ctx> {
// check that this authority actually can vote in this group.
if !context.is_member_of(&from, &votes.group_id) {
let (sig, valid) = match vote {
ValidityVote::Valid(s) => (s, true),
ValidityVote::Invalid(s) => (s, false),
let sig = match vote {
ValidityVote::Valid(s) => s,
ValidityVote::Issued(_) =>
panic!("implicit issuance vote only cast from `import_candidate` after \
checking group membership of issuer; qed"),
......@@ -521,11 +482,7 @@ impl<Ctx: Context> Table<Ctx> {
statement: SignedStatement {
signature: sig,
sender: from,
statement: if valid {
Statement::Valid(digest)
} else {
Statement::Invalid(digest)
}
statement: Statement::Valid(digest),
}
}));
}
......@@ -542,16 +499,6 @@ impl<Ctx: Context> Table<Ctx> {
(ValidityVote::Valid(good), ValidityVote::Issued(iss)) =>
make_vdv(ValidityDoubleVote::IssuedAndValidity((votes.candidate.clone(), iss), (digest, good))),
// invalid vote conflicting with candidate statement
(ValidityVote::Issued(iss), ValidityVote::Invalid(bad)) |
(ValidityVote::Invalid(bad), ValidityVote::Issued(iss)) =>
make_vdv(ValidityDoubleVote::IssuedAndInvalidity((votes.candidate.clone(), iss), (digest, bad))),
// valid vote conflicting with invalid vote
(ValidityVote::Valid(good), ValidityVote::Invalid(bad)) |
(ValidityVote::Invalid(bad), ValidityVote::Valid(good)) =>
make_vdv(ValidityDoubleVote::ValidityAndInvalidity(votes.candidate.clone(), good, bad)),
// two signatures on same candidate
(ValidityVote::Issued(a), ValidityVote::Issued(b)) =>
make_ds(DoubleSign::Seconded(votes.candidate.clone(), a, b)),
......@@ -559,20 +506,12 @@ impl<Ctx: Context> Table<Ctx> {
// two signatures on same validity vote
(ValidityVote::Valid(a), ValidityVote::Valid(b)) =>
make_ds(DoubleSign::Validity(digest, a, b)),
// two signature on same invalidity vote
(ValidityVote::Invalid(a), ValidityVote::Invalid(b)) =>
make_ds(DoubleSign::Invalidity(digest, a, b)),
})
} else {
Ok(None)
}
}
Entry::Vacant(vacant) => {
if let ValidityVote::Invalid(_) = vote {
votes.indicated_bad_by.push(from.clone());
}
vacant.insert(vote);
}
}
......@@ -816,55 +755,6 @@ mod tests {
);
}
#[test]
fn validity_double_vote_is_misbehavior() {
let context = TestContext {
authorities: {
let mut map = HashMap::new();
map.insert(AuthorityId(1), GroupId(2));
map.insert(AuthorityId(2), GroupId(2));
map
}
};
let mut table = create();
let statement = SignedStatement {
statement: Statement::Seconded(Candidate(2, 100)),
signature: Signature(1),
sender: AuthorityId(1),
};
let candidate_digest = Digest(100);
table.import_statement(&context, statement);
assert!(!table.detected_misbehavior.contains_key(&AuthorityId(1)));
let valid_statement = SignedStatement {
statement: Statement::Valid(candidate_digest.clone()),
signature: Signature(2),
sender: AuthorityId(2),
};
let invalid_statement = SignedStatement {
statement: Statement::Invalid(candidate_digest.clone()),
signature: Signature(2),
sender: AuthorityId(2),
};
table.import_statement(&context, valid_statement);
assert!(!table.detected_misbehavior.contains_key(&AuthorityId(2)));
table.import_statement(&context, invalid_statement);
assert_eq!(
table.detected_misbehavior[&AuthorityId(2)][0],
Misbehavior::ValidityDoubleVote(ValidityDoubleVote::ValidityAndInvalidity(
Candidate(2, 100),
Signature(2),
Signature(2),
))
);
}
#[test]
fn candidate_double_signature_is_misbehavior() {
let context = TestContext {
......@@ -896,71 +786,6 @@ mod tests {
assert!(table.detected_misbehavior.contains_key(&AuthorityId(1)));
}
#[test]
fn validity_invalidity_double_signature_is_misbehavior() {
let context = TestContext {
authorities: {
let mut map = HashMap::new();
map.insert(AuthorityId(1), GroupId(2));
map.insert(AuthorityId(2), GroupId(2));
map.insert(AuthorityId(3), GroupId(2));
map
}
};
let mut table = create();
let statement = SignedStatement {
statement: Statement::Seconded(Candidate(2, 100)),
signature: Signature(1),
sender: AuthorityId(1),
};
table.import_statement(&context, statement);
assert!(!table.detected_misbehavior.contains_key(&AuthorityId(1)));
// insert two validity votes from authority 2 with different signatures
{
let statement = SignedStatement {
statement: Statement::Valid(Digest(100)),
signature: Signature(2),
sender: AuthorityId(2),
};
table.import_statement(&context, statement);
assert!(!table.detected_misbehavior.contains_key(&AuthorityId(2)));
let invalid_statement = SignedStatement {
statement: Statement::Valid(Digest(100)),
signature: Signature(222),
sender: AuthorityId(2),
};
table.import_statement(&context, invalid_statement);
assert!(table.detected_misbehavior.contains_key(&AuthorityId(2)));
}
// insert two invalidity votes from authority 2 with different signatures
{
let statement = SignedStatement {
statement: Statement::Invalid(Digest(100)),
signature: Signature(3),
sender: AuthorityId(3),
};
table.import_statement(&context, statement);
assert!(!table.detected_misbehavior.contains_key(&AuthorityId(3)));
let invalid_statement = SignedStatement {
statement: Statement::Invalid(Digest(100)),
signature: Signature(333),
sender: AuthorityId(3),
};
table.import_statement(&context, invalid_statement);
assert!(table.detected_misbehavior.contains_key(&AuthorityId(3)));
}
}
#[test]
fn issue_and_vote_is_misbehavior() {
let context = TestContext {
......@@ -1006,7 +831,6 @@ mod tests {
group_id: GroupId(4),
candidate: Candidate(4, 12345),
validity_votes: HashMap::new(),
indicated_bad_by: Vec::new(),
};
assert!(candidate.attested(validity_threshold).is_none());
......@@ -1017,7 +841,6 @@ mod tests {
assert!(candidate.attested(validity_threshold).is_some());
candidate.indicated_bad_by.push(AuthorityId(1024));
candidate.validity_votes.insert(
AuthorityId(validity_threshold + 100),
ValidityVote::Valid(Signature(validity_threshold + 100)),
......@@ -1061,17 +884,6 @@ mod tests {