diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0f2c7b4c6750..cab037145d85 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/.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 e873b2d48a61..000000000000 --- 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/Nav/Main/RootRPCMethodsUI.js b/app/components/Nav/Main/RootRPCMethodsUI.js index e3bb9bc14533..bbf860429484 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 000000000000..0e7984408ebf --- /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 000000000000..c7d93d4be1f2 --- /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/Snaps/SnapUIAddress/useDisplayName.ts b/app/components/Snaps/SnapUIAddress/useDisplayName.ts index cf8db1cb8c0b..335e86acbe1c 100644 --- a/app/components/Snaps/SnapUIAddress/useDisplayName.ts +++ b/app/components/Snaps/SnapUIAddress/useDisplayName.ts @@ -9,7 +9,6 @@ import { RootState } from '../../../reducers'; import { selectInternalAccounts } from '../../../selectors/accountsController'; import { areAddressesEqual } from '../../../util/address'; import { selectAddressBookByChain } from '../../../selectors/addressBookController'; -import { selectMultichainAccountsState2Enabled } from '../../../selectors/featureFlagController/multichainAccounts'; import { selectAccountGroupsByAddress } from '../../../selectors/multichainAccounts/accounts'; import { toChecksumHexAddress } from '@metamask/controller-utils'; @@ -54,10 +53,6 @@ export const useDisplayName = ( areAddressesEqual(contact.address, address), ); - const showAccountGroupName = useSelector( - selectMultichainAccountsState2Enabled, - ); - const parsedAddress = isEip155 ? toChecksumHexAddress(address) : address; const accountGroups = useSelector((state: RootState) => selectAccountGroupsByAddress(state, [parsedAddress]), @@ -67,7 +62,7 @@ export const useDisplayName = ( const accountName = account?.metadata?.name; return ( - (showAccountGroupName && accountGroupName) || + accountGroupName || accountName || (isEip155 && addressBookEntry?.name) || undefined diff --git a/app/components/Snaps/SnapUIAvatar/SnapUIAvatar.tsx b/app/components/Snaps/SnapUIAvatar/SnapUIAvatar.tsx index f1f364fb5619..10e6f7166b93 100644 --- a/app/components/Snaps/SnapUIAvatar/SnapUIAvatar.tsx +++ b/app/components/Snaps/SnapUIAvatar/SnapUIAvatar.tsx @@ -5,7 +5,6 @@ import { isEvmAccountType } from '@metamask/keyring-api'; import { selectAvatarAccountType } from '../../../selectors/settings'; import AvatarAccount from '../../../component-library/components/Avatars/Avatar/variants/AvatarAccount'; import { AvatarSize } from '../../../component-library/components/Avatars/Avatar'; -import { selectMultichainAccountsState2Enabled } from '../../../selectors/featureFlagController/multichainAccounts'; import { selectAccountGroupsByAddress } from '../../../selectors/multichainAccounts/accounts'; import { RootState } from '../../../reducers'; @@ -34,8 +33,6 @@ export const SnapUIAvatar: React.FunctionComponent = ({ ); const avatarAccountType = useSelector(selectAvatarAccountType); - const useAccountGroups = useSelector(selectMultichainAccountsState2Enabled); - const accountGroups = useSelector((state: RootState) => selectAccountGroupsByAddress(state, [address]), ); @@ -45,8 +42,7 @@ export const SnapUIAvatar: React.FunctionComponent = ({ )?.address; // Display the account group address if it exists as the default. - const displayAddress = - useAccountGroups && accountGroupAddress ? accountGroupAddress : address; + const displayAddress = accountGroupAddress ?? address; return ( { }); 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 850725df3d89..d474e0c41336 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 8fad13118dbb..474a8048cf92 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 366cbba84f76..1b78bb311c94 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 879c49a5b1bb..bcb48e4fb24f 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/components/TransactionDetails/BlockExplorersModal.tsx b/app/components/UI/Bridge/components/TransactionDetails/BlockExplorersModal.tsx index d66460b5e14b..eb79a0cba8cf 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 a397a19a4c11..96bba86e3adb 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 8c866ad93c8e..8752c8b465e6 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, }, - }); + ); } }} /> diff --git a/app/components/UI/Bridge/hooks/useTokenSelection.test.ts b/app/components/UI/Bridge/hooks/useTokenSelection.test.ts index c536f2d79964..521ae8b74ad3 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 6745f7716289..bff8627c3e18 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/components/UI/Bridge/utils/transaction.test.ts b/app/components/UI/Bridge/utils/transaction.test.ts index 036ee27be3a8..405c28d595ef 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 b59e23716d6d..77f981e81568 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/components/UI/Ramp/Deposit/Views/Root/Root.test.tsx b/app/components/UI/Ramp/Deposit/Views/Root/Root.test.tsx index bd2b3a50028d..c3602761d549 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 29ca5540f0fc..43fd556a6a36 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 9f710673bf48..fa8775c55536 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, } } - /> + > + + + + diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.test.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.test.tsx index 069fa1a4d43b..a9bfb9de36d8 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 2314c1168047..686b182c7efe 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 30e105718e10..000000000000 --- 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 51335ff436c4..000000000000 --- 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')} - - - -