From 3b83e0f7ca3c325f00eb9c84ddc15fa0547662b2 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Fri, 2 May 2025 12:04:48 +0200 Subject: [PATCH 01/10] feat: added equivocation evidences to Decided event --- code/Cargo.lock | 3 +++ code/crates/core-consensus/Cargo.toml | 1 + code/crates/core-consensus/src/effect.rs | 2 ++ code/crates/core-consensus/src/handle/decide.rs | 8 +++++++- code/crates/core-driver/src/lib.rs | 1 + code/crates/core-votekeeper/src/lib.rs | 2 ++ code/crates/engine/Cargo.toml | 2 ++ code/crates/engine/src/consensus.rs | 14 ++++++++++++-- code/crates/engine/src/util/events.rs | 12 ++++++++++-- code/crates/test/framework/src/lib.rs | 2 +- 10 files changed, 41 insertions(+), 6 deletions(-) diff --git a/code/Cargo.lock b/code/Cargo.lock index 1cd359292..5b67bea01 100644 --- a/code/Cargo.lock +++ b/code/Cargo.lock @@ -2177,6 +2177,7 @@ dependencies = [ "genawaiter", "informalsystems-malachitebft-core-driver", "informalsystems-malachitebft-core-types", + "informalsystems-malachitebft-core-votekeeper", "informalsystems-malachitebft-metrics", "informalsystems-malachitebft-peer", "informalsystems-malachitebft-test", @@ -2269,7 +2270,9 @@ dependencies = [ "informalsystems-malachitebft-codec", "informalsystems-malachitebft-config", "informalsystems-malachitebft-core-consensus", + "informalsystems-malachitebft-core-driver", "informalsystems-malachitebft-core-types", + "informalsystems-malachitebft-core-votekeeper", "informalsystems-malachitebft-metrics", "informalsystems-malachitebft-network", "informalsystems-malachitebft-sync", diff --git a/code/crates/core-consensus/Cargo.toml b/code/crates/core-consensus/Cargo.toml index 8a212dd8d..8d14cd282 100644 --- a/code/crates/core-consensus/Cargo.toml +++ b/code/crates/core-consensus/Cargo.toml @@ -21,6 +21,7 @@ debug = ["std", "malachitebft-core-driver/debug"] [dependencies] malachitebft-core-types.workspace = true malachitebft-core-driver.workspace = true +malachitebft-core-votekeeper.workspace = true malachitebft-metrics = { workspace = true, optional = true } malachitebft-peer.workspace = true diff --git a/code/crates/core-consensus/src/effect.rs b/code/crates/core-consensus/src/effect.rs index b0c2425c6..70ea65dc3 100644 --- a/code/crates/core-consensus/src/effect.rs +++ b/code/crates/core-consensus/src/effect.rs @@ -133,6 +133,8 @@ where Decide( CommitCertificate, VoteExtensions, + malachitebft_core_driver::EvidenceMap, + malachitebft_core_votekeeper::EvidenceMap, resume::Continue, ), diff --git a/code/crates/core-consensus/src/handle/decide.rs b/code/crates/core-consensus/src/handle/decide.rs index 3a55c5d90..006981a2e 100644 --- a/code/crates/core-consensus/src/handle/decide.rs +++ b/code/crates/core-consensus/src/handle/decide.rs @@ -102,7 +102,13 @@ where perform!( co, - Effect::Decide(certificate, extensions, Default::default()) + Effect::Decide( + certificate, + extensions, + state.driver.evidence().clone(), + state.driver.votes().evidence().clone(), + Default::default() + ) ); Ok(()) diff --git a/code/crates/core-driver/src/lib.rs b/code/crates/core-driver/src/lib.rs index 7bfe463b6..016b5a3d8 100644 --- a/code/crates/core-driver/src/lib.rs +++ b/code/crates/core-driver/src/lib.rs @@ -26,5 +26,6 @@ pub use driver::Driver; pub use error::Error; pub use input::Input; pub use output::Output; +pub use proposal_keeper::EvidenceMap; pub use malachitebft_core_votekeeper::ThresholdParams; diff --git a/code/crates/core-votekeeper/src/lib.rs b/code/crates/core-votekeeper/src/lib.rs index 844492c83..82abde34f 100644 --- a/code/crates/core-votekeeper/src/lib.rs +++ b/code/crates/core-votekeeper/src/lib.rs @@ -20,6 +20,8 @@ pub mod round_votes; pub mod round_weights; pub mod value_weights; +pub use evidence::EvidenceMap; + /// Represents the weight of a vote, /// ie. the voting power of the validator that cast the vote. pub type Weight = malachitebft_core_types::VotingPower; diff --git a/code/crates/engine/Cargo.toml b/code/crates/engine/Cargo.toml index 718d64d3f..f286313b4 100644 --- a/code/crates/engine/Cargo.toml +++ b/code/crates/engine/Cargo.toml @@ -19,7 +19,9 @@ workspace = true malachitebft-codec.workspace = true malachitebft-config.workspace = true malachitebft-core-consensus.workspace = true +malachitebft-core-driver.workspace = true malachitebft-core-types.workspace = true +malachitebft-core-votekeeper.workspace = true malachitebft-network.workspace = true malachitebft-metrics.workspace = true malachitebft-sync.workspace = true diff --git a/code/crates/engine/src/consensus.rs b/code/crates/engine/src/consensus.rs index 9f1c56bbc..63e9baede 100644 --- a/code/crates/engine/src/consensus.rs +++ b/code/crates/engine/src/consensus.rs @@ -1085,12 +1085,22 @@ where Ok(r.resume_with(())) } - Effect::Decide(certificate, extensions, r) => { + Effect::Decide( + certificate, + extensions, + proposal_equivocation_evidence_map, + vote_equivocation_evidence_map, + r, + ) => { assert!(!certificate.commit_signatures.is_empty()); self.wal_flush(state.phase).await?; - self.tx_event.send(|| Event::Decided(certificate.clone())); + self.tx_event.send(|| Event::Decided { + commit_certificate: certificate.clone(), + proposal_equivocation_evidence_map, + vote_equivocation_evidence_map, + }); let height = certificate.height; diff --git a/code/crates/engine/src/util/events.rs b/code/crates/engine/src/util/events.rs index 0df9e166a..986e79235 100644 --- a/code/crates/engine/src/util/events.rs +++ b/code/crates/engine/src/util/events.rs @@ -47,7 +47,11 @@ pub enum Event { Published(SignedConsensusMsg), ProposedValue(LocallyProposedValue), ReceivedProposedValue(ProposedValue, ValueOrigin), - Decided(CommitCertificate), + Decided { + commit_certificate: CommitCertificate, + proposal_equivocation_evidence_map: malachitebft_core_driver::EvidenceMap, + vote_equivocation_evidence_map: malachitebft_core_votekeeper::EvidenceMap, + }, Rebroadcast(SignedVote), RequestedVoteSet(Ctx::Height, Round), SentVoteSetResponse(Ctx::Height, Round, usize, usize), @@ -74,7 +78,11 @@ impl fmt::Display for Event { "ReceivedProposedValue(value: {value:?}, origin: {origin:?})" ) } - Event::Decided(cert) => write!(f, "Decided(value: {})", cert.value_id), + Event::Decided { + commit_certificate, + proposal_equivocation_evidence_map: _, + vote_equivocation_evidence_map: _, + } => write!(f, "Decided(value: {})", commit_certificate.value_id), Event::Rebroadcast(msg) => write!(f, "Rebroadcast(msg: {msg:?})"), Event::RequestedVoteSet(height, round) => { write!(f, "RequestedVoteSet(height: {height}, round: {round})") diff --git a/code/crates/test/framework/src/lib.rs b/code/crates/test/framework/src/lib.rs index 8b62bfac5..e01aae3ef 100644 --- a/code/crates/test/framework/src/lib.rs +++ b/code/crates/test/framework/src/lib.rs @@ -231,7 +231,7 @@ where Event::StartedHeight(height, _is_restart) => { current_height.store(height.as_u64() as usize, Ordering::SeqCst); } - Event::Decided(_) => { + Event::Decided { .. } => { decisions.fetch_add(1, Ordering::SeqCst); } Event::Published(msg) if is_full_node => { From 3dffbd0a5b17be28ed46fe0f509cd68815c834b6 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Tue, 6 May 2025 13:05:40 +0200 Subject: [PATCH 02/10] feat: vote and proposal equivocation event --- code/crates/core-driver/src/driver.rs | 67 ++++++++++++++- code/crates/core-driver/src/mux.rs | 16 +++- .../crates/core-driver/src/proposal_keeper.rs | 15 +++- code/crates/core-votekeeper/src/keeper.rs | 18 ++-- .../core-votekeeper/tests/vote_keeper.rs | 85 ++++++++++--------- code/crates/engine/src/consensus.rs | 27 ++++++ code/crates/engine/src/util/events.rs | 34 +++++++- .../test/mbt/src/tests/votekeeper/runner.rs | 13 ++- 8 files changed, 216 insertions(+), 59 deletions(-) diff --git a/code/crates/core-driver/src/driver.rs b/code/crates/core-driver/src/driver.rs index 3853a147b..21f16b222 100644 --- a/code/crates/core-driver/src/driver.rs +++ b/code/crates/core-driver/src/driver.rs @@ -11,8 +11,8 @@ use malachitebft_core_types::{ SignedProposal, SignedVote, Timeout, TimeoutKind, Validator, ValidatorSet, Validity, Value, ValueId, Vote, VoteType, }; -use malachitebft_core_votekeeper::keeper::Output as VKOutput; use malachitebft_core_votekeeper::keeper::VoteKeeper; +use malachitebft_core_votekeeper::keeper::{Output as VKOutput, RecordVoteError}; use crate::input::Input; use crate::output::Output; @@ -45,9 +45,23 @@ where /// The proposals to decide on. pub(crate) proposal_keeper: ProposalKeeper, + /// The evidence of equivocation (if any) after processing a proposal. + pub(crate) proposal_equivocation_evidence: Option<( + Ctx::Height, + Ctx::Address, + (SignedProposal, SignedProposal), + )>, + /// The vote keeper. pub(crate) vote_keeper: VoteKeeper, + /// The evidence of equivocation (if any) after processing a vote. + pub(crate) vote_equivocation_evidence: Option<( + Ctx::Height, + Ctx::Address, + (SignedVote, SignedVote), + )>, + /// The commit certificates pub(crate) commit_certificates: Vec>, @@ -90,7 +104,9 @@ where threshold_params, validator_set, proposal_keeper, + proposal_equivocation_evidence: None, vote_keeper, + vote_equivocation_evidence: None, round_state, proposer: None, pending_inputs: vec![], @@ -197,6 +213,38 @@ where self.proposal_keeper.evidence() } + /// Return proposal equivocation evidence, if any. + pub fn proposal_equivocation_evidence( + &self, + ) -> Option<( + Ctx::Height, + Ctx::Address, + (SignedProposal, SignedProposal), + )> { + self.proposal_equivocation_evidence.clone() + } + + /// Clear the proposal equivocation evidence. + pub fn clear_proposal_equivocation_evidence(&mut self) { + self.proposal_equivocation_evidence = None; + } + + /// Return vote equivocation evidence, if any. + pub fn vote_equivocation_evidence( + &self, + ) -> Option<( + Ctx::Height, + Ctx::Address, + (SignedVote, SignedVote), + )> { + self.vote_equivocation_evidence.clone() + } + + /// Clear the vote equivocation evidence. + pub fn clear_vote_equivocation_evidence(&mut self) { + self.vote_equivocation_evidence = None; + } + /// Return the proposer for the current round. pub fn get_proposer(&self) -> Result<&Ctx::Validator, Error> { if let Some(proposer) = &self.proposer { @@ -449,8 +497,21 @@ where let vote_round = vote.round(); let this_round = self.round(); - let Some(output) = self.vote_keeper.apply_vote(vote, this_round) else { - return Ok(None); + let validator_address = vote.validator_address().clone(); + let height = vote.height(); + + let output = match self.vote_keeper.apply_vote(vote, this_round) { + Ok(Some(output)) => output, + Ok(None) => return Ok(None), + Err(RecordVoteError::ConflictingVote { + existing, + conflicting, + }) => { + self.vote_equivocation_evidence = + Some((height, validator_address, (existing, conflicting))); + + return Ok(None); + } }; if let VKOutput::PolkaValue(val) = &output { diff --git a/code/crates/core-driver/src/mux.rs b/code/crates/core-driver/src/mux.rs index 0df29a2b2..268d563b1 100644 --- a/code/crates/core-driver/src/mux.rs +++ b/code/crates/core-driver/src/mux.rs @@ -37,6 +37,7 @@ use malachitebft_core_votekeeper::keeper::Output as VKOutput; use malachitebft_core_votekeeper::keeper::VoteKeeper; use malachitebft_core_votekeeper::Threshold; +use crate::proposal_keeper::RecordProposalError; use crate::Driver; impl Driver @@ -199,8 +200,19 @@ where let proposal = signed_proposal.message.clone(); // Store the proposal and its validity - self.proposal_keeper - .store_proposal(signed_proposal, validity); + if let Err(RecordProposalError::ConflictingProposal { + existing, + conflicting, + }) = self + .proposal_keeper + .store_proposal(signed_proposal, validity) + { + let validator_address = existing.validator_address().clone(); + let height = existing.height(); + + self.proposal_equivocation_evidence = + Some((height, validator_address, (existing, conflicting))); + } self.multiplex_proposal(proposal, validity) } diff --git a/code/crates/core-driver/src/proposal_keeper.rs b/code/crates/core-driver/src/proposal_keeper.rs index ffd85027b..1ddf27029 100644 --- a/code/crates/core-driver/src/proposal_keeper.rs +++ b/code/crates/core-driver/src/proposal_keeper.rs @@ -126,18 +126,27 @@ where /// /// # Precondition /// - The given proposal must have been proposed by the expected proposer at the proposal's height and round. - pub fn store_proposal(&mut self, proposal: SignedProposal, validity: Validity) { + pub fn store_proposal( + &mut self, + proposal: SignedProposal, + validity: Validity, + ) -> Result<(), RecordProposalError> { let per_round = self.per_round.entry(proposal.round()).or_default(); match per_round.add(proposal, validity) { - Ok(()) => (), + Ok(()) => Ok(()), Err(RecordProposalError::ConflictingProposal { existing, conflicting, }) => { // This is an equivocating proposal - self.evidence.add(existing, conflicting); + self.evidence.add(existing.clone(), conflicting.clone()); + + Err(RecordProposalError::ConflictingProposal { + existing, + conflicting, + }) } Err(RecordProposalError::InvalidConflictingProposal { diff --git a/code/crates/core-votekeeper/src/keeper.rs b/code/crates/core-votekeeper/src/keeper.rs index aef838ac9..1caf981bf 100644 --- a/code/crates/core-votekeeper/src/keeper.rs +++ b/code/crates/core-votekeeper/src/keeper.rs @@ -56,7 +56,7 @@ where } /// Errors can that be yielded when recording a vote. -#[derive(Error)] +#[derive(Error, Debug, PartialEq, Eq)] pub enum RecordVoteError where Ctx: Context, @@ -217,13 +217,13 @@ where &mut self, vote: SignedVote, round: Round, - ) -> Option>> { + ) -> Result>>, RecordVoteError> { let total_weight = self.total_weight(); let per_round = self.per_round.entry(vote.round()).or_default(); let Some(validator) = self.validator_set.get_by_address(vote.validator_address()) else { // Vote from unknown validator, let's discard it. - return None; + return Ok(None); }; match per_round.add(vote.clone(), validator.voting_power()) { @@ -234,8 +234,10 @@ where }) => { // This is an equivocating vote self.evidence.add(existing.clone(), conflicting); - //panic!("Equivocating vote {:?}, existing {:?}", &vote, &existing); - return None; + return Err(RecordVoteError::ConflictingVote { + existing, + conflicting: vote, + }); } } @@ -250,7 +252,7 @@ where if skip_round { let output = Output::SkipRound(vote.round()); per_round.emitted_outputs.insert(output.clone()); - return Some(output); + return Ok(Some(output)); } } @@ -268,9 +270,9 @@ where // Ensure we do not output the same message twice Some(output) if !per_round.emitted_outputs.contains(&output) => { per_round.emitted_outputs.insert(output.clone()); - Some(output) + Ok(Some(output)) } - _ => None, + _ => Ok(None), } } diff --git a/code/crates/core-votekeeper/tests/vote_keeper.rs b/code/crates/core-votekeeper/tests/vote_keeper.rs index 2196a144b..d8cf95c17 100644 --- a/code/crates/core-votekeeper/tests/vote_keeper.rs +++ b/code/crates/core-votekeeper/tests/vote_keeper.rs @@ -1,7 +1,5 @@ +use informalsystems_malachitebft_core_votekeeper::keeper::{Output, RecordVoteError, VoteKeeper}; use malachitebft_core_types::{NilOrVal, Round, SignedVote}; - -use informalsystems_malachitebft_core_votekeeper::keeper::{Output, VoteKeeper}; - use malachitebft_test::{ Address, Height, PrivateKey, Signature, TestContext, Validator, ValidatorSet, ValueId, Vote, }; @@ -51,15 +49,15 @@ fn prevote_apply_nil() { let vote = new_signed_prevote(height, round, NilOrVal::Nil, addr1); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, round, NilOrVal::Nil, addr2); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, round, NilOrVal::Nil, addr3); let msg = keeper.apply_vote(vote, round); - assert_eq!(msg, Some(Output::PolkaNil)); + assert!(matches!(msg, Ok(Some(Output::PolkaNil)))); } #[test] @@ -71,15 +69,15 @@ fn precommit_apply_nil() { let vote = new_signed_precommit(height, round, NilOrVal::Nil, addr1); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, Round::new(0), NilOrVal::Nil, addr2); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, Round::new(0), NilOrVal::Nil, addr3); let msg = keeper.apply_vote(vote, round); - assert_eq!(msg, Some(Output::PrecommitAny)); + assert!(matches!(msg, Ok(Some(Output::PrecommitAny)))); } #[test] @@ -93,19 +91,19 @@ fn prevote_apply_single_value() { let vote = new_signed_prevote(height, Round::new(0), val, addr1); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, Round::new(0), val, addr2); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote_nil = new_signed_prevote(height, Round::new(0), NilOrVal::Nil, addr3); let msg = keeper.apply_vote(vote_nil, round); - assert_eq!(msg, Some(Output::PolkaAny)); + assert!(matches!(msg, Ok(Some(Output::PolkaAny)))); let vote = new_signed_prevote(height, Round::new(0), val, addr4); let msg = keeper.apply_vote(vote, round); - assert_eq!(msg, Some(Output::PolkaValue(id))); + assert!(matches!(msg, Ok(Some(Output::PolkaValue(x))) if x == id)); } #[test] @@ -119,19 +117,19 @@ fn precommit_apply_single_value() { let vote = new_signed_precommit(height, Round::new(0), val, addr1); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, Round::new(0), val, addr2); let msg = keeper.apply_vote(vote.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote_nil = new_signed_precommit(height, Round::new(0), NilOrVal::Nil, addr3); let msg = keeper.apply_vote(vote_nil, round); - assert_eq!(msg, Some(Output::PrecommitAny)); + assert!(matches!(msg, Ok(Some(Output::PrecommitAny)))); let vote = new_signed_precommit(height, Round::new(0), val, addr4); let msg = keeper.apply_vote(vote, round); - assert_eq!(msg, Some(Output::PrecommitValue(id))); + assert!(matches!(msg, Ok(Some(Output::PrecommitValue(x))) if x == id)); } #[test] @@ -146,15 +144,15 @@ fn skip_round_small_quorum_prevotes_two_vals() { let vote = new_signed_prevote(height, cur_round, val, addr1); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, fut_round, val, addr3); let msg = keeper.apply_vote(vote, cur_round); - assert_eq!(msg, Some(Output::SkipRound(Round::new(1)))); + assert!(matches!(msg, Ok(Some(Output::SkipRound(r))) if r == Round::new(1))); } #[test] @@ -169,15 +167,15 @@ fn skip_round_small_quorum_with_prevote_precommit_two_vals() { let vote = new_signed_prevote(height, cur_round, val, addr1); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, fut_round, val, addr3); let msg = keeper.apply_vote(vote, cur_round); - assert_eq!(msg, Some(Output::SkipRound(Round::new(1)))); + assert!(matches!(msg, Ok(Some(Output::SkipRound(r))) if r == Round::new(1))); } #[test] @@ -192,15 +190,15 @@ fn skip_round_full_quorum_with_prevote_precommit_two_vals() { let vote = new_signed_prevote(height, cur_round, val, addr1); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, fut_round, val, addr3); let msg = keeper.apply_vote(vote, cur_round); - assert_eq!(msg, Some(Output::SkipRound(Round::new(1)))); + assert!(matches!(msg, Ok(Some(Output::SkipRound(r))) if r == Round::new(1))); } #[test] @@ -215,15 +213,15 @@ fn no_skip_round_small_quorum_with_same_val() { let vote = new_signed_prevote(height, cur_round, val, addr1); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote, cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); } #[test] @@ -238,15 +236,15 @@ fn no_skip_round_full_quorum_with_same_val() { let vote = new_signed_prevote(height, cur_round, val, addr1); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_prevote(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote.clone(), cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote = new_signed_precommit(height, fut_round, val, addr2); let msg = keeper.apply_vote(vote, cur_round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); } #[test] @@ -261,11 +259,11 @@ fn same_votes() { let vote1 = new_signed_prevote(height, round, val, addr1); let msg = keeper.apply_vote(vote1.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote2 = new_signed_prevote(height, round, val, addr1); let msg = keeper.apply_vote(vote2.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); assert!(keeper.evidence().is_empty()); assert_eq!(keeper.evidence().get(&addr1), None); @@ -283,25 +281,36 @@ fn equivocation() { let vote11 = new_signed_prevote(height, round, val1, addr1); let msg = keeper.apply_vote(vote11.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let vote12 = new_signed_prevote(height, round, NilOrVal::Nil, addr1); let msg = keeper.apply_vote(vote12.clone(), round); - assert_eq!(msg, None); + assert!(matches!( + msg, + Err(RecordVoteError::ConflictingVote { + existing, + conflicting + }) if existing == vote11 && conflicting == vote12 + )); assert!(!keeper.evidence().is_empty()); assert_eq!(keeper.evidence().get(&addr1), Some(&vec![(vote11, vote12)])); let vote21 = new_signed_prevote(height, round, val1, addr2); let msg = keeper.apply_vote(vote21.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, Ok(None))); let id2 = ValueId::new(2); let val2 = NilOrVal::Val(id2); let vote22 = new_signed_prevote(height, round, val2, addr2); let msg = keeper.apply_vote(vote22.clone(), round); - assert_eq!(msg, None); + assert!(matches!(msg, + Err(RecordVoteError::ConflictingVote { + existing, + conflicting + }) if existing == vote21 && conflicting == vote22 + )); assert_eq!(keeper.evidence().get(&addr2), Some(&vec![(vote21, vote22)])); } diff --git a/code/crates/engine/src/consensus.rs b/code/crates/engine/src/consensus.rs index 63e9baede..e15fd6d67 100644 --- a/code/crates/engine/src/consensus.rs +++ b/code/crates/engine/src/consensus.rs @@ -525,6 +525,18 @@ where { error!(%from, "Error when processing vote: {e}"); } + + if let Some((vote_height, validator_address, evidence)) = + state.consensus.driver.vote_equivocation_evidence() + { + self.tx_event.send(|| Event::VoteEquivocationEvidence { + vote_height, + address: validator_address, + evidence, + }); + + state.consensus.driver.clear_vote_equivocation_evidence(); + } } NetworkEvent::Proposal(from, proposal) => { @@ -539,6 +551,21 @@ where { error!(%from, "Error when processing proposal: {e}"); } + + if let Some((proposal_height, validator_address, evidence)) = + state.consensus.driver.proposal_equivocation_evidence() + { + self.tx_event.send(|| Event::ProposalEquivocationEvidence { + proposal_height, + address: validator_address, + evidence, + }); + + state + .consensus + .driver + .clear_proposal_equivocation_evidence(); + } } NetworkEvent::ProposalPart(from, part) => { diff --git a/code/crates/engine/src/util/events.rs b/code/crates/engine/src/util/events.rs index 986e79235..585fbc8ed 100644 --- a/code/crates/engine/src/util/events.rs +++ b/code/crates/engine/src/util/events.rs @@ -8,7 +8,9 @@ use tokio::sync::broadcast; use malachitebft_core_consensus::{ LocallyProposedValue, ProposedValue, SignedConsensusMsg, WalEntry, }; -use malachitebft_core_types::{CommitCertificate, Context, Round, SignedVote, ValueOrigin}; +use malachitebft_core_types::{ + CommitCertificate, Context, Round, SignedMessage, SignedVote, ValueOrigin, +}; pub type RxEvent = broadcast::Receiver>; @@ -59,6 +61,22 @@ pub enum Event { WalReplayEntry(WalEntry), WalReplayDone(Ctx::Height), WalReplayError(Arc), + ProposalEquivocationEvidence { + proposal_height: Ctx::Height, + address: Ctx::Address, + evidence: ( + SignedMessage::Proposal>, + SignedMessage::Proposal>, + ), + }, + VoteEquivocationEvidence { + vote_height: Ctx::Height, + address: Ctx::Address, + evidence: ( + SignedMessage::Vote>, + SignedMessage::Vote>, + ), + }, } impl fmt::Display for Event { @@ -99,6 +117,20 @@ impl fmt::Display for Event { Event::WalReplayEntry(entry) => write!(f, "WalReplayEntry(entry: {entry:?})"), Event::WalReplayDone(height) => write!(f, "WalReplayDone(height: {height})"), Event::WalReplayError(error) => write!(f, "WalReplayError({error})"), + Event::ProposalEquivocationEvidence { + proposal_height, + address, + evidence, + } => { + write!(f, "ProposalEquivocationEvidence(height: {proposal_height}, address: {address}, evidence: {evidence:?})") + } + Event::VoteEquivocationEvidence { + vote_height, + address, + evidence, + } => { + write!(f, "VoteEquivocationEvidence(height: {vote_height}, address: {address}, evidence: {evidence:?})") + } } } } diff --git a/code/crates/test/mbt/src/tests/votekeeper/runner.rs b/code/crates/test/mbt/src/tests/votekeeper/runner.rs index 22d5e74c8..57e64eba1 100644 --- a/code/crates/test/mbt/src/tests/votekeeper/runner.rs +++ b/code/crates/test/mbt/src/tests/votekeeper/runner.rs @@ -96,10 +96,15 @@ impl ItfRunner for VoteKeeperRunner { debug_assert_eq!(*weight as u64, validator.voting_power); // Execute step. - Ok(actual.apply_vote( - SignedVote::new(vote, Signature::test()), - Round::from(*current_round), - )) + actual + .apply_vote( + SignedVote::new(vote, Signature::test()), + Round::from(*current_round), + ) + .map_err(|err| { + println!("Error applying vote: {:?}", err); + (); + }) } } } From a22afc15529dfbfc4de5ee6a77c22b2b814c801c Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Sun, 11 May 2025 20:24:04 +0200 Subject: [PATCH 03/10] feat: Equivocation proposal and vote middlewares --- code/crates/test/tests/it/equivocation.rs | 73 +++++++++++++++++++++++ code/crates/test/tests/it/main.rs | 1 + 2 files changed, 74 insertions(+) create mode 100644 code/crates/test/tests/it/equivocation.rs diff --git a/code/crates/test/tests/it/equivocation.rs b/code/crates/test/tests/it/equivocation.rs new file mode 100644 index 000000000..e5e905283 --- /dev/null +++ b/code/crates/test/tests/it/equivocation.rs @@ -0,0 +1,73 @@ +use informalsystems_malachitebft_test::{ + middleware::Middleware, Address, Height, TestContext, Value, ValueId, Vote, +}; +use malachitebft_core_consensus::LocallyProposedValue; +use malachitebft_core_types::{NilOrVal, Round}; +use rand::Rng; + +#[derive(Copy, Clone, Debug)] +struct EquivocationProposer; + +impl Middleware for EquivocationProposer { + fn on_propose_value( + &self, + _ctx: &TestContext, + proposal: &mut LocallyProposedValue, + reproposal: bool, + ) { + if !reproposal { + tracing::warn!( + "EquivocationProposer: First time proposing value {:}", + proposal.value.id() + ); + + // Keep the proposal value as is + return; + } + + // Change the proposal value to a different one + let new_value = loop { + let new_value = Value::new(rand::thread_rng().gen_range(100..=100000)); + if new_value != proposal.value { + break new_value; + } + }; + + tracing::warn!( + "EquivocationProposer: Reproposing value {:} instead of {:}", + new_value.id(), + proposal.value.id() + ); + + proposal.value = new_value; + } +} + +#[derive(Clone, Debug)] +struct EquivocationVoter; + +impl Middleware for EquivocationVoter { + fn new_prevote( + &self, + _ctx: &TestContext, + height: Height, + round: Round, + value_id: NilOrVal, + address: Address, + ) -> Vote { + if round.as_i64() % 2 == 0 { + // Vote for the given value + Vote::new_prevote(height, round, value_id, address) + } else { + // Vote for a different value + let new_value = loop { + let new_value = ValueId::new(rand::thread_rng().gen_range(100..=100000)); + if NilOrVal::Val(new_value) != value_id { + break new_value; + } + }; + + Vote::new_prevote(height, round, NilOrVal::Val(new_value), address) + } + } +} diff --git a/code/crates/test/tests/it/main.rs b/code/crates/test/tests/it/main.rs index 7f68d0813..1ab2cbbef 100644 --- a/code/crates/test/tests/it/main.rs +++ b/code/crates/test/tests/it/main.rs @@ -1,3 +1,4 @@ +mod equivocation; mod full_nodes; mod n3f0; mod n3f0_consensus_mode; From 04374df0dcba78a37df1312256be70c4f3aabf27 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Mon, 12 May 2025 11:02:57 +0200 Subject: [PATCH 04/10] refactor(code): Simplify implementation by only retrieving evidence on decide (#1009) * refactor(code): Simplify implementation by only checking for evidence on decide * feat: equivocation logs * fix: missing update * fix: debug -> warn * fix: show full proposal and vote * fix: temporary middlewares warnings --------- Co-authored-by: Bastien Faivre --- code/Cargo.lock | 2 + code/crates/core-consensus/src/effect.rs | 2 - .../core-consensus/src/handle/decide.rs | 14 ++-- code/crates/core-consensus/src/types.rs | 12 ++++ code/crates/core-driver/Cargo.toml | 1 + code/crates/core-driver/src/driver.rs | 70 +++---------------- code/crates/core-driver/src/mux.rs | 15 +--- .../crates/core-driver/src/proposal_keeper.rs | 6 ++ code/crates/core-votekeeper/Cargo.toml | 1 + code/crates/core-votekeeper/src/keeper.rs | 7 ++ code/crates/engine/src/consensus.rs | 41 +---------- code/crates/engine/src/util/events.rs | 48 ++----------- code/crates/test/framework/src/node.rs | 18 ++++- .../test/mbt/src/tests/votekeeper/runner.rs | 1 - code/crates/test/tests/it/equivocation.rs | 8 +-- 15 files changed, 74 insertions(+), 172 deletions(-) diff --git a/code/Cargo.lock b/code/Cargo.lock index ef4b8d9b1..d3fa12546 100644 --- a/code/Cargo.lock +++ b/code/Cargo.lock @@ -2196,6 +2196,7 @@ dependencies = [ "informalsystems-malachitebft-core-votekeeper", "informalsystems-malachitebft-test", "thiserror 2.0.12", + "tracing", ] [[package]] @@ -2225,6 +2226,7 @@ dependencies = [ "informalsystems-malachitebft-core-types", "informalsystems-malachitebft-test", "thiserror 2.0.12", + "tracing", ] [[package]] diff --git a/code/crates/core-consensus/src/effect.rs b/code/crates/core-consensus/src/effect.rs index 70ea65dc3..b0c2425c6 100644 --- a/code/crates/core-consensus/src/effect.rs +++ b/code/crates/core-consensus/src/effect.rs @@ -133,8 +133,6 @@ where Decide( CommitCertificate, VoteExtensions, - malachitebft_core_driver::EvidenceMap, - malachitebft_core_votekeeper::EvidenceMap, resume::Continue, ), diff --git a/code/crates/core-consensus/src/handle/decide.rs b/code/crates/core-consensus/src/handle/decide.rs index 006981a2e..d593debfe 100644 --- a/code/crates/core-consensus/src/handle/decide.rs +++ b/code/crates/core-consensus/src/handle/decide.rs @@ -1,3 +1,4 @@ +use crate::MisbehaviorEvidence; use crate::{handle::signature::verify_commit_certificate, prelude::*}; #[cfg_attr(not(feature = "metrics"), allow(unused_variables))] @@ -100,15 +101,14 @@ where } } + let _evidence = MisbehaviorEvidence { + proposals: state.driver.proposal_evidence().clone(), + votes: state.driver.vote_evidence().clone(), + }; + perform!( co, - Effect::Decide( - certificate, - extensions, - state.driver.evidence().clone(), - state.driver.votes().evidence().clone(), - Default::default() - ) + Effect::Decide(certificate, extensions, Default::default()) ); Ok(()) diff --git a/code/crates/core-consensus/src/types.rs b/code/crates/core-consensus/src/types.rs index 56184592f..afedc1a81 100644 --- a/code/crates/core-consensus/src/types.rs +++ b/code/crates/core-consensus/src/types.rs @@ -114,3 +114,15 @@ pub enum VoteExtensionError { #[error("Invalid vote extension")] InvalidVoteExtension, } + +#[derive_where(Clone, Debug)] +pub struct MisbehaviorEvidence { + pub proposals: malachitebft_core_driver::EvidenceMap, + pub votes: malachitebft_core_votekeeper::EvidenceMap, +} + +impl MisbehaviorEvidence { + pub fn is_empty(&self) -> bool { + self.proposals.is_empty() && self.votes.is_empty() + } +} diff --git a/code/crates/core-driver/Cargo.toml b/code/crates/core-driver/Cargo.toml index 0bcf7a38d..d54087b06 100644 --- a/code/crates/core-driver/Cargo.toml +++ b/code/crates/core-driver/Cargo.toml @@ -26,6 +26,7 @@ malachitebft-core-votekeeper = { workspace = true } derive-where = { workspace = true } thiserror = { workspace = true, default-features = false } +tracing = { workspace = true } [dev-dependencies] malachitebft-test = { workspace = true } diff --git a/code/crates/core-driver/src/driver.rs b/code/crates/core-driver/src/driver.rs index 21f16b222..40654c768 100644 --- a/code/crates/core-driver/src/driver.rs +++ b/code/crates/core-driver/src/driver.rs @@ -11,12 +11,12 @@ use malachitebft_core_types::{ SignedProposal, SignedVote, Timeout, TimeoutKind, Validator, ValidatorSet, Validity, Value, ValueId, Vote, VoteType, }; +use malachitebft_core_votekeeper::keeper::Output as VKOutput; use malachitebft_core_votekeeper::keeper::VoteKeeper; -use malachitebft_core_votekeeper::keeper::{Output as VKOutput, RecordVoteError}; use crate::input::Input; use crate::output::Output; -use crate::proposal_keeper::{EvidenceMap, ProposalKeeper}; +use crate::proposal_keeper::{self, ProposalKeeper}; use crate::Error; use crate::ThresholdParams; @@ -45,23 +45,9 @@ where /// The proposals to decide on. pub(crate) proposal_keeper: ProposalKeeper, - /// The evidence of equivocation (if any) after processing a proposal. - pub(crate) proposal_equivocation_evidence: Option<( - Ctx::Height, - Ctx::Address, - (SignedProposal, SignedProposal), - )>, - /// The vote keeper. pub(crate) vote_keeper: VoteKeeper, - /// The evidence of equivocation (if any) after processing a vote. - pub(crate) vote_equivocation_evidence: Option<( - Ctx::Height, - Ctx::Address, - (SignedVote, SignedVote), - )>, - /// The commit certificates pub(crate) commit_certificates: Vec>, @@ -104,9 +90,7 @@ where threshold_params, validator_set, proposal_keeper, - proposal_equivocation_evidence: None, vote_keeper, - vote_equivocation_evidence: None, round_state, proposer: None, pending_inputs: vec![], @@ -208,41 +192,14 @@ where &self.validator_set } - /// Return recorded evidence of equivocation for this height. - pub fn evidence(&self) -> &EvidenceMap { + /// Return recorded evidence of proposal equivocation for this height. + pub fn proposal_evidence(&self) -> &proposal_keeper::EvidenceMap { self.proposal_keeper.evidence() } - /// Return proposal equivocation evidence, if any. - pub fn proposal_equivocation_evidence( - &self, - ) -> Option<( - Ctx::Height, - Ctx::Address, - (SignedProposal, SignedProposal), - )> { - self.proposal_equivocation_evidence.clone() - } - - /// Clear the proposal equivocation evidence. - pub fn clear_proposal_equivocation_evidence(&mut self) { - self.proposal_equivocation_evidence = None; - } - - /// Return vote equivocation evidence, if any. - pub fn vote_equivocation_evidence( - &self, - ) -> Option<( - Ctx::Height, - Ctx::Address, - (SignedVote, SignedVote), - )> { - self.vote_equivocation_evidence.clone() - } - - /// Clear the vote equivocation evidence. - pub fn clear_vote_equivocation_evidence(&mut self) { - self.vote_equivocation_evidence = None; + /// Return recorded evidence of vote equivocation for this height. + pub fn vote_evidence(&self) -> &malachitebft_core_votekeeper::EvidenceMap { + self.votes().evidence() } /// Return the proposer for the current round. @@ -497,21 +454,10 @@ where let vote_round = vote.round(); let this_round = self.round(); - let validator_address = vote.validator_address().clone(); - let height = vote.height(); - let output = match self.vote_keeper.apply_vote(vote, this_round) { Ok(Some(output)) => output, Ok(None) => return Ok(None), - Err(RecordVoteError::ConflictingVote { - existing, - conflicting, - }) => { - self.vote_equivocation_evidence = - Some((height, validator_address, (existing, conflicting))); - - return Ok(None); - } + Err(_) => return Ok(None), }; if let VKOutput::PolkaValue(val) = &output { diff --git a/code/crates/core-driver/src/mux.rs b/code/crates/core-driver/src/mux.rs index 268d563b1..5a5c4fc9c 100644 --- a/code/crates/core-driver/src/mux.rs +++ b/code/crates/core-driver/src/mux.rs @@ -37,7 +37,6 @@ use malachitebft_core_votekeeper::keeper::Output as VKOutput; use malachitebft_core_votekeeper::keeper::VoteKeeper; use malachitebft_core_votekeeper::Threshold; -use crate::proposal_keeper::RecordProposalError; use crate::Driver; impl Driver @@ -200,19 +199,9 @@ where let proposal = signed_proposal.message.clone(); // Store the proposal and its validity - if let Err(RecordProposalError::ConflictingProposal { - existing, - conflicting, - }) = self + let _ = self .proposal_keeper - .store_proposal(signed_proposal, validity) - { - let validator_address = existing.validator_address().clone(); - let height = existing.height(); - - self.proposal_equivocation_evidence = - Some((height, validator_address, (existing, conflicting))); - } + .store_proposal(signed_proposal, validity); self.multiplex_proposal(proposal, validity) } diff --git a/code/crates/core-driver/src/proposal_keeper.rs b/code/crates/core-driver/src/proposal_keeper.rs index 1ddf27029..e1ea776ab 100644 --- a/code/crates/core-driver/src/proposal_keeper.rs +++ b/code/crates/core-driver/src/proposal_keeper.rs @@ -8,6 +8,7 @@ use derive_where::derive_where; use thiserror::Error; use malachitebft_core_types::{Context, Proposal, Round, SignedProposal, Validity}; +use tracing::warn; /// Errors can that be yielded when recording a proposal. #[derive_where(Debug)] @@ -143,6 +144,11 @@ where // This is an equivocating proposal self.evidence.add(existing.clone(), conflicting.clone()); + warn!( + "Conflicting proposal: existing: {:?}, conflicting: {:?}", + existing, conflicting + ); + Err(RecordProposalError::ConflictingProposal { existing, conflicting, diff --git a/code/crates/core-votekeeper/Cargo.toml b/code/crates/core-votekeeper/Cargo.toml index eb631268a..898254145 100644 --- a/code/crates/core-votekeeper/Cargo.toml +++ b/code/crates/core-votekeeper/Cargo.toml @@ -17,6 +17,7 @@ malachitebft-core-types = { workspace = true } derive-where = { workspace = true } thiserror = { workspace = true, default-features = false } +tracing = { workspace = true } [dev-dependencies] malachitebft-test = { workspace = true } diff --git a/code/crates/core-votekeeper/src/keeper.rs b/code/crates/core-votekeeper/src/keeper.rs index 1caf981bf..7187beaed 100644 --- a/code/crates/core-votekeeper/src/keeper.rs +++ b/code/crates/core-votekeeper/src/keeper.rs @@ -8,6 +8,7 @@ use alloc::collections::{BTreeMap, BTreeSet}; use malachitebft_core_types::{ Context, NilOrVal, Round, SignedVote, Validator, ValidatorSet, ValueId, Vote, VoteType, }; +use tracing::warn; use crate::evidence::EvidenceMap; use crate::round_votes::RoundVotes; @@ -234,6 +235,12 @@ where }) => { // This is an equivocating vote self.evidence.add(existing.clone(), conflicting); + + warn!( + "Conflicting vote: existing: {:?}, conflicting: {:?}", + existing, vote + ); + return Err(RecordVoteError::ConflictingVote { existing, conflicting: vote, diff --git a/code/crates/engine/src/consensus.rs b/code/crates/engine/src/consensus.rs index ce2893b9e..ab3c2ce33 100644 --- a/code/crates/engine/src/consensus.rs +++ b/code/crates/engine/src/consensus.rs @@ -564,18 +564,6 @@ where { error!(%from, "Error when processing vote: {e}"); } - - if let Some((vote_height, validator_address, evidence)) = - state.consensus.driver.vote_equivocation_evidence() - { - self.tx_event.send(|| Event::VoteEquivocationEvidence { - vote_height, - address: validator_address, - evidence, - }); - - state.consensus.driver.clear_vote_equivocation_evidence(); - } } NetworkEvent::Proposal(from, proposal) => { @@ -590,21 +578,6 @@ where { error!(%from, "Error when processing proposal: {e}"); } - - if let Some((proposal_height, validator_address, evidence)) = - state.consensus.driver.proposal_equivocation_evidence() - { - self.tx_event.send(|| Event::ProposalEquivocationEvidence { - proposal_height, - address: validator_address, - evidence, - }); - - state - .consensus - .driver - .clear_proposal_equivocation_evidence(); - } } NetworkEvent::ProposalPart(from, part) => { @@ -1151,22 +1124,12 @@ where Ok(r.resume_with(())) } - Effect::Decide( - certificate, - extensions, - proposal_equivocation_evidence_map, - vote_equivocation_evidence_map, - r, - ) => { + Effect::Decide(certificate, extensions, r) => { assert!(!certificate.commit_signatures.is_empty()); self.wal_flush(state.phase).await?; - self.tx_event.send(|| Event::Decided { - commit_certificate: certificate.clone(), - proposal_equivocation_evidence_map, - vote_equivocation_evidence_map, - }); + self.tx_event.send(|| Event::Decided(certificate.clone())); let height = certificate.height; diff --git a/code/crates/engine/src/util/events.rs b/code/crates/engine/src/util/events.rs index 585fbc8ed..ee11da3a0 100644 --- a/code/crates/engine/src/util/events.rs +++ b/code/crates/engine/src/util/events.rs @@ -8,9 +8,7 @@ use tokio::sync::broadcast; use malachitebft_core_consensus::{ LocallyProposedValue, ProposedValue, SignedConsensusMsg, WalEntry, }; -use malachitebft_core_types::{ - CommitCertificate, Context, Round, SignedMessage, SignedVote, ValueOrigin, -}; +use malachitebft_core_types::{CommitCertificate, Context, Round, SignedVote, ValueOrigin}; pub type RxEvent = broadcast::Receiver>; @@ -49,11 +47,7 @@ pub enum Event { Published(SignedConsensusMsg), ProposedValue(LocallyProposedValue), ReceivedProposedValue(ProposedValue, ValueOrigin), - Decided { - commit_certificate: CommitCertificate, - proposal_equivocation_evidence_map: malachitebft_core_driver::EvidenceMap, - vote_equivocation_evidence_map: malachitebft_core_votekeeper::EvidenceMap, - }, + Decided(CommitCertificate), Rebroadcast(SignedVote), RequestedVoteSet(Ctx::Height, Round), SentVoteSetResponse(Ctx::Height, Round, usize, usize), @@ -61,22 +55,6 @@ pub enum Event { WalReplayEntry(WalEntry), WalReplayDone(Ctx::Height), WalReplayError(Arc), - ProposalEquivocationEvidence { - proposal_height: Ctx::Height, - address: Ctx::Address, - evidence: ( - SignedMessage::Proposal>, - SignedMessage::Proposal>, - ), - }, - VoteEquivocationEvidence { - vote_height: Ctx::Height, - address: Ctx::Address, - evidence: ( - SignedMessage::Vote>, - SignedMessage::Vote>, - ), - }, } impl fmt::Display for Event { @@ -96,11 +74,9 @@ impl fmt::Display for Event { "ReceivedProposedValue(value: {value:?}, origin: {origin:?})" ) } - Event::Decided { - commit_certificate, - proposal_equivocation_evidence_map: _, - vote_equivocation_evidence_map: _, - } => write!(f, "Decided(value: {})", commit_certificate.value_id), + Event::Decided(commit_certificate) => { + write!(f, "Decided(value: {})", commit_certificate.value_id,) + } Event::Rebroadcast(msg) => write!(f, "Rebroadcast(msg: {msg:?})"), Event::RequestedVoteSet(height, round) => { write!(f, "RequestedVoteSet(height: {height}, round: {round})") @@ -117,20 +93,6 @@ impl fmt::Display for Event { Event::WalReplayEntry(entry) => write!(f, "WalReplayEntry(entry: {entry:?})"), Event::WalReplayDone(height) => write!(f, "WalReplayDone(height: {height})"), Event::WalReplayError(error) => write!(f, "WalReplayError({error})"), - Event::ProposalEquivocationEvidence { - proposal_height, - address, - evidence, - } => { - write!(f, "ProposalEquivocationEvidence(height: {proposal_height}, address: {address}, evidence: {evidence:?})") - } - Event::VoteEquivocationEvidence { - vote_height, - address, - evidence, - } => { - write!(f, "VoteEquivocationEvidence(height: {vote_height}, address: {address}, evidence: {evidence:?})") - } } } } diff --git a/code/crates/test/framework/src/node.rs b/code/crates/test/framework/src/node.rs index 047c316c4..7f686cf00 100644 --- a/code/crates/test/framework/src/node.rs +++ b/code/crates/test/framework/src/node.rs @@ -5,7 +5,7 @@ use eyre::bail; use tracing::info; use malachitebft_core_consensus::{LocallyProposedValue, SignedConsensusMsg}; -use malachitebft_core_types::{Context, Height, SignedVote, Vote, VotingPower}; +use malachitebft_core_types::{CommitCertificate, Context, Height, SignedVote, Vote, VotingPower}; use malachitebft_engine::util::events::Event; use malachitebft_test::middleware::{DefaultMiddleware, Middleware}; @@ -226,6 +226,22 @@ where }) } + pub fn on_decided(&mut self, f: F) -> &mut Self + where + F: Fn(CommitCertificate, &mut State) -> Result + + Send + + Sync + + 'static, + { + self.on_event(move |event, state| { + if let Event::Decided(commit_certificate) = event { + f(commit_certificate, state) + } else { + Ok(HandlerResult::WaitForNextEvent) + } + }) + } + pub fn expect_decisions(&mut self, expected: Expected) -> &mut Self { self.steps.push(Step::Expect(expected)); self diff --git a/code/crates/test/mbt/src/tests/votekeeper/runner.rs b/code/crates/test/mbt/src/tests/votekeeper/runner.rs index 57e64eba1..3ad993b14 100644 --- a/code/crates/test/mbt/src/tests/votekeeper/runner.rs +++ b/code/crates/test/mbt/src/tests/votekeeper/runner.rs @@ -103,7 +103,6 @@ impl ItfRunner for VoteKeeperRunner { ) .map_err(|err| { println!("Error applying vote: {:?}", err); - (); }) } } diff --git a/code/crates/test/tests/it/equivocation.rs b/code/crates/test/tests/it/equivocation.rs index e5e905283..a972144f3 100644 --- a/code/crates/test/tests/it/equivocation.rs +++ b/code/crates/test/tests/it/equivocation.rs @@ -6,9 +6,9 @@ use malachitebft_core_types::{NilOrVal, Round}; use rand::Rng; #[derive(Copy, Clone, Debug)] -struct EquivocationProposer; +struct _EquivocationProposer; -impl Middleware for EquivocationProposer { +impl Middleware for _EquivocationProposer { fn on_propose_value( &self, _ctx: &TestContext, @@ -44,9 +44,9 @@ impl Middleware for EquivocationProposer { } #[derive(Clone, Debug)] -struct EquivocationVoter; +struct _EquivocationVoter; -impl Middleware for EquivocationVoter { +impl Middleware for _EquivocationVoter { fn new_prevote( &self, _ctx: &TestContext, From f5211ed41799a3c452531128aff73718f93eab84 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Mon, 12 May 2025 11:48:08 +0200 Subject: [PATCH 05/10] feat: last_evidence approach --- .../crates/core-driver/src/proposal_keeper.rs | 27 ++++++++++- code/crates/core-votekeeper/src/evidence.rs | 46 ++++++++++++++++--- code/crates/engine/src/consensus.rs | 34 +++++++++++++- code/crates/engine/src/util/events.rs | 34 +++++++++++++- 4 files changed, 129 insertions(+), 12 deletions(-) diff --git a/code/crates/core-driver/src/proposal_keeper.rs b/code/crates/core-driver/src/proposal_keeper.rs index e1ea776ab..79d495451 100644 --- a/code/crates/core-driver/src/proposal_keeper.rs +++ b/code/crates/core-driver/src/proposal_keeper.rs @@ -178,6 +178,7 @@ where { #[allow(clippy::type_complexity)] map: BTreeMap, SignedProposal)>>, + last: Option<(Ctx::Address, (SignedProposal, SignedProposal))>, } impl EvidenceMap @@ -202,6 +203,20 @@ where self.map.get(address) } + /// Check if the given proposal is the last equivocation recorded. If it is, return the + /// address of the validator and the evidence. + pub fn is_last_equivocation( + &self, + proposal: &SignedProposal, + ) -> Option<(Ctx::Address, (SignedProposal, SignedProposal))> { + self.last + .as_ref() + .filter(|(address, (_, conflicting))| { + address == proposal.validator_address() && conflicting == proposal + }) + .cloned() + } + /// Add evidence of equivocating proposals, ie. two proposals submitted by the same validator, /// but with different values but for the same height and round. /// @@ -214,12 +229,20 @@ where ); if let Some(evidence) = self.map.get_mut(conflicting.validator_address()) { - evidence.push((existing, conflicting)); + evidence.push((existing.clone(), conflicting.clone())); + self.last = Some(( + conflicting.validator_address().clone(), + (existing, conflicting), + )); } else { self.map.insert( conflicting.validator_address().clone(), - vec![(existing, conflicting)], + vec![(existing.clone(), conflicting.clone())], ); + self.last = Some(( + conflicting.validator_address().clone(), + (existing, conflicting), + )); } } } diff --git a/code/crates/core-votekeeper/src/evidence.rs b/code/crates/core-votekeeper/src/evidence.rs index 0789e2c21..1340c49e7 100644 --- a/code/crates/core-votekeeper/src/evidence.rs +++ b/code/crates/core-votekeeper/src/evidence.rs @@ -15,6 +15,7 @@ where { #[allow(clippy::type_complexity)] map: BTreeMap, SignedVote)>>, + last: Option<(Ctx::Address, (SignedVote, SignedVote))>, } impl EvidenceMap @@ -36,15 +37,46 @@ where self.map.get(address) } - /// Add evidence of equivocation. - pub fn add(&mut self, existing: SignedVote, vote: SignedVote) { - debug_assert_eq!(existing.validator_address(), vote.validator_address()); + /// Check if the given vote is the last equivocation recorded. If it is, return the + /// address of the validator and the evidence. + pub fn is_last_equivocation( + &self, + vote: &SignedVote, + ) -> Option<(Ctx::Address, (SignedVote, SignedVote))> { + self.last + .as_ref() + .filter(|(address, (_, conflicting))| { + address == vote.validator_address() && conflicting == vote + }) + .cloned() + } + + /// Add evidence of equivocating votes, ie. two votes submitted by the same validator, + /// but with different values but for the same height and round. + /// + /// # Precondition + /// - Panics if the two conflicting votes were not proposed by the same validator. + pub fn add(&mut self, existing: SignedVote, conflicting: SignedVote) { + debug_assert_eq!( + existing.validator_address(), + conflicting.validator_address() + ); - if let Some(evidence) = self.map.get_mut(vote.validator_address()) { - evidence.push((existing, vote)); + if let Some(evidence) = self.map.get_mut(conflicting.validator_address()) { + evidence.push((existing.clone(), conflicting.clone())); + self.last = Some(( + conflicting.validator_address().clone(), + (existing, conflicting), + )); } else { - self.map - .insert(vote.validator_address().clone(), vec![(existing, vote)]); + self.map.insert( + conflicting.validator_address().clone(), + vec![(existing.clone(), conflicting.clone())], + ); + self.last = Some(( + conflicting.validator_address().clone(), + (existing, conflicting), + )); } } } diff --git a/code/crates/engine/src/consensus.rs b/code/crates/engine/src/consensus.rs index ab3c2ce33..4f300b368 100644 --- a/code/crates/engine/src/consensus.rs +++ b/code/crates/engine/src/consensus.rs @@ -559,11 +559,24 @@ where NetworkEvent::Vote(from, vote) => { if let Err(e) = self - .process_input(&myself, state, ConsensusInput::Vote(vote)) + .process_input(&myself, state, ConsensusInput::Vote(vote.clone())) .await { error!(%from, "Error when processing vote: {e}"); } + + if let Some((address, evidence)) = state + .consensus + .driver + .vote_evidence() + .is_last_equivocation(&vote) + { + self.tx_event.send(|| Event::VoteEquivocationEvidence { + vote_height: vote.height(), + address, + evidence, + }); + } } NetworkEvent::Proposal(from, proposal) => { @@ -573,11 +586,28 @@ where } if let Err(e) = self - .process_input(&myself, state, ConsensusInput::Proposal(proposal)) + .process_input( + &myself, + state, + ConsensusInput::Proposal(proposal.clone()), + ) .await { error!(%from, "Error when processing proposal: {e}"); } + + if let Some((address, evidence)) = state + .consensus + .driver + .proposal_evidence() + .is_last_equivocation(&proposal) + { + self.tx_event.send(|| Event::ProposalEquivocationEvidence { + proposal_height: proposal.height(), + address, + evidence, + }); + } } NetworkEvent::ProposalPart(from, part) => { diff --git a/code/crates/engine/src/util/events.rs b/code/crates/engine/src/util/events.rs index ee11da3a0..0f1d69f05 100644 --- a/code/crates/engine/src/util/events.rs +++ b/code/crates/engine/src/util/events.rs @@ -8,7 +8,9 @@ use tokio::sync::broadcast; use malachitebft_core_consensus::{ LocallyProposedValue, ProposedValue, SignedConsensusMsg, WalEntry, }; -use malachitebft_core_types::{CommitCertificate, Context, Round, SignedVote, ValueOrigin}; +use malachitebft_core_types::{ + CommitCertificate, Context, Round, SignedMessage, SignedVote, ValueOrigin, +}; pub type RxEvent = broadcast::Receiver>; @@ -55,6 +57,22 @@ pub enum Event { WalReplayEntry(WalEntry), WalReplayDone(Ctx::Height), WalReplayError(Arc), + ProposalEquivocationEvidence { + proposal_height: Ctx::Height, + address: Ctx::Address, + evidence: ( + SignedMessage::Proposal>, + SignedMessage::Proposal>, + ), + }, + VoteEquivocationEvidence { + vote_height: Ctx::Height, + address: Ctx::Address, + evidence: ( + SignedMessage::Vote>, + SignedMessage::Vote>, + ), + }, } impl fmt::Display for Event { @@ -93,6 +111,20 @@ impl fmt::Display for Event { Event::WalReplayEntry(entry) => write!(f, "WalReplayEntry(entry: {entry:?})"), Event::WalReplayDone(height) => write!(f, "WalReplayDone(height: {height})"), Event::WalReplayError(error) => write!(f, "WalReplayError({error})"), + Event::ProposalEquivocationEvidence { + proposal_height, + address, + evidence, + } => { + write!(f, "ProposalEquivocationEvidence(height: {proposal_height}, address: {address}, evidence: {evidence:?})") + } + Event::VoteEquivocationEvidence { + vote_height, + address, + evidence, + } => { + write!(f, "VoteEquivocationEvidence(height: {vote_height}, address: {address}, evidence: {evidence:?})") + } } } } From e0fd9bf5db7cf3a1d6030e2f79afebc4cd269495 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Mon, 12 May 2025 11:56:03 +0200 Subject: [PATCH 06/10] fix: type simplification --- code/crates/engine/src/util/events.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/code/crates/engine/src/util/events.rs b/code/crates/engine/src/util/events.rs index 0f1d69f05..214abfbb4 100644 --- a/code/crates/engine/src/util/events.rs +++ b/code/crates/engine/src/util/events.rs @@ -9,7 +9,7 @@ use malachitebft_core_consensus::{ LocallyProposedValue, ProposedValue, SignedConsensusMsg, WalEntry, }; use malachitebft_core_types::{ - CommitCertificate, Context, Round, SignedMessage, SignedVote, ValueOrigin, + CommitCertificate, Context, Round, SignedProposal, SignedVote, ValueOrigin, }; pub type RxEvent = broadcast::Receiver>; @@ -60,18 +60,12 @@ pub enum Event { ProposalEquivocationEvidence { proposal_height: Ctx::Height, address: Ctx::Address, - evidence: ( - SignedMessage::Proposal>, - SignedMessage::Proposal>, - ), + evidence: (SignedProposal, SignedProposal), }, VoteEquivocationEvidence { vote_height: Ctx::Height, address: Ctx::Address, - evidence: ( - SignedMessage::Vote>, - SignedMessage::Vote>, - ), + evidence: (SignedVote, SignedVote), }, } From 3d1d97ffd27924278b54eb4c244abadabe2fc36c Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Mon, 12 May 2025 12:37:35 +0200 Subject: [PATCH 07/10] feat: vote and proposal equivocation metrics --- code/crates/engine/src/consensus.rs | 4 ++++ code/crates/metrics/src/metrics.rs | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/code/crates/engine/src/consensus.rs b/code/crates/engine/src/consensus.rs index 4f300b368..2d0ea0fdd 100644 --- a/code/crates/engine/src/consensus.rs +++ b/code/crates/engine/src/consensus.rs @@ -571,6 +571,8 @@ where .vote_evidence() .is_last_equivocation(&vote) { + self.metrics.equivocation_votes.inc(); + self.tx_event.send(|| Event::VoteEquivocationEvidence { vote_height: vote.height(), address, @@ -602,6 +604,8 @@ where .proposal_evidence() .is_last_equivocation(&proposal) { + self.metrics.equivocation_proposals.inc(); + self.tx_event.send(|| Event::ProposalEquivocationEvidence { proposal_height: proposal.height(), address, diff --git a/code/crates/metrics/src/metrics.rs b/code/crates/metrics/src/metrics.rs index ae09c43ad..47c1a448b 100644 --- a/code/crates/metrics/src/metrics.rs +++ b/code/crates/metrics/src/metrics.rs @@ -99,6 +99,12 @@ pub struct Inner { /// Time taken to verify a signature pub signature_verification_time: Histogram, + /// Number of equivocation votes + pub equivocation_votes: Counter, + + /// Number of equivocation proposals + pub equivocation_proposals: Counter, + /// Internal state for measuring time taken for consensus instant_consensus_started: Arc, @@ -130,6 +136,8 @@ impl Metrics { round: Gauge::default(), signature_signing_time: Histogram::new(exponential_buckets(0.001, 2.0, 10)), signature_verification_time: Histogram::new(exponential_buckets(0.001, 2.0, 10)), + equivocation_votes: Counter::default(), + equivocation_proposals: Counter::default(), instant_consensus_started: Arc::new(AtomicInstant::empty()), instant_block_started: Arc::new(AtomicInstant::empty()), instant_step_started: Arc::new(Mutex::new((Step::Unstarted, Instant::now()))), @@ -235,6 +243,18 @@ impl Metrics { "Time taken to verify a signature, in seconds", metrics.signature_verification_time.clone(), ); + + registry.register( + "equivocation_votes", + "Number of equivocation votes", + metrics.equivocation_votes.clone(), + ); + + registry.register( + "equivocation_proposals", + "Number of equivocation proposals", + metrics.equivocation_proposals.clone(), + ); }); metrics From 381bf85eb2f30f2cd0ad240b61ae8d94ecbdec66 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Mon, 12 May 2025 12:57:22 +0200 Subject: [PATCH 08/10] fix: applied @romac comments --- code/crates/metrics/src/metrics.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/crates/metrics/src/metrics.rs b/code/crates/metrics/src/metrics.rs index 47c1a448b..8e785426c 100644 --- a/code/crates/metrics/src/metrics.rs +++ b/code/crates/metrics/src/metrics.rs @@ -246,13 +246,13 @@ impl Metrics { registry.register( "equivocation_votes", - "Number of equivocation votes", + "Number of equivocating votes", metrics.equivocation_votes.clone(), ); registry.register( "equivocation_proposals", - "Number of equivocation proposals", + "Number of equivocating proposals", metrics.equivocation_proposals.clone(), ); }); From fd183e3d51cba18f57b1c4d1e2ddbb1d2499c4e6 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Tue, 13 May 2025 11:34:16 +0200 Subject: [PATCH 09/10] feat: inject consensus message --- code/crates/app-channel/src/run.rs | 6 ++- code/crates/app/src/node.rs | 5 +- code/crates/metrics/src/metrics.rs | 4 +- code/crates/starknet/host/src/node.rs | 7 +++ code/crates/test/app/src/node.rs | 7 ++- code/crates/test/framework/src/lib.rs | 5 ++ code/crates/test/framework/src/node.rs | 64 +++++++++++++++++++++++++- code/examples/channel/src/node.rs | 9 +++- 8 files changed, 98 insertions(+), 9 deletions(-) diff --git a/code/crates/app-channel/src/run.rs b/code/crates/app-channel/src/run.rs index baa387d1a..6a87d570a 100644 --- a/code/crates/app-channel/src/run.rs +++ b/code/crates/app-channel/src/run.rs @@ -22,7 +22,7 @@ pub async fn start_engine( cfg: Node::Config, start_height: Option, initial_validator_set: Ctx::ValidatorSet, -) -> Result<(Channels, EngineHandle)> +) -> Result<(Channels, EngineHandle)> where Ctx: Context, Node: node::Node, @@ -80,7 +80,8 @@ where ) .await?; - let (node, handle) = spawn_node_actor(ctx, network, consensus, wal, sync, connector).await?; + let (node, handle) = + spawn_node_actor(ctx, network, consensus.clone(), wal, sync, connector).await?; let channels = Channels { consensus: rx_consensus, @@ -90,6 +91,7 @@ where let handle = EngineHandle { actor: node, + consensus, handle, }; diff --git a/code/crates/app/src/node.rs b/code/crates/app/src/node.rs index ffa02b7ba..ce983c4e5 100644 --- a/code/crates/app/src/node.rs +++ b/code/crates/app/src/node.rs @@ -3,6 +3,7 @@ use std::path::PathBuf; use async_trait::async_trait; +use malachitebft_engine::consensus::{ConsensusRef, Msg}; use rand::{CryptoRng, RngCore}; use serde::de::DeserializeOwned; use serde::Serialize; @@ -18,8 +19,9 @@ use malachitebft_engine::util::events::RxEvent; use crate::types::core::{Context, PrivateKey, PublicKey, VotingPower}; use crate::types::Keypair; -pub struct EngineHandle { +pub struct EngineHandle { pub actor: NodeRef, + pub consensus: ConsensusRef, pub handle: JoinHandle<()>, } @@ -31,6 +33,7 @@ where { fn subscribe(&self) -> RxEvent; async fn kill(&self, reason: Option) -> eyre::Result<()>; + fn inject(&self, message: Msg) -> eyre::Result<()>; } pub trait NodeConfig { diff --git a/code/crates/metrics/src/metrics.rs b/code/crates/metrics/src/metrics.rs index 8e785426c..ef1c19738 100644 --- a/code/crates/metrics/src/metrics.rs +++ b/code/crates/metrics/src/metrics.rs @@ -99,10 +99,10 @@ pub struct Inner { /// Time taken to verify a signature pub signature_verification_time: Histogram, - /// Number of equivocation votes + /// Number of equivocating votes pub equivocation_votes: Counter, - /// Number of equivocation proposals + /// Number of equivocating proposals pub equivocation_proposals: Counter, /// Internal state for measuring time taken for consensus diff --git a/code/crates/starknet/host/src/node.rs b/code/crates/starknet/host/src/node.rs index 690480eaf..f37f33bc9 100644 --- a/code/crates/starknet/host/src/node.rs +++ b/code/crates/starknet/host/src/node.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; +use malachitebft_engine::consensus::Msg; use ractor::async_trait; use rand::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; @@ -64,6 +65,12 @@ impl NodeHandle for Handle { self.handle.abort(); Ok(()) } + + fn inject(&self, _message: Msg) -> eyre::Result<()> { + Err(eyre::eyre!( + "Injecting messages into the node is not supported in this application" + )) + } } #[derive(Clone, Debug)] diff --git a/code/crates/test/app/src/node.rs b/code/crates/test/app/src/node.rs index 6eb3554a9..3492e46dd 100644 --- a/code/crates/test/app/src/node.rs +++ b/code/crates/test/app/src/node.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use std::sync::Arc; use async_trait::async_trait; +use malachitebft_app_channel::app::engine::consensus::Msg; use malachitebft_test::middleware::{DefaultMiddleware, Middleware}; use rand::{CryptoRng, RngCore}; use tokio::task::JoinHandle; @@ -32,7 +33,7 @@ use crate::store::Store; pub struct Handle { pub app: JoinHandle<()>, - pub engine: EngineHandle, + pub engine: EngineHandle, pub tx_event: TxEvent, } @@ -48,6 +49,10 @@ impl NodeHandle for Handle { self.engine.handle.abort(); Ok(()) } + + fn inject(&self, message: Msg) -> eyre::Result<()> { + Ok(self.engine.consensus.cast(message)?) + } } /// Main application struct implementing the consensus node functionality diff --git a/code/crates/test/framework/src/lib.rs b/code/crates/test/framework/src/lib.rs index e01aae3ef..ffa3803f0 100644 --- a/code/crates/test/framework/src/lib.rs +++ b/code/crates/test/framework/src/lib.rs @@ -359,6 +359,11 @@ where } } + Step::Inject(message) => { + info!("Injecting message: {message:?}"); + handle.inject(message).unwrap(); + } + Step::Expect(expected) => { let actual = decisions.load(Ordering::SeqCst); diff --git a/code/crates/test/framework/src/node.rs b/code/crates/test/framework/src/node.rs index 7f686cf00..cbcea70d5 100644 --- a/code/crates/test/framework/src/node.rs +++ b/code/crates/test/framework/src/node.rs @@ -5,8 +5,10 @@ use eyre::bail; use tracing::info; use malachitebft_core_consensus::{LocallyProposedValue, SignedConsensusMsg}; -use malachitebft_core_types::{CommitCertificate, Context, Height, SignedVote, Vote, VotingPower}; -use malachitebft_engine::util::events::Event; +use malachitebft_core_types::{ + CommitCertificate, Context, Height, SignedProposal, SignedVote, Vote, VotingPower, +}; +use malachitebft_engine::{consensus::ConsensusMsg, util::events::Event}; use malachitebft_test::middleware::{DefaultMiddleware, Middleware}; use crate::Expected; @@ -26,6 +28,7 @@ where Expect(Expected), Success, Fail(String), + Inject(ConsensusMsg), } #[derive(Copy, Clone, Debug)] @@ -226,6 +229,63 @@ where }) } + pub fn on_vote_equivocation_evidence(&mut self, f: F) -> &mut Self + where + F: Fn( + Ctx::Height, + Ctx::Address, + (SignedVote, SignedVote), + &mut State, + ) -> Result + + Send + + Sync + + 'static, + { + self.on_event(move |event, state| { + if let Event::VoteEquivocationEvidence { + vote_height, + address, + evidence, + } = event + { + f(vote_height, address, evidence, state) + } else { + Ok(HandlerResult::WaitForNextEvent) + } + }) + } + + pub fn on_proposal_equivocation_evidence(&mut self, f: F) -> &mut Self + where + F: Fn( + Ctx::Height, + Ctx::Address, + (SignedProposal, SignedProposal), + &mut State, + ) -> Result + + Send + + Sync + + 'static, + { + self.on_event(move |event, state| { + if let Event::ProposalEquivocationEvidence { + proposal_height, + address, + evidence, + } = event + { + f(proposal_height, address, evidence, state) + } else { + Ok(HandlerResult::WaitForNextEvent) + } + }) + } + + pub fn inject(&mut self, message: ConsensusMsg) -> &mut Self { + self.steps.push(Step::Inject(message)); + self + } + pub fn on_decided(&mut self, f: F) -> &mut Self where F: Fn(CommitCertificate, &mut State) -> Result diff --git a/code/examples/channel/src/node.rs b/code/examples/channel/src/node.rs index 3bcbe348d..162b60b74 100644 --- a/code/examples/channel/src/node.rs +++ b/code/examples/channel/src/node.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use async_trait::async_trait; +use malachitebft_app_channel::app::engine::consensus::Msg; use rand::{CryptoRng, RngCore}; use tokio::task::JoinHandle; use tracing::Instrument; @@ -43,7 +44,7 @@ pub struct App { pub struct Handle { pub app: JoinHandle<()>, - pub engine: EngineHandle, + pub engine: EngineHandle, pub tx_event: TxEvent, } @@ -59,6 +60,12 @@ impl NodeHandle for Handle { self.engine.handle.abort(); Ok(()) } + + fn inject(&self, _message: Msg) -> eyre::Result<()> { + Err(eyre::eyre!( + "Injecting messages into the node is not supported in this example application" + )) + } } #[async_trait] From e85f9872c7920e57a7f808e390fafb49316fad72 Mon Sep 17 00:00:00 2001 From: Bastien Faivre Date: Tue, 13 May 2025 14:16:01 +0200 Subject: [PATCH 10/10] feat: non-working test --- code/Cargo.lock | 1 + code/crates/test/Cargo.toml | 1 + code/crates/test/tests/it/equivocation.rs | 130 ++++++++++------------ 3 files changed, 61 insertions(+), 71 deletions(-) diff --git a/code/Cargo.lock b/code/Cargo.lock index f2b5acaf3..b9804c0be 100644 --- a/code/Cargo.lock +++ b/code/Cargo.lock @@ -2529,6 +2529,7 @@ dependencies = [ "informalsystems-malachitebft-core-consensus", "informalsystems-malachitebft-core-types", "informalsystems-malachitebft-engine", + "informalsystems-malachitebft-peer", "informalsystems-malachitebft-proto", "informalsystems-malachitebft-signing-ed25519", "informalsystems-malachitebft-sync", diff --git a/code/crates/test/Cargo.toml b/code/crates/test/Cargo.toml index 3e37104f6..35cf287ff 100644 --- a/code/crates/test/Cargo.toml +++ b/code/crates/test/Cargo.toml @@ -16,6 +16,7 @@ malachitebft-codec = { workspace = true } malachitebft-core-types = { workspace = true } malachitebft-config = { workspace = true } malachitebft-core-consensus = { workspace = true } +malachitebft-peer = { workspace = true, features = ["rand"] } malachitebft-proto = { workspace = true } malachitebft-signing-ed25519 = { workspace = true, features = [ "rand", diff --git a/code/crates/test/tests/it/equivocation.rs b/code/crates/test/tests/it/equivocation.rs index a972144f3..50345694c 100644 --- a/code/crates/test/tests/it/equivocation.rs +++ b/code/crates/test/tests/it/equivocation.rs @@ -1,73 +1,61 @@ -use informalsystems_malachitebft_test::{ - middleware::Middleware, Address, Height, TestContext, Value, ValueId, Vote, -}; -use malachitebft_core_consensus::LocallyProposedValue; -use malachitebft_core_types::{NilOrVal, Round}; -use rand::Rng; - -#[derive(Copy, Clone, Debug)] -struct _EquivocationProposer; - -impl Middleware for _EquivocationProposer { - fn on_propose_value( - &self, - _ctx: &TestContext, - proposal: &mut LocallyProposedValue, - reproposal: bool, - ) { - if !reproposal { - tracing::warn!( - "EquivocationProposer: First time proposing value {:}", - proposal.value.id() - ); - - // Keep the proposal value as is - return; - } - - // Change the proposal value to a different one - let new_value = loop { - let new_value = Value::new(rand::thread_rng().gen_range(100..=100000)); - if new_value != proposal.value { - break new_value; - } - }; - - tracing::warn!( - "EquivocationProposer: Reproposing value {:} instead of {:}", - new_value.id(), - proposal.value.id() - ); +use std::time::Duration; - proposal.value = new_value; - } -} - -#[derive(Clone, Debug)] -struct _EquivocationVoter; - -impl Middleware for _EquivocationVoter { - fn new_prevote( - &self, - _ctx: &TestContext, - height: Height, - round: Round, - value_id: NilOrVal, - address: Address, - ) -> Vote { - if round.as_i64() % 2 == 0 { - // Vote for the given value - Vote::new_prevote(height, round, value_id, address) - } else { - // Vote for a different value - let new_value = loop { - let new_value = ValueId::new(rand::thread_rng().gen_range(100..=100000)); - if NilOrVal::Val(new_value) != value_id { - break new_value; - } - }; - - Vote::new_prevote(height, round, NilOrVal::Val(new_value), address) - } - } +use informalsystems_malachitebft_test::{Address, Height, Proposal, Value}; +use malachitebft_core_consensus::LocallyProposedValue; +use malachitebft_core_types::{Round, SignedProposal}; +use malachitebft_engine::{consensus::Msg, network::NetworkEvent}; +use malachitebft_peer::PeerId; +use malachitebft_signing_ed25519::Signature; +use malachitebft_test_framework::TestParams; + +use crate::TestBuilder; + +#[tokio::test] +pub async fn equivocation_proposer() { + const HEIGHT: u64 = 3; + + let mut test = TestBuilder::<()>::new(); + + test.add_node() + .start() + // TODO: We do not have access to the peer id or address, and we cannot + // sign the message + .inject(Msg::NetworkEvent(NetworkEvent::Proposal( + PeerId::random(), + SignedProposal::new( + Proposal::new( + Height::new(1), + Round::Some(0), + Value::new(0), + Round::Nil, + Address::new([0; 20]), + ), + Signature::test(), + ), + ))) + // .on_proposal_equivocation_evidence(|_height, _address, _evidence, _state| { + // info!("Equivocation evidence detected"); + // Ok(HandlerResult::ContinueTest) + // }) + .wait_until(HEIGHT) + .success(); + + test.add_node() + .start() + // TODO: Does not work as engine/driver will not propose two values + .inject(Msg::ProposeValue(LocallyProposedValue { + height: Height::new(1), + round: Round::Some(0), + value: Value::new(0), + })) + // .on_proposal_equivocation_evidence(|_height, _address, _evidence, _state| { + // info!("Equivocation evidence detected"); + // Ok(HandlerResult::ContinueTest) + // }) + .wait_until(HEIGHT) + .success(); + + test.build() + .run_with_params(Duration::from_secs(5), TestParams::default()) + .await }