diff options
113 files changed, 2275 insertions, 686 deletions
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8262b9f90..5857753e14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,7 +103,7 @@ jobs: - name: Restore Ccache cache id: ccache-cache - uses: actions/cache/restore@v3 + uses: actions/cache/restore@v4 with: path: ${{ env.CCACHE_DIR }} key: ${{ github.job }}-ccache-${{ github.run_id }} @@ -113,7 +113,7 @@ jobs: run: ./ci/test_run_all.sh - name: Save Ccache cache - uses: actions/cache/save@v3 + uses: actions/cache/save@v4 if: github.event_name != 'pull_request' && steps.ccache-cache.outputs.cache-hit != 'true' with: path: ${{ env.CCACHE_DIR }} @@ -159,7 +159,7 @@ jobs: - name: Restore static Qt cache id: static-qt-cache - uses: actions/cache/restore@v3 + uses: actions/cache/restore@v4 with: path: C:\Qt_static key: ${{ github.job }}-static-qt-${{ hashFiles('msbuild_version', 'qt_url', 'qt_conf') }} @@ -202,14 +202,14 @@ jobs: - name: Save static Qt cache if: steps.static-qt-cache.outputs.cache-hit != 'true' - uses: actions/cache/save@v3 + uses: actions/cache/save@v4 with: path: C:\Qt_static key: ${{ github.job }}-static-qt-${{ hashFiles('msbuild_version', 'qt_url', 'qt_conf') }} - name: Ccache installation cache id: ccache-installation-cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: | C:\ProgramData\chocolatey\lib\ccache @@ -226,7 +226,7 @@ jobs: - name: Restore Ccache cache id: ccache-cache - uses: actions/cache/restore@v3 + uses: actions/cache/restore@v4 with: path: ~/AppData/Local/ccache key: ${{ github.job }}-ccache-${{ github.run_id }} @@ -242,13 +242,13 @@ jobs: Get-Content -Path "$env:GITHUB_WORKSPACE\vcpkg_commit" - name: vcpkg tools cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: C:/vcpkg/downloads/tools key: ${{ github.job }}-vcpkg-tools - name: vcpkg binary cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/AppData/Local/vcpkg/archives key: ${{ github.job }}-vcpkg-binary-${{ hashFiles('vcpkg_commit', 'msbuild_version', 'toolset_version', 'build_msvc/vcpkg.json') }} @@ -266,7 +266,7 @@ jobs: run: ccache --show-stats - name: Save Ccache cache - uses: actions/cache/save@v3 + uses: actions/cache/save@v4 if: github.event_name != 'pull_request' && steps.ccache-cache.outputs.cache-hit != 'true' with: path: ~/AppData/Local/ccache diff --git a/build_msvc/test_bitcoin/test_bitcoin.vcxproj b/build_msvc/test_bitcoin/test_bitcoin.vcxproj index 2a78f6f2a1..0ae3819e50 100644 --- a/build_msvc/test_bitcoin/test_bitcoin.vcxproj +++ b/build_msvc/test_bitcoin/test_bitcoin.vcxproj @@ -10,7 +10,7 @@ </PropertyGroup> <ItemGroup> <ClCompile Include="..\..\src\test\*_properties.cpp" /> - <ClCompile Include="..\..\src\test\*_tests.cpp" /> + <ClCompile Include="..\..\src\test\*_tests.cpp" Exclude="..\..\src\test\ipc_tests.cpp" /> <ClCompile Include="..\..\src\test\gen\*_gen.cpp" /> <ClCompile Include="..\..\src\test\main.cpp" /> <ClCompile Include="..\..\src\test\util\*.cpp" /> diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index b382567003..0b32049013 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -15,7 +15,7 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}" export CONTAINER_NAME="ci_native_fuzz_msan" export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future -export DEP_OPTS="NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" +export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export USE_MEMORY_SANITIZER="true" diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh index 1f60c46803..4f80d7ed42 100755 --- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh +++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh @@ -16,5 +16,5 @@ export RUN_FUZZ_TESTS=true export FUZZ_TESTS_CONFIG="--valgrind" export GOAL="install" # Temporarily pin dwarf 4, until using Valgrind 3.20 or later -export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC='clang -gdwarf-4' CXX='clang++ -gdwarf-4'" +export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang CXX=clang++ CFLAGS=-gdwarf-4 CXXFLAGS=-gdwarf-4" export CCACHE_MAXSIZE=200M diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index fd075800b0..60987f5011 100755 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -15,7 +15,7 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}" export CONTAINER_NAME="ci_native_msan" export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future -export DEP_OPTS="NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" +export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export USE_MEMORY_SANITIZER="true" diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index daa1a0cdb3..9bdb2b7860 100755 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -14,4 +14,4 @@ export NO_DEPENDS=1 export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export GOAL="install" # Temporarily pin dwarf 4, until using Valgrind 3.20 or later -export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC='clang -gdwarf-4' CXX='clang++ -gdwarf-4'" # TODO enable GUI +export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++ CFLAGS=-gdwarf-4 CXXFLAGS=-gdwarf-4" # TODO enable GUI diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index b15df4b6cc..99813da106 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -36,7 +36,7 @@ if [ -n "$PIP_PACKAGES" ]; then fi if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then - ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b llvmorg-17.0.2 /msan/llvm-project + ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b llvmorg-17.0.6 /msan/llvm-project cmake -G Ninja -B /msan/clang_build/ \ -DLLVM_ENABLE_PROJECTS="clang" \ diff --git a/configure.ac b/configure.ac index adc862d251..50e6870dd9 100644 --- a/configure.ac +++ b/configure.ac @@ -323,6 +323,15 @@ AC_ARG_ENABLE([external-signer], AC_LANG_PUSH([C++]) +dnl Always set -g -O2 in our CXXFLAGS. Autoconf will try and set CXXFLAGS to "-g -O2" by default, +dnl so we suppress that (if CXXFLAGS hasn't been overridden by the user), given we are adding it +dnl ourselves. +CORE_CXXFLAGS="$CORE_CXXFLAGS -g -O2" + +if test "$CXXFLAGS_overridden" = "no"; then + CXXFLAGS="" +fi + dnl Check for a flag to turn compiler warnings into errors. This is helpful for checks which may dnl appear to succeed because by default they merely emit warnings when they fail. dnl @@ -347,12 +356,6 @@ case $host in esac if test "$enable_debug" = "yes"; then - dnl If debugging is enabled, and the user hasn't overridden CXXFLAGS, clear - dnl them, to prevent autoconfs "-g -O2" being added. Otherwise we'd end up - dnl with "-O0 -g3 -g -O2". - if test "$CXXFLAGS_overridden" = "no"; then - CXXFLAGS="" - fi dnl Disable all optimizations AX_CHECK_COMPILE_FLAG([-O0], [DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -O0"], [], [$CXXFLAG_WERROR]) @@ -378,7 +381,8 @@ if test "$use_sanitizers" != ""; then dnl fail if a bad argument is passed, e.g. -fsanitize=undfeined AX_CHECK_COMPILE_FLAG( [-fsanitize=$use_sanitizers], - [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers"], + [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers" + SANITIZER_CFLAGS="-fsanitize=$use_sanitizers"], [AC_MSG_ERROR([compiler did not accept requested flags])]) dnl Some compilers (e.g. GCC) require additional libraries like libasan, @@ -859,13 +863,7 @@ if test "$use_lcov" = "yes"; then [AC_MSG_ERROR([lcov testing requested but --coverage linker flag does not work])]) AX_CHECK_COMPILE_FLAG([--coverage],[CORE_CXXFLAGS="$CORE_CXXFLAGS --coverage"], [AC_MSG_ERROR([lcov testing requested but --coverage flag does not work])]) - dnl If coverage is enabled, and the user hasn't overridden CXXFLAGS, clear - dnl them, to prevent autoconfs "-g -O2" being added. Otherwise we'd end up - dnl with "--coverage -Og -O0 -g -O2". - if test "$CXXFLAGS_overridden" = "no"; then - CXXFLAGS="" - fi - CORE_CXXFLAGS="$CORE_CXXFLAGS -Og -O0" + CORE_CXXFLAGS="$CORE_CXXFLAGS -Og" fi if test "$use_lcov_branch" != "no"; then @@ -1934,6 +1932,9 @@ CPPFLAGS_TEMP="$CPPFLAGS" unset CPPFLAGS CPPFLAGS="$CPPFLAGS_TEMP" +if test -n "$use_sanitizers"; then + export SECP_CFLAGS="$SECP_CFLAGS $SANITIZER_CFLAGS" +fi ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --disable-module-ecdh" AC_CONFIG_SUBDIRS([src/secp256k1]) @@ -1993,11 +1994,11 @@ echo " target os = $host_os" echo " build os = $build_os" echo echo " CC = $CC" -echo " CFLAGS = $PTHREAD_CFLAGS $CFLAGS" +echo " CFLAGS = $PTHREAD_CFLAGS $SANITIZER_CFLAGS $CFLAGS" echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS" echo " CXX = $CXX" -echo " CXXFLAGS = $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS" -echo " LDFLAGS = $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $LDFLAGS" +echo " CXXFLAGS = $CORE_CXXFLAGS $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $SANITIZER_CXXFLAGS $CXXFLAGS" +echo " LDFLAGS = $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $SANITIZER_LDFLAGS $CORE_LDFLAGS $LDFLAGS" echo " AR = $AR" echo " ARFLAGS = $ARFLAGS" echo diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md index 6db77cbbea..ad1ac4a64d 100644 --- a/contrib/seeds/README.md +++ b/contrib/seeds/README.md @@ -4,7 +4,7 @@ Utility to generate the seeds.txt list that is compiled into the client (see [src/chainparamsseeds.h](/src/chainparamsseeds.h) and other utilities in [contrib/seeds](/contrib/seeds)). Be sure to update `PATTERN_AGENT` in `makeseeds.py` to include the current version, -and remove old versions as necessary (at a minimum when GetDesirableServiceFlags +and remove old versions as necessary (at a minimum when SeedsServiceFlags() changes its default return value, as those are the services which seeds are added to addrman with). diff --git a/depends/builders/darwin.mk b/depends/builders/darwin.mk index eb64c97f64..a5f07643de 100644 --- a/depends/builders/darwin.mk +++ b/depends/builders/darwin.mk @@ -16,7 +16,6 @@ darwin_CXX:=$(shell xcrun -f clang++) -stdlib=libc++ -isysroot$(shell xcrun --sh darwin_AR:=$(shell xcrun -f ar) darwin_RANLIB:=$(shell xcrun -f ranlib) darwin_STRIP:=$(shell xcrun -f strip) -darwin_LIBTOOL:=$(shell xcrun -f libtool) darwin_OTOOL:=$(shell xcrun -f otool) darwin_NM:=$(shell xcrun -f nm) darwin_INSTALL_NAME_TOOL:=$(shell xcrun -f install_name_tool) diff --git a/depends/funcs.mk b/depends/funcs.mk index ce87aa579f..24c911b0f7 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -6,7 +6,6 @@ $(1)_objc=$$($$($(1)_type)_OBJC) $(1)_objcxx=$$($$($(1)_type)_OBJCXX) $(1)_ar=$$($$($(1)_type)_AR) $(1)_ranlib=$$($$($(1)_type)_RANLIB) -$(1)_libtool=$$($$($(1)_type)_LIBTOOL) $(1)_nm=$$($$($(1)_type)_NM) $(1)_cflags=$$($$($(1)_type)_CFLAGS) \ $$($$($(1)_type)_$$(release_type)_CFLAGS) diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index b94ac7d56f..29ad7ef252 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -39,7 +39,7 @@ llvm_config_prog=$(shell $(SHELL) $(.SHELLFLAGS) "command -v llvm-config") llvm_lib_dir=$(shell $(llvm_config_prog) --libdir) endif -cctools_TOOLS=AR RANLIB STRIP NM LIBTOOL OTOOL INSTALL_NAME_TOOL DSYMUTIL +cctools_TOOLS=AR RANLIB STRIP NM OTOOL INSTALL_NAME_TOOL DSYMUTIL # Make-only lowercase function lc = $(subst A,a,$(subst B,b,$(subst C,c,$(subst D,d,$(subst E,e,$(subst F,f,$(subst G,g,$(subst H,h,$(subst I,i,$(subst J,j,$(subst K,k,$(subst L,l,$(subst M,m,$(subst N,n,$(subst O,o,$(subst P,p,$(subst Q,q,$(subst R,r,$(subst S,s,$(subst T,t,$(subst U,u,$(subst V,v,$(subst W,w,$(subst X,x,$(subst Y,y,$(subst Z,z,$1)))))))))))))))))))))))))) diff --git a/depends/hosts/default.mk b/depends/hosts/default.mk index bad4568bcb..cf3c90441d 100644 --- a/depends/hosts/default.mk +++ b/depends/hosts/default.mk @@ -7,7 +7,6 @@ default_host_CXX = $(host_toolchain)g++ default_host_AR = $(host_toolchain)ar default_host_RANLIB = $(host_toolchain)ranlib default_host_STRIP = $(host_toolchain)strip -default_host_LIBTOOL = $(host_toolchain)libtool default_host_NM = $(host_toolchain)nm default_host_OBJCOPY = $(host_toolchain)objcopy @@ -39,5 +38,5 @@ host_$1 = $$($(host_arch)_$(host_os)_$1) host_$(release_type)_$1 = $$($(host_arch)_$(host_os)_$(release_type)_$1) endef -$(foreach tool,CC CXX AR RANLIB STRIP LIBTOOL NM OBJCOPY OTOOL INSTALL_NAME_TOOL DSYMUTIL,$(eval $(call add_host_tool_func,$(tool)))) +$(foreach tool,CC CXX AR RANLIB STRIP NM OBJCOPY OTOOL INSTALL_NAME_TOOL DSYMUTIL,$(eval $(call add_host_tool_func,$(tool)))) $(foreach flags,CFLAGS CXXFLAGS CPPFLAGS LDFLAGS, $(eval $(call add_host_flags_func,$(flags)))) diff --git a/depends/packages.md b/depends/packages.md index 4158b46d28..ad91eaffee 100644 --- a/depends/packages.md +++ b/depends/packages.md @@ -74,7 +74,6 @@ These variables may be set to override or append their default values. $(package)_objcxx $(package)_ar $(package)_ranlib - $(package)_libtool $(package)_nm $(package)_cflags $(package)_cxxflags diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk index 2eddc76d9c..caa809ec0b 100644 --- a/depends/packages/libnatpmp.mk +++ b/depends/packages/libnatpmp.mk @@ -3,14 +3,18 @@ $(package)_version=07004b97cf691774efebe70404cf22201e4d330d $(package)_download_path=https://github.com/miniupnp/libnatpmp/archive $(package)_file_name=$($(package)_version).tar.gz $(package)_sha256_hash=9321953ceb39d07c25463e266e50d0ae7b64676bb3a986d932b18881ed94f1fb +$(package)_patches=no_libtool.patch define $(package)_set_vars $(package)_build_opts=CC="$($(package)_cc)" $(package)_build_opts_mingw32=CPPFLAGS=-DNATPMP_STATICLIB - $(package)_build_opts_darwin=LIBTOOL="$($(package)_libtool)" $(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" endef +define $(package)_preprocess_cmds + patch -p1 < $($(package)_patch_dir)/no_libtool.patch +endef + define $(package)_build_cmds $(MAKE) libnatpmp.a $($(package)_build_opts) endef diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk index 7ad2529e47..5698a7cbb1 100644 --- a/depends/packages/miniupnpc.mk +++ b/depends/packages/miniupnpc.mk @@ -3,20 +3,20 @@ $(package)_version=2.2.2 $(package)_download_path=https://miniupnp.tuxfamily.org/files/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=888fb0976ba61518276fe1eda988589c700a3f2a69d71089260d75562afd3687 -$(package)_patches=dont_leak_info.patch respect_mingw_cflags.patch +$(package)_patches=dont_leak_info.patch respect_mingw_cflags.patch no_libtool.patch # Next time this package is updated, ensure that _WIN32_WINNT is still properly set. # See discussion in https://github.com/bitcoin/bitcoin/pull/25964. define $(package)_set_vars $(package)_build_opts=CC="$($(package)_cc)" -$(package)_build_opts_darwin=LIBTOOL="$($(package)_libtool)" $(package)_build_opts_mingw32=-f Makefile.mingw CFLAGS="$($(package)_cflags) -D_WIN32_WINNT=0x0601" $(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" endef define $(package)_preprocess_cmds patch -p1 < $($(package)_patch_dir)/dont_leak_info.patch && \ - patch -p1 < $($(package)_patch_dir)/respect_mingw_cflags.patch + patch -p1 < $($(package)_patch_dir)/respect_mingw_cflags.patch && \ + patch -p1 < $($(package)_patch_dir)/no_libtool.patch endef define $(package)_build_cmds diff --git a/depends/packages/native_libmultiprocess.mk b/depends/packages/native_libmultiprocess.mk index 946e885354..bcdb1f9e7c 100644 --- a/depends/packages/native_libmultiprocess.mk +++ b/depends/packages/native_libmultiprocess.mk @@ -1,8 +1,8 @@ package=native_libmultiprocess -$(package)_version=414542f81e0997354b45b8ade13ca144a3e35ff1 +$(package)_version=8da797c5f1644df1bffd84d10c1ae9836dc70d60 $(package)_download_path=https://github.com/chaincodelabs/libmultiprocess/archive $(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=8542dbaf8c4fce8fd7af6929f5dc9b34dffa51c43e9ee360e93ee0f34b180bc2 +$(package)_sha256_hash=030f4d393d2ac9deba98d2e1973e22fc439ffc009d5f8ae3225c90639f86beb0 $(package)_dependencies=native_capnp define $(package)_config_cmds diff --git a/depends/packages/sqlite.mk b/depends/packages/sqlite.mk index a8ec89c6c6..6809b39113 100644 --- a/depends/packages/sqlite.mk +++ b/depends/packages/sqlite.mk @@ -11,10 +11,12 @@ $(package)_config_opts_linux=--with-pic $(package)_config_opts_freebsd=--with-pic $(package)_config_opts_netbsd=--with-pic $(package)_config_opts_openbsd=--with-pic -$(package)_config_opts_debug=--enable-debug -$(package)_cflags+=-DSQLITE_DQS=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED -$(package)_cflags+=-DSQLITE_OMIT_SHARED_CACHE -DSQLITE_OMIT_JSON -DSQLITE_LIKE_DOESNT_MATCH_BLOBS -$(package)_cflags+=-DSQLITE_OMIT_DECLTYPE -DSQLITE_OMIT_PROGRESS_CALLBACK -DSQLITE_OMIT_AUTOINIT +# We avoid using `--enable-debug` because it overrides CFLAGS, a behavior we want to prevent. +$(package)_cflags_debug += -g +$(package)_cppflags_debug += -DSQLITE_DEBUG +$(package)_cppflags+=-DSQLITE_DQS=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED +$(package)_cppflags+=-DSQLITE_OMIT_SHARED_CACHE -DSQLITE_OMIT_JSON -DSQLITE_LIKE_DOESNT_MATCH_BLOBS +$(package)_cppflags+=-DSQLITE_OMIT_DECLTYPE -DSQLITE_OMIT_PROGRESS_CALLBACK -DSQLITE_OMIT_AUTOINIT endef define $(package)_preprocess_cmds diff --git a/depends/patches/libnatpmp/no_libtool.patch b/depends/patches/libnatpmp/no_libtool.patch new file mode 100644 index 0000000000..2b9f01f6eb --- /dev/null +++ b/depends/patches/libnatpmp/no_libtool.patch @@ -0,0 +1,15 @@ +diff -ruN libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile +--- libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile 2022-07-05 07:49:50.000000000 +0000 ++++ libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile 2024-01-23 20:59:35.674821779 +0000 +@@ -197,11 +197,7 @@ + $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS) + + $(STATICLIB): $(LIBOBJS) +-ifneq (, $(findstring darwin, $(OS))) +- $(LIBTOOL) -static -o $@ $? +-else + $(AR) crs $@ $? +-endif + + $(SHAREDLIB): $(LIBOBJS) + ifneq (, $(findstring darwin, $(OS))) diff --git a/depends/patches/miniupnpc/no_libtool.patch b/depends/patches/miniupnpc/no_libtool.patch new file mode 100644 index 0000000000..bb7d4a87ef --- /dev/null +++ b/depends/patches/miniupnpc/no_libtool.patch @@ -0,0 +1,15 @@ +diff -ruN miniupnpc-2.2.2/Makefile miniupnpc-2.2.2.new/Makefile +--- miniupnpc-2.2.2/Makefile 2020-11-27 18:25:02.000000000 +0000 ++++ miniupnpc-2.2.2.new/Makefile 2024-01-23 20:58:08.387188527 +0000 +@@ -298,11 +298,7 @@ + makedepend -Y -- $(CFLAGS) -- $(SRCS) 2>/dev/null + + $(LIBRARY): $(LIBOBJS) +-ifneq (, $(findstring darwin, $(OS))) +- $(LIBTOOL) -static -o $@ $? +-else + $(AR) crs $@ $? +-endif + + $(SHAREDLIBRARY): $(LIBOBJS) + ifneq (, $(findstring darwin, $(OS))) diff --git a/doc/release-notes-29189.md b/doc/release-notes-29189.md new file mode 100644 index 0000000000..f75e450217 --- /dev/null +++ b/doc/release-notes-29189.md @@ -0,0 +1,15 @@ +libbitcoinconsensus +======================== + +This library is deprecated and will be removed for v28. + +It has existed for nearly 10 years with very little known uptake or impact. It +has become a maintenance burden. + +The underlying functionality does not change between versions, so any users of +the library can continue to use the final release indefinitely, with the +understanding that Taproot is its final consensus update. + +In the future, libbitcoinkernel will provide a much more useful API that is +aware of the UTXO set, and therefore be able to fully validate transactions and +blocks. diff --git a/doc/shared-libraries.md b/doc/shared-libraries.md index 07aee46f0b..3a448c6556 100644 --- a/doc/shared-libraries.md +++ b/doc/shared-libraries.md @@ -2,6 +2,7 @@ Shared Libraries ================ ## bitcoinconsensus +***This library is deprecated and will be removed in v28*** The purpose of this library is to make the verification functionality that is critical to Bitcoin's consensus available to other applications, e.g. to language bindings. diff --git a/src/Makefile.am b/src/Makefile.am index 780f547b4b..e452b42533 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,7 +9,7 @@ print-%: FORCE DIST_SUBDIRS = secp256k1 AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS) -AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) +AM_CXXFLAGS = $(CORE_CXXFLAGS) $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) AM_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS) AM_LIBTOOLFLAGS = --preserve-dup-deps PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) @@ -50,6 +50,10 @@ LIBBITCOIN_WALLET_TOOL=libbitcoin_wallet_tool.a endif LIBBITCOIN_CRYPTO = $(LIBBITCOIN_CRYPTO_BASE) +if USE_ASM +LIBBITCOIN_CRYPTO_SSE4 = crypto/libbitcoin_crypto_sse4.la +LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE4) +endif if ENABLE_SSE41 LIBBITCOIN_CRYPTO_SSE41 = crypto/libbitcoin_crypto_sse41.la LIBBITCOIN_CRYPTO += $(LIBBITCOIN_CRYPTO_SSE41) @@ -541,6 +545,10 @@ libbitcoin_wallet_tool_a_SOURCES = \ # # crypto # + +# crypto_base contains the unspecialized (unoptimized) versions of our +# crypto functions. Functions that require custom compiler flags and/or +# runtime opt-in are omitted. crypto_libbitcoin_crypto_base_la_CPPFLAGS = $(AM_CPPFLAGS) # Specify -static in both CXXFLAGS and LDFLAGS so libtool will only build a @@ -581,9 +589,12 @@ crypto_libbitcoin_crypto_base_la_SOURCES = \ crypto/siphash.cpp \ crypto/siphash.h -if USE_ASM -crypto_libbitcoin_crypto_base_la_SOURCES += crypto/sha256_sse4.cpp -endif +# See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and +# CXXFLAGS above +crypto_libbitcoin_crypto_sse4_la_LDFLAGS = $(AM_LDFLAGS) -static +crypto_libbitcoin_crypto_sse4_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -static +crypto_libbitcoin_crypto_sse4_la_CPPFLAGS = $(AM_CPPFLAGS) +crypto_libbitcoin_crypto_sse4_la_SOURCES = crypto/sha256_sse4.cpp # See explanation for -static in crypto_libbitcoin_crypto_base_la's LDFLAGS and # CXXFLAGS above @@ -906,7 +917,7 @@ lib_LTLIBRARIES += $(LIBBITCOINKERNEL) libbitcoinkernel_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) $(PTHREAD_FLAGS) libbitcoinkernel_la_LIBADD = $(LIBBITCOIN_CRYPTO) $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) -libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) +libbitcoinkernel_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include $(BOOST_CPPFLAGS) $(LEVELDB_CPPFLAGS) # libbitcoinkernel requires default symbol visibility, explicitly specify that # here so that things still work even when user configures with @@ -1012,7 +1023,7 @@ libbitcoinconsensus_la_SOURCES = support/cleanse.cpp $(crypto_libbitcoin_crypto_ libbitcoinconsensus_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS) libbitcoinconsensus_la_LIBADD = $(LIBSECP256K1) -libbitcoinconsensus_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL +libbitcoinconsensus_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL -DDISABLE_OPTIMIZED_SHA256 libbitcoinconsensus_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) endif @@ -1090,6 +1101,7 @@ ipc/capnp/libbitcoin_ipc_a-protocol.$(OBJEXT): $(libbitcoin_ipc_mpgen_input:=.h) if BUILD_MULTIPROCESS LIBBITCOIN_IPC=libbitcoin_ipc.a libbitcoin_ipc_a_SOURCES = \ + ipc/capnp/common-types.h \ ipc/capnp/context.h \ ipc/capnp/init-types.h \ ipc/capnp/protocol.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 870e49bd75..69d44dbde6 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -117,6 +117,7 @@ BITCOIN_TESTS =\ test/net_tests.cpp \ test/netbase_tests.cpp \ test/orphanage_tests.cpp \ + test/peerman_tests.cpp \ test/pmt_tests.cpp \ test/policy_fee_tests.cpp \ test/policyestimator_tests.cpp \ @@ -216,6 +217,39 @@ BITCOIN_TEST_SUITE += \ wallet/test/init_test_fixture.h endif # ENABLE_WALLET +if BUILD_MULTIPROCESS +# Add boost ipc_tests definition to BITCOIN_TESTS +BITCOIN_TESTS += test/ipc_tests.cpp + +# Build ipc_test code in a separate library so it can be compiled with custom +# LIBMULTIPROCESS_CFLAGS without those flags affecting other tests +LIBBITCOIN_IPC_TEST=libbitcoin_ipc_test.a +EXTRA_LIBRARIES += $(LIBBITCOIN_IPC_TEST) +libbitcoin_ipc_test_a_SOURCES = \ + test/ipc_test.cpp \ + test/ipc_test.h +libbitcoin_ipc_test_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +libbitcoin_ipc_test_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) $(LIBMULTIPROCESS_CFLAGS) + +# Generate various .c++/.h files from the ipc_test.capnp file +include $(MPGEN_PREFIX)/include/mpgen.mk +EXTRA_DIST += test/ipc_test.capnp +libbitcoin_ipc_test_mpgen_output = \ + test/ipc_test.capnp.c++ \ + test/ipc_test.capnp.h \ + test/ipc_test.capnp.proxy-client.c++ \ + test/ipc_test.capnp.proxy-server.c++ \ + test/ipc_test.capnp.proxy-types.c++ \ + test/ipc_test.capnp.proxy-types.h \ + test/ipc_test.capnp.proxy.h +nodist_libbitcoin_ipc_test_a_SOURCES = $(libbitcoin_ipc_test_mpgen_output) +CLEANFILES += $(libbitcoin_ipc_test_mpgen_output) +endif + +# Explicitly list dependencies on generated headers as described in +# https://www.gnu.org/software/automake/manual/html_node/Built-Sources-Example.html#Recording-Dependencies-manually +test/libbitcoin_ipc_test_a-ipc_test.$(OBJEXT): test/ipc_test.capnp.h + test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) test_test_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(TESTDEFS) $(BOOST_CPPFLAGS) $(EVENT_CFLAGS) test_test_bitcoin_LDADD = $(LIBTEST_UTIL) @@ -223,6 +257,9 @@ if ENABLE_WALLET test_test_bitcoin_LDADD += $(LIBBITCOIN_WALLET) test_test_bitcoin_CPPFLAGS += $(BDB_CPPFLAGS) endif +if BUILD_MULTIPROCESS +test_test_bitcoin_LDADD += $(LIBBITCOIN_IPC_TEST) $(LIBMULTIPROCESS_LIBS) +endif test_test_bitcoin_LDADD += $(LIBBITCOIN_NODE) $(LIBBITCOIN_CLI) $(LIBBITCOIN_COMMON) $(LIBBITCOIN_UTIL) $(LIBBITCOIN_CONSENSUS) $(LIBBITCOIN_CRYPTO) $(LIBUNIVALUE) \ $(LIBLEVELDB) $(LIBMEMENV) $(LIBSECP256K1) $(EVENT_LIBS) $(EVENT_PTHREADS_LIBS) $(MINISKETCH_LIBS) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index ee8b0a44c5..c1a71ed749 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -117,7 +117,6 @@ int main(int argc, char* argv[]) const ChainstateManager::Options chainman_opts{ .chainparams = *chainparams, .datadir = abs_datadir, - .adjusted_time_callback = NodeClock::now, .notifications = *notifications, }; const node::BlockManager::Options blockman_opts{ diff --git a/src/chain.h b/src/chain.h index f9121fb861..fa165a4aa7 100644 --- a/src/chain.h +++ b/src/chain.h @@ -11,15 +11,20 @@ #include <flatfile.h> #include <kernel/cs_main.h> #include <primitives/block.h> +#include <serialize.h> #include <sync.h> #include <uint256.h> #include <util/time.h> +#include <algorithm> +#include <cassert> +#include <cstdint> +#include <string> #include <vector> /** * Maximum amount of time that a block timestamp is allowed to exceed the - * current network-adjusted time before the block will be accepted. + * current time before the block will be accepted. */ static constexpr int64_t MAX_FUTURE_BLOCK_TIME = 2 * 60 * 60; diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 5761e8b321..db1001111a 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -4,6 +4,10 @@ #include <common/settings.h> +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif + #include <tinyformat.h> #include <univalue.h> #include <util/fs.h> @@ -27,6 +31,9 @@ enum class Source { CONFIG_FILE_DEFAULT_SECTION }; +// Json object key for the auto-generated warning comment +const std::string SETTINGS_WARN_MSG_KEY{"_warning_"}; + //! Merge settings from multiple sources in precedence order: //! Forced config > command line > read-write settings file > config file network-specific section > config file default section //! @@ -81,7 +88,9 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va SettingsValue in; if (!in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) { - errors.emplace_back(strprintf("Unable to parse settings file %s", fs::PathToString(path))); + errors.emplace_back(strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, " + "and can be fixed by removing the file, which will reset settings to default values.", + fs::PathToString(path))); return false; } @@ -106,6 +115,10 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va break; } } + + // Remove auto-generated warning comment from the accessible settings. + values.erase(SETTINGS_WARN_MSG_KEY); + return errors.empty(); } @@ -114,6 +127,10 @@ bool WriteSettings(const fs::path& path, std::vector<std::string>& errors) { SettingsValue out(SettingsValue::VOBJ); + // Add auto-generated warning comment + out.pushKV(SETTINGS_WARN_MSG_KEY, strprintf("This file is automatically generated and updated by %s. Please do not edit this file while the node " + "is running, as any changes might be ignored or overwritten.", PACKAGE_NAME)); + // Push settings values for (const auto& value : values) { out.pushKVEnd(value.first, value.second); } diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 5eacaa44e1..11aabeb1da 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -8,14 +8,15 @@ #include <assert.h> #include <string.h> +#if !defined(DISABLE_OPTIMIZED_SHA256) #include <compat/cpuid.h> -#if defined(__linux__) && defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(__linux__) && defined(ENABLE_ARM_SHANI) #include <sys/auxv.h> #include <asm/hwcap.h> #endif -#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) #include <sys/types.h> #include <sys/sysctl.h> #endif @@ -58,6 +59,7 @@ namespace sha256d64_arm_shani { void Transform_2way(unsigned char* out, const unsigned char* in); } +#endif // DISABLE_OPTIMIZED_SHA256 // Internal implementation code. namespace @@ -567,6 +569,7 @@ bool SelfTest() { return true; } +#if !defined(DISABLE_OPTIMIZED_SHA256) #if defined(USE_ASM) && (defined(__x86_64__) || defined(__amd64__) || defined(__i386__)) /** Check whether the OS has enabled AVX registers. */ bool AVXEnabled() @@ -576,6 +579,7 @@ bool AVXEnabled() return (a & 6) == 6; } #endif +#endif // DISABLE_OPTIMIZED_SHA256 } // namespace @@ -588,6 +592,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem TransformD64_4way = nullptr; TransformD64_8way = nullptr; +#if !defined(DISABLE_OPTIMIZED_SHA256) #if defined(USE_ASM) && defined(HAVE_GETCPUID) bool have_sse4 = false; bool have_xsave = false; @@ -616,7 +621,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem } } -#if defined(ENABLE_X86_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_X86_SHANI) if (have_x86_shani) { Transform = sha256_x86_shani::Transform; TransformD64 = TransformD64Wrapper<sha256_x86_shani::Transform>; @@ -633,13 +638,13 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem TransformD64 = TransformD64Wrapper<sha256_sse4::Transform>; ret = "sse4(1way)"; #endif -#if defined(ENABLE_SSE41) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_SSE41) TransformD64_4way = sha256d64_sse41::Transform_4way; ret += ",sse41(4way)"; #endif } -#if defined(ENABLE_AVX2) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_AVX2) if (have_avx2 && have_avx && enabled_avx) { TransformD64_8way = sha256d64_avx2::Transform_8way; ret += ",avx2(8way)"; @@ -647,7 +652,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem #endif #endif // defined(USE_ASM) && defined(HAVE_GETCPUID) -#if defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(ENABLE_ARM_SHANI) bool have_arm_shani = false; if (use_implementation & sha256_implementation::USE_SHANI) { #if defined(__linux__) @@ -679,6 +684,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem ret = "arm_shani(1way,2way)"; } #endif +#endif // DISABLE_OPTIMIZED_SHA256 assert(SelfTest()); return ret; diff --git a/src/headerssync.cpp b/src/headerssync.cpp index 234fc8da60..e14de004f5 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -5,8 +5,8 @@ #include <headerssync.h> #include <logging.h> #include <pow.h> -#include <timedata.h> #include <util/check.h> +#include <util/time.h> #include <util/vector.h> // The two constants below are computed using the simulation script in @@ -41,7 +41,7 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus // exceeds this bound, because it's not possible for a consensus-valid // chain to be longer than this (at the current time -- in the future we // could try again, if necessary, to sync a longer chain). - m_max_commitments = 6*(Ticks<std::chrono::seconds>(GetAdjustedTime() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}}) + MAX_FUTURE_BLOCK_TIME) / HEADER_COMMITMENT_PERIOD; + m_max_commitments = 6*(Ticks<std::chrono::seconds>(NodeClock::now() - NodeSeconds{std::chrono::seconds{chain_start->GetMedianTimePast()}}) + MAX_FUTURE_BLOCK_TIME) / HEADER_COMMITMENT_PERIOD; LogPrint(BCLog::NET, "Initial headers sync started with peer=%d: height=%i, max_commitments=%i, min_work=%s\n", m_id, m_current_height, m_max_commitments, m_minimum_required_work.ToString()); } diff --git a/src/init.cpp b/src/init.cpp index b825c8ce21..988daefeec 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1448,7 +1448,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ChainstateManager::Options chainman_opts{ .chainparams = chainparams, .datadir = args.GetDataDirNet(), - .adjusted_time_callback = GetAdjustedTime, .notifications = *node.notifications, }; Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction @@ -1743,13 +1742,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 12: start node //// debug print + int64_t best_block_time{}; { LOCK(cs_main); LogPrintf("block tree size = %u\n", chainman.BlockIndex().size()); chain_active_height = chainman.ActiveChain().Height(); + best_block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); if (tip_info) { tip_info->block_height = chain_active_height; - tip_info->block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : chainman.GetParams().GenesisBlock().GetBlockTime(); + tip_info->block_time = best_block_time; tip_info->verification_progress = GuessVerificationProgress(chainman.GetParams().TxData(), chainman.ActiveChain().Tip()); } if (tip_info && chainman.m_best_header) { @@ -1758,7 +1759,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } LogPrintf("nBestHeight = %d\n", chain_active_height); - if (node.peerman) node.peerman->SetBestHeight(chain_active_height); + if (node.peerman) node.peerman->SetBestBlock(chain_active_height, std::chrono::seconds{best_block_time}); // Map ports with UPnP or NAT-PMP. StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); diff --git a/src/ipc/capnp/common-types.h b/src/ipc/capnp/common-types.h new file mode 100644 index 0000000000..39e368491b --- /dev/null +++ b/src/ipc/capnp/common-types.h @@ -0,0 +1,108 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_IPC_CAPNP_COMMON_TYPES_H +#define BITCOIN_IPC_CAPNP_COMMON_TYPES_H + +#include <clientversion.h> +#include <streams.h> +#include <univalue.h> + +#include <cstddef> +#include <mp/proxy-types.h> +#include <type_traits> +#include <utility> + +namespace ipc { +namespace capnp { +//! Use SFINAE to define Serializeable<T> trait which is true if type T has a +//! Serialize(stream) method, false otherwise. +template <typename T> +struct Serializable { +private: + template <typename C> + static std::true_type test(decltype(std::declval<C>().Serialize(std::declval<std::nullptr_t&>()))*); + template <typename> + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test<T>(nullptr))::value; +}; + +//! Use SFINAE to define Unserializeable<T> trait which is true if type T has +//! an Unserialize(stream) method, false otherwise. +template <typename T> +struct Unserializable { +private: + template <typename C> + static std::true_type test(decltype(std::declval<C>().Unserialize(std::declval<std::nullptr_t&>()))*); + template <typename> + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test<T>(nullptr))::value; +}; +} // namespace capnp +} // namespace ipc + +//! Functions to serialize / deserialize common bitcoin types. +namespace mp { +//! Overload multiprocess library's CustomBuildField hook to allow any +//! serializable object to be stored in a capnproto Data field or passed to a +//! canproto interface. Use Priority<1> so this hook has medium priority, and +//! higher priority hooks could take precedence over this one. +template <typename LocalType, typename Value, typename Output> +void CustomBuildField( + TypeList<LocalType>, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output, + // Enable if serializeable and if LocalType is not cv or reference + // qualified. If LocalType is cv or reference qualified, it is important to + // fall back to lower-priority Priority<0> implementation of this function + // that strips cv references, to prevent this CustomBuildField overload from + // taking precedence over more narrow overloads for specific LocalTypes. + std::enable_if_t<ipc::capnp::Serializable<LocalType>::value && + std::is_same_v<LocalType, std::remove_cv_t<std::remove_reference_t<LocalType>>>>* enable = nullptr) +{ + DataStream stream; + value.Serialize(stream); + auto result = output.init(stream.size()); + memcpy(result.begin(), stream.data(), stream.size()); +} + +//! Overload multiprocess library's CustomReadField hook to allow any object +//! with an Unserialize method to be read from a capnproto Data field or +//! returned from canproto interface. Use Priority<1> so this hook has medium +//! priority, and higher priority hooks could take precedence over this one. +template <typename LocalType, typename Input, typename ReadDest> +decltype(auto) +CustomReadField(TypeList<LocalType>, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest, + std::enable_if_t<ipc::capnp::Unserializable<LocalType>::value>* enable = nullptr) +{ + return read_dest.update([&](auto& value) { + if (!input.has()) return; + auto data = input.get(); + SpanReader stream({data.begin(), data.end()}); + value.Unserialize(stream); + }); +} + +template <typename Value, typename Output> +void CustomBuildField(TypeList<UniValue>, Priority<1>, InvokeContext& invoke_context, Value&& value, Output&& output) +{ + std::string str = value.write(); + auto result = output.init(str.size()); + memcpy(result.begin(), str.data(), str.size()); +} + +template <typename Input, typename ReadDest> +decltype(auto) CustomReadField(TypeList<UniValue>, Priority<1>, InvokeContext& invoke_context, Input&& input, + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + auto data = input.get(); + value.read(std::string_view{data.begin(), data.size()}); + }); +} +} // namespace mp + +#endif // BITCOIN_IPC_CAPNP_COMMON_TYPES_H diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 56cb3a63a0..b0e657ba45 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -501,9 +501,9 @@ public: { // For use by test/functional/feature_assumeutxo.py .height = 299, - .hash_serialized = AssumeutxoHash{uint256S("0x61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53")}, - .nChainTx = 300, - .blockhash = uint256S("0x7e0517ef3ea6ecbed9117858e42eedc8eb39e8698a38dcbd1b3962a283233f4c") + .hash_serialized = AssumeutxoHash{uint256S("0xa4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")}, + .nChainTx = 334, + .blockhash = uint256S("0x3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0") }, }; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index ee20eabd79..864aac336e 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -32,7 +32,6 @@ namespace kernel { struct ChainstateManagerOpts { const CChainParams& chainparams; fs::path datadir; - const std::function<NodeClock::time_point()> adjusted_time_callback{nullptr}; std::optional<bool> check_block_index{}; bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED}; //! If set, it will override the minimum work we will assume exists on some valid chain. diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index cae4b04bf6..57c5168e9f 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -205,7 +205,7 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock } auto last = SteadyClock::now(); - LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", + LogPrintf("Dumped mempool: %.3fs to copy, %.3fs to dump\n", Ticks<SecondsDouble>(mid - start), Ticks<SecondsDouble>(last - mid)); } catch (const std::exception& e) { diff --git a/src/net.cpp b/src/net.cpp index 0cc794c2c3..7c82f01d75 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -203,7 +203,7 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn) while (!s.eof()) { CService endpoint; s >> endpoint; - CAddress addr{endpoint, GetDesirableServiceFlags(NODE_NONE)}; + CAddress addr{endpoint, SeedsServiceFlags()}; addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - one_week, -one_week); LogPrint(BCLog::NET, "Added hardcoded seed: %s\n", addr.ToStringAddrPort()); vSeedsOut.push_back(addr); @@ -1827,7 +1827,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock, RandAddEvent((uint32_t)id); } -bool CConnman::AddConnection(const std::string& address, ConnectionType conn_type) +bool CConnman::AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport = false) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); std::optional<int> max_connections; @@ -1860,7 +1860,7 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ CSemaphoreGrant grant(*semOutbound, true); if (!grant) return false; - OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/false); + OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/use_v2transport); return true; } @@ -2267,7 +2267,7 @@ void CConnman::ThreadDNSAddressSeed() AddAddrFetch(seed); } else { std::vector<CAddress> vAdd; - ServiceFlags requiredServiceBits = GetDesirableServiceFlags(NODE_NONE); + constexpr ServiceFlags requiredServiceBits{SeedsServiceFlags()}; std::string host = strprintf("x%x.%s", requiredServiceBits, seed); CNetAddr resolveSource; if (!resolveSource.SetInternal(host)) { @@ -2638,7 +2638,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) const CAddress addr = m_anchors.back(); m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !g_reachable_nets.Contains(addr) || - !HasAllDesirableServiceFlags(addr.nServices) || + !m_msgproc->HasAllDesirableServiceFlags(addr.nServices) || outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToStringAddrPort()); @@ -2704,7 +2704,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // for non-feelers, require all the services we'll want, // for feelers, only require they be a full node (only because most // SPV clients don't have a good address DB available) - if (!fFeeler && !HasAllDesirableServiceFlags(addr.nServices)) { + if (!fFeeler && !m_msgproc->HasAllDesirableServiceFlags(addr.nServices)) { continue; } else if (fFeeler && !MayHaveUsefulAddressDB(addr.nServices)) { continue; @@ -97,7 +97,7 @@ static constexpr bool DEFAULT_FIXEDSEEDS{true}; static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000; static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000; -static constexpr bool DEFAULT_V2_TRANSPORT{false}; +static constexpr bool DEFAULT_V2_TRANSPORT{true}; typedef int64_t NodeId; @@ -1006,6 +1006,12 @@ public: virtual void FinalizeNode(const CNode& node) = 0; /** + * Callback to determine whether the given set of service flags are sufficient + * for a peer to be "relevant". + */ + virtual bool HasAllDesirableServiceFlags(ServiceFlags services) const = 0; + + /** * Process protocol messages received from a given node * * @param[in] pnode The node which we have received messages from. @@ -1189,13 +1195,14 @@ public: * @param[in] address Address of node to try connecting to * @param[in] conn_type ConnectionType::OUTBOUND, ConnectionType::BLOCK_RELAY, * ConnectionType::ADDR_FETCH or ConnectionType::FEELER + * @param[in] use_v2transport Set to true if node attempts to connect using BIP 324 v2 transport protocol. * @return bool Returns false if there are no available * slots for this connection: * - conn_type not a supported ConnectionType * - Max total outbound connection capacity filled * - Max connection capacity for type is filled */ - bool AddConnection(const std::string& address, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); + bool AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); size_t GetNodeCount(ConnectionDirection) const; uint32_t GetMappedAS(const CNetAddr& addr) const; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 80cf610a0d..c8da927763 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -41,12 +41,12 @@ #include <txrequest.h> #include <util/check.h> #include <util/strencodings.h> +#include <util/time.h> #include <util/trace.h> #include <validation.h> #include <algorithm> #include <atomic> -#include <chrono> #include <future> #include <memory> #include <optional> @@ -133,6 +133,8 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; static const int MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10; /** Minimum blocks required to signal NODE_NETWORK_LIMITED */ static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288; +/** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ +static const unsigned int NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; /** Average delay between local address broadcasts */ static constexpr auto AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24h}; /** Average delay between peer address broadcasts */ @@ -499,6 +501,7 @@ public: /** Implement NetEventsInterface */ void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); + bool HasAllDesirableServiceFlags(ServiceFlags services) const override; bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); bool SendMessages(CNode* pto) override @@ -513,12 +516,17 @@ public: bool IgnoresIncomingTxs() override { return m_opts.ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - void SetBestHeight(int height) override { m_best_height = height; }; + void SetBestBlock(int height, std::chrono::seconds time) override + { + m_best_height = height; + m_best_block_time = time; + }; void UnitTestMisbehaving(NodeId peer_id, int howmuch) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), howmuch, ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; + ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const override; private: /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ @@ -721,6 +729,8 @@ private: /** The height of the best chain */ std::atomic<int> m_best_height{-1}; + /** The time of the best chain tip block */ + std::atomic<std::chrono::seconds> m_best_block_time{0s}; /** Next time to check for stale tip */ std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s}; @@ -993,6 +1003,12 @@ private: bool CanDirectFetch() EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** + * Estimates the distance, in blocks, between the best-known block and the network chain tip. + * Utilizes the best-block time and the chainparams blocks spacing to approximate it. + */ + int64_t ApproximateBestBlockDepth() const; + + /** * To prevent fingerprinting attacks, only send blocks/headers outside of * the active chain if they are no more than a month older (both in time, * and in best equivalent proof of work) than the best header chain we know @@ -1311,9 +1327,14 @@ bool PeerManagerImpl::TipMayBeStale() return m_last_tip_update.load() < GetTime<std::chrono::seconds>() - std::chrono::seconds{consensusParams.nPowTargetSpacing * 3} && mapBlocksInFlight.empty(); } +int64_t PeerManagerImpl::ApproximateBestBlockDepth() const +{ + return (GetTime<std::chrono::seconds>() - m_best_block_time.load()).count() / m_chainparams.GetConsensus().nPowTargetSpacing; +} + bool PeerManagerImpl::CanDirectFetch() { - return m_chainman.ActiveChain().Tip()->Time() > GetAdjustedTime() - m_chainparams.GetConsensus().PowTargetSpacing() * 20; + return m_chainman.ActiveChain().Tip()->Time() > NodeClock::now() - m_chainparams.GetConsensus().PowTargetSpacing() * 20; } static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -1651,6 +1672,23 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } +bool PeerManagerImpl::HasAllDesirableServiceFlags(ServiceFlags services) const +{ + // Shortcut for (services & GetDesirableServiceFlags(services)) == GetDesirableServiceFlags(services) + return !(GetDesirableServiceFlags(services) & (~services)); +} + +ServiceFlags PeerManagerImpl::GetDesirableServiceFlags(ServiceFlags services) const +{ + if (services & NODE_NETWORK_LIMITED) { + // Limited peers are desirable when we are close to the tip. + if (ApproximateBestBlockDepth() < NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS) { + return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); + } + } + return ServiceFlags(NODE_NETWORK | NODE_WITNESS); +} + PeerRef PeerManagerImpl::GetPeerRef(NodeId id) const { LOCK(m_peer_mutex); @@ -2047,8 +2085,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha */ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { - SetBestHeight(pindexNew->nHeight); - SetServiceFlagsIBDCache(!fInitialDownload); + SetBestBlock(pindexNew->nHeight, std::chrono::seconds{pindexNew->GetBlockTime()}); // Don't relay inventory during initial block download. if (fInitialDownload) return; @@ -5555,7 +5592,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!state.fSyncStarted && CanServeBlocks(*peer) && !m_chainman.m_blockman.LoadingBlocks()) { // Only actively request headers from a single peer, unless we're close to today. - if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > GetAdjustedTime() - 24h) { + if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > NodeClock::now() - 24h) { const CBlockIndex* pindexStart = m_chainman.m_best_header; /* If possible, start at the block preceding the currently best known header. This ensures that we always get a @@ -5575,7 +5612,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling // to maintain precision std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} * - Ticks<std::chrono::seconds>(GetAdjustedTime() - m_chainman.m_best_header->Time()) / consensusParams.nPowTargetSpacing + Ticks<std::chrono::seconds>(NodeClock::now() - m_chainman.m_best_header->Time()) / consensusParams.nPowTargetSpacing ); nSyncStarted++; } @@ -5879,7 +5916,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Check for headers sync timeouts if (state.fSyncStarted && peer->m_headers_sync_timeout < std::chrono::microseconds::max()) { // Detect whether this is a stalling initial-headers-sync peer - if (m_chainman.m_best_header->Time() <= GetAdjustedTime() - 24h) { + if (m_chainman.m_best_header->Time() <= NodeClock::now() - 24h) { if (current_time > peer->m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) { // Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer, // and we have others we could be using instead. diff --git a/src/net_processing.h b/src/net_processing.h index afdef00067..f8d7a8f511 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -92,8 +92,8 @@ public: /** Send ping message to all peers */ virtual void SendPings() = 0; - /** Set the best height */ - virtual void SetBestHeight(int height) = 0; + /** Set the height of the best block and its time (seconds since epoch). */ + virtual void SetBestBlock(int height, std::chrono::seconds time) = 0; /* Public for unit testing. */ virtual void UnitTestMisbehaving(NodeId peer_id, int howmuch) = 0; @@ -110,6 +110,29 @@ public: /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; + + /** + * Gets the set of service flags which are "desirable" for a given peer. + * + * These are the flags which are required for a peer to support for them + * to be "interesting" to us, ie for us to wish to use one of our few + * outbound connection slots for or for us to wish to prioritize keeping + * their connection around. + * + * Relevant service flags may be peer- and state-specific in that the + * version of the peer may determine which flags are required (eg in the + * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers + * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which + * case NODE_NETWORK_LIMITED suffices). + * + * Thus, generally, avoid calling with 'services' == NODE_NONE, unless + * state-specific flags must absolutely be avoided. When called with + * 'services' == NODE_NONE, the returned desirable service flags are + * guaranteed to not change dependent on state - ie they are suitable for + * use when describing peers which we know to be desirable, but for which + * we do not have a confirmed set of service flags. + */ + virtual ServiceFlags GetDesirableServiceFlags(ServiceFlags services) const = 0; }; #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/netaddress.h b/src/netaddress.h index 08dd77c0ff..0bbde43dd7 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -261,6 +261,18 @@ public: } } + /** + * BIP155 network ids recognized by this software. + */ + enum BIP155Network : uint8_t { + IPV4 = 1, + IPV6 = 2, + TORV2 = 3, + TORV3 = 4, + I2P = 5, + CJDNS = 6, + }; + friend class CSubNet; private: @@ -283,18 +295,6 @@ private: bool SetI2P(const std::string& addr); /** - * BIP155 network ids recognized by this software. - */ - enum BIP155Network : uint8_t { - IPV4 = 1, - IPV6 = 2, - TORV2 = 3, - TORV3 = 4, - I2P = 5, - CJDNS = 6, - }; - - /** * Size of CNetAddr when serialized as ADDRv1 (pre-BIP155) (in bytes). */ static constexpr size_t V1_SERIALIZATION_SIZE = ADDR_IPV6_SIZE; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index e6164c2e59..c499bbfa6a 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -307,6 +307,7 @@ void BlockManager::FindFilesToPrune( // Distribute our -prune budget over all chainstates. const auto target = std::max( MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / chainman.GetAll().size()); + const uint64_t target_sync_height = chainman.m_best_header->nHeight; if (chain.m_chain.Height() < 0 || target == 0) { return; @@ -329,10 +330,13 @@ void BlockManager::FindFilesToPrune( // On a prune event, the chainstate DB is flushed. // To avoid excessive prune events negating the benefit of high dbcache // values, we should not prune too rapidly. - // So when pruning in IBD, increase the buffer a bit to avoid a re-prune too soon. - if (chainman.IsInitialBlockDownload()) { - // Since this is only relevant during IBD, we use a fixed 10% - nBuffer += target / 10; + // So when pruning in IBD, increase the buffer to avoid a re-prune too soon. + const auto chain_tip_height = chain.m_chain.Height(); + if (chainman.IsInitialBlockDownload() && target_sync_height > (uint64_t)chain_tip_height) { + // Since this is only relevant during IBD, we assume blocks are at least 1 MB on average + static constexpr uint64_t average_block_size = 1000000; /* 1 MB */ + const uint64_t remaining_blocks = target_sync_height - chain_tip_height; + nBuffer += average_block_size * remaining_blocks; } for (int fileNumber = 0; fileNumber < this->MaxBlockfileNum(); fileNumber++) { diff --git a/src/node/miner.cpp b/src/node/miner.cpp index ce5452d1f9..87f40e993f 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -20,8 +20,8 @@ #include <policy/policy.h> #include <pow.h> #include <primitives/transaction.h> -#include <timedata.h> #include <util/moneystr.h> +#include <util/time.h> #include <validation.h> #include <algorithm> @@ -31,7 +31,7 @@ namespace node { int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) { int64_t nOldTime = pblock->nTime; - int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime()))}; + int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()))}; if (nOldTime < nNewTime) { pblock->nTime = nNewTime; @@ -133,7 +133,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc pblock->nVersion = gArgs.GetIntArg("-blockversion", pblock->nVersion); } - pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime()); + pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()); m_lock_time_cutoff = pindexPrev->GetMedianTimePast(); int nPackagesSelected = 0; @@ -171,7 +171,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc BlockValidationState state; if (m_options.test_block_validity && !TestBlockValidity(state, chainparams, m_chainstate, *pblock, pindexPrev, - GetAdjustedTime, /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) { + /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) { throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString())); } const auto time_2{SteadyClock::now()}; diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index e8ab2326c1..ef9a30a076 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -32,7 +32,7 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback) { - // BroadcastTransaction can be called by either sendrawtransaction RPC or the wallet. + // BroadcastTransaction can be called by RPC or by the wallet. // chainman, mempool and peerman are initialized before the RPC server and wallet are started // and reset after the RPC sever and wallet are stopped. assert(node.chainman); diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 5440548636..5f1d15c5f2 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -1055,7 +1055,7 @@ void CBlockPolicyEstimator::FlushUnconfirmed() _removeTx(mi->first, false); // this calls erase() on mapMemPoolTxs } const auto endclear{SteadyClock::now()}; - LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear)); + LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %.3fs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear)); } std::chrono::hours CBlockPolicyEstimator::GetFeeEstimatorFileAge() diff --git a/src/prevector.h b/src/prevector.h index bcab1ff00c..4776db789b 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -47,26 +47,28 @@ public: typedef const value_type* const_pointer; class iterator { - T* ptr; + T* ptr{}; public: typedef Diff difference_type; typedef T value_type; typedef T* pointer; typedef T& reference; - typedef std::random_access_iterator_tag iterator_category; + using element_type = T; + using iterator_category = std::contiguous_iterator_tag; + iterator() = default; iterator(T* ptr_) : ptr(ptr_) {} T& operator*() const { return *ptr; } T* operator->() const { return ptr; } - T& operator[](size_type pos) { return ptr[pos]; } - const T& operator[](size_type pos) const { return ptr[pos]; } + T& operator[](size_type pos) const { return ptr[pos]; } iterator& operator++() { ptr++; return *this; } iterator& operator--() { ptr--; return *this; } iterator operator++(int) { iterator copy(*this); ++(*this); return copy; } iterator operator--(int) { iterator copy(*this); --(*this); return copy; } difference_type friend operator-(iterator a, iterator b) { return (&(*a) - &(*b)); } - iterator operator+(size_type n) { return iterator(ptr + n); } + iterator operator+(size_type n) const { return iterator(ptr + n); } + iterator friend operator+(size_type n, iterator x) { return x + n; } iterator& operator+=(size_type n) { ptr += n; return *this; } - iterator operator-(size_type n) { return iterator(ptr - n); } + iterator operator-(size_type n) const { return iterator(ptr - n); } iterator& operator-=(size_type n) { ptr -= n; return *this; } bool operator==(iterator x) const { return ptr == x.ptr; } bool operator!=(iterator x) const { return ptr != x.ptr; } @@ -77,18 +79,17 @@ public: }; class reverse_iterator { - T* ptr; + T* ptr{}; public: typedef Diff difference_type; typedef T value_type; typedef T* pointer; typedef T& reference; typedef std::bidirectional_iterator_tag iterator_category; + reverse_iterator() = default; reverse_iterator(T* ptr_) : ptr(ptr_) {} - T& operator*() { return *ptr; } - const T& operator*() const { return *ptr; } - T* operator->() { return ptr; } - const T* operator->() const { return ptr; } + T& operator*() const { return *ptr; } + T* operator->() const { return ptr; } reverse_iterator& operator--() { ptr++; return *this; } reverse_iterator& operator++() { ptr--; return *this; } reverse_iterator operator++(int) { reverse_iterator copy(*this); ++(*this); return copy; } @@ -98,13 +99,15 @@ public: }; class const_iterator { - const T* ptr; + const T* ptr{}; public: typedef Diff difference_type; typedef const T value_type; typedef const T* pointer; typedef const T& reference; - typedef std::random_access_iterator_tag iterator_category; + using element_type = const T; + using iterator_category = std::contiguous_iterator_tag; + const_iterator() = default; const_iterator(const T* ptr_) : ptr(ptr_) {} const_iterator(iterator x) : ptr(&(*x)) {} const T& operator*() const { return *ptr; } @@ -115,9 +118,10 @@ public: const_iterator operator++(int) { const_iterator copy(*this); ++(*this); return copy; } const_iterator operator--(int) { const_iterator copy(*this); --(*this); return copy; } difference_type friend operator-(const_iterator a, const_iterator b) { return (&(*a) - &(*b)); } - const_iterator operator+(size_type n) { return const_iterator(ptr + n); } + const_iterator operator+(size_type n) const { return const_iterator(ptr + n); } + const_iterator friend operator+(size_type n, const_iterator x) { return x + n; } const_iterator& operator+=(size_type n) { ptr += n; return *this; } - const_iterator operator-(size_type n) { return const_iterator(ptr - n); } + const_iterator operator-(size_type n) const { return const_iterator(ptr - n); } const_iterator& operator-=(size_type n) { ptr -= n; return *this; } bool operator==(const_iterator x) const { return ptr == x.ptr; } bool operator!=(const_iterator x) const { return ptr != x.ptr; } @@ -128,13 +132,14 @@ public: }; class const_reverse_iterator { - const T* ptr; + const T* ptr{}; public: typedef Diff difference_type; typedef const T value_type; typedef const T* pointer; typedef const T& reference; typedef std::bidirectional_iterator_tag iterator_category; + const_reverse_iterator() = default; const_reverse_iterator(const T* ptr_) : ptr(ptr_) {} const_reverse_iterator(reverse_iterator x) : ptr(&(*x)) {} const T& operator*() const { return *ptr; } diff --git a/src/protocol.cpp b/src/protocol.cpp index 27a0a2ffc1..0da160768d 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -9,8 +9,6 @@ #include <atomic> -static std::atomic<bool> g_initial_block_download_completed(false); - namespace NetMsgType { const char* VERSION = "version"; const char* VERACK = "verack"; @@ -125,18 +123,6 @@ bool CMessageHeader::IsCommandValid() const return true; } - -ServiceFlags GetDesirableServiceFlags(ServiceFlags services) { - if ((services & NODE_NETWORK_LIMITED) && g_initial_block_download_completed) { - return ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS); - } - return ServiceFlags(NODE_NETWORK | NODE_WITNESS); -} - -void SetServiceFlagsIBDCache(bool state) { - g_initial_block_download_completed = state; -} - CInv::CInv() { type = 0; diff --git a/src/protocol.h b/src/protocol.h index e405253632..243cd23e6e 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -311,43 +311,12 @@ enum ServiceFlags : uint64_t { std::vector<std::string> serviceFlagsToStr(uint64_t flags); /** - * Gets the set of service flags which are "desirable" for a given peer. - * - * These are the flags which are required for a peer to support for them - * to be "interesting" to us, ie for us to wish to use one of our few - * outbound connection slots for or for us to wish to prioritize keeping - * their connection around. - * - * Relevant service flags may be peer- and state-specific in that the - * version of the peer may determine which flags are required (eg in the - * case of NODE_NETWORK_LIMITED where we seek out NODE_NETWORK peers - * unless they set NODE_NETWORK_LIMITED and we are out of IBD, in which - * case NODE_NETWORK_LIMITED suffices). - * - * Thus, generally, avoid calling with peerServices == NODE_NONE, unless - * state-specific flags must absolutely be avoided. When called with - * peerServices == NODE_NONE, the returned desirable service flags are - * guaranteed to not change dependent on state - ie they are suitable for - * use when describing peers which we know to be desirable, but for which - * we do not have a confirmed set of service flags. - * - * If the NODE_NONE return value is changed, contrib/seeds/makeseeds.py - * should be updated appropriately to filter for the same nodes. - */ -ServiceFlags GetDesirableServiceFlags(ServiceFlags services); - -/** Set the current IBD status in order to figure out the desirable service flags */ -void SetServiceFlagsIBDCache(bool status); - -/** - * A shortcut for (services & GetDesirableServiceFlags(services)) - * == GetDesirableServiceFlags(services), ie determines whether the given - * set of service flags are sufficient for a peer to be "relevant". - */ -static inline bool HasAllDesirableServiceFlags(ServiceFlags services) -{ - return !(GetDesirableServiceFlags(services) & (~services)); -} + * State independent service flags. + * If the return value is changed, contrib/seeds/makeseeds.py + * should be updated appropriately to filter for nodes with + * desired service flags (compatible with our new flags). + */ +constexpr ServiceFlags SeedsServiceFlags() { return ServiceFlags(NODE_NETWORK | NODE_WITNESS); } /** * Checks if a peer with the given service flags may be capable of having a diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index e5a5179d87..7100603616 100644 --- a/src/qt/test/optiontests.cpp +++ b/src/qt/test/optiontests.cpp @@ -61,7 +61,11 @@ void OptionTests::migrateSettings() QVERIFY(!settings.contains("addrSeparateProxyTor")); std::ifstream file(gArgs.GetDataDirNet() / "settings.json"); + std::string default_warning = strprintf("This file is automatically generated and updated by %s. Please do not edit this file while the node " + "is running, as any changes might be ignored or overwritten.", + PACKAGE_NAME); QCOMPARE(std::string(std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()).c_str(), "{\n" + " \"_warning_\": \""+ default_warning+"\",\n" " \"dbcache\": \"600\",\n" " \"listen\": false,\n" " \"onion\": \"onion:234\",\n" diff --git a/src/qt/winshutdownmonitor.h b/src/qt/winshutdownmonitor.h index 78f287637f..060d8546e3 100644 --- a/src/qt/winshutdownmonitor.h +++ b/src/qt/winshutdownmonitor.h @@ -10,7 +10,7 @@ #include <QString> #include <functional> -#include <windef.h> // for HWND +#include <windows.h> #include <QAbstractNativeEventFilter> diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index b0771a75fc..5825efdf82 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -303,6 +303,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendmsgtopeer", 0, "peer_id" }, { "stop", 0, "wait" }, { "addnode", 2, "v2transport" }, + { "addconnection", 2, "v2transport" }, }; // clang-format on diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 49b51b83ae..f1abfb6396 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -27,11 +27,11 @@ #include <script/descriptor.h> #include <script/script.h> #include <script/signingprovider.h> -#include <timedata.h> #include <txmempool.h> #include <univalue.h> #include <util/strencodings.h> #include <util/string.h> +#include <util/time.h> #include <util/translation.h> #include <validation.h> #include <validationinterface.h> @@ -383,7 +383,7 @@ static RPCHelpMan generateblock() LOCK(cs_main); BlockValidationState state; - if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), GetAdjustedTime, false, false)) { + if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString())); } } @@ -697,7 +697,7 @@ static RPCHelpMan getblocktemplate() if (block.hashPrevBlock != pindexPrev->GetBlockHash()) return "inconclusive-not-best-prevblk"; BlockValidationState state; - TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, GetAdjustedTime, false, true); + TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, false, true); return BIP22ValidationResult(state); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e53f889897..5e6f42b596 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -371,6 +371,7 @@ static RPCHelpMan addconnection() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."}, {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."}, + {"v2transport", RPCArg::Type::BOOL, RPCArg::Optional::NO, "Attempt to connect using BIP324 v2 transport protocol"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -379,8 +380,8 @@ static RPCHelpMan addconnection() { RPCResult::Type::STR, "connection_type", "Type of connection opened." }, }}, RPCExamples{ - HelpExampleCli("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\"") - + HelpExampleRpc("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\"") + HelpExampleCli("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\" true") + + HelpExampleRpc("addconnection", "\"192.168.0.6:8333\" \"outbound-full-relay\" true") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -402,11 +403,16 @@ static RPCHelpMan addconnection() } else { throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString()); } + bool use_v2transport = self.Arg<bool>(2); NodeContext& node = EnsureAnyNodeContext(request.context); CConnman& connman = EnsureConnman(node); - const bool success = connman.AddConnection(address, conn_type); + if (use_v2transport && !(connman.GetLocalServices() & NODE_P2P_V2)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Adding v2transport connections requires -v2transport init flag to be set."); + } + + const bool success = connman.AddConnection(address, conn_type, use_v2transport); if (!success) { throw JSONRPCError(RPC_CLIENT_NODE_CAPACITY_REACHED, "Error: Already at capacity for specified connection type."); } diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index c471986a44..a9e11622a7 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -70,7 +70,7 @@ void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, std::optio } } -void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) +UniValue NormalizeOutputs(const UniValue& outputs_in) { if (outputs_in.isNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null"); @@ -94,11 +94,15 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) } outputs = std::move(outputs_dict); } + return outputs; +} +std::vector<std::pair<CTxDestination, CAmount>> ParseOutputs(const UniValue& outputs) +{ // Duplicate checking std::set<CTxDestination> destinations; + std::vector<std::pair<CTxDestination, CAmount>> parsed_outputs; bool has_data{false}; - for (const std::string& name_ : outputs.getKeys()) { if (name_ == "data") { if (has_data) { @@ -106,11 +110,12 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) } has_data = true; std::vector<unsigned char> data = ParseHexV(outputs[name_].getValStr(), "Data"); - - CTxOut out(0, CScript() << OP_RETURN << data); - rawTx.vout.push_back(out); + CTxDestination destination{CNoDestination{CScript() << OP_RETURN << data}}; + CAmount amount{0}; + parsed_outputs.emplace_back(destination, amount); } else { - CTxDestination destination = DecodeDestination(name_); + CTxDestination destination{DecodeDestination(name_)}; + CAmount amount{AmountFromValue(outputs[name_])}; if (!IsValidDestination(destination)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + name_); } @@ -118,13 +123,23 @@ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) if (!destinations.insert(destination).second) { throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + name_); } + parsed_outputs.emplace_back(destination, amount); + } + } + return parsed_outputs; +} + +void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in) +{ + UniValue outputs(UniValue::VOBJ); + outputs = NormalizeOutputs(outputs_in); - CScript scriptPubKey = GetScriptForDestination(destination); - CAmount nAmount = AmountFromValue(outputs[name_]); + std::vector<std::pair<CTxDestination, CAmount>> parsed_outputs = ParseOutputs(outputs); + for (const auto& [destination, nAmount] : parsed_outputs) { + CScript scriptPubKey = GetScriptForDestination(destination); - CTxOut out(nAmount, scriptPubKey); - rawTx.vout.push_back(out); - } + CTxOut out(nAmount, scriptPubKey); + rawTx.vout.push_back(out); } } diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index a863432b7a..964d0b095b 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_RPC_RAWTRANSACTION_UTIL_H #define BITCOIN_RPC_RAWTRANSACTION_UTIL_H +#include <addresstype.h> +#include <consensus/amount.h> #include <map> #include <string> #include <optional> @@ -38,11 +40,16 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const */ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map<COutPoint, Coin>& coins); - /** Normalize univalue-represented inputs and add them to the transaction */ void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, bool rbf); -/** Normalize univalue-represented outputs and add them to the transaction */ +/** Normalize univalue-represented outputs */ +UniValue NormalizeOutputs(const UniValue& outputs_in); + +/** Parse normalized outputs into destination, amount tuples */ +std::vector<std::pair<CTxDestination, CAmount>> ParseOutputs(const UniValue& outputs); + +/** Normalize, parse, and add outputs to the transaction */ void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in); /** Create a transaction from univalue parameters */ diff --git a/src/test/.gitignore b/src/test/.gitignore new file mode 100644 index 0000000000..036df1430c --- /dev/null +++ b/src/test/.gitignore @@ -0,0 +1,2 @@ +# capnp generated files +*.capnp.* diff --git a/src/test/data/tx_valid.json b/src/test/data/tx_valid.json index b874f6f26c..70df0d0f69 100644 --- a/src/test/data/tx_valid.json +++ b/src/test/data/tx_valid.json @@ -319,6 +319,10 @@ [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "HASH160 0x14 0x7c17aff532f22beb54069942f9bf567a66133eaf EQUAL"]], "0200000001000100000000000000000000000000000000000000000000000000000000000000000000030251b2010000000100000000000000000000000000", "NONE"], +["Valid CHECKSEQUENCEVERIFY even with negative tx version number"], +[[["0000000000000000000000000000000000000000000000000000000000000100", 0, "HASH160 0x14 0x7c17aff532f22beb54069942f9bf567a66133eaf EQUAL"]], +"ffffffff01000100000000000000000000000000000000000000000000000000000000000000000000030251b2010000000100000000000000000000000000", "NONE"], + ["Valid P2WPKH (Private key of segwit tests is L5AQtV2HDm4xGsseLokK2VAT2EtYKcTm3c7HwqnJBFt9LdaQULsM)"], [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "0x00 0x14 0x4c9c3dfac4207d5d8cb89df5722cb3d712385e3f", 1000]], "0100000000010100010000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000001976a9144c9c3dfac4207d5d8cb89df5722cb3d712385e3f88ac02483045022100cfb07164b36ba64c1b1e8c7720a56ad64d96f6ef332d3d37f9cb3c96477dc44502200a464cd7a9cf94cd70f66ce4f4f0625ef650052c7afcfe29d7d7e01830ff91ed012103596d3451025c19dbbdeb932d6bf8bfb4ad499b95b6f88db8899efac102e5fc7100000000", "NONE"], diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index ece396aadf..8a54cc656d 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -64,26 +64,13 @@ FUZZ_TARGET(data_stream_addr_man, .init = initialize_addrman) CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& fast_random_context) { CNetAddr addr; - if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { - addr = ConsumeNetAddr(fuzzed_data_provider); - } else { - // The networks [1..6] correspond to CNetAddr::BIP155Network (private). - static const std::map<uint8_t, uint8_t> net_len_map = {{1, ADDR_IPV4_SIZE}, - {2, ADDR_IPV6_SIZE}, - {4, ADDR_TORV3_SIZE}, - {5, ADDR_I2P_SIZE}, - {6, ADDR_CJDNS_SIZE}}; - uint8_t net = fast_random_context.randrange(5) + 1; // [1..5] - if (net == 3) { - net = 6; + assert(!addr.IsValid()); + for (size_t i = 0; i < 8 && !addr.IsValid(); ++i) { + if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(fuzzed_data_provider); + } else { + addr = ConsumeNetAddr(fuzzed_data_provider, &fast_random_context); } - - DataStream s{}; - - s << net; - s << fast_random_context.randbytes(net_len_map.at(net)); - - s >> CAddress::V2_NETWORK(addr); } // Return a dummy IPv4 5.5.5.5 if we generated an invalid address. diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index 4a040c56de..b26151f63c 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -70,11 +70,13 @@ FUZZ_TARGET(banman, .init = initialize_banman) fuzzed_data_provider, [&] { CNetAddr net_addr{ConsumeNetAddr(fuzzed_data_provider)}; - const std::optional<CNetAddr>& addr{LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false)}; - if (addr.has_value() && addr->IsValid()) { - net_addr = *addr; - } else { - contains_invalid = true; + if (!net_addr.IsCJDNS() || !net_addr.IsValid()) { + const std::optional<CNetAddr>& addr{LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false)}; + if (addr.has_value() && addr->IsValid()) { + net_addr = *addr; + } else { + contains_invalid = true; + } } ban_man.Ban(net_addr, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); }, diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 9b97958a5d..2577f9e97a 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -213,7 +213,6 @@ FUZZ_TARGET(integer, .init = initialize_integer) { const ServiceFlags service_flags = (ServiceFlags)u64; - (void)HasAllDesirableServiceFlags(service_flags); (void)MayHaveUsefulAddressDB(service_flags); } diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index 5141d3362d..4803cdccad 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -26,6 +26,12 @@ FUZZ_TARGET(netaddress) if (net_addr.GetNetwork() == Network::NET_ONION) { assert(net_addr.IsTor()); } + if (net_addr.GetNetwork() == Network::NET_I2P) { + assert(net_addr.IsI2P()); + } + if (net_addr.GetNetwork() == Network::NET_CJDNS) { + assert(net_addr.IsCJDNS()); + } if (net_addr.GetNetwork() == Network::NET_INTERNAL) { assert(net_addr.IsInternal()); } @@ -69,6 +75,12 @@ FUZZ_TARGET(netaddress) if (net_addr.IsTor()) { assert(net_addr.GetNetwork() == Network::NET_ONION); } + if (net_addr.IsI2P()) { + assert(net_addr.GetNetwork() == Network::NET_I2P); + } + if (net_addr.IsCJDNS()) { + assert(net_addr.GetNetwork() == Network::NET_CJDNS); + } (void)net_addr.IsValid(); (void)net_addr.ToStringAddr(); diff --git a/src/test/fuzz/util/net.cpp b/src/test/fuzz/util/net.cpp index eb0f14ede0..99151bb84d 100644 --- a/src/test/fuzz/util/net.cpp +++ b/src/test/fuzz/util/net.cpp @@ -25,33 +25,63 @@ class CNode; -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext* rand) noexcept { - const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); - CNetAddr net_addr; - if (network == Network::NET_IPV4) { - in_addr v4_addr = {}; - v4_addr.s_addr = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); - net_addr = CNetAddr{v4_addr}; - } else if (network == Network::NET_IPV6) { - if (fuzzed_data_provider.remaining_bytes() >= 16) { - in6_addr v6_addr = {}; - auto addr_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(16); - if (addr_bytes[0] == CJDNS_PREFIX) { // Avoid generating IPv6 addresses that look like CJDNS. - addr_bytes[0] = 0x55; // Just an arbitrary number, anything != CJDNS_PREFIX would do. - } - memcpy(v6_addr.s6_addr, addr_bytes.data(), 16); - net_addr = CNetAddr{v6_addr, fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; + struct NetAux { + Network net; + CNetAddr::BIP155Network bip155; + size_t len; + }; + + static constexpr std::array<NetAux, 6> nets{ + NetAux{.net = Network::NET_IPV4, .bip155 = CNetAddr::BIP155Network::IPV4, .len = ADDR_IPV4_SIZE}, + NetAux{.net = Network::NET_IPV6, .bip155 = CNetAddr::BIP155Network::IPV6, .len = ADDR_IPV6_SIZE}, + NetAux{.net = Network::NET_ONION, .bip155 = CNetAddr::BIP155Network::TORV3, .len = ADDR_TORV3_SIZE}, + NetAux{.net = Network::NET_I2P, .bip155 = CNetAddr::BIP155Network::I2P, .len = ADDR_I2P_SIZE}, + NetAux{.net = Network::NET_CJDNS, .bip155 = CNetAddr::BIP155Network::CJDNS, .len = ADDR_CJDNS_SIZE}, + NetAux{.net = Network::NET_INTERNAL, .bip155 = CNetAddr::BIP155Network{0}, .len = 0}, + }; + + const size_t nets_index{rand == nullptr + ? fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, nets.size() - 1) + : static_cast<size_t>(rand->randrange(nets.size()))}; + + const auto& aux = nets[nets_index]; + + CNetAddr addr; + + if (aux.net == Network::NET_INTERNAL) { + if (rand == nullptr) { + addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); + } else { + const auto v = rand->randbytes(32); + addr.SetInternal(std::string{v.begin(), v.end()}); } - } else if (network == Network::NET_INTERNAL) { - net_addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); - } else if (network == Network::NET_ONION) { - auto pub_key{fuzzed_data_provider.ConsumeBytes<uint8_t>(ADDR_TORV3_SIZE)}; - pub_key.resize(ADDR_TORV3_SIZE); - const bool ok{net_addr.SetSpecial(OnionToString(pub_key))}; - assert(ok); + return addr; + } + + DataStream s; + + s << static_cast<uint8_t>(aux.bip155); + + std::vector<uint8_t> addr_bytes; + if (rand == nullptr) { + addr_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(aux.len); + addr_bytes.resize(aux.len); + } else { + addr_bytes = rand->randbytes(aux.len); } - return net_addr; + if (aux.net == NET_IPV6 && addr_bytes[0] == CJDNS_PREFIX) { // Avoid generating IPv6 addresses that look like CJDNS. + addr_bytes[0] = 0x55; // Just an arbitrary number, anything != CJDNS_PREFIX would do. + } + if (aux.net == NET_CJDNS) { // Avoid generating CJDNS addresses that don't start with CJDNS_PREFIX because those are !IsValid(). + addr_bytes[0] = CJDNS_PREFIX; + } + s << addr_bytes; + + s >> CAddress::V2_NETWORK(addr); + + return addr; } CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h index 47e4a2fac0..a6c9e23f2e 100644 --- a/src/test/fuzz/util/net.h +++ b/src/test/fuzz/util/net.h @@ -24,7 +24,15 @@ #include <optional> #include <string> -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept; +/** + * Create a CNetAddr. It may have `addr.IsValid() == false`. + * @param[in,out] fuzzed_data_provider Take data for the address from this, if `rand` is `nullptr`. + * @param[in,out] rand If not nullptr, take data from it instead of from `fuzzed_data_provider`. + * Prefer generating addresses using `fuzzed_data_provider` because it is not uniform. Only use + * `rand` if `fuzzed_data_provider` is exhausted or its data is needed for other things. + * @return a "random" network address. + */ +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext* rand = nullptr) noexcept; class FuzzedSock : public Sock { diff --git a/src/test/ipc_test.capnp b/src/test/ipc_test.capnp new file mode 100644 index 0000000000..55a3dc2683 --- /dev/null +++ b/src/test/ipc_test.capnp @@ -0,0 +1,18 @@ +# Copyright (c) 2023 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +@0xd71b0fc8727fdf83; + +using Cxx = import "/capnp/c++.capnp"; +$Cxx.namespace("gen"); + +using Proxy = import "/mp/proxy.capnp"; +$Proxy.include("test/ipc_test.h"); +$Proxy.includeTypes("ipc/capnp/common-types.h"); + +interface FooInterface $Proxy.wrap("FooImplementation") { + add @0 (a :Int32, b :Int32) -> (result :Int32); + passOutPoint @1 (arg :Data) -> (result :Data); + passUniValue @2 (arg :Text) -> (result :Text); +} diff --git a/src/test/ipc_test.cpp b/src/test/ipc_test.cpp new file mode 100644 index 0000000000..ce4edaceb0 --- /dev/null +++ b/src/test/ipc_test.cpp @@ -0,0 +1,67 @@ +// Copyright (c) 2023 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 <logging.h> +#include <mp/proxy-types.h> +#include <test/ipc_test.capnp.h> +#include <test/ipc_test.capnp.proxy.h> +#include <test/ipc_test.h> + +#include <future> +#include <kj/common.h> +#include <kj/memory.h> +#include <kj/test.h> + +#include <boost/test/unit_test.hpp> + +//! Unit test that tests execution of IPC calls without actually creating a +//! separate process. This test is primarily intended to verify behavior of type +//! conversion code that converts C++ objects to Cap'n Proto messages and vice +//! versa. +//! +//! The test creates a thread which creates a FooImplementation object (defined +//! in ipc_test.h) and a two-way pipe accepting IPC requests which call methods +//! on the object through FooInterface (defined in ipc_test.capnp). +void IpcTest() +{ + // Setup: create FooImplemention object and listen for FooInterface requests + std::promise<std::unique_ptr<mp::ProxyClient<gen::FooInterface>>> foo_promise; + std::function<void()> disconnect_client; + std::thread thread([&]() { + mp::EventLoop loop("IpcTest", [](bool raise, const std::string& log) { LogPrintf("LOG%i: %s\n", raise, log); }); + auto pipe = loop.m_io_context.provider->newTwoWayPipe(); + + auto connection_client = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[0])); + auto foo_client = std::make_unique<mp::ProxyClient<gen::FooInterface>>( + connection_client->m_rpc_system.bootstrap(mp::ServerVatId().vat_id).castAs<gen::FooInterface>(), + connection_client.get(), /* destroy_connection= */ false); + foo_promise.set_value(std::move(foo_client)); + disconnect_client = [&] { loop.sync([&] { connection_client.reset(); }); }; + + auto connection_server = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) { + auto foo_server = kj::heap<mp::ProxyServer<gen::FooInterface>>(std::make_shared<FooImplementation>(), connection); + return capnp::Capability::Client(kj::mv(foo_server)); + }); + connection_server->onDisconnect([&] { connection_server.reset(); }); + loop.loop(); + }); + std::unique_ptr<mp::ProxyClient<gen::FooInterface>> foo{foo_promise.get_future().get()}; + + // Test: make sure arguments were sent and return value is received + BOOST_CHECK_EQUAL(foo->add(1, 2), 3); + + COutPoint txout1{Txid::FromUint256(uint256{100}), 200}; + COutPoint txout2{foo->passOutPoint(txout1)}; + BOOST_CHECK(txout1 == txout2); + + UniValue uni1{UniValue::VOBJ}; + uni1.pushKV("i", 1); + uni1.pushKV("s", "two"); + UniValue uni2{foo->passUniValue(uni1)}; + BOOST_CHECK_EQUAL(uni1.write(), uni2.write()); + + // Test cleanup: disconnect pipe and join thread + disconnect_client(); + thread.join(); +} diff --git a/src/test/ipc_test.h b/src/test/ipc_test.h new file mode 100644 index 0000000000..bcfcc2125c --- /dev/null +++ b/src/test/ipc_test.h @@ -0,0 +1,21 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_IPC_TEST_H +#define BITCOIN_TEST_IPC_TEST_H + +#include <primitives/transaction.h> +#include <univalue.h> + +class FooImplementation +{ +public: + int add(int a, int b) { return a + b; } + COutPoint passOutPoint(COutPoint o) { return o; } + UniValue passUniValue(UniValue v) { return v; } +}; + +void IpcTest(); + +#endif // BITCOIN_TEST_IPC_TEST_H diff --git a/src/test/ipc_tests.cpp b/src/test/ipc_tests.cpp new file mode 100644 index 0000000000..6e144b0f41 --- /dev/null +++ b/src/test/ipc_tests.cpp @@ -0,0 +1,13 @@ +// Copyright (c) 2023 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 <test/ipc_test.h> +#include <boost/test/unit_test.hpp> + +BOOST_AUTO_TEST_SUITE(ipc_tests) +BOOST_AUTO_TEST_CASE(ipc_tests) +{ + IpcTest(); +} +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 342d2514ed..d50af4c175 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -12,7 +12,6 @@ #include <policy/policy.h> #include <test/util/random.h> #include <test/util/txmempool.h> -#include <timedata.h> #include <txmempool.h> #include <uint256.h> #include <util/strencodings.h> diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp new file mode 100644 index 0000000000..2c79329385 --- /dev/null +++ b/src/test/peerman_tests.cpp @@ -0,0 +1,76 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include <chainparams.h> +#include <node/miner.h> +#include <net_processing.h> +#include <pow.h> +#include <test/util/setup_common.h> +#include <validation.h> + +#include <boost/test/unit_test.hpp> + +BOOST_FIXTURE_TEST_SUITE(peerman_tests, RegTestingSetup) + +/** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ +static constexpr int64_t NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; + +static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_time) +{ + auto curr_time = GetTime<std::chrono::seconds>(); + SetMockTime(block_time); // update time so the block is created with it + CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr}.CreateNewBlock(CScript() << OP_TRUE)->block; + while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; + block.fChecked = true; // little speedup + SetMockTime(curr_time); // process block at current time + Assert(node.chainman->ProcessNewBlock(std::make_shared<const CBlock>(block), /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)); + SyncWithValidationInterfaceQueue(); // drain events queue +} + +// Verifying when network-limited peer connections are desirable based on the node's proximity to the tip +BOOST_AUTO_TEST_CASE(connections_desirable_service_flags) +{ + std::unique_ptr<PeerManager> peerman = PeerManager::make(*m_node.connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); + auto consensus = m_node.chainman->GetParams().GetConsensus(); + + // Check we start connecting to full nodes + ServiceFlags peer_flags{NODE_WITNESS | NODE_NETWORK_LIMITED}; + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // Make peerman aware of the initial best block and verify we accept limited peers when we start close to the tip time. + auto tip = WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Tip()); + uint64_t tip_block_time = tip->GetBlockTime(); + int tip_block_height = tip->nHeight; + peerman->SetBestBlock(tip_block_height, std::chrono::seconds{tip_block_time}); + + SetMockTime(tip_block_time + 1); // Set node time to tip time + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Check we don't disallow limited peers connections when we are behind but still recoverable (below the connection safety window) + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * (NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS - 1)}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Check we disallow limited peers connections when we are further than the limited peers safety window + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * 2}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // By now, we tested that the connections desirable services flags change based on the node's time proximity to the tip. + // Now, perform the same tests for when the node receives a block. + RegisterValidationInterface(peerman.get()); + + // First, verify a block in the past doesn't enable limited peers connections + // At this point, our time is (NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS + 1) * 10 minutes ahead the tip's time. + mineBlock(m_node, /*block_time=*/std::chrono::seconds{tip_block_time + 1}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); + + // Verify a block close to the tip enables limited peers connections + mineBlock(m_node, /*block_time=*/GetTime<std::chrono::seconds>()); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK_LIMITED | NODE_WITNESS)); + + // Lastly, verify the stale tip checks can disallow limited peers connections after not receiving blocks for a prolonged period. + SetMockTime(GetTime<std::chrono::seconds>() + std::chrono::seconds{consensus.nPowTargetSpacing * NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS + 1}); + BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS)); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index fa8ceb8dd6..41190b3579 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -99,7 +99,9 @@ BOOST_AUTO_TEST_CASE(ReadWrite) // Check invalid json not allowed WriteText(path, R"(invalid json)"); BOOST_CHECK(!common::ReadSettings(path, values, errors)); - std::vector<std::string> fail_parse = {strprintf("Unable to parse settings file %s", fs::PathToString(path))}; + std::vector<std::string> fail_parse = {strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, " + "and can be fixed by removing the file, which will reset settings to default values.", + fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end()); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 8789e86196..9f587d7ec0 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -44,7 +44,6 @@ #include <test/util/net.h> #include <test/util/random.h> #include <test/util/txmempool.h> -#include <timedata.h> #include <txdb.h> #include <txmempool.h> #include <util/chaintype.h> @@ -184,7 +183,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, .datadir = m_args.GetDataDirNet(), - .adjusted_time_callback = GetAdjustedTime, .check_block_index = true, .notifications = *m_node.notifications, .worker_threads_num = 2, diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 368ba8bee4..a33e71d50e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -15,7 +15,6 @@ #include <test/util/random.h> #include <test/util/setup_common.h> #include <test/util/validation.h> -#include <timedata.h> #include <uint256.h> #include <validation.h> #include <validationinterface.h> @@ -383,7 +382,6 @@ struct SnapshotTestSetup : TestChain100Setup { const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, - .adjusted_time_callback = GetAdjustedTime, .notifications = *m_node.notifications, }; const BlockManager::Options blockman_opts{ diff --git a/src/timedata.cpp b/src/timedata.cpp index 15ca90ee6a..d948de976f 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -33,11 +33,6 @@ int64_t GetTimeOffset() return nTimeOffset; } -NodeClock::time_point GetAdjustedTime() -{ - return NodeClock::now() + std::chrono::seconds{GetTimeOffset()}; -} - #define BITCOIN_TIMEDATA_MAX_SAMPLES 200 static std::set<CNetAddr> g_sources; diff --git a/src/timedata.h b/src/timedata.h index c6c36d9a39..3e76f80452 100644 --- a/src/timedata.h +++ b/src/timedata.h @@ -5,11 +5,8 @@ #ifndef BITCOIN_TIMEDATA_H #define BITCOIN_TIMEDATA_H -#include <util/time.h> - #include <algorithm> #include <cassert> -#include <chrono> #include <cstdint> #include <vector> @@ -75,11 +72,10 @@ public: /** Functions to keep track of adjusted P2P time */ int64_t GetTimeOffset(); -NodeClock::time_point GetAdjustedTime(); void AddTimeData(const CNetAddr& ip, int64_t nTime); /** - * Reset the internal state of GetTimeOffset(), GetAdjustedTime() and AddTimeData(). + * Reset the internal state of GetTimeOffset() and AddTimeData(). */ void TestOnlyResetTimeData(); diff --git a/src/validation.cpp b/src/validation.cpp index 8c583c586c..caa4ff3115 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2188,7 +2188,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Also, currently the rule against blocks more than 2 hours in the future // is enforced in ContextualCheckBlockHeader(); we wouldn't want to // re-enforce that rule here (at least until we make it impossible for - // m_adjusted_time_callback() to go backward). + // the clock to go backward). if (!CheckBlock(block, state, params.GetConsensus(), !fJustCheck, !fJustCheck)) { if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) { // We don't write down blocks to disk if they may have been @@ -3777,7 +3777,7 @@ arith_uint256 CalculateHeadersWork(const std::vector<CBlockHeader>& headers) * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev, NodeClock::time_point now) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); assert(pindexPrev != nullptr); @@ -3805,7 +3805,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", "block's timestamp is too early"); // Check timestamp - if (block.Time() > now + std::chrono::seconds{MAX_FUTURE_BLOCK_TIME}) { + if (block.Time() > NodeClock::now() + std::chrono::seconds{MAX_FUTURE_BLOCK_TIME}) { return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", "block timestamp too far in the future"); } @@ -3945,7 +3945,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida LogPrint(BCLog::VALIDATION, "header %s has prev block invalid: %s\n", hash.ToString(), block.hashPrevBlock.ToString()); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } - if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev, m_options.adjusted_time_callback())) { + if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev)) { LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -4230,7 +4230,6 @@ bool TestBlockValidity(BlockValidationState& state, Chainstate& chainstate, const CBlock& block, CBlockIndex* pindexPrev, - const std::function<NodeClock::time_point()>& adjusted_time_callback, bool fCheckPOW, bool fCheckMerkleRoot) { @@ -4244,7 +4243,7 @@ bool TestBlockValidity(BlockValidationState& state, indexDummy.phashBlock = &block_hash; // NOTE: CheckBlockHeader is called by CheckBlock - if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev, adjusted_time_callback())) + if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev)) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString()); if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); @@ -4862,16 +4861,6 @@ void ChainstateManager::CheckBlockIndex() CBlockIndex* pindexFirstAssumeValid = nullptr; // Oldest ancestor of pindex which has BLOCK_ASSUMED_VALID while (pindex != nullptr) { nNodes++; - // Make sure nChainTx sum is correctly computed. - unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; - assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) - // For testing, allow transaction counts to be completely unset. - || (pindex->nChainTx == 0 && pindex->nTx == 0) - // For testing, allow this nChainTx to be unset if previous is also unset. - || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) - // Transaction counts prior to snapshot are unknown. - || pindex->IsAssumedValid()); - if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) { @@ -4954,6 +4943,15 @@ void ChainstateManager::CheckBlockIndex() // Checks for not-invalid blocks. assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents. } + // Make sure nChainTx sum is correctly computed. + unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; + assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) + // Transaction may be completely unset - happens if only the header was accepted but the block hasn't been processed. + || (pindex->nChainTx == 0 && pindex->nTx == 0) + // nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet) + || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) + // Transaction counts prior to snapshot are unknown. + || pindex->IsAssumedValid()); // Chainstate-specific checks on setBlockIndexCandidates for (auto c : GetAll()) { if (c->m_chain.Tip() == nullptr) continue; @@ -5781,7 +5779,6 @@ static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) if (!opts.check_block_index.has_value()) opts.check_block_index = opts.chainparams.DefaultConsistencyChecks(); if (!opts.minimum_chain_work.has_value()) opts.minimum_chain_work = UintToArith256(opts.chainparams.GetConsensus().nMinimumChainWork); if (!opts.assumed_valid_block.has_value()) opts.assumed_valid_block = opts.chainparams.GetConsensus().defaultAssumeValid; - Assert(opts.adjusted_time_callback); return std::move(opts); } diff --git a/src/validation.h b/src/validation.h index 093cecfcd1..fd9b53df8f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -377,7 +377,6 @@ bool TestBlockValidity(BlockValidationState& state, Chainstate& chainstate, const CBlock& block, CBlockIndex* pindexPrev, - const std::function<NodeClock::time_point()>& adjusted_time_callback, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/wallet/db.h b/src/wallet/db.h index c263f54144..084fcadc24 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -20,7 +20,6 @@ class ArgsManager; struct bilingual_str; namespace wallet { -void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename); class DatabaseCursor { diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 5a13b5ac8e..6060f017ce 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -24,34 +24,15 @@ namespace wallet { -static void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector<CRecipient>& recipients) +std::vector<CRecipient> CreateRecipients(const std::vector<std::pair<CTxDestination, CAmount>>& outputs, const std::set<int>& subtract_fee_outputs) { - std::set<CTxDestination> destinations; - int i = 0; - for (const std::string& address: address_amounts.getKeys()) { - CTxDestination dest = DecodeDestination(address); - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + address); - } - - if (destinations.count(dest)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + address); - } - destinations.insert(dest); - - CAmount amount = AmountFromValue(address_amounts[i++]); - - bool subtract_fee = false; - for (unsigned int idx = 0; idx < subtract_fee_outputs.size(); idx++) { - const UniValue& addr = subtract_fee_outputs[idx]; - if (addr.get_str() == address) { - subtract_fee = true; - } - } - - CRecipient recipient = {dest, amount, subtract_fee}; + std::vector<CRecipient> recipients; + for (size_t i = 0; i < outputs.size(); ++i) { + const auto& [destination, amount] = outputs.at(i); + CRecipient recipient{destination, amount, subtract_fee_outputs.contains(i)}; recipients.push_back(recipient); } + return recipients; } static void InterpretFeeEstimationInstructions(const UniValue& conf_target, const UniValue& estimate_mode, const UniValue& fee_rate, UniValue& options) @@ -76,6 +57,37 @@ static void InterpretFeeEstimationInstructions(const UniValue& conf_target, cons } } +std::set<int> InterpretSubtractFeeFromOutputInstructions(const UniValue& sffo_instructions, const std::vector<std::string>& destinations) +{ + std::set<int> sffo_set; + if (sffo_instructions.isNull()) return sffo_set; + if (sffo_instructions.isBool()) { + if (sffo_instructions.get_bool()) sffo_set.insert(0); + return sffo_set; + } + for (const auto& sffo : sffo_instructions.getValues()) { + if (sffo.isStr()) { + for (size_t i = 0; i < destinations.size(); ++i) { + if (sffo.get_str() == destinations.at(i)) { + sffo_set.insert(i); + break; + } + } + } + if (sffo.isNum()) { + int pos = sffo.getInt<int>(); + if (sffo_set.contains(pos)) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated position: %d", pos)); + if (pos < 0) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, negative position: %d", pos)); + if (pos >= int(destinations.size())) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos)); + sffo_set.insert(pos); + } + } + return sffo_set; +} + static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const UniValue& options, const CMutableTransaction& rawTx) { // Make a blank psbt @@ -275,11 +287,6 @@ RPCHelpMan sendtoaddress() if (!request.params[3].isNull() && !request.params[3].get_str().empty()) mapValue["to"] = request.params[3].get_str(); - bool fSubtractFeeFromAmount = false; - if (!request.params[4].isNull()) { - fSubtractFeeFromAmount = request.params[4].get_bool(); - } - CCoinControl coin_control; if (!request.params[5].isNull()) { coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); @@ -296,13 +303,10 @@ RPCHelpMan sendtoaddress() UniValue address_amounts(UniValue::VOBJ); const std::string address = request.params[0].get_str(); address_amounts.pushKV(address, request.params[1]); - UniValue subtractFeeFromAmount(UniValue::VARR); - if (fSubtractFeeFromAmount) { - subtractFeeFromAmount.push_back(address); - } - - std::vector<CRecipient> recipients; - ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(address_amounts), + InterpretSubtractFeeFromOutputInstructions(request.params[4], address_amounts.getKeys()) + ); const bool verbose{request.params[10].isNull() ? false : request.params[10].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, mapValue, verbose); @@ -386,10 +390,6 @@ RPCHelpMan sendmany() if (!request.params[3].isNull() && !request.params[3].get_str().empty()) mapValue["comment"] = request.params[3].get_str(); - UniValue subtractFeeFromAmount(UniValue::VARR); - if (!request.params[4].isNull()) - subtractFeeFromAmount = request.params[4].get_array(); - CCoinControl coin_control; if (!request.params[5].isNull()) { coin_control.m_signal_bip125_rbf = request.params[5].get_bool(); @@ -397,8 +397,10 @@ RPCHelpMan sendmany() SetFeeEstimateMode(*pwallet, coin_control, /*conf_target=*/request.params[6], /*estimate_mode=*/request.params[7], /*fee_rate=*/request.params[8], /*override_min_fee=*/false); - std::vector<CRecipient> recipients; - ParseRecipients(sendTo, subtractFeeFromAmount, recipients); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(sendTo), + InterpretSubtractFeeFromOutputInstructions(request.params[4], sendTo.getKeys()) + ); const bool verbose{request.params[9].isNull() ? false : request.params[9].get_bool()}; return SendMoney(*pwallet, coin_control, recipients, std::move(mapValue), verbose); @@ -488,17 +490,17 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true) return args; } -CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const UniValue& options, CCoinControl& coinControl, bool override_min_fee) +CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector<CRecipient>& recipients, const UniValue& options, CCoinControl& coinControl, bool override_min_fee) { + // We want to make sure tx.vout is not used now that we are passing outputs as a vector of recipients. + // This sets us up to remove tx completely in a future PR in favor of passing the inputs directly. + CHECK_NONFATAL(tx.vout.empty()); // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); std::optional<unsigned int> change_position; bool lockUnspents = false; - UniValue subtractFeeFromOutputs; - std::set<int> setSubtractFeeFromOutputs; - if (!options.isNull()) { if (options.type() == UniValue::VBOOL) { // backward compatibility bool only fallback @@ -553,7 +555,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact if (options.exists("changePosition") || options.exists("change_position")) { int pos = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).getInt<int>(); - if (pos < 0 || (unsigned int)pos > tx.vout.size()) { + if (pos < 0 || (unsigned int)pos > recipients.size()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds"); } change_position = (unsigned int)pos; @@ -595,9 +597,6 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact coinControl.fOverrideFeeRate = true; } - if (options.exists("subtractFeeFromOutputs") || options.exists("subtract_fee_from_outputs") ) - subtractFeeFromOutputs = (options.exists("subtract_fee_from_outputs") ? options["subtract_fee_from_outputs"] : options["subtractFeeFromOutputs"]).get_array(); - if (options.exists("replaceable")) { coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } @@ -703,21 +702,10 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact } } - if (tx.vout.size() == 0) + if (recipients.empty()) throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output"); - for (unsigned int idx = 0; idx < subtractFeeFromOutputs.size(); idx++) { - int pos = subtractFeeFromOutputs[idx].getInt<int>(); - if (setSubtractFeeFromOutputs.count(pos)) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, duplicated position: %d", pos)); - if (pos < 0) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, negative position: %d", pos)); - if (pos >= int(tx.vout.size())) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid parameter, position too large: %d", pos)); - setSubtractFeeFromOutputs.insert(pos); - } - - auto txr = FundTransaction(wallet, tx, change_position, lockUnspents, setSubtractFeeFromOutputs, coinControl); + auto txr = FundTransaction(wallet, tx, recipients, change_position, lockUnspents, coinControl); if (!txr) { throw JSONRPCError(RPC_WALLET_ERROR, ErrorString(txr).original); } @@ -843,11 +831,25 @@ RPCHelpMan fundrawtransaction() if (!DecodeHexTx(tx, request.params[0].get_str(), try_no_witness, try_witness)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); } - + UniValue options = request.params[1]; + std::vector<std::pair<CTxDestination, CAmount>> destinations; + for (const auto& tx_out : tx.vout) { + CTxDestination dest; + ExtractDestination(tx_out.scriptPubKey, dest); + destinations.emplace_back(dest, tx_out.nValue); + } + std::vector<std::string> dummy(destinations.size(), "dummy"); + std::vector<CRecipient> recipients = CreateRecipients( + destinations, + InterpretSubtractFeeFromOutputInstructions(options["subtractFeeFromOutputs"], dummy) + ); CCoinControl coin_control; // Automatically select (additional) coins. Can be overridden by options.add_inputs. coin_control.m_allow_other_inputs = true; - auto txr = FundTransaction(*pwallet, tx, request.params[1], coin_control, /*override_min_fee=*/true); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + tx.vout.clear(); + auto txr = FundTransaction(*pwallet, tx, recipients, options, coin_control, /*override_min_fee=*/true); UniValue result(UniValue::VOBJ); result.pushKV("hex", EncodeHexTx(*txr.tx)); @@ -1275,13 +1277,22 @@ RPCHelpMan send() bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf}; + UniValue outputs(UniValue::VOBJ); + outputs = NormalizeOutputs(request.params[0]); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(outputs), + InterpretSubtractFeeFromOutputInstructions(options["subtract_fee_from_outputs"], outputs.getKeys()) + ); CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; SetOptionsInputWeights(options["inputs"], options); - auto txr = FundTransaction(*pwallet, rawTx, options, coin_control, /*override_min_fee=*/false); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + rawTx.vout.clear(); + auto txr = FundTransaction(*pwallet, rawTx, recipients, options, coin_control, /*override_min_fee=*/false); return FinishTransaction(pwallet, options, CMutableTransaction(*txr.tx)); } @@ -1711,12 +1722,21 @@ RPCHelpMan walletcreatefundedpsbt() const UniValue &replaceable_arg = options["replaceable"]; const bool rbf{replaceable_arg.isNull() ? wallet.m_signal_rbf : replaceable_arg.get_bool()}; CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); + UniValue outputs(UniValue::VOBJ); + outputs = NormalizeOutputs(request.params[1]); + std::vector<CRecipient> recipients = CreateRecipients( + ParseOutputs(outputs), + InterpretSubtractFeeFromOutputInstructions(options["subtractFeeFromOutputs"], outputs.getKeys()) + ); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; SetOptionsInputWeights(request.params[0], options); - auto txr = FundTransaction(wallet, rawTx, options, coin_control, /*override_min_fee=*/true); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + rawTx.vout.clear(); + auto txr = FundTransaction(wallet, rawTx, recipients, options, coin_control, /*override_min_fee=*/true); // Make a blank psbt PartiallySignedTransaction psbtx(CMutableTransaction(*txr.tx)); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 0e30d3e0fe..e6c021d426 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -415,8 +415,8 @@ static std::vector<RPCResult> TransactionDescriptionString() { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, }}, - {RPCResult::Type::STR_HEX, "replaced_by_txid", /*optional=*/true, "The txid if this tx was replaced."}, - {RPCResult::Type::STR_HEX, "replaces_txid", /*optional=*/true, "The txid if the tx replaces one."}, + {RPCResult::Type::STR_HEX, "replaced_by_txid", /*optional=*/true, "Only if 'category' is 'send'. The txid if this tx was replaced."}, + {RPCResult::Type::STR_HEX, "replaces_txid", /*optional=*/true, "Only if 'category' is 'send'. The txid if this tx replaces another."}, {RPCResult::Type::STR, "to", /*optional=*/true, "If a comment to is associated with the transaction."}, {RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."}, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index b7420d5fb7..e07771b17f 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -225,7 +225,7 @@ isminetype LegacyScriptPubKeyMan::IsMine(const CScript& script) const assert(false); } -bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys) +bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key) { { LOCK(cs_KeyStore); @@ -258,7 +258,7 @@ bool LegacyScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key LogPrintf("The wallet is probably corrupted: Some keys decrypt but not all.\n"); throw std::runtime_error("Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."); } - if (keyFail || (!keyPass && !accept_no_keys)) + if (keyFail || !keyPass) return false; fDecryptionThoroughlyChecked = true; } @@ -809,7 +809,9 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu std::vector<unsigned char> vchCryptedSecret; CKeyingMaterial vchSecret{UCharCast(key.begin()), UCharCast(key.end())}; - if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) { + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return EncryptSecret(encryption_key, vchSecret, pubkey.GetHash(), vchCryptedSecret); + })) { return false; } @@ -995,7 +997,9 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const { const CPubKey &vchPubKey = (*mi).second.first; const std::vector<unsigned char> &vchCryptedSecret = (*mi).second.second; - return DecryptKey(m_storage.GetEncryptionKey(), vchCryptedSecret, vchPubKey, keyOut); + return m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, vchCryptedSecret, vchPubKey, keyOut); + }); } return false; } @@ -2043,7 +2047,7 @@ isminetype DescriptorScriptPubKeyMan::IsMine(const CScript& script) const return ISMINE_NO; } -bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys) +bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master_key) { LOCK(cs_desc_man); if (!m_map_keys.empty()) { @@ -2068,7 +2072,7 @@ bool DescriptorScriptPubKeyMan::CheckDecryptionKey(const CKeyingMaterial& master LogPrintf("The wallet is probably corrupted: Some keys decrypt but not all.\n"); throw std::runtime_error("Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."); } - if (keyFail || (!keyPass && !accept_no_keys)) { + if (keyFail || !keyPass) { return false; } m_decryption_thoroughly_checked = true; @@ -2126,7 +2130,9 @@ std::map<CKeyID, CKey> DescriptorScriptPubKeyMan::GetKeys() const const CPubKey& pubkey = key_pair.second.first; const std::vector<unsigned char>& crypted_secret = key_pair.second.second; CKey key; - DecryptKey(m_storage.GetEncryptionKey(), crypted_secret, pubkey, key); + m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return DecryptKey(encryption_key, crypted_secret, pubkey, key); + }); keys[pubkey.GetID()] = key; } return keys; @@ -2260,7 +2266,9 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const std::vector<unsigned char> crypted_secret; CKeyingMaterial secret{UCharCast(key.begin()), UCharCast(key.end())}; - if (!EncryptSecret(m_storage.GetEncryptionKey(), secret, pubkey.GetHash(), crypted_secret)) { + if (!m_storage.WithEncryptionKey([&](const CKeyingMaterial& encryption_key) { + return EncryptSecret(encryption_key, secret, pubkey.GetHash(), crypted_secret); + })) { return false; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 449a75eb6b..1c99e5ffcd 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -22,6 +22,7 @@ #include <boost/signals2/signal.hpp> +#include <functional> #include <optional> #include <unordered_map> @@ -46,7 +47,8 @@ public: virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; virtual bool CanSupportFeature(enum WalletFeature) const = 0; virtual void SetMinVersion(enum WalletFeature, WalletBatch* = nullptr) = 0; - virtual const CKeyingMaterial& GetEncryptionKey() const = 0; + //! Pass the encryption key to cb(). + virtual bool WithEncryptionKey(std::function<bool (const CKeyingMaterial&)> cb) const = 0; virtual bool HasEncryptionKeys() const = 0; virtual bool IsLocked() const = 0; }; @@ -177,7 +179,7 @@ public: virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. - virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } + virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key) { return false; } virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } virtual util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { return util::Error{Untranslated("Not supported")}; } @@ -377,7 +379,7 @@ public: util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; - bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; + bool CheckDecryptionKey(const CKeyingMaterial& master_key) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; @@ -608,7 +610,7 @@ public: util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; - bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; + bool CheckDecryptionKey(const CKeyingMaterial& master_key) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index b51cd6332f..e229962ca5 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1127,7 +1127,12 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( return util::Error{err.empty() ?_("Insufficient funds") : err}; } const SelectionResult& result = *select_coins_res; - TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result.GetAlgo()).c_str(), result.GetTarget(), result.GetWaste(), result.GetSelectedValue()); + TRACE5(coin_selection, selected_coins, + wallet.GetName().c_str(), + GetAlgorithmName(result.GetAlgo()).c_str(), + result.GetTarget(), + result.GetWaste(), + result.GetSelectedValue()); const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee); if (change_amount > 0) { @@ -1336,8 +1341,11 @@ util::Result<CreatedTransactionResult> CreateTransaction( LOCK(wallet.cs_wallet); auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign); - TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), bool(res), - res ? res->fee : 0, res && res->change_pos.has_value() ? *res->change_pos : 0); + TRACE4(coin_selection, normal_create_tx_internal, + wallet.GetName().c_str(), + bool(res), + res ? res->fee : 0, + res && res->change_pos.has_value() ? int32_t(*res->change_pos) : -1); if (!res) return res; const auto& txr_ungrouped = *res; // try with avoidpartialspends unless it's enabled already @@ -1354,8 +1362,12 @@ util::Result<CreatedTransactionResult> CreateTransaction( auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false}; - TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(), - txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() && txr_grouped->change_pos.has_value() ? *txr_grouped->change_pos : 0); + TRACE5(coin_selection, aps_create_tx_internal, + wallet.GetName().c_str(), + use_aps, + txr_grouped.has_value(), + txr_grouped.has_value() ? txr_grouped->fee : 0, + txr_grouped.has_value() && txr_grouped->change_pos.has_value() ? int32_t(*txr_grouped->change_pos) : -1); if (txr_grouped) { wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); @@ -1365,18 +1377,11 @@ util::Result<CreatedTransactionResult> CreateTransaction( return res; } -util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional<unsigned int> change_pos, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl) +util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector<CRecipient>& vecSend, std::optional<unsigned int> change_pos, bool lockUnspents, CCoinControl coinControl) { - std::vector<CRecipient> vecSend; - - // Turn the txout set into a CRecipient vector. - for (size_t idx = 0; idx < tx.vout.size(); idx++) { - const CTxOut& txOut = tx.vout[idx]; - CTxDestination dest; - ExtractDestination(txOut.scriptPubKey, dest); - CRecipient recipient = {dest, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1}; - vecSend.push_back(recipient); - } + // We want to make sure tx.vout is not used now that we are passing outputs as a vector of recipients. + // This sets us up to remove tx completely in a future PR in favor of passing the inputs directly. + assert(tx.vout.empty()); // Set the user desired locktime coinControl.m_locktime = tx.nLockTime; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 3bd37cfd0e..62a7b4e4c8 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -224,7 +224,7 @@ util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const * Insert additional inputs into the transaction by * calling CreateTransaction(); */ -util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional<unsigned int> change_pos, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl); +util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector<CRecipient>& recipients, std::optional<unsigned int> change_pos, bool lockUnspents, CCoinControl); } // namespace wallet #endif // BITCOIN_WALLET_SPEND_H diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index db9989163d..cff3628049 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -377,6 +377,17 @@ void SQLiteDatabase::Close() m_db = nullptr; } +bool SQLiteDatabase::HasActiveTxn() +{ + // 'sqlite3_get_autocommit' returns true by default, and false if a transaction has begun and not been committed or rolled back. + return m_db && sqlite3_get_autocommit(m_db) == 0; +} + +int SQliteExecHandler::Exec(SQLiteDatabase& database, const std::string& statement) +{ + return sqlite3_exec(database.m_db, statement.data(), nullptr, nullptr, nullptr); +} + std::unique_ptr<DatabaseBatch> SQLiteDatabase::MakeBatch(bool flush_on_close) { // We ignore flush_on_close because we don't do manual flushing for SQLite @@ -394,12 +405,18 @@ SQLiteBatch::SQLiteBatch(SQLiteDatabase& database) void SQLiteBatch::Close() { - // If m_db is in a transaction (i.e. not in autocommit mode), then abort the transaction in progress - if (m_database.m_db && sqlite3_get_autocommit(m_database.m_db) == 0) { + bool force_conn_refresh = false; + + // If we began a transaction, and it wasn't committed, abort the transaction in progress + if (m_database.HasActiveTxn()) { if (TxnAbort()) { LogPrintf("SQLiteBatch: Batch closed unexpectedly without the transaction being explicitly committed or aborted\n"); } else { - LogPrintf("SQLiteBatch: Batch closed and failed to abort transaction\n"); + // If transaction cannot be aborted, it means there is a bug or there has been data corruption. Try to recover in this case + // by closing and reopening the database. Closing the database should also ensure that any changes made since the transaction + // was opened will be rolled back and future transactions can succeed without committing old data. + force_conn_refresh = true; + LogPrintf("SQLiteBatch: Batch closed and failed to abort transaction, resetting db connection..\n"); } } @@ -420,6 +437,17 @@ void SQLiteBatch::Close() } *stmt_prepared = nullptr; } + + if (force_conn_refresh) { + m_database.Close(); + try { + m_database.Open(); + } catch (const std::runtime_error&) { + // If open fails, cleanup this object and rethrow the exception + m_database.Close(); + throw; + } + } } bool SQLiteBatch::ReadKey(DataStream&& key, DataStream& value) @@ -606,8 +634,8 @@ std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewPrefixCursor(Span<const std:: bool SQLiteBatch::TxnBegin() { - if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) == 0) return false; - int res = sqlite3_exec(m_database.m_db, "BEGIN TRANSACTION", nullptr, nullptr, nullptr); + if (!m_database.m_db || m_database.HasActiveTxn()) return false; + int res = Assert(m_exec_handler)->Exec(m_database, "BEGIN TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to begin the transaction\n"); } @@ -616,8 +644,8 @@ bool SQLiteBatch::TxnBegin() bool SQLiteBatch::TxnCommit() { - if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) != 0) return false; - int res = sqlite3_exec(m_database.m_db, "COMMIT TRANSACTION", nullptr, nullptr, nullptr); + if (!m_database.HasActiveTxn()) return false; + int res = Assert(m_exec_handler)->Exec(m_database, "COMMIT TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to commit the transaction\n"); } @@ -626,8 +654,8 @@ bool SQLiteBatch::TxnCommit() bool SQLiteBatch::TxnAbort() { - if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) != 0) return false; - int res = sqlite3_exec(m_database.m_db, "ROLLBACK TRANSACTION", nullptr, nullptr, nullptr); + if (!m_database.HasActiveTxn()) return false; + int res = Assert(m_exec_handler)->Exec(m_database, "ROLLBACK TRANSACTION"); if (res != SQLITE_OK) { LogPrintf("SQLiteBatch: Failed to abort the transaction\n"); } diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index f1ce0567e1..ad91be1064 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -36,11 +36,21 @@ public: Status Next(DataStream& key, DataStream& value) override; }; +/** Class responsible for executing SQL statements in SQLite databases. + * Methods are virtual so they can be overridden by unit tests testing unusual database conditions. */ +class SQliteExecHandler +{ +public: + virtual ~SQliteExecHandler() {} + virtual int Exec(SQLiteDatabase& database, const std::string& statement); +}; + /** RAII class that provides access to a WalletDatabase */ class SQLiteBatch : public DatabaseBatch { private: SQLiteDatabase& m_database; + std::unique_ptr<SQliteExecHandler> m_exec_handler{std::make_unique<SQliteExecHandler>()}; sqlite3_stmt* m_read_stmt{nullptr}; sqlite3_stmt* m_insert_stmt{nullptr}; @@ -61,6 +71,8 @@ public: explicit SQLiteBatch(SQLiteDatabase& database); ~SQLiteBatch() override { Close(); } + void SetExecHandler(std::unique_ptr<SQliteExecHandler>&& handler) { m_exec_handler = std::move(handler); } + /* No-op. See comment on SQLiteDatabase::Flush */ void Flush() override {} @@ -142,6 +154,9 @@ public: /** Make a SQLiteBatch connected to this database */ std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override; + /** Return true if there is an on-going txn in this connection */ + bool HasActiveTxn(); + sqlite3* m_db{nullptr}; bool m_use_unsafe_sync; }; diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index d341e84d9b..7e6219378f 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -205,5 +205,82 @@ BOOST_AUTO_TEST_CASE(db_cursor_prefix_byte_test) } } +BOOST_AUTO_TEST_CASE(db_availability_after_write_error) +{ + // Ensures the database remains accessible without deadlocking after a write error. + // To simulate the behavior, record overwrites are disallowed, and the test verifies + // that the database remains active after failing to store an existing record. + for (const auto& database : TestDatabases(m_path_root)) { + // Write original record + std::unique_ptr<DatabaseBatch> batch = database->MakeBatch(); + std::string key = "key"; + std::string value = "value"; + std::string value2 = "value_2"; + BOOST_CHECK(batch->Write(key, value)); + // Attempt to overwrite the record (expect failure) + BOOST_CHECK(!batch->Write(key, value2, /*fOverwrite=*/false)); + // Successfully overwrite the record + BOOST_CHECK(batch->Write(key, value2, /*fOverwrite=*/true)); + // Sanity-check; read and verify the overwritten value + std::string read_value; + BOOST_CHECK(batch->Read(key, read_value)); + BOOST_CHECK_EQUAL(read_value, value2); + } +} + +#ifdef USE_SQLITE + +// Test-only statement execution error +constexpr int TEST_SQLITE_ERROR = -999; + +class DbExecBlocker : public SQliteExecHandler +{ +private: + SQliteExecHandler m_base_exec; + std::set<std::string> m_blocked_statements; +public: + DbExecBlocker(std::set<std::string> blocked_statements) : m_blocked_statements(blocked_statements) {} + int Exec(SQLiteDatabase& database, const std::string& statement) override { + if (m_blocked_statements.contains(statement)) return TEST_SQLITE_ERROR; + return m_base_exec.Exec(database, statement); + } +}; + +BOOST_AUTO_TEST_CASE(txn_close_failure_dangling_txn) +{ + // Verifies that there is no active dangling, to-be-reversed db txn + // after the batch object that initiated it is destroyed. + DatabaseOptions options; + DatabaseStatus status; + bilingual_str error; + std::unique_ptr<SQLiteDatabase> database = MakeSQLiteDatabase(m_path_root / "sqlite", options, status, error); + + std::string key = "key"; + std::string value = "value"; + + std::unique_ptr<SQLiteBatch> batch = std::make_unique<SQLiteBatch>(*database); + BOOST_CHECK(batch->TxnBegin()); + BOOST_CHECK(batch->Write(key, value)); + // Set a handler to prevent txn abortion during destruction. + // Mimicking a db statement execution failure. + batch->SetExecHandler(std::make_unique<DbExecBlocker>(std::set<std::string>{"ROLLBACK TRANSACTION"})); + // Destroy batch + batch.reset(); + + // Ensure there is no dangling, to-be-reversed db txn + BOOST_CHECK(!database->HasActiveTxn()); + + // And, just as a sanity check; verify that new batchs only write what they suppose to write + // and nothing else. + std::string key2 = "key2"; + std::unique_ptr<SQLiteBatch> batch2 = std::make_unique<SQLiteBatch>(*database); + BOOST_CHECK(batch2->Write(key2, value)); + // The first key must not exist + BOOST_CHECK(!batch2->Exists(key)); +} + +#endif // USE_SQLITE + + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 203ab5f606..376060421c 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -132,6 +132,14 @@ struct FuzzedWallet { } } } + std::vector<CRecipient> recipients; + for (size_t idx = 0; idx < tx.vout.size(); idx++) { + const CTxOut& tx_out = tx.vout[idx]; + CTxDestination dest; + ExtractDestination(tx_out.scriptPubKey, dest); + CRecipient recipient = {dest, tx_out.nValue, subtract_fee_from_outputs.count(idx) == 1}; + recipients.push_back(recipient); + } CCoinControl coin_control; coin_control.m_allow_other_inputs = fuzzed_data_provider.ConsumeBool(); CallOneOf( @@ -158,7 +166,10 @@ struct FuzzedWallet { int change_position{fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, tx.vout.size() - 1)}; bilingual_str error; - (void)FundTransaction(*wallet, tx, change_position, /*lockUnspents=*/false, subtract_fee_from_outputs, coin_control); + // Clear tx.vout since it is not meant to be used now that we are passing outputs directly. + // This sets us up for a future PR to completely remove tx from the function signature in favor of passing inputs directly + tx.vout.clear(); + (void)FundTransaction(*wallet, tx, recipients, change_position, /*lockUnspents=*/false, coin_control); } }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e03f5532fc..d03a3e979f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -555,7 +555,7 @@ void CWallet::UpgradeDescriptorCache() SetWalletFlag(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED); } -bool CWallet::Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys) +bool CWallet::Unlock(const SecureString& strWalletPassphrase) { CCrypter crypter; CKeyingMaterial _vMasterKey; @@ -568,7 +568,7 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase, bool accept_no_key return false; if (!crypter.Decrypt(pMasterKey.second.vchCryptedKey, _vMasterKey)) continue; // try another master key - if (Unlock(_vMasterKey, accept_no_keys)) { + if (Unlock(_vMasterKey)) { // Now that we've unlocked, upgrade the key metadata UpgradeKeyMetadata(); // Now that we've unlocked, upgrade the descriptor cache @@ -3374,12 +3374,12 @@ bool CWallet::Lock() return true; } -bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys) +bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn) { { LOCK(cs_wallet); for (const auto& spk_man_pair : m_spk_managers) { - if (!spk_man_pair.second->CheckDecryptionKey(vMasterKeyIn, accept_no_keys)) { + if (!spk_man_pair.second->CheckDecryptionKey(vMasterKeyIn)) { return false; } } @@ -3513,9 +3513,10 @@ void CWallet::SetupLegacyScriptPubKeyMan() AddScriptPubKeyMan(id, std::move(spk_manager)); } -const CKeyingMaterial& CWallet::GetEncryptionKey() const +bool CWallet::WithEncryptionKey(std::function<bool (const CKeyingMaterial&)> cb) const { - return vMasterKey; + LOCK(cs_wallet); + return cb(vMasterKey); } bool CWallet::HasEncryptionKeys() const @@ -3863,7 +3864,11 @@ std::optional<MigrationData> CWallet::GetDescriptorsForLegacy(bilingual_str& err AssertLockHeld(cs_wallet); LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); - assert(legacy_spkm); + if (!Assume(legacy_spkm)) { + // This shouldn't happen + error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing")); + return std::nullopt; + } std::optional<MigrationData> res = legacy_spkm->MigrateToDescriptor(); if (res == std::nullopt) { @@ -3878,8 +3883,9 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) AssertLockHeld(cs_wallet); LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); - if (!legacy_spkm) { - error = _("Error: This wallet is already a descriptor wallet"); + if (!Assume(legacy_spkm)) { + // This shouldn't happen + error = Untranslated(STR_INTERNAL_BUG("Error: Legacy wallet data missing")); return false; } @@ -3921,6 +3927,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) } } + // Get best block locator so that we can copy it to the watchonly and solvables + CBlockLocator best_block_locator; + if (!WalletBatch(GetDatabase()).ReadBestBlock(best_block_locator)) { + error = _("Error: Unable to read wallet's best block locator record"); + return false; + } + // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet. // We need to go through these in the tx insertion order so that lookups to spends works. std::vector<uint256> txids_to_delete; @@ -3931,32 +3944,47 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) LOCK(data.watchonly_wallet->cs_wallet); data.watchonly_wallet->nOrderPosNext = nOrderPosNext; watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext); + // Write the best block locator to avoid rescanning on reload + if (!watchonly_batch->WriteBestBlock(best_block_locator)) { + error = _("Error: Unable to write watchonly wallet best block locator record"); + return false; + } + } + if (data.solvable_wallet) { + // Write the best block locator to avoid rescanning on reload + if (!WalletBatch(data.solvable_wallet->GetDatabase()).WriteBestBlock(best_block_locator)) { + error = _("Error: Unable to write solvable wallet best block locator record"); + return false; + } } for (const auto& [_pos, wtx] : wtxOrdered) { - if (!IsMine(*wtx->tx) && !IsFromMe(*wtx->tx)) { - // Check it is the watchonly wallet's - // solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for - if (data.watchonly_wallet) { - LOCK(data.watchonly_wallet->cs_wallet); - if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) { - // Add to watchonly wallet - const uint256& hash = wtx->GetHash(); - const CWalletTx& to_copy_wtx = *wtx; - if (!data.watchonly_wallet->LoadToWallet(hash, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(data.watchonly_wallet->cs_wallet) { - if (!new_tx) return false; - ins_wtx.SetTx(to_copy_wtx.tx); - ins_wtx.CopyFrom(to_copy_wtx); - return true; - })) { - error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex()); - return false; - } - watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash)); - // Mark as to remove from this wallet + // Check it is the watchonly wallet's + // solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for + bool is_mine = IsMine(*wtx->tx) || IsFromMe(*wtx->tx); + if (data.watchonly_wallet) { + LOCK(data.watchonly_wallet->cs_wallet); + if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) { + // Add to watchonly wallet + const uint256& hash = wtx->GetHash(); + const CWalletTx& to_copy_wtx = *wtx; + if (!data.watchonly_wallet->LoadToWallet(hash, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(data.watchonly_wallet->cs_wallet) { + if (!new_tx) return false; + ins_wtx.SetTx(to_copy_wtx.tx); + ins_wtx.CopyFrom(to_copy_wtx); + return true; + })) { + error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex()); + return false; + } + watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash)); + // Mark as to remove from the migrated wallet only if it does not also belong to it + if (!is_mine) { txids_to_delete.push_back(hash); - continue; } + continue; } + } + if (!is_mine) { // Both not ours and not in the watchonly wallet error = strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex()); return false; @@ -4188,11 +4216,13 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle std::vector<bilingual_str> warnings; // If the wallet is still loaded, unload it so that nothing else tries to use it while we're changing it + bool was_loaded = false; if (auto wallet = GetWallet(context, wallet_name)) { if (!RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings)) { return util::Error{_("Unable to unload the wallet before migrating")}; } UnloadWallet(std::move(wallet)); + was_loaded = true; } // Load the wallet but only in the context of this function. @@ -4213,8 +4243,20 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error}; } + // Helper to reload as normal for some of our exit scenarios + const auto& reload_wallet = [&](std::shared_ptr<CWallet>& to_reload) { + assert(to_reload.use_count() == 1); + std::string name = to_reload->GetName(); + to_reload.reset(); + to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings); + return to_reload != nullptr; + }; + // Before anything else, check if there is something to migrate. - if (!local_wallet->GetLegacyScriptPubKeyMan()) { + if (local_wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + if (was_loaded) { + reload_wallet(local_wallet); + } return util::Error{_("Error: This wallet is already a descriptor wallet")}; } @@ -4223,32 +4265,44 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { + if (was_loaded) { + reload_wallet(local_wallet); + } return util::Error{_("Error: Unable to make a backup of your wallet")}; } res.backup_path = backup_path; bool success = false; - { - LOCK(local_wallet->cs_wallet); - // Unlock the wallet if needed - if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) { - if (passphrase.find('\0') == std::string::npos) { - return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")}; - } else { - return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase entered was incorrect. " - "The passphrase contains a null character (ie - a zero byte). " - "If this passphrase was set with a version of this software prior to 25.0, " - "please try again with only the characters up to — but not including — " - "the first null character.")}; - } + // Unlock the wallet if needed + if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) { + if (was_loaded) { + reload_wallet(local_wallet); + } + if (passphrase.find('\0') == std::string::npos) { + return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")}; + } else { + return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase entered was incorrect. " + "The passphrase contains a null character (ie - a zero byte). " + "If this passphrase was set with a version of this software prior to 25.0, " + "please try again with only the characters up to — but not including — " + "the first null character.")}; } + } + { + LOCK(local_wallet->cs_wallet); // First change to using SQLite if (!local_wallet->MigrateToSQLite(error)) return util::Error{error}; - // Do the migration, and cleanup if it fails - success = DoMigration(*local_wallet, context, error, res); + // Do the migration of keys and scripts for non-blank wallets, and cleanup if it fails + success = local_wallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); + if (!success) { + success = DoMigration(*local_wallet, context, error, res); + } else { + // Make sure that descriptors flag is actually set + local_wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + } } // In case of reloading failure, we need to remember the wallet dirs to remove @@ -4258,24 +4312,19 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle std::set<fs::path> wallet_dirs; if (success) { // Migration successful, unload all wallets locally, then reload them. - const auto& reload_wallet = [&](std::shared_ptr<CWallet>& to_reload) { - assert(to_reload.use_count() == 1); - std::string name = to_reload->GetName(); - wallet_dirs.insert(fs::PathFromString(to_reload->GetDatabase().Filename()).parent_path()); - to_reload.reset(); - to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings); - return to_reload != nullptr; - }; // Reload the main wallet + wallet_dirs.insert(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path()); success = reload_wallet(local_wallet); res.wallet = local_wallet; res.wallet_name = wallet_name; if (success && res.watchonly_wallet) { // Reload watchonly + wallet_dirs.insert(fs::PathFromString(res.watchonly_wallet->GetDatabase().Filename()).parent_path()); success = reload_wallet(res.watchonly_wallet); } if (success && res.solvables_wallet) { // Reload solvables + wallet_dirs.insert(fs::PathFromString(res.solvables_wallet->GetDatabase().Filename()).parent_path()); success = reload_wallet(res.solvables_wallet); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 487921443f..3c7b0ea490 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -302,7 +302,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati private: CKeyingMaterial vMasterKey GUARDED_BY(cs_wallet); - bool Unlock(const CKeyingMaterial& vMasterKeyIn, bool accept_no_keys = false); + bool Unlock(const CKeyingMaterial& vMasterKeyIn); std::atomic<bool> fAbortRescan{false}; std::atomic<bool> fScanningWallet{false}; // controlled by WalletRescanReserver @@ -578,7 +578,7 @@ public: // Used to prevent deleting the passphrase from memory when it is still in use. RecursiveMutex m_relock_mutex; - bool Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys = false); + bool Unlock(const SecureString& strWalletPassphrase); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); @@ -962,7 +962,8 @@ public: //! Make a LegacyScriptPubKeyMan and set it for all types, internal, and external. void SetupLegacyScriptPubKeyMan(); - const CKeyingMaterial& GetEncryptionKey() const override; + bool WithEncryptionKey(std::function<bool (const CKeyingMaterial&)> cb) const override; + bool HasEncryptionKeys() const override; /** Get last block processed height */ diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 9820c7c0ee..f3dd5b328e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1498,20 +1498,26 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas if (format == DatabaseFormat::SQLITE) { #ifdef USE_SQLITE - return MakeSQLiteDatabase(path, options, status, error); -#else - error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", fs::PathToString(path))); - status = DatabaseStatus::FAILED_BAD_FORMAT; - return nullptr; + if constexpr (true) { + return MakeSQLiteDatabase(path, options, status, error); + } else #endif + { + error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", fs::PathToString(path))); + status = DatabaseStatus::FAILED_BAD_FORMAT; + return nullptr; + } } #ifdef USE_BDB - return MakeBerkeleyDatabase(path, options, status, error); -#else - error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path))); - status = DatabaseStatus::FAILED_BAD_FORMAT; - return nullptr; + if constexpr (true) { + return MakeBerkeleyDatabase(path, options, status, error); + } else #endif + { + error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path))); + status = DatabaseStatus::FAILED_BAD_FORMAT; + return nullptr; + } } } // namespace wallet diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index 9839993115..a7ce864fde 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -8,9 +8,8 @@ import os import re import struct -from test_framework.messages import ser_uint256, hash256 +from test_framework.messages import ser_uint256, hash256, MAGIC_BYTES from test_framework.netutil import ADDRMAN_NEW_BUCKET_COUNT, ADDRMAN_TRIED_BUCKET_COUNT, ADDRMAN_BUCKET_SIZE -from test_framework.p2p import MAGIC_BYTES from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ErrorMatch from test_framework.util import assert_equal diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py index 3b75a06d9e..5d68f50f58 100755 --- a/test/functional/feature_anchors.py +++ b/test/functional/feature_anchors.py @@ -99,7 +99,7 @@ class AnchorsTest(BitcoinTestFramework): self.restart_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"]) self.log.info("Add 256-bit-address block-relay-only connections to node") - self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only') + self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only', v2transport=False) self.log.debug("Stop node") with self.nodes[0].assert_debug_log([f"DumpAnchors: Flush 1 outbound block-relay-only peer addresses to anchors.dat"]): diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index f26a300f70..528680f2ca 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -7,7 +7,7 @@ a serialized version of the UTXO set at a certain height, which corresponds to a hash that has been compiled into bitcoind. The assumeutxo value generated and used here is committed to in -`CRegTestParams::m_assumeutxo_data` in `src/chainparams.cpp`. +`CRegTestParams::m_assumeutxo_data` in `src/kernel/chainparams.cpp`. ## Possible test improvements @@ -40,8 +40,10 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, ) -from test_framework.wallet import getnewdestination - +from test_framework.wallet import ( + getnewdestination, + MiniWallet, +) START_HEIGHT = 199 SNAPSHOT_BASE_HEIGHT = 299 @@ -98,10 +100,10 @@ class AssumeutxoTest(BitcoinTestFramework): self.log.info(" - snapshot file with alternated UTXO data") cases = [ - [b"\xff" * 32, 0, "05030e506678f2eca8d624ffed97090ab3beadad1b51ee6e5985ba91c5720e37"], # wrong outpoint hash - [(1).to_bytes(4, "little"), 32, "7d29cfe2c1e242bc6f103878bb70cfffa8b4dac20dbd001ff6ce24b7de2d2399"], # wrong outpoint index - [b"\x81", 36, "f03939a195531f96d5dff983e294a1af62af86049fa7a19a7627246f237c03f1"], # wrong coin code VARINT((coinbase ? 1 : 0) | (height << 1)) - [b"\x83", 36, "e4577da84590fb288c0f7967e89575e1b0aa46624669640f6f5dfef028d39930"], # another wrong coin code + [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b"], # wrong outpoint hash + [(1).to_bytes(4, "little"), 32, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad"], # wrong outpoint index + [b"\x81", 36, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8"], # wrong coin code VARINT((coinbase ? 1 : 0) | (height << 1)) + [b"\x80", 36, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5"], # another wrong coin code ] for content, offset, wrong_hash in cases: @@ -109,7 +111,7 @@ class AssumeutxoTest(BitcoinTestFramework): f.write(valid_snapshot_contents[:(32 + 8 + offset)]) f.write(content) f.write(valid_snapshot_contents[(32 + 8 + offset + len(content)):]) - expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected 61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53, got {wrong_hash}") + expected_error(log_msg=f"[snapshot] bad snapshot content hash: expected a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27, got {wrong_hash}") def test_invalid_chainstate_scenarios(self): self.log.info("Test different scenarios of invalid snapshot chainstate in datadir") @@ -145,6 +147,8 @@ class AssumeutxoTest(BitcoinTestFramework): n1 = self.nodes[1] n2 = self.nodes[2] + self.mini_wallet = MiniWallet(n0) + # Mock time for a deterministic chain for n in self.nodes: n.setmocktime(n.getblockheader(n.getbestblockhash())['time']) @@ -157,6 +161,8 @@ class AssumeutxoTest(BitcoinTestFramework): # isn't waiting forever to see the header of the snapshot's base block # while disconnected from n0. for i in range(100): + if i % 3 == 0: + self.mini_wallet.send_self_transfer(from_node=n0) self.generate(n0, nblocks=1, sync_fun=self.no_op) newblock = n0.getblock(n0.getbestblockhash(), 0) @@ -178,8 +184,8 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal( dump_output['txoutset_hash'], - '61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53') - assert_equal(dump_output['nchaintx'], 300) + "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27") + assert_equal(dump_output["nchaintx"], 334) assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT) # Mine more blocks on top of the snapshot that n1 hasn't yet seen. This diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index 83f1c5003c..f0f32a61ab 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -11,7 +11,7 @@ """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.p2p import MAGIC_BYTES +from test_framework.messages import MAGIC_BYTES from test_framework.util import assert_equal diff --git a/test/functional/feature_settings.py b/test/functional/feature_settings.py index 1bacdd447a..0214e781de 100755 --- a/test/functional/feature_settings.py +++ b/test/functional/feature_settings.py @@ -23,10 +23,11 @@ class SettingsTest(BitcoinTestFramework): settings = node.chain_path / "settings.json" conf = node.datadir_path / "bitcoin.conf" - # Assert empty settings file was created + # Assert default settings file was created self.stop_node(0) + default_settings = {"_warning_": "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."} with settings.open() as fp: - assert_equal(json.load(fp), {}) + assert_equal(json.load(fp), default_settings) # Assert settings are parsed and logged with settings.open("w") as fp: @@ -48,12 +49,12 @@ class SettingsTest(BitcoinTestFramework): # Assert settings are unchanged after shutdown with settings.open() as fp: - assert_equal(json.load(fp), {"string": "string", "num": 5, "bool": True, "null": None, "list": [6, 7]}) + assert_equal(json.load(fp), {**default_settings, **{"string": "string", "num": 5, "bool": True, "null": None, "list": [6, 7]}}) # Test invalid json with settings.open("w") as fp: fp.write("invalid json") - node.assert_start_raises_init_error(expected_msg='Unable to parse settings file', match=ErrorMatch.PARTIAL_REGEX) + node.assert_start_raises_init_error(expected_msg='does not contain valid JSON. This is probably caused by disk corruption or a crash', match=ErrorMatch.PARTIAL_REGEX) # Test invalid json object with settings.open("w") as fp: diff --git a/test/functional/interface_usdt_coinselection.py b/test/functional/interface_usdt_coinselection.py index aff90ea5fc..30931a41cd 100755 --- a/test/functional/interface_usdt_coinselection.py +++ b/test/functional/interface_usdt_coinselection.py @@ -204,6 +204,29 @@ class CoinSelectionTracepointTest(BitcoinTestFramework): assert_equal(success, True) assert_equal(use_aps, None) + self.log.info("Change position is -1 if no change is created with APS when APS was initially not used") + # We should have 2 tracepoints in the order: + # 1. selected_coins (type 1) + # 2. normal_create_tx_internal (type 2) + # 3. attempting_aps_create_tx (type 3) + # 4. selected_coins (type 1) + # 5. aps_create_tx_internal (type 4) + wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True, avoid_reuse=False) + events = self.get_tracepoints([1, 2, 3, 1, 4]) + success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + assert_equal(success, True) + assert_equal(change_pos, -1) + + self.log.info("Change position is -1 if no change is created normally and APS is not used") + # We should have 2 tracepoints in the order: + # 1. selected_coins (type 1) + # 2. normal_create_tx_internal (type 2) + wallet.sendtoaddress(address=wallet.getnewaddress(), amount=wallet.getbalance(), subtractfeefromamount=True) + events = self.get_tracepoints([1, 2]) + success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events) + assert_equal(success, True) + assert_equal(change_pos, -1) + self.bpf.cleanup() diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py index f4462673f2..bd766a279e 100755 --- a/test/functional/p2p_add_connections.py +++ b/test/functional/p2p_add_connections.py @@ -17,7 +17,7 @@ class P2PFeelerReceiver(P2PInterface): # message is received from the test framework. Don't send any responses # to the node's version message since the connection will already be # closed. - pass + self.send_version() class P2PAddConnections(BitcoinTestFramework): def set_test_params(self): diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index 2adcaf178c..b23ec1028b 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -75,6 +75,7 @@ class AddrReceiver(P2PInterface): return self.num_ipv4_received != 0 def on_version(self, message): + self.send_version() self.send_message(msg_verack()) if (self.send_getaddr): self.send_message(msg_getaddr()) diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 0e349ef70c..8f5e6c0387 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -29,6 +29,7 @@ class PeerNoVerack(P2PInterface): # Avoid sending verack in response to version. # When calling add_p2p_connection, wait_for_verack=False must be set (see # comment in add_p2p_connection). + self.send_version() if message.nVersion >= 70016 and self.wtxidrelay: self.send_message(msg_wtxidrelay()) @@ -43,7 +44,8 @@ class SendTxrcnclReceiver(P2PInterface): class P2PFeelerReceiver(SendTxrcnclReceiver): def on_version(self, message): - pass # feeler connections can not send any message other than their own version + # feeler connections can not send any message other than their own version + self.send_version() class PeerTrackMsgOrder(P2PInterface): diff --git a/test/functional/p2p_v2_earlykeyresponse.py b/test/functional/p2p_v2_earlykeyresponse.py new file mode 100755 index 0000000000..1f570e6010 --- /dev/null +++ b/test/functional/p2p_v2_earlykeyresponse.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +import random + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.crypto.ellswift import ellswift_create +from test_framework.p2p import P2PInterface +from test_framework.v2_p2p import EncryptedP2PState + + +class TestEncryptedP2PState(EncryptedP2PState): + """ Modify v2 P2P protocol functions for testing that "The responder waits until one byte is received which does + not match the 16 bytes consisting of the network magic followed by "version\x00\x00\x00\x00\x00"." (see BIP 324) + + - if `send_net_magic` is True, send first 4 bytes of ellswift (match network magic) else send remaining 60 bytes + - `can_data_be_received` is a variable used to assert if data is received on recvbuf. + - v2 TestNode shouldn't respond back if we send V1_PREFIX and data shouldn't be received on recvbuf. + This state is represented using `can_data_be_received` = False. + - v2 TestNode responds back when mismatch from V1_PREFIX happens and data can be received on recvbuf. + This state is represented using `can_data_be_received` = True. + """ + + def __init__(self): + super().__init__(initiating=True, net='regtest') + self.send_net_magic = True + self.can_data_be_received = False + + def initiate_v2_handshake(self, garbage_len=random.randrange(4096)): + """Initiator begins the v2 handshake by sending its ellswift bytes and garbage. + Here, the 64 bytes ellswift is assumed to have it's 4 bytes match network magic bytes. It is sent in 2 phases: + 1. when `send_network_magic` = True, send first 4 bytes of ellswift (matches network magic bytes) + 2. when `send_network_magic` = False, send remaining 60 bytes of ellswift + """ + if self.send_net_magic: + self.privkey_ours, self.ellswift_ours = ellswift_create() + self.sent_garbage = random.randbytes(garbage_len) + self.send_net_magic = False + return b"\xfa\xbf\xb5\xda" + else: + self.can_data_be_received = True + return self.ellswift_ours[4:] + self.sent_garbage + + +class PeerEarlyKey(P2PInterface): + """Custom implementation of P2PInterface which uses modified v2 P2P protocol functions for testing purposes.""" + def __init__(self): + super().__init__() + self.v2_state = None + self.connection_opened = False + + def connection_made(self, transport): + """64 bytes ellswift is sent in 2 parts during `initial_v2_handshake()`""" + self.v2_state = TestEncryptedP2PState() + super().connection_made(transport) + + def data_received(self, t): + # check that data can be received on recvbuf only when mismatch from V1_PREFIX happens (send_net_magic = False) + assert self.v2_state.can_data_be_received and not self.v2_state.send_net_magic + + def on_open(self): + self.connection_opened = True + +class P2PEarlyKey(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [["-v2transport=1", "-peertimeout=3"]] + + def run_test(self): + self.log.info('Sending ellswift bytes in parts to ensure that response from responder is received only when') + self.log.info('ellswift bytes have a mismatch from the 16 bytes(network magic followed by "version\\x00\\x00\\x00\\x00\\x00")') + node0 = self.nodes[0] + self.log.info('Sending first 4 bytes of ellswift which match network magic') + self.log.info('If a response is received, assertion failure would happen in our custom data_received() function') + # send happens in `initiate_v2_handshake()` in `connection_made()` + peer1 = node0.add_p2p_connection(PeerEarlyKey(), wait_for_verack=False, send_version=False, supports_v2_p2p=True) + self.wait_until(lambda: peer1.connection_opened) + self.log.info('Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is') + self.log.info('expected now, our custom data_received() function wouldn\'t result in assertion failure') + ellswift_and_garbage_data = peer1.v2_state.initiate_v2_handshake() + peer1.send_raw_message(ellswift_and_garbage_data) + peer1.wait_for_disconnect(timeout=5) + self.log.info('successful disconnection when MITM happens in the key exchange phase') + + +if __name__ == '__main__': + P2PEarlyKey().main() diff --git a/test/functional/p2p_v2_encrypted.py b/test/functional/p2p_v2_encrypted.py new file mode 100755 index 0000000000..05755dece0 --- /dev/null +++ b/test/functional/p2p_v2_encrypted.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test encrypted v2 p2p proposed in BIP 324 +""" +from test_framework.blocktools import ( + create_block, + create_coinbase, +) +from test_framework.p2p import ( + P2PDataStore, + P2PInterface, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_greater_than, + check_node_connections, +) +from test_framework.crypto.chacha20 import REKEY_INTERVAL + + +class P2PEncrypted(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [["-v2transport=1"], ["-v2transport=1"]] + + def setup_network(self): + self.setup_nodes() + + def generate_blocks(self, node, number): + test_blocks = [] + last_block = node.getbestblockhash() + tip = int(last_block, 16) + tipheight = node.getblockcount() + last_block_time = node.getblock(last_block)['time'] + for _ in range(number): + # Create some blocks + block = create_block(tip, create_coinbase(tipheight + 1), last_block_time + 1) + block.solve() + test_blocks.append(block) + tip = block.sha256 + tipheight += 1 + last_block_time += 1 + return test_blocks + + def create_test_block(self, txs): + block = create_block(self.tip, create_coinbase(self.tipheight + 1), self.last_block_time + 600, txlist=txs) + block.solve() + return block + + def run_test(self): + node0, node1 = self.nodes[0], self.nodes[1] + self.log.info("Check inbound connection to v2 TestNode from v2 P2PConnection is v2") + peer1 = node0.add_p2p_connection(P2PInterface(), wait_for_verack=True, supports_v2_p2p=True) + assert peer1.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v2") + + self.log.info("Check inbound connection to v2 TestNode from v1 P2PConnection is v1") + peer2 = node0.add_p2p_connection(P2PInterface(), wait_for_verack=True, supports_v2_p2p=False) + assert not peer2.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v1") + + self.log.info("Check outbound connection from v2 TestNode to v1 P2PConnection advertised as v1 is v1") + peer3 = node0.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, supports_v2_p2p=False, advertise_v2_p2p=False) + assert not peer3.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v1") + + # v2 TestNode performs downgrading here + self.log.info("Check outbound connection from v2 TestNode to v1 P2PConnection advertised as v2 is v1") + peer4 = node0.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, supports_v2_p2p=False, advertise_v2_p2p=True) + assert not peer4.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v1") + + self.log.info("Check outbound connection from v2 TestNode to v2 P2PConnection advertised as v2 is v2") + peer5 = node0.add_outbound_p2p_connection(P2PInterface(), p2p_idx=2, supports_v2_p2p=True, advertise_v2_p2p=True) + assert peer5.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v2") + + self.log.info("Check if version is sent and verack is received in inbound/outbound connections") + assert_equal(len(node0.getpeerinfo()), 5) # check if above 5 connections are present in node0's getpeerinfo() + for peer in node0.getpeerinfo(): + assert_greater_than(peer['bytessent_per_msg']['version'], 0) + assert_greater_than(peer['bytesrecv_per_msg']['verack'], 0) + + self.log.info("Testing whether blocks propagate - check if tips sync when number of blocks >= REKEY_INTERVAL") + # tests whether rekeying (which happens every REKEY_INTERVAL packets) works correctly + test_blocks = self.generate_blocks(node0, REKEY_INTERVAL+1) + + for i in range(2): + peer6 = node0.add_p2p_connection(P2PDataStore(), supports_v2_p2p=True) + assert peer6.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v2") + + # Consider: node0 <-- peer6. node0 and node1 aren't connected here. + # Construct the following topology: node1 <--> node0 <-- peer6 + # and test that blocks produced by peer6 will be received by node1 if sent normally + # and won't be received by node1 if sent as decoy messages + + # First, check whether blocks produced be peer6 are received by node0 if sent normally + # and not received by node0 if sent as decoy messages. + if i: + # check that node0 receives blocks produced by peer6 + self.log.info("Check if blocks produced by node0's p2p connection is received by node0") + peer6.send_blocks_and_test(test_blocks, node0, success=True) # node0's tip advances + else: + # check that node0 doesn't receive blocks produced by peer6 since they are sent as decoy messages + self.log.info("Check if blocks produced by node0's p2p connection sent as decoys aren't received by node0") + peer6.send_blocks_and_test(test_blocks, node0, success=False, is_decoy=True) # node0's tip doesn't advance + + # Then, connect node0 and node1 using v2 and check whether the blocks are received by node1 + self.connect_nodes(0, 1, peer_advertises_v2=True) + self.log.info("Wait for node1 to receive all the blocks from node0") + self.sync_all() + self.log.info("Make sure node0 and node1 have same block tips") + assert_equal(node0.getbestblockhash(), node1.getbestblockhash()) + + self.disconnect_nodes(0, 1) + + self.log.info("Check the connections opened as expected") + check_node_connections(node=node0, num_in=4, num_out=3) + + self.log.info("Check inbound connection to v1 TestNode from v2 P2PConnection is v1") + self.restart_node(0, ["-v2transport=0"]) + peer1 = node0.add_p2p_connection(P2PInterface(), wait_for_verack=True, supports_v2_p2p=True) + assert not peer1.supports_v2_p2p + assert_equal(node0.getpeerinfo()[-1]["transport_protocol_type"], "v1") + check_node_connections(node=node0, num_in=1, num_out=0) + + +if __name__ == '__main__': + P2PEncrypted().main() diff --git a/test/functional/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py index 72d22cb77f..ec43fc5a97 100755 --- a/test/functional/p2p_v2_transport.py +++ b/test/functional/p2p_v2_transport.py @@ -7,8 +7,7 @@ Test v2 transport """ import socket -from test_framework.messages import NODE_P2P_V2 -from test_framework.p2p import MAGIC_BYTES +from test_framework.messages import MAGIC_BYTES, NODE_P2P_V2 from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index e1820b0f55..afb75ab208 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -244,7 +244,10 @@ class NetTest(BitcoinTestFramework): def test_service_flags(self): self.log.info("Test service flags") self.nodes[0].add_p2p_connection(P2PInterface(), services=(1 << 4) | (1 << 63)) - assert_equal(['UNKNOWN[2^4]', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) + if self.options.v2transport: + assert_equal(['UNKNOWN[2^4]', 'P2P_V2', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) + else: + assert_equal(['UNKNOWN[2^4]', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) self.nodes[0].disconnect_p2ps() def test_getnodeaddresses(self): diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index d008cb39aa..60641f66cf 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -25,7 +25,6 @@ from io import BytesIO import math import random import socket -import struct import time import unittest @@ -75,6 +74,13 @@ MAX_OP_RETURN_RELAY = 83 DEFAULT_MEMPOOL_EXPIRY_HOURS = 336 # hours +MAGIC_BYTES = { + "mainnet": b"\xf9\xbe\xb4\xd9", # mainnet + "testnet3": b"\x0b\x11\x09\x07", # testnet3 + "regtest": b"\xfa\xbf\xb5\xda", # regtest + "signet": b"\x0a\x03\xcf\x40", # signet +} + def sha256(s): return hashlib.sha256(s).digest() @@ -90,24 +96,24 @@ def hash256(s): def ser_compact_size(l): r = b"" if l < 253: - r = struct.pack("B", l) + r = l.to_bytes(1, "little") elif l < 0x10000: - r = struct.pack("<BH", 253, l) + r = (253).to_bytes(1, "little") + l.to_bytes(2, "little") elif l < 0x100000000: - r = struct.pack("<BI", 254, l) + r = (254).to_bytes(1, "little") + l.to_bytes(4, "little") else: - r = struct.pack("<BQ", 255, l) + r = (255).to_bytes(1, "little") + l.to_bytes(8, "little") return r def deser_compact_size(f): - nit = struct.unpack("<B", f.read(1))[0] + nit = int.from_bytes(f.read(1), "little") if nit == 253: - nit = struct.unpack("<H", f.read(2))[0] + nit = int.from_bytes(f.read(2), "little") elif nit == 254: - nit = struct.unpack("<I", f.read(4))[0] + nit = int.from_bytes(f.read(4), "little") elif nit == 255: - nit = struct.unpack("<Q", f.read(8))[0] + nit = int.from_bytes(f.read(8), "little") return nit @@ -272,13 +278,13 @@ class CAddress: """Deserialize from addrv1 format (pre-BIP155)""" if with_time: # VERSION messages serialize CAddress objects without time - self.time = struct.unpack("<I", f.read(4))[0] - self.nServices = struct.unpack("<Q", f.read(8))[0] + self.time = int.from_bytes(f.read(4), "little") + self.nServices = int.from_bytes(f.read(8), "little") # We only support IPv4 which means skip 12 bytes and read the next 4 as IPv4 address. f.read(12) self.net = self.NET_IPV4 self.ip = socket.inet_ntoa(f.read(4)) - self.port = struct.unpack(">H", f.read(2))[0] + self.port = int.from_bytes(f.read(2), "big") def serialize(self, *, with_time=True): """Serialize in addrv1 format (pre-BIP155)""" @@ -286,20 +292,20 @@ class CAddress: r = b"" if with_time: # VERSION messages serialize CAddress objects without time - r += struct.pack("<I", self.time) - r += struct.pack("<Q", self.nServices) + r += self.time.to_bytes(4, "little") + r += self.nServices.to_bytes(8, "little") r += b"\x00" * 10 + b"\xff" * 2 r += socket.inet_aton(self.ip) - r += struct.pack(">H", self.port) + r += self.port.to_bytes(2, "big") return r def deserialize_v2(self, f): """Deserialize from addrv2 format (BIP155)""" - self.time = struct.unpack("<I", f.read(4))[0] + self.time = int.from_bytes(f.read(4), "little") self.nServices = deser_compact_size(f) - self.net = struct.unpack("B", f.read(1))[0] + self.net = int.from_bytes(f.read(1), "little") assert self.net in self.ADDRV2_NET_NAME address_length = deser_compact_size(f) @@ -322,15 +328,15 @@ class CAddress: else: raise Exception(f"Address type not supported") - self.port = struct.unpack(">H", f.read(2))[0] + self.port = int.from_bytes(f.read(2), "big") def serialize_v2(self): """Serialize in addrv2 format (BIP155)""" assert self.net in self.ADDRV2_NET_NAME r = b"" - r += struct.pack("<I", self.time) + r += self.time.to_bytes(4, "little") r += ser_compact_size(self.nServices) - r += struct.pack("B", self.net) + r += self.net.to_bytes(1, "little") r += ser_compact_size(self.ADDRV2_ADDRESS_LENGTH[self.net]) if self.net == self.NET_IPV4: r += socket.inet_aton(self.ip) @@ -348,7 +354,7 @@ class CAddress: r += socket.inet_pton(socket.AF_INET6, self.ip) else: raise Exception(f"Address type not supported") - r += struct.pack(">H", self.port) + r += self.port.to_bytes(2, "big") return r def __repr__(self): @@ -375,12 +381,12 @@ class CInv: self.hash = h def deserialize(self, f): - self.type = struct.unpack("<I", f.read(4))[0] + self.type = int.from_bytes(f.read(4), "little") self.hash = deser_uint256(f) def serialize(self): r = b"" - r += struct.pack("<I", self.type) + r += self.type.to_bytes(4, "little") r += ser_uint256(self.hash) return r @@ -399,12 +405,12 @@ class CBlockLocator: self.vHave = [] def deserialize(self, f): - struct.unpack("<i", f.read(4))[0] # Ignore version field. + int.from_bytes(f.read(4), "little", signed=True) # Ignore version field. self.vHave = deser_uint256_vector(f) def serialize(self): r = b"" - r += struct.pack("<i", 0) # Bitcoin Core ignores version field. Set it to 0. + r += (0).to_bytes(4, "little", signed=True) # Bitcoin Core ignores the version field. Set it to 0. r += ser_uint256_vector(self.vHave) return r @@ -421,12 +427,12 @@ class COutPoint: def deserialize(self, f): self.hash = deser_uint256(f) - self.n = struct.unpack("<I", f.read(4))[0] + self.n = int.from_bytes(f.read(4), "little") def serialize(self): r = b"" r += ser_uint256(self.hash) - r += struct.pack("<I", self.n) + r += self.n.to_bytes(4, "little") return r def __repr__(self): @@ -448,13 +454,13 @@ class CTxIn: self.prevout = COutPoint() self.prevout.deserialize(f) self.scriptSig = deser_string(f) - self.nSequence = struct.unpack("<I", f.read(4))[0] + self.nSequence = int.from_bytes(f.read(4), "little") def serialize(self): r = b"" r += self.prevout.serialize() r += ser_string(self.scriptSig) - r += struct.pack("<I", self.nSequence) + r += self.nSequence.to_bytes(4, "little") return r def __repr__(self): @@ -471,12 +477,12 @@ class CTxOut: self.scriptPubKey = scriptPubKey def deserialize(self, f): - self.nValue = struct.unpack("<q", f.read(8))[0] + self.nValue = int.from_bytes(f.read(8), "little", signed=True) self.scriptPubKey = deser_string(f) def serialize(self): r = b"" - r += struct.pack("<q", self.nValue) + r += self.nValue.to_bytes(8, "little", signed=True) r += ser_string(self.scriptPubKey) return r @@ -575,11 +581,11 @@ class CTransaction: self.wit = copy.deepcopy(tx.wit) def deserialize(self, f): - self.nVersion = struct.unpack("<i", f.read(4))[0] + self.nVersion = int.from_bytes(f.read(4), "little", signed=True) self.vin = deser_vector(f, CTxIn) flags = 0 if len(self.vin) == 0: - flags = struct.unpack("<B", f.read(1))[0] + flags = int.from_bytes(f.read(1), "little") # Not sure why flags can't be zero, but this # matches the implementation in bitcoind if (flags != 0): @@ -592,16 +598,16 @@ class CTransaction: self.wit.deserialize(f) else: self.wit = CTxWitness() - self.nLockTime = struct.unpack("<I", f.read(4))[0] + self.nLockTime = int.from_bytes(f.read(4), "little") self.sha256 = None self.hash = None def serialize_without_witness(self): r = b"" - r += struct.pack("<i", self.nVersion) + r += self.nVersion.to_bytes(4, "little", signed=True) r += ser_vector(self.vin) r += ser_vector(self.vout) - r += struct.pack("<I", self.nLockTime) + r += self.nLockTime.to_bytes(4, "little") return r # Only serialize with witness when explicitly called for @@ -610,11 +616,11 @@ class CTransaction: if not self.wit.is_null(): flags |= 1 r = b"" - r += struct.pack("<i", self.nVersion) + r += self.nVersion.to_bytes(4, "little", signed=True) if flags: dummy = [] r += ser_vector(dummy) - r += struct.pack("<B", flags) + r += flags.to_bytes(1, "little") r += ser_vector(self.vin) r += ser_vector(self.vout) if flags & 1: @@ -624,7 +630,7 @@ class CTransaction: for _ in range(len(self.wit.vtxinwit), len(self.vin)): self.wit.vtxinwit.append(CTxInWitness()) r += self.wit.serialize() - r += struct.pack("<I", self.nLockTime) + r += self.nLockTime.to_bytes(4, "little") return r # Regular serialization is with witness -- must explicitly @@ -703,34 +709,34 @@ class CBlockHeader: self.hash = None def deserialize(self, f): - self.nVersion = struct.unpack("<i", f.read(4))[0] + self.nVersion = int.from_bytes(f.read(4), "little", signed=True) self.hashPrevBlock = deser_uint256(f) self.hashMerkleRoot = deser_uint256(f) - self.nTime = struct.unpack("<I", f.read(4))[0] - self.nBits = struct.unpack("<I", f.read(4))[0] - self.nNonce = struct.unpack("<I", f.read(4))[0] + self.nTime = int.from_bytes(f.read(4), "little") + self.nBits = int.from_bytes(f.read(4), "little") + self.nNonce = int.from_bytes(f.read(4), "little") self.sha256 = None self.hash = None def serialize(self): r = b"" - r += struct.pack("<i", self.nVersion) + r += self.nVersion.to_bytes(4, "little", signed=True) r += ser_uint256(self.hashPrevBlock) r += ser_uint256(self.hashMerkleRoot) - r += struct.pack("<I", self.nTime) - r += struct.pack("<I", self.nBits) - r += struct.pack("<I", self.nNonce) + r += self.nTime.to_bytes(4, "little") + r += self.nBits.to_bytes(4, "little") + r += self.nNonce.to_bytes(4, "little") return r def calc_sha256(self): if self.sha256 is None: r = b"" - r += struct.pack("<i", self.nVersion) + r += self.nVersion.to_bytes(4, "little", signed=True) r += ser_uint256(self.hashPrevBlock) r += ser_uint256(self.hashMerkleRoot) - r += struct.pack("<I", self.nTime) - r += struct.pack("<I", self.nBits) - r += struct.pack("<I", self.nNonce) + r += self.nTime.to_bytes(4, "little") + r += self.nBits.to_bytes(4, "little") + r += self.nNonce.to_bytes(4, "little") self.sha256 = uint256_from_str(hash256(r)) self.hash = hash256(r)[::-1].hex() @@ -874,12 +880,12 @@ class P2PHeaderAndShortIDs: def deserialize(self, f): self.header.deserialize(f) - self.nonce = struct.unpack("<Q", f.read(8))[0] + self.nonce = int.from_bytes(f.read(8), "little") self.shortids_length = deser_compact_size(f) for _ in range(self.shortids_length): # shortids are defined to be 6 bytes in the spec, so append # two zero bytes and read it in as an 8-byte number - self.shortids.append(struct.unpack("<Q", f.read(6) + b'\x00\x00')[0]) + self.shortids.append(int.from_bytes(f.read(6) + b'\x00\x00', "little")) self.prefilled_txn = deser_vector(f, PrefilledTransaction) self.prefilled_txn_length = len(self.prefilled_txn) @@ -887,11 +893,11 @@ class P2PHeaderAndShortIDs: def serialize(self, with_witness=False): r = b"" r += self.header.serialize() - r += struct.pack("<Q", self.nonce) + r += self.nonce.to_bytes(8, "little") r += ser_compact_size(self.shortids_length) for x in self.shortids: # We only want the first 6 bytes - r += struct.pack("<Q", x)[0:6] + r += x.to_bytes(8, "little")[0:6] if with_witness: r += ser_vector(self.prefilled_txn, "serialize_with_witness") else: @@ -956,10 +962,10 @@ class HeaderAndShortIDs: def get_siphash_keys(self): header_nonce = self.header.serialize() - header_nonce += struct.pack("<Q", self.nonce) + header_nonce += self.nonce.to_bytes(8, "little") hash_header_nonce_as_str = sha256(header_nonce) - key0 = struct.unpack("<Q", hash_header_nonce_as_str[0:8])[0] - key1 = struct.unpack("<Q", hash_header_nonce_as_str[8:16])[0] + key0 = int.from_bytes(hash_header_nonce_as_str[0:8], "little") + key1 = int.from_bytes(hash_header_nonce_as_str[8:16], "little") return [ key0, key1 ] # Version 2 compact blocks use wtxid in shortids (rather than txid) @@ -1057,7 +1063,7 @@ class CPartialMerkleTree: self.vBits = [] def deserialize(self, f): - self.nTransactions = struct.unpack("<i", f.read(4))[0] + self.nTransactions = int.from_bytes(f.read(4), "little", signed=True) self.vHash = deser_uint256_vector(f) vBytes = deser_string(f) self.vBits = [] @@ -1066,7 +1072,7 @@ class CPartialMerkleTree: def serialize(self): r = b"" - r += struct.pack("<i", self.nTransactions) + r += self.nTransactions.to_bytes(4, "little", signed=True) r += ser_uint256_vector(self.vHash) vBytesArray = bytearray([0x00] * ((len(self.vBits) + 7)//8)) for i in range(len(self.vBits)): @@ -1117,37 +1123,34 @@ class msg_version: self.relay = 0 def deserialize(self, f): - self.nVersion = struct.unpack("<i", f.read(4))[0] - self.nServices = struct.unpack("<Q", f.read(8))[0] - self.nTime = struct.unpack("<q", f.read(8))[0] + self.nVersion = int.from_bytes(f.read(4), "little", signed=True) + self.nServices = int.from_bytes(f.read(8), "little") + self.nTime = int.from_bytes(f.read(8), "little", signed=True) self.addrTo = CAddress() self.addrTo.deserialize(f, with_time=False) self.addrFrom = CAddress() self.addrFrom.deserialize(f, with_time=False) - self.nNonce = struct.unpack("<Q", f.read(8))[0] + self.nNonce = int.from_bytes(f.read(8), "little") self.strSubVer = deser_string(f).decode('utf-8') - self.nStartingHeight = struct.unpack("<i", f.read(4))[0] + self.nStartingHeight = int.from_bytes(f.read(4), "little", signed=True) # Relay field is optional for version 70001 onwards # But, unconditionally check it to match behaviour in bitcoind - try: - self.relay = struct.unpack("<b", f.read(1))[0] - except struct.error: - self.relay = 0 + self.relay = int.from_bytes(f.read(1), "little") # f.read(1) may return an empty b'' def serialize(self): r = b"" - r += struct.pack("<i", self.nVersion) - r += struct.pack("<Q", self.nServices) - r += struct.pack("<q", self.nTime) + r += self.nVersion.to_bytes(4, "little", signed=True) + r += self.nServices.to_bytes(8, "little") + r += self.nTime.to_bytes(8, "little", signed=True) r += self.addrTo.serialize(with_time=False) r += self.addrFrom.serialize(with_time=False) - r += struct.pack("<Q", self.nNonce) + r += self.nNonce.to_bytes(8, "little") r += ser_string(self.strSubVer.encode('utf-8')) - r += struct.pack("<i", self.nStartingHeight) - r += struct.pack("<b", self.relay) + r += self.nStartingHeight.to_bytes(4, "little", signed=True) + r += self.relay.to_bytes(1, "little") return r def __repr__(self): @@ -1393,11 +1396,11 @@ class msg_ping: self.nonce = nonce def deserialize(self, f): - self.nonce = struct.unpack("<Q", f.read(8))[0] + self.nonce = int.from_bytes(f.read(8), "little") def serialize(self): r = b"" - r += struct.pack("<Q", self.nonce) + r += self.nonce.to_bytes(8, "little") return r def __repr__(self): @@ -1412,11 +1415,11 @@ class msg_pong: self.nonce = nonce def deserialize(self, f): - self.nonce = struct.unpack("<Q", f.read(8))[0] + self.nonce = int.from_bytes(f.read(8), "little") def serialize(self): r = b"" - r += struct.pack("<Q", self.nonce) + r += self.nonce.to_bytes(8, "little") return r def __repr__(self): @@ -1557,16 +1560,16 @@ class msg_filterload: def deserialize(self, f): self.data = deser_string(f) - self.nHashFuncs = struct.unpack("<I", f.read(4))[0] - self.nTweak = struct.unpack("<I", f.read(4))[0] - self.nFlags = struct.unpack("<B", f.read(1))[0] + self.nHashFuncs = int.from_bytes(f.read(4), "little") + self.nTweak = int.from_bytes(f.read(4), "little") + self.nFlags = int.from_bytes(f.read(1), "little") def serialize(self): r = b"" r += ser_string(self.data) - r += struct.pack("<I", self.nHashFuncs) - r += struct.pack("<I", self.nTweak) - r += struct.pack("<B", self.nFlags) + r += self.nHashFuncs.to_bytes(4, "little") + r += self.nTweak.to_bytes(4, "little") + r += self.nFlags.to_bytes(1, "little") return r def __repr__(self): @@ -1618,11 +1621,11 @@ class msg_feefilter: self.feerate = feerate def deserialize(self, f): - self.feerate = struct.unpack("<Q", f.read(8))[0] + self.feerate = int.from_bytes(f.read(8), "little") def serialize(self): r = b"" - r += struct.pack("<Q", self.feerate) + r += self.feerate.to_bytes(8, "little") return r def __repr__(self): @@ -1638,13 +1641,13 @@ class msg_sendcmpct: self.version = version def deserialize(self, f): - self.announce = struct.unpack("<?", f.read(1))[0] - self.version = struct.unpack("<Q", f.read(8))[0] + self.announce = bool(int.from_bytes(f.read(1), "little")) + self.version = int.from_bytes(f.read(8), "little") def serialize(self): r = b"" - r += struct.pack("<?", self.announce) - r += struct.pack("<Q", self.version) + r += int(self.announce).to_bytes(1, "little") + r += self.version.to_bytes(8, "little") return r def __repr__(self): @@ -1727,14 +1730,14 @@ class msg_getcfilters: self.stop_hash = stop_hash def deserialize(self, f): - self.filter_type = struct.unpack("<B", f.read(1))[0] - self.start_height = struct.unpack("<I", f.read(4))[0] + self.filter_type = int.from_bytes(f.read(1), "little") + self.start_height = int.from_bytes(f.read(4), "little") self.stop_hash = deser_uint256(f) def serialize(self): r = b"" - r += struct.pack("<B", self.filter_type) - r += struct.pack("<I", self.start_height) + r += self.filter_type.to_bytes(1, "little") + r += self.start_height.to_bytes(4, "little") r += ser_uint256(self.stop_hash) return r @@ -1752,13 +1755,13 @@ class msg_cfilter: self.filter_data = filter_data def deserialize(self, f): - self.filter_type = struct.unpack("<B", f.read(1))[0] + self.filter_type = int.from_bytes(f.read(1), "little") self.block_hash = deser_uint256(f) self.filter_data = deser_string(f) def serialize(self): r = b"" - r += struct.pack("<B", self.filter_type) + r += self.filter_type.to_bytes(1, "little") r += ser_uint256(self.block_hash) r += ser_string(self.filter_data) return r @@ -1777,14 +1780,14 @@ class msg_getcfheaders: self.stop_hash = stop_hash def deserialize(self, f): - self.filter_type = struct.unpack("<B", f.read(1))[0] - self.start_height = struct.unpack("<I", f.read(4))[0] + self.filter_type = int.from_bytes(f.read(1), "little") + self.start_height = int.from_bytes(f.read(4), "little") self.stop_hash = deser_uint256(f) def serialize(self): r = b"" - r += struct.pack("<B", self.filter_type) - r += struct.pack("<I", self.start_height) + r += self.filter_type.to_bytes(1, "little") + r += self.start_height.to_bytes(4, "little") r += ser_uint256(self.stop_hash) return r @@ -1803,14 +1806,14 @@ class msg_cfheaders: self.hashes = hashes def deserialize(self, f): - self.filter_type = struct.unpack("<B", f.read(1))[0] + self.filter_type = int.from_bytes(f.read(1), "little") self.stop_hash = deser_uint256(f) self.prev_header = deser_uint256(f) self.hashes = deser_uint256_vector(f) def serialize(self): r = b"" - r += struct.pack("<B", self.filter_type) + r += self.filter_type.to_bytes(1, "little") r += ser_uint256(self.stop_hash) r += ser_uint256(self.prev_header) r += ser_uint256_vector(self.hashes) @@ -1829,12 +1832,12 @@ class msg_getcfcheckpt: self.stop_hash = stop_hash def deserialize(self, f): - self.filter_type = struct.unpack("<B", f.read(1))[0] + self.filter_type = int.from_bytes(f.read(1), "little") self.stop_hash = deser_uint256(f) def serialize(self): r = b"" - r += struct.pack("<B", self.filter_type) + r += self.filter_type.to_bytes(1, "little") r += ser_uint256(self.stop_hash) return r @@ -1852,13 +1855,13 @@ class msg_cfcheckpt: self.headers = headers def deserialize(self, f): - self.filter_type = struct.unpack("<B", f.read(1))[0] + self.filter_type = int.from_bytes(f.read(1), "little") self.stop_hash = deser_uint256(f) self.headers = deser_uint256_vector(f) def serialize(self): r = b"" - r += struct.pack("<B", self.filter_type) + r += self.filter_type.to_bytes(1, "little") r += ser_uint256(self.stop_hash) r += ser_uint256_vector(self.headers) return r @@ -1876,13 +1879,13 @@ class msg_sendtxrcncl: self.salt = 0 def deserialize(self, f): - self.version = struct.unpack("<I", f.read(4))[0] - self.salt = struct.unpack("<Q", f.read(8))[0] + self.version = int.from_bytes(f.read(4), "little") + self.salt = int.from_bytes(f.read(8), "little") def serialize(self): r = b"" - r += struct.pack("<I", self.version) - r += struct.pack("<Q", self.salt) + r += self.version.to_bytes(4, "little") + r += self.salt.to_bytes(8, "little") return r def __repr__(self): diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 34fe467d23..dc04696114 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -73,6 +73,7 @@ from test_framework.messages import ( msg_wtxidrelay, NODE_NETWORK, NODE_WITNESS, + MAGIC_BYTES, sha256, ) from test_framework.util import ( @@ -80,6 +81,11 @@ from test_framework.util import ( p2p_port, wait_until_helper_internal, ) +from test_framework.v2_p2p import ( + EncryptedP2PState, + MSGTYPE_TO_SHORTID, + SHORTID, +) logger = logging.getLogger("TestFramework.p2p") @@ -141,13 +147,6 @@ MESSAGEMAP = { b"wtxidrelay": msg_wtxidrelay, } -MAGIC_BYTES = { - "mainnet": b"\xf9\xbe\xb4\xd9", # mainnet - "testnet3": b"\x0b\x11\x09\x07", # testnet3 - "regtest": b"\xfa\xbf\xb5\xda", # regtest - "signet": b"\x0a\x03\xcf\x40", # signet -} - class P2PConnection(asyncio.Protocol): """A low-level connection object to a node's P2P interface. @@ -166,11 +165,20 @@ class P2PConnection(asyncio.Protocol): # The underlying transport of the connection. # Should only call methods on this from the NetworkThread, c.f. call_soon_threadsafe self._transport = None + # This lock is acquired before sending messages over the socket. There's an implied lock order and + # p2p_lock must not be acquired after _send_lock as it could result in deadlocks. + self._send_lock = threading.Lock() + self.v2_state = None # EncryptedP2PState object needed for v2 p2p connections + self.reconnect = False # set if reconnection needs to happen @property def is_connected(self): return self._transport is not None + @property + def supports_v2_p2p(self): + return self.v2_state is not None + def peer_connect_helper(self, dstaddr, dstport, net, timeout_factor): assert not self.is_connected self.timeout_factor = timeout_factor @@ -181,16 +189,21 @@ class P2PConnection(asyncio.Protocol): self.recvbuf = b"" self.magic_bytes = MAGIC_BYTES[net] - def peer_connect(self, dstaddr, dstport, *, net, timeout_factor): + def peer_connect(self, dstaddr, dstport, *, net, timeout_factor, supports_v2_p2p): self.peer_connect_helper(dstaddr, dstport, net, timeout_factor) + if supports_v2_p2p: + self.v2_state = EncryptedP2PState(initiating=True, net=net) loop = NetworkThread.network_event_loop logger.debug('Connecting to Bitcoin Node: %s:%d' % (self.dstaddr, self.dstport)) coroutine = loop.create_connection(lambda: self, host=self.dstaddr, port=self.dstport) return lambda: loop.call_soon_threadsafe(loop.create_task, coroutine) - def peer_accept_connection(self, connect_id, connect_cb=lambda: None, *, net, timeout_factor): + def peer_accept_connection(self, connect_id, connect_cb=lambda: None, *, net, timeout_factor, supports_v2_p2p, reconnect): self.peer_connect_helper('0', 0, net, timeout_factor) + self.reconnect = reconnect + if supports_v2_p2p: + self.v2_state = EncryptedP2PState(initiating=False, net=net) logger.debug('Listening for Bitcoin Node with id: {}'.format(connect_id)) return lambda: NetworkThread.listen(self, connect_cb, idx=connect_id) @@ -206,14 +219,21 @@ class P2PConnection(asyncio.Protocol): assert not self._transport logger.debug("Connected & Listening: %s:%d" % (self.dstaddr, self.dstport)) self._transport = transport - if self.on_connection_send_msg: - self.send_message(self.on_connection_send_msg) - self.on_connection_send_msg = None # Never used again + # in an inbound connection to the TestNode with P2PConnection as the initiator, [TestNode <---- P2PConnection] + # send the initial handshake immediately + if self.supports_v2_p2p and self.v2_state.initiating and not self.v2_state.tried_v2_handshake: + send_handshake_bytes = self.v2_state.initiate_v2_handshake() + self.send_raw_message(send_handshake_bytes) + # for v1 outbound connections, send version message immediately after opening + # (for v2 outbound connections, send it after the initial v2 handshake) + if self.p2p_connected_to_node and not self.supports_v2_p2p: + self.send_version() self.on_open() def connection_lost(self, exc): """asyncio callback when a connection is closed.""" - if exc: + # don't display warning if reconnection needs to be attempted using v1 P2P + if exc and not self.reconnect: logger.warning("Connection lost to {}:{} due to {}".format(self.dstaddr, self.dstport, exc)) else: logger.debug("Closed connection to: %s:%d" % (self.dstaddr, self.dstport)) @@ -221,13 +241,66 @@ class P2PConnection(asyncio.Protocol): self.recvbuf = b"" self.on_close() + # v2 handshake method + def _on_data_v2_handshake(self): + """v2 handshake performed before P2P messages are exchanged (see BIP324). P2PConnection is the initiator + (in inbound connections to TestNode) and the responder (in outbound connections from TestNode). + Performed by: + * initiator using `initiate_v2_handshake()`, `complete_handshake()` and `authenticate_handshake()` + * responder using `respond_v2_handshake()`, `complete_handshake()` and `authenticate_handshake()` + + `initiate_v2_handshake()` is immediately done by the initiator when the connection is established in + `connection_made()`. The rest of the initial v2 handshake functions are handled here. + """ + if not self.v2_state.peer: + if not self.v2_state.initiating and not self.v2_state.sent_garbage: + # if the responder hasn't sent garbage yet, the responder is still reading ellswift bytes + # reads ellswift bytes till the first mismatch from 12 bytes V1_PREFIX + length, send_handshake_bytes = self.v2_state.respond_v2_handshake(BytesIO(self.recvbuf)) + self.recvbuf = self.recvbuf[length:] + if send_handshake_bytes == -1: + self.v2_state = None + return + elif send_handshake_bytes: + self.send_raw_message(send_handshake_bytes) + elif send_handshake_bytes == b"": + return # only after send_handshake_bytes are sent can `complete_handshake()` be done + + # `complete_handshake()` reads the remaining ellswift bytes from recvbuf + # and sends response after deriving shared ECDH secret using received ellswift bytes + length, response = self.v2_state.complete_handshake(BytesIO(self.recvbuf)) + self.recvbuf = self.recvbuf[length:] + if response: + self.send_raw_message(response) + else: + return # only after response is sent can `authenticate_handshake()` be done + + # `self.v2_state.peer` is instantiated only after shared ECDH secret/BIP324 derived keys and ciphers + # is derived in `complete_handshake()`. + # so `authenticate_handshake()` which uses the BIP324 derived ciphers gets called after `complete_handshake()`. + assert self.v2_state.peer + length, is_mac_auth = self.v2_state.authenticate_handshake(self.recvbuf) + if not is_mac_auth: + raise ValueError("invalid v2 mac tag in handshake authentication") + self.recvbuf = self.recvbuf[length:] + if self.v2_state.tried_v2_handshake: + # for v2 outbound connections, send version message immediately after v2 handshake + if self.p2p_connected_to_node: + self.send_version() + # process post-v2-handshake data immediately, if available + if len(self.recvbuf) > 0: + self._on_data() + # Socket read methods def data_received(self, t): """asyncio callback when data is read from the socket.""" if len(t) > 0: self.recvbuf += t - self._on_data() + if self.supports_v2_p2p and not self.v2_state.tried_v2_handshake: + self._on_data_v2_handshake() + else: + self._on_data() def _on_data(self): """Try to read P2P messages from the recv buffer. @@ -237,23 +310,48 @@ class P2PConnection(asyncio.Protocol): the on_message callback for processing.""" try: while True: - if len(self.recvbuf) < 4: - return - if self.recvbuf[:4] != self.magic_bytes: - raise ValueError("magic bytes mismatch: {} != {}".format(repr(self.magic_bytes), repr(self.recvbuf))) - if len(self.recvbuf) < 4 + 12 + 4 + 4: - return - msgtype = self.recvbuf[4:4+12].split(b"\x00", 1)[0] - msglen = struct.unpack("<i", self.recvbuf[4+12:4+12+4])[0] - checksum = self.recvbuf[4+12+4:4+12+4+4] - if len(self.recvbuf) < 4 + 12 + 4 + 4 + msglen: - return - msg = self.recvbuf[4+12+4+4:4+12+4+4+msglen] - th = sha256(msg) - h = sha256(th) - if checksum != h[:4]: - raise ValueError("got bad checksum " + repr(self.recvbuf)) - self.recvbuf = self.recvbuf[4+12+4+4+msglen:] + if self.supports_v2_p2p: + # v2 P2P messages are read + msglen, msg = self.v2_state.v2_receive_packet(self.recvbuf) + if msglen == -1: + raise ValueError("invalid v2 mac tag " + repr(self.recvbuf)) + elif msglen == 0: # need to receive more bytes in recvbuf + return + self.recvbuf = self.recvbuf[msglen:] + + if msg is None: # ignore decoy messages + return + assert msg # application layer messages (which aren't decoy messages) are non-empty + shortid = msg[0] # 1-byte short message type ID + if shortid == 0: + # next 12 bytes are interpreted as ASCII message type if shortid is b'\x00' + if len(msg) < 13: + raise IndexError("msg needs minimum required length of 13 bytes") + msgtype = msg[1:13].rstrip(b'\x00') + msg = msg[13:] # msg is set to be payload + else: + # a 1-byte short message type ID + msgtype = SHORTID.get(shortid, f"unknown-{shortid}") + msg = msg[1:] + else: + # v1 P2P messages are read + if len(self.recvbuf) < 4: + return + if self.recvbuf[:4] != self.magic_bytes: + raise ValueError("magic bytes mismatch: {} != {}".format(repr(self.magic_bytes), repr(self.recvbuf))) + if len(self.recvbuf) < 4 + 12 + 4 + 4: + return + msgtype = self.recvbuf[4:4+12].split(b"\x00", 1)[0] + msglen = struct.unpack("<i", self.recvbuf[4+12:4+12+4])[0] + checksum = self.recvbuf[4+12+4:4+12+4+4] + if len(self.recvbuf) < 4 + 12 + 4 + 4 + msglen: + return + msg = self.recvbuf[4+12+4+4:4+12+4+4+msglen] + th = sha256(msg) + h = sha256(th) + if checksum != h[:4]: + raise ValueError("got bad checksum " + repr(self.recvbuf)) + self.recvbuf = self.recvbuf[4+12+4+4+msglen:] if msgtype not in MESSAGEMAP: raise ValueError("Received unknown msgtype from %s:%d: '%s' %s" % (self.dstaddr, self.dstport, msgtype, repr(msg))) f = BytesIO(msg) @@ -262,7 +360,8 @@ class P2PConnection(asyncio.Protocol): self._log_message("receive", t) self.on_message(t) except Exception as e: - logger.exception('Error reading message:', repr(e)) + if not self.reconnect: + logger.exception('Error reading message:', repr(e)) raise def on_message(self, message): @@ -271,14 +370,15 @@ class P2PConnection(asyncio.Protocol): # Socket write methods - def send_message(self, message): + def send_message(self, message, is_decoy=False): """Send a P2P message over the socket. This method takes a P2P payload, builds the P2P header and adds the message to the send buffer to be sent over the socket.""" - tmsg = self.build_message(message) - self._log_message("send", message) - return self.send_raw_message(tmsg) + with self._send_lock: + tmsg = self.build_message(message, is_decoy) + self._log_message("send", message) + return self.send_raw_message(tmsg) def send_raw_message(self, raw_message_bytes): if not self.is_connected: @@ -294,19 +394,29 @@ class P2PConnection(asyncio.Protocol): # Class utility methods - def build_message(self, message): + def build_message(self, message, is_decoy=False): """Build a serialized P2P message""" msgtype = message.msgtype data = message.serialize() - tmsg = self.magic_bytes - tmsg += msgtype - tmsg += b"\x00" * (12 - len(msgtype)) - tmsg += struct.pack("<I", len(data)) - th = sha256(data) - h = sha256(th) - tmsg += h[:4] - tmsg += data - return tmsg + if self.supports_v2_p2p: + if msgtype in SHORTID.values(): + tmsg = MSGTYPE_TO_SHORTID.get(msgtype).to_bytes(1, 'big') + else: + tmsg = b"\x00" + tmsg += msgtype + tmsg += b"\x00" * (12 - len(msgtype)) + tmsg += data + return self.v2_state.v2_enc_packet(tmsg, ignore=is_decoy) + else: + tmsg = self.magic_bytes + tmsg += msgtype + tmsg += b"\x00" * (12 - len(msgtype)) + tmsg += struct.pack("<I", len(data)) + th = sha256(data) + h = sha256(th) + tmsg += h[:4] + tmsg += data + return tmsg def _log_message(self, direction, msg): """Logs a message being sent or received over the connection.""" @@ -450,6 +560,11 @@ class P2PInterface(P2PConnection): def on_version(self, message): assert message.nVersion >= MIN_P2P_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_P2P_VERSION_SUPPORTED) + # for inbound connections, reply to version with own version message + # (could be due to v1 reconnect after a failed v2 handshake) + if not self.p2p_connected_to_node: + self.send_version() + self.reconnect = False if message.nVersion >= 70016 and self.wtxidrelay: self.send_message(msg_wtxidrelay()) if self.support_addrv2: @@ -478,6 +593,11 @@ class P2PInterface(P2PConnection): test_function = lambda: not self.is_connected self.wait_until(test_function, timeout=timeout, check_connected=False) + def wait_for_reconnect(self, timeout=60): + def test_function(): + return self.is_connected and self.last_message.get('version') and not self.supports_v2_p2p + self.wait_until(test_function, timeout=timeout, check_connected=False) + # Message receiving helper methods def wait_for_tx(self, txid, timeout=60): @@ -556,6 +676,11 @@ class P2PInterface(P2PConnection): # Message sending helper functions + def send_version(self): + if self.on_connection_send_msg: + self.send_message(self.on_connection_send_msg) + self.on_connection_send_msg = None # Never used again + def send_and_ping(self, message, timeout=60): self.send_message(message) self.sync_with_ping(timeout=timeout) @@ -622,6 +747,11 @@ class NetworkThread(threading.Thread): if addr is None: addr = '127.0.0.1' + def exception_handler(loop, context): + if not p2p.reconnect: + loop.default_exception_handler(context) + + cls.network_event_loop.set_exception_handler(exception_handler) coroutine = cls.create_listen_server(addr, port, callback, p2p) cls.network_event_loop.call_soon_threadsafe(cls.network_event_loop.create_task, coroutine) @@ -635,7 +765,9 @@ class NetworkThread(threading.Thread): protocol function from that dict, and returns it so the event loop can start executing it.""" response = cls.protos.get((addr, port)) - cls.protos[(addr, port)] = None + # remove protocol function from dict only when reconnection doesn't need to happen/already happened + if not proto.reconnect: + cls.protos[(addr, port)] = None return response if (addr, port) not in cls.listeners: @@ -708,7 +840,7 @@ class P2PDataStore(P2PInterface): if response is not None: self.send_message(response) - def send_blocks_and_test(self, blocks, node, *, success=True, force_send=False, reject_reason=None, expect_disconnect=False, timeout=60): + def send_blocks_and_test(self, blocks, node, *, success=True, force_send=False, reject_reason=None, expect_disconnect=False, timeout=60, is_decoy=False): """Send blocks to test node and test whether the tip advances. - add all blocks to our block_store @@ -727,9 +859,11 @@ class P2PDataStore(P2PInterface): reject_reason = [reject_reason] if reject_reason else [] with node.assert_debug_log(expected_msgs=reject_reason): + if is_decoy: # since decoy messages are ignored by the recipient - no need to wait for response + force_send = True if force_send: for b in blocks: - self.send_message(msg_block(block=b)) + self.send_message(msg_block(block=b), is_decoy) else: self.send_message(msg_headers([CBlockHeader(block) for block in blocks])) self.wait_until( diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 77f6e69e98..838dcba141 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -27,7 +27,8 @@ from .authproxy import ( serialization_fallback, ) from .descriptors import descsum_create -from .p2p import P2P_SUBVERSION +from .messages import NODE_P2P_V2 +from .p2p import P2P_SERVICES, P2P_SUBVERSION from .util import ( MAX_NODES, assert_equal, @@ -129,8 +130,15 @@ class TestNode(): # Default behavior from global -v2transport flag is added to args to persist it over restarts. # May be overwritten in individual tests, using extra_args. self.default_to_v2 = v2transport - if self.default_to_v2: - self.args.append("-v2transport=1") + if self.version_is_at_least(260000): + # 26.0 and later support v2transport + if v2transport: + self.args.append("-v2transport=1") + else: + self.args.append("-v2transport=0") + else: + # v2transport requested but not supported for node + assert not v2transport self.cli = TestNodeCLI(bitcoin_cli, self.datadir_path) self.use_cli = use_cli @@ -659,18 +667,30 @@ class TestNode(): assert_msg += "with expected error " + expected_msg self._raise_assertion_error(assert_msg) - def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, **kwargs): + def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, supports_v2_p2p=False, **kwargs): """Add an inbound p2p connection to the node. This method adds the p2p connection to the self.p2ps list and also - returns the connection to the caller.""" + returns the connection to the caller. + + When self.use_v2transport is True, TestNode advertises NODE_P2P_V2 service flag + + An inbound connection is made from TestNode <------ P2PConnection + - if TestNode doesn't advertise NODE_P2P_V2 service, P2PConnection sends version message and v1 P2P is followed + - if TestNode advertises NODE_P2P_V2 service, (and if P2PConnections supports v2 P2P) + P2PConnection sends ellswift bytes and v2 P2P is followed + """ if 'dstport' not in kwargs: kwargs['dstport'] = p2p_port(self.index) if 'dstaddr' not in kwargs: kwargs['dstaddr'] = '127.0.0.1' p2p_conn.p2p_connected_to_node = True - p2p_conn.peer_connect(**kwargs, send_version=send_version, net=self.chain, timeout_factor=self.timeout_factor)() + if self.use_v2transport: + kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2 + supports_v2_p2p = self.use_v2transport and supports_v2_p2p + p2p_conn.peer_connect(**kwargs, send_version=send_version, net=self.chain, timeout_factor=self.timeout_factor, supports_v2_p2p=supports_v2_p2p)() + self.p2ps.append(p2p_conn) p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False) if send_version: @@ -701,7 +721,7 @@ class TestNode(): return p2p_conn - def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", **kwargs): + def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=False, advertise_v2_p2p=False, **kwargs): """Add an outbound p2p connection from node. Must be an "outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection. @@ -711,14 +731,37 @@ class TestNode(): p2p_idx must be different for simultaneously connected peers. When reusing it for the next peer after disconnecting the previous one, it is necessary to wait for the disconnect to finish to avoid a race condition. + + Parameters: + supports_v2_p2p: whether p2p_conn supports v2 P2P or not + advertise_v2_p2p: whether p2p_conn is advertised to support v2 P2P or not + + An outbound connection is made from TestNode -------> P2PConnection + - if P2PConnection doesn't advertise_v2_p2p, TestNode sends version message and v1 P2P is followed + - if P2PConnection both supports_v2_p2p and advertise_v2_p2p, TestNode sends ellswift bytes and v2 P2P is followed + - if P2PConnection doesn't supports_v2_p2p but advertise_v2_p2p, + TestNode sends ellswift bytes and P2PConnection disconnects, + TestNode reconnects by sending version message and v1 P2P is followed """ def addconnection_callback(address, port): self.log.debug("Connecting to %s:%d %s" % (address, port, connection_type)) - self.addconnection('%s:%d' % (address, port), connection_type) + self.addconnection('%s:%d' % (address, port), connection_type, advertise_v2_p2p) p2p_conn.p2p_connected_to_node = False - p2p_conn.peer_accept_connection(connect_cb=addconnection_callback, connect_id=p2p_idx + 1, net=self.chain, timeout_factor=self.timeout_factor, **kwargs)() + if advertise_v2_p2p: + kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2 + assert self.use_v2transport # only a v2 TestNode could make a v2 outbound connection + + # if P2PConnection is advertised to support v2 P2P when it doesn't actually support v2 P2P, + # reconnection needs to be attempted using v1 P2P by sending version message + reconnect = advertise_v2_p2p and not supports_v2_p2p + # P2PConnection needs to be advertised to support v2 P2P so that ellswift bytes are sent instead of msg_version + supports_v2_p2p = supports_v2_p2p and advertise_v2_p2p + p2p_conn.peer_accept_connection(connect_cb=addconnection_callback, connect_id=p2p_idx + 1, net=self.chain, timeout_factor=self.timeout_factor, supports_v2_p2p=supports_v2_p2p, reconnect=reconnect, **kwargs)() + + if reconnect: + p2p_conn.wait_for_reconnect() if connection_type == "feeler": # feeler connections are closed as soon as the node receives a `version` message diff --git a/test/functional/test_framework/v2_p2p.py b/test/functional/test_framework/v2_p2p.py new file mode 100644 index 0000000000..8f79623bd8 --- /dev/null +++ b/test/functional/test_framework/v2_p2p.py @@ -0,0 +1,285 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Class for v2 P2P protocol (see BIP 324)""" + +import logging +import random + +from .crypto.bip324_cipher import FSChaCha20Poly1305 +from .crypto.chacha20 import FSChaCha20 +from .crypto.ellswift import ellswift_create, ellswift_ecdh_xonly +from .crypto.hkdf import hkdf_sha256 +from .key import TaggedHash +from .messages import MAGIC_BYTES + +logger = logging.getLogger("TestFramework.v2_p2p") + +CHACHA20POLY1305_EXPANSION = 16 +HEADER_LEN = 1 +IGNORE_BIT_POS = 7 +LENGTH_FIELD_LEN = 3 +MAX_GARBAGE_LEN = 4095 +TRANSPORT_VERSION = b'' + +SHORTID = { + 1: b"addr", + 2: b"block", + 3: b"blocktxn", + 4: b"cmpctblock", + 5: b"feefilter", + 6: b"filteradd", + 7: b"filterclear", + 8: b"filterload", + 9: b"getblocks", + 10: b"getblocktxn", + 11: b"getdata", + 12: b"getheaders", + 13: b"headers", + 14: b"inv", + 15: b"mempool", + 16: b"merkleblock", + 17: b"notfound", + 18: b"ping", + 19: b"pong", + 20: b"sendcmpct", + 21: b"tx", + 22: b"getcfilters", + 23: b"cfilter", + 24: b"getcfheaders", + 25: b"cfheaders", + 26: b"getcfcheckpt", + 27: b"cfcheckpt", + 28: b"addrv2", +} + +# Dictionary which contains short message type ID for the P2P message +MSGTYPE_TO_SHORTID = {msgtype: shortid for shortid, msgtype in SHORTID.items()} + + +class EncryptedP2PState: + """A class for managing the state when v2 P2P protocol is used. Performs initial v2 handshake and encrypts/decrypts + P2P messages. P2PConnection uses an object of this class. + + + Args: + initiating (bool): defines whether the P2PConnection is an initiator or responder. + - initiating = True for inbound connections in the test framework [TestNode <------- P2PConnection] + - initiating = False for outbound connections in the test framework [TestNode -------> P2PConnection] + + net (string): chain used (regtest, signet etc..) + + Methods: + perform an advanced form of diffie-hellman handshake to instantiate the encrypted transport. before exchanging + any P2P messages, 2 nodes perform this handshake in order to determine a shared secret that is unique to both + of them and use it to derive keys to encrypt/decrypt P2P messages. + - initial v2 handshakes is performed by: (see BIP324 section #overall-handshake-pseudocode) + 1. initiator using initiate_v2_handshake(), complete_handshake() and authenticate_handshake() + 2. responder using respond_v2_handshake(), complete_handshake() and authenticate_handshake() + - initialize_v2_transport() sets various BIP324 derived keys and ciphers. + + encrypt/decrypt v2 P2P messages using v2_enc_packet() and v2_receive_packet(). + """ + def __init__(self, *, initiating, net): + self.initiating = initiating # True if initiator + self.net = net + self.peer = {} # object with various BIP324 derived keys and ciphers + self.privkey_ours = None + self.ellswift_ours = None + self.sent_garbage = b"" + self.received_garbage = b"" + self.received_prefix = b"" # received ellswift bytes till the first mismatch from 16 bytes v1_prefix + self.tried_v2_handshake = False # True when the initial handshake is over + # stores length of packet contents to detect whether first 3 bytes (which contains length of packet contents) + # has been decrypted. set to -1 if decryption hasn't been done yet. + self.contents_len = -1 + self.found_garbage_terminator = False + + @staticmethod + def v2_ecdh(priv, ellswift_theirs, ellswift_ours, initiating): + """Compute BIP324 shared secret. + + Returns: + bytes - BIP324 shared secret + """ + ecdh_point_x32 = ellswift_ecdh_xonly(ellswift_theirs, priv) + if initiating: + # Initiating, place our public key encoding first. + return TaggedHash("bip324_ellswift_xonly_ecdh", ellswift_ours + ellswift_theirs + ecdh_point_x32) + else: + # Responding, place their public key encoding first. + return TaggedHash("bip324_ellswift_xonly_ecdh", ellswift_theirs + ellswift_ours + ecdh_point_x32) + + def generate_keypair_and_garbage(self): + """Generates ellswift keypair and 4095 bytes garbage at max""" + self.privkey_ours, self.ellswift_ours = ellswift_create() + garbage_len = random.randrange(MAX_GARBAGE_LEN + 1) + self.sent_garbage = random.randbytes(garbage_len) + logger.debug(f"sending {garbage_len} bytes of garbage data") + return self.ellswift_ours + self.sent_garbage + + def initiate_v2_handshake(self): + """Initiator begins the v2 handshake by sending its ellswift bytes and garbage + + Returns: + bytes - bytes to be sent to the peer when starting the v2 handshake as an initiator + """ + return self.generate_keypair_and_garbage() + + def respond_v2_handshake(self, response): + """Responder begins the v2 handshake by sending its ellswift bytes and garbage. However, the responder + sends this after having received at least one byte that mismatches 16-byte v1_prefix. + + Returns: + 1. int - length of bytes that were consumed so that recvbuf can be updated + 2. bytes - bytes to be sent to the peer when starting the v2 handshake as a responder. + - returns b"" if more bytes need to be received before we can respond and start the v2 handshake. + - returns -1 to downgrade the connection to v1 P2P. + """ + v1_prefix = MAGIC_BYTES[self.net] + b'version\x00\x00\x00\x00\x00' + while len(self.received_prefix) < 16: + byte = response.read(1) + # return b"" if we need to receive more bytes + if not byte: + return len(self.received_prefix), b"" + self.received_prefix += byte + if self.received_prefix[-1] != v1_prefix[len(self.received_prefix) - 1]: + return len(self.received_prefix), self.generate_keypair_and_garbage() + # return -1 to decide v1 only after all 16 bytes processed + return len(self.received_prefix), -1 + + def complete_handshake(self, response): + """ Instantiates the encrypted transport and + sends garbage terminator + optional decoy packets + transport version packet. + Done by both initiator and responder. + + Returns: + 1. int - length of bytes that were consumed. returns 0 if all 64 bytes from ellswift haven't been received yet. + 2. bytes - bytes to be sent to the peer when completing the v2 handshake + """ + ellswift_theirs = self.received_prefix + response.read(64 - len(self.received_prefix)) + # return b"" if we need to receive more bytes + if len(ellswift_theirs) != 64: + return 0, b"" + ecdh_secret = self.v2_ecdh(self.privkey_ours, ellswift_theirs, self.ellswift_ours, self.initiating) + self.initialize_v2_transport(ecdh_secret) + # Send garbage terminator + msg_to_send = self.peer['send_garbage_terminator'] + # Optionally send decoy packets after garbage terminator. + aad = self.sent_garbage + for decoy_content_len in [random.randint(1, 100) for _ in range(random.randint(0, 10))]: + msg_to_send += self.v2_enc_packet(decoy_content_len * b'\x00', aad=aad, ignore=True) + aad = b'' + # Send version packet. + msg_to_send += self.v2_enc_packet(TRANSPORT_VERSION, aad=aad) + return 64 - len(self.received_prefix), msg_to_send + + def authenticate_handshake(self, response): + """ Ensures that the received optional decoy packets and transport version packet are authenticated. + Marks the v2 handshake as complete. Done by both initiator and responder. + + Returns: + 1. int - length of bytes that were processed so that recvbuf can be updated + 2. bool - True if the authentication was successful/more bytes need to be received and False otherwise + """ + processed_length = 0 + + # Detect garbage terminator in the received bytes + if not self.found_garbage_terminator: + received_garbage = response[:16] + response = response[16:] + processed_length = len(received_garbage) + for i in range(MAX_GARBAGE_LEN + 1): + if received_garbage[-16:] == self.peer['recv_garbage_terminator']: + # Receive, decode, and ignore version packet. + # This includes skipping decoys and authenticating the received garbage. + self.found_garbage_terminator = True + self.received_garbage = received_garbage[:-16] + break + else: + # don't update recvbuf since more bytes need to be received + if len(response) == 0: + return 0, True + received_garbage += response[:1] + processed_length += 1 + response = response[1:] + else: + # disconnect since garbage terminator was not seen after 4 KiB of garbage. + return processed_length, False + + # Process optional decoy packets and transport version packet + while not self.tried_v2_handshake: + length, contents = self.v2_receive_packet(response, aad=self.received_garbage) + if length == -1: + return processed_length, False + elif length == 0: + return processed_length, True + processed_length += length + self.received_garbage = b"" + # decoy packets have contents = None. v2 handshake is complete only when version packet + # (can be empty with contents = b"") with contents != None is received. + if contents is not None: + assert contents == b"" # currently TestNode sends an empty version packet + self.tried_v2_handshake = True + return processed_length, True + response = response[length:] + + def initialize_v2_transport(self, ecdh_secret): + """Sets the peer object with various BIP324 derived keys and ciphers.""" + peer = {} + salt = b'bitcoin_v2_shared_secret' + MAGIC_BYTES[self.net] + for name in ('initiator_L', 'initiator_P', 'responder_L', 'responder_P', 'garbage_terminators', 'session_id'): + peer[name] = hkdf_sha256(salt=salt, ikm=ecdh_secret, info=name.encode('utf-8'), length=32) + if self.initiating: + self.peer['send_L'] = FSChaCha20(peer['initiator_L']) + self.peer['send_P'] = FSChaCha20Poly1305(peer['initiator_P']) + self.peer['send_garbage_terminator'] = peer['garbage_terminators'][:16] + self.peer['recv_L'] = FSChaCha20(peer['responder_L']) + self.peer['recv_P'] = FSChaCha20Poly1305(peer['responder_P']) + self.peer['recv_garbage_terminator'] = peer['garbage_terminators'][16:] + else: + self.peer['send_L'] = FSChaCha20(peer['responder_L']) + self.peer['send_P'] = FSChaCha20Poly1305(peer['responder_P']) + self.peer['send_garbage_terminator'] = peer['garbage_terminators'][16:] + self.peer['recv_L'] = FSChaCha20(peer['initiator_L']) + self.peer['recv_P'] = FSChaCha20Poly1305(peer['initiator_P']) + self.peer['recv_garbage_terminator'] = peer['garbage_terminators'][:16] + self.peer['session_id'] = peer['session_id'] + + def v2_enc_packet(self, contents, aad=b'', ignore=False): + """Encrypt a BIP324 packet. + + Returns: + bytes - encrypted packet contents + """ + assert len(contents) <= 2**24 - 1 + header = (ignore << IGNORE_BIT_POS).to_bytes(HEADER_LEN, 'little') + plaintext = header + contents + aead_ciphertext = self.peer['send_P'].encrypt(aad, plaintext) + enc_plaintext_len = self.peer['send_L'].crypt(len(contents).to_bytes(LENGTH_FIELD_LEN, 'little')) + return enc_plaintext_len + aead_ciphertext + + def v2_receive_packet(self, response, aad=b''): + """Decrypt a BIP324 packet + + Returns: + 1. int - number of bytes consumed (or -1 if error) + 2. bytes - contents of decrypted non-decoy packet if any (or None otherwise) + """ + if self.contents_len == -1: + if len(response) < LENGTH_FIELD_LEN: + return 0, None + enc_contents_len = response[:LENGTH_FIELD_LEN] + self.contents_len = int.from_bytes(self.peer['recv_L'].crypt(enc_contents_len), 'little') + response = response[LENGTH_FIELD_LEN:] + if len(response) < HEADER_LEN + self.contents_len + CHACHA20POLY1305_EXPANSION: + return 0, None + aead_ciphertext = response[:HEADER_LEN + self.contents_len + CHACHA20POLY1305_EXPANSION] + plaintext = self.peer['recv_P'].decrypt(aad, aead_ciphertext) + if plaintext is None: + return -1, None # disconnect + header = plaintext[:HEADER_LEN] + length = LENGTH_FIELD_LEN + HEADER_LEN + self.contents_len + CHACHA20POLY1305_EXPANSION + self.contents_len = -1 + return length, None if (header[0] & (1 << IGNORE_BIT_POS)) else plaintext[HEADER_LEN:] diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 9c8f0eca26..9bfc9aa7d4 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -262,6 +262,8 @@ BASE_SCRIPTS = [ 'p2p_invalid_tx.py', 'p2p_invalid_tx.py --v2transport', 'p2p_v2_transport.py', + 'p2p_v2_encrypted.py', + 'p2p_v2_earlykeyresponse.py', 'example_test.py', 'wallet_txn_doublespend.py --legacy-wallet', 'wallet_multisig_descriptor_psbt.py --descriptors', @@ -333,6 +335,8 @@ BASE_SCRIPTS = [ 'wallet_send.py --descriptors', 'wallet_sendall.py --legacy-wallet', 'wallet_sendall.py --descriptors', + 'wallet_sendmany.py --descriptors', + 'wallet_sendmany.py --legacy-wallet', 'wallet_create_tx.py --descriptors', 'wallet_inactive_hdchains.py --legacy-wallet', 'wallet_spend_unconfirmed.py', diff --git a/test/functional/wallet_assumeutxo.py b/test/functional/wallet_assumeutxo.py index fce4868293..3c1a997bd1 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -16,6 +16,7 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, ) +from test_framework.wallet import MiniWallet START_HEIGHT = 199 SNAPSHOT_BASE_HEIGHT = 299 @@ -55,6 +56,8 @@ class AssumeutxoTest(BitcoinTestFramework): n0 = self.nodes[0] n1 = self.nodes[1] + self.mini_wallet = MiniWallet(n0) + # Mock time for a deterministic chain for n in self.nodes: n.setmocktime(n.getblockheader(n.getbestblockhash())['time']) @@ -69,7 +72,9 @@ class AssumeutxoTest(BitcoinTestFramework): # though, we have to ferry over the new headers to n1 so that it # isn't waiting forever to see the header of the snapshot's base block # while disconnected from n0. - for _ in range(100): + for i in range(100): + if i % 3 == 0: + self.mini_wallet.send_self_transfer(from_node=n0) self.generate(n0, nblocks=1, sync_fun=self.no_op) newblock = n0.getblock(n0.getbestblockhash(), 0) @@ -94,8 +99,8 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal( dump_output['txoutset_hash'], - '61d9c2b29a2571a5fe285fe2d8554f91f93309666fc9b8223ee96338de25ff53') - assert_equal(dump_output['nchaintx'], 300) + "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27") + assert_equal(dump_output["nchaintx"], 334) assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT) # Mine more blocks on top of the snapshot that n1 hasn't yet seen. This diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index a331ba997e..d886a59ac1 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -8,10 +8,13 @@ from decimal import Decimal from itertools import product from math import ceil +from test_framework.address import address_to_scriptpubkey from test_framework.descriptors import descsum_create from test_framework.messages import ( COIN, + CTransaction, + CTxOut, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -147,6 +150,34 @@ class RawTransactionsTest(BitcoinTestFramework): self.test_22670() self.test_feerate_rounding() self.test_input_confs_control() + self.test_duplicate_outputs() + + def test_duplicate_outputs(self): + self.log.info("Test deserializing and funding a transaction with duplicate outputs") + self.nodes[1].createwallet("fundtx_duplicate_outputs") + w = self.nodes[1].get_wallet_rpc("fundtx_duplicate_outputs") + + addr = w.getnewaddress(address_type="bech32") + self.nodes[0].sendtoaddress(addr, 5) + self.generate(self.nodes[0], 1) + + address = self.nodes[0].getnewaddress("bech32") + tx = CTransaction() + tx.vin = [] + tx.vout = [CTxOut(1 * COIN, bytearray(address_to_scriptpubkey(address)))] * 2 + tx.nLockTime = 0 + tx_hex = tx.serialize().hex() + res = w.fundrawtransaction(tx_hex, add_inputs=True) + signed_res = w.signrawtransactionwithwallet(res["hex"]) + txid = w.sendrawtransaction(signed_res["hex"]) + assert self.nodes[1].getrawtransaction(txid) + + self.log.info("Test SFFO with duplicate outputs") + + res_sffo = w.fundrawtransaction(tx_hex, add_inputs=True, subtractFeeFromOutputs=[0,1]) + signed_res_sffo = w.signrawtransactionwithwallet(res_sffo["hex"]) + txid_sffo = w.sendrawtransaction(signed_res_sffo["hex"]) + assert self.nodes[1].getrawtransaction(txid_sffo) def test_change_position(self): """Ensure setting changePosition in fundraw with an exact match is handled properly.""" diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 7f01d23941..e647fb2d5c 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -24,6 +24,7 @@ from test_framework.address import ( AddressType, ADDRESS_BCRT1_UNSPENDABLE, ) +from test_framework.messages import COIN from test_framework.util import ( assert_equal, set_node_times, @@ -144,8 +145,10 @@ TIMESTAMP_WINDOW = 2 * 60 * 60 AMOUNT_DUST = 0.00000546 -def get_rand_amount(): - r = random.uniform(AMOUNT_DUST, 1) +def get_rand_amount(min_amount=AMOUNT_DUST): + assert min_amount <= 1 + r = random.uniform(min_amount, 1) + # note: min_amount can get rounded down here return Decimal(str(round(r, 8))) @@ -270,7 +273,9 @@ class ImportRescanTest(BitcoinTestFramework): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() * 2 + # Ensure output is large enough to pay for fees: conservatively assuming txsize of + # 500 vbytes and feerate of 20 sats/vbytes + variant.initial_amount = get_rand_amount(min_amount=((500 * 20 / COIN) + AMOUNT_DUST)) variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.confirmation_height = 0 variant.timestamp = timestamp diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 3d68dbe07a..f9919716be 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -52,15 +52,26 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(file_magic, b'SQLite format 3\x00') assert_equal(self.nodes[0].get_wallet_rpc(wallet_name).getwalletinfo()["format"], "sqlite") - def create_legacy_wallet(self, wallet_name, disable_private_keys=False): - self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, disable_private_keys=disable_private_keys) + def create_legacy_wallet(self, wallet_name, **kwargs): + self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, **kwargs) wallet = self.nodes[0].get_wallet_rpc(wallet_name) info = wallet.getwalletinfo() assert_equal(info["descriptors"], False) assert_equal(info["format"], "bdb") - assert_equal(info["private_keys_enabled"], not disable_private_keys) return wallet + def migrate_wallet(self, wallet_rpc, *args, **kwargs): + # Helper to ensure that only migration happens + # Since we may rescan on loading of a wallet, make sure that the best block + # is written before beginning migration + # Reload to force write that record + wallet_name = wallet_rpc.getwalletinfo()["walletname"] + wallet_rpc.unloadwallet() + self.nodes[0].loadwallet(wallet_name) + # Migrate, checking that rescan does not occur + with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=["Rescanning"]): + return wallet_rpc.migratewallet(*args, **kwargs) + def assert_addr_info_equal(self, addr_info, addr_info_old): assert_equal(addr_info["address"], addr_info_old["address"]) assert_equal(addr_info["scriptPubKey"], addr_info_old["scriptPubKey"]) @@ -105,7 +116,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(old_change_addr_info["hdkeypath"], "m/0'/1'/0'") # Note: migration could take a while. - basic0.migratewallet() + self.migrate_wallet(basic0) # Verify created descriptors assert_equal(basic0.getwalletinfo()["descriptors"], True) @@ -146,7 +157,7 @@ class WalletMigrationTest(BitcoinTestFramework): txs = basic1.listtransactions() addr_gps = basic1.listaddressgroupings() - basic1_migrate = basic1.migratewallet() + basic1_migrate = self.migrate_wallet(basic1) assert_equal(basic1.getwalletinfo()["descriptors"], True) self.assert_is_sqlite("basic1") assert_equal(basic1.getbalance(), bal) @@ -187,7 +198,7 @@ class WalletMigrationTest(BitcoinTestFramework): basic2_txs = basic2.listtransactions() # Now migrate and test that we still see have the same balance/transactions - basic2.migratewallet() + self.migrate_wallet(basic2) assert_equal(basic2.getwalletinfo()["descriptors"], True) self.assert_is_sqlite("basic2") assert_equal(basic2.getbalance(), basic2_balance) @@ -209,7 +220,7 @@ class WalletMigrationTest(BitcoinTestFramework): ms_info = multisig0.addmultisigaddress(2, [addr1, addr2, addr3]) - multisig0.migratewallet() + self.migrate_wallet(multisig0) assert_equal(multisig0.getwalletinfo()["descriptors"], True) self.assert_is_sqlite("multisig0") ms_addr_info = multisig0.getaddressinfo(ms_info["address"]) @@ -244,7 +255,7 @@ class WalletMigrationTest(BitcoinTestFramework): # Migrating multisig1 should see the multisig is no longer part of multisig1 # A new wallet multisig1_watchonly is created which has the multisig address # Transaction to multisig is in multisig1_watchonly and not multisig1 - multisig1.migratewallet() + self.migrate_wallet(multisig1) assert_equal(multisig1.getwalletinfo()["descriptors"], True) self.assert_is_sqlite("multisig1") assert_equal(multisig1.getaddressinfo(addr1)["ismine"], False) @@ -314,27 +325,31 @@ class WalletMigrationTest(BitcoinTestFramework): send = default.sendall(recipients=[default.getnewaddress()], inputs=[received_sent_watchonly_utxo]) sent_watchonly_txid = send["txid"] - self.generate(self.nodes[0], 1) + # Tx that has both a watchonly and spendable output + watchonly_spendable_txid = default.send(outputs=[{received_addr: 1}, {import_addr:1}])["txid"] + + self.generate(self.nodes[0], 2) received_watchonly_tx_info = imports0.gettransaction(received_watchonly_txid, True) received_sent_watchonly_tx_info = imports0.gettransaction(received_sent_watchonly_utxo["txid"], True) balances = imports0.getbalances() spendable_bal = balances["mine"]["trusted"] watchonly_bal = balances["watchonly"]["trusted"] - assert_equal(len(imports0.listtransactions(include_watchonly=True)), 4) + assert_equal(len(imports0.listtransactions(include_watchonly=True)), 6) # Mock time forward a bit so we can check that tx metadata is preserved self.nodes[0].setmocktime(int(time.time()) + 100) # Migrate - imports0.migratewallet() + self.migrate_wallet(imports0) assert_equal(imports0.getwalletinfo()["descriptors"], True) self.assert_is_sqlite("imports0") assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_watchonly_txid) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, received_sent_watchonly_utxo['txid']) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", imports0.gettransaction, sent_watchonly_txid) - assert_equal(len(imports0.listtransactions(include_watchonly=True)), 1) + assert_equal(len(imports0.listtransactions(include_watchonly=True)), 2) imports0.gettransaction(received_txid) + imports0.gettransaction(watchonly_spendable_txid) assert_equal(imports0.getbalance(), spendable_bal) assert_equal("imports0_watchonly" in self.nodes[0].listwallets(), True) @@ -350,9 +365,10 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(received_sent_watchonly_tx_info["time"], received_sent_migrated_watchonly_tx_info["time"]) assert_equal(received_sent_watchonly_tx_info["timereceived"], received_sent_migrated_watchonly_tx_info["timereceived"]) watchonly.gettransaction(sent_watchonly_txid) + watchonly.gettransaction(watchonly_spendable_txid) assert_equal(watchonly.getbalance(), watchonly_bal) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", watchonly.gettransaction, received_txid) - assert_equal(len(watchonly.listtransactions(include_watchonly=True)), 3) + assert_equal(len(watchonly.listtransactions(include_watchonly=True)), 4) # Check that labels were migrated and persisted to watchonly wallet self.nodes[0].unloadwallet("imports0_watchonly") @@ -380,7 +396,7 @@ class WalletMigrationTest(BitcoinTestFramework): default.sendtoaddress(addr, 10) self.generate(self.nodes[0], 1) - watchonly0.migratewallet() + self.migrate_wallet(watchonly0) assert_equal("watchonly0_watchonly" in self.nodes[0].listwallets(), False) info = watchonly0.getwalletinfo() assert_equal(info["descriptors"], True) @@ -412,7 +428,7 @@ class WalletMigrationTest(BitcoinTestFramework): # Before migrating, we can fetch addr1 from the keypool assert_equal(watchonly1.getnewaddress(address_type="bech32"), addr1) - watchonly1.migratewallet() + self.migrate_wallet(watchonly1) info = watchonly1.getwalletinfo() assert_equal(info["descriptors"], True) assert_equal(info["private_keys_enabled"], False) @@ -432,7 +448,7 @@ class WalletMigrationTest(BitcoinTestFramework): bals = wallet.getbalances() - wallet.migratewallet() + self.migrate_wallet(wallet) assert_equal(bals, wallet.getbalances()) @@ -451,7 +467,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_raises_rpc_error(-4, "Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect", wallet.migratewallet, None, "badpass") assert_raises_rpc_error(-4, "The passphrase contains a null character", wallet.migratewallet, None, "pass\0with\0null") - wallet.migratewallet(passphrase="pass") + self.migrate_wallet(wallet, passphrase="pass") info = wallet.getwalletinfo() assert_equal(info["descriptors"], True) @@ -513,7 +529,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.log.info("Test migration of the wallet named as the empty string") wallet = self.create_legacy_wallet("") - wallet.migratewallet() + self.migrate_wallet(wallet) info = wallet.getwalletinfo() assert_equal(info["descriptors"], True) assert_equal(info["format"], "sqlite") @@ -535,7 +551,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(info["descriptors"], False) assert_equal(info["format"], "bdb") - wallet.migratewallet() + self.migrate_wallet(wallet) info = wallet.getwalletinfo() assert_equal(info["descriptors"], True) assert_equal(info["format"], "sqlite") @@ -623,7 +639,7 @@ class WalletMigrationTest(BitcoinTestFramework): check(addr_info, wallet) # Migrate wallet - info_migration = wallet.migratewallet() + info_migration = self.migrate_wallet(wallet) wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"]) wallet_solvables = self.nodes[0].get_wallet_rpc(info_migration["solvables_name"]) @@ -718,7 +734,7 @@ class WalletMigrationTest(BitcoinTestFramework): wallet.rpc.importaddress(address=script_sh_pkh.hex(), label=label_sh_pkh, rescan=False, p2sh=True) # Migrate wallet and re-check balance - info_migration = wallet.migratewallet() + info_migration = self.migrate_wallet(wallet) wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"]) # Watch-only balance is under "mine". @@ -781,7 +797,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(wallet.gettransaction(txid=child_txid)["confirmations"], -1) assert_equal(wallet.gettransaction(txid=conflict_txid)["confirmations"], 1) - wallet.migratewallet() + self.migrate_wallet(wallet) assert_equal(wallet.gettransaction(txid=parent_txid)["confirmations"], -1) assert_equal(wallet.gettransaction(txid=child_txid)["confirmations"], -1) assert_equal(wallet.gettransaction(txid=conflict_txid)["confirmations"], 1) @@ -814,7 +830,7 @@ class WalletMigrationTest(BitcoinTestFramework): p2wpkh_addr = key_to_p2wpkh(hybrid_pubkey) wallet.importaddress(p2wpkh_addr) - migrate_info = wallet.migratewallet() + migrate_info = self.migrate_wallet(wallet) # Both addresses should only appear in the watchonly wallet p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr) @@ -876,6 +892,13 @@ class WalletMigrationTest(BitcoinTestFramework): _, _, magic = struct.unpack("QII", data) assert_equal(magic, BTREE_MAGIC) + def test_blank(self): + self.log.info("Test that a blank wallet is migrated") + wallet = self.create_legacy_wallet("blank", blank=True) + assert_equal(wallet.getwalletinfo()["blank"], True) + wallet.migratewallet() + assert_equal(wallet.getwalletinfo()["blank"], True) + assert_equal(wallet.getwalletinfo()["descriptors"], True) def test_avoidreuse(self): self.log.info("Test that avoidreuse persists after migration") @@ -987,6 +1010,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_failed_migration_cleanup() self.test_avoidreuse() self.test_preserve_tx_extra_info() + self.test_blank() if __name__ == '__main__': WalletMigrationTest().main() diff --git a/test/functional/wallet_sendmany.py b/test/functional/wallet_sendmany.py new file mode 100755 index 0000000000..5751993143 --- /dev/null +++ b/test/functional/wallet_sendmany.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the sendmany RPC command.""" + +from test_framework.test_framework import BitcoinTestFramework + +class SendmanyTest(BitcoinTestFramework): + # Setup and helpers + def add_options(self, parser): + self.add_wallet_options(parser) + + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + + def set_test_params(self): + self.num_nodes = 1 + self.setup_clean_chain = True + + def test_sffo_repeated_address(self): + addr_1 = self.wallet.getnewaddress() + addr_2 = self.wallet.getnewaddress() + addr_3 = self.wallet.getnewaddress() + + self.log.info("Test using duplicate address in SFFO argument") + self.def_wallet.sendmany(dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[addr_1, addr_1, addr_1]) + self.log.info("Test using address not present in tx.vout in SFFO argument") + self.def_wallet.sendmany(dummy='', amounts={addr_1: 1, addr_2: 1}, subtractfeefrom=[addr_3]) + + def run_test(self): + self.nodes[0].createwallet("activewallet") + self.wallet = self.nodes[0].get_wallet_rpc("activewallet") + self.def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.generate(self.nodes[0], 101) + + self.test_sffo_repeated_address() + + +if __name__ == '__main__': + SendmanyTest().main() diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index e72977fac0..4e24c07699 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -24,6 +24,7 @@ def get_fuzz_env(*, target, source_dir): 'UBSAN_SYMBOLIZER_PATH':symbolizer, "ASAN_OPTIONS": "detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1", 'ASAN_SYMBOLIZER_PATH':symbolizer, + 'MSAN_SYMBOLIZER_PATH':symbolizer, } @@ -347,13 +348,18 @@ def run_once(*, fuzz_pool, corpus, test_list, src_dir, build_dir, using_libfuzze text=True, ) output += result.stderr - return output, result + return output, result, t jobs.append(fuzz_pool.submit(job, t, args)) + stats = [] for future in as_completed(jobs): - output, result = future.result() + output, result, target = future.result() logging.debug(output) + if using_libfuzzer: + done_stat = [l for l in output.splitlines() if "DONE" in l] + assert len(done_stat) == 1 + stats.append((target, done_stat[0])) try: result.check_returncode() except subprocess.CalledProcessError as e: @@ -364,6 +370,13 @@ def run_once(*, fuzz_pool, corpus, test_list, src_dir, build_dir, using_libfuzze logging.info(f"Target {result.args} failed with exit code {e.returncode}") sys.exit(1) + if using_libfuzzer: + print("Summary:") + max_len = max(len(t[0]) for t in stats) + for t, s in sorted(stats): + t = t.ljust(max_len + 1) + print(f"{t}{s}") + def parse_test_list(*, fuzz_bin): test_list_all = subprocess.run( @@ -372,8 +385,8 @@ def parse_test_list(*, fuzz_bin): 'PRINT_ALL_FUZZ_TARGETS_AND_ABORT': '' }, stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, text=True, + check=True, ).stdout.splitlines() return test_list_all diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index b22caa15a5..dadbe8c4f6 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -23,6 +23,7 @@ implicit-integer-sign-change:secp256k1/ implicit-signed-integer-truncation:*/include/c++/ implicit-signed-integer-truncation:leveldb/ implicit-signed-integer-truncation:secp256k1/ +implicit-signed-integer-truncation,implicit-integer-sign-change:secp256k1_modinv64_posdivsteps_62_var implicit-unsigned-integer-truncation:*/include/c++/ implicit-unsigned-integer-truncation:leveldb/ implicit-unsigned-integer-truncation:secp256k1/ |