From 66388aa74f7b06222ec924fbec8809408665369b Mon Sep 17 00:00:00 2001 From: ffmcgee <51971598+ffmcgee725@users.noreply.github.com> Date: Fri, 14 Nov 2025 10:42:48 +0100 Subject: [PATCH 01/17] fix: switching networks from dapp permissions dapp icon has no effect (#22692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** While connected to a dapp, if the user tries to switch networks from dapp permissions, this has no effect. When switching networks, the icon displayed is always the same. This happens because `MultichainPermissionsSummary.tsx` uses `.hostname` while `SelectedNetworkController` stores domains by origin (e.g., "https://example.com"). This mismatch prevents the lookup from finding the per-dapp network. This PR proposes a fix by using `.origin` instead. ## **Changelog** CHANGELOG entry: fix issue where switching networks from dapp permissions dapp icon has no effect ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/22423 ## **Manual testing steps** ```gherkin Feature: fix switching networks from dapp permissions dapp icon having no effect Scenario: user switches network Given it's done via top right icon of In App Browser When user switches network Then network icon should update accordingly ``` ## **Screenshots/Recordings** ### **Before** ### **After** https://github.com/user-attachments/assets/472ff108-8e39-4a96-899a-93c9983bb260 ## **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] > Replaces hostname with full URL origin for dapp host handling in MultichainPermissionsSummary, updating navigation params and tests. > > - **MultichainPermissionsSummary**: > - Compute `hostname` as `URL.origin` instead of `URL.hostname`. > - Pass origin to `useNetworkInfo`, titles, and navigation params for `CONNECTION_DETAILS` and `REVOKE_ALL_ACCOUNT_PERMISSIONS` (`hostInfo.metadata.origin`). > - **Tests**: > - Update expectations to use `https://mock-dapp.example.com` for `origin` in navigation assertions. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ffbe2d115fc9167c11830c9930cdaccef8874f51. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../MultichainPermissionsSummary.test.tsx | 4 ++-- .../MultichainPermissionsSummary.tsx | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.test.tsx b/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.test.tsx index 7bdac8f87dea..0d386a6b700e 100644 --- a/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.test.tsx +++ b/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.test.tsx @@ -449,7 +449,7 @@ describe('MultichainPermissionsSummary', () => { params: expect.objectContaining({ hostInfo: expect.objectContaining({ metadata: expect.objectContaining({ - origin: 'mock-dapp.example.com', + origin: 'https://mock-dapp.example.com', }), }), onRevokeAll: expect.any(Function), @@ -475,7 +475,7 @@ describe('MultichainPermissionsSummary', () => { params: expect.objectContaining({ hostInfo: expect.objectContaining({ metadata: expect.objectContaining({ - origin: 'mock-dapp.example.com', + origin: 'https://mock-dapp.example.com', }), }), onRevokeAll: expect.any(Function), diff --git a/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx b/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx index a1eeab2e9829..dc50701e8b19 100644 --- a/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx +++ b/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx @@ -132,7 +132,7 @@ const MultichainPermissionsSummary = ({ const hostname = useMemo(() => { try { - return new URL(currentPageInformation.url).hostname; + return new URL(currentPageInformation.url).origin; } catch { return currentPageInformation.url; } @@ -252,9 +252,7 @@ const MultichainPermissionsSummary = ({ params: { hostInfo: { metadata: { - origin: - currentPageInformation?.url && - new URL(currentPageInformation?.url).hostname, + origin: hostname, }, }, connectionDateTime: new Date().getTime(), From 6ef1fc786640603c7c4641cc67181bef38621e84 Mon Sep 17 00:00:00 2001 From: ffmcgee <51971598+ffmcgee725@users.noreply.github.com> Date: Fri, 14 Nov 2025 10:44:46 +0100 Subject: [PATCH 02/17] feat(SDKConnectV2): add toasts for non-success states (#22610) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR adds to the current implementation of MetaMask Connect toasts to ensure that error messages are displayed in the Wallet when a user rejects a request or when other errors occur during the request lifecycle. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-825 ## **Manual testing steps** ```gherkin Feature: MM Connect toasts for non-success states 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** https://github.com/user-attachments/assets/e977ff69-aedd-4136-99a9-9e9e90f5e663 ## **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] > Show an error toast when bridge responses contain JSON-RPC errors and use the connection id for error notifications; add tests for success/error paths. > > - **SDKConnectV2**: > - **Connection (`services/connection.ts`)**: On bridge `response`, detect JSON-RPC errors (`error` in payload) and call `hostApp.showConnectionError(this.info)`; otherwise call `showReturnToApp(this.info)`. Always forward payload to client. > - **Host App Adapter (`adapters/host-application-adapter.ts`)**: Change `showConnectionError` signature to `conninfo?` and use `conninfo.id` when provided (fallback to timestamp). > - **Types (`types/host-application-adapter.ts`)**: Update interface for `showConnectionError(conninfo?)` and clarify return-to-app success doc. > - **Tests**: > - Add/update cases validating error vs success toasts and that error notifications use the connection id; ensure responses without `id` don’t trigger return-to-app. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 803a6b5cafd63eb9fe466cc88225c48953468523. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../adapters/host-application-adapter.test.ts | 16 +++++ .../adapters/host-application-adapter.ts | 4 +- .../SDKConnectV2/services/connection.test.ts | 69 +++++++++++++++++++ app/core/SDKConnectV2/services/connection.ts | 11 ++- .../types/host-application-adapter.ts | 4 +- 5 files changed, 99 insertions(+), 5 deletions(-) diff --git a/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts b/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts index 744fa8ad6704..4b5a9c69558f 100644 --- a/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts +++ b/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts @@ -118,6 +118,22 @@ describe('HostApplicationAdapter', () => { }); expect(store.dispatch).toHaveBeenCalledTimes(1); }); + + it('dispatches an error notification when request is rejected or fails', () => { + adapter.showConnectionError( + createMockConnectionInfo('session-123', 'Test DApp'), + ); + + expect(showSimpleNotification).toHaveBeenCalledTimes(1); + expect(showSimpleNotification).toHaveBeenCalledWith({ + id: 'session-123', + autodismiss: 5000, + title: 'sdk_connect_v2.show_error.title', + description: 'sdk_connect_v2.show_error.description', + status: 'error', + }); + expect(store.dispatch).toHaveBeenCalledTimes(1); + }); }); describe('showReturnToApp', () => { diff --git a/app/core/SDKConnectV2/adapters/host-application-adapter.ts b/app/core/SDKConnectV2/adapters/host-application-adapter.ts index 288cbc740fae..7f9892b6f14e 100644 --- a/app/core/SDKConnectV2/adapters/host-application-adapter.ts +++ b/app/core/SDKConnectV2/adapters/host-application-adapter.ts @@ -33,10 +33,10 @@ export class HostApplicationAdapter implements IHostApplicationAdapter { store.dispatch(hideNotificationById(conninfo.id)); } - showConnectionError(): void { + showConnectionError(conninfo?: ConnectionInfo): void { store.dispatch( showSimpleNotification({ - id: Date.now().toString(), + id: conninfo?.id || Date.now().toString(), autodismiss: 5000, title: strings('sdk_connect_v2.show_error.title'), description: strings('sdk_connect_v2.show_error.description'), diff --git a/app/core/SDKConnectV2/services/connection.test.ts b/app/core/SDKConnectV2/services/connection.test.ts index b9cbd60beaa3..8b4ba37866ae 100644 --- a/app/core/SDKConnectV2/services/connection.test.ts +++ b/app/core/SDKConnectV2/services/connection.test.ts @@ -331,5 +331,74 @@ describe('Connection', () => { responsePayload, ); }); + + it('shows error toast when bridge response includes an error', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const errorResponsePayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: -32000, + message: 'User rejected the request', + }, + }, + }; + + // Simulate the RPCBridgeAdapter emitting an error response + onBridgeResponseCallback(errorResponsePayload); + + // Should show error toast, not success toast + expect(mockHostApp.showConnectionError).toHaveBeenCalledTimes(1); + expect(mockHostApp.showConnectionError).toHaveBeenCalledWith( + mockConnectionInfo, + ); + expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled(); + + // And still send the error response to the client + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledWith( + errorResponsePayload, + ); + }); + + it('shows success toast for successful response with result', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const successResponsePayload = { + data: { + id: 1, + jsonrpc: '2.0', + result: ['0x123'], + }, + }; + + // Simulate the RPCBridgeAdapter emitting a success response + onBridgeResponseCallback(successResponsePayload); + + // Should show success toast, not error toast + expect(mockHostApp.showReturnToApp).toHaveBeenCalledTimes(1); + expect(mockHostApp.showReturnToApp).toHaveBeenCalledWith( + mockConnectionInfo, + ); + expect(mockHostApp.showConnectionError).not.toHaveBeenCalled(); + + // And still send the response to the client + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledWith( + successResponsePayload, + ); + }); }); }); diff --git a/app/core/SDKConnectV2/services/connection.ts b/app/core/SDKConnectV2/services/connection.ts index 3138ed0b83cc..b4e30a004c5c 100644 --- a/app/core/SDKConnectV2/services/connection.ts +++ b/app/core/SDKConnectV2/services/connection.ts @@ -46,7 +46,16 @@ export class Connection { // If the payload includes an id, its a JSON-RPC response, otherwise its a notification if ('data' in payload && 'id' in payload.data) { - this.hostApp.showReturnToApp(this.info); + const responseData = payload.data; + // Check if the response is an error (JSON-RPC error responses have an 'error' property) + const isError = + 'error' in responseData && responseData.error !== undefined; + + if (isError) { + this.hostApp.showConnectionError(this.info); + } else { + this.hostApp.showReturnToApp(this.info); + } } this.client.sendResponse(payload); diff --git a/app/core/SDKConnectV2/types/host-application-adapter.ts b/app/core/SDKConnectV2/types/host-application-adapter.ts index ddd729171a32..acd12299b0f3 100644 --- a/app/core/SDKConnectV2/types/host-application-adapter.ts +++ b/app/core/SDKConnectV2/types/host-application-adapter.ts @@ -23,10 +23,10 @@ export interface IHostApplicationAdapter { /** * Displays a global, non-interactive error modal. */ - showConnectionError(): void; + showConnectionError(conninfo?: ConnectionInfo): void; /** - * Displays a "Return to App" toast notification. + * Displays a "Return to App" toast notification for successful requests. */ showReturnToApp(conninfo: ConnectionInfo): void; From 759b2f8c9922fa5aa9d577c505e8aa18a3cdb7f1 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Fri, 14 Nov 2025 10:54:41 +0100 Subject: [PATCH 03/17] fix: Change to available fiat value text when fiat mode is enabled (#22541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR adds the total available fiat value when user switch to fiat mode. ## **Changelog** CHANGELOG entry: Change available value text to total fiat value when fiat mode is enabled ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/6204 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** Screenshot 2025-11-12 at 14 58 39 ### **After** Screenshot 2025-11-12 at 14 58 27 ## **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] > Display available balance in fiat when fiat mode is on, with memoized computation and updated tests. > > - **Send Amount UI**: > - Compute `balanceDisplayValue` via `useMemo`; in fiat mode shows `getFiatDisplayValue(balance) + " available"`, otherwise `${balance} ${balanceUnit} available`. > - Replace inline balance text with `balanceDisplayValue` in `amount.tsx`. > - **Tests**: > - Update `amount.test.tsx` to assert `$ 250.00 available` when fiat mode is active. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ea1e325014c1dbe2bc65c21a0fb21a623f3efb79. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../components/send/amount/amount.test.tsx | 1 + .../confirmations/components/send/amount/amount.tsx | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/components/Views/confirmations/components/send/amount/amount.test.tsx b/app/components/Views/confirmations/components/send/amount/amount.test.tsx index b526251258bc..fd3356cde1e3 100644 --- a/app/components/Views/confirmations/components/send/amount/amount.test.tsx +++ b/app/components/Views/confirmations/components/send/amount/amount.test.tsx @@ -219,6 +219,7 @@ describe('Amount', () => { fireEvent.press(getByTestId('fiat_toggle')); fireEvent.press(getByRole('button', { name: '5' })); expect(getByText('1 ETH')).toBeTruthy(); + expect(getByText('$ 250.00 available')).toBeTruthy(); }); it('calls metrics methods on changing fiat mode', () => { diff --git a/app/components/Views/confirmations/components/send/amount/amount.tsx b/app/components/Views/confirmations/components/send/amount/amount.tsx index 2a11773a6ed1..aa704e10d275 100644 --- a/app/components/Views/confirmations/components/send/amount/amount.tsx +++ b/app/components/Views/confirmations/components/send/amount/amount.tsx @@ -98,6 +98,14 @@ export const Amount = () => { assetSymbol ?? (parseInt(balance) === 1 ? strings('send.unit') : strings('send.units')); + const balanceDisplayValue = useMemo( + () => + fiatMode + ? `${getFiatDisplayValue(balance)} ${strings('send.available')}` + : `${balance} ${balanceUnit} ${strings('send.available')}`, + [balance, balanceUnit, fiatMode, getFiatDisplayValue], + ); + const defaultValue = fiatMode ? '0.00' : '0'; let textColor = TextColor.Default; if (amountError) { @@ -166,9 +174,7 @@ export const Amount = () => { - {`${balance} ${balanceUnit} ${strings('send.available')}`} + {balanceDisplayValue} Date: Fri, 14 Nov 2025 11:46:29 +0100 Subject: [PATCH 04/17] fix: remove browser navigation on account click when in initial connection flow (#22604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR removes browser navigation when a user clicks the account group in the initial connection flow. ## **Changelog** CHANGELOG entry: Fixes navigation on account group click in initial connection flow ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-829 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user clicks account group Given it's on initial connection flow When user clicks account group Then selected account is changed, and no navigation to in app browser happens ``` ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/fc58a302-7177-4d14-90c0-4b73a89eecf6 ### **After** https://github.com/user-attachments/assets/d1a079aa-74bc-43e1-925d-cc4c964e1bec ## **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] > Selecting an account during the initial connection flow now only updates the selected account group without showing a toast or navigating; wiring and tests updated accordingly. > > - **MultichainAccountsConnectedList (`MultichainAccountsConnectedList.tsx`)** > - Add `isConnectionFlow` prop (default `false`). > - Update `handleSelectAccount` to early-return in connection flow (only sets `AccountTreeController.setSelectedAccountGroup`). > - Maintain existing toast + `navigation.navigate(Routes.BROWSER.HOME)` when not in connection flow; update hook deps. > - **MultichainPermissionsSummary (`MultichainPermissionsSummary.tsx`)** > - Pass `isConnectionFlow={!isAlreadyConnected}` to `MultichainAccountsConnectedList` and update memo deps. > - **Tests (`MultichainAccountsConnectedList.test.tsx`)** > - Add/adjust cases to verify: selection-only in connection flow; toast + navigation when not in connection flow. > - Explicitly set `isConnectionFlow` in relevant tests and validate correct group IDs and side effects. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 952b327640448cd0d4ae4d17203c37fc0db677e9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../MultichainAccountsConnectedList.test.tsx | 55 ++++++++++++++++--- .../MultichainAccountsConnectedList.tsx | 17 +++++- .../MultichainPermissionsSummary.tsx | 8 ++- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.test.tsx b/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.test.tsx index f8ce6b627a56..e7473beb70bd 100644 --- a/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.test.tsx +++ b/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.test.tsx @@ -423,8 +423,10 @@ describe('MultichainAccountsConnectedList', () => { }); describe('handleSelectAccount functionality', () => { - it('calls setSelectedAccountGroup when account is selected', () => { - const { getByText } = renderMultichainAccountsConnectedList(); + it('calls setSelectedAccountGroup when account is selected (not in connection flow)', () => { + const { getByText } = renderMultichainAccountsConnectedList({ + isConnectionFlow: false, + }); const accountCell = getByText('Account 1'); @@ -437,7 +439,9 @@ describe('MultichainAccountsConnectedList', () => { }); it('calls setSelectedAccountGroup with correct account ID for different accounts', () => { - const { getByText } = renderMultichainAccountsConnectedList(); + const { getByText } = renderMultichainAccountsConnectedList({ + isConnectionFlow: false, + }); const account1Cell = getByText('Account 1'); const account2Cell = getByText('Account 2'); @@ -456,6 +460,22 @@ describe('MultichainAccountsConnectedList', () => { expect(mockSetSelectedAccountGroup).toHaveBeenCalledTimes(2); }); + + it('only calls setSelectedAccountGroup when account is selected in connection flow', () => { + const mockHandleEdit = jest.fn(); + const { getByText } = renderMultichainAccountsConnectedList({ + isConnectionFlow: true, + handleEditAccountsButtonPress: mockHandleEdit, + }); + + const accountCell = getByText('Account 1'); + fireEvent.press(accountCell); + + // Should only call set selected account group instead of navigating + expect(mockSetSelectedAccountGroup).toHaveBeenCalledTimes(1); + expect(mockHandleEdit).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); }); describe('Toast Functionality', () => { @@ -465,8 +485,10 @@ describe('MultichainAccountsConnectedList', () => { mockSetSelectedAccountGroup.mockClear(); }); - it('shows toast when account is selected', () => { - const { getByText } = renderMultichainAccountsConnectedList(); + it('shows toast when account is selected (not in connection flow)', () => { + const { getByText } = renderMultichainAccountsConnectedList({ + isConnectionFlow: false, + }); const accountCell = getByText('Account 1'); fireEvent.press(accountCell); @@ -487,9 +509,11 @@ describe('MultichainAccountsConnectedList', () => { }); }); - it('navigates to browser home after showing toast', () => { - // Given a connected account - const { getByText } = renderMultichainAccountsConnectedList(); + it('navigates to browser home after showing toast (not in connection flow)', () => { + // Given a connected account (not in connection flow) + const { getByText } = renderMultichainAccountsConnectedList({ + isConnectionFlow: false, + }); // When selecting an account const accountCell = getByText('Account 1'); @@ -499,5 +523,20 @@ describe('MultichainAccountsConnectedList', () => { expect(mockNavigate).toHaveBeenCalledTimes(1); expect(mockNavigate).toHaveBeenCalledWith(Routes.BROWSER.HOME); }); + + it('does not show toast or navigate when in connection flow', () => { + const mockHandleEdit = jest.fn(); + const { getByText } = renderMultichainAccountsConnectedList({ + isConnectionFlow: true, + handleEditAccountsButtonPress: mockHandleEdit, + }); + + const accountCell = getByText('Account 1'); + fireEvent.press(accountCell); + + // Should not show toast or navigate + expect(mockShowToast).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); }); }); diff --git a/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.tsx b/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.tsx index 84c7d44d8c88..c46400774aa4 100644 --- a/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.tsx +++ b/app/components/Views/MultichainAccounts/MultichainAccountsConnectedList/MultichainAccountsConnectedList.tsx @@ -38,10 +38,12 @@ const MultichainAccountsConnectedList = ({ privacyMode, selectedAccountGroups, handleEditAccountsButtonPress, + isConnectionFlow = false, }: { privacyMode: boolean; selectedAccountGroups: AccountGroupObject[]; handleEditAccountsButtonPress: () => void; + isConnectionFlow?: boolean; }) => { const { styles } = useStyles(styleSheet, { itemHeight: 64, @@ -67,6 +69,12 @@ const MultichainAccountsConnectedList = ({ (accountGroup: AccountGroupObject) => { const { AccountTreeController } = Engine.context; AccountTreeController.setSelectedAccountGroup(accountGroup.id); + + // During connection flow, clicking an account should only change the selected account group instead of navigating + if (isConnectionFlow) { + return; + } + const address = iconSeedAddresses[accountGroup.id]; const activeAccountName = accountGroups.find( (group) => group.id === accountGroup.id, @@ -88,7 +96,14 @@ const MultichainAccountsConnectedList = ({ }); navigation.navigate(Routes.BROWSER.HOME); }, - [navigation, iconSeedAddresses, accountAvatarType, toastRef, accountGroups], + [ + isConnectionFlow, + navigation, + iconSeedAddresses, + accountAvatarType, + toastRef, + accountGroups, + ], ); const renderItem = useCallback( diff --git a/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx b/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx index dc50701e8b19..067f00176201 100644 --- a/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx +++ b/app/components/Views/MultichainAccounts/MultichainPermissionsSummary/MultichainPermissionsSummary.tsx @@ -524,10 +524,16 @@ const MultichainPermissionsSummary = ({ privacyMode={privacyMode} selectedAccountGroups={selectedAccountGroups} handleEditAccountsButtonPress={handleEditAccountsButtonPress} + isConnectionFlow={!isAlreadyConnected} {...restAccountsConnectedTabProps} /> ), - [privacyMode, selectedAccountGroups, handleEditAccountsButtonPress], + [ + privacyMode, + selectedAccountGroups, + handleEditAccountsButtonPress, + isAlreadyConnected, + ], ); const renderTabsContent = () => { From 18c3221863fb1b8944c530891f2218354a9ea069 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Fri, 14 Nov 2025 13:12:25 +0100 Subject: [PATCH 05/17] fix: cp-7.59.0 Fix submit loading for nonEVM send transactions (#22697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** NonEVM transacion confirmations are getting stuck on recipient page after cancelling the transaction confirmation. The reason why this is not happening on EVM is the recipient route is unmounting when it navigates to EVM confirmation but for nonEVM recipient page is still there, not reset submit loading state persist and it doesn't allow you to submit again. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/22699 ## **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** https://github.com/user-attachments/assets/57ea1d2d-d43a-4f1a-89f6-ad45e0742813 ### **After** https://github.com/user-attachments/assets/cd09d89d-adff-4f8a-a48c-ced51374adca ## **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] > Await submit and clear `isSubmittingTransaction` in recipient review and list-selection flows to prevent stuck loading. > > - **Send/Recipient (`recipient.tsx`)**: > - Make `handleReview` and `onRecipientSelected(...)` async and `await` `handleSubmitPress(...)`. > - After awaiting, explicitly call `setIsSubmittingTransaction(false)` to reset submit/loading state. > - Preserve prechecks and guards; no other logic changes. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 112ad13e9ebbc60c99594a740bfda78975d52ef2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../confirmations/components/send/recipient/recipient.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/components/Views/confirmations/components/send/recipient/recipient.tsx b/app/components/Views/confirmations/components/send/recipient/recipient.tsx index 92b7b62ff52b..112d7e6267a1 100644 --- a/app/components/Views/confirmations/components/send/recipient/recipient.tsx +++ b/app/components/Views/confirmations/components/send/recipient/recipient.tsx @@ -70,10 +70,11 @@ export const Recipient = () => { } setIsSubmittingTransaction(true); setPastedRecipient(undefined); - handleSubmitPress(resolvedAddress || to); captureRecipientSelected( isPasted ? RecipientInputMethod.Pasted : RecipientInputMethod.Manual, ); + await handleSubmitPress(resolvedAddress || to); + setIsSubmittingTransaction(false); }, [ to, @@ -113,7 +114,7 @@ export const Recipient = () => { | typeof RecipientInputMethod.SelectAccount | typeof RecipientInputMethod.SelectContact, ) => - (recipient: RecipientType) => { + async (recipient: RecipientType) => { if (isSubmittingTransaction) { return; } @@ -125,8 +126,9 @@ export const Recipient = () => { const selectedAddress = recipient.address; setIsRecipientSelectedFromList(true); updateTo(selectedAddress); - handleSubmitPress(selectedAddress); captureRecipientSelected(recipientInputMethod); + await handleSubmitPress(selectedAddress); + setIsSubmittingTransaction(false); }, [ updateTo, From 3e900346596848d462c658f4e85ce7baa11cff8b Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Fri, 14 Nov 2025 13:46:10 +0100 Subject: [PATCH 06/17] fix: Remove raised amount error when value deleted on fiat mode (#22529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** ## **Changelog** CHANGELOG entry: Fix the issue when user removes fiat amount in send flow via keypad but error state persist on fiat symbol ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/22338 ## **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** - [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] > `getNativeValueFn` now returns an empty string when the input amount is empty, and tests are updated to reflect this behavior. > > - **Logic** > - Adjust `getNativeValueFn` in `app/components/Views/confirmations/hooks/send/useCurrencyConversions.ts` to return `''` when `amount === ''`. > - **Tests** > - Update `getNativeValueFn` test in `app/components/Views/confirmations/hooks/send/useCurrencyConversions.test.ts` to expect `''` for empty input and rename the test accordingly. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 13d2826c2eb964b54da27b8b10e2379994a5e83b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../confirmations/hooks/send/useCurrencyConversions.test.ts | 4 ++-- .../confirmations/hooks/send/useCurrencyConversions.ts | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/components/Views/confirmations/hooks/send/useCurrencyConversions.test.ts b/app/components/Views/confirmations/hooks/send/useCurrencyConversions.test.ts index 77a952196138..6ef886bdfb21 100644 --- a/app/components/Views/confirmations/hooks/send/useCurrencyConversions.test.ts +++ b/app/components/Views/confirmations/hooks/send/useCurrencyConversions.test.ts @@ -88,7 +88,7 @@ describe('getNativeValueFn', () => { ).toStrictEqual('10.00'); }); - it('return 0 if input is empty string', () => { + it('return empty string if input is empty string', () => { expect( getNativeValueFn({ conversionRate: 1, @@ -96,7 +96,7 @@ describe('getNativeValueFn', () => { amount: '', decimals: 2, }), - ).toStrictEqual('0'); + ).toStrictEqual(''); }); it('return 0 if input is invalid decimal', () => { diff --git a/app/components/Views/confirmations/hooks/send/useCurrencyConversions.ts b/app/components/Views/confirmations/hooks/send/useCurrencyConversions.ts index 9bf23d2b0119..0ff252a0c815 100644 --- a/app/components/Views/confirmations/hooks/send/useCurrencyConversions.ts +++ b/app/components/Views/confirmations/hooks/send/useCurrencyConversions.ts @@ -65,6 +65,12 @@ export const getNativeValueFn = ({ decimals, exchangeRate, }: ConversionArgs) => { + // In order to handle the case where the amount is empty string, + // we return empty string so it allows to go back initial state of the amount input + if (amount === '') { + return ''; + } + if (!amount || !isValidPositiveNumericString(amount)) { return '0'; } From 862f26e8b84181c5fd514151080bc00514fe670d Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Fri, 14 Nov 2025 19:58:34 +0700 Subject: [PATCH 07/17] feat: ui experience enhancements in createpassword screen (#22687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** * UI experience enhancements in createpassword screen * Open keyboard by default when screen opens and focus on textInput automatically. * Create Password button visibility * Jira: https://consensyssoftware.atlassian.net/browse/SL-299 ## **Changelog** CHANGELOG entry: UI experience enhancements in createpassword screen CHANGELOG entry: Open keyboard by default when screen opens and focus on textInput automatically. CHANGELOG entry: Create Password button visibility ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: UI experience enhancements in createpassword screen 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** https://github.com/user-attachments/assets/bcc47231-d545-433f-b78e-22c6c8364f51 ### **After** https://github.com/user-attachments/assets/10bb584e-6262-41ca-ba55-864ad6186c1c ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Adds autofocus to the password field, disables and validates confirm input with error messaging, replaces CTAs with component-library Buttons, and tweaks layout/styling. > > - **ChoosePassword UI/UX**: > - **Password field**: enable `autoFocus`; focused border color updated to primary (`#4459ff`). > - **Confirm password**: now disabled until a password is entered; adds mismatch error state/text; sets `returnKeyType="done"` and dismisses keyboard on submit; adds accessibility label. > - **Submit CTA**: replaced `TouchableOpacity` with `Button`; enabled only when passwords match and meet `MIN_PASSWORD_LENGTH` (and checkbox selected unless social login). > - **Checkbox/learn more**: uses `Button` for link-style label; shows marketing opt-in copy for social login; keeps "Learn more" link. > - **Styling**: minor layout tweaks (remove some margins, adjust containers); snapshots updated accordingly. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 00cba6249aa01d2688aaa373e18b0b309fe51c45. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../Views/ChoosePassword/__snapshots__/index.test.tsx.snap | 6 ++---- app/components/Views/ChoosePassword/index.js | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/components/Views/ChoosePassword/__snapshots__/index.test.tsx.snap b/app/components/Views/ChoosePassword/__snapshots__/index.test.tsx.snap index 7953dca9c6db..bc353fc7c744 100644 --- a/app/components/Views/ChoosePassword/__snapshots__/index.test.tsx.snap +++ b/app/components/Views/ChoosePassword/__snapshots__/index.test.tsx.snap @@ -173,7 +173,7 @@ exports[`ChoosePassword render matches snapshot 1`] = ` { "alignItems": "center", "backgroundColor": "#ffffff", - "borderColor": "#b7bbc8", + "borderColor": "#4459ff", "borderRadius": 8, "borderWidth": 1, "flexDirection": "row", @@ -194,7 +194,7 @@ exports[`ChoosePassword render matches snapshot 1`] = ` width: '100%', flexDirection: 'column', rowGap: 18, - marginTop: 'auto', marginBottom: Platform.select({ ios: 16, android: 24, @@ -140,7 +139,6 @@ const createStyles = (colors) => justifyContent: 'flex-start', gap: 8, marginTop: 8, - marginBottom: 16, backgroundColor: colors.background.section, borderRadius: 8, padding: 16, @@ -717,7 +715,6 @@ class ChoosePassword extends PureComponent { canSubmit = passwordsMatch && isSelected && password.length >= MIN_PASSWORD_LENGTH; } - const previousScreen = this.props.route.params?.[PREVIOUS_SCREEN]; const colors = this.context.colors || mockTheme.colors; const themeAppearance = this.context.themeAppearance || 'light'; const styles = createStyles(colors); @@ -788,6 +785,7 @@ class ChoosePassword extends PureComponent { {strings('choose_password.password')} Date: Fri, 14 Nov 2025 14:31:59 +0100 Subject: [PATCH 08/17] feat: [Trending] quick actions and restructuring (1/2) (#22700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR focuses on adding the quick actions section within trending and restructure quite a bit of the code for sections to be dynamically created. Here is a summary of the components that get dynamically created based on a centralized configuration: - Quick Action Buttons - 🟢 - Search Sections - 🟢 (needs hook to be added to useExploreSearch) - Actual sections - 🟠 ## **Changelog** CHANGELOG entry: trending quick actions section and dynamic restructuring (1/2) ## **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** image ## **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] > Introduce centralized `sections.config` powering QuickActions, section headers, and explore search; refactor related components and tests to consume it. > > - **Core configuration** > - Add `config/sections.config.tsx` with `SECTIONS_CONFIG` and `SECTIONS_ARRAY` to define titles, navigation, rendering, search, and keys for `tokens`, `perps`, and `predictions`. > - Remove legacy `exploreSearchConfig.tsx` and migrate usages. > - **Explore Search** > - Update `ExploreSearchResults.tsx` to render sections/items/skeletons via `SECTIONS_CONFIG` and iterate `SECTIONS_ARRAY`. > - Refactor `useExploreSearch.ts` to generically process sections from `SECTIONS_ARRAY`; add debounced data fetching and filtering; update tests accordingly. > - **UI components** > - Add `components/QuickActions/QuickActions.tsx` to auto-generate action buttons from `SECTIONS_ARRAY` and wire navigation. > - Refactor `components/SectionHeader/SectionHeader.tsx` to accept `sectionId` and trigger section-specific navigation; update tests. > - Update `TrendingTokensSection`, `PredictionSection`, and `PerpsSection` to use new `SectionHeader`. > - Insert `QuickActions` into `TrendingView` feed. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2292ae1f8ee9e76cf4b8bfa1e79c340a7fdc3bde. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../ExploreSearchResults.tsx | 37 ++---- .../config/exploreSearchConfig.tsx | 86 ------------ .../config/useExploreSearch.test.ts | 6 +- .../config/useExploreSearch.ts | 12 +- .../PerpsSection/PerpsSection.tsx | 15 +-- .../PredictionSection/PredictionSection.tsx | 20 +-- .../TrendingTokensSection.tsx | 10 +- .../Views/TrendingView/TrendingView.tsx | 3 +- .../components/QuickActions/QuickActions.tsx | 37 ++++++ .../SectionHeader/SectionHeader.test.tsx | 41 +++++- .../SectionHeader/SectionHeader.tsx | 51 +++++--- .../TrendingView/config/sections.config.tsx | 122 ++++++++++++++++++ 12 files changed, 254 insertions(+), 186 deletions(-) delete mode 100644 app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/exploreSearchConfig.tsx create mode 100644 app/components/Views/TrendingView/components/QuickActions/QuickActions.tsx create mode 100644 app/components/Views/TrendingView/config/sections.config.tsx diff --git a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/ExploreSearchResults.tsx b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/ExploreSearchResults.tsx index 604c1543e1da..43a678480b64 100644 --- a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/ExploreSearchResults.tsx +++ b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/ExploreSearchResults.tsx @@ -9,9 +9,10 @@ import { } from '@metamask/design-system-react-native'; import { strings } from '../../../../../../../locales/i18n'; import { - SEARCH_SECTION_ARRAY, + SECTIONS_CONFIG, + SECTIONS_ARRAY, type SectionId, -} from './config/exploreSearchConfig'; +} from '../../../config/sections.config'; import { useExploreSearch } from './config/useExploreSearch'; import { StyleSheet } from 'react-native'; @@ -50,13 +51,6 @@ const ExploreSearchResults: React.FC = ({ const navigation = useNavigation(); const { data, isLoading } = useExploreSearch(searchQuery); - // Helper to get section config by id - const getSectionById = useCallback( - (sectionId: SectionId) => - SEARCH_SECTION_ARRAY.find((s) => s.id === sectionId), - [], - ); - const renderSectionHeader = useCallback( (title: string) => ( @@ -72,7 +66,7 @@ const ExploreSearchResults: React.FC = ({ const flatData = useMemo(() => { const result: FlatListItem[] = []; - SEARCH_SECTION_ARRAY.forEach((section) => { + SECTIONS_ARRAY.forEach((section) => { const items = data[section.id]; const sectionIsLoading = isLoading[section.id]; @@ -115,7 +109,7 @@ const ExploreSearchResults: React.FC = ({ return renderSectionHeader(item.data); } - const section = getSectionById(item.sectionId); + const section = SECTIONS_CONFIG[item.sectionId]; if (!section) return null; if (item.type === 'skeleton') { @@ -127,22 +121,17 @@ const ExploreSearchResults: React.FC = ({ const onPressHandler = section.getOnPressHandler?.(navigation as never); return section.renderItem(item.data as never, onPressHandler as never); }, - [navigation, getSectionById, renderSectionHeader], + [navigation, renderSectionHeader], ); - const keyExtractor = useCallback( - (item: FlatListItem, index: number) => { - if (item.type === 'header') return `header-${item.data}`; - if (item.type === 'skeleton') - return `skeleton-${item.sectionId}-${item.index}`; + const keyExtractor = useCallback((item: FlatListItem, index: number) => { + if (item.type === 'header') return `header-${item.data}`; + if (item.type === 'skeleton') + return `skeleton-${item.sectionId}-${item.index}`; - const section = getSectionById(item.sectionId); - return section - ? section.keyExtractor(item.data as never) - : `item-${index}`; - }, - [getSectionById], - ); + const section = SECTIONS_CONFIG[item.sectionId]; + return section ? section.keyExtractor(item.data as never) : `item-${index}`; + }, []); if (flatData.length === 0) { return ( diff --git a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/exploreSearchConfig.tsx b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/exploreSearchConfig.tsx deleted file mode 100644 index ea47dd02bb56..000000000000 --- a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/exploreSearchConfig.tsx +++ /dev/null @@ -1,86 +0,0 @@ -import React from 'react'; -import type { NavigationProp, ParamListBase } from '@react-navigation/native'; -import type { TrendingAsset } from '@metamask/assets-controllers'; -import TrendingTokenRowItem from '../../../../TrendingTokensSection/TrendingTokensList/TrendingTokenRowItem/TrendingTokenRowItem'; -import TrendingTokensSkeleton from '../../../../TrendingTokensSection/TrendingTokenSkeleton/TrendingTokensSkeleton'; -import PerpsMarketRowItem from '../../../../../../UI/Perps/components/PerpsMarketRowItem'; -import PerpsMarketRowSkeleton from '../../../../../../UI/Perps/Views/PerpsMarketListView/components/PerpsMarketRowSkeleton'; -import type { PerpsMarketData } from '../../../../../../UI/Perps/controllers/types'; -import PredictMarket from '../../../../../../UI/Predict/components/PredictMarket'; -import type { PredictMarket as PredictMarketType } from '../../../../../../UI/Predict/types'; -import type { PerpsNavigationParamList } from '../../../../../../UI/Perps/types/navigation'; -import Routes from '../../../../../../../constants/navigation/Routes'; -import PredictMarketSkeleton from '../../../../../../UI/Predict/components/PredictMarketSkeleton'; - -export type SectionId = 'tokens' | 'perps' | 'predictions'; - -export interface SectionData { - data: unknown[]; - isLoading: boolean; -} - -interface SearchSectionConfig { - id: SectionId; - title: string; - renderItem: (item: T, onPress?: (item: T) => void) => JSX.Element; - renderSkeleton: () => JSX.Element; - getSearchableText: (item: T) => string; - keyExtractor: (item: T) => string; - getOnPressHandler?: (navigation: NavigationProp) => (item: T) => void; -} - -// Token section configuration -const tokensConfig: SearchSectionConfig = { - id: 'tokens', - title: 'Tokens', - renderItem: (item) => ( - undefined} /> - ), - renderSkeleton: () => , - getSearchableText: (item) => `${item.symbol} ${item.name}`.toLowerCase(), - keyExtractor: (item) => `token-${item.assetId}`, -}; - -// Perps section configuration -const perpsConfig: SearchSectionConfig< - PerpsMarketData, - PerpsNavigationParamList -> = { - id: 'perps', - title: 'Perps', - renderItem: (item, onPress) => ( - onPress?.(item)} - showBadge={false} - /> - ), - renderSkeleton: () => , - getSearchableText: (item) => - `${item.symbol} ${item.name || ''}`.toLowerCase(), - keyExtractor: (item) => `perp-${item.symbol}`, - getOnPressHandler: - (navigation: NavigationProp) => - (market: PerpsMarketData) => { - navigation.navigate(Routes.PERPS.ROOT, { - screen: Routes.PERPS.MARKET_DETAILS, - params: { market }, - }); - }, -}; - -// Predictions section configuration -const predictionsConfig: SearchSectionConfig = { - id: 'predictions', - title: 'Predictions', - renderItem: (item) => , - renderSkeleton: () => , - getSearchableText: (item) => item.title.toLowerCase(), - keyExtractor: (item) => `prediction-${item.id}`, -}; - -export const SEARCH_SECTION_ARRAY = [ - tokensConfig, - perpsConfig, - predictionsConfig, -]; diff --git a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.test.ts b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.test.ts index 70c6b395584e..baadfcf5e314 100644 --- a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.test.ts +++ b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.test.ts @@ -1,6 +1,6 @@ import { renderHook, waitFor, act } from '@testing-library/react-native'; import { useExploreSearch } from './useExploreSearch'; -import { SEARCH_SECTION_ARRAY } from './exploreSearchConfig'; +import { SECTIONS_ARRAY } from '../../../../config/sections.config'; const mockTrendingTokens = [ { assetId: '1', symbol: 'BTC', name: 'Bitcoin' }, @@ -270,10 +270,10 @@ describe('useExploreSearch', () => { }); }); - it('processes all sections defined in SEARCH_SECTION_ARRAY', () => { + it('processes all sections defined in config', () => { const { result } = renderHook(() => useExploreSearch('')); - SEARCH_SECTION_ARRAY.forEach((section) => { + SECTIONS_ARRAY.forEach((section) => { expect(result.current.data[section.id]).toBeDefined(); expect(result.current.isLoading[section.id]).toBeDefined(); }); diff --git a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.ts b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.ts index cff5bffb71ad..bdb2d5b3fedb 100644 --- a/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.ts +++ b/app/components/Views/TrendingView/ExploreSearchScreen/components/ExploreSearchResults/config/useExploreSearch.ts @@ -1,9 +1,9 @@ import { useState, useEffect, useMemo } from 'react'; import { - SEARCH_SECTION_ARRAY, + SECTIONS_ARRAY, type SectionId, type SectionData, -} from './exploreSearchConfig'; +} from '../../../../config/sections.config'; import { usePerpsMarkets } from '../../../../../../UI/Perps/hooks/usePerpsMarkets'; import { usePredictMarketData } from '../../../../../../UI/Predict/hooks/usePredictMarketData'; import { useTrendingRequest } from '../../../../../../UI/Assets/hooks/useTrendingRequest'; @@ -53,14 +53,14 @@ const useExploreSearchData = ( * GENERIC EXPLORE SEARCH HOOK * * This hook is completely generic and processes data from any sections - * defined in exploreSearchConfig.tsx. It handles: + * defined in sections.config.tsx. It handles: * - Debouncing the search query * - Filtering results based on section configurations * - Returning top 3 items when no query is present * * TO ADD A NEW SECTION: - * 1. Add section configuration to exploreSearchConfig.tsx - * 2. Add hook call to useEploreSearchData above + * 1. Add section configuration to sections.config.tsx + * 2. Add hook call to useExploreSearchData above * * @param query - Search query string * @returns Search results grouped by section @@ -92,7 +92,7 @@ export const useExploreSearch = (query: string): ExploreSearchResult => { const searchTerm = debouncedQuery.toLowerCase(); // Process each section generically - SEARCH_SECTION_ARRAY.forEach((section) => { + SECTIONS_ARRAY.forEach((section) => { const sectionData = allSectionsData[section.id]; isLoading[section.id] = sectionData.isLoading; diff --git a/app/components/Views/TrendingView/PerpsSection/PerpsSection.tsx b/app/components/Views/TrendingView/PerpsSection/PerpsSection.tsx index b3fc0fe25339..ed1ba6d91135 100644 --- a/app/components/Views/TrendingView/PerpsSection/PerpsSection.tsx +++ b/app/components/Views/TrendingView/PerpsSection/PerpsSection.tsx @@ -1,6 +1,5 @@ import React, { useCallback } from 'react'; import { View } from 'react-native'; -import { strings } from '../../../../../locales/i18n'; import SectionHeader from '../components/SectionHeader/SectionHeader'; import SectionCard from '../components/SectionCard/SectionCard'; import PerpsMarketRowSkeleton from '../../../UI/Perps/Views/PerpsMarketListView/components/PerpsMarketRowSkeleton'; @@ -16,15 +15,6 @@ const PerpsSection = () => { const { markets, isLoading } = usePerpsMarkets(); const perpsTokens = markets.slice(0, 3); - const handleViewAll = useCallback(() => { - navigation.navigate(Routes.PERPS.ROOT, { - screen: Routes.PERPS.MARKET_LIST, - params: { - defaultMarketTypeFilter: 'all', - }, - }); - }, [navigation]); - const handleTokenPress = useCallback( (market: PerpsMarketData) => { navigation.navigate(Routes.PERPS.ROOT, { @@ -37,10 +27,7 @@ const PerpsSection = () => { return ( - + {isLoading || perpsTokens.length === 0 ? ( <> diff --git a/app/components/Views/TrendingView/PredictionSection/PredictionSection.tsx b/app/components/Views/TrendingView/PredictionSection/PredictionSection.tsx index 43258cf8a463..b034fefdcf03 100644 --- a/app/components/Views/TrendingView/PredictionSection/PredictionSection.tsx +++ b/app/components/Views/TrendingView/PredictionSection/PredictionSection.tsx @@ -12,7 +12,6 @@ import { Pressable, } from 'react-native'; import { FlashList, FlashListRef } from '@shopify/flash-list'; -import { strings } from '../../../../../locales/i18n'; import { usePredictMarketData } from '../../../UI/Predict/hooks/usePredictMarketData'; import PredictMarket from '../../../UI/Predict/components/PredictMarket'; import { PredictMarket as PredictMarketType } from '../../../UI/Predict/types'; @@ -21,8 +20,6 @@ import PredictMarketSkeleton from '../../../UI/Predict/components/PredictMarketS import { useStyles } from '../../../../component-library/hooks'; import styleSheet from './PredictionSection.styles'; import SectionHeader from '../components/SectionHeader/SectionHeader'; -import { useNavigation } from '@react-navigation/native'; -import Routes from '../../../../constants/navigation/Routes'; const { width: SCREEN_WIDTH } = Dimensions.get('window'); const CARD_WIDTH = SCREEN_WIDTH - 32; // 16px padding on each side @@ -33,7 +30,6 @@ const SNAP_INTERVAL = ACTUAL_CARD_WIDTH + CARD_SPACING; const PredictionSection = () => { const [activeIndex, setActiveIndex] = useState(0); const flashListRef = useRef>(null); - const navigation = useNavigation(); const { styles } = useStyles(styleSheet, { activeIndex, @@ -65,12 +61,6 @@ const PredictionSection = () => { setActiveIndex(index); }, []); - const handleViewAll = useCallback(() => { - navigation.navigate(Routes.PREDICT.ROOT, { - screen: Routes.PREDICT.MARKET_LIST, - }); - }, [navigation]); - const renderCarouselItem = useCallback( ({ item, index }: { item: PredictMarketType; index: number }) => { const isLast = index === marketDataLength - 1; @@ -120,10 +110,7 @@ const PredictionSection = () => { if (isFetching) { return ( - + { return ( - + { const { results: trendingTokensResults, isLoading } = useTrendingRequest({}); const trendingTokens = trendingTokensResults.slice(0, 3); - const handleViewAll = useCallback(() => { - // TODO: Implement view all logic - }, []); - const handleTokenPress = useCallback((token: TrendingAsset) => { // eslint-disable-next-line no-console console.log('🚀 ~ TrendingTokensSection ~ token:', token); @@ -24,10 +19,7 @@ const TrendingTokensSection = () => { return ( - + {isLoading || trendingTokens.length === 0 ? ( diff --git a/app/components/Views/TrendingView/TrendingView.tsx b/app/components/Views/TrendingView/TrendingView.tsx index e00590431880..ee7e25446564 100644 --- a/app/components/Views/TrendingView/TrendingView.tsx +++ b/app/components/Views/TrendingView/TrendingView.tsx @@ -31,13 +31,13 @@ import { PredictModalStack } from '../../UI/Predict/routes'; import PredictionSection from './PredictionSection/PredictionSection'; import PerpsSection from './PerpsSection/PerpsSection'; import { PerpsConnectionProvider } from '../../UI/Perps/providers/PerpsConnectionProvider'; +import QuickActions from './components/QuickActions/QuickActions'; const Stack = createStackNavigator(); const styles = StyleSheet.create({ scrollView: { flex: 1, - marginTop: 10, paddingLeft: 16, paddingRight: 16, }, @@ -128,6 +128,7 @@ const TrendingFeed: React.FC = () => { style={styles.scrollView} showsVerticalScrollIndicator={false} > + diff --git a/app/components/Views/TrendingView/components/QuickActions/QuickActions.tsx b/app/components/Views/TrendingView/components/QuickActions/QuickActions.tsx new file mode 100644 index 000000000000..c44b4403c1cf --- /dev/null +++ b/app/components/Views/TrendingView/components/QuickActions/QuickActions.tsx @@ -0,0 +1,37 @@ +import React from 'react'; +import { ScrollView } from 'react-native'; +import { useNavigation } from '@react-navigation/native'; +import { Box, TextVariant } from '@metamask/design-system-react-native'; +import ButtonFilter from '../../../../../component-library/components-temp/ButtonFilter'; +import { SECTIONS_ARRAY } from '../../config/sections.config'; + +/** + * A dynamic component that automatically generates action buttons based on the + * centralized sections configuration. When a new section is added to SECTIONS_CONFIG, + * a corresponding button will automatically appear here. + */ +const QuickActions: React.FC = () => { + const navigation = useNavigation(); + + return ( + + + + {SECTIONS_ARRAY.map((section) => ( + section.navigationAction(navigation)} + testID={`quick-action-${section.id}`} + textProps={{ variant: TextVariant.BodySm }} + > + {section.title} + + ))} + + + + ); +}; + +export default QuickActions; diff --git a/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.test.tsx b/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.test.tsx index 8c700e283ddd..f8fc26eaf09b 100644 --- a/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.test.tsx +++ b/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.test.tsx @@ -4,6 +4,15 @@ import renderWithProvider from '../../../../../util/test/renderWithProvider'; import { backgroundState } from '../../../../../util/test/initial-root-state'; import SectionHeader from './SectionHeader'; +const mockNavigate = jest.fn(); + +jest.mock('@react-navigation/native', () => ({ + ...jest.requireActual('@react-navigation/native'), + useNavigation: () => ({ + navigate: mockNavigate, + }), +})); + const initialState = { engine: { backgroundState, @@ -11,15 +20,13 @@ const initialState = { }; describe('SectionHeader', () => { - const mockOnViewAll = jest.fn(); - beforeEach(() => { jest.clearAllMocks(); }); - it('renders title and view all text correctly', () => { + it('renders title and view all text for predictions section', () => { const { getByText } = renderWithProvider( - , + , { state: initialState }, ); @@ -27,14 +34,34 @@ describe('SectionHeader', () => { expect(getByText('View all')).toBeOnTheScreen(); }); - it('calls onViewAll when view all button is pressed', () => { + it('renders title and view all text for tokens section', () => { + const { getByText } = renderWithProvider( + , + { state: initialState }, + ); + + expect(getByText('Tokens')).toBeOnTheScreen(); + expect(getByText('View all')).toBeOnTheScreen(); + }); + + it('renders title and view all text for perps section', () => { + const { getByText } = renderWithProvider( + , + { state: initialState }, + ); + + expect(getByText('Perps')).toBeOnTheScreen(); + expect(getByText('View all')).toBeOnTheScreen(); + }); + + it('calls navigation action when view all button is pressed', () => { const { getByText } = renderWithProvider( - , + , { state: initialState }, ); fireEvent.press(getByText('View all')); - expect(mockOnViewAll).toHaveBeenCalledTimes(1); + expect(mockNavigate).toHaveBeenCalledTimes(1); }); }); diff --git a/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.tsx b/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.tsx index 1047dd79d1bb..823dc8da2069 100644 --- a/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.tsx +++ b/app/components/Views/TrendingView/components/SectionHeader/SectionHeader.tsx @@ -11,10 +11,11 @@ import Text, { TextVariant, } from '../../../../../component-library/components/Texts/Text'; import { strings } from '../../../../../../locales/i18n'; +import { SectionId, SECTIONS_CONFIG } from '../../config/sections.config'; +import { useNavigation } from '@react-navigation/native'; interface SectionHeaderProps { - title: string; - onViewAll: () => void; + sectionId: SectionId; } const styles = StyleSheet.create({ @@ -24,22 +25,36 @@ const styles = StyleSheet.create({ }, }); -const SectionHeader: React.FC = ({ title, onViewAll }) => ( - - - {title} - - - - {strings('trending.view_all')} +/** + * Displays a section header with title and "View All" button. + * All configuration is pulled from sections.config.tsx based on the sectionId. + * + * This component is part of the centralized section management system that ensures + * consistency between QuickActions buttons and section "View All" buttons. + */ +const SectionHeader: React.FC = ({ sectionId }) => { + const navigation = useNavigation(); + const sectionConfig = SECTIONS_CONFIG[sectionId]; + + return ( + + + {sectionConfig.title} - - -); + sectionConfig.navigationAction(navigation)} + > + + {strings('trending.view_all')} + + + + ); +}; export default SectionHeader; diff --git a/app/components/Views/TrendingView/config/sections.config.tsx b/app/components/Views/TrendingView/config/sections.config.tsx new file mode 100644 index 000000000000..fd92ebc2689d --- /dev/null +++ b/app/components/Views/TrendingView/config/sections.config.tsx @@ -0,0 +1,122 @@ +import React from 'react'; +import type { NavigationProp, ParamListBase } from '@react-navigation/native'; +import type { TrendingAsset } from '@metamask/assets-controllers'; +import Routes from '../../../../constants/navigation/Routes'; +import { strings } from '../../../../../locales/i18n'; +import TrendingTokenRowItem from '../TrendingTokensSection/TrendingTokensList/TrendingTokenRowItem/TrendingTokenRowItem'; +import TrendingTokensSkeleton from '../TrendingTokensSection/TrendingTokenSkeleton/TrendingTokensSkeleton'; +import PerpsMarketRowItem from '../../../UI/Perps/components/PerpsMarketRowItem'; +import PerpsMarketRowSkeleton from '../../../UI/Perps/Views/PerpsMarketListView/components/PerpsMarketRowSkeleton'; +import type { PerpsMarketData } from '../../../UI/Perps/controllers/types'; +import PredictMarket from '../../../UI/Predict/components/PredictMarket'; +import type { PredictMarket as PredictMarketType } from '../../../UI/Predict/types'; +import type { PerpsNavigationParamList } from '../../../UI/Perps/types/navigation'; +import PredictMarketSkeleton from '../../../UI/Predict/components/PredictMarketSkeleton'; + +export type SectionId = 'predictions' | 'tokens' | 'perps'; + +export interface SectionData { + data: unknown[]; + isLoading: boolean; +} + +/** + * Configuration for each section in the Trending View. + * This includes navigation, display, and search functionality. + */ +export interface SectionConfig { + title: string; + navigationAction: (navigation: NavigationProp) => void; + renderItem: (item: unknown, onPress?: (item: unknown) => void) => JSX.Element; + renderSkeleton: () => JSX.Element; + getSearchableText: (item: unknown) => string; + keyExtractor: (item: unknown) => string; + getOnPressHandler?: ( + navigation: NavigationProp, + ) => (item: unknown) => void; +} + +const tokensConfig: SectionConfig = { + title: strings('trending.tokens'), + navigationAction: (_navigation) => { + // TODO: Implement tokens navigation when ready + // _navigation.navigate(...); + }, + renderItem: (item) => ( + undefined} + /> + ), + renderSkeleton: () => , + getSearchableText: (item) => + `${(item as TrendingAsset).symbol} ${(item as TrendingAsset).name}`.toLowerCase(), + keyExtractor: (item) => `token-${(item as TrendingAsset).assetId}`, +}; + +const perpsConfig: SectionConfig = { + title: strings('trending.perps'), + navigationAction: (navigation) => { + navigation.navigate(Routes.PERPS.ROOT, { + screen: Routes.PERPS.MARKET_LIST, + params: { + defaultMarketTypeFilter: 'all', + }, + }); + }, + renderItem: (item, onPress) => ( + onPress?.(item)} + showBadge={false} + /> + ), + renderSkeleton: () => , + getSearchableText: (item) => + `${(item as PerpsMarketData).symbol} ${(item as PerpsMarketData).name || ''}`.toLowerCase(), + keyExtractor: (item) => `perp-${(item as PerpsMarketData).symbol}`, + getOnPressHandler: (navigation) => (market) => { + (navigation as NavigationProp).navigate( + Routes.PERPS.ROOT, + { + screen: Routes.PERPS.MARKET_DETAILS, + params: { market: market as PerpsMarketData }, + }, + ); + }, +}; + +const predictionsConfig: SectionConfig = { + title: strings('wallet.predict'), + navigationAction: (navigation) => { + navigation.navigate(Routes.PREDICT.ROOT, { + screen: Routes.PREDICT.MARKET_LIST, + }); + }, + renderItem: (item) => , + renderSkeleton: () => , + getSearchableText: (item) => (item as PredictMarketType).title.toLowerCase(), + keyExtractor: (item) => `prediction-${(item as PredictMarketType).id}`, +}; + +/** + * Centralized configuration for all Trending View sections. + * This config is used by QuickActions, SectionHeaders, and Search functionality. + * + * To add a new section: + * 1. Add the section ID to the SectionId type + * 2. Create a config constant above (e.g., newSectionConfig) + * 3. Add it to both SECTIONS_CONFIG and SECTIONS_ARRAY below + * 4. Add data fetching in useExploreSearchData hook + */ +export const SECTIONS_CONFIG: Record = { + tokens: tokensConfig, + perps: perpsConfig, + predictions: predictionsConfig, +}; + +export const SECTIONS_ARRAY: (SectionConfig & { id: SectionId })[] = [ + { id: 'tokens', ...tokensConfig }, + { id: 'perps', ...perpsConfig }, + { id: 'predictions', ...predictionsConfig }, +]; From ca43970bb6a439406715012a42fa8327ab1efde9 Mon Sep 17 00:00:00 2001 From: cmd-ob Date: Fri, 14 Nov 2025 14:28:22 +0000 Subject: [PATCH 09/17] fix(e2e): clean up Android adb reverse (#22693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - Fix issue where subsequent tests are failing due to `Error: Command failed: "/opt/android-sdk/platform-tools/adb" -s emulator-17452 reverse tcp:46721 tcp:46721 adb: error: cannot bind listener: Address a... ` ## **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** - [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 - [ ] I’ve included tests if applicable - [ ] 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] > Cleanly removes stale Android adb reverse bindings for known test ports before running fixtures to prevent port binding conflicts. > > - **E2E Infrastructure**: > - Add `cleanupAllAndroidPortForwarding()` in `e2e/framework/fixtures/FixtureUtils.ts` to remove specific fallback adb reverse bindings on Android (skips BrowserStack, targets current device). > - Invoke cleanup at the start of `withFixtures()` in `e2e/framework/fixtures/FixtureHelper.ts`; import added. > - Limits removals to known fallback ports (`fixture`, `command-queue`, `mock`, `ganache`, `anvil`, `dapp-server` instances) to avoid interfering with Detox. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2b5837e5ab2fdec394c6ecc90d7bda076711b9b5. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- e2e/framework/fixtures/FixtureHelper.ts | 5 +++ e2e/framework/fixtures/FixtureUtils.ts | 55 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/e2e/framework/fixtures/FixtureHelper.ts b/e2e/framework/fixtures/FixtureHelper.ts index cd66a3e93474..11f31851ee83 100644 --- a/e2e/framework/fixtures/FixtureHelper.ts +++ b/e2e/framework/fixtures/FixtureHelper.ts @@ -13,6 +13,7 @@ import { getFixturesServerPort, startResourceWithRetry, startMultiInstanceResourceWithRetry, + cleanupAllAndroidPortForwarding, } from './FixtureUtils'; import Utilities from '../../framework/Utilities'; import TestHelpers from '../../helpers'; @@ -497,6 +498,10 @@ export async function withFixtures( useCommandQueueServer = false, } = options; + // Clean up any stale port forwarding from previous failed tests + // This ensures we start with a clean slate on Android + await cleanupAllAndroidPortForwarding(); + // Prepare android devices for testing to avoid having this in all tests await TestHelpers.reverseServerPort(); diff --git a/e2e/framework/fixtures/FixtureUtils.ts b/e2e/framework/fixtures/FixtureUtils.ts index f6d49fc27af6..2ca28dba5b43 100644 --- a/e2e/framework/fixtures/FixtureUtils.ts +++ b/e2e/framework/fixtures/FixtureUtils.ts @@ -53,6 +53,61 @@ function getFallbackPort(resourceType: ResourceType): number { } } +/** + * Removes specific test port bindings for the current device. + * This is called at the start of tests to clean up any stale bindings from previous failed tests. + * + * IMPORTANT: We only remove known fallback ports to avoid interfering with Detox's + * own port management. Using --remove-all would remove Detox's ports and cause errors. + */ +export async function cleanupAllAndroidPortForwarding(): Promise { + // Only remove port forwarding on Android + if (device.getPlatform() !== 'android') { + return; + } + + // Skip on BrowserStack + if (isBrowserStack()) { + return; + } + + // Get device ID to target specific device (important for CI with multiple devices) + const deviceId = device.id || ''; + const deviceFlag = deviceId ? `-s ${deviceId}` : ''; + + // Clean up only the specific fallback ports we use + // This prevents conflicts with Detox's own port management + const fallbackPorts = [ + FALLBACK_FIXTURE_SERVER_PORT, // 12345 + FALLBACK_COMMAND_QUEUE_SERVER_PORT, // 12346 + FALLBACK_MOCKSERVER_PORT, // 8000 + FALLBACK_GANACHE_PORT, // 8546 + DEFAULT_ANVIL_PORT, // 8545 + FALLBACK_DAPP_SERVER_PORT, // 8085 + FALLBACK_DAPP_SERVER_PORT + 1, // 8086 (dapp-server-1) + FALLBACK_DAPP_SERVER_PORT + 2, // 8087 (dapp-server-2) + ]; + + logger.debug('Cleaning up test port forwards before test...'); + + for (const port of fallbackPorts) { + try { + const command = `adb ${deviceFlag} reverse --remove tcp:${port}`; + await execAsync(command); + logger.debug(`✓ Removed port forwarding for tcp:${port}`); + } catch (error) { + // Silently ignore "not found" errors - the port might not have been forwarded + const errorMessage = + error instanceof Error ? error.message : String(error); + if (!errorMessage.includes('not found')) { + logger.debug(`Note: Could not remove tcp:${port}: ${errorMessage}`); + } + } + } + + logger.debug('✓ Cleaned up test port forwarding'); +} + /** * Sets up adb reverse for Android to map fallback port to actual allocated port. * From 5e8e410f566219c0922dd001994a415de5e6f45d Mon Sep 17 00:00:00 2001 From: ieow <4881057+ieow@users.noreply.github.com> Date: Fri, 14 Nov 2025 22:41:37 +0800 Subject: [PATCH 10/17] fix: onboarding rehydrate tracking (#22686) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** fix tracking for rehydration Currently the screen support isComingFromOauthOnboarding and isSeedlessPasswordOudated add check for isComingFromOauthOnboarding before tracking to avoid miss track for isSeedlessPasswordOudated ## **Changelog** CHANGELOG entry: null ## **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** - [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] > [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) is generating a summary for commit 4476b00f34f33ce7296e3397eb4b953145bda203. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../Views/OAuthRehydration/index.tsx | 89 +++++++++++-------- 1 file changed, 51 insertions(+), 38 deletions(-) diff --git a/app/components/Views/OAuthRehydration/index.tsx b/app/components/Views/OAuthRehydration/index.tsx index f2bdc1b64bb8..04f0b8795ccd 100644 --- a/app/components/Views/OAuthRehydration/index.tsx +++ b/app/components/Views/OAuthRehydration/index.tsx @@ -256,26 +256,32 @@ const OAuthRehydration: React.FC = ({ seedlessError.message === SeedlessOnboardingControllerErrorMessage.IncorrectPassword ) { - track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { - account_type: 'social', - failed_attempts: rehydrationFailedAttempts, - error_type: 'incorrect_password', - }); + if (isComingFromOauthOnboarding) { + track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { + account_type: 'social', + failed_attempts: rehydrationFailedAttempts, + error_type: 'incorrect_password', + }); + } setError(strings('login.invalid_password')); return; } else if ( seedlessError.message === SeedlessOnboardingControllerErrorMessage.TooManyLoginAttempts ) { + // Synchronize rehydrationFailedAttempts with numberOfAttempts from the error data if (seedlessError.data?.numberOfAttempts !== undefined) { setRehydrationFailedAttempts(seedlessError.data.numberOfAttempts); } - track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { - account_type: 'social', - failed_attempts: - seedlessError.data?.numberOfAttempts ?? rehydrationFailedAttempts, - error_type: 'incorrect_password', - }); + if (isComingFromOauthOnboarding) { + track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { + account_type: 'social', + failed_attempts: + seedlessError.data?.numberOfAttempts ?? + rehydrationFailedAttempts, + error_type: 'incorrect_password', + }); + } if (typeof seedlessError.data?.remainingTime === 'number') { tooManyAttemptsError(seedlessError.data?.remainingTime).catch( () => null, @@ -288,11 +294,13 @@ const OAuthRehydration: React.FC = ({ seedlessError.code === SeedlessOnboardingControllerErrorType.PasswordRecentlyUpdated ) { - track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { - account_type: 'social', - failed_attempts: rehydrationFailedAttempts, - error_type: 'unknown_error', - }); + if (isComingFromOauthOnboarding) { + track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { + account_type: 'social', + failed_attempts: rehydrationFailedAttempts, + error_type: 'unknown_error', + }); + } setError(strings('login.seedless_password_outdated')); return; } @@ -320,23 +328,25 @@ const OAuthRehydration: React.FC = ({ ); setError(errMessage); - track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { - account_type: 'social', - failed_attempts: rehydrationFailedAttempts, - error_type: 'unknown_error', - }); - - if (isMetricsEnabled()) { - captureException(seedlessError, { - tags: { - view: 'Login', - context: 'OAuth rehydration failed - user consented to analytics', - }, + if (isComingFromOauthOnboarding) { + track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { + account_type: 'social', + failed_attempts: rehydrationFailedAttempts, + error_type: 'unknown_error', }); - } else { - setErrorToThrow( - new Error(`OAuth rehydration failed: ${seedlessError.message}`), - ); + + if (isMetricsEnabled()) { + captureException(seedlessError, { + tags: { + view: 'Login', + context: 'OAuth rehydration failed - user consented to analytics', + }, + }); + } else { + setErrorToThrow( + new Error(`OAuth rehydration failed: ${seedlessError.message}`), + ); + } } }, [ @@ -384,7 +394,7 @@ const OAuthRehydration: React.FC = ({ toLowerCaseEquals(loginErrorMessage, WRONG_PASSWORD_ERROR_ANDROID) || toLowerCaseEquals(loginErrorMessage, WRONG_PASSWORD_ERROR_ANDROID_2); - if (isWrongPasswordError) { + if (isWrongPasswordError && isComingFromOauthOnboarding) { track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { account_type: 'social', failed_attempts: rehydrationFailedAttempts, @@ -410,11 +420,13 @@ const OAuthRehydration: React.FC = ({ setError(loginErrorMessage); } - track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { - account_type: 'social', - failed_attempts: rehydrationFailedAttempts, - error_type: 'unknown_error', - }); + if (isComingFromOauthOnboarding) { + track(MetaMetricsEvents.REHYDRATION_PASSWORD_FAILED, { + account_type: 'social', + failed_attempts: rehydrationFailedAttempts, + error_type: 'unknown_error', + }); + } setLoading(false); Logger.error(loginErr as Error, 'Failed to rehydrate'); @@ -426,6 +438,7 @@ const OAuthRehydration: React.FC = ({ handlePasswordError, updateBiometryChoice, route.params?.onboardingTraceCtx, + isComingFromOauthOnboarding, ], ); From ded0b2d592563b46d8b37b1fdcef75ac71097c8f Mon Sep 17 00:00:00 2001 From: Andre Pimenta Date: Fri, 14 Nov 2025 15:01:46 +0000 Subject: [PATCH 11/17] fix: Fix Predict Navigation to Cash Out and Single Market (#22711) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Fix Predict Navigation to Cash Out and Single Market ## Overview This PR fixes navigation issues in the Predict feature that prevented users from properly navigating to the cash out flow and single market details screens. This was caused because cash out modal and Single market details modal are not on the ROOT navigator. CHANGELOG entry: null ## Problem Users were unable to navigate correctly to: - **Cash out flow**: When attempting to sell positions - **Single market details**: When viewing market information **Root Cause**: The cash out modal and Single market details modal are not on the ROOT navigator ## Changes ### Files Modified 1. **`app/components/UI/Predict/routes/index.tsx`** - Swapped the order of `UNAVAILABLE` and `GTM_MODAL` screen declarations 2. **`app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.tsx`** - Updated navigation calls to work with new modal stack order - Restores navigation to single market details screen 3. **`app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.tsx`** - Updated navigation calls to work with new modal stack order - Restores navigation to cash out flow ## Testing ### Manual Testing - Cash Out Flow 1. Navigate to Predict feature 2. Open an active position 3. Tap "Cash out" button 4. **Expected**: Cash out screen should display correctly 5. **Previously**: Navigation was blocked or failed ### Manual Testing - Single Market 1. Navigate to Predict feature 2. Tap on any market to view details 3. **Expected**: Single market details screen should display 4. **Previously**: Navigation was blocked or failed ### Verification - ✅ Cash out flow navigates correctly - ✅ Single market details navigates correctly - ✅ No regression in modal functionality (UNAVAILABLE, GTM) - ✅ All other Predict navigation flows work as expected ## Impact - **Medium Risk**: Fixes critical navigation blocking issues - **No API Changes**: Internal navigation structure only - **User Impact**: Restores cash out and market details functionality - **No Breaking Changes**: Only fixes existing broken navigation --- > [!NOTE] > Updates Predict navigation to push `MARKET_DETAILS` and `SELL_PREVIEW` directly (not via `MODALS.ROOT`) and reorders modal stack; adjusts tests accordingly. > > - **Navigation**: > - `PredictMarketSingle`: onPress navigates to `Routes.PREDICT.MARKET_DETAILS` with params (no `MODALS.ROOT`). > - `PredictPositionDetail`: cash out navigates to `Routes.PREDICT.MODALS.SELL_PREVIEW` directly (no `MODALS.ROOT`). > - **Routes**: > - Reorders modal stack screens: `UNAVAILABLE` declared before `GTM_MODAL`. > - **Tests**: > - Update expectations for navigation calls in `PredictMarketSingle.test.tsx`, `PredictPositionDetail.test.tsx`, and `PredictMarketDetails.test.tsx` to match direct navigation and reordered modals. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 75a984d57ccb46a4ecd3c07f762bc7495f97aebf. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../PredictMarketSingle.test.tsx | 13 +++++-------- .../PredictMarketSingle/PredictMarketSingle.tsx | 13 +++++-------- .../PredictPositionDetail.test.tsx | 8 ++++---- .../PredictPositionDetail/PredictPositionDetail.tsx | 13 +++++-------- app/components/UI/Predict/routes/index.tsx | 8 ++++---- .../PredictMarketDetails.test.tsx | 8 ++++---- 6 files changed, 27 insertions(+), 36 deletions(-) diff --git a/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.test.tsx b/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.test.tsx index d1022876b9e1..2a657c6fa82c 100644 --- a/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.test.tsx +++ b/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.test.tsx @@ -311,14 +311,11 @@ describe('PredictMarketSingle', () => { ); fireEvent.press(marketTitle); - expect(mockNavigate).toHaveBeenCalledWith(Routes.PREDICT.MODALS.ROOT, { - screen: Routes.PREDICT.MARKET_DETAILS, - params: { - marketId: mockMarket.id, - entryPoint: PredictEventValues.ENTRY_POINT.PREDICT_FEED, - title: mockMarket.title, - image: mockMarket.image, - }, + expect(mockNavigate).toHaveBeenCalledWith(Routes.PREDICT.MARKET_DETAILS, { + marketId: mockMarket.id, + entryPoint: PredictEventValues.ENTRY_POINT.PREDICT_FEED, + title: mockMarket.title, + image: mockMarket.image, }); }); diff --git a/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.tsx b/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.tsx index 25441c0205ba..49b5ef170d69 100644 --- a/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.tsx +++ b/app/components/UI/Predict/components/PredictMarketSingle/PredictMarketSingle.tsx @@ -191,14 +191,11 @@ const PredictMarketSingle: React.FC = ({ { - navigation.navigate(Routes.PREDICT.MODALS.ROOT, { - screen: Routes.PREDICT.MARKET_DETAILS, - params: { - marketId: market.id, - entryPoint, - title: market.title, - image: getImageUrl(), - }, + navigation.navigate(Routes.PREDICT.MARKET_DETAILS, { + marketId: market.id, + entryPoint, + title: market.title, + image: getImageUrl(), }); }} > diff --git a/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.test.tsx b/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.test.tsx index 98af747dc777..de8616aab966 100644 --- a/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.test.tsx +++ b/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.test.tsx @@ -242,13 +242,13 @@ describe('PredictPositionDetail', () => { fireEvent.press(screen.getByText('Cash out')); - expect(global.__mockNavigate).toHaveBeenCalledWith('PREDICT_MODALS_ROOT', { - screen: 'PREDICT_SELL_PREVIEW', - params: expect.objectContaining({ + expect(global.__mockNavigate).toHaveBeenCalledWith( + 'PREDICT_SELL_PREVIEW', + expect.objectContaining({ position: expect.objectContaining({ id: 'pos-1' }), outcome: expect.objectContaining({ id: 'outcome-1' }), }), - }); + ); }); describe('optimistic updates UI', () => { diff --git a/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.tsx b/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.tsx index ca1f4c2df06b..daf1539a765b 100644 --- a/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.tsx +++ b/app/components/UI/Predict/components/PredictPositionDetail/PredictPositionDetail.tsx @@ -67,14 +67,11 @@ const PredictPosition: React.FC = ({ const _outcome = market?.outcomes.find( (o) => o.id === position.outcomeId, ); - navigate(Routes.PREDICT.MODALS.ROOT, { - screen: Routes.PREDICT.MODALS.SELL_PREVIEW, - params: { - market, - position, - outcome: _outcome, - entryPoint: PredictEventValues.ENTRY_POINT.PREDICT_MARKET_DETAILS, - }, + navigate(Routes.PREDICT.MODALS.SELL_PREVIEW, { + market, + position, + outcome: _outcome, + entryPoint: PredictEventValues.ENTRY_POINT.PREDICT_MARKET_DETAILS, }); }, { attemptedAction: PredictEventValues.ATTEMPTED_ACTION.CASHOUT }, diff --git a/app/components/UI/Predict/routes/index.tsx b/app/components/UI/Predict/routes/index.tsx index 20782707d598..459c7fedc400 100644 --- a/app/components/UI/Predict/routes/index.tsx +++ b/app/components/UI/Predict/routes/index.tsx @@ -28,8 +28,8 @@ const PredictModalStack = () => ( }} > ({ cardStyle: { @@ -39,8 +39,8 @@ const PredictModalStack = () => ( }} /> ({ cardStyle: { diff --git a/app/components/UI/Predict/views/PredictMarketDetails/PredictMarketDetails.test.tsx b/app/components/UI/Predict/views/PredictMarketDetails/PredictMarketDetails.test.tsx index e8613dacf182..2e08d6ce1706 100644 --- a/app/components/UI/Predict/views/PredictMarketDetails/PredictMarketDetails.test.tsx +++ b/app/components/UI/Predict/views/PredictMarketDetails/PredictMarketDetails.test.tsx @@ -1367,15 +1367,15 @@ describe('PredictMarketDetails', () => { const cashOutButton = screen.getByText('predict.cash_out'); fireEvent.press(cashOutButton); - expect(mockNavigate).toHaveBeenCalledWith(Routes.PREDICT.MODALS.ROOT, { - screen: Routes.PREDICT.MODALS.SELL_PREVIEW, - params: { + expect(mockNavigate).toHaveBeenCalledWith( + Routes.PREDICT.MODALS.SELL_PREVIEW, + { position: mockPosition, outcome: expect.any(Object), market: expect.any(Object), entryPoint: 'predict_market_details', }, - }); + ); }); it('handles Yes button press for betting', () => { From 47354bffd015c555d5e26703c1ea20a81a4f9163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20=C5=81ucka?= <5708018+PatrykLucka@users.noreply.github.com> Date: Fri, 14 Nov 2025 16:29:54 +0100 Subject: [PATCH 12/17] refactor: update multichain address sorting logic and tests (#22623) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The current ordering of chains in the Receiving Address and Account Selector screens feels arbitrary, mixing EVM and non-EVM networks without logical prioritization. For users managing multiple chains, especially newcomers, this inconsistency causes cognitive load and confusion when trying to locate primary networks like Ethereum, Bitcoin, or Solana. This PR changes the order of networks to: first set of networks being [Ethereum, Bitcoin, Solana, Tron, Linea] and then the rest. ## **Changelog** CHANGELOG entry: Updates the order of networks on networks list ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-175 ## **Manual testing steps** ```gherkin Feature: Receiving address Scenario: user lists receiving address Given user onboarded When user clicks copy button at the top of the main page Then the networks list have Ethereum, Bitcoin, Solana, Tron, Linea at the beginning of the list ``` ## **Screenshots/Recordings** ### **Before** Screenshot 2025-11-13 at 12 34 03 ### **After** Screenshot 2025-11-13 at 12 49 13 ## **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] > Reorders multichain address sorting to ETH > BTC > SOL > TRX > Linea, normalizes CAIP IDs, and updates selectors, tests, and snapshots to match. > > - **Multichain sorting utils (`MultichainAddressRowsList.utils.ts`)**: > - Implement new priority: `Ethereum (0)`, `Bitcoin (1)`, `Solana (2)`, `Tron (3)`, `Linea (4)`, then featured, others, testnets last. > - Normalize EVM CAIP IDs by converting decimal to hex in `extractHexChainId`. > - Add `BtcScope` and `TrxScope` handling; update sorting docstring. > - **Selectors (`app/selectors/multichainAccounts/accounts.ts`)**: > - Use `sortNetworkAddressItems` to order account-scope items consistently across views. > - **Tests & snapshots**: > - Update unit tests to assert new ordering in utils, AddressSelector, and selectors. > - Refresh AddressSelector snapshot to reflect reordered networks and labels. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit dae32b65fd36508a4ced57a359950caf88563511. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../MultichainAddressRowsList.utils.test.ts | 54 +++++++++++++++++-- .../MultichainAddressRowsList.utils.ts | 31 ++++++++--- .../AddressSelector/AddressSelector.test.tsx | 9 ++-- .../AddressSelector.test.tsx.snap | 18 +++---- .../multichainAccounts/accounts.test.ts | 13 ++--- app/selectors/multichainAccounts/accounts.ts | 28 +++++++++- 6 files changed, 122 insertions(+), 31 deletions(-) diff --git a/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.test.ts b/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.test.ts index c4dc32cbaea3..65cf50e5e83f 100644 --- a/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.test.ts +++ b/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.test.ts @@ -1,6 +1,6 @@ import { InternalAccount } from '@metamask/keyring-internal-api'; import { CHAIN_IDS } from '@metamask/transaction-controller'; -import { SolScope } from '@metamask/keyring-api'; +import { SolScope, BtcScope, TrxScope } from '@metamask/keyring-api'; import { CaipChainId } from '@metamask/utils'; import { sortNetworkAddressItems, @@ -62,7 +62,7 @@ describe('MultichainAddressRowsList Utils', () => { expect(sorted[0].chainId).toBe(`eip155:${CHAIN_IDS.MAINNET}`); }); - it('sorts networks with Solana second after Ethereum', () => { + it('sorts networks with Bitcoin second and Solana third after Ethereum', () => { const items: NetworkAddressItem[] = [ { chainId: 'eip155:0x89', networkName: 'Polygon', address: '0x123' }, { chainId: SolScope.Mainnet, networkName: 'Solana', address: '0x123' }, @@ -71,11 +71,59 @@ describe('MultichainAddressRowsList Utils', () => { networkName: 'Ethereum', address: '0x123', }, + { chainId: BtcScope.Mainnet, networkName: 'Bitcoin', address: '0x123' }, ]; const sorted = sortNetworkAddressItems(items); expect(sorted[0].chainId).toBe(`eip155:${CHAIN_IDS.MAINNET}`); - expect(sorted[1].chainId).toBe(SolScope.Mainnet); + expect(sorted[1].chainId).toBe(BtcScope.Mainnet); + expect(sorted[2].chainId).toBe(SolScope.Mainnet); + }); + + it('sorts networks with Tron fourth after Ethereum, Bitcoin, and Solana', () => { + const items: NetworkAddressItem[] = [ + { chainId: TrxScope.Mainnet, networkName: 'Tron', address: '0x123' }, + { chainId: 'eip155:0x89', networkName: 'Polygon', address: '0x123' }, + { chainId: SolScope.Mainnet, networkName: 'Solana', address: '0x123' }, + { + chainId: `eip155:${CHAIN_IDS.MAINNET}`, + networkName: 'Ethereum', + address: '0x123', + }, + { chainId: BtcScope.Mainnet, networkName: 'Bitcoin', address: '0x123' }, + ]; + + const sorted = sortNetworkAddressItems(items); + expect(sorted[0].chainId).toBe(`eip155:${CHAIN_IDS.MAINNET}`); + expect(sorted[1].chainId).toBe(BtcScope.Mainnet); + expect(sorted[2].chainId).toBe(SolScope.Mainnet); + expect(sorted[3].chainId).toBe(TrxScope.Mainnet); + }); + + it('sorts networks with Linea fifth after Ethereum, Bitcoin, Solana, and Tron', () => { + const items: NetworkAddressItem[] = [ + { + chainId: `eip155:${CHAIN_IDS.LINEA_MAINNET}`, + networkName: 'Linea', + address: '0x123', + }, + { chainId: TrxScope.Mainnet, networkName: 'Tron', address: '0x123' }, + { chainId: 'eip155:0x89', networkName: 'Polygon', address: '0x123' }, + { chainId: SolScope.Mainnet, networkName: 'Solana', address: '0x123' }, + { + chainId: `eip155:${CHAIN_IDS.MAINNET}`, + networkName: 'Ethereum', + address: '0x123', + }, + { chainId: BtcScope.Mainnet, networkName: 'Bitcoin', address: '0x123' }, + ]; + + const sorted = sortNetworkAddressItems(items); + expect(sorted[0].chainId).toBe(`eip155:${CHAIN_IDS.MAINNET}`); + expect(sorted[1].chainId).toBe(BtcScope.Mainnet); + expect(sorted[2].chainId).toBe(SolScope.Mainnet); + expect(sorted[3].chainId).toBe(TrxScope.Mainnet); + expect(sorted[4].chainId).toBe(`eip155:${CHAIN_IDS.LINEA_MAINNET}`); }); it('sorts test networks last', () => { diff --git a/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.ts b/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.ts index 71fbbe6e2c08..e78fb75ecf86 100644 --- a/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.ts +++ b/app/component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils.ts @@ -1,6 +1,6 @@ import { InternalAccount } from '@metamask/keyring-internal-api'; import { CHAIN_IDS } from '@metamask/transaction-controller'; -import { SolScope } from '@metamask/keyring-api'; +import { SolScope, BtcScope, TrxScope } from '@metamask/keyring-api'; import { CaipChainId } from '@metamask/utils'; import { TEST_NETWORK_IDS } from '../../../../constants/network'; import { PopularList } from '../../../../util/networks/customNetworks'; @@ -18,7 +18,12 @@ export interface NetworkAddressItem { */ const extractHexChainId = (chainId: CaipChainId): string => { if (chainId.startsWith('eip155:')) { - return chainId.split(':')[1]; + const chainIdPart = chainId.split(':')[1]; + // Convert decimal to hex format if needed (CAIP format uses decimal) + if (!chainIdPart.startsWith('0x')) { + return `0x${parseInt(chainIdPart, 10).toString(16)}`; + } + return chainIdPart; } return chainId; }; @@ -33,30 +38,40 @@ const getNetworkPriority = (chainId: CaipChainId): number => { // For EVM networks, extract hex chain ID for comparison const hexChainId = extractHexChainId(chainId); + // Hardcoded order for top networks if (hexChainId === CHAIN_IDS.MAINNET) { return 0; } // Ethereum first - if (chainId === SolScope.Mainnet) { + if (chainId === BtcScope.Mainnet) { return 1; - } // Solana second + } // Bitcoin second + if (chainId === SolScope.Mainnet) { + return 2; + } // Solana third + if (chainId === TrxScope.Mainnet) { + return 3; + } // Tron fourth + if (hexChainId === CHAIN_IDS.LINEA_MAINNET) { + return 4; + } // Linea fifth if ( TEST_NETWORK_IDS.includes(hexChainId as (typeof TEST_NETWORK_IDS)[number]) ) { - return 4; + return 7; } // Test networks last // Featured networks (popular networks) const popularChainIds = PopularList.map((network) => network.chainId); if (popularChainIds.includes(hexChainId as `0x${string}`)) { - return 2; + return 5; } - return 3; // Other custom networks + return 6; // Other custom networks }; /** * Sorts network address items according to priority: - * 1. Ethereum first, 2. Solana second, 3. Featured networks, 4. Other custom networks, 5. Test networks last + * 1. Ethereum, 2. Bitcoin, 3. Solana, 4. Tron, 5. Linea, 6. Featured networks, 7. Other custom networks, 8. Test networks last * * @param items - Array of NetworkAddressItem objects to sort * @returns Sorted array of NetworkAddressItem objects diff --git a/app/components/Views/AddressSelector/AddressSelector.test.tsx b/app/components/Views/AddressSelector/AddressSelector.test.tsx index 6aff0617f4fb..9e11d31602f9 100644 --- a/app/components/Views/AddressSelector/AddressSelector.test.tsx +++ b/app/components/Views/AddressSelector/AddressSelector.test.tsx @@ -134,14 +134,15 @@ describe('AccountSelector', () => { 'multichain-address-row-network-name', ).map((node) => node.props.children); + // Networks are sorted: * 1. Ethereum, 2. Bitcoin, 3. Solana, 4. Tron, 5. Linea, 6. Featured networks, 7. Other custom networks, 8. Test networks last expect(networkNames).toEqual([ MAINNET_DISPLAY_NAME, - BNB_DISPLAY_NAME, - POLYGON_DISPLAY_NAME, - OPTIMISM_DISPLAY_NAME, - ARBITRUM_DISPLAY_NAME, LINEA_MAINNET_DISPLAY_NAME, + ARBITRUM_DISPLAY_NAME, BASE_DISPLAY_NAME, + BNB_DISPLAY_NAME, + OPTIMISM_DISPLAY_NAME, + POLYGON_DISPLAY_NAME, ]); expect(networkNames).not.toContain('Solana'); }); diff --git a/app/components/Views/AddressSelector/__snapshots__/AddressSelector.test.tsx.snap b/app/components/Views/AddressSelector/__snapshots__/AddressSelector.test.tsx.snap index e1c145701835..737c9efd91d6 100644 --- a/app/components/Views/AddressSelector/__snapshots__/AddressSelector.test.tsx.snap +++ b/app/components/Views/AddressSelector/__snapshots__/AddressSelector.test.tsx.snap @@ -989,7 +989,7 @@ exports[`AccountSelector renders correctly and matches snapshot 1`] = ` } testID="multichain-address-row-network-name" > - BNB Chain + Solana - 0x4FeC2...fdcB5 + FcdCd3m...GPZgj - Polygon + Linea - OP + Arbitrum - Arbitrum + Base - Linea + BNB Chain - Base + OP - Solana + Polygon - FcdCd3m...GPZgj + 0x4FeC2...fdcB5 { selectInternalAccountListSpreadByScopesByGroupId(mockState)( ENTROPY_GROUP_ID, ); + // Results should be sorted: Ethereum (priority 0), Solana (priority 2), then other networks alphabetically expect(result).toEqual([ { account: mockEvmAccount, @@ -1400,9 +1401,9 @@ describe('accounts selectors', () => { networkName: 'Ethereum', }, { - account: mockEvmAccount, - scope: 'eip155:33875', - networkName: 'Base', + account: mockSolanaAccount, + scope: SOLANA_MAINNET_SCOPE, + networkName: 'Solana Mainnet', }, { account: mockEvmAccount, @@ -1410,9 +1411,9 @@ describe('accounts selectors', () => { networkName: 'Arbitrum One', }, { - account: mockSolanaAccount, - scope: SOLANA_MAINNET_SCOPE, - networkName: 'Solana Mainnet', + account: mockEvmAccount, + scope: 'eip155:33875', + networkName: 'Base', }, ]); }); diff --git a/app/selectors/multichainAccounts/accounts.ts b/app/selectors/multichainAccounts/accounts.ts index 51c03b8daf4c..ab1edd0ab2ad 100644 --- a/app/selectors/multichainAccounts/accounts.ts +++ b/app/selectors/multichainAccounts/accounts.ts @@ -21,6 +21,7 @@ import { } from '../networkController'; import { TEST_NETWORK_IDS } from '../../constants/network'; import type { AccountGroupWithInternalAccounts } from './accounts.type'; +import { sortNetworkAddressItems } from '../../component-library/components-temp/MultichainAccounts/MultichainAddressRowsList/MultichainAddressRowsList.utils'; /** * Extracts the wallet ID from an account group ID. @@ -282,7 +283,7 @@ export const selectInternalAccountListSpreadByScopesByGroupId = return (groupId: AccountGroupId) => { const accounts = internalAccounts(groupId); - return accounts.flatMap((account) => { + const items = accounts.flatMap((account) => { // Determine scopes based on account type const scopes = account.type === EthAccountType.Eoa @@ -299,6 +300,31 @@ export const selectInternalAccountListSpreadByScopesByGroupId = networkConfigurations[scope]?.name || 'Unknown Network', })); }); + + // Sort items using the same sorting logic as MultichainAddressRowsList + const sortedItems = sortNetworkAddressItems( + items.map((item) => ({ + chainId: item.scope, + networkName: item.networkName, + address: item.account.address, + })), + ); + + // Map back to the original format with sorted order + return sortedItems.map((sortedItem) => { + const originalItem = items.find( + (item) => + item.scope === sortedItem.chainId && + item.account.address === sortedItem.address, + ); + // originalItem should always exist since sortedItems is derived from items + if (!originalItem) { + throw new Error( + `Failed to find original item for scope ${sortedItem.chainId} and address ${sortedItem.address}`, + ); + } + return originalItem; + }); }; }, ); From f15d60484abbfc8437bbdb56d58fb59bd6ba39f9 Mon Sep 17 00:00:00 2001 From: abretonc7s <107169956+abretonc7s@users.noreply.github.com> Date: Fri, 14 Nov 2025 23:30:45 +0800 Subject: [PATCH 13/17] fix(perps): update slippage configuration and error handling (#22615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** ### What is the reason for the change? Market orders in the Perps feature were experiencing a 15-25% failure rate during volatile market conditions with the error: `"Order could not immediately match against any resting orders. asset={asset_id}"`. This IocCancel error occurs when HyperLiquid's IOC (Immediate or Cancel) order mechanism cannot find matching orders at the specified limit price. **Root Cause**: Our implementation used a 1% slippage tolerance for all order types. HyperLiquid's platform interface defaults (configurable under "Adjust Max Slippage"): - **8% for closing positions** - **10% for TP/SL orders** - **3% for TWAP orders** (protocol constraint) The insufficient 1% slippage caused orders to fail when prices moved >1% between order submission and execution, especially during: - High volatility periods - Network latency - Thin order books - Flash price movements ### What is the improvement/solution? **1. Conservative Slippage Increase (Measured Rollout)** - Replaced single `DEFAULT_SLIPPAGE_BPS: 100` (1%) with order-type-specific constants: - `DEFAULT_MARKET_SLIPPAGE_BPS: 300` (3%) - conservative starting point - `DEFAULT_TPSL_SLIPPAGE_BPS: 1000` (10%) - matches HyperLiquid platform default - `DEFAULT_LIMIT_SLIPPAGE_BPS: 100` (1%) - unchanged **2. Fixed Critical Stop Loss Slippage Direction Bug** - **Bug**: Stop loss slippage was inverted - limit prices were set in the wrong direction - Long positions: SL limit was 10% ABOVE trigger (should be BELOW) - Short positions: SL limit was 10% BELOW trigger (should be ABOVE) - **Impact**: Stop losses could fail to execute during adverse price movements, exposing traders to larger losses - **Fix**: Corrected slippage direction logic in `buildOrdersArray` function - Long positions now set SL limit 10% BELOW trigger (willing to accept less when selling) - Short positions now set SL limit 10% ABOVE trigger (willing to pay more when buying) - **Testing**: Added 10 comprehensive test cases to validate correct slippage direction **3. Implemented Smart Retry for $10 Minimum Order Errors** - **Issue**: Orders exactly at $10 minimum were rejected due to price feed differences between UI and HyperLiquid's orderbook - UI price feed: $0.003918 (WebSocket live prices) - HyperLiquid validation: $0.003915 (actual orderbook price at validation time) - Result: $10.00 order calculated as $9.995 by API → rejected - **Solution**: Automatic retry mechanism with minimal adjustment - Attempt order with exact calculation ($10.00) - If rejected with "$10 minimum" error, retry with 1.5% buffer ($10.15) - Limited to 1 retry to prevent infinite loops - **Benefits**: - No upfront buffer - users see exactly what they request - Self-correcting - only adds buffer when necessary - Minimal impact - adds only $0.15 for edge case $10 orders **4. Enhanced Error Handling** - Added `IOC_CANCEL` error code detection - Implemented user-friendly error message: "Insufficient liquidity to execute order. Try using a limit order or retry in a moment." - Pattern-based error matching for HyperLiquid's error responses **Expected Impact**: - **Significant reduction** in market order failures (measured rollout with 3% vs previous 1%) - Improved user experience during volatile conditions - Clear actionable error messages when failures do occur - Room to increase to 8% if 3% proves insufficient **Files Modified** (12 total): - `perpsConfig.ts` - Order-type-specific slippage constants - `orderCalculations.ts` - Market order and TP/SL slippage logic + **CRITICAL BUG FIX** (stop loss slippage direction) - `orderCalculations.test.ts` - Added 10 comprehensive tests for stop loss slippage direction - `perpsErrorCodes.ts` - Added IOC_CANCEL error code - `translatePerpsError.ts` - Error message mapping - `HyperLiquidProvider.ts` - Error detection, slippage constant updates, + **$10 minimum retry mechanism** - `en.json` - User-friendly error message - `PerpsClosePositionView.tsx` - Slippage constant reference - `PerpsOrderView.tsx` - Slippage constant reference - `types/index.ts` - Updated comment for new default - `PerpsClosePositionView.test.tsx` - Updated test expectations for new slippage values ## **Changelog** CHANGELOG entry: Improved perpetuals market order reliability by aligning slippage tolerance with HyperLiquid platform standards, reducing order failures by up to 90% during volatile market conditions ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-1965 Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2070 ## **Manual testing steps** ```gherkin Feature: Perpetuals Market Order Execution with Improved Slippage Tolerance Scenario: user places market order during stable market conditions Given user has connected wallet with sufficient USDC balance And user is on the Perps order view for BTC market And market conditions are stable (low volatility) When user enters USD amount of "100" And user selects "Market" order type And user selects "Buy" direction And user confirms the order Then order executes successfully without IOC errors And position is opened with expected size And no error messages are displayed Scenario: user places market order during volatile market conditions Given user has connected wallet with sufficient USDC balance And user is on the Perps order view for BTC market And market is experiencing high volatility (price moving >3% per minute) When user enters USD amount of "100" And user selects "Market" order type And user selects "Sell" direction And user confirms the order Then order executes successfully (vs. previous IOC failures) And position is opened with slippage up to 3% And no error messages are displayed Scenario: user places TP/SL orders with improved slippage Given user has an open long position of 0.01 BTC And user is on the Perps order view for BTC market And current BTC price is $50,000 When user enters Take Profit price of "$55,000" And user enters Stop Loss price of "$45,000" And user confirms the order Then TP order is created with trigger at $55,000 And SL order is created with trigger at $45,000 and 10% slippage tolerance And orders are visible in open orders list Scenario: user receives clear error message for insufficient liquidity Given user has connected wallet with sufficient USDC balance And user is on the Perps order view for an illiquid alt-coin market And order book has extremely thin liquidity When user enters USD amount of "10000" And user selects "Market" order type And user confirms the order Then order fails to execute due to insufficient liquidity And user sees error message: "Insufficient liquidity to execute order. Try using a limit order or retry in a moment." And funds are returned to user's account And no technical error codes are shown to user Scenario: user places limit order with unchanged slippage Given user has connected wallet with sufficient USDC balance And user is on the Perps order view for ETH market And current ETH price is $3,000 When user enters USD amount of "100" And user selects "Limit" order type And user enters limit price of "$2,950" And user confirms the order Then limit order is created successfully And order uses 1% slippage tolerance (unchanged behavior) And order is visible in open orders list Scenario: user closes position during volatile conditions Given user has an open position in BTC market And market is experiencing high volatility When user navigates to position details And user taps "Close Position" And user confirms the closure Then position closes successfully with market order execution And 3% slippage tolerance prevents IOC failures And user's balance is updated with realized P&L Scenario: stop loss executes correctly for long position during rapid price drop Given user has an open long position of 0.1 BTC at entry price $100,000 And user has set a stop loss at $95,000 And BTC price rapidly drops from $96,000 to $85,000 When stop loss is triggered at $95,000 And price continues dropping rapidly Then stop loss order executes successfully (limit price at $85,500, 10% below trigger) And position is closed protecting user from further losses And user's realized loss is limited to approximately $14,500 (vs. potential $15,000+ without fix) Scenario: stop loss executes correctly for short position during rapid price spike Given user has an open short position of 0.1 BTC at entry price $90,000 And user has set a stop loss at $95,000 And BTC price rapidly spikes from $94,000 to $105,000 When stop loss is triggered at $95,000 And price continues spiking rapidly Then stop loss order executes successfully (limit price at $104,500, 10% above trigger) And position is closed protecting user from further losses And user's realized loss is limited to approximately $14,500 (vs. potential unlimited loss without fix) Scenario: minimum order value retry handles price feed differences Given user has connected wallet with $15 USDC balance And user is on the Perps order view for PUMP (low-price asset at $0.003918) And there is a slight difference between UI price feed and HyperLiquid orderbook price When user enters exactly "$10.00" USD amount And user selects "Market" order type with 3x leverage And user confirms the order Then order is attempted with exact calculation (2553 tokens = $10.00) And if HyperLiquid rejects with "$10 minimum" error, order automatically retries And retry adjusts to $10.15 (2557 tokens) to account for price difference And order executes successfully on retry And user is charged the adjusted amount (~$10.15) And no error message is shown to user (transparent retry) ``` ## **Screenshots/Recordings** ### **Before** N/A - This PR addresses backend logic for order execution reliability. The UI remains unchanged; improvements are in order success rates and error messaging. ### **After** N/A - This PR addresses backend logic for order execution reliability. The UI remains unchanged; improvements are in order success rates and error messaging. ## **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] > Refactors slippage (3% market, 10% TP/SL, 1% limit), fixes stop-loss slippage direction, adds a one-time $10-min retry for orders, maps IOC cancel errors, and updates UI/tests accordingly. > > - **Config & Utils**: > - Adjust slippage defaults in `constants/perpsConfig.ts`: `DEFAULT_MARKET_SLIPPAGE_BPS=300`, `DEFAULT_TPSL_SLIPPAGE_BPS=1000`, `DEFAULT_LIMIT_SLIPPAGE_BPS=100`. > - Apply new slippage in `orderCalculations.ts` and fix SL limit-price direction in `buildOrdersArray` (long: below trigger, short: above). > - **Provider (HyperLiquid)**: > - `placeOrder`: add one-time retry for "$10 minimum" rejections by increasing `usdAmount` 1.5%; validate price required; use market slippage for edits/closes. > - Map "could not immediately match" to `PERPS_ERROR_CODES.IOC_CANCEL`; propagate in error handling. > - **UI**: > - Use order-type slippage in `PerpsOrderView.tsx` and `PerpsClosePositionView.tsx` (`maxSlippageBps` per type). > - **Errors/i18n**: > - Add `IOC_CANCEL` code in `perpsErrorCodes.ts`; translate to user-facing messages in `translatePerpsError.ts`/`perpsErrorHandler.ts`; add locale string in `en.json`. > - **Types**: > - Update `slippage` default doc in `controllers/types/index.ts` to 3% market. > - **Tests**: > - Extend `HyperLiquidProvider.test.ts` with $10-min retry and validations. > - Update close position test for 3% market slippage. > - Add comprehensive SL direction tests in `orderCalculations.test.ts`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5d21aea4a76604245683a2a6bc4dd560028033d8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../PerpsClosePositionView.test.tsx | 2 +- .../PerpsClosePositionView.tsx | 5 +- .../Views/PerpsOrderView/PerpsOrderView.tsx | 5 +- .../UI/Perps/constants/perpsConfig.ts | 20 +- .../UI/Perps/controllers/perpsErrorCodes.ts | 2 + .../providers/HyperLiquidProvider.test.ts | 188 +++++++++++++++- .../providers/HyperLiquidProvider.ts | 61 +++++- .../UI/Perps/controllers/types/index.ts | 2 +- .../UI/Perps/utils/orderCalculations.test.ts | 200 ++++++++++++++++++ .../UI/Perps/utils/orderCalculations.ts | 21 +- .../UI/Perps/utils/perpsErrorHandler.ts | 1 + .../UI/Perps/utils/translatePerpsError.ts | 1 + locales/languages/en.json | 1 + 13 files changed, 491 insertions(+), 18 deletions(-) diff --git a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx index 1aa9e56d6ec9..a86795fe0bba 100644 --- a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx +++ b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx @@ -2094,7 +2094,7 @@ describe('PerpsClosePositionView', () => { slippage: { usdAmount: undefined, // undefined for full close to bypass $10 minimum validation priceAtCalculation: 3000, // effectivePrice: currentPrice for market orders - maxSlippageBps: 100, // maxSlippageBps: 1% slippage tolerance (100 basis points) + maxSlippageBps: 300, // maxSlippageBps: 3% slippage tolerance (300 basis points) - conservative default }, }); }); diff --git a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx index abaf193f17fe..b32cba890f36 100644 --- a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx +++ b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx @@ -400,7 +400,10 @@ const PerpsClosePositionView: React.FC = () => { slippage: { usdAmount: isFullClose ? undefined : closingValueString, priceAtCalculation: effectivePrice, - maxSlippageBps: ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS, + maxSlippageBps: + orderType === 'limit' + ? ORDER_SLIPPAGE_CONFIG.DEFAULT_LIMIT_SLIPPAGE_BPS // 1% for limit orders + : ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS, // 3% for market orders }, }); }; diff --git a/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx b/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx index bf9b213892f4..44ecefe5a633 100644 --- a/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx +++ b/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx @@ -757,7 +757,10 @@ const PerpsOrderViewContentBase: React.FC = () => { // USD as source of truth (hybrid approach) usdAmount: orderForm.amount, // USD amount (primary source of truth, provider calculates size from this) priceAtCalculation: assetData.price, // Price snapshot when size was calculated (for slippage validation) - maxSlippageBps: ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS, // Slippage tolerance in basis points (100 = 1%) + maxSlippageBps: + orderForm.type === 'limit' + ? ORDER_SLIPPAGE_CONFIG.DEFAULT_LIMIT_SLIPPAGE_BPS // 1% for limit orders + : ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS, // 3% for market orders // Only add TP/SL/Limit if they are truthy and/or not empty strings ...(orderForm.type === 'limit' && orderForm.limitPrice ? { price: orderForm.limitPrice } diff --git a/app/components/UI/Perps/constants/perpsConfig.ts b/app/components/UI/Perps/constants/perpsConfig.ts index 1decbcce5647..287c3ad2081e 100644 --- a/app/components/UI/Perps/constants/perpsConfig.ts +++ b/app/components/UI/Perps/constants/perpsConfig.ts @@ -73,13 +73,25 @@ export const VALIDATION_THRESHOLDS = { /** * Order slippage configuration - * Controls default slippage tolerance for market orders + * Controls default slippage tolerance for different order types + * Conservative defaults based on HyperLiquid platform interface + * See: docs/perps/hyperliquid/ORDER-MATCHING-ERRORS.md */ export const ORDER_SLIPPAGE_CONFIG = { - // Default slippage for all market orders (basis points) + // Market order slippage (basis points) + // 300 basis points = 3% = 0.03 decimal + // Conservative default for measured rollout, prevents most IOC failures + DEFAULT_MARKET_SLIPPAGE_BPS: 300, + + // TP/SL order slippage (basis points) + // 1000 basis points = 10% = 0.10 decimal + // Aligns with HyperLiquid platform default for triggered orders + DEFAULT_TPSL_SLIPPAGE_BPS: 1000, + + // Limit order slippage (basis points) // 100 basis points = 1% = 0.01 decimal - // Used when price moves between calculation and execution - DEFAULT_SLIPPAGE_BPS: 100, + // Kept conservative as limit orders rest on book (not IOC/immediate execution) + DEFAULT_LIMIT_SLIPPAGE_BPS: 100, } as const; /** diff --git a/app/components/UI/Perps/controllers/perpsErrorCodes.ts b/app/components/UI/Perps/controllers/perpsErrorCodes.ts index 945d82e54708..7cab88c00fba 100644 --- a/app/components/UI/Perps/controllers/perpsErrorCodes.ts +++ b/app/components/UI/Perps/controllers/perpsErrorCodes.ts @@ -16,6 +16,8 @@ export const PERPS_ERROR_CODES = { UNKNOWN_ERROR: 'UNKNOWN_ERROR', // Provider-agnostic order errors ORDER_LEVERAGE_REDUCTION_FAILED: 'ORDER_LEVERAGE_REDUCTION_FAILED', + // HyperLiquid-specific order errors + IOC_CANCEL: 'IOC_CANCEL', // Order could not immediately match (insufficient liquidity) // Connection errors CONNECTION_TIMEOUT: 'CONNECTION_TIMEOUT', } as const; diff --git a/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.test.ts b/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.test.ts index abde8b9a7d71..4bbc5cd7b575 100644 --- a/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.test.ts +++ b/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.test.ts @@ -767,7 +767,193 @@ describe('HyperLiquidProvider', () => { expect(result.success).toBe(true); }); - it('should close a position successfully', async () => { + it('retries USD-based order when rejected for $10 minimum with adjusted amount', async () => { + // Add PUMP to the asset mapping + Object.defineProperty(provider, 'coinToAssetId', { + value: new Map([ + ['BTC', 0], + ['ETH', 1], + ['PUMP', 2], + ]), + writable: true, + }); + Object.defineProperty(provider, 'assetIdToCoin', { + value: new Map([ + [0, 'BTC'], + [1, 'ETH'], + [2, 'PUMP'], + ]), + writable: true, + }); + + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + meta: jest.fn().mockResolvedValue({ + universe: [ + { name: 'BTC', szDecimals: 3, maxLeverage: 50 }, + { name: 'ETH', szDecimals: 4, maxLeverage: 50 }, + { name: 'PUMP', szDecimals: 2, maxLeverage: 20 }, + ], + }), + allMids: jest + .fn() + .mockResolvedValue({ BTC: '50000', ETH: '3000', PUMP: '0.003918' }), + }), + ); + + const orderParams: OrderParams = { + coin: 'PUMP', + isBuy: true, + size: '2553', + orderType: 'market', + usdAmount: '10.00', + currentPrice: 0.003918, + }; + + mockClientService.getExchangeClient = jest.fn().mockReturnValue({ + ...createMockExchangeClient(), + order: jest + .fn() + .mockRejectedValueOnce( + new Error('Order must have minimum value of $10'), + ) + .mockResolvedValueOnce({ + status: 'ok', + response: { data: { statuses: [{ resting: { oid: 456 } }] } }, + }), + }); + + const result = await provider.placeOrder(orderParams); + + expect(result.success).toBe(true); + expect(mockClientService.getExchangeClient().order).toHaveBeenCalledTimes( + 2, + ); + }); + + it('retries size-based order with currentPrice when rejected for $10 minimum', async () => { + // Add PUMP to the asset mapping + Object.defineProperty(provider, 'coinToAssetId', { + value: new Map([ + ['BTC', 0], + ['ETH', 1], + ['PUMP', 2], + ]), + writable: true, + }); + Object.defineProperty(provider, 'assetIdToCoin', { + value: new Map([ + [0, 'BTC'], + [1, 'ETH'], + [2, 'PUMP'], + ]), + writable: true, + }); + + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + meta: jest.fn().mockResolvedValue({ + universe: [ + { name: 'BTC', szDecimals: 3, maxLeverage: 50 }, + { name: 'ETH', szDecimals: 4, maxLeverage: 50 }, + { name: 'PUMP', szDecimals: 2, maxLeverage: 20 }, + ], + }), + allMids: jest + .fn() + .mockResolvedValue({ BTC: '50000', ETH: '3000', PUMP: '0.003918' }), + }), + ); + + const orderParams: OrderParams = { + coin: 'PUMP', + isBuy: true, + size: '2553', + orderType: 'market', + currentPrice: 0.003918, + }; + + mockClientService.getExchangeClient = jest.fn().mockReturnValue({ + ...createMockExchangeClient(), + order: jest + .fn() + .mockRejectedValueOnce( + new Error('Order 0: Order must have minimum value'), + ) + .mockResolvedValueOnce({ + status: 'ok', + response: { data: { statuses: [{ resting: { oid: 789 } }] } }, + }), + }); + + const result = await provider.placeOrder(orderParams); + + expect(result.success).toBe(true); + expect(mockClientService.getExchangeClient().order).toHaveBeenCalledTimes( + 2, + ); + }); + + it('validates price requirement before attempting order placement', async () => { + // Add PUMP to the asset mapping + Object.defineProperty(provider, 'coinToAssetId', { + value: new Map([ + ['BTC', 0], + ['ETH', 1], + ['PUMP', 2], + ]), + writable: true, + }); + Object.defineProperty(provider, 'assetIdToCoin', { + value: new Map([ + [0, 'BTC'], + [1, 'ETH'], + [2, 'PUMP'], + ]), + writable: true, + }); + + mockClientService.getInfoClient = jest.fn().mockReturnValue( + createMockInfoClient({ + meta: jest.fn().mockResolvedValue({ + universe: [ + { name: 'BTC', szDecimals: 3, maxLeverage: 50 }, + { name: 'ETH', szDecimals: 4, maxLeverage: 50 }, + { name: 'PUMP', szDecimals: 2, maxLeverage: 20 }, + ], + }), + allMids: jest + .fn() + .mockResolvedValue({ BTC: '50000', ETH: '3000', PUMP: '0.003918' }), + }), + ); + + const orderParams: OrderParams = { + coin: 'PUMP', + isBuy: true, + size: '2553', + orderType: 'market', + }; + + mockClientService.getExchangeClient = jest.fn().mockReturnValue({ + ...createMockExchangeClient(), + order: jest + .fn() + .mockRejectedValueOnce( + new Error('Order must have minimum value of $10'), + ), + }); + + const result = await provider.placeOrder(orderParams); + + expect(result.success).toBe(false); + expect(result.error).toBe('perps.order.validation.price_required'); + expect( + mockClientService.getExchangeClient().order, + ).not.toHaveBeenCalled(); + }); + + it('closes a position successfully', async () => { const closeParams: ClosePositionParams = { coin: 'BTC', orderType: 'market', diff --git a/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.ts b/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.ts index cac49b0227fb..9dbb3ccc0c9d 100644 --- a/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.ts +++ b/app/components/UI/Perps/controllers/providers/HyperLiquidProvider.ts @@ -261,6 +261,7 @@ export class HyperLiquidProvider implements IPerpsProvider { private readonly ERROR_MAPPINGS = { 'isolated position does not have sufficient margin available to decrease leverage': PERPS_ERROR_CODES.ORDER_LEVERAGE_REDUCTION_FAILED, + 'could not immediately match': PERPS_ERROR_CODES.IOC_CANCEL, }; // Track whether clients have been initialized (lazy initialization) @@ -1871,8 +1872,11 @@ export class HyperLiquidProvider implements IPerpsProvider { * * Refactored to use helper methods for better maintainability and reduced complexity. * Each helper method is focused on a single responsibility. + * + * @param params - Order parameters + * @param retryCount - Internal retry counter to prevent infinite loops (default: 0) */ - async placeOrder(params: OrderParams): Promise { + async placeOrder(params: OrderParams, retryCount = 0): Promise { try { DevLogger.log('Placing order via HyperLiquid SDK:', params); @@ -2027,6 +2031,56 @@ export class HyperLiquidProvider implements IPerpsProvider { assetId, }); } catch (error) { + // Retry mechanism for $10 minimum order errors + // This handles the case where UI price feed slightly differs from HyperLiquid's orderbook price + const errorMessage = + error instanceof Error ? error.message : String(error); + const isMinimumOrderError = + errorMessage.includes('Order must have minimum value of $10') || + errorMessage.includes('Order 0: Order must have minimum value'); + + if (isMinimumOrderError && retryCount === 0) { + let adjustedUsdAmount: string; + let originalValue: string | undefined; + + if (params.usdAmount) { + // USD-based order: adjust the USD amount directly + originalValue = params.usdAmount; + adjustedUsdAmount = (parseFloat(params.usdAmount) * 1.015).toFixed(2); + } else if (params.currentPrice) { + // Size-based order: calculate USD from size and adjust + const sizeValue = parseFloat(params.size); + const estimatedUsd = sizeValue * params.currentPrice; + originalValue = `${estimatedUsd.toFixed(2)} (calculated from size ${params.size})`; + adjustedUsdAmount = (estimatedUsd * 1.015).toFixed(2); + } else { + // No price information available - cannot retry + return this.handleOrderError({ + error, + coin: params.coin, + orderType: params.orderType, + isBuy: params.isBuy, + }); + } + + Logger.log( + 'Retrying order with adjusted size due to minimum value error', + { + originalValue, + adjustedUsdAmount, + retryCount, + }, + ); + + return this.placeOrder( + { + ...params, + usdAmount: adjustedUsdAmount, + }, + 1, // Retry count = 1, prevents further retries + ); + } + return this.handleOrderError({ error, coin: params.coin, @@ -2107,7 +2161,7 @@ export class HyperLiquidProvider implements IPerpsProvider { const positionSize = parseFloat(params.newOrder.size); const slippage = params.newOrder.slippage ?? - ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS / 10000; + ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS / 10000; orderPrice = params.newOrder.isBuy ? currentPrice * (1 + slippage) : currentPrice * (1 - slippage); @@ -2435,7 +2489,8 @@ export class HyperLiquidProvider implements IPerpsProvider { } // Calculate order price with slippage - const slippage = ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS / 10000; + const slippage = + ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS / 10000; const orderPrice = isBuy ? currentPrice * (1 + slippage) : currentPrice * (1 - slippage); diff --git a/app/components/UI/Perps/controllers/types/index.ts b/app/components/UI/Perps/controllers/types/index.ts index 468db3274cb5..b8a0c015cec2 100644 --- a/app/components/UI/Perps/controllers/types/index.ts +++ b/app/components/UI/Perps/controllers/types/index.ts @@ -108,7 +108,7 @@ export type OrderParams = { takeProfitPrice?: string; // Take profit price stopLossPrice?: string; // Stop loss price clientOrderId?: string; // Optional client-provided order ID - slippage?: number; // Slippage tolerance for market orders (default: ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS / 10000 = 1%) + slippage?: number; // Slippage tolerance for market orders (default: ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS / 10000 = 3%) grouping?: 'na' | 'normalTpsl' | 'positionTpsl'; // Override grouping (defaults: 'na' without TP/SL, 'normalTpsl' with TP/SL) currentPrice?: number; // Current market price (avoids extra API call if provided) leverage?: number; // Leverage to apply for the order (e.g., 10 for 10x leverage) diff --git a/app/components/UI/Perps/utils/orderCalculations.test.ts b/app/components/UI/Perps/utils/orderCalculations.test.ts index f73a975c1e98..7c57a3327b77 100644 --- a/app/components/UI/Perps/utils/orderCalculations.test.ts +++ b/app/components/UI/Perps/utils/orderCalculations.test.ts @@ -2,7 +2,9 @@ import { calculatePositionSize, calculateMarginRequired, getMaxAllowedAmount, + buildOrdersArray, } from './orderCalculations'; +import { ORDER_SLIPPAGE_CONFIG } from '../constants/perpsConfig'; describe('orderCalculations', () => { describe('calculatePositionSize', () => { @@ -354,4 +356,202 @@ describe('orderCalculations', () => { expect(result).toBeLessThanOrEqual(50); // 10 * 5 leverage }); }); + + describe('buildOrdersArray', () => { + describe('Stop Loss Slippage Direction', () => { + const baseParams = { + assetId: 0, + formattedPrice: '50000', + formattedSize: '1.0', + reduceOnly: false, + orderType: 'market' as const, + szDecimals: 5, + }; + + it('should apply slippage BELOW trigger price for long position stop loss (sell to exit)', () => { + // Long position: isBuy=true (opening long) + // Stop loss: selling to exit at $95,000 trigger + // Expected: limit price 10% BELOW trigger = $85,500 + const result = buildOrdersArray({ + ...baseParams, + isBuy: true, // Long position (buying to open) + stopLossPrice: '95000', + }); + + expect(result.orders).toHaveLength(2); // Main order + SL order + + const slOrder = result.orders[1]; + expect(slOrder.b).toBe(false); // Selling to close long + expect(slOrder.t).toEqual({ + trigger: { + isMarket: true, + triggerPx: '95000', + tpsl: 'sl', + }, + }); + + // Verify limit price is 10% BELOW trigger (85500) + const expectedLimitPrice = + 95000 * (1 - ORDER_SLIPPAGE_CONFIG.DEFAULT_TPSL_SLIPPAGE_BPS / 10000); + expect(parseFloat(String(slOrder.p))).toBe(expectedLimitPrice); + expect(parseFloat(String(slOrder.p))).toBe(85500); // 95000 * 0.90 + }); + + it('should apply slippage ABOVE trigger price for short position stop loss (buy to exit)', () => { + // Short position: isBuy=false (opening short) + // Stop loss: buying to exit at $105,000 trigger + // Expected: limit price 10% ABOVE trigger = $115,500 + const result = buildOrdersArray({ + ...baseParams, + isBuy: false, // Short position (selling to open) + stopLossPrice: '105000', + }); + + expect(result.orders).toHaveLength(2); // Main order + SL order + + const slOrder = result.orders[1]; + expect(slOrder.b).toBe(true); // Buying to close short + expect(slOrder.t).toEqual({ + trigger: { + isMarket: true, + triggerPx: '105000', + tpsl: 'sl', + }, + }); + + // Verify limit price is 10% ABOVE trigger (115500) + const limitPrice = parseFloat(String(slOrder.p)); + expect(limitPrice).toBeCloseTo(115500, 0); // 105000 * 1.10 (allow rounding) + expect(limitPrice).toBeGreaterThan(105000); // Most importantly: ABOVE trigger + }); + + it('should use 10% slippage for stop loss orders', () => { + // Verify that the 10% slippage (1000 bps) is applied correctly + const result = buildOrdersArray({ + ...baseParams, + isBuy: true, + stopLossPrice: '100000', + }); + + const slOrder = result.orders[1]; + const slippageValue = + ORDER_SLIPPAGE_CONFIG.DEFAULT_TPSL_SLIPPAGE_BPS / 10000; + + expect(slippageValue).toBe(0.1); // 10% + expect(parseFloat(String(slOrder.p))).toBe(100000 * 0.9); // 90000 + }); + + it('should protect against adverse price movements in volatile conditions', () => { + // Real-world scenario: BTC long with SL at $95K during rapid price drop + // Slippage protection ensures order can execute even if price drops quickly + const result = buildOrdersArray({ + ...baseParams, + isBuy: true, + stopLossPrice: '95000', + }); + + const slOrder = result.orders[1]; + const limitPrice = parseFloat(String(slOrder.p)); + + // SL triggers at $95K, limit allows execution down to $85.5K + expect(limitPrice).toBe(85500); + + // This protects against price dropping from $95K to $85.5K + const protectionRange = 95000 - limitPrice; + expect(protectionRange).toBe(9500); // $9,500 protection range + }); + + it('should handle fractional trigger prices correctly', () => { + // Test with altcoin prices (e.g., $1.50 trigger) + const result = buildOrdersArray({ + ...baseParams, + isBuy: false, // Short position + stopLossPrice: '1.50', + }); + + const slOrder = result.orders[1]; + const limitPrice = parseFloat(String(slOrder.p)); + + // For short: limit should be 10% ABOVE trigger + // Most importantly: verify direction is correct (ABOVE trigger) + expect(limitPrice).toBeGreaterThan(1.5); + // Verify it's approximately 10% higher (allowing for price formatting) + expect(limitPrice).toBeGreaterThanOrEqual(1.6); + expect(limitPrice).toBeLessThanOrEqual(1.75); + }); + }); + + describe('Stop Loss Order Structure', () => { + const baseParams = { + assetId: 0, + formattedPrice: '50000', + formattedSize: '1.0', + reduceOnly: false, + orderType: 'market' as const, + szDecimals: 5, + isBuy: true, + }; + + it('should create SL order with reduce-only flag', () => { + const result = buildOrdersArray({ + ...baseParams, + stopLossPrice: '95000', + }); + + const slOrder = result.orders[1]; + expect(slOrder.r).toBe(true); // Reduce-only + }); + + it('should create SL order with market execution on trigger', () => { + const result = buildOrdersArray({ + ...baseParams, + stopLossPrice: '95000', + }); + + const slOrder = result.orders[1]; + expect('trigger' in slOrder.t && slOrder.t.trigger?.isMarket).toBe( + true, + ); + expect('trigger' in slOrder.t && slOrder.t.trigger?.tpsl).toBe('sl'); + }); + + it('should set SL order direction opposite to main order', () => { + // Long position (isBuy=true) → SL sells (b=false) + const longResult = buildOrdersArray({ + ...baseParams, + isBuy: true, + stopLossPrice: '95000', + }); + expect(longResult.orders[1].b).toBe(false); + + // Short position (isBuy=false) → SL buys (b=true) + const shortResult = buildOrdersArray({ + ...baseParams, + isBuy: false, + stopLossPrice: '105000', + }); + expect(shortResult.orders[1].b).toBe(true); + }); + + it('should not create SL order when stopLossPrice is undefined', () => { + const result = buildOrdersArray({ + ...baseParams, + stopLossPrice: undefined, + }); + + expect(result.orders).toHaveLength(1); // Only main order + }); + + it('should create both TP and SL orders with correct grouping', () => { + const result = buildOrdersArray({ + ...baseParams, + takeProfitPrice: '55000', + stopLossPrice: '45000', + }); + + expect(result.orders).toHaveLength(3); // Main + TP + SL + expect(result.grouping).toBe('normalTpsl'); + }); + }); + }); }); diff --git a/app/components/UI/Perps/utils/orderCalculations.ts b/app/components/UI/Perps/utils/orderCalculations.ts index 191aef29a174..8a9640e5c11a 100644 --- a/app/components/UI/Perps/utils/orderCalculations.ts +++ b/app/components/UI/Perps/utils/orderCalculations.ts @@ -201,7 +201,7 @@ export function calculateFinalPositionSize( ((currentPrice - priceAtCalculation) / priceAtCalculation) * 10000, ); const maxSlippageBpsValue = - maxSlippageBps ?? ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS; + maxSlippageBps ?? ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS; if (priceDeltaBps > maxSlippageBpsValue) { throw new Error( @@ -301,9 +301,9 @@ export function calculateOrderPriceAndSize( let formattedSize: string; if (orderType === 'market') { - // Market orders: add slippage + // Market orders: add slippage (3% conservative default) const slippageValue = - slippage ?? ORDER_SLIPPAGE_CONFIG.DEFAULT_SLIPPAGE_BPS / 10000; + slippage ?? ORDER_SLIPPAGE_CONFIG.DEFAULT_MARKET_SLIPPAGE_BPS / 10000; orderPrice = isBuy ? currentPrice * (1 + slippageValue) : currentPrice * (1 - slippageValue); @@ -312,7 +312,7 @@ export function calculateOrderPriceAndSize( szDecimals, }); } else { - // Limit orders: use provided price + // Limit orders: use provided price (no slippage applied) if (!limitPrice) { throw new Error( strings('perps.errors.orderValidation.limitPriceRequired'), @@ -400,11 +400,20 @@ export function buildOrdersArray( // 3. Stop Loss order if (stopLossPrice) { + // Apply 10% slippage to SL limit price (executes as market order when triggered) + // HyperLiquid recommended: 10% for TP/SL orders + const stopLossPriceNum = parseFloat(stopLossPrice); + const slippageValue = + ORDER_SLIPPAGE_CONFIG.DEFAULT_TPSL_SLIPPAGE_BPS / 10000; + const limitPriceWithSlippage = !isBuy + ? stopLossPriceNum * (1 + slippageValue) // Buying to close short: willing to pay MORE (slippage protection) + : stopLossPriceNum * (1 - slippageValue); // Selling to close long: willing to accept LESS (slippage protection) + const slOrder: SDKOrderParams = { a: assetId, b: !isBuy, p: formatHyperLiquidPrice({ - price: parseFloat(stopLossPrice), + price: limitPriceWithSlippage, szDecimals, }), s: formattedSize, @@ -413,7 +422,7 @@ export function buildOrdersArray( trigger: { isMarket: true, triggerPx: formatHyperLiquidPrice({ - price: parseFloat(stopLossPrice), + price: stopLossPriceNum, szDecimals, }), tpsl: 'sl', diff --git a/app/components/UI/Perps/utils/perpsErrorHandler.ts b/app/components/UI/Perps/utils/perpsErrorHandler.ts index cda6149bfd3e..ac937ceca8d2 100644 --- a/app/components/UI/Perps/utils/perpsErrorHandler.ts +++ b/app/components/UI/Perps/utils/perpsErrorHandler.ts @@ -38,6 +38,7 @@ const ERROR_CODE_TO_I18N_KEY: Record = { [PERPS_ERROR_CODES.UNKNOWN_ERROR]: 'perps.errors.unknownError', [PERPS_ERROR_CODES.ORDER_LEVERAGE_REDUCTION_FAILED]: 'perps.errors.orderLeverageReductionFailed', + [PERPS_ERROR_CODES.IOC_CANCEL]: 'perps.errors.iocCancel', [PERPS_ERROR_CODES.CONNECTION_TIMEOUT]: 'perps.errors.connectionTimeout', }; diff --git a/app/components/UI/Perps/utils/translatePerpsError.ts b/app/components/UI/Perps/utils/translatePerpsError.ts index 04e750e5d6b0..18f8efd3c595 100644 --- a/app/components/UI/Perps/utils/translatePerpsError.ts +++ b/app/components/UI/Perps/utils/translatePerpsError.ts @@ -24,6 +24,7 @@ const ERROR_CODE_TO_I18N_KEY: Record = { [PERPS_ERROR_CODES.UNKNOWN_ERROR]: 'perps.errors.unknownError', [PERPS_ERROR_CODES.ORDER_LEVERAGE_REDUCTION_FAILED]: 'perps.errors.orderLeverageReductionFailed', + [PERPS_ERROR_CODES.IOC_CANCEL]: 'perps.errors.insufficientLiquidity', [PERPS_ERROR_CODES.CONNECTION_TIMEOUT]: 'perps.errors.connectionTimeout', }; diff --git a/locales/languages/en.json b/locales/languages/en.json index ad7728d08f59..ce365af8e981 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -1331,6 +1331,7 @@ "assetMappingFailed": "Failed to build asset mapping", "depositFailed": "Deposit failed", "orderLeverageReductionFailed": "You cannot reduce your leverage", + "insufficientLiquidity": "Insufficient liquidity to execute order. Try using a limit order or retry in a moment.", "connectionTimeout": "Connection timed out. Please check your network and try again.", "connectionFailed": { "title": "Couldn't connect to perps", From 52e2d2187e3b172ffdc003afbffae1e8f712fc05 Mon Sep 17 00:00:00 2001 From: AxelGes <34173844+AxelGes@users.noreply.github.com> Date: Fri, 14 Nov 2025 12:32:29 -0300 Subject: [PATCH 14/17] feat(ramps): use goToRamps for Aggregator and Deposit entry points (#22196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** We need to update all instances where we were directly using `createBuyNavigationDetails`, `createSellNavigationDetails` and `createDepositNavigationDetails` to now use the useRampsNavigation hook. We also need to create a `withRampsNavigation` HOC for pure components. ## **Changelog** CHANGELOG entry: Use new Ramps routing ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TRAM-2813?atlOrigin=eyJpIjoiMzY1YTIxZTQ0OTYyNGM5YmE2YTBlNDUwMzMxZDExNzgiLCJwIjoiaiJ9 ## **Manual testing steps** ## **Screenshots/Recordings** ### **Unified Buy flag OFF** https://github.com/user-attachments/assets/09cdb4db-9301-47f9-9aa5-6dc10a8cad40 ### **Unified Buy flag ON** https://github.com/user-attachments/assets/fecec070-0fcd-4133-b3c3-0dabb8364ca7 ## **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] > Replaces direct ramp route navigation with goToRamps across UI, adds smart routing in useRampNavigation (incl. override), introduces withRampNavigation HOC, and updates tests accordingly. > > - **Ramps Navigation**: > - Replace all `createBuy/Sell/DepositNavigationDetails` usages with `goToRamps` from `useRampNavigation` (e.g., `AssetOverview`, `FundActionMenu`, `BalanceEmptyState`, Card Add Funds, Orders, Modals, Swaps, Alerts, Receive/Send/Approve views). > - Add smart routing in `useRampNavigation` using `rampRoutingDecision` with support for `overrideUnifiedBuyFlag`. > - Consolidate Aggregator routes via `createRampNavigationDetails`. > - **HOC**: > - Introduce `withRampNavigation` to inject `goToRamps`, `RampMode`, and `AggregatorRampType` into class/pure components. > - **Behavioral tweaks**: > - Pass ramp intent (`assetId`) where applicable. > - Unified Buy/Deposit entry points updated (e.g., Settings/Unsupported region/state modals, Orders list). > - **Tests**: > - Update unit tests to mock `useRampNavigation`/`goToRamps` and new routing behavior throughout. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f3fc2950d3087ca9f402cac03b4815ac5e2d18a0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --------- Co-authored-by: Pedro Pablo Aste Kompen --- .../UI/AssetOverview/AssetOverview.tsx | 16 +- .../BalanceEmptyState.test.tsx | 33 +- .../BalanceEmptyState/BalanceEmptyState.tsx | 11 +- .../UI/Card/Views/CardHome/CardHome.test.tsx | 12 + .../AddFundsBottomSheet.test.tsx | 15 +- .../AddFundsBottomSheet.tsx | 12 +- .../UI/FundActionMenu/FundActionMenu.test.tsx | 47 ++- .../UI/FundActionMenu/FundActionMenu.tsx | 54 +-- .../Modals/Settings/SettingsModal.test.tsx | 14 +- .../Views/Modals/Settings/SettingsModal.tsx | 10 +- .../Views/OrderDetails/OrderDetails.test.tsx | 15 +- .../Views/OrderDetails/OrderDetails.tsx | 19 +- .../Views/OrdersList/OrdersList.test.tsx | 30 +- .../Views/OrdersList/OrdersList.tsx | 7 +- .../UI/Ramp/Aggregator/routes/utils.ts | 5 +- .../ConfigurationModal.test.tsx | 15 +- .../ConfigurationModal/ConfigurationModal.tsx | 15 +- .../UnsupportedRegionModal.test.tsx | 13 +- .../UnsupportedRegionModal.tsx | 15 +- .../UnsupportedStateModal.test.tsx | 13 +- .../UnsupportedStateModal.tsx | 15 +- .../UI/Ramp/hooks/useRampNavigation.test.ts | 357 ++++++++++++++---- .../UI/Ramp/hooks/useRampNavigation.ts | 41 +- .../UI/Ramp/hooks/withRampNavigation.tsx | 28 ++ app/components/UI/ReceiveRequest/index.js | 24 +- app/components/UI/Swaps/QuotesView.js | 11 +- .../useInsufficientBalanceAlert.test.ts | 10 + .../alerts/useInsufficientBalanceAlert.ts | 16 +- .../legacy/SendFlow/SendTo/index.js | 21 +- .../ApproveTransactionReview/index.js | 27 +- 30 files changed, 666 insertions(+), 255 deletions(-) create mode 100644 app/components/UI/Ramp/hooks/withRampNavigation.tsx diff --git a/app/components/UI/AssetOverview/AssetOverview.tsx b/app/components/UI/AssetOverview/AssetOverview.tsx index 960fd77815bb..77a9aa270e36 100644 --- a/app/components/UI/AssetOverview/AssetOverview.tsx +++ b/app/components/UI/AssetOverview/AssetOverview.tsx @@ -62,7 +62,8 @@ import { } from '../../../util/analytics/actionButtonTracking'; import { selectSelectedAccountGroup } from '../../../selectors/multichainAccounts/accountTreeController'; import { selectSelectedInternalAccountByScope } from '../../../selectors/multichainAccounts/accounts'; -import { createBuyNavigationDetails } from '../Ramp/Aggregator/routes/utils'; +import { useRampNavigation, RampMode } from '../Ramp/hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../Ramp/Aggregator/types'; import { TokenI } from '../Tokens/types'; import AssetDetailsActions from '../../../components/Views/AssetDetails/AssetDetailsActions'; import { @@ -168,6 +169,7 @@ const AssetOverview: React.FC = ({ ///: END:ONLY_INCLUDE_IF const currentAddress = asset.address as Hex; + const { goToRamps } = useRampNavigation(); const { data: prices = [], isLoading } = useTokenHistoricalPrices({ asset, @@ -337,11 +339,13 @@ const AssetOverview: React.FC = ({ assetId = undefined; } - navigation.navigate( - ...createBuyNavigationDetails({ - assetId, - }), - ); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { + rampType: AggregatorRampType.BUY, + intent: assetId ? { assetId } : undefined, + }, + }); trackEvent( createEventBuilder(MetaMetricsEvents.BUY_BUTTON_CLICKED) diff --git a/app/components/UI/BalanceEmptyState/BalanceEmptyState.test.tsx b/app/components/UI/BalanceEmptyState/BalanceEmptyState.test.tsx index 8c24b173b3d9..42a00070c23f 100644 --- a/app/components/UI/BalanceEmptyState/BalanceEmptyState.test.tsx +++ b/app/components/UI/BalanceEmptyState/BalanceEmptyState.test.tsx @@ -5,26 +5,13 @@ import { backgroundState } from '../../../util/test/initial-root-state'; import BalanceEmptyState from './BalanceEmptyState'; import { BalanceEmptyStateProps } from './BalanceEmptyState.types'; -// Mock navigation (component requires it) -const mockNavigate = jest.fn(); -jest.mock('@react-navigation/native', () => ({ - ...jest.requireActual('@react-navigation/native'), - useNavigation: () => ({ - navigate: mockNavigate, - }), +// Mock useRampNavigation hook +const mockGoToRamps = jest.fn(); +jest.mock('../Ramp/hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, })); -// Mock buy navigation details -const mockBuyNavigationDetails = ['RampBuy', { screen: 'GetStarted' }]; -jest.mock('../Ramp/Aggregator/routes/utils', () => ({ - createBuyNavigationDetails: jest.fn(() => mockBuyNavigationDetails), -})); - -// Get the mock function to verify calls -const { createBuyNavigationDetails } = jest.requireMock( - '../Ramp/Aggregator/routes/utils', -); - describe('BalanceEmptyState', () => { beforeEach(() => { jest.clearAllMocks(); @@ -60,15 +47,11 @@ describe('BalanceEmptyState', () => { expect(actionButton).toBeDefined(); - // Press the button fireEvent.press(actionButton); - // Verify that buy navigation details are created - expect(createBuyNavigationDetails).toHaveBeenCalled(); - - // Verify that navigation was triggered with buy flow parameters - expect(mockNavigate).toHaveBeenCalledWith('RampBuy', { - screen: 'GetStarted', + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'AGGREGATOR', + params: { rampType: expect.anything() }, }); }); }); diff --git a/app/components/UI/BalanceEmptyState/BalanceEmptyState.tsx b/app/components/UI/BalanceEmptyState/BalanceEmptyState.tsx index bcd7e190c2bc..052963f2b353 100644 --- a/app/components/UI/BalanceEmptyState/BalanceEmptyState.tsx +++ b/app/components/UI/BalanceEmptyState/BalanceEmptyState.tsx @@ -1,6 +1,5 @@ import React from 'react'; import { Image } from 'react-native'; -import { useNavigation } from '@react-navigation/native'; import { useSelector } from 'react-redux'; import { Box, @@ -21,7 +20,8 @@ import { MetaMetricsEvents, useMetrics } from '../../hooks/useMetrics'; import { getDecimalChainId } from '../../../util/networks'; import { selectChainId } from '../../../selectors/networkController'; import { trace, TraceName } from '../../../util/trace'; -import { createBuyNavigationDetails } from '../Ramp/Aggregator/routes/utils'; +import { useRampNavigation, RampMode } from '../Ramp/hooks/useRampNavigation'; +import { RampType } from '../Ramp/Aggregator/types'; import { BalanceEmptyStateProps } from './BalanceEmptyState.types'; import bankTransferImage from '../../../images/bank-transfer.png'; import { getDetectedGeolocation } from '../../../reducers/fiatOrders'; @@ -36,12 +36,15 @@ const BalanceEmptyState: React.FC = ({ }) => { const tw = useTailwind(); const chainId = useSelector(selectChainId); - const navigation = useNavigation(); const { trackEvent, createEventBuilder } = useMetrics(); const rampGeodetectedRegion = useSelector(getDetectedGeolocation); + const { goToRamps } = useRampNavigation(); const handleAction = () => { - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: RampType.BUY }, + }); trackEvent( createEventBuilder(MetaMetricsEvents.BUY_BUTTON_CLICKED).build(), diff --git a/app/components/UI/Card/Views/CardHome/CardHome.test.tsx b/app/components/UI/Card/Views/CardHome/CardHome.test.tsx index 85cd8d736af8..e1477007690f 100644 --- a/app/components/UI/Card/Views/CardHome/CardHome.test.tsx +++ b/app/components/UI/Card/Views/CardHome/CardHome.test.tsx @@ -185,6 +185,11 @@ jest.mock('../../hooks/useOpenSwaps', () => ({ useOpenSwaps: jest.fn(), })); +jest.mock('../../../Ramp/hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); + jest.mock('../../hooks/useIsSwapEnabledForPriorityToken', () => ({ useIsSwapEnabledForPriorityToken: jest.fn(), })); @@ -563,6 +568,13 @@ describe('CardHome Component', () => { openSwaps: mockOpenSwaps, }); + const { useRampNavigation } = jest.requireMock( + '../../../Ramp/hooks/useRampNavigation', + ); + (useRampNavigation as jest.Mock).mockReturnValue({ + goToRamps: jest.fn(), + }); + (useMetrics as jest.Mock).mockReturnValue({ trackEvent: mockTrackEvent, createEventBuilder: mockCreateEventBuilder, diff --git a/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.test.tsx b/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.test.tsx index 7f3cd83a60d1..4705fa178c7a 100644 --- a/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.test.tsx +++ b/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.test.tsx @@ -10,15 +10,20 @@ import { trace, TraceName } from '../../../../../util/trace'; import { CardTokenAllowance, AllowanceState } from '../../types'; import { renderScreen } from '../../../../../util/test/renderWithProvider'; import { backgroundState } from '../../../../../util/test/initial-root-state'; -import { createDepositNavigationDetails } from '../../../Ramp/Deposit/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../Ramp/hooks/useRampNavigation'; import { CardHomeSelectors } from '../../../../../../e2e/selectors/Card/CardHome.selectors'; // Mock hooks first - must be hoisted before imports const mockUseParams = jest.fn(); const mockGoBack = jest.fn(); const mockNavigate = jest.fn(); +const mockGoToRamps = jest.fn(); // Mock dependencies +jest.mock('../../../Ramp/hooks/useRampNavigation'); jest.mock('../../hooks/useOpenSwaps', () => ({ useOpenSwaps: jest.fn(), })); @@ -135,6 +140,10 @@ describe('AddFundsBottomSheet', () => { openSwaps: mockOpenSwaps, }); + (useRampNavigation as jest.Mock).mockReturnValue({ + goToRamps: mockGoToRamps, + }); + (useDepositEnabled as jest.Mock).mockReturnValue({ isDepositEnabled: true, }); @@ -292,9 +301,7 @@ describe('AddFundsBottomSheet', () => { fireEvent.press(getByText('Fund with cash')); - expect(mockNavigate).toHaveBeenCalledWith( - ...createDepositNavigationDetails(), - ); + expect(mockGoToRamps).toHaveBeenCalledWith({ mode: RampMode.DEPOSIT }); }); it('renders component correctly', () => { diff --git a/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.tsx b/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.tsx index ea5eba841ad9..989df00607a6 100644 --- a/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.tsx +++ b/app/components/UI/Card/components/AddFundsBottomSheet/AddFundsBottomSheet.tsx @@ -1,6 +1,5 @@ import React, { useCallback, useRef } from 'react'; import { useSelector } from 'react-redux'; -import { useNavigation } from '@react-navigation/native'; import BottomSheet, { BottomSheetRef, } from '../../../../../component-library/components/BottomSheets/BottomSheet'; @@ -31,7 +30,10 @@ import { useOpenSwaps } from '../../hooks/useOpenSwaps'; import { MetaMetricsEvents, useMetrics } from '../../../../hooks/useMetrics'; import { strings } from '../../../../../../locales/i18n'; import { CardHomeSelectors } from '../../../../../../e2e/selectors/Card/CardHome.selectors'; -import { createDepositNavigationDetails } from '../../../Ramp/Deposit/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../Ramp/hooks/useRampNavigation'; import { safeFormatChainIdToHex } from '../../util/safeFormatChainIdToHex'; import { getDetectedGeolocation } from '../../../../../reducers/fiatOrders'; import { @@ -52,7 +54,6 @@ export const createAddFundsModalNavigationDetails = const AddFundsBottomSheet: React.FC = () => { const sheetRef = useRef(null); - const navigation = useNavigation(); const { priorityToken } = useParams(); const { isDepositEnabled } = useDepositEnabled(); @@ -63,6 +64,7 @@ const AddFundsBottomSheet: React.FC = () => { }); const { trackEvent, createEventBuilder } = useMetrics(); const rampGeodetectedRegion = useSelector(getDetectedGeolocation); + const { goToRamps } = useRampNavigation(); const closeBottomSheetAndNavigate = useCallback( (navigateFunc: () => void) => { @@ -80,7 +82,7 @@ const AddFundsBottomSheet: React.FC = () => { const openDeposit = useCallback(() => { closeBottomSheetAndNavigate(() => { - navigation.navigate(...createDepositNavigationDetails()); + goToRamps({ mode: RampMode.DEPOSIT }); }); trackEvent( createEventBuilder( @@ -106,7 +108,7 @@ const AddFundsBottomSheet: React.FC = () => { }, [ rampGeodetectedRegion, closeBottomSheetAndNavigate, - navigation, + goToRamps, trackEvent, createEventBuilder, priorityToken, diff --git a/app/components/UI/FundActionMenu/FundActionMenu.test.tsx b/app/components/UI/FundActionMenu/FundActionMenu.test.tsx index 9b0130b242a1..f8b766d239d1 100644 --- a/app/components/UI/FundActionMenu/FundActionMenu.test.tsx +++ b/app/components/UI/FundActionMenu/FundActionMenu.test.tsx @@ -8,13 +8,13 @@ import { useSelector } from 'react-redux'; import { MetaMetricsEvents } from '../../../core/Analytics'; import { WalletActionsBottomSheetSelectorsIDs } from '../../../../e2e/selectors/wallet/WalletActionsBottomSheet.selectors'; import { RampType } from '../../../reducers/fiatOrders/types'; -import { createDepositNavigationDetails } from '../Ramp/Deposit/routes/utils'; // Internal dependencies. import { useMetrics } from '../../hooks/useMetrics'; import useRampNetwork from '../Ramp/Aggregator/hooks/useRampNetwork'; import useDepositEnabled from '../Ramp/Deposit/hooks/useDepositEnabled'; import useRampsUnifiedV1Enabled from '../Ramp/hooks/useRampsUnifiedV1Enabled'; +import { useRampNavigation, RampMode } from '../Ramp/hooks/useRampNavigation'; import { trace, TraceName } from '../../../util/trace'; import FundActionMenu from './FundActionMenu'; @@ -56,6 +56,7 @@ jest.mock('../../hooks/useMetrics'); jest.mock('../Ramp/Aggregator/hooks/useRampNetwork'); jest.mock('../Ramp/Deposit/hooks/useDepositEnabled'); jest.mock('../Ramp/hooks/useRampsUnifiedV1Enabled'); +jest.mock('../Ramp/hooks/useRampNavigation'); jest.mock('../../../util/trace'); jest.mock('../../../util/networks', () => ({ getDecimalChainId: jest.fn(), @@ -85,6 +86,9 @@ const mockUseRampsUnifiedV1Enabled = useRampsUnifiedV1Enabled as jest.MockedFunction< typeof useRampsUnifiedV1Enabled >; +const mockUseRampNavigation = useRampNavigation as jest.MockedFunction< + typeof useRampNavigation +>; const mockTrace = trace as jest.MockedFunction; const { getDecimalChainId } = jest.requireMock('../../../util/networks'); const { createBuyNavigationDetails, createSellNavigationDetails } = @@ -93,6 +97,7 @@ const { createBuyNavigationDetails, createSellNavigationDetails } = describe('FundActionMenu', () => { // Mock functions const mockNavigate = jest.fn(); + const mockGoToRamps = jest.fn(); const mockTrackEvent = jest.fn(); const mockCreateEventBuilder = jest.fn(); const mockBuild = jest.fn(); @@ -135,6 +140,7 @@ describe('FundActionMenu', () => { mockUseRampNetwork.mockReturnValue([true, true]); mockUseDepositEnabled.mockReturnValue({ isDepositEnabled: true }); mockUseRampsUnifiedV1Enabled.mockReturnValue(false); + mockUseRampNavigation.mockReturnValue({ goToRamps: mockGoToRamps }); getDecimalChainId.mockReturnValue(1); createBuyNavigationDetails.mockReturnValue(['BuyScreen', {}] as never); createSellNavigationDetails.mockReturnValue(['SellScreen', {}] as never); @@ -265,9 +271,7 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith( - ...createDepositNavigationDetails(), - ); + expect(mockGoToRamps).toHaveBeenCalledWith({ mode: RampMode.DEPOSIT }); }); }); @@ -279,7 +283,10 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('BuyScreen', {}); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: RampMode.AGGREGATOR, + params: { rampType: expect.anything() }, + }); }); }); @@ -311,7 +318,10 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('BuyScreen', {}); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: RampMode.AGGREGATOR, + params: { rampType: expect.anything() }, + }); }); }); }); @@ -350,10 +360,13 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(createBuyNavigationDetails).toHaveBeenCalledWith({ - assetId: 'eip155:137/slip44:60', + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: RampMode.AGGREGATOR, + params: { + rampType: expect.anything(), + intent: { assetId: 'eip155:137/slip44:60' }, + }, }); - expect(mockNavigate).toHaveBeenCalledWith('BuyScreen', {}); }); }); @@ -369,8 +382,10 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(createBuyNavigationDetails).toHaveBeenCalledWith(); - expect(mockNavigate).toHaveBeenCalledWith('BuyScreen', {}); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: RampMode.AGGREGATOR, + params: { rampType: expect.anything() }, + }); }); }); @@ -548,7 +563,10 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(createBuyNavigationDetails).toHaveBeenCalledWith(); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: RampMode.AGGREGATOR, + params: { rampType: expect.anything() }, + }); }); }); @@ -567,8 +585,9 @@ describe('FundActionMenu', () => { ); await waitFor(() => { - expect(createBuyNavigationDetails).toHaveBeenCalledWith({ - assetId: undefined, + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: RampMode.AGGREGATOR, + params: { rampType: expect.anything() }, }); }); }); diff --git a/app/components/UI/FundActionMenu/FundActionMenu.tsx b/app/components/UI/FundActionMenu/FundActionMenu.tsx index 7aae2e623399..ce8185eefa83 100644 --- a/app/components/UI/FundActionMenu/FundActionMenu.tsx +++ b/app/components/UI/FundActionMenu/FundActionMenu.tsx @@ -1,7 +1,7 @@ // Third party dependencies. import React, { useCallback, useRef, useMemo } from 'react'; import { useSelector } from 'react-redux'; -import { useNavigation, useRoute } from '@react-navigation/native'; +import { useRoute } from '@react-navigation/native'; // External dependencies. import BottomSheet, { @@ -18,15 +18,12 @@ import { strings } from '../../../../locales/i18n'; // Internal dependencies import { useMetrics } from '../../hooks/useMetrics'; -import { - createBuyNavigationDetails, - createSellNavigationDetails, -} from '../Ramp/Aggregator/routes/utils'; import { trace, TraceName } from '../../../util/trace'; import { selectCanSignTransactions } from '../../../selectors/accountsController'; import { RampType } from '../../../reducers/fiatOrders/types'; import useDepositEnabled from '../Ramp/Deposit/hooks/useDepositEnabled'; -import { createDepositNavigationDetails } from '../Ramp/Deposit/routes/utils'; +import { useRampNavigation, RampMode } from '../Ramp/hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../Ramp/Aggregator/types'; // Types import type { @@ -38,7 +35,6 @@ import useRampsUnifiedV1Enabled from '../Ramp/hooks/useRampsUnifiedV1Enabled'; const FundActionMenu = () => { const sheetRef = useRef(null); - const { navigate } = useNavigation(); const route = useRoute(); const customOnBuy = route.params?.onBuy; @@ -52,6 +48,7 @@ const FundActionMenu = () => { const canSignTransactions = useSelector(selectCanSignTransactions); const rampGeodetectedRegion = useSelector(getDetectedGeolocation); const rampUnifiedV1Enabled = useRampsUnifiedV1Enabled(); + const { goToRamps } = useRampNavigation(); const closeBottomSheetAndNavigate = useCallback( (navigateFunc: () => void) => { @@ -121,18 +118,19 @@ const FundActionMenu = () => { chain_id_destination: getChainIdForAsset(), region: rampGeodetectedRegion, }, - // TODO: Using same action for now, replace with go to buy action navigationAction: () => { if (customOnBuy) { customOnBuy(); - } else if (assetContext) { - navigate( - ...createBuyNavigationDetails({ - assetId: assetContext.assetId, - }), - ); } else { - navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { + rampType: AggregatorRampType.BUY, + intent: assetContext?.assetId + ? { assetId: assetContext.assetId } + : undefined, + }, + }); } }, }, @@ -152,7 +150,7 @@ const FundActionMenu = () => { region: rampGeodetectedRegion, }, traceName: TraceName.LoadDepositExperience, - navigationAction: () => navigate(...createDepositNavigationDetails()), + navigationAction: () => goToRamps({ mode: RampMode.DEPOSIT }), }, { type: 'buy', @@ -173,14 +171,16 @@ const FundActionMenu = () => { navigationAction: () => { if (customOnBuy) { customOnBuy(); - } else if (assetContext) { - navigate( - ...createBuyNavigationDetails({ - assetId: assetContext.assetId, - }), - ); } else { - navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { + rampType: AggregatorRampType.BUY, + intent: assetContext?.assetId + ? { assetId: assetContext.assetId } + : undefined, + }, + }); } }, }, @@ -201,7 +201,11 @@ const FundActionMenu = () => { }, traceName: TraceName.LoadRampExperience, traceProperties: { tags: { rampType: RampType.SELL } }, - navigationAction: () => navigate(...createSellNavigationDetails()), + navigationAction: () => + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.SELL }, + }), }, ] as ActionConfig[], [ @@ -212,9 +216,9 @@ const FundActionMenu = () => { getChainIdForAsset, isNetworkRampSupported, canSignTransactions, - navigate, customOnBuy, assetContext, + goToRamps, ], ); diff --git a/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.test.tsx b/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.test.tsx index 5c825775d1cc..6e6218cd5a3f 100644 --- a/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.test.tsx +++ b/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.test.tsx @@ -9,12 +9,12 @@ import { } from '../../../../../../../util/test/renderWithProvider'; import { backgroundState } from '../../../../../../../util/test/initial-root-state'; import Routes from '../../../../../../../constants/navigation/Routes'; -import { createDepositNavigationDetails } from '../../../../Deposit/routes/utils'; import { RampSDK } from '../../../sdk'; const mockNavigate = jest.fn(); const mockGoBack = jest.fn(); const mockDangerouslyGetParent = jest.fn(); +const mockGoToRamps = jest.fn(); const mockTrackEvent = jest.fn(); jest.mock('@react-navigation/native', () => { @@ -32,6 +32,11 @@ jest.mock('@react-navigation/native', () => { jest.mock('../../../../hooks/useAnalytics', () => () => mockTrackEvent); +jest.mock('../../../../hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); + const mockUseRampSDKValues: DeepPartial = { selectedRegion: { id: 'us' }, }; @@ -112,9 +117,10 @@ describe('SettingsModal', () => { fireEvent.press(newBuyExperienceButton); expect(mockDangerouslyGetParent).toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith( - ...createDepositNavigationDetails(), - ); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'DEPOSIT', + overrideUnifiedBuyFlag: true, + }); }); it('navigates back through parent navigation when deposit is pressed', () => { diff --git a/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.tsx b/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.tsx index 03852f2e3292..52f79d15f0f0 100644 --- a/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.tsx +++ b/app/components/UI/Ramp/Aggregator/Views/Modals/Settings/SettingsModal.tsx @@ -9,7 +9,10 @@ import { IconName } from '../../../../../../../component-library/components/Icon import Routes from '../../../../../../../constants/navigation/Routes'; import { createNavigationDetails } from '../../../../../../../util/navigation/navUtils'; import MenuItem from '../../../../components/MenuItem'; -import { createDepositNavigationDetails } from '../../../../Deposit/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../../hooks/useRampNavigation'; import useAnalytics from '../../../../hooks/useAnalytics'; import { useRampSDK } from '../../../sdk'; @@ -21,6 +24,7 @@ export const createBuySettingsModalNavigationDetails = createNavigationDetails( function SettingsModal() { const sheetRef = useRef(null); const navigation = useNavigation(); + const { goToRamps } = useRampNavigation(); const { selectedRegion } = useRampSDK(); const trackEvent = useAnalytics(); @@ -43,8 +47,8 @@ function SettingsModal() { }); sheetRef.current?.onCloseBottomSheet(); navigation.dangerouslyGetParent()?.dangerouslyGetParent()?.goBack(); - navigation.navigate(...createDepositNavigationDetails()); - }, [navigation, selectedRegion?.id, trackEvent]); + goToRamps({ mode: RampMode.DEPOSIT, overrideUnifiedBuyFlag: true }); + }, [navigation, goToRamps, selectedRegion?.id, trackEvent]); const handleClosePress = useCallback(() => { sheetRef.current?.onCloseBottomSheet(); diff --git a/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.test.tsx b/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.test.tsx index 68c4bd809da8..1cf1cd0da99a 100644 --- a/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.test.tsx +++ b/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.test.tsx @@ -28,8 +28,13 @@ const mockGoBack = jest.fn(); const mockSetNavigationOptions = jest.fn(); const mockTrackEvent = jest.fn(); const mockDispatch = jest.fn(); +const mockGoToRamps = jest.fn(); jest.mock('../../../hooks/useAnalytics', () => () => mockTrackEvent); +jest.mock('../../../hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux'), useDispatch: () => mockDispatch, @@ -302,7 +307,10 @@ describe('OrderDetails', () => { fireEvent.press(screen.getByRole('button', { name: 'Start a new order' })); expect(mockGoBack).toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith(Routes.RAMP.BUY); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'AGGREGATOR', + params: { rampType: expect.anything() }, + }); }); it('navigates to sell flow when the user attempts to make another purchase', async () => { @@ -322,7 +330,10 @@ describe('OrderDetails', () => { fireEvent.press(screen.getByRole('button', { name: 'Start a new order' })); expect(mockGoBack).toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith(Routes.RAMP.SELL); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'AGGREGATOR', + params: { rampType: expect.anything() }, + }); }); it('renders a created order', async () => { diff --git a/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.tsx b/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.tsx index 813842ced1a9..1d9f005ac2f4 100644 --- a/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.tsx +++ b/app/components/UI/Ramp/Aggregator/Views/OrderDetails/OrderDetails.tsx @@ -31,10 +31,8 @@ import { FIAT_ORDER_STATES } from '../../../../../../constants/on-ramp'; import ErrorView from '../../components/ErrorView'; import useInterval from '../../../../../hooks/useInterval'; import AppConstants from '../../../../../../core/AppConstants'; -import { - createBuyNavigationDetails, - createSellNavigationDetails, -} from '../../routes/utils'; +import { useRampNavigation, RampMode } from '../../../hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../../types'; import { useAggregatorOrderNetworkName } from '../../hooks/useAggregatorOrderNetworkName'; interface OrderDetailsParams { @@ -61,6 +59,7 @@ const OrderDetails = () => { const dispatch = useDispatch(); const dispatchThunk = useThunkDispatch(); const getAggregatorOrderNetworkName = useAggregatorOrderNetworkName(); + const { goToRamps } = useRampNavigation(); const [isRefreshing, setIsRefreshing] = useState(false); const [isRefreshingInterval, setIsRefreshingInterval] = useState(false); @@ -189,11 +188,17 @@ const OrderDetails = () => { const handleMakeAnotherPurchase = useCallback(() => { navigation.goBack(); if (order?.orderType === OrderOrderTypeEnum.Buy) { - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + }); } else { - navigation.navigate(...createSellNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.SELL }, + }); } - }, [navigation, order?.orderType]); + }, [navigation, order?.orderType, goToRamps]); useInterval( () => { diff --git a/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.test.tsx b/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.test.tsx index 8b663444427c..40f5e908bf80 100644 --- a/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.test.tsx +++ b/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.test.tsx @@ -202,6 +202,7 @@ function render(Component: React.ReactElement, orders = testOrders) { } const mockNavigate = jest.fn(); +const mockGoToRamps = jest.fn(); jest.mock('@react-navigation/native', () => { const actualReactNavigation = jest.requireActual('@react-navigation/native'); @@ -213,6 +214,11 @@ jest.mock('@react-navigation/native', () => { }; }); +jest.mock('../../../hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); + describe('OrdersList', () => { it('renders correctly', () => { render(); @@ -306,25 +312,9 @@ describe('OrdersList', () => { fireEvent.press(screen.getByRole('button', { name: 'Purchased' })); fireEvent.press(screen.getByRole('button', { name: /USDT Deposit/ })); - expect(mockNavigate).toHaveBeenCalled(); - expect(mockNavigate.mock.calls).toMatchInlineSnapshot(` - [ - [ - "OrderDetails", - { - "orderId": "test-order-2", - }, - ], - [ - "DepositOrderDetails", - { - "orderId": "test-deposit-order-1", - }, - ], - [ - "Deposit", - ], - ] - `); + + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'DEPOSIT', + }); }); }); diff --git a/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.tsx b/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.tsx index ecb107e435f5..ae8e71b24761 100644 --- a/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.tsx +++ b/app/components/UI/Ramp/Aggregator/Views/OrdersList/OrdersList.tsx @@ -6,7 +6,7 @@ import { useSelector } from 'react-redux'; import { OrderOrderTypeEnum } from '@consensys/on-ramp-sdk/dist/API'; import { createOrderDetailsNavDetails } from '../OrderDetails/OrderDetails'; -import { createDepositNavigationDetails } from '../../../Deposit/routes/utils'; +import { useRampNavigation, RampMode } from '../../../hooks/useRampNavigation'; import OrderListItem from '../../components/OrderListItem'; import Row from '../../components/Row'; import createStyles from './OrdersList.styles'; @@ -54,6 +54,7 @@ function OrdersList() { const navigation = useNavigation(); const allOrders = useSelector(getOrders); const [currentFilter, setCurrentFilter] = useState('ALL'); + const { goToRamps } = useRampNavigation(); const orders = allOrders.filter((order) => { if (currentFilter === 'PURCHASE') { return ( @@ -83,7 +84,7 @@ function OrdersList() { const order = orders.find((o) => o.id === orderId); if (order?.state === FIAT_ORDER_STATES.CREATED) { - navigation.navigate(...createDepositNavigationDetails()); + goToRamps({ mode: RampMode.DEPOSIT }); } else { navigation.navigate( ...createDepositOrderDetailsNavDetails({ @@ -92,7 +93,7 @@ function OrdersList() { ); } }, - [navigation, orders], + [navigation, orders, goToRamps], ); const renderItem = ({ item }: { item: FiatOrder }) => ( diff --git a/app/components/UI/Ramp/Aggregator/routes/utils.ts b/app/components/UI/Ramp/Aggregator/routes/utils.ts index 4bfa48a5c5d3..caf49f3ae92c 100644 --- a/app/components/UI/Ramp/Aggregator/routes/utils.ts +++ b/app/components/UI/Ramp/Aggregator/routes/utils.ts @@ -2,7 +2,10 @@ import { RampIntent, RampType } from '../types'; import Routes from '../../../../../constants/navigation/Routes'; // import useRampsUnifiedV1Enabled from '../../hooks/useRampsUnifiedV1Enabled'; -function createRampNavigationDetails(rampType: RampType, intent?: RampIntent) { +export function createRampNavigationDetails( + rampType: RampType, + intent?: RampIntent, +) { const route = rampType === RampType.BUY ? Routes.RAMP.BUY : Routes.RAMP.SELL; if (!intent) { return [route] as const; diff --git a/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.test.tsx b/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.test.tsx index 82a856aeaff0..03f983e1db45 100644 --- a/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.test.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.test.tsx @@ -7,7 +7,6 @@ import { fireEvent, waitFor } from '@testing-library/react-native'; import Routes from '../../../../../../../constants/navigation/Routes'; import { TRANSAK_SUPPORT_URL } from '../../../constants/constants'; import { ToastContext } from '../../../../../../../component-library/components/Toast'; -import { createBuyNavigationDetails } from '../../../../Aggregator/routes/utils'; const mockShowToast = jest.fn(); const mockToastRef = { @@ -46,6 +45,7 @@ const mockNavigate = jest.fn(); const mockGoBack = jest.fn(); const mockSetNavigationOptions = jest.fn(); const mockClearAuthToken = jest.fn(); +const mockGoToRamps = jest.fn(); const mockTrackEvent = jest.fn(); jest.mock('../../../../hooks/useAnalytics', () => () => mockTrackEvent); @@ -80,6 +80,11 @@ jest.mock('../../../sdk', () => ({ useDepositSDK: () => mockUseDepositSDK(), })); +jest.mock('../../../../hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); + jest.mock('../../../../../../../component-library/components/Toast', () => { const actualToast = jest.requireActual( '../../../../../../../component-library/components/Toast', @@ -121,8 +126,14 @@ describe('ConfigurationModal', () => { it('navigates to aggregator when more ways to buy is pressed', () => { const { getByText } = renderWithProvider(ConfigurationModal); const moreWaysToBuyButton = getByText('More ways to buy'); + fireEvent.press(moreWaysToBuyButton); - expect(mockNavigate).toHaveBeenCalledWith(...createBuyNavigationDetails()); + + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'AGGREGATOR', + params: { rampType: expect.anything() }, + overrideUnifiedBuyFlag: true, + }); }); it('should open support URL when contact support is pressed', () => { diff --git a/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.tsx b/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.tsx index 75131337f18d..6ef0e2bf70a2 100644 --- a/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Modals/ConfigurationModal/ConfigurationModal.tsx @@ -9,7 +9,11 @@ import { } from '../../../../../../../component-library/components/Icons/Icon'; import { createNavigationDetails } from '../../../../../../../util/navigation/navUtils'; -import { createBuyNavigationDetails } from '../../../../Aggregator/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../../hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../../../../Aggregator/types'; import Routes from '../../../../../../../constants/navigation/Routes'; import { strings } from '../../../../../../../../locales/i18n'; import { TRANSAK_SUPPORT_URL } from '../../../constants/constants'; @@ -36,6 +40,7 @@ function ConfigurationModal() { const { toastRef } = useContext(ToastContext); const trackEvent = useAnalytics(); + const { goToRamps } = useRampNavigation(); const { logoutFromProvider, isAuthenticated, selectedRegion } = useDepositSDK(); @@ -61,8 +66,12 @@ function ConfigurationModal() { region: selectedRegion?.isoCode as string, }); navigation.dangerouslyGetParent()?.dangerouslyGetParent()?.goBack(); - navigation.navigate(...createBuyNavigationDetails()); - }, [navigation, selectedRegion?.isoCode, trackEvent]); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + overrideUnifiedBuyFlag: true, + }); + }, [navigation, selectedRegion?.isoCode, trackEvent, goToRamps]); const handleLogOut = useCallback(async () => { try { diff --git a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.test.tsx b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.test.tsx index 4e1a275eb3e4..9308e7a440e9 100644 --- a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.test.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.test.tsx @@ -2,12 +2,12 @@ import React from 'react'; import { fireEvent } from '@testing-library/react-native'; import UnsupportedRegionModal from './UnsupportedRegionModal'; import { renderScreen } from '../../../../../../../util/test/renderWithProvider'; -import { createBuyNavigationDetails } from '../../../../Aggregator/routes/utils'; import { createRegionSelectorModalNavigationDetails } from '../RegionSelectorModal'; import Routes from '../../../../../../../constants/navigation/Routes'; import { MOCK_REGIONS } from '../../../testUtils'; const mockNavigate = jest.fn(); +const mockGoToRamps = jest.fn(); const mockUseDepositSDK = jest.fn(); const mockGoBack = jest.fn(); const mockPop = jest.fn(); @@ -31,6 +31,11 @@ jest.mock('../../../sdk', () => ({ useDepositSDK: () => mockUseDepositSDK(), })); +jest.mock('../../../../hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); + const mockUseParams = jest.fn().mockReturnValue({ regions: MOCK_REGIONS, }); @@ -94,7 +99,11 @@ describe('UnsupportedRegionModal', () => { expect(mockDangerouslyGetParent).toHaveBeenCalled(); expect(mockPop).toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith(...createBuyNavigationDetails()); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'AGGREGATOR', + overrideUnifiedBuyFlag: true, + params: { rampType: 'buy' }, + }); }); it('navigates to region selector when Change region button is pressed', () => { diff --git a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.tsx b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.tsx index 040e254223ce..8af9638495c4 100644 --- a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedRegionModal/UnsupportedRegionModal.tsx @@ -28,7 +28,11 @@ import { strings } from '../../../../../../../../locales/i18n'; import { createRegionSelectorModalNavigationDetails } from '../RegionSelectorModal'; import { useDepositSDK } from '../../../sdk'; -import { createBuyNavigationDetails } from '../../../../Aggregator/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../../hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../../../../Aggregator/types'; export interface UnsupportedRegionModalParams { regions: DepositRegion[]; @@ -44,6 +48,7 @@ function UnsupportedRegionModal() { const navigation = useNavigation(); const { selectedRegion } = useDepositSDK(); const { regions } = useParams(); + const { goToRamps } = useRampNavigation(); const { styles } = useStyles(styleSheet, {}); @@ -51,9 +56,13 @@ function UnsupportedRegionModal() { sheetRef.current?.onCloseBottomSheet(() => { // @ts-expect-error navigation prop mismatch navigation.dangerouslyGetParent()?.pop(); - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + overrideUnifiedBuyFlag: true, + }); }); - }, [navigation]); + }, [navigation, goToRamps]); const handleSelectDifferentRegion = useCallback(() => { sheetRef.current?.onCloseBottomSheet(() => { diff --git a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.test.tsx b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.test.tsx index 448c3b36bfc2..2f929108f86f 100644 --- a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.test.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.test.tsx @@ -4,12 +4,12 @@ import { fireEvent } from '@testing-library/react-native'; import UnsupportedStateModal from './UnsupportedStateModal'; import { renderScreen } from '../../../../../../../util/test/renderWithProvider'; import { backgroundState } from '../../../../../../../util/test/initial-root-state'; -import { createBuyNavigationDetails } from '../../../../Aggregator/routes/utils.ts'; import { createStateSelectorModalNavigationDetails } from '../StateSelectorModal/StateSelectorModal.tsx'; import Routes from '../../../../../../../constants/navigation/Routes'; const mockUseDepositSDK = jest.fn(); const mockNavigate = jest.fn(); +const mockGoToRamps = jest.fn(); const mockDangerouslyGetParent = jest.fn(); const mockPop = jest.fn(); const mockGoBack = jest.fn(); @@ -53,6 +53,11 @@ jest.mock('../../../../../../../util/navigation/navUtils', () => ({ })), })); +jest.mock('../../../../hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(() => ({ goToRamps: mockGoToRamps })), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); + function render(Component: React.ComponentType) { return renderScreen( Component, @@ -99,7 +104,11 @@ describe('UnsupportedStateModal', () => { expect(mockDangerouslyGetParent).toHaveBeenCalled(); expect(mockPop).toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith(...createBuyNavigationDetails()); + expect(mockGoToRamps).toHaveBeenCalledWith({ + mode: 'AGGREGATOR', + overrideUnifiedBuyFlag: true, + params: { rampType: 'buy' }, + }); }); it('handles select different state button press correctly', () => { diff --git a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.tsx b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.tsx index 6648362cb80b..f5e0e7e218ea 100644 --- a/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Modals/UnsupportedStateModal/UnsupportedStateModal.tsx @@ -27,7 +27,11 @@ import { strings } from '../../../../../../../../locales/i18n'; import { createStateSelectorModalNavigationDetails } from '../StateSelectorModal'; import { useDepositSDK } from '../../../sdk'; -import { createBuyNavigationDetails } from '../../../../Aggregator/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../../hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../../../../Aggregator/types'; export interface UnsupportedStateModalParams { stateCode?: string; @@ -47,6 +51,7 @@ function UnsupportedStateModal() { const { selectedRegion } = useDepositSDK(); const { stateCode, stateName, onStateSelect } = useParams(); + const { goToRamps } = useRampNavigation(); const { styles } = useStyles(styleSheet, {}); @@ -72,9 +77,13 @@ function UnsupportedStateModal() { closeBottomSheetAndNavigate(() => { // @ts-expect-error navigation prop mismatch navigation.dangerouslyGetParent()?.pop(); - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + overrideUnifiedBuyFlag: true, + }); }); - }, [closeBottomSheetAndNavigate, navigation]); + }, [closeBottomSheetAndNavigate, navigation, goToRamps]); const handleClose = useCallback(() => { closeBottomSheetAndNavigate(() => { diff --git a/app/components/UI/Ramp/hooks/useRampNavigation.test.ts b/app/components/UI/Ramp/hooks/useRampNavigation.test.ts index 911b3792ed96..724e7e5aa0e2 100644 --- a/app/components/UI/Ramp/hooks/useRampNavigation.test.ts +++ b/app/components/UI/Ramp/hooks/useRampNavigation.test.ts @@ -1,14 +1,12 @@ -import { renderHook } from '@testing-library/react-hooks'; +import { renderHookWithProvider } from '../../../../util/test/renderWithProvider'; import { useNavigation } from '@react-navigation/native'; import Routes from '../../../../constants/navigation/Routes'; import { useRampNavigation, RampMode } from './useRampNavigation'; -import { - createBuyNavigationDetails, - createSellNavigationDetails, -} from '../Aggregator/routes/utils'; +import { createRampNavigationDetails } from '../Aggregator/routes/utils'; import { createDepositNavigationDetails } from '../Deposit/routes/utils'; import { RampType as AggregatorRampType } from '../Aggregator/types'; import useRampsUnifiedV1Enabled from './useRampsUnifiedV1Enabled'; +import { UnifiedRampRoutingType } from '../../../../reducers/fiatOrders'; jest.mock('@react-navigation/native'); jest.mock('../Aggregator/routes/utils'); @@ -24,36 +22,38 @@ const mockUseRampsUnifiedV1Enabled = typeof useRampsUnifiedV1Enabled >; +let mockRampRoutingDecision: UnifiedRampRoutingType | null = null; + describe('useRampNavigation', () => { - const mockCreateBuyNavigationDetails = - createBuyNavigationDetails as jest.MockedFunction< - typeof createBuyNavigationDetails - >; - const mockCreateSellNavigationDetails = - createSellNavigationDetails as jest.MockedFunction< - typeof createSellNavigationDetails + const mockCreateRampNavigationDetails = + createRampNavigationDetails as jest.MockedFunction< + typeof createRampNavigationDetails >; const mockCreateDepositNavigationDetails = createDepositNavigationDetails as jest.MockedFunction< typeof createDepositNavigationDetails >; + const createMockState = () => ({ + fiatOrders: { + rampRoutingDecision: mockRampRoutingDecision, + }, + }); + beforeEach(() => { jest.clearAllMocks(); + mockRampRoutingDecision = null; + mockUseNavigation.mockReturnValue({ navigate: mockNavigate, } as unknown as ReturnType); mockUseRampsUnifiedV1Enabled.mockReturnValue(false); - mockCreateBuyNavigationDetails.mockReturnValue([ + mockCreateRampNavigationDetails.mockReturnValue([ Routes.RAMP.BUY, - ] as unknown as ReturnType); - - mockCreateSellNavigationDetails.mockReturnValue([ - Routes.RAMP.SELL, - ] as unknown as ReturnType); + ] as unknown as ReturnType); mockCreateDepositNavigationDetails.mockReturnValue([ Routes.DEPOSIT.ID, @@ -63,23 +63,29 @@ describe('useRampNavigation', () => { describe('RampMode.AGGREGATOR', () => { it('navigates to buy route when mode is AGGREGATOR without params (defaults to BUY)', () => { const mockNavDetails = [Routes.RAMP.BUY] as const; - mockCreateBuyNavigationDetails.mockReturnValue(mockNavDetails); + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.AGGREGATOR }); - expect(mockCreateBuyNavigationDetails).toHaveBeenCalledWith(undefined); + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); - expect(mockCreateSellNavigationDetails).not.toHaveBeenCalled(); expect(mockCreateDepositNavigationDetails).not.toHaveBeenCalled(); }); it('navigates to buy route when mode is AGGREGATOR with rampType BUY', () => { const mockNavDetails = [Routes.RAMP.BUY] as const; - mockCreateBuyNavigationDetails.mockReturnValue(mockNavDetails); + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.AGGREGATOR, @@ -88,16 +94,20 @@ describe('useRampNavigation', () => { }, }); - expect(mockCreateBuyNavigationDetails).toHaveBeenCalledWith(undefined); + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); - expect(mockCreateSellNavigationDetails).not.toHaveBeenCalled(); }); it('navigates to sell route when mode is AGGREGATOR with rampType SELL', () => { const mockNavDetails = [Routes.RAMP.SELL] as const; - mockCreateSellNavigationDetails.mockReturnValue(mockNavDetails); + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.AGGREGATOR, @@ -106,17 +116,21 @@ describe('useRampNavigation', () => { }, }); - expect(mockCreateSellNavigationDetails).toHaveBeenCalledWith(undefined); + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.SELL, + undefined, + ); expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); - expect(mockCreateBuyNavigationDetails).not.toHaveBeenCalled(); }); - it('passes intent to createBuyNavigationDetails when provided', () => { + it('passes intent to createRampNavigationDetails when provided for BUY', () => { const intent = { assetId: 'eip155:1/erc20:0x123' }; const mockNavDetails = [Routes.RAMP.BUY] as const; - mockCreateBuyNavigationDetails.mockReturnValue(mockNavDetails); + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.AGGREGATOR, @@ -126,16 +140,21 @@ describe('useRampNavigation', () => { }, }); - expect(mockCreateBuyNavigationDetails).toHaveBeenCalledWith(intent); + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + intent, + ); expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); }); - it('passes intent to createSellNavigationDetails when provided', () => { + it('passes intent to createRampNavigationDetails when provided for SELL', () => { const intent = { assetId: 'eip155:1/erc20:0x123' }; const mockNavDetails = [Routes.RAMP.SELL] as const; - mockCreateSellNavigationDetails.mockReturnValue(mockNavDetails); + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.AGGREGATOR, @@ -145,7 +164,10 @@ describe('useRampNavigation', () => { }, }); - expect(mockCreateSellNavigationDetails).toHaveBeenCalledWith(intent); + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.SELL, + intent, + ); expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); }); }); @@ -155,7 +177,9 @@ describe('useRampNavigation', () => { const mockNavDetails = [Routes.DEPOSIT.ID] as const; mockCreateDepositNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.DEPOSIT }); @@ -163,8 +187,7 @@ describe('useRampNavigation', () => { undefined, ); expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); - expect(mockCreateBuyNavigationDetails).not.toHaveBeenCalled(); - expect(mockCreateSellNavigationDetails).not.toHaveBeenCalled(); + expect(mockCreateRampNavigationDetails).not.toHaveBeenCalled(); }); it('passes params to createDepositNavigationDetails when provided', () => { @@ -172,7 +195,9 @@ describe('useRampNavigation', () => { const mockNavDetails = [Routes.DEPOSIT.ID] as const; mockCreateDepositNavigationDetails.mockReturnValue(mockNavDetails); - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); result.current.goToRamps({ mode: RampMode.DEPOSIT, params }); @@ -181,68 +206,234 @@ describe('useRampNavigation', () => { }); }); - // TODO: use smart routing logic when we have it describe('when unified V1 is enabled', () => { beforeEach(() => { mockUseRampsUnifiedV1Enabled.mockReturnValue(true); }); - it('returns early without navigating for AGGREGATOR mode', () => { - const { result } = renderHook(() => useRampNavigation()); + describe('smart routing based on routing decision', () => { + it('navigates to deposit when routing decision is DEPOSIT', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.DEPOSIT; + const mockNavDetails = [Routes.DEPOSIT.ID] as const; + mockCreateDepositNavigationDetails.mockReturnValue(mockNavDetails); - result.current.goToRamps({ mode: RampMode.AGGREGATOR }); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); - expect(mockNavigate).not.toHaveBeenCalled(); - expect(mockCreateBuyNavigationDetails).not.toHaveBeenCalled(); - expect(mockCreateSellNavigationDetails).not.toHaveBeenCalled(); - expect(mockCreateDepositNavigationDetails).not.toHaveBeenCalled(); - }); + result.current.goToRamps({ mode: RampMode.AGGREGATOR }); - it('returns early without navigating for AGGREGATOR mode with BUY type', () => { - const { result } = renderHook(() => useRampNavigation()); + expect(mockCreateDepositNavigationDetails).toHaveBeenCalledWith( + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + expect(mockCreateRampNavigationDetails).not.toHaveBeenCalled(); + }); - result.current.goToRamps({ - mode: RampMode.AGGREGATOR, - params: { - rampType: AggregatorRampType.BUY, - }, + it('navigates to deposit with params when routing decision is DEPOSIT and mode is DEPOSIT', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.DEPOSIT; + const params = { assetId: 'eip155:1/erc20:0x123', amount: '100' }; + const mockNavDetails = [Routes.DEPOSIT.ID] as const; + mockCreateDepositNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ mode: RampMode.DEPOSIT, params }); + + expect(mockCreateDepositNavigationDetails).toHaveBeenCalledWith(params); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); }); - expect(mockNavigate).not.toHaveBeenCalled(); - expect(mockCreateBuyNavigationDetails).not.toHaveBeenCalled(); - }); + it('navigates to aggregator when routing decision is AGGREGATOR', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.AGGREGATOR; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - it('returns early without navigating for AGGREGATOR mode with SELL type', () => { - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); - result.current.goToRamps({ - mode: RampMode.AGGREGATOR, - params: { - rampType: AggregatorRampType.SELL, - }, + result.current.goToRamps({ mode: RampMode.DEPOSIT }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + expect(mockCreateDepositNavigationDetails).not.toHaveBeenCalled(); }); - expect(mockNavigate).not.toHaveBeenCalled(); - expect(mockCreateSellNavigationDetails).not.toHaveBeenCalled(); - }); + it('navigates to aggregator with BUY when routing decision is AGGREGATOR and params specify BUY', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.AGGREGATOR; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ + mode: RampMode.AGGREGATOR, + params: { + rampType: AggregatorRampType.BUY, + }, + }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + }); - it('returns early without navigating for DEPOSIT mode', () => { - const { result } = renderHook(() => useRampNavigation()); + it('navigates to aggregator with SELL when routing decision is AGGREGATOR and params specify SELL', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.AGGREGATOR; + const mockNavDetails = [Routes.RAMP.SELL] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ + mode: RampMode.AGGREGATOR, + params: { + rampType: AggregatorRampType.SELL, + }, + }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.SELL, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + }); - result.current.goToRamps({ mode: RampMode.DEPOSIT }); + it('navigates to aggregator with intent when routing decision is AGGREGATOR and intent is provided', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.AGGREGATOR; + const intent = { assetId: 'eip155:1/erc20:0x123' }; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ + mode: RampMode.AGGREGATOR, + params: { + intent, + rampType: AggregatorRampType.BUY, + }, + }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + intent, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + }); - expect(mockNavigate).not.toHaveBeenCalled(); - expect(mockCreateDepositNavigationDetails).not.toHaveBeenCalled(); - }); + it('navigates to aggregator when routing decision is UNSUPPORTED (defaults to aggregator)', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.UNSUPPORTED; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); - it('returns early without navigating for DEPOSIT mode with params', () => { - const params = { assetId: 'eip155:1/erc20:0x123', amount: '100' }; - const { result } = renderHook(() => useRampNavigation()); + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); - result.current.goToRamps({ mode: RampMode.DEPOSIT, params }); + result.current.goToRamps({ mode: RampMode.AGGREGATOR }); - expect(mockNavigate).not.toHaveBeenCalled(); - expect(mockCreateDepositNavigationDetails).not.toHaveBeenCalled(); + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + }); + + it('navigates to aggregator when routing decision is ERROR (defaults to aggregator)', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.ERROR; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ mode: RampMode.AGGREGATOR }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + }); + + it('navigates to aggregator when routing decision is null (defaults to aggregator)', () => { + mockRampRoutingDecision = null; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ mode: RampMode.AGGREGATOR }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + }); + }); + + describe('overrideUnifiedBuyFlag', () => { + it('uses original navigation logic when overrideUnifiedBuyFlag is true', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.DEPOSIT; + const mockNavDetails = [Routes.RAMP.BUY] as const; + mockCreateRampNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ + mode: RampMode.AGGREGATOR, + overrideUnifiedBuyFlag: true, + }); + + expect(mockCreateRampNavigationDetails).toHaveBeenCalledWith( + AggregatorRampType.BUY, + undefined, + ); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + expect(mockCreateDepositNavigationDetails).not.toHaveBeenCalled(); + }); + + it('uses original navigation logic for DEPOSIT mode when overrideUnifiedBuyFlag is true', () => { + mockRampRoutingDecision = UnifiedRampRoutingType.AGGREGATOR; + const params = { assetId: 'eip155:1/erc20:0x123', amount: '100' }; + const mockNavDetails = [Routes.DEPOSIT.ID] as const; + mockCreateDepositNavigationDetails.mockReturnValue(mockNavDetails); + + const { result } = renderHookWithProvider(() => useRampNavigation(), { + state: createMockState(), + }); + + result.current.goToRamps({ + mode: RampMode.DEPOSIT, + params, + overrideUnifiedBuyFlag: true, + }); + + expect(mockCreateDepositNavigationDetails).toHaveBeenCalledWith(params); + expect(mockNavigate).toHaveBeenCalledWith(...mockNavDetails); + expect(mockCreateRampNavigationDetails).not.toHaveBeenCalled(); + }); }); }); }); diff --git a/app/components/UI/Ramp/hooks/useRampNavigation.ts b/app/components/UI/Ramp/hooks/useRampNavigation.ts index 791b1908a1b1..ee887b243706 100644 --- a/app/components/UI/Ramp/hooks/useRampNavigation.ts +++ b/app/components/UI/Ramp/hooks/useRampNavigation.ts @@ -1,16 +1,18 @@ import { useCallback } from 'react'; import { useNavigation } from '@react-navigation/native'; +import { useSelector } from 'react-redux'; import { RampIntent, RampType as AggregatorRampType, } from '../Aggregator/types'; import { DepositNavigationParams } from '../Deposit/types/navigationParams'; -import { - createBuyNavigationDetails, - createSellNavigationDetails, -} from '../Aggregator/routes/utils'; +import { createRampNavigationDetails } from '../Aggregator/routes/utils'; import { createDepositNavigationDetails } from '../Deposit/routes/utils'; import useRampsUnifiedV1Enabled from './useRampsUnifiedV1Enabled'; +import { + getRampRoutingDecision, + UnifiedRampRoutingType, +} from '../../../../reducers/fiatOrders'; export enum RampMode { AGGREGATOR = 'AGGREGATOR', @@ -25,11 +27,13 @@ interface AggregatorParams { interface AggregatorGoToRampsParams { mode: RampMode.AGGREGATOR; params?: AggregatorParams; + overrideUnifiedBuyFlag?: boolean; } interface DepositGoToRampsParams { mode: RampMode.DEPOSIT; params?: DepositNavigationParams; + overrideUnifiedBuyFlag?: boolean; } type GoToRampsParams = AggregatorGoToRampsParams | DepositGoToRampsParams; @@ -43,11 +47,25 @@ type GoToRampsParams = AggregatorGoToRampsParams | DepositGoToRampsParams; export const useRampNavigation = () => { const navigation = useNavigation(); const isRampsUnifiedV1Enabled = useRampsUnifiedV1Enabled(); + const rampRoutingDecision = useSelector(getRampRoutingDecision); const goToRamps = useCallback( - ({ mode, params }: GoToRampsParams) => { - if (isRampsUnifiedV1Enabled) { - // TODO: Implement smart routing hook + ({ mode, params, overrideUnifiedBuyFlag }: GoToRampsParams) => { + if (isRampsUnifiedV1Enabled && !overrideUnifiedBuyFlag) { + if (rampRoutingDecision === UnifiedRampRoutingType.DEPOSIT) { + navigation.navigate( + ...createDepositNavigationDetails( + mode === RampMode.DEPOSIT ? params : undefined, + ), + ); + } else { + const aggregatorParams = + mode === RampMode.AGGREGATOR ? params : undefined; + const { intent, rampType = AggregatorRampType.BUY } = + aggregatorParams || {}; + + navigation.navigate(...createRampNavigationDetails(rampType, intent)); + } return; } @@ -55,15 +73,10 @@ export const useRampNavigation = () => { navigation.navigate(...createDepositNavigationDetails(params)); } else { const { intent, rampType = AggregatorRampType.BUY } = params || {}; - - if (rampType === AggregatorRampType.BUY) { - navigation.navigate(...createBuyNavigationDetails(intent)); - } else { - navigation.navigate(...createSellNavigationDetails(intent)); - } + navigation.navigate(...createRampNavigationDetails(rampType, intent)); } }, - [navigation, isRampsUnifiedV1Enabled], + [navigation, isRampsUnifiedV1Enabled, rampRoutingDecision], ); return { goToRamps }; diff --git a/app/components/UI/Ramp/hooks/withRampNavigation.tsx b/app/components/UI/Ramp/hooks/withRampNavigation.tsx new file mode 100644 index 000000000000..0a2fb41e4d5c --- /dev/null +++ b/app/components/UI/Ramp/hooks/withRampNavigation.tsx @@ -0,0 +1,28 @@ +import React from 'react'; +import { useRampNavigation, RampMode } from './useRampNavigation'; +import { RampType as AggregatorRampType } from '../Aggregator/types'; + +export interface WithRampNavigationProps { + goToRamps: ReturnType['goToRamps']; + RampMode: typeof RampMode; + AggregatorRampType: typeof AggregatorRampType; +} + +export function withRampNavigation

( + Component: React.ComponentType

, +): React.ComponentType> { + return function WithRampNavigationWrapper( + props: Omit, + ) { + const { goToRamps } = useRampNavigation(); + + return ( + + ); + }; +} diff --git a/app/components/UI/ReceiveRequest/index.js b/app/components/UI/ReceiveRequest/index.js index e0fd54d10039..6e3b9ac5f52a 100644 --- a/app/components/UI/ReceiveRequest/index.js +++ b/app/components/UI/ReceiveRequest/index.js @@ -26,7 +26,7 @@ import ClipboardManager from '../../../core/ClipboardManager'; import { ThemeContext, mockTheme } from '../../../util/theme'; import { selectChainId } from '../../../selectors/networkController'; import { isNetworkRampSupported } from '../Ramp/Aggregator/utils'; -import { createBuyNavigationDetails } from '../Ramp/Aggregator/routes/utils'; +import { withRampNavigation } from '../Ramp/hooks/withRampNavigation'; import { selectSelectedInternalAccountFormattedAddress } from '../../../selectors/accountsController'; import { getDetectedGeolocation, @@ -77,6 +77,18 @@ class ReceiveRequest extends PureComponent { /* Triggers global alert */ showAlert: PropTypes.func, + /** + * Function to navigate to ramp flows + */ + goToRamps: PropTypes.func, + /** + * RampMode enum + */ + RampMode: PropTypes.object, + /** + * AggregatorRampType enum + */ + AggregatorRampType: PropTypes.object, /** * Network provider chain id */ @@ -144,14 +156,18 @@ class ReceiveRequest extends PureComponent { * Shows an alert message with a coming soon message */ onBuy = async () => { - const { navigation, isNetworkBuySupported } = this.props; + const { isNetworkBuySupported, goToRamps, RampMode, AggregatorRampType } = + this.props; if (!isNetworkBuySupported) { Alert.alert( strings('fiat_on_ramp.network_not_supported'), strings('fiat_on_ramp.switch_network'), ); } else { - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + }); this.props.metrics.trackEvent( this.props.metrics @@ -270,4 +286,4 @@ const mapDispatchToProps = (dispatch) => ({ export default connect( mapStateToProps, mapDispatchToProps, -)(withMetricsAwareness(ReceiveRequest)); +)(withRampNavigation(withMetricsAwareness(ReceiveRequest))); diff --git a/app/components/UI/Swaps/QuotesView.js b/app/components/UI/Swaps/QuotesView.js index e15e204bbfdc..48f7a6247bad 100644 --- a/app/components/UI/Swaps/QuotesView.js +++ b/app/components/UI/Swaps/QuotesView.js @@ -110,7 +110,8 @@ import { selectAccounts } from '../../../selectors/accountTrackerController'; import { selectContractBalances } from '../../../selectors/tokenBalancesController'; import { selectSelectedInternalAccountFormattedAddress } from '../../../selectors/accountsController'; import { resetTransaction, setRecipient } from '../../../actions/transaction'; -import { createBuyNavigationDetails } from '../Ramp/Aggregator/routes/utils'; +import { useRampNavigation, RampMode } from '../Ramp/hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../Ramp/Aggregator/types'; import { SwapsViewSelectorsIDs } from '../../../../e2e/selectors/swaps/SwapsView.selectors'; import { useMetrics } from '../../../components/hooks/useMetrics'; import { addTransaction } from '../../../util/transaction-controller'; @@ -415,6 +416,7 @@ function SwapsQuotesView({ /* Get params from navigation */ const route = useRoute(); const { trackEvent, createEventBuilder } = useMetrics(); + const { goToRamps } = useRampNavigation(); const { colors } = useTheme(); const styles = createStyles(colors); @@ -1503,7 +1505,10 @@ function SwapsQuotesView({ const buyEth = useCallback(() => { try { - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + }); } catch (error) { Logger.error(error, 'Navigation: Error when navigating to buy ETH.'); } @@ -1513,7 +1518,7 @@ function SwapsQuotesView({ MetaMetricsEvents.RECEIVE_OPTIONS_PAYMENT_REQUEST, ).build(), ); - }, [navigation, trackEvent, createEventBuilder]); + }, [goToRamps, trackEvent, createEventBuilder]); const handleTermsPress = useCallback( () => diff --git a/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.test.ts b/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.test.ts index 9714dacda147..570f71c8d645 100644 --- a/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.test.ts +++ b/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.test.ts @@ -15,6 +15,7 @@ import { useConfirmActions } from '../useConfirmActions'; import { useTransactionPayToken } from '../pay/useTransactionPayToken'; import { noop } from 'lodash'; import { useConfirmationContext } from '../../context/confirmation-context'; +import { useRampNavigation } from '../../../../UI/Ramp/hooks/useRampNavigation'; import { useIsGaslessSupported } from '../gas/useIsGaslessSupported'; jest.mock('../../../../../util/navigation/navUtils', () => ({ @@ -48,6 +49,10 @@ jest.mock('../../../../../reducers/transaction', () => ({ selectTransactionState: jest.fn(), })); jest.mock('../../context/confirmation-context'); +jest.mock('../../../../UI/Ramp/hooks/useRampNavigation', () => ({ + useRampNavigation: jest.fn(), + RampMode: { AGGREGATOR: 'AGGREGATOR', DEPOSIT: 'DEPOSIT' }, +})); jest.mock('../gas/useIsGaslessSupported'); describe('useInsufficientBalanceAlert', () => { @@ -61,6 +66,8 @@ describe('useInsufficientBalanceAlert', () => { ); const mockUseTransactionPayToken = jest.mocked(useTransactionPayToken); const mockUseConfirmationContext = jest.mocked(useConfirmationContext); + const mockUseRampNavigation = jest.mocked(useRampNavigation); + const mockGoToRamps = jest.fn(); const useIsGaslessSupportedMock = jest.mocked(useIsGaslessSupported); const mockChainId = '0x1'; @@ -116,6 +123,9 @@ describe('useInsufficientBalanceAlert', () => { mockUseConfirmationContext.mockReturnValue({ isTransactionValueUpdating: false, } as unknown as ReturnType); + mockUseRampNavigation.mockReturnValue({ + goToRamps: mockGoToRamps, + }); }); it('return empty array when no transaction metadata is available', () => { diff --git a/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.ts b/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.ts index 7e268671c090..a0050f274e0c 100644 --- a/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.ts +++ b/app/components/Views/confirmations/hooks/alerts/useInsufficientBalanceAlert.ts @@ -1,7 +1,6 @@ import { useMemo } from 'react'; import { Hex, add0x } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; -import { useNavigation } from '@react-navigation/native'; import { useSelector } from 'react-redux'; import { addHexes, @@ -10,7 +9,11 @@ import { } from '../../../../../util/conversions'; import { strings } from '../../../../../../locales/i18n'; import { selectNetworkConfigurations } from '../../../../../selectors/networkController'; -import { createBuyNavigationDetails } from '../../../../UI/Ramp/Aggregator/routes/utils'; +import { + useRampNavigation, + RampMode, +} from '../../../../UI/Ramp/hooks/useRampNavigation'; +import { RampType as AggregatorRampType } from '../../../../UI/Ramp/Aggregator/types'; import { RowAlertKey } from '../../components/UI/info-row/alert-row/constants'; import { AlertKeys } from '../../constants/alerts'; import { Alert, Severity } from '../../types/alerts'; @@ -35,7 +38,7 @@ export const useInsufficientBalanceAlert = ({ }: { ignoreGasFeeToken?: boolean; } = {}): Alert[] => { - const navigation = useNavigation(); + const { goToRamps } = useRampNavigation(); const transactionMetadata = useTransactionMetadataRequest(); const networkConfigurations = useSelector(selectNetworkConfigurations); const { balanceWeiInHex } = useAccountNativeBalance( @@ -92,7 +95,10 @@ export const useInsufficientBalanceAlert = ({ nativeCurrency, }), callback: () => { - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + }); onReject(undefined, true); }, }, @@ -112,9 +118,9 @@ export const useInsufficientBalanceAlert = ({ ignoreGasFeeToken, isGaslessSupported, isTransactionValueUpdating, - navigation, networkConfigurations, onReject, transactionMetadata, + goToRamps, ]); }; diff --git a/app/components/Views/confirmations/legacy/SendFlow/SendTo/index.js b/app/components/Views/confirmations/legacy/SendFlow/SendTo/index.js index aca99e7e1b03..237e13581738 100644 --- a/app/components/Views/confirmations/legacy/SendFlow/SendTo/index.js +++ b/app/components/Views/confirmations/legacy/SendFlow/SendTo/index.js @@ -59,7 +59,7 @@ import { } from '../../../../../../selectors/accountsController'; import AddToAddressBookWrapper from '../../../../../UI/AddToAddressBookWrapper'; import { isNetworkRampNativeTokenSupported } from '../../../../../UI/Ramp/Aggregator/utils'; -import { createBuyNavigationDetails } from '../../../../../UI/Ramp/Aggregator/routes/utils'; +import { withRampNavigation } from '../../../../../UI/Ramp/hooks/withRampNavigation'; import { getDetectedGeolocation, getRampNetworks, @@ -101,6 +101,18 @@ class SendFlow extends PureComponent { * Selected address as string */ selectedAddress: PropTypes.string, + /** + * Function to navigate to ramp flows + */ + goToRamps: PropTypes.func, + /** + * RampMode enum + */ + RampMode: PropTypes.object, + /** + * AggregatorRampType enum + */ + AggregatorRampType: PropTypes.object, /** * List of accounts from the AccountsController */ @@ -335,7 +347,10 @@ class SendFlow extends PureComponent { }; goToBuy = () => { - this.props.navigation.navigate(...createBuyNavigationDetails()); + this.props.goToRamps({ + mode: this.props.RampMode.AGGREGATOR, + params: { rampType: this.props.AggregatorRampType.BUY }, + }); this.props.metrics.trackEvent( this.props.metrics @@ -799,4 +814,4 @@ const mapDispatchToProps = (dispatch) => ({ export default connect( mapStateToProps, mapDispatchToProps, -)(withMetricsAwareness(SendFlow)); +)(withRampNavigation(withMetricsAwareness(SendFlow))); diff --git a/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js b/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js index 085dc9746de0..066d4e10287a 100644 --- a/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js +++ b/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js @@ -97,7 +97,7 @@ import TransactionBlockaidBanner from '../TransactionBlockaidBanner/TransactionB import { regex } from '../../../../../../util/regex'; import { withMetricsAwareness } from '../../../../../../components/hooks/useMetrics'; import { selectShouldUseSmartTransaction } from '../../../../../../selectors/smartTransactionsController'; -import { createBuyNavigationDetails } from '../../../../../UI/Ramp/Aggregator/routes/utils'; +import { withRampNavigation } from '../../../../../UI/Ramp/hooks/withRampNavigation'; import SDKConnect from '../../../../../../core/SDKConnect/SDKConnect'; import DevLogger from '../../../../../../core/SDKConnect/utils/DevLogger'; import { WC2Manager } from '../../../../../../core/WalletConnect/WalletConnectV2'; @@ -142,6 +142,18 @@ class ApproveTransactionReview extends PureComponent { * Current provider ticker */ ticker: PropTypes.string, + /** + * Function to navigate to ramp flows + */ + goToRamps: PropTypes.func, + /** + * RampMode enum + */ + RampMode: PropTypes.object, + /** + * AggregatorRampType enum + */ + AggregatorRampType: PropTypes.object, /** * Number of tokens */ @@ -1229,11 +1241,14 @@ class ApproveTransactionReview extends PureComponent { }; buyEth = () => { - const { navigation } = this.props; + const { goToRamps, RampMode, AggregatorRampType } = this.props; /* this is kinda weird, we have to reject the transaction to collapse the modal */ this.onCancelPress(); try { - navigation.navigate(...createBuyNavigationDetails()); + goToRamps({ + mode: RampMode.AGGREGATOR, + params: { rampType: AggregatorRampType.BUY }, + }); } catch (error) { Logger.error(error, 'Navigation: Error when navigating to buy ETH.'); } @@ -1382,7 +1397,9 @@ export default connect( mapStateToProps, mapDispatchToProps, )( - withNavigation( - withQRHardwareAwareness(withMetricsAwareness(ApproveTransactionReview)), + withRampNavigation( + withNavigation( + withQRHardwareAwareness(withMetricsAwareness(ApproveTransactionReview)), + ), ), ); From c722d72610507139caa82fdcbb321965b7a4e137 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 14 Nov 2025 15:37:11 +0000 Subject: [PATCH 15/17] refactor: remove dead code, and add/cleanup notification tests (#22681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Removed dead notification code (e.g. using notifee badges), and improves test suites ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** N/A ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] 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] > Simplifies the notifications list to a single data source, removes badge updates on item click, and replaces snapshots with data-driven tests using shared mocks and selectors. > > - **Notifications UI**: > - Simplifies `Notifications` props to only `allNotifications` and `loading`; removes `walletNotifications`/`web3Notifications`. > - Adds `TEST_IDS.loadingContainer` and uses test IDs for list states and items. > - Removes badge updates from `useNotificationOnClick`; now only marks as read, tracks metrics, and conditionally navigates when a modal exists. > - **Notifications View**: > - Consolidates filtering to return `allNotifications` only; updates rendering accordingly. > - Keeps "Mark all as read" flow; still sets badge count to 0. > - **Tests**: > - Deletes snapshot file; replaces with state-driven, parameterized tests for loading/empty/data and item rendering. > - Mocks `useMarkNotificationAsRead`; verifies metrics and conditional navigation. > - Updates notification-state and node-guard tests to use shared processed mocks (`mockNotificationsWithMetaData`) and controller mocks. > - **Mocks/Utilities**: > - Introduces `mockNotificationsWithMetaData` (with `hasModal`) and switches imports to `notification-services` module mocks. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fa706304e5fbf94f9178d240d2f9e69f3f6aab68. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../List/__snapshots__/index.test.tsx.snap | 149 ----------- .../UI/Notification/List/index.test.tsx | 247 +++++++++--------- app/components/UI/Notification/List/index.tsx | 18 +- .../__mocks__/mock_notifications.ts | 105 +++++++- .../Views/Notifications/index.test.tsx | 4 +- app/components/Views/Notifications/index.tsx | 35 +-- .../notification-states/index.test.tsx | 107 +------- .../notification-states/node-guard.test.ts | 29 +- 8 files changed, 260 insertions(+), 434 deletions(-) delete mode 100644 app/components/UI/Notification/List/__snapshots__/index.test.tsx.snap diff --git a/app/components/UI/Notification/List/__snapshots__/index.test.tsx.snap b/app/components/UI/Notification/List/__snapshots__/index.test.tsx.snap deleted file mode 100644 index aef0bac40de2..000000000000 --- a/app/components/UI/Notification/List/__snapshots__/index.test.tsx.snap +++ /dev/null @@ -1,149 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`NotificationsList renders correctly 1`] = ` - - - - - -`; - -exports[`NotificationsList renders empty state 1`] = ` - - - } - contentContainerStyle={ - { - "flexGrow": 1, - "paddingBottom": 100, - } - } - data={[]} - getItem={[Function]} - getItemCount={[Function]} - initialNumToRender={10} - keyExtractor={[Function]} - maxToRenderPerBatch={2} - onContentSizeChange={[Function]} - onEndReachedThreshold={0.5} - onLayout={[Function]} - onMomentumScrollBegin={[Function]} - onMomentumScrollEnd={[Function]} - onRefresh={[Function]} - onScroll={[Function]} - onScrollBeginDrag={[Function]} - onScrollEndDrag={[Function]} - refreshControl={ - - } - refreshing={false} - removeClippedSubviews={false} - renderItem={[Function]} - scrollEventThrottle={0.0001} - stickyHeaderIndices={[]} - tabLabel="" - testID="notification-menu-scroll-view" - viewabilityConfigCallbackPairs={[]} - > - - - - - - Nothing to see here - - - This is where you can find notifications once there’s activity in your wallet. - - - - - -`; diff --git a/app/components/UI/Notification/List/index.test.tsx b/app/components/UI/Notification/List/index.test.tsx index a2b492c98747..e0ca70c05775 100644 --- a/app/components/UI/Notification/List/index.test.tsx +++ b/app/components/UI/Notification/List/index.test.tsx @@ -1,25 +1,20 @@ import React from 'react'; import { renderHook, act } from '@testing-library/react-hooks'; -import { processNotification } from '@metamask/notification-services-controller/notification-services'; -import { createMockNotificationEthSent } from '@metamask/notification-services-controller/notification-services/mocks'; +import { type INotification } from '@metamask/notification-services-controller/notification-services'; import NotificationsList, { NotificationsListItem, + TEST_IDS, useNotificationOnClick, } from './'; -import NotificationsService from '../../../../util/notifications/services/NotificationService'; import renderWithProvider from '../../../../util/test/renderWithProvider'; -import MOCK_NOTIFICATIONS from '../__mocks__/mock_notifications'; +import { mockNotificationsWithMetaData } from '../__mocks__/mock_notifications'; import { createNavigationProps } from '../../../../util/testUtils'; -import { - hasNotificationComponents, - NotificationComponentState, -} from '../../../../util/notifications/notification-states'; +import { NotificationsViewSelectorsIDs } from '../../../../../e2e/selectors/wallet/NotificationsView.selectors'; +import { NotificationMenuViewSelectorsIDs } from '../../../../../e2e/selectors/Notifications/NotificationMenuView.selectors'; // eslint-disable-next-line import/no-namespace -import * as Actions from '../../../../actions/notification/helpers'; -import { NavigationProp, ParamListBase } from '@react-navigation/native'; +import * as UseNotificationsModule from '../../../../util/notifications/hooks/useNotifications'; const mockNavigation = createNavigationProps({}); - const mockTrackEvent = jest.fn(); const mockCreateEventBuilder = jest.fn(() => ({ addProperties: jest.fn(() => ({ @@ -27,23 +22,6 @@ const mockCreateEventBuilder = jest.fn(() => ({ })), })); -jest.mock('../../../../util/notifications/constants', () => ({ - ...jest.requireActual('../../../../util/notifications/constants'), - isNotificationsFeatureEnabled: () => true, -})); - -jest.mock( - '../../../../util/notifications/services/NotificationService', - () => ({ - ...jest.requireActual( - '../../../../util/notifications/services/NotificationService', - ), - getBadgeCount: jest.fn(), - decrementBadgeCount: jest.fn(), - setBadgeCount: jest.fn(), - }), -); - jest.mock('../../../hooks/useMetrics', () => ({ useMetrics: () => ({ trackEvent: mockTrackEvent, @@ -54,96 +32,115 @@ jest.mock('../../../hooks/useMetrics', () => ({ }, })); -jest.mock('../NotificationMenuItem', () => ({ - NotificationMenuItem: { - Root: ({ children }: { children: React.ReactNode }) => ( -

{children}
- ), - Icon: jest.fn(({ isRead }: { isRead: boolean }) => ( -
{isRead ? 'Read Icon' : 'Unread Icon'}
- )), - Content: jest.fn(() =>
Mocked Content
), - Cta: jest.fn(() => null), - }, -})); - -function arrangeActions() { - const mockMarkNotificationAsRead = jest - .spyOn(Actions, 'markNotificationsAsRead') - .mockResolvedValue(undefined); - - return { - mockMarkNotificationAsRead, - }; -} - -describe('NotificationsList', () => { - it('renders correctly', () => { - const { toJSON } = renderWithProvider( - , - ); - expect(toJSON()).toMatchSnapshot(); - }); - - it('renders empty state', () => { - const { toJSON } = renderWithProvider( - , - ); - expect(toJSON()).toMatchSnapshot(); - }); - - it('derives notificationState correctly based on notification type', () => { - const notification = MOCK_NOTIFICATIONS[2]; - if (!hasNotificationComponents(notification.type)) { - throw new Error('Test Setup Failure - incorrect mock'); - } - - const notifState = NotificationComponentState[notification.type]; - const mockCreateMenuItem = jest.spyOn(notifState, 'createMenuItem'); +describe('NotificationsList States', () => { + const mockNotifSlice = mockNotificationsWithMetaData.slice(0, 1); + const itemIds = mockNotifSlice + .map(({ notification }) => + NotificationMenuViewSelectorsIDs.ITEM(notification.id), + ) + .slice(0, 1); + const statesTests = [ + { + type: 'loading', + elemsRendered: [TEST_IDS.loadingContainer], + elemsNotRendered: [ + NotificationsViewSelectorsIDs.NO_NOTIFICATIONS_CONTAINER, + ...itemIds, + ], + }, + { + type: 'empty', + elemsRendered: [NotificationsViewSelectorsIDs.NO_NOTIFICATIONS_CONTAINER], + elemsNotRendered: [TEST_IDS.loadingContainer, ...itemIds], + }, + { + type: 'data', + elemsRendered: [...itemIds], + elemsNotRendered: [ + TEST_IDS.loadingContainer, + NotificationsViewSelectorsIDs.NO_NOTIFICATIONS_CONTAINER, + ], + }, + ] as const; + + it.each(statesTests)( + 'renders correct list state - $type', + ({ type, elemsRendered, elemsNotRendered }) => { + const getTestState = () => { + if (type === 'loading') { + return { allNotifications: [], loading: true }; + } + if (type === 'empty') { + return { allNotifications: [], loading: false }; + } + if (type === 'data') { + return { + allNotifications: mockNotifSlice.map((n) => n.notification), + loading: false, + }; + } + throw new Error('TEST FAIL - NO TEST STATE FOUND'); + }; + + const { getByTestId, queryByTestId } = renderWithProvider( + , + ); + + elemsRendered.forEach((id) => { + expect(getByTestId(id)).toBeOnTheScreen(); + }); + + elemsNotRendered.forEach((id) => { + expect(queryByTestId(id)).not.toBeOnTheScreen(); + }); + }, + ); +}); - renderWithProvider( +describe('NotificationsListItem', () => { + it('returns null on invalid notification', () => { + const { root } = renderWithProvider( , ); - - expect(mockCreateMenuItem).toHaveBeenCalledWith(MOCK_NOTIFICATIONS[2]); + expect(root).toBeUndefined(); }); + + it.each(mockNotificationsWithMetaData)( + 'renders notification menu item - $type', + ({ notification }) => { + const { getByTestId } = renderWithProvider( + , + ); + + expect( + getByTestId(NotificationMenuViewSelectorsIDs.ITEM(notification.id)), + ).toBeOnTheScreen(); + }, + ); }); describe('useNotificationOnClick', () => { const arrangeMocks = () => { - const { mockMarkNotificationAsRead } = arrangeActions(); - const mockGetBadgeCount = jest - .mocked(NotificationsService.getBadgeCount) - .mockResolvedValue(1); - const mockDecrementBadgeCount = jest.mocked( - NotificationsService.decrementBadgeCount, - ); - const mockSetBadgeConut = jest.mocked(NotificationsService.setBadgeCount); + const mockMarkNotificationAsRead = jest.fn(); + jest + .spyOn(UseNotificationsModule, 'useMarkNotificationAsRead') + .mockReturnValue({ + loading: false, + markNotificationAsRead: mockMarkNotificationAsRead, + }); return { mockMarkNotificationAsRead, - mockGetBadgeCount, - mockDecrementBadgeCount, - mockSetBadgeConut, mockTrackEvent, - mockNavigation: createNavigationProps({}).navigation as jest.MockedObject< - NavigationProp - >, + mockNavigation: createNavigationProps({}).navigation, }; }; @@ -151,28 +148,22 @@ describe('useNotificationOnClick', () => { jest.clearAllMocks(); }); - it('call correct logic, and invoke navigation + events', async () => { - const mocks = arrangeMocks(); - const hook = renderHook(() => - useNotificationOnClick({ navigation: mocks.mockNavigation }), - ); - const notification = processNotification(createMockNotificationEthSent()); - - await act(() => hook.result.current.onNotificationClick(notification)); - - // Assert - Controller Action - expect(mocks.mockMarkNotificationAsRead).toHaveBeenCalledWith([ - expect.objectContaining({ id: notification.id }), - ]); - - // Assert - Page Navigation - expect(mocks.mockNavigation.navigate).toHaveBeenCalled(); - - // Assert - Badge Update - expect(mocks.mockGetBadgeCount).toHaveBeenCalled(); - expect(mocks.mockDecrementBadgeCount).toHaveBeenCalled(); - - // Assert - Event Fired - expect(mocks.mockTrackEvent).toHaveBeenCalled(); - }); + it.each(mockNotificationsWithMetaData)( + 'invokes click callback and attempts navigation for notification - $type', + async ({ notification, hasModal }) => { + const mocks = arrangeMocks(); + const hook = renderHook(() => + useNotificationOnClick({ navigation: mocks.mockNavigation }), + ); + await act(() => hook.result.current.onNotificationClick(notification)); + expect(mocks.mockMarkNotificationAsRead).toHaveBeenCalled(); + expect(mocks.mockTrackEvent).toHaveBeenCalled(); + + if (hasModal) { + expect(mocks.mockNavigation.navigate).toHaveBeenCalled(); + } else { + expect(mocks.mockNavigation.navigate).not.toHaveBeenCalled(); + } + }, + ); }); diff --git a/app/components/UI/Notification/List/index.tsx b/app/components/UI/Notification/List/index.tsx index d9aae57bbc58..9dcda2cc9cc6 100644 --- a/app/components/UI/Notification/List/index.tsx +++ b/app/components/UI/Notification/List/index.tsx @@ -3,7 +3,6 @@ import { ActivityIndicator, FlatList, FlatListProps, View } from 'react-native'; import { NavigationProp, ParamListBase } from '@react-navigation/native'; import { Box } from '@metamask/design-system-react-native'; import { useTailwind } from '@metamask/design-system-twrnc-preset'; -import NotificationsService from '../../../../util/notifications/services/NotificationService'; import { NotificationsViewSelectorsIDs } from '../../../../../e2e/selectors/wallet/NotificationsView.selectors'; import { hasNotificationComponents, @@ -22,11 +21,14 @@ import Empty from '../Empty'; import { NotificationMenuItem } from '../NotificationMenuItem'; import useStyles from './useStyles'; import { NotificationMenuViewSelectorsIDs } from '../../../../../e2e/selectors/Notifications/NotificationMenuView.selectors'; + +export const TEST_IDS = { + loadingContainer: 'notification-list-loading', +}; + interface NotificationsListProps { navigation: NavigationProp; allNotifications: INotification[]; - walletNotifications: INotification[]; - web3Notifications: INotification[]; loading: boolean; } @@ -46,7 +48,7 @@ function Loading() { } = useStyles(); return ( - + ); @@ -91,14 +93,6 @@ export function useNotificationOnClick( }) .build(), ); - - NotificationsService.getBadgeCount().then((count) => { - if (count > 0) { - NotificationsService.decrementBadgeCount(1); - } else { - NotificationsService.setBadgeCount(0); - } - }); }, [createEventBuilder, markNotificationAsRead, trackEvent], ); diff --git a/app/components/UI/Notification/__mocks__/mock_notifications.ts b/app/components/UI/Notification/__mocks__/mock_notifications.ts index 0751210873a5..92588632adff 100644 --- a/app/components/UI/Notification/__mocks__/mock_notifications.ts +++ b/app/components/UI/Notification/__mocks__/mock_notifications.ts @@ -1,9 +1,6 @@ -import { NotificationServicesController } from '@metamask/notification-services-controller'; - -const { - Processors: { processNotification }, - Mocks, -} = NotificationServicesController; +import { processNotification } from '@metamask/notification-services-controller/notification-services'; +// eslint-disable-next-line import/no-namespace +import * as Mocks from '@metamask/notification-services-controller/notification-services/mocks'; export const MOCK_ON_CHAIN_NOTIFICATIONS = [ processNotification(Mocks.createMockNotificationEthSent()), @@ -65,4 +62,100 @@ export const createMockNotificationLidoWithdrawalCompleted = () => export const createMockFeatureAnnouncementRaw = () => processNotification(Mocks.createMockFeatureAnnouncementRaw()); +export const mockNotificationsWithMetaData = [ + { + notification: processNotification(Mocks.createMockNotificationEthSent()), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationEthReceived(), + ), + hasModal: true, + }, + { + notification: processNotification(Mocks.createMockNotificationERC20Sent()), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationERC20Received(), + ), + hasModal: true, + }, + { + notification: processNotification(Mocks.createMockNotificationERC721Sent()), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationERC721Received(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationERC1155Sent(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationERC1155Received(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationMetaMaskSwapsCompleted(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationRocketPoolStakeCompleted(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationRocketPoolUnStakeCompleted(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationLidoStakeCompleted(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationLidoWithdrawalRequested(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationLidoReadyToBeWithdrawn(), + ), + hasModal: true, + }, + { + notification: processNotification( + Mocks.createMockNotificationLidoWithdrawalCompleted(), + ), + hasModal: true, + }, + { + notification: processNotification(Mocks.createMockFeatureAnnouncementRaw()), + hasModal: true, + }, + { + notification: processNotification(Mocks.createMockPlatformNotification()), + hasModal: false, + hasCta: true, + }, +].map((x) => ({ ...x, type: x.notification.type })); + export default MOCK_NOTIFICATIONS; diff --git a/app/components/Views/Notifications/index.test.tsx b/app/components/Views/Notifications/index.test.tsx index 5c65206529f2..49ee5276a278 100644 --- a/app/components/Views/Notifications/index.test.tsx +++ b/app/components/Views/Notifications/index.test.tsx @@ -182,11 +182,9 @@ describe('useNotificationFilters', () => { expect(hook.result.current.allNotifications).toHaveLength(1); }); - it('filters and returns wallet notifications and announcement notifications', () => { + it('returns all notifications', () => { const notifications = [createEthNotif(), createAnnonucementNotif()]; const hook = renderHook(() => useNotificationFilters({ notifications })); expect(hook.result.current.allNotifications).toHaveLength(2); - expect(hook.result.current.walletNotifications).toHaveLength(1); - expect(hook.result.current.announcementNotifications).toHaveLength(1); }); }); diff --git a/app/components/Views/Notifications/index.tsx b/app/components/Views/Notifications/index.tsx index ecce0d9dd2d0..758bc58b0671 100644 --- a/app/components/Views/Notifications/index.tsx +++ b/app/components/Views/Notifications/index.tsx @@ -1,10 +1,7 @@ import React, { useCallback, useMemo } from 'react'; import { View } from 'react-native'; import { useSelector } from 'react-redux'; -import { - TRIGGER_TYPES, - INotification, -} from '@metamask/notification-services-controller/notification-services'; +import { INotification } from '@metamask/notification-services-controller/notification-services'; import { useMetrics } from '../../../components/hooks/useMetrics'; import { NotificationsViewSelectorsIDs } from '../../../../e2e/selectors/wallet/NotificationsView.selectors'; @@ -82,30 +79,7 @@ export function useNotificationFilters(props: { return sortedNotifications; }, [notifications]); - // Wallet notifications - const walletNotifications = useMemo( - () => - (allNotifications ?? []).filter( - (n) => - n.type !== TRIGGER_TYPES.FEATURES_ANNOUNCEMENT && - n.type !== TRIGGER_TYPES.SNAP, - ), - [allNotifications], - ); - - const announcementNotifications = useMemo( - () => - (allNotifications ?? []).filter( - (n) => n.type === TRIGGER_TYPES.FEATURES_ANNOUNCEMENT, - ), - [allNotifications], - ); - - return { - allNotifications, - walletNotifications, - announcementNotifications, - }; + return { allNotifications }; } const NotificationsView = ({ @@ -123,8 +97,7 @@ const NotificationsView = ({ notifications, }); - const { allNotifications, walletNotifications, announcementNotifications } = - useNotificationFilters({ notifications }); + const { allNotifications } = useNotificationFilters({ notifications }); const unreadCount = useMemo( () => allNotifications.filter((n) => !n.isRead).length, @@ -141,8 +114,6 @@ const NotificationsView = ({ {!isLoading && unreadCount > 0 && ( diff --git a/app/util/notifications/notification-states/index.test.tsx b/app/util/notifications/notification-states/index.test.tsx index 934dc448a845..5a9f25e34092 100644 --- a/app/util/notifications/notification-states/index.test.tsx +++ b/app/util/notifications/notification-states/index.test.tsx @@ -1,96 +1,13 @@ -import { - TRIGGER_TYPES, - processNotification, -} from '@metamask/notification-services-controller/notification-services'; -import { - createMockNotificationEthSent, - createMockNotificationEthReceived, - createMockNotificationERC20Sent, - createMockNotificationERC20Received, - createMockNotificationERC721Sent, - createMockNotificationERC721Received, - createMockNotificationERC1155Sent, - createMockNotificationERC1155Received, - createMockNotificationMetaMaskSwapsCompleted, - createMockNotificationRocketPoolStakeCompleted, - createMockNotificationRocketPoolUnStakeCompleted, - createMockNotificationLidoStakeCompleted, - createMockNotificationLidoWithdrawalRequested, - createMockNotificationLidoReadyToBeWithdrawn, - createMockNotificationLidoWithdrawalCompleted, - createMockPlatformNotification, - createMockFeatureAnnouncementRaw, -} from '@metamask/notification-services-controller/notification-services/mocks'; +import { TRIGGER_TYPES } from '@metamask/notification-services-controller/notification-services'; import { hasNotificationComponents, hasNotificationModal, NotificationComponentState, } from '.'; - -const mockAllNotifications = [ - { n: processNotification(createMockNotificationEthSent()), hasModal: true }, - { - n: processNotification(createMockNotificationEthReceived()), - hasModal: true, - }, - { n: processNotification(createMockNotificationERC20Sent()), hasModal: true }, - { - n: processNotification(createMockNotificationERC20Received()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationERC721Sent()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationERC721Received()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationERC1155Sent()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationERC1155Received()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationMetaMaskSwapsCompleted()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationRocketPoolStakeCompleted()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationRocketPoolUnStakeCompleted()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationLidoStakeCompleted()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationLidoWithdrawalRequested()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationLidoReadyToBeWithdrawn()), - hasModal: true, - }, - { - n: processNotification(createMockNotificationLidoWithdrawalCompleted()), - hasModal: true, - }, - { - n: processNotification(createMockFeatureAnnouncementRaw()), - hasModal: true, - }, - { n: processNotification(createMockPlatformNotification()), hasModal: false }, -].map((x) => ({ ...x, type: x.n.type })); +import { mockNotificationsWithMetaData } from '../../../components/UI/Notification/__mocks__/mock_notifications'; describe('hasNotificationComponents()', () => { - it.each(mockAllNotifications)( + it.each(mockNotificationsWithMetaData)( 'returns true for all supported notifications - $type', ({ type }) => { expect(hasNotificationComponents(type)).toBe(true); @@ -105,14 +22,14 @@ describe('hasNotificationComponents()', () => { }); describe('hasNotificationModal()', () => { - it.each(mockAllNotifications.filter((x) => x.hasModal))( + it.each(mockNotificationsWithMetaData.filter((x) => x.hasModal))( 'returns true for all notifications that should render a modal details screen - $type', ({ type }) => { expect(hasNotificationModal(type)).toBe(true); }, ); - it.each(mockAllNotifications.filter((x) => !x.hasModal))( + it.each(mockNotificationsWithMetaData.filter((x) => !x.hasModal))( 'returns false for all notifications that should not render a modal details screen - $type', ({ type }) => { expect(hasNotificationModal(type)).toBe(false); @@ -127,15 +44,15 @@ describe('hasNotificationModal()', () => { }); describe('NotificationComponentState', () => { - it.each(mockAllNotifications)( + it.each(mockNotificationsWithMetaData)( 'computes notification component state for each notification type - $type', - ({ n, hasModal }) => { - if (!hasNotificationComponents(n.type)) { + ({ notification, hasModal }) => { + if (!hasNotificationComponents(notification.type)) { throw new Error('UNSUPPORTED NOTIFICATION'); } - const notificationState = NotificationComponentState[n.type]; - expect(notificationState.createMenuItem(n)).toStrictEqual( + const notificationState = NotificationComponentState[notification.type]; + expect(notificationState.createMenuItem(notification)).toStrictEqual( expect.objectContaining({ title: expect.any(String), description: expect.objectContaining({ @@ -145,7 +62,9 @@ describe('NotificationComponentState', () => { }), ); - expect(notificationState.createModalDetails?.(n)).toStrictEqual( + expect( + notificationState.createModalDetails?.(notification), + ).toStrictEqual( !hasModal ? undefined : expect.objectContaining({ diff --git a/app/util/notifications/notification-states/node-guard.test.ts b/app/util/notifications/notification-states/node-guard.test.ts index edd720617ec2..b252a573ffa7 100644 --- a/app/util/notifications/notification-states/node-guard.test.ts +++ b/app/util/notifications/notification-states/node-guard.test.ts @@ -2,32 +2,41 @@ import { isOfTypeNodeGuard } from './node-guard'; import { INotification, TRIGGER_TYPES, + processNotification, } from '@metamask/notification-services-controller/notification-services'; -import MOCK_NOTIFICATIONS from '../../../components/UI/Notification/__mocks__/mock_notifications'; +import { + createMockNotificationERC1155Received, + createMockNotificationERC721Received, + createMockNotificationEthReceived, +} from '@metamask/notification-services-controller/notification-services/mocks'; describe('isOfTypeNodeGuard', () => { + const erc1155Notification = processNotification( + createMockNotificationERC1155Received(), + ); + const erc721Notification = processNotification( + createMockNotificationERC721Received(), + ); + const otherNotification = processNotification( + createMockNotificationEthReceived(), + ); + const sampleTypes = [ TRIGGER_TYPES.ERC1155_RECEIVED, TRIGGER_TYPES.ERC721_RECEIVED, ]; - const isERC1155Or721ReceivedNotification = isOfTypeNodeGuard(sampleTypes); it('returns true for notifications with matching types', () => { - const erc1155Notification: INotification = MOCK_NOTIFICATIONS[7]; - expect(isERC1155Or721ReceivedNotification(erc1155Notification)).toBe(true); }); it('returns false for notifications with non-matching types', () => { - const otherNotification: INotification = MOCK_NOTIFICATIONS[1]; - expect(isERC1155Or721ReceivedNotification(otherNotification)).toBe(false); }); it('returns undefined for notifications with undefined type', () => { const undefinedTypeNotification: Partial = {}; - expect( isERC1155Or721ReceivedNotification( undefinedTypeNotification as INotification, @@ -37,9 +46,9 @@ describe('isOfTypeNodeGuard', () => { it('narrows types correctly when used in a type guard context', () => { const mixedNotifications: INotification[] = [ - MOCK_NOTIFICATIONS[7], - MOCK_NOTIFICATIONS[1], - MOCK_NOTIFICATIONS[5], + erc1155Notification, + erc721Notification, + otherNotification, {} as INotification, ]; From 97becda9703e0ddb20f0a8a930aefd1edde28498 Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Fri, 14 Nov 2025 17:06:45 +0100 Subject: [PATCH 16/17] fix: prevent concurrency for `createAccount` for Snap account providers cp-7.59.0 (#22719) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** It seems that having concurrent calls to `keyring_createAccount` cause some synchronization issues between Snap's accounts and MetaMask accounts. We're not sure of the real root cause yet for this, but preventing concurrent calls seems to mitigate (or even completely prevent) this kind of issues. ## **Changelog** CHANGELOG entry: null ## **Related issues** N/A ## **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] > Enforces single-concurrency createAccount and shared timeouts for Snap-backed BTC/TRX providers and SOL via providerConfigs. > > - **Multichain Account Service** (`app/core/Engine/controllers/multichain-account-service/multichain-account-service-init.ts`): > - **New Snap provider config**: `maxConcurrency: 1` plus discovery/create timeouts. > - **Providers**: > - Pass config to `new BtcAccountProvider(...)` and `new TrxAccountProvider(...)` via `AccountProviderWrapper`. > - **Service config**: > - Add `providerConfigs` with `[SOL_ACCOUNT_PROVIDER_NAME]` mapped to the same Snap config. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8be27eb77eb194f01d65a5281379ada144c1840a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../multichain-account-service-init.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/app/core/Engine/controllers/multichain-account-service/multichain-account-service-init.ts b/app/core/Engine/controllers/multichain-account-service/multichain-account-service-init.ts index c4ba5bc9df2b..a1add41caed0 100644 --- a/app/core/Engine/controllers/multichain-account-service/multichain-account-service-init.ts +++ b/app/core/Engine/controllers/multichain-account-service/multichain-account-service-init.ts @@ -4,6 +4,7 @@ import { BtcAccountProvider, TrxAccountProvider, AccountProviderWrapper, + SOL_ACCOUNT_PROVIDER_NAME, } from '@metamask/multichain-account-service'; import { ControllerInitFunction } from '../../types'; import Engine from '../../Engine'; @@ -25,11 +26,27 @@ export const multichainAccountServiceInit: ControllerInitFunction< MultichainAccountServiceMessenger, MultichainAccountServiceInitMessenger > = ({ controllerMessenger, initMessenger }) => { + const snapAccountProviderConfig = { + // READ THIS CAREFULLY: + // We using 1 to prevent any concurrent `keyring_createAccount` requests, that make sure + // we prevent any desync between Snap's accounts and Metamask's accounts. + maxConcurrency: 1, + // Re-use the default config for the rest: + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + createAccounts: { + timeoutMs: 3000, + }, + }; + /// BEGIN:ONLY_INCLUDE_IF(bitcoin) // Create Bitcoin provider wrapped for feature flag control const btcProvider = new AccountProviderWrapper( controllerMessenger, - new BtcAccountProvider(controllerMessenger), + new BtcAccountProvider(controllerMessenger, snapAccountProviderConfig), ); /// END:ONLY_INCLUDE_IF @@ -37,7 +54,7 @@ export const multichainAccountServiceInit: ControllerInitFunction< // Create Tron provider wrapped for feature flag control const trxProvider = new AccountProviderWrapper( controllerMessenger, - new TrxAccountProvider(controllerMessenger), + new TrxAccountProvider(controllerMessenger, snapAccountProviderConfig), ); /// END:ONLY_INCLUDE_IF @@ -53,6 +70,9 @@ export const multichainAccountServiceInit: ControllerInitFunction< const controller = new MultichainAccountService({ messenger: controllerMessenger, providers, + providerConfigs: { + [SOL_ACCOUNT_PROVIDER_NAME]: snapAccountProviderConfig, + }, }); // Handle provider feature flags From e700d867c217da42356bde2b42b04053afdb110a Mon Sep 17 00:00:00 2001 From: jvbriones <1674192+jvbriones@users.noreply.github.com> Date: Fri, 14 Nov 2025 18:03:42 +0100 Subject: [PATCH 17/17] chore: refactor AI files - keeping same logic (#22706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** ## **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] > Harden grep and related-files handlers, add tool result preview/error logging in analyzer, and update select-tags prompt guidance. > > - **AI Tools**: > - **`ai-tools/handlers/grep-codebase.ts`**: > - Replace shell escaping with `sanitizeGrepPattern`; validate/reject dangerous input and escape shell metacharacters while allowing grep regex. > - Switch to `grep -Erni`; preserve and report using raw input in messages; add explicit invalid-pattern handling. > - **`ai-tools/handlers/related-files.ts`**: > - Add `escapeGrepRegex` and build safe `-E` grep pattern for importer search; improve filename handling and quoting. > - **Analyzer** (`analysis/analyzer.ts`): > - Log tool result previews and flag errors; keep finalize flow and conservative fallback. > - **Prompt** (`modes/select-tags/prompt.ts`): > - Import `CLAUDE_CONFIG`; adjust guidance to allow selecting all or no tags and to respect `maxIterations`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c060eba17d13555d741c147478d496a335813e6b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../ai-tools/handlers/grep-codebase.ts | 43 +++++++++++++------ .../ai-tools/handlers/related-files.ts | 28 +++++++++--- .../e2e-ai-analyzer/analysis/analyzer.ts | 13 ++++++ .../modes/select-tags/prompt.ts | 6 ++- 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/grep-codebase.ts b/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/grep-codebase.ts index 4f2590d056ac..848c714502ca 100644 --- a/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/grep-codebase.ts +++ b/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/grep-codebase.ts @@ -9,26 +9,38 @@ import { ToolInput } from '../../types'; import { TOOL_LIMITS } from '../../config'; /** - * Escapes shell special characters to prevent command injection + * Validates and sanitizes grep pattern + * Rejects dangerous patterns, allows safe grep regex */ -function escapeShell(str: string): string { - return str.replace(/[`$\\"\n]/g, '\\$&'); +function sanitizeGrepPattern(str: string): string { + // Reject patterns with command substitution or command chaining + if (str.includes('`') || str.includes('$(') || str.includes('\n')) { + throw new Error('Invalid pattern: contains dangerous characters'); + } + + // Escape shell metacharacters that could cause issues + // Escapes: $, ", \ (shell interpretation) + // Preserves: |, *, ., [], {}, +, ? (grep regex) + return str.replace(/[$"\\]/g, '\\$&'); } export function handleGrepCodebase(input: ToolInput, baseDir: string): string { - const pattern = escapeShell(input.pattern as string); - const filePattern = escapeShell((input.file_pattern as string) || '*'); + const rawPattern = input.pattern as string; + const rawFilePattern = (input.file_pattern as string) || '*'; const maxResults = (input.max_results as number) || TOOL_LIMITS.grepMaxResults; - if (!pattern) { + if (!rawPattern) { return 'Error: pattern is required'; } try { - // Use grep with common source code file extensions - // -r: recursive, -n: line numbers, -i: case insensitive, --include: file pattern - const command = `grep -rni --include="${filePattern}" "${pattern}" app/ | head -${maxResults}`; + const pattern = sanitizeGrepPattern(rawPattern); + const filePattern = sanitizeGrepPattern(rawFilePattern); + + // Use grep -E for extended regex (supports |, +, ?, etc.) + // -E: extended regex, -r: recursive, -n: line numbers, -i: case insensitive + const command = `grep -Erni --include="${filePattern}" "${pattern}" app/ | head -${maxResults}`; const result = execSync(command, { encoding: 'utf-8', @@ -37,14 +49,19 @@ export function handleGrepCodebase(input: ToolInput, baseDir: string): string { }); if (!result.trim()) { - return `No matches found for pattern: "${pattern}" in files: ${filePattern}`; + return `No matches found for pattern: "${rawPattern}" in files: ${rawFilePattern}`; } const lines = result.trim().split('\n'); const resultCount = lines.length; - return `Found ${resultCount} matches for "${pattern}" (showing up to ${maxResults}):\n\n${result}`; + return `Found ${resultCount} matches for "${rawPattern}" (showing up to ${maxResults}):\n\n${result}`; } catch (error: unknown) { + // Check if it's a validation error + if (error instanceof Error && error.message.includes('Invalid pattern')) { + return `Invalid pattern: ${error.message}`; + } + // grep returns exit code 1 when no matches found if ( error && @@ -52,10 +69,10 @@ export function handleGrepCodebase(input: ToolInput, baseDir: string): string { 'status' in error && error.status === 1 ) { - return `No matches found for pattern: "${pattern}" in files: ${filePattern}`; + return `No matches found for pattern: "${rawPattern}" in files: ${rawFilePattern}`; } - return `Error searching for pattern "${pattern}": ${ + return `Error searching for pattern "${rawPattern}": ${ error instanceof Error ? error.message : String(error) }`; } diff --git a/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/related-files.ts b/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/related-files.ts index 974739262409..4cfc48dc9ea1 100644 --- a/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/related-files.ts +++ b/e2e/tools/e2e-ai-analyzer/ai-tools/handlers/related-files.ts @@ -17,6 +17,13 @@ function escapeShell(str: string): string { return str.replace(/[`$\\"\n]/g, '\\$&'); } +/** + * Escapes grep regex metacharacters to treat as literal + */ +function escapeGrepRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + export function handleRelatedFiles(input: ToolInput, baseDir: string): string { const filePath = escapeShell(input.file_path as string); const searchType = input.search_type as string; @@ -123,20 +130,31 @@ function findImporters( maxResults: number, ): string { try { - const fileName = escapeShell( + const rawFileName = filePath .replace(/^app\//, '') .replace(/\.(ts|tsx|js|jsx)$/, '') .split('/') - .pop() || '', - ); + .pop() || ''; - if (!fileName) { + if (!rawFileName) { return `Cannot extract filename from ${filePath}`; } + // Escape fileName for grep regex (. * + ? etc. become literals) + const fileNameEscaped = escapeGrepRegex(rawFileName); + + // Then escape for shell + const fileNameSafe = escapeShell(fileNameEscaped); + + // Find files that import this file + // Pattern matches: from './fileName' or from "../fileName" (with space after from) + // Build pattern with properly escaped quotes for shell + // eslint-disable-next-line no-useless-escape + const pattern = `from ['\\\"].*${fileNameSafe}`; // from ['\"].*fileName + const importers = execSync( - `grep -r -l --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" "from.*['"].*${fileName}" app/ 2>/dev/null | grep -v "${filePath}" | head -${maxResults} || true`, + `grep -r -l --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" -E "${pattern}" app/ 2>/dev/null | grep -v "${filePath}" | head -${maxResults} || true`, { encoding: 'utf-8', cwd: baseDir }, ) .trim() diff --git a/e2e/tools/e2e-ai-analyzer/analysis/analyzer.ts b/e2e/tools/e2e-ai-analyzer/analysis/analyzer.ts index 9a404583e0f2..d73b4e3e2402 100644 --- a/e2e/tools/e2e-ai-analyzer/analysis/analyzer.ts +++ b/e2e/tools/e2e-ai-analyzer/analysis/analyzer.ts @@ -140,6 +140,19 @@ export async function analyzeWithAgent( }, ); + // Log tool result summary + const resultPreview = toolResult + .substring(0, 150) + .replace(/\n/g, ' '); + console.log( + ` → ${resultPreview}${toolResult.length > 150 ? '...' : ''}`, + ); + + // Check for actual errors (starts with "Error:") + if (toolResult.startsWith('Error:')) { + console.log(` ⚠️ Tool returned error`); + } + // Handle finalize tool (mode-specific) if (toolUse.name === modeConfig.finalizeToolName) { const analysis = await modeConfig.processAnalysis( diff --git a/e2e/tools/e2e-ai-analyzer/modes/select-tags/prompt.ts b/e2e/tools/e2e-ai-analyzer/modes/select-tags/prompt.ts index 9dd1d260d296..19328da32a01 100644 --- a/e2e/tools/e2e-ai-analyzer/modes/select-tags/prompt.ts +++ b/e2e/tools/e2e-ai-analyzer/modes/select-tags/prompt.ts @@ -10,6 +10,7 @@ import { buildConfidenceGuidanceSection, buildRiskAssessmentSection, } from '../shared/base-system-prompt'; +import { CLAUDE_CONFIG } from '../../config'; /** * Builds the system prompt, i.e. the initial system message @@ -19,10 +20,11 @@ export function buildSystemPrompt(): string { const goal = `GOAL: Analyze code changes and select appropriate test tags to run.`; const guidanceSection = `GUIDANCE: -Use your judgment - selecting 0 tags is acceptable for non-functional changes. +Use your judgment - selecting all tags is acceptable (recommended as conservative approach), as well as selecting none of them. Critical files (marked in file list) typically warrant testing. Use tools to investigate when uncertain. For E2E test infrastructure related changes, consider running the necessary tests or all of them in case the changes are wide-ranging. -Balance thoroughness with efficiency, and be conservative in the assessment of risk and tags to run.`; +Balance thoroughness with efficiency, and be conservative in the assessment of risk and tags to run. +Do not exceed the maximum number of analysis iterations which is ${CLAUDE_CONFIG.maxIterations}.`; const prompt = [ role,