From 2930caa7fe501f1b2a92213e6c9ad459bc2ae019 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 26 Nov 2024 11:51:41 -0500 Subject: [PATCH] test: Support BITCOIN_CMD environment variable Support new BITCOIN_CMD environment variable in functional test to be able to test the new bitcoin wrapper executable and run other commands through it instead of calling them directly. --- contrib/signet/miner | 7 +- .../test_framework/test_framework.py | 65 +++++++++++++------ test/functional/test_framework/test_node.py | 19 +++--- test/functional/tool_signet_miner.py | 7 +- test/functional/tool_wallet.py | 7 +- 5 files changed, 69 insertions(+), 36 deletions(-) diff --git a/contrib/signet/miner b/contrib/signet/miner index 3c90fe96a1..ce55c3fd2b 100755 --- a/contrib/signet/miner +++ b/contrib/signet/miner @@ -9,6 +9,7 @@ import logging import math import os import re +import shlex import struct import sys import time @@ -86,7 +87,7 @@ def finish_block(block, signet_solution, grind_cmd): block.solve() else: headhex = CBlockHeader.serialize(block).hex() - cmd = grind_cmd.split(" ") + [headhex] + cmd = shlex.split(grind_cmd) + [headhex] newheadhex = subprocess.run(cmd, stdout=subprocess.PIPE, input=b"", check=True).stdout.strip() newhead = from_hex(CBlockHeader(), newheadhex.decode('utf8')) block.nNonce = newhead.nNonce @@ -479,7 +480,7 @@ def do_calibrate(args): header.nTime = i header.nNonce = 0 headhex = header.serialize().hex() - cmd = args.grind_cmd.split(" ") + [headhex] + cmd = shlex.split(args.grind_cmd) + [headhex] newheadhex = subprocess.run(cmd, stdout=subprocess.PIPE, input=b"", check=True).stdout.strip() avg = (time.time() - start) * 1.0 / TRIALS @@ -549,7 +550,7 @@ def main(): args = parser.parse_args(sys.argv[1:]) - args.bcli = lambda *a, input=b"", **kwargs: bitcoin_cli(args.cli.split(" "), list(a), input=input, **kwargs) + args.bcli = lambda *a, input=b"", **kwargs: bitcoin_cli(shlex.split(args.cli), list(a), input=input, **kwargs) if hasattr(args, "address") and hasattr(args, "descriptor"): args.derived_addresses = {} diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 7e8c40cf16..1a76ef9b1d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -13,11 +13,13 @@ import platform import pdb import random import re +import shlex import shutil import subprocess import sys import tempfile import time +import types from .address import create_deterministic_address_bcrt1_p2tr_op_true from .authproxy import JSONRPCException @@ -56,6 +58,33 @@ class SkipTest(Exception): self.message = message +class BitcoinEnv: + def __init__(self, paths, bin_path=None): + self.paths = paths + self.bin_path = bin_path + + def daemon_args(self): + return self.args("daemon", "bitcoind") + + def rpc_args(self): + # Add -nonamed because "bitcoin rpc" enables -named by default, but bitcoin-cli doesn't + return self.args("rpc", "bitcoincli") + ["-nonamed"] + + def util_args(self): + return self.args("util", "bitcoinutil") + + def wallet_args(self): + return self.args("wallet", "bitcoinwallet") + + def args(self, command, path_attr): + if self.bin_path is not None: + return [os.path.join(self.bin_path, os.path.basename(getattr(self.paths, path_attr)))] + elif self.paths.bitcoin_cmd is not None: + return self.paths.bitcoin_cmd + [command] + else: + return [getattr(self.paths, path_attr)] + + class BitcoinTestMetaClass(type): """Metaclass for BitcoinTestFramework. @@ -222,6 +251,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): config = configparser.ConfigParser() config.read_file(open(self.options.configfile)) self.config = config + self.paths = self.get_binary_paths() if self.options.v1transport: self.options.v2transport=False @@ -245,9 +275,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): PortSeed.n = self.options.port_seed - def set_binary_paths(self): - """Update self.options with the paths of all binaries from environment variables or their default values""" + def get_binary_paths(self): + """Get paths of all binaries from environment variables or their default values""" + paths = types.SimpleNamespace() binaries = { "bitcoind": ("bitcoind", "BITCOIND"), "bitcoin-cli": ("bitcoincli", "BITCOINCLI"), @@ -260,7 +291,12 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): "src", binary + self.config["environment"]["EXEEXT"], ) - setattr(self.options, attribute_name, os.getenv(env_variable_name, default=default_filename)) + setattr(paths, attribute_name, os.getenv(env_variable_name, default=default_filename)) + + # BITCOIN_CMD environment variable can be specified to invoke bitcoin + # binary wrapper binary instead of other executables. + paths.bitcoin_cmd = shlex.split(os.getenv("BITCOIN_CMD", "")) or None + return paths def setup(self): """Call this method to start up the test framework object with options set.""" @@ -271,8 +307,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): config = self.config - self.set_binary_paths() - os.environ['PATH'] = os.pathsep.join([ os.path.join(config['environment']['BUILDDIR'], 'src'), os.path.join(config['environment']['BUILDDIR'], 'src', 'qt'), os.environ['PATH'] @@ -482,14 +516,14 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): group.add_argument("--legacy-wallet", action='store_const', const=False, **kwargs, help="Run test using legacy wallets", dest='descriptors') - def add_nodes(self, num_nodes: int, extra_args=None, *, rpchost=None, binary=None, binary_cli=None, versions=None): + def add_nodes(self, num_nodes: int, extra_args=None, *, rpchost=None, versions=None): """Instantiate TestNode objects. Should only be called once after the nodes have been specified in set_test_params().""" - def get_bin_from_version(version, bin_name, bin_default): + def bin_path_from_version(version): if not version: - return bin_default + return None if version > 219999: # Starting at client version 220000 the first two digits represent # the major version, e.g. v22.0 instead of v0.22.0. @@ -507,7 +541,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): ), ), 'bin', - bin_name, ) if self.bind_to_localhost_only: @@ -522,15 +555,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): extra_args[i] = extra_args[i] + ["-whitelist=noban,in,out@127.0.0.1"] if versions is None: versions = [None] * num_nodes - if binary is None: - binary = [get_bin_from_version(v, 'bitcoind', self.options.bitcoind) for v in versions] - if binary_cli is None: - binary_cli = [get_bin_from_version(v, 'bitcoin-cli', self.options.bitcoincli) for v in versions] + bin_path = [bin_path_from_version(v) for v in versions] assert_equal(len(extra_confs), num_nodes) assert_equal(len(extra_args), num_nodes) assert_equal(len(versions), num_nodes) - assert_equal(len(binary), num_nodes) - assert_equal(len(binary_cli), num_nodes) + assert_equal(len(bin_path), num_nodes) for i in range(num_nodes): args = list(extra_args[i]) test_node_i = TestNode( @@ -540,8 +569,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): rpchost=rpchost, timewait=self.rpc_timeout, timeout_factor=self.options.timeout_factor, - bitcoind=binary[i], - bitcoin_cli=binary_cli[i], + env=BitcoinEnv(self.paths, bin_path[i]), version=versions[i], coverage_dir=self.options.coveragedir, cwd=self.options.tmpdir, @@ -857,8 +885,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): rpchost=None, timewait=self.rpc_timeout, timeout_factor=self.options.timeout_factor, - bitcoind=self.options.bitcoind, - bitcoin_cli=self.options.bitcoincli, + env=BitcoinEnv(self.paths), coverage_dir=None, cwd=self.options.tmpdir, descriptors=self.options.descriptors, diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index ff94e86468..93dd82c5e9 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -75,7 +75,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, v2transport=False): + def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, env, 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 @@ -91,7 +91,7 @@ class TestNode(): self.chain = chain self.rpchost = rpchost self.rpc_timeout = timewait - self.binary = bitcoind + self.env = env self.coverage_dir = coverage_dir self.cwd = cwd self.descriptors = descriptors @@ -108,8 +108,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.binary, + self.args = self.env.daemon_args() + [ f"-datadir={self.datadir_path}", "-logtimemicros", "-debug", @@ -148,7 +147,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(bitcoin_cli, self.datadir_path) + self.cli = TestNodeCLI(env, self.datadir_path) self.use_cli = use_cli self.start_perf = start_perf @@ -866,16 +865,16 @@ def arg_to_cli(arg): class TestNodeCLI(): """Interface to bitcoin-cli for an individual node""" - def __init__(self, binary, datadir): + def __init__(self, env, datadir): self.options = [] - self.binary = binary + self.env = env self.datadir = datadir 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.binary, self.datadir) + cli = TestNodeCLI(self.env, self.datadir) cli.options = [str(o) for o in options] cli.input = input return cli @@ -896,7 +895,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 = [str(key) + "=" + arg_to_cli(value) for (key, value) in kwargs.items()] - p_args = [self.binary, f"-datadir={self.datadir}"] + self.options + p_args = self.env.rpc_args() + [f"-datadir={self.datadir}"] + self.options if named_args: p_args += ["-named"] if clicommand is not None: @@ -912,7 +911,7 @@ class TestNodeCLI(): code, message = match.groups() raise JSONRPCException(dict(code=int(code), message=message)) # Ignore cli_stdout, raise with cli_stderr - raise subprocess.CalledProcessError(returncode, self.binary, output=cli_stderr) + raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr) try: return json.loads(cli_stdout, parse_float=decimal.Decimal) except (json.JSONDecodeError, decimal.InvalidOperation): diff --git a/test/functional/tool_signet_miner.py b/test/functional/tool_signet_miner.py index 67fb5c9f94..2311073d14 100755 --- a/test/functional/tool_signet_miner.py +++ b/test/functional/tool_signet_miner.py @@ -5,6 +5,7 @@ """Test signet miner tool""" import os.path +import shlex import subprocess import sys import time @@ -48,13 +49,15 @@ class SignetMinerTest(BitcoinTestFramework): # generate block with signet miner tool base_dir = self.config["environment"]["SRCDIR"] signet_miner_path = os.path.join(base_dir, "contrib", "signet", "miner") + rpc_args = node.env.rpc_args() + [f"-datadir={node.cli.datadir}"] + util_args = node.env.util_args() + ["grind"] subprocess.run([ sys.executable, signet_miner_path, - f'--cli={node.cli.binary} -datadir={node.cli.datadir}', + f'--cli={shlex.join(rpc_args)}', 'generate', f'--address={node.getnewaddress()}', - f'--grind-cmd={self.options.bitcoinutil} grind', + f'--grind-cmd={shlex.join(util_args)}', '--nbits=1d00ffff', f'--set-block-time={int(time.time())}', '--poolnum=99', diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 86292c0f0b..52895a514e 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -12,7 +12,10 @@ import textwrap from collections import OrderedDict -from test_framework.test_framework import BitcoinTestFramework +from test_framework.test_framework import ( + BitcoinEnv, + BitcoinTestFramework, +) from test_framework.util import ( assert_equal, assert_greater_than, @@ -44,7 +47,7 @@ class ToolWalletTest(BitcoinTestFramework): if "dump" in args and self.options.bdbro: default_args.append("-withinternalbdb") - return subprocess.Popen([self.options.bitcoinwallet] + default_args + list(args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + return subprocess.Popen(BitcoinEnv(self.paths).wallet_args() + default_args + list(args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) def assert_raises_tool_error(self, error, *args): p = self.bitcoin_wallet_process(*args)