Merge pull request #5183 from freqtrade/remove_order_book_max

Remove order book max
2021-06-26 14:56:10 +02:00 · 2021-06-26 14:56:10 +02:00 · 60c7308126
commit 60c7308126
parent fa72ed10b6 c62fad0088
17 changed files with 90 additions and 100 deletions
--- a/config_binance.json.example
+++ b/config_binance.json.example
@ -13,7 +13,7 @@
    },
    "bid_strategy": {
        "ask_last_balance": 0.0,
-        "use_order_book": false,
+        "use_order_book": true,
        "order_book_top": 1,
        "check_depth_of_market": {
            "enabled": false,
@ -21,9 +21,8 @@
        }
    },
    "ask_strategy": {
-        "use_order_book": false,
-        "order_book_min": 1,
-        "order_book_max": 1,
+        "use_order_book": true,
+        "order_book_top": 1,
        "use_sell_signal": true,
        "sell_profit_only": false,
        "ignore_roi_if_buy_signal": false
--- a/config_bittrex.json.example
+++ b/config_bittrex.json.example
@ -12,7 +12,7 @@
        "sell": 30
    },
    "bid_strategy": {
-        "use_order_book": false,
+        "use_order_book": true,
        "ask_last_balance": 0.0,
        "order_book_top": 1,
        "check_depth_of_market": {
@ -21,9 +21,8 @@
        }
    },
    "ask_strategy":{
-        "use_order_book": false,
-        "order_book_min": 1,
-        "order_book_max": 1,
+        "use_order_book": true,
+        "order_book_top": 1,
        "use_sell_signal": true,
        "sell_profit_only": false,
        "ignore_roi_if_buy_signal": false
--- a/config_ftx.json.example
+++ b/config_ftx.json.example
@ -13,7 +13,7 @@
    },
    "bid_strategy": {
        "ask_last_balance": 0.0,
-        "use_order_book": false,
+        "use_order_book": true,
        "order_book_top": 1,
        "check_depth_of_market": {
            "enabled": false,
@ -21,9 +21,8 @@
        }
    },
    "ask_strategy": {
-        "use_order_book": false,
-        "order_book_min": 1,
-        "order_book_max": 1,
+        "use_order_book": true,
+        "order_book_top": 1,
        "use_sell_signal": true,
        "sell_profit_only": false,
        "ignore_roi_if_buy_signal": false
--- a/config_full.json.example
+++ b/config_full.json.example
@ -28,7 +28,7 @@
    },
    "bid_strategy": {
        "price_side": "bid",
-        "use_order_book": false,
+        "use_order_book": true,
        "ask_last_balance": 0.0,
        "order_book_top": 1,
        "check_depth_of_market": {
@ -38,9 +38,8 @@
    },
    "ask_strategy":{
        "price_side": "ask",
-        "use_order_book": false,
-        "order_book_min": 1,
-        "order_book_max": 1,
+        "use_order_book": true,
+        "order_book_top": 1,
        "use_sell_signal": true,
        "sell_profit_only": false,
        "sell_profit_offset": 0.0,
--- a/config_kraken.json.example
+++ b/config_kraken.json.example
@ -12,7 +12,7 @@
        "sell": 30
    },
    "bid_strategy": {
-        "use_order_book": false,
+        "use_order_book": true,
        "ask_last_balance": 0.0,
        "order_book_top": 1,
        "check_depth_of_market": {
@ -21,9 +21,8 @@
        }
    },
    "ask_strategy":{
-        "use_order_book": false,
-        "order_book_min": 1,
-        "order_book_max": 1,
+        "use_order_book": true,
+        "order_book_top": 1,
        "use_sell_signal": true,
        "sell_profit_only": false,
        "ignore_roi_if_buy_signal": false
--- a/docs/configuration.md
+++ b/docs/configuration.md
@ -74,14 +74,13 @@ Mandatory parameters are marked as **Required**, which means that they are requi
 | `bid_strategy.price_side` | Select the side of the spread the bot should look at to get the buy rate. [More information below](#buy-price-side).<br> *Defaults to `bid`.* <br> **Datatype:** String (either `ask` or `bid`).
 | `bid_strategy.ask_last_balance` | **Required.** Interpolate the bidding price. More information [below](#buy-price-without-orderbook-enabled).
 | `bid_strategy.use_order_book` | Enable buying using the rates in [Order Book Bids](#buy-price-with-orderbook-enabled). <br> **Datatype:** Boolean
-| `bid_strategy.order_book_top` | Bot will use the top N rate in Order Book Bids to buy. I.e. a value of 2 will allow the bot to pick the 2nd bid rate in [Order Book Bids](#buy-price-with-orderbook-enabled). <br>*Defaults to `1`.*  <br> **Datatype:** Positive Integer
+| `bid_strategy.order_book_top` | Bot will use the top N rate in Order Book "price_side" to buy. I.e. a value of 2 will allow the bot to pick the 2nd bid rate in [Order Book Bids](#buy-price-with-orderbook-enabled). <br>*Defaults to `1`.*  <br> **Datatype:** Positive Integer
 | `bid_strategy. check_depth_of_market.enabled` | Do not buy if the difference of buy orders and sell orders is met in Order Book. [Check market depth](#check-depth-of-market). <br>*Defaults to `false`.* <br> **Datatype:** Boolean
 | `bid_strategy. check_depth_of_market.bids_to_ask_delta` | The difference ratio of buy orders and sell orders found in Order Book. A value below 1 means sell order size is greater, while value greater than 1 means buy order size is higher. [Check market depth](#check-depth-of-market) <br> *Defaults to `0`.*  <br> **Datatype:** Float (as ratio)
 | `ask_strategy.price_side` | Select the side of the spread the bot should look at to get the sell rate. [More information below](#sell-price-side).<br> *Defaults to `ask`.* <br> **Datatype:** String (either `ask` or `bid`).
 | `ask_strategy.bid_last_balance` | Interpolate the selling price. More information [below](#sell-price-without-orderbook-enabled).
 | `ask_strategy.use_order_book` | Enable selling of open trades using [Order Book Asks](#sell-price-with-orderbook-enabled). <br> **Datatype:** Boolean
-| `ask_strategy.order_book_min` | Bot will scan from the top min to max Order Book Asks searching for a profitable rate. <br>*Defaults to `1`.* <br> **Datatype:** Positive Integer
-| `ask_strategy.order_book_max` | Bot will scan from the top min to max Order Book Asks searching for a profitable rate. <br>*Defaults to `1`.* <br> **Datatype:** Positive Integer
+| `ask_strategy.order_book_top` | Bot will use the top N rate in Order Book "price_side" to sell. I.e. a value of 2 will allow the bot to pick the 2nd ask rate in [Order Book Asks](#sell-price-with-orderbook-enabled)<br>*Defaults to `1`.* <br> **Datatype:** Positive Integer
 | `ask_strategy.use_sell_signal` | Use sell signals produced by the strategy in addition to the `minimal_roi`. [Strategy Override](#parameters-in-the-strategy). <br>*Defaults to `true`.* <br> **Datatype:** Boolean
 | `ask_strategy.sell_profit_only` | Wait until the bot reaches `ask_strategy.sell_profit_offset` before taking a sell decision. [Strategy Override](#parameters-in-the-strategy). <br>*Defaults to `false`.* <br> **Datatype:** Boolean
 | `ask_strategy.sell_profit_offset` | Sell-signal is only active above this value. [Strategy Override](#parameters-in-the-strategy). <br>*Defaults to `0.0`.* <br> **Datatype:** Float (as ratio)
--- a/docs/deprecated.md
+++ b/docs/deprecated.md
@ -33,3 +33,8 @@ The old section of configuration parameters (`"pairlist"`) has been deprecated i
 ### deprecation of bidVolume and askVolume from volume-pairlist

 Since only quoteVolume can be compared between assets, the other options (bidVolume, askVolume) have been deprecated in 2020.4, and have been removed in 2020.9.
+
+### Using order book steps for sell price
+
+Using `order_book_min` and `order_book_max` used to allow stepping the orderbook and trying to find the next ROI slot - trying to place sell-orders early.
+As this does however increase risk and provides no benefit, it's been removed for maintainability purposes in 2021.7.
--- a/docs/includes/pricing.md
+++ b/docs/includes/pricing.md
@ -47,7 +47,7 @@ Also, prices at the "ask" side of the spread are higher than prices at the "bid"

 #### Buy price with Orderbook enabled

-When buying with the orderbook enabled (`bid_strategy.use_order_book=True`), Freqtrade fetches the `bid_strategy.order_book_top` entries from the orderbook and then uses the entry specified as `bid_strategy.order_book_top` on the configured side (`bid_strategy.price_side`) of the orderbook. 1 specifies the topmost entry in the orderbook, while 2 would use the 2nd entry in the orderbook, and so on.
+When buying with the orderbook enabled (`bid_strategy.use_order_book=True`), Freqtrade fetches the `bid_strategy.order_book_top` entries from the orderbook and uses the entry specified as `bid_strategy.order_book_top` on the configured side (`bid_strategy.price_side`) of the orderbook. 1 specifies the topmost entry in the orderbook, while 2 would use the 2nd entry in the orderbook, and so on.

 #### Buy price without Orderbook enabled

@ -82,22 +82,9 @@ In line with that, if `ask_strategy.price_side` is set to `"bid"`, then the bot

 #### Sell price with Orderbook enabled

-When selling with the orderbook enabled (`ask_strategy.use_order_book=True`), Freqtrade fetches the `ask_strategy.order_book_max` entries in the orderbook. Then each of the orderbook steps between `ask_strategy.order_book_min` and `ask_strategy.order_book_max` on the configured orderbook side are validated for a profitable sell-possibility based on the strategy configuration (`minimal_roi` conditions) and the sell order is placed at the first profitable spot.
+When selling with the orderbook enabled (`ask_strategy.use_order_book=True`), Freqtrade fetches the `ask_strategy.order_book_top` entries in the orderbook and uses the entry specified as `ask_strategy.order_book_top` from the configured side (`ask_strategy.price_side`) as selling price.

-!!! Note
-    Using `order_book_max` higher than `order_book_min` only makes sense when ask_strategy.price_side is set to `"ask"`.
-
-The idea here is to place the sell order early, to be ahead in the queue.
-
-A fixed slot (mirroring `bid_strategy.order_book_top`) can be defined by setting `ask_strategy.order_book_min` and `ask_strategy.order_book_max` to the same number.
-
-!!! Warning "Order_book_max > 1 - increased risks for stoplosses!"
-    Using `ask_strategy.order_book_max` higher than 1 will increase the risk the stoploss on exchange is cancelled too early, since an eventual [stoploss on exchange](#understand-order_types) will be cancelled as soon as the order is placed.
-    Also, the sell order will remain on the exchange for `unfilledtimeout.sell` (or until it's filled) - which can lead to missed stoplosses (with or without using stoploss on exchange).
-
-!!! Warning "Order_book_max > 1 in dry-run"
-    Using `ask_strategy.order_book_max` higher than 1 will result in improper dry-run results (significantly better than real orders executed on exchange), since dry-run assumes orders to be filled almost instantly.
-    It is therefore advised to not use this setting for dry-runs.
+1 specifies the topmost entry in the orderbook, while 2 would use the 2nd entry in the orderbook, and so on.

 #### Sell price without Orderbook enabled

--- a/freqtrade/configuration/config_validation.py
+++ b/freqtrade/configuration/config_validation.py
@ -79,6 +79,7 @@ def validate_config_consistency(conf: Dict[str, Any]) -> None:
    _validate_whitelist(conf)
    _validate_protections(conf)
    _validate_unlimited_amount(conf)
+    _validate_ask_orderbook(conf)

    # validate configuration before returning
    logger.info('Validating configuration ...')
@ -186,3 +187,23 @@ def _validate_protections(conf: Dict[str, Any]) -> None:
                "Protections must specify either `lookback_period` or `lookback_period_candles`.\n"
                f"Please fix the protection {prot.get('method')}"
            )
+
+
+def _validate_ask_orderbook(conf: Dict[str, Any]) -> None:
+    ask_strategy = conf.get('ask_strategy', {})
+    ob_min = ask_strategy.get('order_book_min')
+    ob_max = ask_strategy.get('order_book_max')
+    if ob_min is not None and ob_max is not None and ask_strategy.get('use_order_book'):
+        if ob_min != ob_max:
+            raise OperationalException(
+                "Using order_book_max != order_book_min in ask_strategy is no longer supported."
+                "Please pick one value and use `order_book_top` in the future."
+            )
+        else:
+            # Move value to order_book_top
+            ask_strategy['order_book_top'] = ob_min
+            logger.warning(
+                "DEPRECATED: "
+                "Please use `order_book_top` instead of `order_book_min` and `order_book_max` "
+                "for your `ask_strategy` configuration."
+            )
--- a/freqtrade/constants.py
+++ b/freqtrade/constants.py
@ -154,7 +154,7 @@ CONF_SCHEMA = {
                },
                'price_side': {'type': 'string', 'enum': ORDERBOOK_SIDES, 'default': 'bid'},
                'use_order_book': {'type': 'boolean'},
-                'order_book_top': {'type': 'integer', 'maximum': 20, 'minimum': 1},
+                'order_book_top': {'type': 'integer', 'minimum': 1, 'maximum': 50, },
                'check_depth_of_market': {
                    'type': 'object',
                    'properties': {
@ -163,7 +163,7 @@ CONF_SCHEMA = {
                    }
                },
            },
-            'required': ['ask_last_balance']
+            'required': ['price_side']
        },
        'ask_strategy': {
            'type': 'object',
@ -176,13 +176,13 @@ CONF_SCHEMA = {
                    'exclusiveMaximum': False,
                },
                'use_order_book': {'type': 'boolean'},
-                'order_book_min': {'type': 'integer', 'minimum': 1},
-                'order_book_max': {'type': 'integer', 'minimum': 1, 'maximum': 50},
+                'order_book_top': {'type': 'integer', 'minimum': 1, 'maximum': 50, },
                'use_sell_signal': {'type': 'boolean'},
                'sell_profit_only': {'type': 'boolean'},
                'sell_profit_offset': {'type': 'number'},
                'ignore_roi_if_buy_signal': {'type': 'boolean'}
-            }
+            },
+            'required': ['price_side']
        },
        'order_types': {
            'type': 'object',
--- a/freqtrade/exchange/exchange.py
+++ b/freqtrade/exchange/exchange.py
@ -999,15 +999,6 @@ class Exchange:
        except ccxt.BaseError as e:
            raise OperationalException(e) from e

-    def _order_book_gen(self, pair: str, side: str, order_book_max: int = 1,
-                        order_book_min: int = 1):
-        """
-        Helper generator to query orderbook in loop (used for early sell-order placing)
-        """
-        order_book = self.fetch_l2_order_book(pair, order_book_max)
-        for i in range(order_book_min, order_book_max + 1):
-            yield order_book[side][i - 1][0]
-
    def get_buy_rate(self, pair: str, refresh: bool) -> float:
        """
        Calculates bid target between current ask price and last price
@ -1076,10 +1067,15 @@ class Exchange:
            logger.info(
                f"Getting price from order book {ask_strategy['price_side'].capitalize()} side."
            )
+            order_book_top = ask_strategy.get('order_book_top', 1)
+            order_book = self.fetch_l2_order_book(pair, order_book_top)
            try:
-                rate = next(self._order_book_gen(pair, f"{ask_strategy['price_side']}s"))
+                rate = order_book[f"{ask_strategy['price_side']}s"][order_book_top - 1][0]
            except (IndexError, KeyError) as e:
-                logger.warning("Sell Price at location from orderbook could not be determined.")
+                logger.warning(
+                    f"Sell Price at location {order_book_top} from orderbook could not be "
+                    f"determined. Orderbook: {order_book}"
+                )
                raise PricingError from e
        else:
            ticker = self.fetch_ticker(pair)
--- a/freqtrade/freqtradebot.py
+++ b/freqtrade/freqtradebot.py
@ -693,37 +693,10 @@ class FreqtradeBot(LoggingMixin):

            (buy, sell) = self.strategy.get_signal(trade.pair, self.strategy.timeframe, analyzed_df)

-        if config_ask_strategy.get('use_order_book', False):
-            order_book_min = config_ask_strategy.get('order_book_min', 1)
-            order_book_max = config_ask_strategy.get('order_book_max', 1)
-            logger.debug(f'Using order book between {order_book_min} and {order_book_max} '
-                         f'for selling {trade.pair}...')
-
-            order_book = self.exchange._order_book_gen(
-                trade.pair, f"{config_ask_strategy['price_side']}s",
-                order_book_min=order_book_min, order_book_max=order_book_max)
-            for i in range(order_book_min, order_book_max + 1):
-                try:
-                    sell_rate = next(order_book)
-                except (IndexError, KeyError) as e:
-                    logger.warning(
-                        f"Sell Price at location {i} from orderbook could not be determined."
-                    )
-                    raise PricingError from e
-                logger.debug(f"  order book {config_ask_strategy['price_side']} top {i}: "
-                             f"{sell_rate:0.8f}")
-                # Assign sell-rate to cache - otherwise sell-rate is never updated in the cache,
-                # resulting in outdated RPC messages
-                self.exchange._sell_rate_cache[trade.pair] = sell_rate
-
-                if self._check_and_execute_sell(trade, sell_rate, buy, sell):
-                    return True
-
-        else:
-            logger.debug('checking sell')
-            sell_rate = self.exchange.get_sell_rate(trade.pair, True)
-            if self._check_and_execute_sell(trade, sell_rate, buy, sell):
-                return True
+        logger.debug('checking sell')
+        sell_rate = self.exchange.get_sell_rate(trade.pair, True)
+        if self._check_and_execute_sell(trade, sell_rate, buy, sell):
+            return True

        logger.debug('Found no sell signal for %s.', trade)
        return False
--- a/freqtrade/templates/base_config.json.j2
+++ b/freqtrade/templates/base_config.json.j2
@ -15,7 +15,7 @@
    "bid_strategy": {
        "price_side": "bid",
        "ask_last_balance": 0.0,
-        "use_order_book": false,
+        "use_order_book": true,
        "order_book_top": 1,
        "check_depth_of_market": {
            "enabled": false,
@ -24,9 +24,8 @@
    },
    "ask_strategy": {
        "price_side": "ask",
-        "use_order_book": false,
-        "order_book_min": 1,
-        "order_book_max": 1,
+        "use_order_book": true,
+        "order_book_top": 1,
        "use_sell_signal": true,
        "sell_profit_only": false,
        "ignore_roi_if_buy_signal": false
--- a/tests/conftest.py
+++ b/tests/conftest.py
@ -290,8 +290,7 @@ def get_default_conf(testdatadir):
        },
        "ask_strategy": {
            "use_order_book": False,
-            "order_book_min": 1,
-            "order_book_max": 1
+            "order_book_top": 1,
        },
        "exchange": {
            "name": "binance",
--- a/tests/exchange/test_exchange.py
+++ b/tests/exchange/test_exchange.py
@ -1844,8 +1844,7 @@ def test_get_sell_rate_orderbook(default_conf, mocker, caplog, side, expected, o
    # Test orderbook mode
    default_conf['ask_strategy']['price_side'] = side
    default_conf['ask_strategy']['use_order_book'] = True
-    default_conf['ask_strategy']['order_book_min'] = 1
-    default_conf['ask_strategy']['order_book_max'] = 2
+    default_conf['ask_strategy']['order_book_top'] = 1
    pair = "ETH/BTC"
    mocker.patch('freqtrade.exchange.Exchange.fetch_l2_order_book', order_book_l2)
    exchange = get_patched_exchange(mocker, default_conf)
@ -1862,8 +1861,7 @@ def test_get_sell_rate_orderbook_exception(default_conf, mocker, caplog):
    # Test orderbook mode
    default_conf['ask_strategy']['price_side'] = 'ask'
    default_conf['ask_strategy']['use_order_book'] = True
-    default_conf['ask_strategy']['order_book_min'] = 1
-    default_conf['ask_strategy']['order_book_max'] = 2
+    default_conf['ask_strategy']['order_book_top'] = 1
    pair = "ETH/BTC"
    # Test What happens if the exchange returns an empty orderbook.
    mocker.patch('freqtrade.exchange.Exchange.fetch_l2_order_book',
@ -1871,7 +1869,8 @@ def test_get_sell_rate_orderbook_exception(default_conf, mocker, caplog):
    exchange = get_patched_exchange(mocker, default_conf)
    with pytest.raises(PricingError):
        exchange.get_sell_rate(pair, True)
-    assert log_has("Sell Price at location from orderbook could not be determined.", caplog)
+    assert log_has_re(r"Sell Price at location 1 from orderbook could not be determined\..*",
+                      caplog)


 def test_get_sell_rate_exception(default_conf, mocker, caplog):
--- a/tests/test_configuration.py
+++ b/tests/test_configuration.py
@ -935,6 +935,23 @@ def test_validate_protections(default_conf, protconf, expected):
        validate_config_consistency(conf)


+def test_validate_ask_orderbook(default_conf, caplog) -> None:
+    conf = deepcopy(default_conf)
+    conf['ask_strategy']['use_order_book'] = True
+    conf['ask_strategy']['order_book_min'] = 2
+    conf['ask_strategy']['order_book_max'] = 2
+
+    validate_config_consistency(conf)
+    assert log_has_re(r"DEPRECATED: Please use `order_book_top` instead of.*", caplog)
+    assert conf['ask_strategy']['order_book_top'] == 2
+
+    conf['ask_strategy']['order_book_max'] = 5
+
+    with pytest.raises(OperationalException,
+                       match=r"Using order_book_max != order_book_min in ask_strategy.*"):
+        validate_config_consistency(conf)
+
+
 def test_load_config_test_comments() -> None:
    """
    Load config with comments
--- a/tests/test_freqtradebot.py
+++ b/tests/test_freqtradebot.py
@ -3990,8 +3990,7 @@ def test_order_book_ask_strategy(default_conf, limit_buy_order_open, limit_buy_o
    mocker.patch('freqtrade.exchange.Exchange.fetch_l2_order_book', order_book_l2)
    default_conf['exchange']['name'] = 'binance'
    default_conf['ask_strategy']['use_order_book'] = True
-    default_conf['ask_strategy']['order_book_min'] = 1
-    default_conf['ask_strategy']['order_book_max'] = 2
+    default_conf['ask_strategy']['order_book_top'] = 1
    default_conf['telegram']['enabled'] = False
    patch_RPCManager(mocker)
    patch_exchange(mocker)
@ -4027,7 +4026,8 @@ def test_order_book_ask_strategy(default_conf, limit_buy_order_open, limit_buy_o
                 return_value={'bids': [[]], 'asks': [[]]})
    with pytest.raises(PricingError):
        freqtrade.handle_trade(trade)
-    assert log_has('Sell Price at location 1 from orderbook could not be determined.', caplog)
+    assert log_has_re(r'Sell Price at location 1 from orderbook could not be determined\..*',
+                      caplog)


 def test_startup_state(default_conf, mocker):