Introduce WalletError as a generic wallet-layer error type that can carry a machine-readable WalletErrorCode and a translated user-facing message.
The WalletErrorCode::GenericError code is intended for failures that callers should only display to the user. More specific codes should only be added when callers can handle the condition differently.
e9ed898a0d validation: Don't use m_chain.Tip() in FlushStateToDisk (Martin Zumsande)
3679f1ecf5 index: Don't commit ahead of the flushed chainstate (Martin Zumsande)
65735728a5 index: Remove return value from Commit() (Martin Zumsande)
09c06960c6 validation: track last flushed block (Martin Zumsande)
13c02b5466 test: add test for index commits ahead of the last flushed block (Martin Zumsande)
Pull request description:
If indexes commit their data ahead of the flushed chainstate, and there is an unclean shutdown, the index will be corrupted. This is especially the case for the coinstatsindex, which has state (the muhash) which can't easily be rolled back without access to the blocks. This was only partly fixed in #33212 (for reorg scenarios) but could still happen during initial sync.
Fix this more thoroughly by having the node keep track of the last flushed block, and skipping index commits if the current block of the index is not an ancestor of the node's last flushed block (similar to the suggestion by stickies-v in https://github.com/bitcoin/bitcoin/pull/33212#pullrequestreview-31408570890.
Fixes#33208Fixes#34261
ACKs for top commit:
achow101:
ACK e9ed898a0d
sedited:
Re-ACK e9ed898a0d
fjahr:
re-ACK e9ed898a0d
Tree-SHA512: 7f4dc6fb942d6726587eb75dece24c79c679d8630320502aca9fa2d2b03b1d25999cd11255ec20344ebbb8985552747e2554e2557b9d2ad0c75db71652d615ab
In DisconnectBlock(), we can call FlushStateToDisk after updating
the coins but before changing the tip, which is still at the
disconnected block. This means that the ChainStateFlushed
signal would have the wrong block in the locator.
Also remove an outdated comment - the wallet doesn't use
ChainStateFlushed anymore, currently only indexes do.
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
Otherwise, if the node has an unclean restart,
indexes with state (coinstatsindex) couldn't reorg to the
last flushed tip and would be corrupted.
Also updates documentation of Commit() -
the locator functionality isn't used, so the previous text was wrong:
We must have the best block in our block index after a restart.
Co-authored-by: Fabian Jahr <fjahr@protonmail.com>
Assigning `ToIntegral<int64_t>("-1")` to the `optional<uint64_t>` `n`
is a silent underflow. `BOOST_CHECK_EQUAL` then promotes `int` to
`uint64_t`, which also underflows. The correct check is to do this
inline.
1a3cbf1bd2 net: optimize compact block extra tx iteration (Lőrinc)
Pull request description:
**Problem:** `vExtraTxnForCompact` gives compact block reconstruction one more source for recently removed transactions.
Before this PR, the first insertion resized the cache to its configured capacity, so before the cache was full, `PartiallyDownloadedBlock::InitData()` also scanned default `{Wtxid::ZERO, nullptr}` entries that had never been added to the cache.
Those unused entries could still participate in reconstruction short-id matching.
**Fix:** Reserve the configured capacity and append entries until the cache is full.
Once full, keep the same ring-buffer overwrite behavior, configured maximum, and replacement order.
**Risk:** Triggering the affected duplicate-match branch requires a block-specific 6-byte short-id collision while the extra-txn cache still contains default null slots.
With a 16-thread benchmark of the `CBlockHeaderAndShortTxIDs` nonce-grinding path, the 50% collision time was ~178 days on my machine.
ACKs for top commit:
davidgumberg:
crACK 1a3cbf1bd2
darosior:
utACK 1a3cbf1bd2
w0xlt:
ACK 1a3cbf1bd2
sedited:
ACK 1a3cbf1bd2
Tree-SHA512: 9022a2ea5f3ff4279bdee7fa0316d6e5c922be3f7c566743b4d16708c49c0c8dc8a8dcbd60530a0128277db6f192355be88312419952213ae06353a0d937c507
fabafd91f1 refactor: Use u64 over size_t for all cache sizes to fix a 32-bit overflow (MarcoFalke)
Pull request description:
This is a refactor on 64-bit systems, because size_t is equal to u64.
However, on 32-bit systems, it fixes an integer overflow while calculating the cache sizes:
```
src/node/caches.cpp:71:49: runtime error: unsigned integer overflow: 471859200 * 10 cannot be represented in type size_t (aka "unsigned int")
```
This happens while multiplying the default cache size (450MiB) by 10:
```
index_sizes.tx_index = std::min(total_cache * 10 / 100, ...)
^^^^^^^^^^^^^^^^
```
The issue was introduced in commit d06dabf26b.
----
This change follows similar changed one in the past, like 3789215f73, ac76d94117, or 28a523fb94.
Generally, using fixed sized integer types for calculations is beneficial, because all platforms behave exactly the same way. With platform-dependent types there is a risk that the same calculation yields different results. This has several resulting benefits:
* Easier review, because there is no need to review the same code several times for each supported platform.
* Easier quality assurance, because there is less need to run the same code several times in sanitizers for each supported platform, which is [tedious](https://github.com/bitcoin/bitcoin/issues/32375#issuecomment-4825318068).
There are also no downsides, because there is no measurable overhead on 32-bit for u64 calculations that are done only once in the lifetime of the program. Also, there is no measurable memory overhead when a few fields on 32-bit store some extra zero bytes.
----
As said, testing is only possible by picking one of the tedious options:
* Apply a diff on 64-bit arch and compile with `-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DSANITIZERS=integer`
```diff
diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index c98b8ce604..cfd49b60d3 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -58,3 +58,3 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
{
- size_t total_cache{CalculateDbCacheBytes(args)};
+ uint32_t total_cache(CalculateDbCacheBytes(args));
@@ -72,6 +72,6 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
IndexCacheSizes index_sizes;
- index_sizes.tx_index = std::min(total_cache * 10 / 100, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
- index_sizes.txospender_index = std::min(total_cache * 5 / 100, args.GetBoolArg("-txospenderindex", DEFAULT_TXOSPENDERINDEX) ? MAX_TXOSPENDER_INDEX_CACHE : 0);
+ index_sizes.tx_index = std::min<uint32_t>(total_cache * 10 / 100, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0);
+ index_sizes.txospender_index = std::min<uint32_t>(total_cache * 5 / 100, args.GetBoolArg("-txospenderindex", DEFAULT_TXOSPENDERINDEX) ? MAX_TXOSPENDER_INDEX_CACHE : 0);
if (n_indexes > 0) {
- size_t max_cache = std::min(total_cache * 5 / 100, MAX_FILTER_INDEX_CACHE);
+ size_t max_cache = std::min<uint32_t>(total_cache * 5 / 100, MAX_FILTER_INDEX_CACHE);
index_sizes.filter_index = max_cache / n_indexes;
```
This will give a roughly similar error:
```
sh-5.3$ echo 'Bw==' | base64 -d > /tmp/blob
sh-5.3$ UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=block_index_tree ./bld-cmake/bin/fuzz /tmp/blob
./src/node/caches.cpp:73:59: runtime error: unsigned integer overflow: 1073741824 * 10 cannot be represented in type 'uint32_t' (aka 'unsigned int')
#0 0x55ca78da5c47 in node::CalculateCacheSizes(ArgsManager const&, unsigned long) ./src/node/caches.cpp:73:59
```
* Alternatively, to reproduce in a fresh `podman run -it --rm --platform linux/i386 debian:unstable`:
```
export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git ./b-c && cd b-c && apt install build-essential cmake pkg-config python3-zmq libzmq3-dev libevent-dev libboost-dev libsqlite3-dev systemtap-sdt-dev libcapnp-dev capnproto libqrencode-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev clang llvm libc++-dev libc++abi-dev mold -y && cmake -B ./bld-cmake -DAPPEND_CXXFLAGS='-O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=mold -DCMAKE_C_COMPILER='clang;-ftrivial-auto-var-init=pattern' -DCMAKE_CXX_COMPILER='clang++;-ftrivial-auto-var-init=pattern' -DSANITIZERS=address,float-divide-by-zero,integer,undefined --preset=dev-mode && cmake --build ./bld-cmake --parallel $(nproc)
echo 'Bw==' | base64 -d > /tmp/blob
UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=block_index_tree ./bld-cmake/bin/fuzz /tmp/blob
# or:
UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ASAN_OPTIONS="detect_leaks=0" ./bld-cmake/bin/test_bitcoin-qt
```
ACKs for top commit:
l0rinc:
ACK fabafd91f1
sedited:
Re-ACK fabafd91f1
theStack:
re-ACK fabafd91f1
Tree-SHA512: cebbc29636b4074917c96cf3af8fcc176dcd328821d5032dc8609475e18778dfda4e287ded889c024ab29b79d81004bc9a8e57a88eaa0a5eaefe5f8bba1462ab
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
This is a refactor on 64-bit systems, because size_t is equal to u64.
However, on 32-bit systems, it fixes an integer overflow while calculating the cache sizes:
src/node/caches.cpp:71:49: runtime error: unsigned integer overflow: 471859200 * 10 cannot be represented in type size_t (aka "unsigned int")
This happens while multiplying the default cache size (450MiB) by 10:
index_sizes.tx_index = std::min(total_cache * 10 / 100, ...)
^^^^^^^^^^^^^^^^
The issue was introduced in commit d06dabf26b.
====
Also, add missing includes in touched files, according to IWYU.
22ac4ad949 ci: ensure we use correct lld version in OpenBSD job (fanquake)
495f43f7b3 ci: FreeBSD 15.1 (fanquake)
244739db9d depends: move FreeBSD SDK handling to CI (fanquake)
Pull request description:
Followup to https://github.com/bitcoin/bitcoin/pull/35397#discussion_r3493131172, which partially reverts changes from #34491.
Also, make sure we install the correct version of `lld` in OpenBSD job.
ACKs for top commit:
willcl-ark:
ACK 22ac4ad949
Tree-SHA512: 0ce4bdebbeaf5a0c65f2fa4ae177853be246e69a8cab3943cad610a32e990b1d199ae3795d99b889a1d47dbc9c27e0311b026e26ce9f23312581056183dd0a1f
2b6e767d96 doc: archive release notes for v31.1 (fanquake)
Pull request description:
v31.1 has been tagged: https://github.com/bitcoin/bitcoin/releases/tag/v31.1/.
ACKs for top commit:
willcl-ark:
ACK 2b6e767d96
Tree-SHA512: 29e30534e56ffc0ea6ad077bdded05792bcc1ce3dd277332208658d1ae2b8aa9ef30289fed72be25370fc62621e71a0d2da563fe58726541d17dfe5d365a2470
fa615bd163 refactor: Move LoadGenesisBlock to ChainstateManager (MarcoFalke)
Pull request description:
The function does not need anything from any chainstate, so it should not sit in the Chainstate class.
ACKs for top commit:
l0rinc:
Tested ACK fa615bd163
janb84:
reACK fa615bd163
sedited:
ACK fa615bd163
Tree-SHA512: 482b5c140faa35944a890c941fd185a896a3c04fec46d0cc6dd56830f62ee8fe5200fd13d25f7a80a5da0eb4fb42717f42c8bce837d933668432f5786f8380e3
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
31abaa264c doc: add an AI contribution policy (will)
Pull request description:
This policy, adapted from ripgrep, f0cec341ab/AI_POLICY.md who in turn adapted it from uv c5187e200d/AI_POLICY.md, works as a reasonable and pragmatic AI contribution policy at this point in time.
It codifies roughly how the project is currently operating, it's expectations when Ai is being used, and what we don't wish to see.
Link to the document directly from the new PR and issue helptext.
ACKs for top commit:
Sjors:
re-ACK 31abaa264c
achow101:
ACK 31abaa264c
sedited:
Re-ACK 31abaa264c
l0rinc:
ACK 31abaa264c
Tree-SHA512: 667bda2d02717889ee6878438b4e4c7155025ae6933ac748b49f7ca2a04c94515bdd04d522a778828995c3e328780521c1734dd0d8cca4711a702fc9f242756f
The function does not need anything from any chainstate, so it should
not sit in the Chainstate class.
Also, mark it [[nodiscard]], and the one place that ignores the return
value with (void).
Also, change the error log strings to not include the __func__, which is
redundant with -logsourcelocations. This is not a refactor, but this log
is only for debugging extremely rare errors.
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.
68cb7840d2 doc: improve offline-signing-tutorial after 32489 (Pablo Martin)
Pull request description:
General improvements noted in the #32489 review and deferred by the author:
- Remove [a stale NOTE](https://github.com/bitcoin/bitcoin/pull/32489#discussion_r3484961398) referencing `walletcreatefundedpsbt`; the tutorial was updated to use the send RPC instead.
- [Fix](https://github.com/bitcoin/bitcoin/pull/32489#discussion_r3484961536) `listtransactions` example output from `{...}` to `[...]`; the RPC returns a JSON array, not an object.
ACKs for top commit:
polespinasa:
ACK 68cb7840d2
Tree-SHA512: 0615f042a98f68d1a3bd71bf04ad0f66aa88b7011572b58e2b349623da0ac334d5d388caeee16e1bd922650e5aa6a9fa7d9d00da9a70d227dad22a78dd9e6b76
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
a99148d576 test kernel: Don't log on warnings change (sedited)
5b4fd284f4 kernel: Generate a signet with a challenge (sedited)
Pull request description:
Adds a function for creating a chainparams with a signet challenge to the kernel API. This was requested in issue https://github.com/bitcoin/bitcoin/issues/35362.
Also takes this opportunity to de-noise the test kernel binary log output a bit.
ACKs for top commit:
stickies-v:
ACK a99148d576
yuvicc:
lgtm! ACK a99148d576
musaHaruna:
reACK [a99148d](a99148d576)
Tree-SHA512: 44b414e1d43af59080c03940579307c205aa0533138bc94b67a1bfb72d347f3d3c5d7c349f0a4b3983456ee1face42794c1ed46d08154fc6f5b71d3d6915ae98
eccb04a321 refactor: Use `NetworkErrorString` for macOS code in `netif.cpp` (Hennadii Stepanov)
Pull request description:
Although these `sysctl` calls report generic system errors rather than network errors, `NetworkErrorString()` is identical to `SysErrorString()` on POSIX systems, so this does not change behavior. It allows removing `#include <util/syserror.h>`, which was only needed by `__APPLE__` code and would otherwise require an `#ifdef` guard to silence an IWYU unused-include warning on other platforms.
Related to https://github.com/bitcoin/bitcoin/pull/34995.
ACKs for top commit:
sedited:
ACK eccb04a321
Tree-SHA512: 25838dcf206f728fb0968a9ff336863870b69f857436a47b9916269e624d0f157bd566b461911dfea69716de482bbde6903d51bb97e6f70631dfcd274390ac3d
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
`vExtraTxnForCompact` was resized to its configured capacity on the first insertion.
Before the ring was filled, compact block reconstruction scanned default `{Wtxid::ZERO, nullptr}` entries created for unused slots.
Reserve the configured capacity and append entries until the cache is full, then keep the same ring-buffer overwrite behavior.
This preserves the configured maximum and replacement order while keeping reconstruction from scanning unused capacity.
cddbad325d doc: Add release notes for 32489 (exportwatchonlywallet RPC) (Pablo Martin)
Pull request description:
This is a follow-up to #32489.
ACKs for top commit:
polespinasa:
ACK cddbad325d
Tree-SHA512: bfc14c1d8576395caea9636fcc57182c7fe027e397b8529d60901f7310e30acfe9157fbc0e11f6ad8c16c96cb96df9d7d70d0965c5e8a06d6fa3e9e7b5a65533
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.
Although these `sysctl` calls report generic system errors rather
than network errors, `NetworkErrorString()` is identical to
`SysErrorString()` on POSIX systems, so this does not change behavior.
It allows removing `#include <util/syserror.h>`, which was only needed
by `__APPLE__` code and would otherwise require an `#ifdef` guard to
silence an IWYU unused-include warning on other platforms.
256482ab56 validation: In AcceptBlock, ignore flush result (optout)
Pull request description:
Shortcut: see https://github.com/bitcoin/bitcoin/pull/35570#issuecomment-4827437071 and https://github.com/bitcoin/bitcoin/pull/29700#discussion_r3488550312 .
**Summary.** At the end of `ChainstateManager::AcceptBlock`, the flushing of the chain state is initiated, but any potential failure from `FlushStateToDisk` is ignored. However, the error message might get propagated upwards. This change makes sure that both the error status and the error message are ignored, and adds an explanatory comment.
**Related/background.** The potential inconsistency between the return value and state returned from `AcceptBlock` was found during refactoring of the return of `BlockValidationState` in #35570. PR #29700 also touches this part by adding an explanatory comment, see https://github.com/bitcoin/bitcoin/pull/29700#discussion_r3488550312 . Splitting this minor behavior change out of #35570 makes that PR a clean (no-bahavior-change) refactor (this option was mentioned from the start, and also proposed by reviewers; see: https://github.com/bitcoin/bitcoin/pull/35570#issuecomment-4827437071 ).
**Motivation**
- Get rid of the potential inconsistency between the return value (`true`) and state returned (`Error`). This facilitates refactoring of the error returns in #35570.
- Make the ignoring clear and explicit, so there is no doubt about it being intentional or not.
**Details.** `ChainstateManager::AcceptBlock` is called for new block candidates. After checks and inclusion, it triggers flushing of the updated chain state to disk, by calling `Chainstate::FlushStateToDisk`. This method returns a bool and also takes & returns a `BlockValidationState` state. In case of error it returns `false`, and sets the state (to `Error`) and the reject reason string. At the call site, the return value is ignored, but for the state the state variable of `AcceptBlock` is provided, so an Error state may get propagated upwards in the call chain. A caller may react to this Error state.
**Rationale for ignoring the flush error**
The validation code flushes internally in several places, and mostly doesn't treat flush failures as errors returned to callers. Disk errors should never be mistreated as block validation failure. Note that the fatal error notification inside `FlushStateToDisk` still fires, so the node will shut down on unrecoverable flush errors regardless.
**Relevant use case.** `AcceptBlock` is called from the following methods (excluding test code):
- `ChainstateManager::ProcessNewBlock`
- Twice from `ChainstateManager::LoadExternalBlockFile`
Of these, in two places the `state` returned is not used at all. Only at `validation.cpp:5056` is the state used.
```
BlockValidationState state;
if (AcceptBlock(pblock, state, nullptr, true, dbp, nullptr, true)) {
nLoaded++;
}
if (state.IsError()) {
break;
}
```
Here a flush error *may* trigger the `break`, causing an early exit from the loop over blocks in a block file. Post-change the returned state will not be `Error` in a disk error case, so the `break` will not be hit here. However:
- This branch is only called for blocks that are not known yet
- In case of disk error case the fatal error handler will cause the node to shut down in any case.
Based on the above, the change in error return behavior is acceptable.
ACKs for top commit:
maflcko:
lgtm ACK 256482ab56
l0rinc:
lightly tested ACK 256482ab56
dergoegge:
Code review ACK 256482ab56
sedited:
ACK 256482ab56
ismaelsadeeq:
ACK 256482ab56
Tree-SHA512: c72747b5725739117d6061c3b3d5e6910bd06325a072b20708475c8e178a7ad8a6509b30cc8b547c92011a1267e4ad2feef31a992110072e5f37a4ac1f5c79d8
Pure `getifaddrs()`/`freeifaddrs()` calls do not need any type
definitions beyond those provided by `<ifaddrs.h>` itself.
The subsequent `struct ifaddrs` processing in `netif.cpp` does not
involve any symbols from `<sys/types.h>`.
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
b2de59d486 wallet, bdbro: Validate btree page levels (Ava Chow)
dc3a2b9c3b wallet, bdbro: Enforce overflow data lengths (Ava Chow)
Pull request description:
Alternative to #34946
BDB's overflow records include the total length of the data to be read from the overflow pages. If this length is impossible (larger than max page * page size), or if the data that we are reading exceeds the stated length, then throw an exception as this is an invalid BDB file. This prevents infinite looping if an overflow page makes a circular reference.
BDB BTrees also include the level in the tree that the page is supposed to be at. Leaf pages are always at level 1. Starting from the root page, we can validate that each child has a level one less than the parent, until we reach a leaf page with a level of 1. This also ensures that we cannot have circular internal page references.
ACKs for top commit:
sedited:
ACK b2de59d486
rkrux:
code review ACK b2de59d486
Tree-SHA512: b740c349f68c78150ee57b77e8e81605bb45a7db4fbca3c3a0d87318178dfa197f37faf0cbfd8bef791ab20f4a9afc3d2571842960055a70b6c081d605f59f94
