Instead of replacing the tx when a witness alternative appears, add it
to the set of wtxid alternates.
In order to determine whether the added transaction is the canonical
transaction, Update also needs to know how the state is changing, so it
will also update the state if it is being changed.
A transaction can have several valid witnesses that share its txid but
differ in wtxid, e.g. when a taproot output is spent via the key path
in one variant and the script path in another.
CWalletTx now keeps all of them in a map indexed by wtxid (m_txs) and
marks one as canonical (m_canonical_wtxid): a confirmed variant if there
is one, otherwise the one with a witness and the lowest weight. GetTx()
and serialization return the canonical variant, so existing callers
don't need to change.
The other variants are stored in their own wtxvariant records keyed by
(txid, wtxid) and merged back into the CWalletTx at load. The tx record
keeps its old format, holding the canonical transaction, so older soft
versions can still read and rewrite it without dropping those records.
When CWalletTx will have multiple transactions, tx will no longer exist
and accessing the single canonical tx should be done through an getter
function.
When loading a transaction, instead of constructing a CWalletTx with no
transaction, pass the DataStream into the constructor so that the
CWalletTx is RAII. This allows us to ensure that the transaction is
never a nullptr so that dereferences, especially once multiple txs are
stored, will not cause a segfault.
db35b9238f ipc # build: Fix fuzz target CMakeLists.txt for external libmultiprocess (Ryan Ofsky)
Pull request description:
CMake `WITH_EXTERNAL_LIBMULTIPROCESS` and `BUILD_FUZZ_BINARY` options stopped working together recently due to #35118 commit 037ad77071 because an non-namespaced `Libmultiprocess::multiprocess` target name was referenced.
Fix by specifying the full target name which is better for readability anyway.
ACKs for top commit:
sedited:
ACK db35b9238f
hebasto:
ACK db35b9238f, I have reviewed the code and it looks OK.
Tree-SHA512: 7cfb19a66dd4ccdcd6dfdedc3000fd20f488851f3235852717fd31698a7e8efb5c4e14705b0dc0c1ea506b38268f1ab030bca7865ddfdd85144598f435581247
fed3cf6f0e wallet: Replace CWalletTx's vOrderForm with specific fields (Ava Chow)
4f8823e8e1 wallet: Drop vOrderForm from CommitTransaction (Ava Chow)
a2b0bfcd85 wallet: Drop mapValue from CWalletTx (Ava Chow)
cb99864c91 wallet: Throw if unknown entry is found in mapValue (Ava Chow)
98d5cdae66 wallet: Make CWalletTx "replaces_txid" and "replaced_by_txid" member variables (Ava Chow)
7ef8a6efc2 wallet: Make CWalletTx "comment" and "to" member variables (Ava Chow)
2155e913d3 wallet: Make CWalletTx "from" and "message" member variables (Ava Chow)
c6ba98dcc8 wallet: Drop mapValue from CommitTransaction (Ava Chow)
00abb174a8 wallet: Pass comment and comment_to to CommitTransaction (Ava Chow)
1a219a37a2 wallet: Pass replaces_txid to CommitTransaction outside of mapValue (Ava Chow)
Pull request description:
`mapValue` and `vOrderForm` are opaque data structures that contain transaction metadata. It is hard to determine what actual data each field contains, and they can ostensibly be misused where metadata is added in the future without developers realizing that such metadata exists.
It's much clearer to have all of that metadata live in their own explicit member variables within `CWalletTx`. This PR implements that change.
Since the serialization format of `CWalletTx` depends on `mapValue` and `vOrderForm`, the serialization remains unchanged, so when serializing these new members, they need to be shoved/extracted from a temporary `mapValue` or `vOrderForm`.
This does end up breaking forwards compatibility as unknown fields in `mapValue` and `vOrderForm` are stripped out if the record is rewritten. However, I don't expect that we would continue to use these fields for future metadata, so I think that risk is low.
ACKs for top commit:
ajtowns:
reACK fed3cf6f0e
w0xlt:
ACK fed3cf6f0e with above nits/caveats.
Eunovo:
ACK fed3cf6f0e
Tree-SHA512: c7deab5aaeac13656012f8b13c0161fd420d2a5348eebd7649310e78ccb1216995aa6a7cbd506ac8d11d7b46b0856d6e6a897bc39965b51cfcf2268356ace261
a8223bb4e6 wallet: Introduce WalletError with machine-readable error code (pseudoramdom)
Pull request description:
Per discussion in https://github.com/bitcoin/bitcoin/pull/35436#issuecomment-4923786884, `WalletError` is split out so that it can be reused by multiple wallet interface changes (#34861 in particular)
----
Introduce a `wallet::WalletError`, a generic wallet-layer error type that contains
- a machine-readable `WalletErrorCode` for programmatic handling
- a translated user-facing `bilingual_str` message
The initial enum is intentionally small. `WALLET_ERROR` is used for generic failures that callers should display to the user. The intention is to have more specific codes only when the callers can handle the condition differently.
ACKs for top commit:
achow101:
ACK a8223bb4e6
davidgumberg:
ACK a8223bb4e6
polespinasa:
ACK a8223bb4e6
Tree-SHA512: 15fedf96cb5c3e8167a236bfa1a4d01d94f258a22943e53330ab432ad04a31f81f2eac8749f38390b2d34156d283d3bf76f293c640724bd0b090dc494123b230
2bab6bc73f refactor: Drop support for FreeBSD < 14 (Hennadii Stepanov)
91b5c8a07c refactor: Remove FreeBSD-specific workaround (Hennadii Stepanov)
56701ff6d5 doc: Clarify supported *BSD releases (Hennadii Stepanov)
Pull request description:
This PR establishes a baseline for the oldest *BSD releases supported by Bitcoin Core. Clarifying these minimum requirements paves the way for dropping compatibility code and workarounds for unsupported versions.
The obsolete FreeBSD-specific workaround and version check have been dropped.
ACKs for top commit:
maflcko:
lgtm ACK 2bab6bc73f
willcl-ark:
ACK 2bab6bc73f
theStack:
lgtm ACK 2bab6bc73f
sedited:
ACK 2bab6bc73f
Tree-SHA512: 6d9ca0ff881a60c33fe3aa18a03726426f07f2896b2f56b12804865acfa910aca7efdc1312eb4055e35aab8423d0c2326b89c1da448e01b4fa213f73dfd2b118
a2e4cd7ad2 depends: capnp 1.5.0 (fanquake)
Pull request description:
Update capnp in depends to [`1.5.0`](https://github.com/capnproto/capnproto/releases/tag/v1.5.0), which contains numerous security and bugfixes. More details here: https://github.com/capnproto/capnproto/blob/v2/security-advisories/2026-07-09-capnproto-v1.5-rollup.md:
> Like many projects, in recent months, Cap'n Proto has experienced an uptick in security reports, almost certainly driven by the use of AI to find vulnerabilities.
> In the past, I have always issued a separate security advisory with a CVE for each and every bug. However, this is time-consuming, and I just don't have the bandwidth to keep up. Therefore, given the quantity of bugs reported, I have opted to issue a single combined security advisory for this release.
>According to CVE rules, each distinct bug must have a separate CVE. Roll-ups are not allowed. Unfortunately, this means I cannot request a CVE for this advisory.
See https://github.com/capnproto/capnproto/compare/release-1.4.0...release-1.5.0 for all changes since 1.4.0.
ACKs for top commit:
hebasto:
ACK a2e4cd7ad2.
janb84:
ACK a2e4cd7ad2
Tree-SHA512: e616ab88a7692f17611865fbb36fc0c598ff6d8bcf18035f22d84681bbda8b6f5e7b8d7270baeeaf0d18e035de474003345f3bd75f020e615388ae04b3dd22f4
7508ac319d bench: replace CreateMockableWalletDatabase with MakeInMemoryWalletDatabase (Pablo Martin)
Pull request description:
Benchmarks don't need mock-specific behaviour (overridden `Filename()`, `Format()`, or the exposed batch-level `WriteKey()`). Replace `CreateMockableWalletDatabase()` with `MakeInMemoryWalletDatabase()` across all 6 call sites in `src/bench/` (5 files - 4 wallet bench files + 1 in coin_selection), using the same in-memory SQLite path that production code uses.
`wallet_migration.cpp` is excluded: it calls `GetOrCreateLegacyDataSPKM()` which asserts `Format() == "sqlite-mock"`, a deliberate signal that allows legacy SPKM setup in `test/bench` contexts. `MockableSQLiteDatabase` is still correct there.
For `coin_selection.cpp`, which had no other dependencies on `wallet/test/util.h`, the include is switched to `<wallet/sqlite.h>`. The remaining 4 files retain `wallet/test/util.h` for other utilities but also add an explicit `<wallet/sqlite.h>` include as required by IWYU.
Follow-up suggested in #35655.
ACKs for top commit:
sedited:
ACK 7508ac319d
janb84:
ACK 7508ac319d
Tree-SHA512: 6f4086eb5700ba3da882378dcee76b3c075670cdea5d71b45c9b369e48c72b8f591e0c5a6bd4504719dd512b3b7d2b23230a8b76e471c6c0571c13325fa3cba0
**What failed:** Build error: 'mp/proxy-io.h' file not found when
building the ipc fuzz target.
**Root cause:** Upstream commit 037ad77071 (fuzz: add IPC round-trip
target) added target_link_libraries(fuzz bitcoin_ipc_fuzz multiprocess)
using the bare 'multiprocess' target, which only exists in subtree
builds. With WITH_EXTERNAL_LIBMULTIPROCESS=ON, no 'multiprocess' cmake
target is defined, so the INTERFACE_INCLUDE_DIRECTORIES from the
installed libmultiprocess headers were never propagated to the fuzz
target, causing the missing-header error.
**Fix:** Changed to 'Libmultiprocess::multiprocess', which is the
canonical namespaced target available in both subtree and external builds.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fad5809cb9 doc: Update enum class constant naming style guide (MarcoFalke)
Pull request description:
Lately, it seems there are frequent scripted-diffs and refactors to rename ALL_CAPS enum class constant names to something else, due to third-party macro clashes. E.g.:
* https://github.com/bitcoin/bitcoin/pull/35588
* https://github.com/bitcoin/bitcoin/pull/35487
* https://github.com/bitcoin/bitcoin/pull/34454
* etc... (not listing the intermittent pull request force pushes that lead to early CI failures due to macro clashes)
Try to steer away from ALL_CAPS here by discouraging it in new code.
ACKs for top commit:
kevkevinpal:
ACK [fad5809](fad5809cb9)
hebasto:
ACK fad5809cb9.
pablomartin4btc:
ACK fad5809cb9
stickies-v:
ACK fad5809cb9
yuvicc:
ACK fad5809cb9
musaHaruna:
ACK [fad5809](fad5809cb9)
janb84:
ACK fad5809cb9
Tree-SHA512: f652c0127022a5ea131e956aef0a2d8c98f4c4317519475a38e5f527a5f672b93a8e51743c1a588f38ddc795531168b68ab68f86471b61f25229761eff0f3879
Benchmarks don't need mock-specific behaviour (overridden Filename(),
Format(), or the exposed batch-level WriteKey()). Replace
CreateMockableWalletDatabase() with MakeInMemoryWalletDatabase() across
6 call sites in src/bench/ (5 files), using the same in-memory SQLite
path that production code uses.
wallet_migration.cpp is excluded: it calls GetOrCreateLegacyDataSPKM()
which asserts Format() == "sqlite-mock", a signal used to allow legacy
SPKM setup in test/bench contexts. MockableSQLiteDatabase is still
correct there.
For coin_selection.cpp, which had no other dependencies on
wallet/test/util.h, also switch the include to <wallet/sqlite.h>.
Follow-up suggested in #35655.
63c5f9d22c test: Remove `mock_process.cpp` (rustaceanrob)
Pull request description:
Picked from #35587, but I think has motivation to go in on its own.
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
ACKs for top commit:
kevkevinpal:
ACK [63c5f9d](63c5f9d22c)
maflcko:
review ACK 63c5f9d22c🧀
hebasto:
ACK 63c5f9d22c.
Tree-SHA512: 15ff5ad49256149bb419beb72f002dd55fc62139e61fd80d2cecceff2699f291a5c14b39b027c412352aa965ec320bac320be32170e6d9d944b43e14f4864c35
6d0ea4cf5b doc: add release notes (Andrew Toth)
a2b1c86903 txospenderindex: disable bloom filters to optimize disk usage (Andrew Toth)
Pull request description:
LevelDB bloom filters are only consulted on `Get` point reads. This can be verified in https://github.com/bitcoin/bitcoin/blob/master/src/leveldb/table/table.cc#L224-L228. `InternalGet` is the only place that consults the filter, and it is only reached via a `Get` or `Exists` point read. The filters are never consulted for iterator seeks with an iterator created via `NewIterator`.
txospenderindex only reads via iterator seeks, so building them is wasted effort and space.
For a db as large as txospenderindex, this results in measurable performance and disk usage.
On master, a full sync took 4h37m, and the resulting db was 85.0 GiB.
On this branch, a full sync took 3h57m, and the resulting db was 80.9 GiB.
So this is a sync speedup of 39 minutes (1.17x), and a disk space reduction of 4.2 GiB.
ACKs for top commit:
l0rinc:
ACK 6d0ea4cf5b
sedited:
Re-ACK 6d0ea4cf5b
fjahr:
Code review ACK 6d0ea4cf5b
Tree-SHA512: fb88b9f9a16ff31562d388e3fd9fd9590c7864dbe6093cd9430ecbce9cdc3f2a8d3fc612aade743d26ad4c6eca1e5dc9b3f1ca28d75caea1209e5c784895405d
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
777d23f25c test: add regression test for in-memory SQLiteDatabase reopen (Pablo Martin)
d1e7f8c986 wallet: use in-memory SQLite for temporary wallet in exportwatchonlywallet (Pablo Martin)
ee43743f12 wallet: store m_additional_flags in SQLiteDatabase to fix reopen path (Pablo Martin)
Pull request description:
Since #33032 landed (in-memory `SQLiteDatabase` via `SQLITE_OPEN_MEMORY`), the intermediate wallet built during `exportwatchonlywallet` can live entirely in memory instead of being written to the wallets directory as a temporary file.
The temp wallet is a pure build artifact: it is populated with descriptors, transactions, and address book data, then immediately discarded once `BackupWallet()` copies its contents to the destination file. Making it in-memory removes all on-disk footprint and eliminates the `cleanup_watchonly_wallet` RAII handler — along with the `wallet_path` and `cleanup_files` variables it needed — which previously ensured the temp files were deleted on both success and failure paths.
This PR introduces `InMemoryWalletDatabase` (a minimal `SQLiteDatabase` subclass) and `MakeInMemoryWalletDatabase()` factory in `sqlite.h/cpp`, following the same pattern as `MockableSQLiteDatabase` / `CreateMockableWalletDatabase()`. `MockableSQLiteDatabase` now derives from `InMemoryWalletDatabase`, removing its redundant `Files()` override.
Suggested by Sjors in #32489 ([comment](https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-4874894955)).
---
Also fixes a related issue found (by Sjors) during review:
- `SQLiteDatabase::Open()` (the no-arg public override) hardcoded 0 as `additional_flags` when reopening after a failed `TxnAbort()`, which would reopen an in-memory database as on-disk. Fixed by storing `m_additional_flags` in the constructor and using it in the reopen path. For in-memory databases, both the `force_conn_refresh` path and the public `Open()` now throw instead of silently creating a fresh empty connection. A regression test for the `Open()` throw is included in a separate commit.
---
As a follow-up, `InMemoryWalletDatabase` could replace `MockableSQLiteDatabase` in `src/bench/` (5 files, 6 call sites), since benchmarks don't need mock-specific behaviour and benefit from using the same in-memory path as production code.
ACKs for top commit:
Sjors:
re-utACK 777d23f25c
achow101:
ACK 777d23f25c
janb84:
ACK 777d23f25c
Tree-SHA512: 71178ce99c7ebc0fc5ba17956c27d37f90e3d36cefbdc0d15d1424b7a70bf15f05a13cb03c268d885678aba1fd3c90567d8389d3680650c8ae46f4cb4b10b28a
c43b7a1115 ci: add netBSD cross CI job (fanquake)
699c21aea4 depends: add netbsd_LDFLAGS (fanquake)
Pull request description:
This adds a Linux cross job for NetBSD; similar to https://github.com/bitcoin/bitcoin/pull/34491 (FreeBSD).
This uses the upcoming NetBSD 11.0 release (rc5).
ACKs for top commit:
willcl-ark:
ACK c43b7a1115
Tree-SHA512: 717b5c68e9cf4dd2b6f9101320ebe94df5047745445c2698005d0dfa034776c2fa836b599155c7cb7e93024b15ba7b91cef07275185cd1c07f58fea678e2354d
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
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
