Skip to content

Commit 642d0ad

Browse files
authored
refactor: remove unused swap selectors and fix selector types (#38413)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This cleans up `bridge` redux state, selectors and actions to prepare for the AssetPicker refactor - remove destination exchange rate management since the data now comes from the bridge-controller - `toTokenExchangeRate` and `toTokenUsdExchangeRate` from state - update `useBridgeExchangeRates` to only fetch source exchange rates - remove unused selectors: `getAllBridgeableNetworks`, `getToTokenConversionRate`, `getIsBridgeTx` - tighten selector parameter/return types - replace `NetworkConfiguration` type references with `BridgeNetwork` (minimal network object required by swaps) for better type safety - use typed state overrides in unit test mock data builder (many unit test mock data was updated as a result) <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38413?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: refactor: remove unused swap selectors and fix types ## **Related issues** Fixes: N/A ## **Manual testing steps** No user-facing changes ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Cleans up bridge state and selectors by dropping destination exchange-rate handling, tightening types (incl. BridgeNetwork), simplifying actions/hooks, and updating tests/stories accordingly. > > - **State/Actions**: > - Remove `toTokenExchangeRate` and `toTokenUsdExchangeRate` from `bridge` state and related actions/thunks. > - Simplify `setFromChain` to accept `chainId` (Hex/CAIP) and derive network client id; auto-select native token for non‑EVM. > - Keep only source exchange-rate fetching via `setSrcTokenExchangeRates`. > - **Selectors**: > - Replace `getAllBridgeableNetworks` with record-based lookup from `multichainNetworkConfigurationsByChainId`. > - Remove unused selectors (`getToTokenConversionRate`, `getIsBridgeTx`). > - Tighten types and memoization; add `BridgeNetwork` type; refine balances and validation logic; expose price impact thresholds and no-fee assets via feature flags. > - **Hooks/Utils**: > - `useBridgeExchangeRates` now fetches only source token rates and uses cache. > - `useBridgeQueryParams` updated to work with new types and chain selection flow. > - Remove unused `useSolanaBridgeTransactionMapping` hook. > - Improve `isQuoteExpiredOrInvalid`, `isNetworkAdded`, and image/balance handling; add `safeAmountForCalc`. > - **UI**: > - Prepare page and quote card wired to new selector/state shape; switch‑tokens enablement uses updated network checks. > - **Tests/Stories**: > - Update mocks to typed `featureFlagOverrides: { bridgeConfig }` and `BridgeControllerState` fields; adjust snapshots and fixtures to new shapes. > - **Constants**: > - Mark `ALLOWED_BRIDGE_CHAIN_IDS` as `const`; add `SLIP44_ASSET_NAMESPACE`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 92155e9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 87eb2b8 commit 642d0ad

32 files changed

+368
-1635
lines changed

shared/constants/bridge.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const ALLOWED_BRIDGE_CHAIN_IDS = [
4141
///: BEGIN:ONLY_INCLUDE_IF(tron)
4242
MultichainNetworks.TRON,
4343
///: END:ONLY_INCLUDE_IF
44-
];
44+
] as const;
4545

4646
export const ALLOWED_BRIDGE_CHAIN_IDS_IN_CAIP =
4747
ALLOWED_EVM_BRIDGE_CHAIN_IDS.map(toEvmCaipChainId).concat(

shared/constants/multichain/assets.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,5 @@ export const TRON_RESOURCE_SYMBOLS = Object.values(
7979

8080
export const TRON_RESOURCE_SYMBOLS_SET: ReadonlySet<TronResourceSymbol> =
8181
new Set(TRON_RESOURCE_SYMBOLS);
82+
83+
export const SLIP44_ASSET_NAMESPACE = 'slip44';

test/data/bridge/mock-bridge-store.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
22
getDefaultBridgeControllerState,
33
formatChainIdToCaip,
4+
FeatureFlagResponse,
5+
BridgeControllerState,
46
} from '@metamask/bridge-controller';
57
import { DEFAULT_BRIDGE_STATUS_CONTROLLER_STATE } from '@metamask/bridge-status-controller';
68
import { AVAILABLE_MULTICHAIN_NETWORK_CONFIGURATIONS } from '@metamask/multichain-network-controller';
@@ -118,25 +120,21 @@ export const MOCK_EVM_ACCOUNT_2 = {
118120
};
119121

120122
export const createBridgeMockStore = ({
121-
featureFlagOverrides = {},
123+
featureFlagOverrides = { bridgeConfig: {} },
122124
bridgeSliceOverrides = {},
123125
bridgeStateOverrides = {},
124126
bridgeStatusStateOverrides = {},
125127
metamaskStateOverrides = {},
126128
stateOverrides = {},
127129
}: {
128-
// featureFlagOverrides?: Partial<BridgeControllerState['bridgeFeatureFlags']>;
129-
// bridgeStateOverrides?: Partial<BridgeControllerState>;
130+
featureFlagOverrides?: { bridgeConfig: Partial<FeatureFlagResponse> };
131+
bridgeStateOverrides?: Partial<BridgeControllerState>;
130132
// bridgeStatusStateOverrides?: Partial<BridgeStatusState>;
131133
// metamaskStateOverrides?: Partial<BridgeAppState['metamask']>;
132134
// TODO replace these with correct types
133135
// eslint-disable-next-line @typescript-eslint/no-explicit-any
134-
featureFlagOverrides?: Record<string, any>;
135-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
136136
bridgeSliceOverrides?: Record<string, any>;
137137
// eslint-disable-next-line @typescript-eslint/no-explicit-any
138-
bridgeStateOverrides?: Record<string, any>;
139-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
140138
bridgeStatusStateOverrides?: Record<string, any>;
141139
// eslint-disable-next-line @typescript-eslint/no-explicit-any
142140
metamaskStateOverrides?: Record<string, any>;
@@ -352,7 +350,6 @@ export const createBridgeMockStore = ({
352350
support: false,
353351
refreshRate: 5000,
354352
maxRefreshCount: 5,
355-
...featureFlagOverrides?.extensionConfig,
356353
...featureFlagOverrides?.bridgeConfig,
357354
chains: {
358355
[formatChainIdToCaip('0x1')]: {
@@ -361,9 +358,7 @@ export const createBridgeMockStore = ({
361358
},
362359
...Object.fromEntries(
363360
Object.entries(
364-
featureFlagOverrides?.extensionConfig?.chains ??
365-
featureFlagOverrides?.bridgeConfig?.chains ??
366-
{},
361+
featureFlagOverrides?.bridgeConfig?.chains ?? {},
367362
).map(([chainId, config]) => [
368363
formatChainIdToCaip(chainId),
369364
config,

test/e2e/tests/settings/state-logs.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,6 @@
110110
"sortOrder": "string",
111111
"toChainId": "null",
112112
"toToken": "null",
113-
"toTokenExchangeRate": "null",
114-
"toTokenUsdExchangeRate": "null",
115113
"txAlert": "null",
116114
"wasTxDeclined": "boolean"
117115
},

ui/ducks/bridge/actions.ts

Lines changed: 19 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,11 @@ import {
77
getNativeAssetForChainId,
88
type RequiredEventContextFromClient,
99
UnifiedSwapBridgeEventName,
10+
formatChainIdToHex,
1011
} from '@metamask/bridge-controller';
11-
import { type InternalAccount } from '@metamask/keyring-internal-api';
12-
import { type CaipChainId } from '@metamask/utils';
13-
import type {
14-
AddNetworkFields,
15-
NetworkConfiguration,
16-
} from '@metamask/network-controller';
12+
import type { CaipChainId, Hex } from '@metamask/utils';
1713
import { trace, TraceName } from '../../../shared/lib/trace';
14+
import { selectDefaultNetworkClientIdsByChainId } from '../../../shared/modules/selectors/networks';
1815
import {
1916
forceUpdateMetamaskState,
2017
setActiveNetworkWithError,
@@ -23,15 +20,14 @@ import { submitRequestToBackground } from '../../store/background-connection';
2320
import type { MetaMaskReduxDispatch } from '../../store/store';
2421
import {
2522
bridgeSlice,
26-
setDestTokenExchangeRates,
27-
setDestTokenUsdExchangeRates,
2823
setSrcTokenExchangeRates,
2924
setTxAlerts,
3025
setEVMSrcTokenBalance as setEVMSrcTokenBalance_,
3126
setEVMSrcNativeBalance,
3227
} from './bridge';
33-
import type { TokenPayload } from './types';
34-
import { isNetworkAdded, isNonEvmChain } from './utils';
28+
import { type TokenPayload } from './types';
29+
import { type BridgeAppState } from './selectors';
30+
import { isNonEvmChain } from './utils';
3531

3632
const {
3733
setToChainId,
@@ -52,8 +48,6 @@ export {
5248
setToToken,
5349
setFromToken,
5450
setFromTokenInputValue,
55-
setDestTokenExchangeRates,
56-
setDestTokenUsdExchangeRates,
5751
setSrcTokenExchangeRates,
5852
setSortOrder,
5953
setSelectedQuote,
@@ -145,33 +139,26 @@ export const setEVMSrcTokenBalance = (
145139
};
146140

147141
export const setFromChain = ({
148-
networkConfig,
149-
selectedAccount,
142+
chainId,
150143
token = null,
151144
}: {
152-
networkConfig?:
153-
| NetworkConfiguration
154-
| AddNetworkFields
155-
| (Omit<NetworkConfiguration, 'chainId'> & { chainId: CaipChainId });
156-
selectedAccount: InternalAccount | null;
145+
chainId: Hex | CaipChainId;
157146
token?: TokenPayload['payload'];
158147
}) => {
159-
return async (dispatch: MetaMaskReduxDispatch) => {
160-
if (!networkConfig) {
161-
return;
162-
}
163-
148+
return async (
149+
dispatch: MetaMaskReduxDispatch,
150+
getState: () => BridgeAppState,
151+
) => {
164152
// Check for ALL non-EVM chains
165-
const isNonEvm = isNonEvmChain(networkConfig.chainId);
153+
const isNonEvm = isNonEvmChain(chainId);
166154

167155
// Set the src network
168156
if (isNonEvm) {
169-
dispatch(setActiveNetworkWithError(networkConfig.chainId));
157+
dispatch(setActiveNetworkWithError(chainId));
170158
} else {
171-
const networkId = isNetworkAdded(networkConfig)
172-
? networkConfig.rpcEndpoints?.[networkConfig.defaultRpcEndpointIndex]
173-
?.networkClientId
174-
: null;
159+
const hexChainId = formatChainIdToHex(chainId);
160+
const networkId =
161+
selectDefaultNetworkClientIdsByChainId(getState())[hexChainId];
175162
if (networkId) {
176163
dispatch(setActiveNetworkWithError(networkId));
177164
}
@@ -182,33 +169,15 @@ export const setFromChain = ({
182169
dispatch(setFromToken(token));
183170
} else if (isNonEvm) {
184171
// Auto-select native token for non-EVM chains when switching
185-
const nativeAsset = getNativeAssetForChainId(networkConfig.chainId);
172+
const nativeAsset = getNativeAssetForChainId(chainId);
186173
if (nativeAsset) {
187174
dispatch(
188175
setFromToken({
189176
...nativeAsset,
190-
chainId: networkConfig.chainId,
177+
chainId,
191178
}),
192179
);
193180
}
194181
}
195-
196-
// Fetch the native balance (EVM only)
197-
if (selectedAccount && !isNonEvm) {
198-
trace({
199-
name: TraceName.BridgeBalancesUpdated,
200-
data: {
201-
srcChainId: formatChainIdToCaip(networkConfig.chainId),
202-
isNative: true,
203-
},
204-
startTime: Date.now(),
205-
});
206-
await dispatch(
207-
setEVMSrcNativeBalance({
208-
selectedAddress: selectedAccount.address,
209-
chainId: networkConfig.chainId,
210-
}),
211-
);
212-
}
213182
};
214183
};

ui/ducks/bridge/bridge.test.ts

Lines changed: 0 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,8 @@ import {
55
BridgeBackgroundAction,
66
BridgeUserAction,
77
formatChainIdToCaip,
8-
getNativeAssetForChainId,
98
} from '@metamask/bridge-controller';
10-
import * as controllerUtils from '@metamask/controller-utils';
119
import { createBridgeMockStore } from '../../../test/data/bridge/mock-bridge-store';
12-
import { toAssetId } from '../../../shared/lib/asset-utils';
1310
import { CHAIN_IDS } from '../../../shared/constants/network';
1411
import { setBackgroundConnection } from '../../store/background-connection';
1512
import { MultichainNetworks } from '../../../shared/constants/multichain/networks';
@@ -23,7 +20,6 @@ import {
2320
setToChainId,
2421
updateQuoteRequestParams,
2522
resetBridgeState,
26-
setDestTokenExchangeRates,
2723
setWasTxDeclined,
2824
setSlippage,
2925
} from './actions';
@@ -110,7 +106,6 @@ describe('Ducks - Bridge', () => {
110106
chainId: '0xa',
111107
image:
112108
'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/10/erc20/0x13341431.png',
113-
string: '0',
114109
}),
115110
);
116111
});
@@ -144,11 +139,9 @@ describe('Ducks - Bridge', () => {
144139
slippage: SlippageValue.BridgeDefault,
145140
fromTokenInputValue: null,
146141
sortOrder: 'cost_ascending',
147-
toTokenExchangeRate: null,
148142
fromTokenExchangeRate: null,
149143
wasTxDeclined: false,
150144
txAlert: null,
151-
toTokenUsdExchangeRate: null,
152145
fromTokenBalance: null,
153146
fromNativeBalance: null,
154147
});
@@ -245,119 +238,13 @@ describe('Ducks - Bridge', () => {
245238
toChainId: null,
246239
toToken: null,
247240
txAlert: null,
248-
toTokenExchangeRate: null,
249241
wasTxDeclined: false,
250-
toTokenUsdExchangeRate: null,
251242
fromTokenBalance: null,
252243
fromNativeBalance: null,
253244
});
254245
});
255246
});
256247

257-
describe('setDestTokenExchangeRates', () => {
258-
it('fetches token prices and updates dest exchange rates in state, native dest token', async () => {
259-
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31973
260-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
261-
const mockStore = configureMockStore<any>(middleware)(
262-
createBridgeMockStore(),
263-
);
264-
const state = mockStore.getState().bridge;
265-
const fetchTokenExchangeRatesSpy = jest
266-
.spyOn(controllerUtils, 'handleFetch')
267-
.mockResolvedValue({
268-
[getNativeAssetForChainId(CHAIN_IDS.LINEA_MAINNET).assetId]: {
269-
price: 0.356628,
270-
},
271-
});
272-
273-
await mockStore.dispatch(
274-
setDestTokenExchangeRates({
275-
chainId: CHAIN_IDS.LINEA_MAINNET,
276-
tokenAddress: zeroAddress(),
277-
currency: 'usd',
278-
}) as never,
279-
);
280-
281-
expect(fetchTokenExchangeRatesSpy).toHaveBeenCalledTimes(1);
282-
expect(fetchTokenExchangeRatesSpy).toHaveBeenCalledWith(
283-
'https://price.api.cx.metamask.io/v3/spot-prices?assetIds=eip155%3A59144%2Fslip44%3A60&includeMarketData=true&vsCurrency=usd',
284-
{
285-
headers: { 'X-Client-Id': 'extension' },
286-
method: 'GET',
287-
signal: undefined,
288-
},
289-
);
290-
291-
const actions = mockStore.getActions();
292-
expect(actions).toHaveLength(2);
293-
expect(actions[0].type).toStrictEqual(
294-
'bridge/setDestTokenExchangeRates/pending',
295-
);
296-
expect(actions[1].type).toStrictEqual(
297-
'bridge/setDestTokenExchangeRates/fulfilled',
298-
);
299-
const newState = bridgeReducer(state, actions[1]);
300-
expect(newState).toStrictEqual({
301-
toChainId: null,
302-
toTokenExchangeRate: 0.356628,
303-
sortOrder: 'cost_ascending',
304-
});
305-
});
306-
307-
it('fetches token prices and updates dest exchange rates in state, erc20 dest token', async () => {
308-
// TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31973
309-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
310-
const mockStore = configureMockStore<any>(middleware)(
311-
createBridgeMockStore(),
312-
);
313-
const state = mockStore.getState().bridge;
314-
const fetchTokenExchangeRatesSpy = jest
315-
.spyOn(controllerUtils, 'handleFetch')
316-
.mockResolvedValue({
317-
[toAssetId(
318-
'0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359'.toLowerCase(),
319-
formatChainIdToCaip(CHAIN_IDS.LINEA_MAINNET),
320-
) as never]: {
321-
price: 0.999881,
322-
},
323-
});
324-
325-
await mockStore.dispatch(
326-
setDestTokenExchangeRates({
327-
chainId: CHAIN_IDS.LINEA_MAINNET,
328-
tokenAddress:
329-
'0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359'.toLowerCase(),
330-
currency: 'usd',
331-
}) as never,
332-
);
333-
334-
expect(fetchTokenExchangeRatesSpy).toHaveBeenCalledTimes(1);
335-
expect(fetchTokenExchangeRatesSpy).toHaveBeenCalledWith(
336-
'https://price.api.cx.metamask.io/v3/spot-prices?assetIds=eip155%3A59144%2Ferc20%3A0x3c499c542cef5e3811e1192ce70d8cc03d5c3359&includeMarketData=true&vsCurrency=usd',
337-
{
338-
headers: { 'X-Client-Id': 'extension' },
339-
method: 'GET',
340-
signal: undefined,
341-
},
342-
);
343-
344-
const actions = mockStore.getActions();
345-
expect(actions).toHaveLength(2);
346-
expect(actions[0].type).toStrictEqual(
347-
'bridge/setDestTokenExchangeRates/pending',
348-
);
349-
expect(actions[1].type).toStrictEqual(
350-
'bridge/setDestTokenExchangeRates/fulfilled',
351-
);
352-
const newState = bridgeReducer(state, actions[1]);
353-
expect(newState).toStrictEqual({
354-
toChainId: null,
355-
toTokenExchangeRate: 0.999881,
356-
sortOrder: 'cost_ascending',
357-
});
358-
});
359-
});
360-
361248
describe('setWasTxDeclined', () => {
362249
it('sets the wasTxDeclined flag to true', () => {
363250
const state = store.getState().bridge;

0 commit comments

Comments
 (0)