Fix race condition for simultaneous calls to queryTradableMarkets fro…

…m private exchanges

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/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 open 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 SSL SUPPORT");
+  }
   return curlVersionInfoStr;
 }
 

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
 )

src/tech/include/threadpool.hpp

@@ -40,7 +40,7 @@ class ThreadPool {
 
   // add new work item to the pool
   template <class Func, class... Args>
-  std::future<typename std::invoke_result<Func, Args...>::type> enqueue(Func&& f, Args&&... args);
+  std::future<std::invoke_result_t<Func, Args...>> enqueue(Func&& func, Args&&... args);
 
   // Parallel version of std::transform with unary operation.
   // This function will first enqueue all the tasks at one, using waiting threads of the thread pool,
@@ -104,11 +104,11 @@ inline ThreadPool::~ThreadPool() {
 }
 
 template <class Func, class... Args>
-inline std::future<typename std::invoke_result<Func, Args...>::type> ThreadPool::enqueue(Func&& f, Args&&... args) {
-  using return_type = typename std::invoke_result<Func, Args...>::type;
+inline std::future<std::invoke_result_t<Func, Args...>> ThreadPool::enqueue(Func&& func, Args&&... args) {
+  using return_type = std::invoke_result_t<Func, Args...>;
 
   auto task = std::make_shared<std::packaged_task<return_type()>>(
-      std::bind(std::forward<Func>(f), std::forward<Args>(args)...));
+      std::bind(std::forward<Func>(func), std::forward<Args>(args)...));
 
   std::future<return_type> res = task->get_future();
   {