BEEFY: Define basic fisherman (#4328)

Related to https://github.com/paritytech/polkadot-sdk/pull/1903

For #1903 we will need to add a Fisherman struct. This PR:
- defines a basic version of `Fisherman` and moves into it the logic
that we have now for reporting double voting equivocations
- splits the logic for generating the key ownership proofs into a more
generic separate method
- renames `EquivocationProof` to `DoubleVotingProof` since later we will
introduce a new type of equivocation

The PR doesn't contain any functional changes

substrate/client/consensus/beefy/Cargo.toml

@@ -39,7 +39,6 @@ sp-consensus-beefy = { path = "../../../primitives/consensus/beefy" }
 sp-core = { path = "../../../primitives/core" }
 sp-crypto-hashing = { path = "../../../primitives/crypto/hashing" }
 sp-keystore = { path = "../../../primitives/keystore" }
-sp-mmr-primitives = { path = "../../../primitives/merkle-mountain-range" }
 sp-runtime = { path = "../../../primitives/runtime" }
 tokio = "1.37"
 
@@ -51,6 +50,7 @@ sc-block-builder = { path = "../../block-builder" }
 sc-network-test = { path = "../../network/test" }
 sp-consensus-grandpa = { path = "../../../primitives/consensus/grandpa" }
 sp-keyring = { path = "../../../primitives/keyring" }
+sp-mmr-primitives = { path = "../../../primitives/merkle-mountain-range" }
 sp-tracing = { path = "../../../primitives/tracing" }
 substrate-test-runtime-client = { path = "../../../test-utils/runtime/client" }
 

substrate/client/consensus/beefy/src/fisherman.rs

@@ -0,0 +1,162 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+use crate::{error::Error, keystore::BeefyKeystore, round::Rounds, LOG_TARGET};
+use log::{debug, error, warn};
+use sc_client_api::Backend;
+use sp_api::ProvideRuntimeApi;
+use sp_blockchain::HeaderBackend;
+use sp_consensus_beefy::{
+	check_equivocation_proof,
+	ecdsa_crypto::{AuthorityId, Signature},
+	BeefyApi, BeefySignatureHasher, DoubleVotingProof, OpaqueKeyOwnershipProof, ValidatorSetId,
+};
+use sp_runtime::{
+	generic::BlockId,
+	traits::{Block, NumberFor},
+};
+use std::{marker::PhantomData, sync::Arc};
+
+/// Helper struct containing the id and the key ownership proof for a validator.
+pub struct ProvedValidator<'a> {
+	pub id: &'a AuthorityId,
+	pub key_owner_proof: OpaqueKeyOwnershipProof,
+}
+
+/// Helper used to check and report equivocations.
+pub struct Fisherman<B, BE, RuntimeApi> {
+	backend: Arc<BE>,
+	runtime: Arc<RuntimeApi>,
+	key_store: Arc<BeefyKeystore<AuthorityId>>,
+
+	_phantom: PhantomData<B>,
+}
+
+impl<B: Block, BE: Backend<B>, RuntimeApi: ProvideRuntimeApi<B>> Fisherman<B, BE, RuntimeApi>
+where
+	RuntimeApi::Api: BeefyApi<B, AuthorityId>,
+{
+	pub fn new(
+		backend: Arc<BE>,
+		runtime: Arc<RuntimeApi>,
+		keystore: Arc<BeefyKeystore<AuthorityId>>,
+	) -> Self {
+		Self { backend, runtime, key_store: keystore, _phantom: Default::default() }
+	}
+
+	fn prove_offenders<'a>(
+		&self,
+		at: BlockId<B>,
+		offender_ids: impl Iterator<Item = &'a AuthorityId>,
+		validator_set_id: ValidatorSetId,
+	) -> Result<Vec<ProvedValidator<'a>>, Error> {
+		let hash = match at {
+			BlockId::Hash(hash) => hash,
+			BlockId::Number(number) => self
+				.backend
+				.blockchain()
+				.expect_block_hash_from_id(&BlockId::Number(number))
+				.map_err(|err| {
+					Error::Backend(format!(
+						"Couldn't get hash for block #{:?} (error: {:?}). \
+						Skipping report for equivocation",
+						at, err
+					))
+				})?,
+		};
+
+		let runtime_api = self.runtime.runtime_api();
+		let mut proved_offenders = vec![];
+		for offender_id in offender_ids {
+			match runtime_api.generate_key_ownership_proof(
+				hash,
+				validator_set_id,
+				offender_id.clone(),
+			) {
+				Ok(Some(key_owner_proof)) => {
+					proved_offenders.push(ProvedValidator { id: offender_id, key_owner_proof });
+				},
+				Ok(None) => {
+					debug!(
+						target: LOG_TARGET,
+						"🥩 Equivocation offender {} not part of the authority set {}.",
+						offender_id, validator_set_id
+					);
+				},
+				Err(e) => {
+					error!(
+						target: LOG_TARGET,
+						"🥩 Error generating key ownership proof for equivocation offender {} \
+						in authority set {}: {}",
+						offender_id, validator_set_id, e
+					);
+				},
+			};
+		}
+
+		Ok(proved_offenders)
+	}
+
+	/// Report the given equivocation to the BEEFY runtime module. This method
+	/// generates a session membership proof of the offender and then submits an
+	/// extrinsic to report the equivocation. In particular, the session membership
+	/// proof must be generated at the block at which the given set was active which
+	/// isn't necessarily the best block if there are pending authority set changes.
+	pub fn report_double_voting(
+		&self,
+		proof: DoubleVotingProof<NumberFor<B>, AuthorityId, Signature>,
+		active_rounds: &Rounds<B>,
+	) -> Result<(), Error> {
+		let (validators, validator_set_id) =
+			(active_rounds.validators(), active_rounds.validator_set_id());
+		let offender_id = proof.offender_id();
+
+		if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) {
+			debug!(target: LOG_TARGET, "🥩 Skipping report for bad equivocation {:?}", proof);
+			return Ok(())
+		}
+
+		if let Some(local_id) = self.key_store.authority_id(validators) {
+			if offender_id == &local_id {
+				warn!(target: LOG_TARGET, "🥩 Skipping report for own equivocation");
+				return Ok(())
+			}
+		}
+
+		let key_owner_proofs = self.prove_offenders(
+			BlockId::Number(*proof.round_number()),
+			vec![offender_id].into_iter(),
+			validator_set_id,
+		)?;
+
+		// submit equivocation report at **best** block
+		let best_block_hash = self.backend.blockchain().info().best_hash;
+		for ProvedValidator { key_owner_proof, .. } in key_owner_proofs {
+			self.runtime
+				.runtime_api()
+				.submit_report_equivocation_unsigned_extrinsic(
+					best_block_hash,
+					proof.clone(),
+					key_owner_proof,
+				)
+				.map_err(Error::RuntimeApi)?;
+		}
+
+		Ok(())
+	}
+}

substrate/client/consensus/beefy/src/lib.rs

@@ -43,11 +43,10 @@ use sp_api::ProvideRuntimeApi;
 use sp_blockchain::{Backend as BlockchainBackend, HeaderBackend};
 use sp_consensus::{Error as ConsensusError, SyncOracle};
 use sp_consensus_beefy::{
-	ecdsa_crypto::AuthorityId, BeefyApi, ConsensusLog, MmrRootHash, PayloadProvider, ValidatorSet,
+	ecdsa_crypto::AuthorityId, BeefyApi, ConsensusLog, PayloadProvider, ValidatorSet,
 	BEEFY_ENGINE_ID,
 };
 use sp_keystore::KeystorePtr;
-use sp_mmr_primitives::MmrApi;
 use sp_runtime::traits::{Block, Header as HeaderT, NumberFor, Zero};
 use std::{
 	collections::{BTreeMap, VecDeque},
@@ -69,6 +68,7 @@ pub mod justification;
 
 use crate::{
 	communication::gossip::GossipValidator,
+	fisherman::Fisherman,
 	justification::BeefyVersionedFinalityProof,
 	keystore::BeefyKeystore,
 	metrics::VoterMetrics,
@@ -80,6 +80,7 @@ pub use communication::beefy_protocol_name::{
 };
 use sp_runtime::generic::OpaqueDigestItemId;
 
+mod fisherman;
 #[cfg(test)]
 mod tests;
 
@@ -305,14 +306,16 @@ where
 		pending_justifications: BTreeMap<NumberFor<B>, BeefyVersionedFinalityProof<B>>,
 		is_authority: bool,
 	) -> BeefyWorker<B, BE, P, R, S, N> {
+		let key_store = Arc::new(self.key_store);
 		BeefyWorker {
-			backend: self.backend,
-			runtime: self.runtime,
-			key_store: self.key_store,
-			metrics: self.metrics,
-			persisted_state: self.persisted_state,
+			backend: self.backend.clone(),
+			runtime: self.runtime.clone(),
+			key_store: key_store.clone(),
 			payload_provider,
 			sync,
+			fisherman: Arc::new(Fisherman::new(self.backend, self.runtime, key_store)),
+			metrics: self.metrics,
+			persisted_state: self.persisted_state,
 			comms,
 			links,
 			pending_justifications,
@@ -487,7 +490,7 @@ pub async fn start_beefy_gadget<B, BE, C, N, P, R, S>(
 	C: Client<B, BE> + BlockBackend<B>,
 	P: PayloadProvider<B> + Clone,
 	R: ProvideRuntimeApi<B>,
-	R::Api: BeefyApi<B, AuthorityId> + MmrApi<B, MmrRootHash, NumberFor<B>>,
+	R::Api: BeefyApi<B, AuthorityId>,
 	N: GossipNetwork<B> + NetworkRequest + Send + Sync + 'static,
 	S: GossipSyncing<B> + SyncOracle + 'static,
 {

substrate/client/consensus/beefy/src/round.rs

@@ -22,7 +22,7 @@ use codec::{Decode, Encode};
 use log::{debug, info};
 use sp_consensus_beefy::{
 	ecdsa_crypto::{AuthorityId, Signature},
-	Commitment, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage,
+	Commitment, DoubleVotingProof, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage,
 };
 use sp_runtime::traits::{Block, NumberFor};
 use std::collections::BTreeMap;
@@ -61,7 +61,7 @@ pub fn threshold(authorities: usize) -> usize {
 pub enum VoteImportResult<B: Block> {
 	Ok,
 	RoundConcluded(SignedCommitment<NumberFor<B>, Signature>),
-	Equivocation(EquivocationProof<NumberFor<B>, AuthorityId, Signature>),
+	DoubleVoting(DoubleVotingProof<NumberFor<B>, AuthorityId, Signature>),
 	Invalid,
 	Stale,
 }
@@ -153,7 +153,7 @@ where
 					target: LOG_TARGET,
 					"🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", previous_vote, vote
 				);
-				return VoteImportResult::Equivocation(EquivocationProof {
+				return VoteImportResult::DoubleVoting(DoubleVotingProof {
 					first: previous_vote.clone(),
 					second: vote,
 				})
@@ -207,7 +207,7 @@ mod tests {
 	use sc_network_test::Block;
 
 	use sp_consensus_beefy::{
-		known_payloads::MMR_ROOT_ID, test_utils::Keyring, Commitment, EquivocationProof, Payload,
+		known_payloads::MMR_ROOT_ID, test_utils::Keyring, Commitment, DoubleVotingProof, Payload,
 		SignedCommitment, ValidatorSet, VoteMessage,
 	};
 
@@ -494,7 +494,7 @@ mod tests {
 		let mut alice_vote2 = alice_vote1.clone();
 		alice_vote2.commitment = commitment2;
 
-		let expected_result = VoteImportResult::Equivocation(EquivocationProof {
+		let expected_result = VoteImportResult::DoubleVoting(DoubleVotingProof {
 			first: alice_vote1.clone(),
 			second: alice_vote2.clone(),
 		});

substrate/client/consensus/beefy/src/tests.rs

@@ -59,7 +59,7 @@ use sp_consensus_beefy::{
 	known_payloads,
 	mmr::{find_mmr_root_digest, MmrRootProvider},
 	test_utils::Keyring as BeefyKeyring,
-	BeefyApi, Commitment, ConsensusLog, EquivocationProof, MmrRootHash, OpaqueKeyOwnershipProof,
+	BeefyApi, Commitment, ConsensusLog, DoubleVotingProof, MmrRootHash, OpaqueKeyOwnershipProof,
 	Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, VoteMessage,
 	BEEFY_ENGINE_ID,
 };
@@ -259,7 +259,7 @@ pub(crate) struct TestApi {
 	pub validator_set: Option<BeefyValidatorSet>,
 	pub mmr_root_hash: MmrRootHash,
 	pub reported_equivocations:
-		Option<Arc<Mutex<Vec<EquivocationProof<NumberFor<Block>, AuthorityId, Signature>>>>>,
+		Option<Arc<Mutex<Vec<DoubleVotingProof<NumberFor<Block>, AuthorityId, Signature>>>>>,
 }
 
 impl TestApi {
@@ -313,7 +313,7 @@ sp_api::mock_impl_runtime_apis! {
 		}
 
 		fn submit_report_equivocation_unsigned_extrinsic(
-			proof: EquivocationProof<NumberFor<Block>, AuthorityId, Signature>,
+			proof: DoubleVotingProof<NumberFor<Block>, AuthorityId, Signature>,
 			_dummy: OpaqueKeyOwnershipProof,
 		) -> Option<()> {
 			if let Some(equivocations_buf) = self.inner.reported_equivocations.as_ref() {

substrate/client/consensus/beefy/src/worker.rs

@@ -23,6 +23,7 @@ use crate::{
 	},
 	error::Error,
 	find_authorities_change,
+	fisherman::Fisherman,
 	justification::BeefyVersionedFinalityProof,
 	keystore::BeefyKeystore,
 	metric_inc, metric_set,
@@ -39,10 +40,9 @@ use sp_api::ProvideRuntimeApi;
 use sp_arithmetic::traits::{AtLeast32Bit, Saturating};
 use sp_consensus::SyncOracle;
 use sp_consensus_beefy::{
-	check_equivocation_proof,
 	ecdsa_crypto::{AuthorityId, Signature},
-	BeefyApi, BeefySignatureHasher, Commitment, EquivocationProof, PayloadProvider, ValidatorSet,
-	VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID,
+	BeefyApi, Commitment, DoubleVotingProof, PayloadProvider, ValidatorSet, VersionedFinalityProof,
+	VoteMessage, BEEFY_ENGINE_ID,
 };
 use sp_runtime::{
 	generic::BlockId,
@@ -377,9 +377,10 @@ pub(crate) struct BeefyWorker<B: Block, BE, P, RuntimeApi, S, N> {
 	// utilities
 	pub backend: Arc<BE>,
 	pub runtime: Arc<RuntimeApi>,
-	pub key_store: BeefyKeystore<AuthorityId>,
+	pub key_store: Arc<BeefyKeystore<AuthorityId>>,
 	pub payload_provider: P,
 	pub sync: Arc<S>,
+	pub fisherman: Arc<Fisherman<B, BE, RuntimeApi>>,
 
 	// communication (created once, but returned and reused if worker is restarted/reinitialized)
 	pub comms: BeefyComms<B, N>,
@@ -590,9 +591,9 @@ where
 				}
 				metric_inc!(self.metrics, beefy_good_votes_processed);
 			},
-			VoteImportResult::Equivocation(proof) => {
+			VoteImportResult::DoubleVoting(proof) => {
 				metric_inc!(self.metrics, beefy_equivocation_votes);
-				self.report_equivocation(proof)?;
+				self.report_double_voting(proof)?;
 			},
 			VoteImportResult::Invalid => metric_inc!(self.metrics, beefy_invalid_votes),
 			VoteImportResult::Stale => metric_inc!(self.metrics, beefy_stale_votes),
@@ -941,64 +942,13 @@ where
 		(error, self.comms)
 	}
 
-	/// Report the given equivocation to the BEEFY runtime module. This method
-	/// generates a session membership proof of the offender and then submits an
-	/// extrinsic to report the equivocation. In particular, the session membership
-	/// proof must be generated at the block at which the given set was active which
-	/// isn't necessarily the best block if there are pending authority set changes.
-	pub(crate) fn report_equivocation(
+	/// Report the given equivocation to the BEEFY runtime module.
+	fn report_double_voting(
 		&self,
-		proof: EquivocationProof<NumberFor<B>, AuthorityId, Signature>,
+		proof: DoubleVotingProof<NumberFor<B>, AuthorityId, Signature>,
 	) -> Result<(), Error> {
 		let rounds = self.persisted_state.voting_oracle.active_rounds()?;
-		let (validators, validator_set_id) = (rounds.validators(), rounds.validator_set_id());
-		let offender_id = proof.offender_id().clone();
-
-		if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) {
-			debug!(target: LOG_TARGET, "🥩 Skip report for bad equivocation {:?}", proof);
-			return Ok(())
-		} else if let Some(local_id) = self.key_store.authority_id(validators) {
-			if offender_id == local_id {
-				warn!(target: LOG_TARGET, "🥩 Skip equivocation report for own equivocation");
-				return Ok(())
-			}
-		}
-
-		let number = *proof.round_number();
-		let hash = self
-			.backend
-			.blockchain()
-			.expect_block_hash_from_id(&BlockId::Number(number))
-			.map_err(|err| {
-				let err_msg = format!(
-					"Couldn't get hash for block #{:?} (error: {:?}), skipping report for equivocation",
-					number, err
-				);
-				Error::Backend(err_msg)
-			})?;
-		let runtime_api = self.runtime.runtime_api();
-		// generate key ownership proof at that block
-		let key_owner_proof = match runtime_api
-			.generate_key_ownership_proof(hash, validator_set_id, offender_id)
-			.map_err(Error::RuntimeApi)?
-		{
-			Some(proof) => proof,
-			None => {
-				debug!(
-					target: LOG_TARGET,
-					"🥩 Equivocation offender not part of the authority set."
-				);
-				return Ok(())
-			},
-		};
-
-		// submit equivocation report at **best** block
-		let best_block_hash = self.backend.blockchain().info().best_hash;
-		runtime_api
-			.submit_report_equivocation_unsigned_extrinsic(best_block_hash, proof, key_owner_proof)
-			.map_err(Error::RuntimeApi)?;
-
-		Ok(())
+		self.fisherman.report_double_voting(proof, rounds)
 	}
 }
 
@@ -1165,13 +1115,15 @@ pub(crate) mod tests {
 		.unwrap();
 		let payload_provider = MmrRootProvider::new(api.clone());
 		let comms = BeefyComms { gossip_engine, gossip_validator, on_demand_justifications };
+		let key_store: Arc<BeefyKeystore<AuthorityId>> = Arc::new(Some(keystore).into());
 		BeefyWorker {
-			backend,
-			runtime: api,
-			key_store: Some(keystore).into(),
+			backend: backend.clone(),
+			runtime: api.clone(),
+			key_store: key_store.clone(),
 			metrics,
 			payload_provider,
 			sync: Arc::new(sync),
+			fisherman: Arc::new(Fisherman::new(backend, api, key_store)),
 			links,
 			comms,
 			pending_justifications: BTreeMap::new(),
@@ -1590,6 +1542,11 @@ pub(crate) mod tests {
 		let mut net = BeefyTestNet::new(1);
 		let mut worker = create_beefy_worker(net.peer(0), &keys[0], 1, validator_set.clone());
 		worker.runtime = api_alice.clone();
+		worker.fisherman = Arc::new(Fisherman::new(
+			worker.backend.clone(),
+			worker.runtime.clone(),
+			worker.key_store.clone(),
+		));
 
 		// let there be a block with num = 1:
 		let _ = net.peer(0).push_blocks(1, false);
@@ -1604,7 +1561,7 @@ pub(crate) mod tests {
 		);
 		{
 			// expect voter (Alice) to successfully report it
-			assert_eq!(worker.report_equivocation(good_proof.clone()), Ok(()));
+			assert_eq!(worker.report_double_voting(good_proof.clone()), Ok(()));
 			// verify Alice reports Bob equivocation to runtime
 			let reported = api_alice.reported_equivocations.as_ref().unwrap().lock();
 			assert_eq!(reported.len(), 1);
@@ -1616,7 +1573,7 @@ pub(crate) mod tests {
 		let mut bad_proof = good_proof.clone();
 		bad_proof.first.id = Keyring::Charlie.public();
 		// bad proofs are simply ignored
-		assert_eq!(worker.report_equivocation(bad_proof), Ok(()));
+		assert_eq!(worker.report_double_voting(bad_proof), Ok(()));
 		// verify nothing reported to runtime
 		assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty());
 
@@ -1625,7 +1582,7 @@ pub(crate) mod tests {
 		old_proof.first.commitment.validator_set_id = 0;
 		old_proof.second.commitment.validator_set_id = 0;
 		// old proofs are simply ignored
-		assert_eq!(worker.report_equivocation(old_proof), Ok(()));
+		assert_eq!(worker.report_double_voting(old_proof), Ok(()));
 		// verify nothing reported to runtime
 		assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty());
 
@@ -1635,7 +1592,7 @@ pub(crate) mod tests {
 			(block_num, payload2.clone(), set_id, &Keyring::Alice),
 		);
 		// equivocations done by 'self' are simply ignored (not reported)
-		assert_eq!(worker.report_equivocation(self_proof), Ok(()));
+		assert_eq!(worker.report_double_voting(self_proof), Ok(()));
 		// verify nothing reported to runtime
 		assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty());
 	}