From 9cacf677a918ba89b6a47f4ae2fde8d62f1cf384 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Thu, 23 Apr 2026 14:50:56 +0100 Subject: [PATCH] rpc: add fee_rate_estimator option to estimatesmartfee Add a string fee_rate_estimator option (default "none") to estimatesmartfee options. "block_policy" consults only the block policy fee rate estimator, "none" uses the fee rate estimator manager selected behaviour, and unknown values are treated as "none". Unknown option keys are rejected. Still only the block policy fee rate estimator, so the result is unchanged; a subsequent commit will change the default behaviour. Also adds GetFeeRateEstimate(FeeRateEstimatorType, target, conservative) to FeeRateEstimatorManager so callers can query a single estimator by type; NONE returns the manager-selected combined estimate. --- src/policy/fees/estimator_man.cpp | 11 +++++++++ src/policy/fees/estimator_man.h | 9 ++++++++ src/rpc/client.cpp | 1 + src/rpc/fees.cpp | 20 ++++++++++++++-- src/test/CMakeLists.txt | 1 + src/test/fees_util_tests.cpp | 19 +++++++++++++++ src/util/CMakeLists.txt | 1 + src/util/fees.cpp | 14 ++++++++++++ src/util/fees.h | 3 +++ src/wallet/test/fuzz/fees.cpp | 6 +++++ test/functional/feature_fee_estimation.py | 28 +++++++++++------------ test/functional/rpc_estimatefee.py | 8 ++++++- 12 files changed, 104 insertions(+), 17 deletions(-) create mode 100644 src/test/fees_util_tests.cpp create mode 100644 src/util/fees.cpp diff --git a/src/policy/fees/estimator_man.cpp b/src/policy/fees/estimator_man.cpp index 8e5db6c6244..18d44a2e4c8 100644 --- a/src/policy/fees/estimator_man.cpp +++ b/src/policy/fees/estimator_man.cpp @@ -18,6 +18,17 @@ util::Expected FeeRateEstimatorManage return m_block_policy_estimator->EstimateFeeRate(target, conservative); } +util::Expected FeeRateEstimatorManager::GetFeeRateEstimate(FeeRateEstimatorType type, int target, bool conservative) const +{ + switch (type) { + case FeeRateEstimatorType::NONE: + return GetFeeRateEstimate(target, conservative); + case FeeRateEstimatorType::BLOCK_POLICY: + return m_block_policy_estimator->EstimateFeeRate(target, conservative); + } // no default case, so the compiler can warn about missing cases + assert(false); +} + void FeeRateEstimatorManager::IntervalFlush() { m_block_policy_estimator->FlushFeeEstimates(); diff --git a/src/policy/fees/estimator_man.h b/src/policy/fees/estimator_man.h index 4c2086612f2..a7fca0e0353 100644 --- a/src/policy/fees/estimator_man.h +++ b/src/policy/fees/estimator_man.h @@ -42,6 +42,15 @@ public: */ virtual util::Expected GetFeeRateEstimate(int target, bool conservative) const; + /** + * Like GetFeeRateEstimate, but only consults the specified estimator type. + * @param[in] type The estimator to query. NONE returns the manager-selected combined estimate. + * @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 from the specified estimator, or an error on failure. + */ + virtual util::Expected GetFeeRateEstimate(FeeRateEstimatorType type, int target, bool conservative) const; + /** Flush recorded data to disk. */ void IntervalFlush(); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 6add22db277..21ed8795304 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -305,6 +305,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getrawmempool", 1, "mempool_sequence" }, { "getorphantxs", 0, "verbosity" }, { "estimatesmartfee", 0, "conf_target" }, + { "estimatesmartfee", 2, "options" }, { "estimaterawfee", 0, "conf_target" }, { "estimaterawfee", 1, "threshold" }, { "prioritisetransaction", 1, "dummy" }, diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp index 8ea7cb4cb85..f8475b7ab32 100644 --- a/src/rpc/fees.cpp +++ b/src/rpc/fees.cpp @@ -42,6 +42,15 @@ static RPCMethod estimatesmartfee() {"conf_target", RPCArg::Type::NUM, RPCArg::Optional::NO, "Confirmation target in blocks (1 - 1008)"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"economical"}, "The fee estimate mode.\n" + FeeModesDetail(std::string("default mode will be used"))}, + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + { + {"fee_rate_estimator", RPCArg::Type::STR, RPCArg::Default{"none"}, + "Selects which fee rate estimator to use.\n" + "\"none\" lets the fee rate estimator manager choose.\n" + "\"block_policy\" uses only the block policy fee rate estimator.\n" + "Unknown values are treated as \"none\"."}, + }, + }, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -74,11 +83,18 @@ static RPCMethod estimatesmartfee() if (!FeeModeFromString(self.Arg("estimate_mode"), fee_mode)) { throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage()); } + const UniValue options{request.params[2].isNull() ? UniValue::VOBJ : request.params[2]}; + RPCTypeCheckObj(options, + { + {"fee_rate_estimator", UniValueType(UniValue::VSTR)}, + }, /*fAllowNull=*/true, /*fStrict=*/true); + const auto fee_rate_estimator{FeeRateEstimatorTypeFromString( + options["fee_rate_estimator"].isNull() ? "none" : options["fee_rate_estimator"].get_str())}; + bool conservative{fee_mode == FeeEstimateMode::CONSERVATIVE}; UniValue result(UniValue::VOBJ); UniValue errors(UniValue::VARR); - bool conservative{fee_mode == FeeEstimateMode::CONSERVATIVE}; - const auto estimate{fee_estimator_man.GetFeeRateEstimate(conf_target, conservative)}; + const auto estimate{fee_estimator_man.GetFeeRateEstimate(fee_rate_estimator, conf_target, conservative)}; if (estimate) { const CFeeRate min_mempool_feerate{mempool.GetMinFee()}; const CFeeRate min_relay_feerate{mempool.m_opts.min_relay_feerate}; diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 2ccc44297d2..47e37cc9dfa 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -45,6 +45,7 @@ add_executable(test_bitcoin denialofservice_tests.cpp descriptor_tests.cpp disconnected_transactions.cpp + fees_util_tests.cpp feefrac_tests.cpp feerounder_tests.cpp flatfile_tests.cpp diff --git a/src/test/fees_util_tests.cpp b/src/test/fees_util_tests.cpp new file mode 100644 index 00000000000..366b845ee39 --- /dev/null +++ b/src/test/fees_util_tests.cpp @@ -0,0 +1,19 @@ +// 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 + +BOOST_AUTO_TEST_SUITE(fees_util_tests) + +BOOST_AUTO_TEST_CASE(fee_rate_estimator_type_from_string) +{ + BOOST_CHECK(FeeRateEstimatorTypeFromString("none") == FeeRateEstimatorType::NONE); + BOOST_CHECK(FeeRateEstimatorTypeFromString("block_policy") == FeeRateEstimatorType::BLOCK_POLICY); + BOOST_CHECK(FeeRateEstimatorTypeFromString("mempool_policy") == FeeRateEstimatorType::NONE); + BOOST_CHECK(FeeRateEstimatorTypeFromString("unknown") == FeeRateEstimatorType::NONE); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 3444fb09361..522818f60fa 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -11,6 +11,7 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL check.cpp exec.cpp exception.cpp + fees.cpp feefrac.cpp fs.cpp fs_helpers.cpp diff --git a/src/util/fees.cpp b/src/util/fees.cpp new file mode 100644 index 00000000000..82998869c84 --- /dev/null +++ b/src/util/fees.cpp @@ -0,0 +1,14 @@ +// 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 + +FeeRateEstimatorType FeeRateEstimatorTypeFromString(std::string_view feerate_estimator_type) +{ + const auto normalized{ToLower(feerate_estimator_type)}; + if (normalized == "block_policy") return FeeRateEstimatorType::BLOCK_POLICY; + return FeeRateEstimatorType::NONE; +} diff --git a/src/util/fees.h b/src/util/fees.h index cc6fa7657e8..bbb41a9b260 100644 --- a/src/util/fees.h +++ b/src/util/fees.h @@ -10,6 +10,7 @@ #include #include +#include #include /* Used to determine type of fee estimation requested */ @@ -78,4 +79,6 @@ inline const FeeRateEstimation& FeeRateEstimationRef(const util::Expected(1000, 1000000)); return FeeRateEstimation{FeeRateEstimatorType::BLOCK_POLICY, feerate, fuzzed_data_provider.ConsumeIntegralInRange(2, 1004)}; } + util::Expected GetFeeRateEstimate(FeeRateEstimatorType type, int confTarget, bool conservative) const override + { + auto res = GetFeeRateEstimate(confTarget, conservative); + if (res) res->feerate_estimator = type; + return res; + } unsigned int MaximumTarget() const override { return fuzzed_data_provider.ConsumeIntegralInRange(1, 1004); diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 73ebbddcb46..32aaa92c351 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -92,7 +92,7 @@ def check_smart_estimates(node, fees_seen): """Call estimatesmartfee and verify that the estimates meet certain invariants.""" delta = 1.0e-6 # account for rounding error - all_smart_estimates = [node.estimatesmartfee(i) for i in range(1, 26)] + all_smart_estimates = [node.estimatesmartfee(i, "economical", {"fee_rate_estimator": "block_policy"}) for i in range(1, 26)] mempoolMinFee = node.getmempoolinfo()["mempoolminfee"] minRelaytxFee = node.getmempoolinfo()["minrelaytxfee"] feerate_ceiling = max(max(fees_seen), float(mempoolMinFee), float(minRelaytxFee)) @@ -132,9 +132,10 @@ def make_tx(wallet, utxo, feerate): ) def check_fee_estimates_btw_modes(node, expected_conservative, expected_economical): - fee_est_conservative = node.estimatesmartfee(1, estimate_mode="conservative")['feerate'] - fee_est_economical = node.estimatesmartfee(1, estimate_mode="economical")['feerate'] - fee_est_default = node.estimatesmartfee(1)['feerate'] + fee_est_conservative = node.estimatesmartfee(1, "conservative", {"fee_rate_estimator": "block_policy"})['feerate'] + fee_est_economical = node.estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})['feerate'] + # Omit estimate_mode to check that the default mode is economical. + fee_est_default = node.estimatesmartfee(1, options={"fee_rate_estimator": "block_policy"})['feerate'] assert_equal(fee_est_conservative, expected_conservative) assert_equal(fee_est_economical, expected_economical) assert_equal(fee_est_default, expected_economical) @@ -244,7 +245,7 @@ class EstimateFeeTest(BitcoinTestFramework): check_estimates(self.nodes[1], self.fees_per_kb) def test_estimates_with_highminrelaytxfee(self): - high_val = 3 * self.nodes[1].estimatesmartfee(2)["feerate"] + high_val = 3 * self.nodes[1].estimatesmartfee(2, "economical", {"fee_rate_estimator": "block_policy"})["feerate"] self.restart_node(1, extra_args=[f"-minrelaytxfee={high_val}"]) check_smart_estimates(self.nodes[1], self.fees_per_kb) self.restart_node(1) @@ -309,16 +310,16 @@ class EstimateFeeTest(BitcoinTestFramework): # Only 10% of the transactions were really confirmed with a low feerate, # the rest needed to be RBF'd. We must return the 90% conf rate feerate. high_feerate_kvb = Decimal(high_feerate) / COIN * 10 ** 3 - est_feerate = node.estimatesmartfee(2)["feerate"] + est_feerate = node.estimatesmartfee(2, "economical", {"fee_rate_estimator": "block_policy"})["feerate"] assert_equal(est_feerate, high_feerate_kvb) def test_old_fee_estimate_file(self): # Get the initial fee rate while node is running - fee_rate = self.nodes[0].estimatesmartfee(1)["feerate"] + fee_rate = self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"] # Restart node to ensure fee_estimate.dat file is read self.restart_node(0) - assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate) + 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" @@ -329,7 +330,7 @@ class EstimateFeeTest(BitcoinTestFramework): # Start node and ensure the fee_estimates.dat file was not read self.start_node(0) - assert_equal(self.nodes[0].estimatesmartfee(1)["errors"], ["Insufficient data or no feerate found"]) + assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["errors"], ["Insufficient data or no feerate found"]) def test_estimate_dat_is_flushed_periodically(self): @@ -387,7 +388,7 @@ class EstimateFeeTest(BitcoinTestFramework): def test_acceptstalefeeestimates_option(self): # Get the initial fee rate while node is running - fee_rate = self.nodes[0].estimatesmartfee(1)["feerate"] + fee_rate = self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"] self.stop_node(0) @@ -399,7 +400,7 @@ class EstimateFeeTest(BitcoinTestFramework): # Restart node with -acceptstalefeeestimates option to ensure fee_estimate.dat file is read self.start_node(0,extra_args=["-acceptstalefeeestimates"]) - assert_equal(self.nodes[0].estimatesmartfee(1)["feerate"], fee_rate) + 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") @@ -410,7 +411,7 @@ class EstimateFeeTest(BitcoinTestFramework): self.connect_nodes(0, 1) self.connect_nodes(0, 2) self.sync_blocks() - assert_equal(self.nodes[0].estimatesmartfee(1)["errors"], ["Insufficient data or no feerate found"]) + assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["errors"], ["Insufficient data or no feerate found"]) def broadcast_many(self, broadcaster, feerate, count, miner=None): """Broadcast and maybe mine some number of transactions with a specified fee rate.""" @@ -451,8 +452,7 @@ class EstimateFeeTest(BitcoinTestFramework): for i in range(6): self.broadcast_many(self.nodes[1], feerate_0_5_s_per_vb, TXS_COUNT) self.broadcast_many(self.nodes[1], feerate_1_s_per_vb, TXS_COUNT, self.nodes[2]) - assert_equal(feerate_0_5_s_per_vb, self.nodes[0].estimatesmartfee(1)["feerate"]) - + assert_equal(feerate_0_5_s_per_vb, self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"]) def run_test(self): self.log.info("This test is time consuming, please be patient") diff --git a/test/functional/rpc_estimatefee.py b/test/functional/rpc_estimatefee.py index 6c44b1af83a..3cb07f2e514 100755 --- a/test/functional/rpc_estimatefee.py +++ b/test/functional/rpc_estimatefee.py @@ -28,12 +28,16 @@ class EstimateFeeTest(BitcoinTestFramework): assert_raises_rpc_error(-3, "JSON value of type string is not of expected type number", self.nodes[0].estimaterawfee, 'foo') # wrong type for estimatesmartfee(estimate_mode) assert_raises_rpc_error(-3, "JSON value of type number is not of expected type string", self.nodes[0].estimatesmartfee, 1, 1) + # wrong type for estimatesmartfee(options.fee_rate_estimator) + assert_raises_rpc_error(-3, "JSON value of type number for field fee_rate_estimator is not of expected type string", self.nodes[0].estimatesmartfee, 1, 'ECONOMICAL', {'fee_rate_estimator': 1}) # wrong type for estimaterawfee(threshold) assert_raises_rpc_error(-3, "JSON value of type string is not of expected type number", self.nodes[0].estimaterawfee, 1, 'foo') assert_raises_rpc_error(-8, 'Invalid estimate_mode parameter, must be one of: "unset", "economical", "conservative"', self.nodes[0].estimatesmartfee, 1, 'foo') + assert_raises_rpc_error(-8, "Unknown named parameter fee_rate_estimator", self.nodes[0].estimatesmartfee, 1, fee_rate_estimator=True) + assert_raises_rpc_error(-3, "Unexpected key block_policy_only", self.nodes[0].estimatesmartfee, 1, 'ECONOMICAL', {'block_policy_only': True}) # extra params - assert_raises_rpc_error(-1, "estimatesmartfee", self.nodes[0].estimatesmartfee, 1, 'ECONOMICAL', 1) + assert_raises_rpc_error(-1, "estimatesmartfee", self.nodes[0].estimatesmartfee, 1, 'ECONOMICAL', {}, 1) assert_raises_rpc_error(-1, "estimaterawfee", self.nodes[0].estimaterawfee, 1, 1, 1) # max value of 1008 per src/policy/fees/block_policy_estimator.h @@ -45,6 +49,8 @@ class EstimateFeeTest(BitcoinTestFramework): self.nodes[0].estimatesmartfee(1, 'ECONOMICAL') self.nodes[0].estimatesmartfee(1, 'unset') self.nodes[0].estimatesmartfee(1, 'conservative') + self.nodes[0].estimatesmartfee(1, 'ECONOMICAL', {"fee_rate_estimator": "block_policy"}) + self.nodes[0].estimatesmartfee(1, 'ECONOMICAL', {"fee_rate_estimator": "foo"}) self.nodes[0].estimaterawfee(1) self.nodes[0].estimaterawfee(1, None)