From 652311165c4ef298dab71d7162f9054abf439f77 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sat, 28 Nov 2020 11:41:25 +0000 Subject: [PATCH 1/6] [test] Move MIN_VERSION_SUPPORTED to p2p.py The messages.py module should contain code and helpers for [de]serializing p2p messages. Specific usage of those messages should be in p2p.py. Therefore move MIN_VERSION_SUPPORTED to p2p.py. Also rename to MIN_P2P_VERSION_SUPPORTED to distinguish it from other versioning used in Bitcoin/Bitcoin Core. --- test/functional/test_framework/messages.py | 1 - test/functional/test_framework/p2p.py | 6 ++++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 561d1813c1a..623c01908bf 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -31,7 +31,6 @@ import time from test_framework.siphash import siphash256 from test_framework.util import hex_str_to_bytes, assert_equal -MIN_VERSION_SUPPORTED = 60001 MY_VERSION = 70016 # past wtxid relay MY_SUBVERSION = "/python-p2p-tester:0.0.3/" MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index fa4a567aac5..befadc894f7 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -31,7 +31,6 @@ import threading from test_framework.messages import ( CBlockHeader, MAX_HEADERS_RESULTS, - MIN_VERSION_SUPPORTED, msg_addr, msg_addrv2, msg_block, @@ -79,6 +78,9 @@ from test_framework.util import ( logger = logging.getLogger("TestFramework.p2p") +# The minimum P2P version that this test framework supports +MIN_P2P_VERSION_SUPPORTED = 60001 + MESSAGEMAP = { b"addr": msg_addr, b"addrv2": msg_addrv2, @@ -417,7 +419,7 @@ class P2PInterface(P2PConnection): pass def on_version(self, message): - assert message.nVersion >= MIN_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_VERSION_SUPPORTED) + assert message.nVersion >= MIN_P2P_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_P2P_VERSION_SUPPORTED) if message.nVersion >= 70016 and self.wtxidrelay: self.send_message(msg_wtxidrelay()) if self.support_addrv2: From 7e158a69104831611462cb555da931331b237c78 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sat, 28 Nov 2020 11:41:25 +0000 Subject: [PATCH 2/6] [test] Move MY_VERSION to p2p.py The messages.py module should contain code and helpers for [de]serializing p2p messages. Specific usage of those messages should be in p2p.py. Therefore move MY_VERSION to p2p.py. Also rename to P2P_VERSION to distinguish it from other versioning used in Bitcoin/Bitcoin Core. Also always set the nVersion field in CBlockLocator to 0 and ignore the field in deserialized messages. The field is not currently used for anything in Bitcoin Core. --- test/functional/p2p_filter.py | 7 ++++++- test/functional/test_framework/messages.py | 11 ++++------- test/functional/test_framework/p2p.py | 4 ++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 458e5235b6c..2324a3f5889 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -19,7 +19,11 @@ from test_framework.messages import ( msg_mempool, msg_version, ) -from test_framework.p2p import P2PInterface, p2p_lock +from test_framework.p2p import ( + P2PInterface, + P2P_VERSION, + p2p_lock, +) from test_framework.script import MAX_SCRIPT_ELEMENT_SIZE from test_framework.test_framework import BitcoinTestFramework @@ -218,6 +222,7 @@ class FilterTest(BitcoinTestFramework): filter_peer_without_nrelay = self.nodes[0].add_p2p_connection(P2PBloomFilter(), send_version=False, wait_for_verack=False) # Send version with fRelay=False version_without_fRelay = msg_version() + version_without_fRelay.nVersion = P2P_VERSION version_without_fRelay.nRelay = 0 filter_peer_without_nrelay.send_message(version_without_fRelay) filter_peer_without_nrelay.wait_for_verack() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 623c01908bf..b120ce2438d 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -31,7 +31,6 @@ import time from test_framework.siphash import siphash256 from test_framework.util import hex_str_to_bytes, assert_equal -MY_VERSION = 70016 # past wtxid relay MY_SUBVERSION = "/python-p2p-tester:0.0.3/" MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) @@ -325,22 +324,20 @@ class CBlockLocator: __slots__ = ("nVersion", "vHave") def __init__(self): - self.nVersion = MY_VERSION self.vHave = [] def deserialize(self, f): - self.nVersion = struct.unpack(" Date: Sat, 28 Nov 2020 11:41:25 +0000 Subject: [PATCH 3/6] [test] Move MY_SUBVERSION to p2p.py The messages.py module should contain code and helpers for [de]serializing p2p messages. Specific usage of those messages should be in p2p.py. Therefore move MY_SUBVERSION to p2p.py. Also rename to P2P_SUBVERSION. --- test/functional/p2p_filter.py | 2 ++ test/functional/p2p_leak.py | 6 +++++- test/functional/test_framework/messages.py | 3 +-- test/functional/test_framework/p2p.py | 3 +++ test/functional/test_framework/test_node.py | 4 ++-- 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 2324a3f5889..78bdb32cf44 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -21,6 +21,7 @@ from test_framework.messages import ( ) from test_framework.p2p import ( P2PInterface, + P2P_SUBVERSION, P2P_VERSION, p2p_lock, ) @@ -223,6 +224,7 @@ class FilterTest(BitcoinTestFramework): # Send version with fRelay=False version_without_fRelay = msg_version() version_without_fRelay.nVersion = P2P_VERSION + version_without_fRelay.strSubVer = P2P_SUBVERSION version_without_fRelay.nRelay = 0 filter_peer_without_nrelay.send_message(version_without_fRelay) filter_peer_without_nrelay.wait_for_verack() diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index ca8bf908a97..2cf327fc185 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -17,7 +17,10 @@ from test_framework.messages import ( msg_ping, msg_version, ) -from test_framework.p2p import P2PInterface +from test_framework.p2p import ( + P2PInterface, + P2P_SUBVERSION, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -131,6 +134,7 @@ class P2PLeakTest(BitcoinTestFramework): p2p_old_peer = self.nodes[0].add_p2p_connection(P2PInterface(), send_version=False, wait_for_verack=False) old_version_msg = msg_version() old_version_msg.nVersion = 31799 + old_version_msg.strSubVer = P2P_SUBVERSION with self.nodes[0].assert_debug_log(['peer=3 using obsolete version 31799; disconnecting']): p2p_old_peer.send_message(old_version_msg) p2p_old_peer.wait_for_disconnect() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index b120ce2438d..cee662aaded 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -31,7 +31,6 @@ import time from test_framework.siphash import siphash256 from test_framework.util import hex_str_to_bytes, assert_equal -MY_SUBVERSION = "/python-p2p-tester:0.0.3/" MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) MAX_LOCATOR_SZ = 101 @@ -1030,7 +1029,7 @@ class msg_version: self.addrTo = CAddress() self.addrFrom = CAddress() self.nNonce = random.getrandbits(64) - self.strSubVer = MY_SUBVERSION + self.strSubVer = '' self.nStartingHeight = -1 self.nRelay = MY_RELAY diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 998763ff4d7..0814dec5467 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -83,6 +83,8 @@ MIN_P2P_VERSION_SUPPORTED = 60001 # The P2P version that this test framework implements and sends in its `version` message # Version 70016 supports wtxid relay P2P_VERSION = 70016 +# The P2P user agent string that this test framework sends in its `version` message +P2P_SUBVERSION = "/python-p2p-tester:0.0.3/" MESSAGEMAP = { b"addr": msg_addr, @@ -333,6 +335,7 @@ class P2PInterface(P2PConnection): # Send a version msg vt = msg_version() vt.nVersion = P2P_VERSION + vt.strSubVer = P2P_SUBVERSION vt.nServices = services vt.addrTo.ip = self.dstaddr vt.addrTo.port = self.dstport diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 9f2b5709130..24f48d55352 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -23,7 +23,7 @@ import sys from .authproxy import JSONRPCException from .descriptors import descsum_create -from .messages import MY_SUBVERSION +from .p2p import P2P_SUBVERSION from .util import ( MAX_NODES, append_config, @@ -572,7 +572,7 @@ class TestNode(): def num_test_p2p_connections(self): """Return number of test framework p2p connections to the node.""" - return len([peer for peer in self.getpeerinfo() if peer['subver'] == MY_SUBVERSION]) + return len([peer for peer in self.getpeerinfo() if peer['subver'] == P2P_SUBVERSION]) def disconnect_p2ps(self): """Close all p2p connections to the node.""" From 010542614dbebba5f5ad6a58c0554930e9e214fc Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sat, 28 Nov 2020 11:41:25 +0000 Subject: [PATCH 4/6] [test] Move MY_RELAY to p2p.py messages.py is for message and primitive data structures. Specifics about the test framework's p2p implementation should be in p2p.py. Also rename to P2P_VERSION_RELAY. Also rename msg_version.nRelay to relay. In Bitcoin Core, this is referred to as fRelay, since it's a bool, so this field has always been misnamed. --- test/functional/p2p_filter.py | 4 ++-- test/functional/p2p_leak.py | 4 +++- test/functional/test_framework/messages.py | 18 ++++++++---------- test/functional/test_framework/p2p.py | 3 +++ 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 78bdb32cf44..4df3dd6a215 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -221,11 +221,11 @@ class FilterTest(BitcoinTestFramework): self.log.info('Test BIP 37 for a node with fRelay = False') # Add peer but do not send version yet filter_peer_without_nrelay = self.nodes[0].add_p2p_connection(P2PBloomFilter(), send_version=False, wait_for_verack=False) - # Send version with fRelay=False + # Send version with relay=False version_without_fRelay = msg_version() version_without_fRelay.nVersion = P2P_VERSION version_without_fRelay.strSubVer = P2P_SUBVERSION - version_without_fRelay.nRelay = 0 + version_without_fRelay.relay = 0 filter_peer_without_nrelay.send_message(version_without_fRelay) filter_peer_without_nrelay.wait_for_verack() assert not self.nodes[0].getpeerinfo()[0]['relaytxes'] diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index 2cf327fc185..42e9b9f2701 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -20,6 +20,7 @@ from test_framework.messages import ( from test_framework.p2p import ( P2PInterface, P2P_SUBVERSION, + P2P_VERSION_RELAY, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -128,13 +129,14 @@ class P2PLeakTest(BitcoinTestFramework): assert_equal(ver.addrFrom.port, 0) assert_equal(ver.addrFrom.ip, '0.0.0.0') assert_equal(ver.nStartingHeight, 201) - assert_equal(ver.nRelay, 1) + assert_equal(ver.relay, 1) self.log.info('Check that old peers are disconnected') p2p_old_peer = self.nodes[0].add_p2p_connection(P2PInterface(), send_version=False, wait_for_verack=False) old_version_msg = msg_version() old_version_msg.nVersion = 31799 old_version_msg.strSubVer = P2P_SUBVERSION + old_version_msg.relay = P2P_VERSION_RELAY with self.nodes[0].assert_debug_log(['peer=3 using obsolete version 31799; disconnecting']): p2p_old_peer.send_message(old_version_msg) p2p_old_peer.wait_for_disconnect() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index cee662aaded..5170d430b7e 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -31,8 +31,6 @@ import time from test_framework.siphash import siphash256 from test_framework.util import hex_str_to_bytes, assert_equal -MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) - MAX_LOCATOR_SZ = 101 MAX_BLOCK_BASE_SIZE = 1000000 MAX_BLOOM_FILTER_SIZE = 36000 @@ -1018,7 +1016,7 @@ class CMerkleBlock: # Objects that correspond to messages on the wire class msg_version: - __slots__ = ("addrFrom", "addrTo", "nNonce", "nRelay", "nServices", + __slots__ = ("addrFrom", "addrTo", "nNonce", "relay", "nServices", "nStartingHeight", "nTime", "nVersion", "strSubVer") msgtype = b"version" @@ -1031,7 +1029,7 @@ class msg_version: self.nNonce = random.getrandbits(64) self.strSubVer = '' self.nStartingHeight = -1 - self.nRelay = MY_RELAY + self.relay = 0 def deserialize(self, f): self.nVersion = struct.unpack("= 70001: # Relay field is optional for version 70001 onwards try: - self.nRelay = struct.unpack(" Date: Sat, 28 Nov 2020 11:41:25 +0000 Subject: [PATCH 5/6] [test] Add P2P_SERVICES to p2p.py The messages.py module should contain code and helpers for [de]serializing p2p messages. Specific usage of those messages should be in p2p.py. Therefore specify the nServices value in the calling code, not in the messages.py module. --- test/functional/p2p_filter.py | 2 ++ test/functional/p2p_leak.py | 2 ++ test/functional/test_framework/messages.py | 2 +- test/functional/test_framework/p2p.py | 4 +++- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 4df3dd6a215..8f644191384 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -21,6 +21,7 @@ from test_framework.messages import ( ) from test_framework.p2p import ( P2PInterface, + P2P_SERVICES, P2P_SUBVERSION, P2P_VERSION, p2p_lock, @@ -225,6 +226,7 @@ class FilterTest(BitcoinTestFramework): version_without_fRelay = msg_version() version_without_fRelay.nVersion = P2P_VERSION version_without_fRelay.strSubVer = P2P_SUBVERSION + version_without_fRelay.nServices = P2P_SERVICES version_without_fRelay.relay = 0 filter_peer_without_nrelay.send_message(version_without_fRelay) filter_peer_without_nrelay.wait_for_verack() diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index 42e9b9f2701..12b8b7baff0 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -20,6 +20,7 @@ from test_framework.messages import ( from test_framework.p2p import ( P2PInterface, P2P_SUBVERSION, + P2P_SERVICES, P2P_VERSION_RELAY, ) from test_framework.test_framework import BitcoinTestFramework @@ -136,6 +137,7 @@ class P2PLeakTest(BitcoinTestFramework): old_version_msg = msg_version() old_version_msg.nVersion = 31799 old_version_msg.strSubVer = P2P_SUBVERSION + old_version_msg.nServices = P2P_SERVICES old_version_msg.relay = P2P_VERSION_RELAY with self.nodes[0].assert_debug_log(['peer=3 using obsolete version 31799; disconnecting']): p2p_old_peer.send_message(old_version_msg) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 5170d430b7e..a18a9ec1090 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1022,7 +1022,7 @@ class msg_version: def __init__(self): self.nVersion = 0 - self.nServices = NODE_NETWORK | NODE_WITNESS + self.nServices = 0 self.nTime = int(time.time()) self.addrTo = CAddress() self.addrFrom = CAddress() diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 179d03e93a9..05099f33398 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -83,6 +83,8 @@ MIN_P2P_VERSION_SUPPORTED = 60001 # The P2P version that this test framework implements and sends in its `version` message # Version 70016 supports wtxid relay P2P_VERSION = 70016 +# The services that this test framework offers in its `version` message +P2P_SERVICES = NODE_NETWORK | NODE_WITNESS # The P2P user agent string that this test framework sends in its `version` message P2P_SUBVERSION = "/python-p2p-tester:0.0.3/" # Value for relay that this test framework sends in its `version` message @@ -346,7 +348,7 @@ class P2PInterface(P2PConnection): vt.addrFrom.port = 0 self.on_connection_send_msg = vt # Will be sent in connection_made callback - def peer_connect(self, *args, services=NODE_NETWORK | NODE_WITNESS, send_version=True, **kwargs): + def peer_connect(self, *args, services=P2P_SERVICES, send_version=True, **kwargs): create_conn = super().peer_connect(*args, **kwargs) if send_version: From 9f21ed4037758f407b536c0dd129f8da83173c79 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sat, 28 Nov 2020 12:41:15 +0000 Subject: [PATCH 6/6] [test] Check user agent string from test framework connections Add a check that new connections from the test framework to the node have the correct user agent string. This makes bugs easier to detect if the user agent string ever changes. --- test/functional/test_framework/test_node.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 24f48d55352..5bc1409ba29 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -26,6 +26,7 @@ from .descriptors import descsum_create from .p2p import P2P_SUBVERSION from .util import ( MAX_NODES, + assert_equal, append_config, delete_cookie_file, get_auth_cookie, @@ -545,6 +546,11 @@ class TestNode(): # in comparison to the upside of making tests less fragile and unexpected intermittent errors less likely. p2p_conn.sync_with_ping() + # Consistency check that the Bitcoin Core has received our user agent string. This checks the + # node's newest peer. It could be racy if another Bitcoin Core node has connected since we opened + # our connection, but we don't expect that to happen. + assert_equal(self.getpeerinfo()[-1]['subver'], P2P_SUBVERSION) + return p2p_conn def add_outbound_p2p_connection(self, p2p_conn, *, p2p_idx, connection_type="outbound-full-relay", **kwargs):