Merge #18593: test: complete impl. of msg_merkleblock and wait_for_merkleblock

8543828 refactor: test: improve wait_for{header,merkleblock} interface (Sebastian Falbesoner)
1356a45 test: complete impl. of msg_merkleblock and wait_for_merkleblock (Sebastian Falbesoner)

Pull request description:

  Implements the missing initialization/serialization methods for `msg_merkleblock`, based on the already present class `CMerkleBlock`. Also changes the method `wait_for_merkleblock()` to be more precise by waiting for a merkleblock with a specified blockhash instead of an arbitrary one.

  In the BIP37 test `p2p_filter.py`, this new method is used to make the test of receiving merkleblock and tx if a filter is set to be more precise, by checking if they also arrive in the right order.

  In the course of this PR, also the interface for the methods `wait_for_merkleblock()` and `wait_for_header()` are improved to take a hex string instead of an integer, which is more typesafe and less of a burden to the caller.

ACKs for top commit:
  MarcoFalke:
    ACK 8543828

Tree-SHA512: adaf0ac728ef0b9929cb417a7a7b4c1346c400b2d365bf6914515c67b6cfe8f4a7ecc62fb514afdce9792f0bed833416f6bca6b9620f3d5dcdc66e4d5b0b7ea3

Author: MarcoFalke

test/functional/p2p_filter.py

@@ -14,10 +14,7 @@
     msg_filteradd,
     msg_filterclear,
 )
-from test_framework.mininode import (
-    P2PInterface,
-    mininode_lock,
-)
+from test_framework.mininode import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 
 
@@ -69,18 +66,15 @@ def run_test(self):
         filter_address = self.nodes[0].decodescript(filter_node.watch_script_pubkey)['addresses'][0]
 
         self.log.info('Check that we receive merkleblock and tx if the filter matches a tx in a block')
-        filter_node.merkleblock_received = False
         block_hash = self.nodes[0].generatetoaddress(1, filter_address)[0]
         txid = self.nodes[0].getblock(block_hash)['tx'][0]
+        filter_node.wait_for_merkleblock(block_hash)
         filter_node.wait_for_tx(txid)
-        assert filter_node.merkleblock_received
 
         self.log.info('Check that we only receive a merkleblock if the filter does not match a tx in a block')
-        with mininode_lock:
-            filter_node.last_message.pop("merkleblock", None)
         filter_node.tx_received = False
-        self.nodes[0].generatetoaddress(1, self.nodes[0].getnewaddress())
-        filter_node.wait_for_merkleblock()
+        block_hash = self.nodes[0].generatetoaddress(1, self.nodes[0].getnewaddress())[0]
+        filter_node.wait_for_merkleblock(block_hash)
         assert not filter_node.tx_received
 
         self.log.info('Check that we not receive a tx if the filter does not match a mempool tx')

test/functional/p2p_invalid_locator.py

@@ -34,7 +34,7 @@ def run_test(self):
             msg.locator.vHave = [int(node.getblockhash(i - 1), 16) for i in range(block_count, block_count - (MAX_LOCATOR_SZ), -1)]
             node.p2p.send_message(msg)
             if type(msg) == msg_getheaders:
-                node.p2p.wait_for_header(int(node.getbestblockhash(), 16))
+                node.p2p.wait_for_header(node.getbestblockhash())
             else:
                 node.p2p.wait_for_block(int(node.getbestblockhash(), 16))
 

test/functional/test_framework/messages.py

@@ -1321,10 +1321,23 @@ def __repr__(self):
 
 
 class msg_merkleblock:
+    __slots__ = ("merkleblock",)
     command = b"merkleblock"
 
+    def __init__(self, merkleblock=None):
+        if merkleblock is None:
+            self.merkleblock = CMerkleBlock()
+        else:
+            self.merkleblock = merkleblock
+
     def deserialize(self, f):
-        pass  # Placeholder for now
+        self.merkleblock.deserialize(f)
+
+    def serialize(self):
+        return self.merkleblock.serialize()
+
+    def __repr__(self):
+        return "msg_merkleblock(merkleblock=%s)" % (repr(self.merkleblock))
 
 
 class msg_filterload:

test/functional/test_framework/mininode.py

@@ -393,18 +393,17 @@ def test_function():
             last_headers = self.last_message.get('headers')
             if not last_headers:
                 return False
-            return last_headers.headers[0].rehash() == blockhash
+            return last_headers.headers[0].rehash() == int(blockhash, 16)
 
         wait_until(test_function, timeout=timeout, lock=mininode_lock)
 
-    def wait_for_merkleblock(self, timeout=60):
+    def wait_for_merkleblock(self, blockhash, timeout=60):
         def test_function():
             assert self.is_connected
             last_filtered_block = self.last_message.get('merkleblock')
             if not last_filtered_block:
                 return False
-            # TODO change this method to take a hash value and only return true if the correct block has been received
-            return True
+            return last_filtered_block.merkleblock.header.rehash() == int(blockhash, 16)
 
         wait_until(test_function, timeout=timeout, lock=mininode_lock)