Modify `join_network` in tests to skip mempool resync.

Prior to this change, calling `join_network` after a network split
only worked in the case that no new non-coinbase transactions were
created during the network partition; in the case that such a
transaction was created, `join_network` would fail when waiting
for mempool synchronization, because zcashd nodes do not.
automatically broadcast their mempool contents on restart.

This change modifies `setup_network` to wait for mempool synchronization
or not on the basis of a newly added do_mempool_sync flag. In the
case of `join_network`, this flag is set to `False`; the default value
is set to `True` to preserve previous functionality elsewhere.

Tests should explicitly use the `resendwallettransactions` RPC method
to ensure that mempool transactions are rebroadcast before attempting
to synchronize the network after a `join_network` call.

qa/rpc-tests/orchard_reorg.py

@@ -14,14 +14,10 @@ from test_framework.util import (
     CANOPY_BRANCH_ID,
     NU5_BRANCH_ID,
     assert_equal,
-    connect_nodes_bi,
     get_coinbase_address,
     nuparams,
     start_nodes,
-    stop_nodes,
-    sync_blocks,
     wait_and_assert_operationid_status,
-    wait_bitcoinds,
 )
 
 from finalsaplingroot import ORCHARD_TREE_EMPTY_ROOT
@@ -122,19 +118,7 @@ class OrchardReorgTest(BitcoinTestFramework):
 
         # Reconnect the nodes; node 0 will re-org to node 2's chain.
         print("Re-joining the network so that node 0 reorgs")
-        # We can't use `self.join_network()` because the coinbase-spending second Orchard
-        # transaction doesn't propagate from node 1's mempool to node 2 on restart. Inline
-        # the block-syncing parts here.
-        assert self.is_network_split
-        stop_nodes(self.nodes)
-        wait_bitcoinds()
-        self.nodes = self.setup_nodes()
-        connect_nodes_bi(self.nodes, 1, 2)
-        sync_blocks(self.nodes[1:3])
-        connect_nodes_bi(self.nodes, 0, 1)
-        connect_nodes_bi(self.nodes, 2, 3)
-        self.is_network_split = False
-        sync_blocks(self.nodes)
+        self.join_network()
 
         # Verify that node 0's latest Orchard root matches what we expect.
         orchardroot_postreorg = self.nodes[0].getblock(self.nodes[2].getbestblockhash())['finalorchardroot']

qa/rpc-tests/test_framework/test_framework.py

@@ -52,7 +52,7 @@ class BitcoinTestFramework(object):
     def setup_nodes(self):
         return start_nodes(self.num_nodes, self.options.tmpdir)
 
-    def setup_network(self, split = False):
+    def setup_network(self, split = False, do_mempool_sync = True):
         self.nodes = self.setup_nodes()
 
         # Connect the nodes as a "chain".  This allows us
@@ -64,12 +64,13 @@ class BitcoinTestFramework(object):
         if not split:
             connect_nodes_bi(self.nodes, 1, 2)
             sync_blocks(self.nodes[1:3])
-            sync_mempools(self.nodes[1:3])
+            if do_mempool_sync:
+                sync_mempools(self.nodes[1:3])
 
         connect_nodes_bi(self.nodes, 0, 1)
         connect_nodes_bi(self.nodes, 2, 3)
         self.is_network_split = split
-        self.sync_all()
+        self.sync_all(do_mempool_sync)
 
     def split_network(self):
         """
@@ -80,15 +81,17 @@ class BitcoinTestFramework(object):
         wait_bitcoinds()
         self.setup_network(True)
 
-    def sync_all(self):
+    def sync_all(self, do_mempool_sync = True):
         if self.is_network_split:
             sync_blocks(self.nodes[:2])
             sync_blocks(self.nodes[2:])
-            sync_mempools(self.nodes[:2])
-            sync_mempools(self.nodes[2:])
+            if do_mempool_sync:
+                sync_mempools(self.nodes[:2])
+                sync_mempools(self.nodes[2:])
         else:
             sync_blocks(self.nodes)
-            sync_mempools(self.nodes)
+            if do_mempool_sync:
+                sync_mempools(self.nodes)
 
     def join_network(self):
         """
@@ -97,7 +100,7 @@ class BitcoinTestFramework(object):
         assert self.is_network_split
         stop_nodes(self.nodes)
         wait_bitcoinds()
-        self.setup_network(False)
+        self.setup_network(False, False)
 
     def main(self):
 

qa/rpc-tests/wallet_orchard.py

@@ -7,14 +7,10 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     NU5_BRANCH_ID,
     assert_equal,
-    connect_nodes_bi,
     get_coinbase_address,
     nuparams,
     start_nodes,
-    stop_nodes,
-    sync_blocks,
     wait_and_assert_operationid_status,
-    wait_bitcoinds,
 )
 
 from decimal import Decimal
@@ -130,17 +126,7 @@ class WalletOrchardTest(BitcoinTestFramework):
 
         # Reconnect the nodes; nodes 2 and 3 will re-org to node 0's chain.
         print("Re-joining the network so that nodes 2 and 3 reorg")
-        # We can't use `self.join_network()` because the the nodes's mempools fail to synchronize on restart
-        assert self.is_network_split
-        stop_nodes(self.nodes)
-        wait_bitcoinds()
-        self.nodes = self.setup_nodes()
-        connect_nodes_bi(self.nodes, 1, 2)
-        sync_blocks(self.nodes[1:3])
-        connect_nodes_bi(self.nodes, 0, 1)
-        connect_nodes_bi(self.nodes, 2, 3)
-        self.is_network_split = False
-        sync_blocks(self.nodes)
+        self.join_network()
 
         # split 0/1's chain should have won, so their wallet balance should be consistent
         assert_equal(
@@ -151,25 +137,43 @@ class WalletOrchardTest(BitcoinTestFramework):
         # un-mined and returned to the mempool
         assert_equal(set([rollback_tx]), set(self.nodes[2].getrawmempool()))
 
-        # our sole Sapling note is spent, so our confirmed balance is currently 0
+        # acct2's sole Sapling note is spent by a transaction in the mempool, so our 
+        # confirmed balance is currently 0
         assert_equal(
                 {'pools': {}, 'minimum_confirmations': 1}, 
                 self.nodes[2].z_getbalanceforaccount(acct2))
 
-        # our incoming change (unconfirmed, still in the mempool) is 9 zec
+        # acct2's incoming change (unconfirmed, still in the mempool) is 9 zec
         assert_equal(
                 {'pools': {'sapling': {'valueZat': Decimal('900000000')}}, 'minimum_confirmations': 0}, 
                 self.nodes[2].z_getbalanceforaccount(acct2, 0))
 
-        # the transaction was un-mined, so we have no confirmed balance
+        # The transaction was un-mined, so acct3 should have no confirmed balance
         assert_equal(
                 {'pools': {}, 'minimum_confirmations': 1}, 
                 self.nodes[3].z_getbalanceforaccount(acct3))
 
-        # our unconfirmed balance is 1 zec
+        # acct3's unconfirmed balance is 1 zec
         assert_equal(
                 {'pools': {'orchard': {'valueZat': Decimal('100000000')}}, 'minimum_confirmations': 0}, 
                 self.nodes[3].z_getbalanceforaccount(acct3, 0))
 
+        # Manually resend the transaction in node 2's mempool
+        self.nodes[2].resendwallettransactions()
+
+        # Sync the network
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        # The un-mined transaction should now have been re-mined
+        assert_equal(
+                {'pools': {'sapling': {'valueZat': Decimal('900000000')}}, 'minimum_confirmations': 1}, 
+                self.nodes[2].z_getbalanceforaccount(acct2))
+
+        assert_equal(
+                {'pools': {'orchard': {'valueZat': Decimal('100000000')}}, 'minimum_confirmations': 1}, 
+                self.nodes[3].z_getbalanceforaccount(acct3))
+
 if __name__ == '__main__':
     WalletOrchardTest().main()