From ba6c61bbdd32d01fb1d20ebcd1e48e3b32cf0932 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Thu, 23 Apr 2026 14:33:28 +0100 Subject: [PATCH] fees: add FeeRateEstimatorManager class Introduce FeeRateEstimatorManager to wrap CBlockPolicyEstimator and act as the single point of contact for fee rate estimation in the node. It inherits CValidationInterface so it can register directly with the validation signals and receive mempool/block events. Wire it into NodeContext (fee_estimator_man), init, shutdown, the RPC server utility helpers (EnsureAnyFeeEstimatorMan), and the wallet-facing interfaces::Chain API. The Chain method estimateSmartFee is renamed to getFeeRateEstimate and now returns util::Expected instead of CFeeRate, so callers get the full estimation context without needing FeeCalculation. estimateMaxBlocks is renamed to maximumFeeEstimationTargetBlocks (still returns the max target). CBlockPolicyEstimator no longer inherits CValidationInterface; the manager now receives the mempool/block validation events and forwards them to the CBlockPolicyEstimator. Co-authored-by: willcl-ark --- src/CMakeLists.txt | 1 + src/init.cpp | 19 +++--- src/interfaces/chain.h | 9 +-- src/node/context.cpp | 2 +- src/node/context.h | 4 +- src/node/interfaces.cpp | 16 +++-- src/policy/feerate.h | 9 +++ src/policy/fees/block_policy_estimator.cpp | 15 ----- src/policy/fees/block_policy_estimator.h | 15 +---- src/policy/fees/estimator_man.cpp | 59 +++++++++++++++++ src/policy/fees/estimator_man.h | 76 ++++++++++++++++++++++ src/rpc/fees.cpp | 30 ++++----- src/rpc/server_util.cpp | 12 ++-- src/rpc/server_util.h | 6 +- src/test/util/setup_common.cpp | 2 +- src/util/fees.h | 14 +++- src/wallet/coincontrol.h | 2 +- src/wallet/fees.cpp | 15 +++-- src/wallet/rpc/spend.cpp | 2 +- src/wallet/test/fuzz/fees.cpp | 40 ++++++------ 20 files changed, 242 insertions(+), 106 deletions(-) create mode 100644 src/policy/fees/estimator_man.cpp create mode 100644 src/policy/fees/estimator_man.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 15f5dd276b4..fbe96ac32cf 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -248,6 +248,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL policy/ephemeral_policy.cpp policy/fees/block_policy_estimator.cpp policy/fees/block_policy_estimator_args.cpp + policy/fees/estimator_man.cpp policy/packages.cpp policy/rbf.cpp policy/settings.cpp diff --git a/src/init.cpp b/src/init.cpp index c25e07bf65f..cbab2c1c9e4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -68,6 +68,7 @@ #include #include #include +#include #include #include #include @@ -368,10 +369,10 @@ void Shutdown(NodeContext& node) // Drop transactions we were still watching, record fee estimations and unregister // fee estimator from validation interface. - if (node.fee_estimator) { - node.fee_estimator->Flush(); + if (node.fee_estimator_man) { + node.fee_estimator_man->ShutdownFlush(); if (node.validation_signals) { - node.validation_signals->UnregisterValidationInterface(node.fee_estimator.get()); + node.validation_signals->UnregisterValidationInterface(node.fee_estimator_man.get()); } } @@ -431,7 +432,7 @@ void Shutdown(NodeContext& node) node.validation_signals->UnregisterAllValidationInterfaces(); } node.mempool.reset(); - node.fee_estimator.reset(); + node.fee_estimator_man.reset(); node.chainman.reset(); node.validation_signals.reset(); node.scheduler.reset(); @@ -1679,7 +1680,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) rng.rand64(), *node.addrman, *node.netgroupman, chainparams, args.GetBoolArg("-networkactive", true)); - assert(!node.fee_estimator); + assert(!node.fee_estimator_man); // Don't initialize fee estimation with old data if we don't relay transactions, // as they would never get updated. if (!peerman_opts.ignore_incoming_txs) { @@ -1687,12 +1688,12 @@ 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 = std::make_unique(FeeestPath(args), read_stale_estimates); + node.fee_estimator_man = std::make_unique(FeeestPath(args), read_stale_estimates); // Flush estimates to disk periodically - CBlockPolicyEstimator* fee_estimator = node.fee_estimator.get(); - scheduler.scheduleEvery([fee_estimator] { fee_estimator->FlushFeeEstimates(); }, FEE_FLUSH_INTERVAL); - validation_signals.RegisterValidationInterface(fee_estimator); + FeeRateEstimatorManager* fee_estimator_man = node.fee_estimator_man.get(); + scheduler.scheduleEvery([fee_estimator_man] { fee_estimator_man->IntervalFlush(); }, FEE_FLUSH_INTERVAL); + validation_signals.RegisterValidationInterface(fee_estimator_man); } for (const std::string& socket_addr : args.GetArgs("-bind")) { diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 20369fd2e26..4b63d885a97 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -10,6 +10,8 @@ #include // IWYU pragma: export #include #include +#include +#include #include #include @@ -33,7 +35,6 @@ enum class MemPoolRemovalReason; enum class RBFTransactionState; struct bilingual_str; struct CBlockLocator; -struct FeeCalculation; namespace kernel { struct ChainstateRole; } // namespace kernel @@ -256,11 +257,11 @@ public: //! Check if transaction will pass the mempool's chain limits. virtual util::Result checkChainLimits(const CTransactionRef& tx) = 0; - //! Estimate smart fee. - virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; + //! Estimate a fee rate. + virtual util::Expected getFeeRateEstimate(int num_blocks, bool conservative) const = 0; //! Fee estimator max target. - virtual unsigned int estimateMaxBlocks() = 0; + virtual unsigned int maximumFeeEstimationTargetBlocks() const = 0; //! Mempool minimum fee. virtual CFeeRate mempoolMinFee() = 0; diff --git a/src/node/context.cpp b/src/node/context.cpp index 164361601f9..977d0362ce6 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/node/context.h b/src/node/context.h index b8b3274f090..80988d8028e 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -18,8 +18,8 @@ class ArgsManager; class AddrMan; class BanMan; class BaseIndex; -class CBlockPolicyEstimator; class CConnman; +class FeeRateEstimatorManager; class ValidationSignals; class CScheduler; class CTxMemPool; @@ -70,7 +70,7 @@ struct NodeContext { std::unique_ptr connman; std::unique_ptr mempool; std::unique_ptr netgroupman; - std::unique_ptr fee_estimator; + std::unique_ptr fee_estimator_man; std::unique_ptr peerman; std::unique_ptr tor_controller; std::unique_ptr chainman; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 4ac4fd5f57a..130f9602bc6 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -46,7 +46,7 @@ #include #include #include -#include +#include #include #include #include @@ -61,6 +61,8 @@ #include #include #include +#include +#include #include #include #include @@ -736,15 +738,15 @@ public: } return {}; } - CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override + util::Expected getFeeRateEstimate(int num_blocks, bool conservative) const override { - if (!m_node.fee_estimator) return {}; - return m_node.fee_estimator->estimateSmartFee(num_blocks, calc, conservative); + if (!m_node.fee_estimator_man) return EstimationError(FeeRateEstimatorType::NONE, /*returned_target=*/0, /*error=*/{}); + return m_node.fee_estimator_man->GetFeeRateEstimate(num_blocks, conservative); } - unsigned int estimateMaxBlocks() override + unsigned int maximumFeeEstimationTargetBlocks() const override { - if (!m_node.fee_estimator) return 0; - return m_node.fee_estimator->MaximumTarget(); + if (!m_node.fee_estimator_man) return 0; + return m_node.fee_estimator_man->MaximumTarget(); } CFeeRate mempoolMinFee() override { diff --git a/src/policy/feerate.h b/src/policy/feerate.h index 965edf25f22..01d453ad955 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -47,6 +47,15 @@ public: */ CFeeRate(const CAmount& nFeePaid, int32_t virtual_bytes); + /** + * Construct from a fee rate expressed as FeePerVSize. + * + * Lossless: CFeeRate is internally a FeePerVSize, so the exact + * fee/vsize fraction is preserved. A feerate whose size is less than + * or equal to 0 results in 0 fee rate per 0 size. + */ + explicit CFeeRate(const FeePerVSize& feerate) : m_feerate{feerate.size > 0 ? feerate : FeePerVSize{}} {} + /** * Return the fee in satoshis for the given vsize in vbytes. * If the calculated fee would have fractional satoshis, then the diff --git a/src/policy/fees/block_policy_estimator.cpp b/src/policy/fees/block_policy_estimator.cpp index 2d812517df5..1e29993db94 100644 --- a/src/policy/fees/block_policy_estimator.cpp +++ b/src/policy/fees/block_policy_estimator.cpp @@ -595,21 +595,6 @@ CBlockPolicyEstimator::CBlockPolicyEstimator(const fs::path& estimation_filepath CBlockPolicyEstimator::~CBlockPolicyEstimator() = default; -void CBlockPolicyEstimator::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) -{ - processTransaction(tx); -} - -void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) -{ - removeTx(tx->GetHash()); -} - -void CBlockPolicyEstimator::MempoolTransactionsRemovedForBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) -{ - processBlock(txs_removed_for_block, nBlockHeight); -} - void CBlockPolicyEstimator::processTransaction(const NewMempoolTransactionInfo& tx) { LOCK(m_cs_fee_estimator); diff --git a/src/policy/fees/block_policy_estimator.h b/src/policy/fees/block_policy_estimator.h index 2b5417cc950..0fabb3db8ab 100644 --- a/src/policy/fees/block_policy_estimator.h +++ b/src/policy/fees/block_policy_estimator.h @@ -7,13 +7,13 @@ #include #include +#include #include #include #include #include #include #include -#include #include #include @@ -24,9 +24,6 @@ #include -// How often to flush fee estimates to fee_estimates.dat. -inline constexpr std::chrono::hours FEE_FLUSH_INTERVAL{1}; - /** fee_estimates.dat 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. @@ -145,7 +142,7 @@ struct FeeCalculation * a certain number of blocks. Every time a block is added to the best chain, this class records * stats on the transactions included in that block */ -class CBlockPolicyEstimator : public CValidationInterface +class CBlockPolicyEstimator { private: /** Track confirm delays up to 12 blocks for short horizon */ @@ -272,14 +269,6 @@ public: util::Expected EstimateFeeRate(int target, bool conservative) const EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); -protected: - /** Overridden from CValidationInterface. */ - void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) override - EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); - void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) override - EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); - void MempoolTransactionsRemovedForBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) override - EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); private: mutable Mutex m_cs_fee_estimator; diff --git a/src/policy/fees/estimator_man.cpp b/src/policy/fees/estimator_man.cpp new file mode 100644 index 00000000000..8e5db6c6244 --- /dev/null +++ b/src/policy/fees/estimator_man.cpp @@ -0,0 +1,59 @@ +// 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 + +FeeRateEstimatorManager::FeeRateEstimatorManager(const fs::path& block_policy_path, bool read_stale_estimates) + : m_block_policy_estimator(std::make_unique(block_policy_path, read_stale_estimates)) +{ +} + +util::Expected FeeRateEstimatorManager::GetFeeRateEstimate(int target, bool conservative) const +{ + return m_block_policy_estimator->EstimateFeeRate(target, conservative); +} + +void FeeRateEstimatorManager::IntervalFlush() +{ + m_block_policy_estimator->FlushFeeEstimates(); +} + +void FeeRateEstimatorManager::ShutdownFlush() +{ + m_block_policy_estimator->Flush(); +} + +void FeeRateEstimatorManager::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) +{ + m_block_policy_estimator->processTransaction(tx); +} + +void FeeRateEstimatorManager::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) +{ + m_block_policy_estimator->removeTx(tx->GetHash()); +} + +void FeeRateEstimatorManager::MempoolTransactionsRemovedForBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) +{ + m_block_policy_estimator->processBlock(txs_removed_for_block, nBlockHeight); +} + +CFeeRate FeeRateEstimatorManager::BlockPolicyEstimateRawFee(unsigned int target, double threshold, FeeEstimateHorizon horizon, EstimationResult* buckets) const +{ + return m_block_policy_estimator->estimateRawFee(target, threshold, horizon, buckets); +} + +unsigned int FeeRateEstimatorManager::BlockPolicyHighestTargetTracked(FeeEstimateHorizon horizon) const +{ + return m_block_policy_estimator->HighestTargetTracked(horizon); +} + +unsigned int FeeRateEstimatorManager::MaximumTarget() const +{ + return m_block_policy_estimator->MaximumTarget(); +} diff --git a/src/policy/fees/estimator_man.h b/src/policy/fees/estimator_man.h new file mode 100644 index 00000000000..4c2086612f2 --- /dev/null +++ b/src/policy/fees/estimator_man.h @@ -0,0 +1,76 @@ +// 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_MAN_H +#define BITCOIN_POLICY_FEES_ESTIMATOR_MAN_H + +#include +#include +#include +#include +#include +#include + +#include +#include + +class CFeeRate; + +// How often to flush data to disk +inline constexpr std::chrono::hours FEE_FLUSH_INTERVAL{1}; + +/** \class FeeRateEstimatorManager + * Manages fee rate estimators. + */ +class FeeRateEstimatorManager : public CValidationInterface +{ +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. + */ + FeeRateEstimatorManager(const fs::path& block_policy_path, bool read_stale_estimates); + + virtual ~FeeRateEstimatorManager() = default; + + /** + * @brief Get a fee rate estimate from block policy estimator. + * @param[in] target The target within which the transaction should be confirmed. + * @param[in] conservative Whether to select a more conservative, potentially higher, fee rate estimate. + * @return fee rate estimation, or an error on failure. + */ + virtual util::Expected GetFeeRateEstimate(int target, bool conservative) const; + + /** Flush recorded data to disk. */ + void IntervalFlush(); + + /** Flush recorded data to disk as part of shutdown sequence. */ + void ShutdownFlush(); + + /** + * @brief Returns the maximum supported confirmation target from all fee rate estimators. + */ + virtual unsigned int MaximumTarget() const; + + /** + * @brief Delegate to the block policy estimator's estimateRawFee (used by the estimaterawfee RPC). + */ + CFeeRate BlockPolicyEstimateRawFee(unsigned int target, double threshold, FeeEstimateHorizon horizon, EstimationResult* buckets) const; + + /** + * @brief Returns the maximum supported confirmation target of block policy estimator. + */ + unsigned int BlockPolicyHighestTargetTracked(FeeEstimateHorizon horizon) const; + +protected: + /** Overridden from CValidationInterface. */ + void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t /*unused*/) override; + void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason /*unused*/, uint64_t /*unused*/) override; + void MempoolTransactionsRemovedForBlock(const std::vector& txs_removed_for_block, unsigned int nBlockHeight) override; + +private: + std::unique_ptr m_block_policy_estimator; +}; + +#endif // BITCOIN_POLICY_FEES_ESTIMATOR_MAN_H diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp index f4c135444bb..8ea7cb4cb85 100644 --- a/src/rpc/fees.cpp +++ b/src/rpc/fees.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -62,12 +63,12 @@ static RPCMethod estimatesmartfee() }, [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue { - CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context); + FeeRateEstimatorManager& fee_estimator_man = EnsureAnyFeeEstimatorMan(request.context); const NodeContext& node = EnsureAnyNodeContext(request.context); const CTxMemPool& mempool = EnsureMemPool(node); CHECK_NONFATAL(mempool.m_opts.signals)->SyncWithValidationInterfaceQueue(); - unsigned int max_target = fee_estimator.MaximumTarget(); + unsigned int max_target = fee_estimator_man.MaximumTarget(); unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target); FeeEstimateMode fee_mode; if (!FeeModeFromString(self.Arg("estimate_mode"), fee_mode)) { @@ -76,19 +77,18 @@ static RPCMethod estimatesmartfee() UniValue result(UniValue::VOBJ); UniValue errors(UniValue::VARR); - FeeCalculation feeCalc; bool conservative{fee_mode == FeeEstimateMode::CONSERVATIVE}; - CFeeRate feeRate{fee_estimator.estimateSmartFee(conf_target, &feeCalc, conservative)}; - if (feeRate != CFeeRate(0)) { - CFeeRate min_mempool_feerate{mempool.GetMinFee()}; - CFeeRate min_relay_feerate{mempool.m_opts.min_relay_feerate}; - feeRate = std::max({feeRate, min_mempool_feerate, min_relay_feerate}); - result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK())); + const auto estimate{fee_estimator_man.GetFeeRateEstimate(conf_target, conservative)}; + if (estimate) { + const CFeeRate min_mempool_feerate{mempool.GetMinFee()}; + const CFeeRate min_relay_feerate{mempool.m_opts.min_relay_feerate}; + const auto fee_rate{std::max({CFeeRate(estimate->feerate), min_mempool_feerate, min_relay_feerate})}; + result.pushKV("feerate", ValueFromAmount(fee_rate.GetFeePerK())); } else { - errors.push_back("Insufficient data or no feerate found"); + errors.push_back(estimate.error().reason); result.pushKV("errors", std::move(errors)); } - result.pushKV("blocks", feeCalc.returnedTarget); + result.pushKV("blocks", FeeRateEstimationRef(estimate).returned_target); return result; }, }; @@ -155,11 +155,11 @@ static RPCMethod estimaterawfee() }, [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue { - CBlockPolicyEstimator& fee_estimator = EnsureAnyFeeEstimator(request.context); + FeeRateEstimatorManager& fee_estimator_man = EnsureAnyFeeEstimatorMan(request.context); const NodeContext& node = EnsureAnyNodeContext(request.context); CHECK_NONFATAL(node.validation_signals)->SyncWithValidationInterfaceQueue(); - unsigned int max_target = fee_estimator.MaximumTarget(); + unsigned int max_target = fee_estimator_man.MaximumTarget(); unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target); double threshold = 0.95; if (!request.params[1].isNull()) { @@ -176,9 +176,9 @@ static RPCMethod estimaterawfee() EstimationResult buckets; // Only output results for horizons which track the target - if (conf_target > fee_estimator.HighestTargetTracked(horizon)) continue; + if (conf_target > fee_estimator_man.BlockPolicyHighestTargetTracked(horizon)) continue; - feeRate = fee_estimator.estimateRawFee(conf_target, threshold, horizon, &buckets); + feeRate = fee_estimator_man.BlockPolicyEstimateRawFee(conf_target, threshold, horizon, &buckets); UniValue horizon_result(UniValue::VOBJ); UniValue errors(UniValue::VARR); UniValue passbucket(UniValue::VOBJ); diff --git a/src/rpc/server_util.cpp b/src/rpc/server_util.cpp index 69fc2429897..ac1a4c9198d 100644 --- a/src/rpc/server_util.cpp +++ b/src/rpc/server_util.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include @@ -84,17 +84,17 @@ ChainstateManager& EnsureAnyChainman(const std::any& context) return EnsureChainman(EnsureAnyNodeContext(context)); } -CBlockPolicyEstimator& EnsureFeeEstimator(const NodeContext& node) +FeeRateEstimatorManager& EnsureFeeEstimatorMan(const NodeContext& node) { - if (!node.fee_estimator) { + if (!node.fee_estimator_man) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Fee estimation disabled"); } - return *node.fee_estimator; + return *node.fee_estimator_man; } -CBlockPolicyEstimator& EnsureAnyFeeEstimator(const std::any& context) +FeeRateEstimatorManager& EnsureAnyFeeEstimatorMan(const std::any& context) { - return EnsureFeeEstimator(EnsureAnyNodeContext(context)); + return EnsureFeeEstimatorMan(EnsureAnyNodeContext(context)); } CConnman& EnsureConnman(const NodeContext& node) diff --git a/src/rpc/server_util.h b/src/rpc/server_util.h index 14bb1c8fb80..658e8d140f3 100644 --- a/src/rpc/server_util.h +++ b/src/rpc/server_util.h @@ -12,7 +12,7 @@ class AddrMan; class ArgsManager; class CBlockIndex; -class CBlockPolicyEstimator; +class FeeRateEstimatorManager; class CConnman; class CTxMemPool; class ChainstateManager; @@ -34,8 +34,8 @@ ArgsManager& EnsureArgsman(const node::NodeContext& node); ArgsManager& EnsureAnyArgsman(const std::any& context); ChainstateManager& EnsureChainman(const node::NodeContext& node); ChainstateManager& EnsureAnyChainman(const std::any& context); -CBlockPolicyEstimator& EnsureFeeEstimator(const node::NodeContext& node); -CBlockPolicyEstimator& EnsureAnyFeeEstimator(const std::any& context); +FeeRateEstimatorManager& EnsureFeeEstimatorMan(const node::NodeContext& node); +FeeRateEstimatorManager& EnsureAnyFeeEstimatorMan(const std::any& context); CConnman& EnsureConnman(const node::NodeContext& node); interfaces::Mining& EnsureMining(const node::NodeContext& node); PeerManager& EnsurePeerman(const node::NodeContext& node); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 6c4d80fe388..71e8a2e11e9 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -342,7 +342,7 @@ ChainTestingSetup::~ChainTestingSetup() m_node.netgroupman.reset(); m_node.args = nullptr; m_node.mempool.reset(); - Assert(!m_node.fee_estimator); // Each test must create a local object, if they wish to use the fee_estimator + Assert(!m_node.fee_estimator_man); // Each test must create a local object, if they wish to use the fee_estimator_man m_node.chainman.reset(); m_node.validation_signals.reset(); m_node.scheduler.reset(); diff --git a/src/util/fees.h b/src/util/fees.h index 332b3927781..cc6fa7657e8 100644 --- a/src/util/fees.h +++ b/src/util/fees.h @@ -33,6 +33,7 @@ enum class FeeReason { * Identifier for fee rate estimator. */ enum class FeeRateEstimatorType { + NONE, BLOCK_POLICY, }; @@ -43,9 +44,9 @@ enum class FeeRateEstimatorType { struct FeeRateEstimation { //! This identifies which fee rate estimator is providing this feerate estimate FeeRateEstimatorType feerate_estimator; - //! Fee rate sufficient to confirm a package within target + //! Fee rate sufficient for confirmation within target. FeePerVSize feerate; - //! The returned target at which the package is likely to confirm within + //! The returned confirmation target for the estimate. int returned_target; }; @@ -68,4 +69,13 @@ inline util::Unexpected EstimationError(FeeRateEstimator return util::Unexpected{FeeRateEstimationError{{estimator, FeePerVSize{0, 0}, returned_target}, std::move(error)}}; } +/** + * Return the estimation carried by a fee rate estimate result: the + * successful estimation, or the error's zero-value estimation. + */ +inline const FeeRateEstimation& FeeRateEstimationRef(const util::Expected& result LIFETIMEBOUND) +{ + return result ? *result : result.error().estimation; +} + #endif // BITCOIN_UTIL_FEES_H diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 53fc0e8fcf0..0ed51f161f4 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -103,7 +103,7 @@ public: bool m_avoid_partial_spends = DEFAULT_AVOIDPARTIALSPENDS; //! Forbids inclusion of dirty (previously used) addresses bool m_avoid_address_reuse = false; - //! Fee estimation mode to control arguments to estimateSmartFee + //! Fee estimation mode. FeeEstimateMode m_fee_mode = FeeEstimateMode::UNSET; //! Minimum chain depth value for coin availability int m_min_depth = DEFAULT_MIN_DEPTH; diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index f5c4e62e5bd..87e34e56e33 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -59,13 +58,14 @@ MinimumFeeRateResult GetMinimumFeeRate(const CWallet& wallet, const CCoinControl else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false; - FeeCalculation feeCalc; - CFeeRate fee_rate{wallet.chain().estimateSmartFee(target, conservative_estimate, &feeCalc)}; + const auto fee_estimation_res = wallet.chain().getFeeRateEstimate(target, conservative_estimate); + const FeeRateEstimation& estimation{FeeRateEstimationRef(fee_estimation_res)}; + CFeeRate fee_rate{estimation.feerate}; FeeReason fee_reason{FeeReason::FEE_RATE_ESTIMATOR}; // Only fee rate estimator results have a returned target. - std::optional returned_target{feeCalc.returnedTarget}; + std::optional returned_target{estimation.returned_target}; if (fee_rate == CFeeRate(0)) { - // if we don't have enough data for estimateSmartFee, then use fallback fee + // if we don't have enough data for getFeeRateEstimate, then use fallback fee fee_rate = wallet.m_fallback_fee; fee_reason = FeeReason::FALLBACK; returned_target = std::nullopt; @@ -89,8 +89,9 @@ MinimumFeeRateResult GetMinimumFeeRate(const CWallet& wallet, const CCoinControl CFeeRate GetDiscardRate(const CWallet& wallet) { - unsigned int highest_target = wallet.chain().estimateMaxBlocks(); - CFeeRate discard_rate = wallet.chain().estimateSmartFee(highest_target, /*conservative=*/false); + unsigned int highest_target = wallet.chain().maximumFeeEstimationTargetBlocks(); + const auto res = wallet.chain().getFeeRateEstimate(highest_target, /*conservative=*/false); + auto discard_rate = res ? CFeeRate(res->feerate) : CFeeRate(0); // Don't let discard_rate be greater than longest possible fee estimate if we get a valid fee estimate discard_rate = (discard_rate == CFeeRate(0)) ? wallet.m_discard_rate : std::min(discard_rate, wallet.m_discard_rate); // Discard rate must be at least dust relay feerate diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index bfe5851ad7c..1cc5e0445cb 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -235,7 +235,7 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage()); } if (!conf_target.isNull()) { - cc.m_confirm_target = ParseConfirmTarget(conf_target, wallet.chain().estimateMaxBlocks()); + cc.m_confirm_target = ParseConfirmTarget(conf_target, wallet.chain().maximumFeeEstimationTargetBlocks()); } } diff --git a/src/wallet/test/fuzz/fees.cpp b/src/wallet/test/fuzz/fees.cpp index 4886b2ab17c..9fe3cf2dffb 100644 --- a/src/wallet/test/fuzz/fees.cpp +++ b/src/wallet/test/fuzz/fees.cpp @@ -2,13 +2,14 @@ // 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 +#include #include #include #include @@ -21,45 +22,46 @@ namespace wallet { namespace { -struct FeeEstimatorTestingSetup : public TestingSetup { - FeeEstimatorTestingSetup(const ChainType chain_type, TestOpts opts) : TestingSetup{chain_type, opts} +struct FeeEstimatorManTestingSetup : public TestingSetup { + FeeEstimatorManTestingSetup(const ChainType chain_type, TestOpts opts) : TestingSetup{chain_type, opts} { } - ~FeeEstimatorTestingSetup() { - m_node.fee_estimator.reset(); + ~FeeEstimatorManTestingSetup() + { + m_node.fee_estimator_man.reset(); } - void SetFeeEstimator(std::unique_ptr fee_estimator) + void SetFeeEstimatorMan(std::unique_ptr fee_estimator_man) { - m_node.fee_estimator = std::move(fee_estimator); + m_node.fee_estimator_man = std::move(fee_estimator_man); } }; -FeeEstimatorTestingSetup* g_setup; +FeeEstimatorManTestingSetup* g_setup; -class FuzzedBlockPolicyEstimator : public CBlockPolicyEstimator +class FuzzedFeeEstimatorMan : public FeeRateEstimatorManager { FuzzedDataProvider& fuzzed_data_provider; public: - FuzzedBlockPolicyEstimator(FuzzedDataProvider& provider) - : CBlockPolicyEstimator(fs::path{}, false), fuzzed_data_provider(provider) {} + FuzzedFeeEstimatorMan(FuzzedDataProvider& provider) + : FeeRateEstimatorManager(fs::path{}, false), fuzzed_data_provider(provider) {} - CFeeRate estimateSmartFee(int confTarget, FeeCalculation* feeCalc, bool conservative) const override + util::Expected GetFeeRateEstimate(int confTarget, bool conservative) const override { - return CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/1'000'000)}; + FeePerVSize feerate(ConsumeMoney(fuzzed_data_provider, /*max=*/1'000'000), fuzzed_data_provider.ConsumeIntegralInRange(1000, 1000000)); + return FeeRateEstimation{FeeRateEstimatorType::BLOCK_POLICY, feerate, fuzzed_data_provider.ConsumeIntegralInRange(2, 1004)}; } - - unsigned int HighestTargetTracked(FeeEstimateHorizon horizon) const override + unsigned int MaximumTarget() const override { - return fuzzed_data_provider.ConsumeIntegralInRange(1, 1000); + return fuzzed_data_provider.ConsumeIntegralInRange(1, 1004); } }; void initialize_setup() { - static const auto testing_setup = MakeNoLogFileContext(); + static const auto testing_setup = MakeNoLogFileContext(); g_setup = testing_setup.get(); } @@ -78,8 +80,8 @@ FUZZ_TARGET(wallet_fees, .init = initialize_setup) .dust_relay_feerate = CFeeRate{ConsumeMoney(fuzzed_data_provider, 1'000'000)} }; node.mempool = std::make_unique(mempool_opts, error); - std::unique_ptr fee_estimator = std::make_unique(fuzzed_data_provider); - g_setup->SetFeeEstimator(std::move(fee_estimator)); + std::unique_ptr fee_estimator_man = std::make_unique(fuzzed_data_provider); + g_setup->SetFeeEstimatorMan(std::move(fee_estimator_man)); auto target_feerate{CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/1'000'000)}}; if (target_feerate > node.mempool->m_opts.incremental_relay_feerate && target_feerate > node.mempool->m_opts.min_relay_feerate) {