Skip to content
GitLab
Unverified Commit 25694f6f authored by Peter Goodspeed-Niklaus's avatar Peter Goodspeed-Niklaus Committed by GitHub
Browse files

misbehavior: report multiple offenses per validator as necessary (#2222) 

* use proper descriptive generic type names

* cleanup

* Table stores a list of detected misbehavior per authority

* add Table::drain_misbehaviors_for

* WIP: unify misbehavior types; report multiple misbehaviors per validator

Code checks, but tests don't yet pass.

* update drain_misbehaviors: return authority id as well as specific misbehavior

* enable unchecked construction of Signed structs in tests

* remove test-features feature & unnecessary generic

* fix backing tests

This took a while to figure out, because where we'd previously been
passing around `SignedFullStatement`s, we now needed to construct
those on the fly within the test, to take advantage of the signature-
checking in the constructor. That, in turn, necessitated changing the
iterable type of `drain_misbehaviors` to return the validator index,
and passing that validator index along within the misbehavior report.

Once that was sorted, however, it became relatively straightforward:
just needed to add appropriate methods to deconstruct the misbehavior
reports, and then we could construct the signed statements directly.

* fix bad merge
parent 3396396e
Pipeline #122176 passed with stages
in 28 minutes and 24 seconds
Expand all Show whitespace changes
Inline Side-by-side
node/core/backing/src/lib.rs
View file @ 25694f6f
......@@ -19,7 +19,6 @@
#![deny(unused_crate_dependencies)]
use std::collections::{HashMap, HashSet};
use std::convert::TryFrom;
use std::pin::Pin;
use std::sync::Arc;
......@@ -28,13 +27,12 @@ use futures::{channel::{mpsc, oneshot}, Future, FutureExt, SinkExt, StreamExt};
use sp_keystore::SyncCryptoStorePtr;
use polkadot_primitives::v1::{
CommittedCandidateReceipt, BackedCandidate, Id as ParaId, ValidatorId,
ValidatorIndex, SigningContext, PoV, CandidateHash,
CandidateDescriptor, AvailableData, ValidatorSignature, Hash, CandidateReceipt,
CoreState, CoreIndex, CollatorId, ValidityAttestation, CandidateCommitments,
AvailableData, BackedCandidate, CandidateCommitments, CandidateDescriptor, CandidateHash,
CandidateReceipt, CollatorId, CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId,
PoV, SigningContext, ValidatorId, ValidatorIndex, ValidatorSignature, ValidityAttestation,
};
use polkadot_node_primitives::{
FromTableMisbehavior, Statement, SignedFullStatement, MisbehaviorReport, ValidationResult,
Statement, SignedFullStatement, ValidationResult,
};
use polkadot_subsystem::{
JaegerSpan, PerLeafSpan,
......@@ -60,8 +58,9 @@ use statement_table::{
Context as TableContextTrait,
Table,
v1::{
SignedStatement as TableSignedStatement,
Statement as TableStatement,
SignedStatement as TableSignedStatement, Summary as TableSummary,
Summary as TableSummary,
},
};
use thiserror::Error;
......@@ -145,8 +144,6 @@ struct CandidateBackingJob {
/// The candidates that are includable, by hash. Each entry here indicates
/// that we've sent the provisioner the backed candidate.
backed: HashSet<CandidateHash>,
/// We have already reported misbehaviors for these validators.
reported_misbehavior_for: HashSet<ValidatorIndex>,
keystore: SyncCryptoStorePtr,
table: Table<TableContext>,
table_context: TableContext,
......@@ -644,36 +641,17 @@ impl CandidateBackingJob {
}
/// Check if there have happened any new misbehaviors and issue necessary messages.
///
/// TODO: Report multiple misbehaviors (https://github.com/paritytech/polkadot/issues/1387)
#[tracing::instrument(level = "trace", skip(self), fields(subsystem = LOG_TARGET))]
async fn issue_new_misbehaviors(&mut self) -> Result<(), Error> {
let mut reports = Vec::new();
for (k, v) in self.table.get_misbehavior().iter() {
if !self.reported_misbehavior_for.contains(k) {
self.reported_misbehavior_for.insert(*k);
let f = FromTableMisbehavior {
id: *k,
report: v.clone(),
signing_context: self.table_context.signing_context.clone(),
key: self.table_context.validators[*k as usize].clone(),
};
if let Ok(report) = MisbehaviorReport::try_from(f) {
let message = ProvisionerMessage::ProvisionableData(
// collect the misbehaviors to avoid double mutable self borrow issues
let misbehaviors: Vec<_> = self.table.drain_misbehaviors().collect();
for (validator_id, report) in misbehaviors {
self.send_to_provisioner(
ProvisionerMessage::ProvisionableData(
self.parent,
ProvisionableData::MisbehaviorReport(self.parent, report),
);
reports.push(message);
}
}
}
for report in reports.drain(..) {
self.send_to_provisioner(report).await?
ProvisionableData::MisbehaviorReport(self.parent, validator_id, report)
)
).await?
}
Ok(())
......@@ -1086,7 +1064,6 @@ impl util::JobTrait for CandidateBackingJob {
seconded: None,
unbacked_candidates: HashMap::new(),
backed: HashSet::new(),
reported_misbehavior_for: HashSet::new(),
keystore,
table: Table::default(),
table_context,
......@@ -1199,9 +1176,7 @@ mod tests {
use super::*;
use assert_matches::assert_matches;
use futures::{future, Future};
use polkadot_primitives::v1::{
ScheduledCore, BlockData, PersistedValidationData, HeadData, GroupRotationInfo,
};
use polkadot_primitives::v1::{BlockData, GroupRotationInfo, HeadData, PersistedValidationData, ScheduledCore};
use polkadot_subsystem::{
messages::{RuntimeApiRequest, RuntimeApiMessage},
ActiveLeavesUpdate, FromOverseer, OverseerSignal,
......@@ -1210,12 +1185,23 @@ mod tests {
use sp_keyring::Sr25519Keyring;
use sp_application_crypto::AppKey;
use sp_keystore::{CryptoStore, SyncCryptoStore};
use statement_table::v1::Misbehavior;
use std::collections::HashMap;
fn validator_pubkeys(val_ids: &[Sr25519Keyring]) -> Vec<ValidatorId> {
val_ids.iter().map(|v| v.public().into()).collect()
}
fn table_statement_to_primitive(
statement: TableStatement,
) -> Statement {
match statement {
TableStatement::Candidate(committed_candidate_receipt) => Statement::Seconded(committed_candidate_receipt),
TableStatement::Valid(candidate_hash) => Statement::Valid(candidate_hash),
TableStatement::Invalid(candidate_hash) => Statement::Invalid(candidate_hash),
}
}
struct TestState {
chain_ids: Vec<ParaId>,
keystore: SyncCryptoStorePtr,
......@@ -1950,19 +1936,30 @@ mod tests {
_,
ProvisionableData::MisbehaviorReport(
relay_parent,
MisbehaviorReport::SelfContradiction(_, s1, s2),
validator_index,
Misbehavior::ValidityDoubleVote(vdv),
)
)
) if relay_parent == test_state.relay_parent => {
s1.check_signature(
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[s1.validator_index() as usize],
).unwrap();
&test_state.validator_public[validator_index as usize],
).expect("signature must be valid");
s2.check_signature(
SignedFullStatement::new(
t2,
validator_index,
s2,
&test_state.signing_context,
&test_state.validator_public[s2.validator_index() as usize],
).unwrap();
&test_state.validator_public[validator_index as usize],
).expect("signature must be valid");
}
);
......@@ -1979,19 +1976,30 @@ mod tests {
_,
ProvisionableData::MisbehaviorReport(
relay_parent,
MisbehaviorReport::SelfContradiction(_, s1, s2),
validator_index,
Misbehavior::ValidityDoubleVote(vdv),
)
)
) if relay_parent == test_state.relay_parent => {
s1.check_signature(
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[s1.validator_index() as usize],
).unwrap();
&test_state.validator_public[validator_index as usize],
).expect("signature must be valid");
s2.check_signature(
SignedFullStatement::new(
t2,
validator_index,
s2,
&test_state.signing_context,
&test_state.validator_public[s2.validator_index() as usize],
).unwrap();
&test_state.validator_public[validator_index as usize],
).expect("signature must be valid");
}
);
});
......@@ -2464,6 +2472,7 @@ mod tests {
#[test]
fn candidate_backing_reorders_votes() {
use sp_core::Encode;
use std::convert::TryFrom;
let relay_parent = [1; 32].into();
let para_id = ParaId::from(10);
......
node/primitives/src/lib.rs
View file @ 25694f6f
......@@ -25,17 +25,9 @@
use futures::Future;
use parity_scale_codec::{Decode, Encode};
use polkadot_primitives::v1::{
Hash, CommittedCandidateReceipt, CandidateReceipt, CompactStatement,
EncodeAs, Signed, SigningContext, ValidatorIndex, ValidatorId,
UpwardMessage, ValidationCode, PersistedValidationData,
HeadData, PoV, CollatorPair, Id as ParaId, OutboundHrmpMessage, CandidateCommitments, CandidateHash,
};
use polkadot_statement_table::{
generic::{
ValidityDoubleVote as TableValidityDoubleVote,
MultipleCandidates as TableMultipleCandidates,
},
v1::Misbehavior as TableMisbehavior,
CandidateCommitments, CandidateHash, CollatorPair, CommittedCandidateReceipt, CompactStatement,
EncodeAs, Hash, HeadData, Id as ParaId, OutboundHrmpMessage, PersistedValidationData, PoV,
Signed, UpwardMessage, ValidationCode,
};
use std::pin::Pin;
......@@ -105,36 +97,6 @@ impl EncodeAs<CompactStatement> for Statement {
/// Only the compact `SignedStatement` is suitable for submission to the chain.
pub type SignedFullStatement = Signed<Statement, CompactStatement>;
/// A misbehaviour report.
#[derive(Debug, Clone)]
pub enum MisbehaviorReport {
/// These validator nodes disagree on this candidate's validity, please figure it out
///
/// Most likely, the list of statments all agree except for the final one. That's not
/// guaranteed, though; if somehow we become aware of lots of
/// statements disagreeing about the validity of a candidate before taking action,
/// this message should be dispatched with all of them, in arbitrary order.
///
/// This variant is also used when our own validity checks disagree with others'.
CandidateValidityDisagreement(CandidateReceipt, Vec<SignedFullStatement>),
/// I've noticed a peer contradicting itself about a particular candidate
SelfContradiction(CandidateReceipt, SignedFullStatement, SignedFullStatement),
/// This peer has seconded more than one parachain candidate for this relay parent head
DoubleVote(SignedFullStatement, SignedFullStatement),
}
/// A utility struct used to convert `TableMisbehavior` to `MisbehaviorReport`s.
pub struct FromTableMisbehavior {
/// Index of the validator.
pub id: ValidatorIndex,
/// The misbehavior reported by the table.
pub report: TableMisbehavior,
/// Signing context.
pub signing_context: SigningContext,
/// Misbehaving validator's public key.
pub key: ValidatorId,
}
/// Candidate invalidity details
#[derive(Debug)]
pub enum InvalidCandidate {
......@@ -170,102 +132,6 @@ pub enum ValidationResult {
Invalid(InvalidCandidate),
}
impl std::convert::TryFrom<FromTableMisbehavior> for MisbehaviorReport {
type Error = ();
fn try_from(f: FromTableMisbehavior) -> Result<Self, Self::Error> {
match f.report {
TableMisbehavior::ValidityDoubleVote(
TableValidityDoubleVote::IssuedAndValidity((c, s1), (d, s2))
) => {
let receipt = c.clone();
let signed_1 = SignedFullStatement::new(
Statement::Seconded(c),
f.id,
s1,
&f.signing_context,
&f.key,
).ok_or(())?;
let signed_2 = SignedFullStatement::new(
Statement::Valid(d),
f.id,
s2,
&f.signing_context,
&f.key,
).ok_or(())?;
Ok(MisbehaviorReport::SelfContradiction(receipt.to_plain(), signed_1, signed_2))
}
TableMisbehavior::ValidityDoubleVote(
TableValidityDoubleVote::IssuedAndInvalidity((c, s1), (d, s2))
) => {
let receipt = c.clone();
let signed_1 = SignedFullStatement::new(
Statement::Seconded(c),
f.id,
s1,
&f.signing_context,
&f.key,
).ok_or(())?;
let signed_2 = SignedFullStatement::new(
Statement::Invalid(d),
f.id,
s2,
&f.signing_context,
&f.key,
).ok_or(())?;
Ok(MisbehaviorReport::SelfContradiction(receipt.to_plain(), signed_1, signed_2))
}
TableMisbehavior::ValidityDoubleVote(
TableValidityDoubleVote::ValidityAndInvalidity(c, s1, s2)
) => {
let signed_1 = SignedFullStatement::new(
Statement::Valid(c.hash()),
f.id,
s1,
&f.signing_context,
&f.key,
).ok_or(())?;
let signed_2 = SignedFullStatement::new(
Statement::Invalid(c.hash()),
f.id,
s2,
&f.signing_context,
&f.key,
).ok_or(())?;
Ok(MisbehaviorReport::SelfContradiction(c.to_plain(), signed_1, signed_2))
}
TableMisbehavior::MultipleCandidates(
TableMultipleCandidates {
first,
second,
}
) => {
let signed_1 = SignedFullStatement::new(
Statement::Seconded(first.0),
f.id,
first.1,
&f.signing_context,
&f.key,
).ok_or(())?;
let signed_2 = SignedFullStatement::new(
Statement::Seconded(second.0),
f.id,
second.1,
&f.signing_context,
&f.key,
).ok_or(())?;
Ok(MisbehaviorReport::DoubleVote(signed_1, signed_2))
}
_ => Err(()),
}
}
}
/// The output of a collator.
///
/// This differs from `CandidateCommitments` in two ways:
......
node/subsystem/src/messages.rs
View file @ 25694f6f
......@@ -28,7 +28,7 @@ use polkadot_node_network_protocol::{
v1 as protocol_v1, NetworkBridgeEvent, ReputationChange, PeerId,
};
use polkadot_node_primitives::{
CollationGenerationConfig, MisbehaviorReport, SignedFullStatement, ValidationResult,
CollationGenerationConfig, SignedFullStatement, ValidationResult,
approval::{BlockApprovalMeta, IndirectAssignmentCert, IndirectSignedApprovalVote},
};
use polkadot_primitives::v1::{
......@@ -41,6 +41,7 @@ use polkadot_primitives::v1::{
ValidatorIndex, ValidatorSignature, InboundDownwardMessage, InboundHrmpMessage,
CandidateIndex,
};
use polkadot_statement_table::v1::Misbehavior;
use std::{sync::Arc, collections::btree_map::BTreeMap};
/// Subsystem messages where each message is always bound to a relay parent.
......@@ -508,7 +509,7 @@ pub enum ProvisionableData {
/// The Candidate Backing subsystem believes that this candidate is valid, pending availability.
BackedCandidate(CandidateReceipt),
/// Misbehavior reports are self-contained proofs of validator misbehavior.
MisbehaviorReport(Hash, MisbehaviorReport),
MisbehaviorReport(Hash, ValidatorIndex, Misbehavior),
/// Disputes trigger a broad dispute resolution process.
Dispute(Hash, ValidatorSignature),
}
......
roadmap/implementers-guide/src/types/overseer-protocol.md
View file @ 25694f6f
......@@ -95,7 +95,8 @@ struct BlockApprovalMeta {
hash: Hash,
/// The number of the block.
number: BlockNumber,
/// The hash of the parent block.
/// The candidates included by the block. Note that these are not the same as the candidates that appear within the
/// block body.
parent_hash: Hash,
/// The candidates included by the block. Note that these are not the same as the candidates that appear within the
/// block body.
......@@ -110,7 +111,9 @@ enum ApprovalDistributionMessage {
NewBlocks(Vec<BlockApprovalMeta>),
/// Distribute an assignment cert from the local validator. The cert is assumed
/// to be valid, relevant, and for the given relay-parent and validator index.
DistributeAssignment(IndirectAssignmentCert, CandidateIndex),
///
/// The `u32` param is the candidate index in the fully-included list.
DistributeAssignment(IndirectAssignmentCert, u32),
/// Distribute an approval vote for the local validator. The approval vote is assumed to be
/// valid, relevant, and the corresponding approval already issued. If not, the subsystem is free to drop
/// the message.
......@@ -173,12 +176,12 @@ enum AvailabilityStoreMessage {
QueryDataAvailability(CandidateHash, ResponseChannel<bool>),
/// Query a specific availability chunk of the candidate's erasure-coding by validator index.
/// Returns the chunk and its inclusion proof against the candidate's erasure-root.
QueryChunk(CandidateHash, ValidatorIndex, ResponseChannel<Option<ErasureChunk>>),
/// Store a specific chunk of the candidate's erasure-coding, with an
QueryChunk(CandidateHash, ValidatorIndex, ResponseChannel<Option<AvailabilityChunkAndProof>>),
/// Store a specific chunk of the candidate's erasure-coding by validator index, with an
/// accompanying proof.
StoreChunk(CandidateHash, ErasureChunk, ResponseChannel<Result<()>>),
StoreChunk(CandidateHash, ValidatorIndex, AvailabilityChunkAndProof, ResponseChannel<Result<()>>),
/// Store `AvailableData`. If `ValidatorIndex` is provided, also store this validator's
/// `ErasureChunk`.
/// `AvailabilityChunkAndProof`.
StoreAvailableData(CandidateHash, Option<ValidatorIndex>, u32, AvailableData, ResponseChannel<Result<()>>),
}
```
......@@ -320,7 +323,7 @@ enum NetworkBridgeMessage {
/// Send a message to one or more peers on the validation peerset.
SendValidationMessage([PeerId], ValidationProtocolV1),
/// Send a message to one or more peers on the collation peerset.
SendCollationMessage([PeerId], CollationProtocolV1),
SendCollationMessage([PeerId], ValidationProtocolV1),
/// Send multiple validation messages.
SendValidationMessages([([PeerId, ValidationProtocolV1])]),
/// Send multiple collation messages.
......@@ -344,25 +347,64 @@ enum NetworkBridgeMessage {
## Misbehavior Report
```rust
enum MisbehaviorReport {
/// These validator nodes disagree on this candidate's validity, please figure it out
///
/// Most likely, the list of statments all agree except for the final one. That's not
/// guaranteed, though; if somehow we become aware of lots of
/// statements disagreeing about the validity of a candidate before taking action,
/// this message should be dispatched with all of them, in arbitrary order.
pub type Misbehavior = generic::Misbehavior<
CommittedCandidateReceipt,
CandidateHash,
ValidatorIndex,
ValidatorSignature,
>;
mod generic {
/// Misbehavior: voting more than one way on candidate validity.
///
/// This variant is also used when our own validity checks disagree with others'.
CandidateValidityDisagreement(CandidateReceipt, Vec<SignedFullStatement>),
/// I've noticed a peer contradicting itself about a particular candidate
SelfContradiction(CandidateReceipt, SignedFullStatement, SignedFullStatement),
/// This peer has seconded more than one parachain candidate for this relay parent head
DoubleVote(CandidateReceipt, SignedFullStatement, SignedFullStatement),
/// Since there are three possible ways to vote, a double vote is possible in
/// three possible combinations (unordered)
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),
}
/// Misbehavior: multiple signatures on same statement.
pub enum DoubleSign<Candidate, Digest, Signature> {
/// On candidate.
Candidate(Candidate, Signature, Signature),
/// On validity.
Validity(Digest, Signature, Signature),
/// On invalidity.
Invalidity(Digest, Signature, Signature),
}
/// Misbehavior: declaring multiple candidates.
pub struct MultipleCandidates<Candidate, Signature> {
/// The first candidate seen.
pub first: (Candidate, Signature),
/// The second candidate seen.
pub second: (Candidate, Signature),
}
/// Misbehavior: submitted statement for wrong group.
pub struct UnauthorizedStatement<Candidate, Digest, AuthorityId, Signature> {
/// A signed statement which was submitted without proper authority.
pub statement: SignedStatement<Candidate, Digest, AuthorityId, Signature>,
}
pub enum Misbehavior<Candidate, Digest, AuthorityId, Signature> {
/// Voted invalid and valid on validity.
ValidityDoubleVote(ValidityDoubleVote<Candidate, Digest, Signature>),
/// Submitted multiple candidates.
MultipleCandidates(MultipleCandidates<Candidate, Signature>),
/// Submitted a message that was unauthorized.
UnauthorizedStatement(UnauthorizedStatement<Candidate, Digest, AuthorityId, Signature>),
/// Submitted two valid signatures for the same message.
DoubleSign(DoubleSign<Candidate, Digest, Signature>),
}
}
```
If this subsystem chooses to second a parachain block, it dispatches a `CandidateBackingSubsystemMessage`.
## PoV Distribution Message
This is a network protocol that receives messages of type [`PoVDistributionV1Message`][PoVDistributionV1NetworkMessage].
......
statement-table/src/generic.rs
View file @ 25694f6f
This diff is collapsed.
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment