Merge #13192: [tests] Fixed intermittent failure in p2p_sendheaders.py.

12d1b77 [tests] Fixed intermittent failure in p2p_sendheaders.py. (lmanners)

Pull request description:

  Added handling for the case where headers are announced over more than one message.
  refs #12453

Tree-SHA512: 2c5b48ff019089b86e358181ba170d3aac09d4ae41ec79c2718e0ee83705860501bbcb8fd94d0f5c4f86c0d54a96781a967716621bb8c5ecc991b39af3cec506

Author: MarcoFalke

test/functional/p2p_sendheaders.py

@@ -116,6 +116,7 @@ def __init__(self):
 
         self.block_announced = False
         self.last_blockhash_announced = None
+        self.recent_headers_announced = []
 
     def send_get_data(self, block_hashes):
         """Request data for a list of block hashes."""
@@ -163,40 +164,45 @@ def on_inv(self, message):
     def on_headers(self, message):
         if len(message.headers):
             self.block_announced = True
-            message.headers[-1].calc_sha256()
+            for x in message.headers:
+                x.calc_sha256()
+                # append because headers may be announced over multiple messages.
+                self.recent_headers_announced.append(x.sha256)
             self.last_blockhash_announced = message.headers[-1].sha256
 
-    def clear_last_announcement(self):
+    def clear_block_announcements(self):
         with mininode_lock:
             self.block_announced = False
             self.last_message.pop("inv", None)
             self.last_message.pop("headers", None)
+            self.recent_headers_announced = []
 
-    def check_last_announcement(self, headers=None, inv=None):
-        """Test whether the last announcement received had the right header or the right inv.
 
-        inv and headers should be lists of block hashes."""
+    def check_last_headers_announcement(self, headers):
+        """Test whether the last headers announcements received are right.
+           Headers may be announced across more than one message."""
+        test_function = lambda: (len(self.recent_headers_announced) >= len(headers))
+        wait_until(test_function, timeout=60, lock=mininode_lock)
+        with mininode_lock:
+            assert_equal(self.recent_headers_announced, headers)
+            self.block_announced = False
+            self.last_message.pop("headers", None)
+            self.recent_headers_announced = []
+
+    def check_last_inv_announcement(self, inv):
+        """Test whether the last announcement received had the right inv.
+        inv should be a list of block hashes."""
 
         test_function = lambda: self.block_announced
         wait_until(test_function, timeout=60, lock=mininode_lock)
 
         with mininode_lock:
-            self.block_announced = False
-
             compare_inv = []
             if "inv" in self.last_message:
                 compare_inv = [x.hash for x in self.last_message["inv"].inv]
-            if inv is not None:
-                assert_equal(compare_inv, inv)
-
-            compare_headers = []
-            if "headers" in self.last_message:
-                compare_headers = [x.sha256 for x in self.last_message["headers"].headers]
-            if headers is not None:
-                assert_equal(compare_headers, headers)
-
+            assert_equal(compare_inv, inv)
+            self.block_announced = False
             self.last_message.pop("inv", None)
-            self.last_message.pop("headers", None)
 
 class SendHeadersTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -206,8 +212,8 @@ def set_test_params(self):
     def mine_blocks(self, count):
         """Mine count blocks and return the new tip."""
 
-        # Clear out last block announcement from each p2p listener
-        [x.clear_last_announcement() for x in self.nodes[0].p2ps]
+        # Clear out block announcements from each p2p listener
+        [x.clear_block_announcements() for x in self.nodes[0].p2ps]
         self.nodes[0].generate(count)
         return int(self.nodes[0].getbestblockhash(), 16)
 
@@ -222,7 +228,7 @@ def mine_reorg(self, length):
         sync_blocks(self.nodes, wait=0.1)
         for x in self.nodes[0].p2ps:
             x.wait_for_block_announcement(int(self.nodes[0].getbestblockhash(), 16))
-            x.clear_last_announcement()
+            x.clear_block_announcements()
 
         tip_height = self.nodes[1].getblockcount()
         hash_to_invalidate = self.nodes[1].getblockhash(tip_height - (length - 1))
@@ -255,25 +261,25 @@ def test_null_locators(self, test_node, inv_node):
         tip = self.nodes[0].getblockheader(self.nodes[0].generate(1)[0])
         tip_hash = int(tip["hash"], 16)
 
-        inv_node.check_last_announcement(inv=[tip_hash], headers=[])
-        test_node.check_last_announcement(inv=[tip_hash], headers=[])
+        inv_node.check_last_inv_announcement(inv=[tip_hash])
+        test_node.check_last_inv_announcement(inv=[tip_hash])
 
         self.log.info("Verify getheaders with null locator and valid hashstop returns headers.")
-        test_node.clear_last_announcement()
+        test_node.clear_block_announcements()
         test_node.send_get_headers(locator=[], hashstop=tip_hash)
-        test_node.check_last_announcement(headers=[tip_hash])
+        test_node.check_last_headers_announcement(headers=[tip_hash])
 
         self.log.info("Verify getheaders with null locator and invalid hashstop does not return headers.")
         block = create_block(int(tip["hash"], 16), create_coinbase(tip["height"] + 1), tip["mediantime"] + 1)
         block.solve()
         test_node.send_header_for_blocks([block])
-        test_node.clear_last_announcement()
+        test_node.clear_block_announcements()
         test_node.send_get_headers(locator=[], hashstop=int(block.hash, 16))
         test_node.sync_with_ping()
         assert_equal(test_node.block_announced, False)
-        inv_node.clear_last_announcement()
+        inv_node.clear_block_announcements()
         test_node.send_message(msg_block(block))
-        inv_node.check_last_announcement(inv=[int(block.hash, 16)], headers=[])
+        inv_node.check_last_inv_announcement(inv=[int(block.hash, 16)])
 
     def test_nonnull_locators(self, test_node, inv_node):
         tip = int(self.nodes[0].getbestblockhash(), 16)
@@ -284,8 +290,8 @@ def test_nonnull_locators(self, test_node, inv_node):
         for i in range(4):
             old_tip = tip
             tip = self.mine_blocks(1)
-            inv_node.check_last_announcement(inv=[tip], headers=[])
-            test_node.check_last_announcement(inv=[tip], headers=[])
+            inv_node.check_last_inv_announcement(inv=[tip])
+            test_node.check_last_inv_announcement(inv=[tip])
             # Try a few different responses; none should affect next announcement
             if i == 0:
                 # first request the block
@@ -296,7 +302,7 @@ def test_nonnull_locators(self, test_node, inv_node):
                 test_node.send_get_headers(locator=[old_tip], hashstop=tip)
                 test_node.send_get_data([tip])
                 test_node.wait_for_block(tip)
-                test_node.clear_last_announcement()  # since we requested headers...
+                test_node.clear_block_announcements()  # since we requested headers...
             elif i == 2:
                 # this time announce own block via headers
                 height = self.nodes[0].getblockcount()
@@ -308,8 +314,8 @@ def test_nonnull_locators(self, test_node, inv_node):
                 test_node.wait_for_getdata([new_block.sha256])
                 test_node.send_message(msg_block(new_block))
                 test_node.sync_with_ping()  # make sure this block is processed
-                inv_node.clear_last_announcement()
-                test_node.clear_last_announcement()
+                inv_node.clear_block_announcements()
+                test_node.clear_block_announcements()
 
         self.log.info("Part 1: success!")
         self.log.info("Part 2: announce blocks with headers after sendheaders message...")
@@ -323,8 +329,8 @@ def test_nonnull_locators(self, test_node, inv_node):
 
         # Now that we've synced headers, headers announcements should work
         tip = self.mine_blocks(1)
-        inv_node.check_last_announcement(inv=[tip], headers=[])
-        test_node.check_last_announcement(headers=[tip])
+        inv_node.check_last_inv_announcement(inv=[tip])
+        test_node.check_last_headers_announcement(headers=[tip])
 
         height = self.nodes[0].getblockcount() + 1
         block_time += 10  # Advance far enough ahead
@@ -368,8 +374,8 @@ def test_nonnull_locators(self, test_node, inv_node):
                 assert "inv" not in inv_node.last_message
                 assert "headers" not in inv_node.last_message
                 tip = self.mine_blocks(1)
-                inv_node.check_last_announcement(inv=[tip], headers=[])
-                test_node.check_last_announcement(headers=[tip])
+                inv_node.check_last_inv_announcement(inv=[tip])
+                test_node.check_last_headers_announcement(headers=[tip])
                 height += 1
                 block_time += 1
 
@@ -383,16 +389,16 @@ def test_nonnull_locators(self, test_node, inv_node):
             # First try mining a reorg that can propagate with header announcement
             new_block_hashes = self.mine_reorg(length=7)
             tip = new_block_hashes[-1]
-            inv_node.check_last_announcement(inv=[tip], headers=[])
-            test_node.check_last_announcement(headers=new_block_hashes)
+            inv_node.check_last_inv_announcement(inv=[tip])
+            test_node.check_last_headers_announcement(headers=new_block_hashes)
 
             block_time += 8
 
             # Mine a too-large reorg, which should be announced with a single inv
             new_block_hashes = self.mine_reorg(length=8)
             tip = new_block_hashes[-1]
-            inv_node.check_last_announcement(inv=[tip], headers=[])
-            test_node.check_last_announcement(inv=[tip], headers=[])
+            inv_node.check_last_inv_announcement(inv=[tip])
+            test_node.check_last_inv_announcement(inv=[tip])
 
             block_time += 9
 
@@ -401,15 +407,15 @@ def test_nonnull_locators(self, test_node, inv_node):
 
             # Use getblocks/getdata
             test_node.send_getblocks(locator=[fork_point])
-            test_node.check_last_announcement(inv=new_block_hashes, headers=[])
+            test_node.check_last_inv_announcement(inv=new_block_hashes)
             test_node.send_get_data(new_block_hashes)
             test_node.wait_for_block(new_block_hashes[-1])
 
             for i in range(3):
                 # Mine another block, still should get only an inv
                 tip = self.mine_blocks(1)
-                inv_node.check_last_announcement(inv=[tip], headers=[])
-                test_node.check_last_announcement(inv=[tip], headers=[])
+                inv_node.check_last_inv_announcement(inv=[tip])
+                test_node.check_last_inv_announcement(inv=[tip])
                 if i == 0:
                     self.log.debug("Just get the data -- shouldn't cause headers announcements to resume")
                     test_node.send_get_data([tip])
@@ -431,8 +437,8 @@ def test_nonnull_locators(self, test_node, inv_node):
                         test_node.sync_with_ping()
             # New blocks should now be announced with header
             tip = self.mine_blocks(1)
-            inv_node.check_last_announcement(inv=[tip], headers=[])
-            test_node.check_last_announcement(headers=[tip])
+            inv_node.check_last_inv_announcement(inv=[tip])
+            test_node.check_last_headers_announcement(headers=[tip])
 
         self.log.info("Part 3: success!")