From 1356a45ef042e7bd3d539fbb606d6b1be547d00f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 10 Apr 2020 16:01:33 +0200 Subject: [PATCH 1/2] test: complete impl. of msg_merkleblock and wait_for_merkleblock 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. --- test/functional/p2p_filter.py | 14 ++++---------- test/functional/test_framework/messages.py | 15 ++++++++++++++- test/functional/test_framework/mininode.py | 5 ++--- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 188b130a570..aa2c92c2771 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -14,10 +14,7 @@ from test_framework.messages import ( 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 @@ class FilterTest(BitcoinTestFramework): 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(int(block_hash, 16)) 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(int(block_hash, 16)) 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') diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 5f8fcc6fd86..45b49bcf9e5 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1321,10 +1321,23 @@ class msg_headers: 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: diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index ad330f2a936..b17254faf82 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -397,14 +397,13 @@ class P2PInterface(P2PConnection): 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() == blockhash wait_until(test_function, timeout=timeout, lock=mininode_lock) From 854382885f18aa9a95cdde3d11591b05c305ad3f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 11 Apr 2020 15:53:01 +0200 Subject: [PATCH 2/2] refactor: test: improve wait_for{header,merkleblock} interface The interfaces for the methods wait_for_header() and wait_for_merkleblock() are changed to take a hex string instead of an integer, improving type safety and removing the burden from the caller to always do the transformation via `int(...)`. As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/18593#discussion_r407062253 --- test/functional/p2p_filter.py | 4 ++-- test/functional/p2p_invalid_locator.py | 2 +- test/functional/test_framework/mininode.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index aa2c92c2771..b73d5784aa1 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -68,13 +68,13 @@ class FilterTest(BitcoinTestFramework): self.log.info('Check that we receive merkleblock and tx if the filter matches a tx in a block') block_hash = self.nodes[0].generatetoaddress(1, filter_address)[0] txid = self.nodes[0].getblock(block_hash)['tx'][0] - filter_node.wait_for_merkleblock(int(block_hash, 16)) + filter_node.wait_for_merkleblock(block_hash) filter_node.wait_for_tx(txid) self.log.info('Check that we only receive a merkleblock if the filter does not match a tx in a block') filter_node.tx_received = False block_hash = self.nodes[0].generatetoaddress(1, self.nodes[0].getnewaddress())[0] - filter_node.wait_for_merkleblock(int(block_hash, 16)) + 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') diff --git a/test/functional/p2p_invalid_locator.py b/test/functional/p2p_invalid_locator.py index 33b7060060d..58be703bf65 100755 --- a/test/functional/p2p_invalid_locator.py +++ b/test/functional/p2p_invalid_locator.py @@ -34,7 +34,7 @@ class InvalidLocatorTest(BitcoinTestFramework): 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)) diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index b17254faf82..beda7eeabad 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -393,7 +393,7 @@ class P2PInterface(P2PConnection): 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) @@ -403,7 +403,7 @@ class P2PInterface(P2PConnection): last_filtered_block = self.last_message.get('merkleblock') if not last_filtered_block: return False - return last_filtered_block.merkleblock.header.rehash() == blockhash + return last_filtered_block.merkleblock.header.rehash() == int(blockhash, 16) wait_until(test_function, timeout=timeout, lock=mininode_lock)