[Bug Report] Crossing fills are mis-sized — over-fills the first maker and zero-fills every maker after it; a SELL taker that rests keeps a stale `filled_quantity`

[OPTIONPOOL]

Found while integrating exchange 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. This is a reproducible snapshot of one commit, offered back; it's not a verdict on the project.

Two independent matching defects in OrderBook::process_order (crates/engine/src/engine/orderbook.rs) let a single incoming order trade far more (or far less) than it should. Both reproduce with a handful of orders.

Pinned at ed9f044 (ed9f044dc79ee713da9518648524e0c68a70ddf7). The matcher only needs rust_decimal / rust_decimal_macros / serde / uuid, so it can be exercised by copying engine/orderbook.rs + types/engine.rs into a small crate (or as a #[test] inside the engine crate) — none of the Redis/Postgres I/O layer is involved.

Bug 1 — each fill is sized as maker_total − taker_running, not min(maker_remaining, taker_remaining)

Each fill quantity is taken from the maker's total size minus the taker's running fill total, rather than from the smaller of the two orders' remaining quantities. Two consequences fall out of the same line:

1. Over-fill: the first maker a taker touches is filled to the maker's full size, ignoring how much the taker actually wanted — a qty-3 buy against a resting qty-79 ask reports a 79-lot trade and "executes" 79.
2. Under-fill / phantom fill: once executed_quantity > 0, every subsequent maker is sized as maker_total − executed_quantity, which is 0 once the running total reaches that maker's size (and negative if it exceeds it) — so a qty-10 buy across two qty-4 asks fills 4 then 0, emits a zero-quantity Fill, and stops with executed_quantity = 4 instead of 8.

match_asks (orderbook.rs:68-69; match_bids is the symmetric copy at :102-103):

let filled_quantity =
    std::cmp::min(ask.quantity - executed_quantity, ask.quantity);

Here ask.quantity is the maker's full original size and executed_quantity is the taker's running total. Because executed_quantity >= 0, the first operand ask.quantity - executed_quantity is always <= ask.quantity, so the min(.., ask.quantity) never binds — the expression reduces to ask.quantity - executed_quantity. That is neither the maker's remaining (ask.quantity - ask.filled_quantity) nor the taker's remaining (order.quantity - executed_quantity):

• First maker (executed_quantity == 0) ⇒ ask.quantity — the maker's whole size, with no regard for what the taker wanted ⇒ over-fill.
• Later makers ⇒ ask.quantity - executed_quantity ⇒ 0 once the taker's running total reaches that maker's size, negative beyond it ⇒ under-fill, phantom zero-qty Fill, and a backwards step in executed_quantity.

The executed_quantity < order.quantity guard at :67 bounds whether matching continues, but not the per-fill size, so it doesn't rescue either case.

Repro. Built via the engine's own Order / OrderBook (Order.filled_quantity starts at 0):

let mut b = OrderBook::new(asset_pair, 0);

// (1) over-fill: one maker bigger than the taker
b.process_order(sell(price=100, qty=79));            // rest an ask
let r = b.process_order(buy(price=100, qty=3));      // buy only 3
// actual:   r.executed_quantity == 79, r.fills == [Fill{ quantity: 79 }]
// expected: r.executed_quantity == 3,  r.fills == [Fill{ quantity: 3  }]

// (2) under-fill: two makers, taker spans both
let mut b = OrderBook::new(asset_pair, 0);
b.process_order(sell(price=100, qty=4));
b.process_order(sell(price=100, qty=4));
let r = b.process_order(buy(price=100, qty=10));
// actual:   r.executed_quantity == 4, fills == [4, 0]   (the 0 is a phantom trade with a fresh trade_id)
// expected: r.executed_quantity == 8, fills == [4, 4]

Fix. Size each fill from the two remaining quantities — the standard price-time fill:

// match_asks
let filled_quantity = std::cmp::min(
    ask.quantity - ask.filled_quantity,   // maker's remaining
    order.quantity - executed_quantity,   // taker's remaining
);
// match_bids: the same with `bid`

Order.filled_quantity already exists and is maintained on the maker side (ask.filled_quantity += filled_quantity), so this needs no new state. With it applied, the two cases above report 3 / [3] and 8 / [4,4]. (Confirmed compiling and resolving on a copy of the pinned source.)

Bug 2 — a SELL taker that partially fills then rests keeps a stale filled_quantity (the BUY arm doesn't)

process_order's two arms are asymmetric. The BUY arm records how much the taker already executed before resting the residual; the SELL arm omits that one line (orderbook.rs:36-58):

OrderSide::BUY => {
    order_result = self.match_asks(&order);
    order.filled_quantity = order_result.executed_quantity;   // :38  — records the fill
    if order_result.executed_quantity < order.quantity { /* rest in bids */ }
    ...
}
OrderSide::SELL => {
    order_result = self.match_bids(&order);
    // <-- no `order.filled_quantity = order_result.executed_quantity;`
    if order_result.executed_quantity < order.quantity { /* rest in asks */ }
    ...
}

So a SELL order that partially fills as a taker and then rests is stored in self.asks with filled_quantity = 0 — it advertises its full original size even though part of it has already traded. The symmetric BUY case stores the correct amount.

Repro. Rest a BUY 70 @ 100, then send a SELL 79 @ 100; it fills 70 and rests 9. Inspect the resting ask: filled_quantity == 0 (it should be 70). Run the mirror case — rest a SELL 70 @ 100, send a BUY 79 @ 100 — and the resting bid correctly shows filled_quantity == 70. The two arms disagree for identical flows.

This is independent of Bug 1 but compounds with its fix: once fills are sized from ask.quantity - ask.filled_quantity, a resting SELL whose filled_quantity is wrongly 0 reports its full original size as available, so the next aggressor over-fills it (e.g. trades 79 against a resting ask that has only 9 left).

Fix. Mirror the BUY arm — add the missing assignment to the SELL arm:

OrderSide::SELL => {
    order_result = self.match_bids(&order);
    order.filled_quantity = order_result.executed_quantity;   // mirror the BUY arm
    if order_result.executed_quantity < order.quantity { /* rest in asks */ }
    ...
}

With both fixes applied, the symmetric residuals both report filled_quantity == 70.

---

Both are a time-stamped observation against ed9f044, offered back rather than aimed at anyone — happy to share the standalone repro. Thanks for building and sharing the exchange.

Respectfully submitted.