From 038133d5c584c66c3732f75c2e6d7c4d488f2b92 Mon Sep 17 00:00:00 2001 From: khanti42 Date: Wed, 12 Nov 2025 01:16:16 +0100 Subject: [PATCH 1/2] chore: bump network-enablement-controller cp-7.59.0 (#22492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps the network-enablement-controller to version 3.1.0 that includes monad in the constant. This allows monad to be selected when clicking on the `All popular networks` button in the Networks Modal. ## **Changelog** CHANGELOG entry: bumps network-enablement-controller and apply patch similar to patch on 3.0.0 ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** Screenshot 2025-11-11 at 17 01 05 ### **After** Screenshot 2025-11-11 at 17 11 43 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Updates @metamask/network-enablement-controller to 3.1.0 with a patch that enables additional EVM networks by default. > > - **Dependencies**: > - Bump `@metamask/network-enablement-controller` from `3.0.0` to `3.1.0` using a Yarn patch. > - **Network defaults** (`dist/NetworkEnablementController.cjs`): > - Extend default-enabled EVM networks by adding `ArbitrumOne`, `BscMainnet`, `OptimismMainnet`, `PolygonMainnet`, and `SeiMainnet`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b41bb09268ae0fb30fcbe78b8ff8fdc798d6aa5b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- ...ement-controller-npm-3.1.0-1c0cfefdc3.patch | 16 ++++++++++++++++ package.json | 2 +- yarn.lock | 18 +++++++++--------- 3 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 .yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch diff --git a/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch b/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch new file mode 100644 index 00000000000..7d1746545fc --- /dev/null +++ b/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch @@ -0,0 +1,16 @@ +diff --git a/dist/NetworkEnablementController.cjs b/dist/NetworkEnablementController.cjs +index d4a40bea9e4ed3c28e347d96e309efe1ff889e81..fab280760de6bd5cdfdbecf01495c2d5616b2e16 100644 +--- a/dist/NetworkEnablementController.cjs ++++ b/dist/NetworkEnablementController.cjs +@@ -25,6 +25,11 @@ const getDefaultNetworkEnablementControllerState = () => ({ + [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.Mainnet]]: true, + [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.LineaMainnet]]: true, + [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.BaseMainnet]]: true, ++ [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.ArbitrumOne]]: true, ++ [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.BscMainnet]]: true, ++ [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.OptimismMainnet]]: true, ++ [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.PolygonMainnet]]: true, ++ [controller_utils_1.ChainId[controller_utils_1.BuiltInNetworkName.SeiMainnet]]: true, + }, + [utils_1.KnownCaipNamespace.Solana]: { + [keyring_api_1.SolScope.Mainnet]: true, diff --git a/package.json b/package.json index 098b9a5732b..0b641e89271 100644 --- a/package.json +++ b/package.json @@ -250,7 +250,7 @@ "@metamask/multichain-transactions-controller": "^6.0.0", "@metamask/native-utils": "^0.5.0", "@metamask/network-controller": "^25.0.0", - "@metamask/network-enablement-controller": "patch:@metamask/network-enablement-controller@npm%3A3.0.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.0.0-cfba64ad39.patch", + "@metamask/network-enablement-controller": "patch:@metamask/network-enablement-controller@npm%3A3.1.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch", "@metamask/notification-services-controller": "^19.0.0", "@metamask/permission-controller": "^12.1.0", "@metamask/phishing-controller": "^15.0.0", diff --git a/yarn.lock b/yarn.lock index 8130ae686f4..dbf75cfc686 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8057,9 +8057,9 @@ __metadata: languageName: node linkType: hard -"@metamask/network-enablement-controller@npm:3.0.0": - version: 3.0.0 - resolution: "@metamask/network-enablement-controller@npm:3.0.0" +"@metamask/network-enablement-controller@npm:3.1.0": + version: 3.1.0 + resolution: "@metamask/network-enablement-controller@npm:3.1.0" dependencies: "@metamask/base-controller": "npm:^9.0.0" "@metamask/controller-utils": "npm:^11.14.1" @@ -8071,13 +8071,13 @@ __metadata: "@metamask/multichain-network-controller": ^2.0.0 "@metamask/network-controller": ^25.0.0 "@metamask/transaction-controller": ^61.0.0 - checksum: 10/1e44c1a13d02f1c68601aea308c915d897f7df344e04954b2c7ba7cc0400c81bbab75526572085d17279014b0510bd2a081d828468c5468cbf84c8a4ab52498e + checksum: 10/3cb56dd859580e7fb613677c193cc4af377e59e9d76b86ae54c71b0e732dffbdd41b96825de2413bce7f1c57184c1bfed6dc1070641d7848d47ae559f2f915c5 languageName: node linkType: hard -"@metamask/network-enablement-controller@patch:@metamask/network-enablement-controller@npm%3A3.0.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.0.0-cfba64ad39.patch": - version: 3.0.0 - resolution: "@metamask/network-enablement-controller@patch:@metamask/network-enablement-controller@npm%3A3.0.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.0.0-cfba64ad39.patch::version=3.0.0&hash=0805d0" +"@metamask/network-enablement-controller@patch:@metamask/network-enablement-controller@npm%3A3.1.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch": + version: 3.1.0 + resolution: "@metamask/network-enablement-controller@patch:@metamask/network-enablement-controller@npm%3A3.1.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch::version=3.1.0&hash=0805d0" dependencies: "@metamask/base-controller": "npm:^9.0.0" "@metamask/controller-utils": "npm:^11.14.1" @@ -8089,7 +8089,7 @@ __metadata: "@metamask/multichain-network-controller": ^2.0.0 "@metamask/network-controller": ^25.0.0 "@metamask/transaction-controller": ^61.0.0 - checksum: 10/3bbb6a13e2f1c08df6f79cbe5d619df20524e13e986307b2fb120e4950f3244b039a0f93710c1cfe9ce4b42b39f7a02d943bba7a93eaaa895e081af5324cfea4 + checksum: 10/97b00477ec1550b19c7863991cd377ae73936ac466faf149cd2903325a28462f50647cdce9cd9c7aee4395e6cbf0aec8d5417012942c595d8aa3bb69682e8dc9 languageName: node linkType: hard @@ -34291,7 +34291,7 @@ __metadata: "@metamask/multichain-transactions-controller": "npm:^6.0.0" "@metamask/native-utils": "npm:^0.5.0" "@metamask/network-controller": "npm:^25.0.0" - "@metamask/network-enablement-controller": "patch:@metamask/network-enablement-controller@npm%3A3.0.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.0.0-cfba64ad39.patch" + "@metamask/network-enablement-controller": "patch:@metamask/network-enablement-controller@npm%3A3.1.0#~/.yarn/patches/@metamask-network-enablement-controller-npm-3.1.0-1c0cfefdc3.patch" "@metamask/notification-services-controller": "npm:^19.0.0" "@metamask/object-multiplex": "npm:^1.1.0" "@metamask/permission-controller": "npm:^12.1.0" From af573905351cebffe32d6aa2b7579328b1034b0a Mon Sep 17 00:00:00 2001 From: Aslau Mario-Daniel Date: Wed, 12 Nov 2025 04:10:03 +0000 Subject: [PATCH 2/2] fix: New Persistence Improvements based on Abuse testing cp-7.59.0 (#21990) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** When Redux state migrations fail, redux-persist resets the state to defaults and updates the version number to the latest, preventing migrations from re-running. This leaves users stuck on the onboarding screen unable to access their wallets, even though their vault backup exists in secure storage. ## Implementation ### 1. Vault Recovery Detection on Onboarding Screen Added automatic detection when users land on the onboarding screen with an existing vault backup: - Detects migration failure scenario: `!existingUser` (Redux reset) + vault backup exists - Skips detection if user explicitly deleted their wallet (`route.params.delete`) - Automatically navigates to vault recovery screen - Users can restore their wallet using their password - Prevents data loss from failed migrations **Files Changed:** - `app/components/Views/Onboarding/index.js` - Added vault recovery detection - `app/components/Views/RestoreWallet/WalletRestored.tsx` - Sets `existingUser` flag after restore ### 2. Fixed Vault Recovery Persistence Bug Fixed critical bug where vault recovery didn't persist controller state changes: - Added `setupEnginePersistence()` call in `initializeVaultFromBackup()` path - Ensures controller state changes are saved to individual files after vault recovery - Previously caused infinite vault recovery loops after successful restore - Consolidated persistence setup in `initializeControllers()` for consistency **Files Changed:** - `app/core/EngineService/EngineService.ts` - Fixed persistence setup in vault recovery path ### 3. Fixed Race Condition in Wallet Deletion Prevents temporary wallets (created during reset) from being backed up: - Added `disableAutomaticVaultBackup` flag to temporarily disable vault backups during wallet reset - Clears all vault backups before creating temporary wallet - Re-enables automatic backups in `finally` block for robustness - Applies to both manual wallet deletion and OAuth error recovery - Prevents false vault recovery prompts after intentional wallet resets **Files Changed:** - `app/core/Engine/Engine.ts` - Added circuit breaker flag - `app/components/hooks/DeleteWallet/useDeleteWallet.ts` - Manual deletion fix - `app/core/Authentication/Authentication.ts` - Uncommented `setExistingUser(true)` for new wallets - `app/core/BackupVault/backupVault.ts` - Vault backup utilities ### 4. Fixed Migration Inflation/Deflation Logic Corrected conditions for the new file-based persistence system (migrations 104-105): - **Inflation**: `> 106` - Only migrations 106+ need to inflate from individual controller files - **Deflation**: `>= 106` - Migration 106 is the first to deflate into individual controller files - Only future migrations require inflation, that will follow this PR - Ensures smooth transition to new persistence system without breaking app on restart **Files Changed:** - `app/store/migrations/index.ts` - Fixed inflation/deflation conditions and removed debug logs - `app/store/migrations/index.test.ts` - Updated tests to reflect correct logic ### 5. Code Cleanup Removed all debugging logs added during investigation: - `app/components/Views/Onboarding/index.js` - Removed migration detection logs - `app/core/EngineService/EngineService.ts` - Removed persistence setup logs - `app/store/migrations/index.ts` - Removed KeyringController debug logs - `app/core/BackupVault/backupVault.ts` - Simplified error handling - `app/core/Engine/Engine.ts` - Removed vault backup logs ### 6. Test Compliance Fixed unit test naming violations to comply with project guidelines: - Updated 19 tests in `EngineService.test.ts` to remove "should" from test names - Ensured all tests follow action-oriented naming convention ## **Changelog** CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` **Feature: Vault Recovery After Migration Failure** Scenario: user restores wallet after migration failure Given user has MetaMask installed with wallet data And a state migration fails during app startup And redux state is reset to defaults And vault backup exists in secure storage When user opens the app Then user sees the vault recovery screen automatically When user enters their wallet password Then user successfully restores their wallet And user can access their accounts and assets **Feature: Manual Wallet Deletion** Scenario: user deletes wallet without false vault recovery Given user has MetaMask installed with an active wallet And user is on the Settings screen When user navigates to "Security & Privacy" And user taps "Delete Wallet" And user confirms the deletion And user restarts the app Then user sees the onboarding screen And user does NOT see the vault recovery screen **Feature: OAuth Login Error Recovery** Feature: Vault Recovery After Migration Failure Scenario: user restores wallet after migration failure Given user has MetaMask installed with wallet data And a state migration fails during app startup And redux state is reset to defaults And vault backup exists in secure storage When user opens the app Then user sees the vault recovery screen automatically When user enters their wallet password Then user successfully restores their wallet And user can access their accounts and assets **Feature: Manual Wallet Deletion** Scenario: user deletes wallet without false vault recovery Given user has MetaMask installed with an active wallet And user is on the Settings screen When user navigates to "Security & Privacy" And user taps "Delete Wallet" And user confirms the deletion And user restarts the app Then user sees the onboarding screen And user does NOT see the vault recovery screen **Feature: OAuth Login Error Recovery** Scenario: user experiences OAuth backup failure without false vault recovery Given user is creating a new wallet with OAuth (Google/Apple login) And local wallet creation succeeds And cloud backup fails during OAuth process When the error handler resets the wallet state And user restarts the app Then user sees the onboarding screen And user does NOT see the vault recovery screen And user can retry OAuth login or import with seed phrase **Feature: Migration System Upgrade Path** Scenario: user upgrades from version 103 to version 104+ Given user has MetaMask version with migrations up to 103 And controller data is stored in redux-persist And user has an active wallet When user upgrades to version 104 or higher Then migrations 104+ run successfully And controller data is deflated to individual files And user's wallet remains accessible And app functions normally after restart Scenario: user upgrades from version 104 to version 105+ Given user has MetaMask version 104 And controller data is in individual files And user has an active wallet When user upgrades to version 105 or higher Then controller data is inflated for migrations And new migrations run successfully And controller data is deflated back to individual files And user's wallet remains accessible And app functions normally after restart**: user experiences OAuth backup failure without false vault recovery Given user is creating a new wallet with OAuth (Google/Apple login) And local wallet creation succeeds And cloud backup fails during OAuth process When the error handler resets the wallet state And user restarts the app Then user sees the onboarding screen And user does NOT see the vault recovery screen And user can retry OAuth login or import with seed phrase **Feature: Migration System Upgrade Path** Scenario: user upgrades from version 103 to version 104+ Given user has MetaMask version with migrations up to 103 And controller data is stored in redux-persist And user has an active wallet When user upgrades to version 104 or higher Then migrations 104+ run successfully And controller data is deflated to individual files And user's wallet remains accessible And app functions normally after restart Scenario: user upgrades from version 104 to version 105+ Given user has MetaMask version 104 And controller data is in individual files And user has an active wallet When user upgrades to version 105 or higher Then controller data is inflated for migrations And new migrations run successfully And controller data is deflated back to individual files And user's wallet remains accessible And app functions normally after restart ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Detects migration failure and routes to vault recovery, ensures controller persistence after recovery, prevents backing up temporary wallets during resets, and updates login/recovery flows with tests. > > - **Onboarding/Recovery**: > - Detects migration failure on `Onboarding` by checking vault backup and `existingUser`; skips in E2E and explicit delete, then navigates to `Routes.VAULT_RECOVERY.RESTORE_WALLET`. > - `WalletRestored` now routes to `Routes.ONBOARDING.LOGIN` with `isVaultRecovery` instead of auto-auth; adds tests. > - `Login` sets `existingUser` via `setExistingUser(true)` after successful unlock when `isVaultRecovery` is true; adds tests. > - **Engine/Authentication**: > - Adds `Engine.disableAutomaticVaultBackup` and skips `backupVault` when true. > - Clears vault backups and temporarily disables auto-backup during wallet reset and OAuth error recovery; always re-enables. > - `EngineService` moves filesystem persistence setup to `initializeControllers` and ensures it runs after vault recovery; updates batching and tests. > - **Hooks/Tests**: > - `useDeleteWallet` clears backups first, disables auto-backup during reset, re-enables in `finally`, and resets controllers; adds robust tests. > - Renames and tightens numerous test titles/expectations; snapshot names updated. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ed1a8a7c80c6d74d4783b2a66cbcc82fda607b05. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- app/components/Views/Login/index.test.tsx | 161 +++++++-- app/components/Views/Login/index.tsx | 14 +- .../__snapshots__/index.test.tsx.snap | 8 +- app/components/Views/Onboarding/index.js | 55 ++++ .../Views/Onboarding/index.test.tsx | 310 ++++++++++++++++-- .../RestoreWallet/WalletRestored.test.tsx | 145 ++++++++ .../Views/RestoreWallet/WalletRestored.tsx | 20 +- .../DeleteWallet/useDeleteWallet.test.tsx | 88 ++++- .../hooks/DeleteWallet/useDeleteWallet.ts | 31 +- .../Authentication/Authentication.test.ts | 7 + app/core/Authentication/Authentication.ts | 18 +- app/core/Engine/Engine.ts | 9 + app/core/EngineService/EngineService.test.ts | 65 ++-- app/core/EngineService/EngineService.ts | 8 +- 14 files changed, 816 insertions(+), 123 deletions(-) create mode 100644 app/components/Views/RestoreWallet/WalletRestored.test.tsx diff --git a/app/components/Views/Login/index.test.tsx b/app/components/Views/Login/index.test.tsx index 6550d9de905..d490c79a195 100644 --- a/app/components/Views/Login/index.test.tsx +++ b/app/components/Views/Login/index.test.tsx @@ -25,6 +25,7 @@ import { TRUE, } from '../../../constants/storage'; import { useMetrics } from '../../hooks/useMetrics'; +import { setExistingUser } from '../../../actions/user'; const mockNavigate = jest.fn(); const mockReplace = jest.fn(); @@ -113,6 +114,13 @@ jest.mock('../../../actions/security', () => ({ }, })); +jest.mock('../../../actions/user', () => ({ + setExistingUser: jest.fn((value) => ({ + type: 'SET_EXISTING_USER', + existingUser: value, + })), +})); + jest.mock('../../../store/storage-wrapper', () => ({ getItem: jest.fn().mockResolvedValue(null), setItem: jest.fn(), @@ -263,7 +271,7 @@ describe('Login', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should call trace function for AuthenticateUser during non-OAuth login', async () => { + it('calls trace function for AuthenticateUser during non-OAuth login', async () => { (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { if (key === OPTIN_META_METRICS_UI_SEEN) return true; return null; @@ -296,11 +304,14 @@ describe('Login', () => { }); describe('Forgot Password', () => { - it('show the forgot password modal', () => { + it('shows forgot password modal when reset wallet pressed', () => { + // Arrange const { getByTestId } = renderWithProvider(); - expect(getByTestId(LoginViewSelectors.RESET_WALLET)).toBeTruthy(); + + // Act fireEvent.press(getByTestId(LoginViewSelectors.RESET_WALLET)); + // Assert expect(mockNavigate).toHaveBeenCalledWith(Routes.MODAL.ROOT_MODAL_FLOW, { screen: Routes.MODAL.DELETE_WALLET, params: { @@ -320,7 +331,7 @@ describe('Login', () => { }); }); - it('should navigate to opt-in metrics when UI not seen and metrics disabled', async () => { + it('navigates to opt-in metrics when UI not seen and metrics disabled', async () => { // Arrange (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { if (key === OPTIN_META_METRICS_UI_SEEN) return Promise.resolve(null); @@ -354,7 +365,6 @@ describe('Login', () => { fireEvent.press(loginButton); }); - // Wait for async operations to complete await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); }); @@ -375,7 +385,7 @@ describe('Login', () => { }); }); - it('should navigate to home when UI not seen but metrics enabled', async () => { + it('navigates to home when UI not seen but metrics enabled', async () => { // Arrange (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { if (key === OPTIN_META_METRICS_UI_SEEN) return Promise.resolve(null); @@ -418,7 +428,7 @@ describe('Login', () => { expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.HOME_NAV); }); - it('should navigate to home when UI seen and metrics disabled', async () => { + it('navigates to home when UI seen and metrics disabled', async () => { // Arrange (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { if (key === OPTIN_META_METRICS_UI_SEEN) @@ -462,7 +472,7 @@ describe('Login', () => { expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.HOME_NAV); }); - it('should navigate to home when UI seen and metrics enabled', async () => { + it('navigates to home when UI seen and metrics enabled', async () => { // Arrange (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { if (key === OPTIN_META_METRICS_UI_SEEN) @@ -726,7 +736,103 @@ describe('Login', () => { fireEvent.changeText(passwordInput, 'testpassword123'); // Assert - expect(passwordInput).toBeTruthy(); + expect(passwordInput).toBeOnTheScreen(); + }); + }); + + describe('Vault Recovery', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('dispatches setExistingUser after successful login from vault recovery', async () => { + // Arrange + mockRoute.mockReturnValue({ + params: { + locked: false, + oauthLoginSuccess: false, + isVaultRecovery: true, + }, + }); + + (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { + if (key === OPTIN_META_METRICS_UI_SEEN) return Promise.resolve('true'); + return Promise.resolve(null); + }); + + (Authentication.userEntryAuth as jest.Mock).mockResolvedValueOnce( + undefined, + ); + ( + Authentication.componentAuthenticationType as jest.Mock + ).mockResolvedValueOnce({ + currentAuthType: 'password', + }); + + const { getByTestId } = renderWithProvider(); + const passwordInput = getByTestId(LoginViewSelectors.PASSWORD_INPUT); + const loginButton = getByTestId(LoginViewSelectors.LOGIN_BUTTON_ID); + + // Act + await act(async () => { + fireEvent.changeText(passwordInput, 'validPassword123'); + }); + + await act(async () => { + fireEvent.press(loginButton); + }); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + // Assert + expect(setExistingUser).toHaveBeenCalledWith(true); + }); + + it('does not dispatch setExistingUser when not from vault recovery', async () => { + // Arrange + mockRoute.mockReturnValue({ + params: { + locked: false, + oauthLoginSuccess: false, + isVaultRecovery: false, + }, + }); + + (StorageWrapper.getItem as jest.Mock).mockImplementation((key) => { + if (key === OPTIN_META_METRICS_UI_SEEN) return Promise.resolve('true'); + return Promise.resolve(null); + }); + + (Authentication.userEntryAuth as jest.Mock).mockResolvedValueOnce( + undefined, + ); + ( + Authentication.componentAuthenticationType as jest.Mock + ).mockResolvedValueOnce({ + currentAuthType: 'password', + }); + + const { getByTestId } = renderWithProvider(); + const passwordInput = getByTestId(LoginViewSelectors.PASSWORD_INPUT); + const loginButton = getByTestId(LoginViewSelectors.LOGIN_BUTTON_ID); + + // Act + await act(async () => { + fireEvent.changeText(passwordInput, 'validPassword123'); + }); + + await act(async () => { + fireEvent.press(loginButton); + }); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + // Assert + expect(setExistingUser).not.toHaveBeenCalled(); }); }); @@ -795,7 +901,7 @@ describe('Login', () => { }); // Should render biometric button when biometric is available - expect(getByTestId(LoginViewSelectors.BIOMETRY_BUTTON)).toBeTruthy(); + expect(getByTestId(LoginViewSelectors.BIOMETRY_BUTTON)).toBeOnTheScreen(); }); it('biometric button is not shown when device is locked', async () => { @@ -860,7 +966,8 @@ describe('Login', () => { jest.clearAllMocks(); }); - it('should handle WRONG_PASSWORD_ERROR', async () => { + it('displays invalid password error when decryption fails', async () => { + // Arrange mockRoute.mockReturnValue({ params: { locked: false, @@ -880,6 +987,7 @@ describe('Login', () => { const { getByTestId } = renderWithProvider(); const passwordInput = getByTestId(LoginViewSelectors.PASSWORD_INPUT); + // Act await act(async () => { fireEvent.changeText(passwordInput, 'valid-password123'); }); @@ -887,8 +995,9 @@ describe('Login', () => { fireEvent(passwordInput, 'submitEditing'); }); + // Assert const errorElement = getByTestId(LoginViewSelectors.PASSWORD_ERROR); - expect(errorElement).toBeTruthy(); + expect(errorElement).toBeOnTheScreen(); expect(errorElement.props.children).toEqual( strings('login.invalid_password'), ); @@ -910,7 +1019,7 @@ describe('Login', () => { expect(rehydrationCall).toBeDefined(); }); - it('should handle WRONG_PASSWORD_ERROR_ANDROID', async () => { + it('displays invalid password error for Android BAD_DECRYPT error', async () => { mockRoute.mockReturnValue({ params: { locked: false, @@ -940,7 +1049,7 @@ describe('Login', () => { }); const errorElement = getByTestId(LoginViewSelectors.PASSWORD_ERROR); - expect(errorElement).toBeTruthy(); + expect(errorElement).toBeOnTheScreen(); expect(errorElement.props.children).toEqual( strings('login.invalid_password'), ); @@ -962,7 +1071,7 @@ describe('Login', () => { expect(rehydrationCall).toBeDefined(); }); - it('should handle WRONG_PASSWORD_ERROR_ANDROID_2', async () => { + it('displays invalid password error for Android DoCipher error', async () => { mockRoute.mockReturnValue({ params: { locked: false, @@ -990,7 +1099,7 @@ describe('Login', () => { }); const errorElement = getByTestId(LoginViewSelectors.PASSWORD_ERROR); - expect(errorElement).toBeTruthy(); + expect(errorElement).toBeOnTheScreen(); expect(errorElement.props.children).toEqual( strings('login.invalid_password'), ); @@ -1012,7 +1121,7 @@ describe('Login', () => { expect(rehydrationCall).toBeDefined(); }); - it('should handle PASSWORD_REQUIREMENTS_NOT_MET error', async () => { + it('displays invalid password error when password requirements not met', async () => { mockRoute.mockReturnValue({ params: { locked: false, @@ -1040,7 +1149,7 @@ describe('Login', () => { }); const errorElement = getByTestId(LoginViewSelectors.PASSWORD_ERROR); - expect(errorElement).toBeTruthy(); + expect(errorElement).toBeOnTheScreen(); expect(errorElement.props.children).toEqual( strings('login.invalid_password'), ); @@ -1054,7 +1163,7 @@ describe('Login', () => { expect(rehydrationCall).toBeUndefined(); }); - it('should handle generic error (else case)', async () => { + it('displays generic error message for unexpected errors', async () => { (Authentication.userEntryAuth as jest.Mock).mockRejectedValue( new Error('Some unexpected error'), ); @@ -1070,13 +1179,13 @@ describe('Login', () => { }); const errorElement = getByTestId(LoginViewSelectors.PASSWORD_ERROR); - expect(errorElement).toBeTruthy(); + expect(errorElement).toBeOnTheScreen(); expect(errorElement.props.children).toEqual( 'Error: Some unexpected error', ); }); - it('should handle OnboardingPasswordLoginError trace during onboarding flow', async () => { + it('traces OnboardingPasswordLoginError during onboarding flow', async () => { mockRoute.mockReturnValue({ params: { locked: false, @@ -1100,7 +1209,7 @@ describe('Login', () => { }); const errorElement = getByTestId(LoginViewSelectors.PASSWORD_ERROR); - expect(errorElement).toBeTruthy(); + expect(errorElement).toBeOnTheScreen(); expect(errorElement.props.children).toEqual( 'Error: Some unexpected error', ); @@ -1131,7 +1240,7 @@ describe('Login', () => { jest.clearAllMocks(); }); - it('should handle PASSCODE_NOT_SET_ERROR with alert', async () => { + it('displays alert when passcode not set', async () => { const mockAlert = jest .spyOn(Alert, 'alert') .mockImplementation(() => undefined); @@ -1220,7 +1329,8 @@ describe('Login', () => { }); }); - it('handle biometric authentication failure', async () => { + it('does not navigate when biometric authentication fails', async () => { + // Arrange (passcodeType as jest.Mock).mockReturnValueOnce('device_passcode'); (Authentication.getType as jest.Mock).mockResolvedValueOnce({ currentAuthType: AUTHENTICATION_TYPE.PASSCODE, @@ -1239,13 +1349,14 @@ describe('Login', () => { const biometryButton = getByTestId(LoginViewSelectors.BIOMETRY_BUTTON); + // Act await act(async () => { fireEvent.press(biometryButton); }); + // Assert expect(Authentication.appTriggeredAuth).toHaveBeenCalled(); expect(mockReplace).not.toHaveBeenCalled(); - expect(biometryButton).toBeTruthy(); }); }); diff --git a/app/components/Views/Login/index.tsx b/app/components/Views/Login/index.tsx index 40f3cd5d030..cb99bfe6c5c 100644 --- a/app/components/Views/Login/index.tsx +++ b/app/components/Views/Login/index.tsx @@ -27,7 +27,8 @@ import { saveOnboardingEvent as saveEvent, } from '../../../actions/onboarding'; import { setAllowLoginWithRememberMe as setAllowLoginWithRememberMeUtil } from '../../../actions/security'; -import { connect, useSelector } from 'react-redux'; +import { setExistingUser } from '../../../actions/user'; +import { connect, useDispatch, useSelector } from 'react-redux'; import { Dispatch } from 'redux'; import { passcodeType, @@ -124,6 +125,7 @@ interface LoginRouteParams { locked: boolean; oauthLoginSuccess?: boolean; onboardingTraceCtx?: TraceContext; + isVaultRecovery?: boolean; } interface LoginProps { @@ -153,6 +155,7 @@ const Login: React.FC = ({ saveOnboardingEvent }) => { const [rehydrationFailedAttempts, setRehydrationFailedAttempts] = useState(0); const navigation = useNavigation>(); const route = useRoute>(); + const dispatch = useDispatch(); const { styles, theme: { colors, themeAppearance }, @@ -163,6 +166,8 @@ const Login: React.FC = ({ saveOnboardingEvent }) => { // coming from oauth onboarding flow flag const isComingFromOauthOnboarding = route?.params?.oauthLoginSuccess ?? false; + // coming from vault recovery flow flag + const isComingFromVaultRecovery = route?.params?.isVaultRecovery ?? false; const { isDeletingInProgress, promptSeedlessRelogin } = usePromptSeedlessRelogin(); @@ -668,6 +673,13 @@ const Login: React.FC = ({ saveOnboardingEvent }) => { }, ); + // CRITICAL: Set existingUser = true after successful vault unlock from recovery + // This prevents the vault recovery screen from appearing again on app restart + // Only set after successful unlock to ensure vault is unlocked and credentials are stored + if (isComingFromVaultRecovery) { + dispatch(setExistingUser(true)); + } + if (isComingFromOauthOnboarding) { track(MetaMetricsEvents.REHYDRATION_COMPLETED, { account_type: 'social', diff --git a/app/components/Views/Onboarding/__snapshots__/index.test.tsx.snap b/app/components/Views/Onboarding/__snapshots__/index.test.tsx.snap index 20b6d954583..f56d9a7ad2a 100644 --- a/app/components/Views/Onboarding/__snapshots__/index.test.tsx.snap +++ b/app/components/Views/Onboarding/__snapshots__/index.test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Onboarding should render correctly 1`] = ` +exports[`Onboarding renders correctly 1`] = ` `; -exports[`Onboarding should render correctly with android 1`] = ` +exports[`Onboarding renders correctly with android 1`] = ` `; -exports[`Onboarding should render correctly with large device and iphoneX 1`] = ` +exports[`Onboarding renders correctly with large device and iphoneX 1`] = ` `; -exports[`Onboarding should render correctly with medium device and android 1`] = ` +exports[`Onboarding renders correctly with medium device and android 1`] = ` true; @@ -323,6 +327,7 @@ class Onboarding extends PureComponent { this.props.disableNewPrivacyPolicyToast(); InteractionManager.runAfterInteractions(() => { + this.checkForMigrationFailureAndVaultBackup(); PreventScreenshot.forbid(); if (this.props.route.params?.delete) { this.showNotification(); @@ -349,6 +354,56 @@ class Onboarding extends PureComponent { } } + /** + * Check for migration failure scenario and vault backup availability + * This detects when a migration has failed and left the user with a corrupted state + * but a valid vault backup still exists in the secure keychain + */ + async checkForMigrationFailureAndVaultBackup() { + // Prevent multiple checks - only run once per instance + if (this.hasCheckedVaultBackup) { + return; + } + + this.hasCheckedVaultBackup = true; + + // Skip check in E2E test environment + // E2E tests start with fresh state but may have vault backups from fixtures/previous runs + // This would trigger false positive vault recovery redirects and break onboarding tests + if (isE2E) { + return; + } + + // Skip check if this is an intentional wallet reset + // (route.params.delete is set when user explicitly resets their wallet) + if (this.props.route?.params?.delete) { + return; + } + + const { existingUser } = this.props; + + try { + const vaultBackupResult = await getVaultFromBackup(); + + // Detect migration failure scenario: + // - existingUser is false (Redux state was corrupted/reset) + // - BUT vault backup exists (user previously had a wallet) + const migrationFailureDetected = + !existingUser && vaultBackupResult.success && vaultBackupResult.vault; + + if (migrationFailureDetected) { + this.props.navigation.reset({ + routes: [{ name: Routes.VAULT_RECOVERY.RESTORE_WALLET }], + }); + } + } catch (error) { + Logger.error( + error, + 'Failed to check for migration failure and vault backup', + ); + } + } + onLogin = async () => { const { passwordSet } = this.props; if (!passwordSet) { diff --git a/app/components/Views/Onboarding/index.test.tsx b/app/components/Views/Onboarding/index.test.tsx index f3978f8a40e..433cbbc5ed4 100644 --- a/app/components/Views/Onboarding/index.test.tsx +++ b/app/components/Views/Onboarding/index.test.tsx @@ -31,6 +31,15 @@ import { OAuthError, OAuthErrorType } from '../../../core/OAuthService/error'; // Mock netinfo - using existing mock jest.mock('@react-native-community/netinfo'); +// Create a mutable mock for isE2E that can be controlled per test +let mockIsE2E = false; +jest.mock('../../../util/test/utils', () => ({ + ...jest.requireActual('../../../util/test/utils'), + get isE2E() { + return mockIsE2E; + }, +})); + import { fetch as netInfoFetch } from '@react-native-community/netinfo'; const mockNetInfoFetch = netInfoFetch as jest.Mock; @@ -244,7 +253,7 @@ describe('Onboarding', () => { Platform.OS = 'ios'; }); - it('should render correctly', () => { + it('renders correctly', () => { const { toJSON } = renderScreen( Onboarding, { name: 'Onboarding' }, @@ -255,7 +264,7 @@ describe('Onboarding', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should render correctly with large device and iphoneX', () => { + it('renders correctly with large device and iphoneX', () => { (Device.isLargeDevice as jest.Mock).mockReturnValue(true); (Device.isIphoneX as jest.Mock).mockReturnValue(true); (Device.isAndroid as jest.Mock).mockReturnValue(false); @@ -271,7 +280,7 @@ describe('Onboarding', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should render correctly with medium device and android', () => { + it('renders correctly with medium device and android', () => { (Device.isMediumDevice as jest.Mock).mockReturnValue(true); (Device.isIphoneX as jest.Mock).mockReturnValue(false); (Device.isAndroid as jest.Mock).mockReturnValue(true); @@ -287,7 +296,7 @@ describe('Onboarding', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should render correctly with android', () => { + it('renders correctly with android', () => { (Device.isAndroid as jest.Mock).mockReturnValue(true); (Device.isIos as jest.Mock).mockReturnValue(false); (Device.isLargeDevice as jest.Mock).mockReturnValue(false); @@ -305,7 +314,7 @@ describe('Onboarding', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should click on create wallet button', () => { + it('handles click on create wallet button', () => { (Device.isAndroid as jest.Mock).mockReturnValue(true); (Device.isIos as jest.Mock).mockReturnValue(false); (Device.isLargeDevice as jest.Mock).mockReturnValue(false); @@ -325,7 +334,7 @@ describe('Onboarding', () => { fireEvent.press(createWalletButton); }); - it('should click on have an existing wallet button', () => { + it('handles click on have an existing wallet button', () => { (Device.isAndroid as jest.Mock).mockReturnValue(true); (Device.isIos as jest.Mock).mockReturnValue(false); (Device.isLargeDevice as jest.Mock).mockReturnValue(false); @@ -352,7 +361,7 @@ describe('Onboarding', () => { mockNetInfoFetch.mockReset(); }); - it('should navigate to onboarding sheet when create wallet is pressed for new user', async () => { + it('navigates to onboarding sheet when create wallet is pressed for new user', async () => { mockSeedlessOnboardingEnabled.mockReturnValue(true); (StorageWrapper.getItem as jest.Mock).mockResolvedValue(null); @@ -387,7 +396,7 @@ describe('Onboarding', () => { ); }); - it('should navigate to ChoosePassword when create wallet is pressed with seedless disabled', async () => { + it('navigates to ChoosePassword when create wallet is pressed with seedless disabled', async () => { mockSeedlessOnboardingEnabled.mockReturnValue(false); (StorageWrapper.getItem as jest.Mock).mockResolvedValue(null); @@ -419,7 +428,7 @@ describe('Onboarding', () => { ); }); - it('should navigate to offline error sheet when there is no internet', async () => { + it('navigates to offline error sheet when there is no internet', async () => { mockSeedlessOnboardingEnabled.mockReturnValue(true); (StorageWrapper.getItem as jest.Mock).mockResolvedValue(null); @@ -491,7 +500,7 @@ describe('Onboarding', () => { afterEach(() => { mockSeedlessOnboardingEnabled.mockReset(); }); - it('should navigate to onboarding sheet when have an existing wallet button is pressed for new user', async () => { + it('navigates to onboarding sheet when have an existing wallet button is pressed for new user', async () => { mockSeedlessOnboardingEnabled.mockReturnValue(true); (StorageWrapper.getItem as jest.Mock).mockResolvedValue(null); @@ -526,7 +535,7 @@ describe('Onboarding', () => { ); }); - it('should navigate to import flow when import wallet is pressed with seedless disabled', async () => { + it('navigates to import flow when import wallet is pressed with seedless disabled', async () => { mockSeedlessOnboardingEnabled.mockReturnValue(false); const { getByTestId } = renderScreen( Onboarding, @@ -559,7 +568,7 @@ describe('Onboarding', () => { }); describe('Navigation behavior', () => { - it('should navigate to HOME_NAV when unlock is pressed and password is not set', async () => { + it('navigates to HOME_NAV when unlock is pressed and password is not set', async () => { const { getByText } = renderScreen( Onboarding, { name: 'Onboarding' }, @@ -584,7 +593,7 @@ describe('Onboarding', () => { expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.HOME_NAV); }); - it('should navigate to LOGIN when unlock is pressed and password is set', async () => { + it('navigates to LOGIN when unlock is pressed and password is set', async () => { const { getByText } = renderScreen( Onboarding, { name: 'Onboarding' }, @@ -611,7 +620,7 @@ describe('Onboarding', () => { }); describe('componentDidMount behavior', () => { - it('should check for existing user on mount', async () => { + it('checks for existing user on mount', async () => { renderScreen( Onboarding, { name: 'Onboarding' }, @@ -627,7 +636,7 @@ describe('Onboarding', () => { }); }); - it('should disable back press when component mounts', () => { + it('disables back press when component mounts', () => { renderScreen( Onboarding, { name: 'Onboarding' }, @@ -642,7 +651,7 @@ describe('Onboarding', () => { ); }); - it('should trigger animatedTimingStart', async () => { + it('triggers animatedTimingStart', async () => { jest.useFakeTimers(); const animatedTimingSpy = jest.spyOn(Animated, 'timing'); @@ -697,7 +706,7 @@ describe('Onboarding', () => { mockSeedlessOnboardingEnabled.mockReset(); }); - it('should call Google OAuth login for create wallet flow on iOS and navigate to SocialLoginSuccessNewUser', async () => { + it('calls Google OAuth login for create wallet flow on iOS and navigates to SocialLoginSuccessNewUser', async () => { mockCreateLoginHandler.mockReturnValue('mockGoogleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ type: 'success', @@ -746,7 +755,7 @@ describe('Onboarding', () => { ); }); - it('should call Google OAuth login for create wallet flow on Android and navigate directly to ChoosePassword', async () => { + it('calls Google OAuth login for create wallet flow on Android and navigates directly to ChoosePassword', async () => { Platform.OS = 'android'; // Set platform to Android mockCreateLoginHandler.mockReturnValue('mockGoogleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ @@ -800,7 +809,7 @@ describe('Onboarding', () => { Platform.OS = 'ios'; }); - it('should call Apple OAuth login for create wallet flow on iOS and navigate to SocialLoginSuccessNewUser', async () => { + it('calls Apple OAuth login for create wallet flow on iOS and navigates to SocialLoginSuccessNewUser', async () => { mockCreateLoginHandler.mockReturnValue('mockAppleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ type: 'success', @@ -850,7 +859,7 @@ describe('Onboarding', () => { ); }); - it('should call Apple OAuth login for import wallet flow', async () => { + it('calls Apple OAuth login for import wallet flow', async () => { mockCreateLoginHandler.mockReturnValue('mockAppleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ type: 'success', @@ -899,7 +908,7 @@ describe('Onboarding', () => { ); }); - it('should show error sheet for OAuth user cancellation', async () => { + it('shows error sheet for OAuth user cancellation', async () => { const cancelError = new OAuthError('', OAuthErrorType.UserCancelled); mockCreateLoginHandler.mockReturnValue('mockGoogleHandler'); mockOAuthService.handleOAuthLogin.mockRejectedValue(cancelError); @@ -993,7 +1002,7 @@ describe('Onboarding', () => { ); }); - it('should navigate to AccountAlreadyExists for existing user in create wallet flow', async () => { + it('navigates to AccountAlreadyExists for existing user in create wallet flow', async () => { mockCreateLoginHandler.mockReturnValue('mockGoogleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ type: 'success', @@ -1038,7 +1047,7 @@ describe('Onboarding', () => { ); }); - it('should navigate to AccountNotFound for new user in import wallet flow', async () => { + it('navigates to AccountNotFound for new user in import wallet flow', async () => { mockCreateLoginHandler.mockReturnValue('mockAppleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ type: 'success', @@ -1083,7 +1092,7 @@ describe('Onboarding', () => { ); }); - it('should show error sheet for OAuth when no credential is available in Android', async () => { + it('shows error sheet for OAuth when no credential is available in Android', async () => { const noCredentialError = new OAuthError( '', OAuthErrorType.GoogleLoginNoCredential, @@ -1139,7 +1148,7 @@ describe('Onboarding', () => { ); }); - it('should show error sheet for OAuth when no matching credential in Android', async () => { + it('shows error sheet for OAuth when no matching credential in Android', async () => { const noCredentialError = new OAuthError( '', OAuthErrorType.GoogleLoginNoMatchingCredential, @@ -1195,7 +1204,7 @@ describe('Onboarding', () => { ); }); - it('should enable social login metrics when OAuth login succeeds', async () => { + it('enables social login metrics when OAuth login succeeds', async () => { mockCreateLoginHandler.mockReturnValue('mockGoogleHandler'); mockOAuthService.handleOAuthLogin.mockResolvedValue({ type: 'success', @@ -1252,7 +1261,7 @@ describe('Onboarding', () => { mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false); }); - it('should disable social login metrics when non-OAuth user creates wallet', async () => { + it('disables social login metrics when non-OAuth user creates wallet', async () => { mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false); mockEnableSocialLogin.mockClear(); @@ -1274,7 +1283,7 @@ describe('Onboarding', () => { expect(mockEnableSocialLogin).toHaveBeenCalledWith(false); }); - it('should disable social login metrics when non-OAuth user imports wallet', async () => { + it('disables social login metrics when non-OAuth user imports wallet', async () => { mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(false); mockEnableSocialLogin.mockClear(); @@ -1296,7 +1305,7 @@ describe('Onboarding', () => { expect(mockEnableSocialLogin).toHaveBeenCalledWith(false); }); - it('should disable social login metrics when non-OAuth user creates wallet', async () => { + it('disables social login metrics when OAuth user creates wallet', async () => { mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(true); mockEnableSocialLogin.mockClear(); @@ -1318,7 +1327,7 @@ describe('Onboarding', () => { expect(mockEnableSocialLogin).toHaveBeenCalledWith(false); }); - it('should disable social login metrics when OAuth user imports wallet', async () => { + it('disables social login metrics when OAuth user imports wallet', async () => { mockOAuthService.getMetricStateBeforeOauth.mockReturnValue(true); mockEnableSocialLogin.mockClear(); @@ -1341,6 +1350,243 @@ describe('Onboarding', () => { }); }); + describe('checkForMigrationFailureAndVaultBackup', () => { + beforeEach(() => { + jest.clearAllMocks(); + (StorageWrapper.getItem as jest.Mock).mockResolvedValue(null); + }); + + it('returns early when route.params.delete is true', async () => { + // Arrange + const { toJSON } = renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + { route: { params: { delete: true } } }, + ); + + // Act - Component mounts and checkForMigrationFailureAndVaultBackup is called + await waitFor(() => { + expect(toJSON()).toBeDefined(); + }); + + // Assert - When delete param is true, vault backup check is skipped + expect(StorageWrapper.getItem).not.toHaveBeenCalled(); + }); + + it('skips vault backup check when running in E2E test environment', async () => { + // Arrange + mockIsE2E = true; + + // Act + renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + // Assert + expect(StorageWrapper.getItem).not.toHaveBeenCalled(); + + // Cleanup + mockIsE2E = false; + }); + + it('accesses existingUser prop from Redux state', async () => { + // Arrange + const { toJSON } = renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialStateWithExistingUser, + }, + ); + + // Act - Component mounts and reads existingUser from props + await waitFor(() => { + expect(toJSON()).toBeDefined(); + }); + + // Assert - Component renders without errors when existingUser is true + expect(toJSON()).toBeTruthy(); + }); + + it('reads existingUser as false for new users', async () => { + // Arrange + const { toJSON } = renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + ); + + // Act - Component mounts with existingUser false + await waitFor(() => { + expect(toJSON()).toBeDefined(); + }); + + // Assert - Component handles new user case without errors + expect(toJSON()).toBeTruthy(); + }); + }); + + describe('showNotification', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.clearAllMocks(); + jest.spyOn(BackHandler, 'addEventListener').mockImplementation(() => ({ + remove: jest.fn(), + })); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('calls Animated.timing when delete param is present', async () => { + // Arrange + const animatedTimingSpy = jest.spyOn(Animated, 'timing'); + + // Act + renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + { route: { params: { delete: true } } }, + ); + + // Assert - Animation is triggered + await waitFor(() => { + expect(animatedTimingSpy).toHaveBeenCalled(); + }); + + animatedTimingSpy.mockRestore(); + }); + + it('calls BackHandler.addEventListener when notification is shown', async () => { + // Arrange + const backHandlerSpy = jest.spyOn(BackHandler, 'addEventListener'); + + // Act + renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + { route: { params: { delete: true } } }, + ); + + // Assert - Verifies disableBackPress was called + await waitFor(() => { + expect(backHandlerSpy).toHaveBeenCalledWith( + 'hardwareBackPress', + expect.any(Function), + ); + }); + + backHandlerSpy.mockRestore(); + }); + + it('registers event listener with handler function', async () => { + // Arrange + const backHandlerSpy = jest.spyOn(BackHandler, 'addEventListener'); + + // Act + renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + { route: { params: { delete: true } } }, + ); + + // Assert - BackHandler.addEventListener called with function handler + await waitFor(() => { + expect(backHandlerSpy).toHaveBeenCalledWith( + 'hardwareBackPress', + expect.any(Function), + ); + }); + + backHandlerSpy.mockRestore(); + }); + }); + + describe('disableBackPress', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(BackHandler, 'addEventListener').mockImplementation(() => ({ + remove: jest.fn(), + })); + }); + + it('creates hardwareBackPress handler function', async () => { + // Arrange + const backHandlerSpy = jest.spyOn(BackHandler, 'addEventListener'); + + // Act + renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + { route: { params: { delete: true } } }, + ); + + // Assert - Verifies handler function is created and registered + await waitFor(() => { + expect(backHandlerSpy).toHaveBeenCalledWith( + 'hardwareBackPress', + expect.any(Function), + ); + }); + + backHandlerSpy.mockRestore(); + }); + + it('registers event listener with hardwareBackPress event name', async () => { + // Arrange + const backHandlerSpy = jest.spyOn(BackHandler, 'addEventListener'); + + // Act + renderScreen( + Onboarding, + { name: 'Onboarding' }, + { + state: mockInitialState, + }, + { route: { params: { delete: true } } }, + ); + + // Assert - Event listener registered with correct event name + await waitFor(() => { + expect(backHandlerSpy).toHaveBeenCalledWith( + 'hardwareBackPress', + expect.any(Function), + ); + }); + + const callArgs = backHandlerSpy.mock.calls[0]; + expect(callArgs[0]).toBe('hardwareBackPress'); + + backHandlerSpy.mockRestore(); + }); + }); + describe('ErrorBoundary Tests', () => { const mockOAuthService = jest.requireMock( '../../../core/OAuthService/OAuthService', @@ -1359,7 +1605,7 @@ describe('Onboarding', () => { mockSeedlessOnboardingEnabled.mockReset(); }); - it('should trigger ErrorBoundary for OAuth login failures when analytics disabled', async () => { + it('triggers ErrorBoundary for OAuth login failures when analytics disabled', async () => { mockMetricsIsEnabled.mockReturnValueOnce(false); mockCreateEventBuilder.mockReturnValue({ addProperties: jest.fn().mockReturnThis(), @@ -1404,7 +1650,7 @@ describe('Onboarding', () => { ); }); - it('should not trigger ErrorBoundary for OAuth login failures when analytics enabled', async () => { + it('does not trigger ErrorBoundary for OAuth login failures when analytics enabled', async () => { mockMetricsIsEnabled.mockReturnValue(true); const dismissError = new OAuthError('', OAuthErrorType.AuthServerError); mockCreateLoginHandler.mockReturnValue('mockAppleHandler'); diff --git a/app/components/Views/RestoreWallet/WalletRestored.test.tsx b/app/components/Views/RestoreWallet/WalletRestored.test.tsx new file mode 100644 index 00000000000..93642040b1c --- /dev/null +++ b/app/components/Views/RestoreWallet/WalletRestored.test.tsx @@ -0,0 +1,145 @@ +import React from 'react'; +import { fireEvent, waitFor } from '@testing-library/react-native'; +import { Linking } from 'react-native'; +import WalletRestored from './WalletRestored'; +import { useNavigation } from '@react-navigation/native'; +import { useMetrics } from '../../../components/hooks/useMetrics'; +import generateDeviceAnalyticsMetaData from '../../../util/metrics'; +import Routes from '../../../constants/navigation/Routes'; +import { SRP_GUIDE_URL } from '../../../constants/urls'; +import renderWithProvider from '../../../util/test/renderWithProvider'; + +// Mock all external dependencies +jest.mock('@react-navigation/native', () => ({ + ...jest.requireActual('@react-navigation/native'), + useNavigation: jest.fn(), +})); +jest.mock('../../../util/theme', () => ({ + useAppThemeFromContext: jest.fn(() => ({ + colors: { + primary: { + inverse: '#FFFFFF', + }, + background: { + default: '#000000', + }, + }, + })), +})); +jest.mock('../../../components/hooks/useMetrics'); +jest.mock('../../../util/metrics'); +jest.mock('react-native/Libraries/Linking/Linking', () => ({ + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + openURL: jest.fn(), + canOpenURL: jest.fn(), + getInitialURL: jest.fn(), +})); + +describe('WalletRestored', () => { + const mockNavigation = { + replace: jest.fn(), + navigate: jest.fn(), + goBack: jest.fn(), + }; + + const mockTrackEvent = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + + // Create a fresh mock chain each time + const mockBuild = jest.fn().mockReturnValue({ name: 'test-event' }); + const mockAddProperties = jest.fn().mockReturnValue({ + build: mockBuild, + }); + const mockCreateEventBuilder = jest.fn().mockReturnValue({ + addProperties: mockAddProperties, + }); + + (useNavigation as jest.Mock).mockReturnValue(mockNavigation); + (useMetrics as jest.Mock).mockReturnValue({ + trackEvent: mockTrackEvent, + createEventBuilder: mockCreateEventBuilder, + }); + (generateDeviceAnalyticsMetaData as jest.Mock).mockReturnValue({ + os: 'ios', + version: '1.0.0', + }); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('renders correctly with all required elements', () => { + // Arrange + const { getByText } = renderWithProvider(); + + // Assert + expect(getByText('🎉')).toBeOnTheScreen(); + expect(getByText('Your wallet is ready!')).toBeOnTheScreen(); + expect(getByText('Continue to wallet')).toBeOnTheScreen(); + }); + + it('opens SRP guide URL when backup link is pressed', async () => { + // Arrange + const { getByText } = renderWithProvider(); + const backupLink = getByText(' back up your Secret Recovery Phrase '); + + // Act + fireEvent.press(backupLink); + + // Assert + await waitFor(() => { + expect(Linking.openURL).toHaveBeenCalledWith(SRP_GUIDE_URL); + }); + }); + + it('navigates to LOGIN with vault recovery flag when continue is pressed', () => { + // Arrange + const { getByText } = renderWithProvider(); + const continueButton = getByText('Continue to wallet'); + + // Act + fireEvent.press(continueButton); + + // Assert + expect(mockNavigation.replace).toHaveBeenCalledWith( + Routes.ONBOARDING.LOGIN, + { isVaultRecovery: true }, + ); + }); + + it('tracks continue button press event with device metadata', () => { + // Arrange + const { getByText } = renderWithProvider(); + const continueButton = getByText('Continue to wallet'); + + // Act + fireEvent.press(continueButton); + + // Assert + expect(mockTrackEvent).toHaveBeenCalledWith( + expect.objectContaining({ + name: expect.any(String), + }), + ); + }); + + it('generates device metadata once using useMemo', () => { + // Arrange & Act + renderWithProvider(); + + // Assert + expect(generateDeviceAnalyticsMetaData).toHaveBeenCalledTimes(1); + }); + + it('does not call navigation on mount', () => { + // Arrange & Act + renderWithProvider(); + + // Assert + expect(mockNavigation.replace).not.toHaveBeenCalled(); + }); +}); diff --git a/app/components/Views/RestoreWallet/WalletRestored.tsx b/app/components/Views/RestoreWallet/WalletRestored.tsx index 76b25d7c63e..72441704d43 100644 --- a/app/components/Views/RestoreWallet/WalletRestored.tsx +++ b/app/components/Views/RestoreWallet/WalletRestored.tsx @@ -17,7 +17,6 @@ import { createNavigationDetails } from '../../../util/navigation/navUtils'; import Routes from '../../../constants/navigation/Routes'; import { SafeAreaView } from 'react-native-safe-area-context'; import { useNavigation } from '@react-navigation/native'; -import { Authentication } from '../../../core'; import { useAppThemeFromContext } from '../../../util/theme'; import { MetaMetricsEvents } from '../../../core/Analytics'; import generateDeviceAnalyticsMetaData from '../../../util/metrics'; @@ -50,21 +49,20 @@ const WalletRestored = () => { ); }, [deviceMetaData, trackEvent, createEventBuilder]); - const finishWalletRestore = useCallback(async (): Promise => { - try { - await Authentication.appTriggeredAuth(); - navigation.replace(Routes.ONBOARDING.HOME_NAV); - } catch (e) { - // we were not able to log in automatically so we will go back to login - navigation.replace(Routes.ONBOARDING.LOGIN); - } + const finishWalletRestore = useCallback((): void => { + // After vault recovery, navigate to Login for manual password entry + // This unlocks the restored vault AND stores credentials in keychain + // Note: appTriggeredAuth cannot work here - no credentials exist yet + navigation.replace(Routes.ONBOARDING.LOGIN, { + isVaultRecovery: true, + }); }, [navigation]); const onPressBackupSRP = useCallback(async (): Promise => { Linking.openURL(SRP_GUIDE_URL); }, []); - const handleOnNext = useCallback(async (): Promise => { + const handleOnNext = useCallback((): void => { setLoading(true); trackEvent( createEventBuilder( @@ -73,7 +71,7 @@ const WalletRestored = () => { .addProperties({ ...deviceMetaData }) .build(), ); - await finishWalletRestore(); + finishWalletRestore(); }, [deviceMetaData, finishWalletRestore, trackEvent, createEventBuilder]); return ( diff --git a/app/components/hooks/DeleteWallet/useDeleteWallet.test.tsx b/app/components/hooks/DeleteWallet/useDeleteWallet.test.tsx index af2e79bb9db..925e6a8f5e7 100644 --- a/app/components/hooks/DeleteWallet/useDeleteWallet.test.tsx +++ b/app/components/hooks/DeleteWallet/useDeleteWallet.test.tsx @@ -2,8 +2,10 @@ import { renderHookWithProvider } from '../../../util/test/renderWithProvider'; import useDeleteWallet from './useDeleteWallet'; import AUTHENTICATION_TYPE from '../../../constants/userProperties'; import Engine from '../../../core/Engine'; +import { Engine as EngineClass } from '../../../core/Engine/Engine'; import Logger from '../../../util/Logger'; import { Authentication } from '../../../core'; +import { clearAllVaultBackups } from '../../../core/BackupVault'; import { resetProviderToken as depositResetProviderToken } from '../../UI/Ramp/Deposit/utils/ProviderTokenVault'; jest.mock('../../../core/Engine', () => ({ @@ -17,6 +19,12 @@ jest.mock('../../../core/Engine', () => ({ }, })); +jest.mock('../../../core/Engine/Engine', () => ({ + Engine: { + disableAutomaticVaultBackup: false, + }, +})); + jest.mock('../../../store/storage-wrapper', () => ({ getItem: jest.fn(), setItem: jest.fn(), @@ -52,16 +60,65 @@ jest.mock('../../UI/Ramp/Deposit/utils/ProviderTokenVault', () => ({ describe('useDeleteWallet', () => { beforeEach(() => { jest.clearAllMocks(); + EngineClass.disableAutomaticVaultBackup = false; }); - test('it should provide two outputs of type function', () => { + test('provides two functions for wallet operations', () => { + // Arrange & Act const { result } = renderHookWithProvider(() => useDeleteWallet()); const [resetWalletState, deleteUser] = result.current; + + // Assert expect(typeof resetWalletState).toBe('function'); expect(typeof deleteUser).toBe('function'); }); - test('it should call the appropriate methods to reset the wallet', async () => { + test('calls vault backup clear before creating temporary wallet', async () => { + // Arrange + const { result } = renderHookWithProvider(() => useDeleteWallet()); + const [resetWalletState] = result.current; + const clearVaultSpy = jest.mocked(clearAllVaultBackups); + const newWalletSpy = jest.spyOn(Authentication, 'newWalletAndKeychain'); + + // Act + await resetWalletState(); + + // Assert + expect(clearVaultSpy).toHaveBeenCalledTimes(1); + const clearCallOrder = clearVaultSpy.mock.invocationCallOrder[0]; + const newWalletCallOrder = newWalletSpy.mock.invocationCallOrder[0]; + expect(clearCallOrder).toBeLessThan(newWalletCallOrder); + }); + + test('disables automatic vault backup during wallet reset', async () => { + // Arrange + const { result } = renderHookWithProvider(() => useDeleteWallet()); + const [resetWalletState] = result.current; + + // Act + await resetWalletState(); + + // Assert - flag is re-enabled after reset completes + expect(EngineClass.disableAutomaticVaultBackup).toBe(false); + }); + + test('re-enables automatic vault backup even when error occurs', async () => { + // Arrange + const { result } = renderHookWithProvider(() => useDeleteWallet()); + const [resetWalletState] = result.current; + jest + .spyOn(Authentication, 'newWalletAndKeychain') + .mockRejectedValueOnce(new Error('Authentication failed')); + + // Act + await resetWalletState(); + + // Assert - flag is still re-enabled despite error + expect(EngineClass.disableAutomaticVaultBackup).toBe(false); + }); + + test('calls all required methods to reset wallet state', async () => { + // Arrange const { result } = renderHookWithProvider(() => useDeleteWallet()); // eslint-disable-next-line @typescript-eslint/no-unused-vars const [resetWalletState, _] = result.current; @@ -73,14 +130,14 @@ describe('useDeleteWallet', () => { Engine.context.SeedlessOnboardingController, 'clearState', ); - const resetRewardsSpy = jest.spyOn(Engine.controllerMessenger, 'call'); - const loggerSpy = jest.spyOn(Logger, 'log'); const resetProviderTokenSpy = jest.mocked(depositResetProviderToken); + // Act await resetWalletState(); + // Assert expect(newWalletAndKeychain).toHaveBeenCalledWith(expect.any(String), { currentAuthType: AUTHENTICATION_TYPE.UNKNOWN, }); @@ -91,10 +148,10 @@ describe('useDeleteWallet', () => { expect(resetProviderTokenSpy).toHaveBeenCalledTimes(1); }); - test('it should handle errors gracefully when resetWalletState fails', async () => { + test('logs error when resetWalletState fails', async () => { + // Arrange const { result } = renderHookWithProvider(() => useDeleteWallet()); const [resetWalletState] = result.current; - const newWalletAndKeychain = jest.spyOn( Authentication, 'newWalletAndKeychain', @@ -104,7 +161,10 @@ describe('useDeleteWallet', () => { new Error('Authentication failed'), ); - await expect(resetWalletState()).resolves.not.toThrow(); + // Act + await resetWalletState(); + + // Assert expect(newWalletAndKeychain).toHaveBeenCalled(); expect(loggerSpy).toHaveBeenCalledWith( expect.any(Error), @@ -112,25 +172,27 @@ describe('useDeleteWallet', () => { ); }); - test('it should call the appropriate methods to delete the user', async () => { + test('dispatches Redux action to delete user', async () => { + // Arrange const { result } = renderHookWithProvider(() => useDeleteWallet()); // eslint-disable-next-line @typescript-eslint/no-unused-vars const [_, deleteUser] = result.current; const loggerSpy = jest.spyOn(Logger, 'log'); + // Act await deleteUser(); - // Check that the Redux action was dispatched (this is handled by the store) + // Assert - Redux action was dispatched (handled by store) expect(loggerSpy).not.toHaveBeenCalled(); }); - test('it should handle errors gracefully when deleteUser fails', async () => { + test('completes without throwing when deleteUser succeeds', async () => { + // Arrange const { result } = renderHookWithProvider(() => useDeleteWallet()); const [, deleteUser] = result.current; - const loggerSpy = jest.spyOn(Logger, 'log'); - // Since the metrics hook is already mocked to return a working function, - // we'll just verify that the function completes without throwing + + // Act & Assert await expect(deleteUser()).resolves.not.toThrow(); expect(loggerSpy).not.toHaveBeenCalled(); }); diff --git a/app/components/hooks/DeleteWallet/useDeleteWallet.ts b/app/components/hooks/DeleteWallet/useDeleteWallet.ts index ad71df643ca..ff478e003f9 100644 --- a/app/components/hooks/DeleteWallet/useDeleteWallet.ts +++ b/app/components/hooks/DeleteWallet/useDeleteWallet.ts @@ -7,6 +7,7 @@ import AUTHENTICATION_TYPE from '../../../constants/userProperties'; import { clearAllVaultBackups } from '../../../core/BackupVault'; import { useMetrics } from '../useMetrics'; import Engine from '../../../core/Engine'; +import { Engine as EngineClass } from '../../../core/Engine/Engine'; import { resetProviderToken as depositResetProviderToken } from '../../UI/Ramp/Deposit/utils/ProviderTokenVault'; const useDeleteWallet = () => { @@ -15,20 +16,30 @@ const useDeleteWallet = () => { const resetWalletState = useCallback(async () => { try { - await Authentication.newWalletAndKeychain(`${Date.now()}`, { - currentAuthType: AUTHENTICATION_TYPE.UNKNOWN, - }); + // Clear vault backups BEFORE creating temporary wallet + await clearAllVaultBackups(); - Engine.context.SeedlessOnboardingController.clearState(); + // CRITICAL: Disable automatic vault backups during wallet RESET + // This prevents the temporary wallet (created during reset) from being backed up + EngineClass.disableAutomaticVaultBackup = true; - await depositResetProviderToken(); + try { + await Authentication.newWalletAndKeychain(`${Date.now()}`, { + currentAuthType: AUTHENTICATION_TYPE.UNKNOWN, + }); - await Engine.controllerMessenger.call('RewardsController:resetAll'); + Engine.context.SeedlessOnboardingController.clearState(); - await clearAllVaultBackups(); - // lock the app but do not navigate to login screen as it should - // navigate to onboarding screen after deleting the wallet - await Authentication.lockApp({ navigateToLogin: false }); + await depositResetProviderToken(); + + await Engine.controllerMessenger.call('RewardsController:resetAll'); + + // Lock the app and navigate to onboarding + await Authentication.lockApp({ navigateToLogin: false }); + } finally { + // ALWAYS re-enable automatic vault backups, even if error occurs + EngineClass.disableAutomaticVaultBackup = false; + } } catch (error) { const errorMsg = `Failed to createNewVaultAndKeychain: ${error}`; Logger.log(error, errorMsg); diff --git a/app/core/Authentication/Authentication.test.ts b/app/core/Authentication/Authentication.test.ts index 45bd61087d3..b880924e2fd 100644 --- a/app/core/Authentication/Authentication.test.ts +++ b/app/core/Authentication/Authentication.test.ts @@ -124,6 +124,13 @@ jest.mock('../Engine', () => ({ }, })); +// Mock for Engine class (used in error recovery) +jest.mock('../Engine/Engine', () => ({ + Engine: class MockEngine { + static disableAutomaticVaultBackup = false; + }, +})); + jest.mock('../NavigationService', () => ({ navigation: { reset: jest.fn(), diff --git a/app/core/Authentication/Authentication.ts b/app/core/Authentication/Authentication.ts index d64632210a3..9313fc1fbf5 100644 --- a/app/core/Authentication/Authentication.ts +++ b/app/core/Authentication/Authentication.ts @@ -1,5 +1,6 @@ import SecureKeychain from '../SecureKeychain'; import Engine from '../Engine'; +import { Engine as EngineClass } from '../Engine/Engine'; import { BIOMETRY_CHOICE_DISABLED, TRUE, @@ -761,10 +762,21 @@ class AuthenticationService { this.dispatchOauthReset(); } catch (error) { - await this.newWalletAndKeychain(`${Date.now()}`, { - currentAuthType: AUTHENTICATION_TYPE.UNKNOWN, - }); + // Clear vault backups BEFORE creating temporary wallet await clearAllVaultBackups(); + + // Disable automatic vault backups during OAuth error recovery + EngineClass.disableAutomaticVaultBackup = true; + + try { + await this.newWalletAndKeychain(`${Date.now()}`, { + currentAuthType: AUTHENTICATION_TYPE.UNKNOWN, + }); + } finally { + // ALWAYS re-enable automatic backups, even if error occurs + EngineClass.disableAutomaticVaultBackup = false; + } + SeedlessOnboardingController.clearState(); throw error; } diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index 655e75c33e6..a92d312ebc0 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -188,6 +188,10 @@ export class Engine { * The global Engine singleton */ static instance: Engine | null; + /** + * Flag to disable automatic vault backups (used during wallet reset) + */ + static disableAutomaticVaultBackup = false; /** * A collection of all controller instances */ @@ -696,6 +700,11 @@ export class Engine { this.controllerMessenger.subscribe( AppConstants.KEYRING_STATE_CHANGE_EVENT, (state: KeyringControllerState) => { + // Check if automatic backups are disabled (during wallet reset) + if (Engine.disableAutomaticVaultBackup) { + return; + } + if (!state.vault) { return; } diff --git a/app/core/EngineService/EngineService.test.ts b/app/core/EngineService/EngineService.test.ts index 625d2e163a6..11c26aaf7b9 100644 --- a/app/core/EngineService/EngineService.test.ts +++ b/app/core/EngineService/EngineService.test.ts @@ -208,14 +208,14 @@ describe('EngineService', () => { jest.useRealTimers(); }); - it('should have Engine initialized', async () => { + it('initializes Engine context', async () => { engineService.start(); await waitFor(() => { expect(Engine.context).toBeDefined(); }); }); - it('should log Engine initialization with state info (existing installation)', async () => { + it('logs Engine initialization with state info for existing installation', async () => { // Mock ControllerStorage to return actual state (existing installation) (ControllerStorage.getAllPersistedState as jest.Mock).mockResolvedValue({ backgroundState: { @@ -235,7 +235,7 @@ describe('EngineService', () => { }); }); - it('should log Engine initialization with empty state (fresh install)', async () => { + it('logs Engine initialization with empty state for fresh install', async () => { // Mock ControllerStorage to return empty state (fresh install) (ControllerStorage.getAllPersistedState as jest.Mock).mockResolvedValue({ backgroundState: {}, @@ -262,12 +262,15 @@ describe('EngineService', () => { }); }); - it('should have recovered vault on redux store and log initialization', async () => { - // Use real timers for this test to handle the Promise-based setTimeout + it('recovers vault on redux store and logs initialization', async () => { + // Arrange jest.useRealTimers(); + // Act await engineService.start(); const { success } = await engineService.initializeVaultFromBackup(); + + // Assert expect(success).toBeTruthy(); expect(Engine.context.KeyringController.state.vault).toBeDefined(); expect(Logger.log).toHaveBeenCalledWith( @@ -281,7 +284,27 @@ describe('EngineService', () => { jest.useFakeTimers(); }); - it('should navigate to vault recovery if Engine fails to initialize', async () => { + it('sets up persistence subscriptions after vault recovery', async () => { + // Arrange + jest.useRealTimers(); + + // Act + await engineService.initializeVaultFromBackup(); + + // Assert - verify setupEnginePersistence was called during vault recovery + // This ensures controller state changes are persisted after recovery + const persistenceLogCalls = (Logger.log as jest.Mock).mock.calls.filter( + (call) => + call[0] === + 'Individual controller persistence subscriptions set up successfully', + ); + expect(persistenceLogCalls.length).toBeGreaterThan(0); + + // Restore fake timers for other tests + jest.useFakeTimers(); + }); + + it('navigates to vault recovery when Engine fails to initialize', async () => { jest.spyOn(Engine, 'init').mockImplementation(() => { throw new Error('Failed to initialize Engine'); }); @@ -311,7 +334,7 @@ describe('EngineService', () => { }; } - it('should batch initial state key', async () => { + it('batches initial state key', async () => { engineService.start(); // Access private property with proper typing @@ -328,7 +351,7 @@ describe('EngineService', () => { }); }); - it('should handle UPDATE_BG_STATE_KEY actions in updateBatcher', async () => { + it('handles UPDATE_BG_STATE_KEY actions in updateBatcher', async () => { engineService.start(); const keys = [ @@ -360,7 +383,7 @@ describe('EngineService', () => { }); }); - it('should handle both INIT and UPDATE actions in updateBatcher', async () => { + it('handles both INIT and UPDATE actions in updateBatcher', async () => { engineService.start(); // Add both INIT and UPDATE keys @@ -400,7 +423,7 @@ describe('EngineService', () => { }) => void; } - it('should handle missing engine context gracefully', () => { + it('handles missing engine context without errors', () => { // Arrange const mockEngine = { context: null, @@ -422,7 +445,7 @@ describe('EngineService', () => { ); }); - it('should handle missing vault metadata in subscribeOnceIf callback', async () => { + it('handles missing vault metadata in subscribeOnceIf callback without errors', async () => { // Types for Engine mock interface MockEngineType { controllerMessenger: { @@ -468,7 +491,7 @@ describe('EngineService', () => { ); }); - it('should handle missing vault metadata in update callback', async () => { + it('handles missing vault metadata in update callback without errors', async () => { // Types for Engine mock interface MockEngineType { controllerMessenger: { @@ -517,7 +540,7 @@ describe('EngineService', () => { ); }); - it('should skip CronjobController events', async () => { + it('skips CronjobController events', async () => { // Types for Engine mock interface MockEngineType { controllerMessenger: { @@ -555,7 +578,7 @@ describe('EngineService', () => { }); describe('start method conditions', () => { - it('should handle existing user with vault check', async () => { + it('logs vault check for existing user', async () => { // Arrange const mockGetState = jest.fn().mockReturnValue({ user: { existingUser: true }, @@ -582,7 +605,7 @@ describe('EngineService', () => { ); }); - it('should handle existing user without vault', async () => { + it('logs missing vault for existing user', async () => { // Arrange const mockGetState = jest.fn().mockReturnValue({ user: { existingUser: true }, @@ -609,7 +632,7 @@ describe('EngineService', () => { ); }); - it('should handle new user (no existing user flag)', async () => { + it('skips vault check for new user without existing user flag', async () => { // Arrange const mockGetState = jest.fn().mockReturnValue({ user: { existingUser: false }, @@ -677,7 +700,7 @@ describe('EngineService', () => { }); }); - it('should set up persistence subscriptions for controllers with persistent state', async () => { + it('sets up persistence subscriptions for controllers with persistent state', async () => { // Act await engineService.start(); @@ -703,7 +726,7 @@ describe('EngineService', () => { ); }); - it('should create persist controller with correct debounce time', async () => { + it('creates persist controller with 200ms debounce time', async () => { // Act await engineService.start(); @@ -711,7 +734,7 @@ describe('EngineService', () => { expect(mockCreatePersistController).toHaveBeenCalledWith(200); }); - it('should skip CronjobController state change events', async () => { + it('skips CronjobController state change events', async () => { // Act await engineService.start(); @@ -732,7 +755,7 @@ describe('EngineService', () => { expect(mockSubscribe).toHaveBeenCalledTimes(4); // KeyringController (2x), PreferencesController, NetworkController }); - it('should handle controller state changes correctly', async () => { + it('persists controller state changes to filesystem', async () => { // Arrange await engineService.start(); @@ -857,7 +880,7 @@ describe('EngineService', () => { expect(skipMessages).toHaveLength(0); }); - it('should handle missing controllerMessenger gracefully', async () => { + it('handles missing controllerMessenger without errors', async () => { // Arrange Object.defineProperty(Engine, 'controllerMessenger', { value: null, diff --git a/app/core/EngineService/EngineService.ts b/app/core/EngineService/EngineService.ts index 91035dc26c2..a602f55abd9 100644 --- a/app/core/EngineService/EngineService.ts +++ b/app/core/EngineService/EngineService.ts @@ -99,6 +99,11 @@ export class EngineService { update_bg_state_cb(controllerName), ); }); + + // CRITICAL: Set up filesystem persistence for all controllers + // This is called automatically after Redux subscriptions to ensure + // both Redux and filesystem are kept in sync when controller state changes + this.setupEnginePersistence(); }; /** @@ -149,8 +154,6 @@ export class EngineService { Engine.init(state, null, metaMetricsId); // `Engine.init()` call mutates `typeof UntypedEngine` to `TypedEngine` this.initializeControllers(Engine as unknown as TypedEngine); - - this.setupEnginePersistence(); } catch (error) { trackVaultCorruption((error as Error).message, { error_type: 'engine_initialization_failure', @@ -206,7 +209,6 @@ export class EngineService { eventName, async (controllerState: StateConstraint) => { try { - // Filter out non-persistent fields based on controller metadata const filteredState = getPersistentState( controllerState, // @ts-expect-error - Engine context has stateless controllers, so metadata may not be available