From d06dabf26bea7d9ca8d635e8338f64aec74c56a8 Mon Sep 17 00:00:00 2001 From: Sebastian van Staa Date: Fri, 20 Feb 2026 13:18:54 +0100 Subject: [PATCH 001/120] node: allocate index caches proportional to usage patterns add comment explaining coinstatsindex cache exclusion update cache allocations to 10%/5%/5% --- src/node/caches.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index cb8afbc967d3..d1a54b95f81c 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -58,16 +58,25 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) { size_t total_cache{CalculateDbCacheBytes(args)}; + // Allocate proportional to usage pattern benefit: + // - txindex (10%): serves getrawtransaction RPCs with mostly unique, + // non-repetitive lookups across the entire blockchain. + // - blockfilterindex (5%): serves BIP 157 light clients that repeatedly + // query recent blocks, benefiting most from LevelDB cache. + // - txospenderindex (5%): serves gettxspendingprevout RPCs with very + // specific, rarely repeated outpoint queries. + // - coinstatsindex: intentionally not included here, since usage pattern + // does not seem to suggest it would be necessary to cache. IndexCacheSizes index_sizes; - index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0); - total_cache -= index_sizes.tx_index; - index_sizes.txospender_index = std::min(total_cache / 8, args.GetBoolArg("-txospenderindex", DEFAULT_TXOSPENDERINDEX) ? MAX_TXOSPENDER_INDEX_CACHE : 0); - total_cache -= index_sizes.txospender_index; + index_sizes.tx_index = std::min(total_cache * 10 / 100, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0); + index_sizes.txospender_index = std::min(total_cache * 5 / 100, args.GetBoolArg("-txospenderindex", DEFAULT_TXOSPENDERINDEX) ? MAX_TXOSPENDER_INDEX_CACHE : 0); if (n_indexes > 0) { - size_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE); + size_t max_cache = std::min(total_cache * 5 / 100, MAX_FILTER_INDEX_CACHE); index_sizes.filter_index = max_cache / n_indexes; total_cache -= index_sizes.filter_index * n_indexes; } + total_cache -= index_sizes.tx_index; + total_cache -= index_sizes.txospender_index; return {index_sizes, kernel::CacheSizes{total_cache}}; } From 4afbabdcef86c095b19b3b42b70a2483db8cab4a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 31 Mar 2026 10:20:59 +0200 Subject: [PATCH 002/120] Fix startup failure with RLIM_INFINITY fd limits When setting the fd limit to unlimited, the node fails to start: ulimit -n unlimited build/bin/bitcoind Error: Not enough file descriptors available. -1 available, 160 required. This was caused by RaiseFileDescriptorLimit() casting limitFD.rlim_cur to int, which for RLIM_INFINITY overflows to -1. Fix it by returning std::numeric_limits::max() instead. This commit also adds a functional test, which is skipped on environments with a hard limit below infinity. Co-authored-by: Luke Dashjr Co-authored-by: winterrdog --- src/util/fs_helpers.cpp | 35 +++++++++++++------ src/util/fs_helpers.h | 13 ++++++- test/functional/feature_init.py | 26 ++++++++++++++ .../test_framework/test_framework.py | 12 +++++++ 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index a41bf65ad866..3efc9fda4b3e 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -9,12 +9,14 @@ #include #include // IWYU pragma: keep +#include #include #include #include #include #include +#include #include #include #include @@ -152,27 +154,38 @@ bool TruncateFile(FILE* file, unsigned int length) #endif } -/** - * this function tries to raise the file descriptor limit to the requested number. - * It returns the actual file descriptor limit (which may be more or less than nMinFD) - */ -int RaiseFileDescriptorLimit(int nMinFD) +int RaiseFileDescriptorLimit(int min_fd) { + Assert(min_fd >= 0); #if defined(WIN32) return 2048; #else struct rlimit limitFD; if (getrlimit(RLIMIT_NOFILE, &limitFD) != -1) { - if (limitFD.rlim_cur < (rlim_t)nMinFD) { - limitFD.rlim_cur = nMinFD; - if (limitFD.rlim_cur > limitFD.rlim_max) + // If the current soft limit is already higher, don't raise it + if (limitFD.rlim_cur != RLIM_INFINITY && std::cmp_less(limitFD.rlim_cur, min_fd)) { + const auto current_limit{limitFD.rlim_cur}; + static_assert(std::in_range(std::numeric_limits::max())); + limitFD.rlim_cur = static_cast(min_fd); + // Don't raise soft limit beyond hard limit + if ((limitFD.rlim_max != RLIM_INFINITY) && (limitFD.rlim_cur > limitFD.rlim_max)) { limitFD.rlim_cur = limitFD.rlim_max; - setrlimit(RLIMIT_NOFILE, &limitFD); - getrlimit(RLIMIT_NOFILE, &limitFD); + } + if (current_limit != limitFD.rlim_cur) { + setrlimit(RLIMIT_NOFILE, &limitFD); + getrlimit(RLIMIT_NOFILE, &limitFD); + } + } + // Check the (possibly raised) current soft limit against the special + // value of RLIM_INFINITY. Some platforms implement this as the maximum + // uint64, others as int64 (-1). Avoid casting even if the return type + // is changed to uint64_t. + if (limitFD.rlim_cur == RLIM_INFINITY) { + return std::numeric_limits::max(); } return limitFD.rlim_cur; } - return nMinFD; // getrlimit failed, assume it's fine + return min_fd; // getrlimit failed, assume it's fine #endif } diff --git a/src/util/fs_helpers.h b/src/util/fs_helpers.h index face17fd8b53..b84fddef4b8f 100644 --- a/src/util/fs_helpers.h +++ b/src/util/fs_helpers.h @@ -45,7 +45,18 @@ bool FileCommit(FILE* file); void DirectoryCommit(const fs::path& dirname); bool TruncateFile(FILE* file, unsigned int length); -int RaiseFileDescriptorLimit(int nMinFD); + +/** + * Try to raise the file descriptor limit to the requested number. + * + * @param[in] min_fd The requested minimum number of file descriptors. + * @returns The actual file descriptor limit. It may be lower or + * higher than min_fd. Returns std::numeric_limits::max() + * if the OS imposes no limit (RLIM_INFINITY). + * + */ +int RaiseFileDescriptorLimit(int min_fd); + void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length); /** diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 259e07b18695..ee28a2871a56 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -323,12 +323,38 @@ def init_empty_test(self): for option in options: self.restart_node(1, option) + def restart_node_with_fd_limit(self, limit): + """Restart node 1 with a given soft RLIMIT_NOFILE. Skips if the limit cannot be set.""" + import resource + soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE) + try: + resource.setrlimit(resource.RLIMIT_NOFILE, (limit, hard)) + except (ValueError, OSError): + self.log.info(f"Skipping rlimit test: cannot set soft limit (hard={hard})") + return + try: + self.restart_node(1) + self.log.debug(f"Node started successfully with RLIM_INFINITY limit (soft={limit})") + finally: + resource.setrlimit(resource.RLIMIT_NOFILE, (soft, hard)) + self.log.debug(f"Restored previous RLIMIT_NOFILE limits (soft={soft}, hard={hard})") + + def init_rlimit_test(self): + """Test that bitcoind starts correctly when the soft RLIMIT_NOFILE limit is RLIM_INFINITY.""" + if self.RLIM_INFINITY is None: + self.log.info("Skipping: resource module not available") + return + + self.log.info("Testing node startup with RLIM_INFINITY fd limit") + self.restart_node_with_fd_limit(self.RLIM_INFINITY) + def run_test(self): self.init_pid_test() self.init_stress_test_interrupt() self.init_stress_test_removals() self.break_wait_test() self.init_empty_test() + self.init_rlimit_test() if __name__ == '__main__': diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 067388c6f67f..fdb8090433ea 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -8,6 +8,7 @@ from enum import Enum import argparse from datetime import datetime, timezone +from importlib.util import find_spec import logging import os from pathlib import Path @@ -948,6 +949,17 @@ def skip_if_no_previous_releases(self): if not self.has_previous_releases(): raise SkipTest("previous releases not available or disabled") + def has_resource_module(self): + """Checks whether the resource module is available.""" + return find_spec('resource') is not None + + @property + def RLIM_INFINITY(self): + if not self.has_resource_module(): + return None + import resource + return resource.RLIM_INFINITY + def has_previous_releases(self): """Checks whether previous releases are present and enabled.""" if not os.path.isdir(self.options.previous_releases_path): From 8ab4b9fc856433ebcaaefb31524cb71ef8ff8089 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 31 Mar 2026 10:21:18 +0200 Subject: [PATCH 003/120] init: clamp fd limits to int When setting the fd limit to 1 >> 31, the node fails to start: ulimit -n 214748364 build/bin/bitcoind Error: Not enough file descriptors available. -2147483648 available, 160 required. Similar to the previous commit, this is fixed by capping the limit to std::numeric_limits::max(). Co-authored-by: Luke Dashjr --- src/util/fs_helpers.cpp | 8 +++++--- test/functional/feature_init.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index 3efc9fda4b3e..e2b4c899c998 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -179,11 +179,13 @@ int RaiseFileDescriptorLimit(int min_fd) // Check the (possibly raised) current soft limit against the special // value of RLIM_INFINITY. Some platforms implement this as the maximum // uint64, others as int64 (-1). Avoid casting even if the return type - // is changed to uint64_t. - if (limitFD.rlim_cur == RLIM_INFINITY) { + // is changed to uint64_t. We also cap unlikely but possible values + // that would overflow int. + if (limitFD.rlim_cur == RLIM_INFINITY || + std::cmp_greater_equal(limitFD.rlim_cur, std::numeric_limits::max())) { return std::numeric_limits::max(); } - return limitFD.rlim_cur; + return static_cast(limitFD.rlim_cur); } return min_fd; // getrlimit failed, assume it's fine #endif diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index ee28a2871a56..e7c226b25561 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -348,6 +348,15 @@ def init_rlimit_test(self): self.log.info("Testing node startup with RLIM_INFINITY fd limit") self.restart_node_with_fd_limit(self.RLIM_INFINITY) + def init_rlimit_large_test(self): + """Test that bitcoind starts correctly when the soft RLIMIT_NOFILE limit is above INT_MAX.""" + if self.RLIM_INFINITY is None: + self.log.info("Skipping: resource module not available") + return + + self.log.info("Testing node startup with fd limit above INT_MAX") + self.restart_node_with_fd_limit(1 << 31) + def run_test(self): self.init_pid_test() self.init_stress_test_interrupt() @@ -355,6 +364,7 @@ def run_test(self): self.break_wait_test() self.init_empty_test() self.init_rlimit_test() + self.init_rlimit_large_test() if __name__ == '__main__': From 735b25519aad2d3c24345c2d6f69c30d1040f473 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 31 Mar 2026 10:21:31 +0200 Subject: [PATCH 004/120] support: clamp RLIMIT_MEMLOCK to size_t On 32-bit systems we build with _FILE_OFFSET_BITS=64 (see CMakeLists.txt), which makes rlim_t 64-bit when building against glibc (see bits/resource.h). Since size_t could be 32-bit, clamp RLIMIT_MEMLOCK to std::numeric_limits::max() in PosixLockedPageAllocator::GetLimit(). Co-authored-by: Luke Dashjr --- src/support/lockedpool.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index ff3a9e69c581..97f0df409fb9 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -262,7 +262,8 @@ size_t PosixLockedPageAllocator::GetLimit() #ifdef RLIMIT_MEMLOCK struct rlimit rlim; if (getrlimit(RLIMIT_MEMLOCK, &rlim) == 0) { - if (rlim.rlim_cur != RLIM_INFINITY) { + if (rlim.rlim_cur != RLIM_INFINITY && + std::cmp_less_equal(rlim.rlim_cur, static_cast(std::numeric_limits::max()))) { return rlim.rlim_cur; } } From 47d68cd981fb2985acca0b4d2b8d79dbdccb136d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 24 Apr 2026 10:17:29 +0200 Subject: [PATCH 005/120] ci: backport iwyu PR 2013 std::hash mapping Cherry-pick include-what-you-use commit 52f85e1f4d99 onto the clang_22 branch tracked by ci/test/00_setup_env_native_iwyu.sh, fixing the false positive where std::hash was mapped to / instead of its real provider (, , etc). --- ci/test/01_base_install.sh | 5 ++++- ci/test/02_iwyu_hash.patch | 44 ++++++++++++++++++++++++++++++++++++++ ci/test_imagefile | 3 ++- 3 files changed, 50 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 5a6e06811fa0..9a93890b41a5 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -86,7 +86,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 )" From a9301cfa0730d1d08c9e982f990dfc8aa0b3a27a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 27 Apr 2026 14:14:07 +0200 Subject: [PATCH 006/120] refactor: disable default std::hash for CTransactionRef The default std::hash for shared_ptr compares by pointer. CTransactionRefHash or a custom hasher should be used instead. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- src/primitives/transaction.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 0e4b0bacecf94063342c7f9eb9b03dac8a7a7936 Mon Sep 17 00:00:00 2001 From: marcofleon Date: Mon, 27 Apr 2026 18:16:44 +0100 Subject: [PATCH 007/120] validation: Don't add pruned blocks to m_blocks_unlinked on startup LoadBlockIndex() adds to m_blocks_unlinked based only on nTx > 0, without checking BLOCK_HAVE_DATA. Pruning preserves nTx but clears BLOCK_HAVE_DATA, so a pruned block whose parent was header-only gets re-added on every restart, causing the CheckBlockIndex() assertion that entries must have data on disk to fail. Check that BLOCK_HAVE_DATA is set before inserting into m_blocks_unlinked. Fixes #35050. --- src/node/blockstorage.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index b0842a005059..411f0ca849b0 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -487,7 +487,9 @@ bool BlockManager::LoadBlockIndex(const std::optional& snapshot_blockha pindex->m_chain_tx_count = pindex->pprev->m_chain_tx_count + pindex->nTx; } else { pindex->m_chain_tx_count = 0; - m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex)); + if (pindex->nStatus & BLOCK_HAVE_DATA) { + m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex)); + } } } else { pindex->m_chain_tx_count = pindex->nTx; From 5a2e3592137d595fb5fd0f9d66c9cac6c020d7bd Mon Sep 17 00:00:00 2001 From: Sebastian van Staa Date: Mon, 4 May 2026 21:21:29 +0200 Subject: [PATCH 008/120] clarify blockfilterindex cache allocation rationale --- src/node/caches.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index d1a54b95f81c..03c58648b7ff 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -62,7 +62,9 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) // - txindex (10%): serves getrawtransaction RPCs with mostly unique, // non-repetitive lookups across the entire blockchain. // - blockfilterindex (5%): serves BIP 157 light clients that repeatedly - // query recent blocks, benefiting most from LevelDB cache. + // query recent blocks, benefiting from LevelDB cache, but the + // working set for a typical 2-week offline gap is ~200kiB, well within 5% + // of the total cache. // - txospenderindex (5%): serves gettxspendingprevout RPCs with very // specific, rarely repeated outpoint queries. // - coinstatsindex: intentionally not included here, since usage pattern From b3a3f88346dfd218a049acec6a77166f319c70e8 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sun, 10 May 2026 12:49:50 +0200 Subject: [PATCH 009/120] crypto: cleanse HMAC stack buffers after use CHMAC_SHA256 and CHMAC_SHA512 leave two stack buffers populated on return: rkey[] holds K' XOR 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. Cleanse both buffers with memory_cleanse(), matching the convention in chacha20.cpp and chacha20poly1305.cpp. No observable change for callers. --- src/crypto/hmac_sha256.cpp | 4 ++++ src/crypto/hmac_sha512.cpp | 4 ++++ 2 files changed, 8 insertions(+) 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)); } From 3f44f9aef7ccd0417fcd0c2f33f20615ea5c11e6 Mon Sep 17 00:00:00 2001 From: marcofleon Date: Mon, 27 Apr 2026 19:15:46 +0100 Subject: [PATCH 010/120] test: Add coverage for m_blocks_unlinked invariant in LoadBlockIndex A pruned stale-fork block whose parent doesn't have any transactions shouldn't be added to m_blocks_unlinked when starting up a node. --- test/functional/feature_prune_stale_fork.py | 39 +++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 40 insertions(+) create mode 100755 test/functional/feature_prune_stale_fork.py diff --git a/test/functional/feature_prune_stale_fork.py b/test/functional/feature_prune_stale_fork.py new file mode 100755 index 000000000000..5badca0a7cfc --- /dev/null +++ b/test/functional/feature_prune_stale_fork.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026-present 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 node restart with a pruned stale-fork block whose parent has no transactions.""" + +from test_framework.blocktools import create_empty_fork +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_raises_rpc_error + + +class FeaturePruneStaleForkTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [["-prune=1", "-fastprune"]] + + def run_test(self): + node = self.nodes[0] + + self.log.info("Create a 2-block stale fork: parent has no transactions, child has transactions") + [side_parent, side_child] = create_empty_fork(node, 2) + + node.submitheader(side_parent.serialize().hex()) + node.submitblock(side_child.serialize().hex()) + assert_equal(node.getblockheader(side_parent.hash_hex)["nTx"], 0) + assert_equal(node.getblockheader(side_child.hash_hex)["nTx"], 1) + + self.log.info("Advance and prune so the stale-fork child's block data is removed from disk") + self.generate(node, 500) + node.pruneblockchain(node.getblockcount() - 100) + assert_raises_rpc_error(-1, "Block not available (pruned data)", node.getblock, side_child.hash_hex) + + self.log.info("Restart and mine; node must reload cleanly after the stale-fork child was pruned") + self.restart_node(0) + self.generate(node, 1) + + +if __name__ == '__main__': + FeaturePruneStaleForkTest(__file__).main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 698742bc79e7..9236f3a90d39 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -369,6 +369,7 @@ 'feature_blocksdir.py', 'wallet_startup.py', 'feature_remove_pruned_files_on_startup.py', + 'feature_prune_stale_fork.py', 'p2p_i2p_ports.py', 'p2p_i2p_sessions.py', 'feature_presegwit_node_upgrade.py', From 21a1380c13470d28f53cc0b85d902693365d39d3 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 12 May 2026 21:24:19 +0200 Subject: [PATCH 011/120] key: cleanse ChainCode on destruction HMAC primitives cleanse their internal stack buffers, but a caller's ChainCode remains populated in memory after use. Promote 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. Retype MUSIG_CHAINCODE from `constexpr uint256` to `const ChainCode` to match its BIP328 semantic role. Dropping `constexpr` (ChainCode is no longer a literal type) also removes the GCC-14 consteval lambda workaround. Remove the duplicate typedef in pubkey.h (which includes hash.h transitively). Two fuzz-test call sites in test/fuzz/key.cpp now construct the chain-code argument explicitly rather than relying on the typedef. --- src/hash.h | 10 +++++++++- src/musig.cpp | 5 +---- src/pubkey.h | 2 -- src/test/fuzz/key.cpp | 4 ++-- 4 files changed, 12 insertions(+), 9 deletions(-) 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 706874be2cf5..b04a26db123c 100644 --- a/src/musig.cpp +++ b/src/musig.cpp @@ -9,10 +9,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 e4bedff8b512..19101ae81c2d 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -85,7 +85,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)); @@ -275,7 +275,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 fa2afba28b5776a710386a3bacd5cb308149d671 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 15 May 2026 11:15:36 +0200 Subject: [PATCH 012/120] p2p: Release m_peer_mutex early in InitiateTxBroadcastToAll --- 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 631e968da7f0..c4be0e61b585 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -575,6 +575,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); @@ -1785,6 +1788,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 { { @@ -2243,9 +2257,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 661571018b36..34004c6768ba 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 faf7e38973673e13aff30dba8f8c57df02537c74 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 27 May 2026 15:44:01 +0200 Subject: [PATCH 013/120] ci: refactor: Avoid warning: INSTALL_BCC_TRACING_TOOLS: unbound variable The variable is never set and will always be unbound. The only way to set it correctly is via this hack: .github/workflows/ci.yml- # In the image build step, no external environment variables are available, .github/workflows/ci.yml- # so any settings will need to be written to the settings env file: .github/workflows/ci.yml: run: sed -i "s|\${INSTALL_BCC_TRACING_TOOLS}|true|g" ./ci/test/00_setup_env_native_asan.sh So just silence the warning, which happens when running the task locally: ``` ./ci/test/00_setup_env_native_asan.sh: line 12: INSTALL_BCC_TRACING_TOOLS: unbound variable ``` --- ci/test/00_setup_env_native_asan.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 2465c70bb487..58be89fa1041 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8 export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:24.04" # Only install BCC tracing packages in CI. Container has to match the host for BCC to work. -if [[ "${INSTALL_BCC_TRACING_TOOLS}" == "true" ]]; then +if [[ "${INSTALL_BCC_TRACING_TOOLS:-}" == "true" ]]; then # Required for USDT functional tests to run BPFCC_PACKAGE="bpfcc-tools linux-headers-$(uname --kernel-release)" export CI_CONTAINER_CAP="--privileged -v /sys/kernel:/sys/kernel:rw" From fa98d449517da406203e042aaccb8acc698d7a0c Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 27 May 2026 16:53:31 +0200 Subject: [PATCH 014/120] ci: Rewrite broken wrap-valgrind.sh to .py Shellcheck marked the script as violating SC2044. Instead of re-writing the Bash from scratch, just use Python. --- ci/test/03_test_script.sh | 2 +- ci/test/wrap-valgrind.py | 32 ++++++++++++++++++++++++++++++++ ci/test/wrap-valgrind.sh | 18 ------------------ 3 files changed, 33 insertions(+), 19 deletions(-) create mode 100755 ci/test/wrap-valgrind.py delete mode 100755 ci/test/wrap-valgrind.sh diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index b3ab17298535..5ecbe863a39e 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -168,7 +168,7 @@ if [ -n "${CI_LIMIT_STACK_SIZE}" ]; then fi if [ -n "$USE_VALGRIND" ]; then - "${BASE_ROOT_DIR}/ci/test/wrap-valgrind.sh" + "${BASE_ROOT_DIR}/ci/test/wrap-valgrind.py" fi if [ "$RUN_CHECK_DEPS" = "true" ]; then diff --git a/ci/test/wrap-valgrind.py b/ci/test/wrap-valgrind.py new file mode 100755 index 000000000000..6a79f2aaae1d --- /dev/null +++ b/ci/test/wrap-valgrind.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +# Copyright (c) The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://opensource.org/license/mit/. + +import os +import shlex +from pathlib import Path + + +def main(): + base_root = Path(os.environ["BASE_ROOT_DIR"]) + base_out = Path(os.environ["BASE_OUTDIR"]) + suppressions_file = base_root / "test" / "sanitizer_suppressions" / "valgrind.supp" + target_names = {b.name for b in (base_out / "bin").iterdir()} + + for exe in base_root.rglob("*"): + if exe.name in target_names and exe.is_file() and os.access(exe, os.X_OK): + print(f"Wrap {exe} ...") + original_path = exe.with_name(f"{exe.name}_orig") + exe.rename(original_path) + exe.write_text( + "#!/usr/bin/env bash\n" + "exec valgrind --gen-suppressions=all --quiet --error-exitcode=1 " + f"--suppressions={shlex.quote(str(suppressions_file))} " + f'{shlex.quote(str(original_path))} "$@"\n' + ) + exe.chmod(exe.stat().st_mode | 0o111) + + +if __name__ == "__main__": + main() diff --git a/ci/test/wrap-valgrind.sh b/ci/test/wrap-valgrind.sh deleted file mode 100755 index 4ed3f2d66c54..000000000000 --- a/ci/test/wrap-valgrind.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env bash -# -# Copyright (c) 2018-present The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -export LC_ALL=C.UTF-8 - -for b_name in "${BASE_OUTDIR}/bin"/*; do - # shellcheck disable=SC2044 - for b in $(find "${BASE_ROOT_DIR}" -executable -type f -name "$(basename "$b_name")"); do - echo "Wrap $b ..." - mv "$b" "${b}_orig" - echo '#!/usr/bin/env bash' > "$b" - echo "exec valgrind --gen-suppressions=all --quiet --error-exitcode=1 --suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/valgrind.supp \"${b}_orig\" \"\$@\"" >> "$b" - chmod +x "$b" - done -done From 4731049ba4f8a820cc4aa13a745e41bdfbee284a Mon Sep 17 00:00:00 2001 From: Sanjana2906 Date: Tue, 2 Jun 2026 15:37:07 +0530 Subject: [PATCH 015/120] build: exclude mptest target from compile commands build: exclude mptest target from compile commands --- cmake/libmultiprocess.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/libmultiprocess.cmake b/cmake/libmultiprocess.cmake index 027109a90511..0dab1f66ee2b 100644 --- a/cmake/libmultiprocess.cmake +++ b/cmake/libmultiprocess.cmake @@ -34,5 +34,5 @@ function(add_libmultiprocess subdir) # exclusion, tools like clang-tidy and IWYU that make use of compilation # database would complain that the generated c++ source files do not exist. An # alternate fix could build "mpexamples" by default like "mptests" above. - set_target_properties(mpcalculator mpprinter mpexample PROPERTIES EXPORT_COMPILE_COMMANDS OFF) + set_target_properties(mpcalculator mpprinter mpexample mptest PROPERTIES EXPORT_COMPILE_COMMANDS OFF) endfunction() From 726e196ef26490115edc595973f6703e61e1295b Mon Sep 17 00:00:00 2001 From: will Date: Tue, 2 Jun 2026 12:30:18 +0100 Subject: [PATCH 016/120] ci: inline runner selection We are currently sometimes backlogged on waiting for runner selection. Selecting Warp or GitHub-hosted runners directly from the repository context avoids serializing all CI jobs behind a metadata job. This keeps forks on public runners while allowing upstream jobs to schedule immediately on the intended runner labels. --- .github/workflows/ci.yml | 57 +++++++++++++--------------------------- 1 file changed, 18 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8eaa68ab0e2c..fa837b928c80 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,6 @@ concurrency: env: CI_FAILFAST_TEST_LEAVE_DANGLING: 1 # GHA does not care about dangling processes and setting this variable avoids killing the CI script itself on error - REPO_USE_WARP_RUNNERS: 'bitcoin/bitcoin' # Use warp runners for this repo, instead of falling back to the slow GHA runners defaults: run: @@ -28,12 +27,16 @@ defaults: shell: bash jobs: - runners: - name: '[meta] determine runners' - runs-on: ubuntu-slim - outputs: - provider: ${{ steps.runners.outputs.provider }} + test-each-commit: + name: 'test ancestor commits' + runs-on: ${{ github.repository == 'bitcoin/bitcoin' && 'warp-ubuntu-latest-x64-8x' || 'ubuntu-latest' }} + env: + TEST_RUNNER_PORT_MIN: "14000" # Use a larger port range to avoid colliding with other CI services. + if: github.event_name == 'pull_request' && github.event.pull_request.commits != 1 + timeout-minutes: 360 # Use maximum time, see https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes. steps: + - name: Determine fetch depth + run: echo "FETCH_DEPTH=$((${{ github.event.pull_request.commits }} + 2))" >> "$GITHUB_ENV" - &ANNOTATION_PR_NUMBER name: Annotate with pull request number # This annotation is machine-readable and can be used to assign a check @@ -44,28 +47,6 @@ jobs: if [ "${{ github.event_name }}" = "pull_request" ]; then echo "::notice title=debug_pull_request_number_str::${{ github.event.number }}" fi - - id: runners - run: | - if [[ "${REPO_USE_WARP_RUNNERS}" == "${{ github.repository }}" ]]; then - echo "provider=warp" >> "$GITHUB_OUTPUT" - echo "::notice title=Runner Selection::Using Warp Runners" - else - echo "provider=gha" >> "$GITHUB_OUTPUT" - echo "::notice title=Runner Selection::Using GitHub-hosted runners" - fi - - test-each-commit: - name: 'test ancestor commits' - needs: runners - runs-on: ${{ needs.runners.outputs.provider == 'warp' && 'warp-ubuntu-latest-x64-8x' || 'ubuntu-latest' }} - env: - TEST_RUNNER_PORT_MIN: "14000" # Use a larger port range to avoid colliding with other CI services. - if: github.event_name == 'pull_request' && github.event.pull_request.commits != 1 - timeout-minutes: 360 # Use maximum time, see https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idtimeout-minutes. - steps: - - name: Determine fetch depth - run: echo "FETCH_DEPTH=$((${{ github.event.pull_request.commits }} + 2))" >> "$GITHUB_ENV" - - *ANNOTATION_PR_NUMBER - uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.head.sha }} @@ -324,8 +305,8 @@ jobs: windows-cross: name: 'Windows-cross to x86_64, ${{ matrix.crt }}' - needs: [runners, record-frozen-commit] - runs-on: ${{ needs.runners.outputs.provider == 'warp' && 'warp-ubuntu-latest-x64-4x' || 'ubuntu-latest' }} + needs: record-frozen-commit + runs-on: ${{ github.repository == 'bitcoin/bitcoin' && 'warp-ubuntu-latest-x64-4x' || 'ubuntu-latest' }} if: ${{ vars.SKIP_BRANCH_PUSH != 'true' || github.event_name == 'pull_request' }} strategy: @@ -359,7 +340,7 @@ jobs: id: restore-cache uses: ./.github/actions/cache/restore with: - provider: ${{ needs.runners.outputs.provider }} + provider: ${{ github.repository == 'bitcoin/bitcoin' && 'warp' || 'gha' }} - name: Configure Docker uses: ./.github/actions/configure-docker @@ -370,7 +351,7 @@ jobs: - name: Save caches uses: ./.github/actions/cache/save with: - provider: ${{ needs.runners.outputs.provider }} + provider: ${{ github.repository == 'bitcoin/bitcoin' && 'warp' || 'gha' }} - name: Upload built executables uses: actions/upload-artifact@v7 @@ -437,8 +418,7 @@ jobs: ci-matrix: name: ${{ matrix.name }} - needs: runners - runs-on: ${{ needs.runners.outputs.provider == 'warp' && matrix.warp-runner || matrix.fallback-runner }} + runs-on: ${{ github.repository == 'bitcoin/bitcoin' && matrix.warp-runner || matrix.fallback-runner }} if: ${{ vars.SKIP_BRANCH_PUSH != 'true' || github.event_name == 'pull_request' }} timeout-minutes: ${{ matrix.timeout-minutes }} @@ -553,13 +533,13 @@ jobs: id: restore-cache uses: ./.github/actions/cache/restore with: - provider: ${{ matrix.provider || needs.runners.outputs.provider }} + provider: ${{ matrix.provider || (github.repository == 'bitcoin/bitcoin' && 'warp' || 'gha') }} - name: Configure Docker uses: ./.github/actions/configure-docker - name: Clear unnecessary files - if: ${{ needs.runners.outputs.provider == 'gha' && true || false }} # Only needed on GHA runners + if: ${{ github.repository != 'bitcoin/bitcoin' || matrix.provider == 'gha' }} # Only needed on GHA runners uses: ./.github/actions/clear-files - name: Enable bpfcc script @@ -579,12 +559,11 @@ jobs: - name: Save caches uses: ./.github/actions/cache/save with: - provider: ${{ matrix.provider || needs.runners.outputs.provider }} + provider: ${{ matrix.provider || (github.repository == 'bitcoin/bitcoin' && 'warp' || 'gha') }} lint: name: 'lint' - needs: runners - runs-on: ${{ needs.runners.outputs.provider == 'warp' && 'warp-ubuntu-latest-x64-2x' || 'ubuntu-latest' }} + runs-on: ${{ github.repository == 'bitcoin/bitcoin' && 'warp-ubuntu-latest-x64-2x' || 'ubuntu-latest' }} if: ${{ vars.SKIP_BRANCH_PUSH != 'true' || github.event_name == 'pull_request' }} timeout-minutes: 20 env: From de92208c2b508b40fa690624d026c775ed876606 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 1 Jun 2026 14:37:57 -0700 Subject: [PATCH 017/120] migrate: Handle HD chains that have identical seeds but different IDs The seed ID is calculated from a pubkey produced by treating the seed as a private key. This calculation includes a pubkey compression parameter, even thought that compression is completely irrelevant for the usage of the seed as a BIP 32 seed. Thus migration should detect if a seed has been used multiple times by checking if the computed master key was already processed. The spkm_migration fuzzer needs to have it's added descriptors accounting to be updated for this fix. --- src/wallet/scriptpubkeyman.cpp | 25 ++++++++++++++++-------- src/wallet/test/fuzz/scriptpubkeyman.cpp | 6 +++--- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index dad8aef39dc5..0759e2d5220a 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -628,19 +628,28 @@ std::optional LegacyDataSPKM::MigrateToDescriptor() bool can_support_hd_split_feature = m_hd_chain.nVersion >= CHDChain::VERSION_HD_CHAIN_SPLIT; + std::set master_xpubs; for (const CHDChain& chain : chains) { + if (chain.seed_id.IsNull()) continue; + + // Get the master xprv + CKey seed_key; + if (!GetKey(chain.seed_id, seed_key)) { + assert(false); + } + CExtKey master_key; + master_key.SetSeed(seed_key); + + // Get the xpub and verify that we haven't already seen this xpub before + CExtPubKey master_xpub = master_key.Neuter(); + const auto& [_, inserted] = master_xpubs.insert(master_xpub); + if (!inserted) continue; + for (int i = 0; i < 2; ++i) { // Skip if doing internal chain and split chain is not supported - if (chain.seed_id.IsNull() || (i == 1 && !can_support_hd_split_feature)) { + if (i == 1 && !can_support_hd_split_feature) { continue; } - // Get the master xprv - CKey seed_key; - if (!GetKey(chain.seed_id, seed_key)) { - assert(false); - } - CExtKey master_key; - master_key.SetSeed(seed_key); // Make the combo descriptor std::string xpub = EncodeExtPubKey(master_key.Neuter()); diff --git a/src/wallet/test/fuzz/scriptpubkeyman.cpp b/src/wallet/test/fuzz/scriptpubkeyman.cpp index b6b3c5d55973..bb848d1c9ddc 100644 --- a/src/wallet/test/fuzz/scriptpubkeyman.cpp +++ b/src/wallet/test/fuzz/scriptpubkeyman.cpp @@ -257,10 +257,10 @@ FUZZ_TARGET(spkm_migration, .init = initialize_spkm_migration) bool add_inactive_hd_chain{fuzzed_data_provider.ConsumeBool() && !keys.empty()}; if (add_inactive_hd_chain) { - hd_key = PickValue(fuzzed_data_provider, keys); + CKey inactive_hd_key = PickValue(fuzzed_data_provider, keys); hd_chain.nVersion = fuzzed_data_provider.ConsumeBool() ? CHDChain::VERSION_HD_CHAIN_SPLIT : CHDChain::VERSION_HD_BASE; - bool dup_chain = hd_chain.seed_id == hd_key.GetPubKey().GetID(); - hd_chain.seed_id = hd_key.GetPubKey().GetID(); + bool dup_chain = hd_key.IsValid() && std::equal(hd_key.begin(), hd_key.end(), inactive_hd_key.begin()); + hd_chain.seed_id = inactive_hd_key.GetPubKey().GetID(); legacy_data.AddInactiveHDChain(hd_chain); if (!dup_chain) added_chains++; } From 2189a6f5f226d5a2905f1939eb7eea9571502b90 Mon Sep 17 00:00:00 2001 From: codeabysss <248203105+codeabysss@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:47:45 +0300 Subject: [PATCH 018/120] p2p: Saturate LocalServiceInfo::nScore updates at INT_MAX Signed overflow on nScore updates is undefined behavior. Use SaturatingAdd in AddLocal() and SeenLocal() so increments saturate at INT_MAX instead of overflowing. Add unit test coverage for saturation in both code paths. --- 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 f8149ef309d1..ca0289c8316e 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 32801d97b8bd..7168d3dcaec2 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 b5e91e946c8a07d5a2915d067eeae68473b24d24 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Wed, 3 Jun 2026 21:19:05 +0200 Subject: [PATCH 019/120] wallet: Remove CoinsResult::Clear() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead we fully reset CoinsResult-instances without forgetting any fields. Fixes: https://github.com/bitcoin/bitcoin/issues/35449 Variant resetting the other fields in Clear(): ₿ hyperfine --warmup 2 "./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test" Benchmark 1: ./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test Time (mean ± σ): 4.289 s ± 0.008 s [User: 4.252 s, System: 0.027 s] Range (min … max): 4.279 s … 4.301 s 10 runs This variant: ₿ hyperfine --warmup 2 "./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test" Benchmark 1: ./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test Time (mean ± σ): 4.279 s ± 0.005 s [User: 4.244 s, System: 0.026 s] Range (min … max): 4.271 s … 4.287 s 10 runs --- src/wallet/spend.cpp | 4 ---- src/wallet/spend.h | 3 +-- src/wallet/test/coinselector_tests.cpp | 20 ++++++++++---------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 9384299d9a1b..dba7b8829595 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -210,10 +210,6 @@ std::vector CoinsResult::All() const return all; } -void CoinsResult::Clear() { - coins.clear(); -} - void CoinsResult::Erase(const std::unordered_set& coins_to_remove) { for (auto& [type, vec] : coins) { diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 925111dce031..b013a3fabdf9 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -40,7 +40,7 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* walle * This struct is really just a wrapper around OutputType vectors with a convenient * method for concatenating and returning all COutputs as one vector. * - * Size(), Clear(), Erase(), Shuffle(), and Add() methods are implemented to + * Size(), Erase(), Shuffle(), and Add() methods are implemented to * allow easy interaction with the struct. */ struct CoinsResult { @@ -54,7 +54,6 @@ struct CoinsResult { size_t Size() const; /** Return how many different output types this struct stores */ size_t TypesCount() const { return coins.size(); } - void Clear(); void Erase(const std::unordered_set& coins_to_remove); void Shuffle(FastRandomContext& rng_fast); void Add(OutputType type, const COutput& out); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index ee6a639a43db..5713466e4188 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); // Test fees subtracted from output: - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate); available_coins.All().at(0).input_bytes = 40; const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); @@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) { - available_coins.Clear(); + available_coins = {}; // with an empty wallet we can't even pay one cent BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 1 * CENT, CENT)); @@ -412,7 +412,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, 6*CENT); add_coin(available_coins, *wallet, 7*CENT); @@ -470,7 +470,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 1 / 10); add_coin(available_coins, *wallet, CENT * 2 / 10); add_coin(available_coins, *wallet, CENT * 3 / 10); @@ -502,7 +502,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - available_coins.Clear(); + available_coins = {}; for (int j = 0; j < 20; j++) add_coin(available_coins, *wallet, 50000 * COIN); @@ -515,7 +515,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest coin: - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 5 / 10); add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 7 / 10); @@ -526,7 +526,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_CHECK_EQUAL(result20->GetInputSet().size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 4 / 10); add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 8 / 10); @@ -537,7 +537,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_CHECK_EQUAL(result21->GetInputSet().size(), 2U); // in two coins 0.4+0.6 // test avoiding small change - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 5 / 100); add_coin(available_coins, *wallet, CENT * 1); add_coin(available_coins, *wallet, CENT * 100); @@ -557,7 +557,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test with many inputs for (CAmount amt=1500; amt < COIN; amt*=10) { - available_coins.Clear(); + available_coins = {}; // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) for (uint16_t j = 0; j < 676; j++) add_coin(available_coins, *wallet, amt); @@ -583,7 +583,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test randomness { - available_coins.Clear(); + available_coins = {}; for (int i2 = 0; i2 < 100; i2++) add_coin(available_coins, *wallet, COIN); From 43ca54ca000e66ebf20e8f537f0addad02a3dfbc 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 020/120] 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