[Bug Report] `match()` emits phantom zero-quantity trades — the price-level loop has no remaining-quantity guard

[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.

When an incoming limit order is fully filled at one price level but its price still crosses the next level, MatchDataLimitSellHolder.match() / MatchDataLimitBuyHolder.match() keep walking into that next level and emit an extra MatchDetailMq with amount = 0. The book state stays correct — every real fill is right and nothing extra is removed — but the returned fill list that the engine forwards downstream picks up phantom zero-quantity trades naming makers that were never actually touched.

Pinned at the current tip of main (7d331e5013e08c122c5d498dc22375b5f74d9e6e), in match/src/main/java/com/lmxdawn/match/disruptor/MatchDataLimitSellHolder.java and …/MatchDataLimitBuyHolder.java. The matcher classes are Java-8 source; I compiled and ran them here with JDK 21 + lombok 1.18.30 / fastutil 8.3.0.

What happens

match() has two nested loops: an outer loop over price levels and an inner loop over the FIFO orders within a level. The inner loop correctly stops once the incoming order is exhausted, but the outer loop does not check the remaining quantity — it only stops when the next level's price no longer crosses. So if the order fully fills at level k while level k+1 still crosses, the outer loop advances into level k+1 and runs the inner body once more with zero quantity left to fill, producing a 0-amount trade.

Minimal reproduction

Rest two asks at two distinct prices, then send a buy that crosses both but is fully filled by the first level (MatchEvent fields trimmed to the ones the matcher reads):

// resting SELL book: 5 @ 100  and  10 @ 105
MatchDataLimitSellHolder.put(mk(/*id*/1, /*dir*/2, /*price*/100.0, /*amount*/5.0));
MatchDataLimitSellHolder.put(mk(/*id*/2, /*dir*/2, /*price*/105.0, /*amount*/10.0));

// incoming BUY 5 @ 110 — crosses BOTH ask levels (110 >= 100 and 110 >= 105),
// but is fully filled by the 5 available at level 100.
List<MatchDetailMq> trades = MatchDataLimitSellHolder.match(mk(3, /*buy*/1, 110.0, 5.0));

Observed (the returned list has two entries):

maker=1 taker=3 price=100.0 amount=5.0     <- real fill
maker=2 taker=3 price=105.0 amount=0.0     <- phantom: zero quantity, maker 2 untouched

Correct output is a single trade (taker 3 × 5 against maker 1 @ 100). In the benchmark, with the debug System.out.printlns removed, every divergence from the consensus tape on the normal scenario was exactly one of these …,amount=0,… lines (14,686 of them); the 192-point book-state audit still passed, i.e. only the trade stream was wrong.

Mechanism / root cause

MatchDataLimitSellHolder.match(MatchEvent buy) — line numbers at the pinned commit:

// :107  entry guard already tests "is there anything left to fill?"
if ((buyAmount.compareTo(BigDecimal.ZERO) > 0 || buyTotal.compareTo(BigDecimal.ZERO) > 0) && map.size() > 0) {
    Iterator<Map.Entry<BigDecimal, List<MatchEvent>>> iterator = map.entrySet().iterator();
    while (iterator.hasNext()) {                                  // :110  outer loop — NO remaining-qty guard
        Map.Entry<BigDecimal, List<MatchEvent>> listEntry = iterator.next();
        BigDecimal bigPrice = listEntry.getKey();
        if (type == 1 && buyPrice.compareTo(bigPrice) < 0) break; // :115  only stops when the next level doesn't cross
        ...
        while (eventIterator.hasNext()) {                         // :121  inner loop
            ...
            if (bigAmount.compareTo(buyAmount) == 0) { ... }      // :154
            else if (bigAmount.compareTo(buyAmount) < 0) { ... }  // :158
            else {                                                // :162  maker has more than the taker wants…
                completeAmount = buyAmount;                       // :163  …but buyAmount is already 0 here -> completeAmount = 0
                matchDetailMq.setIsComplete(1);
                matchEvent.setAmount(bigAmount.subtract(completeAmount).doubleValue());  // :166  maker unchanged
            }
            matchDetailMq.setAmount(completeAmount.doubleValue());// :171  amount = 0
            matchDetailMqList.add(matchDetailMq);                 // :173  phantom trade pushed
            if (type == 1) {
                buyAmount = buyAmount.subtract(completeAmount);   // :177
                if (buyAmount.compareTo(BigDecimal.ZERO) <= 0) break;  // :178-179  breaks the INNER loop only
            }
            ...
        }
        ...
    }
}

After the inner loop breaks at :179 with buyAmount == 0, control returns to the outer while (iterator.hasNext()) at :110. The crossing-price test at :115 does not stop it (the next level still crosses), so it re-enters the inner loop; with nothing left to fill, the else branch at :162 sets completeAmount = buyAmount = 0 and a zero-quantity MatchDetailMq is built and pushed at :171–:173. The maker is left unchanged (:166 subtracts 0), so the book stays consistent — the defect is purely the additive phantom trade. If the order crosses three or more levels and fills early, one phantom is emitted per remaining crossing level.

MatchDataLimitBuyHolder.match(MatchEvent sell) is the exact mirror: outer loop at :111, crossing test at :116 (sellPrice > bigPrice), the zero-fill else at :159, push at :170, and the inner-only break at :173–:174 on sellAmount.

Suggested fix

Give each outer loop the same remaining-quantity continuation condition its own entry guard already uses, so it stops as soon as nothing is left to fill.

MatchDataLimitSellHolder.java:110 (mirrors the :107 guard):

-            while (iterator.hasNext()) {
+            while (iterator.hasNext()
+                    && (buyAmount.compareTo(BigDecimal.ZERO) > 0 || buyTotal.compareTo(BigDecimal.ZERO) > 0)) {

MatchDataLimitBuyHolder.java:111 (mirrors the :108 guard):

-            while (iterator.hasNext()) {
+            while (iterator.hasNext() && sellAmount.compareTo(BigDecimal.ZERO) > 0) {

I applied both and recompiled: the reproduction above returns a single trade, the phantom is gone, and every real fill is unchanged. In the benchmark, with this in place the trade stream is byte-identical to the consensus on all five scenarios. It's behaviour-neutral for every genuine fill — only the zero-quantity emissions disappear.

---

This is a time-stamped snapshot of a specific commit, offered back in case it's useful — not a verdict on the project. Happy to share the failing workload. Thanks for making the engine available.

Respectfully submitted.