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.

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