Merge bitcoin/bitcoin#22311: test: Add missing syncwithvalidationinterfacequeue in p2p_blockfilters

fadddd1 test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)
faa211f test: Misc cleanup (MarcoFalke)
fa1668b test: Run pep-8 (MarcoFalke)
facd97a scripted-diff: Renames (MarcoFalke)

Pull request description:

  The index on the block filters is running in the background on the validation interface. To avoid intermittent test failures, it needs to be synced.

  Also other cleanups.

ACKs for top commit:
  lsilva01:
    Tested ACK bitcoin/bitcoin@fadddd1 on Ubuntu 20.04

Tree-SHA512: d858405db426a2f9d5620059dd88bcead4e3fba3ccc6bd8023f624b768fbcfa2203246fb0b2db620490321730d990f0e78063b21a26988c969cb126d4bd697bd

Author: MarcoFalke

test/functional/p2p_blockfilters.py

@@ -24,7 +24,7 @@
     assert_equal,
 )
 
-class CFiltersClient(P2PInterface):
+class FiltersClient(P2PInterface):
     def __init__(self):
         super().__init__()
         # Store the cfilters received.
@@ -39,6 +39,7 @@ def on_cfilter(self, message):
         """Store cfilters received in a list."""
         self.cfilters.append(message)
 
+
 class CompactFiltersTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
@@ -51,8 +52,8 @@ def set_test_params(self):
 
     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)
@@ -61,16 +62,16 @@ def run_test(self):
         # 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]
+        self.nodes[0].syncwithvalidationinterfacequeue()
+        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 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
@@ -79,17 +80,18 @@ def run_test(self):
         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),
         )
-        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)
 
         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"
@@ -98,74 +100,73 @@ def run_test(self):
         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),
         )
-        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)
 
         main_cfcheckpt = self.nodes[0].getblockfilter(main_block_hash, 'basic')['header']
         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),
         )
-        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(
             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),
         )
-        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(
             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),
         )
-        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(
             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.")
         stop_hash = self.nodes[0].getblockhash(10)
         request = msg_getcfilters(
             filter_type=FILTER_TYPE_BASIC,
             start_height=1,
-            stop_hash=int(stop_hash, 16)
+            stop_hash=int(stop_hash, 16),
         )
-        node0.send_message(request)
-        node0.sync_with_ping()
-        response = node0.pop_cfilters()
+        peer_0.send_and_ping(request)
+        response = peer_0.pop_cfilters()
         assert_equal(len(response), 10)
 
         self.log.info("Check that cfilter responses are correct.")
@@ -180,11 +181,10 @@ def run_test(self):
         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),
         )
-        node0.send_message(request)
-        node0.sync_with_ping()
-        response = node0.pop_cfilters()
+        peer_0.send_and_ping(request)
+        response = peer_0.pop_cfilters()
         assert_equal(len(response), 1)
 
         cfilter = response[0]
@@ -197,42 +197,42 @@ def run_test(self):
         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:
-            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 = [
             # Requesting too many filters results in disconnection.
             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(
@@ -241,9 +241,10 @@ def run_test(self):
             ),
         ]
         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."""
@@ -252,5 +253,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()