Merge pull request #3259 from freqtrade/fix/filled

Fix handling of partially or non-filled timedout orders

freqtrade/freqtradebot.py

@@ -901,7 +901,8 @@ class FreqtradeBot:
         Buy timeout - cancel order
         :return: True if order was fully cancelled
         """
-        if order['status'] != 'canceled':
+        # 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,
                                                             trade.amount)
@@ -912,7 +913,10 @@ class FreqtradeBot:
 
         logger.info('Buy order %s for %s.', reason, trade)
 
-        if safe_value_fallback(corder, order, 'remaining', 'remaining') == order['amount']:
+        # 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)
@@ -924,8 +928,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 = filled_amount
         trade.stake_amount = trade.amount * trade.open_rate
         self.update_trade_state(trade, corder, trade.amount)
 

tests/conftest.py

@@ -878,6 +878,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 {

tests/test_freqtradebot.py

@@ -2313,19 +2313,41 @@ 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.0
+    limit_buy_order['status'] = 'open'
     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
 
+    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)
 
 
+@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},
@@ -2347,12 +2369,14 @@ 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
 
     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