Fix race condition on get all approximated order books when several e…

…xchange private call to convert at the same time
sjanel · Dec 9, 2023 · 40a67e3 · 40a67e3
1 parent 1b4aa11
commit 40a67e3
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 13 deletions.
diff --git a/src/api/common/include/exchangepublicapi.hpp b/src/api/common/include/exchangepublicapi.hpp
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mutex>
 #include <optional>
 #include <string_view>
 
@@ -51,19 +52,19 @@ class ExchangePublic : public ExchangeBase {
 
   /// Attempts to convert amount into a target currency.
   /// Conversion is made according to given price options, which uses the 'Maker' prices by default.
-  std::optional<MonetaryAmount> convert(MonetaryAmount a, CurrencyCode toCurrency,
+  std::optional<MonetaryAmount> convert(MonetaryAmount from, CurrencyCode toCurrency,
                                         const PriceOptions &priceOptions = PriceOptions()) {
     MarketOrderBookMap marketOrderBookMap;
     Fiats fiats = queryFiats();
     MarketSet markets;
-    MarketsPath conversionPath = findMarketsPath(a.currencyCode(), toCurrency, markets, fiats, true);
-    return convert(a, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions);
+    MarketsPath conversionPath = findMarketsPath(from.currencyCode(), toCurrency, markets, fiats, true);
+    return convert(from, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions);
   }
 
   /// Attempts to convert amount into a target currency.
   /// Conversion is made according to given price options, which uses the 'Maker' prices by default.
   /// No external calls is made with this version, it has all what it needs
-  std::optional<MonetaryAmount> convert(MonetaryAmount a, CurrencyCode toCurrency, const MarketsPath &conversionPath,
+  std::optional<MonetaryAmount> convert(MonetaryAmount from, CurrencyCode toCurrency, const MarketsPath &conversionPath,
                                         const Fiats &fiats, MarketOrderBookMap &marketOrderBookMap,
                                         const PriceOptions &priceOptions = PriceOptions());
 
@@ -178,6 +179,8 @@ class ExchangePublic : public ExchangeBase {
   CommonAPI &_commonApi;
   const CoincenterInfo &_coincenterInfo;
   const ExchangeInfo &_exchangeInfo;
+  std::mutex _tradableMarketsMutex;
+  std::mutex _allOrderBooksMutex;
 };
 }  // namespace api
 }  // namespace cct
diff --git a/src/api/common/src/exchangepublicapi.cpp b/src/api/common/src/exchangepublicapi.cpp
@@ -33,47 +33,48 @@ ExchangePublic::ExchangePublic(std::string_view name, FiatConverter &fiatConvert
       _coincenterInfo(coincenterInfo),
       _exchangeInfo(coincenterInfo.exchangeInfo(name)) {}
 
-std::optional<MonetaryAmount> ExchangePublic::convert(MonetaryAmount amount, CurrencyCode toCurrency,
+std::optional<MonetaryAmount> ExchangePublic::convert(MonetaryAmount from, CurrencyCode toCurrency,
                                                       const MarketsPath &conversionPath, const Fiats &fiats,
                                                       MarketOrderBookMap &marketOrderBookMap,
                                                       const PriceOptions &priceOptions) {
-  if (amount.currencyCode() == toCurrency) {
-    return amount;
+  if (from.currencyCode() == toCurrency) {
+    return from;
   }
   if (conversionPath.empty()) {
     return std::nullopt;
   }
   const ExchangeInfo::FeeType feeType =
       priceOptions.isTakerStrategy() ? ExchangeInfo::FeeType::kTaker : ExchangeInfo::FeeType::kMaker;
   for (Market mk : conversionPath) {
-    CurrencyCode mFromCurrencyCode = amount.currencyCode();
+    CurrencyCode mFromCurrencyCode = from.currencyCode();
     assert(mk.canTrade(mFromCurrencyCode));
-    CurrencyCode mToCurrencyCode = mk.base() == amount.currencyCode() ? mk.quote() : mk.base();
+    CurrencyCode mToCurrencyCode = mk.base() == from.currencyCode() ? mk.quote() : mk.base();
     std::optional<CurrencyCode> optFiatLikeFrom = _coincenterInfo.fiatCurrencyIfStableCoin(mFromCurrencyCode);
     CurrencyCode fiatFromLikeCurCode = (optFiatLikeFrom ? *optFiatLikeFrom : mFromCurrencyCode);
     std::optional<CurrencyCode> optFiatLikeTo = _coincenterInfo.fiatCurrencyIfStableCoin(mToCurrencyCode);
     CurrencyCode fiatToLikeCurCode = (optFiatLikeTo ? *optFiatLikeTo : mToCurrencyCode);
     bool isFromFiatLike = optFiatLikeFrom || fiats.contains(mFromCurrencyCode);
     bool isToFiatLike = optFiatLikeTo || fiats.contains(mToCurrencyCode);
     if (isFromFiatLike && isToFiatLike) {
-      amount = _fiatConverter.convert(MonetaryAmount(amount, fiatFromLikeCurCode), fiatToLikeCurCode);
+      from = _fiatConverter.convert(MonetaryAmount(from, fiatFromLikeCurCode), fiatToLikeCurCode);
     } else {
       if (marketOrderBookMap.empty()) {
+        std::lock_guard<std::mutex> guard(_allOrderBooksMutex);
         marketOrderBookMap = queryAllApproximatedOrderBooks(1);
       }
       auto it = marketOrderBookMap.find(mk);
       if (it == marketOrderBookMap.end()) {
         return std::nullopt;
       }
       const MarketOrderBook &marketOrderbook = it->second;
-      std::optional<MonetaryAmount> optA = marketOrderbook.convert(amount, priceOptions);
+      std::optional<MonetaryAmount> optA = marketOrderbook.convert(from, priceOptions);
       if (!optA) {
         return std::nullopt;
       }
-      amount = _exchangeInfo.applyFee(*optA, feeType);
+      from = _exchangeInfo.applyFee(*optA, feeType);
     }
   }
-  return amount;
+  return from;
 }
 
 namespace {
@@ -145,6 +146,7 @@ MarketsPath ExchangePublic::findMarketsPath(CurrencyCode fromCurrency, CurrencyC
       return ret;
     }
     if (markets.empty()) {
+      std::lock_guard<std::mutex> guard(_tradableMarketsMutex);
       markets = queryTradableMarkets();
       if (markets.empty()) {
         log::error("No markets retrieved for {}", _name);