Skip to content

fix(CCHAIN-1176): Change equivocation tests' logic so they aren't flaky on CI#1510

Open
sergio-mena wants to merge 2 commits intocirclefin:mainfrom
sergio-mena:CCHAIN-1176-equivocation-tests-flaky
Open

fix(CCHAIN-1176): Change equivocation tests' logic so they aren't flaky on CI#1510
sergio-mena wants to merge 2 commits intocirclefin:mainfrom
sergio-mena:CCHAIN-1176-equivocation-tests-flaky

Conversation

@sergio-mena
Copy link
Contributor

@sergio-mena sergio-mena commented Feb 27, 2026

Closes: #1509

  • Replace on_vote sleep-based synchronization with target_time finalization window, giving late equivocation evidence time to arrive
  • Proposal test: give node 3 enough VP (>2/3) so consensus always progresses regardless of equivocator behavior
  • Vote test: expand from 3 to 7 nodes with a PrevoteRandom middleware to reliably induce prevote equivocation while keeping the equivocator under 1/3 VP

Story: https://circlepay.atlassian.net/browse/CCHAIN-1176


PR author checklist

Contribution eligibility

  • I am a core contributor, OR I have been explicitly assigned to the linked issue
  • I have read CONTRIBUTING.md and my PR complies with all requirements
  • I understand that PRs not meeting these requirements will be closed without review

For all contributors

For external contributors

@sergio-mena sergio-mena changed the title Change tests logic so they aren't flaky on CI fic(CCHAIN-1176): Change tests logic so they aren't flaky on CI Feb 27, 2026
@sergio-mena sergio-mena changed the title fic(CCHAIN-1176): Change tests logic so they aren't flaky on CI fix(CCHAIN-1176): Change tests logic so they aren't flaky on CI Feb 27, 2026
@sergio-mena sergio-mena marked this pull request as ready for review February 27, 2026 13:29
@sergio-mena sergio-mena changed the title fix(CCHAIN-1176): Change tests logic so they aren't flaky on CI fix(CCHAIN-1176): Change equivocation tests' logic so they aren't flaky on CI Feb 27, 2026
Copy link
Contributor

@cason cason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this. Works like a charm locally, single height always. Lets see in CI.

};
let mut test = TestBuilder::<()>::new();

// Node 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Node 1
// Node 1 - Byzantine

.start()
.on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION)))
.success();
// Node 2 (same validator key as node 1).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Node 2 (same validator key as node 1).
// Node 2 - Byzantine: same validator key as node 1

test.add_node().start().success();

// Node 3 -- checking proposal equivocation evidence
// Node 3 -- checking proposal equivocation evidence.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Node 3 -- checking proposal equivocation evidence.
// Node 3: correct, with >2/3 of the total voting power.
// Checks for proposal equivocation.

/// * 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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub async fn equivocation_two_vals_same_pk_vote() {
pub async fn equivocation_two_vals_same_key_vote() {

.start()
.on_vote(|_v, _s| Ok(HandlerResult::SleepAndContinueTest(VOTE_DURATION)))
.success();
// Node 1 votes normally.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Node 1 votes normally.
// Node 1 - Byzantine

// Node 3 checks for proposal equivocation evidence.
#[tokio::test]
#[ignore] // Flaky test
pub async fn equivocation_two_vals_same_pk_proposal() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub async fn equivocation_two_vals_same_pk_proposal() {
pub async fn equivocation_two_vals_same_key_proposal() {

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Node 2 (same validator key as node 1) prevotes for random values.
// Node 2 - Byzantine: same validator key as node 1 and prevotes for random values.


// Node 3 -- checking vote equivocation evidence
// Nodes 3 to 6 (honest)
for _ in 0..4 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for _ in 0..4 {
for _ in 3..=6 {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky equivocation tests

2 participants