Merge bitcoin/bitcoin#25768: wallet: Properly rebroadcast unconfirmed transaction chains

3405f3eed5cf841b23a569b64a376c2e5b5026cd test: Test that an unconfirmed not-in-mempool chain is rebroadcast (Andrew Chow)
10d91c5abe9ed7dcc237c9d52c588e7d26e162a4 wallet: Deduplicate Resend and ReacceptWalletTransactions (Andrew Chow)

Pull request description:

  Currently `ResendWalletTransactions` (used for normal rebroadcasts) will attempt to rebroadcast all of the transactions in the wallet in the order they are stored in `mapWallet`. This ends up being random as `mapWallet` is a `std::unordered_map`. However `ReacceptWalletTransactions` (used for adding to the mempool on loading) first sorts the txs by wallet insertion order, then submits them. The result is that `ResendWalletTranactions` will fail to rebroadcast child transactions if their txids happen to be lexicographically less than their parent's txid. This PR resolves this issue by combining `ReacceptWalletTransactions` and `ResendWalletTransactions` into a new `ResubmitWalletTransactions` so that the iteration code and basic checks are shared.

  A test has also been added that checks that such transaction chains are rebroadcast correctly.

ACKs for top commit:
  naumenkogs:
    utACK 3405f3eed5cf841b23a569b64a376c2e5b5026cd
  1440000bytes:
    reACK https://github.com/bitcoin/bitcoin/pull/25768/commits/3405f3eed5cf841b23a569b64a376c2e5b5026cd
  furszy:
    Late code review ACK 3405f3ee
  stickies-v:
    ACK 3405f3eed5cf841b23a569b64a376c2e5b5026cd

Tree-SHA512: 1240d9690ecc2ae8d476286b79e2386f537a90c41dd2b8b8a5a9c2a917aa3af85d6aee019fbbb05e772985a2b197e2788305586d9d5dac78ccba1ee5aa31d77a

3 files changed, 55 insertions, 12 deletions

diff --git a/test/functional/mempool_expiry.py b/test/functional/mempool_expiry.py
index 47ae0c762b..21721177e6 100755
--- a/test/functional/mempool_expiry.py
+++ b/test/functional/mempool_expiry.py
@@ -13,6 +13,7 @@ definable expiry timeout via the '-mempoolexpiry=<n>' command line argument
 from datetime import timedelta
 
 from test_framework.blocktools import COINBASE_MATURITY
+from test_framework.messages import DEFAULT_MEMPOOL_EXPIRY_HOURS
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -20,7 +21,6 @@ from test_framework.util import (
 )
 from test_framework.wallet import MiniWallet
 
-DEFAULT_MEMPOOL_EXPIRY_HOURS = 336  # hours
 CUSTOM_MEMPOOL_EXPIRY = 10  # hours
 
 
diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py
index 4e757b64ca..8a928a1e50 100755
--- a/test/functional/test_framework/messages.py
+++ b/test/functional/test_framework/messages.py
@@ -71,6 +71,7 @@ DEFAULT_DESCENDANT_LIMIT = 25  # default max number of in-mempool descendants
 # Default setting for -datacarriersize. 80 bytes of data, +1 for OP_RETURN, +2 for the pushdata opcodes.
 MAX_OP_RETURN_RELAY = 83
 
+DEFAULT_MEMPOOL_EXPIRY_HOURS = 336  # hours
 
 def sha256(s):
     return hashlib.sha256(s).digest()
diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py
index 4ef259efe1..26df0841d8 100755
--- a/test/functional/wallet_resendwallettransactions.py
+++ b/test/functional/wallet_resendwallettransactions.py
@@ -9,10 +9,13 @@ from test_framework.blocktools import (
     create_block,
     create_coinbase,
 )
+from test_framework.messages import DEFAULT_MEMPOOL_EXPIRY_HOURS
 from test_framework.p2p import P2PTxInvStore
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
-
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+)
 
 class ResendWalletTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -27,13 +30,9 @@ class ResendWalletTransactionsTest(BitcoinTestFramework):
         peer_first = node.add_p2p_connection(P2PTxInvStore())
 
         self.log.info("Create a new transaction and wait until it's broadcast")
-        txid = node.sendtoaddress(node.getnewaddress(), 1)
-
-        # Wallet rebroadcast is first scheduled 1 min sec after startup (see
-        # nNextResend in ResendWalletTransactions()). Tell scheduler to call
-        # MaybeResendWalletTxs now to initialize nNextResend before the first
-        # setmocktime call below.
-        node.mockscheduler(60)
+        parent_utxo, indep_utxo = node.listunspent()[:2]
+        addr = node.getnewaddress()
+        txid = node.send(outputs=[{addr: 1}], options={"inputs": [parent_utxo]})["txid"]
 
         # Can take a few seconds due to transaction trickling
         peer_first.wait_for_broadcast([txid])
@@ -51,7 +50,7 @@ class ResendWalletTransactionsTest(BitcoinTestFramework):
         block.solve()
         node.submitblock(block.serialize().hex())
 
-        # Set correct m_best_block_time, which is used in ResendWalletTransactions
+        # Set correct m_best_block_time, which is used in ResubmitWalletTransactions
         node.syncwithvalidationinterfacequeue()
         now = int(time.time())
 
@@ -66,7 +65,7 @@ class ResendWalletTransactionsTest(BitcoinTestFramework):
         self.log.info("Bump time & check that transaction is rebroadcast")
         # Transaction should be rebroadcast approximately 24 hours in the future,
         # but can range from 12-36. So bump 36 hours to be sure.
-        with node.assert_debug_log(['ResendWalletTransactions: resubmit 1 unconfirmed transactions']):
+        with node.assert_debug_log(['resubmit 1 unconfirmed transactions']):
             node.setmocktime(now + 36 * 60 * 60)
             # Tell scheduler to call MaybeResendWalletTxs now.
             node.mockscheduler(60)
@@ -74,6 +73,49 @@ class ResendWalletTransactionsTest(BitcoinTestFramework):
         node.setmocktime(now + 36 * 60 * 60 + 600)
         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_txid = node.send(outputs=[{addr: 0.5}], options={"inputs": [{"txid":txid, "vout":0}]})["txid"]
+        while True:
+            txids = node.listreceivedbyaddress(minconf=0, address_filter=addr)[0]["txids"]
+            if txids == [child_txid, txid]:
+                break
+            bumped = node.bumpfee(child_txid)
+            node.removeprunedfunds(child_txid)
+            child_txid = bumped["txid"]
+        entry_time = node.getmempoolentry(child_txid)["time"]
+
+        block_time = entry_time + 6 * 60
+        node.setmocktime(block_time)
+        block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockcount() + 1), block_time)
+        block.solve()
+        node.submitblock(block.serialize().hex())
+        node.syncwithvalidationinterfacequeue()
+
+        # Evict these txs from the mempool
+        evict_time = block_time + 60 * 60 * DEFAULT_MEMPOOL_EXPIRY_HOURS + 5
+        node.setmocktime(evict_time)
+        indep_send = node.send(outputs=[{node.getnewaddress(): 1}], options={"inputs": [indep_utxo]})
+        node.syncwithvalidationinterfacequeue()
+        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']):
+            node.setmocktime(evict_time + 36 * 60 * 60) # 36 hrs is the upper limit of the resend timer
+            node.mockscheduler(60)
+        node.getmempoolentry(txid)
+        node.getmempoolentry(child_txid)
+
 
 if __name__ == '__main__':
     ResendWalletTransactionsTest().main()