fix: terminate taker at the match cap instead of resting a crossed remainder (#93)#106
Merged
Conversation
…mainder (#93) MAX_MATCHES_PER_ORDER (=10_000) bounds per-command work on the single consensus thread, but when it fired mid-sweep both engines treated the leftover taker quantity as "ran out of liquidity". A LIMIT remainder then RESTED on the book even though crossing liquidity existed at unreached levels (a crossed book: a bid resting above unreached asks), and a capped MARKET order fell into the else branch and was mis-reported FILLED, hiding a partial fill from OMS. Terminate at the cap instead. Both matching-engine impls now carry a matchLimitReached flag (reset per order, set at the cap early-return sites, breaks the per-level loop). The LIMIT path surfaces the truncation via the existing rest-reject mechanism (lastRestRejectReason = MATCH_LIMIT) so Engine publishes a terminal CANCELLED with the true filled quantity through its existing "restReason != NONE" branch, with no new Engine branches. The MARKET path publishes CANCELLED with the true filled quantity when wasMatchLimitReached() (trusting the explicit flag, not a leftover-budget heuristic). An order that fully fills in exactly the cap's matches does not set the flag and stays FILLED. The prod cap stays a hardcoded deterministic constant (never an env var: a divergent cap between replicas would fork the state machine). A package-private constructor overload injects a small cap so the termination path is unit-testable with a handful of orders. New OrderRejectReason.MATCH_LIMIT = 7 (codes 0-6 unchanged). Tests (new MatchLimitCapTest, 17 cases across both impls): capped LIMIT terminates with the remainder not rested and the book not crossed; capped MARKET buy and sell report CANCELLED with the true filled qty; exact-cap full fill stays FILLED (flag not set); array/direct A/B equivalence at the small cap. Full match-cluster suite green (433) minus embedded infra; determinism corpus goldens unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MAX_MATCHES_PER_ORDER(=10_000, in bothArrayMatchingEngineandDirectMatchingEngine) caps the number of matches a single command may generate, bounding per-command work on the single consensus thread. When that cap fired mid-sweep, both enginesreturned early leavingtakerRemaining > 0, and back inEngine.processCreate/processUpdatethat leftover was treated as "ran out of liquidity":processCreateonly branchedmatchCount == 0(REJECTED) vselse(FILLED), so a market order capped mid-sweep was reported FILLED, hiding a partial fill from OMS.Fix: terminate at the cap
The cap stays 10_000; it exists to bound per-command work, so the right behavior is to stop the taker at the cap, never to rest a crossed remainder or hide a partial.
matchLimitReachedflag, reset at the start of each order, SET at the cap early-return sites (only reachable with taker quantity still remaining, since the loop guard proved it), and it breaks the outer per-level loop. Exposed aswasMatchLimitReached()on theMatchingEngineinterface.lastRestRejectReason = OrderRejectReason.MATCH_LIMIT. Engine's existingrestReason != NONEbranch then publishes the loud terminal status (CANCELLED with the truefilledQtywhenmatchCount > 0) with zero new Engine branches, for bothprocessCreateandprocessUpdate.matchCount > 0 && engine.wasMatchLimitReached()publishes CANCELLED with the true filled quantity. We trust the explicit flag, not a leftover-budget heuristic (the budget/price-precision break also leavesbudget > 0).matchCount == 0with the flag set is impossible (cap >= 1).OrderRejectReason.MATCH_LIMIT = 7(+describe). Codes 0-6 are unchanged (INVALID_QUANTITY=6 from fix: reject quantity<=0 at order admission — silent loss + LIMIT_MAKER poison pill (#91) #103 kept its number).Why terminate, not continue in chunks?
The cap is a determinism/liveness guard on the consensus thread. Continuing the sweep across multiple commands (or looping past the cap) would either defeat the bound or require carrying taker state across log records, adding a re-entrancy surface to the replicated state machine for a case that only arises with 10_000+ resting orders swept by one order. Terminating at the cap keeps the state machine a pure per-command function and gives OMS a truthful terminal status (CANCELLED + true filled) that it already knows how to reconcile.
Determinism
Pure state-machine change. The prod cap remains a hardcoded deterministic constant (never an env var: a divergent cap between replicas would fork the state machine). Triggering it at the prod cap needs 10_000+ command lines, so per the agreed approach the termination path is covered at unit level via a package-private constructor overload that injects a small cap (=4); the committed determinism corpus goldens are unchanged (verified with
git diffafter running the corpus).Tests
New
MatchLimitCapTest(17 cases, exercised against both the array and direct impls, injected cap=4):filledQty = 4.bestBid < bestAskwhen both sides non-empty) applied throughout.Full match-cluster suite green (433 tests, excluding EmbeddedClusterTest / ArchiveHousekeepingTest which need the live stack); determinism corpus goldens unchanged.
Closes #93
🤖 Generated with Claude Code