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(