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)