[Bug Report] Price-time comparator can invert FIFO priority via `(int)` overflow on a long-resting order

[OPTIONPOOL]

Found while integrating baoyingwang/orderbook 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 reading the order-book core for that integration I noticed the same-price FIFO tiebreak in the bid/ask comparators narrows a long time difference to int, which can overflow and flip the ordering for an order that has rested a long time.

Pinned at current master (90bca09c2c9b69faf85dc3efb347607a7e054adc).

Environment

• Commit 90bca09, Java. The (int) narrowing is defined by the language spec, so it behaves identically on every JDK; I reproduced the arithmetic on JDK 25 (linux/aarch64).
• The observation is in baoying.orderbook.core.OrderBook only — no FIX/Vert.x/Web layer involved.

What happens

When two orders sit at the same price, the book heaps order them by entry time (FIFO).
The comparator computes the time difference as a long and then returns (int) x. Once a
resting order's _enteringMS is more than Integer.MAX_VALUE milliseconds (~24.85 days)
away from a newly-arriving same-price order, the narrowing cast overflows and the
comparator returns the wrong sign — so the newer order is treated as having earlier time
priority, and it would be filled ahead of the order that was actually resting first.

Mechanism / root cause

OrderBook.createBidBook() (OrderBook.java:396-417) — the offer side
createAskBook() (:419-441) is identical in the tiebreak:

public int compare(ExecutingOrder o1, ExecutingOrder o2) {
    if ( Math.abs(o1._origOrder._price - o2._origOrder._price) < MIN_DIFF_FOR_PRICE) {
        long x = o1._enteringMS - o2._enteringMS != 0
                   ? o1._enteringMS - o2._enteringMS
                   : o1._increasingID - o2._increasingID;
        return (int) x;          // <-- narrows a long difference to int
    }
    if (o1._origOrder._price > o2._origOrder._price) { return -1; } else { return 1; }
}

_enteringMS is wall-clock time, set in the ExecutingOrder constructor (OrderBook.java:391):

_enteringMS = System.currentTimeMillis();

System.currentTimeMillis() differences exceed the int range after ~24.855 days
(Integer.MAX_VALUE = 2,147,483,647 ms). Past that, (int) x wraps and can change sign,
which violates the Comparator general contract and inverts the intended FIFO order. A
PriorityQueue built on an inconsistent comparator can then surface the wrong order at the
top of book, so an aggressor crosses the newer same-price order before the older one.

Demonstration of the overflow (the exact expression from the comparator):

long olderMs = 1_700_000_000_000L;          // a long-resting order's _enteringMS
long newerMs = olderMs + 2_200_000_000L;    // a same-price order ~25.46 days later
long x = newerMs - olderMs;                  // = 2_200_000_000  (a long)
System.out.println((int) x);                 // prints -2094967296  -> NEGATIVE

The difference is positive (the second order is newer), but (int) x is negative, so
compare(newer, older) reports the newer order as "less than" (earlier priority than) the
older one. For any gap under ~24.85 days the value fits in int and the sign is correct,
which is why ordinary books behave correctly and this only bites a very-long-resting order.

I want to be upfront that I have not reproduced a mis-ordered match end-to-end: a
natural trigger needs a resting limit order older than ~24.85 days at a price a new order
arrives on, and _enteringMS is final and clock-derived, so I verified the defect by
inspection plus the arithmetic above rather than by driving the matcher for 25 days. The
fallback _increasingID branch is only reached when _enteringMS is equal (same
millisecond), where the id gap cannot approach 2^31, so that path is not at risk.

Suggested fix

Return the sign of the long comparison directly instead of narrowing it, e.g. with
Long.compare (in both createBidBook() and createAskBook()):

if (o1._enteringMS != o2._enteringMS) {
    return Long.compare(o1._enteringMS, o2._enteringMS);
}
return Long.compare(o1._increasingID, o2._increasingID);

This compiles, keeps the exact FIFO intent (older _enteringMS first, then lower
_increasingID), and removes the overflow entirely. It is also a touch clearer than the
ternary-plus-cast.

---

This is just a time-stamped snapshot of a specific commit, offered back — not a comment on
the project or the author; the core reads cleanly and the matching logic itself is correct
on ordinary books. Thank you for sharing the order-book core; happy to provide the
standalone arithmetic snippet or a small comparator unit test if that would help.

Respectfully submitted.