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,