From c389d44e9ac35cdf1f3c4a903f8d89ca8bb1b6f3 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 5 Jun 2021 15:22:52 +0200 Subject: [PATCH] Improve filling logic --- freqtrade/exchange/exchange.py | 36 +++++++++++++++++++++------------ tests/exchange/test_exchange.py | 4 +++- tests/rpc/test_rpc.py | 15 ++++++++------ tests/rpc/test_rpc_apiserver.py | 2 +- tests/rpc/test_rpc_telegram.py | 14 ++++++------- tests/test_freqtradebot.py | 11 ++++++++-- 6 files changed, 52 insertions(+), 30 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index ea3a7d7cd..19b646a93 100644 --- a/freqtrade/exchange/exchange.py +++ b/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'] - 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) - + order = self.check_dry_limit_order_filled(order) return order except KeyError as e: # Gracefully handle errors with dry-run orders. diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index 2c4ddacb4..42bb07175 100644 --- a/tests/exchange/test_exchange.py +++ b/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') == {} diff --git a/tests/rpc/test_rpc.py b/tests/rpc/test_rpc.py index 7556dde6d..fd231b614 100644 --- a/tests/rpc/test_rpc.py +++ b/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') - assert msg == {'result': 'Created sell order for trade 1.'} + msg = rpc._rpc_forcesell('2') + 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') - assert msg == {'result': 'Created sell order for trade 2.'} + msg = rpc._rpc_forcesell('4') + assert msg == {'result': 'Created sell order for trade 4.'} assert cancel_order_mock.call_count == 2 assert trade.amount == amount diff --git a/tests/rpc/test_rpc_apiserver.py b/tests/rpc/test_rpc_apiserver.py index f47819568..f30825b7b 100644 --- a/tests/rpc/test_rpc_apiserver.py +++ b/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)) diff --git a/tests/rpc/test_rpc_telegram.py b/tests/rpc/test_rpc_telegram.py index 50c8a36ce..c933ac648 100644 --- a/tests/rpc/test_rpc_telegram.py +++ b/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 - assert msg_mock.call_count == 12 - msg = msg_mock.call_args_list[2][0][0] + # Called for each trade 2 times + assert msg_mock.call_count == 8 + msg = msg_mock.call_args_list[1][0][0] assert { 'type': RPCMessageType.SELL, 'trade_id': 1, diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 0866deead..4bcc578c4 100644 --- a/tests/test_freqtradebot.py +++ b/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