diff --git a/doc/files.md b/doc/files.md index e8f28692926..3eda7919856 100644 --- a/doc/files.md +++ b/doc/files.md @@ -54,6 +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 `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` @@ -65,7 +66,6 @@ Subdirectory | File(s) | Description `./` | `bitcoin.conf` | User-defined [configuration settings](bitcoin-conf.md) for `bitcoind` or `bitcoin-qt`. File is not written to by the software and must be created manually. Path can be specified by `-conf` option `./` | `bitcoind.pid` | Stores the process ID (PID) of `bitcoind` or `bitcoin-qt` while running; created at start and deleted on shutdown; can be specified by `-pid` option `./` | `debug.log` | Contains debug information and general logging generated by `bitcoind` or `bitcoin-qt`; can be specified by `-debuglogfile` option -`./` | `fee_estimates.dat` | Stores statistics used to estimate minimum transaction fees required for confirmation `./` | `guisettings.ini.bak` | Backup of former [GUI settings](#gui-settings) after `-resetguisettings` option is used `./` | `mempool.dat` | Dump of the mempool's transactions `./` | `onion_v3_private_key` | Cached Tor onion service private key for `-listenonion` option diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 917417fc12b..15b60ef9aea 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -247,7 +247,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL noui.cpp policy/ephemeral_policy.cpp policy/fees/block_policy_estimator.cpp - policy/fees/block_policy_estimator_args.cpp + policy/fees/estimator_args.cpp policy/fees/estimator_man.cpp policy/fees/mempool_estimator.cpp policy/packages.cpp diff --git a/src/init.cpp b/src/init.cpp index 1f1c7064b24..5dee251f2a2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -67,7 +67,7 @@ #include #include #include -#include +#include #include #include #include @@ -1917,7 +1917,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) if (read_stale_estimates && (chainparams.GetChainType() != ChainType::REGTEST)) { return InitError(strprintf(_("acceptstalefeeestimates is not supported on %s chain."), chainparams.GetChainTypeString())); } - node.fee_estimator_man = std::make_unique(FeeestPath(args), read_stale_estimates, *Assert(node.mempool), chainman); + MaybeMigrateLegacyFeeEstimates(args); + node.fee_estimator_man = std::make_unique(BlockPolicyFeeEstPath(args), read_stale_estimates, *Assert(node.mempool), chainman); // Flush estimates to disk periodically FeeRateEstimatorManager* fee_estimator_man = node.fee_estimator_man.get(); diff --git a/src/policy/fees/block_policy_estimator.cpp b/src/policy/fees/block_policy_estimator.cpp index 1e29993db94..3148ac540ab 100644 --- a/src/policy/fees/block_policy_estimator.cpp +++ b/src/policy/fees/block_policy_estimator.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include // The current format written, and the version required to read. Must be @@ -981,6 +982,15 @@ void CBlockPolicyEstimator::Flush() { void CBlockPolicyEstimator::FlushFeeEstimates() { + if (!m_estimation_filepath.parent_path().empty()) { + std::error_code error; + fs::create_directories(m_estimation_filepath.parent_path(), error); + if (error) { + LogWarning("Failed to create fee estimates directory %s: %s. Continue anyway.", fs::PathToString(m_estimation_filepath.parent_path()), error.message()); + return; + } + } + AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "wb")}; if (est_file.IsNull() || !Write(est_file)) { LogWarning("Failed to write fee estimates to %s. Continue anyway.", fs::PathToString(m_estimation_filepath)); diff --git a/src/policy/fees/block_policy_estimator.h b/src/policy/fees/block_policy_estimator.h index 0fabb3db8ab..6253d3954ca 100644 --- a/src/policy/fees/block_policy_estimator.h +++ b/src/policy/fees/block_policy_estimator.h @@ -24,7 +24,7 @@ #include -/** fee_estimates.dat that are more than 60 hours (2.5 days) old will not be read, +/** Block policy estimate files that are more than 60 hours (2.5 days) old will not be read, * as fee estimates are based on historical data and may be inaccurate if * network activity has changed. */ diff --git a/src/policy/fees/block_policy_estimator_args.cpp b/src/policy/fees/block_policy_estimator_args.cpp deleted file mode 100644 index 6622a658105..00000000000 --- a/src/policy/fees/block_policy_estimator_args.cpp +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) 2021-present The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include - -#include - -namespace { -const char* FEE_ESTIMATES_FILENAME = "fee_estimates.dat"; -} // namespace - -fs::path FeeestPath(const ArgsManager& argsman) -{ - return argsman.GetDataDirNet() / FEE_ESTIMATES_FILENAME; -} diff --git a/src/policy/fees/block_policy_estimator_args.h b/src/policy/fees/block_policy_estimator_args.h deleted file mode 100644 index a8f5d56a6bf..00000000000 --- a/src/policy/fees/block_policy_estimator_args.h +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) 2022-present The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_POLICY_FEES_BLOCK_POLICY_ESTIMATOR_ARGS_H -#define BITCOIN_POLICY_FEES_BLOCK_POLICY_ESTIMATOR_ARGS_H - -#include - -class ArgsManager; - -/** @return The fee estimates data file path. */ -fs::path FeeestPath(const ArgsManager& argsman); - -#endif // BITCOIN_POLICY_FEES_BLOCK_POLICY_ESTIMATOR_ARGS_H diff --git a/src/policy/fees/estimator_args.cpp b/src/policy/fees/estimator_args.cpp new file mode 100644 index 00000000000..09366276669 --- /dev/null +++ b/src/policy/fees/estimator_args.cpp @@ -0,0 +1,54 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +#include + +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"}; + +fs::path LegacyFeeEstPath(const ArgsManager& argsman) +{ + return argsman.GetDataDirNet() / LEGACY_FEE_ESTIMATES_FILENAME; +} +} // namespace + +void MaybeMigrateLegacyFeeEstimates(const ArgsManager& argsman) +{ + const fs::path legacy_path{LegacyFeeEstPath(argsman)}; + const fs::path block_policy_path{BlockPolicyFeeEstPath(argsman)}; + if (!fs::exists(legacy_path)) return; + std::error_code error; + if (fs::exists(block_policy_path)) { + fs::remove(legacy_path, error); + if (error) { + LogWarning("Failed to remove legacy fee estimates file %s: %s. Continuing anyway.", fs::PathToString(legacy_path), error.message()); + return; + } + LogInfo("Removed legacy fee estimates file %s.", fs::PathToString(legacy_path)); + return; + } + fs::create_directories(block_policy_path.parent_path(), error); + if (error) { + LogWarning("Failed to create block policy fee estimates directory %s: %s. Continuing without migration.", fs::PathToString(block_policy_path.parent_path()), error.message()); + return; + } + fs::rename(legacy_path, block_policy_path, error); + if (error) { + LogWarning("Failed to migrate fee estimates from %s to %s: %s. Continuing with fresh estimates.", fs::PathToString(legacy_path), fs::PathToString(block_policy_path), error.message()); + return; + } + LogInfo("Migrated fee estimates from %s to %s.", fs::PathToString(legacy_path), fs::PathToString(block_policy_path)); +} + +fs::path BlockPolicyFeeEstPath(const ArgsManager& argsman) +{ + return argsman.GetDataDirNet() / FEES_BASE_DIR / BLOCK_POLICY_ESTIMATES_FILENAME; +} diff --git a/src/policy/fees/estimator_args.h b/src/policy/fees/estimator_args.h new file mode 100644 index 00000000000..92140ebb626 --- /dev/null +++ b/src/policy/fees/estimator_args.h @@ -0,0 +1,18 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_POLICY_FEES_ESTIMATOR_ARGS_H +#define BITCOIN_POLICY_FEES_ESTIMATOR_ARGS_H + +#include + +class ArgsManager; + +/** Move a legacy fee_estimates.dat file to the current block policy fee estimator path, if needed. */ +void MaybeMigrateLegacyFeeEstimates(const ArgsManager& argsman); + +/** @return The block policy fee estimator data file path. */ +fs::path BlockPolicyFeeEstPath(const ArgsManager& argsman); + +#endif // BITCOIN_POLICY_FEES_ESTIMATOR_ARGS_H diff --git a/src/test/blockpolicyestimator_tests.cpp b/src/test/blockpolicyestimator_tests.cpp index 1c1aa0dfd6b..0c2cec6cbcc 100644 --- a/src/test/blockpolicyestimator_tests.cpp +++ b/src/test/blockpolicyestimator_tests.cpp @@ -3,23 +3,22 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include +#include #include +#include #include #include #include #include #include -#include - #include BOOST_FIXTURE_TEST_SUITE(blockpolicyestimator_tests, ChainTestingSetup) BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) { - CBlockPolicyEstimator feeEst{FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; + CBlockPolicyEstimator feeEst{BlockPolicyFeeEstPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; TestMemPoolEntryHelper entry; CAmount basefee(2000); CAmount deltaFee(100); diff --git a/src/test/fuzz/block_policy_estimator.cpp b/src/test/fuzz/block_policy_estimator.cpp index 2efc1fc12e1..2855722adc0 100644 --- a/src/test/fuzz/block_policy_estimator.cpp +++ b/src/test/fuzz/block_policy_estimator.cpp @@ -2,9 +2,10 @@ // 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 #include #include #include @@ -32,7 +33,7 @@ FUZZ_TARGET(block_policy_estimator, .init = initialize_block_policy_estimator) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); bool good_data{true}; - CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; + CBlockPolicyEstimator block_policy_estimator{BlockPolicyFeeEstPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; uint32_t current_height{0}; const auto advance_height{ diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp index a95faff0ac1..10dad417aec 100644 --- a/src/test/fuzz/policy_estimator_io.cpp +++ b/src/test/fuzz/policy_estimator_io.cpp @@ -3,7 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include +#include #include #include #include @@ -28,7 +28,7 @@ FUZZ_TARGET(policy_estimator_io, .init = initialize_policy_estimator_io) FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider}; AutoFile fuzzed_auto_file{fuzzed_file_provider.open()}; // Reusing block_policy_estimator across runs to avoid costly creation of CBlockPolicyEstimator object. - static CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; + static CBlockPolicyEstimator block_policy_estimator{BlockPolicyFeeEstPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES}; if (block_policy_estimator.Read(fuzzed_auto_file)) { block_policy_estimator.Write(fuzzed_auto_file); } diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 2eafb0427f0..40539e09f6c 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -31,6 +31,7 @@ SECONDS_PER_HOUR = 60 * 60 MIN_BUCKET_FEERATE = Decimal(100) / Decimal(COIN) TXS_COUNT = 24 BLOCK_POLICY_ESTIMATOR_ERROR = "Insufficient data or no feerate found" +BLOCK_POLICY_ESTIMATOR_FILE_PATH = "fees/block_policy_estimates.dat" def small_txpuzzle_randfee( wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment, batch_reqs @@ -333,69 +334,91 @@ class EstimateFeeTest(BitcoinTestFramework): self.restart_node(0) assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"], fee_rate) - fee_dat = self.nodes[0].chain_path / "fee_estimates.dat" + block_policy_fee_dat = self.nodes[0].chain_path / BLOCK_POLICY_ESTIMATOR_FILE_PATH + legacy_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 + # If only the legacy fee_estimates.dat file exists, it is migrated to + # the new block policy estimator path. 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)) + os.rename(block_policy_fee_dat, legacy_fee_dat) + self.start_node(0) + assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"], fee_rate) + self.stop_node(0) + assert_equal(os.path.isfile(block_policy_fee_dat), True) + assert_equal(os.path.isfile(legacy_fee_dat), False) - # Start node and ensure the fee_estimates.dat file was not read + # If both files exist, the new block policy estimator path is used and + # the obsolete legacy file is removed. + with open(legacy_fee_dat, "wb") as f: + f.write(b"ignored legacy fee estimates") + self.start_node(0) + assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"], fee_rate) + self.stop_node(0) + assert_equal(os.path.isfile(legacy_fee_dat), False) + + # Stop the node and backdate the block policy estimator file more than MAX_FILE_AGE + last_modified_time = time.time() - (MAX_FILE_AGE + 1) * SECONDS_PER_HOUR + os.utime(block_policy_fee_dat, (last_modified_time, last_modified_time)) + + # Start node and ensure the block policy estimator file was not read self.start_node(0) assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["errors"], [BLOCK_POLICY_ESTIMATOR_ERROR]) 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 + block_policy_fees_dat = self.nodes[0].chain_path / BLOCK_POLICY_ESTIMATOR_FILE_PATH + os.remove(block_policy_fees_dat) if os.path.exists(block_policy_fees_dat) else None + if os.path.isdir(block_policy_fees_dat.parent): + os.rmdir(block_policy_fees_dat.parent) - # 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. + # Verify that block policy estimator file and its parent directory do not exist + assert_equal(os.path.isfile(block_policy_fees_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 {fee_dat}."] + expected_messages = [f"Flushed fee estimates to {block_policy_fees_dat}."] with self.nodes[0].assert_debug_log(expected_msgs=expected_messages, timeout=1): - # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat + # Mock the scheduler for an hour to flush fee estimates to block_policy_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 fee estimates were flushed and block policy estimator directory and file 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 block_hash_before = self.nodes[0].getbestblockhash() - fee_dat_initial_content = open(fee_dat, "rb").read() + block_policy_dat_initial_content = open(block_policy_fees_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 fee_estimates.dat + # Mock the scheduler for an hour to flush fee estimates to the block policy estimator file 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) + 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 self.restart_node(0) - fee_dat_current_content = open(fee_dat, "rb").read() - assert_equal(fee_dat_current_content, fee_dat_initial_content) + 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 with self.nodes[0].assert_debug_log(expected_msgs=expected_messages, timeout=1): - # Mock the scheduler for an hour to flush fee estimates to fee_estimates.dat + # Mock the scheduler for an hour to flush fee estimates to block_policy_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_not_equal(fee_dat_current_content, fee_dat_initial_content) - - fee_dat_initial_content = fee_dat_current_content + 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 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_not_equal(fee_dat_current_content, fee_dat_initial_content) + + 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) def test_acceptstalefeeestimates_option(self): @@ -404,20 +427,20 @@ class EstimateFeeTest(BitcoinTestFramework): self.stop_node(0) - fee_dat = self.nodes[0].chain_path / "fee_estimates.dat" + fee_dat = self.nodes[0].chain_path / BLOCK_POLICY_ESTIMATOR_FILE_PATH - # Stop the node and backdate the fee_estimates.dat file more than MAX_FILE_AGE + # Stop the node and backdate the block policy estimator 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 + # Restart node with -acceptstalefeeestimates option to ensure block policy estimator file is read self.start_node(0,extra_args=["-acceptstalefeeestimates"]) assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["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" + fee_dat = self.nodes[0].chain_path / BLOCK_POLICY_ESTIMATOR_FILE_PATH os.remove(fee_dat) self.start_node(0) self.connect_nodes(0, 1) @@ -552,7 +575,7 @@ 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.log.info("Test fees data 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 @@ -564,7 +587,7 @@ class EstimateFeeTest(BitcoinTestFramework): self.log.info("Test acceptstalefeeestimates option") self.test_acceptstalefeeestimates_option() - self.log.info("Test reading old fee_estimates.dat") + self.log.info("Test reading old block policy estimator file") self.test_old_fee_estimate_file() self.clear_estimates() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 0617d334119..70aeb426662 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -965,6 +965,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): return os.path.join(cache_node_dir, self.chain, *paths) os.rmdir(cache_path('wallets')) # Remove empty wallets dir + shutil.rmtree(cache_path('fees'), ignore_errors=True) for entry in os.listdir(cache_path()): if entry not in ['chainstate', 'blocks', 'indexes']: # Only indexes, chainstate and blocks folders os.remove(cache_path(entry))