Merge pull request #1457 from freqtrade/fix/minroi

minroi sequence of keys should not matter

docs/bot-optimization.md

@@ -179,10 +179,9 @@ minimal_roi = {
 The above configuration would therefore mean:
 
 - Sell whenever 4% profit was reached
-- Sell after 20 minutes when 2% profit was reached
-- Sell after 20 minutes when 2% profit was reached
-- Sell after 30 minutes when 1% profit was reached
-- Sell after 40 minutes when the trade is non-loosing (no profit)
+- Sell when 2% profit was reached (in effect after 20 minutes)
+- Sell when 1% profit was reached (in effect after 30 minutes)
+- Sell when trade is non-loosing (in effect after 40 minutes)
 
 The calculation does include fees.
 

freqtrade/optimize/backtesting.py

@@ -219,8 +219,9 @@ class Backtesting(object):
                     # Set close_rate to stoploss
                     closerate = trade.stop_loss
                 elif sell.sell_type == (SellType.ROI):
-                    # get entry in min_roi >= to trade duration
-                    roi_entry = max(list(filter(lambda x: trade_dur >= x,
+                    # 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]
 

freqtrade/strategy/interface.py

@@ -321,7 +321,7 @@ class IStrategy(ABC):
         time_diff = (current_time.timestamp() - trade.open_date.timestamp()) / 60
         for duration, threshold in self.minimal_roi.items():
             if time_diff <= duration:
-                return False
+                continue
             if current_profit > threshold:
                 return True
 

freqtrade/tests/strategy/test_interface.py

@@ -117,26 +117,30 @@ def test_tickerdata_to_dataframe(default_conf) -> None:
 
 
 def test_min_roi_reached(default_conf, fee) -> None:
-    strategy = DefaultStrategy(default_conf)
-    strategy.minimal_roi = {0: 0.1, 20: 0.05, 55: 0.01}
-    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.01, arrow.utcnow().shift(minutes=-55).datetime)
-    assert strategy.min_roi_reached(trade, 0.12, arrow.utcnow().shift(minutes=-55).datetime)
+    min_roi_list = [{20: 0.05, 55: 0.01, 0: 0.1},
+                    {0: 0.1, 20: 0.05, 55: 0.01}]
+    for roi in min_roi_list:
+        strategy = DefaultStrategy(default_conf)
+        strategy.minimal_roi = 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.04, arrow.utcnow().shift(minutes=-39).datetime)
-    assert strategy.min_roi_reached(trade, 0.06, arrow.utcnow().shift(minutes=-39).datetime)
+        assert not strategy.min_roi_reached(trade, 0.02, arrow.utcnow().shift(minutes=-56).datetime)
+        assert strategy.min_roi_reached(trade, 0.12, arrow.utcnow().shift(minutes=-56).datetime)
 
-    assert not strategy.min_roi_reached(trade, -0.01, arrow.utcnow().shift(minutes=-1).datetime)
-    assert strategy.min_roi_reached(trade, 0.02, arrow.utcnow().shift(minutes=-1).datetime)
+        assert not strategy.min_roi_reached(trade, 0.04, arrow.utcnow().shift(minutes=-39).datetime)
+        assert strategy.min_roi_reached(trade, 0.06, arrow.utcnow().shift(minutes=-39).datetime)
+
+        assert not strategy.min_roi_reached(trade, -0.01, arrow.utcnow().shift(minutes=-1).datetime)
+        assert strategy.min_roi_reached(trade, 0.02, arrow.utcnow().shift(minutes=-1).datetime)
 
 
 def test_analyze_ticker_default(ticker_history, mocker, caplog) -> None: