Skip to content

Commit

Permalink
[Code cleaning] - Fix shadow warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
sjanel committed Nov 11, 2024
1 parent 73c3684 commit a63c44a
Show file tree
Hide file tree
Showing 33 changed files with 715 additions and 72 deletions.
4 changes: 2 additions & 2 deletions cmake/CoincenterUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function (target_set_coincenter_options name)
endif()
else()
if(MSVC)
target_compile_options(${name} PRIVATE /W1)
target_compile_options(${name} PRIVATE /W1)
else()
target_compile_options(${name} PRIVATE -Wdisabled-optimization -Winline)
endif()
Expand All @@ -46,4 +46,4 @@ function(add_coincenter_executable name)
add_executable(${name} ${MY_UNPARSED_ARGUMENTS})

target_set_coincenter_options(${name})
endfunction()
endfunction()
2 changes: 1 addition & 1 deletion src/api/common/include/exchange-permanent-curl-options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ class ExchangePermanentCurlOptions {

} // namespace api

} // namespace cct
} // namespace cct
2 changes: 1 addition & 1 deletion src/api/common/src/exchange-permanent-curl-options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ PermanentCurlOptions::Builder ExchangePermanentCurlOptions::builderBase(Api api)
return builder;
}

} // namespace cct::api
} // namespace cct::api
2 changes: 1 addition & 1 deletion src/api/common/src/exchangeprivateapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ PlaceOrderInfo ExchangePrivate::computeSimulatedMatchedPlacedOrderInfo(MonetaryA
const bool isSell = tradeInfo.tradeContext.side == TradeSide::kSell;

MonetaryAmount toAmount = isSell ? volume.convertTo(price) : volume;
ExchangeConfig::FeeType feeType = isTakerStrategy ? ExchangeConfig::FeeType::kTaker : ExchangeConfig::FeeType::kMaker;
ExchangeConfig::FeeType feeType = isTakerStrategy ? ExchangeConfig::FeeType::Taker : ExchangeConfig::FeeType::Maker;
toAmount = _coincenterInfo.exchangeConfig(_exchangePublic.name()).applyFee(toAmount, feeType);
PlaceOrderInfo placeOrderInfo(OrderInfo(TradedAmounts(isSell ? volume : volume.toNeutral() * price, toAmount)),
OrderId("SimulatedOrderId"));
Expand Down
2 changes: 1 addition & 1 deletion src/api/common/src/exchangepublicapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ std::optional<MonetaryAmount> ExchangePublic::convert(MonetaryAmount from, Curre
return std::nullopt;
}
const ExchangeConfig::FeeType feeType =
priceOptions.isTakerStrategy() ? ExchangeConfig::FeeType::kTaker : ExchangeConfig::FeeType::kMaker;
priceOptions.isTakerStrategy() ? ExchangeConfig::FeeType::Taker : ExchangeConfig::FeeType::Maker;

if (marketOrderBookMap.empty()) {
std::lock_guard<std::recursive_mutex> guard(_publicRequestsMutex);
Expand Down
2 changes: 1 addition & 1 deletion src/api/common/test/exchangeprivateapi_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ TEST_F(ExchangePrivateTest, SimulatedOrderShouldNotCallPlaceOrder) {

// In simulation mode, fee is applied
MonetaryAmount toAmount =
exchangePublic.exchangeConfig().applyFee(from.toNeutral() * askPrice1, ExchangeConfig::FeeType::kMaker);
exchangePublic.exchangeConfig().applyFee(from.toNeutral() * askPrice1, ExchangeConfig::FeeType::Maker);

EXPECT_EQ(exchangePrivate.trade(from, market.quote(), tradeOptions), TradedAmounts(from, toAmount));
}
Expand Down
14 changes: 7 additions & 7 deletions src/api/common/test/exchangepublicapi_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ TEST_F(ExchangePublicConvertTest, ConvertSimple) {
exchangePublic.convert(from, toCurrency, conversionPath, fiats, marketOrderBookMap, priceOptions);
ASSERT_TRUE(ret.has_value());
MonetaryAmount res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::kMaker);
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::Maker);
EXPECT_EQ(ret, std::optional<MonetaryAmount>(res));
}

Expand All @@ -210,9 +210,9 @@ TEST_F(ExchangePublicConvertTest, ConvertDouble) {
ASSERT_TRUE(ret.has_value());

MonetaryAmount res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::kMaker);
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::Maker);
res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook2.convert(res, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::kMaker);
marketOrderBook2.convert(res, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::Maker);
EXPECT_EQ(ret.value_or(MonetaryAmount{}), res);
}

Expand All @@ -232,7 +232,7 @@ TEST_F(ExchangePublicConvertTest, ConvertWithFiatAtBeginning) {

MonetaryAmount res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook3.convert(optRes.value_or(MonetaryAmount{}), priceOptions).value_or(MonetaryAmount{-1}),
ExchangeConfig::FeeType::kMaker);
ExchangeConfig::FeeType::Maker);

EXPECT_EQ(ret.value_or(MonetaryAmount{}), res);
}
Expand All @@ -249,11 +249,11 @@ TEST_F(ExchangePublicConvertTest, ConvertWithFiatAtEnd) {
ASSERT_TRUE(ret.has_value());

MonetaryAmount res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::kMaker);
marketOrderBook1.convert(from, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::Maker);
res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook4.convert(res, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::kMaker);
marketOrderBook4.convert(res, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::Maker);
res = exchangePublic.exchangeConfig().applyFee(
marketOrderBook3.convert(res, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::kMaker);
marketOrderBook3.convert(res, priceOptions).value_or(MonetaryAmount{-1}), ExchangeConfig::FeeType::Maker);

EXPECT_EQ(ret, fiatConverter.convert(res, toCurrency));
}
Expand Down
1 change: 1 addition & 0 deletions src/api/exchanges/include/bithumbprivateapi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <unordered_map>

#include "cache-file-updator-interface.hpp"
#include "cachedresult.hpp"
#include "cct_json-container.hpp"
#include "curlhandle.hpp"
Expand Down
3 changes: 1 addition & 2 deletions src/api/exchanges/src/bithumbprivateapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,7 @@ PlaceOrderInfo BithumbPrivate::placeOrder(MonetaryAmount /*from*/, MonetaryAmoun

// Volume is gross amount if from amount is in quote currency, we should remove the fees
if (fromCurrencyCode == mk.quote()) {
ExchangeConfig::FeeType feeType =
isTakerStrategy ? ExchangeConfig::FeeType::kTaker : ExchangeConfig::FeeType::kMaker;
ExchangeConfig::FeeType feeType = isTakerStrategy ? ExchangeConfig::FeeType::Taker : ExchangeConfig::FeeType::Maker;
const ExchangeConfig& exchangeConfig = _coincenterInfo.exchangeConfig(_exchangePublic.name());
volume = exchangeConfig.applyFee(volume, feeType);
}
Expand Down
3 changes: 1 addition & 2 deletions src/api/exchanges/src/upbitprivateapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,7 @@ void UpbitPrivate::applyFee(Market mk, CurrencyCode fromCurrencyCode, bool isTak
MonetaryAmount& volume) {
if (fromCurrencyCode == mk.quote()) {
// For 'buy', from amount is fee excluded
ExchangeConfig::FeeType feeType =
isTakerStrategy ? ExchangeConfig::FeeType::kTaker : ExchangeConfig::FeeType::kMaker;
ExchangeConfig::FeeType feeType = isTakerStrategy ? ExchangeConfig::FeeType::Taker : ExchangeConfig::FeeType::Maker;
const ExchangeConfig& exchangeConfig = _coincenterInfo.exchangeConfig(_exchangePublic.name());
if (isTakerStrategy) {
from = exchangeConfig.applyFee(from, feeType);
Expand Down
9 changes: 9 additions & 0 deletions src/basic-objects/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,13 @@ add_unit_test(
coincenter_basic-objects
DEFINITIONS
CCT_DISABLE_SPDLOG
)

add_unit_test(
monetaryamountbycurrencyset_test
test/monetaryamountbycurrencyset_test.cpp
LIBRARIES
coincenter_basic-objects
DEFINITIONS
CCT_DISABLE_SPDLOG
)
8 changes: 4 additions & 4 deletions src/basic-objects/include/cct_const.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace cct {

enum class SupportedExchangeName : int8_t { binance, bithumb, huobi, kraken, kucoin, upbit };
enum class ExchangeNameEnum : int8_t { binance, bithumb, huobi, kraken, kucoin, upbit };

static constexpr std::string_view kDefaultDataDir = CCT_DATA_DIR;

Expand All @@ -32,16 +32,16 @@ static constexpr std::string_view kExchangeConfigFileName = "exchangeconfig.json

// To make enum serializable as strings
template <>
struct glz::meta<cct::SupportedExchangeName> {
using enum cct::SupportedExchangeName;
struct glz::meta<cct::ExchangeNameEnum> {
using enum cct::ExchangeNameEnum;

static constexpr auto value = enumerate(binance, bithumb, huobi, kraken, kucoin, upbit);
};

namespace cct {

/// Ordered list of supported exchange names.
static constexpr auto kSupportedExchanges = json::reflect<SupportedExchangeName>::keys;
static constexpr auto kSupportedExchanges = json::reflect<ExchangeNameEnum>::keys;

static_assert(std::ranges::is_sorted(kSupportedExchanges));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class MonetaryAmountByCurrencySet {

void clear() noexcept { _set.clear(); }

const_iterator find(const MonetaryAmount &v) const { return _set.find(v); }
bool contains(const MonetaryAmount &v) const { return find(v) != end(); }
const_iterator find(const value_type &v) const { return _set.find(v); }
bool contains(const value_type &v) const { return find(v) != end(); }

const_iterator find(CurrencyCode standardCode) const {
// This is possible as MonetaryAmount are ordered by standard code
Expand All @@ -65,22 +65,46 @@ class MonetaryAmountByCurrencySet {

bool contains(CurrencyCode standardCode) const { return find(standardCode) != end(); }

std::pair<iterator, bool> insert(const MonetaryAmount &v) { return _set.insert(v); }
std::pair<iterator, bool> insert(MonetaryAmount &&v) { return _set.insert(std::move(v)); }
std::pair<iterator, bool> insert(const value_type &v) { return _set.insert(v); }
std::pair<iterator, bool> insert(value_type &&v) { return _set.insert(std::move(v)); }

iterator insert(const_iterator hint, const MonetaryAmount &v) { return _set.insert(hint, v); }
iterator insert(const_iterator hint, MonetaryAmount &&v) { return _set.insert(hint, std::move(v)); }
iterator insert(const_iterator hint, const value_type &v) { return _set.insert(hint, v); }
iterator insert(const_iterator hint, value_type &&v) { return _set.insert(hint, std::move(v)); }

template <class InputIt>
void insert(InputIt first, InputIt last) {
_set.insert(first, last);
}

iterator insert_or_assign(const value_type &v) {
auto [it, inserted] = _set.insert(v);
// We only change the amount, not the currency, so the order is still valid
const_cast<value_type &>(*it) = v;
return it;
}

// Inserts elements in the set as if by insert, but if an amount in the range [first, last) already exists in the
// set for a given currency, assigns the amount of that element with the amount of this MonetaryAmount being inserted.
template <class InputIt>
void insert_or_assign(InputIt first, InputIt last) {
iterator insertHint = begin();
while (first != last) {
insertHint = _set.insert(insertHint, *first);

// We only change the amount, not the currency, so the order is still valid
const_cast<value_type &>(*insertHint) = *first;

++first;
}
}

template <class... Args>
std::pair<iterator, bool> emplace(Args &&...args) {
return _set.emplace(std::forward<Args &&>(args)...);
}

auto operator<=>(const MonetaryAmountByCurrencySet &) const = default;

using trivially_relocatable = is_trivially_relocatable<SetType>::type;

private:
Expand Down
1 change: 1 addition & 0 deletions src/basic-objects/test/currencycode_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <algorithm>
#include <map>
#include <string>

#include "cct_invalid_argument_exception.hpp"
#include "cct_json-serialization.hpp"
Expand Down
31 changes: 31 additions & 0 deletions src/basic-objects/test/monetaryamountbycurrencyset_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#include "monetaryamountbycurrencyset.hpp"

#include <gtest/gtest.h>

namespace cct {

class MonetaryAmountByCurrencySetTest : public ::testing::Test {
protected:
MonetaryAmountByCurrencySet set{MonetaryAmount("1.5 EUR"), MonetaryAmount("2.5 DOGE"), MonetaryAmount("3.5 BTC")};
};

TEST_F(MonetaryAmountByCurrencySetTest, InsertOrAssign) {
set.insert_or_assign(MonetaryAmount("2.5 EUR"));
auto it = set.find(CurrencyCode("EUR"));
ASSERT_NE(it, set.end());
EXPECT_EQ(*it, MonetaryAmount("2.5 EUR"));
}

TEST_F(MonetaryAmountByCurrencySetTest, InsertOrAssignRange) {
MonetaryAmountByCurrencySet other{MonetaryAmount("4 ETH"), MonetaryAmount("5.5 DOGE"), MonetaryAmount("6.5 POW")};
set.insert_or_assign(other.begin(), other.end()); // Inserting the same currency as in set should overwrite the value

EXPECT_EQ(set.size(), 5U);

MonetaryAmountByCurrencySet expected{MonetaryAmount("1.5 EUR"), MonetaryAmount("4 ETH"), MonetaryAmount("5.5 DOGE"),
MonetaryAmount("6.5 POW"), MonetaryAmount("3.5 BTC")};

EXPECT_EQ(set, expected);
}

} // namespace cct
4 changes: 2 additions & 2 deletions src/objects/include/exchangeconfig.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
namespace cct {
class ExchangeConfig {
public:
enum class FeeType : int8_t { kMaker, kTaker };
enum class FeeType : int8_t { Maker, Taker };
enum class MarketDataSerialization : int8_t { kYes, kNo };

struct APIUpdateFrequencies {
Expand Down Expand Up @@ -62,7 +62,7 @@ class ExchangeConfig {
/// Apply the general maker fee defined for this exchange on given MonetaryAmount.
/// In other words, convert a gross amount into a net amount with maker fees
MonetaryAmount applyFee(MonetaryAmount ma, FeeType feeType) const {
return ma * (feeType == FeeType::kMaker ? _generalMakerRatio : _generalTakerRatio);
return ma * (feeType == FeeType::Maker ? _generalMakerRatio : _generalTakerRatio);
}

MonetaryAmount getMakerFeeRatio() const { return _generalMakerRatio; }
Expand Down
3 changes: 2 additions & 1 deletion src/objects/include/exchangeconfigparser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "currencycode.hpp"
#include "currencycodevector.hpp"
#include "durationstring.hpp"
#include "exchange-config.hpp"
#include "monetaryamount.hpp"
#include "monetaryamountbycurrencyset.hpp"

Expand All @@ -24,7 +25,7 @@ class TopLevelOption {
// Top level option names
static constexpr std::string_view kAssetsOptionStr = "asset";
static constexpr std::string_view kQueryOptionStr = "query";
static constexpr std::string_view kTradeFeesOptionStr = "tradefees";
static constexpr std::string_view kTradeFeesOptionStr = "tradeFees";
static constexpr std::string_view kWithdrawOptionStr = "withdraw";

/// @brief Create a TopLevelOption from given json data.
Expand Down
27 changes: 8 additions & 19 deletions src/objects/src/exchangeconfigdefault.hpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
#pragma once

#include "cct_json-container.hpp"
#include <string_view>

namespace cct {
struct ExchangeConfigDefault {
static json::container Prod() {
// Use a static method instead of an inline static const variable to avoid the infamous 'static initialization order
// fiasco' problem.

static const json::container kProd = R"(
struct ExchangeConfigDefault {
static constexpr std::string_view kProd = R"(
{
"asset": {
"default": {
Expand Down Expand Up @@ -142,17 +139,11 @@ struct ExchangeConfigDefault {
"validateDepositAddressesInFile": true
}
}
}
)"_json;
return kProd;
}
}
)";

/// ExchangeInfos for tests only. Some tests rely on provided values so changing them may make them fail.
static json::container Test() {
// Use a static method instead of an inline static const variable to avoid the infamous 'static initialization order
// fiasco' problem.

static const json::container kTest = R"(
static constexpr std::string_view kTest = R"(
{
"asset": {
"default": {
Expand Down Expand Up @@ -212,7 +203,7 @@ struct ExchangeConfigDefault {
"validateApiKey": true
}
},
"tradefees": {
"tradeFees": {
"default": {
"maker": "0.1",
"taker": "0.2"
Expand All @@ -224,8 +215,6 @@ struct ExchangeConfigDefault {
}
}
}
)"_json;
return kTest;
}
)";
};
} // namespace cct
4 changes: 2 additions & 2 deletions src/objects/src/exchangeconfigmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
#include <utility>

#include "cct_const.hpp"
#include "cct_json-container.hpp"
#include "cct_log.hpp"
#include "cct_string.hpp"
#include "exchange-config.hpp"
#include "exchangeconfig.hpp"
#include "exchangeconfigdefault.hpp"
#include "exchangeconfigparser.hpp"
Expand All @@ -24,7 +24,7 @@ namespace cct {
ExchangeConfigMap ComputeExchangeConfigMap(std::string_view fileName, const json::container &jsonData) {
ExchangeConfigMap map;

const json::container &prodDefault = ExchangeConfigDefault::Prod();
json::container prodDefault = json::container::parse(ExchangeConfigDefault::kProd);

TopLevelOption assetTopLevelOption(TopLevelOption::kAssetsOptionStr, prodDefault, jsonData);
TopLevelOption queryTopLevelOption(TopLevelOption::kQueryOptionStr, prodDefault, jsonData);
Expand Down
Loading

0 comments on commit a63c44a

Please sign in to comment.