[wallet] update wallet to resubmit to node rather than rebroadcast

With the new functionality added to the mempool, the wallet
no longer needs to rebroadcast transactions to peers directly.
Instead, it resubmits txns to the nodes once a day in case they
were dropped (expired, evicted, etc.) from the local mempool
before being confirmed.

src/wallet/wallet.cpp

@@ -46,6 +46,8 @@ const std::map<uint64_t,std::string> WALLET_FLAG_CAVEATS{
 };
 
 static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10;
+// frequency of resubmitting txns to mempool- 24 hours in ms
+static const int RESEND_TXS_FREQUENCY = 1000 * 60 * 60 * 24;
 
 static CCriticalSection cs_wallets;
 static std::vector<std::shared_ptr<CWallet>> vpwallets GUARDED_BY(cs_wallets);
@@ -2344,47 +2346,29 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
         return CTransaction(tx1) == CTransaction(tx2);
 }
 
-// Rebroadcast transactions from the wallet. We do this on a random timer
-// to slightly obfuscate which transactions come from our wallet.
-//
-// Ideally, we'd only resend transactions that we think should have been
-// mined in the most recent block. Any transaction that wasn't in the top
-// blockweight of transactions in the mempool shouldn't have been mined,
-// and so is probably just sitting in the mempool waiting to be confirmed.
-// Rebroadcasting does nothing to speed up confirmation and only damages
-// privacy.
+// Once a day, resbumit all wallet transactions to the node,
+// in case it has been dropped from your mempool.
 void CWallet::ResendWalletTransactions()
 {
     // During reindex, importing and IBD, old wallet transactions become
-    // unconfirmed. Don't resend them as that would spam other nodes.
+    // unconfirmed. Don't need to resubmit to our node.
     if (!chain().isReadyToBroadcast()) return;
 
-    // Do this infrequently and randomly to avoid giving away
-    // that these are our transactions.
-    if (GetTime() < nNextResend || !fBroadcastTransactions) return;
-    bool fFirst = (nNextResend == 0);
-    nNextResend = GetTime() + GetRand(30 * 60);
-    if (fFirst) return;
-
-    // Only do it if there's been a new block since last time
-    if (m_best_block_time < nLastResend) return;
-    nLastResend = GetTime();
+    // Do this once per day.
+    if (GetTime() < nNextResend) return;
+    nNextResend = GetTime() + RESEND_TXS_FREQUENCY;
 
     int submitted_tx_count = 0;
 
     { // locked_chain and cs_wallet scope
         auto locked_chain = chain().lock();
         LOCK(cs_wallet);
 
-        // Relay transactions
+        // Resubmit transactions
         for (std::pair<const uint256, CWalletTx>& item : mapWallet) {
             CWalletTx& wtx = item.second;
-            // Attempt to rebroadcast all txes more than 5 minutes older than
-            // the last block. SubmitMemoryPoolAndRelay() will not rebroadcast
-            // any confirmed or conflicting txs.
-            if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue;
             std::string unused_err_string;
-            if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true, *locked_chain)) ++submitted_tx_count;
+            if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, false, *locked_chain)) ++submitted_tx_count;
         }
     } // locked_chain and cs_wallet
 

src/wallet/wallet.h

@@ -722,7 +722,6 @@ class CWallet final : public FillableSigningProvider, private interfaces::Chain:
     int nWalletMaxVersion GUARDED_BY(cs_wallet) = FEATURE_BASE;
 
     int64_t nNextResend = 0;
-    int64_t nLastResend = 0;
     bool fBroadcastTransactions = false;
     // Local time that the tip block was received. Used to schedule wallet rebroadcasts.
     std::atomic<int64_t> m_best_block_time {0};

test/functional/wallet_resendwallettransactions.py

@@ -6,11 +6,10 @@
 from collections import defaultdict
 import time
 
-from test_framework.blocktools import create_block, create_coinbase
-from test_framework.messages import ToHex
-from test_framework.mininode import P2PInterface, mininode_lock
+from test_framework.blocktools import create_coinbase
+from test_framework.mininode import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal, wait_until
+from test_framework.util import wait_until
 
 class P2PStoreTxInvs(P2PInterface):
     def __init__(self):
@@ -32,45 +31,45 @@ def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
 
     def run_test(self):
-        node = self.nodes[0]  # alias
 
-        node.add_p2p_connection(P2PStoreTxInvs())
+        node = self.nodes[0]
 
-        self.log.info("Create a new transaction and wait until it's broadcast")
-        txid = int(node.sendtoaddress(node.getnewaddress(), 1), 16)
+        self.log.info("Create a new wallet transaction")
+
+        relayfee = node.getnetworkinfo()['relayfee']
+        node.settxfee(relayfee)
+        txhsh = node.sendtoaddress(node.getnewaddress(), 1)
+
+        assert txhsh in node.getrawmempool()
+
+        # bump mocktime so the transaction should expire
+        # add an extra hour for good measure
+        two_weeks_in_seconds = 60 * 60 * 24 * 14
+        mocktime = int(time.time()) + two_weeks_in_seconds + 60 * 60
+        node.setmocktime(mocktime)
+
+        # making a new transaction invokes ATMP which expires old txns
+        node.sendtoaddress(node.getnewaddress(), 1)
+
+        # confirm txn is no longer in mempool
+        self.log.info("Confirm transaction is no longer in mempool")
+        assert txhsh not in node.getrawmempool()
+
+        # bumptime so ResendWalletTransactions triggers
+        # we resend once / day, so bump 25 hours just to be sure
+        # we don't resubmit the first time, so we bump mocktime
+        # twice so the resend occurs the second time around
+        one_day_in_seconds = 60 * 60 * 25
+        node.setmocktime(mocktime + one_day_in_seconds)
 
-        # Wallet rebroadcast is first scheduled 1 sec after startup (see
-        # nNextResend in ResendWalletTransactions()). Sleep for just over a
-        # second to be certain that it has been called before the first
-        # setmocktime call below.
         time.sleep(1.1)
 
-        # Can take a few seconds due to transaction trickling
-        wait_until(lambda: node.p2p.tx_invs_received[txid] >= 1, lock=mininode_lock)
-
-        # Add a second peer since txs aren't rebroadcast to the same peer (see filterInventoryKnown)
-        node.add_p2p_connection(P2PStoreTxInvs())
-
-        self.log.info("Create a block")
-        # Create and submit a block without the transaction.
-        # Transactions are only rebroadcast if there has been a block at least five minutes
-        # after the last time we tried to broadcast. Use mocktime and give an extra minute to be sure.
-        block_time = int(time.time()) + 6 * 60
-        node.setmocktime(block_time)
-        block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockcount() + 1), block_time)
-        block.rehash()
-        block.solve()
-        node.submitblock(ToHex(block))
-
-        # Transaction should not be rebroadcast
-        node.p2ps[1].sync_with_ping()
-        assert_equal(node.p2ps[1].tx_invs_received[txid], 0)
-
-        self.log.info("Transaction should be rebroadcast after 30 minutes")
-        # Use mocktime and give an extra 5 minutes to be sure.
-        rebroadcast_time = int(time.time()) + 41 * 60
-        node.setmocktime(rebroadcast_time)
-        wait_until(lambda: node.p2ps[1].tx_invs_received[txid] >= 1, lock=mininode_lock)
+        node.setmocktime(mocktime + 2 * one_day_in_seconds)
+
+        # confirm that its back in the mempool
+        self.log.info("Transaction should be resubmitted to mempool")
+        wait_until(lambda: txhsh in node.getrawmempool(), timeout=30)
+
 
 if __name__ == '__main__':
     ResendWalletTransactionsTest().main()