From 970f02096d37447d04bf84993f2922a1e6f7d7a7 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Fri, 21 Nov 2025 14:51:12 +0000 Subject: [PATCH] fees: persist mempool policy estimator data Persist MemPoolFeeRateEstimator's recent mined-block statistics to fees/mempool_policy_estimator.dat and reload them at startup. Without this, the mempool estimator starts cold after each restart and treats the mempool as unhealthy until MEMPOOL_HEALTH_WINDOW_BLOCKS blocks have been observed, causing the default combined estimatesmartfee request to return a mempool fee rate estimator error. Files with more stats than MEMPOOL_HEALTH_WINDOW_BLOCKS, non-consecutive block heights, or a final block that does not match the active chain tip are rejected on read, preserving the invariant that loaded stats describe the current chain. Add MempoolPolicyEstimatorPath(), pass the path through FeeRateEstimatorManager, and flush both block-policy and mempool-policy estimator files on interval and shutdown. --- doc/files.md | 2 +- src/init.cpp | 2 +- src/policy/fees/estimator_args.cpp | 6 + src/policy/fees/estimator_args.h | 3 + src/policy/fees/estimator_man.cpp | 10 +- src/policy/fees/estimator_man.h | 7 +- src/policy/fees/mempool_estimator.cpp | 167 ++++++++++++++++++++++ src/policy/fees/mempool_estimator.h | 17 ++- src/test/mempool_fee_estimator_tests.cpp | 7 +- src/wallet/test/fuzz/fees.cpp | 2 +- test/functional/feature_fee_estimation.py | 128 ++++++++++++----- 11 files changed, 309 insertions(+), 42 deletions(-) diff --git a/doc/files.md b/doc/files.md index 3eda7919856..7176b5c6993 100644 --- a/doc/files.md +++ b/doc/files.md @@ -54,7 +54,7 @@ Subdirectory | File(s) | Description `blocks/` | `revNNNNN.dat`[\[2\]](#note2) | Block undo data (custom format) `blocks/` | `xor.dat` | Rolling XOR pattern for block and undo data files `chainstate/` | LevelDB database | Blockchain state (a compact representation of all currently unspent transaction outputs (UTXOs) and metadata about the transactions they are from) -`fees/` | `block_policy_estimates.dat` | Stores statistics used to estimate minimum transaction fees required for confirmation +`fees/` | `block_policy_estimates.dat` and `mempool_policy_estimator.dat` | Stores block policy and mempool policy estimator data `indexes/txindex/` | LevelDB database | Transaction index; *optional*, used if `-txindex=1` `indexes/txospenderindex/` | LevelDB database | Transaction spender index; *optional*, used if `-txospenderindex=1` `indexes/blockfilter/basic/db/` | LevelDB database | Blockfilter index LevelDB database for the basic filtertype; *optional*, used if `-blockfilterindex=basic` diff --git a/src/init.cpp b/src/init.cpp index 5dee251f2a2..c24d30dd3a3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1918,7 +1918,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(strprintf(_("acceptstalefeeestimates is not supported on %s chain."), chainparams.GetChainTypeString())); } MaybeMigrateLegacyFeeEstimates(args); - node.fee_estimator_man = std::make_unique(BlockPolicyFeeEstPath(args), read_stale_estimates, *Assert(node.mempool), chainman); + node.fee_estimator_man = std::make_unique(BlockPolicyFeeEstPath(args), read_stale_estimates, MempoolPolicyEstimatorPath(args), *Assert(node.mempool), chainman); // Flush estimates to disk periodically FeeRateEstimatorManager* fee_estimator_man = node.fee_estimator_man.get(); diff --git a/src/policy/fees/estimator_args.cpp b/src/policy/fees/estimator_args.cpp index 09366276669..edb72017a49 100644 --- a/src/policy/fees/estimator_args.cpp +++ b/src/policy/fees/estimator_args.cpp @@ -13,6 +13,7 @@ namespace { constexpr const char* FEES_BASE_DIR{"fees"}; constexpr const char* BLOCK_POLICY_ESTIMATES_FILENAME{"block_policy_estimates.dat"}; constexpr const char* LEGACY_FEE_ESTIMATES_FILENAME{"fee_estimates.dat"}; +constexpr const char* MEMPOOL_POLICY_ESTIMATOR_FILENAME{"mempool_policy_estimator.dat"}; fs::path LegacyFeeEstPath(const ArgsManager& argsman) { @@ -52,3 +53,8 @@ fs::path BlockPolicyFeeEstPath(const ArgsManager& argsman) { return argsman.GetDataDirNet() / FEES_BASE_DIR / BLOCK_POLICY_ESTIMATES_FILENAME; } + +fs::path MempoolPolicyEstimatorPath(const ArgsManager& argsman) +{ + return argsman.GetDataDirNet() / FEES_BASE_DIR / MEMPOOL_POLICY_ESTIMATOR_FILENAME; +} diff --git a/src/policy/fees/estimator_args.h b/src/policy/fees/estimator_args.h index 92140ebb626..fc2bcd4e7e2 100644 --- a/src/policy/fees/estimator_args.h +++ b/src/policy/fees/estimator_args.h @@ -15,4 +15,7 @@ void MaybeMigrateLegacyFeeEstimates(const ArgsManager& argsman); /** @return The block policy fee estimator data file path. */ fs::path BlockPolicyFeeEstPath(const ArgsManager& argsman); +/** @return The mempool policy estimator data file path. */ +fs::path MempoolPolicyEstimatorPath(const ArgsManager& argsman); + #endif // BITCOIN_POLICY_FEES_ESTIMATOR_ARGS_H diff --git a/src/policy/fees/estimator_man.cpp b/src/policy/fees/estimator_man.cpp index 97e2ca549de..498e0580fdc 100644 --- a/src/policy/fees/estimator_man.cpp +++ b/src/policy/fees/estimator_man.cpp @@ -12,9 +12,13 @@ FeeRateEstimatorManager::~FeeRateEstimatorManager() = default; -FeeRateEstimatorManager::FeeRateEstimatorManager(const fs::path& block_policy_path, bool read_stale_estimates, const CTxMemPool& mempool, ChainstateManager& chainman) +FeeRateEstimatorManager::FeeRateEstimatorManager(const fs::path& block_policy_path, + bool read_stale_estimates, + const fs::path& mempool_estimator_path, + const CTxMemPool& mempool, + ChainstateManager& chainman) : m_block_policy_estimator(std::make_unique(block_policy_path, read_stale_estimates)), - m_mempool_estimator(std::make_unique(mempool, chainman)) + m_mempool_estimator(std::make_unique(mempool_estimator_path, mempool, chainman)) { } @@ -55,11 +59,13 @@ util::Expected FeeRateEstimatorManage void FeeRateEstimatorManager::IntervalFlush() { m_block_policy_estimator->FlushFeeEstimates(); + m_mempool_estimator->FlushMinedBlockStats(); } void FeeRateEstimatorManager::ShutdownFlush() { m_block_policy_estimator->Flush(); + m_mempool_estimator->FlushMinedBlockStats(); } std::vector FeeRateEstimatorManager::MempoolPolicyEstimatorBlocksStats() const diff --git a/src/policy/fees/estimator_man.h b/src/policy/fees/estimator_man.h index 1a33a004c6e..6950414ac19 100644 --- a/src/policy/fees/estimator_man.h +++ b/src/policy/fees/estimator_man.h @@ -33,10 +33,15 @@ public: /** * @param[in] block_policy_path Path to the block policy fee estimates file. * @param[in] read_stale_estimates Whether to load stale estimates from disk. + * @param[in] mempool_estimator_path Path to the mempool policy estimator data file. * @param[in] mempool The mempool to use for the mempool fee rate estimator. * @param[in] chainman The chainstate manager. */ - FeeRateEstimatorManager(const fs::path& block_policy_path, bool read_stale_estimates, const CTxMemPool& mempool, ChainstateManager& chainman); + FeeRateEstimatorManager(const fs::path& block_policy_path, + bool read_stale_estimates, + const fs::path& mempool_estimator_path, + const CTxMemPool& mempool, + ChainstateManager& chainman); virtual ~FeeRateEstimatorManager(); diff --git a/src/policy/fees/mempool_estimator.cpp b/src/policy/fees/mempool_estimator.cpp index ca8912248e4..88290ba6f26 100644 --- a/src/policy/fees/mempool_estimator.cpp +++ b/src/policy/fees/mempool_estimator.cpp @@ -8,12 +8,17 @@ #include #include #include +#include +#include +#include #include #include #include #include #include #include +#include +#include #include #include @@ -22,9 +27,25 @@ #include #include #include +#include #include +constexpr int CURRENT_MEMPOOL_ESTIMATOR_VERSION{1}; + namespace { +struct MinedBlockStatsFormatter { + template + void Ser(Stream& s, const MinedBlockStats& v) + { + s << v.m_height << v.m_removed_block_txs_weight << v.m_block_weight; + } + template + void Unser(Stream& s, MinedBlockStats& v) + { + s >> v.m_height >> v.m_removed_block_txs_weight >> v.m_block_weight; + } +}; + void AddMinedBlockStats(std::vector& mined_blocks, MinedBlockStats stats) { const auto stale_begin{std::find_if(mined_blocks.begin(), mined_blocks.end(), [&](const MinedBlockStats& block) { @@ -56,6 +77,19 @@ void AddMinedBlockStats(std::vector& mined_blocks, MinedBlockSt if (mined_blocks.size() == MEMPOOL_HEALTH_WINDOW_BLOCKS) mined_blocks.erase(mined_blocks.begin()); mined_blocks.push_back(stats); } + +struct ActiveTip { + int height; + uint256 hash; +}; + +std::optional GetActiveTip(const ChainstateManager& chainman) +{ + LOCK(::cs_main); + const CBlockIndex* tip{chainman.ActiveTip()}; + if (!tip) return std::nullopt; + return ActiveTip{tip->nHeight, tip->GetBlockHash()}; +} } // namespace MemPoolFeeRateEstimator::Percentiles MemPoolFeeRateEstimator::CalculateMaxWeightPercentiles(std::span chunk_feerates) @@ -125,6 +159,138 @@ static std::optional MempoolHealthError(MemPoolFeeRateEstimato return std::nullopt; } +MemPoolFeeRateEstimator::MemPoolFeeRateEstimator(fs::path mempool_estimator_file_path, + const CTxMemPool& mempool, + ChainstateManager& chainman) + : m_mempool(mempool), + m_chainman(chainman), + m_mempool_estimator_file_path(std::move(mempool_estimator_file_path)) +{ + ReadFromDisk(); +} + +void MemPoolFeeRateEstimator::ReadFromDisk() +{ + AutoFile file{fsbridge::fopen(m_mempool_estimator_file_path, "rb")}; + if (file.IsNull()) { + LogDebug(BCLog::ESTIMATEFEE, "%s: %s does not exist. Continuing anyway", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + fs::PathToString(m_mempool_estimator_file_path)); + return; + } + if (Read(file)) { + LogDebug(BCLog::ESTIMATEFEE, "%s: mined-block stats successfully read from %s.", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + fs::PathToString(m_mempool_estimator_file_path)); + } +} + +bool MemPoolFeeRateEstimator::Read(AutoFile& file) +{ + try { + int version_required; + file >> version_required; + if (version_required != CURRENT_MEMPOOL_ESTIMATOR_VERSION) { + LogWarning("%s: file version not supported; continuing anyway", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY)); + return false; + } + // Stage into a local buffer and commit to the member only after validation passes. + std::vector blocks; + file >> Using>(blocks); + uint256 tip_hash; + file >> tip_hash; + if (blocks.size() > MEMPOOL_HEALTH_WINDOW_BLOCKS) { + LogWarning("%s: Number of previously mined blocks read exceeds the maximum of %s; ignoring file", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + MEMPOOL_HEALTH_WINDOW_BLOCKS); + return false; + } + for (size_t i = 1; i < blocks.size(); ++i) { + if (blocks[i].m_height != blocks[i - 1].m_height + 1) { + LogWarning("%s: Non-consecutive block heights read, expected height %s but found %s; ignoring file", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + blocks[i - 1].m_height + 1, blocks[i].m_height); + return false; + } + } + if (!blocks.empty()) { + const auto& last_block{blocks.back()}; + const std::optional active_tip{GetActiveTip(m_chainman)}; + if (!active_tip) { + LogWarning("%s: Mined-block stats read end at height %s block %s, but there is no active chain tip; ignoring file", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + last_block.m_height, tip_hash.ToString()); + return false; + } + if (last_block.m_height != static_cast(active_tip->height) || tip_hash != active_tip->hash) { + LogWarning("%s: Mined-block stats read end at height %s block %s, but the active chain tip is height %s block %s; ignoring file", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + last_block.m_height, tip_hash.ToString(), + active_tip->height, active_tip->hash.ToString()); + return false; + } + } + LOCK(cs); + m_prev_mined_blocks = std::move(blocks); + m_mined_blocks_tip_hash = tip_hash; + m_cache.Clear(); + } catch (const std::exception&) { + LogWarning("%s: Unable to read mined-block stats from stream (non-fatal)", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY)); + return false; + } + return true; +} + +bool MemPoolFeeRateEstimator::Write(AutoFile& file) const +{ + try { + LOCK(cs); + file << CURRENT_MEMPOOL_ESTIMATOR_VERSION; + file << Using>(m_prev_mined_blocks); + file << m_mined_blocks_tip_hash; + } catch (const std::exception&) { + return false; + } + return true; +} + +void MemPoolFeeRateEstimator::FlushMinedBlockStats() +{ + if (!m_mempool_estimator_file_path.parent_path().empty()) { + std::error_code error; + fs::create_directories(m_mempool_estimator_file_path.parent_path(), error); + if (error) { + LogWarning("%s: failed to create mempool policy estimator directory %s: %s. Continuing anyway", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + fs::PathToString(m_mempool_estimator_file_path.parent_path()), error.message()); + return; + } + } + AutoFile file{fsbridge::fopen(m_mempool_estimator_file_path, "wb")}; + if (file.IsNull()) { + LogWarning("%s: unable to open %s for writing. Continuing anyway", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + fs::PathToString(m_mempool_estimator_file_path)); + return; + } + if (!Write(file)) { + LogWarning("%s: Unable to write mined-block stats to %s (non-fatal)", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + fs::PathToString(m_mempool_estimator_file_path)); + } + if (file.fclose() != 0) { + LogWarning("Failed to close mempool policy estimator file %s: %s. Continuing anyway.", + fs::PathToString(m_mempool_estimator_file_path), SysErrorString(errno)); + return; + } + LogDebug(BCLog::ESTIMATEFEE, "%s: mined-block stats flushed to %s.", + FeeRateEstimatorTypeToString(FeeRateEstimatorType::MEMPOOL_POLICY), + fs::PathToString(m_mempool_estimator_file_path)); +} + + void MemPoolFeeRateEstimator::MempoolTxsRemovedForBlock(const std::shared_ptr& block, const std::vector& txs_removed_for_block, unsigned int block_height) @@ -146,6 +312,7 @@ void MemPoolFeeRateEstimator::MempoolTxsRemovedForBlock(const std::shared_ptrGetHash(); m_cache.Clear(); } diff --git a/src/policy/fees/mempool_estimator.h b/src/policy/fees/mempool_estimator.h index 68fc593e70f..7420227ac34 100644 --- a/src/policy/fees/mempool_estimator.h +++ b/src/policy/fees/mempool_estimator.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -21,6 +22,7 @@ #include class CBlock; +class AutoFile; class ChainstateManager; class CTxMemPool; @@ -91,8 +93,10 @@ public: FeePerVSize p75; }; - MemPoolFeeRateEstimator(const CTxMemPool& mempool, ChainstateManager& chainman) - : m_mempool(mempool), m_chainman(chainman) {} + MemPoolFeeRateEstimator(fs::path mempool_estimator_file_path, + const CTxMemPool& mempool, + ChainstateManager& chainman); + ~MemPoolFeeRateEstimator() = default; /** * Calculate the 50th and 75th percentile fee rates from block template chunks, * sorted in descending mining-score order. A percentile is left empty when the @@ -130,15 +134,24 @@ public: MempoolHealth GetMempoolHealth() const EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Checks if recent mined blocks indicate a healthy mempool state. bool IsMempoolHealthy() const EXCLUSIVE_LOCKS_REQUIRED(!cs) { return GetMempoolHealth() == MempoolHealth::HEALTHY; } + void FlushMinedBlockStats() EXCLUSIVE_LOCKS_REQUIRED(!cs); + //! Deserialize mined-block stats without taking ownership of file. + bool Read(AutoFile& file) EXCLUSIVE_LOCKS_REQUIRED(!cs); + //! Serialize mined-block stats without taking ownership of file. + //! Callers must explicitly close file and check for errors after writing. + bool Write(AutoFile& file) const EXCLUSIVE_LOCKS_REQUIRED(!cs); private: + void ReadFromDisk() EXCLUSIVE_LOCKS_REQUIRED(!cs); //! Tracks weight statistics for the last MEMPOOL_HEALTH_WINDOW_BLOCKS mined blocks. std::vector m_prev_mined_blocks GUARDED_BY(cs); + uint256 m_mined_blocks_tip_hash GUARDED_BY(cs); const CTxMemPool& m_mempool; ChainstateManager& m_chainman; mutable Mutex cs; mutable MemPoolFeeRateEstimatorCache m_cache GUARDED_BY(cs); + const fs::path m_mempool_estimator_file_path; }; #endif // BITCOIN_POLICY_FEES_MEMPOOL_ESTIMATOR_H diff --git a/src/test/mempool_fee_estimator_tests.cpp b/src/test/mempool_fee_estimator_tests.cpp index cb31fb48b36..23b84b0b488 100644 --- a/src/test/mempool_fee_estimator_tests.cpp +++ b/src/test/mempool_fee_estimator_tests.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license. See the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include +#include #include #include #include @@ -128,7 +130,7 @@ BOOST_AUTO_TEST_CASE(mempool_fee_rate_estimator_cache) BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) { - auto mempool_estimator = MemPoolFeeRateEstimator(*m_node.mempool, *m_node.chainman); + auto mempool_estimator = MemPoolFeeRateEstimator(MempoolPolicyEstimatorPath(*m_node.args), *m_node.mempool, *m_node.chainman); BOOST_CHECK_EQUAL(mempool_estimator.MaximumTarget(), MEMPOOL_FEE_ESTIMATOR_MAX_TARGET); // Before the mempool has finished loading, no estimate is available. { @@ -150,7 +152,8 @@ BOOST_AUTO_TEST_CASE(MempoolFeeRateEstimator) BOOST_CHECK_EQUAL(result.error().reason, insufficient_err); } { - MemPoolFeeRateEstimator custom_mempool_estimator{*m_node.mempool, *m_node.chainman}; + MemPoolFeeRateEstimator custom_mempool_estimator{ + MempoolPolicyEstimatorPath(*m_node.args), *m_node.mempool, *m_node.chainman}; unsigned int custom_height{100}; for (size_t block_count{1}; block_count < MEMPOOL_HEALTH_WINDOW_BLOCKS; ++block_count) { AddRemovedBlock(custom_mempool_estimator, diff --git a/src/wallet/test/fuzz/fees.cpp b/src/wallet/test/fuzz/fees.cpp index 781bfa6d38e..6f7167db222 100644 --- a/src/wallet/test/fuzz/fees.cpp +++ b/src/wallet/test/fuzz/fees.cpp @@ -46,7 +46,7 @@ class FuzzedFeeEstimatorMan : public FeeRateEstimatorManager public: FuzzedFeeEstimatorMan(FuzzedDataProvider& provider, const CTxMemPool& mempool, ChainstateManager& chainman) - : FeeRateEstimatorManager(fs::path{}, false, mempool, chainman), fuzzed_data_provider(provider) {} + : FeeRateEstimatorManager(fs::path{}, false, fs::path{}, mempool, chainman), fuzzed_data_provider(provider) {} util::Expected GetFeeRateEstimate(int confTarget, bool conservative) const override { diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 40539e09f6c..d3627d47e93 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -146,7 +146,7 @@ def check_fee_estimates_btw_modes(node, expected_conservative, expected_economic assert_equal(fee_est_default, expected_economical) def verify_estimate_response(estimate, feerate, errors): - if feerate: + if feerate is not None: assert_equal(estimate["feerate"], feerate) if errors: assert all(err in estimate["errors"] for err in errors) @@ -330,7 +330,7 @@ class EstimateFeeTest(BitcoinTestFramework): # Get the initial fee rate while node is running fee_rate = self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"] - # Restart node to ensure fee_estimate.dat file is read + # Restart node to ensure block policy estimator file is read self.restart_node(0) assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"], fee_rate) @@ -367,58 +367,69 @@ class EstimateFeeTest(BitcoinTestFramework): def test_estimate_dat_is_flushed_periodically(self): block_policy_fees_dat = self.nodes[0].chain_path / BLOCK_POLICY_ESTIMATOR_FILE_PATH + mempool_policy_dat = self.nodes[0].chain_path / "fees/mempool_policy_estimator.dat" + mempool_estimator_name_str = "mempool_policy" os.remove(block_policy_fees_dat) if os.path.exists(block_policy_fees_dat) else None + os.remove(mempool_policy_dat) if os.path.exists(mempool_policy_dat) else None if os.path.isdir(block_policy_fees_dat.parent): os.rmdir(block_policy_fees_dat.parent) - # Verify that block policy estimator file and its parent directory do not exist + # Verify that estimator data files and their parent directory do not exist assert_equal(os.path.isfile(block_policy_fees_dat), False) + assert_equal(os.path.isfile(mempool_policy_dat), False) assert_equal(os.path.isdir(block_policy_fees_dat.parent), False) # Verify if the string "Flushed fee estimates to block_policy_estimates.dat." is present in the debug log file. - # If present, it indicates that fee estimates have been successfully flushed to disk. - expected_messages = [f"Flushed fee estimates to {block_policy_fees_dat}."] + # If present, it indicates that fee estimator data has been successfully flushed to disk. + block_policy_estimator_message = f"Flushed fee estimates to {block_policy_fees_dat}." + mempool_policy_estimator_message = ( + f"{mempool_estimator_name_str}: mined-block stats flushed to {mempool_policy_dat}." + ) + expected_messages = [block_policy_estimator_message, mempool_policy_estimator_message] with self.nodes[0].assert_debug_log(expected_msgs=expected_messages, timeout=1): - # Mock the scheduler for an hour to flush fee estimates to block_policy_estimates.dat + # Mock the scheduler for an hour to flush estimator data. self.nodes[0].mockscheduler(SECONDS_PER_HOUR) - # Verify that fee estimates were flushed and block policy estimator directory and file are created + # Verify that estimator data was flushed and the estimator directory and files are created assert_equal(os.path.isdir(block_policy_fees_dat.parent), True) assert_equal(os.path.isfile(block_policy_fees_dat), True) - - # Verify that the estimates remain the same if there are no blocks in the flush interval + assert_equal(os.path.isfile(mempool_policy_dat), True) + # Verify that estimator data remains the same if there are no blocks in the flush interval block_hash_before = self.nodes[0].getbestblockhash() - block_policy_dat_initial_content = open(block_policy_fees_dat, "rb").read() + block_policy_fees_dat_initial_content = open(block_policy_fees_dat, "rb").read() + mempool_policy_dat_initial_content = open(mempool_policy_dat, "rb").read() with self.nodes[0].assert_debug_log(expected_msgs=expected_messages, timeout=1): - # Mock the scheduler for an hour to flush fee estimates to the block policy estimator file + # Mock the scheduler for an hour to flush estimator data. 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()) - - block_policy_fee_dat_current_content = open(block_policy_fees_dat, "rb").read() - assert_equal(block_policy_dat_initial_content, block_policy_fee_dat_current_content) - - # Verify that the estimates remain the same after shutdown with no blocks before shutdown + block_policy_fees_dat_current_content = open(block_policy_fees_dat, "rb").read() + mempool_policy_dat_current_content = open(mempool_policy_dat, "rb").read() + assert_equal(block_policy_fees_dat_initial_content, block_policy_fees_dat_current_content) + assert_equal(mempool_policy_dat_initial_content, mempool_policy_dat_current_content) + # Verify that estimator data remains the same after shutdown with no blocks before shutdown self.restart_node(0) - block_policy_fee_dat_current_content = open(block_policy_fees_dat, "rb").read() - assert_equal(block_policy_dat_initial_content, block_policy_fee_dat_current_content) - - # Verify that the estimates are not the same if new blocks were produced in the flush interval + block_policy_fees_dat_current_content = open(block_policy_fees_dat, "rb").read() + mempool_policy_dat_current_content = open(mempool_policy_dat, "rb").read() + assert_equal(block_policy_fees_dat_initial_content, block_policy_fees_dat_current_content) + assert_equal(mempool_policy_dat_initial_content, mempool_policy_dat_current_content) + # Verify that estimator data changes if new blocks were produced in the flush interval with self.nodes[0].assert_debug_log(expected_msgs=expected_messages, timeout=1): - # Mock the scheduler for an hour to flush fee estimates to block_policy_estimates.dat + # Mock the scheduler for an hour to flush estimator data. self.generate(self.nodes[0], 5, sync_fun=self.no_op) self.nodes[0].mockscheduler(SECONDS_PER_HOUR) - - block_policy_fee_dat_current_content = open(block_policy_fees_dat, "rb").read() - assert_not_equal(block_policy_fee_dat_current_content, block_policy_dat_initial_content) - block_policy_dat_initial_content = block_policy_fee_dat_current_content - - # Generate blocks before shutdown and verify that the fee estimates are not the same + block_policy_fees_dat_current_content = open(block_policy_fees_dat, "rb").read() + assert_not_equal(block_policy_fees_dat_current_content, block_policy_fees_dat_initial_content) + block_policy_fees_dat_initial_content = block_policy_fees_dat_current_content + mempool_policy_dat_current_content = open(mempool_policy_dat, "rb").read() + assert_not_equal(mempool_policy_dat_current_content, mempool_policy_dat_initial_content) + mempool_policy_dat_initial_content = mempool_policy_dat_current_content + # Generate blocks before shutdown and verify that estimator data changes self.generate(self.nodes[0], 5, sync_fun=self.no_op) self.restart_node(0) - - block_policy_fee_dat_current_content = open(block_policy_fees_dat, "rb").read() - assert_not_equal(block_policy_dat_initial_content, block_policy_fee_dat_current_content) + block_policy_fees_dat_current_content = open(block_policy_fees_dat, "rb").read() + mempool_policy_dat_current_content = open(mempool_policy_dat, "rb").read() + assert_not_equal(block_policy_fees_dat_initial_content, block_policy_fees_dat_current_content) + assert_not_equal(mempool_policy_dat_initial_content, mempool_policy_dat_current_content) def test_acceptstalefeeestimates_option(self): @@ -538,6 +549,13 @@ class EstimateFeeTest(BitcoinTestFramework): self.send_transactions(low_utxos, low_feerate, target_vsize) lower_estimate = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "none"}) verify_estimate_response(lower_estimate, low_feerate, []) + # The mempool block stats are persisted across restarts, so the mempool + # stays healthy and the lower mempool estimate is still returned after a + # restart. Without persistence, the combined estimate would return a + # mempool-policy error until enough new blocks are observed. + self.restart_node(0) + estimate_post_restart = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "none"}) + verify_estimate_response(estimate_post_restart, low_feerate, []) self.log.info("Test estimatesmartfee returns the fee rate floor when the mempool is empty but healthy") self.generate(node0, 1, sync_fun=lambda: None) @@ -553,6 +571,49 @@ class EstimateFeeTest(BitcoinTestFramework): verify_estimate_response(combined_estimate, floor, []) assert_equal(combined_estimate["estimator"], "mempool_policy") + def test_stale_mempool_block_stats_are_rejected_on_load(self): + # Persisted mempool block stats must be tied to the best block hash, + # not just height, because a reorg can replace the tip without + # changing the height. + node0 = self.nodes[0] + miner = self.nodes[1] + mempool_policy_dat = node0.chain_path / "fees/mempool_policy_estimator.dat" + healthy_feerate = Decimal("0.004") + self.connect_nodes(0, 1) + self.connect_nodes(0, 2) + self.sync_all() + # Build a full, healthy window whose tracked heights match the current tip. + self.broadcast_and_maybe_mine(node0, healthy_feerate, TXS_COUNT, 6, miner) + stale_stats = node0.estimatesmartfee( + 1, + "economical", + {"verbosity": 2, "fee_rate_estimator": "none"}, + )["mempool_health_statistics"] + assert_equal(len(stale_stats), 6) + stale_height = node0.getblockcount() + assert_equal(stale_stats[0]["block_height"], stale_height) + stale_tip = node0.getbestblockhash() + self.stop_node(0) + stale_stats_snapshot = open(mempool_policy_dat, "rb").read() + self.start_node(0) + node0.invalidateblock(stale_tip) + assert_equal(node0.getblockcount(), stale_height - 1) + reorged_tip = self.generate(node0, 1, sync_fun=lambda: None)[0] + assert_equal(node0.getblockcount(), stale_height) + assert_not_equal(reorged_tip, stale_tip) + self.stop_node(0) + with open(mempool_policy_dat, "wb") as f: + f.write(stale_stats_snapshot) + self.start_node(0) + assert_equal(node0.getblockcount(), stale_height) + assert_equal(node0.getbestblockhash(), reorged_tip) + stats_after_restart = node0.estimatesmartfee( + 1, + "economical", + {"verbosity": 2, "fee_rate_estimator": "none"}, + )["mempool_health_statistics"] + assert_equal(stats_after_restart, []) + 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") @@ -575,7 +636,7 @@ class EstimateFeeTest(BitcoinTestFramework): self.log.info("Testing estimates with single transactions.") self.sanity_check_estimates_range() - self.log.info("Test fees data is flushed periodically") + self.log.info("Test fees/block_policy_estimates.dat is flushed periodically") self.test_estimate_dat_is_flushed_periodically() # check that estimatesmartfee feerate is greater than or equal to maximum of mempoolminfee and minrelaytxfee @@ -607,6 +668,9 @@ class EstimateFeeTest(BitcoinTestFramework): self.clear_estimates() self.test_estimatesmartfee_return_mempool_estimates() + self.log.info("Test that stale mempool block stats are rejected on load") + self.test_stale_mempool_block_stats_are_rejected_on_load() + self.log.info("Testing that fee estimation is disabled in blocksonly.") self.restart_node(0, ["-blocksonly"]) assert_raises_rpc_error(