From 5978b7bb93ca46c8d8e62f0168c7f43e7efc690d Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 26 Sep 2019 06:26:25 +0200 Subject: [PATCH 1/4] Add explicit test for halfbought fee adjustment --- tests/test_freqtradebot.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index c26186e9d..45a2960e1 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -3205,6 +3205,30 @@ def test_get_real_amount_invalid_order(default_conf, trades_for_order, buy_order assert freqtrade.get_real_amount(trade, limit_buy_order) == amount +def test_get_real_amount_wrong_amount(default_conf, trades_for_order, buy_order_fee, mocker): + limit_buy_order = deepcopy(buy_order_fee) + limit_buy_order['fee'] = {'cost': 0.004} + limit_buy_order['amount'] = limit_buy_order['amount'] + 0.1 + + patch_RPCManager(mocker) + patch_exchange(mocker) + mocker.patch('freqtrade.exchange.Exchange.get_trades_for_order', return_value=trades_for_order) + amount = float(sum(x['amount'] for x in trades_for_order)) + trade = Trade( + pair='LTC/ETH', + amount=amount, + exchange='binance', + open_rate=0.245441, + open_order_id="123456" + ) + freqtrade = FreqtradeBot(default_conf) + patch_get_signal(freqtrade) + + # Amount does not change + with pytest.raises(OperationalException, match=r"Half bought\? Amounts don't match"): + freqtrade.get_real_amount(trade, limit_buy_order) + + def test_get_real_amount_invalid(default_conf, trades_for_order, buy_order_fee, mocker): # Remove "Currency" from fee dict trades_for_order[0]['fee'] = {'cost': 0.008} From 49f0a721213cbdf1a6bca8c27b61e17432dbc4a8 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 26 Sep 2019 06:48:46 +0200 Subject: [PATCH 2/4] Add test for rounding error on fload aggregation --- freqtrade/constants.py | 1 + tests/test_freqtradebot.py | 55 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/freqtrade/constants.py b/freqtrade/constants.py index d7ace131c..abf43b24d 100644 --- a/freqtrade/constants.py +++ b/freqtrade/constants.py @@ -22,6 +22,7 @@ ORDERTYPE_POSSIBILITIES = ['limit', 'market'] ORDERTIF_POSSIBILITIES = ['gtc', 'fok', 'ioc'] AVAILABLE_PAIRLISTS = ['StaticPairList', 'VolumePairList'] DRY_RUN_WALLET = 999.9 +MATH_CLOSE_PREC = 1e-14 # Precision used for float comparisons TICKER_INTERVALS = [ '1m', '3m', '5m', '15m', '30m', diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 45a2960e1..dd535a0b0 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -4,6 +4,7 @@ import logging import time from copy import deepcopy +from math import isclose from unittest.mock import MagicMock, PropertyMock import arrow @@ -12,6 +13,7 @@ import requests from freqtrade import (DependencyException, InvalidOrderException, OperationalException, TemporaryError, constants) +from freqtrade.constants import MATH_CLOSE_PREC from freqtrade.data.dataprovider import DataProvider from freqtrade.freqtradebot import FreqtradeBot from freqtrade.persistence import Trade @@ -1635,6 +1637,31 @@ def test_update_trade_state_withorderdict(default_conf, trades_for_order, limit_ assert trade.amount == limit_buy_order['amount'] +def test_update_trade_state_withorderdict_rounding_fee(default_conf, trades_for_order, + limit_buy_order, mocker, caplog): + trades_for_order[0]['amount'] = limit_buy_order['amount'] + 1e-14 + mocker.patch('freqtrade.exchange.Exchange.get_trades_for_order', return_value=trades_for_order) + # get_order should not be called!! + mocker.patch('freqtrade.exchange.Exchange.get_order', MagicMock(side_effect=ValueError)) + patch_exchange(mocker) + Trade.session = MagicMock() + amount = sum(x['amount'] for x in trades_for_order) + freqtrade = get_patched_freqtradebot(mocker, default_conf) + trade = Trade( + pair='LTC/ETH', + amount=amount, + exchange='binance', + open_rate=0.245441, + open_order_id="123456", + is_open=True, + open_date=arrow.utcnow().datetime, + ) + freqtrade.update_trade_state(trade, limit_buy_order) + assert trade.amount != amount + assert trade.amount == limit_buy_order['amount'] + assert log_has_re(r'Applying fee on amount for .*', caplog) + + def test_update_trade_state_exception(mocker, default_conf, limit_buy_order, caplog) -> None: freqtrade = get_patched_freqtradebot(mocker, default_conf) @@ -3207,8 +3234,7 @@ def test_get_real_amount_invalid_order(default_conf, trades_for_order, buy_order def test_get_real_amount_wrong_amount(default_conf, trades_for_order, buy_order_fee, mocker): limit_buy_order = deepcopy(buy_order_fee) - limit_buy_order['fee'] = {'cost': 0.004} - limit_buy_order['amount'] = limit_buy_order['amount'] + 0.1 + limit_buy_order['amount'] = limit_buy_order['amount'] - 0.001 patch_RPCManager(mocker) patch_exchange(mocker) @@ -3229,6 +3255,31 @@ def test_get_real_amount_wrong_amount(default_conf, trades_for_order, buy_order_ freqtrade.get_real_amount(trade, limit_buy_order) +def test_get_real_amount_wrong_amount_rounding(default_conf, trades_for_order, buy_order_fee, + mocker): + # Floats should not be compared directly. + limit_buy_order = deepcopy(buy_order_fee) + trades_for_order[0]['amount'] = trades_for_order[0]['amount'] + 1e-15 + + patch_RPCManager(mocker) + patch_exchange(mocker) + mocker.patch('freqtrade.exchange.Exchange.get_trades_for_order', return_value=trades_for_order) + amount = float(sum(x['amount'] for x in trades_for_order)) + trade = Trade( + pair='LTC/ETH', + amount=amount, + exchange='binance', + open_rate=0.245441, + open_order_id="123456" + ) + freqtrade = FreqtradeBot(default_conf) + patch_get_signal(freqtrade) + + # Amount changes by fee amount. + assert isclose(freqtrade.get_real_amount(trade, limit_buy_order), amount - (amount * 0.001), + rel_tol=MATH_CLOSE_PREC,) + + def test_get_real_amount_invalid(default_conf, trades_for_order, buy_order_fee, mocker): # Remove "Currency" from fee dict trades_for_order[0]['fee'] = {'cost': 0.008} From 8d92f8b3624555bf50f8463ebe961653a1db2435 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 26 Sep 2019 07:04:56 +0200 Subject: [PATCH 3/4] Compare floats via isclose instead of == --- freqtrade/freqtradebot.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index f56b1f2ea..c6d3d0f44 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -6,6 +6,7 @@ import copy import logging import traceback from datetime import datetime +from math import isclose from typing import Any, Dict, List, Optional, Tuple import arrow @@ -510,7 +511,7 @@ class FreqtradeBot: trade.pair.startswith(exectrade['fee']['currency'])): fee_abs += exectrade['fee']['cost'] - if amount != order_amount: + if not isclose(amount, order_amount, rel_tol=constants.MATH_CLOSE_PREC): logger.warning(f"Amount {amount} does not match amount {trade.amount}") raise OperationalException("Half bought? Amounts don't match") real_amount = amount - fee_abs @@ -535,7 +536,7 @@ class FreqtradeBot: # Try update amount (binance-fix) try: new_amount = self.get_real_amount(trade, order) - if order['amount'] != new_amount: + if not isclose(order['amount'], new_amount, rel_tol=constants.MATH_CLOSE_PREC): order['amount'] = new_amount # Fee was applied, so set to 0 trade.fee_open = 0 From 43f2ef226c969894d6a9a3487a52eeef95f7f5a7 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 28 Sep 2019 10:30:12 +0200 Subject: [PATCH 4/4] Change rel_tol to abs_tol to avoid surprises with high priced pairs --- freqtrade/freqtradebot.py | 4 ++-- tests/test_freqtradebot.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index c6d3d0f44..3f7eab27a 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -511,7 +511,7 @@ class FreqtradeBot: trade.pair.startswith(exectrade['fee']['currency'])): fee_abs += exectrade['fee']['cost'] - if not isclose(amount, order_amount, rel_tol=constants.MATH_CLOSE_PREC): + if not isclose(amount, order_amount, abs_tol=constants.MATH_CLOSE_PREC): logger.warning(f"Amount {amount} does not match amount {trade.amount}") raise OperationalException("Half bought? Amounts don't match") real_amount = amount - fee_abs @@ -536,7 +536,7 @@ class FreqtradeBot: # Try update amount (binance-fix) try: new_amount = self.get_real_amount(trade, order) - if not isclose(order['amount'], new_amount, rel_tol=constants.MATH_CLOSE_PREC): + if not isclose(order['amount'], new_amount, abs_tol=constants.MATH_CLOSE_PREC): order['amount'] = new_amount # Fee was applied, so set to 0 trade.fee_open = 0 diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index dd535a0b0..ee28f2e58 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -3277,7 +3277,7 @@ def test_get_real_amount_wrong_amount_rounding(default_conf, trades_for_order, b # Amount changes by fee amount. assert isclose(freqtrade.get_real_amount(trade, limit_buy_order), amount - (amount * 0.001), - rel_tol=MATH_CLOSE_PREC,) + abs_tol=MATH_CLOSE_PREC,) def test_get_real_amount_invalid(default_conf, trades_for_order, buy_order_fee, mocker):