refactor: Use u64 over size_t for all cache sizes to fix a 32-bit overflow by maflcko · Pull Request #35616 · bitcoin/bitcoin

maflcko · 2026-06-28T09:02:13Z

This is a refactor on 64-bit systems, because size_t is equal to u64.

However, on 32-bit systems, it fixes an integer overflow while calculating the cache sizes:

src/node/caches.cpp:71:49: runtime error: unsigned integer overflow: 471859200 * 10 cannot be represented in type size_t (aka "unsigned int")

This happens while multiplying the default cache size (450MiB) by 10:

index_sizes.tx_index = std::min(total_cache * 10 / 100, ...)
                                ^^^^^^^^^^^^^^^^

The issue was introduced in commit d06dabf.

This change follows similar changed one in the past, like 3789215, ac76d94, or 28a523f.

Generally, using fixed sized integer types for calculations is beneficial, because all platforms behave exactly the same way. With platform-dependent types there is a risk that the same calculation yields different results. This has several resulting benefits:

Easier review, because there is no need to review the same code several times for each supported platform.
Easier quality assurance, because there is less need to run the same code several times in sanitizers for each supported platform, which is tedious.

There are also no downsides, because there is no measurable overhead on 32-bit for u64 calculations that are done only once in the lifetime of the program. Also, there is no measurable memory overhead when a few fields on 32-bit store some extra zero bytes.

As said, testing is only possible by picking one of the tedious options:

Apply a diff on 64-bit arch and compile with -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DSANITIZERS=integer

diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index c98b8ce604..cfd49b60d3 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -58,3 +58,3 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
 {
-    size_t total_cache{CalculateDbCacheBytes(args)};
+    uint32_t total_cache(CalculateDbCacheBytes(args));
 
@@ -72,6 +72,6 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
     IndexCacheSizes index_sizes;
-    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);
+    index_sizes.tx_index = std::min<uint32_t>(total_cache * 10 / 100, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
+    index_sizes.txospender_index = std::min<uint32_t>(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 * 5 / 100, MAX_FILTER_INDEX_CACHE);
+        size_t max_cache = std::min<uint32_t>(total_cache * 5 / 100, MAX_FILTER_INDEX_CACHE);
         index_sizes.filter_index = max_cache / n_indexes;

This will give a roughly similar error:

sh-5.3$ echo 'Bw==' | base64 -d > /tmp/blob
sh-5.3$ UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=block_index_tree ./bld-cmake/bin/fuzz /tmp/blob
./src/node/caches.cpp:73:59: runtime error: unsigned integer overflow: 1073741824 * 10 cannot be represented in type 'uint32_t' (aka 'unsigned int')
    #0 0x55ca78da5c47 in node::CalculateCacheSizes(ArgsManager const&, unsigned long) ./src/node/caches.cpp:73:59

Alternatively, to reproduce in a fresh podman run -it --rm --platform linux/i386 debian:unstable:

export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git   ./b-c && cd b-c && apt install  build-essential cmake pkg-config  python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev  systemtap-sdt-dev  libcapnp-dev capnproto  libqrencode-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev  clang llvm libc++-dev libc++abi-dev  mold -y   &&  cmake -B ./bld-cmake -DAPPEND_CXXFLAGS='-O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=mold -DCMAKE_C_COMPILER='clang;-ftrivial-auto-var-init=pattern' -DCMAKE_CXX_COMPILER='clang++;-ftrivial-auto-var-init=pattern' -DSANITIZERS=address,float-divide-by-zero,integer,undefined --preset=dev-mode                              && cmake --build ./bld-cmake --parallel  $(nproc)

echo 'Bw==' | base64 -d > /tmp/blob

UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=block_index_tree ./bld-cmake/bin/fuzz /tmp/blob

# or:

UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ASAN_OPTIONS="detect_leaks=0"  ./bld-cmake/bin/test_bitcoin-qt

DrahtBot · 2026-06-28T09:02:21Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35616.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	sedited, theStack, l0rinc
Concept ACK	fjahr

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35205 (kernel,node: clean up dbcache helpers and add kernel API by l0rinc)
#35200 (node: smooth oversized dbcache warnings by l0rinc)
#34844 (util: Add util::NotNull by maflcko)
#32427 (kernel: Replace leveldb-based BlockTreeDB with WAL and .dat file based store by sedited)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot · 2026-06-28T09:59:55Z

🚧 At least one of the CI tasks failed.
_{Task i686, no IPC: https://github.com/bitcoin/bitcoin/actions/runs/28317284514/job/83892802764}
_{LLM reason (✨ experimental): CI failed due to a C++ build error: -Werror=narrowing treated as fatal when assigning uint64_t (block_tree_db) to size_t in src/init.cpp.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

fjahr · 2026-06-28T19:20:00Z

Concept ACK

sedited · 2026-06-28T21:01:31Z

Since we recently discussed removing the index caches, might we just do that instead?

maflcko · 2026-06-29T07:07:25Z

Since we recently discussed removing the index caches, might we just do that instead?

Hmm, I may have missed the discussion. Maybe someone can create a separate pull, and then this one can be closed/rebased? (I've moved to draft for now)

Personally, I'd still think even without the indexes, the change here should be done for the other caches as a belt-and-suspenders refactor.

sedited · 2026-06-29T16:50:25Z

Hmm, I may have missed the discussion. Maybe someone can create a separate pull, and then this one can be closed/rebased?

It was brought up on a recent IRC meeting: https://www.erisian.com.au/bitcoin-core-dev/log-2026-06-18.html#l-317

(I've moved to draft for now)
the change here should be done for the other caches as a belt-and-suspenders refactor.

Yes, mind moving it out of draft again?

l0rinc · 2026-06-29T17:52:48Z

Since we recently discussed removing the index caches, might we just do that instead?

Pushed it to #35620

l0rinc

ACK fabd75f

Personally I would prefer the other cleanups be merged before this, but I don't mind if this gets in first - it's a good change.

Rebased locally, all tests are passing, changes make sense. Left some nits.

l0rinc · 2026-06-29T18:02:55Z

-        constexpr auto max_db_cache{sizeof(void*) == 4 ? MAX_32BIT_DBCACHE : std::numeric_limits<size_t>::max()};
-        return std::max<size_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, max_db_cache));
+        constexpr uint64_t max_db_cache{sizeof(void*) == 4 ? MAX_32BIT_DBCACHE : std::numeric_limits<uint64_t>::max()};
+        return std::max<uint64_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, max_db_cache));


Suggested change

return std::max<uint64_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, max_db_cache));

return std::max(MIN_DB_CACHE, std::min(db_cache_bytes, max_db_cache));

Could probably use std::clamp here instead

max/min was chosen exactly the avoid the potential ambiguity of clamp for cases when the min isn't smaller than the max...

Right, but I wonder if it matters. I also wondered if it makes sense to introduce a safe Clamp<MIN,MAX>(val), but not sure if it is worth it.

I'm fine with both, already acked.

maflcko · 2026-07-06T07:38:50Z

Yes, mind moving it out of draft again?

Sure, done

theStack

Code-review ACK fabd75f

sedited

ACK fabd75f

…rflow This is a refactor on 64-bit systems, because size_t is equal to u64. However, on 32-bit systems, it fixes an integer overflow while calculating the cache sizes: src/node/caches.cpp:71:49: runtime error: unsigned integer overflow: 471859200 * 10 cannot be represented in type size_t (aka "unsigned int") This happens while multiplying the default cache size (450MiB) by 10: index_sizes.tx_index = std::min(total_cache * 10 / 100, ...) ^^^^^^^^^^^^^^^^ The issue was introduced in commit d06dabf. ==== Also, add missing includes in touched files, according to IWYU.

sedited

Re-ACK fabafd9

theStack

re-ACK fabafd9

l0rinc · 2026-07-09T00:23:21Z

ACK fabafd9

I would prefer merging #35200 first, but this is also a good change.

maflcko · 2026-07-09T07:39:21Z

I would prefer merging #35200 first, but this is also a good change.

Looks like it has outstanding review feedback from all reviewers: https://github.com/bitcoin/bitcoin/pull/35200/changes#r3436118842. So I'd say it would be kind to address the feedback if all reviewers are asking for it. When two pulls conflict, it seems less churn to rebase the one that has to be force pushed anyway, than to force push both. But this is just my thinking, and it is up to the maintainers to decide the merge order.

sedited · 2026-07-09T08:32:11Z

I think it is preferable to fix these types first, so putting this in now.

…05fc7ab 81405fc7ab Merge bitcoin/bitcoin#35689: test: Inline incorrect check in `util_tests` cd2a4bc510 test: Redeclare variable as signed in `util_tests` 6f1c56f03a Merge bitcoin/bitcoin#35670: net: optimize compact block extra tx iteration e94fda8a40 Merge bitcoin/bitcoin#35685: doc: Archive 30.3 release notes 5223cf1795 Merge bitcoin/bitcoin#35616: refactor: Use u64 over size_t for all cache sizes to fix a 32-bit overflow c0e91efdb3 Merge bitcoin/bitcoin#35295: validation: fetch block input prevouts in parallel during ConnectBlock f0da26cfc8 Merge bitcoin/bitcoin#34997: p2p: Don't participate in addr relay with feelers 443179a9eb doc: Archive 30.3 release notes fabafd91f1 refactor: Use u64 over size_t for all cache sizes to fix a 32-bit overflow 32941e1314 Merge bitcoin/bitcoin#35649: depends: move FreeBSD SDK handling to CI f379d716b1 Merge bitcoin/bitcoin#35669: doc: archive release notes for v31.1 881bb7b355 Merge bitcoin/bitcoin#35673: refactor: Move LoadGenesisBlock to ChainstateManager 3f3e644beb Merge bitcoin/bitcoin#35678: private broadcast: define and use new RPC_LIMIT_EXCEEDED error code ( + other follow-ups) e3b026bf56 Merge bitcoin/bitcoin#34020: mining: add getTransactions(ByWitnessID) IPC methods 62f9089343 Merge bitcoin/bitcoin#35386: doc: add an AI contribution policy fa615bd163 refactor: Move LoadGenesisBlock to ChainstateManager 8ac222484c private broadcast: remove no-op [[nodiscard]] 191bdcba26 test: align test better with described scenario 7ad311be18 test: use BOOST_CHECK_EQUAL for PrivateBroadcast::AddResult 82a02a2a22 rpc: define and use new RPC_LIMIT_EXCEEDED error code a64df338e6 Merge bitcoin/bitcoin#35651: doc: Improve offline-signing-tutorial after 32489 4498fa5d5b Merge bitcoin/bitcoin#35406: private broadcast: limit outstanding txs to count of 10,000 ec5edcd72b Merge bitcoin/bitcoin#35661: Update libmultiprocess subtree to add `ThreadMap.makePool` method ea67fea062 Merge bitcoin/bitcoin#35464: kernel: Add function for creating chainparams with a signet challenge a7b5f23bb8 Merge bitcoin/bitcoin#35667: refactor: Use `NetworkErrorString` for macOS code in `netif.cpp` 69fc991791 Merge bitcoin/bitcoin#32606: p2p: Drop unsolicited CMPCTBLOCK from non-HB peer and when blocksonly 1a3cbf1bd2 net: optimize compact block extra tx iteration bc33509ae2 Merge bitcoin/bitcoin#35650: doc: Add release notes for 32489 (exportwatchonlywallet RPC) 2b6e767d96 doc: archive release notes for v31.1 b0735336ee p2p: Don't participate in addr relay with feeler connections 302733fd96 Merge bitcoin/bitcoin#35652: init: fix reindex deadlock by waking cv after interrupt c1313b199f init: wake genesis wait after ImportBlocks() returns eccb04a321 refactor: Use `NetworkErrorString` for macOS code in `netif.cpp` f79ecfd9c7 Merge bitcoin/bitcoin#35658: refactor: Drop unneeded `<sys/types.h>` include before `<ifaddrs.h>` 22ac4ad949 ci: ensure we use correct lld version in OpenBSD job 495f43f7b3 ci: FreeBSD 15.1 244739db9d depends: move FreeBSD SDK handling to CI bab0120053 Merge bitcoin/bitcoin#35621: validation: Ignore eventual error message from flushing in AcceptBlock 02afa66169 Merge commit '6b0a907302364649dcaffeb0340b985f14141b4e' into pr/subtree-11 6b0a907302 Squashed 'src/ipc/libmultiprocess/' changes from 3edbe8f67c1..16bf05dea02 22c328d388 refactor: Drop unneeded `<sys/types.h>` include before `<ifaddrs.h>` 2063f02bd5 Merge bitcoin/bitcoin#35510: test: SOCKS5 proxy: expect that connection may be reset during SOCKS5 handshake or data forwarding 1360001f43 Merge bitcoin/bitcoin#34959: wallet: Enforce BDB btree levels and overflow item sizes 10ffef4b3b Merge bitcoin/bitcoin#35604: log: expose `-logratelimit` in normal help b393985aa0 Merge bitcoin/bitcoin#35634: txospenderindex: use zero-byte entry values 1835f2fcbf Merge bitcoin/bitcoin#35653: fuzz: Remove `ConsumeUniValue` 9f3e427228 fuzz: Remove ConsumeUniValue 68cb7840d2 doc: improve offline-signing-tutorial after 32489 32ddfc92d9 Merge bitcoin/bitcoin#35599: doc: Add release notes for #33671 (getbalances nonmempool field) f56804bcf5 Merge bitcoin/bitcoin#35640: ci: use a 8x instance over 16x for riscv job 239d6c5260 Merge bitcoin/bitcoin#35614: HTTPServer: Prevent race condition between worker thread and I/O thread cddbad325d doc: Add release notes for 32489 (exportwatchonlywallet RPC) 0a1bbec688 Merge bitcoin/bitcoin#32489: wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet 4a007126fb Merge bitcoin/bitcoin#35147: depends: Boost 1.91.0-1 31abaa264c doc: add an AI contribution policy a15bdc0598 doc: update offline-signing-tutorial to use exportwatchonlywallet rpc a388076401 test: Test for exportwatchonlywallet d053e3e5c8 wallet, rpc: Add exportwatchonlywallet RPC 444878efef wallet: Add CWallet::ExportWatchOnly f9273f01db wallet: Move listdescriptors retrieving from RPC to CWallet a1c83789a7 wallet: Write new descriptor's cache in AddWalletDescriptor 1e996640e6 wallet: Use Descriptor::CanSelfExpand() in CanGetAddresses() d2ee9227da descriptor: Add CanSelfExpand() 2990bd7735 Merge bitcoin/bitcoin#35118: fuzz: add ipc round-trip fuzz target fb1d152c24 depends: Boost 1.91.0-1 ba48852f9e Merge bitcoin/bitcoin#35438: test: introduce NodeSigner, run feature_taproot.py without wallet compiled 47bbed052e ci: use true|false over "true|false" 9a25bc3989 ci: use a 8x instance over 16x 9871dc7ab4 Merge bitcoin/bitcoin#31425: CI: Add Riscv bare metal job 113402286e doc: add txospenderindex release note ce06878288 index: shrink txospenderindex value markers a99148d576 test kernel: Don't log on warnings change 5b4fd284f4 kernel: Generate a signet with a challenge 037ad77071 fuzz: add IPC round-trip target a8823c0996 Merge bitcoin/bitcoin#35607: nanobench: fix performance counter buffer initialization f26c15bdd2 Merge bitcoin/bitcoin#35397: ci: add OpenBSD Clang cross job 91586f701e test: introduce NodeSigner, run feature_taproot.py without wallet compiled 771200ca43 test: return full keypair from `getnewdestination` helper 4e29de719e private broadcast: add release note for limited cap cbf8c107c1 Release cs_main between individual private tx re-attempts 5aea3d0373 private broadcast: limit outstanding txs to count of 10,000 dc282ff31d Merge bitcoin/bitcoin#35597: logging: More fully remove libevent log category ec98037f7a Merge bitcoin/bitcoin#35615: fuzz: restore CreateSock in PCP targets 43d89bb6e1 Merge bitcoin/bitcoin#35610: bitcoin-util: Add netmagic command e35c805489 Merge bitcoin/bitcoin#35129: test: add fuzz test for private broadcast dc1c17c085 doc: add release notes 0e10937184 fuzz: add coins_view_stacked fuzz harness to test concurrent leveldb reads ce610a6ff4 fuzz: update harnesses to cover CoinsViewOverlay::StartFetching 760fb22dc3 test: add unit tests for CoinsViewOverlay::StartFetching d69a3b20de doc: update CoinsViewOverlay docstring to describe parallel fetching ab2a379237 coins: fetch inputs in parallel fdf283036a coins: add ready flag to InputToFetch ede11b8314 validation: collect block inputs in CoinsViewOverlay before ConnectBlock f82043af50 coins: introduce thread pool in CoinsViewOverlay 5bf1c32008 validation: add -prevoutfetchthreads configuration option a318f43254 bitcoin-util: Add netmagic command 256482ab56 validation: In AcceptBlock, ignore flush result 3c76bd4356 Merge bitcoin/bitcoin#35603: build: QRencode cleanups f595daf1dd test: ensure HTTPServer race condition is fixed b98b10c072 test: introduce a worker thread in http socket error test 922b08d375 test: socket error handling in HTTPServer using ErrorSock mock socket 73da2a8a52 http: prevent race condition between worker thread and I/O thread 57b3bf8496 Merge bitcoin/bitcoin#35609: ci: Bump tsan config to ubuntu:26.04 with -U_FORTIFY_SOURCE d64ea15824 ci: add openBSD cross CI job 5404b62074 depends: add openbsd_LDFLAGS 2a9e35d293 Merge bitcoin/bitcoin#35588: scripted-diff: Rename `Sock::{RECV,SEND,ERR}` a0e5e30010 fuzz: restore CreateSock in PCP targets 41ceea400e scripted-diff: Rename `StatusLevel::{INFO,WARN,ERR}` f395acdeee scripted-diff: Rename `Sock::{RECV,SEND,ERR}` 7a74f65293 Merge bitcoin/bitcoin#35536: fuzz: share a single mocked steady clock across FuzzedSock instances e1290ce7f7 Merge bitcoin/bitcoin#35543: test: introduce ExtendedPrivateKey and ExtendedPublicKey classes 7ac25c9177 util, refactor: Rename local `ERR` in `Sock::Accept` bbbbab86a8 ci: Bump tsan config to ubuntu:26.04 with -U_FORTIFY_SOURCE 095596ddf7 log: expose -logratelimit option ea9afb61a1 Merge bitcoin/bitcoin#35602: doc: Clarify build docs about `pkgconf` / `pkg-config` requirements 672eedc46b Merge bitcoin/bitcoin#35220: fuzz: connman: strengthen assertions and extend coverage 0e5c718d8a Merge bitcoin/bitcoin#35506: test: ensure group data cluster pointers are live b6b1d0653a nanobench: fix perf counter buffer init 93012d7ff9 Merge bitcoin/bitcoin#35601: wallet: remove experimental warning from send and sendall 829255c8be cmake: Remove `SelectLibraryConfigurations` from `FindQRencode` module 5c55606da9 depends: Remove unused `lib/pkgconfig` in `qrencode` package 402ba10b20 cmake: Drop optional `PkgConfig` use in `FindQRencode` module 8ebfff0f88 doc: add send RPC release note fb8a103886 doc: Clarify build docs about `pkgconf` / `pkg-config` requirements 5884f5a4fa wallet: remove experimental warning from send RPCs 7b84e5106c Merge bitcoin/bitcoin#35595: ci: remove some packages from Chimera job 58560c281d ci: remove some packages from Chimera job 295ce6f45c Merge bitcoin/bitcoin#35576: test: raise `feature_reindex` RPC timeout b36730a3ef Add CI job for riscv bare metal bfdbf513f6 Add CI job for producing a static bare metal binary a9a1d92a1d build: Add option for building for bare metal envs 9b2ef81757 doc: add release notes for #33671 (getbalances nonmempool field) 633044f143 Merge bitcoin/bitcoin#35266: rpc, wallet: add an option to not load the wallet after migrating d6269e2a90 Merge bitcoin/bitcoin#35594: fuzz: cover async chainstate compaction 3765b428d1 logging: More fully remove libevent log category 703a671fbc fuzz: compact coins view db during fuzzing 0868c85fd5 refactor: rename async coin compaction 6fa4132298 fuzz: share a single mocked steady clock across FuzzedSock instances 8791c4764c test: use ExtendedPrivateKey in wallet_taproot.py 89ceafafb9 test: use ExtendedPrivateKey in wallet_listdescriptors.py bbfffcab58 test: use ExtendedPrivateKey in wallet_send.py 2ab6e590f7 test: use ExtendedPrivateKey in wallet_keypool.py 9e20118720 test: use ExtendedPrivateKey in wallet_fundrawtransaction.py 06af0cddbb test: use ExtendedPrivateKey in wallet_descriptor.py 4100fac20e test: use ExtendedPrivateKey in wallet_createwallet.py ff3f6def9a test: use ExtendedPrivateKey in wallet_bumpfee.py 003f2a01f6 test: use ExtendedPrivateKey in feature_notifications.py f988e6d6e6 test: use ExtendedPrivateKey in wallet_importdescriptors.py 0cdd817a82 add release note 517d37ce3e test: tests wallet migration with load_wallet disabled b98dd63da7 rpc: Add load_wallet argument to migratewallet RPC 4acd063ba6 wallet: make loading the wallet after migrating optional 1a3cfdf1b7 fuzz: connman: cover AddLocalServices/RemoveLocalServices c507fb3063 fuzz: connman: add outbound-bytes invariants 4a6fce43ea fuzz: connman: add AddNode/RemoveAddedNode invariants 9e6546c517 test: raise reindex mining RPC timeout d2a03d50ac test: add extendedkey.py unit tests by using BIP32 test vectors afdb378082 test: introduce ExtendedPrivateKey and ExtendedPublicKey classes 55e3a57f22 qa: Avoid UTXO reuse between test functions 9c5dd2926a p2p: Ignore CMPCTBLOCK from peer that hasn't sent SENDCMPCT bf9884f4e5 p2p: make blocksonly nodes ignore CMPCTBLOCK messages 92cea63c71 test: (Un)solicited invalid cb -> get disconnected. e845e26344 test: p2p: Nodes ignore unsolicited CMPCTBLOCK's 8313591715 p2p: Drop unsolicited CMPCTBLOCK from non-HB peer 44f377a71f refactor: test: Static assert_highbandwidth_states 25457a3272 test: Tighten getblocktxn checks in parallel cb reconstruction test. 2ee4fafa3f test: add fuzz test for private broadcast 08b7c61fc7 private broadcast: enforce sending to unique node ids 9a8ef9b0a3 test: SOCKS5 proxy: expect that connection may be reset during handshake eb3208364a test: SOCKS5 proxy: expect that connection may be reset when forwarding 4dbaa7cc65 test: generalise byte_to_base58 utility function to allow more version types df9eb72b12 test: ensure group data cluster pointers are live a5859edef4 fuzz: connman: set m_local_services/m_use_addrman_outgoing/m_max_automatic_connections 4b84c9125a fuzz: connman: add network activity invariants 9784818442 mining: add getTransactionsByWitnessID() IPC method d282ae6883 mining: add getTransactionsByTxID() IPC method 0d5e4d4712 test: restart node after IPC option override test f16b3613cd ipc: Serialize null CTransactionRef as empty Data 0f466e1094 mempool: add lookup by witness hash 51dd90fb50 refactor: Merge announce_cmpct_block() defs into one 97d08d62ba refactor: store wallet names to MigrationResult 8a739a5510 build: allow ipc fuzz builds b2de59d486 wallet, bdbro: Validate btree page levels dc3a2b9c3b wallet, bdbro: Enforce overflow data lengths git-subtree-dir: libbitcoinkernel-sys/bitcoin git-subtree-split: 81405fc7abbd1889f3978b8924e7acbe12b3403b

…che sizes to fix a 32-bit overflow 0444042 refactor: Use u64 over size_t for all cache sizes to fix a 32-bit overflow (MarcoFalke) Pull request description: This is a refactor on 64-bit systems, because size_t is equal to u64. However, on 32-bit systems, it fixes an integer overflow while calculating the cache sizes: ``` src/node/caches.cpp:71:49: runtime error: unsigned integer overflow: 471859200 * 10 cannot be represented in type size_t (aka "unsigned int") ``` This happens while multiplying the default cache size (450MiB) by 10: ``` index_sizes.tx_index = std::min(total_cache * 10 / 100, ...) ^^^^^^^^^^^^^^^^ ``` The issue was introduced in commit 041a9b3. ---- This change follows similar changed one in the past, like 2fae0fa, 1eef3be, or df90a09. Generally, using fixed sized integer types for calculations is beneficial, because all platforms behave exactly the same way. With platform-dependent types there is a risk that the same calculation yields different results. This has several resulting benefits: * Easier review, because there is no need to review the same code several times for each supported platform. * Easier quality assurance, because there is less need to run the same code several times in sanitizers for each supported platform, which is [tedious](bitcoin/bitcoin#32375 (comment)). There are also no downsides, because there is no measurable overhead on 32-bit for u64 calculations that are done only once in the lifetime of the program. Also, there is no measurable memory overhead when a few fields on 32-bit store some extra zero bytes. ---- As said, testing is only possible by picking one of the tedious options: * Apply a diff on 64-bit arch and compile with `-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DSANITIZERS=integer` ```diff diff --git a/src/node/caches.cpp b/src/node/caches.cpp index c98b8ce..cfd49b60d3 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -58,3 +58,3 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) { - size_t total_cache{CalculateDbCacheBytes(args)}; + uint32_t total_cache(CalculateDbCacheBytes(args)); @@ -72,6 +72,6 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) IndexCacheSizes index_sizes; - 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); + index_sizes.tx_index = std::min<uint32_t>(total_cache * 10 / 100, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0); + index_sizes.txospender_index = std::min<uint32_t>(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 * 5 / 100, MAX_FILTER_INDEX_CACHE); + size_t max_cache = std::min<uint32_t>(total_cache * 5 / 100, MAX_FILTER_INDEX_CACHE); index_sizes.filter_index = max_cache / n_indexes; ``` This will give a roughly similar error: ``` sh-5.3$ echo 'Bw==' | base64 -d > /tmp/blob sh-5.3$ UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=block_index_tree ./bld-cmake/bin/fuzz /tmp/blob ./src/node/caches.cpp:73:59: runtime error: unsigned integer overflow: 1073741824 * 10 cannot be represented in type 'uint32_t' (aka 'unsigned int') #0 0x55ca78da5c47 in node::CalculateCacheSizes(ArgsManager const&, unsigned long) ./src/node/caches.cpp:73:59 ``` * Alternatively, to reproduce in a fresh `podman run -it --rm --platform linux/i386 debian:unstable`: ``` export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git ./b-c && cd b-c && apt install build-essential cmake pkg-config python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev systemtap-sdt-dev libcapnp-dev capnproto libqrencode-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev clang llvm libc++-dev libc++abi-dev mold -y && cmake -B ./bld-cmake -DAPPEND_CXXFLAGS='-O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=mold -DCMAKE_C_COMPILER='clang;-ftrivial-auto-var-init=pattern' -DCMAKE_CXX_COMPILER='clang++;-ftrivial-auto-var-init=pattern' -DSANITIZERS=address,float-divide-by-zero,integer,undefined --preset=dev-mode && cmake --build ./bld-cmake --parallel $(nproc) echo 'Bw==' | base64 -d > /tmp/blob UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=block_index_tree ./bld-cmake/bin/fuzz /tmp/blob # or: UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ASAN_OPTIONS="detect_leaks=0" ./bld-cmake/bin/test_bitcoin-qt ``` ACKs for top commit: l0rinc: ACK 0444042 sedited: Re-ACK 0444042 theStack: re-ACK 0444042 Tree-SHA512: cebbc29636b4074917c96cf3af8fcc176dcd328821d5032dc8609475e18778dfda4e287ded889c024ab29b79d81004bc9a8e57a88eaa0a5eaefe5f8bba1462ab

DrahtBot added the Refactoring label Jun 28, 2026

maflcko force-pushed the 2606-fix-u32 branch from fa54438 to faa5239 Compare June 28, 2026 09:59

DrahtBot added the CI failed label Jun 28, 2026

maflcko force-pushed the 2606-fix-u32 branch from faa5239 to fabd75f Compare June 28, 2026 10:00

maflcko mentioned this pull request Jun 28, 2026

node: allocate index caches proportional to usage patterns #34636

Merged

DrahtBot removed the CI failed label Jun 28, 2026

maflcko marked this pull request as draft June 29, 2026 07:03

maflcko added this to the 32.0 milestone Jun 29, 2026

This was referenced Jun 29, 2026

kernel,node: add dbcache setter and clarify defaults #35205

Open

node: smooth oversized dbcache warnings #35200

Merged

DrahtBot mentioned this pull request Jun 29, 2026

kernel: Replace leveldb-based BlockTreeDB with WAL and .dat file based store #32427

Draft

l0rinc approved these changes Jun 29, 2026

View reviewed changes

DrahtBot requested a review from fjahr June 29, 2026 19:43

maflcko marked this pull request as ready for review July 6, 2026 07:37

theStack approved these changes Jul 8, 2026

View reviewed changes

Comment thread src/dbwrapper.h

sedited approved these changes Jul 8, 2026

View reviewed changes

maflcko force-pushed the 2606-fix-u32 branch from fabd75f to fabafd9 Compare July 8, 2026 14:40

sedited approved these changes Jul 8, 2026

View reviewed changes

DrahtBot requested review from l0rinc and theStack July 8, 2026 15:19

theStack approved these changes Jul 8, 2026

View reviewed changes

DrahtBot mentioned this pull request Jul 9, 2026

util: Add util::NotNull<SmartPtrType> #34844

Open

sedited merged commit 5223cf1 into bitcoin:master Jul 9, 2026
28 checks passed

maflcko deleted the 2606-fix-u32 branch July 9, 2026 09:01

	return std::max<uint64_t>(MIN_DB_CACHE, std::min<uint64_t>(db_cache_bytes, max_db_cache));
	return std::max(MIN_DB_CACHE, std::min(db_cache_bytes, max_db_cache));

Uh oh!

Conversation

maflcko commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

DrahtBot commented Jun 28, 2026

Uh oh!

fjahr commented Jun 28, 2026

Uh oh!

sedited commented Jun 28, 2026

Uh oh!

maflcko commented Jun 29, 2026

Uh oh!

sedited commented Jun 29, 2026

Uh oh!

l0rinc commented Jun 29, 2026

Uh oh!

l0rinc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

l0rinc Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

maflcko Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

maflcko Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Jul 6, 2026

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

l0rinc commented Jul 9, 2026

Uh oh!

maflcko commented Jul 9, 2026

Uh oh!

sedited commented Jul 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maflcko commented Jun 28, 2026 •

edited

Loading

DrahtBot commented Jun 28, 2026 •

edited

Loading