diff options
99 files changed, 1473 insertions, 790 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index 1a5d94dcfe..4a6e73ac85 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -4,7 +4,7 @@ env: # Global defaults MAKEJOBS: "-j10" TEST_RUNNER_PORT_MIN: "14000" # Must be larger than 12321, which is used for the http cache. See https://cirrus-ci.org/guide/writing-tasks/#http-cache CI_FAILFAST_TEST_LEAVE_DANGLING: "1" # Cirrus CI does not care about dangling process and setting this variable avoids killing the CI script itself on error - CCACHE_SIZE: "200M" + CCACHE_MAXSIZE: "200M" CCACHE_DIR: "/tmp/ccache_dir" CCACHE_NOHASHDIR: "1" # Debug info might contain a stale path if the build dir changes, but this is fine @@ -14,8 +14,24 @@ cirrus_ephemeral_worker_template_env: &CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV persistent_worker_template_env: &PERSISTENT_WORKER_TEMPLATE_ENV RESTART_CI_DOCKER_BEFORE_RUN: "1" +# https://cirrus-ci.org/guide/persistent-workers/ +# +# It is possible to select a specific persistent worker by label. Refer to the +# Cirrus CI docs for more details. +# +# Generally, a persistent worker must run Ubuntu 23.04+ or Debian 12+. +# Specifically, +# - apt-get is required due to PACKAGE_MANAGER_INSTALL +# - podman-docker-4.1+ is required due to the use of `podman` when +# RESTART_CI_DOCKER_BEFORE_RUN is set and 4.1+ due to the bugfix in 4.1 +# (https://github.com/bitcoin/bitcoin/pull/21652) +# - The ./ci/ depedencies should be installed: +# apt update && apt install screen python3 bash podman-docker curl -y +# +# The following specific types should exist, with the following requirements: +# - lunar: For a machine running the Linux kernel shipped with Ubuntu Lunar 23.04. The machine is recommended to have 4 CPUs and 16 GB of memory. persistent_worker_template: &PERSISTENT_WORKER_TEMPLATE - persistent_worker: {} # https://cirrus-ci.org/guide/persistent-workers/ + persistent_worker: {} # Only use this if the task does not care about the type at all # https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks filter_template: &FILTER_TEMPLATE @@ -170,7 +186,7 @@ task: build_script: - '%x64_NATIVE_TOOLS%' - cd %CIRRUS_WORKING_DIR% - - ccache --zero-stats --max-size=%CCACHE_SIZE% + - ccache --zero-stats - python build_msvc\msvc-autogen.py - msbuild build_msvc\bitcoin.sln -property:CLToolExe=%WRAPPED_CL%;UseMultiToolTask=true;Configuration=Release -maxCpuCount -verbosity:minimal -noLogo - ccache --show-stats @@ -257,22 +273,17 @@ task: task: name: '[ASan + LSan + UBSan + integer, no depends, USDT] [lunar]' + enable_bpfcc_script: + # In the image build step, no external environment variables are available, + # so any settings will need to be written to the settings env file: + - sed -i "s|\${CIRRUS_CI}|true|g" ./ci/test/00_setup_env_native_asan.sh << : *GLOBAL_TASK_TEMPLATE - # We can't use a 'container' for the USDT interface tests as the CirrusCI - # containers don't have privileges to hook into bitcoind. CirrusCI uses - # Google Compute Engine instances: https://cirrus-ci.org/guide/custom-vms/ - # Images can be found here: https://cloud.google.com/compute/docs/images/os-details - compute_engine_instance: - image_project: ubuntu-os-cloud - image: family/ubuntu-2304-amd64 # https://cirrus-ci.org/guide/custom-vms/#custom-compute-engine-vms - cpu: 4 - disk: 100 - memory: 12G + persistent_worker: + labels: + type: lunar # Must use the lunar-specific worker (needed for USDT functional tests) env: - << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV - HOME: /root/ # Only needed for compute_engine_instance + << : *PERSISTENT_WORKER_TEMPLATE_ENV FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" - MAKEJOBS: "-j4" # Avoid excessive memory use task: name: '[fuzzer,address,undefined,integer, no depends] [lunar]' diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 148819f468..8113500fb2 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -6,6 +6,8 @@ export LC_ALL=C +export PATH=$PWD/ci/retry:$PATH + ${CI_RETRY_EXE} apt-get update # Lint dependencies: # - curl/xz-utils (to install shellcheck) @@ -33,7 +35,7 @@ python3 --version ${CI_RETRY_EXE} pip3 install \ codespell==2.2.5 \ - flake8==6.0.0 \ + flake8==6.1.0 \ lief==0.13.2 \ mypy==1.4.1 \ pyzmq==25.1.0 \ diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index 69fd05051e..ba8fc861e8 100755 --- a/ci/test/00_setup_env.sh +++ b/ci/test/00_setup_env.sh @@ -15,9 +15,12 @@ export BASE_ROOT_DIR # The depends dir. # This folder exists only on the ci guest, and on the ci host as a volume. export DEPENDS_DIR=${DEPENDS_DIR:-$BASE_ROOT_DIR/depends} -# A folder for the ci system to put temporary files (ccache, datadirs for tests, ...) -# This folder only exists on the ci host. +# A folder for the ci system to put temporary files (build result, datadirs for tests, ...) +# This folder only exists on the ci guest. export BASE_SCRATCH_DIR=${BASE_SCRATCH_DIR:-$BASE_ROOT_DIR/ci/scratch} +# A folder for the ci system to put executables. +# This folder only exists on the ci guest. +export BINS_SCRATCH_DIR="${BASE_SCRATCH_DIR}/bins/" echo "Setting specific values in env" if [ -n "${FILE_ENV}" ]; then @@ -51,7 +54,7 @@ export RUN_FUZZ_TESTS=${RUN_FUZZ_TESTS:-false} export BOOST_TEST_RANDOM=${BOOST_TEST_RANDOM:-1} # See man 7 debconf export DEBIAN_FRONTEND=noninteractive -export CCACHE_SIZE=${CCACHE_SIZE:-100M} +export CCACHE_MAXSIZE=${CCACHE_MAXSIZE:-100M} export CCACHE_TEMPDIR=${CCACHE_TEMPDIR:-/tmp/.ccache-temp} export CCACHE_COMPRESS=${CCACHE_COMPRESS:-1} # The cache dir. @@ -64,10 +67,8 @@ export BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build} # The folder for previous release binaries. # This folder exists only on the ci guest, and on the ci host as a volume. export PREVIOUS_RELEASES_DIR=${PREVIOUS_RELEASES_DIR:-$BASE_ROOT_DIR/releases/$HOST} -export DIR_IWYU="${BASE_SCRATCH_DIR}/iwyu" export SDK_URL=${SDK_URL:-https://bitcoincore.org/depends-sources/sdks} export CI_BASE_PACKAGES=${CI_BASE_PACKAGES:-build-essential libtool autotools-dev automake pkg-config bsdmainutils curl ca-certificates ccache python3 rsync git procps bison} export GOAL=${GOAL:-install} export DIR_QA_ASSETS=${DIR_QA_ASSETS:-${BASE_SCRATCH_DIR}/qa-assets} -export PATH=${BASE_ROOT_DIR}/ci/retry:$PATH export CI_RETRY_EXE=${CI_RETRY_EXE:-"retry --"} diff --git a/ci/test/00_setup_env_mac_native_arm64.sh b/ci/test/00_setup_env_mac_native_arm64.sh index 09c05f3bbd..eee72db435 100755 --- a/ci/test/00_setup_env_mac_native_arm64.sh +++ b/ci/test/00_setup_env_mac_native_arm64.sh @@ -13,6 +13,6 @@ export BITCOIN_CONFIG="--with-gui --with-miniupnpc --with-natpmp --enable-reduce export CI_OS_NAME="macos" export NO_DEPENDS=1 export OSX_SDK="" -export CCACHE_SIZE=300M +export CCACHE_MAXSIZE=300M export RUN_FUZZ_TESTS=true export FUZZ_TESTS_CONFIG="--exclude banman" # https://github.com/bitcoin/bitcoin/issues/27924 diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index a5c80c2afc..dff86b1ffe 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -8,9 +8,11 @@ export LC_ALL=C.UTF-8 # Only install BCC tracing packages in Cirrus CI. if [[ "${CIRRUS_CI}" == "true" ]]; then - export BPFCC_PACKAGE="bpfcc-tools" + BPFCC_PACKAGE="bpfcc-tools linux-headers-$(uname --kernel-release)" + export CI_CONTAINER_CAP="--privileged -v /sys/kernel:/sys/kernel:rw" else - export BPFCC_PACKAGE="" + BPFCC_PACKAGE="" + export CI_CONTAINER_CAP="--cap-add SYS_PTRACE" # If run with (ASan + LSan), the container needs access to ptrace (https://github.com/google/sanitizers/issues/764) fi export CONTAINER_NAME=ci_native_asan diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index 481925dbc1..bfd51be6d0 100755 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -14,6 +14,7 @@ export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=true export GOAL="install" +export CI_CONTAINER_CAP="--cap-add SYS_PTRACE" # If run with (ASan + LSan), the container needs access to ptrace (https://github.com/google/sanitizers/issues/764) export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,address,undefined,float-divide-by-zero,integer \ CC='clang-16 -ftrivial-auto-var-init=pattern' CXX='clang++-16 -ftrivial-auto-var-init=pattern'" -export CCACHE_SIZE=200M +export CCACHE_MAXSIZE=200M diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 1f9adc0682..89006bf95e 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -22,4 +22,4 @@ export USE_MEMORY_SANITIZER="true" export RUN_UNIT_TESTS="false" export RUN_FUNCTIONAL_TESTS="false" export RUN_FUZZ_TESTS=true -export CCACHE_SIZE=250M +export CCACHE_MAXSIZE=250M diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh index abc3854762..30f512c38a 100755 --- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh +++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh @@ -17,4 +17,4 @@ export FUZZ_TESTS_CONFIG="--valgrind" export GOAL="install" # Temporarily pin dwarf 4, until using Valgrind 3.20 or later export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC='clang -gdwarf-4' CXX='clang++ -gdwarf-4'" -export CCACHE_SIZE=200M +export CCACHE_MAXSIZE=200M diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 34d60c6c4c..a96ed30dd3 100755 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -20,4 +20,4 @@ export GOAL="install" export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" -export CCACHE_SIZE=250M +export CCACHE_MAXSIZE=250M diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 920180a274..c86722c551 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -16,4 +16,4 @@ export RUN_FUZZ_TESTS=false export RUN_TIDY=true export GOAL="install" export BITCOIN_CONFIG="CC=clang-16 CXX=clang++-16 --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-16/lib/clang/16/include'" -export CCACHE_SIZE=200M +export CCACHE_MAXSIZE=200M diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 76cde42161..83213b2856 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -72,9 +72,9 @@ if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then fi if [[ "${RUN_TIDY}" == "true" ]]; then - git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_16 "${DIR_IWYU}"/include-what-you-use - cmake -B "${DIR_IWYU}"/build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-16 -S "${DIR_IWYU}"/include-what-you-use - make -C "${DIR_IWYU}"/build/ install "$MAKEJOBS" + git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_16 /include-what-you-use + cmake -B /iwyu-build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-16 -S /include-what-you-use + make -C /iwyu-build/ install "$MAKEJOBS" fi mkdir -p "${DEPENDS_DIR}/SDKs" "${DEPENDS_DIR}/sdk-sources" diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 626461df03..205c79328a 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -18,19 +18,15 @@ export ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order= export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan" export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1:log_path=${BASE_SCRATCH_DIR}/sanitizer-output/tsan" export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" -if [[ $BITCOIN_CONFIG = *--with-sanitizers=*address* ]]; then # If ran with (ASan + LSan), Docker needs access to ptrace (https://github.com/google/sanitizers/issues/764) - CI_CONTAINER_CAP="--cap-add SYS_PTRACE" -fi - -export P_CI_DIR="$PWD" -export BINS_SCRATCH_DIR="${BASE_SCRATCH_DIR}/bins/" if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then # Export all env vars to avoid missing some. # Though, exclude those with newlines to avoid parsing problems. - python3 -c 'import os; [print(f"{key}={value}") for key, value in os.environ.items() if "\n" not in value and "HOME" not in key]' | tee /tmp/env + python3 -c 'import os; [print(f"{key}={value}") for key, value in os.environ.items() if "\n" not in value and "HOME" != key and "PATH" != key and "USER" != key]' | tee /tmp/env + # System-dependent env vars must be kept as is. So read them from the container. + docker run --rm "${CI_IMAGE_NAME_TAG}" bash -c "env | grep --extended-regexp '^(HOME|PATH|USER)='" | tee --append /tmp/env echo "Creating $CI_IMAGE_NAME_TAG container to run in" - DOCKER_BUILDKIT=1 ${CI_RETRY_EXE} docker build \ + DOCKER_BUILDKIT=1 docker build \ --file "${BASE_ROOT_DIR}/ci/test_imagefile" \ --build-arg "CI_IMAGE_NAME_TAG=${CI_IMAGE_NAME_TAG}" \ --build-arg "FILE_ENV=${FILE_ENV}" \ @@ -53,7 +49,6 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then --mount "type=volume,src=${CONTAINER_NAME}_ccache,dst=$CCACHE_DIR" \ --mount "type=volume,src=${CONTAINER_NAME}_depends,dst=$DEPENDS_DIR" \ --mount "type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" \ - -w $BASE_ROOT_DIR \ --env-file /tmp/env \ --name $CONTAINER_NAME \ $CONTAINER_NAME) @@ -64,7 +59,7 @@ else fi CI_EXEC () { - $CI_EXEC_CMD_PREFIX bash -c "export PATH=${BINS_SCRATCH_DIR}:\$PATH && cd \"$P_CI_DIR\" && $*" + $CI_EXEC_CMD_PREFIX bash -c "export PATH=${BINS_SCRATCH_DIR}:${BASE_ROOT_DIR}/ci/retry:\$PATH && cd \"${BASE_ROOT_DIR}\" && $*" } export -f CI_EXEC diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index b298d35170..02358db789 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -75,7 +75,7 @@ if [ -z "$NO_WERROR" ]; then BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} --enable-werror" fi -ccache --zero-stats --max-size="${CCACHE_SIZE}" +ccache --zero-stats PRINT_CCACHE_STATISTICS="ccache --version | head -n 1 && ccache --show-stats" if [ -n "$ANDROID_TOOLS_URL" ]; then @@ -148,9 +148,13 @@ if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then fi if [ "${RUN_TIDY}" = "true" ]; then + cmake -B /tidy-build -DLLVM_DIR=/usr/lib/llvm-16/cmake -DCMAKE_BUILD_TYPE=Release -S "${BASE_ROOT_DIR}"/contrib/devtools/bitcoin-tidy + cmake --build /tidy-build "$MAKEJOBS" + cmake --build /tidy-build --target bitcoin-tidy-tests "$MAKEJOBS" + set -eo pipefail cd "${BASE_BUILD_DIR}/bitcoin-$HOST/src/" - ( run-clang-tidy-16 -quiet "${MAKEJOBS}" ) | grep -C5 "error" + ( run-clang-tidy-16 -quiet -load="/tidy-build/libbitcoin-tidy.so" "${MAKEJOBS}" ) | grep -C5 "error" # Filter out files by regex here, because regex may not be # accepted in src/.bear-tidy-config # Filter out: @@ -158,13 +162,13 @@ if [ "${RUN_TIDY}" = "true" ]; then jq 'map(select(.file | test("src/qt/qrc_.*\\.cpp$|/moc_.*\\.cpp$") | not))' ../compile_commands.json > tmp.json mv tmp.json ../compile_commands.json cd "${BASE_BUILD_DIR}/bitcoin-$HOST/" - python3 "${DIR_IWYU}/include-what-you-use/iwyu_tool.py" \ + python3 "/include-what-you-use/iwyu_tool.py" \ -p . "${MAKEJOBS}" \ -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp" \ -Xiwyu --max_line_length=160 \ 2>&1 | tee /tmp/iwyu_ci.out cd "${BASE_ROOT_DIR}/src" - python3 "${DIR_IWYU}/include-what-you-use/fix_includes.py" --nosafe_headers < /tmp/iwyu_ci.out + python3 "/include-what-you-use/fix_includes.py" --nosafe_headers < /tmp/iwyu_ci.out git --no-pager diff fi diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt new file mode 100644 index 0000000000..9ed18696d4 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt @@ -0,0 +1,45 @@ +cmake_minimum_required(VERSION 3.9) + +project(bitcoin-tidy VERSION 1.0.0 DESCRIPTION "clang-tidy checks for Bitcoin Core") + +include(GNUInstallDirs) + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED True) +set(CMAKE_CXX_EXTENSIONS False) + +# TODO: Figure out how to avoid the terminfo check +find_package(LLVM REQUIRED CONFIG) +find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy" HINTS ${LLVM_TOOLS_BINARY_DIR}) +message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") +message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}") + +add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp) +target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS}) + +# Disable RTTI and exceptions as necessary +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /GR-) +else() + target_compile_options(bitcoin-tidy PRIVATE -fno-rtti) + target_compile_options(bitcoin-tidy PRIVATE -fno-exceptions) +endif() + +# Add warnings +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /W4) +else() + target_compile_options(bitcoin-tidy PRIVATE -Wall) + target_compile_options(bitcoin-tidy PRIVATE -Wextra) +endif() + +set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=${CMAKE_BINARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}bitcoin-tidy${CMAKE_SHARED_LIBRARY_SUFFIX}" "-checks=-*,bitcoin-*") + +# Create a dummy library that runs clang-tidy tests as a side-effect of building +add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp) +add_dependencies(bitcoin-tidy-tests bitcoin-tidy) + +set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") + + +install(TARGETS bitcoin-tidy LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/contrib/devtools/bitcoin-tidy/README b/contrib/devtools/bitcoin-tidy/README new file mode 100644 index 0000000000..1669fe98f5 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/README @@ -0,0 +1,8 @@ +# Bitcoin Tidy + +Example Usage: + +```bash +cmake -S . -B build -DLLVM_DIR=/path/to/lib/cmake/llvm -DCMAKE_BUILD_TYPE=Release +make -C build -j$(nproc) +``` diff --git a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp new file mode 100644 index 0000000000..0f34d37793 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp @@ -0,0 +1,22 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "logprintf.h" + +#include <clang-tidy/ClangTidyModule.h> +#include <clang-tidy/ClangTidyModuleRegistry.h> + +class BitcoinModule final : public clang::tidy::ClangTidyModule +{ +public: + void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override + { + CheckFactories.registerCheck<bitcoin::LogPrintfCheck>("bitcoin-unterminated-logprintf"); + } +}; + +static clang::tidy::ClangTidyModuleRegistry::Add<BitcoinModule> + X("bitcoin-module", "Adds bitcoin checks."); + +volatile int BitcoinModuleAnchorSource = 0; diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp new file mode 100644 index 0000000000..a3d2768964 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp @@ -0,0 +1,91 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +// Warn about any use of LogPrintf that does not end with a newline. +#include <string> + +enum LogFlags { + NONE +}; + +enum Level { + None +}; + +template <typename... Args> +static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const LogFlags flag, const Level level, const char* fmt, const Args&... args) +{ +} + +#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) +#define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__) + +// Use a macro instead of a function for conditional logging to prevent +// evaluating arguments when logging for the category is not enabled. +#define LogPrint(category, ...) \ + do { \ + LogPrintf(__VA_ARGS__); \ + } while (0) + + +class CWallet +{ + std::string GetDisplayName() const + { + return "default wallet"; + } + +public: + template <typename... Params> + void WalletLogPrintf(std::string fmt, Params... parameters) const + { + LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); + }; +}; + +void good_func() +{ + LogPrintf("hello world!\n"); +} +void good_func2() +{ + CWallet wallet; + wallet.WalletLogPrintf("hi\n"); + + const CWallet& walletref = wallet; + walletref.WalletLogPrintf("hi\n"); + + auto* walletptr = new CWallet(); + walletptr->WalletLogPrintf("hi\n"); + delete walletptr; +} +void bad_func() +{ + LogPrintf("hello world!"); +} +void bad_func2() +{ + LogPrintf(""); +} +void bad_func3() +{ + // Ending in "..." has no special meaning. + LogPrintf("hello world!..."); +} +void bad_func4_ignored() +{ + LogPrintf("hello world!"); // NOLINT(bitcoin-unterminated-logprintf) +} +void bad_func5() +{ + CWallet wallet; + wallet.WalletLogPrintf("hi"); + + const CWallet& walletref = wallet; + walletref.WalletLogPrintf("hi"); + + auto* walletptr = new CWallet(); + walletptr->WalletLogPrintf("hi"); + delete walletptr; +} diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp new file mode 100644 index 0000000000..1690c8fde0 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.cpp @@ -0,0 +1,62 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "logprintf.h" + +#include <clang/AST/ASTContext.h> +#include <clang/ASTMatchers/ASTMatchFinder.h> + + +namespace { +AST_MATCHER(clang::StringLiteral, unterminated) +{ + size_t len = Node.getLength(); + if (len > 0 && Node.getCodeUnit(len - 1) == '\n') { + return false; + } + return true; +} +} // namespace + +namespace bitcoin { + +void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder) +{ + using namespace clang::ast_matchers; + + /* + Logprintf(..., ..., ..., ..., ..., "foo", ...) + */ + + finder->addMatcher( + callExpr( + callee(functionDecl(hasName("LogPrintf_"))), + hasArgument(5, stringLiteral(unterminated()).bind("logstring"))), + this); + + /* + CWallet wallet; + auto walletptr = &wallet; + wallet.WalletLogPrintf("foo"); + wallet->WalletLogPrintf("foo"); + */ + finder->addMatcher( + cxxMemberCallExpr( + thisPointerType(qualType(hasDeclaration(cxxRecordDecl(hasName("CWallet"))))), + callee(cxxMethodDecl(hasName("WalletLogPrintf"))), + hasArgument(0, stringLiteral(unterminated()).bind("logstring"))), + this); +} + +void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result) +{ + if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs<clang::StringLiteral>("logstring")) { + const clang::ASTContext& ctx = *Result.Context; + const auto user_diag = diag(lit->getEndLoc(), "Unterminated format string used with LogPrintf"); + const auto& loc = lit->getLocationOfByte(lit->getByteLength(), *Result.SourceManager, ctx.getLangOpts(), ctx.getTargetInfo()); + user_diag << clang::FixItHint::CreateInsertion(loc, "\\n"); + } +} + +} // namespace bitcoin diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h new file mode 100644 index 0000000000..466849ef01 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.h @@ -0,0 +1,28 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef LOGPRINTF_CHECK_H +#define LOGPRINTF_CHECK_H + +#include <clang-tidy/ClangTidyCheck.h> + +namespace bitcoin { + +class LogPrintfCheck final : public clang::tidy::ClangTidyCheck +{ +public: + LogPrintfCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context) + : clang::tidy::ClangTidyCheck(Name, Context) {} + + bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override + { + return LangOpts.CPlusPlus; + } + void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; + void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; +}; + +} // namespace bitcoin + +#endif // LOGPRINTF_CHECK_H diff --git a/doc/build-unix.md b/doc/build-unix.md index 848dd5f71a..bf367fc421 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -55,7 +55,7 @@ SQLite is required for the descriptor wallet: sudo apt install libsqlite3-dev Berkeley DB is only required for the legacy wallet. Ubuntu and Debian have their own `libdb-dev` and `libdb++-dev` packages, -but these will install Berkeley DB 5.1 or later. This will break binary wallet compatibility with the distributed +but these will install Berkeley DB 5.3 or later. This will break binary wallet compatibility with the distributed executables, which are based on BerkeleyDB 4.8. If you do not care about wallet compatibility, pass `--with-incompatible-bdb` to configure. Otherwise, you can build Berkeley DB [yourself](#berkeley-db). @@ -111,11 +111,7 @@ SQLite is required for the descriptor wallet: sudo dnf install sqlite-devel -Berkeley DB is required for the legacy wallet: - - sudo dnf install libdb4-devel libdb4-cxx-devel - -Berkeley DB is only required for the legacy wallet. Newer Fedora releases have only `libdb-devel` and `libdb-cxx-devel` packages, but these will install +Berkeley DB is only required for the legacy wallet. Fedora releases have only `libdb-devel` and `libdb-cxx-devel` packages, but these will install Berkeley DB 5.3 or later. This will break binary wallet compatibility with the distributed executables, which are based on Berkeley DB 4.8. If you do not care about wallet compatibility, pass `--with-incompatible-bdb` to configure. Otherwise, you can build Berkeley DB [yourself](#berkeley-db). diff --git a/doc/design/assumeutxo.md b/doc/design/assumeutxo.md index 469c551536..1492877e62 100644 --- a/doc/design/assumeutxo.md +++ b/doc/design/assumeutxo.md @@ -17,10 +17,9 @@ respectively generate and load UTXO snapshots. The utility script - A new block index `nStatus` flag is introduced, `BLOCK_ASSUMED_VALID`, to mark block index entries that are required to be assumed-valid by a chainstate created - from a UTXO snapshot. This flag is mostly used as a way to modify certain + from a UTXO snapshot. This flag is used as a way to modify certain CheckBlockIndex() logic to account for index entries that are pending validation by a - chainstate running asynchronously in the background. We also use this flag to control - which index entries are added to setBlockIndexCandidates during LoadBlockIndex(). + chainstate running asynchronously in the background. - The concept of UTXO snapshots is treated as an implementation detail that lives behind the ChainstateManager interface. The external presentation of the changes diff --git a/doc/release-process.md b/doc/release-process.md index 930110922c..c7a8998b8b 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -12,7 +12,7 @@ Release Process ### Before every major and minor release -* Update [bips.md](bips.md) to account for changes since the last release (don't forget to bump the version number on the first line). +* Update [bips.md](bips.md) to account for changes since the last release. * Update version in `configure.ac` (don't forget to set `CLIENT_VERSION_RC` to `0`). * Update manpages (see previous section) * Write release notes (see "Write the release notes" below). @@ -57,7 +57,7 @@ Release Process - Update the versions. - Create the draft, named "*version* Release Notes Draft", as a [collaborative wiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/_new). - Clear the release notes: `cp doc/release-notes-empty-template.md doc/release-notes.md` -- Create a pinned meta-issue for testing the release candidate (see [this issue](https://github.com/bitcoin/bitcoin/issues/17079) for an example) and provide a link to it in the release announcements where useful. +- Create a pinned meta-issue for testing the release candidate (see [this issue](https://github.com/bitcoin/bitcoin/issues/27621) for an example) and provide a link to it in the release announcements where useful. - Translations on Transifex - Change the auto-update URL for the new major version's resource away from `master` and to the branch, e.g. `https://raw.githubusercontent.com/bitcoin/bitcoin/<branch>/src/qt/locale/bitcoin_en.xlf`. Do not forget this or it will keep tracking the translations on master instead, drifting away from the specific major release. - Prune inputs from the qa-assets repo (See [pruning @@ -72,7 +72,7 @@ Release Process To tag the version (or release candidate) in git, use the `make-tag.py` script from [bitcoin-maintainer-tools](https://github.com/bitcoin-core/bitcoin-maintainer-tools). From the root of the repository run: - ../bitcoin-maintainer-tools/make-tag.py v(new version, e.g. 23.0) + ../bitcoin-maintainer-tools/make-tag.py v(new version, e.g. 25.0) This will perform a few last-minute consistency checks in the build system files, and if they pass, create a signed tag. @@ -96,11 +96,9 @@ Open a draft of the release notes for collaborative editing at https://github.co For the period during which the notes are being edited on the wiki, the version on the branch should be wiped and replaced with a link to the wiki which should be used for all announcements until `-final`. -Generate the change log. As this is a huge amount of work to do manually, there is the `list-pulls` script to do a pre-sorting step based on github PR metadata. See the [documentation in the README.md](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/README.md#list-pulls). - Generate list of authors: - git log --format='- %aN' v(current version, e.g. 24.0)..v(new version, e.g. 24.1) | sort -fiu + git log --format='- %aN' v(current version, e.g. 25.0)..v(new version, e.g. 25.1) | grep -v 'merge-script' | sort -fiu ### Setup and perform Guix builds @@ -109,7 +107,7 @@ Checkout the Bitcoin Core version you'd like to build: ```sh pushd ./bitcoin SIGNER='(your builder key, ie bluematt, sipa, etc)' -VERSION='(new version without v-prefix, e.g. 24.0)' +VERSION='(new version without v-prefix, e.g. 25.0)' git fetch origin "v${VERSION}" git checkout "v${VERSION}" popd @@ -144,10 +142,11 @@ Follow the relevant Guix README.md sections: pushd ./guix.sigs git add "${VERSION}/${SIGNER}"/noncodesigned.SHA256SUMS{,.asc} git commit -m "Add attestations by ${SIGNER} for ${VERSION} non-codesigned" -git push # Assuming you can push to the guix.sigs tree popd ``` +Then open a Pull Request to the [guix.sigs repository](https://github.com/bitcoin-core/guix.sigs). + ## Codesigning ### macOS codesigner only: Create detached macOS signatures (assuming [signapple](https://github.com/achow101/signapple/) is installed and up to date with master branch) @@ -202,10 +201,11 @@ popd pushd ./guix.sigs git add "${VERSION}/${SIGNER}"/all.SHA256SUMS{,.asc} git commit -m "Add attestations by ${SIGNER} for ${VERSION} codesigned" -git push # Assuming you can push to the guix.sigs tree popd ``` +Then open a Pull Request to the [guix.sigs repository](https://github.com/bitcoin-core/guix.sigs). + ## After 3 or more people have guix-built and their results match Combine the `all.SHA256SUMS.asc` file from all signers into `SHA256SUMS.asc`: @@ -268,17 +268,7 @@ cat "$VERSION"/*/all.SHA256SUMS.asc > SHA256SUMS.asc - bitcoincore.org RPC documentation update - - Install [golang](https://golang.org/doc/install) - - - Install the new Bitcoin Core release - - - Run bitcoind on regtest - - - Clone the [bitcoincore.org repository](https://github.com/bitcoin-core/bitcoincore.org) - - - Run: `go run generate.go` while being in `contrib/doc-gen` folder, and with bitcoin-cli in PATH - - - Add the generated files to git + - See https://github.com/bitcoin-core/bitcoincore.org/blob/master/contrib/doc-gen/ - Update packaging repo diff --git a/src/.clang-tidy b/src/.clang-tidy index 84c9d5fb3a..b4d50135dd 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,5 +1,6 @@ Checks: ' -*, +bitcoin-unterminated-logprintf, bugprone-argument-comment, bugprone-use-after-move, misc-unused-using-decls, diff --git a/src/Makefile.am b/src/Makefile.am index 1238bbec12..a108e60c86 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -717,6 +717,7 @@ libbitcoin_util_a_SOURCES = \ logging.cpp \ random.cpp \ randomenv.cpp \ + streams.cpp \ support/cleanse.cpp \ sync.cpp \ util/asmap.cpp \ @@ -959,6 +960,7 @@ libbitcoinkernel_la_SOURCES = \ script/sigcache.cpp \ script/standard.cpp \ signet.cpp \ + streams.cpp \ support/cleanse.cpp \ support/lockedpool.cpp \ sync.cpp \ diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 10c8389c80..51bfb1e459 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -52,7 +52,8 @@ bench_bench_bitcoin_SOURCES = \ bench/streams_findbyte.cpp \ bench/strencodings.cpp \ bench/util_time.cpp \ - bench/verify_script.cpp + bench/verify_script.cpp \ + bench/xor.cpp nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) diff --git a/src/bench/bench.h b/src/bench/bench.h index 78196134e7..6065ddf3fc 100644 --- a/src/bench/bench.h +++ b/src/bench/bench.h @@ -14,7 +14,7 @@ #include <string> #include <vector> -#include <bench/nanobench.h> +#include <bench/nanobench.h> // IWYU pragma: export /* * Usage: diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp index 1378a7b20a..252cbb163b 100644 --- a/src/bench/load_external.cpp +++ b/src/bench/load_external.cpp @@ -49,14 +49,13 @@ static void LoadExternalBlockFile(benchmark::Bench& bench) fclose(file); } - Chainstate& chainstate{testing_setup->m_node.chainman->ActiveChainstate()}; std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent; FlatFilePos pos; bench.run([&] { // "rb" is "binary, O_RDONLY", positioned to the start of the file. // The file will be closed by LoadExternalBlockFile(). FILE* file{fsbridge::fopen(blkfile, "rb")}; - chainstate.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); + testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); }); fs::remove(blkfile); } diff --git a/src/bench/xor.cpp b/src/bench/xor.cpp new file mode 100644 index 0000000000..edda74214a --- /dev/null +++ b/src/bench/xor.cpp @@ -0,0 +1,24 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit/. + +#include <bench/bench.h> + +#include <random.h> +#include <streams.h> + +#include <cstddef> +#include <vector> + +static void Xor(benchmark::Bench& bench) +{ + FastRandomContext frc{/*fDeterministic=*/true}; + auto data{frc.randbytes<std::byte>(1024)}; + auto key{frc.randbytes<std::byte>(31)}; + + bench.batch(data.size()).unit("byte").run([&] { + util::Xor(data, key); + }); +} + +BENCHMARK(Xor, benchmark::PriorityLevel::HIGH); diff --git a/src/chain.h b/src/chain.h index f5dd0fd315..2e1fb37bec 100644 --- a/src/chain.h +++ b/src/chain.h @@ -113,10 +113,10 @@ enum BlockStatus : uint32_t { BLOCK_VALID_TRANSACTIONS = 3, //! Outputs do not overspend inputs, no double spends, coinbase output ok, no immature coinbase spends, BIP30. - //! Implies all parents are also at least CHAIN. + //! Implies all parents are either at least VALID_CHAIN, or are ASSUMED_VALID BLOCK_VALID_CHAIN = 4, - //! Scripts & signatures ok. Implies all parents are also at least SCRIPTS. + //! Scripts & signatures ok. Implies all parents are either at least VALID_SCRIPTS, or are ASSUMED_VALID. BLOCK_VALID_SCRIPTS = 5, //! All validity bits. @@ -134,10 +134,18 @@ enum BlockStatus : uint32_t { BLOCK_OPT_WITNESS = 128, //!< block data in blk*.dat was received with a witness-enforcing client /** - * If set, this indicates that the block index entry is assumed-valid. - * Certain diagnostics will be skipped in e.g. CheckBlockIndex(). - * It almost certainly means that the block's full validation is pending - * on a background chainstate. See `doc/design/assumeutxo.md`. + * If ASSUMED_VALID is set, it means that this block has not been validated + * and has validity status less than VALID_SCRIPTS. Also that it may have + * descendant blocks with VALID_SCRIPTS set, because they can be validated + * based on an assumeutxo snapshot. + * + * When an assumeutxo snapshot is loaded, the ASSUMED_VALID flag is added to + * unvalidated blocks at the snapshot height and below. Then, as the background + * validation progresses, and these blocks are validated, the ASSUMED_VALID + * flags are removed. See `doc/design/assumeutxo.md` for details. + * + * This flag is only used to implement checks in CheckBlockIndex() and + * should not be used elsewhere. */ BLOCK_ASSUMED_VALID = 256, }; diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 847d40eefa..c95937ba75 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -101,7 +101,7 @@ public: assert(p <= limit); base[std::min(bufsize - 1, (int)(p - base))] = '\0'; - LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); /* Continued */ + LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); // NOLINT(bitcoin-unterminated-logprintf) if (base != buffer) { delete[] base; } diff --git a/src/hash.h b/src/hash.h index 2e3ed11b43..89c6f0dab9 100644 --- a/src/hash.h +++ b/src/hash.h @@ -160,7 +160,6 @@ public: template<typename T> CHashWriter& operator<<(const T& obj) { - // Serialize to this stream ::Serialize(*this, obj); return (*this); } @@ -228,7 +227,6 @@ public: template<typename T> CHashVerifier<Source>& operator>>(T&& obj) { - // Unserialize from this stream ::Unserialize(*this, obj); return (*this); } diff --git a/src/index/base.cpp b/src/index/base.cpp index 55fb154d99..f18205a76f 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -270,7 +270,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const // in the ValidationInterface queue backlog even after the sync thread has caught up to the // new chain tip. In this unlikely event, log a warning and let the queue clear. if (best_block_index->GetAncestor(pindex->nHeight - 1) != pindex->pprev) { - LogPrintf("%s: WARNING: Block %s does not connect to an ancestor of " /* Continued */ + LogPrintf("%s: WARNING: Block %s does not connect to an ancestor of " "known best chain (tip=%s); not updating index\n", __func__, pindex->GetBlockHash().ToString(), best_block_index->GetBlockHash().ToString()); @@ -322,7 +322,7 @@ void BaseIndex::ChainStateFlushed(const CBlockLocator& locator) // event, log a warning and let the queue clear. const CBlockIndex* best_block_index = m_best_block_index.load(); if (best_block_index->GetAncestor(locator_tip_index->nHeight) != locator_tip_index) { - LogPrintf("%s: WARNING: Locator contains block (hash=%s) not on known best " /* Continued */ + LogPrintf("%s: WARNING: Locator contains block (hash=%s) not on known best " "chain (tip=%s); not writing index locator\n", __func__, locator_tip_hash.ToString(), best_block_index->GetBlockHash().ToString()); diff --git a/src/init.cpp b/src/init.cpp index 685c135b74..33389e695c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -591,7 +591,11 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-acceptstalefeeestimates", strprintf("Read fee estimates even if they are stale (%sdefault: %u) fee estimates are considered stale if they are %s hours old", "regtest only; ", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES, Ticks<std::chrono::hours>(MAX_FILE_AGE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); - argsman.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-datacarriersize", + strprintf("Relay and mine transactions whose data-carrying raw scriptPubKey " + "is of this size or less (default: %u)", + MAX_OP_RETURN_RELAY), + ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-mempoolfullrbf", strprintf("Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); @@ -1079,7 +1083,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Warn about relative -datadir path. if (args.IsArgSet("-datadir") && !args.GetPathArg("-datadir").is_absolute()) { - LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " /* Continued */ + LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " "current working directory '%s'. This is fragile, because if bitcoin is started in the future " "from a different location, it will be unable to locate the current data files. There could " "also be data loss if bitcoin is started while in a temporary directory.\n", diff --git a/src/net.cpp b/src/net.cpp index 1eda51e9df..b51043ba27 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -83,6 +83,9 @@ static constexpr std::chrono::seconds MAX_UPLOAD_TIMEFRAME{60 * 60 * 24}; // A random time period (0 to 1 seconds) is added to feeler connections to prevent synchronization. static constexpr auto FEELER_SLEEP_WINDOW{1s}; +/** Frequency to attempt extra connections to reachable networks we're not connected to yet **/ +static constexpr auto EXTRA_NETWORK_PEER_INTERVAL{5min}; + /** Used to pass flags to the Bind() function */ enum BindFlags { BF_NONE = 0, @@ -1138,6 +1141,9 @@ void CConnman::DisconnectNodes() // close socket and cleanup pnode->CloseSocketDisconnect(); + // update connection count by network + if (pnode->IsManualOrFullOutboundConn()) --m_network_conn_counts[pnode->addr.GetNetwork()]; + // hold in disconnected pool until all refs are released pnode->Release(); m_nodes_disconnected.push_back(pnode); @@ -1605,6 +1611,28 @@ std::unordered_set<Network> CConnman::GetReachableEmptyNetworks() const return networks; } +bool CConnman::MultipleManualOrFullOutboundConns(Network net) const +{ + AssertLockHeld(m_nodes_mutex); + return m_network_conn_counts[net] > 1; +} + +bool CConnman::MaybePickPreferredNetwork(std::optional<Network>& network) +{ + std::array<Network, 5> nets{NET_IPV4, NET_IPV6, NET_ONION, NET_I2P, NET_CJDNS}; + Shuffle(nets.begin(), nets.end(), FastRandomContext()); + + LOCK(m_nodes_mutex); + for (const auto net : nets) { + if (IsReachable(net) && m_network_conn_counts[net] == 0 && addrman.Size(net) != 0) { + network = net; + return true; + } + } + + return false; +} + void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); @@ -1635,6 +1663,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // Minimum time before next feeler connection (in microseconds). auto next_feeler = GetExponentialRand(start, FEELER_INTERVAL); auto next_extra_block_relay = GetExponentialRand(start, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL); + auto next_extra_network_peer{GetExponentialRand(start, EXTRA_NETWORK_PEER_INTERVAL)}; const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED); bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS); const bool use_seednodes{gArgs.IsArgSet("-seednode")}; @@ -1747,6 +1776,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) auto now = GetTime<std::chrono::microseconds>(); bool anchor = false; bool fFeeler = false; + std::optional<Network> preferred_net; // Determine what type of connection to open. Opening // BLOCK_RELAY connections to addresses from anchors.dat gets the highest @@ -1796,6 +1826,17 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) next_feeler = GetExponentialRand(now, FEELER_INTERVAL); conn_type = ConnectionType::FEELER; fFeeler = true; + } else if (nOutboundFullRelay == m_max_outbound_full_relay && + m_max_outbound_full_relay == MAX_OUTBOUND_FULL_RELAY_CONNECTIONS && + now > next_extra_network_peer && + MaybePickPreferredNetwork(preferred_net)) { + // Full outbound connection management: Attempt to get at least one + // outbound peer from each reachable network by making extra connections + // and then protecting "only" peers from a network during outbound eviction. + // This is not attempted if the user changed -maxconnections to a value + // so low that less than MAX_OUTBOUND_FULL_RELAY_CONNECTIONS are made, + // to prevent interactions with otherwise protected outbound peers. + next_extra_network_peer = GetExponentialRand(now, EXTRA_NETWORK_PEER_INTERVAL); } else { // skip to next iteration of while loop continue; @@ -1849,7 +1890,10 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) } } else { // Not a feeler - std::tie(addr, addr_last_try) = addrman.Select(); + // If preferred_net has a value set, pick an extra outbound + // peer from that network. The eviction logic in net_processing + // ensures that a peer from another network will be evicted. + std::tie(addr, addr_last_try) = addrman.Select(false, preferred_net); } // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups @@ -1896,6 +1940,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) } LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToStringAddrPort()); } + + if (preferred_net != std::nullopt) LogPrint(BCLog::NET, "Making network specific connection to %s on %s.\n", addrConnect.ToStringAddrPort(), GetNetworkName(preferred_net.value())); + // Record addrman failure attempts when node has at least 2 persistent outbound connections to peers with // different netgroups in ipv4/ipv6 networks + all peers in Tor/I2P/CJDNS networks. // Don't record addrman failure attempts when node is offline. This can be identified since all local @@ -2035,6 +2082,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai { LOCK(m_nodes_mutex); m_nodes.push_back(pnode); + + // update connection count by network + if (pnode->IsManualOrFullOutboundConn()) ++m_network_conn_counts[pnode->addr.GetNetwork()]; } } @@ -465,6 +465,22 @@ public: return m_conn_type == ConnectionType::MANUAL; } + bool IsManualOrFullOutboundConn() const + { + switch (m_conn_type) { + case ConnectionType::INBOUND: + case ConnectionType::FEELER: + case ConnectionType::BLOCK_RELAY: + case ConnectionType::ADDR_FETCH: + return false; + case ConnectionType::OUTBOUND_FULL_RELAY: + case ConnectionType::MANUAL: + return true; + } // no default case, so the compiler can warn about missing cases + + assert(false); + } + bool IsBlockOnlyConn() const { return m_conn_type == ConnectionType::BLOCK_RELAY; } @@ -777,6 +793,9 @@ public: void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); bool CheckIncomingNonce(uint64_t nonce); + // alias for thread safety annotations only, not defined + RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); + bool ForNode(NodeId id, std::function<bool(CNode* pnode)> func); void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex); @@ -893,6 +912,8 @@ public: /** Return true if we should disconnect the peer for failing an inactivity check. */ bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; + bool MultipleManualOrFullOutboundConns(Network net) const EXCLUSIVE_LOCKS_REQUIRED(m_nodes_mutex); + private: struct ListenSocket { public: @@ -1010,6 +1031,18 @@ private: */ std::vector<CAddress> GetCurrentBlockRelayOnlyConns() const; + /** + * Search for a "preferred" network, a reachable network to which we + * currently don't have any OUTBOUND_FULL_RELAY or MANUAL connections. + * There needs to be at least one address in AddrMan for a preferred + * network to be picked. + * + * @param[out] network Preferred network, if found. + * + * @return bool Whether a preferred network was found. + */ + bool MaybePickPreferredNetwork(std::optional<Network>& network); + // Whether the node should be passed out in ForEach* callbacks static bool NodeFullyConnected(const CNode* pnode); @@ -1048,6 +1081,9 @@ private: std::atomic<NodeId> nLastNodeId{0}; unsigned int nPrevNodeCount{0}; + // Stores number of full-tx connections (outbound and manual) per network + std::array<unsigned int, Network::NET_MAX> m_network_conn_counts GUARDED_BY(m_nodes_mutex) = {}; + /** * Cache responses to addr requests to minimize privacy leak. * Attack example: scraping addrs in real-time may allow an attacker diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 11be981cd9..e2bbfe3308 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2213,7 +2213,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // Fast-path: in this case it is possible to serve the block directly from disk, // as the network format matches the format on disk std::vector<uint8_t> block_data; - if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos(), m_chainparams.MessageStart())) { + if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos())) { assert(!"cannot load block from disk"); } m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, Span{block_data})); @@ -3049,7 +3049,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, uint32_t stop_height = stop_index->nHeight; if (start_height > stop_height) { - LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ + LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " "start height %d and stop height %d\n", node.GetId(), start_height, stop_height); node.fDisconnect = true; @@ -5143,10 +5143,12 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) // Pick the outbound-full-relay peer that least recently announced // us a new block, with ties broken by choosing the more recent // connection (higher node id) + // Protect peers from eviction if we don't have another connection + // to their network, counting both outbound-full-relay and manual peers. NodeId worst_peer = -1; int64_t oldest_block_announcement = std::numeric_limits<int64_t>::max(); - m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_connman.GetNodesMutex()) { AssertLockHeld(::cs_main); // Only consider outbound-full-relay peers that are not already @@ -5156,6 +5158,9 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers if (state->m_chain_sync.m_protect) return; + // If this is the only connection on a particular network that is + // OUTBOUND_FULL_RELAY or MANUAL, protect it. + if (!m_connman.MultipleManualOrFullOutboundConns(pnode->addr.GetNetwork())) return; if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { worst_peer = pnode->GetId(); oldest_block_announcement = state->m_last_block_announcement; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 78416ec576..01b4c36a8f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -476,7 +476,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n) return &m_blockfile_info.at(n); } -bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) const +bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const { // Open history file to append AutoFile fileout{OpenUndoFile(pos)}; @@ -486,7 +486,7 @@ bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos // Write index header unsigned int nSize = GetSerializeSize(blockundo, CLIENT_VERSION); - fileout << messageStart << nSize; + fileout << GetParams().MessageStart() << nSize; // Write undo data long fileOutPos = ftell(fileout.Get()); @@ -618,7 +618,7 @@ fs::path BlockManager::GetBlockPosFilename(const FlatFilePos& pos) const return BlockFileSeq().FileName(pos); } -bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, CChain& active_chain, uint64_t nTime, bool fKnown) +bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown) { LOCK(cs_LastBlockFile); @@ -644,7 +644,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne // when the undo file is keeping up with the block file, we want to flush it explicitly // when it is lagging behind (more blocks arrive than are being connected), we let the // undo block write case handle it - finalize_undo = (m_blockfile_info[nFile].nHeightLast == (unsigned int)active_chain.Tip()->nHeight); + finalize_undo = (m_blockfile_info[nFile].nHeightLast == m_undo_height_in_last_blockfile); nFile++; if (m_blockfile_info.size() <= nFile) { m_blockfile_info.resize(nFile + 1); @@ -660,6 +660,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne } FlushBlockFile(!fKnown, finalize_undo); m_last_blockfile = nFile; + m_undo_height_in_last_blockfile = 0; // No undo data yet in the new file, so reset our undo-height tracking. } m_blockfile_info[nFile].AddBlock(nHeight, nTime); @@ -707,7 +708,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP return true; } -bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const CMessageHeader::MessageStartChars& messageStart) const +bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const { // Open history file to append CAutoFile fileout(OpenBlockFile(pos), SER_DISK, CLIENT_VERSION); @@ -717,7 +718,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const // Write index header unsigned int nSize = GetSerializeSize(block, fileout.GetVersion()); - fileout << messageStart << nSize; + fileout << GetParams().MessageStart() << nSize; // Write block long fileOutPos = ftell(fileout.Get()); @@ -739,7 +740,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo, CLIENT_VERSION) + 40)) { return error("ConnectBlock(): FindUndoPos failed"); } - if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash(), GetParams().MessageStart())) { + if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) { return FatalError(m_opts.notifications, state, "Failed to write undo data"); } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) @@ -749,8 +750,9 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid // the FindBlockPos function if (_pos.nFile < m_last_blockfile && static_cast<uint32_t>(block.nHeight) == m_blockfile_info[_pos.nFile].nHeightLast) { FlushUndoFile(_pos.nFile, true); + } else if (_pos.nFile == m_last_blockfile && static_cast<uint32_t>(block.nHeight) > m_undo_height_in_last_blockfile) { + m_undo_height_in_last_blockfile = block.nHeight; } - // update nUndoPos in block index block.nUndoPos = _pos.nPos; block.nStatus |= BLOCK_HAVE_UNDO; @@ -804,7 +806,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co return true; } -bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start) const +bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos) const { FlatFilePos hpos = pos; hpos.nPos -= 8; // Seek back 8 bytes for meta header @@ -819,10 +821,10 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF filein >> blk_start >> blk_size; - if (memcmp(blk_start, message_start, CMessageHeader::MESSAGE_START_SIZE)) { + if (memcmp(blk_start, GetParams().MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) { return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(), HexStr(blk_start), - HexStr(message_start)); + HexStr(GetParams().MessageStart())); } if (blk_size > MAX_SIZE) { @@ -839,7 +841,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF return true; } -FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CChain& active_chain, const FlatFilePos* dbp) +FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, const FlatFilePos* dbp) { unsigned int nBlockSize = ::GetSerializeSize(block, CLIENT_VERSION); FlatFilePos blockPos; @@ -852,12 +854,12 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CCha // we add BLOCK_SERIALIZATION_HEADER_SIZE only for new blocks since they will have the serialization header added when written to disk. nBlockSize += static_cast<unsigned int>(BLOCK_SERIALIZATION_HEADER_SIZE); } - if (!FindBlockPos(blockPos, nBlockSize, nHeight, active_chain, block.GetBlockTime(), position_known)) { + if (!FindBlockPos(blockPos, nBlockSize, nHeight, block.GetBlockTime(), position_known)) { error("%s: FindBlockPos failed", __func__); return FlatFilePos(); } if (!position_known) { - if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) { + if (!WriteBlockToDisk(block, blockPos)) { m_opts.notifications.fatalError("Failed to write block"); return FlatFilePos(); } @@ -905,7 +907,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile break; // This error is logged in OpenBlockFile } LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile); - chainman.ActiveChainstate().LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); + chainman.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); if (chainman.m_interrupt) { LogPrintf("Interrupt requested. Exit %s\n", __func__); return; @@ -924,7 +926,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile FILE* file = fsbridge::fopen(path, "rb"); if (file) { LogPrintf("Importing blocks file %s...\n", fs::PathToString(path)); - chainman.ActiveChainstate().LoadExternalBlockFile(file); + chainman.LoadExternalBlockFile(file); if (chainman.m_interrupt) { LogPrintf("Interrupt requested. Exit %s\n", __func__); return; diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index c2e903e470..0180124a79 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -24,7 +24,6 @@ class BlockValidationState; class CBlock; class CBlockFileInfo; class CBlockUndo; -class CChain; class CChainParams; class Chainstate; class ChainstateManager; @@ -94,7 +93,7 @@ private: EXCLUSIVE_LOCKS_REQUIRED(cs_main); void FlushBlockFile(bool fFinalize = false, bool finalize_undo = false); void FlushUndoFile(int block_file, bool finalize = false); - bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, CChain& active_chain, uint64_t nTime, bool fKnown); + bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown); bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize); FlatFileSeq BlockFileSeq() const; @@ -102,8 +101,8 @@ private: FILE* OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const; - bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const CMessageHeader::MessageStartChars& messageStart) const; - bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) const; + bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const; + bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const; /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ void FindFilesToPruneManual(std::set<int>& setFilesToPrune, int nManualPruneHeight, int chain_tip_height); @@ -128,6 +127,19 @@ private: RecursiveMutex cs_LastBlockFile; std::vector<CBlockFileInfo> m_blockfile_info; int m_last_blockfile = 0; + + // Track the height of the highest block in m_last_blockfile whose undo + // data has been written. Block data is written to block files in download + // order, but is written to undo files in validation order, which is + // usually in order by height. To avoid wasting disk space, undo files will + // be trimmed whenever the corresponding block file is finalized and + // the height of the highest block written to the block file equals the + // height of the highest block written to the undo file. This is a + // heuristic and can sometimes preemptively trim undo files that will write + // more data later, and sometimes fail to trim undo files that can't have + // more data written later. + unsigned int m_undo_height_in_last_blockfile = 0; + /** Global flag to indicate we should check to see if there are * block/undo files that should be deleted. Set on startup * or if we allocate more file space when we're in prune mode @@ -202,7 +214,7 @@ public: EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Store block on disk. If dbp is not nullptr, then it provides the known position of the block within a block file on disk. */ - FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight, CChain& active_chain, const FlatFilePos* dbp); + FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight, const FlatFilePos* dbp); /** Whether running in -prune mode. */ [[nodiscard]] bool IsPruneMode() const { return m_prune_mode; } @@ -252,7 +264,7 @@ public: /** Functions for disk access for blocks */ bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) const; bool ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) const; - bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start) const; + bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos) const; bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& index) const; diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 255d8be0ec..0828f64856 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -221,7 +221,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // A reload of the block index is required to recompute setBlockIndexCandidates // for the fully validated chainstate. - chainman.ActiveChainstate().UnloadBlockIndex(); + chainman.ActiveChainstate().ClearBlockIndexCandidates(); auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options); if (init_status != ChainstateLoadStatus::SUCCESS) { diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp index 036a25d0a5..976421e455 100644 --- a/src/node/utxo_snapshot.cpp +++ b/src/node/utxo_snapshot.cpp @@ -49,7 +49,7 @@ bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir) { if (!fs::exists(chaindir)) { - LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " /* Continued */ + LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " "exists at path %s\n", fs::PathToString(chaindir)); return std::nullopt; } @@ -57,7 +57,7 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir) const std::string read_from_str = fs::PathToString(read_from); if (!fs::exists(read_from)) { - LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " /* Continued */ + LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " "exists at path %s. Try deleting %s and calling loadtxoutset again?\n", fs::PathToString(chaindir), read_from_str); return std::nullopt; diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index 80cc458377..6b3662996c 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -58,11 +58,9 @@ static RPCHelpMan setmocktime() throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Mocktime cannot be negative: %s.", time)); } SetMockTime(time); - auto node_context = util::AnyPtr<NodeContext>(request.context); - if (node_context) { - for (const auto& chain_client : node_context->chain_clients) { - chain_client->setMockTime(time); - } + const NodeContext& node_context{EnsureAnyNodeContext(request.context)}; + for (const auto& chain_client : node_context.chain_clients) { + chain_client->setMockTime(time); } return UniValue::VNULL; @@ -90,10 +88,9 @@ static RPCHelpMan mockscheduler() throw std::runtime_error("delta_time must be between 1 and 3600 seconds (1 hr)"); } - auto node_context = CHECK_NONFATAL(util::AnyPtr<NodeContext>(request.context)); - // protect against null pointer dereference - CHECK_NONFATAL(node_context->scheduler); - node_context->scheduler->MockForward(std::chrono::seconds(delta_seconds)); + const NodeContext& node_context{EnsureAnyNodeContext(request.context)}; + CHECK_NONFATAL(node_context.scheduler)->MockForward(std::chrono::seconds{delta_seconds}); + SyncWithValidationInterfaceQueue(); return UniValue::VNULL; }, diff --git a/src/serialize.h b/src/serialize.h index 0cda0ac7d5..39f2c0f3ae 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -642,23 +642,14 @@ template<typename Stream, typename C> void Unserialize(Stream& is, std::basic_st * prevector * prevectors of unsigned char are a special case and are intended to be serialized as a single opaque blob. */ -template<typename Stream, unsigned int N, typename T> void Serialize_impl(Stream& os, const prevector<N, T>& v, const unsigned char&); -template<typename Stream, unsigned int N, typename T, typename V> void Serialize_impl(Stream& os, const prevector<N, T>& v, const V&); template<typename Stream, unsigned int N, typename T> inline void Serialize(Stream& os, const prevector<N, T>& v); -template<typename Stream, unsigned int N, typename T> void Unserialize_impl(Stream& is, prevector<N, T>& v, const unsigned char&); -template<typename Stream, unsigned int N, typename T, typename V> void Unserialize_impl(Stream& is, prevector<N, T>& v, const V&); template<typename Stream, unsigned int N, typename T> inline void Unserialize(Stream& is, prevector<N, T>& v); /** * vector * vectors of unsigned char are a special case and are intended to be serialized as a single opaque blob. */ -template<typename Stream, typename T, typename A> void Serialize_impl(Stream& os, const std::vector<T, A>& v, const unsigned char&); -template<typename Stream, typename T, typename A> void Serialize_impl(Stream& os, const std::vector<T, A>& v, const bool&); -template<typename Stream, typename T, typename A, typename V> void Serialize_impl(Stream& os, const std::vector<T, A>& v, const V&); template<typename Stream, typename T, typename A> inline void Serialize(Stream& os, const std::vector<T, A>& v); -template<typename Stream, typename T, typename A> void Unserialize_impl(Stream& is, std::vector<T, A>& v, const unsigned char&); -template<typename Stream, typename T, typename A, typename V> void Unserialize_impl(Stream& is, std::vector<T, A>& v, const V&); template<typename Stream, typename T, typename A> inline void Unserialize(Stream& is, std::vector<T, A>& v); /** @@ -751,122 +742,82 @@ void Unserialize(Stream& is, std::basic_string<C>& str) /** * prevector */ -template<typename Stream, unsigned int N, typename T> -void Serialize_impl(Stream& os, const prevector<N, T>& v, const unsigned char&) -{ - WriteCompactSize(os, v.size()); - if (!v.empty()) - os.write(MakeByteSpan(v)); -} - -template<typename Stream, unsigned int N, typename T, typename V> -void Serialize_impl(Stream& os, const prevector<N, T>& v, const V&) -{ - Serialize(os, Using<VectorFormatter<DefaultFormatter>>(v)); -} - -template<typename Stream, unsigned int N, typename T> -inline void Serialize(Stream& os, const prevector<N, T>& v) -{ - Serialize_impl(os, v, T()); -} - - -template<typename Stream, unsigned int N, typename T> -void Unserialize_impl(Stream& is, prevector<N, T>& v, const unsigned char&) -{ - // Limit size per read so bogus size value won't cause out of memory - v.clear(); - unsigned int nSize = ReadCompactSize(is); - unsigned int i = 0; - while (i < nSize) - { - unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); - v.resize_uninitialized(i + blk); - is.read(AsWritableBytes(Span{&v[i], blk})); - i += blk; +template <typename Stream, unsigned int N, typename T> +void Serialize(Stream& os, const prevector<N, T>& v) +{ + if constexpr (std::is_same_v<T, unsigned char>) { + WriteCompactSize(os, v.size()); + if (!v.empty()) + os.write(MakeByteSpan(v)); + } else { + Serialize(os, Using<VectorFormatter<DefaultFormatter>>(v)); } } -template<typename Stream, unsigned int N, typename T, typename V> -void Unserialize_impl(Stream& is, prevector<N, T>& v, const V&) -{ - Unserialize(is, Using<VectorFormatter<DefaultFormatter>>(v)); -} -template<typename Stream, unsigned int N, typename T> -inline void Unserialize(Stream& is, prevector<N, T>& v) +template <typename Stream, unsigned int N, typename T> +void Unserialize(Stream& is, prevector<N, T>& v) { - Unserialize_impl(is, v, T()); + if constexpr (std::is_same_v<T, unsigned char>) { + // Limit size per read so bogus size value won't cause out of memory + v.clear(); + unsigned int nSize = ReadCompactSize(is); + unsigned int i = 0; + while (i < nSize) { + unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); + v.resize_uninitialized(i + blk); + is.read(AsWritableBytes(Span{&v[i], blk})); + i += blk; + } + } else { + Unserialize(is, Using<VectorFormatter<DefaultFormatter>>(v)); + } } - /** * vector */ -template<typename Stream, typename T, typename A> -void Serialize_impl(Stream& os, const std::vector<T, A>& v, const unsigned char&) -{ - WriteCompactSize(os, v.size()); - if (!v.empty()) - os.write(MakeByteSpan(v)); -} - -template<typename Stream, typename T, typename A> -void Serialize_impl(Stream& os, const std::vector<T, A>& v, const bool&) -{ - // A special case for std::vector<bool>, as dereferencing - // std::vector<bool>::const_iterator does not result in a const bool& - // due to std::vector's special casing for bool arguments. - WriteCompactSize(os, v.size()); - for (bool elem : v) { - ::Serialize(os, elem); +template <typename Stream, typename T, typename A> +void Serialize(Stream& os, const std::vector<T, A>& v) +{ + if constexpr (std::is_same_v<T, unsigned char>) { + WriteCompactSize(os, v.size()); + if (!v.empty()) + os.write(MakeByteSpan(v)); + } else if constexpr (std::is_same_v<T, bool>) { + // A special case for std::vector<bool>, as dereferencing + // std::vector<bool>::const_iterator does not result in a const bool& + // due to std::vector's special casing for bool arguments. + WriteCompactSize(os, v.size()); + for (bool elem : v) { + ::Serialize(os, elem); + } + } else { + Serialize(os, Using<VectorFormatter<DefaultFormatter>>(v)); } } -template<typename Stream, typename T, typename A, typename V> -void Serialize_impl(Stream& os, const std::vector<T, A>& v, const V&) -{ - Serialize(os, Using<VectorFormatter<DefaultFormatter>>(v)); -} -template<typename Stream, typename T, typename A> -inline void Serialize(Stream& os, const std::vector<T, A>& v) +template <typename Stream, typename T, typename A> +void Unserialize(Stream& is, std::vector<T, A>& v) { - Serialize_impl(os, v, T()); -} - - -template<typename Stream, typename T, typename A> -void Unserialize_impl(Stream& is, std::vector<T, A>& v, const unsigned char&) -{ - // Limit size per read so bogus size value won't cause out of memory - v.clear(); - unsigned int nSize = ReadCompactSize(is); - unsigned int i = 0; - while (i < nSize) - { - unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); - v.resize(i + blk); - is.read(AsWritableBytes(Span{&v[i], blk})); - i += blk; + if constexpr (std::is_same_v<T, unsigned char>) { + // Limit size per read so bogus size value won't cause out of memory + v.clear(); + unsigned int nSize = ReadCompactSize(is); + unsigned int i = 0; + while (i < nSize) { + unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); + v.resize(i + blk); + is.read(AsWritableBytes(Span{&v[i], blk})); + i += blk; + } + } else { + Unserialize(is, Using<VectorFormatter<DefaultFormatter>>(v)); } } -template<typename Stream, typename T, typename A, typename V> -void Unserialize_impl(Stream& is, std::vector<T, A>& v, const V&) -{ - Unserialize(is, Using<VectorFormatter<DefaultFormatter>>(v)); -} - -template<typename Stream, typename T, typename A> -inline void Unserialize(Stream& is, std::vector<T, A>& v) -{ - Unserialize_impl(is, v, T()); -} - - /** * pair @@ -1039,28 +990,16 @@ public: int GetVersion() const { return nVersion; } }; -template<typename Stream> -void SerializeMany(Stream& s) -{ -} - -template<typename Stream, typename Arg, typename... Args> -void SerializeMany(Stream& s, const Arg& arg, const Args&... args) -{ - ::Serialize(s, arg); - ::SerializeMany(s, args...); -} - -template<typename Stream> -inline void UnserializeMany(Stream& s) +template <typename Stream, typename... Args> +void SerializeMany(Stream& s, const Args&... args) { + (::Serialize(s, args), ...); } -template<typename Stream, typename Arg, typename... Args> -inline void UnserializeMany(Stream& s, Arg&& arg, Args&&... args) +template <typename Stream, typename... Args> +inline void UnserializeMany(Stream& s, Args&&... args) { - ::Unserialize(s, arg); - ::UnserializeMany(s, args...); + (::Unserialize(s, args), ...); } template<typename Stream, typename... Args> diff --git a/src/streams.cpp b/src/streams.cpp new file mode 100644 index 0000000000..6921dad677 --- /dev/null +++ b/src/streams.cpp @@ -0,0 +1,66 @@ +// Copyright (c) 2009-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit/. + +#include <span.h> +#include <streams.h> + +#include <array> + +std::size_t AutoFile::detail_fread(Span<std::byte> dst) +{ + if (!m_file) throw std::ios_base::failure("AutoFile::read: file handle is nullptr"); + if (m_xor.empty()) { + return std::fread(dst.data(), 1, dst.size(), m_file); + } else { + const auto init_pos{std::ftell(m_file)}; + if (init_pos < 0) throw std::ios_base::failure("AutoFile::read: ftell failed"); + std::size_t ret{std::fread(dst.data(), 1, dst.size(), m_file)}; + util::Xor(dst.subspan(0, ret), m_xor, init_pos); + return ret; + } +} + +void AutoFile::read(Span<std::byte> dst) +{ + if (detail_fread(dst) != dst.size()) { + throw std::ios_base::failure(feof() ? "AutoFile::read: end of file" : "AutoFile::read: fread failed"); + } +} + +void AutoFile::ignore(size_t nSize) +{ + if (!m_file) throw std::ios_base::failure("AutoFile::ignore: file handle is nullptr"); + unsigned char data[4096]; + while (nSize > 0) { + size_t nNow = std::min<size_t>(nSize, sizeof(data)); + if (std::fread(data, 1, nNow, m_file) != nNow) { + throw std::ios_base::failure(feof() ? "AutoFile::ignore: end of file" : "AutoFile::ignore: fread failed"); + } + nSize -= nNow; + } +} + +void AutoFile::write(Span<const std::byte> src) +{ + if (!m_file) throw std::ios_base::failure("AutoFile::write: file handle is nullptr"); + if (m_xor.empty()) { + if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { + throw std::ios_base::failure("AutoFile::write: write failed"); + } + } else { + auto current_pos{std::ftell(m_file)}; + if (current_pos < 0) throw std::ios_base::failure("AutoFile::write: ftell failed"); + std::array<std::byte, 4096> buf; + while (src.size() > 0) { + auto buf_now{Span{buf}.first(std::min<size_t>(src.size(), buf.size()))}; + std::copy(src.begin(), src.begin() + buf_now.size(), buf_now.begin()); + util::Xor(buf_now, m_xor, current_pos); + if (std::fwrite(buf_now.data(), 1, buf_now.size(), m_file) != buf_now.size()) { + throw std::ios_base::failure{"XorFile::write: failed"}; + } + src = src.subspan(buf_now.size()); + current_pos += buf_now.size(); + } + } +} diff --git a/src/streams.h b/src/streams.h index 03df20b5db..5ff952be76 100644 --- a/src/streams.h +++ b/src/streams.h @@ -13,6 +13,7 @@ #include <algorithm> #include <assert.h> +#include <cstddef> #include <cstdio> #include <ios> #include <limits> @@ -23,6 +24,27 @@ #include <utility> #include <vector> +namespace util { +inline void Xor(Span<std::byte> write, Span<const std::byte> key, size_t key_offset = 0) +{ + if (key.size() == 0) { + return; + } + key_offset %= key.size(); + + for (size_t i = 0, j = key_offset; i != write.size(); i++) { + write[i] ^= key[j++]; + + // This potentially acts on very many bytes of data, so it's + // important that we calculate `j`, i.e. the `key` index in this + // way instead of doing a %, which would effectively be a division + // for each byte Xor'd -- much slower than need be. + if (j == key.size()) + j = 0; + } +} +} // namespace util + template<typename Stream> class OverrideStream { @@ -37,7 +59,6 @@ public: template<typename T> OverrideStream<Stream>& operator<<(const T& obj) { - // Serialize to this stream ::Serialize(*this, obj); return (*this); } @@ -45,7 +66,6 @@ public: template<typename T> OverrideStream<Stream>& operator>>(T&& obj) { - // Unserialize from this stream ::Unserialize(*this, obj); return (*this); } @@ -110,7 +130,6 @@ class CVectorWriter template<typename T> CVectorWriter& operator<<(const T& obj) { - // Serialize to this stream ::Serialize(*this, obj); return (*this); } @@ -151,7 +170,6 @@ public: template<typename T> SpanReader& operator>>(T&& obj) { - // Unserialize from this stream ::Unserialize(*this, obj); return (*this); } @@ -296,7 +314,6 @@ public: template<typename T> DataStream& operator<<(const T& obj) { - // Serialize to this stream ::Serialize(*this, obj); return (*this); } @@ -304,7 +321,6 @@ public: template<typename T> DataStream& operator>>(T&& obj) { - // Unserialize from this stream ::Unserialize(*this, obj); return (*this); } @@ -316,20 +332,7 @@ public: */ void Xor(const std::vector<unsigned char>& key) { - if (key.size() == 0) { - return; - } - - for (size_type i = 0, j = 0; i != size(); i++) { - vch[i] ^= std::byte{key[j++]}; - - // This potentially acts on very many bytes of data, so it's - // important that we calculate `j`, i.e. the `key` index in this - // way instead of doing a %, which would effectively be a division - // for each byte Xor'd -- much slower than need be. - if (j == key.size()) - j = 0; - } + util::Xor(MakeWritableByteSpan(*this), MakeByteSpan(key)); } }; @@ -469,7 +472,6 @@ public: } }; - /** Non-refcounted RAII wrapper for FILE* * * Will automatically close the file when it goes out of scope if not null. @@ -479,81 +481,60 @@ public: class AutoFile { protected: - FILE* file; + std::FILE* m_file; + const std::vector<std::byte> m_xor; public: - explicit AutoFile(FILE* filenew) : file{filenew} {} + explicit AutoFile(std::FILE* file, std::vector<std::byte> data_xor={}) : m_file{file}, m_xor{std::move(data_xor)} {} - ~AutoFile() - { - fclose(); - } + ~AutoFile() { fclose(); } // Disallow copies AutoFile(const AutoFile&) = delete; AutoFile& operator=(const AutoFile&) = delete; + bool feof() const { return std::feof(m_file); } + int fclose() { - int retval{0}; - if (file) { - retval = ::fclose(file); - file = nullptr; - } - return retval; + if (auto rel{release()}) return std::fclose(rel); + return 0; } /** Get wrapped FILE* with transfer of ownership. * @note This will invalidate the AutoFile object, and makes it the responsibility of the caller * of this function to clean up the returned FILE*. */ - FILE* release() { FILE* ret = file; file = nullptr; return ret; } + std::FILE* release() + { + std::FILE* ret{m_file}; + m_file = nullptr; + return ret; + } /** Get wrapped FILE* without transfer of ownership. * @note Ownership of the FILE* will remain with this class. Use this only if the scope of the * AutoFile outlives use of the passed pointer. */ - FILE* Get() const { return file; } + std::FILE* Get() const { return m_file; } /** Return true if the wrapped FILE* is nullptr, false otherwise. */ - bool IsNull() const { return (file == nullptr); } + bool IsNull() const { return m_file == nullptr; } + + /** Implementation detail, only used internally. */ + std::size_t detail_fread(Span<std::byte> dst); // // Stream subset // - void read(Span<std::byte> dst) - { - if (!file) throw std::ios_base::failure("AutoFile::read: file handle is nullptr"); - if (fread(dst.data(), 1, dst.size(), file) != dst.size()) { - throw std::ios_base::failure(feof(file) ? "AutoFile::read: end of file" : "AutoFile::read: fread failed"); - } - } - - void ignore(size_t nSize) - { - if (!file) throw std::ios_base::failure("AutoFile::ignore: file handle is nullptr"); - unsigned char data[4096]; - while (nSize > 0) { - size_t nNow = std::min<size_t>(nSize, sizeof(data)); - if (fread(data, 1, nNow, file) != nNow) - throw std::ios_base::failure(feof(file) ? "AutoFile::ignore: end of file" : "AutoFile::read: fread failed"); - nSize -= nNow; - } - } - - void write(Span<const std::byte> src) - { - if (!file) throw std::ios_base::failure("AutoFile::write: file handle is nullptr"); - if (fwrite(src.data(), 1, src.size(), file) != src.size()) { - throw std::ios_base::failure("AutoFile::write: write failed"); - } - } + void read(Span<std::byte> dst); + void ignore(size_t nSize); + void write(Span<const std::byte> src); template <typename T> AutoFile& operator<<(const T& obj) { - if (!file) throw std::ios_base::failure("AutoFile::operator<<: file handle is nullptr"); ::Serialize(*this, obj); return *this; } @@ -561,7 +542,6 @@ public: template <typename T> AutoFile& operator>>(T&& obj) { - if (!file) throw std::ios_base::failure("AutoFile::operator>>: file handle is nullptr"); ::Unserialize(*this, obj); return *this; } @@ -574,16 +554,13 @@ private: const int nVersion; public: - CAutoFile(FILE* filenew, int nTypeIn, int nVersionIn) : AutoFile{filenew}, nType(nTypeIn), nVersion(nVersionIn) {} + explicit CAutoFile(std::FILE* file, int type, int version, std::vector<std::byte> data_xor = {}) : AutoFile{file, std::move(data_xor)}, nType{type}, nVersion{version} {} int GetType() const { return nType; } int GetVersion() const { return nVersion; } template<typename T> CAutoFile& operator<<(const T& obj) { - // Serialize to this stream - if (!file) - throw std::ios_base::failure("CAutoFile::operator<<: file handle is nullptr"); ::Serialize(*this, obj); return (*this); } @@ -591,9 +568,6 @@ public: template<typename T> CAutoFile& operator>>(T&& obj) { - // Unserialize from this stream - if (!file) - throw std::ios_base::failure("CAutoFile::operator>>: file handle is nullptr"); ::Unserialize(*this, obj); return (*this); } @@ -742,7 +716,6 @@ public: template<typename T> CBufferedFile& operator>>(T&& obj) { - // Unserialize from this stream ::Unserialize(*this, obj); return (*this); } diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index e4ed861b12..f52c692649 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -30,22 +30,21 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) .notifications = notifications, }; BlockManager blockman{m_node.kernel->interrupt, blockman_opts}; - CChain chain {}; // simulate adding a genesis block normally - BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, chain, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); // simulate what happens during reindex // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file // the block is found at offset 8 because there is an 8 byte serialization header // consisting of 4 magic bytes + 4 length bytes before each block in a well-formed blk file. FlatFilePos pos{0, BLOCK_SERIALIZATION_HEADER_SIZE}; - BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, chain, &pos).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, &pos).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); // now simulate what happens after reindex for the first new block processed // the actual block contents don't matter, just that it's a block. // verify that the write position is at offset 0x12d. // this is a check to make sure that https://github.com/bitcoin/bitcoin/issues/21379 does not recur // 8 bytes (for serialization header) + 285 (for serialized genesis block) = 293 // add another 8 bytes for the second block's serialization header and we get 293 + 8 = 301 - FlatFilePos actual{blockman.SaveBlockToDisk(params->GenesisBlock(), 1, chain, nullptr)}; + FlatFilePos actual{blockman.SaveBlockToDisk(params->GenesisBlock(), 1, nullptr)}; BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(params->GenesisBlock(), CLIENT_VERSION) + BLOCK_SERIALIZATION_HEADER_SIZE); } diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 74d6d7231a..787a196a0c 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -98,7 +98,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) LOCK(cs_main); BlockValidationState state; BOOST_CHECK(CheckBlock(block, state, params.GetConsensus())); - BOOST_CHECK(chainstate.AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr, true)); + BOOST_CHECK(m_node.chainman->AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr, true)); CCoinsViewCache view(&chainstate.CoinsTip()); BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view)); } diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 9193d9a8b3..b740a51574 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -107,9 +107,19 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) peerman.FinalizeNode(dummyNode1); } -static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType) +static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType, bool onion_peer = false) { - CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); + CAddress addr; + + if (onion_peer) { + auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)}; + BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr))); + } + + while (!addr.IsRoutable()) { + addr = CAddress(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); + } + vNodes.emplace_back(new CNode{id++, /*sock=*/nullptr, addr, @@ -197,6 +207,30 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true); BOOST_CHECK(vNodes.back()->fDisconnect == false); + vNodes[max_outbound_full_relay - 1]->fDisconnect = false; + + // Add an onion peer, that will be protected because it is the only one for + // its network, so another peer gets disconnected instead. + SetMockTime(time_init); + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY, /*onion_peer=*/true); + SetMockTime(time_later); + peerLogic->CheckForStaleTipAndEvictPeers(); + + for (int i = 0; i < max_outbound_full_relay - 2; ++i) { + BOOST_CHECK(vNodes[i]->fDisconnect == false); + } + BOOST_CHECK(vNodes[max_outbound_full_relay - 2]->fDisconnect == false); + BOOST_CHECK(vNodes[max_outbound_full_relay - 1]->fDisconnect == true); + BOOST_CHECK(vNodes[max_outbound_full_relay]->fDisconnect == false); + + // Add a second onion peer which won't be protected + SetMockTime(time_init); + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY, /*onion_peer=*/true); + SetMockTime(time_later); + peerLogic->CheckForStaleTipAndEvictPeers(); + + BOOST_CHECK(vNodes.back()->fDisconnect == true); + for (const CNode *node : vNodes) { peerLogic->FinalizeNode(*node); } diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 8ac1330dcb..02df4590de 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -263,40 +263,21 @@ FUZZ_TARGET(addrman, .init = initialize_addrman) [&] { std::vector<CAddress> addresses; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) { - const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); - if (!opt_address) { - break; - } - addresses.push_back(*opt_address); - } - const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider); - if (opt_net_addr) { - addr_man.Add(addresses, *opt_net_addr, std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)}); + addresses.push_back(ConsumeAddress(fuzzed_data_provider)); } + addr_man.Add(addresses, ConsumeNetAddr(fuzzed_data_provider), std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.Good(*opt_service, NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); - } + addr_man.Good(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.Attempt(*opt_service, fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); - } + addr_man.Attempt(ConsumeService(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.Connected(*opt_service, NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); - } + addr_man.Connected(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.SetServices(*opt_service, ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS)); - } + addr_man.SetServices(ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS)); }); } const AddrMan& const_addr_man{addr_man}; @@ -334,4 +315,4 @@ FUZZ_TARGET(addrman_serdeser, .init = initialize_addrman) data_stream << addr_man1; data_stream >> addr_man2; assert(addr_man1 == addr_man2); -} +}
\ No newline at end of file diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 23fadd8984..cdf240dc59 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -32,7 +32,7 @@ FUZZ_TARGET(connman, .init = initialize_connman) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - CConnman connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), + ConnmanTestMsg connman{fuzzed_data_provider.ConsumeIntegral<uint64_t>(), fuzzed_data_provider.ConsumeIntegral<uint64_t>(), *g_setup->m_node.addrman, *g_setup->m_node.netgroupman, @@ -41,6 +41,12 @@ FUZZ_TARGET(connman, .init = initialize_connman) CNode random_node = ConsumeNode(fuzzed_data_provider); CSubNet random_subnet; std::string random_string; + + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100) { + CNode& p2p_node{*ConsumeNodeAsUniquePtr(fuzzed_data_provider).release()}; + connman.AddTestNode(p2p_node); + } + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) { CallOneOf( fuzzed_data_provider, @@ -128,4 +134,6 @@ FUZZ_TARGET(connman, .init = initialize_connman) (void)connman.GetTotalBytesSent(); (void)connman.GetTryNewOutboundPeer(); (void)connman.GetUseAddrmanOutgoing(); + + connman.ClearTestNodes(); } diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index b879b0d141..8ed659323c 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -85,7 +85,7 @@ public: std::string desc; desc.reserve(mocked_desc.size()); - // Replace all occurences of '%' followed by two hex characters with the corresponding key. + // Replace all occurrences of '%' followed by two hex characters with the corresponding key. for (size_t i = 0; i < mocked_desc.size();) { if (mocked_desc[i] == '%') { if (i + 3 >= mocked_desc.size()) return {}; diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index 502f7b897c..bdaa4ad1b8 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -35,9 +35,9 @@ FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_fil // Corresponds to the -reindex case (track orphan blocks across files). FlatFilePos flat_file_pos; std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent; - g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file, &flat_file_pos, &blocks_with_unknown_parent); + g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file, &flat_file_pos, &blocks_with_unknown_parent); } else { // Corresponds to the -loadblock= case (orphan blocks aren't tracked across files). - g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file); + g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file); } } diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 2699d316da..e448805e69 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -83,10 +83,10 @@ BOOST_AUTO_TEST_CASE(logging_timer) BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) { LogInstance().m_log_sourcelocations = true; - LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s", "bar1\n"); - LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::None, "foo2: %s", "bar2\n"); - LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo3: %s", "bar3\n"); - LogPrintf_("fn4", "src4", 4, BCLog::LogFlags::NONE, BCLog::Level::None, "foo4: %s", "bar4\n"); + LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s\n", "bar1"); + LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::None, "foo2: %s\n", "bar2"); + LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo3: %s\n", "bar3"); + LogPrintf_("fn4", "src4", 4, BCLog::LogFlags::NONE, BCLog::Level::None, "foo4: %s\n", "bar4"); std::ifstream file{tmp_log_path}; std::vector<std::string> log_lines; for (std::string log; std::getline(file, log);) { diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 55e4f200b1..5232175824 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -6,6 +6,7 @@ #include <test/util/random.h> #include <test/util/setup_common.h> #include <util/fs.h> +#include <util/strencodings.h> #include <boost/test/unit_test.hpp> @@ -13,6 +14,55 @@ using namespace std::string_literals; BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup) +BOOST_AUTO_TEST_CASE(xor_file) +{ + fs::path xor_path{m_args.GetDataDirBase() / "test_xor.bin"}; + auto raw_file{[&](const auto& mode) { return fsbridge::fopen(xor_path, mode); }}; + const std::vector<uint8_t> test1{1, 2, 3}; + const std::vector<uint8_t> test2{4, 5}; + const std::vector<std::byte> xor_pat{std::byte{0xff}, std::byte{0x00}}; + { + // Check errors for missing file + AutoFile xor_file{raw_file("rb"), xor_pat}; + BOOST_CHECK_EXCEPTION(xor_file << std::byte{}, std::ios_base::failure, HasReason{"AutoFile::write: file handle is nullpt"}); + BOOST_CHECK_EXCEPTION(xor_file >> std::byte{}, std::ios_base::failure, HasReason{"AutoFile::read: file handle is nullpt"}); + BOOST_CHECK_EXCEPTION(xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: file handle is nullpt"}); + } + { + AutoFile xor_file{raw_file("wbx"), xor_pat}; + xor_file << test1 << test2; + } + { + // Read raw from disk + AutoFile non_xor_file{raw_file("rb")}; + std::vector<std::byte> raw(7); + non_xor_file >> Span{raw}; + BOOST_CHECK_EQUAL(HexStr(raw), "fc01fd03fd04fa"); + // Check that no padding exists + BOOST_CHECK_EXCEPTION(non_xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: end of file"}); + } + { + AutoFile xor_file{raw_file("rb"), xor_pat}; + std::vector<std::byte> read1, read2; + xor_file >> read1 >> read2; + BOOST_CHECK_EQUAL(HexStr(read1), HexStr(test1)); + BOOST_CHECK_EQUAL(HexStr(read2), HexStr(test2)); + // Check that eof was reached + BOOST_CHECK_EXCEPTION(xor_file >> std::byte{}, std::ios_base::failure, HasReason{"AutoFile::read: end of file"}); + } + { + AutoFile xor_file{raw_file("rb"), xor_pat}; + std::vector<std::byte> read2; + // Check that ignore works + xor_file.ignore(4); + xor_file >> read2; + BOOST_CHECK_EQUAL(HexStr(read2), HexStr(test2)); + // Check that ignore and read fail now + BOOST_CHECK_EXCEPTION(xor_file.ignore(1), std::ios_base::failure, HasReason{"AutoFile::ignore: end of file"}); + BOOST_CHECK_EXCEPTION(xor_file >> std::byte{}, std::ios_base::failure, HasReason{"AutoFile::read: end of file"}); + } +} + BOOST_AUTO_TEST_CASE(streams_vector_writer) { unsigned char a(1); diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index bf8f8b5819..7f55916870 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -50,7 +50,7 @@ CreateAndActivateUTXOSnapshot( UniValue result = CreateUTXOSnapshot( node, node.chainman->ActiveChainstate(), auto_outfile, snapshot_path, snapshot_path); LogPrintf( - "Wrote UTXO snapshot to %s: %s", fs::PathToString(snapshot_path.make_preferred()), result.write()); + "Wrote UTXO snapshot to %s: %s\n", fs::PathToString(snapshot_path.make_preferred()), result.write()); // Read the written snapshot in and then activate it. // @@ -71,6 +71,7 @@ CreateAndActivateUTXOSnapshot( // This is a stripped-down version of node::LoadChainstate which // preserves the block index. LOCK(::cs_main); + CBlockIndex *orig_tip = node.chainman->ActiveChainstate().m_chain.Tip(); uint256 gen_hash = node.chainman->ActiveChainstate().m_chain[0]->GetBlockHash(); node.chainman->ResetChainstates(); node.chainman->InitializeChainstate(node.mempool.get()); @@ -83,6 +84,22 @@ CreateAndActivateUTXOSnapshot( chain.setBlockIndexCandidates.insert(node.chainman->m_blockman.LookupBlockIndex(gen_hash)); chain.LoadChainTip(); node.chainman->MaybeRebalanceCaches(); + + // Reset the HAVE_DATA flags below the snapshot height, simulating + // never-having-downloaded them in the first place. + // TODO: perhaps we could improve this by using pruning to delete + // these blocks instead + CBlockIndex *pindex = orig_tip; + while (pindex && pindex != chain.m_chain.Tip()) { + pindex->nStatus &= ~BLOCK_HAVE_DATA; + pindex->nStatus &= ~BLOCK_HAVE_UNDO; + // We have to set the ASSUMED_VALID flag, because otherwise it + // would not be possible to have a block index entry without HAVE_DATA + // and with nTx > 0 (since we aren't setting the pruned flag); + // see CheckBlockIndex(). + pindex->nStatus |= BLOCK_ASSUMED_VALID; + pindex = pindex->pprev; + } } BlockValidationState state; if (!node.chainman->ActiveChainstate().ActivateBestChain(state)) { diff --git a/src/test/util/net.h b/src/test/util/net.h index e6506b0d08..b2f6ebb163 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -29,7 +29,10 @@ struct ConnmanTestMsg : public CConnman { { LOCK(m_nodes_mutex); m_nodes.push_back(&node); + + if (node.IsManualOrFullOutboundConn()) ++m_network_conn_counts[node.addr.GetNetwork()]; } + void ClearTestNodes() { LOCK(m_nodes_mutex); diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 2078fcd8f8..fe2d2ba592 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -77,6 +77,13 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // After adding some blocks to the tip, best block should have changed. BOOST_CHECK(::g_best_block != curr_tip); + // Grab block 1 from disk; we'll add it to the background chain later. + std::shared_ptr<CBlock> pblockone = std::make_shared<CBlock>(); + { + LOCK(::cs_main); + chainman.m_blockman.ReadBlockFromDisk(*pblockone, *chainman.ActiveChain()[1]); + } + BOOST_REQUIRE(CreateAndActivateUTXOSnapshot( this, NoMalleation, /*reset_chainstate=*/ true)); @@ -104,11 +111,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) assert(false); }()}; - // Create a block to append to the validation chain. - std::vector<CMutableTransaction> noTxns; - CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; - CBlock validation_block = this->CreateBlock(noTxns, scriptPubKey, background_cs); - auto pblock = std::make_shared<const CBlock>(validation_block); + // Append the first block to the background chain. BlockValidationState state; CBlockIndex* pindex = nullptr; const CChainParams& chainparams = Params(); @@ -118,17 +121,18 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) // once it is changed to support multiple chainstates. { LOCK(::cs_main); - bool checked = CheckBlock(*pblock, state, chainparams.GetConsensus()); + bool checked = CheckBlock(*pblockone, state, chainparams.GetConsensus()); BOOST_CHECK(checked); - bool accepted = background_cs.AcceptBlock( - pblock, state, &pindex, true, nullptr, &newblock, true); + bool accepted = chainman.AcceptBlock( + pblockone, state, &pindex, true, nullptr, &newblock, true); BOOST_CHECK(accepted); } + // UpdateTip is called here - bool block_added = background_cs.ActivateBestChain(state, pblock); + bool block_added = background_cs.ActivateBestChain(state, pblockone); // Ensure tip is as expected - BOOST_CHECK_EQUAL(background_cs.m_chain.Tip()->GetBlockHash(), validation_block.GetHash()); + BOOST_CHECK_EQUAL(background_cs.m_chain.Tip()->GetBlockHash(), pblockone->GetHash()); // g_best_block should be unchanged after adding a block to the background // validation chain. diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 99860961a2..160a807f69 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -49,6 +49,9 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) c1.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); + c1.LoadGenesisBlock(); + BlockValidationState val_state; + BOOST_CHECK(c1.ActivateBestChain(val_state, nullptr)); BOOST_CHECK(!manager.IsSnapshotActive()); BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated())); @@ -58,7 +61,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) auto& active_chain = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain()); BOOST_CHECK_EQUAL(&active_chain, &c1.m_chain); - BOOST_CHECK_EQUAL(WITH_LOCK(manager.GetMutex(), return manager.ActiveHeight()), -1); + BOOST_CHECK_EQUAL(WITH_LOCK(manager.GetMutex(), return manager.ActiveHeight()), 0); auto active_tip = WITH_LOCK(manager.GetMutex(), return manager.ActiveTip()); auto exp_tip = c1.m_chain.Tip(); @@ -68,7 +71,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a snapshot-based chainstate. // - const uint256 snapshot_blockhash = GetRandHash(); + const uint256 snapshot_blockhash = active_tip->GetBlockHash(); Chainstate& c2 = WITH_LOCK(::cs_main, return manager.ActivateExistingSnapshot( &mempool, snapshot_blockhash)); chainstates.push_back(&c2); @@ -78,8 +81,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); WITH_LOCK(::cs_main, c2.InitCoinsCache(1 << 23)); - // Unlike c1, which doesn't have any blocks. Gets us different tip, height. - c2.LoadGenesisBlock(); + c2.m_chain.SetTip(*active_tip); BlockValidationState _; BOOST_CHECK(c2.ActivateBestChain(_, nullptr)); @@ -99,16 +101,14 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) auto exp_tip2 = c2.m_chain.Tip(); BOOST_CHECK_EQUAL(active_tip2, exp_tip2); - // Ensure that these pointers actually correspond to different - // CCoinsViewCache instances. - BOOST_CHECK(exp_tip != exp_tip2); + BOOST_CHECK_EQUAL(exp_tip, exp_tip2); // Let scheduler events finish running to avoid accessing memory that is going to be unloaded SyncWithValidationInterfaceQueue(); } //! Test rebalancing the caches associated with each chainstate. -BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) +BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) { ChainstateManager& manager = *m_node.chainman; CTxMemPool& mempool = *m_node.mempool; @@ -121,7 +121,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a legacy (IBD) chainstate. // - Chainstate& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(&mempool)); + Chainstate& c1 = manager.ActiveChainstate(); chainstates.push_back(&c1); c1.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); @@ -129,8 +129,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) { LOCK(::cs_main); c1.InitCoinsCache(1 << 23); - BOOST_REQUIRE(c1.LoadGenesisBlock()); - c1.CoinsTip().SetBestBlock(InsecureRand256()); manager.MaybeRebalanceCaches(); } @@ -139,7 +137,8 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a snapshot-based chainstate. // - Chainstate& c2 = WITH_LOCK(cs_main, return manager.ActivateExistingSnapshot(&mempool, GetRandHash())); + CBlockIndex* snapshot_base{WITH_LOCK(manager.GetMutex(), return manager.ActiveChain()[manager.ActiveChain().Height() / 2])}; + Chainstate& c2 = WITH_LOCK(cs_main, return manager.ActivateExistingSnapshot(&mempool, *snapshot_base->phashBlock)); chainstates.push_back(&c2); c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); @@ -147,8 +146,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) { LOCK(::cs_main); c2.InitCoinsCache(1 << 23); - BOOST_REQUIRE(c2.LoadGenesisBlock()); - c2.CoinsTip().SetBestBlock(InsecureRand256()); manager.MaybeRebalanceCaches(); } @@ -415,7 +412,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, SnapshotTestSetup) //! - Then mark a region of the chain BLOCK_ASSUMED_VALID and introduce a second chainstate //! that will tolerate assumed-valid blocks. Run LoadBlockIndex() and ensure that the first //! chainstate only contains fully validated blocks and the other chainstate contains all blocks, -//! even those assumed-valid. +//! except those marked assume-valid, because those entries don't HAVE_DATA. //! BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) { @@ -430,28 +427,34 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) const int assumed_valid_start_idx = last_assumed_valid_idx - expected_assumed_valid; CBlockIndex* validated_tip{nullptr}; + CBlockIndex* assumed_base{nullptr}; CBlockIndex* assumed_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; auto reload_all_block_indexes = [&]() { + // For completeness, we also reset the block sequence counters to + // ensure that no state which affects the ranking of tip-candidates is + // retained (even though this isn't strictly necessary). + WITH_LOCK(::cs_main, return chainman.ResetBlockSequenceCounters()); for (Chainstate* cs : chainman.GetAll()) { LOCK(::cs_main); - cs->UnloadBlockIndex(); + cs->ClearBlockIndexCandidates(); BOOST_CHECK(cs->setBlockIndexCandidates.empty()); } WITH_LOCK(::cs_main, chainman.LoadBlockIndex()); }; - // Ensure that without any assumed-valid BlockIndex entries, all entries are considered - // tip candidates. + // Ensure that without any assumed-valid BlockIndex entries, only the current tip is + // considered as a candidate. reload_all_block_indexes(); - BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), cs1.m_chain.Height() + 1); + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), 1); - // Mark some region of the chain assumed-valid. + // Mark some region of the chain assumed-valid, and remove the HAVE_DATA flag. for (int i = 0; i <= cs1.m_chain.Height(); ++i) { LOCK(::cs_main); auto index = cs1.m_chain[i]; + // Blocks with heights in range [20, 40) are marked ASSUMED_VALID if (i < last_assumed_valid_idx && i >= assumed_valid_start_idx) { index->nStatus = BlockStatus::BLOCK_VALID_TREE | BlockStatus::BLOCK_ASSUMED_VALID; } @@ -464,25 +467,41 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) validated_tip = index; BOOST_CHECK(!index->IsAssumedValid()); } + // Note the last assumed valid block as the snapshot base + if (i == last_assumed_valid_idx - 1) { + assumed_base = index; + BOOST_CHECK(index->IsAssumedValid()); + } else if (i == last_assumed_valid_idx) { + BOOST_CHECK(!index->IsAssumedValid()); + } } BOOST_CHECK_EQUAL(expected_assumed_valid, num_assumed_valid); + // Note: cs2's tip is not set when ActivateExistingSnapshot is called. Chainstate& cs2 = WITH_LOCK(::cs_main, - return chainman.ActivateExistingSnapshot(&mempool, GetRandHash())); + return chainman.ActivateExistingSnapshot(&mempool, *assumed_base->phashBlock)); + + // Set tip of the fully validated chain to be the validated tip + cs1.m_chain.SetTip(*validated_tip); + + // Set tip of the assume-valid-based chain to the assume-valid block + cs2.m_chain.SetTip(*assumed_base); reload_all_block_indexes(); - // The fully validated chain only has candidates up to the start of the assumed-valid - // blocks. + // The fully validated chain should have the current validated tip + // and the assumed valid base as candidates. + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), 2); BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(validated_tip), 1); - BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(assumed_tip), 0); - BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), assumed_valid_start_idx); + BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(assumed_base), 1); - // The assumed-valid tolerant chain has all blocks as candidates. - BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(validated_tip), 1); + // The assumed-valid tolerant chain has the assumed valid base as a + // candidate, but otherwise has none of the assumed-valid (which do not + // HAVE_DATA) blocks as candidates. + BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(validated_tip), 0); BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(assumed_tip), 1); - BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes); + BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes - last_assumed_valid_idx + 1); } //! Ensure that snapshot chainstates initialize properly when found on disk. diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 845fbdb66e..79b2b4ec94 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -155,12 +155,12 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256>& vHashes } util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateAncestorsAndCheckLimits( - size_t entry_size, + int64_t entry_size, size_t entry_count, CTxMemPoolEntry::Parents& staged_ancestors, const Limits& limits) const { - size_t totalSizeWithAncestors = entry_size; + int64_t totalSizeWithAncestors = entry_size; setEntries ancestors; while (!staged_ancestors.empty()) { @@ -171,11 +171,11 @@ util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateAncestorsAndCheckLimit staged_ancestors.erase(stage); totalSizeWithAncestors += stageit->GetTxSize(); - if (stageit->GetSizeWithDescendants() + entry_size > static_cast<uint64_t>(limits.descendant_size_vbytes)) { + if (stageit->GetSizeWithDescendants() + entry_size > limits.descendant_size_vbytes) { return util::Error{Untranslated(strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_size_vbytes))}; } else if (stageit->GetCountWithDescendants() + entry_count > static_cast<uint64_t>(limits.descendant_count)) { return util::Error{Untranslated(strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limits.descendant_count))}; - } else if (totalSizeWithAncestors > static_cast<uint64_t>(limits.ancestor_size_vbytes)) { + } else if (totalSizeWithAncestors > limits.ancestor_size_vbytes) { return util::Error{Untranslated(strprintf("exceeds ancestor size limit [limit: %u]", limits.ancestor_size_vbytes))}; } @@ -201,7 +201,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, std::string &errString) const { CTxMemPoolEntry::Parents staged_ancestors; - size_t total_size = 0; + int64_t total_size = 0; for (const auto& tx : package) { total_size += GetVirtualTransactionSize(*tx); for (const auto& input : tx->vin) { diff --git a/src/txmempool.h b/src/txmempool.h index 846def02cd..a1867eb895 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -442,7 +442,7 @@ private: * * @return all in-mempool ancestors, or an error if any ancestor or descendant limits were hit */ - util::Result<setEntries> CalculateAncestorsAndCheckLimits(size_t entry_size, + util::Result<setEntries> CalculateAncestorsAndCheckLimits(int64_t entry_size, size_t entry_count, CTxMemPoolEntry::Parents &staged_ancestors, const Limits& limits diff --git a/src/util/trace.h b/src/util/trace.h index 7a63f39c83..051921a0d2 100644 --- a/src/util/trace.h +++ b/src/util/trace.h @@ -9,19 +9,28 @@ #include <sys/sdt.h> -#define TRACE(context, event) DTRACE_PROBE(context, event) -#define TRACE1(context, event, a) DTRACE_PROBE1(context, event, a) -#define TRACE2(context, event, a, b) DTRACE_PROBE2(context, event, a, b) -#define TRACE3(context, event, a, b, c) DTRACE_PROBE3(context, event, a, b, c) -#define TRACE4(context, event, a, b, c, d) DTRACE_PROBE4(context, event, a, b, c, d) -#define TRACE5(context, event, a, b, c, d, e) DTRACE_PROBE5(context, event, a, b, c, d, e) -#define TRACE6(context, event, a, b, c, d, e, f) DTRACE_PROBE6(context, event, a, b, c, d, e, f) -#define TRACE7(context, event, a, b, c, d, e, f, g) DTRACE_PROBE7(context, event, a, b, c, d, e, f, g) -#define TRACE8(context, event, a, b, c, d, e, f, g, h) DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h) -#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i) -#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j) -#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k) -#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) +// Disabling this warning can be removed once we switch to C++20 +#if defined(__clang__) && __cplusplus < 202002L +#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wgnu-zero-variadic-macro-arguments\"") +#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP _Pragma("clang diagnostic pop") +#else +#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH +#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#endif + +#define TRACE(context, event) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE(context, event) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE1(context, event, a) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE1(context, event, a) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE2(context, event, a, b) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE2(context, event, a, b) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE3(context, event, a, b, c) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE3(context, event, a, b, c) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE4(context, event, a, b, c, d) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE4(context, event, a, b, c, d) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE5(context, event, a, b, c, d, e) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE5(context, event, a, b, c, d, e) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE6(context, event, a, b, c, d, e, f) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE6(context, event, a, b, c, d, e, f) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE7(context, event, a, b, c, d, e, f, g) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE7(context, event, a, b, c, d, e, f, g) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE8(context, event, a, b, c, d, e, f, g, h) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP #else diff --git a/src/validation.cpp b/src/validation.cpp index e4b5381f00..c45c847471 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1579,6 +1579,13 @@ Chainstate::Chainstate( m_chainman(chainman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} +const CBlockIndex* Chainstate::SnapshotBase() +{ + if (!m_from_snapshot_blockhash) return nullptr; + if (!m_cached_snapshot_base) m_cached_snapshot_base = Assert(m_chainman.m_blockman.LookupBlockIndex(*m_from_snapshot_blockhash)); + return m_cached_snapshot_base; +} + void Chainstate::InitCoinsDB( size_t cache_size_bytes, bool in_memory, @@ -3095,7 +3102,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< // Belt-and-suspenders check that we aren't attempting to advance the background // chainstate past the snapshot base block. if (WITH_LOCK(::cs_main, return m_disabled)) { - LogPrintf("m_disabled is set - this chainstate should not be in operation. " /* Continued */ + LogPrintf("m_disabled is set - this chainstate should not be in operation. " "Please report this as a bug. %s\n", PACKAGE_BUGREPORT); return false; } @@ -3193,7 +3200,8 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< // that the best block hash is non-null. if (m_chainman.m_interrupt) break; } while (pindexNewTip != pindexMostWork); - CheckBlockIndex(); + + m_chainman.CheckBlockIndex(); // Write changes periodically to disk, after relay. if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) { @@ -3213,17 +3221,17 @@ bool Chainstate::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) // Nothing to do, this block is not at the tip. return true; } - if (m_chain.Tip()->nChainWork > nLastPreciousChainwork) { + if (m_chain.Tip()->nChainWork > m_chainman.nLastPreciousChainwork) { // The chain has been extended since the last call, reset the counter. - nBlockReverseSequenceId = -1; + m_chainman.nBlockReverseSequenceId = -1; } - nLastPreciousChainwork = m_chain.Tip()->nChainWork; + m_chainman.nLastPreciousChainwork = m_chain.Tip()->nChainWork; setBlockIndexCandidates.erase(pindex); - pindex->nSequenceId = nBlockReverseSequenceId; - if (nBlockReverseSequenceId > std::numeric_limits<int32_t>::min()) { + pindex->nSequenceId = m_chainman.nBlockReverseSequenceId; + if (m_chainman.nBlockReverseSequenceId > std::numeric_limits<int32_t>::min()) { // We can't keep reducing the counter if somebody really wants to // call preciousblock 2**31-1 times on the same set of tips... - nBlockReverseSequenceId--; + m_chainman.nBlockReverseSequenceId--; } if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && pindex->HaveTxsDownloaded()) { setBlockIndexCandidates.insert(pindex); @@ -3339,7 +3347,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde to_mark_failed = invalid_walk_tip; } - CheckBlockIndex(); + m_chainman.CheckBlockIndex(); { LOCK(cs_main); @@ -3416,8 +3424,32 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) { } } +void Chainstate::TryAddBlockIndexCandidate(CBlockIndex* pindex) +{ + AssertLockHeld(cs_main); + // The block only is a candidate for the most-work-chain if it has more work than our current tip. + if (m_chain.Tip() != nullptr && setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) { + return; + } + + bool is_active_chainstate = this == &m_chainman.ActiveChainstate(); + if (is_active_chainstate) { + // The active chainstate should always add entries that have more + // work than the tip. + setBlockIndexCandidates.insert(pindex); + } else if (!m_disabled) { + // For the background chainstate, we only consider connecting blocks + // towards the snapshot base (which can't be nullptr or else we'll + // never make progress). + const CBlockIndex* snapshot_base{Assert(m_chainman.GetSnapshotBaseBlock())}; + if (snapshot_base->GetAncestor(pindex->nHeight) == pindex) { + setBlockIndexCandidates.insert(pindex); + } + } +} + /** Mark a block as having its data received and checked (up to BLOCK_VALID_TRANSACTIONS). */ -void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) +void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) { AssertLockHeld(cs_main); pindexNew->nTx = block.vtx.size(); @@ -3426,7 +3458,7 @@ void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pin pindexNew->nDataPos = pos.nPos; pindexNew->nUndoPos = 0; pindexNew->nStatus |= BLOCK_HAVE_DATA; - if (DeploymentActiveAt(*pindexNew, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { + if (DeploymentActiveAt(*pindexNew, *this, Consensus::DEPLOYMENT_SEGWIT)) { pindexNew->nStatus |= BLOCK_OPT_WITNESS; } pindexNew->RaiseValidity(BLOCK_VALID_TRANSACTIONS); @@ -3443,8 +3475,8 @@ void Chainstate::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pin queue.pop_front(); pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx; pindex->nSequenceId = nBlockSequenceId++; - if (m_chain.Tip() == nullptr || !setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) { - setBlockIndexCandidates.insert(pindex); + for (Chainstate *c : GetAll()) { + c->TryAddBlockIndexCandidate(pindex); } std::pair<std::multimap<CBlockIndex*, CBlockIndex*>::iterator, std::multimap<CBlockIndex*, CBlockIndex*>::iterator> range = m_blockman.m_blocks_unlinked.equal_range(pindex); while (range.first != range.second) { @@ -3858,7 +3890,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>& for (const CBlockHeader& header : headers) { CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast bool accepted{AcceptBlockHeader(header, state, &pindex, min_pow_checked)}; - ActiveChainstate().CheckBlockIndex(); + CheckBlockIndex(); if (!accepted) { return false; @@ -3905,7 +3937,7 @@ void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t } /** Store block on disk. If dbp is non-nullptr, the file is known to already reside on disk */ -bool Chainstate::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) +bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) { const CBlock& block = *pblock; @@ -3915,23 +3947,24 @@ bool Chainstate::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockV CBlockIndex *pindexDummy = nullptr; CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy; - bool accepted_header{m_chainman.AcceptBlockHeader(block, state, &pindex, min_pow_checked)}; + bool accepted_header{AcceptBlockHeader(block, state, &pindex, min_pow_checked)}; CheckBlockIndex(); if (!accepted_header) return false; - // Try to process all requested blocks that we don't have, but only - // process an unrequested block if it's new and has enough work to - // advance our tip, and isn't too many blocks ahead. + // Check all requested blocks that we do not already have for validity and + // save them to disk. Skip processing of unrequested blocks as an anti-DoS + // measure, unless the blocks have more work than the active chain tip, and + // aren't too far ahead of it, so are likely to be attached soon. bool fAlreadyHave = pindex->nStatus & BLOCK_HAVE_DATA; - bool fHasMoreOrSameWork = (m_chain.Tip() ? pindex->nChainWork >= m_chain.Tip()->nChainWork : true); + bool fHasMoreOrSameWork = (ActiveTip() ? pindex->nChainWork >= ActiveTip()->nChainWork : true); // Blocks that are too out-of-order needlessly limit the effectiveness of // pruning, because pruning will not delete block files that contain any // blocks which are too close in height to the tip. Apply this test // regardless of whether pruning is enabled; it should generally be safe to // not process unrequested blocks. - bool fTooFarAhead{pindex->nHeight > m_chain.Height() + int(MIN_BLOCKS_TO_KEEP)}; + bool fTooFarAhead{pindex->nHeight > ActiveHeight() + int(MIN_BLOCKS_TO_KEEP)}; // TODO: Decouple this function from the block download logic by removing fRequested // This requires some new chain data structure to efficiently look up if a @@ -3951,13 +3984,13 @@ bool Chainstate::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockV // If our tip is behind, a peer could try to send us // low-work blocks on a fake chain that we would never // request; don't process these. - if (pindex->nChainWork < m_chainman.MinimumChainWork()) return true; + if (pindex->nChainWork < MinimumChainWork()) return true; } - const CChainParams& params{m_chainman.GetParams()}; + const CChainParams& params{GetParams()}; if (!CheckBlock(block, state, params.GetConsensus()) || - !ContextualCheckBlock(block, state, m_chainman, pindex->pprev)) { + !ContextualCheckBlock(block, state, *this, pindex->pprev)) { if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; m_blockman.m_dirty_blockindex.insert(pindex); @@ -3967,23 +4000,30 @@ bool Chainstate::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockV // Header is valid/has work, merkle tree and segwit merkle tree are good...RELAY NOW // (but if it does not build on our best tip, let the SendMessages loop relay it) - if (!IsInitialBlockDownload() && m_chain.Tip() == pindex->pprev) + if (!ActiveChainstate().IsInitialBlockDownload() && ActiveTip() == pindex->pprev) GetMainSignals().NewPoWValidBlock(pindex, pblock); // Write block to history file if (fNewBlock) *fNewBlock = true; try { - FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, pindex->nHeight, m_chain, dbp)}; + FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, pindex->nHeight, dbp)}; if (blockPos.IsNull()) { state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__)); return false; } ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { - return FatalError(m_chainman.GetNotifications(), state, std::string("System error: ") + e.what()); + return FatalError(GetNotifications(), state, std::string("System error: ") + e.what()); } - FlushStateToDisk(state, FlushStateMode::NONE); + // TODO: FlushStateToDisk() handles flushing of both block and chainstate + // data, so we should move this to ChainstateManager so that we can be more + // intelligent about how we flush. + // For now, since FlushStateMode::NONE is used, all that can happen is that + // the block files may be pruned, so we can just call this on one + // chainstate (particularly if we haven't implemented pruning with + // background validation yet). + ActiveChainstate().FlushStateToDisk(state, FlushStateMode::NONE); CheckBlockIndex(); @@ -4011,7 +4051,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr<const CBlock>& blo bool ret = CheckBlock(*block, state, GetConsensus()); if (ret) { // Store to disk - ret = ActiveChainstate().AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block, min_pow_checked); + ret = AcceptBlock(block, state, &pindex, force_processing, nullptr, new_block, min_pow_checked); } if (!ret) { GetMainSignals().BlockChecked(*block, state); @@ -4379,10 +4419,9 @@ bool Chainstate::NeedsRedownload() const return false; } -void Chainstate::UnloadBlockIndex() +void Chainstate::ClearBlockIndexCandidates() { AssertLockHeld(::cs_main); - nBlockSequenceId = 1; setBlockIndexCandidates.clear(); } @@ -4401,62 +4440,19 @@ bool ChainstateManager::LoadBlockIndex() std::sort(vSortedByHeight.begin(), vSortedByHeight.end(), CBlockIndexHeightOnlyComparator()); - // Find start of assumed-valid region. - int first_assumed_valid_height = std::numeric_limits<int>::max(); - - for (const CBlockIndex* block : vSortedByHeight) { - if (block->IsAssumedValid()) { - auto chainstates = GetAll(); - - // If we encounter an assumed-valid block index entry, ensure that we have - // one chainstate that tolerates assumed-valid entries and another that does - // not (i.e. the background validation chainstate), since assumed-valid - // entries should always be pending validation by a fully-validated chainstate. - auto any_chain = [&](auto fnc) { return std::any_of(chainstates.cbegin(), chainstates.cend(), fnc); }; - assert(any_chain([](auto chainstate) { return chainstate->reliesOnAssumedValid(); })); - assert(any_chain([](auto chainstate) { return !chainstate->reliesOnAssumedValid(); })); - - first_assumed_valid_height = block->nHeight; - LogPrintf("Saw first assumedvalid block at height %d (%s)\n", - first_assumed_valid_height, block->ToString()); - break; - } - } - for (CBlockIndex* pindex : vSortedByHeight) { if (m_interrupt) return false; - if (pindex->IsAssumedValid() || + // If we have an assumeutxo-based chainstate, then the snapshot + // block will be a candidate for the tip, but it may not be + // VALID_TRANSACTIONS (eg if we haven't yet downloaded the block), + // so we special-case the snapshot block as a potential candidate + // here. + if (pindex == GetSnapshotBaseBlock() || (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) { - // Fill each chainstate's block candidate set. Only add assumed-valid - // blocks to the tip candidate set if the chainstate is allowed to rely on - // assumed-valid blocks. - // - // If all setBlockIndexCandidates contained the assumed-valid blocks, the - // background chainstate's ActivateBestChain() call would add assumed-valid - // blocks to the chain (based on how FindMostWorkChain() works). Obviously - // we don't want this since the purpose of the background validation chain - // is to validate assued-valid blocks. - // - // Note: This is considering all blocks whose height is greater or equal to - // the first assumed-valid block to be assumed-valid blocks, and excluding - // them from the background chainstate's setBlockIndexCandidates set. This - // does mean that some blocks which are not technically assumed-valid - // (later blocks on a fork beginning before the first assumed-valid block) - // might not get added to the background chainstate, but this is ok, - // because they will still be attached to the active chainstate if they - // actually contain more work. - // - // Instead of this height-based approach, an earlier attempt was made at - // detecting "holistically" whether the block index under consideration - // relied on an assumed-valid ancestor, but this proved to be too slow to - // be practical. for (Chainstate* chainstate : GetAll()) { - if (chainstate->reliesOnAssumedValid() || - pindex->nHeight < first_assumed_valid_height) { - chainstate->setBlockIndexCandidates.insert(pindex); - } + chainstate->TryAddBlockIndexCandidate(pindex); } } if (pindex->nStatus & BLOCK_FAILED_MASK && (!m_best_invalid || pindex->nChainWork > m_best_invalid->nChainWork)) { @@ -4496,12 +4492,12 @@ bool Chainstate::LoadGenesisBlock() try { const CBlock& block = params.GenesisBlock(); - FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, 0, m_chain, nullptr)}; + FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, 0, nullptr)}; if (blockPos.IsNull()) { return error("%s: writing genesis block to disk failed", __func__); } CBlockIndex* pindex = m_blockman.AddToBlockIndex(block, m_chainman.m_best_header); - ReceivedBlockTransactions(block, pindex, blockPos); + m_chainman.ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { return error("%s: failed to write genesis block: %s", __func__, e.what()); } @@ -4509,18 +4505,16 @@ bool Chainstate::LoadGenesisBlock() return true; } -void Chainstate::LoadExternalBlockFile( +void ChainstateManager::LoadExternalBlockFile( FILE* fileIn, FlatFilePos* dbp, std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent) { - AssertLockNotHeld(m_chainstate_mutex); - // Either both should be specified (-reindex), or neither (-loadblock). assert(!dbp == !blocks_with_unknown_parent); const auto start{SteadyClock::now()}; - const CChainParams& params{m_chainman.GetParams()}; + const CChainParams& params{GetParams()}; int nLoaded = 0; try { @@ -4530,7 +4524,7 @@ void Chainstate::LoadExternalBlockFile( // such as a block fails to deserialize. uint64_t nRewind = blkdat.GetPos(); while (!blkdat.eof()) { - if (m_chainman.m_interrupt) return; + if (m_interrupt) return; blkdat.SetPos(nRewind); nRewind++; // start one byte further next time, in case of failure @@ -4605,8 +4599,15 @@ void Chainstate::LoadExternalBlockFile( // Activate the genesis block so normal node progress can continue if (hash == params.GetConsensus().hashGenesisBlock) { - BlockValidationState state; - if (!ActivateBestChain(state, nullptr)) { + bool genesis_activation_failure = false; + for (auto c : GetAll()) { + BlockValidationState state; + if (!c->ActivateBestChain(state, nullptr)) { + genesis_activation_failure = true; + break; + } + } + if (genesis_activation_failure) { break; } } @@ -4619,14 +4620,21 @@ void Chainstate::LoadExternalBlockFile( // until after all of the block files are loaded. ActivateBestChain can be // called by concurrent network message processing. but, that is not // reliable for the purpose of pruning while importing. - BlockValidationState state; - if (!ActivateBestChain(state, pblock)) { - LogPrint(BCLog::REINDEX, "failed to activate chain (%s)\n", state.ToString()); + bool activation_failure = false; + for (auto c : GetAll()) { + BlockValidationState state; + if (!c->ActivateBestChain(state, pblock)) { + LogPrint(BCLog::REINDEX, "failed to activate chain (%s)\n", state.ToString()); + activation_failure = true; + break; + } + } + if (activation_failure) { break; } } - NotifyHeaderTip(*this); + NotifyHeaderTip(ActiveChainstate()); if (!blocks_with_unknown_parent) continue; @@ -4652,7 +4660,7 @@ void Chainstate::LoadExternalBlockFile( } range.first++; blocks_with_unknown_parent->erase(it); - NotifyHeaderTip(*this); + NotifyHeaderTip(ActiveChainstate()); } } } catch (const std::exception& e) { @@ -4671,14 +4679,14 @@ void Chainstate::LoadExternalBlockFile( } } } catch (const std::runtime_error& e) { - m_chainman.GetNotifications().fatalError(std::string("System error: ") + e.what()); + GetNotifications().fatalError(std::string("System error: ") + e.what()); } LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks<std::chrono::milliseconds>(SteadyClock::now() - start)); } -void Chainstate::CheckBlockIndex() +void ChainstateManager::CheckBlockIndex() { - if (!m_chainman.ShouldCheckBlockIndex()) { + if (!ShouldCheckBlockIndex()) { return; } @@ -4687,7 +4695,7 @@ void Chainstate::CheckBlockIndex() // During a reindex, we read the genesis block and call CheckBlockIndex before ActivateBestChain, // so we have the genesis block in m_blockman.m_block_index but no active chain. (A few of the // tests when iterating the block tree require that m_chain has been initialized.) - if (m_chain.Height() < 0) { + if (ActiveChain().Height() < 0) { assert(m_blockman.m_block_index.size() <= 1); return; } @@ -4717,12 +4725,12 @@ void Chainstate::CheckBlockIndex() CBlockIndex* pindexFirstNotTransactionsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_TRANSACTIONS (regardless of being valid or not). CBlockIndex* pindexFirstNotChainValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_CHAIN (regardless of being valid or not). CBlockIndex* pindexFirstNotScriptsValid = nullptr; // Oldest ancestor of pindex which does not have BLOCK_VALID_SCRIPTS (regardless of being valid or not). + CBlockIndex* pindexFirstAssumeValid = nullptr; // Oldest ancestor of pindex which has BLOCK_ASSUMED_VALID while (pindex != nullptr) { nNodes++; + if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; - // Assumed-valid index entries will not have data since we haven't downloaded the - // full block yet. - if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA) && !pindex->IsAssumedValid()) { + if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) { pindexFirstMissing = pindex; } if (pindexFirstNeverProcessed == nullptr && pindex->nTx == 0) pindexFirstNeverProcessed = pindex; @@ -4751,8 +4759,12 @@ void Chainstate::CheckBlockIndex() // Begin: actual consistency checks. if (pindex->pprev == nullptr) { // Genesis block checks. - assert(pindex->GetBlockHash() == m_chainman.GetConsensus().hashGenesisBlock); // Genesis block's hash must match. - assert(pindex == m_chain.Genesis()); // The current active chain's genesis block must be this block. + assert(pindex->GetBlockHash() == GetConsensus().hashGenesisBlock); // Genesis block's hash must match. + for (auto c : GetAll()) { + if (c->m_chain.Genesis() != nullptr) { + assert(pindex == c->m_chain.Genesis()); // The chain's genesis block must be this block. + } + } } if (!pindex->HaveTxsDownloaded()) assert(pindex->nSequenceId <= 0); // nSequenceId can't be set positive for blocks that aren't linked (negative is used for preciousblock) // VALID_TRANSACTIONS is equivalent to nTx > 0 for all nodes (whether or not pruning has occurred). @@ -4762,7 +4774,13 @@ void Chainstate::CheckBlockIndex() if (!m_blockman.m_have_pruned && !pindex->IsAssumedValid()) { // If we've never pruned, then HAVE_DATA should be equivalent to nTx > 0 assert(!(pindex->nStatus & BLOCK_HAVE_DATA) == (pindex->nTx == 0)); - assert(pindexFirstMissing == pindexFirstNeverProcessed); + if (pindexFirstAssumeValid == nullptr) { + // If we've got some assume valid blocks, then we might have + // missing blocks (not HAVE_DATA) but still treat them as + // having been processed (with a fake nTx value). Otherwise, we + // can assert that these are the same. + assert(pindexFirstMissing == pindexFirstNeverProcessed); + } } else { // If we have pruned, then we can only say that HAVE_DATA implies nTx > 0 if (pindex->nStatus & BLOCK_HAVE_DATA) assert(pindex->nTx > 0); @@ -4792,27 +4810,32 @@ void Chainstate::CheckBlockIndex() // Checks for not-invalid blocks. assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents. } - if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) { - if (pindexFirstInvalid == nullptr) { - const bool is_active = this == &m_chainman.ActiveChainstate(); - - // If this block sorts at least as good as the current tip and - // is valid and we have all data for its parents, it must be in - // setBlockIndexCandidates. m_chain.Tip() must also be there - // even if some data has been pruned. - // - // Don't perform this check for the background chainstate since - // its setBlockIndexCandidates shouldn't have some entries (i.e. those past the - // snapshot block) which do exist in the block index for the active chainstate. - if (is_active && (pindexFirstMissing == nullptr || pindex == m_chain.Tip())) { - assert(setBlockIndexCandidates.count(pindex)); + // Chainstate-specific checks on setBlockIndexCandidates + for (auto c : GetAll()) { + if (c->m_chain.Tip() == nullptr) continue; + if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) { + if (pindexFirstInvalid == nullptr) { + const bool is_active = c == &ActiveChainstate(); + // If this block sorts at least as good as the current tip and + // is valid and we have all data for its parents, it must be in + // setBlockIndexCandidates. m_chain.Tip() must also be there + // even if some data has been pruned. + // + if ((pindexFirstMissing == nullptr || pindex == c->m_chain.Tip())) { + // The active chainstate should always have this block + // as a candidate, but a background chainstate should + // only have it if it is an ancestor of the snapshot base. + if (is_active || GetSnapshotBaseBlock()->GetAncestor(pindex->nHeight) == pindex) { + assert(c->setBlockIndexCandidates.count(pindex)); + } + } + // If some parent is missing, then it could be that this block was in + // setBlockIndexCandidates but had to be removed because of the missing data. + // In this case it must be in m_blocks_unlinked -- see test below. } - // If some parent is missing, then it could be that this block was in - // setBlockIndexCandidates but had to be removed because of the missing data. - // In this case it must be in m_blocks_unlinked -- see test below. + } else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates. + assert(c->setBlockIndexCandidates.count(pindex) == 0); } - } else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates. - assert(setBlockIndexCandidates.count(pindex) == 0); } // Check whether this block is in m_blocks_unlinked. std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangeUnlinked = m_blockman.m_blocks_unlinked.equal_range(pindex->pprev); @@ -4833,18 +4856,23 @@ void Chainstate::CheckBlockIndex() if (pindexFirstMissing == nullptr) assert(!foundInUnlinked); // We aren't missing data for any parent -- cannot be in m_blocks_unlinked. if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && pindexFirstNeverProcessed == nullptr && pindexFirstMissing != nullptr) { // We HAVE_DATA for this block, have received data for all parents at some point, but we're currently missing data for some parent. - assert(m_blockman.m_have_pruned); // We must have pruned. + assert(m_blockman.m_have_pruned || pindexFirstAssumeValid != nullptr); // We must have pruned, or else we're using a snapshot (causing us to have faked the received data for some parent(s)). // This block may have entered m_blocks_unlinked if: // - it has a descendant that at some point had more work than the // tip, and // - we tried switching to that descendant but were missing // data for some intermediate block between m_chain and the // tip. - // So if this block is itself better than m_chain.Tip() and it wasn't in + // So if this block is itself better than any m_chain.Tip() and it wasn't in // setBlockIndexCandidates, then it must be in m_blocks_unlinked. - if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && setBlockIndexCandidates.count(pindex) == 0) { - if (pindexFirstInvalid == nullptr) { - assert(foundInUnlinked); + for (auto c : GetAll()) { + const bool is_active = c == &ActiveChainstate(); + if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && c->setBlockIndexCandidates.count(pindex) == 0) { + if (pindexFirstInvalid == nullptr) { + if (is_active || GetSnapshotBaseBlock()->GetAncestor(pindex->nHeight) == pindex) { + assert(foundInUnlinked); + } + } } } } @@ -4871,6 +4899,7 @@ void Chainstate::CheckBlockIndex() if (pindex == pindexFirstNotTransactionsValid) pindexFirstNotTransactionsValid = nullptr; if (pindex == pindexFirstNotChainValid) pindexFirstNotChainValid = nullptr; if (pindex == pindexFirstNotScriptsValid) pindexFirstNotScriptsValid = nullptr; + if (pindex == pindexFirstAssumeValid) pindexFirstAssumeValid = nullptr; // Find our parent. CBlockIndex* pindexPar = pindex->pprev; // Find which child we just visited. @@ -5197,7 +5226,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( auto maybe_au_data = ExpectedAssumeutxo(base_height, GetParams()); if (!maybe_au_data) { - LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " /* Continued */ + LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " "(%d) - refusing to load snapshot\n", base_height); return false; } @@ -5444,8 +5473,8 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() }; if (index_new.GetBlockHash() != snapshot_blockhash) { - LogPrintf("[snapshot] supposed base block %s does not match the " /* Continued */ - "snapshot base block %s (height %d). Snapshot is not valid.", + LogPrintf("[snapshot] supposed base block %s does not match the " + "snapshot base block %s (height %d). Snapshot is not valid.\n", index_new.ToString(), snapshot_blockhash.ToString(), snapshot_base_height); handle_invalid_snapshot(); return SnapshotCompletionResult::BASE_BLOCKHASH_MISMATCH; @@ -5465,7 +5494,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() auto maybe_au_data = ExpectedAssumeutxo(curr_height, m_options.chainparams); if (!maybe_au_data) { - LogPrintf("[snapshot] assumeutxo data not found for height " /* Continued */ + LogPrintf("[snapshot] assumeutxo data not found for height " "(%d) - refusing to validate snapshot\n", curr_height); handle_invalid_snapshot(); return SnapshotCompletionResult::MISSING_CHAINPARAMS; @@ -5473,7 +5502,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() const AssumeutxoData& au_data = *maybe_au_data; std::optional<CCoinsStats> maybe_ibd_stats; - LogPrintf("[snapshot] computing UTXO stats for background chainstate to validate " /* Continued */ + LogPrintf("[snapshot] computing UTXO stats for background chainstate to validate " "snapshot - this could take a few minutes\n"); try { maybe_ibd_stats = ComputeUTXOStats( @@ -5682,9 +5711,7 @@ util::Result<void> Chainstate::InvalidateCoinsDBOnDisk() const CBlockIndex* ChainstateManager::GetSnapshotBaseBlock() const { - const auto blockhash_op = this->SnapshotBlockhash(); - if (!blockhash_op) return nullptr; - return Assert(m_blockman.LookupBlockIndex(*blockhash_op)); + return m_active_chainstate ? m_active_chainstate->SnapshotBase() : nullptr; } std::optional<int> ChainstateManager::GetSnapshotBaseHeight() const @@ -5713,7 +5740,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() // is in-memory, in which case we can't do on-disk cleanup. You'd better be // in a unittest! if (!ibd_chainstate_path_maybe || !snapshot_chainstate_path_maybe) { - LogPrintf("[snapshot] snapshot chainstate cleanup cannot happen with " /* Continued */ + LogPrintf("[snapshot] snapshot chainstate cleanup cannot happen with " "in-memory chainstates. You are testing, right?\n"); return false; } @@ -5756,7 +5783,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() throw; } - LogPrintf("[snapshot] moving snapshot chainstate (%s) to " /* Continued */ + LogPrintf("[snapshot] moving snapshot chainstate (%s) to " "default chainstate directory (%s)\n", fs::PathToString(snapshot_chainstate_path), fs::PathToString(ibd_chainstate_path)); @@ -5770,7 +5797,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() if (!DeleteCoinsDBFromDisk(tmp_old, /*is_snapshot=*/false)) { // No need to FatalError because once the unneeded bg chainstate data is // moved, it will not interfere with subsequent initialization. - LogPrintf("Deletion of %s failed. Please remove it manually, as the " /* Continued */ + LogPrintf("Deletion of %s failed. Please remove it manually, as the " "directory is now unnecessary.\n", fs::PathToString(tmp_old)); } else { diff --git a/src/validation.h b/src/validation.h index af8ceb5dfa..d7ad86a5e8 100644 --- a/src/validation.h +++ b/src/validation.h @@ -466,17 +466,6 @@ class Chainstate { protected: /** - * Every received block is assigned a unique and increasing identifier, so we - * know which one to give priority in case of a fork. - */ - /** Blocks loaded from disk are assigned id 0, so start the counter at 1. */ - int32_t nBlockSequenceId GUARDED_BY(::cs_main) = 1; - /** Decreasing counter (used by subsequent preciousblock calls). */ - int32_t nBlockReverseSequenceId = -1; - /** chainwork for the last block that preciousblock has been applied to. */ - arith_uint256 nLastPreciousChainwork = 0; - - /** * The ChainState Mutex * A lock that must be held when modifying this ChainState - held in ActivateBestChain() and * InvalidateBlock() @@ -511,6 +500,9 @@ protected: //! is set to true on the snapshot chainstate. bool m_disabled GUARDED_BY(::cs_main) {false}; + //! Cached result of LookupBlockIndex(*m_from_snapshot_blockhash) + const CBlockIndex* m_cached_snapshot_base GUARDED_BY(::cs_main) {nullptr}; + public: //! Reference to a BlockManager instance which itself is shared across all //! Chainstate instances. @@ -562,9 +554,12 @@ public: */ const std::optional<uint256> m_from_snapshot_blockhash; - //! Return true if this chainstate relies on blocks that are assumed-valid. In - //! practice this means it was created based on a UTXO snapshot. - bool reliesOnAssumedValid() { return m_from_snapshot_blockhash.has_value(); } + /** + * The base of the snapshot this chainstate was created from. + * + * nullptr if this chainstate was not created from a snapshot. + */ + const CBlockIndex* SnapshotBase() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** * The set of all CBlockIndex entries with either BLOCK_VALID_TRANSACTIONS (for @@ -620,37 +615,6 @@ public: bool ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** - * Import blocks from an external file - * - * During reindexing, this function is called for each block file (datadir/blocks/blk?????.dat). - * It reads all blocks contained in the given file and attempts to process them (add them to the - * block index). The blocks may be out of order within each file and across files. Often this - * function reads a block but finds that its parent hasn't been read yet, so the block can't be - * processed yet. The function will add an entry to the blocks_with_unknown_parent map (which is - * passed as an argument), so that when the block's parent is later read and processed, this - * function can re-read the child block from disk and process it. - * - * Because a block's parent may be in a later file, not just later in the same file, the - * blocks_with_unknown_parent map must be passed in and out with each call. It's a multimap, - * rather than just a map, because multiple blocks may have the same parent (when chain splits - * or stale blocks exist). It maps from parent-hash to child-disk-position. - * - * This function can also be used to read blocks from user-specified block files using the - * -loadblock= option. There's no unknown-parent tracking, so the last two arguments are omitted. - * - * - * @param[in] fileIn FILE handle to file containing blocks to read - * @param[in] dbp (optional) Disk block position (only for reindex) - * @param[in,out] blocks_with_unknown_parent (optional) Map of disk positions for blocks with - * unknown parent, key is parent block hash - * (only used for reindex) - * */ - void LoadExternalBlockFile( - FILE* fileIn, - FlatFilePos* dbp = nullptr, - std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr) - EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex); /** * Update the on-disk chain state. @@ -702,8 +666,6 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex) LOCKS_EXCLUDED(::cs_main); - bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - // Block (dis)connection on a given view: DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -738,9 +700,11 @@ public: /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */ bool LoadGenesisBlock(); + void TryAddBlockIndexCandidate(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void PruneBlockIndexCandidates(); - void UnloadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void ClearBlockIndexCandidates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload() const; @@ -748,13 +712,6 @@ public: /** Find the last common block of this chain and a locator. */ const CBlockIndex* FindForkInGlobalIndex(const CBlockLocator& locator) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** - * Make various assertions about the state of the block index. - * - * By default this only executes fully when using the Regtest chain; see: m_options.check_block_index. - */ - void CheckBlockIndex(); - /** Load the persisted mempool from disk */ void LoadMempool(const fs::path& load_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); @@ -784,7 +741,6 @@ private: void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -971,6 +927,13 @@ public: kernel::Notifications& GetNotifications() const { return m_options.notifications; }; /** + * Make various assertions about the state of the block index. + * + * By default this only executes fully when using the Regtest chain; see: m_options.check_block_index. + */ + void CheckBlockIndex(); + + /** * Alias for ::cs_main. * Should be used in new code to make it easier to make ::cs_main a member * of this class. @@ -991,6 +954,27 @@ public: node::BlockManager m_blockman; /** + * Every received block is assigned a unique and increasing identifier, so we + * know which one to give priority in case of a fork. + */ + /** Blocks loaded from disk are assigned id 0, so start the counter at 1. */ + int32_t nBlockSequenceId GUARDED_BY(::cs_main) = 1; + /** Decreasing counter (used by subsequent preciousblock calls). */ + int32_t nBlockReverseSequenceId = -1; + /** chainwork for the last block that preciousblock has been applied to. */ + arith_uint256 nLastPreciousChainwork = 0; + + // Reset the memory-only sequence counters we use to track block arrival + // (used by tests to reset state) + void ResetBlockSequenceCounters() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) + { + AssertLockHeld(::cs_main); + nBlockSequenceId = 1; + nBlockReverseSequenceId = -1; + } + + + /** * In order to efficiently track invalidity of headers, we keep the set of * blocks which we tried to connect and found to be invalid here (ie which * were set to BLOCK_FAILED_VALID since the last restart). We can then @@ -1086,6 +1070,37 @@ public: } /** + * Import blocks from an external file + * + * During reindexing, this function is called for each block file (datadir/blocks/blk?????.dat). + * It reads all blocks contained in the given file and attempts to process them (add them to the + * block index). The blocks may be out of order within each file and across files. Often this + * function reads a block but finds that its parent hasn't been read yet, so the block can't be + * processed yet. The function will add an entry to the blocks_with_unknown_parent map (which is + * passed as an argument), so that when the block's parent is later read and processed, this + * function can re-read the child block from disk and process it. + * + * Because a block's parent may be in a later file, not just later in the same file, the + * blocks_with_unknown_parent map must be passed in and out with each call. It's a multimap, + * rather than just a map, because multiple blocks may have the same parent (when chain splits + * or stale blocks exist). It maps from parent-hash to child-disk-position. + * + * This function can also be used to read blocks from user-specified block files using the + * -loadblock= option. There's no unknown-parent tracking, so the last two arguments are omitted. + * + * + * @param[in] fileIn FILE handle to file containing blocks to read + * @param[in] dbp (optional) Disk block position (only for reindex) + * @param[in,out] blocks_with_unknown_parent (optional) Map of disk positions for blocks with + * unknown parent, key is parent block hash + * (only used for reindex) + * */ + void LoadExternalBlockFile( + FILE* fileIn, + FlatFilePos* dbp = nullptr, + std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr); + + /** * Process an incoming block. This only returns after the best known valid * block is made active. Note that it does not, however, guarantee that the * specific block passed to it has been checked for validity! @@ -1125,6 +1140,29 @@ public: bool ProcessNewBlockHeaders(const std::vector<CBlockHeader>& block, bool min_pow_checked, BlockValidationState& state, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); /** + * Sufficiently validate a block for disk storage (and store on disk). + * + * @param[in] pblock The block we want to process. + * @param[in] fRequested Whether we requested this block from a + * peer. + * @param[in] dbp The location on disk, if we are importing + * this block from prior storage. + * @param[in] min_pow_checked True if proof-of-work anti-DoS checks have + * been done by caller for headers chain + * + * @param[out] state The state of the block validation. + * @param[out] ppindex Optional return parameter to get the + * CBlockIndex pointer for this block. + * @param[out] fNewBlock Optional return parameter to indicate if the + * block is new to our storage. + * + * @returns False if the block or header is invalid, or if saving to disk fails (likely a fatal error); true otherwise. + */ + bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock, bool min_pow_checked) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + /** * Try to add a transaction to the memory pool. * * @param[in] tx The transaction to submit for mempool acceptance. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8fa93b97d6..6b2755ea53 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2319,7 +2319,7 @@ OutputType CWallet::TransactionChangeType(const std::optional<OutputType>& chang void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm) { LOCK(cs_wallet); - WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); /* Continued */ + WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py index 76b50a5bfc..740d3b7f0e 100755 --- a/test/functional/feature_abortnode.py +++ b/test/functional/feature_abortnode.py @@ -25,7 +25,7 @@ class AbortNodeTest(BitcoinTestFramework): self.generate(self.nodes[0], 3, sync_fun=self.no_op) # Deleting the undo file will result in reorg failure - (self.nodes[0].chain_path / "blocks" / "rev00000.dat").unlink() + (self.nodes[0].blocks_path / "rev00000.dat").unlink() # Connecting to a node with a more work chain will trigger a reorg # attempt. diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py index 0961f21a40..3b75a06d9e 100755 --- a/test/functional/feature_anchors.py +++ b/test/functional/feature_anchors.py @@ -6,12 +6,15 @@ import os -from test_framework.p2p import P2PInterface +from test_framework.p2p import P2PInterface, P2P_SERVICES +from test_framework.socks5 import Socks5Configuration, Socks5Server +from test_framework.messages import CAddress, hash256 from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import check_node_connections +from test_framework.util import check_node_connections, assert_equal, p2p_port INBOUND_CONNECTIONS = 5 BLOCK_RELAY_CONNECTIONS = 2 +ONION_ADDR = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion:8333" class AnchorsTest(BitcoinTestFramework): @@ -54,7 +57,7 @@ class AnchorsTest(BitcoinTestFramework): else: inbound_nodes_port.append(hex(int(addr_split[1]))[2:]) - self.log.info("Stop node 0") + self.log.debug("Stop node") self.stop_node(0) # It should contain only the block-relay-only addresses @@ -78,12 +81,64 @@ class AnchorsTest(BitcoinTestFramework): tweaked_contents[20:20] = b'1' out_file_handler.write(bytes(tweaked_contents)) - self.log.info("Start node") + self.log.debug("Start node") self.start_node(0) self.log.info("When node starts, check if anchors.dat doesn't exist anymore") assert not os.path.exists(node_anchors_path) + self.log.info("Ensure addrv2 support") + # Use proxies to catch outbound connections to networks with 256-bit addresses + onion_conf = Socks5Configuration() + onion_conf.auth = True + onion_conf.unauth = True + onion_conf.addr = ('127.0.0.1', p2p_port(self.num_nodes)) + onion_conf.keep_alive = True + onion_proxy = Socks5Server(onion_conf) + onion_proxy.start() + self.restart_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"]) + + self.log.info("Add 256-bit-address block-relay-only connections to node") + self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only') + + self.log.debug("Stop node") + with self.nodes[0].assert_debug_log([f"DumpAnchors: Flush 1 outbound block-relay-only peer addresses to anchors.dat"]): + self.stop_node(0) + # Manually close keep_alive proxy connection + onion_proxy.stop() + + self.log.info("Check for addrv2 addresses in anchors.dat") + caddr = CAddress() + caddr.net = CAddress.NET_TORV3 + caddr.ip, port_str = ONION_ADDR.split(":") + caddr.port = int(port_str) + # TorV3 addrv2 serialization: + # time(4) | services(1) | networkID(1) | address length(1) | address(32) + expected_pubkey = caddr.serialize_v2()[7:39].hex() + + # position of services byte of first addr in anchors.dat + # network magic, vector length, version, nTime + services_index = 4 + 1 + 4 + 4 + data = bytes() + with open(node_anchors_path, "rb") as file_handler: + data = file_handler.read() + assert_equal(data[services_index], 0x00) # services == NONE + anchors2 = data.hex() + assert expected_pubkey in anchors2 + + with open(node_anchors_path, "wb") as file_handler: + # Modify service flags for this address even though we never connected to it. + # This is necessary because on restart we will not attempt an anchor connection + # to a host without our required services, even if its address is in the anchors.dat file + new_data = bytearray(data)[:-32] + new_data[services_index] = P2P_SERVICES + new_data_hash = hash256(new_data) + file_handler.write(new_data + new_data_hash) + + self.log.info("Restarting node attempts to reconnect to anchors") + with self.nodes[0].assert_debug_log([f"Trying to make an anchor connection to {ONION_ADDR}"]): + self.start_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"]) + if __name__ == "__main__": AnchorsTest().main() diff --git a/test/functional/feature_dirsymlinks.py b/test/functional/feature_dirsymlinks.py index 288754c04c..96f4aed08a 100755 --- a/test/functional/feature_dirsymlinks.py +++ b/test/functional/feature_dirsymlinks.py @@ -26,7 +26,7 @@ class SymlinkTest(BitcoinTestFramework): self.stop_node(0) rename_and_link( - from_name=self.nodes[0].chain_path / "blocks", + from_name=self.nodes[0].blocks_path, to_name=dir_new_blocks, ) rename_and_link( diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py index c90ccc4936..12d65fde68 100755 --- a/test/functional/feature_loadblock.py +++ b/test/functional/feature_loadblock.py @@ -37,7 +37,7 @@ class LoadblockTest(BitcoinTestFramework): cfg_file = os.path.join(data_dir, "linearize.cfg") bootstrap_file = os.path.join(self.options.tmpdir, "bootstrap.dat") genesis_block = self.nodes[0].getblockhash(0) - blocks_dir = self.nodes[0].chain_path / "blocks" + blocks_dir = self.nodes[0].blocks_path hash_list = tempfile.NamedTemporaryFile(dir=data_dir, mode='w', delete=False, diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index fcbb49d420..83f1c5003c 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -38,7 +38,7 @@ class ReindexTest(BitcoinTestFramework): # In this test environment, blocks will always be in order (since # we're generating them rather than getting them from peers), so to # test out-of-order handling, swap blocks 1 and 2 on disk. - blk0 = self.nodes[0].chain_path / "blocks" / "blk00000.dat" + blk0 = self.nodes[0].blocks_path / "blk00000.dat" with open(blk0, 'r+b') as bf: # Read at least the first few blocks (including genesis) b = bf.read(2000) diff --git a/test/functional/feature_remove_pruned_files_on_startup.py b/test/functional/feature_remove_pruned_files_on_startup.py index a55e08ef1a..c128587949 100755 --- a/test/functional/feature_remove_pruned_files_on_startup.py +++ b/test/functional/feature_remove_pruned_files_on_startup.py @@ -20,10 +20,10 @@ class FeatureRemovePrunedFilesOnStartupTest(BitcoinTestFramework): self.sync_blocks() def run_test(self): - blk0 = self.nodes[0].chain_path / "blocks" / "blk00000.dat" - rev0 = self.nodes[0].chain_path / "blocks" / "rev00000.dat" - blk1 = self.nodes[0].chain_path / "blocks" / "blk00001.dat" - rev1 = self.nodes[0].chain_path / "blocks" / "rev00001.dat" + blk0 = self.nodes[0].blocks_path / "blk00000.dat" + rev0 = self.nodes[0].blocks_path / "rev00000.dat" + blk1 = self.nodes[0].blocks_path / "blk00001.dat" + rev1 = self.nodes[0].blocks_path / "rev00001.dat" self.mine_batches(800) fo1 = os.open(blk0, os.O_RDONLY) fo2 = os.open(rev1, os.O_RDONLY) diff --git a/test/functional/feature_txindex_compatibility.py b/test/functional/feature_txindex_compatibility.py index a5b25cbd71..572e12df13 100755 --- a/test/functional/feature_txindex_compatibility.py +++ b/test/functional/feature_txindex_compatibility.py @@ -11,16 +11,16 @@ import os import shutil from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_raises_rpc_error from test_framework.wallet import MiniWallet class TxindexCompatibilityTest(BitcoinTestFramework): def set_test_params(self): - self.num_nodes = 3 + self.num_nodes = 2 self.extra_args = [ ["-reindex", "-txindex"], [], - [], ] def skip_test_if_missing_module(self): @@ -33,12 +33,10 @@ class TxindexCompatibilityTest(BitcoinTestFramework): versions=[ 160300, # Last release with legacy txindex None, # For MiniWallet, without migration code - 220000, # Last release with migration code (0.17.x - 22.x) ], ) self.start_nodes() self.connect_nodes(0, 1) - self.connect_nodes(1, 2) def run_test(self): mini_wallet = MiniWallet(self.nodes[1]) @@ -47,22 +45,12 @@ class TxindexCompatibilityTest(BitcoinTestFramework): self.generate(self.nodes[1], 1) self.log.info("Check legacy txindex") + assert_raises_rpc_error(-5, "Use -txindex", lambda: self.nodes[1].getrawtransaction(txid=spend_utxo["txid"])) self.nodes[0].getrawtransaction(txid=spend_utxo["txid"]) # Requires -txindex self.stop_nodes() legacy_chain_dir = self.nodes[0].chain_path - self.log.info("Migrate legacy txindex") - migrate_chain_dir = self.nodes[2].chain_path - shutil.rmtree(migrate_chain_dir) - shutil.copytree(legacy_chain_dir, migrate_chain_dir) - with self.nodes[2].assert_debug_log([ - "Upgrading txindex database...", - "txindex is enabled at height 200", - ]): - self.start_node(2, extra_args=["-txindex"]) - self.nodes[2].getrawtransaction(txid=spend_utxo["txid"]) # Requires -txindex - self.log.info("Drop legacy txindex") drop_index_chain_dir = self.nodes[1].chain_path shutil.rmtree(drop_index_chain_dir) @@ -73,16 +61,14 @@ class TxindexCompatibilityTest(BitcoinTestFramework): ) # Build txindex from scratch and check there is no error this time self.start_node(1, extra_args=["-txindex"]) - self.nodes[2].getrawtransaction(txid=spend_utxo["txid"]) # Requires -txindex + self.wait_until(lambda: self.nodes[1].getindexinfo()["txindex"]["synced"] == True) + self.nodes[1].getrawtransaction(txid=spend_utxo["txid"]) # Requires -txindex self.stop_nodes() self.log.info("Check migrated txindex cannot be read by legacy node") err_msg = f": You need to rebuild the database using -reindex to change -txindex.{os.linesep}Please restart with -reindex or -reindex-chainstate to recover." shutil.rmtree(legacy_chain_dir) - shutil.copytree(migrate_chain_dir, legacy_chain_dir) - self.nodes[0].assert_start_raises_init_error(extra_args=["-txindex"], expected_msg=err_msg) - shutil.rmtree(legacy_chain_dir) shutil.copytree(drop_index_chain_dir, legacy_chain_dir) self.nodes[0].assert_start_raises_init_error(extra_args=["-txindex"], expected_msg=err_msg) diff --git a/test/functional/feature_unsupported_utxo_db.py b/test/functional/feature_unsupported_utxo_db.py index 1c8c08d1d8..6acf551216 100755 --- a/test/functional/feature_unsupported_utxo_db.py +++ b/test/functional/feature_unsupported_utxo_db.py @@ -40,9 +40,9 @@ class UnsupportedUtxoDbTest(BitcoinTestFramework): self.log.info("Check init error") legacy_utxos_dir = self.nodes[0].chain_path / "chainstate" - legacy_blocks_dir = self.nodes[0].chain_path / "blocks" + legacy_blocks_dir = self.nodes[0].blocks_path recent_utxos_dir = self.nodes[1].chain_path / "chainstate" - recent_blocks_dir = self.nodes[1].chain_path / "blocks" + recent_blocks_dir = self.nodes[1].blocks_path shutil.copytree(legacy_utxos_dir, recent_utxos_dir) shutil.copytree(legacy_blocks_dir, recent_blocks_dir) self.nodes[1].assert_start_raises_init_error( diff --git a/test/functional/mempool_compatibility.py b/test/functional/mempool_compatibility.py index 3f632d3d56..fd3e219586 100755 --- a/test/functional/mempool_compatibility.py +++ b/test/functional/mempool_compatibility.py @@ -10,8 +10,6 @@ In case we need to break mempool compatibility we can continue to use the test b Previous releases are required by this test, see test/README.md. """ -import os - from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.wallet import ( @@ -23,6 +21,7 @@ from test_framework.wallet import ( class MempoolCompatibilityTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 + self.setup_clean_chain = True def skip_test_if_missing_module(self): self.skip_if_no_previous_releases() @@ -55,9 +54,9 @@ class MempoolCompatibilityTest(BitcoinTestFramework): self.stop_node(1) self.log.info("Move mempool.dat from old to new node") - old_node_mempool = os.path.join(old_node.chain_path, 'mempool.dat') - new_node_mempool = os.path.join(new_node.chain_path, 'mempool.dat') - os.rename(old_node_mempool, new_node_mempool) + old_node_mempool = old_node.chain_path / "mempool.dat" + new_node_mempool = new_node.chain_path / "mempool.dat" + old_node_mempool.rename(new_node_mempool) self.log.info("Start new node and verify mempool contains the tx") self.start_node(1) @@ -70,7 +69,7 @@ class MempoolCompatibilityTest(BitcoinTestFramework): self.stop_node(1) self.log.info("Move mempool.dat from new to old node") - os.rename(new_node_mempool, old_node_mempool) + new_node_mempool.rename(old_node_mempool) self.log.info("Start old node again and verify mempool contains both txs") self.start_node(0, ['-nowallet']) diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py index c370d8fa91..951bf37ae8 100755 --- a/test/functional/mempool_datacarrier.py +++ b/test/functional/mempool_datacarrier.py @@ -22,16 +22,18 @@ from test_framework.wallet import MiniWallet class DataCarrierTest(BitcoinTestFramework): def set_test_params(self): - self.num_nodes = 3 + self.num_nodes = 4 self.extra_args = [ [], ["-datacarrier=0"], - ["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"] + ["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"], + ["-datacarrier=1", f"-datacarriersize=2"], ] - def test_null_data_transaction(self, node: TestNode, data: bytes, success: bool) -> None: + def test_null_data_transaction(self, node: TestNode, data, success: bool) -> None: tx = self.wallet.create_self_transfer(fee_rate=0)["tx"] - tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN, data]))) + data = [] if data is None else [data] + tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN] + data))) tx.vout[0].nValue -= tx.get_vsize() # simply pay 1sat/vbyte fee tx_hex = tx.serialize().hex() @@ -49,6 +51,8 @@ class DataCarrierTest(BitcoinTestFramework): default_size_data = random_bytes(MAX_OP_RETURN_RELAY - 3) too_long_data = random_bytes(MAX_OP_RETURN_RELAY - 2) small_data = random_bytes(MAX_OP_RETURN_RELAY - 4) + one_byte = random_bytes(1) + zero_bytes = random_bytes(0) self.log.info("Testing null data transaction with default -datacarrier and -datacarriersize values.") self.test_null_data_transaction(node=self.nodes[0], data=default_size_data, success=True) @@ -65,6 +69,24 @@ class DataCarrierTest(BitcoinTestFramework): self.log.info("Testing a null data transaction with a size smaller than accepted by -datacarriersize.") self.test_null_data_transaction(node=self.nodes[2], data=small_data, success=True) + self.log.info("Testing a null data transaction with no data.") + self.test_null_data_transaction(node=self.nodes[0], data=None, success=True) + self.test_null_data_transaction(node=self.nodes[1], data=None, success=False) + self.test_null_data_transaction(node=self.nodes[2], data=None, success=True) + self.test_null_data_transaction(node=self.nodes[3], data=None, success=True) + + self.log.info("Testing a null data transaction with zero bytes of data.") + self.test_null_data_transaction(node=self.nodes[0], data=zero_bytes, success=True) + self.test_null_data_transaction(node=self.nodes[1], data=zero_bytes, success=False) + self.test_null_data_transaction(node=self.nodes[2], data=zero_bytes, success=True) + self.test_null_data_transaction(node=self.nodes[3], data=zero_bytes, success=True) + + self.log.info("Testing a null data transaction with one byte of data.") + self.test_null_data_transaction(node=self.nodes[0], data=one_byte, success=True) + self.test_null_data_transaction(node=self.nodes[1], data=one_byte, success=False) + self.test_null_data_transaction(node=self.nodes[2], data=one_byte, success=True) + self.test_null_data_transaction(node=self.nodes[3], data=one_byte, success=False) + if __name__ == '__main__': DataCarrierTest().main() diff --git a/test/functional/p2p_addrv2_relay.py b/test/functional/p2p_addrv2_relay.py index 9ab190871f..f9a8c44be2 100755 --- a/test/functional/p2p_addrv2_relay.py +++ b/test/functional/p2p_addrv2_relay.py @@ -20,19 +20,24 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal I2P_ADDR = "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p" +ONION_ADDR = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion" ADDRS = [] for i in range(10): addr = CAddress() addr.time = int(time.time()) + i + addr.port = 8333 + i addr.nServices = P2P_SERVICES - # Add one I2P address at an arbitrary position. + # Add one I2P and one onion V3 address at an arbitrary position. if i == 5: addr.net = addr.NET_I2P addr.ip = I2P_ADDR + addr.port = 0 + elif i == 8: + addr.net = addr.NET_TORV3 + addr.ip = ONION_ADDR else: addr.ip = f"123.123.123.{i % 256}" - addr.port = 8333 + i ADDRS.append(addr) @@ -52,6 +57,17 @@ class AddrReceiver(P2PInterface): self.wait_until(lambda: "addrv2" in self.last_message) +def calc_addrv2_msg_size(addrs): + size = 1 # vector length byte + for addr in addrs: + size += 4 # time + size += 1 # services, COMPACTSIZE(P2P_SERVICES) + size += 1 # network id + size += 1 # address length byte + size += addr.ADDRV2_ADDRESS_LENGTH[addr.net] # address + size += 2 # port + return size + class AddrTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -71,9 +87,10 @@ class AddrTest(BitcoinTestFramework): self.log.info('Check that addrv2 message content is relayed and added to addrman') addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) msg.addrs = ADDRS + msg_size = calc_addrv2_msg_size(ADDRS) with self.nodes[0].assert_debug_log([ - 'received: addrv2 (159 bytes) peer=0', - 'sending addrv2 (159 bytes) peer=1', + f'received: addrv2 ({msg_size} bytes) peer=0', + f'sending addrv2 ({msg_size} bytes) peer=1', ]): addr_source.send_and_ping(msg) self.nodes[0].setmocktime(int(time.time()) + 30 * 60) diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py index 1c9ad7289b..60b43c32ae 100755 --- a/test/functional/p2p_getaddr_caching.py +++ b/test/functional/p2p_getaddr_caching.py @@ -6,7 +6,6 @@ import time -from test_framework.messages import msg_getaddr from test_framework.p2p import ( P2PInterface, p2p_lock @@ -21,6 +20,7 @@ from test_framework.util import ( MAX_ADDR_TO_SEND = 1000 MAX_PCT_ADDR_TO_SEND = 23 + class AddrReceiver(P2PInterface): def __init__(self): @@ -70,11 +70,8 @@ class AddrTest(BitcoinTestFramework): cur_mock_time = int(time.time()) for i in range(N): addr_receiver_local = self.nodes[0].add_p2p_connection(AddrReceiver()) - addr_receiver_local.send_and_ping(msg_getaddr()) addr_receiver_onion1 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port1) - addr_receiver_onion1.send_and_ping(msg_getaddr()) addr_receiver_onion2 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port2) - addr_receiver_onion2.send_and_ping(msg_getaddr()) # Trigger response cur_mock_time += 5 * 60 @@ -105,11 +102,8 @@ class AddrTest(BitcoinTestFramework): self.log.info('After time passed, see a new response to addr request') addr_receiver_local = self.nodes[0].add_p2p_connection(AddrReceiver()) - addr_receiver_local.send_and_ping(msg_getaddr()) addr_receiver_onion1 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port1) - addr_receiver_onion1.send_and_ping(msg_getaddr()) addr_receiver_onion2 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port2) - addr_receiver_onion2.send_and_ping(msg_getaddr()) # Trigger response cur_mock_time += 5 * 60 @@ -123,5 +117,6 @@ class AddrTest(BitcoinTestFramework): assert set(last_response_on_onion_bind1) != set(addr_receiver_onion1.get_received_addrs()) assert set(last_response_on_onion_bind2) != set(addr_receiver_onion2.get_received_addrs()) + if __name__ == '__main__': AddrTest().main() diff --git a/test/functional/p2p_invalid_locator.py b/test/functional/p2p_invalid_locator.py index 626422370a..32a23532a2 100755 --- a/test/functional/p2p_invalid_locator.py +++ b/test/functional/p2p_invalid_locator.py @@ -32,7 +32,7 @@ class InvalidLocatorTest(BitcoinTestFramework): within_max_peer = node.add_p2p_connection(P2PInterface()) msg.locator.vHave = [int(node.getblockhash(i - 1), 16) for i in range(block_count, block_count - (MAX_LOCATOR_SZ), -1)] within_max_peer.send_message(msg) - if type(msg) == msg_getheaders: + if type(msg) is msg_getheaders: within_max_peer.wait_for_header(node.getbestblockhash()) else: within_max_peer.wait_for_block(int(node.getbestblockhash(), 16)) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 5f2bece733..18a0a0c6cc 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -577,8 +577,8 @@ class BlockchainTest(BitcoinTestFramework): self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data") def move_block_file(old, new): - old_path = self.nodes[0].chain_path / "blocks" / old - new_path = self.nodes[0].chain_path / "blocks" / new + old_path = self.nodes[0].blocks_path / old + new_path = self.nodes[0].blocks_path / new old_path.rename(new_path) # Move instead of deleting so we can restore chain state afterwards diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 4260e95629..2cae602cc2 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -4,13 +4,15 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the generation of UTXO snapshots using `dumptxoutset`. """ +from pathlib import Path from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error - -import hashlib -from pathlib import Path +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + sha256sum_file, +) class DumptxoutsetTest(BitcoinTestFramework): @@ -39,11 +41,10 @@ class DumptxoutsetTest(BitcoinTestFramework): out['base_hash'], '09abf0e7b510f61ca6cf33bab104e9ee99b3528b371d27a2d4b39abb800fba7e') - with open(str(expected_path), 'rb') as f: - digest = hashlib.sha256(f.read()).hexdigest() - # UTXO snapshot hash should be deterministic based on mocked time. - assert_equal( - digest, 'b1bacb602eacf5fbc9a7c2ef6eeb0d229c04e98bdf0c2ea5929012cd0eae3830') + # UTXO snapshot hash should be deterministic based on mocked time. + assert_equal( + sha256sum_file(str(expected_path)).hex(), + 'b1bacb602eacf5fbc9a7c2ef6eeb0d229c04e98bdf0c2ea5929012cd0eae3830') assert_equal( out['txoutset_hash'], '1f7e3befd45dc13ae198dfbb22869a9c5c4196f8e9ef9735831af1288033f890') diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index c4ed4da0f2..b574a370d6 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -327,7 +327,7 @@ class PSBTTest(BitcoinTestFramework): assert_raises_rpc_error(-3, "Invalid amount", self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {param: invalid_value, "add_inputs": True}) # Test fee_rate values that cannot be represented in sat/vB. - for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: assert_raises_rpc_error(-3, "Invalid amount", self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": invalid_value, "add_inputs": True}) @@ -883,6 +883,9 @@ class PSBTTest(BitcoinTestFramework): comb_psbt = self.nodes[0].combinepsbt([psbt, parsed_psbt.to_base64()]) assert_equal(comb_psbt, psbt) + self.log.info("Test walletprocesspsbt raises if an invalid sighashtype is passed") + assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all") + self.log.info("Test decoding PSBT with per-input preimage types") # note that the decodepsbt RPC doesn't check whether preimages and hashes match hash_ripemd160, preimage_ripemd160 = random_bytes(20), random_bytes(50) @@ -982,5 +985,9 @@ class PSBTTest(BitcoinTestFramework): rawtx = self.nodes[2].finalizepsbt(psbt)["hex"] self.nodes[2].sendrawtransaction(rawtx) + self.log.info("Test descriptorprocesspsbt raises if an invalid sighashtype is passed") + assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all") + + if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/rpc_signer.py b/test/functional/rpc_signer.py index 5ba0d35835..488682e959 100755 --- a/test/functional/rpc_signer.py +++ b/test/functional/rpc_signer.py @@ -21,7 +21,7 @@ class RPCSignerTest(BitcoinTestFramework): def mock_signer_path(self): path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'signer.py') if platform.system() == "Windows": - return "py " + path + return "py -3 " + path else: return path diff --git a/test/functional/rpc_signrawtransactionwithkey.py b/test/functional/rpc_signrawtransactionwithkey.py index ac7a86704f..0913f5057e 100755 --- a/test/functional/rpc_signrawtransactionwithkey.py +++ b/test/functional/rpc_signrawtransactionwithkey.py @@ -14,6 +14,7 @@ from test_framework.address import ( from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_raises_rpc_error, find_vout_for_address, ) from test_framework.script_util import ( @@ -33,6 +34,14 @@ from decimal import ( Decimal, ) +INPUTS = [ + # Valid pay-to-pubkey scripts + {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, + 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, + {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0, + 'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'}, +] +OUTPUTS = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1} class SignRawTransactionWithKeyTest(BitcoinTestFramework): def set_test_params(self): @@ -47,6 +56,11 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): txid = self.nodes[0].sendrawtransaction(self.nodes[0].signrawtransactionwithkey(rawtx, [self.nodes[0].get_deterministic_priv_key().key])["hex"], 0) return txid + def assert_signing_completed_successfully(self, signed_tx): + assert 'errors' not in signed_tx + assert 'complete' in signed_tx + assert_equal(signed_tx['complete'], True) + def successful_signing_test(self): """Create and sign a valid raw transaction with one input. @@ -56,25 +70,10 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): 2) No script verification error occurred""" self.log.info("Test valid raw transaction with one input") privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA'] + rawTx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS) + rawTxSigned = self.nodes[0].signrawtransactionwithkey(rawTx, privKeys, INPUTS) - inputs = [ - # Valid pay-to-pubkey scripts - {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, - 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, - {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0, - 'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'}, - ] - - outputs = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1} - - rawTx = self.nodes[0].createrawtransaction(inputs, outputs) - rawTxSigned = self.nodes[0].signrawtransactionwithkey(rawTx, privKeys, inputs) - - # 1) The transaction has a complete set of signatures - assert rawTxSigned['complete'] - - # 2) No script verification error occurred - assert 'errors' not in rawTxSigned + self.assert_signing_completed_successfully(rawTxSigned) def witness_script_test(self): self.log.info("Test signing transaction to P2SH-P2WSH addresses without wallet") @@ -95,9 +94,7 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys spending_tx = self.nodes[0].createrawtransaction([unspent_output], {getnewdestination()[2]: Decimal("49.998")}) spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [unspent_output]) - # Check the signing completed successfully - assert 'complete' in spending_tx_signed - assert_equal(spending_tx_signed['complete'], True) + self.assert_signing_completed_successfully(spending_tx_signed) # Now test with P2PKH and P2PK scripts as the witnessScript for tx_type in ['P2PKH', 'P2PK']: # these tests are order-independent @@ -120,14 +117,19 @@ class SignRawTransactionWithKeyTest(BitcoinTestFramework): # Now create and sign a transaction spending that output on node[0], which doesn't know the scripts or keys spending_tx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], {getnewdestination()[2]: Decimal("9.999")}) spending_tx_signed = self.nodes[0].signrawtransactionwithkey(spending_tx, [embedded_privkey], [{'txid': txid, 'vout': vout, 'scriptPubKey': script_pub_key, 'redeemScript': redeem_script, 'witnessScript': witness_script, 'amount': 10}]) - # Check the signing completed successfully - assert 'complete' in spending_tx_signed - assert_equal(spending_tx_signed['complete'], True) + self.assert_signing_completed_successfully(spending_tx_signed) self.nodes[0].sendrawtransaction(spending_tx_signed['hex']) + def invalid_sighashtype_test(self): + self.log.info("Test signing transaction with invalid sighashtype") + tx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS) + privkeys = [self.nodes[0].get_deterministic_priv_key().key] + assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all") + def run_test(self): self.successful_signing_test() self.witness_script_test() + self.invalid_sighashtype_test() if __name__ == '__main__': diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index a6764365c5..4d635556f4 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -27,6 +27,7 @@ import random import socket import struct import time +import unittest from test_framework.siphash import siphash256 from test_framework.util import assert_equal @@ -77,6 +78,10 @@ def sha256(s): return hashlib.sha256(s).digest() +def sha3(s): + return hashlib.sha3_256(s).digest() + + def hash256(s): return sha256(sha256(s)) @@ -229,16 +234,25 @@ class CAddress: # see https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki NET_IPV4 = 1 + NET_IPV6 = 2 + NET_TORV3 = 4 NET_I2P = 5 + NET_CJDNS = 6 ADDRV2_NET_NAME = { NET_IPV4: "IPv4", - NET_I2P: "I2P" + NET_IPV6: "IPv6", + NET_TORV3: "TorV3", + NET_I2P: "I2P", + NET_CJDNS: "CJDNS" } ADDRV2_ADDRESS_LENGTH = { NET_IPV4: 4, - NET_I2P: 32 + NET_IPV6: 16, + NET_TORV3: 32, + NET_I2P: 32, + NET_CJDNS: 16 } I2P_PAD = "====" @@ -285,7 +299,7 @@ class CAddress: self.nServices = deser_compact_size(f) self.net = struct.unpack("B", f.read(1))[0] - assert self.net in (self.NET_IPV4, self.NET_I2P) + assert self.net in self.ADDRV2_NET_NAME address_length = deser_compact_size(f) assert address_length == self.ADDRV2_ADDRESS_LENGTH[self.net] @@ -293,14 +307,25 @@ class CAddress: addr_bytes = f.read(address_length) if self.net == self.NET_IPV4: self.ip = socket.inet_ntoa(addr_bytes) - else: + elif self.net == self.NET_IPV6: + self.ip = socket.inet_ntop(socket.AF_INET6, addr_bytes) + elif self.net == self.NET_TORV3: + prefix = b".onion checksum" + version = bytes([3]) + checksum = sha3(prefix + addr_bytes + version)[:2] + self.ip = b32encode(addr_bytes + checksum + version).decode("ascii").lower() + ".onion" + elif self.net == self.NET_I2P: self.ip = b32encode(addr_bytes)[0:-len(self.I2P_PAD)].decode("ascii").lower() + ".b32.i2p" + elif self.net == self.NET_CJDNS: + self.ip = socket.inet_ntop(socket.AF_INET6, addr_bytes) + else: + raise Exception(f"Address type not supported") self.port = struct.unpack(">H", f.read(2))[0] def serialize_v2(self): """Serialize in addrv2 format (BIP155)""" - assert self.net in (self.NET_IPV4, self.NET_I2P) + assert self.net in self.ADDRV2_NET_NAME r = b"" r += struct.pack("<I", self.time) r += ser_compact_size(self.nServices) @@ -308,10 +333,20 @@ class CAddress: r += ser_compact_size(self.ADDRV2_ADDRESS_LENGTH[self.net]) if self.net == self.NET_IPV4: r += socket.inet_aton(self.ip) - else: + elif self.net == self.NET_IPV6: + r += socket.inet_pton(socket.AF_INET6, self.ip) + elif self.net == self.NET_TORV3: + sfx = ".onion" + assert self.ip.endswith(sfx) + r += b32decode(self.ip[0:-len(sfx)], True)[0:32] + elif self.net == self.NET_I2P: sfx = ".b32.i2p" assert self.ip.endswith(sfx) r += b32decode(self.ip[0:-len(sfx)] + self.I2P_PAD, True) + elif self.net == self.NET_CJDNS: + r += socket.inet_pton(socket.AF_INET6, self.ip) + else: + raise Exception(f"Address type not supported") r += struct.pack(">H", self.port) return r @@ -1852,3 +1887,19 @@ class msg_sendtxrcncl: def __repr__(self): return "msg_sendtxrcncl(version=%lu, salt=%lu)" %\ (self.version, self.salt) + +class TestFrameworkScript(unittest.TestCase): + def test_addrv2_encode_decode(self): + def check_addrv2(ip, net): + addr = CAddress() + addr.net, addr.ip = net, ip + ser = addr.serialize_v2() + actual = CAddress() + actual.deserialize_v2(BytesIO(ser)) + self.assertEqual(actual, addr) + + check_addrv2("1.65.195.98", CAddress.NET_IPV4) + check_addrv2("2001:41f0::62:6974:636f:696e", CAddress.NET_IPV6) + check_addrv2("2bqghnldu6mcug4pikzprwhtjjnsyederctvci6klcwzepnjd46ikjyd.onion", CAddress.NET_TORV3) + check_addrv2("255fhcp6ajvftnyo7bwz3an3t4a4brhopm3bamyh2iu5r3gnr2rq.b32.i2p", CAddress.NET_I2P) + check_addrv2("fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa", CAddress.NET_CJDNS) diff --git a/test/functional/test_framework/siphash.py b/test/functional/test_framework/siphash.py index 884dbcab46..bd13b2c948 100644 --- a/test/functional/test_framework/siphash.py +++ b/test/functional/test_framework/siphash.py @@ -31,7 +31,7 @@ def siphash_round(v0, v1, v2, v3): def siphash(k0, k1, data): - assert type(data) == bytes + assert type(data) is bytes v0 = 0x736f6d6570736575 ^ k0 v1 = 0x646f72616e646f6d ^ k1 v2 = 0x6c7967656e657261 ^ k0 @@ -61,5 +61,5 @@ def siphash(k0, k1, data): def siphash256(k0, k1, num): - assert type(num) == int + assert type(num) is int return siphash(k0, k1, num.to_bytes(32, 'little')) diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py index 799b1c74b8..0ca06a7396 100644 --- a/test/functional/test_framework/socks5.py +++ b/test/functional/test_framework/socks5.py @@ -40,6 +40,7 @@ class Socks5Configuration(): self.af = socket.AF_INET # Bind address family self.unauth = False # Support unauthenticated self.auth = False # Support authentication + self.keep_alive = False # Do not automatically close connections class Socks5Command(): """Information about an incoming socks5 command.""" @@ -115,13 +116,14 @@ class Socks5Connection(): cmdin = Socks5Command(cmd, atyp, addr, port, username, password) self.serv.queue.put(cmdin) - logger.info('Proxy: %s', cmdin) + logger.debug('Proxy: %s', cmdin) # Fall through to disconnect except Exception as e: logger.exception("socks5 request handling failed.") self.serv.queue.put(e) finally: - self.conn.close() + if not self.serv.keep_alive: + self.conn.close() class Socks5Server(): def __init__(self, conf): @@ -133,6 +135,7 @@ class Socks5Server(): self.running = False self.thread = None self.queue = queue.Queue() # report connections and exceptions to client + self.keep_alive = conf.keep_alive def run(self): while self.running: @@ -157,4 +160,3 @@ class Socks5Server(): s.connect(self.conf.addr) s.close() self.thread.join() - diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 0615a430fd..544a81602e 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -421,6 +421,10 @@ class TestNode(): return self.chain_path / 'debug.log' @property + def blocks_path(self) -> Path: + return self.chain_path / "blocks" + + @property def wallets_path(self) -> Path: return self.chain_path / "wallets" @@ -645,10 +649,11 @@ class TestNode(): p2p_conn.sync_with_ping() # Consistency check that the node received our user agent string. - # Find our connection in getpeerinfo by our address:port, as it is unique. + # Find our connection in getpeerinfo by our address:port and theirs, as this combination is unique. sockname = p2p_conn._transport.get_extra_info("socket").getsockname() our_addr_and_port = f"{sockname[0]}:{sockname[1]}" - info = [peer for peer in self.getpeerinfo() if peer["addr"] == our_addr_and_port] + dst_addr_and_port = f"{p2p_conn.dstaddr}:{p2p_conn.dstport}" + info = [peer for peer in self.getpeerinfo() if peer["addr"] == our_addr_and_port and peer["addrbind"] == dst_addr_and_port] assert_equal(len(info), 1) assert_equal(info[0]["subver"], P2P_SUBVERSION) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 9762476a5d..d93e6fd6da 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -76,6 +76,7 @@ TEST_FRAMEWORK_MODULES = [ "blocktools", "ellswift", "key", + "messages", "muhash", "ripemd160", "script", @@ -783,8 +784,8 @@ def check_script_prefixes(): def check_script_list(*, src_dir, fail_on_warn): """Check scripts directory. - Check that there are no scripts in the functional tests directory which are - not being run by pull-tester.py.""" + Check that all python files in this directory are categorized + as a test script or meta script.""" script_dir = src_dir + '/test/functional/' python_files = set([test_file for test_file in os.listdir(script_dir) if test_file.endswith(".py")]) missed_tests = list(python_files - set(map(lambda x: x.split()[0], ALL_SCRIPTS + NON_SCRIPTS))) diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 327dd43e5a..9d381a2cd2 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test bitcoin-wallet.""" -import hashlib import os import stat import subprocess @@ -13,9 +12,10 @@ import textwrap from collections import OrderedDict from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal - -BUFFER_SIZE = 16 * 1024 +from test_framework.util import ( + assert_equal, + sha256sum_file, +) class ToolWalletTest(BitcoinTestFramework): @@ -53,12 +53,7 @@ class ToolWalletTest(BitcoinTestFramework): assert_equal(p.poll(), 0) def wallet_shasum(self): - h = hashlib.sha1() - mv = memoryview(bytearray(BUFFER_SIZE)) - with open(self.wallet_path, 'rb', buffering=0) as f: - for n in iter(lambda: f.readinto(mv), 0): - h.update(mv[:n]) - return h.hexdigest() + return sha256sum_file(self.wallet_path).hex() def wallet_timestamp(self): return os.path.getmtime(self.wallet_path) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index a1b805c09e..a4eb6ee29a 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -328,7 +328,7 @@ class WalletTest(BitcoinTestFramework): for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]: assert_raises_rpc_error(-3, msg, self.nodes[2].sendmany, amounts={address: 1.0}, fee_rate=invalid_value) # Test fee_rate values that cannot be represented in sat/vB. - for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: assert_raises_rpc_error(-3, msg, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=invalid_value) # Test fee_rate out of range (negative number). assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=-1) @@ -523,7 +523,7 @@ class WalletTest(BitcoinTestFramework): for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]: assert_raises_rpc_error(-3, msg, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=invalid_value) # Test fee_rate values that cannot be represented in sat/vB. - for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: assert_raises_rpc_error(-3, msg, self.nodes[2].sendtoaddress, address=address, amount=10, fee_rate=invalid_value) # Test fee_rate out of range (negative number). assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=-1) diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 4bc01f3035..e69c1829ca 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -141,7 +141,7 @@ class BumpFeeTest(BitcoinTestFramework): for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]: assert_raises_rpc_error(-3, msg, rbf_node.bumpfee, rbfid, fee_rate=invalid_value) # Test fee_rate values that cannot be represented in sat/vB. - for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: assert_raises_rpc_error(-3, msg, rbf_node.bumpfee, rbfid, fee_rate=invalid_value) # Test fee_rate out of range (negative number). assert_raises_rpc_error(-3, "Amount out of range", rbf_node.bumpfee, rbfid, fee_rate=-1) diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index f4b67bae1b..6f563987cc 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -235,9 +235,11 @@ class WalletDescriptorTest(BitcoinTestFramework): self.nodes[0].createwallet(wallet_name="crashme", descriptors=True) self.nodes[0].unloadwallet("crashme") wallet_db = os.path.join(self.nodes[0].wallets_path, "crashme", self.wallet_data_filename) - with sqlite3.connect(wallet_db) as conn: + conn = sqlite3.connect(wallet_db) + with conn: # add "cscript" entry: key type is uint160 (20 bytes), value type is CScript (zero-length here) conn.execute('INSERT INTO main VALUES(?, ?)', (b'\x07cscript' + b'\x00'*20, b'\x00')) + conn.close() assert_raises_rpc_error(-4, "Unexpected legacy entry in descriptor wallet found.", self.nodes[0].loadwallet, "crashme") diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 4778a8e567..e367daae2c 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -832,7 +832,7 @@ class RawTransactionsTest(BitcoinTestFramework): for invalid_value in ["", 0.000000001, 1e-09, 1.111111111, 1111111111111111, "31.999999999999999999999"]: assert_raises_rpc_error(-3, "Invalid amount", node.fundrawtransaction, rawtx, add_inputs=True, **{param: invalid_value}) # Test fee_rate values that cannot be represented in sat/vB. - for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: assert_raises_rpc_error(-3, "Invalid amount", node.fundrawtransaction, rawtx, fee_rate=invalid_value, add_inputs=True) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index d7bb6ab1e7..4728f53be7 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -387,7 +387,7 @@ class WalletSendTest(BitcoinTestFramework): self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg)) self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg)) # Test fee_rate values that cannot be represented in sat/vB. - for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999, "0.0001", "0.00000001", "0.00099999", "31.99999999"]: + for invalid_value in [0.0001, 0.00000001, 0.00099999, 31.99999999]: self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=invalid_value, expect_error=(-3, msg)) self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=invalid_value, expect_error=(-3, msg)) # Test fee_rate out of range (negative number). diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 3e7c613e55..2735ec1706 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -25,21 +25,21 @@ class WalletSignerTest(BitcoinTestFramework): def mock_signer_path(self): path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'signer.py') if platform.system() == "Windows": - return "py " + path + return "py -3 " + path else: return path def mock_invalid_signer_path(self): path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'invalid_signer.py') if platform.system() == "Windows": - return "py " + path + return "py -3 " + path else: return path def mock_multi_signers_path(self): path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'multi_signers.py') if platform.system() == "Windows": - return "py " + path + return "py -3 " + path else: return path diff --git a/test/functional/wallet_signrawtransactionwithwallet.py b/test/functional/wallet_signrawtransactionwithwallet.py index 3d2f41cb83..d560dfdc11 100755 --- a/test/functional/wallet_signrawtransactionwithwallet.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -33,6 +33,10 @@ from decimal import ( getcontext, ) + +RAW_TX = '020000000156b958f78e3f24e0b2f4e4db1255426b0902027cb37e3ddadb52e37c3557dddb0000000000ffffffff01c0a6b929010000001600149a2ee8c77140a053f36018ac8124a6ececc1668a00000000' + + class SignRawTransactionWithWalletTest(BitcoinTestFramework): def add_options(self, parser): self.add_wallet_options(parser) @@ -47,10 +51,12 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): def test_with_lock_outputs(self): self.log.info("Test correct error reporting when trying to sign a locked output") self.nodes[0].encryptwallet("password") + assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].signrawtransactionwithwallet, RAW_TX) + self.nodes[0].walletpassphrase("password", 9999) - rawTx = '020000000156b958f78e3f24e0b2f4e4db1255426b0902027cb37e3ddadb52e37c3557dddb0000000000ffffffff01c0a6b929010000001600149a2ee8c77140a053f36018ac8124a6ececc1668a00000000' - - assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].signrawtransactionwithwallet, rawTx) + def test_with_invalid_sighashtype(self): + self.log.info("Test signrawtransactionwithwallet raises if an invalid sighashtype is passed") + assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all") def script_verification_error_test(self): """Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script. @@ -299,6 +305,7 @@ class SignRawTransactionWithWalletTest(BitcoinTestFramework): self.script_verification_error_test() self.OP_1NEGATE_test() self.test_with_lock_outputs() + self.test_with_invalid_sighashtype() self.test_fully_signed_tx() self.test_signing_with_csv() self.test_signing_with_cltv() diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 43addab2f3..5ac5840ecf 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -77,7 +77,7 @@ def main(): matching_files_filtered = [] for matching_file in matching_files: - if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)', matching_file): + if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)|contrib/devtools/bitcoin-tidy/example_logprintf.cpp', matching_file): matching_files_filtered.append(matching_file) matching_files_filtered.sort() diff --git a/test/lint/lint-include-guards.py b/test/lint/lint-include-guards.py index 5867aae028..48b918e9da 100755 --- a/test/lint/lint-include-guards.py +++ b/test/lint/lint-include-guards.py @@ -17,7 +17,8 @@ from typing import List HEADER_ID_PREFIX = 'BITCOIN_' HEADER_ID_SUFFIX = '_H' -EXCLUDE_FILES_WITH_PREFIX = ['src/crypto/ctaes', +EXCLUDE_FILES_WITH_PREFIX = ['contrib/devtools/bitcoin-tidy', + 'src/crypto/ctaes', 'src/leveldb', 'src/crc32c', 'src/secp256k1', diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index b14caa4855..8e79ba5121 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -15,7 +15,8 @@ import sys from subprocess import check_output, CalledProcessError -EXCLUDED_DIRS = ["src/leveldb/", +EXCLUDED_DIRS = ["contrib/devtools/bitcoin-tidy/", + "src/leveldb/", "src/crc32c/", "src/secp256k1/", "src/minisketch/", diff --git a/test/lint/lint-logs.py b/test/lint/lint-logs.py deleted file mode 100755 index de04a1aeca..0000000000 --- a/test/lint/lint-logs.py +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (c) 2018-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. -# -# Check that all logs are terminated with '\n' -# -# Some logs are continued over multiple lines. They should be explicitly -# commented with /* Continued */ - -import re -import sys - -from subprocess import check_output - - -def main(): - 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)] - - if unterminated_logs != []: - print("All calls to LogPrintf(), LogPrintfCategory(), LogPrint(), LogPrintLevel(), and WalletLogPrintf() should be terminated with \"\\n\".") - print("") - - for line in unterminated_logs: - print(line) - - sys.exit(1) - - -if __name__ == "__main__": - main() diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 68f2bee322..533e2eae51 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -12,20 +12,25 @@ unsigned-integer-overflow:*/include/c++/ unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange unsigned-integer-overflow:leveldb/ unsigned-integer-overflow:minisketch/ +unsigned-integer-overflow:secp256k1/ unsigned-integer-overflow:test/fuzz/crypto_diff_fuzz_chacha20.cpp implicit-integer-sign-change:*/include/boost/ implicit-integer-sign-change:*/include/c++/ implicit-integer-sign-change:*/new_allocator.h implicit-integer-sign-change:crc32c/ implicit-integer-sign-change:minisketch/ +implicit-integer-sign-change:secp256k1/ implicit-signed-integer-truncation:*/include/c++/ implicit-signed-integer-truncation:leveldb/ +implicit-signed-integer-truncation:secp256k1/ implicit-unsigned-integer-truncation:*/include/c++/ implicit-unsigned-integer-truncation:leveldb/ +implicit-unsigned-integer-truncation:secp256k1/ implicit-unsigned-integer-truncation:test/fuzz/crypto_diff_fuzz_chacha20.cpp shift-base:*/include/c++/ shift-base:leveldb/ shift-base:minisketch/ +shift-base:secp256k1/ shift-base:test/fuzz/crypto_diff_fuzz_chacha20.cpp # Unsigned integer overflow occurs when the result of an unsigned integer # computation cannot be represented in its type. Unlike signed integer overflow, |