Skip to content

fix: dispute two-candidate consensus splits - #581

Merged
huangminghuang merged 6 commits into
masterfrom
fix/wire-362-two-candidate-disputes
Aug 28, 2026
Merged

fix: dispute two-candidate consensus splits#581
huangminghuang merged 6 commits into
masterfrom
fix/wire-362-two-candidate-disputes

Conversation

@huangminghuang

@huangminghuang huangminghuang commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Implements the WIRE-362 terminality rule in sysio.msgch: a post-boundary two-version no-majority split opens only after every eligible batch operator has delivered. The existing post-boundary three-or-more-version behavior is unchanged.
  • Preserves the normal strict-majority path: an incomplete 1-1 split with a silent eligible operator remains undecided and unpaused until a later delivery can form a majority.
  • Lowers the defensive sysio.chalg candidate floor to two versions, while keeping terminality, the live delivery tally, and the Tier-1 preflight in msgch. An empty Tier-1 electorate soft-declines the terminal delivery and permits a later chkcons retry; direct chalg calls still reject an unresolvable electorate.
  • Corrects the sysio.chalg README and public contract documentation to describe the exact E=2 terminal gate and unchanged 3+ behavior.

Regression coverage

  • End-to-end chkcons to evalcons to opendispute coverage for terminal E=2 / 1-1 and E=6 / 3-3 ties, including exact candidate tallies and dispute resolution.
  • Incomplete 1-1 / three-eligible regression proves a late delivery can still establish strict-majority consensus without opening a dispute.
  • Empty-Tier-1 regression proves terminal delivery soft-declines unpaused and the same crank opens a dispute after Tier-1 registration.
  • opendispute rejects a one-candidate direct call with the exact ABI assertion message.

Contract artifacts

  • Regenerated and byte-validated sysio.msgch.wasm: f9ad20e767ec53d0d97e24002d71ab7fffe33e650c41aa394b178daa6fa4840e.
  • sysio.chalg.wasm byte-matches the release CDT output: 112d1c92d6c8bca69554bec109527ebfa8156c444a7b54207464ece1b5154652.

Validation

Delivery notes

  • Normal follow-up commits only; no history was rewritten.
  • The earlier platform attempt was canceled because it predated the corrected companion terminal-flow topology and is not claimed as validation.

Change-Id: I073d15645a2334b4cf0075d8e3f1ecba3a49d0db
@huangminghuang
huangminghuang marked this pull request as ready for review August 21, 2026 18:09
@huangminghuang
huangminghuang requested review from a team and heifner August 21, 2026 18:09
@heifner

heifner commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Not the same class of change as #573 — but drop the terminality gate

Reviewed against #573 (closed today), since both descend from the same root: after #569, eligible_batch_operators() intersects the seated group with live opreg ACTIVE status and carries no floor, so the eligible set can shrink mid-epoch. They are not the same change, and the reason #573 died does not transfer.

Reachability inverts

#573 needed the ACTIVE roster to fall below batch_operator_minimum_active (21 = 7 × 3), which bootstrapped batch operators make near-unreachable in code — termcheck early-returns on is_bootstrapped, meets_role_min returns true, they are created ACTIVE. Inert when the roster is healthy.

This PR's condition needs one operator, not a roster collapse. A 7-member group losing a single member mid-epoch gives eligible == 6; a 3–3 split then has no strict majority possible (3 * 2 > 6 is false) and, on master, no dispute path either — maybe_open_dispute returns at seen_checksums.size() < 3 (sysio.msgch.cpp:1190). Permanent deadlock.

That is the same mid-window attrition #573 accepted as residual scope, at even arity instead of arity-1. .claude/rules/opp-consensus.md already states even sizes are reachable "whenever the serving group is short of its configured size."

Blast radius is much smaller

#573 mutated group membership, breaking the "window groups are disjoint by construction" invariant that batch_operator_plugin (my_group, last match wins) and OPPInbound._resolveChunkPosition (first match) rely on in opposite directions — the 462 × EffectAccountMissing that failed flow-batch-operator-termination.

This PR changes only when a dispute opens. No membership, no scheduling, no attestation the outposts position against. The opendispute >= 3>= 2 widening sits behind require_auth(MSGCH_ACCOUNT), so msgch's rule is the effective gate. #573's failure mode has no analogue here.

The terminality gate should go

The total_deliveries != group_size branch adds a precondition that a missing operator makes permanently unsatisfiable. A 1–1 split of 3 eligible with the third operator silent never reaches total_deliveries == group_size, can never form a majority (needs 2), and so never opens a dispute — the same permanent stall this PR sets out to fix, one arity over. And the silent operator cannot be terminated out of the way, because termcheck only runs from advance, which is what is stalled.

That is backwards: the case where adjudication is most needed — an operator is gone and the remainder disagree — is the case terminality refuses to adjudicate.

The premise behind the gate does not hold either. We do not expect any divergent envelope versions, much less two. Post-boundary, ≥2 distinct versions with no strict majority is already the anomaly — that is the dispute trigger. And the boundary is a deadline, not a checkpoint: an operator that has not delivered by then has burned the entire epoch and is a miss, not a straggler. opp-consensus.md already defines late delivery as a benign no-op.

Resolution then works exactly as intended: Tier-1 votes canonical, resolvedisp dispatches the winner, and the next advance slashes every operator whose delivered checksum ≠ winner. Silence is never slashed — it stays on the recorddel/termcheck miss ladder. So the 1–1-of-3 case lands correctly: the wrong deliverer is slashed, the silent one rides the miss ladder to termination.

Suggested change — this makes the diff smaller, not larger:

  • delete the total_deliveries != group_size branch and the total_deliveries parameter from maybe_open_dispute
  • msgch::maybe_open_dispute: seen_checksums.size() < 3< 2
  • chalg::opendispute: candidates.size() >= 3>= 2

Boundary check and majority check are unchanged and both already sit ahead of it. That the silent-operator hole closes without a special case is a good sign the gate was the wrong shape.

Two consequences, both accepted

1. Once a dispute exists, evalcons no-ops for that bucket.

// If a dispute has been opened for this (outpost, epoch), the dispute-vote flow owns its
// resolution and winner dispatch (via `resolvedisp`). evalcons is a no-op for this bucket
// so a late delivery cannot re-open the dispute or re-dispatch the envelope.

So opening a dispute forecloses free automatic resolution by a late delivery. Accepted — post-boundary non-delivery is already a miss.

2. chkdispute has no timeout escape, and that is fine.

// After the deadline: a quorum of cast votes AND a strict majority of cast votes.
// No plurality / tie-break — an unresolved tally just keeps waiting for more votes.
if (!resolved) return;

Past the deadline you still need cast >= Q where Q = N/2 + 1 of the snapshotted electorate. If a majority of Tier-1 owners never vote, the dispute stays OPEN, open_disputes never decrements, and the epoch stays paused indefinitely.

This is an accepted design decision, not a gap to close. The trade is deliberate: replace "operators must deliver" with "a majority of Tier-1 must vote." Pausing until Tier-1 votes is the correct outcome — the alternatives (plurality, or abandon-and-resume) either pick a winner on weak evidence or hand back the original deadlock. Worth stating explicitly in the PR body so it is a recorded decision rather than an implicit one, and worth confirming the batch-operator crank actually drives chkdispute on its ~15s cadence under the flows.

Still needs the isolated A/B

The PR body notes the platform-flow gates are pending. That is exactly the state #573 was in before the run that closed it — 659/659 unit green, independent reviews passed, and an earlier GREEN 5-flow gate on a prior commit. It still regressed once the right flow ran against the right head.

Same protocol as runs 32486851217 / 32492513750: every ref pinned by full SHA, wire-sysio the only variable, PR head vs master. flow-batch-operator-slashing as the target (it is the flow that produces non-canonical deliveries) and flow-batch-operator-termination as the control, since that is the one #573 broke.

Change-Id: Id0cbaa4fa450435cdc928b501a90222f65c2f38e
@huangminghuang huangminghuang changed the title fix: resolve terminal two-candidate consensus ties fix: dispute two-candidate consensus splits Aug 21, 2026
Change-Id: I510b5f4ee60976920fa494faef70406eeb0f86a7
@huangminghuang

Copy link
Copy Markdown
Contributor Author

Thanks — agreed, and implemented as suggested.

  • Removed the total_deliveries != group_size terminality gate and its parameter from maybe_open_dispute.
  • Lowered both candidate floors from 3 to 2 in sysio.msgch and sysio.chalg; the boundary and strict-majority checks are unchanged.
  • Added end-to-end chkcons regressions for 1–1 and 3–3 ties, including the 1–1-of-3 eligible topology with a silent third operator. They drive Tier-1 resolution and verify the dispatched winner.
  • Recorded the intentional post-dispute behavior: evalcons is a no-op once a dispute exists, and an unresolved Tier-1 tally keeps the epoch paused rather than selecting a weak winner.

The requested A/B is green: flow-batch-operator-slashing passed in 445.9s and the flow-batch-operator-termination control passed in 977.8s, both with the heartbeat monitor and no fatal chain diagnostics. The current merged PR heads also passed the full-platform E2E (build, artifact verification, and standard all-flow pool): https://github.com/Wire-Network/wire-platform-build-system/actions/runs/32541857619

@heifner heifner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at d9b885c0c against WIRE-362.

The fix is right and the floor of 2 is correct. An even split across exactly two versions had no path at all — unanimity fails, count * 2 > group_size fails on an exact half, and the 3-candidate floor refused the dispute. Not "stalls until voters decide": there was no vote to have, and no operator left who could change it. Lowering the floor gives that split a ballot.

Worth recording why 3 was never the right number: group sizes are odd by policy, and with an odd group a two-way split always yields a strict majority, so 2 candidates never needed a vote. The floor of 3 was a proxy for "nobody can win here." That proxy breaks as soon as the live eligible count goes even — which slashing does mid-window, since the odd-size policy is a scheduling precondition and not a runtime invariant.

On the reversal in d9ac657dc. The ticket's revised plan (comment 16040) gates the two-candidate path on total_deliveries == group_size, and explicitly supersedes the blanket floor drop as unsafe. This PR ships the blanket drop, and chkcons_opens_dispute_for_incomplete_two_way_split asserts the inverse of that plan's acceptance criterion. That looks deliberate rather than accidental — the doc comment says "including when an eligible operator was silent" — but the ticket was never updated, so the superseded plan currently reads as the spec. Either way one of the two should move; see the comment on maybe_open_dispute for why I think the completeness condition is still worth having.

Separately, and not this PR's problem: the majority branch is being reworked — WIRE-370 (terminate the minority without seizing collateral; the existing Tier-1 electorate then votes capture or refund) and WIRE-369 (never slash a bootstrapped operator; a bootstrapped minority goes to dispute instead). WIRE-369 blocks WIRE-370. Nothing here conflicts with either — this is the no-majority fallback and stays correct under that design.

if (seen_checksums.size() < 3) {
sysio::print_f("msgch::maybe_open_dispute: no dispute for (chain=%llu, epoch=%u): %u distinct version(s), a vote needs >=3\n",
chain_code, epoch_index, (uint32_t)seen_checksums.size());
if (seen_checksums.size() < chalg_limits::minimum_dispute_candidate_versions) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MEDIUM — the candidate set freezes before the eligible group has finished delivering, and the right envelope can be excluded from its own ballot.

The gate here is candidate-count + past-boundary + no-majority. Nothing checks that the outstanding eligible operators have delivered, and total_deliveries is not even passed in.

That matters because the ballot is final once it opens. evalcons returns immediately when a dispute row exists for the (outpost, epoch)"a late delivery cannot re-open the dispute or re-dispatch the envelope" — so the candidate list is frozen at whatever had arrived.

Concretely, with a 5-operator group: two lagging or faulty operators deliver X and Y shortly past the boundary, before the honest three deliver Z. Any chkcons crank in that window opens a dispute on {X, Y}. Tier-1 is now asked to choose between two versions, neither of which is the one an honest majority was about to produce, and Z can never be added. The three honest operators then hold a checksum that does not match the winner.

This is not new — the same shape was reachable at the 3-candidate floor with three divergent early deliveries — but the floor of 2 makes it reachable with two, and deliver has no boundary cutoff, so the window is real rather than theoretical.

The ticket's total_deliveries == group_size condition closes it: the ballot cannot be frozen while an eligible operator can still change the outcome. If the intent is instead that a post-boundary two-way split is anomalous enough to adjudicate even with outstanding deliverers, that is a defensible call — but it is the opposite of the plan recorded in WIRE-362, and the ticket should say so rather than leaving the superseded version standing.

require_auth(MSGCH_ACCOUNT);
check(candidates.size() >= 3,
"a dispute requires at least 3 candidate envelope versions");
check(candidates.size() >= chalg_limits::minimum_dispute_candidate_versions,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MEDIUM — an empty Tier-1 electorate now reverts the enclosing delivery transaction.

opendispute is an inline action of evalcons, which is itself inline from deliver and from chkcons. So check(!electorate.empty(), ...) does not just decline the dispute — it reverts the whole enclosing transaction.

The in-place comment justifies the assert on the grounds that "the conflicting deliveries keep this epoch from reaching consensus regardless." That held under the 3-candidate floor, where a dispute-eligible split was already unresolvable. It no longer holds: with the floor at 2, a dispute can now open on a split that a further delivery would have resolved into a clean majority. Once that state exists and the electorate is empty — e.g. during a roa generation rotation before Tier-1 owners re-register — every subsequent deliver and chkcons for that bucket reverts, including the delivery that would have produced the majority.

Suggest checking the electorate on the msgch side in maybe_open_dispute and printing-and-returning, matching the silent-return diagnostic convention the other branches in this function already use.

} FC_LOG_AND_RETHROW() }

/// One envelope version has no competing candidate, so chalg must retain the two-version floor.
BOOST_FIXTURE_TEST_CASE(opendispute_rejects_one_candidate, sysio_dispute_tester) { try {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LOW — this test no longer pins the invariant it exists for.

The replacement asserts opendispute(...) != success(), which passes for any failure: a wrong assertion message, an auth failure, a duplicate-dispute rejection, or a fixture regression that makes the push fail for an unrelated reason. The previous version asserted the exact message, which is what made it a test of the candidate floor specifically.

Since the floor is the invariant this PR changes, it is worth keeping it pinned — assert the exact new message the way the old test asserted the old one.

// Group envelopes by checksum, tracking the operators that delivered each version (CDT-compatible
// parallel vectors). The per-version operator lists become the dispute candidates on a 3+-way
// split.
// parallel vectors). The per-version operator lists become the dispute candidates on a terminal

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LOW — this comment contradicts the implemented behaviour.

"a terminal two-way or an existing multi-version split" describes the superseded plan. The shipped trigger deliberately fires on a non-terminal two-way split — chkcons_opens_dispute_for_incomplete_two_way_split covers exactly that case, a 1–1 split with a silent third eligible operator.

The comments at :1181 and :1507 use the accurate wording ("including when an eligible operator was silent"); this one should match. Calling the trigger "terminal" invites the next reader to assume the delivery set is complete when the candidates freeze, which is the assumption the comment on maybe_open_dispute turns on.

Change-Id: I4fc0ebee435dc14bdea536cef33e2d1c7ae7e7f6
…ndidate-disputes

Change-Id: I580ca5ed3ab8a76f9cbc3bf9a78976c13d86e5c5
Change-Id: Id8df1153a715b5ddcd76a85e7c95536c4efe24b1
@heifner

heifner commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

For visibility on the design this PR sits inside — the consensus-outcome model was settled
in a meeting on 2026-08-27 and is now recorded on WIRE-362:

  • Majority, no bootstrapped operator in the minority → the minority is TERMINATED so it
    stops participating, collateral held rather than seized; the existing Tier-1
    electorate then votes capture or refund, and only a capture slashes (WIRE-370).
  • Majority, bootstrapped operator in the minority → dispute. Bootstrapped operators are
    trusted and are never terminated or slashed (WIRE-369opreg::slash has no
    is_bootstrapped guard today, so this is currently mis-handled in shipped code).
  • No majority → dispute. That is this PR's branch, and it is unaffected by the above.

So nothing in that rework conflicts with #581, and the floor of 2 stands under it.

Separately, confirming the review findings against 234abff29: the completeness condition
is restored with total_deliveries threaded through (2 candidates + incomplete → wait, 3+
unchanged), the Tier-1 electorate is now checked on the msgch side with a print-and-return
so a missing electorate cannot revert the terminal delivery, the floor test asserts the
exact message again, and the terminal wording now matches the behaviour. All four are
resolved — thanks for the quick turnaround.

@huangminghuang
huangminghuang merged commit de368a2 into master Aug 28, 2026
12 checks passed
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.

2 participants