From 649879192b6453520ffa550ba7eb509696989c8b Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 30 Sep 2022 15:12:57 +0200 Subject: [PATCH 1/7] Implement partial sell --- freqtrade/freqtradebot.py | 52 +++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 387bae534..67d734ce2 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1409,37 +1409,41 @@ class FreqtradeBot(LoggingMixin): :return: True if exit order was cancelled, false otherwise """ cancelled = False - # if trade is not partially completed, just cancel the order - if order['remaining'] == order['amount'] or order.get('filled') == 0.0: - if not self.exchange.check_order_canceled_empty(order): - try: - # if trade is not partially completed, just delete the order - co = self.exchange.cancel_order_with_result(trade.open_order_id, trade.pair, - trade.amount) - trade.update_order(co) - except InvalidOrderException: - logger.exception( - f"Could not cancel {trade.exit_side} order {trade.open_order_id}") - return False - logger.info('%s order %s for %s.', trade.exit_side.capitalize(), reason, trade) - else: - reason = constants.CANCEL_REASON['CANCELLED_ON_EXCHANGE'] - logger.info('%s order %s for %s.', trade.exit_side.capitalize(), reason, trade) - trade.update_order(order) + # Cancelled orders may have the status of 'canceled' or 'closed' + if order['status'] not in constants.NON_OPEN_EXCHANGE_STATES: + filled_val: float = order.get('filled', 0.0) or 0.0 + filled_rem_stake = trade.stake_amount - filled_val * trade.open_rate + minstake = self.exchange.get_min_pair_stake_amount( + trade.pair, trade.open_rate, self.strategy.stoploss) + # Double-check remaining amount + if filled_val > 0 and minstake and filled_rem_stake < minstake: + logger.warning( + f"Order {trade.open_order_id} for {trade.pair} not cancelled, " + f"as the filled amount of {filled_val} would result in an unexitable trade.") + return False + try: + co = self.exchange.cancel_order_with_result(trade.open_order_id, trade.pair, + trade.amount) + except InvalidOrderException: + logger.exception( + f"Could not cancel {trade.exit_side} order {trade.open_order_id}") + return False trade.close_rate = None trade.close_rate_requested = None trade.close_profit = None trade.close_profit_abs = None - trade.open_order_id = None trade.exit_reason = None - cancelled = True - self.wallets.update() - else: - # TODO: figure out how to handle partially complete sell orders - reason = constants.CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN'] - cancelled = False + self.update_trade_state(trade, trade.open_order_id, co) + logger.info(f'{trade.exit_side.capitalize()} order {reason} for {trade}.') + cancelled = True + else: + reason = constants.CANCEL_REASON['CANCELLED_ON_EXCHANGE'] + logger.info(f'{trade.exit_side.capitalize()} order {reason} for {trade}.') + self.update_trade_state(trade, trade.open_order_id, order) + + self.wallets.update() order_obj = trade.select_order_by_order_id(order['id']) if not order_obj: raise DependencyException( From c946d30596c46c823ad02a738c072594e555f24d Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 30 Sep 2022 16:17:48 +0200 Subject: [PATCH 2/7] Add partial cancel message --- freqtrade/freqtradebot.py | 23 ++++++++++++++++++----- tests/test_freqtradebot.py | 16 +++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 67d734ce2..b98135fa5 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1416,11 +1416,24 @@ class FreqtradeBot(LoggingMixin): minstake = self.exchange.get_min_pair_stake_amount( trade.pair, trade.open_rate, self.strategy.stoploss) # Double-check remaining amount - if filled_val > 0 and minstake and filled_rem_stake < minstake: - logger.warning( - f"Order {trade.open_order_id} for {trade.pair} not cancelled, " - f"as the filled amount of {filled_val} would result in an unexitable trade.") - return False + if filled_val > 0: + reason = constants.CANCEL_REASON['PARTIALLY_FILLED'] + if minstake and filled_rem_stake < minstake: + logger.warning( + f"Order {trade.open_order_id} for {trade.pair} not cancelled, " + f"as the filled amount of {filled_val} would result in an unexitable trade.") + reason = constants.CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN'] + + order_obj = trade.select_order_by_order_id(order['id']) + if not order_obj: + raise DependencyException( + f"Order_obj not found for {order['id']}. This should not have happened.") + self._notify_exit_cancel( + trade, + order_type=self.strategy.order_types['exit'], + reason=reason, order=order_obj, sub_trade=trade.amount != order['amount'] + ) + return False try: co = self.exchange.cancel_order_with_result(trade.open_order_id, trade.pair, diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 0f1a05ab4..415abbc10 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -3111,6 +3111,9 @@ def test_handle_cancel_exit_limit(mocker, default_conf_usdt, fee) -> None: cancel_order=cancel_order_mock, ) mocker.patch('freqtrade.exchange.Exchange.get_rate', return_value=0.245441) + mocker.patch('freqtrade.exchange.Exchange.get_min_pair_stake_amount', return_value=0.2) + + mocker.patch('freqtrade.freqtradebot.FreqtradeBot.handle_order_fee') freqtrade = FreqtradeBot(default_conf_usdt) @@ -3178,7 +3181,9 @@ def test_handle_cancel_exit_limit(mocker, default_conf_usdt, fee) -> None: send_msg_mock.reset_mock() + # Partial exit - below exit threshold order['amount'] = 2 + order['filled'] = 1.9 assert not freqtrade.handle_cancel_exit(trade, order, reason) # Assert cancel_order was not called (callcount remains unchanged) assert cancel_order_mock.call_count == 1 @@ -3188,12 +3193,21 @@ def test_handle_cancel_exit_limit(mocker, default_conf_usdt, fee) -> None: assert not freqtrade.handle_cancel_exit(trade, order, reason) - send_msg_mock.call_args_list[0][0][0]['reason'] = CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN'] + assert (send_msg_mock.call_args_list[0][0][0]['reason'] + == CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN']) # Message should not be iterated again assert trade.exit_order_status == CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN'] assert send_msg_mock.call_count == 1 + send_msg_mock.reset_mock() + + order['filled'] = 1 + assert freqtrade.handle_cancel_exit(trade, order, reason) + assert send_msg_mock.call_count == 1 + assert (send_msg_mock.call_args_list[0][0][0]['reason'] + == CANCEL_REASON['PARTIALLY_FILLED']) + def test_handle_cancel_exit_cancel_exception(mocker, default_conf_usdt) -> None: patch_RPCManager(mocker) From 819488c906a01cc25995d715c3da36656748518e Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 30 Sep 2022 16:59:23 +0200 Subject: [PATCH 3/7] Improve exit message wording --- freqtrade/freqtradebot.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index b98135fa5..37bf032fa 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1446,8 +1446,10 @@ class FreqtradeBot(LoggingMixin): trade.close_rate_requested = None trade.close_profit = None trade.close_profit_abs = None - trade.exit_reason = None + # Set exit_reason for fill message + trade.exit_reason = trade.exit_reason + f", {reason}" if trade.exit_reason else reason self.update_trade_state(trade, trade.open_order_id, co) + trade.exit_reason = None logger.info(f'{trade.exit_side.capitalize()} order {reason} for {trade}.') cancelled = True From 47ef99f5886ca1373256b19ccf1878a9abf4f9bc Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 30 Sep 2022 17:18:27 +0200 Subject: [PATCH 4/7] Simplify interface to notify_exit_cancel --- freqtrade/freqtradebot.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 37bf032fa..2b20e40fd 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1420,18 +1420,15 @@ class FreqtradeBot(LoggingMixin): reason = constants.CANCEL_REASON['PARTIALLY_FILLED'] if minstake and filled_rem_stake < minstake: logger.warning( - f"Order {trade.open_order_id} for {trade.pair} not cancelled, " - f"as the filled amount of {filled_val} would result in an unexitable trade.") + f"Order {trade.open_order_id} for {trade.pair} not cancelled, as " + f"the filled amount of {filled_val} would result in an unexitable trade.") reason = constants.CANCEL_REASON['PARTIALLY_FILLED_KEEP_OPEN'] - order_obj = trade.select_order_by_order_id(order['id']) - if not order_obj: - raise DependencyException( - f"Order_obj not found for {order['id']}. This should not have happened.") self._notify_exit_cancel( trade, order_type=self.strategy.order_types['exit'], - reason=reason, order=order_obj, sub_trade=trade.amount != order['amount'] + reason=reason, order_id=order['id'], + sub_trade=trade.amount != order['amount'] ) return False @@ -1459,16 +1456,11 @@ class FreqtradeBot(LoggingMixin): self.update_trade_state(trade, trade.open_order_id, order) self.wallets.update() - order_obj = trade.select_order_by_order_id(order['id']) - if not order_obj: - raise DependencyException( - f"Order_obj not found for {order['id']}. This should not have happened.") - sub_trade = order_obj.amount != trade.amount self._notify_exit_cancel( trade, order_type=self.strategy.order_types['exit'], - reason=reason, order=order_obj, sub_trade=sub_trade + reason=reason, order_id=order['id'], sub_trade=trade.amount != order['amount'] ) return cancelled @@ -1665,7 +1657,7 @@ class FreqtradeBot(LoggingMixin): self.rpc.send_msg(msg) def _notify_exit_cancel(self, trade: Trade, order_type: str, reason: str, - order: Order, sub_trade: bool = False) -> None: + order_id: str, sub_trade: bool = False) -> None: """ Sends rpc notification when a sell cancel occurred. """ @@ -1674,6 +1666,11 @@ class FreqtradeBot(LoggingMixin): else: trade.exit_order_status = reason + order = trade.select_order_by_order_id(order_id) + if not order: + raise DependencyException( + f"Order_obj not found for {order_id}. This should not have happened.") + profit_rate = trade.close_rate if trade.close_rate else trade.close_rate_requested profit_trade = trade.calc_profit(rate=profit_rate) current_rate = self.exchange.get_rate( From d0b8c8b1a0a1c39f062d4e756ac5c128302ae287 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 2 Oct 2022 08:45:41 +0200 Subject: [PATCH 5/7] improve invalid canceled order response handling --- freqtrade/exchange/exchange.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index 61a6efb45..5648d8716 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -1292,7 +1292,14 @@ class Exchange: order = self.fetch_order(order_id, pair) except InvalidOrderException: logger.warning(f"Could not fetch cancelled order {order_id}.") - order = {'id': order_id, 'fee': {}, 'status': 'canceled', 'amount': amount, 'info': {}} + order = { + 'id': order_id, + 'status': 'canceled', + 'amount': amount, + 'filled': 0.0, + 'fee': {}, + 'info': {} + } return order From 1727f99b58906420bd656f4ef08162ab98500d58 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 2 Oct 2022 18:11:52 +0200 Subject: [PATCH 6/7] Fix missing mock --- tests/test_freqtradebot.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 462857dd6..cdea772dc 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -3095,6 +3095,9 @@ def test_handle_cancel_enter_corder_empty(mocker, default_conf_usdt, limit_order cancel_order_mock.reset_mock() l_order['filled'] = 1.0 + order = deepcopy(l_order) + order['status'] = 'canceled' + mocker.patch('freqtrade.exchange.Exchange.fetch_order', return_value=order) assert not freqtrade.handle_cancel_enter(trade, l_order, reason) assert cancel_order_mock.call_count == 1 From ca22d857b7369c868214ffea456b322b159e7da2 Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 3 Oct 2022 18:09:53 +0200 Subject: [PATCH 7/7] Improve handling of trades that fail to cancel as they are closed --- freqtrade/freqtradebot.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 2b20e40fd..4ec9c34ce 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1444,9 +1444,14 @@ class FreqtradeBot(LoggingMixin): trade.close_profit = None trade.close_profit_abs = None # Set exit_reason for fill message + exit_reason_prev = trade.exit_reason trade.exit_reason = trade.exit_reason + f", {reason}" if trade.exit_reason else reason self.update_trade_state(trade, trade.open_order_id, co) - trade.exit_reason = None + # Order might be filled above in odd timing issues. + if co.get('status') in ('canceled', 'cancelled'): + trade.exit_reason = None + else: + trade.exit_reason = exit_reason_prev logger.info(f'{trade.exit_side.capitalize()} order {reason} for {trade}.') cancelled = True