From f040c2068850641bea7c04ff2b991dbaf728dfbb Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 5 May 2020 06:41:01 +0200 Subject: [PATCH 1/5] Use filled in tests --- tests/test_freqtradebot.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 6f2ce9f3c..a098709a6 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -2348,11 +2348,12 @@ def test_handle_timedout_limit_buy_corder_empty(mocker, default_conf, limit_buy_ trade = MagicMock() trade.pair = 'LTC/ETH' limit_buy_order['remaining'] = limit_buy_order['amount'] + limit_buy_order['filled'] = 0.0 assert freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 cancel_order_mock.reset_mock() - limit_buy_order['amount'] = 2 + limit_buy_order['filled'] = 1.0 assert not freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 From b4aeb93a183d53d456ce81bdd5f28b941b88aebe Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 5 May 2020 07:07:42 +0200 Subject: [PATCH 2/5] Add test testing the different ways exchanges may return data --- tests/conftest.py | 93 ++++++++++++++++++++++++++++++++++++++ tests/test_freqtradebot.py | 25 +++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d95475b8c..29bb36f64 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -877,6 +877,99 @@ def limit_buy_order_old_partial_canceled(limit_buy_order_old_partial): return res +@pytest.fixture(scope='function') +def limit_buy_order_canceled_empty(request): + # Indirect fixture + # Documentation: + # https://docs.pytest.org/en/latest/example/parametrize.html#apply-indirect-on-particular-arguments + + exchange_name = request.param + if exchange_name == 'ftx': + return { + 'info': {}, + 'id': '1234512345', + 'clientOrderId': None, + 'timestamp': arrow.utcnow().shift(minutes=-601).timestamp, + 'datetime': arrow.utcnow().shift(minutes=-601).isoformat(), + 'lastTradeTimestamp': None, + 'symbol': 'LTC/USDT', + 'type': 'limit', + 'side': 'buy', + 'price': 34.3225, + 'amount': 0.55, + 'cost': 0.0, + 'average': None, + 'filled': 0.0, + 'remaining': 0.0, + 'status': 'closed', + 'fee': None, + 'trades': None + } + elif exchange_name == 'kraken': + return { + 'info': {}, + 'id': 'AZNPFF-4AC4N-7MKTAT', + 'clientOrderId': None, + 'timestamp': arrow.utcnow().shift(minutes=-601).timestamp, + 'datetime': arrow.utcnow().shift(minutes=-601).isoformat(), + 'lastTradeTimestamp': None, + 'status': 'canceled', + 'symbol': 'LTC/USDT', + 'type': 'limit', + 'side': 'buy', + 'price': 34.3225, + 'cost': 0.0, + 'amount': 0.55, + 'filled': 0.0, + 'average': 0.0, + 'remaining': 0.55, + 'fee': {'cost': 0.0, 'rate': None, 'currency': 'USDT'}, + 'trades': [] + } + elif exchange_name == 'binance': + return { + 'info': {}, + 'id': '1234512345', + 'clientOrderId': 'alb1234123', + 'timestamp': arrow.utcnow().shift(minutes=-601).timestamp, + 'datetime': arrow.utcnow().shift(minutes=-601).isoformat(), + 'lastTradeTimestamp': None, + 'symbol': 'LTC/USDT', + 'type': 'limit', + 'side': 'buy', + 'price': 0.016804, + 'amount': 0.55, + 'cost': 0.0, + 'average': None, + 'filled': 0.0, + 'remaining': 0.55, + 'status': 'canceled', + 'fee': None, + 'trades': None + } + else: + return { + 'info': {}, + 'id': '1234512345', + 'clientOrderId': 'alb1234123', + 'timestamp': arrow.utcnow().shift(minutes=-601).timestamp, + 'datetime': arrow.utcnow().shift(minutes=-601).isoformat(), + 'lastTradeTimestamp': None, + 'symbol': 'LTC/USDT', + 'type': 'limit', + 'side': 'buy', + 'price': 0.016804, + 'amount': 0.55, + 'cost': 0.0, + 'average': None, + 'filled': 0.0, + 'remaining': 0.55, + 'status': 'canceled', + 'fee': None, + 'trades': None + } + + @pytest.fixture def limit_sell_order(): return { diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index a098709a6..2b5127cc0 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -2313,12 +2313,12 @@ def test_handle_timedout_limit_buy(mocker, caplog, default_conf, limit_buy_order Trade.session = MagicMock() trade = MagicMock() trade.pair = 'LTC/ETH' - limit_buy_order['remaining'] = limit_buy_order['amount'] + limit_buy_order['filled'] = 0 assert freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 cancel_order_mock.reset_mock() - limit_buy_order['amount'] = 2 + limit_buy_order['filled'] = 2 assert not freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 @@ -2326,6 +2326,27 @@ def test_handle_timedout_limit_buy(mocker, caplog, default_conf, limit_buy_order assert not freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) +@pytest.mark.parametrize("limit_buy_order_canceled_empty", ['binance', 'ftx', 'kraken', 'bittrex'], + indirect=['limit_buy_order_canceled_empty']) +def test_handle_timedout_limit_buy_exchanges(mocker, caplog, default_conf, + limit_buy_order_canceled_empty) -> None: + patch_RPCManager(mocker) + patch_exchange(mocker) + cancel_order_mock = mocker.patch( + 'freqtrade.exchange.Exchange.cancel_order_with_result', + return_value=limit_buy_order_canceled_empty) + + freqtrade = FreqtradeBot(default_conf) + + Trade.session = MagicMock() + trade = MagicMock() + trade.pair = 'LTC/ETH' + assert freqtrade.handle_timedout_limit_buy(trade, limit_buy_order_canceled_empty) + assert cancel_order_mock.call_count == 0 + assert log_has_re(r'Buy order fully cancelled. Removing .* from database\.', caplog) + + + @pytest.mark.parametrize('cancelorder', [ {}, {'remaining': None}, From 981976681a88423117ef9bd6fe18e56f96df4476 Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 5 May 2020 07:08:24 +0200 Subject: [PATCH 3/5] Use filled, it's the safer choice when determining the filled amount. --- freqtrade/freqtradebot.py | 7 +++---- tests/test_freqtradebot.py | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 30eebb962..9db7e4467 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -898,7 +898,7 @@ class FreqtradeBot: Buy timeout - cancel order :return: True if order was fully cancelled """ - if order['status'] != 'canceled': + if order['status'] not in ('canceled', 'closed'): reason = "cancelled due to timeout" corder = self.exchange.cancel_order_with_result(trade.open_order_id, trade.pair, trade.amount) @@ -909,7 +909,7 @@ class FreqtradeBot: logger.info('Buy order %s for %s.', reason, trade) - if safe_value_fallback(corder, order, 'remaining', 'remaining') == order['amount']: + if safe_value_fallback(corder, order, 'filled', 'filled') == 0.0: logger.info('Buy order fully cancelled. Removing %s from database.', trade) # if trade is not partially completed, just delete the trade Trade.session.delete(trade) @@ -921,8 +921,7 @@ class FreqtradeBot: # cancel_order may not contain the full order dict, so we need to fallback # to the order dict aquired before cancelling. # we need to fall back to the values from order if corder does not contain these keys. - trade.amount = order['amount'] - safe_value_fallback(corder, order, - 'remaining', 'remaining') + trade.amount = safe_value_fallback(corder, order, 'filled', 'filled') trade.stake_amount = trade.amount * trade.open_rate self.update_trade_state(trade, corder, trade.amount) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 2b5127cc0..f2b2f6b5c 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -2346,7 +2346,6 @@ def test_handle_timedout_limit_buy_exchanges(mocker, caplog, default_conf, assert log_has_re(r'Buy order fully cancelled. Removing .* from database\.', caplog) - @pytest.mark.parametrize('cancelorder', [ {}, {'remaining': None}, From d3a0ab8096bcd119ca3fd57c4880a8648783ee4d Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 5 May 2020 07:12:49 +0200 Subject: [PATCH 4/5] Change mock-status to be open when testing unfilled... --- tests/test_freqtradebot.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index f2b2f6b5c..7a30e9015 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -2314,6 +2314,7 @@ def test_handle_timedout_limit_buy(mocker, caplog, default_conf, limit_buy_order trade = MagicMock() trade.pair = 'LTC/ETH' limit_buy_order['filled'] = 0 + limit_buy_order['status'] = 'open' assert freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 @@ -2367,8 +2368,9 @@ def test_handle_timedout_limit_buy_corder_empty(mocker, default_conf, limit_buy_ Trade.session = MagicMock() trade = MagicMock() trade.pair = 'LTC/ETH' - limit_buy_order['remaining'] = limit_buy_order['amount'] limit_buy_order['filled'] = 0.0 + limit_buy_order['status'] = 'open' + assert freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 From 1ba2df79c6a770958bddb3bf1e1fee6bd8c41f0f Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 7 May 2020 06:51:02 +0200 Subject: [PATCH 5/5] Ause isclose for comparison, assign filled to variable add some comments --- freqtrade/freqtradebot.py | 8 ++++++-- tests/test_freqtradebot.py | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 9db7e4467..57eb7d1a8 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -898,6 +898,7 @@ class FreqtradeBot: Buy timeout - cancel order :return: True if order was fully cancelled """ + # Cancelled orders may have the status of 'canceled' or 'closed' if order['status'] not in ('canceled', 'closed'): reason = "cancelled due to timeout" corder = self.exchange.cancel_order_with_result(trade.open_order_id, trade.pair, @@ -909,7 +910,10 @@ class FreqtradeBot: logger.info('Buy order %s for %s.', reason, trade) - if safe_value_fallback(corder, order, 'filled', 'filled') == 0.0: + # Using filled to determine the filled amount + filled_amount = safe_value_fallback(corder, order, 'filled', 'filled') + + if isclose(filled_amount, 0.0, abs_tol=constants.MATH_CLOSE_PREC): logger.info('Buy order fully cancelled. Removing %s from database.', trade) # if trade is not partially completed, just delete the trade Trade.session.delete(trade) @@ -921,7 +925,7 @@ class FreqtradeBot: # cancel_order may not contain the full order dict, so we need to fallback # to the order dict aquired before cancelling. # we need to fall back to the values from order if corder does not contain these keys. - trade.amount = safe_value_fallback(corder, order, 'filled', 'filled') + trade.amount = filled_amount trade.stake_amount = trade.amount * trade.open_rate self.update_trade_state(trade, corder, trade.amount) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 7a30e9015..c502e264c 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -2313,7 +2313,7 @@ def test_handle_timedout_limit_buy(mocker, caplog, default_conf, limit_buy_order Trade.session = MagicMock() trade = MagicMock() trade.pair = 'LTC/ETH' - limit_buy_order['filled'] = 0 + limit_buy_order['filled'] = 0.0 limit_buy_order['status'] = 'open' assert freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 @@ -2323,6 +2323,7 @@ def test_handle_timedout_limit_buy(mocker, caplog, default_conf, limit_buy_order assert not freqtrade.handle_timedout_limit_buy(trade, limit_buy_order) assert cancel_order_mock.call_count == 1 + limit_buy_order['filled'] = 2 mocker.patch('freqtrade.exchange.Exchange.cancel_order', side_effect=InvalidOrderException) assert not freqtrade.handle_timedout_limit_buy(trade, limit_buy_order)