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 @@ - + 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_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/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/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/src/Makefile.am b/src/Makefile.am index 9e77b265c6..a50d634c26 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) @@ -546,6 +550,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 @@ -586,9 +594,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 @@ -909,7 +920,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 @@ -1017,7 +1028,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 @@ -1095,6 +1106,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 1a1eb6eae9..78abe81b52 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -218,6 +218,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) @@ -225,6 +258,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/common/settings.cpp b/src/common/settings.cpp index 5761e8b321..913ad6d76a 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -4,6 +4,10 @@ #include +#if defined(HAVE_CONFIG_H) +#include +#endif + #include #include #include @@ -81,7 +85,9 @@ bool ReadSettings(const fs::path& path, std::map& va SettingsValue in; if (!in.read(std::string{std::istreambuf_iterator(file), std::istreambuf_iterator()})) { - 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; } @@ -114,6 +120,13 @@ bool WriteSettings(const fs::path& path, std::vector& errors) { SettingsValue out(SettingsValue::VOBJ); + // Add auto-generated warning comment only if it does not exist + if (!values.contains("_warning_")) { + out.pushKV("_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)); + } + // 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 #include +#if !defined(DISABLE_OPTIMIZED_SHA256) #include -#if defined(__linux__) && defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(__linux__) && defined(ENABLE_ARM_SHANI) #include #include #endif -#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) && !defined(BUILD_BITCOIN_INTERNAL) +#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) #include #include #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; @@ -633,13 +638,13 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem TransformD64 = TransformD64Wrapper; 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/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 +#include +#include + +#include +#include +#include +#include + +namespace ipc { +namespace capnp { +//! Use SFINAE to define Serializeable trait which is true if type T has a +//! Serialize(stream) method, false otherwise. +template +struct Serializable { +private: + template + static std::true_type test(decltype(std::declval().Serialize(std::declval()))*); + template + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test(nullptr))::value; +}; + +//! Use SFINAE to define Unserializeable trait which is true if type T has +//! an Unserialize(stream) method, false otherwise. +template +struct Unserializable { +private: + template + static std::true_type test(decltype(std::declval().Unserialize(std::declval()))*); + template + static std::false_type test(...); + +public: + static constexpr bool value = decltype(test(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 +void CustomBuildField( + TypeList, 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::value && + std::is_same_v>>>* 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 +decltype(auto) +CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, ReadDest&& read_dest, + std::enable_if_t::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 +void CustomBuildField(TypeList, 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 +decltype(auto) CustomReadField(TypeList, 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/node/blockstorage.cpp b/src/node/blockstorage.cpp index af34f74753..deb90b140b 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -308,6 +308,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; @@ -330,10 +331,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/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(file), std::istreambuf_iterator()).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 #include -#include // for HWND +#include #include diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index f6d9d42f0f..04d2e68939 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -353,17 +353,15 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo } return o; } else { + UniValue a(UniValue::VARR); uint64_t mempool_sequence; - std::vector vtxid; { LOCK(pool.cs); - pool.queryHashes(vtxid); + for (const CTxMemPoolEntry& e : pool.entryAll()) { + a.push_back(e.GetTx().GetHash().ToString()); + } mempool_sequence = pool.GetSequence(); } - UniValue a(UniValue::VARR); - for (const uint256& hash : vtxid) - a.push_back(hash.ToString()); - if (!include_mempool_sequence) { return a; } else { 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> ParseOutputs(const UniValue& outputs) +{ // Duplicate checking std::set destinations; + std::vector> 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 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> 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 +#include #include #include #include @@ -38,11 +40,16 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const */ void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map& 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> 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/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 4ad0956201..8c0b0d7029 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -101,9 +101,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, Cha if (!info_all.empty()) { const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, MemPoolRemovalReason::BLOCK /* dummy */)); - std::vector all_txids; - tx_pool.queryHashes(all_txids); - assert(all_txids.size() < info_all.size()); + assert(tx_pool.size() < info_all.size()); WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1)); } SyncWithValidationInterfaceQueue(); 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 +#include +#include +#include +#include + +#include +#include +#include +#include + +#include + +//! 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>> foo_promise; + std::function 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(loop, kj::mv(pipe.ends[0])); + auto foo_client = std::make_unique>( + connection_client->m_rpc_system.bootstrap(mp::ServerVatId().vat_id).castAs(), + 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(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) { + auto foo_server = kj::heap>(std::make_shared(), connection); + return capnp::Capability::Client(kj::mv(foo_server)); + }); + connection_server->onDisconnect([&] { connection_server.reset(); }); + loop.loop(); + }); + std::unique_ptr> 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 +#include + +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 +#include + +BOOST_AUTO_TEST_SUITE(ipc_tests) +BOOST_AUTO_TEST_CASE(ipc_tests) +{ + IpcTest(); +} +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 fail_parse = {strprintf("Unable to parse settings file %s", fs::PathToString(path))}; + std::vector 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/txmempool.cpp b/src/txmempool.cpp index acee56fe78..57a86549d9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -803,19 +803,6 @@ std::vector CTxMemPool::Get return iters; } -void CTxMemPool::queryHashes(std::vector& vtxid) const -{ - LOCK(cs); - auto iters = GetSortedDepthAndScore(); - - vtxid.clear(); - vtxid.reserve(mapTx.size()); - - for (auto it : iters) { - vtxid.push_back(it->GetTx().GetHash()); - } -} - static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) { return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; } diff --git a/src/txmempool.h b/src/txmempool.h index 9da51756e6..ca842632da 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -485,7 +485,6 @@ class CTxMemPool void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid=false); - void queryHashes(std::vector& vtxid) const; bool isSpent(const COutPoint& outpoint) const; unsigned int GetTransactionsUpdated() const; void AddTransactionsUpdated(unsigned int n); 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& recipients) +std::vector CreateRecipients(const std::vector>& outputs, const std::set& subtract_fee_outputs) { - std::set 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 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 InterpretSubtractFeeFromOutputInstructions(const UniValue& sffo_instructions, const std::vector& destinations) +{ + std::set 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(); + 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 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 recipients; - ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + std::vector 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 recipients; - ParseRecipients(sendTo, subtractFeeFromAmount, recipients); + std::vector 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 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& 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 change_position; bool lockUnspents = false; - UniValue subtractFeeFromOutputs; - std::set 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(); - 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(); - 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> destinations; + for (const auto& tx_out : tx.vout) { + CTxDestination dest; + ExtractDestination(tx_out.scriptPubKey, dest); + destinations.emplace_back(dest, tx_out.nValue); + } + std::vector dummy(destinations.size(), "dummy"); + std::vector 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 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 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 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 6bbe6a1647..dc5c442b95 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -811,7 +811,9 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu std::vector 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; } @@ -997,7 +999,9 @@ bool LegacyScriptPubKeyMan::GetKey(const CKeyID &address, CKey& keyOut) const { const CPubKey &vchPubKey = (*mi).second.first; const std::vector &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; } @@ -2128,7 +2132,9 @@ std::map DescriptorScriptPubKeyMan::GetKeys() const const CPubKey& pubkey = key_pair.second.first; const std::vector& 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; @@ -2262,7 +2268,9 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const std::vector 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..b63ba5bda0 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -22,6 +22,7 @@ #include +#include #include #include @@ -46,7 +47,8 @@ class WalletStorage 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 cb) const = 0; virtual bool HasEncryptionKeys() const = 0; virtual bool IsLocked() const = 0; }; 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 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 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 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 CreateTransaction( return res; } -util::Result FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional change_pos, bool lockUnspents, const std::set& setSubtractFeeFromOutputs, CCoinControl coinControl) +util::Result FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector& vecSend, std::optional change_pos, bool lockUnspents, CCoinControl coinControl) { - std::vector 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 CreateTransaction(CWallet& wallet, const * Insert additional inputs into the transaction by * calling CreateTransaction(); */ -util::Result FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional change_pos, bool lockUnspents, const std::set& setSubtractFeeFromOutputs, CCoinControl); +util::Result FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const std::vector& recipients, std::optional change_pos, bool lockUnspents, CCoinControl); } // namespace wallet #endif // BITCOIN_WALLET_SPEND_H 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 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(-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..e93cd4b4b9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3513,9 +3513,10 @@ void CWallet::SetupLegacyScriptPubKeyMan() AddScriptPubKeyMan(id, std::move(spk_manager)); } -const CKeyingMaterial& CWallet::GetEncryptionKey() const +bool CWallet::WithEncryptionKey(std::function cb) const { - return vMasterKey; + LOCK(cs_wallet); + return cb(vMasterKey); } bool CWallet::HasEncryptionKeys() const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d044b2d715..648f2504b2 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -962,7 +962,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Make a LegacyScriptPubKeyMan and set it for all types, internal, and external. void SetupLegacyScriptPubKeyMan(); - const CKeyingMaterial& GetEncryptionKey() const override; + bool WithEncryptionKey(std::function 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 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_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 @@ def run_test(self): 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 @@ def run_test(self): # 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 @@ def run_test(self): 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/test_runner.py b/test/functional/test_runner.py index 1225f18535..bc9138a8ae 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -333,6 +333,8 @@ '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_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 6f1d335f7a..e1cdb75dab 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 @@ def run_test(self): 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..56cb71cd62 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -24,6 +24,7 @@ AddressType, ADDRESS_BCRT1_UNSPENDABLE, ) +from test_framework.messages import COIN from test_framework.util import ( assert_equal, set_node_times, @@ -270,7 +271,9 @@ def run_test(self): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() * 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 = max(get_rand_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_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..8a5853b162 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, } @@ -372,8 +373,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/