#35786 wallet: drop spent parents redundant cache invalidation and notification

full analysis

https://github.com/bitcoin/bitcoin/pull/35786 · furszy · +108/-32 in 4 files, 3 commits · labels: Wallet

Goal

  • Remove redundant parent transaction cache invalidations and UI notifications when outputs are spent
  • Clean up outdated balance-tracking logic made obsolete by earlier wallet refactoring

This PR removes `MarkInputsDirty` calls across several wallet paths (`AddToWallet`, `SyncTransaction`, `RecursiveUpdateTxState`, and `CommitTransaction`) and eliminates parent transaction GUI change notifications during `CommitTransaction`. Following #27286, `CWalletTx` caches only its own incoming/outgoing amounts and origin rather than available wallet balance, so spending an output does not change the parent transaction's cached data.

Problem: Before #27286, `CWalletTx` cached available balance, requiring parent transactions to be invalidated whenever their outputs were spent. After that refactor, available balance is tracked separately in `m_txos` and `mapTxSpends`, leaving the parent cache invalidation and UI notification loops redundant.

Category: Wallet (#64 of 84)

P3 · cleanup

  • P3 because it is an internal cleanup removing dead invalidation logic left over from earlier refactoring
  • Improves readability and eliminates minor overhead but fixes no major bug and adds no new capabilities

P3 because this is an internal cleanup removing dead invalidation logic left behind after #27286. While it simplifies code readability and removes unnecessary recomputations, the operational overhead removed is minor and does not fix a major bug or add new capabilities.

Membership: Modifies core wallet transaction caching and notification in src/wallet/wallet.cpp and transaction.h.

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

Reviewability: Ready

  • Ready for review
  • Passing CI and clean mergeability, with an edge-case report awaiting author response

The PR has passing CI, clean mergeability, and recent review activity. The author has only had two days to evaluate w0xlt's reproduction test.

Author status: active; last pushed 2026-09-14, awaiting response to w0xlt's 2026-09-15 edge-case report

Open concerns:

  • w0xlt identified a narrow edge case where spending an output expands the keypool, causing another output of the parent transaction to become recognized as wallet-owned; without the invalidation, `gettransaction` continues reporting the outdated cached amount.

Resolved concerns:

  • pablomartin4btc raised minor nits on comments and test naming, which the author addressed or clarified.

Agreement: Mild

  • General support for removing redundant invalidations and cleaning up confusing code
  • Verified code and test changes (pablomartin4btc, Bicaru20)
  • Concept approval noting clearer mark-dirty logic (rkrux)
  • Unaddressed objection: stale cached amounts reported if keypool expands during spend (w0xlt)

Supported by multiple reviewers, but w0xlt showed an edge case where stale cached amounts persist after keypool expansion.

w0xlt demonstrated with a functional test that keypool expansion during a spend can cause a parent's newly owned output to be missed if the cache is not invalidated. While acknowledged as a narrow edge case, it remains unaddressed by the author.

  • pablomartin4btc gave Concept ACK and full ACK after reviewing code and tests
  • Bicaru20 ACKed verifying cache semantics and tests
  • rkrux Concept ACKed noting the removal cleans up confusing mark-dirty calls
  • w0xlt provided a reproduction test where gettransaction reports an outdated amount when keypool expansion uncovers another parent output

Review verdicts (DrahtBot): 0 (+2)

Files

101 lines under test/bench/ci.

  • src/wallet/test/wallet_tests.cpp +101/-0
  • src/wallet/wallet.cpp +0/-26
  • src/wallet/transaction.h +7/-3
  • src/wallet/wallet.h +0/-3

Card

This PR removes redundant cache invalidation calls and GUI notifications for parent transactions when their outputs are spent. Since #27286, `CWalletTx` only caches static per-transaction debit/credit values rather than wallet spendable balances, making repeated recomputations on spend obsolete. The change is supported by several wallet contributors, though an edge case was recently surfaced by w0xlt showing that keypool expansions triggered by a spend could leave parent cached amounts stale.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:20, confidence high, input hash 2d192164dc4423fc