From e37a70bf71d05d2e355e6d0ac49c58a4b446454f Mon Sep 17 00:00:00 2001 From: will Date: Mon, 23 Jun 2025 10:56:58 +0100 Subject: [PATCH 1/8] build: add root dir to CMAKE_PREFIX_PATH Nix patches cmake to remove the root directory `/` from `CMAKE_SYSTEM_PREFIX_PATH`: https://github.com/NixOS/nixpkgs/blob/428b49b28ebc8938a6d9f6c540d32d7a06713972/pkgs/by-name/cm/cmake/001-search-path.diff#L10 Without this, and when using the toolchain for depends builds, cmake's `find_path()` and `find_package()` do not know where to find dependencies, causing issues like: https://github.com/bitcoin/bitcoin/issues/32428 Adding this path back via CMAKE_PREFIX_PATH is harmless on other systems, and fixes the toolchain for Nix users. We append the `/` dir a maximum of once, as the toolchain may be called repeatedly during builds. Co-authored-by: Russell Yanofsky Co-authored-by: josibake Github-Pull: #32798 Rebased-From: e27a94596f2a1f5e04722a16165717cc6e891d36 --- depends/toolchain.cmake.in | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/depends/toolchain.cmake.in b/depends/toolchain.cmake.in index 65036d07a9e..c68de84b22e 100644 --- a/depends/toolchain.cmake.in +++ b/depends/toolchain.cmake.in @@ -92,6 +92,22 @@ set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY) set(QT_TRANSLATIONS_DIR "${CMAKE_CURRENT_LIST_DIR}/translations") +# The following is only necessary when using cmake from Nix or NixOS, because +# Nix patches cmake to remove the root directory `/` from +# CMAKE_SYSTEM_PREFIX_PATH. Adding it back is harmless on other platforms and +# necessary on Nix because without it cmake find_path, find_package, etc +# functions do not know where to look in CMAKE_FIND_ROOT_PATH for dependencies +# (https://github.com/bitcoin/bitcoin/issues/32428). +# +# TODO: longer term, it may be possible to use a dependency provider, which +# would bring the find_package calls completely under our control, making this +# patch unnecessary. +# +# Make sure we only append once, as this file may be called repeatedly. +if(NOT "/" IN_LIST CMAKE_PREFIX_PATH) + list(APPEND CMAKE_PREFIX_PATH "/") +endif() + if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND NOT CMAKE_HOST_APPLE) # The find_package(Qt ...) function internally uses find_library() # calls for all dependencies to ensure their availability. From 5987c1b6abaefad61d8d2cca605349354432398a Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 13 Jun 2025 01:24:50 +0200 Subject: [PATCH 2/8] test: fix catchup loop in outbound eviction functional test The catchup loop in the outbound eviction functional test currently has a small flaw, as the contained waiting for a `getheaders` message just waits for any such message instead of one with the intended block hash. The reason is that the `prev_prev_hash` variable is set incorrectly, since the `tip_header` instance is not updated and its field `.hash` is None. Fix that by updating `tip_header` and use the correct field -- we want the tip header's previous hash (`.hashPrevBlock`). Github-Pull: #32742 Rebased-From: dd8447f70faf6419b4617da3c1b57098e9cd66a6 --- test/functional/p2p_outbound_eviction.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/p2p_outbound_eviction.py b/test/functional/p2p_outbound_eviction.py index 30ac85e32f3..3d6f154a34a 100755 --- a/test/functional/p2p_outbound_eviction.py +++ b/test/functional/p2p_outbound_eviction.py @@ -88,6 +88,7 @@ class P2POutEvict(BitcoinTestFramework): # Generate an additional block so the peers is 2 blocks behind prev_header = from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False)) best_block_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"] + tip_header = from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False)) peer.sync_with_ping() # Advance time but not enough to evict the peer @@ -100,7 +101,7 @@ class P2POutEvict(BitcoinTestFramework): # Send a header with the previous tip (so we go back to 1 block behind) peer.send_and_ping(msg_headers([prev_header])) - prev_prev_hash = tip_header.hash + prev_prev_hash = tip_header.hashPrevBlock self.log.info("Create an outbound connection and take some time to catch up, but do it in time") # Check that if the peer manages to catch up within time, the timeouts are removed (and the peer is not disconnected) From a990c1002b50a79ba051d322a2269670e6827dd9 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 24 Jun 2025 20:49:27 +0100 Subject: [PATCH 3/8] cmake: Use `HINTS` instead of `PATHS` in `find_*` commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the CMake documentation, `HINTS` "should be paths computed by system introspection, such as a hint provided by the location of another item already found", which is precisely the case in the `FindQRencode` module. Entries in `HINTS` are searched before those in `PATHS`. On macOS, Homebrew’s `libqrencode` will therefore be located at its real path rather than via the symlink in the default prefix. Github-Pull: #32805 Rebased-From: ead44687483e9c936ba970de890c01d5e7ad3485 --- cmake/module/FindQRencode.cmake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmake/module/FindQRencode.cmake b/cmake/module/FindQRencode.cmake index 39e3b8b679b..575bfecc8b1 100644 --- a/cmake/module/FindQRencode.cmake +++ b/cmake/module/FindQRencode.cmake @@ -21,16 +21,16 @@ endif() find_path(QRencode_INCLUDE_DIR NAMES qrencode.h - PATHS ${PC_QRencode_INCLUDE_DIRS} + HINTS ${PC_QRencode_INCLUDE_DIRS} ) find_library(QRencode_LIBRARY_RELEASE NAMES qrencode - PATHS ${PC_QRencode_LIBRARY_DIRS} + HINTS ${PC_QRencode_LIBRARY_DIRS} ) find_library(QRencode_LIBRARY_DEBUG NAMES qrencoded qrencode - PATHS ${PC_QRencode_LIBRARY_DIRS} + HINTS ${PC_QRencode_LIBRARY_DIRS} ) include(SelectLibraryConfigurations) select_library_configurations(QRencode) From a3c1939d6eb45be4a790b9943c81a3b15af1418a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 25 Jun 2025 18:45:43 +0100 Subject: [PATCH 4/8] cmake: Explicitly specify `Boost_ROOT` for Homebrew's package On macOS, this change ensures that the Boost package is located at its real path rather than via the symlink in the default prefix. Github-Pull: #32814 Rebased-From: 8800b5acc1ef7abe6c5260ae0be5386b1d593a19 --- cmake/module/AddBoostIfNeeded.cmake | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cmake/module/AddBoostIfNeeded.cmake b/cmake/module/AddBoostIfNeeded.cmake index ecd0d6f2aba..948021f5d5d 100644 --- a/cmake/module/AddBoostIfNeeded.cmake +++ b/cmake/module/AddBoostIfNeeded.cmake @@ -17,6 +17,18 @@ function(add_boost_if_needed) directory and other added INTERFACE properties. ]=] + if(CMAKE_HOST_APPLE) + find_program(HOMEBREW_EXECUTABLE brew) + if(HOMEBREW_EXECUTABLE) + execute_process( + COMMAND ${HOMEBREW_EXECUTABLE} --prefix boost + OUTPUT_VARIABLE Boost_ROOT + ERROR_QUIET + OUTPUT_STRIP_TRAILING_WHITESPACE + ) + endif() + endif() + # We cannot rely on find_package(Boost ...) to work properly without # Boost_NO_BOOST_CMAKE set until we require a more recent Boost because # upstream did not ship proper CMake files until 1.82.0. From eafea2393d972606678f3a825a71754558ac9d5c Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 15 May 2025 17:45:44 -0400 Subject: [PATCH 5/8] init: cap -maxmempool to 500 MB on 32-bit systems 32-bit architecture is limited to 4GiB, so it doesn't make sense to set a too high value. 500 MB is chosen as an arbitrary maximum value that seems reasonable. Github-Pull: #32530 Rebased-From: 2c43b6adebbfabb3c8dd82fe821ce0a5d6173b3b --- src/node/mempool_args.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/node/mempool_args.cpp b/src/node/mempool_args.cpp index e3bf5fb57cb..33ae697168e 100644 --- a/src/node/mempool_args.cpp +++ b/src/node/mempool_args.cpp @@ -25,6 +25,9 @@ using common::AmountErrMsg; using kernel::MemPoolLimits; using kernel::MemPoolOptions; +//! Maximum mempool size on 32-bit systems. +static constexpr int MAX_32BIT_MEMPOOL_MB{500}; + namespace { void ApplyArgsManOptions(const ArgsManager& argsman, MemPoolLimits& mempool_limits) { @@ -42,7 +45,13 @@ util::Result ApplyArgsManOptions(const ArgsManager& argsman, const CChainP { mempool_opts.check_ratio = argsman.GetIntArg("-checkmempool", mempool_opts.check_ratio); - if (auto mb = argsman.GetIntArg("-maxmempool")) mempool_opts.max_size_bytes = *mb * 1'000'000; + if (auto mb = argsman.GetIntArg("-maxmempool")) { + constexpr bool is_32bit{sizeof(void*) == 4}; + if (is_32bit && *mb > MAX_32BIT_MEMPOOL_MB) { + return util::Error{Untranslated(strprintf("-maxmempool is set to %i but can't be over %i MB on 32-bit systems", *mb, MAX_32BIT_MEMPOOL_MB))}; + } + mempool_opts.max_size_bytes = *mb * 1'000'000; + } if (auto hours = argsman.GetIntArg("-mempoolexpiry")) mempool_opts.expiry = std::chrono::hours{*hours}; From 1c0e19b93a876ba5503da15e1ce14315d804194b Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 16 May 2025 09:12:25 -0400 Subject: [PATCH 6/8] node: cap -dbcache to 1GiB on 32-bit architectures 32-bit architecture is limited to 4GiB, so it doesn't make sense to set a too high value. Since this setting is performance critical, pick an arbitrary value higher than for -maxmempool but still reasonable. Github-Pull: #32530 Rebased-From: 9f8e7b0b3b787b873045a4a8194e77d0b0a2b3b6 --- src/node/caches.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/node/caches.cpp b/src/node/caches.cpp index 8b432637c73..d5d69fc2044 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -19,6 +19,8 @@ static constexpr size_t MAX_TX_INDEX_CACHE{1024_MiB}; //! Max memory allocated to all block filter index caches combined in bytes. static constexpr size_t MAX_FILTER_INDEX_CACHE{1024_MiB}; +//! Maximum dbcache size on 32-bit systems. +static constexpr size_t MAX_32BIT_DBCACHE{1024_MiB}; namespace node { CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) @@ -28,7 +30,8 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) if (std::optional db_cache = args.GetIntArg("-dbcache")) { if (*db_cache < 0) db_cache = 0; uint64_t db_cache_bytes = SaturatingLeftShift(*db_cache, 20); - total_cache = std::max(MIN_DB_CACHE, std::min(db_cache_bytes, std::numeric_limits::max())); + constexpr auto max_db_cache{sizeof(void*) == 4 ? MAX_32BIT_DBCACHE : std::numeric_limits::max()}; + total_cache = std::max(MIN_DB_CACHE, std::min(db_cache_bytes, max_db_cache)); } IndexCacheSizes index_sizes; From 4b656e2023099f1ae5fe0aab6d46c97fe161d51f Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 30 Jun 2025 11:13:11 +0100 Subject: [PATCH 7/8] test: Add msgtype to msg_generic slots Github-Pull: #32833 Rebased-From: 7dc43ea503a2c145ffd4fe14b794300bfc2bcdee --- test/functional/test_framework/messages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 9ebb683a9db..971427842ff 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1358,7 +1358,7 @@ class msg_block: # for cases where a user needs tighter control over what is sent over the wire # note that the user must supply the name of the msgtype, and the data class msg_generic: - __slots__ = ("data") + __slots__ = ("msgtype", "data") def __init__(self, msgtype, data=None): self.msgtype = msgtype From ef2a013e31cf6fbded5735a998b4c992c176493d Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 25 Jun 2025 11:39:18 +0100 Subject: [PATCH 8/8] doc: update release notes for 29.x --- doc/release-notes.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index ae2ad5231fc..c60c728ad27 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -37,6 +37,13 @@ unsupported systems. Notable changes =============== +### Updated Settings + +- The `-maxmempool` and `-dbcache` startup parameters are now capped on + 32-bit systems to 500MB and 1GiB respectively. + +- #32530 node: cap -maxmempool and -dbcache values for 32-bit + ### Wallet - #31757 wallet: fix crash on double block disconnection @@ -50,6 +57,8 @@ Notable changes - #32483 test: fix two intermittent failures in wallet_basic.py - #32630 test: fix sync function in rpc_psbt.py - #32765 test: Fix list index out of range error in feature_bip68_sequence.py +- #32742 test: fix catchup loop in outbound eviction functional test +- #32833 test: Add msgtype to msg_generic slots ### Util @@ -66,6 +75,9 @@ Notable changes - #32678 guix: warn and abort when SOURCE_DATE_EPOCH is set - #32690 depends: fix SHA256SUM command on OpenBSD (use GNU mode output) - #32760 depends: capnp 1.2.0 +- #32798 build: add root dir to CMAKE_PREFIX_PATH in toolchain +- #32805 cmake: Use HINTS instead of PATHS in find_* commands +- #32814 cmake: Explicitly specify Boost_ROOT for Homebrew's package ### Gui @@ -106,6 +118,7 @@ Thanks to everyone who directly contributed to this release: - benthecarman - Brandon Odiwuor - davidgumberg +- dergoegge - enirox001 - fanquake - furszy