Chore/3034123 - #664
Chore/3034123#664
Conversation
leofelix077
added
wip
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new LOCKED authentication status to improve the user experience when locking/unlocking the wallet app. The change distinguishes between session expiration (HASH_KEY_EXPIRED) and intentional app locking (LOCKED), preserving the temporary store and private keys during lock to enable faster unlocking.
Changes:
- Added
LOCKEDauth status that preserves encrypted temporary store during logout - Implemented lazy loading of private keys on-demand instead of pre-loading all keys during sign-in
- Optimized account discovery to skip network calls for accounts already stored locally
- Added loading state during account switching to prevent showing stale data
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config/types.ts | Added new LOCKED auth status constant |
| src/config/constants.ts | Added AUTH_STATUS storage key for persisting lock state |
| src/ducks/auth.ts | Core authentication logic changes including LOCKED state handling, lazy private key loading, account switching improvements, and optimized account discovery |
| src/navigators/RootNavigator.tsx | Updated routing logic to handle LOCKED status (includes debugging console.log statements) |
| src/providers/WalletKitProvider.tsx | Added LOCKED status checks for wallet connection and transaction signing |
| src/hooks/useAuthCheck.ts | Updated to skip auth checks when in LOCKED state |
| src/hooks/useWelcomeBanner.ts | Added account switching tracking to prevent showing welcome banner during transitions |
| src/ducks/history.ts | Skip history fetching for unfunded accounts (performance optimization) |
| src/components/screens/HomeScreen/HomeScreen.tsx | Display loading screen during account switching |
| src/i18n/locales/en/translations.json | Added error message for missing mnemonic phrase |
| src/i18n/locales/pt/translations.json | Added Portuguese translation for missing mnemonic phrase error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // User is locked. Hash key is expired but temporary store with private keys is preserved. | ||
| LOCKED: "LOCKED", |
There was a problem hiding this comment.
The new LOCKED auth status and its associated behaviors (preserving temporary store on logout, handling unlock flow with shouldCreateTempStore parameter, lazy loading of private keys) lack test coverage. The existing test file tests/ducks/auth.test.ts should be updated to include tests for: 1) logout with LOCKED state preservation, 2) sign-in from LOCKED state, 3) getAuthStatus returning LOCKED when persisted, 4) lazy loading of private keys in getActiveAccount, and 5) account switching with the isSwitchingAccount flag.
| // Track when account switching completes and balances are loaded | ||
| useEffect(() => { | ||
| if (!isSwitchingAccount && !isLoadingBalances) { | ||
| setAccountSwitchCompleted(true); | ||
| } else if (isSwitchingAccount) { | ||
| setAccountSwitchCompleted(false); | ||
| } | ||
| }, [isSwitchingAccount, isLoadingBalances]); |
There was a problem hiding this comment.
The new isSwitchingAccount parameter and accountSwitchCompleted state logic in useWelcomeBanner lack test coverage. Since other hooks in this codebase have comprehensive tests (as evidenced by tests/hooks/ directory), tests should be added to verify: 1) accountSwitchCompleted is set correctly based on isSwitchingAccount and isLoadingBalances, 2) welcome banner is not shown during account switching, and 3) welcome banner is shown after account switch completes.
…nto chore/3034123
|
@CassioMG I adjusted the comments above. was gonna reply one by one, but decided to keep it in a single place Adjusted the history refresh on account switch and importing new wallets. it was not clearing in all cases before; added a UI block when user presses to switch accounts + auto dismiss the modal, so it cannot be pressed multiple times causing this weird in-between state; adjusted the code to only clear the session data from the snippet only during full logout / new wallet import no new findings from my side after testing on iOS Simulator + android device. let me know if can check again and if you find anything else. thanks a lot for checking it as well! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Wait for all async operations to complete | ||
| await act(async () => { | ||
| await new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| resolve(undefined); | ||
| }, 100); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
These tests rely on real setTimeout(..., 100/200) delays to let effects run. This slows the suite and can be flaky under CI load. The repo already uses jest.useFakeTimers() in other tests; consider switching to fake timers here and advancing time deterministically (and/or using waitFor).
| if (shouldCreateTempStore) { | ||
| // Fresh login: Generate new hash key and create new temporary store | ||
| const newHashKey = await generateHashKey(password); | ||
| await secureDataStorage.setItem( | ||
| SENSITIVE_STORAGE_KEYS.HASH_KEY, | ||
| JSON.stringify(newHashKey), | ||
| ); | ||
|
|
||
| await createTemporaryStore({ | ||
| password, | ||
| mnemonicPhrase: keyExtraData.mnemonicPhrase, | ||
| activeKeyPair: { | ||
| publicKey: loadedKey.publicKey, | ||
| privateKey: loadedKey.privateKey, | ||
| accountName: account.name, | ||
| id: loadedKey.id, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
In signIn (fresh login path), a new hash key is generated and persisted, but createTemporaryStore() will generate and persist another hash key again (default shouldRefreshHashKey=true). This results in redundant/expensive key derivation on every login and makes the first newHashKey write unused. Consider letting createTemporaryStore be the single place that generates+persists the hash key, or pass shouldRefreshHashKey: false and reuse the already-generated hash key (by extending the helper signature).
|
@leofelix077 it looks like the main issues are fixed now, the only thing I noticed is that now the account switch is feeling "laggy" on iOS because of the Could we fix it somehow? I have 2 possible suggestions in mind:
Wdyt? Of course those are only suggestions from the top of my head, feel free to use another solution you may think is a better fit ScreenRecording_02-10-2026.14-06-30_1.MP4 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…nto chore/3034123
|
@CassioMG I tried with the full screen loading and the inline, directly on the row. spinner on the entire modal level dosnt look too good, kind of off centered. I liked the 2nd one best. wdyt? Screen.Recording.2026-02-17.at.14.36.14.movScreen.Recording.2026-02-17.at.15.39.27.mov |
@leofelix077 agree, I think the second option is looking good. In addition to displaying the spinner over the selected row, could we also decrease opacity of the selected row? Indicating that it has been selected/disabled E.g. the row in orange below should have lower opacity: Wdyt? |
|
oh yes. think it looks smoother imo Screen.Recording.2026-02-17.at.16.39.19.mov |
@leofelix077 nice, thanks for adjusting it 👌 Could you review this Copilot comment to check if it's applicable? I'll request Copilot review again to see if it finds something else Other than that, I think we should be good to merge this PR |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| modalRef={bottomSheetRef} | ||
| handleCloseModal={handleCloseModal} | ||
| enablePanDownToClose={false} | ||
| enablePanDownToClose |
There was a problem hiding this comment.
The enablePanDownToClose property was changed from false to true (line 135). This means users can now swipe down to dismiss the account management bottom sheet even while an account switch is in progress.
However, the dismissal logic at lines 109-112 already handles the case where account switching completes, so allowing manual dismissal during switching could create a race condition where the sheet is dismissed before the switch completes. Consider keeping enablePanDownToClose={false} or conditionally disabling it based on isAccountSwitching: enablePanDownToClose={!isSwitchingAccount}
| enablePanDownToClose | |
| enablePanDownToClose={!isSwitchingAccount} |
| // LOCKED state unlock: Generate new hash key and re-encrypt existing temporary store | ||
| // IMPORTANT: Generate key first but don't store it yet - reEncryptTemporaryStore | ||
| // needs the OLD hash key to decrypt before we can re-encrypt with the new one | ||
| const newHashKey = await generateHashKey(password); | ||
| await reEncryptTemporaryStore(newHashKey); | ||
|
|
||
| // Now store the new hash key (reEncryptTemporaryStore used it for encryption | ||
| // but retrieved the old key itself via getHashKey() for decryption) | ||
| await secureDataStorage.setItem( | ||
| SENSITIVE_STORAGE_KEYS.HASH_KEY, | ||
| JSON.stringify(newHashKey), | ||
| ); |
There was a problem hiding this comment.
The shouldCreateTempStore parameter logic has an issue. When unlocking from LOCKED state (shouldCreateTempStore = false), the code calls reEncryptTemporaryStore(newHashKey) at line 1313. However, reEncryptTemporaryStore needs the OLD hash key from storage to decrypt the temporary store. The problem is that at line 1312, a new hash key has been generated but not stored yet, which is correct. But there's no guarantee that the old hash key is still in storage and not expired, especially if the user took a long time to unlock.
Consider adding a validation check to ensure the old hash key exists and is valid before attempting re-encryption. If it's missing or expired, fall back to creating a new temporary store instead.
| // LOCKED state unlock: Generate new hash key and re-encrypt existing temporary store | |
| // IMPORTANT: Generate key first but don't store it yet - reEncryptTemporaryStore | |
| // needs the OLD hash key to decrypt before we can re-encrypt with the new one | |
| const newHashKey = await generateHashKey(password); | |
| await reEncryptTemporaryStore(newHashKey); | |
| // Now store the new hash key (reEncryptTemporaryStore used it for encryption | |
| // but retrieved the old key itself via getHashKey() for decryption) | |
| await secureDataStorage.setItem( | |
| SENSITIVE_STORAGE_KEYS.HASH_KEY, | |
| JSON.stringify(newHashKey), | |
| ); | |
| // LOCKED state unlock: attempt to re-encrypt existing temporary store | |
| // using the old hash key (from storage) and a newly generated hash key. | |
| // If the old hash key is missing or expired, fall back to creating a new | |
| // temporary store instead. | |
| const existingHashKey = await getHashKey(); | |
| const isExistingHashKeyValid = | |
| !!existingHashKey && | |
| typeof existingHashKey.createdAt === "number" && | |
| Date.now() - existingHashKey.createdAt < HASH_KEY_EXPIRATION_MS; | |
| if (isExistingHashKeyValid) { | |
| // Old hash key is still valid: re-encrypt the existing temporary store | |
| // with a newly generated hash key. | |
| const newHashKey = await generateHashKey(password); | |
| await reEncryptTemporaryStore(newHashKey); | |
| // Now store the new hash key (reEncryptTemporaryStore used it for encryption | |
| // but retrieved the old key itself via getHashKey() for decryption) | |
| await secureDataStorage.setItem( | |
| SENSITIVE_STORAGE_KEYS.HASH_KEY, | |
| JSON.stringify(newHashKey), | |
| ); | |
| } else { | |
| // Old hash key missing or expired: fall back to creating a new temporary store, | |
| // same behavior as the fresh login path. | |
| const newHashKey = await generateHashKey(password); | |
| await secureDataStorage.setItem( | |
| SENSITIVE_STORAGE_KEYS.HASH_KEY, | |
| JSON.stringify(newHashKey), | |
| ); | |
| await createTemporaryStore({ | |
| password, | |
| mnemonicPhrase: keyExtraData.mnemonicPhrase, | |
| activeKeyPair: { | |
| publicKey: loadedKey.publicKey, | |
| privateKey: loadedKey.privateKey, | |
| accountName: account.name, | |
| id: loadedKey.id, | |
| }, | |
| shouldRefreshHashKey: false, | |
| }); | |
| } |
Main pain points:
Login taking forever - users waiting ~10-13s just to get into the app. Found two main culprits: network account discovery running on every login (~6-10s depending on network), and upfront derivation of all private keys (~7-9s)
Unnecessary network calls - account discovery was checking for funded accounts every single login even though the accounts are deterministic (same seed = same accounts). Extension only does this on wallet import, not login
Logout wiped everything - hitting logout cleared all the decrypted data, so next login had to redo all the expensive crypto operations from scratch. No "lock" concept like the extension has
Related findings while adjusting
Account switching showed stale data - when switching accounts you'd briefly see the old account's balances/history before new data loaded. Confusing and looked broken
Unfunded accounts spamming errors - newly created accounts that don't exist on-chain yet were still making history API calls that would just 400
Main Changes
Only discover accounts on first wallet import - cuts it from ~10s (depends on network) to under 1s for normal logins. Discovery still runs once when importing a wallet to find your funded accounts, just skips it on regular logins
Added LOCKED state - logout now locks instead of nuking everything. Encrypted keys stay on disk so unlocking takes ~0.5s instead of the full re-auth (~10-13s). Stays locked even if you close/reopen the app
Lazy key loading - private keys load on-demand when you first open an account instead of deriving all of them at login. Saves ~7-9s, and once cached switching back is instant
Clears stale data on account switch - immediately wipes old balances/history/prices when switching so you don't see the wrong account's data for a second while new stuff loads
Skips history calls for unfunded accounts - new accounts don't exist on-chain yet so we just skip the API call that would 400 anyway
Overall login - went from ~10-13s to ~0.5-1s (roughly 90-95% faster)
Account switching - stays fast at ~0.5-0.6s with cached keys, no network calls (apart from fetching balances and history) or re-derivation
Checklist
PR structure
Testing
Release