From 70dc44d5752a600ea49ea6d4b641223e05cb1f7b Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 9 Jun 2026 10:31:34 +0200 Subject: [PATCH 01/20] Merge bitcoin/bitcoin#35487: scripted-diff: Rename UNIQUE_NAME to BITCOIN_UNIQUE_NAME fba713a28c89ee565cc2d20bdd3b0690e3a1f5b8 scripted-diff: Rename UNIQUE_NAME to BITCOIN_UNIQUE_NAME (Hennadii Stepanov) Pull request description: https://github.com/bitcoin/bitcoin/pull/34454#issuecomment-3822800049: > ... it is annoying that we keep running into the same bug over and over again (IIRC it happened in the past at least once for Bitcoin Core). Surely this is going to happen again in the future. And here we go again. --- The `nb30.h` Windows header [defines](https://github.com/mingw-w64/mingw-w64/blob/b536c4fdb038a9c59a7e5fb36e7d1293c4dc61d6/mingw-w64-headers/include/nb30.h#L78) `UNIQUE_NAME` as a macro. This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before `UNIQUE_NAME` is used, the preprocessor expands it into a numeric literal, causing syntax errors. Rename the macro to `BITCOIN_UNIQUE_NAME` to remove this fragility and avoid the collision entirely. --- Noticed while doing a Guix build of the [QML repo](https://github.com/bitcoin-core/gui-qml) for Windows. Recent similar PRs: https://github.com/bitcoin/bitcoin/pull/34454 and https://github.com/bitcoin/bitcoin/pull/34868. ACKs for top commit: maflcko: lgtm ACK fba713a28c89ee565cc2d20bdd3b0690e3a1f5b8 sedited: ACK fba713a28c89ee565cc2d20bdd3b0690e3a1f5b8 w0xlt: ACK fba713a28c89ee565cc2d20bdd3b0690e3a1f5b8 Tree-SHA512: 7a63b99a754e797eb8fa5d6a598606150f47ae1130d1d26067c509830e6575f0378ce63fe0ca35c69dce9a394451a34ddadd8b3d5f6f9a7e4c529108af546fb6 (cherry picked from commit 9868e1bf651d46836625dab0cb4b07660277891f) --- src/logging/timer.h | 8 ++++---- src/sync.h | 4 ++-- src/test/util/logging.h | 2 +- src/util/macros.h | 2 +- src/util/stdmutex.h | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/logging/timer.h b/src/logging/timer.h index 2b183822e969..1706c9e71bb3 100644 --- a/src/logging/timer.h +++ b/src/logging/timer.h @@ -99,13 +99,13 @@ class Timer #define LOG_TIME_MICROS_WITH_CATEGORY(end_msg, log_category) \ - BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) + BCLog::Timer BITCOIN_UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) #define LOG_TIME_MILLIS_WITH_CATEGORY(end_msg, log_category) \ - BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) + BCLog::Timer BITCOIN_UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category) #define LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE(end_msg, log_category) \ - BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category, /* msg_on_completion=*/false) + BCLog::Timer BITCOIN_UNIQUE_NAME(logging_timer)(__func__, end_msg, log_category, /* msg_on_completion=*/false) #define LOG_TIME_SECONDS(end_msg) \ - BCLog::Timer UNIQUE_NAME(logging_timer)(__func__, end_msg) + BCLog::Timer BITCOIN_UNIQUE_NAME(logging_timer)(__func__, end_msg) #endif // BITCOIN_LOGGING_TIMER_H diff --git a/src/sync.h b/src/sync.h index 123184d44f61..28bc78e91157 100644 --- a/src/sync.h +++ b/src/sync.h @@ -251,7 +251,7 @@ class SCOPED_LOCKABLE UniqueLock : public MutexType::unique_lock // it is not possible to use the lock's copy of the mutex for that purpose. // Instead, the original mutex needs to be passed back to the reverse_lock for // the sake of thread-safety analysis, but it is not actually used otherwise. -#define REVERSE_LOCK(g, cs) typename std::decay::type::reverse_lock UNIQUE_NAME(revlock)(g, cs, #cs, __FILE__, __LINE__) +#define REVERSE_LOCK(g, cs) typename std::decay::type::reverse_lock BITCOIN_UNIQUE_NAME(revlock)(g, cs, #cs, __FILE__, __LINE__) // When locking a Mutex, require negative capability to ensure the lock // is not already held @@ -265,7 +265,7 @@ inline MutexType& MaybeCheckNotHeld(MutexType& m) LOCKS_EXCLUDED(m) LOCK_RETURNE template inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNED(m) { return m; } -#define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) +#define LOCK(cs) UniqueLock BITCOIN_UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) #define LOCK2(cs1, cs2) \ UniqueLock criticalblock1(MaybeCheckNotHeld(cs1), #cs1, __FILE__, __LINE__); \ UniqueLock criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__) diff --git a/src/test/util/logging.h b/src/test/util/logging.h index ed3ee19e1bbf..6df208d841d5 100644 --- a/src/test/util/logging.h +++ b/src/test/util/logging.h @@ -39,6 +39,6 @@ class DebugLogHelper MatchFn m_match; }; -#define ASSERT_DEBUG_LOG(message) DebugLogHelper UNIQUE_NAME(debugloghelper)(message) +#define ASSERT_DEBUG_LOG(message) DebugLogHelper BITCOIN_UNIQUE_NAME(debugloghelper)(message) #endif // BITCOIN_TEST_UTIL_LOGGING_H diff --git a/src/util/macros.h b/src/util/macros.h index 9ad2fa6ef46b..eedd23c69265 100644 --- a/src/util/macros.h +++ b/src/util/macros.h @@ -8,7 +8,7 @@ #define PASTE(x, y) x ## y #define PASTE2(x, y) PASTE(x, y) -#define UNIQUE_NAME(name) PASTE2(name, __COUNTER__) +#define BITCOIN_UNIQUE_NAME(name) PASTE2(name, __COUNTER__) /** * Converts the parameter X to a string after macro replacement on X has been performed. diff --git a/src/util/stdmutex.h b/src/util/stdmutex.h index 2cc05013a59a..f6bd53c033bd 100644 --- a/src/util/stdmutex.h +++ b/src/util/stdmutex.h @@ -38,7 +38,7 @@ class LOCKABLE StdMutex : public std::mutex }; // Provide STDLOCK(..) wrapper around StdMutex::Guard that checks the lock is not already held -#define STDLOCK(cs) StdMutex::Guard UNIQUE_NAME(criticalblock){StdMutex::CheckNotHeld(cs)} +#define STDLOCK(cs) StdMutex::Guard BITCOIN_UNIQUE_NAME(criticalblock){StdMutex::CheckNotHeld(cs)} using StdLockGuard = StdMutex::Guard; // TODO: remove, provided for backwards compat only From bf407da5c382d47c1e5eb0a1c9389646bd51860d Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 9 Jun 2026 11:26:22 +0200 Subject: [PATCH 02/20] Merge bitcoin/bitcoin#35297: p2p: Release m_peer_mutex early in InitiateTxBroadcastToAll fa2afba28b5776a710386a3bacd5cb308149d671 p2p: Release m_peer_mutex early in InitiateTxBroadcastToAll (MarcoFalke) Pull request description: The `InitiateTxBroadcastToAll` method holds the `m_peer_mutex` while updating the bloom filters for all peers. This is perfectly fine, because updating the bloom filters is fast. Though, from a style-perspective, the lock does not need to be held for the whole function. Also, holding the lock longer, may confuse Tsan into a lock-order inversion false-positive (ref: https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514926359). So "fix" both issues in this style-refactor. ACKs for top commit: xyzconstant: Code review ACK fa2afba28b5776a710386a3bacd5cb308149d671 shuv-amp: ACK fa2afba28b5776a710386a3bacd5cb308149d671 danielabrozzoni: Code Review ACK fa2afba28b5776a710386a3bacd5cb308149d671 sedited: ACK fa2afba28b5776a710386a3bacd5cb308149d671 Tree-SHA512: c47849a4c3cc11c74b61fec3425db8ec7f78db4ca43d7bf3145ce640f7b0872701c09495f0dfe77109d09d5716d920ad3d7308483fe41564c30867b3e80432e7 (cherry picked from commit 17ed7f50609685893d641d6074ac0af114a15802) --- src/net_processing.cpp | 21 ++++++++++++++++++--- test/sanitizer_suppressions/tsan | 5 ----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1e6d1a6a78e4..72f9c53aa9bc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -569,6 +569,9 @@ class PeerManagerImpl final : public PeerManager * May return an empty shared_ptr if the Peer object can't be found. */ PeerRef RemovePeer(NodeId id) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + /// Get all existing peers in m_peer_map. + std::vector GetAllPeers() const EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + /** Mark a peer as misbehaving, which will cause it to be disconnected and its * address discouraged. */ void Misbehaving(Peer& peer, const std::string& message); @@ -1684,6 +1687,17 @@ PeerRef PeerManagerImpl::RemovePeer(NodeId id) return ret; } +std::vector PeerManagerImpl::GetAllPeers() const +{ + std::vector peers; + LOCK(m_peer_mutex); + peers.reserve(m_peer_map.size()); + for (const auto& [_, peer] : m_peer_map) { + peers.push_back(peer); + } + return peers; +} + bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const { { @@ -2137,9 +2151,10 @@ void PeerManagerImpl::SendPings() void PeerManagerImpl::InitiateTxBroadcastToAll(const Txid& txid, const Wtxid& wtxid) { - LOCK(m_peer_mutex); - for(auto& it : m_peer_map) { - Peer& peer = *it.second; + for (const PeerRef& peer_ref : GetAllPeers()) { + if (!peer_ref) continue; + Peer& peer{*peer_ref}; + auto tx_relay = peer.GetTxRelay(); if (!tx_relay) continue; diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 71446593fa82..a57d195c6ec6 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -3,11 +3,6 @@ # # https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions -# deadlock (TODO fix) -# To reproduce, see: -# https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514926359 -deadlock:Chainstate::ConnectTip - # Intentional deadlock in tests deadlock:sync_tests::potential_deadlock_detected From 65c43b77856bee869c4e2b508c6123734fda1b5c Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 9 Jun 2026 14:09:19 +0200 Subject: [PATCH 03/20] Merge bitcoin/bitcoin#35448: ci: don't build libunwind in msan 087f02c929cc1c09300822b02981acc710f1ab40 ci: skip libunwind runtime in LLVM build (fanquake) 6d47f7cc6fc6802fc620bfe578d8f6a765be1fc0 ci: use llvm 22.1.7 (fanquake) Pull request description: Also document why we use `LIBCXXABI_USE_LLVM_UNWINDER=OFF`. Upstream issue is https://github.com/llvm/llvm-project/issues/84348. ACKs for top commit: maflcko: lgtm ACK 087f02c929cc1c09300822b02981acc710f1ab40 sedited: ACK 087f02c929cc1c09300822b02981acc710f1ab40 Tree-SHA512: b93c798fd5a016cad40db9d24cb36cb72e531b284aee5458de41e062960514783e30c6f1413c0e62fa261758d783d0004a0973541cbb36bd34b77800c629bd7a (cherry picked from commit 543c00f47dd237fb233d324bef22af0cb9396715) --- ci/test/01_base_install.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 512d1a32e87c..8b10ba10baac 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -62,10 +62,13 @@ if [ -n "$PIP_PACKAGES" ]; then fi if [[ -n "${USE_INSTRUMENTED_LIBCPP}" ]]; then - ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-22.1.3" /llvm-project + ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-22.1.7" /llvm-project +# LLVM is configured with LIBCXXABI_USE_LLVM_UNWINDER=OFF, +# because libunwind doesn't handle exceptions under MSAN. +# https://github.com/llvm/llvm-project/issues/84348 cmake -G Ninja -B /cxx_build/ \ - -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \ + -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" \ -DCMAKE_BUILD_TYPE=Release \ -DLLVM_USE_SANITIZER="${USE_INSTRUMENTED_LIBCPP}" \ -DCMAKE_C_COMPILER=clang \ From f9055d93e3fd0290e16887bb9586aa0ade5c4895 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:38:36 +0100 Subject: [PATCH 04/20] Merge bitcoin/bitcoin#35459: guix: add setup.sh 54de023a7c9a002987c442b2b91826050f536bd5 guix: add setup.sh (fanquake) Pull request description: This is the first change in #25573, which splits out the setup & tarball generation code from `build.sh`, so that it can be re-used, from multiple (future) build scripts. ACKs for top commit: willcl-ark: ACK 54de023a7c9a002987c442b2b91826050f536bd5 hebasto: ACK 54de023a7c9a002987c442b2b91826050f536bd5. Tree-SHA512: 9a7f2fe322d281b9867414511af5243f4dd659ea42637f4eb8cc0c8629c94dab842669bb7c503f9fa67cab3fac65561364f07b5c0fda8e6d8c24e7bf161025ef (cherry picked from commit 4b91316643fab0cd21b357fc2191ecf29228a617) --- contrib/guix/libexec/build.sh | 87 +-------------------------------- contrib/guix/libexec/setup.sh | 91 +++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 85 deletions(-) create mode 100755 contrib/guix/libexec/setup.sh diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index cc1b3aba2c7d..98d1c486d86b 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -5,73 +5,13 @@ export LC_ALL=C set -e -o pipefail -# Environment variables for determinism -export TAR_OPTIONS="--no-same-owner --owner=0 --group=0 --numeric-owner --mtime='@${SOURCE_DATE_EPOCH}' --sort=name" -export TZ=UTC - -# Although Guix _does_ set umask when building its own packages (in our case, -# this is all packages in manifest.scm), it does not set it for `guix -# shell`. It does make sense for at least `guix shell --container` -# to set umask, so if that change gets merged upstream and we bump the -# time-machine to a commit which includes the aforementioned change, we can -# remove this line. -# -# This line should be placed before any commands which creates files. -umask 0022 - -if [ -n "$V" ]; then - # Print both unexpanded (-v) and expanded (-x) forms of commands as they are - # read from this file. - set -vx - # Set VERBOSE for CMake-based builds - export VERBOSE="$V" -fi - -# Check that required environment variables are set -cat << EOF -Required environment variables as seen inside the container: - DIST_ARCHIVE_BASE: ${DIST_ARCHIVE_BASE:?not set} - DISTNAME: ${DISTNAME:?not set} - HOST: ${HOST:?not set} - SOURCE_DATE_EPOCH: ${SOURCE_DATE_EPOCH:?not set} - JOBS: ${JOBS:?not set} - DISTSRC: ${DISTSRC:?not set} - OUTDIR: ${OUTDIR:?not set} -EOF - -ACTUAL_OUTDIR="${OUTDIR}" -OUTDIR="${DISTSRC}/output" - -##################### -# Environment Setup # -##################### - -# The depends folder also serves as a base-prefix for depends packages for -# $HOSTs after successfully building. -BASEPREFIX="${PWD}/depends" - -# Given a package name and an output name, return the path of that output in our -# current guix environment -store_path() { - grep --extended-regexp "/[^-]{32}-${1}-[^-]+${2:+-${2}}" "${GUIX_ENVIRONMENT}/manifest" \ - | head --lines=1 \ - | sed --expression='s|\x29*$||' \ - --expression='s|^[[:space:]]*"||' \ - --expression='s|"[[:space:]]*$||' -} - +# shellcheck source=setup.sh +source "$(dirname "${BASH_SOURCE[0]}")/setup.sh" # Set environment variables to point the NATIVE toolchain to the right # includes/libs NATIVE_GCC="$(store_path gcc-toolchain)" -unset LIBRARY_PATH -unset CPATH -unset C_INCLUDE_PATH -unset CPLUS_INCLUDE_PATH -unset OBJC_INCLUDE_PATH -unset OBJCPLUS_INCLUDE_PATH - # Set native toolchain build_CC="${NATIVE_GCC}/bin/gcc -isystem ${NATIVE_GCC}/include" build_CXX="${NATIVE_GCC}/bin/g++ -isystem ${NATIVE_GCC}/include/c++ -isystem ${NATIVE_GCC}/include" @@ -134,15 +74,6 @@ for p in "${PATHS[@]}"; do fi done -# Disable Guix ld auto-rpath behavior -export GUIX_LD_WRAPPER_DISABLE_RPATH=yes - -# Make /usr/bin if it doesn't exist -[ -e /usr/bin ] || mkdir -p /usr/bin - -# Symlink env to a conventional path -[ -e /usr/bin/env ] || ln -s --no-dereference "$(command -v env)" /usr/bin/env - # Determine the correct value for -Wl,--dynamic-linker for the current $HOST case "$HOST" in *linux*) @@ -186,20 +117,6 @@ case "$HOST" in ;; esac -########################### -# Source Tarball Building # -########################### - -GIT_ARCHIVE="${DIST_ARCHIVE_BASE}/${DISTNAME}.tar.gz" - -# Create the source tarball if not already there -if [ ! -e "$GIT_ARCHIVE" ]; then - mkdir -p "$(dirname "$GIT_ARCHIVE")" - git archive --prefix="${DISTNAME}/" --output="$GIT_ARCHIVE" HEAD -fi - -mkdir -p "$OUTDIR" - ########################### # Binary Tarball Building # ########################### diff --git a/contrib/guix/libexec/setup.sh b/contrib/guix/libexec/setup.sh new file mode 100755 index 000000000000..617a7327f7bf --- /dev/null +++ b/contrib/guix/libexec/setup.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# Copyright (c) The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://opensource.org/license/mit. +export LC_ALL=C +set -e -o pipefail + +# Environment variables for determinism +export TAR_OPTIONS="--no-same-owner --owner=0 --group=0 --numeric-owner --mtime='@${SOURCE_DATE_EPOCH}' --sort=name" +export TZ=UTC + +# Although Guix _does_ set umask when building its own packages (in our case, +# this is all packages in manifest.scm), it does not set it for `guix +# shell`. It does make sense for at least `guix shell --container` +# to set umask, so if that change gets merged upstream and we bump the +# time-machine to a commit which includes the aforementioned change, we can +# remove this line. +# +# This line should be placed before any commands which creates files. +umask 0022 + +if [ -n "$V" ]; then + # Print both unexpanded (-v) and expanded (-x) forms of commands as they are + # read from this file. + set -vx + # Set VERBOSE for CMake-based builds + export VERBOSE="$V" +fi + +# Check that required environment variables are set +cat << EOF +Required environment variables as seen inside the container: + DIST_ARCHIVE_BASE: ${DIST_ARCHIVE_BASE:?not set} + DISTNAME: ${DISTNAME:?not set} + HOST: ${HOST:?not set} + SOURCE_DATE_EPOCH: ${SOURCE_DATE_EPOCH:?not set} + JOBS: ${JOBS:?not set} + DISTSRC: ${DISTSRC:?not set} + OUTDIR: ${OUTDIR:?not set} +EOF + +export ACTUAL_OUTDIR="${OUTDIR}" +export OUTDIR="${DISTSRC}/output" + +##################### +# Environment Setup # +##################### + +# The depends folder also serves as a base-prefix for depends packages for +# $HOSTs after successfully building. +export BASEPREFIX="${PWD}/depends" + +# Given a package name and an output name, return the path of that output in our +# current guix environment +store_path() { + grep --extended-regexp "/[^-]{32}-${1}-[^-]+${2:+-${2}}" "${GUIX_ENVIRONMENT}/manifest" \ + | head --lines=1 \ + | sed --expression='s|\x29*$||' \ + --expression='s|^[[:space:]]*"||' \ + --expression='s|"[[:space:]]*$||' +} + +# Disable Guix ld auto-rpath behavior +export GUIX_LD_WRAPPER_DISABLE_RPATH=yes + +# Make /usr/bin if it doesn't exist +[ -e /usr/bin ] || mkdir -p /usr/bin + +# Symlink env to a conventional path +[ -e /usr/bin/env ] || ln -s --no-dereference "$(command -v env)" /usr/bin/env + +########################### +# Source Tarball Building # +########################### + +GIT_ARCHIVE="${DIST_ARCHIVE_BASE}/${DISTNAME}.tar.gz" + +# Create the source tarball if not already there +if [ ! -e "$GIT_ARCHIVE" ]; then + mkdir -p "$(dirname "$GIT_ARCHIVE")" + git archive --prefix="${DISTNAME}/" --output="$GIT_ARCHIVE" HEAD +fi + +mkdir -p "$OUTDIR" + +unset LIBRARY_PATH +unset CPATH +unset C_INCLUDE_PATH +unset CPLUS_INCLUDE_PATH +unset OBJC_INCLUDE_PATH +unset OBJCPLUS_INCLUDE_PATH From 4ba948768aae930dc7cd3a9e2abcc49cbaef7497 Mon Sep 17 00:00:00 2001 From: merge-script Date: Wed, 10 Jun 2026 17:05:07 +0200 Subject: [PATCH 05/20] Merge bitcoin/bitcoin#35478: fuzz: reset the mockable steady clock between iterations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 19b32a2e18017c3bf8ea12d25d48b17347f490b6 fuzz: reset the mockable steady clock between iterations (Hao Xu) Pull request description: Fix the issue mentioned by https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-4422112607 And this is my investigation on it: https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-4639472489 `CheckGlobalsImpl`'s constructor runs at the start of every fuzz iteration and already resets the global RNG flags and the mockable `NodeClock` (`SetMockTime(0s)`), but it never reset the mockable steady clock. A value written to `g_mock_steady_time` by one input therefore leaks into the next iteration. The most common source is `FuzzedSock`'s constructor, which calls `SetMockTime(INITIAL_MOCK_TIME)` (through `ElapseTime(0s)`) and never clears it: once any input constructs a `FuzzedSock`, the steady clock stays mocked for every subsequent iteration in the same process. This is one of the global-state leaks tracked in #29018. ### Fix Reset `MockableSteadyClock` symmetrically with `NodeClock`: ```diff g_used_system_time = false; SetMockTime(0s); +MockableSteadyClock::ClearMockTime(); ``` Besides removing the leak, this puts the steady clock under the same discipline as the system clock: a target that reads `MockableSteadyClock::now()` without first mocking it (via `FuzzedSock`, `SteadyClockContext`, …) is now caught by the existing `g_used_system_time` check at the end of the iteration, instead of silently reusing a value left over from a previous input. Clearing in `~FuzzedSock()` would be wrong: several `FuzzedSock`s can be alive simultaneously (e.g. `process_messages` adds 1–3 peers), so clearing in one destructor would corrupt the mock observed by the others. Resetting at the iteration boundary keeps it decoupled from socket lifetimes. ### Testing Verified with the global-state-detector approach from #29018 (snapshotting/diffing the writable globals around each iteration): - **Before:** a single empty input to `process_message` reports `g_mock_steady_time` changing `00 → 01` (`0` → `INITIAL_MOCK_TIME`). - **After:** that report is gone; the only remaining diffs are the benign one-time initialization of `ConsumeTime`'s function-local statics. `p2p_headers_presync` (uses `SteadyClockContext`) and `pcp_request_port_map` (uses `FuzzedSock`) still run to `succeeded` without aborting, confirming existing steady-clock readers are unaffected. This leak is invisible to coverage-based checks such as `deterministic-fuzz-coverage`, because `g_mock_steady_time` is only consumed through coarse time comparisons (e.g. the 250 ms presync rate-limiter): a changed value doesn't change the executed branches, so only a memory-diffing detector can see it. ACKs for top commit: maflcko: lgtm ACK 19b32a2e18017c3bf8ea12d25d48b17347f490b6 marcofleon: Nice catch, ACK 19b32a2e18017c3bf8ea12d25d48b17347f490b6 Tree-SHA512: b875795addb2914eae489adc703438483f8e464b9a210bd5d76189f13266dae5843c8749590d59e78bf171f19aa7cee21ca678cd311843d8a88cbe9831f20b6a (cherry picked from commit c85e04f0791abd58f0153f4f9dbe897419a1ffe6) --- src/test/fuzz/util/check_globals.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/fuzz/util/check_globals.cpp b/src/test/fuzz/util/check_globals.cpp index ca3111534d3b..87d96ebd9726 100644 --- a/src/test/fuzz/util/check_globals.cpp +++ b/src/test/fuzz/util/check_globals.cpp @@ -19,6 +19,7 @@ struct CheckGlobalsImpl { g_seeded_g_prng_zero = false; g_used_system_time = false; SetMockTime(0s); + MockableSteadyClock::ClearMockTime(); } ~CheckGlobalsImpl() { From c5e9528cd224724f27026c0dbe032bf23632d0c2 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 10 Jun 2026 10:47:16 -0700 Subject: [PATCH 06/20] Merge bitcoin/bitcoin#35254: crypto: cleanse HMAC stack buffers after use and ChainCode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 21a1380c13470d28f53cc0b85d902693365d39d3 key: cleanse ChainCode on destruction (Thomas) b3a3f88346dfd218a049acec6a77166f319c70e8 crypto: cleanse HMAC stack buffers after use (Thomas) Pull request description: `CHMAC_SHA256` and `CHMAC_SHA512` leave two stack buffers populated on return: `rkey[]` holds `K' ⊕ ipad` after the constructor, and `temp[]` holds the inner-hash output after `Finalize()`. When the HMAC is keyed with sensitive material (chain code in `BIP32Hash()` in `hash.cpp` for BIP32 child key derivation; PRK in HKDF-Expand in `hkdf_sha256_32.cpp`, used for BIP324 transport keying), `rkey` is one constant XOR from that key, and `temp` is a one-way digest covering it. This PR cleanses both buffers with `memory_cleanse()`, matching the convention already used in `chacha20.cpp` and `chacha20poly1305.cpp`. No observable change for callers. Update: Cleansing the HMAC primitive's internal buffers still leaves a caller's `ChainCode` value populated in memory after use. The second commit promotes `ChainCode` from `typedef uint256` to a `base_blob<256>` subclass with a `memory_cleanse()` destructor, so chain codes in `CExtKey`, `CExtPubKey`, and local variables are cleansed on scope exit. `MUSIG_CHAINCODE` is retyped from `constexpr uint256` to `const ChainCode` to match its BIP328 semantic role; this also removes the GCC-14 consteval lambda workaround. ACKs for top commit: davidgumberg: crACK https://github.com/bitcoin/bitcoin/pull/35254/commits/21a1380c13470d28f53cc0b85d902693365d39d3 optout21: ACK 21a1380c13470d28f53cc0b85d902693365d39d3 achow101: ACK 21a1380c13470d28f53cc0b85d902693365d39d3 winterrdog: ACK 21a1380c13470d28f53cc0b85d902693365d39d3 Tree-SHA512: 022c8372da3e2c9c269ef55b695d8415241acf64be04692f30da0e682dd1d05178f95601a3bd208573fd0630656b3dedcf6de34a2a3cf794515c0268e710af75 (cherry picked from commit 288018131ec06a0be85c554bf78c5c547102510b) --- src/crypto/hmac_sha256.cpp | 4 ++++ src/crypto/hmac_sha512.cpp | 4 ++++ src/hash.h | 10 +++++++++- src/musig.cpp | 5 +---- src/pubkey.h | 2 -- src/test/fuzz/key.cpp | 4 ++-- 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/crypto/hmac_sha256.cpp b/src/crypto/hmac_sha256.cpp index a95ef70849b5..0796bbeb3271 100644 --- a/src/crypto/hmac_sha256.cpp +++ b/src/crypto/hmac_sha256.cpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -26,6 +27,8 @@ CHMAC_SHA256::CHMAC_SHA256(const unsigned char* key, size_t keylen) for (int n = 0; n < 64; n++) rkey[n] ^= 0x5c ^ 0x36; inner.Write(rkey, 64); + + memory_cleanse(rkey, sizeof(rkey)); } void CHMAC_SHA256::Finalize(unsigned char hash[OUTPUT_SIZE]) @@ -33,4 +36,5 @@ void CHMAC_SHA256::Finalize(unsigned char hash[OUTPUT_SIZE]) unsigned char temp[32]; inner.Finalize(temp); outer.Write(temp, 32).Finalize(hash); + memory_cleanse(temp, sizeof(temp)); } diff --git a/src/crypto/hmac_sha512.cpp b/src/crypto/hmac_sha512.cpp index f37e709d13cf..0a9d1041a67d 100644 --- a/src/crypto/hmac_sha512.cpp +++ b/src/crypto/hmac_sha512.cpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -26,6 +27,8 @@ CHMAC_SHA512::CHMAC_SHA512(const unsigned char* key, size_t keylen) for (int n = 0; n < 128; n++) rkey[n] ^= 0x5c ^ 0x36; inner.Write(rkey, 128); + + memory_cleanse(rkey, sizeof(rkey)); } void CHMAC_SHA512::Finalize(unsigned char hash[OUTPUT_SIZE]) @@ -33,4 +36,5 @@ void CHMAC_SHA512::Finalize(unsigned char hash[OUTPUT_SIZE]) unsigned char temp[64]; inner.Finalize(temp); outer.Write(temp, 64).Finalize(hash); + memory_cleanse(temp, sizeof(temp)); } diff --git a/src/hash.h b/src/hash.h index 34486af64a1d..b671761fbb6e 100644 --- a/src/hash.h +++ b/src/hash.h @@ -13,12 +13,20 @@ #include #include #include +#include #include #include #include -typedef uint256 ChainCode; +/** A BIP32 chain code. Cleansed on destruction. */ +class ChainCode : public base_blob<256> { +public: + constexpr ChainCode() = default; + constexpr explicit ChainCode(std::span vch) : base_blob<256>(vch) {} + constexpr explicit ChainCode(const base_blob<256>& b) : base_blob<256>(b) {} + ~ChainCode() { memory_cleanse(data(), size()); } +}; /** A hasher class for Bitcoin's 256-bit hash (double SHA-256). */ class CHash256 { diff --git a/src/musig.cpp b/src/musig.cpp index d211790c43b2..d187ad001338 100644 --- a/src/musig.cpp +++ b/src/musig.cpp @@ -11,10 +11,7 @@ //! MuSig2 chaincode as defined by BIP 328 using namespace util::hex_literals; -constexpr uint256 MUSIG_CHAINCODE{ - // Use immediate lambda to work around GCC-14 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117966 - []() consteval { return uint256{"868087ca02a6f974c4598924c36b57762d32cb45717167e300622c7167e38965"_hex_u8}; }(), -}; +const ChainCode MUSIG_CHAINCODE{"868087ca02a6f974c4598924c36b57762d32cb45717167e300622c7167e38965"_hex_u8}; static bool GetMuSig2KeyAggCache(const std::vector& pubkeys, secp256k1_musig_keyagg_cache& keyagg_cache) { diff --git a/src/pubkey.h b/src/pubkey.h index 02ad7371a796..0391609ebcc3 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -27,8 +27,6 @@ class CKeyID : public uint160 explicit CKeyID(const uint160& in) : uint160(in) {} }; -typedef uint256 ChainCode; - /** An encapsulated public key. */ class CPubKey { diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index b774df43d5d3..57d2e5bd9cba 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -84,7 +84,7 @@ FUZZ_TARGET(key, .init = initialize_key) { CKey child_key; ChainCode child_chaincode; - const bool ok = key.Derive(child_key, child_chaincode, 0, random_uint256); + const bool ok = key.Derive(child_key, child_chaincode, 0, ChainCode{random_uint256}); assert(ok); assert(child_key.IsValid()); assert(!(child_key == key)); @@ -274,7 +274,7 @@ FUZZ_TARGET(key, .init = initialize_key) { CPubKey child_pubkey; ChainCode child_chaincode; - const bool ok = pubkey.Derive(child_pubkey, child_chaincode, 0, random_uint256); + const bool ok = pubkey.Derive(child_pubkey, child_chaincode, 0, ChainCode{random_uint256}); assert(ok); assert(child_pubkey != pubkey); assert(child_pubkey.IsCompressed()); From 51ebebfea5b34f64a2af49fa0e79dcf626cffaa5 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 10 Jun 2026 11:04:39 -0700 Subject: [PATCH 07/20] Merge bitcoin/bitcoin#35101: refactor: disable default std::hash for CTransactionRef a9301cfa0730d1d08c9e982f990dfc8aa0b3a27a refactor: disable default std::hash for CTransactionRef (Sjors Provoost) 47d68cd981fb2985acca0b4d2b8d79dbdccb136d ci: backport iwyu PR 2013 std::hash mapping (Sjors Provoost) Pull request description: While working on #33922 I initially forgot to add `CTransactionRefComp` to the `std::unordered_map`, while a manual build on Ubuntu (version 0.26 with clang version 22.1.1) wants ``. Various workarounds were discussed in: - https://github.com/include-what-you-use/include-what-you-use/issues/2007 - https://github.com/bitcoin/bitcoin/pull/35073 - https://github.com/bitcoin/bitcoin/pull/33922#discussion_r3100806462 Addressed by back-porting: - https://github.com/include-what-you-use/include-what-you-use/pull/2013 ACKs for top commit: achow101: ACK a9301cfa0730d1d08c9e982f990dfc8aa0b3a27a vasild: ACK a9301cfa0730d1d08c9e982f990dfc8aa0b3a27a w0xlt: lgtm ACK a9301cfa0730d1d08c9e982f990dfc8aa0b3a27a Tree-SHA512: 11b3f8698e66457a14d1c16f55cac3ee17a572e9c1c98f5aa9c8a8f1e8928246b1676f7544f2819bc16ec4dcf025585b9cbd60ab3f20839d3d9bcc65ee9e7c0f (cherry picked from commit 3bbc3c67ad49b2a3754097af32b3b77000a0de03) --- ci/test/01_base_install.sh | 5 +++- ci/test/02_iwyu_hash.patch | 44 ++++++++++++++++++++++++++++++++++++ ci/test_imagefile | 3 ++- src/primitives/transaction.h | 12 ++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 ci/test/02_iwyu_hash.patch diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 8b10ba10baac..54dafbe8fd40 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -89,7 +89,10 @@ fi if [[ "${RUN_IWYU}" == true ]]; then ${CI_RETRY_EXE} git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_"${IWYU_LLVM_V}" /include-what-you-use - (cd /include-what-you-use && patch -p1 < /ci_container_base/ci/test/01_iwyu.patch) + pushd /include-what-you-use + patch -p1 < /ci_container_base/ci/test/01_iwyu.patch + patch -p1 < /ci_container_base/ci/test/02_iwyu_hash.patch + popd cmake -B /iwyu-build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-"${IWYU_LLVM_V}" -S /include-what-you-use make -C /iwyu-build/ install "$MAKEJOBS" fi diff --git a/ci/test/02_iwyu_hash.patch b/ci/test/02_iwyu_hash.patch new file mode 100644 index 000000000000..12df7a04ed5c --- /dev/null +++ b/ci/test/02_iwyu_hash.patch @@ -0,0 +1,44 @@ +Map std::hash to its providing standard headers. +Backport of https://github.com/include-what-you-use/include-what-you-use/pull/2013 +(commit 52f85e1f4d990f55fc6556d543eb051d79364a16) to the clang_22 release +branch. Drop once the upstream fix lands in the IWYU branch tracked by +ci/test/00_setup_env_native_iwyu.sh. + +--- a/std_symbol_map.inc ++++ b/std_symbol_map.inc +@@ -1054,12 +1054,27 @@ + { "std::has_unique_object_representations_v", kPrivate, "", kPublic }, + { "std::has_virtual_destructor", kPrivate, "", kPublic }, + { "std::has_virtual_destructor_v", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, + { "std::hash>", kPrivate, "", kPublic }, + { "std::hash, :0>>", kPrivate, "", kPublic }, + { "std::hash, :0>>", kPrivate, "", kPublic }, + { "std::hash, :0>>", kPrivate, "", kPublic }, + { "std::hash, :0>>", kPrivate, "", kPublic }, + { "std::hash, :0>>", kPrivate, "", kPublic }, ++{ "std::hash>", kPrivate, "", kPublic }, + { "std::hash>", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, +@@ -1069,6 +1084,7 @@ + { "std::hash>", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, ++{ "std::hash", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, + { "std::hash", kPrivate, "", kPublic }, diff --git a/ci/test_imagefile b/ci/test_imagefile index 908e9a0f5ab4..dac6b5531564 100644 --- a/ci/test_imagefile +++ b/ci/test_imagefile @@ -16,7 +16,8 @@ ENV BASE_ROOT_DIR=${BASE_ROOT_DIR} # Make retry available in PATH, needed for CI_RETRY_EXE COPY ./ci/retry/retry /usr/bin/retry -COPY ./ci/test/00_setup_env.sh ./${FILE_ENV} ./ci/test/01_base_install.sh ./ci/test/01_iwyu.patch /ci_container_base/ci/test/ +COPY ./ci/test/00_setup_env.sh ./${FILE_ENV} ./ci/test/01_base_install.sh /ci_container_base/ci/test/ +COPY ./ci/test/*.patch /ci_container_base/ci/test/ # Bash is required, so install it when missing RUN sh -c "bash -c 'true' || ( apk update && apk add --no-cache bash )" diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 34bb9571c153..3a7735e149b6 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -403,4 +403,16 @@ struct CMutableTransaction typedef std::shared_ptr CTransactionRef; template static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared(std::forward(txIn)); } +namespace std { +/** Disable default std::hash for CTransactionRef to prevent accidentally + * comparing by pointer. Use CTransactionRefHash or provide a custom + * hasher. */ +template <> +struct hash { + hash() = delete; + // Belt-and-suspenders, already implied by the above. + size_t operator()(const CTransactionRef&) const = delete; +}; +} // namespace std + #endif // BITCOIN_PRIMITIVES_TRANSACTION_H From f8ed7ac0af0833e2bcd5747083f859fe89ac7e55 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 10 Jun 2026 11:12:37 -0700 Subject: [PATCH 08/20] Merge bitcoin/bitcoin#34028: p2p: Prevent integer overflow in LocalServiceInfo::nScore 2189a6f5f226d5a2905f1939eb7eea9571502b90 p2p: Saturate LocalServiceInfo::nScore updates at INT_MAX (codeabysss) Pull request description: The overflow for signed arithmetic yields undefined behavior. This changes prevents undefined behavior in local address scoring by saturating `nScore` updates at `INT_MAX` in both `SeenLocal()` and `AddLocal()` update paths. Fixes: #24049. ACKs for top commit: Crypt-iQ: ACK 2189a6f5f226d5a2905f1939eb7eea9571502b90 pending CI achow101: ACK 2189a6f5f226d5a2905f1939eb7eea9571502b90 sedited: ACK 2189a6f5f226d5a2905f1939eb7eea9571502b90 Tree-SHA512: b861e58ec9d6e18b17768f5cbee31ee825717e1a7216c332eb6fcbe63a7ac24e213ba638aea6f03cb710d9c2d8fe736cc626f11011ed66c3938acf6c38b0ef2a (cherry picked from commit 53b836cdcedca79b7504c95b1ba9b6ce7c608b02) --- src/net.cpp | 5 +++-- src/test/net_tests.cpp | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 00e7c1f8ccd2..4e6452a65de3 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -296,7 +297,7 @@ bool AddLocal(const CService& addr_, int nScore) const auto [it, is_newly_added] = mapLocalHost.emplace(addr, LocalServiceInfo()); LocalServiceInfo &info = it->second; if (is_newly_added || nScore >= info.nScore) { - info.nScore = nScore + (is_newly_added ? 0 : 1); + info.nScore = SaturatingAdd(nScore, is_newly_added ? 0 : 1); info.nPort = addr.GetPort(); } } @@ -325,7 +326,7 @@ bool SeenLocal(const CService& addr) LOCK(g_maplocalhost_mutex); const auto it = mapLocalHost.find(addr); if (it == mapLocalHost.end()) return false; - ++it->second.nScore; + it->second.nScore = SaturatingAdd(it->second.nScore, 1); return true; } diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 946d77bbe675..de71b7d62475 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -802,6 +802,41 @@ BOOST_AUTO_TEST_CASE(LocalAddress_BasicLifecycle) BOOST_CHECK(!IsLocal(addr)); } +BOOST_AUTO_TEST_CASE(LocalAddress_nScore_Overflow) +{ + g_reachable_nets.Add(NET_IPV4); + const CService addr{UtilBuildAddress(0x002, 0x001, 0x001, 0x001), 1000}; // 2.1.1.1:1000 + + const auto get_score = [](const CService& service) -> int { + LOCK(g_maplocalhost_mutex); + const auto it = mapLocalHost.find(service); + return it != mapLocalHost.end() ? it->second.nScore : 0; + }; + + const int initial_score = 1000; + BOOST_REQUIRE(AddLocal(addr, initial_score)); + BOOST_REQUIRE(IsLocal(addr)); + BOOST_CHECK_EQUAL(get_score(addr), initial_score); + + // SeenLocal should increment nScore by 1. + BOOST_CHECK(SeenLocal(addr)); + BOOST_CHECK_EQUAL(get_score(addr), initial_score + 1); + + // AddLocal() saturates nScore when updating an existing entry at INT_MAX. + BOOST_REQUIRE(AddLocal(addr, std::numeric_limits::max())); + BOOST_CHECK_EQUAL(get_score(addr), std::numeric_limits::max()); + + BOOST_CHECK(AddLocal(addr, std::numeric_limits::max())); + BOOST_CHECK_EQUAL(get_score(addr), std::numeric_limits::max()); + + // SeenLocal() also saturates at INT_MAX. + BOOST_CHECK(SeenLocal(addr)); + BOOST_CHECK_EQUAL(get_score(addr), std::numeric_limits::max()); + + RemoveLocal(addr); + BOOST_CHECK(!IsLocal(addr)); +} + BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) { LOCK(NetEventsInterface::g_msgproc_mutex); From bff05e19eb62514e66e4c80bf38b6e57d4679b32 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 10 Jun 2026 14:49:00 -0700 Subject: [PATCH 09/20] Merge bitcoin/bitcoin#35451: lint: Grep for `AUTO` test suites in file names f6bdbcf79d9e4b17ef9fc4e254e4ada789be88a1 lint: Grep for `AUTO` test suites in file names (rustaceanrob) Pull request description: Tests without a fixture did not have their file names linted because the grep matches on `BOOST_FIXTURE`. Updates to match `BOOST_FIXTURE` or `BOOST_TEST`. ACKs for top commit: l0rinc: ACK f6bdbcf79d9e4b17ef9fc4e254e4ada789be88a1 achow101: ACK f6bdbcf79d9e4b17ef9fc4e254e4ada789be88a1 hebasto: ACK f6bdbcf79d9e4b17ef9fc4e254e4ada789be88a1. Tree-SHA512: dd1763b6ac90fa87b7e0d2faa56d1c7beedb1e2d37d16367c60ebcadd155f5955113fff7cf5c0ce5eaa9e63aeeb67ffff2c8e081f7c23978cb072207f072f2ef (cherry picked from commit 809f909e5824141aa5038f16ed7484a31fe94ae7) --- src/test/feerounder_tests.cpp | 2 +- test/lint/lint-tests.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/feerounder_tests.cpp b/src/test/feerounder_tests.cpp index 82cd7d65b409..d88a4f771b6c 100644 --- a/src/test/feerounder_tests.cpp +++ b/src/test/feerounder_tests.cpp @@ -9,7 +9,7 @@ #include -BOOST_AUTO_TEST_SUITE(fee_rounder_tests) +BOOST_AUTO_TEST_SUITE(feerounder_tests) BOOST_AUTO_TEST_CASE(FeeRounder) { diff --git a/test/lint/lint-tests.py b/test/lint/lint-tests.py index fb9e0ca80129..4ecd3502acde 100755 --- a/test/lint/lint-tests.py +++ b/test/lint/lint-tests.py @@ -13,12 +13,12 @@ import sys -def grep_boost_fixture_test_suite(): +def grep_boost_test_suites(): command = [ "git", "grep", "-E", - r"^BOOST_FIXTURE_TEST_SUITE\(", + r"^(BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\(", "--", "src/ipc/test/**.cpp", "src/test/**.cpp", @@ -30,7 +30,7 @@ def check_matching_test_names(test_suite_list): not_matching = [ x for x in test_suite_list - if re.search(r"/(.*?)\.cpp:BOOST_FIXTURE_TEST_SUITE\(\1(_[a-z0-9]+)?, .*\)", x) is None + if re.search(r"/(.*?)\.cpp:(?:BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\(\1(_[a-z0-9]+)?[,)]", x) is None ] if len(not_matching) > 0: not_matching = "\n".join(not_matching) @@ -60,7 +60,7 @@ def get_duplicates(input_list): def check_unique_test_names(test_suite_list): - output = [re.search(r"\((.*?),", x) for x in test_suite_list] + output = [re.search(r"\((.*?)[,)]", x) for x in test_suite_list] output = [x.group(1) for x in output if x is not None] output = get_duplicates(output) output = sorted(list(output)) @@ -77,7 +77,7 @@ def check_unique_test_names(test_suite_list): def main(): - test_suite_list = grep_boost_fixture_test_suite().splitlines() + test_suite_list = grep_boost_test_suites().splitlines() exit_code = check_matching_test_names(test_suite_list) exit_code |= check_unique_test_names(test_suite_list) sys.exit(exit_code) From cec1d93622bd283693fe4fa2fe06c699d333d0dd Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 11 Jun 2026 09:22:29 +0200 Subject: [PATCH 10/20] Merge bitcoin/bitcoin#35489: fuzz: test non-max descriptor satisfaction weight 526aae3768df09b2d39621bc19cd18a388f03e42 fuzz: test non-max descriptor satisfaction weight (woltx) Pull request description: The descriptor fuzz target is intended to exercise descriptor satisfaction-size estimation for solvable descriptors. It currently calls `MaxSatisfactionWeight(true)` twice, so the `false` branch is never exercised. This PR changes `max_sat_nonmaxsig` to call `MaxSatisfactionWeight(false)`, so fuzzing covers both branches. ACKs for top commit: brunoerg: reACK 526aae3768df09b2d39621bc19cd18a388f03e42 sedited: ACK 526aae3768df09b2d39621bc19cd18a388f03e42 Tree-SHA512: 029750d76c1d50f5c6a008b826a0a2dc187feb420be96401d2e15747b44901341d32ac75e86a5e10585919d419c607800d8e117a1cbce50b1db40121d3610f9c (cherry picked from commit e0fb41fd2a159b954467a3f59c3a98958969dba1) --- src/test/fuzz/descriptor_parse.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index 6b3084e23d15..2e73cfcc438a 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -55,7 +55,11 @@ static void TestDescriptor(const Descriptor& desc, FlatSigningProvider& sig_prov } const auto max_sat_maxsig{desc.MaxSatisfactionWeight(true)}; - const auto max_sat_nonmaxsig{desc.MaxSatisfactionWeight(true)}; + const auto max_sat_nonmaxsig{desc.MaxSatisfactionWeight(false)}; + // Whether an estimate is available must not depend on the signature-size + // assumption, and assuming non-max-size signatures must never increase it. + assert(max_sat_maxsig.has_value() == max_sat_nonmaxsig.has_value()); + assert(max_sat_nonmaxsig <= max_sat_maxsig); const auto max_elems{desc.MaxSatisfactionElems()}; // We must be able to estimate the max satisfaction size for any solvable descriptor (but combo). const bool is_nontop_or_nonsolvable{!*is_solvable || !desc.GetOutputType()}; From 95b458240bfa9c830f60e9bd366b35cbc1125068 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 11 Jun 2026 09:47:12 +0200 Subject: [PATCH 11/20] Merge bitcoin/bitcoin#35455: fuzz: improve dbwrapper_concurrent_reads performance 1ce9e262395bb17e03a7203efab0ef5ae168459b fuzz: improve dbwrapper_concurrent_reads performance (Andrew Toth) Pull request description: The recently merged fuzz harness targeting concurrent reads suffers from poor performance and memory leaks (https://github.com/bitcoin/bitcoin/pull/34866#issuecomment-4614323925). Fix this by - using a global thread pool instead of a local one per iteration - reduce thread count to 8 from 16 - use a std::map oracle to check results inline instead of reading from the db to get a baseline and storing results ACKs for top commit: marcofleon: reACK 1ce9e262395bb17e03a7203efab0ef5ae168459b l0rinc: ACK 1ce9e262395bb17e03a7203efab0ef5ae168459b sedited: ACK 1ce9e262395bb17e03a7203efab0ef5ae168459b Tree-SHA512: 2e532caf246f389105e4a9b487496386d1fe9add7b27fba9ecbbf51a432ef493765ad7095288dd7e0a896860ff150d89ecb6afb8baf311a4af94d8e01b77dba5 (cherry picked from commit d0b8d445fb7a936d52f88c7d526184f97e462fa2) --- src/test/fuzz/dbwrapper.cpp | 113 ++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 51 deletions(-) diff --git a/src/test/fuzz/dbwrapper.cpp b/src/test/fuzz/dbwrapper.cpp index 784b696d8166..1b5cd7b025d0 100644 --- a/src/test/fuzz/dbwrapper.cpp +++ b/src/test/fuzz/dbwrapper.cpp @@ -167,7 +167,16 @@ void VerifyIterator(CDBWrapper& dbw, const Oracle& oracle, } /** Maximum number of concurrent reader threads in dbwrapper_concurrent_reads. */ -constexpr size_t MAX_READ_WORKERS{16}; +constexpr size_t MAX_READ_WORKERS{8}; + +ThreadPool g_read_pool{"dbfuzz"}; +Mutex g_read_pool_mutex; + +void StartReadPoolIfNeeded() EXCLUSIVE_LOCKS_REQUIRED(!g_read_pool_mutex) +{ + LOCK(g_read_pool_mutex); + if (!g_read_pool.WorkersCount()) g_read_pool.Start(MAX_READ_WORKERS); +} /** Build randomized DBParams from the fuzz input, shared by all targets. */ DBParams ConsumeDBParams(FuzzedDataProvider& provider, leveldb::Env* testing_env, @@ -185,39 +194,6 @@ DBParams ConsumeDBParams(FuzzedDataProvider& provider, leveldb::Env* testing_env }; } -/** A single read-only operation run concurrently in dbwrapper_concurrent_reads. */ -enum class ReadOp { Read, Exists, IteratorSeek }; -using ReadQuery = std::tuple; -using Results = std::vector>; - -Results RunReadQueries(CDBWrapper& db, const std::vector& queries, FastRandomContext& rng) -{ - std::vector order(queries.size()); - std::iota(order.begin(), order.end(), size_t{0}); - std::shuffle(order.begin(), order.end(), rng); - - Results results(queries.size()); - for (const auto i : order) { - const auto& [op, key] = queries[i]; - std::string v; - switch (op) { - case ReadOp::Read: - if (db.Read(key, v)) results[i] = std::move(v); - break; - case ReadOp::Exists: - if (db.Exists(key)) results[i] = std::move(v); - break; - case ReadOp::IteratorSeek: { - const std::unique_ptr it{db.NewIterator()}; - it->Seek(key); - if (it->Valid() && it->GetValue(v)) results[i] = std::move(v); - break; - } - } - } - return results; -} - template void TestDbWrapper(FuzzedDataProvider& provider, leveldb::Env* testing_env, @@ -381,8 +357,9 @@ FUZZ_TARGET(dbwrapper_threaded, .init = [] { static auto setup{MakeNoLogFileCont /*allow_force_compact=*/true); } -FUZZ_TARGET(dbwrapper_concurrent_reads, .init = [] { static auto setup{MakeNoLogFileContext<>()}; }) +FUZZ_TARGET(dbwrapper_concurrent_reads, .init = [] { static auto setup{MakeNoLogFileContext<>()}; }) EXCLUSIVE_LOCKS_REQUIRED(!g_read_pool_mutex) { + StartReadPoolIfNeeded(); SeedRandomStateForTest(SeedRand::ZEROS); FuzzedDataProvider provider{buffer.data(), buffer.size()}; @@ -394,17 +371,20 @@ FUZZ_TARGET(dbwrapper_concurrent_reads, .init = [] { static auto setup{MakeNoLog // Seed the DB. Drain work after small batches so we don't deadlock on a // scheduled compaction. - const size_t num_entries{provider.ConsumeIntegralInRange(100, 5'000)}; + const size_t num_entries{provider.ConsumeIntegralInRange(100, 3'000)}; std::vector keys; keys.reserve(num_entries); + Oracle oracle; constexpr size_t SEED_BATCH_SIZE{400}; for (size_t start{0}; start < num_entries; start += SEED_BATCH_SIZE) { CDBBatch batch{db}; const size_t end{std::min(start + SEED_BATCH_SIZE, num_entries)}; for (size_t i{start}; i < end; ++i) { const auto k{ConsumeKey(provider)}; - batch.Write(k, MakeValue(k, ConsumeValueSize(provider))); + const auto size{ConsumeValueSize(provider)}; + batch.Write(k, MakeValue(k, size)); keys.push_back(k); + oracle[k] = size; } det_env.DrainWork(); db.WriteBatch(batch, /*fSync=*/true); @@ -414,40 +394,71 @@ FUZZ_TARGET(dbwrapper_concurrent_reads, .init = [] { static auto setup{MakeNoLog // Build query list from seeded and random keys. const size_t num_queries{provider.ConsumeIntegralInRange(1, 2'000)}; - std::vector queries; + enum class ReadOp { Read, Exists, IteratorSeek }; + std::vector> queries; queries.reserve(num_queries); for (size_t i{0}; i < num_queries; ++i) { - const auto op{static_cast(provider.ConsumeIntegralInRange(0, 2))}; + const auto op{provider.PickValueInArray({ReadOp::Read, ReadOp::Exists, ReadOp::IteratorSeek})}; const uint16_t key{provider.ConsumeBool() ? keys[provider.ConsumeIntegralInRange(0, keys.size() - 1)] : ConsumeKey(provider)}; queries.emplace_back(op, key); } - // Baseline read on a single thread - FastRandomContext rng{ConsumeUInt256(provider)}; - const Results baseline{RunReadQueries(db, queries, rng)}; - - ThreadPool pool{"dbfuzz"}; - pool.Start(MAX_READ_WORKERS); // Workers + main thread synchronize on the latch so all reads start together. std::latch start_latch{static_cast(MAX_READ_WORKERS + 1)}; - std::vector> tasks(MAX_READ_WORKERS); - std::generate(tasks.begin(), tasks.end(), [&] { - return [&, seed = rng.rand256()]() -> Results { + std::vector> tasks(MAX_READ_WORKERS); + FastRandomContext rng{ConsumeUInt256(provider)}; + std::ranges::generate(tasks, [&] { + return [&, seed = rng.rand256()] { FastRandomContext thread_rng{seed}; + std::vector order(queries.size()); + std::iota(order.begin(), order.end(), size_t{0}); + std::ranges::shuffle(order, thread_rng); + std::vector v; + std::string key_str; start_latch.arrive_and_wait(); - return RunReadQueries(db, queries, thread_rng); + const std::unique_ptr it{db.NewIterator()}; + // Every read must agree with the oracle, the source of truth. + for (const auto i : order) { + const auto& [op, key] = queries[i]; + switch (op) { + case ReadOp::Read: + if (const auto oit{oracle.find(key)}; oit != oracle.end()) { + assert(db.Read(key, v) && v == MakeValue(key, oit->second)); + } else { + assert(!db.Read(key, v)); + } + break; + case ReadOp::Exists: + assert(db.Exists(key) == oracle.contains(key)); + break; + case ReadOp::IteratorSeek: + it->Seek(key); + // Skip the obfuscation metadata entry (a non-uint16_t key) if we land + // on it, so the result matches the oracle, which only tracks user keys. + if (it->Valid() && it->GetKey(key_str) && key_str == OBFUSCATION_KEY) it->Next(); + if (const auto oit{oracle.lower_bound(key)}; oit != oracle.end()) { + assert(it->Valid()); + uint16_t actual_key; + assert(it->GetKey(actual_key) && actual_key == oit->first); + assert(it->GetValue(v) && v == MakeValue(actual_key, oit->second)); + } else { + assert(!it->Valid()); + } + break; + } + } }; }); - auto futures{*Assert(pool.Submit(std::move(tasks)))}; + auto futures{*Assert(g_read_pool.Submit(std::move(tasks)))}; // Release the workers and immediately run the queued compaction on this // thread, so compaction races against the concurrent reads. start_latch.arrive_and_wait(); det_env.DrainWork(); - for (auto& fut : futures) assert(fut.get() == baseline); + for (auto& fut : futures) fut.get(); det_env.DrainWork(); } From 3116a792001665fc3c602d5af153a5c3bd852c63 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 11 Jun 2026 13:35:40 +0200 Subject: [PATCH 12/20] Merge bitcoin/bitcoin#35120: btcsignals: delete broken scoped_connection move assignment b83a999b1449c0b0b2eac3173359539323438fde btcsignals: delete broken scoped_connection move assignment (Thomas) Pull request description: The defaulted move-assignment operator for `scoped_connection` overwrites `m_conn` without first calling `disconnect()`. Since disconnection is signaled via the liveness flag (which is never cleared) the old callback remains registered in the signal and keeps firing, violating the RAII contract: ```cpp int val{0}; btcsignals::scoped_connection sc0 = sig.connect(IncrementCallback); btcsignals::scoped_connection sc1 = sig.connect(SquareCallback); sc0 = std::move(sc1); val = 3; sig(val); // Expected: 9 (only SquareCallback) // Actual: 16 (both callbacks fire, old connection leaked) ``` Move assignment is unused in the codebase, so following the review discussion this deletes the broken operator instead of fixing it. A correct implementation can be added if a use case arises. Earlier versions of this PR fixed the operator and added a default constructor to enable the member-variable assignment pattern; that was dropped in favor of removing the unused operation. ACKs for top commit: maflcko: lgtm ACK b83a999b1449c0b0b2eac3173359539323438fde sedited: ACK b83a999b1449c0b0b2eac3173359539323438fde Tree-SHA512: 794347e9cb868d50957ea298f7df6eac5b9f55b9d35ab09e41be269923c45e0709194431dea66b7977c74f802150ba53cb2d12d35937f4966ec302bffb9c95f8 (cherry picked from commit 3b712b9d0245d0c1692224292123a9601a5ca96b) --- src/btcsignals.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/btcsignals.h b/src/btcsignals.h index ebe7a5acb783..9eeca7699618 100644 --- a/src/btcsignals.h +++ b/src/btcsignals.h @@ -123,11 +123,11 @@ class scoped_connection scoped_connection(connection rhs) noexcept : m_conn{std::move(rhs)} {} scoped_connection(scoped_connection&&) noexcept = default; - scoped_connection& operator=(scoped_connection&&) noexcept = default; /** - * For simplicity, disable copy assignment and construction. + * For simplicity, disable copy construction and copy/move assignment. */ + scoped_connection& operator=(scoped_connection&&) = delete; scoped_connection& operator=(const scoped_connection&) = delete; scoped_connection(const scoped_connection&) = delete; From 333aa8c05f5b30d436cf1202b15ffc6194e90e03 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 11 Jun 2026 16:35:50 +0200 Subject: [PATCH 13/20] Merge bitcoin/bitcoin#35514: ci: Alpine 3.24 3be1115ade347dfdf9d0074f554034e100eba4fe ci: Alpine 3.24 (fanquake) Pull request description: Switch to [Alpine 3.24](https://www.alpinelinux.org/posts/Alpine-3.24.0-released.html) in the Alpine CI job. ACKs for top commit: maflcko: lgtm ACK 3be1115ade347dfdf9d0074f554034e100eba4fe sedited: ACK 3be1115ade347dfdf9d0074f554034e100eba4fe Tree-SHA512: 735a193a3511da611ac3399a66917cf69d5f895c39e50667c1293f1e449795ae85bb5a9686ce28928f73547369b036d091e1cf7b523854652a10ec7cf529d1d3 (cherry picked from commit c117bbc467f80b4f28bb8c4d97a4181cd714bfc9) --- ci/test/00_setup_env_native_alpine_musl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_alpine_musl.sh b/ci/test/00_setup_env_native_alpine_musl.sh index 4f63472997e9..591b38116c00 100755 --- a/ci/test/00_setup_env_native_alpine_musl.sh +++ b/ci/test/00_setup_env_native_alpine_musl.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_alpine_musl -export CI_IMAGE_NAME_TAG="mirror.gcr.io/alpine:3.23" +export CI_IMAGE_NAME_TAG="mirror.gcr.io/alpine:3.24" export CI_BASE_PACKAGES="build-base musl-dev pkgconf curl ccache make ninja git python3-dev py3-pip which patch xz procps rsync util-linux bison e2fsprogs cmake dash linux-headers" export PIP_PACKAGES="--break-system-packages pycapnp" export DEP_OPTS="DEBUG=1" From b32f944dccd4b533811a428206d25f2858b0fca2 Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 8 Jun 2026 19:30:36 +0200 Subject: [PATCH 14/20] Merge bitcoin/bitcoin#35359: blockstorage: Remove cs_LastBlockFile recursive mutex ec6cf49b91f726aa8cb70087eb3fcacb14b4ff22 blockstorage: Remove cs_LastBlockFile recursive mutex (sedited) Pull request description: The `cs_LastBlockFile` mutex is redundant: all critical sections are already covered by cs_main. This is demonstrated in this patch by replacing all instances of locking `cs_LastBlockFile` with pairs of `AssertLockHeld(::cs_main)` and `EXCLUSIVE_LOCKS_REQUIRED(::cs_main)` annotations. No additional `::cs_main` LOCK(...)s are introduced (besides for test-only code). It is also not clear for which sections `cs_LastBlockFile` is responsible for. It is annotated for `m_blockfile_cursors`, but sporadically and inconsistently also covers `m_blockfile_info` (e.g. in `LoadBlockIndexDB`). Since it has no semantic meaning, and seems confusing to developers, remove it. An alternative to this patch would be expanding the scope of what `cs_LastBlockFile` covers and turning it into a non-recursive mutex. I prepared such a patch some time ago, but found it unsatisfactory. It was not clear to me if the lock was now covering too much or too little, and its purpose remained unclear. If this patch is accepted, I would expect the project to eventually implement a separate, narrowly-scoped block storage lock to allow for a more parallelizable block processing routine. ACKs for top commit: stickies-v: re-ACK ec6cf49b91f726aa8cb70087eb3fcacb14b4ff22 janb84: re- ACK ec6cf49b91f726aa8cb70087eb3fcacb14b4ff22 pablomartin4btc: ACK ec6cf49b91f726aa8cb70087eb3fcacb14b4ff22 Tree-SHA512: e5942bc87300b0db9a0b91d5fe26dab455049e6cef7c96bb12b28141fa04711d46c6af105c0e1a83a9f261edde2c8b8b43ecf577a27d54b4610d784676a85627 --- src/bench/readwriteblock.cpp | 5 +++-- src/chainstate.cpp | 1 - src/node/blockstorage.cpp | 31 +++++++++++++------------------ src/node/blockstorage.h | 23 +++++++++++------------ src/test/blockmanager_tests.cpp | 2 ++ 5 files changed, 29 insertions(+), 33 deletions(-) diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp index 87551eb9a8e0..ef567931b230 100644 --- a/src/bench/readwriteblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -32,6 +32,7 @@ static void WriteBlockBench(benchmark::Bench& bench) auto& blockman{testing_setup->m_node.chainman->m_blockman}; const CBlock block{CreateTestBlock()}; bench.run([&] { + LOCK(::cs_main); const auto pos{blockman.WriteBlock(block, 413'567)}; assert(!pos.IsNull()); }); @@ -43,7 +44,7 @@ static void ReadBlockBench(benchmark::Bench& bench) auto& blockman{testing_setup->m_node.chainman->m_blockman}; const auto& test_block{CreateTestBlock()}; const auto& expected_hash{test_block.GetHash()}; - const auto& pos{blockman.WriteBlock(test_block, 413'567)}; + const auto& pos{WITH_LOCK(::cs_main, return blockman.WriteBlock(test_block, 413'567))}; bench.run([&] { CBlock block; const auto success{blockman.ReadBlock(block, pos, expected_hash)}; @@ -55,7 +56,7 @@ static void ReadRawBlockBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; auto& blockman{testing_setup->m_node.chainman->m_blockman}; - const auto pos{blockman.WriteBlock(CreateTestBlock(), 413'567)}; + const auto pos{WITH_LOCK(::cs_main, return blockman.WriteBlock(CreateTestBlock(), 413'567))}; bench.run([&] { const auto res{blockman.ReadRawBlock(pos)}; assert(res); diff --git a/src/chainstate.cpp b/src/chainstate.cpp index 12b81ebc15be..3b6d07ff5599 100644 --- a/src/chainstate.cpp +++ b/src/chainstate.cpp @@ -293,7 +293,6 @@ bool Chainstate::FlushStateToDisk( bool fFlushForPrune = false; CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(); - LOCK(m_blockman.cs_LastBlockFile); if (m_blockman.IsPruneMode() && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && m_chainman.m_blockman.m_blockfiles_indexed) { // make sure we don't prune above any of the prune locks bestblocks // pruning is height-based diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index f5f274cb9620..72f32b2a7047 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -254,7 +254,6 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block, CBlockInde void BlockManager::PruneOneBlockFile(const int fileNumber) { AssertLockHeld(cs_main); - LOCK(cs_LastBlockFile); for (auto& entry : m_block_index) { CBlockIndex* pindex = &entry.second; @@ -292,7 +291,7 @@ void BlockManager::FindFilesToPruneManual( { assert(IsPruneMode() && nManualPruneHeight > 0); - LOCK2(cs_main, cs_LastBlockFile); + LOCK(::cs_main); if (chain.m_chain.Height() < 0) { return; } @@ -320,7 +319,7 @@ void BlockManager::FindFilesToPrune( const Chainstate& chain, ChainstateManager& chainman) { - LOCK2(cs_main, cs_LastBlockFile); + LOCK(::cs_main); // Compute `target` value with maximum size (in bytes) of blocks below the // `last_prune` height which should be preserved and not pruned. const auto target = std::max( @@ -488,12 +487,13 @@ void BlockManager::WriteBlockIndexDB() vBlocks.push_back(*it); m_dirty_blockindex.erase(it++); } - int max_blockfile = WITH_LOCK(cs_LastBlockFile, return this->MaxBlockfileNum()); + int max_blockfile{this->MaxBlockfileNum()}; m_block_tree_db->WriteBatchSync(vFiles, max_blockfile, vBlocks); } bool BlockManager::LoadBlockIndexDB() { + AssertLockHeld(::cs_main); if (!LoadBlockIndex()) { return false; } @@ -533,7 +533,6 @@ bool BlockManager::LoadBlockIndexDB() { // Initialize the blockfile cursor. - LOCK(cs_LastBlockFile); if (!m_blockfile_info.empty()) { m_blockfile_cursor = {static_cast(m_blockfile_info.size() - 1), 0}; } @@ -556,7 +555,7 @@ bool BlockManager::LoadBlockIndexDB() void BlockManager::ScanAndUnlinkAlreadyPrunedFiles() { AssertLockHeld(::cs_main); - int max_blockfile = WITH_LOCK(cs_LastBlockFile, return this->MaxBlockfileNum()); + int max_blockfile{this->MaxBlockfileNum()}; if (!m_have_pruned) { return; } @@ -654,8 +653,7 @@ void BlockManager::CleanupBlockRevFiles() const CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n) { - LOCK(cs_LastBlockFile); - + AssertLockHeld(::cs_main); return &m_blockfile_info.at(n); } @@ -706,8 +704,8 @@ bool BlockManager::FlushUndoFile(int block_file, bool finalize) bool BlockManager::FlushBlockFile(int blockfile_num, bool fFinalize, bool finalize_undo) { + AssertLockHeld(::cs_main); bool success = true; - LOCK(cs_LastBlockFile); if (m_blockfile_info.size() < 1) { // Return if we haven't loaded any blockfiles yet. This happens during @@ -735,14 +733,13 @@ bool BlockManager::FlushBlockFile(int blockfile_num, bool fFinalize, bool finali bool BlockManager::FlushChainstateBlockFile(int tip_height) { - LOCK(cs_LastBlockFile); + AssertLockHeld(::cs_main); return FlushBlockFile(m_blockfile_cursor.file_num, /*fFinalize=*/false, /*finalize_undo=*/false); } uint64_t BlockManager::CalculateCurrentUsage() { - LOCK(cs_LastBlockFile); - + AssertLockHeld(::cs_main); uint64_t retval = 0; for (const CBlockFileInfo& file : m_blockfile_info) { retval += file.nSize + file.nUndoSize; @@ -781,7 +778,7 @@ fs::path BlockManager::GetBlockPosFilename(const FlatFilePos& pos) const FlatFilePos BlockManager::FindNextBlockPos(unsigned int nAddSize, unsigned int nHeight, uint64_t nTime) { - LOCK(cs_LastBlockFile); + AssertLockHeld(::cs_main); const int last_blockfile = m_blockfile_cursor.file_num; @@ -862,8 +859,7 @@ FlatFilePos BlockManager::FindNextBlockPos(unsigned int nAddSize, unsigned int n void BlockManager::UpdateBlockInfo(const CBlock& block, unsigned int nHeight, const FlatFilePos& pos) { - LOCK(cs_LastBlockFile); - + AssertLockHeld(::cs_main); // Update the cursor so it points to the last file. if (m_blockfile_cursor.file_num < pos.nFile) { m_blockfile_cursor = BlockfileCursor{pos.nFile}; @@ -882,10 +878,9 @@ void BlockManager::UpdateBlockInfo(const CBlock& block, unsigned int nHeight, co bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize) { + AssertLockHeld(::cs_main); pos.nFile = nFile; - LOCK(cs_LastBlockFile); - pos.nPos = m_blockfile_info[nFile].nUndoSize; m_blockfile_info[nFile].nUndoSize += nAddSize; m_dirty_fileinfo.insert(nFile); @@ -905,7 +900,6 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) { AssertLockHeld(::cs_main); - LOCK(cs_LastBlockFile); auto& cursor{m_blockfile_cursor}; // Write undo information to disk @@ -1071,6 +1065,7 @@ BlockManager::ReadRawBlockResult BlockManager::ReadRawBlock(const FlatFilePos& p FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) { + AssertLockHeld(::cs_main); const unsigned int block_size{static_cast(GetSerializeSize(TX_WITH_WITNESS(block)))}; FlatFilePos pos{FindNextBlockPos(block_size + STORAGE_HEADER_BYTES, nHeight, block.GetBlockTime())}; if (pos.IsNull()) { diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 99a0c5919210..00a2da5e4117 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -199,7 +199,7 @@ class BlockManager EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Return false if block file or undo file flushing fails. */ - [[nodiscard]] bool FlushBlockFile(int blockfile_num, bool fFinalize, bool finalize_undo); + [[nodiscard]] bool FlushBlockFile(int blockfile_num, bool fFinalize, bool finalize_undo) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Return false if undo file flushing fails. */ [[nodiscard]] bool FlushUndoFile(int block_file, bool finalize = false); @@ -213,9 +213,9 @@ class BlockManager * The nAddSize argument passed to this function should include not just the size of the serialized CBlock, but also the size of * separator fields (STORAGE_HEADER_BYTES). */ - [[nodiscard]] FlatFilePos FindNextBlockPos(unsigned int nAddSize, unsigned int nHeight, uint64_t nTime); - [[nodiscard]] bool FlushChainstateBlockFile(int tip_height); - bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize); + [[nodiscard]] FlatFilePos FindNextBlockPos(unsigned int nAddSize, unsigned int nHeight, uint64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + [[nodiscard]] bool FlushChainstateBlockFile(int tip_height) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + [[nodiscard]] bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); AutoFile OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const; @@ -247,13 +247,12 @@ class BlockManager const Chainstate& chain, ChainstateManager& chainman); - RecursiveMutex cs_LastBlockFile; - //! The current blockfile cursor used for appending new blocks. - BlockfileCursor m_blockfile_cursor GUARDED_BY(cs_LastBlockFile){}; + BlockfileCursor m_blockfile_cursor GUARDED_BY(::cs_main){}; - int MaxBlockfileNum() const EXCLUSIVE_LOCKS_REQUIRED(cs_LastBlockFile) + int MaxBlockfileNum() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); return m_blockfile_cursor.file_num; } @@ -341,7 +340,7 @@ class BlockManager const CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Get block file info entry for one block file */ - CBlockFileInfo* GetBlockFileInfo(size_t n); + CBlockFileInfo* GetBlockFileInfo(size_t n) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -354,7 +353,7 @@ class BlockManager * @returns in case of success, the position to which the block was written to * in case of an error, an empty FlatFilePos */ - FlatFilePos WriteBlock(const CBlock& block, int nHeight); + FlatFilePos WriteBlock(const CBlock& block, int nHeight) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Update blockfile info while processing a block during reindex. The block must be available on disk. * @@ -362,7 +361,7 @@ class BlockManager * @param[in] nHeight the height of the block * @param[in] pos the position of the serialized CBlock on disk */ - void UpdateBlockInfo(const CBlock& block, unsigned int nHeight, const FlatFilePos& pos); + void UpdateBlockInfo(const CBlock& block, unsigned int nHeight, const FlatFilePos& pos) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Whether running in -prune mode. */ [[nodiscard]] bool IsPruneMode() const { return m_prune_mode; } @@ -374,7 +373,7 @@ class BlockManager [[nodiscard]] bool LoadingBlocks() const { return m_importing || !m_blockfiles_indexed; } /** Calculate the amount of disk space the block & undo files currently use */ - uint64_t CalculateCurrentUsage(); + uint64_t CalculateCurrentUsage() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Check if all blocks in the [upper_block, lower_block] range have data available as //! defined by the status mask. diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 04bfd20c06d7..5f18afb9af4d 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -42,6 +42,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally + LOCK(::cs_main); BOOST_CHECK_EQUAL(blockman.WriteBlock(params->GenesisBlock(), 0).nPos, STORAGE_HEADER_BYTES); // simulate what happens during reindex // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file @@ -257,6 +258,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) constexpr int TEST_BLOCK_SIZE{81}; // Blockstore is empty + LOCK(::cs_main); BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), 0); // Write the first block to a new location. From 06b7022f40fecc5a482bfc4e1bfe6cd5345367d5 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Wed, 3 Jun 2026 22:06:21 +0200 Subject: [PATCH 15/20] refactor(test): Make CAmount arg explicit for BuildCreditingTransaction() --- src/test/util/transaction_utils.cpp | 2 +- src/test/util/transaction_utils.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/util/transaction_utils.cpp b/src/test/util/transaction_utils.cpp index b65a9568c65d..cf4ed9346690 100644 --- a/src/test/util/transaction_utils.cpp +++ b/src/test/util/transaction_utils.cpp @@ -7,7 +7,7 @@ #include