From f644a413ca50ba8dc8f30876ada9ce43d54979b5 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Thu, 12 Feb 2026 14:24:31 +0000 Subject: [PATCH] Change tests logic so they aren't flaky on CI --- code/crates/test/framework/src/lib.rs | 4 -- code/crates/test/framework/src/node.rs | 1 - code/crates/test/tests/it/equivocation.rs | 74 +++++++++++------------ code/crates/test/tests/it/middlewares.rs | 20 ++++++ 4 files changed, 56 insertions(+), 43 deletions(-) diff --git a/code/crates/test/framework/src/lib.rs b/code/crates/test/framework/src/lib.rs index e0781f4a8..4bfd98976 100644 --- a/code/crates/test/framework/src/lib.rs +++ b/code/crates/test/framework/src/lib.rs @@ -362,10 +362,6 @@ where Ok(HandlerResult::ContinueTest) => { break 'inner; } - Ok(HandlerResult::SleepAndContinueTest(duration)) => { - sleep(duration).await; - break 'inner; - } Err(e) => { error!("Event handler returned an error: {e}"); diff --git a/code/crates/test/framework/src/node.rs b/code/crates/test/framework/src/node.rs index 8480914e4..0a48644e9 100644 --- a/code/crates/test/framework/src/node.rs +++ b/code/crates/test/framework/src/node.rs @@ -37,7 +37,6 @@ where pub enum HandlerResult { WaitForNextEvent, ContinueTest, - SleepAndContinueTest(Duration), } pub type EventHandler = diff --git a/code/crates/test/tests/it/equivocation.rs b/code/crates/test/tests/it/equivocation.rs index ce3304541..3d8a0dfce 100644 --- a/code/crates/test/tests/it/equivocation.rs +++ b/code/crates/test/tests/it/equivocation.rs @@ -4,9 +4,10 @@ use malachitebft_core_consensus::MisbehaviorEvidence; use malachitebft_core_types::{Context, Proposal, Vote}; use malachitebft_test_framework::{HandlerResult, TestParams}; +use crate::middlewares::PrevoteRandom; use crate::TestBuilder; -const VOTE_DURATION: Duration = Duration::from_millis(300); +const TARGET_TIME: Duration = Duration::from_secs(1); #[allow(clippy::never_loop)] fn check_decided_impl(evidence: &MisbehaviorEvidence) { @@ -27,36 +28,28 @@ fn check_decided_impl(evidence: &MisbehaviorEvidence) { } } -// Verifies that sharing a validator key across two nodes -// induces equivocation and that decide-time `MisbehaviorEvidence` -// contains double proposals and double prevotes for the equivocator. -// Node 3 checks for proposal equivocation evidence. #[tokio::test] -#[ignore] // Flaky test pub async fn equivocation_two_vals_same_pk_proposal() { - // Nodes 1 and 2 share a validator key to induce proposal equivocation + // Nodes 1 and 2 share a validator key to induce proposal equivocation. let params = TestParams { shared_key_group: HashSet::from([1, 2]), + target_time: Some(TARGET_TIME), ..Default::default() }; let mut test = TestBuilder::<()>::new(); // Node 1 - test.add_node() - .start() - .on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION))) - .success(); + test.add_node().start().success(); - // Node 2 (same validator key as node 1) - test.add_node() - .start() - .on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION))) - .success(); + // Node 2 (same validator key as node 1). + test.add_node().start().success(); - // Node 3 -- checking proposal equivocation evidence + // Node 3 -- checking proposal equivocation evidence. + // Voting power is set to hold >2/3 worth of VP so consensus always progresses. + // Proposals are processed regardless of consensus state. test.add_node() + .with_voting_power(5) .start() - .on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION))) .on_finalized(|_c, evidence, _s| { check_decided_impl(&evidence); let result = if evidence.proposals.is_empty() { @@ -73,48 +66,53 @@ pub async fn equivocation_two_vals_same_pk_proposal() { .await; } -// Verifies that sharing a validator key across two nodes -// induces equivocation and that decide-time `MisbehaviorEvidence` -// contains double proposals and double prevotes for the equivocator. -// Node 3 checks for vote equivocation evidence. +/// Vote equivocation test with 7 nodes. +/// +/// Nodes 1 and 2 share validator key. Node 2 uses `PrevoteRandom`, node 1 votes normally. +/// +/// Need five honest nodes (3-7) so that +/// * equivocator holds < 1/3 of total VP +/// * no single honest node has >2/3 of total VP, so needs to collect votes from others #[tokio::test] -#[ignore] // Flaky test pub async fn equivocation_two_vals_same_pk_vote() { - // Nodes 1 and 2 share a validator key to induce vote equivocation + // Nodes 1 and 2 share a validator key to induce vote equivocation. let params = TestParams { shared_key_group: HashSet::from([1, 2]), + target_time: Some(TARGET_TIME), ..Default::default() }; let mut test = TestBuilder::<()>::new(); - // Node 1 - test.add_node() - .start() - .on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION))) - .success(); + // Node 1 votes normally. + test.add_node().start().success(); - // Node 2 (same validator key as node 1) + // Node 2 (same validator key as node 1) prevotes for random values. test.add_node() + .with_middleware(PrevoteRandom) .start() - .on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION))) .success(); - // Node 3 -- checking vote equivocation evidence + // Nodes 3 to 6 (honest) + for _ in 0..4 { + test.add_node().start().success(); + } + + // Node 7 (honest) checks vote equivocation evidence. test.add_node() .start() - .on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION))) - .on_finalized(|_c, evidence, _s| { + .on_finalized(move |_c, evidence, _s| { check_decided_impl(&evidence); - let result = if evidence.votes.is_empty() { - HandlerResult::WaitForNextEvent - } else { + let has_vote_equivocation = !evidence.votes.is_empty(); + let result = if has_vote_equivocation { HandlerResult::ContinueTest + } else { + HandlerResult::WaitForNextEvent }; Ok(result) }) .success(); test.build() - .run_with_params(Duration::from_secs(15), params) + .run_with_params(Duration::from_secs(30), params) .await; } diff --git a/code/crates/test/tests/it/middlewares.rs b/code/crates/test/tests/it/middlewares.rs index fd0644744..4a3a2ec8b 100644 --- a/code/crates/test/tests/it/middlewares.rs +++ b/code/crates/test/tests/it/middlewares.rs @@ -85,3 +85,23 @@ impl Middleware for PrevoteNil { } } } + +#[derive(Copy, Clone, Debug)] +pub struct PrevoteRandom; + +impl Middleware for PrevoteRandom { + fn new_prevote( + &self, + _ctx: &TestContext, + height: Height, + round: Round, + _value_id: NilOrVal, + address: Address, + ) -> Vote { + use rand::Rng; + let random = NilOrVal::Val(ValueId::new( + rand::thread_rng().gen_range(100_000..=999_999), + )); + Vote::new_prevote(height, round, random, address) + } +}