diff options
88 files changed, 934 insertions, 679 deletions
diff --git a/.appveyor.yml b/.appveyor.yml index 0d026748b5..bf93d7a990 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -24,6 +24,7 @@ install: git pull origin master > $null git -c advice.detachedHead=false checkout $env:VCPKG_COMMIT_ID .\bootstrap-vcpkg.bat > $null + Add-Content "C:\tools\vcpkg\triplets\$env:PLATFORM-windows-static.cmake" "set(VCPKG_BUILD_TYPE release)" cd "$env:APPVEYOR_BUILD_FOLDER" before_build: # Powershell block below is to download and extract the Qt static libraries. The pseudo code is: diff --git a/.cirrus.yml b/.cirrus.yml index 237560fc2e..dcd1323291 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -63,7 +63,7 @@ task: FILE_ENV: "./ci/test/00_setup_env_arm.sh" task: - name: 'Win64 [GOAL: deploy] [unit tests, no gui, no boost::process, no functional tests]' + name: 'Win64 [GOAL: deploy] [unit tests, no gui tests, no boost::process, no functional tests]' << : *GLOBAL_TASK_TEMPLATE container: image: ubuntu:bionic @@ -71,7 +71,16 @@ task: FILE_ENV: "./ci/test/00_setup_env_win64.sh" task: - name: 'x86_64 Linux [GOAL: install] [bionic] [C++17, previous releases, uses qt5 dev package and some depends packages] [unsigned char]' + name: '32-bit + dash [GOAL: install] [CentOS 8] [gui]' + << : *GLOBAL_TASK_TEMPLATE + container: + image: centos:8 + env: + PACKAGE_MANAGER_INSTALL: "yum install -y" + FILE_ENV: "./ci/test/00_setup_env_i686_centos.sh" + +task: + name: 'x86_64 Linux [GOAL: install] [bionic] [previous releases, uses qt5 dev package and some depends packages] [unsigned char]' << : *GLOBAL_TASK_TEMPLATE container: image: ubuntu:bionic @@ -122,7 +131,15 @@ task: FILE_ENV: "./ci/test/00_setup_env_native_multiprocess.sh" task: - name: 'macOS 10.12 [GOAL: deploy] [no functional tests]' + name: 'x86_64 Linux [GOAL: install] [bionic] [no wallet]' + << : *GLOBAL_TASK_TEMPLATE + container: + image: ubuntu:bionic + env: + FILE_ENV: "./ci/test/00_setup_env_native_nowallet.sh" + +task: + name: 'macOS 10.14 [GOAL: deploy] [no functional tests]' << : *GLOBAL_TASK_TEMPLATE container: image: ubuntu:bionic @@ -130,7 +147,7 @@ task: FILE_ENV: "./ci/test/00_setup_env_mac.sh" task: - name: 'macOS 10.14 native [GOAL: install] [GUI] [no depends]' + name: 'macOS 10.15 native [GOAL: install] [GUI] [no depends]' macos_brew_addon_script: - brew install boost libevent berkeley-db4 qt miniupnpc ccache zeromq qrencode sqlite libtool automake pkg-config gnu-getopt << : *GLOBAL_TASK_TEMPLATE diff --git a/.travis.yml b/.travis.yml index 780bc9944f..dd76aaacaf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -64,13 +64,3 @@ jobs: - set -o errexit; source ./ci/lint/05_before_script.sh script: - set -o errexit; source ./ci/lint/06_script.sh - - - stage: test - name: '32-bit + dash [GOAL: install] [CentOS 8] [gui]' - env: >- - FILE_ENV="./ci/test/00_setup_env_i686_centos.sh" - - - stage: test - name: 'x86_64 Linux [GOAL: install] [bionic] [no wallet]' - env: >- - FILE_ENV="./ci/test/00_setup_env_native_nowallet.sh" diff --git a/build-aux/m4/bitcoin_qt.m4 b/build-aux/m4/bitcoin_qt.m4 index 6c7665830b..658dfc5476 100644 --- a/build-aux/m4/bitcoin_qt.m4 +++ b/build-aux/m4/bitcoin_qt.m4 @@ -128,7 +128,6 @@ AC_DEFUN([BITCOIN_QT_CONFIGURE],[ _BITCOIN_QT_CHECK_STATIC_PLUGINS([Q_IMPORT_PLUGIN(QXcbIntegrationPlugin)],[-lqxcb -lxcb-static]) AC_DEFINE(QT_QPA_PLATFORM_XCB, 1, [Define this symbol if the qt platform is xcb]) elif test "x$TARGET_OS" = xdarwin; then - AX_CHECK_LINK_FLAG([[-framework IOKit]],[QT_LIBS="$QT_LIBS -framework IOKit"],[AC_MSG_ERROR(could not iokit framework)]) _BITCOIN_QT_CHECK_STATIC_PLUGINS([Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin)],[-lqcocoa]) AC_DEFINE(QT_QPA_PLATFORM_COCOA, 1, [Define this symbol if the qt platform is cocoa]) elif test "x$TARGET_OS" = xandroid; then @@ -202,7 +201,7 @@ AC_DEFUN([BITCOIN_QT_CONFIGURE],[ *darwin*) BITCOIN_QT_CHECK([ MOC_DEFS="${MOC_DEFS} -DQ_OS_MAC" - base_frameworks="-framework Foundation -framework ApplicationServices -framework AppKit" + base_frameworks="-framework Foundation -framework AppKit" AX_CHECK_LINK_FLAG([[$base_frameworks]],[QT_LIBS="$QT_LIBS $base_frameworks"],[AC_MSG_ERROR(could not find base frameworks)]) ]) ;; diff --git a/ci/test/00_setup_env_i686_centos.sh b/ci/test/00_setup_env_i686_centos.sh index 52cdb75a0c..eaa68b2ded 100644 --- a/ci/test/00_setup_env_i686_centos.sh +++ b/ci/test/00_setup_env_i686_centos.sh @@ -14,3 +14,4 @@ export GOAL="install" export DEP_OPTS="NO_QT=1" # Gui disabled for now to avoid build failures export BITCOIN_CONFIG="--enable-zmq --with-gui=no --enable-reduce-exports --with-boost-process" export CONFIG_SHELL="/bin/dash" +export TEST_RUNNER_ENV="LC_ALL=en_US.UTF-8" diff --git a/ci/test/00_setup_env_mac.sh b/ci/test/00_setup_env_mac.sh index e4450a65ce..4a022f9b39 100644 --- a/ci/test/00_setup_env_mac.sh +++ b/ci/test/00_setup_env_mac.sh @@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_macos_cross export DOCKER_NAME_TAG=ubuntu:18.04 # Check that bionic can cross-compile to macos (bionic is used in the gitian build as well) -export HOST=x86_64-apple-darwin16 +export HOST=x86_64-apple-darwin18 export PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python3-dev python3-setuptools" export XCODE_VERSION=11.3.1 export XCODE_BUILD_ID=11C505 diff --git a/ci/test/00_setup_env_mac_host.sh b/ci/test/00_setup_env_mac_host.sh index 7c25a34cfe..906f51fc1a 100644 --- a/ci/test/00_setup_env_mac_host.sh +++ b/ci/test/00_setup_env_mac_host.sh @@ -6,7 +6,7 @@ export LC_ALL=C.UTF-8 -export HOST=x86_64-apple-darwin16 +export HOST=x86_64-apple-darwin18 export PIP_PACKAGES="zmq" export GOAL="install" export BITCOIN_CONFIG="--with-gui --enable-reduce-exports --enable-werror --with-boost-process" diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index b88ee2b50f..3ce50f816f 100644 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -15,7 +15,7 @@ export BDB_PREFIX="${BASE_ROOT_DIR}/db4" export CONTAINER_NAME="ci_native_msan" export PACKAGES="clang-9 llvm-9 cmake" -export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}' boost_cxxflags='-std=c++11 -fvisibility=hidden -fPIC ${MSAN_AND_LIBCXX_FLAGS}' zeromq_cxxflags='-std=c++11 ${MSAN_AND_LIBCXX_FLAGS}'" +export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}' boost_cxxflags='-std=c++17 -fvisibility=hidden -fPIC ${MSAN_AND_LIBCXX_FLAGS}' zeromq_cxxflags='-std=c++17 ${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" export BITCOIN_CONFIG="--enable-wallet --with-sanitizers=memory --with-asm=no --prefix=${BASE_ROOT_DIR}/depends/x86_64-pc-linux-gnu/ CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}' BDB_LIBS='-L${BDB_PREFIX}/lib -ldb_cxx-4.8' BDB_CFLAGS='-I${BDB_PREFIX}/include'" export USE_MEMORY_SANITIZER="true" diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index cce0c7e49e..f0ed314d19 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -6,9 +6,6 @@ export LC_ALL=C.UTF-8 -if [[ $DOCKER_NAME_TAG == centos* ]]; then - export LC_ALL=en_US.utf8 -fi if [[ $QEMU_USER_CMD == qemu-s390* ]]; then export LC_ALL=C fi diff --git a/ci/test/05_before_script.sh b/ci/test/05_before_script.sh index 42c244c2f5..d7dd5d9dec 100755 --- a/ci/test/05_before_script.sh +++ b/ci/test/05_before_script.sh @@ -40,7 +40,7 @@ if [ -z "$NO_DEPENDS" ]; then # CentOS has problems building the depends if the config shell is not explicitly set # (i.e. for libevent a Makefile with an empty SHELL variable is generated, leading to # an error as the first command is executed) - SHELL_OPTS="CONFIG_SHELL=/bin/bash" + SHELL_OPTS="LC_ALL=en_US.UTF-8 CONFIG_SHELL=/bin/bash" else SHELL_OPTS="CONFIG_SHELL=" fi diff --git a/configure.ac b/configure.ac index a512fce83e..e89d86df8b 100644 --- a/configure.ac +++ b/configure.ac @@ -643,7 +643,7 @@ case $host in bdb_prefix=$($BREW --prefix berkeley-db4 2>/dev/null) qt5_prefix=$($BREW --prefix qt5 2>/dev/null) - if test x$bdb_prefix != x && test "x$BDB_CFLAGS" = "x" && test "x$BDB_LIBS" = "x"; then + if test x$bdb_prefix != x && test "x$BDB_CFLAGS" = "x" && test "x$BDB_LIBS" = "x" && test "$use_bdb" != "no"; then dnl This must precede the call to BITCOIN_FIND_BDB48 below. BDB_CFLAGS="-I$bdb_prefix/include" BDB_LIBS="-L$bdb_prefix/lib -ldb_cxx-4.8" diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml index 852207edbb..4119e88003 100644 --- a/contrib/gitian-descriptors/gitian-osx.yml +++ b/contrib/gitian-descriptors/gitian-osx.yml @@ -37,7 +37,7 @@ script: | set -e -o pipefail WRAP_DIR=$HOME/wrapped - HOSTS="x86_64-apple-darwin16" + HOSTS="x86_64-apple-darwin18" CONFIGFLAGS="--enable-reduce-exports --disable-bench --disable-gui-tests GENISOIMAGE=$WRAP_DIR/genisoimage" FAKETIME_HOST_PROGS="" FAKETIME_PROGS="ar ranlib date dmg genisoimage" diff --git a/depends/README.md b/depends/README.md index 5225a6d5c4..bf8f829848 100644 --- a/depends/README.md +++ b/depends/README.md @@ -25,7 +25,7 @@ Common `host-platform-triplets` for cross compilation are: - `i686-pc-linux-gnu` for Linux 32 bit - `x86_64-pc-linux-gnu` for x86 Linux - `x86_64-w64-mingw32` for Win64 -- `x86_64-apple-darwin16` for macOS +- `x86_64-apple-darwin18` for macOS - `arm-linux-gnueabihf` for Linux ARM 32 bit - `aarch64-linux-gnu` for Linux ARM 64 bit - `powerpc64-linux-gnu` for Linux POWER 64-bit (big endian) diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index 6099fd4c71..e9faeba336 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -1,4 +1,4 @@ -OSX_MIN_VERSION=10.12 +OSX_MIN_VERSION=10.14 OSX_SDK_VERSION=10.15.1 XCODE_VERSION=11.3.1 XCODE_BUILD_ID=11C505 diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index 5953341d9f..9e533cc61b 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -11,7 +11,7 @@ $(package)_config_opts=--disable-shared --enable-cxx --disable-replication --ena $(package)_config_opts_mingw32=--enable-mingw $(package)_config_opts_linux=--with-pic $(package)_cflags+=-Wno-error=implicit-function-declaration -$(package)_cxxflags=-std=c++11 +$(package)_cxxflags=-std=c++17 $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE endef diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index d8bce108b1..ff8a252db9 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -1,50 +1,45 @@ package=boost -$(package)_version=1_70_0 -$(package)_download_path=https://dl.bintray.com/boostorg/release/1.70.0/source/ +$(package)_version=1_71_0 +$(package)_download_path=https://dl.bintray.com/boostorg/release/$(subst _,.,$($(package)_version))/source/ $(package)_file_name=boost_$($(package)_version).tar.bz2 -$(package)_sha256_hash=430ae8354789de4fd19ee52f3b1f739e1fba576f0aded0897c3c2bc00fb38778 -$(package)_patches=unused_var_in_process.patch +$(package)_sha256_hash=d73a8da01e8bf8c7eda40b4c84915071a8c8a0df4a6734537ddde4a8580524ee +$(package)_dependencies=native_b2 define $(package)_set_vars $(package)_config_opts_release=variant=release $(package)_config_opts_debug=variant=debug $(package)_config_opts=--layout=tagged --build-type=complete --user-config=user-config.jam -$(package)_config_opts+=threading=multi link=static -sNO_BZIP2=1 -sNO_ZLIB=1 +$(package)_config_opts+=threading=multi link=static -sNO_COMPRESSION=1 $(package)_config_opts_linux=target-os=linux threadapi=pthread runtime-link=shared $(package)_config_opts_darwin=target-os=darwin runtime-link=shared $(package)_config_opts_mingw32=target-os=windows binary-format=pe threadapi=win32 runtime-link=static -$(package)_config_opts_x86_64_mingw32=address-model=64 -$(package)_config_opts_i686_mingw32=address-model=32 -$(package)_config_opts_i686_linux=address-model=32 architecture=x86 -$(package)_config_opts_i686_android=address-model=32 -$(package)_config_opts_aarch64_android=address-model=64 -$(package)_config_opts_x86_64_android=address-model=64 -$(package)_config_opts_armv7a_android=address-model=32 -$(package)_toolset_$(host_os)=gcc -$(package)_toolset_darwin=clang +$(package)_config_opts_x86_64=architecture=x86 address-model=64 +$(package)_config_opts_i686=architecture=x86 address-model=32 +$(package)_config_opts_aarch64=address-model=64 +$(package)_config_opts_armv7a=address-model=32 ifneq (,$(findstring clang,$($(package)_cxx))) - $(package)_toolset_$(host_os)=clang +$(package)_toolset_$(host_os)=clang +else +$(package)_toolset_$(host_os)=gcc endif -$(package)_archiver_$(host_os)=$($(package)_ar) $(package)_config_libraries=filesystem,system,thread,test -$(package)_cxxflags=-std=c++11 -fvisibility=hidden +$(package)_cxxflags=-std=c++17 -fvisibility=hidden $(package)_cxxflags_linux=-fPIC $(package)_cxxflags_android=-fPIC endef define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/unused_var_in_process.patch && \ - echo "using $($(package)_toolset_$(host_os)) : : $($(package)_cxx) : <cxxflags>\"$($(package)_cxxflags) $($(package)_cppflags)\" <linkflags>\"$($(package)_ldflags)\" <archiver>\"$($(package)_archiver_$(host_os))\" <striper>\"$(host_STRIP)\" <ranlib>\"$(host_RANLIB)\" <rc>\"$(host_WINDRES)\" : ;" > user-config.jam + echo "using $($(package)_toolset_$(host_os)) : : $($(package)_cxx) : <cflags>\"$($(package)_cflags)\" <cxxflags>\"$($(package)_cxxflags)\" <compileflags>\"$($(package)_cppflags)\" <linkflags>\"$($(package)_ldflags)\" <archiver>\"$($(package)_ar)\" <striper>\"$(host_STRIP)\" <ranlib>\"$(host_RANLIB)\" <rc>\"$(host_WINDRES)\" : ;" > user-config.jam endef define $(package)_config_cmds - ./bootstrap.sh --without-icu --with-libraries=$($(package)_config_libraries) --with-toolset=$($(package)_toolset_$(host_os)) + ./bootstrap.sh --without-icu --with-libraries=$($(package)_config_libraries) --with-toolset=$($(package)_toolset_$(host_os)) --with-bjam=b2 endef define $(package)_build_cmds - ./b2 -d2 -j2 -d1 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) toolset=$($(package)_toolset_$(host_os)) stage + b2 -d2 -j2 -d1 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) toolset=$($(package)_toolset_$(host_os)) stage endef define $(package)_stage_cmds - ./b2 -d0 -j4 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) toolset=$($(package)_toolset_$(host_os)) install + b2 -d0 -j4 --prefix=$($(package)_staging_prefix_dir) $($(package)_config_opts) toolset=$($(package)_toolset_$(host_os)) install endef diff --git a/depends/packages/native_b2.mk b/depends/packages/native_b2.mk new file mode 100644 index 0000000000..aaa37cdcfa --- /dev/null +++ b/depends/packages/native_b2.mk @@ -0,0 +1,20 @@ +package=native_b2 +$(package)_version=$(boost_version) +$(package)_download_path=$(boost_download_path) +$(package)_file_name=$(boost_file_name) +$(package)_sha256_hash=$(boost_sha256_hash) +$(package)_build_subdir=tools/build/src/engine +ifneq (,$(findstring clang,$($(package)_cxx))) +$(package)_toolset_$(host_os)=clang +else +$(package)_toolset_$(host_os)=gcc +endif + +define $(package)_build_cmds + CXX="$($(package)_cxx)" CXXFLAGS="$($(package)_cxxflags)" ./build.sh "$($(package)_toolset_$(host_os))" +endef + +define $(package)_stage_cmds + mkdir -p "$($(package)_staging_prefix_dir)"/bin/ && \ + cp b2 "$($(package)_staging_prefix_dir)"/bin/ +endef diff --git a/depends/packages/packages.mk b/depends/packages/packages.mk index ca06b6dd7e..4b4cc7d9ff 100644 --- a/depends/packages/packages.mk +++ b/depends/packages/packages.mk @@ -22,6 +22,8 @@ multiprocess_native_packages = native_libmultiprocess native_capnp darwin_native_packages = native_ds_store native_mac_alias +$(host_arch)_$(host_os)_native_packages += native_b2 + ifneq ($(build_os),darwin) darwin_native_packages += native_cctools native_cdrkit native_libdmg-hfsplus endif diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 6c77dca8d5..dd755efcbc 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -12,7 +12,7 @@ $(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch $(package)_patches+= fix_rcc_determinism.patch fix_riscv64_arch.patch xkb-default.patch no-xlib.patch $(package)_patches+= fix_android_qmake_conf.patch fix_android_jni_static.patch dont_hardcode_pwd.patch $(package)_patches+= freetype_back_compat.patch drop_lrelease_dependency.patch fix_powerpc_libpng.patch -$(package)_patches+= fix_mingw_cross_compile.patch +$(package)_patches+= fix_mingw_cross_compile.patch fix_qpainter_non_determinism.patch # Update OSX_QT_TRANSLATIONS when this is updated $(package)_qttranslations_file_name=qttranslations-$($(package)_suffix) @@ -26,9 +26,10 @@ $(package)_extra_sources += $($(package)_qttools_file_name) define $(package)_set_vars $(package)_config_opts_release = -release +$(package)_config_opts_release += -silent $(package)_config_opts_debug = -debug $(package)_config_opts += -bindir $(build_prefix)/bin -$(package)_config_opts += -c++std c++11 +$(package)_config_opts += -c++std c++1z $(package)_config_opts += -confirm-license $(package)_config_opts += -hostprefix $(build_prefix) $(package)_config_opts += -no-compile-examples @@ -77,7 +78,6 @@ $(package)_config_opts += -qt-pcre $(package)_config_opts += -qt-harfbuzz $(package)_config_opts += -system-zlib $(package)_config_opts += -static -$(package)_config_opts += -silent $(package)_config_opts += -v $(package)_config_opts += -no-feature-bearermanagement $(package)_config_opts += -no-feature-colordialog @@ -231,6 +231,7 @@ define $(package)_preprocess_cmds patch -p1 -i $($(package)_patch_dir)/fix_riscv64_arch.patch && \ patch -p1 -i $($(package)_patch_dir)/no-xlib.patch && \ patch -p1 -i $($(package)_patch_dir)/fix_mingw_cross_compile.patch && \ + patch -p1 -i $($(package)_patch_dir)/fix_qpainter_non_determinism.patch &&\ sed -i.old "s|updateqm.commands = \$$$$\$$$$LRELEASE|updateqm.commands = $($(package)_extract_dir)/qttools/bin/lrelease|" qttranslations/translations/translations.pro && \ mkdir -p qtbase/mkspecs/macx-clang-linux &&\ cp -f qtbase/mkspecs/macx-clang/qplatformdefs.h qtbase/mkspecs/macx-clang-linux/ &&\ diff --git a/depends/packages/zeromq.mk b/depends/packages/zeromq.mk index c93aa1a74d..3b7f3690a4 100644 --- a/depends/packages/zeromq.mk +++ b/depends/packages/zeromq.mk @@ -12,7 +12,7 @@ define $(package)_set_vars $(package)_config_opts += --disable-Werror --disable-drafts --enable-option-checking $(package)_config_opts_linux=--with-pic $(package)_config_opts_android=--with-pic - $(package)_cxxflags=-std=c++11 + $(package)_cxxflags=-std=c++17 endef define $(package)_preprocess_cmds diff --git a/depends/patches/boost/unused_var_in_process.patch b/depends/patches/boost/unused_var_in_process.patch deleted file mode 100644 index 722f7bb5ea..0000000000 --- a/depends/patches/boost/unused_var_in_process.patch +++ /dev/null @@ -1,22 +0,0 @@ -commit dbd95cdaefdea95307d004f019a1c394cf9389f0 -Author: fanquake <fanquake@gmail.com> -Date: Mon Aug 17 20:15:17 2020 +0800 - - Remove unused variable in Boost Process - - This causes issues with our linters / CI. - - Can be removed once depends Boost is 1.71.0 or later. - -diff --git a/boost/process/detail/posix/wait_group.hpp b/boost/process/detail/posix/wait_group.hpp -index 9dc249803..2502d9772 100644 ---- a/boost/process/detail/posix/wait_group.hpp -+++ b/boost/process/detail/posix/wait_group.hpp -@@ -137,7 +137,6 @@ inline bool wait_until( - - do - { -- int ret_sig = 0; - int status; - if ((::waitpid(timeout_pid, &status, WNOHANG) != 0) - && (WIFEXITED(status) || WIFSIGNALED(status))) diff --git a/depends/patches/qt/fix_qpainter_non_determinism.patch b/depends/patches/qt/fix_qpainter_non_determinism.patch new file mode 100644 index 0000000000..3cfcc22f03 --- /dev/null +++ b/depends/patches/qt/fix_qpainter_non_determinism.patch @@ -0,0 +1,63 @@ +commit 2a8f7dc6ddfc414a66491522501c1574a1343ee1 +Author: Andrew Chow <achow101-github@achow101.com> +Date: Sat Nov 21 01:11:04 2020 -0500 + + build: Fix determinism issue when building with Clang 8 + + When building Qt with LLVM/Clang 8 under -O3 (the default), we run into + a determinism issue in `qt_interset_spans`. The issue has been fixed for + LLVM/Clang 9, see + https://github.com/llvm/llvm-project/commit/db101864bdc938deb1d63fe4f7da761bd38e5cae + and https://reviews.llvm.org/D64601, however this fix was not backported + to 8.x. Once LLVM/Clang 9 is used, this patch can be dropped. + + The particular issue appears to be an optimization done by -O3 which + adds a temporary variable for `spans->y` in `qt_intersect_spans`. When + it does this, sometimes it chooses to use a 32-bit movs instruction + (movswl), and other times it chooses a 64-bit movs instruction (movswq). + By patching `qt_intersect_spans` to always make a temporary variable for + `spans->y`, we are able to sidestep this problem. + +diff --git a/qtbase/src/gui/painting/qpaintengine_raster.cpp b/qtbase/src/gui/painting/qpaintengine_raster.cpp +index 92ab6e8375..f018009e0b 100644 +--- a/qtbase/src/gui/painting/qpaintengine_raster.cpp ++++ b/qtbase/src/gui/painting/qpaintengine_raster.cpp +@@ -3971,22 +3971,23 @@ static const QSpan *qt_intersect_spans(const QClipData *clip, int *currentClip, + const QSpan *clipEnd = clip->m_spans + clip->count; + + while (available && spans < end ) { ++ const short spans_y = spans->y; + if (clipSpans >= clipEnd) { + spans = end; + break; + } +- if (clipSpans->y > spans->y) { ++ if (clipSpans->y > spans_y) { + ++spans; + continue; + } +- if (spans->y != clipSpans->y) { +- if (spans->y < clip->count && clip->m_clipLines[spans->y].spans) +- clipSpans = clip->m_clipLines[spans->y].spans; ++ if (spans_y != clipSpans->y) { ++ if (spans_y < clip->count && clip->m_clipLines[spans_y].spans) ++ clipSpans = clip->m_clipLines[spans_y].spans; + else + ++clipSpans; + continue; + } +- Q_ASSERT(spans->y == clipSpans->y); ++ Q_ASSERT(spans_y == clipSpans->y); + + int sx1 = spans->x; + int sx2 = sx1 + spans->len; +@@ -4005,7 +4006,7 @@ static const QSpan *qt_intersect_spans(const QClipData *clip, int *currentClip, + if (len) { + out->x = qMax(sx1, cx1); + out->len = qMin(sx2, cx2) - out->x; +- out->y = spans->y; ++ out->y = spans_y; + out->coverage = qt_div_255(spans->coverage * clipSpans->coverage); + ++out; + --available; + diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index 2b051c078c..dccd7b1335 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -15,6 +15,7 @@ pkg_add qt5 # (optional for enabling the GUI) pkg_add autoconf # (select highest version, e.g. 2.69) pkg_add automake # (select highest version, e.g. 1.16) pkg_add python # (select highest version, e.g. 3.8) +pkg_add bash git clone https://github.com/bitcoin/bitcoin.git ``` diff --git a/doc/build-osx.md b/doc/build-osx.md index 2a7d71eea6..0a091f6afd 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -111,6 +111,6 @@ tail -f $HOME/Library/Application\ Support/Bitcoin/debug.log ``` ## Notes -* Tested on OS X 10.12 Sierra through macOS 10.15 Catalina on 64-bit Intel +* Tested on OS X 10.14 Mojave through macOS 11 Big Sur on 64-bit Intel processors only. * Building with downloaded Qt binaries is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714). diff --git a/doc/dependencies.md b/doc/dependencies.md index 4df7c761da..76e8910871 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -6,7 +6,7 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | Dependency | Version used | Minimum required | CVEs | Shared | [Bundled Qt library](https://doc.qt.io/qt-5/configure-options.html#third-party-libraries) | | --- | --- | --- | --- | --- | --- | | Berkeley DB | [4.8.30](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.x | No | | | -| Boost | [1.70.0](https://www.boost.org/users/download/) | [1.58.0](https://github.com/bitcoin/bitcoin/pull/19667) | No | | | +| Boost | [1.71.0](https://www.boost.org/users/download/) | [1.58.0](https://github.com/bitcoin/bitcoin/pull/19667) | No | | | | Clang | | [5.0+](https://releases.llvm.org/download.html) (C++17 support) | | | | | Expat | [2.2.7](https://libexpat.github.io/) | | No | Yes | | | fontconfig | [2.12.1](https://www.freedesktop.org/software/fontconfig/release/) | | No | Yes | | diff --git a/doc/release-notes.md b/doc/release-notes.md index 10d859395c..f286a4493b 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -46,12 +46,12 @@ Compatibility ============== Bitcoin Core is supported and extensively tested on operating systems -using the Linux kernel, macOS 10.12+, and Windows 7 and newer. Bitcoin +using the Linux kernel, macOS 10.14+, and Windows 7 and newer. Bitcoin Core should also work on most other Unix-like systems but is not as frequently tested on them. It is not recommended to use Bitcoin Core on unsupported systems. -From Bitcoin Core 0.20.0 onwards, macOS versions earlier than 10.12 are no +From Bitcoin Core 0.22.0 onwards, macOS versions earlier than 10.14 are no longer supported. Additionally, Bitcoin Core does not yet change appearance when macOS "dark mode" is activated. diff --git a/share/qt/Info.plist.in b/share/qt/Info.plist.in index 8c0eb22540..207d60aca3 100644 --- a/share/qt/Info.plist.in +++ b/share/qt/Info.plist.in @@ -3,7 +3,7 @@ <plist version="0.9"> <dict> <key>LSMinimumSystemVersion</key> - <string>10.12.0</string> + <string>10.14.0</string> <key>LSArchitecturePriority</key> <array> @@ -17,7 +17,7 @@ <string>APPL</string> <key>NSHumanReadableCopyright</key> - <string>@CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@.@CLIENT_VERSION_BUILD@, Copyright © 2009-@COPYRIGHT_YEAR@ @COPYRIGHT_HOLDERS_FINAL@</string> + <string>@CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@CLIENT_VERSION_BUILD@, Copyright © 2009-@COPYRIGHT_YEAR@ @COPYRIGHT_HOLDERS_FINAL@</string> <key>CFBundleShortVersionString</key> <string>@CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@CLIENT_VERSION_BUILD@</string> diff --git a/share/setup.nsi.in b/share/setup.nsi.in index 1c4dcf3842..681f243d04 100644 --- a/share/setup.nsi.in +++ b/share/setup.nsi.in @@ -56,7 +56,7 @@ CRCCheck on XPStyle on BrandingText " " ShowInstDetails show -VIProductVersion @CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@.@CLIENT_VERSION_BUILD@ +VIProductVersion @CLIENT_VERSION_MAJOR@.@CLIENT_VERSION_MINOR@.@CLIENT_VERSION_BUILD@.0 VIAddVersionKey ProductName "@PACKAGE_NAME@" VIAddVersionKey ProductVersion "@PACKAGE_VERSION@" VIAddVersionKey CompanyName "${COMPANY}" diff --git a/src/Makefile.am b/src/Makefile.am index 0409faee1e..4a080ef1fb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,9 +20,8 @@ else LIBUNIVALUE = $(UNIVALUE_LIBS) endif -BITCOIN_INCLUDES=-I$(builddir) $(BDB_CPPFLAGS) $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) +BITCOIN_INCLUDES=-I$(builddir) -I$(srcdir)/secp256k1/include $(BDB_CPPFLAGS) $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) -BITCOIN_INCLUDES += -I$(srcdir)/secp256k1/include BITCOIN_INCLUDES += $(UNIVALUE_CFLAGS) LIBBITCOIN_SERVER=libbitcoin_server.a @@ -299,14 +298,13 @@ libbitcoin_server_a_SOURCES = \ index/blockfilterindex.cpp \ index/txindex.cpp \ init.cpp \ - interfaces/chain.cpp \ - interfaces/node.cpp \ miner.cpp \ net.cpp \ net_processing.cpp \ node/coin.cpp \ node/coinstats.cpp \ node/context.cpp \ + node/interfaces.cpp \ node/psbt.cpp \ node/transaction.cpp \ node/ui_interface.cpp \ @@ -359,13 +357,13 @@ endif libbitcoin_wallet_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(SQLITE_CFLAGS) libbitcoin_wallet_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_wallet_a_SOURCES = \ - interfaces/wallet.cpp \ wallet/coincontrol.cpp \ wallet/context.cpp \ wallet/crypter.cpp \ wallet/db.cpp \ wallet/feebumper.cpp \ wallet/fees.cpp \ + wallet/interfaces.cpp \ wallet/load.cpp \ wallet/rpcdump.cpp \ wallet/rpcwallet.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 87166ecb79..f4c726b0b2 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -1105,7 +1105,7 @@ test_fuzz_script_interpreter_SOURCES = test/fuzz/script_interpreter.cpp test_fuzz_script_assets_test_minimizer_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_script_assets_test_minimizer_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_script_assets_test_minimizer_LDADD = $(FUZZ_SUITE_LD_COMMON) -test_fuzz_script_assets_test_minimizer_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_assets_test_minimizer_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON) test_fuzz_script_assets_test_minimizer_SOURCES = test/fuzz/script_assets_test_minimizer.cpp test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) diff --git a/src/attributes.h b/src/attributes.h index 45099bd8b8..995c24e13f 100644 --- a/src/attributes.h +++ b/src/attributes.h @@ -6,17 +6,14 @@ #ifndef BITCOIN_ATTRIBUTES_H #define BITCOIN_ATTRIBUTES_H -#if defined(__has_cpp_attribute) -# if __has_cpp_attribute(nodiscard) -# define NODISCARD [[nodiscard]] -# endif -#endif -#ifndef NODISCARD -# if defined(_MSC_VER) && _MSC_VER >= 1700 -# define NODISCARD _Check_return_ +#if defined(__clang__) +# if __has_attribute(lifetimebound) +# define LIFETIMEBOUND [[clang::lifetimebound]] # else -# define NODISCARD __attribute__((warn_unused_result)) +# define LIFETIMEBOUND # endif +#else +# define LIFETIMEBOUND #endif #endif // BITCOIN_ATTRIBUTES_H diff --git a/src/base58.cpp b/src/base58.cpp index 0dc6044145..780846c6c5 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -35,7 +35,7 @@ static const int8_t mapBase58[256] = { -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, }; -NODISCARD static bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch, int max_ret_len) +[[nodiscard]] static bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch, int max_ret_len) { // Skip leading spaces. while (*psz && IsSpace(*psz)) @@ -141,7 +141,7 @@ std::string EncodeBase58Check(Span<const unsigned char> input) return EncodeBase58(vch); } -NODISCARD static bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len) +[[nodiscard]] static bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len) { if (!DecodeBase58(psz, vchRet, max_ret_len > std::numeric_limits<int>::max() - 4 ? std::numeric_limits<int>::max() : max_ret_len + 4) || (vchRet.size() < 4)) { diff --git a/src/base58.h b/src/base58.h index 468c3e2589..60551a12ae 100644 --- a/src/base58.h +++ b/src/base58.h @@ -29,7 +29,7 @@ std::string EncodeBase58(Span<const unsigned char> input); * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ -NODISCARD bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len); +[[nodiscard]] bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len); /** * Encode a byte span into a base58-encoded string, including checksum @@ -40,6 +40,6 @@ std::string EncodeBase58Check(Span<const unsigned char> input); * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -NODISCARD bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len); +[[nodiscard]] bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len); #endif // BITCOIN_BASE58_H diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp index a2dbefa54a..73af244ce0 100644 --- a/src/bench/prevector.cpp +++ b/src/bench/prevector.cpp @@ -9,24 +9,16 @@ #include <bench/bench.h> -// GCC 4.8 is missing some C++11 type_traits, -// https://www.gnu.org/software/gcc/gcc-5/changes.html -#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 5 -#define IS_TRIVIALLY_CONSTRUCTIBLE std::has_trivial_default_constructor -#else -#define IS_TRIVIALLY_CONSTRUCTIBLE std::is_trivially_default_constructible -#endif - struct nontrivial_t { int x; nontrivial_t() :x(-1) {} SERIALIZE_METHODS(nontrivial_t, obj) { READWRITE(obj.x); } }; -static_assert(!IS_TRIVIALLY_CONSTRUCTIBLE<nontrivial_t>::value, +static_assert(!std::is_trivially_default_constructible<nontrivial_t>::value, "expected nontrivial_t to not be trivially constructible"); typedef unsigned char trivial_t; -static_assert(IS_TRIVIALLY_CONSTRUCTIBLE<trivial_t>::value, +static_assert(std::is_trivially_default_constructible<trivial_t>::value, "expected trivial_t to be trivially constructible"); template <typename T> diff --git a/src/core_io.h b/src/core_io.h index 80ec80cd50..aaee9c445d 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -22,8 +22,8 @@ class UniValue; // core_read.cpp CScript ParseScript(const std::string& s); std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false); -NODISCARD bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true); -NODISCARD bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); +[[nodiscard]] bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true); +[[nodiscard]] bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); /** diff --git a/src/hash.h b/src/hash.h index 6d876076ee..083ac12523 100644 --- a/src/hash.h +++ b/src/hash.h @@ -197,7 +197,7 @@ uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL } /** Single-SHA256 a 32-byte input (represented as uint256). */ -NODISCARD uint256 SHA256Uint256(const uint256& input); +[[nodiscard]] uint256 SHA256Uint256(const uint256& input); unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vDataToHash); diff --git a/src/init.cpp b/src/init.cpp index 1e41616e94..9137050323 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -113,7 +113,7 @@ static fs::path GetPidFile(const ArgsManager& args) return AbsPathForConfigVal(fs::path(args.GetArg("-pid", BITCOIN_PID_FILENAME))); } -NODISCARD static bool CreatePidFile(const ArgsManager& args) +[[nodiscard]] static bool CreatePidFile(const ArgsManager& args) { fsbridge::ofstream file{GetPidFile(args)}; if (file) { @@ -1389,16 +1389,9 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA assert(!node.connman); node.connman = MakeUnique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()), args.GetBoolArg("-networkactive", true)); - // Make mempool generally available in the node context. For example the connection manager, wallet, or RPC threads, - // which are all started after this, may use it from the node context. assert(!node.mempool); - node.mempool = MakeUnique<CTxMemPool>(&::feeEstimator); - if (node.mempool) { - int ratio = std::min<int>(std::max<int>(args.GetArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000); - if (ratio != 0) { - node.mempool->setSanityCheck(1.0 / ratio); - } - } + int check_ratio = std::min<int>(std::max<int>(args.GetArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000); + node.mempool = MakeUnique<CTxMemPool>(&::feeEstimator, check_ratio); assert(!node.chainman); node.chainman = &g_chainman; diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp deleted file mode 100644 index 2c5f8627e6..0000000000 --- a/src/interfaces/node.cpp +++ /dev/null @@ -1,303 +0,0 @@ -// Copyright (c) 2018-2020 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include <interfaces/node.h> - -#include <addrdb.h> -#include <banman.h> -#include <chain.h> -#include <chainparams.h> -#include <init.h> -#include <interfaces/chain.h> -#include <interfaces/handler.h> -#include <interfaces/wallet.h> -#include <net.h> -#include <net_processing.h> -#include <netaddress.h> -#include <netbase.h> -#include <node/context.h> -#include <node/ui_interface.h> -#include <policy/feerate.h> -#include <policy/fees.h> -#include <policy/settings.h> -#include <primitives/block.h> -#include <rpc/server.h> -#include <shutdown.h> -#include <support/allocators/secure.h> -#include <sync.h> -#include <txmempool.h> -#include <util/check.h> -#include <util/ref.h> -#include <util/system.h> -#include <util/translation.h> -#include <validation.h> -#include <warnings.h> - -#if defined(HAVE_CONFIG_H) -#include <config/bitcoin-config.h> -#endif - -#include <univalue.h> - -#include <boost/signals2/signal.hpp> - -namespace interfaces { -namespace { - -class NodeImpl : public Node -{ -public: - NodeImpl(NodeContext* context) { setContext(context); } - void initLogging() override { InitLogging(*Assert(m_context->args)); } - void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); } - bilingual_str getWarnings() override { return GetWarnings(true); } - uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); } - bool baseInitialize() override - { - return AppInitBasicSetup(gArgs) && AppInitParameterInteraction(gArgs) && AppInitSanityChecks() && - AppInitLockDataDirectory() && AppInitInterfaces(*m_context); - } - bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override - { - return AppInitMain(m_context_ref, *m_context, tip_info); - } - void appShutdown() override - { - Interrupt(*m_context); - Shutdown(*m_context); - } - void startShutdown() override - { - StartShutdown(); - // Stop RPC for clean shutdown if any of waitfor* commands is executed. - if (gArgs.GetBoolArg("-server", false)) { - InterruptRPC(); - StopRPC(); - } - } - bool shutdownRequested() override { return ShutdownRequested(); } - void mapPort(bool use_upnp) override - { - if (use_upnp) { - StartMapPort(); - } else { - InterruptMapPort(); - StopMapPort(); - } - } - bool getProxy(Network net, proxyType& proxy_info) override { return GetProxy(net, proxy_info); } - size_t getNodeCount(CConnman::NumConnections flags) override - { - return m_context->connman ? m_context->connman->GetNodeCount(flags) : 0; - } - bool getNodesStats(NodesStats& stats) override - { - stats.clear(); - - if (m_context->connman) { - std::vector<CNodeStats> stats_temp; - m_context->connman->GetNodeStats(stats_temp); - - stats.reserve(stats_temp.size()); - for (auto& node_stats_temp : stats_temp) { - stats.emplace_back(std::move(node_stats_temp), false, CNodeStateStats()); - } - - // Try to retrieve the CNodeStateStats for each node. - TRY_LOCK(::cs_main, lockMain); - if (lockMain) { - for (auto& node_stats : stats) { - std::get<1>(node_stats) = - GetNodeStateStats(std::get<0>(node_stats).nodeid, std::get<2>(node_stats)); - } - } - return true; - } - return false; - } - bool getBanned(banmap_t& banmap) override - { - if (m_context->banman) { - m_context->banman->GetBanned(banmap); - return true; - } - return false; - } - bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override - { - if (m_context->banman) { - m_context->banman->Ban(net_addr, ban_time_offset); - return true; - } - return false; - } - bool unban(const CSubNet& ip) override - { - if (m_context->banman) { - m_context->banman->Unban(ip); - return true; - } - return false; - } - bool disconnectByAddress(const CNetAddr& net_addr) override - { - if (m_context->connman) { - return m_context->connman->DisconnectNode(net_addr); - } - return false; - } - bool disconnectById(NodeId id) override - { - if (m_context->connman) { - return m_context->connman->DisconnectNode(id); - } - return false; - } - int64_t getTotalBytesRecv() override { return m_context->connman ? m_context->connman->GetTotalBytesRecv() : 0; } - int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; } - size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; } - size_t getMempoolDynamicUsage() override { return m_context->mempool ? m_context->mempool->DynamicMemoryUsage() : 0; } - bool getHeaderTip(int& height, int64_t& block_time) override - { - LOCK(::cs_main); - if (::pindexBestHeader) { - height = ::pindexBestHeader->nHeight; - block_time = ::pindexBestHeader->GetBlockTime(); - return true; - } - return false; - } - int getNumBlocks() override - { - LOCK(::cs_main); - return ::ChainActive().Height(); - } - uint256 getBestBlockHash() override - { - const CBlockIndex* tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip()); - return tip ? tip->GetBlockHash() : Params().GenesisBlock().GetHash(); - } - int64_t getLastBlockTime() override - { - LOCK(::cs_main); - if (::ChainActive().Tip()) { - return ::ChainActive().Tip()->GetBlockTime(); - } - return Params().GenesisBlock().GetBlockTime(); // Genesis block's time of current network - } - double getVerificationProgress() override - { - const CBlockIndex* tip; - { - LOCK(::cs_main); - tip = ::ChainActive().Tip(); - } - return GuessVerificationProgress(Params().TxData(), tip); - } - bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); } - bool getReindex() override { return ::fReindex; } - bool getImporting() override { return ::fImporting; } - void setNetworkActive(bool active) override - { - if (m_context->connman) { - m_context->connman->SetNetworkActive(active); - } - } - bool getNetworkActive() override { return m_context->connman && m_context->connman->GetNetworkActive(); } - CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) override - { - FeeCalculation fee_calc; - CFeeRate result = ::feeEstimator.estimateSmartFee(num_blocks, &fee_calc, conservative); - if (returned_target) { - *returned_target = fee_calc.returnedTarget; - } - return result; - } - CFeeRate getDustRelayFee() override { return ::dustRelayFee; } - UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override - { - JSONRPCRequest req(m_context_ref); - req.params = params; - req.strMethod = command; - req.URI = uri; - return ::tableRPC.execute(req); - } - std::vector<std::string> listRpcCommands() override { return ::tableRPC.listCommands(); } - void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); } - void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); } - bool getUnspentOutput(const COutPoint& output, Coin& coin) override - { - LOCK(::cs_main); - return ::ChainstateActive().CoinsTip().GetCoin(output, coin); - } - WalletClient& walletClient() override - { - return *Assert(m_context->wallet_client); - } - std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override - { - return MakeHandler(::uiInterface.InitMessage_connect(fn)); - } - std::unique_ptr<Handler> handleMessageBox(MessageBoxFn fn) override - { - return MakeHandler(::uiInterface.ThreadSafeMessageBox_connect(fn)); - } - std::unique_ptr<Handler> handleQuestion(QuestionFn fn) override - { - return MakeHandler(::uiInterface.ThreadSafeQuestion_connect(fn)); - } - std::unique_ptr<Handler> handleShowProgress(ShowProgressFn fn) override - { - return MakeHandler(::uiInterface.ShowProgress_connect(fn)); - } - std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override - { - return MakeHandler(::uiInterface.NotifyNumConnectionsChanged_connect(fn)); - } - std::unique_ptr<Handler> handleNotifyNetworkActiveChanged(NotifyNetworkActiveChangedFn fn) override - { - return MakeHandler(::uiInterface.NotifyNetworkActiveChanged_connect(fn)); - } - std::unique_ptr<Handler> handleNotifyAlertChanged(NotifyAlertChangedFn fn) override - { - return MakeHandler(::uiInterface.NotifyAlertChanged_connect(fn)); - } - std::unique_ptr<Handler> handleBannedListChanged(BannedListChangedFn fn) override - { - return MakeHandler(::uiInterface.BannedListChanged_connect(fn)); - } - std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override - { - return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) { - fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, - GuessVerificationProgress(Params().TxData(), block)); - })); - } - std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) override - { - return MakeHandler( - ::uiInterface.NotifyHeaderTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) { - fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, - /* verification progress is unused when a header was received */ 0); - })); - } - NodeContext* context() override { return m_context; } - void setContext(NodeContext* context) override - { - m_context = context; - if (context) { - m_context_ref.Set(*context); - } else { - m_context_ref.Clear(); - } - } - NodeContext* m_context{nullptr}; - util::Ref m_context_ref; -}; - -} // namespace - -std::unique_ptr<Node> MakeNode(NodeContext* context) { return MakeUnique<NodeImpl>(context); } - -} // namespace interfaces diff --git a/src/interfaces/chain.cpp b/src/node/interfaces.cpp index 4c5ebe66fc..77a5957a56 100644 --- a/src/interfaces/chain.cpp +++ b/src/node/interfaces.cpp @@ -2,18 +2,25 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <interfaces/chain.h> - +#include <addrdb.h> +#include <banman.h> +#include <boost/signals2/signal.hpp> #include <chain.h> #include <chainparams.h> +#include <init.h> +#include <interfaces/chain.h> #include <interfaces/handler.h> +#include <interfaces/node.h> #include <interfaces/wallet.h> #include <net.h> #include <net_processing.h> +#include <netaddress.h> +#include <netbase.h> #include <node/coin.h> #include <node/context.h> #include <node/transaction.h> #include <node/ui_interface.h> +#include <policy/feerate.h> #include <policy/fees.h> #include <policy/policy.h> #include <policy/rbf.h> @@ -23,20 +30,287 @@ #include <rpc/protocol.h> #include <rpc/server.h> #include <shutdown.h> +#include <support/allocators/secure.h> #include <sync.h> #include <timedata.h> #include <txmempool.h> #include <uint256.h> #include <univalue.h> +#include <util/check.h> +#include <util/ref.h> #include <util/system.h> +#include <util/translation.h> #include <validation.h> #include <validationinterface.h> +#include <warnings.h> + +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif #include <memory> #include <utility> -namespace interfaces { +using interfaces::BlockTip; +using interfaces::Chain; +using interfaces::FoundBlock; +using interfaces::Handler; +using interfaces::MakeHandler; +using interfaces::Node; +using interfaces::WalletClient; + +namespace node { namespace { +class NodeImpl : public Node +{ +public: + NodeImpl(NodeContext* context) { setContext(context); } + void initLogging() override { InitLogging(*Assert(m_context->args)); } + void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); } + bilingual_str getWarnings() override { return GetWarnings(true); } + uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); } + bool baseInitialize() override + { + return AppInitBasicSetup(gArgs) && AppInitParameterInteraction(gArgs) && AppInitSanityChecks() && + AppInitLockDataDirectory() && AppInitInterfaces(*m_context); + } + bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override + { + return AppInitMain(m_context_ref, *m_context, tip_info); + } + void appShutdown() override + { + Interrupt(*m_context); + Shutdown(*m_context); + } + void startShutdown() override + { + StartShutdown(); + // Stop RPC for clean shutdown if any of waitfor* commands is executed. + if (gArgs.GetBoolArg("-server", false)) { + InterruptRPC(); + StopRPC(); + } + } + bool shutdownRequested() override { return ShutdownRequested(); } + void mapPort(bool use_upnp) override + { + if (use_upnp) { + StartMapPort(); + } else { + InterruptMapPort(); + StopMapPort(); + } + } + bool getProxy(Network net, proxyType& proxy_info) override { return GetProxy(net, proxy_info); } + size_t getNodeCount(CConnman::NumConnections flags) override + { + return m_context->connman ? m_context->connman->GetNodeCount(flags) : 0; + } + bool getNodesStats(NodesStats& stats) override + { + stats.clear(); + + if (m_context->connman) { + std::vector<CNodeStats> stats_temp; + m_context->connman->GetNodeStats(stats_temp); + + stats.reserve(stats_temp.size()); + for (auto& node_stats_temp : stats_temp) { + stats.emplace_back(std::move(node_stats_temp), false, CNodeStateStats()); + } + + // Try to retrieve the CNodeStateStats for each node. + TRY_LOCK(::cs_main, lockMain); + if (lockMain) { + for (auto& node_stats : stats) { + std::get<1>(node_stats) = + GetNodeStateStats(std::get<0>(node_stats).nodeid, std::get<2>(node_stats)); + } + } + return true; + } + return false; + } + bool getBanned(banmap_t& banmap) override + { + if (m_context->banman) { + m_context->banman->GetBanned(banmap); + return true; + } + return false; + } + bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override + { + if (m_context->banman) { + m_context->banman->Ban(net_addr, ban_time_offset); + return true; + } + return false; + } + bool unban(const CSubNet& ip) override + { + if (m_context->banman) { + m_context->banman->Unban(ip); + return true; + } + return false; + } + bool disconnectByAddress(const CNetAddr& net_addr) override + { + if (m_context->connman) { + return m_context->connman->DisconnectNode(net_addr); + } + return false; + } + bool disconnectById(NodeId id) override + { + if (m_context->connman) { + return m_context->connman->DisconnectNode(id); + } + return false; + } + int64_t getTotalBytesRecv() override { return m_context->connman ? m_context->connman->GetTotalBytesRecv() : 0; } + int64_t getTotalBytesSent() override { return m_context->connman ? m_context->connman->GetTotalBytesSent() : 0; } + size_t getMempoolSize() override { return m_context->mempool ? m_context->mempool->size() : 0; } + size_t getMempoolDynamicUsage() override { return m_context->mempool ? m_context->mempool->DynamicMemoryUsage() : 0; } + bool getHeaderTip(int& height, int64_t& block_time) override + { + LOCK(::cs_main); + if (::pindexBestHeader) { + height = ::pindexBestHeader->nHeight; + block_time = ::pindexBestHeader->GetBlockTime(); + return true; + } + return false; + } + int getNumBlocks() override + { + LOCK(::cs_main); + return ::ChainActive().Height(); + } + uint256 getBestBlockHash() override + { + const CBlockIndex* tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip()); + return tip ? tip->GetBlockHash() : Params().GenesisBlock().GetHash(); + } + int64_t getLastBlockTime() override + { + LOCK(::cs_main); + if (::ChainActive().Tip()) { + return ::ChainActive().Tip()->GetBlockTime(); + } + return Params().GenesisBlock().GetBlockTime(); // Genesis block's time of current network + } + double getVerificationProgress() override + { + const CBlockIndex* tip; + { + LOCK(::cs_main); + tip = ::ChainActive().Tip(); + } + return GuessVerificationProgress(Params().TxData(), tip); + } + bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); } + bool getReindex() override { return ::fReindex; } + bool getImporting() override { return ::fImporting; } + void setNetworkActive(bool active) override + { + if (m_context->connman) { + m_context->connman->SetNetworkActive(active); + } + } + bool getNetworkActive() override { return m_context->connman && m_context->connman->GetNetworkActive(); } + CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) override + { + FeeCalculation fee_calc; + CFeeRate result = ::feeEstimator.estimateSmartFee(num_blocks, &fee_calc, conservative); + if (returned_target) { + *returned_target = fee_calc.returnedTarget; + } + return result; + } + CFeeRate getDustRelayFee() override { return ::dustRelayFee; } + UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override + { + JSONRPCRequest req(m_context_ref); + req.params = params; + req.strMethod = command; + req.URI = uri; + return ::tableRPC.execute(req); + } + std::vector<std::string> listRpcCommands() override { return ::tableRPC.listCommands(); } + void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); } + void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); } + bool getUnspentOutput(const COutPoint& output, Coin& coin) override + { + LOCK(::cs_main); + return ::ChainstateActive().CoinsTip().GetCoin(output, coin); + } + WalletClient& walletClient() override + { + return *Assert(m_context->wallet_client); + } + std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override + { + return MakeHandler(::uiInterface.InitMessage_connect(fn)); + } + std::unique_ptr<Handler> handleMessageBox(MessageBoxFn fn) override + { + return MakeHandler(::uiInterface.ThreadSafeMessageBox_connect(fn)); + } + std::unique_ptr<Handler> handleQuestion(QuestionFn fn) override + { + return MakeHandler(::uiInterface.ThreadSafeQuestion_connect(fn)); + } + std::unique_ptr<Handler> handleShowProgress(ShowProgressFn fn) override + { + return MakeHandler(::uiInterface.ShowProgress_connect(fn)); + } + std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override + { + return MakeHandler(::uiInterface.NotifyNumConnectionsChanged_connect(fn)); + } + std::unique_ptr<Handler> handleNotifyNetworkActiveChanged(NotifyNetworkActiveChangedFn fn) override + { + return MakeHandler(::uiInterface.NotifyNetworkActiveChanged_connect(fn)); + } + std::unique_ptr<Handler> handleNotifyAlertChanged(NotifyAlertChangedFn fn) override + { + return MakeHandler(::uiInterface.NotifyAlertChanged_connect(fn)); + } + std::unique_ptr<Handler> handleBannedListChanged(BannedListChangedFn fn) override + { + return MakeHandler(::uiInterface.BannedListChanged_connect(fn)); + } + std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override + { + return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) { + fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, + GuessVerificationProgress(Params().TxData(), block)); + })); + } + std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) override + { + return MakeHandler( + ::uiInterface.NotifyHeaderTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) { + fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, + /* verification progress is unused when a header was received */ 0); + })); + } + NodeContext* context() override { return m_context; } + void setContext(NodeContext* context) override + { + m_context = context; + if (context) { + m_context_ref.Set(*context); + } else { + m_context_ref.Clear(); + } + } + NodeContext* m_context{nullptr}; + util::Ref m_context_ref; +}; bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock) { @@ -411,7 +685,9 @@ public: NodeContext& m_node; }; } // namespace +} // namespace node -std::unique_ptr<Chain> MakeChain(NodeContext& node) { return MakeUnique<ChainImpl>(node); } - +namespace interfaces { +std::unique_ptr<Node> MakeNode(NodeContext* context) { return MakeUnique<node::NodeImpl>(context); } +std::unique_ptr<Chain> MakeChain(NodeContext& context) { return MakeUnique<node::ChainImpl>(context); } } // namespace interfaces diff --git a/src/node/transaction.h b/src/node/transaction.h index 6491700d44..0c016ff04e 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -36,6 +36,6 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; * @param[in] wait_callback wait until callbacks have been processed to avoid stale result due to a sequentially RPC. * return error */ -NODISCARD TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback); +[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback); #endif // BITCOIN_NODE_TRANSACTION_H diff --git a/src/outputtype.h b/src/outputtype.h index 77a16b1d05..bb7f39323b 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -22,7 +22,7 @@ enum class OutputType { extern const std::array<OutputType, 3> OUTPUT_TYPES; -NODISCARD bool ParseOutputType(const std::string& str, OutputType& output_type); +[[nodiscard]] bool ParseOutputType(const std::string& str, OutputType& output_type); const std::string& FormatOutputType(OutputType type); /** diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 91997aa883..4e33fd6cb5 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -92,14 +92,15 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR for (const CTxIn& txin : tx.vin) { - // Biggest 'standard' txin is a 15-of-15 P2SH multisig with compressed - // keys (remember the 520 byte limit on redeemScript size). That works - // out to a (15*(33+1))+3=513 byte redeemScript, 513+1+15*(73+1)+3=1627 - // bytes of scriptSig, which we round off to 1650 bytes for some minor - // future-proofing. That's also enough to spend a 20-of-20 - // CHECKMULTISIG scriptPubKey, though such a scriptPubKey is not - // considered standard. - if (txin.scriptSig.size() > 1650) { + // Biggest 'standard' txin involving only keys is a 15-of-15 P2SH + // multisig with compressed keys (remember the 520 byte limit on + // redeemScript size). That works out to a (15*(33+1))+3=513 byte + // redeemScript, 513+1+15*(73+1)+3=1627 bytes of scriptSig, which + // we round off to 1650(MAX_STANDARD_SCRIPTSIG_SIZE) bytes for + // some minor future-proofing. That's also enough to spend a + // 20-of-20 CHECKMULTISIG scriptPubKey, though such a scriptPubKey + // is not considered standard. + if (txin.scriptSig.size() > MAX_STANDARD_SCRIPTSIG_SIZE) { reason = "scriptsig-size"; return false; } diff --git a/src/policy/policy.h b/src/policy/policy.h index 8090dff4c6..726a14a27e 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -38,12 +38,14 @@ static const unsigned int DEFAULT_BYTES_PER_SIGOP = 20; static const bool DEFAULT_PERMIT_BAREMULTISIG = true; /** The maximum number of witness stack items in a standard P2WSH script */ static const unsigned int MAX_STANDARD_P2WSH_STACK_ITEMS = 100; -/** The maximum size of each witness stack item in a standard P2WSH script */ +/** The maximum size in bytes of each witness stack item in a standard P2WSH script */ static const unsigned int MAX_STANDARD_P2WSH_STACK_ITEM_SIZE = 80; -/** The maximum size of each witness stack item in a standard BIP 342 script (Taproot, leaf version 0xc0) */ +/** The maximum size in bytes of each witness stack item in a standard BIP 342 script (Taproot, leaf version 0xc0) */ static const unsigned int MAX_STANDARD_TAPSCRIPT_STACK_ITEM_SIZE = 80; -/** The maximum size of a standard witnessScript */ +/** The maximum size in bytes of a standard witnessScript */ static const unsigned int MAX_STANDARD_P2WSH_SCRIPT_SIZE = 3600; +/** The maximum size of a standard ScriptSig */ +static const unsigned int MAX_STANDARD_SCRIPTSIG_SIZE = 1650; /** Min feerate for defining dust. Historically this has been based on the * minRelayTxFee, however changing the dust limit changes which transactions are * standard and should be done with care and ideally rarely. It makes sense to @@ -103,7 +105,9 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, /** * Check if the transaction is over standard P2WSH resources limit: * 3600bytes witnessScript size, 80bytes per witness stack element, 100 witness stack elements - * These limits are adequate for multi-signature up to n-of-100 using OP_CHECKSIG, OP_ADD, and OP_EQUAL, + * These limits are adequate for multisignatures up to n-of-100 using OP_CHECKSIG, OP_ADD, and OP_EQUAL. + * + * Also enforce a maximum stack item size limit and no annexes for tapscript spends. */ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs); diff --git a/src/psbt.h b/src/psbt.h index 0951b76f83..b566726ee3 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -398,7 +398,7 @@ struct PartiallySignedTransaction /** Merge psbt into this. The two psbts must have the same underlying CTransaction (i.e. the * same actual Bitcoin transaction.) Returns true if the merge succeeded, false otherwise. */ - NODISCARD bool Merge(const PartiallySignedTransaction& psbt); + [[nodiscard]] bool Merge(const PartiallySignedTransaction& psbt); bool AddInput(const CTxIn& txin, PSBTInput& psbtin); bool AddOutput(const CTxOut& txout, const PSBTOutput& psbtout); PartiallySignedTransaction() {} @@ -605,11 +605,11 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti * @param[in] psbtxs the PSBTs to combine * @return error (OK if we successfully combined the transactions, other error if they were not compatible) */ -NODISCARD TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs); +[[nodiscard]] TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector<PartiallySignedTransaction>& psbtxs); //! Decode a base64ed PSBT into a PartiallySignedTransaction -NODISCARD bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error); +[[nodiscard]] bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error); //! Decode a raw (binary blob) PSBT into a PartiallySignedTransaction -NODISCARD bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error); +[[nodiscard]] bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error); #endif // BITCOIN_PSBT_H diff --git a/src/pubkey.h b/src/pubkey.h index 0f784b86e4..80d0c18540 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -170,6 +170,15 @@ public: /* * Check syntactic correctness. * + * When setting a pubkey (Set()) or deserializing fails (its header bytes + * don't match the length of the data), the size is set to 0. Thus, + * by checking size, one can observe whether Set() or deserialization has + * failed. + * + * This does not check for more than that. In particular, it does not verify + * that the coordinates correspond to a point on the curve (see IsFullyValid() + * for that instead). + * * Note that this is consensus critical as CheckECDSASignature() calls it! */ bool IsValid() const diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index 3b4967421a..3be8b664dd 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -199,6 +199,8 @@ void AskPassphraseDialog::textChanged() acceptable = !ui->passEdit2->text().isEmpty() && !ui->passEdit3->text().isEmpty(); break; case Unlock: // Old passphrase x1 + acceptable = !ui->passEdit1->text().isEmpty(); + break; case ChangePass: // Old passphrase x1, new passphrase x2 acceptable = !ui->passEdit1->text().isEmpty() && !ui->passEdit2->text().isEmpty() && !ui->passEdit3->text().isEmpty(); break; diff --git a/src/qt/bantablemodel.cpp b/src/qt/bantablemodel.cpp index 2739b21a9d..2676de96d7 100644 --- a/src/qt/bantablemodel.cpp +++ b/src/qt/bantablemodel.cpp @@ -11,6 +11,7 @@ #include <QDateTime> #include <QList> +#include <QLocale> #include <QModelIndex> #include <QVariant> @@ -122,7 +123,7 @@ QVariant BanTableModel::data(const QModelIndex &index, int role) const case Bantime: QDateTime date = QDateTime::fromMSecsSinceEpoch(0); date = date.addSecs(rec->banEntry.nBanUntil); - return date.toString(Qt::SystemLocaleLongDate); + return QLocale::system().toString(date, QLocale::LongFormat); } } diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 70e76f765b..53ffb27f50 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -43,6 +43,7 @@ #include <QKeyEvent> #include <QLineEdit> #include <QList> +#include <QLocale> #include <QMenu> #include <QMouseEvent> #include <QProgressDialog> @@ -67,7 +68,7 @@ namespace GUIUtil { QString dateTimeStr(const QDateTime &date) { - return date.date().toString(Qt::SystemLocaleShortDate) + QString(" ") + date.toString("hh:mm"); + return QLocale::system().toString(date.date(), QLocale::ShortFormat) + QString(" ") + date.toString("hh:mm"); } QString dateTimeStr(qint64 nTime) diff --git a/src/qt/notificator.cpp b/src/qt/notificator.cpp index 4b91c19761..d518a2065c 100644 --- a/src/qt/notificator.cpp +++ b/src/qt/notificator.cpp @@ -17,12 +17,7 @@ #include <stdint.h> #include <QtDBus> #endif -// Include ApplicationServices.h after QtDbus to avoid redefinition of check(). -// This affects at least OSX 10.6. See /usr/include/AssertMacros.h for details. -// Note: This could also be worked around using: -// #define __ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORES 0 #ifdef Q_OS_MAC -#include <ApplicationServices/ApplicationServices.h> #include <qt/macnotificationhandler.h> #endif diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 6c0a98cca2..8d8fa185ba 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -731,7 +731,7 @@ enum class ParseScriptContext { }; /** Parse a key path, being passed a split list of elements (the first element is ignored). */ -NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out, std::string& error) +[[nodiscard]] bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out, std::string& error) { for (size_t i = 1; i < split.size(); ++i) { Span<const char> elem = split[i]; diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 5735e7df66..bb5a7158a5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1834,9 +1834,13 @@ static bool ExecuteWitnessScript(const Span<const valtype>& stack_span, const CS static bool VerifyTaprootCommitment(const std::vector<unsigned char>& control, const std::vector<unsigned char>& program, const CScript& script, uint256& tapleaf_hash) { const int path_len = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE; + //! The inner pubkey (x-only, so no Y coordinate parity). const XOnlyPubKey p{uint256(std::vector<unsigned char>(control.begin() + 1, control.begin() + TAPROOT_CONTROL_BASE_SIZE))}; + //! The output pubkey (taken from the scriptPubKey). const XOnlyPubKey q{uint256(program)}; + // Compute the tapleaf hash. tapleaf_hash = (CHashWriter(HASHER_TAPLEAF) << uint8_t(control[0] & TAPROOT_LEAF_MASK) << script).GetSHA256(); + // Compute the Merkle root from the leaf and the provided path. uint256 k = tapleaf_hash; for (int i = 0; i < path_len; ++i) { CHashWriter ss_branch{HASHER_TAPBRANCH}; @@ -1848,7 +1852,9 @@ static bool VerifyTaprootCommitment(const std::vector<unsigned char>& control, c } k = ss_branch.GetSHA256(); } + // Compute the tweak from the Merkle root and the inner pubkey. k = (CHashWriter(HASHER_TAPTWEAK) << MakeSpan(p) << k).GetSHA256(); + // Verify that the output pubkey matches the tweaked inner pubkey, after correcting for parity. return q.CheckPayToContract(p, k, control[0] & 1); } diff --git a/src/span.h b/src/span.h index 4afb383a59..830164514b 100644 --- a/src/span.h +++ b/src/span.h @@ -18,6 +18,16 @@ #define ASSERT_IF_DEBUG(x) #endif +#if defined(__clang__) +#if __has_attribute(lifetimebound) +#define SPAN_ATTR_LIFETIMEBOUND [[clang::lifetimebound]] +#else +#define SPAN_ATTR_LIFETIMEBOUND +#endif +#else +#define SPAN_ATTR_LIFETIMEBOUND +#endif + /** A Span is an object that can refer to a contiguous sequence of objects. * * It implements a subset of C++20's std::span. @@ -84,6 +94,14 @@ class Span C* m_data; std::size_t m_size; + template <class T> + struct is_Span_int : public std::false_type {}; + template <class T> + struct is_Span_int<Span<T>> : public std::true_type {}; + template <class T> + struct is_Span : public is_Span_int<typename std::remove_cv<T>::type>{}; + + public: constexpr Span() noexcept : m_data(nullptr), m_size(0) {} @@ -134,8 +152,19 @@ public: * To prevent surprises, only Spans for constant value types are supported when passing in temporaries. * Note that this restriction does not exist when converting arrays or other Spans (see above). */ - template <typename V, typename std::enable_if<(std::is_const<C>::value || std::is_lvalue_reference<V>::value) && std::is_convertible<typename std::remove_pointer<decltype(std::declval<V&>().data())>::type (*)[], C (*)[]>::value && std::is_convertible<decltype(std::declval<V&>().size()), std::size_t>::value, int>::type = 0> - constexpr Span(V&& v) noexcept : m_data(v.data()), m_size(v.size()) {} + template <typename V> + constexpr Span(V& other SPAN_ATTR_LIFETIMEBOUND, + typename std::enable_if<!is_Span<V>::value && + std::is_convertible<typename std::remove_pointer<decltype(std::declval<V&>().data())>::type (*)[], C (*)[]>::value && + std::is_convertible<decltype(std::declval<V&>().size()), std::size_t>::value, std::nullptr_t>::type = nullptr) + : m_data(other.data()), m_size(other.size()){} + + template <typename V> + constexpr Span(const V& other SPAN_ATTR_LIFETIMEBOUND, + typename std::enable_if<!is_Span<V>::value && + std::is_convertible<typename std::remove_pointer<decltype(std::declval<const V&>().data())>::type (*)[], C (*)[]>::value && + std::is_convertible<decltype(std::declval<const V&>().size()), std::size_t>::value, std::nullptr_t>::type = nullptr) + : m_data(other.data()), m_size(other.size()){} constexpr C* data() const noexcept { return m_data; } constexpr C* begin() const noexcept { return m_data; } @@ -192,9 +221,9 @@ public: /** MakeSpan for arrays: */ template <typename A, int N> Span<A> constexpr MakeSpan(A (&a)[N]) { return Span<A>(a, N); } /** MakeSpan for temporaries / rvalue references, only supporting const output. */ -template <typename V> constexpr auto MakeSpan(V&& v) -> typename std::enable_if<!std::is_lvalue_reference<V>::value, Span<const typename std::remove_pointer<decltype(v.data())>::type>>::type { return std::forward<V>(v); } +template <typename V> constexpr auto MakeSpan(V&& v SPAN_ATTR_LIFETIMEBOUND) -> typename std::enable_if<!std::is_lvalue_reference<V>::value, Span<const typename std::remove_pointer<decltype(v.data())>::type>>::type { return std::forward<V>(v); } /** MakeSpan for (lvalue) references, supporting mutable output. */ -template <typename V> constexpr auto MakeSpan(V& v) -> Span<typename std::remove_pointer<decltype(v.data())>::type> { return v; } +template <typename V> constexpr auto MakeSpan(V& v SPAN_ATTR_LIFETIMEBOUND) -> Span<typename std::remove_pointer<decltype(v.data())>::type> { return v; } /** Pop the last element off a span, and return a reference to that element. */ template <typename T> diff --git a/src/sync.cpp b/src/sync.cpp index 322198a852..2e431720e6 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -13,10 +13,14 @@ #include <util/strencodings.h> #include <util/threadnames.h> +#include <boost/thread/mutex.hpp> + #include <map> +#include <mutex> #include <set> #include <system_error> #include <thread> +#include <type_traits> #include <unordered_map> #include <utility> #include <vector> @@ -135,16 +139,50 @@ static void potential_deadlock_detected(const LockPair& mismatch, const LockStac throw std::logic_error(strprintf("potential deadlock detected: %s -> %s -> %s", mutex_b, mutex_a, mutex_b)); } -static void push_lock(void* c, const CLockLocation& locklocation) +static void double_lock_detected(const void* mutex, LockStack& lock_stack) { + LogPrintf("DOUBLE LOCK DETECTED\n"); + LogPrintf("Lock order:\n"); + for (const LockStackItem& i : lock_stack) { + if (i.first == mutex) { + LogPrintf(" (*)"); /* Continued */ + } + LogPrintf(" %s\n", i.second.ToString()); + } + if (g_debug_lockorder_abort) { + tfm::format(std::cerr, "Assertion failed: detected double lock at %s:%i, details in debug log.\n", __FILE__, __LINE__); + abort(); + } + throw std::logic_error("double lock detected"); +} + +template <typename MutexType> +static void push_lock(MutexType* c, const CLockLocation& locklocation) +{ + constexpr bool is_recursive_mutex = + std::is_base_of<RecursiveMutex, MutexType>::value || + std::is_base_of<std::recursive_mutex, MutexType>::value; + LockData& lockdata = GetLockData(); std::lock_guard<std::mutex> lock(lockdata.dd_mutex); LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; lock_stack.emplace_back(c, locklocation); - for (const LockStackItem& i : lock_stack) { - if (i.first == c) - break; + for (size_t j = 0; j < lock_stack.size() - 1; ++j) { + const LockStackItem& i = lock_stack[j]; + if (i.first == c) { + if (is_recursive_mutex) { + break; + } + // It is not a recursive mutex and it appears in the stack two times: + // at position `j` and at the end (which we added just before this loop). + // Can't allow locking the same (non-recursive) mutex two times from the + // same thread as that results in an undefined behavior. + auto lock_stack_copy = lock_stack; + lock_stack.pop_back(); + double_lock_detected(c, lock_stack_copy); + // double_lock_detected() does not return. + } const LockPair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) @@ -175,10 +213,16 @@ static void pop_lock() } } -void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) +template <typename MutexType> +void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry) { push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, util::ThreadGetInternalName())); } +template void EnterCritical(const char*, const char*, int, Mutex*, bool); +template void EnterCritical(const char*, const char*, int, RecursiveMutex*, bool); +template void EnterCritical(const char*, const char*, int, std::mutex*, bool); +template void EnterCritical(const char*, const char*, int, std::recursive_mutex*, bool); +template void EnterCritical(const char*, const char*, int, boost::mutex*, bool); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) { diff --git a/src/sync.h b/src/sync.h index 41f4e43bdd..0948083c7f 100644 --- a/src/sync.h +++ b/src/sync.h @@ -48,7 +48,8 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII /////////////////////////////// #ifdef DEBUG_LOCKORDER -void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); +template <typename MutexType> +void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false); void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); @@ -66,7 +67,8 @@ bool LockStackEmpty(); */ extern bool g_debug_lockorder_abort; #else -inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} +template <typename MutexType> +inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false) {} inline void LeaveCritical() {} inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} template <typename MutexType> @@ -135,7 +137,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base private: void Enter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex())); + EnterCritical(pszName, pszFile, nLine, Base::mutex()); #ifdef DEBUG_LOCKCONTENTION if (!Base::try_lock()) { PrintLockContention(pszName, pszFile, nLine); @@ -148,7 +150,7 @@ private: bool TryEnter(const char* pszName, const char* pszFile, int nLine) { - EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true); + EnterCritical(pszName, pszFile, nLine, Base::mutex(), true); Base::try_lock(); if (!Base::owns_lock()) LeaveCritical(); @@ -205,7 +207,7 @@ public: ~reverse_lock() { templock.swap(lock); - EnterCritical(lockname.c_str(), file.c_str(), line, (void*)lock.mutex()); + EnterCritical(lockname.c_str(), file.c_str(), line, lock.mutex()); lock.lock(); } @@ -236,7 +238,7 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove #define ENTER_CRITICAL_SECTION(cs) \ { \ - EnterCritical(#cs, __FILE__, __LINE__, (void*)(&cs)); \ + EnterCritical(#cs, __FILE__, __LINE__, &cs); \ (cs).lock(); \ } diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 173ec5e3d9..6bfcf242d0 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -38,7 +38,7 @@ class CCoinsViewTest : public CCoinsView std::map<COutPoint, Coin> map_; public: - NODISCARD bool GetCoin(const COutPoint& outpoint, Coin& coin) const override + [[nodiscard]] bool GetCoin(const COutPoint& outpoint, Coin& coin) const override { std::map<COutPoint, Coin>::const_iterator it = map_.find(outpoint); if (it == map_.end()) { diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index a455992b13..7350ec7838 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -14,7 +14,7 @@ #if __has_builtin(__builtin_add_overflow) #define HAVE_BUILTIN_ADD_OVERFLOW #endif -#elif defined(__GNUC__) && (__GNUC__ >= 5) +#elif defined(__GNUC__) #define HAVE_BUILTIN_ADD_OVERFLOW #endif diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 0ceeea2d36..ae595be742 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -22,12 +22,22 @@ void initialize() SelectParams(CBaseChainParams::REGTEST); } +class CAddrManDeterministic : public CAddrMan +{ +public: + void MakeDeterministic(const uint256& random_seed) + { + insecure_rand = FastRandomContext{random_seed}; + Clear(); + } +}; + void test_one_input(const std::vector<uint8_t>& buffer) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - SetMockTime(ConsumeTime(fuzzed_data_provider)); - CAddrMan addr_man; + CAddrManDeterministic addr_man; + addr_man.MakeDeterministic(ConsumeUInt256(fuzzed_data_provider)); if (fuzzed_data_provider.ConsumeBool()) { addr_man.m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); if (!SanityCheckASMap(addr_man.m_asmap)) { diff --git a/src/test/fuzz/multiplication_overflow.cpp b/src/test/fuzz/multiplication_overflow.cpp index a4b158c18b..08dc660a19 100644 --- a/src/test/fuzz/multiplication_overflow.cpp +++ b/src/test/fuzz/multiplication_overflow.cpp @@ -14,7 +14,7 @@ #if __has_builtin(__builtin_mul_overflow) #define HAVE_BUILTIN_MUL_OVERFLOW #endif -#elif defined(__GNUC__) && (__GNUC__ >= 5) +#elif defined(__GNUC__) #define HAVE_BUILTIN_MUL_OVERFLOW #endif diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index e99ed8d72d..cf666a8b93 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -35,23 +35,23 @@ #include <string> #include <vector> -NODISCARD inline std::vector<uint8_t> ConsumeRandomLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept +[[nodiscard]] inline std::vector<uint8_t> ConsumeRandomLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept { const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(max_length); return {s.begin(), s.end()}; } -NODISCARD inline std::vector<bool> ConsumeRandomLengthBitVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept +[[nodiscard]] inline std::vector<bool> ConsumeRandomLengthBitVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept { return BytesToBits(ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length)); } -NODISCARD inline CDataStream ConsumeDataStream(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept +[[nodiscard]] inline CDataStream ConsumeDataStream(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept { return {ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length), SER_NETWORK, INIT_PROTO_VERSION}; } -NODISCARD inline std::vector<std::string> ConsumeRandomLengthStringVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16, const size_t max_string_length = 16) noexcept +[[nodiscard]] inline std::vector<std::string> ConsumeRandomLengthStringVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16, const size_t max_string_length = 16) noexcept { const size_t n_elements = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size); std::vector<std::string> r; @@ -62,7 +62,7 @@ NODISCARD inline std::vector<std::string> ConsumeRandomLengthStringVector(Fuzzed } template <typename T> -NODISCARD inline std::vector<T> ConsumeRandomLengthIntegralVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16) noexcept +[[nodiscard]] inline std::vector<T> ConsumeRandomLengthIntegralVector(FuzzedDataProvider& fuzzed_data_provider, const size_t max_vector_size = 16) noexcept { const size_t n_elements = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size); std::vector<T> r; @@ -73,7 +73,7 @@ NODISCARD inline std::vector<T> ConsumeRandomLengthIntegralVector(FuzzedDataProv } template <typename T> -NODISCARD inline std::optional<T> ConsumeDeserializable(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept +[[nodiscard]] inline std::optional<T> ConsumeDeserializable(FuzzedDataProvider& fuzzed_data_provider, const size_t max_length = 4096) noexcept { const std::vector<uint8_t> buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, max_length); CDataStream ds{buffer, SER_NETWORK, INIT_PROTO_VERSION}; @@ -86,35 +86,36 @@ NODISCARD inline std::optional<T> ConsumeDeserializable(FuzzedDataProvider& fuzz return obj; } -NODISCARD inline opcodetype ConsumeOpcodeType(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline opcodetype ConsumeOpcodeType(FuzzedDataProvider& fuzzed_data_provider) noexcept { return static_cast<opcodetype>(fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, MAX_OPCODE)); } -NODISCARD inline CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider) noexcept { return fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, MAX_MONEY); } -NODISCARD inline int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider) noexcept { - static const int64_t time_min = ParseISO8601DateTime("1970-01-01T00:00:00Z"); + // Avoid t=0 (1970-01-01T00:00:00Z) since SetMockTime(0) is a no-op. + static const int64_t time_min = ParseISO8601DateTime("1970-01-01T00:00:01Z"); static const int64_t time_max = ParseISO8601DateTime("9999-12-31T23:59:59Z"); return fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(time_min, time_max); } -NODISCARD inline CScript ConsumeScript(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline CScript ConsumeScript(FuzzedDataProvider& fuzzed_data_provider) noexcept { const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); return {b.begin(), b.end()}; } -NODISCARD inline CScriptNum ConsumeScriptNum(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline CScriptNum ConsumeScriptNum(FuzzedDataProvider& fuzzed_data_provider) noexcept { return CScriptNum{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; } -NODISCARD inline uint160 ConsumeUInt160(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline uint160 ConsumeUInt160(FuzzedDataProvider& fuzzed_data_provider) noexcept { const std::vector<uint8_t> v160 = fuzzed_data_provider.ConsumeBytes<uint8_t>(160 / 8); if (v160.size() != 160 / 8) { @@ -123,7 +124,7 @@ NODISCARD inline uint160 ConsumeUInt160(FuzzedDataProvider& fuzzed_data_provider return uint160{v160}; } -NODISCARD inline uint256 ConsumeUInt256(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline uint256 ConsumeUInt256(FuzzedDataProvider& fuzzed_data_provider) noexcept { const std::vector<uint8_t> v256 = fuzzed_data_provider.ConsumeBytes<uint8_t>(256 / 8); if (v256.size() != 256 / 8) { @@ -132,12 +133,12 @@ NODISCARD inline uint256 ConsumeUInt256(FuzzedDataProvider& fuzzed_data_provider return uint256{v256}; } -NODISCARD inline arith_uint256 ConsumeArithUInt256(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline arith_uint256 ConsumeArithUInt256(FuzzedDataProvider& fuzzed_data_provider) noexcept { return UintToArith256(ConsumeUInt256(fuzzed_data_provider)); } -NODISCARD inline CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept +[[nodiscard]] inline CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept { // Avoid: // policy/feerate.cpp:28:34: runtime error: signed integer overflow: 34873208148477500 * 1000 cannot be represented in type 'long' @@ -152,7 +153,7 @@ NODISCARD inline CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzze return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, spends_coinbase, sig_op_cost, {}}; } -NODISCARD inline CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept { CTxDestination tx_destination; switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 5)) { @@ -190,7 +191,7 @@ NODISCARD inline CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_ } template <typename T> -NODISCARD bool MultiplicationOverflow(const T i, const T j) noexcept +[[nodiscard]] bool MultiplicationOverflow(const T i, const T j) noexcept { static_assert(std::is_integral<T>::value, "Integral required."); if (std::numeric_limits<T>::is_signed) { @@ -213,7 +214,7 @@ NODISCARD bool MultiplicationOverflow(const T i, const T j) noexcept } template <class T> -NODISCARD bool AdditionOverflow(const T i, const T j) noexcept +[[nodiscard]] bool AdditionOverflow(const T i, const T j) noexcept { static_assert(std::is_integral<T>::value, "Integral required."); if (std::numeric_limits<T>::is_signed) { @@ -223,7 +224,7 @@ NODISCARD bool AdditionOverflow(const T i, const T j) noexcept return std::numeric_limits<T>::max() - i < j; } -NODISCARD inline bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept +[[nodiscard]] inline bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept { for (const CTxIn& tx_in : tx.vin) { const Coin& coin = inputs.AccessCoin(tx_in.prevout); @@ -238,7 +239,7 @@ NODISCARD inline bool ContainsSpentInput(const CTransaction& tx, const CCoinsVie * Returns a byte vector of specified size regardless of the number of remaining bytes available * from the fuzzer. Pads with zero value bytes if needed to achieve the specified size. */ -NODISCARD inline std::vector<uint8_t> ConsumeFixedLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t length) noexcept +[[nodiscard]] inline std::vector<uint8_t> ConsumeFixedLengthByteVector(FuzzedDataProvider& fuzzed_data_provider, const size_t length) noexcept { std::vector<uint8_t> result(length); const std::vector<uint8_t> random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(length); @@ -419,7 +420,7 @@ public: } }; -NODISCARD inline FuzzedFileProvider ConsumeFile(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline FuzzedFileProvider ConsumeFile(FuzzedDataProvider& fuzzed_data_provider) noexcept { return {fuzzed_data_provider}; } @@ -440,7 +441,7 @@ public: } }; -NODISCARD inline FuzzedAutoFileProvider ConsumeAutoFile(FuzzedDataProvider& fuzzed_data_provider) noexcept +[[nodiscard]] inline FuzzedAutoFileProvider ConsumeAutoFile(FuzzedDataProvider& fuzzed_data_provider) noexcept { return {fuzzed_data_provider}; } diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp index 19029ebd3c..6c14867211 100644 --- a/src/test/sync_tests.cpp +++ b/src/test/sync_tests.cpp @@ -6,6 +6,9 @@ #include <test/util/setup_common.h> #include <boost/test/unit_test.hpp> +#include <boost/thread/mutex.hpp> + +#include <mutex> namespace { template <typename MutexType> @@ -29,6 +32,38 @@ void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2) BOOST_CHECK(!error_thrown); #endif } + +#ifdef DEBUG_LOCKORDER +template <typename MutexType> +void TestDoubleLock2(MutexType& m) +{ + ENTER_CRITICAL_SECTION(m); + LEAVE_CRITICAL_SECTION(m); +} + +template <typename MutexType> +void TestDoubleLock(bool should_throw) +{ + const bool prev = g_debug_lockorder_abort; + g_debug_lockorder_abort = false; + + MutexType m; + ENTER_CRITICAL_SECTION(m); + if (should_throw) { + BOOST_CHECK_EXCEPTION( + TestDoubleLock2(m), std::logic_error, [](const std::logic_error& e) { + return strcmp(e.what(), "double lock detected") == 0; + }); + } else { + BOOST_CHECK_NO_THROW(TestDoubleLock2(m)); + } + LEAVE_CRITICAL_SECTION(m); + + BOOST_CHECK(LockStackEmpty()); + + g_debug_lockorder_abort = prev; +} +#endif /* DEBUG_LOCKORDER */ } // namespace BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup) @@ -55,4 +90,24 @@ BOOST_AUTO_TEST_CASE(potential_deadlock_detected) #endif } +/* Double lock would produce an undefined behavior. Thus, we only do that if + * DEBUG_LOCKORDER is activated to detect it. We don't want non-DEBUG_LOCKORDER + * build to produce tests that exhibit known undefined behavior. */ +#ifdef DEBUG_LOCKORDER +BOOST_AUTO_TEST_CASE(double_lock_mutex) +{ + TestDoubleLock<Mutex>(true /* should throw */); +} + +BOOST_AUTO_TEST_CASE(double_lock_boost_mutex) +{ + TestDoubleLock<boost::mutex>(true /* should throw */); +} + +BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex) +{ + TestDoubleLock<RecursiveMutex>(false /* should not throw */); +} +#endif /* DEBUG_LOCKORDER */ + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2d3137e1e2..adf5970206 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -141,8 +141,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const pblocktree.reset(new CBlockTreeDB(1 << 20, true)); - m_node.mempool = MakeUnique<CTxMemPool>(&::feeEstimator); - m_node.mempool->setSanityCheck(1.0); + m_node.mempool = MakeUnique<CTxMemPool>(&::feeEstimator, 1); m_node.chainman = &::g_chainman; m_node.chainman->InitializeChainstate(*m_node.mempool); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 8f2fc71cce..36fa1a0299 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1423,10 +1423,18 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32) BOOST_CHECK(ParseInt32("2147483647", &n) && n == 2147483647); BOOST_CHECK(ParseInt32("-2147483648", &n) && n == (-2147483647 - 1)); // (-2147483647 - 1) equals INT_MIN BOOST_CHECK(ParseInt32("-1234", &n) && n == -1234); + BOOST_CHECK(ParseInt32("00000000000000001234", &n) && n == 1234); + BOOST_CHECK(ParseInt32("-00000000000000001234", &n) && n == -1234); + BOOST_CHECK(ParseInt32("00000000000000000000", &n) && n == 0); + BOOST_CHECK(ParseInt32("-00000000000000000000", &n) && n == 0); // Invalid values BOOST_CHECK(!ParseInt32("", &n)); BOOST_CHECK(!ParseInt32(" 1", &n)); // no padding inside BOOST_CHECK(!ParseInt32("1 ", &n)); + BOOST_CHECK(!ParseInt32("++1", &n)); + BOOST_CHECK(!ParseInt32("+-1", &n)); + BOOST_CHECK(!ParseInt32("-+1", &n)); + BOOST_CHECK(!ParseInt32("--1", &n)); BOOST_CHECK(!ParseInt32("1a", &n)); BOOST_CHECK(!ParseInt32("aap", &n)); BOOST_CHECK(!ParseInt32("0x1", &n)); // no hex @@ -1482,10 +1490,19 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt32) BOOST_CHECK(ParseUInt32("2147483647", &n) && n == 2147483647); BOOST_CHECK(ParseUInt32("2147483648", &n) && n == (uint32_t)2147483648); BOOST_CHECK(ParseUInt32("4294967295", &n) && n == (uint32_t)4294967295); + BOOST_CHECK(ParseUInt32("+1234", &n) && n == 1234); + BOOST_CHECK(ParseUInt32("00000000000000001234", &n) && n == 1234); + BOOST_CHECK(ParseUInt32("00000000000000000000", &n) && n == 0); // Invalid values + BOOST_CHECK(!ParseUInt32("-00000000000000000000", &n)); BOOST_CHECK(!ParseUInt32("", &n)); BOOST_CHECK(!ParseUInt32(" 1", &n)); // no padding inside BOOST_CHECK(!ParseUInt32(" -1", &n)); + BOOST_CHECK(!ParseUInt32("++1", &n)); + BOOST_CHECK(!ParseUInt32("+-1", &n)); + BOOST_CHECK(!ParseUInt32("-+1", &n)); + BOOST_CHECK(!ParseUInt32("--1", &n)); + BOOST_CHECK(!ParseUInt32("-1", &n)); BOOST_CHECK(!ParseUInt32("1 ", &n)); BOOST_CHECK(!ParseUInt32("1a", &n)); BOOST_CHECK(!ParseUInt32("aap", &n)); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 0c2b731967..d18182c07d 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -331,15 +331,10 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, assert(int(nSigOpCostWithAncestors) >= 0); } -CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) - : nTransactionsUpdated(0), minerPolicyEstimator(estimator), m_epoch(0), m_has_epoch_guard(false) +CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator, int check_ratio) + : m_check_ratio(check_ratio), minerPolicyEstimator(estimator) { _clear(); //lock free clear - - // Sanity checks off by default for performance, because otherwise - // accepting transactions becomes O(N^2) where N is the number - // of transactions in the pool - nCheckFrequency = 0; } bool CTxMemPool::isSpent(const COutPoint& outpoint) const @@ -523,7 +518,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem if (it2 != mapTx.end()) continue; const Coin &coin = pcoins->AccessCoin(txin.prevout); - if (nCheckFrequency != 0) assert(!coin.IsSpent()); + if (m_check_ratio != 0) assert(!coin.IsSpent()); if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) { txToRemove.insert(it); break; @@ -619,13 +614,11 @@ static void CheckInputsAndUpdateCoins(const CTransaction& tx, CCoinsViewCache& m void CTxMemPool::check(const CCoinsViewCache *pcoins) const { - LOCK(cs); - if (nCheckFrequency == 0) - return; + if (m_check_ratio == 0) return; - if (GetRand(std::numeric_limits<uint32_t>::max()) >= nCheckFrequency) - return; + if (GetRand(m_check_ratio) >= 1) return; + LOCK(cs); LogPrint(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size()); uint64_t checkTotal = 0; diff --git a/src/txmempool.h b/src/txmempool.h index f513f14af6..78ad62aae6 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -488,8 +488,8 @@ public: class CTxMemPool { private: - uint32_t nCheckFrequency GUARDED_BY(cs); //!< Value n means that n times in 2^32 we check. - std::atomic<unsigned int> nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation + const int m_check_ratio; //!< Value n means that 1 times in n we check. + std::atomic<unsigned int> nTransactionsUpdated{0}; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation CBlockPolicyEstimator* minerPolicyEstimator; uint64_t totalTxSize; //!< sum of all mempool tx's virtual sizes. Differs from serialized tx size since witness data is discounted. Defined in BIP 141. @@ -498,8 +498,8 @@ private: mutable int64_t lastRollingFeeUpdate; mutable bool blockSinceLastRollingFeeBump; mutable double rollingMinimumFeeRate; //!< minimum fee to get into the pool, decreases exponentially - mutable uint64_t m_epoch; - mutable bool m_has_epoch_guard; + mutable uint64_t m_epoch{0}; + mutable bool m_has_epoch_guard{false}; // In-memory counter for external mempool tracking purposes. // This number is incremented once every time a transaction @@ -601,8 +601,14 @@ public: std::map<uint256, CAmount> mapDeltas; /** Create a new CTxMemPool. + * Sanity checks will be off by default for performance, because otherwise + * accepting transactions becomes O(N^2) where N is the number of transactions + * in the pool. + * + * @param[in] estimator is used to estimate appropriate transaction fees. + * @param[in] check_ratio is the ratio used to determine how often sanity checks will run. */ - explicit CTxMemPool(CBlockPolicyEstimator* estimator = nullptr); + explicit CTxMemPool(CBlockPolicyEstimator* estimator = nullptr, int check_ratio = 0); /** * If sanity-checking is turned on, check makes sure the pool is @@ -611,7 +617,6 @@ public: * check does nothing. */ void check(const CCoinsViewCache *pcoins) const; - void setSanityCheck(double dFrequency = 1.0) { LOCK(cs); nCheckFrequency = static_cast<uint32_t>(dFrequency * 4294967295.0); } // addUnchecked must updated state for all ancestors of a given transaction, // to track size/count of descendant transactions. First version of diff --git a/src/util/bip32.h b/src/util/bip32.h index 347e83db9e..8f86f2aaa6 100644 --- a/src/util/bip32.h +++ b/src/util/bip32.h @@ -10,7 +10,7 @@ #include <vector> /** Parse an HD keypaths like "m/7/0'/2000". */ -NODISCARD bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath); +[[nodiscard]] bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath); /** Write HD keypaths as strings */ std::string WriteHDKeypath(const std::vector<uint32_t>& keypath); diff --git a/src/util/moneystr.h b/src/util/moneystr.h index 9d2b6da0fc..da7f673cda 100644 --- a/src/util/moneystr.h +++ b/src/util/moneystr.h @@ -19,6 +19,6 @@ */ std::string FormatMoney(const CAmount& n); /** Parse an amount denoted in full coins. E.g. "0.0034" supplied on the command line. **/ -NODISCARD bool ParseMoney(const std::string& str, CAmount& nRet); +[[nodiscard]] bool ParseMoney(const std::string& str, CAmount& nRet); #endif // BITCOIN_UTIL_MONEYSTR_H diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 3236184b0b..f3d54a2ac9 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -280,7 +280,7 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid) return std::string((const char*)vchRet.data(), vchRet.size()); } -NODISCARD static bool ParsePrechecks(const std::string& str) +[[nodiscard]] static bool ParsePrechecks(const std::string& str) { if (str.empty()) // No empty string allowed return false; diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 1a217dd12d..8ee43c620b 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -101,42 +101,42 @@ constexpr inline bool IsSpace(char c) noexcept { * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -NODISCARD bool ParseInt32(const std::string& str, int32_t *out); +[[nodiscard]] bool ParseInt32(const std::string& str, int32_t *out); /** * Convert string to signed 64-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -NODISCARD bool ParseInt64(const std::string& str, int64_t *out); +[[nodiscard]] bool ParseInt64(const std::string& str, int64_t *out); /** * Convert decimal string to unsigned 8-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -NODISCARD bool ParseUInt8(const std::string& str, uint8_t *out); +[[nodiscard]] bool ParseUInt8(const std::string& str, uint8_t *out); /** * Convert decimal string to unsigned 32-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -NODISCARD bool ParseUInt32(const std::string& str, uint32_t *out); +[[nodiscard]] bool ParseUInt32(const std::string& str, uint32_t *out); /** * Convert decimal string to unsigned 64-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -NODISCARD bool ParseUInt64(const std::string& str, uint64_t *out); +[[nodiscard]] bool ParseUInt64(const std::string& str, uint64_t *out); /** * Convert string to double with strict parse error feedback. * @returns true if the entire string could be parsed as valid double, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -NODISCARD bool ParseDouble(const std::string& str, double *out); +[[nodiscard]] bool ParseDouble(const std::string& str, double *out); /** * Convert a span of bytes to a lower-case hexadecimal string. @@ -170,7 +170,7 @@ bool TimingResistantEqual(const T& a, const T& b) * @returns true on success, false on error. * @note The result must be in the range (-10^18,10^18), otherwise an overflow error will trigger. */ -NODISCARD bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); +[[nodiscard]] bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); /** Convert from one power-of-2 number base to another. */ template<int frombits, int tobits, bool pad, typename O, typename I> diff --git a/src/util/string.h b/src/util/string.h index a0c87bd00c..5ffdc80d88 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -15,7 +15,7 @@ #include <string> #include <vector> -NODISCARD inline std::string TrimString(const std::string& str, const std::string& pattern = " \f\n\r\t\v") +[[nodiscard]] inline std::string TrimString(const std::string& str, const std::string& pattern = " \f\n\r\t\v") { std::string::size_type front = str.find_first_not_of(pattern); if (front == std::string::npos) { @@ -59,7 +59,7 @@ inline std::string Join(const std::vector<std::string>& list, const std::string& /** * Check if a string does not contain any embedded NUL (\0) characters */ -NODISCARD inline bool ValidAsCString(const std::string& str) noexcept +[[nodiscard]] inline bool ValidAsCString(const std::string& str) noexcept { return str.size() == strlen(str.c_str()); } @@ -80,7 +80,7 @@ std::string ToString(const T& t) * Check whether a container begins with the given prefix. */ template <typename T1, size_t PREFIX_LEN> -NODISCARD inline bool HasPrefix(const T1& obj, +[[nodiscard]] inline bool HasPrefix(const T1& obj, const std::array<uint8_t, PREFIX_LEN>& prefix) { return obj.size() >= PREFIX_LEN && diff --git a/src/util/system.h b/src/util/system.h index 1df194ca84..2be8bb754b 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -60,7 +60,7 @@ bool FileCommit(FILE *file); bool TruncateFile(FILE *file, unsigned int length); int RaiseFileDescriptorLimit(int nMinFD); void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length); -bool RenameOver(fs::path src, fs::path dest); +[[nodiscard]] bool RenameOver(fs::path src, fs::path dest); bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false); void UnlockDirectory(const fs::path& directory, const std::string& lockfile_name); bool DirIsWritable(const fs::path& directory); @@ -188,7 +188,7 @@ protected: std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args GUARDED_BY(cs_args); std::list<SectionInfo> m_config_sections GUARDED_BY(cs_args); - NODISCARD bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false); + [[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false); /** * Returns true if settings values from the default section should be used, @@ -220,8 +220,8 @@ public: */ void SelectConfigNetwork(const std::string& network); - NODISCARD bool ParseParameters(int argc, const char* const argv[], std::string& error); - NODISCARD bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); + [[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error); + [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); /** * Log warnings for options in m_section_only_args when diff --git a/src/validation.cpp b/src/validation.cpp index 71402ef263..d8f7bfc913 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -696,7 +696,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); } - // Check for non-standard witness in P2WSH + // Check for non-standard witnesses. if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard"); @@ -5110,7 +5110,9 @@ bool DumpMempool(const CTxMemPool& pool) if (!FileCommit(file.Get())) throw std::runtime_error("FileCommit failed"); file.fclose(); - RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat"); + if (!RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat")) { + throw std::runtime_error("Rename failed"); + } int64_t last = GetTimeMicros(); LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*MICRO, (last-mid)*MICRO); } catch (const std::exception& e) { diff --git a/src/interfaces/wallet.cpp b/src/wallet/interfaces.cpp index f68016b557..3fbba9ab92 100644 --- a/src/interfaces/wallet.cpp +++ b/src/wallet/interfaces.cpp @@ -31,9 +31,22 @@ #include <utility> #include <vector> -namespace interfaces { -namespace { +using interfaces::Chain; +using interfaces::FoundBlock; +using interfaces::Handler; +using interfaces::MakeHandler; +using interfaces::Wallet; +using interfaces::WalletAddress; +using interfaces::WalletBalances; +using interfaces::WalletClient; +using interfaces::WalletOrderForm; +using interfaces::WalletTx; +using interfaces::WalletTxOut; +using interfaces::WalletTxStatus; +using interfaces::WalletValueMap; +namespace wallet { +namespace { //! Construct wallet tx struct. WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) { @@ -561,14 +574,14 @@ public: std::vector<std::unique_ptr<Handler>> m_rpc_handlers; std::list<CRPCCommand> m_rpc_commands; }; - } // namespace +} // namespace wallet -std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; } +namespace interfaces { +std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<wallet::WalletImpl>(wallet) : nullptr; } std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args) { - return MakeUnique<WalletClientImpl>(chain, args); + return MakeUnique<wallet::WalletClientImpl>(chain, args); } - } // namespace interfaces diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9425efa2dd..7ea6a214b2 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -197,25 +197,23 @@ static std::string LabelFromValue(const UniValue& value) /** * Update coin control with fee estimation based on the given parameters * - * @param[in] pwallet Wallet pointer + * @param[in] wallet Wallet reference * @param[in,out] cc Coin control to be updated * @param[in] conf_target UniValue integer; confirmation target in blocks, values between 1 and 1008 are valid per policy/fees.h; - * if a fee_rate is present, 0 is allowed here as a no-op positional placeholder * @param[in] estimate_mode UniValue string; fee estimation mode, valid values are "unset", "economical" or "conservative"; - * if a fee_rate is present, "" is allowed here as a no-op positional placeholder * @param[in] fee_rate UniValue real; fee rate in sat/vB; - * if a fee_rate is present, both conf_target and estimate_mode must either be null, or no-op + * if present, both conf_target and estimate_mode must either be null, or "unset" * @param[in] override_min_fee bool; whether to set fOverrideFeeRate to true to disable minimum fee rate checks and instead * verify only that fee_rate is greater than 0 * @throws a JSONRPCError if conf_target, estimate_mode, or fee_rate contain invalid values or are in conflict */ -static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, bool override_min_fee) +static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, bool override_min_fee) { if (!fee_rate.isNull()) { - if (!conf_target.isNull() && conf_target.get_int() > 0) { + if (!conf_target.isNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); } - if (!estimate_mode.isNull() && !estimate_mode.get_str().empty()) { + if (!estimate_mode.isNull() && estimate_mode.get_str() != "unset") { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate"); } cc.m_feerate = CFeeRate(AmountFromValue(fee_rate), COIN); @@ -228,7 +226,7 @@ static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const U throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage()); } if (!conf_target.isNull()) { - cc.m_confirm_target = ParseConfirmTarget(conf_target, pwallet->chain().estimateMaxBlocks()); + cc.m_confirm_target = ParseConfirmTarget(conf_target, wallet.chain().estimateMaxBlocks()); } } @@ -466,8 +464,8 @@ static RPCHelpMan sendtoaddress() + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1") + "\nSend 0.1 BTC with a confirmation target of 6 blocks in economical fee estimate mode using positional arguments\n" + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"donation\" \"sean's outpost\" false true 6 economical") + - "\nSend 0.1 BTC with a fee rate of 1 " + CURRENCY_ATOM + "/vB, subtract fee from amount, BIP125-replaceable, using positional arguments\n" - + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"drinks\" \"room77\" true true 0 \"\" 1") + + "\nSend 0.1 BTC with a fee rate of 1.1 " + CURRENCY_ATOM + "/vB, subtract fee from amount, BIP125-replaceable, using positional arguments\n" + + HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1 \"drinks\" \"room77\" true true null \"unset\" null 1.1") + "\nSend 0.2 BTC with a confirmation target of 6 blocks in economical fee estimate mode using named arguments\n" + HelpExampleCli("-named sendtoaddress", "address=\"" + EXAMPLE_ADDRESS[0] + "\" amount=0.2 conf_target=6 estimate_mode=\"economical\"") + "\nSend 0.5 BTC with a fee rate of 25 " + CURRENCY_ATOM + "/vB using named arguments\n" @@ -507,7 +505,7 @@ static RPCHelpMan sendtoaddress() // We also enable partial spend avoidance if reuse avoidance is set. coin_control.m_avoid_partial_spends |= coin_control.m_avoid_address_reuse; - SetFeeEstimateMode(pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[9], /* override_min_fee */ false); + SetFeeEstimateMode(*pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[9], /* override_min_fee */ false); EnsureWalletIsUnlocked(pwallet); @@ -935,7 +933,7 @@ static RPCHelpMan sendmany() coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); } - SetFeeEstimateMode(pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[8], /* override_min_fee */ false); + SetFeeEstimateMode(*pwallet, coin_control, /* conf_target */ request.params[6], /* estimate_mode */ request.params[7], /* fee_rate */ request.params[8], /* override_min_fee */ false); std::vector<CRecipient> recipients; ParseRecipients(sendTo, subtractFeeFromAmount, recipients); @@ -2791,7 +2789,7 @@ static RPCHelpMan unloadwallet() "Unloads the wallet referenced by the request endpoint otherwise unloads the wallet specified in the argument.\n" "Specifying the wallet name on a wallet endpoint is invalid.", { - {"wallet_name", RPCArg::Type::STR, /* default */ "the wallet name from the RPC request", "The name of the wallet to unload."}, + {"wallet_name", RPCArg::Type::STR, /* default */ "the wallet name from the RPC endpoint", "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."}, {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{RPCResult::Type::OBJ, "", "", { @@ -2805,8 +2803,8 @@ static RPCHelpMan unloadwallet() { std::string wallet_name; if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { - if (!request.params[0].isNull()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot unload the requested wallet"); + if (!(request.params[0].isNull() || request.params[0].get_str() == wallet_name)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "RPC endpoint wallet and wallet_name parameter specify different wallets"); } } else { wallet_name = request.params[0].get_str(); @@ -3159,7 +3157,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f if (options.exists("replaceable")) { coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } - SetFeeEstimateMode(pwallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); + SetFeeEstimateMode(*pwallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); } } else { // if options is null and not a bool @@ -3487,7 +3485,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) if (options.exists("replaceable")) { coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool(); } - SetFeeEstimateMode(pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /* override_min_fee */ false); + SetFeeEstimateMode(*pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /* override_min_fee */ false); } // Make sure the results are valid at least up to the most recent block @@ -4076,10 +4074,10 @@ static RPCHelpMan send() RPCExamples{"" "\nSend 0.1 BTC with a confirmation target of 6 blocks in economical fee estimate mode\n" + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.1}' 6 economical\n") + - "Send 0.2 BTC with a fee rate of 1 " + CURRENCY_ATOM + "/vB using positional arguments\n" - + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' 0 \"\" 1\n") + + "Send 0.2 BTC with a fee rate of 1.1 " + CURRENCY_ATOM + "/vB using positional arguments\n" + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' null \"unset\" 1.1\n") + "Send 0.2 BTC with a fee rate of 1 " + CURRENCY_ATOM + "/vB using the options argument\n" - + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' '{\"fee_rate\": 1}'\n") + + + HelpExampleCli("send", "'{\"" + EXAMPLE_ADDRESS[0] + "\": 0.2}' null \"unset\" null '{\"fee_rate\": 1}'\n") + "Send 0.3 BTC with a fee rate of 25 " + CURRENCY_ATOM + "/vB using named arguments\n" + HelpExampleCli("-named send", "outputs='{\"" + EXAMPLE_ADDRESS[0] + "\": 0.3}' fee_rate=25\n") + "Create a transaction that should confirm the next block, with a specific input, and return result without adding to wallet or broadcasting to the network\n" @@ -4236,7 +4234,7 @@ static RPCHelpMan sethdseed() // Do not do anything to non-HD wallets if (!pwallet->CanSupportFeature(FEATURE_HD)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed on a non-HD wallet. Use the upgradewallet RPC in order to upgrade a non-HD wallet to HD"); + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set an HD seed on a non-HD wallet. Use the upgradewallet RPC in order to upgrade a non-HD wallet to HD"); } EnsureWalletIsUnlocked(pwallet); @@ -4467,14 +4465,18 @@ static RPCHelpMan walletcreatefundedpsbt() static RPCHelpMan upgradewallet() { return RPCHelpMan{"upgradewallet", - "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified\n" + "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified.\n" "New keys may be generated and a new wallet backup will need to be made.", { - {"version", RPCArg::Type::NUM, /* default */ strprintf("%d", FEATURE_LATEST), "The version number to upgrade to. Default is the latest wallet version"} + {"version", RPCArg::Type::NUM, /* default */ strprintf("%d", FEATURE_LATEST), "The version number to upgrade to. Default is the latest wallet version."} }, RPCResult{ RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"}, + {RPCResult::Type::NUM, "previous_version", "Version of wallet before this operation"}, + {RPCResult::Type::NUM, "current_version", "Version of wallet after this operation"}, + {RPCResult::Type::STR, "result", /* optional */ true, "Description of result, if no error"}, {RPCResult::Type::STR, "error", /* optional */ true, "Error message (if there is one)"} }, }, @@ -4497,11 +4499,27 @@ static RPCHelpMan upgradewallet() version = request.params[0].get_int(); } bilingual_str error; - if (!pwallet->UpgradeWallet(version, error)) { - throw JSONRPCError(RPC_WALLET_ERROR, error.original); + const int previous_version{pwallet->GetVersion()}; + const bool wallet_upgraded{pwallet->UpgradeWallet(version, error)}; + const int current_version{pwallet->GetVersion()}; + std::string result; + + if (wallet_upgraded) { + if (previous_version == current_version) { + result = "Already at latest version. Wallet version unchanged."; + } else { + result = strprintf("Wallet upgraded successfully from version %i to version %i.", previous_version, current_version); + } } + UniValue obj(UniValue::VOBJ); - if (!error.empty()) { + obj.pushKV("wallet_name", pwallet->GetName()); + obj.pushKV("previous_version", previous_version); + obj.pushKV("current_version", current_version); + if (!result.empty()) { + obj.pushKV("result", result); + } else { + CHECK_NONFATAL(!error.empty()); obj.pushKV("error", error.original); } return obj; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 7ed20e4394..15972fe7bb 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -456,7 +456,7 @@ bool LegacyScriptPubKeyMan::Upgrade(int prev_version, int new_version, bilingual hd_upgrade = true; } // Upgrade to HD chain split if necessary - if (IsFeatureSupported(new_version, FEATURE_HD_SPLIT)) { + if (!IsFeatureSupported(prev_version, FEATURE_HD_SPLIT) && IsFeatureSupported(new_version, FEATURE_HD_SPLIT)) { WalletLogPrintf("Upgrading wallet to use HD chain split\n"); m_storage.SetMinVersion(FEATURE_PRE_SPLIT_KEYPOOL); split_upgrade = FEATURE_HD_SPLIT > prev_version; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ff8bfff872..65b54f39b4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -419,7 +419,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; if (!crypter.Encrypt(_vMasterKey, pMasterKey.second.vchCryptedKey)) return false; - WalletBatch(*database).WriteMasterKey(pMasterKey.first, pMasterKey.second); + WalletBatch(GetDatabase()).WriteMasterKey(pMasterKey.first, pMasterKey.second); if (fWasLocked) Lock(); return true; @@ -432,7 +432,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, void CWallet::chainStateFlushed(const CBlockLocator& loc) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); batch.WriteBestBlock(loc); } @@ -444,7 +444,7 @@ void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in) nWalletVersion = nVersion; { - WalletBatch* batch = batch_in ? batch_in : new WalletBatch(*database); + WalletBatch* batch = batch_in ? batch_in : new WalletBatch(GetDatabase()); if (nWalletVersion > 40000) batch->WriteMinVersion(nWalletVersion); if (!batch_in) @@ -484,12 +484,12 @@ bool CWallet::HasWalletSpend(const uint256& txid) const void CWallet::Flush() { - database->Flush(); + GetDatabase().Flush(); } void CWallet::Close() { - database->Close(); + GetDatabase().Close(); } void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> range) @@ -615,7 +615,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) { LOCK(cs_wallet); mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; - WalletBatch* encrypted_batch = new WalletBatch(*database); + WalletBatch* encrypted_batch = new WalletBatch(GetDatabase()); if (!encrypted_batch->TxnBegin()) { delete encrypted_batch; encrypted_batch = nullptr; @@ -667,12 +667,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // Need to completely rewrite the wallet file; if we don't, bdb might keep // bits of the unencrypted private key in slack space in the database file. - database->Rewrite(); + GetDatabase().Rewrite(); // BDB seems to have a bad habit of writing old data into // slack space in .dat files; that is bad if the old data is // unencrypted private keys. So: - database->ReloadDbEnv(); + GetDatabase().ReloadDbEnv(); } NotifyStatusChanged(this); @@ -683,7 +683,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) DBErrors CWallet::ReorderTransactions() { LOCK(cs_wallet); - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); // Old wallets didn't have any defined order for transactions // Probably a bad idea to change the output of this @@ -744,7 +744,7 @@ int64_t CWallet::IncOrderPosNext(WalletBatch* batch) if (batch) { batch->WriteOrderPosNext(nOrderPosNext); } else { - WalletBatch(*database).WriteOrderPosNext(nOrderPosNext); + WalletBatch(GetDatabase()).WriteOrderPosNext(nOrderPosNext); } return nRet; } @@ -774,7 +774,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash) wtx.mapValue["replaced_by_txid"] = newHash.ToString(); - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); bool success = true; if (!batch.WriteTx(wtx)) { @@ -846,7 +846,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio { LOCK(cs_wallet); - WalletBatch batch(*database, fFlushOnClose); + WalletBatch batch(GetDatabase(), fFlushOnClose); uint256 hash = tx->GetHash(); @@ -1045,7 +1045,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) { LOCK(cs_wallet); - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); std::set<uint256> todo; std::set<uint256> done; @@ -1108,7 +1108,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c return; // Do not flush the wallet here for performance reasons - WalletBatch batch(*database, false); + WalletBatch batch(GetDatabase(), false); std::set<uint256> todo; std::set<uint256> done; @@ -1446,13 +1446,13 @@ void CWallet::SetWalletFlag(uint64_t flags) { LOCK(cs_wallet); m_wallet_flags |= flags; - if (!WalletBatch(*database).WriteWalletFlags(m_wallet_flags)) + if (!WalletBatch(GetDatabase()).WriteWalletFlags(m_wallet_flags)) throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } void CWallet::UnsetWalletFlag(uint64_t flag) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); UnsetWalletFlagWithDB(batch, flag); } @@ -1491,7 +1491,7 @@ bool CWallet::AddWalletFlags(uint64_t flags) LOCK(cs_wallet); // We should never be writing unknown non-tolerable wallet flags assert(((flags & KNOWN_WALLET_FLAGS) >> 32) == (flags >> 32)); - if (!WalletBatch(*database).WriteWalletFlags(flags)) { + if (!WalletBatch(GetDatabase()).WriteWalletFlags(flags)) { throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } @@ -1582,7 +1582,7 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScri return false; } if (apply_label) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); for (const CScript& script : script_pub_keys) { CTxDestination dest; ExtractDestination(script, dest); @@ -3177,10 +3177,10 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) LOCK(cs_wallet); fFirstRunRet = false; - DBErrors nLoadWalletRet = WalletBatch(*database).LoadWallet(this); + DBErrors nLoadWalletRet = WalletBatch(GetDatabase()).LoadWallet(this); if (nLoadWalletRet == DBErrors::NEED_REWRITE) { - if (database->Rewrite("\x04pool")) + if (GetDatabase().Rewrite("\x04pool")) { for (const auto& spk_man_pair : m_spk_managers) { spk_man_pair.second->RewriteDB(); @@ -3204,7 +3204,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) { AssertLockHeld(cs_wallet); - DBErrors nZapSelectTxRet = WalletBatch(*database).ZapSelectTx(vHashIn, vHashOut); + DBErrors nZapSelectTxRet = WalletBatch(GetDatabase()).ZapSelectTx(vHashIn, vHashOut); for (const uint256& hash : vHashOut) { const auto& it = mapWallet.find(hash); wtxOrdered.erase(it->second.m_it_wtxOrdered); @@ -3216,7 +3216,7 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 if (nZapSelectTxRet == DBErrors::NEED_REWRITE) { - if (database->Rewrite("\x04pool")) + if (GetDatabase().Rewrite("\x04pool")) { for (const auto& spk_man_pair : m_spk_managers) { spk_man_pair.second->RewriteDB(); @@ -3254,14 +3254,14 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& strPurpose) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); return SetAddressBookWithDB(batch, address, strName, strPurpose); } bool CWallet::DelAddressBook(const CTxDestination& address) { bool is_mine; - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); { LOCK(cs_wallet); // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) @@ -4008,7 +4008,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st int rescan_height = 0; if (!gArgs.GetBoolArg("-rescan", false)) { - WalletBatch batch(*walletInstance->database); + WalletBatch batch(walletInstance->GetDatabase()); CBlockLocator locator; if (batch.ReadBestBlock(locator)) { if (const Optional<int> fork_height = chain.findLocatorFork(locator)) { @@ -4071,7 +4071,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st } } walletInstance->chainStateFlushed(chain.getTipLocator()); - walletInstance->database->IncrementUpdateCounter(); + walletInstance->GetDatabase().IncrementUpdateCounter(); } { @@ -4111,9 +4111,8 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) } else { WalletLogPrintf("Allowing wallet upgrade up to %i\n", version); } - if (version < prev_version) - { - error = _("Cannot downgrade wallet"); + if (version < prev_version) { + error = strprintf(_("Cannot downgrade wallet from version %i to version %i. Wallet version unchanged."), prev_version, version); return false; } @@ -4121,7 +4120,7 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) // Do not upgrade versions to any version between HD_SPLIT and FEATURE_PRE_SPLIT_KEYPOOL unless already supporting HD_SPLIT if (!CanSupportFeature(FEATURE_HD_SPLIT) && version >= FEATURE_HD_SPLIT && version < FEATURE_PRE_SPLIT_KEYPOOL) { - error = _("Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use version 169900 or no version specified."); + error = strprintf(_("Cannot upgrade a non HD split wallet from version %i to version %i without upgrading to support pre-split keypool. Please use version %i or no version specified."), prev_version, version, FEATURE_PRE_SPLIT_KEYPOOL); return false; } @@ -4150,7 +4149,7 @@ void CWallet::postInitProcess() bool CWallet::BackupWallet(const std::string& strDest) const { - return database->Backup(strDest); + return GetDatabase().Backup(strDest); } CKeyPool::CKeyPool() @@ -4453,7 +4452,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) { - WalletBatch batch(*database); + WalletBatch batch(GetDatabase()); if (!batch.WriteActiveScriptPubKeyMan(static_cast<uint8_t>(type), id, internal)) { throw std::runtime_error(std::string(__func__) + ": writing active ScriptPubKeyMan id failed"); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 69cf6b66a4..e6beb111fb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -695,7 +695,7 @@ private: std::string m_name; /** Internal database handle. */ - std::unique_ptr<WalletDatabase> database; + std::unique_ptr<WalletDatabase> const m_database; /** * The following is used to keep track of how far behind the wallet is @@ -729,14 +729,11 @@ public: */ mutable RecursiveMutex cs_wallet; - /** Get database handle used by this wallet. Ideally this function would - * not be necessary. - */ - WalletDatabase& GetDBHandle() + WalletDatabase& GetDatabase() const override { - return *database; + assert(static_cast<bool>(m_database)); + return *m_database; } - WalletDatabase& GetDatabase() const override { return *database; } /** * Select a set of coins such that nValueRet >= nTargetValue and at least @@ -758,7 +755,7 @@ public: CWallet(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database) : m_chain(chain), m_name(name), - database(std::move(database)) + m_database(std::move(database)) { } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 45807ae6fd..c0521d3386 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -945,7 +945,7 @@ void MaybeCompactWalletDB() } for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { - WalletDatabase& dbh = pwallet->GetDBHandle(); + WalletDatabase& dbh = pwallet->GetDatabase(); unsigned int nUpdateCounter = dbh.nUpdateCounter; diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index d3f76ec66c..9be610e8bd 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -91,13 +91,9 @@ bool IsFeatureSupported(int wallet_version, int feature_version) WalletFeature GetClosestWalletFeature(int version) { - if (version >= FEATURE_LATEST) return FEATURE_LATEST; - if (version >= FEATURE_PRE_SPLIT_KEYPOOL) return FEATURE_PRE_SPLIT_KEYPOOL; - if (version >= FEATURE_NO_DEFAULT_KEY) return FEATURE_NO_DEFAULT_KEY; - if (version >= FEATURE_HD_SPLIT) return FEATURE_HD_SPLIT; - if (version >= FEATURE_HD) return FEATURE_HD; - if (version >= FEATURE_COMPRPUBKEY) return FEATURE_COMPRPUBKEY; - if (version >= FEATURE_WALLETCRYPT) return FEATURE_WALLETCRYPT; - if (version >= FEATURE_BASE) return FEATURE_BASE; + const std::array<WalletFeature, 8> wallet_features{{FEATURE_LATEST, FEATURE_PRE_SPLIT_KEYPOOL, FEATURE_NO_DEFAULT_KEY, FEATURE_HD_SPLIT, FEATURE_HD, FEATURE_COMPRPUBKEY, FEATURE_WALLETCRYPT, FEATURE_BASE}}; + for (const WalletFeature& wf : wallet_features) { + if (version >= wf) return wf; + } return static_cast<WalletFeature>(0); } diff --git a/test/config.ini.in b/test/config.ini.in index 4b4a092a9d..77c9a720c3 100644 --- a/test/config.ini.in +++ b/test/config.ini.in @@ -17,6 +17,7 @@ RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py # Which components are enabled. These are commented out by `configure` if they were disabled when running config. @ENABLE_WALLET_TRUE@ENABLE_WALLET=true @USE_SQLITE_TRUE@USE_SQLITE=true +@USE_BDB_TRUE@USE_BDB=true @BUILD_BITCOIN_CLI_TRUE@ENABLE_CLI=true @BUILD_BITCOIN_WALLET_TRUE@ENABLE_WALLET_TOOL=true @BUILD_BITCOIND_TRUE@ENABLE_BITCOIND=true diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index 0f0fe8a34a..116eb7e3d7 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -444,6 +444,8 @@ def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh= * standard: whether the (valid version of) spending is expected to be standard * err_msg: a string with an expected error message for failure (or None, if not cared about) * sigops_weight: the pre-taproot sigops weight consumed by a successful spend + * need_vin_vout_mismatch: whether this test requires being tested in a transaction input that has no corresponding + transaction output. """ conf = dict() diff --git a/test/functional/mempool_compatibility.py b/test/functional/mempool_compatibility.py index 7168cb4ab2..8ac91bd008 100755 --- a/test/functional/mempool_compatibility.py +++ b/test/functional/mempool_compatibility.py @@ -29,7 +29,7 @@ class MempoolCompatibilityTest(BitcoinTestFramework): def setup_network(self): self.add_nodes(self.num_nodes, versions=[ - 150200, # oldest version supported by the test framework + 190100, # oldest version with getmempoolinfo.loaded (used to avoid intermittent issues) None, ]) self.start_nodes() @@ -72,5 +72,6 @@ class MempoolCompatibilityTest(BitcoinTestFramework): assert old_tx_hash in old_node.getrawmempool() assert unbroadcasted_tx_hash in old_node.getrawmempool() + if __name__ == "__main__": MempoolCompatibilityTest().main() diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 8e5848d493..26ccab3039 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -824,21 +824,33 @@ def taproot_tree_helper(scripts): h = TaggedHash("TapBranch", left_h + right_h) return (left + right, h) +# A TaprootInfo object has the following fields: +# - scriptPubKey: the scriptPubKey (witness v1 CScript) +# - inner_pubkey: the inner pubkey (32 bytes) +# - negflag: whether the pubkey in the scriptPubKey was negated from inner_pubkey+tweak*G (bool). +# - tweak: the tweak (32 bytes) +# - leaves: a dict of name -> TaprootLeafInfo objects for all known leaves TaprootInfo = namedtuple("TaprootInfo", "scriptPubKey,inner_pubkey,negflag,tweak,leaves") + +# A TaprootLeafInfo object has the following fields: +# - script: the leaf script (CScript or bytes) +# - version: the leaf version (0xc0 for BIP342 tapscript) +# - merklebranch: the merkle branch to use for this leaf (32*N bytes) TaprootLeafInfo = namedtuple("TaprootLeafInfo", "script,version,merklebranch") def taproot_construct(pubkey, scripts=None): """Construct a tree of Taproot spending conditions - pubkey: an ECPubKey object for the internal pubkey + pubkey: a 32-byte xonly pubkey for the internal pubkey (bytes) scripts: a list of items; each item is either: - - a (name, CScript) tuple - - a (name, CScript, leaf version) tuple + - a (name, CScript or bytes, leaf version) tuple + - a (name, CScript or bytes) tuple (defaulting to leaf version 0xc0) - another list of items (with the same structure) - - a function, which specifies how to compute the hashing partner - in function of the hash of whatever it is combined with + - a list of two items; the first of which is an item itself, and the + second is a function. The function takes as input the Merkle root of the + first item, and produces a (fictitious) partner to hash with. - Returns: script (sPK or redeemScript), tweak, {name:(script, leaf version, negation flag, innerkey, merklepath), ...} + Returns: a TaprootInfo object """ if scripts is None: scripts = [] diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 831599913d..bf047c5f68 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -782,6 +782,11 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): if not self.is_sqlite_compiled(): raise SkipTest("sqlite has not been compiled.") + def skip_if_no_bdb(self): + """Skip the running test if BDB has not been compiled.""" + if not self.is_bdb_compiled(): + raise SkipTest("BDB has not been compiled.") + def skip_if_no_wallet_tool(self): """Skip the running test if bitcoin-wallet has not been compiled.""" if not self.is_wallet_tool_compiled(): @@ -822,5 +827,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): return self.config["components"].getboolean("ENABLE_ZMQ") def is_sqlite_compiled(self): - """Checks whether the wallet module was compiled.""" + """Checks whether the wallet module was compiled with Sqlite support.""" return self.config["components"].getboolean("USE_SQLITE") + + def is_bdb_compiled(self): + """Checks whether the wallet module was compiled with BDB support.""" + return self.config["components"].getboolean("USE_BDB") diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 046efe730e..0a5b7f551c 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -544,7 +544,7 @@ class TestNode(): def num_test_p2p_connections(self): """Return number of test framework p2p connections to the node.""" - return len([peer for peer in self.getpeerinfo() if peer['subver'] == MY_SUBVERSION]) + return len([peer for peer in self.getpeerinfo() if peer['subver'] == MY_SUBVERSION.decode("utf-8")]) def disconnect_p2ps(self): """Close all p2p connections to the node.""" diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index ead56046a4..ac4a6e4948 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -235,8 +235,7 @@ class WalletTest(BitcoinTestFramework): fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 explicit_fee_rate_btc_kvb = Decimal(fee_rate_btc_kvb) / 1000 - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - txid = self.nodes[2].sendmany(amounts={address: 10}, conf_target=0, estimate_mode="", fee_rate=fee_rate_sat_vb) + txid = self.nodes[2].sendmany(amounts={address: 10}, fee_rate=fee_rate_sat_vb) self.nodes[2].generate(1) self.sync_all(self.nodes[0:3]) balance = self.nodes[2].getbalance() @@ -407,8 +406,7 @@ class WalletTest(BitcoinTestFramework): fee_rate_sat_vb = 2 fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8 - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - txid = self.nodes[2].sendtoaddress(address=address, amount=amount, conf_target=0, estimate_mode="", fee_rate=fee_rate_sat_vb) + txid = self.nodes[2].sendtoaddress(address=address, amount=amount, fee_rate=fee_rate_sat_vb) tx_size = self.get_vsize(self.nodes[2].gettransaction(txid)['hex']) self.nodes[0].generate(1) self.sync_all(self.nodes[0:3]) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index df16ec741f..fb4532bcf6 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -127,7 +127,7 @@ class MultiWalletTest(BitcoinTestFramework): os.mkdir(wallet_dir('no_access')) os.chmod(wallet_dir('no_access'), 0) try: - with self.nodes[0].assert_debug_log(expected_msgs=['Too many levels of symbolic links', 'Error scanning']): + with self.nodes[0].assert_debug_log(expected_msgs=['Error scanning']): walletlist = self.nodes[0].listwalletdir()['wallets'] finally: # Need to ensure access is restored for cleanup @@ -355,12 +355,18 @@ class MultiWalletTest(BitcoinTestFramework): assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].unloadwallet) assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", self.nodes[0].unloadwallet, "dummy") assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", node.get_wallet_rpc("dummy").unloadwallet) - assert_raises_rpc_error(-8, "Cannot unload the requested wallet", w1.unloadwallet, "w2"), + assert_raises_rpc_error(-8, "RPC endpoint wallet and wallet_name parameter specify different wallets", w1.unloadwallet, "w2"), # Successfully unload the specified wallet name self.nodes[0].unloadwallet("w1") assert 'w1' not in self.nodes[0].listwallets() + # Unload w1 again, this time providing the wallet name twice + self.nodes[0].loadwallet("w1") + assert 'w1' in self.nodes[0].listwallets() + w1.unloadwallet("w1") + assert 'w1' not in self.nodes[0].listwallets() + # Successfully unload the wallet referenced by the request endpoint # Also ensure unload works during walletpassphrase timeout w2.encryptwallet('test') diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 685e805877..192e9065e6 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -260,17 +260,16 @@ class WalletSendTest(BitcoinTestFramework): res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=1, add_to_wallet=False) assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"]) - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0, estimate_mode="", fee_rate=7, add_to_wallet=False) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=7, add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007")) - res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, add_to_wallet=False) + # "unset" and None are treated the same for estimate_mode + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=2, estimate_mode="unset", add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002")) - # Passing conf_target 0, estimate_mode "" as placeholder arguments should allow fee_rate to apply. - res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0, arg_estimate_mode="", arg_fee_rate=4.531, add_to_wallet=False) + res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=4.531, add_to_wallet=False) fee = self.nodes[1].decodepsbt(res["psbt"])["fee"] assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531")) diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 8ab4b3f76c..d0bb6135a8 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -22,9 +22,7 @@ from test_framework.messages import deser_compact_size, deser_string from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - assert_greater_than, assert_is_hex_string, - assert_raises_rpc_error, sha256sum_file, ) @@ -92,6 +90,32 @@ class UpgradeWalletTest(BitcoinTestFramework): v16_3_node.submitblock(b) assert_equal(v16_3_node.getblockcount(), to_height) + def test_upgradewallet(self, wallet, previous_version, requested_version=None, expected_version=None): + unchanged = expected_version == previous_version + new_version = previous_version if unchanged else expected_version if expected_version else requested_version + assert_equal(wallet.getwalletinfo()["walletversion"], previous_version) + assert_equal(wallet.upgradewallet(requested_version), + { + "wallet_name": "", + "previous_version": previous_version, + "current_version": new_version, + "result": "Already at latest version. Wallet version unchanged." if unchanged else "Wallet upgraded successfully from version {} to version {}.".format(previous_version, new_version), + } + ) + assert_equal(wallet.getwalletinfo()["walletversion"], new_version) + + def test_upgradewallet_error(self, wallet, previous_version, requested_version, msg): + assert_equal(wallet.getwalletinfo()["walletversion"], previous_version) + assert_equal(wallet.upgradewallet(requested_version), + { + "wallet_name": "", + "previous_version": previous_version, + "current_version": previous_version, + "error": msg, + } + ) + assert_equal(wallet.getwalletinfo()["walletversion"], previous_version) + def run_test(self): self.nodes[0].generatetoaddress(101, self.nodes[0].getnewaddress()) self.dumb_sync_blocks() @@ -158,14 +182,8 @@ class UpgradeWalletTest(BitcoinTestFramework): self.restart_node(0) copy_v16() wallet = node_master.get_wallet_rpc(self.default_wallet_name) - old_version = wallet.getwalletinfo()["walletversion"] - - # calling upgradewallet without version arguments - # should return nothing if successful - assert_equal(wallet.upgradewallet(), {}) - new_version = wallet.getwalletinfo()["walletversion"] - # upgraded wallet version should be greater than older one - assert_greater_than(new_version, old_version) + self.log.info("Test upgradewallet without a version argument") + self.test_upgradewallet(wallet, previous_version=159900, expected_version=169900) # wallet should still contain the same balance assert_equal(wallet.getbalance(), v16_3_balance) @@ -173,32 +191,28 @@ class UpgradeWalletTest(BitcoinTestFramework): wallet = node_master.get_wallet_rpc(self.default_wallet_name) # should have no master key hash before conversion assert_equal('hdseedid' in wallet.getwalletinfo(), False) - # calling upgradewallet with explicit version number - # should return nothing if successful - assert_equal(wallet.upgradewallet(169900), {}) - new_version = wallet.getwalletinfo()["walletversion"] - # upgraded wallet should have version 169900 - assert_equal(new_version, 169900) + self.log.info("Test upgradewallet with explicit version number") + self.test_upgradewallet(wallet, previous_version=60000, requested_version=169900) # after conversion master key hash should be present assert_is_hex_string(wallet.getwalletinfo()['hdseedid']) - self.log.info('Intermediary versions don\'t effect anything') + self.log.info("Intermediary versions don't effect anything") copy_non_hd() # Wallet starts with 60000 assert_equal(60000, wallet.getwalletinfo()['walletversion']) wallet.unloadwallet() before_checksum = sha256sum_file(node_master_wallet) node_master.loadwallet('') - # Can "upgrade" to 129999 which should have no effect on the wallet - wallet.upgradewallet(129999) - assert_equal(60000, wallet.getwalletinfo()['walletversion']) + # Test an "upgrade" from 60000 to 129999 has no effect, as the next version is 130000 + self.test_upgradewallet(wallet, previous_version=60000, requested_version=129999, expected_version=60000) wallet.unloadwallet() assert_equal(before_checksum, sha256sum_file(node_master_wallet)) node_master.loadwallet('') self.log.info('Wallets cannot be downgraded') copy_non_hd() - assert_raises_rpc_error(-4, 'Cannot downgrade wallet', wallet.upgradewallet, 40000) + self.test_upgradewallet_error(wallet, previous_version=60000, requested_version=40000, + msg="Cannot downgrade wallet from version 60000 to version 40000. Wallet version unchanged.") wallet.unloadwallet() assert_equal(before_checksum, sha256sum_file(node_master_wallet)) node_master.loadwallet('') @@ -208,8 +222,7 @@ class UpgradeWalletTest(BitcoinTestFramework): orig_kvs = dump_bdb_kv(node_master_wallet) assert b'\x07hdchain' not in orig_kvs # Upgrade to HD, no split - wallet.upgradewallet(130000) - assert_equal(130000, wallet.getwalletinfo()['walletversion']) + self.test_upgradewallet(wallet, previous_version=60000, requested_version=130000) # Check that there is now a hd chain and it is version 1, no internal chain counter new_kvs = dump_bdb_kv(node_master_wallet) assert b'\x07hdchain' in new_kvs @@ -236,16 +249,13 @@ class UpgradeWalletTest(BitcoinTestFramework): assert_equal('m/0\'/0\'/1\'', info['hdkeypath']) self.log.info('Cannot upgrade to HD Split, needs Pre Split Keypool') - assert_raises_rpc_error(-4, 'Cannot upgrade a non HD split wallet without upgrading to support pre split keypool', wallet.upgradewallet, 139900) - assert_equal(130000, wallet.getwalletinfo()['walletversion']) - assert_raises_rpc_error(-4, 'Cannot upgrade a non HD split wallet without upgrading to support pre split keypool', wallet.upgradewallet, 159900) - assert_equal(130000, wallet.getwalletinfo()['walletversion']) - assert_raises_rpc_error(-4, 'Cannot upgrade a non HD split wallet without upgrading to support pre split keypool', wallet.upgradewallet, 169899) - assert_equal(130000, wallet.getwalletinfo()['walletversion']) + for version in [139900, 159900, 169899]: + self.test_upgradewallet_error(wallet, previous_version=130000, requested_version=version, + msg="Cannot upgrade a non HD split wallet from version {} to version {} without upgrading to " + "support pre-split keypool. Please use version 169900 or no version specified.".format(130000, version)) self.log.info('Upgrade HD to HD chain split') - wallet.upgradewallet(169900) - assert_equal(169900, wallet.getwalletinfo()['walletversion']) + self.test_upgradewallet(wallet, previous_version=130000, requested_version=169900) # Check that the hdchain updated correctly new_kvs = dump_bdb_kv(node_master_wallet) hd_chain = new_kvs[b'\x07hdchain'] @@ -271,8 +281,7 @@ class UpgradeWalletTest(BitcoinTestFramework): self.log.info('Upgrade non-HD to HD chain split') copy_non_hd() - wallet.upgradewallet(169900) - assert_equal(169900, wallet.getwalletinfo()['walletversion']) + self.test_upgradewallet(wallet, previous_version=60000, requested_version=169900) # Check that the hdchain updated correctly new_kvs = dump_bdb_kv(node_master_wallet) hd_chain = new_kvs[b'\x07hdchain'] @@ -333,8 +342,8 @@ class UpgradeWalletTest(BitcoinTestFramework): # Check the wallet has a default key initially old_kvs = dump_bdb_kv(node_master_wallet) defaultkey = old_kvs[b'\x0adefaultkey'] - # Upgrade the wallet. Should still have the same default key - wallet.upgradewallet(159900) + self.log.info("Upgrade the wallet. Should still have the same default key.") + self.test_upgradewallet(wallet, previous_version=139900, requested_version=159900) new_kvs = dump_bdb_kv(node_master_wallet) up_defaultkey = new_kvs[b'\x0adefaultkey'] assert_equal(defaultkey, up_defaultkey) @@ -342,5 +351,6 @@ class UpgradeWalletTest(BitcoinTestFramework): v16_3_kvs = dump_bdb_kv(v16_3_wallet) assert b'\x0adefaultkey' not in v16_3_kvs + if __name__ == '__main__': UpgradeWalletTest().main() |