Skip to content

fix(v2): gate the optimistic timeout default on a reveal-quorum floor (#167) - #185

Merged
collinsezedike merged 4 commits into
drydocs:mainfrom
ZacLou:fix/withheld-reveal-quorum-167
Sep 6, 2026
Merged

collinsezedike merged 4 commits into
drydocs:mainfrom
ZacLou:fix/withheld-reveal-quorum-167

Conversation

@ZacLou

@ZacLou ZacLou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #167.

Problem

register freezes weight into resolution.eligible_total regardless of whether the position ever reveals. Only revealed weight enters agree_weight/disagree_weight, but non-revealed weight stays counted when the strict-majority threshold and the optimistic timeout default are checked. An actor can register heavy weight on the side that helps the asserter's claim stand, never reveal it, and thereby (a) make it harder for a genuine, fully-revealed opposing majority to exceed half of the eligible total, and (b) push an otherwise-clear dispute into the timeout default — the asserted outcome stands — instead of the outcome real participants revealed for. As the issue notes, the coalition's net cost can even be below the nominal bond if it recycles forfeiture through positions that did reveal.

Change (per the confirmed design direction in the issue thread)

  1. A 50% reveal-quorum floor gates the timeout default. At close, revealed_weight * 2 > eligible_total must hold — the same subtraction form the strict-majority lock already uses (side_weight > W - side_weight), so at-or-below-half never qualifies and the threshold needs no new justification, it's the existing one. At or below half, revealed weight isn't a genuine majority of everything committed and the default is withheld. The 50% figure is a starting hypothesis: the property tests below sweep the boundary region, and the trade-off analysis is spelled out in the module docs for maintainer review, per the issue's scope note.

  2. If the quorum fails, the round is voided, not defaulted. New TerminalCause::RevealQuorumNotMet, appended last so every already-persisted XDR variant encoding stays stable (the TimeoutDefaultRule doc gained the gating note). final_outcome stays None. Settlement pays the same bonds-back, no-forfeiture pool AdminCancelled uses (see settlement_pool): every funded position, revealed or not, recovers its exact principal through the ordinary settle/withdraw path. No reward for withholding: the round produces no outcome and the withheld strategy earns nothing it didn't already have.

  3. RoundVoided instead of Resolved. A distinct event (carrying revealed_weight/eligible_total so the void condition is auditable straight from the event), separate for the same reason RoundCancelled is separate from Resolved: indexers can tell a real outcome from a round that produced none without inferring it from assertion state.

Tests

118 pass (107 pre-existing + 11 affected/new-touched). New withheld_reveal_quorum module:

  • Quorum-met timeout default survives (the gate must not over-fire).
  • The exact issue attack — register-heavy on the claim's side, never reveal — now voids; bonds back; conservation of the frozen eligible total holds.
  • Below-half close (a quarter revealed) voids.
  • Zero-third-party dispute still resolves OptimisticTimeout (revealed == eligible, quorum met): the gate can't strand a no-registration dispute forever.
  • Property test (96 cases proptest across voter counts, stakes, sides, reveal masks; driver-reveal forced where needed for v2's persisted-reveal_deadline clock) asserting the invariant trichotomy on every closed round: strict majority (outcome set) / quorum-met timeout (revealed*2 > eligible, asserted outcome stands) / voided (revealed*2 <= eligible, final_outcome None, every position settles to exactly principal).

The proptest_settlement mirrors (expected_pool, is_recipient, dust-recipient) were extended to the new cause so the existing conservation properties continue to hold unchanged across the voided case.

Compatibility note

Resolution/Position/PolicySnapshotV2 shapes are untouched; the only on-chain data-model change is one appended enum variant in TerminalCause (XDR variant tags unchanged for all pre-existing values) — no storage migration needed. Behavior change is deliberate and confined to closes with revealed_weight * 2 <= eligible_total, which under the old code would have taken the asserted-outcome default: those rounds now void with bonds back.

…drydocs#167)

Withheld-reveal registration could deny a genuine majority and steer a
dispute into the optimistic timeout default (asserted outcome stands):
an actor registers heavy weight on the side that helps the claim stand,
never reveals it, and inflates the eligible total so no revealed weight
can reach strict majority nor a legitimate quorum. Per the maintainer's
confirmed direction:

- The timeout default now only applies when revealed weight is a genuine
  majority of everything committed: strictly more than half of the frozen
  eligible_total, the same subtraction form the strict-majority check
  uses (side_weight > W - side_weight), so at-or-below-half never
  qualifies. The 50% floor is a starting hypothesis to stress-test.
- If the quorum fails, the round voids instead: new TerminalCause::
  RevealQuorumNotMet (appended last so already-persisted XDR variant
  encodings stay stable), final_outcome stays None, and settlement pays
  the same bonds-back, no-forfeiture pool a stalled v1 dispute gets
  (drydocs#166's shape): every funded position, revealed or not, recovers its
  exact principal. No reward for withholding.
- RoundVoided event (revealed_weight + eligible_total carried) published
  instead of Resolved, a distinct event for the same reason
  RoundCancelled is distinct from Resolved.

Tests: 7 unit/behavior tests (quorum-met default survives, the attack
scenario voids, at-or-below-half boundary, exact economics of the
register-heavy/reveal-light attack, conservation) plus a 96-case
property test sweeping voter counts, stakes, reveal masks, and sides,
asserting the invariant trichotomy (strict majority / quorum-met
timeout / voided with bonds back) on every closed round.

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two things outside this diff, so I can't anchor them inline. resolve_outcome's doc comment (contracts/tholos-v2/src/lib.rs:1700) still says closing locks the outcome as strict majority if reached, OptimisticTimeout otherwise, with no mention of the new RevealQuorumNotMet/RoundVoided path this PR adds, an integrator relying on that comment to reason about possible return values won't anticipate it. docs/src/V2_RESOLUTION.md's state diagram and terminal-cause table have the same gap, still describing the old unconditional default-to-asserted-outcome timeout with no reveal-quorum gate. Please update both.

Separately: this PR adds 27 proptest-generated snapshot files under contracts/tholos-v2/test_snapshots/test/withheld_reveal_quorum/proptest_quorum/, and that path isn't in .gitignore even though three sibling proptest paths already are, with the comment that proptest snapshots are non-deterministic and must not be committed. Please add this path to .gitignore and drop these files from the PR.

…or quorum void

- resolve_outcome doc: mention RevealQuorumNotMet/RoundVoided path
- V2_RESOLUTION.md state diagram: add voided-round transition
- V2_RESOLUTION.md storage table: add RevealQuorumNotMet terminal cause
- V2_RESOLUTION.md decision summary: update bonds and outcome rule
@ZacLou

ZacLou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@collinsezedike Both doc gaps fixed in cb497e3:

  1. resolve_outcome doc comment (lib.rs:1700): now lists all three terminal causes — strict majority, OptimisticTimeout (majority revealed but no side won), and RevealQuorumNotMet (at or below half, round voided). Also mentions RoundVoided as the emitted event替代 Resolved in that case.

  2. V2_RESOLUTION.md: state diagram now has a Reveal --> Resolved transition for the quorum-void path (revealed weight <= half, #167). Storage table's terminal cause description includes RevealQuorumNotMet. Decision summary's bonds row and outcome-rule row both updated to describe the voided-round settlement.

@ZacLou

ZacLou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@collinsezedike Both doc updates pushed (cb497e3), CI all green (test ✅, demo ✅, sdk ✅):

  1. resolve_outcome doc comment — now lists all three terminal causes: strict majority, OptimisticTimeout (majority revealed, no side won), RevealQuorumNotMet (at or below half, round voided). Also mentions RoundVoided as the emitted event.

  2. V2_RESOLUTION.md — state diagram has a new Reveal --> Resolved transition for the voided path, storage table includes RevealQuorumNotMet, decision summary updated for bonds and outcome rules.

@collinsezedike

Copy link
Copy Markdown
Collaborator

@ZacLou Two things.

  1. The new proptest module's generated snapshot files got committed, about 99 files and 236k lines under contracts/tholos-v2/test_snapshots/test/withheld_reveal_quorum/proptest_quorum/. The repo's .gitignore already has a comment on a different proptest module explaining exactly why these can't be committed, they're non-deterministic and differ every run. Please add this path to .gitignore and remove the committed snapshots.
  2. settlement_pool's doc comment still says four terminal causes after this PR adds a fifth. The match arm and panic message below it already correctly say five.

… doc count

- Add withheld_reveal_quorum/proptest_quorum/ to .gitignore
- Remove 99 committed non-deterministic snapshot files
- settlement_pool doc comment: four -> five terminal causes
@ZacLou

ZacLou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@collinsezedike Both items addressed in the latest two commits (cb497e3 and e489c4f):

  1. Documentation gap closed: resolve_outcome doc comment and docs/src/V2_RESOLUTION.md now explicitly describe the RevealQuorumNotMet / RoundVoided path added by this PR.
  2. Proptest snapshot churn: Added contracts/tholos-v2/test_snapshots/test/withheld_reveal_quorum/proptest_quorum/ to .gitignore.

Ready for another look — thanks!

@ZacLou

ZacLou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Hi @collinsezedike, the review items should now be addressed:

  1. Documentation gapsresolve_outcome's doc comment now describes the three terminal paths (strict majority, quorum-met optimistic timeout, and RevealQuorumNotMet/round void). docs/src/V2_RESOLUTION.md state diagram and terminal-cause table were also updated in cb497e3.
  2. Proptest snapshotscontracts/tholos-v2/test_snapshots/test/withheld_reveal_quorum/proptest_quorum/ is now in .gitignore, and the committed snapshot files were removed in e489c4f.

All CI checks pass. Re-requesting your review — thanks again.

@ZacLou

ZacLou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Hi @collinsezedike, just a friendly follow-up: all the review feedback has been addressed and CI is fully green. Could you take another look when you have a moment? Thanks!

@ZacLou

ZacLou commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@collinsezedike The review feedback has been addressed in the latest commits:

  1. resolve_outcome doc comment — Updated to explicitly mention the RevealQuorumNotMet / RoundVoided path alongside OptimisticTimeout and strict majority (commit cb497e3).
  2. V2_RESOLUTION.md — State diagram and terminal-cause table updated to include the voided-round transition and RevealQuorumNotMet cause (commit cb497e3).
  3. Proptest snapshots — Added withheld_reveal_quorum/proptest_quorum/ to .gitignore and removed the 99 committed non-deterministic snapshot files (commit e489c4f).

Please re-review when you have a moment. Thanks!

@ZacLou

ZacLou commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@collinsezedike Both review items from your last pass should now be addressed in the latest push:

  1. Proptest snapshots: the contracts/tholos-v2/test_snapshots/test/withheld_reveal_quorum/proptest_quorum/ path is already in .gitignore, and no generated snapshot files remain tracked or in the working tree.
  2. settlement_pool doc comment: the comment now reads one of the five causes handled here, matching the RevealQuorumNotMet arm and the unreachable! panic message.

CI is green across the board (build, demo, sdk, test). Could you take another look when you have a moment?

@ZacLou

ZacLou commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@collinsezedike All three items from your review are addressed:

  1. resolve_outcome doc comment (contracts/tholos-v2/src/lib.rs): now explicitly documents the RevealQuorumNotMet / RoundVoided path added by [Bug] Withheld-reveal registration can deny a genuine majority in tholos-v2 #167.
  2. docs/src/V2_RESOLUTION.md: the state diagram and terminal-cause table now include the quorum-voided round path.
  3. .gitignore: contracts/tholos-v2/test_snapshots/test/withheld_reveal_quorum/proptest_quorum/ is now ignored, and the proptest snapshot files have been removed from the PR.

Ready for another pass — thanks!

@collinsezedike collinsezedike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this, both prior findings are fixed cleanly and the new test coverage is thorough. Merging now.

@collinsezedike
collinsezedike merged commit 73ed4c5 into drydocs:main Sep 6, 2026
4 checks passed
@collinsezedike

collinsezedike commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

@ZacLou If you have a moment, a star on the repo would be appreciated!

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.

[Bug] Withheld-reveal registration can deny a genuine majority in tholos-v2

2 participants