diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 64b1e4e182..8a7a978994 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -70,7 +70,7 @@ if [ "$CI_OS_NAME" == "macos" ]; then fi CI_EXEC () { - $CI_EXEC_CMD_PREFIX bash -c "export PATH=${BINS_SCRATCH_DIR}:${BASE_ROOT_DIR}/ci/retry:\$PATH && cd \"${BASE_ROOT_DIR}\" && $*" + $CI_EXEC_CMD_PREFIX bash -c "export PATH=\"/path_with space:${BINS_SCRATCH_DIR}:${BASE_ROOT_DIR}/ci/retry:\$PATH\" && cd \"${BASE_ROOT_DIR}\" && $*" } export -f CI_EXEC diff --git a/configure.ac b/configure.ac index 68edc84ded..adc862d251 100644 --- a/configure.ac +++ b/configure.ac @@ -321,11 +321,6 @@ AC_ARG_ENABLE([external-signer], [use_external_signer=$enableval], [use_external_signer=auto]) -AC_ARG_ENABLE([lto], - [AS_HELP_STRING([--enable-lto],[build using LTO (default is no)])], - [enable_lto=$enableval], - [enable_lto=no]) - AC_LANG_PUSH([C++]) dnl Check for a flag to turn compiler warnings into errors. This is helpful for checks which may @@ -377,11 +372,6 @@ if test "$enable_debug" = "yes"; then AX_CHECK_COMPILE_FLAG([-ftrapv], [DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -ftrapv"], [], [$CXXFLAG_WERROR]) fi -if test "$enable_lto" = "yes"; then - AX_CHECK_COMPILE_FLAG([-flto], [LTO_CXXFLAGS="$LTO_CXXFLAGS -flto"], [AC_MSG_ERROR([compile failed with -flto])], [$CXXFLAG_WERROR]) - AX_CHECK_LINK_FLAG([-flto], [LTO_LDFLAGS="$LTO_LDFLAGS -flto"], [AC_MSG_ERROR([link failed with -flto])], [$CXXFLAG_WERROR]) -fi - if test "$use_sanitizers" != ""; then dnl First check if the compiler accepts flags. If an incompatible pair like dnl -fsanitize=address,thread is used here, this check will fail. This will also @@ -1891,8 +1881,6 @@ AC_SUBST(GPROF_LDFLAGS) AC_SUBST(HARDENED_CXXFLAGS) AC_SUBST(HARDENED_CPPFLAGS) AC_SUBST(HARDENED_LDFLAGS) -AC_SUBST(LTO_CXXFLAGS) -AC_SUBST(LTO_LDFLAGS) AC_SUBST(PIC_FLAGS) AC_SUBST(PIE_FLAGS) AC_SUBST(SANITIZER_CXXFLAGS) @@ -2000,7 +1988,6 @@ echo " sanitizers = $use_sanitizers" echo " debug enabled = $enable_debug" echo " gprof enabled = $enable_gprof" echo " werror = $enable_werror" -echo " LTO = $enable_lto" echo echo " target os = $host_os" echo " build os = $build_os" @@ -2009,8 +1996,8 @@ echo " CC = $CC" echo " CFLAGS = $PTHREAD_CFLAGS $CFLAGS" echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS" echo " CXX = $CXX" -echo " CXXFLAGS = $LTO_CXXFLAGS $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS" -echo " LDFLAGS = $LTO_LDFLAGS $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $LDFLAGS" +echo " CXXFLAGS = $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS" +echo " LDFLAGS = $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $LDFLAGS" echo " AR = $AR" echo " ARFLAGS = $ARFLAGS" echo diff --git a/contrib/devtools/clang-format-diff.py b/contrib/devtools/clang-format-diff.py index 420bf7ff33..e2b661d65d 100755 --- a/contrib/devtools/clang-format-diff.py +++ b/contrib/devtools/clang-format-diff.py @@ -1,166 +1,190 @@ #!/usr/bin/env python3 # -#===- clang-format-diff.py - ClangFormat Diff Reformatter ----*- python -*--===# +# ===- clang-format-diff.py - ClangFormat Diff Reformatter ----*- python -*--===# # -# The LLVM Compiler Infrastructure +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception # -# This file is distributed under the University of Illinois Open Source -# License. -# -# ============================================================ -# -# University of Illinois/NCSA -# Open Source License -# -# Copyright (c) 2007-2015 University of Illinois at Urbana-Champaign. -# All rights reserved. -# -# Developed by: -# -# LLVM Team -# -# University of Illinois at Urbana-Champaign -# -# http://llvm.org -# -# Permission is hereby granted, free of charge, to any person obtaining a copy of -# this software and associated documentation files (the "Software"), to deal with -# the Software without restriction, including without limitation the rights to -# use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies -# of the Software, and to permit persons to whom the Software is furnished to do -# so, subject to the following conditions: -# -# * Redistributions of source code must retain the above copyright notice, -# this list of conditions and the following disclaimers. -# -# * Redistributions in binary form must reproduce the above copyright notice, -# this list of conditions and the following disclaimers in the -# documentation and/or other materials provided with the distribution. -# -# * Neither the names of the LLVM Team, University of Illinois at -# Urbana-Champaign, nor the names of its contributors may be used to -# endorse or promote products derived from this Software without specific -# prior written permission. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS -# FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH THE -# SOFTWARE. -# -# ============================================================ -# -#===------------------------------------------------------------------------===# - -r""" -ClangFormat Diff Reformatter -============================ +# ===------------------------------------------------------------------------===# +""" This script reads input from a unified diff and reformats all the changed lines. This is useful to reformat all the lines touched by a specific patch. Example usage for git/svn users: - git diff -U0 HEAD^ | clang-format-diff.py -p1 -i - svn diff --diff-cmd=diff -x-U0 | clang-format-diff.py -i + git diff -U0 --no-color --relative HEAD^ | {clang_format_diff} -p1 -i + svn diff --diff-cmd=diff -x-U0 | {clang_format_diff} -i +It should be noted that the filename contained in the diff is used unmodified +to determine the source file to update. Users calling this script directly +should be careful to ensure that the path in the diff is correct relative to the +current working directory. """ +from __future__ import absolute_import, division, print_function import argparse import difflib -import io import re import subprocess import sys - -# Change this to the full path if clang-format is not on the path. -binary = 'clang-format' +from io import StringIO def main(): - parser = argparse.ArgumentParser(description= - 'Reformat changed lines in diff. Without -i ' - 'option just output the diff that would be ' - 'introduced.') - parser.add_argument('-i', action='store_true', default=False, - help='apply edits to files instead of displaying a diff') - parser.add_argument('-p', metavar='NUM', default=0, - help='strip the smallest prefix containing P slashes') - parser.add_argument('-regex', metavar='PATTERN', default=None, - help='custom pattern selecting file paths to reformat ' - '(case sensitive, overrides -iregex)') - parser.add_argument('-iregex', metavar='PATTERN', default= - r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc|js|ts|proto' - r'|protodevel|java)', - help='custom pattern selecting file paths to reformat ' - '(case insensitive, overridden by -regex)') - parser.add_argument('-sort-includes', action='store_true', default=False, - help='let clang-format sort include blocks') - parser.add_argument('-v', '--verbose', action='store_true', - help='be more verbose, ineffective without -i') - args = parser.parse_args() - - # Extract changed lines for each file. - filename = None - lines_by_file = {} - for line in sys.stdin: - match = re.search(r'^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line) - if match: - filename = match.group(2) - if filename is None: - continue - - if args.regex is not None: - if not re.match('^%s$' % args.regex, filename): - continue - else: - if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE): - continue - - match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line) - if match: - start_line = int(match.group(1)) - line_count = 1 - if match.group(3): - line_count = int(match.group(3)) - if line_count == 0: - continue - end_line = start_line + line_count - 1 - lines_by_file.setdefault(filename, []).extend( - ['-lines', str(start_line) + ':' + str(end_line)]) - - # Reformat files containing changes in place. - for filename, lines in lines_by_file.items(): - if args.i and args.verbose: - print('Formatting {}'.format(filename)) - command = [binary, filename] - if args.i: - command.append('-i') - if args.sort_includes: - command.append('-sort-includes') - command.extend(lines) - command.extend(['-style=file', '-fallback-style=none']) - p = subprocess.Popen(command, - stdout=subprocess.PIPE, - stderr=None, - stdin=subprocess.PIPE, - text=True) - stdout, stderr = p.communicate() - if p.returncode != 0: - sys.exit(p.returncode) - - if not args.i: - with open(filename, encoding="utf8") as f: - code = f.readlines() - formatted_code = io.StringIO(stdout).readlines() - diff = difflib.unified_diff(code, formatted_code, - filename, filename, - '(before formatting)', '(after formatting)') - diff_string = ''.join(diff) - if len(diff_string) > 0: - sys.stdout.write(diff_string) - -if __name__ == '__main__': - main() + parser = argparse.ArgumentParser( + description=__doc__.format(clang_format_diff="%(prog)s"), + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument( + "-i", + action="store_true", + default=False, + help="apply edits to files instead of displaying a diff", + ) + parser.add_argument( + "-p", + metavar="NUM", + default=0, + help="strip the smallest prefix containing P slashes", + ) + parser.add_argument( + "-regex", + metavar="PATTERN", + default=None, + help="custom pattern selecting file paths to reformat " + "(case sensitive, overrides -iregex)", + ) + parser.add_argument( + "-iregex", + metavar="PATTERN", + default=r".*\.(?:cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp" + r"|hxx|m|mm|inc|js|ts|proto|protodevel|java|cs|json|s?vh?)", + help="custom pattern selecting file paths to reformat " + "(case insensitive, overridden by -regex)", + ) + parser.add_argument( + "-sort-includes", + action="store_true", + default=False, + help="let clang-format sort include blocks", + ) + parser.add_argument( + "-v", + "--verbose", + action="store_true", + help="be more verbose, ineffective without -i", + ) + parser.add_argument( + "-style", + help="formatting style to apply (LLVM, GNU, Google, Chromium, " + "Microsoft, Mozilla, WebKit)", + ) + parser.add_argument( + "-fallback-style", + help="The name of the predefined style used as a" + "fallback in case clang-format is invoked with" + "-style=file, but can not find the .clang-format" + "file to use.", + ) + parser.add_argument( + "-binary", + default="clang-format", + help="location of binary to use for clang-format", + ) + args = parser.parse_args() + + # Extract changed lines for each file. + filename = None + lines_by_file = {} + for line in sys.stdin: + match = re.search(r"^\+\+\+\ (.*?/){%s}(\S*)" % args.p, line) + if match: + filename = match.group(2) + if filename is None: + continue + + if args.regex is not None: + if not re.match("^%s$" % args.regex, filename): + continue + else: + if not re.match("^%s$" % args.iregex, filename, re.IGNORECASE): + continue + + match = re.search(r"^@@.*\+(\d+)(?:,(\d+))?", line) + if match: + start_line = int(match.group(1)) + line_count = 1 + if match.group(2): + line_count = int(match.group(2)) + # The input is something like + # + # @@ -1, +0,0 @@ + # + # which means no lines were added. + if line_count == 0: + continue + # Also format lines range if line_count is 0 in case of deleting + # surrounding statements. + end_line = start_line + if line_count != 0: + end_line += line_count - 1 + lines_by_file.setdefault(filename, []).extend( + ["-lines", str(start_line) + ":" + str(end_line)] + ) + + # Reformat files containing changes in place. + for filename, lines in lines_by_file.items(): + if args.i and args.verbose: + print("Formatting {}".format(filename)) + command = [args.binary, filename] + if args.i: + command.append("-i") + if args.sort_includes: + command.append("-sort-includes") + command.extend(lines) + if args.style: + command.extend(["-style", args.style]) + if args.fallback_style: + command.extend(["-fallback-style", args.fallback_style]) + + try: + p = subprocess.Popen( + command, + stdout=subprocess.PIPE, + stderr=None, + stdin=subprocess.PIPE, + universal_newlines=True, + ) + except OSError as e: + # Give the user more context when clang-format isn't + # found/isn't executable, etc. + raise RuntimeError( + 'Failed to run "%s" - %s"' % (" ".join(command), e.strerror) + ) + + stdout, stderr = p.communicate() + if p.returncode != 0: + sys.exit(p.returncode) + + if not args.i: + with open(filename, encoding="utf8") as f: + code = f.readlines() + formatted_code = StringIO(stdout).readlines() + diff = difflib.unified_diff( + code, + formatted_code, + filename, + filename, + "(before formatting)", + "(after formatting)", + ) + diff_string = "".join(diff) + if len(diff_string) > 0: + sys.stdout.write(diff_string) + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index 590c2ed87d..f57e9abfec 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -192,6 +192,16 @@ def check_MACHO_control_flow(binary) -> bool: return True return False +def check_MACHO_branch_protection(binary) -> bool: + ''' + Check for branch protection instrumentation + ''' + content = binary.get_content_from_virtual_address(binary.entrypoint, 4, lief.Binary.VA_TYPES.AUTO) + + if content.tolist() == [95, 36, 3, 213]: # bti + return True + return False + BASE_ELF = [ ('PIE', check_PIE), ('NX', check_NX), @@ -231,7 +241,7 @@ def check_MACHO_control_flow(binary) -> bool: lief.ARCHITECTURES.X86: BASE_MACHO + [('PIE', check_PIE), ('NX', check_NX), ('CONTROL_FLOW', check_MACHO_control_flow)], - lief.ARCHITECTURES.ARM64: BASE_MACHO, + lief.ARCHITECTURES.ARM64: BASE_MACHO + [('BRANCH_PROTECTION', check_MACHO_branch_protection)], } } diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index 64daabad4e..48823c7e45 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -137,12 +137,12 @@ def test_MACHO(self): else: # arm64 darwin doesn't support non-PIE binaries, control flow or executable stacks self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']), - (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains']), + (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS BRANCH_PROTECTION')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (1, executable+': failed NOUNDEFS Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (1, executable+': failed NOUNDEFS')) - self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (0, '')) diff --git a/depends/Makefile b/depends/Makefile index e8842b3f1e..88aae7ad81 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -147,8 +147,8 @@ include packages/packages.mk # 2. Before including packages/*.mk (excluding packages/packages.mk), since # they rely on the build_id variables # -build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(build_AR)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))') -$(host_arch)_$(host_os)_id:=$(shell env CC='$(host_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(host_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(host_AR)' RANLIB='$(host_RANLIB)' STRIP='$(host_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(HOST_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))') +build_id:=$(shell env CC='$(build_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(build_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(build_AR) 'NM='$(build_NM)' RANLIB='$(build_RANLIB)' STRIP='$(build_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(BUILD_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))') +$(host_arch)_$(host_os)_id:=$(shell env CC='$(host_CC)' C_STANDARD='$(C_STANDARD)' CXX='$(host_CXX)' CXX_STANDARD='$(CXX_STANDARD)' AR='$(host_AR)' NM='$(host_NM)' RANLIB='$(host_RANLIB)' STRIP='$(host_STRIP)' SHA256SUM='$(build_SHA256SUM)' DEBUG='$(DEBUG)' LTO='$(LTO)' NO_HARDEN='$(NO_HARDEN)' ./gen_id '$(HOST_ID_SALT)' 'GUIX_ENVIRONMENT=$(realpath $(GUIX_ENVIRONMENT))') boost_packages_$(NO_BOOST) = $(boost_packages) diff --git a/depends/README.md b/depends/README.md index a33037b6a6..a8dfc83e3b 100644 --- a/depends/README.md +++ b/depends/README.md @@ -120,7 +120,7 @@ The following can be set when running make: `make FOO=bar` - `LOG`: Use file-based logging for individual packages. During a package build its log file resides in the `depends` directory, and the log file is printed out automatically in case of build error. After successful build log files are moved along with package archives -- `LTO`: Use LTO when building packages. +- `LTO`: Enable options needed for LTO. Does not add `-flto` related options to *FLAGS. - `NO_HARDEN=1`: Don't use hardening options when building packages If some packages are not built, for example `make NO_WALLET=1`, the appropriate diff --git a/depends/builders/darwin.mk b/depends/builders/darwin.mk index 8ed82b276d..eb64c97f64 100644 --- a/depends/builders/darwin.mk +++ b/depends/builders/darwin.mk @@ -11,8 +11,8 @@ build_darwin_SHA256SUM=shasum -a 256 build_darwin_DOWNLOAD=curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o #darwin host on darwin builder. overrides darwin host preferences. -darwin_CC=$(shell xcrun -f clang) -mmacosx-version-min=$(OSX_MIN_VERSION) -isysroot$(shell xcrun --show-sdk-path) -darwin_CXX:=$(shell xcrun -f clang++) -mmacosx-version-min=$(OSX_MIN_VERSION) -stdlib=libc++ -isysroot$(shell xcrun --show-sdk-path) +darwin_CC=$(shell xcrun -f clang) -isysroot$(shell xcrun --show-sdk-path) +darwin_CXX:=$(shell xcrun -f clang++) -stdlib=libc++ -isysroot$(shell xcrun --show-sdk-path) darwin_AR:=$(shell xcrun -f ar) darwin_RANLIB:=$(shell xcrun -f ranlib) darwin_STRIP:=$(shell xcrun -f strip) diff --git a/depends/config.site.in b/depends/config.site.in index 9bc599e7cd..29b2a67ed0 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -78,10 +78,6 @@ if test "@host_os@" = darwin; then BREW=no fi -if test -z "$enable_lto" && test -n "@lto@"; then - enable_lto=yes -fi - if test -z "$enable_hardening" && test -n "@no_harden@"; then enable_hardening=no fi diff --git a/depends/funcs.mk b/depends/funcs.mk index 987be4e611..ce87aa579f 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -139,9 +139,9 @@ $(1)_config_env+=PKG_CONFIG_LIBDIR=$($($(1)_type)_prefix)/lib/pkgconfig $(1)_config_env+=PKG_CONFIG_PATH=$($($(1)_type)_prefix)/share/pkgconfig $(1)_config_env+=PKG_CONFIG_SYSROOT_DIR=/ $(1)_config_env+=CMAKE_MODULE_PATH=$($($(1)_type)_prefix)/lib/cmake -$(1)_config_env+=PATH=$(build_prefix)/bin:$(PATH) -$(1)_build_env+=PATH=$(build_prefix)/bin:$(PATH) -$(1)_stage_env+=PATH=$(build_prefix)/bin:$(PATH) +$(1)_config_env+=PATH="$(build_prefix)/bin:$(PATH)" +$(1)_build_env+=PATH="$(build_prefix)/bin:$(PATH)" +$(1)_stage_env+=PATH="$(build_prefix)/bin:$(PATH)" # Setting a --build type that differs from --host will explicitly enable # cross-compilation mode. Note that --build defaults to the output of diff --git a/depends/gen_id b/depends/gen_id index 3341310e46..8518b4e674 100755 --- a/depends/gen_id +++ b/depends/gen_id @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Usage: env [ CC=... ] [ C_STANDARD=...] [ CXX=... ] [CXX_STANDARD=...] \ -# [ AR=... ] [ RANLIB=... ] [ STRIP=... ] [ DEBUG=... ] \ +# [ AR=... ] [ NM=... ] [ RANLIB=... ] [ STRIP=... ] [ DEBUG=... ] \ # [ LTO=... ] [ NO_HARDEN=... ] ./build-id [ID_SALT]... # # Prints to stdout a SHA256 hash representing the current toolset, used by @@ -56,6 +56,11 @@ echo "ZERO_AR_DATE=${ZERO_AR_DATE}" echo "END AR" + echo "BEGIN NM" + bash -c "${NM} --version" + env | grep '^NM_' + echo "END NM" + echo "BEGIN RANLIB" bash -c "${RANLIB} --version" env | grep '^RANLIB_' diff --git a/depends/hosts/android.mk b/depends/hosts/android.mk index b53966dcf8..a1c8c56dba 100644 --- a/depends/hosts/android.mk +++ b/depends/hosts/android.mk @@ -9,11 +9,6 @@ endif android_CFLAGS=-std=$(C_STANDARD) android_CXXFLAGS=-std=$(CXX_STANDARD) -ifneq ($(LTO),) -android_CFLAGS += -flto -android_LDFLAGS += -flto -endif - android_AR=$(ANDROID_TOOLCHAIN_BIN)/llvm-ar android_RANLIB=$(ANDROID_TOOLCHAIN_BIN)/llvm-ranlib diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index a89c82e408..b94ac7d56f 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -79,28 +79,27 @@ $(foreach TOOL,$(cctools_TOOLS),$(eval darwin_$(TOOL) = $$(build_prefix)/bin/$$( darwin_CC=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \ -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH \ -u LIBRARY_PATH \ - $(clang_prog) --target=$(host) -mmacosx-version-min=$(OSX_MIN_VERSION) \ - -B$(build_prefix)/bin -mlinker-version=$(LD64_VERSION) \ + $(clang_prog) --target=$(host) \ + -B$(build_prefix)/bin \ -isysroot$(OSX_SDK) -nostdlibinc \ -iwithsysroot/usr/include -iframeworkwithsysroot/System/Library/Frameworks darwin_CXX=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \ -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH \ -u LIBRARY_PATH \ - $(clangxx_prog) --target=$(host) -mmacosx-version-min=$(OSX_MIN_VERSION) \ - -B$(build_prefix)/bin -mlinker-version=$(LD64_VERSION) \ + $(clangxx_prog) --target=$(host) \ + -B$(build_prefix)/bin \ -isysroot$(OSX_SDK) -nostdlibinc \ -iwithsysroot/usr/include/c++/v1 \ -iwithsysroot/usr/include -iframeworkwithsysroot/System/Library/Frameworks -darwin_CFLAGS=-pipe -std=$(C_STANDARD) -darwin_CXXFLAGS=-pipe -std=$(CXX_STANDARD) +darwin_CFLAGS=-pipe -std=$(C_STANDARD) -mmacosx-version-min=$(OSX_MIN_VERSION) +darwin_CXXFLAGS=-pipe -std=$(CXX_STANDARD) -mmacosx-version-min=$(OSX_MIN_VERSION) darwin_LDFLAGS=-Wl,-platform_version,macos,$(OSX_MIN_VERSION),$(OSX_SDK_VERSION) -ifneq ($(LTO),) -darwin_CFLAGS += -flto -darwin_CXXFLAGS += -flto -darwin_LDFLAGS += -flto +ifneq ($(build_os),darwin) +darwin_CFLAGS += -mlinker-version=$(LD64_VERSION) +darwin_CXXFLAGS += -mlinker-version=$(LD64_VERSION) endif darwin_release_CFLAGS=-O2 diff --git a/depends/hosts/freebsd.mk b/depends/hosts/freebsd.mk index 5351d0b900..055097b03d 100644 --- a/depends/hosts/freebsd.mk +++ b/depends/hosts/freebsd.mk @@ -1,12 +1,6 @@ freebsd_CFLAGS=-pipe -std=$(C_STANDARD) freebsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) -ifneq ($(LTO),) -freebsd_CFLAGS += -flto -freebsd_CXXFLAGS += -flto -freebsd_LDFLAGS += -flto -endif - freebsd_release_CFLAGS=-O2 freebsd_release_CXXFLAGS=$(freebsd_release_CFLAGS) diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index 1f33640c66..8be23be57d 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -2,10 +2,6 @@ linux_CFLAGS=-pipe -std=$(C_STANDARD) linux_CXXFLAGS=-pipe -std=$(CXX_STANDARD) ifneq ($(LTO),) -linux_CFLAGS += -flto -linux_CXXFLAGS += -flto -linux_LDFLAGS += -flto - linux_AR = $(host_toolchain)gcc-ar linux_NM = $(host_toolchain)gcc-nm linux_RANLIB = $(host_toolchain)gcc-ranlib diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk index fc1cc1afbe..15aa7cd25a 100644 --- a/depends/hosts/mingw32.mk +++ b/depends/hosts/mingw32.mk @@ -6,10 +6,6 @@ mingw32_CFLAGS=-pipe -std=$(C_STANDARD) mingw32_CXXFLAGS=-pipe -std=$(CXX_STANDARD) ifneq ($(LTO),) -mingw32_CFLAGS += -flto -mingw32_CXXFLAGS += -flto -mingw32_LDFLAGS += -flto - mingw32_AR = $(host_toolchain)gcc-ar mingw32_NM = $(host_toolchain)gcc-nm mingw32_RANLIB = $(host_toolchain)gcc-ranlib diff --git a/depends/hosts/netbsd.mk b/depends/hosts/netbsd.mk index 14121dca20..f33b2d2889 100644 --- a/depends/hosts/netbsd.mk +++ b/depends/hosts/netbsd.mk @@ -2,10 +2,6 @@ netbsd_CFLAGS=-pipe -std=$(C_STANDARD) netbsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) ifneq ($(LTO),) -netbsd_CFLAGS += -flto -netbsd_CXXFLAGS += -flto -netbsd_LDFLAGS += -flto - netbsd_AR = $(host_toolchain)gcc-ar netbsd_NM = $(host_toolchain)gcc-nm netbsd_RANLIB = $(host_toolchain)gcc-ranlib diff --git a/depends/hosts/openbsd.mk b/depends/hosts/openbsd.mk index d330e94d2e..bdd36dc9b3 100644 --- a/depends/hosts/openbsd.mk +++ b/depends/hosts/openbsd.mk @@ -1,12 +1,6 @@ openbsd_CFLAGS=-pipe -std=$(C_STANDARD) openbsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) -ifneq ($(LTO),) -openbsd_CFLAGS += -flto -openbsd_CXXFLAGS += -flto -openbsd_LDFLAGS += -flto -endif - openbsd_release_CFLAGS=-O2 openbsd_release_CXXFLAGS=$(openbsd_release_CFLAGS) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 1d670d39f1..ece36cb088 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -739,25 +739,25 @@ logging messages. They should be used as follows: Note that `LogPrint(BCLog::CATEGORY, fmt, params...)` is a deprecated alias for `LogDebug`. -- `LogInfo(fmt, params...)` should only be used rarely, eg for startup +- `LogInfo(fmt, params...)` should only be used rarely, e.g. for startup messages or for infrequent and important events such as a new block tip being found or a new outbound connection being made. These log messages - are unconditional so care must be taken that they can't be used by an + are unconditional, so care must be taken that they can't be used by an attacker to fill up storage. Note that `LogPrintf(fmt, params...)` is a deprecated alias for `LogInfo`. - `LogError(fmt, params...)` should be used in place of `LogInfo` for severe problems that require the node (or a subsystem) to shut down - entirely (eg, insufficient storage space). + entirely (e.g., insufficient storage space). - `LogWarning(fmt, params...)` should be used in place of `LogInfo` for severe problems that the node admin should address, but are not - severe enough to warrant shutting down the node (eg, system time + severe enough to warrant shutting down the node (e.g., system time appears to be wrong, unknown soft fork appears to have activated). -- `LogTrace(BCLog::CATEGORY, fmt, params...) should be used in place of +- `LogTrace(BCLog::CATEGORY, fmt, params...)` should be used in place of `LogDebug` for log messages that would be unusable on a production - system, eg due to being too noisy in normal use, or too resource + system, e.g. due to being too noisy in normal use, or too resource intensive to process. These will be logged if the startup options `-debug=category -loglevel=category:trace` or `-debug=1 -loglevel=trace` are selected. diff --git a/src/Makefile.am b/src/Makefile.am index 8b77f6632b..9e77b265c6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -8,8 +8,8 @@ print-%: FORCE DIST_SUBDIRS = secp256k1 -AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(LTO_LDFLAGS) $(CORE_LDFLAGS) -AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(LTO_CXXFLAGS) $(CORE_CXXFLAGS) +AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS) +AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) AM_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS) AM_LIBTOOLFLAGS = --preserve-dup-deps PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) diff --git a/src/addrman.h b/src/addrman.h index ef9c766eff..be2ee8c2cb 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -111,13 +111,16 @@ class AddrMan /** * Attempt to add one or more addresses to addrman's new table. + * If an address already exists in addrman, the existing entry may be updated + * (e.g. adding additional service flags). If the existing entry is in the new table, + * it may be added to more buckets, improving the probability of selection. * * @param[in] vAddr Address records to attempt to add. * @param[in] source The address of the node that sent us these addr records. * @param[in] time_penalty A "time penalty" to apply to the address record's nTime. If a peer * sends us an address record with nTime=n, then we'll add it to our * addrman with nTime=(n - time_penalty). - * @return true if at least one address is successfully added. */ + * @return true if at least one address is successfully added, or added to an additional bucket. Unaffected by updates. */ bool Add(const std::vector& vAddr, const CNetAddr& source, std::chrono::seconds time_penalty = 0s); /** diff --git a/src/crypto/chacha20.cpp b/src/crypto/chacha20.cpp index a3cc87e81b..4feed862cb 100644 --- a/src/crypto/chacha20.cpp +++ b/src/crypto/chacha20.cpp @@ -11,15 +11,14 @@ #include #include +#include #include -constexpr static inline uint32_t rotl32(uint32_t v, int c) { return (v << c) | (v >> (32 - c)); } - #define QUARTERROUND(a,b,c,d) \ - a += b; d = rotl32(d ^ a, 16); \ - c += d; b = rotl32(b ^ c, 12); \ - a += b; d = rotl32(d ^ a, 8); \ - c += d; b = rotl32(b ^ c, 7); + a += b; d = std::rotl(d ^ a, 16); \ + c += d; b = std::rotl(b ^ c, 12); \ + a += b; d = std::rotl(d ^ a, 8); \ + c += d; b = std::rotl(b ^ c, 7); #define REPEAT10(a) do { {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; } while(0) diff --git a/src/crypto/sha3.cpp b/src/crypto/sha3.cpp index 9c0c42fa77..770500bfe2 100644 --- a/src/crypto/sha3.cpp +++ b/src/crypto/sha3.cpp @@ -11,15 +11,10 @@ #include #include // For std::begin and std::end. +#include #include -// Internal implementation code. -namespace -{ -uint64_t Rotl(uint64_t x, int n) { return (x << n) | (x >> (64 - n)); } -} // namespace - void KeccakF(uint64_t (&st)[25]) { static constexpr uint64_t RNDC[24] = { @@ -41,38 +36,38 @@ void KeccakF(uint64_t (&st)[25]) bc2 = st[2] ^ st[7] ^ st[12] ^ st[17] ^ st[22]; bc3 = st[3] ^ st[8] ^ st[13] ^ st[18] ^ st[23]; bc4 = st[4] ^ st[9] ^ st[14] ^ st[19] ^ st[24]; - t = bc4 ^ Rotl(bc1, 1); st[0] ^= t; st[5] ^= t; st[10] ^= t; st[15] ^= t; st[20] ^= t; - t = bc0 ^ Rotl(bc2, 1); st[1] ^= t; st[6] ^= t; st[11] ^= t; st[16] ^= t; st[21] ^= t; - t = bc1 ^ Rotl(bc3, 1); st[2] ^= t; st[7] ^= t; st[12] ^= t; st[17] ^= t; st[22] ^= t; - t = bc2 ^ Rotl(bc4, 1); st[3] ^= t; st[8] ^= t; st[13] ^= t; st[18] ^= t; st[23] ^= t; - t = bc3 ^ Rotl(bc0, 1); st[4] ^= t; st[9] ^= t; st[14] ^= t; st[19] ^= t; st[24] ^= t; + t = bc4 ^ std::rotl(bc1, 1); st[0] ^= t; st[5] ^= t; st[10] ^= t; st[15] ^= t; st[20] ^= t; + t = bc0 ^ std::rotl(bc2, 1); st[1] ^= t; st[6] ^= t; st[11] ^= t; st[16] ^= t; st[21] ^= t; + t = bc1 ^ std::rotl(bc3, 1); st[2] ^= t; st[7] ^= t; st[12] ^= t; st[17] ^= t; st[22] ^= t; + t = bc2 ^ std::rotl(bc4, 1); st[3] ^= t; st[8] ^= t; st[13] ^= t; st[18] ^= t; st[23] ^= t; + t = bc3 ^ std::rotl(bc0, 1); st[4] ^= t; st[9] ^= t; st[14] ^= t; st[19] ^= t; st[24] ^= t; // Rho Pi t = st[1]; - bc0 = st[10]; st[10] = Rotl(t, 1); t = bc0; - bc0 = st[7]; st[7] = Rotl(t, 3); t = bc0; - bc0 = st[11]; st[11] = Rotl(t, 6); t = bc0; - bc0 = st[17]; st[17] = Rotl(t, 10); t = bc0; - bc0 = st[18]; st[18] = Rotl(t, 15); t = bc0; - bc0 = st[3]; st[3] = Rotl(t, 21); t = bc0; - bc0 = st[5]; st[5] = Rotl(t, 28); t = bc0; - bc0 = st[16]; st[16] = Rotl(t, 36); t = bc0; - bc0 = st[8]; st[8] = Rotl(t, 45); t = bc0; - bc0 = st[21]; st[21] = Rotl(t, 55); t = bc0; - bc0 = st[24]; st[24] = Rotl(t, 2); t = bc0; - bc0 = st[4]; st[4] = Rotl(t, 14); t = bc0; - bc0 = st[15]; st[15] = Rotl(t, 27); t = bc0; - bc0 = st[23]; st[23] = Rotl(t, 41); t = bc0; - bc0 = st[19]; st[19] = Rotl(t, 56); t = bc0; - bc0 = st[13]; st[13] = Rotl(t, 8); t = bc0; - bc0 = st[12]; st[12] = Rotl(t, 25); t = bc0; - bc0 = st[2]; st[2] = Rotl(t, 43); t = bc0; - bc0 = st[20]; st[20] = Rotl(t, 62); t = bc0; - bc0 = st[14]; st[14] = Rotl(t, 18); t = bc0; - bc0 = st[22]; st[22] = Rotl(t, 39); t = bc0; - bc0 = st[9]; st[9] = Rotl(t, 61); t = bc0; - bc0 = st[6]; st[6] = Rotl(t, 20); t = bc0; - st[1] = Rotl(t, 44); + bc0 = st[10]; st[10] = std::rotl(t, 1); t = bc0; + bc0 = st[7]; st[7] = std::rotl(t, 3); t = bc0; + bc0 = st[11]; st[11] = std::rotl(t, 6); t = bc0; + bc0 = st[17]; st[17] = std::rotl(t, 10); t = bc0; + bc0 = st[18]; st[18] = std::rotl(t, 15); t = bc0; + bc0 = st[3]; st[3] = std::rotl(t, 21); t = bc0; + bc0 = st[5]; st[5] = std::rotl(t, 28); t = bc0; + bc0 = st[16]; st[16] = std::rotl(t, 36); t = bc0; + bc0 = st[8]; st[8] = std::rotl(t, 45); t = bc0; + bc0 = st[21]; st[21] = std::rotl(t, 55); t = bc0; + bc0 = st[24]; st[24] = std::rotl(t, 2); t = bc0; + bc0 = st[4]; st[4] = std::rotl(t, 14); t = bc0; + bc0 = st[15]; st[15] = std::rotl(t, 27); t = bc0; + bc0 = st[23]; st[23] = std::rotl(t, 41); t = bc0; + bc0 = st[19]; st[19] = std::rotl(t, 56); t = bc0; + bc0 = st[13]; st[13] = std::rotl(t, 8); t = bc0; + bc0 = st[12]; st[12] = std::rotl(t, 25); t = bc0; + bc0 = st[2]; st[2] = std::rotl(t, 43); t = bc0; + bc0 = st[20]; st[20] = std::rotl(t, 62); t = bc0; + bc0 = st[14]; st[14] = std::rotl(t, 18); t = bc0; + bc0 = st[22]; st[22] = std::rotl(t, 39); t = bc0; + bc0 = st[9]; st[9] = std::rotl(t, 61); t = bc0; + bc0 = st[6]; st[6] = std::rotl(t, 20); t = bc0; + st[1] = std::rotl(t, 44); // Chi Iota bc0 = st[0]; bc1 = st[1]; bc2 = st[2]; bc3 = st[3]; bc4 = st[4]; diff --git a/src/crypto/siphash.cpp b/src/crypto/siphash.cpp index 2c328d0840..8004a0548e 100644 --- a/src/crypto/siphash.cpp +++ b/src/crypto/siphash.cpp @@ -4,15 +4,15 @@ #include -#define ROTL(x, b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b)))) +#include #define SIPROUND do { \ - v0 += v1; v1 = ROTL(v1, 13); v1 ^= v0; \ - v0 = ROTL(v0, 32); \ - v2 += v3; v3 = ROTL(v3, 16); v3 ^= v2; \ - v0 += v3; v3 = ROTL(v3, 21); v3 ^= v0; \ - v2 += v1; v1 = ROTL(v1, 17); v1 ^= v2; \ - v2 = ROTL(v2, 32); \ + v0 += v1; v1 = std::rotl(v1, 13); v1 ^= v0; \ + v0 = std::rotl(v0, 32); \ + v2 += v3; v3 = std::rotl(v3, 16); v3 ^= v2; \ + v0 += v3; v3 = std::rotl(v3, 21); v3 ^= v0; \ + v2 += v1; v1 = std::rotl(v1, 17); v1 ^= v2; \ + v2 = std::rotl(v2, 32); \ } while (0) CSipHasher::CSipHasher(uint64_t k0, uint64_t k1) diff --git a/src/hash.cpp b/src/hash.cpp index 1ece8c5a79..78a969ecd3 100644 --- a/src/hash.cpp +++ b/src/hash.cpp @@ -7,13 +7,9 @@ #include #include +#include #include -inline uint32_t ROTL32(uint32_t x, int8_t r) -{ - return (x << r) | (x >> (32 - r)); -} - unsigned int MurmurHash3(unsigned int nHashSeed, Span vDataToHash) { // The following is MurmurHash3 (x86_32), see https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp @@ -31,11 +27,11 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span vData uint32_t k1 = ReadLE32(blocks + i*4); k1 *= c1; - k1 = ROTL32(k1, 15); + k1 = std::rotl(k1, 15); k1 *= c2; h1 ^= k1; - h1 = ROTL32(h1, 13); + h1 = std::rotl(h1, 13); h1 = h1 * 5 + 0xe6546b64; } @@ -55,7 +51,7 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span vData case 1: k1 ^= tail[0]; k1 *= c1; - k1 = ROTL32(k1, 15); + k1 = std::rotl(k1, 15); k1 *= c2; h1 ^= k1; } diff --git a/src/init/common.cpp b/src/init/common.cpp index 6560258ef5..0800cd93d8 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -31,7 +31,7 @@ void AddLoggingArgs(ArgsManager& argsman) ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-debugexclude=", "Exclude debug and trace logging for a category. Can be used in conjunction with -debug=1 to output debug and trace logging for all categories except the specified category. This option can be specified multiple times to exclude multiple categories.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-loglevel=|:", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC: %s (default=%s); warning and error levels are always logged. If : is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-loglevel=|:", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC. Possible values are %s (default=%s). The following levels are always logged: error, warning, info. If : is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); #ifdef HAVE_THREAD_LOCAL argsman.AddArg("-logthreadnames", strprintf("Prepend debug output with name of the originating thread (only available on platforms supporting thread_local) (default: %u)", DEFAULT_LOGTHREADNAMES), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index 0808d42452..cae4b04bf6 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -67,10 +67,20 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active return false; } file.SetXor(xor_key); - uint64_t num; - file >> num; - while (num) { - --num; + uint64_t total_txns_to_load; + file >> total_txns_to_load; + uint64_t txns_tried = 0; + LogInfo("Loading %u mempool transactions from disk...\n", total_txns_to_load); + int next_tenth_to_report = 0; + while (txns_tried < total_txns_to_load) { + const int percentage_done(100.0 * txns_tried / total_txns_to_load); + if (next_tenth_to_report < percentage_done / 10) { + LogInfo("Progress loading mempool transactions from disk: %d%% (tried %u, %u remaining)\n", + percentage_done, txns_tried, total_txns_to_load - txns_tried); + next_tenth_to_report = percentage_done / 10; + } + ++txns_tried; + CTransactionRef tx; int64_t nTime; int64_t nFeeDelta; diff --git a/src/key.cpp b/src/key.cpp index 512790252a..2bd6396298 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -179,7 +179,7 @@ CPrivKey CKey::GetPrivKey() const { size_t seckeylen; seckey.resize(SIZE); seckeylen = SIZE; - ret = ec_seckey_export_der(secp256k1_context_sign, seckey.data(), &seckeylen, begin(), fCompressed); + ret = ec_seckey_export_der(secp256k1_context_sign, seckey.data(), &seckeylen, UCharCast(begin()), fCompressed); assert(ret); seckey.resize(seckeylen); return seckey; @@ -190,7 +190,7 @@ CPubKey CKey::GetPubKey() const { secp256k1_pubkey pubkey; size_t clen = CPubKey::SIZE; CPubKey result; - int ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pubkey, begin()); + int ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pubkey, UCharCast(begin())); assert(ret); secp256k1_ec_pubkey_serialize(secp256k1_context_sign, (unsigned char*)result.begin(), &clen, &pubkey, fCompressed ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED); assert(result.size() == clen); @@ -220,19 +220,19 @@ bool CKey::Sign(const uint256 &hash, std::vector& vchSig, bool gr WriteLE32(extra_entropy, test_case); secp256k1_ecdsa_signature sig; uint32_t counter = 0; - int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, (!grind && test_case) ? extra_entropy : nullptr); + int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), UCharCast(begin()), secp256k1_nonce_function_rfc6979, (!grind && test_case) ? extra_entropy : nullptr); // Grind for low R while (ret && !SigHasLowR(&sig) && grind) { WriteLE32(extra_entropy, ++counter); - ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, extra_entropy); + ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), UCharCast(begin()), secp256k1_nonce_function_rfc6979, extra_entropy); } assert(ret); secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); // Additional verification step to prevent using a potentially corrupted signature secp256k1_pubkey pk; - ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, begin()); + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, UCharCast(begin())); assert(ret); ret = secp256k1_ecdsa_verify(secp256k1_context_static, &sig, hash.begin(), &pk); assert(ret); @@ -258,7 +258,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector& vchSig) vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE); int rec = -1; secp256k1_ecdsa_recoverable_signature rsig; - int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); + int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), UCharCast(begin()), secp256k1_nonce_function_rfc6979, nullptr); assert(ret); ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &rsig); assert(ret); @@ -266,7 +266,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector& vchSig) vchSig[0] = 27 + rec + (fCompressed ? 4 : 0); // Additional verification step to prevent using a potentially corrupted signature secp256k1_pubkey epk, rpk; - ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, begin()); + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, UCharCast(begin())); assert(ret); ret = secp256k1_ecdsa_recover(secp256k1_context_static, &rpk, &rsig, hash.begin()); assert(ret); @@ -279,7 +279,7 @@ bool CKey::SignSchnorr(const uint256& hash, Span sig, const uint2 { assert(sig.size() == 64); secp256k1_keypair keypair; - if (!secp256k1_keypair_create(secp256k1_context_sign, &keypair, begin())) return false; + if (!secp256k1_keypair_create(secp256k1_context_sign, &keypair, UCharCast(begin()))) return false; if (merkle_root) { secp256k1_xonly_pubkey pubkey; if (!secp256k1_keypair_xonly_pub(secp256k1_context_sign, &pubkey, nullptr, &keypair)) return false; @@ -324,7 +324,7 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const BIP32Hash(cc, nChild, *pubkey.begin(), pubkey.begin()+1, vout.data()); } else { assert(size() == 32); - BIP32Hash(cc, nChild, 0, begin(), vout.data()); + BIP32Hash(cc, nChild, 0, UCharCast(begin()), vout.data()); } memcpy(ccChild.begin(), vout.data()+32, 32); keyChild.Set(begin(), begin() + 32, true); diff --git a/src/key.h b/src/key.h index 9cac716ec8..d6b26f891d 100644 --- a/src/key.h +++ b/src/key.h @@ -100,7 +100,7 @@ class CKey { if (size_t(pend - pbegin) != std::tuple_size_v) { ClearKeyData(); - } else if (Check(&pbegin[0])) { + } else if (Check(UCharCast(&pbegin[0]))) { MakeKeyData(); memcpy(keydata->data(), (unsigned char*)&pbegin[0], keydata->size()); fCompressed = fCompressedIn; @@ -112,8 +112,8 @@ class CKey //! Simple read-only vector-like interface. unsigned int size() const { return keydata ? keydata->size() : 0; } const std::byte* data() const { return keydata ? reinterpret_cast(keydata->data()) : nullptr; } - const unsigned char* begin() const { return keydata ? keydata->data() : nullptr; } - const unsigned char* end() const { return begin() + size(); } + const std::byte* begin() const { return data(); } + const std::byte* end() const { return data() + size(); } //! Check whether this private key is valid. bool IsValid() const { return !!keydata; } diff --git a/src/key_io.cpp b/src/key_io.cpp index 5bcbb8a069..a373a2201d 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -228,7 +228,7 @@ std::string EncodeSecret(const CKey& key) { assert(key.IsValid()); std::vector data = Params().Base58Prefix(CChainParams::SECRET_KEY); - data.insert(data.end(), key.begin(), key.end()); + data.insert(data.end(), UCharCast(key.begin()), UCharCast(key.end())); if (key.IsCompressed()) { data.push_back(1); } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ce8dfd491c..479942a9a1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3426,6 +3426,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, vRecv >> CNetAddr::V1(addrMe); if (!pfrom.IsInboundConn()) { + // Overwrites potentially existing services. In contrast to this, + // unvalidated services received via gossip relay in ADDR/ADDRV2 + // messages are only ever added but cannot replace existing ones. m_addrman.SetServices(pfrom.addr, nServices); } if (pfrom.ExpectServicesFromConn() && !HasAllDesirableServiceFlags(nServices)) diff --git a/src/pubkey.h b/src/pubkey.h index 2b655c3f73..15d7e7bc07 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -285,10 +285,11 @@ class XOnlyPubKey CPubKey GetEvenCorrespondingCPubKey() const; const unsigned char& operator[](int pos) const { return *(m_keydata.begin() + pos); } - const unsigned char* data() const { return m_keydata.begin(); } static constexpr size_t size() { return decltype(m_keydata)::size(); } + const unsigned char* data() const { return m_keydata.begin(); } const unsigned char* begin() const { return m_keydata.begin(); } const unsigned char* end() const { return m_keydata.end(); } + unsigned char* data() { return m_keydata.begin(); } unsigned char* begin() { return m_keydata.begin(); } unsigned char* end() { return m_keydata.end(); } bool operator==(const XOnlyPubKey& other) const { return m_keydata == other.m_keydata; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 5285ef0433..fcea4c612e 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2873,12 +2873,11 @@ static RPCHelpMan loadtxoutset() if (!chainman.ActivateSnapshot(afile, metadata, false)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to load UTXO snapshot " + fs::PathToString(path)); } - CBlockIndex* new_tip{WITH_LOCK(::cs_main, return chainman.ActiveTip())}; UniValue result(UniValue::VOBJ); result.pushKV("coins_loaded", metadata.m_coins_count); - result.pushKV("tip_hash", new_tip->GetBlockHash().ToString()); - result.pushKV("base_height", new_tip->nHeight); + result.pushKV("tip_hash", snapshot_start_block->GetBlockHash().ToString()); + result.pushKV("base_height", snapshot_start_block->nHeight); result.pushKV("path", fs::PathToString(path)); return result; }, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 5fea8c22c9..e53f889897 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -313,7 +313,7 @@ static RPCHelpMan addnode() { {"node", RPCArg::Type::STR, RPCArg::Optional::NO, "The address of the peer to connect to"}, {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "'add' to add a node to the list, 'remove' to remove a node from the list, 'onetry' to try a connection to the node once"}, - {"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol (ignored for 'remove' command)"}, + {"v2transport", RPCArg::Type::BOOL, RPCArg::DefaultHint{"set by -v2transport"}, "Attempt to connect using BIP324 v2 transport protocol (ignored for 'remove' command)"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -332,9 +332,10 @@ static RPCHelpMan addnode() CConnman& connman = EnsureConnman(node); const std::string node_arg{request.params[0].get_str()}; - bool use_v2transport = self.Arg(2); + bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2; + bool use_v2transport = self.MaybeArg(2).value_or(node_v2transport); - if (use_v2transport && !(node.connman->GetLocalServices() & NODE_P2P_V2)) { + if (use_v2transport && !node_v2transport) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: v2transport requested but not enabled (see -v2transport)"); } diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 5bd4f271cd..9668a85484 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -1068,7 +1068,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) BOOST_AUTO_TEST_CASE(addrman_update_address) { - // Tests updating nTime via Connected() and nServices via SetServices() + // Tests updating nTime via Connected() and nServices via SetServices() and Add() auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source{ResolveIP("252.2.2.2")}; CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)}; @@ -1095,6 +1095,32 @@ BOOST_AUTO_TEST_CASE(addrman_update_address) BOOST_CHECK_EQUAL(vAddr2.size(), 1U); BOOST_CHECK(vAddr2.at(0).nTime >= start_time + 10000s); BOOST_CHECK_EQUAL(vAddr2.at(0).nServices, NODE_NETWORK_LIMITED); + + // Updating an existing addr through Add() (used in gossip relay) can add additional services but can't remove existing ones. + CAddress addr_v2{CAddress(ResolveService("250.1.1.1", 8333), NODE_P2P_V2)}; + addr_v2.nTime = start_time; + BOOST_CHECK(!addrman->Add({addr_v2}, source)); + std::vector vAddr3{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr3.size(), 1U); + BOOST_CHECK_EQUAL(vAddr3.at(0).nServices, NODE_P2P_V2 | NODE_NETWORK_LIMITED); + + // SetServices() (used when we connected to them) overwrites existing service flags + addrman->SetServices(addr, NODE_NETWORK); + std::vector vAddr4{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr4.size(), 1U); + BOOST_CHECK_EQUAL(vAddr4.at(0).nServices, NODE_NETWORK); + + // Promoting to Tried does not affect the service flags + BOOST_CHECK(addrman->Good(addr)); // addr has NODE_NONE + std::vector vAddr5{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr5.size(), 1U); + BOOST_CHECK_EQUAL(vAddr5.at(0).nServices, NODE_NETWORK); + + // Adding service flags even works when the addr is in Tried + BOOST_CHECK(!addrman->Add({addr_v2}, source)); + std::vector vAddr6{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr6.size(), 1U); + BOOST_CHECK_EQUAL(vAddr6.at(0).nServices, NODE_NETWORK | NODE_P2P_V2); } BOOST_AUTO_TEST_CASE(addrman_size) diff --git a/src/validation.cpp b/src/validation.cpp index 4af3b3a7ae..074e68a363 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4925,7 +4925,9 @@ void ChainstateManager::CheckBlockIndex() // For testing, allow transaction counts to be completely unset. || (pindex->nChainTx == 0 && pindex->nTx == 0) // For testing, allow this nChainTx to be unset if previous is also unset. - || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev)); + || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) + // Transaction counts prior to snapshot are unknown. + || pindex->IsAssumedValid()); if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 8f0b72c2b0..6bbe6a1647 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -280,7 +280,7 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat { const CKey &key = mKey.second; CPubKey vchPubKey = key.GetPubKey(); - CKeyingMaterial vchSecret(key.begin(), key.end()); + CKeyingMaterial vchSecret{UCharCast(key.begin()), UCharCast(key.end())}; std::vector vchCryptedSecret; if (!EncryptSecret(master_key, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) { encrypted_batch = nullptr; @@ -810,7 +810,7 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu } std::vector vchCryptedSecret; - CKeyingMaterial vchSecret(key.begin(), key.end()); + CKeyingMaterial vchSecret{UCharCast(key.begin()), UCharCast(key.end())}; if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) { return false; } @@ -2088,7 +2088,7 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle { const CKey &key = key_in.second; CPubKey pubkey = key.GetPubKey(); - CKeyingMaterial secret(key.begin(), key.end()); + CKeyingMaterial secret{UCharCast(key.begin()), UCharCast(key.end())}; std::vector crypted_secret; if (!EncryptSecret(master_key, secret, pubkey.GetHash(), crypted_secret)) { return false; @@ -2261,7 +2261,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const } std::vector crypted_secret; - CKeyingMaterial secret(key.begin(), key.end()); + CKeyingMaterial secret{UCharCast(key.begin()), UCharCast(key.end())}; if (!EncryptSecret(m_storage.GetEncryptionKey(), secret, pubkey.GetHash(), crypted_secret)) { return false; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bf8cfcb082..e03f5532fc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3099,6 +3099,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri if (time_first_key) walletInstance->MaybeUpdateBirthTime(*time_first_key); if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { + walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded return nullptr; } diff --git a/test/README.md b/test/README.md index fee8828d34..0d1e06c0ad 100644 --- a/test/README.md +++ b/test/README.md @@ -326,35 +326,7 @@ Use the `-v` option for verbose output. ### Lint tests -#### Dependencies - -| Lint test | Dependency | -|-----------|:----------:| -| [`lint-python.py`](lint/lint-python.py) | [flake8](https://gitlab.com/pycqa/flake8) -| [`lint-python.py`](lint/lint-python.py) | [lief](https://github.com/lief-project/LIEF) -| [`lint-python.py`](lint/lint-python.py) | [mypy](https://github.com/python/mypy) -| [`lint-python.py`](lint/lint-python.py) | [pyzmq](https://github.com/zeromq/pyzmq) -| [`lint-python-dead-code.py`](lint/lint-python-dead-code.py) | [vulture](https://github.com/jendrikseipp/vulture) -| [`lint-shell.py`](lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck) -| [`lint-spelling.py`](lint/lint-spelling.py) | [codespell](https://github.com/codespell-project/codespell) - -In use versions and install instructions are available in the [CI setup](../ci/lint/04_install.sh). - -Please be aware that on Linux distributions all dependencies are usually available as packages, but could be outdated. - -#### Running the tests - -Individual tests can be run by directly calling the test script, e.g.: - -``` -test/lint/lint-files.py -``` - -You can run all the shell-based lint tests by running: - -``` -test/lint/all-lint.py -``` +See the README in [test/lint](/test/lint). # Writing functional tests diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 0ee332b75b..d8ae20981d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -602,10 +602,10 @@ def connect_nodes(self, a, b, *, peer_advertises_v2=None, wait_for_connect: bool if peer_advertises_v2 is None: peer_advertises_v2 = from_connection.use_v2transport - if peer_advertises_v2: - from_connection.addnode(node=ip_port, command="onetry", v2transport=True) + if peer_advertises_v2 != from_connection.use_v2transport: + from_connection.addnode(node=ip_port, command="onetry", v2transport=peer_advertises_v2) else: - # skip the optional third argument (default false) for + # skip the optional third argument if it matches the default, for # compatibility with older clients from_connection.addnode(ip_port, "onetry") @@ -1021,5 +1021,4 @@ def is_bdb_compiled(self): return self.config["components"].getboolean("USE_BDB") def has_blockfile(self, node, filenum: str): - blocksdir = node.datadir_path / self.chain / 'blocks' - return (blocksdir / f"blk{filenum}.dat").is_file() + return (node.blocks_path/ f"blk{filenum}.dat").is_file() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 17b92be4f4..1225f18535 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -336,6 +336,7 @@ 'wallet_create_tx.py --descriptors', 'wallet_inactive_hdchains.py --legacy-wallet', 'wallet_spend_unconfirmed.py', + 'wallet_rescan_unconfirmed.py --descriptors', 'p2p_fingerprint.py', 'feature_uacomment.py', 'feature_init.py', diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 0ac67607e1..7f01d23941 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -20,7 +20,10 @@ """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.address import AddressType +from test_framework.address import ( + AddressType, + ADDRESS_BCRT1_UNSPENDABLE, +) from test_framework.util import ( assert_equal, set_node_times, @@ -109,7 +112,7 @@ def check(self, txid=None, amount=None, confirmation_height=None): address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) - assert_equal(address["amount"], self.expected_balance) + assert_equal(address["amount"], self.amount_received) assert_equal(address["confirmations"], confirmations) # Verify the transaction is correctly marked watchonly depending on # whether the transaction pays to an imported public key or @@ -223,11 +226,11 @@ def run_test(self): variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] variant.do_import(variant.timestamp) if expect_rescan: - variant.expected_balance = variant.initial_amount + variant.amount_received = variant.initial_amount variant.expected_txs = 1 variant.check(variant.initial_txid, variant.initial_amount, variant.confirmation_height) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() @@ -247,7 +250,7 @@ def run_test(self): # Check the latest results from getbalance and listtransactions. for variant in IMPORT_VARIANTS: self.log.info('Run check for variant {}'.format(variant)) - variant.expected_balance += variant.sent_amount + variant.amount_received += variant.sent_amount variant.expected_txs += 1 variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height) @@ -267,14 +270,45 @@ def run_test(self): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() + variant.initial_amount = get_rand_amount() * 2 variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.confirmation_height = 0 variant.timestamp = timestamp + # Mine a block so these parents are confirmed + assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) + self.sync_mempools() + block_to_disconnect = self.generate(self.nodes[0], 1)[0] + assert_equal(len(self.nodes[0].getrawmempool()), 0) + + # For each variant, create an unconfirmed child transaction from initial_txid, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + # An equivalent test for descriptors exists in wallet_rescan_unconfirmed.py. + unspent_txid_map = {txin["txid"] : txin for txin in self.nodes[1].listunspent()} + for variant in mempool_variants: + # Send full amount, subtracting fee from outputs, to ensure no change is created. + child = self.nodes[1].send( + add_to_wallet=False, + inputs=[unspent_txid_map[variant.initial_txid]], + outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}], + subtract_fee_from_outputs=[0] + ) + variant.child_txid = child["txid"] + variant.amount_received = 0 + self.nodes[0].sendrawtransaction(child["hex"]) + + # Mempools should contain the child transactions for each variant. assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) self.sync_mempools() + # Mock a reorg so the parent transactions are added back to the mempool + for node in self.nodes: + node.invalidateblock(block_to_disconnect) + # Mempools should now contain the parent and child for each variant. + assert_equal(len(node.getrawmempool()), 2 * len(mempool_variants)) + # For each variation of wallet key import, invoke the import RPC and # check the results from getbalance and listtransactions. for variant in mempool_variants: @@ -283,11 +317,15 @@ def run_test(self): variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] variant.do_import(variant.timestamp) if expect_rescan: - variant.expected_balance = variant.initial_amount + # Ensure both transactions were rescanned. This would raise a JSONRPCError if the + # transactions were not identified as belonging to the wallet. + assert_equal(variant.node.gettransaction(variant.initial_txid)['confirmations'], 0) + assert_equal(variant.node.gettransaction(variant.child_txid)['confirmations'], 0) + variant.amount_received = variant.initial_amount variant.expected_txs = 1 - variant.check(variant.initial_txid, variant.initial_amount) + variant.check(variant.initial_txid, variant.initial_amount, 0) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() diff --git a/test/functional/wallet_rescan_unconfirmed.py b/test/functional/wallet_rescan_unconfirmed.py new file mode 100755 index 0000000000..ad9fa081c2 --- /dev/null +++ b/test/functional/wallet_rescan_unconfirmed.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024 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 that descriptor wallets rescan mempool transactions properly when importing.""" + +from test_framework.address import ( + address_to_scriptpubkey, + ADDRESS_BCRT1_UNSPENDABLE, +) +from test_framework.messages import COIN +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet +from test_framework.wallet_util import test_address + + +class WalletRescanUnconfirmed(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + + def set_test_params(self): + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + self.skip_if_no_sqlite() + + def run_test(self): + self.log.info("Create wallets and mine initial chain") + node = self.nodes[0] + tester_wallet = MiniWallet(node) + + node.createwallet(wallet_name='w0', disable_private_keys=False) + w0 = node.get_wallet_rpc('w0') + + self.log.info("Create a parent tx and mine it in a block that will later be disconnected") + parent_address = w0.getnewaddress() + tx_parent_to_reorg = tester_wallet.send_to( + from_node=node, + scriptPubKey=address_to_scriptpubkey(parent_address), + amount=COIN, + ) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + block_to_reorg = self.generate(tester_wallet, 1)[0] + assert_equal(len(node.getrawmempool()), 0) + node.syncwithvalidationinterfacequeue() + assert_equal(w0.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 1) + + # Create an unconfirmed child transaction from the parent tx, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + w0_utxos = w0.listunspent() + + self.log.info("Create a child tx and wait for it to propagate to all mempools") + # The only UTXO available to spend is tx_parent_to_reorg. + assert_equal(len(w0_utxos), 1) + assert_equal(w0_utxos[0]["txid"], tx_parent_to_reorg["txid"]) + tx_child_unconfirmed_sweep = w0.sendall([ADDRESS_BCRT1_UNSPENDABLE]) + assert tx_child_unconfirmed_sweep["txid"] in node.getrawmempool() + node.syncwithvalidationinterfacequeue() + + self.log.info("Mock a reorg, causing parent to re-enter mempools after its child") + node.invalidateblock(block_to_reorg) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + + self.log.info("Import descriptor wallet on another node") + descriptors_to_import = [{"desc": w0.getaddressinfo(parent_address)['parent_desc'], "timestamp": 0, "label": "w0 import"}] + + node.createwallet(wallet_name="w1", disable_private_keys=True) + w1 = node.get_wallet_rpc("w1") + w1.importdescriptors(descriptors_to_import) + + self.log.info("Check that the importing node has properly rescanned mempool transactions") + # Check that parent address is correctly determined as ismine + test_address(w1, parent_address, solvable=True, ismine=True) + # This would raise a JSONRPCError if the transactions were not identified as belonging to the wallet. + assert_equal(w1.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 0) + assert_equal(w1.gettransaction(tx_child_unconfirmed_sweep["txid"])["confirmations"], 0) + +if __name__ == '__main__': + WalletRescanUnconfirmed().main() diff --git a/test/lint/README.md b/test/lint/README.md index 484008298b..1fba41d9ea 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -16,12 +16,36 @@ result is cached and it prevents issues when the image changes. test runner =========== -To run the checks in the test runner outside the docker, use: +To run all the lint checks in the test runner outside the docker, use: ```sh ( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && cargo run ) ``` +#### Dependencies + +| Lint test | Dependency | +|-----------|:----------:| +| [`lint-python.py`](lint/lint-python.py) | [flake8](https://gitlab.com/pycqa/flake8) +| [`lint-python.py`](lint/lint-python.py) | [lief](https://github.com/lief-project/LIEF) +| [`lint-python.py`](lint/lint-python.py) | [mypy](https://github.com/python/mypy) +| [`lint-python.py`](lint/lint-python.py) | [pyzmq](https://github.com/zeromq/pyzmq) +| [`lint-python-dead-code.py`](lint/lint-python-dead-code.py) | [vulture](https://github.com/jendrikseipp/vulture) +| [`lint-shell.py`](lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck) +| [`lint-spelling.py`](lint/lint-spelling.py) | [codespell](https://github.com/codespell-project/codespell) + +In use versions and install instructions are available in the [CI setup](../ci/lint/04_install.sh). + +Please be aware that on Linux distributions all dependencies are usually available as packages, but could be outdated. + +#### Running the tests + +Individual tests can be run by directly calling the test script, e.g.: + +``` +test/lint/lint-files.py +``` + check-doc.py ============ Check for missing documentation of command line options. @@ -59,7 +83,3 @@ To do so, add the upstream repository as remote: ``` git remote add --fetch secp256k1 https://github.com/bitcoin-core/secp256k1.git ``` - -all-lint.py -=========== -Calls other scripts with the `lint-` prefix. diff --git a/test/lint/all-lint.py b/test/lint/all-lint.py deleted file mode 100755 index c7889796c6..0000000000 --- a/test/lint/all-lint.py +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (c) 2017-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. -# -# This script runs all test/lint/lint-* files, and fails if any exit -# with a non-zero status code. - -from glob import glob -from pathlib import Path -from subprocess import run -from sys import executable - -exit_code = 0 -mod_path = Path(__file__).parent -for lint in glob(f"{mod_path}/lint-*.py"): - result = run([executable, lint]) - if result.returncode != 0: - print(f"^---- failure generated from {lint.split('/')[-1]}") - exit_code |= result.returncode - -exit(exit_code) diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index ce94c3b628..1dc79e97bd 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -3,6 +3,7 @@ // file COPYING or https://opensource.org/license/mit/. use std::env; +use std::fs; use std::path::PathBuf; use std::process::Command; use std::process::ExitCode; @@ -29,8 +30,8 @@ fn check_output(cmd: &mut std::process::Command) -> Result { } /// Return the git root as utf8, or panic -fn get_git_root() -> String { - check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap() +fn get_git_root() -> PathBuf { + PathBuf::from(check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap()) } fn lint_subtree() -> LintResult { @@ -94,11 +95,24 @@ fn lint_doc() -> LintResult { } fn lint_all() -> LintResult { - if Command::new("test/lint/all-lint.py") - .status() - .expect("command error") - .success() - { + let mut good = true; + let lint_dir = get_git_root().join("test/lint"); + for entry in fs::read_dir(lint_dir).unwrap() { + let entry = entry.unwrap(); + let entry_fn = entry.file_name().into_string().unwrap(); + if entry_fn.starts_with("lint-") + && entry_fn.ends_with(".py") + && !Command::new("python3") + .arg(entry.path()) + .status() + .expect("command error") + .success() + { + good = false; + println!("^---- failure generated from {}", entry_fn); + } + } + if good { Ok(()) } else { Err("".to_string()) @@ -110,10 +124,10 @@ fn main() -> ExitCode { ("subtree check", lint_subtree), ("std::filesystem check", lint_std_filesystem), ("-help=1 documentation check", lint_doc), - ("all-lint.py script", lint_all), + ("lint-*.py scripts", lint_all), ]; - let git_root = PathBuf::from(get_git_root()); + let git_root = get_git_root(); let mut test_failed = false; for (lint_name, lint_fn) in test_list { diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index f0ee698909..b22caa15a5 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -69,7 +69,6 @@ implicit-signed-integer-truncation:crypto/ implicit-unsigned-integer-truncation:crypto/ shift-base:arith_uint256.cpp shift-base:crypto/ -shift-base:ROTL32 shift-base:streams.h shift-base:FormatHDKeypath shift-base:xoroshiro128plusplus.h