-
Notifications
You must be signed in to change notification settings - Fork 116
fix(CCHAIN-1176): Change equivocation tests' logic so they aren't flaky on CI #1510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -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<Ctx: Context>(evidence: &MisbehaviorEvidence<Ctx>) { | ||||||||
|
|
@@ -27,36 +28,28 @@ fn check_decided_impl<Ctx: Context>(evidence: &MisbehaviorEvidence<Ctx>) { | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| // 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 | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| 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). | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| test.add_node().start().success(); | ||||||||
|
|
||||||||
| // Node 3 -- checking proposal equivocation evidence | ||||||||
| // Node 3 -- checking proposal equivocation evidence. | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| // 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() { | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| // 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. | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| 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. | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| 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 { | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| 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; | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.