From facd97ae0f0d816107aa3bc9de321244200636a0 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 22 Jun 2021 18:28:18 +0200 Subject: [PATCH 1/4] scripted-diff: Renames -BEGIN VERIFY SCRIPT- ren() { sed -i "s/\<$1\>/$2/g" test/functional/p2p_blockfilters.py ; } # Rename from "node" to "peer" to avoid confusion with self.nodes ren node0 peer_0 ren node1 peer_1 # Remove the confusing "C" prefix ren CFiltersClient FiltersClient -END VERIFY SCRIPT- --- test/functional/p2p_blockfilters.py | 54 ++++++++++++++--------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 03662babeff..e8c27c8e660 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -24,7 +24,7 @@ from test_framework.util import ( assert_equal, ) -class CFiltersClient(P2PInterface): +class FiltersClient(P2PInterface): def __init__(self): super().__init__() # Store the cfilters received. @@ -51,8 +51,8 @@ class CompactFiltersTest(BitcoinTestFramework): def run_test(self): # Node 0 supports COMPACT_FILTERS, node 1 does not. - node0 = self.nodes[0].add_p2p_connection(CFiltersClient()) - node1 = self.nodes[1].add_p2p_connection(CFiltersClient()) + peer_0 = self.nodes[0].add_p2p_connection(FiltersClient()) + peer_1 = self.nodes[1].add_p2p_connection(FiltersClient()) # Nodes 0 & 1 share the same first 999 blocks in the chain. self.nodes[0].generate(999) @@ -69,8 +69,8 @@ class CompactFiltersTest(BitcoinTestFramework): self.wait_until(lambda: self.nodes[1].getblockcount() == 2000) # Check that nodes have signalled NODE_COMPACT_FILTERS correctly. - assert node0.nServices & NODE_COMPACT_FILTERS != 0 - assert node1.nServices & NODE_COMPACT_FILTERS == 0 + assert peer_0.nServices & NODE_COMPACT_FILTERS != 0 + assert peer_1.nServices & NODE_COMPACT_FILTERS == 0 # Check that the localservices is as expected. assert int(self.nodes[0].getnetworkinfo()['localservices'], 16) & NODE_COMPACT_FILTERS != 0 @@ -81,8 +81,8 @@ class CompactFiltersTest(BitcoinTestFramework): filter_type=FILTER_TYPE_BASIC, stop_hash=int(stale_block_hash, 16) ) - node0.send_and_ping(message=request) - response = node0.last_message['cfcheckpt'] + peer_0.send_and_ping(message=request) + response = peer_0.last_message['cfcheckpt'] assert_equal(response.filter_type, request.filter_type) assert_equal(response.stop_hash, request.stop_hash) assert_equal(len(response.headers), 1) @@ -100,8 +100,8 @@ class CompactFiltersTest(BitcoinTestFramework): filter_type=FILTER_TYPE_BASIC, stop_hash=int(tip_hash, 16) ) - node0.send_and_ping(request) - response = node0.last_message['cfcheckpt'] + peer_0.send_and_ping(request) + response = peer_0.last_message['cfcheckpt'] assert_equal(response.filter_type, request.filter_type) assert_equal(response.stop_hash, request.stop_hash) @@ -117,8 +117,8 @@ class CompactFiltersTest(BitcoinTestFramework): filter_type=FILTER_TYPE_BASIC, stop_hash=int(stale_block_hash, 16) ) - node0.send_and_ping(request) - response = node0.last_message['cfcheckpt'] + peer_0.send_and_ping(request) + response = peer_0.last_message['cfcheckpt'] stale_cfcheckpt = self.nodes[0].getblockfilter(stale_block_hash, 'basic')['header'] assert_equal( @@ -132,8 +132,8 @@ class CompactFiltersTest(BitcoinTestFramework): start_height=1, stop_hash=int(main_block_hash, 16) ) - node0.send_and_ping(request) - response = node0.last_message['cfheaders'] + peer_0.send_and_ping(request) + response = peer_0.last_message['cfheaders'] main_cfhashes = response.hashes assert_equal(len(main_cfhashes), 1000) assert_equal( @@ -147,8 +147,8 @@ class CompactFiltersTest(BitcoinTestFramework): start_height=1, stop_hash=int(stale_block_hash, 16) ) - node0.send_and_ping(request) - response = node0.last_message['cfheaders'] + peer_0.send_and_ping(request) + response = peer_0.last_message['cfheaders'] stale_cfhashes = response.hashes assert_equal(len(stale_cfhashes), 1000) assert_equal( @@ -163,9 +163,9 @@ class CompactFiltersTest(BitcoinTestFramework): start_height=1, stop_hash=int(stop_hash, 16) ) - node0.send_message(request) - node0.sync_with_ping() - response = node0.pop_cfilters() + peer_0.send_message(request) + peer_0.sync_with_ping() + response = peer_0.pop_cfilters() assert_equal(len(response), 10) self.log.info("Check that cfilter responses are correct.") @@ -182,9 +182,9 @@ class CompactFiltersTest(BitcoinTestFramework): start_height=1000, stop_hash=int(stale_block_hash, 16) ) - node0.send_message(request) - node0.sync_with_ping() - response = node0.pop_cfilters() + peer_0.send_message(request) + peer_0.sync_with_ping() + response = peer_0.pop_cfilters() assert_equal(len(response), 1) cfilter = response[0] @@ -211,9 +211,9 @@ class CompactFiltersTest(BitcoinTestFramework): ), ] for request in requests: - node1 = self.nodes[1].add_p2p_connection(P2PInterface()) - node1.send_message(request) - node1.wait_for_disconnect() + peer_1 = self.nodes[1].add_p2p_connection(P2PInterface()) + peer_1.send_message(request) + peer_1.wait_for_disconnect() self.log.info("Check that invalid requests result in disconnection.") requests = [ @@ -241,9 +241,9 @@ class CompactFiltersTest(BitcoinTestFramework): ), ] for request in requests: - node0 = self.nodes[0].add_p2p_connection(P2PInterface()) - node0.send_message(request) - node0.wait_for_disconnect() + peer_0 = self.nodes[0].add_p2p_connection(P2PInterface()) + peer_0.send_message(request) + peer_0.wait_for_disconnect() def compute_last_header(prev_header, hashes): """Compute the last filter header from a starting header and a sequence of filter hashes.""" From fa1668bf5084a190b26b022b9e625a7be3defa6e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 22 Jun 2021 18:24:22 +0200 Subject: [PATCH 2/4] test: Run pep-8 Can be reviewed with --word-diff-regex=. --- test/functional/p2p_blockfilters.py | 37 ++++++++++++++++------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index e8c27c8e660..a0bfa4eff61 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -39,6 +39,7 @@ class FiltersClient(P2PInterface): """Store cfilters received in a list.""" self.cfilters.append(message) + class CompactFiltersTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -79,7 +80,7 @@ class CompactFiltersTest(BitcoinTestFramework): self.log.info("get cfcheckpt on chain to be re-orged out.") request = msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, - stop_hash=int(stale_block_hash, 16) + stop_hash=int(stale_block_hash, 16), ) peer_0.send_and_ping(message=request) response = peer_0.last_message['cfcheckpt'] @@ -98,7 +99,7 @@ class CompactFiltersTest(BitcoinTestFramework): tip_hash = self.nodes[0].getbestblockhash() request = msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, - stop_hash=int(tip_hash, 16) + stop_hash=int(tip_hash, 16), ) peer_0.send_and_ping(request) response = peer_0.last_message['cfcheckpt'] @@ -109,13 +110,13 @@ class CompactFiltersTest(BitcoinTestFramework): tip_cfcheckpt = self.nodes[0].getblockfilter(tip_hash, 'basic')['header'] assert_equal( response.headers, - [int(header, 16) for header in (main_cfcheckpt, tip_cfcheckpt)] + [int(header, 16) for header in (main_cfcheckpt, tip_cfcheckpt)], ) self.log.info("Check that peers can fetch cfcheckpt on stale chain.") request = msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, - stop_hash=int(stale_block_hash, 16) + stop_hash=int(stale_block_hash, 16), ) peer_0.send_and_ping(request) response = peer_0.last_message['cfcheckpt'] @@ -123,14 +124,14 @@ class CompactFiltersTest(BitcoinTestFramework): stale_cfcheckpt = self.nodes[0].getblockfilter(stale_block_hash, 'basic')['header'] assert_equal( response.headers, - [int(header, 16) for header in (stale_cfcheckpt,)] + [int(header, 16) for header in (stale_cfcheckpt, )], ) self.log.info("Check that peers can fetch cfheaders on active chain.") request = msg_getcfheaders( filter_type=FILTER_TYPE_BASIC, start_height=1, - stop_hash=int(main_block_hash, 16) + stop_hash=int(main_block_hash, 16), ) peer_0.send_and_ping(request) response = peer_0.last_message['cfheaders'] @@ -138,14 +139,14 @@ class CompactFiltersTest(BitcoinTestFramework): assert_equal(len(main_cfhashes), 1000) assert_equal( compute_last_header(response.prev_header, response.hashes), - int(main_cfcheckpt, 16) + int(main_cfcheckpt, 16), ) self.log.info("Check that peers can fetch cfheaders on stale chain.") request = msg_getcfheaders( filter_type=FILTER_TYPE_BASIC, start_height=1, - stop_hash=int(stale_block_hash, 16) + stop_hash=int(stale_block_hash, 16), ) peer_0.send_and_ping(request) response = peer_0.last_message['cfheaders'] @@ -153,7 +154,7 @@ class CompactFiltersTest(BitcoinTestFramework): assert_equal(len(stale_cfhashes), 1000) assert_equal( compute_last_header(response.prev_header, response.hashes), - int(stale_cfcheckpt, 16) + int(stale_cfcheckpt, 16), ) self.log.info("Check that peers can fetch cfilters.") @@ -161,7 +162,7 @@ class CompactFiltersTest(BitcoinTestFramework): request = msg_getcfilters( filter_type=FILTER_TYPE_BASIC, start_height=1, - stop_hash=int(stop_hash, 16) + stop_hash=int(stop_hash, 16), ) peer_0.send_message(request) peer_0.sync_with_ping() @@ -180,7 +181,7 @@ class CompactFiltersTest(BitcoinTestFramework): request = msg_getcfilters( filter_type=FILTER_TYPE_BASIC, start_height=1000, - stop_hash=int(stale_block_hash, 16) + stop_hash=int(stale_block_hash, 16), ) peer_0.send_message(request) peer_0.sync_with_ping() @@ -197,17 +198,17 @@ class CompactFiltersTest(BitcoinTestFramework): requests = [ msg_getcfcheckpt( filter_type=FILTER_TYPE_BASIC, - stop_hash=int(main_block_hash, 16) + stop_hash=int(main_block_hash, 16), ), msg_getcfheaders( filter_type=FILTER_TYPE_BASIC, start_height=1000, - stop_hash=int(main_block_hash, 16) + stop_hash=int(main_block_hash, 16), ), msg_getcfilters( filter_type=FILTER_TYPE_BASIC, start_height=1000, - stop_hash=int(main_block_hash, 16) + stop_hash=int(main_block_hash, 16), ), ] for request in requests: @@ -221,18 +222,18 @@ class CompactFiltersTest(BitcoinTestFramework): msg_getcfilters( filter_type=FILTER_TYPE_BASIC, start_height=0, - stop_hash=int(main_block_hash, 16) + stop_hash=int(main_block_hash, 16), ), # Requesting too many filter headers results in disconnection. msg_getcfheaders( filter_type=FILTER_TYPE_BASIC, start_height=0, - stop_hash=int(tip_hash, 16) + stop_hash=int(tip_hash, 16), ), # Requesting unknown filter type results in disconnection. msg_getcfcheckpt( filter_type=255, - stop_hash=int(main_block_hash, 16) + stop_hash=int(main_block_hash, 16), ), # Requesting unknown hash results in disconnection. msg_getcfcheckpt( @@ -245,6 +246,7 @@ class CompactFiltersTest(BitcoinTestFramework): peer_0.send_message(request) peer_0.wait_for_disconnect() + def compute_last_header(prev_header, hashes): """Compute the last filter header from a starting header and a sequence of filter hashes.""" header = ser_uint256(prev_header) @@ -252,5 +254,6 @@ def compute_last_header(prev_header, hashes): header = hash256(ser_uint256(filter_hash) + header) return uint256_from_str(header) + if __name__ == '__main__': CompactFiltersTest().main() From faa211fc6e3d4984b8edff1d762dd4cba205d982 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 22 Jun 2021 19:41:07 +0200 Subject: [PATCH 3/4] test: Misc cleanup * Replace wait_until with assert_equal where possible * Use send_and_ping helper where possible --- test/functional/p2p_blockfilters.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index a0bfa4eff61..47ee7add757 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -62,12 +62,11 @@ class CompactFiltersTest(BitcoinTestFramework): # Stale blocks by disconnecting nodes 0 & 1, mining, then reconnecting self.disconnect_nodes(0, 1) - self.nodes[0].generate(1) - self.wait_until(lambda: self.nodes[0].getblockcount() == 1000) - stale_block_hash = self.nodes[0].getblockhash(1000) + stale_block_hash = self.nodes[0].generate(1)[0] + assert_equal(self.nodes[0].getblockcount(), 1000) self.nodes[1].generate(1001) - self.wait_until(lambda: self.nodes[1].getblockcount() == 2000) + assert_equal(self.nodes[1].getblockcount(), 2000) # Check that nodes have signalled NODE_COMPACT_FILTERS correctly. assert peer_0.nServices & NODE_COMPACT_FILTERS != 0 @@ -164,8 +163,7 @@ class CompactFiltersTest(BitcoinTestFramework): start_height=1, stop_hash=int(stop_hash, 16), ) - peer_0.send_message(request) - peer_0.sync_with_ping() + peer_0.send_and_ping(request) response = peer_0.pop_cfilters() assert_equal(len(response), 10) @@ -183,8 +181,7 @@ class CompactFiltersTest(BitcoinTestFramework): start_height=1000, stop_hash=int(stale_block_hash, 16), ) - peer_0.send_message(request) - peer_0.sync_with_ping() + peer_0.send_and_ping(request) response = peer_0.pop_cfilters() assert_equal(len(response), 1) From fadddd13eef4428f5fa7237583d4be41a9335cd9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 22 Jun 2021 19:43:02 +0200 Subject: [PATCH 4/4] test: Add missing syncwithvalidationinterfacequeue --- test/functional/p2p_blockfilters.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 47ee7add757..63fc2a98d4f 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -63,6 +63,7 @@ class CompactFiltersTest(BitcoinTestFramework): self.disconnect_nodes(0, 1) stale_block_hash = self.nodes[0].generate(1)[0] + self.nodes[0].syncwithvalidationinterfacequeue() assert_equal(self.nodes[0].getblockcount(), 1000) self.nodes[1].generate(1001) @@ -90,6 +91,7 @@ class CompactFiltersTest(BitcoinTestFramework): self.log.info("Reorg node 0 to a new chain.") self.connect_nodes(0, 1) self.sync_blocks(timeout=600) + self.nodes[0].syncwithvalidationinterfacequeue() main_block_hash = self.nodes[0].getblockhash(1000) assert main_block_hash != stale_block_hash, "node 0 chain did not reorganize"