From bdb904e7147650be3634f15dc3875545f6e5374b Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 22 May 2022 10:15:58 +0200 Subject: [PATCH 1/5] Should_exit should return all sell signals --- freqtrade/freqtradebot.py | 15 ++++++++------- freqtrade/optimize/backtesting.py | 14 +++++++++++--- freqtrade/strategy/interface.py | 16 +++++++--------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 3dccb45e4..4ae55e31c 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1106,7 +1106,7 @@ class FreqtradeBot(LoggingMixin): """ Check and execute trade exit """ - should_exit: ExitCheckTuple = self.strategy.should_exit( + exits: List[ExitCheckTuple] = self.strategy.should_exit( trade, exit_rate, datetime.now(timezone.utc), @@ -1114,12 +1114,13 @@ class FreqtradeBot(LoggingMixin): exit_=exit_, force_stoploss=self.edge.stoploss(trade.pair) if self.edge else 0 ) - - if should_exit.exit_flag: - logger.info(f'Exit for {trade.pair} detected. Reason: {should_exit.exit_type}' - f'Tag: {exit_tag if exit_tag is not None else "None"}') - self.execute_trade_exit(trade, exit_rate, should_exit, exit_tag=exit_tag) - return True + for should_exit in exits: + if should_exit.exit_flag: + logger.info(f'Exit for {trade.pair} detected. Reason: {should_exit.exit_type}' + f'Tag: {exit_tag if exit_tag is not None else "None"}') + exited = self.execute_trade_exit(trade, exit_rate, should_exit, exit_tag=exit_tag) + if exited: + return True return False def manage_open_orders(self) -> None: diff --git a/freqtrade/optimize/backtesting.py b/freqtrade/optimize/backtesting.py index 4e604898f..4286f5b95 100755 --- a/freqtrade/optimize/backtesting.py +++ b/freqtrade/optimize/backtesting.py @@ -527,15 +527,23 @@ class Backtesting: if check_adjust_entry: trade = self._get_adjust_trade_entry_for_candle(trade, row) - exit_candle_time: datetime = row[DATE_IDX].to_pydatetime() enter = row[SHORT_IDX] if trade.is_short else row[LONG_IDX] exit_sig = row[ESHORT_IDX] if trade.is_short else row[ELONG_IDX] - exit_ = self.strategy.should_exit( - trade, row[OPEN_IDX], exit_candle_time, # type: ignore + exits = self.strategy.should_exit( + trade, row[OPEN_IDX], row[DATE_IDX].to_pydatetime(), # type: ignore enter=enter, exit_=exit_sig, low=row[LOW_IDX], high=row[HIGH_IDX] ) + for exit_ in exits: + t = self._get_exit_for_signal(trade, row, exit_) + if t: + return t + return None + def _get_exit_for_signal(self, trade: LocalTrade, row: Tuple, + exit_: ExitCheckTuple) -> Optional[LocalTrade]: + + exit_candle_time: datetime = row[DATE_IDX].to_pydatetime() if exit_.exit_flag: trade.close_date = exit_candle_time exit_reason = exit_.exit_reason diff --git a/freqtrade/strategy/interface.py b/freqtrade/strategy/interface.py index 57afbf32a..15627722c 100644 --- a/freqtrade/strategy/interface.py +++ b/freqtrade/strategy/interface.py @@ -878,16 +878,16 @@ class IStrategy(ABC, HyperStrategyMixin): def should_exit(self, trade: Trade, rate: float, current_time: datetime, *, enter: bool, exit_: bool, low: float = None, high: float = None, - force_stoploss: float = 0) -> ExitCheckTuple: + force_stoploss: float = 0) -> List[ExitCheckTuple]: """ This function evaluates if one of the conditions required to trigger an exit order has been reached, which can either be a stop-loss, ROI or exit-signal. :param low: Only used during backtesting to simulate (long)stoploss/(short)ROI :param high: Only used during backtesting, to simulate (short)stoploss/(long)ROI :param force_stoploss: Externally provided stoploss - :return: True if trade should be exited, False otherwise + :return: List of exit reasons - or empty list. """ - + exits: List[ExitCheckTuple] = [] current_rate = rate current_profit = trade.calc_profit_ratio(current_rate) @@ -938,7 +938,7 @@ class IStrategy(ABC, HyperStrategyMixin): logger.debug(f"{trade.pair} - Sell signal received. " f"exit_type=ExitType.{exit_signal.name}" + (f", custom_reason={custom_reason}" if custom_reason else "")) - return ExitCheckTuple(exit_type=exit_signal, exit_reason=custom_reason) + exits.append(ExitCheckTuple(exit_type=exit_signal, exit_reason=custom_reason)) # Sequence: # Exit-signal @@ -946,16 +946,14 @@ class IStrategy(ABC, HyperStrategyMixin): # Stoploss if roi_reached and stoplossflag.exit_type != ExitType.STOP_LOSS: logger.debug(f"{trade.pair} - Required profit reached. exit_type=ExitType.ROI") - return ExitCheckTuple(exit_type=ExitType.ROI) + exits.append(ExitCheckTuple(exit_type=ExitType.ROI)) if stoplossflag.exit_flag: logger.debug(f"{trade.pair} - Stoploss hit. exit_type={stoplossflag.exit_type}") - return stoplossflag + exits.append(stoplossflag) - # This one is noisy, commented out... - # logger.debug(f"{trade.pair} - No exit signal.") - return ExitCheckTuple(exit_type=ExitType.NONE) + return exits def stop_loss_reached(self, current_rate: float, trade: Trade, current_time: datetime, current_profit: float, From b7388557a9418c2ec8cb1310a2aea6e41acc5366 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 22 May 2022 10:20:01 +0200 Subject: [PATCH 2/5] Update interface tests --- tests/strategy/test_interface.py | 21 ++++++++++----------- tests/test_integration.py | 14 +++++++------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/tests/strategy/test_interface.py b/tests/strategy/test_interface.py index 6e57a3182..55cfcaf98 100644 --- a/tests/strategy/test_interface.py +++ b/tests/strategy/test_interface.py @@ -495,34 +495,33 @@ def test_custom_exit(default_conf, fee, caplog) -> None: enter=False, exit_=False, low=None, high=None) - assert res.exit_flag is False - assert res.exit_type == ExitType.NONE + assert res == [] strategy.custom_exit = MagicMock(return_value=True) res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, low=None, high=None) - assert res.exit_flag is True - assert res.exit_type == ExitType.CUSTOM_EXIT - assert res.exit_reason == 'custom_exit' + assert res[0].exit_flag is True + assert res[0].exit_type == ExitType.CUSTOM_EXIT + assert res[0].exit_reason == 'custom_exit' strategy.custom_exit = MagicMock(return_value='hello world') res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, low=None, high=None) - assert res.exit_type == ExitType.CUSTOM_EXIT - assert res.exit_flag is True - assert res.exit_reason == 'hello world' + assert res[0].exit_type == ExitType.CUSTOM_EXIT + assert res[0].exit_flag is True + assert res[0].exit_reason == 'hello world' caplog.clear() strategy.custom_exit = MagicMock(return_value='h' * 100) res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, low=None, high=None) - assert res.exit_type == ExitType.CUSTOM_EXIT - assert res.exit_flag is True - assert res.exit_reason == 'h' * 64 + assert res[0].exit_type == ExitType.CUSTOM_EXIT + assert res[0].exit_flag is True + assert res[0].exit_reason == 'h' * 64 assert log_has_re('Custom exit reason returned from custom_exit is too long.*', caplog) diff --git a/tests/test_integration.py b/tests/test_integration.py index d2ad8c981..83f54becb 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -52,8 +52,8 @@ def test_may_execute_exit_stoploss_on_exchange_multi(default_conf, ticker, fee, side_effect=[stoploss_order_closed, stoploss_order_open, stoploss_order_open]) # Sell 3rd trade (not called for the first trade) should_sell_mock = MagicMock(side_effect=[ - ExitCheckTuple(exit_type=ExitType.NONE), - ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL)] + [], + [ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL)]] ) cancel_order_mock = MagicMock() mocker.patch('freqtrade.exchange.Binance.stoploss', stoploss) @@ -160,11 +160,11 @@ def test_forcebuy_last_unlimited(default_conf, ticker, fee, mocker, balance_rati _notify_exit=MagicMock(), ) should_sell_mock = MagicMock(side_effect=[ - ExitCheckTuple(exit_type=ExitType.NONE), - ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL), - ExitCheckTuple(exit_type=ExitType.NONE), - ExitCheckTuple(exit_type=ExitType.NONE), - ExitCheckTuple(exit_type=ExitType.NONE)] + [], + [ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL)], + [], + [], + []] ) mocker.patch("freqtrade.strategy.interface.IStrategy.should_exit", should_sell_mock) From ce3bfd59f5e92f2b5f56848d76763fe17b76ad07 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 22 May 2022 10:31:29 +0200 Subject: [PATCH 3/5] Add explicit should_sell test --- freqtrade/enums/exitchecktuple.py | 3 ++ tests/strategy/test_interface.py | 74 +++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/freqtrade/enums/exitchecktuple.py b/freqtrade/enums/exitchecktuple.py index c245a05da..580b4e21c 100644 --- a/freqtrade/enums/exitchecktuple.py +++ b/freqtrade/enums/exitchecktuple.py @@ -15,3 +15,6 @@ class ExitCheckTuple: @property def exit_flag(self): return self.exit_type != ExitType.NONE + + def __eq__(self, other): + return self.exit_type == other.exit_type and self.exit_reason == other.exit_reason diff --git a/tests/strategy/test_interface.py b/tests/strategy/test_interface.py index 55cfcaf98..8bdea852a 100644 --- a/tests/strategy/test_interface.py +++ b/tests/strategy/test_interface.py @@ -525,6 +525,80 @@ def test_custom_exit(default_conf, fee, caplog) -> None: assert log_has_re('Custom exit reason returned from custom_exit is too long.*', caplog) +def test_should_sell(default_conf, fee, caplog) -> None: + + strategy = StrategyResolver.load_strategy(default_conf) + trade = Trade( + pair='ETH/BTC', + stake_amount=0.01, + amount=1, + open_date=arrow.utcnow().shift(hours=-1).datetime, + fee_open=fee.return_value, + fee_close=fee.return_value, + exchange='binance', + open_rate=1, + ) + now = arrow.utcnow().datetime + res = strategy.should_exit(trade, 1, now, + enter=False, exit_=False, + low=None, high=None) + + assert res == [] + strategy.min_roi_reached = MagicMock(return_value=True) + + res = strategy.should_exit(trade, 1, now, + enter=False, exit_=False, + low=None, high=None) + assert len(res) == 1 + assert res == [ExitCheckTuple(exit_type=ExitType.ROI)] + + strategy.min_roi_reached = MagicMock(return_value=True) + strategy.stop_loss_reached = MagicMock( + return_value=ExitCheckTuple(exit_type=ExitType.STOP_LOSS)) + + res = strategy.should_exit(trade, 1, now, + enter=False, exit_=False, + low=None, high=None) + assert len(res) == 2 + assert res == [ + ExitCheckTuple(exit_type=ExitType.ROI), + ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ] + + strategy.custom_exit = MagicMock(return_value='hello world') + + res = strategy.should_exit(trade, 1, now, + enter=False, exit_=False, + low=None, high=None) + assert len(res) == 3 + assert res == [ + ExitCheckTuple(exit_type=ExitType.CUSTOM_EXIT, exit_reason='hello world'), + ExitCheckTuple(exit_type=ExitType.ROI), + ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ] + + # Regular exit signal + res = strategy.should_exit(trade, 1, now, + enter=False, exit_=True, + low=None, high=None) + assert len(res) == 3 + assert res == [ + ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL), + ExitCheckTuple(exit_type=ExitType.ROI), + ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ] + + # Regular exit signal, no ROI + strategy.min_roi_reached = MagicMock(return_value=False) + res = strategy.should_exit(trade, 1, now, + enter=False, exit_=True, + low=None, high=None) + assert len(res) == 2 + assert res == [ + ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL), + ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ] + @pytest.mark.parametrize('side', TRADE_SIDES) def test_leverage_callback(default_conf, side) -> None: default_conf['strategy'] = 'StrategyTestV2' From 3692fcd3d5c56ca546496ac5b9bd65e899fa92f4 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 22 May 2022 11:01:18 +0200 Subject: [PATCH 4/5] Improve exit signal sequence --- freqtrade/enums/exitchecktuple.py | 3 +++ freqtrade/strategy/interface.py | 15 +++++++++++---- tests/strategy/test_interface.py | 15 +++++++++------ 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/freqtrade/enums/exitchecktuple.py b/freqtrade/enums/exitchecktuple.py index 580b4e21c..cb6411caf 100644 --- a/freqtrade/enums/exitchecktuple.py +++ b/freqtrade/enums/exitchecktuple.py @@ -18,3 +18,6 @@ class ExitCheckTuple: def __eq__(self, other): return self.exit_type == other.exit_type and self.exit_reason == other.exit_reason + + def __repr__(self): + return f"ExitCheckTuple({self.exit_type}, {self.exit_reason})" diff --git a/freqtrade/strategy/interface.py b/freqtrade/strategy/interface.py index 15627722c..69a3f9742 100644 --- a/freqtrade/strategy/interface.py +++ b/freqtrade/strategy/interface.py @@ -942,15 +942,22 @@ class IStrategy(ABC, HyperStrategyMixin): # Sequence: # Exit-signal - # ROI (if not stoploss) # Stoploss - if roi_reached and stoplossflag.exit_type != ExitType.STOP_LOSS: + # ROI + # Trailing stoploss + + if stoplossflag.exit_type == ExitType.STOP_LOSS: + + logger.debug(f"{trade.pair} - Stoploss hit. exit_type={stoplossflag.exit_type}") + exits.append(stoplossflag) + + if roi_reached: logger.debug(f"{trade.pair} - Required profit reached. exit_type=ExitType.ROI") exits.append(ExitCheckTuple(exit_type=ExitType.ROI)) - if stoplossflag.exit_flag: + if stoplossflag.exit_type == ExitType.TRAILING_STOP_LOSS: - logger.debug(f"{trade.pair} - Stoploss hit. exit_type={stoplossflag.exit_type}") + logger.debug(f"{trade.pair} - Trailing stoploss hit.") exits.append(stoplossflag) return exits diff --git a/tests/strategy/test_interface.py b/tests/strategy/test_interface.py index 8bdea852a..2cedea962 100644 --- a/tests/strategy/test_interface.py +++ b/tests/strategy/test_interface.py @@ -525,7 +525,7 @@ def test_custom_exit(default_conf, fee, caplog) -> None: assert log_has_re('Custom exit reason returned from custom_exit is too long.*', caplog) -def test_should_sell(default_conf, fee, caplog) -> None: +def test_should_sell(default_conf, fee) -> None: strategy = StrategyResolver.load_strategy(default_conf) trade = Trade( @@ -561,22 +561,24 @@ def test_should_sell(default_conf, fee, caplog) -> None: low=None, high=None) assert len(res) == 2 assert res == [ - ExitCheckTuple(exit_type=ExitType.ROI), ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ExitCheckTuple(exit_type=ExitType.ROI), ] strategy.custom_exit = MagicMock(return_value='hello world') - + # custom-exit and exit-signal is first res = strategy.should_exit(trade, 1, now, enter=False, exit_=False, low=None, high=None) assert len(res) == 3 assert res == [ ExitCheckTuple(exit_type=ExitType.CUSTOM_EXIT, exit_reason='hello world'), - ExitCheckTuple(exit_type=ExitType.ROI), ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ExitCheckTuple(exit_type=ExitType.ROI), ] + strategy.stop_loss_reached = MagicMock( + return_value=ExitCheckTuple(exit_type=ExitType.TRAILING_STOP_LOSS)) # Regular exit signal res = strategy.should_exit(trade, 1, now, enter=False, exit_=True, @@ -585,7 +587,7 @@ def test_should_sell(default_conf, fee, caplog) -> None: assert res == [ ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL), ExitCheckTuple(exit_type=ExitType.ROI), - ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ExitCheckTuple(exit_type=ExitType.TRAILING_STOP_LOSS), ] # Regular exit signal, no ROI @@ -596,9 +598,10 @@ def test_should_sell(default_conf, fee, caplog) -> None: assert len(res) == 2 assert res == [ ExitCheckTuple(exit_type=ExitType.EXIT_SIGNAL), - ExitCheckTuple(exit_type=ExitType.STOP_LOSS), + ExitCheckTuple(exit_type=ExitType.TRAILING_STOP_LOSS), ] + @pytest.mark.parametrize('side', TRADE_SIDES) def test_leverage_callback(default_conf, side) -> None: default_conf['strategy'] = 'StrategyTestV2' From 938a66511a4fcd9961c8e55334979e4fe7e15fe5 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 22 May 2022 11:28:11 +0200 Subject: [PATCH 5/5] Update Documentation for new confirm_trade_exit behavior --- docs/backtesting.md | 3 ++- docs/strategy-callbacks.md | 11 +++++++++++ freqtrade/freqtradebot.py | 6 +++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/backtesting.md b/docs/backtesting.md index b4d9aef80..76718d206 100644 --- a/docs/backtesting.md +++ b/docs/backtesting.md @@ -530,8 +530,9 @@ Since backtesting lacks some detailed information about what happens within a ca - Exit-reason does not explain if a trade was positive or negative, just what triggered the exit (this can look odd if negative ROI values are used) - Evaluation sequence (if multiple signals happen on the same candle) - Exit-signal - - ROI (if not stoploss) - Stoploss + - ROI + - Trailing stoploss Taking these assumptions, backtesting tries to mirror real trading as closely as possible. However, backtesting will **never** replace running a strategy in dry-run mode. Also, keep in mind that past results don't guarantee future success. diff --git a/docs/strategy-callbacks.md b/docs/strategy-callbacks.md index ab67a3c26..7f3249c88 100644 --- a/docs/strategy-callbacks.md +++ b/docs/strategy-callbacks.md @@ -563,6 +563,14 @@ class AwesomeStrategy(IStrategy): `confirm_trade_exit()` can be used to abort a trade exit (sell) at the latest second (maybe because the price is not what we expect). +`confirm_trade_exit()` may be called multiple times within one iteration for the same trade if different exit-reasons apply. +The exit-reasons (if applicable) will be in the following sequence: + +* `exit_signal` / `custom_exit` +* `stop_loss` +* `roi` +* `trailing_stop_loss` + ``` python from freqtrade.persistence import Trade @@ -605,6 +613,9 @@ class AwesomeStrategy(IStrategy): ``` +!!! Warning + `confirm_trade_exit()` can prevent stoploss exits, causing significant losses as this would ignore stoploss exits. + ## Adjust trade position The `position_adjustment_enable` strategy property enables the usage of `adjust_trade_position()` callback in the strategy. diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 4ae55e31c..08f5474fd 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -1117,7 +1117,7 @@ class FreqtradeBot(LoggingMixin): for should_exit in exits: if should_exit.exit_flag: logger.info(f'Exit for {trade.pair} detected. Reason: {should_exit.exit_type}' - f'Tag: {exit_tag if exit_tag is not None else "None"}') + f'{f" Tag: {exit_tag}" if exit_tag is not None else ""}') exited = self.execute_trade_exit(trade, exit_rate, should_exit, exit_tag=exit_tag) if exited: return True @@ -1407,7 +1407,7 @@ class FreqtradeBot(LoggingMixin): :param trade: Trade instance :param limit: limit rate for the sell order :param exit_check: CheckTuple with signal and reason - :return: True if it succeeds (supported) False (not supported) + :return: True if it succeeds False """ trade.funding_fees = self.exchange.get_funding_fees( pair=trade.pair, @@ -1454,7 +1454,7 @@ class FreqtradeBot(LoggingMixin): time_in_force=time_in_force, exit_reason=exit_reason, sell_reason=exit_reason, # sellreason -> compatibility current_time=datetime.now(timezone.utc)): - logger.info(f"User requested abortion of exiting {trade.pair}") + logger.info(f"User requested abortion of {trade.pair} exit.") return False try: