fix: dispute two-candidate consensus splits - #581
Conversation
Change-Id: I073d15645a2334b4cf0075d8e3f1ecba3a49d0db
Not the same class of change as #573 — but drop the terminality gateReviewed against #573 (closed today), since both descend from the same root: after #569, Reachability inverts#573 needed the ACTIVE roster to fall below This PR's condition needs one operator, not a roster collapse. A 7-member group losing a single member mid-epoch gives That is the same mid-window attrition #573 accepted as residual scope, at even arity instead of arity-1. Blast radius is much smaller#573 mutated group membership, breaking the "window groups are disjoint by construction" invariant that This PR changes only when a dispute opens. No membership, no scheduling, no attestation the outposts position against. The The terminality gate should goThe 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. Resolution then works exactly as intended: Tier-1 votes canonical, Suggested change — this makes the diff smaller, not larger:
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 accepted1. Once a dispute exists, // 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. // 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 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 Still needs the isolated A/BThe 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 |
Change-Id: Id0cbaa4fa450435cdc928b501a90222f65c2f38e
Change-Id: I510b5f4ee60976920fa494faef70406eeb0f86a7
|
Thanks — agreed, and implemented as suggested.
The requested A/B is green: |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
|
For visibility on the design this PR sits inside — the consensus-outcome model was settled
So nothing in that rework conflicts with #581, and the floor of 2 stands under it. Separately, confirming the review findings against |
Summary
Regression coverage
Contract artifacts
Validation
Delivery notes