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) {