From 7d74264f8859798df1e901b591f7daba5c789fc6 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Wed, 21 Dec 2022 18:27:13 +0200 Subject: [PATCH 01/41] client/beefy: simplify self_vote logic --- client/beefy/src/metrics.rs | 6 +++ client/beefy/src/round.rs | 91 +++++++++---------------------------- client/beefy/src/worker.rs | 17 ++++--- 3 files changed, 37 insertions(+), 77 deletions(-) diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 71e34e24c4fa0..55fdecc36d4b0 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -30,6 +30,8 @@ pub(crate) struct Metrics { pub beefy_round_concluded: Gauge, /// Best block finalized by BEEFY pub beefy_best_block: Gauge, + /// Best block BEEFY voted on + pub beefy_best_voted: Gauge, /// Next block BEEFY should vote on pub beefy_should_vote_on: Gauge, /// 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, diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 48d3d087299d0..42bda82c683a6 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -30,26 +30,20 @@ use std::{collections::BTreeMap, hash::Hash}; /// /// Does not do any validation on votes or signatures, layers above need to handle that (gossip). #[derive(Debug, Decode, Default, Encode, PartialEq)] -struct RoundTracker { - self_vote: bool, +pub(crate) struct RoundTracker { votes: BTreeMap, } impl RoundTracker { - fn add_vote(&mut self, vote: (Public, Signature), self_vote: bool) -> bool { + fn add_vote(&mut self, vote: (Public, Signature)) -> bool { if self.votes.contains_key(&vote.0) { return false } - self.self_vote = self.self_vote || self_vote; self.votes.insert(vote.0, vote.1); true } - fn has_self_vote(&self) -> bool { - self.self_vote - } - fn is_done(&self, threshold: usize) -> bool { self.votes.len() >= threshold } @@ -109,16 +103,10 @@ where self.mandatory_done } - pub(crate) fn should_self_vote(&self, round: &(P, NumberFor)) -> bool { - Some(round.1) > self.best_done && - self.rounds.get(round).map(|tracker| !tracker.has_self_vote()).unwrap_or(true) - } - pub(crate) fn add_vote( &mut self, round: &(P, NumberFor), vote: (Public, Signature), - self_vote: bool, ) -> bool { let num = round.1; if num < self.session_start || Some(num) <= self.best_done { @@ -132,7 +120,7 @@ where ); false } else { - self.rounds.entry(round.clone()).or_default().add_vote(vote, self_vote) + self.rounds.entry(round.clone()).or_default().add_vote(vote) } } @@ -195,26 +183,18 @@ mod tests { let bob_vote = (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")); let threshold = 2; - // self vote not added yet - assert!(!rt.has_self_vote()); - // adding new vote allowed - assert!(rt.add_vote(bob_vote.clone(), false)); + assert!(rt.add_vote(bob_vote.clone())); // adding existing vote not allowed - assert!(!rt.add_vote(bob_vote, false)); - - // self vote still not added yet - assert!(!rt.has_self_vote()); + assert!(!rt.add_vote(bob_vote)); // vote is not done assert!(!rt.is_done(threshold)); let alice_vote = (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")); // adding new vote (self vote this time) allowed - assert!(rt.add_vote(alice_vote, true)); + assert!(rt.add_vote(alice_vote)); - // self vote registered - assert!(rt.has_self_vote()); // vote is now done assert!(rt.is_done(threshold)); } @@ -269,41 +249,25 @@ mod tests { let session_start = 1u64.into(); let mut rounds = Rounds::::new(session_start, validators); - // no self vote yet, should self vote - assert!(rounds.should_self_vote(&round)); - // add 1st good vote - assert!(rounds.add_vote( - &round, - (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")), - true - )); + assert!(rounds + .add_vote(&round, (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")))); // round not concluded assert!(rounds.should_conclude(&round).is_none()); - // self vote already present, should not self vote - assert!(!rounds.should_self_vote(&round)); // double voting not allowed - assert!(!rounds.add_vote( - &round, - (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")), - true - )); + assert!(!rounds + .add_vote(&round, (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")))); // invalid vote (Dave is not a validator) - assert!(!rounds.add_vote( - &round, - (Keyring::Dave.public(), Keyring::Dave.sign(b"I am committed")), - false - )); + assert!(!rounds + .add_vote(&round, (Keyring::Dave.public(), Keyring::Dave.sign(b"I am committed")),)); assert!(rounds.should_conclude(&round).is_none()); // add 2nd good vote - assert!(rounds.add_vote( - &round, - (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")), - false - )); + assert!( + rounds.add_vote(&round, (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")),) + ); // round not concluded assert!(rounds.should_conclude(&round).is_none()); @@ -311,18 +275,14 @@ mod tests { assert!(rounds.add_vote( &round, (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am committed")), - false )); // round concluded assert!(rounds.should_conclude(&round).is_some()); rounds.conclude(round.1); // Eve is a validator, but round was concluded, adding vote disallowed - assert!(!rounds.add_vote( - &round, - (Keyring::Eve.public(), Keyring::Eve.sign(b"I am committed")), - false - )); + assert!(!rounds + .add_vote(&round, (Keyring::Eve.public(), Keyring::Eve.sign(b"I am committed")),)); } #[test] @@ -341,7 +301,7 @@ mod tests { let mut vote = (H256::from_low_u64_le(1), 9); // add vote for previous session, should fail - assert!(!rounds.add_vote(&vote, alice.clone(), true)); + assert!(!rounds.add_vote(&vote, alice.clone())); // no votes present assert!(rounds.rounds.is_empty()); @@ -349,15 +309,15 @@ mod tests { rounds.best_done = Some(11); // add votes for current session, but already concluded rounds, should fail vote.1 = 10; - assert!(!rounds.add_vote(&vote, alice.clone(), true)); + assert!(!rounds.add_vote(&vote, alice.clone())); vote.1 = 11; - assert!(!rounds.add_vote(&vote, alice.clone(), true)); + assert!(!rounds.add_vote(&vote, alice.clone())); // no votes present assert!(rounds.rounds.is_empty()); // add good vote vote.1 = 12; - assert!(rounds.add_vote(&vote, alice, true)); + assert!(rounds.add_vote(&vote, alice)); // good vote present assert_eq!(rounds.rounds.len(), 1); } @@ -384,51 +344,42 @@ mod tests { assert!(rounds.add_vote( &(H256::from_low_u64_le(1), 1), (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")), - true, )); assert!(rounds.add_vote( &(H256::from_low_u64_le(1), 1), (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")), - false, )); assert!(rounds.add_vote( &(H256::from_low_u64_le(1), 1), (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am committed")), - false, )); // round 2 assert!(rounds.add_vote( &(H256::from_low_u64_le(2), 2), (Keyring::Alice.public(), Keyring::Alice.sign(b"I am again committed")), - true, )); assert!(rounds.add_vote( &(H256::from_low_u64_le(2), 2), (Keyring::Bob.public(), Keyring::Bob.sign(b"I am again committed")), - false, )); assert!(rounds.add_vote( &(H256::from_low_u64_le(2), 2), (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am again committed")), - false, )); // round 3 assert!(rounds.add_vote( &(H256::from_low_u64_le(3), 3), (Keyring::Alice.public(), Keyring::Alice.sign(b"I am still committed")), - true, )); assert!(rounds.add_vote( &(H256::from_low_u64_le(3), 3), (Keyring::Bob.public(), Keyring::Bob.sign(b"I am still committed")), - false, )); assert!(rounds.add_vote( &(H256::from_low_u64_le(3), 3), (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am still committed")), - false, )); assert_eq!(3, rounds.rounds.len()); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index b48dfa369eb2d..ba45b0b322a92 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -264,6 +264,8 @@ pub(crate) struct PersistedState { best_grandpa_block_header: ::Header, /// Best block a BEEFY voting round has been concluded for. best_beefy_block: NumberFor, + /// Best block we voted on. + best_voted: NumberFor, /// Chooses which incoming votes to accept and which votes to generate. /// Keeps track of voting seen for current and future rounds. voting_oracle: VoterOracle, @@ -279,6 +281,7 @@ impl PersistedState { VoterOracle::checked_new(sessions, min_block_delta).map(|voting_oracle| PersistedState { best_grandpa_block_header: grandpa_header, best_beefy_block: best_beefy, + best_voted: Zero::zero(), voting_oracle, }) } @@ -550,7 +553,7 @@ where .active_rounds_mut() .ok_or(Error::UninitSession)?; - if rounds.add_vote(&round, vote, self_vote) { + if rounds.add_vote(&round, vote) { if let Some(signatures) = rounds.should_conclude(&round) { self.gossip_validator.conclude_round(round.1); @@ -702,7 +705,9 @@ where .voting_target(self.best_beefy_block(), self.best_grandpa_block()) { metric_set!(self, beefy_should_vote_on, target); - self.do_vote(target)?; + if target > self.persisted_state.best_voted { + self.do_vote(target)?; + } } Ok(()) } @@ -752,10 +757,6 @@ where .voting_oracle .active_rounds_mut() .ok_or(Error::UninitSession)?; - if !rounds.should_self_vote(&(payload.clone(), target_number)) { - debug!(target: "beefy", "🥩 Don't double vote for block number: {:?}", target_number); - return Ok(()) - } let (validators, validator_set_id) = (rounds.validators(), rounds.validator_set_id()); let authority_id = if let Some(id) = self.key_store.authority_id(validators) { @@ -801,6 +802,8 @@ where } self.gossip_engine.gossip_message(topic::(), encoded_message, false); + self.persisted_state.best_voted = target_number; + metric_set!(self, beefy_best_voted, target_number); Ok(()) } @@ -987,7 +990,7 @@ pub(crate) mod tests { use sc_network_test::TestNetFactory; use sp_api::HeaderT; use sp_blockchain::Backend as BlockchainBackendT; - use sp_runtime::traits::{One, Zero}; + use sp_runtime::traits::One; use substrate_test_runtime_client::{ runtime::{Block, Digest, DigestItem, Header, H256}, Backend, From c366330cdfad211b841d90282786153d13587d4a Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Wed, 21 Dec 2022 19:17:51 +0200 Subject: [PATCH 02/41] client/beefy: migrate to new state version --- client/beefy/src/aux_schema.rs | 125 ++++++++++++++++++++++++++++++--- client/beefy/src/round.rs | 14 ++++ 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/client/beefy/src/aux_schema.rs b/client/beefy/src/aux_schema.rs index 67deee19b58b0..29d523b6aeae6 100644 --- a/client/beefy/src/aux_schema.rs +++ b/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 = 1; +const CURRENT_VERSION: u32 = 2; -pub(crate) fn write_current_version(backend: &B) -> ClientResult<()> { +pub(crate) fn write_current_version(backend: &BE) -> ClientResult<()> { info!(target: "beefy", "🥩 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( - backend: &B, - state: &PersistedState, +pub(crate) fn write_voter_state( + backend: &BE, + state: &PersistedState, ) -> ClientResult<()> { trace!(target: "beefy", "🥩 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(backend: &B, key: &[u8]) -> ClientResult> { +fn load_decode(backend: &BE, key: &[u8]) -> ClientResult> { match backend.get_aux(key)? { None => Ok(None), Some(t) => T::decode(&mut &t[..]) @@ -63,7 +63,8 @@ where match version { None => (), - Some(1) => return load_decode::<_, PersistedState>(backend, WORKER_STATE), + Some(1) => return v1::migrate_from_version1::(backend), + Some(2) => return load_decode::<_, PersistedState>(backend, WORKER_STATE_KEY), other => return Err(ClientError::Backend(format!("Unsupported BEEFY DB version: {:?}", other))), } @@ -72,6 +73,112 @@ where Ok(None) } +mod v1 { + use super::*; + use crate::{round::RoundTracker, worker::PersistedState, Rounds}; + use beefy_primitives::{ + crypto::{Public, Signature}, + Payload, ValidatorSet, + }; + use sp_runtime::traits::NumberFor; + use std::collections::{BTreeMap, VecDeque}; + + #[derive(Decode)] + struct V1RoundTracker { + self_vote: bool, + votes: BTreeMap, + } + + impl Into for V1RoundTracker { + fn into(self) -> RoundTracker { + // make the compiler happy by using this deprecated field + let _ = self.self_vote; + RoundTracker::new(self.votes) + } + } + + #[derive(Decode)] + struct V1Rounds { + rounds: BTreeMap<(Payload, NumberFor), V1RoundTracker>, + session_start: NumberFor, + validator_set: ValidatorSet, + mandatory_done: bool, + best_done: Option>, + } + + impl Into> for V1Rounds + where + B: BlockT, + { + fn into(self) -> Rounds { + let rounds = self.rounds.into_iter().map(|it| (it.0, it.1.into())).collect(); + Rounds::::new_manual( + rounds, + self.session_start, + self.validator_set, + self.mandatory_done, + self.best_done, + ) + } + } + + #[derive(Decode)] + pub(crate) struct V1VoterOracle { + sessions: VecDeque>, + min_block_delta: u32, + } + + #[derive(Decode)] + pub(crate) struct V1PersistedState { + /// Best block we received a GRANDPA finality for. + best_grandpa_block_header: ::Header, + /// Best block a BEEFY voting round has been concluded for. + best_beefy_block: NumberFor, + /// Chooses which incoming votes to accept and which votes to generate. + /// Keeps track of voting seen for current and future rounds. + voting_oracle: V1VoterOracle, + } + + impl TryInto> for V1PersistedState + where + B: BlockT, + { + type Error = (); + fn try_into(self) -> Result, Self::Error> { + let Self { best_grandpa_block_header, best_beefy_block, voting_oracle } = self; + let V1VoterOracle { sessions, min_block_delta } = voting_oracle; + let sessions = sessions + .into_iter() + .map( as Into>>::into) + .collect(); + PersistedState::checked_new( + best_grandpa_block_header, + best_beefy_block, + sessions, + min_block_delta, + ) + .ok_or(()) + } + } + + pub(super) fn migrate_from_version1( + backend: &BE, + ) -> ClientResult>> + where + B: BlockT, + BE: Backend, + { + write_current_version(backend)?; + if let Some(new_state) = load_decode::<_, V1PersistedState>(backend, WORKER_STATE_KEY)? + .and_then(|old_state| old_state.try_into().ok()) + { + write_voter_state(backend, &new_state)?; + return Ok(Some(new_state)) + } + Ok(None) + } +} + #[cfg(test)] pub(crate) mod tests { use super::*; diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 42bda82c683a6..6783ae6a3d053 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -35,6 +35,10 @@ pub(crate) struct RoundTracker { } impl RoundTracker { + pub(crate) fn new(votes: BTreeMap) -> Self { + Self { votes } + } + fn add_vote(&mut self, vote: (Public, Signature)) -> bool { if self.votes.contains_key(&vote.0) { return false @@ -83,6 +87,16 @@ where } } + pub(crate) fn new_manual( + rounds: BTreeMap<(P, NumberFor), RoundTracker>, + session_start: NumberFor, + validator_set: ValidatorSet, + mandatory_done: bool, + best_done: Option>, + ) -> Self { + Rounds { rounds, session_start, validator_set, mandatory_done, best_done } + } + pub(crate) fn validator_set(&self) -> &ValidatorSet { &self.validator_set } From a9f067eb4d51b0a95b0a6add4030fecac978e091 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 22 Dec 2022 15:37:23 +0200 Subject: [PATCH 03/41] client/beefy: detect equivocated votes --- client/beefy/src/aux_schema.rs | 1 + client/beefy/src/round.rs | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/client/beefy/src/aux_schema.rs b/client/beefy/src/aux_schema.rs index 29d523b6aeae6..a9699b3f9a8c6 100644 --- a/client/beefy/src/aux_schema.rs +++ b/client/beefy/src/aux_schema.rs @@ -114,6 +114,7 @@ mod v1 { let rounds = self.rounds.into_iter().map(|it| (it.0, it.1.into())).collect(); Rounds::::new_manual( rounds, + BTreeMap::new(), self.session_start, self.validator_set, self.mandatory_done, diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 6783ae6a3d053..c516dc8a2d0cb 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -66,6 +66,7 @@ pub fn threshold(authorities: usize) -> usize { #[derive(Debug, Decode, Encode, PartialEq)] pub(crate) struct Rounds { rounds: BTreeMap<(Payload, NumberFor), RoundTracker>, + equivocations: BTreeMap<(Public, NumberFor), Payload>, session_start: NumberFor, validator_set: ValidatorSet, mandatory_done: bool, @@ -80,6 +81,7 @@ where pub(crate) fn new(session_start: NumberFor, validator_set: ValidatorSet) -> Self { Rounds { rounds: BTreeMap::new(), + equivocations: BTreeMap::new(), session_start, validator_set, mandatory_done: false, @@ -89,12 +91,13 @@ where pub(crate) fn new_manual( rounds: BTreeMap<(P, NumberFor), RoundTracker>, + equivocations: BTreeMap<(Public, NumberFor), P>, session_start: NumberFor, validator_set: ValidatorSet, mandatory_done: bool, best_done: Option>, ) -> Self { - Rounds { rounds, session_start, validator_set, mandatory_done, best_done } + Rounds { rounds, equivocations, session_start, validator_set, mandatory_done, best_done } } pub(crate) fn validator_set(&self) -> &ValidatorSet { @@ -123,6 +126,7 @@ where vote: (Public, Signature), ) -> bool { let num = round.1; + let equivocation_key = (vote.0.clone(), round.1); if num < self.session_start || Some(num) <= self.best_done { debug!(target: "beefy", "🥩 received vote for old stale round {:?}, ignoring", num); false @@ -133,6 +137,17 @@ where vote ); false + } else if self + .equivocations + // is the same public key voting for a different payload? + .get(&equivocation_key) + .map(|payload| payload != &round.0) + .unwrap_or_else(|| { + self.equivocations.insert(equivocation_key, round.0.clone()); + false + }) { + debug!(target: "beefy", "🥩 detected equivocated vote {:?}", vote); + false } else { self.rounds.entry(round.clone()).or_default().add_vote(vote) } @@ -165,6 +180,7 @@ where pub(crate) fn conclude(&mut self, round_num: NumberFor) { // Remove this and older (now stale) rounds. self.rounds.retain(|&(_, number), _| number > round_num); + self.equivocations.retain(|&(_, number), _| number > round_num); self.mandatory_done = self.mandatory_done || round_num == self.session_start; self.best_done = self.best_done.max(Some(round_num)); debug!(target: "beefy", "🥩 Concluded round #{}", round_num); From 4becc37f7011aa637b0b195e3631de96373b6cbb Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 4 Jan 2023 16:50:54 +0200 Subject: [PATCH 04/41] fix typos --- client/consensus/babe/src/lib.rs | 2 +- frame/beefy-mmr/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index 41c00169e5412..e5025acb921b1 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -1053,7 +1053,7 @@ where .map_err(|e| Error::Client(e.into()))?; // generate a key ownership proof. we start by trying to generate the - // key owernship proof at the parent of the equivocating header, this + // key ownership proof at the parent of the equivocating header, this // will make sure that proof generation is successful since it happens // during the on-going session (i.e. session keys are available in the // state to be able to generate the proof). this might fail if the diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 0b7fc22cd279b..e6efc1c8019a7 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -23,8 +23,8 @@ //! While both BEEFY and Merkle Mountain Range (MMR) can be used separately, //! these tools were designed to work together in unison. //! -//! The pallet provides a standardized MMR Leaf format that is can be used -//! to bridge BEEFY+MMR-based networks (both standalone and polkadot-like). +//! The pallet provides a standardized MMR Leaf format that can be used +//! to bridge BEEFY+MMR-based networks (both standalone and Polkadot-like). //! //! The MMR leaf contains: //! 1. Block number and parent block hash. From 6db1632f1ba08f32f42d5b944cee520264e73029 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 4 Jan 2023 17:01:29 +0200 Subject: [PATCH 05/41] sp-beefy: add equivocation primitives --- primitives/beefy/src/lib.rs | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index d7ac091491bff..5f144ca0754ce 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -193,6 +193,74 @@ pub struct VoteMessage { pub signature: Signature, } +/// An equivocation (double-vote) in a given round. +#[derive(Debug, Encode, Decode, TypeInfo)] +pub struct Equivocation { + /// The round number equivocated in + pub round_number: Number, + /// Node authority id + pub id: Id, + /// The first vote in the equivocation + pub first: VoteMessage, + /// The second vote in the equivocation + pub second: VoteMessage, +} + +/// Proof of voter misbehavior on a given set id. Misbehavior/equivocation in +/// BEEFY happens when a voter votes on the same round/block for different payloads. +/// Proving is achieved by collecting the signed commitments of conflicting votes. +#[derive(Debug, Decode, Encode, TypeInfo)] +pub struct EquivocationProof { + set_id: ValidatorSetId, + equivocation: Equivocation, +} + +/// Check a commitment signature by encoding the commitment and +/// verifying the provided signature using the expected authority id. +pub fn check_commitment_signature( + commitment: &Commitment, + authority_id: &Id, + signature: &Signature, +) -> bool +where + Id: BeefyAuthorityId, + Signature: BeefyVerify, + Number: Clone + Encode + PartialEq, + MsgHash: Hash, +{ + let encoded_commitment = commitment.encode(); + BeefyVerify::::verify(signature, &encoded_commitment, authority_id) +} + +/// Verifies the equivocation proof by making sure that both votes target +/// different blocks and that its signatures are valid. +pub fn check_equivocation_proof( + report: EquivocationProof, +) -> bool +where + Id: BeefyAuthorityId + PartialEq, + Signature: BeefyVerify, + Number: Clone + Encode + PartialEq, + MsgHash: Hash, +{ + let first = &report.equivocation.first; + let second = &report.equivocation.second; + + // if votes come from different authorities, + // or both votes have the same commitment, + // --> the equivocation is invalid. + if first.id != second.id || first.commitment == second.commitment { + return false + } + + // check signatures on both votes are valid + let valid_first = check_commitment_signature(&first.commitment, &first.id, &first.signature); + let valid_second = + check_commitment_signature(&second.commitment, &second.id, &second.signature); + + return valid_first && valid_second +} + /// New BEEFY validator set notification hook. pub trait OnNewValidatorSet { /// Function called by the pallet when BEEFY validator set changes. From e8f50ac69efdc680685243e20044ea3bf2e6164f Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 4 Jan 2023 20:20:55 +0200 Subject: [PATCH 06/41] client/beefy: refactor vote processing --- client/beefy/src/aux_schema.rs | 18 ++--- client/beefy/src/communication/gossip.rs | 2 +- client/beefy/src/round.rs | 99 +++++++++++++----------- client/beefy/src/worker.rs | 78 ++++++++----------- 4 files changed, 95 insertions(+), 102 deletions(-) diff --git a/client/beefy/src/aux_schema.rs b/client/beefy/src/aux_schema.rs index a9699b3f9a8c6..86815cdd4829d 100644 --- a/client/beefy/src/aux_schema.rs +++ b/client/beefy/src/aux_schema.rs @@ -106,14 +106,16 @@ mod v1 { best_done: Option>, } - impl Into> for V1Rounds + impl Into> for V1Rounds where B: BlockT, { - fn into(self) -> Rounds { - let rounds = self.rounds.into_iter().map(|it| (it.0, it.1.into())).collect(); - Rounds::::new_manual( - rounds, + fn into(self) -> Rounds { + // FIXME + // let rounds = self.rounds.into_iter().map(|it| (it.0, it.1.into())).collect(); + Rounds::::new_manual( + // FIXME + BTreeMap::new(), BTreeMap::new(), self.session_start, self.validator_set, @@ -148,10 +150,8 @@ mod v1 { fn try_into(self) -> Result, Self::Error> { let Self { best_grandpa_block_header, best_beefy_block, voting_oracle } = self; let V1VoterOracle { sessions, min_block_delta } = voting_oracle; - let sessions = sessions - .into_iter() - .map( as Into>>::into) - .collect(); + let sessions = + sessions.into_iter().map( as Into>>::into).collect(); PersistedState::checked_new( best_grandpa_block_header, best_beefy_block, diff --git a/client/beefy/src/communication/gossip.rs b/client/beefy/src/communication/gossip.rs index bbc35ac8e526e..2123e205aa919 100644 --- a/client/beefy/src/communication/gossip.rs +++ b/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) { debug!(target: "beefy", "🥩 About to note gossip round #{}", round); self.known_votes.write().insert(round); diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index c516dc8a2d0cb..ffd56dea7f38e 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -17,13 +17,13 @@ // along with this program. If not, see . use beefy_primitives::{ - crypto::{Public, Signature}, - ValidatorSet, ValidatorSetId, + crypto::{AuthorityId, Public, Signature}, + Commitment, Payload, ValidatorSet, ValidatorSetId, VoteMessage, }; use codec::{Decode, Encode}; -use log::{debug, trace}; +use log::debug; use sp_runtime::traits::{Block, NumberFor}; -use std::{collections::BTreeMap, hash::Hash}; +use std::collections::BTreeMap; /// Tracks for each round which validators have voted/signed and /// whether the local `self` validator has voted/signed. @@ -59,13 +59,22 @@ pub fn threshold(authorities: usize) -> usize { authorities - faulty } +#[derive(Debug, PartialEq)] +pub enum VoteImportResult { + Invalid, + Stale, + Equivocation, + OkRoundInProgress, + OkRoundConcluded(Vec>), +} + /// Keeps track of all voting rounds (block numbers) within a session. /// Only round numbers > `best_done` are of interest, all others are considered stale. /// /// Does not do any validation on votes or signatures, layers above need to handle that (gossip). #[derive(Debug, Decode, Encode, PartialEq)] -pub(crate) struct Rounds { - rounds: BTreeMap<(Payload, NumberFor), RoundTracker>, +pub(crate) struct Rounds { + rounds: BTreeMap>, RoundTracker>, equivocations: BTreeMap<(Public, NumberFor), Payload>, session_start: NumberFor, validator_set: ValidatorSet, @@ -73,9 +82,8 @@ pub(crate) struct Rounds { best_done: Option>, } -impl Rounds +impl Rounds where - P: Ord + Hash + Clone, B: Block, { pub(crate) fn new(session_start: NumberFor, validator_set: ValidatorSet) -> Self { @@ -90,8 +98,8 @@ where } pub(crate) fn new_manual( - rounds: BTreeMap<(P, NumberFor), RoundTracker>, - equivocations: BTreeMap<(Public, NumberFor), P>, + rounds: BTreeMap>, RoundTracker>, + equivocations: BTreeMap<(Public, NumberFor), Payload>, session_start: NumberFor, validator_set: ValidatorSet, mandatory_done: bool, @@ -122,64 +130,61 @@ where pub(crate) fn add_vote( &mut self, - round: &(P, NumberFor), - vote: (Public, Signature), - ) -> bool { - let num = round.1; - let equivocation_key = (vote.0.clone(), round.1); + vote: VoteMessage, AuthorityId, Signature>, + ) -> VoteImportResult { + let num = vote.commitment.block_number; + let equivocation_key = (vote.id.clone(), num); if num < self.session_start || Some(num) <= self.best_done { debug!(target: "beefy", "🥩 received vote for old stale round {:?}, ignoring", num); - false - } else if !self.validators().iter().any(|id| vote.0 == *id) { + VoteImportResult::Stale + } else if vote.commitment.validator_set_id != self.validator_set_id() { + debug!( + target: "beefy", "🥩 expected set_id {:?}, ignoring vote {:?}.", + self.validator_set_id(), vote, + ); + VoteImportResult::Invalid + } else if !self.validators().iter().any(|id| &vote.id == id) { debug!( target: "beefy", "🥩 received vote {:?} from validator that is not in the validator set, ignoring", vote ); - false + VoteImportResult::Invalid } else if self .equivocations // is the same public key voting for a different payload? .get(&equivocation_key) - .map(|payload| payload != &round.0) + .map(|payload| payload != &vote.commitment.payload) .unwrap_or_else(|| { - self.equivocations.insert(equivocation_key, round.0.clone()); + // this is the first vote done by `id` for `num` + self.equivocations.insert(equivocation_key, vote.commitment.payload.clone()); + // no equivocation false }) { debug!(target: "beefy", "🥩 detected equivocated vote {:?}", vote); - false - } else { - self.rounds.entry(round.clone()).or_default().add_vote(vote) - } - } - - pub(crate) fn should_conclude( - &mut self, - round: &(P, NumberFor), - ) -> Option>> { - let done = self - .rounds - .get(round) - .map(|tracker| tracker.is_done(threshold(self.validator_set.len()))) - .unwrap_or(false); - trace!(target: "beefy", "🥩 Round #{} done: {}", round.1, done); - - if done { - let signatures = self.rounds.remove(round)?.votes; - Some( - self.validators() - .iter() - .map(|authority_id| signatures.get(authority_id).cloned()) - .collect(), - ) + VoteImportResult::Equivocation } else { - None + let round = self.rounds.entry(vote.commitment.clone()).or_default(); + if round.add_vote((vote.id, vote.signature)) && + round.is_done(threshold(self.validator_set.len())) + { + if let Some(round) = self.rounds.remove(&vote.commitment) { + let votes = round.votes; + let signatures = self + .validators() + .iter() + .map(|authority_id| votes.get(authority_id).cloned()) + .collect(); + return VoteImportResult::OkRoundConcluded(signatures) + } + } + VoteImportResult::OkRoundInProgress } } pub(crate) fn conclude(&mut self, round_num: NumberFor) { // Remove this and older (now stale) rounds. - self.rounds.retain(|&(_, number), _| number > round_num); + self.rounds.retain(|commitment, _| commitment.block_number > round_num); self.equivocations.retain(|&(_, number), _| number > round_num); self.mandatory_done = self.mandatory_done || round_num == self.session_start; self.best_done = self.best_done.max(Some(round_num)); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index ba45b0b322a92..aca25a329a2e6 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -26,12 +26,12 @@ use crate::{ keystore::BeefyKeystore, metric_inc, metric_set, metrics::Metrics, - round::Rounds, + round::{Rounds, VoteImportResult}, BeefyVoterLinks, }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, - Commitment, ConsensusLog, Payload, PayloadProvider, SignedCommitment, ValidatorSet, + Commitment, ConsensusLog, PayloadProvider, SignedCommitment, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; use codec::{Codec, Decode, Encode}; @@ -55,6 +55,7 @@ use std::{ marker::PhantomData, sync::Arc, }; + /// Bound for the number of buffered future voting rounds. const MAX_BUFFERED_VOTE_ROUNDS: usize = 600; /// Bound for the number of buffered votes per round number. @@ -83,17 +84,14 @@ pub(crate) struct VoterOracle { /// 3. lagging behind GRANDPA: queue has [1, N] elements, where all `mandatory_done == false`. /// In this state, everytime a session gets its mandatory block BEEFY finalized, it's /// popped off the queue, eventually getting to state `2. up-to-date`. - sessions: VecDeque>, + sessions: VecDeque>, /// Min delta in block numbers between two blocks, BEEFY should vote on. min_block_delta: u32, } impl VoterOracle { /// Verify provided `sessions` satisfies requirements, then build `VoterOracle`. - pub fn checked_new( - sessions: VecDeque>, - min_block_delta: u32, - ) -> Option { + pub fn checked_new(sessions: VecDeque>, min_block_delta: u32) -> Option { let mut prev_start = Zero::zero(); let mut prev_validator_id = None; // verifies the @@ -136,13 +134,13 @@ impl VoterOracle { // Return reference to rounds pertaining to first session in the queue. // Voting will always happen at the head of the queue. - fn active_rounds(&self) -> Option<&Rounds> { + fn active_rounds(&self) -> Option<&Rounds> { self.sessions.front() } // Return mutable reference to rounds pertaining to first session in the queue. // Voting will always happen at the head of the queue. - fn active_rounds_mut(&mut self) -> Option<&mut Rounds> { + fn active_rounds_mut(&mut self) -> Option<&mut Rounds> { self.sessions.front_mut() } @@ -157,7 +155,7 @@ impl VoterOracle { } /// Add new observed session to the Oracle. - pub fn add_session(&mut self, rounds: Rounds) { + pub fn add_session(&mut self, rounds: Rounds) { self.sessions.push_back(rounds); // Once we add a new session we can drop/prune previous session if it's been finalized. self.try_prune(); @@ -275,7 +273,7 @@ impl PersistedState { pub fn checked_new( grandpa_header: ::Header, best_beefy: NumberFor, - sessions: VecDeque>, + sessions: VecDeque>, min_block_delta: u32, ) -> Option { VoterOracle::checked_new(sessions, min_block_delta).map(|voting_oracle| PersistedState { @@ -384,7 +382,7 @@ where &self.persisted_state.voting_oracle } - fn active_rounds(&mut self) -> Option<&Rounds> { + fn active_rounds(&mut self) -> Option<&Rounds> { self.persisted_state.voting_oracle.active_rounds() } @@ -487,12 +485,9 @@ where ) -> Result<(), Error> { let block_num = vote.commitment.block_number; let best_grandpa = self.best_grandpa_block(); + self.gossip_validator.note_round(block_num); match self.voting_oracle().triage_round(block_num, best_grandpa)? { - RoundAction::Process => self.handle_vote( - (vote.commitment.payload, vote.commitment.block_number), - (vote.id, vote.signature), - false, - )?, + RoundAction::Process => self.handle_vote(vote, false)?, RoundAction::Enqueue => { debug!(target: "beefy", "🥩 Buffer vote for round: {:?}.", block_num); if self.pending_votes.len() < MAX_BUFFERED_VOTE_ROUNDS { @@ -541,44 +536,43 @@ where fn handle_vote( &mut self, - round: (Payload, NumberFor), - vote: (AuthorityId, Signature), + vote: VoteMessage, AuthorityId, Signature>, self_vote: bool, ) -> Result<(), Error> { - self.gossip_validator.note_round(round.1); - let rounds = self .persisted_state .voting_oracle .active_rounds_mut() .ok_or(Error::UninitSession)?; - if rounds.add_vote(&round, vote) { - if let Some(signatures) = rounds.should_conclude(&round) { - self.gossip_validator.conclude_round(round.1); + let number = vote.commitment.block_number; + let round = (vote.commitment.payload.clone(), number); + + match rounds.add_vote(vote) { + VoteImportResult::OkRoundConcluded(signatures) => { + self.gossip_validator.conclude_round(number); - let block_num = round.1; let commitment = Commitment { payload: round.0, - block_number: block_num, + block_number: number, validator_set_id: rounds.validator_set_id(), }; - let finality_proof = VersionedFinalityProof::V1(SignedCommitment { commitment, signatures }); - metric_set!(self, beefy_round_concluded, block_num); + metric_set!(self, beefy_round_concluded, number); - info!(target: "beefy", "🥩 Round #{} concluded, finality_proof: {:?}.", round.1, finality_proof); + info!(target: "beefy", "🥩 Round #{} concluded, finality_proof: {:?}.", number, finality_proof); // We created the `finality_proof` and know to be valid. // New state is persisted after finalization. self.finalize(finality_proof)?; - } else { + }, + VoteImportResult::OkRoundInProgress => { let mandatory_round = self .voting_oracle() .mandatory_pending() - .map(|p| p.0 == round.1) + .map(|(mandatory_num, _)| mandatory_num == number) .unwrap_or(false); // Persist state after handling self vote to avoid double voting in case // of voter restarts. @@ -587,8 +581,10 @@ where crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string()))?; } - } - } + }, + VoteImportResult::Equivocation => unimplemented!(), + VoteImportResult::Invalid | VoteImportResult::Stale => (), + }; Ok(()) } @@ -684,11 +680,7 @@ where for (num, votes) in votes_to_handle.into_iter() { debug!(target: "beefy", "🥩 Handle buffered votes for: {:?}.", num); for v in votes.into_iter() { - if let Err(err) = self.handle_vote( - (v.commitment.payload, v.commitment.block_number), - (v.id, v.signature), - false, - ) { + if let Err(err) = self.handle_vote(v, false) { error!(target: "beefy", "🥩 Error handling buffered vote: {}", err); }; } @@ -793,11 +785,7 @@ where debug!(target: "beefy", "🥩 Sent vote message: {:?}", message); - if let Err(err) = self.handle_vote( - (message.commitment.payload, message.commitment.block_number), - (message.id, message.signature), - true, - ) { + if let Err(err) = self.handle_vote(message, true) { error!(target: "beefy", "🥩 Error handling self vote: {}", err); } @@ -1001,7 +989,7 @@ pub(crate) mod tests { &self.voting_oracle } - pub fn active_round(&self) -> Option<&Rounds> { + pub fn active_round(&self) -> Option<&Rounds> { self.voting_oracle.active_rounds() } @@ -1015,7 +1003,7 @@ pub(crate) mod tests { } impl VoterOracle { - pub fn sessions(&self) -> &VecDeque> { + pub fn sessions(&self) -> &VecDeque> { &self.sessions } } From fc35fd549e1aadb9a71d278faaaad8e1bff6e047 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 5 Jan 2023 16:29:07 +0200 Subject: [PATCH 07/41] fix version migration for new rounds struct --- client/beefy/src/aux_schema.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/client/beefy/src/aux_schema.rs b/client/beefy/src/aux_schema.rs index 86815cdd4829d..023e20e9e575a 100644 --- a/client/beefy/src/aux_schema.rs +++ b/client/beefy/src/aux_schema.rs @@ -78,7 +78,7 @@ mod v1 { use crate::{round::RoundTracker, worker::PersistedState, Rounds}; use beefy_primitives::{ crypto::{Public, Signature}, - Payload, ValidatorSet, + Commitment, Payload, ValidatorSet, }; use sp_runtime::traits::NumberFor; use std::collections::{BTreeMap, VecDeque}; @@ -111,11 +111,16 @@ mod v1 { B: BlockT, { fn into(self) -> Rounds { - // FIXME - // let rounds = self.rounds.into_iter().map(|it| (it.0, it.1.into())).collect(); + let validator_set_id = self.validator_set.id(); + let rounds = self + .rounds + .into_iter() + .map(|((payload, block_number), v1_tracker)| { + (Commitment { payload, block_number, validator_set_id }, v1_tracker.into()) + }) + .collect(); Rounds::::new_manual( - // FIXME - BTreeMap::new(), + rounds, BTreeMap::new(), self.session_start, self.validator_set, From 800c1d93e4ec3446f1f7a213231dd6d9268a5681 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 5 Jan 2023 17:14:30 +0200 Subject: [PATCH 08/41] client/beefy: track equivocations and create proofs --- client/beefy/src/round.rs | 91 +++++++++++++++++++++---------------- client/beefy/src/worker.rs | 26 ++++++++--- primitives/beefy/src/lib.rs | 8 ++-- 3 files changed, 77 insertions(+), 48 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index ffd56dea7f38e..68c9ee15f62c4 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -18,7 +18,7 @@ use beefy_primitives::{ crypto::{AuthorityId, Public, Signature}, - Commitment, Payload, ValidatorSet, ValidatorSetId, VoteMessage, + Commitment, Equivocation, EquivocationProof, ValidatorSet, ValidatorSetId, VoteMessage, }; use codec::{Decode, Encode}; use log::debug; @@ -59,23 +59,23 @@ pub fn threshold(authorities: usize) -> usize { authorities - faulty } -#[derive(Debug, PartialEq)] -pub enum VoteImportResult { +#[derive(Debug)] +pub enum VoteImportResult { + Ok, + RoundConcluded(Vec>), + Equivocation(EquivocationProof, Public, Signature>), Invalid, Stale, - Equivocation, - OkRoundInProgress, - OkRoundConcluded(Vec>), } /// Keeps track of all voting rounds (block numbers) within a session. /// Only round numbers > `best_done` are of interest, all others are considered stale. /// /// Does not do any validation on votes or signatures, layers above need to handle that (gossip). -#[derive(Debug, Decode, Encode, PartialEq)] +#[derive(Debug, Decode, Encode)] pub(crate) struct Rounds { rounds: BTreeMap>, RoundTracker>, - equivocations: BTreeMap<(Public, NumberFor), Payload>, + equivocations: BTreeMap<(Public, NumberFor), VoteMessage, Public, Signature>>, session_start: NumberFor, validator_set: ValidatorSet, mandatory_done: bool, @@ -99,7 +99,10 @@ where pub(crate) fn new_manual( rounds: BTreeMap>, RoundTracker>, - equivocations: BTreeMap<(Public, NumberFor), Payload>, + equivocations: BTreeMap< + (Public, NumberFor), + VoteMessage, Public, Signature>, + >, session_start: NumberFor, validator_set: ValidatorSet, mandatory_done: bool, @@ -131,55 +134,65 @@ where pub(crate) fn add_vote( &mut self, vote: VoteMessage, AuthorityId, Signature>, - ) -> VoteImportResult { + ) -> VoteImportResult { let num = vote.commitment.block_number; let equivocation_key = (vote.id.clone(), num); + if num < self.session_start || Some(num) <= self.best_done { debug!(target: "beefy", "🥩 received vote for old stale round {:?}, ignoring", num); - VoteImportResult::Stale + return VoteImportResult::Stale } else if vote.commitment.validator_set_id != self.validator_set_id() { debug!( target: "beefy", "🥩 expected set_id {:?}, ignoring vote {:?}.", self.validator_set_id(), vote, ); - VoteImportResult::Invalid + return VoteImportResult::Invalid } else if !self.validators().iter().any(|id| &vote.id == id) { debug!( target: "beefy", "🥩 received vote {:?} from validator that is not in the validator set, ignoring", vote ); - VoteImportResult::Invalid - } else if self - .equivocations + return VoteImportResult::Invalid + } + + if let Some(existing_vote) = self.equivocations.get(&equivocation_key) { // is the same public key voting for a different payload? - .get(&equivocation_key) - .map(|payload| payload != &vote.commitment.payload) - .unwrap_or_else(|| { - // this is the first vote done by `id` for `num` - self.equivocations.insert(equivocation_key, vote.commitment.payload.clone()); - // no equivocation - false - }) { - debug!(target: "beefy", "🥩 detected equivocated vote {:?}", vote); - VoteImportResult::Equivocation + if existing_vote.commitment.payload != vote.commitment.payload { + debug!( + target: "beefy", "🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", + existing_vote, vote + ); + let equivocation = Equivocation { + round_number: equivocation_key.1, + id: equivocation_key.0, + first: existing_vote.clone(), + second: vote, + }; + let set_id = self.validator_set_id(); + return VoteImportResult::Equivocation(EquivocationProof { set_id, equivocation }) + } } else { - let round = self.rounds.entry(vote.commitment.clone()).or_default(); - if round.add_vote((vote.id, vote.signature)) && - round.is_done(threshold(self.validator_set.len())) - { - if let Some(round) = self.rounds.remove(&vote.commitment) { - let votes = round.votes; - let signatures = self - .validators() - .iter() - .map(|authority_id| votes.get(authority_id).cloned()) - .collect(); - return VoteImportResult::OkRoundConcluded(signatures) - } + // this is the first vote sent by `id` for `num`, all good + self.equivocations.insert(equivocation_key, vote.clone()); + } + + // add valid vote + let round = self.rounds.entry(vote.commitment.clone()).or_default(); + if round.add_vote((vote.id, vote.signature)) && + round.is_done(threshold(self.validator_set.len())) + { + if let Some(round) = self.rounds.remove(&vote.commitment) { + let votes = round.votes; + let signatures = self + .validators() + .iter() + .map(|authority_id| votes.get(authority_id).cloned()) + .collect(); + return VoteImportResult::RoundConcluded(signatures) } - VoteImportResult::OkRoundInProgress } + VoteImportResult::Ok } pub(crate) fn conclude(&mut self, round_num: NumberFor) { diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index aca25a329a2e6..3696e5cc95135 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -31,7 +31,7 @@ use crate::{ }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, - Commitment, ConsensusLog, PayloadProvider, SignedCommitment, ValidatorSet, + Commitment, ConsensusLog, EquivocationProof, PayloadProvider, SignedCommitment, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; use codec::{Codec, Decode, Encode}; @@ -73,7 +73,7 @@ pub(crate) enum RoundAction { /// Responsible for the voting strategy. /// It chooses which incoming votes to accept and which votes to generate. /// Keeps track of voting seen for current and future rounds. -#[derive(Debug, Decode, Encode, PartialEq)] +#[derive(Debug, Decode, Encode)] pub(crate) struct VoterOracle { /// Queue of known sessions. Keeps track of voting rounds (block numbers) within each session. /// @@ -256,7 +256,7 @@ pub(crate) struct WorkerParams { pub persisted_state: PersistedState, } -#[derive(Debug, Decode, Encode, PartialEq)] +#[derive(Debug, Decode, Encode)] pub(crate) struct PersistedState { /// Best block we received a GRANDPA finality for. best_grandpa_block_header: ::Header, @@ -549,7 +549,7 @@ where let round = (vote.commitment.payload.clone(), number); match rounds.add_vote(vote) { - VoteImportResult::OkRoundConcluded(signatures) => { + VoteImportResult::RoundConcluded(signatures) => { self.gossip_validator.conclude_round(number); let commitment = Commitment { @@ -568,7 +568,7 @@ where // New state is persisted after finalization. self.finalize(finality_proof)?; }, - VoteImportResult::OkRoundInProgress => { + VoteImportResult::Ok => { let mandatory_round = self .voting_oracle() .mandatory_pending() @@ -582,7 +582,9 @@ where .map_err(|e| Error::Backend(e.to_string()))?; } }, - VoteImportResult::Equivocation => unimplemented!(), + VoteImportResult::Equivocation(proof) => { + self.report_equivocation(proof)?; + }, VoteImportResult::Invalid | VoteImportResult::Stale => (), }; Ok(()) @@ -901,6 +903,18 @@ where } } } + + /// 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( + &self, + _proof: EquivocationProof, AuthorityId, Signature>, + ) -> Result<(), Error> { + todo!() + } } /// Scan the `header` digest log for a BEEFY validator set change. Return either the new diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 5f144ca0754ce..e721fecc680c8 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -183,7 +183,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. -#[derive(Debug, Decode, Encode, TypeInfo)] +#[derive(Clone, Debug, Decode, Encode, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block pub commitment: Commitment, @@ -211,8 +211,10 @@ pub struct Equivocation { /// Proving is achieved by collecting the signed commitments of conflicting votes. #[derive(Debug, Decode, Encode, TypeInfo)] pub struct EquivocationProof { - set_id: ValidatorSetId, - equivocation: Equivocation, + /// BEEFY validator set id active during this equivocation. + pub set_id: ValidatorSetId, + /// Equivocation details including the conflicting votes. + pub equivocation: Equivocation, } /// Check a commitment signature by encoding the commitment and From bbc786a72f9b007a2ebaa81c4388491ee8f2bd07 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 5 Jan 2023 19:22:38 +0200 Subject: [PATCH 09/41] client/beefy: adjust tests for new voting logic --- client/beefy/src/round.rs | 236 +++++++++++++++++++----------------- client/beefy/src/worker.rs | 6 +- primitives/beefy/src/lib.rs | 6 +- 3 files changed, 128 insertions(+), 120 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 68c9ee15f62c4..3d5d77359844f 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -59,7 +59,7 @@ pub fn threshold(authorities: usize) -> usize { authorities - faulty } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum VoteImportResult { Ok, RoundConcluded(Vec>), @@ -72,7 +72,7 @@ pub enum VoteImportResult { /// Only round numbers > `best_done` are of interest, all others are considered stale. /// /// Does not do any validation on votes or signatures, layers above need to handle that (gossip). -#[derive(Debug, Decode, Encode)] +#[derive(Debug, Decode, Encode, PartialEq)] pub(crate) struct Rounds { rounds: BTreeMap>, RoundTracker>, equivocations: BTreeMap<(Public, NumberFor), VoteMessage, Public, Signature>>, @@ -208,16 +208,16 @@ where #[cfg(test)] mod tests { use sc_network_test::Block; - use sp_core::H256; - use beefy_primitives::{crypto::Public, ValidatorSet}; + use beefy_primitives::{ + crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, Payload, ValidatorSet, VoteMessage, + }; - use super::{threshold, Block as BlockT, Hash, RoundTracker, Rounds}; - use crate::keystore::tests::Keyring; + use super::{threshold, Block as BlockT, RoundTracker, Rounds}; + use crate::{keystore::tests::Keyring, round::VoteImportResult}; - impl Rounds + impl Rounds where - P: Ord + Hash + Clone, B: BlockT, { pub(crate) fn test_set_mandatory_done(&mut self, done: bool) { @@ -268,7 +268,7 @@ mod tests { .unwrap(); let session_start = 1u64.into(); - let rounds = Rounds::::new(session_start, validators); + let rounds = Rounds::::new(session_start, validators); assert_eq!(42, rounds.validator_set_id()); assert_eq!(1, rounds.session_start()); @@ -292,45 +292,53 @@ mod tests { Default::default(), ) .unwrap(); - let round = (H256::from_low_u64_le(1), 1); + let validator_set_id = validators.id(); let session_start = 1u64.into(); - let mut rounds = Rounds::::new(session_start, validators); - + let mut rounds = Rounds::::new(session_start, validators); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); + let block_number = 1; + let commitment = Commitment { block_number, payload, validator_set_id }; + let mut vote = VoteMessage { + id: Keyring::Alice.public(), + commitment, + signature: Keyring::Alice.sign(b"I am committed"), + }; // add 1st good vote - assert!(rounds - .add_vote(&round, (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")))); - // round not concluded - assert!(rounds.should_conclude(&round).is_none()); + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); - // double voting not allowed - assert!(!rounds - .add_vote(&round, (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")))); + // double voting (same vote), ok, no effect + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); + vote.id = Keyring::Dave.public(); + vote.signature = Keyring::Dave.sign(b"I am committed"); // invalid vote (Dave is not a validator) - assert!(!rounds - .add_vote(&round, (Keyring::Dave.public(), Keyring::Dave.sign(b"I am committed")),)); - assert!(rounds.should_conclude(&round).is_none()); + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Invalid); + vote.id = Keyring::Bob.public(); + vote.signature = Keyring::Bob.sign(b"I am committed"); // add 2nd good vote - assert!( - rounds.add_vote(&round, (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")),) + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Ok); + + vote.id = Keyring::Charlie.public(); + vote.signature = Keyring::Charlie.sign(b"I am committed"); + // add 3rd good vote -> round concluded -> signatures present + assert_eq!( + rounds.add_vote(vote.clone()), + VoteImportResult::RoundConcluded(vec![ + Some(Keyring::Alice.sign(b"I am committed")), + Some(Keyring::Bob.sign(b"I am committed")), + Some(Keyring::Charlie.sign(b"I am committed")), + None, + ]) ); - // round not concluded - assert!(rounds.should_conclude(&round).is_none()); - - // add 3rd good vote - assert!(rounds.add_vote( - &round, - (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am committed")), - )); - // round concluded - assert!(rounds.should_conclude(&round).is_some()); - rounds.conclude(round.1); + rounds.conclude(block_number); + vote.id = Keyring::Eve.public(); + vote.signature = Keyring::Eve.sign(b"I am committed"); // Eve is a validator, but round was concluded, adding vote disallowed - assert!(!rounds - .add_vote(&round, (Keyring::Eve.public(), Keyring::Eve.sign(b"I am committed")),)); + assert_eq!(rounds.add_vote(vote), VoteImportResult::Stale); } #[test] @@ -342,30 +350,39 @@ mod tests { 42, ) .unwrap(); - let alice = (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")); + let validator_set_id = validators.id(); + // active rounds starts at block 10 let session_start = 10u64.into(); - let mut rounds = Rounds::::new(session_start, validators); - - let mut vote = (H256::from_low_u64_le(1), 9); + let mut rounds = Rounds::::new(session_start, validators); + + // vote on round 9 + let block_number = 9; + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); + let commitment = Commitment { block_number, payload, validator_set_id }; + let mut vote = VoteMessage { + id: Keyring::Alice.public(), + commitment, + signature: Keyring::Alice.sign(b"I am committed"), + }; // add vote for previous session, should fail - assert!(!rounds.add_vote(&vote, alice.clone())); + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Stale); // no votes present assert!(rounds.rounds.is_empty()); // simulate 11 was concluded rounds.best_done = Some(11); // add votes for current session, but already concluded rounds, should fail - vote.1 = 10; - assert!(!rounds.add_vote(&vote, alice.clone())); - vote.1 = 11; - assert!(!rounds.add_vote(&vote, alice.clone())); + vote.commitment.block_number = 10; + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Stale); + vote.commitment.block_number = 11; + assert_eq!(rounds.add_vote(vote.clone()), VoteImportResult::Stale); // no votes present assert!(rounds.rounds.is_empty()); - // add good vote - vote.1 = 12; - assert!(rounds.add_vote(&vote, alice)); + // add vote for active round 12 + vote.commitment.block_number = 12; + assert_eq!(rounds.add_vote(vote), VoteImportResult::Ok); // good vote present assert_eq!(rounds.rounds.len(), 1); } @@ -375,79 +392,70 @@ mod tests { sp_tracing::try_init_simple(); let validators = ValidatorSet::::new( - vec![ - Keyring::Alice.public(), - Keyring::Bob.public(), - Keyring::Charlie.public(), - Keyring::Dave.public(), - ], + vec![Keyring::Alice.public(), Keyring::Bob.public(), Keyring::Charlie.public()], Default::default(), ) .unwrap(); + let validator_set_id = validators.id(); let session_start = 1u64.into(); - let mut rounds = Rounds::::new(session_start, validators); - - // round 1 - assert!(rounds.add_vote( - &(H256::from_low_u64_le(1), 1), - (Keyring::Alice.public(), Keyring::Alice.sign(b"I am committed")), - )); - assert!(rounds.add_vote( - &(H256::from_low_u64_le(1), 1), - (Keyring::Bob.public(), Keyring::Bob.sign(b"I am committed")), - )); - assert!(rounds.add_vote( - &(H256::from_low_u64_le(1), 1), - (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am committed")), - )); - - // round 2 - assert!(rounds.add_vote( - &(H256::from_low_u64_le(2), 2), - (Keyring::Alice.public(), Keyring::Alice.sign(b"I am again committed")), - )); - assert!(rounds.add_vote( - &(H256::from_low_u64_le(2), 2), - (Keyring::Bob.public(), Keyring::Bob.sign(b"I am again committed")), - )); - assert!(rounds.add_vote( - &(H256::from_low_u64_le(2), 2), - (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am again committed")), - )); - - // round 3 - assert!(rounds.add_vote( - &(H256::from_low_u64_le(3), 3), - (Keyring::Alice.public(), Keyring::Alice.sign(b"I am still committed")), - )); - assert!(rounds.add_vote( - &(H256::from_low_u64_le(3), 3), - (Keyring::Bob.public(), Keyring::Bob.sign(b"I am still committed")), - )); - assert!(rounds.add_vote( - &(H256::from_low_u64_le(3), 3), - (Keyring::Charlie.public(), Keyring::Charlie.sign(b"I am still committed")), - )); - assert_eq!(3, rounds.rounds.len()); - - // conclude unknown round - assert!(rounds.should_conclude(&(H256::from_low_u64_le(5), 5)).is_none()); - assert_eq!(3, rounds.rounds.len()); - - // conclude round 2 - let signatures = rounds.should_conclude(&(H256::from_low_u64_le(2), 2)).unwrap(); - rounds.conclude(2); + let mut rounds = Rounds::::new(session_start, validators); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![]); + let commitment = Commitment { block_number: 1, payload, validator_set_id }; + let mut alice_vote = VoteMessage { + id: Keyring::Alice.public(), + commitment: commitment.clone(), + signature: Keyring::Alice.sign(b"I am committed"), + }; + let mut bob_vote = VoteMessage { + id: Keyring::Bob.public(), + commitment: commitment.clone(), + signature: Keyring::Bob.sign(b"I am committed"), + }; + let mut charlie_vote = VoteMessage { + id: Keyring::Charlie.public(), + commitment, + signature: Keyring::Charlie.sign(b"I am committed"), + }; + let expected_signatures = vec![ + Some(Keyring::Alice.sign(b"I am committed")), + Some(Keyring::Bob.sign(b"I am committed")), + Some(Keyring::Charlie.sign(b"I am committed")), + ]; + + // round 1 - only 2 out of 3 vote + assert_eq!(rounds.add_vote(alice_vote.clone()), VoteImportResult::Ok); + assert_eq!(rounds.add_vote(charlie_vote.clone()), VoteImportResult::Ok); + // should be 1 active round assert_eq!(1, rounds.rounds.len()); + // round 2 - only Charlie votes + charlie_vote.commitment.block_number = 2; + assert_eq!(rounds.add_vote(charlie_vote.clone()), VoteImportResult::Ok); + // should be 2 active rounds + assert_eq!(2, rounds.rounds.len()); + + // round 3 - all validators vote -> round is concluded + alice_vote.commitment.block_number = 3; + bob_vote.commitment.block_number = 3; + charlie_vote.commitment.block_number = 3; + assert_eq!(rounds.add_vote(alice_vote.clone()), VoteImportResult::Ok); + assert_eq!(rounds.add_vote(bob_vote.clone()), VoteImportResult::Ok); assert_eq!( - signatures, - vec![ - Some(Keyring::Alice.sign(b"I am again committed")), - Some(Keyring::Bob.sign(b"I am again committed")), - Some(Keyring::Charlie.sign(b"I am again committed")), - None - ] + rounds.add_vote(charlie_vote.clone()), + VoteImportResult::RoundConcluded(expected_signatures) ); + // should be only 2 active since this one auto-concluded + assert_eq!(2, rounds.rounds.len()); + + // conclude round 2 + rounds.conclude(2); + // should be no more active rounds since 2 was officially concluded and round "1" is stale + assert!(rounds.rounds.is_empty()); + + // conclude round 3 + rounds.conclude(3); + assert!(rounds.equivocations.is_empty()); } } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 3696e5cc95135..3333ae1fb1a52 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -73,7 +73,7 @@ pub(crate) enum RoundAction { /// Responsible for the voting strategy. /// It chooses which incoming votes to accept and which votes to generate. /// Keeps track of voting seen for current and future rounds. -#[derive(Debug, Decode, Encode)] +#[derive(Debug, Decode, Encode, PartialEq)] pub(crate) struct VoterOracle { /// Queue of known sessions. Keeps track of voting rounds (block numbers) within each session. /// @@ -256,7 +256,7 @@ pub(crate) struct WorkerParams { pub persisted_state: PersistedState, } -#[derive(Debug, Decode, Encode)] +#[derive(Debug, Decode, Encode, PartialEq)] pub(crate) struct PersistedState { /// Best block we received a GRANDPA finality for. best_grandpa_block_header: ::Header, @@ -984,7 +984,7 @@ pub(crate) mod tests { }, BeefyRPCLinks, KnownPeers, }; - use beefy_primitives::{known_payloads, mmr::MmrRootProvider}; + use beefy_primitives::{known_payloads, mmr::MmrRootProvider, Payload}; use futures::{future::poll_fn, task::Poll}; use parking_lot::Mutex; use sc_client_api::{Backend as BackendT, HeaderBackend}; diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index e721fecc680c8..5ff52d206c5d7 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -183,7 +183,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. -#[derive(Clone, Debug, Decode, Encode, TypeInfo)] +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block pub commitment: Commitment, @@ -194,7 +194,7 @@ pub struct VoteMessage { } /// An equivocation (double-vote) in a given round. -#[derive(Debug, Encode, Decode, TypeInfo)] +#[derive(Debug, Encode, Decode, PartialEq, TypeInfo)] pub struct Equivocation { /// The round number equivocated in pub round_number: Number, @@ -209,7 +209,7 @@ pub struct Equivocation { /// Proof of voter misbehavior on a given set id. Misbehavior/equivocation in /// BEEFY happens when a voter votes on the same round/block for different payloads. /// Proving is achieved by collecting the signed commitments of conflicting votes. -#[derive(Debug, Decode, Encode, TypeInfo)] +#[derive(Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct EquivocationProof { /// BEEFY validator set id active during this equivocation. pub set_id: ValidatorSetId, From d6f6c18ce6a6ca07bdc98dbd3882556b9e704f75 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 5 Jan 2023 19:43:36 +0200 Subject: [PATCH 10/41] sp-beefy: fix commitment ordering and equality --- primitives/beefy/src/commitment.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/primitives/beefy/src/commitment.rs b/primitives/beefy/src/commitment.rs index 5765ff3609dbb..8642a83aae4be 100644 --- a/primitives/beefy/src/commitment.rs +++ b/primitives/beefy/src/commitment.rs @@ -77,6 +77,7 @@ where self.validator_set_id .cmp(&other.validator_set_id) .then_with(|| self.block_number.cmp(&other.block_number)) + .then_with(|| self.payload.cmp(&other.payload)) } } From 495892869a9b63a2821acc2b13ec606e86976773 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 5 Jan 2023 20:10:50 +0200 Subject: [PATCH 11/41] client/beefy: simplify handle_vote() a bit --- client/beefy/src/round.rs | 53 ++++++++++++++++++++++++-------------- client/beefy/src/worker.rs | 34 ++++++++++-------------- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 3d5d77359844f..43eaeeacc3b5e 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -18,7 +18,8 @@ use beefy_primitives::{ crypto::{AuthorityId, Public, Signature}, - Commitment, Equivocation, EquivocationProof, ValidatorSet, ValidatorSetId, VoteMessage, + Commitment, Equivocation, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, + VoteMessage, }; use codec::{Decode, Encode}; use log::debug; @@ -62,7 +63,7 @@ pub fn threshold(authorities: usize) -> usize { #[derive(Debug, PartialEq)] pub enum VoteImportResult { Ok, - RoundConcluded(Vec>), + RoundConcluded(SignedCommitment, Signature>), Equivocation(EquivocationProof, Public, Signature>), Invalid, Stale, @@ -182,19 +183,26 @@ where if round.add_vote((vote.id, vote.signature)) && round.is_done(threshold(self.validator_set.len())) { - if let Some(round) = self.rounds.remove(&vote.commitment) { - let votes = round.votes; - let signatures = self - .validators() - .iter() - .map(|authority_id| votes.get(authority_id).cloned()) - .collect(); - return VoteImportResult::RoundConcluded(signatures) + if let Some(round) = self.rounds.remove_entry(&vote.commitment) { + return VoteImportResult::RoundConcluded(self.signed_commitment(round)) } } VoteImportResult::Ok } + fn signed_commitment( + &mut self, + round: (Commitment>, RoundTracker), + ) -> SignedCommitment, Signature> { + let votes = round.1.votes; + let signatures = self + .validators() + .iter() + .map(|authority_id| votes.get(authority_id).cloned()) + .collect(); + SignedCommitment { commitment: round.0, signatures } + } + pub(crate) fn conclude(&mut self, round_num: NumberFor) { // Remove this and older (now stale) rounds. self.rounds.retain(|commitment, _| commitment.block_number > round_num); @@ -210,7 +218,8 @@ mod tests { use sc_network_test::Block; use beefy_primitives::{ - crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, Payload, ValidatorSet, VoteMessage, + crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, Payload, SignedCommitment, + ValidatorSet, VoteMessage, }; use super::{threshold, Block as BlockT, RoundTracker, Rounds}; @@ -302,7 +311,7 @@ mod tests { let commitment = Commitment { block_number, payload, validator_set_id }; let mut vote = VoteMessage { id: Keyring::Alice.public(), - commitment, + commitment: commitment.clone(), signature: Keyring::Alice.sign(b"I am committed"), }; // add 1st good vote @@ -326,12 +335,15 @@ mod tests { // add 3rd good vote -> round concluded -> signatures present assert_eq!( rounds.add_vote(vote.clone()), - VoteImportResult::RoundConcluded(vec![ - Some(Keyring::Alice.sign(b"I am committed")), - Some(Keyring::Bob.sign(b"I am committed")), - Some(Keyring::Charlie.sign(b"I am committed")), - None, - ]) + VoteImportResult::RoundConcluded(SignedCommitment { + commitment, + signatures: vec![ + Some(Keyring::Alice.sign(b"I am committed")), + Some(Keyring::Bob.sign(b"I am committed")), + Some(Keyring::Charlie.sign(b"I am committed")), + None, + ] + }) ); rounds.conclude(block_number); @@ -444,7 +456,10 @@ mod tests { assert_eq!(rounds.add_vote(bob_vote.clone()), VoteImportResult::Ok); assert_eq!( rounds.add_vote(charlie_vote.clone()), - VoteImportResult::RoundConcluded(expected_signatures) + VoteImportResult::RoundConcluded(SignedCommitment { + commitment: charlie_vote.commitment, + signatures: expected_signatures + }) ); // should be only 2 active since this one auto-concluded assert_eq!(2, rounds.rounds.len()); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 3333ae1fb1a52..b1d9201945504 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -31,7 +31,7 @@ use crate::{ }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, - Commitment, ConsensusLog, EquivocationProof, PayloadProvider, SignedCommitment, ValidatorSet, + Commitment, ConsensusLog, EquivocationProof, PayloadProvider, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; use codec::{Codec, Decode, Encode}; @@ -545,25 +545,17 @@ where .active_rounds_mut() .ok_or(Error::UninitSession)?; - let number = vote.commitment.block_number; - let round = (vote.commitment.payload.clone(), number); - + let block_number = vote.commitment.block_number; match rounds.add_vote(vote) { - VoteImportResult::RoundConcluded(signatures) => { - self.gossip_validator.conclude_round(number); - - let commitment = Commitment { - payload: round.0, - block_number: number, - validator_set_id: rounds.validator_set_id(), - }; - let finality_proof = - VersionedFinalityProof::V1(SignedCommitment { commitment, signatures }); - - metric_set!(self, beefy_round_concluded, number); - - info!(target: "beefy", "🥩 Round #{} concluded, finality_proof: {:?}.", number, finality_proof); - + VoteImportResult::RoundConcluded(signed_commitment) => { + self.gossip_validator.conclude_round(block_number); + metric_set!(self, beefy_round_concluded, block_number); + + let finality_proof = VersionedFinalityProof::V1(signed_commitment); + info!( + target: "beefy", "🥩 Round #{} concluded, finality_proof: {:?}.", + block_number, finality_proof + ); // We created the `finality_proof` and know to be valid. // New state is persisted after finalization. self.finalize(finality_proof)?; @@ -572,7 +564,7 @@ where let mandatory_round = self .voting_oracle() .mandatory_pending() - .map(|(mandatory_num, _)| mandatory_num == number) + .map(|(mandatory_num, _)| mandatory_num == block_number) .unwrap_or(false); // Persist state after handling self vote to avoid double voting in case // of voter restarts. @@ -984,7 +976,7 @@ pub(crate) mod tests { }, BeefyRPCLinks, KnownPeers, }; - use beefy_primitives::{known_payloads, mmr::MmrRootProvider, Payload}; + use beefy_primitives::{known_payloads, mmr::MmrRootProvider, Payload, SignedCommitment}; use futures::{future::poll_fn, task::Poll}; use parking_lot::Mutex; use sc_client_api::{Backend as BackendT, HeaderBackend}; From acfcb1fe98da685f5960a463f581b573787a3a21 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 6 Jan 2023 18:43:35 +0200 Subject: [PATCH 12/41] client/beefy: add simple equivocation test --- client/beefy/src/round.rs | 47 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 43eaeeacc3b5e..4ba769b3db335 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -218,8 +218,8 @@ mod tests { use sc_network_test::Block; use beefy_primitives::{ - crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, Payload, SignedCommitment, - ValidatorSet, VoteMessage, + crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, Equivocation, EquivocationProof, + Payload, SignedCommitment, ValidatorSet, VoteMessage, }; use super::{threshold, Block as BlockT, RoundTracker, Rounds}; @@ -473,4 +473,47 @@ mod tests { rounds.conclude(3); assert!(rounds.equivocations.is_empty()); } + + #[test] + fn should_provide_equivocation_proof() { + sp_tracing::try_init_simple(); + + let validators = ValidatorSet::::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::::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 { + set_id: validator_set_id, + equivocation: Equivocation { + id: Keyring::Alice.public(), + round_number: 1, + 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); + } } From 8c0d67b9030bdf45a6b2cc83a0fee701f176d3e2 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 9 Jan 2023 14:22:40 +0200 Subject: [PATCH 13/41] client/beefy: submit equivocation proof - WIP --- client/beefy/src/error.rs | 4 ++- client/beefy/src/worker.rs | 60 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/client/beefy/src/error.rs b/client/beefy/src/error.rs index dd5fd649d52ce..bdb04c434785e 100644 --- a/client/beefy/src/error.rs +++ b/client/beefy/src/error.rs @@ -22,12 +22,14 @@ 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")] diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index b1d9201945504..6b0f4d8cd6a03 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -903,9 +903,65 @@ where /// isn't necessarily the best block if there are pending authority set changes. pub(crate) fn report_equivocation( &self, - _proof: EquivocationProof, AuthorityId, Signature>, + proof: EquivocationProof, AuthorityId, Signature>, ) -> Result<(), Error> { - todo!() + let rounds = + self.persisted_state.voting_oracle.active_rounds().ok_or(Error::UninitSession)?; + let (validators, validator_set_id) = (rounds.validators(), rounds.validator_set_id()); + + if proof.set_id != validator_set_id { + debug!(target: "beefy", "🥩 Skip equivocation report for old set id: {:?}", proof.set_id); + return Ok(()) + } else if let Some(local_id) = self.key_store.authority_id(validators) { + if proof.equivocation.id == local_id { + debug!(target: "beefy", "🥩 Skip equivocation report for own equivocation"); + return Ok(()) + } + } + + let number = proof.equivocation.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 equivocation..", + number, err + ); + error!(target: "beefy", "🥩 {}", err_msg); + Error::Backend(err_msg) + })?; + // generate key ownership proof at that block + // let key_owner_proof = match self + // .runtime + // .runtime_api() + // .generate_key_ownership_proof( + // &BlockId::Hash(hash), + // validator_set_id, + // proof.equivocation.id.clone(), + // ) + // .map_err(Error::RuntimeApi)? + // { + // Some(proof) => proof, + // None => { + // debug!(target: "beefy", "🥩 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; + // self.client + // .runtime_api() + // .submit_report_equivocation_unsigned_extrinsic( + // &BlockId::Hash(best_block_hash), + // proof, + // key_owner_proof, + // ) + // .map_err(Error::RuntimeApi)?; + + Ok(()) } } From 71140743421108ee72444e5410cb62ff0a6ab81a Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 9 Jan 2023 18:23:58 +0200 Subject: [PATCH 14/41] frame/beefy: add equivocation report runtime api - part 1 --- Cargo.lock | 1 + frame/beefy/Cargo.toml | 2 + frame/beefy/src/lib.rs | 142 ++++++++++++++++++++++++++++++++++-- primitives/beefy/src/lib.rs | 58 ++++++++++++++- 4 files changed, 192 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f82343e442a89..d84f13c2cae50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4936,6 +4936,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "sp-session", "sp-staking", "sp-std", ] diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index 707e8e25712ab..91c6057c0b049 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -17,6 +17,7 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../su frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } pallet-session = { version = "4.0.0-dev", default-features = false, path = "../session" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" } +sp-session = { version = "4.0.0-dev", default-features = false, path = "../../primitives/session" } sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" } [dev-dependencies] @@ -35,6 +36,7 @@ std = [ "scale-info/std", "serde", "sp-runtime/std", + "sp-session/std", "sp-std/std", ] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index 4cb23107e7843..aa740d0a26be6 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -20,21 +20,25 @@ use codec::{Encode, MaxEncodedLen}; use frame_support::{ + dispatch::{DispatchResultWithPostInfo, Pays}, log, traits::{Get, OneSessionHandler}, + weights::Weight, BoundedSlice, BoundedVec, Parameter, }; +use frame_system::pallet_prelude::BlockNumberFor; use sp_runtime::{ generic::DigestItem, traits::{IsMember, Member}, - RuntimeAppPublic, + KeyTypeId, RuntimeAppPublic, }; +use sp_session::{GetSessionNumber, GetValidatorCount}; use sp_std::prelude::*; use beefy_primitives::{ - AuthorityIndex, ConsensusLog, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, - GENESIS_AUTHORITY_SET_ID, + AuthorityIndex, ConsensusLog, EquivocationProof, OnNewValidatorSet, OpaqueKeyOwnershipProof, + ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; #[cfg(test)] @@ -45,10 +49,13 @@ mod tests; pub use pallet::*; +const LOG_TARGET: &str = "runtime::beefy"; + #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::pallet_prelude::*; + use frame_support::{pallet_prelude::*, traits::KeyOwnerProofSystem}; + use frame_system::{ensure_signed, pallet_prelude::OriginFor}; #[pallet::config] pub trait Config: frame_system::Config { @@ -59,6 +66,22 @@ pub mod pallet { + MaybeSerializeDeserialize + MaxEncodedLen; + /// The proof of key ownership, used for validating equivocation reports + /// The proof must include the session index and validator count of the + /// session at which the equivocation occurred. + type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount; + + /// The identification of a key owner, used when reporting equivocations. + type KeyOwnerIdentification: Parameter; + + /// A system for proving ownership of keys, i.e. that a given key was part + /// of a validator set, needed for validating equivocation reports. + type KeyOwnerProofSystem: KeyOwnerProofSystem< + (KeyTypeId, Self::BeefyId), + Proof = Self::KeyOwnerProof, + IdentificationTuple = Self::KeyOwnerIdentification, + >; + /// The maximum number of authorities that can be added. type MaxAuthorities: Get; @@ -68,6 +91,9 @@ pub mod pallet { /// externally apart from having it in the storage. For instance you might cache a light /// weight MMR root over validators and make it available for Light Clients. type OnNewValidatorSet: OnNewValidatorSet<::BeefyId>; + + /// Weights for this pallet. + type WeightInfo: WeightInfo; } #[pallet::pallet] @@ -112,6 +138,58 @@ pub mod pallet { .expect("Authorities vec too big"); } } + + #[pallet::error] + pub enum Error { + /// A key ownership proof provided as part of an equivocation report is invalid. + InvalidKeyOwnershipProof, + /// An equivocation proof provided as part of an equivocation report is invalid. + InvalidEquivocationProof, + /// A given equivocation report is valid but already previously reported. + DuplicateOffenceReport, + } + + #[pallet::call] + impl Pallet { + /// Report voter equivocation/misbehavior. This method will verify the + /// equivocation proof and validate the given key ownership proof + /// against the extracted offender. If both are valid, the offence + /// will be reported. + #[pallet::call_index(0)] + #[pallet::weight(T::WeightInfo::report_equivocation(key_owner_proof.validator_count()))] + pub fn report_equivocation( + origin: OriginFor, + equivocation_proof: Box< + EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + let reporter = ensure_signed(origin)?; + + Self::do_report_equivocation(Some(reporter), *equivocation_proof, key_owner_proof) + } + + /// TODO + #[pallet::call_index(1)] + #[pallet::weight(T::WeightInfo::report_equivocation(key_owner_proof.validator_count()))] + pub fn report_equivocation_unsigned( + origin: OriginFor, + equivocation_proof: Box< + EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + todo!() + } + } } impl Pallet { @@ -122,6 +200,33 @@ impl Pallet { ValidatorSet::::new(validators, id) } + /// Submits an extrinsic to report an equivocation. This method will create + /// an unsigned extrinsic with a call to `report_equivocation_unsigned` and + /// will push the transaction to the pool. Only useful in an offchain context. + pub fn submit_unsigned_equivocation_report( + equivocation_proof: EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + key_owner_proof: OpaqueKeyOwnershipProof, + ) -> Option<()> { + // TODO: + // use frame_system::offchain::SubmitTransaction; + // + // let call = Call::report_equivocation_unsigned { + // equivocation_proof: Box::new(equivocation_proof), + // key_owner_proof, + // }; + // + // match SubmitTransaction::>::submit_unsigned_transaction(call.into()) { + // Ok(()) => log::info!(target: LOG_TARGET, "Submitted BEEFY equivocation report."), + // Err(e) => log::error!(target: LOG_TARGET, "Error submitting equivocation: {:?}", e), + // } + + Some(()) + } + fn change_authorities( new: BoundedVec, queued: BoundedVec, @@ -182,6 +287,21 @@ impl Pallet { } Ok(()) } + + fn do_report_equivocation( + reporter: Option, + equivocation_proof: EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + todo!(); + + // waive the fee since the report is valid and beneficial + Ok(Pays::No.into()) + } } impl sp_runtime::BoundToRuntimeAppPublic for Pallet { @@ -208,9 +328,10 @@ impl OneSessionHandler for Pallet { let next_authorities = validators.map(|(_, k)| k).collect::>(); if next_authorities.len() as u32 > T::MaxAuthorities::get() { log::error!( - target: "runtime::beefy", + target: LOG_TARGET, "authorities list {:?} truncated to length {}", - next_authorities, T::MaxAuthorities::get(), + next_authorities, + T::MaxAuthorities::get(), ); } let bounded_next_authorities = @@ -219,9 +340,10 @@ impl OneSessionHandler for Pallet { let next_queued_authorities = queued_validators.map(|(_, k)| k).collect::>(); if next_queued_authorities.len() as u32 > T::MaxAuthorities::get() { log::error!( - target: "runtime::beefy", + target: LOG_TARGET, "queued authorities list {:?} truncated to length {}", - next_queued_authorities, T::MaxAuthorities::get(), + next_queued_authorities, + T::MaxAuthorities::get(), ); } let bounded_next_queued_authorities = @@ -247,3 +369,7 @@ impl IsMember for Pallet { Self::authorities().iter().any(|id| id == authority_id) } } + +pub trait WeightInfo { + fn report_equivocation(validator_count: u32) -> Weight; +} diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 5ff52d206c5d7..967892a4fcc3d 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -43,7 +43,7 @@ use codec::{Codec, Decode, Encode}; use scale_info::TypeInfo; use sp_application_crypto::RuntimeAppPublic; use sp_core::H256; -use sp_runtime::traits::Hash; +use sp_runtime::traits::{Hash, NumberFor}; use sp_std::prelude::*; /// Key type for BEEFY module. @@ -194,7 +194,7 @@ pub struct VoteMessage { } /// An equivocation (double-vote) in a given round. -#[derive(Debug, Encode, Decode, PartialEq, TypeInfo)] +#[derive(Clone, Debug, Encode, Decode, PartialEq, TypeInfo)] pub struct Equivocation { /// The round number equivocated in pub round_number: Number, @@ -209,7 +209,7 @@ pub struct Equivocation { /// Proof of voter misbehavior on a given set id. Misbehavior/equivocation in /// BEEFY happens when a voter votes on the same round/block for different payloads. /// Proving is achieved by collecting the signed commitments of conflicting votes. -#[derive(Debug, Decode, Encode, PartialEq, TypeInfo)] +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct EquivocationProof { /// BEEFY validator set id active during this equivocation. pub set_id: ValidatorSetId, @@ -277,12 +277,64 @@ impl OnNewValidatorSet for () { fn on_new_validator_set(_: &ValidatorSet, _: &ValidatorSet) {} } +/// An opaque type used to represent the key ownership proof at the runtime API +/// boundary. The inner value is an encoded representation of the actual key +/// ownership proof which will be parameterized when defining the runtime. At +/// the runtime API boundary this type is unknown and as such we keep this +/// opaque representation, implementors of the runtime API will have to make +/// sure that all usages of `OpaqueKeyOwnershipProof` refer to the same type. +#[derive(Decode, Encode, PartialEq)] +pub struct OpaqueKeyOwnershipProof(Vec); +impl OpaqueKeyOwnershipProof { + /// Create a new `OpaqueKeyOwnershipProof` using the given encoded + /// representation. + pub fn new(inner: Vec) -> OpaqueKeyOwnershipProof { + OpaqueKeyOwnershipProof(inner) + } + + /// Try to decode this `OpaqueKeyOwnershipProof` into the given concrete key + /// ownership proof type. + pub fn decode(self) -> Option { + codec::Decode::decode(&mut &self.0[..]).ok() + } +} + sp_api::decl_runtime_apis! { /// API necessary for BEEFY voters. pub trait BeefyApi { /// Return the current active BEEFY validator set fn validator_set() -> Option>; + + /// Submits an unsigned extrinsic to report an equivocation. The caller + /// must provide the equivocation proof and a key ownership proof + /// (should be obtained using `generate_key_ownership_proof`). The + /// extrinsic will be unsigned and should only be accepted for local + /// authorship (not to be broadcast to the network). This method returns + /// `None` when creation of the extrinsic fails, e.g. if equivocation + /// reporting is disabled for the given runtime (i.e. this method is + /// hardcoded to return `None`). Only useful in an offchain context. + fn submit_report_equivocation_unsigned_extrinsic( + equivocation_proof: + EquivocationProof, crypto::AuthorityId, crypto::Signature>, + key_owner_proof: OpaqueKeyOwnershipProof, + ) -> Option<()>; + + /// Generates a proof of key ownership for the given authority in the + /// given set. An example usage of this module is coupled with the + /// session historical module to prove that a given authority key is + /// tied to a given staking identity during a specific session. Proofs + /// of key ownership are necessary for submitting equivocation reports. + /// NOTE: even though the API takes a `set_id` as parameter the current + /// implementations ignores this parameter and instead relies on this + /// method being called at the correct block height, i.e. any point at + /// which the given set id is live on-chain. Future implementations will + /// instead use indexed data through an offchain worker, not requiring + /// older states to be available. + fn generate_key_ownership_proof( + set_id: ValidatorSetId, + authority_id: crypto::AuthorityId, + ) -> Option; } } From 3219c2fdc81c36dcb1272b731cf9ed494375f3c0 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 16:30:53 +0200 Subject: [PATCH 15/41] frame/beefy: report equivocation logic - part 2 --- client/beefy/src/keystore.rs | 4 ++-- frame/beefy/Cargo.toml | 2 ++ frame/beefy/src/lib.rs | 33 +++++++++++++++++++++++---- primitives/beefy/src/lib.rs | 44 +++++++++++++----------------------- 4 files changed, 49 insertions(+), 34 deletions(-) diff --git a/client/beefy/src/keystore.rs b/client/beefy/src/keystore.rs index 886c00fc5d817..8d77410746e52 100644 --- a/client/beefy/src/keystore.rs +++ b/client/beefy/src/keystore.rs @@ -25,7 +25,7 @@ use log::warn; use beefy_primitives::{ crypto::{Public, Signature}, - BeefyVerify, KEY_TYPE, + BeefyAuthorityId, KEY_TYPE, }; use crate::error; @@ -99,7 +99,7 @@ impl BeefyKeystore { /// /// Return `true` if the signature is authentic, `false` otherwise. pub fn verify(public: &Public, sig: &Signature, message: &[u8]) -> bool { - BeefyVerify::::verify(sig, message, public) + BeefyAuthorityId::::verify(public, sig, message) } } diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index 91c6057c0b049..a2b66b27916f3 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -18,6 +18,7 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys pallet-session = { version = "4.0.0-dev", default-features = false, path = "../session" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" } sp-session = { version = "4.0.0-dev", default-features = false, path = "../../primitives/session" } +sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../primitives/staking" } sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" } [dev-dependencies] @@ -37,6 +38,7 @@ std = [ "serde", "sp-runtime/std", "sp-session/std", + "sp-staking/std", "sp-std/std", ] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index aa740d0a26be6..529378f8b4547 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -22,7 +22,7 @@ use codec::{Encode, MaxEncodedLen}; use frame_support::{ dispatch::{DispatchResultWithPostInfo, Pays}, log, - traits::{Get, OneSessionHandler}, + traits::{Get, KeyOwnerProofSystem, OneSessionHandler}, weights::Weight, BoundedSlice, BoundedVec, Parameter, }; @@ -34,11 +34,12 @@ use sp_runtime::{ KeyTypeId, RuntimeAppPublic, }; use sp_session::{GetSessionNumber, GetValidatorCount}; +use sp_staking::offence::ReportOffence; use sp_std::prelude::*; use beefy_primitives::{ - AuthorityIndex, ConsensusLog, EquivocationProof, OnNewValidatorSet, OpaqueKeyOwnershipProof, - ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, + AuthorityIndex, BeefyAuthorityId, ConsensusLog, EquivocationProof, OnNewValidatorSet, + OpaqueKeyOwnershipProof, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; #[cfg(test)] @@ -62,7 +63,8 @@ pub mod pallet { /// Authority identifier type type BeefyId: Member + Parameter - + RuntimeAppPublic + // todo: use custom hashing type + + BeefyAuthorityId + MaybeSerializeDeserialize + MaxEncodedLen; @@ -297,6 +299,29 @@ impl Pallet { >, key_owner_proof: T::KeyOwnerProof, ) -> DispatchResultWithPostInfo { + // We check the equivocation within the context of its set id (and + // associated session) and round. We also need to know the validator + // set count at the time of the offence since it is required to calculate + // the slash amount. + let set_id = equivocation_proof.set_id; + let round = equivocation_proof.equivocation.round_number; + let offender_id = equivocation_proof.equivocation.id.clone(); + let session_index = key_owner_proof.session(); + let validator_count = key_owner_proof.validator_count(); + + // validate the key ownership proof extracting the id of the offender. + let offender = T::KeyOwnerProofSystem::check_proof( + (beefy_primitives::KEY_TYPE, offender_id), + key_owner_proof, + ) + .ok_or(Error::::InvalidKeyOwnershipProof)?; + + // validate equivocation proof (check votes are different and signatures are valid). + if !beefy_primitives::check_equivocation_proof(equivocation_proof) { + return Err(Error::::InvalidEquivocationProof.into()) + } + + // todo: ReportOffence todo!(); // waive the fee since the report is valid and beneficial diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 967892a4fcc3d..c83a4733a46f7 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -49,20 +49,14 @@ use sp_std::prelude::*; /// Key type for BEEFY module. pub const KEY_TYPE: sp_application_crypto::KeyTypeId = sp_application_crypto::KeyTypeId(*b"beef"); -/// Trait representing BEEFY authority id. -pub trait BeefyAuthorityId: RuntimeAppPublic {} - -/// Means of verification for a BEEFY authority signature. +/// Trait representing BEEFY authority id, including custom signature verification. /// /// Accepts custom hashing fn for the message and custom convertor fn for the signer. -pub trait BeefyVerify { - /// Type of the signer. - type Signer: BeefyAuthorityId; - +pub trait BeefyAuthorityId: RuntimeAppPublic { /// Verify a signature. /// - /// Return `true` if signature is valid for the value. - fn verify(&self, msg: &[u8], signer: &Self::Signer) -> bool; + /// Return `true` if signature over `msg` is valid for this id. + fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool; } /// BEEFY cryptographic types @@ -78,7 +72,7 @@ pub trait BeefyVerify { /// The current underlying crypto scheme used is ECDSA. This can be changed, /// without affecting code restricted against the above listed crypto types. pub mod crypto { - use super::{BeefyAuthorityId, BeefyVerify, Hash}; + use super::{BeefyAuthorityId, Hash, RuntimeAppPublic}; use sp_application_crypto::{app_crypto, ecdsa}; use sp_core::crypto::Wraps; app_crypto!(ecdsa, crate::KEY_TYPE); @@ -89,21 +83,17 @@ pub mod crypto { /// Signature for a BEEFY authority using ECDSA as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId {} - - impl BeefyVerify for AuthoritySignature + impl BeefyAuthorityId for AuthorityId where ::Output: Into<[u8; 32]>, { - type Signer = AuthorityId; - - fn verify(&self, msg: &[u8], signer: &Self::Signer) -> bool { + fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { let msg_hash = ::hash(msg).into(); match sp_io::crypto::secp256k1_ecdsa_recover_compressed( - self.as_inner_ref().as_ref(), + signature.as_inner_ref().as_ref(), &msg_hash, ) { - Ok(raw_pubkey) => raw_pubkey.as_ref() == AsRef::<[u8]>::as_ref(signer), + Ok(raw_pubkey) => raw_pubkey.as_ref() == AsRef::<[u8]>::as_ref(self), _ => false, } } @@ -219,29 +209,27 @@ pub struct EquivocationProof { /// Check a commitment signature by encoding the commitment and /// verifying the provided signature using the expected authority id. -pub fn check_commitment_signature( +pub fn check_commitment_signature( commitment: &Commitment, authority_id: &Id, - signature: &Signature, + signature: &::Signature, ) -> bool where - Id: BeefyAuthorityId, - Signature: BeefyVerify, + Id: BeefyAuthorityId, Number: Clone + Encode + PartialEq, MsgHash: Hash, { let encoded_commitment = commitment.encode(); - BeefyVerify::::verify(signature, &encoded_commitment, authority_id) + BeefyAuthorityId::::verify(authority_id, signature, &encoded_commitment) } /// Verifies the equivocation proof by making sure that both votes target /// different blocks and that its signatures are valid. -pub fn check_equivocation_proof( - report: EquivocationProof, +pub fn check_equivocation_proof( + report: EquivocationProof::Signature>, ) -> bool where - Id: BeefyAuthorityId + PartialEq, - Signature: BeefyVerify, + Id: BeefyAuthorityId + PartialEq, Number: Clone + Encode + PartialEq, MsgHash: Hash, { From 9278bf0324a80a527156df52c4a62f31ba6ca867 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 17:22:40 +0200 Subject: [PATCH 16/41] frame/beefy: add pluggable Equivocation handler - part 3 --- Cargo.lock | 1 + frame/beefy/Cargo.toml | 2 + frame/beefy/src/equivocation.rs | 385 ++++++++++++++++++++++++++++++++ frame/beefy/src/lib.rs | 29 ++- 4 files changed, 413 insertions(+), 4 deletions(-) create mode 100644 frame/beefy/src/equivocation.rs diff --git a/Cargo.lock b/Cargo.lock index d84f13c2cae50..cc96b7748dc9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4928,6 +4928,7 @@ version = "4.0.0-dev" dependencies = [ "frame-support", "frame-system", + "pallet-authorship", "pallet-session", "parity-scale-codec", "scale-info", diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index a2b66b27916f3..4b3c4b4884530 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -15,6 +15,7 @@ serde = { version = "1.0.136", optional = true } beefy-primitives = { version = "4.0.0-dev", default-features = false, path = "../../primitives/beefy", package = "sp-beefy" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } +pallet-authorship = { version = "4.0.0-dev", default-features = false, path = "../authorship" } pallet-session = { version = "4.0.0-dev", default-features = false, path = "../session" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" } sp-session = { version = "4.0.0-dev", default-features = false, path = "../../primitives/session" } @@ -33,6 +34,7 @@ std = [ "codec/std", "frame-support/std", "frame-system/std", + "pallet-authorship/std", "pallet-session/std", "scale-info/std", "serde", diff --git a/frame/beefy/src/equivocation.rs b/frame/beefy/src/equivocation.rs new file mode 100644 index 0000000000000..42c9b38b4b8cb --- /dev/null +++ b/frame/beefy/src/equivocation.rs @@ -0,0 +1,385 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! An opt-in utility module for reporting equivocations. +//! +//! This module defines an offence type for BEEFY equivocations +//! and some utility traits to wire together: +//! - a key ownership proof system (e.g. to prove that a given authority was part of a session); +//! - a system for reporting offences; +//! - a system for signing and submitting transactions; +//! - a way to get the current block author; +//! +//! These can be used in an offchain context in order to submit equivocation +//! reporting extrinsics (from the client that's running the BEEFY protocol). +//! And in a runtime context, so that the BEEFY pallet can validate the +//! equivocation proofs in the extrinsic and report the offences. +//! +//! IMPORTANT: +//! When using this module for enabling equivocation reporting it is required +//! that the `ValidateUnsigned` for the BEEFY pallet is used in the runtime +//! definition. + +use sp_std::prelude::*; + +use beefy_primitives::{EquivocationProof, ValidatorSetId}; +use codec::{self as codec, Decode, Encode}; +use frame_support::{ + log, + traits::{Get, KeyOwnerProofSystem}, +}; +use frame_system::pallet_prelude::BlockNumberFor; +use sp_runtime::{ + transaction_validity::{ + InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, + TransactionValidityError, ValidTransaction, + }, + DispatchResult, Perbill, RuntimeAppPublic, +}; +use sp_staking::{ + offence::{Kind, Offence, OffenceError, ReportOffence}, + SessionIndex, +}; + +use super::{Call, Config, Pallet, LOG_TARGET}; + +/// A trait with utility methods for handling equivocation reports in BEEFY. +/// The offence type is generic, and the trait provides, reporting an offence +/// triggered by a valid equivocation report, and also for creating and +/// submitting equivocation report extrinsics (useful only in offchain context). +pub trait HandleEquivocation { + /// The offence type used for reporting offences on valid equivocation reports. + type Offence: BeefyOffence, T::KeyOwnerIdentification>; + + /// The longevity, in blocks, that the equivocation report is valid for. When using the staking + /// pallet this should be equal to the bonding duration (in blocks, not eras). + type ReportLongevity: Get; + + /// Report an offence proved by the given reporters. + fn report_offence( + reporters: Vec, + offence: Self::Offence, + ) -> Result<(), OffenceError>; + + /// Returns true if all of the offenders at the given time slot have already been reported. + fn is_known_offence( + offenders: &[T::KeyOwnerIdentification], + time_slot: &>::TimeSlot, + ) -> bool; + + /// Create and dispatch an equivocation report extrinsic. + fn submit_unsigned_equivocation_report( + equivocation_proof: EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResult; + + /// Fetch the current block author id, if defined. + fn block_author() -> Option; +} + +impl HandleEquivocation for () { + type Offence = BeefyEquivocationOffence, T::KeyOwnerIdentification>; + type ReportLongevity = (); + + fn report_offence( + _reporters: Vec, + _offence: BeefyEquivocationOffence, T::KeyOwnerIdentification>, + ) -> Result<(), OffenceError> { + Ok(()) + } + + fn is_known_offence( + _offenders: &[T::KeyOwnerIdentification], + _time_slot: &BeefyTimeSlot>, + ) -> bool { + true + } + + fn submit_unsigned_equivocation_report( + _equivocation_proof: EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + _key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResult { + Ok(()) + } + + fn block_author() -> Option { + None + } +} + +/// Generic equivocation handler. This type implements `HandleEquivocation` +/// using existing subsystems that are part of frame (type bounds described +/// below) and will dispatch to them directly, it's only purpose is to wire all +/// subsystems together. +pub struct EquivocationHandler> { + _phantom: sp_std::marker::PhantomData<(N, I, R, L, O)>, +} + +impl Default for EquivocationHandler { + fn default() -> Self { + Self { _phantom: Default::default() } + } +} + +impl HandleEquivocation + for EquivocationHandler, T::KeyOwnerIdentification, R, L, O> +where + // We use the authorship pallet to fetch the current block author and use + // `offchain::SendTransactionTypes` for unsigned extrinsic creation and + // submission. + T: Config + pallet_authorship::Config + frame_system::offchain::SendTransactionTypes>, + // A system for reporting offences after valid equivocation reports are + // processed. + R: ReportOffence, + // The longevity (in blocks) that the equivocation report is valid for. When using the staking + // pallet this should be the bonding duration. + L: Get, + // The offence type that should be used when reporting. + O: BeefyOffence, T::KeyOwnerIdentification>, +{ + type Offence = O; + type ReportLongevity = L; + + fn report_offence(reporters: Vec, offence: O) -> Result<(), OffenceError> { + R::report_offence(reporters, offence) + } + + fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &O::TimeSlot) -> bool { + R::is_known_offence(offenders, time_slot) + } + + fn submit_unsigned_equivocation_report( + equivocation_proof: EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResult { + use frame_system::offchain::SubmitTransaction; + + let call = Call::report_equivocation_unsigned { + equivocation_proof: Box::new(equivocation_proof), + key_owner_proof, + }; + + match SubmitTransaction::>::submit_unsigned_transaction(call.into()) { + Ok(()) => log::info!(target: LOG_TARGET, "Submitted BEEFY equivocation report.",), + Err(e) => + log::error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e,), + } + + Ok(()) + } + + fn block_author() -> Option { + >::author() + } +} + +/// A round number and set id which point on the time of an offence. +#[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Encode, Decode)] +pub struct BeefyTimeSlot { + // The order of these matters for `derive(Ord)`. + /// BEEFY Set ID. + pub set_id: ValidatorSetId, + /// Round number. + pub round: N, +} + +/// Methods for the `ValidateUnsigned` implementation: +/// It restricts calls to `report_equivocation_unsigned` to local calls (i.e. extrinsics generated +/// on this node) or that already in a block. This guarantees that only block authors can include +/// unsigned equivocation reports. +impl Pallet { + pub fn validate_unsigned(source: TransactionSource, call: &Call) -> TransactionValidity { + if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call { + // discard equivocation report not coming from the local node + match source { + TransactionSource::Local | TransactionSource::InBlock => { /* allowed */ }, + _ => { + log::warn!( + target: LOG_TARGET, + "rejecting unsigned report equivocation transaction because it is not local/in-block." + ); + + return InvalidTransaction::Call.into() + }, + } + + // check report staleness + is_known_offence::(equivocation_proof, key_owner_proof)?; + + let longevity = + >::ReportLongevity::get(); + + ValidTransaction::with_tag_prefix("BeefyEquivocation") + // We assign the maximum priority for any equivocation report. + .priority(TransactionPriority::MAX) + // Only one equivocation report for the same offender at the same slot. + .and_provides(( + equivocation_proof.equivocation.id.clone(), + equivocation_proof.set_id, + equivocation_proof.equivocation.round_number, + )) + .longevity(longevity) + // We don't propagate this. This can never be included on a remote node. + .propagate(false) + .build() + } else { + InvalidTransaction::Call.into() + } + } + + pub fn pre_dispatch(call: &Call) -> Result<(), TransactionValidityError> { + if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call { + is_known_offence::(equivocation_proof, key_owner_proof) + } else { + Err(InvalidTransaction::Call.into()) + } + } +} + +fn is_known_offence( + equivocation_proof: &EquivocationProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + key_owner_proof: &T::KeyOwnerProof, +) -> Result<(), TransactionValidityError> { + // check the membership proof to extract the offender's id + let key = (beefy_primitives::KEY_TYPE, equivocation_proof.equivocation.id.clone()); + + let offender = T::KeyOwnerProofSystem::check_proof(key, key_owner_proof.clone()) + .ok_or(InvalidTransaction::BadProof)?; + + // check if the offence has already been reported, + // and if so then we can discard the report. + let time_slot = >::Offence::new_time_slot( + equivocation_proof.set_id, + equivocation_proof.equivocation.round_number, + ); + + let is_known_offence = T::HandleEquivocation::is_known_offence(&[offender], &time_slot); + + if is_known_offence { + Err(InvalidTransaction::Stale.into()) + } else { + Ok(()) + } +} + +/// A BEEFY equivocation offence report. +pub struct BeefyEquivocationOffence +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + /// Time slot at which this incident happened. + pub time_slot: BeefyTimeSlot, + /// The session index in which the incident happened. + pub session_index: SessionIndex, + /// The size of the validator set at the time of the offence. + pub validator_set_count: u32, + /// The authority which produced this equivocation. + pub offender: FullIdentification, +} + +/// An interface for types that will be used as BEEFY offences and must also +/// implement the `Offence` trait. This trait provides a constructor that is +/// provided all available data during processing of BEEFY equivocations. +pub trait BeefyOffence: Offence +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + /// Create a new BEEFY offence using the given equivocation details. + fn new( + session_index: SessionIndex, + validator_set_count: u32, + offender: FullIdentification, + set_id: ValidatorSetId, + round: N, + ) -> Self; + + /// Create a new BEEFY offence time slot. + fn new_time_slot(set_id: ValidatorSetId, round: N) -> Self::TimeSlot; +} + +impl BeefyOffence + for BeefyEquivocationOffence +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + fn new( + session_index: SessionIndex, + validator_set_count: u32, + offender: FullIdentification, + set_id: ValidatorSetId, + round: N, + ) -> Self { + BeefyEquivocationOffence { + session_index, + validator_set_count, + offender, + time_slot: BeefyTimeSlot { set_id, round }, + } + } + + fn new_time_slot(set_id: ValidatorSetId, round: N) -> Self::TimeSlot { + BeefyTimeSlot { set_id, round } + } +} + +impl Offence + for BeefyEquivocationOffence +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + const ID: Kind = *b"beefy:equivocati"; + type TimeSlot = BeefyTimeSlot; + + fn offenders(&self) -> Vec { + vec![self.offender.clone()] + } + + fn session_index(&self) -> SessionIndex { + self.session_index + } + + fn validator_set_count(&self) -> u32 { + self.validator_set_count + } + + fn time_slot(&self) -> Self::TimeSlot { + self.time_slot + } + + fn slash_fraction(&self, offenders_count: u32) -> Perbill { + // the formula is min((3k / n)^2, 1) + let x = Perbill::from_rational(3 * offenders_count, self.validator_set_count); + // _ ^ 2 + x.square() + } +} diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index 529378f8b4547..b2c5ad7bbc82a 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -34,7 +34,6 @@ use sp_runtime::{ KeyTypeId, RuntimeAppPublic, }; use sp_session::{GetSessionNumber, GetValidatorCount}; -use sp_staking::offence::ReportOffence; use sp_std::prelude::*; use beefy_primitives::{ @@ -42,12 +41,14 @@ use beefy_primitives::{ OpaqueKeyOwnershipProof, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; +mod equivocation; #[cfg(test)] mod mock; - #[cfg(test)] mod tests; +use crate::equivocation::{BeefyOffence, HandleEquivocation}; + pub use pallet::*; const LOG_TARGET: &str = "runtime::beefy"; @@ -84,6 +85,14 @@ pub mod pallet { IdentificationTuple = Self::KeyOwnerIdentification, >; + /// The equivocation handling subsystem, defines methods to report an + /// offence (after the equivocation has been validated) and for submitting a + /// transaction to report an equivocation (from an offchain context). + /// NOTE: when enabling equivocation handling (i.e. this type isn't set to + /// `()`) you must use this pallet's `ValidateUnsigned` in the runtime + /// definition. + type HandleEquivocation: HandleEquivocation; + /// The maximum number of authorities that can be added. type MaxAuthorities: Get; @@ -321,8 +330,20 @@ impl Pallet { return Err(Error::::InvalidEquivocationProof.into()) } - // todo: ReportOffence - todo!(); + // todo: cross-check session index with equivocation report + + // report to the offences module rewarding the sender. + T::HandleEquivocation::report_offence( + reporter.into_iter().collect(), + >::Offence::new( + session_index, + validator_count, + offender, + set_id, + round, + ), + ) + .map_err(|_| Error::::DuplicateOffenceReport)?; // waive the fee since the report is valid and beneficial Ok(Pays::No.into()) From db11585ffb008d4e85e7c49eace7d2f58153f136 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 18:58:29 +0200 Subject: [PATCH 17/41] frame/beefy: impl ValidateUnsigned for equivocations reporting --- frame/beefy/src/equivocation.rs | 1 - frame/beefy/src/lib.rs | 56 +++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/frame/beefy/src/equivocation.rs b/frame/beefy/src/equivocation.rs index 42c9b38b4b8cb..3598640046e11 100644 --- a/frame/beefy/src/equivocation.rs +++ b/frame/beefy/src/equivocation.rs @@ -224,7 +224,6 @@ impl Pallet { target: LOG_TARGET, "rejecting unsigned report equivocation transaction because it is not local/in-block." ); - return InvalidTransaction::Call.into() }, } diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index b2c5ad7bbc82a..962dc5d6ad8d1 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -38,7 +38,7 @@ use sp_std::prelude::*; use beefy_primitives::{ AuthorityIndex, BeefyAuthorityId, ConsensusLog, EquivocationProof, OnNewValidatorSet, - OpaqueKeyOwnershipProof, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, + ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; mod equivocation; @@ -57,14 +57,14 @@ const LOG_TARGET: &str = "runtime::beefy"; pub mod pallet { use super::*; use frame_support::{pallet_prelude::*, traits::KeyOwnerProofSystem}; - use frame_system::{ensure_signed, pallet_prelude::OriginFor}; + use frame_system::{ensure_none, ensure_signed, pallet_prelude::OriginFor}; #[pallet::config] pub trait Config: frame_system::Config { /// Authority identifier type type BeefyId: Member + Parameter - // todo: use custom hashing type + // todo: use custom signature hashing type + BeefyAuthorityId + MaybeSerializeDeserialize + MaxEncodedLen; @@ -184,7 +184,15 @@ pub mod pallet { Self::do_report_equivocation(Some(reporter), *equivocation_proof, key_owner_proof) } - /// TODO + /// Report voter equivocation/misbehavior. This method will verify the + /// equivocation proof and validate the given key ownership proof + /// against the extracted offender. If both are valid, the offence + /// will be reported. + /// + /// This extrinsic must be called unsigned and it is expected that only + /// block authors will call it (validated in `ValidateUnsigned`), as such + /// if the block author is defined it will be defined as the equivocation + /// reporter. #[pallet::call_index(1)] #[pallet::weight(T::WeightInfo::report_equivocation(key_owner_proof.validator_count()))] pub fn report_equivocation_unsigned( @@ -198,7 +206,24 @@ pub mod pallet { >, key_owner_proof: T::KeyOwnerProof, ) -> DispatchResultWithPostInfo { - todo!() + ensure_none(origin)?; + + Self::do_report_equivocation( + T::HandleEquivocation::block_author(), + *equivocation_proof, + key_owner_proof, + ) + } + } + + #[pallet::validate_unsigned] + impl ValidateUnsigned for Pallet { + type Call = Call; + fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { + Self::pre_dispatch(call) + } + fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity { + Self::validate_unsigned(source, call) } } } @@ -220,22 +245,13 @@ impl Pallet { T::BeefyId, ::Signature, >, - key_owner_proof: OpaqueKeyOwnershipProof, + key_owner_proof: T::KeyOwnerProof, ) -> Option<()> { - // TODO: - // use frame_system::offchain::SubmitTransaction; - // - // let call = Call::report_equivocation_unsigned { - // equivocation_proof: Box::new(equivocation_proof), - // key_owner_proof, - // }; - // - // match SubmitTransaction::>::submit_unsigned_transaction(call.into()) { - // Ok(()) => log::info!(target: LOG_TARGET, "Submitted BEEFY equivocation report."), - // Err(e) => log::error!(target: LOG_TARGET, "Error submitting equivocation: {:?}", e), - // } - - Some(()) + T::HandleEquivocation::submit_unsigned_equivocation_report( + equivocation_proof, + key_owner_proof, + ) + .ok() } fn change_authorities( From 57d6897c9a1facd031e8b9ca2dfbb61983595c42 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 19:11:59 +0200 Subject: [PATCH 18/41] client/beefy: submit report equivocation unsigned extrinsic --- client/beefy/src/lib.rs | 3 +- client/beefy/src/worker.rs | 65 ++++++++++++++++++++------------------ 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 5b6531822a0a1..0039c6203a1ff 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -284,6 +284,7 @@ where let worker_params = worker::WorkerParams { backend, payload_provider, + runtime, network, key_store: key_store.into(), gossip_engine, @@ -294,7 +295,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), diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 6b0f4d8cd6a03..abe0bfac9cfe1 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -31,7 +31,7 @@ use crate::{ }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, - Commitment, ConsensusLog, EquivocationProof, PayloadProvider, ValidatorSet, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, PayloadProvider, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, }; use codec::{Codec, Decode, Encode}; @@ -41,7 +41,7 @@ use sc_client_api::{Backend, FinalityNotification, FinalityNotifications, Header use sc_network_common::service::{NetworkEventStream, NetworkRequest}; use sc_network_gossip::GossipEngine; use sc_utils::notification::NotificationReceiver; -use sp_api::BlockId; +use sp_api::{BlockId, ProvideRuntimeApi}; use sp_arithmetic::traits::{AtLeast32Bit, Saturating}; use sp_consensus::SyncOracle; use sp_runtime::{ @@ -243,9 +243,10 @@ impl VoterOracle { } } -pub(crate) struct WorkerParams { +pub(crate) struct WorkerParams { pub backend: Arc, pub payload_provider: P, + pub runtime: Arc, pub network: N, pub key_store: BeefyKeystore, pub gossip_engine: GossipEngine, @@ -294,10 +295,11 @@ impl PersistedState { } /// A BEEFY worker plays the BEEFY protocol -pub(crate) struct BeefyWorker { +pub(crate) struct BeefyWorker { // utilities backend: Arc, payload_provider: P, + runtime: Arc, network: N, key_store: BeefyKeystore, @@ -327,11 +329,13 @@ pub(crate) struct BeefyWorker { persisted_state: PersistedState, } -impl BeefyWorker +impl BeefyWorker where B: Block + Codec, BE: Backend, P: PayloadProvider, + R: ProvideRuntimeApi, + R::Api: BeefyApi, N: NetworkEventStream + NetworkRequest + SyncOracle + Send + Sync + Clone + 'static, { /// Return a new BEEFY worker instance. @@ -340,10 +344,11 @@ where /// BEEFY pallet has been deployed on-chain. /// /// The BEEFY pallet is needed in order to keep track of the BEEFY authority set. - pub(crate) fn new(worker_params: WorkerParams) -> Self { + pub(crate) fn new(worker_params: WorkerParams) -> Self { let WorkerParams { backend, payload_provider, + runtime, key_store, network, gossip_engine, @@ -357,6 +362,7 @@ where BeefyWorker { backend, payload_provider, + runtime, network, key_store, gossip_engine, @@ -932,34 +938,33 @@ where error!(target: "beefy", "🥩 {}", err_msg); Error::Backend(err_msg) })?; + + let runtime_api = self.runtime.runtime_api(); // generate key ownership proof at that block - // let key_owner_proof = match self - // .runtime - // .runtime_api() - // .generate_key_ownership_proof( - // &BlockId::Hash(hash), - // validator_set_id, - // proof.equivocation.id.clone(), - // ) - // .map_err(Error::RuntimeApi)? - // { - // Some(proof) => proof, - // None => { - // debug!(target: "beefy", "🥩 Equivocation offender not part of the authority set."); - // return Ok(()) - // }, - // }; + let key_owner_proof = match runtime_api + .generate_key_ownership_proof( + &BlockId::Hash(hash), + validator_set_id, + proof.equivocation.id.clone(), + ) + .map_err(Error::RuntimeApi)? + { + Some(proof) => proof, + None => { + debug!(target: "beefy", "🥩 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; - // self.client - // .runtime_api() - // .submit_report_equivocation_unsigned_extrinsic( - // &BlockId::Hash(best_block_hash), - // proof, - // key_owner_proof, - // ) - // .map_err(Error::RuntimeApi)?; + runtime_api + .submit_report_equivocation_unsigned_extrinsic( + &BlockId::Hash(best_block_hash), + proof, + key_owner_proof, + ) + .map_err(Error::RuntimeApi)?; Ok(()) } From 8a344ccf61b666d9688919efff550fdd7ecc6498 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 19:19:28 +0200 Subject: [PATCH 19/41] primitives/beefy: fix tests --- primitives/beefy/src/lib.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index c83a4733a46f7..79811039b83cb 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -358,23 +358,31 @@ mod tests { pair.as_inner_ref().sign_prehashed(&blake2_256(msg)).into(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyVerify::::verify(&keccak_256_signature, msg, &pair.public())); - assert!(BeefyVerify::::verify(&blake2_256_signature, msg, &pair.public())); + assert!(BeefyAuthorityId::::verify(&pair.public(), &keccak_256_signature, msg)); + assert!(BeefyAuthorityId::::verify( + &pair.public(), + &blake2_256_signature, + msg + )); // Verification fails if distinct hashing functions are used when signing and verifying. - assert!(!BeefyVerify::::verify(&blake2_256_signature, msg, &pair.public())); - assert!(!BeefyVerify::::verify(&keccak_256_signature, msg, &pair.public())); + assert!(!BeefyAuthorityId::::verify(&pair.public(), &blake2_256_signature, msg)); + assert!(!BeefyAuthorityId::::verify( + &pair.public(), + &keccak_256_signature, + msg + )); // Other public key doesn't work let (other_pair, _) = crypto::Pair::generate(); - assert!(!BeefyVerify::::verify( + assert!(!BeefyAuthorityId::::verify( + &other_pair.public(), &keccak_256_signature, msg, - &other_pair.public() )); - assert!(!BeefyVerify::::verify( + assert!(!BeefyAuthorityId::::verify( + &other_pair.public(), &blake2_256_signature, msg, - &other_pair.public() )); } } From f6aa2e036b82533172fcda1f7b52eef00b99141e Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 19:54:36 +0200 Subject: [PATCH 20/41] frame/beefy: add default weights --- frame/beefy/src/default_weights.rs | 54 ++++++++++++++++++++++++++++++ frame/beefy/src/equivocation.rs | 2 +- frame/beefy/src/lib.rs | 6 ++-- 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 frame/beefy/src/default_weights.rs diff --git a/frame/beefy/src/default_weights.rs b/frame/beefy/src/default_weights.rs new file mode 100644 index 0000000000000..0b642cb45e9ce --- /dev/null +++ b/frame/beefy/src/default_weights.rs @@ -0,0 +1,54 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2023 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Default weights for the BEEFY Pallet +//! This file was not auto-generated. + +use frame_support::weights::{ + constants::{RocksDbWeight as DbWeight, WEIGHT_REF_TIME_PER_MICROS, WEIGHT_REF_TIME_PER_NANOS}, + Weight, +}; + +impl crate::WeightInfo for () { + fn report_equivocation(validator_count: u32) -> Weight { + // we take the validator set count from the membership proof to + // calculate the weight but we set a floor of 100 validators. + let validator_count = validator_count.max(100) as u64; + + // worst case we are considering is that the given offender is backed by 200 nominators + const MAX_NOMINATORS: u64 = 200; + + // checking membership proof + Weight::from_ref_time(35u64 * WEIGHT_REF_TIME_PER_MICROS) + .saturating_add( + Weight::from_ref_time(175u64 * WEIGHT_REF_TIME_PER_NANOS) + .saturating_mul(validator_count), + ) + .saturating_add(DbWeight::get().reads(5)) + // check equivocation proof + .saturating_add(Weight::from_ref_time(95u64 * WEIGHT_REF_TIME_PER_MICROS)) + // report offence + .saturating_add(Weight::from_ref_time(110u64 * WEIGHT_REF_TIME_PER_MICROS)) + .saturating_add(Weight::from_ref_time( + 25u64 * WEIGHT_REF_TIME_PER_MICROS * MAX_NOMINATORS, + )) + .saturating_add(DbWeight::get().reads(14 + 3 * MAX_NOMINATORS)) + .saturating_add(DbWeight::get().writes(10 + 3 * MAX_NOMINATORS)) + // fetching set id -> session index mappings + .saturating_add(DbWeight::get().reads(2)) + } +} diff --git a/frame/beefy/src/equivocation.rs b/frame/beefy/src/equivocation.rs index 3598640046e11..be939f4e5c735 100644 --- a/frame/beefy/src/equivocation.rs +++ b/frame/beefy/src/equivocation.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2017-2023 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index 962dc5d6ad8d1..0986f82401327 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -41,14 +41,16 @@ use beefy_primitives::{ ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; +mod default_weights; mod equivocation; #[cfg(test)] mod mock; #[cfg(test)] mod tests; -use crate::equivocation::{BeefyOffence, HandleEquivocation}; - +pub use crate::equivocation::{ + BeefyEquivocationOffence, BeefyOffence, BeefyTimeSlot, EquivocationHandler, HandleEquivocation, +}; pub use pallet::*; const LOG_TARGET: &str = "runtime::beefy"; From 5cf25601afcef037179a8414b06121486df9725d Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 20:01:21 +0200 Subject: [PATCH 21/41] frame/beefy: fix tests --- Cargo.lock | 6 ++ frame/beefy/Cargo.toml | 6 ++ frame/beefy/src/mock.rs | 141 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 148 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc96b7748dc9b..be9fb7bdd4522 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4926,10 +4926,16 @@ dependencies = [ name = "pallet-beefy" version = "4.0.0-dev" dependencies = [ + "frame-election-provider-support", "frame-support", "frame-system", "pallet-authorship", + "pallet-balances", + "pallet-offences", "pallet-session", + "pallet-staking", + "pallet-staking-reward-curve", + "pallet-timestamp", "parity-scale-codec", "scale-info", "serde", diff --git a/frame/beefy/Cargo.toml b/frame/beefy/Cargo.toml index 4b3c4b4884530..2b3e5c85640d4 100644 --- a/frame/beefy/Cargo.toml +++ b/frame/beefy/Cargo.toml @@ -23,6 +23,12 @@ sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../pr sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" } [dev-dependencies] +frame-election-provider-support = { version = "4.0.0-dev", path = "../election-provider-support" } +pallet-balances = { version = "4.0.0-dev", path = "../balances" } +pallet-offences = { version = "4.0.0-dev", path = "../offences" } +pallet-staking = { version = "4.0.0-dev", path = "../staking" } +pallet-staking-reward-curve = { version = "4.0.0-dev", path = "../staking/reward-curve" } +pallet-timestamp = { version = "4.0.0-dev", path = "../timestamp" } sp-core = { version = "7.0.0", path = "../../primitives/core" } sp-io = { version = "7.0.0", path = "../../primitives/io" } sp-staking = { version = "4.0.0-dev", path = "../../primitives/staking" } diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index ad3a672333dd5..c63b4505b73d5 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -17,20 +17,25 @@ use std::vec; +use beefy_primitives::crypto::AuthorityId; +use frame_election_provider_support::{onchain, SequentialPhragmen}; use frame_support::{ construct_runtime, parameter_types, sp_io::TestExternalities, - traits::{ConstU16, ConstU32, ConstU64, GenesisBuild}, + traits::{ConstU16, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem}, BasicExternalities, }; -use sp_core::H256; +use pallet_session::historical as pallet_session_historical; +use sp_core::{crypto::KeyTypeId, ConstU128, H256}; use sp_runtime::{ app_crypto::ecdsa::Public, + curve::PiecewiseLinear, impl_opaque_keys, - testing::Header, + testing::{Header, TestXt}, traits::{BlakeTwo256, ConvertInto, IdentityLookup, OpaqueKeys}, Perbill, }; +use sp_staking::{EraIndex, SessionIndex}; use crate as pallet_beefy; @@ -52,8 +57,14 @@ construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, - Beefy: pallet_beefy::{Pallet, Config, Storage}, + Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, + Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, + Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, + Beefy: pallet_beefy::{Pallet, Call, Config, Storage}, + Staking: pallet_staking::{Pallet, Call, Config, Storage, Event}, Session: pallet_session::{Pallet, Call, Storage, Event, Config}, + Offences: pallet_offences::{Pallet, Storage, Event}, + Historical: pallet_session_historical::{Pallet}, } ); @@ -75,7 +86,7 @@ impl frame_system::Config for Test { type BlockHashCount = ConstU64<250>; type Version = (); type PalletInfo = PalletInfo; - type AccountData = (); + type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (); type SystemWeightInfo = (); @@ -84,10 +95,34 @@ impl frame_system::Config for Test { type MaxConsumers = ConstU32<16>; } +impl frame_system::offchain::SendTransactionTypes for Test +where + RuntimeCall: From, +{ + type OverarchingCall = RuntimeCall; + type Extrinsic = TestXt; +} + +parameter_types! { + pub const Period: u64 = 1; + pub const ReportLongevity: u64 = + BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * Period::get(); +} + impl pallet_beefy::Config for Test { type BeefyId = BeefyId; + type KeyOwnerProofSystem = Historical; + type KeyOwnerProof = + >::Proof; + type KeyOwnerIdentification = >::IdentificationTuple; + type HandleEquivocation = + super::EquivocationHandler; type MaxAuthorities = ConstU32<100>; type OnNewValidatorSet = (); + type WeightInfo = (); } parameter_types! { @@ -106,6 +141,102 @@ impl pallet_session::Config for Test { type WeightInfo = (); } +impl pallet_session::historical::Config for Test { + type FullIdentification = pallet_staking::Exposure; + type FullIdentificationOf = pallet_staking::ExposureOf; +} + +impl pallet_authorship::Config for Test { + type FindAuthor = (); + type UncleGenerations = ConstU64<0>; + type FilterUncle = (); + type EventHandler = (); +} + +impl pallet_balances::Config for Test { + type MaxLocks = (); + type MaxReserves = (); + type ReserveIdentifier = [u8; 8]; + type Balance = u128; + type DustRemoval = (); + type RuntimeEvent = RuntimeEvent; + type ExistentialDeposit = ConstU128<1>; + type AccountStore = System; + type WeightInfo = (); +} + +impl pallet_timestamp::Config for Test { + type Moment = u64; + type OnTimestampSet = (); + type MinimumPeriod = ConstU64<3>; + type WeightInfo = (); +} + +pallet_staking_reward_curve::build! { + const REWARD_CURVE: PiecewiseLinear<'static> = curve!( + min_inflation: 0_025_000u64, + max_inflation: 0_100_000, + ideal_stake: 0_500_000, + falloff: 0_050_000, + max_piece_count: 40, + test_precision: 0_005_000, + ); +} + +parameter_types! { + pub const SessionsPerEra: SessionIndex = 3; + pub const BondingDuration: EraIndex = 3; + pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; + pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(17); +} + +pub struct OnChainSeqPhragmen; +impl onchain::Config for OnChainSeqPhragmen { + type System = Test; + type Solver = SequentialPhragmen; + type DataProvider = Staking; + type WeightInfo = (); + type MaxWinners = ConstU32<100>; + type VotersBound = ConstU32<{ u32::MAX }>; + type TargetsBound = ConstU32<{ u32::MAX }>; +} + +impl pallet_staking::Config for Test { + type MaxNominations = ConstU32<16>; + type RewardRemainder = (); + type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote; + type RuntimeEvent = RuntimeEvent; + type Currency = Balances; + type CurrencyBalance = ::Balance; + type Slash = (); + type Reward = (); + type SessionsPerEra = SessionsPerEra; + type BondingDuration = BondingDuration; + type SlashDeferDuration = (); + type AdminOrigin = frame_system::EnsureRoot; + type SessionInterface = Self; + type UnixTime = pallet_timestamp::Pallet; + type EraPayout = pallet_staking::ConvertCurve; + type MaxNominatorRewardedPerValidator = ConstU32<64>; + type OffendingValidatorsThreshold = OffendingValidatorsThreshold; + type NextNewSession = Session; + type ElectionProvider = onchain::OnChainExecution; + type GenesisElectionProvider = Self::ElectionProvider; + type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; + type TargetList = pallet_staking::UseValidatorsMap; + type MaxUnlockingChunks = ConstU32<32>; + type HistoryDepth = ConstU32<84>; + type OnStakerSlash = (); + type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; + type WeightInfo = (); +} + +impl pallet_offences::Config for Test { + type RuntimeEvent = RuntimeEvent; + type IdentificationTuple = pallet_session::historical::IdentificationTuple; + type OnOffenceHandler = Staking; +} + pub struct MockSessionManager; impl pallet_session::SessionManager for MockSessionManager { From 2c2c5d6a3ed83b7a930b0ee8c24eb5555794da69 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Tue, 10 Jan 2023 20:17:31 +0200 Subject: [PATCH 22/41] client/beefy: fix tests --- client/beefy/src/error.rs | 14 ++++++++++++++ client/beefy/src/worker.rs | 6 ++++-- test-utils/runtime/src/lib.rs | 13 +++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/client/beefy/src/error.rs b/client/beefy/src/error.rs index bdb04c434785e..1b44e3801e303 100644 --- a/client/beefy/src/error.rs +++ b/client/beefy/src/error.rs @@ -35,3 +35,17 @@ pub enum Error { #[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, + } + } +} diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index abe0bfac9cfe1..73dcb2931006d 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1084,6 +1084,7 @@ pub(crate) mod tests { Block, Backend, MmrRootProvider, + TestApi, Arc>, > { let keystore = create_beefy_keystore(*key); @@ -1128,10 +1129,11 @@ pub(crate) mod tests { min_block_delta, ) .unwrap(); - let payload_provider = MmrRootProvider::new(api); + let payload_provider = MmrRootProvider::new(api.clone()); let worker_params = crate::worker::WorkerParams { backend, payload_provider, + runtime: api, key_store: Some(keystore).into(), links, gossip_engine, @@ -1141,7 +1143,7 @@ pub(crate) mod tests { on_demand_justifications, persisted_state, }; - BeefyWorker::<_, _, _, _>::new(worker_params) + BeefyWorker::<_, _, _, _, _>::new(worker_params) } #[test] diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 8b64528e243bd..98c3d41fbb67f 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -976,6 +976,19 @@ cfg_if! { fn validator_set() -> Option> { None } + fn submit_report_equivocation_unsigned_extrinsic( + _equivocation_proof: beefy_primitives::EquivocationProof< + NumberFor, + beefy_primitives::crypto::AuthorityId, + beefy_primitives::crypto::Signature + >, + _key_owner_proof: beefy_primitives::OpaqueKeyOwnershipProof, + ) -> Option<()> { None } + + fn generate_key_ownership_proof( + _set_id: beefy_primitives::ValidatorSetId, + _authority_id: beefy_primitives::crypto::AuthorityId, + ) -> Option { None } } impl beefy_merkle_tree::BeefyMmrApi for Runtime { From 93493502179bee6d9807a6d1cc75bb720ac0a428 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 16 Jan 2023 11:14:55 +0200 Subject: [PATCH 23/41] frame/beefy-mmr: fix tests --- frame/beefy-mmr/src/mock.rs | 13 +++++++++++-- frame/beefy/src/lib.rs | 16 ++++++++-------- frame/beefy/src/mock.rs | 5 ++--- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/frame/beefy-mmr/src/mock.rs b/frame/beefy-mmr/src/mock.rs index 0a64ad3fc9976..bb75bb401f263 100644 --- a/frame/beefy-mmr/src/mock.rs +++ b/frame/beefy-mmr/src/mock.rs @@ -22,10 +22,10 @@ use codec::Encode; use frame_support::{ construct_runtime, parameter_types, sp_io::TestExternalities, - traits::{ConstU16, ConstU32, ConstU64, GenesisBuild}, + traits::{ConstU16, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem}, BasicExternalities, }; -use sp_core::{Hasher, H256}; +use sp_core::{crypto::KeyTypeId, Hasher, H256}; use sp_runtime::{ app_crypto::ecdsa::Public, impl_opaque_keys, @@ -124,8 +124,17 @@ impl pallet_mmr::Config for Test { impl pallet_beefy::Config for Test { type BeefyId = BeefyId; + type KeyOwnerProofSystem = (); + type KeyOwnerProof = + >::Proof; + type KeyOwnerIdentification = >::IdentificationTuple; + type HandleEquivocation = (); type MaxAuthorities = ConstU32<100>; type OnNewValidatorSet = BeefyMmr; + type WeightInfo = (); } parameter_types! { diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index 0986f82401327..a8c9432b988e5 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -71,14 +71,6 @@ pub mod pallet { + MaybeSerializeDeserialize + MaxEncodedLen; - /// The proof of key ownership, used for validating equivocation reports - /// The proof must include the session index and validator count of the - /// session at which the equivocation occurred. - type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount; - - /// The identification of a key owner, used when reporting equivocations. - type KeyOwnerIdentification: Parameter; - /// A system for proving ownership of keys, i.e. that a given key was part /// of a validator set, needed for validating equivocation reports. type KeyOwnerProofSystem: KeyOwnerProofSystem< @@ -87,6 +79,14 @@ pub mod pallet { IdentificationTuple = Self::KeyOwnerIdentification, >; + /// The proof of key ownership, used for validating equivocation reports + /// The proof must include the session index and validator count of the + /// session at which the equivocation occurred. + type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount; + + /// The identification of a key owner, used when reporting equivocations. + type KeyOwnerIdentification: Parameter; + /// The equivocation handling subsystem, defines methods to report an /// offence (after the equivocation has been validated) and for submitting a /// transaction to report an equivocation (from an offchain context). diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index c63b4505b73d5..191219514c6bb 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -17,7 +17,6 @@ use std::vec; -use beefy_primitives::crypto::AuthorityId; use frame_election_provider_support::{onchain, SequentialPhragmen}; use frame_support::{ construct_runtime, parameter_types, @@ -113,10 +112,10 @@ impl pallet_beefy::Config for Test { type BeefyId = BeefyId; type KeyOwnerProofSystem = Historical; type KeyOwnerProof = - >::Proof; + >::Proof; type KeyOwnerIdentification = >::IdentificationTuple; type HandleEquivocation = super::EquivocationHandler; From b5e2c219235eea0ae4a59d09eab462515c2681c4 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 16 Jan 2023 11:41:52 +0200 Subject: [PATCH 24/41] frame/beefy: cross-check session index with equivocation report --- frame/beefy/src/lib.rs | 48 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index a8c9432b988e5..a49591bd4109f 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -22,18 +22,22 @@ use codec::{Encode, MaxEncodedLen}; use frame_support::{ dispatch::{DispatchResultWithPostInfo, Pays}, log, + pallet_prelude::*, traits::{Get, KeyOwnerProofSystem, OneSessionHandler}, weights::Weight, BoundedSlice, BoundedVec, Parameter, }; -use frame_system::pallet_prelude::BlockNumberFor; - +use frame_system::{ + ensure_none, ensure_signed, + pallet_prelude::{BlockNumberFor, OriginFor}, +}; use sp_runtime::{ generic::DigestItem, traits::{IsMember, Member}, KeyTypeId, RuntimeAppPublic, }; use sp_session::{GetSessionNumber, GetValidatorCount}; +use sp_staking::SessionIndex; use sp_std::prelude::*; use beefy_primitives::{ @@ -58,8 +62,6 @@ const LOG_TARGET: &str = "runtime::beefy"; #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{pallet_prelude::*, traits::KeyOwnerProofSystem}; - use frame_system::{ensure_none, ensure_signed, pallet_prelude::OriginFor}; #[pallet::config] pub trait Config: frame_system::Config { @@ -130,6 +132,15 @@ pub mod pallet { pub(super) type NextAuthorities = StorageValue<_, BoundedVec, ValueQuery>; + /// A mapping from BEEFY set ID to the index of the *most recent* session for which its + /// members were responsible. + /// + /// TWOX-NOTE: `ValidatorSetId` is not under user control. + #[pallet::storage] + #[pallet::getter(fn session_for_set)] + pub(super) type SetIdSession = + StorageMap<_, Twox64Concat, beefy_primitives::ValidatorSetId, SessionIndex>; + #[pallet::genesis_config] pub struct GenesisConfig { pub authorities: Vec, @@ -145,7 +156,7 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - Pallet::::initialize_authorities(&self.authorities) + Pallet::::initialize(&self.authorities) // we panic here as runtime maintainers can simply reconfigure genesis and restart // the chain easily .expect("Authorities vec too big"); @@ -284,7 +295,7 @@ impl Pallet { } } - fn initialize_authorities(authorities: &Vec) -> Result<(), ()> { + fn initialize(authorities: &Vec) -> Result<(), ()> { if authorities.is_empty() { return Ok(()) } @@ -314,6 +325,12 @@ impl Pallet { ); } } + + // NOTE: initialize first session of first set. this is necessary for + // the genesis set and session since we only update the set -> session + // mapping whenever a new session starts, i.e. through `on_new_session`. + SetIdSession::::insert(0, 0); + Ok(()) } @@ -348,7 +365,13 @@ impl Pallet { return Err(Error::::InvalidEquivocationProof.into()) } - // todo: cross-check session index with equivocation report + // check that the session id for the membership proof is within the + // bounds of the set id reported in the equivocation. + let set_id_session_index = + Self::session_for_set(set_id).ok_or(Error::::InvalidEquivocationProof)?; + if session_index != set_id_session_index { + return Err(Error::::InvalidEquivocationProof.into()) + } // report to the offences module rewarding the sender. T::HandleEquivocation::report_offence( @@ -372,7 +395,10 @@ impl sp_runtime::BoundToRuntimeAppPublic for Pallet { type Public = T::BeefyId; } -impl OneSessionHandler for Pallet { +impl OneSessionHandler for Pallet +where + T: pallet_session::Config, +{ type Key = T::BeefyId; fn on_genesis_session<'a, I: 'a>(validators: I) @@ -382,7 +408,7 @@ impl OneSessionHandler for Pallet { let authorities = validators.map(|(_, k)| k).collect::>(); // we panic here as runtime maintainers can simply reconfigure genesis and restart the // chain easily - Self::initialize_authorities(&authorities).expect("Authorities vec too big"); + Self::initialize(&authorities).expect("Authorities vec too big"); } fn on_new_session<'a, I: 'a>(_changed: bool, validators: I, queued_validators: I) @@ -416,6 +442,10 @@ impl OneSessionHandler for Pallet { // Always issue a change on each `session`, even if validator set hasn't changed. // We want to have at least one BEEFY mandatory block per session. Self::change_authorities(bounded_next_authorities, bounded_next_queued_authorities); + + // Update the mapping for the new set id that corresponds to the latest session (i.e. now). + let session_index = >::current_index(); + SetIdSession::::insert(Self::validator_set_id(), &session_index); } fn on_disabled(i: u32) { From 1eca5b202bd73aa980edd1672f21cd7ad3496784 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 17 Jan 2023 13:03:58 +0200 Subject: [PATCH 25/41] sp-beefy: make test Keyring useable in pallet --- Cargo.lock | 2 +- client/beefy/Cargo.toml | 1 - client/beefy/src/communication/gossip.rs | 5 ++- client/beefy/src/justification.rs | 5 ++- client/beefy/src/keystore.rs | 56 ++---------------------- client/beefy/src/round.rs | 6 +-- client/beefy/src/tests.rs | 2 +- client/beefy/src/worker.rs | 5 ++- primitives/beefy/Cargo.toml | 1 + primitives/beefy/src/lib.rs | 56 ++++++++++++++++++++++++ 10 files changed, 74 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c3e6fdda2205..a339d20efe60e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -494,7 +494,6 @@ dependencies = [ "sp-mmr-primitives", "sp-runtime", "sp-tracing", - "strum", "substrate-prometheus-endpoint", "substrate-test-runtime-client", "tempfile", @@ -9665,6 +9664,7 @@ dependencies = [ "sp-mmr-primitives", "sp-runtime", "sp-std", + "strum", ] [[package]] diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index b039faa2d095d..40f981ece3023 100644 --- a/client/beefy/Cargo.toml +++ b/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" } diff --git a/client/beefy/src/communication/gossip.rs b/client/beefy/src/communication/gossip.rs index 2123e205aa919..e359d496cba73 100644 --- a/client/beefy/src/communication/gossip.rs +++ b/client/beefy/src/communication/gossip.rs @@ -239,9 +239,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::*; diff --git a/client/beefy/src/justification.rs b/client/beefy/src/justification.rs index 7243c692727f0..6f869015ba763 100644 --- a/client/beefy/src/justification.rs +++ b/client/beefy/src/justification.rs @@ -81,12 +81,13 @@ fn verify_with_validator_set( #[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, diff --git a/client/beefy/src/keystore.rs b/client/beefy/src/keystore.rs index 8d77410746e52..1d0f80a11e404 100644 --- a/client/beefy/src/keystore.rs +++ b/client/beefy/src/keystore.rs @@ -114,63 +114,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 for crypto::Pair { - fn from(k: Keyring) -> Self { - k.pair() - } - } - - impl From for ecdsa::Pair { - fn from(k: Keyring) -> Self { - k.pair().into() - } - } - fn keystore() -> SyncCryptoStorePtr { Arc::new(LocalKeystore::in_memory()) } diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 4ba769b3db335..edd027fc8819c 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -218,12 +218,12 @@ mod tests { use sc_network_test::Block; use beefy_primitives::{ - crypto::Public, known_payloads::MMR_ROOT_ID, Commitment, Equivocation, EquivocationProof, - Payload, SignedCommitment, ValidatorSet, VoteMessage, + crypto::Public, keyring::Keyring, known_payloads::MMR_ROOT_ID, Commitment, Equivocation, + EquivocationProof, Payload, SignedCommitment, ValidatorSet, VoteMessage, }; use super::{threshold, Block as BlockT, RoundTracker, Rounds}; - use crate::{keystore::tests::Keyring, round::VoteImportResult}; + use crate::round::VoteImportResult; impl Rounds where diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 66d3a210d7eba..a88fcc8eaed00 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -26,12 +26,12 @@ use crate::{ }, gossip_protocol_name, justification::*, - keystore::tests::Keyring as BeefyKeyring, load_or_init_voter_state, wait_for_runtime_pallet, BeefyRPCLinks, BeefyVoterLinks, KnownPeers, PersistedState, }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, + keyring::Keyring as BeefyKeyring, known_payloads, mmr::MmrRootProvider, BeefyApi, Commitment, ConsensusLog, MmrRootHash, Payload, SignedCommitment, ValidatorSet, diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 2842d8faf1d6e..5c756f0d25155 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1030,14 +1030,15 @@ pub(crate) mod tests { use super::*; use crate::{ communication::notification::{BeefyBestBlockStream, BeefyVersionedFinalityProofStream}, - keystore::tests::Keyring, tests::{ create_beefy_keystore, get_beefy_streams, make_beefy_ids, two_validators::TestApi, BeefyPeer, BeefyTestNet, }, BeefyRPCLinks, KnownPeers, }; - use beefy_primitives::{known_payloads, mmr::MmrRootProvider, Payload, SignedCommitment}; + use beefy_primitives::{ + keyring::Keyring, known_payloads, mmr::MmrRootProvider, Payload, SignedCommitment, + }; use futures::{future::poll_fn, task::Poll}; use parking_lot::Mutex; use sc_client_api::{Backend as BackendT, HeaderBackend}; diff --git a/primitives/beefy/Cargo.toml b/primitives/beefy/Cargo.toml index b286d9878b44e..0d1853ed8fe17 100644 --- a/primitives/beefy/Cargo.toml +++ b/primitives/beefy/Cargo.toml @@ -22,6 +22,7 @@ sp-io = { version = "7.0.0", default-features = false, path = "../io" } sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../merkle-mountain-range" } sp-runtime = { version = "7.0.0", default-features = false, path = "../runtime" } sp-std = { version = "5.0.0", default-features = false, path = "../std" } +strum = { version = "0.24.1", features = ["derive"] } [dev-dependencies] array-bytes = "4.1" diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 79811039b83cb..1c6dc89be9bcd 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -326,6 +326,62 @@ sp_api::decl_runtime_apis! { } } +#[cfg(feature = "std")] +/// Test accounts using [`beefy_primitives::crypto`] types. +pub mod keyring { + use super::*; + use sp_core::{ecdsa, keccak_256, Pair}; + + /// Set of test accounts using [`beefy_primitives::crypto`] types. + #[allow(missing_docs)] + #[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display, strum::EnumIter)] + pub 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 for crypto::Pair { + fn from(k: Keyring) -> Self { + k.pair() + } + } + + impl From for ecdsa::Pair { + fn from(k: Keyring) -> Self { + k.pair().into() + } + } +} + #[cfg(test)] mod tests { use super::*; From fccf5c788100c2cc130c91950cbb518d2710bd13 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 17 Jan 2023 16:25:03 +0200 Subject: [PATCH 26/41] frame/beefy: add basic equivocation test --- Cargo.lock | 1 + frame/beefy/src/mock.rs | 129 +++++++++++++++++++++++++------- frame/beefy/src/tests.rs | 144 +++++++++++++++++++++++++++++++++--- primitives/beefy/Cargo.toml | 1 + primitives/beefy/src/lib.rs | 38 ++++++++-- 5 files changed, 271 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a339d20efe60e..e6648f6cec895 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9653,6 +9653,7 @@ name = "sp-beefy" version = "4.0.0-dev" dependencies = [ "array-bytes", + "lazy_static", "parity-scale-codec", "scale-info", "serde", diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 191219514c6bb..2551569a49dd6 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -17,11 +17,18 @@ use std::vec; +use beefy_primitives::{ + keyring::Keyring as BeefyKeyring, Commitment, Equivocation, Payload, ValidatorSetId, + VoteMessage, +}; +use codec::Encode; use frame_election_provider_support::{onchain, SequentialPhragmen}; use frame_support::{ construct_runtime, parameter_types, sp_io::TestExternalities, - traits::{ConstU16, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem}, + traits::{ + ConstU16, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem, OnFinalize, OnInitialize, + }, BasicExternalities, }; use pallet_session::historical as pallet_session_historical; @@ -31,14 +38,17 @@ use sp_runtime::{ curve::PiecewiseLinear, impl_opaque_keys, testing::{Header, TestXt}, - traits::{BlakeTwo256, ConvertInto, IdentityLookup, OpaqueKeys}, + traits::{BlakeTwo256, IdentityLookup, OpaqueKeys}, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; use crate as pallet_beefy; -pub use beefy_primitives::{crypto::AuthorityId as BeefyId, ConsensusLog, BEEFY_ENGINE_ID}; +pub use beefy_primitives::{ + crypto::{AuthorityId as BeefyId, AuthoritySignature as BeefySignature}, + ConsensusLog, EquivocationProof, BEEFY_ENGINE_ID, +}; impl_opaque_keys! { pub struct MockSessionKeys { @@ -131,10 +141,10 @@ parameter_types! { impl pallet_session::Config for Test { type RuntimeEvent = RuntimeEvent; type ValidatorId = u64; - type ValidatorIdOf = ConvertInto; + type ValidatorIdOf = pallet_staking::StashOf; type ShouldEndSession = pallet_session::PeriodicSessions, ConstU64<0>>; type NextSessionRotation = pallet_session::PeriodicSessions, ConstU64<0>>; - type SessionManager = MockSessionManager; + type SessionManager = pallet_session::historical::NoteHistoricalRoot; type SessionHandler = ::KeyTypeIdProviders; type Keys = MockSessionKeys; type WeightInfo = (); @@ -236,22 +246,6 @@ impl pallet_offences::Config for Test { type OnOffenceHandler = Staking; } -pub struct MockSessionManager; - -impl pallet_session::SessionManager for MockSessionManager { - fn end_session(_: sp_staking::SessionIndex) {} - fn start_session(_: sp_staking::SessionIndex) {} - fn new_session(idx: sp_staking::SessionIndex) -> Option> { - if idx == 0 || idx == 1 { - Some(vec![1, 2]) - } else if idx == 2 { - Some(vec![3, 4]) - } else { - None - } - } -} - // Note, that we can't use `UintAuthorityId` here. Reason is that the implementation // of `to_public_key()` assumes, that a public key is 32 bytes long. This is true for // ed25519 and sr25519 but *not* for ecdsa. A compressed ecdsa public key is 33 bytes, @@ -264,20 +258,27 @@ pub fn mock_beefy_id(id: u8) -> BeefyId { BeefyId::from(pk) } -pub fn mock_authorities(vec: Vec) -> Vec<(u64, BeefyId)> { - vec.into_iter().map(|id| ((id as u64), mock_beefy_id(id))).collect() +pub fn mock_authorities(vec: Vec) -> Vec { + vec.into_iter().map(|id| mock_beefy_id(id)).collect() } pub fn new_test_ext(ids: Vec) -> TestExternalities { new_test_ext_raw_authorities(mock_authorities(ids)) } -pub fn new_test_ext_raw_authorities(authorities: Vec<(u64, BeefyId)>) -> TestExternalities { +pub fn new_test_ext_raw_authorities(authorities: Vec) -> TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + let balances: Vec<_> = (0..authorities.len()).map(|i| (i as u64, 10_000_000)).collect(); + + pallet_balances::GenesisConfig:: { balances } + .assimilate_storage(&mut t) + .unwrap(); + let session_keys: Vec<_> = authorities .iter() - .map(|id| (id.0 as u64, id.0 as u64, MockSessionKeys { dummy: id.1.clone() })) + .enumerate() + .map(|(i, k)| (i as u64, i as u64, MockSessionKeys { dummy: k.clone() })) .collect(); BasicExternalities::execute_with_storage(&mut t, || { @@ -290,5 +291,83 @@ pub fn new_test_ext_raw_authorities(authorities: Vec<(u64, BeefyId)>) -> TestExt .assimilate_storage(&mut t) .unwrap(); + // controllers are the index + 1000 + let stakers: Vec<_> = (0..authorities.len()) + .map(|i| { + (i as u64, i as u64 + 1000, 10_000, pallet_staking::StakerStatus::::Validator) + }) + .collect(); + + let staking_config = pallet_staking::GenesisConfig:: { + stakers, + validator_count: 2, + force_era: pallet_staking::Forcing::ForceNew, + minimum_validator_count: 0, + invulnerables: vec![], + ..Default::default() + }; + + staking_config.assimilate_storage(&mut t).unwrap(); + t.into() } + +pub fn start_session(session_index: SessionIndex) { + for i in Session::current_index()..session_index { + System::on_finalize(System::block_number()); + Session::on_finalize(System::block_number()); + Staking::on_finalize(System::block_number()); + Beefy::on_finalize(System::block_number()); + + let parent_hash = if System::block_number() > 1 { + let hdr = System::finalize(); + hdr.hash() + } else { + System::parent_hash() + }; + + System::reset_events(); + System::initialize(&(i as u64 + 1), &parent_hash, &Default::default()); + System::set_block_number((i + 1).into()); + Timestamp::set_timestamp(System::block_number() * 6000); + + System::on_initialize(System::block_number()); + Session::on_initialize(System::block_number()); + Staking::on_initialize(System::block_number()); + Beefy::on_initialize(System::block_number()); + } + + assert_eq!(Session::current_index(), session_index); +} + +pub fn start_era(era_index: EraIndex) { + start_session((era_index * 3).into()); + assert_eq!(Staking::current_era(), Some(era_index)); +} + +pub fn generate_equivocation_proof( + validator_set_id: ValidatorSetId, + reporter: &BeefyKeyring, + vote1: (u64, Payload, &BeefyKeyring), + vote2: (u64, Payload, &BeefyKeyring), +) -> EquivocationProof { + let block_number = vote1.0; + + let signed_vote = |block_number: u64, payload: Payload, keyring: &BeefyKeyring| { + let commitment = Commitment { validator_set_id, block_number, payload }; + let signature = keyring.sign(&commitment.encode()); + VoteMessage { commitment, id: keyring.public(), signature } + }; + + let first = signed_vote(vote1.0, vote1.1, vote1.2); + let second = signed_vote(vote2.0, vote2.1, vote2.2); + EquivocationProof { + set_id: validator_set_id, + equivocation: Equivocation { + round_number: block_number, + id: reporter.public(), + first, + second, + }, + } +} diff --git a/frame/beefy/src/tests.rs b/frame/beefy/src/tests.rs index 4136b0c1f1ecf..58bd5dc8dd66b 100644 --- a/frame/beefy/src/tests.rs +++ b/frame/beefy/src/tests.rs @@ -17,12 +17,18 @@ use std::vec; -use beefy_primitives::ValidatorSet; +use beefy_primitives::{ + check_equivocation_proof, keyring::Keyring as BeefyKeyring, known_payloads::MMR_ROOT_ID, + Payload, ValidatorSet, +}; use codec::Encode; use sp_runtime::DigestItem; -use frame_support::traits::OnInitialize; +use frame_support::{ + assert_ok, + traits::{Currency, KeyOwnerProofSystem, OnInitialize}, +}; use crate::mock::*; @@ -37,12 +43,13 @@ pub fn beefy_log(log: ConsensusLog) -> DigestItem { #[test] fn genesis_session_initializes_authorities() { - let want = vec![mock_beefy_id(1), mock_beefy_id(2), mock_beefy_id(3), mock_beefy_id(4)]; + let authorities = mock_authorities(vec![1, 2, 3, 4]); + let want = authorities.clone(); - new_test_ext(vec![1, 2, 3, 4]).execute_with(|| { + new_test_ext_raw_authorities(authorities).execute_with(|| { let authorities = Beefy::authorities(); - assert!(authorities.len() == 2); + assert_eq!(authorities.len(), 4); assert_eq!(want[0], authorities[0]); assert_eq!(want[1], authorities[1]); @@ -50,7 +57,7 @@ fn genesis_session_initializes_authorities() { let next_authorities = Beefy::next_authorities(); - assert!(next_authorities.len() == 2); + assert_eq!(next_authorities.len(), 4); assert_eq!(want[0], next_authorities[0]); assert_eq!(want[1], next_authorities[1]); }); @@ -58,6 +65,9 @@ fn genesis_session_initializes_authorities() { #[test] fn session_change_updates_authorities() { + let authorities = mock_authorities(vec![1, 2, 3, 4]); + let want_validators = authorities.clone(); + new_test_ext(vec![1, 2, 3, 4]).execute_with(|| { assert!(0 == Beefy::validator_set_id()); @@ -66,7 +76,7 @@ fn session_change_updates_authorities() { assert!(1 == Beefy::validator_set_id()); let want = beefy_log(ConsensusLog::AuthoritiesChange( - ValidatorSet::new(vec![mock_beefy_id(1), mock_beefy_id(2)], 1).unwrap(), + ValidatorSet::new(want_validators, 1).unwrap(), )); let log = System::digest().logs[0].clone(); @@ -77,7 +87,7 @@ fn session_change_updates_authorities() { assert!(2 == Beefy::validator_set_id()); let want = beefy_log(ConsensusLog::AuthoritiesChange( - ValidatorSet::new(vec![mock_beefy_id(3), mock_beefy_id(4)], 2).unwrap(), + ValidatorSet::new(vec![mock_beefy_id(2), mock_beefy_id(4)], 2).unwrap(), )); let log = System::digest().logs[1].clone(); @@ -92,16 +102,18 @@ fn session_change_updates_next_authorities() { new_test_ext(vec![1, 2, 3, 4]).execute_with(|| { let next_authorities = Beefy::next_authorities(); - assert!(next_authorities.len() == 2); + assert_eq!(next_authorities.len(), 4); assert_eq!(want[0], next_authorities[0]); assert_eq!(want[1], next_authorities[1]); + assert_eq!(want[2], next_authorities[2]); + assert_eq!(want[3], next_authorities[3]); init_block(1); let next_authorities = Beefy::next_authorities(); - assert!(next_authorities.len() == 2); - assert_eq!(want[2], next_authorities[0]); + assert_eq!(next_authorities.len(), 2); + assert_eq!(want[1], next_authorities[0]); assert_eq!(want[3], next_authorities[1]); }); } @@ -126,6 +138,10 @@ fn validator_set_updates_work() { new_test_ext(vec![1, 2, 3, 4]).execute_with(|| { let vs = Beefy::validator_set().unwrap(); assert_eq!(vs.id(), 0u64); + assert_eq!(want[0], vs.validators()[0]); + assert_eq!(want[1], vs.validators()[1]); + assert_eq!(want[2], vs.validators()[2]); + assert_eq!(want[3], vs.validators()[3]); init_block(1); @@ -140,7 +156,111 @@ fn validator_set_updates_work() { let vs = Beefy::validator_set().unwrap(); assert_eq!(vs.id(), 2u64); - assert_eq!(want[2], vs.validators()[0]); + assert_eq!(want[1], vs.validators()[0]); assert_eq!(want[3], vs.validators()[1]); }); } + +/// Returns a list with 3 authorities with known keys: +/// Alice, Bob and Charlie. +pub fn test_authorities() -> Vec { + let authorities = vec![BeefyKeyring::Alice, BeefyKeyring::Bob, BeefyKeyring::Charlie]; + authorities.into_iter().map(|id| id.public()).collect() +} + +#[test] +fn should_sign_and_verify() { + use sp_runtime::traits::Keccak256; + + let set_id = 3; + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + + // generate an equivocation proof, with two votes in the same round for + // same payload signed by the same key + let equivocation_proof = generate_equivocation_proof( + set_id, + &BeefyKeyring::Alice, + (1, payload1.clone(), &BeefyKeyring::Bob), + (1, payload1.clone(), &BeefyKeyring::Bob), + ); + // expect invalid equivocation proof + assert!(!check_equivocation_proof::<_, _, Keccak256>(equivocation_proof)); + + // generate an equivocation proof, with two votes in different rounds for + // different payloads signed by the same key + let equivocation_proof = generate_equivocation_proof( + set_id, + &BeefyKeyring::Alice, + (1, payload1.clone(), &BeefyKeyring::Bob), + (2, payload2.clone(), &BeefyKeyring::Bob), + ); + // expect invalid equivocation proof + assert!(!check_equivocation_proof::<_, _, Keccak256>(equivocation_proof)); + + // generate an equivocation proof, with two votes in the same round for + // different payloads signed by the same key + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + let equivocation_proof = generate_equivocation_proof( + set_id, + &BeefyKeyring::Alice, + (1, payload1, &BeefyKeyring::Bob), + (1, payload2, &BeefyKeyring::Bob), + ); + // expect valid equivocation proof + assert!(check_equivocation_proof::<_, _, Keccak256>(equivocation_proof)); +} + +#[test] +fn report_equivocation_current_set_works() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + assert_eq!(Staking::current_era(), Some(0)); + assert_eq!(Session::current_index(), 0); + + start_era(1); + + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + let validators = Session::validators(); + + // make sure that all validators have the same balance + for validator in &validators { + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(1, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + // generate an equivocation proof, with two votes in the same round for + // different payloads signed by the same key + let equivocation_proof = generate_equivocation_proof( + set_id, + &BeefyKeyring::Alice, + (1, payload1, &equivocation_keyring), + (1, payload2, &equivocation_keyring), + ); + + // create the key ownership proof + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + // report the equivocation and the tx should be dispatched successfully + assert_ok!(Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ),); + }); +} diff --git a/primitives/beefy/Cargo.toml b/primitives/beefy/Cargo.toml index 0d1853ed8fe17..db9364efc521e 100644 --- a/primitives/beefy/Cargo.toml +++ b/primitives/beefy/Cargo.toml @@ -23,6 +23,7 @@ sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = ". sp-runtime = { version = "7.0.0", default-features = false, path = "../runtime" } sp-std = { version = "5.0.0", default-features = false, path = "../std" } strum = { version = "0.24.1", features = ["derive"] } +lazy_static = "1.4.0" [dev-dependencies] array-bytes = "4.1" diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 1c6dc89be9bcd..ce43f4345bab9 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -236,10 +236,17 @@ where let first = &report.equivocation.first; let second = &report.equivocation.second; - // if votes come from different authorities, - // or both votes have the same commitment, - // --> the equivocation is invalid. - if first.id != second.id || first.commitment == second.commitment { + // if votes + // come from different authorities, + // are for different rounds, + // have different validator set ids, + // or both votes have the same commitment, + // --> the equivocation is invalid. + if first.id != second.id || + first.commitment.block_number != second.commitment.block_number || + first.commitment.validator_set_id != second.commitment.validator_set_id || + first.commitment.payload == second.commitment.payload + { return false } @@ -331,10 +338,12 @@ sp_api::decl_runtime_apis! { pub mod keyring { use super::*; use sp_core::{ecdsa, keccak_256, Pair}; + use std::collections::HashMap; + use strum::IntoEnumIterator; /// Set of test accounts using [`beefy_primitives::crypto`] types. #[allow(missing_docs)] - #[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display, strum::EnumIter)] + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumIter)] pub enum Keyring { Alice, Bob, @@ -349,6 +358,7 @@ pub mod keyring { impl Keyring { /// Sign `msg`. pub fn sign(self, msg: &[u8]) -> crypto::Signature { + // todo: use custom signature hashing type let msg = keccak_256(msg); ecdsa::Pair::from(self).sign_prehashed(&msg).into() } @@ -367,6 +377,18 @@ pub mod keyring { pub fn to_seed(self) -> String { format!("//{}", self) } + + /// Get Keyring from public key. + pub fn from_public(who: &crypto::Public) -> Option { + Self::iter().find(|&k| &crypto::Public::from(k) == who) + } + } + + lazy_static::lazy_static! { + static ref PRIVATE_KEYS: HashMap = + Keyring::iter().map(|i| (i, i.pair())).collect(); + static ref PUBLIC_KEYS: HashMap = + PRIVATE_KEYS.iter().map(|(&name, pair)| (name, pair.public())).collect(); } impl From for crypto::Pair { @@ -380,6 +402,12 @@ pub mod keyring { k.pair().into() } } + + impl From for crypto::Public { + fn from(k: Keyring) -> Self { + (*PUBLIC_KEYS).get(&k).cloned().unwrap() + } + } } #[cfg(test)] From 9d68c1d2d2c0e8ba22be2067b1cfc4855eeb6fc3 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 17 Jan 2023 17:37:07 +0200 Subject: [PATCH 27/41] frame/beefy: test verify equivocation results in slashing --- frame/beefy/src/mock.rs | 4 ++-- frame/beefy/src/tests.rs | 47 ++++++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 2551569a49dd6..387eaf8957546 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -347,7 +347,7 @@ pub fn start_era(era_index: EraIndex) { pub fn generate_equivocation_proof( validator_set_id: ValidatorSetId, - reporter: &BeefyKeyring, + reporter_public: BeefyId, vote1: (u64, Payload, &BeefyKeyring), vote2: (u64, Payload, &BeefyKeyring), ) -> EquivocationProof { @@ -365,7 +365,7 @@ pub fn generate_equivocation_proof( set_id: validator_set_id, equivocation: Equivocation { round_number: block_number, - id: reporter.public(), + id: reporter_public, first, second, }, diff --git a/frame/beefy/src/tests.rs b/frame/beefy/src/tests.rs index 58bd5dc8dd66b..f57e329aadf69 100644 --- a/frame/beefy/src/tests.rs +++ b/frame/beefy/src/tests.rs @@ -180,7 +180,7 @@ fn should_sign_and_verify() { // same payload signed by the same key let equivocation_proof = generate_equivocation_proof( set_id, - &BeefyKeyring::Alice, + BeefyKeyring::Alice.public(), (1, payload1.clone(), &BeefyKeyring::Bob), (1, payload1.clone(), &BeefyKeyring::Bob), ); @@ -191,7 +191,7 @@ fn should_sign_and_verify() { // different payloads signed by the same key let equivocation_proof = generate_equivocation_proof( set_id, - &BeefyKeyring::Alice, + BeefyKeyring::Alice.public(), (1, payload1.clone(), &BeefyKeyring::Bob), (2, payload2.clone(), &BeefyKeyring::Bob), ); @@ -203,7 +203,7 @@ fn should_sign_and_verify() { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); let equivocation_proof = generate_equivocation_proof( set_id, - &BeefyKeyring::Alice, + BeefyKeyring::Alice.public(), (1, payload1, &BeefyKeyring::Bob), (1, payload2, &BeefyKeyring::Bob), ); @@ -221,6 +221,7 @@ fn report_equivocation_current_set_works() { start_era(1); + let block_num = System::block_number(); let validator_set = Beefy::validator_set().unwrap(); let authorities = validator_set.validators(); let set_id = validator_set.id(); @@ -237,7 +238,10 @@ fn report_equivocation_current_set_works() { ); } - let equivocation_authority_index = 0; + assert_eq!(authorities.len(), 2); + let reporter_index = 0; + let _reporter_key = authorities[reporter_index].clone(); + let equivocation_authority_index = 1; let equivocation_key = &authorities[equivocation_authority_index]; let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); @@ -247,9 +251,11 @@ fn report_equivocation_current_set_works() { // different payloads signed by the same key let equivocation_proof = generate_equivocation_proof( set_id, - &BeefyKeyring::Alice, - (1, payload1, &equivocation_keyring), - (1, payload2, &equivocation_keyring), + // FIXME: test only passes for equivocation auto-report, why? + // _reporter_key.clone(), + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), ); // create the key ownership proof @@ -262,5 +268,32 @@ fn report_equivocation_current_set_works() { Box::new(equivocation_proof), key_owner_proof, ),); + + start_era(2); + + // check that the balance of 0-th validator is slashed 100%. + let equivocation_validator_id = validators[equivocation_authority_index]; + + assert_eq!(Balances::total_balance(&equivocation_validator_id), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&equivocation_validator_id), 0); + assert_eq!( + Staking::eras_stakers(2, equivocation_validator_id), + pallet_staking::Exposure { total: 0, own: 0, others: vec![] }, + ); + + // check that the balances of all other validators are left intact. + for validator in &validators { + if *validator == equivocation_validator_id { + continue + } + + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(2, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } }); } From a08b3193285d8b5461f93eca7fa93b3c1e65d0f6 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 17 Jan 2023 18:38:20 +0200 Subject: [PATCH 28/41] frame/beefy: test report_equivocation_old_set --- frame/beefy/src/tests.rs | 90 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/frame/beefy/src/tests.rs b/frame/beefy/src/tests.rs index f57e329aadf69..1f56123869144 100644 --- a/frame/beefy/src/tests.rs +++ b/frame/beefy/src/tests.rs @@ -297,3 +297,93 @@ fn report_equivocation_current_set_works() { } }); } + +#[test] +fn report_equivocation_old_set_works() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let validators = Session::validators(); + let old_set_id = validator_set.id(); + + assert_eq!(authorities.len(), 2); + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let reporter_index = 1; + let _reporter_key = authorities[reporter_index].clone(); + + // create the key ownership proof in the "old" set + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + start_era(2); + + // make sure that all authorities have the same balance + for validator in &validators { + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(2, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + + let validator_set = Beefy::validator_set().unwrap(); + let new_set_id = validator_set.id(); + assert_eq!(old_set_id + 3, new_set_id); + + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + // generate an equivocation proof for the old set, + let equivocation_proof = generate_equivocation_proof( + old_set_id, + // FIXME: test only passes for equivocation auto-report, why? + // _reporter_key.clone(), + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), + ); + + // report the equivocation and the tx should be dispatched successfully + assert_ok!(Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ),); + + start_era(3); + + // check that the balance of 0-th validator is slashed 100%. + let equivocation_validator_id = validators[equivocation_authority_index]; + + assert_eq!(Balances::total_balance(&equivocation_validator_id), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&equivocation_validator_id), 0); + assert_eq!( + Staking::eras_stakers(3, equivocation_validator_id), + pallet_staking::Exposure { total: 0, own: 0, others: vec![] }, + ); + + // check that the balances of all other validators are left intact. + for validator in &validators { + if *validator == equivocation_validator_id { + continue + } + + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(3, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + }); +} From 64114b45d1e75b374d811ec2026ef73928decb68 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 18 Jan 2023 12:08:09 +0200 Subject: [PATCH 29/41] frame/beefy: add more equivocation tests --- frame/beefy/src/tests.rs | 396 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 394 insertions(+), 2 deletions(-) diff --git a/frame/beefy/src/tests.rs b/frame/beefy/src/tests.rs index 1f56123869144..272e1f8897ba4 100644 --- a/frame/beefy/src/tests.rs +++ b/frame/beefy/src/tests.rs @@ -26,11 +26,12 @@ use codec::Encode; use sp_runtime::DigestItem; use frame_support::{ - assert_ok, + assert_err, assert_ok, + dispatch::{GetDispatchInfo, Pays}, traits::{Currency, KeyOwnerProofSystem, OnInitialize}, }; -use crate::mock::*; +use crate::{mock::*, Call, Config, Error, Weight, WeightInfo}; fn init_block(block: u64) { System::set_block_number(block); @@ -387,3 +388,394 @@ fn report_equivocation_old_set_works() { } }); } + +#[test] +fn report_equivocation_invalid_set_id() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + // generate an equivocation for a future set + let equivocation_proof = generate_equivocation_proof( + set_id + 1, + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), + ); + + // the call for reporting the equivocation should error + assert_err!( + Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ), + Error::::InvalidEquivocationProof, + ); + }); +} + +#[test] +fn report_equivocation_invalid_session() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // generate a key ownership proof at current era set id + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + start_era(2); + + let set_id = Beefy::validator_set().unwrap().id(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + // generate an equivocation proof at following era set id = 2 + let equivocation_proof = generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), + ); + + // report an equivocation for the current set using an key ownership + // proof from the previous set, the session should be invalid. + assert_err!( + Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ), + Error::::InvalidEquivocationProof, + ); + }); +} + +#[test] +fn report_equivocation_invalid_key_owner_proof() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let invalid_owner_authority_index = 1; + let invalid_owner_key = &authorities[invalid_owner_authority_index]; + + // generate a key ownership proof for the authority at index 1 + let invalid_key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &invalid_owner_key)).unwrap(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + // generate an equivocation proof for the authority at index 0 + let equivocation_proof = generate_equivocation_proof( + set_id + 1, + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), + ); + + // we need to start a new era otherwise the key ownership proof won't be + // checked since the authorities are part of the current session + start_era(2); + + // report an equivocation for the current set using a key ownership + // proof for a different key than the one in the equivocation proof. + assert_err!( + Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + invalid_key_owner_proof, + ), + Error::::InvalidKeyOwnershipProof, + ); + }); +} + +#[test] +fn report_equivocation_invalid_equivocation_proof() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // generate a key ownership proof at set id in era 1 + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + let assert_invalid_equivocation_proof = |equivocation_proof| { + assert_err!( + Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof.clone(), + ), + Error::::InvalidEquivocationProof, + ); + }; + + start_era(2); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + + // both votes target the same block number and payload, + // there is no equivocation. + assert_invalid_equivocation_proof(generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1.clone(), &equivocation_keyring), + (block_num, payload1.clone(), &equivocation_keyring), + )); + + // votes targetting different rounds, there is no equivocation. + assert_invalid_equivocation_proof(generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1.clone(), &equivocation_keyring), + (block_num + 1, payload2.clone(), &equivocation_keyring), + )); + + // votes signed with different authority keys + assert_invalid_equivocation_proof(generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1.clone(), &equivocation_keyring), + (block_num, payload1.clone(), &BeefyKeyring::Charlie), + )); + + // votes signed with a key that isn't part of the authority set + assert_invalid_equivocation_proof(generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1.clone(), &equivocation_keyring), + (block_num, payload1.clone(), &BeefyKeyring::Dave), + )); + }); +} + +#[test] +fn report_equivocation_validate_unsigned_prevents_duplicates() { + use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, + ValidTransaction, + }; + + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + // generate and report an equivocation for the validator at index 0 + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + let equivocation_proof = generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), + ); + + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + let call = Call::report_equivocation_unsigned { + equivocation_proof: Box::new(equivocation_proof.clone()), + key_owner_proof: key_owner_proof.clone(), + }; + + // only local/inblock reports are allowed + assert_eq!( + ::validate_unsigned( + TransactionSource::External, + &call, + ), + InvalidTransaction::Call.into(), + ); + + // the transaction is valid when passed as local + let tx_tag = (equivocation_key, set_id, 3u64); + + assert_eq!( + ::validate_unsigned( + TransactionSource::Local, + &call, + ), + TransactionValidity::Ok(ValidTransaction { + priority: TransactionPriority::max_value(), + requires: vec![], + provides: vec![("BeefyEquivocation", tx_tag).encode()], + longevity: ReportLongevity::get(), + propagate: false, + }) + ); + + // the pre dispatch checks should also pass + assert_ok!(::pre_dispatch(&call)); + + // we submit the report + Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ) + .unwrap(); + + // the report should now be considered stale and the transaction is invalid + // the check for staleness should be done on both `validate_unsigned` and on `pre_dispatch` + assert_err!( + ::validate_unsigned( + TransactionSource::Local, + &call, + ), + InvalidTransaction::Stale, + ); + + assert_err!( + ::pre_dispatch(&call), + InvalidTransaction::Stale, + ); + }); +} + +#[test] +fn report_equivocation_has_valid_weight() { + // the weight depends on the size of the validator set, + // but there's a lower bound of 100 validators. + assert!((1..=100) + .map(::WeightInfo::report_equivocation) + .collect::>() + .windows(2) + .all(|w| w[0] == w[1])); + + // after 100 validators the weight should keep increasing + // with every extra validator. + assert!((100..=1000) + .map(::WeightInfo::report_equivocation) + .collect::>() + .windows(2) + .all(|w| w[0].ref_time() < w[1].ref_time())); +} + +#[test] +fn valid_equivocation_reports_dont_pay_fees() { + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // generate equivocation proof + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + let equivocation_proof = generate_equivocation_proof( + set_id, + equivocation_key.clone(), + (block_num, payload1, &equivocation_keyring), + (block_num, payload2, &equivocation_keyring), + ); + + // create the key ownership proof. + let key_owner_proof = + Historical::prove((beefy_primitives::KEY_TYPE, &equivocation_key)).unwrap(); + + // check the dispatch info for the call. + let info = Call::::report_equivocation_unsigned { + equivocation_proof: Box::new(equivocation_proof.clone()), + key_owner_proof: key_owner_proof.clone(), + } + .get_dispatch_info(); + + // it should have non-zero weight and the fee has to be paid. + assert!(info.weight.any_gt(Weight::zero())); + assert_eq!(info.pays_fee, Pays::Yes); + + // report the equivocation. + let post_info = Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof.clone()), + key_owner_proof.clone(), + ) + .unwrap(); + + // the original weight should be kept, but given that the report + // is valid the fee is waived. + assert!(post_info.actual_weight.is_none()); + assert_eq!(post_info.pays_fee, Pays::No); + + // report the equivocation again which is invalid now since it is + // duplicate. + let post_info = Beefy::report_equivocation_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ) + .err() + .unwrap() + .post_info; + + // the fee is not waived and the original weight is kept. + assert!(post_info.actual_weight.is_none()); + assert_eq!(post_info.pays_fee, Pays::Yes); + }) +} From 56348055e14fa3e86ddb3332ccaee69a6c1f6468 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 18 Jan 2023 19:41:21 +0200 Subject: [PATCH 30/41] sp-beefy: fix docs --- primitives/beefy/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index ce43f4345bab9..61e65b50c1b46 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -334,14 +334,14 @@ sp_api::decl_runtime_apis! { } #[cfg(feature = "std")] -/// Test accounts using [`beefy_primitives::crypto`] types. +/// Test accounts using [`crate::crypto`] types. pub mod keyring { use super::*; use sp_core::{ecdsa, keccak_256, Pair}; use std::collections::HashMap; use strum::IntoEnumIterator; - /// Set of test accounts using [`beefy_primitives::crypto`] types. + /// Set of test accounts using [`crate::crypto`] types. #[allow(missing_docs)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumIter)] pub enum Keyring { From d8898a58430b3acbcda4d1dec5ebde77f3c1c5e7 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 19 Jan 2023 10:37:35 +0200 Subject: [PATCH 31/41] beefy: simplify equivocations and fix tests --- client/beefy/src/keystore.rs | 6 +- client/beefy/src/round.rs | 22 ++---- client/beefy/src/worker.rs | 21 +++--- frame/beefy/src/equivocation.rs | 15 ++-- frame/beefy/src/lib.rs | 8 +-- frame/beefy/src/mock.rs | 31 +++----- frame/beefy/src/tests.rs | 122 ++++++++++++++------------------ primitives/beefy/src/lib.rs | 42 +++++------ 8 files changed, 119 insertions(+), 148 deletions(-) diff --git a/client/beefy/src/keystore.rs b/client/beefy/src/keystore.rs index 1d0f80a11e404..c52cfc51f23be 100644 --- a/client/beefy/src/keystore.rs +++ b/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; +/// 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. @@ -99,7 +101,7 @@ impl BeefyKeystore { /// /// Return `true` if the signature is authentic, `false` otherwise. pub fn verify(public: &Public, sig: &Signature, message: &[u8]) -> bool { - BeefyAuthorityId::::verify(public, sig, message) + BeefyAuthorityId::::verify(public, sig, message) } } diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index edd027fc8819c..3516dc4d8946b 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -18,8 +18,7 @@ use beefy_primitives::{ crypto::{AuthorityId, Public, Signature}, - Commitment, Equivocation, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, - VoteMessage, + Commitment, EquivocationProof, SignedCommitment, ValidatorSet, ValidatorSetId, VoteMessage, }; use codec::{Decode, Encode}; use log::debug; @@ -164,14 +163,10 @@ where target: "beefy", "🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", existing_vote, vote ); - let equivocation = Equivocation { - round_number: equivocation_key.1, - id: equivocation_key.0, + return VoteImportResult::Equivocation(EquivocationProof { first: existing_vote.clone(), second: vote, - }; - let set_id = self.validator_set_id(); - return VoteImportResult::Equivocation(EquivocationProof { set_id, equivocation }) + }) } } else { // this is the first vote sent by `id` for `num`, all good @@ -218,7 +213,7 @@ mod tests { use sc_network_test::Block; use beefy_primitives::{ - crypto::Public, keyring::Keyring, known_payloads::MMR_ROOT_ID, Commitment, Equivocation, + crypto::Public, keyring::Keyring, known_payloads::MMR_ROOT_ID, Commitment, EquivocationProof, Payload, SignedCommitment, ValidatorSet, VoteMessage, }; @@ -501,13 +496,8 @@ mod tests { alice_vote2.commitment = commitment2; let expected_result = VoteImportResult::Equivocation(EquivocationProof { - set_id: validator_set_id, - equivocation: Equivocation { - id: Keyring::Alice.public(), - round_number: 1, - first: alice_vote1.clone(), - second: alice_vote2.clone(), - }, + first: alice_vote1.clone(), + second: alice_vote2.clone(), }); // vote on one payload - ok diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 5c756f0d25155..c67fd44dcc339 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -23,13 +23,14 @@ use crate::{ }, error::Error, justification::BeefyVersionedFinalityProof, - keystore::BeefyKeystore, + keystore::{BeefyKeystore, BeefySignatureHasher}, metric_inc, metric_set, metrics::Metrics, round::{Rounds, VoteImportResult}, BeefyVoterLinks, }; use beefy_primitives::{ + check_equivocation_proof, crypto::{AuthorityId, Signature}, BeefyApi, Commitment, ConsensusLog, EquivocationProof, PayloadProvider, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, @@ -914,18 +915,22 @@ where let rounds = self.persisted_state.voting_oracle.active_rounds().ok_or(Error::UninitSession)?; let (validators, validator_set_id) = (rounds.validators(), rounds.validator_set_id()); + let offender_id = proof.offender_id().clone(); - if proof.set_id != validator_set_id { - debug!(target: "beefy", "🥩 Skip equivocation report for old set id: {:?}", proof.set_id); + if proof.set_id() != validator_set_id { + debug!(target: "beefy", "🥩 Skip equivocation report for old set id: {:?}", proof.set_id()); + return Ok(()) + } else if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) { + debug!(target: "beefy", "🥩 Skip report for bad equivocation {:?}", proof); return Ok(()) } else if let Some(local_id) = self.key_store.authority_id(validators) { - if proof.equivocation.id == local_id { + if offender_id == local_id { debug!(target: "beefy", "🥩 Skip equivocation report for own equivocation"); return Ok(()) } } - let number = proof.equivocation.round_number; + let number = *proof.round_number(); let hash = self .backend .blockchain() @@ -942,11 +947,7 @@ where 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( - &BlockId::Hash(hash), - validator_set_id, - proof.equivocation.id.clone(), - ) + .generate_key_ownership_proof(&BlockId::Hash(hash), validator_set_id, offender_id) .map_err(Error::RuntimeApi)? { Some(proof) => proof, diff --git a/frame/beefy/src/equivocation.rs b/frame/beefy/src/equivocation.rs index be939f4e5c735..ea6a9f9122351 100644 --- a/frame/beefy/src/equivocation.rs +++ b/frame/beefy/src/equivocation.rs @@ -239,9 +239,9 @@ impl Pallet { .priority(TransactionPriority::MAX) // Only one equivocation report for the same offender at the same slot. .and_provides(( - equivocation_proof.equivocation.id.clone(), - equivocation_proof.set_id, - equivocation_proof.equivocation.round_number, + equivocation_proof.offender_id().clone(), + equivocation_proof.set_id(), + *equivocation_proof.round_number(), )) .longevity(longevity) // We don't propagate this. This can never be included on a remote node. @@ -269,8 +269,9 @@ fn is_known_offence( >, key_owner_proof: &T::KeyOwnerProof, ) -> Result<(), TransactionValidityError> { - // check the membership proof to extract the offender's id - let key = (beefy_primitives::KEY_TYPE, equivocation_proof.equivocation.id.clone()); + // check the membership proof to extract the offender's id, + // equivocation validity will be fully checked during the call. + let key = (beefy_primitives::KEY_TYPE, equivocation_proof.offender_id().clone()); let offender = T::KeyOwnerProofSystem::check_proof(key, key_owner_proof.clone()) .ok_or(InvalidTransaction::BadProof)?; @@ -278,8 +279,8 @@ fn is_known_offence( // check if the offence has already been reported, // and if so then we can discard the report. let time_slot = >::Offence::new_time_slot( - equivocation_proof.set_id, - equivocation_proof.equivocation.round_number, + equivocation_proof.set_id(), + *equivocation_proof.round_number(), ); let is_known_offence = T::HandleEquivocation::is_known_offence(&[offender], &time_slot); diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index a49591bd4109f..f53f86e1a78fb 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -347,9 +347,9 @@ impl Pallet { // associated session) and round. We also need to know the validator // set count at the time of the offence since it is required to calculate // the slash amount. - let set_id = equivocation_proof.set_id; - let round = equivocation_proof.equivocation.round_number; - let offender_id = equivocation_proof.equivocation.id.clone(); + let set_id = equivocation_proof.set_id(); + let round = *equivocation_proof.round_number(); + let offender_id = equivocation_proof.offender_id().clone(); let session_index = key_owner_proof.session(); let validator_count = key_owner_proof.validator_count(); @@ -361,7 +361,7 @@ impl Pallet { .ok_or(Error::::InvalidKeyOwnershipProof)?; // validate equivocation proof (check votes are different and signatures are valid). - if !beefy_primitives::check_equivocation_proof(equivocation_proof) { + if !beefy_primitives::check_equivocation_proof(&equivocation_proof) { return Err(Error::::InvalidEquivocationProof.into()) } diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 387eaf8957546..058955018df09 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -18,8 +18,7 @@ use std::vec; use beefy_primitives::{ - keyring::Keyring as BeefyKeyring, Commitment, Equivocation, Payload, ValidatorSetId, - VoteMessage, + keyring::Keyring as BeefyKeyring, Commitment, Payload, ValidatorSetId, VoteMessage, }; use codec::Encode; use frame_election_provider_support::{onchain, SequentialPhragmen}; @@ -346,28 +345,18 @@ pub fn start_era(era_index: EraIndex) { } pub fn generate_equivocation_proof( - validator_set_id: ValidatorSetId, - reporter_public: BeefyId, - vote1: (u64, Payload, &BeefyKeyring), - vote2: (u64, Payload, &BeefyKeyring), + vote1: (u64, Payload, ValidatorSetId, &BeefyKeyring), + vote2: (u64, Payload, ValidatorSetId, &BeefyKeyring), ) -> EquivocationProof { - let block_number = vote1.0; - - let signed_vote = |block_number: u64, payload: Payload, keyring: &BeefyKeyring| { + let signed_vote = |block_number: u64, + payload: Payload, + validator_set_id: ValidatorSetId, + keyring: &BeefyKeyring| { let commitment = Commitment { validator_set_id, block_number, payload }; let signature = keyring.sign(&commitment.encode()); VoteMessage { commitment, id: keyring.public(), signature } }; - - let first = signed_vote(vote1.0, vote1.1, vote1.2); - let second = signed_vote(vote2.0, vote2.1, vote2.2); - EquivocationProof { - set_id: validator_set_id, - equivocation: Equivocation { - round_number: block_number, - id: reporter_public, - first, - second, - }, - } + let first = signed_vote(vote1.0, vote1.1, vote1.2, vote1.3); + let second = signed_vote(vote2.0, vote2.1, vote2.2, vote2.3); + EquivocationProof { first, second } } diff --git a/frame/beefy/src/tests.rs b/frame/beefy/src/tests.rs index 272e1f8897ba4..055cca129b77d 100644 --- a/frame/beefy/src/tests.rs +++ b/frame/beefy/src/tests.rs @@ -180,36 +180,46 @@ fn should_sign_and_verify() { // generate an equivocation proof, with two votes in the same round for // same payload signed by the same key let equivocation_proof = generate_equivocation_proof( - set_id, - BeefyKeyring::Alice.public(), - (1, payload1.clone(), &BeefyKeyring::Bob), - (1, payload1.clone(), &BeefyKeyring::Bob), + (1, payload1.clone(), set_id, &BeefyKeyring::Bob), + (1, payload1.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_equivocation_proof::<_, _, Keccak256>(equivocation_proof)); + assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes in different rounds for // different payloads signed by the same key let equivocation_proof = generate_equivocation_proof( - set_id, - BeefyKeyring::Alice.public(), - (1, payload1.clone(), &BeefyKeyring::Bob), - (2, payload2.clone(), &BeefyKeyring::Bob), + (1, payload1.clone(), set_id, &BeefyKeyring::Bob), + (2, payload2.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_equivocation_proof::<_, _, Keccak256>(equivocation_proof)); + assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + + // generate an equivocation proof, with two votes by different authorities + let equivocation_proof = generate_equivocation_proof( + (1, payload1.clone(), set_id, &BeefyKeyring::Alice), + (1, payload2.clone(), set_id, &BeefyKeyring::Bob), + ); + // expect invalid equivocation proof + assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + + // generate an equivocation proof, with two votes in different set ids + let equivocation_proof = generate_equivocation_proof( + (1, payload1.clone(), set_id, &BeefyKeyring::Bob), + (1, payload2.clone(), set_id + 1, &BeefyKeyring::Bob), + ); + // expect invalid equivocation proof + assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes in the same round for // different payloads signed by the same key let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); let equivocation_proof = generate_equivocation_proof( - set_id, - BeefyKeyring::Alice.public(), - (1, payload1, &BeefyKeyring::Bob), - (1, payload2, &BeefyKeyring::Bob), + (1, payload1, set_id, &BeefyKeyring::Bob), + (1, payload2, set_id, &BeefyKeyring::Bob), ); // expect valid equivocation proof - assert!(check_equivocation_proof::<_, _, Keccak256>(equivocation_proof)); + assert!(check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); } #[test] @@ -240,8 +250,6 @@ fn report_equivocation_current_set_works() { } assert_eq!(authorities.len(), 2); - let reporter_index = 0; - let _reporter_key = authorities[reporter_index].clone(); let equivocation_authority_index = 1; let equivocation_key = &authorities[equivocation_authority_index]; let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); @@ -251,12 +259,8 @@ fn report_equivocation_current_set_works() { // generate an equivocation proof, with two votes in the same round for // different payloads signed by the same key let equivocation_proof = generate_equivocation_proof( - set_id, - // FIXME: test only passes for equivocation auto-report, why? - // _reporter_key.clone(), - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id, &equivocation_keyring), ); // create the key ownership proof @@ -315,8 +319,6 @@ fn report_equivocation_old_set_works() { assert_eq!(authorities.len(), 2); let equivocation_authority_index = 0; let equivocation_key = &authorities[equivocation_authority_index]; - let reporter_index = 1; - let _reporter_key = authorities[reporter_index].clone(); // create the key ownership proof in the "old" set let key_owner_proof = @@ -345,12 +347,8 @@ fn report_equivocation_old_set_works() { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof for the old set, let equivocation_proof = generate_equivocation_proof( - old_set_id, - // FIXME: test only passes for equivocation auto-report, why? - // _reporter_key.clone(), - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, old_set_id, &equivocation_keyring), + (block_num, payload2, old_set_id, &equivocation_keyring), ); // report the equivocation and the tx should be dispatched successfully @@ -412,10 +410,8 @@ fn report_equivocation_invalid_set_id() { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation for a future set let equivocation_proof = generate_equivocation_proof( - set_id + 1, - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, set_id + 1, &equivocation_keyring), + (block_num, payload2, set_id + 1, &equivocation_keyring), ); // the call for reporting the equivocation should error @@ -457,10 +453,8 @@ fn report_equivocation_invalid_session() { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof at following era set id = 2 let equivocation_proof = generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id, &equivocation_keyring), ); // report an equivocation for the current set using an key ownership @@ -503,10 +497,8 @@ fn report_equivocation_invalid_key_owner_proof() { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof for the authority at index 0 let equivocation_proof = generate_equivocation_proof( - set_id + 1, - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, set_id + 1, &equivocation_keyring), + (block_num, payload2, set_id + 1, &equivocation_keyring), ); // we need to start a new era otherwise the key ownership proof won't be @@ -565,34 +557,32 @@ fn report_equivocation_invalid_equivocation_proof() { // both votes target the same block number and payload, // there is no equivocation. assert_invalid_equivocation_proof(generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1.clone(), &equivocation_keyring), - (block_num, payload1.clone(), &equivocation_keyring), + (block_num, payload1.clone(), set_id, &equivocation_keyring), + (block_num, payload1.clone(), set_id, &equivocation_keyring), )); - // votes targetting different rounds, there is no equivocation. + // votes targeting different rounds, there is no equivocation. assert_invalid_equivocation_proof(generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1.clone(), &equivocation_keyring), - (block_num + 1, payload2.clone(), &equivocation_keyring), + (block_num, payload1.clone(), set_id, &equivocation_keyring), + (block_num + 1, payload2.clone(), set_id, &equivocation_keyring), )); // votes signed with different authority keys assert_invalid_equivocation_proof(generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1.clone(), &equivocation_keyring), - (block_num, payload1.clone(), &BeefyKeyring::Charlie), + (block_num, payload1.clone(), set_id, &equivocation_keyring), + (block_num, payload1.clone(), set_id, &BeefyKeyring::Charlie), )); // votes signed with a key that isn't part of the authority set assert_invalid_equivocation_proof(generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1.clone(), &equivocation_keyring), - (block_num, payload1.clone(), &BeefyKeyring::Dave), + (block_num, payload1.clone(), set_id, &equivocation_keyring), + (block_num, payload1.clone(), set_id, &BeefyKeyring::Dave), + )); + + // votes targeting different set ids + assert_invalid_equivocation_proof(generate_equivocation_proof( + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id + 1, &equivocation_keyring), )); }); } @@ -622,10 +612,8 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); let equivocation_proof = generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id, &equivocation_keyring), ); let key_owner_proof = @@ -729,10 +717,8 @@ fn valid_equivocation_reports_dont_pay_fees() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); let equivocation_proof = generate_equivocation_proof( - set_id, - equivocation_key.clone(), - (block_num, payload1, &equivocation_keyring), - (block_num, payload2, &equivocation_keyring), + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id, &equivocation_keyring), ); // create the key ownership proof. diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 61e65b50c1b46..a35bd7dfca930 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -183,28 +183,30 @@ pub struct VoteMessage { pub signature: Signature, } -/// An equivocation (double-vote) in a given round. -#[derive(Clone, Debug, Encode, Decode, PartialEq, TypeInfo)] -pub struct Equivocation { - /// The round number equivocated in - pub round_number: Number, - /// Node authority id - pub id: Id, - /// The first vote in the equivocation - pub first: VoteMessage, - /// The second vote in the equivocation - pub second: VoteMessage, -} - /// Proof of voter misbehavior on a given set id. Misbehavior/equivocation in /// BEEFY happens when a voter votes on the same round/block for different payloads. /// Proving is achieved by collecting the signed commitments of conflicting votes. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct EquivocationProof { - /// BEEFY validator set id active during this equivocation. - pub set_id: ValidatorSetId, - /// Equivocation details including the conflicting votes. - pub equivocation: Equivocation, + /// The first vote in the equivocation. + pub first: VoteMessage, + /// The second vote in the equivocation. + pub second: VoteMessage, +} + +impl EquivocationProof { + /// Returns the authority id of the equivocator. + pub fn offender_id(&self) -> &Id { + &self.first.id + } + /// Returns the round number at which the equivocation occurred. + pub fn round_number(&self) -> &Number { + &self.first.commitment.block_number + } + /// Returns the set id at which the equivocation occurred. + pub fn set_id(&self) -> ValidatorSetId { + self.first.commitment.validator_set_id + } } /// Check a commitment signature by encoding the commitment and @@ -226,15 +228,15 @@ where /// Verifies the equivocation proof by making sure that both votes target /// different blocks and that its signatures are valid. pub fn check_equivocation_proof( - report: EquivocationProof::Signature>, + report: &EquivocationProof::Signature>, ) -> bool where Id: BeefyAuthorityId + PartialEq, Number: Clone + Encode + PartialEq, MsgHash: Hash, { - let first = &report.equivocation.first; - let second = &report.equivocation.second; + let first = &report.first; + let second = &report.second; // if votes // come from different authorities, From 7388c55537ee9c1d09226f56b87d3bfbaf5093d3 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 19 Jan 2023 10:41:22 +0200 Subject: [PATCH 32/41] client/beefy: address review comments --- client/beefy/src/round.rs | 27 +++++++++++++++++---------- client/beefy/src/worker.rs | 15 +-------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 3516dc4d8946b..2a55af0233605 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -75,7 +75,7 @@ pub enum VoteImportResult { #[derive(Debug, Decode, Encode, PartialEq)] pub(crate) struct Rounds { rounds: BTreeMap>, RoundTracker>, - equivocations: BTreeMap<(Public, NumberFor), VoteMessage, Public, Signature>>, + previous_votes: BTreeMap<(Public, NumberFor), VoteMessage, Public, Signature>>, session_start: NumberFor, validator_set: ValidatorSet, mandatory_done: bool, @@ -89,7 +89,7 @@ where pub(crate) fn new(session_start: NumberFor, validator_set: ValidatorSet) -> Self { Rounds { rounds: BTreeMap::new(), - equivocations: BTreeMap::new(), + previous_votes: BTreeMap::new(), session_start, validator_set, mandatory_done: false, @@ -108,7 +108,14 @@ where mandatory_done: bool, best_done: Option>, ) -> Self { - Rounds { rounds, equivocations, session_start, validator_set, mandatory_done, best_done } + Rounds { + rounds, + previous_votes: equivocations, + session_start, + validator_set, + mandatory_done, + best_done, + } } pub(crate) fn validator_set(&self) -> &ValidatorSet { @@ -156,21 +163,21 @@ where return VoteImportResult::Invalid } - if let Some(existing_vote) = self.equivocations.get(&equivocation_key) { + if let Some(previous_vote) = self.previous_votes.get(&equivocation_key) { // is the same public key voting for a different payload? - if existing_vote.commitment.payload != vote.commitment.payload { + if previous_vote.commitment.payload != vote.commitment.payload { debug!( target: "beefy", "🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", - existing_vote, vote + previous_vote, vote ); return VoteImportResult::Equivocation(EquivocationProof { - first: existing_vote.clone(), + first: previous_vote.clone(), second: vote, }) } } else { // this is the first vote sent by `id` for `num`, all good - self.equivocations.insert(equivocation_key, vote.clone()); + self.previous_votes.insert(equivocation_key, vote.clone()); } // add valid vote @@ -201,7 +208,7 @@ where pub(crate) fn conclude(&mut self, round_num: NumberFor) { // Remove this and older (now stale) rounds. self.rounds.retain(|commitment, _| commitment.block_number > round_num); - self.equivocations.retain(|&(_, number), _| number > round_num); + self.previous_votes.retain(|&(_, number), _| number > round_num); self.mandatory_done = self.mandatory_done || round_num == self.session_start; self.best_done = self.best_done.max(Some(round_num)); debug!(target: "beefy", "🥩 Concluded round #{}", round_num); @@ -466,7 +473,7 @@ mod tests { // conclude round 3 rounds.conclude(3); - assert!(rounds.equivocations.is_empty()); + assert!(rounds.previous_votes.is_empty()); } #[test] diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index c67fd44dcc339..34b5eb3635e5e 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -931,23 +931,10 @@ where } 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 equivocation..", - number, err - ); - error!(target: "beefy", "🥩 {}", err_msg); - 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(&BlockId::Hash(hash), validator_set_id, offender_id) + .generate_key_ownership_proof(&BlockId::Number(number), validator_set_id, offender_id) .map_err(Error::RuntimeApi)? { Some(proof) => proof, From 6ff53e530569a7ad0312ce6d38fb79e0e07016a1 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 20 Jan 2023 20:03:19 +0200 Subject: [PATCH 33/41] frame/beefy: add ValidateUnsigned to test/mock runtime --- frame/beefy/src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 058955018df09..2118ffc3d50d0 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -68,7 +68,7 @@ construct_runtime!( Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Beefy: pallet_beefy::{Pallet, Call, Config, Storage}, + Beefy: pallet_beefy::{Pallet, Call, Config, Storage, ValidateUnsigned}, Staking: pallet_staking::{Pallet, Call, Config, Storage, Event}, Session: pallet_session::{Pallet, Call, Storage, Event, Config}, Offences: pallet_offences::{Pallet, Storage, Event}, From ab11674dc4d0fe8f954cd1ec400566a955a31a54 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 24 Jan 2023 18:21:53 +0200 Subject: [PATCH 34/41] client/beefy: fixes after merge master --- client/beefy/src/round.rs | 10 ++++++---- client/beefy/src/worker.rs | 19 +++++++++++++------ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index ba244565718ec..5c702bc05d339 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -152,8 +152,10 @@ where return VoteImportResult::Stale } else if vote.commitment.validator_set_id != self.validator_set_id() { debug!( - target: LOG_TARGET, "🥩 expected set_id {:?}, ignoring vote {:?}.", - self.validator_set_id(), vote, + target: LOG_TARGET, + "🥩 expected set_id {:?}, ignoring vote {:?}.", + self.validator_set_id(), + vote, ); return VoteImportResult::Invalid } else if !self.validators().iter().any(|id| &vote.id == id) { @@ -169,8 +171,8 @@ where // is the same public key voting for a different payload? if previous_vote.commitment.payload != vote.commitment.payload { debug!( - target: "beefy", "🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", - previous_vote, vote + target: LOG_TARGET, + "🥩 detected equivocated vote: 1st: {:?}, 2nd: {:?}", previous_vote, vote ); return VoteImportResult::Equivocation(EquivocationProof { first: previous_vote.clone(), diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index ffacffe5730f7..bb9c4ccd27780 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -568,8 +568,8 @@ where let finality_proof = VersionedFinalityProof::V1(signed_commitment); info!( - target: LOG_TARGET, "🥩 Round #{} concluded, finality_proof: {:?}.", - block_number, finality_proof + target: LOG_TARGET, + "🥩 Round #{} concluded, finality_proof: {:?}.", block_number, finality_proof ); // We created the `finality_proof` and know to be valid. // New state is persisted after finalization. @@ -936,14 +936,18 @@ where let offender_id = proof.offender_id().clone(); if proof.set_id() != validator_set_id { - debug!(target: "beefy", "🥩 Skip equivocation report for old set id: {:?}", proof.set_id()); + debug!( + target: LOG_TARGET, + "🥩 Skip equivocation report for old set id: {:?}", + proof.set_id() + ); return Ok(()) } else if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) { - debug!(target: "beefy", "🥩 Skip report for bad equivocation {:?}", 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 { - debug!(target: "beefy", "🥩 Skip equivocation report for own equivocation"); + debug!(target: LOG_TARGET, "🥩 Skip equivocation report for own equivocation"); return Ok(()) } } @@ -957,7 +961,10 @@ where { Some(proof) => proof, None => { - debug!(target: "beefy", "🥩 Equivocation offender not part of the authority set."); + debug!( + target: LOG_TARGET, + "🥩 Equivocation offender not part of the authority set." + ); return Ok(()) }, }; From 711832b2615beeb8bc4f5a587f711605169306b7 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 6 Feb 2023 15:46:11 +0200 Subject: [PATCH 35/41] fix missed merge damage --- client/beefy/src/round.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index ad055e96d7f2b..55aebcd51d64b 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -37,10 +37,6 @@ pub(crate) struct RoundTracker { } impl RoundTracker { - pub(crate) fn new(votes: BTreeMap) -> Self { - Self { votes } - } - fn add_vote(&mut self, vote: (Public, Signature)) -> bool { if self.votes.contains_key(&vote.0) { return false @@ -99,27 +95,6 @@ where } } - pub(crate) fn new_manual( - rounds: BTreeMap>, RoundTracker>, - equivocations: BTreeMap< - (Public, NumberFor), - VoteMessage, Public, Signature>, - >, - session_start: NumberFor, - validator_set: ValidatorSet, - mandatory_done: bool, - best_done: Option>, - ) -> Self { - Rounds { - rounds, - previous_votes: equivocations, - session_start, - validator_set, - mandatory_done, - best_done, - } - } - pub(crate) fn validator_set(&self) -> &ValidatorSet { &self.validator_set } From ab975f45b3d3de5e62915e1767d6be4fc5c5a806 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 13 Feb 2023 13:58:27 +0200 Subject: [PATCH 36/41] client/beefy: add test for reporting equivocations Also validated there's no unexpected equivocations reported in the other tests. Signed-off-by: acatangiu --- client/beefy/src/tests.rs | 128 +++++++++++++++++++++++++++++--------- 1 file changed, 97 insertions(+), 31 deletions(-) diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 932897c775895..2096307061d2a 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -34,8 +34,9 @@ use beefy_primitives::{ keyring::Keyring as BeefyKeyring, known_payloads, mmr::MmrRootProvider, - BeefyApi, Commitment, ConsensusLog, MmrRootHash, Payload, SignedCommitment, ValidatorSet, - VersionedFinalityProof, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, MmrRootHash, OpaqueKeyOwnershipProof, + Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, + BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, }; use futures::{future, stream::FuturesUnordered, Future, StreamExt}; use parking_lot::Mutex; @@ -55,7 +56,7 @@ use sp_api::{ApiRef, ProvideRuntimeApi}; use sp_consensus::BlockOrigin; use sp_core::H256; use sp_keystore::{testing::KeyStore as TestKeystore, SyncCryptoStore, SyncCryptoStorePtr}; -use sp_mmr_primitives::{EncodableOpaqueLeaf, Error as MmrError, MmrApi, Proof}; +use sp_mmr_primitives::{Error as MmrError, MmrApi}; use sp_runtime::{ codec::Encode, generic::BlockId, @@ -73,6 +74,7 @@ fn beefy_gossip_proto_name() -> ProtocolName { const GOOD_MMR_ROOT: MmrRootHash = MmrRootHash::repeat_byte(0xbf); const BAD_MMR_ROOT: MmrRootHash = MmrRootHash::repeat_byte(0x42); +const ALTERNATE_BAD_MMR_ROOT: MmrRootHash = MmrRootHash::repeat_byte(0x13); type BeefyBlockImport = crate::BeefyBlockImport< Block, @@ -235,6 +237,8 @@ pub(crate) struct TestApi { pub beefy_genesis: u64, pub validator_set: BeefyValidatorSet, pub mmr_root_hash: MmrRootHash, + pub reported_equivocations: + Option, AuthorityId, Signature>>>>>, } impl TestApi { @@ -243,7 +247,12 @@ impl TestApi { validator_set: &BeefyValidatorSet, mmr_root_hash: MmrRootHash, ) -> Self { - TestApi { beefy_genesis, validator_set: validator_set.clone(), mmr_root_hash } + TestApi { + beefy_genesis, + validator_set: validator_set.clone(), + mmr_root_hash, + reported_equivocations: None, + } } pub fn with_validator_set(validator_set: &BeefyValidatorSet) -> Self { @@ -251,8 +260,13 @@ impl TestApi { beefy_genesis: 1, validator_set: validator_set.clone(), mmr_root_hash: GOOD_MMR_ROOT, + reported_equivocations: None, } } + + pub fn allow_equivocations(&mut self) { + self.reported_equivocations = Some(Arc::new(Mutex::new(vec![]))); + } } // compiler gets confused and warns us about unused inner @@ -276,31 +290,29 @@ sp_api::mock_impl_runtime_apis! { fn validator_set() -> Option { Some(self.inner.validator_set.clone()) } + + fn submit_report_equivocation_unsigned_extrinsic( + proof: EquivocationProof, AuthorityId, Signature>, + _dummy: OpaqueKeyOwnershipProof, + ) -> Option<()> { + if let Some(equivocations_buf) = self.inner.reported_equivocations.as_ref() { + equivocations_buf.lock().push(proof); + None + } else { + panic!("Equivocations not expected, but following proof was reported: {:?}", proof); + } + } + + fn generate_key_ownership_proof( + _dummy1: ValidatorSetId, + _dummy2: AuthorityId, + ) -> Option { Some(OpaqueKeyOwnershipProof::new(vec![])) } } impl MmrApi> for RuntimeApi { fn mmr_root() -> Result { Ok(self.inner.mmr_root_hash) } - - fn generate_proof( - _block_numbers: Vec, - _best_known_block_number: Option - ) -> Result<(Vec, Proof), MmrError> { - unimplemented!() - } - - fn verify_proof(_leaves: Vec, _proof: Proof) -> Result<(), MmrError> { - unimplemented!() - } - - fn verify_proof_stateless( - _root: MmrRootHash, - _leaves: Vec, - _proof: Proof - ) -> Result<(), MmrError> { - unimplemented!() - } } } @@ -329,7 +341,7 @@ pub(crate) fn create_beefy_keystore(authority: BeefyKeyring) -> SyncCryptoStoreP keystore } -fn voter_init_setup( +async fn voter_init_setup( net: &mut BeefyTestNet, finality: &mut futures::stream::Fuse>, api: &TestApi, @@ -344,9 +356,7 @@ fn voter_init_setup( gossip_validator, None, ); - let best_grandpa = - futures::executor::block_on(wait_for_runtime_pallet(api, &mut gossip_engine, finality)) - .unwrap(); + let best_grandpa = wait_for_runtime_pallet(api, &mut gossip_engine, finality).await.unwrap(); load_or_init_voter_state(&*backend, api, best_grandpa, 1) } @@ -979,7 +989,7 @@ async fn should_initialize_voter_at_genesis() { let api = TestApi::with_validator_set(&validator_set); // load persistent state - nothing in DB, should init at genesis - let persisted_state = voter_init_setup(&mut net, &mut finality, &api).unwrap(); + let persisted_state = voter_init_setup(&mut net, &mut finality, &api).await.unwrap(); // Test initialization at session boundary. // verify voter initialized with two sessions starting at blocks 1 and 10 @@ -1028,7 +1038,7 @@ async fn should_initialize_voter_at_custom_genesis() { net.peer(0).client().as_client().finalize_block(hashes[8], None).unwrap(); // load persistent state - nothing in DB, should init at genesis - let persisted_state = voter_init_setup(&mut net, &mut finality, &api).unwrap(); + let persisted_state = voter_init_setup(&mut net, &mut finality, &api).await.unwrap(); // Test initialization at session boundary. // verify voter initialized with single session starting at block `custom_pallet_genesis` (7) @@ -1089,7 +1099,7 @@ async fn should_initialize_voter_when_last_final_is_session_boundary() { let api = TestApi::with_validator_set(&validator_set); // load persistent state - nothing in DB, should init at session boundary - let persisted_state = voter_init_setup(&mut net, &mut finality, &api).unwrap(); + let persisted_state = voter_init_setup(&mut net, &mut finality, &api).await.unwrap(); // verify voter initialized with single session starting at block 10 assert_eq!(persisted_state.voting_oracle().sessions().len(), 1); @@ -1147,7 +1157,7 @@ async fn should_initialize_voter_at_latest_finalized() { let api = TestApi::with_validator_set(&validator_set); // load persistent state - nothing in DB, should init at last BEEFY finalized - let persisted_state = voter_init_setup(&mut net, &mut finality, &api).unwrap(); + let persisted_state = voter_init_setup(&mut net, &mut finality, &api).await.unwrap(); // verify voter initialized with single session starting at block 12 assert_eq!(persisted_state.voting_oracle().sessions().len(), 1); @@ -1204,3 +1214,59 @@ async fn beefy_finalizing_after_pallet_genesis() { // GRANDPA finalize #21 -> BEEFY finalize #20 (mandatory) and #21 finalize_block_and_wait_for_beefy(&net, peers.clone(), &[hashes[21]], &[20, 21]).await; } + +#[tokio::test] +async fn beefy_reports_equivocations() { + sp_tracing::try_init_simple(); + + let peers = [BeefyKeyring::Alice, BeefyKeyring::Bob, BeefyKeyring::Charlie]; + let validator_set = ValidatorSet::new(make_beefy_ids(&peers), 0).unwrap(); + let session_len = 10; + let min_block_delta = 4; + + let mut net = BeefyTestNet::new(3); + + // Alice votes on good MMR roots, equivocations are allowed/expected. + let mut api_alice = TestApi::with_validator_set(&validator_set); + api_alice.allow_equivocations(); + let api_alice = Arc::new(api_alice); + let alice = (0, &peers[0], api_alice.clone()); + tokio::spawn(initialize_beefy(&mut net, vec![alice], min_block_delta)); + + // Bob votes on bad MMR roots, equivocations are allowed/expected. + let mut api_bob = TestApi::new(1, &validator_set, BAD_MMR_ROOT); + api_bob.allow_equivocations(); + let api_bob = Arc::new(api_bob); + let bob = (1, &peers[1], api_bob.clone()); + tokio::spawn(initialize_beefy(&mut net, vec![bob], min_block_delta)); + + // We spawn another node voting with Bob key, on alternate bad MMR roots (equivocating). + // Equivocations are allowed/expected. + let mut api_bob_prime = TestApi::new(1, &validator_set, ALTERNATE_BAD_MMR_ROOT); + api_bob_prime.allow_equivocations(); + let api_bob_prime = Arc::new(api_bob_prime); + let bob_prime = (2, &BeefyKeyring::Bob, api_bob_prime.clone()); + tokio::spawn(initialize_beefy(&mut net, vec![bob_prime], min_block_delta)); + + // push 42 blocks including `AuthorityChange` digests every 10 blocks. + let hashes = net.generate_blocks_and_sync(42, session_len, &validator_set, false).await; + + let net = Arc::new(Mutex::new(net)); + + // Minimum BEEFY block delta is 4. + + let peers = peers.into_iter().enumerate(); + // finalize block #1 -> BEEFY should not finalize anything (each node votes on different MMR). + finalize_block_and_wait_for_beefy(&net, peers, &[hashes[1]], &[]).await; + + // Verify neither Bob or Bob_Prime report themselves as equivocating. + assert!(api_bob.reported_equivocations.as_ref().unwrap().lock().is_empty()); + assert!(api_bob_prime.reported_equivocations.as_ref().unwrap().lock().is_empty()); + + // Verify Alice reports Bob/Bob_Prime equivocation. + let alice_reported_equivocations = api_alice.reported_equivocations.as_ref().unwrap().lock(); + assert_eq!(alice_reported_equivocations.len(), 1); + let equivocation_proof = alice_reported_equivocations.get(0).unwrap(); + assert_eq!(equivocation_proof.first.id, BeefyKeyring::Bob.public()); + assert_eq!(equivocation_proof.first.commitment.block_number, 1); +} From f6bfc817cdc4803d58236170b13dca4578bc40e3 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 13 Feb 2023 14:53:22 +0200 Subject: [PATCH 37/41] sp-beefy: move test utils to their own file --- client/beefy/src/communication/gossip.rs | 4 +- client/beefy/src/justification.rs | 3 +- client/beefy/src/keystore.rs | 2 +- client/beefy/src/round.rs | 4 +- client/beefy/src/tests.rs | 9 +- client/beefy/src/worker.rs | 4 +- frame/beefy/src/mock.rs | 21 --- frame/beefy/src/tests.rs | 4 +- primitives/beefy/src/lib.rs | 81 +---------- primitives/beefy/src/mmr.rs | 56 -------- primitives/beefy/src/test_utils.rs | 164 +++++++++++++++++++++++ 11 files changed, 180 insertions(+), 172 deletions(-) create mode 100644 primitives/beefy/src/test_utils.rs diff --git a/client/beefy/src/communication/gossip.rs b/client/beefy/src/communication/gossip.rs index 5d51610214610..7e60eb11ea731 100644 --- a/client/beefy/src/communication/gossip.rs +++ b/client/beefy/src/communication/gossip.rs @@ -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::*; diff --git a/client/beefy/src/justification.rs b/client/beefy/src/justification.rs index 6f869015ba763..9f433aa6ad1d7 100644 --- a/client/beefy/src/justification.rs +++ b/client/beefy/src/justification.rs @@ -81,8 +81,7 @@ fn verify_with_validator_set( #[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; diff --git a/client/beefy/src/keystore.rs b/client/beefy/src/keystore.rs index 73788a31b7fdc..29436e36e65bb 100644 --- a/client/beefy/src/keystore.rs +++ b/client/beefy/src/keystore.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; diff --git a/client/beefy/src/round.rs b/client/beefy/src/round.rs index 55aebcd51d64b..b2c4e97a420fe 100644 --- a/client/beefy/src/round.rs +++ b/client/beefy/src/round.rs @@ -199,8 +199,8 @@ mod tests { use sc_network_test::Block; use beefy_primitives::{ - crypto::Public, keyring::Keyring, known_payloads::MMR_ROOT_ID, Commitment, - EquivocationProof, 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}; diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 2096307061d2a..1965533773884 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -31,12 +31,9 @@ use crate::{ }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, - keyring::Keyring as BeefyKeyring, - known_payloads, - mmr::MmrRootProvider, - BeefyApi, Commitment, ConsensusLog, EquivocationProof, MmrRootHash, OpaqueKeyOwnershipProof, - Payload, SignedCommitment, ValidatorSet, ValidatorSetId, VersionedFinalityProof, - BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, + known_payloads, BeefyApi, Commitment, ConsensusLog, EquivocationProof, Keyring as BeefyKeyring, + MmrRootHash, MmrRootProvider, OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, + ValidatorSetId, VersionedFinalityProof, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, }; use futures::{future, stream::FuturesUnordered, Future, StreamExt}; use parking_lot::Mutex; diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index ddad261e618b7..9bdeaaa6be6be 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1045,9 +1045,7 @@ pub(crate) mod tests { }, BeefyRPCLinks, KnownPeers, }; - use beefy_primitives::{ - keyring::Keyring, known_payloads, mmr::MmrRootProvider, Payload, SignedCommitment, - }; + use beefy_primitives::{known_payloads, Keyring, MmrRootProvider, Payload, SignedCommitment}; use futures::{future::poll_fn, task::Poll}; use parking_lot::Mutex; use sc_client_api::{Backend as BackendT, HeaderBackend}; diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 96da34a43a547..8d8b831950d8e 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -17,10 +17,6 @@ use std::vec; -use beefy_primitives::{ - keyring::Keyring as BeefyKeyring, Commitment, Payload, ValidatorSetId, VoteMessage, -}; -use codec::Encode; use frame_election_provider_support::{onchain, SequentialPhragmen}; use frame_support::{ construct_runtime, parameter_types, @@ -341,20 +337,3 @@ pub fn start_era(era_index: EraIndex) { start_session((era_index * 3).into()); assert_eq!(Staking::current_era(), Some(era_index)); } - -pub fn generate_equivocation_proof( - vote1: (u64, Payload, ValidatorSetId, &BeefyKeyring), - vote2: (u64, Payload, ValidatorSetId, &BeefyKeyring), -) -> EquivocationProof { - let signed_vote = |block_number: u64, - payload: Payload, - validator_set_id: ValidatorSetId, - keyring: &BeefyKeyring| { - let commitment = Commitment { validator_set_id, block_number, payload }; - let signature = keyring.sign(&commitment.encode()); - VoteMessage { commitment, id: keyring.public(), signature } - }; - let first = signed_vote(vote1.0, vote1.1, vote1.2, vote1.3); - let second = signed_vote(vote2.0, vote2.1, vote2.2, vote2.3); - EquivocationProof { first, second } -} diff --git a/frame/beefy/src/tests.rs b/frame/beefy/src/tests.rs index 055cca129b77d..a92f90b6107bc 100644 --- a/frame/beefy/src/tests.rs +++ b/frame/beefy/src/tests.rs @@ -18,8 +18,8 @@ use std::vec; use beefy_primitives::{ - check_equivocation_proof, keyring::Keyring as BeefyKeyring, known_payloads::MMR_ROOT_ID, - Payload, ValidatorSet, + check_equivocation_proof, generate_equivocation_proof, known_payloads::MMR_ROOT_ID, + Keyring as BeefyKeyring, Payload, ValidatorSet, }; use codec::Encode; diff --git a/primitives/beefy/src/lib.rs b/primitives/beefy/src/lib.rs index 75d5422860dd8..ad9c1b8d1432a 100644 --- a/primitives/beefy/src/lib.rs +++ b/primitives/beefy/src/lib.rs @@ -34,10 +34,14 @@ mod commitment; pub mod mmr; mod payload; +#[cfg(feature = "std")] +mod test_utils; pub mod witness; pub use commitment::{Commitment, SignedCommitment, VersionedFinalityProof}; pub use payload::{known_payloads, BeefyPayloadId, Payload, PayloadProvider}; +#[cfg(feature = "std")] +pub use test_utils::*; use codec::{Codec, Decode, Encode}; use scale_info::TypeInfo; @@ -338,83 +342,6 @@ sp_api::decl_runtime_apis! { } } -#[cfg(feature = "std")] -/// Test accounts using [`crate::crypto`] types. -pub mod keyring { - use super::*; - use sp_core::{ecdsa, keccak_256, Pair}; - use std::collections::HashMap; - use strum::IntoEnumIterator; - - /// Set of test accounts using [`crate::crypto`] types. - #[allow(missing_docs)] - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumIter)] - pub enum Keyring { - Alice, - Bob, - Charlie, - Dave, - Eve, - Ferdie, - One, - Two, - } - - impl Keyring { - /// Sign `msg`. - pub fn sign(self, msg: &[u8]) -> crypto::Signature { - // todo: use custom signature hashing type - 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) - } - - /// Get Keyring from public key. - pub fn from_public(who: &crypto::Public) -> Option { - Self::iter().find(|&k| &crypto::Public::from(k) == who) - } - } - - lazy_static::lazy_static! { - static ref PRIVATE_KEYS: HashMap = - Keyring::iter().map(|i| (i, i.pair())).collect(); - static ref PUBLIC_KEYS: HashMap = - PRIVATE_KEYS.iter().map(|(&name, pair)| (name, pair.public())).collect(); - } - - impl From for crypto::Pair { - fn from(k: Keyring) -> Self { - k.pair() - } - } - - impl From for ecdsa::Pair { - fn from(k: Keyring) -> Self { - k.pair().into() - } - } - - impl From for crypto::Public { - fn from(k: Keyring) -> Self { - (*PUBLIC_KEYS).get(&k).cloned().unwrap() - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/primitives/beefy/src/mmr.rs b/primitives/beefy/src/mmr.rs index 549d2edbdf287..423ddecea0ab6 100644 --- a/primitives/beefy/src/mmr.rs +++ b/primitives/beefy/src/mmr.rs @@ -137,62 +137,6 @@ pub fn find_mmr_root_digest(header: &B::Header) -> Option header.digest().convert_first(|l| l.try_to(id).and_then(filter)) } -#[cfg(feature = "std")] -pub use mmr_root_provider::MmrRootProvider; -#[cfg(feature = "std")] -mod mmr_root_provider { - use super::*; - use crate::{known_payloads, payload::PayloadProvider, Payload}; - use sp_api::{NumberFor, ProvideRuntimeApi}; - use sp_mmr_primitives::MmrApi; - use sp_runtime::generic::BlockId; - use sp_std::{marker::PhantomData, sync::Arc}; - - /// A [`crate::Payload`] provider where payload is Merkle Mountain Range root hash. - /// - /// Encoded payload contains a [`crate::MmrRootHash`] type (i.e. 32-bytes hash). - pub struct MmrRootProvider { - runtime: Arc, - _phantom: PhantomData, - } - - impl MmrRootProvider - where - B: Block, - R: ProvideRuntimeApi, - R::Api: MmrApi>, - { - /// Create new BEEFY Payload provider with MMR Root as payload. - pub fn new(runtime: Arc) -> Self { - Self { runtime, _phantom: PhantomData } - } - - /// Simple wrapper that gets MMR root from header digests or from client state. - fn mmr_root_from_digest_or_runtime(&self, header: &B::Header) -> Option { - find_mmr_root_digest::(header).or_else(|| { - self.runtime - .runtime_api() - .mmr_root(&BlockId::hash(header.hash())) - .ok() - .and_then(|r| r.ok()) - }) - } - } - - impl PayloadProvider for MmrRootProvider - where - B: Block, - R: ProvideRuntimeApi, - R::Api: MmrApi>, - { - fn payload(&self, header: &B::Header) -> Option { - self.mmr_root_from_digest_or_runtime(header).map(|mmr_root| { - Payload::from_single_entry(known_payloads::MMR_ROOT_ID, mmr_root.encode()) - }) - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/primitives/beefy/src/test_utils.rs b/primitives/beefy/src/test_utils.rs new file mode 100644 index 0000000000000..6a48c57ec2258 --- /dev/null +++ b/primitives/beefy/src/test_utils.rs @@ -0,0 +1,164 @@ +// This file is part of Substrate. + +// Copyright (C) 2021-2023 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#![cfg(feature = "std")] + +use crate::{ + crypto, known_payloads, mmr::find_mmr_root_digest, payload::PayloadProvider, Commitment, + EquivocationProof, MmrRootHash, Payload, ValidatorSetId, VoteMessage, +}; +use codec::Encode; +use sp_api::{NumberFor, ProvideRuntimeApi}; +use sp_core::{ecdsa, keccak_256, Pair}; +use sp_mmr_primitives::MmrApi; +use sp_runtime::{ + generic::BlockId, + traits::{Block, Header}, +}; +use sp_std::{marker::PhantomData, sync::Arc}; +use std::collections::HashMap; +use strum::IntoEnumIterator; + +/// Set of test accounts using [`crate::crypto`] types. +#[allow(missing_docs)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumIter)] +pub enum Keyring { + Alice, + Bob, + Charlie, + Dave, + Eve, + Ferdie, + One, + Two, +} + +impl Keyring { + /// Sign `msg`. + pub fn sign(self, msg: &[u8]) -> crypto::Signature { + // todo: use custom signature hashing type + 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) + } + + /// Get Keyring from public key. + pub fn from_public(who: &crypto::Public) -> Option { + Self::iter().find(|&k| &crypto::Public::from(k) == who) + } +} + +lazy_static::lazy_static! { + static ref PRIVATE_KEYS: HashMap = + Keyring::iter().map(|i| (i, i.pair())).collect(); + static ref PUBLIC_KEYS: HashMap = + PRIVATE_KEYS.iter().map(|(&name, pair)| (name, pair.public())).collect(); +} + +impl From for crypto::Pair { + fn from(k: Keyring) -> Self { + k.pair() + } +} + +impl From for ecdsa::Pair { + fn from(k: Keyring) -> Self { + k.pair().into() + } +} + +impl From for crypto::Public { + fn from(k: Keyring) -> Self { + (*PUBLIC_KEYS).get(&k).cloned().unwrap() + } +} + +/// A [`crate::Payload`] provider where payload is Merkle Mountain Range root hash. +/// +/// Encoded payload contains a [`crate::MmrRootHash`] type (i.e. 32-bytes hash). +pub struct MmrRootProvider { + runtime: Arc, + _phantom: PhantomData, +} + +impl MmrRootProvider +where + B: Block, + R: ProvideRuntimeApi, + R::Api: MmrApi>, +{ + /// Create new BEEFY Payload provider with MMR Root as payload. + pub fn new(runtime: Arc) -> Self { + Self { runtime, _phantom: PhantomData } + } + + /// Simple wrapper that gets MMR root from header digests or from client state. + fn mmr_root_from_digest_or_runtime(&self, header: &B::Header) -> Option { + find_mmr_root_digest::(header).or_else(|| { + self.runtime + .runtime_api() + .mmr_root(&BlockId::hash(header.hash())) + .ok() + .and_then(|r| r.ok()) + }) + } +} + +impl PayloadProvider for MmrRootProvider +where + B: Block, + R: ProvideRuntimeApi, + R::Api: MmrApi>, +{ + fn payload(&self, header: &B::Header) -> Option { + self.mmr_root_from_digest_or_runtime(header).map(|mmr_root| { + Payload::from_single_entry(known_payloads::MMR_ROOT_ID, mmr_root.encode()) + }) + } +} + +/// Create a new `EquivocationProof` based on given arguments. +pub fn generate_equivocation_proof( + vote1: (u64, Payload, ValidatorSetId, &Keyring), + vote2: (u64, Payload, ValidatorSetId, &Keyring), +) -> EquivocationProof { + let signed_vote = |block_number: u64, + payload: Payload, + validator_set_id: ValidatorSetId, + keyring: &Keyring| { + let commitment = Commitment { validator_set_id, block_number, payload }; + let signature = keyring.sign(&commitment.encode()); + VoteMessage { commitment, id: keyring.public(), signature } + }; + let first = signed_vote(vote1.0, vote1.1, vote1.2, vote1.3); + let second = signed_vote(vote2.0, vote2.1, vote2.2, vote2.3); + EquivocationProof { first, second } +} From 02e166e40124d357f6cf41f69c1f0345c9f2a49e Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 13 Feb 2023 15:33:53 +0200 Subject: [PATCH 38/41] client/beefy: add negative test for equivocation reports --- client/beefy/src/worker.rs | 66 +++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 9bdeaaa6be6be..1b5e101a83856 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1045,7 +1045,10 @@ pub(crate) mod tests { }, BeefyRPCLinks, KnownPeers, }; - use beefy_primitives::{known_payloads, Keyring, MmrRootProvider, Payload, SignedCommitment}; + use beefy_primitives::{ + generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, Keyring, + MmrRootProvider, Payload, SignedCommitment, + }; use futures::{future::poll_fn, task::Poll}; use parking_lot::Mutex; use sc_client_api::{Backend as BackendT, HeaderBackend}; @@ -1597,4 +1600,65 @@ pub(crate) mod tests { assert_eq!(votes.next().unwrap().first().unwrap().commitment.block_number, 21); assert_eq!(votes.next().unwrap().first().unwrap().commitment.block_number, 22); } + + #[tokio::test] + async fn should_not_report_bad_old_or_self_equivocations() { + let block_num = 1; + let set_id = 1; + let keys = [Keyring::Alice]; + let validator_set = ValidatorSet::new(make_beefy_ids(&keys), set_id).unwrap(); + // Alice votes on good MMR roots, equivocations are allowed/expected + let mut api_alice = TestApi::with_validator_set(&validator_set); + api_alice.allow_equivocations(); + let api_alice = Arc::new(api_alice); + + 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(); + + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + + // generate an equivocation proof, with Bob as perpetrator + let good_proof = generate_equivocation_proof( + (block_num, payload1.clone(), set_id, &Keyring::Bob), + (block_num, payload2.clone(), set_id, &Keyring::Bob), + ); + { + // expect voter (Alice) to successfully report it + assert_eq!(worker.report_equivocation(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); + assert_eq!(*reported.get(0).unwrap(), good_proof); + } + api_alice.reported_equivocations.as_ref().unwrap().lock().clear(); + + // now let's try with a bad proof + 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(())); + // verify nothing reported to runtime + assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty()); + + // now let's try with old set it + let mut old_proof = good_proof.clone(); + 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(())); + // verify nothing reported to runtime + assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty()); + + // now let's try reporting a self-equivocation + let self_proof = generate_equivocation_proof( + (block_num, payload1.clone(), set_id, &Keyring::Alice), + (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(())); + // verify nothing reported to runtime + assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty()); + } } From 42ef521bec7b87fc7303ab5cdc2a4af38ff96838 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 13 Feb 2023 15:58:52 +0200 Subject: [PATCH 39/41] sp-beefy: move back MmrRootProvider - used in polkadot-service --- client/beefy/src/tests.rs | 8 +++-- client/beefy/src/worker.rs | 4 +-- primitives/beefy/src/mmr.rs | 56 ++++++++++++++++++++++++++++++ primitives/beefy/src/test_utils.rs | 56 +----------------------------- 4 files changed, 64 insertions(+), 60 deletions(-) diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 1965533773884..8320a8a31e134 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -31,9 +31,11 @@ use crate::{ }; use beefy_primitives::{ crypto::{AuthorityId, Signature}, - known_payloads, BeefyApi, Commitment, ConsensusLog, EquivocationProof, Keyring as BeefyKeyring, - MmrRootHash, MmrRootProvider, OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, - ValidatorSetId, VersionedFinalityProof, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, + known_payloads, + mmr::MmrRootProvider, + BeefyApi, Commitment, ConsensusLog, EquivocationProof, Keyring as BeefyKeyring, MmrRootHash, + OpaqueKeyOwnershipProof, Payload, SignedCommitment, ValidatorSet, ValidatorSetId, + VersionedFinalityProof, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, }; use futures::{future, stream::FuturesUnordered, Future, StreamExt}; use parking_lot::Mutex; diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 1b5e101a83856..299e9d9d1ab7c 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1046,8 +1046,8 @@ pub(crate) mod tests { BeefyRPCLinks, KnownPeers, }; use beefy_primitives::{ - generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, Keyring, - MmrRootProvider, Payload, SignedCommitment, + generate_equivocation_proof, known_payloads, known_payloads::MMR_ROOT_ID, + mmr::MmrRootProvider, Keyring, Payload, SignedCommitment, }; use futures::{future::poll_fn, task::Poll}; use parking_lot::Mutex; diff --git a/primitives/beefy/src/mmr.rs b/primitives/beefy/src/mmr.rs index 423ddecea0ab6..549d2edbdf287 100644 --- a/primitives/beefy/src/mmr.rs +++ b/primitives/beefy/src/mmr.rs @@ -137,6 +137,62 @@ pub fn find_mmr_root_digest(header: &B::Header) -> Option header.digest().convert_first(|l| l.try_to(id).and_then(filter)) } +#[cfg(feature = "std")] +pub use mmr_root_provider::MmrRootProvider; +#[cfg(feature = "std")] +mod mmr_root_provider { + use super::*; + use crate::{known_payloads, payload::PayloadProvider, Payload}; + use sp_api::{NumberFor, ProvideRuntimeApi}; + use sp_mmr_primitives::MmrApi; + use sp_runtime::generic::BlockId; + use sp_std::{marker::PhantomData, sync::Arc}; + + /// A [`crate::Payload`] provider where payload is Merkle Mountain Range root hash. + /// + /// Encoded payload contains a [`crate::MmrRootHash`] type (i.e. 32-bytes hash). + pub struct MmrRootProvider { + runtime: Arc, + _phantom: PhantomData, + } + + impl MmrRootProvider + where + B: Block, + R: ProvideRuntimeApi, + R::Api: MmrApi>, + { + /// Create new BEEFY Payload provider with MMR Root as payload. + pub fn new(runtime: Arc) -> Self { + Self { runtime, _phantom: PhantomData } + } + + /// Simple wrapper that gets MMR root from header digests or from client state. + fn mmr_root_from_digest_or_runtime(&self, header: &B::Header) -> Option { + find_mmr_root_digest::(header).or_else(|| { + self.runtime + .runtime_api() + .mmr_root(&BlockId::hash(header.hash())) + .ok() + .and_then(|r| r.ok()) + }) + } + } + + impl PayloadProvider for MmrRootProvider + where + B: Block, + R: ProvideRuntimeApi, + R::Api: MmrApi>, + { + fn payload(&self, header: &B::Header) -> Option { + self.mmr_root_from_digest_or_runtime(header).map(|mmr_root| { + Payload::from_single_entry(known_payloads::MMR_ROOT_ID, mmr_root.encode()) + }) + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/primitives/beefy/src/test_utils.rs b/primitives/beefy/src/test_utils.rs index 6a48c57ec2258..1a5e03be33fb4 100644 --- a/primitives/beefy/src/test_utils.rs +++ b/primitives/beefy/src/test_utils.rs @@ -17,19 +17,9 @@ #![cfg(feature = "std")] -use crate::{ - crypto, known_payloads, mmr::find_mmr_root_digest, payload::PayloadProvider, Commitment, - EquivocationProof, MmrRootHash, Payload, ValidatorSetId, VoteMessage, -}; +use crate::{crypto, Commitment, EquivocationProof, Payload, ValidatorSetId, VoteMessage}; use codec::Encode; -use sp_api::{NumberFor, ProvideRuntimeApi}; use sp_core::{ecdsa, keccak_256, Pair}; -use sp_mmr_primitives::MmrApi; -use sp_runtime::{ - generic::BlockId, - traits::{Block, Header}, -}; -use sp_std::{marker::PhantomData, sync::Arc}; use std::collections::HashMap; use strum::IntoEnumIterator; @@ -101,50 +91,6 @@ impl From for crypto::Public { } } -/// A [`crate::Payload`] provider where payload is Merkle Mountain Range root hash. -/// -/// Encoded payload contains a [`crate::MmrRootHash`] type (i.e. 32-bytes hash). -pub struct MmrRootProvider { - runtime: Arc, - _phantom: PhantomData, -} - -impl MmrRootProvider -where - B: Block, - R: ProvideRuntimeApi, - R::Api: MmrApi>, -{ - /// Create new BEEFY Payload provider with MMR Root as payload. - pub fn new(runtime: Arc) -> Self { - Self { runtime, _phantom: PhantomData } - } - - /// Simple wrapper that gets MMR root from header digests or from client state. - fn mmr_root_from_digest_or_runtime(&self, header: &B::Header) -> Option { - find_mmr_root_digest::(header).or_else(|| { - self.runtime - .runtime_api() - .mmr_root(&BlockId::hash(header.hash())) - .ok() - .and_then(|r| r.ok()) - }) - } -} - -impl PayloadProvider for MmrRootProvider -where - B: Block, - R: ProvideRuntimeApi, - R::Api: MmrApi>, -{ - fn payload(&self, header: &B::Header) -> Option { - self.mmr_root_from_digest_or_runtime(header).map(|mmr_root| { - Payload::from_single_entry(known_payloads::MMR_ROOT_ID, mmr_root.encode()) - }) - } -} - /// Create a new `EquivocationProof` based on given arguments. pub fn generate_equivocation_proof( vote1: (u64, Payload, ValidatorSetId, &Keyring), From 282d18f533f276dc0190068234b7a0813e0f09c6 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 16 Feb 2023 11:56:24 +0200 Subject: [PATCH 40/41] impl review suggestions --- client/beefy/src/worker.rs | 9 +-------- frame/beefy/src/lib.rs | 2 +- test-utils/runtime/src/lib.rs | 1 + 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 299e9d9d1ab7c..863000cc0533a 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -935,14 +935,7 @@ where let (validators, validator_set_id) = (rounds.validators(), rounds.validator_set_id()); let offender_id = proof.offender_id().clone(); - if proof.set_id() != validator_set_id { - debug!( - target: LOG_TARGET, - "🥩 Skip equivocation report for old set id: {:?}", - proof.set_id() - ); - return Ok(()) - } else if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) { + 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) { diff --git a/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index 7424b876477ad..d29440457f5bf 100644 --- a/frame/beefy/src/lib.rs +++ b/frame/beefy/src/lib.rs @@ -69,7 +69,7 @@ pub mod pallet { /// Authority identifier type type BeefyId: Member + Parameter - // todo: use custom signature hashing type + // todo: use custom signature hashing type instead of hardcoded `Keccak256` + BeefyAuthorityId + MaybeSerializeDeserialize + MaxEncodedLen; diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 9887fd2f3075b..35906107015c5 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -980,6 +980,7 @@ cfg_if! { fn validator_set() -> Option> { None } + fn submit_report_equivocation_unsigned_extrinsic( _equivocation_proof: beefy_primitives::EquivocationProof< NumberFor, From f070f6306fb32b4de32c219863035f192b92f3b6 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 16 Feb 2023 15:08:13 +0200 Subject: [PATCH 41/41] client/beefy: add equivocation metrics --- client/beefy/src/metrics.rs | 47 ++++++++++++++++++++++++------------- client/beefy/src/worker.rs | 9 +++---- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index e3b48e6ec8d54..c74fb420d13f4 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -46,10 +46,16 @@ pub struct VoterMetrics { pub beefy_no_authority_found_in_store: Counter, /// Number of currently buffered votes pub beefy_buffered_votes: Gauge, - /// Number of valid but stale votes received - pub beefy_stale_votes: Counter, /// Number of votes dropped due to full buffers pub beefy_buffered_votes_dropped: Counter, + /// Number of good votes successfully handled + pub beefy_good_votes_processed: Counter, + /// Number of equivocation votes received + pub beefy_equivocation_votes: Counter, + /// Number of invalid votes received + pub beefy_invalid_votes: Counter, + /// Number of valid but stale votes received + pub beefy_stale_votes: Counter, /// Number of currently buffered justifications pub beefy_buffered_justifications: Gauge, /// Number of valid but stale justifications received @@ -60,8 +66,6 @@ pub struct VoterMetrics { pub beefy_buffered_justifications_dropped: Counter, /// Trying to set Best Beefy block to old block pub beefy_best_block_set_last_failure: Gauge, - /// Number of Successful handled votes - pub beefy_successful_handled_votes: Counter, } 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, - )?, }) } } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index b4ec762c8a37d..8783997cc6b22 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -578,6 +578,7 @@ where // We created the `finality_proof` and know to be valid. // New state is persisted after finalization. self.finalize(finality_proof)?; + metric_inc!(self, beefy_good_votes_processed); }, VoteImportResult::Ok => { // Persist state after handling mandatory block vote. @@ -590,13 +591,15 @@ where crate::aux_schema::write_voter_state(&*self.backend, &self.persisted_state) .map_err(|e| Error::Backend(e.to_string()))?; } + metric_inc!(self, beefy_good_votes_processed); }, VoteImportResult::Equivocation(proof) => { + metric_inc!(self, beefy_equivocation_votes); self.report_equivocation(proof)?; }, - VoteImportResult::Invalid | VoteImportResult::Stale => (), + VoteImportResult::Invalid => metric_inc!(self, beefy_invalid_votes), + VoteImportResult::Stale => metric_inc!(self, beefy_stale_votes), }; - metric_inc!(self, beefy_successful_handled_votes); Ok(()) } @@ -818,8 +821,6 @@ where } self.gossip_engine.gossip_message(topic::(), encoded_message, false); - self.persisted_state.best_voted = target_number; - metric_set!(self, beefy_best_voted, target_number); // Persist state after vote to avoid double voting in case of voter restarts. self.persisted_state.best_voted = target_number;