Merge pull request #324 from northwesternfintech/fix-ids

Add warning for duplicate order IDs in orderbook
northwesternfintech · Oct 30, 2024 · e8f16e2 · e8f16e2
2 parents b7f9cee + 514c8f7
commit e8f16e2
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 12 deletions.
diff --git a/exchange/src/exchange/orders/orderbook/order_id_tracker.cpp b/exchange/src/exchange/orders/orderbook/order_id_tracker.cpp
@@ -16,8 +16,12 @@ OrderIdTracker::remove_order(common::order_id_t order_id)
 void
 OrderIdTracker::add_order(LimitOrderBook::stored_limit_order order)
 {
-    if (!order->ioc) {
-        order_map_.emplace(order->order_id, order);
-    }
+    if (order->ioc)
+        return;
+
+    if (order_map_.contains(order->order_id)) [[unlikely]]
+        throw std::runtime_error("Two orders added to orderbook with same ID");
+
+    order_map_.emplace(order->order_id, order);
 }
 } // namespace nutc::exchange
diff --git a/exchange/test/src/unit/matching/basic_matching.cpp b/exchange/test/src/unit/matching/basic_matching.cpp
@@ -10,6 +10,7 @@
 using nutc::common::Ticker;
 using nutc::common::Side::buy;
 using nutc::common::Side::sell;
+using namespace nutc::test;
 
 class UnitBasicMatching : public ::testing::Test {
 protected:
@@ -99,12 +100,13 @@ TEST_F(UnitBasicMatching, NoMatchThenMatchSell)
     tagged_limit_order order1{trader1, Ticker::ETH, buy, 1.0, 1.0};
     tagged_limit_order order2{trader2, Ticker::ETH, buy, 1.0, 1.0};
     tagged_limit_order order3{trader3, Ticker::ETH, sell, 2.0, 0.0};
+    tagged_limit_order order4{trader1, Ticker::ETH, buy, 1.0, 1.0};
 
     auto matches = add_to_engine_(order1);
     ASSERT_EQ(matches.size(), 0);
     matches = add_to_engine_(order2);
     ASSERT_EQ(matches.size(), 0);
-    matches = add_to_engine_(order1);
+    matches = add_to_engine_(order4);
     ASSERT_EQ(matches.size(), 0);
     matches = add_to_engine_(order3);
     ASSERT_EQ(matches.size(), 2);

diff --git a/exchange/test/src/unit/matching/invalid_orders.cpp b/exchange/test/src/unit/matching/invalid_orders.cpp
@@ -39,8 +39,10 @@ TEST_F(UnitInvalidOrders, RemoveThenAddFunds)
 {
     trader1.get_portfolio().modify_capital(-TEST_STARTING_CAPITAL);
 
-    tagged_limit_order order2{trader2, Ticker::ETH, sell, 1.0, 1.0};
     tagged_limit_order order1{trader1, Ticker::ETH, buy, 1.0, 1.0};
+    tagged_limit_order order11{trader1, Ticker::ETH, buy, 1.0, 1.0};
+    tagged_limit_order order2{trader2, Ticker::ETH, sell, 1.0, 1.0};
+    tagged_limit_order order22{trader2, Ticker::ETH, sell, 1.0, 1.0};
 
     // Thrown out
     auto matches = add_to_engine_(order1);
@@ -53,11 +55,11 @@ TEST_F(UnitInvalidOrders, RemoveThenAddFunds)
     trader1.get_portfolio().modify_capital(TEST_STARTING_CAPITAL);
 
     // Kept, but not matched
-    matches = add_to_engine_(order2);
+    matches = add_to_engine_(order22);
     ASSERT_EQ(matches.size(), 0);
 
     // Kept and matched
-    matches = add_to_engine_(order1);
+    matches = add_to_engine_(order11);
     ASSERT_EQ(matches.size(), 1);
     ASSERT_EQ_MATCH(matches[0], Ticker::ETH, "ABC", "DEF", buy, 1, 1);
 }

diff --git a/exchange/test/src/unit/matching/many_orders.cpp b/exchange/test/src/unit/matching/many_orders.cpp
@@ -65,11 +65,12 @@ TEST_F(UnitManyOrders, CorrectTimePriority)
 TEST_F(UnitManyOrders, OnlyMatchesOne)
 {
     tagged_limit_order order1{trader1, Ticker::ETH, buy, 1.0, 1.0};
+    tagged_limit_order order11{trader1, Ticker::ETH, buy, 1.0, 1.0};
     tagged_limit_order order2{trader2, Ticker::ETH, sell, 1.0, 1.0};
 
     auto matches = add_to_engine_(order1);
     ASSERT_EQ(matches.size(), 0);
-    matches = add_to_engine_(order1);
+    matches = add_to_engine_(order11);
     ASSERT_EQ(matches.size(), 0);
 
     matches = add_to_engine_(order2);

diff --git a/exchange/test/src/unit/matching/order_fee_matching.cpp b/exchange/test/src/unit/matching/order_fee_matching.cpp
@@ -109,14 +109,15 @@ TEST_F(UnitOrderFeeMatching, NoMatchThenMatchBuy)
 TEST_F(UnitOrderFeeMatching, NoMatchThenMatchSell)
 {
     tagged_limit_order order1{trader1, Ticker::ETH, buy, 1.0, 1.0, 0};
+    tagged_limit_order order11{trader1, Ticker::ETH, buy, 1.0, 1.0, 0};
     tagged_limit_order order2{trader2, Ticker::ETH, buy, 1.0, 1.0, 0};
     tagged_limit_order order3{trader3, Ticker::ETH, sell, 2.0, 0.0, 0};
 
     auto matches = add_to_engine_(order1);
     ASSERT_EQ(matches.size(), 0);
     matches = add_to_engine_(order2);
     ASSERT_EQ(matches.size(), 0);
-    matches = add_to_engine_(order1);
+    matches = add_to_engine_(order11);
     ASSERT_EQ(matches.size(), 0);
     matches = add_to_engine_(order3);
     ASSERT_EQ(matches.size(), 2);
@@ -284,8 +285,10 @@ TEST_F(UnitOrderFeeMatching, NotEnoughToEnough)
 {
     trader1.get_portfolio().modify_capital(-TEST_STARTING_CAPITAL + 1);
 
-    tagged_limit_order order2{trader2, Ticker::ETH, sell, 1.0, 1.0, 0};
     tagged_limit_order order1{trader1, Ticker::ETH, buy, 1.0, 1.0, 0};
+    tagged_limit_order order11{trader1, Ticker::ETH, buy, 1.0, 1.0, 0};
+    tagged_limit_order order2{trader2, Ticker::ETH, sell, 1.0, 1.0, 0};
+    tagged_limit_order order22{trader2, Ticker::ETH, sell, 1.0, 1.0, 0};
 
     // Thrown out
     auto matches = add_to_engine_(order1);
@@ -301,11 +304,11 @@ TEST_F(UnitOrderFeeMatching, NotEnoughToEnough)
     trader1.get_portfolio().modify_capital(0.5);
 
     // Kept, but not matched
-    matches = add_to_engine_(order2);
+    matches = add_to_engine_(order22);
     ASSERT_EQ(matches.size(), 0);
 
     // Kept and matched
-    matches = add_to_engine_(order1);
+    matches = add_to_engine_(order11);
     ASSERT_EQ(matches.size(), 1);
     ASSERT_EQ_MATCH(matches[0], Ticker::ETH, "ABC", "DEF", buy, 1, 1);