when `GetTotalRAM` returns an `std::nullopt` now we're getting:
```
The following tests did not run:
106 - system_ram_tests (Skipped)
```
GitHub-Pull: #33435
Rebased-From: 56791b582958e905e5ba5cbf172a8ea7dad1a8b0
This patch achieves two things:
1. Fix unused variable warning (https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2362493046)
2. Enable GetTotalRAM() on other platforms where it was tested to work.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Github-Pull: #33435
Rebased-From: 337a6e738616781f81504275bac8ed7bcf8068df
Oversized allocations can cause out-of-memory errors or [heavy swapping](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663637321), [grinding the system to a halt](https://x.com/murchandamus/status/1964432335849607224).
`LogOversizedDbCache()` now emits a startup warning if the configured `-dbcache` exceeds a cap derived from system RAM, using the same parsing/clamping as cache sizing via CalculateDbCacheBytes(). This isn't meant as a recommended setting, rather a likely upper limit.
Note that we're not modifying the set value, just issuing a warning.
Also note that the 75% calculation is rounded for the last two numbers since we have to divide first before multiplying, otherwise we wouldn't stay inside size_t on 32-bit systems - and this was simpler than casting back and forth.
We could have chosen the remaining free memory for the warning (e.g. warn if free memory is less than 1 GiB), but this is just a heuristic, we assumed that on systems with a lot of memory, other processes are also running, while memory constrained ones run only Core.
If total RAM < 2 GiB, cap is `DEFAULT_DB_CACHE` (`450 MiB`), otherwise it's 75% of total RAM.
The threshold is chosen to be close to values commonly used in [raspiblitz](https://github.com/raspiblitz/raspiblitz/blob/dev/home.admin/_provision.setup.sh#L98-L115) for common setups:
| Total RAM | `dbcache` (MiB) | raspiblitz % | proposed cap (MiB) |
|----------:|----------------:|-------------:|-------------------:|
| 1 GiB | 512 | 50.0% | 450* |
| 2 GiB | 1536 | 75.0% | 1536 |
| 4 GiB | 2560 | 62.5% | 3072 |
| 8 GiB | 4096 | 50.0% | 6144 |
| 16 GiB | 4096 | 25.0% | 12288 |
| 32 GiB | 4096 | 12.5% | 24576 |
[Umbrel issues](https://github.com/getumbrel/umbrel-os/issues/64#issuecomment-663816367) also mention 75% being the upper limit.
Starting `bitcoind` on an 8 GiB rpi4b with a dbcache of 7 GiB:
> ./build/bin/bitcoind -dbcache=7000
warns now as follows:
```
2025-09-07T17:24:29Z [warning] A 7000 MiB dbcache may be too large for a system memory of only 7800 MiB.
2025-09-07T17:24:29Z Cache configuration:
2025-09-07T17:24:29Z * Using 2.0 MiB for block index database
2025-09-07T17:24:29Z * Using 8.0 MiB for chain state database
2025-09-07T17:24:29Z * Using 6990.0 MiB for in-memory UTXO set (plus up to 286.1 MiB of unused mempool space)
```
Besides the [godbolt](https://godbolt.org/z/EPsaE3xTj) reproducers for the new total memory method, we also tested the warnings manually on:
- [x] Apple M4 Max, macOS 15.6.1
- [x] Intel Core i9-9900K, Ubuntu 24.04.2 LTS
- [x] Raspberry Pi 4 Model B, Armbian Linux 6.12.22-current-bcm2711
- [x] Intel Xeon x64, Windows 11 Home Version 24H2, OS Build 26100.4351
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: w0xlt <woltx@protonmail.com>
Github-Pull: #33333
Rebased-From: 168360f4ae47cbfdb30a2cc4704435bc67e12f16
Added a minimal system helper to query total physical RAM on [Linux/macOS/Windows](https://stackoverflow.com/a/2513561) (on other platforms we just return an empty optional).
The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 10 TiB.
The max value is only validated on 64 bits, since it's not unreasonable for 32 bits to have max memory, but on 64 bits it's likely an error.
https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-memorystatusex
> ullTotalPhys The amount of actual physical memory, in bytes.
https://man7.org/linux/man-pages/man3/sysconf.3.html:
> _SC_PHYS_PAGES The number of pages of physical memory. Note that it is possible for the product of this value and the value of _SC_PAGESIZE to overflow.
> _SC_PAGESIZE Size of a page in bytes. Must not be less than 1.
See https://godbolt.org/z/ec81Tjvrj for further details
Github-Pull: #33333
Rebased-From: 6c720459beead5c825b354a1d5c11969b6e3a170
In the presence of smaller transactions on the network, blocks can sustain a
higher relay rate than 7tx/second. In this event, the per-peer inventory queues
can grow too large.
This commit bumps the rate up to 14 tx/s (for inbound peers), increasing the
safety margin by a factor of 2.
Outbound peers continue to receive relayed transactions at 2.5x the rate of
inbound peers, for a rate of 35tx/second.
Co-Authored-By: Suhas Daftuar <sdaftuar@gmail.com>
Github-Pull: #28592
Rebased-From: b81f37031c8f2ccad9346f1b65ee0f8083c44796
Tor inbound connections do not reveal the peer's actual network address.
Therefore do not apply whitelist permissions to them.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Github-Pull: #33395
Rebased-From: f563ce90818d486d2a199439d2f6ba39cd106352
mzumsande pointed out https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369 that this test was causing a warning:
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
Fix by setting regtest instead of mainnet network before running the test.
Github-Pull: #33391
Rebased-From: bdf01c6f61262cd6e211ead3c0dbc66ccb48b32f
m_cached_from_me is used to track whether a transaction is "from me", i.e. has
any inputs which belong to the wallet. This is held in memory only in
the same way that a transaction's balances are.
Github-Pull: #33268
Rebased-From: 113a4228229baedda2a730e097f2d59ad58a4b0d
Instead of checking whether the total amount of inputs known by the
wallet is greater than 0, we should be checking for whether the input is
known by the wallet. This enables us to determine whether a transaction
spends an of output with an amount of 0, which is necessary for marking
0-value dust outputs as spent.
Github-Pull: #33268
Rebased-From: 39a7dbdd277d1dea9a70314d8cc5ae057999ee88
790b440197bde322432a5bab161f1869b667e681 Fix benchmark CSV output (Hennadii Stepanov)
Pull request description:
The `SHA256AutoDetect` return output is used, among other use cases, to name benchmarks. Using a comma breaks the `bench_bitcoin` CSV output.
This PR replaces the comma with a semicolon, which fixes https://github.com/bitcoin/bitcoin/issues/33331.
ACKs for top commit:
Raimo33:
Code Review ACK 790b440197bde322432a5bab161f1869b667e681
l0rinc:
Code review ACK 790b440197bde322432a5bab161f1869b667e681
janb84:
code review ACK 790b440197bde322432a5bab161f1869b667e681
Tree-SHA512: 096bfa29a0639a4d97d510a3e2a15f071f384148c3035e4d0fc525794682e499c45a0d0c95728d5c78010098393b2c486a7fa9c21c1e2fbb600dea7c5638a55f
8b6264768030db1840041abeeaeefd6c227a2644 test: send duplicate blocktxn message in p2p_compactblocks.py (Eugene Siegel)
5e585a0fc4fd68dd7b4982054b34deae2e7aeb89 net: check for empty header before calling FillBlock (Eugene Siegel)
Pull request description:
This avoids an Assume crash if multiple blocktxn messages are received. The first call to `FillBlock` would make the header empty via `SetNull` and the call right before the second `FillBlock` would crash [here](689a321976/src/net_processing.cpp (L3333)) since `LookupBlockIndex` won't find anything. Fix that by checking for an empty header before the Assume.
ACKs for top commit:
instagibbs:
reACK 8b62647680
fjahr:
tACK 8b6264768030db1840041abeeaeefd6c227a2644
achow101:
ACK 8b6264768030db1840041abeeaeefd6c227a2644
mzumsande:
Code Review ACK 8b6264768030db1840041abeeaeefd6c227a2644
Tree-SHA512: d43a6f652161d4f7e6137f207a3e95259fc51509279d20347b1698c91179c39c8fcb75d2668b13a6b220f478a03578573208a415804be1d8843acb057fa1a73a
c76797481155754329ec6a6f58e8402569043944 clang-tidy: Fix critical warnings (Fabian Jahr)
54dc34ec2279378c78fa2d9155668e39e20decda index: Remove unused coinstatsindex recovery code (Fabian Jahr)
37c4fba1f4c18632bceb16f41f5a8f1a61fb9096 index: Check BIP30 blocks when rewinding Coinstatsindex (Fabian Jahr)
51df9de8e5b9c8ecd8339d95b630f312fcb9414e doc: Add release note for 30469 (Fabian Jahr)
bb8d673183294a43c716ff8738da2492f3d7a94b test: Add coinstatsindex compatibility test (Fabian Jahr)
b2e8b64ddc351124ac1390ee906a8fcd2781ca50 index, refactor: Append blocks to coinstatsindex without db read (Fabian Jahr)
431a076ae6e3cc32a8725d4a01483d27c5081a34 index: Fix coinstatsindex overflow issue (Fabian Jahr)
84e813a02bb7b3c735ae413f06c0fc156bfeb7ac index, refactor: DRY coinbase check (Fabian Jahr)
fab842b3248744fb0030486f64d3febe815f8377 index, refactor: Rename ReverseBlock to RevertBlock (Fabian Jahr)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/26362
This continues the work that was started with #26426. It fixes the overflow issue by switching the tracked values that are in danger of overflowing from `CAmount` to `arith_uint256`.
The current approach opts for a simple solution to ensure compatibility with datadirs including the previous version of the index: The new version of the index goes into a separate location in the datadir (`index/coinstatsindex/` rather than `index/coinstats/` before, the new naming is more consistent with the naming of the other indexes). There is no explicit concept of versioning of the index which earlier versions of this PR had. Having the two different versions of the index in separate places allows for downgrading of the node without having to rebuild the index. However, there will be a warning printed in the logs if the new code (v30) detects the old index still being present. A future version could delete a left-over legacy index automatically.
The PR also includes several minor improvements but most notably it lets new entries be calculated and stored without needing to read any DB records.
ACKs for top commit:
achow101:
ACK c76797481155754329ec6a6f58e8402569043944
TheCharlatan:
ACK c76797481155754329ec6a6f58e8402569043944
mzumsande:
Tested / Code Review ACK c76797481155754329ec6a6f58e8402569043944
Tree-SHA512: 3fa4a19dd1a01c1b01390247bc9daa6871eece7c1899eac976e0cc21ede09c79c65f758d14daafc46a43c4ddd7055c85fb28ff03029132d48936b248639c6ab9
Previously in debug builds, this would cause an Assume crash if
FillBlock had been called previously. This could happen when multiple
blocktxn messages were received.
Co-Authored-By: Greg Sanders <gsanders87@gmail.com>
188de70c86414b8b2ad5143f5c607b67686526ea net: Add interrupt to pcp retry loop (TheCharlatan)
Pull request description:
Without this interrupt bitcoind takes a long time to exit if requested to do so after a failed pcp lookup on startup.
ACKs for top commit:
achow101:
ACK 188de70c86414b8b2ad5143f5c607b67686526ea
fjahr:
utACK 188de70c86414b8b2ad5143f5c607b67686526ea
hodlinator:
utACK 188de70c86414b8b2ad5143f5c607b67686526ea
Tree-SHA512: 426dabd10ac0ef5de246c83d281ba70957e4032d251054aa6028b4d7ce4e35cd35ac70e67dc07bd418673bcdd2f4457b76f174ac5e7d0dd3caa05de5da952dac
589b65f06c3376df4cde3fac5c1d39a2d3254920 clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc` (Hennadii Stepanov)
Pull request description:
The warnings are false positive and have been fixed upstream. See: https://github.com/capnproto/capnproto/pull/2334.
This PR:
1. Disables the `UndefinedBinaryOperatorResult` clang-tidy check for source files generated by the `mpgen` tool.
2. Is an alternative to the draft https://github.com/bitcoin/bitcoin/pull/33281.
3. Fixes https://github.com/bitcoin/bitcoin/issues/33256.
ACKs for top commit:
Sjors:
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
fjahr:
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
achow101:
ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920
ryanofsky:
Code review ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920. Thanks for the fix!
Tree-SHA512: 6d376a82641a5b85d4dd1fa164fdcbd8e15f1262e7d4f582f4d9959031d35852e28ff1b8268336e39ba6779fdd10ecdb986af42407d0545f4217f41d64556272
The `SHA256AutoDetect` return output is used, among other use cases, to
name benchmarks. Using a comma breaks the CSV output.
This change replaces the comma with a semicolon, which fixes the issue.
The std::move in coinstatsindex was not necessary since it was passed as a const reference argument.
The other change in the utxo supply fuzz test changes a line that seems to have triggered a false alarm.
The coinstatsindex currently looks for block data at a hash key if the prev block in CustomAppend is different than expected. This is not needed since base index should always prevent us ending up in this scenario since it should rewind the index before calling CustomAppend in this case. But even if we run into this and our belt-and-suspenders code is getting hit, the index could not recover properly from the hash key index data so it can be removed without any real impact.
This is practically irrelevant due to the unlikeliness of a re-org
reaching so deep that it would drop the BIP30 blocks from the chain
(91842 and 91880). However this serves as documentation and ensures that
the functions RevertBlock and CustomAppend are consistent.
The index originally stored cumulative values in a CAmount type but this allowed for
potential overflow issues which were observed on Signet. Fix this by
storing the values that are in danger of overflowing in a arith_uint256.
Also turns an unnecessary copy into a reference in RevertBlock and
CustomAppend and gets
rid of the explicit total unspendable tracking which can be calculated
by adding the four categories of unspendables together.
The warnings are false positive and have been fixed upstream.
See: https://github.com/capnproto/capnproto/pull/2334.
This change disables the `UndefinedBinaryOperatorResult` clang-tidy
check for source files generated by the `mpgen` tool.
When the router doesn't support natpmp and PCP, one'd normally expect
the UDP packet to be ignored, and hit a time out. This logs a warning
that is already in the debug category. However, there's also the case in
which sending an UDP packet causes a ICMP response. This is returned to
user space as "connection refused" (despite UDP having no concept of
connections).
Move the warnings from `Send` and `Recv` to debug level too, to reduce
log spam in that case.
Closes#33301.
88db09bafe9ec363525e5e526c5f6cdd13691447 net: handle multi-part netlink responses (willcl-ark)
42e99ad77396e4e9b02d67daf46349e215e99a0f net: skip non-route netlink responses (willcl-ark)
57ce645f05d18d8ad10711c347a5989076f1f788 net: filter for default routes in netlink responses (willcl-ark)
Pull request description:
...for default route in pcp pinholing.
Currently we only make a single recv call, which trucates results from large routing tables, or in the case the kernel may split the message into multiple responses (which may happen with `NLM_F_DUMP`).
We also do not filter on the default route. For IPv6, this led to selecting the first route with an `RTA_GATEWAY` attribute, often a non-default route instead of the actual default. This caused PCP port mapping failures because the wrong gateway was used.
Fix both issues by adding multi-part handling of responses and filter for the default route.
Limit responses to ~ 1MB to prevent any router-based DoS.
ACKs for top commit:
achow101:
ACK 88db09bafe9ec363525e5e526c5f6cdd13691447
davidgumberg:
Code Review re-ACK 88db09b
Sjors:
re-utACK 88db09bafe9ec363525e5e526c5f6cdd13691447
Tree-SHA512: ea5948edebfad5896a487a61737aa5af99f529fad3cf3da68dced456266948238a7143383847e79a7bb90134e023eb173c25116d8eb80ff57fa4c4a0377ca1ed
Handle multi-part netlink responses to prevent truncated results from
large routing tables.
Previously, we only made a single recv call, which led to incomplete
results when the kernel split the message into multiple responses (which
happens frequently with NLM_F_DUMP).
Also guard against a potential hanging issue where the code would
indefinitely wait for NLMSG_DONE for non-multi-part responses by
detecting the NLM_F_MULTI flag and only continue waiting when necessary.
2885bd0e1c4fc863a7f28ff0fd353f5cffb03442 doc: unify `datacarriersize` warning with release notes (Lőrinc)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/32406
---
The [release notes](a189d63618/doc/release-notes-32406.md (L1)) claim
> [...] marked as deprecated and are expected to be removed in a future release
but the [warning itself](2885bd0e1c/src/init.cpp (L907)) claims
> [...] marked as deprecated. They **will** be removed in a future version.
To be less aggressive (since some have objected against this version online) - and to unify the deprecation warning with the release notes - I have changed the warning to communicate our expectation in a friendlier way.
ACKs for top commit:
cedwies:
ACK 2885bd0
ryanofsky:
Code review ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442. I don't think it is good for the release notes and the runtime warning message to say two different things. I'd also be happy if release notes were updated to match the runtime warning, instead of vice versa. Whatever is more accurate is better.
ajtowns:
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
kevkevinpal:
ACK [2885bd0](2885bd0e1c)
achow101:
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
janb84:
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
Zero-1729:
crACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
jonatack:
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
hodlinator:
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
w0xlt:
ACK 2885bd0e1c
optout21:
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
Tree-SHA512: a9d2a64ab96b3dd7f3a1a29622930054fd5c56e573bc96330f4ef3327dc024b21b3fbc8a698d17aea7c76f57f0c2ccd6403b2df344ae2f69c645ceb8b6fa54a5
b7b249d3adfbd3c7b0c4d0499a86300f57982972 Revert "[refactor] rewrite vTxHashes as a vector of CTransactionRef" (Anthony Towns)
b9300d8d0a74b15a220a2ce529d5157d109c7ed3 Revert "refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef>" (Anthony Towns)
df5a50e5de204013ba56e1c4967a249ca07ba086 bench/blockencodings: add compact block reconstruction benchmark (Anthony Towns)
Pull request description:
Reconstructing compact blocks is on the hot path for block relay, so revert changes from #28391 and #29752 that made it slower. Also add a benchmark to validate reconstruction performance, and a comment giving some background as to the approach.
ACKs for top commit:
achow101:
ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
polespinasa:
lgtm code review and tested ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
cedwies:
code-review ACK b7b249d
davidgumberg:
crACK b7b249d3adfbd3c
instagibbs:
ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
Tree-SHA512: dc266e7ac08281a5899fb1d8d0ad43eb4085f8ec42606833832800a568f4a43e3931f942d4dc53cf680af620b7e893e80c9fe9220f83894b4609184b1b3b3b42
