c74c6cfd84 ci: Match `VCPKG_HOST_TRIPLET` to `VCPKG_TARGET_TRIPLET` (Hennadii Stepanov)
Pull request description:
Using a non-default target triplet in https://github.com/bitcoin/bitcoin/pull/34883 introduced a [regression](https://github.com/bitcoin/bitcoin/pull/34883#issuecomment-4204744932) because packages with `"host": true` in their vcpkg configurations were still picking up the default `x64-windows` triplet, effectively building both release and debug packages.
This PR fixes this regression by setting `VCPKG_HOST_TRIPLET` explicitly.
Upstream issue: https://github.com/microsoft/vcpkg/issues/50927.
ACKs for top commit:
hodlinator:
utACK c74c6cfd84
Tree-SHA512: ee17d800a2db16eccfd2751da296c0c30d4d41f60e94513098be96116aedd83efe258349be824ff1f571c11adffac6a72e4a0c9b8b7278ab736d453ff7c089d6
1401011f71 test: Add test for exceeding max line length in torcontrol (Fabian Jahr)
84c1f32071 test: Add torcontrol coverage for PoW defense enablement (Fabian Jahr)
7dff9ec298 test: Add test for partial message handling in torcontrol (Fabian Jahr)
569383356e test: Add simple functional test for torcontrol (Fabian Jahr)
4117b92e67 fuzz: Improve torcontrol fuzz test (Fabian Jahr)
b1869e9a2d torcontrol: Move tor controller into node context (Fabian Jahr)
eae193e750 torcontrol: Remove libevent usage (Fabian Jahr)
8444efbd4a refactor: Get rid of unnecessary newlines in logs (Fabian Jahr)
6bcb60354e refactor: Modernize member variable names in torcontrol (Fabian Jahr)
a36591d194 refactor: Use constexpr in torcontrol where possible (Fabian Jahr)
Pull request description:
This is part of the effort to remove the libevent dependency from our code base: https://github.com/bitcoin/bitcoin/issues/31194
The current approach tries to reuse existing code and follows roughly similar design decisions. It replaces the libevent-based async I/O with blocking I/O utilizing the existing `Sock` and `CThreadInterrupt`. The controller runs in a dedicated thread.
There are some optional code modernizations thrown in made along the way (namings, constexpr etc.). These are not strictly necessary but make the end result with the new code more consistent.
ACKs for top commit:
achow101:
ACK 1401011f71
janb84:
re ACK 1401011f71
pinheadmz:
ACK 1401011f71
Tree-SHA512: 167f1d98a634524568cb1d723e7bdb7234bade2c5686586caf2accea58c3308f83a32e0705edc570d6db691ae578a91e474ae4773f126ec2e1619d3adf7df622
Using a non-default target triplet introduced a regression because
packages with `"host": true` in their vcpkg configurations were still
picking up the default `x64-windows` triplet, effectively building both
release and debug packages.
Fix this regression by setting `VCPKG_HOST_TRIPLET` explicitly.
b555a0b789 test: remove macOS REDUCE_EXPORTS exception workaround (Alfonso Roman Zubeldia)
Pull request description:
Fixes#34723.
The underlying fix was merged in #34977 (subtree update from bitcoin-core/libmultiprocess#268). This PR
removes the test workaround that accepted the degraded unknown non-KJ exception of type: `kj::Exception`
error message on Darwin.
Tested: `interface_ipc_mining.py` passes on macOS (arm64) with REDUCE_EXPORTS=ON.
ACKs for top commit:
achow101:
ACK b555a0b789
ryanofsky:
Code review ACK b555a0b789. Thanks for the fixes!
w0xlt:
ACK b555a0b789
Tree-SHA512: fa5a316db2ed9ce67dabbd7c529818664dfca01faeb7aff9c5cbb9a1b876de3f498f675eae4bdb74de71e73b7b8cf8cc85c44f520e59f9e3e7a6d6c442ea8273
d517fa0a94 rpc: fix initialization-order-fiasco by lazy-init of decodepsbt_inputs (Eugene Siegel)
Pull request description:
Prior to this commit, `decodepsbt_inputs` would call `TxDoc` during initialization which lives in another TLU. `TxDoc` relies on `CURRENCY_UNIT` to be initialized when it may not have been (note this is different from the TLU containing `decodepsbt_inputs` which also has a `CURRENCY_UNIT`). Fix this by lazy initializing `decodepsbt_inputs`. Also prevent the issue in the future by doing the same for `decodepsbt_outputs` and `getblock_vin`.
Curious why the CI missed this, it broke fuzzamoto. It was introduced in fadf901fd4. I was able to trigger this with clang-21 and `ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1"`:
```
=================================================================
==926804==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x5631e5784288 at pc 0x5631e3309f9c bp 0x7ffdb6abc7b0 sp 0x7ffdb6abc7a8
READ of size 8 at 0x5631e5784288 thread T0
#0 0x5631e3309f9b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::size() const /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:1064:16
#1 0x5631e3309f9b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> std::operator+<char, std::char_traits<char>, std::allocator<char>>(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.tcc:618:35
#2 0x5631e4238948 in TxDoc(TxDocOptions const&) /root/bitcoin/src/rpc/rawtransaction_util.cpp:382:76
#3 0x5631e316945e in __cxx_global_var_init.9 /root/bitcoin/src/rpc/rawtransaction.cpp:784:17
#4 0x5631e3186355 in _GLOBAL__sub_I_rawtransaction.cpp /root/bitcoin/src/rpc/rawtransaction.cpp
#5 0x7fa6ca046375 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27375) (BuildId: 79005c16293efa45b441fed45f4f29b138557e9e)
#6 0x5631e31cf160 in _start (/root/bitcoin/build/bin/bitcoind+0x25b160)
0x5631e5784288 is located 56 bytes before global variable 'CURRENCY_ATOM[abi:cxx11]' defined in '/root/bitcoin/src/policy/feerate.h:20' (0x5631e57842c0) of size 32
registered at:
#0 0x5631e31e5738 in __asan_register_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:431:3
#1 0x5631e31e68a9 in __asan_register_elf_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:414:3
0x5631e5784288 is located 8 bytes inside of global variable 'CURRENCY_UNIT[abi:cxx11]' defined in '/root/bitcoin/src/policy/feerate.h:19' (0x5631e5784280) of size 32
registered at:
#0 0x5631e31e5738 in __asan_register_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:431:3
#1 0x5631e31e68a9 in __asan_register_elf_globals /root/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:414:3
SUMMARY: AddressSanitizer: initialization-order-fiasco /root/bitcoin/src/rpc/rawtransaction_util.cpp:382:76 in TxDoc(TxDocOptions const&)
Shadow bytes around the buggy address:
0x5631e5784000: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784080: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784100: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784180: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784200: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
=>0x5631e5784280: f6[f6]f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784300: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784380: f6 f6 f6 f6 01 f9 f9 f9 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784400: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784480: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
0x5631e5784500: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==926804==ABORTING
```
Can be reviewed with `git diff HEAD~1 -w` since it's mostly indentation.
ACKs for top commit:
davidgumberg:
crACK d517fa0a94
nervana21:
tACK d517fa0a94
dergoegge:
tACK d517fa0a94
Tree-SHA512: cfcaf4baccbb5748ab10ba5537cfacf38d7137b7ff051ccb9aba0f673718be1532487586b4f4e6d2f4526ca1786733009bd73a870d79a966b9c1fb38f7aaf5cc
91cd0e3aaa fuzz: remove GetDescriptorChecksum from string harness (Bruno Garcia)
Pull request description:
This function is already strongly fuzzed by other harness. E.g: descriptor_parse calls it several times during parsing and serialization. Also, calling GetDescriptorChecksum with a string of length 32 is not effective to exercise it.
ACKs for top commit:
frankomosh:
utACK 91cd0e3aaa
sipa:
ACK 91cd0e3aaa. I don't think this tests anything useful.
Tree-SHA512: ac5e5c3f04669d4f7c24c25390565b02350ac2d4b1044aa1b3b9b83b3a70e57b3a109bd8d58298e722ce97d4a76efc62f11b31147a810a65db7ac592606acc3c
3edbe8f67c Merge bitcoin-core/libmultiprocess#268: Use throwRecoverableException instead of raw throw for stored exceptions
23be44b0d3 Use throwRecoverableException instead of raw throw for stored exceptions
75c2a2764c Merge bitcoin-core/libmultiprocess#266: test: increase spawn test child timeout to 30 seconds
8b5f805301 Merge bitcoin-core/libmultiprocess#267: doc: Bump version 9 > 10
cc0b23fc32 test: increase spawn test child timeout to 30 seconds
050f878db8 doc: Improve versions.md descriptions and formatting
c6a288a889 doc: Bump version 9 > 10
git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 3edbe8f67c182dde91c0050065d79ae268722489
39668f1eeb contrib: Add bash completion for new bitcoin command (CaesarCoder)
Pull request description:
Adds a bash completion script for the new bitcoin command-line tool (introduced in #31375), which unifies the main Bitcoin Core executables under a single interface. This feature improves usability, reduces errors, and makes the command-line tools more easily discoverable for users working in a Linux bash environment.
The completion script dynamically lists available commands and options by parsing `bitcoin --help` and `bitcoin help`. It also incorporates the existing bash completions for `bitcoind`, `bitcoin-cli`, and `bitcoin-tx`, depending on the argument provided (`node`, `gui`, `rpc`, or `tx`). This ensures that all relevant completions are available seamlessly through a single interface without modifying core functionality.
**Key points:**
- Improves developer and user experience on the command line.
- Does not modify or replace existing functionality.
- Placed under `contrib/` for easy installation by users and distributions.
ACKs for top commit:
willcl-ark:
ACK 39668f1eeb
BrandonOdiwuor:
Tested ACK 39668f1eeb
Tree-SHA512: a84cd53adefbdf69ec510584d6117b70e1b6131686502170c53b6a3a23bc5431244a47e72a323111c5a07aa75d267aaeaa52bab0837901431db1c8b1d266723e
851152e42a validation: Remove stale BlockManager param in ContextualCheckBlockHeader (ismaelsadeeq)
Pull request description:
The `BlockManager` reference in `ContextualCheckBlockHeader` is unused.
Fix this by removing it from the function parameter list.
Remove `BlockManager` reference argument in `ContextualCheckBlockHeader` invocations.
ACKs for top commit:
maflcko:
lgtm ACK 851152e42a
stratospher:
ACK 851152e. not used anymore since checkpoints were removed in 3c5d1a4.
Tree-SHA512: 4d4c32cfc8c280774271ab1b5daff32b330030c3c4f275713836d7d2030c1842cab8a6f7758318fbe760c9158938a49d772246e1e328302ad2f8505e44e55695
8825051e08 refactor: improve benchmark setup and execution for various tests (Lőrinc)
83b8528ddb bench: add fluent API for untimed setup steps in `nanobench` (Lőrinc)
Pull request description:
### Context
As described in https://github.com/martinus/nanobench/issues/130, we have a few benchmarks where we have to reset the state between runs; otherwise, the repetitions will do something different than the first iteration.
### Upstream
I have opened a PR to `nanobench` to introduce an untimed setup phase, see: https://github.com/martinus/nanobench/pull/136
### Tests
Tests were only added upstream. It would be a bit awkward to wire them into `nanobench.h` outside the benchmarking setup:
58350cfe59 (diff-88160f647ce57661afe7d755fa70a5fa342a2b79d72d3511596878e69ed5cdc3)
### Fix
I have moved the changes here as well and applied them to a few simple benchmarks as a demonstration.
We can revert the ones that are controversial and add others in follow-ups. This PR is mostly meant to add the `setup` feature.
### Benchmarks
Most benchmarks show a modest "speedup"; others a "slowdown" - but it's only the effect of the setup that's not measured anymore - and a `run` phase that does the same operation in each epoch iteration (wallet benchmark changes were reverted for simplicity):
<img width="1496" height="882" alt="image" src="https://github.com/user-attachments/assets/34c14565-f3df-41e5-9a86-95b2ca21703a" />
ACKs for top commit:
achow101:
ACK 8825051e08
janb84:
re ACK 8825051e08
sedited:
ACK 8825051e08
Tree-SHA512: b3e385abcfca013a21b3785b0b837c2b61e302d71a098dadcd8d2f0cb42f6bbf4a222299771443f095962d1b24e696d5684f2b8efdb6f63f2f939699961cdf0d
fc9987dfc6 doc: remove stale shortid collision TODO (w0xlt)
Pull request description:
`PartiallyDownloadedBlock::InitData()` already treats duplicate short IDs in a compact block as `READ_STATUS_FAILED`.
For an honest peer, block-level short-ID collisions are rare enough that failing the compact block early is preferable to scanning the mempool or attempting selective recovery (see https://github.com/bitcoin/bitcoin/pull/34932#issuecomment-4139177024)
This removes the old TODO suggesting that both collided transactions should be requested, since that comment no longer matches the intended handling.
No behavior change. Comment-only cleanup.
ACKs for top commit:
l0rinc:
ACK fc9987dfc6
instagibbs:
ACK fc9987dfc6
Tree-SHA512: 46d8a64076e29b6dd6b6ccbfc74101da00dea0c7e88a94b10136585b8df82bb328a4922aa9ac1cd0dfc7f9d491b3fff59492e2d445d91e48668fe0c375a70a85
facaeb9c76 doc: Discourage trailing doxygen comments, and fix the broken ones (MarcoFalke)
Pull request description:
The trailing doxygen comments are problematic:
* They are often wrong and need to be fixed up manually, see e.g https://github.com/bitcoin/bitcoin/pull/7793, https://github.com/bitcoin/bitcoin/pull/14331, ..., and this pull
* Also, changing any type or name in a way that changes the max line length may require reflowing the whole struct with clang-format.
* Also, once they become multi-line, they will read even more confusingly.
Fix all issues by tolerating them, but favoring descriptions before the described item.
Also, fix the broken ones.
Example doxygen rendering: https://doxygen.bitcoincore.org/struct_private_broadcast_1_1_send_status.html (broken before this pull, fixed in this pull)
ACKs for top commit:
l0rinc:
ACK facaeb9c76
stickies-v:
ACK facaeb9c76
Tree-SHA512: db725140bd907ac61f2fe58681d4a23f392ef712ef8772564f3a3d83ed446e068bb390907ee30c2015d8b396a42a31817f56b8dcb2d86d1314aed9208a86e2bd
325afe664d net: delay stale evaluation and expose time_added in private broadcast (Mccalabrese)
999d18ab1c net: introduce TxSendStatus internal state container (Mccalabrese)
Pull request description:
**Motivation**
Currently, freshly added transactions in `private_broadcast` are almost immediately flagged and logged as stale by the `resend-stale` job.
**The Bug**
`m_transactions` maps a transaction to a `std::vector<SendStatus>`. When `try_emplace` adds a new transaction, this vector is empty. When `GetStale()` runs, `DerivePriority()` evaluates the empty vector and returns a default `Priority` struct where `last_confirmed` evaluates to the Unix Epoch (Jan 1, 1970). The stale checker sees a 50-year-old timestamp and flags it on the next resend-stale cycle.
**The Fix**
Rather than modifying the transient `Priority` struct or creating a "Zombie Transaction" edge case by ignoring transactions with 0 picks, this PR modifies the state container:
* Wraps the `SendStatus` vector in a new `TxSendStatus` struct inside `private_broadcast.h`.
* `TxSendStatus` automatically captures `time_added` upon emplace.
* `GetStale()` now checks `p.num_confirmed == 0` to measure age against `time_added` using a new 5-minute `INITIAL_STALE_DURATION` grace period, falling back to `last_confirmed` and the standard 1-minute `STALE_DURATION` once network interaction begins.
**Additional Polish**
* Exposed `time_added` via the `getprivatebroadcastinfo` RPC endpoint so users can see when a transaction entered the queue.
* Added a dedicated `stale_unpicked_tx` test case and updated `private_broadcast_tests.cpp` to properly mock the passage of time for the new grace period.
Closes#34862
ACKs for top commit:
achow101:
ACK 325afe664d
andrewtoth:
ACK 325afe664d
vasild:
ACK 325afe664d
Tree-SHA512: b7790aa5468f7c161ed93e99e9a6d8b4db39ff7d6d6a920764afd18825e08d83bc30b3fb0debeb6175730b5d2496c6be67f3be8674be93f4d07b1e77d17b4a14
Gets rid of the Dummy class and adds coverage of get_socks_cb.
Also explicitly handles an exception case within Torcontrol rather than
relying on a guard in the fuzz test.
Prior to this commit, decodepsbt_inputs would call TxDoc during initialization
which lives in another TLU. TxDoc relies on CURRENCY_UNIT to be initialized when
it may not have been (note this is different from the TLU containing
decodepsbt_inputs which also has a CURRENCY_UNIT). Fix this by lazy initializing
decodepsbt_inputs.
Prevent the issue from occurring in the future by also doing the same for
decodepsbt_outputs and getblock_vin.
0587c56091 kernel: Expose context-free block validation (w0xlt)
71f827c3c2 kernel: Expose consensus parameters (`btck_ConsensusParams`) (w0xlt)
Pull request description:
This PR exposes Bitcoin Core’s context‑free block checks to library users via a new C API entry point, `btck_check_block_context_free`.
Callers can validate a block’s structure (size/weight, coinbase rules, per‑tx context‑free checks) and optionally re‑run Proof‑of‑Work and Merkle‑root verification without touching chainstate, the block index, or the UTXO set.
Rationale
Clients embedding the kernel need a pure block sanity check without requiring node state or disk writes (candidate block validation, for example). This API offers that surface in a single call, with optional PoW/Merkle toggles to avoid redundant work when the header has already been validated or when Merkle verification is deferred.
ACKs for top commit:
yuvicc:
re-ACK 0587c56091
achow101:
ACK 0587c56091
sedited:
ACK 0587c56091
Tree-SHA512: 6bd53e4964909335d1f2fee30ff96c95a8dd2c84bcdfe11c50ba369301822e5dea9bbe2376bb6d6b4652875152071eb0446657042b00429f29581da4fcea71a9
43c528aba9 wallet, test: update `gethdkeys` functional test (rkrux)
6e3a0afc2f wallet: fix `gethdkeys` RPC for descriptors with partial xprvs (rkrux)
Pull request description:
Fixes#34378
A non-watch-only wallet allows to import descriptors with partial private keys, eg: a multisig descriptor with one private key and one public key. In case an xpub is imported in any such descriptors whose private key the wallet doesn't have, then the `gethdkeys` RPC throws an unhandled error like below when the private keys are requested.
This fix ensures that such calls are properly handled by conditionally finding the corresponding xprv and the related functional test is accordingly updated.
```
➜ bitcoincli -named gethdkeys private=true
error code: -1
error message:
map::at: key not found
```
ACKs for top commit:
achow101:
ACK 43c528aba9
w0xlt:
ACK 43c528aba9
Eunovo:
Tested ACK 43c528aba9
Tree-SHA512: 106e02ee368a3fa94d116f54f2fa71f9886e4753e331b91ce13a346559d5497ef6cd7ddaa8736fcfe1a7ac427a44d647fb75e523eb72f917fa866adbe0dbad30
fa955af618 lint: Clarify rmtree/remove_all error message with preferred alternatives (MarcoFalke)
Pull request description:
An error message without a suggestion to fix will often lead developers to come up with a creative solution to side-step the grep in the linter (such as using an `auto` alias or other style-change).
Try to avoid this by mentioning a recommended solution.
ACKs for top commit:
davidgumberg:
ACK fa955af618
achow101:
ACK fa955af618
hodlinator:
ACK fa955af618
Tree-SHA512: d5741b2fdb1cfcffc26144c3bb78478e46d7ecb5c3340cbb09584fe1274a3cc8c6872b997e33b8bd2097630d916133a3f5bf41a1d09527e597be9829d6d3e51b
PartiallyDownloadedBlock::InitData() intentionally treats duplicate short IDs in a compact block as READ_STATUS_FAILED.
For an honest peer, block-level short-ID collisions are rare enough that failing the compact block early is preferable to spending CPU scanning the mempool or attempting selective recovery.
Remove the old TODO that suggested requesting both collided transactions, since it since it points at unwanted behavior.
75608547b4 kernel: Remove NONNULL annotation from destroy method (Alexander Wiederin)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/33796#discussion_r2959817508
### Summary
This PR removes the `BITCOINKERNEL_ARG_NONNULL` annotation from the `btck_block_validation_state_destroy` method in the Kernel API.
### Motivation
No other *_destroy function in the Kernel API carries the NONNULL annotation. Following the convention set by free(), destroy functions should accept null pointers.
### Usage:
Before:
```c
btck_BlockValidationState* state = NULL;
btck_block_validation_state_destroy(state); // violates nonnull contract
```
After:
```c
btck_BlockValidationState* state = NULL;
btck_block_validation_state_destroy(state); // well-defined
```
ACKs for top commit:
yuvicc:
lgtm! ACK 75608547b4
kevkevinpal:
ACK [7560854](75608547b4)
achow101:
ACK 75608547b4
janb84:
ACK 75608547b4
theStack:
ACK 75608547b4
w0xlt:
ACK 75608547b4
stickies-v:
ACK 75608547b4
Tree-SHA512: 9577098940f49b8f7fd1fb20afe750e9eaaf9d90d25ec3c4f423de223a1fbd0bca6d520061f576eb3923f18e1f2744b3f60d0d0715dfa3832fea83cb538170c2
9316d96240 test: sock: Enable socket pair tests on Windows (David Gumberg)
Pull request description:
Some `class Sock` tests were previously disabled because Windows lacks [`socketpair(2)`](https://man7.org/linux/man-pages/man2/socketpair.2.html) which is used as a helper function in the socket tests, but is not strictly necessary or related to the `Socket` class under testing. This PR adds a `CreateSocketPair()` helper which creates a sender socket and receiver socket with a TCP connection, enabling these test cases for Windows. This also enables future tests that require more granular control over sockets than what `socketpair()` allows for, like using `setsockopt()` before connecting a socket.
This change is generally an improvement, but is also broken out of a [branch](github.com/davidgumberg/bitcoin/tree/2025-09-02-0xB10C-prefill-rebase) that does compact block prefilling up to the available bytes in the connection's current TCP window (see [delving post](https://delvingbitcoin.org/t/stats-on-compact-block-reconstructions/1052/34)). Creating connected socket pairs is useful for added tests in that branch that validate querying the current TCP window state, and without this change those tests don't run on Windows.
ACKs for top commit:
achow101:
ACK 9316d96240
sedited:
ACK 9316d96240
w0xlt:
reACK 9316d96240
Tree-SHA512: 23ad7070555bb934b0eb18d114e918bd2d50657d20d2221d8c98cd0e558e27e32e5ef89e8074c108a90d7309e539318d23cea43d2ad8eb10e635b114f5f1a87d
This function is already strongly fuzzed by other harness.
E.g: descriptor_parse calls it several times during parsing
and serialization. Also, calling GetDescriptorChecksum with
a string of length 32 is not effective to exercise it.
No other *_destroy function in the Kernel API carries this annotation.
Following the convention set by free(), destroy functions should accept
null pointers.
325f743eed guix: Clean up module list in manifest (Hennadii Stepanov)
Pull request description:
These modules are no longer in use:
1. `cmake-build-system` since bitcoin/bitcoin#34550.
2. `gnu-build-system` since bitcoin/bitcoin#34627.
3. `download` since bitcoin/bitcoin#34550.
4. `cc-for-target` from `utils` since bitcoin/bitcoin#34627.
ACKs for top commit:
janb84:
ACK 325f743eed
Tree-SHA512: 5f32a74cc5c13cd4cd181f6d23fe5198e71a0d8f18378f4a2e38aa7a06eedd6f2c5cfbeea9b8eb50baac4d6c5c5bf6aa4d898143c8f2f9899871c6c2991d1b59
2e041b4905 help: enrich help text for `-loadblock` (Hao Xu)
Pull request description:
`-loadblock` doesn't support XOR-ed files, mention it in its help text to avoid troubles for users.
ACKs for top commit:
maflcko:
lgtm ACK 2e041b4905
sedited:
ACK 2e041b4905
Tree-SHA512: 1883824c15687799d60ddabc53d5dc2c274e56c8292da5e7e412adaef65abf70c7f778ef7675c65f738c1e4855e5b0f929e87631b0a52695873e58f8245b3a8d
98fcd7af23 wallet: rpc: Improve error message for low feerates. (David Gumberg)
Pull request description:
Giving the user a hint about what action to take when they encounter an error related to a feerate that is below the minimum. I encountered this myself not knowing about `-mintxfee` and the manpage was slightly less than clear,
```
-mintxfee=<amt>
Fee rates (in BTC/kvB) smaller than this are considered zero fee for transaction creation
(default: 0.00001)
```
ACKs for top commit:
achow101:
ACK 98fcd7af23
ismaelsadeeq:
ACK 98fcd7af23
w0xlt:
ACK 98fcd7af23
Tree-SHA512: 8471bfd5682ca59137541ab4da670e85947cf7812baa9e07d816e352fccfb5e0e71a06c2baf1a5422d78a7da8c08b9fcbb73d85cc9daba88b7c22e802abe5b2e
5a81d73a81 scripted-diff: rpc: Don't pointlessly capture in RPCMethod lambdas (Anthony Towns)
4e789299af scripted-diff: rpc: Rename RPCHelpMan to RPCMethod (Anthony Towns)
Pull request description:
When defining `RPCHelpMan` objects, we usually return a lambda, and mostly we define those via `[&](...) { ... }` which explicitly captures any parameters or local variables by reference. If we were to actually use any of those captures (we don't), we would invoke undefined behaviour. So instead, convert all the `[&]` to `[]` to avoid capturing.
While we're at it, rename `RPCHelpMan` to `RPCMethod`, reflecting its greater responsibility since #19386.
ACKs for top commit:
maflcko:
review ACK 5a81d73a81🏣
stickies-v:
ACK 5a81d73a81
rkrux:
code review ACK 5a81d73a81
Tree-SHA512: 72e9232857ba5bf4c346fb963a2028047f7c7e9b420ef58b3108669204bfbb6952342cfcfd2e8a06f813a21545abf7fc8b0ad422f00d7ec9c1134626cd1650e6
12c3c3f81d test: mining: add coverage for GBT's "coinbasevalue" result field (Sebastian Falbesoner)
Pull request description:
This PR adds missing coverage for the "coinbasevalue" result field of the `getblocktemplate` RPC call. Specifically, the introduced test checks that the value is set to claim the full block reward (subsidy plus fees). Can be verified with the following patch, which succeeds on master and fails on the PR branch:
```diff
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index a935810d91..ba9ac9dadb 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -998,7 +998,7 @@ static RPCHelpMan getblocktemplate()
result.pushKV("previousblockhash", block.hashPrevBlock.GetHex());
result.pushKV("transactions", std::move(transactions));
result.pushKV("coinbaseaux", std::move(aux));
- result.pushKV("coinbasevalue", block.vtx[0]->vout[0].nValue);
+ result.pushKV("coinbasevalue", block.vtx[0]->vout[0].nValue - 1);
result.pushKV("longpollid", tip.GetHex() + ToString(nTransactionsUpdatedLast));
result.pushKV("target", hashTarget.GetHex());
result.pushKV("mintime", GetMinimumTime(pindexPrev, consensusParams.DifficultyAdjustmentInterval()));
```
I'm not sure how relevant this field is nowadays in real-world mining scenarios (we use it for the signet miner at least), but it seems useful to have a test for it anyways. Stumbled upon this while looking at https://github.com/bitcoin-inquisition/bitcoin/pull/111.
ACKs for top commit:
maflcko:
lgtm ACK 12c3c3f81d
Sjors:
ACK 12c3c3f81d
Tree-SHA512: ae10f63c99b21cf7771feefe3d0e47b2e0d511aceb344cf11efa9182d78f2960f1e079797b8a36db110a3c54acb27a3706413a710a74bf56aed29ea162a6aab2
5fa6898818 policy: remove incorrect MANDATORY_SCRIPT_VERIFY_FLAGS comment (ismaelsadeeq)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/33050
This comment claiming that failing mandatory script checks may trigger a DoS ban/punishment is false.
`CheckInputScript` also dispute this comment.
b0f68f0a3a/src/validation.cpp (L2113-L2118)
ACKs for top commit:
darosior:
utACK 5fa6898818
instagibbs:
ACK 5fa6898818
Tree-SHA512: 84ad68b158e334b0e945f33007de58807b0b3335b1873412b0f359f0fa52d0819e93f42b28ebb6fbe5b355f2710f61e0dd731dc4bce20ccc1b7b2c546a9df550
fabab69e9e refactor: Return std::optional from ParseDouble (MarcoFalke)
fa0a09441d refactor: Return std::optional from GetWalletNameFromJSONRPCRequest (MarcoFalke)
fafb0c4cbe refactor: Return std::optional from GetLogCategory (MarcoFalke)
Pull request description:
Using a bool to indicate whether a mutable in-out param was written to is fine in legacy code, but otherwise confusing and brittle:
* Sometimes the in-out-param is written to, even when the function returns `false`, like in `ParseDouble`.
* Call sites must manually check the return value
Fix those issues by returning `std::optional<_>` from `ParseDouble` (and a few other functions).
This refactor is a style cleanup and does not change any behavior.
ACKs for top commit:
stickies-v:
re-ACK fabab69e9e
hodlinator:
crACK fabab69e9e
rkrux:
code review ACK fabab69e9e
Tree-SHA512: e27a27174e9d2200da8b0ca9b6cc056e94d2b25e6332975f1ad660ee85b02680a65ac93b2ed29f10da0ae5f6dc8395bddc9e973a3f925c68a0c8116c9282ea09
