0b0785daa0 guix: split macOS and win builds (fanquake)
008a3e29c8 guix: split builds into Linux(gui) and macOS/Windows (fanquake)
a3b5dc0572 cmake: Add `GenerateWindowsInstaller` script (Hennadii Stepanov)
Pull request description:
The next change split out of #25573, which splits the Guix build into separate Linux, Linux GUI, macOS, macOS GUI and Windows and Windows GUI builds.
Closes#29914.
ACKs for top commit:
hebasto:
ACK 0b0785daa0.
Tree-SHA512: 6222024c891d1bec69bb93ae28864142d7e75aae31968b727a3f7226e0debe89f2dc6954a0c26ccfccf2ce8a28d002aa0284a778c2df6c330c5262d41057602d
5d57f2cefe test: cover unused mempool space in coins cache (woltx)
Pull request description:
This PR extends the existing unit test for `Chainstate::GetCoinsCacheSizeState()`.
`Chainstate::GetCoinsCacheSizeState()` calculates when the UTXO/coins cache is too large and should be flushed. Part of that calculation includes unused `-maxmempool` space. For example, if the mempool limit is 300 MiB but the mempool is mostly empty, some of that unused space can be counted toward the coins cache limit.
The existing `validation_flush_tests.cpp` test checks this calculation by calling:
```c++
chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes)
```
This change extends the test to also check the no-argument call:
```c++
chainstate.GetCoinsCacheSizeState()
```
That is the call used by validation code during normal operation.
The test grows the coins cache above the coins-only limit, then checks that:
- calling the explicit helper with `max_mempool_size_bytes=0` reports `CRITICAL`
- calling the normal no-argument method reports `OK`, because it includes unused mempool space
This makes sure future refactors do not accidentally drop unused mempool space from the normal cache-size calculation.
ACKs for top commit:
l0rinc:
ACK 5d57f2cefe
sedited:
ACK 5d57f2cefe
Tree-SHA512: c4131cdedd7bd48224d3323fdc1bbce668f171105939c8aa3c452080bbe22819f3910d2a5ce3f46a5a1b7eb625b63240a8c0b7c26fbd5f30927712545b0f765f
1fc9277a1c test: cover disconnect on private broadcast peer with relay=false (Bruno Garcia)
Pull request description:
Currently, there is no test case to verify the disconnection of a private broadcast connection when the peer does not support transaction relay. This PR addresses it.
Can be tested with:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 27f0a63c55..5d9a8f9c06 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3741,7 +3741,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
} else {
LogDebug(BCLog::PRIVBROADCAST, "Disconnecting: does not support transaction relay (connected in vain), %s",
pfrom.LogPeer());
- pfrom.fDisconnect = true;
+ pfrom.fDisconnect = false;
}
return;
}
```
ACKs for top commit:
Herb-ops:
ACK 1fc9277a1c
w0xlt:
ACK 1fc9277a1c
nebula-21:
ACK 1fc9277a1c
Tree-SHA512: 1fd7ad3a5fc690a25276a9654e297858d95607fa9c696a0a6f53223aeb3dde7f80e836d2cbf835a39487f953b04f73b4b8af0c79a2ee8f58c894259f7c9383ff
fd59d68c26 qt, test: Enable tests on macOS with `minimal` QPA plugin (Hennadii Stepanov)
c8b2aeb226 qt: Avoid implicit `NSApplication` instantiation (Hennadii Stepanov)
Pull request description:
On macOS, `test_bitcoin-qt` has skipped `AppTests`, `WalletTests`, and `AddressBookTests` under the `minimal` QPA plugin since a3197c5294 (2018, Qt 5.11), pointing to [QTBUG-49686](https://qt-project.atlassian.net/browse/QTBUG-49686).
The recent re-evaluation of the Bitcoin Core codebase showed that the Qt bug report is no longer relevant. The remaining causes were in our own code, and they are fixed in the first commit.
ACKs for top commit:
maflcko:
lgtm ACK fd59d68c26
pablomartin4btc:
tACK fd59d68c26
Tree-SHA512: 9fae09668cffe8b207736c6a202af01666a06ae1a8adbd3061717b92a692c0b65b6fe853f192cdc8793073b1c02c3f6a61350a5974397ac016ff8924a51e084a
6eca11175b lint: remove E731 Ruff ignore (will)
b52454538b lint: remove E712 Ruff ignore (will)
Pull request description:
Follows up on #34547 by dropping (and fixing) most remaining default Ruff rule ignores:
- [E712](https://docs.astral.sh/ruff/rules/true-false-comparison/) avoids noisy `== True/False` comparisons and makes boolean intent clearer.
- [E731](https://docs.astral.sh/ruff/rules/lambda-assignment/) named `def` helpers are easier to debug, trace, type-check, and extend than assigned lambdas.
Dropping the rules means these issues cannot be re-introduced.
E501 remains as this has 8909 callsites to fix, and IMO it's not clear that having an arbitrary-line length improves anything about the codebase, or for developers.
E741 remains based on review feedback that it's unnecessary to enforce.
ACKs for top commit:
stickies-v:
ACK 6eca11175b
sedited:
ACK 6eca11175b
Tree-SHA512: 763ad88b66fbe1df09c88c688865b7ebdcc4b431858776b5557eb7eec61b98795fb6946b32549f2e631648eae03a6a2882fcd2a673948e1a04d8b2ea358d83ce
`[NSApplication sharedApplication]` creates the shared application
object if it does not yet exist. When running with the `minimal` or
`offscreen` QPA plugins, which is common for testing purposes, the Cocoa
platform plugin never creates it, so these call sites were instantiating
`NSApplication` as a side effect.
Use the `NSApp` global instead and return early when it is `nil`.
d24d3cbad0 fuzz: add p2p_private_broadcast harness (frankomosh)
Pull request description:
Add a fuzz harness for `ConnectionType::PRIVATE_BROADCAST`, a privacy-preserving transaction relay mechanism whose p2p code paths had no meaningful fuzz coverage.
Current `process_message` touches it but is insufficient in exercising it. It creates `PRIVATE_BROADCAST` nodes via `ConsumeNode()`, but some structural problems prevent it from covering the relevant logic:
1. `m_tx_for_private_broadcast` is never seeded, `PushPrivateBroadcastTx` always takes the immediate disconnect path (7 accidental hits, all on lines 3559–3562). Lines 3564–3570 (the actual INV send) had 0 hits.
2. `ALL_NET_MESSAGE_TYPES` is used as the message pool. `CConnman::PushMessage` silently drops anything outside the four-type allowlist for private broadcast connections, wasting most iterations.
3. Connection types are picked randomly, hence private broadcast coverage is accidental.
To solve the issues above;
- this harness explicitly constructs nodes with `ConnectionType::PRIVATE_BROADCAST`
- seeds `m_tx_for_private_broadcast` via `InitiateTxBroadcastPrivate` before the peer connects, so `PushPrivateBroadcastTx` reaches the transaction send path
- constrains the message pool to the four types permitted by handshake or private-broadcast filter in PeerManagerImpl::ProcessMessage
- passes `{NODE_NONE}` to `InitializeNode`, matching what `PushNodeVersion` advertises for private broadcast peers.
ACKs for top commit:
instagibbs:
ACK d24d3cbad0
brunoerg:
code review ACK d24d3cbad0
andrewtoth:
ACK d24d3cbad0
Tree-SHA512: 0d2ff9a79aa87a6eb7d7efdbe03ebba545abad1d1e995dfe617e5bd8fd7a5098fd44d92fa6eac1695aa81f38e762b30ed7a7fd0db6c85aaa32bd1d068c946e38
The new script is parameterized by paths to executables. It can be used
to generate the installer from the install tree rather than the build
tree. This capability will be used by the Guix scripts in subsequent
commits.
6ee05c4b18 test: wallet: BnB incomplete result on attempt-limit success (Bruno Garcia)
Pull request description:
BnB can return a valid selection before exhausting the search tree, then hit `TOTAL_TRIES` while continuing to look for a better one. Add a unit test for that path using a known exhaustion fixture plus an exact-match coin, and assert the result is marked incomplete via `GetAlgoCompleted() == false`.
It kills the following mutant:
```diff
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 8d69957c30..e4d07415f3 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -212,7 +212,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
if (curr_try >= TOTAL_TRIES) {
// Solution is not guaranteed to be optimal if `curr_try` hit TOTAL_TRIES
- result.SetAlgoCompleted(false);
+ result.SetAlgoCompleted(true);
break;
}
```
ACKs for top commit:
yashbhutwala:
tACK 6ee05c4b18
achow101:
ACK 6ee05c4b18
murchandamus:
ACK 6ee05c4b18
Tree-SHA512: 32d0cc6da7586abdd195b87459da0a6ebbdcf8115bef41f5e625864bfa9ce8231a58f8d4b61708279ebeaef372e4f3b5fb18eab28a01dad53f043ba083330b3d
BnB can return a valid selection before exhausting
the search tree, then hit TOTAL_TRIES while continuing
to look for a better one. Add a unit test for that path
using a known exhaustion fixture plus an exact-match
coin, and assert the result is marked incomplete via
GetAlgoCompleted() == false.
Co-authored-by: Murch <murch@murch.one>
3e8e21b2ef txgraph: avoid moving primitive members (Lőrinc)
d9f94aa882 rpc: avoid moving RPC enum types (Lőrinc)
b67baed4e7 coins: avoid moving `COutPoint` values (Lőrinc)
Pull request description:
Inspired by https://github.com/bitcoin/bitcoin/pull/34320#discussion_r2751764873.
**Problem:** A few code paths use rvalue references or `std::move()` for types where moving provides no benefit.
`EmplaceCoinInternalDANGER` took `COutPoint&&`, forcing callers to pass trivially copyable outpoints as rvalues even though the cache stores its own key.
Some call sites also use `std::move()` on enum and primitive values, where it only adds noise.
> [!NOTE]
> `CheckTriviallyCopyableMove` remains `false` since `std::move()` on trivially copyable types can still be useful as intent documentation, for example to signal that a value should not be reused after a call.
**Fix:** Take trivially copyable arguments by const reference where the callee only needs to store its own copy, and pass existing values directly at the call sites.
Also remove `std::move()` from enum and primitive assignments where it has no semantic effect.
ACKs for top commit:
maflcko:
review ACK 3e8e21b2ef 🖇
hodlinator:
re-ACK 3e8e21b2ef
andrewtoth:
ACK 3e8e21b2ef
hebasto:
ACK 3e8e21b2ef, I have reviewed the code and it looks OK.
Tree-SHA512: cbe55b13290ae261bba359dc6e5a3bbdfb7ae9d31bdf8e0da2eef65a0df776e4081ceeac3c82731a631bda656a86b3789651fa1f4d87875cee1dc96351bdfd7c
Serialization parameters should be embedded into the object being
serialized rather than passed as a separate argument. This works here
because only serialization is performed and no new object needs to be
constructed.
Replace assigned lambdas with local functions so Ruff can enforce E731.
For platform-specific immutable file cleanup, store the command as data
instead of creating conditional callbacks.
dab7f2c984 test: cover -externalip/onlynet interaction in functional test (will)
657a5aa3f3 test: cover -externalip bypassing -onlynet (will)
8c87e32bd3 net: let -externalip bypass -onlynet (will)
f4af02e827 net: add an add_even_if_unreachable argument to AddLocal (will)
Pull request description:
`-onlynet` is documented to restrict automatic outbound connections, but it also currently prevents `-externalip` addresses from being advertised when their network is not in the `-onlynet` set. This happens because `AddLocal()` rejects addresses outside `g_reachable_nets`, regardless of whether the address was explicitly configured by the user.
Previous attempts to fix this (#24835 and #25690) removed the `g_reachable_nets` check from `AddLocal()`.
This PR instead adds an explicit `add_even_if_unreachable` argument to `AddLocal()`. The argument defaults to `false`, and is set to `true` only when adding addresses from `-externalip`.
As a result, explicitly configured `-externalip` addresses can still be advertised even when their network is excluded by `-onlynet`, while discovered, mapped, bound, Tor control, and I2P SAM addresses continue to use the existing reachable-network filter.
This keeps the fix scoped to `-externalip` and addresses the concern raised in #25690:
> I think it might also be weird for a user to activate -onlynet and keep on advertising their clearnet address to the network
The branch adds unit coverage for `AddLocal()` and functional coverage in `p2p_addr_selfannouncement.py` for `-onlynet=ipv4 -externalip=<onion>`.
Fixes: #25336Fixes: #25669
ACKs for top commit:
achow101:
ACK dab7f2c984
mzumsande:
re-ACK dab7f2c984
w0xlt:
ACK dab7f2c984
Tree-SHA512: a4ac9334b85da8b6902d3850e21d3a1c9d7dce70bcb79182448c8d5684e24462cd6e440385af7aa4420d9582e4dff9dc9e827ca7a6da0363fff2d3c531784d9b
4c9de7d5b3 external_signer: validate fingerprint from enumerate response (Kyle 🐆)
Pull request description:
`enumeratesigners` takes the `fingerprint` field from the external signer's `enumerate` output and stores it without checking it. That value is later handed back to the signer command as `--fingerprint <value>` (e.g. in `displayaddress`), so a malformed value propagates unchecked.
A master key fingerprint is 4 bytes, i.e. 8 hex characters. This adds a check that the reported fingerprint is exactly 8 hex characters and throws a clear error otherwise. A functional test covers empty, wrong-length, and non-hex inputs.
ACKs for top commit:
Sjors:
utACK 4c9de7d5b3
achow101:
ACK 4c9de7d5b3
sedited:
ACK 4c9de7d5b3
Tree-SHA512: 7c3303b24e234e13a4c20c0b93552145b9ccffc29d1bae42ce8a2faf548377f051e52f8ffb3924679065b27d15fc7bf3859e5ae32a2bb185738cc29bc0ade486
72db4accbf coins: drop stale cursor null checks (Lőrinc)
3d2f2d8de0 coins: pass UTXO stats view by reference (Lőrinc)
35aedb2823 coins: drop cursor from base view (Lőrinc)
c6fbe2f66c coins: pass DB view to cursor users (Lőrinc)
Pull request description:
**Problem:** `CCoinsView::Cursor()` makes cursor iteration look like a generic coins view operation, but cursor iteration is only supported by the DB-backed coins view.
The cache override only threw, and the `coins_view` fuzz target only asserted that deterministic unsupported throw path.
**Fix:** Make cursor iteration a `CCoinsViewDB` operation.
Cursor users now take the DB-backed view directly, `CCoinsView` no longer exposes `Cursor()`, and the fuzz target keeps DB-backed cursor coverage while dropping the unsupported cache throw probe.
The UTXO stats path is also tightened to pass the non-null DB view by reference, and stale null handling for DB cursors is removed.
This was extracted from review discussion in https://github.com/bitcoin/bitcoin/pull/35295#discussion_r3420576781 and extended based on https://github.com/bitcoin/bitcoin/pull/35562#issuecomment-4746585893.
ACKs for top commit:
achow101:
ACK 72db4accbf
sedited:
Re-ACK 72db4accbf
w0xlt:
ACK 72db4accbf
andrewtoth:
ACK 72db4accbf
Tree-SHA512: 12a81330a6ec1b91a7e4393f3761ea9ed4702ecb24312f1defa5a9a079a396ce921fc52f74fe296e5ac7ab20d5b5a8a84e858c96847f333c58b7fa9de9e8143e
3ae3a94f2b wallet: avoid call bumpfeediscount with negative values (Pol Espinasa)
Pull request description:
in https://github.com/bitcoin/bitcoin/issues/34232 dergoegge reported an assertion fail in `SetBumpFeeDiscount`.
**Context**
The bump-fee discount in the savings that we can have when multiple UTXOs that we selected for our transaction share a common unconfirmed ancestor transaction.
We know we can have some savings because we first calculate the `summed_bump_fees` and then compare to the `combined_bump_fee`.
For context:
- `bump_fees`: Extra fees that the new transaction must pay to contribute to get his ancestor confirmed.
- `summed_bump_fees`: The sum of each input ancestor `bump_fee`. If we have two inputs with unconfirmed ancestors A and B and both have a `bump_fee = 100` then `summed_bump_fees = 200`.
- `combined_bump_fee`: Is the total `bump_fees` summed of all inputs, but taking into account shared ancestors. If A and B share the transaction ancestor then `combined_bump_fee = 100` not `200` as the transaction must be bumped only once.
If `summed_bumpfees` > `combined_bump_fee` we are overestimating the `bump_fee` as we are counting multiple times the same ancestors so we can discount it using `SetBumpFeeDiscount(summed_bump_fees - combined_bump_fees)`.
**Problem**
To calculate `summed_bump_fees` and `combined_bump_fee` we use two different fresh MiniMiner snapshots of the mempool. Because they are called in different moments the two snapshots of the mempool might be different. An artificial feerate decrease of an ancestor using `prioritizesettransaction` can make `combined_bump_fee > summed_bump_fees` creating a negative discount. This cause calling `SetBumpFeeDiscount` with a negative vaule triggering an assertion `discount >= 0`.
**Fix**
This PR fixes it by ensuring not only that a discount exist but also that is greater the 0.
**Test**
It is hard to manually trigger this race condition. dergoegge coded a patch and test to trigger it that can be used to test the fix.
5320e2fd21
ACKs for top commit:
achow101:
ACK 3ae3a94f2b
pablomartin4btc:
ACK 3ae3a94f2b
Tree-SHA512: e76693eb66c4883ed3ef5edf1baa972657b0d532487803706312d486b4d4a4997f5dcbc64781a1a21b7b5628b06fcc97a6633382509061734d0f5615e851bef1
Remove the `Mutex` from the `coins_view` and `coinscache_sim`
pool startup helpers. Fuzz targets are entered sequentially within a
process and parallel fuzzing uses separate processes/forks, which each
have their own copy of the global thread pool. Therefore, a mutex to
prevent two in-process callers from racing to start the pool isn't needed.
fab8eeed82 fuzz: clang-format LIMITED_WHILE (MarcoFalke)
fa0d777ce2 fuzz: Clang-format LIMITED_WHILE like while (MarcoFalke)
fa1a9bde5a fuzz: Remove unused workaround after fix in libmultiprocess byte-span serializer (MarcoFalke)
fa55385ab3 fuzz: Use LIMITED_WHILE over for-loop with consumed size integral (MarcoFalke)
6d5f753921 Squashed 'src/ipc/libmultiprocess/' changes from 28e056576a..e8de5c7b68 (MarcoFalke)
Pull request description:
Includes several changes, to first update the subtree. Then, modify the fuzz test to address review comments:
* https://github.com/bitcoin/bitcoin/pull/35118#discussion_r3506566815
* https://github.com/bitcoin/bitcoin/pull/35118#discussion_r3523175145
ACKs for top commit:
ryanofsky:
Code review ACK fab8eeed82. Just fuzz test clang-format cleanups added since last review, which seem nice
Tree-SHA512: 0836628f8ee54adf02571025456211a74f63d05058b72280b10111ecfbb93d30945f4a48f30cc790774de4e2b489907313e86b0fa6729f3480c64850c94848b4
6667dc4eec kernel: expose scriptSig for btck_TransactionInput (Peter Zafonte)
e6de3a2d3c kernel: expose witness stack for btck_TransactionInput (Peter Zafonte)
Pull request description:
Silent payments scanning needs the public key from every input. For SegWit inputs it is in the witness stack. For P2PKH inputs it is in scriptSig. Without these new functions, callers must deserialize the raw transaction themselves to reach that data, which is difficult and error-prone.
Introduces a `btck_WitnessStack` type and adds the following functions:
**Witness stack:**
`btck_transaction_input_get_witness_stack`: returns a non-owning `const btck_WitnessStack* `view
`btck_witness_stack_count_items `: item count
`btck_witness_stack_get_item_at `: single item by index via btck_WriteBytes
`btck_witness_stack_copy` / `btck_witness_stack_destroy`: lifecycle for owned copies
**scriptSig:**
`btck_transaction_input_get_script_sig`: full scriptSig via btck_WriteBytes
All functions are exposed in the C++ wrapper via `WitnessStackView`, `WitnessStack`, and `WitnessStackApi` , and `GetScriptSig()`.
ACKs for top commit:
sedited:
ACK 6667dc4eec
musaHaruna:
ACK [6667dc](6667dc4eec)
stickies-v:
ACK 6667dc4eec
Tree-SHA512: b5e9d32ec87a5f5a9fea5652ed69737eae1d1f9cfb777544b96d703bbd057c114e0b7afae4e9e9d09b8b546694b14fb923b747c04a8f0211ab6caa015d06967d
This is a whitespace-only clang-format change.
To verify it, one can run:
```sh
(git show | git apply --reverse ) && ( git diff -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v ) && git diff HEAD
```
A few minor, non-macro formatting adjustments were made in touched files:
* `src/wallet/test/fuzz/crypter.cpp`: Removed a redundant double semicolon
* `src/test/fuzz/txorphan.cpp`: Corrected indentation on an `else if` block.
* `src/test/fuzz/mini_miner.cpp`: Removed an unnecessary empty line.
This is a style cleanup. The general pattern to use `LIMITED_WHILE`,
which all other fuzz tests use, has some benefits:
* When no data is available, a simple and single (let's say) 64 value in
the fuzz input will not result in 64 loops over the same body with the
same default/fallback values.
* When no data is available, `ConsumeBool` falls back to `false` and
breaks the loop early.
* When further data is available, the overhead is just a single byte,
making it also possibly easier for the fuzz engine to mutate the data,
as a single int that influences the whole remainder of the fuzz input
can lead to the 'havoc' effect.
* When a crash is reduced, deleting bytes will directly influence the
execution length, so byte-length of the fuzz input roughly corresponds
to run-time length.
e8de5c7b68 Merge bitcoin-core/libmultiprocess#305: refactor: memcpy to std::ranges::copy to work around ubsan warn
9307e68e5a Merge bitcoin-core/libmultiprocess#306: doc: Bump version 12 > 13
fac7b9b7f6 refactor: memcpy to std::ranges::copy to work around ubsan warn
1bd7025609 Merge bitcoin-core/libmultiprocess#297: test: add map serialization round-trip coverage
438fdd243d doc: Bump version 12 > 13
463d073cb8 test: rename vBool to vector_bool
85df233845 test: add mapStringInt to foo.capnp to cover map serialization and deserialization
git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: e8de5c7b68e0ae21c94ae92aa22e5c3b213f9c12
d164a04342 node: smooth oversized `-dbcache` warnings (Lőrinc)
Pull request description:
**Problem:** The oversized `-dbcache` warning threshold has a sharp formula cliff when detected RAM crosses the cutoff used by the warning logic.
This was reported during review of [#34641](https://github.com/bitcoin/bitcoin/pull/34641#discussion_r2900769756), where an earlier version could jump from the auto default at `4095 MiB` RAM to `75%` of RAM at `4096 MiB`.
That made `1 MiB` of extra detected RAM raise the warning threshold from about `511 MiB` to `3072 MiB`.
The surviving warning-only code has the same shape at a different boundary.
Below `2 GiB` RAM the cap is `DEFAULT_DB_CACHE` (`450 MiB`), but at `2 GiB` it switches to `75%` of total RAM, so a tiny increase in detected RAM can suddenly raise the warning threshold from `450 MiB` to about `1536 MiB`.
<img width="1484" height="881" alt="Image" src="https://github.com/user-attachments/assets/b51d5d24-31b8-4a2a-8f70-e7536481f855" />
**Fix:** Base the warning on a reserved non-dbcache memory budget instead:
```math
\text{warn if } \mathit{dbcache} > \max\left(\mathit{DEFAULT\_DB\_CACHE}, 0.75 \cdot \max(\text{total RAM} - \mathit{DBCACHE\_WARNING\_RESERVED\_RAM}, 0)\right)
```
`DBCACHE_WARNING_RESERVED_RAM` is `2 GiB`, so the fixed `DEFAULT_DB_CACHE` cap remains the floor below that reserve and the warning threshold grows monotonically above it.
This keeps the warning conservative around low-memory boundaries and avoids treating a boundary-crossing `1 MiB` RAM difference as a reason to allow a much larger explicit `-dbcache`.
**Quick reference:**
| System RAM | Previous warning cap | New warning cap |
| ---------- | -------------------- | --------------- |
| 1 GiB | 450 MiB | 450 MiB |
| 2 GiB | 1536 MiB | 450 MiB |
| 3 GiB | 2304 MiB | 768 MiB |
| 4 GiB | 3072 MiB | 1536 MiB |
| 8 GiB | 6144 MiB | 4608 MiB |
| 16 GiB | 12288 MiB | 10752 MiB |
| 32 GiB | 24576 MiB | 23040 MiB |
On 32-bit builds, effective `-dbcache` values are still capped to `1024 MiB` before the warning check, so thresholds above that cap are not reachable there.
ACKs for top commit:
optout21:
reACK d164a04342
sedited:
Re-ACK d164a04342
w0xlt:
reACK d164a04342
Tree-SHA512: deb81f0e192261f01dda6f1575a7b2e147f1e07e813d0833c7aeaf6a3fbd7594c0145ec1c98ded74efee9005108d229942949279c9458cbec4046913387db845
9e62e4b1f3 test: slow down rescaning process (Pol Espinasa)
336f5a738b wallet: reserve walletrescan before checking wallet is at the tip (Pol Espinasa)
Pull request description:
`ImportDescriptors` rpc has a race condition where two imports running in parallel can both succeed or fail one of them.
The race happens when there are two threads A and B trying to importdescriptors at the same time.
1. Thread A calls `BlockUntilSyncedToCurrentChain()` (holding `cs_wallet` fast, no contention) and then `reserve()`, acquiring the `WalletRescanReserver`. It proceeds to `ProcessDescriptorImport()`, which holds `cs_wallet` for an extended time (specially on slow machines) while importing descriptors.
2. B reaches `BlockUntilSyncedToCurrentChain()`, which internally does `WITH_LOCK(cs_wallet, ...)`. Since A holds `cs_wallet`, B blocks here for the entire duration of Thread A's descriptors import.
3. Then A finishes importing, releases `cs_wallet`, rescans (fast in regtest), and sets `fScanningWallet = false`.
4. B can now continue in `BlockUntilSyncedToCurrentChain()` acquiring `cs_wallet`, and then calls `reserve()` which succeeds because `fScanningWallet` is already `false`. Both imports succeed.
I don't think the behavior is problematic at all from a usability PoV, but it can be a bad UX if some imports fails and other's no. It also makes testing difficult as race conditions are not easy to test.
This PR fixes it by calling `reserver.reserve()` before `cs_wallet` is locked, so multiple threads will be aware of currently imports before being stuck at any point. So only one `importdescriptor` call can be done at the same time.
I think this should fix https://github.com/bitcoin/bitcoin/issues/35544#issuecomment-4763488259
ACKs for top commit:
achow101:
ACK 9e62e4b1f3
nebula-21:
ACK 9e62e4b1f3
w0xlt:
lgtm reACK 9e62e4b1f3
Tree-SHA512: be0027e1a7b77252ed9fb514c3b3311d6905903d4b0bfc1021a1e1c2bb06872ef599647ff8a4536929240717ae9db62fb75374f629cd3046c7192e2b8b4d7344
db35b9238f ipc # build: Fix fuzz target CMakeLists.txt for external libmultiprocess (Ryan Ofsky)
Pull request description:
CMake `WITH_EXTERNAL_LIBMULTIPROCESS` and `BUILD_FUZZ_BINARY` options stopped working together recently due to #35118 commit 037ad77071 because an non-namespaced `Libmultiprocess::multiprocess` target name was referenced.
Fix by specifying the full target name which is better for readability anyway.
ACKs for top commit:
sedited:
ACK db35b9238f
hebasto:
ACK db35b9238f, I have reviewed the code and it looks OK.
Tree-SHA512: 7cfb19a66dd4ccdcd6dfdedc3000fd20f488851f3235852717fd31698a7e8efb5c4e14705b0dc0c1ea506b38268f1ab030bca7865ddfdd85144598f435581247
fed3cf6f0e wallet: Replace CWalletTx's vOrderForm with specific fields (Ava Chow)
4f8823e8e1 wallet: Drop vOrderForm from CommitTransaction (Ava Chow)
a2b0bfcd85 wallet: Drop mapValue from CWalletTx (Ava Chow)
cb99864c91 wallet: Throw if unknown entry is found in mapValue (Ava Chow)
98d5cdae66 wallet: Make CWalletTx "replaces_txid" and "replaced_by_txid" member variables (Ava Chow)
7ef8a6efc2 wallet: Make CWalletTx "comment" and "to" member variables (Ava Chow)
2155e913d3 wallet: Make CWalletTx "from" and "message" member variables (Ava Chow)
c6ba98dcc8 wallet: Drop mapValue from CommitTransaction (Ava Chow)
00abb174a8 wallet: Pass comment and comment_to to CommitTransaction (Ava Chow)
1a219a37a2 wallet: Pass replaces_txid to CommitTransaction outside of mapValue (Ava Chow)
Pull request description:
`mapValue` and `vOrderForm` are opaque data structures that contain transaction metadata. It is hard to determine what actual data each field contains, and they can ostensibly be misused where metadata is added in the future without developers realizing that such metadata exists.
It's much clearer to have all of that metadata live in their own explicit member variables within `CWalletTx`. This PR implements that change.
Since the serialization format of `CWalletTx` depends on `mapValue` and `vOrderForm`, the serialization remains unchanged, so when serializing these new members, they need to be shoved/extracted from a temporary `mapValue` or `vOrderForm`.
This does end up breaking forwards compatibility as unknown fields in `mapValue` and `vOrderForm` are stripped out if the record is rewritten. However, I don't expect that we would continue to use these fields for future metadata, so I think that risk is low.
ACKs for top commit:
ajtowns:
reACK fed3cf6f0e
w0xlt:
ACK fed3cf6f0e with above nits/caveats.
Eunovo:
ACK fed3cf6f0e
Tree-SHA512: c7deab5aaeac13656012f8b13c0161fd420d2a5348eebd7649310e78ccb1216995aa6a7cbd506ac8d11d7b46b0856d6e6a897bc39965b51cfcf2268356ace261
a8223bb4e6 wallet: Introduce WalletError with machine-readable error code (pseudoramdom)
Pull request description:
Per discussion in https://github.com/bitcoin/bitcoin/pull/35436#issuecomment-4923786884, `WalletError` is split out so that it can be reused by multiple wallet interface changes (#34861 in particular)
----
Introduce a `wallet::WalletError`, a generic wallet-layer error type that contains
- a machine-readable `WalletErrorCode` for programmatic handling
- a translated user-facing `bilingual_str` message
The initial enum is intentionally small. `WALLET_ERROR` is used for generic failures that callers should display to the user. The intention is to have more specific codes only when the callers can handle the condition differently.
ACKs for top commit:
achow101:
ACK a8223bb4e6
davidgumberg:
ACK a8223bb4e6
polespinasa:
ACK a8223bb4e6
Tree-SHA512: 15fedf96cb5c3e8167a236bfa1a4d01d94f258a22943e53330ab432ad04a31f81f2eac8749f38390b2d34156d283d3bf76f293c640724bd0b090dc494123b230
2bab6bc73f refactor: Drop support for FreeBSD < 14 (Hennadii Stepanov)
91b5c8a07c refactor: Remove FreeBSD-specific workaround (Hennadii Stepanov)
56701ff6d5 doc: Clarify supported *BSD releases (Hennadii Stepanov)
Pull request description:
This PR establishes a baseline for the oldest *BSD releases supported by Bitcoin Core. Clarifying these minimum requirements paves the way for dropping compatibility code and workarounds for unsupported versions.
The obsolete FreeBSD-specific workaround and version check have been dropped.
ACKs for top commit:
maflcko:
lgtm ACK 2bab6bc73f
willcl-ark:
ACK 2bab6bc73f
theStack:
lgtm ACK 2bab6bc73f
sedited:
ACK 2bab6bc73f
Tree-SHA512: 6d9ca0ff881a60c33fe3aa18a03726426f07f2896b2f56b12804865acfa910aca7efdc1312eb4055e35aab8423d0c2326b89c1da448e01b4fa213f73dfd2b118
a2e4cd7ad2 depends: capnp 1.5.0 (fanquake)
Pull request description:
Update capnp in depends to [`1.5.0`](https://github.com/capnproto/capnproto/releases/tag/v1.5.0), which contains numerous security and bugfixes. More details here: https://github.com/capnproto/capnproto/blob/v2/security-advisories/2026-07-09-capnproto-v1.5-rollup.md:
> Like many projects, in recent months, Cap'n Proto has experienced an uptick in security reports, almost certainly driven by the use of AI to find vulnerabilities.
> In the past, I have always issued a separate security advisory with a CVE for each and every bug. However, this is time-consuming, and I just don't have the bandwidth to keep up. Therefore, given the quantity of bugs reported, I have opted to issue a single combined security advisory for this release.
>According to CVE rules, each distinct bug must have a separate CVE. Roll-ups are not allowed. Unfortunately, this means I cannot request a CVE for this advisory.
See https://github.com/capnproto/capnproto/compare/release-1.4.0...release-1.5.0 for all changes since 1.4.0.
ACKs for top commit:
hebasto:
ACK a2e4cd7ad2.
janb84:
ACK a2e4cd7ad2
Tree-SHA512: e616ab88a7692f17611865fbb36fc0c598ff6d8bcf18035f22d84681bbda8b6f5e7b8d7270baeeaf0d18e035de474003345f3bd75f020e615388ae04b3dd22f4
7508ac319d bench: replace CreateMockableWalletDatabase with MakeInMemoryWalletDatabase (Pablo Martin)
Pull request description:
Benchmarks don't need mock-specific behaviour (overridden `Filename()`, `Format()`, or the exposed batch-level `WriteKey()`). Replace `CreateMockableWalletDatabase()` with `MakeInMemoryWalletDatabase()` across all 6 call sites in `src/bench/` (5 files - 4 wallet bench files + 1 in coin_selection), using the same in-memory SQLite path that production code uses.
`wallet_migration.cpp` is excluded: it calls `GetOrCreateLegacyDataSPKM()` which asserts `Format() == "sqlite-mock"`, a deliberate signal that allows legacy SPKM setup in `test/bench` contexts. `MockableSQLiteDatabase` is still correct there.
For `coin_selection.cpp`, which had no other dependencies on `wallet/test/util.h`, the include is switched to `<wallet/sqlite.h>`. The remaining 4 files retain `wallet/test/util.h` for other utilities but also add an explicit `<wallet/sqlite.h>` include as required by IWYU.
Follow-up suggested in #35655.
ACKs for top commit:
sedited:
ACK 7508ac319d
janb84:
ACK 7508ac319d
Tree-SHA512: 6f4086eb5700ba3da882378dcee76b3c075670cdea5d71b45c9b369e48c72b8f591e0c5a6bd4504719dd512b3b7d2b23230a8b76e471c6c0571c13325fa3cba0
**What failed:** Build error: 'mp/proxy-io.h' file not found when
building the ipc fuzz target.
**Root cause:** Upstream commit 037ad77071 (fuzz: add IPC round-trip
target) added target_link_libraries(fuzz bitcoin_ipc_fuzz multiprocess)
using the bare 'multiprocess' target, which only exists in subtree
builds. With WITH_EXTERNAL_LIBMULTIPROCESS=ON, no 'multiprocess' cmake
target is defined, so the INTERFACE_INCLUDE_DIRECTORIES from the
installed libmultiprocess headers were never propagated to the fuzz
target, causing the missing-header error.
**Fix:** Changed to 'Libmultiprocess::multiprocess', which is the
canonical namespaced target available in both subtree and external builds.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
