BEEFY: client support for detecting equivocated votes (paritytech#13285)

* client/beefy: detect equivocated votes

* client/beefy: make sure to persist state after voting

* client/beefy: drop never-used aux-schema v2 migration

* impl review suggestion

---------

Signed-off-by: Adrian Catangiu <[email protected]>

client/beefy/Cargo.toml

@@ -39,7 +39,6 @@ sp-runtime = { version = "7.0.0", path = "../../primitives/runtime" }
 
 [dev-dependencies]
 serde = "1.0.136"
-strum = { version = "0.24.1", features = ["derive"] }
 tempfile = "3.1.0"
 tokio = "1.22.0"
 sc-block-builder = { version = "0.10.0-dev", path = "../block-builder" }

client/beefy/src/aux_schema.rs

@@ -26,25 +26,25 @@ use sp_blockchain::{Error as ClientError, Result as ClientResult};
 use sp_runtime::traits::Block as BlockT;
 
 const VERSION_KEY: &[u8] = b"beefy_auxschema_version";
-const WORKER_STATE: &[u8] = b"beefy_voter_state";
+const WORKER_STATE_KEY: &[u8] = b"beefy_voter_state";
 
 const CURRENT_VERSION: u32 = 2;
 
-pub(crate) fn write_current_version<B: AuxStore>(backend: &B) -> ClientResult<()> {
+pub(crate) fn write_current_version<BE: AuxStore>(backend: &BE) -> ClientResult<()> {
 	info!(target: LOG_TARGET, "🥩 write aux schema version {:?}", CURRENT_VERSION);
 	AuxStore::insert_aux(backend, &[(VERSION_KEY, CURRENT_VERSION.encode().as_slice())], &[])
 }
 
 /// Write voter state.
-pub(crate) fn write_voter_state<Block: BlockT, B: AuxStore>(
-	backend: &B,
-	state: &PersistedState<Block>,
+pub(crate) fn write_voter_state<B: BlockT, BE: AuxStore>(
+	backend: &BE,
+	state: &PersistedState<B>,
 ) -> ClientResult<()> {
 	trace!(target: LOG_TARGET, "🥩 persisting {:?}", state);
-	backend.insert_aux(&[(WORKER_STATE, state.encode().as_slice())], &[])
+	AuxStore::insert_aux(backend, &[(WORKER_STATE_KEY, state.encode().as_slice())], &[])
 }
 
-fn load_decode<B: AuxStore, T: Decode>(backend: &B, key: &[u8]) -> ClientResult<Option<T>> {
+fn load_decode<BE: AuxStore, T: Decode>(backend: &BE, key: &[u8]) -> ClientResult<Option<T>> {
 	match backend.get_aux(key)? {
 		None => Ok(None),
 		Some(t) => T::decode(&mut &t[..])
@@ -64,7 +64,7 @@ where
 	match version {
 		None => (),
 		Some(1) => (), // version 1 is totally obsolete and should be simply ignored
-		Some(2) => return load_decode::<_, PersistedState<B>>(backend, WORKER_STATE),
+		Some(2) => return load_decode::<_, PersistedState<B>>(backend, WORKER_STATE_KEY),
 		other =>
 			return Err(ClientError::Backend(format!("Unsupported BEEFY DB version: {:?}", other))),
 	}

client/beefy/src/communication/gossip.rs

@@ -120,7 +120,7 @@ where
 
 	/// Note a voting round.
 	///
-	/// Noting round will start a live `round`.
+	/// Noting round will track gossiped votes for `round`.
 	pub(crate) fn note_round(&self, round: NumberFor<B>) {
 		debug!(target: LOG_TARGET, "🥩 About to note gossip round #{}", round);
 		self.known_votes.write().insert(round);
@@ -242,9 +242,10 @@ mod tests {
 	use sc_network_test::Block;
 	use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr};
 
-	use crate::keystore::{tests::Keyring, BeefyKeystore};
+	use crate::keystore::BeefyKeystore;
 	use beefy_primitives::{
-		crypto::Signature, known_payloads, Commitment, MmrRootHash, Payload, VoteMessage, KEY_TYPE,
+		crypto::Signature, keyring::Keyring, known_payloads, Commitment, MmrRootHash, Payload,
+		VoteMessage, KEY_TYPE,
 	};
 
 	use super::*;

client/beefy/src/justification.rs

@@ -81,12 +81,13 @@ fn verify_with_validator_set<Block: BlockT>(
 #[cfg(test)]
 pub(crate) mod tests {
 	use beefy_primitives::{
-		known_payloads, Commitment, Payload, SignedCommitment, VersionedFinalityProof,
+		keyring::Keyring, known_payloads, Commitment, Payload, SignedCommitment,
+		VersionedFinalityProof,
 	};
 	use substrate_test_runtime_client::runtime::Block;
 
 	use super::*;
-	use crate::{keystore::tests::Keyring, tests::make_beefy_ids};
+	use crate::tests::make_beefy_ids;
 
 	pub(crate) fn new_finality_proof(
 		block_num: NumberFor<Block>,

client/beefy/src/keystore.rs

@@ -19,7 +19,6 @@
 use sp_application_crypto::RuntimeAppPublic;
 use sp_core::keccak_256;
 use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr};
-use sp_runtime::traits::Keccak256;
 
 use log::warn;
 
@@ -30,6 +29,9 @@ use beefy_primitives::{
 
 use crate::{error, LOG_TARGET};
 
+/// Hasher used for BEEFY signatures.
+pub(crate) type BeefySignatureHasher = sp_runtime::traits::Keccak256;
+
 /// A BEEFY specific keystore implemented as a `Newtype`. This is basically a
 /// wrapper around [`sp_keystore::SyncCryptoStore`] and allows to customize
 /// common cryptographic functionality.
@@ -104,7 +106,7 @@ impl BeefyKeystore {
 	///
 	/// Return `true` if the signature is authentic, `false` otherwise.
 	pub fn verify(public: &Public, sig: &Signature, message: &[u8]) -> bool {
-		BeefyAuthorityId::<Keccak256>::verify(public, sig, message)
+		BeefyAuthorityId::<BeefySignatureHasher>::verify(public, sig, message)
 	}
 }
 
@@ -119,63 +121,13 @@ pub mod tests {
 	use std::sync::Arc;
 
 	use sc_keystore::LocalKeystore;
-	use sp_core::{ecdsa, keccak_256, Pair};
-	use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr};
+	use sp_core::{ecdsa, Pair};
 
-	use beefy_primitives::{crypto, KEY_TYPE};
+	use beefy_primitives::{crypto, keyring::Keyring};
 
-	use super::BeefyKeystore;
+	use super::*;
 	use crate::error::Error;
 
-	/// Set of test accounts using [`beefy_primitives::crypto`] types.
-	#[allow(missing_docs)]
-	#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display, strum::EnumIter)]
-	pub(crate) enum Keyring {
-		Alice,
-		Bob,
-		Charlie,
-		Dave,
-		Eve,
-		Ferdie,
-		One,
-		Two,
-	}
-
-	impl Keyring {
-		/// Sign `msg`.
-		pub fn sign(self, msg: &[u8]) -> crypto::Signature {
-			let msg = keccak_256(msg);
-			ecdsa::Pair::from(self).sign_prehashed(&msg).into()
-		}
-
-		/// Return key pair.
-		pub fn pair(self) -> crypto::Pair {
-			ecdsa::Pair::from_string(self.to_seed().as_str(), None).unwrap().into()
-		}
-
-		/// Return public key.
-		pub fn public(self) -> crypto::Public {
-			self.pair().public()
-		}
-
-		/// Return seed string.
-		pub fn to_seed(self) -> String {
-			format!("//{}", self)
-		}
-	}
-
-	impl From<Keyring> for crypto::Pair {
-		fn from(k: Keyring) -> Self {
-			k.pair()
-		}
-	}
-
-	impl From<Keyring> for ecdsa::Pair {
-		fn from(k: Keyring) -> Self {
-			k.pair().into()
-		}
-	}
-
 	fn keystore() -> SyncCryptoStorePtr {
 		Arc::new(LocalKeystore::in_memory())
 	}

client/beefy/src/metrics.rs

@@ -30,6 +30,8 @@ pub(crate) struct Metrics {
 	pub beefy_round_concluded: Gauge<U64>,
 	/// Best block finalized by BEEFY
 	pub beefy_best_block: Gauge<U64>,
+	/// Best block BEEFY voted on
+	pub beefy_best_voted: Gauge<U64>,
 	/// Next block BEEFY should vote on
 	pub beefy_should_vote_on: Gauge<U64>,
 	/// Number of sessions with lagging signed commitment on mandatory block
@@ -61,6 +63,10 @@ impl Metrics {
 				Gauge::new("substrate_beefy_best_block", "Best block finalized by BEEFY")?,
 				registry,
 			)?,
+			beefy_best_voted: register(
+				Gauge::new("substrate_beefy_best_voted", "Best block voted on by BEEFY")?,
+				registry,
+			)?,
 			beefy_should_vote_on: register(
 				Gauge::new("substrate_beefy_should_vote_on", "Next block, BEEFY should vote on")?,
 				registry,