BEEFY: implement equivocations detection, reporting and slashing (#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 <>

Author: acatangiu

Cargo.lock

@@ -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::*;

client/beefy/src/communication/gossip.rs

@@ -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,
+		}
+	}
+}

client/beefy/src/error.rs

@@ -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;
 

client/beefy/src/justification.rs

@@ -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;

client/beefy/src/keystore.rs

@@ -282,6 +282,7 @@ where
 	let worker_params = worker::WorkerParams {
 		backend,
 		payload_provider,
+		runtime,
 		network,
 		key_store: key_store.into(),
 		gossip_engine,
@@ -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),

client/beefy/src/lib.rs

@@ -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
@@ -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 {
@@ -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,
 			)?,
@@ -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,
-			)?,
 		})
 	}
 }

client/beefy/src/metrics.rs

@@ -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;
@@ -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,
 }
@@ -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
@@ -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};
@@ -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);
+	}
 }