[Suggestion] `Market::match()` silently discards any order that doesn't fully cross within the same call — worth documenting on the public `Market`/`OrderPending` API

[OPTIONPOOL]

Found while integrating E2Quant/e2q 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.

To be upfront: I don't think this is a bug in the "wrong output" sense, and I'm not asking for the matching behavior to change — the source itself gives good evidence the discard-if-not-immediately-fillable behavior is deliberate (see "Why this looks intentional" below). The problem is that nothing at the Market/OrderPending API level (the headers a caller actually reads) says so, while the project's own top-level README describes the OMS's quoting as a "Price/Time" algorithm doing "订单撮合" (order matching) — language that, read alone, suggests the usual continuously-resting order book, where a passive order waits until some future counter-order crosses it. Wiring Market up that way (insert, then attempt-match, once per incoming order — a natural way to drive the public API) makes essentially every resting order vanish within one call of arriving, with no trade and, in one of the two paths below, no report at all. A short doc comment on Market::match() (or a caveat on the README's "Price/Time" bullet) would save a future integrator the same detour into the source this took.

Pinned at current main (4d82493d726f870663cb20c0bc1559609000d849).

Environment

• Commit: 4d82493d726f870663cb20c0bc1559609000d849
• g++ (C++20), aarch64
• Driven through the engine's own public API only: e2q::Market::insert/match, e2q::OrderItem/OrderPending (include/OMSPack/OrderBook/Market.hpp, include/OMSPack/OrderBook/Order.hpp, src/OMSPack/OrderBook/Market.cpp)
• These three files don't compile standalone against a few of the repo's other headers (unrelated PostgreSQL/Kafka/scripting-VM #includes several hops away in foreign.hpp aggregator headers, and two include trees from the companion E2Quant/e2 language repo rather than this one — assembler/, E2L/); I compiled against a copy with those aggregator branches trimmed and the e2-repo paths stubbed to their minimal used surface. Market.hpp/Market.cpp/Order.hpp themselves are untouched — happy to share the stub headers and trimmed aggregator diff if useful.
• The reproduction below also needs the project's own prerequisites on the include path: the quickfix headers (for FIX::SessionID) and the E2Quant/e2 headers (or equivalent stubs) — including the using namespace std; that the real assembler/ExternFunction.hpp:51 chain provides, since pristine Market.cpp's unqualified min(bid_qty, ask_qty) (:285) relies on it.

Mechanism

Market::match() is the engine's only crossing entry point, and it runs as a while (true) loop that always drives the book to convergence before returning. It has exactly two ways to stop, and both discard rather than rest:

1. One side of the book is momentarily empty — the entire resting side is unconditionally cancelled, not just the top of it (src/OMSPack/OrderBook/Market.cpp:187-208 for an empty bid side; the symmetric :209-230 for an empty ask side):

if (bempty && !aempty) {
    spread_ask = _askOrders.spread();
    if (spread_ask != nullptr) {
        // (elided: a commented-out `FinFabr->_settlement` guard, Market.cpp:190-191 and 205, and an #ifdef DEBUG-only elog::info call, Market.cpp:192-199)
        spread_ask->cancel();
        spread_ask->Closeed();
        if (!spread_ask->isBot()) {
            Lots.push(make(spread_ask->getTicket()));
        }
    }
    continue;
}

OrderPending::Closeed() (include/OMSPack/OrderBook/Order.hpp:121-125) zeroes _openQuantity (for an order that never executed; in general it sets it to _executedQuantity), which makes isFilled() (:113-119) — and volatility() (:102), which just returns it — report true. So the next iteration's spread() treats this order as consumed and advances to the next one in the same price level, repeating until the whole side is gone, one order at a time, every time the other side is empty — not a single-order edge case.

2. Both sides non-empty, but the top of book doesn't cross — i.e. exactly the normal state of a live two-sided book — both the current best bid and best ask are disable()d and the call returns, with no Lots entry pushed at all (Market.cpp:260-274):

if (ordtype == e2::OrdType::ot_limit) {
    if (spread_bid->getPrice() < price) {
        // (elided: the rest of this elog::bug(...) call's arguments and a bare /** disable order; */ comment, Market.cpp:263-269)
        elog::bug("price null eq, tick:", spread_bid->getTicket());
        spread_ask->disable();
        spread_bid->disable();
        break;
    }
}

OrderPending::disable() (Order.hpp:139-143) zeroes the same three fields directly. A resting bid below a resting ask (bid < ask) is a perfectly ordinary, valid quote — not a fault state — but it's treated the same as case 1: both legs are destroyed on the spot.

Between the two, match() has no third outcome for "nothing crosses right now, leave the book as it is." Any call either trades, or destroys the best bid/ask pair that failed to cross (and, when one side is empty, the entire resting other side).

Why this looks intentional

The doc comment directly above match() frames it around resolving one backtest bar/session against parallel test runs, not a continuously open book (Market.cpp:144-148, 目前先处理 Limit or market / Limit 由于是多线程多进程，有可能会产生 不同的的 order 顺序 / 比如： 前一次测试在 low 成交了，后一次因为时间慢了，可能在 close 也成交不了 — "for Limit, because of multi-thread/multi-process [replay], different order sequences can result — e.g. one run fills at the day's low, a slower run might not even fill by the close"). The empty-side branch itself carries an explicit rationale a few lines up (Market.cpp:184-186, 订单可以过夜不? 先默认删除订单，否则订单会不小心 在策略的后面几天才成交，这样就会出错了 — "should orders carry over overnight? Default to deleting them first, otherwise an order might accidentally fill days later [in a replay] and produce inconsistent results"). That's a reasonable safety valve for a multi-process backtest replaying bars against historical data — it just isn't disclosed anywhere a caller of the public Market type would see it (no comment on the class in Market.hpp, none on match()'s declaration, and the non-crossing-top branch in point 2 above carries only a bare /** disable order; */ label (Market.cpp:267-269) with no rationale, unlike point 1).

Minimal reproduction

Public API only (Market::insert, Market::match, OrderPending accessors), two independent cases:

#include "OMSPack/OrderBook/Market.hpp"
#include <iostream>
#include <memory>

namespace e2q {
std::shared_ptr<FinancialFabricate> FinFabr = std::make_shared<FinancialFabricate>();
}
using namespace e2q;
static FIX::SessionID owner(const std::string& s) { return FIX::SessionID(s, "M", "T"); }

int main() {
    std::cout << "== Case 1: resting order, then a LATER crossing counter-order ==\n";
    {
        Market m;
        OrderItem* b = MALLOC(OrderItem, 1, "b1", "SYM", owner("B"),
                              e2::Side::os_Buy, e2::OrdType::ot_limit, 100.0, 10, 0, 1, 1);
        b->hasMargin(1e18);
        m.insert(b);
        std::queue<OrderLots> lots1;
        m.match(lots1, 0, 0, 2);   // book is one-sided (no ask yet)
        std::cout << "after resting buy@100 alone + match(): openQty="
                  << b->Pending()->getOpenQuantity() << "\n";

        OrderItem* s = MALLOC(OrderItem, 2, "s1", "SYM", owner("S"),
                              e2::Side::os_Sell, e2::OrdType::ot_limit, 100.0, 10, 0, 3, 3);
        s->hasMargin(1e18);
        m.insert(s);
        std::queue<OrderLots> lots2;
        m.match(lots2, 0, 0, 4);
        std::cout << "sell@100 arrives (same price, would fully cross) + match(): lots emitted="
                  << lots2.size() << ", sell openQty=" << s->Pending()->getOpenQuantity() << "\n";
    }

    std::cout << "\n== Case 2: valid two-sided NON-crossing quote (buy@99 / sell@101) ==\n";
    {
        Market m;
        OrderItem* b = MALLOC(OrderItem, 3, "b2", "SYM", owner("B"),
                              e2::Side::os_Buy, e2::OrdType::ot_limit, 99.0, 10, 0, 1, 1);
        b->hasMargin(1e18);
        OrderItem* s = MALLOC(OrderItem, 4, "s2", "SYM", owner("S"),
                              e2::Side::os_Sell, e2::OrdType::ot_limit, 101.0, 10, 0, 1, 1);
        s->hasMargin(1e18);
        m.insert(b);
        m.insert(s);
        std::queue<OrderLots> lots;
        bool any = m.match(lots, 0, 0, 2);
        std::cout << "match() on buy@99/sell@101 (bid<ask, valid quote): any=" << any
                  << " lots emitted=" << lots.size() << "\n";
        std::cout << "buy openQty=" << b->Pending()->getOpenQuantity()
                  << "  sell openQty=" << s->Pending()->getOpenQuantity() << "\n";
    }
}

== Case 1: resting order, then a LATER crossing counter-order ==
after resting buy@100 alone + match(): openQty=0
sell@100 arrives (same price, would fully cross) + match(): lots emitted=1, sell openQty=0

== Case 2: valid two-sided NON-crossing quote (buy@99 / sell@101) ==
match() on buy@99/sell@101 (bid<ask, valid quote): any=0 lots emitted=0
buy openQty=0  sell openQty=0

Case 1: a buy@100 and a later sell@100 — same price, would fully cross — never trade, because the buy is destroyed by the empty-side branch before the sell ever arrives. Case 2: an ordinary non-crossing quote (buy@99, ask@101) is wiped out in one match() call, with lots.size() == 0 — no record pushed anywhere that anything happened.

Suggested handling

No matching-logic change requested, given the "Why this looks intentional" section above. Two documentation-only options that would have saved the detour into Market.cpp/Order.hpp:

• A doc comment directly on Market::match()'s declaration in Market.hpp (and/or a class-level comment on Market) stating the contract plainly: a call resolves the book once against its current top and discards (via disable()/Closeed()) anything that doesn't cross in that same call — it does not leave a resting order for a future call to find.
• A short caveat under the README's "采用 ticket 报价: Price/Time 算法" bullet noting that this applies within a single match() resolution, and that orders aren't held open across calls the way "Price/Time" order-book matching usually implies — since that's the one place a newcomer is likely to form the opposite expectation.

Happy to share the trimmed-header build setup used above, or turn the two cases here into a small standalone test file, if either would help.

Respectfully submitted.

[vyouzhis]

thanks , i will test this bug

[vyouzhis]

update for the issues in this tag:2bc6612

thanks

[OPTIONPOOL]

I'm glad that it was helpful! Thank you for your kind words.