diff options
34 files changed, 251 insertions, 119 deletions
diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 64b1e4e182..8a7a978994 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -70,7 +70,7 @@ if [ "$CI_OS_NAME" == "macos" ]; then fi CI_EXEC () { - $CI_EXEC_CMD_PREFIX bash -c "export PATH=${BINS_SCRATCH_DIR}:${BASE_ROOT_DIR}/ci/retry:\$PATH && cd \"${BASE_ROOT_DIR}\" && $*" + $CI_EXEC_CMD_PREFIX bash -c "export PATH=\"/path_with space:${BINS_SCRATCH_DIR}:${BASE_ROOT_DIR}/ci/retry:\$PATH\" && cd \"${BASE_ROOT_DIR}\" && $*" } export -f CI_EXEC diff --git a/configure.ac b/configure.ac index 68edc84ded..adc862d251 100644 --- a/configure.ac +++ b/configure.ac @@ -321,11 +321,6 @@ AC_ARG_ENABLE([external-signer], [use_external_signer=$enableval], [use_external_signer=auto]) -AC_ARG_ENABLE([lto], - [AS_HELP_STRING([--enable-lto],[build using LTO (default is no)])], - [enable_lto=$enableval], - [enable_lto=no]) - AC_LANG_PUSH([C++]) dnl Check for a flag to turn compiler warnings into errors. This is helpful for checks which may @@ -377,11 +372,6 @@ if test "$enable_debug" = "yes"; then AX_CHECK_COMPILE_FLAG([-ftrapv], [DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -ftrapv"], [], [$CXXFLAG_WERROR]) fi -if test "$enable_lto" = "yes"; then - AX_CHECK_COMPILE_FLAG([-flto], [LTO_CXXFLAGS="$LTO_CXXFLAGS -flto"], [AC_MSG_ERROR([compile failed with -flto])], [$CXXFLAG_WERROR]) - AX_CHECK_LINK_FLAG([-flto], [LTO_LDFLAGS="$LTO_LDFLAGS -flto"], [AC_MSG_ERROR([link failed with -flto])], [$CXXFLAG_WERROR]) -fi - if test "$use_sanitizers" != ""; then dnl First check if the compiler accepts flags. If an incompatible pair like dnl -fsanitize=address,thread is used here, this check will fail. This will also @@ -1891,8 +1881,6 @@ AC_SUBST(GPROF_LDFLAGS) AC_SUBST(HARDENED_CXXFLAGS) AC_SUBST(HARDENED_CPPFLAGS) AC_SUBST(HARDENED_LDFLAGS) -AC_SUBST(LTO_CXXFLAGS) -AC_SUBST(LTO_LDFLAGS) AC_SUBST(PIC_FLAGS) AC_SUBST(PIE_FLAGS) AC_SUBST(SANITIZER_CXXFLAGS) @@ -2000,7 +1988,6 @@ echo " sanitizers = $use_sanitizers" echo " debug enabled = $enable_debug" echo " gprof enabled = $enable_gprof" echo " werror = $enable_werror" -echo " LTO = $enable_lto" echo echo " target os = $host_os" echo " build os = $build_os" @@ -2009,8 +1996,8 @@ echo " CC = $CC" echo " CFLAGS = $PTHREAD_CFLAGS $CFLAGS" echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS" echo " CXX = $CXX" -echo " CXXFLAGS = $LTO_CXXFLAGS $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS" -echo " LDFLAGS = $LTO_LDFLAGS $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $LDFLAGS" +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 " AR = $AR" echo " ARFLAGS = $ARFLAGS" echo diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index 590c2ed87d..f57e9abfec 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -192,6 +192,16 @@ def check_MACHO_control_flow(binary) -> bool: return True return False +def check_MACHO_branch_protection(binary) -> bool: + ''' + Check for branch protection instrumentation + ''' + content = binary.get_content_from_virtual_address(binary.entrypoint, 4, lief.Binary.VA_TYPES.AUTO) + + if content.tolist() == [95, 36, 3, 213]: # bti + return True + return False + BASE_ELF = [ ('PIE', check_PIE), ('NX', check_NX), @@ -231,7 +241,7 @@ CHECKS = { lief.ARCHITECTURES.X86: BASE_MACHO + [('PIE', check_PIE), ('NX', check_NX), ('CONTROL_FLOW', check_MACHO_control_flow)], - lief.ARCHITECTURES.ARM64: BASE_MACHO, + lief.ARCHITECTURES.ARM64: BASE_MACHO + [('BRANCH_PROTECTION', check_MACHO_branch_protection)], } } diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index 64daabad4e..48823c7e45 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -137,12 +137,12 @@ class TestSecurityChecks(unittest.TestCase): else: # arm64 darwin doesn't support non-PIE binaries, control flow or executable stacks self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-no_fixup_chains']), - (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains']), + (1, executable+': failed NOUNDEFS Canary FIXUP_CHAINS BRANCH_PROTECTION')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fno-stack-protector', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (1, executable+': failed NOUNDEFS Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-flat_namespace','-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (1, executable+': failed NOUNDEFS')) - self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains']), + self.assertEqual(call_security_check(cc, source, executable, ['-fstack-protector-all', '-Wl,-fixup_chains', '-mbranch-protection=bti']), (0, '')) diff --git a/depends/README.md b/depends/README.md index a33037b6a6..a8dfc83e3b 100644 --- a/depends/README.md +++ b/depends/README.md @@ -120,7 +120,7 @@ The following can be set when running make: `make FOO=bar` - `LOG`: Use file-based logging for individual packages. During a package build its log file resides in the `depends` directory, and the log file is printed out automatically in case of build error. After successful build log files are moved along with package archives -- `LTO`: Use LTO when building packages. +- `LTO`: Enable options needed for LTO. Does not add `-flto` related options to *FLAGS. - `NO_HARDEN=1`: Don't use hardening options when building packages If some packages are not built, for example `make NO_WALLET=1`, the appropriate diff --git a/depends/builders/darwin.mk b/depends/builders/darwin.mk index 8ed82b276d..eb64c97f64 100644 --- a/depends/builders/darwin.mk +++ b/depends/builders/darwin.mk @@ -11,8 +11,8 @@ build_darwin_SHA256SUM=shasum -a 256 build_darwin_DOWNLOAD=curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o #darwin host on darwin builder. overrides darwin host preferences. -darwin_CC=$(shell xcrun -f clang) -mmacosx-version-min=$(OSX_MIN_VERSION) -isysroot$(shell xcrun --show-sdk-path) -darwin_CXX:=$(shell xcrun -f clang++) -mmacosx-version-min=$(OSX_MIN_VERSION) -stdlib=libc++ -isysroot$(shell xcrun --show-sdk-path) +darwin_CC=$(shell xcrun -f clang) -isysroot$(shell xcrun --show-sdk-path) +darwin_CXX:=$(shell xcrun -f clang++) -stdlib=libc++ -isysroot$(shell xcrun --show-sdk-path) darwin_AR:=$(shell xcrun -f ar) darwin_RANLIB:=$(shell xcrun -f ranlib) darwin_STRIP:=$(shell xcrun -f strip) diff --git a/depends/config.site.in b/depends/config.site.in index 9bc599e7cd..29b2a67ed0 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -78,10 +78,6 @@ if test "@host_os@" = darwin; then BREW=no fi -if test -z "$enable_lto" && test -n "@lto@"; then - enable_lto=yes -fi - if test -z "$enable_hardening" && test -n "@no_harden@"; then enable_hardening=no fi diff --git a/depends/funcs.mk b/depends/funcs.mk index 987be4e611..ce87aa579f 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -139,9 +139,9 @@ $(1)_config_env+=PKG_CONFIG_LIBDIR=$($($(1)_type)_prefix)/lib/pkgconfig $(1)_config_env+=PKG_CONFIG_PATH=$($($(1)_type)_prefix)/share/pkgconfig $(1)_config_env+=PKG_CONFIG_SYSROOT_DIR=/ $(1)_config_env+=CMAKE_MODULE_PATH=$($($(1)_type)_prefix)/lib/cmake -$(1)_config_env+=PATH=$(build_prefix)/bin:$(PATH) -$(1)_build_env+=PATH=$(build_prefix)/bin:$(PATH) -$(1)_stage_env+=PATH=$(build_prefix)/bin:$(PATH) +$(1)_config_env+=PATH="$(build_prefix)/bin:$(PATH)" +$(1)_build_env+=PATH="$(build_prefix)/bin:$(PATH)" +$(1)_stage_env+=PATH="$(build_prefix)/bin:$(PATH)" # Setting a --build type that differs from --host will explicitly enable # cross-compilation mode. Note that --build defaults to the output of diff --git a/depends/hosts/android.mk b/depends/hosts/android.mk index b53966dcf8..a1c8c56dba 100644 --- a/depends/hosts/android.mk +++ b/depends/hosts/android.mk @@ -9,11 +9,6 @@ endif android_CFLAGS=-std=$(C_STANDARD) android_CXXFLAGS=-std=$(CXX_STANDARD) -ifneq ($(LTO),) -android_CFLAGS += -flto -android_LDFLAGS += -flto -endif - android_AR=$(ANDROID_TOOLCHAIN_BIN)/llvm-ar android_RANLIB=$(ANDROID_TOOLCHAIN_BIN)/llvm-ranlib diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index a89c82e408..b94ac7d56f 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -79,28 +79,27 @@ $(foreach TOOL,$(cctools_TOOLS),$(eval darwin_$(TOOL) = $$(build_prefix)/bin/$$( darwin_CC=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \ -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH \ -u LIBRARY_PATH \ - $(clang_prog) --target=$(host) -mmacosx-version-min=$(OSX_MIN_VERSION) \ - -B$(build_prefix)/bin -mlinker-version=$(LD64_VERSION) \ + $(clang_prog) --target=$(host) \ + -B$(build_prefix)/bin \ -isysroot$(OSX_SDK) -nostdlibinc \ -iwithsysroot/usr/include -iframeworkwithsysroot/System/Library/Frameworks darwin_CXX=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \ -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH \ -u LIBRARY_PATH \ - $(clangxx_prog) --target=$(host) -mmacosx-version-min=$(OSX_MIN_VERSION) \ - -B$(build_prefix)/bin -mlinker-version=$(LD64_VERSION) \ + $(clangxx_prog) --target=$(host) \ + -B$(build_prefix)/bin \ -isysroot$(OSX_SDK) -nostdlibinc \ -iwithsysroot/usr/include/c++/v1 \ -iwithsysroot/usr/include -iframeworkwithsysroot/System/Library/Frameworks -darwin_CFLAGS=-pipe -std=$(C_STANDARD) -darwin_CXXFLAGS=-pipe -std=$(CXX_STANDARD) +darwin_CFLAGS=-pipe -std=$(C_STANDARD) -mmacosx-version-min=$(OSX_MIN_VERSION) +darwin_CXXFLAGS=-pipe -std=$(CXX_STANDARD) -mmacosx-version-min=$(OSX_MIN_VERSION) darwin_LDFLAGS=-Wl,-platform_version,macos,$(OSX_MIN_VERSION),$(OSX_SDK_VERSION) -ifneq ($(LTO),) -darwin_CFLAGS += -flto -darwin_CXXFLAGS += -flto -darwin_LDFLAGS += -flto +ifneq ($(build_os),darwin) +darwin_CFLAGS += -mlinker-version=$(LD64_VERSION) +darwin_CXXFLAGS += -mlinker-version=$(LD64_VERSION) endif darwin_release_CFLAGS=-O2 diff --git a/depends/hosts/freebsd.mk b/depends/hosts/freebsd.mk index 5351d0b900..055097b03d 100644 --- a/depends/hosts/freebsd.mk +++ b/depends/hosts/freebsd.mk @@ -1,12 +1,6 @@ freebsd_CFLAGS=-pipe -std=$(C_STANDARD) freebsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) -ifneq ($(LTO),) -freebsd_CFLAGS += -flto -freebsd_CXXFLAGS += -flto -freebsd_LDFLAGS += -flto -endif - freebsd_release_CFLAGS=-O2 freebsd_release_CXXFLAGS=$(freebsd_release_CFLAGS) diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index 1f33640c66..8be23be57d 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -2,10 +2,6 @@ linux_CFLAGS=-pipe -std=$(C_STANDARD) linux_CXXFLAGS=-pipe -std=$(CXX_STANDARD) ifneq ($(LTO),) -linux_CFLAGS += -flto -linux_CXXFLAGS += -flto -linux_LDFLAGS += -flto - linux_AR = $(host_toolchain)gcc-ar linux_NM = $(host_toolchain)gcc-nm linux_RANLIB = $(host_toolchain)gcc-ranlib diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk index fc1cc1afbe..15aa7cd25a 100644 --- a/depends/hosts/mingw32.mk +++ b/depends/hosts/mingw32.mk @@ -6,10 +6,6 @@ mingw32_CFLAGS=-pipe -std=$(C_STANDARD) mingw32_CXXFLAGS=-pipe -std=$(CXX_STANDARD) ifneq ($(LTO),) -mingw32_CFLAGS += -flto -mingw32_CXXFLAGS += -flto -mingw32_LDFLAGS += -flto - mingw32_AR = $(host_toolchain)gcc-ar mingw32_NM = $(host_toolchain)gcc-nm mingw32_RANLIB = $(host_toolchain)gcc-ranlib diff --git a/depends/hosts/netbsd.mk b/depends/hosts/netbsd.mk index 14121dca20..f33b2d2889 100644 --- a/depends/hosts/netbsd.mk +++ b/depends/hosts/netbsd.mk @@ -2,10 +2,6 @@ netbsd_CFLAGS=-pipe -std=$(C_STANDARD) netbsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) ifneq ($(LTO),) -netbsd_CFLAGS += -flto -netbsd_CXXFLAGS += -flto -netbsd_LDFLAGS += -flto - netbsd_AR = $(host_toolchain)gcc-ar netbsd_NM = $(host_toolchain)gcc-nm netbsd_RANLIB = $(host_toolchain)gcc-ranlib diff --git a/depends/hosts/openbsd.mk b/depends/hosts/openbsd.mk index d330e94d2e..bdd36dc9b3 100644 --- a/depends/hosts/openbsd.mk +++ b/depends/hosts/openbsd.mk @@ -1,12 +1,6 @@ openbsd_CFLAGS=-pipe -std=$(C_STANDARD) openbsd_CXXFLAGS=-pipe -std=$(CXX_STANDARD) -ifneq ($(LTO),) -openbsd_CFLAGS += -flto -openbsd_CXXFLAGS += -flto -openbsd_LDFLAGS += -flto -endif - openbsd_release_CFLAGS=-O2 openbsd_release_CXXFLAGS=$(openbsd_release_CFLAGS) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 1d670d39f1..ece36cb088 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -739,25 +739,25 @@ logging messages. They should be used as follows: Note that `LogPrint(BCLog::CATEGORY, fmt, params...)` is a deprecated alias for `LogDebug`. -- `LogInfo(fmt, params...)` should only be used rarely, eg for startup +- `LogInfo(fmt, params...)` should only be used rarely, e.g. for startup messages or for infrequent and important events such as a new block tip being found or a new outbound connection being made. These log messages - are unconditional so care must be taken that they can't be used by an + are unconditional, so care must be taken that they can't be used by an attacker to fill up storage. Note that `LogPrintf(fmt, params...)` is a deprecated alias for `LogInfo`. - `LogError(fmt, params...)` should be used in place of `LogInfo` for severe problems that require the node (or a subsystem) to shut down - entirely (eg, insufficient storage space). + entirely (e.g., insufficient storage space). - `LogWarning(fmt, params...)` should be used in place of `LogInfo` for severe problems that the node admin should address, but are not - severe enough to warrant shutting down the node (eg, system time + severe enough to warrant shutting down the node (e.g., system time appears to be wrong, unknown soft fork appears to have activated). -- `LogTrace(BCLog::CATEGORY, fmt, params...) should be used in place of +- `LogTrace(BCLog::CATEGORY, fmt, params...)` should be used in place of `LogDebug` for log messages that would be unusable on a production - system, eg due to being too noisy in normal use, or too resource + system, e.g. due to being too noisy in normal use, or too resource intensive to process. These will be logged if the startup options `-debug=category -loglevel=category:trace` or `-debug=1 -loglevel=trace` are selected. diff --git a/src/Makefile.am b/src/Makefile.am index b6f0daaaba..780f547b4b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -8,8 +8,8 @@ print-%: FORCE DIST_SUBDIRS = secp256k1 -AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(LTO_LDFLAGS) $(CORE_LDFLAGS) -AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(LTO_CXXFLAGS) $(CORE_CXXFLAGS) +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_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS) AM_LIBTOOLFLAGS = --preserve-dup-deps PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) diff --git a/src/addrman.h b/src/addrman.h index ef9c766eff..be2ee8c2cb 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -111,13 +111,16 @@ public: /** * Attempt to add one or more addresses to addrman's new table. + * If an address already exists in addrman, the existing entry may be updated + * (e.g. adding additional service flags). If the existing entry is in the new table, + * it may be added to more buckets, improving the probability of selection. * * @param[in] vAddr Address records to attempt to add. * @param[in] source The address of the node that sent us these addr records. * @param[in] time_penalty A "time penalty" to apply to the address record's nTime. If a peer * sends us an address record with nTime=n, then we'll add it to our * addrman with nTime=(n - time_penalty). - * @return true if at least one address is successfully added. */ + * @return true if at least one address is successfully added, or added to an additional bucket. Unaffected by updates. */ bool Add(const std::vector<CAddress>& vAddr, const CNetAddr& source, std::chrono::seconds time_penalty = 0s); /** diff --git a/src/init/common.cpp b/src/init/common.cpp index 6560258ef5..0800cd93d8 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -31,7 +31,7 @@ void AddLoggingArgs(ArgsManager& argsman) ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-debugexclude=<category>", "Exclude debug and trace logging for a category. Can be used in conjunction with -debug=1 to output debug and trace logging for all categories except the specified category. This option can be specified multiple times to exclude multiple categories.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-loglevel=<level>|<category>:<level>", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC: %s (default=%s); warning and error levels are always logged. If <category>:<level> is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. <category> can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-loglevel=<level>|<category>:<level>", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC. Possible values are %s (default=%s). The following levels are always logged: error, warning, info. If <category>:<level> is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. <category> can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); #ifdef HAVE_THREAD_LOCAL argsman.AddArg("-logthreadnames", strprintf("Prepend debug output with name of the originating thread (only available on platforms supporting thread_local) (default: %u)", DEFAULT_LOGTHREADNAMES), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index 0808d42452..cae4b04bf6 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -67,10 +67,20 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active return false; } file.SetXor(xor_key); - uint64_t num; - file >> num; - while (num) { - --num; + uint64_t total_txns_to_load; + file >> total_txns_to_load; + uint64_t txns_tried = 0; + LogInfo("Loading %u mempool transactions from disk...\n", total_txns_to_load); + int next_tenth_to_report = 0; + while (txns_tried < total_txns_to_load) { + const int percentage_done(100.0 * txns_tried / total_txns_to_load); + if (next_tenth_to_report < percentage_done / 10) { + LogInfo("Progress loading mempool transactions from disk: %d%% (tried %u, %u remaining)\n", + percentage_done, txns_tried, total_txns_to_load - txns_tried); + next_tenth_to_report = percentage_done / 10; + } + ++txns_tried; + CTransactionRef tx; int64_t nTime; int64_t nFeeDelta; diff --git a/src/key.cpp b/src/key.cpp index 512790252a..2bd6396298 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -179,7 +179,7 @@ CPrivKey CKey::GetPrivKey() const { size_t seckeylen; seckey.resize(SIZE); seckeylen = SIZE; - ret = ec_seckey_export_der(secp256k1_context_sign, seckey.data(), &seckeylen, begin(), fCompressed); + ret = ec_seckey_export_der(secp256k1_context_sign, seckey.data(), &seckeylen, UCharCast(begin()), fCompressed); assert(ret); seckey.resize(seckeylen); return seckey; @@ -190,7 +190,7 @@ CPubKey CKey::GetPubKey() const { secp256k1_pubkey pubkey; size_t clen = CPubKey::SIZE; CPubKey result; - int ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pubkey, begin()); + int ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pubkey, UCharCast(begin())); assert(ret); secp256k1_ec_pubkey_serialize(secp256k1_context_sign, (unsigned char*)result.begin(), &clen, &pubkey, fCompressed ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED); assert(result.size() == clen); @@ -220,19 +220,19 @@ bool CKey::Sign(const uint256 &hash, std::vector<unsigned char>& vchSig, bool gr WriteLE32(extra_entropy, test_case); secp256k1_ecdsa_signature sig; uint32_t counter = 0; - int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, (!grind && test_case) ? extra_entropy : nullptr); + int ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), UCharCast(begin()), secp256k1_nonce_function_rfc6979, (!grind && test_case) ? extra_entropy : nullptr); // Grind for low R while (ret && !SigHasLowR(&sig) && grind) { WriteLE32(extra_entropy, ++counter); - ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, extra_entropy); + ret = secp256k1_ecdsa_sign(secp256k1_context_sign, &sig, hash.begin(), UCharCast(begin()), secp256k1_nonce_function_rfc6979, extra_entropy); } assert(ret); secp256k1_ecdsa_signature_serialize_der(secp256k1_context_sign, vchSig.data(), &nSigLen, &sig); vchSig.resize(nSigLen); // Additional verification step to prevent using a potentially corrupted signature secp256k1_pubkey pk; - ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, begin()); + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &pk, UCharCast(begin())); assert(ret); ret = secp256k1_ecdsa_verify(secp256k1_context_static, &sig, hash.begin(), &pk); assert(ret); @@ -258,7 +258,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig) vchSig.resize(CPubKey::COMPACT_SIGNATURE_SIZE); int rec = -1; secp256k1_ecdsa_recoverable_signature rsig; - int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); + int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &rsig, hash.begin(), UCharCast(begin()), secp256k1_nonce_function_rfc6979, nullptr); assert(ret); ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &rsig); assert(ret); @@ -266,7 +266,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector<unsigned char>& vchSig) vchSig[0] = 27 + rec + (fCompressed ? 4 : 0); // Additional verification step to prevent using a potentially corrupted signature secp256k1_pubkey epk, rpk; - ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, begin()); + ret = secp256k1_ec_pubkey_create(secp256k1_context_sign, &epk, UCharCast(begin())); assert(ret); ret = secp256k1_ecdsa_recover(secp256k1_context_static, &rpk, &rsig, hash.begin()); assert(ret); @@ -279,7 +279,7 @@ bool CKey::SignSchnorr(const uint256& hash, Span<unsigned char> sig, const uint2 { assert(sig.size() == 64); secp256k1_keypair keypair; - if (!secp256k1_keypair_create(secp256k1_context_sign, &keypair, begin())) return false; + if (!secp256k1_keypair_create(secp256k1_context_sign, &keypair, UCharCast(begin()))) return false; if (merkle_root) { secp256k1_xonly_pubkey pubkey; if (!secp256k1_keypair_xonly_pub(secp256k1_context_sign, &pubkey, nullptr, &keypair)) return false; @@ -324,7 +324,7 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const BIP32Hash(cc, nChild, *pubkey.begin(), pubkey.begin()+1, vout.data()); } else { assert(size() == 32); - BIP32Hash(cc, nChild, 0, begin(), vout.data()); + BIP32Hash(cc, nChild, 0, UCharCast(begin()), vout.data()); } memcpy(ccChild.begin(), vout.data()+32, 32); keyChild.Set(begin(), begin() + 32, true); @@ -100,7 +100,7 @@ public: { if (size_t(pend - pbegin) != std::tuple_size_v<KeyType>) { ClearKeyData(); - } else if (Check(&pbegin[0])) { + } else if (Check(UCharCast(&pbegin[0]))) { MakeKeyData(); memcpy(keydata->data(), (unsigned char*)&pbegin[0], keydata->size()); fCompressed = fCompressedIn; @@ -112,8 +112,8 @@ public: //! Simple read-only vector-like interface. unsigned int size() const { return keydata ? keydata->size() : 0; } const std::byte* data() const { return keydata ? reinterpret_cast<const std::byte*>(keydata->data()) : nullptr; } - const unsigned char* begin() const { return keydata ? keydata->data() : nullptr; } - const unsigned char* end() const { return begin() + size(); } + const std::byte* begin() const { return data(); } + const std::byte* end() const { return data() + size(); } //! Check whether this private key is valid. bool IsValid() const { return !!keydata; } diff --git a/src/key_io.cpp b/src/key_io.cpp index 5bcbb8a069..a373a2201d 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -228,7 +228,7 @@ std::string EncodeSecret(const CKey& key) { assert(key.IsValid()); std::vector<unsigned char> data = Params().Base58Prefix(CChainParams::SECRET_KEY); - data.insert(data.end(), key.begin(), key.end()); + data.insert(data.end(), UCharCast(key.begin()), UCharCast(key.end())); if (key.IsCompressed()) { data.push_back(1); } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 683a7aa10f..80cf610a0d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3376,6 +3376,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, vRecv >> CNetAddr::V1(addrMe); if (!pfrom.IsInboundConn()) { + // Overwrites potentially existing services. In contrast to this, + // unvalidated services received via gossip relay in ADDR/ADDRV2 + // messages are only ever added but cannot replace existing ones. m_addrman.SetServices(pfrom.addr, nServices); } if (pfrom.ExpectServicesFromConn() && !HasAllDesirableServiceFlags(nServices)) diff --git a/src/pubkey.h b/src/pubkey.h index 2b655c3f73..15d7e7bc07 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -285,10 +285,11 @@ public: CPubKey GetEvenCorrespondingCPubKey() const; const unsigned char& operator[](int pos) const { return *(m_keydata.begin() + pos); } - const unsigned char* data() const { return m_keydata.begin(); } static constexpr size_t size() { return decltype(m_keydata)::size(); } + const unsigned char* data() const { return m_keydata.begin(); } const unsigned char* begin() const { return m_keydata.begin(); } const unsigned char* end() const { return m_keydata.end(); } + unsigned char* data() { return m_keydata.begin(); } unsigned char* begin() { return m_keydata.begin(); } unsigned char* end() { return m_keydata.end(); } bool operator==(const XOnlyPubKey& other) const { return m_keydata == other.m_keydata; } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 5fea8c22c9..e53f889897 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -313,7 +313,7 @@ static RPCHelpMan addnode() { {"node", RPCArg::Type::STR, RPCArg::Optional::NO, "The address of the peer to connect to"}, {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "'add' to add a node to the list, 'remove' to remove a node from the list, 'onetry' to try a connection to the node once"}, - {"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol (ignored for 'remove' command)"}, + {"v2transport", RPCArg::Type::BOOL, RPCArg::DefaultHint{"set by -v2transport"}, "Attempt to connect using BIP324 v2 transport protocol (ignored for 'remove' command)"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -332,9 +332,10 @@ static RPCHelpMan addnode() CConnman& connman = EnsureConnman(node); const std::string node_arg{request.params[0].get_str()}; - bool use_v2transport = self.Arg<bool>(2); + bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2; + bool use_v2transport = self.MaybeArg<bool>(2).value_or(node_v2transport); - if (use_v2transport && !(node.connman->GetLocalServices() & NODE_P2P_V2)) { + if (use_v2transport && !node_v2transport) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: v2transport requested but not enabled (see -v2transport)"); } diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 5bd4f271cd..9668a85484 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -1068,7 +1068,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) BOOST_AUTO_TEST_CASE(addrman_update_address) { - // Tests updating nTime via Connected() and nServices via SetServices() + // Tests updating nTime via Connected() and nServices via SetServices() and Add() auto addrman = std::make_unique<AddrMan>(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); CNetAddr source{ResolveIP("252.2.2.2")}; CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)}; @@ -1095,6 +1095,32 @@ BOOST_AUTO_TEST_CASE(addrman_update_address) BOOST_CHECK_EQUAL(vAddr2.size(), 1U); BOOST_CHECK(vAddr2.at(0).nTime >= start_time + 10000s); BOOST_CHECK_EQUAL(vAddr2.at(0).nServices, NODE_NETWORK_LIMITED); + + // Updating an existing addr through Add() (used in gossip relay) can add additional services but can't remove existing ones. + CAddress addr_v2{CAddress(ResolveService("250.1.1.1", 8333), NODE_P2P_V2)}; + addr_v2.nTime = start_time; + BOOST_CHECK(!addrman->Add({addr_v2}, source)); + std::vector<CAddress> vAddr3{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr3.size(), 1U); + BOOST_CHECK_EQUAL(vAddr3.at(0).nServices, NODE_P2P_V2 | NODE_NETWORK_LIMITED); + + // SetServices() (used when we connected to them) overwrites existing service flags + addrman->SetServices(addr, NODE_NETWORK); + std::vector<CAddress> vAddr4{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr4.size(), 1U); + BOOST_CHECK_EQUAL(vAddr4.at(0).nServices, NODE_NETWORK); + + // Promoting to Tried does not affect the service flags + BOOST_CHECK(addrman->Good(addr)); // addr has NODE_NONE + std::vector<CAddress> vAddr5{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr5.size(), 1U); + BOOST_CHECK_EQUAL(vAddr5.at(0).nServices, NODE_NETWORK); + + // Adding service flags even works when the addr is in Tried + BOOST_CHECK(!addrman->Add({addr_v2}, source)); + std::vector<CAddress> vAddr6{addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt)}; + BOOST_CHECK_EQUAL(vAddr6.size(), 1U); + BOOST_CHECK_EQUAL(vAddr6.at(0).nServices, NODE_NETWORK | NODE_P2P_V2); } BOOST_AUTO_TEST_CASE(addrman_size) diff --git a/src/validation.cpp b/src/validation.cpp index 0f3d5d1454..8c583c586c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4868,7 +4868,9 @@ void ChainstateManager::CheckBlockIndex() // 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)); + || (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; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 8f0b72c2b0..6bbe6a1647 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -280,7 +280,7 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat { const CKey &key = mKey.second; CPubKey vchPubKey = key.GetPubKey(); - CKeyingMaterial vchSecret(key.begin(), key.end()); + CKeyingMaterial vchSecret{UCharCast(key.begin()), UCharCast(key.end())}; std::vector<unsigned char> vchCryptedSecret; if (!EncryptSecret(master_key, vchSecret, vchPubKey.GetHash(), vchCryptedSecret)) { encrypted_batch = nullptr; @@ -810,7 +810,7 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyInner(const CKey& key, const CPubKey &pu } std::vector<unsigned char> vchCryptedSecret; - CKeyingMaterial vchSecret(key.begin(), key.end()); + CKeyingMaterial vchSecret{UCharCast(key.begin()), UCharCast(key.end())}; if (!EncryptSecret(m_storage.GetEncryptionKey(), vchSecret, pubkey.GetHash(), vchCryptedSecret)) { return false; } @@ -2088,7 +2088,7 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle { const CKey &key = key_in.second; CPubKey pubkey = key.GetPubKey(); - CKeyingMaterial secret(key.begin(), key.end()); + CKeyingMaterial secret{UCharCast(key.begin()), UCharCast(key.end())}; std::vector<unsigned char> crypted_secret; if (!EncryptSecret(master_key, secret, pubkey.GetHash(), crypted_secret)) { return false; @@ -2261,7 +2261,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const } std::vector<unsigned char> crypted_secret; - CKeyingMaterial secret(key.begin(), key.end()); + CKeyingMaterial secret{UCharCast(key.begin()), UCharCast(key.end())}; if (!EncryptSecret(m_storage.GetEncryptionKey(), secret, pubkey.GetHash(), crypted_secret)) { return false; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bf8cfcb082..e03f5532fc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3099,6 +3099,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri if (time_first_key) walletInstance->MaybeUpdateBirthTime(*time_first_key); if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { + walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded return nullptr; } diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 0ee332b75b..4baf12c3f9 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -602,10 +602,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): if peer_advertises_v2 is None: peer_advertises_v2 = from_connection.use_v2transport - if peer_advertises_v2: - from_connection.addnode(node=ip_port, command="onetry", v2transport=True) + if peer_advertises_v2 != from_connection.use_v2transport: + from_connection.addnode(node=ip_port, command="onetry", v2transport=peer_advertises_v2) else: - # skip the optional third argument (default false) for + # skip the optional third argument if it matches the default, for # compatibility with older clients from_connection.addnode(ip_port, "onetry") diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 826705598f..9c8f0eca26 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -336,6 +336,7 @@ BASE_SCRIPTS = [ 'wallet_create_tx.py --descriptors', 'wallet_inactive_hdchains.py --legacy-wallet', 'wallet_spend_unconfirmed.py', + 'wallet_rescan_unconfirmed.py --descriptors', 'p2p_fingerprint.py', 'feature_uacomment.py', 'feature_init.py', diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 0ac67607e1..7f01d23941 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -20,7 +20,10 @@ happened previously. """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.address import AddressType +from test_framework.address import ( + AddressType, + ADDRESS_BCRT1_UNSPENDABLE, +) from test_framework.util import ( assert_equal, set_node_times, @@ -109,7 +112,7 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) - assert_equal(address["amount"], self.expected_balance) + assert_equal(address["amount"], self.amount_received) assert_equal(address["confirmations"], confirmations) # Verify the transaction is correctly marked watchonly depending on # whether the transaction pays to an imported public key or @@ -223,11 +226,11 @@ class ImportRescanTest(BitcoinTestFramework): variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] variant.do_import(variant.timestamp) if expect_rescan: - variant.expected_balance = variant.initial_amount + variant.amount_received = variant.initial_amount variant.expected_txs = 1 variant.check(variant.initial_txid, variant.initial_amount, variant.confirmation_height) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() @@ -247,7 +250,7 @@ class ImportRescanTest(BitcoinTestFramework): # Check the latest results from getbalance and listtransactions. for variant in IMPORT_VARIANTS: self.log.info('Run check for variant {}'.format(variant)) - variant.expected_balance += variant.sent_amount + variant.amount_received += variant.sent_amount variant.expected_txs += 1 variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height) @@ -267,14 +270,45 @@ class ImportRescanTest(BitcoinTestFramework): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() + variant.initial_amount = get_rand_amount() * 2 variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.confirmation_height = 0 variant.timestamp = timestamp + # Mine a block so these parents are confirmed + assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) + self.sync_mempools() + block_to_disconnect = self.generate(self.nodes[0], 1)[0] + assert_equal(len(self.nodes[0].getrawmempool()), 0) + + # For each variant, create an unconfirmed child transaction from initial_txid, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + # An equivalent test for descriptors exists in wallet_rescan_unconfirmed.py. + unspent_txid_map = {txin["txid"] : txin for txin in self.nodes[1].listunspent()} + for variant in mempool_variants: + # Send full amount, subtracting fee from outputs, to ensure no change is created. + child = self.nodes[1].send( + add_to_wallet=False, + inputs=[unspent_txid_map[variant.initial_txid]], + outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}], + subtract_fee_from_outputs=[0] + ) + variant.child_txid = child["txid"] + variant.amount_received = 0 + self.nodes[0].sendrawtransaction(child["hex"]) + + # Mempools should contain the child transactions for each variant. assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) self.sync_mempools() + # Mock a reorg so the parent transactions are added back to the mempool + for node in self.nodes: + node.invalidateblock(block_to_disconnect) + # Mempools should now contain the parent and child for each variant. + assert_equal(len(node.getrawmempool()), 2 * len(mempool_variants)) + # For each variation of wallet key import, invoke the import RPC and # check the results from getbalance and listtransactions. for variant in mempool_variants: @@ -283,11 +317,15 @@ class ImportRescanTest(BitcoinTestFramework): variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] variant.do_import(variant.timestamp) if expect_rescan: - variant.expected_balance = variant.initial_amount + # Ensure both transactions were rescanned. This would raise a JSONRPCError if the + # transactions were not identified as belonging to the wallet. + assert_equal(variant.node.gettransaction(variant.initial_txid)['confirmations'], 0) + assert_equal(variant.node.gettransaction(variant.child_txid)['confirmations'], 0) + variant.amount_received = variant.initial_amount variant.expected_txs = 1 - variant.check(variant.initial_txid, variant.initial_amount) + variant.check(variant.initial_txid, variant.initial_amount, 0) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() diff --git a/test/functional/wallet_rescan_unconfirmed.py b/test/functional/wallet_rescan_unconfirmed.py new file mode 100755 index 0000000000..ad9fa081c2 --- /dev/null +++ b/test/functional/wallet_rescan_unconfirmed.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024 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 that descriptor wallets rescan mempool transactions properly when importing.""" + +from test_framework.address import ( + address_to_scriptpubkey, + ADDRESS_BCRT1_UNSPENDABLE, +) +from test_framework.messages import COIN +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet +from test_framework.wallet_util import test_address + + +class WalletRescanUnconfirmed(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + + def set_test_params(self): + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + self.skip_if_no_sqlite() + + def run_test(self): + self.log.info("Create wallets and mine initial chain") + node = self.nodes[0] + tester_wallet = MiniWallet(node) + + node.createwallet(wallet_name='w0', disable_private_keys=False) + w0 = node.get_wallet_rpc('w0') + + self.log.info("Create a parent tx and mine it in a block that will later be disconnected") + parent_address = w0.getnewaddress() + tx_parent_to_reorg = tester_wallet.send_to( + from_node=node, + scriptPubKey=address_to_scriptpubkey(parent_address), + amount=COIN, + ) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + block_to_reorg = self.generate(tester_wallet, 1)[0] + assert_equal(len(node.getrawmempool()), 0) + node.syncwithvalidationinterfacequeue() + assert_equal(w0.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 1) + + # Create an unconfirmed child transaction from the parent tx, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + w0_utxos = w0.listunspent() + + self.log.info("Create a child tx and wait for it to propagate to all mempools") + # The only UTXO available to spend is tx_parent_to_reorg. + assert_equal(len(w0_utxos), 1) + assert_equal(w0_utxos[0]["txid"], tx_parent_to_reorg["txid"]) + tx_child_unconfirmed_sweep = w0.sendall([ADDRESS_BCRT1_UNSPENDABLE]) + assert tx_child_unconfirmed_sweep["txid"] in node.getrawmempool() + node.syncwithvalidationinterfacequeue() + + self.log.info("Mock a reorg, causing parent to re-enter mempools after its child") + node.invalidateblock(block_to_reorg) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + + self.log.info("Import descriptor wallet on another node") + descriptors_to_import = [{"desc": w0.getaddressinfo(parent_address)['parent_desc'], "timestamp": 0, "label": "w0 import"}] + + node.createwallet(wallet_name="w1", disable_private_keys=True) + w1 = node.get_wallet_rpc("w1") + w1.importdescriptors(descriptors_to_import) + + self.log.info("Check that the importing node has properly rescanned mempool transactions") + # Check that parent address is correctly determined as ismine + test_address(w1, parent_address, solvable=True, ismine=True) + # This would raise a JSONRPCError if the transactions were not identified as belonging to the wallet. + assert_equal(w1.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 0) + assert_equal(w1.gettransaction(tx_child_unconfirmed_sweep["txid"])["confirmations"], 0) + +if __name__ == '__main__': + WalletRescanUnconfirmed().main() |