Merge bitcoin/bitcoin#35334: test: Allow --usecli in more tests

faf02674b3 refactor: Set TestNode.cli only after RPC is connected (MarcoFalke)
fae376cafb refactor: Inline get_rpc_proxy (MarcoFalke)
fa00c7c7a4 test: Allow rpc_bind.py --usecli (MarcoFalke)
fa8f25118c refactor: Use create_new_rpc_connection in wallet_multiwallet.py (MarcoFalke)
fa3ae6c7d3 test: Allow feature_shutdown.py --usecli (MarcoFalke)
fa2a3683d5 test: Allow mining_getblocktemplate_longpoll.py --usecli (MarcoFalke)
fa37c6a529 test: [move-only] Extract create_new_rpc_connection (MarcoFalke)

Pull request description:

  Some tests disallow to be run under `--usecli`. This reduces the coverage and risks that bugs in the bitcoin-cli go unnoticed.

  The commits should be self-explanatory and can be reviewed and tested one-by-one.

ACKs for top commit:
  willcl-ark:
    reACK faf02674b3

Tree-SHA512: 83cd6a696e6dd6efd4f2d295e65bfac51fe26404c37f25936808005cc6136e469a30aebaac547af1c722ed5ac827eaf009a150d82420b6b4e242e89305475abe
This commit is contained in:
merge-script
2026-05-22 15:00:25 +01:00
7 changed files with 59 additions and 80 deletions

View File

@@ -5,7 +5,7 @@
"""Test bitcoind shutdown."""
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, get_rpc_proxy
from test_framework.util import assert_equal
from threading import Thread
def test_long_call(node):
@@ -17,10 +17,9 @@ class ShutdownTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 1
self.supports_cli = False
def run_test(self):
node = get_rpc_proxy(self.nodes[0].url, 1, timeout=600, coveragedir=self.nodes[0].coverage_dir)
node = self.nodes[0].create_new_rpc_connection()
# Force connection establishment by executing a dummy command.
node.getblockcount()
Thread(target=test_long_call, args=(node,)).start()

View File

@@ -9,7 +9,7 @@ import threading
from test_framework.blocktools import NORMAL_GBT_REQUEST_PARAMS
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, get_rpc_proxy
from test_framework.util import assert_equal
from test_framework.wallet import MiniWallet
@@ -19,9 +19,8 @@ class LongpollThread(threading.Thread):
# query current longpollid
template = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
self.longpollid = template['longpollid']
# create a new connection to the node, we can't use the same
# connection from two threads
self.node = get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir)
# create a new connection to the node for this thread
self.node = node.create_new_rpc_connection(client_timeout=600)
def run(self):
self.node.getblocktemplate({'longpollid': self.longpollid, **NORMAL_GBT_REQUEST_PARAMS})
@@ -29,7 +28,6 @@ class LongpollThread(threading.Thread):
class GetBlockTemplateLPTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
self.supports_cli = False
def run_test(self):
self.log.info("Warning: this test will take about 70 seconds in the best case. Be patient.")

View File

@@ -7,14 +7,13 @@
from test_framework.netutil import all_interfaces, addr_to_hex, get_bind_addrs, test_ipv6_local
from test_framework.test_framework import BitcoinTestFramework, SkipTest
from test_framework.test_node import ErrorMatch
from test_framework.util import assert_equal, assert_raises_rpc_error, get_rpc_proxy, rpc_port, rpc_url
from test_framework.util import assert_equal, assert_raises_rpc_error, rpc_port
class RPCBindTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.bind_to_localhost_only = False
self.num_nodes = 1
self.supports_cli = False
def skip_test_if_missing_module(self):
self.skip_if_platform_not_posix()
@@ -70,8 +69,9 @@ class RPCBindTest(BitcoinTestFramework):
['-rpcbind='+addr for addr in ['127.0.0.1', "%s:%d" % (rpchost, rpcport)]] # Bind to localhost as well so start_nodes doesn't hang
self.nodes[0].rpchost = None
self.start_nodes([node_args])
self.nodes[0].rpchost = f"{rpchost}:{rpcport}"
# connect to node through non-loopback interface
node = get_rpc_proxy(rpc_url(self.nodes[0].datadir_path, 0, self.chain, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir)
node = self.nodes[0].create_new_rpc_connection()
node.getnetworkinfo()
self.stop_nodes()
@@ -164,6 +164,9 @@ class RPCBindTest(BitcoinTestFramework):
# Check that with invalid rpcallowip, we are denied
self.run_allowip_test([self.non_loopback_ip], self.non_loopback_ip, self.defaultport)
if self.options.usecli:
self.log.info("Skip negative IP test with CLI, because the CLI can not throw the tested exception type")
return
assert_raises_rpc_error(-342, "non-JSON HTTP response with '403 Forbidden' from server", self.run_allowip_test, ['1.1.1.1'], self.non_loopback_ip, self.defaultport)
if __name__ == '__main__':

View File

@@ -24,9 +24,11 @@ from collections.abc import Iterable
from pathlib import Path
from .authproxy import (
AuthServiceProxy,
JSONRPCException,
serialization_fallback,
)
from . import coverage
from .messages import NODE_P2P_V2
from .p2p import P2P_SERVICES, P2P_SUBVERSION
from .util import (
@@ -36,8 +38,7 @@ from .util import (
append_config,
delete_cookie_file,
get_auth_cookie,
get_rpc_proxy,
rpc_url,
rpc_port,
wait_until_helper_internal,
p2p_port,
tor_port,
@@ -76,6 +77,9 @@ class ErrorMatch(Enum):
PARTIAL_REGEX = 3
RPCConnectionType = Enum("RPCConnectionType", ["AUTO", "AUTHPROXY", "CLI"])
class TestNode():
"""A class for representing a bitcoind node under test.
@@ -185,11 +189,7 @@ class TestNode():
self.args.append("-v2transport=0")
# if v2transport is requested via global flag but not supported for node version, ignore it
self.cli = TestNodeCLI(
binaries,
self.datadir_path,
self.rpc_timeout // 2, # timeout identical to the one used in self._rpc
)
self.cli = None
self.use_cli = use_cli
self.start_perf = start_perf
@@ -197,7 +197,7 @@ class TestNode():
self.process = None
self.rpc_connected = False
self._rpc = None # Should usually not be accessed directly in tests to allow for --usecli mode
self.reuse_http_connections = True # Must be set before calling get_rpc_proxy() i.e. before restarting node
self.reuse_http_connections = True # Must be set before create_new_rpc_connection(), i.e. before restarting node
self.url = None
self.log = logging.getLogger('TestFramework.node%d' % i)
# Cache perf subprocesses here by their data output filename.
@@ -301,6 +301,36 @@ class TestNode():
if self.start_perf:
self._start_perf()
def create_new_rpc_connection(self, *, mode="AUTO", client_timeout=None):
"""Create an additional RPC connection, likely to be used in a new thread."""
mode = RPCConnectionType[mode]
if mode == RPCConnectionType.AUTO:
mode = RPCConnectionType.CLI if self.use_cli else RPCConnectionType.AUTHPROXY
client_timeout = client_timeout or (self.rpc_timeout // 2) # Shorter timeout to allow for one retry in case of ETIMEDOUT
host = "127.0.0.1"
port = rpc_port(self.index)
if self.rpchost:
parts = self.rpchost.split(":")
if len(parts) == 2:
host, port = parts
else:
host = self.rpchost
if mode == RPCConnectionType.AUTHPROXY:
rpc_u, rpc_p = get_auth_cookie(self.datadir_path, self.chain)
url = f"http://{rpc_u}:{rpc_p}@{host}:{port}"
proxy = AuthServiceProxy(url, timeout=int(client_timeout))
coverage_logfile = coverage.get_filename(self.coverage_dir, self.index) if self.coverage_dir else None
rpc = coverage.AuthServiceProxyWrapper(proxy, url, coverage_logfile)
rpc.auth_service_proxy_instance.reuse_http_connections = self.reuse_http_connections
return rpc
else: # mode==CLI
return TestNodeCLI(self.binaries)(
f"-datadir={self.datadir_path}",
f"-rpcclienttimeout={client_timeout}",
f"-rpcconnect={host}",
f"-rpcport={port}",
)
def wait_for_rpc_connection(self, *, wait_for_import=True):
"""Sets up an RPC connection to the bitcoind process. Returns False if unable to connect."""
# Poll at a rate of four times per second
@@ -322,13 +352,7 @@ class TestNode():
raise FailedToStartError(self._node_msg(
f'bitcoind exited with status {self.process.returncode} during initialization. {str_error}'))
try:
rpc = get_rpc_proxy(
rpc_url(self.datadir_path, self.index, self.chain, self.rpchost),
self.index,
timeout=self.rpc_timeout // 2, # Shorter timeout to allow for one retry in case of ETIMEDOUT
coveragedir=self.coverage_dir,
)
rpc.auth_service_proxy_instance.reuse_http_connections = self.reuse_http_connections
rpc = self.create_new_rpc_connection(mode="AUTHPROXY")
rpc.getblockcount()
# If the call to getblockcount() succeeds then the RPC connection is up
if self.version_is_at_least(190000) and wait_for_import:
@@ -355,6 +379,7 @@ class TestNode():
self.log.debug("RPC successfully started")
# Set rpc_connected even if we are in use_cli mode so that we know we can call self.stop() if needed.
self.rpc_connected = True
self.cli = self.create_new_rpc_connection(mode="CLI")
if self.use_cli:
return
self._rpc = rpc
@@ -958,18 +983,16 @@ def arg_to_cli(arg):
class TestNodeCLI():
"""Interface to bitcoin-cli for an individual node"""
def __init__(self, binaries, datadir, rpc_timeout):
def __init__(self, binaries):
self.options = []
self.binaries = binaries
self.datadir = datadir
self.rpc_timeout = rpc_timeout
self.input = None
self.log = logging.getLogger('TestFramework.bitcoincli')
def __call__(self, *options, input=None):
# TestNodeCLI is callable with bitcoin-cli command-line options
cli = TestNodeCLI(self.binaries, self.datadir, self.rpc_timeout)
cli.options = [str(o) for o in options]
cli = TestNodeCLI(self.binaries)
cli.options = self.options + [str(o) for o in options]
cli.input = input
return cli
@@ -989,10 +1012,7 @@ class TestNodeCLI():
"""Run bitcoin-cli command. Deserializes returned string as python object."""
pos_args = [arg_to_cli(arg) for arg in args]
named_args = [key + "=" + arg_to_cli(value) for (key, value) in kwargs.items() if value is not None]
p_args = self.binaries.rpc_argv() + [
f"-datadir={self.datadir}",
f"-rpcclienttimeout={int(self.rpc_timeout)}",
] + self.options
p_args = self.binaries.rpc_argv() + self.options
if named_args:
p_args += ["-named"]
base_arg_pos = len(p_args)

View File

@@ -20,8 +20,7 @@ import shlex
import time
import types
from . import coverage
from .authproxy import AuthServiceProxy, JSONRPCException
from .authproxy import JSONRPCException
from .descriptors import descsum_create
from collections.abc import Callable
from typing import Optional, Union
@@ -478,32 +477,6 @@ class PortSeed:
# Must be initialized with a unique integer for each process
n = None
def get_rpc_proxy(url: str, node_number: int, *, timeout: Optional[int]=None, coveragedir: Optional[str]=None) -> coverage.AuthServiceProxyWrapper:
"""
Args:
url: URL of the RPC server to call
node_number: the node number (or id) that this calls to
Kwargs:
timeout: HTTP timeout in seconds
coveragedir: Directory
Returns:
AuthServiceProxy. convenience object for making RPC calls.
"""
proxy_kwargs = {}
if timeout is not None:
proxy_kwargs['timeout'] = int(timeout)
proxy = AuthServiceProxy(url, **proxy_kwargs)
coverage_logfile = coverage.get_filename(coveragedir, node_number) if coveragedir else None
return coverage.AuthServiceProxyWrapper(proxy, url, coverage_logfile)
def p2p_port(n):
assert n <= MAX_NODES
return PORT_MIN + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES)
@@ -517,19 +490,6 @@ def tor_port(n):
return p2p_port(n) + PORT_RANGE * 2
def rpc_url(datadir, i, chain, rpchost):
rpc_u, rpc_p = get_auth_cookie(datadir, chain)
host = '127.0.0.1'
port = rpc_port(i)
if rpchost:
parts = rpchost.split(':')
if len(parts) == 2:
host, port = parts
else:
host = rpchost
return "http://%s:%s@%s:%d" % (rpc_u, rpc_p, host, int(port))
# Node functions
################

View File

@@ -69,7 +69,7 @@ class SignetMinerTest(BitcoinTestFramework):
n_blocks = node.getblockcount()
base_dir = self.config["environment"]["SRCDIR"]
signet_miner_path = os.path.join(base_dir, "contrib", "signet", "miner")
rpc_argv = node.binaries.rpc_argv() + [f"-datadir={node.cli.datadir}"]
rpc_argv = node.binaries.rpc_argv() + [f"-datadir={node.datadir_path}"]
util_argv = node.binaries.util_argv() + ["grind"]
subprocess.run([
sys.executable,
@@ -89,7 +89,7 @@ class SignetMinerTest(BitcoinTestFramework):
n_blocks = node.getblockcount()
base_dir = self.config["environment"]["SRCDIR"]
signet_miner_path = os.path.join(base_dir, "contrib", "signet", "miner")
rpc_argv = node.binaries.rpc_argv() + [f"-datadir={node.cli.datadir}"]
rpc_argv = node.binaries.rpc_argv() + [f"-datadir={node.datadir_path}"]
util_argv = node.binaries.util_argv() + ["grind"]
base_cmd = [
sys.executable,

View File

@@ -20,7 +20,6 @@ from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
ensure_for,
get_rpc_proxy,
)
got_loading_error = False
@@ -328,7 +327,7 @@ class MultiWalletTest(BitcoinTestFramework):
self.log.info("Concurrent wallet loading")
threads = []
for _ in range(3):
n = node.cli if self.options.usecli else get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir)
n = node.create_new_rpc_connection()
t = Thread(target=test_load_unload, args=(n, wallet_names[2]))
t.start()
threads.append(t)