From 727bf6e9d1a46eb2a84a13a770759fa9ee9f257c Mon Sep 17 00:00:00 2001 From: George Weiler Date: Tue, 17 Feb 2026 10:41:26 -0700 Subject: [PATCH 01/11] fix(ramps): prevents blank "DepositRoot" screen on Android cp-7.66.0 (#26140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fixes a bug where clicking "Buy" / "Add Funds" on Android lands on a blank screen showing only "DepositRoot" in the header, instead of proceeding to the Buy flow. ### Root Cause When the V1 unified buy flag is enabled, selecting a token routes to the Deposit flow's `Root` component. This component renders nothing (`return null`) and relies on an async `useEffect` to check for an existing auth token via `SecureKeychain.getSecureItem()` before navigating to `BuildQuote`. On Android, the Keystore operation can hang indefinitely in certain device states, causing `initializeFlow` to never complete — leaving the user stuck on a blank screen. ### Changes - **Added a 5-second timeout** around `checkExistingToken()` so the flow continues even if the Android Keystore hangs - **Added `.catch()` error handling** on `initializeFlow()` so any unexpected failure falls back to navigating to `BuildQuote` instead of leaving the user stranded - **Replaced `return null` with a loading spinner** so users see visual feedback during initialization instead of a blank page - **Fixed variable shadowing** of `params` inside the effect (renamed to `navParams`) - **Added test coverage** for the error fallback path ## **Changelog** CHANGELOG entry: Fixes a buy navigation bug on Android and adds a loading spinner ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/26079 ## **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 Deposit navigation initialization and introduces timeout/error fallback behavior; risk is moderate due to potential routing changes, but scope is contained and covered by tests. > > **Overview** > Prevents the Deposit `Root` flow from getting stuck by wrapping `checkExistingToken()` in a **5s timeout** and logging/falling back to `Routes.DEPOSIT.BUILD_QUOTE` when token lookup fails or `initializeFlow` throws. > > Replaces the previous blank render (`return null`) with a centered loading spinner, and cleans up navigation param handling (avoids `params` shadowing). Adds a new unit test + snapshot update to cover the rejection fallback path. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit bd626fb97b9b7eb202118b65ec2f26aa34e57b2e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../UI/Ramp/Deposit/Views/Root/Root.test.tsx | 23 ++++- .../UI/Ramp/Deposit/Views/Root/Root.tsx | 93 ++++++++++++++----- .../Root/__snapshots__/Root.test.tsx.snap | 23 ++++- 3 files changed, 114 insertions(+), 25 deletions(-) diff --git a/app/components/UI/Ramp/Deposit/Views/Root/Root.test.tsx b/app/components/UI/Ramp/Deposit/Views/Root/Root.test.tsx index bd2b3a50028..c3602761d54 100644 --- a/app/components/UI/Ramp/Deposit/Views/Root/Root.test.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Root/Root.test.tsx @@ -123,7 +123,7 @@ describe('Root Component', () => { ( getAllDepositOrders as jest.MockedFunction ).mockReturnValue(mockOrders); - mockCheckExistingToken.mockResolvedValue(true); // User is authenticated + mockCheckExistingToken.mockResolvedValue(true); render(Root); await waitFor(() => { @@ -154,7 +154,7 @@ describe('Root Component', () => { ( getAllDepositOrders as jest.MockedFunction ).mockReturnValue(mockOrders); - mockCheckExistingToken.mockResolvedValue(false); // User is not authenticated + mockCheckExistingToken.mockResolvedValue(false); render(Root); await waitFor(() => { @@ -173,6 +173,25 @@ describe('Root Component', () => { }); }); + it('falls back to BUILD_QUOTE when checkExistingToken rejects', async () => { + mockCheckExistingToken.mockRejectedValue( + new Error('SecureKeychain unavailable'), + ); + render(Root); + + await waitFor(() => { + expect(mockReset).toHaveBeenCalledWith({ + index: 0, + routes: [ + { + name: Routes.DEPOSIT.BUILD_QUOTE, + params: { animationEnabled: false }, + }, + ], + }); + }); + }); + describe('intent handling', () => { it('calls setIntent with params when params are provided', () => { const mockParams = { assetId: 'eip155:1/0x123' }; diff --git a/app/components/UI/Ramp/Deposit/Views/Root/Root.tsx b/app/components/UI/Ramp/Deposit/Views/Root/Root.tsx index 29ca5540f0f..43fd556a6a3 100644 --- a/app/components/UI/Ramp/Deposit/Views/Root/Root.tsx +++ b/app/components/UI/Ramp/Deposit/Views/Root/Root.tsx @@ -1,5 +1,7 @@ -import { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; +import { ActivityIndicator } from 'react-native'; import { useNavigation } from '@react-navigation/native'; +import { Box } from '@metamask/design-system-react-native'; import Routes from '../../../../../../constants/navigation/Routes'; import { useDepositSDK } from '../../sdk'; import { useSelector } from 'react-redux'; @@ -9,6 +11,23 @@ import { createBankDetailsNavDetails } from '../BankDetails/BankDetails'; import { createEnterEmailNavDetails } from '../EnterEmail/EnterEmail'; import { DepositNavigationParams } from '../../types'; import { useParams } from '../../../../../../util/navigation/navUtils'; +import { useTheme } from '../../../../../../util/theme'; +import Logger from '../../../../../../util/Logger'; + +export const TOKEN_CHECK_TIMEOUT_MS = 5000; + +function withTimeout(promise: Promise, ms: number): Promise { + let timeoutId: ReturnType; + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout( + () => reject(new Error(`Operation timed out after ${ms}ms`)), + ms, + ); + }); + return Promise.race([promise, timeoutPromise]).finally(() => + clearTimeout(timeoutId), + ); +} const Root = () => { const navigation = useNavigation(); @@ -17,6 +36,7 @@ const Root = () => { const { checkExistingToken, setIntent } = useDepositSDK(); const hasCheckedToken = useRef(false); const orders = useSelector(getAllDepositOrders); + const theme = useTheme(); useEffect(() => { if (params) { @@ -25,10 +45,40 @@ const Root = () => { }, [params, setIntent]); useEffect(() => { + const navigateToDefaultRoute = () => { + navigation.reset({ + index: 0, + routes: [ + { + name: initialRoute, + params: { + animationEnabled: false, + ...(params?.shouldRouteImmediately && { + shouldRouteImmediately: true, + }), + ...(params?.amount !== undefined && { amount: params.amount }), + }, + }, + ], + }); + }; + const initializeFlow = async () => { if (hasCheckedToken.current) return; - const isAuthenticatedFromToken = await checkExistingToken(); + let isAuthenticatedFromToken = false; + try { + isAuthenticatedFromToken = await withTimeout( + checkExistingToken(), + TOKEN_CHECK_TIMEOUT_MS, + ); + } catch (error) { + Logger.error( + error as Error, + 'Deposit Root: checkExistingToken failed or timed out', + ); + } + hasCheckedToken.current = true; const createdOrder = orders.find( @@ -37,7 +87,7 @@ const Root = () => { if (createdOrder) { if (!isAuthenticatedFromToken) { - const [routeName, params] = createEnterEmailNavDetails({ + const [routeName, navParams] = createEnterEmailNavDetails({ redirectToRootAfterAuth: true, }); navigation.reset({ @@ -45,42 +95,37 @@ const Root = () => { routes: [ { name: routeName, - params: { ...params, animationEnabled: false }, + params: { ...navParams, animationEnabled: false }, }, ], }); return; } - const [routeName, params] = createBankDetailsNavDetails({ + const [routeName, navParams] = createBankDetailsNavDetails({ orderId: createdOrder.id, }); - navigation.reset({ - index: 0, - routes: [ - { name: routeName, params: { ...params, animationEnabled: false } }, - ], - }); - } else { navigation.reset({ index: 0, routes: [ { - name: initialRoute, - params: { - animationEnabled: false, - ...(params?.shouldRouteImmediately && { - shouldRouteImmediately: true, - }), - ...(params?.amount !== undefined && { amount: params.amount }), - }, + name: routeName, + params: { ...navParams, animationEnabled: false }, }, ], }); + } else { + navigateToDefaultRoute(); } }; - initializeFlow(); + initializeFlow().catch((error) => { + Logger.error( + error as Error, + 'Deposit Root: initializeFlow failed unexpectedly', + ); + navigateToDefaultRoute(); + }); }, [ checkExistingToken, orders, @@ -90,7 +135,11 @@ const Root = () => { params?.amount, ]); - return null; + return ( + + + + ); }; export default Root; diff --git a/app/components/UI/Ramp/Deposit/Views/Root/__snapshots__/Root.test.tsx.snap b/app/components/UI/Ramp/Deposit/Views/Root/__snapshots__/Root.test.tsx.snap index 9f710673bf4..fa8775c5553 100644 --- a/app/components/UI/Ramp/Deposit/Views/Root/__snapshots__/Root.test.tsx.snap +++ b/app/components/UI/Ramp/Deposit/Views/Root/__snapshots__/Root.test.tsx.snap @@ -311,7 +311,28 @@ exports[`Root Component render matches snapshot 1`] = ` "flex": 1, } } - /> + > + + + + From 858c51d42a49c2ab035becffe4459e754554015c Mon Sep 17 00:00:00 2001 From: Micaela <100321200+micaelae@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:36:03 -0800 Subject: [PATCH 02/11] chore: use chain-agnostic gasFeeEstimatesByChainId in bridge view (#26047) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Bumps the bridge-controller and bridge-status-controller versions. Also passing in the `gasFeeEstimatesByChainId` state which the bridge-controller needs for calculating network fees. ## **Changelog** CHANGELOG entry: chore: use chain-agnostic gas fee estimates source for bridging ## **Related issues** Fixes: N/A ## **Manual testing steps** N/A - no user-facing changes and swaps e2e flow should not change ## **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** > Moderate risk due to upgrading `@metamask/bridge-controller`/`bridge-status-controller` and changing the gas fee estimates shape consumed by bridge selectors, which can affect quote calculation and bridging UX across networks. > > **Overview** > Updates the bridge integration to use chain-agnostic gas fee data by passing `GasFeeController.gasFeeEstimatesByChainId` into the bridge “app state” selectors (replacing the single-chain `selectGasFeeControllerEstimates` output). > > Removes the local Yarn patch for `@metamask/bridge-controller` by bumping to `^66.1.1` (and `bridge-status-controller` to `^66.0.2`), letting upstream provide the prior behavior changes. Also adjusts a token actions unit test mock signature to allow an optional `chainId` parameter. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 31591bfb2133651566da7f38cb391cdab4709171. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- ...dge-controller-npm-65.3.0-f766782954.patch | 78 ------------------- .../hooks/useTokenActions.test.ts | 2 +- app/core/redux/slices/bridge/index.ts | 6 +- app/selectors/bridgeController/index.ts | 6 +- package.json | 4 +- yarn.lock | 68 +--------------- 6 files changed, 12 insertions(+), 152 deletions(-) delete mode 100644 .yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch diff --git a/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch b/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch deleted file mode 100644 index e873b2d48a6..00000000000 --- a/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch +++ /dev/null @@ -1,78 +0,0 @@ -diff --git a/dist/bridge-controller.cjs b/dist/bridge-controller.cjs -index c203cad4f6ec44415adf77c64a0daf3f1195c3d1..cc9ee0d06463008acaf293fee02b16f827518f1b 100644 ---- a/dist/bridge-controller.cjs -+++ b/dist/bridge-controller.cjs -@@ -125,7 +125,6 @@ class BridgeController extends (0, polling_controller_1.StaticIntervalPollingCon - this.update((state) => { - state.quoteRequest = updatedQuoteRequest; - }); -- await __classPrivateFieldGet(this, _BridgeController_fetchAssetExchangeRates, "f").call(this, updatedQuoteRequest).catch((error) => console.warn('Failed to fetch asset exchange rates', error)); - if ((0, quote_1.isValidQuoteRequest)(updatedQuoteRequest)) { - __classPrivateFieldSet(this, _BridgeController_quotesFirstFetched, Date.now(), "f"); - const isSrcChainNonEVM = (0, bridge_2.isNonEvmChainId)(updatedQuoteRequest.srcChainId); -@@ -330,6 +329,7 @@ class BridgeController extends (0, polling_controller_1.StaticIntervalPollingCon - _BridgeController_fetchBridgeQuotes.set(this, async ({ updatedQuoteRequest, context, }) => { - __classPrivateFieldGet(this, _BridgeController_abortController, "f")?.abort(constants_1.AbortReason.NewQuoteRequest); - __classPrivateFieldSet(this, _BridgeController_abortController, new AbortController(), "f"); -+ __classPrivateFieldGet(this, _BridgeController_fetchAssetExchangeRates, "f").call(this, updatedQuoteRequest).catch((error) => console.warn('Failed to fetch asset exchange rates', error)); - this.trackUnifiedSwapBridgeEvent(constants_1.UnifiedSwapBridgeEventName.QuotesRequested, context); - const { sse, maxRefreshCount } = (0, feature_flags_1.getBridgeFeatureFlags)(this.messenger); - const shouldStream = sse?.enabled && -diff --git a/dist/bridge-controller.mjs b/dist/bridge-controller.mjs -index 1de7775a849180414cd68543b215cbef1cc394d6..0ee429bcdb8a664bb30a30562d82c6310cce9b54 100644 ---- a/dist/bridge-controller.mjs -+++ b/dist/bridge-controller.mjs -@@ -122,7 +122,6 @@ export class BridgeController extends StaticIntervalPollingController() { - this.update((state) => { - state.quoteRequest = updatedQuoteRequest; - }); -- await __classPrivateFieldGet(this, _BridgeController_fetchAssetExchangeRates, "f").call(this, updatedQuoteRequest).catch((error) => console.warn('Failed to fetch asset exchange rates', error)); - if (isValidQuoteRequest(updatedQuoteRequest)) { - __classPrivateFieldSet(this, _BridgeController_quotesFirstFetched, Date.now(), "f"); - const isSrcChainNonEVM = isNonEvmChainId(updatedQuoteRequest.srcChainId); -@@ -327,6 +326,7 @@ export class BridgeController extends StaticIntervalPollingController() { - _BridgeController_fetchBridgeQuotes.set(this, async ({ updatedQuoteRequest, context, }) => { - __classPrivateFieldGet(this, _BridgeController_abortController, "f")?.abort(AbortReason.NewQuoteRequest); - __classPrivateFieldSet(this, _BridgeController_abortController, new AbortController(), "f"); -+ __classPrivateFieldGet(this, _BridgeController_fetchAssetExchangeRates, "f").call(this, updatedQuoteRequest).catch((error) => console.warn('Failed to fetch asset exchange rates', error)); - this.trackUnifiedSwapBridgeEvent(UnifiedSwapBridgeEventName.QuotesRequested, context); - const { sse, maxRefreshCount } = getBridgeFeatureFlags(this.messenger); - const shouldStream = sse?.enabled && -diff --git a/dist/selectors.cjs b/dist/selectors.cjs -index 2226a2f7a7aff0262a729b67ef85d76ef97ece33..f9dd333ee49838db8a00d677ea7844418e4f7317 100644 ---- a/dist/selectors.cjs -+++ b/dist/selectors.cjs -@@ -218,7 +218,13 @@ const selectSortedBridgeQuotes = createBridgeSelector([ - case types_1.SortOrder.ETA_ASC: - return (0, lodash_1.orderBy)(quotesWithMetadata, (quote) => quote.estimatedProcessingTimeInSeconds, 'asc'); - default: -- return (0, lodash_1.orderBy)(quotesWithMetadata, ({ cost }) => cost.valueInCurrency ? Number(cost.valueInCurrency) : 0, 'asc'); -+ if (quotesWithMetadata.every((quote) => quote.cost.valueInCurrency)) { -+ return (0, lodash_1.orderBy)(quotesWithMetadata, ({ cost }) => Number(cost.valueInCurrency), 'asc'); -+ } -+ if (quotesWithMetadata.every((quote) => quote.quote.priceData?.priceImpact)) { -+ return (0, lodash_1.orderBy)(quotesWithMetadata, ({ quote }) => Number(quote.priceData?.priceImpact), 'asc'); -+ } -+ return (0, lodash_1.orderBy)(quotesWithMetadata, ({ quote }) => Number(quote.destTokenAmount), 'desc'); - } - }); - const selectRecommendedQuote = createBridgeSelector([selectSortedBridgeQuotes], (quotes) => (quotes.length > 0 ? quotes[0] : null)); -diff --git a/dist/selectors.mjs b/dist/selectors.mjs -index 90dc1282ada72abf36afbc69eb5201d7b258480d..c49ce3cc18e66036d473a2947f9e1454df75f54f 100644 ---- a/dist/selectors.mjs -+++ b/dist/selectors.mjs -@@ -214,7 +214,13 @@ const selectSortedBridgeQuotes = createBridgeSelector([ - case SortOrder.ETA_ASC: - return orderBy(quotesWithMetadata, (quote) => quote.estimatedProcessingTimeInSeconds, 'asc'); - default: -- return orderBy(quotesWithMetadata, ({ cost }) => cost.valueInCurrency ? Number(cost.valueInCurrency) : 0, 'asc'); -+ if (quotesWithMetadata.every((quote) => quote.cost.valueInCurrency)) { -+ return orderBy(quotesWithMetadata, ({ cost }) => Number(cost.valueInCurrency), 'asc'); -+ } -+ if (quotesWithMetadata.every((quote) => quote.quote.priceData?.priceImpact)) { -+ return orderBy(quotesWithMetadata, ({ quote }) => Number(quote.priceData?.priceImpact), 'asc'); -+ } -+ return orderBy(quotesWithMetadata, ({ quote }) => Number(quote.destTokenAmount), 'desc'); - } - }); - const selectRecommendedQuote = createBridgeSelector([selectSortedBridgeQuotes], (quotes) => (quotes.length > 0 ? quotes[0] : null)); diff --git a/app/components/UI/TokenDetails/hooks/useTokenActions.test.ts b/app/components/UI/TokenDetails/hooks/useTokenActions.test.ts index 19e769ef373..103c439bea9 100644 --- a/app/components/UI/TokenDetails/hooks/useTokenActions.test.ts +++ b/app/components/UI/TokenDetails/hooks/useTokenActions.test.ts @@ -392,7 +392,7 @@ describe('useTokenActions', () => { // Default mock behavior for assetId generation mockIsCaipAssetType.mockReturnValue(false); mockFormatAddressToAssetId.mockImplementation( - (address: string, chainId: string | number) => { + (address: string, chainId?: string | number) => { // Simulate the real behavior for EVM tokens const numericChainId = typeof chainId === 'string' ? parseInt(chainId, 16) : chainId; diff --git a/app/core/redux/slices/bridge/index.ts b/app/core/redux/slices/bridge/index.ts index ae2ebf31cc4..7e4e11da88b 100644 --- a/app/core/redux/slices/bridge/index.ts +++ b/app/core/redux/slices/bridge/index.ts @@ -28,9 +28,7 @@ import { BridgeToken, BridgeViewMode, } from '../../../../components/UI/Bridge/types'; -import { selectGasFeeControllerEstimates } from '../../../../selectors/gasFeeController'; import { analytics } from '../../../../util/analytics/analytics'; -import { GasFeeEstimates } from '@metamask/gas-fee-controller'; import { selectRemoteFeatureFlags } from '../../../../selectors/featureFlagController'; import { getTokenExchangeRate } from '../../../../components/UI/Bridge/utils/exchange-rates'; import { selectCanSignTransactions } from '../../../../selectors/accountsController'; @@ -439,7 +437,9 @@ export const selectIsGasIncluded7702Supported = (state: RootState) => const selectControllerFields = (state: RootState) => ({ ...state.engine.backgroundState.BridgeController, - gasFeeEstimates: selectGasFeeControllerEstimates(state) as GasFeeEstimates, + gasFeeEstimatesByChainId: + state.engine.backgroundState.GasFeeController.gasFeeEstimatesByChainId ?? + {}, ...state.engine.backgroundState.MultichainAssetsRatesController, ...state.engine.backgroundState.TokenRatesController, ...state.engine.backgroundState.CurrencyRateController, diff --git a/app/selectors/bridgeController/index.ts b/app/selectors/bridgeController/index.ts index 4b1028decbe..5cb96fab78c 100644 --- a/app/selectors/bridgeController/index.ts +++ b/app/selectors/bridgeController/index.ts @@ -5,9 +5,7 @@ import { selectMinimumBalanceForRentExemptionInSOL, } from '@metamask/bridge-controller'; import { selectRemoteFeatureFlags } from '../featureFlagController'; -import { selectGasFeeControllerEstimates } from '../gasFeeController'; import { MetaMetrics } from '../../core/Analytics'; -import { GasFeeEstimates } from '@metamask/gas-fee-controller'; export const selectBridgeControllerState = (state: RootState) => state.engine.backgroundState.BridgeController; @@ -21,7 +19,9 @@ export const selectQuoteRequest = createSelector( // Create the BridgeAppState selector following the same pattern as in bridge slice export const selectBridgeAppState = (state: RootState) => ({ ...state.engine.backgroundState.BridgeController, - gasFeeEstimates: selectGasFeeControllerEstimates(state) as GasFeeEstimates, + gasFeeEstimatesByChainId: + state.engine.backgroundState.GasFeeController.gasFeeEstimatesByChainId ?? + {}, ...state.engine.backgroundState.MultichainAssetsRatesController, ...state.engine.backgroundState.TokenRatesController, ...state.engine.backgroundState.CurrencyRateController, diff --git a/package.json b/package.json index 5fad0c213e4..602befeb4ff 100644 --- a/package.json +++ b/package.json @@ -194,8 +194,8 @@ "@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A99.2.0#~/.yarn/patches/@metamask-assets-controllers-npm-99.2.0-bf6ecbcde9.patch", "@metamask/base-controller": "^9.0.0", "@metamask/bitcoin-wallet-snap": "^1.10.0", - "@metamask/bridge-controller": "patch:@metamask/bridge-controller@npm%3A65.3.0#~/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch", - "@metamask/bridge-status-controller": "^66.0.0", + "@metamask/bridge-controller": "^66.1.1", + "@metamask/bridge-status-controller": "^66.0.2", "@metamask/chain-agnostic-permission": "^1.3.0", "@metamask/connectivity-controller": "^0.1.0", "@metamask/controller-utils": "^11.18.0", diff --git a/yarn.lock b/yarn.lock index e2bf0b47748..d40d1f046bc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8006,37 +8006,6 @@ __metadata: languageName: node linkType: hard -"@metamask/bridge-controller@npm:65.3.0": - version: 65.3.0 - resolution: "@metamask/bridge-controller@npm:65.3.0" - dependencies: - "@ethersproject/address": "npm:^5.7.0" - "@ethersproject/bignumber": "npm:^5.7.0" - "@ethersproject/constants": "npm:^5.7.0" - "@ethersproject/contracts": "npm:^5.7.0" - "@ethersproject/providers": "npm:^5.7.0" - "@metamask/accounts-controller": "npm:^35.0.2" - "@metamask/assets-controllers": "npm:^99.2.0" - "@metamask/base-controller": "npm:^9.0.0" - "@metamask/controller-utils": "npm:^11.18.0" - "@metamask/gas-fee-controller": "npm:^26.0.2" - "@metamask/keyring-api": "npm:^21.0.0" - "@metamask/messenger": "npm:^0.3.0" - "@metamask/metamask-eth-abis": "npm:^3.1.1" - "@metamask/multichain-network-controller": "npm:^3.0.2" - "@metamask/network-controller": "npm:^29.0.0" - "@metamask/polling-controller": "npm:^16.0.2" - "@metamask/remote-feature-flag-controller": "npm:^4.0.0" - "@metamask/snaps-controllers": "npm:^17.2.0" - "@metamask/transaction-controller": "npm:^62.14.0" - "@metamask/utils": "npm:^11.9.0" - bignumber.js: "npm:^9.1.2" - reselect: "npm:^5.1.1" - uuid: "npm:^8.3.2" - checksum: 10/204030f6754dc62f220db7c90ca9e248ab00da20759c9eb895f9b0c00f342895a918434bf10650c5c156cdb92bce3ff2263bd1b08ddd60691e0f6cd016c3a770 - languageName: node - linkType: hard - "@metamask/bridge-controller@npm:^66.1.1": version: 66.1.1 resolution: "@metamask/bridge-controller@npm:66.1.1" @@ -8068,38 +8037,7 @@ __metadata: languageName: node linkType: hard -"@metamask/bridge-controller@patch:@metamask/bridge-controller@npm%3A65.3.0#~/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch": - version: 65.3.0 - resolution: "@metamask/bridge-controller@patch:@metamask/bridge-controller@npm%3A65.3.0#~/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch::version=65.3.0&hash=bc4abc" - dependencies: - "@ethersproject/address": "npm:^5.7.0" - "@ethersproject/bignumber": "npm:^5.7.0" - "@ethersproject/constants": "npm:^5.7.0" - "@ethersproject/contracts": "npm:^5.7.0" - "@ethersproject/providers": "npm:^5.7.0" - "@metamask/accounts-controller": "npm:^35.0.2" - "@metamask/assets-controllers": "npm:^99.2.0" - "@metamask/base-controller": "npm:^9.0.0" - "@metamask/controller-utils": "npm:^11.18.0" - "@metamask/gas-fee-controller": "npm:^26.0.2" - "@metamask/keyring-api": "npm:^21.0.0" - "@metamask/messenger": "npm:^0.3.0" - "@metamask/metamask-eth-abis": "npm:^3.1.1" - "@metamask/multichain-network-controller": "npm:^3.0.2" - "@metamask/network-controller": "npm:^29.0.0" - "@metamask/polling-controller": "npm:^16.0.2" - "@metamask/remote-feature-flag-controller": "npm:^4.0.0" - "@metamask/snaps-controllers": "npm:^17.2.0" - "@metamask/transaction-controller": "npm:^62.14.0" - "@metamask/utils": "npm:^11.9.0" - bignumber.js: "npm:^9.1.2" - reselect: "npm:^5.1.1" - uuid: "npm:^8.3.2" - checksum: 10/a0b7ee2ca6c4c65886b1c8b090e0a6d339cb47a3436fe8efd488331022777573ad63446e8ff5281d84c44f72a4907eb1caca2f006239e7ff0804621599f5d0a9 - languageName: node - linkType: hard - -"@metamask/bridge-status-controller@npm:^66.0.0, @metamask/bridge-status-controller@npm:^66.0.2": +"@metamask/bridge-status-controller@npm:^66.0.2": version: 66.0.2 resolution: "@metamask/bridge-status-controller@npm:66.0.2" dependencies: @@ -35531,8 +35469,8 @@ __metadata: "@metamask/auto-changelog": "npm:^5.3.0" "@metamask/base-controller": "npm:^9.0.0" "@metamask/bitcoin-wallet-snap": "npm:^1.10.0" - "@metamask/bridge-controller": "patch:@metamask/bridge-controller@npm%3A65.3.0#~/.yarn/patches/@metamask-bridge-controller-npm-65.3.0-f766782954.patch" - "@metamask/bridge-status-controller": "npm:^66.0.0" + "@metamask/bridge-controller": "npm:^66.1.1" + "@metamask/bridge-status-controller": "npm:^66.0.2" "@metamask/browser-passworder": "npm:^5.0.0" "@metamask/browser-playground": "npm:0.2.0" "@metamask/build-utils": "npm:^3.0.0" From 261d0752706c049f6ea166a98efcd10160e67a6d Mon Sep 17 00:00:00 2001 From: infiniteflower <139582705+infiniteflower@users.noreply.github.com> Date: Wed, 18 Feb 2026 04:03:09 +0900 Subject: [PATCH 03/11] chore: add more guards to ensure autosign only gets called for hardware swaps/bridge txs (#25919) ## **Description** This PR adds additional guards to ensure that `autoSign` is only called for hardware Swap/Bridge txs. ## **Changelog** CHANGELOG entry: Removed notifications for all swap/bridge txs ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/SWAPS-4076 ## **Manual testing steps** ```gherkin Feature: Hardware wallet guard for swap and bridge auto-signing Scenario: Software wallet swap transaction does not trigger auto-sign Given the user has a software wallet account selected When the user submits a swap transaction Then the transaction is auto confirmed and auto-sign is not called Scenario: Hardware wallet swap transaction triggers auto-sign Given the user has a Ledger or QR hardware wallet account selected When the user submits a swap transaction Then auto-sign is called and the hardware signing modal is presented Scenario: Software wallet bridge transaction does not trigger auto-sign Given the user has a software wallet account selected When the user submits a bridge transaction Then the transaction is auto confirmed and auto-sign is not called Scenario: Hardware wallet bridge transaction triggers auto-sign Given the user has a Ledger or QR hardware wallet account selected When the user submits a bridge transaction Then auto-sign is called and the hardware signing modal is presented ``` ## **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** > Changes transaction-type classification and the auto-sign trigger conditions; mistakes could prevent auto-sign for valid hardware swap/bridge flows or incorrectly classify swaps. > > **Overview** > Tightens `autoSign` eligibility so it only triggers for **hardware wallet** swap/bridge transactions, by extracting the unapproved-tx handler into a dedicated `onUnapprovedTransaction` helper and adding explicit hardware guards for swap (`isHardwareSwapApproveOrSwapTransaction`) and bridge (`isHardwareBridgeTransaction`). > > Swap detection no longer relies on `origin`; it now keys off `transactionMeta.type` (`swap`/`swapApproval`) plus validated contract/spender addresses, and smart-transaction utilities/tests were updated accordingly. New unit tests cover the new handler and the hardware-guard helpers. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 646a516fc4a42bd1f5b8eb0452c41667c39cd64e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- app/components/Nav/Main/RootRPCMethodsUI.js | 58 +------ .../Nav/Main/onUnapprovedTransaction.test.ts | 159 ++++++++++++++++++ .../Nav/Main/onUnapprovedTransaction.ts | 39 +++++ .../UI/Bridge/utils/transaction.test.ts | 56 +++++- app/components/UI/Bridge/utils/transaction.ts | 10 ++ app/util/smart-transactions/index.test.ts | 8 +- app/util/smart-transactions/index.ts | 6 +- .../smart-publish-hook.test.ts | 2 + app/util/transactions/index.js | 66 +++++--- app/util/transactions/index.test.ts | 102 ++++++++--- 10 files changed, 409 insertions(+), 97 deletions(-) create mode 100644 app/components/Nav/Main/onUnapprovedTransaction.test.ts create mode 100644 app/components/Nav/Main/onUnapprovedTransaction.ts diff --git a/app/components/Nav/Main/RootRPCMethodsUI.js b/app/components/Nav/Main/RootRPCMethodsUI.js index e3bb9bc1453..bbf86042948 100644 --- a/app/components/Nav/Main/RootRPCMethodsUI.js +++ b/app/components/Nav/Main/RootRPCMethodsUI.js @@ -2,12 +2,10 @@ import React, { useEffect, useCallback } from 'react'; import { Alert } from 'react-native'; import PropTypes from 'prop-types'; -import NotificationManager from '../../../core/NotificationManager'; import Engine from '../../../core/Engine'; import { strings } from '../../../../locales/i18n'; -import { getIsSwapApproveOrSwapTransaction } from '../../../util/transactions'; +import { onUnapprovedTransaction } from './onUnapprovedTransaction'; import Logger from '../../../util/Logger'; -import TransactionTypes from '../../../core/TransactionTypes'; import { KEYSTONE_TX_CANCELED } from '../../../constants/error'; import { MetaMetricsEvents } from '../../../core/Analytics'; import { isHardwareAccount } from '../../../util/address'; @@ -25,7 +23,6 @@ import ExtendedKeyringTypes from '../../../constants/keyringTypes'; import { ConfirmRoot } from '../../../components/Views/confirmations/components/confirm'; import { useMetrics } from '../../../components/hooks/useMetrics'; import { STX_NO_HASH_ERROR } from '../../../util/smart-transactions/smart-publish-hook'; -import { cloneDeep } from 'lodash'; ///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps) import InstallSnapApproval from '../../Approvals/InstallSnapApproval'; @@ -33,7 +30,6 @@ import SnapDialogApproval from '../../Snaps/SnapDialogApproval/SnapDialogApprova ///: END:ONLY_INCLUDE_IF ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) import SnapAccountCustomNameApproval from '../../Approvals/SnapAccountCustomNameApproval'; -import { getIsBridgeTransaction } from '../../UI/Bridge/utils/transaction'; ///: END:ONLY_INCLUDE_IF const RootRPCMethodsUI = (props) => { @@ -41,28 +37,7 @@ const RootRPCMethodsUI = (props) => { const autoSign = useCallback( async (transactionMeta) => { - const { id: transactionId } = transactionMeta; - try { - Engine.controllerMessenger.subscribeOnceIf( - 'TransactionController:transactionFinished', - (transactionMeta) => { - try { - if (transactionMeta.status === 'submitted') { - NotificationManager.watchSubmittedTransaction({ - ...transactionMeta, - assetType: transactionMeta.txParams.assetType, - }); - } else { - throw transactionMeta.error; - } - } catch (error) { - console.error(error, 'error while trying to send transaction'); - } - }, - (transactionMeta) => transactionMeta.id === transactionId, - ); - const isLedgerAccount = isHardwareAccount( transactionMeta.txParams.from, [ExtendedKeyringTypes.ledger], @@ -127,26 +102,11 @@ const RootRPCMethodsUI = (props) => { [props.navigation, trackEvent, createEventBuilder], ); - const onUnapprovedTransaction = useCallback( - async (transactionMetaOriginal) => { - const transactionMeta = cloneDeep(transactionMetaOriginal); - - if (transactionMeta.origin === TransactionTypes.MMM) return; - - const to = transactionMeta.txParams.to?.toLowerCase(); - const { data } = transactionMeta.txParams; - - if ( - getIsSwapApproveOrSwapTransaction( - data, - transactionMeta.origin, - to, - transactionMeta.chainId, - ) || - getIsBridgeTransaction(transactionMeta) - ) { - autoSign(transactionMeta); - } + const handleUnapprovedTransaction = useCallback( + (transactionMeta) => { + onUnapprovedTransaction(transactionMeta, { + autoSign, + }); }, [autoSign], ); @@ -155,15 +115,15 @@ const RootRPCMethodsUI = (props) => { useEffect(() => { Engine.controllerMessenger.subscribe( 'TransactionController:unapprovedTransactionAdded', - onUnapprovedTransaction, + handleUnapprovedTransaction, ); return () => { Engine.controllerMessenger.unsubscribe( 'TransactionController:unapprovedTransactionAdded', - onUnapprovedTransaction, + handleUnapprovedTransaction, ); }; - }, [onUnapprovedTransaction]); + }, [handleUnapprovedTransaction]); useEffect( () => diff --git a/app/components/Nav/Main/onUnapprovedTransaction.test.ts b/app/components/Nav/Main/onUnapprovedTransaction.test.ts new file mode 100644 index 00000000000..0e7984408eb --- /dev/null +++ b/app/components/Nav/Main/onUnapprovedTransaction.test.ts @@ -0,0 +1,159 @@ +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; +import { ORIGIN_METAMASK } from '@metamask/controller-utils'; +import { onUnapprovedTransaction } from './onUnapprovedTransaction'; +import { isHardwareAccount } from '../../../util/address'; + +jest.mock('../../../util/address', () => ({ + ...jest.requireActual('../../../util/address'), + isHardwareAccount: jest.fn(), +})); + +const isHardwareAccountMock = jest.mocked(isHardwareAccount); + +// Real swap contract address on mainnet (from @metamask/bridge-controller ALLOWED_CONTRACT_ADDRESSES) +const SWAP_CONTRACT_ADDRESS = '0x881d40237659c251811cec9c364ef91dc08d300c'; + +// Real swap tx data from existing test fixtures (not a transfer signature) +const SWAP_TX_DATA = + '0x5f5755290000000000000000000000000000000000000000000000000000000000000080000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c0'; + +const mockCallbacks = () => ({ + autoSign: jest.fn(), +}); + +const buildSwapTxMeta = (overrides: Partial = {}) => + ({ + origin: ORIGIN_METAMASK, + type: TransactionType.swap, + chainId: '0x1', + txParams: { + from: '0xFromAddress', + to: SWAP_CONTRACT_ADDRESS, + data: SWAP_TX_DATA, + }, + ...overrides, + }) as unknown as TransactionMeta; + +const buildBridgeTxMeta = (overrides: Partial = {}) => + ({ + origin: ORIGIN_METAMASK, + type: TransactionType.bridge, + chainId: '0x1', + txParams: { + from: '0xFromAddress', + to: '0xBridgeContractAddress', + data: '0x', + }, + ...overrides, + }) as unknown as TransactionMeta; + +const buildSimpleSendTxMeta = () => + ({ + origin: 'https://some-dapp.com', + type: TransactionType.simpleSend, + chainId: '0x1', + txParams: { + from: '0xFromAddress', + to: '0xRecipient', + data: undefined, + }, + }) as unknown as TransactionMeta; + +describe('onUnapprovedTransaction', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('skips processing when origin is MetaMask Mobile (MMM)', () => { + const callbacks = mockCallbacks(); + const txMeta = buildSwapTxMeta({ origin: 'MetaMask Mobile' }); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).not.toHaveBeenCalled(); + }); + + it('calls autoSign for hardware wallet swap', () => { + isHardwareAccountMock.mockReturnValue(true); + const callbacks = mockCallbacks(); + const txMeta = buildSwapTxMeta(); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).toHaveBeenCalledTimes(1); + }); + + it('does not call autoSign for non-hardware wallet swap', () => { + isHardwareAccountMock.mockReturnValue(false); + const callbacks = mockCallbacks(); + const txMeta = buildSwapTxMeta(); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).not.toHaveBeenCalled(); + }); + + it('calls autoSign for hardware wallet bridge', () => { + isHardwareAccountMock.mockReturnValue(true); + const callbacks = mockCallbacks(); + const txMeta = buildBridgeTxMeta(); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).toHaveBeenCalledTimes(1); + }); + + it('does not call autoSign for non-hardware wallet bridge', () => { + isHardwareAccountMock.mockReturnValue(false); + const callbacks = mockCallbacks(); + const txMeta = buildBridgeTxMeta(); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).not.toHaveBeenCalled(); + }); + + it('calls autoSign for hardware wallet bridgeApproval', () => { + isHardwareAccountMock.mockReturnValue(true); + const callbacks = mockCallbacks(); + const txMeta = buildBridgeTxMeta({ type: TransactionType.bridgeApproval }); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).toHaveBeenCalledTimes(1); + }); + + it('does not call autoSign for non-swap non-bridge transaction', () => { + isHardwareAccountMock.mockReturnValue(false); + const callbacks = mockCallbacks(); + const txMeta = buildSimpleSendTxMeta(); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).not.toHaveBeenCalled(); + }); + + it('does not call autoSign for non-swap non-bridge transaction even from hardware wallet', () => { + isHardwareAccountMock.mockReturnValue(true); + const callbacks = mockCallbacks(); + const txMeta = buildSimpleSendTxMeta(); + + onUnapprovedTransaction(txMeta, callbacks); + + expect(callbacks.autoSign).not.toHaveBeenCalled(); + }); + + it('does not mutate the original transaction meta', () => { + isHardwareAccountMock.mockReturnValue(false); + const callbacks = mockCallbacks(); + const txMeta = buildSwapTxMeta(); + const originalTo = txMeta.txParams.to; + + onUnapprovedTransaction(txMeta, callbacks); + + expect(txMeta.txParams.to).toBe(originalTo); + }); +}); diff --git a/app/components/Nav/Main/onUnapprovedTransaction.ts b/app/components/Nav/Main/onUnapprovedTransaction.ts new file mode 100644 index 00000000000..c7d93d4be1f --- /dev/null +++ b/app/components/Nav/Main/onUnapprovedTransaction.ts @@ -0,0 +1,39 @@ +import { TransactionMeta } from '@metamask/transaction-controller'; +import { cloneDeep } from 'lodash'; +import TransactionTypes from '../../../core/TransactionTypes'; +import { isHardwareSwapApproveOrSwapTransaction } from '../../../util/transactions'; +import { isHardwareBridgeTransaction } from '../../UI/Bridge/utils/transaction'; + +interface OnUnapprovedTransactionCallbacks { + autoSign: (transactionMeta: TransactionMeta) => void; +} + +/** + * Handles an unapproved transaction event. + * + * For hardware wallet swap/bridge transactions only: triggers auto-sign. + */ +export function onUnapprovedTransaction( + transactionMetaOriginal: TransactionMeta, + callbacks: OnUnapprovedTransactionCallbacks, +) { + const transactionMeta = cloneDeep(transactionMetaOriginal); + + if (transactionMeta.origin === TransactionTypes.MMM) return; + + const to = transactionMeta.txParams.to?.toLowerCase(); + const data = transactionMeta.txParams.data as string; + + if ( + isHardwareSwapApproveOrSwapTransaction( + data, + to, + transactionMeta.chainId, + transactionMeta.type, + transactionMeta.txParams.from, + ) || + isHardwareBridgeTransaction(transactionMeta) + ) { + callbacks.autoSign(transactionMeta); + } +} diff --git a/app/components/UI/Bridge/utils/transaction.test.ts b/app/components/UI/Bridge/utils/transaction.test.ts index 036ee27be3a..405c28d595e 100644 --- a/app/components/UI/Bridge/utils/transaction.test.ts +++ b/app/components/UI/Bridge/utils/transaction.test.ts @@ -3,7 +3,16 @@ import { TransactionMeta, TransactionType, } from '@metamask/transaction-controller'; -import { getIsBridgeTransaction } from './transaction'; +import { isHardwareAccount } from '../../../../util/address'; +import { + getIsBridgeTransaction, + isHardwareBridgeTransaction, +} from './transaction'; + +jest.mock('../../../../util/address', () => ({ + ...jest.requireActual('../../../../util/address'), + isHardwareAccount: jest.fn(), +})); describe('getIsBridgeTransaction', () => { it('returns true when origin is MetaMask and type is bridge', () => { @@ -76,3 +85,48 @@ describe('getIsBridgeTransaction', () => { expect(result).toBe(false); }); }); + +describe('isHardwareBridgeTransaction', () => { + const isHardwareAccountMock = jest.mocked(isHardwareAccount); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('returns true when it is a bridge transaction from a hardware wallet', () => { + isHardwareAccountMock.mockReturnValue(true); + + const txMeta = { + origin: ORIGIN_METAMASK, + type: TransactionType.bridge, + txParams: { from: '0xHardwareAddress' }, + } as unknown as TransactionMeta; + + expect(isHardwareBridgeTransaction(txMeta)).toBe(true); + expect(isHardwareAccountMock).toHaveBeenCalledWith('0xHardwareAddress'); + }); + + it('returns false when it is a bridge transaction but not from a hardware wallet', () => { + isHardwareAccountMock.mockReturnValue(false); + + const txMeta = { + origin: ORIGIN_METAMASK, + type: TransactionType.bridge, + txParams: { from: '0xSoftwareAddress' }, + } as unknown as TransactionMeta; + + expect(isHardwareBridgeTransaction(txMeta)).toBe(false); + }); + + it('returns false when it is from a hardware wallet but not a bridge transaction', () => { + isHardwareAccountMock.mockReturnValue(true); + + const txMeta = { + origin: ORIGIN_METAMASK, + type: TransactionType.contractInteraction, + txParams: { from: '0xHardwareAddress' }, + } as unknown as TransactionMeta; + + expect(isHardwareBridgeTransaction(txMeta)).toBe(false); + }); +}); diff --git a/app/components/UI/Bridge/utils/transaction.ts b/app/components/UI/Bridge/utils/transaction.ts index b59e23716d6..77f981e8156 100644 --- a/app/components/UI/Bridge/utils/transaction.ts +++ b/app/components/UI/Bridge/utils/transaction.ts @@ -3,6 +3,7 @@ import { TransactionMeta, TransactionType, } from '@metamask/transaction-controller'; +import { isHardwareAccount } from '../../../../util/address'; export const getIsBridgeTransaction = (txMeta: TransactionMeta) => { const { origin } = txMeta; @@ -13,3 +14,12 @@ export const getIsBridgeTransaction = (txMeta: TransactionMeta) => { txMeta.type === TransactionType.bridge) ); }; + +/** + * Determines if the transaction is a bridge transaction + * from a hardware wallet (Ledger or QR). + * Used as an additional guard at the call site before invoking autoSign. + */ +export const isHardwareBridgeTransaction = (txMeta: TransactionMeta) => + isHardwareAccount(txMeta.txParams.from as string) && + getIsBridgeTransaction(txMeta); diff --git a/app/util/smart-transactions/index.test.ts b/app/util/smart-transactions/index.test.ts index fe219fa9382..97ba7851b73 100644 --- a/app/util/smart-transactions/index.test.ts +++ b/app/util/smart-transactions/index.test.ts @@ -1,4 +1,7 @@ -import { TransactionMeta } from '@metamask/transaction-controller'; +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; import { getShouldStartApprovalRequest, getShouldUpdateApprovalRequest, @@ -181,6 +184,7 @@ describe('Smart Transactions utils', () => { it('returns correct type type for Swap transaction for ETH From token', () => { const txMeta = { chainId: '0x1', + type: TransactionType.swap, deviceConfirmedOn: 'metamask_mobile', gasFeeEstimatesLoaded: true, id: 'b3095a90-0990-11ef-9909-c3c3278f64e5', @@ -220,6 +224,7 @@ describe('Smart Transactions utils', () => { it('returns correct type type for Swap approve transaction for ERC20 From token', () => { const txMeta = { chainId: '0x1', + type: TransactionType.swapApproval, deviceConfirmedOn: 'metamask_mobile', gasFeeEstimatesLoaded: true, id: '15879650-0991-11ef-9ce4-2f3037ea41a6', @@ -259,6 +264,7 @@ describe('Smart Transactions utils', () => { it('returns correct type type for Swap transaction for ERC20 From token', () => { const txMeta = { chainId: '0x1', + type: TransactionType.swap, deviceConfirmedOn: 'metamask_mobile', gasFeeEstimatesLoaded: true, id: '1587e470-0991-11ef-9ce4-2f3037ea41a6', diff --git a/app/util/smart-transactions/index.ts b/app/util/smart-transactions/index.ts index 8bf39b25de1..6e7188e234a 100644 --- a/app/util/smart-transactions/index.ts +++ b/app/util/smart-transactions/index.ts @@ -33,21 +33,21 @@ export const getTransactionType = ( const isSwapApproveOrSwapTransaction = getIsSwapApproveOrSwapTransaction( data, - transactionMeta.origin, to, chainId, + transactionMeta.type, ); const isSwapApproveTx = getIsSwapApproveTransaction( data, - transactionMeta.origin, to, chainId, + transactionMeta.type, ); const isSwapTransaction = getIsSwapTransaction( data, - transactionMeta.origin, to, chainId, + transactionMeta.type, ); const isNativeTokenTransferred = getIsNativeTokenTransferred( diff --git a/app/util/smart-transactions/smart-publish-hook.test.ts b/app/util/smart-transactions/smart-publish-hook.test.ts index 791b14026f3..ace912a1e51 100644 --- a/app/util/smart-transactions/smart-publish-hook.test.ts +++ b/app/util/smart-transactions/smart-publish-hook.test.ts @@ -507,6 +507,7 @@ describe('submitSmartTransactionHook', () => { networkID: undefined, chainId: '0x1', origin: 'EXAMPLE_FOX_CODE', + type: TransactionType.swapApproval, status: TransactionStatus.signed, time: 1713381359223, txParams: { @@ -630,6 +631,7 @@ describe('submitSmartTransactionHook', () => { id: '01c48130-fcf0-11ee-8f32-2f9930c68b06', networkID: undefined, origin: 'EXAMPLE_FOX_CODE', + type: TransactionType.swap, rawTransaction: '0x02f903560182021a840339802785063b5f780783038e2494881d40237659c251811cec9c364ef91dc08d300c80b902e65f5755290000000000000000000000000000000000000000000000000000000000000080000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb480000000000000000000000000000000000000000000000000000000000a7d8c000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000136f6e65496e6368563546656544796e616d6963000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a7d8c0000000000000000000000000000000000000000000000000000c8e72d12c36ac000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000001cf42ad63350000000000000000000000000f326e4de8f66a0bdc0970b79e0924e33c79f1915000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000c80502b1c5000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb480000000000000000000000000000000000000000000000000000000000a7d8c0000000000000000000000000000000000000000000000000000caad2bdb673320000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000140000000000000003b6d0340b4e16d0168e52d35cacd2c6185b44281ec28c9dc7dcbea7c000000000000000000000000000000000000000000000000001b', securityAlertResponse: undefined, diff --git a/app/util/transactions/index.js b/app/util/transactions/index.js index 788c634a5a6..aefa5034bb8 100644 --- a/app/util/transactions/index.js +++ b/app/util/transactions/index.js @@ -17,7 +17,11 @@ import { } from '@metamask/transaction-controller'; import Engine from '../../core/Engine'; import I18n, { strings } from '../../../locales/i18n'; -import { safeToChecksumAddress, toChecksumAddress } from '../address'; +import { + safeToChecksumAddress, + toChecksumAddress, + isHardwareAccount, +} from '../address'; import { balanceToFiatNumber, BNToHex, @@ -1867,12 +1871,7 @@ export const minimumTokenAllowance = (tokenDecimals) => { /** * For a MM Swap tx: Determines if the transaction is an ERC20 approve tx OR the actual swap tx where tokens are transferred */ -export const getIsSwapApproveOrSwapTransaction = ( - data, - origin, - to, - chainId, -) => { +export const getIsSwapApproveOrSwapTransaction = (data, to, chainId, type) => { if (!data) { return false; } @@ -1883,50 +1882,69 @@ export const getIsSwapApproveOrSwapTransaction = ( return false; } - const isLegacySwap = origin === process.env.MM_FOX_CODE; - const isUnifiedSwap = origin === ORIGIN_METAMASK; + const isSwap = + type === TransactionType.swap && + to && + isValidSwapsContractAddress(chainId, to); - // if approval data includes metaswap contract - // if destination address is metaswap contract - return ( - (isLegacySwap || isUnifiedSwap) && + const isSwapApproval = + type === TransactionType.swapApproval && to && - (isValidSwapsContractAddress(chainId, to) || - (data?.startsWith(APPROVE_FUNCTION_SIGNATURE) && - decodeApproveData(data).spenderAddress?.toLowerCase() === - getSwapsContractAddress(chainId))) - ); + data?.startsWith(APPROVE_FUNCTION_SIGNATURE) && + decodeApproveData(data).spenderAddress?.toLowerCase() === + getSwapsContractAddress(chainId); + + return isSwap || isSwapApproval; }; +/** + * Determines if the transaction is a swap approve or swap transaction + * from a hardware wallet (Ledger or QR). + * Used as an additional guard at the call site before invoking autoSign. + */ +export const isHardwareSwapApproveOrSwapTransaction = ( + data, + to, + chainId, + type, + from, +) => + isHardwareAccount(from) && + getIsSwapApproveOrSwapTransaction(data, to, chainId, type); + /** * For a MM Swap tx: Determines if the transaction is an ERC20 approve tx */ -export const getIsSwapApproveTransaction = (data, origin, to, chainId) => { +export const getIsSwapApproveTransaction = (data, to, chainId, type) => { if (!data) { return false; } - const isFromSwaps = origin === process.env.MM_FOX_CODE; const isApproveFunction = data && getFourByteSignature(data) === APPROVE_FUNCTION_SIGNATURE; const isSpenderSwapsContract = decodeApproveData(data).spenderAddress?.toLowerCase() === getSwapsContractAddress(chainId); - return isFromSwaps && to && isApproveFunction && isSpenderSwapsContract; + return ( + type === TransactionType.swapApproval && + to && + isApproveFunction && + isSpenderSwapsContract + ); }; /** * For a MM Swap tx: Determines if the transaction is the actual swap tx where tokens are transferred */ -export const getIsSwapTransaction = (data, origin, to, chainId) => { +export const getIsSwapTransaction = (data, to, chainId, type) => { const isSwapApproveOrSwapTransaction = getIsSwapApproveOrSwapTransaction( data, - origin, to, chainId, + type, ); - const isSwapApprove = getIsSwapApproveTransaction(data, origin, to, chainId); + const isSwapApprove = getIsSwapApproveTransaction(data, to, chainId, type); return isSwapApproveOrSwapTransaction && !isSwapApprove; }; diff --git a/app/util/transactions/index.test.ts b/app/util/transactions/index.test.ts index 4f9e2d37cf1..1550bb3168d 100644 --- a/app/util/transactions/index.test.ts +++ b/app/util/transactions/index.test.ts @@ -2,7 +2,7 @@ import BN from 'bnjs4'; import * as controllerUtilsModule from '@metamask/controller-utils'; -import { ERC721, ERC1155, ORIGIN_METAMASK } from '@metamask/controller-utils'; +import { ERC721, ERC1155 } from '@metamask/controller-utils'; import * as bridgeControllerModule from '@metamask/bridge-controller'; import { handleMethodData } from '../../util/transaction-controller'; @@ -32,6 +32,7 @@ import { parseTransactionLegacy, getIsNativeTokenTransferred, getIsSwapApproveOrSwapTransaction, + isHardwareSwapApproveOrSwapTransaction, getIsSwapApproveTransaction, getIsSwapTransaction, INCREASE_ALLOWANCE_SIGNATURE, @@ -65,6 +66,7 @@ import { isTransactionIncomplete, } from '.'; import Engine from '../../core/Engine'; +import { isHardwareAccount } from '../address'; import { strings } from '../../../locales/i18n'; import { EIP_7702_REVOKE_ADDRESS } from '../../components/Views/confirmations/hooks/7702/useEIP7702Accounts'; import { @@ -149,6 +151,10 @@ jest.mock('../../core/Engine'); const ENGINE_MOCK = Engine as jest.MockedClass; jest.mock('../../util/transaction-controller'); +jest.mock('../address', () => ({ + ...jest.requireActual('../address'), + isHardwareAccount: jest.fn(), +})); const MOCK_ADDRESS1 = '0x0001'; const MOCK_ADDRESS2 = '0x0002'; @@ -1261,6 +1267,7 @@ const dappTxMeta = { }; const sendEthTxMeta = { chainId: '0x1', + type: TransactionType.simpleSend, origin: 'MetaMask Mobile', transaction: { from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452', @@ -1276,6 +1283,7 @@ const sendEthTxMeta = { }; const sendERC20TxMeta = { chainId: '0x1', + type: TransactionType.tokenMethodTransfer, origin: 'MetaMask Mobile', transaction: { from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452', @@ -1292,6 +1300,7 @@ const sendERC20TxMeta = { const swapFlowApproveERC20TxMeta = { chainId: '0x1', + type: TransactionType.swapApproval, origin: process.env.MM_FOX_CODE, transaction: { from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452', @@ -1307,6 +1316,7 @@ const swapFlowApproveERC20TxMeta = { }; const swapFlowSwapERC20TxMeta = { chainId: '0x1', + type: TransactionType.swap, origin: process.env.MM_FOX_CODE, transaction: { from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452', @@ -1322,6 +1332,7 @@ const swapFlowSwapERC20TxMeta = { }; const swapFlowSwapEthTxMeta = { chainId: '0x1', + type: TransactionType.swap, origin: process.env.MM_FOX_CODE, transaction: { from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452', @@ -1340,61 +1351,61 @@ describe('Transactions utils :: getIsSwapApproveOrSwapTransaction', () => { it('returns true if the transaction is an approve tx in the swap flow for ERC20 from token', () => { const result = getIsSwapApproveOrSwapTransaction( swapFlowApproveERC20TxMeta.transaction.data, - swapFlowApproveERC20TxMeta.origin, swapFlowApproveERC20TxMeta.transaction.to, swapFlowApproveERC20TxMeta.chainId, + swapFlowApproveERC20TxMeta.type, ); expect(result).toBe(true); }); it('returns true if the transaction is a swap tx in the swap flow for ERC20 from token', () => { const result = getIsSwapApproveOrSwapTransaction( swapFlowSwapERC20TxMeta.transaction.data, - swapFlowSwapERC20TxMeta.origin, swapFlowSwapERC20TxMeta.transaction.to, swapFlowSwapERC20TxMeta.chainId, + swapFlowSwapERC20TxMeta.type, ); expect(result).toBe(true); }); it('returns true if the transaction is a swap tx in the swap flow for ETH from token', () => { const result = getIsSwapApproveOrSwapTransaction( swapFlowSwapEthTxMeta.transaction.data, - swapFlowSwapEthTxMeta.origin, swapFlowSwapEthTxMeta.transaction.to, swapFlowSwapEthTxMeta.chainId, + swapFlowSwapEthTxMeta.type, ); expect(result).toBe(true); }); it('returns false if the transaction is a send ERC20 tx', () => { const result = getIsSwapApproveOrSwapTransaction( sendERC20TxMeta.transaction.data, - sendERC20TxMeta.origin, sendERC20TxMeta.transaction.to, sendERC20TxMeta.chainId, + sendERC20TxMeta.type, ); expect(result).toBe(false); }); it('returns false if the transaction is a send ETH tx', () => { const result = getIsSwapApproveOrSwapTransaction( sendEthTxMeta.transaction.data, - sendEthTxMeta.origin, sendEthTxMeta.transaction.to, sendEthTxMeta.chainId, + sendEthTxMeta.type, ); expect(result).toBe(false); }); it('returns false if the transaction is a dapp tx', () => { const result = getIsSwapApproveOrSwapTransaction( dappTxMeta.transaction.data, - dappTxMeta.origin, dappTxMeta.transaction.to, dappTxMeta.chainId, + TransactionType.contractInteraction, ); expect(result).toBe(false); }); it('returns false if the transaction is a token transfer from swap origin', () => { const tokenTransferFromSwapOrigin = { chainId: '0x1', - origin: ORIGIN_METAMASK, + type: TransactionType.swap, transaction: { from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452', data: '0xa9059cbb000000000000000000000000dc738206f559bdae106894a62876a119e470aee20000000000000000000000000000000000000000000000000de0b6b3a7640000', @@ -1407,9 +1418,62 @@ describe('Transactions utils :: getIsSwapApproveOrSwapTransaction', () => { const result = getIsSwapApproveOrSwapTransaction( tokenTransferFromSwapOrigin.transaction.data, - tokenTransferFromSwapOrigin.origin, tokenTransferFromSwapOrigin.transaction.to, tokenTransferFromSwapOrigin.chainId, + tokenTransferFromSwapOrigin.type, + ); + + expect(result).toBe(false); + }); +}); + +describe('Transactions utils :: isHardwareSwapApproveOrSwapTransaction', () => { + const isHardwareAccountMock = jest.mocked(isHardwareAccount); + + beforeEach(() => { + isHardwareAccountMock.mockReset(); + }); + + it('returns true when it is a swap transaction from a hardware wallet', () => { + isHardwareAccountMock.mockReturnValue(true); + + const result = isHardwareSwapApproveOrSwapTransaction( + swapFlowSwapERC20TxMeta.transaction.data, + swapFlowSwapERC20TxMeta.transaction.to, + swapFlowSwapERC20TxMeta.chainId, + swapFlowSwapERC20TxMeta.type, + swapFlowSwapERC20TxMeta.transaction.from, + ); + + expect(result).toBe(true); + expect(isHardwareAccountMock).toHaveBeenCalledWith( + swapFlowSwapERC20TxMeta.transaction.from, + ); + }); + + it('returns false when it is a swap transaction but not from a hardware wallet', () => { + isHardwareAccountMock.mockReturnValue(false); + + const result = isHardwareSwapApproveOrSwapTransaction( + swapFlowSwapERC20TxMeta.transaction.data, + swapFlowSwapERC20TxMeta.transaction.to, + swapFlowSwapERC20TxMeta.chainId, + swapFlowSwapERC20TxMeta.type, + swapFlowSwapERC20TxMeta.transaction.from, + ); + + expect(result).toBe(false); + }); + + it('returns false when it is from a hardware wallet but not a swap transaction', () => { + isHardwareAccountMock.mockReturnValue(true); + + const result = isHardwareSwapApproveOrSwapTransaction( + sendERC20TxMeta.transaction.data, + sendERC20TxMeta.transaction.to, + sendERC20TxMeta.chainId, + sendERC20TxMeta.type, + sendERC20TxMeta.transaction.from, ); expect(result).toBe(false); @@ -1420,45 +1484,45 @@ describe('Transactions utils :: getIsSwapApproveTransaction', () => { it('returns true if the transaction is an approve ERC20 tx in the swap flow', () => { const result = getIsSwapApproveTransaction( swapFlowApproveERC20TxMeta.transaction.data, - swapFlowApproveERC20TxMeta.origin, swapFlowApproveERC20TxMeta.transaction.to, swapFlowApproveERC20TxMeta.chainId, + swapFlowApproveERC20TxMeta.type, ); expect(result).toBe(true); }); it('returns false if the transaction is a swap ERC20 tx in the swap flow', () => { const result = getIsSwapApproveTransaction( swapFlowSwapERC20TxMeta.transaction.data, - swapFlowSwapERC20TxMeta.origin, swapFlowSwapERC20TxMeta.transaction.to, swapFlowSwapERC20TxMeta.chainId, + swapFlowSwapERC20TxMeta.type, ); expect(result).toBe(false); }); it('returns false if the transaction is a send ETH tx', () => { const result = getIsSwapApproveTransaction( sendEthTxMeta.transaction.data, - sendEthTxMeta.origin, sendEthTxMeta.transaction.to, sendEthTxMeta.chainId, + sendEthTxMeta.type, ); expect(result).toBe(false); }); it('returns false if the transaction is a send ERC20 tx', () => { const result = getIsSwapApproveTransaction( sendERC20TxMeta.transaction.data, - sendERC20TxMeta.origin, sendERC20TxMeta.transaction.to, sendERC20TxMeta.chainId, + sendERC20TxMeta.type, ); expect(result).toBe(false); }); it('returns false if the transaction is a dapp tx', () => { const result = getIsSwapApproveTransaction( dappTxMeta.transaction.data, - dappTxMeta.origin, dappTxMeta.transaction.to, dappTxMeta.chainId, + TransactionType.contractInteraction, ); expect(result).toBe(false); }); @@ -1468,45 +1532,45 @@ describe('Transactions utils :: getIsSwapTransaction', () => { it('returns false if the transaction is an approve ERC20 tx in the swap flow', () => { const result = getIsSwapTransaction( swapFlowApproveERC20TxMeta.transaction.data, - swapFlowApproveERC20TxMeta.origin, swapFlowApproveERC20TxMeta.transaction.to, swapFlowApproveERC20TxMeta.chainId, + swapFlowApproveERC20TxMeta.type, ); expect(result).toBe(false); }); it('returns true if the transaction is a swap ERC20 tx in the swap flow', () => { const result = getIsSwapTransaction( swapFlowSwapERC20TxMeta.transaction.data, - swapFlowSwapERC20TxMeta.origin, swapFlowSwapERC20TxMeta.transaction.to, swapFlowSwapERC20TxMeta.chainId, + swapFlowSwapERC20TxMeta.type, ); expect(result).toBe(true); }); it('returns true if the transaction is a swap ETH tx in the swap flow', () => { const result = getIsSwapTransaction( swapFlowSwapEthTxMeta.transaction.data, - swapFlowSwapEthTxMeta.origin, swapFlowSwapEthTxMeta.transaction.to, swapFlowSwapEthTxMeta.chainId, + swapFlowSwapEthTxMeta.type, ); expect(result).toBe(true); }); it('returns false if the transaction is a send tx', () => { const result = getIsSwapTransaction( sendEthTxMeta.transaction.data, - sendEthTxMeta.origin, sendEthTxMeta.transaction.to, sendEthTxMeta.chainId, + sendEthTxMeta.type, ); expect(result).toBe(false); }); it('returns false if the transaction is a dapp tx', () => { const result = getIsSwapTransaction( dappTxMeta.transaction.data, - dappTxMeta.origin, dappTxMeta.transaction.to, dappTxMeta.chainId, + TransactionType.contractInteraction, ); expect(result).toBe(false); }); From 463ce808fb2fa21175ada4515c6681f094c62c7c Mon Sep 17 00:00:00 2001 From: asalsys Date: Tue, 17 Feb 2026 14:19:01 -0500 Subject: [PATCH 04/11] chore: consolidate navigation style - swaps team (#25879) ## **Description** This PR consolidates navigation styles across the codebase by replacing inline `navigationOptions` style definitions with the shared `getNavigationOptionsTitle` utility function. **Reason for change:** - Preparing the codebase for the upgrade to React Navigation v6 - Navigation header styles were defined inconsistently across components using the legacy `navigationOptions` pattern - Centralizing navigation options will make the migration to React Navigation v6's new API smoother and reduce the scope of changes required during the upgrade **Solution:** - Replaced inline style definitions with the centralized `getNavigationOptionsTitle` function from `app/components/UI/Navbar` - This ensures consistent navigation header styling across the app and simplifies the upcoming React Navigation v6 migration ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/23765 Part of React Navigation v6 upgrade preparation (split from original PR for easier code review by team) ## **Manual testing steps** Feature: Navigation header consistency Scenario: User navigates to screens affected by this PR Given the app is running When user navigates to any screen modified in this PR Then the navigation header should display correctly with proper title styling And the back button should function as expected ## **Screenshots/Recordings** ### **Before** N/A - No visual changes ### **After** N/A - No visual changes ## **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 navigation change limited to route targets/params for opening the in-app browser and the bridge block-explorer modal; main risk is misrouting if route signatures differ across navigators. > > **Overview** > Bridge transaction details and the block-explorer modal now navigate *directly* to `Routes.BROWSER.VIEW` with `newTabUrl`/`timestamp`, instead of routing through `Routes.BROWSER.HOME` with nested `screen`/`params`. > > For bridge transactions, opening the block explorer modal now navigates straight to `Routes.BRIDGE.MODALS.TRANSACTION_DETAILS_BLOCK_EXPLORER` rather than via the modal stack root. Associated tests were updated to assert the new navigation call shape. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 79a9f8da2fcd75ba4aeafbbcac0868754cf4de84. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../TransactionDetails/BlockExplorersModal.tsx | 18 ++++++------------ .../TransactionDetails.test.tsx | 7 ++----- .../TransactionDetails/TransactionDetails.tsx | 18 +++++++----------- 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/app/components/UI/Bridge/components/TransactionDetails/BlockExplorersModal.tsx b/app/components/UI/Bridge/components/TransactionDetails/BlockExplorersModal.tsx index d66460b5e14..eb79a0cba8c 100644 --- a/app/components/UI/Bridge/components/TransactionDetails/BlockExplorersModal.tsx +++ b/app/components/UI/Bridge/components/TransactionDetails/BlockExplorersModal.tsx @@ -110,12 +110,9 @@ const BlockExplorersModal = (props: BlockExplorersModalProps) => { } onPress={() => { - navigation.navigate(Routes.BROWSER.HOME, { - screen: Routes.BROWSER.VIEW, - params: { - newTabUrl: srcExplorerData.explorerTxUrl, - timestamp: Date.now(), - }, + navigation.navigate(Routes.BROWSER.VIEW, { + newTabUrl: srcExplorerData.explorerTxUrl, + timestamp: Date.now(), }); }} /> @@ -142,12 +139,9 @@ const BlockExplorersModal = (props: BlockExplorersModalProps) => { } onPress={() => { - navigation.navigate(Routes.BROWSER.HOME, { - screen: Routes.BROWSER.VIEW, - params: { - newTabUrl: bridgeDestExplorerData.explorerTxUrl, - timestamp: Date.now(), - }, + navigation.navigate(Routes.BROWSER.VIEW, { + newTabUrl: bridgeDestExplorerData.explorerTxUrl, + timestamp: Date.now(), }); }} /> diff --git a/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.test.tsx b/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.test.tsx index a397a19a4c1..96bba86e3ad 100644 --- a/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.test.tsx +++ b/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.test.tsx @@ -161,12 +161,9 @@ describe('BridgeTransactionDetails', () => { // Should navigate to browser, not to the modal expect(mockNavigate).toHaveBeenCalledWith( - Routes.BROWSER.HOME, + Routes.BROWSER.VIEW, expect.objectContaining({ - screen: Routes.BROWSER.VIEW, - params: expect.objectContaining({ - newTabUrl: expect.stringContaining('solana-tx-hash-123'), - }), + newTabUrl: expect.stringContaining('solana-tx-hash-123'), }), ); }); diff --git a/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.tsx b/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.tsx index 8c866ad93c8..8752c8b465e 100644 --- a/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.tsx +++ b/app/components/UI/Bridge/components/TransactionDetails/TransactionDetails.tsx @@ -395,23 +395,19 @@ export const BridgeTransactionDetails = ( onPress={() => { // For swaps, go directly to block explorer web view if (isSwap && swapSrcExplorerData?.explorerTxUrl) { - navigation.navigate(Routes.BROWSER.HOME, { - screen: Routes.BROWSER.VIEW, - params: { - newTabUrl: swapSrcExplorerData.explorerTxUrl, - timestamp: Date.now(), - }, + navigation.navigate(Routes.BROWSER.VIEW, { + newTabUrl: swapSrcExplorerData.explorerTxUrl, + timestamp: Date.now(), }); } else { // For bridges, show the modal with both explorers - navigation.navigate(Routes.BRIDGE.MODALS.ROOT, { - screen: - Routes.BRIDGE.MODALS.TRANSACTION_DETAILS_BLOCK_EXPLORER, - params: { + navigation.navigate( + Routes.BRIDGE.MODALS.TRANSACTION_DETAILS_BLOCK_EXPLORER, + { evmTxMeta: props.route.params.evmTxMeta, multiChainTx: props.route.params.multiChainTx, }, - }); + ); } }} /> From b446cc51c1caac7098ab2ef4f2f53d7f2e52af01 Mon Sep 17 00:00:00 2001 From: tommasini <46944231+tommasini@users.noreply.github.com> Date: Tue, 17 Feb 2026 20:08:57 +0000 Subject: [PATCH 05/11] ci: align build workflow with builds.yml, add iOS env setup and artifact renaming (#26141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** 1. **What is the reason for the change?** - The build workflow was failing on some runners (e.g. self-hosted) during iOS setup: missing Ruby 3.1.6 (rbenv) and/or wrong Gemfile path (`ios/ios/Gemfile`) when using job-level `BUNDLE_GEMFILE` with setup-e2e-env. The workflow also relied on remap env vars for secrets instead of `builds.yml`, and artifact upload/naming was inconsistent across build types. 2. **What is the improvement/solution?** - Use the **setup-e2e-env** action for iOS (same as build-ios-e2e): install Ruby 3.1.6, Bundler, and CocoaPods before `yarn setup:github-ci`, and remove job-level `BUNDLE_GEMFILE` so the action’s steps that run from `ios/` find the Gemfile correctly. - In `build.sh`, when `GITHUB_ACTIONS` is set, use `loadBuildConfig()` from `builds.yml` instead of remap*; keep remap* for Bitrise/local. - Add `scripts/rename-artifacts.js` and a single “Rename build artifacts” + upload flow for all build types (iOS and Android), with consistent naming (e.g. `metamask-{environment}-{buildType}-{version}-{buildNumber}`). To note, codefencing or built in feature flags values are not being applied properly, It's missing some changes that will be reflected on a new PR ([Bigger PR here for reference](https://github.com/MetaMask/metamask-mobile/pull/25177/)) Android and IOS is working Android build: https://github.com/MetaMask/metamask-mobile/actions/runs/22079638138 IOS build: https://github.com/MetaMask/metamask-mobile/actions/runs/22080704672 ## **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 modify CI build configuration and artifact handling for both iOS and Android; misconfiguration could break builds or produce incorrectly named/missing artifacts, but no production runtime code is affected. > > **Overview** > Build workflow now applies non-secret config from `builds.yml` into `GITHUB_ENV` and updates `scripts/build.sh` to load build config via `apply-build-config.js` when running in GitHub Actions (keeping legacy Bitrise env remapping for non-GHA runs). > > CI setup is adjusted to improve iOS reliability on self-hosted runners by adding the `setup-e2e-env` action (Ruby/Bundler/CocoaPods) and removing job-level `BUNDLE_GEMFILE`/Android gradle-properties copying. The workflow also adds `scripts/rename-artifacts.js` to standardize Android/iOS artifact names and replaces the prior iOS-only dev-build upload with consistent artifact uploads for both platforms. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f52c4d9a3948a6aba359bfd0c5dda62bdc1875f9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .github/workflows/build.yml | 47 +++++-- scripts/build.sh | 137 +++++++++++++------ scripts/rename-artifacts.js | 255 ++++++++++++++++++++++++++++++++++++ 3 files changed, 384 insertions(+), 55 deletions(-) create mode 100644 scripts/rename-artifacts.js diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0f2c7b4c675..cab037145d8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -38,7 +38,7 @@ permissions: id-token: write jobs: - # Load config + # Load config prepare: runs-on: ubuntu-latest outputs: @@ -80,9 +80,6 @@ jobs: # Android: Cirrus lg (large) runner for 8GB Gradle heap; iOS: GitHub-hosted macOS runs-on: ${{ matrix.platform == 'ios' && 'ghcr.io/cirruslabs/macos-runner:sequoia-xl' || 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-lg' }} environment: ${{ needs.prepare.outputs.github_environment }} - env: - # So bundle exec (in yarn pod:install) finds ios/Gemfile when running from repo root - BUNDLE_GEMFILE: ios/Gemfile steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 @@ -174,6 +171,17 @@ jobs: aws-secret-name: ${{ needs.prepare.outputs.signing_aws_secret }} android-keystore-path: ${{ needs.prepare.outputs.signing_android_keystore_path }} + # iOS: install Ruby, Bundler, and CocoaPods so setup:github-ci has a working Ruby (fixes self-hosted runners) + - name: Installing iOS Environment Setup + if: matrix.platform == 'ios' + timeout-minutes: 15 + uses: MetaMask/github-tools/.github/actions/setup-e2e-env@v1 + with: + platform: ios + configure-keystores: false + setup-simulator: false + ruby-version: '3.1.6' + - name: Setup project dependencies with retry uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 #v3.0.2 with: @@ -188,11 +196,6 @@ jobs: yarn setup:github-ci --no-build-ios fi - # Android: use CI Gradle settings (8GB heap, OOM handling) to avoid daemon OOM - - name: Use CI Gradle properties (Android) - if: matrix.platform == 'android' - run: cp android/gradle.properties.github android/gradle.properties - - name: Build ${{ matrix.platform }} timeout-minutes: 115 uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 #v3.0.2 @@ -207,10 +210,26 @@ jobs: SCRIPT_NAME="build:${{ matrix.platform }}:${BUILD_NAME//-/:}" yarn "$SCRIPT_NAME" - # Expo development builds: upload iOS .app for simulator (Runway-style artifact) - - name: Upload iOS Expo development build - if: matrix.platform == 'ios' && (inputs.build_name == 'main-dev' || inputs.build_name == 'flask-dev' || inputs.build_name == 'qa-dev') + # Rename build artifacts + - name: Rename ${{ matrix.platform }} artifacts + run: node scripts/rename-artifacts.js ${{ matrix.platform }} + + # Upload build artifacts (only if build succeeded) + - name: Upload iOS artifacts + if: matrix.platform == 'ios' + uses: actions/upload-artifact@v4 + with: + name: ios-${{ inputs.build_name }} + path: | + ios/build/Build/Products/ + ios/build/output/ + ios/build/*.xcarchive + if-no-files-found: error + + - name: Upload Android artifacts + if: matrix.platform == 'android' && success() uses: actions/upload-artifact@v4 with: - name: expo-dev-ios-${{ inputs.build_name }} - path: ios/build/Build/Products/Debug-iphonesimulator/ \ No newline at end of file + name: android-${{ inputs.build_name }} + path: android/app/build/outputs/ + if-no-files-found: error diff --git a/scripts/build.sh b/scripts/build.sh index b4582b5c2b6..4ec2125a085 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -159,23 +159,66 @@ checkParameters(){ #TODO: Add check for valid METAMASK_BUILD_TYPE once commands are fully refactored } -remapEnvVariable() { - # Get the old and new variable names - old_var_name=$1 - new_var_name=$2 +# ───────────────────────────────────────────────────────────────────────────── +# Load build configuration from builds.yml +# Used when GITHUB_ACTIONS is set (workflow already set secrets; this fills CONFIGURATION, IS_SIM_BUILD, etc.) +# ───────────────────────────────────────────────────────────────────────────── +loadBuildConfig() { + local build_type="$1" + local environment="$2" + + # Normalize environment name (production -> prod for build name) + local normalized_env="$environment" + case "$environment" in + production) normalized_env="prod" ;; + esac + + # Construct build name (e.g., main-prod, flask-dev) + local build_name="${build_type}-${normalized_env}" + + echo "" + echo "📦 Loading configuration from builds.yml for '${build_name}'..." + echo "" - # Check if the old variable exists - if [ -z "${!old_var_name}" ]; then - echo "Error: $old_var_name does not exist in the environment." - return 1 - fi + # Load config using apply-build-config.js + local config_output + config_output=$(node "${__DIRNAME__}/apply-build-config.js" "${build_name}" --export 2>&1) + local exit_code=$? + + if [ $exit_code -ne 0 ]; then + echo "❌ Failed to load build configuration" + echo "" + echo "Error: ${config_output}" + echo "" + echo "Available builds can be found in builds.yml" + echo "Run 'node scripts/validate-build-config.js' to check config validity." + return 1 + fi + + # Apply the configuration (exports environment variables) + eval "$config_output" - # Remap the variable - export $new_var_name="${!old_var_name}" + echo "✅ Configuration loaded from builds.yml" + echo " Build: ${build_name}" + echo "" - unset $old_var_name + return 0 +} - echo "Successfully remapped $old_var_name to $new_var_name." +# ───────────────────────────────────────────────────────────────────────────── +# Legacy env remapping (Bitrise). Used only when GITHUB_ACTIONS is not set. +# GitHub Actions uses loadBuildConfig + builds.yml; secrets are set with canonical names. +# ───────────────────────────────────────────────────────────────────────────── +remapEnvVariable() { + local old_var_name="$1" + local new_var_name="$2" + if [ -z "${!old_var_name}" ]; then + echo "Error: $old_var_name does not exist in the environment." + return 1 + fi + export $new_var_name="${!old_var_name}" + unset $old_var_name + echo "Successfully remapped $old_var_name to $new_var_name." } # Mapping for Main env variables in the dev environment @@ -935,40 +978,52 @@ checkParameters "$@" printTitle -# Map environment variables based on mode. -# TODO: MODE should be renamed to TARGET -# Skip environment variable remapping for expo-update platform +# ───────────────────────────────────────────────────────────────────────────── +# Load build configuration: GitHub Actions uses builds.yml; Bitrise uses legacy remap. +# Both paths supported until Bitrise is deprecated. +# ───────────────────────────────────────────────────────────────────────────── if [ "$PLATFORM" != "expo-update" ]; then + # Set flags for main builds if [ "$METAMASK_BUILD_TYPE" == "main" ]; then export GENERATE_BUNDLE=true # Used only for Android export PRE_RELEASE=true # Used mostly for iOS, for Android only deletes old APK and installs new one - if [ "$METAMASK_ENVIRONMENT" == "production" ]; then - remapMainProdEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "beta" ]; then - remapMainBetaEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "rc" ]; then - remapMainReleaseCandidateEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "exp" ]; then - remapMainExperimentalEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "test" ]; then - remapMainTestEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "e2e" ]; then - remapMainE2EEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "dev" ]; then - remapMainDevEnvVariables + fi + + if [ -n "${GITHUB_ACTIONS:-}" ]; then + # GitHub Actions: config from builds.yml (Apply build config step sets env; loadBuildConfig fills any gaps) + if ! loadBuildConfig "$METAMASK_BUILD_TYPE" "$METAMASK_ENVIRONMENT"; then + echo "❌ Build configuration failed. Exiting." + exit 1 fi - elif [ "$METAMASK_BUILD_TYPE" == "flask" ]; then - # TODO: Map environment variables based on environment - if [ "$METAMASK_ENVIRONMENT" == "production" ]; then - remapFlaskProdEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "test" ]; then - remapFlaskTestEnvVariables - elif [ "$METAMASK_ENVIRONMENT" == "e2e" ]; then - remapFlaskE2EEnvVariables + else + # Bitrise (or local): legacy env remapping (Bitrise secrets use per-env names, e.g. SEGMENT_WRITE_KEY_PROD) + if [ "$METAMASK_BUILD_TYPE" == "main" ]; then + if [ "$METAMASK_ENVIRONMENT" == "production" ]; then + remapMainProdEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "beta" ]; then + remapMainBetaEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "rc" ]; then + remapMainReleaseCandidateEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "exp" ]; then + remapMainExperimentalEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "test" ]; then + remapMainTestEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "e2e" ]; then + remapMainE2EEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "dev" ]; then + remapMainDevEnvVariables + fi + elif [ "$METAMASK_BUILD_TYPE" == "flask" ]; then + if [ "$METAMASK_ENVIRONMENT" == "production" ]; then + remapFlaskProdEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "test" ]; then + remapFlaskTestEnvVariables + elif [ "$METAMASK_ENVIRONMENT" == "e2e" ]; then + remapFlaskE2EEnvVariables + fi + elif [ "$METAMASK_BUILD_TYPE" == "qa" ] || [ "$METAMASK_BUILD_TYPE" == "QA" ]; then + remapEnvVariableQA fi - elif [ "$METAMASK_BUILD_TYPE" == "qa" ] || [ "$METAMASK_BUILD_TYPE" == "QA" ]; then - # TODO: Map environment variables based on environment - remapEnvVariableQA fi fi diff --git a/scripts/rename-artifacts.js b/scripts/rename-artifacts.js new file mode 100644 index 00000000000..372ecb38e2a --- /dev/null +++ b/scripts/rename-artifacts.js @@ -0,0 +1,255 @@ +#!/usr/bin/env node +/** + * Rename build artifacts with standardized naming convention. + * Called after Android/iOS builds complete to rename outputs. + * + * Usage: + * node scripts/rename-artifacts.js android + * node scripts/rename-artifacts.js ios + */ + +const fs = require('fs'); +const path = require('path'); +const { execSync } = require('child_process'); + +const platform = process.argv[2]; + +if (!platform || !['android', 'ios'].includes(platform)) { + console.error('Usage: node scripts/rename-artifacts.js '); + process.exit(1); +} + +// Get version from package.json +const packageJson = JSON.parse( + fs.readFileSync(path.join(__dirname, '../package.json'), 'utf8'), +); +const appVersion = packageJson.version; + +// Get build number from environment (GitHub Actions run number or 1 for local) +const buildNumber = process.env.GITHUB_RUN_NUMBER || '1'; + +// Required env vars +const buildType = process.env.METAMASK_BUILD_TYPE; +const environment = process.env.METAMASK_ENVIRONMENT; +const configuration = process.env.CONFIGURATION; + +if (!buildType || !environment) { + console.error( + '❌ Required env vars not set: METAMASK_BUILD_TYPE, METAMASK_ENVIRONMENT', + ); + process.exit(1); +} + +/** + * Find files matching a pattern + */ +function findFiles(dir, pattern) { + try { + const output = execSync(`find "${dir}" -name "${pattern}" -type f 2>/dev/null || true`, { + encoding: 'utf8', + }).trim(); + return output ? output.split('\n') : []; + } catch { + return []; + } +} + +/** + * Find directories matching a pattern + */ +function findDirs(dir, pattern) { + try { + const output = execSync(`find "${dir}" -name "${pattern}" -type d 2>/dev/null || true`, { + encoding: 'utf8', + }).trim(); + return output ? output.split('\n') : []; + } catch { + return []; + } +} + +/** + * Rename Android artifacts + */ +function renameAndroid() { + console.log('📦 Renaming Android artifacts...'); + + // Determine flavor based on build type + let appFlavor; + switch (buildType) { + case 'main': + appFlavor = 'prod'; + break; + case 'flask': + appFlavor = 'flask'; + break; + case 'qa': + appFlavor = 'qa'; + break; + default: + console.error(`❌ Unknown build type: ${buildType}`); + process.exit(1); + } + + // Determine if Debug or Release + const buildConfig = configuration === 'Debug' ? 'debug' : 'release'; + + // Set paths + const apkDir = path.join( + __dirname, + `../android/app/build/outputs/apk/${appFlavor}/${buildConfig}`, + ); + const bundleDir = path.join( + __dirname, + `../android/app/build/outputs/bundle/${appFlavor}Release`, + ); + + // Create new base name: metamask-{environment}-{buildType}-{version}-{buildNumber} + const newBaseName = `metamask-${environment}-${buildType}-${appVersion}-${buildNumber}`; + console.log(`📝 Renaming artifacts to: ${newBaseName}`); + + // Rename APK + const oldApk = path.join(apkDir, `app-${appFlavor}-${buildConfig}.apk`); + if (fs.existsSync(oldApk)) { + const newApk = path.join(apkDir, `${newBaseName}.apk`); + fs.copyFileSync(oldApk, newApk); + console.log(`✅ Renamed APK: ${newApk}`); + } else { + console.log(`⚠️ APK not found: ${oldApk}`); + } + + // Rename AAB (only for Release builds) + if (buildConfig === 'release') { + const oldAab = path.join(bundleDir, `app-${appFlavor}-release.aab`); + if (fs.existsSync(oldAab)) { + const newAab = path.join(bundleDir, `${newBaseName}.aab`); + fs.copyFileSync(oldAab, newAab); + console.log(`✅ Renamed AAB: ${newAab}`); + } else { + console.log(`⚠️ AAB not found: ${oldAab}`); + } + } + + // List final artifacts + console.log('📦 Final artifacts:'); + const outputDir = path.join(__dirname, '../android/app/build/outputs'); + const apks = findFiles(outputDir, '*.apk'); + const aabs = findFiles(outputDir, '*.aab'); + [...apks, ...aabs].forEach((file) => { + try { + const stats = fs.statSync(file); + const sizeMB = (stats.size / (1024 * 1024)).toFixed(2); + console.log(` ${file} (${sizeMB} MB)`); + } catch { + console.log(` ${file}`); + } + }); +} + +/** + * Rename iOS artifacts + */ +function renameIos() { + console.log('📦 Renaming iOS artifacts...'); + + const isSimBuild = process.env.IS_SIM_BUILD === 'true'; + + // Determine app name based on build type + let appName; + switch (buildType) { + case 'main': + appName = 'MetaMask'; + break; + case 'flask': + appName = 'MetaMask-Flask'; + break; + case 'qa': + appName = 'MetaMask-QA'; + break; + default: + console.error(`❌ Unknown build type: ${buildType}`); + process.exit(1); + } + + // Determine build paths based on simulator vs device + let buildDir, deviceType, binaryExtension; + if (isSimBuild) { + buildDir = path.join( + __dirname, + `../ios/build/Build/Products/${configuration || 'Release'}-iphonesimulator`, + ); + deviceType = 'simulator'; + binaryExtension = '.app'; + } else { + buildDir = path.join(__dirname, '../ios/build/output'); + deviceType = 'device'; + binaryExtension = '.ipa'; + } + + // Create new base name: metamask-{deviceType}-{environment}-{buildType}-{version}-{buildNumber} + const newBaseName = `metamask-${deviceType}-${environment}-${buildType}-${appVersion}-${buildNumber}`; + console.log(`📝 Renaming artifacts to: ${newBaseName}`); + + // Rename binary (.app or .ipa) + const oldBinary = path.join(buildDir, `${appName}${binaryExtension}`); + if (fs.existsSync(oldBinary)) { + const newBinary = path.join(buildDir, `${newBaseName}${binaryExtension}`); + // Use cp -r for directories (.app), regular copy for files (.ipa) + if (binaryExtension === '.app') { + execSync(`cp -r "${oldBinary}" "${newBinary}"`); + } else { + fs.copyFileSync(oldBinary, newBinary); + } + console.log(`✅ Renamed binary: ${newBinary}`); + } else { + console.log(`⚠️ Binary not found: ${oldBinary}`); + } + + // Rename xcarchive (only for device builds) + if (!isSimBuild) { + const oldArchive = path.join(__dirname, `../ios/build/${appName}.xcarchive`); + if (fs.existsSync(oldArchive)) { + const newArchive = path.join( + __dirname, + `../ios/build/${newBaseName}.xcarchive`, + ); + execSync(`cp -r "${oldArchive}" "${newArchive}"`); + console.log(`✅ Renamed archive: ${newArchive}`); + } else { + console.log(`⚠️ Archive not found: ${oldArchive}`); + } + } + + // List final artifacts + console.log('📦 Final artifacts:'); + if (isSimBuild) { + const apps = findDirs( + path.join(__dirname, '../ios/build/Build/Products'), + '*.app', + ); + apps.forEach((app) => { + console.log(` ${app}`); + }); + } else { + const ipas = findFiles(path.join(__dirname, '../ios/build/output'), '*.ipa'); + const archives = findDirs(path.join(__dirname, '../ios/build'), '*.xcarchive'); + [...ipas, ...archives].forEach((file) => { + try { + const stats = fs.statSync(file); + const sizeMB = (stats.size / (1024 * 1024)).toFixed(2); + console.log(` ${file} (${sizeMB} MB)`); + } catch { + console.log(` ${file}`); + } + }); + } +} + +// Execute +if (platform === 'android') { + renameAndroid(); +} else if (platform === 'ios') { + renameIos(); +} + +console.log('✅ Artifact renaming complete'); From e910e861f93196ccf7881c800eac4bfa169f00eb Mon Sep 17 00:00:00 2001 From: Bryan Fullam Date: Tue, 17 Feb 2026 21:09:54 +0100 Subject: [PATCH 06/11] fix: swaps vertical networks fixes (#26174) ## **Description** This PR fixes Bridge token selector network behavior across source and destination flows. Reason for change: - Source and destination selectors used different chain-ranking logic, causing inconsistent network visibility. - Visible network pills were component-local state, so selection context did not persist cleanly across picker flows. - Selecting tokens on unconfigured networks could block expected source selection behavior. Improvement/solution: - Unified source/destination network ranking to `selectAllowedChainRanking` (feature-flag ranking filtered by `ALLOWED_BRIDGE_CHAIN_IDS`). - Removed source-only filtering by user-configured networks in network pills and network list modal so all supported bridge networks are visible. - Lifted visible network-pill state into Redux (`visiblePillChainIds`) so ordering persists across picker flows. - Updated pills behavior so selecting a non-visible network promotes it to the front. - Added network auto-add during token selection via `PopularList` + `NetworkController.addNetwork`: - Source selection aborts if add-network fails. - Destination selection continues if add-network fails. ## **Changelog** CHANGELOG entry: Fixed Bridge token selectors to show all supported networks, persist selected network pills, and auto-add missing networks on token selection. ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Bridge token selector network visibility and selection behavior Scenario: source and destination selectors show the same supported network set Given a wallet with only a subset of networks configured And Bridge is enabled When the user opens the source token selector network controls And the user opens the destination token selector network controls Then both selectors show networks from the same allowed bridge chain ranking Scenario: selecting a non-visible network promotes it into pills Given the token selector shows limited visible network pills and a "+X more" control When the user opens the network list modal and selects a network not currently visible as a pill Then the selected network appears as the first visible pill And the remaining pills shift accordingly Scenario: selecting a token on an unconfigured source network adds the network Given a token exists on a supported network that is not currently configured in wallet settings When the user selects that token as source Then the app attempts to add the network configuration And source token selection completes if network add succeeds Scenario: source vs destination behavior when auto-add fails Given network add fails for a token on an unconfigured network When the user selects that token as source Then source selection is aborted and the selector closes When the user selects that token as destination Then the flow continues without blocking ``` ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **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 Bridge token selection flow and introduces automatic network addition via `NetworkController.addNetwork`, which could affect navigation and state if misconfigured or if the add-network call fails unexpectedly. > > **Overview** > Bridge token selection and network filtering are unified so source/dest pickers now use the same `selectAllowedChainRanking` list (feature-flag ranking filtered by `ALLOWED_BRIDGE_CHAIN_IDS`), and the network list modal no longer depends on route `type`. > > Network pill ordering is lifted into Redux via new `visiblePillChainIds`, so selecting a network from the modal can promote it into the visible pill set and the order persists across picker flows. > > Token selection now attempts to auto-add missing networks from `PopularList` via `NetworkController.addNetwork`, aborting *source* selection on failure but allowing *dest* selection to proceed; tests and bridge mocks are updated accordingly. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2b37e664f380efd83a047358467e09f81a8c9cb0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../UI/Bridge/_mocks_/bridgeReducerState.ts | 1 + .../BridgeTokenSelector.test.tsx | 5 +- .../BridgeTokenSelector.tsx | 13 +- .../NetworkListModal.test.tsx | 17 +- .../BridgeTokenSelector/NetworkListModal.tsx | 17 +- .../BridgeTokenSelector/NetworkPills.test.tsx | 93 ++++--- .../BridgeTokenSelector/NetworkPills.tsx | 42 +-- .../UI/Bridge/hooks/useTokenSelection.test.ts | 250 ++++++++++++------ .../UI/Bridge/hooks/useTokenSelection.ts | 46 ++++ app/core/redux/slices/bridge/index.test.ts | 164 +++++------- app/core/redux/slices/bridge/index.ts | 61 ++--- 11 files changed, 386 insertions(+), 323 deletions(-) diff --git a/app/components/UI/Bridge/_mocks_/bridgeReducerState.ts b/app/components/UI/Bridge/_mocks_/bridgeReducerState.ts index 230e98593b6..7e7c1160d8e 100644 --- a/app/components/UI/Bridge/_mocks_/bridgeReducerState.ts +++ b/app/components/UI/Bridge/_mocks_/bridgeReducerState.ts @@ -36,4 +36,5 @@ export const mockBridgeReducerState: BridgeState = { isSelectingToken: false, isDestTokenManuallySet: false, tokenSelectorNetworkFilter: undefined, + visiblePillChainIds: undefined, }; diff --git a/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.test.tsx b/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.test.tsx index 6925a752f22..faf949f7dae 100644 --- a/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.test.tsx +++ b/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.test.tsx @@ -95,10 +95,7 @@ jest.mock('../../../../../core/redux/slices/bridge', () => { }); return { selectBridgeFeatureFlags: jest.fn(() => getMockBridgeFeatureFlags()), - selectSourceChainRanking: jest.fn( - () => getMockBridgeFeatureFlags().chainRanking, - ), - selectDestChainRanking: jest.fn( + selectAllowedChainRanking: jest.fn( () => getMockBridgeFeatureFlags().chainRanking, ), setIsSelectingToken: jest.fn(() => ({ diff --git a/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.tsx b/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.tsx index 850725df3d8..d474e0c4133 100644 --- a/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.tsx +++ b/app/components/UI/Bridge/components/BridgeTokenSelector/BridgeTokenSelector.tsx @@ -27,8 +27,7 @@ import { CaipChainId } from '@metamask/utils'; import { useStyles } from '../../../../../component-library/hooks'; import TextFieldSearch from '../../../../../component-library/components/Form/TextFieldSearch'; import { - selectSourceChainRanking, - selectDestChainRanking, + selectAllowedChainRanking, selectTokenSelectorNetworkFilter, setIsSelectingToken, setTokenSelectorNetworkFilter, @@ -104,13 +103,7 @@ export const BridgeTokenSelector: React.FC = () => { [searchString], ); - // Use appropriate chain ranking based on selector type - const sourceChainRanking = useSelector(selectSourceChainRanking); - const destChainRanking = useSelector(selectDestChainRanking); - const enabledChainRanking = - route.params?.type === TokenSelectorType.Source - ? sourceChainRanking - : destChainRanking; + const enabledChainRanking = useSelector(selectAllowedChainRanking); // Set navigation options for header useEffect(() => { @@ -539,10 +532,8 @@ export const BridgeTokenSelector: React.FC = () => { onMorePress={() => navigation.navigate(Routes.BRIDGE.MODALS.ROOT, { screen: Routes.BRIDGE.MODALS.NETWORK_LIST_MODAL, - params: { type: route.params?.type }, }) } - type={route.params?.type} /> ({ useDispatch: jest.fn(), })); -jest.mock('@react-navigation/native', () => ({ - useRoute: () => ({ - params: { type: 'source' }, - }), -})); - jest.mock('../../../../../util/networks', () => ({ getNetworkImageSource: jest.fn(() => ({ uri: 'mock-network-icon' })), })); @@ -68,8 +61,7 @@ jest.mock( ); jest.mock('../../../../../core/redux/slices/bridge', () => ({ - selectSourceChainRanking: jest.fn(), - selectDestChainRanking: jest.fn(), + selectAllowedChainRanking: jest.fn(), selectTokenSelectorNetworkFilter: jest.fn(), setTokenSelectorNetworkFilter: jest.fn((chainId) => ({ type: 'bridge/setTokenSelectorNetworkFilter', @@ -119,10 +111,7 @@ describe('NetworkListModal', () => { (useDispatch as jest.Mock).mockReturnValue(mockDispatch); mockUseSelector.mockImplementation((selector: unknown) => { - if ( - selector === selectSourceChainRanking || - selector === selectDestChainRanking - ) { + if (selector === selectAllowedChainRanking) { return mockChainRanking; } if (selector === selectTokenSelectorNetworkFilter) { diff --git a/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkListModal.tsx b/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkListModal.tsx index 8fad13118db..474a8048cf9 100644 --- a/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkListModal.tsx +++ b/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkListModal.tsx @@ -16,29 +16,16 @@ import { AvatarSize } from '../../../../../component-library/components/Avatars/ import { CaipChainId } from '@metamask/utils'; import { getNetworkImageSource } from '../../../../../util/networks'; import { - selectSourceChainRanking, - selectDestChainRanking, + selectAllowedChainRanking, selectTokenSelectorNetworkFilter, setTokenSelectorNetworkFilter, } from '../../../../../core/redux/slices/bridge'; -import { useRoute, RouteProp } from '@react-navigation/native'; -import { TokenSelectorType } from '../../types'; - -interface NetworkListModalParams { - type: TokenSelectorType; -} const NetworkListModal: React.FC = () => { const dispatch = useDispatch(); - const route = - useRoute>(); const sheetRef = useRef(null); - const type = route.params?.type; - const sourceChainRanking = useSelector(selectSourceChainRanking); - const destChainRanking = useSelector(selectDestChainRanking); - const chainRanking = - type === TokenSelectorType.Source ? sourceChainRanking : destChainRanking; + const chainRanking = useSelector(selectAllowedChainRanking); const selectedChainId = useSelector(selectTokenSelectorNetworkFilter); const handleClose = useCallback(() => { diff --git a/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.test.tsx b/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.test.tsx index 366cbba84f7..1b78bb311c9 100644 --- a/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.test.tsx +++ b/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.test.tsx @@ -2,11 +2,17 @@ import React from 'react'; import { render, fireEvent } from '@testing-library/react-native'; import { NetworkPills } from './NetworkPills'; import { CaipChainId } from '@metamask/utils'; -import { useSelector } from 'react-redux'; -import { TokenSelectorType } from '../../types'; +import { useSelector, useDispatch } from 'react-redux'; +import { + selectAllowedChainRanking, + selectVisiblePillChainIds, +} from '../../../../../core/redux/slices/bridge'; + +const mockDispatch = jest.fn(); jest.mock('react-redux', () => ({ useSelector: jest.fn(), + useDispatch: jest.fn(), })); const mockUseSelector = useSelector as jest.Mock; @@ -34,6 +40,15 @@ const mockSmallChainRanking = [ { chainId: 'eip155:137' as CaipChainId, name: 'Polygon' }, ]; +jest.mock('../../../../../core/redux/slices/bridge', () => ({ + selectAllowedChainRanking: jest.fn(), + selectVisiblePillChainIds: jest.fn(), + setVisiblePillChainIds: jest.fn((ids) => ({ + type: 'bridge/setVisiblePillChainIds', + payload: ids, + })), +})); + jest.mock('@metamask/design-system-twrnc-preset', () => ({ useTailwind: () => ({ style: (...args: unknown[]) => args }), })); @@ -89,7 +104,16 @@ describe('NetworkPills', () => { beforeEach(() => { jest.clearAllMocks(); - mockUseSelector.mockReturnValue(mockChainRanking); + (useDispatch as jest.Mock).mockReturnValue(mockDispatch); + mockUseSelector.mockImplementation((selector: unknown) => { + if (selector === selectAllowedChainRanking) { + return mockChainRanking; + } + if (selector === selectVisiblePillChainIds) { + return undefined; // default: use first N from chainRanking + } + return undefined; + }); }); describe('rendering', () => { @@ -99,7 +123,6 @@ describe('NetworkPills', () => { selectedChainId={undefined} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -121,7 +144,6 @@ describe('NetworkPills', () => { selectedChainId={undefined} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -138,7 +160,6 @@ describe('NetworkPills', () => { selectedChainId={undefined} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -147,14 +168,16 @@ describe('NetworkPills', () => { }); it('does not render "+X more" when all networks are visible', () => { - mockUseSelector.mockReturnValue(mockSmallChainRanking); + mockUseSelector.mockImplementation((selector: unknown) => { + if (selector === selectVisiblePillChainIds) return undefined; + return mockSmallChainRanking; + }); const { queryByTestId } = render( , ); @@ -169,14 +192,16 @@ describe('NetworkPills', () => { { chainId: 'eip155:1' as CaipChainId, name: 'Ethereum' }, { chainId: 'eip155:56' as CaipChainId, name: 'BNB Chain' }, ]; - mockUseSelector.mockReturnValue(customRanking); + mockUseSelector.mockImplementation((selector: unknown) => { + if (selector === selectVisiblePillChainIds) return undefined; + return customRanking; + }); const { getByText, queryByText } = render( , ); @@ -197,7 +222,6 @@ describe('NetworkPills', () => { selectedChainId={'eip155:1' as CaipChainId} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -212,7 +236,6 @@ describe('NetworkPills', () => { selectedChainId={undefined} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -227,7 +250,6 @@ describe('NetworkPills', () => { selectedChainId={undefined} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -244,7 +266,6 @@ describe('NetworkPills', () => { selectedChainId={'eip155:56' as CaipChainId} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -257,7 +278,6 @@ describe('NetworkPills', () => { selectedChainId={undefined} onChainSelect={mockOnChainSelect} onMorePress={mockOnMorePress} - type={TokenSelectorType.Source} />, ); @@ -266,59 +286,62 @@ describe('NetworkPills', () => { }); describe('visible pills update on selection', () => { - it('adds non-visible chain to front of visible list when selected', () => { - const { rerender, getByText, queryByText } = render( + it('dispatches new visible list when non-visible chain is selected', () => { + const { rerender } = render( , ); - // Initially Polygon is not visible - expect(queryByText('Polygon')).toBeNull(); - - // Select Polygon (non-visible chain) by passing it as selectedChainId + // Select Polygon (non-visible chain) rerender( , ); - // Now Polygon should be visible (pushed to front, Solana popped) - expect(getByText('Polygon')).toBeTruthy(); - expect(queryByText('Solana')).toBeNull(); + // Should dispatch with Polygon at front, Solana popped + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'bridge/setVisiblePillChainIds', + payload: [ + 'eip155:137', // Polygon pushed to front + 'eip155:1', // Ethereum + 'eip155:56', // BNB Chain + 'bip122:000000000019d6689c085ae165831e93', // Bitcoin + ], + }); }); - it('does not change visible list when selecting an already visible chain', () => { - const { rerender, getByText } = render( + it('does not dispatch when selecting an already visible chain', () => { + const { rerender } = render( , ); + mockDispatch.mockClear(); + // Select Ethereum (already visible) rerender( , ); - // All original 4 should still be visible - expect(getByText('Ethereum')).toBeTruthy(); - expect(getByText('BNB Chain')).toBeTruthy(); - expect(getByText('Bitcoin')).toBeTruthy(); - expect(getByText('Solana')).toBeTruthy(); + // Should not dispatch setVisiblePillChainIds + expect(mockDispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: 'bridge/setVisiblePillChainIds', + }), + ); }); }); }); diff --git a/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.tsx b/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.tsx index 879c49a5b1b..bcb48e4fb24 100644 --- a/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.tsx +++ b/app/components/UI/Bridge/components/BridgeTokenSelector/NetworkPills.tsx @@ -1,5 +1,5 @@ -import React, { useEffect, useMemo, useRef, useState } from 'react'; -import { useSelector } from 'react-redux'; +import React, { useEffect, useMemo, useRef } from 'react'; +import { useSelector, useDispatch } from 'react-redux'; import { useTailwind } from '@metamask/design-system-twrnc-preset'; import { AvatarBaseShape, @@ -15,14 +15,14 @@ import { } from '@metamask/design-system-react-native'; import { strings } from '../../../../../../locales/i18n'; import { - selectSourceChainRanking, - selectDestChainRanking, + selectAllowedChainRanking, + selectVisiblePillChainIds, + setVisiblePillChainIds, } from '../../../../../core/redux/slices/bridge'; import { CaipChainId } from '@metamask/utils'; import { ScrollView } from 'react-native-gesture-handler'; import ButtonToggle from '../../../../../component-library/components-temp/Buttons/ButtonToggle'; import { ButtonSize } from '../../../../../component-library/components/Buttons/Button'; -import { TokenSelectorType } from '../../types'; import { getNetworkImageSource } from '../../../../../util/networks'; /** Maximum number of network pills visible in the horizontal list */ @@ -35,7 +35,6 @@ interface NetworkPillsProps { selectedChainId?: CaipChainId; onChainSelect: (chainId?: CaipChainId) => void; onMorePress: () => void; - type: TokenSelectorType; } interface ChainRankingEntry { @@ -55,20 +54,20 @@ export const NetworkPills: React.FC = ({ selectedChainId, onChainSelect, onMorePress, - type, }) => { const tw = useTailwind(); + const dispatch = useDispatch(); const scrollViewRef = useRef(null); - const sourceChainRanking = useSelector(selectSourceChainRanking); - const destChainRanking = useSelector(selectDestChainRanking); - const chainRanking: ChainRankingEntry[] = - type === TokenSelectorType.Source ? sourceChainRanking : destChainRanking; - - // Track which chain IDs are visible as pills - const [visibleChainIds, setVisibleChainIds] = useState(() => - getVisibleChainIds(chainRanking), + const chainRanking: ChainRankingEntry[] = useSelector( + selectAllowedChainRanking, ); + // Visible pill chain IDs from Redux (shared across source/dest pickers). + // Falls back to first N from chainRanking on initial mount. + const reduxVisibleChainIds = useSelector(selectVisiblePillChainIds); + const visibleChainIds = + reduxVisibleChainIds ?? getVisibleChainIds(chainRanking); + // Resolve visible chains to full entries from chainRanking const visibleChains = useMemo( () => @@ -85,11 +84,10 @@ export const NetworkPills: React.FC = ({ // Also scroll the pills to bring the selected network into view. // // Only `selectedChainId` is listed as a dependency because - // `visibleChainIds` is a state variable that this effect mutates; + // `visibleChainIds` is derived from Redux state that this effect updates; // including it would cause an infinite update loop. useEffect(() => { if (!selectedChainId) { - // "All" selected — scroll to start scrollViewRef.current?.scrollTo({ x: 0, animated: true }); return; } @@ -98,10 +96,12 @@ export const NetworkPills: React.FC = ({ if (existingIndex === -1) { // Non-visible network: push to front and scroll to start - setVisibleChainIds((prev) => [ - selectedChainId, - ...prev.slice(0, MAX_VISIBLE_PILLS - 1), - ]); + dispatch( + setVisiblePillChainIds([ + selectedChainId, + ...visibleChainIds.slice(0, MAX_VISIBLE_PILLS - 1), + ]), + ); scrollViewRef.current?.scrollTo({ x: 0, animated: true }); } else { // Already visible: scroll to bring it into view diff --git a/app/components/UI/Bridge/hooks/useTokenSelection.test.ts b/app/components/UI/Bridge/hooks/useTokenSelection.test.ts index c536f2d7996..521ae8b74ad 100644 --- a/app/components/UI/Bridge/hooks/useTokenSelection.test.ts +++ b/app/components/UI/Bridge/hooks/useTokenSelection.test.ts @@ -6,11 +6,12 @@ import { setIsDestTokenManuallySet, } from '../../../../core/redux/slices/bridge'; import { createMockToken } from '../testUtils/fixtures'; -import { TokenSelectorType } from '../types'; +import { BridgeToken, TokenSelectorType } from '../types'; const mockDispatch = jest.fn(); const mockHandleSwitchTokensInner = jest.fn().mockResolvedValue(undefined); const mockHandleSwitchTokens = jest.fn(() => mockHandleSwitchTokensInner); +const mockAddNetwork = jest.fn(); jest.mock('react-redux', () => ({ useDispatch: () => mockDispatch, @@ -37,40 +38,106 @@ jest.mock('./useAutoUpdateDestToken', () => ({ }), })); +jest.mock('../../../../core/Engine', () => ({ + __esModule: true, + default: { + context: { + NetworkController: { + addNetwork: mockAddNetwork, + }, + }, + }, +})); + +jest.mock('../../../../util/networks/customNetworks', () => { + const actual = jest.requireActual('../../../../util/networks/customNetworks'); + + return { + ...actual, + PopularList: [ + { + chainId: '0xa', + nickname: 'OP', + rpcUrl: 'https://op-mainnet.infura.io/v3/mock', + failoverRpcUrls: [], + ticker: 'ETH', + rpcPrefs: { + blockExplorerUrl: 'https://optimistic.etherscan.io', + }, + }, + ], + }; +}); + import { useSelector } from 'react-redux'; import { useIsNetworkEnabled } from './useIsNetworkEnabled'; const mockUseSelector = useSelector as jest.Mock; const mockUseIsNetworkEnabled = useIsNetworkEnabled as jest.Mock; -describe('useTokenSelection', () => { - const mockSourceToken = createMockToken({ - address: '0xsource', - symbol: 'SRC', - }); - const mockDestToken = createMockToken({ - address: '0xdest', - symbol: 'DST', - chainId: '0xa', +const mockSourceToken = createMockToken({ + address: '0xsource', + symbol: 'SRC', +}); +const mockDestToken = createMockToken({ + address: '0xdest', + symbol: 'DST', + chainId: '0xa', +}); +const mockDestAmount = '100'; + +interface SelectorState { + sourceToken: BridgeToken | null; + destToken: BridgeToken | null; + destAmount: string; + networkConfigurations: Record | undefined; +} + +const defaultSelectorState: SelectorState = { + sourceToken: mockSourceToken, + destToken: mockDestToken, + destAmount: mockDestAmount, + networkConfigurations: { + '0x1': { name: 'Ethereum Mainnet' }, + '0xa': { name: 'OP Mainnet' }, + } as Record, +}; + +const renderTokenSelectionHook = ( + type: TokenSelectorType, + selectorOverrides: Partial = {}, +) => { + const selectorState = { + ...defaultSelectorState, + ...selectorOverrides, + }; + + const selectorValues = [ + selectorState.sourceToken, + selectorState.destToken, + selectorState.destAmount, + selectorState.networkConfigurations, + ]; + let selectorCallIndex = 0; + mockUseSelector.mockImplementation(() => { + const value = selectorValues[selectorCallIndex % selectorValues.length]; + selectorCallIndex += 1; + return value; }); - const mockDestAmount = '100'; + return renderHook(() => useTokenSelection(type)); +}; + +describe('useTokenSelection', () => { beforeEach(() => { jest.clearAllMocks(); mockUseIsNetworkEnabled.mockReturnValue(true); + mockHandleSwitchTokensInner.mockResolvedValue(undefined); + mockAddNetwork.mockResolvedValue(undefined); }); describe('source token selection', () => { - beforeEach(() => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) // selectSourceToken - .mockReturnValueOnce(mockDestToken) // selectDestToken - .mockReturnValueOnce(mockDestAmount); // selectDestAmount - }); - it('dispatches setSourceToken and calls autoUpdateDestToken when selecting new source token', async () => { - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source); const newToken = createMockToken({ address: '0xnewtoken', symbol: 'NEW', @@ -86,9 +153,7 @@ describe('useTokenSelection', () => { }); it('calls handleSwitchTokens when selecting current dest token as source', async () => { - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source); await act(async () => { await result.current.handleTokenPress(mockDestToken); @@ -101,14 +166,7 @@ describe('useTokenSelection', () => { }); it('returns source token as selectedToken', () => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) - .mockReturnValueOnce(mockDestToken) - .mockReturnValueOnce(mockDestAmount); - - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source); expect(result.current.selectedToken).toBe(mockSourceToken); }); @@ -116,9 +174,7 @@ describe('useTokenSelection', () => { it('does not swap when dest network is disabled', async () => { mockUseIsNetworkEnabled.mockReturnValue(false); - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source); await act(async () => { await result.current.handleTokenPress(mockDestToken); @@ -130,17 +186,8 @@ describe('useTokenSelection', () => { }); describe('dest token selection', () => { - beforeEach(() => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) - .mockReturnValueOnce(mockDestToken) - .mockReturnValueOnce(mockDestAmount); - }); - it('dispatches setDestToken when selecting new dest token', async () => { - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Dest), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Dest); const newToken = createMockToken({ address: '0xnewdest', symbol: 'NEWDST', @@ -158,9 +205,7 @@ describe('useTokenSelection', () => { }); it('calls handleSwitchTokens when selecting current source token as dest', async () => { - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Dest), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Dest); await act(async () => { await result.current.handleTokenPress(mockSourceToken); @@ -172,29 +217,60 @@ describe('useTokenSelection', () => { }); it('returns dest token as selectedToken', () => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) - .mockReturnValueOnce(mockDestToken) - .mockReturnValueOnce(mockDestAmount); - - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Dest), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Dest); expect(result.current.selectedToken).toBe(mockDestToken); }); }); - describe('edge cases', () => { - it('handles null source token', async () => { - mockUseSelector - .mockReturnValueOnce(null) - .mockReturnValueOnce(mockDestToken) - .mockReturnValueOnce(mockDestAmount); + describe('network auto-add', () => { + it('aborts source selection when addNetwork rejects', async () => { + mockAddNetwork.mockRejectedValueOnce(new Error('addNetwork failed')); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source, { + networkConfigurations: {}, + }); + const sourceTokenOnMissingNetwork = createMockToken({ + chainId: '0xa', + }); + + await act(async () => { + await result.current.handleTokenPress(sourceTokenOnMissingNetwork); + }); - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), + expect(mockDispatch).not.toHaveBeenCalled(); + expect(mockAutoUpdateDestToken).not.toHaveBeenCalled(); + expect(mockGoBack).toHaveBeenCalledTimes(1); + }); + + it('continues dest selection when addNetwork rejects', async () => { + mockAddNetwork.mockRejectedValueOnce(new Error('addNetwork failed')); + const { result } = renderTokenSelectionHook(TokenSelectorType.Dest, { + networkConfigurations: {}, + }); + const destTokenOnMissingNetwork = createMockToken({ + address: '0xdest-new', + chainId: '0xa', + }); + + await act(async () => { + await result.current.handleTokenPress(destTokenOnMissingNetwork); + }); + + expect(mockDispatch).toHaveBeenCalledWith( + setDestToken(destTokenOnMissingNetwork), + ); + expect(mockDispatch).toHaveBeenCalledWith( + setIsDestTokenManuallySet(true), ); + expect(mockGoBack).toHaveBeenCalled(); + }); + }); + + describe('edge cases', () => { + it('handles null source token', async () => { + const { result } = renderTokenSelectionHook(TokenSelectorType.Source, { + sourceToken: null, + }); const newToken = createMockToken(); await act(async () => { @@ -207,14 +283,9 @@ describe('useTokenSelection', () => { }); it('handles null dest token', async () => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) - .mockReturnValueOnce(null) - .mockReturnValueOnce(mockDestAmount); - - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Dest), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Dest, { + destToken: null, + }); const newToken = createMockToken(); await act(async () => { @@ -226,14 +297,7 @@ describe('useTokenSelection', () => { }); it('does not swap when addresses match but chainIds differ', async () => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) - .mockReturnValueOnce(mockDestToken) - .mockReturnValueOnce(mockDestAmount); - - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source); const sameAddressToken = createMockToken({ address: mockDestToken.address, chainId: '0x5', @@ -252,14 +316,7 @@ describe('useTokenSelection', () => { }); it('does not swap when chainIds match but addresses differ', async () => { - mockUseSelector - .mockReturnValueOnce(mockSourceToken) - .mockReturnValueOnce(mockDestToken) - .mockReturnValueOnce(mockDestAmount); - - const { result } = renderHook(() => - useTokenSelection(TokenSelectorType.Source), - ); + const { result } = renderTokenSelectionHook(TokenSelectorType.Source); const sameChainToken = createMockToken({ address: '0xdifferent', chainId: mockDestToken.chainId, @@ -274,5 +331,22 @@ describe('useTokenSelection', () => { expect(mockAutoUpdateDestToken).toHaveBeenCalledWith(sameChainToken); expect(mockHandleSwitchTokens).not.toHaveBeenCalled(); }); + + it('falls back to empty network configurations when selector returns undefined', async () => { + const { result } = renderTokenSelectionHook(TokenSelectorType.Source, { + networkConfigurations: undefined as unknown as Record, + }); + const tokenOnMissingNetwork = createMockToken({ + chainId: '0x1', + }); + + await act(async () => { + await result.current.handleTokenPress(tokenOnMissingNetwork); + }); + + expect(mockDispatch).toHaveBeenCalledWith( + setSourceToken(tokenOnMissingNetwork), + ); + }); }); }); diff --git a/app/components/UI/Bridge/hooks/useTokenSelection.ts b/app/components/UI/Bridge/hooks/useTokenSelection.ts index 6745f771628..bff8627c3e1 100644 --- a/app/components/UI/Bridge/hooks/useTokenSelection.ts +++ b/app/components/UI/Bridge/hooks/useTokenSelection.ts @@ -13,6 +13,12 @@ import { BridgeToken, TokenSelectorType } from '../types'; import { useSwitchTokens } from './useSwitchTokens'; import { useIsNetworkEnabled } from './useIsNetworkEnabled'; import { useAutoUpdateDestToken } from './useAutoUpdateDestToken'; +import { RpcEndpointType } from '@metamask/network-controller'; +import { toHex } from '@metamask/controller-utils'; +import { Hex } from '@metamask/utils'; +import Engine from '../../../../core/Engine'; +import { selectNetworkConfigurations } from '../../../../selectors/networkController'; +import { PopularList } from '../../../../util/networks/customNetworks'; /** * Hook to manage token selection logic for Bridge token selector @@ -28,6 +34,7 @@ export const useTokenSelection = (type: TokenSelectorType) => { const { handleSwitchTokens } = useSwitchTokens(); const isDestNetworkEnabled = useIsNetworkEnabled(destToken?.chainId); const { autoUpdateDestToken } = useAutoUpdateDestToken(); + const networkConfigurations = useSelector(selectNetworkConfigurations); const handleTokenPress = useCallback( async (token: BridgeToken) => { @@ -40,6 +47,44 @@ export const useTokenSelection = (type: TokenSelectorType) => { token.address === otherToken.address && token.chainId === otherToken.chainId; + // Add the network if the user hasn't configured it yet + const isNetworkAdded = Boolean(networkConfigurations?.[token.chainId]); + if (!isNetworkAdded) { + const popularNetwork = PopularList.find( + (network) => network.chainId === token.chainId, + ); + if (popularNetwork) { + try { + const hexChainId = toHex(popularNetwork.chainId) as Hex; + const { blockExplorerUrl } = popularNetwork.rpcPrefs; + await Engine.context.NetworkController.addNetwork({ + chainId: hexChainId, + blockExplorerUrls: blockExplorerUrl ? [blockExplorerUrl] : [], + defaultRpcEndpointIndex: 0, + defaultBlockExplorerUrlIndex: blockExplorerUrl ? 0 : undefined, + name: popularNetwork.nickname, + nativeCurrency: popularNetwork.ticker, + rpcEndpoints: [ + { + url: popularNetwork.rpcUrl, + failoverUrls: popularNetwork.failoverRpcUrls, + name: popularNetwork.nickname, + type: RpcEndpointType.Custom, + }, + ], + }); + } catch { + if (isSourcePicker) { + // Source requires a configured network to sign transactions. + // Abort selection if the network couldn't be added. + navigation.goBack(); + return; + } + // Dest can fail silently + } + } + } + if (isSelectingOtherToken && sourceToken && destToken) { // Only allow swap if the destination network (which would become source) is enabled if (!isDestNetworkEnabled) { @@ -79,6 +124,7 @@ export const useTokenSelection = (type: TokenSelectorType) => { handleSwitchTokens, isDestNetworkEnabled, autoUpdateDestToken, + networkConfigurations, ], ); diff --git a/app/core/redux/slices/bridge/index.test.ts b/app/core/redux/slices/bridge/index.test.ts index 39d346bd6f9..3d0c2b9376a 100644 --- a/app/core/redux/slices/bridge/index.test.ts +++ b/app/core/redux/slices/bridge/index.test.ts @@ -14,10 +14,11 @@ import reducer, { selectBip44DefaultPair, selectGasIncludedQuoteParams, selectIsBridgeEnabledSource, - selectDestChainRanking, - selectSourceChainRanking, + selectAllowedChainRanking, setTokenSelectorNetworkFilter, selectTokenSelectorNetworkFilter, + setVisiblePillChainIds, + selectVisiblePillChainIds, } from '.'; import { BridgeToken, @@ -70,6 +71,7 @@ describe('bridge slice', () => { isMaxSourceAmount: false, isDestTokenManuallySet: false, tokenSelectorNetworkFilter: undefined, + visiblePillChainIds: undefined, }); }); }); @@ -558,88 +560,15 @@ describe('bridge slice', () => { }); }); - describe('selectSourceChainRanking', () => { - it('returns only supported and user-configured chains', () => { - const result = selectSourceChainRanking( + describe('selectAllowedChainRanking', () => { + it('returns all supported chains from feature flags', () => { + const result = selectAllowedChainRanking( mockRootState as unknown as RootState, ); - // Should return chains that are both in ALLOWED_BRIDGE_CHAIN_IDS - // and in the user's configured networks expect(Array.isArray(result)).toBe(true); expect(result.length).toBeGreaterThan(0); - // Ethereum (0x1) is allowed and configured in the mock state - expect(result.some((chain) => chain.chainId === 'eip155:1')).toBe(true); - }); - - it('filters out unsupported EVM chains from chainRanking', () => { - const mockState = cloneDeep(mockRootState); - mockState.engine.backgroundState.RemoteFeatureFlagController.remoteFeatureFlags.bridgeConfigV2.chainRanking = - [ - ...mockState.engine.backgroundState.RemoteFeatureFlagController - .remoteFeatureFlags.bridgeConfigV2.chainRanking, - { chainId: 'eip155:99999', name: 'Unsupported EVM Chain' }, - ]; - - const result = selectSourceChainRanking( - mockState as unknown as RootState, - ); - - expect(result.some((chain) => chain.chainId === 'eip155:99999')).toBe( - false, - ); - }); - - it('filters out unsupported non-EVM chains from chainRanking', () => { - const mockState = cloneDeep(mockRootState); - mockState.engine.backgroundState.RemoteFeatureFlagController.remoteFeatureFlags.bridgeConfigV2.chainRanking = - [ - ...mockState.engine.backgroundState.RemoteFeatureFlagController - .remoteFeatureFlags.bridgeConfigV2.chainRanking, - { chainId: 'cosmos:cosmoshub-4', name: 'Unsupported Cosmos Chain' }, - ]; - - const result = selectSourceChainRanking( - mockState as unknown as RootState, - ); - - expect( - result.some((chain) => chain.chainId === 'cosmos:cosmoshub-4'), - ).toBe(false); - }); - - it('filters out chains not in user-configured networks', () => { - const result = selectSourceChainRanking( - mockRootState as unknown as RootState, - ); - - // Optimism (0xa) is in chainRanking and ALLOWED_BRIDGE_CHAIN_IDS - // AND in the mock user's configured networks - const hasOptimism = result.some((chain) => chain.chainId === 'eip155:10'); - expect(hasOptimism).toBe(true); - - // Verify no chains appear that aren't in the user's configured networks - // The user only has Ethereum (0x1) and Optimism (0xa) configured as EVM networks - result.forEach((chain) => { - if (chain.chainId.startsWith('eip155:')) { - expect(['eip155:1', 'eip155:10']).toContain(chain.chainId); - } - }); - }); - }); - - describe('selectDestChainRanking', () => { - it('returns chainRanking from feature flags', () => { - const result = selectDestChainRanking( - mockRootState as unknown as RootState, - ); - - // Verify it returns an array with chain ranking data - expect(Array.isArray(result)).toBe(true); - expect(result.length).toBeGreaterThan(0); - - // Verify the structure of each item result.forEach((chain) => { expect(chain).toHaveProperty('chainId'); expect(chain).toHaveProperty('name'); @@ -647,22 +576,11 @@ describe('bridge slice', () => { expect(typeof chain.name).toBe('string'); }); - // Verify Ethereum is included (commonly in chainRanking) - const hasEthereum = result.some( - (chain) => chain.chainId === 'eip155:1' && chain.name === 'Ethereum', - ); - expect(hasEthereum).toBe(true); - }); - - it('returns all supported chains without filtering by user-configured networks', () => { - const result = selectDestChainRanking( - mockRootState as unknown as RootState, - ); - - // selectDestChainRanking should return all supported chains from feature flags - // This is the key difference from selectSourceChainRanking which filters - // by user-configured networks - expect(result.length).toBeGreaterThan(0); + expect( + result.some( + (chain) => chain.chainId === 'eip155:1' && chain.name === 'Ethereum', + ), + ).toBe(true); }); it('filters out unsupported EVM chains not in ALLOWED_BRIDGE_CHAIN_IDS', () => { @@ -671,10 +589,12 @@ describe('bridge slice', () => { [ ...mockState.engine.backgroundState.RemoteFeatureFlagController .remoteFeatureFlags.bridgeConfigV2.chainRanking, - { chainId: 'eip155:99999', name: 'Unsupported Future Chain' }, + { chainId: 'eip155:99999', name: 'Unsupported EVM Chain' }, ]; - const result = selectDestChainRanking(mockState as unknown as RootState); + const result = selectAllowedChainRanking( + mockState as unknown as RootState, + ); expect(result.some((chain) => chain.chainId === 'eip155:99999')).toBe( false, @@ -691,7 +611,9 @@ describe('bridge slice', () => { { chainId: 'cosmos:cosmoshub-4', name: 'Unsupported Cosmos Chain' }, ]; - const result = selectDestChainRanking(mockState as unknown as RootState); + const result = selectAllowedChainRanking( + mockState as unknown as RootState, + ); expect( result.some((chain) => chain.chainId === 'cosmos:cosmoshub-4'), @@ -764,6 +686,54 @@ describe('bridge slice', () => { }); }); + describe('setVisiblePillChainIds', () => { + it('sets the visible pill chain IDs', () => { + const chainIds = ['eip155:1', 'eip155:137'] as CaipChainId[]; + const action = setVisiblePillChainIds(chainIds); + const state = reducer(initialState, action); + + expect(state.visiblePillChainIds).toEqual(chainIds); + }); + + it('clears visible pill chain IDs when set to undefined', () => { + const stateWithPills = { + ...initialState, + visiblePillChainIds: ['eip155:1'] as CaipChainId[], + }; + const action = setVisiblePillChainIds(undefined); + const state = reducer(stateWithPills, action); + + expect(state.visiblePillChainIds).toBeUndefined(); + }); + }); + + describe('selectVisiblePillChainIds', () => { + it('returns undefined when no pills are set', () => { + const mockState = cloneDeep(mockRootState); + (mockState as any).bridge = { ...initialState }; + + const result = selectVisiblePillChainIds( + mockState as unknown as RootState, + ); + + expect(result).toBeUndefined(); + }); + + it('returns the set chain IDs', () => { + const mockState = cloneDeep(mockRootState); + (mockState as any).bridge = { + ...initialState, + visiblePillChainIds: ['eip155:1', 'eip155:10'], + }; + + const result = selectVisiblePillChainIds( + mockState as unknown as RootState, + ); + + expect(result).toEqual(['eip155:1', 'eip155:10']); + }); + }); + describe('selectIsBridgeEnabledSource - ALLOWED_BRIDGE_CHAIN_IDS filtering', () => { it('returns false for a chain in chainRanking but not in ALLOWED_BRIDGE_CHAIN_IDS', () => { const mockState = cloneDeep(mockRootState); diff --git a/app/core/redux/slices/bridge/index.ts b/app/core/redux/slices/bridge/index.ts index 7e4e11da88b..8ee87948588 100644 --- a/app/core/redux/slices/bridge/index.ts +++ b/app/core/redux/slices/bridge/index.ts @@ -67,6 +67,12 @@ export interface BridgeState { * When undefined, tokens from all chains are shown ("All" filter). */ tokenSelectorNetworkFilter: CaipChainId | undefined; + /** + * Ordered list of chain IDs shown as pills in the token selector. + * Shared across source and dest pickers so pill order persists within a session. + * When undefined, defaults to the first N entries from chainRanking. + */ + visiblePillChainIds: CaipChainId[] | undefined; } export const initialState: BridgeState = { @@ -87,6 +93,7 @@ export const initialState: BridgeState = { isGasIncluded7702Supported: false, isDestTokenManuallySet: false, tokenSelectorNetworkFilter: undefined, + visiblePillChainIds: undefined, }; const name = 'bridge'; @@ -181,6 +188,12 @@ const slice = createSlice({ ) => { state.tokenSelectorNetworkFilter = action.payload; }, + setVisiblePillChainIds: ( + state, + action: PayloadAction, + ) => { + state.visiblePillChainIds = action.payload; + }, }, extraReducers: (builder) => { builder.addCase(setSourceTokenExchangeRate.pending, (state) => { @@ -285,11 +298,11 @@ const isAllowedBridgeChainId = (caipChainId: string): boolean => { }; /** - * Base selector: filters chainRanking from feature flags by ALLOWED_BRIDGE_CHAIN_IDS. - * This is the single place where the allowlist check is applied to chainRanking. - * All other chain ranking selectors should derive from this. + * Selector that returns chainRanking from feature flags filtered by + * ALLOWED_BRIDGE_CHAIN_IDS. This ensures chains added to the remote flag + * in the future won't be surfaced by older app versions that lack support. */ -const selectAllowedChainRanking = createSelector( +export const selectAllowedChainRanking = createSelector( selectBridgeFeatureFlags, (bridgeFeatureFlags) => (bridgeFeatureFlags.chainRanking ?? []).filter((chain) => @@ -297,40 +310,6 @@ const selectAllowedChainRanking = createSelector( ), ); -/** - * Selector that returns all chains from chainRanking that are supported by this - * version of the client (filtered by ALLOWED_BRIDGE_CHAIN_IDS). - * Used by NetworkPills in DEST mode to show all available destination networks. - */ -export const selectDestChainRanking = selectAllowedChainRanking; - -/** - * Selector that returns the chainRanking filtered by: - * 1. Chains supported by this version of the client (via selectAllowedChainRanking) - * 2. User-configured networks - * Used by NetworkPills in SOURCE mode to show all networks the user has added. - */ -export const selectSourceChainRanking = createSelector( - selectAllowedChainRanking, - selectNetworkConfigurations, - (allowedChains, networkConfigurations) => { - const configuredChainIds = new Set(Object.keys(networkConfigurations)); - - return allowedChains.filter((chain) => { - const { chainId } = chain; - - // For EVM chains (eip155:*), extract the hex chain ID and check if user has it configured - if (chainId.startsWith('eip155:')) { - const hexChainId = formatChainIdToHex(chainId); - return configuredChainIds.has(hexChainId); - } - - // For non-EVM chains, check directly against the CAIP chain ID - return configuredChainIds.has(chainId); - }); - }, -); - /** * Factory selector that returns a function to check if bridge is enabled for a source chain. * Use this when you need to check multiple chain IDs or when the chain ID is determined after render. @@ -592,6 +571,11 @@ export const selectTokenSelectorNetworkFilter = createSelector( (bridgeState) => bridgeState.tokenSelectorNetworkFilter, ); +export const selectVisiblePillChainIds = createSelector( + selectBridgeState, + (bridgeState) => bridgeState.visiblePillChainIds, +); + export const selectIsDestTokenManuallySet = createSelector( selectBridgeState, (bridgeState) => bridgeState.isDestTokenManuallySet, @@ -675,4 +659,5 @@ export const { setIsGasIncludedSTXSendBundleSupported, setIsGasIncluded7702Supported, setTokenSelectorNetworkFilter, + setVisiblePillChainIds, } = actions; From e2eb8293936d53f83c7d1492117b545c7e27c067 Mon Sep 17 00:00:00 2001 From: Nico MASSART Date: Tue, 17 Feb 2026 21:28:25 +0100 Subject: [PATCH 07/11] refactor(analytics): migrate Batch 2-4: confirmations (#26139) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Phase 2 analytics migration (Batch 2-4): migrate Confirmations' signature metrics hook from MetaMetrics.getInstance() to the new analytics system (useAnalytics + AnalyticsEventBuilder). **Reason**: Deprecate MetaMetrics in favour of the shared analytics utility and AnalyticsController. **Changes**: `useSignatureMetrics.ts` now uses `useAnalytics()` and `createEventBuilder` from the analytics hook instead of `MetaMetrics.getInstance().trackEvent()` and `MetricsEventBuilder`; test mocks updated to mock `useAnalytics` at `app/components/hooks/useAnalytics/useAnalytics` with a chainable createEventBuilder. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MCWP-297 (Batch 2-4) ## **Manual testing steps** ```gherkin Feature: Confirmations analytics Scenario: user triggers a confirmations flow event Given app is open and user is in a confirmations flow When user performs an action that triggers analytics (e.g. signature requested, signature approved, signature rejected) Then the event is tracked on Mixpanel ``` ## **Screenshots/Recordings** N/A – analytics migration, no UI change. ## **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** > Refactor-only change to how analytics events are dispatched; behavior should be equivalent aside from potential wiring/mocking mismatches. > > **Overview** > Migrates the confirmations `useSignatureMetrics` hook off `MetaMetrics.getInstance()`/`MetricsEventBuilder` to the new analytics API via `useAnalytics()` with `trackEvent` + `createEventBuilder`. > > Updates `useSignatureMetrics.test.ts` to mock `useAnalytics` (including a chainable event builder) instead of mocking `MetaMetrics`, while keeping existing signature event coverage (requested/approved/rejected) and property expectations intact. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a27ea8f206eb6e838f2962fe4704a442f50cd8fe. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../signatures/useSignatureMetrics.test.ts | 25 +++++++------------ .../hooks/signatures/useSignatureMetrics.ts | 13 +++++----- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.test.ts b/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.test.ts index 071fa17be19..a4d091bbc6e 100644 --- a/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.test.ts +++ b/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.test.ts @@ -21,25 +21,18 @@ jest.mock('../../../../../util/address', () => ({ getAddressAccountType: (str: string) => str, })); -const mockTrackEvent = jest.fn().mockImplementation(); -jest.mock('../../../../../core/Analytics', () => ({ - ...jest.requireActual('../../../../../core/Analytics'), - MetaMetrics: { - getInstance: () => ({ - trackEvent: mockTrackEvent, - updateDataRecordingFlag: jest.fn(), - }), - }, -})); - +const mockTrackEvent = jest.fn(); const mockAddProperties = jest .fn() .mockImplementation(() => ({ build: () => ({}) })); -jest.mock('../../../../../core/Analytics/MetricsEventBuilder', () => ({ - ...jest.requireActual('../../../../../core/Analytics/MetricsEventBuilder'), - MetricsEventBuilder: { - createEventBuilder: () => ({ addProperties: mockAddProperties }), - }, +jest.mock('../../../../../components/hooks/useAnalytics/useAnalytics', () => ({ + useAnalytics: () => ({ + trackEvent: mockTrackEvent, + createEventBuilder: () => ({ + addProperties: mockAddProperties, + build: () => ({}), + }), + }), })); const SignatureMetrics = { diff --git a/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.ts b/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.ts index 41754cce20c..175ddc53b46 100644 --- a/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.ts +++ b/app/components/Views/confirmations/hooks/signatures/useSignatureMetrics.ts @@ -4,8 +4,8 @@ import { SecurityAlertResponse } from '@metamask/transaction-controller'; import { useCallback, useEffect, useMemo } from 'react'; import getDecimalChainId from '../../../../../util/networks/getDecimalChainId'; -import { MetricsEventBuilder } from '../../../../../core/Analytics/MetricsEventBuilder'; -import { MetaMetrics, MetaMetricsEvents } from '../../../../../core/Analytics'; +import { MetaMetricsEvents } from '../../../../../core/Analytics'; +import { useAnalytics } from '../../../../../components/hooks/useAnalytics/useAnalytics'; import { getAddressAccountType } from '../../../../../util/address'; import { getBlockaidMetricsParams } from '../../../../../util/blockaid'; import { getHostFromUrl } from '../../utils/generic'; @@ -67,6 +67,7 @@ const getAnalyticsParams = ( }; export const useSignatureMetrics = () => { + const { trackEvent, createEventBuilder } = useAnalytics(); const signatureRequest = useSignatureRequest(); const isSimulationEnabled = useTypedSignSimulationEnabled(); const { securityAlertResponse } = useSecurityAlertResponse(); @@ -115,13 +116,11 @@ export const useSignatureMetrics = () => { return; } - MetaMetrics.getInstance().trackEvent( - MetricsEventBuilder.createEventBuilder(event) - .addProperties(analyticsParams) - .build(), + trackEvent( + createEventBuilder(event).addProperties(analyticsParams).build(), ); }, - [analyticsParams], + [analyticsParams, trackEvent, createEventBuilder], ); useEffect(() => { From 8d668fad58f48de6c1ca32a5f4f34c99eceb41dc Mon Sep 17 00:00:00 2001 From: sophieqgu <37032128+sophieqgu@users.noreply.github.com> Date: Tue, 17 Feb 2026 16:22:14 -0500 Subject: [PATCH 08/11] chore: remove rewards optout section from settings (#26189) ## **Description** Remove optout section ## **Changelog** CHANGELOG entry: Remove opt out button from Rewards settings ## **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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > **Low Risk** > UI removal and test cleanup only; no changes to rewards linking/opt-in logic or data handling. > > **Overview** > Removes the Rewards settings *opt-out* section from the account group settings screen by dropping `RewardSettingsOptOut` from the list footer. > > Cleans up tests accordingly: deletes `RewardSettingsOptOut` and its unit test, and updates `RewardSettingsAccountGroupList` tests to stop mocking `useOptout` and to no longer assert opt-out-related `testID`s (footer now only verifies presence via `list-footer`). > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 565bbc27b03e8d45de7c01c1897d6e9287f26a16. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../RewardSettingsAccountGroupList.test.tsx | 30 +-- .../RewardSettingsAccountGroupList.tsx | 2 - .../Settings/RewardSettingsOptOut.test.tsx | 222 ------------------ .../Settings/RewardSettingsOptOut.tsx | 54 ----- 4 files changed, 3 insertions(+), 305 deletions(-) delete mode 100644 app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.test.tsx delete mode 100644 app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.tsx diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.test.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.test.tsx index 069fa1a4d43..a9bfb9de36d 100644 --- a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.test.tsx +++ b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.test.tsx @@ -6,7 +6,6 @@ import { useRewardOptinSummary, WalletWithAccountGroupsWithOptInStatus, } from '../../hooks/useRewardOptinSummary'; -import { useOptout } from '../../hooks/useOptout'; import { useMetrics } from '../../../../hooks/useMetrics'; import { useBulkLinkState } from '../../hooks/useBulkLinkState'; import { AccountWalletType } from '@metamask/account-api'; @@ -31,10 +30,6 @@ jest.mock('../../hooks/useRewardOptinSummary', () => ({ useRewardOptinSummary: jest.fn(), })); -jest.mock('../../hooks/useOptout', () => ({ - useOptout: jest.fn(), -})); - jest.mock('../../../../hooks/useMetrics', () => ({ useMetrics: jest.fn(), })); @@ -371,7 +366,6 @@ const mockUseSelector = useSelector as jest.MockedFunction; const mockUseRewardOptinSummary = useRewardOptinSummary as jest.MockedFunction< typeof useRewardOptinSummary >; -const mockUseOptout = useOptout as jest.MockedFunction; const mockUseMetrics = useMetrics as jest.MockedFunction; const mockUseBulkLinkState = useBulkLinkState as jest.MockedFunction< typeof useBulkLinkState @@ -452,7 +446,6 @@ describe('RewardSettingsAccountGroupList', () => { }, ] as unknown as WalletWithAccountGroupsWithOptInStatus[]; - const mockShowOptoutBottomSheet = jest.fn(); const mockTrackEvent = jest.fn(); const mockCreateEventBuilder = jest.fn(() => ({ addProperties: jest.fn().mockReturnThis(), @@ -497,13 +490,6 @@ describe('RewardSettingsAccountGroupList', () => { currentAccountGroupPartiallySupported: null, }); - // Mock useOptout hook - mockUseOptout.mockReturnValue({ - optout: jest.fn().mockResolvedValue(true), - isLoading: false, - showOptoutBottomSheet: mockShowOptoutBottomSheet, - }); - // Mock useMetrics hook mockUseMetrics.mockReturnValue({ trackEvent: mockTrackEvent, @@ -586,7 +572,7 @@ describe('RewardSettingsAccountGroupList', () => { expect(getByTestId('rewards-settings-skeleton-2')).toBeOnTheScreen(); }); - it('renders header and footer in loading state', () => { + it('renders header in loading state', () => { mockUseRewardOptinSummary.mockReturnValue({ byWallet: [], isLoading: true, @@ -600,7 +586,6 @@ describe('RewardSettingsAccountGroupList', () => { const { getByTestId } = render(); expect(getByTestId('rewards-settings-header')).toBeOnTheScreen(); - expect(getByTestId('rewards-settings-opt-out')).toBeOnTheScreen(); }); }); @@ -641,7 +626,7 @@ describe('RewardSettingsAccountGroupList', () => { expect(mockFetchOptInStatus).toHaveBeenCalledTimes(1); }); - it('renders header and footer in error state', () => { + it('renders header in error state', () => { mockUseRewardOptinSummary.mockReturnValue({ byWallet: [], isLoading: false, @@ -655,7 +640,6 @@ describe('RewardSettingsAccountGroupList', () => { const { getByTestId } = render(); expect(getByTestId('rewards-settings-header')).toBeOnTheScreen(); - expect(getByTestId('rewards-settings-opt-out')).toBeOnTheScreen(); }); }); @@ -680,13 +664,7 @@ describe('RewardSettingsAccountGroupList', () => { const { getByTestId } = render(); expect(getByTestId('rewards-settings-header')).toBeOnTheScreen(); - expect(getByTestId('rewards-settings-opt-out')).toBeOnTheScreen(); - }); - - it('renders opt-out button', () => { - const { getByTestId } = render(); - - expect(getByTestId('rewards-opt-out-button')).toBeOnTheScreen(); + expect(getByTestId('list-footer')).toBeOnTheScreen(); }); }); @@ -945,8 +923,6 @@ describe('RewardSettingsAccountGroupList', () => { // Test that all major components have testIDs expect(getByTestId('rewards-settings-flash-list')).toBeOnTheScreen(); expect(getByTestId('rewards-settings-header')).toBeOnTheScreen(); - expect(getByTestId('rewards-settings-opt-out')).toBeOnTheScreen(); - expect(getByTestId('rewards-opt-out-button')).toBeOnTheScreen(); }); }); diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx index 2314c116804..686b182c7ef 100644 --- a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx +++ b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx @@ -25,7 +25,6 @@ import Button, { ButtonVariants, } from '../../../../../component-library/components/Buttons/Button'; import RewardSettingsAccountGroup from './RewardSettingsAccountGroup'; -import RewardSettingsOptOut from './RewardSettingsOptOut'; import ReferredByCodeSection from './ReferredByCodeSection'; import { RewardSettingsAccountGroupListFlatListItem } from './types'; import RewardsErrorBanner from '../RewardsErrorBanner'; @@ -350,7 +349,6 @@ const RewardSettingsAccountGroupList: React.FC = () => { () => ( - ), [], diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.test.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.test.tsx deleted file mode 100644 index 30e105718e1..00000000000 --- a/app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.test.tsx +++ /dev/null @@ -1,222 +0,0 @@ -import React from 'react'; -import { render, fireEvent } from '@testing-library/react-native'; -import RewardSettingsOptOut from './RewardSettingsOptOut'; -import { useOptout } from '../../hooks/useOptout'; -import { useMetrics } from '../../../../hooks/useMetrics'; -import Routes from '../../../../../constants/navigation/Routes'; -import { RewardsMetricsButtons } from '../../utils'; - -jest.mock('@metamask/design-system-react-native', () => { - const ReactActual = jest.requireActual('react'); - const { Text: RNText, View } = jest.requireActual('react-native'); - - return { - Box: ({ - children, - testID, - ...props - }: { - children?: React.ReactNode; - testID?: string; - [key: string]: unknown; - }) => ReactActual.createElement(View, { testID, ...props }, children), - Text: ({ - children, - ...props - }: { - children?: React.ReactNode; - [key: string]: unknown; - }) => ReactActual.createElement(RNText, props, children), - TextVariant: { - BodySm: 'BodySm', - HeadingSm: 'HeadingSm', - }, - }; -}); - -jest.mock('../../../../../component-library/components/Buttons/Button', () => { - const ReactActual = jest.requireActual('react'); - const { TouchableOpacity, Text } = jest.requireActual('react-native'); - - return { - __esModule: true, - default: ({ - testID, - label, - onPress, - isDisabled, - }: { - testID?: string; - label: string; - onPress: () => void; - isDisabled?: boolean; - }) => - ReactActual.createElement( - TouchableOpacity, - { - testID, - onPress, - disabled: isDisabled, - accessibilityState: { disabled: isDisabled }, - }, - ReactActual.createElement(Text, {}, label), - ), - ButtonVariants: { - Secondary: 'secondary', - }, - }; -}); - -jest.mock('../../../../../../locales/i18n', () => ({ - strings: jest.fn((key: string) => key), -})); - -jest.mock('../../hooks/useOptout', () => ({ - useOptout: jest.fn(), -})); - -jest.mock('../../../../hooks/useMetrics', () => ({ - useMetrics: jest.fn(), - MetaMetricsEvents: { - REWARDS_PAGE_BUTTON_CLICKED: 'REWARDS_PAGE_BUTTON_CLICKED', - }, -})); - -const mockUseOptout = useOptout as jest.MockedFunction; -const mockUseMetrics = useMetrics as jest.MockedFunction; - -describe('RewardSettingsOptOut', () => { - const mockShowOptoutBottomSheet = jest.fn(); - const mockTrackEvent = jest.fn(); - const mockCreateEventBuilder = jest.fn(() => ({ - addProperties: jest.fn().mockReturnThis(), - build: jest.fn(() => ({ event: 'REWARDS_PAGE_BUTTON_CLICKED' })), - })); - - beforeEach(() => { - jest.clearAllMocks(); - - mockUseOptout.mockReturnValue({ - optout: jest.fn().mockResolvedValue(true), - isLoading: false, - showOptoutBottomSheet: mockShowOptoutBottomSheet, - }); - - mockUseMetrics.mockReturnValue({ - trackEvent: mockTrackEvent, - createEventBuilder: mockCreateEventBuilder, - addTraitsToUser: jest.fn(), - isEnabled: true, - enable: jest.fn(), - createDataDeletionTask: jest.fn(), - checkDataDeleteStatus: jest.fn(), - getDataDeletionTaskStatus: jest.fn(), - getDataDeletionTaskId: jest.fn(), - getDataDeletionTaskUrl: jest.fn(), - } as never); - }); - - describe('Rendering', () => { - it('renders the opt-out section with correct testID', () => { - const { getByTestId } = render(); - - expect(getByTestId('rewards-settings-opt-out')).toBeOnTheScreen(); - }); - - it('renders the opt-out button', () => { - const { getByTestId } = render(); - - expect(getByTestId('rewards-opt-out-button')).toBeOnTheScreen(); - }); - - it('renders the title and description text', () => { - const { getByText } = render(); - - expect(getByText('rewards.optout.title')).toBeOnTheScreen(); - expect(getByText('rewards.optout.description')).toBeOnTheScreen(); - }); - - it('renders the button label', () => { - const { getByText } = render(); - - expect(getByText('rewards.optout.confirm')).toBeOnTheScreen(); - }); - }); - - describe('Opt-out Button State', () => { - it('disables opt-out button when isLoading is true', () => { - mockUseOptout.mockReturnValue({ - optout: jest.fn().mockResolvedValue(true), - isLoading: true, - showOptoutBottomSheet: mockShowOptoutBottomSheet, - }); - - const { getByTestId } = render(); - - const optOutButton = getByTestId('rewards-opt-out-button'); - expect(optOutButton.props.accessibilityState?.disabled).toBe(true); - }); - - it('enables opt-out button when isLoading is false', () => { - mockUseOptout.mockReturnValue({ - optout: jest.fn().mockResolvedValue(true), - isLoading: false, - showOptoutBottomSheet: mockShowOptoutBottomSheet, - }); - - const { getByTestId } = render(); - - const optOutButton = getByTestId('rewards-opt-out-button'); - expect(optOutButton.props.accessibilityState?.disabled).toBe(false); - }); - }); - - describe('Opt-out Button Interaction', () => { - it('calls showOptoutBottomSheet with correct route when pressed', () => { - const { getByTestId } = render(); - - const optOutButton = getByTestId('rewards-opt-out-button'); - fireEvent.press(optOutButton); - - expect(mockShowOptoutBottomSheet).toHaveBeenCalledTimes(1); - expect(mockShowOptoutBottomSheet).toHaveBeenCalledWith( - Routes.REWARDS_SETTINGS_VIEW, - ); - }); - - it('tracks metric event when button is pressed', () => { - const { getByTestId } = render(); - - const optOutButton = getByTestId('rewards-opt-out-button'); - fireEvent.press(optOutButton); - - expect(mockCreateEventBuilder).toHaveBeenCalledWith( - 'REWARDS_PAGE_BUTTON_CLICKED', - ); - expect(mockTrackEvent).toHaveBeenCalledTimes(1); - }); - - it('tracks metric event with correct button_type property', () => { - const mockAddProperties = jest.fn().mockReturnThis(); - const mockBuild = jest.fn(() => ({ - event: 'REWARDS_PAGE_BUTTON_CLICKED', - properties: { button_type: RewardsMetricsButtons.OPT_OUT }, - })); - - mockCreateEventBuilder.mockReturnValue({ - addProperties: mockAddProperties, - build: mockBuild, - }); - - const { getByTestId } = render(); - - const optOutButton = getByTestId('rewards-opt-out-button'); - fireEvent.press(optOutButton); - - expect(mockAddProperties).toHaveBeenCalledWith({ - button_type: RewardsMetricsButtons.OPT_OUT, - }); - expect(mockBuild).toHaveBeenCalled(); - }); - }); -}); diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.tsx deleted file mode 100644 index 51335ff436c..00000000000 --- a/app/components/UI/Rewards/components/Settings/RewardSettingsOptOut.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import React, { memo, useCallback } from 'react'; -import { Box, Text, TextVariant } from '@metamask/design-system-react-native'; -import { strings } from '../../../../../../locales/i18n'; -import Button, { - ButtonVariants, -} from '../../../../../component-library/components/Buttons/Button'; -import Routes from '../../../../../constants/navigation/Routes'; -import { RewardsMetricsButtons } from '../../utils'; -import { useOptout } from '../../hooks/useOptout'; -import { useMetrics, MetaMetricsEvents } from '../../../../hooks/useMetrics'; - -const RewardSettingsOptOut: React.FC = () => { - const { isLoading: isOptingOut, showOptoutBottomSheet } = useOptout(); - const { trackEvent, createEventBuilder } = useMetrics(); - - const handleOptOutPress = useCallback(() => { - showOptoutBottomSheet(Routes.REWARDS_SETTINGS_VIEW); - trackEvent( - createEventBuilder(MetaMetricsEvents.REWARDS_PAGE_BUTTON_CLICKED) - .addProperties({ - button_type: RewardsMetricsButtons.OPT_OUT, - }) - .build(), - ); - }, [showOptoutBottomSheet, trackEvent, createEventBuilder]); - - return ( - - - - {strings('rewards.optout.title')} - - - {strings('rewards.optout.description')} - - - -