qt: move PSBT signing off GUI thread

[kiwidream]

Summary

• Move PSBT signing initiated by PSBTOperationsDialog into a cloned-wallet worker context so multi-input P2MR signing no longer blocks the Qt event loop.
• Publish a worker-owned PSBT on the GUI thread only after successful completion, with queued progress for preparation, counter reservation, signing, finalization, and verification.
• Allow cancellation before durable PQC counter reservation and disable it after the existing post-commit reservation observer fires.
• Add deterministic Qt lifecycle/cancellation coverage and real-wallet P2MR counter-boundary regression tests.

Closes #140.

Testing

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

Commands and results:

• cmake --build build --target bitcoin_wallet qbit-qt test_qbit-qt test_qbit -j"$(sysctl -n hw.ncpu)" — passed.
• build/bin/test_qbit '--run_test=wallet_p2mr_parallel_signing_tests,psbt_p2mr_tests,psbt_wallet_tests,wallet_p2mr_signing_tests' --log_level=test_suite — 40/40 passed.
• QT_QPA_PLATFORM=cocoa build/bin/test_qbit-qt — the new PSBTOperationsDialogTests passed 9/9 and all other relevant wallet/Qt lifecycle classes passed. The binary still reports one unrelated failure in the unchanged RPCNestedTests stale genesis-transaction hash assertion.
• Repository Docker lint workflow — passed with exit code 0 (All checks passed!).
• git diff --check — passed.

Target Branch

• This PR targets the maintainer-requested 1.x.x release branch.

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:

• PSBT input signing remains serial; this change moves the existing signing flow off the GUI thread without changing provider ordering or introducing batch counter reservation.
• A cloned wallet and QPointer/generation guards prevent callbacks into destroyed dialogs or wallet models. Dialog destruction joins the worker, and wallet unload leaves the worker-owned clone valid until completion.
• External-signer calls run in the worker but cannot be interrupted after the external process call begins. Shutdown may therefore wait for an in-flight external signer or post-reservation cryptography to finish.
• PQC usage information is retained for callers, but presentation changes remain intentionally out of scope for Preserve PQC usage in Qt transaction, PSBT, and fee-bump signing flows #141.

Docs / Process Impact

• 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 changes Qt responsiveness, progress, cancellation, and lifetime handling without changing public integration or operator guidance.

libbitcoinpqc Subtree Checklist (if src/libbitcoinpqc changed)

Not applicable; src/libbitcoinpqc is unchanged.

---

Note

Medium Risk
Touches wallet PSBT signing, PQC counter reservation, and async Qt lifetime; behavior changes are mostly UI threading but incorrect cancel or clone handling could affect signing outcomes.

Overview
PSBT signing in the Qt GUI no longer blocks the event loop. PSBTOperationsDialog runs fillPSBT on a worker thread using a cloned wallet, shows phased progress (prepare, PQC counter reservation, signing, finalize, verify), and applies the signed PSBT on the GUI thread only after success.

Cancellation is honored via a new SigningProgressCallback on interfaces::Wallet::fillPSBT and CWallet::FillPSBT, with cancel disabled once PQC counters are durably reserved. The send flow maps the new verifying phase into its existing finalizing progress UI.

Status UX appends PQC usage details after signing attempts, and nine Qt tests cover responsiveness, cancel boundaries, dialog/wallet lifetime, and failure preserving the original PSBT.

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

---

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

[kiwidream]

PR Review

Findings

• [medium] src/qt/psbtoperationsdialog.cpp:282 - Post-reservation PQC usage is retained but never reported. m_last_sign_pqc_usage is assigned here and reset before the next attempt, but it has no reader. Consequently, if one P2MR input durably reserves a counter and a later input fails, the dialog preserves the original PSBT but displays only the generic PSBT error. This leaves issue Sign multi-input P2MR PSBTs outside the qbit-qt GUI thread #140’s consumed-capacity reporting criterion unmet. The PR references Preserve PQC usage in Qt transaction, PSBT, and fee-bump signing flows #141 for presentation work, but Sign multi-input P2MR PSBTs outside the qbit-qt GUI thread #140 has no comment or amendment narrowing its acceptance criteria.
  • Potential resolution: Display the nonempty usage report on success and failure, and extend the synthetic failure test to reserve a counter and assert the resulting capacity message. If this is intentionally deferred to Preserve PQC usage in Qt transaction, PSBT, and fee-bump signing flows #141, explicitly narrow Sign multi-input P2MR PSBTs outside the qbit-qt GUI thread #140’s acceptance criteria before merge rather than retaining an unread private report.

Consensus Impact

Consensus-adjacent

The PR changes wallet signing orchestration and stateful PQC counter cancellation boundaries in CWallet::FillPSBT, the wallet interface, and PSBTOperationsDialog. Issue #140 explicitly authorizes these control-flow changes. Signature hashing, provider ordering, script verification rules, witnesses, and PSBT serialization remain unchanged. The real-wallet before/after-reservation tests and existing P2MR PSBT verification suites are the relevant required coverage; no new consensus vectors are needed.

Review Scope

Reviewed issue #140 as the backing contract, issue #141 as the stated follow-up boundary, the PR body, all three commits and 15 changed files, and surrounding wallet, SPKM, Qt, external-signer, and unlock-lifetime paths. Both prior review threads are resolved, with no open threads. Exact-head CI and sanitizer logs were inspected.

Deep Checks Performed

Traced worker-owned PSBT publication, cloned-wallet lifetime, unlock/relock handling, generation guards, queued callbacks, dialog destruction, wallet unload, and thread joining. I followed cancellation through the pre-reservation callback and the post-commit counter observer, checked partial-signing and error paths, and attempted to disprove the finding by searching every use of the captured usage report and reviewing the failure tests.

Issue Fit

The implementation otherwise satisfies the central contract: signing runs through a worker-owned wallet and PSBT, progress is queued to the GUI, cancellation is honored before durable reservation and ignored afterward, successful results are applied once, failures preserve the displayed PSBT, and destroyed models/dialogs are guarded. Provider ordering remains serial, as permitted by #140.

The remaining mismatch is reporting capacity consumed before a later signing failure. The current failure test neither simulates a reservation nor supplies/asserts a usage report.

qbit-Specific Checks

Reviewed P2MR counter reservation timing, encrypted-wallet snapshots, cancellation boundaries, original-PSBT preservation, external-signer fallback, and wallet-unload lifetime. PQC usage remains wallet-local and is not inserted into the portable PSBT. No changes were made to P2MR commitments, signature inputs, address encoding, witness layout, consensus, mining, or networking.

Validation Reviewed

Full Validation and required gates passed at head 5b9e50a80faf6ffbfaca611f59a981cb90756634. ASan/LSan/UBSan/integer CI passed all 182 tests, including test_qbit-qt, wallet_p2mr_parallel_signing_tests, psbt_p2mr_tests, psbt_wallet_tests, and wallet_tests. TSan and MSan passed all 181 enabled unit tests, including the changed real-wallet suite. Windows ran test_qbit-qt offscreen successfully, and Docker lint reported all checks passed.

The author additionally reports a successful build, 40/40 focused wallet tests, and all current PSBTOperationsDialogTests passing under macOS Cocoa; the full local Qt binary retained an unrelated RPCNestedTests failure. I ran git diff --check successfully but did not perform a separate local build.

Residual Risk

• Qt lifecycle tests use a synthetic wallet, while real P2MR counter tests invoke CWallet::FillPSBT directly; no single test exercises the complete real-wallet Qt path.
• An in-flight external-signer process or post-reservation cryptography remains non-interruptible, so dialog destruction or shutdown may wait for it.
• Large-PSBT latency in GUI-thread post-completion analysis/display was not measured; the responsiveness test covers the worker interval.

[kiwidream]

Addressed in GPG-signed commit 435fbba461.

• Every nonempty PQCUsageReport is now appended to the PSBT signing status on both success and failure, including the affected key, used/limit count, remaining capacity, state, and warnings.
• The failure path still leaves the displayed/original PSBT unchanged.
• The synthetic regression now simulates durable counter reservation followed by signing failure and asserts the consumed-capacity report. The success test also verifies the report and exactly one displayed-PSBT update.

Verification at this head:

• cmake --build build --target qbit-qt test_qbit-qt -j8
• QT_QPA_PLATFORM=cocoa build/bin/test_qbit-qt — all tests passed; 11/11 PSBTOperationsDialogTests
• build/bin/test_qbit '--run_test=wallet_p2mr_parallel_signing_tests,psbt_p2mr_tests,psbt_wallet_tests,wallet_p2mr_signing_tests' --log_level=test_suite — 40/40 passed
• Repository Docker lint workflow — all checks passed
• git diff --check

This is intentionally scoped to PSBTOperationsDialog; the broader reusable usage-presentation work remains with #141.

[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 435fbba. Configure here.}

[cursor]

Ignored cancel on progress updates

Medium Severity

Several new FillPSBT progress updates after an input signs, after finalize, and during verification call the cancellable progress callback but ignore its result. The PSBT dialog still records cancel_observed when the user cancels on those updates, so signing can finish successfully yet the UI reports cancellation and never applies the signed PSBT.

Additional Locations (1)

• src/qt/psbtoperationsdialog.cpp#L325-L330

 

^{Reviewed by Cursor Bugbot for commit 435fbba. Configure here.}