diff options
117 files changed, 1819 insertions, 912 deletions
diff --git a/.appveyor.yml b/.appveyor.yml index 7dcf9388b9..6722c1fbec 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -11,7 +11,7 @@ environment: QT_DOWNLOAD_HASH: '9a8c6eb20967873785057fdcd329a657c7f922b0af08c5fde105cc597dd37e21' QT_LOCAL_PATH: 'C:\Qt5.9.8_x64_static_vs2019' VCPKG_INSTALL_PATH: 'C:\tools\vcpkg\installed' - VCPKG_COMMIT_ID: 'ed0df8ecc4ed7e755ea03e18aaf285fd9b4b4a74' + VCPKG_COMMIT_ID: 'f3f329a048eaff759c1992c458f2e12351486bc7' install: # Disable zmq test for now since python zmq library on Windows would cause Access violation sometimes. # - cmd: pip install zmq diff --git a/Makefile.am b/Makefile.am index 1d6358b1d5..8cdd3ff647 100644 --- a/Makefile.am +++ b/Makefile.am @@ -181,6 +181,7 @@ $(BITCOIN_WALLET_BIN): FORCE if USE_LCOV LCOV_FILTER_PATTERN = \ + -p "/usr/local/" \ -p "/usr/include/" \ -p "/usr/lib/" \ -p "/usr/lib64/" \ diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh index f9d869b4fd..6f2e39429c 100644 --- a/ci/test/00_setup_env_native_qt5.sh +++ b/ci/test/00_setup_env_native_qt5.sh @@ -15,5 +15,5 @@ export RUN_SECURITY_TESTS="true" export RUN_UNIT_TESTS_SEQUENTIAL="true" export RUN_UNIT_TESTS="false" export GOAL="install" -export PREVIOUS_RELEASES_TO_DOWNLOAD="v0.15.2 v0.16.3 v0.17.1 v0.18.1 v0.19.1" +export PREVIOUS_RELEASES_TO_DOWNLOAD="v0.15.2 v0.16.3 v0.17.2 v0.18.1 v0.19.1" export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports --enable-c++17 --enable-debug CFLAGS=\"-g0 -O2 -funsigned-char\" CXXFLAGS=\"-g0 -O2 -funsigned-char\" --with-boost-process" diff --git a/ci/test/05_before_script.sh b/ci/test/05_before_script.sh index 131ea21677..8ce839fc04 100755 --- a/ci/test/05_before_script.sh +++ b/ci/test/05_before_script.sh @@ -48,6 +48,6 @@ if [ -z "$NO_DEPENDS" ]; then fi if [ -n "$PREVIOUS_RELEASES_TO_DOWNLOAD" ]; then BEGIN_FOLD previous-versions - DOCKER_EXEC contrib/devtools/previous_release.py -b -t "$PREVIOUS_RELEASES_DIR" "${PREVIOUS_RELEASES_TO_DOWNLOAD}" + DOCKER_EXEC test/get_previous_releases.py -b -t "$PREVIOUS_RELEASES_DIR" "${PREVIOUS_RELEASES_TO_DOWNLOAD}" END_FOLD fi diff --git a/configure.ac b/configure.ac index acd4e0cf6c..14ce47092b 100644 --- a/configure.ac +++ b/configure.ac @@ -795,6 +795,14 @@ if test x$use_hardening != xno; then AX_CHECK_COMPILE_FLAG([-Wstack-protector],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -Wstack-protector"]) AX_CHECK_COMPILE_FLAG([-fstack-protector-all],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-protector-all"]) + AX_CHECK_COMPILE_FLAG([-fcf-protection=full],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fcf-protection=full"]) + + dnl stack-clash-protection does not work properly when building for Windows. + dnl We use the test case from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458 + dnl to determine if it can be enabled. + AX_CHECK_COMPILE_FLAG([-fstack-clash-protection],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-clash-protection"],[],["-O0"], + [AC_LANG_SOURCE([[class D {public: unsigned char buf[32768];}; int main() {D d; return 0;}]])]) + dnl When enable_debug is yes, all optimizations are disabled. dnl However, FORTIFY_SOURCE requires that there is some level of optimization, otherwise it does nothing and just creates a compiler warning. dnl Since FORTIFY_SOURCE is a no-op without optimizations, do not enable it when enable_debug is yes. @@ -1039,13 +1047,13 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdint.h> [ AC_MSG_RESULT(no)] ) -dnl LevelDB platform checks AC_MSG_CHECKING(for fdatasync) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <unistd.h>]], [[ fdatasync(0); ]])], [ AC_MSG_RESULT(yes); HAVE_FDATASYNC=1 ], [ AC_MSG_RESULT(no); HAVE_FDATASYNC=0 ] ) +AC_DEFINE_UNQUOTED([HAVE_FDATASYNC], [$HAVE_FDATASYNC], [Define to 1 if fdatasync is available.]) AC_MSG_CHECKING(for F_FULLFSYNC) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <fcntl.h>]], diff --git a/depends/funcs.mk b/depends/funcs.mk index 6fc20543bb..81ecbedf5c 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -157,12 +157,17 @@ ifneq ($($(1)_ldflags),) $(1)_autoconf += LDFLAGS="$$($(1)_ldflags)" endif -$(1)_cmake=cmake -DCMAKE_INSTALL_PREFIX=$($($(1)_type)_prefix) +$(1)_cmake=env CC="$$($(1)_cc)" \ + CFLAGS="$$($(1)_cppflags) $$($(1)_cflags)" \ + CXX="$$($(1)_cxx)" \ + CXXFLAGS="$$($(1)_cppflags) $$($(1)_cxxflags)" \ + LDFLAGS="$$($(1)_ldflags)" \ + cmake -DCMAKE_INSTALL_PREFIX:PATH="$$($($(1)_type)_prefix)" ifneq ($($(1)_type),build) ifneq ($(host),$(build)) -$(1)_cmake += -DCMAKE_SYSTEM_NAME=$($(host_os)_cmake_system) -DCMAKE_SYSROOT=$(host_prefix) -$(1)_cmake += -DCMAKE_C_COMPILER_TARGET=$(host) -DCMAKE_C_COMPILER=$(firstword $($($(1)_type)_CC)) -DCMAKE_C_FLAGS="$(wordlist 2,1000,$($($(1)_type)_CC))" -$(1)_cmake += -DCMAKE_CXX_COMPILER_TARGET=$(host) -DCMAKE_CXX_COMPILER=$(firstword $($($(1)_type)_CXX)) -DCMAKE_CXX_FLAGS="$(wordlist 2,1000,$($($(1)_type)_CXX))" +$(1)_cmake += -DCMAKE_SYSTEM_NAME=$($(host_os)_cmake_system) +$(1)_cmake += -DCMAKE_C_COMPILER_TARGET=$(host) +$(1)_cmake += -DCMAKE_CXX_COMPILER_TARGET=$(host) endif endif endef diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index b679438c6f..06cf974f75 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -4,6 +4,7 @@ $(package)_download_path=https://download.oracle.com/berkeley-db $(package)_file_name=db-$($(package)_version).NC.tar.gz $(package)_sha256_hash=12edc0df75bf9abd7f82f821795bcee50f42cb2e5f76a6a281b85732798364ef $(package)_build_subdir=build_unix +$(package)_patches=clang_cxx_11.patch define $(package)_set_vars $(package)_config_opts=--disable-shared --enable-cxx --disable-replication --enable-option-checking @@ -14,8 +15,7 @@ $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE endef define $(package)_preprocess_cmds - sed -i.old 's/__atomic_compare_exchange/__atomic_compare_exchange_db/' dbinc/atomic.h && \ - sed -i.old 's/atomic_init/atomic_init_db/' dbinc/atomic.h mp/mp_region.c mp/mp_mvcc.c mp/mp_fget.c mutex/mut_method.c mutex/mut_tas.c && \ + patch -p1 < $($(package)_patch_dir)/clang_cxx_11.patch && \ cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub dist endef diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index 4f6b543aff..d8bce108b1 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -3,6 +3,7 @@ $(package)_version=1_70_0 $(package)_download_path=https://dl.bintray.com/boostorg/release/1.70.0/source/ $(package)_file_name=boost_$($(package)_version).tar.bz2 $(package)_sha256_hash=430ae8354789de4fd19ee52f3b1f739e1fba576f0aded0897c3c2bc00fb38778 +$(package)_patches=unused_var_in_process.patch define $(package)_set_vars $(package)_config_opts_release=variant=release @@ -31,9 +32,8 @@ $(package)_cxxflags_linux=-fPIC $(package)_cxxflags_android=-fPIC endef -# Fix unused variable in boost_process, can be removed after upgrading to 1.72 define $(package)_preprocess_cmds - sed -i.old "s/int ret_sig = 0;//" boost/process/detail/posix/wait_group.hpp && \ + 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 endef diff --git a/depends/packages/fontconfig.mk b/depends/packages/fontconfig.mk index 128599ba77..0d5f94f380 100644 --- a/depends/packages/fontconfig.mk +++ b/depends/packages/fontconfig.mk @@ -4,23 +4,23 @@ $(package)_download_path=https://www.freedesktop.org/software/fontconfig/release $(package)_file_name=$(package)-$($(package)_version).tar.bz2 $(package)_sha256_hash=b449a3e10c47e1d1c7a6ec6e2016cca73d3bd68fbbd4f0ae5cc6b573f7d6c7f3 $(package)_dependencies=freetype expat +$(package)_patches=remove_char_width_usage.patch gperf_header_regen.patch define $(package)_set_vars $(package)_config_opts=--disable-docs --disable-static --disable-libxml2 --disable-iconv $(package)_config_opts += --disable-dependency-tracking --enable-option-checking endef +define $(package)_preprocess_cmds + patch -p1 < $($(package)_patch_dir)/remove_char_width_usage.patch && \ + patch -p1 < $($(package)_patch_dir)/gperf_header_regen.patch +endef + define $(package)_config_cmds $($(package)_autoconf) endef -# 2.12.1 uses CHAR_WIDTH which is reserved and clashes with some glibc versions, but newer versions of fontconfig -# have broken makefiles which needlessly attempt to re-generate headers with gperf. -# Instead, change all uses of CHAR_WIDTH, and disable the rule that forces header re-generation. -# This can be removed once the upstream build is fixed. define $(package)_build_cmds - sed -i 's/CHAR_WIDTH/CHARWIDTH/g' fontconfig/fontconfig.h src/fcobjshash.gperf src/fcobjs.h src/fcobjshash.h && \ - sed -i 's/fcobjshash.h: fcobjshash.gperf/fcobjshash.h:/' src/Makefile && \ $(MAKE) endef diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk index fdbe22cda6..49a584e462 100644 --- a/depends/packages/miniupnpc.mk +++ b/depends/packages/miniupnpc.mk @@ -3,6 +3,7 @@ $(package)_version=2.0.20180203 $(package)_download_path=https://miniupnp.tuxfamily.org/files/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=90dda8c7563ca6cd4a83e23b3c66dbbea89603a1675bfdb852897c2c9cc220b7 +$(package)_patches=dont_use_wingen.patch define $(package)_set_vars $(package)_build_opts=CC="$($(package)_cc)" @@ -14,7 +15,7 @@ endef define $(package)_preprocess_cmds mkdir dll && \ sed -e 's|MINIUPNPC_VERSION_STRING \"version\"|MINIUPNPC_VERSION_STRING \"$($(package)_version)\"|' -e 's|OS/version|$(host)|' miniupnpcstrings.h.in > miniupnpcstrings.h && \ - sed -i.old "s|miniupnpcstrings.h: miniupnpcstrings.h.in wingenminiupnpcstrings|miniupnpcstrings.h: miniupnpcstrings.h.in|" Makefile.mingw + patch -p1 < $($(package)_patch_dir)/dont_use_wingen.patch endef define $(package)_build_cmds diff --git a/depends/packages/native_cctools.mk b/depends/packages/native_cctools.mk index 5022ed980f..d56b636695 100644 --- a/depends/packages/native_cctools.mk +++ b/depends/packages/native_cctools.mk @@ -4,6 +4,8 @@ $(package)_download_path=https://github.com/tpoechtrager/cctools-port/archive $(package)_file_name=$($(package)_version).tar.gz $(package)_sha256_hash=e51995a843533a3dac155dd0c71362dd471597a2d23f13dff194c6285362f875 $(package)_build_subdir=cctools +$(package)_patches=ld64_disable_threading.patch + ifeq ($(strip $(FORCE_USE_SYSTEM_CLANG)),) $(package)_clang_version=8.0.0 $(package)_clang_download_path=https://releases.llvm.org/$($(package)_clang_version) @@ -78,7 +80,7 @@ endef define $(package)_preprocess_cmds CC=$($(package)_cc) CXX=$($(package)_cxx) INSTALLPREFIX=$($(package)_extract_dir) ./libtapi/build.sh && \ CC=$($(package)_cc) CXX=$($(package)_cxx) INSTALLPREFIX=$($(package)_extract_dir) ./libtapi/install.sh && \ - sed -i.old "/define HAVE_PTHREADS/d" $($(package)_build_subdir)/ld64/src/ld/InputFiles.h + patch -p1 < $($(package)_patch_dir)/ld64_disable_threading.patch endef define $(package)_config_cmds diff --git a/depends/packages/native_cdrkit.mk b/depends/packages/native_cdrkit.mk index 14c37a0fc7..7bdf2d7dfd 100644 --- a/depends/packages/native_cdrkit.mk +++ b/depends/packages/native_cdrkit.mk @@ -12,7 +12,7 @@ endef # Starting with 10.1, GCC defaults to -fno-common, resulting in linking errors. # Pass -fcommon to retain the legacy behaviour. define $(package)_config_cmds - cmake -DCMAKE_INSTALL_PREFIX=$(build_prefix) -DCMAKE_C_FLAGS="-fcommon" + $($(package)_cmake) -DCMAKE_C_FLAGS="$$($(1)_cflags) -fcommon" endef define $(package)_build_cmds diff --git a/depends/packages/native_libdmg-hfsplus.mk b/depends/packages/native_libdmg-hfsplus.mk index c0f0ce74de..035b767188 100644 --- a/depends/packages/native_libdmg-hfsplus.mk +++ b/depends/packages/native_libdmg-hfsplus.mk @@ -12,7 +12,7 @@ define $(package)_preprocess_cmds endef define $(package)_config_cmds - cmake -DCMAKE_INSTALL_PREFIX:PATH=$(build_prefix) -DCMAKE_C_FLAGS="-Wl,--build-id=none" .. + $($(package)_cmake) -DCMAKE_C_FLAGS="$$($(1)_cflags) -Wl,--build-id=none" .. endef define $(package)_build_cmds diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 500881e442..f560099b6a 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -8,7 +8,10 @@ $(package)_dependencies=zlib $(package)_linux_dependencies=freetype fontconfig libxcb $(package)_build_subdir=qtbase $(package)_qt_libs=corelib network widgets gui plugins testlib -$(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch fix_no_printer.patch fix_rcc_determinism.patch fix_riscv64_arch.patch xkb-default.patch no-xlib.patch fix_android_qmake_conf.patch fix_android_jni_static.patch +$(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch fix_no_printer.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 # Update OSX_QT_TRANSLATIONS when this is updated $(package)_qttranslations_file_name=qttranslations-$($(package)_suffix) @@ -190,11 +193,10 @@ define $(package)_extract_cmds endef define $(package)_preprocess_cmds - sed -i.old "s|FT_Get_Font_Format|FT_Get_X11_Font_Format|" qtbase/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp && \ + patch -p1 -i $($(package)_patch_dir)/freetype_back_compat.patch && \ sed -i.old "s|updateqm.commands = \$$$$\$$$$LRELEASE|updateqm.commands = $($(package)_extract_dir)/qttools/bin/lrelease|" qttranslations/translations/translations.pro && \ - sed -i.old "/updateqm.depends =/d" qttranslations/translations/translations.pro && \ - sed -i.old "s/src_plugins.depends = src_sql src_network/src_plugins.depends = src_network/" qtbase/src/src.pro && \ - sed -i.old -e 's/if \[ "$$$$XPLATFORM_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/if \[ "$$$$BUILD_ON_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/' -e 's|/bin/pwd|pwd|' qtbase/configure && \ + patch -p1 -i $($(package)_patch_dir)/drop_lrelease_dependency.patch && \ + patch -p1 -i $($(package)_patch_dir)/dont_hardcode_pwd.patch &&\ mkdir -p qtbase/mkspecs/macx-clang-linux &&\ cp -f qtbase/mkspecs/macx-clang/Info.plist.lib qtbase/mkspecs/macx-clang-linux/ &&\ cp -f qtbase/mkspecs/macx-clang/Info.plist.app qtbase/mkspecs/macx-clang-linux/ &&\ diff --git a/depends/packages/zeromq.mk b/depends/packages/zeromq.mk index ce1b25bcf1..c93aa1a74d 100644 --- a/depends/packages/zeromq.mk +++ b/depends/packages/zeromq.mk @@ -3,6 +3,7 @@ $(package)_version=4.3.1 $(package)_download_path=https://github.com/zeromq/libzmq/releases/download/v$($(package)_version)/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=bcbabe1e2c7d0eec4ed612e10b94b112dd5f06fcefa994a0c79a45d835cd21eb +$(package)_patches=remove_libstd_link.patch define $(package)_set_vars $(package)_config_opts=--without-docs --disable-shared --disable-curve --disable-curve-keygen --disable-perf @@ -15,7 +16,8 @@ define $(package)_set_vars endef define $(package)_preprocess_cmds - cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub config + patch -p1 < $($(package)_patch_dir)/remove_libstd_link.patch && \ + cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub config endef define $(package)_config_cmds @@ -31,6 +33,5 @@ define $(package)_stage_cmds endef define $(package)_postprocess_cmds - sed -i.old "s/ -lstdc++//" lib/pkgconfig/libzmq.pc && \ rm -rf bin share lib/*.la endef diff --git a/depends/patches/bdb/clang_cxx_11.patch b/depends/patches/bdb/clang_cxx_11.patch new file mode 100644 index 0000000000..58f7ddc7d5 --- /dev/null +++ b/depends/patches/bdb/clang_cxx_11.patch @@ -0,0 +1,147 @@ +commit 3311d68f11d1697565401eee6efc85c34f022ea7 +Author: fanquake <fanquake@gmail.com> +Date: Mon Aug 17 20:03:56 2020 +0800 + + Fix C++11 compatibility + +diff --git a/dbinc/atomic.h b/dbinc/atomic.h +index 0034dcc..7c11d4a 100644 +--- a/dbinc/atomic.h ++++ b/dbinc/atomic.h +@@ -70,7 +70,7 @@ typedef struct { + * These have no memory barriers; the caller must include them when necessary. + */ + #define atomic_read(p) ((p)->value) +-#define atomic_init(p, val) ((p)->value = (val)) ++#define atomic_init_db(p, val) ((p)->value = (val)) + + #ifdef HAVE_ATOMIC_SUPPORT + +@@ -144,7 +144,7 @@ typedef LONG volatile *interlocked_val; + #define atomic_inc(env, p) __atomic_inc(p) + #define atomic_dec(env, p) __atomic_dec(p) + #define atomic_compare_exchange(env, p, o, n) \ +- __atomic_compare_exchange((p), (o), (n)) ++ __atomic_compare_exchange_db((p), (o), (n)) + static inline int __atomic_inc(db_atomic_t *p) + { + int temp; +@@ -176,7 +176,7 @@ static inline int __atomic_dec(db_atomic_t *p) + * http://gcc.gnu.org/onlinedocs/gcc-4.1.0/gcc/Atomic-Builtins.html + * which configure could be changed to use. + */ +-static inline int __atomic_compare_exchange( ++static inline int __atomic_compare_exchange_db( + db_atomic_t *p, atomic_value_t oldval, atomic_value_t newval) + { + atomic_value_t was; +@@ -206,7 +206,7 @@ static inline int __atomic_compare_exchange( + #define atomic_dec(env, p) (--(p)->value) + #define atomic_compare_exchange(env, p, oldval, newval) \ + (DB_ASSERT(env, atomic_read(p) == (oldval)), \ +- atomic_init(p, (newval)), 1) ++ atomic_init_db(p, (newval)), 1) + #else + #define atomic_inc(env, p) __atomic_inc(env, p) + #define atomic_dec(env, p) __atomic_dec(env, p) +diff --git a/mp/mp_fget.c b/mp/mp_fget.c +index 5fdee5a..0b75f57 100644 +--- a/mp/mp_fget.c ++++ b/mp/mp_fget.c +@@ -617,7 +617,7 @@ alloc: /* Allocate a new buffer header and data space. */ + + /* Initialize enough so we can call __memp_bhfree. */ + alloc_bhp->flags = 0; +- atomic_init(&alloc_bhp->ref, 1); ++ atomic_init_db(&alloc_bhp->ref, 1); + #ifdef DIAGNOSTIC + if ((uintptr_t)alloc_bhp->buf & (sizeof(size_t) - 1)) { + __db_errx(env, +@@ -911,7 +911,7 @@ alloc: /* Allocate a new buffer header and data space. */ + MVCC_MPROTECT(bhp->buf, mfp->stat.st_pagesize, + PROT_READ); + +- atomic_init(&alloc_bhp->ref, 1); ++ atomic_init_db(&alloc_bhp->ref, 1); + MUTEX_LOCK(env, alloc_bhp->mtx_buf); + alloc_bhp->priority = bhp->priority; + alloc_bhp->pgno = bhp->pgno; +diff --git a/mp/mp_mvcc.c b/mp/mp_mvcc.c +index 34467d2..f05aa0c 100644 +--- a/mp/mp_mvcc.c ++++ b/mp/mp_mvcc.c +@@ -276,7 +276,7 @@ __memp_bh_freeze(dbmp, infop, hp, bhp, need_frozenp) + #else + memcpy(frozen_bhp, bhp, SSZA(BH, buf)); + #endif +- atomic_init(&frozen_bhp->ref, 0); ++ atomic_init_db(&frozen_bhp->ref, 0); + if (mutex != MUTEX_INVALID) + frozen_bhp->mtx_buf = mutex; + else if ((ret = __mutex_alloc(env, MTX_MPOOL_BH, +@@ -428,7 +428,7 @@ __memp_bh_thaw(dbmp, infop, hp, frozen_bhp, alloc_bhp) + #endif + alloc_bhp->mtx_buf = mutex; + MUTEX_LOCK(env, alloc_bhp->mtx_buf); +- atomic_init(&alloc_bhp->ref, 1); ++ atomic_init_db(&alloc_bhp->ref, 1); + F_CLR(alloc_bhp, BH_FROZEN); + } + +diff --git a/mp/mp_region.c b/mp/mp_region.c +index e6cece9..ddbe906 100644 +--- a/mp/mp_region.c ++++ b/mp/mp_region.c +@@ -224,7 +224,7 @@ __memp_init(env, dbmp, reginfo_off, htab_buckets, max_nreg) + MTX_MPOOL_FILE_BUCKET, 0, &htab[i].mtx_hash)) != 0) + return (ret); + SH_TAILQ_INIT(&htab[i].hash_bucket); +- atomic_init(&htab[i].hash_page_dirty, 0); ++ atomic_init_db(&htab[i].hash_page_dirty, 0); + } + + /* +@@ -269,7 +269,7 @@ __memp_init(env, dbmp, reginfo_off, htab_buckets, max_nreg) + hp->mtx_hash = (mtx_base == MUTEX_INVALID) ? MUTEX_INVALID : + mtx_base + i; + SH_TAILQ_INIT(&hp->hash_bucket); +- atomic_init(&hp->hash_page_dirty, 0); ++ atomic_init_db(&hp->hash_page_dirty, 0); + #ifdef HAVE_STATISTICS + hp->hash_io_wait = 0; + hp->hash_frozen = hp->hash_thawed = hp->hash_frozen_freed = 0; +diff --git a/mutex/mut_method.c b/mutex/mut_method.c +index 2588763..5c6d516 100644 +--- a/mutex/mut_method.c ++++ b/mutex/mut_method.c +@@ -426,7 +426,7 @@ atomic_compare_exchange(env, v, oldval, newval) + MUTEX_LOCK(env, mtx); + ret = atomic_read(v) == oldval; + if (ret) +- atomic_init(v, newval); ++ atomic_init_db(v, newval); + MUTEX_UNLOCK(env, mtx); + + return (ret); +diff --git a/mutex/mut_tas.c b/mutex/mut_tas.c +index f3922e0..e40fcdf 100644 +--- a/mutex/mut_tas.c ++++ b/mutex/mut_tas.c +@@ -46,7 +46,7 @@ __db_tas_mutex_init(env, mutex, flags) + + #ifdef HAVE_SHARED_LATCHES + if (F_ISSET(mutexp, DB_MUTEX_SHARED)) +- atomic_init(&mutexp->sharecount, 0); ++ atomic_init_db(&mutexp->sharecount, 0); + else + #endif + if (MUTEX_INIT(&mutexp->tas)) { +@@ -486,7 +486,7 @@ __db_tas_mutex_unlock(env, mutex) + F_CLR(mutexp, DB_MUTEX_LOCKED); + /* Flush flag update before zeroing count */ + MEMBAR_EXIT(); +- atomic_init(&mutexp->sharecount, 0); ++ atomic_init_db(&mutexp->sharecount, 0); + } else { + DB_ASSERT(env, sharecount > 0); + MEMBAR_EXIT(); diff --git a/depends/patches/boost/unused_var_in_process.patch b/depends/patches/boost/unused_var_in_process.patch new file mode 100644 index 0000000000..722f7bb5ea --- /dev/null +++ b/depends/patches/boost/unused_var_in_process.patch @@ -0,0 +1,22 @@ +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/fontconfig/gperf_header_regen.patch b/depends/patches/fontconfig/gperf_header_regen.patch new file mode 100644 index 0000000000..7401b83d84 --- /dev/null +++ b/depends/patches/fontconfig/gperf_header_regen.patch @@ -0,0 +1,24 @@ +commit 7b6eb33ecd88768b28c67ce5d2d68a7eed5936b6 +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 25 14:34:53 2020 +0800 + + Remove rule that causes inadvertant header regeneration + + Otherwise the makefile will needlessly attempt to re-generate the + headers with gperf. This can be dropped once the upstream build is fixed. + + See #10851. + +diff --git a/src/Makefile.in b/src/Makefile.in +index f4626ad..4ae1b00 100644 +--- a/src/Makefile.in ++++ b/src/Makefile.in +@@ -903,7 +903,7 @@ fcobjshash.gperf: fcobjshash.gperf.h fcobjs.h + ' - > $@.tmp && \ + mv -f $@.tmp $@ || ( $(RM) $@.tmp && false ) + +-fcobjshash.h: fcobjshash.gperf ++fcobjshash.h: + $(AM_V_GEN) $(GPERF) -m 100 $< > $@.tmp && \ + mv -f $@.tmp $@ || ( $(RM) $@.tmp && false ) + diff --git a/depends/patches/fontconfig/remove_char_width_usage.patch b/depends/patches/fontconfig/remove_char_width_usage.patch new file mode 100644 index 0000000000..9f69081890 --- /dev/null +++ b/depends/patches/fontconfig/remove_char_width_usage.patch @@ -0,0 +1,62 @@ +commit 28165a9b078583dc8e9e5c344510e37582284cef +Author: fanquake <fanquake@gmail.com> +Date: Mon Aug 17 20:35:42 2020 +0800 + + Remove usage of CHAR_WIDTH + + CHAR_WIDTH which is reserved and clashes with glibc 2.25+ + + See #10851. + +diff --git a/fontconfig/fontconfig.h b/fontconfig/fontconfig.h +index 5c72b22..843c532 100644 +--- a/fontconfig/fontconfig.h ++++ b/fontconfig/fontconfig.h +@@ -128,7 +128,7 @@ typedef int FcBool; + #define FC_USER_CACHE_FILE ".fonts.cache-" FC_CACHE_VERSION + + /* Adjust outline rasterizer */ +-#define FC_CHAR_WIDTH "charwidth" /* Int */ ++#define FC_CHARWIDTH "charwidth" /* Int */ + #define FC_CHAR_HEIGHT "charheight"/* Int */ + #define FC_MATRIX "matrix" /* FcMatrix */ + +diff --git a/src/fcobjs.h b/src/fcobjs.h +index 1fc4f65..d27864b 100644 +--- a/src/fcobjs.h ++++ b/src/fcobjs.h +@@ -51,7 +51,7 @@ FC_OBJECT (DPI, FcTypeDouble, NULL) + FC_OBJECT (RGBA, FcTypeInteger, NULL) + FC_OBJECT (SCALE, FcTypeDouble, NULL) + FC_OBJECT (MINSPACE, FcTypeBool, NULL) +-FC_OBJECT (CHAR_WIDTH, FcTypeInteger, NULL) ++FC_OBJECT (CHARWIDTH, FcTypeInteger, NULL) + FC_OBJECT (CHAR_HEIGHT, FcTypeInteger, NULL) + FC_OBJECT (MATRIX, FcTypeMatrix, NULL) + FC_OBJECT (CHARSET, FcTypeCharSet, FcCompareCharSet) +diff --git a/src/fcobjshash.gperf b/src/fcobjshash.gperf +index 80a0237..eb4ad84 100644 +--- a/src/fcobjshash.gperf ++++ b/src/fcobjshash.gperf +@@ -44,7 +44,7 @@ int id; + "rgba",FC_RGBA_OBJECT + "scale",FC_SCALE_OBJECT + "minspace",FC_MINSPACE_OBJECT +-"charwidth",FC_CHAR_WIDTH_OBJECT ++"charwidth",FC_CHARWIDTH_OBJECT + "charheight",FC_CHAR_HEIGHT_OBJECT + "matrix",FC_MATRIX_OBJECT + "charset",FC_CHARSET_OBJECT +diff --git a/src/fcobjshash.h b/src/fcobjshash.h +index 5a4d1ea..4e66bb0 100644 +--- a/src/fcobjshash.h ++++ b/src/fcobjshash.h +@@ -284,7 +284,7 @@ FcObjectTypeLookup (register const char *str, register unsigned int len) + {(int)(long)&((struct FcObjectTypeNamePool_t *)0)->FcObjectTypeNamePool_str43,FC_CHARSET_OBJECT}, + {-1}, + #line 47 "fcobjshash.gperf" +- {(int)(long)&((struct FcObjectTypeNamePool_t *)0)->FcObjectTypeNamePool_str45,FC_CHAR_WIDTH_OBJECT}, ++ {(int)(long)&((struct FcObjectTypeNamePool_t *)0)->FcObjectTypeNamePool_str45,FC_CHARWIDTH_OBJECT}, + #line 48 "fcobjshash.gperf" + {(int)(long)&((struct FcObjectTypeNamePool_t *)0)->FcObjectTypeNamePool_str46,FC_CHAR_HEIGHT_OBJECT}, + #line 55 "fcobjshash.gperf" diff --git a/depends/patches/miniupnpc/dont_use_wingen.patch b/depends/patches/miniupnpc/dont_use_wingen.patch new file mode 100644 index 0000000000..a1cc9b50d1 --- /dev/null +++ b/depends/patches/miniupnpc/dont_use_wingen.patch @@ -0,0 +1,26 @@ +commit e8077044df239bcf0d9e9980b0e1afb9f1f5c446 +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 18 20:50:19 2020 +0800 + + Don't use wingenminiupnpcstrings when generating miniupnpcstrings.h + + The wingenminiupnpcstrings tool is used on Windows to generate version + information. This information is irrelevant for us, and trying to use + wingenminiupnpcstrings would cause builds to fail, so just don't use it. + + We should be able to drop this once we are using 2.1 or later. See + upstream commit: 9663c55c61408fdcc39a82987d2243f816b22932. + +diff --git a/Makefile.mingw b/Makefile.mingw +index 574720e..fcc17bb 100644 +--- a/Makefile.mingw ++++ b/Makefile.mingw +@@ -74,7 +74,7 @@ wingenminiupnpcstrings: wingenminiupnpcstrings.o + + wingenminiupnpcstrings.o: wingenminiupnpcstrings.c + +-miniupnpcstrings.h: miniupnpcstrings.h.in wingenminiupnpcstrings ++miniupnpcstrings.h: miniupnpcstrings.h.in + wingenminiupnpcstrings $< $@ + + minixml.o: minixml.c minixml.h diff --git a/depends/patches/native_cctools/ld64_disable_threading.patch b/depends/patches/native_cctools/ld64_disable_threading.patch new file mode 100644 index 0000000000..d6c58c102f --- /dev/null +++ b/depends/patches/native_cctools/ld64_disable_threading.patch @@ -0,0 +1,26 @@ +commit 584668415039adeed073decee7e04de28248afd3 +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 18 01:20:24 2020 +0000 + + Disable threading to fix non-determinism + + A bug in the file parser can cause dependencies to be calculated + differently based on which files have already been parsed. This is more + likely to occur on systems with more CPUs. + + Just disable threading for now. There is no noticable slowdown. + + See #9891. + +diff --git a/cctools/ld64/src/ld/InputFiles.h b/cctools/ld64/src/ld/InputFiles.h +index ef9c756..90a70b6 100644 +--- a/cctools/ld64/src/ld/InputFiles.h ++++ b/cctools/ld64/src/ld/InputFiles.h +@@ -25,7 +25,6 @@ + #ifndef __INPUT_FILES_H__ + #define __INPUT_FILES_H__ + +-#define HAVE_PTHREADS 1 + + #include <stdlib.h> + #include <sys/types.h> diff --git a/depends/patches/qt/dont_hardcode_pwd.patch b/depends/patches/qt/dont_hardcode_pwd.patch new file mode 100644 index 0000000000..a74e9cb098 --- /dev/null +++ b/depends/patches/qt/dont_hardcode_pwd.patch @@ -0,0 +1,27 @@ +commit 0e953866fc4672486e29e1ba6d83b4207e7b2f0b +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 18 15:09:06 2020 +0800 + + Don't hardcode pwd path + + Let a man use his builtins if he wants to! Also, removes the unnecessary + assumption that pwd lives under /bin/pwd. + + See #15581. + +diff --git a/qtbase/configure b/qtbase/configure +index 08b49a8d..faea5b55 100755 +--- a/qtbase/configure ++++ b/qtbase/configure +@@ -36,9 +36,9 @@ + relconf=`basename $0` + # the directory of this script is the "source tree" + relpath=`dirname $0` +-relpath=`(cd "$relpath"; /bin/pwd)` ++relpath=`(cd "$relpath"; pwd)` + # the current directory is the "build tree" or "object tree" +-outpath=`/bin/pwd` ++outpath=`pwd` + + WHICH="which" + diff --git a/depends/patches/qt/drop_lrelease_dependency.patch b/depends/patches/qt/drop_lrelease_dependency.patch new file mode 100644 index 0000000000..f6b2c9fc80 --- /dev/null +++ b/depends/patches/qt/drop_lrelease_dependency.patch @@ -0,0 +1,20 @@ +commit 67b3ed7406e1d0762188dbad2c44a06824ba0778 +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 18 15:24:01 2020 +0800 + + Drop dependency on lrelease + + Qts buildsystem insists on using the installed lrelease, but gets + confused about how to find it. Since we manually control the build + order, just drop the dependency. + + See #9469 + +diff --git a/qttranslations/translations/translations.pro b/qttranslations/translations/translations.pro +index 694544c..eff339d 100644 +--- a/qttranslations/translations/translations.pro ++++ b/qttranslations/translations/translations.pro +@@ -109,3 +109,2 @@ updateqm.commands = $$LRELEASE ${QMAKE_FILE_IN} -qm ${QMAKE_FILE_OUT} + silent:updateqm.commands = @echo lrelease ${QMAKE_FILE_IN} && $$updateqm.commands +-updateqm.depends = $$LRELEASE_EXE + updateqm.name = LRELEASE ${QMAKE_FILE_IN} diff --git a/depends/patches/qt/freetype_back_compat.patch b/depends/patches/qt/freetype_back_compat.patch new file mode 100644 index 0000000000..b0f1c98aa6 --- /dev/null +++ b/depends/patches/qt/freetype_back_compat.patch @@ -0,0 +1,28 @@ +commit 14bc77db61bf9d56f9b6c8b84aa02573605c19c6 +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 18 15:15:08 2020 +0800 + + Fix backwards compatibility with older Freetype versions at runtime + + A few years ago, libfreetype introduced FT_Get_Font_Format() as an alias + for FT_Get_X11_Font_Format(), but FT_Get_X11_Font_Format() was kept for abi + backwards-compatibility. + + Qt 5.9 introduced a call to FT_Get_Font_Format(). Replace it with FT_Get_X11_Font_Format() + in order to remain compatibile with older freetype, which is still used by e.g. Ubuntu Trusty. + + See #14348. + +diff --git a/qtbase/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp b/qtbase/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp +index 3f543755..8ecc1c8c 100644 +--- a/qtbase/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp ++++ b/qtbase/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp +@@ -898,7 +898,7 @@ bool QFontEngineFT::init(FaceId faceId, bool antialias, GlyphFormat format, + } + } + #if defined(FT_FONT_FORMATS_H) +- const char *fmt = FT_Get_Font_Format(face); ++ const char *fmt = FT_Get_X11_Font_Format(face); + if (fmt && qstrncmp(fmt, "CFF", 4) == 0) { + FT_Bool no_stem_darkening = true; + FT_Error err = FT_Property_Get(qt_getFreetype(), "cff", "no-stem-darkening", &no_stem_darkening); diff --git a/depends/patches/zeromq/remove_libstd_link.patch b/depends/patches/zeromq/remove_libstd_link.patch new file mode 100644 index 0000000000..ddf91e6abf --- /dev/null +++ b/depends/patches/zeromq/remove_libstd_link.patch @@ -0,0 +1,25 @@ +commit 47d4cd12a2c051815ddda78adebdb3923b260d8a +Author: fanquake <fanquake@gmail.com> +Date: Tue Aug 18 14:45:40 2020 +0800 + + Remove needless linking against libstdc++ + + This is broken for a number of reasons, including: + - g++ understands "static-libstdc++ -lstdc++" to mean "link against + whatever libstdc++ exists, probably shared", which in itself is buggy. + - another stdlib (libc++ for example) may be in use + + See #11981. + +diff --git a/src/libzmq.pc.in b/src/libzmq.pc.in +index 233bc3a..3c2bf0d 100644 +--- a/src/libzmq.pc.in ++++ b/src/libzmq.pc.in +@@ -7,6 +7,6 @@ Name: libzmq + Description: 0MQ c++ library + Version: @VERSION@ + Libs: -L${libdir} -lzmq +-Libs.private: -lstdc++ @pkg_config_libs_private@ ++Libs.private: @pkg_config_libs_private@ + Requires.private: @pkg_config_names_private@ + Cflags: -I${includedir} @pkg_config_defines@ diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 6ae7e770e8..ef9ecbb085 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -746,6 +746,72 @@ the upper cycle, etc. Threads and synchronization ---------------------------- +- Prefer `Mutex` type to `RecursiveMutex` one + +- Consistently use [Clang Thread Safety Analysis](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) annotations to + get compile-time warnings about potential race conditions in code. Combine annotations in function declarations with + run-time asserts in function definitions: + +```C++ +// txmempool.h +class CTxMemPool +{ +public: + ... + mutable RecursiveMutex cs; + ... + void UpdateTransactionsFromBlock(...) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, cs); + ... +} + +// txmempool.cpp +void CTxMemPool::UpdateTransactionsFromBlock(...) +{ + AssertLockHeld(::cs_main); + AssertLockHeld(cs); + ... +} +``` + +```C++ +// validation.h +class ChainstateManager +{ +public: + ... + bool ProcessNewBlock(...) EXCLUSIVE_LOCKS_REQUIRED(!::cs_main); + ... +} + +// validation.cpp +bool ChainstateManager::ProcessNewBlock(...) +{ + AssertLockNotHeld(::cs_main); + ... + LOCK(::cs_main); + ... +} +``` + +- When Clang Thread Safety Analysis is unable to determine if a mutex is locked, use `LockAssertion` class instances: + +```C++ +// net_processing.h +void RelayTransaction(...) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + +// net_processing.cpp +void RelayTransaction(...) +{ + AssertLockHeld(::cs_main); + + connman.ForEachNode([&txid, &wtxid](CNode* pnode) { + LockAssertion lock(::cs_main); + ... + }); +} + +``` + - Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential deadlocks are introduced. As of 0.12, this is defined by default when configuring with `--enable-debug`. diff --git a/doc/release-notes-18244.md b/doc/release-notes-18244.md new file mode 100644 index 0000000000..625fbaf7a1 --- /dev/null +++ b/doc/release-notes-18244.md @@ -0,0 +1,7 @@ +Updated RPCs +------------ + +- `fundrawtransaction` and `walletcreatefundedpsbt` when used with the `lockUnspents` + argument now lock manually selected coins, in addition to automatically selected + coins. Note that locked coins are never used in automatic coin selection, but + can still be manually selected. diff --git a/doc/release-notes-19671.md b/doc/release-notes-19671.md new file mode 100644 index 0000000000..fb2d56d9a5 --- /dev/null +++ b/doc/release-notes-19671.md @@ -0,0 +1,6 @@ +Wallet +------ + +* The `-zapwallettxes` startup option has been removed and its functionality removed from the wallet. + This option was originally intended to allow for the fee bumping of transactions that did not + signal RBF. This functionality has been superseded with the abandon transaction feature. diff --git a/doc/zmq.md b/doc/zmq.md index 3a1194de1c..ac26fc0a48 100644 --- a/doc/zmq.md +++ b/doc/zmq.md @@ -98,6 +98,20 @@ ZMQ_SUBSCRIBE option set to one or either of these prefixes (for instance, just `hash`); without doing so will result in no messages arriving. Please see [`contrib/zmq/zmq_sub.py`](/contrib/zmq/zmq_sub.py) for a working example. +The ZMQ_PUB socket's ZMQ_TCP_KEEPALIVE option is enabled. This means that +the underlying SO_KEEPALIVE option is enabled when using a TCP transport. +The effective TCP keepalive values are managed through the underlying +operating system configuration and must be configured prior to connection establishment. + +For example, when running on GNU/Linux, one might use the following +to lower the keepalive setting to 10 minutes: + +sudo sysctl -w net.ipv4.tcp_keepalive_time=600 + +Setting the keepalive values appropriately for your operating environment may +improve connectivity in situations where long-lived connections are silently +dropped by network middle boxes. + ## Remarks From the perspective of bitcoind, the ZeroMQ socket is write-only; PUB diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 0068c94070..91cdecad40 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -70,6 +70,7 @@ FUZZ_TARGETS = \ test/fuzz/message \ test/fuzz/messageheader_deserialize \ test/fuzz/multiplication_overflow \ + test/fuzz/net \ test/fuzz/net_permissions \ test/fuzz/netaddr_deserialize \ test/fuzz/netaddress \ @@ -132,6 +133,8 @@ FUZZ_TARGETS = \ test/fuzz/script_sigcache \ test/fuzz/script_sign \ test/fuzz/scriptnum_ops \ + test/fuzz/secp256k1_ec_seckey_import_export_der \ + test/fuzz/secp256k1_ecdsa_signature_parse_der_lax \ test/fuzz/service_deserialize \ test/fuzz/signature_checker \ test/fuzz/snapshotmetadata_deserialize \ @@ -722,6 +725,12 @@ test_fuzz_multiplication_overflow_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_multiplication_overflow_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_multiplication_overflow_SOURCES = test/fuzz/multiplication_overflow.cpp +test_fuzz_net_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_net_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_net_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_net_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_net_SOURCES = test/fuzz/net.cpp + test_fuzz_net_permissions_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_net_permissions_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_net_permissions_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -1094,6 +1103,18 @@ test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_scriptnum_ops_SOURCES = test/fuzz/scriptnum_ops.cpp +test_fuzz_secp256k1_ec_seckey_import_export_der_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_secp256k1_ec_seckey_import_export_der_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_secp256k1_ec_seckey_import_export_der_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_secp256k1_ec_seckey_import_export_der_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_secp256k1_ec_seckey_import_export_der_SOURCES = test/fuzz/secp256k1_ec_seckey_import_export_der.cpp + +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_SOURCES = test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp + test_fuzz_service_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DSERVICE_DESERIALIZE=1 test_fuzz_service_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_service_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/base58.cpp b/src/base58.cpp index 18cd2090e0..0dc6044145 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, }; -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); } -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 b87664b78b..468c3e2589 100644 --- a/src/base58.h +++ b/src/base58.h @@ -26,13 +26,6 @@ std::string EncodeBase58(Span<const unsigned char> input); /** - * Decode a base58-encoded string (psz) into a byte vector (vchRet). - * return true if decoding is successful. - * psz cannot be nullptr. - */ -NODISCARD bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len); - -/** * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ @@ -44,12 +37,6 @@ NODISCARD bool DecodeBase58(const std::string& str, std::vector<unsigned char>& std::string EncodeBase58Check(Span<const unsigned char> input); /** - * Decode a base58-encoded string (psz) that includes a checksum into a byte - * vector (vchRet), return true if decoding is successful - */ -NODISCARD bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len); - -/** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 01466d0b6f..012057e792 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -70,7 +70,10 @@ void benchmark::BenchRunner::RunAll(const Args& args) } std::cout << bench.complexityBigO() << std::endl; } - benchmarkResults.push_back(bench.results().back()); + + if (!bench.results().empty()) { + benchmarkResults.push_back(bench.results().back()); + } } GenerateTemplateResults(benchmarkResults, args.output_csv, "# Benchmark, evals, iterations, total, min, max, median\n" diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index 19d7bc0dbc..ffa772d8c1 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -14,8 +14,6 @@ #include <vector> - -static const int MIN_CORES = 2; static const size_t BATCHES = 101; static const size_t BATCH_SIZE = 30; static const int PREVECTOR_SIZE = 28; @@ -26,6 +24,9 @@ static const unsigned int QUEUE_BATCH_SIZE = 128; // and there is a little bit of work done between calls to Add. static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) { + // We shouldn't ever be running with the checkqueue on a single core machine. + if (GetNumCores() <= 1) return; + const ECCVerifyHandle verify_handle; ECC_Start(); @@ -44,7 +45,9 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) }; CCheckQueue<PrevectorJob> queue {QUEUE_BATCH_SIZE}; boost::thread_group tg; - for (auto x = 0; x < std::max(MIN_CORES, GetNumCores()); ++x) { + // The main thread should be counted to prevent thread oversubscription, and + // to decrease the variance of benchmark results. + for (auto x = 0; x < GetNumCores() - 1; ++x) { tg.create_thread([&]{queue.Thread();}); } diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 227626f40f..02074f820a 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -44,7 +44,6 @@ static void WaitForShutdown(NodeContext& node) static bool AppInit(int argc, char* argv[]) { NodeContext node; - node.chain = interfaces::MakeChain(node); bool fRet = false; @@ -144,7 +143,7 @@ static bool AppInit(int argc, char* argv[]) // If locking the data directory failed, exit immediately return false; } - fRet = AppInitMain(context, node); + fRet = AppInitInterfaces(node) && AppInitMain(context, node); } catch (const std::exception& e) { PrintExceptionContinue(&e, "AppInit()"); diff --git a/src/chain.h b/src/chain.h index 802e23f775..43e8a39f36 100644 --- a/src/chain.h +++ b/src/chain.h @@ -398,12 +398,6 @@ public: return vChain[nHeight]; } - /** Compare two chains efficiently. */ - friend bool operator==(const CChain &a, const CChain &b) { - return a.vChain.size() == b.vChain.size() && - a.vChain[a.vChain.size() - 1] == b.vChain[b.vChain.size() - 1]; - } - /** Efficiently check whether a block is present in this chain. */ bool Contains(const CBlockIndex *pindex) const { return (*this)[pindex->nHeight] == pindex; diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 380d4eb8ac..8d2dcd0279 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -4,11 +4,8 @@ #include <util/system.h> #include <walletinitinterface.h> -#include <support/allocators/secure.h> class CWallet; -enum class WalletCreationStatus; -struct bilingual_str; namespace interfaces { class Chain; @@ -49,7 +46,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-walletdir=<dir>", "-walletnotify=<cmd>", "-walletrbf", - "-zapwallettxes=<mode>", "-dblogsize=<n>", "-flushwallet", "-privdb", @@ -59,37 +55,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const const WalletInitInterface& g_wallet_init_interface = DummyWalletInit(); -fs::path GetWalletDir() -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -std::vector<fs::path> ListWalletDir() -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -std::vector<std::shared_ptr<CWallet>> GetWallets() -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result) -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>; -std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet) -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - namespace interfaces { std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) diff --git a/src/init.cpp b/src/init.cpp index ecd57960ad..bb93f5c797 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1229,6 +1229,17 @@ bool AppInitLockDataDirectory() return true; } +bool AppInitInterfaces(NodeContext& node) +{ + node.chain = interfaces::MakeChain(node); + // Create client interfaces for wallets that are supposed to be loaded + // according to -wallet and -disablewallet options. This only constructs + // the interfaces, it doesn't load wallet data. Wallets actually get loaded + // when load() and start() interface methods are called below. + g_wallet_init_interface.Construct(node); + return true; +} + bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { const ArgsManager& args = *Assert(node.args); @@ -1318,12 +1329,6 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA GetMainSignals().RegisterBackgroundSignalScheduler(*node.scheduler); - // Create client interfaces for wallets that are supposed to be loaded - // according to -wallet and -disablewallet options. This only constructs - // the interfaces, it doesn't load wallet data. Wallets actually get loaded - // when load() and start() interface methods are called below. - g_wallet_init_interface.Construct(node); - /* Register RPC commands regardless of -server setting so they will be * available in the GUI RPC console even if external calls are disabled. */ @@ -1550,7 +1555,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA const int64_t load_block_index_start_time = GetTimeMillis(); try { LOCK(cs_main); - chainman.InitializeChainstate(); + chainman.InitializeChainstate(*Assert(node.mempool)); chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; @@ -1828,20 +1833,15 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA } #if HAVE_SYSTEM - if (args.IsArgSet("-blocknotify")) { - const std::string block_notify = args.GetArg("-blocknotify", ""); - const auto BlockNotifyCallback = [block_notify](SynchronizationState sync_state, const CBlockIndex* pBlockIndex) { - if (sync_state != SynchronizationState::POST_INIT || !pBlockIndex) - return; - - std::string strCmd = block_notify; - if (!strCmd.empty()) { - boost::replace_all(strCmd, "%s", pBlockIndex->GetBlockHash().GetHex()); - std::thread t(runCommand, strCmd); - t.detach(); // thread runs free - } - }; - uiInterface.NotifyBlockTip_connect(BlockNotifyCallback); + const std::string block_notify = args.GetArg("-blocknotify", ""); + if (!block_notify.empty()) { + uiInterface.NotifyBlockTip_connect([block_notify](SynchronizationState sync_state, const CBlockIndex* pBlockIndex) { + if (sync_state != SynchronizationState::POST_INIT || !pBlockIndex) return; + std::string command = block_notify; + boost::replace_all(command, "%s", pBlockIndex->GetBlockHash().GetHex()); + std::thread t(runCommand, command); + t.detach(); // thread runs free + }); } #endif diff --git a/src/init.h b/src/init.h index ce12a80dc7..679e875da1 100644 --- a/src/init.h +++ b/src/init.h @@ -53,6 +53,10 @@ bool AppInitSanityChecks(); */ bool AppInitLockDataDirectory(); /** + * Initialize node and wallet interface pointers. Has no prerequisites or side effects besides allocating memory. + */ +bool AppInitInterfaces(NodeContext& node); +/** * Bitcoin core main initialization. * @note This should only be done after daemonization. Call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called. diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 053d40335f..6e50ccb27a 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -314,24 +314,11 @@ public: //! Set mock time. virtual void setMockTime(int64_t time) = 0; - - //! Return interfaces for accessing wallets (if any). - virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; }; //! Return implementation of Chain interface. std::unique_ptr<Chain> MakeChain(NodeContext& node); -//! Return implementation of ChainClient interface for a wallet client. This -//! function will be undefined in builds where ENABLE_WALLET is false. -//! -//! Currently, wallets are the only chain clients. But in the future, other -//! types of chain clients could be added, such as tools for monitoring, -//! analysis, or fee estimation. These clients need to expose their own -//! MakeXXXClient functions returning their implementations of the ChainClient -//! interface. -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames); - } // namespace interfaces #endif // BITCOIN_INTERFACES_CHAIN_H diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 206262eb03..2c5f8627e6 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -27,6 +27,7 @@ #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> @@ -41,16 +42,7 @@ #include <boost/signals2/signal.hpp> -class CWallet; -fs::path GetWalletDir(); -std::vector<fs::path> ListWalletDir(); -std::vector<std::shared_ptr<CWallet>> GetWallets(); -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings); -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result); -std::unique_ptr<interfaces::Handler> HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet); - namespace interfaces { - namespace { class NodeImpl : public Node @@ -64,11 +56,10 @@ public: bool baseInitialize() override { return AppInitBasicSetup(gArgs) && AppInitParameterInteraction(gArgs) && AppInitSanityChecks() && - AppInitLockDataDirectory(); + AppInitLockDataDirectory() && AppInitInterfaces(*m_context); } bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override { - m_context->chain = MakeChain(*m_context); return AppInitMain(m_context_ref, *m_context, tip_info); } void appShutdown() override @@ -240,36 +231,9 @@ public: LOCK(::cs_main); return ::ChainstateActive().CoinsTip().GetCoin(output, coin); } - std::string getWalletDir() override - { - return GetWalletDir().string(); - } - std::vector<std::string> listWalletDir() override - { - std::vector<std::string> paths; - for (auto& path : ListWalletDir()) { - paths.push_back(path.string()); - } - return paths; - } - std::vector<std::unique_ptr<Wallet>> getWallets() override + WalletClient& walletClient() override { - std::vector<std::unique_ptr<Wallet>> wallets; - for (auto& client : m_context->chain_clients) { - auto client_wallets = client->getWallets(); - std::move(client_wallets.begin(), client_wallets.end(), std::back_inserter(wallets)); - } - return wallets; - } - std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override - { - return MakeWallet(LoadWallet(*m_context->chain, name, error, warnings)); - } - std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) override - { - std::shared_ptr<CWallet> wallet; - status = CreateWallet(*m_context->chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); - return MakeWallet(wallet); + return *Assert(m_context->wallet_client); } std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override { @@ -287,10 +251,6 @@ public: { return MakeHandler(::uiInterface.ShowProgress_connect(fn)); } - std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override - { - return HandleLoadWallet(std::move(fn)); - } std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override { return MakeHandler(::uiInterface.NotifyNumConnectionsChanged_connect(fn)); diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 0cff7ae3a1..5079be038e 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -29,14 +29,13 @@ class RPCTimerInterface; class UniValue; class proxyType; enum class SynchronizationState; -enum class WalletCreationStatus; struct CNodeStateStats; struct NodeContext; struct bilingual_str; namespace interfaces { class Handler; -class Wallet; +class WalletClient; struct BlockTip; //! Block and header tip information @@ -173,22 +172,8 @@ public: //! Get unspent outputs associated with a transaction. virtual bool getUnspentOutput(const COutPoint& output, Coin& coin) = 0; - //! Return default wallet directory. - virtual std::string getWalletDir() = 0; - - //! Return available wallets in wallet directory. - virtual std::vector<std::string> listWalletDir() = 0; - - //! Return interfaces for accessing wallets (if any). - virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; - - //! Attempts to load a wallet from file or directory. - //! The loaded wallet is also notified to handlers previously registered - //! with handleLoadWallet. - virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; - - //! Create a wallet from file - virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) = 0; + //! Get wallet client. + virtual WalletClient& walletClient() = 0; //! Register handler for init messages. using InitMessageFn = std::function<void(const std::string& message)>; @@ -210,10 +195,6 @@ public: using ShowProgressFn = std::function<void(const std::string& title, int progress, bool resume_possible)>; virtual std::unique_ptr<Handler> handleShowProgress(ShowProgressFn fn) = 0; - //! Register handler for load wallet messages. - using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>; - virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0; - //! Register handler for number of connections changed messages. using NotifyNumConnectionsChangedFn = std::function<void(int new_num_connections)>; virtual std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 7fd24425cf..63c109658e 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -37,6 +37,7 @@ namespace { //! Construct wallet tx struct. WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) { + LOCK(wallet.cs_wallet); WalletTx result; result.tx = wtx.tx; result.txin_is_mine.reserve(wtx.tx->vin.size()); @@ -132,7 +133,11 @@ public: { return m_wallet->SignMessage(message, pkhash, str_sig); } - bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; } + bool isSpendable(const CTxDestination& dest) override + { + LOCK(m_wallet->cs_wallet); + return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; + } bool haveWatchOnly() override { auto spk_man = m_wallet->GetLegacyScriptPubKeyMan(); @@ -480,7 +485,7 @@ public: std::shared_ptr<CWallet> m_wallet; }; -class WalletClientImpl : public ChainClient +class WalletClientImpl : public WalletClient { public: WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) @@ -489,6 +494,9 @@ public: m_context.chain = &chain; m_context.args = &args; } + ~WalletClientImpl() override { UnloadWallets(); } + + //! ChainClient methods void registerRpcs() override { for (const CRPCCommand& command : GetWalletRPCCommands()) { @@ -504,6 +512,30 @@ public: void flush() override { return FlushWallets(); } void stop() override { return StopWallets(); } void setMockTime(int64_t time) override { return SetMockTime(time); } + + //! WalletClient methods + std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) override + { + std::shared_ptr<CWallet> wallet; + status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); + return MakeWallet(std::move(wallet)); + } + std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override + { + return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), error, warnings)); + } + std::string getWalletDir() override + { + return GetWalletDir().string(); + } + std::vector<std::string> listWalletDir() override + { + std::vector<std::string> paths; + for (auto& path : ListWalletDir()) { + paths.push_back(path.string()); + } + return paths; + } std::vector<std::unique_ptr<Wallet>> getWallets() override { std::vector<std::unique_ptr<Wallet>> wallets; @@ -512,7 +544,10 @@ public: } return wallets; } - ~WalletClientImpl() override { UnloadWallets(); } + std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override + { + return HandleLoadWallet(std::move(fn)); + } WalletContext m_context; const std::vector<std::string> m_wallet_filenames; @@ -524,7 +559,7 @@ public: std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; } -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) +std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) { return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames)); } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 3cdadbc72e..186f5d81a5 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -6,6 +6,7 @@ #define BITCOIN_INTERFACES_WALLET_H #include <amount.h> // For CAmount +#include <interfaces/chain.h> // For ChainClient #include <pubkey.h> // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation) #include <script/standard.h> // For CTxDestination #include <support/allocators/secure.h> // For SecureString @@ -28,9 +29,11 @@ class CWallet; enum class FeeReason; enum class OutputType; enum class TransactionError; +enum class WalletCreationStatus; enum isminetype : unsigned int; struct CRecipient; struct PartiallySignedTransaction; +struct WalletContext; struct bilingual_str; typedef uint8_t isminefilter; @@ -301,6 +304,34 @@ public: virtual CWallet* wallet() { return nullptr; } }; +//! Wallet chain client that in addition to having chain client methods for +//! starting up, shutting down, and registering RPCs, also has additional +//! methods (called by the GUI) to load and create wallets. +class WalletClient : public ChainClient +{ +public: + //! Create new wallet. + virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; + + //! Load existing wallet. + virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; + + //! Return default wallet directory. + virtual std::string getWalletDir() = 0; + + //! Return available wallets in wallet directory. + virtual std::vector<std::string> listWalletDir() = 0; + + //! Return interfaces for accessing wallets (if any). + virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; + + //! Register handler for load wallet messages. This callback is triggered by + //! createWallet and loadWallet above, and also triggered when wallets are + //! loaded at startup or by RPC. + using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>; + virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0; +}; + //! Information about one wallet address. struct WalletAddress { @@ -379,6 +410,10 @@ struct WalletTxOut //! dummywallet.cpp and throws if the wallet component is not compiled. std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet); +//! Return implementation of ChainClient interface for a wallet client. This +//! function will be undefined in builds where ENABLE_WALLET is false. +std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames); + } // namespace interfaces #endif // BITCOIN_INTERFACES_WALLET_H diff --git a/src/key.cpp b/src/key.cpp index 4ed74a39b1..868a8b9b0e 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -31,7 +31,7 @@ static secp256k1_context* secp256k1_context_sign = nullptr; * * out32 must point to an output buffer of length at least 32 bytes. */ -static int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *seckey, size_t seckeylen) { +int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *seckey, size_t seckeylen) { const unsigned char *end = seckey + seckeylen; memset(out32, 0, 32); /* sequence header */ @@ -88,7 +88,7 @@ static int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out * will be set to the number of bytes used in the buffer. * key32 must point to a 32-byte raw private key. */ -static int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, size_t *seckeylen, const unsigned char *key32, bool compressed) { +int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, size_t *seckeylen, const unsigned char *key32, bool compressed) { assert(*seckeylen >= CKey::SIZE); secp256k1_pubkey pubkey; size_t pubkeylen = 0; diff --git a/src/net.cpp b/src/net.cpp index 883e57bdf0..dc34743c5b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -904,10 +904,10 @@ bool CConnman::AttemptToEvictConnection() // Protect the 8 nodes with the lowest minimum ping time. // An attacker cannot manipulate this metric without physically moving nodes closer to the target. EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8); - // Protect 4 nodes that most recently sent us transactions. + // Protect 4 nodes that most recently sent us novel transactions accepted into our mempool. // An attacker cannot manipulate this metric without performing useful work. EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); - // Protect 4 nodes that most recently sent us blocks. + // Protect 4 nodes that most recently sent us novel blocks. // An attacker cannot manipulate this metric without performing useful work. EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); // Protect the half of the remaining nodes which have been connected the longest. @@ -932,8 +932,17 @@ public: // Used for headers announcements - unfiltered blocks to relay std::vector<uint256> vBlockHashesToAnnounce GUARDED_BY(cs_inventory); - // Block and TXN accept times + /** UNIX epoch time of the last block received from this peer that we had + * not yet seen (e.g. not already received from another peer), that passed + * preliminary validity checks and was saved to disk, even if we don't + * connect the block or it eventually fails connection. Used as an inbound + * peer eviction criterium in CConnman::AttemptToEvictConnection. */ std::atomic<int64_t> nLastBlockTime{0}; + + /** UNIX epoch time of the last transaction received from this peer that we + * had not yet seen (e.g. not already received from another peer) and that + * was accepted into our mempool. Used as an inbound peer eviction criterium + * in CConnman::AttemptToEvictConnection. */ std::atomic<int64_t> nLastTXTime{0}; // Ping time measurement: diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 60bdfbe9f5..0e049bd666 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -278,12 +278,6 @@ struct CNodeState { const CService address; //! Whether we have a fully established connection. bool fCurrentlyConnected; - //! Accumulated misbehaviour score for this peer. - int nMisbehavior; - //! Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). - bool m_should_discourage; - //! String name of this peer (debugging/logging purposes). - const std::string name; //! The best known block we know this peer has announced. const CBlockIndex *pindexBestKnownBlock; //! The hash of the last unknown block this peer has announced. @@ -432,13 +426,10 @@ struct CNodeState { //! Whether this peer relays txs via wtxid bool m_wtxid_relay{false}; - CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) : - address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound), - m_is_manual_connection (is_manual) + CNodeState(CAddress addrIn, bool is_inbound, bool is_manual) + : address(addrIn), m_is_inbound(is_inbound), m_is_manual_connection(is_manual) { fCurrentlyConnected = false; - nMisbehavior = 0; - m_should_discourage = false; pindexBestKnownBlock = nullptr; hashLastUnknownBlock.SetNull(); pindexLastCommonBlock = nullptr; @@ -476,6 +467,50 @@ static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return &it->second; } +/** + * Data structure for an individual peer. This struct is not protected by + * cs_main since it does not contain validation-critical data. + * + * Memory is owned by shared pointers and this object is destructed when + * the refcount drops to zero. + * + * TODO: move most members from CNodeState to this structure. + * TODO: move remaining application-layer data members from CNode to this structure. + */ +struct Peer { + /** Same id as the CNode object for this peer */ + const NodeId m_id{0}; + + /** Protects misbehavior data members */ + Mutex m_misbehavior_mutex; + /** Accumulated misbehavior score for this peer */ + int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0}; + /** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */ + bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false}; + + Peer(NodeId id) : m_id(id) {} +}; + +using PeerRef = std::shared_ptr<Peer>; + +/** + * Map of all Peer objects, keyed by peer id. This map is protected + * by the global g_peer_mutex. Once a shared pointer reference is + * taken, the lock may be released. Individual fields are protected by + * their own locks. + */ +Mutex g_peer_mutex; +static std::map<NodeId, PeerRef> g_peer_map GUARDED_BY(g_peer_mutex); + +/** Get a shared pointer to the Peer object. + * May return nullptr if the Peer object can't be found. */ +static PeerRef GetPeerRef(NodeId id) +{ + LOCK(g_peer_mutex); + auto it = g_peer_map.find(id); + return it != g_peer_map.end() ? it->second : nullptr; +} + static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { nPreferredDownload -= state->fPreferredDownload; @@ -628,13 +663,12 @@ static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman& connma } } connman.ForNode(nodeid, [&connman](CNode* pfrom){ - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1; if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { // As per BIP152, we only get 3 of our peers to announce // blocks using compact encodings. connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [&connman, nCMPCTBLOCKVersion](CNode* pnodeStop){ - AssertLockHeld(cs_main); connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); return true; }); @@ -841,7 +875,12 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { NodeId nodeid = pnode->GetId(); { LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->IsInboundConn(), pnode->IsManualConn())); + mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, pnode->IsInboundConn(), pnode->IsManualConn())); + } + { + PeerRef peer = std::make_shared<Peer>(nodeid); + LOCK(g_peer_mutex); + g_peer_map.emplace_hint(g_peer_map.end(), nodeid, std::move(peer)); } if(!pnode->IsInboundConn()) PushNodeVersion(*pnode, m_connman, GetTime()); @@ -870,13 +909,21 @@ void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) { fUpdateConnectionTime = false; LOCK(cs_main); + int misbehavior{0}; + { + PeerRef peer = GetPeerRef(nodeid); + assert(peer != nullptr); + misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); + LOCK(g_peer_mutex); + g_peer_map.erase(nodeid); + } CNodeState *state = State(nodeid); assert(state != nullptr); if (state->fSyncStarted) nSyncStarted--; - if (state->nMisbehavior == 0 && state->fCurrentlyConnected) { + if (misbehavior == 0 && state->fCurrentlyConnected) { fUpdateConnectionTime = true; } @@ -906,17 +953,23 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim } bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) { - LOCK(cs_main); - CNodeState *state = State(nodeid); - if (state == nullptr) - return false; - stats.nMisbehavior = state->nMisbehavior; - stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1; - stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1; - for (const QueuedBlock& queue : state->vBlocksInFlight) { - if (queue.pindex) - stats.vHeightInFlight.push_back(queue.pindex->nHeight); + { + LOCK(cs_main); + CNodeState* state = State(nodeid); + if (state == nullptr) + return false; + stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1; + stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1; + for (const QueuedBlock& queue : state->vBlocksInFlight) { + if (queue.pindex) + stats.vHeightInFlight.push_back(queue.pindex->nHeight); + } } + + PeerRef peer = GetPeerRef(nodeid); + if (peer == nullptr) return false; + stats.m_misbehavior_score = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); + return true; } @@ -1060,21 +1113,21 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. */ -void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) { assert(howmuch > 0); - CNodeState* const state = State(pnode); - if (state == nullptr) return; + PeerRef peer = GetPeerRef(pnode); + if (peer == nullptr) return; - state->nMisbehavior += howmuch; + LOCK(peer->m_misbehavior_mutex); + peer->m_misbehavior_score += howmuch; const std::string message_prefixed = message.empty() ? "" : (": " + message); - if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) - { - LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed); - state->m_should_discourage = true; + if (peer->m_misbehavior_score >= DISCOURAGEMENT_THRESHOLD && peer->m_misbehavior_score - howmuch < DISCOURAGEMENT_THRESHOLD) { + LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed); + peer->m_should_discourage = true; } else { - LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed); + LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed); } } @@ -1096,7 +1149,6 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s case BlockValidationResult::BLOCK_CONSENSUS: case BlockValidationResult::BLOCK_MUTATED: if (!via_compact_block) { - LOCK(cs_main); Misbehaving(nodeid, 100, message); return true; } @@ -1120,18 +1172,12 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s case BlockValidationResult::BLOCK_INVALID_HEADER: case BlockValidationResult::BLOCK_CHECKPOINT: case BlockValidationResult::BLOCK_INVALID_PREV: - { - LOCK(cs_main); - Misbehaving(nodeid, 100, message); - } + Misbehaving(nodeid, 100, message); return true; // Conflicting (but not necessarily invalid) data or different policy: case BlockValidationResult::BLOCK_MISSING_PREV: - { - // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) - LOCK(cs_main); - Misbehaving(nodeid, 10, message); - } + // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) + Misbehaving(nodeid, 10, message); return true; case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE: case BlockValidationResult::BLOCK_TIME_FUTURE: @@ -1155,11 +1201,8 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, break; // The node is providing invalid data: case TxValidationResult::TX_CONSENSUS: - { - LOCK(cs_main); - Misbehaving(nodeid, 100, message); - return true; - } + Misbehaving(nodeid, 100, message); + return true; // Conflicting (but not necessarily invalid) data or different policy: case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE: case TxValidationResult::TX_INPUTS_NOT_STANDARD: @@ -1327,7 +1370,7 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: } m_connman.ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); // TODO: Avoid the repeated-serialization here if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) @@ -1422,54 +1465,48 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidatio // -bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool static AlreadyHaveTx(const GenTxid& gtxid, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - switch (inv.type) - { - case MSG_TX: - case MSG_WITNESS_TX: - case MSG_WTX: - { - assert(recentRejects); - if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) - { - // If the chain tip has changed previously rejected transactions - // might be now valid, e.g. due to a nLockTime'd tx becoming valid, - // or a double-spend. Reset the rejects filter and give those - // txs a second chance. - hashRecentRejectsChainTip = ::ChainActive().Tip()->GetBlockHash(); - recentRejects->reset(); - } - - { - LOCK(g_cs_orphans); - if (!inv.IsMsgWtx() && mapOrphanTransactions.count(inv.hash)) { - return true; - } else if (inv.IsMsgWtx() && g_orphans_by_wtxid.count(inv.hash)) { - return true; - } - } + assert(recentRejects); + if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) { + // If the chain tip has changed previously rejected transactions + // might be now valid, e.g. due to a nLockTime'd tx becoming valid, + // or a double-spend. Reset the rejects filter and give those + // txs a second chance. + hashRecentRejectsChainTip = ::ChainActive().Tip()->GetBlockHash(); + recentRejects->reset(); + } - { - LOCK(g_cs_recent_confirmed_transactions); - if (g_recent_confirmed_transactions->contains(inv.hash)) return true; - } + const uint256& hash = gtxid.GetHash(); - return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); + { + LOCK(g_cs_orphans); + if (!gtxid.IsWtxid() && mapOrphanTransactions.count(hash)) { + return true; + } else if (gtxid.IsWtxid() && g_orphans_by_wtxid.count(hash)) { + return true; } - case MSG_BLOCK: - case MSG_WITNESS_BLOCK: - return LookupBlockIndex(inv.hash) != nullptr; } - // Don't know what it is, just say we already got one - return true; + + { + LOCK(g_cs_recent_confirmed_transactions); + if (g_recent_confirmed_transactions->contains(hash)) return true; + } + + return recentRejects->contains(hash) || mempool.exists(gtxid); +} + +bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + return LookupBlockIndex(block_hash) != nullptr; } void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) { connman.ForEachNode([&txid, &wtxid](CNode* pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); + CNodeState &state = *State(pnode->GetId()); if (state.m_wtxid_relay) { pnode->PushTxInventory(wtxid); @@ -1564,7 +1601,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // disconnect node in case we have reached the outbound limit for serving historical blocks if (send && connman.OutboundTargetReached(true) && - (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && + (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); @@ -1590,7 +1627,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c std::shared_ptr<const CBlock> pblock; if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { pblock = a_recent_block; - } else if (inv.type == MSG_WITNESS_BLOCK) { + } else if (inv.IsMsgWitnessBlk()) { // Fast-path: in this case it is possible to serve the block directly from disk, // as the network format matches the format on disk std::vector<uint8_t> block_data; @@ -1607,12 +1644,11 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c pblock = pblockRead; } if (pblock) { - if (inv.type == MSG_BLOCK) + if (inv.IsMsgBlk()) { connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_WITNESS_BLOCK) + } else if (inv.IsMsgWitnessBlk()) { connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_FILTERED_BLOCK) - { + } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; if (pfrom.m_tx_relay != nullptr) { @@ -1636,9 +1672,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } // else // no response - } - else if (inv.type == MSG_CMPCT_BLOCK) - { + } else if (inv.IsMsgCmpctBlk()) { // If a peer is asking for old blocks, we're almost guaranteed // they won't have a useful mempool to match against a compact block, // and we don't feel like constructing the object for them, so @@ -1766,7 +1800,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm // expensive to process. if (it != pfrom.vRecvGetData.end() && !pfrom.fPauseSend) { const CInv &inv = *it++; - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK || inv.type == MSG_WITNESS_BLOCK) { + if (inv.IsGenBlkMsg()) { ProcessGetBlockData(pfrom, chainparams, inv, connman); } // else: If the first item on the queue is an unknown type, we erase it @@ -1806,7 +1840,6 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac BlockTransactions resp(req); for (size_t i = 0; i < req.indexes.size(); i++) { if (req.indexes[i] >= block.vtx.size()) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 100, "getblocktxn with out-of-bounds tx indices"); return; } @@ -2318,7 +2351,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // Each connection can only send one version message if (pfrom.nVersion != 0) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 1, "redundant version message"); return; } @@ -2478,7 +2510,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (pfrom.nVersion == 0) { // Must have a version message before anything else - LOCK(cs_main); Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake"); return; } @@ -2545,7 +2576,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else - LOCK(cs_main); Misbehaving(pfrom.GetId(), 1, "non-verack message before version handshake"); return; } @@ -2559,7 +2589,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } if (vAddr.size() > MAX_ADDR_TO_SEND) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size())); return; } @@ -2638,7 +2667,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("inv message size = %u", vInv.size())); return; } @@ -2654,14 +2682,11 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty LOCK(cs_main); - uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime<std::chrono::microseconds>(); uint256* best_block{nullptr}; - for (CInv &inv : vInv) - { - if (interruptMsgProc) - return; + for (CInv& inv : vInv) { + if (interruptMsgProc) return; // Ignore INVs that don't match wtxidrelay setting. // Note that orphan parent fetching always uses MSG_TX GETDATAs regardless of the wtxidrelay setting. @@ -2672,14 +2697,10 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (inv.IsMsgWtx()) continue; } - bool fAlreadyHave = AlreadyHave(inv, m_mempool); - LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); + if (inv.IsMsgBlk()) { + const bool fAlreadyHave = AlreadyHaveBlock(inv.hash); + LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - if (inv.IsMsgTx()) { - inv.type |= nFetchFlags; - } - - if (inv.type == MSG_BLOCK) { UpdateBlockAvailability(pfrom.GetId(), inv.hash); if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) { // Headers-first is the primary method of announcement on @@ -2689,15 +2710,21 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // then fetch the blocks we need to catch up. best_block = &inv.hash; } - } else { + } else if (inv.IsGenTxMsg()) { + const GenTxid gtxid = ToGenTxid(inv); + const bool fAlreadyHave = AlreadyHaveTx(gtxid, mempool); + LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); + pfrom.AddKnownTx(inv.hash); if (fBlocksOnly) { LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId()); pfrom.fDisconnect = true; return; } else if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { - RequestTx(State(pfrom.GetId()), ToGenTxid(inv), current_time); + RequestTx(State(pfrom.GetId()), gtxid, current_time); } + } else { + LogPrint(BCLog::NET, "Unknown inv type \"%s\" received from peer=%d\n", inv.ToString(), pfrom.GetId()); } } @@ -2714,7 +2741,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("getdata message size = %u", vInv.size())); return; } @@ -2969,7 +2995,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (!AlreadyHave(CInv(MSG_WTX, wtxid), m_mempool) && + if (!AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool) && AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { m_mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman); @@ -3013,7 +3039,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } } if (!fRejectedParents) { - uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime<std::chrono::microseconds>(); for (const uint256& parent_txid : unique_parents) { @@ -3022,9 +3047,9 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - CInv _inv(MSG_TX | nFetchFlags, parent_txid); + const GenTxid gtxid{/* is_wtxid=*/false, parent_txid}; pfrom.AddKnownTx(parent_txid); - if (!AlreadyHave(_inv, m_mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); + if (!AlreadyHaveTx(gtxid, m_mempool)) RequestTx(State(pfrom.GetId()), gtxid, current_time); } AddOrphanTx(ptx, pfrom.GetId()); @@ -3439,7 +3464,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks. unsigned int nCount = ReadCompactSize(vRecv); if (nCount > MAX_HEADERS_RESULTS) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount)); return; } @@ -3641,7 +3665,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (!filter.IsWithinSizeConstraints()) { // There is no excuse for sending a too-large filter - LOCK(cs_main); Misbehaving(pfrom.GetId(), 100, "too-large bloom filter"); } else if (pfrom.m_tx_relay != nullptr) @@ -3675,7 +3698,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } } if (bad) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 100, "bad filteradd message"); } return; @@ -3761,15 +3783,17 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode) { const NodeId peer_id{pnode.GetId()}; + PeerRef peer = GetPeerRef(peer_id); + if (peer == nullptr) return false; + { - LOCK(cs_main); - CNodeState& state = *State(peer_id); + LOCK(peer->m_misbehavior_mutex); // There's nothing to do if the m_should_discourage flag isn't set - if (!state.m_should_discourage) return false; + if (!peer->m_should_discourage) return false; - state.m_should_discourage = false; - } // cs_main + peer->m_should_discourage = false; + } // peer.m_misbehavior_mutex if (pnode.HasPermission(PF_NOBAN)) { // We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag @@ -3957,7 +3981,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) int64_t oldest_block_announcement = std::numeric_limits<int64_t>::max(); m_connman.ForEachNode([&](CNode* pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); // Ignore non-outbound peers, or nodes marked for disconnect already if (!pnode->IsOutboundOrBlockRelayConn() || pnode->fDisconnect) return; @@ -3974,7 +3998,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) }); if (worst_peer != -1) { bool disconnected = m_connman.ForNode(worst_peer, [&](CNode *pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); // Only disconnect a peer that has been connected to us for // some reasonable fraction of our check-frequency, to give @@ -4575,7 +4599,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); CInv inv(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), gtxid.GetHash()); - if (!AlreadyHave(inv, m_mempool)) { + if (!AlreadyHaveTx(ToGenTxid(inv), m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. const auto last_request_time = GetTxRequestTime(gtxid); diff --git a/src/net_processing.h b/src/net_processing.h index 74d6603747..4f1c52fc17 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -97,7 +97,7 @@ private: }; struct CNodeStateStats { - int nMisbehavior = 0; + int m_misbehavior_score = 0; int nSyncHeight = -1; int nCommonHeight = -1; std::vector<int> vHeightInFlight; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 8adfe38dc9..b50cf74069 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -270,17 +270,6 @@ bool CNetAddr::IsLocal() const */ bool CNetAddr::IsValid() const { - // Cleanup 3-byte shifted addresses caused by garbage in size field - // of addr messages from versions before 0.2.9 checksum. - // Two consecutive addr messages look like this: - // header20 vectorlen3 addr26 addr26 addr26 header20 vectorlen3 addr26 addr26 addr26... - // so if the first length field is garbled, it reads the second batch - // of addr misaligned by 3 bytes. - if (IsIPv6() && memcmp(m_addr.data(), IPV4_IN_IPV6_PREFIX.data() + 3, - sizeof(IPV4_IN_IPV6_PREFIX) - 3) == 0) { - return false; - } - // unspecified IPv6 address (::/128) unsigned char ipNone6[16] = {}; if (IsIPv6() && memcmp(m_addr.data(), ipNone6, sizeof(ipNone6)) == 0) { diff --git a/src/node/context.h b/src/node/context.h index be568cba36..793c9dfc34 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -20,6 +20,7 @@ class PeerLogicValidation; namespace interfaces { class Chain; class ChainClient; +class WalletClient; } // namespace interfaces //! NodeContext struct containing references to chain state and connection @@ -40,7 +41,11 @@ struct NodeContext { std::unique_ptr<BanMan> banman; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct std::unique_ptr<interfaces::Chain> chain; + //! List of all chain clients (wallet processes or other client) connected to node. std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients; + //! Reference to chain client that should used to load or create wallets + //! opened by the gui. + interfaces::WalletClient* wallet_client{nullptr}; std::unique_ptr<CScheduler> scheduler; std::function<void()> rpc_interruption_point = [] {}; diff --git a/src/protocol.h b/src/protocol.h index 2e6c767cdd..a059bf482a 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -247,7 +247,7 @@ extern const char* CFCHECKPT; * txid. * @since protocol version 70016 as described by BIP 339. */ -extern const char *WTXIDRELAY; +extern const char* WTXIDRELAY; }; // namespace NetMsgType /* Get a vector of all valid message types (see above) */ @@ -418,12 +418,22 @@ public: std::string ToString() const; // Single-message helper methods - bool IsMsgTx() const { return type == MSG_TX; } - bool IsMsgWtx() const { return type == MSG_WTX; } - bool IsMsgWitnessTx() const { return type == MSG_WITNESS_TX; } + bool IsMsgTx() const { return type == MSG_TX; } + bool IsMsgBlk() const { return type == MSG_BLOCK; } + bool IsMsgWtx() const { return type == MSG_WTX; } + bool IsMsgFilteredBlk() const { return type == MSG_FILTERED_BLOCK; } + bool IsMsgCmpctBlk() const { return type == MSG_CMPCT_BLOCK; } + bool IsMsgWitnessBlk() const { return type == MSG_WITNESS_BLOCK; } // Combined-message helper methods - bool IsGenTxMsg() const { return type == MSG_TX || type == MSG_WTX || type == MSG_WITNESS_TX; } + bool IsGenTxMsg() const + { + return type == MSG_TX || type == MSG_WTX || type == MSG_WITNESS_TX; + } + bool IsGenBlkMsg() const + { + return type == MSG_BLOCK || type == MSG_FILTERED_BLOCK || type == MSG_CMPCT_BLOCK || type == MSG_WITNESS_BLOCK; + } int type; uint256 hash; diff --git a/src/pubkey.cpp b/src/pubkey.cpp index ef42aa5bc7..fc14f41a0c 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -24,7 +24,7 @@ secp256k1_context* secp256k1_context_verify = nullptr; * strict DER before being passed to this module, and we know it supports all * violations present in the blockchain before that point. */ -static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) { +int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) { size_t rpos, rlen, spos, slen; size_t pos = 0; size_t lenbyte; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index e63ffdfb36..eaaaaeb904 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -206,6 +206,7 @@ BitcoinApplication::BitcoinApplication(): returnValue(0), platformStyle(nullptr) { + // Qt runs setlocale(LC_ALL, "") on initialization. RegisterMetaTypes(); setQuitOnLastWindowClosed(false); } @@ -266,6 +267,7 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle) // We don't hold a direct pointer to the splash screen after creation, but the splash // screen will take care of deleting itself when finish() happens. m_splash->show(); + connect(this, &BitcoinApplication::requestedInitialize, m_splash, &SplashScreen::handleLoadWallet); connect(this, &BitcoinApplication::splashFinished, m_splash, &SplashScreen::finish); connect(this, &BitcoinApplication::requestedShutdown, m_splash, &QWidget::close); } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 8935ff19bf..aa58c0b10e 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1189,7 +1189,7 @@ void BitcoinGUI::incomingTransaction(const QString& date, int unit, const CAmoun // On new transaction, make an info balloon QString msg = tr("Date: %1\n").arg(date) + tr("Amount: %1\n").arg(BitcoinUnits::formatWithUnit(unit, amount, true)); - if (m_node.getWallets().size() > 1 && !walletName.isEmpty()) { + if (m_node.walletClient().getWallets().size() > 1 && !walletName.isEmpty()) { msg += tr("Wallet: %1\n").arg(walletName); } msg += tr("Type: %1\n").arg(type); diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 93840b4169..d210faec03 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -290,7 +290,7 @@ <item row="11" column="0"> <widget class="QLabel" name="label_3"> <property name="text"> - <string>Current number of blocks</string> + <string>Current block height</string> </property> </widget> </item> diff --git a/src/qt/forms/receivecoinsdialog.ui b/src/qt/forms/receivecoinsdialog.ui index 7dbee6d689..06d39426c9 100644 --- a/src/qt/forms/receivecoinsdialog.ui +++ b/src/qt/forms/receivecoinsdialog.ui @@ -114,6 +114,12 @@ <iconset resource="../bitcoin.qrc"> <normaloff>:/icons/receiving_addresses</normaloff>:/icons/receiving_addresses</iconset> </property> + <property name="autoDefault"> + <bool>false</bool> + </property> + <property name="default"> + <bool>true</bool> + </property> </widget> </item> <item> diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index 5debded4ea..d374d610ee 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -242,22 +242,6 @@ void ReceiveCoinsDialog::resizeEvent(QResizeEvent *event) columnResizingFixer->stretchColumnWidth(RecentRequestsTableModel::Message); } -void ReceiveCoinsDialog::keyPressEvent(QKeyEvent *event) -{ - if (event->key() == Qt::Key_Return) - { - // press return -> submit form - if (ui->reqLabel->hasFocus() || ui->reqAmount->hasFocus() || ui->reqMessage->hasFocus()) - { - event->ignore(); - on_receiveButton_clicked(); - return; - } - } - - this->QDialog::keyPressEvent(event); -} - QModelIndex ReceiveCoinsDialog::selectedRow() { if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel()) diff --git a/src/qt/receivecoinsdialog.h b/src/qt/receivecoinsdialog.h index 2f48cd58f0..27455e2906 100644 --- a/src/qt/receivecoinsdialog.h +++ b/src/qt/receivecoinsdialog.h @@ -49,9 +49,6 @@ public Q_SLOTS: void reject() override; void accept() override; -protected: - virtual void keyPressEvent(QKeyEvent *event) override; - private: Ui::ReceiveCoinsDialog *ui; GUIUtil::TableViewLastColumnResizingFixer *columnResizingFixer; diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index bd63d6e7fb..8e381861a0 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -185,21 +185,21 @@ static void ShowProgress(SplashScreen *splash, const std::string &title, int nPr : _("press q to shutdown").translated) + strprintf("\n%d", nProgress) + "%"); } -#ifdef ENABLE_WALLET -void SplashScreen::ConnectWallet(std::unique_ptr<interfaces::Wallet> wallet) -{ - m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false))); - m_connected_wallets.emplace_back(std::move(wallet)); -} -#endif void SplashScreen::subscribeToCoreSignals() { // Connect signals to client m_handler_init_message = m_node->handleInitMessage(std::bind(InitMessage, this, std::placeholders::_1)); m_handler_show_progress = m_node->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); +} + +void SplashScreen::handleLoadWallet() +{ #ifdef ENABLE_WALLET - m_handler_load_wallet = m_node->handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { ConnectWallet(std::move(wallet)); }); + m_handler_load_wallet = m_node->walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { + m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false))); + m_connected_wallets.emplace_back(std::move(wallet)); + }); #endif } diff --git a/src/qt/splashscreen.h b/src/qt/splashscreen.h index 2213b02c55..a0cd677d3d 100644 --- a/src/qt/splashscreen.h +++ b/src/qt/splashscreen.h @@ -43,6 +43,9 @@ public Q_SLOTS: /** Show message and progress */ void showMessage(const QString &message, int alignment, const QColor &color); + /** Handle wallet load notifications. */ + void handleLoadWallet(); + protected: bool eventFilter(QObject * obj, QEvent * ev) override; @@ -53,8 +56,6 @@ private: void unsubscribeFromCoreSignals(); /** Initiate shutdown */ void shutdown(); - /** Connect wallet signals to splash screen */ - void ConnectWallet(std::unique_ptr<interfaces::Wallet> wallet); QPixmap pixmap; QString curMessage; diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 3aed98e0e8..828f84ffcc 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -35,11 +35,11 @@ WalletController::WalletController(ClientModel& client_model, const PlatformStyl , m_platform_style(platform_style) , m_options_model(client_model.getOptionsModel()) { - m_handler_load_wallet = m_node.handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { + m_handler_load_wallet = m_node.walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { getOrCreateWallet(std::move(wallet)); }); - for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.getWallets()) { + for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.walletClient().getWallets()) { getOrCreateWallet(std::move(wallet)); } @@ -66,7 +66,7 @@ std::map<std::string, bool> WalletController::listWalletDir() const { QMutexLocker locker(&m_mutex); std::map<std::string, bool> wallets; - for (const std::string& name : m_node.listWalletDir()) { + for (const std::string& name : m_node.walletClient().listWalletDir()) { wallets[name] = false; } for (WalletModel* wallet_model : m_wallets) { @@ -250,7 +250,7 @@ void CreateWalletActivity::createWallet() QTimer::singleShot(500, worker(), [this, name, flags] { WalletCreationStatus status; - std::unique_ptr<interfaces::Wallet> wallet = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message, status); + std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().createWallet(name, m_passphrase, flags, status, m_error_message, m_warning_message); if (status == WalletCreationStatus::SUCCESS) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); @@ -321,7 +321,7 @@ void OpenWalletActivity::open(const std::string& path) showProgressDialog(tr("Opening Wallet <b>%1</b>...").arg(name.toHtmlEscaped())); QTimer::singleShot(0, worker(), [this, path] { - std::unique_ptr<interfaces::Wallet> wallet = node().loadWallet(path, m_error_message, m_warning_message); + std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().loadWallet(path, m_error_message, m_warning_message); if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index e374dd191c..6a3f903206 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -413,7 +413,7 @@ void WalletModel::subscribeToCoreSignals() m_handler_transaction_changed = m_wallet->handleTransactionChanged(std::bind(NotifyTransactionChanged, this, std::placeholders::_1, std::placeholders::_2)); m_handler_show_progress = m_wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2)); m_handler_watch_only_changed = m_wallet->handleWatchOnlyChanged(std::bind(NotifyWatchonlyChanged, this, std::placeholders::_1)); - m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(boost::bind(NotifyCanGetAddressesChanged, this)); + m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(std::bind(NotifyCanGetAddressesChanged, this)); } void WalletModel::unsubscribeFromCoreSignals() @@ -581,7 +581,7 @@ QString WalletModel::getDisplayName() const bool WalletModel::isMultiwallet() { - return m_node.getWallets().size() > 1; + return m_node.walletClient().getWallets().size() > 1; } void WalletModel::refresh(bool pk_hash_only) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 76aa9dbfc1..b04e106b2d 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -81,9 +81,9 @@ static UniValue GetNetworkHashPS(int lookup, int height) { return workDiff.getdouble() / timeDiff; } -static UniValue getnetworkhashps(const JSONRPCRequest& request) +static RPCHelpMan getnetworkhashps() { - RPCHelpMan{"getnetworkhashps", + return RPCHelpMan{"getnetworkhashps", "\nReturns the estimated network hashes per second based on the last n blocks.\n" "Pass in [blocks] to override # of blocks, -1 specifies since last difficulty change.\n" "Pass in [height] to estimate the network speed at the time when a certain block was found.\n", @@ -97,10 +97,12 @@ static UniValue getnetworkhashps(const JSONRPCRequest& request) HelpExampleCli("getnetworkhashps", "") + HelpExampleRpc("getnetworkhashps", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1); +}, + }; } static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& max_tries, unsigned int& extra_nonce, uint256& block_hash) @@ -200,9 +202,9 @@ static bool getScriptFromDescriptor(const std::string& descriptor, CScript& scri } } -static UniValue generatetodescriptor(const JSONRPCRequest& request) +static RPCHelpMan generatetodescriptor() { - RPCHelpMan{ + return RPCHelpMan{ "generatetodescriptor", "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n", { @@ -218,9 +220,8 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) }, RPCExamples{ "\nGenerate 11 blocks to mydesc\n" + HelpExampleCli("generatetodescriptor", "11 \"mydesc\"")}, - } - .Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ const int num_blocks{request.params[0].get_int()}; const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].get_int()}; @@ -234,22 +235,25 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) ChainstateManager& chainman = EnsureChainman(request.context); return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); +}, + }; } -static UniValue generate(const JSONRPCRequest& request) +static RPCHelpMan generate() { - const std::string help_str{"generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information."}; + return RPCHelpMan{"generate", "has been replaced by the -generate cli option. Refer to -help for more information.", {}, {}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { if (request.fHelp) { - throw std::runtime_error(help_str); + throw std::runtime_error(self.ToString()); } else { - throw JSONRPCError(RPC_METHOD_NOT_FOUND, help_str); + throw JSONRPCError(RPC_METHOD_NOT_FOUND, self.ToString()); } + }}; } -static UniValue generatetoaddress(const JSONRPCRequest& request) +static RPCHelpMan generatetoaddress() { - RPCHelpMan{"generatetoaddress", + return RPCHelpMan{"generatetoaddress", "\nMine blocks immediately to a specified address (before the RPC call returns)\n", { {"nblocks", RPCArg::Type::NUM, RPCArg::Optional::NO, "How many blocks are generated immediately."}, @@ -267,8 +271,8 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) + "If you are using the " PACKAGE_NAME " wallet, you can get a new address to send the newly generated bitcoin to with:\n" + HelpExampleCli("getnewaddress", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ const int num_blocks{request.params[0].get_int()}; const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].get_int()}; @@ -283,11 +287,13 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) CScript coinbase_script = GetScriptForDestination(destination); return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); +}, + }; } -static UniValue generateblock(const JSONRPCRequest& request) +static RPCHelpMan generateblock() { - RPCHelpMan{"generateblock", + return RPCHelpMan{"generateblock", "\nMine a block with a set of ordered transactions immediately to a specified address or descriptor (before the RPC call returns)\n", { {"output", RPCArg::Type::STR, RPCArg::Optional::NO, "The address or descriptor to send the newly generated bitcoin to."}, @@ -309,8 +315,8 @@ static UniValue generateblock(const JSONRPCRequest& request) "\nGenerate a block to myaddress, with txs rawtx and mempool_txid\n" + HelpExampleCli("generateblock", R"("myaddress" '["rawtx", "mempool_txid"]')") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ const auto address_or_descriptor = request.params[0].get_str(); CScript coinbase_script; std::string error; @@ -390,11 +396,13 @@ static UniValue generateblock(const JSONRPCRequest& request) UniValue obj(UniValue::VOBJ); obj.pushKV("hash", block_hash.GetHex()); return obj; +}, + }; } -static UniValue getmininginfo(const JSONRPCRequest& request) +static RPCHelpMan getmininginfo() { - RPCHelpMan{"getmininginfo", + return RPCHelpMan{"getmininginfo", "\nReturns a json object containing mining-related information.", {}, RPCResult{ @@ -413,8 +421,8 @@ static UniValue getmininginfo(const JSONRPCRequest& request) HelpExampleCli("getmininginfo", "") + HelpExampleRpc("getmininginfo", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); const CTxMemPool& mempool = EnsureMemPool(request.context); @@ -423,18 +431,20 @@ static UniValue getmininginfo(const JSONRPCRequest& request) if (BlockAssembler::m_last_block_weight) obj.pushKV("currentblockweight", *BlockAssembler::m_last_block_weight); if (BlockAssembler::m_last_block_num_txs) obj.pushKV("currentblocktx", *BlockAssembler::m_last_block_num_txs); obj.pushKV("difficulty", (double)GetDifficulty(::ChainActive().Tip())); - obj.pushKV("networkhashps", getnetworkhashps(request)); + obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request)); obj.pushKV("pooledtx", (uint64_t)mempool.size()); obj.pushKV("chain", Params().NetworkIDString()); obj.pushKV("warnings", GetWarnings(false).original); return obj; +}, + }; } // NOTE: Unlike wallet RPC (which use BTC values), mining RPCs follow GBT (BIP 22) in using satoshi amounts -static UniValue prioritisetransaction(const JSONRPCRequest& request) +static RPCHelpMan prioritisetransaction() { - RPCHelpMan{"prioritisetransaction", + return RPCHelpMan{"prioritisetransaction", "Accepts the transaction into mined blocks at a higher (or lower) priority\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id."}, @@ -451,8 +461,8 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) HelpExampleCli("prioritisetransaction", "\"txid\" 0.0 10000") + HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -464,6 +474,8 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) EnsureMemPool(request.context).PrioritiseTransaction(hash, nAmount); return true; +}, + }; } @@ -495,9 +507,9 @@ static std::string gbt_vb_name(const Consensus::DeploymentPos pos) { return s; } -static UniValue getblocktemplate(const JSONRPCRequest& request) +static RPCHelpMan getblocktemplate() { - RPCHelpMan{"getblocktemplate", + return RPCHelpMan{"getblocktemplate", "\nIf the request parameters include a 'mode' key, that is used to explicitly select between the default 'template' request or a 'proposal'.\n" "It returns data needed to construct a block to work on.\n" "For full specification, see BIPs 22, 23, 9, and 145:\n" @@ -511,12 +523,13 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings", { - {"support", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasetxn', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, + {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, }, }, {"rules", RPCArg::Type::ARR, RPCArg::Optional::NO, "A list of strings", { - {"support", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported softfork deployment"}, + {"segwit", RPCArg::Type::STR, RPCArg::Optional::NO, "(literal) indicates client side segwit support"}, + {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "other client side supported softfork deployment"}, }, }, }, @@ -528,7 +541,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {RPCResult::Type::NUM, "version", "The preferred block version"}, {RPCResult::Type::ARR, "rules", "specific block rules that are to be enforced", { - {RPCResult::Type::STR, "", "rulename"}, + {RPCResult::Type::STR, "", "name of a rule the client must understand to some extent; see BIP 9 for format"}, }}, {RPCResult::Type::OBJ_DYN, "vbavailable", "set of pending, supported versionbit (BIP 9) softfork deployments", { @@ -536,7 +549,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) }}, {RPCResult::Type::NUM, "vbrequired", "bit mask of versionbits the server requires set in submissions"}, {RPCResult::Type::STR, "previousblockhash", "The hash of current highest block"}, - {RPCResult::Type::ARR, "", "contents of non-coinbase transactions that should be included in the next block", + {RPCResult::Type::ARR, "transactions", "contents of non-coinbase transactions that should be included in the next block", { {RPCResult::Type::OBJ, "", "", { @@ -552,15 +565,12 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {RPCResult::Type::NUM, "weight", "total transaction weight, as counted for purposes of block limits"}, }}, }}, - {RPCResult::Type::OBJ, "coinbaseaux", "data that should be included in the coinbase's scriptSig content", + {RPCResult::Type::OBJ_DYN, "coinbaseaux", "data that should be included in the coinbase's scriptSig content", { - {RPCResult::Type::ELISION, "", ""}, + {RPCResult::Type::STR_HEX, "key", "values must be in the coinbase (keys may be ignored)"}, }}, {RPCResult::Type::NUM, "coinbasevalue", "maximum allowable input to coinbase transaction, including the generation award and transaction fees (in satoshis)"}, - {RPCResult::Type::OBJ, "coinbasetxn", "information for coinbase transaction", - { - {RPCResult::Type::ELISION, "", ""}, - }}, + {RPCResult::Type::STR, "longpollid", "an id to include with a request to longpoll on an update to this template"}, {RPCResult::Type::STR, "target", "The hash target"}, {RPCResult::Type::NUM_TIME, "mintime", "The minimum timestamp appropriate for the next block time, expressed in " + UNIX_EPOCH_TIME}, {RPCResult::Type::ARR, "mutable", "list of ways the block template may be changed", @@ -574,13 +584,14 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME}, {RPCResult::Type::STR, "bits", "compressed target of next block"}, {RPCResult::Type::NUM, "height", "The height of the next block"}, + {RPCResult::Type::STR, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"} }}, RPCExamples{ HelpExampleCli("getblocktemplate", "'{\"rules\": [\"segwit\"]}'") + HelpExampleRpc("getblocktemplate", "{\"rules\": [\"segwit\"]}") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); std::string strMode = "template"; @@ -888,6 +899,8 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) } return result; +}, + }; } class submitblock_StateCatcher final : public CValidationInterface @@ -908,10 +921,10 @@ protected: } }; -static UniValue submitblock(const JSONRPCRequest& request) +static RPCHelpMan submitblock() { // We allow 2 arguments for compliance with BIP22. Argument 2 is ignored. - RPCHelpMan{"submitblock", + return RPCHelpMan{"submitblock", "\nAttempts to submit new block to network.\n" "See https://en.bitcoin.it/wiki/BIP_0022 for full specification.\n", { @@ -923,8 +936,8 @@ static UniValue submitblock(const JSONRPCRequest& request) HelpExampleCli("submitblock", "\"mydata\"") + HelpExampleRpc("submitblock", "\"mydata\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CBlock> blockptr = std::make_shared<CBlock>(); CBlock& block = *blockptr; if (!DecodeHexBlk(block, request.params[0].get_str())) { @@ -969,11 +982,13 @@ static UniValue submitblock(const JSONRPCRequest& request) return "inconclusive"; } return BIP22ValidationResult(sc->state); +}, + }; } -static UniValue submitheader(const JSONRPCRequest& request) +static RPCHelpMan submitheader() { - RPCHelpMan{"submitheader", + return RPCHelpMan{"submitheader", "\nDecode the given hexdata as a header and submit it as a candidate chain tip if valid." "\nThrows when the header is invalid.\n", { @@ -985,8 +1000,8 @@ static UniValue submitheader(const JSONRPCRequest& request) HelpExampleCli("submitheader", "\"aabbcc\"") + HelpExampleRpc("submitheader", "\"aabbcc\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ CBlockHeader h; if (!DecodeHexBlockHeader(h, request.params[0].get_str())) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Block header decode failed"); @@ -1005,11 +1020,13 @@ static UniValue submitheader(const JSONRPCRequest& request) throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); } throw JSONRPCError(RPC_VERIFY_ERROR, state.GetRejectReason()); +}, + }; } -static UniValue estimatesmartfee(const JSONRPCRequest& request) +static RPCHelpMan estimatesmartfee() { - RPCHelpMan{"estimatesmartfee", + return RPCHelpMan{"estimatesmartfee", "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n" "confirmation within conf_target blocks if possible and return the number of blocks\n" "for which the estimate is valid. Uses virtual transaction size as defined\n" @@ -1043,8 +1060,8 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request) RPCExamples{ HelpExampleCli("estimatesmartfee", "6") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR}); RPCTypeCheckArgument(request.params[0], UniValue::VNUM); unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); @@ -1070,11 +1087,13 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request) } result.pushKV("blocks", feeCalc.returnedTarget); return result; +}, + }; } -static UniValue estimaterawfee(const JSONRPCRequest& request) +static RPCHelpMan estimaterawfee() { - RPCHelpMan{"estimaterawfee", + return RPCHelpMan{"estimaterawfee", "\nWARNING: This interface is unstable and may disappear or change!\n" "\nWARNING: This is an advanced API call that is tightly coupled to the specific\n" " implementation of fee estimation. The parameters it can be called with\n" @@ -1126,8 +1145,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) RPCExamples{ HelpExampleCli("estimaterawfee", "6 0.9") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true); RPCTypeCheckArgument(request.params[0], UniValue::VNUM); unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); @@ -1186,6 +1205,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) result.pushKV(StringForFeeEstimateHorizon(horizon), horizon_result); } return result; +}, + }; } void RegisterMiningRPCCommands(CRPCTable &t) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e9343b3348..7e2052c7cb 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -197,7 +197,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) if (fStateStats) { if (IsDeprecatedRPCEnabled("banscore")) { // banscore is deprecated in v0.21 for removal in v0.22 - obj.pushKV("banscore", statestats.nMisbehavior); + obj.pushKV("banscore", statestats.m_misbehavior_score); } obj.pushKV("synced_headers", statestats.nSyncHeight); obj.pushKV("synced_blocks", statestats.nCommonHeight); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9c8e7fe04a..f32d9abac6 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -261,13 +261,11 @@ CRPCTable::CRPCTable() } } -bool CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) +void CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) { - if (IsRPCRunning()) - return false; + CHECK_NONFATAL(!IsRPCRunning()); // Only add commands before rpc is running mapCommands[name].push_back(pcmd); - return true; } bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd) diff --git a/src/rpc/server.h b/src/rpc/server.h index 6da3e94ea2..b2358ac5b2 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -160,7 +160,7 @@ public: /** * Appends a CRPCCommand to the dispatch table. * - * Returns false if RPC server is already running (dump concurrency protection). + * Precondition: RPC server is not running * * Commands with different method names but the same unique_id will * be considered aliases, and only the first registered method name will @@ -169,7 +169,7 @@ public: * between calls based on method name, and aliased commands can also * register different names, types, and numbers of parameters. */ - bool appendCommand(const std::string& name, const CRPCCommand* pcmd); + void appendCommand(const std::string& name, const CRPCCommand* pcmd); bool removeCommand(const std::string& name, const CRPCCommand* pcmd); }; diff --git a/src/sync.cpp b/src/sync.cpp index 4be13a3c48..322198a852 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -238,12 +238,15 @@ void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +template <typename MutexType> +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { if (!LockHeld(cs)) return; tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); abort(); } +template void AssertLockNotHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockNotHeldInternal(const char*, const char*, int, RecursiveMutex*); void DeleteLock(void* cs) { diff --git a/src/sync.h b/src/sync.h index 05ff2ee8a9..7b397a8003 100644 --- a/src/sync.h +++ b/src/sync.h @@ -53,8 +53,9 @@ void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); template <typename MutexType> -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs); +template <typename MutexType> +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs); void DeleteLock(void* cs); bool LockStackEmpty(); @@ -69,8 +70,9 @@ inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, v inline void LeaveCritical() {} inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} template <typename MutexType> -inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} -inline void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs) {} +template <typename MutexType> +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) {} inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } #endif diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index c0a2fca9ca..93bfa88024 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -232,10 +232,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged - } + Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged { LOCK(dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); @@ -249,20 +246,14 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(&dummyNode2); dummyNode2.nVersion = 1; dummyNode2.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1); - } + Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1); { LOCK(dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not discouraged yet... BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be - { - LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold - } + Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold { LOCK(dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); @@ -292,10 +283,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.nVersion = 1; dummyNode.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); - } + Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); { LOCK(dummyNode.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp new file mode 100644 index 0000000000..1ff9d6b286 --- /dev/null +++ b/src/test/fuzz/net.cpp @@ -0,0 +1,156 @@ +// Copyright (c) 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 <chainparams.h> +#include <chainparamsbase.h> +#include <net.h> +#include <net_permissions.h> +#include <netaddress.h> +#include <optional.h> +#include <protocol.h> +#include <random.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> + +#include <cstdint> +#include <string> +#include <vector> + +void initialize() +{ + static const BasicTestingSetup basic_testing_setup; +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + + const std::optional<CAddress> address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!address) { + return; + } + const std::optional<CAddress> address_bind = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!address_bind) { + return; + } + + CNode node{fuzzed_data_provider.ConsumeIntegral<NodeId>(), + static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()), + fuzzed_data_provider.ConsumeIntegral<int>(), + INVALID_SOCKET, + *address, + fuzzed_data_provider.ConsumeIntegral<uint64_t>(), + fuzzed_data_provider.ConsumeIntegral<uint64_t>(), + *address_bind, + fuzzed_data_provider.ConsumeRandomLengthString(32), + fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH})}; + while (fuzzed_data_provider.ConsumeBool()) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 12)) { + case 0: { + node.CloseSocketDisconnect(); + break; + } + case 1: { + node.MaybeSetAddrName(fuzzed_data_provider.ConsumeRandomLengthString(32)); + break; + } + case 2: { + node.SetSendVersion(fuzzed_data_provider.ConsumeIntegral<int>()); + break; + } + case 3: { + const std::vector<bool> asmap = ConsumeRandomLengthIntegralVector<bool>(fuzzed_data_provider, 128); + if (!SanityCheckASMap(asmap)) { + break; + } + CNodeStats stats; + node.copyStats(stats, asmap); + break; + } + case 4: { + node.SetRecvVersion(fuzzed_data_provider.ConsumeIntegral<int>()); + break; + } + case 5: { + const CNode* add_ref_node = node.AddRef(); + assert(add_ref_node == &node); + break; + } + case 6: { + if (node.GetRefCount() > 0) { + node.Release(); + } + break; + } + case 7: { + if (node.m_addr_known == nullptr) { + break; + } + const std::optional<CAddress> addr_opt = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!addr_opt) { + break; + } + node.AddAddressKnown(*addr_opt); + break; + } + case 8: { + if (node.m_addr_known == nullptr) { + break; + } + const std::optional<CAddress> addr_opt = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!addr_opt) { + break; + } + FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)}; + node.PushAddress(*addr_opt, fast_random_context); + break; + } + case 9: { + const std::optional<CInv> inv_opt = ConsumeDeserializable<CInv>(fuzzed_data_provider); + if (!inv_opt) { + break; + } + node.AddKnownTx(inv_opt->hash); + break; + } + case 10: { + node.PushTxInventory(ConsumeUInt256(fuzzed_data_provider)); + break; + } + case 11: { + const std::optional<CService> service_opt = ConsumeDeserializable<CService>(fuzzed_data_provider); + if (!service_opt) { + break; + } + node.SetAddrLocal(*service_opt); + break; + } + case 12: { + const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); + bool complete; + node.ReceiveMsgBytes((const char*)b.data(), b.size(), complete); + break; + } + } + } + + (void)node.GetAddrLocal(); + (void)node.GetAddrName(); + (void)node.GetId(); + (void)node.GetLocalNonce(); + (void)node.GetLocalServices(); + (void)node.GetMyStartingHeight(); + (void)node.GetRecvVersion(); + const int ref_count = node.GetRefCount(); + assert(ref_count >= 0); + (void)node.GetSendVersion(); + (void)node.IsAddrRelayPeer(); + + const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ? + fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({NetPermissionFlags::PF_NONE, NetPermissionFlags::PF_BLOOMFILTER, NetPermissionFlags::PF_RELAY, NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL}) : + static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>()); + (void)node.HasPermission(net_permission_flags); +} diff --git a/src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp b/src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp new file mode 100644 index 0000000000..d4f302a8d3 --- /dev/null +++ b/src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp @@ -0,0 +1,38 @@ +// Copyright (c) 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 <key.h> +#include <secp256k1.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <vector> + +int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char* out32, const unsigned char* seckey, size_t seckeylen); +int ec_seckey_export_der(const secp256k1_context* ctx, unsigned char* seckey, size_t* seckeylen, const unsigned char* key32, bool compressed); + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + secp256k1_context* secp256k1_context_sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN); + { + std::vector<uint8_t> out32(32); + (void)ec_seckey_import_der(secp256k1_context_sign, out32.data(), ConsumeFixedLengthByteVector(fuzzed_data_provider, CKey::SIZE).data(), CKey::SIZE); + } + { + std::vector<uint8_t> seckey(CKey::SIZE); + const std::vector<uint8_t> key32 = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32); + size_t seckeylen = CKey::SIZE; + const bool compressed = fuzzed_data_provider.ConsumeBool(); + const bool exported = ec_seckey_export_der(secp256k1_context_sign, seckey.data(), &seckeylen, key32.data(), compressed); + if (exported) { + std::vector<uint8_t> out32(32); + const bool imported = ec_seckey_import_der(secp256k1_context_sign, out32.data(), seckey.data(), seckey.size()) == 1; + assert(imported && key32 == out32); + } + } + secp256k1_context_destroy(secp256k1_context_sign); +} diff --git a/src/test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp b/src/test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp new file mode 100644 index 0000000000..ed8c7aba89 --- /dev/null +++ b/src/test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp @@ -0,0 +1,33 @@ +// Copyright (c) 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 <key.h> +#include <secp256k1.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <vector> + +bool SigHasLowR(const secp256k1_ecdsa_signature* sig); +int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char* input, size_t inputlen); + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const std::vector<uint8_t> signature_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); + if (signature_bytes.data() == nullptr) { + return; + } + secp256k1_context* secp256k1_context_verify = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY); + secp256k1_ecdsa_signature sig_der_lax; + const bool parsed_der_lax = ecdsa_signature_parse_der_lax(secp256k1_context_verify, &sig_der_lax, signature_bytes.data(), signature_bytes.size()) == 1; + if (parsed_der_lax) { + ECC_Start(); + (void)SigHasLowR(&sig_der_lax); + ECC_Stop(); + } + secp256k1_context_destroy(secp256k1_context_verify); +} diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index d9a00c2205..8a88e75892 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -141,8 +141,11 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const pblocktree.reset(new CBlockTreeDB(1 << 20, true)); + m_node.mempool = &::mempool; + m_node.mempool->setSanityCheck(1.0); + m_node.chainman = &::g_chainman; - m_node.chainman->InitializeChainstate(); + m_node.chainman->InitializeChainstate(*m_node.mempool); ::ChainstateActive().InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); assert(!::ChainstateActive().CanFlushToDisk()); @@ -164,8 +167,6 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const } g_parallel_script_checks = true; - m_node.mempool = &::mempool; - m_node.mempool->setSanityCheck(1.0); m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests. m_node.peer_logic = MakeUnique<PeerLogicValidation>(*m_node.connman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool); diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 2076a1096a..c8a375275f 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -20,6 +20,7 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup) BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) { ChainstateManager manager; + CTxMemPool mempool; //! Create and add a Coin with DynamicMemoryUsage of 80 bytes to the given view. auto add_coin = [](CCoinsViewCache& coins_view) -> COutPoint { @@ -34,7 +35,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) return outp; }; - CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate()); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool)); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 887a48124f..36badafc4e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -23,12 +23,13 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, TestingSetup) BOOST_AUTO_TEST_CASE(chainstatemanager) { ChainstateManager manager; + CTxMemPool mempool; std::vector<CChainState*> chainstates; const CChainParams& chainparams = Params(); // Create a legacy (IBD) chainstate. // - CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate()); + CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(mempool)); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -54,7 +55,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a snapshot-based chainstate. // - CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(GetRandHash())); + CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(mempool, GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -104,6 +105,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) { ChainstateManager manager; + CTxMemPool mempool; size_t max_cache = 10000; manager.m_total_coinsdb_cache = max_cache; manager.m_total_coinstip_cache = max_cache; @@ -112,7 +114,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a legacy (IBD) chainstate. // - CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate()); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool)); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -129,7 +131,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a snapshot-based chainstate. // - CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(GetRandHash())); + CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool, GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -147,7 +149,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) BOOST_CHECK_CLOSE(c1.m_coinsdb_cache_size_bytes, max_cache * 0.05, 1); BOOST_CHECK_CLOSE(c2.m_coinstip_cache_size_bytes, max_cache * 0.95, 1); BOOST_CHECK_CLOSE(c2.m_coinsdb_cache_size_bytes, max_cache * 0.95, 1); - } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 8bac914f05..a3b344d2c9 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -18,8 +18,9 @@ BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, BasicTestingSetup) //! BOOST_AUTO_TEST_CASE(getcoinscachesizestate) { + CTxMemPool mempool; BlockManager blockman{}; - CChainState chainstate{blockman}; + CChainState chainstate{mempool, blockman}; chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false); WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10)); CTxMemPool tx_pool{}; diff --git a/src/threadsafety.h b/src/threadsafety.h index 5f2c40bac6..52bf83b676 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -18,9 +18,7 @@ #define LOCKABLE __attribute__((lockable)) #define SCOPED_LOCKABLE __attribute__((scoped_lockable)) #define GUARDED_BY(x) __attribute__((guarded_by(x))) -#define GUARDED_VAR __attribute__((guarded_var)) #define PT_GUARDED_BY(x) __attribute__((pt_guarded_by(x))) -#define PT_GUARDED_VAR __attribute__((pt_guarded_var)) #define ACQUIRED_AFTER(...) __attribute__((acquired_after(__VA_ARGS__))) #define ACQUIRED_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__))) #define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__((exclusive_lock_function(__VA_ARGS__))) @@ -33,14 +31,12 @@ #define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_required(__VA_ARGS__))) #define SHARED_LOCKS_REQUIRED(...) __attribute__((shared_locks_required(__VA_ARGS__))) #define NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) -#define ASSERT_EXCLUSIVE_LOCK(...) __attribute((assert_exclusive_lock(__VA_ARGS__))) +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_exclusive_lock(__VA_ARGS__))) #else #define LOCKABLE #define SCOPED_LOCKABLE #define GUARDED_BY(x) -#define GUARDED_VAR #define PT_GUARDED_BY(x) -#define PT_GUARDED_VAR #define ACQUIRED_AFTER(...) #define ACQUIRED_BEFORE(...) #define EXCLUSIVE_LOCK_FUNCTION(...) diff --git a/src/util/system.cpp b/src/util/system.cpp index 00aa53df70..999937d906 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1019,7 +1019,7 @@ bool FileCommit(FILE *file) return false; } #else - #if defined(HAVE_FDATASYNC) + #if HAVE_FDATASYNC if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync LogPrintf("%s: fdatasync failed: %d\n", __func__, errno); return false; diff --git a/src/validation.cpp b/src/validation.cpp index cf2f9dde62..58af8744d9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -370,9 +370,10 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) +static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, mempool.cs) { AssertLockHeld(cs_main); + AssertLockHeld(mempool.cs); std::vector<uint256> vHashUpdate; // disconnectpool's insertion_order index sorts the entries from // oldest to newest, but the oldest entry will be the last tx from the @@ -1254,8 +1255,9 @@ void CoinsViews::InitCache() m_cacheview = MakeUnique<CCoinsViewCache>(&m_catcherview); } -CChainState::CChainState(BlockManager& blockman, uint256 from_snapshot_blockhash) +CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash) : m_blockman(blockman), + m_mempool(mempool), m_from_snapshot_blockhash(from_snapshot_blockhash) {} void CChainState::InitCoinsDB( @@ -2280,7 +2282,7 @@ bool CChainState::FlushStateToDisk( { bool fFlushForPrune = false; bool fDoFullFlush = false; - CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&::mempool); + CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { if (nManualPruneHeight > 0) { @@ -2426,7 +2428,7 @@ static void AppendWarning(bilingual_str& res, const bilingual_str& warn) } /** Check warning conditions and do some notifications on new chain tip set. */ -void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainParams) +static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const CChainParams& chainParams) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { // New best block @@ -2472,7 +2474,6 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar FormatISO8601DateTime(pindexNew->GetBlockTime()), GuessVerificationProgress(chainParams.TxData(), pindexNew), ::ChainstateActive().CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), ::ChainstateActive().CoinsTip().GetCacheSize(), !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages.original) : ""); - } /** Disconnect m_chain's tip. @@ -2485,8 +2486,11 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar * disconnectpool (note that the caller is responsible for mempool consistency * in any case). */ -bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool) +bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) { + AssertLockHeld(cs_main); + AssertLockHeld(m_mempool.cs); + CBlockIndex *pindexDelete = m_chain.Tip(); assert(pindexDelete); // Read block from disk. @@ -2517,14 +2521,14 @@ bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) { // Drop the earliest entry, and remove its children from the mempool. auto it = disconnectpool->queuedTx.get<insertion_order>().begin(); - mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + m_mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); disconnectpool->removeEntry(it); } } m_chain.SetTip(pindexDelete->pprev); - UpdateTip(pindexDelete->pprev, chainparams); + UpdateTip(m_mempool, pindexDelete->pprev, chainparams); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: GetMainSignals().BlockDisconnected(pblock, pindexDelete); @@ -2585,6 +2589,9 @@ public: */ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) { + AssertLockHeld(cs_main); + AssertLockHeld(m_mempool.cs); + assert(pindexNew->pprev == m_chain.Tip()); // Read block from disk. int64_t nTime1 = GetTimeMicros(); @@ -2625,11 +2632,11 @@ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& ch int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime5 - nTime4) * MILLI, nTimeChainState * MICRO, nTimeChainState * MILLI / nBlocksTotal); // Remove conflicting transactions from the mempool.; - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); + m_mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); disconnectpool.removeForBlock(blockConnecting.vtx); // Update m_chain & related variables. m_chain.SetTip(pindexNew); - UpdateTip(pindexNew, chainparams); + UpdateTip(m_mempool, pindexNew, chainparams); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint(BCLog::BENCH, " - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime6 - nTime5) * MILLI, nTimePostConnect * MICRO, nTimePostConnect * MILLI / nBlocksTotal); @@ -2719,6 +2726,7 @@ void CChainState::PruneBlockIndexCandidates() { bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); + AssertLockHeld(m_mempool.cs); const CBlockIndex *pindexOldTip = m_chain.Tip(); const CBlockIndex *pindexFork = m_chain.FindFork(pindexMostWork); @@ -2730,7 +2738,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai if (!DisconnectTip(state, chainparams, &disconnectpool)) { // This is likely a fatal error, but keep the mempool consistent, // just in case. Only remove from the mempool in this case. - UpdateMempoolForReorg(disconnectpool, false); + UpdateMempoolForReorg(m_mempool, disconnectpool, false); // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort @@ -2774,7 +2782,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai // A system error occurred (disk space, database error, ...). // Make the mempool consistent with the current tip, just in case // any observers try to use it before shutdown. - UpdateMempoolForReorg(disconnectpool, false); + UpdateMempoolForReorg(m_mempool, disconnectpool, false); return false; } } else { @@ -2791,9 +2799,9 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai if (fBlocksDisconnected) { // If any blocks were disconnected, disconnectpool may be non empty. Add // any disconnected transactions back to the mempool. - UpdateMempoolForReorg(disconnectpool, true); + UpdateMempoolForReorg(m_mempool, disconnectpool, true); } - mempool.check(&CoinsTip()); + m_mempool.check(&CoinsTip()); // Callbacks/notifications for a new best chain. if (fInvalidFound) @@ -2867,7 +2875,8 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar LimitValidationInterfaceQueue(); { - LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed + LOCK(cs_main); + LOCK(m_mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -3020,7 +3029,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, const CChainParam LimitValidationInterfaceQueue(); LOCK(cs_main); - LOCK(::mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between + LOCK(m_mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -3034,7 +3043,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, const CChainParam // transactions back to the mempool if disconnecting was successful, // and we're not doing a very deep invalidation (in which case // keeping the mempool up to date is probably futile anyway). - UpdateMempoolForReorg(disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); + UpdateMempoolForReorg(m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); if (!ret) return false; assert(invalid_walk_tip->pprev == m_chain.Tip()); @@ -4517,7 +4526,8 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { - LOCK2(cs_main, ::mempool.cs); + LOCK(cs_main); + LOCK(m_mempool.cs); // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active) assert(tip == m_chain.Tip()); if (tip == nullptr || tip->nHeight < nHeight) break; @@ -5246,7 +5256,7 @@ std::vector<CChainState*> ChainstateManager::GetAll() return out; } -CChainState& ChainstateManager::InitializeChainstate(const uint256& snapshot_blockhash) +CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash) { bool is_snapshot = !snapshot_blockhash.IsNull(); std::unique_ptr<CChainState>& to_modify = @@ -5255,8 +5265,7 @@ CChainState& ChainstateManager::InitializeChainstate(const uint256& snapshot_blo if (to_modify) { throw std::logic_error("should not be overwriting a chainstate"); } - - to_modify.reset(new CChainState(m_blockman, snapshot_blockhash)); + to_modify.reset(new CChainState(mempool, m_blockman, snapshot_blockhash)); // Snapshot chainstates and initial IBD chaintates always become active. if (is_snapshot || (!is_snapshot && !m_active_chainstate)) { diff --git a/src/validation.h b/src/validation.h index 534162d64a..cac9473c7a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -245,7 +245,7 @@ bool TestLockPointValidity(const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_mai * * See consensus/consensus.h for flag definitions. */ -bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs); /** * Closure representing one script verification @@ -511,11 +511,14 @@ private: //! easily as opposed to referencing a global. BlockManager& m_blockman; + //! mempool that is kept in sync with the chain + CTxMemPool& m_mempool; + //! Manages the UTXO set, which is a reflection of the contents of `m_chain`. std::unique_ptr<CoinsViews> m_coins_views; public: - explicit CChainState(BlockManager& blockman, uint256 from_snapshot_blockhash = uint256()); + explicit CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash = uint256()); /** * Initialize the CoinsViews UTXO set database management data structures. The in-memory @@ -642,7 +645,7 @@ public: CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. - bool DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); // Manual block validity manipulation: bool PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); @@ -685,8 +688,8 @@ public: std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); private: - bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); - bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); + bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); void InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -818,9 +821,11 @@ public: //! Instantiate a new chainstate and assign it based upon whether it is //! from a snapshot. //! + //! @param[in] mempool The mempool to pass to the chainstate + // constructor //! @param[in] snapshot_blockhash If given, signify that this chainstate //! is based on a snapshot. - CChainState& InitializeChainstate(const uint256& snapshot_blockhash = uint256()) + CChainState& InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash = uint256()) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Get all chainstates currently being used. diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index bf05ef844a..3910599ca7 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -5,6 +5,7 @@ #include <init.h> #include <interfaces/chain.h> +#include <interfaces/wallet.h> #include <net.h> #include <node/context.h> #include <node/ui_interface.h> @@ -66,13 +67,13 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID and %w is replaced by wallet name. %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - argsman.AddArg("-zapwallettxes=<mode>", "Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup" - " (1 = keep tx meta data e.g. payment request information, 2 = drop tx meta data)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-dblogsize=<n>", strprintf("Flush wallet database activity from memory to disk log every <n> megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", DEFAULT_WALLET_PRIVDB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-walletrejectlongchains", strprintf("Wallet will not create transactions that violate mempool chain limits (default: %u)", DEFAULT_WALLET_REJECT_LONG_CHAINS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); + + argsman.AddHiddenArgs({"-zapwallettxes"}); } bool WalletInit::ParameterInteraction() const @@ -85,26 +86,12 @@ bool WalletInit::ParameterInteraction() const return true; } - const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; - if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - bool zapwallettxes = gArgs.GetBoolArg("-zapwallettxes", false); - // -zapwallettxes implies dropping the mempool on startup - if (zapwallettxes && gArgs.SoftSetBoolArg("-persistmempool", false)) { - LogPrintf("%s: parameter interaction: -zapwallettxes enabled -> setting -persistmempool=0\n", __func__); - } - - // -zapwallettxes implies a rescan - if (zapwallettxes) { - if (is_multiwallet) { - return InitError(strprintf(Untranslated("%s is only allowed with a single wallet file"), "-zapwallettxes")); - } - if (gArgs.SoftSetBoolArg("-rescan", true)) { - LogPrintf("%s: parameter interaction: -zapwallettxes enabled -> setting -rescan=1\n", __func__); - } + if (gArgs.IsArgSet("-zapwallettxes")) { + return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead.")); } if (gArgs.GetBoolArg("-sysperms", false)) @@ -129,5 +116,7 @@ void WalletInit::Construct(NodeContext& node) const settings.rw_settings["wallet"] = wallets; }); } - node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"))); + auto wallet_client = interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet")); + node.wallet_client = wallet_client.get(); + node.chain_clients.emplace_back(std::move(wallet_client)); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e0c3a1287a..9e36a09780 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -90,9 +90,9 @@ static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver, } } -UniValue importprivkey(const JSONRPCRequest& request) +RPCHelpMan importprivkey() { - RPCHelpMan{"importprivkey", + return RPCHelpMan{"importprivkey", "\nAdds a private key (as returned by dumpprivkey) to your wallet. Requires a new wallet backup.\n" "Hint: use importmulti to import more than one private key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" @@ -116,8 +116,8 @@ UniValue importprivkey(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("importprivkey", "\"mykey\", \"testing\", false") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -189,11 +189,13 @@ UniValue importprivkey(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue abortrescan(const JSONRPCRequest& request) +RPCHelpMan abortrescan() { - RPCHelpMan{"abortrescan", + return RPCHelpMan{"abortrescan", "\nStops current wallet rescan triggered by an RPC call, e.g. by an importprivkey call.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", {}, @@ -206,8 +208,8 @@ UniValue abortrescan(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("abortrescan", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -215,11 +217,13 @@ UniValue abortrescan(const JSONRPCRequest& request) if (!pwallet->IsScanning() || pwallet->IsAbortingRescan()) return false; pwallet->AbortRescan(); return true; +}, + }; } -UniValue importaddress(const JSONRPCRequest& request) +RPCHelpMan importaddress() { - RPCHelpMan{"importaddress", + return RPCHelpMan{"importaddress", "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" @@ -243,8 +247,8 @@ UniValue importaddress(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("importaddress", "\"myaddress\", \"testing\", false") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -315,11 +319,13 @@ UniValue importaddress(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue importprunedfunds(const JSONRPCRequest& request) +RPCHelpMan importprunedfunds() { - RPCHelpMan{"importprunedfunds", + return RPCHelpMan{"importprunedfunds", "\nImports funds without rescan. Corresponding address or script must previously be included in wallet. Aimed towards pruned wallets. The end-user is responsible to import additional transactions that subsequently spend the imported outputs or rescan after the point in the blockchain the transaction is included.\n", { {"rawtransaction", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A raw transaction in hex funding an already-existing address in wallet"}, @@ -327,8 +333,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request) }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{""}, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -371,11 +377,13 @@ UniValue importprunedfunds(const JSONRPCRequest& request) } throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "No addresses in wallet correspond to included transaction"); +}, + }; } -UniValue removeprunedfunds(const JSONRPCRequest& request) +RPCHelpMan removeprunedfunds() { - RPCHelpMan{"removeprunedfunds", + return RPCHelpMan{"removeprunedfunds", "\nDeletes the specified transaction from the wallet. Meant for use with pruned wallets and as a companion to importprunedfunds. This will affect wallet balances.\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex-encoded id of the transaction you are deleting"}, @@ -386,8 +394,8 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("removeprunedfunds", "\"a8d0c0184dde994a09ec054286f1ce581bebf46446a512166eae7628734ea0a5\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -408,11 +416,13 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue importpubkey(const JSONRPCRequest& request) +RPCHelpMan importpubkey() { - RPCHelpMan{"importpubkey", + return RPCHelpMan{"importpubkey", "\nAdds a public key (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "Hint: use importmulti to import more than one public key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" @@ -432,8 +442,8 @@ UniValue importpubkey(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("importpubkey", "\"mypubkey\", \"testing\", false") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -492,12 +502,14 @@ UniValue importpubkey(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue importwallet(const JSONRPCRequest& request) +RPCHelpMan importwallet() { - RPCHelpMan{"importwallet", + return RPCHelpMan{"importwallet", "\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { @@ -512,8 +524,8 @@ UniValue importwallet(const JSONRPCRequest& request) "\nImport using the json rpc call\n" + HelpExampleRpc("importwallet", "\"test\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -649,11 +661,13 @@ UniValue importwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding some keys/scripts to wallet"); return NullUniValue; +}, + }; } -UniValue dumpprivkey(const JSONRPCRequest& request) +RPCHelpMan dumpprivkey() { - RPCHelpMan{"dumpprivkey", + return RPCHelpMan{"dumpprivkey", "\nReveals the private key corresponding to 'address'.\n" "Then the importprivkey can be used with this output\n", { @@ -667,8 +681,8 @@ UniValue dumpprivkey(const JSONRPCRequest& request) + HelpExampleCli("importprivkey", "\"mykey\"") + HelpExampleRpc("dumpprivkey", "\"myaddress\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; const CWallet* const pwallet = wallet.get(); @@ -693,12 +707,14 @@ UniValue dumpprivkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } return EncodeSecret(vchSecret); +}, + }; } -UniValue dumpwallet(const JSONRPCRequest& request) +RPCHelpMan dumpwallet() { - RPCHelpMan{"dumpwallet", + return RPCHelpMan{"dumpwallet", "\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n" "Imported scripts are included in the dumpfile, but corresponding BIP173 addresses, etc. may not be added automatically by importwallet.\n" "Note that if your wallet contains keys which are not derived from your HD seed (e.g. imported keys), these are not covered by\n" @@ -716,8 +732,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) HelpExampleCli("dumpwallet", "\"test\"") + HelpExampleRpc("dumpwallet", "\"test\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; @@ -829,6 +845,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) reply.pushKV("filename", filepath.string()); return reply; +}, + }; } struct ImportData @@ -1239,9 +1257,9 @@ static int64_t GetImportTimestamp(const UniValue& data, int64_t now) throw JSONRPCError(RPC_TYPE_ERROR, "Missing required timestamp field for key"); } -UniValue importmulti(const JSONRPCRequest& mainRequest) +RPCHelpMan importmulti() { - RPCHelpMan{"importmulti", + return RPCHelpMan{"importmulti", "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), optionally rescanning the blockchain from the earliest creation time of the imported scripts. Requires a new wallet backup.\n" "If an address/script is imported without all of the private keys required to spend from that address, it will be watchonly. The 'watchonly' option must be set to true in this case or a warning will be returned.\n" "Conversely, if all the private keys are provided and the address/script is spendable, the watchonly option must be set to false, or a warning will be returned.\n" @@ -1314,8 +1332,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) "{ \"scriptPubKey\": { \"address\": \"<my 2nd address>\" }, \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") + HelpExampleCli("importmulti", "'[{ \"scriptPubKey\": { \"address\": \"<my address>\" }, \"timestamp\":1455191478 }]' '{ \"rescan\": false}'") }, - }.Check(mainRequest); - + [&](const RPCHelpMan& self, const JSONRPCRequest& mainRequest) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(mainRequest); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -1423,6 +1441,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } return response; +}, + }; } static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) @@ -1564,9 +1584,9 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& return result; } -UniValue importdescriptors(const JSONRPCRequest& main_request) +RPCHelpMan importdescriptors() { - RPCHelpMan{"importdescriptors", + return RPCHelpMan{"importdescriptors", "\nImport descriptors. This will trigger a rescan of the blockchain based on the earliest timestamp of all descriptors being imported. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if using an early timestamp; during that time, other rpc calls\n" "may report that the imported keys, addresses or scripts exist but related transactions are still missing.\n", @@ -1615,8 +1635,8 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) "{ \"desc\": \"<my desccriptor 2>\", \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") + HelpExampleCli("importdescriptors", "'[{ \"desc\": \"<my descriptor>\", \"timestamp\":1455191478, \"active\": true, \"range\": [0,100], \"label\": \"<my bech32 wallet>\" }]'") }, - }.Check(main_request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& main_request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(main_request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -1713,4 +1733,6 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) } return response; +}, + }; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7a03c4f544..7ef0b2129b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2061,6 +2061,7 @@ static UniValue lockunspent(const JSONRPCRequest& request) "Temporarily lock (unlock=false) or unlock (unlock=true) specified transaction outputs.\n" "If no transaction outputs are specified when unlocking then all current locked transaction outputs are unlocked.\n" "A locked transaction output will not be chosen by automatic coin selection, when spending bitcoins.\n" + "Manually selected coins are automatically unlocked.\n" "Locks are stored in memory only. Nodes start with zero locked outputs, and the locked output list\n" "is always cleared (by virtue of process exit) when a node stops or fails.\n" "Also see the listunspent call\n", @@ -2494,7 +2495,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) RPCHelpMan{"loadwallet", "\nLoads a wallet from a wallet file or directory." "\nNote that all wallet command-line options used when starting bitcoind will be" - "\napplied to the new wallet (eg -zapwallettxes, rescan, etc).\n", + "\napplied to the new wallet (eg -rescan, etc).\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, {"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."}, @@ -2836,6 +2837,15 @@ static UniValue listunspent(const JSONRPCRequest& request) if (!request.params[4].isNull()) { const UniValue& options = request.params[4].get_obj(); + RPCTypeCheckObj(options, + { + {"minimumAmount", UniValueType()}, + {"maximumAmount", UniValueType()}, + {"minimumSumAmount", UniValueType()}, + {"maximumCount", UniValueType(UniValue::VNUM)}, + }, + true, true); + if (options.exists("minimumAmount")) nMinimumAmount = AmountFromValue(options["minimumAmount"]); @@ -4161,17 +4171,17 @@ static UniValue upgradewallet(const JSONRPCRequest& request) return error.original; } -UniValue abortrescan(const JSONRPCRequest& request); // in rpcdump.cpp -UniValue dumpprivkey(const JSONRPCRequest& request); // in rpcdump.cpp -UniValue importprivkey(const JSONRPCRequest& request); -UniValue importaddress(const JSONRPCRequest& request); -UniValue importpubkey(const JSONRPCRequest& request); -UniValue dumpwallet(const JSONRPCRequest& request); -UniValue importwallet(const JSONRPCRequest& request); -UniValue importprunedfunds(const JSONRPCRequest& request); -UniValue removeprunedfunds(const JSONRPCRequest& request); -UniValue importmulti(const JSONRPCRequest& request); -UniValue importdescriptors(const JSONRPCRequest& request); +RPCHelpMan abortrescan(); +RPCHelpMan dumpprivkey(); +RPCHelpMan importprivkey(); +RPCHelpMan importaddress(); +RPCHelpMan importpubkey(); +RPCHelpMan dumpwallet(); +RPCHelpMan importwallet(); +RPCHelpMan importprunedfunds(); +RPCHelpMan removeprunedfunds(); +RPCHelpMan importmulti(); +RPCHelpMan importdescriptors(); Span<const CRPCCommand> GetWalletRPCCommands() { diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index a96d971734..14fb1fa89f 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -535,7 +535,7 @@ private: //! keeps track of whether Unlock has run a thorough check before bool m_decryption_thoroughly_checked = false; - bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey); + bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 35bd965673..c23dea1338 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -10,7 +10,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - m_chain_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {}); + m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {}); std::string sep; sep += fs::path::preferred_separator; diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h index c95b4f1f6e..f5bade77df 100644 --- a/src/wallet/test/init_test_fixture.h +++ b/src/wallet/test/init_test_fixture.h @@ -6,6 +6,7 @@ #define BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H #include <interfaces/chain.h> +#include <interfaces/wallet.h> #include <node/context.h> #include <test/util/setup_common.h> @@ -19,7 +20,7 @@ struct InitWalletDirTestingSetup: public BasicTestingSetup { fs::path m_cwd; std::map<std::string, fs::path> m_walletdir_path_cases; std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); - std::unique_ptr<interfaces::ChainClient> m_chain_client; + std::unique_ptr<interfaces::WalletClient> m_wallet_client; }; #endif // BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index c228e06009..9b905569fc 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -15,7 +15,7 @@ BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default) { SetWalletDir(m_walletdir_path_cases["default"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); @@ -25,7 +25,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom) { SetWalletDir(m_walletdir_path_cases["custom"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]); @@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist) SetWalletDir(m_walletdir_path_cases["nonexistent"]); { ASSERT_DEBUG_LOG("does not exist"); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == false); } } @@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_directory) SetWalletDir(m_walletdir_path_cases["file"]); { ASSERT_DEBUG_LOG("is not a directory"); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == false); } } @@ -57,7 +57,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative) SetWalletDir(m_walletdir_path_cases["relative"]); { ASSERT_DEBUG_LOG("is a relative path"); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == false); } } @@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) { SetWalletDir(m_walletdir_path_cases["trailing"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); @@ -75,7 +75,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) { SetWalletDir(m_walletdir_path_cases["trailing2"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 44f9eb5ddd..9241470745 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -11,5 +11,5 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) bool fFirstRun; m_wallet.LoadWallet(fFirstRun); m_chain_notifications_handler = m_chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); - m_chain_client->registerRpcs(); + m_wallet_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 99d7cfe921..12ad13b49b 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -21,7 +21,7 @@ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); - std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); + std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7ef06663b5..c46e90d64b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -24,9 +24,9 @@ #include <boost/test/unit_test.hpp> #include <univalue.h> -extern UniValue importmulti(const JSONRPCRequest& request); -extern UniValue dumpwallet(const JSONRPCRequest& request); -extern UniValue importwallet(const JSONRPCRequest& request); +RPCHelpMan importmulti(); +RPCHelpMan dumpwallet(); +RPCHelpMan importwallet(); // Ensure that fee levels defined in the wallet are at least as high // as the default levels for node policy. @@ -219,7 +219,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) request.params.setArray(); request.params.push_back(keys); - UniValue response = importmulti(request); + UniValue response = importmulti().HandleRequest(request); BOOST_CHECK_EQUAL(response.write(), strprintf("[{\"success\":false,\"error\":{\"code\":-1,\"message\":\"Rescan failed for key with creation " "timestamp %d. There was an error reading a block from time %d, which is after or within %d " @@ -274,7 +274,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.setArray(); request.params.push_back(backup_file); - ::dumpwallet(request); + ::dumpwallet().HandleRequest(request); RemoveWallet(wallet); } @@ -291,7 +291,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.push_back(backup_file); AddWallet(wallet); wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); - ::importwallet(request); + ::importwallet().HandleRequest(request); RemoveWallet(wallet); BOOST_CHECK_EQUAL(wallet->mapWallet.size(), 3U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c132a4be42..839e74a2be 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -183,11 +183,6 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocati return wallet; } -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) -{ - return LoadWallet(chain, WalletLocation(name), error, warnings); -} - WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result) { // Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted @@ -276,7 +271,7 @@ std::string COutput::ToString() const const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const { - LOCK(cs_wallet); + AssertLockHeld(cs_wallet); std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(hash); if (it == mapWallet.end()) return nullptr; @@ -1210,15 +1205,13 @@ void CWallet::BlockUntilSyncedToCurrentChain() const { isminetype CWallet::IsMine(const CTxIn &txin) const { + AssertLockHeld(cs_wallet); + std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash); + if (mi != mapWallet.end()) { - LOCK(cs_wallet); - std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash); - if (mi != mapWallet.end()) - { - const CWalletTx& prev = (*mi).second; - if (txin.prevout.n < prev.tx->vout.size()) - return IsMine(prev.tx->vout[txin.prevout.n]); - } + const CWalletTx& prev = (*mi).second; + if (txin.prevout.n < prev.tx->vout.size()) + return IsMine(prev.tx->vout[txin.prevout.n]); } return ISMINE_NO; } @@ -1243,16 +1236,19 @@ CAmount CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const isminetype CWallet::IsMine(const CTxOut& txout) const { + AssertLockHeld(cs_wallet); return IsMine(txout.scriptPubKey); } isminetype CWallet::IsMine(const CTxDestination& dest) const { + AssertLockHeld(cs_wallet); return IsMine(GetScriptForDestination(dest)); } isminetype CWallet::IsMine(const CScript& script) const { + AssertLockHeld(cs_wallet); isminetype result = ISMINE_NO; for (const auto& spk_man_pair : m_spk_managers) { result = std::max(result, spk_man_pair.second->IsMine(script)); @@ -1264,6 +1260,7 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons { if (!MoneyRange(txout.nValue)) throw std::runtime_error(std::string(__func__) + ": value out of range"); + LOCK(cs_wallet); return ((IsMine(txout) & filter) ? txout.nValue : 0); } @@ -1281,13 +1278,12 @@ bool CWallet::IsChange(const CScript& script) const // a better way of identifying which outputs are 'the send' and which are // 'the change' will need to be implemented (maybe extend CWalletTx to remember // which output, if any, was change). + AssertLockHeld(cs_wallet); if (IsMine(script)) { CTxDestination address; if (!ExtractDestination(script, address)) return true; - - LOCK(cs_wallet); if (!FindAddressBookEntry(address)) { return true; } @@ -1297,6 +1293,7 @@ bool CWallet::IsChange(const CScript& script) const CAmount CWallet::GetChange(const CTxOut& txout) const { + AssertLockHeld(cs_wallet); if (!MoneyRange(txout.nValue)) throw std::runtime_error(std::string(__func__) + ": value out of range"); return (IsChange(txout) ? txout.nValue : 0); @@ -1304,6 +1301,7 @@ CAmount CWallet::GetChange(const CTxOut& txout) const bool CWallet::IsMine(const CTransaction& tx) const { + AssertLockHeld(cs_wallet); for (const CTxOut& txout : tx.vout) if (IsMine(txout)) return true; @@ -1362,6 +1360,7 @@ CAmount CWallet::GetCredit(const CTransaction& tx, const isminefilter& filter) c CAmount CWallet::GetChange(const CTransaction& tx) const { + LOCK(cs_wallet); CAmount nChange = 0; for (const CTxOut& txout : tx.vout) { @@ -1597,6 +1596,7 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived, nFee = nDebit - nValueOut; } + LOCK(pwallet->cs_wallet); // Sent/received. for (unsigned int i = 0; i < tx->vout.size(); ++i) { @@ -1965,36 +1965,38 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted() const { - std::set<uint256> s; - return IsTrusted(s); + std::set<uint256> trusted_parents; + LOCK(pwallet->cs_wallet); + return pwallet->IsTrusted(*this, trusted_parents); } -bool CWalletTx::IsTrusted(std::set<uint256>& trusted_parents) const +bool CWallet::IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents) const { + AssertLockHeld(cs_wallet); // Quick answer in most cases - if (!pwallet->chain().checkFinalTx(*tx)) return false; - int nDepth = GetDepthInMainChain(); + if (!chain().checkFinalTx(*wtx.tx)) return false; + int nDepth = wtx.GetDepthInMainChain(); if (nDepth >= 1) return true; if (nDepth < 0) return false; // using wtx's cached debit - if (!pwallet->m_spend_zero_conf_change || !IsFromMe(ISMINE_ALL)) return false; + if (!m_spend_zero_conf_change || !wtx.IsFromMe(ISMINE_ALL)) return false; // Don't trust unconfirmed transactions from us unless they are in the mempool. - if (!InMempool()) return false; + if (!wtx.InMempool()) return false; // Trusted if all inputs are from us and are in the mempool: - for (const CTxIn& txin : tx->vin) + for (const CTxIn& txin : wtx.tx->vin) { // Transactions not sent by us: not trusted - const CWalletTx* parent = pwallet->GetWalletTx(txin.prevout.hash); + const CWalletTx* parent = GetWalletTx(txin.prevout.hash); if (parent == nullptr) return false; const CTxOut& parentOut = parent->tx->vout[txin.prevout.n]; // Check that this specific input being spent is trusted - if (pwallet->IsMine(parentOut) != ISMINE_SPENDABLE) return false; + if (IsMine(parentOut) != ISMINE_SPENDABLE) return false; // If we've already trusted this parent, continue if (trusted_parents.count(parent->GetHash())) continue; // Recurse to check that the parent is also trusted - if (!parent->IsTrusted(trusted_parents)) return false; + if (!IsTrusted(*parent, trusted_parents)) return false; trusted_parents.insert(parent->GetHash()); } return true; @@ -2080,7 +2082,7 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons for (const auto& entry : mapWallet) { const CWalletTx& wtx = entry.second; - const bool is_trusted{wtx.IsTrusted(trusted_parents)}; + const bool is_trusted{IsTrusted(wtx, trusted_parents)}; const int tx_depth{wtx.GetDepthInMainChain()}; const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; @@ -2148,7 +2150,7 @@ void CWallet::AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe, const if (nDepth == 0 && !wtx.InMempool()) continue; - bool safeTx = wtx.IsTrusted(trusted_parents); + bool safeTx = IsTrusted(wtx, trusted_parents); // We should not consider coins from transactions that are replacing // other transactions. @@ -2580,10 +2582,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC if (!coinControl.IsSelected(txin.prevout)) { tx.vin.push_back(txin); - if (lockUnspents) { - LockCoin(txin.prevout); - } } + if (lockUnspents) { + LockCoin(txin.prevout); + } + } return true; @@ -3172,28 +3175,10 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 return DBErrors::LOAD_OK; } -DBErrors CWallet::ZapWalletTx(std::list<CWalletTx>& vWtx) -{ - DBErrors nZapWalletTxRet = WalletBatch(*database,"cr+").ZapWalletTx(vWtx); - if (nZapWalletTxRet == DBErrors::NEED_REWRITE) - { - if (database->Rewrite("\x04pool")) - { - for (const auto& spk_man_pair : m_spk_managers) { - spk_man_pair.second->RewriteDB(); - } - } - } - - if (nZapWalletTxRet != DBErrors::LOAD_OK) - return nZapWalletTxRet; - - return DBErrors::LOAD_OK; -} - bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose) { bool fUpdated = false; + bool is_mine; { LOCK(cs_wallet); std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address); @@ -3201,8 +3186,9 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add m_address_book[address].SetLabel(strName); if (!strPurpose.empty()) /* update purpose only if requested */ m_address_book[address].purpose = strPurpose; + is_mine = IsMine(address) != ISMINE_NO; } - NotifyAddressBookChanged(this, address, strName, IsMine(address) != ISMINE_NO, + NotifyAddressBookChanged(this, address, strName, is_mine, strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) ); if (!strPurpose.empty() && !batch.WritePurpose(EncodeDestination(address), strPurpose)) return false; @@ -3217,17 +3203,16 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s bool CWallet::DelAddressBook(const CTxDestination& address) { - // 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) - // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! - // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). - if (IsMine(address)) { - WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); - return false; - } - + bool is_mine; { 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) + // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! + // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). + if (IsMine(address)) { + WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); + return false; + } // Delete destdata tuples associated with address std::string strAddress = EncodeDestination(address); for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata) @@ -3235,9 +3220,10 @@ bool CWallet::DelAddressBook(const CTxDestination& address) WalletBatch(*database).EraseDestData(strAddress, item.first); } m_address_book.erase(address); + is_mine = IsMine(address) != ISMINE_NO; } - NotifyAddressBookChanged(this, address, "", IsMine(address) != ISMINE_NO, "", CT_DELETED); + NotifyAddressBookChanged(this, address, "", is_mine, "", CT_DELETED); WalletBatch(*database).ErasePurpose(EncodeDestination(address)); return WalletBatch(*database).EraseName(EncodeDestination(address)); @@ -3345,7 +3331,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const { const CWalletTx& wtx = walletEntry.second; - if (!wtx.IsTrusted(trusted_parents)) + if (!IsTrusted(wtx, trusted_parents)) continue; if (wtx.IsImmatureCoinBase()) @@ -3364,9 +3350,6 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const continue; CAmount n = IsSpent(walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue; - - if (!balances.count(addr)) - balances[addr] = 0; balances[addr] += n; } } @@ -3770,20 +3753,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, { const std::string walletFile = WalletDataFilePath(location.GetPath()).string(); - // needed to restore wallet transaction meta data after -zapwallettxes - std::list<CWalletTx> vWtx; - - if (gArgs.GetBoolArg("-zapwallettxes", false)) { - chain.initMessage(_("Zapping all transactions from wallet...").translated); - - std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(&chain, location, CreateWalletDatabase(location.GetPath())); - DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); - if (nZapWalletRet != DBErrors::LOAD_OK) { - error = strprintf(_("Error loading %s: Wallet corrupted"), walletFile); - return nullptr; - } - } - chain.initMessage(_("Loading wallet...").translated); int64_t nStart = GetTimeMillis(); @@ -4060,30 +4029,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, } walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->database->IncrementUpdateCounter(); - - // Restore wallet transaction metadata after -zapwallettxes=1 - if (gArgs.GetBoolArg("-zapwallettxes", false) && gArgs.GetArg("-zapwallettxes", "1") != "2") - { - WalletBatch batch(*walletInstance->database); - - for (const CWalletTx& wtxOld : vWtx) - { - uint256 hash = wtxOld.GetHash(); - std::map<uint256, CWalletTx>::iterator mi = walletInstance->mapWallet.find(hash); - if (mi != walletInstance->mapWallet.end()) - { - const CWalletTx* copyFrom = &wtxOld; - CWalletTx* copyTo = &mi->second; - copyTo->mapValue = copyFrom->mapValue; - copyTo->vOrderForm = copyFrom->vOrderForm; - copyTo->nTimeReceived = copyFrom->nTimeReceived; - copyTo->nTimeSmart = copyFrom->nTimeSmart; - copyTo->fFromMe = copyFrom->fFromMe; - copyTo->nOrderPos = copyFrom->nOrderPos; - batch.WriteTx(*copyTo); - } - } - } } { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2f9d301000..42622c9312 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -275,7 +275,7 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, class CWalletTx { private: - const CWallet* pwallet; + const CWallet* const pwallet; /** Constant used in hashBlock to indicate tx has been abandoned, only used at * serialization/deserialization to avoid ambiguity with conflicted. @@ -502,7 +502,6 @@ public: bool InMempool() const; bool IsTrusted() const; - bool IsTrusted(std::set<uint256>& trusted_parents) const; int64_t GetTxTime() const; @@ -805,7 +804,8 @@ public: /** Interface for accessing chain state. */ interfaces::Chain& chain() const { assert(m_chain); return *m_chain; } - const CWalletTx* GetWalletTx(const uint256& hash) const; + const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! check whether we are allowed to upgrade (or already support) to the named feature bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } @@ -1051,20 +1051,20 @@ public: bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error); bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error); - isminetype IsMine(const CTxDestination& dest) const; - isminetype IsMine(const CScript& script) const; - isminetype IsMine(const CTxIn& txin) const; + isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + isminetype IsMine(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Returns amount of debit if the input matches the * filter, otherwise returns 0 */ CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; - isminetype IsMine(const CTxOut& txout) const; + isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const; - bool IsChange(const CTxOut& txout) const; - bool IsChange(const CScript& script) const; - CAmount GetChange(const CTxOut& txout) const; - bool IsMine(const CTransaction& tx) const; + bool IsChange(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsChange(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + CAmount GetChange(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** should probably be renamed to IsRelevantToMe */ bool IsFromMe(const CTransaction& tx) const; CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; @@ -1075,7 +1075,6 @@ public: void chainStateFlushed(const CBlockLocator& loc) override; DBErrors LoadWallet(bool& fFirstRunRet); - DBErrors ZapWalletTx(std::list<CWalletTx>& vWtx); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& purpose); @@ -1179,7 +1178,7 @@ public: * Obviously holding cs_main/cs_wallet when going into this call may cause * deadlock */ - void BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(cs_main, cs_wallet); + void BlockUntilSyncedToCurrentChain() const EXCLUSIVE_LOCKS_REQUIRED(!::cs_main, !cs_wallet); /** set a single wallet flag */ void SetWalletFlag(uint64_t flags); @@ -1285,7 +1284,7 @@ public: void LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal); //! Create new DescriptorScriptPubKeyMans and add them to the wallet - void SetupDescriptorScriptPubKeyMans(); + void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index fa6814d0d3..f25acee1e9 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -926,23 +926,6 @@ DBErrors WalletBatch::ZapSelectTx(std::vector<uint256>& vTxHashIn, std::vector<u return DBErrors::LOAD_OK; } -DBErrors WalletBatch::ZapWalletTx(std::list<CWalletTx>& vWtx) -{ - // build list of wallet TXs - std::vector<uint256> vTxHash; - DBErrors err = FindWalletTx(vTxHash, vWtx); - if (err != DBErrors::LOAD_OK) - return err; - - // erase each wallet TX - for (const uint256& hash : vTxHash) { - if (!EraseTx(hash)) - return DBErrors::CORRUPT; - } - - return DBErrors::LOAD_OK; -} - void MaybeCompactWalletDB() { static std::atomic<bool> fOneThread(false); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 64d60b1f44..c13de01319 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -257,7 +257,6 @@ public: DBErrors LoadWallet(CWallet* pwallet); DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx); - DBErrors ZapWalletTx(std::list<CWalletTx>& vWtx); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index 04806903c2..e2431cbbb7 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -86,6 +86,14 @@ bool CZMQAbstractPublishNotifier::Initialize(void *pcontext) return false; } + const int so_keepalive_option {1}; + rc = zmq_setsockopt(psocket, ZMQ_TCP_KEEPALIVE, &so_keepalive_option, sizeof(so_keepalive_option)); + if (rc != 0) { + zmqError("Failed to set SO_KEEPALIVE"); + zmq_close(psocket); + return false; + } + rc = zmq_bind(psocket, address.c_str()); if (rc != 0) { diff --git a/src/zmq/zmqrpc.cpp b/src/zmq/zmqrpc.cpp index cce6210129..1dd751b493 100644 --- a/src/zmq/zmqrpc.cpp +++ b/src/zmq/zmqrpc.cpp @@ -13,9 +13,9 @@ namespace { -UniValue getzmqnotifications(const JSONRPCRequest& request) +static RPCHelpMan getzmqnotifications() { - RPCHelpMan{"getzmqnotifications", + return RPCHelpMan{"getzmqnotifications", "\nReturns information about the active ZeroMQ notifications.\n", {}, RPCResult{ @@ -33,8 +33,8 @@ UniValue getzmqnotifications(const JSONRPCRequest& request) HelpExampleCli("getzmqnotifications", "") + HelpExampleRpc("getzmqnotifications", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ UniValue result(UniValue::VARR); if (g_zmq_notification_interface != nullptr) { for (const auto* n : g_zmq_notification_interface->GetActiveNotifiers()) { @@ -47,6 +47,8 @@ UniValue getzmqnotifications(const JSONRPCRequest& request) } return result; +}, + }; } const CRPCCommand commands[] = diff --git a/test/functional/feature_backwards_compatibility.py b/test/functional/feature_backwards_compatibility.py index 07dd0f8f82..126c9fe929 100755 --- a/test/functional/feature_backwards_compatibility.py +++ b/test/functional/feature_backwards_compatibility.py @@ -6,7 +6,7 @@ Test various backwards compatibility scenarios. Download the previous node binaries: -contrib/devtools/previous_release.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2 +test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2 v0.15.2 is not required by this test, but it is used in wallet_upgradewallet.py. Due to a hardfork in regtest, it can't be used to sync nodes. diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 89c55f31f3..ef4780cacb 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -54,28 +54,31 @@ class ZMQTest (BitcoinTestFramework): self.ctx.destroy(linger=None) def test_basic(self): - # All messages are received in the same socket which means - # that this test fails if the publishing order changes. - # Note that the publishing order is not defined in the documentation and - # is subject to change. import zmq # Invalid zmq arguments don't take down the node, see #17185. self.restart_node(0, ["-zmqpubrawtx=foo", "-zmqpubhashtx=bar"]) address = 'tcp://127.0.0.1:28332' - socket = self.ctx.socket(zmq.SUB) - socket.set(zmq.RCVTIMEO, 60000) + sockets = [] + subs = [] + services = [b"hashblock", b"hashtx", b"rawblock", b"rawtx"] + for service in services: + sockets.append(self.ctx.socket(zmq.SUB)) + sockets[-1].set(zmq.RCVTIMEO, 60000) + subs.append(ZMQSubscriber(sockets[-1], service)) # Subscribe to all available topics. - hashblock = ZMQSubscriber(socket, b"hashblock") - hashtx = ZMQSubscriber(socket, b"hashtx") - rawblock = ZMQSubscriber(socket, b"rawblock") - rawtx = ZMQSubscriber(socket, b"rawtx") + hashblock = subs[0] + hashtx = subs[1] + rawblock = subs[2] + rawtx = subs[3] self.restart_node(0, ["-zmqpub%s=%s" % (sub.topic.decode(), address) for sub in [hashblock, hashtx, rawblock, rawtx]]) connect_nodes(self.nodes[0], 1) - socket.connect(address) + for socket in sockets: + socket.connect(address) + # Relax so that the subscriber is ready before publishing zmq messages sleep(0.2) @@ -96,15 +99,16 @@ class ZMQTest (BitcoinTestFramework): tx.calc_sha256() assert_equal(tx.hash, txid.hex()) + # Should receive the generated raw block. + block = rawblock.receive() + assert_equal(genhashes[x], hash256_reversed(block[:80]).hex()) + # Should receive the generated block hash. hash = hashblock.receive().hex() assert_equal(genhashes[x], hash) # The block should only have the coinbase txid. assert_equal([txid.hex()], self.nodes[1].getblock(hash)["tx"]) - # Should receive the generated raw block. - block = rawblock.receive() - assert_equal(genhashes[x], hash256_reversed(block[:80]).hex()) if self.is_wallet_compiled(): self.log.info("Wait for tx from second node") @@ -119,6 +123,13 @@ class ZMQTest (BitcoinTestFramework): hex = rawtx.receive() assert_equal(payment_txid, hash256_reversed(hex).hex()) + # Mining the block with this tx should result in second notification + # after coinbase tx notification + self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE) + hashtx.receive() + txid = hashtx.receive() + assert_equal(payment_txid, txid.hex()) + self.log.info("Test the getzmqnotifications RPC") assert_equal(self.nodes[0].getzmqnotifications(), [ @@ -131,30 +142,67 @@ class ZMQTest (BitcoinTestFramework): assert_equal(self.nodes[1].getzmqnotifications(), []) def test_reorg(self): + if not self.is_wallet_compiled(): + self.log.info("Skipping reorg test because wallet is disabled") + return + import zmq address = 'tcp://127.0.0.1:28333' - socket = self.ctx.socket(zmq.SUB) - socket.set(zmq.RCVTIMEO, 60000) - hashblock = ZMQSubscriber(socket, b'hashblock') + + services = [b"hashblock", b"hashtx"] + sockets = [] + subs = [] + for service in services: + sockets.append(self.ctx.socket(zmq.SUB)) + # 2 second timeout to check end of notifications + sockets[-1].set(zmq.RCVTIMEO, 2000) + subs.append(ZMQSubscriber(sockets[-1], service)) + + # Subscribe to all available topics. + hashblock = subs[0] + hashtx = subs[1] # Should only notify the tip if a reorg occurs - self.restart_node(0, ['-zmqpub%s=%s' % (hashblock.topic.decode(), address)]) - socket.connect(address) + self.restart_node(0, ["-zmqpub%s=%s" % (sub.topic.decode(), address) for sub in [hashblock, hashtx]]) + for socket in sockets: + socket.connect(address) # Relax so that the subscriber is ready before publishing zmq messages sleep(0.2) - # Generate 1 block in nodes[0] and receive all notifications - self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE) + # Generate 1 block in nodes[0] with 1 mempool tx and receive all notifications + payment_txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) + disconnect_block = self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)[0] + disconnect_cb = self.nodes[0].getblock(disconnect_block)["tx"][0] assert_equal(self.nodes[0].getbestblockhash(), hashblock.receive().hex()) + assert_equal(hashtx.receive().hex(), payment_txid) + assert_equal(hashtx.receive().hex(), disconnect_cb) # Generate 2 blocks in nodes[1] - self.nodes[1].generatetoaddress(2, ADDRESS_BCRT1_UNSPENDABLE) + connect_blocks = self.nodes[1].generatetoaddress(2, ADDRESS_BCRT1_UNSPENDABLE) # nodes[0] will reorg chain after connecting back nodes[1] connect_nodes(self.nodes[0], 1) + self.sync_blocks() # tx in mempool valid but not advertised # Should receive nodes[1] tip assert_equal(self.nodes[1].getbestblockhash(), hashblock.receive().hex()) + # During reorg: + # Get old payment transaction notification from disconnect and disconnected cb + assert_equal(hashtx.receive().hex(), payment_txid) + assert_equal(hashtx.receive().hex(), disconnect_cb) + # And the payment transaction again due to mempool entry + assert_equal(hashtx.receive().hex(), payment_txid) + assert_equal(hashtx.receive().hex(), payment_txid) + # And the new connected coinbases + for i in [0, 1]: + assert_equal(hashtx.receive().hex(), self.nodes[1].getblock(connect_blocks[i])["tx"][0]) + + # If we do a simple invalidate we announce the disconnected coinbase + self.nodes[0].invalidateblock(connect_blocks[1]) + assert_equal(hashtx.receive().hex(), self.nodes[1].getblock(connect_blocks[1])["tx"][0]) + # And the current tip + assert_equal(hashtx.receive().hex(), self.nodes[1].getblock(connect_blocks[0])["tx"][0]) + if __name__ == '__main__': ZMQTest().main() diff --git a/test/functional/mempool_compatibility.py b/test/functional/mempool_compatibility.py index 31fb751904..75ca8d3236 100755 --- a/test/functional/mempool_compatibility.py +++ b/test/functional/mempool_compatibility.py @@ -8,7 +8,7 @@ NOTE: The test is designed to prevent cases when compatibility is broken acciden In case we need to break mempool compatibility we can continue to use the test by just bumping the version number. Download node binaries: -contrib/devtools/previous_release.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2 +test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2 Only v0.15.2 is required by this test. The rest is used in other backwards compatibility tests. """ diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 2155c1d0e7..9503391030 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -25,6 +25,7 @@ from test_framework.messages import ( MSG_BLOCK, MSG_TX, MSG_WITNESS_FLAG, + MSG_WITNESS_TX, MSG_WTX, NODE_NETWORK, NODE_WITNESS, @@ -2157,7 +2158,7 @@ class SegWitTest(BitcoinTestFramework): self.wtx_node.wait_for_getdata([tx.sha256], 60) with p2p_lock: lgd = self.wtx_node.lastgetdata[:] - assert_equal(lgd, [CInv(MSG_TX|MSG_WITNESS_FLAG, tx.sha256)]) + assert_equal(lgd, [CInv(MSG_WITNESS_TX, tx.sha256)]) # Send tx through test_transaction_acceptance(self.nodes[0], self.wtx_node, tx, with_witness=False, accepted=True) diff --git a/test/functional/rpc_generate.py b/test/functional/rpc_generate.py index 9404f1e25e..e55f2e6d12 100755 --- a/test/functional/rpc_generate.py +++ b/test/functional/rpc_generate.py @@ -17,7 +17,8 @@ class RPCGenerateTest(BitcoinTestFramework): def run_test(self): message = ( - "generate ( nblocks maxtries ) has been replaced by the -generate " + "generate\n" + "has been replaced by the -generate " "cli option. Refer to -help for more information." ) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index f7f23bc8f4..1c7dc98d16 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -103,7 +103,16 @@ class PSBTTest(BitcoinTestFramework): final_tx = self.nodes[0].finalizepsbt(signed_tx)['hex'] self.nodes[0].sendrawtransaction(final_tx) - # Get pubkeys + # Manually selected inputs can be locked: + assert_equal(len(self.nodes[0].listlockunspent()), 0) + utxo1 = self.nodes[0].listunspent()[0] + psbtx1 = self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():1}, 0,{"lockUnspents": True})["psbt"] + assert_equal(len(self.nodes[0].listlockunspent()), 1) + + # Locks are ignored for manually selected inputs + self.nodes[0].walletcreatefundedpsbt([{"txid": utxo1['txid'], "vout": utxo1['vout']}], {self.nodes[2].getnewaddress():1}, 0) + + # Create p2sh, p2wpkh, and p2wsh addresses pubkey0 = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())['pubkey'] pubkey1 = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress())['pubkey'] pubkey2 = self.nodes[2].getaddressinfo(self.nodes[2].getnewaddress())['pubkey'] diff --git a/test/functional/test_framework/key.py b/test/functional/test_framework/key.py index 912c0ca978..adbffb7dc7 100644 --- a/test/functional/test_framework/key.py +++ b/test/functional/test_framework/key.py @@ -8,22 +8,7 @@ keys, and is trivially vulnerable to side channel attacks. Do not use for anything but tests.""" import random -def modinv(a, n): - """Compute the modular inverse of a modulo n - - See https://en.wikipedia.org/wiki/Extended_Euclidean_algorithm#Modular_integers. - """ - t1, t2 = 0, 1 - r1, r2 = n, a - while r2 != 0: - q = r1 // r2 - t1, t2 = t2, t1 - q * t2 - r1, r2 = r2, r1 - q * r2 - if r1 > 1: - return None - if t1 < 0: - t1 += n - return t1 +from .util import modinv def jacobi_symbol(n, k): """Compute the Jacobi symbol of n modulo k diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index bd4a53876e..8d68af2d07 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -63,6 +63,7 @@ MSG_CMPCT_BLOCK = 4 MSG_WTX = 5 MSG_WITNESS_FLAG = 1 << 30 MSG_TYPE_MASK = 0xffffffff >> 2 +MSG_WITNESS_TX = MSG_TX | MSG_WITNESS_FLAG FILTER_TYPE_BASIC = 0 diff --git a/test/functional/test_framework/muhash.py b/test/functional/test_framework/muhash.py new file mode 100644 index 0000000000..97d02359cb --- /dev/null +++ b/test/functional/test_framework/muhash.py @@ -0,0 +1,110 @@ +# Copyright (c) 2020 Pieter Wuille +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Native Python MuHash3072 implementation.""" + +import hashlib +import unittest + +from .util import modinv + +def rot32(v, bits): + """Rotate the 32-bit value v left by bits bits.""" + bits %= 32 # Make sure the term below does not throw an exception + return ((v << bits) & 0xffffffff) | (v >> (32 - bits)) + +def chacha20_doubleround(s): + """Apply a ChaCha20 double round to 16-element state array s. + + See https://cr.yp.to/chacha/chacha-20080128.pdf and https://tools.ietf.org/html/rfc8439 + """ + QUARTER_ROUNDS = [(0, 4, 8, 12), + (1, 5, 9, 13), + (2, 6, 10, 14), + (3, 7, 11, 15), + (0, 5, 10, 15), + (1, 6, 11, 12), + (2, 7, 8, 13), + (3, 4, 9, 14)] + + for a, b, c, d in QUARTER_ROUNDS: + s[a] = (s[a] + s[b]) & 0xffffffff + s[d] = rot32(s[d] ^ s[a], 16) + s[c] = (s[c] + s[d]) & 0xffffffff + s[b] = rot32(s[b] ^ s[c], 12) + s[a] = (s[a] + s[b]) & 0xffffffff + s[d] = rot32(s[d] ^ s[a], 8) + s[c] = (s[c] + s[d]) & 0xffffffff + s[b] = rot32(s[b] ^ s[c], 7) + +def chacha20_32_to_384(key32): + """Specialized ChaCha20 implementation with 32-byte key, 0 IV, 384-byte output.""" + # See RFC 8439 section 2.3 for chacha20 parameters + CONSTANTS = [0x61707865, 0x3320646e, 0x79622d32, 0x6b206574] + + key_bytes = [0]*8 + for i in range(8): + key_bytes[i] = int.from_bytes(key32[(4 * i):(4 * (i+1))], 'little') + + INITIALIZATION_VECTOR = [0] * 4 + init = CONSTANTS + key_bytes + INITIALIZATION_VECTOR + out = bytearray() + for counter in range(6): + init[12] = counter + s = init.copy() + for _ in range(10): + chacha20_doubleround(s) + for i in range(16): + out.extend(((s[i] + init[i]) & 0xffffffff).to_bytes(4, 'little')) + return bytes(out) + +def data_to_num3072(data): + """Hash a 32-byte array data to a 3072-bit number using 6 Chacha20 operations.""" + bytes384 = chacha20_32_to_384(data) + return int.from_bytes(bytes384, 'little') + +class MuHash3072: + """Class representing the MuHash3072 computation of a set. + + See https://cseweb.ucsd.edu/~mihir/papers/inchash.pdf and https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-May/014337.html + """ + + MODULUS = 2**3072 - 1103717 + + def __init__(self): + """Initialize for an empty set.""" + self.numerator = 1 + self.denominator = 1 + + def insert(self, data): + """Insert a byte array data in the set.""" + self.numerator = (self.numerator * data_to_num3072(data)) % self.MODULUS + + def remove(self, data): + """Remove a byte array from the set.""" + self.denominator = (self.denominator * data_to_num3072(data)) % self.MODULUS + + def digest(self): + """Extract the final hash. Does not modify this object.""" + val = (self.numerator * modinv(self.denominator, self.MODULUS)) % self.MODULUS + bytes384 = val.to_bytes(384, 'little') + return hashlib.sha256(bytes384).digest() + +class TestFrameworkMuhash(unittest.TestCase): + def test_muhash(self): + muhash = MuHash3072() + muhash.insert([0]*32) + muhash.insert([1] + [0]*31) + muhash.remove([2] + [0]*31) + finalized = muhash.digest() + # This mirrors the result in the C++ MuHash3072 unit test + self.assertEqual(finalized[::-1].hex(), "a44e16d5e34d259b349af21c06e65d653915d2e208e4e03f389af750dc0bfdc3") + + def test_chacha20(self): + def chacha_check(key, result): + self.assertEqual(chacha20_32_to_384(key)[:64].hex(), result) + + # Test vectors from https://tools.ietf.org/html/draft-agl-tls-chacha20poly1305-04#section-7 + # Since the nonce is hardcoded to 0 in our function we only use those vectors. + chacha_check([0]*32, "76b8e0ada0f13d90405d6ae55386bd28bdd219b8a08ded1aa836efcc8b770dc7da41597c5157488d7724e03fb8d84a376a43b8f41518a11cc387b669b2ee6586") + chacha_check([0]*31 + [1], "4540f05a9f1fb296d7736e7b208e3c96eb4fe1834688d2604f450952ed432d41bbe2a0b6ea7566d2a5d1e7e20d42af2c53d792b1c43fea817e9ad275ae546963") diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 6c8024c01c..af7f0b62f4 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -15,6 +15,7 @@ import os import random import re import time +import unittest from . import coverage from .authproxy import AuthServiceProxy, JSONRPCException @@ -625,3 +626,33 @@ def find_vout_for_address(node, txid, addr): if any([addr == a for a in tx["vout"][i]["scriptPubKey"]["addresses"]]): return i raise RuntimeError("Vout not found for address: txid=%s, addr=%s" % (txid, addr)) + +def modinv(a, n): + """Compute the modular inverse of a modulo n using the extended Euclidean + Algorithm. See https://en.wikipedia.org/wiki/Extended_Euclidean_algorithm#Modular_integers. + """ + # TODO: Change to pow(a, -1, n) available in Python 3.8 + t1, t2 = 0, 1 + r1, r2 = n, a + while r2 != 0: + q = r1 // r2 + t1, t2 = t2, t1 - q * t2 + r1, r2 = r2, r1 - q * r2 + if r1 > 1: + return None + if t1 < 0: + t1 += n + return t1 + +class TestFrameworkUtil(unittest.TestCase): + def test_modinv(self): + test_vectors = [ + [7, 11], + [11, 29], + [90, 13], + [1891, 3797], + [6003722857, 77695236973], + ] + + for a, n in test_vectors: + self.assertEqual(modinv(a, n), pow(a, n-2, n)) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 01232bda3c..578afe5f30 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -69,7 +69,9 @@ TEST_EXIT_SKIPPED = 77 TEST_FRAMEWORK_MODULES = [ "address", "blocktools", + "muhash", "script", + "util", ] EXTENDED_SCRIPTS = [ @@ -105,7 +107,6 @@ BASE_SCRIPTS = [ 'wallet_listtransactions.py', # vv Tests less than 60s vv 'p2p_sendheaders.py', - 'wallet_zapwallettxes.py', 'wallet_importmulti.py', 'mempool_limit.py', 'rpc_txoutproof.py', diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 71a1a3f4f6..147c43f2f7 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -135,11 +135,19 @@ class WalletTest(BitcoinTestFramework): self.nodes[2].lockunspent, False, [{"txid": unspent_0["txid"], "vout": 999}]) - # An output should be unlocked when spent + # The lock on a manually selected output is ignored unspent_0 = self.nodes[1].listunspent()[0] self.nodes[1].lockunspent(False, [unspent_0]) tx = self.nodes[1].createrawtransaction([unspent_0], { self.nodes[1].getnewaddress() : 1 }) - tx = self.nodes[1].fundrawtransaction(tx)['hex'] + self.nodes[1].fundrawtransaction(tx,{"lockUnspents": True}) + + # fundrawtransaction can lock an input + self.nodes[1].lockunspent(True, [unspent_0]) + assert_equal(len(self.nodes[1].listlockunspent()), 0) + tx = self.nodes[1].fundrawtransaction(tx,{"lockUnspents": True})['hex'] + assert_equal(len(self.nodes[1].listlockunspent()), 1) + + # Send transaction tx = self.nodes[1].signrawtransactionwithwallet(tx)["hex"] self.nodes[1].sendrawtransaction(tx) assert_equal(len(self.nodes[1].listlockunspent()), 0) @@ -526,8 +534,6 @@ class WalletTest(BitcoinTestFramework): maintenance = [ '-rescan', '-reindex', - '-zapwallettxes=1', - '-zapwallettxes=2', ] chainlimit = 6 for m in maintenance: diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 1872545cdb..5c9d7ff629 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -134,11 +134,6 @@ class MultiWalletTest(BitcoinTestFramework): open(not_a_dir, 'a', encoding="utf8").close() self.nodes[0].assert_start_raises_init_error(['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory') - self.log.info("Do not allow -zapwallettxes with multiwallet") - self.nodes[0].assert_start_raises_init_error(['-zapwallettxes', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=2', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') os.rename(wallet_dir(), wallet_dir2) diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 1a76f65215..f3d6e74829 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/test/functional/wallet_upgradewallet.py @@ -6,7 +6,7 @@ Test upgradewallet RPC. Download node binaries: -contrib/devtools/previous_release.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2 +test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2 Only v0.15.2 and v0.16.3 are required by this test. The others are used in feature_backwards_compatibility.py """ diff --git a/test/functional/wallet_zapwallettxes.py b/test/functional/wallet_zapwallettxes.py deleted file mode 100755 index 1287092cac..0000000000 --- a/test/functional/wallet_zapwallettxes.py +++ /dev/null @@ -1,79 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2014-2018 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test the zapwallettxes functionality. - -- start two bitcoind nodes -- create two transactions on node 0 - one is confirmed and one is unconfirmed. -- restart node 0 and verify that both the confirmed and the unconfirmed - transactions are still available. -- restart node 0 with zapwallettxes and persistmempool, and verify that both - the confirmed and the unconfirmed transactions are still available. -- restart node 0 with just zapwallettxes and verify that the confirmed - transactions are still available, but that the unconfirmed transaction has - been zapped. -""" -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import ( - assert_equal, - assert_raises_rpc_error, -) - - -class ZapWalletTXesTest (BitcoinTestFramework): - def set_test_params(self): - self.setup_clean_chain = True - self.num_nodes = 2 - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def run_test(self): - self.log.info("Mining blocks...") - self.nodes[0].generate(1) - self.sync_all() - self.nodes[1].generate(100) - self.sync_all() - - # This transaction will be confirmed - txid1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 10) - - self.nodes[0].generate(1) - self.sync_all() - - # This transaction will not be confirmed - txid2 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 20) - - # Confirmed and unconfirmed transactions are now in the wallet. - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - - # Restart node0. Both confirmed and unconfirmed transactions remain in the wallet. - self.restart_node(0) - - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - - # Restart node0 with zapwallettxes and persistmempool. The unconfirmed - # transaction is zapped from the wallet, but is re-added when the mempool is reloaded. - self.restart_node(0, ["-persistmempool=1", "-zapwallettxes=2"]) - - self.wait_until(lambda: self.nodes[0].getmempoolinfo()['size'] == 1, timeout=3) - self.nodes[0].syncwithvalidationinterfacequeue() # Flush mempool to wallet - - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - - # Restart node0 with zapwallettxes, but not persistmempool. - # The unconfirmed transaction is zapped and is no longer in the wallet. - self.restart_node(0, ["-zapwallettxes=2"]) - - # tx1 is still be available because it was confirmed - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - - # This will raise an exception because the unconfirmed transaction has been zapped - assert_raises_rpc_error(-5, 'Invalid or non-wallet transaction id', self.nodes[0].gettransaction, txid2) - -if __name__ == '__main__': - ZapWalletTXesTest().main() diff --git a/contrib/devtools/previous_release.py b/test/get_previous_releases.py index 5599051cf3..1348b8246b 100755 --- a/contrib/devtools/previous_release.py +++ b/test/get_previous_releases.py @@ -20,6 +20,40 @@ import sys import hashlib +SHA256_SUMS = { +"d40f18b4e43c6e6370ef7db9131f584fbb137276ec2e3dba67a4b267f81cb644": "bitcoin-0.15.2-aarch64-linux-gnu.tar.gz", +"54fb877a148a6ad189a1e1ab1ff8b11181e58ff2aaf430da55b3fd46ae549a6b": "bitcoin-0.15.2-arm-linux-gnueabihf.tar.gz", +"2b843506c3f1af0eeca5854a920264f9a829f02d0d50328005950ddcbe88874d": "bitcoin-0.15.2-i686-pc-linux-gnu.tar.gz", +"87e9340ff3d382d543b2b69112376077f0c8b4f7450d372e83b68f5a1e22b2df": "bitcoin-0.15.2-osx64.tar.gz", +"566be44190fd76daa01f13d428939dadfb8e3daacefc8fa17f433cad28f73bd5": "bitcoin-0.15.2-x86_64-linux-gnu.tar.gz", + +"0768c6c15caffbaca6524824c9563b42c24f70633c681c2744649158aa3fd484": "bitcoin-0.16.3-aarch64-linux-gnu.tar.gz", +"fb2818069854a6ad20ea03b28b55dbd35d8b1f7d453e90b83eace5d0098a2a87": "bitcoin-0.16.3-arm-linux-gnueabihf.tar.gz", +"75a537844313b0a84bdb61ffcdc5c4ce19a738f7ddf71007cd2edf664efd7c37": "bitcoin-0.16.3-i686-pc-linux-gnu.tar.gz", +"78c3bff3b619a19aed575961ea43cc9e142959218835cf51aede7f0b764fc25d": "bitcoin-0.16.3-osx64.tar.gz", +"5d422a9d544742bc0df12427383f9c2517433ce7b58cf672b9a9b17c2ef51e4f": "bitcoin-0.16.3-x86_64-linux-gnu.tar.gz", + +"5a6b35d1a348a402f2d2d6ab5aed653a1a1f13bc63aaaf51605e3501b0733b7a": "bitcoin-0.17.2-aarch64-linux-gnu.tar.gz", +"d1913a5d19c8e8da4a67d1bd5205d03c8614dfd2e02bba2fe3087476643a729e": "bitcoin-0.17.2-arm-linux-gnueabihf.tar.gz", +"d295fc93f39bbf0fd937b730a93184899a2eb6c3a6d53f3d857cbe77ef89b98c": "bitcoin-0.17.2-i686-pc-linux-gnu.tar.gz", +"a783ba20706dbfd5b47fbedf42165fce70fbbc7d78003305d964f6b3da14887f": "bitcoin-0.17.2-osx64.tar.gz", +"943f9362b9f11130177839116f48f809d83478b4c28591d486ee9a7e35179da6": "bitcoin-0.17.2-x86_64-linux-gnu.tar.gz", + +"88f343af72803b851c7da13874cc5525026b0b55e63e1b5e1298390c4688adc6": "bitcoin-0.18.1-aarch64-linux-gnu.tar.gz", +"cc7d483e4b20c5dabd4dcaf304965214cf4934bcc029ca99cbc9af00d3771a1f": "bitcoin-0.18.1-arm-linux-gnueabihf.tar.gz", +"989e847b3e95fc9fedc0b109cae1b4fa43348f2f712e187a118461876af9bd16": "bitcoin-0.18.1-i686-pc-linux-gnu.tar.gz", +"b7bbcee7a7540f711b171d6981f939ca8482005fde22689bc016596d80548bb1": "bitcoin-0.18.1-osx64.tar.gz", +"425ee5ec631ae8da71ebc1c3f5c0269c627cf459379b9b030f047107a28e3ef8": "bitcoin-0.18.1-riscv64-linux-gnu.tar.gz", +"600d1db5e751fa85903e935a01a74f5cc57e1e7473c15fd3e17ed21e202cfe5a": "bitcoin-0.18.1-x86_64-linux-gnu.tar.gz", + +"3a80431717842672df682bdb619e66523b59541483297772a7969413be3502ff": "bitcoin-0.19.1-aarch64-linux-gnu.tar.gz", +"657f28213823d240dd3324d14829702f9ad6f0710f8bdd1c379cb3c447197f48": "bitcoin-0.19.1-arm-linux-gnueabihf.tar.gz", +"10d1e53208aa7603022f4acc084a046299ab4ccf25fe01e81b3fb6f856772589": "bitcoin-0.19.1-i686-pc-linux-gnu.tar.gz", +"1ae1b87de26487075cd2fd22e0d4ead87d969bd55c44f2f1d873ecdc6147ebb3": "bitcoin-0.19.1-osx64.tar.gz", +"aa7a9563b48aa79252c8e7b6a41c07a5441bd9f14c5e4562cc72720ea6cb0ee5": "bitcoin-0.19.1-riscv64-linux-gnu.tar.gz", +"5fcac9416e486d4960e1a946145566350ca670f9aaba99de6542080851122e4c": "bitcoin-0.19.1-x86_64-linux-gnu.tar.gz" +} + @contextlib.contextmanager def pushd(new_dir) -> None: previous_dir = os.getcwd() @@ -44,14 +78,10 @@ def download_binary(tag, args) -> int: match.group(1), match.group(2)) tarball = 'bitcoin-{tag}-{platform}.tar.gz'.format( tag=tag[1:], platform=args.platform) - sha256Sums = "SHA256SUMS-{tag}.asc".format(tag=tag[1:]) tarballUrl = 'https://bitcoincore.org/{bin_path}/{tarball}'.format( bin_path=bin_path, tarball=tarball) - sha256SumsUrl = 'https://bitcoincore.org/{bin_path}/SHA256SUMS.asc'.format( - bin_path=bin_path) print('Fetching: {tarballUrl}'.format(tarballUrl=tarballUrl)) - print('Fetching: {sha256SumsUrl}'.format(sha256SumsUrl=sha256SumsUrl)) header, status = subprocess.Popen( ['curl', '--head', tarballUrl], stdout=subprocess.PIPE).communicate() @@ -60,8 +90,7 @@ def download_binary(tag, args) -> int: return 1 curlCmds = [ - ['curl', '--remote-name', tarballUrl], - ['curl', '--output', sha256Sums, sha256SumsUrl], + ['curl', '--remote-name', tarballUrl] ] for cmd in curlCmds: @@ -73,29 +102,12 @@ def download_binary(tag, args) -> int: with open(tarball, "rb") as afile: hasher.update(afile.read()) tarballHash = hasher.hexdigest() - tarballHash = '{} {}\n'.format(tarballHash, tarball) - with open(sha256Sums, 'r', encoding="utf-8") as afile: - shasums = afile.readlines() - if tarballHash not in shasums: + if tarballHash not in SHA256_SUMS or SHA256_SUMS[tarballHash] != tarball: print("Checksum did not match") - Path(tarball).unlink() return 1 print("Checksum matched") - # Bitcoin Core Release Signing Keys v0.11.0+ - signingKey = "01EA5486DE18A882D4C2684590C8019E36C2E964" - - isKeyPresent = subprocess.run( - ["gpg", "--list-keys", signingKey]).returncode - if isKeyPresent: - return isKeyPresent - - isVerified = subprocess.run( - ["gpg", "--verify", sha256Sums]).returncode - if isVerified: - return isVerified - # Extract tarball ret = subprocess.run(['tar', '-zxf', tarball, '-C', tag, '--strip-components=1', @@ -104,7 +116,6 @@ def download_binary(tag, args) -> int: return ret Path(tarball).unlink() - Path(sha256Sums).unlink() return 0 diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index bd947d194c..f77242d335 100755 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -23,7 +23,7 @@ CMD_GREP_WALLET_ARGS = r"git grep --function-context 'void WalletInit::AddWallet CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR) CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR) # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb']) +SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb', '-zapwallettxes']) def lint_missing_argument_documentation(): diff --git a/test/lint/lint-cpp.sh b/test/lint/lint-cpp.sh new file mode 100755 index 0000000000..cac57b968d --- /dev/null +++ b/test/lint/lint-cpp.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# +# Copyright (c) 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. +# +# Check for various C++ code patterns we want to avoid. + +export LC_ALL=C + +EXIT_CODE=0 + +OUTPUT=$(git grep -E "boost::bind\(" -- "*.cpp" "*.h") +if [[ ${OUTPUT} != "" ]]; then + echo "Use of boost::bind detected. Use std::bind instead." + echo + echo "${OUTPUT}" + EXIT_CODE=1 +fi + +exit ${EXIT_CODE}
\ No newline at end of file diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index 2e5b801849..e5657f7555 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -4,6 +4,39 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. export LC_ALL=C + +# Be aware that bitcoind and bitcoin-qt differ in terms of localization: Qt +# opts in to POSIX localization by running setlocale(LC_ALL, "") on startup, +# whereas no such call is made in bitcoind. +# +# Qt runs setlocale(LC_ALL, "") on initialization. This installs the locale +# specified by the user's LC_ALL (or LC_*) environment variable as the new +# C locale. +# +# In contrast, bitcoind does not opt in to localization -- no call to +# setlocale(LC_ALL, "") is made and the environment variables LC_* are +# thus ignored. +# +# This results in situations where bitcoind is guaranteed to be running +# with the classic locale ("C") whereas the locale of bitcoin-qt will vary +# depending on the user's environment variables. +# +# An example: Assuming the environment variable LC_ALL=de_DE then the +# call std::to_string(1.23) will return "1.230000" in bitcoind but +# "1,230000" in bitcoin-qt. +# +# From the Qt documentation: +# "On Unix/Linux Qt is configured to use the system locale settings by default. +# This can cause a conflict when using POSIX functions, for instance, when +# converting between data types such as floats and strings, since the notation +# may differ between locales. To get around this problem, call the POSIX function +# setlocale(LC_NUMERIC,"C") right after initializing QApplication, QGuiApplication +# or QCoreApplication to reset the locale that is used for number formatting to +# "C"-locale." +# +# See https://doc.qt.io/qt-5/qcoreapplication.html#locale-settings and +# https://stackoverflow.com/a/34878283 for more details. + KNOWN_VIOLATIONS=( "src/bitcoin-tx.cpp.*stoul" "src/bitcoin-tx.cpp.*trim_right" diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 3d0ac7f995..3ba2b2a103 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -24,6 +24,7 @@ race:WalletBatch::WriteHDChain race:BerkeleyBatch race:BerkeleyDatabase race:DatabaseBatch +race:leveldb::DBImpl::DeleteObsoleteFiles race:zmq::* race:bitcoin-qt # deadlock (TODO fix) |