qt: allow a locked wallet to redeem DigiDollar via on-demand unlock - #440
qt: allow a locked wallet to redeem DigiDollar via on-demand unlock#440JohnnyLawDGB wants to merge 2 commits into
Conversation
The DigiDollar redeem widget and the positions-tab row action both
disable themselves when an encrypted wallet is locked, so a user has
to reach for `walletpassphrase` in the node console before the GUI
will let them close a vault.
This is inconsistent with the sibling DigiDollar flows, which all
confirm and then unlock on demand: digidollarsendwidget.cpp:649 and
digidollarmintwidget.cpp:834 both open with
WalletModel::UnlockContext ctx(m_walletModel->requestUnlock());
if (!ctx.isValid()) return;
Redeem has exactly that block, at digidollarredeemwidget.cpp:625 --
but with the button disabled it can never run. So this change does
not introduce a new pattern; it lets redeem reach the one its
siblings already use. The RPC layer has always behaved this way, see
test/functional/digidollar_encrypted_wallet.py.
Both widgets already distinguish a permanent inability to sign
("Watch-Only", private keys disabled) from a temporary lock ("Wallet
Locked") in their labels and tooltips, then collapse the two to
setEnabled(false). Only the former is a real incapability: a locked
wallet still holds its keys.
Split the two states apart:
- canWalletSignRedemption() now reports only permanent incapability
(no wallet model, or privateKeysDisabled()). The lock check moves
to a new walletNeedsUnlockToRedeem() predicate that drives
presentation, not enablement, and the corresponding branch is
dropped from redeemDisabledReason() so the remaining disabled
states still report an accurate reason.
- The redeem button reads "Unlock & Redeem" with a tooltip
announcing the passphrase prompt while locked, and keeps the
existing "Redeem & Unlock DGB" wording once unlocked.
- In the positions tab, walletCanSign no longer folds in the lock,
so a matured vault stays clickable. The row keeps its distinct
locked-wallet colour, now with the metrics of the ordinary
redeemable button so the same "Redeem" label does not change size
with lock state. The row action only navigates to the redeem tab,
so it inherits the unlock prompt rather than duplicating it.
Watch-only wallets keep the disabled "Watch-Only" badge, and a
locked wallet whose vault has not matured still shows the timelock
"Locked" state.
Also narrow the two ReconcilePositionStates() guards, which skipped
reconciliation whenever privateKeysDisabled() || status == Locked.
That function reads mapWallet and the chain UTXO set via
chain().findCoins() and rewrites the is_active flag through
WalletBatch; it never retrieves key material, so the lock half of
the condition was unrelated to what it does. It was not merely
redundant: with the row action no longer gated on the lock,
pos.canRedeem would have been computed from an is_active flag
deliberately left stale, so a vault whose collateral had been spent
out of band would offer an enabled "Redeem" until the user unlocked.
Both guards are now privateKeysDisabled() only, renamed
walletIsWatchOnly.
Tests:
- redeemWidgetButtonStateLockedWallet ->
redeemWidgetButtonStateLockedWalletOffersUnlock. Was: disabled
button, tooltip contains "Unlock". Now: enabled, "Unlock &
Redeem", tooltip announcing the passphrase step. The contract
changed, so the assertion did. Its fixture also funds the $DD burn
generously; the exact-amount funding it used before was never
actually exercised, because the lock check short-circuited ahead
of the balance check.
- positionsWidgetDisablesRedeemForLockedEncryptedWallet ->
positionsWidgetKeepsRedeemActionableForLockedEncryptedWallet, now
requiring the row action to stay enabled. Its mock position is
anchored on a live coinbase output, because a locked wallet now
really does reconcile and would otherwise correctly retire a
position whose collateral outpoint does not exist.
- New redeemWidgetLockedWalletRequestsUnlockOnRedeem drives
onRedeemClicked() on a locked wallet, accepts the confirmation
box, and asserts that WalletModel::requireUnlock fires exactly
once -- pinning that the requestUnlock() call site is reachable,
which is the point of the change. It also covers the dismissal
path: nothing answers the request, the wallet stays locked, and
redemptionCompleted() is not emitted.
- New widgetsDisableRedeemForEncryptedWatchOnlyWallet covers the one
state where the two conditions overlap.
WalletModel::getEncryptionStatus() reports Locked, not NoKeys, for
a wallet that is both crypted and privateKeysDisabled(), and both
widgets must still refuse it.
- redeemWidgetRefreshesWhenWalletUnlocks keeps asserting the locked
-> unlocked transition; only its locked half now checks the
"Unlock & Redeem" affordance instead of a disabled button.
The two tests asserting a disabled button for privateKeysDisabled()
wallets are unchanged: that behaviour is correct.
The GUI can only unlock a wallet as a side effect of starting an operation that needs keys: WalletView::unlockWallet() is reached solely through the WalletModel::requireUnlock signal. A user who wants the wallet unlocked up front has to use `walletpassphrase` in the node console. Add a Settings menu entry following the encryptWalletAction / changePassphraseAction pattern: declare the QAction, create it in createActions(), connect it, add it to the menu, and drive its enabled state from setEncryptionStatus() so it is offered only for an encrypted wallet that is currently locked. setWalletActionsEnabled() also toggles it so it is disabled with no wallet loaded; setEncryptionStatus() runs immediately afterwards via WalletFrame::currentWalletSet and narrows it to the Locked case, the same way encryptWalletAction is handled. The existing unlockWallet() slot is deliberately synchronous, because WalletModel::requestUnlock() inspects the encryption status again as soon as the dialog returns. Nothing waits on a menu-driven unlock, so wiring QAction::triggered into that slot would nest a modal event loop inside an action handler -- precisely what GUIUtil::ShowModalDialogAsynchronously() exists to avoid, and what the other menu-driven wallet dialogs (WalletView::encryptWallet, WalletView::changePassphrase) already use. Add a sibling WalletView::unlockWalletFromMenu() that shows the same AskPassphraseDialog asynchronously, plus a WalletFrame forwarder, and leave the synchronous slot wired only to WalletModel::requireUnlock.
918b69a to
409f84e
Compare
Note on the macOS CI failure (previous run)The macOS 14 ARM64 job on the previous push failed in Run Functional Tests, on This is a pre-existing flake on
I've force-pushed to re-trigger CI. Happy to open a separate issue tracking the |
Update: stronger evidence, and a correction to my previous commentI called this a flake above and said a re-run would clear it. The re-run reproduced it immediately, on the other platform, so "flake" understates it — this looks like a genuine intermittent shutdown bug in The two runs, on an identical tree (I amended the tip commit to re-trigger CI;
Same test both times —
The platforms trading places across two runs of the same tree rules out both the diff and any platform-specific cause. It also reproduces on bare For completeness on why it can't be this PR: the diff is 11 files, all under Separate CI gap worth fixingThe functional-test logs are never captured, so there's no post-mortem evidence for failures like this one:
I'm chasing the shutdown bug locally now and will open a separate issue with whatever I find. Happy to file the artifact-path gap separately too. Neither should block review of this PR. |
|
Root-caused and filed as #441 — Nothing to change in this PR — it remains a Qt-only diff, and the red check here is that bug. |
1 participant
Problem
When an encrypted wallet is locked, the Qt DigiDollar redeem actions are disabled — but the click handler they would invoke already contains the intended
WalletModel::requestUnlock()passphrase flow (digidollarredeemwidget.cpp, inonRedeemClicked()).Because the action can never be clicked, that flow is unreachable dead code. There is also no standalone unlock action in the Settings menu, so the practical workaround is to drop to the console and run
walletpassphrasebefore redeeming. Users hitting this have had to be walked through the RPC by hand.The RPC layer is already correct:
test/functional/digidollar_encrypted_wallet.pycovers locked-rejects and unlocked-succeeds. Only the GUI is affected.Why this is a real defect, not a design choice
The widgets already distinguish permanent signing incapability from a temporary lock — a watch-only wallet gets a
"Watch-Only"badge, a locked encrypted wallet gets"Wallet Locked"— but they collapse both tosetEnabled(false).Only the first is a genuine incapability. A locked encrypted wallet still holds its keys; it needs authentication, not refusal. The unreachable
requestUnlock()call is direct evidence that on-demand unlock was the original intent.This also makes redeem consistent with its siblings:
digidollarsendwidget.cppanddigidollarmintwidget.cppalready use the confirm →requestUnlock()→ act pattern. Redeem was the odd one out.Changes
Commit 1 —
qt: allow locked wallet to redeem DigiDollarSeparates permanent incapability (
privateKeysDisabled(), or no wallet model) from a temporary lock in bothDigiDollarRedeemWidgetandDigiDollarPositionsWidget. Only the former disables the action; a locked encrypted wallet stays actionable and the existingrequestUnlock()prompt runs on click.Two gates were involved, not one: besides the button-enable predicate,
DigiDollarPositionsWidget::loadPositionsFromWallet()folded the lock intowalletCanSign, which feedspos.canRedeemand also drives the context-menu action. Fixing only the button would have left locked vaults falling through to the disabled timelock branch.The reconcile-time guards previously named
walletCannotSignare nowwalletIsWatchOnly.ReconcilePositionStates()touches no private keys — it reads coins viachain().findCoins()/IsSpent()and writesWalletBatch::WriteDDTimeLock(), all of which work on a locked wallet — so gating it on the lock only served to leaveis_activestale, which would have surfaced as an enabled Redeem on a vault whose collateral was already spent.Commit 2 —
qt: add "Unlock Wallet" action to Settings menuAdds a standalone unlock action, enabled only when the wallet is
Locked. Kept as a separate commit so it can be dropped independently if maintainers prefer, since upstream Bitcoin Core deliberately has no such action.This adds no unlock logic:
WalletFrame::unlockWallet()/WalletView::unlockWallet()already existed and already showedAskPassphraseDialog(Unlock); they were simply unreachable from any menu. Because the existing slot is deliberately synchronous (WalletModel::requestUnlock()requires it), the menu path uses a newunlockWalletFromMenu()built onGUIUtil::ShowModalDialogAsynchronously(), matching howencryptWalletandchangePassphraseare wired and avoiding a nested modal loop from an action handler.Tests
Two existing tests asserted the old contract and are intentionally flipped — renamed so the change is explicit rather than a silent boolean inversion. Both would fail against pre-branch code. No assertion was weakened; assertion counts went up (5→7 and 3→4), and both watch-only tests are unchanged and still assert disabled.
Added:
onRedeemClicked()on a locked wallet and assertsWalletModel::requireUnlockactually fires — pinning the reachability this PR exists to restore — plus the cancel pathDigiDollarWidgetTestsgoes 87 → 89 passed (0 failed, 3 skipped — the pre-existing platform-gated skips), and the full Qt suite passes.Note on GUI/RPC parity
listdigidollarpositionsstill reportscan_redeem/spendableas false while the wallet is locked, whereas the GUI now offers the action. That asymmetry is intentional: the GUI can prompt for a passphrase and the RPC cannot, so the RPC's answer to "can this be redeemed right now, without further input?" remains correct. Happy to align the RPC wording separately if maintainers would prefer.Prepared with AI assistance; all changes were human-reviewed, and every test result above was independently reproduced before submission.