Merge #10114: [tests] sync_with_ping should assert that ping hasn't timed out

6a18bb9 [tests] sync_with_ping should assert that ping hasn't timed out (John Newbery)
6426716 Add send_await_disconnect() method to p2p-compactblocks.py (John Newbery)

Tree-SHA512: d0eeb066ed3f6aef594caaa693280d10abedac9b719e795c9659e52ddae315085eb2845131644793007e30a16e3e0bf3e3925b40ca2ae023cad923c5ab32df25

8 files changed, 34 insertions, 66 deletions

diff --git a/test/functional/assumevalid.py b/test/functional/assumevalid.py
index a9c2b17b4a..8e301c4379 100755
--- a/test/functional/assumevalid.py
+++ b/test/functional/assumevalid.py
@@ -190,7 +190,8 @@ class AssumeValidTest(BitcoinTestFramework):
         # Send all blocks to node1. All blocks will be accepted.
         for i in range(2202):
             node1.send_message(msg_block(self.blocks[i]))
-        node1.sync_with_ping()  # make sure the most recent block is synced
+        # Syncing 2200 blocks can take a while on slow systems. Give it plenty of time to sync.
+        node1.sync_with_ping(120)
         assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202)
 
         # Send blocks to node2. Block 102 will be rejected.
diff --git a/test/functional/maxuploadtarget.py b/test/functional/maxuploadtarget.py
index b26c10796d..9b42bf276c 100755
--- a/test/functional/maxuploadtarget.py
+++ b/test/functional/maxuploadtarget.py
@@ -68,15 +68,6 @@ class TestNode(NodeConnCB):
     def on_close(self, conn):
         self.peer_disconnected = True
 
-    # Sync up with the node after delivery of a block
-    def sync_with_ping(self, timeout=30):
-        def received_pong():
-            return (self.last_pong.nonce == self.ping_counter)
-        self.connection.send_message(msg_ping(nonce=self.ping_counter))
-        success = wait_until(received_pong, timeout=timeout)
-        self.ping_counter += 1
-        return success
-
 class MaxUploadTest(BitcoinTestFramework):
  
     def __init__(self):
diff --git a/test/functional/p2p-acceptblock.py b/test/functional/p2p-acceptblock.py
index 7bdbe2bb12..c09945baa6 100755
--- a/test/functional/p2p-acceptblock.py
+++ b/test/functional/p2p-acceptblock.py
@@ -88,21 +88,6 @@ class TestNode(NodeConnCB):
     def on_pong(self, conn, message):
         self.last_pong = message
 
-    # Sync up with the node after delivery of a block
-    def sync_with_ping(self, timeout=30):
-        self.connection.send_message(msg_ping(nonce=self.ping_counter))
-        received_pong = False
-        sleep_time = 0.05
-        while not received_pong and timeout > 0:
-            time.sleep(sleep_time)
-            timeout -= sleep_time
-            with mininode_lock:
-                if self.last_pong.nonce == self.ping_counter:
-                    received_pong = True
-        self.ping_counter += 1
-        return received_pong
-
-
 class AcceptBlockTest(BitcoinTestFramework):
     def add_options(self, parser):
         parser.add_option("--testbinary", dest="testbinary",
diff --git a/test/functional/p2p-compactblocks.py b/test/functional/p2p-compactblocks.py
index 86b767b98a..bf8d113767 100755
--- a/test/functional/p2p-compactblocks.py
+++ b/test/functional/p2p-compactblocks.py
@@ -32,6 +32,13 @@ class TestNode(NodeConnCB):
         # This is for synchronizing the p2p message traffic,
         # so we can eg wait until a particular block is announced.
         self.set_announced_blockhashes = set()
+        self.connected = False
+
+    def on_open(self, conn):
+        self.connected = True
+
+    def on_close(self, conn):
+        self.connected = False
 
     def on_sendcmpct(self, conn, message):
         self.last_sendcmpct.append(message)
@@ -107,6 +114,18 @@ class TestNode(NodeConnCB):
             return (block_hash in self.set_announced_blockhashes)
         return wait_until(received_hash, timeout=timeout)
 
+    def send_await_disconnect(self, message, timeout=30):
+        """Sends a message to the node and wait for disconnect.
+
+        This is used when we want to send a message into the node that we expect
+        will get us disconnected, eg an invalid block."""
+        self.send_message(message)
+        success = wait_until(lambda: not self.connected, timeout=timeout)
+        if not success:
+            logger.error("send_await_disconnect failed!")
+            raise AssertionError("send_await_disconnect failed!")
+        return success
+
 class CompactBlocksTest(BitcoinTestFramework):
     def __init__(self):
         super().__init__()
@@ -274,8 +293,8 @@ class CompactBlocksTest(BitcoinTestFramework):
         # This index will be too high
         prefilled_txn = PrefilledTransaction(1, block.vtx[0])
         cmpct_block.prefilled_txn = [prefilled_txn]
-        self.test_node.send_and_ping(msg_cmpctblock(cmpct_block))
-        assert(int(self.nodes[0].getbestblockhash(), 16) == block.hashPrevBlock)
+        self.test_node.send_await_disconnect(msg_cmpctblock(cmpct_block))
+        assert_equal(int(self.nodes[0].getbestblockhash(), 16), block.hashPrevBlock)
 
     # Compare the generated shortids to what we expect based on BIP 152, given
     # bitcoind's choice of nonce.
diff --git a/test/functional/p2p-mempool.py b/test/functional/p2p-mempool.py
index 3d3a9939d4..5064ce74aa 100755
--- a/test/functional/p2p-mempool.py
+++ b/test/functional/p2p-mempool.py
@@ -62,15 +62,6 @@ class TestNode(NodeConnCB):
     def on_close(self, conn):
         self.peer_disconnected = True
 
-    # Sync up with the node after delivery of a block
-    def sync_with_ping(self, timeout=30):
-        def received_pong():
-            return (self.last_pong.nonce == self.ping_counter)
-        self.connection.send_message(msg_ping(nonce=self.ping_counter))
-        success = wait_until(received_pong, timeout=timeout)
-        self.ping_counter += 1
-        return success
-
     def send_mempool(self):
         self.lastInv = []
         self.send_message(msg_mempool())
diff --git a/test/functional/p2p-segwit.py b/test/functional/p2p-segwit.py
index 69f829279c..cd7b788eb4 100755
--- a/test/functional/p2p-segwit.py
+++ b/test/functional/p2p-segwit.py
@@ -80,13 +80,6 @@ class TestNode(NodeConnCB):
             timeout -= self.sleep_time
         raise AssertionError("Sync failed to complete")
         
-    def sync_with_ping(self, timeout=60):
-        self.send_message(msg_ping(nonce=self.ping_counter))
-        test_function = lambda: self.last_pong.nonce == self.ping_counter
-        self.sync(test_function, timeout)
-        self.ping_counter += 1
-        return
-
     def wait_for_block(self, blockhash, timeout=60):
         test_function = lambda: self.last_block != None and self.last_block.sha256 == blockhash
         self.sync(test_function, timeout)
@@ -148,7 +141,7 @@ class TestNode(NodeConnCB):
         if with_witness:
             tx_message = msg_witness_tx(tx)
         self.send_message(tx_message)
-        self.sync_with_ping()
+        self.sync_with_ping(60)
         assert_equal(tx.hash in self.connection.rpc.getrawmempool(), accepted)
         if (reason != None and not accepted):
             # Check the rejection reason as well.
@@ -161,7 +154,7 @@ class TestNode(NodeConnCB):
             self.send_message(msg_witness_block(block))
         else:
             self.send_message(msg_block(block))
-        self.sync_with_ping()
+        self.sync_with_ping(60)
         assert_equal(self.connection.rpc.getbestblockhash() == block.hash, accepted)
 
 
@@ -235,7 +228,7 @@ class SegWitTest(BitcoinTestFramework):
         block = self.build_next_block(nVersion=1)
         block.solve()
         self.test_node.send_message(msg_block(block))
-        self.test_node.sync_with_ping() # make sure the block was processed
+        self.test_node.sync_with_ping(60) # make sure the block was processed
         txid = block.vtx[0].sha256
 
         self.nodes[0].generate(99) # let the block mature
@@ -251,7 +244,7 @@ class SegWitTest(BitcoinTestFramework):
         assert_equal(msg_tx(tx).serialize(), msg_witness_tx(tx).serialize())
 
         self.test_node.send_message(msg_witness_tx(tx))
-        self.test_node.sync_with_ping() # make sure the tx was processed
+        self.test_node.sync_with_ping(60) # make sure the tx was processed
         assert(tx.hash in self.nodes[0].getrawmempool())
         # Save this transaction for later
         self.utxo.append(UTXO(tx.sha256, 0, 49*100000000))
@@ -291,7 +284,7 @@ class SegWitTest(BitcoinTestFramework):
         # But it should not be permanently marked bad...
         # Resend without witness information.
         self.test_node.send_message(msg_block(block))
-        self.test_node.sync_with_ping()
+        self.test_node.sync_with_ping(60)
         assert_equal(self.nodes[0].getbestblockhash(), block.hash)
 
         sync_blocks(self.nodes)
@@ -1257,7 +1250,7 @@ class SegWitTest(BitcoinTestFramework):
         # Spending a higher version witness output is not allowed by policy,
         # even with fRequireStandard=false.
         self.test_node.test_transaction_acceptance(tx3, with_witness=True, accepted=False)
-        self.test_node.sync_with_ping()
+        self.test_node.sync_with_ping(60)
         with mininode_lock:
             assert(b"reserved for soft-fork upgrades" in self.test_node.last_reject.reason)
 
@@ -1387,7 +1380,7 @@ class SegWitTest(BitcoinTestFramework):
         for i in range(NUM_TESTS):
             # Ping regularly to keep the connection alive
             if (not i % 100):
-                self.test_node.sync_with_ping()
+                self.test_node.sync_with_ping(60)
             # Choose random number of inputs to use.
             num_inputs = random.randint(1, 10)
             # Create a slight bias for producing more utxos
diff --git a/test/functional/p2p-versionbits-warning.py b/test/functional/p2p-versionbits-warning.py
index d4d03861a4..da960e2d80 100755
--- a/test/functional/p2p-versionbits-warning.py
+++ b/test/functional/p2p-versionbits-warning.py
@@ -46,21 +46,6 @@ class TestNode(NodeConnCB):
     def on_pong(self, conn, message):
         self.last_pong = message
 
-    # Sync up with the node after delivery of a block
-    def sync_with_ping(self, timeout=30):
-        self.connection.send_message(msg_ping(nonce=self.ping_counter))
-        received_pong = False
-        sleep_time = 0.05
-        while not received_pong and timeout > 0:
-            time.sleep(sleep_time)
-            timeout -= sleep_time
-            with mininode_lock:
-                if self.last_pong.nonce == self.ping_counter:
-                    received_pong = True
-        self.ping_counter += 1
-        return received_pong
-
-
 class VersionBitsWarningTest(BitcoinTestFramework):
     def __init__(self):
         super().__init__()
diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py
index b02ffbe14a..ebb846a237 100755
--- a/test/functional/test_framework/mininode.py
+++ b/test/functional/test_framework/mininode.py
@@ -1563,11 +1563,14 @@ class NodeConnCB(object):
         self.sync_with_ping()
 
     # Sync up with the node
-    def sync_with_ping(self, timeout=30):
+    def sync_with_ping(self, timeout=60):
         def received_pong():
             return (self.last_pong.nonce == self.ping_counter)
         self.send_message(msg_ping(nonce=self.ping_counter))
         success = wait_until(received_pong, timeout=timeout)
+        if not success:
+            logger.error("sync_with_ping failed!")
+            raise AssertionError("sync_with_ping failed!")
         self.ping_counter += 1
 
         return success