Skip to content

Commit f351629

Browse files
acatangiunathanwhit
authored andcommitted
BEEFY: implement equivocations detection, reporting and slashing (paritytech#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 <adrian@parity.io> * 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 <adrian@parity.io> Co-authored-by: parity-processbot <>
1 parent ea71d2e commit f351629

File tree

22 files changed

+2140
-213
lines changed

22 files changed

+2140
-213
lines changed

Cargo.lock

+8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

client/beefy/src/communication/gossip.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,8 @@ mod tests {
244244

245245
use crate::keystore::BeefyKeystore;
246246
use beefy_primitives::{
247-
crypto::Signature, keyring::Keyring, known_payloads, Commitment, MmrRootHash, Payload,
248-
VoteMessage, KEY_TYPE,
247+
crypto::Signature, known_payloads, Commitment, Keyring, MmrRootHash, Payload, VoteMessage,
248+
KEY_TYPE,
249249
};
250250

251251
use super::*;

client/beefy/src/error.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,30 @@
2222
2323
use std::fmt::Debug;
2424

25-
#[derive(Debug, thiserror::Error, PartialEq)]
25+
#[derive(Debug, thiserror::Error)]
2626
pub enum Error {
2727
#[error("Backend: {0}")]
2828
Backend(String),
2929
#[error("Keystore error: {0}")]
3030
Keystore(String),
31+
#[error("Runtime api error: {0}")]
32+
RuntimeApi(sp_api::ApiError),
3133
#[error("Signature error: {0}")]
3234
Signature(String),
3335
#[error("Session uninitialized")]
3436
UninitSession,
3537
}
38+
39+
#[cfg(test)]
40+
impl PartialEq for Error {
41+
fn eq(&self, other: &Self) -> bool {
42+
match (self, other) {
43+
(Error::Backend(s1), Error::Backend(s2)) => s1 == s2,
44+
(Error::Keystore(s1), Error::Keystore(s2)) => s1 == s2,
45+
(Error::RuntimeApi(_), Error::RuntimeApi(_)) => true,
46+
(Error::Signature(s1), Error::Signature(s2)) => s1 == s2,
47+
(Error::UninitSession, Error::UninitSession) => true,
48+
_ => false,
49+
}
50+
}
51+
}

client/beefy/src/justification.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ fn verify_with_validator_set<Block: BlockT>(
8181
#[cfg(test)]
8282
pub(crate) mod tests {
8383
use beefy_primitives::{
84-
keyring::Keyring, known_payloads, Commitment, Payload, SignedCommitment,
85-
VersionedFinalityProof,
84+
known_payloads, Commitment, Keyring, Payload, SignedCommitment, VersionedFinalityProof,
8685
};
8786
use substrate_test_runtime_client::runtime::Block;
8887

client/beefy/src/keystore.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ pub mod tests {
123123
use sc_keystore::LocalKeystore;
124124
use sp_core::{ecdsa, Pair};
125125

126-
use beefy_primitives::{crypto, keyring::Keyring};
126+
use beefy_primitives::{crypto, Keyring};
127127

128128
use super::*;
129129
use crate::error::Error;

client/beefy/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ where
282282
let worker_params = worker::WorkerParams {
283283
backend,
284284
payload_provider,
285+
runtime,
285286
network,
286287
key_store: key_store.into(),
287288
gossip_engine,
@@ -292,7 +293,7 @@ where
292293
persisted_state,
293294
};
294295

295-
let worker = worker::BeefyWorker::<_, _, _, _>::new(worker_params);
296+
let worker = worker::BeefyWorker::<_, _, _, _, _>::new(worker_params);
296297

297298
futures::future::join(
298299
worker.run(block_import_justif, finality_notifications),

client/beefy/src/metrics.rs

+31-16
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,16 @@ pub struct VoterMetrics {
4646
pub beefy_no_authority_found_in_store: Counter<U64>,
4747
/// Number of currently buffered votes
4848
pub beefy_buffered_votes: Gauge<U64>,
49-
/// Number of valid but stale votes received
50-
pub beefy_stale_votes: Counter<U64>,
5149
/// Number of votes dropped due to full buffers
5250
pub beefy_buffered_votes_dropped: Counter<U64>,
51+
/// Number of good votes successfully handled
52+
pub beefy_good_votes_processed: Counter<U64>,
53+
/// Number of equivocation votes received
54+
pub beefy_equivocation_votes: Counter<U64>,
55+
/// Number of invalid votes received
56+
pub beefy_invalid_votes: Counter<U64>,
57+
/// Number of valid but stale votes received
58+
pub beefy_stale_votes: Counter<U64>,
5359
/// Number of currently buffered justifications
5460
pub beefy_buffered_justifications: Gauge<U64>,
5561
/// Number of valid but stale justifications received
@@ -60,8 +66,6 @@ pub struct VoterMetrics {
6066
pub beefy_buffered_justifications_dropped: Counter<U64>,
6167
/// Trying to set Best Beefy block to old block
6268
pub beefy_best_block_set_last_failure: Gauge<U64>,
63-
/// Number of Successful handled votes
64-
pub beefy_successful_handled_votes: Counter<U64>,
6569
}
6670

6771
impl PrometheusRegister for VoterMetrics {
@@ -109,17 +113,35 @@ impl PrometheusRegister for VoterMetrics {
109113
Gauge::new("substrate_beefy_buffered_votes", "Number of currently buffered votes")?,
110114
registry,
111115
)?,
112-
beefy_stale_votes: register(
116+
beefy_buffered_votes_dropped: register(
117+
Counter::new(
118+
"substrate_beefy_buffered_votes_dropped",
119+
"Number of votes dropped due to full buffers",
120+
)?,
121+
registry,
122+
)?,
123+
beefy_good_votes_processed: register(
124+
Counter::new(
125+
"substrate_beefy_successful_handled_votes",
126+
"Number of good votes successfully handled",
127+
)?,
128+
registry,
129+
)?,
130+
beefy_equivocation_votes: register(
113131
Counter::new(
114132
"substrate_beefy_stale_votes",
115-
"Number of valid but stale votes received",
133+
"Number of equivocation votes received",
116134
)?,
117135
registry,
118136
)?,
119-
beefy_buffered_votes_dropped: register(
137+
beefy_invalid_votes: register(
138+
Counter::new("substrate_beefy_stale_votes", "Number of invalid votes received")?,
139+
registry,
140+
)?,
141+
beefy_stale_votes: register(
120142
Counter::new(
121-
"substrate_beefy_buffered_votes_dropped",
122-
"Number of votes dropped due to full buffers",
143+
"substrate_beefy_stale_votes",
144+
"Number of valid but stale votes received",
123145
)?,
124146
registry,
125147
)?,
@@ -158,13 +180,6 @@ impl PrometheusRegister for VoterMetrics {
158180
)?,
159181
registry,
160182
)?,
161-
beefy_successful_handled_votes: register(
162-
Counter::new(
163-
"substrate_beefy_successful_handled_votes",
164-
"Number of Successful handled votes",
165-
)?,
166-
registry,
167-
)?,
168183
})
169184
}
170185
}

client/beefy/src/round.rs

+46-6
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::LOG_TARGET;
2020

2121
use beefy_primitives::{
2222
crypto::{AuthorityId, Public, Signature},
23-
Commitment, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage,
23+
Commitment, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage,
2424
};
2525
use codec::{Decode, Encode};
2626
use log::debug;
@@ -61,7 +61,7 @@ pub fn threshold(authorities: usize) -> usize {
6161
pub enum VoteImportResult<B: Block> {
6262
Ok,
6363
RoundConcluded(SignedCommitment<NumberFor<B>, Signature>),
64-
Equivocation, /* TODO: (EquivocationProof<NumberFor<B>, Public, Signature>) */
64+
Equivocation(EquivocationProof<NumberFor<B>, Public, Signature>),
6565
Invalid,
6666
Stale,
6767
}
@@ -149,8 +149,10 @@ where
149149
target: LOG_TARGET,
150150
"🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", previous_vote, vote
151151
);
152-
// TODO: build `EquivocationProof` and return it here.
153-
return VoteImportResult::Equivocation
152+
return VoteImportResult::Equivocation(EquivocationProof {
153+
first: previous_vote.clone(),
154+
second: vote,
155+
})
154156
}
155157
} else {
156158
// this is the first vote sent by `id` for `num`, all good
@@ -197,8 +199,8 @@ mod tests {
197199
use sc_network_test::Block;
198200

199201
use beefy_primitives::{
200-
crypto::Public, keyring::Keyring, known_payloads::MMR_ROOT_ID, Commitment, Payload,
201-
SignedCommitment, ValidatorSet, VoteMessage,
202+
crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, Keyring,
203+
Payload, SignedCommitment, ValidatorSet, VoteMessage,
202204
};
203205

204206
use super::{threshold, Block as BlockT, RoundTracker, Rounds};
@@ -452,4 +454,42 @@ mod tests {
452454
rounds.conclude(3);
453455
assert!(rounds.previous_votes.is_empty());
454456
}
457+
458+
#[test]
459+
fn should_provide_equivocation_proof() {
460+
sp_tracing::try_init_simple();
461+
462+
let validators = ValidatorSet::<Public>::new(
463+
vec![Keyring::Alice.public(), Keyring::Bob.public()],
464+
Default::default(),
465+
)
466+
.unwrap();
467+
let validator_set_id = validators.id();
468+
let session_start = 1u64.into();
469+
let mut rounds = Rounds::<Block>::new(session_start, validators);
470+
471+
let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![1, 1, 1, 1]);
472+
let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![2, 2, 2, 2]);
473+
let commitment1 = Commitment { block_number: 1, payload: payload1, validator_set_id };
474+
let commitment2 = Commitment { block_number: 1, payload: payload2, validator_set_id };
475+
476+
let alice_vote1 = VoteMessage {
477+
id: Keyring::Alice.public(),
478+
commitment: commitment1,
479+
signature: Keyring::Alice.sign(b"I am committed"),
480+
};
481+
let mut alice_vote2 = alice_vote1.clone();
482+
alice_vote2.commitment = commitment2;
483+
484+
let expected_result = VoteImportResult::Equivocation(EquivocationProof {
485+
first: alice_vote1.clone(),
486+
second: alice_vote2.clone(),
487+
});
488+
489+
// vote on one payload - ok
490+
assert_eq!(rounds.add_vote(alice_vote1), VoteImportResult::Ok);
491+
492+
// vote on _another_ commitment/payload -> expected equivocation proof
493+
assert_eq!(rounds.add_vote(alice_vote2), expected_result);
494+
}
455495
}

0 commit comments

Comments
 (0)