From f503a480980ecc22e4b9044fbd2d812844f167ee Mon Sep 17 00:00:00 2001 From: Micaela Estabillo <100321200+micaelae@users.noreply.github.com> Date: Thu, 16 Jan 2025 13:42:23 -0800 Subject: [PATCH] chore: remove bridge src token list from controller state (#29492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This reduces data stored in the BridgeController by removing stored bridge-specific token lists, and reusing token data from the assets controllers instead. Changes - **AssetList**: compare symbols in order to determine which token to highlight as "selected" - **AssetPickerModal**: fetch topAssets list on network change, yield tokens with balances first - **pages/bridge**: remove custom src token list (falls back to AssetPicker's default list) - **bridge-controller, ducks/bridge**: remove src token methods and states - **hooks/bridge**: remove allowlist check for tokens since these have moved to the bridge-api [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29492?quickstart=1) ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMS-1825 Depends on https://github.com/consensys-vertical-apps/va-mmcx-bridge-api/pull/136 ## **Manual testing steps** 1. Verify that Swap+Send token allowlists have not changed 2. Verify that Send token allowlists have not changed 3. Bridging 137:BNB -> 10:USDC should result in no quotes 4. Bridging 10:USDC -> 137:USDC should result in no quotes 5. Bridge src token should be pre-filled when navigating from asset page 6. Bridge inputs should be restored after fetching quotes and reopening extension ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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. --- app/scripts/constants/sentry-state.ts | 3 - .../bridge/bridge-controller.test.ts | 35 +---- .../controllers/bridge/bridge-controller.ts | 20 --- app/scripts/controllers/bridge/constants.ts | 3 - app/scripts/controllers/bridge/types.ts | 1 - app/scripts/metamask-controller.js | 4 - shared/types/bridge.ts | 4 - test/e2e/default-fixture.js | 2 - test/e2e/fixture-builder.js | 2 - test/e2e/tests/metrics/errors.spec.js | 1 - ...rs-after-init-opt-in-background-state.json | 2 - .../errors-after-init-opt-in-ui-state.json | 2 - ...s-before-init-opt-in-background-state.json | 4 +- .../errors-before-init-opt-in-ui-state.json | 4 +- .../asset-picker-modal/AssetList.tsx | 9 +- .../asset-picker-modal/asset-picker-modal.tsx | 60 ++++--- ui/ducks/bridge/actions.ts | 11 -- ui/ducks/bridge/bridge.test.ts | 18 --- ui/ducks/bridge/selectors.test.ts | 37 ----- ui/ducks/bridge/selectors.ts | 36 ++--- ui/ducks/bridge/utils.ts | 3 +- .../useTokensWithFiltering.test.ts.snap | 147 +----------------- ui/hooks/bridge/useBridgeTokens.ts | 39 ----- .../bridge/useTokensWithFiltering.test.ts | 51 +----- ui/hooks/bridge/useTokensWithFiltering.ts | 18 +-- ui/hooks/useTokensToSearch.js | 1 + ui/pages/bridge/index.tsx | 17 +- .../bridge/prepare/prepare-bridge-page.tsx | 67 ++++---- 28 files changed, 109 insertions(+), 492 deletions(-) delete mode 100644 ui/hooks/bridge/useBridgeTokens.ts diff --git a/app/scripts/constants/sentry-state.ts b/app/scripts/constants/sentry-state.ts index 91fb10f4efc7..00cb8606b6a9 100644 --- a/app/scripts/constants/sentry-state.ts +++ b/app/scripts/constants/sentry-state.ts @@ -110,9 +110,6 @@ export const SENTRY_BACKGROUND_STATE = { destTokens: {}, destTopAssets: [], destTokensLoadingStatus: true, - srcTokens: {}, - srcTopAssets: [], - srcTokensLoadingStatus: true, quoteRequest: { walletAddress: false, srcTokenAddress: true, diff --git a/app/scripts/controllers/bridge/bridge-controller.test.ts b/app/scripts/controllers/bridge/bridge-controller.test.ts index e35d3c41e67c..1c4864370b6b 100644 --- a/app/scripts/controllers/bridge/bridge-controller.test.ts +++ b/app/scripts/controllers/bridge/bridge-controller.test.ts @@ -11,8 +11,8 @@ import mockBridgeQuotesErc20Native from '../../../../test/data/bridge/mock-quote import mockBridgeQuotesNativeErc20 from '../../../../test/data/bridge/mock-quotes-native-erc20.json'; import mockBridgeQuotesNativeErc20Eth from '../../../../test/data/bridge/mock-quotes-native-erc20-eth.json'; import { - type QuoteResponse, RequestStatus, + type QuoteResponse, } from '../../../../shared/types/bridge'; import { decimalToHex } from '../../../../shared/modules/conversion.utils'; import BridgeController from './bridge-controller'; @@ -198,39 +198,6 @@ describe('BridgeController', function () { }); }); - it('selectSrcNetwork should set the bridge src tokens and top assets', async function () { - await bridgeController.selectSrcNetwork('0xa'); - expect(bridgeController.state.bridgeState.srcTokens).toStrictEqual({ - '0x0000000000000000000000000000000000000000': { - address: '0x0000000000000000000000000000000000000000', - decimals: 18, - iconUrl: './images/eth_logo.svg', - name: 'Ether', - symbol: 'ETH', - }, - '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984': { - address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984', - symbol: 'ABC', - decimals: 16, - aggregators: ['lifl', 'socket'], - }, - }); - expect( - bridgeController.state.bridgeState.srcTokensLoadingStatus, - ).toStrictEqual(RequestStatus.FETCHED); - expect(bridgeController.state.bridgeState.srcTopAssets).toStrictEqual([ - { - address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984', - symbol: 'ABC', - }, - ]); - expect(bridgeController.state.bridgeState.quoteRequest).toStrictEqual({ - slippage: 0.5, - srcTokenAddress: '0x0000000000000000000000000000000000000000', - walletAddress: undefined, - }); - }); - it('updateBridgeQuoteRequestParams should update the quoteRequest state', function () { bridgeController.updateBridgeQuoteRequestParams({ srcChainId: 1 }); expect(bridgeController.state.bridgeState.quoteRequest).toStrictEqual({ diff --git a/app/scripts/controllers/bridge/bridge-controller.ts b/app/scripts/controllers/bridge/bridge-controller.ts index 005e2d334e9f..2c1c43fa6ed1 100644 --- a/app/scripts/controllers/bridge/bridge-controller.ts +++ b/app/scripts/controllers/bridge/bridge-controller.ts @@ -96,10 +96,6 @@ export default class BridgeController extends StaticIntervalPollingController
{ - this.update((state) => { - state.bridgeState.srcTokensLoadingStatus = RequestStatus.LOADING; - return state; - }); - try { - await this.#setTopAssets(chainId, 'srcTopAssets'); - await this.#setTokens(chainId, 'srcTokens'); - } finally { - this.update((state) => { - state.bridgeState.srcTokensLoadingStatus = RequestStatus.FETCHED; - return state; - }); - } - }; - selectDestNetwork = async (chainId: Hex) => { this.update((state) => { state.bridgeState.destTokensLoadingStatus = RequestStatus.LOADING; diff --git a/app/scripts/controllers/bridge/constants.ts b/app/scripts/controllers/bridge/constants.ts index 65e2840cfb73..b6c07044adc5 100644 --- a/app/scripts/controllers/bridge/constants.ts +++ b/app/scripts/controllers/bridge/constants.ts @@ -20,10 +20,7 @@ export const DEFAULT_BRIDGE_CONTROLLER_STATE: BridgeControllerState = { chains: {}, }, }, - srcTokens: {}, - srcTokensLoadingStatus: undefined, destTokensLoadingStatus: undefined, - srcTopAssets: [], destTokens: {}, destTopAssets: [], quoteRequest: { diff --git a/app/scripts/controllers/bridge/types.ts b/app/scripts/controllers/bridge/types.ts index 811b8dfb119b..a417ddc4d074 100644 --- a/app/scripts/controllers/bridge/types.ts +++ b/app/scripts/controllers/bridge/types.ts @@ -25,7 +25,6 @@ type BridgeControllerActions = | BridgeControllerAction | BridgeControllerAction | BridgeControllerAction - | BridgeControllerAction | BridgeControllerAction | BridgeControllerAction; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index fa714cc1847c..9709c081b834 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -4179,10 +4179,6 @@ export default class MetamaskController extends EventEmitter { this.controllerMessenger, `${BRIDGE_CONTROLLER_NAME}:${BridgeBackgroundAction.GET_BRIDGE_ERC20_ALLOWANCE}`, ), - [BridgeUserAction.SELECT_SRC_NETWORK]: this.controllerMessenger.call.bind( - this.controllerMessenger, - `${BRIDGE_CONTROLLER_NAME}:${BridgeUserAction.SELECT_SRC_NETWORK}`, - ), [BridgeUserAction.SELECT_DEST_NETWORK]: this.controllerMessenger.call.bind( this.controllerMessenger, diff --git a/shared/types/bridge.ts b/shared/types/bridge.ts index 763c2670abad..b688164e9879 100644 --- a/shared/types/bridge.ts +++ b/shared/types/bridge.ts @@ -175,7 +175,6 @@ export enum RequestStatus { ERROR, } export enum BridgeUserAction { - SELECT_SRC_NETWORK = 'selectSrcNetwork', SELECT_DEST_NETWORK = 'selectDestNetwork', UPDATE_QUOTE_PARAMS = 'updateBridgeQuoteRequestParams', } @@ -186,9 +185,6 @@ export enum BridgeBackgroundAction { } export type BridgeControllerState = { bridgeFeatureFlags: BridgeFeatureFlags; - srcTokens: Record; - srcTopAssets: { address: string }[]; - srcTokensLoadingStatus?: RequestStatus; destTokensLoadingStatus?: RequestStatus; destTokens: Record; destTopAssets: { address: string }[]; diff --git a/test/e2e/default-fixture.js b/test/e2e/default-fixture.js index 51338fd3eea8..674b1146688b 100644 --- a/test/e2e/default-fixture.js +++ b/test/e2e/default-fixture.js @@ -140,8 +140,6 @@ function defaultFixture(inputChainId = CHAIN_IDS.LOCALHOST) { }, destTokens: {}, destTopAssets: [], - srcTokens: {}, - srcTopAssets: [], }, }, CurrencyController: { diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index 3c1128331ba6..6a629f97f503 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -457,8 +457,6 @@ class FixtureBuilder { }, destTokens: {}, destTopAssets: [], - srcTokens: {}, - srcTopAssets: [], }, }; return this; diff --git a/test/e2e/tests/metrics/errors.spec.js b/test/e2e/tests/metrics/errors.spec.js index aaa5c0536485..d22f71524120 100644 --- a/test/e2e/tests/metrics/errors.spec.js +++ b/test/e2e/tests/metrics/errors.spec.js @@ -875,7 +875,6 @@ describe('Sentry errors', function () { walletAddress: false, }, destTokensLoadingStatus: false, - srcTokensLoadingStatus: false, quotesLastFetched: true, quotesLoadingStatus: true, quotesRefreshCount: true, diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json index 0df251bd7661..770b8e251c08 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json @@ -69,8 +69,6 @@ "chains": { "0x1": "object", "0xa4b1": "object", "0xe708": "object" } } }, - "srcTokens": {}, - "srcTopAssets": {}, "destTokens": {}, "destTopAssets": {}, "quoteRequest": { diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json index ab3681c55088..4c63b10e175d 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -340,8 +340,6 @@ "chains": { "0x1": "object", "0xa4b1": "object", "0xe708": "object" } } }, - "srcTokens": {}, - "srcTopAssets": {}, "destTokens": {}, "destTopAssets": {}, "quoteRequest": { diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json index c85bf92f4334..7482981d32d7 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json @@ -152,9 +152,7 @@ } }, "destTokens": {}, - "destTopAssets": {}, - "srcTokens": {}, - "srcTopAssets": {} + "destTopAssets": {} } }, "SubjectMetadataController": { "subjectMetadata": "object" }, diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json index f86a5278e054..c3b3f3634ac5 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json @@ -161,9 +161,7 @@ } }, "destTokens": {}, - "destTopAssets": {}, - "srcTokens": {}, - "srcTopAssets": {} + "destTopAssets": {} } }, "TransactionController": { "transactions": "object" }, diff --git a/ui/components/multichain/asset-picker-amount/asset-picker-modal/AssetList.tsx b/ui/components/multichain/asset-picker-amount/asset-picker-modal/AssetList.tsx index 7693aaef4382..afbaae178f2c 100644 --- a/ui/components/multichain/asset-picker-amount/asset-picker-modal/AssetList.tsx +++ b/ui/components/multichain/asset-picker-amount/asset-picker-modal/AssetList.tsx @@ -64,7 +64,6 @@ export default function AssetList({ assetItemProps = {}, }: AssetListProps) { const t = useI18nContext(); - const selectedTokenAddress = asset?.address; const currentNetwork = useSelector(getCurrentNetwork); // If a network is provided, display tokens in that network @@ -101,11 +100,9 @@ export default function AssetList({ const isMatchingChainId = token.chainId === networkToUse?.chainId; const isMatchingAddress = - // the native asset can have an undefined, null, '', or zero address - (token.type === AssetType.native && - !token.address && - !selectedTokenAddress) || - tokenAddress === selectedTokenAddress?.toLowerCase(); + // the native asset can have an undefined, null, '', or zero address so compare symbols + (token.type === AssetType.native && token.symbol === asset?.symbol) || + tokenAddress === asset?.address?.toLowerCase(); const isSelected = isMatchingChainId && isMatchingAddress; const isDisabled = isTokenDisabled?.(token) ?? false; diff --git a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx index 670d3be14344..b9c212da8ea0 100644 --- a/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx +++ b/ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx @@ -42,6 +42,7 @@ import { getShouldHideZeroBalanceTokens, getTokenExchangeRates, getTokenList, + getUseExternalServices, } from '../../../../selectors'; import { getConversionRate, @@ -49,7 +50,6 @@ import { getNativeCurrency, } from '../../../../ducks/metamask/metamask'; import { useTokenTracker } from '../../../../hooks/useTokenTracker'; -import { getTopAssets } from '../../../../ducks/swaps/swaps'; import { getRenderableTokenData } from '../../../../hooks/useTokensToSearch'; import { getSwapsBlockedTokens } from '../../../../ducks/send'; import { isEqualCaseInsensitive } from '../../../../../shared/modules/string-utils'; @@ -60,6 +60,8 @@ import { import { useMultichainBalances } from '../../../../hooks/useMultichainBalances'; import { AvatarType } from '../../avatar-group/avatar-group.types'; import { NETWORK_TO_SHORT_NETWORK_NAME_MAP } from '../../../../../shared/constants/bridge'; +import { useAsyncResult } from '../../../../hooks/useAsyncResult'; +import { fetchTopAssetsList } from '../../../../pages/swaps/swaps.util'; import { ERC20Asset, NativeAsset, @@ -183,9 +185,19 @@ export function AssetPickerModal({ const { assetsWithBalance: multichainTokensWithBalance } = useMultichainBalances(); - // Swaps token list const tokenList = useSelector(getTokenList) as TokenListMap; - const topTokens = useSelector(getTopAssets, isEqual); + + const allowExternalServices = useSelector(getUseExternalServices); + // Swaps top tokens + const { value: topTokensResponse } = useAsyncResult< + { address: string }[] | undefined + >(async () => { + if (allowExternalServices && selectedNetwork?.chainId) { + return await fetchTopAssetsList(selectedNetwork.chainId); + } + return undefined; + }, [selectedNetwork?.chainId, allowExternalServices]); + const topTokens = topTokensResponse ?? []; const getIsDisabled = useCallback( ({ @@ -212,6 +224,18 @@ export function AssetPickerModal({ ); }, [tokensWithBalances, tokens]); + /** + * Generates a list of tokens sorted in this order + * - native tokens with balance + * - tokens with highest to lowest balance in selected currency + * - selected network's native token + * - matches URL token parameter + * - matches search query + * - detected tokens (without balance) + * - popularity + * - all other tokens + * - blocked tokens + */ const tokenListGenerator = useCallback( function* ( shouldAddToken: ( @@ -227,6 +251,18 @@ export function AssetPickerModal({ string?: string; }) > { + const blockedTokens = []; + + // Yield multichain tokens with balances + if (isMultiselectEnabled) { + for (const token of multichainTokensWithBalance) { + if (shouldAddToken(token.symbol, token.address, token.chainId)) { + yield token; + } + } + } + + // Yield the native token for the selected chain const nativeToken: AssetWithDisplayData = { address: null, symbol: nativeCurrency, @@ -248,26 +284,15 @@ export function AssetPickerModal({ yield nativeToken; } - const blockedTokens = []; - - // Yield multichain tokens with balances - if (isMultiselectEnabled) { - for (const token of multichainTokensWithBalance) { - if (shouldAddToken(token.symbol, token.address, token.chainId)) { - yield token; - } - } - } - for (const token of memoizedUsersTokens) { if (shouldAddToken(token.symbol, token.address, currentChainId)) { yield { ...token, chainId: currentChainId }; } } - // topTokens should already be sorted by popularity - for (const address of Object.keys(topTokens)) { - const token = tokenList?.[address]; + // topTokens are sorted by popularity + for (const topToken of topTokens) { + const token = tokenList?.[topToken.address]; if ( token && shouldAddToken(token.symbol, token.address, currentChainId) @@ -384,7 +409,6 @@ export function AssetPickerModal({ customTokenListGenerator, isMultiselectEnabled, selectedChainIds, - isMultiselectEnabled, selectedNetwork, ]); diff --git a/ui/ducks/bridge/actions.ts b/ui/ducks/bridge/actions.ts index e2ea02bad39d..467cffc52267 100644 --- a/ui/ducks/bridge/actions.ts +++ b/ui/ducks/bridge/actions.ts @@ -68,17 +68,6 @@ export const resetBridgeState = () => { }; // User actions -export const setFromChain = (chainId: Hex) => { - return async (dispatch: MetaMaskReduxDispatch) => { - dispatch( - callBridgeControllerMethod( - BridgeUserAction.SELECT_SRC_NETWORK, - chainId, - ), - ); - }; -}; - export const setToChain = (chainId: Hex) => { return async (dispatch: MetaMaskReduxDispatch) => { dispatch( diff --git a/ui/ducks/bridge/bridge.test.ts b/ui/ducks/bridge/bridge.test.ts index 30e2f0fe880a..a4123df75a01 100644 --- a/ui/ducks/bridge/bridge.test.ts +++ b/ui/ducks/bridge/bridge.test.ts @@ -17,7 +17,6 @@ import { setFromTokenInputValue, setToChain, setToToken, - setFromChain, resetInputFields, setToChainId, updateQuoteRequestParams, @@ -136,23 +135,6 @@ describe('Ducks - Bridge', () => { }); }); - describe('setFromChain', () => { - it('calls the selectSrcNetwork background action', async () => { - const mockSelectSrcNetwork = jest.fn().mockReturnValue({}); - setBackgroundConnection({ - [BridgeUserAction.SELECT_SRC_NETWORK]: mockSelectSrcNetwork, - } as never); - - await store.dispatch(setFromChain(CHAIN_IDS.MAINNET) as never); - - expect(mockSelectSrcNetwork).toHaveBeenCalledTimes(1); - expect(mockSelectSrcNetwork).toHaveBeenCalledWith( - '0x1', - expect.anything(), - ); - }); - }); - describe('resetInputFields', () => { it('resets to initalState', async () => { const state = store.getState().bridge; diff --git a/ui/ducks/bridge/selectors.test.ts b/ui/ducks/bridge/selectors.test.ts index f44b5d8dacea..e2d411148fb4 100644 --- a/ui/ducks/bridge/selectors.test.ts +++ b/ui/ducks/bridge/selectors.test.ts @@ -22,7 +22,6 @@ import { getFromChain, getFromChains, getFromToken, - getFromTokens, getIsBridgeTx, getToChain, getToChains, @@ -499,42 +498,6 @@ describe('Bridge selectors', () => { }); }); - describe('getFromTokens', () => { - it('returns src tokens from controller state', () => { - const state = createBridgeMockStore({ - bridgeSliceOverrides: { toChainId: '0x1' }, - bridgeStateOverrides: { - srcTokens: { '0x00': { address: '0x00', symbol: 'TEST' } }, - srcTopAssets: [{ address: '0x01', symbol: 'SYMB' }], - }, - }); - const result = getFromTokens(state as never); - - expect(result).toStrictEqual({ - fromTokens: { - '0x00': { address: '0x00', symbol: 'TEST' }, - }, - fromTopAssets: [{ address: '0x01', symbol: 'SYMB' }], - isLoading: false, - }); - }); - - it('returns src top assets from controller state', () => { - const state = createBridgeMockStore({ - bridgeSliceOverrides: { toChainId: '0x1' }, - bridgeStateOverrides: { - srcTokens: { '0x00': { address: '0x00', symbol: 'TEST' } }, - srcTopAssets: [{ address: '0x00', symbol: 'TEST' }], - }, - }); - const result = getFromTokens(state as never); - - expect(result.fromTopAssets).toStrictEqual([ - { address: '0x00', symbol: 'TEST' }, - ]); - }); - }); - describe('getBridgeQuotes', () => { it('returns quote list and fetch data, insufficientBal=false,quotesRefreshCount=5', () => { const state = createBridgeMockStore({ diff --git a/ui/ducks/bridge/selectors.ts b/ui/ducks/bridge/selectors.ts index f86745e69ae3..6ff6b397443e 100644 --- a/ui/ducks/bridge/selectors.ts +++ b/ui/ducks/bridge/selectors.ts @@ -134,20 +134,6 @@ export const getToChain = createDeepEqualSelector( toChains.find(({ chainId }) => chainId === toChainId), ); -export const getFromTokens = createDeepEqualSelector( - (state: BridgeAppState) => state.metamask.bridgeState.srcTokens, - (state: BridgeAppState) => state.metamask.bridgeState.srcTopAssets, - (state: BridgeAppState) => - state.metamask.bridgeState.srcTokensLoadingStatus === RequestStatus.LOADING, - (fromTokens, fromTopAssets, isLoading) => { - return { - isLoading, - fromTokens: fromTokens ?? {}, - fromTopAssets: fromTopAssets ?? [], - }; - }, -); - export const getToTokens = createDeepEqualSelector( (state: BridgeAppState) => state.metamask.bridgeState.destTokens, (state: BridgeAppState) => state.metamask.bridgeState.destTopAssets, @@ -430,16 +416,18 @@ const _getSelectedQuote = createSelector( ); export const getBridgeQuotes = createSelector( - _getSortedQuotesWithMetadata, - _getSelectedQuote, - (state) => state.metamask.bridgeState.quotesLastFetched, - (state) => - state.metamask.bridgeState.quotesLoadingStatus === RequestStatus.LOADING, - (state: BridgeAppState) => state.metamask.bridgeState.quotesRefreshCount, - (state: BridgeAppState) => state.metamask.bridgeState.quotesInitialLoadTime, - (state: BridgeAppState) => state.metamask.bridgeState.quoteFetchError, - getBridgeQuotesConfig, - getQuoteRequest, + [ + _getSortedQuotesWithMetadata, + _getSelectedQuote, + (state) => state.metamask.bridgeState.quotesLastFetched, + (state) => + state.metamask.bridgeState.quotesLoadingStatus === RequestStatus.LOADING, + (state: BridgeAppState) => state.metamask.bridgeState.quotesRefreshCount, + (state: BridgeAppState) => state.metamask.bridgeState.quotesInitialLoadTime, + (state: BridgeAppState) => state.metamask.bridgeState.quoteFetchError, + getBridgeQuotesConfig, + getQuoteRequest, + ], ( sortedQuotesWithMetadata, selectedQuote, diff --git a/ui/ducks/bridge/utils.ts b/ui/ducks/bridge/utils.ts index 050829d3ca65..4a5cbf034007 100644 --- a/ui/ducks/bridge/utils.ts +++ b/ui/ducks/bridge/utils.ts @@ -160,4 +160,5 @@ export const exchangeRatesFromNativeAndCurrencyRates = ( export const isNetworkAdded = ( v: NetworkConfiguration | AddNetworkFields | undefined, ): v is NetworkConfiguration => - !v || 'networkClientId' in v.rpcEndpoints[v.defaultRpcEndpointIndex]; + v !== undefined && + 'networkClientId' in v.rpcEndpoints[v.defaultRpcEndpointIndex]; diff --git a/ui/hooks/bridge/__snapshots__/useTokensWithFiltering.test.ts.snap b/ui/hooks/bridge/__snapshots__/useTokensWithFiltering.test.ts.snap index 8fceeffe9730..80a9a12221a0 100644 --- a/ui/hooks/bridge/__snapshots__/useTokensWithFiltering.test.ts.snap +++ b/ui/hooks/bridge/__snapshots__/useTokensWithFiltering.test.ts.snap @@ -1,74 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`useTokensWithFiltering should not return tokens that are not in the allowlist 1`] = ` -[ - { - "address": "0x0000000000000000000000000000000000000000", - "balance": "0.000000000000000014", - "chainId": "0xe708", - "decimals": 18, - "image": "./images/eth_logo.svg", - "string": "0.000000000000000014", - "symbol": "ETH", - "tokenFiatAmount": 3.53395e-14, - "type": "NATIVE", - }, - { - "address": "0x0000000000000000000000000000000000000000", - "balance": "0.00000000000000001", - "chainId": "0x1", - "decimals": 18, - "image": "./images/eth_logo.svg", - "string": "0.00000000000000001", - "symbol": "ETH", - "tokenFiatAmount": 2.5242500000000003e-14, - "type": "NATIVE", - }, - { - "address": "0x6b3595068778dd592e39a122f4f5a5cf09c90fe2", - "aggregators": [], - "balance": "", - "chainId": "0x1", - "decimals": 18, - "erc20": true, - "erc721": false, - "iconUrl": "images/contract/sushi.svg", - "image": "images/contract/sushi.svg", - "name": "SushiSwap", - "string": undefined, - "symbol": "SUSHI", - "type": "TOKEN", - }, - { - "address": "0x0000000000000000000000000000000000000000", - "balance": "0x0", - "chainId": "0x1", - "decimals": 18, - "iconUrl": "./images/eth_logo.svg", - "image": "./images/eth_logo.svg", - "name": "Ether", - "string": "0x0", - "symbol": "ETH", - "type": "NATIVE", - }, - { - "address": "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984", - "aggregators": [], - "balance": "", - "chainId": "0x1", - "decimals": 18, - "erc20": true, - "iconUrl": "images/contract/uni.svg", - "image": "images/contract/uni.svg", - "name": "Uniswap", - "string": undefined, - "symbol": "UNI", - "type": "TOKEN", - }, -] -`; - -exports[`useTokensWithFiltering should return all tokens when chainId === activeChainId, sorted by balance 1`] = ` +exports[`useTokensWithFiltering should return tokens that are not in the allowlist 1`] = ` [ { "address": "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984", @@ -115,78 +47,13 @@ exports[`useTokensWithFiltering should return all tokens when chainId === active }, { "address": "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984", - "balance": "", - "chainId": "0x1", - "decimals": 6, - "string": undefined, - "type": "TOKEN", - }, - { - "address": "0x6b3595068778dd592e39a122f4f5a5cf09c90fe2", - "aggregators": [], - "balance": "", - "chainId": "0x1", - "decimals": 18, - "erc20": true, - "erc721": false, - "iconUrl": "images/contract/sushi.svg", - "image": "images/contract/sushi.svg", - "name": "SushiSwap", - "string": undefined, - "symbol": "SUSHI", - "type": "TOKEN", - }, - { - "address": "0x0000000000000000000000000000000000000000", - "balance": "0x0", - "chainId": "0x1", - "decimals": 18, - "iconUrl": "./images/eth_logo.svg", - "image": "./images/eth_logo.svg", - "name": "Ether", - "string": "0x0", - "symbol": "ETH", - "type": "NATIVE", - }, - { - "address": "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984", - "aggregators": [], - "balance": "", - "chainId": "0x1", - "decimals": 18, - "erc20": true, - "iconUrl": "images/contract/uni.svg", - "image": "images/contract/uni.svg", - "name": "Uniswap", - "string": undefined, - "symbol": "UNI", - "type": "TOKEN", - }, - { - "address": "0xdac17f958d2ee523a2206206994597c13d831ec7", - "aggregators": [], - "balance": "", - "chainId": "0x1", - "decimals": 6, - "erc20": true, - "iconUrl": "images/contract/usdt.svg", - "image": "images/contract/usdt.svg", - "name": "Tether USD", - "string": undefined, - "symbol": "USDT", + "balance": "0", + "chainId": "0xe708", + "image": undefined, + "isNative": false, + "string": "0", + "tokenFiatAmount": null, "type": "TOKEN", }, - { - "address": "0x0000000000000000000000000000000000000000", - "balance": "0x0", - "chainId": "0x1", - "decimals": 18, - "iconUrl": "./images/eth_logo.svg", - "image": "./images/eth_logo.svg", - "name": "Ether", - "string": "0x0", - "symbol": "ETH", - "type": "NATIVE", - }, ] `; diff --git a/ui/hooks/bridge/useBridgeTokens.ts b/ui/hooks/bridge/useBridgeTokens.ts deleted file mode 100644 index 354b90bc4a09..000000000000 --- a/ui/hooks/bridge/useBridgeTokens.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { useEffect, useState } from 'react'; -import { useSelector } from 'react-redux'; -import { getAllBridgeableNetworks } from '../../ducks/bridge/selectors'; -import { fetchBridgeTokens } from '../../../shared/modules/bridge-utils/bridge.util'; - -// This hook is used to fetch the bridge tokens for all bridgeable networks -export const useBridgeTokens = () => { - const allBridgeChains = useSelector(getAllBridgeableNetworks); - - const [tokenAllowlistByChainId, setTokenAllowlistByChainId] = useState< - Record> - >({}); - - useEffect(() => { - const tokenAllowlistPromises = Promise.allSettled( - allBridgeChains.map( - async ({ chainId }) => - await fetchBridgeTokens(chainId).then((tokens) => ({ - [chainId]: new Set(Object.keys(tokens)), - })), - ), - ); - - (async () => { - const results = await tokenAllowlistPromises; - const tokenAllowlistResults = Object.fromEntries( - results.map((result) => { - if (result.status === 'fulfilled') { - return Object.entries(result.value)[0]; - } - return []; - }), - ); - setTokenAllowlistByChainId(tokenAllowlistResults); - })(); - }, [allBridgeChains.length]); - - return tokenAllowlistByChainId; -}; diff --git a/ui/hooks/bridge/useTokensWithFiltering.test.ts b/ui/hooks/bridge/useTokensWithFiltering.test.ts index e6903756bcfd..d7540ec95d27 100644 --- a/ui/hooks/bridge/useTokensWithFiltering.test.ts +++ b/ui/hooks/bridge/useTokensWithFiltering.test.ts @@ -19,7 +19,7 @@ describe('useTokensWithFiltering', () => { jest.clearAllMocks(); }); - it('should return all tokens when chainId === activeChainId, sorted by balance', () => { + it('should return tokens that are not in the allowlist', () => { const mockStore = createBridgeMockStore({ metamaskStateOverrides: { completedOnboarding: true, @@ -48,55 +48,6 @@ describe('useTokensWithFiltering', () => { { address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984' }, // USDC { address: '0xdac17f958d2ee523a2206206994597c13d831ec7' }, // USDT ], - { - [CHAIN_IDS.MAINNET]: new Set( - Object.keys(STATIC_MAINNET_TOKEN_LIST), - ), - }, - CHAIN_IDS.MAINNET, - ), - mockStore, - ); - // The first 10 tokens returned - const first10Tokens = [...result.current(() => true)].slice(0, 10); - expect(first10Tokens).toMatchSnapshot(); - }); - - it('should not return tokens that are not in the allowlist', () => { - const mockStore = createBridgeMockStore({ - metamaskStateOverrides: { - completedOnboarding: true, - allDetectedTokens: { - '0x1': { - '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc': [ - { - address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984', - decimals: 6, - }, // USDC - ], - }, - }, - }, - }); - const { result } = renderHookWithProvider( - () => - useTokensWithFiltering( - { - [NATIVE_TOKEN.address]: NATIVE_TOKEN, - ...STATIC_MAINNET_TOKEN_LIST, - }, - [ - { address: '0x6b3595068778dd592e39a122f4f5a5cf09c90fe2' }, // UNI - { address: NATIVE_TOKEN.address }, - { address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984' }, // USDC - { address: '0xdac17f958d2ee523a2206206994597c13d831ec7' }, // USDT - ], - // Only 1 token in allowlist - { - [CHAIN_IDS.MAINNET]: new Set([ - '0x6b3595068778dd592e39a122f4f5a5cf09c90fe2', - ]), - }, CHAIN_IDS.MAINNET, ), mockStore, diff --git a/ui/hooks/bridge/useTokensWithFiltering.ts b/ui/hooks/bridge/useTokensWithFiltering.ts index 61a92b7b9190..1b91a8f245ec 100644 --- a/ui/hooks/bridge/useTokensWithFiltering.ts +++ b/ui/hooks/bridge/useTokensWithFiltering.ts @@ -44,13 +44,11 @@ type FilterPredicate = ( * * @param tokenList - a mapping of token addresses in the selected chainId to token metadata from the bridge-api * @param topTokens - a list of top tokens from the swap-api - * @param tokenAddressAllowlistByChainId - a mapping of all supported chainIds to a Set of allowed token addresses * @param chainId - the selected src/dest chainId */ export const useTokensWithFiltering = ( tokenList: Record, topTokens: { address: string }[], - tokenAddressAllowlistByChainId: Record>, chainId?: ChainId | Hex, ) => { const { token: tokenAddressFromUrl } = useParams(); @@ -103,12 +101,6 @@ export const useTokensWithFiltering = ( }; }; - // This returns whether the token is blocked by any of the supported chainIds - const isTokenBlocked = (tokenAddress: string, tokenChainId: string) => - !tokenAddressAllowlistByChainId[tokenChainId]?.has( - tokenAddress.toLowerCase(), - ); - // shouldAddToken is a filter condition passed in from the AssetPicker that determines whether a token should be included const filteredTokenListGenerator = useCallback( (shouldAddToken: FilterPredicate) => @@ -137,10 +129,7 @@ export const useTokensWithFiltering = ( token.symbol, token.address ?? undefined, token.chainId, - ) && - (token.address - ? !isTokenBlocked(token.address, token.chainId) - : true) + ) ) { // If there's no address, set it to the native address in swaps/bridge yield { ...token, address: token.address || zeroAddress() }; @@ -154,10 +143,7 @@ export const useTokensWithFiltering = ( token.symbol, token.address ?? undefined, token.chainId, - ) && - (token.address - ? !isTokenBlocked(token.address, token.chainId) - : true) + ) ) { yield { ...token, diff --git a/ui/hooks/useTokensToSearch.js b/ui/hooks/useTokensToSearch.js index 657bb0afed09..1b9a9fdda960 100644 --- a/ui/hooks/useTokensToSearch.js +++ b/ui/hooks/useTokensToSearch.js @@ -93,6 +93,7 @@ export function getRenderableTokenData( decimals, name: name || tokenList[address?.toLowerCase()]?.name, rawFiat, + image: token.image || token.iconUrl, }; } diff --git a/ui/pages/bridge/index.tsx b/ui/pages/bridge/index.tsx index 15c3a0365cc9..8be38cc4f58c 100644 --- a/ui/pages/bridge/index.tsx +++ b/ui/pages/bridge/index.tsx @@ -17,11 +17,8 @@ import { ButtonIconSize, IconName, } from '../../components/component-library'; -import { - getCurrentChainId, - getSelectedNetworkClientId, -} from '../../../shared/modules/selectors/networks'; -import { getIsBridgeChain, getIsBridgeEnabled } from '../../selectors'; +import { getSelectedNetworkClientId } from '../../../shared/modules/selectors/networks'; +import { getIsBridgeEnabled } from '../../selectors'; import useBridging from '../../hooks/bridge/useBridging'; import { Content, @@ -30,7 +27,7 @@ import { Page, } from '../../components/multichain/pages/page'; import { useSwapsFeatureFlags } from '../swaps/hooks/useSwapsFeatureFlags'; -import { resetBridgeState, setFromChain } from '../../ducks/bridge/actions'; +import { resetBridgeState } from '../../ducks/bridge/actions'; import { useGasFeeEstimates } from '../../hooks/useGasFeeEstimates'; import { useBridgeExchangeRates } from '../../hooks/bridge/useBridgeExchangeRates'; import { useQuoteFetchEvents } from '../../hooks/bridge/useQuoteFetchEvents'; @@ -51,15 +48,7 @@ const CrossChainSwap = () => { const dispatch = useDispatch(); const isBridgeEnabled = useSelector(getIsBridgeEnabled); - const isBridgeChain = useSelector(getIsBridgeChain); const selectedNetworkClientId = useSelector(getSelectedNetworkClientId); - const chainId = useSelector(getCurrentChainId); - - useEffect(() => { - if (isBridgeChain && isBridgeEnabled && chainId) { - dispatch(setFromChain(chainId)); - } - }, [isBridgeChain, isBridgeEnabled, chainId]); const resetControllerAndInputStates = async () => { await dispatch(resetBridgeState()); diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.tsx index 6083a7d6c2e8..653f8869d84f 100644 --- a/ui/pages/bridge/prepare/prepare-bridge-page.tsx +++ b/ui/pages/bridge/prepare/prepare-bridge-page.tsx @@ -10,8 +10,8 @@ import classnames from 'classnames'; import { debounce } from 'lodash'; import { useHistory, useLocation } from 'react-router-dom'; import { BigNumber } from 'bignumber.js'; +import { type TokenListMap } from '@metamask/assets-controllers'; import { - setFromChain, setFromToken, setFromTokenInputValue, setSelectedQuote, @@ -27,7 +27,6 @@ import { getFromChain, getFromChains, getFromToken, - getFromTokens, getQuoteRequest, getSlippage, getToChain, @@ -90,8 +89,7 @@ import useRamps from '../../../hooks/ramps/useRamps/useRamps'; import { getNativeCurrency } from '../../../ducks/metamask/metamask'; import useLatestBalance from '../../../hooks/bridge/useLatestBalance'; import { useCountdownTimer } from '../../../hooks/bridge/useCountdownTimer'; -import { useBridgeTokens } from '../../../hooks/bridge/useBridgeTokens'; -import { getCurrentKeyring, getLocale } from '../../../selectors'; +import { getCurrentKeyring, getLocale, getTokenList } from '../../../selectors'; import { isHardwareKeyring } from '../../../helpers/utils/hardware'; import { SECOND } from '../../../../shared/constants/time'; import { BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE } from '../../../../shared/constants/bridge'; @@ -104,11 +102,11 @@ const PrepareBridgePage = () => { const t = useI18nContext(); const fromToken = useSelector(getFromToken); - const { - fromTokens, - fromTopAssets, - isLoading: isFromTokensLoading, - } = useSelector(getFromTokens); + const fromTokens = useSelector(getTokenList) as TokenListMap; + const isFromTokensLoading = useMemo( + () => Object.keys(fromTokens).length === 0, + [fromTokens], + ); const toToken = useSelector(getToToken); const { @@ -175,17 +173,9 @@ const PrepareBridgePage = () => { fromChain?.chainId, ); - const tokenAddressAllowlistByChainId = useBridgeTokens(); - const fromTokenListGenerator = useTokensWithFiltering( - fromTokens, - fromTopAssets, - tokenAddressAllowlistByChainId, - fromChain?.chainId, - ); const toTokenListGenerator = useTokensWithFiltering( toTokens, toTopAssets, - tokenAddressAllowlistByChainId, toChain?.chainId, ); @@ -217,22 +207,32 @@ const PrepareBridgePage = () => { }, [rotateSwitchTokens]); useEffect(() => { + // If there's an active quote, assume that the user is returning to the page if (activeQuote) { // Get input data from active quote - const { srcAsset, destAsset, destChainId } = activeQuote.quote; - const quoteSrcToken = fromTokens[srcAsset.address.toLowerCase()]; + const { srcAsset, destAsset, destChainId, srcChainId } = + activeQuote.quote; const quoteDestChainId = decimalToPrefixedHex(destChainId); - const quoteDestToken = toTokens[destAsset.address.toLowerCase()]; + const quoteSrcChainId = decimalToPrefixedHex(srcChainId); - if (quoteSrcToken && quoteDestToken && quoteDestChainId) { + if (srcAsset && destAsset && quoteDestChainId) { // Set inputs to values from active quote - dispatch(setFromTokenInputValue(null)); + dispatch(setToChainId(quoteDestChainId)); dispatch( - setFromToken({ ...quoteSrcToken, image: quoteSrcToken.iconUrl }), + setToToken({ + ...destAsset, + chainId: quoteDestChainId, + image: destAsset.icon, + address: destAsset.address.toLowerCase(), + }), ); - dispatch(setToChainId(quoteDestChainId)); dispatch( - setToToken({ ...quoteDestToken, image: quoteDestToken.iconUrl }), + setFromToken({ + ...srcAsset, + chainId: quoteSrcChainId, + image: srcAsset.icon, + address: srcAsset.address.toLowerCase(), + }), ); } } else { @@ -326,7 +326,7 @@ const PrepareBridgePage = () => { const history = useHistory(); useEffect(() => { - if (!fromChain?.chainId || Object.keys(fromTokens).length === 0) { + if (!fromChain?.chainId || isFromTokensLoading) { return; } @@ -350,10 +350,14 @@ const PrepareBridgePage = () => { removeTokenFromUrl(); break; case fromTokens[tokenAddressFromUrl]?.address?.toLowerCase(): { - // If there is a matching fromToken, set it as the fromToken + // If there is a match, set it as the fromToken const matchedToken = fromTokens[tokenAddressFromUrl]; dispatch( - setFromToken({ ...matchedToken, image: matchedToken.iconUrl }), + setFromToken({ + ...matchedToken, + image: matchedToken.iconUrl, + chainId: fromChain.chainId, + }), ); removeTokenFromUrl(); break; @@ -363,7 +367,7 @@ const PrepareBridgePage = () => { removeTokenFromUrl(); break; } - }, [fromChain, fromToken, fromTokens, search]); + }, [fromChain, fromToken, fromTokens, search, isFromTokensLoading]); return ( @@ -404,16 +408,12 @@ const PrepareBridgePage = () => { ), ); } - dispatch(setFromChain(networkConfig.chainId)); dispatch(setFromToken(null)); dispatch(setFromTokenInputValue(null)); }, header: t('yourNetworks'), }} isMultiselectEnabled - customTokenListGenerator={ - fromTokens && fromTopAssets ? fromTokenListGenerator : undefined - } onMaxButtonClick={(value: string) => { dispatch(setFromTokenInputValue(value)); }} @@ -488,7 +488,6 @@ const PrepareBridgePage = () => { .networkClientId : undefined; toChainClientId && dispatch(setActiveNetwork(toChainClientId)); - toChain && dispatch(setFromChain(toChain.chainId)); dispatch(setFromToken(toToken)); dispatch(setFromTokenInputValue(null)); fromChain?.chainId && dispatch(setToChain(fromChain.chainId));