#35151 wallet, follow-up: Refactor IsSpent to use HowSpent

ReviewabilityReviewsAgreementSize
#35151 musaHaruna wallet, follow-up: Refactor IsSpent to use HowSpentAuthor silent 180d1 -1Mild8
  • Deduplicate spend-checking traversal logic across wallet methods
  • Maintain early-exit behavior when checking if a transaction output is spent
  • Author silent for 142 days, so review feedback may wait a long time for a response
  • Clean refactoring approved after adding an early-exit option (polespinasa)
  • Objection over loop efficiency and missing early exit addressed by author (luke-jr)
  • Concept NACK over loop performance not formally withdrawn (luke-jr)
  • 8 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 24 lines removed in total
  • 2 files, 1 commits
Wallet#84 of 92
P4 · cleanup
  • P4 because it is an internal cleanup with no user-visible payoff or performance gain
  • Removes duplicated spend traversal code without altering wallet behavior
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

Refactor CWallet::IsSpent to delegate directly to CWallet::HowSpent instead of duplicating the spend-check traversal. An optional boolean flag is added to HowSpent to support early exit as soon as any non-conflicted spend is detected, and IsSpent is converted into an inline header wrapper.

Problem: CWallet::IsSpent and CWallet::HowSpent duplicated logic for iterating over mapTxSpends and evaluating whether a wallet transaction is conflicted or abandoned.

Evidence of importance:

  • PR description notes this is a follow-up to review discussion on merged PR #33671: 'Follow-up to #33671'

Scope: Small refactor that adds an optional boolean parameter to CWallet::HowSpent to preserve early-exit behavior.

Reviewability

Stale · Author silent 180d

No author activity for 180 days; a review may wait a long time for a response.

Agreement

Mild

NACK from luke-jr not withdrawn (performance regression from performing unnecessary extra checks and removing early exit in the spend-checking loop)

Thread notes: Initial record for PR 35151. luke-jr's Concept NACK over loop inefficiency was resolved by pushing an early-exit flag to HowSpent as suggested by polespinasa, who subsequently ACKed the squashed branch without further objection from luke-jr.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
luke-jr (contributor)objection, approachperformance regression from performing unnecessary extra checks and removing early exit in the spend-checking loopfixed · fix pushed · v1, not yet re-checkedchangedyes2026-04-25: 'Concept NACK. You've made it much less efficient. Not only does it perform more checks, it also doesn't exit the loop early.'
Status from: 2026-04-30: musaHaruna: 'Added commit 775964ef02e8e8c85ea2caf01b76e41a454212f6 as suggested by @polespinasa to address the early-exit concern raised in review.'

Support:

  • polespinasa (member) (Concept ACK, 2026-04-24): The refactor is nice and clean.
  • polespinasa (member) (ACK, 2026-08-11): ACK ac774b318440c7cc4804516b1f811005e25e8d45; noted that while the isSpent parameter name could be clearer, comments make it clear and the function is small.

Participants:

  • polespinasa (member, support, 8 statements 2026-04-24 to 2026-08-11): concept ACKed, helped design an early-exit parameter for HowSpent, and ACKed the squashed commit
  • luke-jr (contributor, objection, 1 statements 2026-04-25 to 2026-04-25): concept NACKed due to performance degradation and lack of early loop exit

Files

0 lines under test/bench/ci.

  • src/wallet/wallet.cpp +4/-22
  • src/wallet/wallet.h +4/-2

Card

The short text the ranking pass compares across PRs.

This PR refactors CWallet::IsSpent to call CWallet::HowSpent instead of maintaining duplicate traversal logic over mapTxSpends. HowSpent receives an optional boolean flag to early-exit as soon as an outpoint is confirmed spent, and IsSpent becomes an inline one-line wrapper in wallet.h. This is an internal code deduplication follow-up to PR #33671 with no user-facing changes or open dependencies.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-18T03:29, confidence high, input hash 9be37d290045f594
thread read through 2026-08-11 (15 statements), head ac774b3184
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: none

Categories

Wallet

P4 · cleanup

Refactors IsSpent to delegate directly to HowSpent to eliminate duplicated spend traversal logic without changing wallet behavior. As an internal follow-up cleanup to #33671 with no stated performance or user-visible payoff, it has low claim on review attention.

Membership: Modifies core wallet methods CWallet::IsSpent and CWallet::HowSpent in src/wallet/wallet.cpp and src/wallet/wallet.h.

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