[Bug Report] `trade_insert` trigger updates `fulfilled` by `idNum` instead of `order_id`, and `modifyOrder`'s reprice-triggered cross never subtracts `fulfilled` — two independent bugs in the same accounting column that both let a resting order be matched past its true size

[OPTIONPOOL]

Found while integrating DrAshBooth/PyLOB 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 one surfaces while wiring up the order-lifecycle path — new order entry, fills, and in-place modify/reprice — that feeds the trade and resting-size output this benchmark cross-checks against the other engines. Both findings below are independent accounting bugs in how much of a resting order is allowed to trade, spread across two different files.

Pinned at master (c0dd9328027b6d4b39a514b6458dc5589ba245e4).

Environment

• Commit c0dd932, Python 3.12.3, stdlib sqlite3 only (linked SQLite 3.45.1) — no third-party dependencies (setup.cfg's install_requires is empty). MIT licensed.
• At this pin the book is entirely SQLite-backed: OrderBook (src/PyLOB/orderbook.py) is a thin Python class whose __init__ loads every sibling *.sql file in its own directory as an attribute (src/PyLOB/orderbook.py:27-31) and hands them to a cursor; all of price/time priority and fill accounting is enforced by the schema, views and triggers in src/create_lob.sql. (The repo's top-level README.md still describes a pure-Python red/black-tree book credited to the bintrees algorithms — that describes an earlier version; it does not describe the code at this pin.)
• Files involved: src/create_lob.sql (schema + triggers), src/PyLOB/orderbook.py (the OrderBook class), src/PyLOB/modify_order.sql, src/PyLOB/find_order.sql. No adapter/harness files.
• Both repros below register a single trader with allow_self_matching=1 and cross that one trader against itself — best_quotes/matches.sql's (allow_self_matching=1 or trader<>:tid) clause requires this for one trader to trade both sides, and it keeps the repro minimal.

What happens

Two independent bugs in how fulfilled — the cumulative-executed-quantity column every liquidity check reads back as qty - fulfilled — gets written: the trade_insert trigger updates it by matching the wrong id column (Finding 1), and modifyOrder's own reprice-triggered cross computes how much it may trade from the raw requested qty instead of qty - fulfilled (Finding 2). Findings 1 and 2 both let a resting order be matched for more than its stated size; and because an over-filled row silently drops out of best_quotes (qty > fulfilled goes false), a later, wholly unrelated order that should still find real remaining liquidity instead finds none of it and under-fills.

Finding 1 — Mechanism: trade_insert trigger keys its UPDATE on idNum, not order_id

trade.bid_order/trade.ask_order are declared as foreign keys into trade_order(order_id) (src/create_lob.sql:183-194):

create table if not exists trade (
    trade_id integer primary key, 
    bid_order integer,
    ask_order integer,
    event_dt text default(datetime('now')), -- supplied
    price real, -- order price or better
    qty integer, -- accumulates to fulfill of orders
    idNum integer, -- external supplied, optional
    foreign key(bid_order) references trade_order(order_id),
    foreign key(ask_order) references trade_order(order_id)
) -- STRICT
;

and that's exactly what gets inserted into them: processMatchesDB builds every trade tuple from order_id values only, never idNum (src/PyLOB/orderbook.py:92-93, in the loop over match candidates):

            bid_order = quote['order_id'] if quote['side'] == 'bid' else order_id
            ask_order = quote['order_id'] if quote['side'] == 'ask' else order_id

But the trigger that's supposed to apply a new trade's quantity back onto both resting orders' fulfilled filters trade_order by the other id column, idNum (src/create_lob.sql:202-209):

create trigger if not exists trade_insert
    AFTER INSERT ON trade
BEGIN
    -- increase order fulfillment
    update trade_order 
    set fulfilled=fulfilled + new.qty,
    	fulfill_price=fulfill_price + new.qty * new.price
    where idNum in (new.bid_order, new.ask_order);

idNum is a distinct column, documented in the schema itself as "external supplied, optional" (src/create_lob.sql:83) — order_id is the engine's own internal identity (the table's integer primary key, i.e. the SQLite ROWID assigned at insertion, src/create_lob.sql:72), while idNum is whatever the caller supplies, either directly via processOrder(quote, fromData=True, ...) (src/PyLOB/orderbook.py:47-54, the code path processOrder offers for exactly this — a caller-supplied idNum/timestamp instead of an auto-assigned one), or, on the default fromData=False path, from the OrderBook instance's own private nextQuoteID counter, which only happens to track order_id 1:1 as long as nothing else has ever inserted a row into that same trade_order table. The moment a row's idNum isn't numerically identical to its own order_id, where idNum in (new.bid_order, new.ask_order) matches zero rows, fulfilled silently fails to advance for that trade, and — because best_quotes only ever hides an order once qty > fulfilled goes false — the resting order keeps reappearing as fully available and can be matched again and again. The sibling trade_delete trigger, which undoes a deleted trade's effect on fulfilled, gets the same shape right, using order_id (src/create_lob.sql:262-269):

create trigger if not exists trade_delete
    AFTER DELETE ON trade
BEGIN
    -- decrease order fulfillment
    update trade_order 
    set fulfilled=fulfilled - new.qty,
    	fulfill_price=fulfill_price - new.qty * new.price
    where order_id in (new.bid_order, new.ask_order);

— evidently a copy/paste slip between the two triggers.

Verified with a standalone program against the pinned commit, run from the repository root (python3 repro1.py), driving only OrderBook.processOrder:

# repro1.py
import sqlite3
import sys
sys.path.insert(0, "src")
from PyLOB import OrderBook

conn = sqlite3.connect(":memory:")
conn.isolation_level = None
conn.executescript(open("src/create_lob.sql").read())
conn.executescript("""
begin transaction;
insert into trader (tid, name, allow_self_matching) values (1, 't1', 1);
insert into instrument (symbol, currency) values ('FAKE', null);
commit;
""")
lob = OrderBook(db=conn)

# Rest a 10-share bid with an external idNum. fromData=True is processOrder's
# own code path for a caller-supplied idNum/timestamp -- trade_order.idNum is
# documented in the schema as "external supplied, optional".
lob.processOrder(dict(type='limit', side='bid', instrument='FAKE', qty=10,
                       price=100, tid=1, idNum=500, timestamp=1), True, False)

# Two incoming asks -- 10 then 2 -- both crossing the SAME resting bid.
t1, _ = lob.processOrder(dict(type='limit', side='ask', instrument='FAKE',
                  qty=10, price=100, tid=1, idNum=501, timestamp=2), True, False)
t2, _ = lob.processOrder(dict(type='limit', side='ask', instrument='FAKE',
                  qty=2, price=100, tid=1, idNum=502, timestamp=3), True, False)

print("ASK#1 (qty=10) trades:", t1)
print("ASK#2 (qty=2)  trades:", t2)

bid_order_id, fulfilled = conn.execute(
    "select order_id, fulfilled from trade_order where idNum=500").fetchone()
total = conn.execute(
    "select coalesce(sum(qty),0) from trade where bid_order=?", (bid_order_id,)).fetchone()[0]
print(f"resting bid: order_id={bid_order_id} qty=10 trade_order.fulfilled={fulfilled}")
print(f"actual total qty traded against that 10-share bid: {total}")

Actual output against c0dd932 (unmodified):

ASK#1 (qty=10) trades: [(1, 2, 2, 100.0, 10)]
ASK#2 (qty=2)  trades: [(1, 3, 3, 100.0, 2)]
resting bid: order_id=1 qty=10 trade_order.fulfilled=0
actual total qty traded against that 10-share bid: 12

12 shares executed against a 10-share resting order: ASK#1 trades its full 10 against the bid (correctly, since the bid genuinely had 10 available at that point), but because the trigger's WHERE never matched either order's row (idNum 500/501 vs. order_id 1/2), trade_order.fulfilled for the bid is still 0 afterward. ASK#2 then queries best_quotes, sees the same bid reporting a full 10 available (qty(10) - fulfilled(0)), and trades another 2 against it — a resting 10-share order has now filled 12.

Suggested fix

 create trigger if not exists trade_insert
     AFTER INSERT ON trade
 BEGIN
     -- increase order fulfillment
     update trade_order 
     set fulfilled=fulfilled + new.qty,
     	fulfill_price=fulfill_price + new.qty * new.price
-    where idNum in (new.bid_order, new.ask_order);
+    where order_id in (new.bid_order, new.ask_order);

With this one-line change, re-running the exact program above prints trade_order.fulfilled=10 after ASK#1, ASK#2 (qty=2) trades: [] (no match — correctly, nothing is left), and the final total reads 10, not 12. I ran both the unmodified trigger and this patched one against the repro above; both sets of numbers are real output, not derived.

Finding 2 — Mechanism: modifyOrder's own reprice-triggered cross computes how much it may trade from raw qty, never subtracting fulfilled

modifyOrder (src/PyLOB/orderbook.py:136-162) fetches the row's current state through find_order.sql, including fulfilled, but only feeds three of those fields back into the dict it goes on to use:

    def modifyOrder(self, idNum, orderUpdate, time=None, verbose=False):
        if time:
            self.time = time
        else:
            self.updateTime()
        side = orderUpdate['side']
        orderUpdate['idNum'] = idNum
        orderUpdate['timestamp'] = self.time

        ret = [], orderUpdate
        crsr = self.db.cursor()
        crsr.execute('begin transaction')
        row = crsr.execute(self.find_order, (idNum,)).fetchone()
        if row:
            side, instrument, price, qty, fulfilled, cancel, order_id, order_type = row
            orderUpdate.update(
                type=order_type,
                order_id=order_id,
                instrument=instrument,
            )
            if orderUpdate.get('price'):
                orderUpdate['price'] = self.clipPrice(orderUpdate['price'])
            crsr.execute(self.modify_order, orderUpdate)
            if self.betterPrice(side, price, orderUpdate['price']):
                ret = self.processMatchesDB(orderUpdate, crsr, verbose)
        crsr.execute('commit')
        return ret

fulfilled is unpacked from find_order.sql's row (src/PyLOB/find_order.sql:2-4, select side, instrument, price, qty, fulfilled, cancel, order_id, order_type from trade_order where idNum=?) purely as a local variable — the orderUpdate.update(...) call never adds it back in. modify_order.sql itself is fine and never resets the column (src/PyLOB/modify_order.sql:11, qty=(case when :qty<fulfilled then fulfilled else :qty end),, only ever clamps qty up to at least fulfilled; it has no set fulfilled=... at all). The break is what happens next: when the reprice makes the order immediately marketable, betterPrice is true and orderUpdate — still missing fulfilled — is handed straight to processMatchesDB (src/PyLOB/orderbook.py:79-105):

    def processMatchesDB(self, quote, crsr, verbose):
        instrument = quote['instrument']
        quote.update(
            lastprice=self.lastPrice.get(instrument),
        )
        qtyToExec = quote['qty']
        sql_matches = self.matches + self.best_quotes_order_asc
        matches = crsr.execute(sql_matches, quote).fetchall()
        trades = list()
        for match in matches:
            if qtyToExec <= 0:
                break
            order_id, counterparty, price, available = match
            bid_order = quote['order_id'] if quote['side'] == 'bid' else order_id
            ask_order = quote['order_id'] if quote['side'] == 'ask' else order_id
            qty = min(available, qtyToExec)
            qtyToExec -= qty
            trade = bid_order, ask_order, self.time, price, qty
            trades.append(trade)
            if verbose: print('>>> TRADE \nt=%s $%f n=%d p1=%d p2=%d' % 
                              (self.time, price, qty,
                               counterparty, quote['tid']))
        if trades:
            crsr.executemany(self.insert_trade, trades)
            self.lastPrice[instrument] = price
            crsr.execute(self.set_lastprice, dict(instrument=instrument, lastprice=price))
        return trades, quote

qtyToExec = quote['qty'] treats the dict's raw qty as "how much of this order is still left to execute." For a brand-new order (the processOrder path, the only other caller of this same function) that's correct, since a fresh insert always starts at fulfilled=0, so qty is the true remaining. For a modify on an order that already has a nonzero fulfilled, it isn't: the reprice's own self-triggered cross is let loose to consume up to the entire (new) qty from the counterparty, not the true remainder qty - fulfilled, silently over-filling the just-repriced order — its own fulfilled can end up past its own qty — and taking more of the counterparty's resting liquidity than it was ever entitled to. Because best_quotes hides a row once qty > fulfilled goes false, that over-filled order then vanishes from the book, and whatever liquidity it wrongly took is unavailable to the next legitimate order that comes looking for it — an unrelated, later order can under-fill even though the book, honestly accounted, still had enough to give it.

Verified with a standalone program against the pinned commit, run from the repository root (python3 repro2.py), driving processOrder/modifyOrder only:

# repro2.py
import sqlite3
import sys
sys.path.insert(0, "src")
from PyLOB import OrderBook

conn = sqlite3.connect(":memory:")
conn.isolation_level = None
conn.executescript(open("src/create_lob.sql").read())
conn.executescript("""
begin transaction;
insert into trader (tid, name, allow_self_matching) values (1, 't1', 1);
insert into instrument (symbol, currency) values ('FAKE', null);
commit;
""")
lob = OrderBook(db=conn)

# A rests: 10 @ 100 (idNum=1, order_id=1 -- the normal fromData=False path,
# so idNum tracks order_id and Finding 1's bug is not a factor here).
lob.processOrder(dict(type='limit', side='bid', instrument='FAKE',
                       qty=10, price=100, tid=1), False, False)

# Partial fill: an incoming ask for 4 crosses A. A: fulfilled=4, true
# remaining = 6.
lob.processOrder(dict(type='limit', side='ask', instrument='FAKE',
                       qty=4, price=100, tid=1), False, False)

# C rests at 101 -- ABOVE A's bid of 100, so it does NOT cross yet.
lob.processOrder(dict(type='limit', side='ask', instrument='FAKE',
                       qty=20, price=101, tid=1), False, False)

qty_a, fulfilled_a = conn.execute(
    "select qty, fulfilled from trade_order where order_id=1").fetchone()
print(f"before reprice: A qty={qty_a} fulfilled={fulfilled_a} true remaining={qty_a - fulfilled_a}")

# Reprice A up to 105 -- now crosses C. modifyOrder's own self-cross fires,
# and should only be entitled to trade A's true remaining (6), not A's raw
# qty (10).
trades, _ = lob.modifyOrder(1, dict(side='bid', qty=10, price=105, tid=1))
print("modifyOrder's own reprice-triggered trade(s):", trades)

qty_a, fulfilled_a = conn.execute(
    "select qty, fulfilled from trade_order where order_id=1").fetchone()
print(f"after reprice:  A qty={qty_a} fulfilled={fulfilled_a}")

qty_c, fulfilled_c = conn.execute(
    "select qty, fulfilled from trade_order where order_id=3").fetchone()
print(f"after reprice:  C qty={qty_c} fulfilled={fulfilled_c} remaining={qty_c - fulfilled_c}")

# A fresh, wholly unrelated IOC-style bid E arrives next, priced to cross C
# and sized to exactly what C's remaining liquidity should be able to give
# it in full (12 <= the 14 that should still be resting on C if A had only
# taken its true 6).
tradesE, _ = lob.processOrder(dict(type='limit', side='bid', instrument='FAKE',
                                    qty=12, price=101, tid=1), False, False)
filledE = sum(t[4] for t in tradesE)
print(f"fresh bid E (qty=12) vs C's remaining liquidity -> trades: {tradesE}")
print(f"E matched {filledE} of 12 requested "
      f"({'FULLY FILLED' if filledE == 12 else f'UNDER-FILLED by {12 - filledE}'})")

Actual output against c0dd932 (unmodified):

before reprice: A qty=10 fulfilled=4 true remaining=6
modifyOrder's own reprice-triggered trade(s): [(1, 3, 4, 101.0, 10)]
after reprice:  A qty=10 fulfilled=14
after reprice:  C qty=20 fulfilled=10 remaining=10
fresh bid E (qty=12) vs C's remaining liquidity -> trades: [(4, 3, 5, 101.0, 10)]
E matched 10 of 12 requested (UNDER-FILLED by 2)

A's reprice trades a full 10 against C instead of its true remaining 6 (qtyToExec was 10, not 10 - 4), leaving A.fulfilled=14 — past its own qty=10 — and leaving C with only 10 of its 20 remaining instead of 14. The next order, E, a completely ordinary fresh bid sized to exactly what C's honestly-accounted liquidity should still hand it in full (12), only gets 10: under-filled by 2, because A's reprice silently took 4 shares of C's liquidity it was never entitled to.

Suggested fix

Carry fulfilled through to processMatchesDB and have it subtract there — this leaves the processOrder (fresh-order) path unaffected, since a fresh order never has a fulfilled key and .get(..., 0) defaults to 0:

             orderUpdate.update(
                 type=order_type,
                 order_id=order_id,
                 instrument=instrument,
+                fulfilled=fulfilled,
             )

     def processMatchesDB(self, quote, crsr, verbose):
         instrument = quote['instrument']
         quote.update(
             lastprice=self.lastPrice.get(instrument),
         )
-        qtyToExec = quote['qty']
+        qtyToExec = quote['qty'] - quote.get('fulfilled', 0)

Re-running the exact program above against both changes together prints modifyOrder's own reprice-triggered trade(s): [(1, 3, 4, 101.0, 6)], A qty=10 fulfilled=10 (exactly its stated size, not past it), C ... remaining=14, and finally fresh bid E (qty=12) ... trades: [(4, 3, 5, 101.0, 12)] / E matched 12 of 12 requested (FULLY FILLED). I ran both the unmodified and the patched version of this program against the pinned commit and both sets of numbers above are real, not derived.

---

This is just a time-stamped snapshot of one specific commit, offered back in case it's useful — not a comment on the project. Both repros above were run (pure Python, nothing to compile) against the pinned commit exactly as shown — engine API only, no adapter/harness involved; happy to share the two small repro files if that's useful.

Respectfully submitted.