[Test] Fix intermittent sync_blocks failures

>>> backports bitcoin/bitcoin@fa3f9a0

test/functional/feature_reindex.py

@@ -10,7 +10,7 @@
 """
 
 from test_framework.test_framework import PivxTestFramework
-from test_framework.util import wait_until
+from test_framework.util import assert_equal
 import time
 
 class ReindexTest(PivxTestFramework):
@@ -23,11 +23,9 @@ def reindex(self):
         self.nodes[0].generate(3)
         blockcount = self.nodes[0].getblockcount()
         self.stop_nodes()
-        time.sleep(5)
         extra_args = [["-reindex", "-checkblockindex=1"]]
         self.start_nodes(extra_args)
-        time.sleep(15)
-        wait_until(lambda: self.nodes[0].getblockcount() == blockcount)
+        assert_equal(self.nodes[0].getblockcount(), blockcount)  # start_node is blocking on reindex
         self.log.info("Success")
 
     def run_test(self):

test/functional/mempool_persist.py

@@ -70,9 +70,8 @@ def run_test(self):
         self.start_node(1, extra_args=["-persistmempool=0"])
         self.start_node(0)
         self.start_node(2)
-
-        wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"], timeout=1)
-        wait_until(lambda: self.nodes[2].getmempoolinfo()["loaded"], timeout=1)
+        assert self.nodes[0].getmempoolinfo()["loaded"]  # start_node is blocking on the mempool being loaded
+        assert self.nodes[2].getmempoolinfo()["loaded"]
         assert_equal(len(self.nodes[0].getrawmempool()), 5)
         assert_equal(len(self.nodes[2].getrawmempool()), 5)
         # The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now:
@@ -85,13 +84,13 @@ def run_test(self):
         self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.")
         self.stop_nodes()
         self.start_node(0, extra_args=["-persistmempool=0"])
-        wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"])
+        assert self.nodes[0].getmempoolinfo()["loaded"]
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
 
         self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.")
         self.stop_nodes()
         self.start_node(0)
-        wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"])
+        assert self.nodes[0].getmempoolinfo()["loaded"]
         assert_equal(len(self.nodes[0].getrawmempool()), 5)
 
         # Following code is ahead of our current repository state. Future back port.
@@ -107,7 +106,7 @@ def run_test(self):
         os.rename(mempooldat0, mempooldat1)
         self.stop_nodes()
         self.start_node(1, extra_args=[])
-        wait_until(lambda: self.nodes[1].getmempoolinfo()["loaded"])
+        assert self.nodes[0].getmempoolinfo()["loaded"]
         assert_equal(len(self.nodes[1].getrawmempool()), 5)
 
         self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails")

test/functional/test_framework/test_node.py

@@ -121,8 +121,25 @@ def wait_for_rpc_connection(self):
             assert self.process.poll() is None, "pivxd exited with status %i during initialization" % self.process.returncode
             try:
                 self.rpc = get_rpc_proxy(rpc_url(self.datadir, self.index, self.rpchost), self.index, timeout=self.rpc_timeout, coveragedir=self.coverage_dir)
-                while self.rpc.getblockcount() < 0:
-                    time.sleep(1)
+                self.rpc.getblockcount()
+                wait_until(lambda: self.rpc.getmempoolinfo()['loaded'])
+                # Wait for the node to finish reindex, block import, and
+                # loading the mempool. Usually importing happens fast or
+                # even "immediate" when the node is started. However, there
+                # is no guarantee and sometimes ThreadImport might finish
+                # later. This is going to cause intermittent test failures,
+                # because generally the tests assume the node is fully
+                # ready after being started.
+                #
+                # For example, the node will reject block messages from p2p
+                # when it is still importing with the error "Unexpected
+                # block message received"
+                #
+                # The wait is done here to make tests as robust as possible
+                # and prevent racy tests and intermittent failures as much
+                # as possible. Some tests might not need this, but the
+                # overhead is trivial, and the added gurantees are worth
+                # the minimal performance cost.
                 # If the call to getblockcount() succeeds then the RPC connection is up
                 self.rpc_connected = True
                 self.url = self.rpc.url