phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-10 03:47:29 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#29358: test: use v2 everywhere for P2PConnection if --v2transport is enabled

Browse source 
bf5662c678 test: enable v2 for python p2p depending on global --v2transport flag (Martin Zumsande)
6e9e39da43 test: Don't use v2transport when it's too slow. (Martin Zumsande)
87549c8f89 test: enable p2p_invalid_messages.py with v2transport (Martin Zumsande)
5fc9db504b test: enable p2p_sendtxrcncl.py with v2transport (Martin Zumsande)

Pull request description:

  #24748 added v2 transport to the python `P2PConnection`, but so far each test that wants to make use of it needs to enable it on an individual basis.
  This PR changes it so that if the test suite is run with `--v2transport` option, v2 is used in each test by default, not only for connections between two bitcoind instances as before, but also wherever `P2PConnection` is used. Individual tests can override this global option.

  To do that, a few tests need to be adjusted.
  In addition, I added a commit to always use v1 in a few select subtests that send a large number of large messages (e.g. large reorgs). These tests don't have a fundamental problem with v2 but become very slow due to the unoptimised python ChaCha20 implementation (~30 minutes on my computer, so probably not suitable to be run in the CI).

  As a result, `python3 test_runner.py --v2transport` should succeed and use `v2` everywhere (unless v1 is chosen explicitly).

  [Edit]: To make the "test each commit" CI pass, several test fixes were squashed into the last commit, which actually enables v2 p2p for `P2PConnection`. I have an unsquashed version at https://github.com/mzumsande/bitcoin/tree/202401_bip324_alltests_unsquashed, in case that helps with review.

ACKs for top commit:
  fjahr:
    tACK bf5662c678
  vasild:
    ACK bf5662c678
  stratospher:
    reACK bf5662c6.
  theStack:
    Tested ACK bf5662c678

Tree-SHA512: 4f5a08248ba8a755f7d0f48deb2b79bef03292345cacb7deef01be955481093800e4e56ff218ea56734eef5de1fb3ab0f04657447ea27d393bb536539d7b289d
This commit is contained in:
fanquake 2024-02-27 09:25:11 +00:00
commit 5c6d900a27
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
8 changed files with 80 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
test/functional/feature_block.py
View file

@ -1263,6 +1263,10 @@ class FullBlockTest(BitcoinTestFramework):
b89a = self.update_block("89a", [tx])
self.send_blocks([b89a], success=False, reject_reason='bad-txns-inputs-missingorspent', reconnect=True)
# Don't use v2transport for the large reorg, which is too slow with the unoptimized python ChaCha20 implementation
if self.options.v2transport:
self.nodes[0].disconnect_p2ps()
self.helper_peer = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False)
self.log.info("Test a re-org of one week's worth of blocks (1088 blocks)")
self.move_tip(88)

5
test/functional/feature_maxuploadtarget.py
View file

@ -81,7 +81,8 @@ class MaxUploadTest(BitcoinTestFramework):
p2p_conns = []
for _ in range(3):
p2p_conns.append(self.nodes[0].add_p2p_connection(TestP2PConn()))
# Don't use v2transport in this test (too slow with the unoptimized python ChaCha20 implementation)
p2p_conns.append(self.nodes[0].add_p2p_connection(TestP2PConn(), supports_v2_p2p=False))
# Now mine a big block
mine_large_block(self, self.wallet, self.nodes[0])
@ -173,7 +174,7 @@ class MaxUploadTest(BitcoinTestFramework):
self.assert_uploadtarget_state(target_reached=False, serve_historical_blocks=False)
# Reconnect to self.nodes[0]
peer = self.nodes[0].add_p2p_connection(TestP2PConn())
peer = self.nodes[0].add_p2p_connection(TestP2PConn(), supports_v2_p2p=False)
# Sending mempool message shouldn't disconnect peer, as total limit isn't reached yet
peer.send_and_ping(msg_mempool())

3
test/functional/p2p_ibd_stalling.py
View file

@ -80,7 +80,8 @@ class P2PIBDStallingTest(BitcoinTestFramework):
# Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc
# returning the number of downloaded (but not connected) blocks.
self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)
bytes_recv = 172761 if not self.options.v2transport else 169692
self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)
self.all_sync_send_with_ping(peers)
# If there was a peer marked for stalling, it would get disconnected

45
test/functional/p2p_invalid_messages.py
View file

@ -109,6 +109,9 @@ class InvalidMessagesTest(BitcoinTestFramework):
self.nodes[0].disconnect_p2ps()
def test_magic_bytes(self):
# Skip with v2, magic bytes are v1-specific
if self.options.v2transport:
return
self.log.info("Test message with invalid magic bytes disconnects peer")
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
with self.nodes[0].assert_debug_log(['Header error: Wrong MessageStart ffffffff received']):
@ -120,6 +123,9 @@ class InvalidMessagesTest(BitcoinTestFramework):
self.nodes[0].disconnect_p2ps()
def test_checksum(self):
# Skip with v2, the checksum is v1-specific
if self.options.v2transport:
return
self.log.info("Test message with invalid checksum logs an error")
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
with self.nodes[0].assert_debug_log(['Header error: Wrong checksum (badmsg, 2 bytes), expected 78df0a04 was ffffffff']):
@ -137,7 +143,11 @@ class InvalidMessagesTest(BitcoinTestFramework):
def test_size(self):
self.log.info("Test message with oversized payload disconnects peer")
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
with self.nodes[0].assert_debug_log(['Header error: Size too large (badmsg, 4000001 bytes)']):
error_msg = (
['V2 transport error: packet too large (4000014 bytes)'] if self.options.v2transport
else ['Header error: Size too large (badmsg, 4000001 bytes)']
)
with self.nodes[0].assert_debug_log(error_msg):
msg = msg_unrecognized(str_data="d" * (VALID_DATA_LIMIT + 1))
msg = conn.build_message(msg)
conn.send_raw_message(msg)
@ -147,15 +157,26 @@ class InvalidMessagesTest(BitcoinTestFramework):
def test_msgtype(self):
self.log.info("Test message with invalid message type logs an error")
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
with self.nodes[0].assert_debug_log(['Header error: Invalid message type']):
if self.options.v2transport:
msgtype = 99 # not defined
msg = msg_unrecognized(str_data="d")
msg = conn.build_message(msg)
# Modify msgtype
msg = msg[:7] + b'\x00' + msg[7 + 1:]
conn.send_raw_message(msg)
conn.sync_with_ping(timeout=1)
# Check that traffic is accounted for (24 bytes header + 2 bytes payload)
assert_equal(self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg']['*other*'], 26)
contents = msgtype.to_bytes(1, 'big') + msg.serialize()
tmsg = conn.v2_state.v2_enc_packet(contents, ignore=False)
with self.nodes[0].assert_debug_log(['V2 transport error: invalid message type']):
conn.send_raw_message(tmsg)
conn.sync_with_ping(timeout=1)
# Check that traffic is accounted for (20 bytes plus 3 bytes contents)
assert_equal(self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg']['*other*'], 23)
else:
with self.nodes[0].assert_debug_log(['Header error: Invalid message type']):
msg = msg_unrecognized(str_data="d")
msg = conn.build_message(msg)
# Modify msgtype
msg = msg[:7] + b'\x00' + msg[7 + 1:]
conn.send_raw_message(msg)
conn.sync_with_ping(timeout=1)
# Check that traffic is accounted for (24 bytes header + 2 bytes payload)
assert_equal(self.nodes[0].getpeerinfo()[0]['bytesrecv_per_msg']['*other*'], 26)
self.nodes[0].disconnect_p2ps()
def test_addrv2(self, label, required_log_messages, raw_addrv2):
@ -306,8 +327,10 @@ class InvalidMessagesTest(BitcoinTestFramework):
def test_resource_exhaustion(self):
self.log.info("Test node stays up despite many large junk messages")
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
conn2 = self.nodes[0].add_p2p_connection(P2PDataStore())
# Don't use v2 here - the non-optimised encryption would take too long to encrypt
# the large messages
conn = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False)
conn2 = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False)
msg_at_size = msg_unrecognized(str_data="b" * VALID_DATA_LIMIT)
assert len(msg_at_size.serialize()) == MAX_PROTOCOL_MESSAGE_LENGTH

27
test/functional/p2p_timeouts.py
View file

@ -69,11 +69,8 @@ class TimeoutsTest(BitcoinTestFramework):
with self.nodes[0].assert_debug_log(['Unsupported message "ping" prior to verack from peer=0']):
no_verack_node.send_message(msg_ping())
# With v2, non-version messages before the handshake would be interpreted as part of the key exchange.
# Therefore, don't execute this part of the test if v2transport is chosen.
if not self.options.v2transport:
with self.nodes[0].assert_debug_log(['non-version message before version handshake. Message "ping" from peer=1']):
no_version_node.send_message(msg_ping())
with self.nodes[0].assert_debug_log(['non-version message before version handshake. Message "ping" from peer=1']):
no_version_node.send_message(msg_ping())
self.mock_forward(1)
assert "version" in no_verack_node.last_message
@ -83,14 +80,20 @@ class TimeoutsTest(BitcoinTestFramework):
assert no_send_node.is_connected
no_verack_node.send_message(msg_ping())
if not self.options.v2transport:
no_version_node.send_message(msg_ping())
no_version_node.send_message(msg_ping())
expected_timeout_logs = [
"version handshake timeout peer=0",
f"socket no message in first 3 seconds, {'0' if self.options.v2transport else '1'} 0 peer=1",
"socket no message in first 3 seconds, 0 0 peer=2",
]
if self.options.v2transport:
expected_timeout_logs = [
"version handshake timeout peer=0",
"version handshake timeout peer=1",
"version handshake timeout peer=2",
]
else:
expected_timeout_logs = [
"version handshake timeout peer=0",
"socket no message in first 3 seconds, 1 0 peer=1",
"socket no message in first 3 seconds, 0 0 peer=2",
]
with self.nodes[0].assert_debug_log(expected_msgs=expected_timeout_logs):
self.mock_forward(2)

2
test/functional/p2p_v2_earlykeyresponse.py
View file

@ -75,7 +75,7 @@ class P2PEarlyKey(BitcoinTestFramework):
self.log.info('Sending first 4 bytes of ellswift which match network magic')
self.log.info('If a response is received, assertion failure would happen in our custom data_received() function')
# send happens in `initiate_v2_handshake()` in `connection_made()`
peer1 = node0.add_p2p_connection(PeerEarlyKey(), wait_for_verack=False, send_version=False, supports_v2_p2p=True)
peer1 = node0.add_p2p_connection(PeerEarlyKey(), wait_for_verack=False, send_version=False, supports_v2_p2p=True, wait_for_v2_handshake=False)
self.wait_until(lambda: peer1.connection_opened)
self.log.info('Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is')
self.log.info('expected now, our custom data_received() function wouldn\'t result in assertion failure')

13
test/functional/rpc_net.py
View file

@ -117,6 +117,9 @@ class NetTest(BitcoinTestFramework):
peer_info = self.nodes[0].getpeerinfo()[no_version_peer_id]
peer_info.pop("addr")
peer_info.pop("addrbind")
# The next two fields will vary for v2 connections because we send a rng-based number of decoy messages
peer_info.pop("bytesrecv")
peer_info.pop("bytessent")
assert_equal(
peer_info,
{
@ -125,9 +128,7 @@ class NetTest(BitcoinTestFramework):
"addr_relay_enabled": False,
"bip152_hb_from": False,
"bip152_hb_to": False,
"bytesrecv": 0,
"bytesrecv_per_msg": {},
"bytessent": 0,
"bytessent_per_msg": {},
"connection_type": "inbound",
"conntime": no_version_peer_conntime,
@ -136,8 +137,8 @@ class NetTest(BitcoinTestFramework):
"inflight": [],
"last_block": 0,
"last_transaction": 0,
"lastrecv": 0,
"lastsend": 0,
"lastrecv": 0 if not self.options.v2transport else no_version_peer_conntime,
"lastsend": 0 if not self.options.v2transport else no_version_peer_conntime,
"minfeefilter": Decimal("0E-8"),
"network": "not_publicly_routable",
"permissions": [],
@ -145,13 +146,13 @@ class NetTest(BitcoinTestFramework):
"relaytxes": False,
"services": "0000000000000000",
"servicesnames": [],
"session_id": "",
"session_id": "" if not self.options.v2transport else no_version_peer.v2_state.peer['session_id'].hex(),
"startingheight": -1,
"subver": "",
"synced_blocks": -1,
"synced_headers": -1,
"timeoffset": 0,
"transport_protocol_type": "v1" if not self.options.v2transport else "detecting",
"transport_protocol_type": "v1" if not self.options.v2transport else "v2",
"version": 0,
},
)

16
test/functional/test_framework/test_node.py
View file

@ -667,7 +667,7 @@ class TestNode():
assert_msg += "with expected error " + expected_msg
self._raise_assertion_error(assert_msg)
def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=False, **kwargs):
def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=None, wait_for_v2_handshake=True, **kwargs):
"""Add an inbound p2p connection to the node.
This method adds the p2p connection to the self.p2ps list and also
@ -684,6 +684,9 @@ class TestNode():
kwargs['dstport'] = p2p_port(self.index)
if 'dstaddr' not in kwargs:
kwargs['dstaddr'] = '127.0.0.1'
if supports_v2_p2p is None:
supports_v2_p2p = self.use_v2transport
p2p_conn.p2p_connected_to_node = True
if self.use_v2transport:
@ -693,6 +696,8 @@ class TestNode():
self.p2ps.append(p2p_conn)
p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False)
if supports_v2_p2p and wait_for_v2_handshake:
p2p_conn.wait_until(lambda: p2p_conn.v2_state.tried_v2_handshake)
if send_version:
p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg)
if wait_for_verack:
@ -721,7 +726,7 @@ class TestNode():
return p2p_conn
def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=False, advertise_v2_p2p=False, **kwargs):
def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=None, advertise_v2_p2p=None, **kwargs):
"""Add an outbound p2p connection from node. Must be an
"outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection.
@ -749,6 +754,11 @@ class TestNode():
self.addconnection('%s:%d' % (address, port), connection_type, advertise_v2_p2p)
p2p_conn.p2p_connected_to_node = False
if supports_v2_p2p is None:
supports_v2_p2p = self.use_v2transport
if advertise_v2_p2p is None:
advertise_v2_p2p = self.use_v2transport
if advertise_v2_p2p:
kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2
assert self.use_v2transport # only a v2 TestNode could make a v2 outbound connection
@ -771,6 +781,8 @@ class TestNode():
p2p_conn.wait_for_connect()
self.p2ps.append(p2p_conn)
if supports_v2_p2p:
p2p_conn.wait_until(lambda: p2p_conn.v2_state.tried_v2_handshake)
p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg)
if wait_for_verack:
p2p_conn.wait_for_verack()