From fabe885167500749fac43587867b792123aade6a Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Wed, 17 Dec 2025 17:32:35 +0000 Subject: [PATCH 1/9] fix: Navigating twice to the login screen on onboarding on login (#23974) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Navigating twice to login fix ## **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] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **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] > On onboarding unlock, pass navigateToLogin to Authentication.lockApp and remove explicit navigation; update tests to reflect new behavior. > > - **Onboarding flow**: > - Update `onLogin` in `app/components/Views/Onboarding/index.tsx` to call `Authentication.lockApp({ navigateToLogin: true })` when `passwordSet` is true, removing `navigation.replace(Routes.ONBOARDING.LOGIN)`. > - **Tests**: > - Adjust `app/components/Views/Onboarding/index.test.tsx` to stop asserting `navigation.replace` to `Routes.ONBOARDING.LOGIN`, verifying only `Authentication.lockApp` is called. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 412ba83e2dd5f982e9e23ce40ee768a129bf4839. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- app/components/Views/Onboarding/index.test.tsx | 1 - app/components/Views/Onboarding/index.tsx | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/components/Views/Onboarding/index.test.tsx b/app/components/Views/Onboarding/index.test.tsx index 9642454fae6..9da39491973 100644 --- a/app/components/Views/Onboarding/index.test.tsx +++ b/app/components/Views/Onboarding/index.test.tsx @@ -629,7 +629,6 @@ describe('Onboarding', () => { }); expect(Authentication.lockApp).toHaveBeenCalled(); - expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.LOGIN); }); }); diff --git a/app/components/Views/Onboarding/index.tsx b/app/components/Views/Onboarding/index.tsx index daeae05e594..4eaf9d2a06b 100644 --- a/app/components/Views/Onboarding/index.tsx +++ b/app/components/Views/Onboarding/index.tsx @@ -295,8 +295,7 @@ const Onboarding = () => { await Authentication.resetVault(); navigation.replace(Routes.ONBOARDING.HOME_NAV); } else { - await Authentication.lockApp(); - navigation.replace(Routes.ONBOARDING.LOGIN); + await Authentication.lockApp({ navigateToLogin: true }); } }, [navigation, passwordSet]); From 9ea21490cf08bb6d57246b96a9d35913d7e51ba2 Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Wed, 17 Dec 2025 17:35:18 +0000 Subject: [PATCH 2/9] chore: improve state logs to have more info (#23956) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Improve state logs to improve the information gathered from users with trouble, specially on the login ## **Changelog** CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Download state logs Scenario: user download state logs Given [describe expected initial app state] When user long presses fox at login screen for 10 seconds Then state logs should be downloaded and include if vault exists ``` ## **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). - [ ] 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** - [ ] 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] > Enhances exported state logs with sanitized Keyring/Seedless details, logged-in status, and environment/build metadata, with expanded test coverage and updated snapshots. > > - **State log generation (`app/util/logs/index.ts`)**: > - Add `loggedIn` flag to output; default `true` and pass-through parameter. > - Include `KeyringController` summary with `keyrings`, `isUnlocked`, and computed `vaultExists` (no vault contents). > - Replace `SeedlessOnboardingController` with sanitized state: includes `userId`, `authConnection`, `authConnectionId`, `authPubKey`, booleans for sensitive fields, and filtered `socialBackupsMetadata`/`nodeAuthTokens` (no secrets); handles `null`/`undefined`. > - `downloadStateLogs`: append `environment`, `remoteFeatureFlagEnvironment`, `remoteFeatureFlagDistribution`, `otaVersion`, `runtimeVersion`, and conditionally `metaMetricsId` (only if opted in); platform-specific sharing preserved. > - **Tests (`app/util/logs/index.test.ts`)**: > - New/updated tests for `vaultExists` cases, `loggedIn` flag behavior, Seedless sanitization (including auth fields and filtering), null/undefined handling, env/OTA metadata inclusion, and MetaMetrics opt-out. > - Snapshot updates reflecting structured keyrings, `vaultExists`, sanitized Seedless state, and `loggedIn` field. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 305fbf34b7f72fadd54c616a4f86b7716a42efff. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../logs/__snapshots__/index.test.ts.snap | 51 +- app/util/logs/index.test.ts | 467 +++++++++++++++++- app/util/logs/index.ts | 13 +- 3 files changed, 510 insertions(+), 21 deletions(-) diff --git a/app/util/logs/__snapshots__/index.test.ts.snap b/app/util/logs/__snapshots__/index.test.ts.snap index 6170b6d9a4b..3d862aa6ee5 100644 --- a/app/util/logs/__snapshots__/index.test.ts.snap +++ b/app/util/logs/__snapshots__/index.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`logs :: generateStateLogs Sanitized SeedlessOnboardingController State Able to generate logs with Sanitized SeedlessOnboardingController sensitive data 1`] = ` +exports[`logs :: generateStateLogs Sanitized SeedlessOnboardingController State generates logs with sanitized SeedlessOnboardingController sensitive data 1`] = ` { "appVersion": "1", "buildNumber": "123", @@ -128,9 +128,28 @@ exports[`logs :: generateStateLogs Sanitized SeedlessOnboardingController State "KeyringController": { "isUnlocked": true, "keyrings": [ - "keyring1", - "keyring2", + { + "accounts": [ + "0x1", + ], + "metadata": { + "id": "keyring1", + "name": "", + }, + "type": "HD Key Tree", + }, + { + "accounts": [ + "0x2", + ], + "metadata": { + "id": "keyring2", + "name": "", + }, + "type": "Simple Key Pair", + }, ], + "vaultExists": false, }, "LoggingController": { "logs": {}, @@ -747,6 +766,7 @@ exports[`logs :: generateStateLogs Sanitized SeedlessOnboardingController State }, }, }, + "loggedIn": true, "metaMetricsId": "6D796265-7374-4953-6D65-74616D61736B", } `; @@ -879,9 +899,28 @@ exports[`logs :: generateStateLogs generates a valid json export 1`] = ` "KeyringController": { "isUnlocked": true, "keyrings": [ - "keyring1", - "keyring2", + { + "accounts": [ + "0x1", + ], + "metadata": { + "id": "keyring1", + "name": "", + }, + "type": "HD Key Tree", + }, + { + "accounts": [ + "0x2", + ], + "metadata": { + "id": "keyring2", + "name": "", + }, + "type": "Simple Key Pair", + }, ], + "vaultExists": false, }, "LoggingController": { "logs": {}, @@ -1327,6 +1366,7 @@ exports[`logs :: generateStateLogs generates a valid json export 1`] = ` "accessToken": false, "encryptedKeyringEncryptionKey": false, "encryptedSeedlessEncryptionKey": false, + "isSeedlessOnboardingUserAuthenticated": false, "metadataAccessToken": false, "nodeAuthTokens": [], "refreshToken": false, @@ -1480,6 +1520,7 @@ exports[`logs :: generateStateLogs generates a valid json export 1`] = ` }, }, }, + "loggedIn": true, "metaMetricsId": "6D796265-7374-4953-6D65-74616D61736B", } `; diff --git a/app/util/logs/index.test.ts b/app/util/logs/index.test.ts index 0fcf4cbd751..83c7e8dd7b3 100644 --- a/app/util/logs/index.test.ts +++ b/app/util/logs/index.test.ts @@ -15,6 +15,7 @@ import { merge } from 'lodash'; import MetaMetrics from '../../core/Analytics/MetaMetrics'; import Engine from '../../core/Engine'; import { SecretType } from '@metamask/seedless-onboarding-controller'; +import { KeyringObject, KeyringTypes } from '@metamask/keyring-controller'; jest.mock('react-native-fs', () => ({ DocumentDirectoryPath: '/mock/path', @@ -74,6 +75,29 @@ const mockMetrics = { (MetaMetrics.getInstance as jest.Mock).mockReturnValue(mockMetrics); describe('logs :: generateStateLogs', () => { + beforeEach(() => { + Engine.context.KeyringController.state = { + keyrings: [ + { + accounts: ['0x1'], + type: KeyringTypes.hd, + metadata: { id: 'keyring1', name: '' }, + }, + { + accounts: ['0x2'], + type: KeyringTypes.simple, + metadata: { id: 'keyring2', name: '' }, + }, + ] as KeyringObject[], + isUnlocked: true, + vault: undefined, + }; + Engine.context.SeedlessOnboardingController.state = { + socialBackupsMetadata: [], + isSeedlessOnboardingUserAuthenticated: false, + } as typeof Engine.context.SeedlessOnboardingController.state; + }); + it('generates a valid json export', async () => { const mockStateInput = { appVersion: '1', @@ -93,17 +117,26 @@ describe('logs :: generateStateLogs', () => { expect(JSON.parse(logs)).toMatchSnapshot(); }); - it('generates the expected state logs without the explicitly deleted controller states', async () => { + it('excludes deleted controller states from logs', () => { const mockStateInput = { engine: { backgroundState: { ...backgroundState, + NftController: { nfts: [] }, + TokensController: { tokens: [] }, + TokenDetectionController: { detectedTokens: [] }, + NftDetectionController: { detectedNfts: [] }, + PhishingController: { whitelist: [] }, + AssetsContractController: { assets: [] }, + DeFiPositionsController: { positions: [] }, + PredictController: { predictions: [] }, KeyringController: { vault: 'vault mock', }, }, }, }; + const logs = generateStateLogs(mockStateInput); expect(logs.includes('NftController')).toBe(false); @@ -112,6 +145,8 @@ describe('logs :: generateStateLogs', () => { expect(logs.includes('TokenDetectionController')).toBe(false); expect(logs.includes('NftDetectionController')).toBe(false); expect(logs.includes('PhishingController')).toBe(false); + expect(logs.includes('DeFiPositionsController')).toBe(false); + expect(logs.includes('PredictController')).toBe(false); expect(logs.includes("vault: 'vault mock'")).toBe(false); }); @@ -126,6 +161,7 @@ describe('logs :: generateStateLogs', () => { }, }, }; + const logs = generateStateLogs(mockStateInput); const parsedLogs = JSON.parse(logs); @@ -134,6 +170,194 @@ describe('logs :: generateStateLogs', () => { ); }); + it('sets vaultExists to true when vault has a value', () => { + Engine.context.KeyringController.state = { + keyrings: [ + { + accounts: ['0x1'], + type: KeyringTypes.hd, + metadata: { id: 'keyring1', name: '' }, + }, + ] as KeyringObject[], + isUnlocked: true, + vault: 'some-vault-data', + }; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.KeyringController.vaultExists, + ).toBe(true); + }); + + it('sets vaultExists to false when vault is null', () => { + Engine.context.KeyringController.state = { + keyrings: [ + { + accounts: ['0x1'], + type: KeyringTypes.hd, + metadata: { id: 'keyring1', name: '' }, + }, + ] as KeyringObject[], + isUnlocked: true, + // @ts-expect-error - testing null vault handling + vault: null, + }; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.KeyringController.vaultExists, + ).toBe(false); + }); + + it('sets vaultExists to false when vault is undefined', () => { + Engine.context.KeyringController.state = { + keyrings: [ + { + accounts: ['0x1'], + type: KeyringTypes.hd, + metadata: { id: 'keyring1', name: '' }, + }, + ] as KeyringObject[], + isUnlocked: true, + vault: undefined, + }; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.KeyringController.vaultExists, + ).toBe(false); + }); + + it('sets vaultExists to false when vault is empty string', () => { + Engine.context.KeyringController.state = { + keyrings: [ + { + accounts: ['0x1'], + type: KeyringTypes.hd, + metadata: { id: 'keyring1', name: '' }, + }, + ] as KeyringObject[], + isUnlocked: true, + vault: '', + }; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.KeyringController.vaultExists, + ).toBe(false); + }); + + it('sets vaultExists to false when KeyringController state is undefined', () => { + // @ts-expect-error - testing undefined state handling + Engine.context.KeyringController.state = undefined; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.KeyringController.vaultExists, + ).toBe(false); + }); + + it('includes loggedIn parameter in generated logs', () => { + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput, false); + const parsedLogs = JSON.parse(logs); + + expect(parsedLogs.loggedIn).toBe(false); + }); + + it('defaults loggedIn to true when not provided', () => { + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect(parsedLogs.loggedIn).toBe(true); + }); + it('generates extra logs if values added to the state object parameter', () => { const mockStateInput = { appVersion: '1', @@ -163,7 +387,14 @@ describe('logs :: generateStateLogs', () => { }); describe('Sanitized SeedlessOnboardingController State', () => { - it('Able to generate logs when SeedlessOnboardingController state is empty', () => { + beforeEach(() => { + Engine.context.SeedlessOnboardingController.state = { + socialBackupsMetadata: [], + isSeedlessOnboardingUserAuthenticated: false, + } as typeof Engine.context.SeedlessOnboardingController.state; + }); + + it('generates logs when SeedlessOnboardingController state is empty', () => { const mockStateInput = { appVersion: '1', buildNumber: '123', @@ -203,7 +434,7 @@ describe('logs :: generateStateLogs', () => { expect(revokeToken).toBe(false); }); - it('Able to generate logs with Sanitized SeedlessOnboardingController sensitive data', () => { + it('generates logs with sanitized SeedlessOnboardingController sensitive data', () => { Engine.context.SeedlessOnboardingController.state = { userId: 'userId', isSeedlessOnboardingUserAuthenticated: true, @@ -281,6 +512,222 @@ describe('logs :: generateStateLogs', () => { expect(JSON.parse(logs)).toMatchSnapshot(); }); + + it('includes authConnection fields in sanitized state', () => { + Engine.context.SeedlessOnboardingController.state = { + authConnection: + 'google' as typeof Engine.context.SeedlessOnboardingController.state.authConnection, + authConnectionId: 'connection-id-123', + authPubKey: 'pub-key-123', + userId: 'user-123', + socialBackupsMetadata: [], + isSeedlessOnboardingUserAuthenticated: false, + } as typeof Engine.context.SeedlessOnboardingController.state; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .authConnection, + ).toBe('google'); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .authConnectionId, + ).toBe('connection-id-123'); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .authPubKey, + ).toBe('pub-key-123'); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController.userId, + ).toBe('user-123'); + }); + + it('handles SeedlessOnboardingController state being null', () => { + // @ts-expect-error - testing null state handling + Engine.context.SeedlessOnboardingController.state = null; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController.vault, + ).toBe(false); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .socialBackupsMetadata, + ).toEqual([]); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .nodeAuthTokens, + ).toEqual([]); + }); + + it('handles SeedlessOnboardingController state being undefined', () => { + // @ts-expect-error - testing undefined state handling + Engine.context.SeedlessOnboardingController.state = undefined; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController.vault, + ).toBe(false); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .socialBackupsMetadata, + ).toEqual([]); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .nodeAuthTokens, + ).toEqual([]); + }); + + it('filters out sensitive data from socialBackupsMetadata', () => { + Engine.context.SeedlessOnboardingController.state = { + socialBackupsMetadata: [ + { + type: SecretType.Mnemonic, + keyringId: 'keyring1', + hash: 'sensitive-hash', + }, + { + type: SecretType.PrivateKey, + keyringId: 'keyring2', + hash: 'another-hash', + }, + ], + isSeedlessOnboardingUserAuthenticated: false, + } as typeof Engine.context.SeedlessOnboardingController.state; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .socialBackupsMetadata, + ).toEqual([ + { type: SecretType.Mnemonic, keyringId: 'keyring1' }, + { type: SecretType.PrivateKey, keyringId: 'keyring2' }, + ]); + }); + + it('filters out sensitive data from nodeAuthTokens', () => { + Engine.context.SeedlessOnboardingController.state = { + nodeAuthTokens: [ + { + nodeIndex: 1, + nodePubKey: 'pub-key-1', + authToken: 'sensitive-token-1', + }, + { + nodeIndex: 2, + nodePubKey: 'pub-key-2', + authToken: 'sensitive-token-2', + }, + ], + socialBackupsMetadata: [], + isSeedlessOnboardingUserAuthenticated: false, + } as typeof Engine.context.SeedlessOnboardingController.state; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .nodeAuthTokens, + ).toEqual([ + { nodeIndex: 1, nodePubKey: 'pub-key-1' }, + { nodeIndex: 2, nodePubKey: 'pub-key-2' }, + ]); + }); + + it('handles empty arrays in socialBackupsMetadata and nodeAuthTokens', () => { + Engine.context.SeedlessOnboardingController.state = { + socialBackupsMetadata: [], + nodeAuthTokens: [], + isSeedlessOnboardingUserAuthenticated: false, + } as typeof Engine.context.SeedlessOnboardingController.state; + + const mockStateInput = { + engine: { + backgroundState: { + ...backgroundState, + KeyringController: { + vault: 'vault mock', + }, + }, + }, + }; + + const logs = generateStateLogs(mockStateInput); + const parsedLogs = JSON.parse(logs); + + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .socialBackupsMetadata, + ).toEqual([]); + expect( + parsedLogs.engine.backgroundState.SeedlessOnboardingController + .nodeAuthTokens, + ).toEqual([]); + }); }); }); @@ -289,7 +736,7 @@ describe('logs :: downloadStateLogs', () => { jest.clearAllMocks(); }); - it('should generate and share logs successfully on iOS', async () => { + it('generates and shares logs on iOS', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); @@ -320,7 +767,7 @@ describe('logs :: downloadStateLogs', () => { }); }); - it('should generate and share logs successfully on Android', async () => { + it('generates and shares logs on Android', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); @@ -347,7 +794,7 @@ describe('logs :: downloadStateLogs', () => { }); }); - it('should handle errors during log generation', async () => { + it('logs error when state is null during log generation', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); @@ -364,7 +811,7 @@ describe('logs :: downloadStateLogs', () => { ); }); - it('should handle errors during file writing on iOS', async () => { + it('logs error when file writing fails on iOS', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); @@ -392,7 +839,7 @@ describe('logs :: downloadStateLogs', () => { ); }); - it('should handle errors during sharing', async () => { + it('logs error when sharing fails', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); @@ -418,7 +865,7 @@ describe('logs :: downloadStateLogs', () => { ); }); - it('should handle loggedIn as false', async () => { + it('includes loggedIn as false in generated logs', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); @@ -444,7 +891,7 @@ describe('logs :: downloadStateLogs', () => { }); }); - it('does not include metametrics id if not opt-in', async () => { + it('excludes metametrics id when not opted in', async () => { (getApplicationName as jest.Mock).mockResolvedValue('TestApp'); (getVersion as jest.Mock).mockResolvedValue('1.0.0'); (getBuildNumber as jest.Mock).mockResolvedValue('100'); diff --git a/app/util/logs/index.ts b/app/util/logs/index.ts index 5aa184a239c..f02435e0dcf 100644 --- a/app/util/logs/index.ts +++ b/app/util/logs/index.ts @@ -99,10 +99,6 @@ export const generateStateLogs = (state: any, loggedIn = true): string => { // Remove Keyring controller data so that encrypted vault is not included in logs delete fullState.engine.backgroundState.KeyringController; - if (!loggedIn) { - return JSON.stringify(fullState); - } - const { KeyringController } = Engine.context; const newState = { ...fullState, @@ -111,13 +107,18 @@ export const generateStateLogs = (state: any, loggedIn = true): string => { backgroundState: { ...fullState.engine.backgroundState, KeyringController: { - keyrings: KeyringController.state.keyrings, - isUnlocked: KeyringController.state.isUnlocked, + keyrings: KeyringController.state?.keyrings, + isUnlocked: KeyringController.state?.isUnlocked, + vaultExists: + KeyringController.state?.vault !== null && + KeyringController.state?.vault !== undefined && + KeyringController.state?.vault !== '', }, SeedlessOnboardingController: getSanitizedSeedlessOnboardingControllerState(), }, }, + loggedIn, }; return JSON.stringify(newState); From 97d71e6da3201ad537c19ae35cf94c36b7ebb5a6 Mon Sep 17 00:00:00 2001 From: Michal Szorad Date: Wed, 17 Dec 2025 18:46:41 +0100 Subject: [PATCH 3/9] feat(perps): add trade details navigation and trade again button (#24067) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR implements the feature to display trade details when tapping on a recent trade in the Perps home screen and asset detail screen. ## **Changelog** CHANGELOG entry: Added ability to view trade details when tapping on recent trades in Perps, with a "Trade again" button to quickly navigate back to trading ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2193 ## **Manual testing steps** ```gherkin Feature: Perps trade details navigation Scenario: user taps on a recent trade to view details Given user is on the Perps home screen And user has recent trades displayed When user taps on a recent trade Then user sees the trade details screen with date, size, entry/close price, fees, and P&L Scenario: user taps Trade again to navigate to market Given user is on the trade details screen When user taps the "Trade again" button Then user is navigated to the perp market detail page for that asset ``` ## **Screenshots/Recordings** ### **Before** Does not show trade again button ### **After** Shows trade again button Simulator Screenshot - iPhone 17 Pro - 2025-12-16 at 12 33 40 ## **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** - [ ] 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] > Adds a Trade again button on trade details that deep-links to the market, and changes recent activity item presses to open position transaction details; updates tests and i18n. > > - **Perps trade details (`PerpsPositionTransactionView`)**: > - Add `Trade again` button to navigate to `Routes.PERPS.ROOT` → `Routes.PERPS.MARKET_DETAILS` with minimal `market` (from `transaction.asset`) and `source: 'trade_details'`. > - Build `market` via `useMemo`; hide CTA when `asset` is undefined. > - Keep block explorer navigation; minor refactor/imports. > - **Recent activity navigation (`PerpsRecentActivityList`)**: > - Change transaction item press to navigate to `Routes.PERPS.POSITION_TRANSACTION` with the full `transaction` instead of market details. > - **Tests**: > - Add/adjust tests for new navigation flows and conditional rendering of `Trade again`. > - **Localization**: > - Add `perps.transactions.trade_again` string. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d42aef3105dd6af423c230f3a380d1402162cfc1. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../PerpsPositionTransactionView.test.tsx | 38 +++++++++++++++++++ .../PerpsPositionTransactionView.tsx | 37 +++++++++++++++++- .../PerpsRecentActivityList.test.tsx | 24 ++++++------ .../PerpsRecentActivityList.tsx | 9 ++--- locales/languages/en.json | 1 + 5 files changed, 90 insertions(+), 19 deletions(-) diff --git a/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.test.tsx b/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.test.tsx index 9776b363ee9..3e58a74800e 100644 --- a/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.test.tsx +++ b/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.test.tsx @@ -10,6 +10,7 @@ import renderWithProvider, { import { backgroundState } from '../../../../../util/test/initial-root-state'; import { MOCK_ACCOUNTS_CONTROLLER_STATE } from '../../../../../util/test/accountsControllerTestUtils'; import { RootState } from '../../../../../reducers'; +import Routes from '../../../../../constants/navigation/Routes'; const mockNavigate = jest.fn(); const mockSetOptions = jest.fn(); @@ -625,4 +626,41 @@ describe('PerpsPositionTransactionView', () => { // Should still render date row expect(getByText('Date')).toBeOnTheScreen(); }); + + it('navigates to market details when Trade again button is pressed', () => { + const { getByText } = renderWithProvider(, { + state: mockInitialState, + }); + + const tradeAgainButton = getByText('Trade again'); + fireEvent.press(tradeAgainButton); + + expect(mockNavigate).toHaveBeenCalledWith(Routes.PERPS.ROOT, { + screen: Routes.PERPS.MARKET_DETAILS, + params: { + market: { symbol: 'ETH', name: 'ETH' }, + source: 'trade_details', + }, + }); + }); + + it('does not render Trade again button when transaction asset is undefined', () => { + const transactionWithoutAsset = { + ...mockTransaction, + asset: undefined, + }; + + mockUseRoute.mockReturnValue({ + params: { transaction: transactionWithoutAsset }, + }); + + const { queryByText } = renderWithProvider( + , + { + state: mockInitialState, + }, + ); + + expect(queryByText('Trade again')).toBeNull(); + }); }); diff --git a/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.tsx b/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.tsx index 2c59753eaaa..6ce9b8c76fe 100644 --- a/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.tsx +++ b/app/components/UI/Perps/Views/PerpsTransactionsView/PerpsPositionTransactionView.tsx @@ -3,7 +3,7 @@ import { useNavigation, useRoute, } from '@react-navigation/native'; -import React from 'react'; +import React, { useMemo } from 'react'; import { ScrollView, View } from 'react-native'; import { strings } from '../../../../../../locales/i18n'; import Text, { @@ -20,6 +20,7 @@ import Button, { } from '../../../../../component-library/components/Buttons/Button'; import { useStyles } from '../../../../../component-library/hooks'; import { selectSelectedInternalAccountByScope } from '../../../../../selectors/multichainAccounts/accounts'; +import Routes from '../../../../../constants/navigation/Routes'; import ScreenView from '../../../../Base/ScreenView'; import { getPerpsTransactionsDetailsNavbar } from '../../../Navbar'; import PerpsTransactionDetailAssetHero from '../../components/PerpsTransactionDetailAssetHero'; @@ -36,6 +37,7 @@ import { PRICE_RANGES_UNIVERSAL, } from '../../utils/formatUtils'; import { styleSheet } from './PerpsPositionTransactionView.styles'; +import type { PerpsMarketData } from '../../controllers/types'; const PerpsPositionTransactionView: React.FC = () => { const { styles } = useStyles(styleSheet, {}); @@ -49,6 +51,16 @@ const PerpsPositionTransactionView: React.FC = () => { // Get transaction from route params const transaction = route.params?.transaction as PerpsTransaction; + // Create a minimal market object from transaction asset for navigation + // This is used to navigate to the market details page without requiring the stream provider + const market = useMemo | undefined>( + () => + transaction?.asset + ? { symbol: transaction.asset, name: transaction.asset } + : undefined, + [transaction?.asset], + ); + navigation.setOptions( getPerpsTransactionsDetailsNavbar( navigation, @@ -83,6 +95,19 @@ const PerpsPositionTransactionView: React.FC = () => { }); }; + const handleTradeAgain = () => { + if (!market) { + return; + } + navigation.navigate(Routes.PERPS.ROOT, { + screen: Routes.PERPS.MARKET_DETAILS, + params: { + market, + source: 'trade_details', + }, + }); + }; + // Main detail rows - only show if values exist const mainDetailRows = [ { @@ -215,6 +240,16 @@ const PerpsPositionTransactionView: React.FC = () => { + {/* Trade again button */} + {market && ( +