#27865 wallet: Track no-longer-spendable TXOs separately

ReviewabilityReviewsAgreementSize
#27865 achow101 wallet: Track no-longer-spendable TXOs separatelyNeeds rebase0 (+1)Mild389 + 160 tests
  • Speed up balance calculation and coin selection for active wallets with extensive transaction histories
  • Avoid repeatedly scanning outputs that are already spent or conflicted
  • Worth reviewing now because the rebase conflict with master is mechanical
  • Broad support for massive performance improvements on large wallets (remyers, w0xlt, Zeegaths)
  • Concept approval as a natural follow-up or pending performance verification (murchandamus, jonatack, rkrux)
  • Unaddressed objections: reorg balance omissions, assertion aborts, and db writes on load failure (jeanpablojp)
  • 389 lines added or modified outside tests
  • 160 lines added or modified in tests
  • 162 lines removed in total
  • 18 files, 19 commits
Wallet#26 of 92
P2 · speedup
  • P2 because it cuts balance checking and coin selection overhead by orders of magnitude for active wallets
  • Synthetic benchmarks demonstrate up to 500x speedups by excluding spent historical outputs
  • Ranking pass: Moved down: large speedup, but it is the tip of a chain whose bases (#33033, #33034) must be reviewed first.
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

This change separates definitely unusable transaction outputs into a distinct container rather than keeping them in the primary in-memory TXO set. It also embeds key parent transaction metadata directly into WalletTXO objects so balance calculation and coin selection do not need to look up parent CWalletTx records. State transitions, wallet loading, and block disconnection routines are updated to maintain both sets and sync TXO states.

Problem: After PR 27286 introduced in-memory tracking of all wallet TXOs, balance checking and coin selection iteration must traverse all historical outputs, even those spent long ago. For active wallets with substantial transaction histories, iterating through large numbers of confirmed-spent or conflicted outputs causes significant performance degradation.

Evidence of importance:

  • PR description: "This TXO set is iterated for balance checking and coin selection preparation, which can still be slow for wallets that have had a lot of activity. This PR aims to improve the performance of such wallets by moving UTXOs that are definitely no longer spendable to a different map in the wallet so that far fewer TXOs need to be iterated..."
  • Follows up on merged PR #27286 ('wallet: Keep track of the wallet\'s own transaction outputs in memory') and relates to PR #34360 which adds a 'WalletBalanceManySpent' benchmark for high-history wallet scenarios.

Scope: Modifies CWalletTx serialization format to persist m_from_me on disk, touches wallet loading logic (PruneSpentTXOs, ReorderTransactions), and adapts multiple functional tests for backward compatibility and double-spend tracking.

Reviewability

Ready · Needs rebase

Conflicts with master; review the logic now, the rebase is mechanical.

Agreement

Mild

objection that needs an answer has none yet (jeanpablojp)

Thread notes: Initial record for PR 27865. Several state cache synchronization bugs raised by w0xlt were resolved, while four correctness and safety issues identified by jeanpablojp remain open without author replies.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
w0xlt (contributor)objection, correctnessPopulated optional bool evaluated to true in CachedTxIsTrusted check even when value was false, treating untrusted zero-conf change as trustedfixed · fix pushed · v1, not yet re-checkedansweredyes2026-05-22: 'm_from_me = false passes because a populated optional converts to true'
Status from: 2026-05-26: achow101: 'Fixed. The unit test is not meaningfully helpful.'
w0xlt (contributor)objection, correctnessRemoveTxs does not synchronize m_txos and m_unusable_txos cache sets when deleting wallet transactionsfixed · fix pushed · v1, not yet re-checkedansweredyes2026-05-23: 'CWallet::RemoveTxs() deletes wallet transactions, but it was not keeping the new TXO caches (m_txos and m_unusable_txos) in sync.'
Status from: 2026-07-01: achow101: 'Checking the states to revert them was starting to get a bit complicated, so I ended up doing the overkill thing of recomputing all of the TXOs after removing any transaction.'
w0xlt (contributor)objection, correctnessTXOs are refreshed during wallet migration before m_from_me is updated, leaving them with stale legacy valuesfixed · fix pushed · v1, not yet re-checkedansweredyes2026-05-23: 'RefreshTXOsFromTx(*wtx) creates or updates WalletTXO entries using the current wtx->m_from_me value.'
Status from: 2026-05-26: achow101: 'Fixed, but the issue is not reachable and the unit test is not meaningfully helpful.'
w0xlt (contributor)objection, correctnessAddToWallet marks inputs of an inactive transaction usable without checking whether another confirmed tx spends them, causing assertion aborts in GetBalancefixed · fix pushed · v1, not yet re-checkedansweredyes2026-07-02: 'AddToWallet()'s isInactive() branch marks the inputs of an inactive tx usable again without checking whether another confirmed tx still spends them'
Status from: 2026-07-02: achow101: 'I've changed MarkTXOUsable to always do an IsSpent check.'
jeanpablojp (contributor)objection, correctnessOutputs unconfirmed by reorg and abandoned remain missing from balance and listunspent until wallet reload due to default min_depth in IsSpent checkopen · v1, not yet re-checkedansweredno2026-08-27: 'min_depth left at the default instead of the 1 that matches m_unusable_txos's definition.'
jeanpablojp (contributor)objection, correctnessBlock-conflicted outputs can trigger an assertion failure in getbalances after removeprunedfunds due to missing state check during TXO reconstructionopen · v1, not yet re-checkedansweredno2026-08-27: 'This is missing the TxStateBlockConflicted check that PruneSpentTXOs does.'
jeanpablojp (contributor)objection, safetyReorderTransactions and m_from_me upgrades run before LoadWallet confirms load success, rewriting intact records even when loading a corrupted walletopen · v1, not yet re-checkedansweredno2026-08-27: 'ReorderTransactions used to sit after LoadWallet's if (result != DBErrors::LOAD_OK) return result;, and now both it and the new upgrade loop run before that return.'
jeanpablojp (contributor)objection, correctnessUninitialized m_from_me optional in CWalletTx triggers libc++ hardening assertion aborts and undefined behavior during fuzz testingopen · v1, not yet re-checkedansweredno2026-08-27: 'm_from_me is an optional<bool> and CWalletTx(tx, state) doesn't set it.'

Support:

  • remyers (contributor) (Concept ACK, 2023-09-12): on a large synthetic test wallet I'm seeing a 250x speed up.
  • murchandamus (member) (Concept ACK, 2023-12-28): Concept ACK [not substantive]
  • jonatack (member) (Concept ACK, 2024-11-26): pending https://github.com/bitcoin/bitcoin/pull/27286, if this improves performance for wallets with many transactions.
  • Zeegaths (none) (2025-08-01): TXO separation optimization shows its intended benefit in listunspent (31% improvement)
  • rkrux (member) (Concept ACK, 2025-10-08): Seems like a natural step after #27286
  • w0xlt (contributor) (2026-01-21): PR #27865 (with optimization) ... ~508x faster
  • w0xlt (contributor) (ACK, 2026-01-21): ACK https://github.com/bitcoin/bitcoin/pull/27865/commits/52b677d4550c9d5e32e7e3cb5e9fe76eebcea7b3 [not substantive]
  • w0xlt (contributor) (ACK, 2026-03-25): reACK 412008bcb454b8d02f9e00f7e7a0fbb2aae5f690 [not substantive]
  • jeanpablojp (contributor) (Concept ACK, 2026-08-27): Built this and the base and ran the wallet tests.

Participants:

  • t-bast (contributor, neutral, 1 statements 2023-09-11 to 2023-09-11): routed PR to remyers for testing on large wallets
  • remyers (contributor, support, 2 statements 2023-09-12 to 2023-09-13): Concept ACK with 250x speedup benchmark on large synthetic test wallet
  • murchandamus (member, support, 1 statements 2023-12-28 to 2023-12-28): Concept ACK
  • jonatack (member, support, 1 statements 2024-11-26 to 2024-11-26): Concept ACK if it improves performance for wallets with many transactions
  • maflcko (member, neutral, 1 statements 2025-06-02 to 2025-06-02): grammar nit on test comment
  • w0xlt (contributor, objection, 18 statements 2025-07-03 to 2026-07-02): benchmarked 508x speedup and ACKed, but found and helped fix multiple state synchronization bugs
  • Zeegaths (none, support, 1 statements 2025-08-01 to 2025-08-01): benchmarked a 31% speedup in listunspent on high-activity regtest setup
  • rkrux (member, support, 1 statements 2025-10-08 to 2025-10-08): Concept ACK as a natural follow-up to PR 27286
  • brunoerg (member, neutral, 4 statements 2026-01-21 to 2026-01-21): ran mutation testing and reported unkilled mutants
  • jeanpablojp (contributor, objection, 5 statements 2026-08-27 to 2026-08-27): Concept ACK, but reported reorg balance loss, prune funds assert, db rewrite on load error, and fuzz crashes

Files

181 lines under test/bench/ci.

  • src/wallet/wallet.cpp +230/-51
  • src/wallet/walletdb.cpp +39/-22
  • src/wallet/receive.cpp +27/-29
  • test/functional/wallet_importprunedfunds.py +53/-0
  • src/wallet/transaction.h +37/-12
  • test/functional/wallet_balance.py +32/-0
  • src/wallet/wallet.h +21/-6
  • src/wallet/transaction.cpp +19/-7
  • src/wallet/test/util.cpp +13/-12
  • src/wallet/spend.cpp +13/-11
  • test/functional/wallet_send.py +21/-0
  • src/wallet/test/wallet_tests.cpp +12/-6
  • test/functional/wallet_backwards_compatibility.py +18/-0
  • src/wallet/test/spend_tests.cpp +9/-2
  • src/wallet/rpc/transactions.cpp +2/-2
  • src/wallet/receive.h +1/-1
  • src/wallet/test/util.h +1/-1
  • src/wallet/test/fuzz/spend.cpp +1/-0

Card

The short text the ranking pass compares across PRs.

Bitcoin Core currently iterates over all known wallet outputs during balance checking and coin selection preparation, including outputs that have already been spent in confirmed blocks or invalidated by reorgs. This PR separates confirmed-spent and conflicted TXOs into a secondary container so that AvailableCoins and balance operations only iterate over potentially spendable outputs. It also stores relevant transaction state fields directly in WalletTXO to avoid frequent parent transaction lookups. The change is aimed at noticeably speeding up balance calculation and coin selection for wallets with long transaction histories, building on the in-memory TXO tracking architecture introduced in PR 27286.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-18T03:25, confidence high, input hash bb704226e1ca088b
thread read through 2026-08-27 (54 statements), head 37cd9d6885
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Wallet, Needs rebase

Categories

Wallet

P2 · speedup (ranking pass: Moved down: large speedup, but it is the tip of a chain whose bases (#33033, #33034) must be reviewed first.)

Separating spent TXOs dramatically speeds up balance calculations and coin selection for active wallets with deep transaction histories. Reviewers observed massive speedups (remyers reported 250x and w0xlt reported ~508x on large synthetic test wallets), eliminating a major recurring performance bottleneck introduced when full TXO tracking was added.

Membership: Labeled Wallet and modifies wallet state management and storage files under src/wallet/.

Factors: security/stability 0, bug 0, performance 3, user value 2, leverage 1