TradeInfo objects refactoring. Avoid storing reference inside an obje…

…ct (OrderRef) which has been removed

src/api-objects/include/tradeinfo.hpp

@@ -9,35 +9,25 @@
 
 namespace cct::api {
 
-struct OrderRef {
-#ifndef CCT_AGGR_INIT_CXX20
-  OrderRef(const OrderId &orderId, int64_t nbSecondsSinceEpoch, Market market, TradeSide s)
-      : id(orderId), userRef(nbSecondsSinceEpoch), m(market), side(s) {}
-#endif
+using UserRefInt = int32_t;
+
+struct TradeContext {
+  TradeContext(Market market, TradeSide s, UserRefInt userRef = 0) : m(market), side(s), userRef(userRef) {}
 
   CurrencyCode fromCur() const { return side == TradeSide::kSell ? m.base() : m.quote(); }
   CurrencyCode toCur() const { return side == TradeSide::kBuy ? m.base() : m.quote(); }
 
-  const OrderId &id;
-  int64_t userRef;  // Used by Kraken for instance, used to group orders queries context
   Market m;
   TradeSide side;
+  UserRefInt userRef;  // Used by Kraken for instance, used to group orders queries context
 };
 
 struct TradeInfo {
 #ifndef CCT_AGGR_INIT_CXX20
-  TradeInfo(int64_t nbSecondsSinceEpoch, Market market, TradeSide s, const TradeOptions &opts)
-      : userRef(nbSecondsSinceEpoch), m(market), side(s), options(opts) {}
+  TradeInfo(const TradeContext &tradeContext, const TradeOptions &opts) : tradeContext(tradeContext), options(opts) {}
 #endif
 
-  CurrencyCode fromCur() const { return side == TradeSide::kSell ? m.base() : m.quote(); }
-  CurrencyCode toCur() const { return side == TradeSide::kBuy ? m.base() : m.quote(); }
-
-  OrderRef createOrderRef(const OrderId &orderId) const { return OrderRef(orderId, userRef, m, side); }
-
-  int64_t userRef;  // Used by Kraken for instance, used to group orders queries context
-  Market m;
-  TradeSide side;
+  TradeContext tradeContext;
   TradeOptions options;
 };
 

src/api/common/include/exchangeprivateapi.hpp

@@ -139,11 +139,11 @@ class ExchangePrivate : public ExchangeBase {
   /// Cancel given order id and return its possible matched amounts.
   /// When this methods ends, order should be successfully cancelled and its matched parts returned definitely (trade
   /// automaton will not come back on this order later on)
-  virtual OrderInfo cancelOrder(const OrderRef &orderRef) = 0;
+  virtual OrderInfo cancelOrder(OrderIdView orderId, const TradeContext &tradeContext) = 0;
 
   /// Query an order and return and 'OrderInfo' with its matched parts and if it is closed or not (closed means that its
   /// status and matched parts will not evolve in the future).
-  virtual OrderInfo queryOrderInfo(const OrderRef &orderRef) = 0;
+  virtual OrderInfo queryOrderInfo(OrderIdView orderId, const TradeContext &tradeContext) = 0;
 
   /// Orders a withdraw in mode fire and forget.
   virtual InitiatedWithdrawInfo launchWithdraw(MonetaryAmount grossAmount, Wallet &&wallet) = 0;

src/api/common/include/exchangeprivateapi_mock.hpp

@@ -25,8 +25,8 @@ class MockExchangePrivate : public ExchangePrivate {
 
   MOCK_METHOD(PlaceOrderInfo, placeOrder, (MonetaryAmount, MonetaryAmount, MonetaryAmount, const TradeInfo &),
               (override));
-  MOCK_METHOD(OrderInfo, cancelOrder, (const OrderRef &), (override));
-  MOCK_METHOD(OrderInfo, queryOrderInfo, (const OrderRef &), (override));
+  MOCK_METHOD(OrderInfo, cancelOrder, (OrderIdView, const TradeContext &), (override));
+  MOCK_METHOD(OrderInfo, queryOrderInfo, (OrderIdView, const TradeContext &), (override));
   MOCK_METHOD(InitiatedWithdrawInfo, launchWithdraw, (MonetaryAmount, Wallet &&), (override));
   MOCK_METHOD(SentWithdrawInfo, isWithdrawSuccessfullySent, (const InitiatedWithdrawInfo &), (override));
   MOCK_METHOD(ReceivedWithdrawInfo, isWithdrawReceived, (const InitiatedWithdrawInfo &, const SentWithdrawInfo &),

src/api/common/src/exchangeprivateapi.cpp

@@ -8,6 +8,7 @@
 #include "coincenterinfo.hpp"
 #include "recentdeposit.hpp"
 #include "timedef.hpp"
+#include "unreachable.hpp"
 
 namespace cct::api {
 
@@ -80,44 +81,44 @@ TradedAmounts ExchangePrivate::trade(MonetaryAmount from, CurrencyCode toCurrenc
 }
 
 TradedAmounts ExchangePrivate::marketTrade(MonetaryAmount from, const TradeOptions &options, Market m) {
-  const TimePoint timerStart = Clock::now();
   const CurrencyCode fromCurrency = from.currencyCode();
 
   std::optional<MonetaryAmount> optPrice = _exchangePublic.computeAvgOrderPrice(m, from, options.priceOptions());
   const CurrencyCode toCurrency = m.opposite(fromCurrency);
+  TradedAmounts totalTradedAmounts(fromCurrency, toCurrency);
   if (!optPrice) {
     log::error("Impossible to compute {} average price on {}", _exchangePublic.name(), m);
-    return TradedAmounts(fromCurrency, toCurrency);
+    return totalTradedAmounts;
   }
 
   MonetaryAmount price = *optPrice;
-  const auto nbSecondsSinceEpoch = TimestampToS(timerStart);
+  const TimePoint timerStart = Clock::now();
+  const UserRefInt userRef =
+      static_cast<UserRefInt>(TimestampToS(timerStart) % static_cast<int64_t>(std::numeric_limits<UserRefInt>::max()));
   const bool noEmergencyTime = options.maxTradeTime() == Duration::max();
 
-  TradeSide side = fromCurrency == m.base() ? TradeSide::kSell : TradeSide::kBuy;
-  TradeInfo tradeInfo(nbSecondsSinceEpoch, m, side, options);
+  const TradeSide side = fromCurrency == m.base() ? TradeSide::kSell : TradeSide::kBuy;
+  TradeContext tradeContext(m, side, userRef);
+  TradeInfo tradeInfo(tradeContext, options);
   PlaceOrderInfo placeOrderInfo = placeOrderProcess(from, price, tradeInfo);
 
   if (placeOrderInfo.isClosed()) {
     log::debug("Order {} closed with traded amounts {}", placeOrderInfo.orderId, placeOrderInfo.tradedAmounts());
     return placeOrderInfo.tradedAmounts();
   }
 
-  OrderRef orderRef(placeOrderInfo.orderId, nbSecondsSinceEpoch, m, side);
-
   TimePoint lastPriceUpdateTime = Clock::now();
   MonetaryAmount lastPrice = price;
 
-  TradedAmounts totalTradedAmounts(fromCurrency, toCurrency);
-  do {
-    OrderInfo orderInfo = queryOrderInfo(orderRef);
+  while (true) {
+    OrderInfo orderInfo = queryOrderInfo(placeOrderInfo.orderId, tradeContext);
     if (orderInfo.isClosed) {
       totalTradedAmounts += orderInfo.tradedAmounts;
       log::debug("Order {} closed with last traded amounts {}", placeOrderInfo.orderId, orderInfo.tradedAmounts);
       break;
     }
 
-    enum class NextAction { kPlaceMarketOrder, kNewOrderLimitPrice, kWait };
+    enum class NextAction : int8_t { kPlaceMarketOrder, kNewOrderLimitPrice, kWait };
     NextAction nextAction = NextAction::kWait;
 
     TimePoint t = Clock::now();
@@ -138,7 +139,7 @@ TradedAmounts ExchangePrivate::marketTrade(MonetaryAmount from, const TradeOptio
     if (reachedEmergencyTime || updatePriceNeeded) {
       // Cancel
       log::debug("Cancel order {}", placeOrderInfo.orderId);
-      OrderInfo cancelledOrderInfo = cancelOrder(orderRef);
+      OrderInfo cancelledOrderInfo = cancelOrder(placeOrderInfo.orderId, tradeContext);
       totalTradedAmounts += cancelledOrderInfo.tradedAmounts;
       from -= cancelledOrderInfo.tradedAmounts.tradedFrom;
       if (from == 0) {
@@ -159,7 +160,8 @@ TradedAmounts ExchangePrivate::marketTrade(MonetaryAmount from, const TradeOptio
         } else {
           break;
         }
-      } else {  // updatePriceNeeded
+      } else {
+        // updatePriceNeeded
         nextAction = NextAction::kNewOrderLimitPrice;
       }
       if (nextAction != NextAction::kWait) {
@@ -189,7 +191,7 @@ TradedAmounts ExchangePrivate::marketTrade(MonetaryAmount from, const TradeOptio
         }
       }
     }
-  } while (true);
+  }
 
   return totalTradedAmounts;
 }
@@ -201,11 +203,11 @@ WithdrawInfo ExchangePrivate::withdraw(MonetaryAmount grossAmount, ExchangePriva
       launchWithdraw(grossAmount, targetExchange.queryDepositWallet(currencyCode));
   log::info("Withdraw {} of {} to {} initiated from {} to {}", initiatedWithdrawInfo.withdrawId(), grossAmount,
             initiatedWithdrawInfo.receivingWallet(), exchangeName(), targetExchange.exchangeName());
-  enum class NextAction { kCheckSender, kCheckReceiver, kTerminate };
-  NextAction action = NextAction::kCheckSender;
   SentWithdrawInfo sentWithdrawInfo;
   ReceivedWithdrawInfo receivedWithdrawInfo;
-  do {
+
+  enum class NextAction : int8_t { kCheckSender, kCheckReceiver, kTerminate };
+  for (NextAction action = NextAction::kCheckSender; action != NextAction::kTerminate;) {
     std::this_thread::sleep_for(withdrawRefreshTime);
     switch (action) {
       case NextAction::kCheckSender:
@@ -224,8 +226,10 @@ WithdrawInfo ExchangePrivate::withdraw(MonetaryAmount grossAmount, ExchangePriva
         break;
       case NextAction::kTerminate:
         break;
+      default:
+        unreachable();
     }
-  } while (action != NextAction::kTerminate);
+  }
   WithdrawInfo withdrawInfo(std::move(initiatedWithdrawInfo), receivedWithdrawInfo.netReceivedAmount());
   log::info("Confirmed withdrawal {}", withdrawInfo);
   return withdrawInfo;
@@ -435,8 +439,8 @@ TradedAmountsVectorWithFinalAmount ExchangePrivate::queryDustSweeper(CurrencyCod
 
 PlaceOrderInfo ExchangePrivate::placeOrderProcess(MonetaryAmount &from, MonetaryAmount price,
                                                   const TradeInfo &tradeInfo) {
-  Market m = tradeInfo.m;
-  const bool isSell = tradeInfo.side == TradeSide::kSell;
+  Market m = tradeInfo.tradeContext.m;
+  const bool isSell = tradeInfo.tradeContext.side == TradeSide::kSell;
   MonetaryAmount volume(isSell ? from : MonetaryAmount(from / price, m.base()));
 
   if (tradeInfo.options.isSimulation() && !isSimulatedOrderSupported()) {
@@ -470,7 +474,7 @@ PlaceOrderInfo ExchangePrivate::computeSimulatedMatchedPlacedOrderInfo(MonetaryA
                                                                        const TradeInfo &tradeInfo) const {
   const bool placeSimulatedRealOrder = exchangeInfo().placeSimulateRealOrder();
   const bool isTakerStrategy = tradeInfo.options.isTakerStrategy(placeSimulatedRealOrder);
-  const bool isSell = tradeInfo.side == TradeSide::kSell;
+  const bool isSell = tradeInfo.tradeContext.side == TradeSide::kSell;
   MonetaryAmount toAmount = isSell ? volume.convertTo(price) : volume;
   ExchangeInfo::FeeType feeType = isTakerStrategy ? ExchangeInfo::FeeType::kTaker : ExchangeInfo::FeeType::kMaker;
   toAmount = _coincenterInfo.exchangeInfo(_exchangePublic.name()).applyFee(toAmount, feeType);

src/api/common/test/exchangeprivateapi_test.cpp

@@ -25,10 +25,20 @@ inline BalancePortfolio operator+(const BalancePortfolio &b, const TradedAmounts
 inline bool operator==(const TradedAmountsVectorWithFinalAmount &lhs, const TradedAmountsVectorWithFinalAmount &rhs) {
   return lhs.finalAmount == rhs.finalAmount && lhs.tradedAmountsVector == rhs.tradedAmountsVector;
 }
+
 }  // namespace cct
 
 namespace cct::api {
 
+inline bool operator==(const TradeContext &lhs, const TradeContext &rhs) {
+  // We don't compare on value userRef which is set from a timestamp
+  return lhs.m == rhs.m && lhs.side == rhs.side;
+}
+
+inline bool operator==(const TradeInfo &lhs, const TradeInfo &rhs) {
+  return lhs.tradeContext == rhs.tradeContext && lhs.options == rhs.options;
+}
+
 class ExchangePrivateTest : public ::testing::Test {
  protected:
   void tradeBaseExpectCalls() {
@@ -40,6 +50,7 @@ class ExchangePrivateTest : public ::testing::Test {
   CoincenterInfo coincenterInfo{settings::RunMode::kProd, loadConfiguration};
   CryptowatchAPI cryptowatchAPI{coincenterInfo, settings::RunMode::kProd, Duration::max(), true};
   FiatConverter fiatConverter{coincenterInfo, Duration::max()};  // max to avoid real Fiat converter queries
+
   MockExchangePublic exchangePublic{kSupportedExchanges[0], fiatConverter, cryptowatchAPI, coincenterInfo};
   APIKey key{"test", "testuser", "", "", ""};
   MockExchangePrivate exchangePrivate{exchangePublic, coincenterInfo, key};
@@ -48,7 +59,6 @@ class ExchangePrivateTest : public ::testing::Test {
 
   VolAndPriNbDecimals volAndPriDec{2, 2};
   int depth = 15;
-  int64_t nbSecondsSinceEpoch = 0;
 
   MonetaryAmount askPrice1{"2300.45 EUR"};
   MonetaryAmount bidPrice1{"2300.4 EUR"};
@@ -66,10 +76,6 @@ class ExchangePrivateTest : public ::testing::Test {
       askPrice3, MonetaryAmount("0.96 ETH"), bidPrice3, MonetaryAmount("3.701 ETH"), volAndPriDec, depth};
 };
 
-inline bool operator==(const TradeInfo &lhs, const TradeInfo &rhs) { return lhs.m == rhs.m && lhs.side == rhs.side; }
-
-inline bool operator==(const OrderRef &lhs, const OrderRef &rhs) { return lhs.id == rhs.id; }
-
 TEST_F(ExchangePrivateTest, TakerTradeBaseToQuote) {
   tradeBaseExpectCalls();
 
@@ -79,7 +85,8 @@ TEST_F(ExchangePrivateTest, TakerTradeBaseToQuote) {
 
   PriceOptions priceOptions(PriceStrategy::kTaker);
   TradeOptions tradeOptions(priceOptions);
-  TradeInfo tradeInfo(nbSecondsSinceEpoch, market, TradeSide::kSell, tradeOptions);
+  TradeContext tradeContext(market, TradeSide::kSell);
+  TradeInfo tradeInfo(tradeContext, tradeOptions);
 
   MonetaryAmount tradedTo("23004 EUR");
 
@@ -101,7 +108,8 @@ TEST_F(ExchangePrivateTest, TakerTradeQuoteToBase) {
   MonetaryAmount vol(from / pri, market.base());
   PriceOptions priceOptions(PriceStrategy::kTaker);
   TradeOptions tradeOptions(priceOptions);
-  TradeInfo tradeInfo(nbSecondsSinceEpoch, market, TradeSide::kBuy, tradeOptions);
+  TradeContext tradeContext(market, TradeSide::kBuy);
+  TradeInfo tradeInfo(tradeContext, tradeOptions);
 
   MonetaryAmount tradedTo = vol * pri.toNeutral();
 
@@ -122,25 +130,25 @@ TEST_F(ExchangePrivateTest, MakerTradeBaseToQuote) {
   MonetaryAmount pri(askPrice1);
 
   TradeSide side = TradeSide::kSell;
+  TradeContext tradeContext(market, side);
+
   PriceOptions priceOptions(PriceStrategy::kMaker);
   TradeOptions tradeOptions(priceOptions);
-  TradeInfo tradeInfo(nbSecondsSinceEpoch, market, side, tradeOptions);
+  TradeInfo tradeInfo(tradeContext, tradeOptions);
 
   EXPECT_CALL(exchangePublic, queryOrderBook(market, testing::_)).WillOnce(testing::Return(marketOrderBook1));
 
   PlaceOrderInfo unmatchedPlacedOrderInfo(OrderInfo(TradedAmounts(from.currencyCode(), market.quote()), false),
                                           OrderId("Order # 0"));
 
-  OrderRef orderRef(unmatchedPlacedOrderInfo.orderId, nbSecondsSinceEpoch, market, side);
-
   EXPECT_CALL(exchangePrivate, placeOrder(from, vol, pri, tradeInfo))
       .WillOnce(testing::Return(unmatchedPlacedOrderInfo));
 
   MonetaryAmount partialMatchedFrom = from / 5;
   MonetaryAmount partialMatchedTo = partialMatchedFrom.toNeutral() * askPrice1;
   MonetaryAmount fullMatchedTo = from.toNeutral() * askPrice1;
 
-  EXPECT_CALL(exchangePrivate, queryOrderInfo(orderRef))
+  EXPECT_CALL(exchangePrivate, queryOrderInfo(static_cast<OrderIdView>(unmatchedPlacedOrderInfo.orderId), tradeContext))
       .WillOnce(testing::Return(unmatchedPlacedOrderInfo.orderInfo))
       .WillOnce(testing::Return(OrderInfo(TradedAmounts(partialMatchedFrom, partialMatchedTo), false)))
       .WillOnce(testing::Return(OrderInfo(TradedAmounts(from, fullMatchedTo), true)));
@@ -155,13 +163,14 @@ TEST_F(ExchangePrivateTest, MakerTradeQuoteToBase) {
   MonetaryAmount pri1(bidPrice1);
   MonetaryAmount pri2(bidPrice2);
   TradeSide side = TradeSide::kBuy;
+  TradeContext tradeContext(market, side);
 
   MonetaryAmount vol1(from / pri1, market.base());
   MonetaryAmount vol2(from / pri2, market.base());
 
   TradeOptions tradeOptions(TradeTimeoutAction::kCancel, TradeMode::kReal, Duration::max(), Duration::zero(),
                             TradeTypePolicy::kForceMultiTrade);
-  TradeInfo tradeInfo(nbSecondsSinceEpoch, market, side, tradeOptions);
+  TradeInfo tradeInfo(tradeContext, tradeOptions);
 
   {
     testing::InSequence seq;
@@ -179,24 +188,23 @@ TEST_F(ExchangePrivateTest, MakerTradeQuoteToBase) {
   PlaceOrderInfo unmatchedPlacedOrderInfo1(unmatchedOrderInfo, OrderId("Order # 0"));
   PlaceOrderInfo unmatchedPlacedOrderInfo2(unmatchedOrderInfo, OrderId("Order # 1"));
 
-  OrderRef orderRef1(unmatchedPlacedOrderInfo1.orderId, nbSecondsSinceEpoch, market, side);
-  OrderRef orderRef2(unmatchedPlacedOrderInfo2.orderId, nbSecondsSinceEpoch, market, side);
-
   // Call once queryOrderBook
 
   // Place first order
   EXPECT_CALL(exchangePrivate, placeOrder(from, vol1, pri1, tradeInfo))
       .WillOnce(testing::Return(unmatchedPlacedOrderInfo1));
 
-  EXPECT_CALL(exchangePrivate, queryOrderInfo(orderRef1))
+  EXPECT_CALL(exchangePrivate,
+              queryOrderInfo(static_cast<OrderIdView>(unmatchedPlacedOrderInfo1.orderId), tradeContext))
       .Times(2)
       .WillRepeatedly(testing::Return(unmatchedPlacedOrderInfo1.orderInfo));
 
   // Call once queryOrderBook, price still the same, no change, and queryOrderInfo a second time with unmatched amounts
   // Call once queryOrderBook with new price
 
   // Price change, cancel order
-  EXPECT_CALL(exchangePrivate, cancelOrder(orderRef1)).WillOnce(testing::Return(OrderInfo(zeroTradedAmounts, false)));
+  EXPECT_CALL(exchangePrivate, cancelOrder(static_cast<OrderIdView>(unmatchedPlacedOrderInfo1.orderId), tradeContext))
+      .WillOnce(testing::Return(OrderInfo(zeroTradedAmounts, false)));
 
   // Place second order
   EXPECT_CALL(exchangePrivate, placeOrder(from, vol2, pri2, tradeInfo))
@@ -207,11 +215,12 @@ TEST_F(ExchangePrivateTest, MakerTradeQuoteToBase) {
 
   TradedAmounts partialMatchedTradedAmounts(partialMatchedFrom, partialMatchedTo);
 
-  EXPECT_CALL(exchangePrivate, queryOrderInfo(orderRef2))
+  EXPECT_CALL(exchangePrivate,
+              queryOrderInfo(static_cast<OrderIdView>(unmatchedPlacedOrderInfo2.orderId), tradeContext))
       .WillOnce(testing::Return(unmatchedPlacedOrderInfo2.orderInfo))
       .WillOnce(testing::Return(OrderInfo(partialMatchedTradedAmounts, false)));
 
-  EXPECT_CALL(exchangePrivate, cancelOrder(orderRef2))
+  EXPECT_CALL(exchangePrivate, cancelOrder(static_cast<OrderIdView>(unmatchedPlacedOrderInfo2.orderId), tradeContext))
       .WillOnce(testing::Return(OrderInfo(partialMatchedTradedAmounts, false)));
 
   MonetaryAmount pri3(bidPrice3);
@@ -237,7 +246,8 @@ TEST_F(ExchangePrivateTest, SimulatedOrderShouldNotCallPlaceOrder) {
   TradeSide side = TradeSide::kSell;
   TradeOptions tradeOptions(TradeTimeoutAction::kCancel, TradeMode::kSimulation, Duration::max(), Duration::zero(),
                             TradeTypePolicy::kForceMultiTrade);
-  TradeInfo tradeInfo(nbSecondsSinceEpoch, market, side, tradeOptions);
+  TradeContext tradeContext(market, side);
+  TradeInfo tradeInfo(tradeContext, tradeOptions);
 
   EXPECT_CALL(exchangePublic, queryOrderBook(market, testing::_)).WillOnce(testing::Return(marketOrderBook1));
 
@@ -331,7 +341,8 @@ class ExchangePrivateDustSweeperTest : public ExchangePrivateTest {
     MonetaryAmount vol(from);
 
     Market m{from.currencyCode(), pri.currencyCode()};
-    TradeInfo tradeInfo(nbSecondsSinceEpoch, m, TradeSide::kSell, tradeOptions);
+    TradeContext tradeContext(m, TradeSide::kSell);
+    TradeInfo tradeInfo(tradeContext, tradeOptions);
 
     MonetaryAmount tradedTo = vol.toNeutral() * pri;
 
@@ -356,7 +367,8 @@ class ExchangePrivateDustSweeperTest : public ExchangePrivateTest {
     MonetaryAmount from = to.toNeutral() * bidPri;
     MonetaryAmount vol(from / askPri, m.base());
 
-    TradeInfo tradeInfo(nbSecondsSinceEpoch, m, TradeSide::kBuy, tradeOptions);
+    TradeContext tradeContext(m, TradeSide::kBuy);
+    TradeInfo tradeInfo(tradeContext, tradeOptions);
 
     TradedAmounts tradedAmounts(MonetaryAmount{success ? from : MonetaryAmount(0), askPri.currencyCode()},
                                 success ? vol : MonetaryAmount{0, vol.currencyCode()});