Merge pull request #1952 from hroff-1902/fix/1948

Fix #1948

freqtrade/optimize/backtesting.py

@@ -252,22 +252,20 @@ class Backtesting(object):
             sell = self.strategy.should_sell(trade, sell_row.open, sell_row.date, sell_row.buy,
                                              sell_row.sell, low=sell_row.low, high=sell_row.high)
             if sell.sell_flag:
-
                 trade_dur = int((sell_row.date - buy_row.date).total_seconds() // 60)
                 # Special handling if high or low hit STOP_LOSS or ROI
                 if sell.sell_type in (SellType.STOP_LOSS, SellType.TRAILING_STOP_LOSS):
                     # Set close_rate to stoploss
                     closerate = trade.stop_loss
                 elif sell.sell_type == (SellType.ROI):
-                    # get next entry in min_roi > to trade duration
-                    # Interface.py skips on trade_duration <= duration
-                    roi_entry = max(list(filter(lambda x: trade_dur >= x,
-                                                self.strategy.minimal_roi.keys())))
-                    roi = self.strategy.minimal_roi[roi_entry]
-
-                    # - (Expected abs profit + open_rate + open_fee) / (fee_close -1)
-                    closerate = - (trade.open_rate * roi + trade.open_rate *
-                                   (1 + trade.fee_open)) / (trade.fee_close - 1)
+                    roi = self.strategy.min_roi_reached_entry(trade_dur)
+                    if roi is not None:
+                        # - (Expected abs profit + open_rate + open_fee) / (fee_close -1)
+                        closerate = - (trade.open_rate * roi + trade.open_rate *
+                                       (1 + trade.fee_open)) / (trade.fee_close - 1)
+                    else:
+                        # This should not be reached...
+                        closerate = sell_row.open
                 else:
                     closerate = sell_row.open
 

freqtrade/strategy/interface.py

@@ -6,7 +6,7 @@ import logging
 from abc import ABC, abstractmethod
 from datetime import datetime
 from enum import Enum
-from typing import Dict, List, NamedTuple, Tuple
+from typing import Dict, List, NamedTuple, Optional, Tuple
 import warnings
 
 import arrow
@@ -347,23 +347,32 @@ class IStrategy(ABC):
 
         return SellCheckTuple(sell_flag=False, sell_type=SellType.NONE)
 
+    def min_roi_reached_entry(self, trade_dur: int) -> Optional[float]:
+        """
+        Based on trade duration defines the ROI entry that may have been reached.
+        :param trade_dur: trade duration in minutes
+        :return: minimal ROI entry value or None if none proper ROI entry was found.
+        """
+        # Get highest entry in ROI dict where key <= trade-duration
+        roi_list = list(filter(lambda x: x <= trade_dur, self.minimal_roi.keys()))
+        if not roi_list:
+            return None
+        roi_entry = max(roi_list)
+        return self.minimal_roi[roi_entry]
+
     def min_roi_reached(self, trade: Trade, current_profit: float, current_time: datetime) -> bool:
         """
-        Based an earlier trade and current price and ROI configuration, decides whether bot should
+        Based on trade duration, current price and ROI configuration, decides whether bot should
         sell. Requires current_profit to be in percent!!
         :return: True if bot should sell at current rate
         """
-
         # Check if time matches and current rate is above threshold
-        trade_dur = (current_time.timestamp() - trade.open_date.timestamp()) / 60
-
-        # Get highest entry in ROI dict where key >= trade-duration
-        roi_entry = max(list(filter(lambda x: trade_dur >= x, self.minimal_roi.keys())))
-        threshold = self.minimal_roi[roi_entry]
-        if current_profit > threshold:
-            return True
-
-        return False
+        trade_dur = int((current_time.timestamp() - trade.open_date.timestamp()) // 60)
+        roi = self.min_roi_reached_entry(trade_dur)
+        if roi is None:
+            return False
+        else:
+            return current_profit > roi
 
     def tickerdata_to_dataframe(self, tickerdata: Dict[str, List]) -> Dict[str, DataFrame]:
         """

freqtrade/tests/strategy/test_interface.py

@@ -186,6 +186,39 @@ def test_min_roi_reached2(default_conf, fee) -> None:
         assert strategy.min_roi_reached(trade, 0.31, arrow.utcnow().shift(minutes=-2).datetime)
 
 
+def test_min_roi_reached3(default_conf, fee) -> None:
+
+    # test for issue #1948
+    min_roi = {20: 0.07,
+               30: 0.05,
+               55: 0.30,
+               }
+    strategy = DefaultStrategy(default_conf)
+    strategy.minimal_roi = min_roi
+    trade = Trade(
+            pair='ETH/BTC',
+            stake_amount=0.001,
+            open_date=arrow.utcnow().shift(hours=-1).datetime,
+            fee_open=fee.return_value,
+            fee_close=fee.return_value,
+            exchange='bittrex',
+            open_rate=1,
+    )
+
+    assert not strategy.min_roi_reached(trade, 0.02, arrow.utcnow().shift(minutes=-56).datetime)
+    assert not strategy.min_roi_reached(trade, 0.12, arrow.utcnow().shift(minutes=-56).datetime)
+
+    assert not strategy.min_roi_reached(trade, 0.04, arrow.utcnow().shift(minutes=-39).datetime)
+    assert strategy.min_roi_reached(trade, 0.071, arrow.utcnow().shift(minutes=-39).datetime)
+
+    assert not strategy.min_roi_reached(trade, 0.04, arrow.utcnow().shift(minutes=-26).datetime)
+    assert strategy.min_roi_reached(trade, 0.06, arrow.utcnow().shift(minutes=-26).datetime)
+
+    # Should not trigger with 20% profit since after 55 minutes only 30% is active.
+    assert not strategy.min_roi_reached(trade, 0.20, arrow.utcnow().shift(minutes=-2).datetime)
+    assert strategy.min_roi_reached(trade, 0.31, arrow.utcnow().shift(minutes=-2).datetime)
+
+
 def test_analyze_ticker_default(ticker_history, mocker, caplog) -> None:
     caplog.set_level(logging.DEBUG)
     ind_mock = MagicMock(side_effect=lambda x, meta: x)