#36583 wallet: fix migration of watch-only wallets with transactions
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36583 wallet: fix migration of watch-only wallets with transactions | Ready | 1 | Crickets | 9 + 55 tests | |||
|
|
|
|
| |||
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
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