Merge bitcoin/bitcoin#27622: Fee estimation: avoid serving stale fee estimate

d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576 test: ensure old fee_estimate.dat not read on restart and flushed (ismaelsadeeq)
cf219f29f3c5b41070eaab9a549a476f01990f3a tx fees, policy: read stale fee estimates with a regtest-only option (ismaelsadeeq)
3eb241a141defa564c94cb95c5bbaf4c5bd9682e tx fees, policy: do not read estimates of old fee_estimates.dat (ismaelsadeeq)
5b886f2b436eaa8c2b7de58dc4644dc6223040da tx fees, policy: periodically flush fee estimates to fee_estimates.dat (ismaelsadeeq)

Pull request description:

  Fixes #27555

  The issue arises when an old `fee_estimates.dat` file is sometimes read during initialization.
  Or after an unclean shutdown, the latest fee estimates are not flushed to `fee_estimates.dat`.
  If the fee estimates in the old file are old, they can cause transactions to become stuck in the mempool.
  This  PR ensures that nodes do not use stale estimates from the old file during initialization. If  `fee_estimates.dat`
  has not been updated for 60 hours or more, it is considered stale and will not be read during initialization. To avoid
  having old estimates, the `fee_estimates.dat` file will be flushed periodically every hour. As mentioned #27555

  > "The immediate improvement would be to store fee estimates to disk once an hour or so to reduce the chance of having an old file. From there, this case could probably be detected, and refuse to serve estimates until we sync."

  In addition, I will follow-up PR to persist the `mempoolminfee` across restarts.

ACKs for top commit:
  willcl-ark:
    ACK d2b39e09bc
  instagibbs:
    reACK https://github.com/bitcoin/bitcoin/pull/27622/commits/d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576
  glozow:
    ACK d2b39e09bc6a5982fc5cf4b538b7fdb0e3cae576. One nit if you follow up.

Tree-SHA512: 4f6e0c296995d0eea5cf80c6aefdd79b7295a6a0ba446f2166f32afc105fe4f831cfda1ad3abd13c5c752b4fbea982cf4b97eaeda2af1fd7184670d41edcfeec

1 files changed, 101 insertions, 0 deletions

diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py
index 05ee556ece..03970415ac 100755
--- a/test/functional/feature_fee_estimation.py
+++ b/test/functional/feature_fee_estimation.py
@@ -7,6 +7,7 @@ from copy import deepcopy
 from decimal import Decimal
 import os
 import random
+import time
 
 from test_framework.messages import (
     COIN,
@@ -21,6 +22,8 @@ from test_framework.util import (
 )
 from test_framework.wallet import MiniWallet
 
+MAX_FILE_AGE = 60
+SECONDS_PER_HOUR = 60 * 60
 
 def small_txpuzzle_randfee(
     wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment, batch_reqs
@@ -290,6 +293,95 @@ class EstimateFeeTest(BitcoinTestFramework):
         est_feerate = node.estimatesmartfee(2)["feerate"]
         assert_equal(est_feerate, high_feerate_kvb)
 
+    def test_old_fee_estimate_file(self):
+        # Get the initial fee rate while node is running
+        fee_rate = self.nodes[0].estimatesmartfee(1)["feerate"]
+
+        # Restart node to ensure fee_estimate.dat file is read
+        self.restart_node(0)
+        assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate)
+
+        fee_dat = self.nodes[0].chain_path / "fee_estimates.dat"
+
+        # Stop the node and backdate the fee_estimates.dat file more than MAX_FILE_AGE
+        self.stop_node(0)
+        last_modified_time = time.time() - (MAX_FILE_AGE + 1) * SECONDS_PER_HOUR
+        os.utime(fee_dat, (last_modified_time, last_modified_time))
+
+        # Start node and ensure the fee_estimates.dat file was not read
+        self.start_node(0)
+        assert_equal(self.nodes[0].estimatesmartfee(1)["errors"], ["Insufficient data or no feerate found"])
+
+
+    def test_estimate_dat_is_flushed_periodically(self):
+        fee_dat = self.nodes[0].chain_path / "fee_estimates.dat"
+        os.remove(fee_dat) if os.path.exists(fee_dat) else None
+
+        # Verify that fee_estimates.dat does not exist
+        assert_equal(os.path.isfile(fee_dat), False)
+
+        # Verify if the string "Flushed fee estimates to fee_estimates.dat." is present in the debug log file.
+        # If present, it indicates that fee estimates have been successfully flushed to disk.
+        with self.nodes[0].assert_debug_log(expected_msgs=["Flushed fee estimates to fee_estimates.dat."], timeout=1):
+            # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat
+            self.nodes[0].mockscheduler(SECONDS_PER_HOUR)
+
+        # Verify that fee estimates were flushed and fee_estimates.dat file is created
+        assert_equal(os.path.isfile(fee_dat), True)
+
+        # Verify that the estimates remain the same if there are no blocks in the flush interval
+        block_hash_before = self.nodes[0].getbestblockhash()
+        fee_dat_initial_content = open(fee_dat, "rb").read()
+        with self.nodes[0].assert_debug_log(expected_msgs=["Flushed fee estimates to fee_estimates.dat."], timeout=1):
+            # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat
+            self.nodes[0].mockscheduler(SECONDS_PER_HOUR)
+
+        # Verify that there were no blocks in between the flush interval
+        assert_equal(block_hash_before, self.nodes[0].getbestblockhash())
+
+        fee_dat_current_content = open(fee_dat, "rb").read()
+        assert_equal(fee_dat_current_content, fee_dat_initial_content)
+
+        # Verify that the estimates remain the same after shutdown with no blocks before shutdown
+        self.restart_node(0)
+        fee_dat_current_content = open(fee_dat, "rb").read()
+        assert_equal(fee_dat_current_content, fee_dat_initial_content)
+
+        # Verify that the estimates are not the same if new blocks were produced in the flush interval
+        with self.nodes[0].assert_debug_log(expected_msgs=["Flushed fee estimates to fee_estimates.dat."], timeout=1):
+            # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat
+            self.generate(self.nodes[0], 5, sync_fun=self.no_op)
+            self.nodes[0].mockscheduler(SECONDS_PER_HOUR)
+
+        fee_dat_current_content = open(fee_dat, "rb").read()
+        assert fee_dat_current_content != fee_dat_initial_content
+
+        fee_dat_initial_content = fee_dat_current_content
+
+        # Generate blocks before shutdown and verify that the fee estimates are not the same
+        self.generate(self.nodes[0], 5, sync_fun=self.no_op)
+        self.restart_node(0)
+        fee_dat_current_content = open(fee_dat, "rb").read()
+        assert fee_dat_current_content != fee_dat_initial_content
+
+
+    def test_acceptstalefeeestimates_option(self):
+        # Get the initial fee rate while node is running
+        fee_rate = self.nodes[0].estimatesmartfee(1)["feerate"]
+
+        self.stop_node(0)
+
+        fee_dat = self.nodes[0].chain_path / "fee_estimates.dat"
+
+        # Stop the node and backdate the fee_estimates.dat file more than MAX_FILE_AGE
+        last_modified_time = time.time() - (MAX_FILE_AGE + 1) * SECONDS_PER_HOUR
+        os.utime(fee_dat, (last_modified_time, last_modified_time))
+
+        # Restart node with -acceptstalefeeestimates option to ensure fee_estimate.dat file is read
+        self.start_node(0,extra_args=["-acceptstalefeeestimates"])
+        assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate)
+
+
     def run_test(self):
         self.log.info("This test is time consuming, please be patient")
         self.log.info("Splitting inputs so we can generate tx's")
@@ -312,12 +404,21 @@ class EstimateFeeTest(BitcoinTestFramework):
         self.log.info("Testing estimates with single transactions.")
         self.sanity_check_estimates_range()
 
+        self.log.info("Test fee_estimates.dat is flushed periodically")
+        self.test_estimate_dat_is_flushed_periodically()
+
         # check that the effective feerate is greater than or equal to the mempoolminfee even for high mempoolminfee
         self.log.info(
             "Test fee rate estimation after restarting node with high MempoolMinFee"
         )
         self.test_feerate_mempoolminfee()
 
+        self.log.info("Test acceptstalefeeestimates option")
+        self.test_acceptstalefeeestimates_option()
+
+        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].datadir, self.chain, "fee_estimates.dat")