Merge bitcoin#30275: Fee Estimation: change estimatesmartfee default mode to economical

25bf86a [test]: ensure `estimatesmartfee` default mode is `economical` (ismaelsadeeq)
41a2545 [fees]: change `estimatesmartfee` default mode to `economical` (ismaelsadeeq)

Pull request description:

  Fixes bitcoin#30009

  This PR changes the `estimatesmartfee` default mode to `economical`.

  This was also suggested on IRC https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-04-26#1021609

  - `conservative` mode: This is the `estimatesmartfee` RPC mode which considers a longer history of blocks. It potentially returns a higher fee rate and is more likely to be sufficient for the desired target, but it is not as responsive to short-term drops in the prevailing fee market.
  - `economical` mode: This is the `estimatesmartfee` RPC mode where estimates are potentially lower and more responsive to short-term drops in the prevailing fee market.

  Since users are likely to use the default mode, this change will reduce overestimation for many users. The conservative mode remains available for those who wish to opt-in.

  For an in-depth analysis of how significantly the `conservative` mode overestimates, see
  https://delvingbitcoin.org/t/bitcoind-policy-estimator-modes-analysis/964.

ACKs for top commit:
  instagibbs:
    reACK bitcoin@25bf86a
  glozow:
    ACK 25bf86a
  willcl-ark:
    ACK 25bf86a

Tree-SHA512: 78ebda667eb9c8f87dcc2f0e6c14968bd1de30358dc77a13611b186fb8427ad97d9f537bad6e32e0a1aa477ccd8c64fee4d41e19308ef3cb184ff1664e6ba8a6

Author: fanquake

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/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(