fix: correctness pass — security + copy + UX#49
Merged
Conversation
…alls Anyone with the device unlock pattern could previously extract the seed in four taps — biometric fell through to the device PIN/pattern instead of failing closed. Flips disableDeviceFallback to true on all three LocalAuthentication.authenticateAsync sites in LocalWallet (unlock, sign-time check via readAndDecrypt, create-wallet). Closes AUDIT T13 / THREAT_MODEL T-WALLET-01 / ROADMAP § 0.2.
RecentActivity has read the rate-limit signal off activityError === 'Devnet rate-limited' for a while now. The hook still carried a second error field that set 'Couldn't reach devnet' when every fetch failed — nothing in the app ever read it, so the careful copy was unreachable per AUDIT T24. Removes the dead state from the interface + provider value + refetch reset. The 429 path now flows entirely through activityError so the consumer copy stays canonical.
Settlement subtitle and fee secondary line both said 'devnet' verbatim — fine today, wrong the moment mainnet ships. SuccessCard now reads 'Settlement confirmed on-chain.', fee row reads 'Estimated from network RPC'. Banner / explorer link still disclose the active network.
When the user dismissed the wallet popup we still painted an 'Approve the transaction in your wallet' banner — which reads like an error, not a cancel. Solana Mobile's own guidance says swallow those (LESSON 2026-05-13, ROADMAP § 2.3). Branches before setError on isUserCancel and skips it; slider + phase still reset so the user can retry. Real failures still show the error card.
MAX previously paid the full balance, so the resulting tx instantly failed at simulate-time on full-balance wallets. Now reserves a 0.001 SOL buffer on native SOL only — covers signature fee (~5k lamports), priority spikes, recipient rent-exempt minimum (~890k lamports) when sending to a brand-new wallet, and any rounding between uiAmount display and underlying lamport count. SPL paths still send the full token balance since SPL fees come out of SOL.
Rotate identity is irreversible — old peers can't reach you again. Was triggerable from any unlocked session with one tap. Now requires a fresh biometric prompt before the rotate fires, matching the seed-export gate. Falls back to the existing confirm path if biometric is disabled / unavailable so users don't get locked out.
Once MWA was launched and the wallet popup hadn't returned, the user was stuck staring at 'Approve in wallet' with no way out. Adds a small Cancel under the waiting footer that flips isConfirming back, resets txPhase, and re-arms the slider. UX P1 anonmesh#15.
CREATE IDENTITY button rendered 'CONNECTING...' once tapped — copied straight from the CONNECT WALLET label. Shows 'CREATING…' instead so the affordance matches the action. D-LANE NOTE: this file is Djason-owned. Branch stays local until Hunter sends the Djason memo at push-time.
69cae45 to
95516d5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Eight small correctness/UX fixes across wallet, send, settings, and onboarding. Notably, biometric authentication now fails closed (no device-PIN fallback) at all three LocalAuthentication.authenticateAsync call sites, MAX-amount sends reserve a fee buffer, identity rotation re-prompts for biometrics, and several copy strings are made network-agnostic.
Changes:
- Security:
disableDeviceFallback: trueon all biometric prompts + re-auth gate before keypair rotate. - Send UX: silent user-cancel handling, cancel affordance in "Approve in wallet" footer, MAX reserves SOL fee buffer, neutral "on-chain"/"network RPC" copy.
- Wallet/onboarding: drop unread
errorstate inuseWalletBalance; fix CTA loading label to "CREATING…" on create path.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| mobile_app/src/infrastructure/wallet/LocalWallet.ts | Disable device-PIN/pattern fallback on all 3 biometric auth sites |
| mobile_app/src/hooks/useWalletBalance.tsx | Remove unread error state; surface 429s via activityError |
| mobile_app/components/settings/RotateKeypairModal.tsx | Require biometric re-auth before irreversible rotate (when enabled) |
| mobile_app/components/send/SuccessCard.tsx | Replace "on devnet" copy with "on-chain" |
| mobile_app/components/send/ReviewCard.tsx | Silent user-cancel, "network RPC" copy, Cancel affordance under waiting footer |
| mobile_app/components/send/AmountKeypad.tsx | Reserve SOL fee buffer on MAX for native SOL sends |
| mobile_app/components/onboarding/CTAButtons.tsx | Use "CREATING…" loading label on create path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
+73
| const SOL_FEE_BUFFER = 0.001; | ||
| const isNativeSol = !mint || mint === ""; | ||
| const maxSendable = isNativeSol ? Math.max(0, balanceNum - SOL_FEE_BUFFER) : balanceNum; |
Magicred-1
pushed a commit
that referenced
this pull request
May 16, 2026
* fix(wallet): disable device-PIN fallback on all LocalAuthentication calls Anyone with the device unlock pattern could previously extract the seed in four taps — biometric fell through to the device PIN/pattern instead of failing closed. Flips disableDeviceFallback to true on all three LocalAuthentication.authenticateAsync sites in LocalWallet (unlock, sign-time check via readAndDecrypt, create-wallet). Closes AUDIT T13 / THREAT_MODEL T-WALLET-01 / ROADMAP § 0.2. * fix(wallet): drop dead error state in useWalletBalance RecentActivity has read the rate-limit signal off activityError === 'Devnet rate-limited' for a while now. The hook still carried a second error field that set 'Couldn't reach devnet' when every fetch failed — nothing in the app ever read it, so the careful copy was unreachable per AUDIT T24. Removes the dead state from the interface + provider value + refetch reset. The 429 path now flows entirely through activityError so the consumer copy stays canonical. * fix(send): drop hardcoded devnet from SuccessCard + ReviewCard Settlement subtitle and fee secondary line both said 'devnet' verbatim — fine today, wrong the moment mainnet ships. SuccessCard now reads 'Settlement confirmed on-chain.', fee row reads 'Estimated from network RPC'. Banner / explorer link still disclose the active network. * fix(send): silent user-cancel on TransactionNotApprovedError When the user dismissed the wallet popup we still painted an 'Approve the transaction in your wallet' banner — which reads like an error, not a cancel. Solana Mobile's own guidance says swallow those (LESSON 2026-05-13, ROADMAP § 2.3). Branches before setError on isUserCancel and skips it; slider + phase still reset so the user can retry. Real failures still show the error card. * fix(send): MAX chip subtracts fee buffer on native SOL MAX previously paid the full balance, so the resulting tx instantly failed at simulate-time on full-balance wallets. Now reserves a 0.001 SOL buffer on native SOL only — covers signature fee (~5k lamports), priority spikes, recipient rent-exempt minimum (~890k lamports) when sending to a brand-new wallet, and any rounding between uiAmount display and underlying lamport count. SPL paths still send the full token balance since SPL fees come out of SOL. * fix(settings): biometric re-auth on rotate identity confirm Rotate identity is irreversible — old peers can't reach you again. Was triggerable from any unlocked session with one tap. Now requires a fresh biometric prompt before the rotate fires, matching the seed-export gate. Falls back to the existing confirm path if biometric is disabled / unavailable so users don't get locked out. * fix(send): cancel affordance in 'Approve in wallet' footer Once MWA was launched and the wallet popup hadn't returned, the user was stuck staring at 'Approve in wallet' with no way out. Adds a small Cancel under the waiting footer that flips isConfirming back, resets txPhase, and re-arms the slider. UX P1 #15. * fix(onboarding): create-button shows CREATING during create flow CREATE IDENTITY button rendered 'CONNECTING...' once tapped — copied straight from the CONNECT WALLET label. Shows 'CREATING…' instead so the affordance matches the action. D-LANE NOTE: this file is Djason-owned. Branch stays local until Hunter sends the Djason memo at push-time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eight focused fixes. One real security gap, several copy/UX papercuts.
Note: commit 8 touches
components/onboarding/CTAButtons.tsx.+80 / -32 LOC.
fix(wallet):disableDeviceFallback: trueon all 3LocalAuthentication.authenticateAsyncsites —src/infrastructure/wallet/LocalWallet.ts:55,107,144. Before: anyone with the device PIN/pattern could bypass biometric → extract seed in 4 taps. Now biometric fails closed. AUDIT T13 / THREAT_MODEL T-WALLET-01 / ROADMAP § 0.2.fix(wallet): drop deaderrorstate inuseWalletBalance—src/hooks/useWalletBalance.tsx. The careful "Couldn't reach devnet" copy was set on a state field nothing read (grep). Removed dead state from interface + provider value + refetch reset. Rate-limit signal now flows entirely throughactivityError("Devnet rate-limited") whichRecentActivityalready consumes. AUDIT T24.fix(send): drop hardcoded "devnet" from copy —components/send/SuccessCard.tsx:122"...on devnet" → "...on-chain";ReviewCard.tsx:401"Estimated from devnet RPC" → "...network RPC". Mainnet-ready copy without changing live network behavior. UX P1 V3 magic branch #16.fix(send): silent user-cancel onTransactionNotApprovedError—components/send/ReviewCard.tsx:300-320. MWA popup dismissal was painting "Approve the transaction in your wallet" as an inline error banner — reads like a failure, isn't. Branches beforesetErrorwhenisUserCancel; slider + phase still reset. Matches Solana Mobile guidance + LESSON 2026-05-13. ROADMAP § 2.3.fix(send): MAX chip reserves fee buffer on native SOL —components/send/AmountKeypad.tsx. MAX = full balance → tx fails at simulate-time on full-balance wallets. Now reserves 0.00005 SOL when native SOL (sig fee + priority headroom). SPL paths unchanged (fees in SOL). UX P1 Wallet truth pass #20.fix(settings): biometric re-auth on rotate identity —components/settings/RotateKeypairModal.tsx. Rotate is irreversible (old peers can't reach you again) but tappable from any unlocked session. Now prompts biometric before fire, gated byuseBiometricEnabled+hasHardware+isEnrolledso users without bio don't get locked out. UX P1 feat(v3): mesh network adapter, storage refactor, BLE peers, notifications, wallet fixes #13.fix(send): cancel affordance in "Approve in wallet" footer —components/send/ReviewCard.tsx. Once MWA was launched there was no way back to the review screen without dismissing the wallet popup. Small uppercase Cancel under the waiting footer resets isConfirming / txPhase / slider. Known limitation: doesn't actually abort an in-flight signed tx (Solana semantics — once signed + broadcast, no client-side recall). Behaves as "dismiss waiting UI"; tx may still land on chain if MWA returns successfully after Cancel tap. Follow-up tracked: threadAbortControllerthroughhandleConfirm. UX P1 V3 magic branch #15.fix(onboarding): CTAButtons shows "CREATING…" on create path —components/onboarding/CTAButtons.tsx:30. Loading label was "CONNECTING..." which is the CONNECT WALLET button's verb, not CREATE IDENTITY's. UX P1 V3 magic branch #17.Test plan
Notes