[Bug Report] Repricing one order at a price level silently drops the *other* orders resting at that level

[OPTIONPOOL]

Found while integrating limitorderbook 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 deciding how to map the benchmark's modify operation onto the library, I found that modifyOrder on an order that shares its price level with at least one other order removes that order's level from the price tree, so the sibling orders at that level stop matching even though they are still in the book.

Pinned at master (8179bedc3519f7d3fcca157e36c78c6ab4b4545f).

Environment

• Commit: 8179bedc3519f7d3fcca157e36c78c6ab4b4545f
• Package: lob.js (TypeScript); the matcher is OrderBook in src/model/OrderBook.ts, backed by OrderTree/OrderList
• Toolchain: Node ≥ 20, TypeScript; pnpm install then pnpm test (vitest) or pnpm build (tsup)

What happens

When two or more orders rest at the same price and you modifyOrder one of them to a new price, the engine deletes the whole original price level from the tree — including the orders you did not touch. Those siblings remain in the id index (getOrder still returns them, getOrderCount still counts them, the side's running volume still includes them), but they have vanished from the price tree: they no longer appear in getBestBid/getBestAsk or getVolumeAtPrice, and an incoming order that should match them trades nothing. A modify that only changes quantity, or that reprices an order that is alone at its level, is unaffected — the problem is specific to repricing an order that shares its level.

Minimal reproduction

A small script against the library's own OrderBook (the same import the test suite uses, import OrderBook from "../model/OrderBook.js"):

import OrderBook from "../model/OrderBook.js";

const book = new OrderBook({ enableEvents: false });

// Rest two bids at the SAME price level, 100.
const a = book.processOrder({ type: "limit", side: "bid", price: 100, quantity: 10 });
const b = book.processOrder({ type: "limit", side: "bid", price: 100, quantity: 10 });
const aId = a.orderInBook!.orderId;
const bId = b.orderInBook!.orderId;            // the second bid — meant to keep resting at 100

// Reprice the FIRST bid down to 99 (it currently shares level 100 with the second bid).
book.modifyOrder(aId, { type: "limit", side: "bid", price: 99, quantity: 10, orderId: aId, time: Date.now() });

console.log(book.getBestBid());                 // 99    — expected 100 (the second bid still rests there)
console.log(book.getVolumeAtPrice("bid", 100)); // 0     — expected 10
console.log(book.getOrder(bId) !== null);       // true  — the order is still in the id index…

// …but it can no longer be matched: a crossing sell that should fill it does nothing.
const sell = book.processOrder({ type: "limit", side: "ask", price: 100, quantity: 10 });
console.log(sell.trades.length);                // 0     — expected 1 (a fill of 10 @ 100)

Actual output:

99
0
true
0

Expected: getBestBid() → 100, getVolumeAtPrice("bid", 100) → 10, and the crossing sell produces one trade of 10 @ 100. With a single resting bid at 100 (no sibling) the same reprice behaves correctly, which is what isolates the cause to the shared-level case.

Mechanism / root cause

In OrderTree.updateOrder, the reprice branch removes the order from its price level's list directly and then calls insertOrder (src/model/OrderTree.ts:94-102):

if (orderUpdate.price !== order.price) {
    const orderList = this.priceMap.get(order.price);
    if (orderList) {
        orderList.removeOrder(order);            // (1) first removal from the level's list
        if (orderList.length === 0) {
            this.removePrice(order.price);
        }
    }
    this.insertOrder(orderUpdate);               // (2) ...but the order is still in orderMap
} else {
    order.updateQuantity(orderUpdate.quantity, orderUpdate.time);
}

The order is taken out of the OrderList at (1), but it is not removed from orderMap. So when insertOrder runs at (2), its own dedup guard sees the id is still mapped and removes it a second time (src/model/OrderTree.ts:66-69):

insertOrder(quote: Quote) {
    if (this.orderExists(quote)) {
        this.removeOrderById(quote.orderId);     // removeOrderById -> OrderList.removeOrder(order) again
    }
    ...

OrderList.removeOrder decrements the list's length each time it is called (src/model/OrderList.ts:43-46), so this second removal drops length to 0 while a sibling order is still physically in the list. removeOrderById then sees order.orderList.length === 0 and calls removePrice(order.price) (src/model/OrderTree.ts:117-121), deleting a price level that still holds other orders. Those siblings are now orphaned: still in orderMap (and in the side's volume total), but gone from priceMap, so they no longer match or show up in depth/BBO.

In short, the reprice path removes the order from its level's list once itself and once again inside insertOrder, and the double-decremented length makes a non-empty level look empty.

Suggested fix

Remove the order once, through the existing removeOrderById (which takes it out of the list, the id index, and the running volume together, and only deletes the level when it is genuinely empty). That also clears the orderMap entry, so insertOrder no longer triggers a second removal:

 		if (orderUpdate.price !== order.price) {
-			const orderList = this.priceMap.get(order.price);
-			if (orderList) {
-				orderList.removeOrder(order);
-				if (orderList.length === 0) {
-					this.removePrice(order.price);
-				}
-			}
-			this.insertOrder(orderUpdate);
+			this.removeOrderById(order.orderId);
+			this.insertOrder(orderUpdate);
 		} else {
 			order.updateQuantity(orderUpdate.quantity, orderUpdate.time);
 		}

The trailing this.volume += order.quantity - originalQuantity still balances out: in the reprice branch the order object's quantity is unchanged, so it adds 0, while removeOrderById (−old qty) and insertOrder (+new qty) keep the side volume correct; the same-price branch is untouched. I applied this to a copy at the pinned SHA and re-ran: the shared-level reprice above now leaves the sibling resting (getBestBid() → 100, getVolumeAtPrice("bid",100) → 10, the crossing sell fills 10 @ 100), and a same-price quantity change and a lone-order reprice both behave exactly as before.

---

This is just a time-stamped snapshot of one commit, offered back in case it is useful — not a comment on the project, which was a pleasure to read. Thanks for sharing limitorderbook; happy to send the standalone reproduction or the patched copy if that would help.

Respectfully submitted.