Merge pull request #3724 from freqtrade/fix/3084

Forcesell should use the available methods for handling a trade correctly

freqtrade/constants.py

@@ -338,9 +338,12 @@ SCHEMA_MINIMAL_REQUIRED = [
 
 CANCEL_REASON = {
     "TIMEOUT": "cancelled due to timeout",
-    "PARTIALLY_FILLED": "partially filled - keeping order open",
+    "PARTIALLY_FILLED_KEEP_OPEN": "partially filled - keeping order open",
+    "PARTIALLY_FILLED": "partially filled",
+    "FULLY_CANCELLED": "fully cancelled",
     "ALL_CANCELLED": "cancelled (all unfilled and partially filled open orders cancelled)",
     "CANCELLED_ON_EXCHANGE": "cancelled on exchange",
+    "FORCE_SELL": "forcesold",
 }
 
 # List of pairs with their timeframes

freqtrade/exchange/exchange.py

@@ -973,6 +973,11 @@ class Exchange:
     @retrier
     def cancel_order(self, order_id: str, pair: str) -> Dict:
         if self._config['dry_run']:
+            order = self._dry_run_open_orders.get(order_id)
+            if order:
+                order.update({'status': 'canceled', 'filled': 0.0, 'remaining': order['amount']})
+                return order
+            else:
                 return {}
 
         try:

freqtrade/freqtradebot.py

@@ -618,7 +618,7 @@ class FreqtradeBot:
         # Send the message
         self.rpc.send_msg(msg)
 
-    def _notify_buy_cancel(self, trade: Trade, order_type: str) -> None:
+    def _notify_buy_cancel(self, trade: Trade, order_type: str, reason: str) -> None:
         """
         Sends rpc notification when a buy cancel occured.
         """
@@ -637,6 +637,7 @@ class FreqtradeBot:
             'amount': trade.amount,
             'open_date': trade.open_date,
             'current_rate': current_rate,
+            'reason': reason,
         }
 
         # Send the message
@@ -974,7 +975,6 @@ class FreqtradeBot:
 
         # Cancelled orders may have the status of 'canceled' or 'closed'
         if order['status'] not in ('canceled', 'closed'):
-            reason = constants.CANCEL_REASON['TIMEOUT']
             corder = self.exchange.cancel_order_with_result(trade.open_order_id, trade.pair,
                                                             trade.amount)
             # Avoid race condition where the order could not be cancelled coz its already filled.
@@ -992,13 +992,13 @@ class FreqtradeBot:
 
         # Using filled to determine the filled amount
         filled_amount = safe_value_fallback2(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)
             Trade.session.flush()
             was_trade_fully_canceled = True
+            reason += f", {constants.CANCEL_REASON['FULLY_CANCELLED']}"
         else:
             # if trade is partially complete, edit the stake details for the trade
             # and close the order
@@ -1011,13 +1011,11 @@ class FreqtradeBot:
 
             trade.open_order_id = None
             logger.info('Partial buy order timeout for %s.', trade)
-            self.rpc.send_msg({
+            reason += f", {constants.CANCEL_REASON['PARTIALLY_FILLED']}"
-                'type': RPCMessageType.STATUS_NOTIFICATION,
-                'status': f'Remaining buy order for {trade.pair} cancelled due to timeout'
-            })
 
         self.wallets.update()
-        self._notify_buy_cancel(trade, order_type=self.strategy.order_types['buy'])
+        self._notify_buy_cancel(trade, order_type=self.strategy.order_types['buy'],
+                                reason=reason)
         return was_trade_fully_canceled
 
     def handle_cancel_sell(self, trade: Trade, order: Dict, reason: str) -> str:
@@ -1048,7 +1046,7 @@ class FreqtradeBot:
             trade.open_order_id = None
         else:
             # TODO: figure out how to handle partially complete sell orders
-            reason = constants.CANCEL_REASON['PARTIALLY_FILLED']
+            reason = constants.CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN']
 
         self.wallets.update()
         self._notify_sell_cancel(

freqtrade/rpc/rpc.py

@@ -11,6 +11,7 @@ from typing import Any, Dict, List, Optional, Tuple, Union
 import arrow
 from numpy import NAN, mean
 
+from freqtrade.constants import CANCEL_REASON
 from freqtrade.exceptions import ExchangeError, PricingError
 from freqtrade.exchange import timeframe_to_minutes, timeframe_to_msecs
 from freqtrade.loggers import bufferHandler
@@ -453,26 +454,19 @@ class RPC:
         """
         def _exec_forcesell(trade: Trade) -> None:
             # Check if there is there is an open order
+            fully_canceled = False
             if trade.open_order_id:
                 order = self._freqtrade.exchange.fetch_order(trade.open_order_id, trade.pair)
 
-                # Cancel open LIMIT_BUY orders and close trade
+                if order['side'] == 'buy':
-                if order and order['status'] == 'open' \
+                    fully_canceled = self._freqtrade.handle_cancel_buy(
-                        and order['type'] == 'limit' \
+                        trade, order, CANCEL_REASON['FORCE_SELL'])
-                        and order['side'] == 'buy':
-                    self._freqtrade.exchange.cancel_order(trade.open_order_id, trade.pair)
-                    trade.close(order.get('price') or trade.open_rate)
-                    # Do the best effort, if we don't know 'filled' amount, don't try selling
-                    if order['filled'] is None:
-                        return
-                    trade.amount = order['filled']
 
-                # Ignore trades with an attached LIMIT_SELL order
+                if order['side'] == 'sell':
-                if order and order['status'] == 'open' \
+                    # Cancel order - so it is placed anew with a fresh price.
-                        and order['type'] == 'limit' \
+                    self._freqtrade.handle_cancel_sell(trade, order, CANCEL_REASON['FORCE_SELL'])
-                        and order['side'] == 'sell':
-                    return
 
+            if not fully_canceled:
                 # Get current rate and execute sell
                 current_rate = self._freqtrade.get_sell_rate(trade.pair, False)
                 self._freqtrade.execute_sell(trade, current_rate, SellType.FORCE_SELL)

freqtrade/rpc/telegram.py

@@ -151,7 +151,7 @@ class Telegram(RPC):
 
         elif msg['type'] == RPCMessageType.BUY_CANCEL_NOTIFICATION:
             message = ("\N{WARNING SIGN} *{exchange}:* "
-                       "Cancelling Open Buy Order for {pair}".format(**msg))
+                       "Cancelling open buy Order for {pair}. Reason: {reason}.".format(**msg))
 
         elif msg['type'] == RPCMessageType.SELL_NOTIFICATION:
             msg['amount'] = round(msg['amount'], 8)

tests/exchange/test_exchange.py

@@ -1761,6 +1761,14 @@ def test_cancel_order_dry_run(default_conf, mocker, exchange_name):
     assert exchange.cancel_order(order_id='123', pair='TKN/BTC') == {}
     assert exchange.cancel_stoploss_order(order_id='123', pair='TKN/BTC') == {}
 
+    order = exchange.buy('ETH/BTC', 'limit', 5, 0.55, 'gtc')
+
+    cancel_order = exchange.cancel_order(order_id=order['id'], pair='ETH/BTC')
+    assert order['id'] == cancel_order['id']
+    assert order['amount'] == cancel_order['amount']
+    assert order['pair'] == cancel_order['pair']
+    assert cancel_order['status'] == 'canceled'
+
 
 @pytest.mark.parametrize("exchange_name", EXCHANGES)
 @pytest.mark.parametrize("order,result", [

tests/rpc/test_rpc.py

@@ -669,7 +669,8 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
             return_value={
                 'status': 'closed',
                 'type': 'limit',
-                'side': 'buy'
+                'side': 'buy',
+                'filled': 0.0,
             }
         ),
         get_fee=fee,
@@ -695,6 +696,7 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
     msg = rpc._rpc_forcesell('all')
     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.'}
 
@@ -707,17 +709,24 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
 
     freqtradebot.state = State.RUNNING
     assert cancel_order_mock.call_count == 0
+    freqtradebot.enter_positions()
     # make an limit-buy open trade
     trade = Trade.query.filter(Trade.id == '1').first()
     filled_amount = trade.amount / 2
+    # Fetch order - it's open first, and closed after cancel_order is called.
     mocker.patch(
         'freqtrade.exchange.Exchange.fetch_order',
-        return_value={
+        side_effect=[{
             'status': 'open',
             'type': 'limit',
             'side': 'buy',
             'filled': filled_amount
-        }
+        }, {
+            'status': 'closed',
+            'type': 'limit',
+            'side': 'buy',
+            'filled': filled_amount
+        }]
     )
     # check that the trade is called, which is done by ensuring exchange.cancel_order is called
     # and trade amount is updated
@@ -725,6 +734,16 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
     assert cancel_order_mock.call_count == 1
     assert trade.amount == filled_amount
 
+    mocker.patch(
+        'freqtrade.exchange.Exchange.fetch_order',
+        return_value={
+            'status': 'open',
+            'type': 'limit',
+            'side': 'buy',
+            'filled': filled_amount
+        })
+
+    freqtradebot.config['max_open_trades'] = 3
     freqtradebot.enter_positions()
     trade = Trade.query.filter(Trade.id == '2').first()
     amount = trade.amount
@@ -744,20 +763,22 @@ def test_rpc_forcesell(default_conf, ticker, fee, mocker) -> None:
     assert cancel_order_mock.call_count == 2
     assert trade.amount == amount
 
-    freqtradebot.enter_positions()
     # make an limit-sell open trade
     mocker.patch(
         'freqtrade.exchange.Exchange.fetch_order',
         return_value={
             'status': 'open',
             'type': 'limit',
-            'side': 'sell'
+            'side': 'sell',
+            'amount': amount,
+            'remaining': amount,
+            'filled': 0.0
         }
     )
     msg = rpc._rpc_forcesell('3')
     assert msg == {'result': 'Created sell order for trade 3.'}
     # status quo, no exchange calls
-    assert cancel_order_mock.call_count == 2
+    assert cancel_order_mock.call_count == 3
 
 
 def test_performance_handle(default_conf, ticker, limit_buy_order, fee,

tests/rpc/test_rpc_telegram.py

@@ -14,6 +14,7 @@ from telegram import Chat, Message, Update
 from telegram.error import NetworkError
 
 from freqtrade import __version__
+from freqtrade.constants import CANCEL_REASON
 from freqtrade.edge import PairInfo
 from freqtrade.freqtradebot import FreqtradeBot
 from freqtrade.loggers import setup_logging
@@ -725,7 +726,7 @@ def test_telegram_forcesell_handle(default_conf, update, ticker, fee,
     context.args = ["1"]
     telegram._forcesell(update=update, context=context)
 
-    assert rpc_mock.call_count == 2
+    assert rpc_mock.call_count == 3
     last_msg = rpc_mock.call_args_list[-1][0][0]
     assert {
         'type': RPCMessageType.SELL_NOTIFICATION,
@@ -784,7 +785,7 @@ def test_telegram_forcesell_down_handle(default_conf, update, ticker, fee,
     context.args = ["1"]
     telegram._forcesell(update=update, context=context)
 
-    assert rpc_mock.call_count == 2
+    assert rpc_mock.call_count == 3
 
     last_msg = rpc_mock.call_args_list[-1][0][0]
     assert {
@@ -834,8 +835,9 @@ def test_forcesell_all_handle(default_conf, update, ticker, fee, mocker) -> None
     context.args = ["all"]
     telegram._forcesell(update=update, context=context)
 
-    assert rpc_mock.call_count == 4
+    # Called for each trade 3 times
-    msg = rpc_mock.call_args_list[0][0][0]
+    assert rpc_mock.call_count == 8
+    msg = rpc_mock.call_args_list[1][0][0]
     assert {
         'type': RPCMessageType.SELL_NOTIFICATION,
         'trade_id': 1,
@@ -1343,9 +1345,10 @@ def test_send_msg_buy_cancel_notification(default_conf, mocker) -> None:
         'type': RPCMessageType.BUY_CANCEL_NOTIFICATION,
         'exchange': 'Bittrex',
         'pair': 'ETH/BTC',
+        'reason': CANCEL_REASON['TIMEOUT']
     })
-    assert msg_mock.call_args[0][0] \
+    assert (msg_mock.call_args[0][0] == '\N{WARNING SIGN} *Bittrex:* '
-        == ('\N{WARNING SIGN} *Bittrex:* Cancelling Open Buy Order for ETH/BTC')
+            'Cancelling open buy Order for ETH/BTC. Reason: cancelled due to timeout.')
 
 
 def test_send_msg_sell_notification(default_conf, mocker) -> None:

tests/test_freqtradebot.py

@@ -2289,7 +2289,7 @@ def test_check_handle_timedout_partial(default_conf, ticker, limit_buy_order_old
     # note this is for a partially-complete buy order
     freqtrade.check_handle_timedout()
     assert cancel_order_mock.call_count == 1
-    assert rpc_mock.call_count == 2
+    assert rpc_mock.call_count == 1
     trades = Trade.query.filter(Trade.open_order_id.is_(open_trade.open_order_id)).all()
     assert len(trades) == 1
     assert trades[0].amount == 23.0
@@ -2324,7 +2324,7 @@ def test_check_handle_timedout_partial_fee(default_conf, ticker, open_trade, cap
     assert log_has_re(r"Applying fee on amount for Trade.*", caplog)
 
     assert cancel_order_mock.call_count == 1
-    assert rpc_mock.call_count == 2
+    assert rpc_mock.call_count == 1
     trades = Trade.query.filter(Trade.open_order_id.is_(open_trade.open_order_id)).all()
     assert len(trades) == 1
     # Verify that trade has been updated
@@ -2364,7 +2364,7 @@ def test_check_handle_timedout_partial_except(default_conf, ticker, open_trade,
     assert log_has_re(r"Could not update trade amount: .*", caplog)
 
     assert cancel_order_mock.call_count == 1
-    assert rpc_mock.call_count == 2
+    assert rpc_mock.call_count == 1
     trades = Trade.query.filter(Trade.open_order_id.is_(open_trade.open_order_id)).all()
     assert len(trades) == 1
     # Verify that trade has been updated
@@ -2527,13 +2527,15 @@ def test_handle_cancel_sell_limit(mocker, default_conf, fee) -> None:
     send_msg_mock.reset_mock()
 
     order['amount'] = 2
-    assert freqtrade.handle_cancel_sell(trade, order, reason) == CANCEL_REASON['PARTIALLY_FILLED']
+    assert freqtrade.handle_cancel_sell(trade, order, reason
+                                        ) == CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN']
     # Assert cancel_order was not called (callcount remains unchanged)
     assert cancel_order_mock.call_count == 1
     assert send_msg_mock.call_count == 1
-    assert freqtrade.handle_cancel_sell(trade, order, reason) == CANCEL_REASON['PARTIALLY_FILLED']
+    assert freqtrade.handle_cancel_sell(trade, order, reason
+                                        ) == CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN']
     # Message should not be iterated again
-    assert trade.sell_order_status == CANCEL_REASON['PARTIALLY_FILLED']
+    assert trade.sell_order_status == CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN']
     assert send_msg_mock.call_count == 1