fix: queue intents until unlocked and bind send to source account (M10, L10)

[n13]

Summary

Addresses findings M10 and L10 of the 2026-07-22 mobile wallet security audit, plus the review findings from the first round.

M10 — intents consumed beneath the lock overlay

Deep-link (/pay, /account) and notification (transaction, multisig proposal) intents were acted on by HomeScreen's listeners while the lock overlay was up.

• All intent handlers in home_screen.dart require localAuthProvider.isAuthenticated before consuming.
• Intents arriving while locked stay queued in their StateProviders and are drained when isAuthenticated flips to true. The cold-start retry on activeAccountProvider drains all pending intents.

L10 — source account re-resolved at submit time

RegularSendStrategy re-resolved the signing account at submit time, so an account switch between review and confirm signed from the new account.

• RegularSendStrategy captures the source Account at flow start (matching the other strategies) and uses it for fee estimation, balance validation and submission.
• effectiveMaxBalanceProviderFamily(accountId) binds spendable-balance validation to the captured account.

Send-flow lifecycle — single entry point, one flow at a time

All send flows start through startSendFlow() (send_providers.dart), the only place that mutates send-flow state:

• Sets sendFlowActiveProvider (bool) before pushing and clears it by awaiting the route's popped future — completed on pop, replacement, and pushAndRemoveUntil, so the flag is correct however the flow exits. No provider mutation in widget initState/dispose, no post-frame callbacks.
• Refuses to start a second flow while one is active (also guards double-tap).
• Owns the Keystone signing-session reset (startNewSendSession), so entry points no longer touch the keystone cache; a QR cached by an earlier flow (potentially stale nonce) can never be reused.
• Intents that would start or interrupt a send (payment, shared-account, proposal) arriving while a send is in flight are consumed and dropped with a log line — never queued to fire later.

First-round review findings

1. Balance-error fallback: effectiveBalanceProviderFamily keeps the last fetched balance on refresh errors instead of zeroing spendable-balance checks; errors before any successful fetch still propagate.
2. Non-regular active account: payment intents and the shared-address sheet's Send tap surface a "Switch to a regular account to send" warning toast (new l10n string, en + id) instead of failing silently; the sheet stays open.
3. _isUnlocked documents the deliberate choice to gate on authentication only, not the visual privacy overlay.
4. Tests added (see below).
5. All logging goes through quantusPrint.
6. Interleaved intents resolved structurally: the in-flight flag is set synchronously at entry, so a second queued intent in the same drain pass is dropped.

Merge with main

Merged current main (through #580). Conflict in shared_address_action_sheet.dart resolved to run main's SS58 fail-closed check first, then the regular-account guard, then startSendFlow.

Verification

• flutter test test/unit test/screens: 266/266 passed.
• New tests: intent queued while locked drains on unlock; payment intent opens the flow bound to the captured account and resets the Keystone session; intents arriving mid-send are dropped; shared-address sheet warns and stays open for non-regular accounts; effective balance keeps last-good value on refresh error and propagates first-fetch errors; Keystone cache fixtures deduplicated into test/fakes.dart.

Notes

• The multisig-propose and encrypted flows already carried their source account in the strategy; only the regular send needed the L10 change.
• Transaction-detail intents are gated on unlock but not dropped mid-send (informational sheet only, no account switch).

[n13]

🔍 Review — M10, L10 (intent gating & send-account binding)

Verdict: 🟡 Approve with non-blocking comments

The core fixes are correct and well-targeted: intent handlers now require isAuthenticated, so nothing is pushed under the live lock overlay (M10), and RegularSendStrategy captures its source Account at flow start and uses it for fee estimation, balance validation, self-send guard and submission (L10). The sendFlowActiveProvider deferral counter is balanced and leak-free. A handful of non-blocking behavior/UX/test-coverage items are worth addressing before or shortly after merge.

What it does

• Gates all four HomeScreen intent handlers (_onTransactionIntent, _onPaymentIntent, _onSharedIntent, _onProposalIntent) on _isUnlocked; deferred intents stay queued in their StateProviders and are drained by new localAuthProvider and sendFlowActiveProvider listeners plus the widened activeAccountProvider cold-start retry (home_screen.dart:82-93).
• Binds RegularSendStrategy to a captured Account (regular_send_strategy.dart:27-34,49-50,62-63,115), replacing the old submit-time re-resolution via SettingsService().getActiveRegularAccount() and activeAccountProvider.
• Adds sendFlowActiveProvider (int counter incremented/decremented in SelectRecipientScreen/InputAmountScreen init/dispose) so payment/shared/proposal intents are deferred while a send is in flight (send_providers.dart:13, input_amount_screen.dart:67,82,107, select_recipient_screen.dart).
• Replaces effectiveMaxBalanceProvider with effectiveMaxBalanceProviderFamily(accountId) bound to the captured account (send_providers.dart:19-35).

Strengths

• Root cause correctly identified. AuthWrapper is a Stack sibling over the live Navigator (app.dart:55), rendered whenever !isAuthenticated (auth_wrapper.dart:19). Gating on the same isAuthenticated flag is the right invariant, and because the overlay is not a route on the Navigator, draining/navigating on unlock does not race the overlay's own dismissal.
• Gate cannot be bypassed. Grep of all four intent providers confirms the only consumers are HomeScreen's listeners; deep_link_service/transaction_service merely produce (state = …). No second unguarded consumer exists.
• Queued intents are preserved, not dropped. Every deferred handler returns before setting provider.state = null, so the payload survives until conditions allow it (unlock, account load, send-flow exit). The re-drain listeners are idempotent.
• L10 binding is complete. Base re-resolved the account in three places (sourceAccountId, estimateFee, submit); all now use the captured account, so a mid-flow switch (even one triggered by a proposal-notification tap) cannot change the signer, fee basis, or self-send guard.
• Counter is symmetric and leak-safe. Increment in initState / decrement in dispose, with the notifier cached in a late final so dispose never touches ref. Terminal/review screens are pushed on top of InputAmountScreen (review_send_screen.dart:74), so the counter stays >0 through confirm/success and only drains after the flow fully unwinds — no premature drain over the success screen.

Findings

1. [non-blocking] Balance-error fallback silently changed. Old effectiveMaxBalanceProvider read balanceProvider, whose error branch returns AsyncValue.data(_cachedBalance) (wallet_providers.dart:207-209). The new effectiveMaxBalanceProviderFamily reads effectiveBalanceProviderFamily, whose error branch propagates AsyncValue.error (wallet_providers.dart:153-154). In input_amount_screen.dart:190 the max-sendable is computed as ...spendableBalanceProvider).value ?? BigInt.zero, so on a transient balance-query error mid-send the Max button and the amount <= spendable check now see 0 instead of the last-good cached balance — a transient RPC hiccup can block an otherwise-valid send. For the normal (data) case the two are identical, so this is a side-effect of the provider swap rather than an intended change. Consider a cached/last-good fallback for the captured account, or confirm 0-on-error is desired.

2. [non-blocking] Payment intent silently dropped for non-regular active account. _onPaymentIntent consumes the intent (paymentIntentProvider.state = null) and then, if active is! RegularAccount, returns with only quantusDebugPrint (home_screen.dart:126-130). The user sees nothing, and because the intent is already consumed the subsequent activeAccountProvider re-drain can't recover it after they switch to a regular account. Contrast with the active == null path, which defers. Consider deferring (don't null the state) until a regular account is active, or surfacing a toast. Same shape in shared_address_action_sheet._sendToAddress (shared_address_action_sheet.dart:64-69): it returns before Navigator.pop, leaving the sheet open so the "Send" tap appears dead.

3. [non-blocking] isVisuallyLocked is outside the gate. _isUnlocked is isAuthenticated only; when authenticated-but-visually-locked (the privacy overlay during the 5s background grace, auth_wrapper.dart:23) an intent would be consumed under the privacy overlay. In practice the resume path (checkAuthentication) clears the visual lock or forces re-auth, and the grace window is intentional, so this is very likely not exploitable — but a one-line comment noting the deliberate choice would help future readers.

4. [non-blocking] No tests added. This is a security remediation with non-trivial new logic (four gated handlers, a deferral counter with init/dispose symmetry, account capture) and the diff touches zero test files. dart analyze clean + 222 pre-existing tests do not exercise the new gate/deferral/binding. A couple of widget/unit tests (intent-while-locked stays queued then drains on unlock; intent-while-send-in-flight defers then drains on flow exit; submit signs from captured account after a switch) would materially reduce regression risk.

5. [nit] Inconsistent debug logging. home_screen.dart uses quantusDebugPrint while shared_address_action_sheet.dart:65 uses raw debugPrint for the analogous "cannot send regular transfers" case.

6. [nit] Two simultaneously-queued intents interleave awkwardly. If both a payment and a proposal intent are queued, one _drainPendingIntents pass runs _onPaymentIntent (pushes InputAmountScreen) and then _onProposalIntent (switches active account + shows the proposal sheet), because the counter isn't incremented until InputAmountScreen.initState runs on the next frame. L10 still holds (the strategy already captured the account), so this is cosmetic, but the UX is messy. Not worth blocking.

Verification

Stated verification (dart analyze: no issues; flutter test test/unit: 222/222) is accurate as far as it goes and the PR-body claims check out against the diff — the three RegularSendStrategy() call sites are all updated (grep found no un-migrated site), the old effectiveMaxBalanceProvider symbol is fully removed with its sole consumer repointed, and the "already carry their account" claim for EncryptedSendStrategy/MultisigProposeStrategy is correct (home_screen.dart:372,402). The gap is that verification is entirely regression-suite based: none of the 222 tests are new, so the actual M10/L10 behaviors (gate, deferral, account binding) are unverified by automated tests (Finding 4). Manual verification of the deep-link-while-locked and switch-account-mid-send scenarios is not described in the PR body.

---

🤖 AI-assisted review generated with Claude Code

[n13]

why does a normal send involve the keystone provider?

[n13]

Review

Checked out the branch locally, ran flutter analyze and the new/changed test files — all 21 tests pass, and the logic holds up under reading. The fixes for M10 (intent gating behind the lock overlay) and L10 (source account captured at flow start) address the audit findings correctly. The startSendFlow() single-entry-point refactor is a genuine improvement: no provider mutation in widget lifecycle, the double-start guard is race-free (read+set with no await in between), and awaiting the route's popped future covers pop/replacement/removal. Test coverage of the lock-queue/drain, in-flight-drop, and account-switch scenarios is exactly the right shape.

Blocking: CI is red

The Analyze check fails on the format step. These four files are not dart format --line-length=120 clean:

• lib/features/components/shared_address_action_sheet.dart
• lib/providers/wallet_providers.dart
• lib/v2/screens/home/home_screen.dart
• test/screens/home_intent_gating_test.dart

dart format lib test --line-length=120 in mobile-app fixes it.

Should fix: unused imports

flutter analyze reports two unused imports added by this PR:

• lib/v2/screens/send/input_amount_screen.dart:16 — send_providers.dart
• lib/v2/screens/send/select_recipient_screen.dart:25 — send_providers.dart

Non-blocking observations

• shared_address_action_sheet.dart:75 — the active is! RegularAccount check conflates "accounts still loading" (value == null) with "wrong account type". On a cold start, a fast tap on "Send To This Account" would show "Switch to a regular account to send" when really nothing is loaded yet. Very unlikely in practice (home screen gates on account load), but a null → return early branch would make it precise.
• _onTransactionIntent is deliberately not gated on sendFlowActiveProvider, so a transaction notification can open a detail sheet over a send flow. That's safe now that the source account is captured, and it neither switches accounts nor starts a send — assuming this is intentional, fine; a one-line comment saying so would help.
• The stale-balance-on-refresh-error change in effectiveBalanceProviderFamily is a good trade for spendable-balance checks mid-send; errors before any successful fetch still propagate, and the test pins both paths.
• Dropping (rather than queuing) payment/shared/proposal intents during an active send is the right call for a security fix; it's logged via quantusPrint, so support can diagnose "my deep link did nothing".

Verdict: Request changes — trivial

The substance is correct and well-tested. The only blockers are mechanical: run dart format --line-length=120 and delete the two unused imports so CI goes green. The null-vs-wrong-type edge in the shared-address sheet is worth a follow-up but shouldn't hold the merge.

(Posted as a comment because GitHub doesn't allow requesting changes on your own PR.)

[n13]

Review verdict: ❌ Request changes

The L10 account capture is complete and the targeted tests pass, but this head is not ready to merge because M10 still has a resume-time lock race and CI has two independent failures.

Blocking findings

1. M10 can still consume intents while the app is visually locked during resume.

   HomeScreen._isUnlocked checks only localAuthProvider.isAuthenticated. After the app has been backgrounded, LocalAuthController can be in the state isAuthenticated == true and isVisuallyLocked == true. On resume, AppLifecycleManager calls checkAuthentication without awaiting it, and checkAuthentication awaits shouldRequireAuthentication before it clears isAuthenticated. An app-link or notification callback delivered in that window passes the current gate, clears its intent provider, and navigates beneath the overlay; only afterward can authentication flip to false. This recreates the behavior M10 is intended to remove for background sessions that require re-authentication.

   Please block intent consumption while isVisuallyLocked is true (or expose an equivalent explicit auth-check-pending state), and drain when the visual lock clears even when isAuthenticated stays true for a grace-period resume. Add a test for the isAuthenticated == true / isVisuallyLocked == true state transitioning to unlocked; the current test only covers isAuthenticated == false.

2. The PR does not pass repository checks.

   The GitHub Analyze job fails formatting. The exact local check reports four files requiring dart format:

   • lib/features/components/shared_address_action_sheet.dart
   • lib/providers/wallet_providers.dart
   • lib/v2/screens/home/home_screen.dart
   • test/screens/home_intent_gating_test.dart

   After that step, flutter analyze also fails on unused send_providers.dart imports in input_amount_screen.dart:16 and select_recipient_screen.dart:25. Remove those imports and rerun formatting/analyze.

Verification

Reviewed head 8d7b57c. The five directly affected test files pass locally: 21/21 tests.

[n13]

Re-review verdict: Approve ✅

Re-checked at d563d1be ("fix: gate intents through visual lock and restore CI").

All findings from the previous review are resolved:

• Formatting — dart format lib test --line-length=120 --set-exit-if-changed is clean locally, and the Analyze CI check now passes on this head.
• Unused imports — both removed (input_amount_screen.dart, select_recipient_screen.dart); flutter analyze reports no issues.
• Tests — all 22 pass locally, including the new visual-lock test.

The new commit also strengthens the M10 fix beyond what was asked: _isUnlocked now requires isAuthenticated && !isVisuallyLocked, so intents stay queued beneath the visual privacy overlay too, with a dedicated test pinning the queue→drain behavior. The listener's prev/next comparison correctly drains only on a locked→unlocked transition.

One leftover nit from the first round, still non-blocking: shared_address_action_sheet.dart treats "accounts still loading" (activeAccountProvider.value == null) the same as "wrong account type", showing "Switch to a regular account to send" — fine to handle in a follow-up.

LGTM — good to merge.