Skip to content

Conversation

@fanquake fanquake added this to the 30.0 milestone Sep 18, 2025
@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 18, 2025

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/33424.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK willcl-ark, hebasto
Stale ACK l0rinc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

  • there are not "failed: Duplicate ID @0xcc316e3f71a040fb" errors. -> there are no "failed: Duplicate ID @0xcc316e3f71a040fb" errors. [“there are not … errors” is unidiomatic/grammatically awkward; “there are no … errors” is correct and clearer]

drahtbot_id_5_m

@hebasto
Copy link
Member

hebasto commented Sep 19, 2025

Backport #33422?

@l0rinc
Copy link
Contributor

l0rinc commented Sep 22, 2025

crACK 2db6dde

thank you for cherry picking these over

ryanofsky and others added 13 commits September 23, 2025 10:20
This change should fix issue bitcoin#33417
reported by zaidmstrr. It's possible to reproduce the `mp/proxy.capnp:0:
failed: Duplicate ID @0xcc316e3f71a040fb` error by installing libmultiprocess
system-wide, or to one of the locations listed in the python test's `imports`
list before the local libmultiprocess subtree, and then running the test.

Github-Pull: bitcoin#33420
Rebased-From: e9c5227
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: bitcoin#28592
Rebased-From: b81f370
…install

Prior releases installed using these paths. Especially annoying was that the lingering registry entry for the uninstaller would show up as "Bitcoin Core (64-bit)" besides the current "Bitcoin Core" entry in the list of installed programs, and whichever was uninstalled last would fail to work as they would default to the same install directory.

Github-Pull: bitcoin#33422
Rebased-From: 79752b9
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: bitcoin#33333
Rebased-From: 6c72045
Oversized allocations can cause out-of-memory errors or [heavy swapping](getumbrel/umbrel-os#64 (comment)), [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](getumbrel/umbrel-os#64 (comment)) 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: bitcoin#33333
Rebased-From: 168360f
This patch achieves two things:
1. Fix unused variable warning (bitcoin#33333 (comment))
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: bitcoin#33435
Rebased-From: 337a6e7
… determined

when `GetTotalRAM` returns an `std::nullopt` now we're getting:
```
The following tests did not run:
        106 - system_ram_tests (Skipped)
```

GitHub-Pull: bitcoin#33435
Rebased-From: 56791b5
Avoid relying on future truthy evaluations of string 'false'.

Github-Pull: bitcoin#33302
Rebased-From: ff18b6b
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Add an optional matrix field allowing opt-out of configuring cirrus
GHA cache when not using cirrus runners.

This is not needed for the cirruslabs/[save|restore]-cache actions, as
they automatically fallback based on runner type.

Github-Pull: bitcoin#33302
Rebased-From: 00c253d
…escription

`bitcoin-wallet` as-is is merely an offline wallet inspection tool
(introduced more than 9 years ago in PR bitcoin#13926) that doesn't have any
relation with IPC/multiprocess, so remove it from the list of binaries
that use `libbitcoin_ipc`.

Github-Pull: bitcoin#33459
Rebased-From: fbde8d9
Copy link
Member

@willcl-ark willcl-ark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7ebdfa2

Backports are clean.

None added to release notes, but I don't think any particularly need to be (#33333 could be the only potential candidate here IMO).

@DrahtBot DrahtBot requested a review from l0rinc September 23, 2025 19:47
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7ebdfa2, I applied all backports locally without conflicts and obtained a zero diff with this PR branch.

@fanquake fanquake merged commit 72c1f13 into bitcoin:30.x Sep 23, 2025
20 checks passed
@fanquake fanquake deleted the 30_0_rc2 branch September 23, 2025 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants