629df81e4c Remove myself as security contact (Pieter Wuille)
Pull request description:
Making the same change here as in https://github.com/bitcoin-core/bitcoincore.org/pull/1264. I remain involved in security discussions, but don't feel like functioning as a first-line contact anymore.
ACKs for top commit:
achow101:
ACK 629df81e4c
fanquake:
ACK 629df81e4c
Tree-SHA512: b76813b7e9679cee60a9dd1e8d8de46cd8e329314c677fd4834bde415f8b1f351ebee4eb4c885e889b8a111e8d46d2d20af6d5a4ff108c64df29874511318c2f
d908063980 chainparams: delete my DNS seed (Pieter Wuille)
Pull request description:
I plan to shut down my DNS seed server in the future (probably after the release of 32.0), so I request it be removed from the master branch already. It has existed for almost 15 years at this point, and isn't getting much maintenance attention from me anymore.
ACKs for top commit:
l0rinc:
ACK d908063980
Sjors:
ACK d908063980
pinheadmz:
ACK d908063980
janb84:
ACK d908063980
Tree-SHA512: b3f5f0361d00557b94987a36a8557cf5c6f0ad1415d5ae6b70d64bce55f45f0a3d4c137b9509270128b1ef97b5ef0a3c67e4bbe6c669d5e4a79bd787cc7801c7
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
The oversized `-dbcache` warning currently switches from a fixed `450 MiB` threshold below `2 GiB` of RAM to `75%` of total RAM at `2 GiB`.
This creates a cliff where a small increase in RAM can raise the warning threshold to about `1536 MiB`.
Apply the `75%` factor only to RAM above a `2 GiB` reserve while keeping `DEFAULT_DB_CACHE` as the minimum threshold.
This removes the cliff: the threshold stays at the default until the percentage term exceeds it, then grows by `0.75 MiB` per additional MiB of RAM.
This also aligns better with the recently merged parallel input prevout fetcher which performs better with slightly lower dbcache memory.
Co-authored-by: Bortlesboat <Bortlesboat@users.noreply.github.com>
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
InMemoryWalletDatabase::Open() now throws to prevent silently returning
a fresh empty connection after close, which would discard all data. Add a
test to pin this behaviour.
Co-authored-by: Jan B <608446+janb84@users.noreply.github.com>
The intermediate watchonly wallet created during exportwatchonlywallet is
a pure build artifact — it is always discarded once BackupWallet() copies
it to the destination. Creating it as an in-memory SQLiteDatabase
(SQLITE_OPEN_MEMORY) removes the need to write files to the wallets
directory and eliminates the cleanup handler that deleted those files on
both success and failure paths.
Introduces InMemoryWalletDatabase (a minimal SQLiteDatabase subclass) and
MakeInMemoryWalletDatabase() factory in sqlite.h/cpp, following the same
pattern as MockableSQLiteDatabase / CreateMockableWalletDatabase() in the
test utilities. MockableSQLiteDatabase now derives from InMemoryWalletDatabase,
removing its redundant Files() override.
The wallet is named after the source wallet ("<name>_watchonly_temp") so
concurrent exports of different wallets use distinct names and log lines
remain traceable to the source wallet.
SQLiteDatabase::Open() (the public override) always reopens the database
with no additional flags. If SQLiteBatch::Close() triggers the
force_conn_refresh path (TxnAbort failed), it calls Open() which drops
the original additional_flags, causing in-memory databases to be reopened
as on-disk instead.
Store additional_flags as a member and use it in Open() so the reconnect
preserves the original flags. For in-memory databases, connection recovery
makes no sense as all data would be lost; both the force_conn_refresh path
and the public Open() now throw instead.
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
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
Add a fuzz harness targeting ConnectionType::PRIVATE_BROADCAST.
Seeds m_tx_for_private_broadcast via InitiateTxBroadcastPrivate
so PushPrivateBroadcastTx reaches the send-INV and other paths.
Guarantees one PRIVATE_BROADCAST peer per iteration, optionally
adds peers of other types, uses CallOneOf() branching between
guided and arbitrary message types, and verifies the outbound
INV is well-formed after the handshake completes.
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
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
Extend the `getrawtransaction` RPC to include a new field `vsize_adjusted` when the transaction is in the mempool.
The `vsize_adjusted` field provides the mempool's accounting size for the transaction based on its sigop cost,
which can exceed its serialized vsize under `-bytespersigop` policies.
Test coverage is added to verify the correct calculation and exposure of the `vsize_adjusted` field via `mempool_sigoplimit.py`.
This commit adds a new `vsize_adjusted` and `vsize_bip141` field to mempool acceptance and submission RPCs,
including `testmempoolaccept` and `submitpackage`,
to report the sigop-adjusted virtual transaction size and virtual transaction size as defined in BIP 141 respectively.
While `vsize` is now marked as deprecated.
RPC help texts are updated to reflect this addition.
Tests in `mempool_accept.py, mempool_accept, p2p_segwit, rpc_packages, mempool_sigoplimit` are extended
to verify the presence and correctness of the new fields.
Co-authored-by: Gloria Zhao <gloriajzhao@gmail.com>
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.
ChooseSelectionResult computes the bump-fee discount as: summed_bump_fees - combined_bump_fee
Where summed_bump_fees is the sum of per-UTXO ancestor bump fees and combined_bump_fee is the
true combined cost taking into account shared ancestors.
Both variables use creates a fresh MiniMiner snapshot of the mempool. Because of that
the two snapshots of the mempool might be different. An artificial feerate decrease
of an ancestor using prioritizesettransaction can make combined_bump_fee > summed_bump_fees.
This cause calling bumpfeediscount with a negative vaule triggering an assertion >= 0.
This commit fixes this by only calling bumpfeediscount when the discount is strictly positive.
Co-authored-by: dergoegge <n.goeggi@gmail.com>
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
