foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#28805: test: Make existing functional tests compatible with --v2transport

Browse source 
35fb9930ad test: enable v2 transport for p2p_timeouts.py (Martin Zumsande)
2c1669c37a test: enable v2 transport for rpc_net.py (Sebastian Falbesoner)
cc961c2695 test: enable v2 transport for p2p_node_network_limited.py (Sebastian Falbesoner)
3598a1b5c9 test: enable --v2transport in combination with --usecli (Martin Zumsande)
68a9001751 test: persist -v2transport over restarts and respect -v2transport=0 (Martin Zumsande)

Pull request description:

  This makes the functional test suite compatible with BIP324, so that
  `python3 test_runner.py --v2transport`
  should succeed (currently, 12 tests fail for me on master).
  Includes two commits by TheStack I found in an old discussion https://github.com/bitcoin/bitcoin/pull/28331#discussion_r1326714164

  Note that even though all tests should pass, the python `p2p.py` module will do v2 connections only after the merge of #24748, so that for now only connections between two full nodes will actually run v2.
  Some of the fixed tests were added with `--v2transport` to the test runner. Though after #24748 we might also want to consider running the entire suite with `--v2transport` in some CI.

ACKs for top commit:
  sipa:
    utACK 35fb9930ad. Thanks for taking care of this.
  achow101:
    ACK 35fb9930ad
  theStack:
    ACK 35fb9930ad
  stratospher:
    ACK 35fb993.

Tree-SHA512: 80dc0bf211fa525ff1d092043aea9f222f14c02e5832a548fb8b83b9ede1fcee03c5e8ade0d05c331bdaa492af9c1cf3d0f0b15b846673c6eacea82dd4cefbc3
This commit is contained in:
Andrew Chow 2023-11-28 14:08:40 -05:00
commit 30a0557829
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
6 changed files with 53 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
test/functional/p2p_node_network_limited.py
View file

@ -8,7 +8,15 @@ Tests that a node configured with -prune=550 signals NODE_NETWORK_LIMITED correc
and that it responds to getdata requests for blocks correctly:
- send a block within 288 + 2 of the tip
- disconnect peers who request blocks older than that."""
from test_framework.messages import CInv, MSG_BLOCK, msg_getdata, msg_verack, NODE_NETWORK_LIMITED, NODE_WITNESS
from test_framework.messages import (
CInv,
MSG_BLOCK,
NODE_NETWORK_LIMITED,
NODE_P2P_V2,
NODE_WITNESS,
msg_getdata,
msg_verack,
)
from test_framework.p2p import P2PInterface
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
@ -50,6 +58,8 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
node = self.nodes[0].add_p2p_connection(P2PIgnoreInv())
expected_services = NODE_WITNESS | NODE_NETWORK_LIMITED
if self.options.v2transport:
expected_services |= NODE_P2P_V2
self.log.info("Check that node has signalled expected services.")
assert_equal(node.nServices, expected_services)

15
test/functional/p2p_timeouts.py
View file

@ -68,11 +68,14 @@ 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 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 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())
self.mock_forward(1)
assert "version" in no_verack_node.last_message
assert no_verack_node.is_connected
@ -80,11 +83,12 @@ class TimeoutsTest(BitcoinTestFramework):
assert no_send_node.is_connected
no_verack_node.send_message(msg_ping())
no_version_node.send_message(msg_ping())
if not self.options.v2transport:
no_version_node.send_message(msg_ping())
expected_timeout_logs = [
"version handshake timeout peer=0",
"socket no message in first 3 seconds, 1 0 peer=1",
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",
]
@ -100,5 +104,6 @@ class TimeoutsTest(BitcoinTestFramework):
extra_args=['-peertimeout=0'],
)
if __name__ == '__main__':
TimeoutsTest().main()

23
test/functional/rpc_net.py
View file

@ -150,7 +150,7 @@ class NetTest(BitcoinTestFramework):
"synced_blocks": -1,
"synced_headers": -1,
"timeoffset": 0,
"transport_protocol_type": "v1",
"transport_protocol_type": "v1" if not self.options.v2transport else "detecting",
"version": 0,
},
)
@ -160,19 +160,23 @@ class NetTest(BitcoinTestFramework):
def test_getnettotals(self):
self.log.info("Test getnettotals")
# Test getnettotals and getpeerinfo by doing a ping. The bytes
# sent/received should increase by at least the size of one ping (32
# bytes) and one pong (32 bytes).
# sent/received should increase by at least the size of one ping
# and one pong. Both have a payload size of 8 bytes, but the total
# size depends on the used p2p version:
# - p2p v1: 24 bytes (header) + 8 bytes (payload) = 32 bytes
# - p2p v2: 21 bytes (header/tag with short-id) + 8 bytes (payload) = 29 bytes
ping_size = 32 if not self.options.v2transport else 29
net_totals_before = self.nodes[0].getnettotals()
peer_info_before = self.nodes[0].getpeerinfo()
self.nodes[0].ping()
self.wait_until(lambda: (self.nodes[0].getnettotals()['totalbytessent'] >= net_totals_before['totalbytessent'] + 32 * 2), timeout=1)
self.wait_until(lambda: (self.nodes[0].getnettotals()['totalbytesrecv'] >= net_totals_before['totalbytesrecv'] + 32 * 2), timeout=1)
self.wait_until(lambda: (self.nodes[0].getnettotals()['totalbytessent'] >= net_totals_before['totalbytessent'] + ping_size * 2), timeout=1)
self.wait_until(lambda: (self.nodes[0].getnettotals()['totalbytesrecv'] >= net_totals_before['totalbytesrecv'] + ping_size * 2), timeout=1)
for peer_before in peer_info_before:
peer_after = lambda: next(p for p in self.nodes[0].getpeerinfo() if p['id'] == peer_before['id'])
self.wait_until(lambda: peer_after()['bytesrecv_per_msg'].get('pong', 0) >= peer_before['bytesrecv_per_msg'].get('pong', 0) + 32, timeout=1)
self.wait_until(lambda: peer_after()['bytessent_per_msg'].get('ping', 0) >= peer_before['bytessent_per_msg'].get('ping', 0) + 32, timeout=1)
self.wait_until(lambda: peer_after()['bytesrecv_per_msg'].get('pong', 0) >= peer_before['bytesrecv_per_msg'].get('pong', 0) + ping_size, timeout=1)
self.wait_until(lambda: peer_after()['bytessent_per_msg'].get('ping', 0) >= peer_before['bytessent_per_msg'].get('ping', 0) + ping_size, timeout=1)
def test_getnetworkinfo(self):
self.log.info("Test getnetworkinfo")
@ -345,7 +349,10 @@ class NetTest(BitcoinTestFramework):
node = self.nodes[0]
self.restart_node(0)
self.connect_nodes(0, 1)
# we want to use a p2p v1 connection here in order to ensure
# a peer id of zero (a downgrade from v2 to v1 would lead
# to an increase of the peer id)
self.connect_nodes(0, 1, peer_advertises_v2=False)
self.log.info("Test sendmsgtopeer")
self.log.debug("Send a valid message")

5
test/functional/test_framework/test_framework.py
View file

@ -507,8 +507,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
assert_equal(len(binary_cli), num_nodes)
for i in range(num_nodes):
args = list(extra_args[i])
if self.options.v2transport and ("-v2transport=0" not in args):
args.append("-v2transport=1")
test_node_i = TestNode(
i,
get_datadir_path(self.options.tmpdir, i),
@ -527,6 +525,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
start_perf=self.options.perf,
use_valgrind=self.options.valgrind,
descriptors=self.options.descriptors,
v2transport=self.options.v2transport,
)
self.nodes.append(test_node_i)
if not test_node_i.version_is_at_least(170000):
@ -601,7 +600,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
ip_port = "127.0.0.1:" + str(p2p_port(b))
if peer_advertises_v2 is None:
peer_advertises_v2 = self.options.v2transport
peer_advertises_v2 = from_connection.use_v2transport
if peer_advertises_v2:
from_connection.addnode(node=ip_port, command="onetry", v2transport=True)

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

@ -67,7 +67,7 @@ class TestNode():
To make things easier for the test writer, any unrecognised messages will
be dispatched to the RPC connection."""
def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False):
def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False, v2transport=False):
"""
Kwargs:
start_perf (bool): If True, begin profiling the node with `perf` as soon as
@ -126,6 +126,12 @@ class TestNode():
if self.version_is_at_least(239000):
self.args.append("-loglevel=trace")
# Default behavior from global -v2transport flag is added to args to persist it over restarts.
# May be overwritten in individual tests, using extra_args.
self.default_to_v2 = v2transport
if self.default_to_v2:
self.args.append("-v2transport=1")
self.cli = TestNodeCLI(bitcoin_cli, self.datadir_path)
self.use_cli = use_cli
self.start_perf = start_perf
@ -198,6 +204,8 @@ class TestNode():
if extra_args is None:
extra_args = self.extra_args
self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args)
# Add a new stdout and stderr file each time bitcoind is started
if stderr is None:
stderr = tempfile.NamedTemporaryFile(dir=self.stderr_dir, delete=False)
@ -782,15 +790,15 @@ class TestNodeCLI():
results.append(dict(error=e))
return results
def send_cli(self, command=None, *args, **kwargs):
def send_cli(self, clicommand=None, *args, **kwargs):
"""Run bitcoin-cli command. Deserializes returned string as python object."""
pos_args = [arg_to_cli(arg) for arg in args]
named_args = [str(key) + "=" + arg_to_cli(value) for (key, value) in kwargs.items()]
p_args = [self.binary, f"-datadir={self.datadir}"] + self.options
if named_args:
p_args += ["-named"]
if command is not None:
p_args += [command]
if clicommand is not None:
p_args += [clicommand]
p_args += pos_args + named_args
self.log.debug("Running bitcoin-cli {}".format(p_args[2:]))
process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)

3
test/functional/test_runner.py
View file

@ -156,6 +156,7 @@ BASE_SCRIPTS = [
'p2p_invalid_messages.py',
'rpc_createmultisig.py',
'p2p_timeouts.py',
'p2p_timeouts.py --v2transport',
'wallet_dump.py --legacy-wallet',
'rpc_signer.py',
'wallet_signer.py --descriptors',
@ -243,6 +244,7 @@ BASE_SCRIPTS = [
'p2p_getdata.py',
'p2p_addrfetch.py',
'rpc_net.py',
'rpc_net.py --v2transport',
'wallet_keypool.py --legacy-wallet',
'wallet_keypool.py --descriptors',
'wallet_descriptor.py --descriptors',
@ -368,6 +370,7 @@ BASE_SCRIPTS = [
'wallet_orphanedreward.py',
'wallet_timelock.py',
'p2p_node_network_limited.py',
'p2p_node_network_limited.py --v2transport',
'p2p_permissions.py',
'feature_blocksdir.py',
'wallet_startup.py',