Skip to content

docs: add full contract reference for tholos-v2 - #104

Open
Theophilus131 wants to merge 1 commit into
drydocs:mainfrom
Theophilus131:docs/tholos-v2-contract-reference
Open

docs: add full contract reference for tholos-v2#104
Theophilus131 wants to merge 1 commit into
drydocs:mainfrom
Theophilus131:docs/tholos-v2-contract-reference

Conversation

@Theophilus131

Copy link
Copy Markdown
close #99 

What

Adds docs/src/CONTRACT_V2.md, a complete reference for contracts/tholos-v2's
public interface — types, every function, the full Error enum, the event
table, security notes, and persistent-storage TTL behavior — in the same
structure CONTRACT.md already uses for v1.

Why

v1 has full reference docs; v2 only had a lightweight lifecycle table in
INTEGRATION.md (signatures + one-liners, no error variants, no event
table, no security/TTL notes) even though v2's lifecycle
(assert_outcomewithdraw, plus set_paused_v2/cancel_round) is now
fully implemented. This closes that documentation gap.

Structure choice

Went with a separate CONTRACT_V2.md (option 1 in the issue) rather than
extending CONTRACT.md with a v2 section — v2's interface is large enough
(11 functions, 36 errors, 12 events) that folding it into the existing
296-line v1 doc would make both harder to scan. Registered under
docs/src/SUMMARY.md as a nested entry under the existing v1 "Contract
Reference" item.

Verification

Every function signature, error variant, event field, and behavioral claim
was checked line-by-line against contracts/tholos-v2/src/lib.rs (not
just copied from doc comments) while writing this. In particular:

  • get_resolution/get_position both surface AssertionNotFound for a
    missing record (not a dedicated error), matched from the source.
  • settle and withdraw fetch AssertionV2 before Resolution and use
    NotResolved/NoCreditToWithdraw specifically for the
    UncontestedFinalize case, which never has a Resolution — documented
    as such rather than assumed.
  • The PhaseV2::Reveal / terminal_cause distinction (a majority can
    lock while phase stays Reveal) is called out explicitly, since it's
    easy to get wrong from the type alone.

Checklist

  • docs/src/CONTRACT_V2.md added, same structure as CONTRACT.md
  • docs/src/SUMMARY.md updated so mdbook build includes the new page
  • Cross-checked against contracts/tholos-v2/src/lib.rs, not restated
    from doc comments

Documents contracts/tholos-v2's public interface in docs/src/CONTRACT_V2.md, matching the structure and level of detail CONTRACT.md already gives v1: lifecycle diagram, every type, every public function with signer/return/error detail, the full Error enum, the event table, security notes, and persistent-storage TTL behavior. Every claim was cross-checked against contracts/tholos-v2/src/lib.rs as written, not restated from doc comments verbatim. Registers the new page in docs/src/SUMMARY.md so mdbook build picks it up. Fixes #<issue-number>

@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.

One general note not tied to a single line: INTEGRATION.md's existing "Lifecycle at a glance" section fully duplicates this doc's lifecycle/function content, with no cross-reference either way. Worth linking one to the other so they don't silently drift.

Comment thread docs/src/CONTRACT_V2.md
[*] --> Pending: assert_outcome
Pending --> Resolved: finalize<br/>(challenge window elapsed,<br/>uncontested)
Pending --> Registration: dispute
Registration --> Reveal: register deadline passes<br/>(lazily, on next register/reveal/resolve_outcome)

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.

The diagram lists register as a trigger for the lazy Registration → Reveal transition, but register never calls open_reveal_phase. After the deadline it just returns Error::RegistrationClosed (see test_register_after_deadline_fails). Only reveal and resolve_outcome open reveal phase.

Comment thread docs/src/CONTRACT_V2.md
Pending --> Resolved: finalize<br/>(challenge window elapsed,<br/>uncontested)
Pending --> Registration: dispute
Registration --> Reveal: register deadline passes<br/>(lazily, on next register/reveal/resolve_outcome)
Reveal --> Resolved: resolve_outcome<br/>(strict majority locked, or<br/>reveal deadline / full reveal reached)

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.

This edge is mislabeled two ways. "Strict majority locked" doesn't by itself cause the phase transition, your own prose right below this diagram says phase stays Reveal after a majority locks. And reveal itself can also close Reveal → Resolved in the same call when it's the last outstanding weight, not just resolve_outcome.

Comment thread docs/src/CONTRACT_V2.md
| `StrictMajorityFor` | Revealed weight agreeing with the asserted outcome exceeded half of the frozen eligible total `W`. |
| `StrictMajorityAgainst` | Revealed weight disagreeing exceeded half of `W`. |
| `OptimisticTimeout` | Neither side reached a strict majority before reveal closed; the originally asserted outcome stands by default. |
| `AdminCancelled` | Set only by `cancel_round`, on a `Registration`/`Reveal`-phase assertion with no terminal cause yet. Every funded position recovers its exact principal, no forfeiture, no reward. |

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.

This row says AdminCancelled only applies to a Registration/Reveal-phase assertion, but cancel_round also handles a still-Pending assertion (refunding the asserter's bond directly). Your own Functions section for cancel_round describes that Pending path correctly, this table row contradicts it.

Comment thread docs/src/CONTRACT_V2.md
| --- | --- |
| `AlreadyInitialized` | `initialize` called on a contract that's already set up. |
| `NotInitialized` | Called before `initialize`. |
| `AssertionNotFound` | No assertion (or, depending on call, resolution/position/credit) exists for the given id/address. |

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.

AssertionNotFound doesn't cover a missing credit. get_credit returns 0 via unwrap_or(0), and withdraw returns NoCreditToWithdraw for a zero balance, not AssertionNotFound. Drop "credit" from this row.

Comment thread docs/src/CONTRACT_V2.md
| `NotInitialized` | Called before `initialize`. |
| `AssertionNotFound` | No assertion (or, depending on call, resolution/position/credit) exists for the given id/address. |
| `InvalidBondAmount` | `base_bond` isn't positive, or exceeds `MAX_BOND_AMOUNT`. |
| `InvalidRegistrationDuration` | `registration_duration_secs` is zero or exceeds 7 days. |

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.

These four "7 days" references (here, InvalidRevealDuration, InvalidChallengeWindow, and the initialize prose) point at three separately-defined constants in lib.rs that just happen to share a value today. Worth naming them individually, like the TTL section does with INSTANCE_BUMP_AMOUNT, so a future change to just one doesn't leave the others silently stale.

Comment thread docs/src/CONTRACT_V2.md
tokens (`assert_outcome`, `finalize`, `dispute`, `register`, `withdraw`)
also holds a contract-wide reentrancy mutex (`ReentrancyGuard`) for the
duration of the transfer, via `enter_reentrancy_guard`/
`exit_reentrancy_guard`. `reveal`, `resolve_outcome`, `settle`, and

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.

This says cancel_round doesn't move tokens, but it does, it refunds the asserter's bond directly (same acquire/transfer/release pattern as withdraw) when the assertion is still Pending. It shouldn't be grouped with reveal/resolve_outcome/settle here.

Comment thread docs/src/SUMMARY.md
@@ -1,4 +1,4 @@
# Summary
# Summary

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.

This PR's diff shows this line changing even though nothing here should differ, it's a UTF-8 BOM getting silently added to the file start. Probably an editor artifact. Worth stripping before merge, it's noise unrelated to the PR's purpose.

@collinsezedike

Copy link
Copy Markdown
Collaborator

Hey @Theophilus131, just checking in on this one, still no rush. There's a review with a few concrete mismatches to fix, let me know if anything's unclear or if you'd like a hand.

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