diff --git a/freqtrade/strategy/strategy_wrapper.py b/freqtrade/strategy/strategy_wrapper.py index 8cb0bde15..53c99423f 100644 --- a/freqtrade/strategy/strategy_wrapper.py +++ b/freqtrade/strategy/strategy_wrapper.py @@ -1,10 +1,9 @@ import logging -from copy import deepcopy from functools import wraps from typing import Any, Callable, TypeVar, cast from freqtrade.exceptions import StrategyError - +from freqtrade.persistence import Trade logger = logging.getLogger(__name__) @@ -23,7 +22,16 @@ def strategy_safe_wrapper(f: F, message: str = "", default_retval=None, supress_ try: if 'trade' in kwargs: # Protect accidental modifications from within the strategy - kwargs['trade'] = deepcopy(kwargs['trade']) + trade = kwargs['trade'] + if isinstance(trade, Trade): + if trade in Trade.query.session.dirty: + Trade.commit() + return_vals = f(*args, **kwargs) + if trade in Trade.query.session.dirty: + logger.warning(f"The `trade` parameter have changed " + f"in the function `{f.__name__}`, this may " + f"lead to unexpected behavior.") + return return_vals return f(*args, **kwargs) except ValueError as error: logger.warning( diff --git a/tests/strategy/test_interface.py b/tests/strategy/test_interface.py index 294021c83..7cd8e3bbb 100644 --- a/tests/strategy/test_interface.py +++ b/tests/strategy/test_interface.py @@ -838,10 +838,10 @@ def test_strategy_safe_wrapper(value): @pytest.mark.usefixtures("init_persistence") -def test_strategy_safe_wrapper_trade_copy(fee): +def test_strategy_safe_wrapper_trade_mutate_warning(fee, caplog): create_mock_trades(fee) - def working_method(trade): + def working_method_mutate(trade): assert len(trade.orders) > 0 assert trade.orders trade.orders = [] @@ -851,12 +851,25 @@ def test_strategy_safe_wrapper_trade_copy(fee): trade = Trade.get_open_trades()[0] # Don't assert anything before strategy_wrapper. # This ensures that relationship loading works correctly. - ret = strategy_safe_wrapper(working_method, message='DeadBeef')(trade=trade) + ret = strategy_safe_wrapper(working_method_mutate, message='DeadBeef')(trade=trade) assert isinstance(ret, Trade) - assert id(trade) != id(ret) - # Did not modify the original order - assert len(trade.orders) > 0 - assert len(ret.orders) == 0 + assert id(trade) == id(ret) + assert len(trade.orders) == 0 + assert log_has_re( + r"The `trade` parameter have changed in the function .*, this may " + r"lead to unexpected behavior\.", + caplog) + + caplog.clear() + + def working_method(trade): + return trade + + strategy_safe_wrapper(working_method, message='DeadBeef')(trade=trade) + assert not log_has_re( + r"The `trade` parameter have changed in the function .*, this may " + r"lead to unexpected behavior\.", + caplog) def test_hyperopt_parameters(): diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 5e580e4fa..549406bac 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -4,6 +4,7 @@ import logging import time from copy import deepcopy +from datetime import datetime from typing import List from unittest.mock import ANY, MagicMock, PropertyMock, patch @@ -1806,7 +1807,7 @@ def test_tsl_on_exchange_compatible_with_edge(mocker, edge_conf, fee, limit_orde trade.is_open = True trade.open_order_id = None trade.stoploss_order_id = 100 - trade.stoploss_last_update = arrow.utcnow() + trade.stoploss_last_update = datetime.utcnow() stoploss_order_hanging = MagicMock(return_value={ 'id': 100, @@ -1957,7 +1958,7 @@ def test_update_trade_state(mocker, default_conf_usdt, limit_order, is_short, ca fee_open=0.001, fee_close=0.001, open_rate=0.01, - open_date=arrow.utcnow().datetime, + open_date=datetime.utcnow(), amount=11, exchange="binance", is_short=is_short, @@ -3286,7 +3287,6 @@ def test_execute_trade_exit_up(default_conf_usdt, ticker_usdt, fee, ticker_usdt_ ) assert rpc_mock.call_count == 0 assert freqtrade.strategy.confirm_trade_exit.call_count == 1 - assert id(freqtrade.strategy.confirm_trade_exit.call_args_list[0][1]['trade']) != id(trade) assert freqtrade.strategy.confirm_trade_exit.call_args_list[0][1]['trade'].id == trade.id # Repatch with true @@ -6114,3 +6114,34 @@ def test_check_and_call_adjust_trade_position(mocker, default_conf_usdt, fee, ca freqtrade.strategy.adjust_trade_position = MagicMock(return_value=-10) freqtrade.process_open_trade_positions() assert log_has_re(r"LIMIT_SELL has been fulfilled.*", caplog) + + +def test_trade_mutated_warning(mocker, default_conf_usdt, fee, + caplog) -> None: + default_conf_usdt.update({ + "position_adjustment_enable": True, + "max_entry_position_adjustment": 0, + }) + freqtrade = get_patched_freqtradebot(mocker, default_conf_usdt) + buy_rate_mock = MagicMock(return_value=10) + mocker.patch.multiple( + 'freqtrade.exchange.Exchange', + get_rate=buy_rate_mock, + fetch_ticker=MagicMock(return_value={ + 'bid': 10, + 'ask': 12, + 'last': 11 + }), + get_min_pair_stake_amount=MagicMock(return_value=1), + get_fee=fee, + ) + create_mock_trades(fee) + + def adjust_pos_with_trade_mutate(trade: Trade, *args, **kargs): + trade.amount = 321 + return 10 + + freqtrade.strategy.adjust_trade_position = adjust_pos_with_trade_mutate + freqtrade.process_open_trade_positions() + assert log_has_re(r"The `trade` parameter have changed in the function .*, " + r"this may lead to unexpected behavior\.", caplog)