Improve filling logic

freqtrade/exchange/exchange.py

@@ -587,13 +587,15 @@ class Exchange:
                 'average': average,
                 'cost': dry_order['amount'] * average,
             })
-            self.add_dry_order_fee(pair, dry_order)
+            dry_order = self.add_dry_order_fee(pair, dry_order)
+
+        dry_order = self.check_dry_limit_order_filled(dry_order)
 
         self._dry_run_open_orders[dry_order["id"]] = dry_order
         # Copy order and close it - so the returned order is open unless it's a market order
         return dry_order
 
-    def add_dry_order_fee(self, pair: str, dry_order: Dict[str, Any]):
+    def add_dry_order_fee(self, pair: str, dry_order: Dict[str, Any]) -> Dict[str, Any]:
         dry_order.update({
             'fee': {
                 'currency': self.get_pair_quote_currency(pair),
@@ -601,6 +603,7 @@ class Exchange:
                 'rate': self.get_fee(pair)
             }
         })
+        return dry_order
 
     def get_dry_market_fill_price(self, pair: str, side: str, amount: float, rate: float) -> float:
         """
@@ -631,7 +634,7 @@ class Exchange:
 
         return rate
 
-    def dry_limit_order_filled(self, pair: str, side: str, limit: float) -> bool:
+    def _is_dry_limit_order_filled(self, pair: str, side: str, limit: float) -> bool:
         if not self.exchange_has('fetchL2OrderBook'):
             return True
         ob = self.fetch_l2_order_book(pair, 1)
@@ -647,6 +650,22 @@ class Exchange:
                 return True
         return False
 
+    def check_dry_limit_order_filled(self, order: Dict[str, Any]) -> Dict[str, Any]:
+        """
+        Check dry-run limit order fill and update fee (if it filled).
+        """
+        if order['status'] != "closed" and order['type'] in ["limit"]:
+            pair = order['symbol']
+            if self._is_dry_limit_order_filled(pair, order['side'], order['price']):
+                order.update({
+                    'status': 'closed',
+                    'filled': order['amount'],
+                    'remaining': 0,
+                })
+                self.add_dry_order_fee(pair, order)
+
+        return order
+
     def fetch_dry_run_order(self, order_id) -> Dict[str, Any]:
         """
         Return dry-run order
@@ -654,16 +673,7 @@ class Exchange:
         """
         try:
             order = self._dry_run_open_orders[order_id]
-            pair = order['symbol']
+            order = self.check_dry_limit_order_filled(order)
-            if order['status'] != "closed" and order['type'] in ["limit"]:
-                if self.dry_limit_order_filled(pair, order['side'], order['price']):
-                    order.update({
-                        'status': 'closed',
-                        'filled': order['amount'],
-                        'remaining': 0,
-                    })
-                    self.add_dry_order_fee(pair, order)
-
             return order
         except KeyError as e:
             # Gracefully handle errors with dry-run orders.

tests/exchange/test_exchange.py

@@ -963,11 +963,13 @@ def test_create_dry_run_order_limit_fill(default_conf, mocker, side, startprice,
 
     order = exchange.create_dry_run_order(
         pair='LTC/USDT', ordertype='limit', side=side, amount=1, rate=startprice)
+    assert order_book_l2_usd.call_count == 1
     assert 'id' in order
     assert f'dry_run_{side}_' in order["id"]
     assert order["side"] == side
     assert order["type"] == "limit"
     assert order["symbol"] == "LTC/USDT"
+    order_book_l2_usd.reset_mock()
 
     order_closed = exchange.fetch_dry_run_order(order['id'])
     assert order_book_l2_usd.call_count == 1
@@ -2181,7 +2183,7 @@ def test_get_historic_trades_notsupported(default_conf, mocker, caplog, exchange
 def test_cancel_order_dry_run(default_conf, mocker, exchange_name):
     default_conf['dry_run'] = True
     exchange = get_patched_exchange(mocker, default_conf, id=exchange_name)
-    mocker.patch('freqtrade.exchange.Exchange.dry_limit_order_filled', return_value=True)
+    mocker.patch('freqtrade.exchange.Exchange._is_dry_limit_order_filled', return_value=True)
     assert exchange.cancel_order(order_id='123', pair='TKN/BTC') == {}
     assert exchange.cancel_stoploss_order(order_id='123', pair='TKN/BTC') == {}
 

tests/rpc/test_rpc.py

@@ -679,6 +679,7 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
                 'filled': 0.0,
             }
         ),
+        _is_dry_limit_order_filled=MagicMock(return_value=True),
         get_fee=fee,
     )
     mocker.patch('freqtrade.wallets.Wallets.get_free', return_value=1000)
@@ -703,8 +704,8 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
     assert msg == {'result': 'Created sell orders for all open trades.'}
 
     freqtradebot.enter_positions()
-    msg = rpc._rpc_forcesell('1')
+    msg = rpc._rpc_forcesell('2')
-    assert msg == {'result': 'Created sell order for trade 1.'}
+    assert msg == {'result': 'Created sell order for trade 2.'}
 
     freqtradebot.state = State.STOPPED
     with pytest.raises(RPCException, match=r'.*trader is not running*'):
@@ -715,9 +716,11 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
 
     freqtradebot.state = State.RUNNING
     assert cancel_order_mock.call_count == 0
+    mocker.patch(
+        'freqtrade.exchange.Exchange._is_dry_limit_order_filled', MagicMock(return_value=False))
     freqtradebot.enter_positions()
     # make an limit-buy open trade
-    trade = Trade.query.filter(Trade.id == '1').first()
+    trade = Trade.query.filter(Trade.id == '3').first()
     filled_amount = trade.amount / 2
     # Fetch order - it's open first, and closed after cancel_order is called.
     mocker.patch(
@@ -738,7 +741,7 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
     )
     # check that the trade is called, which is done by ensuring exchange.cancel_order is called
     # and trade amount is updated
-    rpc._rpc_forcesell('1')
+    rpc._rpc_forcesell('3')
     assert cancel_order_mock.call_count == 1
     assert trade.amount == filled_amount
 
@@ -766,8 +769,8 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
         }
     )
     # check that the trade is called, which is done by ensuring exchange.cancel_order is called
-    msg = rpc._rpc_forcesell('2')
+    msg = rpc._rpc_forcesell('4')
-    assert msg == {'result': 'Created sell order for trade 2.'}
+    assert msg == {'result': 'Created sell order for trade 4.'}
     assert cancel_order_mock.call_count == 2
     assert trade.amount == amount
 

tests/rpc/test_rpc_apiserver.py

@@ -997,7 +997,7 @@ def test_api_forcesell(botclient, mocker, ticker, fee, markets):
         fetch_ticker=ticker,
         get_fee=fee,
         markets=PropertyMock(return_value=markets),
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     patch_get_signal(ftbot, (True, False))
 

tests/rpc/test_rpc_telegram.py

@@ -225,7 +225,7 @@ def test_status_handle(default_conf, update, ticker, fee, mocker) -> None:
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(return_value=True),
     )
     status_table = MagicMock()
     mocker.patch.multiple(
@@ -672,7 +672,7 @@ def test_telegram_forcesell_handle(default_conf, update, ticker, fee,
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(return_value=True),
     )
 
     freqtradebot = FreqtradeBot(default_conf)
@@ -731,7 +731,7 @@ def test_telegram_forcesell_down_handle(default_conf, update, ticker, fee,
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(return_value=True),
     )
 
     freqtradebot = FreqtradeBot(default_conf)
@@ -792,7 +792,7 @@ def test_forcesell_all_handle(default_conf, update, ticker, fee, mocker) -> None
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(return_value=True),
     )
     default_conf['max_open_trades'] = 4
     freqtradebot = FreqtradeBot(default_conf)
@@ -809,9 +809,9 @@ def test_forcesell_all_handle(default_conf, update, ticker, fee, mocker) -> None
     context.args = ["all"]
     telegram._forcesell(update=update, context=context)
 
-    # Called for each trade 4 times
+    # Called for each trade 2 times
-    assert msg_mock.call_count == 12
+    assert msg_mock.call_count == 8
-    msg = msg_mock.call_args_list[2][0][0]
+    msg = msg_mock.call_args_list[1][0][0]
     assert {
         'type': RPCMessageType.SELL,
         'trade_id': 1,

tests/test_freqtradebot.py

@@ -305,6 +305,7 @@ def test_total_open_trades_stakes(mocker, default_conf, ticker, fee) -> None:
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     freqtrade = FreqtradeBot(default_conf)
     patch_get_signal(freqtrade)
@@ -334,6 +335,7 @@ def test_create_trade(default_conf, ticker, limit_buy_order, fee, mocker) -> Non
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
 
     # Save state of current whitelist
@@ -2533,6 +2535,7 @@ def test_execute_sell_up(default_conf, ticker, fee, ticker_sell_up, mocker) -> N
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     patch_whitelist(mocker, default_conf)
     freqtrade = FreqtradeBot(default_conf)
@@ -2596,6 +2599,7 @@ def test_execute_sell_down(default_conf, ticker, fee, ticker_sell_down, mocker)
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     patch_whitelist(mocker, default_conf)
     freqtrade = FreqtradeBot(default_conf)
@@ -2648,6 +2652,7 @@ def test_execute_sell_down_stoploss_on_exchange_dry_run(default_conf, ticker, fe
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     patch_whitelist(mocker, default_conf)
     freqtrade = FreqtradeBot(default_conf)
@@ -2750,7 +2755,7 @@ def test_execute_sell_with_stoploss_on_exchange(default_conf, ticker, fee, ticke
         price_to_precision=lambda s, x, y: y,
         stoploss=stoploss,
         cancel_stoploss_order=cancel_order,
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(side_effect=[True, False]),
     )
 
     freqtrade = FreqtradeBot(default_conf)
@@ -2793,7 +2798,7 @@ def test_may_execute_sell_after_stoploss_on_exchange_hit(default_conf, ticker, f
         get_fee=fee,
         amount_to_precision=lambda s, x, y: y,
         price_to_precision=lambda s, x, y: y,
-        dry_limit_order_filled=MagicMock(return_value=True),
+        _is_dry_limit_order_filled=MagicMock(side_effect=[False, True]),
     )
 
     stoploss = MagicMock(return_value={
@@ -2862,6 +2867,7 @@ def test_execute_sell_market_order(default_conf, ticker, fee,
         'freqtrade.exchange.Exchange',
         fetch_ticker=ticker,
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     patch_whitelist(mocker, default_conf)
     freqtrade = FreqtradeBot(default_conf)
@@ -3467,6 +3473,7 @@ def test_disable_ignore_roi_if_buy_signal(default_conf, limit_buy_order, limit_b
         }),
         buy=MagicMock(return_value=limit_buy_order_open),
         get_fee=fee,
+        _is_dry_limit_order_filled=MagicMock(return_value=False),
     )
     default_conf['ask_strategy'] = {
         'ignore_roi_if_buy_signal': False