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.

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