diff --git a/packages/bridge-controller/CHANGELOG.md b/packages/bridge-controller/CHANGELOG.md index 7bce89cec14..299c19086eb 100644 --- a/packages/bridge-controller/CHANGELOG.md +++ b/packages/bridge-controller/CHANGELOG.md @@ -7,9 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Sentry traces for `BridgeQuotesFetched` and `SwapQuotesFetched` events ([#5780](https://github.com/MetaMask/core/pull/5780)) +- Export `isCrossChain` utility ([#5780](https://github.com/MetaMask/core/pull/5780)) + ### Changed +- **BREAKING:** Remove `BridgeToken` export ([#5768](https://github.com/MetaMask/core/pull/5768)) - **BREAKING** Rename `QuoteResponse.bridgePriceData` to `priceData` ([#5784](https://github.com/MetaMask/core/pull/5784)) +- `traceFn` added to BridgeController constructor to enable clients to pass in a custom sentry trace handler ([#5768](https://github.com/MetaMask/core/pull/5768)) ## [22.0.0] diff --git a/packages/bridge-controller/src/bridge-controller.ts b/packages/bridge-controller/src/bridge-controller.ts index f38c030bce1..c3caf0f1c5a 100644 --- a/packages/bridge-controller/src/bridge-controller.ts +++ b/packages/bridge-controller/src/bridge-controller.ts @@ -2,7 +2,7 @@ import type { BigNumber } from '@ethersproject/bignumber'; import { Contract } from '@ethersproject/contracts'; import { Web3Provider } from '@ethersproject/providers'; import type { StateMetadata } from '@metamask/base-controller'; -import type { ChainId } from '@metamask/controller-utils'; +import type { ChainId, TraceCallback } from '@metamask/controller-utils'; import { SolScope } from '@metamask/keyring-api'; import { abiERC20 } from '@metamask/metamask-eth-abis'; import type { NetworkClientId } from '@metamask/network-controller'; @@ -20,6 +20,7 @@ import { REFRESH_INTERVAL_MS, } from './constants/bridge'; import { CHAIN_IDS } from './constants/chains'; +import { TraceName } from './constants/traces'; import { selectIsAssetExchangeRateInState } from './selectors'; import type { QuoteRequest } from './types'; import { @@ -37,6 +38,7 @@ import { getAssetIdsForToken, toExchangeRates } from './utils/assets'; import { hasSufficientBalance } from './utils/balance'; import { getDefaultBridgeControllerState, + isCrossChain, isSolanaChainId, sumHexes, } from './utils/bridge'; @@ -151,6 +153,8 @@ export class BridgeController extends StaticIntervalPollingController, ) => void; + readonly #trace: TraceCallback; + readonly #config: { customBridgeApiBaseUrl?: string; }; @@ -163,6 +167,7 @@ export class BridgeController extends StaticIntervalPollingController; @@ -182,6 +187,7 @@ export class BridgeController extends StaticIntervalPollingController, ) => void; + traceFn?: TraceCallback; }) { super({ name: BRIDGE_CONTROLLER_NAME, @@ -201,6 +207,7 @@ export class BridgeController extends StaticIntervalPollingController fn?.()) as TraceCallback); // Register action handlers this.messagingSystem.registerActionHandler( @@ -453,7 +460,7 @@ export class BridgeController extends StaticIntervalPollingController { const quotes = await fetchBridgeQuotes( updatedQuoteRequest, // AbortController is always defined by this line, because we assign it a few lines above, @@ -473,6 +480,24 @@ export class BridgeController extends StaticIntervalPollingController { expect(decimalResult).toStrictEqual(stringifiedDecimalResult); }); }); + + describe('isCrossChain', () => { + it('should return false when there is no destChainId', () => { + const result = isCrossChain('0x1'); + expect(result).toBe(false); + }); + + it('should return false when srcChainId is invalid', () => { + const result = isCrossChain('a', '0x1'); + expect(result).toBe(false); + }); + + it('should return false when destChainId is invalid', () => { + const result = isCrossChain('0x1', 'a'); + expect(result).toBe(false); + }); + }); }); diff --git a/packages/bridge-controller/src/utils/bridge.ts b/packages/bridge-controller/src/utils/bridge.ts index aaeea071ac6..6a56de32035 100644 --- a/packages/bridge-controller/src/utils/bridge.ts +++ b/packages/bridge-controller/src/utils/bridge.ts @@ -21,7 +21,11 @@ import { SYMBOL_TO_SLIP44_MAP, type SupportedSwapsNativeCurrencySymbols, } from '../constants/tokens'; -import type { BridgeAsset, BridgeControllerState } from '../types'; +import type { + BridgeAsset, + BridgeControllerState, + GenericQuoteRequest, +} from '../types'; import { ChainId } from '../types'; export const getDefaultBridgeControllerState = (): BridgeControllerState => { @@ -175,3 +179,24 @@ export const isSolanaChainId = ( } return chainId.toString() === ChainId.SOLANA.toString(); }; + +/** + * Checks whether the transaction is a cross-chain transaction by comparing the source and destination chainIds + * + * @param srcChainId - The source chainId + * @param destChainId - The destination chainId + * @returns Whether the transaction is a cross-chain transaction + */ +export const isCrossChain = ( + srcChainId: GenericQuoteRequest['srcChainId'], + destChainId?: GenericQuoteRequest['destChainId'], +) => { + try { + if (!destChainId) { + return false; + } + return formatChainIdToCaip(srcChainId) !== formatChainIdToCaip(destChainId); + } catch { + return false; + } +}; diff --git a/packages/bridge-controller/src/utils/metrics/properties.ts b/packages/bridge-controller/src/utils/metrics/properties.ts index be78b5d293c..84e0ada1dd9 100644 --- a/packages/bridge-controller/src/utils/metrics/properties.ts +++ b/packages/bridge-controller/src/utils/metrics/properties.ts @@ -6,7 +6,7 @@ import type { AccountsControllerState } from '../../../../accounts-controller/sr import { DEFAULT_BRIDGE_CONTROLLER_STATE } from '../../constants/bridge'; import type { BridgeControllerState, QuoteResponse, TxData } from '../../types'; import { type GenericQuoteRequest, type QuoteRequest } from '../../types'; -import { getNativeAssetForChainId } from '../bridge'; +import { getNativeAssetForChainId, isCrossChain } from '../bridge'; import { formatAddressToAssetId, formatChainIdToCaip, @@ -49,11 +49,7 @@ export const getActionType = ( srcChainId?: GenericQuoteRequest['srcChainId'], destChainId?: GenericQuoteRequest['destChainId'], ) => { - if ( - srcChainId && - formatChainIdToCaip(srcChainId) === - formatChainIdToCaip(destChainId ?? srcChainId) - ) { + if (srcChainId && !isCrossChain(srcChainId, destChainId ?? srcChainId)) { return MetricsActionType.SWAPBRIDGE_V1; } return MetricsActionType.CROSSCHAIN_V1; @@ -69,11 +65,7 @@ export const getSwapType = ( srcChainId?: GenericQuoteRequest['srcChainId'], destChainId?: GenericQuoteRequest['destChainId'], ) => { - if ( - srcChainId && - formatChainIdToCaip(srcChainId) === - formatChainIdToCaip(destChainId ?? srcChainId) - ) { + if (srcChainId && !isCrossChain(srcChainId, destChainId ?? srcChainId)) { return MetricsSwapType.SINGLE; } return MetricsSwapType.CROSSCHAIN; diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index 53cfc79a02b..d645a10104c 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -7,8 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Sentry traces for Swap and Bridge `TransactionApprovalCompleted` and `TransactionCompleted` events ([#5780](https://github.com/MetaMask/core/pull/5780)) + ### Changed +- `traceFn` added to BridgeStatusController constructor to enable clients to pass in a custom sentry trace handler ([#5768](https://github.com/MetaMask/core/pull/5768)) - Replace `bridgePriceData` with `priceData` from QuoteResponse object ([#5784](https://github.com/MetaMask/core/pull/5784)) ## [19.0.0] diff --git a/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap b/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap index ee6919ee327..df881b505fc 100644 --- a/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap +++ b/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap @@ -532,6 +532,31 @@ Array [ ] `; +exports[`BridgeStatusController submitTx: EVM should delay after submitting linea approval 4`] = ` +Array [ + Array [ + Object { + "data": Object { + "srcChainId": "eip155:59144", + "stxEnabled": false, + }, + "name": "Bridge Transaction Approval Completed", + }, + [Function], + ], + Array [ + Object { + "data": Object { + "srcChainId": "eip155:59144", + "stxEnabled": false, + }, + "name": "Bridge Transaction Completed", + }, + [Function], + ], +] +`; + exports[`BridgeStatusController submitTx: EVM should handle smart accounts (4337) 1`] = ` Object { "chainId": "0xa4b1", diff --git a/packages/bridge-status-controller/src/bridge-status-controller.test.ts b/packages/bridge-status-controller/src/bridge-status-controller.test.ts index 2723795ebec..a93f593c5a0 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.test.ts @@ -523,7 +523,7 @@ const addTransactionFn = jest.fn(); const estimateGasFeeFn = jest.fn(); const addUserOperationFromTransactionFn = jest.fn(); -const getController = (call: jest.Mock) => { +const getController = (call: jest.Mock, traceFn?: jest.Mock) => { const controller = new BridgeStatusController({ messenger: { call, @@ -536,6 +536,7 @@ const getController = (call: jest.Mock) => { addTransactionFn, estimateGasFeeFn, addUserOperationFromTransactionFn, + traceFn, }); jest.spyOn(controller, 'startPolling').mockImplementation(jest.fn()); @@ -1837,12 +1838,17 @@ describe('BridgeStatusController', () => { const handleLineaDelaySpy = jest .spyOn(transactionUtils, 'handleLineaDelay') .mockResolvedValueOnce(); + const mockTraceFn = jest + .fn() + .mockImplementation((_p, callback) => callback()); setupApprovalMocks(); setupBridgeMocks(); - const { controller, startPollingForBridgeTxStatusSpy } = - getController(mockMessengerCall); + const { controller, startPollingForBridgeTxStatusSpy } = getController( + mockMessengerCall, + mockTraceFn, + ); const lineaQuoteResponse = { ...mockEvmQuoteResponse, @@ -1853,6 +1859,7 @@ describe('BridgeStatusController', () => { const result = await controller.submitTx(lineaQuoteResponse, false); controller.stopAllPolling(); + expect(mockTraceFn).toHaveBeenCalledTimes(2); expect(handleLineaDelaySpy).toHaveBeenCalledTimes(1); expect(result).toMatchSnapshot(); expect(startPollingForBridgeTxStatusSpy).toHaveBeenCalledTimes(1); @@ -1866,6 +1873,7 @@ describe('BridgeStatusController', () => { 1234567890, ); expect(mockMessengerCall.mock.calls).toMatchSnapshot(); + expect(mockTraceFn.mock.calls).toMatchSnapshot(); }); }); }); diff --git a/packages/bridge-status-controller/src/bridge-status-controller.ts b/packages/bridge-status-controller/src/bridge-status-controller.ts index cf8ccc6a80b..28586cf788d 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.ts @@ -13,7 +13,10 @@ import { StatusTypes, UnifiedSwapBridgeEventName, getActionType, + formatChainIdToCaip, + isCrossChain, } from '@metamask/bridge-controller'; +import type { TraceCallback } from '@metamask/controller-utils'; import { toHex } from '@metamask/controller-utils'; import { EthAccountType } from '@metamask/keyring-api'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; @@ -35,6 +38,7 @@ import { BRIDGE_STATUS_CONTROLLER_NAME, DEFAULT_BRIDGE_STATUS_CONTROLLER_STATE, REFRESH_INTERVAL_MS, + TraceName, } from './constants'; import { type BridgeStatusControllerMessenger } from './types'; import type { @@ -103,6 +107,8 @@ export class BridgeStatusController extends StaticIntervalPollingController; @@ -123,6 +130,7 @@ export class BridgeStatusController extends StaticIntervalPollingController fn?.()) as TraceCallback); // Register action handlers this.messagingSystem.registerActionHandler( @@ -528,24 +537,44 @@ export class BridgeStatusController extends StaticIntervalPollingController & QuoteMetadata, ): Promise => { - if (quoteResponse.approval) { - await this.#handleUSDTAllowanceReset(quoteResponse); - const approvalTxMeta = await this.#handleEvmTransaction( - TransactionType.bridgeApproval, - quoteResponse.approval, - quoteResponse, - ); - if (!approvalTxMeta) { - throw new Error( - 'Failed to submit bridge tx: approval txMeta is undefined', + const { approval } = quoteResponse; + + if (approval) { + const approveTx = async () => { + await this.#handleUSDTAllowanceReset(quoteResponse); + + const approvalTxMeta = await this.#handleEvmTransaction( + TransactionType.bridgeApproval, + approval, + quoteResponse, ); - } + if (!approvalTxMeta) { + throw new Error( + 'Failed to submit bridge tx: approval txMeta is undefined', + ); + } + + await handleLineaDelay(quoteResponse); + return approvalTxMeta; + }; - await handleLineaDelay(quoteResponse); - return approvalTxMeta; + return await this.#trace( + { + name: isBridgeTx + ? TraceName.BridgeTransactionApprovalCompleted + : TraceName.SwapTransactionApprovalCompleted, + data: { + srcChainId: formatChainIdToCaip(quoteResponse.quote.srcChainId), + stxEnabled: false, + }, + }, + approveTx, + ); } + return undefined; }; @@ -731,13 +760,31 @@ export class BridgeStatusController extends StaticIntervalPollingController { let txMeta: (TransactionMeta & Partial) | undefined; + + const isBridgeTx = isCrossChain( + quoteResponse.quote.srcChainId, + quoteResponse.quote.destChainId, + ); + // Submit SOLANA tx if ( isSolanaChainId(quoteResponse.quote.srcChainId) && typeof quoteResponse.trade === 'string' ) { - txMeta = await this.#handleSolanaTx( - quoteResponse as QuoteResponse & QuoteMetadata, + txMeta = await this.#trace( + { + name: isBridgeTx + ? TraceName.BridgeTransactionCompleted + : TraceName.SwapTransactionCompleted, + data: { + srcChainId: formatChainIdToCaip(quoteResponse.quote.srcChainId), + stxEnabled: false, + }, + }, + async () => + await this.#handleSolanaTx( + quoteResponse as QuoteResponse & QuoteMetadata, + ), ); this.#trackUnifiedSwapBridgeEvent( UnifiedSwapBridgeEventName.SnapConfirmationViewed, @@ -751,22 +798,49 @@ export class BridgeStatusController extends StaticIntervalPollingController + await this.#handleEvmSmartTransaction( + quoteResponse.trade as TxData, + quoteResponse, + approvalTxId, + ), ); } else { - txMeta = await this.#handleEvmTransaction( - TransactionType.bridge, - quoteResponse.trade, - quoteResponse, - approvalTxId, + txMeta = await this.#trace( + { + name: isBridgeTx + ? TraceName.BridgeTransactionCompleted + : TraceName.SwapTransactionCompleted, + data: { + srcChainId: formatChainIdToCaip(quoteResponse.quote.srcChainId), + stxEnabled: false, + }, + }, + async () => + await this.#handleEvmTransaction( + TransactionType.bridge, + quoteResponse.trade as TxData, + quoteResponse, + approvalTxId, + ), ); } } diff --git a/packages/bridge-status-controller/src/constants.ts b/packages/bridge-status-controller/src/constants.ts index b3a09375045..564363d3729 100644 --- a/packages/bridge-status-controller/src/constants.ts +++ b/packages/bridge-status-controller/src/constants.ts @@ -12,3 +12,10 @@ export const DEFAULT_BRIDGE_STATUS_CONTROLLER_STATE: BridgeStatusControllerState export const BRIDGE_PROD_API_BASE_URL = 'https://bridge.api.cx.metamask.io'; export const LINEA_DELAY_MS = 5000; + +export enum TraceName { + BridgeTransactionApprovalCompleted = 'Bridge Transaction Approval Completed', + BridgeTransactionCompleted = 'Bridge Transaction Completed', + SwapTransactionApprovalCompleted = 'Swap Transaction Approval Completed', + SwapTransactionCompleted = 'Swap Transaction Completed', +}