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/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 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"