From ab15f172cecca47fa8065de6d194e9deb5821b28 Mon Sep 17 00:00:00 2001 From: ffmcgee <51971598+ffmcgee725@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:00:35 +0100 Subject: [PATCH 01/15] feat: improve SDKConnectV2 error toasts (#26972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The wallet-side error handling in SDKConnectV2 previously had a binary classification for RPC response errors: either EVM user rejection (code `4001`) showing "Approval rejected", or anything else showing "Failed to establish connection." This was misleading when the connection itself was fine but an RPC method returned an error (e.g. a Solana `signMessage` rejection showing as a connection failure). This PR introduces three distinct error categories for post-connection RPC responses: - **User rejection** (`REJECTION_CODES` or rejection message pattern) → "Approval rejected" - **Internal/unexpected error** (JSON-RPC internal range `-32603`, `-32000` to `-32099`) → "Something went wrong" - **RPC method failure** (all other codes) → "Request failed" The `showConnectionError` toast is now reserved exclusively for transport/handshake failures in `connection-registry.ts`, which is its intended meaning. A message-based fallback (`isRejectionMessage`) is also included to handle a known bug in `@metamask/eth-snap-keyring` where `_SnapKeyring_handleRequestRejected` discards the original `4001` code and re-throws a plain `Error`, which `serializeError` then wraps as `-32603`. Without this fallback, Solana `signMessage` rejections would show "Something went wrong" instead of "Approval rejected". An upstream fix request has been filed with the Snaps team (`app/core/SDKConnectV2/docs/snap-keyring-rejection-code-loss.md`). Full error payload (`code` + `message`) is now logged at `warn` level whenever any error toast is shown, improving debuggability. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-1135 ## **Manual testing steps** ```gherkin Feature: SDKConnectV2 error toast categorisation Scenario: user rejects a Solana signMessage request Given a dApp is connected via MetaMask Connect (MWP) And the dApp sends a Solana signMessage request When user taps "Reject" in the confirmation UI Then the "Approval rejected" toast is shown And the "Failed to establish connection" toast is NOT shown Scenario: user rejects an EVM eth_sendTransaction request Given a dApp is connected via MetaMask Connect (MWP) And the dApp sends an eth_sendTransaction request When user taps "Reject" in the confirmation UI Then the "Approval rejected" toast is shown Scenario: an RPC method returns an unexpected error Given a dApp is connected via MetaMask Connect (MWP) And the dApp sends a request that results in an internal wallet error When the wallet returns an error response Then the "Something went wrong" toast is shown And the "Failed to establish connection" toast is NOT shown Scenario: connection handshake fails Given a dApp attempts to connect via MetaMask Connect (MWP) When the WebSocket transport fails to establish Then the "Failed to establish connection" toast is shown ``` ## **Screenshots/Recordings** ### **Before** Solana `signMessage` rejection → "Failed to establish connection" toast ### **After** Solana `signMessage` rejection → "Approval rejected" toast ## **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] > **Medium Risk** > Moderate risk because it changes runtime error-handling branches for all SDKConnectV2 RPC responses and adds new host-app adapter API surface; misclassification could show the wrong toast but does not alter transaction/auth flows. > > **Overview** > Improves SDKConnectV2 post-connection RPC error handling by **splitting the previous generic error toast into three categories**: user rejection (code-based + message-pattern fallback), JSON-RPC internal/server errors, and all other method failures. > > Adds new host adapter APIs `showInternalError` and `showMethodError` (with new i18n strings) and updates the `Connection` response handler to log error details at `warn` level while dispatching the appropriate toast; tests are expanded to cover the new classification (including Solana code `5000` and rejection-message fallback). > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit dad4714a94f29f2269518084fca83f6b831abbad. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../adapters/host-application-adapter.test.ts | 68 ++++++ .../adapters/host-application-adapter.ts | 24 ++ .../SDKConnectV2/services/connection.test.ts | 223 +++++++++++++++++- app/core/SDKConnectV2/services/connection.ts | 53 ++++- app/core/SDKConnectV2/services/logger.ts | 4 + .../types/host-application-adapter.ts | 15 +- locales/languages/en.json | 8 + 7 files changed, 378 insertions(+), 17 deletions(-) diff --git a/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts b/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts index 73ac0aaff65..660e7bdaf63 100644 --- a/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts +++ b/app/core/SDKConnectV2/adapters/host-application-adapter.test.ts @@ -136,6 +136,74 @@ describe('HostApplicationAdapter', () => { }); }); + describe('showInternalError', () => { + it('dispatches an error notification with internal error message', () => { + jest.spyOn(Date, 'now').mockReturnValue(1234567890); + + adapter.showInternalError(); + + expect(showSimpleNotification).toHaveBeenCalledTimes(1); + expect(showSimpleNotification).toHaveBeenCalledWith({ + id: '1234567890', + autodismiss: 5000, + title: 'sdk_connect_v2.show_internal_error.title', + description: 'sdk_connect_v2.show_internal_error.description', + status: 'error', + }); + expect(store.dispatch).toHaveBeenCalledTimes(1); + }); + + it('dispatches an internal error notification with connection info', () => { + adapter.showInternalError( + createMockConnectionInfo('session-123', 'Test DApp'), + ); + + expect(showSimpleNotification).toHaveBeenCalledTimes(1); + expect(showSimpleNotification).toHaveBeenCalledWith({ + id: 'session-123', + autodismiss: 5000, + title: 'sdk_connect_v2.show_internal_error.title', + description: 'sdk_connect_v2.show_internal_error.description', + status: 'error', + }); + expect(store.dispatch).toHaveBeenCalledTimes(1); + }); + }); + + describe('showMethodError', () => { + it('dispatches an error notification with method error message', () => { + jest.spyOn(Date, 'now').mockReturnValue(1234567890); + + adapter.showMethodError(); + + expect(showSimpleNotification).toHaveBeenCalledTimes(1); + expect(showSimpleNotification).toHaveBeenCalledWith({ + id: '1234567890', + autodismiss: 5000, + title: 'sdk_connect_v2.show_method_error.title', + description: 'sdk_connect_v2.show_method_error.description', + status: 'error', + }); + expect(store.dispatch).toHaveBeenCalledTimes(1); + }); + + it('dispatches a method error notification with connection info', () => { + adapter.showMethodError( + createMockConnectionInfo('session-123', 'Test DApp'), + ); + + expect(showSimpleNotification).toHaveBeenCalledTimes(1); + expect(showSimpleNotification).toHaveBeenCalledWith({ + id: 'session-123', + autodismiss: 5000, + title: 'sdk_connect_v2.show_method_error.title', + description: 'sdk_connect_v2.show_method_error.description', + status: 'error', + }); + expect(store.dispatch).toHaveBeenCalledTimes(1); + }); + }); + describe('showConfirmationRejectionError', () => { it('dispatches a rejection error notification with connection info', () => { adapter.showConfirmationRejectionError( diff --git a/app/core/SDKConnectV2/adapters/host-application-adapter.ts b/app/core/SDKConnectV2/adapters/host-application-adapter.ts index 61910b89550..33044969465 100644 --- a/app/core/SDKConnectV2/adapters/host-application-adapter.ts +++ b/app/core/SDKConnectV2/adapters/host-application-adapter.ts @@ -45,6 +45,30 @@ export class HostApplicationAdapter implements IHostApplicationAdapter { ); } + showInternalError(conninfo?: ConnectionInfo): void { + store.dispatch( + showSimpleNotification({ + id: conninfo?.id || Date.now().toString(), + autodismiss: 5000, + title: strings('sdk_connect_v2.show_internal_error.title'), + description: strings('sdk_connect_v2.show_internal_error.description'), + status: 'error', + }), + ); + } + + showMethodError(conninfo?: ConnectionInfo): void { + store.dispatch( + showSimpleNotification({ + id: conninfo?.id || Date.now().toString(), + autodismiss: 5000, + title: strings('sdk_connect_v2.show_method_error.title'), + description: strings('sdk_connect_v2.show_method_error.description'), + status: 'error', + }), + ); + } + showConfirmationRejectionError(conninfo?: ConnectionInfo): void { store.dispatch( showSimpleNotification({ diff --git a/app/core/SDKConnectV2/services/connection.test.ts b/app/core/SDKConnectV2/services/connection.test.ts index 051e75f8ff1..f7f54d0b821 100644 --- a/app/core/SDKConnectV2/services/connection.test.ts +++ b/app/core/SDKConnectV2/services/connection.test.ts @@ -76,6 +76,8 @@ const mockHostApp: jest.Mocked = { showConnectionLoading: jest.fn(), hideConnectionLoading: jest.fn(), showConnectionError: jest.fn(), + showInternalError: jest.fn(), + showMethodError: jest.fn(), showNotFoundError: jest.fn(), showConfirmationRejectionError: jest.fn(), showReturnToApp: jest.fn(), @@ -427,7 +429,7 @@ describe('Connection', () => { ); }); - it('shows error toast when bridge response includes an error', async () => { + it('shows internal error toast for server-range error codes (-32000 to -32099)', async () => { await Connection.create( mockConnectionInfo, mockKeyManager, @@ -441,29 +443,94 @@ describe('Connection', () => { jsonrpc: '2.0', error: { code: -32000, - message: 'User rejected the request', + message: 'Server error', }, }, }; - // 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( + expect(mockHostApp.showInternalError).toHaveBeenCalledTimes(1); + expect(mockHostApp.showInternalError).toHaveBeenCalledWith( mockConnectionInfo, ); + expect(mockHostApp.showMethodError).not.toHaveBeenCalled(); + expect(mockHostApp.showConfirmationRejectionError).not.toHaveBeenCalled(); 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 confirmation rejection error toast when bridge response includes user rejected request error', async () => { + it('shows internal error toast for JSON-RPC internal error code (-32603)', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const errorResponsePayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: -32603, + message: 'Internal error', + }, + }, + }; + + onBridgeResponseCallback(errorResponsePayload); + + expect(mockHostApp.showInternalError).toHaveBeenCalledTimes(1); + expect(mockHostApp.showInternalError).toHaveBeenCalledWith( + mockConnectionInfo, + ); + expect(mockHostApp.showMethodError).not.toHaveBeenCalled(); + expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled(); + + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + }); + + it('shows method error toast for non-rejection, non-internal error codes', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const errorResponsePayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: 53, + reason: 'Invalid URL', + }, + }, + }; + + onBridgeResponseCallback(errorResponsePayload); + + expect(mockHostApp.showMethodError).toHaveBeenCalledTimes(1); + expect(mockHostApp.showMethodError).toHaveBeenCalledWith( + mockConnectionInfo, + ); + expect(mockHostApp.showInternalError).not.toHaveBeenCalled(); + expect(mockHostApp.showConfirmationRejectionError).not.toHaveBeenCalled(); + expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled(); + + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledWith( + errorResponsePayload, + ); + }); + + it('shows confirmation rejection error toast for EVM user rejected request (4001)', async () => { await Connection.create( mockConnectionInfo, mockKeyManager, @@ -482,26 +549,158 @@ describe('Connection', () => { }, }; - // Simulate the RPCBridgeAdapter emitting a user rejected error response onBridgeResponseCallback(userRejectedErrorResponsePayload); - // Should show confirmation rejection error toast, not generic error toast expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledTimes( 1, ); expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledWith( mockConnectionInfo, ); - expect(mockHostApp.showConnectionError).not.toHaveBeenCalled(); + expect(mockHostApp.showMethodError).not.toHaveBeenCalled(); + expect(mockHostApp.showInternalError).not.toHaveBeenCalled(); expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled(); - // And still send the error response to the client expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledWith( userRejectedErrorResponsePayload, ); }); + it('shows confirmation rejection error toast for Solana user rejection (5000)', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const solanaRejectionPayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: 5000, + message: 'User rejected the request', + }, + }, + }; + + onBridgeResponseCallback(solanaRejectionPayload); + + expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledTimes( + 1, + ); + expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledWith( + mockConnectionInfo, + ); + expect(mockHostApp.showMethodError).not.toHaveBeenCalled(); + expect(mockHostApp.showInternalError).not.toHaveBeenCalled(); + expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled(); + + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + }); + + it('shows rejection toast when SnapKeyring strips the code but message contains rejection text', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const snapKeyringRejectionPayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: -32603, + message: 'Request rejected by user or snap.', + }, + }, + }; + + onBridgeResponseCallback(snapKeyringRejectionPayload); + + expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledTimes( + 1, + ); + expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledWith( + mockConnectionInfo, + ); + expect(mockHostApp.showInternalError).not.toHaveBeenCalled(); + expect(mockHostApp.showMethodError).not.toHaveBeenCalled(); + expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled(); + + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + }); + + it('shows rejection toast when error message contains "User rejected" regardless of code', async () => { + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const wrappedRejectionPayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: -32603, + message: 'User rejected the request', + }, + }, + }; + + onBridgeResponseCallback(wrappedRejectionPayload); + + expect(mockHostApp.showConfirmationRejectionError).toHaveBeenCalledTimes( + 1, + ); + expect(mockHostApp.showInternalError).not.toHaveBeenCalled(); + expect(mockHostApp.showMethodError).not.toHaveBeenCalled(); + + expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1); + }); + + it('logs error payload at warn level when error toast is shown', async () => { + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(); + + await Connection.create( + mockConnectionInfo, + mockKeyManager, + RELAY_URL, + mockHostApp, + ); + + const errorResponsePayload = { + data: { + id: 1, + jsonrpc: '2.0', + error: { + code: 53, + message: 'Invalid URL', + }, + }, + }; + + onBridgeResponseCallback(errorResponsePayload); + + expect(warnSpy).toHaveBeenCalledWith( + '[SDKConnectV2]', + 'RPC error response', + { + connectionId: mockConnectionInfo.id, + code: 53, + message: 'Invalid URL', + }, + ); + + warnSpy.mockRestore(); + }); + it('shows success toast for successful response with result', async () => { await Connection.create( mockConnectionInfo, diff --git a/app/core/SDKConnectV2/services/connection.ts b/app/core/SDKConnectV2/services/connection.ts index 8d99cdf9a38..66f1f4bf151 100644 --- a/app/core/SDKConnectV2/services/connection.ts +++ b/app/core/SDKConnectV2/services/connection.ts @@ -17,6 +17,40 @@ import { errorCodes, providerErrors } from '@metamask/rpc-errors'; import Engine from '../../Engine'; import NavigationService from '../../NavigationService'; +/** + * Known user-rejection error codes across ecosystems. + * - 4001: EVM standard (EIP-1193) + * - 5000: Solana wallet standard (user rejected) + */ +const REJECTION_CODES: ReadonlySet = new Set([ + errorCodes.provider.userRejectedRequest, // 4001 + 5000, // Solana wallet-standard user rejection +]); + +/** + * Message patterns that indicate a user rejection even when the original + * error code has been lost. The SnapKeyring strips the 4001 code from + * approval rejections and re-throws a plain Error, which serializeError + * then wraps with the fallback code -32603. We match on the message to + * recover the user-rejection intent. + */ +const REJECTION_MESSAGE_PATTERNS: readonly string[] = [ + 'request rejected by user or snap', + 'user rejected', +]; + +const isRejectionMessage = (message: unknown): boolean => { + if (typeof message !== 'string') return false; + const lower = message.toLowerCase(); + return REJECTION_MESSAGE_PATTERNS.some((pattern) => lower.includes(pattern)); +}; + +/** + * Standard JSON-RPC internal error range: -32603, and server errors -32000 to -32099. + */ +const isInternalError = (code: number): boolean => + code === errorCodes.rpc.internal || (code >= -32099 && code <= -32000); + /** * Connection is a live, runtime representation of a dApp connection. */ @@ -100,12 +134,23 @@ export class Connection { 'error' in responseData && responseData.error !== undefined; if (isError) { - if ( - responseData.error.code === errorCodes.provider.userRejectedRequest - ) { + const errCode = responseData.error.code as number; + const errMessage = + (responseData.error as Record).message ?? + (responseData.error as Record).reason; + + logger.warn('RPC error response', { + connectionId: this.id, + code: errCode, + message: errMessage, + }); + + if (REJECTION_CODES.has(errCode) || isRejectionMessage(errMessage)) { this.hostApp.showConfirmationRejectionError(this.info); + } else if (isInternalError(errCode)) { + this.hostApp.showInternalError(this.info); } else { - this.hostApp.showConnectionError(this.info); + this.hostApp.showMethodError(this.info); } } else { this.hostApp.showReturnToApp(this.info); diff --git a/app/core/SDKConnectV2/services/logger.ts b/app/core/SDKConnectV2/services/logger.ts index dde10cd678e..31c0dc1a42f 100644 --- a/app/core/SDKConnectV2/services/logger.ts +++ b/app/core/SDKConnectV2/services/logger.ts @@ -26,6 +26,10 @@ export default { console.debug(prettify(prefix, ...args)); } }, + warn: (...args: unknown[]) => { + // eslint-disable-next-line no-console + console.warn(prefix, ...args); + }, error: (...args: unknown[]) => { // eslint-disable-next-line no-console console.error(prefix, ...args); diff --git a/app/core/SDKConnectV2/types/host-application-adapter.ts b/app/core/SDKConnectV2/types/host-application-adapter.ts index 8686c2f3794..444d61daa83 100644 --- a/app/core/SDKConnectV2/types/host-application-adapter.ts +++ b/app/core/SDKConnectV2/types/host-application-adapter.ts @@ -21,10 +21,23 @@ export interface IHostApplicationAdapter { hideConnectionLoading(conninfo: ConnectionInfo): void; /** - * Displays a global, non-interactive error modal. + * Displays a connection-level error toast. Use only when the MWP + * session/handshake itself fails, not for RPC method errors. */ showConnectionError(conninfo?: ConnectionInfo): void; + /** + * Displays a toast for an unexpected internal error (e.g. URL parsing + * failure, uncategorized error codes). + */ + showInternalError(conninfo?: ConnectionInfo): void; + + /** + * Displays a toast for an RPC method error that is not a user rejection + * (e.g. invalid params, method not found). + */ + showMethodError(conninfo?: ConnectionInfo): void; + /** * Displays a global, non-interactive not found modal. */ diff --git a/locales/languages/en.json b/locales/languages/en.json index 5b1e1b3ff44..daa57076d80 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -7819,6 +7819,14 @@ "show_not_found": { "title": "Connection Not Found", "description": "Please establish a new connection from the app to continue." + }, + "show_internal_error": { + "title": "Something went wrong", + "description": "An unexpected error occurred. Please try again." + }, + "show_method_error": { + "title": "Request failed", + "description": "The request could not be completed. Please try again." } }, "network_connection_banner": { From 8829370f7b19b8f878fd2ed3b368cc23b24b93fc Mon Sep 17 00:00:00 2001 From: Michal Szorad Date: Wed, 4 Mar 2026 13:59:32 +0100 Subject: [PATCH 02/15] feat(perps): inline deposit flow in pay-with token filter (#26543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR moves the perps “Add funds” (deposit) flow into the pay-with token filter so the Pay With modal can show the perps balance row and “Add funds” without receiving a deposit callback from the parent. ## **Changelog** CHANGELOG entry: Inlined perps “Add funds” flow in the pay-with token filter so the Pay With modal no longer depends on a deposit callback from the parent. ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2528 ## **Manual testing steps** ```gherkin Feature: Perps pay-with and inline deposit Scenario: user opens Pay With for a perps deposit-and-order transaction Given a perps deposit-and-order transaction is in the approval flow and the user opens the Pay With modal When the modal is shown Then a “Perps balance” row appears at the top with the perps balance and an “Add funds” button And tapping the row selects perps balance as the payment token And tapping “Add funds” navigates to the perps flow and starts the deposit-with-confirmation flow ``` ## **Screenshots/Recordings** ### **Before** ### **After** simulator_screenshot_77596920-937B-44BE-AFB1-816160B96C76 ## **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] > **Medium Risk** > Medium risk because it changes Pay With list item shape/typing and wires navigation + `depositWithConfirmation()` from within the filter, which could impact payment token selection and deposit initiation for `perpsDepositAndOrder` confirmations. > > **Overview** > For `perpsDepositAndOrder` transactions, replaces the synthetic perps-balance `AssetType` entry with a `HighlightedItem` (outside the asset list) that displays the perps balance and includes an inline **“Add funds”** button. > > The highlighted row now **self-initiates the deposit flow** by calling `navigateToConfirmation({ stack: Routes.PERPS.ROOT })` and `depositWithConfirmation()`, and selecting the row triggers `onPerpsPaymentTokenChange(null)`. > > Tests are updated accordingly, including loosening `util/networks` mocks to spread `jest.requireActual` to satisfy transitive dependencies. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c01f0ad83aa3b86da7fea7aab15b582d8901baba. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --------- Co-authored-by: Cursor --- .../PerpsOrderView/PerpsOrderView.test.tsx | 3 +- .../Views/PerpsOrderView/PerpsPayRow.test.tsx | 1 + .../hooks/usePerpsBalanceTokenFilter.test.ts | 151 ++++++++++++++---- .../Perps/hooks/usePerpsBalanceTokenFilter.ts | 89 +++++++---- 4 files changed, 180 insertions(+), 64 deletions(-) diff --git a/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.test.tsx b/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.test.tsx index 203ea947e40..81b8e3a67c1 100644 --- a/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.test.tsx +++ b/app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.test.tsx @@ -594,8 +594,9 @@ jest.mock('../../components/PerpsNotificationTooltip', () => { }; }); -// Mock network utils - these are external utilities that should be mocked +// Mock network utils - spread actual to satisfy transitive deps (e.g. stakeableTokens.getDecimalChainId), override only what we need jest.mock('../../../../../util/networks', () => ({ + ...jest.requireActual('../../../../../util/networks'), getDefaultNetworkByChainId: jest.fn(() => ({ name: 'Arbitrum' })), getNetworkImageSource: jest.fn(() => ({ uri: 'network-icon' })), })); diff --git a/app/components/UI/Perps/Views/PerpsOrderView/PerpsPayRow.test.tsx b/app/components/UI/Perps/Views/PerpsOrderView/PerpsPayRow.test.tsx index 6a720aad3e1..f9ca8342d61 100644 --- a/app/components/UI/Perps/Views/PerpsOrderView/PerpsPayRow.test.tsx +++ b/app/components/UI/Perps/Views/PerpsOrderView/PerpsPayRow.test.tsx @@ -59,6 +59,7 @@ jest.mock('../../hooks/usePerpsEventTracking'); jest.mock('../../../../../util/address'); jest.mock('../../../../Base/TokenIcon', () => jest.fn(() => null)); jest.mock('../../../../../util/networks', () => ({ + ...jest.requireActual('../../../../../util/networks'), getNetworkImageSource: jest.fn(() => ({ uri: 'network-icon.png' })), })); diff --git a/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.test.ts b/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.test.ts index 3734ea343f7..3f54bbe8c7c 100644 --- a/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.test.ts +++ b/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.test.ts @@ -4,9 +4,14 @@ import { TransactionType } from '@metamask/transaction-controller'; import { usePerpsBalanceTokenFilter } from './usePerpsBalanceTokenFilter'; import { useTransactionMetadataRequest } from '../../../Views/confirmations/hooks/transactions/useTransactionMetadataRequest'; import { useIsPerpsBalanceSelected } from './useIsPerpsBalanceSelected'; -import { PERPS_CONSTANTS } from '@metamask/perps-controller'; -import { PERPS_BALANCE_PLACEHOLDER_ADDRESS } from '../constants/perpsConfig'; -import type { AssetType } from '../../../Views/confirmations/types/token'; +import { + type AssetType, + type TokenListItem, + isHighlightedItemOutsideAssetList, +} from '../../../Views/confirmations/types/token'; +import { usePerpsTrading } from './usePerpsTrading'; +import { useConfirmNavigation } from '../../../Views/confirmations/hooks/useConfirmNavigation'; +import { usePerpsPaymentToken } from './usePerpsPaymentToken'; jest.mock('../../../../../locales/i18n', () => ({ strings: jest.fn((key: string) => key), @@ -16,6 +21,11 @@ jest.mock( '../../../Views/confirmations/hooks/transactions/useTransactionMetadataRequest', ); jest.mock('./useIsPerpsBalanceSelected'); +jest.mock('./usePerpsTrading'); +jest.mock('../../../Views/confirmations/hooks/useConfirmNavigation', () => ({ + useConfirmNavigation: jest.fn(), +})); +jest.mock('./usePerpsPaymentToken'); jest.mock('react-redux', () => ({ useSelector: jest.fn(), })); @@ -40,9 +50,21 @@ const mockUseIsPerpsBalanceSelected = typeof useIsPerpsBalanceSelected >; const mockUseSelector = useSelector as jest.MockedFunction; +const mockUsePerpsTrading = usePerpsTrading as jest.MockedFunction< + typeof usePerpsTrading +>; +const mockUseConfirmNavigation = useConfirmNavigation as jest.MockedFunction< + typeof useConfirmNavigation +>; +const mockUsePerpsPaymentToken = usePerpsPaymentToken as jest.MockedFunction< + typeof usePerpsPaymentToken +>; describe('usePerpsBalanceTokenFilter', () => { const chainId = '0xa4b1'; + const mockDepositWithConfirmation = jest.fn().mockResolvedValue(undefined); + const mockNavigateToConfirmation = jest.fn(); + const mockOnPerpsPaymentTokenChange = jest.fn(); beforeEach(() => { jest.clearAllMocks(); @@ -57,6 +79,15 @@ describe('usePerpsBalanceTokenFilter', () => { } return undefined; }); + mockUsePerpsTrading.mockReturnValue({ + depositWithConfirmation: mockDepositWithConfirmation, + } as unknown as ReturnType); + mockUseConfirmNavigation.mockReturnValue({ + navigateToConfirmation: mockNavigateToConfirmation, + } as unknown as ReturnType); + mockUsePerpsPaymentToken.mockReturnValue({ + onPaymentTokenChange: mockOnPerpsPaymentTokenChange, + } as unknown as ReturnType); }); describe('when transaction is not perpsDepositAndOrder', () => { @@ -76,11 +107,11 @@ describe('usePerpsBalanceTokenFilter', () => { const { result } = renderHook(() => usePerpsBalanceTokenFilter()); const filter = result.current; - const output = filter(inputTokens); + const output: TokenListItem[] = filter(inputTokens); expect(output).toBe(inputTokens); expect(output).toHaveLength(1); - expect(output[0].address).toBe('0xabc'); + expect((output[0] as AssetType).address).toBe('0xabc'); }); it('returns tokens unchanged when transaction meta is undefined', () => { @@ -103,7 +134,7 @@ describe('usePerpsBalanceTokenFilter', () => { } as ReturnType); }); - it('prepends perps balance token with correct shape', () => { + it('prepends highlighted row with perps balance and Add funds button', () => { mockUseSelector.mockReturnValue({ availableBalance: '2000.50', }); @@ -123,23 +154,26 @@ describe('usePerpsBalanceTokenFilter', () => { const output = result.current(inputTokens); expect(output).toHaveLength(2); - const perpsToken = output[0]; - expect(perpsToken.address).toBe(PERPS_BALANCE_PLACEHOLDER_ADDRESS); - expect(perpsToken.tokenId).toBe(PERPS_BALANCE_PLACEHOLDER_ADDRESS); - expect(perpsToken.name).toBe('perps.adjust_margin.perps_balance'); - expect(perpsToken.symbol).toBe('USD'); - expect(perpsToken.balance).toBe('2000.50'); - expect(perpsToken.balanceInSelectedCurrency).toBe('$2000.50'); - expect(perpsToken.decimals).toBe(2); - expect(perpsToken.isETH).toBe(false); - expect(perpsToken.isNative).toBe(false); - expect(perpsToken.isSelected).toBe(true); - expect(perpsToken.description).toBe( - PERPS_CONSTANTS.PerpsBalanceTokenDescription, - ); + expect(isHighlightedItemOutsideAssetList(output[0])).toBe(true); + const highlightedAction = output[0]; + if (isHighlightedItemOutsideAssetList(highlightedAction)) { + expect(highlightedAction.position).toBe('outside_of_asset_list'); + expect(highlightedAction.name).toBe( + 'perps.adjust_margin.perps_balance', + ); + expect(highlightedAction.name_description).toBe('$2000.50'); + expect(highlightedAction.fiat).toBe('$2000.50'); + expect(highlightedAction.fiat_description).toBe('$2000.50'); + expect(highlightedAction.isSelected).toBe(true); + expect(highlightedAction.actions).toHaveLength(1); + expect(highlightedAction.actions?.[0]?.buttonLabel).toBe( + 'perps.add_funds', + ); + } + expect((output[1] as AssetType).address).toBe('0xusdc'); }); - it('uses availableBalance from perps account', () => { + it('uses availableBalance from perps account in highlighted row', () => { mockUseSelector.mockReturnValue({ availableBalance: '999.99', }); @@ -148,8 +182,12 @@ describe('usePerpsBalanceTokenFilter', () => { const { result } = renderHook(() => usePerpsBalanceTokenFilter()); const output = result.current(inputTokens); - expect(output[0].balance).toBe('999.99'); - expect(output[0].balanceInSelectedCurrency).toBe('$999.99'); + expect(output).toHaveLength(1); + expect(isHighlightedItemOutsideAssetList(output[0])).toBe(true); + if (isHighlightedItemOutsideAssetList(output[0])) { + expect(output[0].name_description).toBe('$999.99'); + expect(output[0].fiat).toBe('$999.99'); + } }); it('uses zero balance when perps account is null', () => { @@ -164,8 +202,12 @@ describe('usePerpsBalanceTokenFilter', () => { const { result } = renderHook(() => usePerpsBalanceTokenFilter()); const output = result.current(inputTokens); - expect(output[0].balance).toBe('0'); - expect(output[0].balanceInSelectedCurrency).toBe('$0.00'); + expect(output).toHaveLength(1); + expect(isHighlightedItemOutsideAssetList(output[0])).toBe(true); + if (isHighlightedItemOutsideAssetList(output[0])) { + expect(output[0].name_description).toBe('$0.00'); + expect(output[0].fiat).toBe('$0.00'); + } }); it('clears isSelected on other tokens when perps balance is selected', () => { @@ -195,8 +237,8 @@ describe('usePerpsBalanceTokenFilter', () => { const { result } = renderHook(() => usePerpsBalanceTokenFilter()); const output = result.current(inputTokens); - expect(output[1].isSelected).toBe(false); - expect(output[2].isSelected).toBe(false); + expect((output[1] as AssetType).isSelected).toBe(false); + expect((output[2] as AssetType).isSelected).toBe(false); }); it('keeps token isSelected when perps balance is not selected', () => { @@ -220,7 +262,7 @@ describe('usePerpsBalanceTokenFilter', () => { const { result } = renderHook(() => usePerpsBalanceTokenFilter()); const output = result.current(inputTokens); - expect(output[1].isSelected).toBe(true); + expect((output[1] as AssetType).isSelected).toBe(true); }); it('filters to only allowlisted tokens when allowlist is set', () => { @@ -253,9 +295,56 @@ describe('usePerpsBalanceTokenFilter', () => { const output = result.current(inputTokens); expect(output).toHaveLength(2); - expect(output[0].address).toBe(PERPS_BALANCE_PLACEHOLDER_ADDRESS); - expect(output[1].address).toBe('0xusdc'); - expect(output[1].symbol).toBe('USDC'); + expect(isHighlightedItemOutsideAssetList(output[0])).toBe(true); + expect((output[1] as AssetType).address).toBe('0xusdc'); + expect((output[1] as AssetType).symbol).toBe('USDC'); + }); + + it('calls navigateToConfirmation and depositWithConfirmation when Add funds is pressed', () => { + mockUseSelector.mockReturnValue({ + availableBalance: '500.00', + }); + const inputTokens: AssetType[] = [ + { + address: '0xusdc', + chainId, + symbol: 'USDC', + name: 'USD Coin', + balance: '100', + } as AssetType, + ]; + + const { result } = renderHook(() => usePerpsBalanceTokenFilter()); + const output = result.current(inputTokens); + + expect(output).toHaveLength(2); + const highlightedAction = output[0]; + expect(isHighlightedItemOutsideAssetList(highlightedAction)).toBe(true); + if (isHighlightedItemOutsideAssetList(highlightedAction)) { + highlightedAction.actions?.[0]?.onPress(); + expect(mockNavigateToConfirmation).toHaveBeenCalledWith({ + stack: expect.any(String), + }); + expect(mockDepositWithConfirmation).toHaveBeenCalledTimes(1); + } + }); + + it('calls onPerpsPaymentTokenChange with null when row action is invoked', () => { + mockUseSelector.mockReturnValue({ + availableBalance: '100.00', + }); + const inputTokens: AssetType[] = []; + + const { result } = renderHook(() => usePerpsBalanceTokenFilter()); + const output = result.current(inputTokens); + + expect(output).toHaveLength(1); + const highlightedAction = output[0]; + expect(isHighlightedItemOutsideAssetList(highlightedAction)).toBe(true); + if (isHighlightedItemOutsideAssetList(highlightedAction)) { + highlightedAction.action(); + expect(mockOnPerpsPaymentTokenChange).toHaveBeenCalledWith(null); + } }); }); }); diff --git a/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.ts b/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.ts index 5d40f1a55fd..cfb6515e515 100644 --- a/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.ts +++ b/app/components/UI/Perps/hooks/usePerpsBalanceTokenFilter.ts @@ -1,22 +1,25 @@ import { TransactionType } from '@metamask/transaction-controller'; import { BigNumber } from 'bignumber.js'; +import perpsPayTokenIcon from 'images/perps-pay-token-icon.png'; import { useCallback } from 'react'; import { Image } from 'react-native'; import { useSelector } from 'react-redux'; import { strings } from '../../../../../locales/i18n'; import useFiatFormatter from '../../../UI/SimulationDetails/FiatDisplay/useFiatFormatter'; import { useTransactionMetadataRequest } from '../../../Views/confirmations/hooks/transactions/useTransactionMetadataRequest'; -import { AssetType } from '../../../Views/confirmations/types/token'; -import { hasTransactionType } from '../../../Views/confirmations/utils/transaction'; -import perpsPayTokenIcon from 'images/perps-pay-token-icon.png'; -import { PERPS_CONSTANTS } from '@metamask/perps-controller'; import { - PERPS_BALANCE_CHAIN_ID, - PERPS_BALANCE_PLACEHOLDER_ADDRESS, -} from '../constants/perpsConfig'; + AssetType, + HighlightedItem, + type TokenListItem, +} from '../../../Views/confirmations/types/token'; +import { hasTransactionType } from '../../../Views/confirmations/utils/transaction'; import { selectPerpsPayWithAnyTokenAllowlistAssets } from '../selectors/featureFlags'; import { selectPerpsAccountState } from '../selectors/perpsController'; import { useIsPerpsBalanceSelected } from './useIsPerpsBalanceSelected'; +import { usePerpsPaymentToken } from './usePerpsPaymentToken'; +import Routes from '../../../../constants/navigation/Routes'; +import { usePerpsTrading } from './usePerpsTrading'; +import { useConfirmNavigation } from '../../../Views/confirmations/hooks/useConfirmNavigation'; /** URI for the perps balance token icon, shared with PerpsPayRow and pay-with modal. */ const resolvedPerpsIcon = Image.resolveAssetSource(perpsPayTokenIcon); @@ -32,7 +35,7 @@ export const PERPS_BALANCE_ICON_URI = resolvedPerpsIcon?.uri ?? ''; */ export function usePerpsBalanceTokenFilter(): ( tokens: AssetType[], -) => AssetType[] { +) => TokenListItem[] { const transactionMeta = useTransactionMetadataRequest(); const isPerpsBalanceSelected = useIsPerpsBalanceSelected(); const perpsAccount = useSelector(selectPerpsAccountState); @@ -41,8 +44,25 @@ export function usePerpsBalanceTokenFilter(): ( ); const formatFiat = useFiatFormatter({ currency: 'usd' }); + const { depositWithConfirmation } = usePerpsTrading(); + const { navigateToConfirmation } = useConfirmNavigation(); + + const isPerpsDepositAndOrder = hasTransactionType(transactionMeta, [ + TransactionType.perpsDepositAndOrder, + ]); + + const handlePerpsDepositPress = useCallback(() => { + navigateToConfirmation({ stack: Routes.PERPS.ROOT }); + depositWithConfirmation().catch(() => { + // Deposit flow handles errors (e.g. user rejection). + }); + }, [navigateToConfirmation, depositWithConfirmation]); + + const { onPaymentTokenChange: onPerpsPaymentTokenChange } = + usePerpsPaymentToken(); + const filterAllowedTokens = useCallback( - (tokens: AssetType[]): AssetType[] => { + (tokens: AssetType[]): TokenListItem[] => { if ( !hasTransactionType(transactionMeta, [ TransactionType.perpsDepositAndOrder, @@ -51,8 +71,6 @@ export function usePerpsBalanceTokenFilter(): ( return tokens; } - const chainId = PERPS_BALANCE_CHAIN_ID; - const availableBalance = perpsAccount?.availableBalance || '0'; const balanceInSelectedCurrency = formatFiat( new BigNumber(availableBalance), @@ -60,23 +78,6 @@ export function usePerpsBalanceTokenFilter(): ( const perpsBalanceName = strings('perps.adjust_margin.perps_balance'); - const perpsBalanceToken: AssetType = { - address: PERPS_BALANCE_PLACEHOLDER_ADDRESS, - chainId, - tokenId: PERPS_BALANCE_PLACEHOLDER_ADDRESS, - name: perpsBalanceName, - symbol: PERPS_CONSTANTS.PerpsBalanceTokenSymbol, - balance: availableBalance, - balanceInSelectedCurrency, - image: PERPS_BALANCE_ICON_URI, - logo: PERPS_BALANCE_ICON_URI, - decimals: 2, - isETH: false, - isNative: false, - isSelected: isPerpsBalanceSelected, - description: PERPS_CONSTANTS.PerpsBalanceTokenDescription, - }; - let mappedTokens = tokens.map((token) => ({ ...token, isSelected: @@ -91,14 +92,38 @@ export function usePerpsBalanceTokenFilter(): ( }); } - return [perpsBalanceToken, ...mappedTokens]; + if (!isPerpsDepositAndOrder) { + return mappedTokens; + } + + const highlightedAction: HighlightedItem = { + position: 'outside_of_asset_list', + icon: PERPS_BALANCE_ICON_URI, + name: perpsBalanceName, + name_description: balanceInSelectedCurrency, + fiat: balanceInSelectedCurrency, + fiat_description: balanceInSelectedCurrency, + isSelected: isPerpsBalanceSelected, + action: () => onPerpsPaymentTokenChange(null), + actions: [ + { + buttonLabel: strings('perps.add_funds'), + onPress: handlePerpsDepositPress, + }, + ], + }; + + return [highlightedAction, ...mappedTokens]; }, [ - transactionMeta, - isPerpsBalanceSelected, - perpsAccount?.availableBalance, + handlePerpsDepositPress, + isPerpsDepositAndOrder, allowListAssets, formatFiat, + onPerpsPaymentTokenChange, + isPerpsBalanceSelected, + perpsAccount?.availableBalance, + transactionMeta, ], ); From 7c541c06993ee89599816341502a6bdf1b35075d Mon Sep 17 00:00:00 2001 From: abretonc7s <107169956+abretonc7s@users.noreply.github.com> Date: Wed, 4 Mar 2026 21:39:49 +0800 Subject: [PATCH 03/15] fix(perps): remove duplicate AppState listener causing reconnection race cp-7.67.2 (#26982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** After long backgrounds (5+ min), Perps failed to reload positions, 24h price change was missing, and closing positions threw "BTC is not tradeable asset". After closing a position and backgrounding for >20s, the closed position would reappear on foreground. **Root causes:** 1. **Duplicate AppState listener race** — `usePerpsConnectionLifecycle` hook and `PerpsConnectionManager.setupStateMonitoring()` both fired on foreground. The `force` reconnect path cancelled the hook's in-flight `connect()` and ran a competing `performReconnection()`, cleaning up prewarm subscriptions mid-flight and leaving positions/prices without data. 2. **Stale cache after grace-period disconnect** — `performActualDisconnection()` (fires after 20s grace period) did not clear stream channel caches or reset subscriber state (`hasReceivedFirstUpdate`). On next foreground, the old closed position was served from cache until the throttle window passed. 3. **`isPreloading` not reset on disconnect** — If a preload was in-flight when the grace period fired, `isPreloading` stayed `true`. The next `connect()` would silently skip all subscription prewarm, leaving positions and prices with no data. **Fixes:** - Remove the manager-level AppState listener (hook already handles foreground recovery) - Fix `isInternetReachable` null handling in NetInfo listener (`null` treated as offline was blocking network-restore reconnects) - Add `isPreloading` concurrency guard to `preloadSubscriptions()` to prevent concurrent calls racing on reconnect - Call `clearCache()` on all stream channels in `performActualDisconnection()` — wipes stale positions, resets `hasReceivedFirstUpdate`, puts UI into loading state on next foreground - Reset `isPreloading = false` in `performActualDisconnection()` alongside `hasPreloaded` The NetInfo listener from #26780 is kept — it handles the distinct offline→online restore scenario. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: TAT-2598 ## **Manual testing steps** ```gherkin Feature: Perps reconnection after long background Scenario: user returns to Perps after long background Given the user has open Perps positions And the app has been backgrounded for 5+ minutes When user foregrounds the app Then positions load correctly And 24h price change is displayed And closing a position succeeds without "not tradeable asset" error Scenario: closed position does not reappear after background Given the user closes a Perps position and sees the success toast And the app has been backgrounded for more than 20 seconds When user foregrounds the app Then the closed position is NOT shown in the positions list And no stale "already closed" error appears on interaction Scenario: user returns after network loss Given the user has open Perps positions And airplane mode was enabled then disabled while app was backgrounded When user foregrounds the app Then the connection is restored And positions load correctly ``` ## **Screenshots/Recordings** ### **Before** Positions blank, 24h change missing, close position fails with "BTC is not tradeable asset". Closed positions reappear after >20s background. ### **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] > **Medium Risk** > Touches Perps connection lifecycle (reconnect/disconnect, NetInfo handling, preload guards) and could impact data freshness and reconnection reliability if regressions occur, though changes are localized and well-covered by tests. > > **Overview** > Prevents a foreground reconnection race by **removing the manager-level `AppState` listener** so only the lifecycle hook initiates foreground recovery. > > Hardens reconnect/disconnect behavior by **clearing all Perps stream channel caches on grace-period expiry**, resetting subscriber `hasReceivedFirstUpdate`, and resetting the in-flight preload state (`isPreloading`) so subsequent `connect()` calls reliably prewarm subscriptions. > > Improves offline→online recovery by treating `NetInfo.isInternetReachable: null` as “unknown” and falling back to `isConnected`, and expands `PerpsConnectionManager` tests to cover these concurrency, cache-clearing, and network edge cases. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 72739b2b36696d7b562e08e47cfd2e6d37a9966e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../UI/Perps/providers/PerpsStreamManager.tsx | 1 + .../services/PerpsConnectionManager.test.ts | 291 ++++++++++++++++-- .../Perps/services/PerpsConnectionManager.ts | 85 ++--- 3 files changed, 294 insertions(+), 83 deletions(-) diff --git a/app/components/UI/Perps/providers/PerpsStreamManager.tsx b/app/components/UI/Perps/providers/PerpsStreamManager.tsx index 52609200676..59c9818892b 100644 --- a/app/components/UI/Perps/providers/PerpsStreamManager.tsx +++ b/app/components/UI/Perps/providers/PerpsStreamManager.tsx @@ -318,6 +318,7 @@ abstract class StreamChannel { subscriber.timer = undefined; } subscriber.pendingUpdate = undefined; + subscriber.hasReceivedFirstUpdate = false; }); // Disconnect the old WebSocket subscription to stop receiving old account data diff --git a/app/components/UI/Perps/services/PerpsConnectionManager.test.ts b/app/components/UI/Perps/services/PerpsConnectionManager.test.ts index 2d36d84e60e..1bf10deac69 100644 --- a/app/components/UI/Perps/services/PerpsConnectionManager.test.ts +++ b/app/components/UI/Perps/services/PerpsConnectionManager.test.ts @@ -94,6 +94,7 @@ jest.mock('react-native-background-timer', () => ({ })); // Import non-singleton modules first +import { addEventListener as mockNetInfoAddEventListener } from '@react-native-community/netinfo'; import { DevLogger } from '../../../../core/SDKConnect/utils/DevLogger'; import Engine from '../../../../core/Engine'; import { store } from '../../../../store'; @@ -128,13 +129,20 @@ const resetManager = (manager: unknown) => { isInGracePeriod: boolean; gracePeriodTimer: number | null; hasPreloaded: boolean; + isPreloading: boolean; prewarmCleanups: (() => void)[]; - lastBalanceUpdateTime: number; + netInfoUnsubscribe: (() => void) | null; + wasOffline: boolean; }; // Call unsubscribe if it exists before resetting if (m.unsubscribeFromStore) { m.unsubscribeFromStore(); } + if (m.netInfoUnsubscribe) { + m.netInfoUnsubscribe(); + m.netInfoUnsubscribe = null; + } + m.wasOffline = false; // Clean up any prewarm subscriptions m.prewarmCleanups.forEach((cleanup) => cleanup()); m.prewarmCleanups = []; @@ -155,7 +163,7 @@ const resetManager = (manager: unknown) => { m.isInGracePeriod = false; m.gracePeriodTimer = null; m.hasPreloaded = false; - m.lastBalanceUpdateTime = 0; + m.isPreloading = false; }; describe('PerpsConnectionManager', () => { @@ -209,7 +217,7 @@ describe('PerpsConnectionManager', () => { }); describe('getInstance', () => { - it('should return the same instance when called multiple times', () => { + it('returns the same singleton instance on repeated calls', () => { const instance1 = PerpsConnectionManager; const instance2 = PerpsConnectionManager; @@ -218,7 +226,7 @@ describe('PerpsConnectionManager', () => { }); describe('connect', () => { - it('should initialize providers and connect successfully', async () => { + it('initializes providers and connects on first call', async () => { mockPerpsController.init.mockResolvedValueOnce(); await PerpsConnectionManager.connect(); @@ -229,7 +237,7 @@ describe('PerpsConnectionManager', () => { ); }); - it('should increment reference count on each connect call', async () => { + it('increments reference count on each connect call', async () => { mockPerpsController.init.mockResolvedValue(); await PerpsConnectionManager.connect(); @@ -243,7 +251,7 @@ describe('PerpsConnectionManager', () => { ); }); - it('should return existing promise if already connecting', async () => { + it('returns existing promise when connection is already in progress', async () => { // This test verifies that concurrent connect calls share the same promise // Track promises from both connect calls @@ -274,7 +282,7 @@ describe('PerpsConnectionManager', () => { ); }); - it('should handle connection failures', async () => { + it('sets error state and resets flags when connection fails', async () => { const error = new Error('Connection failed'); mockPerpsController.init.mockRejectedValueOnce(error); @@ -294,7 +302,7 @@ describe('PerpsConnectionManager', () => { expect(state.error).toBe('Connection failed'); }); - it('should skip reconnection if already connected', async () => { + it('skips reconnection when already connected', async () => { // First successful connection mockPerpsController.init.mockResolvedValueOnce(); await PerpsConnectionManager.connect(); @@ -308,7 +316,7 @@ describe('PerpsConnectionManager', () => { expect(mockPerpsController.init).toHaveBeenCalledTimes(1); }); - it('should handle rapid disconnect-connect cycles with grace period', async () => { + it('cancels grace period timer when reconnecting during grace period', async () => { // Setup initial connection mockPerpsController.init.mockResolvedValue(); mockPerpsController.getAccountState.mockResolvedValue({}); @@ -345,7 +353,7 @@ describe('PerpsConnectionManager', () => { mockPerpsController.disconnect.mockResolvedValue(); }); - it('should decrement reference count on disconnect', async () => { + it('decrements reference count on disconnect', async () => { await PerpsConnectionManager.connect(); await PerpsConnectionManager.connect(); @@ -359,7 +367,7 @@ describe('PerpsConnectionManager', () => { expect(mockPerpsController.disconnect).not.toHaveBeenCalled(); }); - it('should only start grace period when reference count reaches zero', async () => { + it('starts grace period only when reference count reaches zero', async () => { await PerpsConnectionManager.connect(); await PerpsConnectionManager.connect(); @@ -374,7 +382,7 @@ describe('PerpsConnectionManager', () => { ); }); - it('should handle disconnect gracefully with grace period', async () => { + it('starts grace period timer on disconnect instead of disconnecting immediately', async () => { await PerpsConnectionManager.connect(); // Disconnect should not throw even if controller would fail later @@ -386,7 +394,7 @@ describe('PerpsConnectionManager', () => { expect(mockPerpsController.disconnect).not.toHaveBeenCalled(); }); - it('should prevent negative reference count', async () => { + it('prevents reference count from going below zero', async () => { await PerpsConnectionManager.disconnect(); await PerpsConnectionManager.disconnect(); @@ -401,7 +409,7 @@ describe('PerpsConnectionManager', () => { expect(state.isConnected).toBe(false); }); - it('should maintain connection state during grace period', async () => { + it('maintains connected state during grace period', async () => { await PerpsConnectionManager.connect(); const connectedState = PerpsConnectionManager.getConnectionState(); @@ -418,7 +426,7 @@ describe('PerpsConnectionManager', () => { expect(gracePeriodState.isInGracePeriod).toBe(true); }); - it('should maintain state monitoring during grace period', async () => { + it('maintains state monitoring during grace period', async () => { // Connect to set up monitoring await PerpsConnectionManager.connect(); @@ -444,7 +452,7 @@ describe('PerpsConnectionManager', () => { }); describe('getConnectionState', () => { - it('should return initial state', () => { + it('returns initial disconnected state', () => { const state = PerpsConnectionManager.getConnectionState(); expect(state).toEqual({ @@ -457,7 +465,7 @@ describe('PerpsConnectionManager', () => { }); }); - it('should return connecting state during connection', async () => { + it('returns connecting state while connection is in progress', async () => { mockPerpsController.init.mockImplementation( () => new Promise((resolve) => setTimeout(resolve, 100)), ); @@ -556,7 +564,7 @@ describe('PerpsConnectionManager', () => { }); describe('concurrent operations', () => { - it('should handle multiple concurrent connect/disconnect operations', async () => { + it('serializes concurrent connect and disconnect operations', async () => { mockPerpsController.init.mockResolvedValue(); mockPerpsController.getAccountState.mockResolvedValue({}); mockPerpsController.disconnect.mockResolvedValue(); @@ -595,7 +603,7 @@ describe('PerpsConnectionManager', () => { (store.getState as jest.Mock).mockReturnValue({}); }); - it('should set up Redux store subscription on first connect', async () => { + it('sets up Redux store subscription on first connect', async () => { // Connect to trigger monitoring setup mockPerpsController.init.mockResolvedValue(); mockPerpsController.getAccountState.mockResolvedValue({}); @@ -610,7 +618,7 @@ describe('PerpsConnectionManager', () => { expect(typeof storeCallbacks[storeCallbacks.length - 1]).toBe('function'); }); - it('should detect account changes and trigger reconnection', async () => { + it('detects account changes and triggers reconnection', async () => { // Setup connected state mockPerpsController.init.mockResolvedValue(); mockPerpsController.getAccountState.mockResolvedValue({}); @@ -645,7 +653,7 @@ describe('PerpsConnectionManager', () => { ); }); - it('should detect network changes and trigger reconnection', async () => { + it('detects network changes and triggers reconnection', async () => { // Setup connected state mockPerpsController.init.mockResolvedValue(); mockPerpsController.getAccountState.mockResolvedValue({}); @@ -680,7 +688,7 @@ describe('PerpsConnectionManager', () => { ); }); - it('should continue monitoring during grace period', async () => { + it('continues monitoring state changes during grace period', async () => { // Setup but don't connect mockPerpsController.init.mockResolvedValue(); mockPerpsController.getAccountState.mockResolvedValue({}); @@ -718,7 +726,7 @@ describe('PerpsConnectionManager', () => { mockPerpsController.reconnectWithNewContext.mockResolvedValue(); }); - it('should clear StreamManager caches', async () => { + it('clears all StreamManager caches on reconnection', async () => { // Setup connected state first mockPerpsController.init.mockResolvedValue(); await PerpsConnectionManager.connect(); @@ -739,7 +747,7 @@ describe('PerpsConnectionManager', () => { expect(mockStreamManagerInstance.prices.clearCache).toHaveBeenCalled(); }); - it('should reinitialize controller with new context', async () => { + it('reinitializes controller with new account and network context', async () => { mockPerpsController.init.mockResolvedValue(); await ( @@ -753,7 +761,7 @@ describe('PerpsConnectionManager', () => { expect(mockPerpsController.init).toHaveBeenCalled(); }); - it('should handle reconnection errors gracefully', async () => { + it('logs error and resets connecting flag when reconnection fails', async () => { const error = new Error('Reconnection failed'); mockPerpsController.init.mockRejectedValueOnce(error); @@ -840,6 +848,84 @@ describe('PerpsConnectionManager', () => { }); }); + describe('preloadSubscriptions concurrency guard', () => { + it('skips concurrent preload when one is already in flight', async () => { + // Arrange + const m = PerpsConnectionManager as unknown as { + isPreloading: boolean; + hasPreloaded: boolean; + preloadSubscriptions: () => Promise; + }; + m.isPreloading = true; + + // Act + await m.preloadSubscriptions(); + + // Assert + expect(mockStreamManagerInstance.prices.prewarm).not.toHaveBeenCalled(); + }); + + it('allows fresh preload after performReconnection resets isPreloading', async () => { + // Arrange + mockPerpsController.init.mockResolvedValue(); + mockPerpsController.disconnect.mockResolvedValue(); + await PerpsConnectionManager.connect(); + expect(mockStreamManagerInstance.prices.prewarm).toHaveBeenCalledTimes(1); + + // Act + await ( + PerpsConnectionManager as unknown as { + reconnectWithNewContext: () => Promise; + } + ).reconnectWithNewContext(); + + // Assert + expect(mockStreamManagerInstance.prices.prewarm).toHaveBeenCalledTimes(2); + }); + }); + + describe('foreground reconnection — single reconnection flow', () => { + it('PerpsConnectionManager has no AppState listener — only the hook triggers foreground reconnect', () => { + // This test documents the fix for the race condition introduced by PR #26780. + // Previously, PerpsConnectionManager registered its own AppState listener in + // setupStateMonitoring(), which competed with usePerpsConnectionLifecycle hook. + // + // Both fired simultaneously on foreground: + // hook → connect() + // manager → reconnectWithNewContext({ force: true }) + // + // The force path cancelled the hook's in-flight connect() and cleaned up + // prewarm subscriptions mid-flight, leaving positions/prices without data. + // + // Fix: removed the manager-level AppState listener entirely. + // The hook is the sole owner of foreground recovery. + + const m = PerpsConnectionManager as unknown as Record; + + // appStateSubscription field must not exist on the manager + expect(m.appStateSubscription).toBeUndefined(); + + // handleAppStateChange method must not exist on the manager + expect(typeof m.handleAppStateChange).not.toBe('function'); + }); + + it('connect() on foreground completes without interference', async () => { + mockPerpsController.init.mockResolvedValue(); + + await PerpsConnectionManager.connect(); + + const state = PerpsConnectionManager.getConnectionState(); + expect(state.isConnected).toBe(true); + expect(state.isConnecting).toBe(false); + expect(state.error).toBeNull(); + // Prewarm subscriptions fired exactly once + expect(mockStreamManagerInstance.prices.prewarm).toHaveBeenCalledTimes(1); + expect(mockStreamManagerInstance.positions.prewarm).toHaveBeenCalledTimes( + 1, + ); + }); + }); + describe('waitForConnection', () => { it('awaits resolving initPromise', async () => { // Arrange — set initPromise to a resolved promise @@ -906,6 +992,161 @@ describe('PerpsConnectionManager', () => { }); }); + describe('performActualDisconnection — grace period expiry', () => { + beforeEach(async () => { + mockPerpsController.init.mockResolvedValue(); + mockPerpsController.disconnect.mockResolvedValue(); + await PerpsConnectionManager.connect(); + // Clear cache mock calls from connect/prewarm so we can assert specifically + Object.values(mockStreamManagerInstance).forEach(({ clearCache }) => + clearCache.mockClear(), + ); + }); + + it('clears all stream channel caches when grace period fires', async () => { + // Arrange + const m = PerpsConnectionManager as unknown as { + isConnected: boolean; + isInitialized: boolean; + connectionRefCount: number; + isPreloading: boolean; + performActualDisconnection: () => Promise; + }; + m.connectionRefCount = 0; + + // Act + await m.performActualDisconnection(); + + // Assert + expect(mockStreamManagerInstance.positions.clearCache).toHaveBeenCalled(); + expect(mockStreamManagerInstance.orders.clearCache).toHaveBeenCalled(); + expect(mockStreamManagerInstance.account.clearCache).toHaveBeenCalled(); + expect(mockStreamManagerInstance.prices.clearCache).toHaveBeenCalled(); + expect( + mockStreamManagerInstance.marketData.clearCache, + ).toHaveBeenCalled(); + expect(mockStreamManagerInstance.oiCaps.clearCache).toHaveBeenCalled(); + expect(mockStreamManagerInstance.fills.clearCache).toHaveBeenCalled(); + expect(mockStreamManagerInstance.topOfBook.clearCache).toHaveBeenCalled(); + expect(mockStreamManagerInstance.candles.clearCache).toHaveBeenCalled(); + }); + + it('resets isPreloading flag so next connect can prewarm', async () => { + // Arrange + const m = PerpsConnectionManager as unknown as { + connectionRefCount: number; + isPreloading: boolean; + performActualDisconnection: () => Promise; + }; + m.connectionRefCount = 0; + m.isPreloading = true; + + // Act + await m.performActualDisconnection(); + + // Assert + expect(m.isPreloading).toBe(false); + }); + + it('resets connection state flags after disconnecting', async () => { + // Arrange + const m = PerpsConnectionManager as unknown as { + connectionRefCount: number; + performActualDisconnection: () => Promise; + }; + m.connectionRefCount = 0; + + // Act + await m.performActualDisconnection(); + + // Assert + const state = PerpsConnectionManager.getConnectionState(); + expect(state.isConnected).toBe(false); + expect(state.isInitialized).toBe(false); + expect(state.isConnecting).toBe(false); + }); + + it('skips disconnection when reference count is still positive', async () => { + // Arrange — refCount > 0 means another consumer reconnected during grace period + const m = PerpsConnectionManager as unknown as { + connectionRefCount: number; + performActualDisconnection: () => Promise; + }; + m.connectionRefCount = 1; + + // Act + await m.performActualDisconnection(); + + // Assert — controller not called, caches not cleared + expect(mockPerpsController.disconnect).not.toHaveBeenCalled(); + expect( + mockStreamManagerInstance.positions.clearCache, + ).not.toHaveBeenCalled(); + }); + }); + + describe('NetInfo isInternetReachable null handling', () => { + type NetInfoCallback = (state: { + isInternetReachable: boolean | null; + isConnected: boolean | null; + }) => void; + + const mockAddEventListener = mockNetInfoAddEventListener as jest.Mock; + + let capturedCallback: NetInfoCallback | null = null; + + beforeEach(async () => { + // Set up to capture the listener, then connect so it registers + mockAddEventListener.mockImplementation((cb: NetInfoCallback) => { + capturedCallback = cb; + return jest.fn(); + }); + mockPerpsController.init.mockResolvedValue(); + await PerpsConnectionManager.connect(); + }); + + it('treats isInternetReachable null as online when isConnected is true', () => { + // Arrange — start disconnected so wasOffline is not set by online path + const m = PerpsConnectionManager as unknown as { + wasOffline: boolean; + isConnected: boolean; + }; + m.isConnected = false; // not connected — online path won't clear wasOffline + m.wasOffline = false; + + // Act — null isInternetReachable falls back to isConnected=true → isOnline=true + // Since isOnline=true and !wasOffline, the "set wasOffline=true" branch is skipped + capturedCallback?.({ isInternetReachable: null, isConnected: true }); + + // Assert — wasOffline stays false (not offline path, and not connected to clear it) + expect(m.wasOffline).toBe(false); + }); + + it('treats isInternetReachable null as offline when isConnected is false', () => { + // Arrange + const m = PerpsConnectionManager as unknown as { wasOffline: boolean }; + m.wasOffline = false; + + // Act — null isInternetReachable falls back to isConnected=false → offline + capturedCallback?.({ isInternetReachable: null, isConnected: false }); + + // Assert + expect(m.wasOffline).toBe(true); + }); + + it('treats isInternetReachable false as offline', () => { + // Arrange + const m = PerpsConnectionManager as unknown as { wasOffline: boolean }; + m.wasOffline = false; + + // Act + capturedCallback?.({ isInternetReachable: false, isConnected: true }); + + // Assert + expect(m.wasOffline).toBe(true); + }); + }); + describe('getActiveProviderName', () => { it('returns activeProvider from PerpsController state', () => { // Arrange diff --git a/app/components/UI/Perps/services/PerpsConnectionManager.ts b/app/components/UI/Perps/services/PerpsConnectionManager.ts index 18ac0acdcb8..dbb95df5884 100644 --- a/app/components/UI/Perps/services/PerpsConnectionManager.ts +++ b/app/components/UI/Perps/services/PerpsConnectionManager.ts @@ -1,4 +1,3 @@ -import { AppState, type AppStateStatus } from 'react-native'; import { addEventListener as netInfoAddEventListener, type NetInfoState, @@ -52,6 +51,7 @@ class PerpsConnectionManagerClass { private initPromise: Promise | null = null; private disconnectPromise: Promise | null = null; private hasPreloaded = false; + private isPreloading = false; private prewarmCleanups: (() => void)[] = []; private unsubscribeFromStore: (() => void) | null = null; private previousAddress: string | undefined; @@ -62,9 +62,6 @@ class PerpsConnectionManagerClass { private isInGracePeriod = false; private pendingReconnectPromise: Promise | null = null; private connectionTimeoutRef: ReturnType | null = null; - private appStateSubscription: ReturnType< - typeof AppState.addEventListener - > | null = null; private netInfoUnsubscribe: (() => void) | null = null; private wasOffline = false; private networkRestoreRetryTimer: ReturnType | null = null; @@ -190,32 +187,12 @@ class PerpsConnectionManagerClass { this.previousHip3Version = currentHip3Version; }); - // Listen for app state changes to reconnect after background - if (!this.appStateSubscription) { - this.appStateSubscription = AppState.addEventListener( - 'change', - (nextAppState: AppStateStatus) => { - this.handleAppStateChange(nextAppState).catch((error) => { - Logger.error( - ensureError(error, 'PerpsConnectionManager.appStateListener'), - { - tags: { feature: PERPS_CONSTANTS.FeatureName }, - context: { - name: 'PerpsConnectionManager.appStateListener', - data: { message: 'Error handling app state change' }, - }, - }, - ); - }); - }, - ); - } - // Listen for connectivity changes (WiFi off/on, airplane mode, etc.) if (!this.netInfoUnsubscribe) { this.netInfoUnsubscribe = netInfoAddEventListener( (netState: NetInfoState) => { - const isOnline = netState.isInternetReachable === true; + const isOnline = + netState.isInternetReachable ?? netState.isConnected ?? false; if (isOnline && this.wasOffline) { DevLogger.log( @@ -238,8 +215,7 @@ class PerpsConnectionManagerClass { /** * Validate the WebSocket connection and force-reconnect if it is stale. - * Shared by both AppState (background→foreground) and NetInfo (offline→online) - * recovery paths. + * Used by the NetInfo (offline→online) recovery path. * * @param context - Caller identifier for error reporting * @param skipPing - Skip ping and force reconnect after known network loss @@ -335,37 +311,10 @@ class PerpsConnectionManagerClass { this.networkRestoreRetryCount = 0; } - /** - * Handle app state transitions to recover from stale WebSocket connections. - * When the app returns from background, the OS may have silently killed the - * WebSocket. A health-check ping detects this and triggers reconnection. - */ - private async handleAppStateChange( - nextAppState: AppStateStatus, - ): Promise { - if (nextAppState !== 'active') { - return; - } - - // Cancel any pending grace period — user is back - if (this.isInGracePeriod) { - DevLogger.log( - 'PerpsConnectionManager: App resumed - cancelling grace period', - ); - this.cancelGracePeriod(); - } - - await this.validateAndReconnect('appResume'); - } - /** * Clean up state monitoring */ private cleanupStateMonitoring(): void { - if (this.appStateSubscription) { - this.appStateSubscription.remove(); - this.appStateSubscription = null; - } if (this.netInfoUnsubscribe) { this.netInfoUnsubscribe(); this.netInfoUnsubscribe = null; @@ -519,11 +468,25 @@ class PerpsConnectionManagerClass { // Clean up preloaded subscriptions this.cleanupPreloadedSubscriptions(); + // Clear all stream caches so subscribers reset to loading state + // and hasReceivedFirstUpdate is reset for clean reconnection + const streamManager = getStreamManagerInstance(); + streamManager.positions.clearCache(); + streamManager.orders.clearCache(); + streamManager.account.clearCache(); + streamManager.prices.clearCache(); + streamManager.marketData.clearCache(); + streamManager.oiCaps.clearCache(); + streamManager.fills.clearCache(); + streamManager.topOfBook.clearCache(); + streamManager.candles.clearCache(); + // Reset state before disconnecting to prevent race conditions this.isConnected = false; this.isInitialized = false; this.isConnecting = false; this.hasPreloaded = false; // Reset pre-load flag on disconnect + this.isPreloading = false; // Reset in-flight preload guard on disconnect this.clearError(); // Clear any errors on disconnect await Engine.context.PerpsController.disconnect(); @@ -911,6 +874,7 @@ class PerpsConnectionManagerClass { this.isConnected = false; this.isInitialized = false; this.hasPreloaded = false; + this.isPreloading = false; // Clear previous errors when starting reconnection attempt this.clearError(); @@ -1078,12 +1042,15 @@ class PerpsConnectionManagerClass { * Also sets up balance update subscriptions for portfolio integration */ private async preloadSubscriptions(): Promise { - // Only pre-load once per session - if (this.hasPreloaded) { - DevLogger.log('PerpsConnectionManager: Already pre-loaded, skipping'); + // Only pre-load once per session, and guard against concurrent calls + if (this.hasPreloaded || this.isPreloading) { + DevLogger.log( + 'PerpsConnectionManager: Already pre-loaded or preloading, skipping', + ); return; } + this.isPreloading = true; try { DevLogger.log( 'PerpsConnectionManager: Pre-loading WebSocket subscriptions via StreamManager', @@ -1137,6 +1104,8 @@ class PerpsConnectionManagerClass { }, ); // Non-critical error - components will still work with on-demand subscriptions + } finally { + this.isPreloading = false; } } From 3fb6a9a5bd4c8d12b22b435963fa353337c40a37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20=C5=81ucka?= <5708018+PatrykLucka@users.noreply.github.com> Date: Wed, 4 Mar 2026 14:46:41 +0100 Subject: [PATCH 04/15] feat(networks): add network deletion logic and update tests (#26983) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** **Reason:** The network details header always showed a trash/delete icon when editing a network, including for networks that cannot be removed (e.g. Ethereum mainnet, Linea, Goerli, testnets). That suggested deletion was possible when it wasn’t. **Solution:** Introduced canDeleteNetwork(chainId) in app/util/networks (aligned with NetworkSelector: mainnet, Linea, Goerli, and testnets are non-deletable). The header delete control is only rendered when !formHook.form.addMode && canDeleteNetwork(formHook.form.chainId ?? ''), so the trash icon is hidden for non-deletable networks. ## **Changelog** CHANGELOG entry: Fixed network details screen showing a delete (trash) icon for networks that cannot be removed (e.g. Ethereum mainnet, Linea, Goerli, testnets). ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-519 ## **Manual testing steps** ```gherkin Feature: Network details – delete icon visibility Scenario: user opens network details for a non-deletable network Given the user has network management enabled and is on the networks list When the user taps "Ethereum Mainnet" (or Linea / Goerli / a testnet) Then the header does not show a trash/delete icon Scenario: user opens network details for a deletable custom network Given the user has added a custom network (e.g. Polygon or a custom RPC) When the user taps that network to edit it Then the header shows the trash/delete icon and delete remains available ``` ## **Screenshots/Recordings** ### **Before** Screenshot 2026-03-04 at 12 53 36 Screenshot 2026-03-04 at 12 53 53 Screenshot 2026-03-04 at 12 54 09 ### **After** Screenshot 2026-03-04 at 12 49 51 Screenshot 2026-03-04 at 12 50 26 Screenshot 2026-03-04 at 12 50 34 ## **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] > **Low Risk** > Low risk UI gating change: it only conditionally hides the delete icon based on chain ID and adds unit coverage; core deletion flow is unchanged. > > **Overview** > Fixes the Network Details header to **only show the trash/delete icon for deletable networks** by introducing `canDeleteNetwork(chainId)` and using it to gate the header end accessory (e.g., hides delete for Ethereum mainnet, Linea mainnet, and testnets). > > Adds unit tests for `canDeleteNetwork` and a UI test ensuring the trash icon is not rendered when editing a non-deletable network. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9852c502c780f7829b41049210393f01ccfa6b98. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../NetworkDetailsView.test.tsx | 28 +++++++++++++++++++ .../NetworkDetailsView/NetworkDetailsView.tsx | 8 ++++-- app/util/networks/index.js | 15 ++++++++++ app/util/networks/index.test.ts | 28 +++++++++++++++++++ 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.test.tsx b/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.test.tsx index 375d14ffff2..ad7f94e5fd4 100644 --- a/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.test.tsx +++ b/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.test.tsx @@ -765,6 +765,34 @@ describe('NetworkDetailsView', () => { expect(ops.removeNetwork).toHaveBeenCalledWith('0x2a'); }); + + it('does not show trash icon when editing non-deletable network (e.g. Ethereum mainnet)', () => { + const mainnetEditForm = createMockFormHook({ + addMode: false, + nickname: 'Ethereum', + chainId: '0x1', + ticker: 'ETH', + rpcUrl: 'https://mainnet.infura.io/v3/key', + rpcName: 'Infura', + rpcUrls: [ + { + url: 'https://mainnet.infura.io/v3/key', + name: 'Infura', + type: 'infura', + }, + ], + blockExplorerUrl: 'https://etherscan.io', + blockExplorerUrls: ['https://etherscan.io'], + editable: true, + }); + mockFormHook.mockReturnValue(mainnetEditForm); + + const { getByTestId } = render(); + + const container = getByTestId(NetworkDetailsViewSelectorsIDs.CONTAINER); + const trashIcons = container.findAllByProps({ name: 'Trash' }); + expect(trashIcons).toHaveLength(0); + }); }); describe('RPC modal interactions', () => { diff --git a/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.tsx b/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.tsx index ba1f67445f4..7532ff6b7f3 100644 --- a/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.tsx +++ b/app/components/Views/NetworksManagement/NetworkDetailsView/NetworkDetailsView.tsx @@ -24,7 +24,10 @@ import { CaipChainId } from '@metamask/utils'; import { strings } from '../../../../../locales/i18n'; import { useTheme } from '../../../../util/theme'; import { useStyles } from '../../../../component-library/hooks/useStyles'; -import { getNetworkImageSource } from '../../../../util/networks'; +import { + canDeleteNetwork, + getNetworkImageSource, +} from '../../../../util/networks'; import { useNetworkEnablement } from '../../../hooks/useNetworkEnablement/useNetworkEnablement'; import { selectIsRpcFailoverEnabled } from '../../../../selectors/featureFlagController/walletFramework'; import HeaderCompactStandard from '../../../../component-library/components-temp/HeaderCompactStandard'; @@ -186,7 +189,8 @@ const NetworkDetailsView = () => { diff --git a/app/util/networks/index.js b/app/util/networks/index.js index 379bf693d9e..62df848cc7f 100644 --- a/app/util/networks/index.js +++ b/app/util/networks/index.js @@ -343,6 +343,21 @@ export const isTestNetworkWithFaucet = (chainId) => */ export const isTestNet = (chainId) => TESTNET_CHAIN_IDS.includes(chainId); +/** + * Returns whether the network can be deleted by the user. + * Aligns with NetworkSelector: mainnet, Linea mainnet, and testnets cannot be removed. + * + * @param {string} chainId - The chain ID to check (e.g. '0x1', '0x89'). + * @returns {boolean} True if the network can be deleted, false otherwise. + */ +export const canDeleteNetwork = (chainId) => + Boolean( + chainId && + !isTestNet(chainId) && + !isMainNet(chainId) && + !isLineaMainnetChainId(chainId), + ); + export function getNetworkTypeById(id) { if (!id) { throw new Error(NetworkSwitchErrorType.missingNetworkId); diff --git a/app/util/networks/index.test.ts b/app/util/networks/index.test.ts index d79efe305db..60c1a04da1b 100644 --- a/app/util/networks/index.test.ts +++ b/app/util/networks/index.test.ts @@ -19,6 +19,7 @@ import NetworkList, { isWhitelistedSymbol, isWhitelistedRpcUrl, isWhitelistedNetworkName, + canDeleteNetwork, } from '.'; import { convertNetworkId, @@ -162,6 +163,33 @@ describe('network-utils', () => { }); }); + describe('canDeleteNetwork', () => { + it('returns false for Ethereum mainnet', () => { + expect(canDeleteNetwork('0x1')).toBe(false); + }); + it('returns false for Linea mainnet', () => { + expect(canDeleteNetwork(NETWORKS_CHAIN_ID.LINEA_MAINNET)).toBe(false); + }); + it('returns false for Goerli', () => { + expect(canDeleteNetwork(NETWORKS_CHAIN_ID.GOERLI)).toBe(false); + }); + it.each(TESTNET_CHAIN_IDS)( + 'returns false for testnet chain ID %s', + (chainId) => { + expect(canDeleteNetwork(chainId)).toBe(false); + }, + ); + it('returns false for empty/falsy chainId', () => { + expect(canDeleteNetwork('')).toBe(false); + }); + it('returns true for custom mainnet (e.g. Polygon)', () => { + expect(canDeleteNetwork(NETWORKS_CHAIN_ID.POLYGON)).toBe(true); + }); + it('returns true for custom chain ID 0x2a', () => { + expect(canDeleteNetwork('0x2a')).toBe(true); + }); + }); + describe('getNetworkTypeById', () => { it.each([getAllNetworks()])( 'should get network type by Id - %s', From 8559e2157df9e3640949f56e32c4c40469c3339c Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 4 Mar 2026 14:03:59 +0000 Subject: [PATCH 05/15] feat: bump `@metamask-assets-controllers` to `^100.1.0` (#26987) ## **Description** - bumps `@metamask-assets-controllers` to `^100.1.0` - removes patch (fixed in 100.1.0) ## **Changelog** CHANGELOG entry: feat: bump `@metamask-assets-controllers` to `^100.1.0` ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2857 ## **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] > **Medium Risk** > Upgrades a core controller package and pulls in newer transitive controller versions (notably `@metamask/transaction-controller` and `@metamask/network-enablement-controller`), which can affect balance/asset and network behavior at runtime. > > **Overview** > Updates `@metamask/assets-controllers` from a locally patched `100.0.3` to the upstream `^100.1.0` package, and deletes the corresponding `.yarn/patches/...assets-controllers...patch` file. > > Refreshes `yarn.lock` to resolve the new release and its transitive dependency bumps (including `@metamask/network-enablement-controller` `4.2.0` and `@metamask/transaction-controller` `62.20.0`). > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0cc5d53d98a429c3be460ec05edd46bcabd5fcba. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- ...s-controllers-npm-100.0.3-ea172b88c9.patch | 28 --------- package.json | 3 +- yarn.lock | 60 +++++++++---------- 3 files changed, 31 insertions(+), 60 deletions(-) delete mode 100644 .yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch diff --git a/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch b/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch deleted file mode 100644 index dc19783ed9a..00000000000 --- a/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch +++ /dev/null @@ -1,28 +0,0 @@ -diff --git a/dist/multi-chain-accounts-service/api-balance-fetcher.cjs b/dist/multi-chain-accounts-service/api-balance-fetcher.cjs -index 6ae7034bd87dfdaa49324fd36a340689df45960e..4719739bf194f5fb8669f382b021b54952294508 100644 ---- a/dist/multi-chain-accounts-service/api-balance-fetcher.cjs -+++ b/dist/multi-chain-accounts-service/api-balance-fetcher.cjs -@@ -150,7 +150,10 @@ class AccountsApiBalanceFetcher { - chains.forEach((chainId) => { - const key = `${address}-${chainId}`; - const existingBalance = nativeBalancesFromAPI.get(key); -- if (!existingBalance) { -+ const isChainIncludedInRequest = chainIds.includes(chainId); -+ const isChainSupported = this.supports(chainId); -+ const shouldZeroOutBalance = !existingBalance && isChainIncludedInRequest && isChainSupported; -+ if (shouldZeroOutBalance) { - // Add zero native balance entry if API succeeded but didn't return one - results.push({ - success: true, -@@ -172,7 +175,10 @@ class AccountsApiBalanceFetcher { - const key = `${account.toLowerCase()}-${tokenLowerCase}-${chainId}`; - const isERC = tokenAddress !== ZERO_ADDRESS; - const existingBalance = nonNativeBalancesFromAPI.get(key); -- if (isERC && !existingBalance) { -+ const isChainIncludedInRequest = chainIds.includes(chainId); -+ const isChainSupported = this.supports(chainId); -+ const shouldZeroOutBalance = !existingBalance && isChainIncludedInRequest && isChainSupported; -+ if (isERC && shouldZeroOutBalance) { - results.push({ - success: true, - value: new bn_js_1.default('0'), diff --git a/package.json b/package.json index e1817282359..75d3424196a 100644 --- a/package.json +++ b/package.json @@ -176,7 +176,6 @@ "bn.js@npm:5.2.1": "5.2.3", "@metamask/bridge-controller@npm:^67.1.1": "patch:@metamask/bridge-controller@npm%3A67.2.0#~/.yarn/patches/@metamask-bridge-controller-npm-67.2.0-32d3aafe1f.patch", "@metamask/bridge-status-controller@npm:^67.0.1": "patch:@metamask/bridge-status-controller@npm%3A67.0.1#~/.yarn/patches/@metamask-bridge-status-controller-npm-67.0.1-d8a41d9c02.patch", - "@metamask/assets-controllers@npm:^99.4.0": "patch:@metamask/assets-controllers@npm%3A100.0.3#~/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch", "@metamask/ramps-controller": "npm:@metamask-previews/ramps-controller@10.0.0-preview-225638478" }, "dependencies": { @@ -203,7 +202,7 @@ "@metamask/app-metadata-controller": "^2.0.0", "@metamask/approval-controller": "^8.0.0", "@metamask/assets-controller": "^2.0.0", - "@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A100.0.3#~/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch", + "@metamask/assets-controllers": "^100.1.0", "@metamask/base-controller": "^9.0.0", "@metamask/bitcoin-wallet-snap": "^1.10.0", "@metamask/bridge-controller": "^67.4.0", diff --git a/yarn.lock b/yarn.lock index 67daa1db544..5c9c46381f4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7707,9 +7707,9 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controllers@npm:100.0.3, @metamask/assets-controllers@npm:^100.0.2, @metamask/assets-controllers@npm:^100.0.3": - version: 100.0.3 - resolution: "@metamask/assets-controllers@npm:100.0.3" +"@metamask/assets-controllers@npm:^100.0.2, @metamask/assets-controllers@npm:^100.0.3, @metamask/assets-controllers@npm:^100.1.0": + version: 100.1.0 + resolution: "@metamask/assets-controllers@npm:100.1.0" dependencies: "@ethereumjs/util": "npm:^9.1.0" "@ethersproject/abi": "npm:^5.7.0" @@ -7732,7 +7732,7 @@ __metadata: "@metamask/metamask-eth-abis": "npm:^3.1.1" "@metamask/multichain-account-service": "npm:^7.0.0" "@metamask/network-controller": "npm:^30.0.0" - "@metamask/network-enablement-controller": "npm:^4.1.2" + "@metamask/network-enablement-controller": "npm:^4.2.0" "@metamask/permission-controller": "npm:^12.2.0" "@metamask/phishing-controller": "npm:^16.3.0" "@metamask/polling-controller": "npm:^16.0.3" @@ -7743,7 +7743,7 @@ __metadata: "@metamask/snaps-sdk": "npm:^10.3.0" "@metamask/snaps-utils": "npm:^11.7.0" "@metamask/storage-service": "npm:^1.0.0" - "@metamask/transaction-controller": "npm:^62.18.0" + "@metamask/transaction-controller": "npm:^62.20.0" "@metamask/utils": "npm:^11.9.0" "@types/bn.js": "npm:^5.1.5" "@types/uuid": "npm:^8.3.0" @@ -7759,13 +7759,13 @@ __metadata: peerDependencies: "@metamask/providers": ^22.0.0 webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0 - checksum: 10/fd5ef80f4b3d55bcc5bb98dcf6a1fcbfa4b7401ca2591658b2474f0cd69d69c4b4ba6b5e2e4c9b13d1e3ba55f38e501494601eb8c16e702b715bd32283271a17 + checksum: 10/25a32346b51432bc9d40028719a1b8f0b4a2797458c0fa22f84c1ac3982a4a16e89d7e0b16e1c439377e65621445733b17905b24d4d25c804317b087a19af0ba languageName: node linkType: hard -"@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A100.0.3#~/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch": - version: 100.0.3 - resolution: "@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A100.0.3#~/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch::version=100.0.3&hash=7df952" +"@metamask/assets-controllers@npm:^99.4.0": + version: 99.4.0 + resolution: "@metamask/assets-controllers@npm:99.4.0" dependencies: "@ethereumjs/util": "npm:^9.1.0" "@ethersproject/abi": "npm:^5.7.0" @@ -7775,23 +7775,23 @@ __metadata: "@ethersproject/providers": "npm:^5.7.0" "@metamask/abi-utils": "npm:^2.0.3" "@metamask/account-tree-controller": "npm:^4.1.1" - "@metamask/accounts-controller": "npm:^36.0.1" + "@metamask/accounts-controller": "npm:^36.0.0" "@metamask/approval-controller": "npm:^8.0.0" "@metamask/base-controller": "npm:^9.0.0" "@metamask/contract-metadata": "npm:^2.4.0" - "@metamask/controller-utils": "npm:^11.19.0" - "@metamask/core-backend": "npm:^6.0.0" + "@metamask/controller-utils": "npm:^11.18.0" + "@metamask/core-backend": "npm:5.0.0" "@metamask/eth-query": "npm:^4.0.0" "@metamask/keyring-api": "npm:^21.5.0" "@metamask/keyring-controller": "npm:^25.1.0" "@metamask/messenger": "npm:^0.3.0" "@metamask/metamask-eth-abis": "npm:^3.1.1" "@metamask/multichain-account-service": "npm:^7.0.0" - "@metamask/network-controller": "npm:^30.0.0" - "@metamask/network-enablement-controller": "npm:^4.1.2" + "@metamask/network-controller": "npm:^29.0.0" + "@metamask/network-enablement-controller": "npm:^4.1.0" "@metamask/permission-controller": "npm:^12.2.0" - "@metamask/phishing-controller": "npm:^16.3.0" - "@metamask/polling-controller": "npm:^16.0.3" + "@metamask/phishing-controller": "npm:^16.2.0" + "@metamask/polling-controller": "npm:^16.0.2" "@metamask/preferences-controller": "npm:^22.1.0" "@metamask/profile-sync-controller": "npm:^27.1.0" "@metamask/rpc-errors": "npm:^7.0.2" @@ -7799,7 +7799,7 @@ __metadata: "@metamask/snaps-sdk": "npm:^10.3.0" "@metamask/snaps-utils": "npm:^11.7.0" "@metamask/storage-service": "npm:^1.0.0" - "@metamask/transaction-controller": "npm:^62.18.0" + "@metamask/transaction-controller": "npm:^62.17.0" "@metamask/utils": "npm:^11.9.0" "@types/bn.js": "npm:^5.1.5" "@types/uuid": "npm:^8.3.0" @@ -7815,7 +7815,7 @@ __metadata: peerDependencies: "@metamask/providers": ^22.0.0 webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0 - checksum: 10/1b3069b2e5e3e431f54c95e15041fbc464abf003f7d35e094f04b4fcd290a228b9b4998f4c05be9ed92fc683b23b444b4f5713429e8a6d8f3b17d8c01185af7d + checksum: 10/2329ba8efe5a19ebe836c8ddc492f732461078d3954b713e825e4f0f3f5dc5fb17d55f5dabd30a20bd25e33366e8f2358a23b227c182f36688908f0128c5046c languageName: node linkType: hard @@ -9155,9 +9155,9 @@ __metadata: languageName: node linkType: hard -"@metamask/network-enablement-controller@npm:^4.1.0, @metamask/network-enablement-controller@npm:^4.1.1, @metamask/network-enablement-controller@npm:^4.1.2": - version: 4.1.2 - resolution: "@metamask/network-enablement-controller@npm:4.1.2" +"@metamask/network-enablement-controller@npm:^4.1.0, @metamask/network-enablement-controller@npm:^4.1.1, @metamask/network-enablement-controller@npm:^4.2.0": + version: 4.2.0 + resolution: "@metamask/network-enablement-controller@npm:4.2.0" dependencies: "@metamask/base-controller": "npm:^9.0.0" "@metamask/controller-utils": "npm:^11.19.0" @@ -9166,10 +9166,10 @@ __metadata: "@metamask/multichain-network-controller": "npm:^3.0.4" "@metamask/network-controller": "npm:^30.0.0" "@metamask/slip44": "npm:^4.3.0" - "@metamask/transaction-controller": "npm:^62.17.1" + "@metamask/transaction-controller": "npm:^62.20.0" "@metamask/utils": "npm:^11.9.0" reselect: "npm:^5.1.1" - checksum: 10/2860220c63c941173a66be21011c58014421868d1e0e67d05d7ae30c156ac8352d752317e31ac773fa1420a5fdc7126b7c78e317d53d404cd76fd0781cf2c0cb + checksum: 10/293efbfbf6b157e248ea1c7bf5fc70abfde4c47802ff2af25978b723ac3b7e657980566a2a3d1311a263209ea650cfa531482a087660dc6178c09e2491806341 languageName: node linkType: hard @@ -9255,7 +9255,7 @@ __metadata: languageName: node linkType: hard -"@metamask/phishing-controller@npm:^16.1.0, @metamask/phishing-controller@npm:^16.3.0": +"@metamask/phishing-controller@npm:^16.1.0, @metamask/phishing-controller@npm:^16.2.0, @metamask/phishing-controller@npm:^16.3.0": version: 16.3.0 resolution: "@metamask/phishing-controller@npm:16.3.0" dependencies: @@ -10068,9 +10068,9 @@ __metadata: languageName: node linkType: hard -"@metamask/transaction-controller@npm:^62.17.0, @metamask/transaction-controller@npm:^62.17.1, @metamask/transaction-controller@npm:^62.18.0, @metamask/transaction-controller@npm:^62.19.0": - version: 62.19.0 - resolution: "@metamask/transaction-controller@npm:62.19.0" +"@metamask/transaction-controller@npm:^62.17.0, @metamask/transaction-controller@npm:^62.17.1, @metamask/transaction-controller@npm:^62.18.0, @metamask/transaction-controller@npm:^62.19.0, @metamask/transaction-controller@npm:^62.20.0": + version: 62.20.0 + resolution: "@metamask/transaction-controller@npm:62.20.0" dependencies: "@ethereumjs/common": "npm:^4.4.0" "@ethereumjs/tx": "npm:^5.4.0" @@ -10090,7 +10090,7 @@ __metadata: "@metamask/metamask-eth-abis": "npm:^3.1.1" "@metamask/network-controller": "npm:^30.0.0" "@metamask/nonce-tracker": "npm:^6.0.0" - "@metamask/remote-feature-flag-controller": "npm:^4.0.0" + "@metamask/remote-feature-flag-controller": "npm:^4.1.0" "@metamask/rpc-errors": "npm:^7.0.2" "@metamask/utils": "npm:^11.9.0" async-mutex: "npm:^0.5.0" @@ -10103,7 +10103,7 @@ __metadata: peerDependencies: "@babel/runtime": ^7.0.0 "@metamask/eth-block-tracker": ">=9" - checksum: 10/d98cff056d00830a962dfd41f26211334fe52cf7a3f2833ad3e542303d7c43011918760f37d5a9dd145088b9c45376914de6a523716686897e899842a5d19038 + checksum: 10/5b0f1af0cf484d39ff29c674bdaed864c6301b98f247cd8c647ac7c51838a27d3b4ddea1bf21c5931e0ce5f60db5f0afb38924e746261afcafb12e28a8581cd6 languageName: node linkType: hard @@ -35378,7 +35378,7 @@ __metadata: "@metamask/app-metadata-controller": "npm:^2.0.0" "@metamask/approval-controller": "npm:^8.0.0" "@metamask/assets-controller": "npm:^2.0.0" - "@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A100.0.3#~/.yarn/patches/@metamask-assets-controllers-npm-100.0.3-ea172b88c9.patch" + "@metamask/assets-controllers": "npm:^100.1.0" "@metamask/auto-changelog": "npm:^5.3.0" "@metamask/base-controller": "npm:^9.0.0" "@metamask/bitcoin-wallet-snap": "npm:^1.10.0" From b44d5a66d834f527e11fe6edf600155efab4a74b Mon Sep 17 00:00:00 2001 From: cmd-ob Date: Wed, 4 Mar 2026 15:00:04 +0000 Subject: [PATCH 06/15] test: mock getQuoteStream with SSE format to fix SmokeTrade abort failures (#26977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** ## Problem `SmokeTrade: Swap from Actions` was failing in CI with: Error: Aborted at IncomingMessage.failWithAbortError (node_modules/mockttp/src/u...) Two compounding issues: 1. **Wrong endpoint mocked.** The bridge controller uses `getQuoteStream` (SSE) for all quote requests, not `getQuote`. The existing mock patterns (`/getQuote.*slippage=.*/`) happened to substring-match `getQuoteStream` for requests *with* a slippage param, but the **initial render request** (fired before `useInitialSlippage` dispatches the first slippage value) has no `slippage=` param and matched no rule. It fell through to `handleDirectFetch` → real network call to `bridge.api.cx.metamask.io` in CI. 2. **Wrong response format.** Even when a mock matched, it returned raw JSON (`[{...}]`). The bridge controller's `fetchServerEvents` SSE parser only calls `onMessage` for lines prefixed with `data:` — raw JSON yields zero quotes, so `activeQuote` was never set and the confirm button never appeared. The combination caused: real API call in flight → bridge controller fires `AbortController.abort()` on the next param change → TCP connection dropped → mockttp throws `Error: Aborted` from its callback chain — bypassing the existing `_installAbortFilter` (which only catches `unhandledRejection`, not errors propagating through the callback chain). ## Changes **`tests/helpers/swap/constants.ts`** - Added `toSSEResponse()` helper that wraps quote fixture arrays into SSE format (`data: {...}\n\n` per quote) **`tests/helpers/swap/swap-mocks.ts`** - Changed all mock URL patterns from `getQuote` → `getQuoteStream` - Wrapped all quote fixture responses in `toSSEResponse()` so the SSE parser receives and dispatches quotes correctly - Added a priority-1 blanket catch-all for any `getQuoteStream` URL as a safety net for no-slippage initial requests **`tests/api-mocking/MockServerE2E.ts`** - Wrapped both `handleDirectFetch` call sites (the `/proxy` catch-all and `forUnmatchedRequest`) with a try-catch that converts `Error: Aborted` to a 499 response, preventing it from propagating out of the mockttp callback chain ## **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 - [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] > **Low Risk** > Low risk: changes are isolated to E2E test mocking and mock server error handling, with no production logic impacted. Main risk is overly-broad catch-all mocks or `499` masking unexpected abort-related failures in tests. > > **Overview** > Fixes flaky swap E2E runs by updating swap quote mocks to match the bridge-controller’s SSE `getQuoteStream` endpoint and returning SSE-formatted quote data via a new `toSSEResponse` helper. > > Adds low-priority catch-all mocks for both `getQuoteStream` (SSE) and `/getQuote?` (JSON fallback) to prevent accidental live-network fallthrough, and hardens `MockServerE2E` forwarding so `Error: Aborted` from dropped client connections is converted into a benign `499` response instead of failing the test harness. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ed9c27d0b02d0f79634df885ad72fdf8fd6eb8aa. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- tests/api-mocking/MockServerE2E.ts | 44 ++++++--- tests/helpers/swap/constants.ts | 9 ++ tests/helpers/swap/swap-mocks.ts | 143 +++++++++++++++++++++++++---- 3 files changed, 166 insertions(+), 30 deletions(-) diff --git a/tests/api-mocking/MockServerE2E.ts b/tests/api-mocking/MockServerE2E.ts index 7161f459264..6ef7082b953 100644 --- a/tests/api-mocking/MockServerE2E.ts +++ b/tests/api-mocking/MockServerE2E.ts @@ -368,12 +368,22 @@ export default class MockServerE2E implements Resource { } } - return handleDirectFetch( - updatedUrl, - method, - request.headers, - method === 'POST' ? requestBodyText : undefined, - ); + try { + return await handleDirectFetch( + updatedUrl, + method, + request.headers, + method === 'POST' ? requestBodyText : undefined, + ); + } catch (error) { + // Client dropped the connection before we could respond (e.g. bridge + // controller AbortController fired mid-request). Return a benign + // response so mockttp doesn't surface an unhandled rejection. + if (error instanceof Error && error.message === 'Aborted') { + return { statusCode: 499, body: '' }; + } + throw error; + } } finally { this._activeRequests--; } @@ -430,12 +440,22 @@ export default class MockServerE2E implements Resource { } } - return handleDirectFetch( - translatedUrl, - request.method, - request.headers, - await request.body.getText(), - ); + try { + return await handleDirectFetch( + translatedUrl, + request.method, + request.headers, + await request.body.getText(), + ); + } catch (error) { + // Client dropped the connection before we could respond (e.g. bridge + // controller AbortController fired mid-request). Return a benign + // response so mockttp doesn't surface an unhandled rejection. + if (error instanceof Error && error.message === 'Aborted') { + return { statusCode: 499, body: '' }; + } + throw error; + } } finally { this._activeRequests--; } diff --git a/tests/helpers/swap/constants.ts b/tests/helpers/swap/constants.ts index 1569d25543f..c07ab445f33 100644 --- a/tests/helpers/swap/constants.ts +++ b/tests/helpers/swap/constants.ts @@ -1,3 +1,12 @@ +/** + * Wraps quote fixture objects as an SSE-formatted response string. + * Required because bridge-controller uses fetchServerEvents (SSE parser) + * which expects lines prefixed with "data: ". Raw JSON returns zero quotes. + */ +export function toSSEResponse(quotes: object[]): string { + return quotes.map((q) => `data: ${JSON.stringify(q)}\n\n`).join(''); +} + export const localNodeOptions = { hardfork: 'london', mnemonic: diff --git a/tests/helpers/swap/swap-mocks.ts b/tests/helpers/swap/swap-mocks.ts index d544c974c10..a37d16bae91 100644 --- a/tests/helpers/swap/swap-mocks.ts +++ b/tests/helpers/swap/swap-mocks.ts @@ -18,6 +18,7 @@ import { GET_TOKENS_API_USDC_RESPONSE, GET_TOKENS_API_USDT_RESPONSE, GET_QUOTE_USDC_GOOGLON_RESPONSE, + toSSEResponse, } from './constants'; const USDC_MAINNET = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; @@ -73,66 +74,164 @@ export const testSpecificMock: TestSpecificMock = async ( ) => { await setupSpotPricesMock(mockServer); - // Mock ETH->USDC with default 2% slippage + // ── SSE path (bridge-controller SSE feature flag ON) ────────────────────── + // Catch-all for getQuoteStream with no slippage param (initial render before + // useInitialSlippage fires). Registered first so specific mocks below at + // priority 999 take precedence. Prevents real network calls that cause + // Error: Aborted when the bridge controller aborts the in-flight request. + await setupMockRequest( + mockServer, + { + requestMethod: 'GET', + url: /getQuoteStream/i, + response: toSSEResponse(GET_QUOTE_ETH_USDC_RESPONSE), + responseCode: 200, + }, + 1, // lower priority than the specific mocks below (999) + ); + + // Mock ETH->USDC with default 2% slippage (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*destTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*slippage=2/i, + response: toSSEResponse(GET_QUOTE_ETH_USDC_RESPONSE), + responseCode: 200, + }); + + // Mock ETH->USDC with 3.5% custom slippage (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*destTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*slippage=3\.5/i, + response: toSSEResponse(GET_QUOTE_ETH_USDC_RESPONSE_CUSTOM_SLIPPAGE), + responseCode: 200, + }); + + // Mock ETH->DAI (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*destTokenAddress=0x6B175474E89094C44Da98b954EedeAC495271d0F/i, + response: toSSEResponse(GET_QUOTE_ETH_DAI_RESPONSE), + responseCode: 200, + }); + + // Mock USDC->USDT (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*destTokenAddress=0xdAC17F958D2ee523a2206206994597C13D831ec7/i, + response: toSSEResponse(GET_QUOTE_USDC_USDT_RESPONSE), + responseCode: 200, + }); + + // No quote when destination is mUSD (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*destTokenAddress=0xacA92E438df0B2401fF60dA7E4337B687a2435DA/i, + response: '', + responseCode: 200, + }); + + // Mock USDC->ETH (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*srcTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*destTokenAddress=0x0000000000000000000000000000000000000000/i, + response: toSSEResponse(GET_QUOTE_USDC_ETH_RESPONSE), + responseCode: 200, + }); + + // Mock ETH->WETH (SSE) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*slippage=2/i, + url: /getQuoteStream.*destTokenAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2/i, + response: toSSEResponse(GET_QUOTE_ETH_WETH_RESPONSE), + responseCode: 200, + }); + + // Mock WETH->ETH (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*srcTokenAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2.*destTokenAddress=0x0000000000000000000000000000000000000000/i, + response: toSSEResponse(GET_QUOTE_WETH_ETH_SAME_CHAIN_RESPONSE), + responseCode: 200, + }); + + // ── JSON path (bridge-controller SSE feature flag OFF) ───────────────────── + // The bridge controller falls back to fetchBridgeQuotes → /getQuote? (no + // "Stream" suffix) returning plain JSON when sse.enabled is false (e.g. local + // dev with BRIDGE_USE_DEV_APIS=true). Use /\/getQuote\?/i so the regex matches + // "getQuote?" but NOT "getQuoteStream?". + + // Catch-all for getQuote (no slippage / initial render) + await setupMockRequest( + mockServer, + { + requestMethod: 'GET', + url: /\/getQuote\?/i, + response: GET_QUOTE_ETH_USDC_RESPONSE, + responseCode: 200, + }, + 1, // lower priority than specific mocks below (999) + ); + + // Mock ETH->USDC with default 2% slippage (JSON) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /\/getQuote\?.*destTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*slippage=2/i, response: GET_QUOTE_ETH_USDC_RESPONSE, responseCode: 200, }); - // Mock ETH->USDC with 3.5% custom slippage + // Mock ETH->USDC with 3.5% custom slippage (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*slippage=3.5/i, + url: /\/getQuote\?.*destTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*slippage=3\.5/i, response: GET_QUOTE_ETH_USDC_RESPONSE_CUSTOM_SLIPPAGE, responseCode: 200, }); - // Mock ETH->DAI + // Mock ETH->DAI (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0x6B175474E89094C44Da98b954EedeAC495271d0F/i, + url: /\/getQuote\?.*destTokenAddress=0x6B175474E89094C44Da98b954EedeAC495271d0F/i, response: GET_QUOTE_ETH_DAI_RESPONSE, responseCode: 200, }); - // Mock USDC->USDT + // Mock USDC->USDT (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0xdAC17F958D2ee523a2206206994597C13D831ec7/i, + url: /\/getQuote\?.*destTokenAddress=0xdAC17F958D2ee523a2206206994597C13D831ec7/i, response: GET_QUOTE_USDC_USDT_RESPONSE, responseCode: 200, }); - // No need quote when destination is mUSD + // No quote when destination is mUSD (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0xacA92E438df0B2401fF60dA7E4337B687a2435DA/i, + url: /\/getQuote\?.*destTokenAddress=0xacA92E438df0B2401fF60dA7E4337B687a2435DA/i, response: [], responseCode: 200, }); - // Mock USDC->ETH (ETH native token address is 0x0000...0000) + // Mock USDC->ETH (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*srcTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*destTokenAddress=0x0000000000000000000000000000000000000000/i, + url: /\/getQuote\?.*srcTokenAddress=0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48.*destTokenAddress=0x0000000000000000000000000000000000000000/i, response: GET_QUOTE_USDC_ETH_RESPONSE, responseCode: 200, }); - // Mock ETH->WETH (wrapped native) + // Mock ETH->WETH (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2/i, + url: /\/getQuote\?.*destTokenAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2/i, response: GET_QUOTE_ETH_WETH_RESPONSE, responseCode: 200, }); - // Mock WETH->ETH (same-chain unwrap for E2E) + // Mock WETH->ETH (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*srcTokenAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2.*destTokenAddress=0x0000000000000000000000000000000000000000/i, + url: /\/getQuote\?.*srcTokenAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2.*destTokenAddress=0x0000000000000000000000000000000000000000/i, response: GET_QUOTE_WETH_ETH_SAME_CHAIN_RESPONSE, responseCode: 200, }); @@ -169,10 +268,18 @@ export const testSpecificMock: TestSpecificMock = async ( responseCode: 200, }); - // Mock USDC->GOOGLON + // Mock USDC->GOOGLON (SSE) + await setupMockRequest(mockServer, { + requestMethod: 'GET', + url: /getQuoteStream.*destTokenAddress=0xba47214edd2bb43099611b208f75e4b42fdcfedc/i, + response: toSSEResponse(GET_QUOTE_USDC_GOOGLON_RESPONSE), + responseCode: 200, + }); + + // Mock USDC->GOOGLON (JSON) await setupMockRequest(mockServer, { requestMethod: 'GET', - url: /getQuote.*destTokenAddress=0xba47214edd2bb43099611b208f75e4b42fdcfedc/i, + url: /\/getQuote\?.*destTokenAddress=0xba47214edd2bb43099611b208f75e4b42fdcfedc/i, response: GET_QUOTE_USDC_GOOGLON_RESPONSE, responseCode: 200, }); From a25945475641c504e970c1382ef2a488e8b6affe Mon Sep 17 00:00:00 2001 From: Bruno Nascimento Date: Wed, 4 Mar 2026 12:16:48 -0300 Subject: [PATCH 07/15] fix(card): cashback UI fixes (#26993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fixes several cashback-related issues in the Card feature: hides the cashback option for US users, shows a differentiated cashback rate for Metal Card holders, navigates back after a successful withdrawal, and applies minor UI polish. **Why**: US users should not see the cashback option. Metal Card holders earn a higher cashback rate (3%) that was not reflected in the UI. After a successful cashback withdrawal, users remained on the Cashback screen with no clear dismissal path. **What changed**: - **`CardHome.tsx`**: Added `userLocation !== 'us'` guard so the cashback list item is hidden for US users. Updated the cashback description to show "Earn 3% back on all spending" when `cardDetails.type` is `METAL`, falling back to the existing 1% description otherwise. - **`Cashback.tsx`**: Added `useNavigation().goBack()` call after a successful withdrawal (when `monitoringStatus === 'success'`), so the user is automatically returned to the Card home screen. Migrated `Skeleton` import to `@metamask/design-system-react-native`. Changed background color tokens from `bg-background-alternative` to `bg-background-muted` for the error and details cards. - **`en.json`**: Added `cashback_description_metal` translation key ("Earn 3% back on all spending"). - **`CardHome.test.tsx`**: Added 7 tests covering cashback item visibility (international vs US, authenticated vs not, verified KYC vs pending), description text for virtual vs metal card, and navigation/analytics on press. - **`Cashback.test.tsx`**: Added `useNavigation` mock and 3 tests verifying `goBack` is called on success and not called on failure or idle status. ## **Changelog** CHANGELOG entry: Hide cashback option for US users on Card home screen. CHANGELOG entry: Show "Earn 3% back" cashback description for Metal Card holders. CHANGELOG entry: Automatically navigate back after successful cashback withdrawal. CHANGELOG entry: Update Cashback screen background tokens to `bg-background-muted`. CHANGELOG entry: Add comprehensive test coverage for cashback visibility, description, and navigation behavior. ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Cashback visibility by region Scenario: Cashback item hidden for US users Given the user is authenticated with verified KYC And the user location is "us" When the Card home screen renders Then the Cashback list item is not visible Scenario: Cashback item visible for non-US users Given the user is authenticated with verified KYC And the user location is not "us" When the Card home screen renders Then the Cashback list item is visible Feature: Cashback description by card type Scenario: Standard cashback description for virtual card Given the user has a virtual card And the user is a non-US authenticated user with verified KYC When the Card home screen renders Then the cashback description reads "Earn 1% back on all spending" Scenario: Metal cashback description for metal card Given the user has a metal card And the user is a non-US authenticated user with verified KYC When the Card home screen renders Then the cashback description reads "Earn 3% back on all spending" Feature: Navigate back after successful withdrawal Scenario: User is returned to Card home after successful withdrawal Given the user is on the Cashback screen with a withdrawable balance When the user taps Withdraw and the withdrawal succeeds Then a success toast is shown And the user is navigated back to the Card home screen ``` ## **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] > **Medium Risk** > Updates Card home eligibility/visibility rules and adds an automatic navigation side effect after cashback withdrawal success, which could change what actions users can access and when they’re redirected. Scope is limited to card UI/UX and copy, with added test coverage to reduce regression risk. > > **Overview** > **Cashback visibility and copy is updated on Card home.** The cashback list item is now hidden for `userLocation === 'us'`, and its description switches to a new Metal-specific string when `cardDetails.type === CardType.METAL`. > > **Cashback withdrawal UX is tightened.** On `monitoringStatus === 'success'`, the Cashback screen now shows the success toast *and* navigates back via `goBack()`, plus minor UI polish (design-system `Skeleton` import and background token change to `bg-background-muted`). Tests are expanded in `CardHome.test.tsx` and `Cashback.test.tsx`, and `en.json` adds `cashback_description_metal`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ddd5192dce9cb0c4dd790b76fc023363a345406b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../UI/Card/Views/CardHome/CardHome.test.tsx | 188 ++++++++++++++++++ .../UI/Card/Views/CardHome/CardHome.tsx | 113 ++++++----- .../UI/Card/Views/Cashback/Cashback.test.tsx | 56 ++++++ .../UI/Card/Views/Cashback/Cashback.tsx | 11 +- locales/languages/en.json | 1 + 5 files changed, 314 insertions(+), 55 deletions(-) diff --git a/app/components/UI/Card/Views/CardHome/CardHome.test.tsx b/app/components/UI/Card/Views/CardHome/CardHome.test.tsx index f5d80659410..19785cc8006 100644 --- a/app/components/UI/Card/Views/CardHome/CardHome.test.tsx +++ b/app/components/UI/Card/Views/CardHome/CardHome.test.tsx @@ -531,6 +531,11 @@ jest.mock('../../../../../../locales/i18n', () => ({ 'Failed to load PIN. Please try again.', 'card.password_bottomsheet.description_view_pin': 'Enter your wallet password to view your card PIN.', + 'card.card_home.manage_card_options.cashback': 'Cashback', + 'card.card_home.manage_card_options.cashback_description': + 'Earn 1% back on all spending', + 'card.card_home.manage_card_options.cashback_description_metal': + 'Earn 3% back on all spending', }; return strings[key] || key; }, @@ -5565,4 +5570,187 @@ describe('CardHome Component', () => { expect(cardDetails.holderName).toBe('Jane Smith'); }); }); + + describe('Cashback List Item', () => { + it('displays cashback item for authenticated international user with VERIFIED KYC', () => { + // Given: authenticated international user with verified KYC + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + kycStatus: { verificationState: 'VERIFIED', userId: 'user-123' }, + }); + + // When: component renders + render(); + + // Then: cashback item is visible + expect( + screen.getByTestId(CardHomeSelectors.CASHBACK_ITEM), + ).toBeOnTheScreen(); + }); + + it('hides cashback item for US users', () => { + // Given: authenticated US user with verified KYC + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'us', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + kycStatus: { verificationState: 'VERIFIED', userId: 'user-123' }, + }); + + // When: component renders + render(); + + // Then: cashback item is not rendered + expect( + screen.queryByTestId(CardHomeSelectors.CASHBACK_ITEM), + ).not.toBeOnTheScreen(); + }); + + it('hides cashback item when user is not authenticated', () => { + // Given: unauthenticated user + setupMockSelectors({ + isAuthenticated: false, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: false, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + }); + + // When: component renders + render(); + + // Then: cashback item is not rendered + expect( + screen.queryByTestId(CardHomeSelectors.CASHBACK_ITEM), + ).not.toBeOnTheScreen(); + }); + + it('hides cashback item when KYC is not verified', () => { + // Given: authenticated international user with pending KYC + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + kycStatus: { verificationState: 'PENDING', userId: 'user-123' }, + }); + + // When: component renders + render(); + + // Then: cashback item is not rendered + expect( + screen.queryByTestId(CardHomeSelectors.CASHBACK_ITEM), + ).not.toBeOnTheScreen(); + }); + + it('shows standard cashback description for virtual card', () => { + // Given: authenticated international user with virtual card + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + kycStatus: { verificationState: 'VERIFIED', userId: 'user-123' }, + }); + + // When: component renders + render(); + + // Then: standard description is shown + expect( + screen.getByText('Earn 1% back on all spending'), + ).toBeOnTheScreen(); + }); + + it('shows metal cashback description for metal card', () => { + // Given: authenticated international user with metal card + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.METAL }, + isLoading: false, + kycStatus: { verificationState: 'VERIFIED', userId: 'user-123' }, + }); + + // When: component renders + render(); + + // Then: metal description is shown + expect( + screen.getByText('Earn 3% back on all spending'), + ).toBeOnTheScreen(); + }); + + it('navigates to cashback screen on press', () => { + // Given: authenticated international user with verified KYC + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + kycStatus: { verificationState: 'VERIFIED', userId: 'user-123' }, + }); + + // When: user taps cashback item + render(); + fireEvent.press(screen.getByTestId(CardHomeSelectors.CASHBACK_ITEM)); + + // Then: navigates to cashback route + expect(mockNavigate).toHaveBeenCalled(); + }); + + it('tracks analytics event on press', () => { + // Given: authenticated international user with verified KYC + setupMockSelectors({ + isAuthenticated: true, + userLocation: 'international', + }); + setupLoadCardDataMock({ + isAuthenticated: true, + isBaanxLoginEnabled: true, + cardDetails: { type: CardType.VIRTUAL }, + isLoading: false, + kycStatus: { verificationState: 'VERIFIED', userId: 'user-123' }, + }); + + // When: user taps cashback item + render(); + fireEvent.press(screen.getByTestId(CardHomeSelectors.CASHBACK_ITEM)); + + // Then: tracks cashback button event + expect(mockTrackEvent).toHaveBeenCalled(); + }); + }); }); diff --git a/app/components/UI/Card/Views/CardHome/CardHome.tsx b/app/components/UI/Card/Views/CardHome/CardHome.tsx index 8ddfdbb1168..9b029c05bf6 100644 --- a/app/components/UI/Card/Views/CardHome/CardHome.tsx +++ b/app/components/UI/Card/Views/CardHome/CardHome.tsx @@ -708,24 +708,6 @@ const CardHome = () => { }); }, [navigation, trackEvent, createEventBuilder, userShippingAddress]); - const isUserEligibleForMetalCard = useMemo( - () => - isMetalCardCheckoutEnabled && - isBaanxLoginEnabled && - isAuthenticated && - userLocation === 'us' && - userShippingAddress && - cardDetails?.type === CardType.VIRTUAL, - [ - isMetalCardCheckoutEnabled, - isBaanxLoginEnabled, - isAuthenticated, - userLocation, - userShippingAddress, - cardDetails, - ], - ); - const showCardDetailsErrorToast = useCallback(() => { toastRef?.current?.showToast({ variant: ToastVariants.Icon, @@ -1044,6 +1026,31 @@ const CardHome = () => { isCardProvisioning, ]); + const isUserEligibleForMetalCard = useMemo( + () => + !isLoading && + !cardSetupState.isKYCPending && + !cardSetupState.needsSetup && + !isCardProvisioning && + isMetalCardCheckoutEnabled && + isBaanxLoginEnabled && + isAuthenticated && + userLocation === 'us' && + userShippingAddress && + cardDetails?.type === CardType.VIRTUAL, + [ + isMetalCardCheckoutEnabled, + isBaanxLoginEnabled, + isAuthenticated, + userLocation, + userShippingAddress, + cardDetails, + isLoading, + cardSetupState, + isCardProvisioning, + ], + ); + useEffect( () => () => { isComponentUnmountedRef.current = true; @@ -1428,6 +1435,19 @@ const CardHome = () => { )} + {isUserEligibleForMetalCard && ( + + )} {isAuthenticated && !isLoading && cardDetails && ( { onPress={navigateToCardPage} testID={CardHomeSelectors.ADVANCED_CARD_MANAGEMENT_ITEM} /> - {isUserEligibleForMetalCard && ( - - )} - {isAuthenticated && kycStatus?.verificationState === 'VERIFIED' && ( - { - trackEvent( - createEventBuilder(MetaMetricsEvents.CARD_BUTTON_CLICKED) - .addProperties({ - action: CardActions.CASHBACK_BUTTON, - }) - .build(), - ); - navigation.navigate(Routes.CARD.CASHBACK); - }} - testID={CardHomeSelectors.CASHBACK_ITEM} - /> - )} + {isAuthenticated && + kycStatus?.verificationState === 'VERIFIED' && + userLocation !== 'us' && ( + { + trackEvent( + createEventBuilder(MetaMetricsEvents.CARD_BUTTON_CLICKED) + .addProperties({ + action: CardActions.CASHBACK_BUTTON, + }) + .build(), + ); + navigation.navigate(Routes.CARD.CASHBACK); + }} + testID={CardHomeSelectors.CASHBACK_ITEM} + /> + )} ({ }, })); +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + goBack: mockGoBack, + }), + }; +}); + jest.mock('../../../../../util/theme', () => { const actual = jest.requireActual('../../../../../util/theme'); return { @@ -413,6 +424,51 @@ describe('Cashback Component', () => { ); }); + it('navigates back after successful withdrawal', () => { + mockHookReturn.cashbackWallet = { + id: 'w1', + balance: '10.00', + currency: 'musd', + isWithdrawable: true, + type: 'reward', + }; + mockHookReturn.monitoringStatus = 'success'; + + render(); + + expect(mockGoBack).toHaveBeenCalled(); + }); + + it('does not navigate back when monitoring fails', () => { + mockHookReturn.cashbackWallet = { + id: 'w1', + balance: '10.00', + currency: 'musd', + isWithdrawable: true, + type: 'reward', + }; + mockHookReturn.monitoringStatus = 'failed'; + + render(); + + expect(mockGoBack).not.toHaveBeenCalled(); + }); + + it('does not navigate back when status is idle', () => { + mockHookReturn.cashbackWallet = { + id: 'w1', + balance: '10.00', + currency: 'musd', + isWithdrawable: true, + type: 'reward', + }; + mockHookReturn.monitoringStatus = 'idle'; + + render(); + + expect(mockGoBack).not.toHaveBeenCalled(); + }); + it('shows failure toast when monitoring fails', () => { mockHookReturn.cashbackWallet = { id: 'w1', diff --git a/app/components/UI/Card/Views/Cashback/Cashback.tsx b/app/components/UI/Card/Views/Cashback/Cashback.tsx index 28f140a2912..4b1ccf0b42a 100644 --- a/app/components/UI/Card/Views/Cashback/Cashback.tsx +++ b/app/components/UI/Card/Views/Cashback/Cashback.tsx @@ -5,11 +5,11 @@ import { Text, TextVariant, TextColor, + Skeleton, } from '@metamask/design-system-react-native'; import { IconName } from '../../../../../component-library/components/Icons/Icon'; import { useTailwind } from '@metamask/design-system-twrnc-preset'; import { useTheme } from '../../../../../util/theme'; -import { Skeleton } from '../../../../../component-library/components/Skeleton'; import { strings } from '../../../../../../locales/i18n'; import Button, { ButtonSize, @@ -26,6 +26,7 @@ import { useAnalytics } from '../../../../hooks/useAnalytics/useAnalytics'; import { MetaMetricsEvents } from '../../../../../core/Analytics'; import { CardActions } from '../../util/metrics'; import { CashbackSelectors } from './Cashback.testIds'; +import { useNavigation } from '@react-navigation/native'; const CURRENCY_DISPLAY_MAP: Record = { musd: 'mUSD', @@ -45,6 +46,7 @@ const formatAmount = (value: string | number): string => { }; const Cashback: React.FC = () => { + const { goBack } = useNavigation(); const tw = useTailwind(); const theme = useTheme(); const { toastRef } = useContext(ToastContext); @@ -94,8 +96,9 @@ const Cashback: React.FC = () => { iconColor: theme.colors.success.default, hasNoTimeout: false, }); + goBack(); } - }, [monitoringStatus, toastRef, theme]); + }, [monitoringStatus, toastRef, theme, goBack]); useEffect(() => { if (monitoringStatus === 'failed' || monitoringError) { @@ -189,7 +192,7 @@ const Cashback: React.FC = () => { {error ? ( - + { ) : ( diff --git a/locales/languages/en.json b/locales/languages/en.json index daa57076d80..00671cb779b 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -7233,6 +7233,7 @@ "order_metal_card_description": "Order your physical Metal Card now", "cashback": "Cashback", "cashback_description": "Earn 1% back on all spending", + "cashback_description_metal": "Earn 3% back on all spending", "freeze_card": "Freeze card", "unfreeze_card": "Unfreeze card", "freeze_card_description": "Pause all spending on your card", From 0f45f2cc4de554c267f53c84628fc718dc959ae4 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Wed, 4 Mar 2026 21:05:49 +0530 Subject: [PATCH 08/15] fix: gas_paid_with metrics parameter for MMPay transactions (#26778) ## **Description** Fix metrics parameter `gas_paid_with` for MMPay transactions. ## **Changelog** CHANGELOG entry: ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/6994 ## **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** NA ## **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] > **Low Risk** > Low risk metrics-only change: updates how `gas_paid_with`/native-balance checks are derived for MM Pay transactions and adds a focused unit test to prevent regressions. > > **Overview** > Fixes gas metrics derivation for **MM Pay** transactions by preferring `transactionMeta.metamaskPay.tokenAddress`/`chainId` over `selectedGasFeeToken`/`chainId` when computing `gas_paid_with` and `gas_insufficient_native_asset`. > > Adds a unit test ensuring `gas_paid_with` resolves to the symbol for the MM Pay token even when `selectedGasFeeToken` differs. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b4fe282c89fdb3f4ea3b2a6e6af2d77049aa7874. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../metrics_properties/gas.test.ts | 20 +++++++++++++++++++ .../metrics_properties/gas.ts | 12 ++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.test.ts b/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.test.ts index bfca61f71ea..9b3f47537c8 100644 --- a/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.test.ts +++ b/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.test.ts @@ -165,4 +165,24 @@ describe('getGasMetricsProperties', () => { expect(result.properties.gas_insufficient_native_asset).toBe(false); }); + + describe('MM Pay transactions', () => { + it('derives gas_paid_with from metamaskPay.tokenAddress instead of selectedGasFeeToken', () => { + const request = createMockRequest({ + selectedGasFeeToken: '0xignored', + metamaskPay: { + chainId: '0x1', + tokenAddress: '0xusdc', + }, + gasFeeTokens: [ + { symbol: 'USDC', tokenAddress: '0xusdc' }, + { symbol: 'IGNORED', tokenAddress: '0xignored' }, + ] as unknown as TransactionMeta['gasFeeTokens'], + }); + + const result = getGasMetricsProperties(request); + + expect(result.properties.gas_paid_with).toBe('USDC'); + }); + }); }); diff --git a/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.ts b/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.ts index 3cc816d2945..a03cce2f579 100644 --- a/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.ts +++ b/app/core/Engine/controllers/transaction-controller/metrics_properties/gas.ts @@ -40,19 +40,25 @@ export function getGasMetricsProperties({ (token) => token.symbol, ); + const { metamaskPay } = transactionMeta; + const gasFeeTokenAddress = metamaskPay?.tokenAddress ?? selectedGasFeeToken; + const gasFeeChainId = metamaskPay?.chainId ?? chainId; + let gas_paid_with = gasFeeTokens?.find( (token) => - token.tokenAddress.toLowerCase() === selectedGasFeeToken?.toLowerCase(), + token.tokenAddress.toLowerCase() === gasFeeTokenAddress?.toLowerCase(), )?.symbol; - if (selectedGasFeeToken?.toLowerCase() === getNativeTokenAddress(chainId)) { + if ( + gasFeeTokenAddress?.toLowerCase() === getNativeTokenAddress(gasFeeChainId) + ) { gas_paid_with = 'pre-funded_ETH'; } const state = getState(); const gas_insufficient_native_asset = getNativeBalance( state, - chainId, + gasFeeChainId, from, ).lt(getMaxGasCost(transactionMeta)); From 0d8e38241b59c123b7cc1ea286cb5286b6b26148 Mon Sep 17 00:00:00 2001 From: Pedro Pablo Aste Kompen Date: Wed, 4 Mar 2026 12:39:15 -0300 Subject: [PATCH 09/15] fix(TMCU-508): trigger NFT detection on homepage focus (#26919) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** When the homepage was redesigned from tabs to sections, the `detectNfts` call that ran when the NFTs tab was visited was lost. This PR ports over the missing behavior by using `useFocusEffect` in `NFTsSection` to trigger `detectNfts` (via the existing `useNftDetection` hook) whenever the Homepage screen gains focus. Detection is aborted on blur/unmount via the effect's cleanup function. ## **Changelog** CHANGELOG entry: Fixed missing NFT detection on homepage focus, restoring auto-detection when the user navigates to the homepage ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-508 ## **Manual testing steps** ```gherkin Feature: NFT detection on homepage focus Scenario: user navigates to homepage and NFTs are detected Given user has NFT-owning accounts configured And the homepage is not currently focused When user navigates to the homepage Then NFT detection is triggered automatically for configured chains Scenario: user navigates away from homepage and detection is aborted Given user is on the homepage and NFT detection is in progress When user navigates away from the homepage Then the in-flight NFT detection is aborted ``` ## **Screenshots/Recordings** N/A — no UI changes, behavioral fix only. ### **Before** NFT detection was not triggered when navigating to the homepage. ### **After** NFT detection runs on homepage focus and is aborted on blur. ## **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] > **Medium Risk** > Adds focus-driven side effects that start/cancel NFT detection, which can impact network activity and loading UI timing when navigating between screens. > > **Overview** > Restores missing NFT auto-detection on the redesigned Homepage by running `useNftDetection().detectNfts()` from `NFTsSection` via `useFocusEffect`, and cancelling via `abortDetection()` on blur/unmount. > > Adjusts loading UI to avoid showing the skeleton during *silent* re-detection after the first successful load, and adds/updates tests to mock `useFocusEffect` and assert detection/abort behavior (including rejected detection). > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 500bdab4415f213d88b2e999596fcfcf80e54f8b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../Views/Homepage/Homepage.test.tsx | 15 ++++++++ .../Sections/NFTs/NFTsSection.test.tsx | 38 +++++++++++++++++++ .../Homepage/Sections/NFTs/NFTsSection.tsx | 31 ++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/app/components/Views/Homepage/Homepage.test.tsx b/app/components/Views/Homepage/Homepage.test.tsx index 3450f18ef7f..d1b945e47c3 100644 --- a/app/components/Views/Homepage/Homepage.test.tsx +++ b/app/components/Views/Homepage/Homepage.test.tsx @@ -13,9 +13,24 @@ jest.mock('@react-navigation/native', () => { useNavigation: () => ({ navigate: jest.fn(), }), + useFocusEffect: (callback: () => void) => { + const React = jest.requireActual('react'); + React.useEffect(callback, [callback]); + }, }; }); +const mockDetectNfts = jest.fn().mockResolvedValue(undefined); +const mockAbortDetection = jest.fn(); + +jest.mock('../../hooks/useNftDetection', () => ({ + useNftDetection: () => ({ + detectNfts: mockDetectNfts, + abortDetection: mockAbortDetection, + chainIdsToDetectNftsFor: [], + }), +})); + // Mock feature flags - enable all sections jest.mock('../../UI/Perps', () => ({ selectPerpsEnabledFlag: jest.fn(() => true), diff --git a/app/components/Views/Homepage/Sections/NFTs/NFTsSection.test.tsx b/app/components/Views/Homepage/Sections/NFTs/NFTsSection.test.tsx index b61f2e122fd..1690687e378 100644 --- a/app/components/Views/Homepage/Sections/NFTs/NFTsSection.test.tsx +++ b/app/components/Views/Homepage/Sections/NFTs/NFTsSection.test.tsx @@ -8,6 +8,8 @@ import { SectionRefreshHandle } from '../../types'; const mockNavigate = jest.fn(); const mockOnRefresh = jest.fn().mockResolvedValue(undefined); +const mockDetectNfts = jest.fn().mockResolvedValue(undefined); +const mockAbortDetection = jest.fn(); jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); @@ -16,6 +18,10 @@ jest.mock('@react-navigation/native', () => { useNavigation: () => ({ navigate: mockNavigate, }), + useFocusEffect: (callback: () => void) => { + const React = jest.requireActual('react'); + React.useEffect(callback, [callback]); + }, }; }); @@ -23,6 +29,14 @@ jest.mock('../../../../../reducers/collectibles', () => ({ isNftFetchingProgressSelector: jest.fn(() => false), })); +jest.mock('../../../../hooks/useNftDetection', () => ({ + useNftDetection: () => ({ + detectNfts: mockDetectNfts, + abortDetection: mockAbortDetection, + chainIdsToDetectNftsFor: [], + }), +})); + jest.mock('../../../../UI/NftGrid/useNftRefresh', () => ({ useNftRefresh: () => ({ refreshing: false, @@ -139,6 +153,30 @@ describe('NFTsSection', () => { expect(screen.queryByText('NFT 7')).not.toBeOnTheScreen(); }); + it('triggers NFT detection on focus', () => { + renderWithProvider(); + + expect(mockDetectNfts).toHaveBeenCalledTimes(1); + }); + + it('calls abortDetection on unmount', () => { + const { unmount } = renderWithProvider(); + + unmount(); + + expect(mockAbortDetection).toHaveBeenCalledTimes(1); + }); + + it('renders without error when detectNfts rejects', async () => { + mockDetectNfts.mockRejectedValueOnce(new Error('Aborted')); + + renderWithProvider(); + + await act(async () => undefined); + + expect(screen.getByText('NFTs')).toBeOnTheScreen(); + }); + it('exposes refresh function via ref that calls useNftRefresh.onRefresh', async () => { const ref = createRef(); diff --git a/app/components/Views/Homepage/Sections/NFTs/NFTsSection.tsx b/app/components/Views/Homepage/Sections/NFTs/NFTsSection.tsx index 201ce697804..b49c0e7dc0e 100644 --- a/app/components/Views/Homepage/Sections/NFTs/NFTsSection.tsx +++ b/app/components/Views/Homepage/Sections/NFTs/NFTsSection.tsx @@ -3,11 +3,12 @@ import React, { useCallback, useImperativeHandle, useMemo, + useRef, useState, } from 'react'; import { View } from 'react-native'; import { useSelector } from 'react-redux'; -import { useNavigation } from '@react-navigation/native'; +import { useFocusEffect, useNavigation } from '@react-navigation/native'; import SkeletonPlaceholder from 'react-native-skeleton-placeholder'; import { useTailwind } from '@metamask/design-system-twrnc-preset'; import { useTheme } from '../../../../../util/theme'; @@ -19,6 +20,7 @@ import { useOwnedNfts } from './hooks'; import NftGridItem from '../../../../UI/NftGrid/NftGridItem'; import { useNftRefresh } from '../../../../UI/NftGrid/useNftRefresh'; import { CollectiblesEmptyState } from '../../../../UI/CollectiblesEmptyState/CollectiblesEmptyState'; +import { useNftDetection } from '../../../../hooks/useNftDetection'; import { SectionRefreshHandle } from '../../types'; import { strings } from '../../../../../../locales/i18n'; import { isNftFetchingProgressSelector } from '../../../../../reducers/collectibles'; @@ -59,6 +61,31 @@ const NFTsSection = forwardRef((_, ref) => { const hasNfts = ownedNfts.length > 0; const isNftFetchingProgress = useSelector(isNftFetchingProgressSelector); const { onRefresh } = useNftRefresh(); + const { detectNfts, abortDetection } = useNftDetection(); + const hasLoadedOnceRef = useRef(false); + const isSilentDetectionRef = useRef(false); + + useFocusEffect( + useCallback(() => { + isSilentDetectionRef.current = hasLoadedOnceRef.current; + + detectNfts() + .catch(() => { + // AbortError is expected when detection is cancelled on blur + }) + .finally(() => { + hasLoadedOnceRef.current = true; + isSilentDetectionRef.current = false; + }); + + return () => { + abortDetection(); + isSilentDetectionRef.current = false; + }; + }, [detectNfts, abortDetection]), + ); + + const showSkeleton = isNftFetchingProgress && !isSilentDetectionRef.current; const title = strings('homepage.sections.nfts'); @@ -128,7 +155,7 @@ const NFTsSection = forwardRef((_, ref) => { ))} - ) : isNftFetchingProgress ? ( + ) : showSkeleton ? ( From 4b57dcdeacb085c4ce41a33bb9ef3cbc2d26b7d2 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 4 Mar 2026 16:55:58 +0100 Subject: [PATCH 10/15] chore: Bump `snaps-controllers` cp-7.67.2 (#26992) ## **Description** Bump `snaps-controllers` to the latest version which includes a mitigation for production issues where the source code of Snaps is unavailable. ## **Changelog** CHANGELOG entry: Fixed an issue with running Snaps ## **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] > **Medium Risk** > Updates a core Snaps dependency used at runtime; even a patch bump can affect Snap execution/permissions and should be regression-tested with common Snaps flows. > > **Overview** > Bumps `@metamask/snaps-controllers` from `^18.0.1` to `^18.0.2` (with corresponding `yarn.lock` updates) to pick up the latest fixes. > > Includes a small transitive dependency update within that package (notably `@metamask/json-rpc-engine` to `^10.2.3`). > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 67b823f187c316717d04e2c63f0ea6e5733b661e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- package.json | 2 +- yarn.lock | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 75d3424196a..00cc0c68aec 100644 --- a/package.json +++ b/package.json @@ -284,7 +284,7 @@ "@metamask/signature-controller": "^35.0.0", "@metamask/slip44": "^4.2.0", "@metamask/smart-transactions-controller": "^22.6.0", - "@metamask/snaps-controllers": "^18.0.1", + "@metamask/snaps-controllers": "^18.0.2", "@metamask/snaps-execution-environments": "^11.0.0", "@metamask/snaps-rpc-methods": "^14.3.0", "@metamask/snaps-sdk": "^10.4.0", diff --git a/yarn.lock b/yarn.lock index 5c9c46381f4..28be25e2923 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9737,13 +9737,13 @@ __metadata: languageName: node linkType: hard -"@metamask/snaps-controllers@npm:^18.0.1": - version: 18.0.1 - resolution: "@metamask/snaps-controllers@npm:18.0.1" +"@metamask/snaps-controllers@npm:^18.0.2": + version: 18.0.2 + resolution: "@metamask/snaps-controllers@npm:18.0.2" dependencies: "@metamask/approval-controller": "npm:^8.0.0" "@metamask/base-controller": "npm:^9.0.0" - "@metamask/json-rpc-engine": "npm:^10.2.2" + "@metamask/json-rpc-engine": "npm:^10.2.3" "@metamask/json-rpc-middleware-stream": "npm:^8.0.8" "@metamask/key-tree": "npm:^10.1.1" "@metamask/messenger": "npm:^0.3.0" @@ -9776,7 +9776,7 @@ __metadata: peerDependenciesMeta: "@metamask/snaps-execution-environments": optional: true - checksum: 10/0b9c3f8097cda0621020bde7e63a74a20f7623a9926d2d102942df56f9b9fefbf0900f7a3a17b175f8648b1b93fc373f5c736ee88e517ee624ed6161985742cb + checksum: 10/3d8f88ff926b2918b1632dc9920c94871df85146a66d4d6dbb8fb31ee241c7012e85d86bdf30be47aec97fc2fecf724f5e235f30a1550bedf4084586175e05eb languageName: node linkType: hard @@ -35470,7 +35470,7 @@ __metadata: "@metamask/signature-controller": "npm:^35.0.0" "@metamask/slip44": "npm:^4.2.0" "@metamask/smart-transactions-controller": "npm:^22.6.0" - "@metamask/snaps-controllers": "npm:^18.0.1" + "@metamask/snaps-controllers": "npm:^18.0.2" "@metamask/snaps-execution-environments": "npm:^11.0.0" "@metamask/snaps-rpc-methods": "npm:^14.3.0" "@metamask/snaps-sdk": "npm:^10.4.0" From 02ac109fb36974ecfe3dbbdecc15b529bc2a8d17 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 4 Mar 2026 17:01:21 +0100 Subject: [PATCH 11/15] fix: Ensure `redux-persist-filesystem-storage` returns a promise and throws correctly cp-7.67.2 (#26979) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR adjusts our existing patch for `redux-persist-filesystem-storage` to always return a promise for `setItem` so it can be awaited correctly, fixing potential race conditions. This was broken when the patch originally was created by adding brackets to an arrow function and not returning. The PR also fixes a bug where not passing in a callback would cause errors to be swallowed. Additionally, this PR moves our existing patch to use the built-in Yarn patch feature (because creating new patches with `patch-package` seems broken). ## **Changelog** CHANGELOG entry: Improve reliability of persistence ## **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] > **Medium Risk** > Touches persistence write behavior by changing the patched `redux-persist-filesystem-storage` `setItem` control flow and error propagation, which could affect app startup/migrations if any callers depend on the old callback-only semantics. Also introduces an iOS-only side effect (exclude-from-backup) after writes. > > **Overview** > This PR updates the Yarn patch for `redux-persist-filesystem-storage` so `setItem` **always returns a Promise** that can be awaited, rather than potentially relying on callback chaining. > > It also fixes error handling so write failures are **thrown when no callback is provided** (instead of being swallowed), and adds an optional `isIOS` flag to run `ReactNativeBlobUtil.ios.excludeFromBackupKey(...)` after successful writes on iOS. > > Dependency wiring is switched from a normal semver dependency to Yarn’s built-in `patch:` reference for `redux-persist-filesystem-storage@4.2.0`, with corresponding `yarn.lock` updates. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 192a5c846d0e01487929a506d71542523b2a96cf. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- ...esystem-storage-npm-4.2.0-3a6fff24ab.patch | 28 +++++++++++-------- package.json | 2 +- yarn.lock | 13 +++++++-- 3 files changed, 29 insertions(+), 14 deletions(-) rename patches/redux-persist-filesystem-storage+4.2.0.patch => .yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch (56%) diff --git a/patches/redux-persist-filesystem-storage+4.2.0.patch b/.yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch similarity index 56% rename from patches/redux-persist-filesystem-storage+4.2.0.patch rename to .yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch index 37aa4a293c5..dda65f1830d 100644 --- a/patches/redux-persist-filesystem-storage+4.2.0.patch +++ b/.yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch @@ -1,7 +1,7 @@ -diff --git a/node_modules/redux-persist-filesystem-storage/index.d.ts b/node_modules/redux-persist-filesystem-storage/index.d.ts -index b0caa94..76b0442 100644 ---- a/node_modules/redux-persist-filesystem-storage/index.d.ts -+++ b/node_modules/redux-persist-filesystem-storage/index.d.ts +diff --git a/index.d.ts b/index.d.ts +index b0caa94ceaa9afaa6c112947a328887e580f76a2..76b0442a7367f39d8ae9300825815edda5b02c44 100644 +--- a/index.d.ts ++++ b/index.d.ts @@ -12,6 +12,7 @@ declare module 'redux-persist-filesystem-storage' { setItem: ( key: string, @@ -10,24 +10,30 @@ index b0caa94..76b0442 100644 callback?: (error?: Error) => void, ) => Promise -diff --git a/node_modules/redux-persist-filesystem-storage/index.js b/node_modules/redux-persist-filesystem-storage/index.js -index d69afb6..0ca3a25 100644 ---- a/node_modules/redux-persist-filesystem-storage/index.js -+++ b/node_modules/redux-persist-filesystem-storage/index.js -@@ -41,11 +41,14 @@ const FilesystemStorage = { +diff --git a/index.js b/index.js +index d69afb678b3d06760ad59831457cfd5c51fdb89b..8d5ecb060a91f137c865ed21f891b640d3cc65fe 100644 +--- a/index.js ++++ b/index.js +@@ -41,11 +41,19 @@ const FilesystemStorage = { onStorageReady = onStorageReadyFactory(options.storagePath); }, - setItem: (key: string, value: string, callback?: (error: ?Error) => void) => +- ReactNativeBlobUtil.fs + setItem: (key: string, value: string, isIOS: boolean = false, callback?: (error: ?Error) => void) => { - ReactNativeBlobUtil.fs ++ return ReactNativeBlobUtil.fs .writeFile(pathForKey(key), value, options.encoding) - .then(() => callback && callback()) - .catch(error => callback && callback(error)), + .then(() => { + if (isIOS) ReactNativeBlobUtil.ios.excludeFromBackupKey(pathForKey(key)); + callback && callback(); -+ }).catch(error => callback && callback(error)); ++ }).catch((error) => { ++ if (!callback) { ++ throw error; ++ } ++ callback(error); ++ }); + }, getItem: onStorageReady( diff --git a/package.json b/package.json index 00cc0c68aec..46e548e2cc6 100644 --- a/package.json +++ b/package.json @@ -478,7 +478,7 @@ "redux": "^4.2.1", "redux-mock-store": "1.5.4", "redux-persist": "6.0.0", - "redux-persist-filesystem-storage": "^4.2.0", + "redux-persist-filesystem-storage": "patch:redux-persist-filesystem-storage@npm%3A4.2.0#~/.yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch", "redux-saga": "^1.3.0", "redux-thunk": "^2.4.2", "reselect": "^5.1.1", diff --git a/yarn.lock b/yarn.lock index 28be25e2923..b85fd93e7be 100644 --- a/yarn.lock +++ b/yarn.lock @@ -35775,7 +35775,7 @@ __metadata: redux-devtools-expo-dev-plugin: "npm:^1.0.0" redux-mock-store: "npm:1.5.4" redux-persist: "npm:6.0.0" - redux-persist-filesystem-storage: "npm:^4.2.0" + redux-persist-filesystem-storage: "patch:redux-persist-filesystem-storage@npm%3A4.2.0#~/.yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch" redux-saga: "npm:^1.3.0" redux-saga-test-plan: "npm:^4.0.6" redux-thunk: "npm:^2.4.2" @@ -41517,7 +41517,7 @@ __metadata: languageName: node linkType: hard -"redux-persist-filesystem-storage@npm:^4.2.0": +"redux-persist-filesystem-storage@npm:4.2.0": version: 4.2.0 resolution: "redux-persist-filesystem-storage@npm:4.2.0" dependencies: @@ -41526,6 +41526,15 @@ __metadata: languageName: node linkType: hard +"redux-persist-filesystem-storage@patch:redux-persist-filesystem-storage@npm%3A4.2.0#~/.yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch": + version: 4.2.0 + resolution: "redux-persist-filesystem-storage@patch:redux-persist-filesystem-storage@npm%3A4.2.0#~/.yarn/patches/redux-persist-filesystem-storage-npm-4.2.0-3a6fff24ab.patch::version=4.2.0&hash=4dfd27" + dependencies: + react-native-blob-util: "npm:^0.18.0" + checksum: 10/fa556e2d1784a5e664e2e7024fa2255b08334e0dacf8993acca676cb912ad82c0f8ef3ba9ec2597d455f9dded83acf3343cc0a66d4e2fc14486e31dd9efe6def + languageName: node + linkType: hard + "redux-persist@npm:6.0.0": version: 6.0.0 resolution: "redux-persist@npm:6.0.0" From 1b155cd5fcc922eab2b7799f199f5085f1e3ae38 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 4 Mar 2026 17:19:31 +0100 Subject: [PATCH 12/15] chore: Update a message for Withdraw: Not enough POL (#27001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The error message shown when a user has insufficient native token (e.g. POL) to cover gas fees during a **Predict Withdraw** was misleading. It suggested switching to a token on another network as a recovery option — which is valid for deposits but not for withdrawals, where the source funds come from the withdrawal transaction itself. This PR introduces a dedicated copy key for post-quote (withdrawal) flows that omits the network-switch hint, and wires it up in the alert hook. ## **Changelog** CHANGELOG entry: Fixed incorrect error message shown when there is not enough native token to cover gas fees during a withdrawal. ## **Related issues** Fixes: #26926 ## **Manual testing steps** ```gherkin Feature: Insufficient native token error message on Predict Withdraw Scenario: user sees correct error message when POL balance is too low to cover gas on withdrawal Given the user has a Predict account with a balance And the user has no POL (or insufficient POL) on their EOA When the user navigates to Withdraw and enters an amount Then the error message reads "Not enough POL to cover fees. Add POL to continue." And the message does NOT mention "Use a token on another network" Scenario: deposit flow still shows the full error message Given the user has insufficient native token to cover gas on a deposit When the user reviews the deposit confirmation Then the error message reads "Not enough to cover fees. Use a token on another network or add more to continue." ``` ## **Screenshots/Recordings** ### **Before** > Not enough POL to cover fees. Use a token on another network or add POL to continue. ### **After** > Not enough POL to cover fees. Add POL to continue. 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] > **Low Risk** > Low risk: this only changes which i18n string is used for an existing blocking alert, plus adds test coverage; no transaction logic or state mutations are altered. > > **Overview** > Updates the insufficient native-token gas-fee alert copy for *post-quote (withdrawal)* flows to remove the misleading “switch network” hint. > > Adds a new i18n key `alert_system.insufficient_pay_token_native_post_quote.message` and switches `useInsufficientPayTokenBalanceAlert` to choose between the standard vs. post-quote message based on `isPostQuote`, with tests updated to assert both behaviors. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit df4ee3ae9f432de5f2532f99b888a962812d3643. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). Signed-off-by: dan437 <80175477+dan437@users.noreply.github.com> --- ...seInsufficientPayTokenBalanceAlert.test.ts | 29 +++++++++++++++++-- .../useInsufficientPayTokenBalanceAlert.ts | 5 +++- locales/languages/en.json | 4 +++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.test.ts b/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.test.ts index 4af8eb287dc..f2504d63c25 100644 --- a/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.test.ts +++ b/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.test.ts @@ -358,6 +358,29 @@ describe('useInsufficientPayTokenBalanceAlert', () => { expect(result.current).toStrictEqual([]); }); + + it('uses the standard message (with network switch hint) for non-post-quote flows', () => { + useTokenWithBalanceMock.mockReturnValue({ + ...NATIVE_TOKEN_MOCK, + balanceRaw: '99', + } as ReturnType); + + const { result } = runHook(); + + expect(result.current).toStrictEqual([ + { + key: AlertKeys.InsufficientPayTokenNative, + field: RowAlertKey.Amount, + isBlocking: true, + title: strings('alert_system.insufficient_pay_token_balance.message'), + message: strings( + 'alert_system.insufficient_pay_token_native.message', + { ticker: 'ETH' }, + ), + severity: Severity.Danger, + }, + ]); + }); }); describe('for post-quote (withdrawal) flows', () => { @@ -461,7 +484,7 @@ describe('useInsufficientPayTokenBalanceAlert', () => { isBlocking: true, title: strings('alert_system.insufficient_pay_token_balance.message'), message: strings( - 'alert_system.insufficient_pay_token_native.message', + 'alert_system.insufficient_pay_token_native_post_quote.message', { ticker: 'POL' }, ), severity: Severity.Danger, @@ -491,7 +514,7 @@ describe('useInsufficientPayTokenBalanceAlert', () => { isBlocking: true, title: strings('alert_system.insufficient_pay_token_balance.message'), message: strings( - 'alert_system.insufficient_pay_token_native.message', + 'alert_system.insufficient_pay_token_native_post_quote.message', { ticker: 'POL' }, ), severity: Severity.Danger, @@ -528,7 +551,7 @@ describe('useInsufficientPayTokenBalanceAlert', () => { isBlocking: true, title: strings('alert_system.insufficient_pay_token_balance.message'), message: strings( - 'alert_system.insufficient_pay_token_native.message', + 'alert_system.insufficient_pay_token_native_post_quote.message', { ticker: 'POL' }, ), severity: Severity.Danger, diff --git a/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.ts b/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.ts index fc9a46635f3..f53788c2af2 100644 --- a/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.ts +++ b/app/components/Views/confirmations/hooks/alerts/useInsufficientPayTokenBalanceAlert.ts @@ -179,7 +179,9 @@ export function useInsufficientPayTokenBalanceAlert({ key: AlertKeys.InsufficientPayTokenNative, title: strings('alert_system.insufficient_pay_token_balance.message'), message: strings( - 'alert_system.insufficient_pay_token_native.message', + isPostQuote + ? 'alert_system.insufficient_pay_token_native_post_quote.message' + : 'alert_system.insufficient_pay_token_native.message', { ticker }, ), }, @@ -191,6 +193,7 @@ export function useInsufficientPayTokenBalanceAlert({ isInsufficientForInput, isInsufficientForFees, isInsufficientForSourceNetwork, + isPostQuote, ticker, ]); } diff --git a/locales/languages/en.json b/locales/languages/en.json index 00671cb779b..31aae4b370b 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -103,6 +103,10 @@ "message": "Not enough {{ticker}} to cover fees. Use a token on another network or add more {{ticker}} to continue.", "title": "Insufficient funds" }, + "insufficient_pay_token_native_post_quote": { + "message": "Not enough {{ticker}} to cover fees. Add {{ticker}} to continue.", + "title": "Insufficient funds for post quote" + }, "no_pay_token_quotes": { "message": "This payment route isn't available right now. Try changing the amount, network, or token and we'll find the best option.", "title": "No quotes" From b140347f5bad10d8c6d9361797adf7820b83b003 Mon Sep 17 00:00:00 2001 From: AxelGes <34173844+AxelGes@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:46:41 -0300 Subject: [PATCH 13/15] chore: bump @metamask/ramps-controller to ^10.2.0 (#27002) ## **Description** Bumps `@metamask/ramps-controller` from the preview version (`10.0.0-preview-225638478`) to the stable release `^10.2.0`. This also removes the preview registry override that was previously set in the `resolutions` field of `package.json`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Ramps controller upgrade Scenario: user accesses buy/sell ramps flow Given the app is installed and the user is logged in When user navigates to the buy or sell flow Then the ramps flow loads correctly with no regression ``` ## **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. --- package.json | 5 ++--- yarn.lock | 10 +++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 46e548e2cc6..1885e20fc9f 100644 --- a/package.json +++ b/package.json @@ -175,8 +175,7 @@ "bn.js@npm:4.11.6": "4.12.3", "bn.js@npm:5.2.1": "5.2.3", "@metamask/bridge-controller@npm:^67.1.1": "patch:@metamask/bridge-controller@npm%3A67.2.0#~/.yarn/patches/@metamask-bridge-controller-npm-67.2.0-32d3aafe1f.patch", - "@metamask/bridge-status-controller@npm:^67.0.1": "patch:@metamask/bridge-status-controller@npm%3A67.0.1#~/.yarn/patches/@metamask-bridge-status-controller-npm-67.0.1-d8a41d9c02.patch", - "@metamask/ramps-controller": "npm:@metamask-previews/ramps-controller@10.0.0-preview-225638478" + "@metamask/bridge-status-controller@npm:^67.0.1": "patch:@metamask/bridge-status-controller@npm%3A67.0.1#~/.yarn/patches/@metamask-bridge-status-controller-npm-67.0.1-d8a41d9c02.patch" }, "dependencies": { "@config-plugins/detox": "^9.0.0", @@ -266,7 +265,7 @@ "@metamask/preinstalled-example-snap": "^0.7.2", "@metamask/profile-metrics-controller": "^2.0.0", "@metamask/profile-sync-controller": "^27.1.0", - "@metamask/ramps-controller": "^10.0.0", + "@metamask/ramps-controller": "^10.2.0", "@metamask/react-native-acm": "^1.0.1", "@metamask/react-native-actionsheet": "2.4.2", "@metamask/react-native-button": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index b85fd93e7be..707d47d9748 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9430,14 +9430,14 @@ __metadata: languageName: node linkType: hard -"@metamask/ramps-controller@npm:@metamask-previews/ramps-controller@10.0.0-preview-225638478": - version: 10.0.0-preview-225638478 - resolution: "@metamask-previews/ramps-controller@npm:10.0.0-preview-225638478" +"@metamask/ramps-controller@npm:^10.2.0": + version: 10.2.0 + resolution: "@metamask/ramps-controller@npm:10.2.0" dependencies: "@metamask/base-controller": "npm:^9.0.0" "@metamask/controller-utils": "npm:^11.19.0" "@metamask/messenger": "npm:^0.3.0" - checksum: 10/dc093dacbe55ee20cbb50cc9b1c9649469b4c115f35db6b0f7a020610742955c6f35ee5e1f74ba079769c481c0210316ffd23e775bd03b1e83798530aaf142cb + checksum: 10/4c9e10f3948a4e0f44f3a98fd2a7a220585e74793ad4cc899b27be6ea3c428c76fb95b0987697a0dd62a98221868ce2bfb3e40495cef00f4909e5dd88dec152e languageName: node linkType: hard @@ -35452,7 +35452,7 @@ __metadata: "@metamask/profile-metrics-controller": "npm:^2.0.0" "@metamask/profile-sync-controller": "npm:^27.1.0" "@metamask/providers": "npm:^18.3.1" - "@metamask/ramps-controller": "npm:^10.0.0" + "@metamask/ramps-controller": "npm:^10.2.0" "@metamask/react-native-acm": "npm:^1.0.1" "@metamask/react-native-actionsheet": "npm:2.4.2" "@metamask/react-native-button": "npm:^3.0.0" From 92b79197da259416bd9799fd069fb49e2e16cc1d Mon Sep 17 00:00:00 2001 From: Nico MASSART Date: Wed, 4 Mar 2026 18:21:46 +0100 Subject: [PATCH 14/15] feat(devex): add Cursor worktree setup configuration (#26986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** 1. **Reason**: When using Cursor's worktree feature to run agents in separate checkouts, each worktree needs the same env files and plan files as the main repo so that install/build and agent context work correctly. 2. **Solution**: - Add `.cursor/worktrees.json` so that when Cursor creates a worktree - Add setup commands that copy `.js.env`, `.ios.env`, `.android.env`, `.e2e.env`, and `.cursor/plans` (recursively as it's a folder) from the root repos into the new worktree. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: N/A ## **Manual testing steps** ```gherkin Feature: Cursor worktree setup Scenario: worktree is created with env and plans copied Given the repo has .cursor/worktrees.json configured When dev creates a new agent tab using "worktree" agent location And Cursor creates a new worktree Then .js.env, .ios.env, .android.env, .e2e.env and .cursor/plans (and content) exist in the worktree ``` ## **Screenshots/Recordings** The worktree menu available when creating a new local chat agent: image ## **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] > **Low Risk** > Low risk developer-experience change that only adds a Cursor configuration file and does not affect runtime app behavior. Main risk is overwriting local env/plan files in worktrees if users have customized them. > > **Overview** > Adds `.cursor/worktrees.json` to configure Cursor’s *worktree* setup so new worktrees automatically copy `.js.env`, `.ios.env`, `.android.env`, `.e2e.env`, and `.cursor/plans` from the root worktree into the new checkout. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 85347a6efbf422e98b0806c79794d57ee7d5d4cd. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .cursor/worktrees.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .cursor/worktrees.json diff --git a/.cursor/worktrees.json b/.cursor/worktrees.json new file mode 100644 index 00000000000..cbb8fe98f1f --- /dev/null +++ b/.cursor/worktrees.json @@ -0,0 +1,9 @@ +{ + "setup-worktree": [ + "cp $ROOT_WORKTREE_PATH/.js.env .js.env", + "cp $ROOT_WORKTREE_PATH/.ios.env .ios.env", + "cp $ROOT_WORKTREE_PATH/.android.env .android.env", + "cp $ROOT_WORKTREE_PATH/.e2e.env .e2e.env", + "cp -r $ROOT_WORKTREE_PATH/.cursor/plans .cursor/plans" + ] +} From 2f8713ebc3e738b9496cbe7b42b9dd4ca548d37a Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Wed, 4 Mar 2026 17:26:28 +0000 Subject: [PATCH 15/15] fix: sourcemap upload to bitrise for react native profiler usage (#26995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** revert of https://github.com/MetaMask/metamask-mobile/pull/25793 (only the part where sourcemaps are not uploaded to bitrise) ## **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] > **Medium Risk** > Changes CI build artifacts and iOS bundling script paths; risk is mainly build/pipeline breakage or missing sourcemaps if paths/envs differ across workflows. > > **Overview** > Restores/introduces Bitrise deployment of React Native sourcemaps for **Android** and **iOS** builds so they’re available as CI artifacts (and can be used for profiling/debugging). > > On iOS, updates `scripts/ios/bundle-js-and-sentry-upload.sh` to write the JS sourcemap to a stable absolute path under `sourcemaps/ios/index.js.map` (anchored at repo root) to avoid CWD-related path mismatches during Sentry upload, and adds a Bitrise step to deploy that file via `BITRISE_APP_STORE_SOURCEMAP_PATH`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f39ba219f59133cdcd002bb69b25dcd603259d58. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- bitrise.yml | 17 +++++++++++++++++ scripts/ios/bundle-js-and-sentry-upload.sh | 10 ++++++++++ 2 files changed, 27 insertions(+) diff --git a/bitrise.yml b/bitrise.yml index 7f79a4f7aa3..07c338117f5 100644 --- a/bitrise.yml +++ b/bitrise.yml @@ -2374,6 +2374,15 @@ workflows: - pipeline_intermediate_files: $PROJECT_LOCATION/app/build/outputs/bundle/$OUTPUT_PATH/$RENAMED_AAB_FILE:BITRISE_PLAY_STORE_ABB_PATH - deploy_path: $PROJECT_LOCATION/app/build/outputs/bundle/$OUTPUT_PATH/$RENAMED_AAB_FILE title: Bitrise Deploy AAB + - deploy-to-bitrise-io@2.2.3: + is_always_run: false + is_skippable: true + run_if: '{{not (enveq "IS_DEV_BUILD" "true")}}' + inputs: + - deploy_path: $PROJECT_LOCATION/app/build/generated/sourcemaps/react/$OUTPUT_PATH + - is_compress: true + - zip_name: Android_Sourcemaps_$OUTPUT_PATH + title: Deploy Android Sourcemaps - script@1: title: Prepare Android build outputs for caching run_if: '{{and (getenv "ANDROID_PR_BUILD_CACHE_KEY" | ne "") (getenv "SHARE_WITH_DETOX" | eq "true")}}' @@ -3050,6 +3059,14 @@ workflows: inputs: - deploy_path: ios/build/$RENAMED_ARCHIVE_FILE title: Deploy Symbols File + - deploy-to-bitrise-io@2.2.3: + is_always_run: false + is_skippable: true + run_if: '{{not (enveq "IS_SIM_BUILD" "true")}}' # Only run for physical builds + inputs: + - pipeline_intermediate_files: sourcemaps/ios/index.js.map:BITRISE_APP_STORE_SOURCEMAP_PATH + - deploy_path: sourcemaps/ios/index.js.map + title: Deploy Source Map - save-cache@1: title: Save iOS PR Build Cache run_if: '{{and (getenv "IOS_PR_BUILD_CACHE_KEY" | ne "") (getenv "SHARE_WITH_DETOX" | eq "true")}}' diff --git a/scripts/ios/bundle-js-and-sentry-upload.sh b/scripts/ios/bundle-js-and-sentry-upload.sh index f36ff96b397..a0dcf21d633 100755 --- a/scripts/ios/bundle-js-and-sentry-upload.sh +++ b/scripts/ios/bundle-js-and-sentry-upload.sh @@ -26,6 +26,16 @@ export SENTRY_DISABLE_AUTO_UPLOAD=${SENTRY_DISABLE_AUTO_UPLOAD:-"true"} export SENTRY_DIST=$CURRENT_PROJECT_VERSION export SENTRY_RELEASE="$PRODUCT_BUNDLE_IDENTIFIER@$MARKETING_VERSION+$SENTRY_DIST" +# Write source map to a fixed absolute path so Bitrise (and other CI) can +# deploy it AND Sentry CLI can locate it for upload. +# Using a relative path breaks Sentry: react-native-xcode.sh writes the file +# with CWD=repo_root, but sentry-cli resolves SOURCEMAP_FILE relative to its +# own CWD (ios/), causing a path mismatch. An absolute path anchored on +# $PROJECT_DIR (set by Xcode to the ios/ directory) is unambiguous for all +# sub-processes. +REPO_ROOT="$(cd "${PROJECT_DIR}/.." && pwd)" +mkdir -p "$REPO_ROOT/sourcemaps/ios" +export SOURCEMAP_FILE="${SOURCEMAP_FILE:-$REPO_ROOT/sourcemaps/ios/index.js.map}" # Generate JS bundle and upload Sentry source maps REACT_NATIVE_XCODE="../node_modules/react-native/scripts/react-native-xcode.sh"