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 bitcoind(s) being tested (using python's asyncore package); the other
 implements the test logic.
 
-- `NodeConn` is the class used to connect to a bitcoind.  If you implement
+- `P2PConnection` is the class used to connect to a bitcoind.  `P2PInterface`
-a callback class that derives from `NodeConnCB` and pass that to the
+contains the higher level logic for processing P2P payloads and connecting to
-`NodeConn` object, your code will receive the appropriate callbacks when
+the Bitcoin Core node application logic. For custom behaviour, subclass the
-events of interest arrive.
+P2PInterface object and override the callback methods.
 
-- Call `NetworkThread.start()` after all `NodeConn` objects are created to
+- Call `NetworkThread.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

@@ -39,14 +39,14 @@ from test_framework.mininode import (CBlockHeader,
                                      CTxIn,
                                      CTxOut,
                                      NetworkThread,
-                                     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
 
-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())
 
         NetworkThread().start() # Start up network handling in another thread
 

test/functional/bipdersig-p2p.py

@@ -54,7 +54,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())
 
         NetworkThread().start() # Start up network handling in another thread
 

test/functional/example_test.py

@@ -18,10 +18,11 @@ from test_framework.blocktools import (create_block, create_coinbase)
 from test_framework.mininode import (
     CInv,
     NetworkThread,
-    NodeConnCB,
+    P2PInterface,
     mininode_lock,
     msg_block,
     msg_getdata,
+    NODE_NETWORK,
 )
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -30,15 +31,15 @@ from test_framework.util import (
     wait_until,
 )
 
-# NodeConnCB is a class containing callbacks to be executed when a P2P
+# P2PInterface is a class containing callbacks to be executed when a P2P
-# message is received from the node-under-test. Subclass NodeConnCB and
+# 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.
@@ -174,7 +175,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)
@@ -199,12 +200,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
+        # 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 NodeConn data to avoid locking
+        # 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())
+        test_node = self.nodes[0].add_p2p_connection(P2PInterface())
-        # min_work_node connects to node1
+        # min_work_node connects to node1 (whitelisted)
-        min_work_node = self.nodes[1].add_p2p_connection(NodeConnCB())
+        min_work_node = self.nodes[1].add_p2p_connection(P2PInterface())
 
         NetworkThread().start() # Start up network handling in another thread
 
@@ -207,7 +207,7 @@ class AcceptBlockTest(BitcoinTestFramework):
         # disconnect/reconnect first
 
         self.nodes[0].disconnect_p2ps()
-        test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
+        test_node = self.nodes[0].add_p2p_connection(P2PInterface())
 
         test_node.wait_for_verack()
         test_node.send_message(msg_block(block_h1f))
@@ -292,7 +292,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())
 
             NetworkThread().start() # Start up network handling in another thread
             test_node.wait_for_verack()

test/functional/p2p-compactblocks.py

@@ -15,7 +15,7 @@ from test_framework.blocktools import create_block, create_coinbase, add_witness
 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-feefilter.py

@@ -22,7 +22,7 @@ def allInvsMatch(invsExpected, testnode):
         time.sleep(1)
     return False
 
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
     def __init__(self):
         super().__init__()
         self.txinvs = []

test/functional/p2p-fingerprint.py

@@ -14,7 +14,7 @@ from test_framework.blocktools import (create_block, create_coinbase)
 from test_framework.mininode import (
     CInv,
     NetworkThread,
-    NodeConnCB,
+    P2PInterface,
     msg_headers,
     msg_block,
     msg_getdata,
@@ -75,7 +75,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())
 
         NetworkThread().start()
         node0.wait_for_verack()

test/functional/p2p-leaktests.py

@@ -20,7 +20,7 @@ from test_framework.util import *
 
 banscore = 10
 
-class CLazyNode(NodeConnCB):
+class CLazyNode(P2PInterface):
     def __init__(self):
         super().__init__()
         self.unexpected_msg = False
@@ -139,10 +139,10 @@ class P2PLeakTest(BitcoinTestFramework):
         self.log.info("Service bits 5 and 7 are allowed after August 1st 2018")
         self.nodes[0].setmocktime(1533168000)  # August 2nd 2018
 
-        allowed_service_bit5_node = self.nodes[0].add_p2p_connection(NodeConnCB(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_5)
+        allowed_service_bit5_node = self.nodes[0].add_p2p_connection(P2PInterface(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_5)
-        allowed_service_bit7_node = self.nodes[0].add_p2p_connection(NodeConnCB(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_7)
+        allowed_service_bit7_node = self.nodes[0].add_p2p_connection(P2PInterface(), services=NODE_NETWORK|NODE_UNSUPPORTED_SERVICE_BIT_7)
 
-        NetworkThread().start()  # Network thread stopped when all previous NodeConnCBs disconnected. Restart it
+        NetworkThread().start()  # Network thread stopped when all previous P2PInterfaces disconnected. Restart it
 
         wait_until(lambda: allowed_service_bit5_node.message_count["verack"], lock=mininode_lock)
         wait_until(lambda: allowed_service_bit7_node.message_count["verack"], lock=mininode_lock)

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())
         NetworkThread().start()
         self.nodes[0].p2p.wait_for_verack()
 

test/functional/p2p-segwit.py

@@ -59,7 +59,7 @@ def test_witness_block(rpc, p2p, block, accepted, with_witness=True):
     p2p.sync_with_ping()
     assert_equal(rpc.getbestblockhash() == block.hash, accepted)
 
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
     def __init__(self):
         super().__init__()
         self.getdataset = set()

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

@@ -91,7 +91,7 @@ from test_framework.mininode import (
     CInv,
     NODE_WITNESS,
     NetworkThread,
-    NodeConnCB,
+    P2PInterface,
     mininode_lock,
     msg_block,
     msg_getblocks,
@@ -110,7 +110,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/comptool.py

@@ -39,7 +39,7 @@ class RejectResult():
     def __repr__(self):
         return '%i:%s' % (self.code,self.reason or '*')
 
-class TestNode(NodeConnCB):
+class TestNode(P2PInterface):
 
     def __init__(self, block_store, tx_store):
         super().__init__()

test/functional/test_framework/mininode.py

@@ -9,10 +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
+P2PConnection: A low-level connection object to a node's P2P interface
-NodeConnCB: a base class that describes the interface for receiving
+P2PInterface: A high-level interface object for communicating to a node over P2P"""
-            callbacks with network messages from a NodeConn
-"""
 import asyncore
 from collections import defaultdict
 from io import BytesIO
@@ -57,7 +55,7 @@ MAGIC_BYTES = {
     "regtest": b"\xfa\xbf\xb5\xda",   # regtest
 }
 
-class NodeConn(asyncore.dispatcher):
+class P2PConnection(asyncore.dispatcher):
     """A low-level connection object to a node's P2P interface.
 
     This class is responsible for:
@@ -68,9 +66,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.
+    sub-classed and the on_message() callback overridden."""
-
-    TODO: rename this class P2PConnection."""
 
     def __init__(self):
         super().__init__(map=mininode_socket_map)
@@ -244,7 +240,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
@@ -252,9 +248,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.
+    if they want to alter message handling behaviour."""
-
-    TODO: rename this class P2PInterface"""
     def __init__(self):
         super().__init__()
 
@@ -399,10 +393,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 = RLock()
 
 class NetworkThread(Thread):