756afe14b5 test: give each ValidateInputsStandardness case its own scope (JP)
5559fa464b test: fix wrong transaction in GetP2SHSigOpCount assertion (JP)
Pull request description:
While reading through `script_p2sh_tests.cpp` I noticed one of the assertions in `ValidateInputsStandardness` checks the wrong transaction.
The test builds `txToNonStd2_no_scriptSig` (which spends a P2SH prevout with an empty scriptSig) and checks its standardness result ("input 0 P2SH redeemscript missing"), but the `GetP2SHSigOpCount` assertion right after it re-checks the previous transaction: line 433 is byte-identical to line 419. Looks like a copy-paste slip from 248c175e3d, which added a `GetP2SHSigOpCount` check after each constructed transaction.
This PR points the assertion at `txToNonStd2_no_scriptSig` and expects 0 sigops. With an empty scriptSig there's no redeemScript push, so `GetSigOpCount(scriptSig)` ends up counting an empty subscript and returns 0. This case wasn't asserted anywhere before. The line above covers the other side, where the same prevout spent with the actual redeemScript counts 20.
To make sure the fix isn't vacuous I also ran the assertion expecting 20, and it fails with `[0 != 20]`.
Tested with:
```
cmake --build build --target test_bitcoin
build/bin/test_bitcoin --run_test=script_p2sh_tests
```
ACKs for top commit:
l0rinc:
ACK 756afe14b5
sedited:
ACK 756afe14b5
Tree-SHA512: 463eda7bb8790fb55619b36a6bedcd437f5c3d753e8c0abaa57dde3154421cde8c36f04293eb712ddf2745d525b5e254e8a1ab898f4f68538520dd80d873dba3
28641fd195 p2p: reject empty getblocktxn requests (furszy)
9871fb726c p2p: reject filtered block inv early when bloom is disabled (furszy)
aaf9412026 refactor: split p2p_getdata.py in sub-cases (furszy)
Pull request description:
Reject requests that make the node read blocks from disk unnecessarily:
* `getblocktxn` is meant to request the txs a peer is missing. When a
peer sends a `getblocktxn` with an empty index vector, it isn't missing
anything, so it shouldn't have sent the message in the first place.
* The peer should not request a filtered block when the node does not
advertise the `NODE_BLOOM` service. Filtered blocks are built from
the bloom filter, which can be loaded only when the `NODE_BLOOM`
service is offered.
Both are disconnected now.
Note: can be split in two PRs if preferred.
ACKs for top commit:
151henry151:
ACK 28641fd195
l0rinc:
lightly tested ACK 28641fd195
mzumsande:
Code Review ACK 28641fd195
winterrdog:
tested ACK 28641fd195
sedited:
ACK 28641fd195
Tree-SHA512: 787ee0741fb797ea0898daab1bf3d7b3a21d91c9d940fc833e893806e95843e9d5a7a79a55ba67ac0f1550c08f2b6cee5ab2b625c082f56885ae795958cc608c
fa06ea4244 ci: Temporarily remove riscv32 config from GHA matrix (MarcoFalke)
873550bea3 ci: verify cross-build SDK archives (Lőrinc)
2c87337efe ci: update NetBSD cross-build SDK (Lőrinc)
Pull request description:
Address a few CI failures in three commits:
* The NetBSD SDK release candidate was removed, so the CI consistently fails. Fix it by bumping to a release.
* Also, include SDK download hashes to catch stuff like rare bit flips due to cosmic rays.
* The riscv32 config consistently fails with download errors as well. Temporarily remove it until issue https://github.com/bitcoin/bitcoin/issues/35853 is fixed.
ACKs for top commit:
hebasto:
ACK fa06ea4244.
sedited:
ACK fa06ea4244
Tree-SHA512: 27c728f2a94386e4ef882757e1cc30ca8336c89775965d1a567dcac17681d9dd49547246f700969293ac8798e882489eb5c1b2b5fed4759c83d0f4463a592579
The macOS and BSD cross-build jobs extract SDK archives fetched at runtime without checking their content.
Keep each expected digest beside the corresponding SDK version and verify every archive before extraction.
Create the OpenBSD library symlinks once after both archives are extracted, when all link targets are present.
This is a follow-up to bitcoin/bitcoin#35221. The `MakeAndPushFeature`
member function template has no callers yet, which triggers
`-Wunused-template` now that Clang 23 enables it as part of `-Wall`.
6573196e63 doc: Release note for export watchonly wallet gui action (Ava Chow)
cb51f97f6c gui: Menu action for exporting a watchonly wallet (Ava Chow)
5907a5c7dc gui: Add ExceptionSafeConnect that takes a lambda (Ava Chow)
Pull request description:
Allows a user to export a watchonly version of their wallet to be used in an airgapped setup.
Built on https://github.com/bitcoin/bitcoin/pull/32489
ACKs for top commit:
polespinasa:
lgtm ACK 6573196e63
pablomartin4btc:
ACK 6573196e63
hebasto:
ACK 6573196e63.
Tree-SHA512: 30732ecf2ff40dbbd62a8a9974a907fd60f0da89afacce618fb706a02349135dc06d7dfcc11009caba0e020609ab7e586a0ebbeb7cd65940ee2df229d23f0605
Otherwise the build will fail with warnings about missing Python (removed
in the next commit). However the non-gui build does not need Python.
Note that #25573 contains the same change, because the use of -static-pie
causes warnings when combined with our/CMakes own PIE handling.
8221d714c7 lint: document CI lief version requirement (fanquake)
594a02c3ae lint: re-add guix scripts to mypy linting (fanquake)
Pull request description:
These were no-longer being linted after https://github.com/bitcoin/bitcoin/pull/32458.
suppress `[union-attr]` warning. i.e:
```bash
contrib/guix/symbol-check.py:309: error: Item "None" of "lief.PE.Binary | lief.ELF.Binary | lief.MachO.Binary | lief.COFF.Binary | None" has no attribute "format" [union-attr]
contrib/guix/security-check.py:284: error: Item "lief.COFF.Binary" of "lief.PE.Binary | lief.ELF.Binary | lief.MachO.Binary | lief.COFF.Binary | None" has no attribute "abstract" [union-attr]
```
Add the comment suggested in [#35855.](https://github.com/bitcoin/bitcoin/pull/35855#discussion_r3694954625).
ACKs for top commit:
maflcko:
lgtm ACK 8221d714c7 in any case.
hebasto:
ACK 8221d714c7, I have reviewed the code and it looks OK.
Tree-SHA512: ec404a8235fd40b212aad71ee3fe3473a3ce6f1ebaed46661d36ef02862b08528e3b6b829e05c5b943c8543380f3876e33da725154ce31d4022ad39ae5ef5ab3
Fix all IWYU warnings in `src/common` and treat them as errors in CI.
Additionally, ensure that our drop-in header replacements are used
instead of system headers:
- `<util/check.h>` instead of `<cassert>`
- `<util/time.h>` instead of `<chrono>`
fa895bb77a fuzz: Rework rpc fuzz target (MarcoFalke)
Pull request description:
The `rpc` fuzz target constructs a vector of string args and passes that to `RPCConvertValues`.
This has many issues:
* Each of those strings could represent an array itself. E.g. via `range argument` or via `ConsumeArrayRPCArgument`. However, those strings may not be converted to an array via `RPCConvertValues` and just be passed on as string argument. Having a call to `ConsumeArrayRPCArgument` that ends up with a plain json string is confusing.
* The strings could only represent an object or json null, when a raw string represented such a serialized json and was also converted to one via `RPCConvertValues`. Having a call to `ConsumeScalarRPCArgument` that was intended to give a raw string but ends up with a arbitrary json object is confusing.
Fix those "stringly-typed" issues by making the fuzz target "type safe":
* Rename `ConsumeScalarRPCArgument` to `ConsumeBasicRPCArgument` and return a proper `UniValue` from it.
* The "consume string" case inside that function, which had a "double meaning" is turned into two type-safe cases: One that returns a json string and one that reads an arbitrary json from a string.
* A new case for json null is added.
* `ConsumeRPCArgument` is changed to cover both json arrays and json dicts properly.
* Pass the resulting positional UniValue array directly to the RPC method, avoiding the need for `RPCConvertValues`.
Making the fuzz target "type safe" is also the first step in making it schema-aware.
ACKs for top commit:
dergoegge:
utACK fa895bb77a
Tree-SHA512: ee22310c981be802f4838454be0e7ef2be213704621c08ffe98dbeab2e3d7cc6ff6a37f7a129e1570185f13a740d34fd9e2c6de3a8281e8b8dc1277f673c4a48
dd2561003d fuzz: cover the mempool interface for transaction announcement (Antoine Poinsot)
Pull request description:
This adds coverage in the existing `tx_pool` harness for the `ExtractBestByMiningScoreWithTopology` method recently added in #34628.
ACKs for top commit:
dergoegge:
utACK dd2561003d
Tree-SHA512: 3e2ea6afff080f48f5b519c49cb6ef51a76eb20186bcfcb5d8cf194f67aa99bf5f5683271b58b97c9bc4e521ea3e08a46ee019e0ffc028222db9dbcb383c48fd
55d3cd51a4 doc: add release note describing change for forbidden clients (Matthew Zipkin)
d1ed2a6e25 http: check rpcallowip immediately after accepting connection (Matthew Zipkin)
Pull request description:
This is a follow-up to #35182 addressing a review comment from that PR: https://github.com/bitcoin/bitcoin/pull/35182#pullrequestreview-4322490068
This update to HTTPServer checks the IP subnet allowlist as soon as possible (immediately after receiving a connection from a client) before any data is received. This does not entirely protect the server from the "slow loris" attack or [CWE-400](https://cwe.mitre.org/data/definitions/400.html) but does restrict the attack surface to localhost and clients explicitly allowed by the user.
If a client is not allowed by the list, we disconnect as soon as possible. This is a behavior change from master branch (and previous release with libevent) where `403 Forbidden` was returned (after a potentially large amount request data was written to memory by the server).
To facilitate existing unit tests, this commit includes a refactor that moves the subnet allow list and relevant methods into the HTTPServer class instead of static file scope. This is needed because otherwise the allow list would be empty when the unit tests run.
There is still plenty of refactoring to do in order to modernize `HTTPServer` and de-globalize it, but since this specific issue has a resource allocation guard, I wanted to open it quickly on its own.
ACKs for top commit:
janb84:
ACK 55d3cd51a4
winterrdog:
ACK 55d3cd51a4
w0xlt:
ACK 55d3cd51a4
fjahr:
Code review ACK 55d3cd51a4
Tree-SHA512: 545911f2e4d2f97ab8bc854e9e57c39eb896428f8c349d34c8e8025a1f6bfb8cfd436f381e36af8b87592c07df3e16210819f3eef7943e23c6626030e615fdf5
A getblocktxn msg is only needed when at least one tx is
missing from a compact block. If no txs are missing, the
block can be reconstructed without sending the request.
This avoids reading the requested block from disk
unnecessarily and also alerts the peer operator about
their node's buggy behavior.
A peer should not request filtered blocks from a node that
does not advertise NODE_BLOOM. Perform this check before
looking up the block to avoid an unnecessary disk read.
Note: currently, the request is ignored only after the
block has been read from disk, in the bloom filter
existence check.
dff44e4c8f util: LineReader - Drop support for raw std::byte spans (Hodlinator)
5d5cdcd79d util: Make LineReader consistently use string_views (Hodlinator)
e8eaa80ce2 util: LineReader - Don't include newline and acknowledge single-char \r (Hodlinator)
Pull request description:
3 commits changing `LineReader`:
* Avoid the duplicate check for `\n` happening inside `RemoveSuffixView()`. https://github.com/bitcoin/bitcoin/pull/35182#discussion_r3333154138
* Use `string_view` internally rather than 2 `span::iterator`s.
* Stop accepting `span<byte>` inputs since internally and as outputs we treat them as strings.
Found while reviewing #35182.
ACKs for top commit:
achow101:
ACK dff44e4c8f
pinheadmz:
ACK dff44e4c8f
furszy:
ACK dff44e4c8f
Tree-SHA512: f4108cdc3895cce879eb21538ae6b11e7af3322abeb7026b766ca25419c53e691921ba94b1bc99dd34696b9bd3abcbde404ea62fc0f1e75d292f256d75fbf154
a99b27f192 validation: handle null mempool on delete (Lőrinc)
Pull request description:
**Problem:** Kernel creates a `ChainstateManager` without a mempool, while `AppInitMain()` supplies one for node applications.
If chainstate wiping is enabled and the data directory contains a saved AssumeUTXO snapshot, `LoadChainstate()` calls `DeleteChainstate()`, which dereferences the snapshot's null mempool pointer.
**Fix:** Accept a missing mempool when deleting the snapshot, matching the existing check in `AddChainstate()`.
<details>
<summary>Failure without the fix</summary>
```
unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_delete_chainstate_no_mempool": memory access violation at address: 0x48: invalid permissions
```
</details>
ACKs for top commit:
achow101:
ACK a99b27f192
sedited:
ACK a99b27f192
andrewtoth:
ACK a99b27f192
Tree-SHA512: 19579c03bdd525be29362db2d869e76d9ac8fd33b53527190942e2198a5b148453250ec536c57f95bf5d8ab414bb53ef114e62725ecad9d6ff6976cfa9cc5c51
90ce21e21d rpc: reject empty node argument in addnode (w0xlt)
69465de447 init: ignore empty addnode values (w0xlt)
Pull request description:
An empty `addnode=` entry currently creates an empty added-node record. The node then repeatedly attempts to connect to the empty destination:
```text
2026-07-23T19:22:33Z [net] trying v2 connection (manual) to , lastseen=0.0hrs
2026-07-23T19:23:33Z [net] trying v2 connection (manual) to , lastseen=0.0hrs
2026-07-23T19:24:34Z [net] trying v2 connection (manual) to , lastseen=0.0hrs
```
With this change, the node ignores empty `-addnode` values when initializing the connection manager. This preserves the existing startup behavior while avoiding useless connection attempts. Non-empty values are unaffected. Values consisting only of whitespace are ignored as well, and every ignored value is logged.
The `addnode` RPC has the same issue, so it now returns `Error: Node address cannot be empty` instead of adding such a record.
Functional tests verify that the node starts with `addnode=`, that no added-node record is created while non-empty values are still added, and that the RPC rejects empty values.
ACKs for top commit:
l0rinc:
lightly tested ACK 90ce21e21d
achow101:
ACK 90ce21e21d
pablomartin4btc:
ACK 90ce21e21d
furszy:
utACK 90ce21e21d
Tree-SHA512: e3074ff8a4e477f12c1f3331bc941854c2c994290afca85d4695a2ade79d0f230d167aaa9a141e619415164a853c54b7225477d0df8e5b96e0f1480223e188e5
278710a88d test: Add missing test case for getdata requests from blocks-only peers (Roqqit)
Pull request description:
ProcessGetData starts by eagerly processing getdata requests. In this loop, a special case checks for peers that have not requested transaction announcements (ie blocksonly) and ignores those requests. This test prevents regressions for that special case, which is currently not covered by existing tests.
ACKs for top commit:
maflcko:
lgtm ACK 278710a88d
achow101:
ACK 278710a88d
sedited:
ACK 278710a88d
nebula-21:
ACK 278710a88d
Tree-SHA512: 2f96efdd4d27e6f754dbdca74c9bf21214f77ee0a05f79ef9d6eb3d8166793346323d71be80bec7fb93bfcb8d764de24f31e5345951591b105557f30ddaabd0e
4cea59573c add release notes (Pol Espinasa)
492a715d78 gui: Adds option to not load the wallet after migration (Pol Espinasa)
Pull request description:
Following https://github.com/bitcoin/bitcoin/pull/35266 this PR adds the option to not load the wallet after migrating to the GUI.
It is only added for the `migrate` option, not for the `restore_and_migrate`. I guess if we are restoring the wallet we always want to load it.
In any case, it's pretty straightforward to implement it there too.
Inside the original migration pop-up box it appears a checkbox that allows the user to choose if want to load the wallet or not, it is checked by default:
<img width="497" height="388" alt="imagen" src="https://github.com/user-attachments/assets/79c76f9b-9b06-4fcb-88fe-3b5db3eaf14f" />
If yes, the wallet is loaded and shown, if not the wallet gets migrated and the GUI returns to it's state.
The checkbox has a tooltip that informs when not loading a migrated wallet can be useful:
<img width="503" height="494" alt="imagen" src="https://github.com/user-attachments/assets/3a3404ff-1fd3-44ef-8f9d-db526c9f032f" />
ACKs for top commit:
achow101:
ACK 4cea59573c
pablomartin4btc:
ACK 4cea59573c
hebasto:
ACK 4cea59573c, I have reviewed the code and it looks OK.
Tree-SHA512: 6256849ecca3888fe24866ed539b262be6275c6192d29d272e7ad1475943c2947c04a3fadaae94e1843b3fcfa43a1f84edb02365933bf954b8ba7d0ae53eda32
aa01721c89 test: add interface_gui.py to test bitcoin-gui startup via RPC (Ryan Ofsky)
Pull request description:
Adds a functional test that starts bitcoin-qt using QT_QPA_PLATFORM=minimal for headless operation, then verifies it responds to a stop RPC call. This detects startup crashes in the GUI that have no CI coverage today like https://github.com/bitcoin-core/gui/issues/940
The new test is currently skipped on macos and windows due to different problems on those platforms that may be resolved with future PRs. Fixing the windows issue should also allow the `tool_bitcoin.py` test to be enabled on windows, and fixing the macos issue should allow Qt addressbook and wallet tests to be enabled on macos.
ACKs for top commit:
achow101:
ACK aa01721c89
sedited:
ACK aa01721c89
pablomartin4btc:
ACK aa01721c89
hebasto:
ACK aa01721c89.
Tree-SHA512: 84873aed41a856322eca1c391d3ff19b6eb4a0aa253d09ace342e3efe970a330cb11506eb4efe2580b7991132b644c7408feb013c18dbfbfaaf879f88f12e02e
683ae4c520 guix: consolidate config flags (fanquake)
665f11d04a guix: consolidate gcc toolchain setup (fanquake)
288f76ed0f guix: consolidate mingw-w64 toolchain setup (fanquake)
cc9b0f2266 guix: consolidate LLVM toolchain setup (fanquake)
b12a70f330 guix: turn linux/win linker warnings into errors (fanquake)
Pull request description:
This deduplicates setup code, as well as adds flags to turn linker warnings into errors, which is easier now that the GUI build has been split out (the gui link warns about shared libs during linking).
ACKs for top commit:
hebasto:
ACK 683ae4c520, I have reviewed the code and it looks OK.
Tree-SHA512: 08b4fa14494481149844750bd6741c61b3a9367eed46923e8c788f3cd22ea9c4b319326075df0114dbb17e11f0dd1340999ba80e40a244208e6b4e90c86e3361
Can do this now that the GUI has been split out.
riscv64-linux-gnu failus due to
https://github.com/boostorg/test/issues/345:
```bash
[102%] Linking CXX executable ../../bin/test_bitcoin
/gnu/store/r03804zpq5i6wsalx0yaqrr5jb7pqrmv-binutils-cross-riscv64-linux-gnu-2.46.0/bin/riscv64-linux-gnu-ld: CMakeFiles/test_bitcoin.dir/main.cpp.o: in function `boost::fpe::disable(unsigned int)':
/bitcoin/depends/riscv64-linux-gnu/boost/include/boost/test/impl/execution_monitor.ipp:1538:(.text+0x9dc8): warning: fedisableexcept is not implemented and will always fail
/gnu/store/r03804zpq5i6wsalx0yaqrr5jb7pqrmv-binutils-cross-riscv64-linux-gnu-2.46.0/bin/riscv64-linux-gnu-ld: CMakeFiles/test_bitcoin.dir/main.cpp.o: in function `boost::fpe::enable(unsigned int)':
/bitcoin/depends/riscv64-linux-gnu/boost/include/boost/test/impl/execution_monitor.ipp:1502:(.text+0x9d76): warning: feenableexcept is not implemented and will always fail
collect2: error: ld returned 1 exit status
```
Darwin could be done after something like
https://github.com/bitcoin/bitcoin/pull/35756.
c9a70f9338 script: qa: Improve Key::Fingerprint type safety (David Gumberg)
Pull request description:
Extracted from pseudoramdom's work in #35436:
Instead of using c style arrays for key fingerprints, use `std::array`'s whose length can always reasoned about at compile time and for most operations the compiler enforces the size being correct.
```cpp
using KeyFingerprint = std::array<unsigned char, 4>;
```
```diff
- unsigned char vchFingerprint[4];
+ KeyFingerprint fingerprint;
```
This allows the replacement of a lot of raw `memcpy` + trust-me-bro lengths, with the assignment operator:
```cpp
- memcpy(ret.vchFingerprint, vchFingerprint, 4);
+ ret.fingerprint = fingerprint;
```
This commit also adds two helper functions for
- Retrieving the [fingerprint of a key identifier](https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#user-content-Key_identifiers) (`CKeyID`)
- Retrieving the fingerprint of the key identifier of an XPUB.
ACKs for top commit:
w0xlt:
ACK c9a70f9338
sedited:
ACK c9a70f9338
pseudoramdom:
Code review ACK w/ some minor nits c9a70f9338
polespinasa:
ACK c9a70f9338
Tree-SHA512: 3ee76742c0bc317dfbc12a6731afdcc40495db6e4d5d94880d0a721990d36cb3e4d374ccc96079ba1f8ad3f88581ee5a609bfe259c0ea7cd28cade373aac1b38
d3d74e701f ipc, refactor: Update mp::g_thread_context references (Ryan Ofsky)
2d3f72fd3f ipc, refactor: Update mp::SpawnProcess call (Ryan Ofsky)
e9f19815ca ipc, refactor: Add Stream type alias and use it (Ryan Ofsky)
3859805f05 ipc, refactor: Add SocketId type alias and use it (Ryan Ofsky)
2ee9b69c7a ipc, refactor: Add ProcessId type alias and use it (Ryan Ofsky)
3449797141 ipc: Avoid 'unistd.h' error with MSVC (Ryan Ofsky)
dbcc192dce ipc, refactor: fix include order (Ryan Ofsky)
7c86d4834e ipc, refactor: use native path separators in test (Ryan Ofsky)
00287b9a34 ipc, refactor: Change Protocol class field order (Ryan Ofsky)
33d37f3c35 ipc, refactor: Drop connect/listen/serve exe_name parameters (Ryan Ofsky)
794940469e ipc, moveonly: combine ipc_test.cpp and ipc_tests.cpp (Ryan Ofsky)
Pull request description:
This PR makes Bitcoin Core changes needed to be compatible with https://github.com/bitcoin-core/libmultiprocess/pull/274, which changes the libmultiprocess API to stop using unix-specific types so it is compatible with windows. (Windows support is added in followups: https://github.com/bitcoin-core/libmultiprocess/pull/231 and https://github.com/bitcoin/bitcoin/pull/32387.)
The PR uses some [compatibility shims](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-wins/src/ipc/util.h) so it can be reviewed and merged without needing to merge https://github.com/bitcoin-core/libmultiprocess/pull/274 first and bump the libmultiprocess subtree. These can be deleted when the subtree is updated.
---
Review note: All the changes here are refactoring, and you don't really need to know anything about IPC or Windows to review this code. It is also a mostly move-only change (131 lines added, 96 removed, 215 moved)
ACKs for top commit:
xyzconstant:
tACK d3d74e701f
enirox001:
ACK d3d74e701f
Sjors:
ACK d3d74e701f
ViniciusCestarii:
re-ACK d3d74e701f tested locally on Linux
Tree-SHA512: cd48708f9fd086ac8127dc75cfaf4bd8f8da81e07d11b2c9e65fd9061ffa33478bffc6fd6fa4b3505e86c6437752578fe6e5bd590c683c3bc9969093103a5608
