From 4c95dd000c66f5a111c46775a1165da4d277b118 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Jan 2023 16:46:05 +0100 Subject: [PATCH 01/10] Merge bitcoin/bitcoin#26226: Bump minimum python version to 3.7 fa8fe5b69669b4d86e0d0970d68502abee8785f3 scripted-diff: Use new python 3.7 keywords (MarcoFalke) fa2a23548aa9656e397189d8da844657709fb831 Revert "contrib: Fix capture_output in getcoins.py" (MarcoFalke) dddd462137a85225955d4c2bcdb52e1e4235bff0 Bump minimum python version to 3.7 (MarcoFalke) Pull request description: While there is nothing that requires a bump, it may require less maintenance to drop python3.6 support. Python3.7 is available through the package manager on all currently supported operating systems. ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/26226/commits/fa8fe5b69669b4d86e0d0970d68502abee8785f3 hebasto: ACK fa8fe5b69669b4d86e0d0970d68502abee8785f3 Tree-SHA512: f6e080d8751948bb0e01c87be601363158f345e8037b70ce7e1bc507c611eb61600e4f24f1d2f8a6e7e44877ab09319302869e33ce8118c4c4f71fc89c0a1198 --- ci/test/04_install.sh | 3 +++ contrib/devtools/clang-format-diff.py | 2 +- contrib/devtools/gen-manpages.py | 4 ++-- contrib/devtools/test-security-check.py | 2 +- contrib/devtools/test-symbol-check.py | 2 +- contrib/macdeploy/macdeployqtplus | 2 +- test/functional/rpc_users.py | 4 ++-- test/functional/test_framework/test_node.py | 2 +- test/functional/test_runner.py | 4 ++-- test/functional/tool_wallet.py | 2 +- test/fuzz/test_runner.py | 10 +++++----- test/lint/lint-assertions.py | 2 +- test/lint/lint-circular-dependencies.py | 4 ++-- test/lint/lint-git-commit-check.py | 6 +++--- test/lint/lint-includes.py | 14 +++++++------- test/lint/lint-locale-dependence.py | 2 +- test/lint/lint-logs.py | 2 +- .../lint/lint-python-mutable-default-parameters.py | 2 +- test/lint/lint-python-utf8-encoding.py | 6 +++--- test/lint/lint-shell.py | 2 +- test/lint/lint-submodule.py | 2 +- test/lint/lint-tests.py | 2 +- test/lint/lint-whitespace.py | 4 ++-- test/util/test_runner.py | 2 +- 24 files changed, 45 insertions(+), 42 deletions(-) diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index bf14526ccad3..87d2ed9b776c 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -81,6 +81,9 @@ if [[ $DOCKER_NAME_TAG == *centos* ]]; then CI_EXEC_ROOT yum -y install epel-release CI_EXEC_ROOT yum -y install "$DOCKER_PACKAGES" "$PACKAGES" elif [ "$CI_USE_APT_INSTALL" != "no" ]; then + if [[ -n "${APPEND_APT_SOURCES_LIST}" ]]; then + CI_EXEC_ROOT echo "${APPEND_APT_SOURCES_LIST}" >> /etc/apt/sources.list + fi ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get update ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get install --no-install-recommends --no-upgrade -y "$PACKAGES" "$DOCKER_PACKAGES" fi diff --git a/contrib/devtools/clang-format-diff.py b/contrib/devtools/clang-format-diff.py index 98eee67f4300..420bf7ff3300 100755 --- a/contrib/devtools/clang-format-diff.py +++ b/contrib/devtools/clang-format-diff.py @@ -146,7 +146,7 @@ def main(): stdout=subprocess.PIPE, stderr=None, stdin=subprocess.PIPE, - universal_newlines=True) + text=True) stdout, stderr = p.communicate() if p.returncode != 0: sys.exit(p.returncode) diff --git a/contrib/devtools/gen-manpages.py b/contrib/devtools/gen-manpages.py index e394e52df91c..59678e7e5b2b 100755 --- a/contrib/devtools/gen-manpages.py +++ b/contrib/devtools/gen-manpages.py @@ -23,7 +23,7 @@ # If not otherwise specified, get top directory from git. topdir = os.getenv('TOPDIR') if not topdir: - r = subprocess.run([git, 'rev-parse', '--show-toplevel'], stdout=subprocess.PIPE, check=True, universal_newlines=True) + r = subprocess.run([git, 'rev-parse', '--show-toplevel'], stdout=subprocess.PIPE, check=True, text=True) topdir = r.stdout.rstrip() # Get input and output directories. @@ -36,7 +36,7 @@ for relpath in BINARIES: abspath = os.path.join(builddir, relpath) try: - r = subprocess.run([abspath, "--version"], stdout=subprocess.PIPE, check=True, universal_newlines=True) + r = subprocess.run([abspath, "--version"], stdout=subprocess.PIPE, check=True, text=True) except IOError: print(f'{abspath} not found or not an executable', file=sys.stderr) sys.exit(1) diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index d64d08fb732f..74e63f7b465b 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -41,7 +41,7 @@ def env_flags() -> List[str]: def call_security_check(cxx, source, executable, options): subprocess.run([*cxx,source,'-o',executable] + env_flags() + options, check=True) - p = subprocess.run([os.path.join(os.path.dirname(__file__), 'security-check.py'), executable], stdout=subprocess.PIPE, universal_newlines=True) + p = subprocess.run([os.path.join(os.path.dirname(__file__), 'security-check.py'), executable], stdout=subprocess.PIPE, text=True) return (p.returncode, p.stdout.rstrip()) def get_arch(cxx, source, executable): diff --git a/contrib/devtools/test-symbol-check.py b/contrib/devtools/test-symbol-check.py index 92966000406b..a562895a4e6e 100755 --- a/contrib/devtools/test-symbol-check.py +++ b/contrib/devtools/test-symbol-check.py @@ -23,7 +23,7 @@ def call_symbol_check(cxx: List[str], source, executable, options): env_flags += filter(None, os.environ.get(var, '').split(' ')) subprocess.run([*cxx,source,'-o',executable] + env_flags + options, check=True) - p = subprocess.run([os.path.join(os.path.dirname(__file__), 'symbol-check.py'), executable], stdout=subprocess.PIPE, universal_newlines=True) + p = subprocess.run([os.path.join(os.path.dirname(__file__), 'symbol-check.py'), executable], stdout=subprocess.PIPE, text=True) os.remove(source) os.remove(executable) return (p.returncode, p.stdout.rstrip()) diff --git a/contrib/macdeploy/macdeployqtplus b/contrib/macdeploy/macdeployqtplus index 9042a06066c7..f86b6f760f66 100755 --- a/contrib/macdeploy/macdeployqtplus +++ b/contrib/macdeploy/macdeployqtplus @@ -185,7 +185,7 @@ def getFrameworks(binaryPath: str, verbose: int) -> List[FrameworkInfo]: objdump = os.getenv("OBJDUMP", "objdump") if verbose: print(f"Inspecting with {objdump}: {binaryPath}") - output = run([objdump, "--macho", "--dylibs-used", binaryPath], stdout=PIPE, stderr=PIPE, universal_newlines=True) + output = run([objdump, "--macho", "--dylibs-used", binaryPath], stdout=PIPE, stderr=PIPE, text=True) if output.returncode != 0: sys.stderr.write(output.stderr) sys.stderr.flush() diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index e8fe872bf5d5..1cd1ac9e8f9c 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -52,13 +52,13 @@ def setup_chain(self): # Generate RPCAUTH with specified password self.rt2password = "8/F3uMDw4KSEbw96U3CA1C4X05dkHDN2BPFjTgZW4KI=" - p = subprocess.Popen([sys.executable, gen_rpcauth, 'rt2', self.rt2password], stdout=subprocess.PIPE, universal_newlines=True) + p = subprocess.Popen([sys.executable, gen_rpcauth, 'rt2', self.rt2password], stdout=subprocess.PIPE, text=True) lines = p.stdout.read().splitlines() rpcauth2 = lines[1] # Generate RPCAUTH without specifying password self.user = ''.join(SystemRandom().choice(string.ascii_letters + string.digits) for _ in range(10)) - p = subprocess.Popen([sys.executable, gen_rpcauth, self.user], stdout=subprocess.PIPE, universal_newlines=True) + p = subprocess.Popen([sys.executable, gen_rpcauth, self.user], stdout=subprocess.PIPE, text=True) lines = p.stdout.read().splitlines() rpcauth3 = lines[1] self.password = lines[3] diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index a93a05ffb66a..0970596cfcf2 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -857,7 +857,7 @@ def send_cli(self, clicommand=None, *args, **kwargs): p_args += [clicommand] p_args += pos_args + named_args self.log.debug("Running dash-cli {}".format(p_args[2:])) - process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) cli_stdout, cli_stderr = process.communicate(input=self.input) returncode = process.poll() if returncode: diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index fcbc405d2d9b..1de960501d32 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -643,7 +643,7 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, attempts=1, enab combined_logs_args = [sys.executable, os.path.join(tests_dir, 'combine_logs.py'), testdir] if BOLD[0]: combined_logs_args += ['--color'] - combined_logs, _ = subprocess.Popen(combined_logs_args, universal_newlines=True, stdout=subprocess.PIPE).communicate() + combined_logs, _ = subprocess.Popen(combined_logs_args, text=True, stdout=subprocess.PIPE).communicate() print("\n".join(deque(combined_logs.splitlines(), combined_logs_len))) if failfast: @@ -730,7 +730,7 @@ def get_next(self): self.jobs.append((test, time.time(), subprocess.Popen([sys.executable, self.tests_dir + test_argv[0]] + test_argv[1:] + self.flags + portseed_arg + tmpdir_arg, - universal_newlines=True, + text=True, stdout=log_stdout, stderr=log_stderr), testdir, diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 1fcd2aa16e2a..c202d44f1b52 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -32,7 +32,7 @@ def dash_wallet_process(self, *args): if self.options.descriptors and 'create' in args: default_args.append('-descriptors') - return subprocess.Popen([self.options.bitcoinwallet] + default_args + list(args), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + return subprocess.Popen([self.options.bitcoinwallet] + 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.dash_wallet_process(*args) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index add9145da517..399d9209e280 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -155,7 +155,7 @@ def main(): timeout=20, check=True, stderr=subprocess.PIPE, - universal_newlines=True, + text=True, ).stderr if "libFuzzer" not in help_output: logging.error("Must be built with libFuzzer") @@ -213,7 +213,7 @@ def job(command, t): env=get_fuzz_env(target=t, source_dir=src_dir), check=True, stderr=subprocess.PIPE, - universal_newlines=True, + text=True, ).stderr)) futures = [] @@ -263,7 +263,7 @@ def job(t, args): env=get_fuzz_env(target=t, source_dir=src_dir), check=True, stderr=subprocess.PIPE, - universal_newlines=True, + text=True, ).stderr logging.debug(output) @@ -298,7 +298,7 @@ def job(t, args): args, env=get_fuzz_env(target=t, source_dir=src_dir), stderr=subprocess.PIPE, - universal_newlines=True, + text=True, ) output += result.stderr return output, result @@ -334,7 +334,7 @@ def parse_test_list(*, fuzz_bin, source_dir): **get_fuzz_env(target="", source_dir=source_dir) }, stdout=subprocess.PIPE, - universal_newlines=True, + text=True, check=True, ).stdout.splitlines() return test_list_all diff --git a/test/lint/lint-assertions.py b/test/lint/lint-assertions.py index 7731dc7901da..76e16147064b 100755 --- a/test/lint/lint-assertions.py +++ b/test/lint/lint-assertions.py @@ -12,7 +12,7 @@ def git_grep(params: [], error_msg: ""): try: - output = subprocess.check_output(["git", "grep", *params], universal_newlines=True, encoding="utf8") + output = subprocess.check_output(["git", "grep", *params], text=True, encoding="utf8") print(error_msg) print(output) return 1 diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 53f961367958..f8be6bdee67f 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -75,14 +75,14 @@ def main(): os.chdir(CODE_DIR) files = subprocess.check_output( ['git', 'ls-files', '--', '*.h', '*.cpp'], - universal_newlines=True, + text=True, ).splitlines() command = [sys.executable, "../contrib/devtools/circular-dependencies.py", *files] dependencies_output = subprocess.run( command, stdout=subprocess.PIPE, - universal_newlines=True, + text=True, ) for dependency_str in dependencies_output.stdout.rstrip().split("\n"): diff --git a/test/lint/lint-git-commit-check.py b/test/lint/lint-git-commit-check.py index 001e48fa0693..ad38617e966e 100755 --- a/test/lint/lint-git-commit-check.py +++ b/test/lint/lint-git-commit-check.py @@ -42,17 +42,17 @@ def main(): commit_range = "HEAD~" + args.prev_commits + "...HEAD" else: # This assumes that the target branch of the pull request will be develop. - merge_base = check_output(["git", "merge-base", "HEAD", "develop"], universal_newlines=True, encoding="utf8").rstrip("\n") + merge_base = check_output(["git", "merge-base", "HEAD", "develop"], text=True, encoding="utf8").rstrip("\n") commit_range = merge_base + "..HEAD" else: commit_range = os.getenv("COMMIT_RANGE") if commit_range == "SKIP_EMPTY_NOT_A_PR": sys.exit(0) - commit_hashes = check_output(["git", "log", commit_range, "--format=%H"], universal_newlines=True, encoding="utf8").splitlines() + commit_hashes = check_output(["git", "log", commit_range, "--format=%H"], text=True, encoding="utf8").splitlines() for hash in commit_hashes: - commit_info = check_output(["git", "log", "--format=%B", "-n", "1", hash], universal_newlines=True, encoding="utf8").splitlines() + commit_info = check_output(["git", "log", "--format=%B", "-n", "1", hash], text=True, encoding="utf8").splitlines() if len(commit_info) >= 2: if commit_info[1]: print(f"The subject line of commit hash {hash} is followed by a non-empty line. Subject lines should always be followed by a blank line.") diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index 5169f5fc685f..9a0cfa127dde 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -43,13 +43,13 @@ EXCLUDED_BOOST_DIRS = ["src/immer/"] def get_toplevel(): - return check_output(["git", "rev-parse", "--show-toplevel"], universal_newlines=True, encoding="utf8").rstrip("\n") + return check_output(["git", "rev-parse", "--show-toplevel"], text=True, encoding="utf8").rstrip("\n") def list_files_by_suffix(suffixes): exclude_args = [":(exclude)" + dir for dir in EXCLUDED_DIRS] - files_list = check_output(["git", "ls-files", "src"] + exclude_args, universal_newlines=True, encoding="utf8").splitlines() + files_list = check_output(["git", "ls-files", "src"] + exclude_args, text=True, encoding="utf8").splitlines() return [file for file in files_list if file.endswith(suffixes)] @@ -71,7 +71,7 @@ def find_included_cpps(): included_cpps = list() try: - included_cpps = check_output(["git", "grep", "-E", r"^#include [<\"][^>\"]+\.cpp[>\"]", "--", "*.cpp", "*.h"], universal_newlines=True, encoding="utf8").splitlines() + included_cpps = check_output(["git", "grep", "-E", r"^#include [<\"][^>\"]+\.cpp[>\"]", "--", "*.cpp", "*.h"], text=True, encoding="utf8").splitlines() except CalledProcessError as e: if e.returncode > 1: raise e @@ -86,7 +86,7 @@ def find_extra_boosts(): exclude_args = [":(exclude)" + dir for dir in EXCLUDED_BOOST_DIRS] try: - included_boosts = check_output(["git", "grep", "-E", r"^#include 1: raise e @@ -109,7 +109,7 @@ def find_quote_syntax_inclusions(): quote_syntax_inclusions = list() try: - quote_syntax_inclusions = check_output(["git", "grep", r"^#include \"", "--", "*.cpp", "*.h"] + exclude_args, universal_newlines=True, encoding="utf8").splitlines() + quote_syntax_inclusions = check_output(["git", "grep", r"^#include \"", "--", "*.cpp", "*.h"] + exclude_args, text=True, encoding="utf8").splitlines() except CalledProcessError as e: if e.returncode > 1: raise e @@ -152,13 +152,13 @@ def main(): if extra_boosts: for boost in extra_boosts: print(f"A new Boost dependency in the form of \"{boost}\" appears to have been introduced:") - print(check_output(["git", "grep", boost, "--", "*.cpp", "*.h"], universal_newlines=True, encoding="utf8")) + print(check_output(["git", "grep", boost, "--", "*.cpp", "*.h"], text=True, encoding="utf8")) exit_code = 1 # Check if Boost dependencies are no longer used for expected_boost in EXPECTED_BOOST_INCLUDES: try: - check_output(["git", "grep", "-q", r"^#include <%s>" % expected_boost, "--", "*.cpp", "*.h"], universal_newlines=True, encoding="utf8") + check_output(["git", "grep", "-q", r"^#include <%s>" % expected_boost, "--", "*.cpp", "*.h"], text=True, encoding="utf8") except CalledProcessError as e: if e.returncode > 1: raise e diff --git a/test/lint/lint-locale-dependence.py b/test/lint/lint-locale-dependence.py index 2a8266d524dc..32cef3cb1248 100755 --- a/test/lint/lint-locale-dependence.py +++ b/test/lint/lint-locale-dependence.py @@ -227,7 +227,7 @@ def find_locale_dependent_function_uses(): git_grep_output = list() try: - git_grep_output = check_output(git_grep_command, universal_newlines=True, encoding="utf8").splitlines() + git_grep_output = check_output(git_grep_command, text=True, encoding="utf8").splitlines() except CalledProcessError as e: if e.returncode > 1: raise e diff --git a/test/lint/lint-logs.py b/test/lint/lint-logs.py index aaf697467db2..de04a1aecaf5 100755 --- a/test/lint/lint-logs.py +++ b/test/lint/lint-logs.py @@ -16,7 +16,7 @@ def main(): - logs_list = check_output(["git", "grep", "--extended-regexp", r"(LogPrintLevel|LogPrintfCategory|LogPrintf?)\(", "--", "*.cpp"], universal_newlines=True, encoding="utf8").splitlines() + logs_list = check_output(["git", "grep", "--extended-regexp", r"(LogPrintLevel|LogPrintfCategory|LogPrintf?)\(", "--", "*.cpp"], text=True, encoding="utf8").splitlines() unterminated_logs = [line for line in logs_list if not re.search(r'(\\n"|/\* Continued \*/)', line)] diff --git a/test/lint/lint-python-mutable-default-parameters.py b/test/lint/lint-python-mutable-default-parameters.py index 7991e3630ba7..f3d73c90722b 100755 --- a/test/lint/lint-python-mutable-default-parameters.py +++ b/test/lint/lint-python-mutable-default-parameters.py @@ -21,7 +21,7 @@ def main(): "--", "*.py", ] - output = subprocess.run(command, stdout=subprocess.PIPE, universal_newlines=True) + output = subprocess.run(command, stdout=subprocess.PIPE, text=True) if len(output.stdout) > 0: error_msg = ( "A mutable list or dict seems to be used as default parameter value:\n\n" diff --git a/test/lint/lint-python-utf8-encoding.py b/test/lint/lint-python-utf8-encoding.py index 4f8b307c04ee..20dc5abea6c9 100755 --- a/test/lint/lint-python-utf8-encoding.py +++ b/test/lint/lint-python-utf8-encoding.py @@ -24,7 +24,7 @@ def check_fileopens(): fileopens = list() try: - fileopens = check_output(["git", "grep", r" open(", "--", "*.py"] + get_exclude_args(), universal_newlines=True, encoding="utf8").splitlines() + fileopens = check_output(["git", "grep", r" open(", "--", "*.py"] + get_exclude_args(), text=True, encoding="utf8").splitlines() except CalledProcessError as e: if e.returncode > 1: raise e @@ -38,12 +38,12 @@ def check_checked_outputs(): checked_outputs = list() try: - checked_outputs = check_output(["git", "grep", "check_output(", "--", "*.py"] + get_exclude_args(), universal_newlines=True, encoding="utf8").splitlines() + checked_outputs = check_output(["git", "grep", "check_output(", "--", "*.py"] + get_exclude_args(), text=True, encoding="utf8").splitlines() except CalledProcessError as e: if e.returncode > 1: raise e - filtered_checked_outputs = [checked_output for checked_output in checked_outputs if re.search(r"universal_newlines=True", checked_output) and not re.search(r"encoding=.(ascii|utf8|utf-8).", checked_output)] + filtered_checked_outputs = [checked_output for checked_output in checked_outputs if re.search(r"text=True", checked_output) and not re.search(r"encoding=.(ascii|utf8|utf-8).", checked_output)] return filtered_checked_outputs diff --git a/test/lint/lint-shell.py b/test/lint/lint-shell.py index c1acd618c120..4be1f297fa11 100755 --- a/test/lint/lint-shell.py +++ b/test/lint/lint-shell.py @@ -25,7 +25,7 @@ def check_shellcheck_install(): sys.exit(0) def get_files(command): - output = subprocess.run(command, stdout=subprocess.PIPE, universal_newlines=True) + output = subprocess.run(command, stdout=subprocess.PIPE, text=True) files = output.stdout.split('\n') # remove whitespace element diff --git a/test/lint/lint-submodule.py b/test/lint/lint-submodule.py index 89d4c80f559b..4d2fbf088f77 100755 --- a/test/lint/lint-submodule.py +++ b/test/lint/lint-submodule.py @@ -13,7 +13,7 @@ def main(): submodules_list = subprocess.check_output(['git', 'submodule', 'status', '--recursive'], - universal_newlines = True, encoding = 'utf8').rstrip('\n') + text = True, encoding = 'utf8').rstrip('\n') if submodules_list: print("These submodules were found, delete them:\n", submodules_list) sys.exit(1) diff --git a/test/lint/lint-tests.py b/test/lint/lint-tests.py index 849ddcb96104..1eeb7bb01452 100755 --- a/test/lint/lint-tests.py +++ b/test/lint/lint-tests.py @@ -23,7 +23,7 @@ def grep_boost_fixture_test_suite(): "src/test/**.cpp", "src/wallet/test/**.cpp", ] - return subprocess.check_output(command, universal_newlines=True, encoding="utf8") + return subprocess.check_output(command, text=True, encoding="utf8") def check_matching_test_names(test_suite_list): diff --git a/test/lint/lint-whitespace.py b/test/lint/lint-whitespace.py index 15d009b29c15..78c145bd004e 100755 --- a/test/lint/lint-whitespace.py +++ b/test/lint/lint-whitespace.py @@ -83,7 +83,7 @@ def get_diff(commit_range, check_only_code): else: what_files = ["."] - diff = check_output(["git", "diff", "-U0", commit_range, "--"] + what_files + exclude_args, universal_newlines=True, encoding="utf8") + diff = check_output(["git", "diff", "-U0", commit_range, "--"] + what_files + exclude_args, text=True, encoding="utf8") return diff @@ -96,7 +96,7 @@ def main(): commit_range = "HEAD~" + args.prev_commits + "...HEAD" else: # This assumes that the target branch of the pull request will be develop. - merge_base = check_output(["git", "merge-base", "HEAD", "develop"], universal_newlines=True, encoding="utf8").rstrip("\n") + merge_base = check_output(["git", "merge-base", "HEAD", "develop"], text=True, encoding="utf8").rstrip("\n") commit_range = merge_base + "..HEAD" else: commit_range = os.getenv("COMMIT_RANGE") diff --git a/test/util/test_runner.py b/test/util/test_runner.py index f96a74629b1f..4f67b1c4aa6a 100755 --- a/test/util/test_runner.py +++ b/test/util/test_runner.py @@ -112,7 +112,7 @@ def bctest(testDir, testObj, buildenv): raise Exception # Run the test - proc = subprocess.Popen(execrun, stdin=stdinCfg, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + proc = subprocess.Popen(execrun, stdin=stdinCfg, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) try: outs = proc.communicate(input=inputData) except OSError: From 50d8d27c185578ae705d880d26a1eb3c1ba23cd3 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 6 Jan 2026 19:50:41 +0700 Subject: [PATCH 02/10] partial Merge bitcoin/bitcoin#15294: refactor: Extract RipeMd160 BACKPORT NOTE: we have no usages for a new caller RipeMd160 as it is used only by witness's related code. It may be useful in the future, but DNM atm --- src/script/sign.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 488e050d751b..55ab8e248c41 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -107,7 +107,6 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator std::vector& ret, TxoutType& whichTypeRet, SigVersion sigversion, SignatureData& sigdata) { CScript scriptRet; - uint160 h160; ret.clear(); std::vector sig; @@ -135,8 +134,8 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator ret.push_back(ToByteVector(pubkey)); return true; } - case TxoutType::SCRIPTHASH: - h160 = uint160(vSolutions[0]); + case TxoutType::SCRIPTHASH: { + uint160 h160{vSolutions[0]}; if (GetCScript(provider, sigdata, CScriptID{h160}, scriptRet)) { ret.push_back(std::vector(scriptRet.begin(), scriptRet.end())); return true; @@ -144,7 +143,7 @@ static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator // Could not find redeemScript, add to missing sigdata.missing_redeem_script = h160; return false; - + } case TxoutType::MULTISIG: { size_t required = vSolutions.front()[0]; ret.push_back(valtype()); // workaround CHECKMULTISIG bug From 900fc3e315ba01e4d7978d6087c03cba5e80cdbf Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 31 Jan 2023 10:22:55 +0100 Subject: [PATCH 03/10] Merge bitcoin/bitcoin#26956: test: refactor: introduce `replace_in_config` helper b530d9605db863fd8d0e45e73ff2eb9462d1ad4c test: refactor: introduce `replace_in_config` helper (Sebastian Falbesoner) Pull request description: Currently two functional tests (p2p_permissions.py and wallet_crosschain.py) include quite similar code for substituting strings in a TestNode's bitcoind configuration file, so refactoring that out to a dedicated helper method seems to make sense (probably other tests could need that too in the future). ACKs for top commit: kouloumos: ACK b530d9605db863fd8d0e45e73ff2eb9462d1ad4c Tree-SHA512: 5ca65a2ef3292460e5720d5c6acf7326335001858e8f71ab054560117f9479dbadb1dacd8c9235f67f3fcfd08dbc761b62704f379cbf619bba8804f16a25bc7b --- test/functional/p2p_permissions.py | 10 ++-------- test/functional/test_framework/test_framework.py | 6 +----- test/functional/test_framework/test_node.py | 15 +++++++++++++++ test/functional/wallet_crosschain.py | 6 +----- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 53bf2e4f8a58..6a860d7901db 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -57,12 +57,12 @@ def run_test(self): # For this, we need to use whitebind instead of bind # by modifying the configuration file. ip_port = "127.0.0.1:{}".format(p2p_port(1)) - self.replaceinconfig(1, "bind=127.0.0.1", "whitebind=bloomfilter,forcerelay@" + ip_port) + self.nodes[1].replace_in_config([("bind=127.0.0.1", "whitebind=bloomfilter,forcerelay@" + ip_port)]) self.checkpermission( ["-whitelist=noban@127.0.0.1"], # Check parameter interaction forcerelay should activate relay ["noban", "bloomfilter", "forcerelay", "relay", "download"]) - self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1") + self.nodes[1].replace_in_config([("whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1")]) self.checkpermission( # legacy whitelistrelay should be ignored @@ -144,12 +144,6 @@ def checkpermission(self, args, expectedPermissions): if p not in peerinfo['permissions']: raise AssertionError("Expected permissions %r is not granted." % p) - def replaceinconfig(self, nodeid, old, new): - with open(self.nodes[nodeid].bitcoinconf, encoding="utf8") as f: - newText = f.read().replace(old, new) - with open(self.nodes[nodeid].bitcoinconf, 'w', encoding="utf8") as f: - f.write(newText) - if __name__ == '__main__': P2PPermissionsTests().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 02a1260c2eeb..6cf71f96eb70 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -565,11 +565,7 @@ def get_bin_from_version(version, bin_name, bin_default): self.nodes.append(test_node_i) if not test_node_i.version_is_at_least(160000): # adjust conf for pre 16 - conf_file = test_node_i.bitcoinconf - with open(conf_file, 'r', encoding='utf8') as conf: - conf_data = conf.read() - with open(conf_file, 'w', encoding='utf8') as conf: - conf.write(conf_data.replace('[regtest]', '')) + test_node_i.replace_in_config([('[regtest]', '')]) def add_dynamically_node(self, extra_args=None, *, rpchost=None, binary=None): if self.bind_to_localhost_only: diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 0970596cfcf2..7da3e233071f 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -427,6 +427,21 @@ def is_node_stopped(self): def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT): wait_until_helper(self.is_node_stopped, timeout=timeout, timeout_factor=self.timeout_factor) + def replace_in_config(self, replacements): + """ + Perform replacements in the configuration file. + The substitutions are passed as a list of search-replace-tuples, e.g. + [("old", "new"), ("foo", "bar"), ...] + """ + with open(self.bitcoinconf, 'r', encoding='utf8') as conf: + conf_data = conf.read() + for replacement in replacements: + assert_equal(len(replacement), 2) + old, new = replacement[0], replacement[1] + conf_data = conf_data.replace(old, new) + with open(self.bitcoinconf, 'w', encoding='utf8') as conf: + conf.write(conf_data) + @property def chain_path(self) -> Path: return Path(self.datadir) / get_chain_folder(self.datadir, self.chain) diff --git a/test/functional/wallet_crosschain.py b/test/functional/wallet_crosschain.py index f25b3ef88a60..a9cf425fcc54 100755 --- a/test/functional/wallet_crosschain.py +++ b/test/functional/wallet_crosschain.py @@ -23,11 +23,7 @@ def setup_network(self): # Switch node 1 to testnet before starting it. self.nodes[1].chain = 'testnet3' self.nodes[1].extra_args = ['-maxconnections=0', '-prune=945'] # disable testnet sync - with open(self.nodes[1].bitcoinconf, 'r', encoding='utf8') as conf: - conf_data = conf.read() - with open (self.nodes[1].bitcoinconf, 'w', encoding='utf8') as conf: - conf.write(conf_data.replace('regtest=', 'testnet=').replace('[regtest]', '[test]')) - + self.nodes[1].replace_in_config([('regtest=', 'testnet='), ('[regtest]', '[test]')]) self.start_nodes() def run_test(self): From 78595bd803a830b15a048b178e69afef074c8860 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 3 Feb 2023 19:16:26 +0000 Subject: [PATCH 04/10] Merge bitcoin-core/gui#653: Show watchonly balance only for Legacy wallets fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e gui: Show watchonly balance only for Legacy wallets (Andrew Chow) Pull request description: Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets. The result is that instead of the send page showing "Watch-only balance: 0.00000000 BTC" for watchonly descriptor wallets, we see the actual balance as "Balance: 10.00000000 BTC" ACKs for top commit: johnny9: tACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e furszy: ACK fdb8dc8a hebasto: ACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e Tree-SHA512: e5c0703a62d25c881c8dadfb9cffd482791f3d437a4ec5ae0088ce1a2069c2455ad6d3ec6c95a4404a3b55fbd727f92694529c35052236951553ca90c4ed31b5 --- src/qt/sendcoinsdialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index b5e6e81ffdde..449f13c375c2 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -787,7 +787,7 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances) CAmount balance = 0; if (model->wallet().hasExternalSigner()) { ui->labelBalanceName->setText(tr("External balance:")); - } else if (model->wallet().privateKeysDisabled()) { + } else if (model->wallet().isLegacy() && model->wallet().privateKeysDisabled()) { balance = balances.watch_only_balance; ui->labelBalanceName->setText(tr("Watch-only balance:")); } else if (m_coin_control->IsUsingCoinJoin()) { From d7e7091d2b56f699556473fdb80fb1099e1e6b29 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 7 Feb 2023 10:38:58 +0000 Subject: [PATCH 05/10] Merge bitcoin/bitcoin#17127: util: Set safe permissions for data directory and `wallets/` subdir c9ba4f9ecb1a282d98e7456a84ca84362b161757 test: Add test for file system permissions (Hennadii Stepanov) 581f16ef3404274cb5c1a79dd3d6ee7b584f9844 Apply default umask in `SetupEnvironment()` (Hennadii Stepanov) 8a6219e54379911605aed860519e0194f1433b72 Remove `-sysperms` option (Hennadii Stepanov) Pull request description: On master (1e7564eca8a688f39c75540877ec3bdfdde766b1) docs say: ``` $ ./src/bitcoind -help | grep -A 3 sysperms -sysperms Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality) ``` Basing on that, one could expect that running `bitcoind` first time will create data directory and `wallets/` subdirectory with safe 0700 permissions. But that is not the case: ``` $ stat .bitcoin | grep id Access: (0775/drwxrwxr-x) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto) $ stat .bitcoin/wallets | grep id Access: (0775/drwxrwxr-x) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto) ``` Both directories, in fact, are created with system default permissions. With this PR: ``` $ stat .bitcoin/wallets | grep id Access: (0700/drwx------) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto) $ stat .bitcoin/wallets | grep id Access: (0700/drwx------) Uid: ( 1000/ hebasto) Gid: ( 1000/ hebasto) ``` --- This PR: - is alternative to bitcoin/bitcoin#13389 - fixes bitcoin/bitcoin#15902 - fixes bitcoin/bitcoin#22595 - closes bitcoin/bitcoin#13371 - reverts bitcoin/bitcoin#4286 Changes in behavior: removed `-sysperms` command-line argument / configure option. The related discussions are here: - https://github.com/bitcoin/bitcoin/pull/13389#issuecomment-395306690 - https://github.com/bitcoin/bitcoin/pull/13389#issuecomment-539906114 - https://github.com/bitcoin/bitcoin/pull/13389#discussion_r279160472 If users rely on non-default access permissions, they could use `chmod`. ACKs for top commit: john-moffett: ACK c9ba4f9ecb1a282d98e7456a84ca84362b161757 willcl-ark: ACK c9ba4f9ecb1a282d98e7456a84ca84362b161757 Tree-SHA512: 96c745339e6bd0e4d7bf65daf9a721e2e1945b2b0ab74ca0f66576d0dc358b5de8eb8cdb89fe2160f3b19c39d2798bb8b291784316085dc73a27102d3415bd57 --- doc/init.md | 2 +- src/i2p.cpp | 2 +- src/init.cpp | 9 ---- src/rpc/request.cpp | 2 +- src/util/system.cpp | 5 +++ src/wallet/init.cpp | 3 -- .../feature_posix_fs_permissions.py | 43 +++++++++++++++++++ .../test_framework/test_framework.py | 5 +++ test/functional/test_runner.py | 1 + 9 files changed, 57 insertions(+), 15 deletions(-) create mode 100755 test/functional/feature_posix_fs_permissions.py diff --git a/doc/init.md b/doc/init.md index 2c184d1c6dca..da216e74adc4 100644 --- a/doc/init.md +++ b/doc/init.md @@ -69,7 +69,7 @@ NOTE: When using the systemd .service file, the creation of the aforementioned directories and the setting of their permissions is automatically handled by systemd. Directories are given a permission of 710, giving the dashcore user and group access to files under it _if_ the files themselves give permission to the -dashcore user and group to do so (e.g. when `-sysperms` is specified). This does not allow +dashcore user and group to do so. This does not allow for the listing of files under the directory. NOTE: It is not currently possible to override `datadir` in diff --git a/src/i2p.cpp b/src/i2p.cpp index 43e14e945cdd..14f8b39ccd70 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -362,7 +362,7 @@ void Session::GenerateAndSavePrivateKey(const Sock& sock) { DestGenerate(sock); - // umask is set to 077 in init.cpp, which is ok (unless -sysperms is given) + // umask is set to 0077 in util/system.cpp, which is ok. if (!WriteBinaryFile(m_private_key_file, std::string(m_private_key.begin(), m_private_key.end()))) { throw std::runtime_error( diff --git a/src/init.cpp b/src/init.cpp index 6e79ed5b0d21..1acd5b39b2ea 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -582,11 +582,6 @@ void SetupServerArgs(ArgsManager& argsman) #if HAVE_SYSTEM argsman.AddArg("-startupnotify=", "Execute command on startup.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-shutdownnotify=", "Execute command immediately before beginning shutdown. The need for shutdown may be urgent, so be careful not to delay it long (if the command doesn't require interaction with the server, consider having it fork into the background).", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); -#endif -#ifndef WIN32 - argsman.AddArg("-sysperms", "Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); -#else - hidden_args.emplace_back("-sysperms"); #endif argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -1119,10 +1114,6 @@ bool AppInitBasicSetup(const ArgsManager& args) } #ifndef WIN32 - if (!args.GetBoolArg("-sysperms", false)) { - umask(077); - } - // Clean shutdown on SIGTERM registerSignalHandler(SIGTERM, HandleSIGTERM); registerSignalHandler(SIGINT, HandleSIGTERM); diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index c00c0b552fc3..476392ede6f1 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -88,7 +88,7 @@ bool GenerateAuthCookie(std::string *cookie_out) std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd); /** the umask determines what permissions are used to create this file - - * these are set to 077 in init.cpp unless overridden with -sysperms. + * these are set to 0077 in util/system.cpp. */ std::ofstream file; fs::path filepath_tmp = GetAuthCookieFile(true); diff --git a/src/util/system.cpp b/src/util/system.cpp index 49218db48acf..d769dbd055be 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1438,6 +1438,11 @@ void SetupEnvironment() SetConsoleCP(CP_UTF8); SetConsoleOutputCP(CP_UTF8); #endif + +#ifndef WIN32 + constexpr mode_t private_umask = 0077; + umask(private_umask); +#endif } bool SetupNetworking() diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index e1be2e7eefb6..7bb836c40083 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -155,9 +155,6 @@ bool WalletInit::ParameterInteraction() const gArgs.ForceRemoveArg("rescan"); } - if (gArgs.GetBoolArg("-sysperms", false)) - return InitError(Untranslated("-sysperms is not allowed in combination with enabled wallet functionality")); - if (gArgs.IsArgSet("-walletbackupsdir")) { if (!fs::is_directory(gArgs.GetArg("-walletbackupsdir", ""))) { InitWarning(strprintf(_("Warning: incorrect parameter %s, path must exist! Using default path."), "-walletbackupsdir")); diff --git a/test/functional/feature_posix_fs_permissions.py b/test/functional/feature_posix_fs_permissions.py new file mode 100755 index 000000000000..c5a543e97ae9 --- /dev/null +++ b/test/functional/feature_posix_fs_permissions.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test file system permissions for POSIX platforms. +""" + +import os +import stat + +from test_framework.test_framework import BitcoinTestFramework + + +class PosixFsPermissionsTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_platform_not_posix() + + def check_directory_permissions(self, dir): + mode = os.lstat(dir).st_mode + self.log.info(f"{stat.filemode(mode)} {dir}") + assert mode == (stat.S_IFDIR | stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) + + def check_file_permissions(self, file): + mode = os.lstat(file).st_mode + self.log.info(f"{stat.filemode(mode)} {file}") + assert mode == (stat.S_IFREG | stat.S_IRUSR | stat.S_IWUSR) + + def run_test(self): + self.stop_node(0) + datadir = os.path.join(self.nodes[0].datadir, self.chain) + self.check_directory_permissions(datadir) + walletsdir = os.path.join(datadir, "wallets") + self.check_directory_permissions(walletsdir) + debuglog = os.path.join(datadir, "debug.log") + self.check_file_permissions(debuglog) + + +if __name__ == '__main__': + PosixFsPermissionsTest().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 6cf71f96eb70..4ab8c9038cfc 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1037,6 +1037,11 @@ def skip_if_platform_not_linux(self): if platform.system() != "Linux": raise SkipTest("not on a Linux system") + def skip_if_platform_not_posix(self): + """Skip the running test if we are not on a POSIX platform""" + if os.name != 'posix': + raise SkipTest("not on a POSIX system") + def skip_if_no_bitcoind_zmq(self): """Skip the running test if dashd has not been compiled with zmq support.""" if not self.is_zmq_compiled(): diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1de960501d32..15854fb3f7b5 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -225,6 +225,7 @@ 'p2p_compactblocks_hb.py --v2transport', 'p2p_disconnect_ban.py --v1transport', 'p2p_disconnect_ban.py --v2transport', + 'feature_posix_fs_permissions.py', 'feature_addressindex.py', 'feature_timestampindex.py', 'feature_spentindex.py', From 1c0b34f4db353c452151f4a601225a6061a6d43f Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 8 Feb 2023 10:36:35 +0000 Subject: [PATCH 06/10] Merge bitcoin/bitcoin#23810: docs: avoid C-style casts; use modern C++ casts 75347236f212f327a5bba10d8a900cc58ebe5de0 docs: document c-style cast prohibition (Pasta) Pull request description: In the words of practicalswift: ``` A C-style cast is equivalent to try casting in the following order: const_cast(...) static_cast(...) const_cast(static_cast(...)) reinterpret_cast(...) const_cast(reinterpret_cast(...)) By using static_cast(...) explicitly we avoid the possibility of an unintentional and dangerous reinterpret_cast. Furthermore static_cast(...) allows for easier grepping of casts. For a more thorough discussion, see "ES.49: If you must use a cast, use a named cast" in the C++ Core Guidelines (Stroustrup & Sutter). ``` Modern tooling, specifically `-Wold-style-cast` can enable us to enforce never using C-style casts. I believe this is especially important due to the number of C-style casts the codebase is currently being used as a reinterpret_cast. reinterpret_casts are especially dangerous, and should never be done via C-style casts. Update the docs to suggest the use of named cast or functional casts. Top commit has no ACKs. Tree-SHA512: 29a98de396f0c78e32d8a1831319162203c4405a670da5add5da956fcc7df200a1cec162ef1cfac4ddfb02714b66406081d40ed435c7f0f28581cfa24d94fac1 --- doc/developer-notes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index f8bb574015e1..c343f3a7e007 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -110,6 +110,10 @@ code. - `nullptr` is preferred over `NULL` or `(void*)0`. - `static_assert` is preferred over `assert` where possible. Generally; compile-time checking is preferred over run-time checking. - Align pointers and references to the left i.e. use `type& var` and not `type &var`. + - Use a named cast or functional cast, not a C-Style cast. When casting + between integer types, use functional casts such as `int(x)` or `int{x}` + instead of `(int) x`. When casting between more complex types, use static_cast. + Use reinterpret_cast and const_cast as appropriate. - Prefer [`list initialization ({})`](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-list) where possible. For example `int x{0};` instead of `int x = 0;` or `int x(0);` From 1a92d2a135059cf896d74c3071177f3164a85f6d Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 14 Feb 2023 10:51:31 +0000 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#27081: Modernize rpcauth.py e4e17907b686c25dda91e569645a8f501ca48f90 Modernize rpcauth.py and its tests (Pieter Wuille) Pull request description: Use Python3 constructions, and f-strings. ACKs for top commit: jamesob: Github ACK https://github.com/bitcoin/bitcoin/pull/27081/commits/e4e17907b686c25dda91e569645a8f501ca48f90 Tree-SHA512: 005573d967e04400fec727f45739f138879be703e692745c0a639272d37d221d230f388de23f2615cb954bb47179fb46e53da0410ae9f0865319b91bb2dc01f4 --- share/rpcauth/rpcauth.py | 14 ++++++-------- test/util/rpcauth-test.py | 11 +++++------ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/share/rpcauth/rpcauth.py b/share/rpcauth/rpcauth.py index acc41bafcbd6..6f94f8fe770c 100755 --- a/share/rpcauth/rpcauth.py +++ b/share/rpcauth/rpcauth.py @@ -4,22 +4,20 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from argparse import ArgumentParser -from base64 import urlsafe_b64encode from getpass import getpass -from os import urandom - +from secrets import token_hex, token_urlsafe import hmac def generate_salt(size): """Create size byte hex salt""" - return urandom(size).hex() + return token_hex(size) def generate_password(): """Create 32 byte b64 password""" - return urlsafe_b64encode(urandom(32)).decode('utf-8') + return token_urlsafe(32) def password_to_hmac(salt, password): - m = hmac.new(bytearray(salt, 'utf-8'), bytearray(password, 'utf-8'), 'SHA256') + m = hmac.new(salt.encode('utf-8'), password.encode('utf-8'), 'SHA256') return m.hexdigest() def main(): @@ -38,8 +36,8 @@ def main(): password_hmac = password_to_hmac(salt, args.password) print('String to be appended to dash.conf:') - print('rpcauth={0}:{1}${2}'.format(args.username, salt, password_hmac)) - print('Your password:\n{0}'.format(args.password)) + print(f'rpcauth={args.username}:{salt}${password_hmac}') + print(f'Your password:\n{args.password}') if __name__ == '__main__': main() diff --git a/test/util/rpcauth-test.py b/test/util/rpcauth-test.py index 53058dc394af..8a7ff26dcb23 100755 --- a/test/util/rpcauth-test.py +++ b/test/util/rpcauth-test.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test share/rpcauth/rpcauth.py """ -import base64 +import re import configparser import hmac import importlib @@ -28,18 +28,17 @@ def test_generate_salt(self): self.assertEqual(len(self.rpcauth.generate_salt(i)), i * 2) def test_generate_password(self): + """Test that generated passwords only consist of urlsafe characters.""" + r = re.compile(r"[0-9a-zA-Z_-]*") password = self.rpcauth.generate_password() - expected_password = base64.urlsafe_b64encode( - base64.urlsafe_b64decode(password)).decode('utf-8') - self.assertEqual(expected_password, password) + self.assertTrue(r.fullmatch(password)) def test_check_password_hmac(self): salt = self.rpcauth.generate_salt(16) password = self.rpcauth.generate_password() password_hmac = self.rpcauth.password_to_hmac(salt, password) - m = hmac.new(bytearray(salt, 'utf-8'), - bytearray(password, 'utf-8'), 'SHA256') + m = hmac.new(salt.encode('utf-8'), password.encode('utf-8'), 'SHA256') expected_password_hmac = m.hexdigest() self.assertEqual(expected_password_hmac, password_hmac) From 2330b1ec925c6a850194f3926d68e0c0066afd12 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 22 Feb 2023 09:26:23 +0000 Subject: [PATCH 08/10] Merge bitcoin/bitcoin#25867: lint: enable E722 do not use bare except 61bb4e783b3acc62b121a228f6b14c2462e23315 lint: enable E722 do not use bare except (Leonardo Lazzaro) Pull request description: Improve test code and enable E722 lint check. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:). Reference: https://peps.python.org/pep-0008/#programming-recommendations ACKs for top commit: MarcoFalke: lgtm ACK 61bb4e783b3acc62b121a228f6b14c2462e23315 Tree-SHA512: c7497769d5745fa02c78a20f4a0e555d8d3996d64af6faf1ce28e22ac1d8be415b98e967294679007b7bda2a9fd04031a9d140b24201e00257ceadeb5c5d7665 --- contrib/devtools/github-merge.py | 2 +- contrib/devtools/optimize-pngs.py | 2 +- contrib/devtools/security-check.py | 2 +- contrib/devtools/update-translations.py | 2 +- test/functional/feature_dbcrash.py | 2 +- test/functional/feature_llmq_connections.py | 4 ++-- test/functional/feature_llmq_is_retroactive.py | 2 +- test/functional/p2p_node_network_limited.py | 2 +- test/functional/rpc_preciousblock.py | 2 +- test/functional/test_framework/p2p.py | 2 +- test/functional/test_framework/test_framework.py | 10 +++++----- test/functional/test_framework/test_node.py | 2 +- test/functional/test_framework/util.py | 6 +++--- test/lint/lint-python.py | 1 + test/util/test_runner.py | 4 ++-- 15 files changed, 23 insertions(+), 22 deletions(-) diff --git a/contrib/devtools/github-merge.py b/contrib/devtools/github-merge.py index 43ed5ff1a220..f7f7d77937cb 100755 --- a/contrib/devtools/github-merge.py +++ b/contrib/devtools/github-merge.py @@ -306,7 +306,7 @@ def main(): print(range_diff_output.decode('utf-8')) try: subprocess.check_call([GIT,'log','--graph','--topo-order','--pretty=format:'+COMMIT_FORMAT]) - except: + except Exception: pass review_reply = ask_prompt("Do you want to continue with force push? Type 'yes' to continue or anything else to abort.").lower() if review_reply != 'yes': diff --git a/contrib/devtools/optimize-pngs.py b/contrib/devtools/optimize-pngs.py index 12de9e784633..6991dfac69c2 100755 --- a/contrib/devtools/optimize-pngs.py +++ b/contrib/devtools/optimize-pngs.py @@ -50,7 +50,7 @@ def content_hash(filename): try: subprocess.call([pngcrush, "-brute", "-ow", "-rem", "gAMA", "-rem", "cHRM", "-rem", "iCCP", "-rem", "sRGB", "-rem", "alla", "-rem", "text", file_path], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - except: + except Exception: print("pngcrush is not installed, aborting...") sys.exit(0) diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index 001e46ceeef7..db0e886525e8 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -34,7 +34,7 @@ def check_ELF_RELRO(binary) -> bool: flags = binary.get(lief.ELF.DYNAMIC_TAGS.FLAGS) if flags.value & lief.ELF.DYNAMIC_FLAGS.BIND_NOW: have_bindnow = True - except: + except Exception: have_bindnow = False return have_gnu_relro and have_bindnow diff --git a/contrib/devtools/update-translations.py b/contrib/devtools/update-translations.py index a0fef0e02b1d..bc889878e6b2 100755 --- a/contrib/devtools/update-translations.py +++ b/contrib/devtools/update-translations.py @@ -56,7 +56,7 @@ def find_format_specifiers(s): break try: specifiers.append(s[percent+1]) - except: + except Exception: print('Failed to get specifier') pos = percent+2 return specifiers diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index 1607c9a45295..0b89d5ed6244 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -85,7 +85,7 @@ def restart_node(self, node_index, expected_tip): self.nodes[node_index].waitforblock(expected_tip) utxo_hash = self.nodes[node_index].gettxoutsetinfo()['hash_serialized_2'] return utxo_hash - except: + except Exception: # An exception here should mean the node is about to crash. # If dashd exits, then try again. wait_for_node_exit() # should raise an exception if dashd doesn't exit. diff --git a/test/functional/feature_llmq_connections.py b/test/functional/feature_llmq_connections.py index a9acd24885a4..fd999f286d21 100755 --- a/test/functional/feature_llmq_connections.py +++ b/test/functional/feature_llmq_connections.py @@ -91,7 +91,7 @@ def run_test(self): self.mine_cycle_quorum() mn.get_node(self).mockscheduler(60) # we check for old connections via the scheduler every 60 seconds removed = True - except: + except Exception: pass # it's ok to not remove connections sometimes if removed: break @@ -105,7 +105,7 @@ def run_test(self): with mn.get_node(self).assert_debug_log(['adding mn inter-quorum connections']): self.mine_cycle_quorum() added = True - except: + except Exception: pass # it's ok to not add connections sometimes if added: break diff --git a/test/functional/feature_llmq_is_retroactive.py b/test/functional/feature_llmq_is_retroactive.py index c8454ea72982..8badb7f1769b 100755 --- a/test/functional/feature_llmq_is_retroactive.py +++ b/test/functional/feature_llmq_is_retroactive.py @@ -37,7 +37,7 @@ def wait_for_tx(self, txid, node, expected=True, timeout=60): def check_tx(): try: return node.getrawtransaction(txid) - except: + except Exception: return False if self.wait_until(check_tx, timeout=timeout, do_assert=expected) and not expected: raise AssertionError("waiting unexpectedly succeeded") diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index 7e3f10dc02fe..382c94f2476c 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -83,7 +83,7 @@ def run_test(self): self.connect_nodes(0, 2) try: self.sync_blocks([self.nodes[0], self.nodes[2]], timeout=5) - except: + except Exception: pass # node2 must remain at height 0 assert_equal(self.nodes[2].getblockheader(self.nodes[2].getbestblockhash())['height'], 0) diff --git a/test/functional/rpc_preciousblock.py b/test/functional/rpc_preciousblock.py index 2e526efd9abd..92198d2e13f7 100755 --- a/test/functional/rpc_preciousblock.py +++ b/test/functional/rpc_preciousblock.py @@ -16,7 +16,7 @@ def unidirectional_node_sync_via_rpc(node_src, node_dest): try: assert len(node_dest.getblock(blockhash, False)) > 0 break - except: + except Exception: blocks_to_copy.append(blockhash) blockhash = node_src.getblockheader(blockhash, True)['previousblockhash'] blocks_to_copy.reverse() diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index a79cff36f8e9..d0d79f5ebc8e 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -559,7 +559,7 @@ def on_message(self, message): self.message_count[msgtype] += 1 self.last_message[msgtype] = message getattr(self, 'on_' + msgtype)(message) - except: + except Exception: print("ERROR delivering %s (%s)" % (repr(message), sys.exc_info()[0])) raise diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 4ab8c9038cfc..67ceeb167625 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -650,7 +650,7 @@ def start_nodes(self, extra_args=None, *args, **kwargs): node.start(extra_args[i], *args, **kwargs) for node in self.nodes: node.wait_for_rpc_connection() - except: + except Exception: # If one node failed to start, stop the others self.stop_nodes() raise @@ -1605,7 +1605,7 @@ def dynamically_add_masternode(self, evo=False, rnd=None, should_be_rejected=Fal try: created_mn_info = self.dynamically_prepare_masternode(mn_idx, evo, rnd) protx_success = True - except: + except Exception: self.log.info("dynamically_prepare_masternode failed") assert_equal(protx_success, not should_be_rejected) @@ -1688,7 +1688,7 @@ def dynamically_evo_update_service(self, evo_info: MasternodeInfo, rnd=None, sho evo_info.bury_tx(self, genIdx=0, txid=protx_result, depth=1) self.log.info("Updated EvoNode %s: platformNodeID=%s, platformP2PPort=%s, platformHTTPPort=%s" % (evo_info.proTxHash, platform_node_id, addrs_platform_p2p, addrs_platform_https)) protx_success = True - except: + except Exception: self.log.info("protx_evo rejected") assert_equal(protx_success, not should_be_rejected) @@ -1936,7 +1936,7 @@ def wait_for_instantlock(self, txid, node, timeout=60): def check_instantlock(): try: return node.getrawtransaction(txid, True)["instantlock"] - except: + except Exception: return False self.log.info(f"Expecting InstantLock for {txid}") @@ -1947,7 +1947,7 @@ def check_chainlocked_block(): try: block = node.getblock(block_hash) return block["confirmations"] > 0 and block["chainlock"] - except: + except Exception: return False self.log.info(f"Expecting ChainLock for {block_hash}") if self.wait_until(check_chainlocked_block, timeout=timeout, do_assert=expected) and not expected: diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 7da3e233071f..2c1497cc3cd5 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -955,7 +955,7 @@ def importaddress(self, address, label=None, rescan=None, p2sh=None): int(address ,16) is_hex = True desc = descsum_create('raw(' + address + ')') - except: + except Exception: desc = descsum_create('addr(' + address + ')') reqs = [{ 'desc': desc, diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 71c0b875a0d7..029140ab4e0d 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -282,7 +282,7 @@ def wait_until_helper(predicate, *, attempts=float('inf'), timeout=float('inf'), else: if predicate(): return True - except: + except Exception: if not allow_exception: raise attempt += 1 @@ -474,7 +474,7 @@ def copy_datadir(from_node, to_node, dirname, chain): src = os.path.join(from_datadir, d) dst = os.path.join(to_datadir, d) shutil.copytree(src, dst) - except: + except Exception: pass # If a cookie file exists in the given datadir, delete it. @@ -496,7 +496,7 @@ def get_chain_folder(datadir, chain): if chain in os.listdir(datadir)[i]: chain = os.listdir(datadir)[i] break - except: + except Exception: pass return chain diff --git a/test/lint/lint-python.py b/test/lint/lint-python.py index a81edfd2fc7f..e94c05c7235b 100755 --- a/test/lint/lint-python.py +++ b/test/lint/lint-python.py @@ -52,6 +52,7 @@ 'E711,' # comparison to None should be 'if cond is None:' 'E714,' # test for object identity should be "is not" 'E721,' # do not compare types, use "isinstance()" + 'E722,' # do not use bare 'except' 'E742,' # do not define classes named "l", "O", or "I" 'E743,' # do not define functions named "l", "O", or "I" 'E901,' # SyntaxError: invalid syntax diff --git a/test/util/test_runner.py b/test/util/test_runner.py index 4f67b1c4aa6a..064d9e065b35 100755 --- a/test/util/test_runner.py +++ b/test/util/test_runner.py @@ -54,7 +54,7 @@ def bctester(testDir, input_basename, buildenv): try: bctest(testDir, testObj, buildenv) logging.info("PASSED: " + testObj["description"]) - except: + except Exception: logging.info("FAILED: " + testObj["description"]) failed_testcases.append(testObj["description"]) @@ -101,7 +101,7 @@ def bctest(testDir, testObj, buildenv): try: with open(os.path.join(testDir, outputFn), encoding="utf8") as f: outputData = f.read() - except: + except Exception: logging.error("Output file " + outputFn + " cannot be opened") raise if not outputData: From bc5bdef972e8140c64d32d7c6fde1f9295a94266 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Jan 2023 09:27:26 +0100 Subject: [PATCH 09/10] Merge bitcoin/bitcoin#26904: build: move rpc/request from util lib to common 87a08cba43f8dc427efccbd45d28acc652db2cb6 build: move rpc/request from util lib to common (fanquake) Pull request description: This is JSON RPC related code that doesn't need to be in util, and should not be required by the kernel. ACKs for top commit: TheCharlatan: ACK 87a08cba43f8dc427efccbd45d28acc652db2cb6 Tree-SHA512: 5f335be9f0f9ff02eff073af47558ecf505c1392c05f18ca24a065b12b8d92529ec3942d84978cc5028c38369c496ed0243653e1fa26d4db2fae26dfe55c3d65 --- src/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 1c5fd1936399..b59ee1c16143 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -946,8 +946,9 @@ libbitcoin_common_a_SOURCES = \ protocol.cpp \ psbt.cpp \ rpc/evo_util.cpp \ - rpc/rawtransaction_util.cpp \ rpc/external_signer.cpp \ + rpc/rawtransaction_util.cpp \ + rpc/request.cpp \ rpc/util.cpp \ saltedhasher.cpp \ scheduler.cpp \ @@ -982,7 +983,6 @@ libbitcoin_util_a_SOURCES = \ messagesigner.cpp \ random.cpp \ randomenv.cpp \ - rpc/request.cpp \ stacktraces.cpp \ support/cleanse.cpp \ sync.cpp \ From 7c5289384653dce84a4ae37b523c44fb25fba735 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 13 Jan 2026 15:52:23 +0700 Subject: [PATCH 10/10] partial Merge bitcoin#26294: build: move util/url to common/url To prevent linkage error function `ConnectAndCallRPC(BaseRequestHandler*, std::__cxx11::basic_string, std::allocator > const&, std::vector, std::allocator >, std::allocator, std::allocator > > > const&, std::optional, std::allocator > > const&)': DASH/src/bitcoin-cli.cpp:788:(.text+0xf23): undefined reference to `GetAuthCookie(std::__cxx11::basic_string, std::allocator >*)' /usr/bin/ld: dash_cli-bitcoin-cli.o: in function `GenerateToAddressRequestHandler::PrepareRequest(std::__cxx11::basic_string, std::allocator > const&, std::vector, std::allocator >, std::allocator, std::allocator > > > const&)': DASH/src/bitcoin-cli.cpp:710:(.text._ZN31GenerateToAddressRequestHandler14PrepareRequestERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKSt6vectorIS5_SaIS5_EE[_ZN31GenerateToAddressRequestHandler14PrepareRequestERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEERKSt6vectorIS5_SaIS5_EE]+0x15d): undefined reference to `JSONRPCRequestObj(std::__cxx11::basic_string, std::allocator > const&, UniValue const&, UniValue const&)' --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Makefile.am b/src/Makefile.am index b59ee1c16143..47f3c6bd55fc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1082,6 +1082,7 @@ endif dash_cli_LDADD = \ $(LIBBITCOIN_CLI) \ $(LIBUNIVALUE) \ + $(LIBBITCOIN_COMMON) \ $(LIBBITCOIN_UTIL) \ $(LIBBITCOIN_CRYPTO) dash_cli_LDADD += $(BACKTRACE_LIBS) $(EVENT_LIBS) $(GMP_LIBS)