What changed, and why it matters
This commit only changes the Bitcoin Core test framework. It adds a convenience option so automated tests can more easily start a test node with inter-process communication (IPC) enabled. It does not change the production Bitcoin Core software that users run, and there is no indication it fixes or introduces a security problem.
No security action needed. Treat as normal test-framework maintenance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch modifies test_framework/test_framework.py and test_node.py to support an extra_init test setting with ipcbind=True. When enabled, the test harness selects the multiprocess bitcoin wrapper binary with -m and appends an appropriate -ipcbind=unix argument, handling long socket paths by creating a temporary directory. This is purely test-infrastructure refactoring to reduce boilerplate in IPC functional tests.
Changed components
test/functional/test_framework/test_framework.pytest/functional/test_framework/test_node.pyInspect captured patch +46 / −14
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index bc722ab7..03c00bec 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -76,9 +76,9 @@ class Binaries:
self.paths = paths
self.bin_dir = bin_dir
- def node_argv(self):
+ def node_argv(self, **kwargs):
"Return argv array that should be used to invoke bitcoind"
- return self._argv("node", self.paths.bitcoind)
+ return self._argv("node", self.paths.bitcoind, **kwargs)
def rpc_argv(self):
"Return argv array that should be used to invoke bitcoin-cli"
@@ -101,16 +101,19 @@ class Binaries:
"Return argv array that should be used to invoke bitcoin-chainstate"
return self._argv("chainstate", self.paths.bitcoinchainstate)
- def _argv(self, command, bin_path):
+ def _argv(self, command, bin_path, need_ipc=False):
"""Return argv array that should be used to invoke the command. It
- either uses the bitcoin wrapper executable (if BITCOIN_CMD is set), or
- the direct binary path (bitcoind, etc). When bin_dir is set (by tests
- calling binaries from previous releases) it always uses the direct
- path."""
+ either uses the bitcoin wrapper executable (if BITCOIN_CMD is set or
+ need_ipc is True), or the direct binary path (bitcoind, etc). When
+ bin_dir is set (by tests calling binaries from previous releases) it
+ always uses the direct path."""
if self.bin_dir is not None:
return [os.path.join(self.bin_dir, os.path.basename(bin_path))]
- elif self.paths.bitcoin_cmd is not None:
- return self.paths.bitcoin_cmd + [command]
+ elif self.paths.bitcoin_cmd is not None or need_ipc:
+ # If the current test needs IPC functionality, use the bitcoin
+ # wrapper binary and append -m so it calls multiprocess binaries.
+ bitcoin_cmd = self.paths.bitcoin_cmd or [self.paths.bitcoin_bin]
+ return bitcoin_cmd + (["-m"] if need_ipc else []) + [command]
else:
return [bin_path]
@@ -158,6 +161,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
self.noban_tx_relay: bool = False
self.nodes: list[TestNode] = []
self.extra_args = None
+ self.extra_init = None
self.network_thread = None
self.rpc_timeout = 60 # Wait for up to 60 seconds for the RPC server to respond
self.supports_cli = True
@@ -553,15 +557,15 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
bin_dirs.append(bin_dir)
+ extra_init = [{}] * num_nodes if self.extra_init is None else self.extra_init # type: ignore[var-annotated]
+ assert_equal(len(extra_init), num_nodes)
assert_equal(len(extra_confs), num_nodes)
assert_equal(len(extra_args), num_nodes)
assert_equal(len(versions), num_nodes)
assert_equal(len(bin_dirs), num_nodes)
for i in range(num_nodes):
args = list(extra_args[i])
- test_node_i = TestNode(
- i,
- get_datadir_path(self.options.tmpdir, i),
+ init = dict(
chain=self.chain,
rpchost=rpchost,
timewait=self.rpc_timeout,
@@ -578,6 +582,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
v2transport=self.options.v2transport,
uses_wallet=self.uses_wallet,
)
+ init.update(extra_init[i])
+ test_node_i = TestNode(
+ i,
+ get_datadir_path(self.options.tmpdir, i),
+ **init)
self.nodes.append(test_node_i)
if not test_node_i.version_is_at_least(170000):
# adjust conf for pre 17
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 3b081463..66536627 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -11,6 +11,7 @@ from enum import Enum
import json
import logging
import os
+import pathlib
import platform
import re
import subprocess
@@ -19,6 +20,7 @@ import time
import urllib.parse
import collections
import shlex
+import shutil
import sys
from pathlib import Path
@@ -52,6 +54,13 @@ CLI_MAX_ARG_SIZE = 131071 # many systems have a 128kb limit per arg (MAX_ARG_STR
NULL_BLK_XOR_KEY = bytes([0] * NUM_XOR_BYTES)
BITCOIN_PID_FILENAME_DEFAULT = "bitcoind.pid"
+if sys.platform.startswith("linux"):
+ UNIX_PATH_MAX = 108 # includes the trailing NUL
+elif sys.platform.startswith(("darwin", "freebsd", "netbsd", "openbsd")):
+ UNIX_PATH_MAX = 104
+else: # safest portable value
+ UNIX_PATH_MAX = 92
+
class FailedToStartError(Exception):
"""Raised when a node fails to start correctly."""
@@ -77,7 +86,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, binaries, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, v2transport=False, uses_wallet=False):
+ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, binaries, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, v2transport=False, uses_wallet=False, ipcbind=False):
"""
Kwargs:
start_perf (bool): If True, begin profiling the node with `perf` as soon as
@@ -109,7 +118,7 @@ class TestNode():
# Configuration for logging is set as command-line args rather than in the bitcoin.conf file.
# This means that starting a bitcoind using the temp dir to debug a failed test won't
# spam debug.log.
- self.args = self.binaries.node_argv() + [
+ self.args = self.binaries.node_argv(need_ipc=ipcbind) + [
f"-datadir={self.datadir_path}",
"-logtimemicros",
"-debug",
@@ -121,6 +130,17 @@ class TestNode():
if uses_wallet is not None and not uses_wallet:
self.args.append("-disablewallet")
+ self.ipc_tmp_dir = None
+ if ipcbind:
+ self.ipc_socket_path = self.chain_path / "node.sock"
+ if len(os.fsencode(self.ipc_socket_path)) < UNIX_PATH_MAX:
+ self.args.append("-ipcbind=unix")
+ else:
+ # Work around default CI path exceeding maximum socket path length.
+ self.ipc_tmp_dir = pathlib.Path(tempfile.mkdtemp(prefix="test-ipc-"))
+ self.ipc_socket_path = self.ipc_tmp_dir / "node.sock"
+ self.args.append(f"-ipcbind=unix:{self.ipc_socket_path}")
+
# Use valgrind, expect for previous release binaries
if use_valgrind and version is None:
default_suppressions_file = Path(__file__).parents[3] / "contrib" / "valgrind.supp"
@@ -208,6 +228,9 @@ class TestNode():
# this destructor is called.
print(self._node_msg("Cleaning up leftover process"), file=sys.stderr)
self.process.kill()
+ if self.ipc_tmp_dir:
+ print(self._node_msg(f"Cleaning up ipc directory {str(self.ipc_tmp_dir)!r}"))
+ shutil.rmtree(self.ipc_tmp_dir)
def __getattr__(self, name):
"""Dispatches any unrecognised messages to the RPC connection or a CLI instance."""
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.