Merge pull request #3107 from orehunt/check_dataframe_after_signals

check that the strategy dataframe matches the one given by the bot

freqtrade/strategy/interface.py

@@ -16,6 +16,7 @@ from freqtrade.data.dataprovider import DataProvider
 from freqtrade.exchange import timeframe_to_minutes
 from freqtrade.persistence import Trade
 from freqtrade.wallets import Wallets
+from freqtrade.exceptions import DependencyException
 
 
 logger = logging.getLogger(__name__)
@@ -241,8 +242,26 @@ class IStrategy(ABC):
 
         return dataframe
 
-    def get_signal(self, pair: str, interval: str,
+    @staticmethod
-                   dataframe: DataFrame) -> Tuple[bool, bool]:
+    def preserve_df(dataframe: DataFrame) -> Tuple[int, float, datetime]:
+        """ keep some data for dataframes """
+        return len(dataframe), dataframe["close"].iloc[-1], dataframe["date"].iloc[-1]
+
+    @staticmethod
+    def assert_df(dataframe: DataFrame, df_len: int, df_close: float, df_date: datetime):
+        """ make sure data is unmodified """
+        message = ""
+        if df_len != len(dataframe):
+            message = "length"
+        elif df_close != dataframe["close"].iloc[-1]:
+            message = "last close price"
+        elif df_date != dataframe["date"].iloc[-1]:
+            message = "last date"
+        if message:
+            raise DependencyException("Dataframe returned from strategy has mismatching "
+                                      f"{message}.")
+
+    def get_signal(self, pair: str, interval: str, dataframe: DataFrame) -> Tuple[bool, bool]:
         """
         Calculates current signal based several technical analysis indicators
         :param pair: pair in format ANT/BTC
@@ -254,14 +273,18 @@ class IStrategy(ABC):
             logger.warning('Empty candle (OHLCV) data for pair %s', pair)
             return False, False
 
+        latest_date = dataframe['date'].max()
         try:
+            df_len, df_close, df_date = self.preserve_df(dataframe)
             dataframe = self._analyze_ticker_internal(dataframe, {'pair': pair})
+            self.assert_df(dataframe, df_len, df_close, df_date)
         except ValueError as error:
-            logger.warning(
+            logger.warning('Unable to analyze candle (OHLCV) data for pair %s: %s',
-                'Unable to analyze candle (OHLCV) data for pair %s: %s',
+                           pair, str(error))
-                pair,
+            return False, False
-                str(error)
+        except DependencyException as error:
-            )
+            logger.warning("Unable to analyze candle (OHLCV) data for pair %s: %s",
+                           pair, str(error))
             return False, False
         except Exception as error:
             logger.exception(
@@ -275,7 +298,7 @@ class IStrategy(ABC):
             logger.warning('Empty dataframe for pair %s', pair)
             return False, False
 
-        latest = dataframe.iloc[-1]
+        latest = dataframe.loc[dataframe['date'] == latest_date].iloc[-1]
 
         # Check if dataframe is out of date
         signal_date = arrow.get(latest['date'])

tests/strategy/test_interface.py

@@ -4,46 +4,52 @@ import logging
 from unittest.mock import MagicMock
 
 import arrow
+import pytest
 from pandas import DataFrame
 
 from freqtrade.configuration import TimeRange
 from freqtrade.data.history import load_data
+from freqtrade.exceptions import DependencyException
 from freqtrade.persistence import Trade
 from freqtrade.resolvers import StrategyResolver
-from .strats.default_strategy import DefaultStrategy
 from tests.conftest import get_patched_exchange, log_has
 
+from .strats.default_strategy import DefaultStrategy
+
 # Avoid to reinit the same object again and again
 _STRATEGY = DefaultStrategy(config={})
 
 
-def test_returns_latest_buy_signal(mocker, default_conf, ohlcv_history):
+def test_returns_latest_signal(mocker, default_conf, ohlcv_history):
-    mocker.patch.object(
+    ohlcv_history.loc[1, 'date'] = arrow.utcnow()
-        _STRATEGY, '_analyze_ticker_internal',
+    # Take a copy to correctly modify the call
-        return_value=DataFrame([{'buy': 1, 'sell': 0, 'date': arrow.utcnow()}])
+    mocked_history = ohlcv_history.copy()
-    )
+    mocked_history['sell'] = 0
-    assert _STRATEGY.get_signal('ETH/BTC', '5m', ohlcv_history) == (True, False)
+    mocked_history['buy'] = 0
+    mocked_history.loc[1, 'sell'] = 1
 
     mocker.patch.object(
         _STRATEGY, '_analyze_ticker_internal',
-        return_value=DataFrame([{'buy': 0, 'sell': 1, 'date': arrow.utcnow()}])
+        return_value=mocked_history
-    )
-    assert _STRATEGY.get_signal('ETH/BTC', '5m', ohlcv_history) == (False, True)
-
-
-def test_returns_latest_sell_signal(mocker, default_conf, ohlcv_history):
-    mocker.patch.object(
-        _STRATEGY, '_analyze_ticker_internal',
-        return_value=DataFrame([{'sell': 1, 'buy': 0, 'date': arrow.utcnow()}])
     )
 
     assert _STRATEGY.get_signal('ETH/BTC', '5m', ohlcv_history) == (False, True)
+    mocked_history.loc[1, 'sell'] = 0
+    mocked_history.loc[1, 'buy'] = 1
 
     mocker.patch.object(
         _STRATEGY, '_analyze_ticker_internal',
-        return_value=DataFrame([{'sell': 0, 'buy': 1, 'date': arrow.utcnow()}])
+        return_value=mocked_history
     )
     assert _STRATEGY.get_signal('ETH/BTC', '5m', ohlcv_history) == (True, False)
+    mocked_history.loc[1, 'sell'] = 0
+    mocked_history.loc[1, 'buy'] = 0
+
+    mocker.patch.object(
+        _STRATEGY, '_analyze_ticker_internal',
+        return_value=mocked_history
+    )
+    assert _STRATEGY.get_signal('ETH/BTC', '5m', ohlcv_history) == (False, False)
 
 
 def test_get_signal_empty(default_conf, mocker, caplog):
@@ -74,26 +80,74 @@ def test_get_signal_empty_dataframe(default_conf, mocker, caplog, ohlcv_history)
         _STRATEGY, '_analyze_ticker_internal',
         return_value=DataFrame([])
     )
+    mocker.patch.object(_STRATEGY, 'assert_df')
+
     assert (False, False) == _STRATEGY.get_signal('xyz', default_conf['ticker_interval'],
                                                   ohlcv_history)
     assert log_has('Empty dataframe for pair xyz', caplog)
 
 
 def test_get_signal_old_dataframe(default_conf, mocker, caplog, ohlcv_history):
-    caplog.set_level(logging.INFO)
     # default_conf defines a 5m interval. we check interval * 2 + 5m
     # this is necessary as the last candle is removed (partial candles) by default
-    oldtime = arrow.utcnow().shift(minutes=-16)
+    ohlcv_history.loc[1, 'date'] = arrow.utcnow().shift(minutes=-16)
-    ticks = DataFrame([{'buy': 1, 'date': oldtime}])
+    # Take a copy to correctly modify the call
+    mocked_history = ohlcv_history.copy()
+    mocked_history['sell'] = 0
+    mocked_history['buy'] = 0
+    mocked_history.loc[1, 'buy'] = 1
+
+    caplog.set_level(logging.INFO)
     mocker.patch.object(
         _STRATEGY, '_analyze_ticker_internal',
-        return_value=DataFrame(ticks)
+        return_value=mocked_history
     )
+    mocker.patch.object(_STRATEGY, 'assert_df')
     assert (False, False) == _STRATEGY.get_signal('xyz', default_conf['ticker_interval'],
                                                   ohlcv_history)
     assert log_has('Outdated history for pair xyz. Last tick is 16 minutes old', caplog)
 
 
+def test_assert_df_raise(default_conf, mocker, caplog, ohlcv_history):
+    # default_conf defines a 5m interval. we check interval * 2 + 5m
+    # this is necessary as the last candle is removed (partial candles) by default
+    ohlcv_history.loc[1, 'date'] = arrow.utcnow().shift(minutes=-16)
+    # Take a copy to correctly modify the call
+    mocked_history = ohlcv_history.copy()
+    mocked_history['sell'] = 0
+    mocked_history['buy'] = 0
+    mocked_history.loc[1, 'buy'] = 1
+
+    caplog.set_level(logging.INFO)
+    mocker.patch.object(
+        _STRATEGY, 'assert_df',
+        side_effect=DependencyException('Dataframe returned...')
+    )
+    assert (False, False) == _STRATEGY.get_signal('xyz', default_conf['ticker_interval'],
+                                                  ohlcv_history)
+    assert log_has('Unable to analyze candle (OHLCV) data for pair xyz: Dataframe returned...',
+                   caplog)
+
+
+def test_assert_df(default_conf, mocker, ohlcv_history):
+    # Ensure it's running when passed correctly
+    _STRATEGY.assert_df(ohlcv_history, len(ohlcv_history),
+                        ohlcv_history.loc[1, 'close'], ohlcv_history.loc[1, 'date'])
+
+    with pytest.raises(DependencyException, match=r"Dataframe returned from strategy.*length\."):
+        _STRATEGY.assert_df(ohlcv_history, len(ohlcv_history) + 1,
+                            ohlcv_history.loc[1, 'close'], ohlcv_history.loc[1, 'date'])
+
+    with pytest.raises(DependencyException,
+                       match=r"Dataframe returned from strategy.*last close price\."):
+        _STRATEGY.assert_df(ohlcv_history, len(ohlcv_history),
+                            ohlcv_history.loc[1, 'close'] + 0.01, ohlcv_history.loc[1, 'date'])
+    with pytest.raises(DependencyException,
+                       match=r"Dataframe returned from strategy.*last date\."):
+        _STRATEGY.assert_df(ohlcv_history, len(ohlcv_history),
+                            ohlcv_history.loc[1, 'close'], ohlcv_history.loc[0, 'date'])
+
+
 def test_get_signal_handles_exceptions(mocker, default_conf):
     exchange = get_patched_exchange(mocker, default_conf)
     mocker.patch.object(