Skip to content

Commit

Permalink
refactor: bridge support for CAIP chainIds and non-hex addresses (#30305
Browse files Browse the repository at this point in the history
)

<!--
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**
**Problem**: Multichain controllers identify chains and addresses in the
CAIP format, but Bridge components assume that everything is either
`Hex` and case-insensitive. This will cause unexpected behavior when
non-evm data is integrated into the experience, such as missing data due
to bad lookups, invalid bridge-api requests, page crashes due to hex to
decimal conversions

**Solution**: Decouple the quote parameters used within components from
the QuoteRequest object sent to the bridge-api. This enables
transforming inputs into any format required by the backend while
allowing any data to be passed around in the frontend

**Changes**
- chainIds and addresses are used as-is and not formatted until they are
needed for metrics or bridge-api requests
- bridge feature flags for chain config are identified by CAIP chain
addresses
- `GenericQuoteRequest` type covers all possible parameter types. this
is converted to `QuoteRequest` in fetchBridgeQuotes
- caip-formatters.ts formats ids and addresses
- chainId in metrics will be in caip format instead of hex

<!--
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/30305?quickstart=1)

## **Related issues**

Fixes: MMS-1867

## **Manual testing steps**

This should not affect the EVM bridging flow. Testing in Flask will be
possible when the asset-picker is updated to include non-evm tokens:
#30313

## **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**

- [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).
- [ ] 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.
  • Loading branch information
micaelae authored Feb 24, 2025
1 parent f4c3889 commit a8804d3
Show file tree
Hide file tree
Showing 38 changed files with 692 additions and 352 deletions.
36 changes: 18 additions & 18 deletions app/scripts/controllers/bridge/bridge-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import nock from 'nock';
import { BigNumber } from 'bignumber.js';
import { add0x } from '@metamask/utils';
import { BRIDGE_API_BASE_URL } from '../../../../shared/constants/bridge';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { SWAPS_API_V2_BASE_URL } from '../../../../shared/constants/swaps';
import { flushPromises } from '../../../../test/lib/timer-helpers';
import * as bridgeUtil from '../../../../shared/modules/bridge-utils/bridge.util';
Expand All @@ -12,6 +11,7 @@ import mockBridgeQuotesNativeErc20 from '../../../../test/data/bridge/mock-quote
import mockBridgeQuotesNativeErc20Eth from '../../../../test/data/bridge/mock-quotes-native-erc20-eth.json';
import { type QuoteResponse } from '../../../../shared/types/bridge';
import { decimalToHex } from '../../../../shared/modules/conversion.utils';
import { MultichainNetworks } from '../../../../shared/constants/multichain/networks';
import BridgeController from './bridge-controller';
import { BridgeControllerMessenger } from './types';
import { DEFAULT_BRIDGE_STATE } from './constants';
Expand Down Expand Up @@ -132,10 +132,10 @@ describe('BridgeController', function () {
refreshRate: 3,
support: true,
chains: {
[CHAIN_IDS.OPTIMISM]: { isActiveSrc: true, isActiveDest: false },
[CHAIN_IDS.SCROLL]: { isActiveSrc: true, isActiveDest: false },
[CHAIN_IDS.POLYGON]: { isActiveSrc: false, isActiveDest: true },
[CHAIN_IDS.ARBITRUM]: { isActiveSrc: false, isActiveDest: true },
'eip155:10': { isActiveSrc: true, isActiveDest: false },
'eip155:534352': { isActiveSrc: true, isActiveDest: false },
'eip155:137': { isActiveSrc: false, isActiveDest: true },
'eip155:42161': { isActiveSrc: false, isActiveDest: true },
},
},
};
Expand Down Expand Up @@ -272,22 +272,22 @@ describe('BridgeController', function () {
});

const quoteParams = {
srcChainId: 1,
destChainId: 10,
srcChainId: '0x1',
destChainId: MultichainNetworks.SOLANA,
srcTokenAddress: '0x0000000000000000000000000000000000000000',
destTokenAddress: '0x123',
srcTokenAmount: '1000000000000000000',
walletAddress: '0x123',
};
const quoteRequest = {
...quoteParams,
slippage: 0.5,
walletAddress: '0x123',
};
await bridgeController.updateBridgeQuoteRequestParams(quoteParams);

expect(stopAllPollingSpy).toHaveBeenCalledTimes(1);
expect(startPollingSpy).toHaveBeenCalledTimes(1);
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
expect(startPollingSpy).toHaveBeenCalledTimes(1);
expect(startPollingSpy).toHaveBeenCalledWith({
networkClientId: expect.anything(),
updatedQuoteRequest: {
Expand All @@ -298,7 +298,7 @@ describe('BridgeController', function () {

expect(bridgeController.state.bridgeState).toStrictEqual(
expect.objectContaining({
quoteRequest: { ...quoteRequest, walletAddress: undefined },
quoteRequest,
quotes: DEFAULT_BRIDGE_STATE.quotes,
quotesLastFetched: DEFAULT_BRIDGE_STATE.quotesLastFetched,
quotesLoadingStatus: DEFAULT_BRIDGE_STATE.quotesLoadingStatus,
Expand Down Expand Up @@ -421,16 +421,16 @@ describe('BridgeController', function () {
});

const quoteParams = {
srcChainId: 1,
destChainId: 10,
srcChainId: '0x1',
destChainId: '0x10',
srcTokenAddress: '0x0000000000000000000000000000000000000000',
destTokenAddress: '0x123',
srcTokenAmount: '1000000000000000000',
walletAddress: '0x123',
};
const quoteRequest = {
...quoteParams,
slippage: 0.5,
walletAddress: '0x123',
};
await bridgeController.updateBridgeQuoteRequestParams(quoteParams);

Expand All @@ -447,7 +447,7 @@ describe('BridgeController', function () {

expect(bridgeController.state.bridgeState).toStrictEqual(
expect.objectContaining({
quoteRequest: { ...quoteRequest, walletAddress: undefined },
quoteRequest,
quotes: DEFAULT_BRIDGE_STATE.quotes,
quotesLastFetched: DEFAULT_BRIDGE_STATE.quotesLastFetched,
quotesInitialLoadTime: undefined,
Expand Down Expand Up @@ -614,16 +614,16 @@ describe('BridgeController', function () {
});

const quoteParams = {
srcChainId: 10,
destChainId: 1,
srcChainId: '0x10',
destChainId: '0x1',
srcTokenAddress: '0x4200000000000000000000000000000000000006',
destTokenAddress: '0x0000000000000000000000000000000000000000',
srcTokenAmount: '991250000000000000',
walletAddress: 'eip:id/id:id/0x123',
};
const quoteRequest = {
...quoteParams,
slippage: 0.5,
walletAddress: '0x123',
};
await bridgeController.updateBridgeQuoteRequestParams(quoteParams);

Expand All @@ -640,7 +640,7 @@ describe('BridgeController', function () {

expect(bridgeController.state.bridgeState).toStrictEqual(
expect.objectContaining({
quoteRequest: { ...quoteRequest, walletAddress: undefined },
quoteRequest,
quotes: DEFAULT_BRIDGE_STATE.quotes,
quotesLastFetched: DEFAULT_BRIDGE_STATE.quotesLastFetched,
quotesLoadingStatus: DEFAULT_BRIDGE_STATE.quotesLoadingStatus,
Expand Down
49 changes: 30 additions & 19 deletions app/scripts/controllers/bridge/bridge-controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { add0x, Hex } from '@metamask/utils';
import { add0x, type Hex } from '@metamask/utils';
import { StaticIntervalPollingController } from '@metamask/polling-controller';
import { NetworkClientId } from '@metamask/network-controller';
import { StateMetadata } from '@metamask/base-controller';
Expand All @@ -18,17 +18,22 @@ import {
} from '../../../../shared/modules/conversion.utils';
import {
type L1GasFees,
type QuoteRequest,
type QuoteResponse,
type TxData,
type BridgeControllerState,
BridgeFeatureFlagsKey,
RequestStatus,
type GenericQuoteRequest,
} from '../../../../shared/types/bridge';
import { isValidQuoteRequest } from '../../../../shared/modules/bridge-utils/quote';
import { hasSufficientBalance } from '../../../../shared/modules/bridge-utils/balance';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { REFRESH_INTERVAL_MS } from '../../../../shared/constants/bridge';
import {
formatAddressToString,
formatChainIdToHex,
} from '../../../../shared/modules/bridge-utils/caip-formatters';
import { MultichainNetworks } from '../../../../shared/constants/multichain/networks';
import {
BRIDGE_CONTROLLER_NAME,
DEFAULT_BRIDGE_STATE,
Expand All @@ -48,7 +53,7 @@ const RESET_STATE_ABORT_MESSAGE = 'Reset controller state';
/** The input to start polling for the {@link BridgeController} */
type BridgePollingInput = {
networkClientId: NetworkClientId;
updatedQuoteRequest: QuoteRequest;
updatedQuoteRequest: GenericQuoteRequest;
};

export default class BridgeController extends StaticIntervalPollingController<BridgePollingInput>()<
Expand Down Expand Up @@ -113,7 +118,7 @@ export default class BridgeController extends StaticIntervalPollingController<Br
};

updateBridgeQuoteRequestParams = async (
paramsToUpdate: Partial<QuoteRequest>,
paramsToUpdate: Partial<GenericQuoteRequest>,
) => {
this.stopAllPolling();
this.#abortController?.abort('Quote request updated');
Expand All @@ -139,38 +144,45 @@ export default class BridgeController extends StaticIntervalPollingController<Br

if (isValidQuoteRequest(updatedQuoteRequest)) {
this.#quotesFirstFetched = Date.now();
const walletAddress = this.#getSelectedAccount().address;
const srcChainIdInHex = add0x(
decimalToHex(updatedQuoteRequest.srcChainId),
);

const insufficientBal =
paramsToUpdate.insufficientBal ||
!(await this.#hasSufficientBalance(updatedQuoteRequest));
const srcChainIdString = updatedQuoteRequest.srcChainId.toString();

// Query the balance of the source token if the source chain is an EVM chain
let insufficientBal: boolean | undefined;
if (srcChainIdString === MultichainNetworks.SOLANA) {
insufficientBal = paramsToUpdate.insufficientBal;
} else {
insufficientBal =
paramsToUpdate.insufficientBal ||
!(await this.#hasSufficientBalance(updatedQuoteRequest));
}

const networkClientId = this.#getSelectedNetworkClientId(srcChainIdInHex);
this.startPolling({
networkClientId,
networkClientId: srcChainIdString,
updatedQuoteRequest: {
...updatedQuoteRequest,
walletAddress,
insufficientBal,
},
});
}
};

#hasSufficientBalance = async (quoteRequest: QuoteRequest) => {
#hasSufficientBalance = async (quoteRequest: GenericQuoteRequest) => {
const walletAddress = this.#getSelectedAccount().address;
const srcChainIdInHex = add0x(decimalToHex(quoteRequest.srcChainId));
const srcChainIdInHex = formatChainIdToHex(quoteRequest.srcChainId);
const provider = this.#getSelectedNetworkClient()?.provider;
const srcTokenAddressWithoutPrefix = formatAddressToString(
quoteRequest.srcTokenAddress,
);

return (
provider &&
srcTokenAddressWithoutPrefix &&
quoteRequest.srcTokenAmount &&
srcChainIdInHex &&
(await hasSufficientBalance(
provider,
walletAddress,
quoteRequest.srcTokenAddress,
srcTokenAddressWithoutPrefix,
quoteRequest.srcTokenAmount,
srcChainIdInHex,
))
Expand Down Expand Up @@ -207,7 +219,6 @@ export default class BridgeController extends StaticIntervalPollingController<Br
}: BridgePollingInput) => {
this.#abortController?.abort('New quote request');
this.#abortController = new AbortController();

const { bridgeState } = this.state;
this.update((_state) => {
_state.bridgeState = {
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/controllers/bridge/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const DEFAULT_BRIDGE_STATE: BridgeState = {
},
quoteRequest: {
walletAddress: undefined,
srcTokenAddress: zeroAddress(),
srcTokenAddress: zeroAddress() as `0x${string}`,
slippage: BRIDGE_DEFAULT_SLIPPAGE,
},
quotesInitialLoadTime: undefined,
Expand Down
3 changes: 2 additions & 1 deletion app/scripts/lib/bridge-status/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '../../../../shared/types/bridge-status';
import { isEthUsdt } from '../../../../shared/modules/bridge-utils/bridge.util';
import { getCommonProperties } from '../../../../shared/lib/bridge-status/metrics';
import { formatChainIdToHex } from '../../../../shared/modules/bridge-utils/caip-formatters';
import { getTokenUsdValue } from './metrics-utils';

type TrackEvent = (
Expand Down Expand Up @@ -62,7 +63,7 @@ export const handleBridgeTransactionComplete = async (
).toNumber();
const destTokenUsdValue =
(await getTokenUsdValue({
chainId: chain_id_destination,
chainId: formatChainIdToHex(chain_id_destination),
tokenAmount: destTokenAmount,
tokenAddress: quote.destAsset.address,
state,
Expand Down
17 changes: 14 additions & 3 deletions shared/constants/swaps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
///: BEGIN:ONLY_INCLUDE_IF(solana-swaps)
import { MultichainNetworks } from './multichain/networks';
///: END:ONLY_INCLUDE_IF
import { MULTICHAIN_NATIVE_CURRENCY_TO_CAIP19 } from './multichain/assets';
import {
MULTICHAIN_TOKEN_IMAGE_MAP,
MultichainNetworks,
} from './multichain/networks';
import {
ETH_TOKEN_IMAGE_URL,
TEST_ETH_TOKEN_IMAGE_URL,
Expand Down Expand Up @@ -129,6 +131,14 @@ export const BASE_SWAPS_TOKEN_OBJECT: SwapsTokenObject = {
...ETH_SWAPS_TOKEN_OBJECT,
} as const;

const SOLANA_SWAPS_TOKEN_OBJECT: SwapsTokenObject = {
symbol: 'SOL',
name: 'Solana',
address: MULTICHAIN_NATIVE_CURRENCY_TO_CAIP19.SOL,
decimals: 9,
iconUrl: MULTICHAIN_TOKEN_IMAGE_MAP[MultichainNetworks.SOLANA],
};

// A gas value for ERC20 approve calls that should be sufficient for all ERC20 approve implementations
export const DEFAULT_ERC20_APPROVE_GAS = '0x1d4c0';

Expand Down Expand Up @@ -288,6 +298,7 @@ export const SWAPS_CHAINID_DEFAULT_TOKEN_MAP = {
[CHAIN_IDS.ZKSYNC_ERA]: ZKSYNC_ERA_SWAPS_TOKEN_OBJECT,
[CHAIN_IDS.LINEA_MAINNET]: LINEA_SWAPS_TOKEN_OBJECT,
[CHAIN_IDS.BASE]: BASE_SWAPS_TOKEN_OBJECT,
[MultichainNetworks.SOLANA]: SOLANA_SWAPS_TOKEN_OBJECT,
} as const;

export const ETHEREUM = 'ethereum';
Expand Down
6 changes: 3 additions & 3 deletions shared/lib/bridge-status/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { ActionType } from '../../../ui/hooks/bridge/events/types';
import { formatProviderLabel } from '../../../ui/pages/bridge/utils/quote';
import { getCurrentKeyring } from '../../../ui/selectors';
import { BRIDGE_DEFAULT_SLIPPAGE } from '../../constants/bridge';
import { decimalToPrefixedHex } from '../../modules/conversion.utils';
import { getIsSmartTransaction } from '../../modules/selectors';
import { formatChainIdToCaip } from '../../modules/bridge-utils/caip-formatters';

export const getCommonProperties = (
bridgeHistoryItem: BridgeHistoryItem,
Expand All @@ -20,10 +20,10 @@ export const getCommonProperties = (
// @ts-expect-error keyring type is possibly wrong
const is_hardware_wallet = isHardwareKeyring(keyring.type) ?? false;

const chain_id_source = decimalToPrefixedHex(
const chain_id_source = formatChainIdToCaip(
bridgeHistoryItem.quote.srcChainId,
);
const chain_id_destination = decimalToPrefixedHex(
const chain_id_destination = formatChainIdToCaip(
bridgeHistoryItem.quote.destChainId,
);

Expand Down
2 changes: 1 addition & 1 deletion shared/modules/bridge-utils/balance.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Web3Provider } from '@ethersproject/providers';
import type { Provider } from '@metamask/network-controller';
import { Hex } from '@metamask/utils';
import type { Hex } from '@metamask/utils';
import { zeroAddress } from 'ethereumjs-util';
import { getAddress } from 'ethers/lib/utils';
import { fetchTokenBalance } from '../../lib/token-util';
Expand Down
22 changes: 15 additions & 7 deletions shared/modules/bridge-utils/bridge.util.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { zeroAddress } from 'ethereumjs-util';
import fetchWithCache from '../../lib/fetch-with-cache';
import { CHAIN_IDS } from '../../constants/network';
import mockBridgeQuotesErc20Erc20 from '../../../test/data/bridge/mock-quotes-erc20-erc20.json';
import mockBridgeQuotesNativeErc20 from '../../../test/data/bridge/mock-quotes-native-erc20.json';
import { ChainId } from '../../types/bridge';
import {
fetchBridgeFeatureFlags,
fetchBridgeQuotes,
Expand Down Expand Up @@ -48,6 +48,10 @@ describe('Bridge utils', () => {
isActiveSrc: false,
isActiveDest: true,
},
[ChainId.SOLANA]: {
isActiveSrc: false,
isActiveDest: true,
},
},
},
};
Expand All @@ -72,27 +76,31 @@ describe('Bridge utils', () => {
refreshRate: 3,
support: true,
chains: {
[CHAIN_IDS.MAINNET]: {
'eip155:1': {
isActiveSrc: true,
isActiveDest: true,
},
[CHAIN_IDS.OPTIMISM]: {
'eip155:10': {
isActiveSrc: true,
isActiveDest: false,
},
[CHAIN_IDS.LINEA_MAINNET]: {
'eip155:59144': {
isActiveSrc: true,
isActiveDest: true,
},
'0x78': {
'eip155:120': {
isActiveSrc: true,
isActiveDest: false,
},
[CHAIN_IDS.POLYGON]: {
'eip155:11111': {
isActiveSrc: false,
isActiveDest: true,
},
'eip155:137': {
isActiveSrc: false,
isActiveDest: true,
},
'0x2b67': {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp': {
isActiveSrc: false,
isActiveDest: true,
},
Expand Down
Loading

0 comments on commit a8804d3

Please sign in to comment.