Fix race condition for simultaneous calls to queryTradableMarkets fro…

…m private exchanges

Dockerfile

@@ -1,6 +1,11 @@
-# Multi stage build to separate docker build image from executable (to make the latter smaller)
 FROM ubuntu:22.04 AS build
 
+# Declare and set default values of following arguments
+ARG BUILD_MODE=Release
+ARG BUILD_TEST=0
+ARG BUILD_ASAN=0
+ARG BUILD_WITH_PROMETHEUS=1
+
 # Install base & build dependencies, needed certificates for curl to work with https
 RUN apt update && \
     apt upgrade -y && \
@@ -24,15 +29,13 @@ COPY data/secret/secret_test.json .
 WORKDIR /app/data/static
 COPY data/static/currency_prefix_translator.json data/static/currencyacronymtranslator.json data/static/stablecoins.json data/static/withdrawfees.json ./
 
+# Copy fiats cache (needed in tests to avoid live requests)
+WORKDIR /app/data/cache
+COPY data/cache/fiatcache.json ./
+
 # Create and go to 'bin' directory
 WORKDIR /app/bin
 
-# Declare and set default values of following arguments
-ARG BUILD_MODE=Release
-ARG BUILD_TEST=0
-ARG BUILD_ASAN=0
-ARG BUILD_WITH_PROMETHEUS=1
-
 # Build and launch tests if any
 RUN cmake -DCMAKE_BUILD_TYPE=${BUILD_MODE} \
     -DCCT_ENABLE_TESTS=${BUILD_TEST} \

alpine.Dockerfile

@@ -1,6 +1,11 @@
-# Multi stage build to separate docker build image from executable (to make the latter smaller)
 FROM alpine:3.19.1 AS build
 
+# Declare and set default values of following arguments
+ARG BUILD_MODE=Release
+ARG BUILD_TEST=0
+ARG BUILD_ASAN=0
+ARG BUILD_WITH_PROMETHEUS=1
+
 # Install base & build dependencies, needed certificates for curl to work with https
 RUN apk add --update --upgrade --no-cache g++ libc-dev openssl-dev curl-dev cmake ninja git ca-certificates
 
@@ -22,15 +27,13 @@ COPY data/secret/secret_test.json .
 WORKDIR /app/data/static
 COPY data/static/currency_prefix_translator.json data/static/currencyacronymtranslator.json data/static/stablecoins.json data/static/withdrawfees.json ./
 
+# Copy fiats cache (needed in tests to avoid live requests)
+WORKDIR /app/data/cache
+COPY data/cache/fiatcache.json ./
+
 # Create and go to 'bin' directory
 WORKDIR /app/bin
 
-# Declare and set default values of following arguments
-ARG BUILD_MODE=Release
-ARG BUILD_TEST=0
-ARG BUILD_ASAN=0
-ARG BUILD_WITH_PROMETHEUS=1
-
 # Build and launch tests if any
 RUN cmake -DCMAKE_BUILD_TYPE=${BUILD_MODE} \
     -DCCT_ENABLE_TESTS=${BUILD_TEST} \

src/api/common/include/exchangepublicapi.hpp

@@ -137,9 +137,9 @@ class ExchangePublic : public ExchangeBase {
   std::optional<MonetaryAmount> computeAvgOrderPrice(Market mk, MonetaryAmount from, const PriceOptions &priceOptions);
 
   /// Retrieve the market in the correct order proposed by the exchange for given couple of currencies.
-  Market retrieveMarket(CurrencyCode c1, CurrencyCode c2, const MarketSet &markets);
+  std::optional<Market> retrieveMarket(CurrencyCode c1, CurrencyCode c2, const MarketSet &markets);
 
-  Market retrieveMarket(CurrencyCode c1, CurrencyCode c2) { return retrieveMarket(c1, c2, queryTradableMarkets()); }
+  std::optional<Market> retrieveMarket(CurrencyCode c1, CurrencyCode c2);
 
   /// Helper method to determine ordered Market from this exchange from a market string representation without currency
   /// separator (for instance, "BTCEUR" should be guessed as a market with BTC as base currency, and EUR as price

src/api/common/src/exchangepublicapi.cpp

@@ -270,34 +270,36 @@ std::optional<MonetaryAmount> ExchangePublic::computeAvgOrderPrice(Market mk, Mo
   return queryOrderBook(mk, depth).computeAvgPrice(from, priceOptions);
 }
 
-Market ExchangePublic::retrieveMarket(CurrencyCode c1, CurrencyCode c2, const MarketSet &markets) {
+std::optional<Market> ExchangePublic::retrieveMarket(CurrencyCode c1, CurrencyCode c2, const MarketSet &markets) {
   Market mk(c1, c2);
   if (!markets.contains(mk)) {
     mk = mk.reverse();
     if (!markets.contains(mk)) {
-      throw exception("Cannot find {}-{} nor {}-{} markets on {}", c1, c2, c2, c1, _name);
+      return {};
     }
   }
   return mk;
 }
 
+std::optional<Market> ExchangePublic::retrieveMarket(CurrencyCode c1, CurrencyCode c2) {
+  std::lock_guard<std::mutex> guard(_tradableMarketsMutex);
+  return retrieveMarket(c1, c2, queryTradableMarkets());
+}
+
 MarketPriceMap ExchangePublic::MarketPriceMapFromMarketOrderBookMap(const MarketOrderBookMap &marketOrderBookMap) {
   MarketPriceMap marketPriceMap;
   marketPriceMap.reserve(marketOrderBookMap.size());
-  for (const auto &it : marketOrderBookMap) {
-    std::optional<MonetaryAmount> optAmount = it.second.averagePrice();
+  for (const auto &[market, marketOrderBook] : marketOrderBookMap) {
+    std::optional<MonetaryAmount> optAmount = marketOrderBook.averagePrice();
     if (optAmount) {
-      marketPriceMap.insert_or_assign(it.first, *optAmount);
+      marketPriceMap.insert_or_assign(market, *optAmount);
     }
   }
   return marketPriceMap;
 }
 
 std::optional<Market> ExchangePublic::determineMarketFromMarketStr(std::string_view marketStr, MarketSet &markets,
                                                                    CurrencyCode filterCur) {
-  std::optional<Market> ret;
-  static constexpr std::string_view::size_type kMinimalCryptoAcronymLen = 3;
-
   if (!filterCur.isNeutral()) {
     std::size_t firstCurLen;
     auto curStr = filterCur.str();
@@ -307,53 +309,63 @@ std::optional<Market> ExchangePublic::determineMarketFromMarketStr(std::string_v
     } else {
       firstCurLen = marketStr.size() - curStr.size();
     }
-    ret = Market(
+    return Market(
         _coincenterInfo.standardizeCurrencyCode(std::string_view(marketStr.data(), firstCurLen)),
         _coincenterInfo.standardizeCurrencyCode(std::string_view(marketStr.begin() + firstCurLen, marketStr.end())));
-  } else if (markets.empty() && marketStr.size() == 2 * kMinimalCryptoAcronymLen) {
-    // optim (to avoid possible queryTradableMarkets): there is no crypto currency acronym shorter than 3 chars - we
-    // can split the "symbol" string currencies with 3 chars each
-    ret = Market(_coincenterInfo.standardizeCurrencyCode(std::string_view(marketStr.data(), kMinimalCryptoAcronymLen)),
+  }
+
+  static constexpr std::string_view::size_type kMinimalCryptoAcronymLen = 3;
+
+  if (markets.empty() && marketStr.size() == 2 * kMinimalCryptoAcronymLen) {
+    // optim (to avoid possible queryTradableMarkets): assuming there is no crypto currency acronym shorter than 3 chars
+    // - we can split the "symbol" string currencies with 3 chars each
+    return Market(_coincenterInfo.standardizeCurrencyCode(std::string_view(marketStr.data(), kMinimalCryptoAcronymLen)),
+                  _coincenterInfo.standardizeCurrencyCode(
+                      std::string_view(marketStr.data() + kMinimalCryptoAcronymLen, kMinimalCryptoAcronymLen)));
+  }
+
+  std::optional<Market> ret;
+
+  // General case, we need to query the markets
+  if (markets.empty()) {
+    // Without any currency, and because "marketStr" is returned without hyphen, there is no easy way to guess the
+    // currencies so we need to compare with the markets that exist
+    std::lock_guard<std::mutex> guard(_tradableMarketsMutex);
+    markets = queryTradableMarkets();
+  }
+  const auto symbolStrSize = marketStr.size();
+  for (auto splitCurPos = kMinimalCryptoAcronymLen; splitCurPos < symbolStrSize; ++splitCurPos) {
+    ret = Market(_coincenterInfo.standardizeCurrencyCode(std::string_view(marketStr.data(), splitCurPos)),
                  _coincenterInfo.standardizeCurrencyCode(
-                     std::string_view(marketStr.data() + kMinimalCryptoAcronymLen, kMinimalCryptoAcronymLen)));
-  } else {  // General case, we need to query the markets
-    if (markets.empty()) {
-      // Without any currency, and because "marketStr" is returned without hyphen, there is no easy way to guess the
-      // currencies so we need to compare with the markets that exist
-      markets = queryTradableMarkets();
-    }
-    const auto symbolStrSize = marketStr.size();
-    for (auto splitCurPos = kMinimalCryptoAcronymLen; splitCurPos < symbolStrSize; ++splitCurPos) {
-      ret = Market(_coincenterInfo.standardizeCurrencyCode(std::string_view(marketStr.data(), splitCurPos)),
-                   _coincenterInfo.standardizeCurrencyCode(
-                       std::string_view(marketStr.data() + splitCurPos, symbolStrSize - splitCurPos)));
-      if (markets.contains(*ret)) {
-        break;
-      }
-      ret = ret->reverse();
-      if (markets.contains(*ret)) {
-        break;
-      }
+                     std::string_view(marketStr.data() + splitCurPos, symbolStrSize - splitCurPos)));
+    if (markets.contains(*ret)) {
+      break;
     }
-    if (!ret || ret->quote().size() < kMinimalCryptoAcronymLen) {
-      log::error("Cannot determine market for {}, skipping", marketStr);
-      ret = std::nullopt;
+    ret = ret->reverse();
+    if (markets.contains(*ret)) {
+      break;
     }
   }
+  if (!ret || ret->quote().size() < kMinimalCryptoAcronymLen) {
+    log::error("Cannot determine market for {}, skipping", marketStr);
+    ret = std::nullopt;
+  }
+
   return ret;
 }
 
 Market ExchangePublic::determineMarketFromFilterCurrencies(MarketSet &markets, CurrencyCode filterCur1,
                                                            CurrencyCode filterCur2) {
   if (markets.empty()) {
+    std::lock_guard<std::mutex> guard(_tradableMarketsMutex);
     markets = queryTradableMarkets();
   }
 
   Market ret;
 
   const auto tryAppendBaseCurrency = [&](CurrencyCode cur) {
     if (!cur.isNeutral()) {
-      auto firstMarketIt = std::ranges::partition_point(markets, [cur](Market mk) { return mk.base() < cur; });
+      const auto firstMarketIt = std::ranges::partition_point(markets, [cur](Market mk) { return mk.base() < cur; });
       if (firstMarketIt != markets.end() && firstMarketIt->base() == cur) {
         ret = Market(cur, CurrencyCode());
         return true;

src/api/common/test/exchangepublicapi_test.cpp

@@ -78,9 +78,9 @@ TEST_F(ExchangePublicTest, FindCurrenciesPath) {
 TEST_F(ExchangePublicTest, RetrieveMarket) {
   EXPECT_CALL(exchangePublic, queryTradableMarkets()).WillOnce(::testing::Return(markets));
 
-  EXPECT_EQ(exchangePublic.retrieveMarket("BTC", "KRW"), Market("BTC", "KRW"));
-  EXPECT_EQ(exchangePublic.retrieveMarket("KRW", "BTC", markets), Market("BTC", "KRW"));
-  EXPECT_THROW(exchangePublic.retrieveMarket("EUR", "EOS", markets), exception);
+  EXPECT_EQ(exchangePublic.retrieveMarket("BTC", "KRW").value(), Market("BTC", "KRW"));
+  EXPECT_EQ(exchangePublic.retrieveMarket("KRW", "BTC", markets).value(), Market("BTC", "KRW"));
+  EXPECT_FALSE(exchangePublic.retrieveMarket("EUR", "EOS", markets).has_value());
 }
 
 class ExchangePublicConvertTest : public ExchangePublicTest {

src/api/exchanges/src/binanceprivateapi.cpp

@@ -34,6 +34,7 @@
 #include "exchangename.hpp"
 #include "exchangeprivateapi.hpp"
 #include "exchangeprivateapitypes.hpp"
+#include "exchangepublicapi.hpp"
 #include "exchangepublicapitypes.hpp"
 #include "httprequesttype.hpp"
 #include "market.hpp"
@@ -275,14 +276,11 @@ Wallet BinancePrivate::DepositWalletFunc::operator()(CurrencyCode currencyCode)
 }
 
 bool BinancePrivate::checkMarketAppendSymbol(Market mk, CurlPostData& params) {
-  MarketSet markets = _exchangePublic.queryTradableMarkets();
-  if (!markets.contains(mk)) {
-    mk = mk.reverse();
-    if (!markets.contains(mk)) {
-      return false;
-    }
+  const auto optMarket = _exchangePublic.retrieveMarket(mk.base(), mk.quote());
+  if (!optMarket) {
+    return false;
   }
-  params.append("symbol", mk.assetsPairStrUpper());
+  params.append("symbol", optMarket->assetsPairStrUpper());
   return true;
 }
 

src/engine/include/staticcommandlineoptioncheck.hpp

@@ -18,16 +18,16 @@ consteval bool StaticCommandLineOptionsDuplicatesCheck(std::array<T, N>... ar) {
   auto all = ComputeAllCommandLineOptions(ar...);
 
   // Check short names equality with a bitset hashmap of presence
-  // (std::bitset is unfortunately not constexpr yet)
+  // (std::bitset is unfortunately not constexpr in C++20)
   uint64_t charPresenceBmp[8]{};
-  for (const auto& f : all) {
-    if (f.hasShortName()) {
-      uint8_t c = static_cast<uint8_t>(f.shortNameChar());
-      uint64_t& subBmp = charPresenceBmp[c / 64];
-      if ((subBmp & (static_cast<uint64_t>(1) << (c % 64)))) {
+  for (const auto& commandLineOption : all) {
+    if (commandLineOption.hasShortName()) {
+      uint8_t shortNameChar = static_cast<uint8_t>(commandLineOption.shortNameChar());
+      uint64_t& subBmp = charPresenceBmp[shortNameChar / 64];
+      if ((subBmp & (static_cast<uint64_t>(1) << (shortNameChar % 64)))) {
         return false;
       }
-      subBmp |= (static_cast<uint64_t>(1) << (c % 64));
+      subBmp |= (static_cast<uint64_t>(1) << (shortNameChar % 64));
     }
   }
 

src/engine/test/exchangedata_test.hpp

@@ -23,7 +23,8 @@ class ExchangesBaseTest : public ::testing::Test {
   LoadConfiguration loadConfiguration{kDefaultDataDir, LoadConfiguration::ExchangeConfigFileType::kTest};
   settings::RunMode runMode = settings::RunMode::kTestKeys;
   CoincenterInfo coincenterInfo{runMode, loadConfiguration};
-  api::CommonAPI commonAPI{coincenterInfo, Duration::max()};
+  api::CommonAPI commonAPI{coincenterInfo, Duration::max(), Duration::max(),
+                           api::CommonAPI::AtInit::kLoadFromFileCache};
   FiatConverter fiatConverter{coincenterInfo, Duration::max()};  // max to avoid real Fiat converter queries
   api::MockExchangePublic exchangePublic1{kSupportedExchanges[0], fiatConverter, commonAPI, coincenterInfo};
   api::MockExchangePublic exchangePublic2{kSupportedExchanges[1], fiatConverter, commonAPI, coincenterInfo};

src/http-request/src/curlhandle.cpp

@@ -69,10 +69,18 @@ void CurlSetLogIfError(CURL *curl, CURLoption curlOption, T value) {
 
 string GetCurlVersionInfo() {
   const curl_version_info_data &curlVersionInfo = *curl_version_info(CURLVERSION_NOW);
+
   string curlVersionInfoStr("curl ");
   curlVersionInfoStr.append(curlVersionInfo.version);
+  if (curlVersionInfo.ssl_version == nullptr) {
+    throw exception("Invalid curl install - no ssl support");
+  }
   curlVersionInfoStr.append(" ssl ").append(curlVersionInfo.ssl_version);
-  curlVersionInfoStr.append(" libz ").append(curlVersionInfo.libz_version);
+  if (curlVersionInfo.libz_version != nullptr) {
+    curlVersionInfoStr.append(" libz ").append(curlVersionInfo.libz_version);
+  } else {
+    curlVersionInfoStr.append(" NO libz support");
+  }
   return curlVersionInfoStr;
 }
 
@@ -85,14 +93,13 @@ CurlHandle::CurlHandle(BestURLPicker bestURLPicker, AbstractMetricGateway *pMetr
       _requestAnswerLogLevel(permanentCurlOptions.requestAnswerLogLevel()),
       _nbMaxRetries(permanentCurlOptions.nbMaxRetries()),
       _tooManyErrorsPolicy(permanentCurlOptions.tooManyErrorsPolicy()) {
-  if (settings::AreQueryResponsesOverriden(runMode)) {
-    _handle = nullptr;
-  } else {
-    _handle = curl_easy_init();
-    if (_handle == nullptr) {
+  if (!settings::AreQueryResponsesOverriden(runMode)) {
+    CURL *curl = curl_easy_init();
+    if (curl == nullptr) {
       throw std::bad_alloc();
     }
-    CURL *curl = reinterpret_cast<CURL *>(_handle);
+
+    _handle = curl;
 
     const string &userAgent = permanentCurlOptions.getUserAgent();
     if (userAgent.empty()) {
@@ -123,11 +130,10 @@ CurlHandle::CurlHandle(BestURLPicker bestURLPicker, AbstractMetricGateway *pMetr
                _bestURLPicker.getNextBaseURL(), DurationToString(_minDurationBetweenQueries));
 
     if (settings::IsProxyRequested(runMode)) {
-      if (IsProxyAvailable()) {
-        setUpProxy(GetProxyURL(), false);
-      } else {
-        throw std::runtime_error("Requesting proxy usage without any available proxy.");
+      if (!IsProxyAvailable()) {
+        throw std::runtime_error("Requesting proxy usage without any available proxy");
       }
+      setUpProxy(GetProxyURL(), false);
     }
   }
 }

src/main/CMakeLists.txt

@@ -1,30 +1,30 @@
 aux_source_directory(src MAIN_SRC)
 
 if(CCT_BUILD_EXEC)
-    add_executable(coincenter ${MAIN_SRC})
+  add_executable(coincenter ${MAIN_SRC})
 
-    # Enable LTO with coincenter in Release mode
-    if(CMAKE_BUILD_TYPE STREQUAL "Release")
-        set_property(TARGET coincenter PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
-        message(STATUS "Activate LTO for coincenter")
-    endif()
+  # Enable LTO with coincenter in Release mode
+  if(CMAKE_BUILD_TYPE STREQUAL "Release")
+    set_property(TARGET coincenter PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
+    message(STATUS "Activate LTO for coincenter")
+  endif()
 else()
-    list(REMOVE_ITEM ENGINE_SRC "src/main.cpp")
-    add_library(coincenter STATIC ${MAIN_SRC})
+  list(REMOVE_ITEM ENGINE_SRC "src/main.cpp")
+  add_library(coincenter STATIC ${MAIN_SRC})
 endif()
 
 target_link_libraries(coincenter PUBLIC coincenter_engine)
 
 set_target_properties(coincenter PROPERTIES
-    VERSION ${PROJECT_VERSION}
-    COMPILE_DEFINITIONS_DEBUG "JSON_DEBUG;JSON_SAFE;JSON_ISO_STRICT"
-    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}
+  VERSION ${PROJECT_VERSION}
+  COMPILE_DEFINITIONS_DEBUG "JSON_DEBUG;JSON_SAFE;JSON_ISO_STRICT"
+  RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}
 )
 
 add_unit_test(
-    processcommandsfromcli_test
-    src/processcommandsfromcli.cpp
-    test/processcommandsfromcli_test.cpp
-    LIBRARIES
-    coincenter_engine
+  processcommandsfromcli_test
+  src/processcommandsfromcli.cpp
+  test/processcommandsfromcli_test.cpp
+  LIBRARIES
+  coincenter_engine
 )