7295b8be70 chainparams: remove my testnet3 seed (Sjors Provoost)
Pull request description:
With testnet3 long deprecated, albeit still not dropped #31975, and [testnet5](https://groups.google.com/g/bitcoindev/c/kGUMTxOvdJA) on the horizon, this seems like a good time. I plan to keep it up and running until v31 is end-of-life, so no need to backport.
ACKs for top commit:
fanquake:
ACK 7295b8be70
nebula-21:
ACK 7295b8be70
willcl-ark:
ACK 7295b8be70
Tree-SHA512: 30c9ad9480a09cf48138157b5529e2878469bc629edb60c5dd2605e078647b9b9b025937936d1b6e3a980c6be3fc36c512bdd91a6864c72c3e093cd60d691860
ca9ffb8e12 rpc: add OpenRPC discovery alias (willcl-ark)
ef0676f400 rpc: factor getaddressinfo embedded field docs (will)
1fb6b60560 test: add functional test for getopenrpcinfo (will)
672dd42d14 rpc: add getopenrpcinfo command (will)
f5116c587f rpc: add placeholder annotation for deprecated params (will)
26c221a980 rpc: expose RPC metadata for introspection (will)
6a1a66c180 rpc: render Type::ANY in help text instead of aborting (will)
06de34a033 rpc: erase empty map entry in removeCommand (will)
d4d64ae739 rpc: add missing string_view include to server.h (will)
Pull request description:
Fixes#29912
This PR adds a machine-readable[ OpenRPC](https://www.open-rpc.org/) 1.4.1 specification of out JSON-RPC interface, auto-generated from existing `RPCHelpMan` metadata.
There is currently no formal, machine-readable specification of the RPC API. As discussed in #29912, this has knock-on consequences:
- Client libraries re-implement the API manually, leading to bugs like unit mistakes (sats vs BTC, vB vs kvB) and missing/incorrect argument types. No existing client library fully and correctly implements the API in a type-safe manner.
- When the API changes, every downstream client must manually discover and adapt, creating downstream maintenance burden. There is no artifact they can diff between releases.
- Implementing a new client in a new language requires reading C++ source or help text and transcribing it, which is error-prone and tedious, and represents an on-going porting cost.
- Existing documentation is either stale or not machine-readable. The developer.bitcoin.org docs are wrong/outdated in places, and the bitcoincore.org/en/doc/ pages are rendered from help output but not in a standard schema format.
- (new/extra) AI/LLM tooling increasingly builds on structured API specifications. A standard spec format enables AI-assisted client generation and integration without the ambiguity of parsing human-readable help text.
This draft builds on prior art by casey and the observations by laanwj, stickies-v, kilianmh, hodlinator, and cdecker in #29912. Casey's work demonstrated that RPCHelpMan already contains all the structured information needed, which makes this feasible without duplicating any API definitions.
This differs from Casey's branches in that it uses the OpenRPC standard rather than an ad-hoc format or raw JSON Schema.
### Why OpenRPC
I seletced OpenRPC for a number of reasons:
- It's purpose-built for JSON-RPC APIs (suggested by stickies-v,nflatrea, and kilianmh).
- It wraps JSON schema for params/results, so consumers get both the method-level structure and the type-level schemas.
- Unlike OpenAPI, it is not path-centric, which better fits our single-endpoint JSON-RPC model (concern raised by hodlinator).
- Although it therefore does not cover our REST interface.
- It's _kind of_ a standard format with (_some_) existing tooling for type generation (TypeScript, Rust, Python, Go) and client scaffolding, though maturity varies by language. More importantly though, IMO, a ~standardised format is inherently more useful than any ad-hoc one: any JSON Schema validator works, any LLM can consume it directly, and anyone can write a bespoke generator against a known schema rather than parsing help text.
### Approach
`RPCHelpMan` metadata → `getopenrpcinfo` / `rpc.discover` → OpenRPC JSON
### Tradeoffs
vs an ad-hoc format OpenRPC gives us interoperability with the (admittedly surprisingly limited) tooling, documentation generators, code generators, and validators, at the cost of needing x-bitcoin-* extensions for Bitcoin-specific concepts. As Casey noted after trying both approaches, JSON Schema "is probably not a great fit". OpenRPC's method-level framing on top of JSON Schema addresses the ergonomic issues while keeping the schema benefits. After testing both, I think I agree.
Types: JSON Schema cannot natively express all Bitcoin-specific semantics. Amount result fields are represented as JSON numbers with `x-bitcoin-unit: amount`; other Bitcoin-specific distinctions remain in descriptions or `x-bitcoin-*` extensions. More structured unit metadata and stronger constraints can be added in follow-up work.
Some RPCs return different types depending on argument values (e.g. verbosity levels). These are represented as `oneOf` in the result schema with free-text condition descriptions. This is accurate but not fully machine-parseable — a code generator cannot automatically determine which result variant corresponds to which argument value without parsing the description. I still we have enough information to satisfy humans an agents alike though.
### Regenerating the spec
The functional test invokes both RPCs, verifies valid JSON, checks public and hidden RPC handling, and covers representative generated schemas. It does not compare a committed generated artifact.
`getopenrpcinfo` omits hidden RPCs and arguments by default; `getopenrpcinfo(true)` includes them. The standard parameterless `rpc.discover` method returns the public document.
The RPC output documents which RPCs are available for any given built binary.
### Discussion questions
- Is this valuable/wanted?
- Do we like openrpc format? (less relevant if we don't want this in this repo, as another repo could generate one or many definitions).
- Should we cover "hidden" RPCs? They are currently hidden, but don't have to be...
My personal thoughts are that this is very nice to have.
ACKs for top commit:
dergoegge:
ACK ca9ffb8e12
achow101:
ACK ca9ffb8e12
sedited:
ACK ca9ffb8e12
w0xlt:
ACK ca9ffb8e12
Tree-SHA512: 5bf7abdb9f119d591306884f31b9da815908e5cb5812706c5496b39162c22cd949465d7b3ff1fdce7ff5b3c1b6367c14b6d28d58dee86698c0161d4d844d84c9
75929b11ed doc: add release note for submitSolution IPC changes (woltx)
ed75d70fdb refactor: centralize SubmitBlock result handling (w0xlt)
cbaa1696f3 mining: add reason and debug output to submitSolution (w0xlt)
83f3bc002d mining: clarify SubmitBlock result handling (w0xlt)
Pull request description:
`BlockTemplate.submitSolution` currently returns only a boolean, so IPC mining clients cannot determine why a submission failed without inspecting Bitcoin Core's debug log.
Returning `reason` and `debug`, as `Mining.submitBlock` already does, lets callers distinguish a concrete block rejection from a duplicate or inconclusive result. Here, `inconclusive` means the method returns failure, but validation did not determine that the submitted block is invalid.
This follow-up was suggested during the review of #34644:
https://github.com/bitcoin/bitcoin/pull/34644#discussion_r2853758006
This PR:
- Extracts a shared `SubmitBlock` helper that wraps `ProcessNewBlock` with `SubmitBlockStateCatcher` to capture `BlockValidationState`
- Adds `reason` and `debug` output parameters to `submitSolution`, matching `submitBlock`
- Makes both methods delegate to the same helper, eliminating duplicated logic
ACKs for top commit:
optout21:
ACK 75929b11ed
achow101:
light ACK 75929b11ed
Sjors:
ACK 75929b11ed
enirox001:
ACK 75929b11ed
sedited:
ACK 75929b11ed
Tree-SHA512: 31b1c305c20aaebdfa2d887665d9927830d0f97ba3c3469e2792148ad799d5a400a000cc0ca0b9add071d314e27c9da44d55228c442533a32a7c031678b78a55
7e19ce200b rpc: Fix descriptorprocesspsbt internal bug on invalid signatures (b-l-u-e)
Pull request description:
Fixes#32849
descriptorprocesspsbt crashes with an internal bug assertion when given a PSBT whose inputs looked “finalized” that is non-empty final_script_sig / witness but whose signatures did not verify like invalid Schnorr bytes with unusual sighash combinations such as SIGHASH_SINGLE | ANYONECANPAY.
Completeness was inferred with PSBTInputSigned, which only checks that final fields are present, not that they pass script verification. That let the RPC treat the PSBT as complete and call FinalizeAndExtractPSBT, where a failing invariant surfaced as CHECK_NONFATAL instead of a normal incomplete outcome.
This PR determines complete the same way as the wallet path: after ProcessPSBT, it recomputes PrecomputedTransactionData and sets complete only if every input passes PSBTInputSignedAndVerified.
Invalid signatures then yield complete: false and no hex, without going through the finalize path that asserted.
ACKs for top commit:
achow101:
ACK 7e19ce200b
rkrux:
lgtm re-ACK 7e19ce200b
Tree-SHA512: c534fb5bd3401ee3ac0bc27eaedaaaaf23e5e8510ed9b96a747bb0e41e2bd3d2031b6b08e933e139bb426e97bd05fab6e91397205f4be1db9d17630eea49a8bf
51d36dfd07 qt: Fix `-Wsfinae-incomplete` warnings when building with GCC 16.x (Hennadii Stepanov)
Pull request description:
According to the CMake documentation for [`AUTOMOC`](https://cmake.org/cmake/help/latest/prop_tgt/AUTOMOC.html), all `moc` output files that are not included in a source file are aggregated into the CMake-generated `<AUTOGEN_BUILD_DIR>/mocs_compilation.cpp`, which is added to the target's sources.
Within that single translation unit, `moc`-generated code checks the completeness of a signal or slot parameter type while it is still only forward-declared, and the type is completed later, when a subsequently included `moc_*.cpp` file pulls in the header that defines it. GCC 16.x diagnoses this pattern with the `-Wsfinae-incomplete` warning, which is enabled by default.
Including the `moc` output files at the end of the corresponding source files excludes them from `mocs_compilation.cpp`, so each one is compiled in a translation unit where the relevant types are complete.
FWIW, Qt itself uses the same approach throughout its own codebase. Also see https://www.youtube.com/watch?v=Cx_m-qVnEjo.
---
Steps to reproduce on the master branch @ 18c05d9301 on Fedora 44 (GCC 16.1.1):
```console
$ cmake --preset dev-mode
$ cmake --build build_dev_mode -t bitcoind
$ cmake --build build_dev_mode -t bitcoin-qt
[166/172] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/mocs_compilation.cpp.o
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_bitcoingui.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:9:
/home/hebasto/dev/bitcoin-gui/src/qt/bitcoingui.h:67:7: warning: defining ‘BitcoinGUI’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
67 | class BitcoinGUI : public QMainWindow
| ^~~~~~~~~~
In file included from /usr/include/qt6/QtCore/qobject.h:19,
from /usr/include/qt6/QtWidgets/qwidget.h:10,
from /usr/include/qt6/QtWidgets/qdialog.h:9,
from /usr/include/qt6/QtWidgets/QDialog:1,
from /home/hebasto/dev/bitcoin-gui/src/qt/addressbookpage.h:8,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_addressbookpage.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:2:
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/src/qt/paymentserver.h:35,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_paymentserver.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:27:
/home/hebasto/dev/bitcoin-gui/src/qt/sendcoinsrecipient.h:15:7: warning: defining ‘SendCoinsRecipient’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
15 | class SendCoinsRecipient
| ^~~~~~~~~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/src/qt/psbtoperationsdialog.h:13,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_psbtoperationsdialog.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:30:
/home/hebasto/dev/bitcoin-gui/src/qt/walletmodel.h:48:7: warning: defining ‘WalletModel’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
48 | class WalletModel : public QObject
| ^~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_qvalidatedlineedit.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:32:
/home/hebasto/dev/bitcoin-gui/src/qt/qvalidatedlineedit.h:13:7: warning: defining ‘QValidatedLineEdit’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
13 | class QValidatedLineEdit : public QLineEdit
| ^~~~~~~~~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_rpcconsole.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:37:
/home/hebasto/dev/bitcoin-gui/src/qt/rpcconsole.h:43:7: warning: defining ‘RPCConsole’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
43 | class RPCConsole: public QWidget
| ^~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
In file included from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/EWIEGA46WW/moc_sendcoinsentry.cpp:9,
from /home/hebasto/dev/bitcoin-gui/build_dev_mode/src/qt/bitcoinqt_autogen/mocs_compilation.cpp:39:
/home/hebasto/dev/bitcoin-gui/src/qt/sendcoinsentry.h:26:7: warning: defining ‘SendCoinsEntry’, which previously failed to be complete in a SFINAE context [-Wsfinae-incomplete=]
26 | class SendCoinsEntry : public QWidget
| ^~~~~~~~~~~~~~
/usr/include/qt6/QtCore/qmetatype.h:344:64: note: here. Use ‘-Wsfinae-incomplete=2’ for a diagnostic at that point
344 | static auto check(U *) -> std::integral_constant<bool, sizeof(U) != 0>;
| ^~~~~~~~~
[172/172] Linking CXX executable bin/bitcoin-qt
```
ACKs for top commit:
maflcko:
review ACK 51d36dfd07🦅
Tree-SHA512: fe48e4925aaccb833bc065aa7f988f22482ef83d3d555e601d3955109f840b221be971c560668eac79506f567999d4e1b2a464ec3717b5cecb9a3eaaa1dbc01b
7298281ba8 bitcoin-util: replace netmagic command with getchainparams command (Anthony Towns)
Pull request description:
This is a follow-up to #35610. It replaces the `netmagic` command with a more versatile `getchainparams` command, as suggested in https://github.com/bitcoin/bitcoin/pull/35610#issuecomment-4974474640.
ACKs for top commit:
maflcko:
re-ACK 7298281ba8📓
ajtowns:
Coauthor ACK 7298281ba8
sedited:
ACK 7298281ba8
Tree-SHA512: 153e97eb8d6bc6d98d925ce9718239c6ff5d7a80b760e7f3b807fc36f78e8da8550033c31016e1d1aee11484392dd80b76f9b1496a8dc6e47825910c170a0cd1
3bfdcbd7ee coins: reuse cache hasher for txid set (Lőrinc)
2beab94896 coins: use SipHash-1-3-UJ for `CCoinsMap` (Lőrinc)
7ff55cc650 bench: add fixed-width SipHash benchmarks (Lőrinc)
3aea85411f test: add SipHash-1-3-UJ coverage (Pieter Wuille)
a0ccd4ad17 crypto: add fixed-width SipHash-1-3-UJ (Pieter Wuille)
c2d7931b5c crypto: add generic SipHash-1-3-UJ (Pieter Wuille)
25bfca06d6 refactor: simplify adding SipHash-1-3-UJ (Lőrinc)
af50ba8500 test: add shared SipHash vectors (Lőrinc)
Pull request description:
**Problem:** The in-memory UTXO cache hashes `COutPoint` keys containing a 32-byte txid and a 32-bit output index.
SipHash-2-4 processes the txid as four independent 64-bit blocks, so its optimized 32-byte and 36-byte paths both take 14 SipRounds.
This also matters for hash-prefix index work such as [#35531](https://github.com/bitcoin/bitcoin/pull/35531): once a persisted key format chooses a hash function, changing it later requires reindexing.
**Fix:** Add `SipHasher13UJ`, a custom block-oriented variant combining Pieter Wuille's jumbo-block suggestion with SipHash-1-3, the reduced-round variant discussed in the [SipHash analysis](https://eprint.iacr.org/2012/351.pdf).
It provides inline `Hash` overloads for the fixed-width inputs used here.
Use a dedicated `SaltedCoinsCacheHasher` for `CCoinsMap` and `CoinsViewOverlay`'s temporary earlier-txid set, while other outpoint tables remain on SipHash-2-4.
The salted hash values vary between restarts and are never persisted or sent over the network.
**Design:** `SipHasher13UJ` accepts normal 64-bit blocks and 256-bit jumbo blocks.
For hash-table use, cryptographic hash outputs must make up all but a small bounded number of retained jumbo blocks.
The construction mixes all four limbs around one SipRound, omits byte-oriented padding, and uses an `"unpadded"` finalizer distinct from standard SipHash-1-3.
The fixed-width paths take four rounds for one `uint256` jumbo block and five when followed by one normal block.
For outpoints, the 32-bit output index is zero-extended into a normal 64-bit block.
Retained `CCoinsMap` entries identify real transaction outputs, so their keys contain computed txids.
Missing-input validation may probe arbitrary claimed prevouts, but `FetchCoin()` immediately erases their temporary entries when the backend lookup fails, so non-hash keys cannot accumulate.
The assumeutxo loader assumes snapshot txids are valid while loading and verifies the complete snapshot's content hash before activation.
Every entry in the temporary earlier-txid set is a computed transaction hash, and the set is bounded by the block's transaction count.
This construction is limited to local hash tables and is not a general-purpose or protocol SipHash replacement.
Pieter discussed the construction with [SipHash co-author Jean-Philippe Aumasson](https://github.com/bitcoin/bitcoin/pull/35215#issuecomment-4385336928), whose preliminary analysis did not find an easier collision construction and supported SipHash-1-3 for this hash-table use.
<img width="2100" height="860" alt="siphash_compare_updated" src="https://github.com/user-attachments/assets/cefec6f8-5ec0-450a-a0a2-f946de9ef36d" />
**Structure:** Shared vectors first cover the existing generic and fixed SipHash-2-4 paths in C++, the generic path in Python, and their randomized equivalence in the fuzzer.
A behavior-neutral refactor then moves the round, compression, and finalization logic into inline `SipHashState` methods; assembly inspection shows that the fixed-width paths retain their instruction counts, while the generic byte loop retains its prior code generation through a local state copy.
Three Pieter-authored commits add the generic UJ specification, fixed-width implementation, and shared correctness coverage.
Benchmarks follow that coverage, then separate commits change `CCoinsMap`'s hasher and reuse it for the temporary earlier-txid set.
**Tests:** The shared JSON supplies the same byte sequences to the generic C++ and Python SipHash-2-4 implementations, with applicable fixed-width paths checked against the same expected output.
The SipHash-2-4 rows include the 64 official vectors for inputs from 0 to 63 bytes and cases that vary input chunking.
The UJ outputs were generated by an independent implementation and are checked using normal blocks, equivalent zero-extended jumbo blocks, and applicable fixed-width `Hash` overloads.
The integer fuzzer extends these comparisons to arbitrary values and mixed normal/jumbo block encodings.
[Counting the dbcache buckets](https://gist.github.com/l0rinc/d68f56c3ed89f76f56da6632ef6f2d92) indicates the new outpoint hasher retains the uniform bucket distribution expected by `CCoinsMap`:
<img width="1200" height="750" alt="ccoinsmap-collisions" src="https://github.com/user-attachments/assets/eeedec81-acdc-4adf-a9c8-bfce089700da" />
**Benchmarks:** Fixed-width microbenchmarks compare SipHash-2-4 with SipHash-1-3-UJ for 32-byte hashes and inputs containing a 32-byte hash plus a 32-bit index.
Reported aarch64 measurements and an [independent x86_64 run](https://github.com/bitcoin/bitcoin/pull/35215#issuecomment-4400609637) show the outpoint path is about 2x faster.
<details><summary>Benchmark runner</summary>
```bash
for COMPILER in gcc clang; do \
if [ "$COMPILER" = gcc ]; then CC=gcc; CXX=g++; else CC=clang; CXX=clang++; fi; \
cmake -B "build-bench-$COMPILER" -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCH=ON -DBUILD_TESTS=OFF -DBUILD_GUI=OFF -DENABLE_WALLET=OFF -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" >/dev/null 2>&1 && \
cmake --build "build-bench-$COMPILER" --target bench_bitcoin -j"$(nproc)" >/dev/null 2>&1 && \
echo "" && echo "$(date -I) | SipHash fixed-width microbench | $("$CXX" --version | head -1) | $(hostname) | $(uname -m) | $(lscpu | awk -F: '/Model name/{print $2; exit}' | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM" && \
"build-bench-$COMPILER/bin/bench_bitcoin" -filter='SipHash.*32b|SipHash.*36b' -min-time=10000; \
done
```
</details>
A two-run GCC `-reindex-chainstate` comparison of the same `CCoinsMap` hot path through height 957,759 with `-dbcache=2000` on a Ryzen 7 3700X/SSD reduced mean wall time from 11,278 s to 10,759 s, a ~5% validation speedup.
ACKs for top commit:
achow101:
light ACK 3bfdcbd7ee
sipa:
ACK 3bfdcbd7ee (to the extent the code/ideas aren't my own)
andrewtoth:
ACK 3bfdcbd7ee
optout21:
ACK 3bfdcbd7ee
Tree-SHA512: c3c66051cb1ebdb0cddbc8b8bed2297c524842f92960531de23d9586c5bb950b302c33d06fc15c2320cbbf97273b22ec3f17fd0e7ddcc7df4a8132a47a606277
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
According to the CMake documentation for `AUTOMOC`, all `moc` output
files that are not included in a source file are aggregated into the
CMake-generated `<AUTOGEN_BUILD_DIR>/mocs_compilation.cpp`, which is
added to the target's sources.
Within that single translation unit, `moc`-generated code checks the
completeness of a signal or slot parameter type while it is still only
forward-declared, and the type is completed later, when a subsequently
included `moc_*.cpp` file pulls in the header that defines it. GCC 16.x
diagnoses this pattern with the `-Wsfinae-incomplete` warning, which is
enabled by default.
Including the `moc` output files at the end of the corresponding source
files excludes them from `mocs_compilation.cpp`, so each one is
compiled in a translation unit where the relevant types are complete.
Additionally:
1. Some of the `BitcoinGUI` class's private members are gated with
`#ifdef ENABLE_WALLET` to prevent `-Wunused-private-field` warnings
when building with `-DENABLE_WALLET=OFF`.
2. `test/lint/lint-includes.py` is adjusted to allow new `#include`
statements.
Lock SipHash-2-4 behavior into shared vectors before refactoring its round and finalization code.
Store inputs as ordered hex byte blocks so `CSipHasher` and the independent Python implementation hash the same byte sequence, with applicable `PresaltedSipHasher` overloads checked against the same vectors.
Add the 64 official SipHash-2-4 vectors alongside block-partition and empty-block cases for the generic path.
Move randomized generic/fixed comparisons to the integer fuzzer.
SipHash-1-3-UJ coverage can add expected outputs for compatible 8- and 32-byte block sequences.
The Python test reads a build-tree copy so functional-test staging behaves consistently when files are symlinked or copied.
Co-authored-by: Pieter Wuille <pieter@wuille.net>
OpenRPC service discovery specifies `rpc.discover` as the discovery
method name.
Expose `rpc.discover` as an alias for `getopenrpcinfo`, so clients that
expect the standard OpenRPC discovery method can retrieve the same
generated document without changing the existing Bitcoin Core RPC.
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
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
The previous binary used a number of `Boost.Test` features:
- `boost::unit_test::disable`
- `BOOST_FAIL`
- `boost::exit_test_failure`
This patch duplicates the previous mock process behavior with no boost features.
With the patch we can:
- simplify the test config
- remove a linted boost include
- remove a file that was not actually a test
dc1c17c085 doc: add release notes (Andrew Toth)
0e10937184 fuzz: add coins_view_stacked fuzz harness to test concurrent leveldb reads (Andrew Toth)
ce610a6ff4 fuzz: update harnesses to cover CoinsViewOverlay::StartFetching (Andrew Toth)
760fb22dc3 test: add unit tests for CoinsViewOverlay::StartFetching (Andrew Toth)
d69a3b20de doc: update CoinsViewOverlay docstring to describe parallel fetching (Andrew Toth)
ab2a379237 coins: fetch inputs in parallel (Andrew Toth)
fdf283036a coins: add ready flag to InputToFetch (Andrew Toth)
ede11b8314 validation: collect block inputs in CoinsViewOverlay before ConnectBlock (Andrew Toth)
f82043af50 coins: introduce thread pool in CoinsViewOverlay (Andrew Toth)
5bf1c32008 validation: add -prevoutfetchthreads configuration option (Andrew Toth)
Pull request description:
This PR is a continuation of https://github.com/bitcoin/bitcoin/pull/31132. All outstanding issues raised there have been resolved, but the volume of stale comments can make that change difficult to review.
Currently, when connecting a block, each input prevout is looked up one at a time. For every input we first check the in-memory coins cache, and on a miss we make a synchronous round-trip to the chainstate LevelDB to read the coin from disk. Because these lookups happen serially as the block is being validated, the disk read latency stacks up and dominates the time spent in `ConnectBlock` whenever many inputs are not already in the cache.
This PR moves those disk reads onto a pool of worker threads that run in parallel with block connection. Before entering `ConnectBlock` the block is handed to a `CoinsViewOverlay`, which kicks off the workers to begin fetching all of the block's prevouts from disk and warming the cache. The main validation thread continues to do exactly the same work it does today, hitting the cache for each input in order. The only difference is that by the time it asks, the coin is much more likely to already be there. There are no validation logic or consensus behavior changes. This is purely a parallelization of an existing read pattern.
The number of fetcher threads is configurable via `-prevoutfetchthreads=<n>`, defaulting to 8 and capped at 16. Setting it to 0 disables input fetching entirely and reverts to the previous serial behavior.
We have measured large performance gains for IBD and `-reindex-chainstate`, as well as worst-case steady-state block connection at the tip. l0rinc ran many thorough benchmarking passes on the original PR across multiple machines, storage types, dbcache sizes[^1], operating systems[^2], and fetcher thread counts[^3]. Many other contributors also posted their benchmark results in the original PR. IBD speedups range from 1.18× to over 3× faster[^4]. Worst-case block connection time for network-attached storage was over 2× faster[^5]. Flamegraph comparisons before and after this change are available[^6].
On safety: `ConnectBlock` runs while holding `cs_main`, so nothing else in the node can mutate the chainstate while the fetchers are reading it.
On LevelDB: [concurrent reads are fully supported](https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/include/leveldb/db.h#L44) and [documented as such](https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/doc/index.md#concurrency). We already rely on this in production today against our other LevelDB-backed databases. The `txindex` DB is read by multiple simultaneous HTTP RPC worker threads via the `getrawtransaction` RPC. The `blockfilterindex` DB is called concurrently from both the P2P `cfilters` / `cfheaders` / `cfcheckpt` message handlers on the `msghand` thread, and from the `getblockfilter` RPC on the HTTP RPC worker threads. We have not yet been issuing concurrent reads against the chainstate DB, but there is no LevelDB-side reason we can't. In fact, the chainstate DB is already being touched by more than one thread on master, because LevelDB schedules its own background compaction work.
For reviewers:
The main change is `CoinsViewOverlay` gets 1 new public and 2 new private methods.
- `StartFetching`: public method called in lieu of `CreateResetGuard` before we enter `ConnectBlock`. It still returns a `ResetGuard` so the view is `Reset` before the block it is working on leaves scope. This kicks off worker threads who each just run `while (ProcessInput()) {}` and then return.
- `StopFetching`: private method called on `Reset` whenever the guard leaves scope or `Flush`. Stops all threads and clears multi threaded state.
- `ProcessInput`: private method that fetches a single input prevout. Returns `true` if an input was fetched and `false` otherwise. This is the only method on `CoinsViewOverlay` that is called concurrently by multiple threads. Every other method on the overlay is still called synchronously on the main thread.
The `CoinsViewOverlay::FetchCoinFromBase` method is also extended to lookup the coins fetched from `ProcessInput` first before falling back to `base->PeekCoin`.
Mutating methods `Reset` and `Flush` are overridden in `CoinsViewOverlay` to call `StopFetching` first.
[^1]: https://github.com/bitcoin/bitcoin/pull/31132#pullrequestreview-3515011880
[^2]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3767758819
[^3]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3617721711
[^4]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3678847806
[^5]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-4071032270
[^6]: https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3617315125
ACKs for top commit:
l0rinc:
reACK dc1c17c085
willcl-ark:
ACK dc1c17c085
theStack:
re-ACK dc1c17c085
ryanofsky:
Code review ACK dc1c17c085 with changes to StopFetching and AllInputsConsumed checking behavior since last review.
Tree-SHA512: 89c1c2890f65aac5cd546edc44504956c47b6fada256d3b86ced47e6dd8c72f633a4357753b3b9805b9ba6ed02790822090d70578aba2964baf50d7eb956864c
b0735336ee p2p: Don't participate in addr relay with feeler connections (Daniela Brozzoni)
Pull request description:
Feeler connections are short-lived connection made to check that a node is alive, useful for test-before-evict, and for moving addresses from the new to the tried table.
We currently send a GETADDR message to feelers, but then disconnect before being able to receive a response. This GETADDR is not useful and can be removed.
I couldn't find any previous discussion about this, but I found PR #22777, that similarly made sure that we don't ask for tx relay to feelers.
---
I noticed this behavior on my peer-observer instance: I would see the number of sent GETADDR messages increase over time, but the number of ADDR messages with >100 addresses received (which are likely GETADDR responses and not self announcements relays) wouldn't increase as much. I later realized that it was my node opening feeler connections, sending a GETADDR, and closing the connection.
You can see the same behavior using this command - the node is making feeler connections, sending getaddr to them, closing before receiving the addr response:
```
~ ₿ tail -f ~/.bitcoin/debug.log | grep -E "(Making feeler connection|Added connection to|sending getaddr|feeler connection completed|Received addr: [0-9]{2,} addresses)"
2026-04-02T13:25:50Z [net] Making feeler connection to xyz.onion:8333
2026-04-02T13:26:06Z [net] Added connection to xyz.onion:8333 peer=27
2026-04-02T13:26:08Z [net] sending getaddr (0 bytes) peer=27
2026-04-02T13:26:08Z [net] feeler connection completed, disconnecting peer=27, peeraddr=xyz.onion:8333
```
On a node that accepts inbounds connections, this command can be used to see in the logs all the nodes that connected, sent a getaddr, and disconnected before receiving a reply. It is possible that these nodes connected to us as a feeler:
```
~ ₿ cat .bitcoin/debug.log | awk '
/received: getaddr/ {
split($0, a, "peer=")
got_getaddr[a[2]] = $0
}
/sending addr/ {
split($0, a, "peer=")
sent_addr[a[2]] = 1
}
/socket closed/ {
split($0, a, "peer=")
id = a[2]
if (id in got_getaddr && !(id in sent_addr)) {
print "possible feeler: " got_getaddr[id]
print " " $0
}
delete got_getaddr[id]
delete sent_addr[id]
}
'
possible feeler: 2026-04-01T21:45:13Z [net] received: getaddr (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] socket closed, disconnecting peer=2311974
possible feeler: 2026-04-02T00:18:58Z [net] received: getaddr (0 bytes) peer=2426389
2026-04-02T00:18:58Z [net] socket closed, disconnecting peer=2426389
...
```
Then, you can manually inspect one of them:
```
~ ₿ cat .bitcoin/debug.log | grep -E "peer=2311974"
2026-04-01T21:45:13Z [net] Added connection peer=2311974
2026-04-01T21:45:13Z [net] received: version (102 bytes) peer=2311974
2026-04-01T21:45:13Z [net] sending version (102 bytes) peer=2311974
2026-04-01T21:45:13Z [net] send version message: version 70016, blocks=943279, txrelay=0, peer=2311974
2026-04-01T21:45:13Z [net] sending wtxidrelay (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] sending sendaddrv2 (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] sending verack (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] receive version message: /Satoshi:27.0.0/: version 70016, blocks=943279, us=x.x.x.x:8333, txrelay=0, peer=2311974
2026-04-01T21:45:13Z [net] received: wtxidrelay (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] received: sendaddrv2 (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] received: verack (0 bytes) peer=2311974
2026-04-01T21:45:13Z New inbound v1 peer connected: version: 70016, blocks=943279, peer=2311974
2026-04-01T21:45:13Z [net] sending sendcmpct (9 bytes) peer=2311974
2026-04-01T21:45:13Z [net] sending ping (8 bytes) peer=2311974
2026-04-01T21:45:13Z [net] sending getheaders (1029 bytes) peer=2311974
2026-04-01T21:45:13Z [net] initial getheaders (943278) to peer=2311974 (startheight:943279)
2026-04-01T21:45:13Z [net] received: getaddr (0 bytes) peer=2311974
2026-04-01T21:45:13Z [net] Advertising address x.x.x.x:8333 to peer=2311974
2026-04-01T21:45:13Z [net] socket closed, disconnecting peer=2311974
2026-04-01T21:45:13Z [net] Resetting socket for peer=2311974
2026-04-01T21:45:13Z [net] sending addrv2 (24665 bytes) peer=2311974
2026-04-01T21:45:13Z [net] Cleared nodestate for peer=2311974
```
ACKs for top commit:
0xB10C:
ACK b0735336ee
achow101:
ACK b0735336ee
andrewtoth:
ACK b0735336ee
stratospher:
ACK b073533. didn't see any addr message from feelers in my node's last 24 hours/it would disconnect before addr message is received. so consistent with today's behaviour.
Tree-SHA512: 1ac220dfd8361c4687399546a0d968d268e447446053fb8b90ba6b987482cc038e2ad94e670f33181d08d6d0c576882328bb5d0a8b8b1175a5e2ec31ff051833
9784818442 mining: add getTransactionsByWitnessID() IPC method (Sjors Provoost)
d282ae6883 mining: add getTransactionsByTxID() IPC method (Sjors Provoost)
0d5e4d4712 test: restart node after IPC option override test (Sjors Provoost)
f16b3613cd ipc: Serialize null CTransactionRef as empty Data (Sjors Provoost)
0f466e1094 mempool: add lookup by witness hash (Sjors Provoost)
Pull request description:
For Stratum v2 custom job declaration to be bandwidth efficient, the pool can request[^0] only the transactions that it doesn't know about.
The spec doesn't specify how this is achieved, but one method is to call the `getrawtransaction` RPC on each transaction id listed in [DeclareMiningJob](https://stratumprotocol.org/specification/06-Job-Declaration-Protocol?query=DeclareMiningJob#644-declareminingjob-client-server) (or a subset if the pool software maintains a cache). Using RPC is inefficient, made worse by the need to make multiple calls. It also doesn't support queuing by witness id (yet, see #34013).
This PR introduces two new IPC methods:
- `getTransactionsById()`: takes a list of `Txid`'s
- `getTransactionsByWitnessID()`: : takes a list of `Wtxid`'s
Both return a list of serialised transactions. An empty element is returned for transactions that were not found.
Unlike the RPC counterpart, the IPC methods do not take advantage of `-txindex`. This could be done in a followup. For `Wtxid` that would involve adding a `-witnesstxindex`.
I thought about having a single (or overloaded) `getTransactions()` that works with both `Txid` and `Wtxid`, but I prefer that clients are intentional about which one they want.
A unit and functional test cover the new functionality.
Sv2 probably only needs `getTransactionsByWitnessID()`, but it's easy enough to just add both.
To rest with Rust use:
- https://github.com/2140-dev/bitcoin-capnp-types/pull/11
[^0]: there's two reasons the pool requests these transactions: to approve the template and to broadcast the block if a solution is found (the miner will also broadcast via their template provider). See also https://github.com/stratum-mining/sv2-spec/issues/170
ACKs for top commit:
achow101:
ACK 9784818442
sedited:
Re-ACK 9784818442
ViniciusCestarii:
Re-ACK 9784818442
ismaelsadeeq:
Code review ACK 9784818442
Tree-SHA512: 3c6ceb572ab7d8bd090a8f31b5e331304a7a19a3d1f1551c9c2e1ee41339d76f96ca6c41bd634c87fca0a969e7d9bfa6a16c26fb06c0dd2315f6ca1c76a16a31
The server isn't running out of memory when the private broadcast
transaction queue is full. Add a new RPC_LIMIT_EXCEEDED code that
can be used whenever a resource is bound and currently at capacity.
4e29de719e private broadcast: add release note for limited cap (Gregory Sanders)
cbf8c107c1 Release cs_main between individual private tx re-attempts (Greg Sanders)
5aea3d0373 private broadcast: limit outstanding txs to count of 10,000 (Gregory Sanders)
Pull request description:
Add a belt-and-suspenders feature, limit the amount of memory and cpu possible when unlucky or simply misconfigured. The worst case limit is roughly 400kB * 10,000 = 4GB, regardless of usage pattern.
Before this change, sheer volume of broadcasts, mismatches in standardness rules, or simply fee mismatches may result in unbounded growth of memory usage. As the feature may be expanded in the future, explicit bounds helps reasoning going forward.
ACKs for top commit:
frankomosh:
tACK 4e29de719e. Ran private_broadcast_tests and p2p_private_broadcast_cap.py. Great to have an explicit bound as the belt-and-suspenders against unbounded queue growth.
vasild:
ACK 4e29de719e
andrewtoth:
ACK 4e29de719e
stickies-v:
ACK 4e29de719e
Tree-SHA512: 18161755f37d07cca185a09e782dbe2fd0025b8befd4f6660e988865cc3a9b705d41769b816161e8142fe6ce31a56e0288bd78efc25135cedfc47fc855011799
Extend p2p_addr_selfannouncement to restart the node with -onlynet=ipv4
-externalip=<onion> and verify the onion address appears in
localaddresses despite its network being unreachable.
55e3a57f22 qa: Avoid UTXO reuse between test functions (Hodlinator)
9c5dd2926a p2p: Ignore CMPCTBLOCK from peer that hasn't sent SENDCMPCT (David Gumberg)
bf9884f4e5 p2p: make blocksonly nodes ignore CMPCTBLOCK messages (David Gumberg)
92cea63c71 test: (Un)solicited invalid cb -> get disconnected. (David Gumberg)
e845e26344 test: p2p: Nodes ignore unsolicited CMPCTBLOCK's (David Gumberg)
8313591715 p2p: Drop unsolicited CMPCTBLOCK from non-HB peer (David Gumberg)
44f377a71f refactor: test: Static assert_highbandwidth_states (David Gumberg)
25457a3272 test: Tighten getblocktxn checks in parallel cb reconstruction test. (David Gumberg)
51dd90fb50 refactor: Merge announce_cmpct_block() defs into one (Hodlinator)
Pull request description:
Processing unsolicited `CMPCTBLOCK`'s from a peer that has not been marked high bandwidth is not well-specified behavior in BIP-0152, in fact the BIP seems to imply that it is not permitted:
> "[...] method is not useful for compact blocks because `cmpctblock` blocks can be sent unsolicitedly in high-bandwidth mode"
See https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki#separate-version-for-segregated-witness
This PR disables processing of CMPCTBLOCK messages in three cases:
$1$. When the block is unsolicited and from a non-HB peer.
$2$. When this node is running in `-blocksonly` mode.
$3$. When the peer has not advertised `CMPCTBLOCK` support with a `SENDCMPCT` message.
Not processing unsolicited blocks slightly raises the cost of discovering a peer's mempool via `CMPCTBLOCK` as described in #28272. As pointed out there, getting an HB slot is relatively easy, so this does not prevent an attacker from doing this, it just slightly raises the bar.
Probably more important is not processing `CMPCTBLOCK` messages as a `-blocksonly` node. A blocksonly node has a lot less surface area for leaking its mempool since it does no transaction relay, and leaking a blocksonly node's mempool is pretty dangerous since it is very likely to be the origin for all of the transactions in its mempool.
ACKs for top commit:
achow101:
ACK 55e3a57f22
w0xlt:
reACK 55e3a57f22
hodlinator:
re-ACK 55e3a57f22
polespinasa:
lgtm ACK 55e3a57f22
Tree-SHA512: 118bea55adca01dbd6467ba5ae3adf420d960794a6a2c40dd30fcc7d79aa944e01af0f6dd6bd6ff6d33dc9155171f6f4f497cbd7e8eb6d3c4c89e12740b51c05
Feeler connections are short-lived connections made to check that a node
is alive, useful for test-before-evict from addrman, and for moving
addresses from the new to the tried table.
We currently send a GETADDR message to feelers, but then disconnect
before being able to receive a response. This wastes some bandwidth, so
we can avoid sending the GETADDR altogether.
Not sending the initial GETADDR will effectively disable addr relay:
we initialize addr relay for the peer when we send GETADDR, and the peer
initializes addr relay to us when they receive it. So the
peer will not relay any announcement to us, and we will not relay any
to them either. This is ok, since the use of feelers is to test if there
is a bitcoin node behind an address, not exchange addresses with them.
c1313b199f init: wake genesis wait after ImportBlocks() returns (ismaelsadeeq)
Pull request description:
During startup with `-reindex`, the block index and chainstate are wiped, and
the normal `LoadGenesisBlock()` startup path is skipped while block files are
being indexed. The init thread can then wait for genesis to be processed via the
tip-block condition variable.
If shutdown is requested after the init thread enters that wait but before the
import thread activates genesis, the reindex scan can return early without
calling the later `LoadGenesisBlock()` fallback or `ActivateBestChains()`.
Because no block tip is connected, the block-tip notification never fires. The
wait predicate would accept `ShutdownRequested(node)`, but it is not evaluated
again unless the condition variable is notified.
Notify the tip-block condition variable after `ImportBlocks()` returns so the
genesis wait can observe shutdown and exit cleanly.
Described in detail https://github.com/bitcoin/bitcoin/pull/35621#issuecomment-4876059120
The current test covers the path, but won't fail deterministically. You can use L0rinc suggested patch for a deterministic patch that demonstrate the deadlock on master https://github.com/bitcoin/bitcoin/pull/35652#pullrequestreview-4627988553
ACKs for top commit:
maflcko:
review ACK c1313b199f🦁
mzumsande:
Code Review ACK [c1313b1](c1313b199f)
sedited:
tACK c1313b199f
Tree-SHA512: b17faaa2042882f2acebf688f4a28fd6bb11cc5d7f978991fe2bcba7a9df6ed82a0a2811f2e71a929189570a7926f148afdbeef5c1ca57a081e3bb914c2f6fe2
If shutdown interrupts ImportBlocks() before genesis activation,
no blockTip notification is sent. The wait predicate allows
shutdown to occur, but the condition variable is never
notified, so init can remain stuck waiting for genesis activation.
Notify the tip condition variable after ImportBlocks() returns so
interrupted imports wake the wait and let it observe the shutdown request.
Add test coverage for interrupting startup after reindex block files import
begins, which exercises the path where ImportBlocks() can return before genesis
activation.
9a8ef9b0a3 test: SOCKS5 proxy: expect that connection may be reset during handshake (Vasil Dimov)
eb3208364a test: SOCKS5 proxy: expect that connection may be reset when forwarding (Vasil Dimov)
Pull request description:
The `forward_sockets()` function used by the SOCKS5 proxy forwards data between two connected sockets. It might happen that one of those sockets gets closed/reset abruptly, without sending EOF first. This is to be expected if e.g. `bitcoind` is shutdown and shouldn't result in noisy harmless messages like:
```
2026-06-03T13:23:56.966859Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
File ".../socks5.py", line 199, in handle
forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../socks5.py", line 76, in forward_sockets
data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
```
Instead turn this into a debug log message with a nice prefix containing enough information to identify the two forwarded sockets.
---
Also expect that the connection might be closed during the SOCKS5 handshake and only log a debug message if that happens.
ACKs for top commit:
optout21:
crACK 9a8ef9b0a3
danielabrozzoni:
reACK 9a8ef9b0a3
sedited:
ACK 9a8ef9b0a3
Tree-SHA512: 24e25a30529eda3536ebf472f63a93fd80fff46273054a7075490c88737f8870c0141b2bc99d9ef39e6b4f592af2801350fdfbc71927f573738b4a14f5fd7ce0
a15bdc0598 doc: update offline-signing-tutorial to use exportwatchonlywallet rpc (Pol Espinasa)
a388076401 test: Test for exportwatchonlywallet (Ava Chow)
d053e3e5c8 wallet, rpc: Add exportwatchonlywallet RPC (Ava Chow)
444878efef wallet: Add CWallet::ExportWatchOnly (Ava Chow)
f9273f01db wallet: Move listdescriptors retrieving from RPC to CWallet (Ava Chow)
a1c83789a7 wallet: Write new descriptor's cache in AddWalletDescriptor (Ava Chow)
1e996640e6 wallet: Use Descriptor::CanSelfExpand() in CanGetAddresses() (Ava Chow)
d2ee9227da descriptor: Add CanSelfExpand() (Ava Chow)
Pull request description:
Currently, if a user wants to use an airgapped setup, they need to manually create the watchonly wallet that will live on the online node by importing the public descriptors. This PR introduces `exportwatchonlywallet` which will create a wallet file with the public descriptors to avoid exposing the specific internals to the user. Additionally, this RPC will copy any existing labels, transactions, and wallet flags. This ensures that the exported watchonly wallet is almost entirely a copy of the original wallet but without private keys.
ACKs for top commit:
polespinasa:
lgtm ACK a15bdc0598
Sjors:
re-utACK a15bdc0598
pablomartin4btc:
re-ACK [a15bdc0](a15bdc0598)
w0xlt:
lgtm reACK a15bdc0598
Tree-SHA512: cfc59415ad9aa13d1445cf2a85db1c051215496b6edcf5a8db463499b2b51b92ee7bf840b709035dff7635f9d0c533423bceb58c851f220500e1ea254d12f3b8
Introduce a new simple `NodeSigner` wallet replacement that delegates
signing to a specified node via the `signrawtransactionwithkey` RPC,
and plug in that new class to the functional test feature_taproot.py,
so that it can be ran without the Bitcoin Core wallet compiled.
This can be useful for spending funds that are received at the created
address. Preparatory for the next commit, which modifies the taproot
functional test to work without the Bitcoin Core wallet compiled in.
Add a belt-and-suspenders feature, limit the amount of
memory and cpu possible when unlucky or simply misconfigured.
The worst case limit is roughly 400kB * 10,000 = 4GB, regardless
of usage pattern.
Before this change, sheer volume of broadcasts, mismatches in
standardness rules, or simply fee mismatches may result in unbounded
growth of memory usage. As the feature may be expanded in
the future, explicit bounds helps reasoning going forward.
3765b428d1 logging: More fully remove libevent log category (Ryan Ofsky)
Pull request description:
Libevent log category was partially removed in 39e9099da5, and this commit extends that with the following changes:
- Stops showing libevent in the list of supported log categories in `bitcoind -help` and `bitcoin-cli help logging` output.
- Stops returning `"libevent": false` in `logging` RPC output.
It's not good to treat libevent as a supported log category when it can't be enabled and trying to enable it results in warnings.
There's also no need to define an unused LIBEVENT constant value and keep more complicated logic for dealing with deprecated log categories, so this change also simplifies code internally.
ACKs for top commit:
l0rinc:
code review ACK 3765b428d1
pinheadmz:
ACK 3765b428d1
sedited:
ACK 3765b428d1
Tree-SHA512: 09e9514f905bb0a79d870689af491886baaa31fa19f2ad6aef4283fa20c2fa6ce8d384178139227aeeabffabff6e83d254114daaeefbbfe2c6172b9da8871298
Introduce a ThreadPool shared pointer to CoinsViewOverlay. A pool managed
externally can be passed in the constructor.
A global thread pool is used in fuzz harnesses since iterations can happen
faster than the OS can create and tear down thread pools.
This can cause a memory leak when fuzzing.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
8791c4764c test: use ExtendedPrivateKey in wallet_taproot.py (rkrux)
89ceafafb9 test: use ExtendedPrivateKey in wallet_listdescriptors.py (rkrux)
bbfffcab58 test: use ExtendedPrivateKey in wallet_send.py (rkrux)
2ab6e590f7 test: use ExtendedPrivateKey in wallet_keypool.py (rkrux)
9e20118720 test: use ExtendedPrivateKey in wallet_fundrawtransaction.py (rkrux)
06af0cddbb test: use ExtendedPrivateKey in wallet_descriptor.py (rkrux)
4100fac20e test: use ExtendedPrivateKey in wallet_createwallet.py (rkrux)
ff3f6def9a test: use ExtendedPrivateKey in wallet_bumpfee.py (rkrux)
003f2a01f6 test: use ExtendedPrivateKey in feature_notifications.py (rkrux)
f988e6d6e6 test: use ExtendedPrivateKey in wallet_importdescriptors.py (rkrux)
d2a03d50ac test: add extendedkey.py unit tests by using BIP32 test vectors (rkrux)
afdb378082 test: introduce ExtendedPrivateKey and ExtendedPublicKey classes (rkrux)
4dbaa7cc65 test: generalise byte_to_base58 utility function to allow more version types (rkrux)
Pull request description:
Many a times there has been a need to come up with dynamic xprvs and xpubs
in the functional tests, but the lack of code that creates them dynamically has
led to the presence of several hardcoded keys in the testing framework. This
is not developer friendly and not self-documenting, clutters the testing code,
and makes it difficult to update the tests in the future.
This PR introduces two utility classes ExtendedPrivateKey and
ExtendedPublicKey that allows the developer to create them on the fly
to be used in the tests. I have intentionally not introduced any library for this
purpose and have reused the existing libraries and functions in the framework.
The implementation is supposed to provide basic functionality for creating
xprv randomly or from a fixed seed, creating corresponding xpub, and
deriving child xprvs and xpubs at custom derivation paths.
I've updated many tests to show how these can be used, there are more
tests as well that can be updated in the future to completely remove such
non-deterministic hardcoded keys.
ACKs for top commit:
achow101:
ACK 8791c4764c
w0xlt:
ACK 8791c4764c
Tree-SHA512: f8ec4e09eaa6cc44b0f1c9a91337e570b12fb882c258be89b470de1a8cecf9d2fd40d9f02ee739dcbf639462ea7710aa145a3726f0f537f5a1f1e7772e5b019d
9e6546c517 test: raise reindex mining RPC timeout (Lőrinc)
Pull request description:
**Problem:** I often hit a timeout in `feature_reindex.py` when running functional tests locally in parallel in debug mode (especially on battery or in power-saving mode).
**Fix:** Increase the test-local RPC timeout for the reindex mining setup.
ACKs for top commit:
mercie-ux:
ACK 9e6546c517
sedited:
ACK 9e6546c517
Tree-SHA512: d3541dd6752f943921a030393b00e684b3b5d00b93aa0b2b1f85c017698bdfe2216d7f5441bb7dd5f153453385e4df6f9cc8055589e57bca83fa48f0b7de4252
0cdd817a82 add release note (Pol Espinasa)
517d37ce3e test: tests wallet migration with load_wallet disabled (Pol Espinasa)
b98dd63da7 rpc: Add load_wallet argument to migratewallet RPC (Pol Espinasa)
4acd063ba6 wallet: make loading the wallet after migrating optional (Pol Espinasa)
97d08d62ba refactor: store wallet names to MigrationResult (Pol Espinasa)
Pull request description:
This PR is motivated by this [Stack Exchange question](https://bitcoin.stackexchange.com/questions/130713/bitcoin-core-quickest-method-legacy-descriptor-wallet-migration).
Long story short, someone who has a node pruned before his legacy wallet birthday, is unable to migrate the wallet as it is not possible to load it.
Loading is not necessary for migration, and migrating without wanting to use the wallet in that node is a valid use-case.
This PR adds a new RPC argument to `migratewallet` that allow the user disabling the wallet loading.
Second commits adds tests for it.
Follow-up: Add an option to the GUI to not load the wallet after migrating.
ACKs for top commit:
achow101:
ACK 0cdd817a82
w0xlt:
ACK 0cdd817a82
pablomartin4btc:
ACK 0cdd817a82
Tree-SHA512: 8389599e63603b1a532e1bfba0b6c652653386c001f5a881bd49843302b74ff4dbaa4131b5b377c24f483d42e0e70a92b96f760244e3c2e2b44ce08cd04ca1e0
Libevent log category was partially removed in 39e9099da5, and this
commit extends that with the following changes:
- Stops showing libevent in the list of supported log categories in
`bitcoind -help` and `bitcoin-cli help logging` output.
- Stops returning `"libevent": false` in `logging` RPC output.
It's not good to treat libevent as a supported log category when it
can't be enabled and trying to enable it results in warnings.
There's also no need to define an unused LIBEVENT constant value and
keep more complicated logic for dealing with deprecated log categories,
so this change also simplifies code internally.
Co-authored-by: David Gumberg <davidzgumberg@gmail.com>
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
abc33ff043 test: announce field must be 0 or 1 in sendcmpct (brunoerg)
2d0dce0af5 net_processing: fix BIP152 first integer interpretation (brunoerg)
Pull request description:
Fixes#35542
According to the BIP152, the first integer in `sendcmpct` message shall be interpreted as a boolean (and MUST have a value of either 1 or 0). We currently correctly interpret it as boolean, however, we accept any value >=1 and treat it as `true`, deviating from the specification. This PR fixes it.
ACKs for top commit:
edilmedeiros:
utACK abc33ff043
davidgumberg:
crACK abc33ff043 Seems reasonable to comply with BIP152 strictly, test looks good as well.
Sjors:
ACK abc33ff043
jonatack:
re-ACK abc33ff043
achow101:
ACK abc33ff043
w0xlt:
ACK abc33ff043
Tree-SHA512: 77fed86d4de81f7c35ff002b6e1b2a90882ea55f159075da4d34a619d1075f625fca34f929cdd981f1b2eb06f76b64f42cda46502dcd1b4a634c690b0882ec7c
b847626562 test: refresh MiniWallet after node restart (Sjors Provoost)
f4e643cb15 test: merge mining options in package feerate check (Sjors Provoost)
280ce6a0ae miner: ensure block_max_weight is flattened before limit checks (Sjors Provoost)
65bd3164fb mining: clarify test_block_validity comment (Sjors Provoost)
978e7216e6 test: use shared default_ipc_timeout (Sjors Provoost)
Pull request description:
This implement the suggested followups from #33966. Each commit links to the original comment.
The most important change is the extra asserts added in `miner: ensure block_max_weight is flattened before limit checks`.
ACKs for top commit:
achow101:
ACK b847626562
enirox001:
tACK b847626562
sedited:
ACK b847626562
w0xlt:
ACK b847626562
Tree-SHA512: 47678eaed604228269bd892ccf8ff58804745bbc7675b4a93528da9a9292a2eb1e0562cdb8341edac77178563420885b48282bb9e5c2b997b28f2fc64ceeff3d
2fe34808fa wallet: reject sendtoaddress and sendmany for external signers (Sjors Provoost)
bd5a32f7db doc: add taproot descriptor to getdescriptors example (woltx)
7131c82937 doc: clarify which commands receive --chain, --fingerprint and --stdin (woltx)
4fdd4d8d29 doc: replace stale signtransaction wording with current signtx flow (woltx)
fab92257fe doc, rpc: document enumerate model field and fingerprint deduplication (woltx)
Pull request description:
This PR aligns the external signer documentation with current behavior, and makes one previously implicit behavior explicit.
Per review feedback, each commit fixes a limited set of issues:
* **doc, rpc: document enumerate model field and fingerprint deduplication** — the `enumerate` response uses the optional `model` field, which Bitcoin Core maps to the `name` field of the `enumeratesigners` RPC result. Duplicate fingerprints are skipped, and wallet operations require exactly one connected signer.
* **doc: replace stale signtransaction wording with current signtx flow** — spending from an external signer wallet uses `send`/`sendall` (and `bumpfee` for fee-bumping), which invoke `<cmd> --stdin` and pass the `signtx` subcommand and PSBT over stdin.
* **doc: clarify which commands receive --chain, --fingerprint and --stdin** — mark `--chain` and `--fingerprint` as required except for `enumerate`, keep `--stdin` required for protocol flexibility, and match the order and form of the actual invocations in the usage examples.
* **doc: add taproot descriptor to getdescriptors example** — show the BIP86 `tr()` descriptor alongside the other address types.
* **wallet: reject sendtoaddress and sendmany for external signers** — return a specific error instead of the misleading "Private keys are disabled for this wallet", with functional test coverage. Cherry-picked from #33112 (thanks Sjors).
How the documentation went stale:
* The `enumerate` example has shown a `name` field since external signer support landed in #16546, but the implementation has always read `model`.
* `sendtoaddress`/`sendmany` external signer support was effectively precluded by #21201, which was merged a few days before #16546, so the interaction was missed in review and the documented `signtransaction` flow never existed in this form.
* Fingerprint deduplication was added in #35251.
* The documentation was last updated in #33765.
ACKs for top commit:
Sjors:
ACK 2fe34808fa
optout21:
ACK 2fe34808fa
naiyoma:
ACK 2fe34808fa
Tree-SHA512: 86859d2f81ac337f3b4b6578c6ee0151ffb76b8374dfa58e28e00ce4eb69dc200cd6bd2d0a99f73d0475c3824d6ac1cb9e2542b119ca124dd835132dc95cd023
