fees: Pass in a filepath instead of referencing gArgs

sethwynne · Jun 28, 2022 · d1684be · d1684be
1 parent 9a3d825
commit d1684be
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 16 deletions.
diff --git a/src/Makefile.am b/src/Makefile.am
@@ -206,6 +206,7 @@ BITCOIN_CORE_H = \
   outputtype.h \
   policy/feerate.h \
   policy/fees.h \
+  policy/fees_args.h \
   policy/packages.h \
   policy/policy.h \
   policy/rbf.h \
@@ -381,6 +382,7 @@ libbitcoin_node_a_SOURCES = \
   node/interface_ui.cpp \
   noui.cpp \
   policy/fees.cpp \
+  policy/fees_args.cpp \
   policy/packages.cpp \
   policy/rbf.cpp \
   policy/settings.cpp \

diff --git a/src/init.cpp b/src/init.cpp
@@ -40,10 +40,11 @@
 #include <node/caches.h>
 #include <node/chainstate.h>
 #include <node/context.h>
-#include <node/miner.h>
 #include <node/interface_ui.h>
+#include <node/miner.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
+#include <policy/fees_args.h>
 #include <policy/policy.h>
 #include <policy/settings.h>
 #include <protocol.h>
@@ -1291,7 +1292,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     assert(!node.fee_estimator);
     // Don't initialize fee estimation with old data if we don't relay transactions,
     // as they would never get updated.
-    if (!ignores_incoming_txs) node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
+    if (!ignores_incoming_txs) node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(args));
 
     // sanitize comments per BIP-0014, format user agent and check total size
     std::vector<std::string> uacomments;

diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
@@ -31,8 +31,6 @@
 #include <stdexcept>
 #include <utility>
 
-static const char* FEE_ESTIMATES_FILENAME = "fee_estimates.dat";
-
 static constexpr double INF_FEERATE = 1e99;
 
 std::string StringForFeeEstimateHorizon(FeeEstimateHorizon horizon)
@@ -529,8 +527,8 @@ bool CBlockPolicyEstimator::_removeTx(const uint256& hash, bool inBlock)
     }
 }
 
-CBlockPolicyEstimator::CBlockPolicyEstimator()
-    : nBestSeenHeight(0), firstRecordedHeight(0), historicalFirst(0), historicalBest(0), trackedTxs(0), untrackedTxs(0)
+CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath)
+    : m_estimation_filepath{estimation_filepath}, nBestSeenHeight{0}, firstRecordedHeight{0}, historicalFirst{0}, historicalBest{0}, trackedTxs{0}, untrackedTxs{0}
 {
     static_assert(MIN_BUCKET_FEERATE > 0, "Min feerate must be nonzero");
     size_t bucketIndex = 0;
@@ -548,10 +546,9 @@ CBlockPolicyEstimator::CBlockPolicyEstimator()
     longStats = std::unique_ptr<TxConfirmStats>(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE));
 
     // If the fee estimation file is present, read recorded estimations
-    fs::path est_filepath = gArgs.GetDataDirNet() / FEE_ESTIMATES_FILENAME;
-    CAutoFile est_file(fsbridge::fopen(est_filepath, "rb"), SER_DISK, CLIENT_VERSION);
+    CAutoFile est_file(fsbridge::fopen(m_estimation_filepath, "rb"), SER_DISK, CLIENT_VERSION);
     if (est_file.IsNull() || !Read(est_file)) {
-        LogPrintf("Failed to read fee estimates from %s. Continue anyway.\n", fs::PathToString(est_filepath));
+        LogPrintf("Failed to read fee estimates from %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
     }
 }
 
@@ -907,10 +904,9 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
 void CBlockPolicyEstimator::Flush() {
     FlushUnconfirmed();
 
-    fs::path est_filepath = gArgs.GetDataDirNet() / FEE_ESTIMATES_FILENAME;
-    CAutoFile est_file(fsbridge::fopen(est_filepath, "wb"), SER_DISK, CLIENT_VERSION);
+    CAutoFile est_file(fsbridge::fopen(m_estimation_filepath, "wb"), SER_DISK, CLIENT_VERSION);
     if (est_file.IsNull() || !Write(est_file)) {
-        LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(est_filepath));
+        LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath));
     }
 }
 

diff --git a/src/policy/fees.h b/src/policy/fees.h
@@ -6,6 +6,7 @@
 #define BITCOIN_POLICY_FEES_H
 
 #include <consensus/amount.h>
+#include <fs.h>
 #include <policy/feerate.h>
 #include <random.h>
 #include <sync.h>
@@ -179,9 +180,10 @@ class CBlockPolicyEstimator
      */
     static constexpr double FEE_SPACING = 1.05;
 
+    const fs::path m_estimation_filepath;
 public:
     /** Create new BlockPolicyEstimator and initialize stats tracking classes with default values */
-    CBlockPolicyEstimator();
+    CBlockPolicyEstimator(const fs::path& estimation_filepath);
     ~CBlockPolicyEstimator();
 
     /** Process all the transactions that have been included in a block */

diff --git a/src/policy/fees_args.cpp b/src/policy/fees_args.cpp
@@ -0,0 +1,12 @@
+#include <policy/fees_args.h>
+
+#include <util/system.h>
+
+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_args.h b/src/policy/fees_args.h
@@ -0,0 +1,15 @@
+// Copyright (c) 2022 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_ARGS_H
+#define BITCOIN_POLICY_FEES_ARGS_H
+
+#include <fs.h>
+
+class ArgsManager;
+
+/** @return The fee estimates data file path. */
+fs::path FeeestPath(const ArgsManager& argsman);
+
+#endif // BITCOIN_POLICY_FEES_ARGS_H
diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <policy/fees.h>
+#include <policy/fees_args.h>
 #include <primitives/transaction.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
@@ -15,15 +16,20 @@
 #include <string>
 #include <vector>
 
+namespace {
+const BasicTestingSetup* g_setup;
+} // namespace
+
 void initialize_policy_estimator()
 {
     static const auto testing_setup = MakeNoLogFileContext<>();
+    g_setup = testing_setup.get();
 }
 
 FUZZ_TARGET_INIT(policy_estimator, initialize_policy_estimator)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
-    CBlockPolicyEstimator block_policy_estimator;
+    CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args)};
     LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
         CallOneOf(
             fuzzed_data_provider,

diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <policy/fees.h>
+#include <policy/fees_args.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
@@ -11,9 +12,14 @@
 #include <cstdint>
 #include <vector>
 
+namespace {
+const BasicTestingSetup* g_setup;
+} // namespace
+
 void initialize_policy_estimator_io()
 {
     static const auto testing_setup = MakeNoLogFileContext<>();
+    g_setup = testing_setup.get();
 }
 
 FUZZ_TARGET_INIT(policy_estimator_io, initialize_policy_estimator_io)
@@ -22,7 +28,7 @@ FUZZ_TARGET_INIT(policy_estimator_io, initialize_policy_estimator_io)
     FuzzedAutoFileProvider fuzzed_auto_file_provider = ConsumeAutoFile(fuzzed_data_provider);
     CAutoFile fuzzed_auto_file = fuzzed_auto_file_provider.open();
     // Re-using block_policy_estimator across runs to avoid costly creation of CBlockPolicyEstimator object.
-    static CBlockPolicyEstimator block_policy_estimator;
+    static CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args)};
     if (block_policy_estimator.Read(fuzzed_auto_file)) {
         block_policy_estimator.Write(fuzzed_auto_file);
     }

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -23,6 +23,7 @@
 #include <node/miner.h>
 #include <noui.h>
 #include <policy/fees.h>
+#include <policy/fees_args.h>
 #include <pow.h>
 #include <rpc/blockchain.h>
 #include <rpc/register.h>
@@ -177,7 +178,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
     m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
     GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler);
 
-    m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
+    m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(*m_node.args));
     m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(m_node));
 
     m_cache_sizes = CalculateCacheSizes(m_args);