From 0d88558f95113a5041fa957432fa5d9c12314466 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Thu, 23 Apr 2026 14:37:44 +0100 Subject: [PATCH] fees: return mempool estimates when it's lower than block policy Integrate MemPoolFeeRateEstimator into FeeRateEstimatorManager. When both estimators succeed, select the lower of the block policy and mempool estimates. When either estimator fails, return its error instead of falling back to the block policy estimate: if the mempool estimator cannot produce an estimate, the combined estimate fails. Callers that want a block-policy-only estimate can request it explicitly via fee_rate_estimator option. estimatesmartfee now emits the estimator field only for successful manager-selected estimates. Add a test that ensures estimatesmartfee returns the mempool fee rate estimate when it is lower than the block policy estimate, and can request the mempool policy estimator explicitly Two wallet functional tests also need adjusting. When the mempool is too sparse to fill its percentile buckets, MemPoolFeeRateEstimator returns a relayable floor of max(min relay fee, mempool min fee), so in regtest getFeeRateEstimate now returns the min relay fee where the wallet previously had no estimate and fell back to a higher rate: - wallet_taproot.py: the cleanup sendall used automatic fee estimation. GetMinimumFeeRate previously fell back to the wallet fallback fee (fallbackfee, 20 sat/vB in the test framework); it now uses the min relay fee floor. At that lower feerate the wallet's underestimate of the taproot script-path witness size drops the effective feerate below min relay, so the transaction is rejected. Pin fee_rate=20 to match the framework fallbackfee. - wallet_bumpfee.py: GetDiscardRate() previously fell back to the wallet discard rate (-discardfee); it now takes the minimum of that and the estimate, so the min relay fee floor collapses the discard rate down to the dust relay feerate. The lower discard rate reduces the cost of change, so the ~614 sat leftover change in test_dust_to_fee is now retained instead of being dropped to fee. Rework the test to leave a sub-dust (20/270 sat) change that is dropped regardless of the discard rate. Co-authored-by: willcl-ark --- src/common/messages.cpp | 8 +- src/init.cpp | 1 + src/policy/fees/estimator_man.cpp | 21 +++- src/policy/fees/estimator_man.h | 2 +- src/rpc/fees.cpp | 20 ++-- src/util/fees.h | 9 ++ src/wallet/test/fuzz/fees.cpp | 6 +- test/functional/feature_fee_estimation.py | 119 +++++++++++++++++----- test/functional/wallet_bumpfee.py | 21 ++-- test/functional/wallet_taproot.py | 10 +- 10 files changed, 161 insertions(+), 56 deletions(-) diff --git a/src/common/messages.cpp b/src/common/messages.cpp index 8e6a83302fe..8b5fc147804 100644 --- a/src/common/messages.cpp +++ b/src/common/messages.cpp @@ -56,13 +56,9 @@ std::string FeeModeInfo(const std::pair& mode, std case FeeEstimateMode::UNSET: return strprintf("%s means no mode set (%s). \n", mode.first, default_info); case FeeEstimateMode::ECONOMICAL: - return strprintf("%s estimates use a shorter time horizon, making them more\n" - "responsive to short-term drops in the prevailing fee market. This mode\n" - "potentially returns a lower fee rate estimate.\n", mode.first); + return strprintf("%s mode potentially returns a lower fee rate estimate.\n", mode.first); case FeeEstimateMode::CONSERVATIVE: - return strprintf("%s estimates use a longer time horizon, making them\n" - "less responsive to short-term drops in the prevailing fee market. This mode\n" - "potentially returns a higher fee rate estimate.\n", mode.first); + return strprintf("%s potentially returns a higher fee rate estimate.\n", mode.first); } // no default case, so the compiler can warn about missing cases assert(false); } diff --git a/src/init.cpp b/src/init.cpp index ed68dfa050d..1f1c7064b24 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -69,6 +69,7 @@ #include #include #include +#include #include #include #include diff --git a/src/policy/fees/estimator_man.cpp b/src/policy/fees/estimator_man.cpp index 15f75818df4..472367e8fa3 100644 --- a/src/policy/fees/estimator_man.cpp +++ b/src/policy/fees/estimator_man.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -19,7 +20,23 @@ FeeRateEstimatorManager::FeeRateEstimatorManager(const fs::path& block_policy_pa util::Expected FeeRateEstimatorManager::GetFeeRateEstimate(int target, bool conservative) const { - return m_block_policy_estimator->EstimateFeeRate(target, conservative); + auto block_policy_estimate = m_block_policy_estimator->EstimateFeeRate(target, conservative); + if (!block_policy_estimate) { + LogDebug(BCLog::ESTIMATEFEE, "%s", block_policy_estimate.error().reason); + return block_policy_estimate; + } + auto mempool_estimate = m_mempool_estimator->EstimateFeeRate(conservative); + if (!mempool_estimate) { + // A failed mempool estimate is surfaced as a warning rather than silently returning the + // block policy estimate, which callers can still request explicitly. + LogDebug(BCLog::ESTIMATEFEE, "%s", mempool_estimate.error().reason); + return mempool_estimate; + } + auto selected_estimate = std::min(*block_policy_estimate, *mempool_estimate); + LogDebug(BCLog::ESTIMATEFEE, "Fee rate estimated using %s: target=%s feerate=%s %s/kvB.", + FeeRateEstimatorTypeToString(selected_estimate.feerate_estimator), + selected_estimate.returned_target, CFeeRate(selected_estimate.feerate).GetFeePerK(), CURRENCY_ATOM); + return selected_estimate; } util::Expected FeeRateEstimatorManager::GetFeeRateEstimate(FeeRateEstimatorType type, int target, bool conservative) const @@ -72,5 +89,5 @@ unsigned int FeeRateEstimatorManager::BlockPolicyHighestTargetTracked(FeeEstimat unsigned int FeeRateEstimatorManager::MaximumTarget() const { - return m_block_policy_estimator->MaximumTarget(); + return std::max(m_block_policy_estimator->MaximumTarget(), m_mempool_estimator->MaximumTarget()); } diff --git a/src/policy/fees/estimator_man.h b/src/policy/fees/estimator_man.h index d7552065daf..bbcb8aa1b28 100644 --- a/src/policy/fees/estimator_man.h +++ b/src/policy/fees/estimator_man.h @@ -40,7 +40,7 @@ public: virtual ~FeeRateEstimatorManager(); /** - * @brief Get a fee rate estimate from block policy estimator. + * @brief Get a fee rate estimate from the available fee rate estimators. * @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. diff --git a/src/rpc/fees.cpp b/src/rpc/fees.cpp index ac8989e216d..57bedd58f0e 100644 --- a/src/rpc/fees.cpp +++ b/src/rpc/fees.cpp @@ -46,7 +46,9 @@ static RPCMethod estimatesmartfee() { {"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" + "\"none\" returns the lower of the block policy and mempool estimates. If the mempool\n" + "estimate is unavailable, it returns that error instead of falling back to the block\n" + "policy estimate; use \"block_policy\" in that case to get the block policy estimate.\n" "\"block_policy\" uses only the block policy fee rate estimator.\n" "\"mempool_policy\" uses only the mempool fee rate estimator.\n" "Unknown values are treated as \"none\"."}, @@ -57,15 +59,15 @@ static RPCMethod estimatesmartfee() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::NUM, "feerate", /*optional=*/true, "estimate fee rate in " + CURRENCY_UNIT + "/kvB (only present if no errors were encountered)"}, + {RPCResult::Type::STR, "estimator", /*optional=*/true, "the fee estimator used to produce the result (only present for successful estimates when fee_rate_estimator is \"none\")"}, {RPCResult::Type::ARR, "errors", /*optional=*/true, "Errors encountered during processing (if there are any)", { {RPCResult::Type::STR, "", "error"}, }}, - {RPCResult::Type::NUM, "blocks", "block number where estimate was found\n" - "The request target will be clamped between 2 and the highest target\n" - "fee estimation is able to return based on how long it has been running.\n" - "An error is returned if not enough transactions and blocks\n" - "have been observed to make an estimate for any number of blocks."}, + {RPCResult::Type::NUM, "blocks", "the confirmation target in blocks for the returned fee rate estimate.\n" + "For the block policy fee rate estimator, this is the target the estimate was found at, clamped to at\n" + "least 2 and at most the estimator's maximum usable target. For the mempool fee rate\n" + "estimator, it is always 2."}, }}, RPCExamples{ HelpExampleCli("estimatesmartfee", "6") + @@ -105,7 +107,11 @@ static RPCMethod estimatesmartfee() errors.push_back(estimate.error().reason); result.pushKV("errors", std::move(errors)); } - result.pushKV("blocks", FeeRateEstimationRef(estimate).returned_target); + if (estimate && fee_rate_estimator == FeeRateEstimatorType::NONE) { + result.pushKV("estimator", FeeRateEstimatorTypeToString(estimate->feerate_estimator)); + } + const FeeRateEstimation& estimation{FeeRateEstimationRef(estimate)}; + result.pushKV("blocks", estimation.returned_target); return result; }, }; diff --git a/src/util/fees.h b/src/util/fees.h index fc61db06493..3ce3775ad39 100644 --- a/src/util/fees.h +++ b/src/util/fees.h @@ -50,6 +50,15 @@ struct FeeRateEstimation { FeePerVSize feerate; //! The returned confirmation target for the estimate. int returned_target; + /** + * Compare two FeeRateEstimation objects based on fee rate + * @param other The other FeeRateEstimation object to compare with + * @return strong ordering of either less, greater or equal; based on feerate ratio comparison. + */ + auto operator<=>(const FeeRateEstimation& other) const + { + return ByRatio{feerate} <=> ByRatio{other.feerate}; + } }; /** diff --git a/src/wallet/test/fuzz/fees.cpp b/src/wallet/test/fuzz/fees.cpp index b8fea1be614..781bfa6d38e 100644 --- a/src/wallet/test/fuzz/fees.cpp +++ b/src/wallet/test/fuzz/fees.cpp @@ -45,8 +45,8 @@ class FuzzedFeeEstimatorMan : public FeeRateEstimatorManager FuzzedDataProvider& fuzzed_data_provider; public: - FuzzedFeeEstimatorMan(FuzzedDataProvider& provider) - : FeeRateEstimatorManager(fs::path{}, false, *g_setup->m_node.mempool, *g_setup->m_node.chainman), fuzzed_data_provider(provider) {} + FuzzedFeeEstimatorMan(FuzzedDataProvider& provider, const CTxMemPool& mempool, ChainstateManager& chainman) + : FeeRateEstimatorManager(fs::path{}, false, mempool, chainman), fuzzed_data_provider(provider) {} util::Expected GetFeeRateEstimate(int confTarget, bool conservative) const override { @@ -86,7 +86,7 @@ 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_man = std::make_unique(fuzzed_data_provider); + std::unique_ptr fee_estimator_man = std::make_unique(fuzzed_data_provider, *node.mempool, *node.chainman); 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 && diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 32aaa92c351..5edc1fc9f28 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -11,6 +11,9 @@ import time from test_framework.messages import ( COIN, + DEFAULT_BLOCK_RESERVED_WEIGHT, + MAX_BLOCK_WEIGHT, + WITNESS_SCALE_FACTOR, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -27,6 +30,7 @@ MAX_FILE_AGE = 60 SECONDS_PER_HOUR = 60 * 60 MIN_BUCKET_FEERATE = Decimal(100) / Decimal(COIN) TXS_COUNT = 24 +BLOCK_POLICY_ESTIMATOR_ERROR = "Insufficient data or no feerate found" def small_txpuzzle_randfee( wallet, from_node, conflist, unconflist, amount, min_fee, fee_increment, batch_reqs @@ -140,6 +144,14 @@ def check_fee_estimates_btw_modes(node, expected_conservative, expected_economic assert_equal(fee_est_economical, expected_economical) assert_equal(fee_est_default, expected_economical) +def verify_estimate_response(estimate, feerate, errors): + if feerate: + assert_equal(estimate["feerate"], feerate) + if errors: + assert all(err in estimate["errors"] for err in errors) + else: + assert "errors" not in estimate + class EstimateFeeTest(BitcoinTestFramework): def set_test_params(self): @@ -330,7 +342,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, "economical", {"fee_rate_estimator": "block_policy"})["errors"], ["Insufficient data or no feerate found"]) + assert_equal(self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["errors"], [BLOCK_POLICY_ESTIMATOR_ERROR]) def test_estimate_dat_is_flushed_periodically(self): @@ -413,35 +425,42 @@ class EstimateFeeTest(BitcoinTestFramework): self.sync_blocks() 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): + def broadcast_and_maybe_mine(self, broadcaster, feerate, txs, blocks=1, miner=None): """Broadcast and maybe mine some number of transactions with a specified fee rate.""" - tx_batch = [] - for _ in range(count): - tx = self.wallet.create_self_transfer(fee_rate=feerate, utxo_to_spend=self.confutxo.pop(0)) - self.memutxo.append(tx["new_utxo"]) - tx_batch.append(tx) - # To speed up the test, submit the transactions in batches to the nodes directly - # avoiding having to wait for p2p to propagate them between the nodes. - batch_send_tx = [broadcaster.sendrawtransaction.get_request(hexstring=tx["hex"]) for tx in tx_batch] - for node in self.nodes: - node.batch(batch_send_tx) - self.sync_mempools(wait=0.1, nodes=[self.nodes[0], self.nodes[1], self.nodes[2]]) - if miner: - mined = miner.getblock(self.generate(miner, 1)[0], True)["tx"] - self.update_utxo(mined) + for _ in range(blocks): + tx_batch = [] + for _ in range(txs): + tx = self.wallet.create_self_transfer(fee_rate=feerate, utxo_to_spend=self.confutxo.pop(0)) + self.memutxo.append(tx["new_utxo"]) + tx_batch.append(tx) + # To speed up the test, submit the transactions in batches to the nodes directly + # avoiding having to wait for p2p to propagate them between the nodes. + batch_send_tx = [broadcaster.sendrawtransaction.get_request(hexstring=tx["hex"]) for tx in tx_batch] + for node in self.nodes: + node.batch(batch_send_tx) + self.sync_mempools(wait=0.1, nodes=[self.nodes[0], self.nodes[1], self.nodes[2]]) + if miner: + mined = miner.getblock(self.generate(miner, 1)[0], True)["tx"] + self.update_utxo(mined) + + def send_transactions(self, utxos, fee_rate, target_vsize): + for utxo in utxos: + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=utxo, + fee_rate=fee_rate, + target_vsize=target_vsize, + ) def test_estimation_modes(self): low_feerate = Decimal("0.001") high_feerate = Decimal("0.005") # Broadcast and mine high fee transactions for the first 12 blocks. - for _ in range(12): - self.broadcast_many(self.nodes[1], high_feerate, TXS_COUNT, self.nodes[2]) + self.broadcast_and_maybe_mine(self.nodes[1], high_feerate, TXS_COUNT, 12, self.nodes[2]) check_fee_estimates_btw_modes(self.nodes[0], high_feerate, high_feerate) - # We now track 12 blocks; short horizon stats will start decaying. # Broadcast and mine low fee transactions for the next 4 blocks. - for _ in range(4): - self.broadcast_many(self.nodes[1], low_feerate, TXS_COUNT, self.nodes[2]) + self.broadcast_and_maybe_mine(self.nodes[1], low_feerate, TXS_COUNT, 4, self.nodes[2]) # conservative mode will consider longer time horizons while economical mode does not # Check the fee estimates for both modes after mining low fee transactions. check_fee_estimates_btw_modes(self.nodes[0], high_feerate, low_feerate) @@ -450,10 +469,60 @@ class EstimateFeeTest(BitcoinTestFramework): feerate_0_5_s_per_vb = MIN_BUCKET_FEERATE * 5 feerate_1_s_per_vb = Decimal(1000) / Decimal(COIN) 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]) + self.broadcast_and_maybe_mine(self.nodes[1], feerate_0_5_s_per_vb, TXS_COUNT) + self.broadcast_and_maybe_mine(self.nodes[1], feerate_1_s_per_vb, TXS_COUNT, 1, self.nodes[2]) assert_equal(feerate_0_5_s_per_vb, self.nodes[0].estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"})["feerate"]) + + def test_estimatesmartfee_return_mempool_estimates(self): + node0 = self.nodes[0] + miner = self.nodes[1] + self.log.info("Ensure node0's mempool is empty at the start") + assert_equal(node0.getmempoolinfo()['size'], 0) + self.log.info("Test estimatesmartfee with empty mempool and no block policy estimator data") + estimate_after_restart = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "none"}) + verify_estimate_response(estimate_after_restart, None, [BLOCK_POLICY_ESTIMATOR_ERROR]) + self.log.info("Populate block policy estimator with high-feerate history") + # Generate high-feerate transactions and mine them over 6 blocks to give block policy data. + high_feerate = Decimal("0.004") + self.broadcast_and_maybe_mine(node0, high_feerate, TXS_COUNT, 6, miner) + self.log.info("Test estimatesmartfee returns block policy estimator estimate when mempool is higher") + # Add 10 large insane-feerate transactions enough to generate a block template + num_txs = 10 + target_vsize = int(((MAX_BLOCK_WEIGHT - DEFAULT_BLOCK_RESERVED_WEIGHT) / WITNESS_SCALE_FACTOR) / num_txs) + utxos = [self.wallet.get_utxo(confirmed_only=True) for _ in range(num_txs)] + insane_feerate = Decimal("0.01") + self.send_transactions(utxos, insane_feerate, target_vsize) + estimate_after_spike = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "none"}) + verify_estimate_response(estimate_after_spike, high_feerate, []) + assert_equal(estimate_after_spike["estimator"], "block_policy") + mempool_policy_estimate = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "mempool_policy"}) + verify_estimate_response(mempool_policy_estimate, insane_feerate, []) + # Confirm the spike transactions so they leave the mempool; the mined block + # keeps the mempool representation healthy. Then broadcast fresh low-feerate + # transactions so the mempool estimate is now the lower of the two. + self.generate(node0, 1, sync_fun=lambda: None) + assert_equal(node0.getmempoolinfo()['size'], 0) + low_feerate = Decimal("0.00004") + low_utxos = [self.wallet.get_utxo(confirmed_only=True) for _ in range(num_txs)] + self.send_transactions(low_utxos, low_feerate, target_vsize) + lower_estimate = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "none"}) + verify_estimate_response(lower_estimate, low_feerate, []) + + self.log.info("Test estimatesmartfee returns the fee rate floor when the mempool is empty but healthy") + self.generate(node0, 1, sync_fun=lambda: None) + assert_equal(node0.getmempoolinfo()['size'], 0) + block_policy_estimate = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "block_policy"}) + assert "feerate" in block_policy_estimate + # With an empty but healthy mempool the mempool estimator has no percentile data, + # so it falls back to the fee rate floor: the max of minrelaytxfee and mempoolminfee. + # That floor is lower than the block policy estimate, so the combined estimator returns it. + mempool_info = node0.getmempoolinfo() + floor = max(mempool_info["minrelaytxfee"], mempool_info["mempoolminfee"]) + combined_estimate = node0.estimatesmartfee(1, "economical", {"fee_rate_estimator": "none"}) + verify_estimate_response(combined_estimate, floor, []) + assert_equal(combined_estimate["estimator"], "mempool_policy") + def run_test(self): self.log.info("This test is time consuming, please be patient") self.log.info("Splitting inputs so we can generate tx's") @@ -504,6 +573,10 @@ class EstimateFeeTest(BitcoinTestFramework): self.log.info("Test that estimatesmartfee returns a sub 1s/vb fee rate estimate") self.test_sub_1s_per_vb_estimates() + self.log.info("Test that estimatesmartfee returns mempool estimates when lower") + self.clear_estimates() + self.test_estimatesmartfee_return_mempool_estimates() + self.log.info("Testing that fee estimation is disabled in blocksonly.") self.restart_node(0, ["-blocksonly"]) assert_raises_rpc_error( diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index ee0c0451511..705c7548d84 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -511,7 +511,7 @@ def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address): def test_dust_to_fee(self, rbf_node, dest_address): self.log.info('Test that bumped output that is dust is dropped to fee') - rbfid = spend_one_input(rbf_node, dest_address) + rbfid = spend_one_input(rbf_node, dest_address, change_size=Decimal("0.00030000"), dest_amount=Decimal("0.00064730")) fulltx = rbf_node.getrawtransaction(rbfid, 1) # The DER formatting used by Bitcoin to serialize ECDSA signatures means that signatures can have a # variable size of 70-72 bytes (or possibly even less), with most being 71 or 72 bytes. The signature @@ -519,16 +519,17 @@ def test_dust_to_fee(self, rbf_node, dest_address): # boundary. Thus expected transaction size (p2wpkh, 1 input, 2 outputs) is 140-141 vbytes, usually 141. if not 140 <= fulltx["vsize"] <= 141: raise AssertionError("Invalid tx vsize of {} (140-141 expected), full tx: {}".format(fulltx["vsize"], fulltx)) - # Bump with fee_rate of 350.25 sat/vB vbytes to create dust. - # Expected fee is 141 vbytes * fee_rate 0.00350250 BTC / 1000 vbytes = 0.00049385 BTC. - # or occasionally 140 vbytes * fee_rate 0.00350250 BTC / 1000 vbytes = 0.00049035 BTC. - # Dust should be dropped to the fee, so actual bump fee is 0.00050000 BTC. - bumped_tx = rbf_node.bumpfee(rbfid, fee_rate=350.25) + # Bump with fee_rate of 250 sat/vB. The leftover change is below the dust + # threshold, so it is dropped and folded into the fee. + # Target fee is 141 vbytes * 0.00250000 BTC / 1000 vbytes = 0.00035250 BTC (20 sat change left), + # or occasionally 140 vbytes * 0.00250000 BTC / 1000 vbytes = 0.00035000 BTC (270 sat change left). + # Either way the sub-dust change is added to the fee, giving 0.00035270 BTC. + bumped_tx = rbf_node.bumpfee(rbfid, fee_rate=250) full_bumped_tx = rbf_node.getrawtransaction(bumped_tx["txid"], 1) - assert_equal(bumped_tx["fee"], Decimal("0.00050000")) + assert_equal(bumped_tx["fee"], Decimal("0.00035270")) assert_equal(len(fulltx["vout"]), 2) assert_equal(len(full_bumped_tx["vout"]), 1) # change output is eliminated - assert_equal(full_bumped_tx["vout"][0]['value'], Decimal("0.00050000")) + assert_equal(full_bumped_tx["vout"][0]['value'], Decimal("0.00064730")) self.clear_mempool() def test_maxtxfee_fails(self, rbf_node, dest_address): @@ -755,10 +756,10 @@ def test_change_script_match(self, rbf_node, dest_address): self.clear_mempool() -def spend_one_input(node, dest_address, change_size=Decimal("0.00049000"), data=None): +def spend_one_input(node, dest_address, change_size=Decimal("0.00049000"), data=None, dest_amount=Decimal("0.00050000")): tx_input = dict( sequence=MAX_BIP125_RBF_SEQUENCE, **next(u for u in node.listunspent() if u["amount"] == Decimal("0.00100000"))) - destinations = {dest_address: Decimal("0.00050000")} + destinations = {dest_address: dest_amount} if change_size > 0: destinations[node.getrawchangeaddress()] = change_size if data: diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index b312d4e1536..1aca5d421c8 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -165,8 +165,9 @@ class WalletTaprootTest(BitcoinTestFramework): self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert rpc_online.gettransaction(res)["confirmations"] > 0 - # Cleanup - txid = rpc_online.sendall(recipients=[self.boring.getnewaddress()])["txid"] + # Match the framework fallbackfee; otherwise the underestimated taproot + # script-path spend size can produce an effective feerate below min relay. + txid = rpc_online.sendall(recipients=[self.boring.getnewaddress()], fee_rate=20)["txid"] self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert rpc_online.gettransaction(txid)["confirmations"] > 0 rpc_online.unloadwallet() @@ -238,8 +239,9 @@ class WalletTaprootTest(BitcoinTestFramework): self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert psbt_online.gettransaction(txid)['confirmations'] > 0 - # Cleanup - psbt = psbt_online.sendall(recipients=[self.boring.getnewaddress()], psbt=True)["psbt"] + # Match the framework fallbackfee; otherwise the underestimated taproot + # script-path spend size can produce an effective feerate below min relay. + psbt = psbt_online.sendall(recipients=[self.boring.getnewaddress()], psbt=True, fee_rate=20)["psbt"] res = psbt_offline.walletprocesspsbt(psbt=psbt, finalize=False) rawtx = self.nodes[0].finalizepsbt(res['psbt'])['hex'] txid = self.nodes[0].sendrawtransaction(rawtx)