From e5b7785447fc130e8eb6d1a5e8ff051c68b0e8c6 Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Thu, 25 Jun 2026 14:55:55 -0700 Subject: [PATCH] test: wallet: resend: avoid internal behavior via removeprunedfunds Currently this test makes assumptions about `listtransaction` exposing a wallet-internal data structure `mapWallet` That is not guaranteed or enforced anywhere. Ideally, this would live in a unit test instead of a functional test, but as a half-measure to simplify the test, just check the behavior 10 times, if there are any dependencies on random ordering inside of a wallet data structure, this is likely to catch them. --- .../wallet_resendwallettransactions.py | 101 +++++++----------- 1 file changed, 36 insertions(+), 65 deletions(-) diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py index 1b15fa17e25..b72c79d3620 100755 --- a/test/functional/wallet_resendwallettransactions.py +++ b/test/functional/wallet_resendwallettransactions.py @@ -5,8 +5,6 @@ """Test that the wallet resends transactions periodically.""" import time -from decimal import Decimal - from test_framework.blocktools import ( create_block, ) @@ -16,8 +14,6 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - get_fee, - try_rpc, ) # 36 hours is the upper limit of the resend timer, see CWallet::SetNextResend() @@ -83,72 +79,47 @@ class ResendWalletTransactionsTest(BitcoinTestFramework): peer_second.wait_for_broadcast([txid]) self.log.info("Chain of unconfirmed not-in-mempool txs are rebroadcast") - # This tests that the node broadcasts the parent transaction before the child transaction. - # To test that scenario, we need a method to reliably get a child transaction placed - # in mapWallet positioned before the parent. We cannot predict the position in mapWallet, - # but we can observe it using listreceivedbyaddress and other related RPCs. - # - # So we will create the child transaction, use listreceivedbyaddress to see what the - # ordering of mapWallet is, if the child is not before the parent, we will create a new - # child (via bumpfee) and remove the old child (via removeprunedfunds) until we get the - # ordering of child before parent. - child_inputs = [{"txid": txid, "vout": 0}] - child_txid = node.sendall(recipients=[addr], inputs=child_inputs)["txid"] - # Get the child tx's info for manual bumping - child_tx_info = node.gettransaction(txid=child_txid, verbose=True) - child_output_value = child_tx_info["decoded"]["vout"][0]["value"] - # Include an additional 1 vbyte buffer to handle when we have a smaller signature - additional_child_fee = get_fee(child_tx_info["decoded"]["vsize"] + 1, Decimal(0.00001100)) - while True: - txids = node.listreceivedbyaddress(minconf=0, address_filter=addr)[0]["txids"] - if txids == [child_txid, txid]: - break - # Manually bump the tx - # The inputs and the output address stay the same, just changing the amount for the new fee - child_output_value -= additional_child_fee - bumped_raw = node.createrawtransaction(inputs=child_inputs, outputs=[{addr: child_output_value}]) - bumped = node.signrawtransactionwithwallet(bumped_raw) - bumped_txid = node.decoderawtransaction(bumped["hex"])["txid"] - # Sometimes we will get a signature that is a little bit shorter than we expect which causes the - # feerate to be a bit higher, then the followup to be a bit lower. This results in a replacement - # that can't be broadcast. We can just skip that and keep grinding. - if try_rpc(-26, "insufficient fee, rejecting replacement", node.sendrawtransaction, bumped["hex"]): - continue - # The scheduler queue creates a copy of the added tx after - # send/bumpfee and re-adds it to the wallet (undoing the next - # removeprunedfunds). So empty the scheduler queue: + # We cannot predict the ordering in mapWallet of parent and child, so + # try a few times to get both. + evict_time = 0 + for _ in range(10): + child_inputs = [{"txid": txid, "vout": 0}] + child_txid = node.sendall(recipients=[addr], inputs=child_inputs)["txid"] + # Get the child tx's info for manual bumping + entry_time = node.getmempoolentry(child_txid)["time"] + + # tx must be at least 5 minutes older than the last block to be rebroadcast + block_time = entry_time + 5 * 60 + 1 + node.setmocktime(block_time) + block = create_block(int(node.getbestblockhash(), 16), height=node.getblockcount() + 1, ntime=block_time) + block.solve() + node.submitblock(block.serialize().hex()) + # Set correct m_best_block_time, which is used in ResubmitWalletTransactions node.syncwithvalidationinterfacequeue() - node.removeprunedfunds(child_txid) - child_txid = bumped_txid - entry_time = node.getmempoolentry(child_txid)["time"] - # tx must be at least 5 minutes older than the last block to be rebroadcast - block_time = entry_time + 6 * 60 - node.setmocktime(block_time) - block = create_block(int(node.getbestblockhash(), 16), height=node.getblockcount() + 1, ntime=block_time) - block.solve() - node.submitblock(block.serialize().hex()) - # Set correct m_best_block_time, which is used in ResubmitWalletTransactions - node.syncwithvalidationinterfacequeue() + evict_time = block_time + 60 * 60 * DEFAULT_MEMPOOL_EXPIRY_HOURS + 5 + # Flush out currently scheduled resubmit attempt now so that there can't be one right between eviction and check. + with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): + node.setmocktime(evict_time) + node.mockscheduler(60) - evict_time = block_time + 60 * 60 * DEFAULT_MEMPOOL_EXPIRY_HOURS + 5 - # Flush out currently scheduled resubmit attempt now so that there can't be one right between eviction and check. - with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): - node.setmocktime(evict_time) - node.mockscheduler(60) + # Evict these txs from the mempool + indep_send = node.send(outputs=[{node.getnewaddress(): 1}], inputs=[indep_utxo]) + node.getmempoolentry(indep_send["txid"]) + assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, txid) + assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, child_txid) - # Evict these txs from the mempool - indep_send = node.send(outputs=[{node.getnewaddress(): 1}], inputs=[indep_utxo]) - node.getmempoolentry(indep_send["txid"]) - assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, txid) - assert_raises_rpc_error(-5, "Transaction not in mempool", node.getmempoolentry, child_txid) + # Rebroadcast and check that parent and child are both in the mempool + with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): + node.setmocktime(evict_time + RESEND_TIMER_LIMIT) + node.mockscheduler(60) + node.getmempoolentry(txid) + node.getmempoolentry(child_txid) - # Rebroadcast and check that parent and child are both in the mempool - with node.assert_debug_log(['resubmit 2 unconfirmed transactions'], timeout=2): - node.setmocktime(evict_time + RESEND_TIMER_LIMIT) - node.mockscheduler(60) - node.getmempoolentry(txid) - node.getmempoolentry(child_txid) + # clear mempool + self.generate(node, 1, sync_fun=self.no_op) + parent_utxo, indep_utxo = node.listunspent()[:2] + txid = node.send(outputs=[{addr: 1}], inputs=[parent_utxo])["txid"] self.log.info("Test rebroadcast of transactions received by others") # clear mempool