From 2499276fca84123700d34eb023a938651b57c111 Mon Sep 17 00:00:00 2001 From: Matthias Date: Wed, 6 Jul 2022 19:15:55 +0200 Subject: [PATCH 1/7] Refactor calculate_fee_rate to take separate parameters instead of an "Order" we passed in a trade object anyway --- freqtrade/exchange/exchange.py | 49 +++++++++++++++++++++------------ freqtrade/freqtradebot.py | 7 +++-- tests/exchange/test_exchange.py | 5 ++-- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index 4febe5652..ced1c71fb 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -1631,27 +1631,30 @@ class Exchange: and order['fee']['cost'] is not None ) - def calculate_fee_rate(self, order: Dict) -> Optional[float]: + def calculate_fee_rate( + self, fee: Dict, symbol: str, cost: float, amount: float) -> Optional[float]: """ Calculate fee rate if it's not given by the exchange. - :param order: Order or trade (one trade) dict + :param fee: ccxt Fee dict - must contain cost / currency / rate + :param symbol: Symbol of the order + :param cost: Total cost of the order + :param amount: Amount of the order """ - if order['fee'].get('rate') is not None: - return order['fee'].get('rate') - fee_curr = order['fee']['currency'] + if fee.get('rate') is not None: + return fee.get('rate') + fee_curr = fee['currency'] # Calculate fee based on order details - if fee_curr in self.get_pair_base_currency(order['symbol']): + if fee_curr in self.get_pair_base_currency(symbol): # Base currency - divide by amount - return round( - order['fee']['cost'] / safe_value_fallback2(order, order, 'filled', 'amount'), 8) - elif fee_curr in self.get_pair_quote_currency(order['symbol']): + return round(fee['cost'] / amount, 8) + elif fee_curr in self.get_pair_quote_currency(symbol): # Quote currency - divide by cost return round(self._contracts_to_amount( - order['symbol'], order['fee']['cost']) / order['cost'], - 8) if order['cost'] else None + symbol, fee['cost']) / cost, + 8) if cost else None else: # If Fee currency is a different currency - if not order['cost']: + if not cost: # If cost is None or 0.0 -> falsy, return None return None try: @@ -1664,18 +1667,28 @@ class Exchange: if not fee_to_quote_rate: return None return round((self._contracts_to_amount( - order['symbol'], order['fee']['cost']) * fee_to_quote_rate) / order['cost'], 8) + symbol, fee['cost']) * fee_to_quote_rate) / cost, 8) - def extract_cost_curr_rate(self, order: Dict) -> Tuple[float, str, Optional[float]]: + def extract_cost_curr_rate(self, fee: Dict, symbol: str, cost: float, + amount: float) -> Tuple[float, str, Optional[float]]: """ Extract tuple of cost, currency, rate. Requires order_has_fee to run first! - :param order: Order or trade (one trade) dict + :param fee: ccxt Fee dict - must contain cost / currency / rate + :param symbol: Symbol of the order + :param cost: Total cost of the order + :param amount: Amount of the order :return: Tuple with cost, currency, rate of the given fee dict """ - return (order['fee']['cost'], - order['fee']['currency'], - self.calculate_fee_rate(order)) + return (fee['cost'], + fee['currency'], + self.calculate_fee_rate( + fee, + symbol, + cost, + amount + ) + ) # Historic data diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index d1404807d..e0532a17d 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1742,7 +1742,8 @@ class FreqtradeBot(LoggingMixin): trade_base_currency = self.exchange.get_pair_base_currency(trade.pair) # use fee from order-dict if possible if self.exchange.order_has_fee(order): - fee_cost, fee_currency, fee_rate = self.exchange.extract_cost_curr_rate(order) + fee_cost, fee_currency, fee_rate = self.exchange.extract_cost_curr_rate( + order['fee'], order['symbol'], order['cost'], order_obj.safe_filled) logger.info(f"Fee for Trade {trade} [{order_obj.ft_order_side}]: " f"{fee_cost:.8g} {fee_currency} - rate: {fee_rate}") if fee_rate is None or fee_rate < 0.02: @@ -1780,7 +1781,9 @@ class FreqtradeBot(LoggingMixin): for exectrade in trades: amount += exectrade['amount'] if self.exchange.order_has_fee(exectrade): - fee_cost_, fee_currency, fee_rate_ = self.exchange.extract_cost_curr_rate(exectrade) + fee_cost_, fee_currency, fee_rate_ = self.exchange.extract_cost_curr_rate( + exectrade['fee'], exectrade['symbol'], exectrade['cost'], exectrade['amount'] + ) fee_cost += fee_cost_ if fee_rate_ is not None: fee_rate_array.append(fee_rate_) diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index 708a0e889..0dc1b1741 100644 --- a/tests/exchange/test_exchange.py +++ b/tests/exchange/test_exchange.py @@ -3544,7 +3544,7 @@ def test_order_has_fee(order, expected) -> None: def test_extract_cost_curr_rate(mocker, default_conf, order, expected) -> None: mocker.patch('freqtrade.exchange.Exchange.calculate_fee_rate', MagicMock(return_value=0.01)) ex = get_patched_exchange(mocker, default_conf) - assert ex.extract_cost_curr_rate(order) == expected + assert ex.extract_cost_curr_rate(order['fee'], order['symbol'], cost=20, amount=1) == expected @pytest.mark.parametrize("order,unknown_fee_rate,expected", [ @@ -3590,7 +3590,8 @@ def test_calculate_fee_rate(mocker, default_conf, order, expected, unknown_fee_r ex = get_patched_exchange(mocker, default_conf) - assert ex.calculate_fee_rate(order) == expected + assert ex.calculate_fee_rate(order['fee'], order['symbol'], + cost=order['cost'], amount=order['amount']) == expected @pytest.mark.parametrize('retrycount,max_retries,expected', [ From 81f7d77d7440e8a64d3ff2bf0b7b8ef7eb0ff7a0 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 7 Jul 2022 07:08:49 +0200 Subject: [PATCH 2/7] Allow fee currency to be empty for futures --- freqtrade/exchange/exchange.py | 22 +++++++++++++--------- tests/exchange/test_exchange.py | 6 ++++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index ced1c71fb..cdfd8aa79 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -1615,8 +1615,7 @@ class Exchange: except ccxt.BaseError as e: raise OperationalException(e) from e - @staticmethod - def order_has_fee(order: Dict) -> bool: + def order_has_fee(self, order: Dict) -> bool: """ Verifies if the passed in order dict has the needed keys to extract fees, and that these keys (currency, cost) are not empty. @@ -1627,7 +1626,8 @@ class Exchange: return False return ('fee' in order and order['fee'] is not None and (order['fee'].keys() >= {'currency', 'cost'}) - and order['fee']['currency'] is not None + and (order['fee']['currency'] is not None + or self.trading_mode == TradingMode.FUTURES) and order['fee']['cost'] is not None ) @@ -1642,16 +1642,20 @@ class Exchange: """ if fee.get('rate') is not None: return fee.get('rate') - fee_curr = fee['currency'] + fee_curr = fee.get('currency') + if fee_curr is None: + # Auto-currency only in futures mode + if self.trading_mode == TradingMode.FUTURES: + fee_curr = self.get_pair_quote_currency(symbol) + else: + return None # Calculate fee based on order details - if fee_curr in self.get_pair_base_currency(symbol): + if fee_curr == self.get_pair_base_currency(symbol): # Base currency - divide by amount return round(fee['cost'] / amount, 8) - elif fee_curr in self.get_pair_quote_currency(symbol): + elif fee_curr == self.get_pair_quote_currency(symbol): # Quote currency - divide by cost - return round(self._contracts_to_amount( - symbol, fee['cost']) / cost, - 8) if cost else None + return round(self._contracts_to_amount(symbol, fee['cost']) / cost, 8) if cost else None else: # If Fee currency is a different currency if not cost: diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index 0dc1b1741..f1d430437 100644 --- a/tests/exchange/test_exchange.py +++ b/tests/exchange/test_exchange.py @@ -3529,8 +3529,10 @@ def test_market_is_active(market, expected_result) -> None: ({'fee': {'currency': 'ETH/BTC', 'cost': None}}, False), ({'fee': {'currency': 'ETH/BTC', 'cost': 0.01}}, True), ]) -def test_order_has_fee(order, expected) -> None: - assert Exchange.order_has_fee(order) == expected +def test_order_has_fee(mocker, default_conf, order, expected) -> None: + ex = get_patched_exchange(mocker, default_conf) + + assert ex.order_has_fee(order) == expected @pytest.mark.parametrize("order,expected", [ From 5b733a723df398e67a9e2fb8d233cbfbe0b0389f Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 7 Jul 2022 19:40:16 +0200 Subject: [PATCH 3/7] use "fees" for trades responses --- freqtrade/exchange/exchange.py | 6 +----- freqtrade/freqtradebot.py | 9 ++++++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index cdfd8aa79..c192ca830 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -1644,11 +1644,7 @@ class Exchange: return fee.get('rate') fee_curr = fee.get('currency') if fee_curr is None: - # Auto-currency only in futures mode - if self.trading_mode == TradingMode.FUTURES: - fee_curr = self.get_pair_quote_currency(symbol) - else: - return None + return None # Calculate fee based on order details if fee_curr == self.get_pair_base_currency(symbol): # Base currency - divide by amount diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index e0532a17d..3fd134078 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1777,12 +1777,19 @@ class FreqtradeBot(LoggingMixin): fee_abs = 0.0 fee_cost = 0.0 trade_base_currency = self.exchange.get_pair_base_currency(trade.pair) + fee_rate_array: List[float] = [] for exectrade in trades: amount += exectrade['amount'] if self.exchange.order_has_fee(exectrade): + # Prefer singular fee + fees = [exectrade['fee']] + else: + fees = exectrade.get('fees', []) + for fee in fees: + fee_cost_, fee_currency, fee_rate_ = self.exchange.extract_cost_curr_rate( - exectrade['fee'], exectrade['symbol'], exectrade['cost'], exectrade['amount'] + fee, exectrade['symbol'], exectrade['cost'], exectrade['amount'] ) fee_cost += fee_cost_ if fee_rate_ is not None: From b7167ec88088a3541e6958233d98f8c4869053b3 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 9 Jul 2022 08:24:29 +0200 Subject: [PATCH 4/7] Fix wrong fee calclulation for gateio futures --- freqtrade/exchange/exchange.py | 11 ++++++++--- freqtrade/exchange/gateio.py | 3 ++- freqtrade/exchange/okx.py | 1 + freqtrade/freqtradebot.py | 1 - freqtrade/persistence/trade_model.py | 2 +- tests/exchange/test_exchange.py | 3 +++ 6 files changed, 15 insertions(+), 6 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index c192ca830..fdf323582 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -77,6 +77,7 @@ class Exchange: "mark_ohlcv_price": "mark", "mark_ohlcv_timeframe": "8h", "ccxt_futures_name": "swap", + "fee_cost_in_contracts": False, # Fee cost needs contract conversion "needs_trading_fees": False, # use fetch_trading_fees to cache fees } _ft_has: Dict = {} @@ -1645,13 +1646,18 @@ class Exchange: fee_curr = fee.get('currency') if fee_curr is None: return None + fee_cost = fee['cost'] + if self._ft_has['fee_cost_in_contracts']: + # Convert cost via "contracts" conversion + fee_cost = self._contracts_to_amount(symbol, fee['cost']) + # Calculate fee based on order details if fee_curr == self.get_pair_base_currency(symbol): # Base currency - divide by amount return round(fee['cost'] / amount, 8) elif fee_curr == self.get_pair_quote_currency(symbol): # Quote currency - divide by cost - return round(self._contracts_to_amount(symbol, fee['cost']) / cost, 8) if cost else None + return round(fee_cost / cost, 8) if cost else None else: # If Fee currency is a different currency if not cost: @@ -1666,8 +1672,7 @@ class Exchange: fee_to_quote_rate = self._config['exchange'].get('unknown_fee_rate', None) if not fee_to_quote_rate: return None - return round((self._contracts_to_amount( - symbol, fee['cost']) * fee_to_quote_rate) / cost, 8) + return round((fee_cost * fee_to_quote_rate) / cost, 8) def extract_cost_curr_rate(self, fee: Dict, symbol: str, cost: float, amount: float) -> Tuple[float, str, Optional[float]]: diff --git a/freqtrade/exchange/gateio.py b/freqtrade/exchange/gateio.py index bf50167da..b9de212de 100644 --- a/freqtrade/exchange/gateio.py +++ b/freqtrade/exchange/gateio.py @@ -32,7 +32,8 @@ class Gateio(Exchange): } _ft_has_futures: Dict = { - "needs_trading_fees": True + "needs_trading_fees": True, + "fee_cost_in_contracts": False, # Set explicitly to false for clarity } _supported_trading_mode_margin_pairs: List[Tuple[TradingMode, MarginMode]] = [ diff --git a/freqtrade/exchange/okx.py b/freqtrade/exchange/okx.py index 012f51080..afd7a672f 100644 --- a/freqtrade/exchange/okx.py +++ b/freqtrade/exchange/okx.py @@ -28,6 +28,7 @@ class Okx(Exchange): } _ft_has_futures: Dict = { "tickers_have_quoteVolume": False, + "fee_cost_in_contracts": True, } _supported_trading_mode_margin_pairs: List[Tuple[TradingMode, MarginMode]] = [ diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 3fd134078..469bfda7e 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1777,7 +1777,6 @@ class FreqtradeBot(LoggingMixin): fee_abs = 0.0 fee_cost = 0.0 trade_base_currency = self.exchange.get_pair_base_currency(trade.pair) - fee_rate_array: List[float] = [] for exectrade in trades: amount += exectrade['amount'] diff --git a/freqtrade/persistence/trade_model.py b/freqtrade/persistence/trade_model.py index 324002685..5f302de71 100644 --- a/freqtrade/persistence/trade_model.py +++ b/freqtrade/persistence/trade_model.py @@ -821,7 +821,7 @@ class LocalTrade(): self.open_rate = total_stake / total_amount self.stake_amount = total_stake / (self.leverage or 1.0) self.amount = total_amount - self.fee_open_cost = self.fee_open * self.stake_amount + self.fee_open_cost = self.fee_open * total_stake self.recalc_open_trade_value() if self.stop_loss_pct is not None and self.open_rate is not None: self.adjust_stop_loss(self.open_rate, self.stop_loss_pct) diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index f1d430437..6fce0bb8f 100644 --- a/tests/exchange/test_exchange.py +++ b/tests/exchange/test_exchange.py @@ -3584,6 +3584,9 @@ def test_extract_cost_curr_rate(mocker, default_conf, order, expected) -> None: 'fee': {'currency': 'POINT', 'cost': 2.0, 'rate': None}}, 1, 4.0), ({'symbol': 'POINT/BTC', 'amount': 0.04, 'cost': 0.5, 'fee': {'currency': 'POINT', 'cost': 2.0, 'rate': None}}, 2, 8.0), + # Missing currency + ({'symbol': 'ETH/BTC', 'amount': 0.04, 'cost': 0.05, + 'fee': {'currency': None, 'cost': 0.005}}, None, None), ]) def test_calculate_fee_rate(mocker, default_conf, order, expected, unknown_fee_rate) -> None: mocker.patch('freqtrade.exchange.Exchange.fetch_ticker', return_value={'last': 0.081}) From c98e7ea0558f1093b4ebdeecb8755ff3f60b58fc Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 9 Jul 2022 08:57:15 +0200 Subject: [PATCH 5/7] Revert allowing empty currency for futures --- freqtrade/exchange/exchange.py | 6 +++--- tests/exchange/test_exchange.py | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index fdf323582..cd13964c4 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -1616,7 +1616,8 @@ class Exchange: except ccxt.BaseError as e: raise OperationalException(e) from e - def order_has_fee(self, order: Dict) -> bool: + @staticmethod + def order_has_fee(order: Dict) -> bool: """ Verifies if the passed in order dict has the needed keys to extract fees, and that these keys (currency, cost) are not empty. @@ -1627,8 +1628,7 @@ class Exchange: return False return ('fee' in order and order['fee'] is not None and (order['fee'].keys() >= {'currency', 'cost'}) - and (order['fee']['currency'] is not None - or self.trading_mode == TradingMode.FUTURES) + and order['fee']['currency'] is not None and order['fee']['cost'] is not None ) diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index 6fce0bb8f..acd48b3fd 100644 --- a/tests/exchange/test_exchange.py +++ b/tests/exchange/test_exchange.py @@ -3529,10 +3529,8 @@ def test_market_is_active(market, expected_result) -> None: ({'fee': {'currency': 'ETH/BTC', 'cost': None}}, False), ({'fee': {'currency': 'ETH/BTC', 'cost': 0.01}}, True), ]) -def test_order_has_fee(mocker, default_conf, order, expected) -> None: - ex = get_patched_exchange(mocker, default_conf) - - assert ex.order_has_fee(order) == expected +def test_order_has_fee(order, expected) -> None: + assert Exchange.order_has_fee(order) == expected @pytest.mark.parametrize("order,expected", [ From aab59a8cafef0b9236f6bd92a6f0c2b98c7f5232 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 9 Jul 2022 09:00:12 +0200 Subject: [PATCH 6/7] Bump ccxt to required version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 2ccadea30..693d408c6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ numpy==1.23.0 pandas==1.4.3 pandas-ta==0.3.14b -ccxt==1.89.96 +ccxt==1.90.38 # Pin cryptography for now due to rust build errors with piwheels cryptography==37.0.2 aiohttp==3.8.1 From ea5f41aa6d5905485c8ca5fbc33702bbfc1bd534 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 10 Jul 2022 09:06:19 +0200 Subject: [PATCH 7/7] Version bump ccxt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 693d408c6..2bce619d3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ numpy==1.23.0 pandas==1.4.3 pandas-ta==0.3.14b -ccxt==1.90.38 +ccxt==1.90.40 # Pin cryptography for now due to rust build errors with piwheels cryptography==37.0.2 aiohttp==3.8.1