Repository: bitcoin/bitcoin (language: cpp)

Issue:
Change estimate_mode default to "ECONOMICAL" in these RPC calls
The default fee estimate mode for the following RPC calls is set to "ECONOMICAL" rather than "CONSERVATIVE".

My observation running a Bitcoin non-custodial exchange and payment processor which creates a lot transactions is that Bitcoin Core on conservative mode consistently overpays transaction fees as compared to mempool's fee estimation algorithm. I have collected data on this, if anyone needs convincing, but I think this is common knowledge.

Since RBF, it is standard best practice for advanced Bitcoin users to "lowall" transaction fees instead and RBF later if their transaction. Unfortunately, Bitcoin Core users and wallets which use Bitcoin Core's fee estimation will probably use the default mode (as users always do) and this will lead them to "overpay" more often than not.

Therefore, I suggest changing the default fee estimation mode to "ECONOMICAL" in the following RPCs

[estimatesmartfee](https://developer.bitcoin.org/reference/rpc/estimatesmartfee.html?highlight=estimate_mode)

[send](https://developer.bitcoin.org/reference/rpc/send.html?highlight=estimate_mode)

[sendmany](https://developer.bitcoin.org/reference/rpc/sendmany.html?highlight=estimate_mode)

[sendtoaddress](https://developer.bitcoin.org/reference/rpc/sendtoaddress.html?highlight=estimate_mode)

[fundrawtransaction](https://developer.bitcoin.org/reference/rpc/fundrawtransaction.html?highlight=estimate_mode)

[Getting Started](https://developer.bitcoin.org/index.html?highlight=estimate_mode)

[walletcreatefundedpsbt](https://developer.bitcoin.org/reference/rpc/walletcreatefundedpsbt.html?highlight=estimate_mode)

I would exclude this change from RBF related RPCs, where the user probably wants the RBF to be sucessful and is clearly willing to pay more for faster confirmation.

[psbtbumpfee](https://developer.bitcoin.org/reference/rpc/psbtbumpfee.html?highlight=estimate_mode)

[bumpfee](https://developer.bitcoin.org/reference/rpc/bumpfee.html?highlight=estimate_mode)

I'm not a developer but am willing to financially support a developer willing to make a PR for this if there is interest.



Discussion / hints:
Another option would be to have a config for default estimate mode but in that case also the default should be "ECONOMICAL" and not "CONSERVATIVE"
> Therefore, I suggest changing the default fee estimation mode to "ECONOMICAL" in the following RPCs

Unfortunately even "economic" mode still overestimate so changing default mode to "economic" does not fix this issue.

There is an ongoing effort "see [delving post](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703)" where contributors are discussing the challenges of `CBlockPolicyEstimator` which is the fee estimation strategy used by all those RPC's.

Yes this is a known issue and there is a work in progress to resolve it.

> Unfortunately even "economic" mode still overestimate so changing default mode to "economic" does not fix this issue.

Maybe it does not fix it all together, but it makes it overestimate less.

The difference between ECONOMICAL and CONSERVATIVE for a confirmation target of  2 to 12 blocks is HUGE (by a factor of 7)
Here is recent data from our node with a confirmation target of 2 blocks
ECONOMICAL:   0.00020106 
CONSERVATIVE:  0.00141359

The CONSERVATIVE mode uses the long stats for the 95% confirmation target, they have a half-time of 1008 blocks!
Any spike in fees and number transaction will get into the long stats and decay only very very slowly.
Agreed that switching to economical is not a long-term fix for fee estimation, but it's still a notable improvement. Implementing this change would likely lead to a very noticeable reduction in fees paid by Bitcoin Core users, and users of Bitcoin Core fee estimation, and Bitcoin users as a whole.

To give an idea of how much this affects a business which does a lot of Bitcoin transaction, we actually created software that will pull the recomended fees from the mempool api and we feed that to Bitcoin Core as feerate instead of using the confirmation target.

Not everybody has the technical capacity to do that, switching to economical as default would provide much needed relief without any additional complexity added to the fee estimation algorithm. 

Willing to create a PR for this if desired!
Are you sure this isn't already happening?

This code suggests it does, but I haven't tested:

https://github.com/bitcoin/bitcoin/blob/9d1a286f20b8a602ffe72928bcd79be09fdbf9d0/src/wallet/fees.cpp#L52-L53

cc @achow101
> This code suggests it does, but I haven't tested:

`estimatesmartfee` rpc defaults to conservative, even if all the wallet stuff defaults to economical via defaulting to rbf-enabled.
(#10589 might be worth looking at for context)

Fix patch:
diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp
index aefe78162be35..662d24ef811f6 100644
--- a/src/rpc/fees.cpp
+++ b/src/rpc/fees.cpp
@@ -36,7 +36,7 @@ static RPCHelpMan estimatesmartfee()
         "in BIP 141 (witness data is discounted).\n",
         {
             {"conf_target", RPCArg::Type::NUM, RPCArg::Optional::NO, "Confirmation target in blocks (1 - 1008)"},
-            {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"conservative"}, "The fee estimate mode.\n"
+            {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"economical"}, "The fee estimate mode.\n"
             "Whether to return a more conservative estimate which also satisfies\n"
             "a longer history. A conservative estimate potentially returns a\n"
             "higher feerate and is more likely to be sufficient for the desired\n"
@@ -71,13 +71,13 @@ static RPCHelpMan estimatesmartfee()
             CHECK_NONFATAL(mempool.m_opts.signals)->SyncWithValidationInterfaceQueue();
             unsigned int max_target = fee_estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
             unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target);
-            bool conservative = true;
+            bool conservative = false;
             if (!request.params[1].isNull()) {
                 FeeEstimateMode fee_mode;
                 if (!FeeModeFromString(request.params[1].get_str(), fee_mode)) {
                     throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage());
                 }
-                if (fee_mode == FeeEstimateMode::ECONOMICAL) conservative = false;
+                if (fee_mode == FeeEstimateMode::CONSERVATIVE) conservative = true;
             }
 
             UniValue result(UniValue::VOBJ);


Test patch:
diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py
index ffc87f8b8bad0..8cc747728490f 100755
--- a/test/functional/feature_fee_estimation.py
+++ b/test/functional/feature_fee_estimation.py
@@ -128,6 +128,14 @@ def make_tx(wallet, utxo, feerate):
         fee_rate=Decimal(feerate * 1000) / COIN,
     )
 
+def check_fee_estimates_btw_modes(node, expected_conservative, expected_economical):
+    fee_est_conservative = node.estimatesmartfee(1, estimate_mode="conservative")['feerate']
+    fee_est_economical = node.estimatesmartfee(1, estimate_mode="economical")['feerate']
+    fee_est_default = node.estimatesmartfee(1)['feerate']
+    assert_equal(fee_est_conservative, expected_conservative)
+    assert_equal(fee_est_economical, expected_economical)
+    assert_equal(fee_est_default, expected_economical)
+
 
 class EstimateFeeTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -382,6 +390,40 @@ def test_acceptstalefeeestimates_option(self):
         self.start_node(0,extra_args=["-acceptstalefeeestimates"])
         assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate)
 
+    def clear_estimates(self):
+        self.log.info("Restarting node with fresh estimation")
+        self.stop_node(0)
+        fee_dat = self.nodes[0].chain_path / "fee_estimates.dat"
+        os.remove(fee_dat)
+        self.start_node(0)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(0, 2)
+        assert_equal(self.nodes[0].estimatesmartfee(1)["errors"], ["Insufficient data or no feerate found"])
+
+    def broadcast_and_mine(self, broadcaster, miner, feerate, count):
+        """Broadcast and mine some number of transactions with a specified fee rate."""
+        for _ in range(count):
+            self.wallet.send_self_transfer(from_node=broadcaster, fee_rate=feerate)
+        self.sync_mempools()
+        self.generate(miner, 1)
+
+    def test_estimation_modes(self):
+        low_feerate = Decimal("0.001")
+        high_feerate = Decimal("0.005")
+        tx_count = 24
+        # Broadcast and mine high fee transactions for the first 12 blocks.
+        for _ in range(12):
+            self.broadcast_and_mine(self.nodes[1], self.nodes[2], high_feerate, tx_count)
+        check_fee_estimates_btw_modes(self.nodes[0], high_feerate, high_feerate)
+
+        # We now track 12 blocks; short horizon stats will start decaying.
+        # Broadcast and mine low fee transactions for the next 4 blocks.
+        for _ in range(4):
+            self.broadcast_and_mine(self.nodes[1], self.nodes[2], low_feerate, tx_count)
+        # conservative mode will consider longer time horizons while economical mode does not
+        # Check the fee estimates for both modes after mining low fee transactions.
+        check_fee_estimates_btw_modes(self.nodes[0], high_feerate, low_feerate)
+
 
     def run_test(self):
         self.log.info("This test is time consuming, please be patient")
@@ -420,17 +462,15 @@ def run_test(self):
         self.log.info("Test reading old fee_estimates.dat")
         self.test_old_fee_estimate_file()
 
-        self.log.info("Restarting node with fresh estimation")
-        self.stop_node(0)
-        fee_dat = os.path.join(self.nodes[0].chain_path, "fee_estimates.dat")
-        os.remove(fee_dat)
-        self.start_node(0)
-        self.connect_nodes(0, 1)
-        self.connect_nodes(0, 2)
+        self.clear_estimates()
 
         self.log.info("Testing estimates with RBF.")
         self.sanity_check_rbf_estimates(self.confutxo + self.memutxo)
 
+        self.clear_estimates()
+        self.log.info("Test estimatesmartfee modes")
+        self.test_estimation_modes()
+
         self.log.info("Testing that fee estimation is disabled in blocksonly.")
         self.restart_node(0, ["-blocksonly"])
         assert_raises_rpc_error(