fix: attempt at fixing wallet import on mobile #11768
Conversation
…en attempted Add saveAttemptedRef to prevent duplicate wallet saves when both handleViewWallet and handleClose could trigger saveAndSelectWallet. The ref is checked at the start of saveAndSelectWallet and set to true before proceeding with the save. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…) on mount This ensures the wallet is saved immediately when the CreateSuccess screen renders, rather than only when the user clicks the "View wallet" button. The existing saveAttemptedRef prevents duplicate save attempts. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ady been Add hasConnectedRef to prevent duplicate wallet connection attempts in ImportSuccess component. This follows the same pattern used in ManualBackup.tsx. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ction() Adds a useEffect hook that calls handleWalletConnection() on component mount. This ensures the wallet is connected immediately when ImportSuccess mounts, rather than only when the user clicks the close/view wallet button. Duplicate calls are prevented by the existing hasConnectedRef guard in handleWalletConnection(). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughRefactored mobile wallet creation and import flows to automatically persist wallets without requiring explicit user interaction. Introduced React hooks for managing async save/connect operations with ref-based guards against duplicate executions. Migrated wallet list fetching in MobileConnect to React Query for improved state management and caching. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dialog as CreateSuccess/<br/>ImportSuccess Component
participant Storage as Vault/Storage
participant Query as React Query
participant Adapter as Mobile Wallet<br/>Adapter
participant State as Global Wallet<br/>State
Note over Dialog: Component mounts
Dialog->>Dialog: Check saveAttemptedRef
alt Save not attempted
Dialog->>Storage: saveWallet(vault label, mnemonic)
Storage-->>Dialog: Success
Dialog->>Query: Invalidate listWallets cache
Dialog->>Dialog: Set saveAttemptedRef = true
end
Note over Dialog: Initiate connection flow
Dialog->>Dialog: Check connectionAttemptedRef
alt Connect not attempted
Dialog->>Adapter: Connect via Mobile adapter
Adapter-->>State: Update wallet state
Dialog->>State: Mark wallet as connected
Dialog->>Dialog: Set connectionAttemptedRef = true
Dialog->>Dialog: Set isConnecting = false
end
Dialog->>Dialog: Close dialog
User-->>Dialog: (or close dialog manually)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NeOMakinG
changed the title
NeOMakinG
force-pushed
the
auto-claude/010-clicking-off-of-view-wallet-in-create-wallet-flow-
branch
from
91abb9b to
46b43d9
Compare
Removes autoclaude configuration files that shouldn't be tracked in git: - .auto-claude-security.json - .auto-claude-status - .claude_settings.json Reverts .gitignore to remove autoclaude directory entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…et-in-create-wallet-flow-
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/MobileWalletDialog/routes/ImportWallet/ImportSuccess.tsx (1)
52-103: Allow retry and only close on successful connect.
connectionAttemptedRefis set before any async work and never reset. SincehandleWalletConnectionswallows errors,handleViewWalletwill still close on failure and subsequent taps become no-ops, leaving the wallet unconnected with no retry path.✅ Suggested fix
- const handleWalletConnection = useCallback(async () => { - if (connectionAttemptedRef.current) return - if (!location.state?.vault) return - connectionAttemptedRef.current = true - const adapter = await getAdapter(KeyManager.Mobile) - if (!adapter) throw new Error('Native adapter not found') - try { + const handleWalletConnection = useCallback(async () => { + if (connectionAttemptedRef.current) return false + if (!location.state?.vault) return false + connectionAttemptedRef.current = true + try { + const adapter = await getAdapter(KeyManager.Mobile) + if (!adapter) throw new Error('Native adapter not found') const deviceId = location.state.vault.id ?? '' const wallet = (await adapter.pairDevice(deviceId)) as NativeHDWallet const mnemonic = location.state.vault.mnemonic @@ - appDispatch(setWelcomeModal({ show: true })) + appDispatch(setWelcomeModal({ show: true })) + return true } } catch (e) { console.log(e) + connectionAttemptedRef.current = false + return false } }, [getAdapter, location.state?.vault, dispatch, localWallet, appDispatch, setWelcomeModal]) const handleViewWallet = useCallback(async () => { setIsConnecting(true) try { - await handleWalletConnection() - onClose() - setTimeout(() => location.state?.vault?.revoke(), 500) + const didConnect = await handleWalletConnection() + if (!didConnect) return + onClose() + setTimeout(() => location.state?.vault?.revoke(), 500) } finally { setIsConnecting(false) } }, [onClose, handleWalletConnection, location.state?.vault])
🤖 Fix all issues with AI agents
In `@src/components/MobileWalletDialog/routes/CreateWallet/CreateSuccess.tsx`:
- Around line 40-55: Wrap the save logic in saveWallet in a try-catch: try the
existing checks and calls to addWallet and queryClient.invalidateQueries, and in
catch handle errors by resetting saveAttemptedRef.current (so a retry is
possible), log the error (or surface it via the app's notification/toast/error
state) and ensure any created wallet is revoked if partially created; keep the
existing success flow (invalidateQueries and wallet.revoke()) inside the try
block. Use the existing symbols saveWallet, addWallet,
queryClient.invalidateQueries, saveAttemptedRef, and location.state?.vault to
locate where to add the try-catch and error handling.
- Around line 40-55: saveWallet currently calls addWallet without error handling
and handleWalletConnection calls addWallet again, risking duplicates; change
saveWallet to wrap the addWallet call in try/catch, log or show an error toast
on failure, and store the resulting wallet reference in component state (e.g.,
useState walletRef) instead of only using saveAttemptedRef; update
handleWalletConnection to reuse the stored walletRef (and only call addWallet if
walletRef is null) and remove the duplicated addWallet invocation; ensure you
still call queryClient.invalidateQueries(['listWallets']) and wallet.revoke()
when appropriate, and replace the console.log(e) in handleWalletConnection with
structured logging or a user-facing error toast.
In `@src/pages/ConnectWallet/MobileConnect.tsx`:
- Around line 91-101: The current useMemo for error (involving queryError,
wallets.length, isLoading) sometimes returns queryError.message (a raw string)
which is passed into <Text translation={error} />; change the logic in the
useMemo so it always returns a translation key (e.g.
'walletProvider.shapeShift.load.error.fetchingWallets' or existing noWallet key)
rather than raw Error.message, and if you still want the raw error for
debugging, log queryError (when instanceof Error) via console.error or process
logger inside the same branch; update references to useMemo, queryError, wallets
and isLoading accordingly so <Text translation={error} /> always receives a
translation key.
🧹 Nitpick comments (2)
src/pages/ConnectWallet/MobileConnect.tsx (1)
78-89: Consider implications ofgcTime: 0on user experience.Setting
gcTime: 0means the query cache is garbage collected immediately when the component unmounts. Combined withstaleTime: 0, this effectively disables React Query's caching benefits entirely. If a user navigates away briefly and returns, they'll see a full loading state again instead of instant data with a background refetch.If the intent is to ensure fresh data on every mount (which makes sense for a wallet list), consider keeping
staleTime: 0but using a smallgcTime(e.g.,gcTime: 5000) to avoid unnecessary loading flickers during quick navigation transitions.💡 Optional: Allow brief cache retention for smoother UX
const { isLoading, data: wallets = [], error: queryError, } = useReactQuery({ queryKey: ['listWallets'], staleTime: 0, - gcTime: 0, + gcTime: 5000, // Keep cache briefly to avoid loading flicker on quick re-mounts retry: false, refetchOnMount: true, queryFn: listWallets, })src/components/MobileWalletDialog/routes/CreateWallet/CreateSuccess.tsx (1)
101-103: Use structured logging instead ofconsole.log.Per coding guidelines, errors should use structured logging with relevant context. Consider using
console.errorwith meaningful context or the project's logger utility.♻️ Suggested improvement
- } catch (e) { - console.log(e) - } + } catch (e) { + console.error('[CreateSuccess] Failed to connect wallet:', e) + }
0xApotheosis
left a comment
0xApotheosis
left a comment
There was a problem hiding this comment.
I think CodeRabbit is onto something with the comment below.
✅ Tapping outside the final modal correctly saves the wallet
✅ Tapping the X on the final modal correctly saves the wallet
❌ Clicking "View Wallet" creates two wallets
❌ One time I clicked "View Wallet" and it crashed the dev server
ScreenRecording_01-27-2026.14-45-40_1.MP4
…et-in-create-wallet-flow-
I probably broke it after testing by cleaning up (I can be stupid) It should be better now! Also took care of safely revoking either ways! |
Description
Attempt at fixing the wallet import when quitting by closing the drawer instead of going to the wallet view
Issue (if applicable)
closes #11142
Risk
Low
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/b0810ba5-348a-42a6-a3a7-f539f41e2d6b
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.