Skip to content

Commit

Permalink
BEEFY: implement equivocations detection, reporting and slashing (par…
Browse files Browse the repository at this point in the history
…itytech#13121)

* client/beefy: simplify self_vote logic

* client/beefy: migrate to new state version

* client/beefy: detect equivocated votes

* fix typos

* sp-beefy: add equivocation primitives

* client/beefy: refactor vote processing

* fix version migration for new rounds struct

* client/beefy: track equivocations and create proofs

* client/beefy: adjust tests for new voting logic

* sp-beefy: fix commitment ordering and equality

* client/beefy: simplify handle_vote() a bit

* client/beefy: add simple equivocation test

* client/beefy: submit equivocation proof - WIP

* frame/beefy: add equivocation report runtime api - part 1

* frame/beefy: report equivocation logic - part 2

* frame/beefy: add pluggable Equivocation handler - part 3

* frame/beefy: impl ValidateUnsigned for equivocations reporting

* client/beefy: submit report equivocation unsigned extrinsic

* primitives/beefy: fix tests

* frame/beefy: add default weights

* frame/beefy: fix tests

* client/beefy: fix tests

* frame/beefy-mmr: fix tests

* frame/beefy: cross-check session index with equivocation report

* sp-beefy: make test Keyring useable in pallet

* frame/beefy: add basic equivocation test

* frame/beefy: test verify equivocation results in slashing

* frame/beefy: test report_equivocation_old_set

* frame/beefy: add more equivocation tests

* sp-beefy: fix docs

* beefy: simplify equivocations and fix tests

* client/beefy: address review comments

* frame/beefy: add ValidateUnsigned to test/mock runtime

* client/beefy: fixes after merge master

* fix missed merge damage

* client/beefy: add test for reporting equivocations

Also validated there's no unexpected equivocations reported in the
other tests.

Signed-off-by: acatangiu <[email protected]>

* sp-beefy: move test utils to their own file

* client/beefy: add negative test for equivocation reports

* sp-beefy: move back MmrRootProvider - used in polkadot-service

* impl review suggestions

* client/beefy: add equivocation metrics

---------

Signed-off-by: acatangiu <[email protected]>
Co-authored-by: parity-processbot <>
  • Loading branch information
acatangiu authored Feb 17, 2023
1 parent d97a188 commit 63a2458
Show file tree
Hide file tree
Showing 22 changed files with 2,140 additions and 213 deletions.
8 changes: 8 additions & 0 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions client/beefy/src/communication/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ mod tests {

use crate::keystore::BeefyKeystore;
use beefy_primitives::{
crypto::Signature, keyring::Keyring, known_payloads, Commitment, MmrRootHash, Payload,
VoteMessage, KEY_TYPE,
crypto::Signature, known_payloads, Commitment, Keyring, MmrRootHash, Payload, VoteMessage,
KEY_TYPE,
};

use super::*;
Expand Down
18 changes: 17 additions & 1 deletion client/beefy/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,30 @@

use std::fmt::Debug;

#[derive(Debug, thiserror::Error, PartialEq)]
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("Backend: {0}")]
Backend(String),
#[error("Keystore error: {0}")]
Keystore(String),
#[error("Runtime api error: {0}")]
RuntimeApi(sp_api::ApiError),
#[error("Signature error: {0}")]
Signature(String),
#[error("Session uninitialized")]
UninitSession,
}

#[cfg(test)]
impl PartialEq for Error {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Error::Backend(s1), Error::Backend(s2)) => s1 == s2,
(Error::Keystore(s1), Error::Keystore(s2)) => s1 == s2,
(Error::RuntimeApi(_), Error::RuntimeApi(_)) => true,
(Error::Signature(s1), Error::Signature(s2)) => s1 == s2,
(Error::UninitSession, Error::UninitSession) => true,
_ => false,
}
}
}
3 changes: 1 addition & 2 deletions client/beefy/src/justification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ fn verify_with_validator_set<Block: BlockT>(
#[cfg(test)]
pub(crate) mod tests {
use beefy_primitives::{
keyring::Keyring, known_payloads, Commitment, Payload, SignedCommitment,
VersionedFinalityProof,
known_payloads, Commitment, Keyring, Payload, SignedCommitment, VersionedFinalityProof,
};
use substrate_test_runtime_client::runtime::Block;

Expand Down
2 changes: 1 addition & 1 deletion client/beefy/src/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub mod tests {
use sc_keystore::LocalKeystore;
use sp_core::{ecdsa, Pair};

use beefy_primitives::{crypto, keyring::Keyring};
use beefy_primitives::{crypto, Keyring};

use super::*;
use crate::error::Error;
Expand Down
3 changes: 2 additions & 1 deletion client/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ where
let worker_params = worker::WorkerParams {
backend,
payload_provider,
runtime,
network,
key_store: key_store.into(),
gossip_engine,
Expand All @@ -292,7 +293,7 @@ where
persisted_state,
};

let worker = worker::BeefyWorker::<_, _, _, _>::new(worker_params);
let worker = worker::BeefyWorker::<_, _, _, _, _>::new(worker_params);

futures::future::join(
worker.run(block_import_justif, finality_notifications),
Expand Down
47 changes: 31 additions & 16 deletions client/beefy/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,16 @@ pub struct VoterMetrics {
pub beefy_no_authority_found_in_store: Counter<U64>,
/// Number of currently buffered votes
pub beefy_buffered_votes: Gauge<U64>,
/// Number of valid but stale votes received
pub beefy_stale_votes: Counter<U64>,
/// Number of votes dropped due to full buffers
pub beefy_buffered_votes_dropped: Counter<U64>,
/// Number of good votes successfully handled
pub beefy_good_votes_processed: Counter<U64>,
/// Number of equivocation votes received
pub beefy_equivocation_votes: Counter<U64>,
/// Number of invalid votes received
pub beefy_invalid_votes: Counter<U64>,
/// Number of valid but stale votes received
pub beefy_stale_votes: Counter<U64>,
/// Number of currently buffered justifications
pub beefy_buffered_justifications: Gauge<U64>,
/// Number of valid but stale justifications received
Expand All @@ -60,8 +66,6 @@ pub struct VoterMetrics {
pub beefy_buffered_justifications_dropped: Counter<U64>,
/// Trying to set Best Beefy block to old block
pub beefy_best_block_set_last_failure: Gauge<U64>,
/// Number of Successful handled votes
pub beefy_successful_handled_votes: Counter<U64>,
}

impl PrometheusRegister for VoterMetrics {
Expand Down Expand Up @@ -109,17 +113,35 @@ impl PrometheusRegister for VoterMetrics {
Gauge::new("substrate_beefy_buffered_votes", "Number of currently buffered votes")?,
registry,
)?,
beefy_stale_votes: register(
beefy_buffered_votes_dropped: register(
Counter::new(
"substrate_beefy_buffered_votes_dropped",
"Number of votes dropped due to full buffers",
)?,
registry,
)?,
beefy_good_votes_processed: register(
Counter::new(
"substrate_beefy_successful_handled_votes",
"Number of good votes successfully handled",
)?,
registry,
)?,
beefy_equivocation_votes: register(
Counter::new(
"substrate_beefy_stale_votes",
"Number of valid but stale votes received",
"Number of equivocation votes received",
)?,
registry,
)?,
beefy_buffered_votes_dropped: register(
beefy_invalid_votes: register(
Counter::new("substrate_beefy_stale_votes", "Number of invalid votes received")?,
registry,
)?,
beefy_stale_votes: register(
Counter::new(
"substrate_beefy_buffered_votes_dropped",
"Number of votes dropped due to full buffers",
"substrate_beefy_stale_votes",
"Number of valid but stale votes received",
)?,
registry,
)?,
Expand Down Expand Up @@ -158,13 +180,6 @@ impl PrometheusRegister for VoterMetrics {
)?,
registry,
)?,
beefy_successful_handled_votes: register(
Counter::new(
"substrate_beefy_successful_handled_votes",
"Number of Successful handled votes",
)?,
registry,
)?,
})
}
}
Expand Down
52 changes: 46 additions & 6 deletions client/beefy/src/round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::LOG_TARGET;

use beefy_primitives::{
crypto::{AuthorityId, Public, Signature},
Commitment, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage,
Commitment, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage,
};
use codec::{Decode, Encode};
use log::debug;
Expand Down Expand Up @@ -61,7 +61,7 @@ pub fn threshold(authorities: usize) -> usize {
pub enum VoteImportResult<B: Block> {
Ok,
RoundConcluded(SignedCommitment<NumberFor<B>, Signature>),
Equivocation, /* TODO: (EquivocationProof<NumberFor<B>, Public, Signature>) */
Equivocation(EquivocationProof<NumberFor<B>, Public, Signature>),
Invalid,
Stale,
}
Expand Down Expand Up @@ -149,8 +149,10 @@ where
target: LOG_TARGET,
"🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", previous_vote, vote
);
// TODO: build `EquivocationProof` and return it here.
return VoteImportResult::Equivocation
return VoteImportResult::Equivocation(EquivocationProof {
first: previous_vote.clone(),
second: vote,
})
}
} else {
// this is the first vote sent by `id` for `num`, all good
Expand Down Expand Up @@ -197,8 +199,8 @@ mod tests {
use sc_network_test::Block;

use beefy_primitives::{
crypto::Public, keyring::Keyring, known_payloads::MMR_ROOT_ID, Commitment, Payload,
SignedCommitment, ValidatorSet, VoteMessage,
crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, Keyring,
Payload, SignedCommitment, ValidatorSet, VoteMessage,
};

use super::{threshold, Block as BlockT, RoundTracker, Rounds};
Expand Down Expand Up @@ -452,4 +454,42 @@ mod tests {
rounds.conclude(3);
assert!(rounds.previous_votes.is_empty());
}

#[test]
fn should_provide_equivocation_proof() {
sp_tracing::try_init_simple();

let validators = ValidatorSet::<Public>::new(
vec![Keyring::Alice.public(), Keyring::Bob.public()],
Default::default(),
)
.unwrap();
let validator_set_id = validators.id();
let session_start = 1u64.into();
let mut rounds = Rounds::<Block>::new(session_start, validators);

let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![1, 1, 1, 1]);
let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![2, 2, 2, 2]);
let commitment1 = Commitment { block_number: 1, payload: payload1, validator_set_id };
let commitment2 = Commitment { block_number: 1, payload: payload2, validator_set_id };

let alice_vote1 = VoteMessage {
id: Keyring::Alice.public(),
commitment: commitment1,
signature: Keyring::Alice.sign(b"I am committed"),
};
let mut alice_vote2 = alice_vote1.clone();
alice_vote2.commitment = commitment2;

let expected_result = VoteImportResult::Equivocation(EquivocationProof {
first: alice_vote1.clone(),
second: alice_vote2.clone(),
});

// vote on one payload - ok
assert_eq!(rounds.add_vote(alice_vote1), VoteImportResult::Ok);

// vote on _another_ commitment/payload -> expected equivocation proof
assert_eq!(rounds.add_vote(alice_vote2), expected_result);
}
}
Loading

0 comments on commit 63a2458

Please sign in to comment.