Merge #11791: [tests] Rename NodeConn and NodeConnCB

873beca6d [tests] Rename NodeConn and NodeConnCB (John Newbery)

Pull request description:

  Final step in #11518

  NodeConn -> P2PConnection
  NodeConnCB -> P2PInterface

  This is basically just a rename. Should be an easy review.

Tree-SHA512: fe1204b2b3d8182c5e324ffa7cb4099a47ef8536380e0bb9d37a5fccf76a24f548d1f1a7988ab8f830986a3058b670696de3fc891af5e5f75dbeb4e3273005d7

test/functional/README.md

@@ -63,12 +63,12 @@ wrappers for them, `msg_block`, `msg_tx`, etc).
 with the dashd(s) being tested (using python's asyncore package); the other
 implements the test logic.
 
-- `NodeConn` is the class used to connect to a dashd.  If you implement
-a callback class that derives from `NodeConnCB` and pass that to the
-`NodeConn` object, your code will receive the appropriate callbacks when
-events of interest arrive.
+- `P2PConnection` is the class used to connect to a dashd.  `P2PInterface`
+contains the higher level logic for processing P2P payloads and connecting to
+the Bitcoin Core node application logic. For custom behaviour, subclass the
+P2PInterface object and override the callback methods.
 
-- Call `network_thread_start()` after all `NodeConn` objects are created to
+- Call `network_thread_start()` after all `P2PInterface` objects are created to
 start the networking thread.  (Continue with the test logic in your existing
 thread.)
 

test/functional/assumevalid.py

@@ -40,14 +40,14 @@ from test_framework.mininode import (CBlockHeader,
                                      CTxOut,
                                      network_thread_join,
                                      network_thread_start,
-                                     NodeConnCB,
+                                     P2PInterface,
                                      msg_block,
                                      msg_headers)
 from test_framework.script import (CScript, OP_TRUE)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (assert_equal, set_node_times)
 
-class BaseNode(NodeConnCB):
+class BaseNode(P2PInterface):
     def send_header_for_blocks(self, new_blocks):
         headers_message = msg_headers()
         headers_message.headers = [CBlockHeader(b) for b in new_blocks]

test/functional/bip65-cltv-p2p.py

@@ -66,7 +66,7 @@ class BIP65Test(BitcoinTestFramework):
         self.setup_clean_chain = True
 
     def run_test(self):
-        self.nodes[0].add_p2p_connection(NodeConnCB())
+        self.nodes[0].add_p2p_connection(P2PInterface())
 
         network_thread_start()
 

test/functional/bipdersig-p2p.py

@@ -55,7 +55,7 @@ class BIP66Test(BitcoinTestFramework):
         self.setup_clean_chain = True
 
     def run_test(self):
-        self.nodes[0].add_p2p_connection(NodeConnCB())
+        self.nodes[0].add_p2p_connection(P2PInterface())
 
         network_thread_start()
 

test/functional/example_test.py

@@ -17,10 +17,11 @@ from collections import defaultdict
 from test_framework.blocktools import (create_block, create_coinbase)
 from test_framework.mininode import (
     CInv,
-    NodeConnCB,
+    P2PInterface,
     mininode_lock,
     msg_block,
     msg_getdata,
+    NODE_NETWORK,
     network_thread_join,
     network_thread_start,
 )
@@ -31,15 +32,15 @@ from test_framework.util import (
     wait_until,
 )
 
-# NodeConnCB is a class containing callbacks to be executed when a P2P
-# message is received from the node-under-test. Subclass NodeConnCB and
+# P2PInterface is a class containing callbacks to be executed when a P2P
+# message is received from the node-under-test. Subclass P2PInterface and
 # override the on_*() methods if you need custom behaviour.
-class BaseNode(NodeConnCB):
+class BaseNode(P2PInterface):
     def __init__(self):
-        """Initialize the NodeConnCB
+        """Initialize the P2PInterface
 
         Used to inialize custom properties for the Node that aren't
-        included by default in the base class. Be aware that the NodeConnCB
+        included by default in the base class. Be aware that the P2PInterface
         base class already stores a counter for each P2P message type and the
         last received message of each type, which should be sufficient for the
         needs of most tests.
@@ -175,7 +176,7 @@ class ExampleTest(BitcoinTestFramework):
             block = create_block(self.tip, create_coinbase(height), self.block_time)
             block.solve()
             block_message = msg_block(block)
-            # Send message is used to send a P2P message to the node over our NodeConn connection
+            # Send message is used to send a P2P message to the node over our P2PInterface
             self.nodes[0].p2p.send_message(block_message)
             self.tip = block.sha256
             blocks.append(self.tip)
@@ -207,12 +208,12 @@ class ExampleTest(BitcoinTestFramework):
         self.nodes[2].p2p.send_message(getdata_request)
 
         # wait_until() will loop until a predicate condition is met. Use it to test properties of the
-        # NodeConnCB objects.
+        # P2PInterface objects.
         wait_until(lambda: sorted(blocks) == sorted(list(self.nodes[2].p2p.block_receive_map.keys())), timeout=5, lock=mininode_lock)
 
         self.log.info("Check that each block was received only once")
-        # The network thread uses a global lock on data access to the NodeConn objects when sending and receiving
-        # messages. The test thread should acquire the global lock before accessing any NodeConn data to avoid locking
+        # The network thread uses a global lock on data access to the P2PConnection objects when sending and receiving
+        # messages. The test thread should acquire the global lock before accessing any P2PConnection data to avoid locking
         # and synchronization issues. Note wait_until() acquires this global lock when testing the predicate.
         with mininode_lock:
             for block in self.nodes[2].p2p.block_receive_map.values():

test/functional/maxuploadtarget.py

@@ -17,7 +17,7 @@ from test_framework.mininode import *
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
     def __init__(self):
         super().__init__()
         self.block_receive_map = defaultdict(int)

test/functional/p2p-acceptblock.py

@@ -7,7 +7,7 @@
 Setup: two nodes, node0+node1, not connected to each other. Node1 will have
 nMinimumChainWork set to 0x10, so it won't process low-work unrequested blocks.
 
-We have one NodeConn connection to node0 called test_node, and one to node1
+We have one P2PInterface connection to node0 called test_node, and one to node1
 called min_work_node.
 
 The test:
@@ -79,9 +79,9 @@ class AcceptBlockTest(BitcoinTestFramework):
     def run_test(self):
         # Setup the p2p connections and start up the network thread.
         # test_node connects to node0 (not whitelisted)
-        test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
-        # min_work_node connects to node1
-        min_work_node = self.nodes[1].add_p2p_connection(NodeConnCB())
+        test_node = self.nodes[0].add_p2p_connection(P2PInterface())
+        # min_work_node connects to node1 (whitelisted)
+        min_work_node = self.nodes[1].add_p2p_connection(P2PInterface())
 
         network_thread_start()
 
@@ -210,7 +210,7 @@ class AcceptBlockTest(BitcoinTestFramework):
         self.nodes[1].disconnect_p2ps()
         network_thread_join()
 
-        test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
+        test_node = self.nodes[0].add_p2p_connection(P2PInterface())
         network_thread_start()
         test_node.wait_for_verack()
 
@@ -296,7 +296,7 @@ class AcceptBlockTest(BitcoinTestFramework):
             test_node.wait_for_disconnect()
 
             self.nodes[0].disconnect_p2ps()
-            test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
+            test_node = self.nodes[0].add_p2p_connection(P2PInterface())
 
             network_thread_start()
             test_node.wait_for_verack()

test/functional/p2p-compactblocks.py

@@ -12,7 +12,7 @@ from test_framework.blocktools import create_block, create_coinbase
 from test_framework.script import CScript, OP_TRUE
 
 # TestNode: A peer we use to send messages to bitcoind, and store responses.
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
     def __init__(self):
         super().__init__()
         self.last_sendcmpct = []

test/functional/p2p-fingerprint.py

@@ -14,7 +14,7 @@ import time
 from test_framework.blocktools import (create_block, create_coinbase)
 from test_framework.mininode import (
     CInv,
-    NodeConnCB,
+    P2PInterface,
     msg_headers,
     msg_block,
     msg_getdata,
@@ -77,7 +77,7 @@ class P2PFingerprintTest(BitcoinTestFramework):
     # This does not currently test that stale blocks timestamped within the
     # last month but that have over a month's worth of work are also withheld.
     def run_test(self):
-        node0 = self.nodes[0].add_p2p_connection(NodeConnCB())
+        node0 = self.nodes[0].add_p2p_connection(P2PInterface())
 
         network_thread_start()
         node0.wait_for_verack()

test/functional/p2p-leaktests.py

@@ -17,7 +17,7 @@ from test_framework.util import *
 
 banscore = 10
 
-class CLazyNode(NodeConnCB):
+class CLazyNode(P2PInterface):
     def __init__(self):
         super().__init__()
         self.unexpected_msg = False

test/functional/p2p-mempool.py

@@ -20,7 +20,7 @@ class P2PMempoolTests(BitcoinTestFramework):
 
     def run_test(self):
         # Add a p2p connection
-        self.nodes[0].add_p2p_connection(NodeConnCB())
+        self.nodes[0].add_p2p_connection(P2PInterface())
         network_thread_start()
         self.nodes[0].p2p.wait_for_verack()
 

test/functional/p2p-timeouts.py

@@ -27,7 +27,7 @@ from test_framework.mininode import *
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
     def on_version(self, message):
         # Don't send a verack in response
         pass

test/functional/p2p-versionbits-warning.py

@@ -23,7 +23,7 @@ WARN_UNKNOWN_RULES_MINED = "Unknown block versions being mined! It's possible un
 WARN_UNKNOWN_RULES_ACTIVE = "unknown new rules activated (versionbit {})".format(VB_UNKNOWN_BIT)
 VB_PATTERN = re.compile("^Warning.*versionbit")
 
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
     def on_inv(self, message):
         pass
 

test/functional/sendheaders.py

@@ -90,7 +90,7 @@ from test_framework.mininode import (
     CBlockHeader,
     CInv,
     network_thread_start,
-    NodeConnCB,
+    P2PInterface,
     mininode_lock,
     msg_block,
     msg_getblocks,
@@ -109,7 +109,7 @@ from test_framework.util import (
 
 DIRECT_FETCH_RESPONSE_TIME = 0.05
 
-class BaseNode(NodeConnCB):
+class BaseNode(P2PInterface):
     def __init__(self):
         super().__init__()
 

test/functional/test_framework/mininode.py

@@ -9,9 +9,8 @@
 This python code was modified from ArtForz' public domain  half-a-node, as
 found in the mini-node branch of http://github.com/jgarzik/pynode.
 
-NodeConn: an object which manages p2p connectivity to a bitcoin node
-NodeConnCB: a base class that describes the interface for receiving
-            callbacks with network messages from a NodeConn
+P2PConnection: A low-level connection object to a node's P2P interface
+P2PInterface: A high-level interface object for communicating to a node over P2P
 P2PDataStore: A p2p interface class that keeps a store of transactions and blocks
               and can respond correctly to getdata and getheaders messages
 """
@@ -75,7 +74,7 @@ MAGIC_BYTES = {
     "devnet": b"\xe2\xca\xff\xce",    # devnet
 }
 
-class NodeConn(asyncore.dispatcher):
+class P2PConnection(asyncore.dispatcher):
     """A low-level connection object to a node's P2P interface.
 
     This class is responsible for:
@@ -86,9 +85,7 @@ class NodeConn(asyncore.dispatcher):
     - logging messages as they are sent and received
 
     This class contains no logic for handing the P2P message payloads. It must be
-    sub-classed and the on_message() callback overridden.
-
-    TODO: rename this class P2PConnection."""
+    sub-classed and the on_message() callback overridden."""
 
     def __init__(self):
         super().__init__(map=mininode_socket_map)
@@ -266,7 +263,7 @@ class NodeConn(asyncore.dispatcher):
         logger.debug(log_message)
 
 
-class NodeConnCB(NodeConn):
+class P2PInterface(P2PConnection):
     """A high-level P2P interface class for communicating with a Bitcoin node.
 
     This class provides high-level callbacks for processing P2P message
@@ -274,9 +271,7 @@ class NodeConnCB(NodeConn):
     node over P2P.
 
     Individual testcases should subclass this and override the on_* methods
-    if they want to alter message handling behaviour.
-
-    TODO: rename this class P2PInterface"""
+    if they want to alter message handling behaviour."""
     def __init__(self):
         super().__init__()
 
@@ -439,10 +434,10 @@ mininode_socket_map = dict()
 
 # One lock for synchronizing all data access between the networking thread (see
 # NetworkThread below) and the thread running the test logic.  For simplicity,
-# NodeConn acquires this lock whenever delivering a message to a NodeConnCB,
+# P2PConnection acquires this lock whenever delivering a message to a P2PInterface,
 # and whenever adding anything to the send buffer (in send_message()).  This
 # lock should be acquired in the thread running the test logic to synchronize
-# access to any data shared with the NodeConnCB or NodeConn.
+# access to any data shared with the P2PInterface or P2PConnection.
 mininode_lock = threading.RLock()
 
 class NetworkThread(threading.Thread):