wallet: avoid deferred keypool unlock deadlock

[kiwidream]

Summary

Fixes #138.

This removes the wallet/SQLite lock cycle that could freeze qbit-qt immediately after a correct passphrase was entered.

• publish address-availability changes only after descriptor locks and database transactions have been released or committed
• deliver the Qt notification through an explicit queued connection
• let interface unlock return promptly and refill a pending initial P2MR keypool through deduplicated scheduler steps
• cover commit timing, lock ordering, relocking, duplicate scheduling, failed transactions, and queued Qt delivery with regressions

Testing

• Built locally.
• Ran focused unit or functional tests for the changed area.
• Ran lint or formatting checks relevant to this change.
• Not run. Reason:

Commands/checks:

• cmake --build build_dev_mode --target test_bitcoin test_bitcoin-qt qbitd qbit-cli -j8
• wallet_tests/DescriptorTopUp*
• wallet_p2mr_deferred_keypool_tests/*
• wallet_p2mr_receive_keypool_tests/*
• wallet_p2mr_change_keypool_tests/*
• wallet_p2mr_markunused_tests/*
• test/functional/wallet_keypool_topup.py
• full ci/lint/06_script.sh through the Ubuntu 24.04 Docker lint image

The new Qt regression compiled successfully. It was not independently executed because the macOS Qt test runner fails in a pre-existing AppTests case before reaching WalletTests.

Target Branch

• This PR targets main or a maintainer-requested release branch such as 0.1.x.

Target: 1.x.x.

Risk / Review Notes

• Consensus, script, crypto, wallet, P2P, release, CI, or security-sensitive behavior changed.
• No consensus, script, crypto, wallet, P2P, release, CI, or security-sensitive behavior changed.

Notes:

This changes wallet notification timing and the interface unlock path. Direct wallet/RPC unlock behavior remains unchanged. Review should focus on transaction-listener lifetime, scheduler deduplication/reset behavior, and lock ordering around cs_wallet, cs_desc_man, and the SQLite writer.

Docs / Process Impact

Choose exactly one:

• I updated public docs because this PR changes user-visible behavior, integration guidance, release/process guidance, or expected validation.
• No public docs update needed. Reason: this restores intended unlock responsiveness and changes only internal wallet maintenance scheduling and notification timing.

libbitcoinpqc Subtree Checklist (if src/libbitcoinpqc changed)

• Source commit is reachable from an immutable release tag in Qbit-Org/qbit-libbitcoinpqc.
• qbit imports the tagged upstream tree directly without pruning or a curated subtree branch.
• Subtree import/update was performed with contrib/devtools/update-libbitcoinpqc-subtree.sh.
• test/lint/libbitcoinpqc-subtree-check.sh passes locally.
• Any default tag change in contrib/devtools/update-libbitcoinpqc-subtree.sh is intentional and matches doc/subtrees/libbitcoinpqc.md.

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

---

Note

High Risk
Changes wallet unlock scheduling, lock ordering among cs_wallet/cs_desc_man/SQLite, and when address availability is visible—core GUI wallet behavior with race/deadlock sensitivity.

Overview
Fixes a post-unlock freeze in qbit-qt by breaking a wallet/SQLite lock cycle around descriptor keypool maintenance.

Descriptor keypool top-ups no longer call TopUpCallback or NotifyCanGetAddressesChanged while cs_desc_man or an open DB transaction is held. New PublishTopUp / *NoNotify paths batch script publication and address-availability signals after commit (including RegisterTxnListener for caller-owned transactions). m_cached_spks is updated under cs_wallet via TopUpCallback, with related lookups taking the wallet lock.

Qt delivers canGetAddressesChanged through an explicit Qt::QueuedConnection. GUI unlock (WalletImpl::unlock) skips synchronous initial keypool work, then MaybeSchedulePendingInitialKeyPoolTopUp runs deduplicated scheduler steps (relock/reschedule handling). Wallet create/load still defer background refill with a longer delay.

Regressions cover commit/abort visibility, lock ordering, interface unlock scheduling, failed top-ups (no spurious notify), and queued Qt delivery.

^{Reviewed by Cursor Bugbot for commit c3eb110. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2220e5f. Configure here.}

[kiwidream]

PR Review

Findings

• [blocking] src/wallet/interfaces.cpp:168 - Scheduled refill races foreground access to the wallet script cache. unlock() now returns after scheduling RunPendingInitialKeyPoolTopUpStep(), which calls descriptor TopUp() without holding cs_wallet. Top-up then reaches CWallet::TopUpCallback() and mutates the m_cached_spks unordered_map, while foreground operations such as CreateTransaction() read the same map under cs_wallet through IsMine() and GetSolvingProvider(). Since the writer does not take that lock, immediate transaction preparation can overlap an unsynchronized map insertion/rehash, producing undefined behavior, crashes, or incorrect script ownership/provider lookup. The green TSan job does not disprove this: the new scheduler test does not run a cache reader concurrently with top-up publication.
  • Potential resolution: Stage new script keys until the relevant database transaction has committed and cs_desc_man is released, then publish them while holding cs_wallet; alternatively, introduce a dedicated mutex consistently used by every m_cached_spks reader and writer. Add a deterministic TSan regression overlapping interface unlock refill with IsMine(), GetSolvingProvider(), or transaction creation.

Consensus Impact

Non-consensus

The diff changes wallet keypool maintenance, notification timing, scheduler ownership, Qt delivery, and test timing. It does not alter transaction or block validity, signature verification, activation, chain selection, or committed serialization.

Review Scope

Reviewed issue #138 as the backing contract, the PR body, all five commits and 14 changed files, surrounding wallet/SPKM/Qt call paths, and the current exact-head CI results. I also inspected all three review threads; all are resolved and none remain open.

Deep Checks Performed

Traced deferred refill from interface unlock through scheduler execution, descriptor locking, SQLite transaction ownership, cache publication, wallet notifications, and Qt queued delivery. I checked commit-listener lifetime and write-failure behavior, lock/relock and duplicate-scheduling races, reservation/refill atomicity, and attempted to disprove the finding by inspecting cache readers/writers and the actual TSan interleavings.

Issue Fit

The PR otherwise matches most of issue #138: interface unlock returns without synchronous full refill, address-availability notifications are moved past descriptor locks and transaction commit, the Qt bridge is explicitly queued, and deterministic tests cover scheduler rescheduling and the original lock/SQLite notification shape. The cache-publication race means the newly asynchronous refill is not yet safe alongside the issue’s requirement that transaction preparation proceed immediately after unlock.

The interface_bitcoin_cli.py mock-time stabilization is minor scope drift but is isolated and supported by passing functional validation.

qbit-Specific Checks

Reviewed P2MR receive/change keypool state, encrypted-wallet lock/relock behavior, deferred-state persistence, duplicate worker ownership, reservation atomicity, failed-write rollback, and notification delivery. Direct RPC unlock behavior remains synchronous. No PQC signature construction, key serialization, P2MR address encoding, consensus, mining, or networking behavior changed.

Validation Reviewed

Full Validation and the required merge gate passed at head 2f30bffc1523a0474a15c49cac76fecf1c3f32c6. The ASan/LSan/UBSan/integer job passed all 182 tests, including test_qbit-qt and the changed P2MR wallet suites. TSan and MSan passed all 181 enabled unit tests, including the deferred/change/receive keypool suites and wallet_tests. Windows and i686 Qt tests passed, and functional CI passed both wallet_keypool_topup.py and interface_bitcoin_cli.py. Docker lint ran ./ci/lint/06_script.sh successfully. I also ran git diff --check; no whitespace errors were reported.

The author additionally reports focused wallet suites and the keypool functional test passing locally. I did not perform a separate local build because no build tree was present and exact-head sanitizer/platform CI covered the changed suites.

Residual Risk

• The original macOS GUI scenario was not reproduced locally; the PR notes a pre-existing macOS AppTests limitation, while Linux, i686, and Windows Qt CI exercised the new queued-delivery test.
• The synthetic regression models the wallet-lock/SQLite overlap but does not reproduce the complete GUI unlock plus blockConnected sequence end to end.