[Bug Report] Matching computes trades but never applies them: filled orders are never removed from the book

[OPTIONPOOL]

Found while integrating order-book-haskell into an open matching-engine benchmark, the Matching Engine Performance Challenge — it cross-checks engines against the byte-identical consensus of other open-source engines. While driving the engine's Core.* API directly, I noticed that orders which fully cross stay resting and can match again, so I wanted to pass along a small reproducible note and a suggested fix.

Pinned at main (4167845f124318ca595f820b74c53769baaa4ef7); GHC 9.4.8 with the stm / containers / time boot packages, driven through Core.Engine.placeOrder / Core.Matching.matchPriceTime (the same API the test suite uses).

placeOrder runs the matcher and returns the matched Trades, but it never writes the reduced book back: the matched orders are left resting at their original quantities. After a full cross both sides remain in the book (so best_bid == best_ask, a locked book), and running the matcher again on the same book re-produces the same trade. Quantity is effectively created out of thin air on every subsequent match.

Repro. Using only the engine's own API (src/Core/):

import Core.Types; import Core.Engine; import Core.Matching
import Control.Concurrent.STM; import Data.Time.Clock; import Data.Time.Calendar (fromGregorian)

t0 = UTCTime (fromGregorian 2025 1 1) 0
t1 = UTCTime (fromGregorian 2025 1 1) 1

main = do
  eng <- atomically (newEngine ["BTC"])
  _ <- atomically (placeOrder eng (Order 1 LimitBuy  "BTC" 100 10 t0))  -- rest a bid
  _ <- atomically (placeOrder eng (Order 2 LimitSell "BTC" 100 10 t1))  -- fully crosses it
  bids <- atomically (readTVar (getBids (orderBook eng) "BTC"))
  asks <- atomically (readTVar (getAsks (orderBook eng) "BTC"))
  print (length bids, length asks)                                       -- expect (0,0)
  again <- atomically (matchPriceTime "BTC" (orderBook eng) t1)
  print (length again)                                                   -- expect 0

Actual output at 4167845:

(1,1)     -- both orders still resting after a full 10-vs-10 cross
1         -- the same trade is produced again

Expected: (0,0) and 0 — a fully matched pair should leave the book empty, and a second match should find nothing.

Mechanism / root cause. placeOrder matches and then drops the result (src/Core/Engine.hs:45-47):

            trades <- matchPriceTime (asset order) book (timestamp order)
            processMatches trades
            return (orderId order)

processMatches is a no-op stub (src/Core/Engine.hs:61-63):

processMatches :: [Trade] -> STM ()
processMatches trades = do
    return ()

and matchPriceTime only reads the two TVars — it never writes a reduced book back (src/Core/Matching.hs:39-47):

matchPriceTime asset book now = do
    bids <- readTVar (getBids book asset)
    asks <- readTVar (getAsks book asset)
    let sortedBids = sortOrders True bids
        sortedAsks = sortOrders False asks
    matchOrders now sortedBids sortedAsks

matchOrders (src/Core/Matching.hs:64-77) returns only [Trade]; the unconsumed remainder it carries in its recursion is thrown away. createTrade does compute the correct leftover quantities (remainingBid / remainingAsk), but nothing persists them, so the book never shrinks. The net effect is that placeOrder is append-only: liquidity is never consumed, the book grows without bound, and any crossing region is re-matched on every later order.

Fix. Return the unconsumed remainder from matching and write it back to the book. A minimal version that reuses the existing createTrade logic (verified to compile and resolve the repro at 4167845):

-- Core/Matching.hs — like matchOrders, but also returns the leftover book.
matchOrdersR :: UTCTime -> [Order] -> [Order] -> ([Trade], [Order], [Order])
matchOrdersR _ [] asks = ([], [], asks)
matchOrdersR _ bids [] = ([], bids, [])
matchOrdersR now (bid:restBids) (ask:restAsks)
  | canMatch bid ask =
      let (trade, remBid, remAsk) = createTrade bid ask now
          (more, lb, la) = case (remBid, remAsk) of
              (Just b, Just a)   -> matchOrdersR now (b:restBids) (a:restAsks)
              (Just b, Nothing)  -> matchOrdersR now (b:restBids) restAsks
              (Nothing, Just a)  -> matchOrdersR now restBids (a:restAsks)
              (Nothing, Nothing) -> matchOrdersR now restBids restAsks
      in (trade : more, lb, la)
  | otherwise = ([], bid:restBids, ask:restAsks)

matchPriceTimePersist :: Asset -> OrderBook -> UTCTime -> STM [Trade]
matchPriceTimePersist asset book now = do
    let bidsV = getBids book asset
        asksV = getAsks book asset
    bids <- readTVar bidsV
    asks <- readTVar asksV
    let (trades, lb, la) = matchOrdersR now (sortOrders True bids)
                                            (sortOrders False asks)
    writeTVar bidsV lb
    writeTVar asksV la
    return trades

and have placeOrder call matchPriceTimePersist instead of matchPriceTime + processMatches (src/Core/Engine.hs:45-46). With this change the repro prints (0,0) then 0, and a partial cross (sell 10, then buy 4) correctly leaves a resting ask of quantity 6.

---

This is just a time-stamped snapshot of 4167845 offered back — thanks very much for sharing the project; the price-time matching core itself reads cleanly, and persisting its result looked like a small, self-contained change. Please feel free to adjust the approach to fit the design.

Respectfully submitted.