Skip to content

[Bug Report] Fully-filled maker lingers in order_id_map; a later cancel/amend of it crashes on an assertion #18

Description

@OPTIONPOOL

Found while running LightMatchingEngine through an open-source matching-engine benchmark, the Matching Engine Performance Challenge — it cross-checks engines against the byte-identical consensus of other open source engines. A typical random workload never hits this (it cancels live resting orders), so the bug stays latent there; it surfaces the moment a cancel or amend arrives for an order that has already been fully executed, and when it does the engine doesn't reject — it aborts with an AssertionError.

When a resting order is fully filled as the passive side during matching, add_order splices it out of the price-level depth but never removes it from order_book.order_id_map. The map entry lingers, so the dead order looks live to cancel_order / amend_order: both gate on order_id_map membership, find the stale entry, skip their "invalid order id → return None" guard, and run on into an invariant assertion that no longer holds (its price level was deleted when the level emptied).

The passive-fill loop deletes the consumed maker from the depth list but not from the id map (only cancel_order ever does the latter):

# lightmatchingengine/lightmatchingengine.pyx  (add_order, buy side; sell side is symmetric)
if hit_order.leaves_qty < 1e-9:
    del order_book.asks[best_price][0]      # off the book...
                                            # ...but order_id_map[hit_order.order_id] stays

So cancel_order passes its if order_id not in order_book.order_id_map: return None check, fetches the filled order, and then trips assert order_price in order_book.asks.keys() — the level was removed when it emptied — raising AssertionError: Order price ... is not in the ask price depth. amend_order hits it one step earlier on the same stale entry: it asserts amended_qty - order.cum_qty >= 1e-9, but a fully-filled maker has cum_qty == qty, so any sane amend-down raises AssertionError: The amended qty (...) cannot be amended below the cum qty (...). Either way a stale request that should be a clean reject takes the process down.

Repro. On an empty book:

  1. add_order("X", 100, 10, SELL) — rests as order id 1.
  2. add_order("X", 100, 10, BUY) — fully fills id 1 (one trade, qty 10); id 1 is now off the book but still in order_id_map.
  3. cancel_order(1, "X") → AssertionError: Order price 100.000000 is not in the ask price depth
    (or amend_order(1, "X", 101, 5) → AssertionError: The amended qty (5.0) cannot be amended below the cum qty (10.0)).

Expected: cancel_order / amend_order return None for a no-longer-resting order (the same way they already do for an unknown id). Actual: assertion failure. A partially filled order behaves correctly — its residual is still in order_id_map and on the book, so the cancel/amend is legitimately live; the bug is specific to the fully-consumed maker whose entry is never cleaned up.

Fix. Drop the maker from order_id_map at the point it leaves the depth, mirroring what cancel_order already does. One line in each side of the match loop:

if hit_order.leaves_qty < 1e-9:
    del order_book.order_id_map[hit_order.order_id]   # add this line (both buy and sell sides)
    del order_book.asks[best_price][0]                # ...asks here / bids on the sell side

With that, the stale entry is gone, the not in order_id_map guard fires, and a cancel/amend of a fully-filled order returns None instead of asserting. Happy to send a PR and share the failing edge-case workload.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions