Replace ledger state machine with OCaml implementation - #3
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
|
cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
|
cursor review |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
cursor review |
|
@macroscope-app review |
|
Manual reviews triggered for commit All prior checks · these links stay valid even if you push more commits. |
|
Review started! Results will be posted as check runs when complete. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review Unable to check for correctness in f2869b0. This PR introduces a complete new OCaml ledger state machine implementation (~1000+ lines of core logic), which constitutes a significant new feature requiring human review. Additionally, there are unresolved HIGH severity findings regarding balancing transfer sentinel handling and timeout overflow checking that should be addressed. You can customize Macroscope's approvability policy. Learn more. |
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Review started! Results will be posted as check runs when complete. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b10615d2d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
|
✅ Action performedReview finished.
✅ Action performedReview finished.
|
|
Review started! Results will be posted as check runs when complete. |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2869b0b28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | None -> | ||
| replace_state state trial; | ||
| results | ||
| | Some failure -> |
There was a problem hiding this comment.
When a linked transfer chain fails, the state mutations from trial must roll back, but transient failures still consume the failing transfer ID; the pinned TigerBeetle tests cover this with exceeds_debits/exceeds_credits inside linked chains and expect a later retry to return id_already_failed. This branch discards the cloned failed_transfers table along with the balances, so a transfer ID that hit Transfer_debit_account_not_found, Transfer_exceeds_debits, etc. inside a failed linked chain can be reused successfully.
Useful? React with 👍 / 👎.
| then Account_linked_event_chain_open | ||
| else Account_linked_event_failed) | ||
| }) |
There was a problem hiding this comment.
For a trailing open linked chain, this branch never executes the requests, so an earlier real failure in that open suffix is hidden as *_linked_event_failed. The pinned corpus has an open chain where a middle account fails with exists_with_different_flags and only the final event reports linked_event_chain_open; with the current shortcut, clients lose the actual validation result and any required transient-failure bookkeeping for transfer chains.
Useful? React with 👍 / 👎.
| | Error `Out_of_range -> error Account_imported_timestamp_out_of_range | ||
| | Error `Regressed -> error Account_imported_timestamp_must_not_regress | ||
| | Ok timestamp -> | ||
| let account = { request with timestamp } in |
There was a problem hiding this comment.
This stores request.flags unchanged, so accounts created with linked or imported keep those event-only bits in lookups and future idempotency comparisons. TigerBeetle clears LNK/IMP on stored records (for example, linked/imported account lookups in the pinned tests show no such flags), so retrying the same account without those request-only flags can incorrectly return Account_exists_with_different_flags; transfers have the same issue at their storage site.
Useful? React with 👍 / 👎.
1 participant
Stacked on #2.
Adds the OCaml ledger state machine and its interface, property and unit tests, workflow changes, documentation, and paired Zig/OCaml benchmark material. It also carries the README cleanup that is unrelated to the copied TigerBeetle source.
The final tree is unchanged from the original PR before this split.
Note
Replace Zig ledger state machine with a full OCaml reimplementation
tigerbeetle_ocaml.state_machinedune library, including a U128 module for 128-bit arithmetic with overflow/underflow signaling.ocam/directory and wrap dune commands withopam exec --.Macroscope summarized f2869b0.