diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 546926d175603919886fe3cb0169fe65ae723b44..e783e565fd214b06c6ca9984a6fffb58e8f14fc9 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/node/core/backing/src/lib.rs @@ -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,14 +554,11 @@ 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) - }; - + 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); - self.sign_import_and_distribute_statement(statement, &parent_span).await?; } } } @@ -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, seconded_2.clone()); - let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed_a.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( diff --git a/polkadot/node/network/statement-distribution/src/lib.rs b/polkadot/node/network/statement-distribution/src/lib.rs index ee5cbeb592878f230773e44344c2c04ebeaa3f10..35bdba3e425c6514b49adca7a1f053ba2047bd5f 100644 --- a/polkadot/node/network/statement-distribution/src/lib.rs +++ b/polkadot/node/network/statement-distribution/src/lib.rs @@ -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; } diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index b5d2ab13d39cb879f4a7e002d72deee5152d8ca4..fa723a27312b173c30e7a644f92aaa1b941ae754 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -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), } } } diff --git a/polkadot/primitives/src/v0.rs b/polkadot/primitives/src/v0.rs index d6dfffb1f03115984fea36e7e92bb2aa9ab6ec79..bdd5b1426ead6e69ab25ff80558f36e07b4b53b6 100644 --- a/polkadot/primitives/src/v0.rs +++ b/polkadot/primitives/src/v0.rs @@ -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, } } } diff --git a/polkadot/statement-table/src/generic.rs b/polkadot/statement-table/src/generic.rs index f9a6bce890ca4b9c7d152bad81dd2e8a6f3624bd..7f93c26a7bc671d3ef0633e770c71b913e1490db 100644 --- a/polkadot/statement-table/src/generic.rs +++ b/polkadot/statement-table/src/generic.rs @@ -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 { table.import_statement(&context, vote); assert!(!table.detected_misbehavior.contains_key(&AuthorityId(2))); assert!(table.attested_candidate(&candidate_digest, &context).is_some()); - - // have the last validity guarantor note invalidity. now it is unincludable. - let vote = SignedStatement { - statement: Statement::Invalid(candidate_digest.clone()), - signature: Signature(3), - sender: AuthorityId(3), - }; - - table.import_statement(&context, vote); - assert!(!table.detected_misbehavior.contains_key(&AuthorityId(3))); - assert!(table.attested_candidate(&candidate_digest, &context).is_some()); } #[test] diff --git a/polkadot/statement-table/src/lib.rs b/polkadot/statement-table/src/lib.rs index aa5a9362b1955f42d6e5b8e1a264ef428e4f11f1..327afb497b80f576398d1247e306c07faa33c248 100644 --- a/polkadot/statement-table/src/lib.rs +++ b/polkadot/statement-table/src/lib.rs @@ -52,7 +52,6 @@ pub mod v1 { fn from(s: &'a Statement) -> PrimitiveStatement { match *s { generic::Statement::Valid(s) => PrimitiveStatement::Valid(s), - generic::Statement::Invalid(s) => PrimitiveStatement::Invalid(s), generic::Statement::Seconded(ref s) => PrimitiveStatement::Seconded(s.hash()), } }