While reviewing the order-book invariants on 55fc781, I found that two resting orders with the same OrderId can make the public views disagree and leave live liquidity that can no longer be cancelled by ID.
Minimal reproduction:
let mut book = OrderBook::new();
let mut fills = Vec::new();
book.add_order(limit(1, Side::Buy, 100, 5), &mut fills);
book.add_order(limit(1, Side::Buy, 99, 7), &mut fills);
assert!(book.cancel(1));
assert!(book.is_empty()); // currently true: the id index is empty
assert_eq!(book.best_bid(), None); // currently Some(100)
assert_eq!(book.depth_at(Side::Buy, 100), 0); // currently 5
assert!(!book.cancel(1)); // the surviving order is no longer indexed
The first node remains in its price-level list and can still match. The second place call's locations.insert(id, ...) replaces the first node's cancel-index entry; cancelling removes only the second node.
I tested a small native fix: return before matching/placement when locations already contains the incoming ID, plus the regression above. With that change, the complete current suite passes:
cargo test --locked
11 passed; 0 failed
I have the two-file patch ready if ignoring a duplicate live ID matches the intended API contract.
While reviewing the order-book invariants on
55fc781, I found that two resting orders with the sameOrderIdcan make the public views disagree and leave live liquidity that can no longer be cancelled by ID.Minimal reproduction:
The first node remains in its price-level list and can still match. The second
placecall'slocations.insert(id, ...)replaces the first node's cancel-index entry; cancelling removes only the second node.I tested a small native fix: return before matching/placement when
locationsalready contains the incoming ID, plus the regression above. With that change, the complete current suite passes:I have the two-file patch ready if ignoring a duplicate live ID matches the intended API contract.