fix: honor post-only on LIMIT_MAKER amends; processUpdate matched as a taker (#92)#104
Merged
Merged
Conversation
…a taker (#92) Engine.processUpdate is cancel-and-replace and used to route EVERY replacement through the MATCHING processLimitOrder path, never reading cmd.getOrderType(). So an amended LIMIT_MAKER that now crosses executed as a taker, silently stripping the post-only guarantee (SbeDemuxer.handleUpdateOrder already decodes and sets the order type). processUpdate now honors LIMIT_MAKER, mirroring processCreate: - A pre-cancel would-cross check (shared wouldCrossOpposite helper). If the amended price would cross, publish REJECTED for the OLD orderId/omsOrderId and return, so the old order stays resting and tradable. Exact for the single- threaded engine: the old order rests on the same side, so cancelling it cannot move the opposite best the check reads. - Otherwise cancel + CANCELLED as before, then rest the replacement via the NO-MATCH addOrderNoMatch path (never processLimitOrder). Could-not-rest (LEVEL_FULL/BOOK_FULL) is a loud terminal REJECTED (old order already gone); success publishes NEW for the full new quantity. Plain LIMIT amends are byte-identical (verified: update_price_then_match golden 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.
Mechanism
Engine.processUpdateis cancel-and-replace and unconditionally routed EVERY replacement through the matchingengine.processLimitOrder(...)path, never readingcmd.getOrderType().SbeDemuxer.handleUpdateOrderalready decodes and sets the order type on the command, but the engine ignored it. So an amendedLIMIT_MAKERwhose new price now crosses the spread executed as a taker, silently stripping the post-only guarantee (e.g. resting ask @60000; post-only bid @59000 amended to 61000 filled 5 @60000 and went FILLED, with no rejection).Fix
processUpdatenow honorsLIMIT_MAKER, mirroringprocessCreate:wouldCrossOppositehelper, extracted so create and amend decide crossing with the exact same expression). It runs AFTER the existing price-validity and match#91 quantity checks (both still fire first) and BEFORE the cancel. If the amended price would cross, it publishesREJECTEDfor the OLD orderId/omsOrderId and returns, leaving the old order resting and tradable. This is exact for the single-threaded engine: the old order rests on the same side as the replacement, so cancelling it cannot move the opposite best the check reads.CANCELLEDas before, then rest the replacement via the no-matchaddOrderNoMatchpath (neverprocessLimitOrder). A could-not-rest result (LEVEL_FULL/BOOK_FULL) is a loud terminalREJECTED(the old order is already gone; FIFO position is forfeit on any amend). Success publishesNEWfor the full new quantity.LIMITamend path is byte-identical (matching cancel-and-replace).Coupling to oms#67
This bug is masked today by the OMS amend bug (oms#67) and is unmasked the moment the OMS amend fix ships, so this must be active on the cluster first. Ship this before oms#67 goes live.
Tests
EngineTest(4 new):LIMIT_MAKERamend (59000 to 61000 with ask @60000) publishesREJECTED, the old bid @59000 stays resting, and a laterMARKETsell fills against it at 59000.CANCELLED(old) +NEW(new), zero trades, replacement rests at 59500.LEVEL_FULLon the direct engine's 64-orders/level cap produces a loudREJECTED, not a phantomNEW. (The array engine cannot fail a same-side amend, since the cancel frees the slot the re-add reclaims, so this terminal branch is exercised on the direct impl.)LIMITamend regression: a crossing plain amend still matches and fills (proves the fix touchesLIMIT_MAKERonly).limit_maker_update.scenario+ golden covering both branches (crossing amend REJECTED with the old order surviving and later filled as maker; non-crossing amend CANCELLED+NEW). A/B convergent (DeterminismAbEquivalenceTest), and no existing golden changed (verified withgit diff, includingupdate_price_then_match).ScenarioRunner.buildUpdategained optionaltype=support (defaultLIMIT= prior behavior), following how CREATE lines express types.EmbeddedClusterTest/ArchiveHousekeepingTest.Closes #92
🤖 Generated with Claude Code