#36583 wallet: fix migration of watch-only wallets with transactions

ReviewabilityReviewsAgreementSize
#36583 yusufyildiz34 wallet: fix migration of watch-only wallets with transactionsReady1Crickets9 + 55 tests
  • Allow users to migrate legacy watch-only wallets that contain transactions to descriptor wallets
  • Prevent crashes from unhandled exceptions when wallet migration aborts
  • Ready for review
  • No substantive feedback yet
  • 9 lines added or modified outside tests
  • 55 lines added or modified in tests
  • 3 lines removed in total
  • 5 files, 2 commits
Wallet#7 of 92
P2 · bug fix
  • P2 because it fixes deterministic migration failures for watch-only wallets with transaction records
  • Prevents unhandled exceptions on abort that bypass wallet migration cleanup
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

Fixes migration of watch-only legacy wallets with transactions during `migratewallet`. It guards database transaction listener execution so unset callbacks are skipped rather than throwing `std::bad_function_call` on commit or abort, and updates the watch-only migration check to account for transaction records queued for deletion.

Problem: Migrating a legacy wallet containing watch-only addresses and transaction records always fails. `CWallet::ApplyMigrationData()` expects in-memory `mapWallet` to be empty before the transaction commit listener has actually purged them, which triggers an abort; calling `TxnAbort()` then unconditionally invokes an unset `on_abort` callback registered by `RemoveTxs()`, throwing an unhandled `std::bad_function_call` that bypasses migration cleanup.

Evidence of importance:

  • Issue #36483 reports 'wallet: Migration fails for legacy wallet with no keys watching an address'
  • PR description explains the two bugs: 'mapWallet is therefore never empty at that point and the check always fails' and 'TxnCommit() and TxnAbort() each invoked their listener's callback unconditionally, so the abort triggered by the failed check above threw std::bad_function_call.'

Risk of a mistake: medium (wallet, wallet_storage). A flaw in wallet database transaction listener handling or migration verification could leave wallets partially migrated or corrupt wallet database state.

Changed since the previous assessment: No functional changes; diff confirms the previous description.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: initial review

Agreement

Crickets

no substantive comment either way

Thread notes: Initial record. Naomi-Gift left an untested ACK without rationale, which is not counted as support because the account has no history in the project. / Initial record. Naomi-Gift left a utACK with no rationale; as an account with no history in the project, it is not recorded as support.

Objections, suggestions, and questions: none recorded.

Participants:

  • Naomi-Gift (none, support, 1 statements 2026-10-22 to 2026-10-22): utACK with no rationale from account with no project history

Files

55 lines under test/bench/ci.

  • src/wallet/test/db_tests.cpp +35/-0
  • test/functional/wallet_migration.py +20/-0
  • src/wallet/wallet.cpp +5/-1
  • src/wallet/walletdb.cpp +2/-2
  • src/wallet/walletdb.h +2/-0

Card

The short text the ranking pass compares across PRs.

This PR fixes a bug preventing legacy watch-only wallets containing transaction records from being migrated to descriptor wallets via migratewallet. Currently, ApplyMigrationData checks whether mapWallet is empty before database commit listeners have cleared it, triggering an abort that throws std::bad_function_call because on_abort is unset. The patch skips unset transaction listeners and validates that all transactions are queued for deletion, resolving issue #36483.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-23T01:19, confidence high, input hash 44f5f05d0e6363f0
thread read through 2026-10-22 (1 statements), head 50f254e53a
labels: Wallet

Categories

Wallet

P2 · bug fix

Fixes a deterministic failure and unhandled exception during legacy-to-descriptor wallet migration for watch-only wallets with transactions. Migrating these wallets currently fails when checking if `mapWallet` is empty and subsequently throws `std::bad_function_call` during `TxnAbort()` because listener callbacks are unset, preventing migration and bypassing cleanup.

Membership: Matches Wallet label and modifies wallet migration logic in src/wallet/wallet.cpp and transaction listener callbacks in src/wallet/walletdb.cpp.

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