diff --git a/packages/bridge-controller/CHANGELOG.md b/packages/bridge-controller/CHANGELOG.md index c71d9d3c227..2f5d85dd977 100644 --- a/packages/bridge-controller/CHANGELOG.md +++ b/packages/bridge-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add and export `getQuotesReceivedProperties` utility to build the metrics payload for clients ([#7182](https://github.com/MetaMask/core/pull/7182)) + ## [61.0.0] ### Changed diff --git a/packages/bridge-controller/src/__snapshots__/bridge-controller.test.ts.snap b/packages/bridge-controller/src/__snapshots__/bridge-controller.test.ts.snap index 1abf5ffb20e..06d48ac4790 100644 --- a/packages/bridge-controller/src/__snapshots__/bridge-controller.test.ts.snap +++ b/packages/bridge-controller/src/__snapshots__/bridge-controller.test.ts.snap @@ -403,7 +403,7 @@ Array [ "usd_quoted_gas": 0, "usd_quoted_return": 100, "warnings": Array [ - "warning1", + "insufficient_balance", ], }, ], @@ -494,7 +494,7 @@ Array [ "usd_quoted_gas": 0, "usd_quoted_return": 100, "warnings": Array [ - "warning1", + "low_return", ], }, ], diff --git a/packages/bridge-controller/src/bridge-controller.test.ts b/packages/bridge-controller/src/bridge-controller.test.ts index 03ce729f772..e5f20e0d06c 100644 --- a/packages/bridge-controller/src/bridge-controller.test.ts +++ b/packages/bridge-controller/src/bridge-controller.test.ts @@ -1050,7 +1050,7 @@ describe('BridgeController', function () { bridgeController.trackUnifiedSwapBridgeEvent( UnifiedSwapBridgeEventName.QuotesReceived, { - warnings: ['warning1'], + warnings: ['low_return'], usd_quoted_gas: 0, gas_included: false, gas_included_7702: false, @@ -2101,6 +2101,99 @@ describe('BridgeController', function () { expect(quotes[1].nonEvmFeesInNative).toBe('0.00005'); // BTC fee as-is }); + it('should catch BTC chain fees errors and return undefined fees', async () => { + jest.useFakeTimers(); + // Use the actual Solana mock which already has string trade type + const btcQuoteResponse = mockBridgeQuotesSolErc20.map((quote) => ({ + ...quote, + quote: { + ...quote.quote, + srcChainId: ChainId.BTC, + }, + })) as unknown as QuoteResponse[]; + + messengerMock.call.mockImplementation( + ( + ...args: Parameters + ): ReturnType => { + const [actionType] = args; + + if (actionType === 'AccountsController:getAccountByAddress') { + return { + type: 'btc:p2wpkh', + id: 'btc-account-1', + scopes: [BtcScope.Mainnet], + methods: [], + address: 'bc1q...', + metadata: { + name: 'BTC Account 1', + importTime: 1717334400, + keyring: { + type: 'Snap Keyring', + }, + snap: { + id: 'btc-snap-id', + name: 'BTC Snap', + }, + }, + } as never; + } + + if (actionType === 'SnapController:handleRequest') { + return new Promise((_resolve, reject) => { + reject(new Error('Failed to compute fees')); + }); + } + + return { + provider: jest.fn() as never, + } as never; + }, + ); + + jest.spyOn(fetchUtils, 'fetchBridgeQuotes').mockResolvedValue({ + quotes: btcQuoteResponse, + validationFailures: [], + }); + + const quoteParams = { + srcChainId: ChainId.BTC.toString(), + destChainId: '1', + srcTokenAddress: 'NATIVE', + destTokenAddress: '0x0000000000000000000000000000000000000000', + srcTokenAmount: '100000', // satoshis + walletAddress: 'bc1q...', + destWalletAddress: '0x5342', + slippage: 0.5, + }; + + await bridgeController.updateBridgeQuoteRequestParams( + quoteParams, + metricsContext, + ); + + // Wait for polling to start + jest.advanceTimersByTime(201); + await flushPromises(); + + // Wait for fetch to trigger + jest.advanceTimersByTime(295); + await flushPromises(); + + // Wait for fetch to complete + jest.advanceTimersByTime(2601); + await flushPromises(); + + // Final wait for fee calculation + jest.advanceTimersByTime(100); + await flushPromises(); + + const { quotes } = bridgeController.state; + expect(quotes).toHaveLength(2); // mockBridgeQuotesSolErc20 has 2 quotes + expect(quotes[0].nonEvmFeesInNative).toBeUndefined(); + expect(quotes[1].nonEvmFeesInNative).toBeUndefined(); + }); + describe('trackUnifiedSwapBridgeEvent client-side calls', () => { beforeEach(async () => { jest.clearAllMocks(); @@ -2267,7 +2360,7 @@ describe('BridgeController', function () { bridgeController.trackUnifiedSwapBridgeEvent( UnifiedSwapBridgeEventName.QuotesReceived, { - warnings: ['warning1'], + warnings: ['insufficient_balance'], usd_quoted_gas: 0, gas_included: false, gas_included_7702: false, @@ -2569,7 +2662,7 @@ describe('BridgeController', function () { bridgeController.trackUnifiedSwapBridgeEvent( UnifiedSwapBridgeEventName.QuotesReceived, { - warnings: ['warning1'], + warnings: ['low_return'], usd_quoted_gas: 0, gas_included: false, gas_included_7702: false, diff --git a/packages/bridge-controller/src/index.ts b/packages/bridge-controller/src/index.ts index a1e8bef2b9e..21a678b8940 100644 --- a/packages/bridge-controller/src/index.ts +++ b/packages/bridge-controller/src/index.ts @@ -13,6 +13,7 @@ export type { RequestMetadata, TxStatusData, QuoteFetchData, + QuoteWarning, } from './utils/metrics/types'; export { @@ -21,6 +22,7 @@ export { getSwapType, isHardwareWallet, isCustomSlippage, + getQuotesReceivedProperties, } from './utils/metrics/properties'; export type { diff --git a/packages/bridge-controller/src/selectors.test.ts b/packages/bridge-controller/src/selectors.test.ts index e46d2e843a8..bc82a628366 100644 --- a/packages/bridge-controller/src/selectors.test.ts +++ b/packages/bridge-controller/src/selectors.test.ts @@ -427,6 +427,7 @@ describe('Bridge Selectors', () => { amount: string; asset: Pick; }, + gasIncluded7702?: boolean, ): BridgeAppState => { const chainId = 56; const currencyRates = { @@ -446,6 +447,10 @@ describe('Bridge Selectors', () => { price: '1', currency: 'BNB', }, + '0x0000000000000000000000000000000000000001': { + price: '1.5498387253001357', + currency: 'BNB', + }, }, } as unknown as Record>; const srcTokenAmount = new BigNumber('10') // $10 worth of src token @@ -473,8 +478,8 @@ describe('Bridge Selectors', () => { }, txFee, }, - gasIncluded: Boolean(txFee), - gasIncluded7702: false, + gasIncluded: Boolean(txFee) && !gasIncluded7702, + gasIncluded7702: Boolean(gasIncluded7702), srcTokenAmount, destTokenAmount: new BigNumber('9') .dividedBy(marketData['0x38'][destAsset.address].price) @@ -875,6 +880,104 @@ describe('Bridge Selectors', () => { } `); }); + + it('when gasIncluded7702=true and is taken from dest token', () => { + const newState = getMockSwapState( + { + address: '0x8AC76a51cc950d9822D68b83fE1Ad97B32Cd580d', + decimals: 18, + assetId: + 'eip155:1/erc20:0x8AC76a51cc950d9822D68b83fE1Ad97B32Cd580d', + }, + { + address: '0x0000000000000000000000000000000000000001', + decimals: 18, + assetId: + 'eip155:1/erc20:0x0000000000000000000000000000000000000001', + }, + { + amount: '1000000000000000000', + asset: { + address: + 'eip155:1/erc20:0x0000000000000000000000000000000000000001', + decimals: 18, + assetId: + 'eip155:1/erc20:0x0000000000000000000000000000000000000001', + }, + }, + true, + ); + + const { sortedQuotes } = selectBridgeQuotes(newState, mockClientParams); + + const { + quote, + trade, + approval, + estimatedProcessingTimeInSeconds, + ...quoteMetadata + } = sortedQuotes[0]; + expect(quoteMetadata).toMatchInlineSnapshot(` + Object { + "adjustedReturn": Object { + "usd": "10.518641979781876096240273601395823616", + "valueInCurrency": "8.999999999999999949780980627632791914", + }, + "cost": Object { + "usd": "1.168737997753541853682403691190760358912", + "valueInCurrency": "1.000000000000000050216414294183215375298", + }, + "gasFee": Object { + "effective": Object { + "amount": "0.000008087", + "usd": "0.00521708544", + "valueInCurrency": "0.00446386226", + }, + "max": Object { + "amount": "0.000016174", + "usd": "0.01043417088", + "valueInCurrency": "0.00892772452", + }, + "total": Object { + "amount": "0.000008087", + "usd": "0.00521708544", + "valueInCurrency": "0.00446386226", + }, + }, + "includedTxFees": Object { + "amount": "1", + "usd": "999.831958465623542784", + "valueInCurrency": "855.479979591168903686", + }, + "minToTokenAmount": Object { + "amount": "0.009994389353314869", + "usd": "9.992709880792782241436661998044855296", + "valueInCurrency": "8.549999999999999909517932616692707134", + }, + "sentAmount": Object { + "amount": "11.689344272882887843", + "usd": "11.687379977535417949922677292586583974912", + "valueInCurrency": "9.999999999999999999997394921816007289298", + }, + "swapRate": "0.00089999999999999999", + "toTokenAmount": Object { + "amount": "0.010520409845594599", + "usd": "10.518641979781876096240273601395823616", + "valueInCurrency": "8.999999999999999949780980627632791914", + }, + "totalMaxNetworkFee": Object { + "amount": "0.000016174", + "usd": "0.01043417088", + "valueInCurrency": "0.00892772452", + }, + "totalNetworkFee": Object { + "amount": "0.000008087", + "usd": "0.00521708544", + "valueInCurrency": "0.00446386226", + }, + } + `); + }); }); it('should only fetch quotes once if balance is insufficient', () => { diff --git a/packages/bridge-controller/src/utils/metrics/properties.test.ts b/packages/bridge-controller/src/utils/metrics/properties.test.ts index 9dce6cc4de3..fd47f58f684 100644 --- a/packages/bridge-controller/src/utils/metrics/properties.test.ts +++ b/packages/bridge-controller/src/utils/metrics/properties.test.ts @@ -8,6 +8,7 @@ import { getSwapTypeFromQuote, formatProviderLabel, getRequestParams, + getQuotesReceivedProperties, } from './properties'; import type { QuoteResponse } from '../../types'; import { getNativeAssetForChainId } from '../bridge'; @@ -293,4 +294,86 @@ describe('properties', () => { }); }); }); + + describe('getQuotesReceivedProperties', () => { + it('should return quotes received properties correctly', () => { + const mockQuoteResponse: QuoteResponse = { + quote: { + requestId: 'request1', + srcChainId: 1, + srcAsset: { + chainId: 1, + address: '0x123', + symbol: 'ETH', + name: 'Ethereum', + decimals: 18, + assetId: 'eip155:1/slip44:60', + }, + srcTokenAmount: '1000000000000000000', + destChainId: 1, + destAsset: { + chainId: 1, + address: '0x456', + symbol: 'USDC', + name: 'USD Coin', + decimals: 6, + assetId: 'eip155:1/erc20:0x456', + }, + destTokenAmount: '1000000', + minDestTokenAmount: '950000', + feeData: { + metabridge: { + amount: '10000000000000000', + asset: { + chainId: 1, + address: '0x123', + symbol: 'ETH', + name: 'Ethereum', + decimals: 18, + assetId: 'eip155:1/slip44:60', + }, + }, + }, + bridgeId: 'bridge1', + bridges: ['bridge1'], + steps: [], + }, + trade: { + chainId: 1, + to: '0x789', + from: '0xabc', + value: '0', + data: '0x', + gasLimit: 100000, + }, + estimatedProcessingTimeInSeconds: 60, + }; + + const result = getQuotesReceivedProperties(mockQuoteResponse, [], false, { + ...mockQuoteResponse, + quote: { + ...mockQuoteResponse.quote, + bridges: ['bridge2'], + bridgeId: 'bridge2', + }, + }); + + expect(result).toMatchInlineSnapshot( + ` + Object { + "best_quote_provider": "bridge2_bridge2", + "can_submit": false, + "gas_included": false, + "gas_included_7702": false, + "price_impact": 0, + "provider": "bridge1_bridge1", + "quoted_time_minutes": 1, + "usd_quoted_gas": 0, + "usd_quoted_return": 0, + "warnings": Array [], + } + `, + ); + }); + }); }); diff --git a/packages/bridge-controller/src/utils/metrics/properties.ts b/packages/bridge-controller/src/utils/metrics/properties.ts index 6d6baa3f6ec..65fcc37dc19 100644 --- a/packages/bridge-controller/src/utils/metrics/properties.ts +++ b/packages/bridge-controller/src/utils/metrics/properties.ts @@ -1,9 +1,14 @@ import type { AccountsControllerState } from '@metamask/accounts-controller'; import { MetricsSwapType } from './constants'; -import type { InputKeys, InputValues, RequestParams } from './types'; +import type { + InputKeys, + InputValues, + QuoteWarning, + RequestParams, +} from './types'; import { DEFAULT_BRIDGE_CONTROLLER_STATE } from '../../constants/bridge'; -import type { QuoteResponse, TxData } from '../../types'; +import type { QuoteMetadata, QuoteResponse, TxData } from '../../types'; import { ChainId, type GenericQuoteRequest, @@ -114,3 +119,28 @@ export const isHardwareWallet = ( export const isCustomSlippage = (slippage: GenericQuoteRequest['slippage']) => { return slippage !== DEFAULT_BRIDGE_CONTROLLER_STATE.quoteRequest.slippage; }; + +export const getQuotesReceivedProperties = ( + activeQuote: null | (QuoteResponse & Partial), + warnings: QuoteWarning[] = [], + isSubmittable: boolean = true, + recommendedQuote?: null | (QuoteResponse & Partial), +) => { + const provider = activeQuote ? formatProviderLabel(activeQuote.quote) : '_'; + return { + can_submit: isSubmittable, + gas_included: Boolean(activeQuote?.quote?.gasIncluded), + gas_included_7702: Boolean(activeQuote?.quote?.gasIncluded7702), + quoted_time_minutes: activeQuote?.estimatedProcessingTimeInSeconds + ? activeQuote.estimatedProcessingTimeInSeconds / 60 + : 0, + usd_quoted_gas: Number(activeQuote?.gasFee?.effective?.usd ?? 0), + usd_quoted_return: Number(activeQuote?.toTokenAmount?.usd ?? 0), + best_quote_provider: recommendedQuote + ? formatProviderLabel(recommendedQuote.quote) + : provider, + provider, + warnings, + price_impact: Number(activeQuote?.quote.priceData?.priceImpact ?? 0), + }; +}; diff --git a/packages/bridge-controller/src/utils/metrics/types.ts b/packages/bridge-controller/src/utils/metrics/types.ts index 829070da14e..1504c332063 100644 --- a/packages/bridge-controller/src/utils/metrics/types.ts +++ b/packages/bridge-controller/src/utils/metrics/types.ts @@ -70,6 +70,15 @@ export type InputValues = { slippage: number; }; +export type QuoteWarning = + | 'low_return' + | 'no_quotes' + | 'insufficient_gas_balance' + | 'insufficient_gas_for_selected_quote' + | 'insufficient_balance' + | 'price_impact' + | 'tx_alert'; + /** * Properties that are required to be provided when trackUnifiedSwapBridgeEvent is called */ @@ -104,7 +113,7 @@ export type RequiredEventContextFromClient = { token_symbol_destination: RequestParams['token_symbol_destination']; }; [UnifiedSwapBridgeEventName.QuotesReceived]: TradeData & { - warnings: string[]; // TODO standardize warnings + warnings: QuoteWarning[]; best_quote_provider: QuoteFetchData['best_quote_provider']; price_impact: QuoteFetchData['price_impact']; can_submit: QuoteFetchData['can_submit']; diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index cb5ce316535..bb9d81841ed 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Update `submitTx` handler to accept optional `isLoading` and `warnings` arguments. When `isLoading=true`, the QuotesReceived event is published ([#7182](https://github.com/MetaMask/core/pull/7182)) + ## [61.0.0] ### Changed 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 450ef55a439..7755ebe12b3 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 @@ -1081,7 +1081,7 @@ Array [ ] `; -exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions 1`] = ` +exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 1`] = ` Object { "batchId": "batchId1", "chainId": "0xa4b1", @@ -1105,7 +1105,7 @@ Object { } `; -exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions 2`] = ` +exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 2`] = ` Object { "account": "0xaccount1", "approvalTxId": undefined, @@ -1227,7 +1227,7 @@ Object { } `; -exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions 3`] = ` +exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 3`] = ` Array [ Array [ Object { @@ -1245,7 +1245,7 @@ Array [ ] `; -exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions 4`] = ` +exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 4`] = ` Array [ Array [ Object { @@ -1279,8 +1279,27 @@ Array [ ] `; -exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions 5`] = ` +exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 5`] = ` Array [ + Array [ + "BridgeController:trackUnifiedSwapBridgeEvent", + "Unified SwapBridge Quotes Received", + Object { + "action_type": "swapbridge-v1", + "best_quote_provider": "lifi_across", + "can_submit": true, + "gas_included": false, + "gas_included_7702": false, + "price_impact": 0, + "provider": "lifi_across", + "quoted_time_minutes": 0.25, + "usd_quoted_gas": 2.5778, + "usd_quoted_return": 0.134214, + "warnings": Array [ + "low_return", + ], + }, + ], Array [ "BridgeController:stopPollingForQuotes", ], 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 640027d8c45..e8ab1b7553b 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.test.ts @@ -2559,7 +2559,8 @@ describe('BridgeStatusController', () => { expect(mockMessengerCall.mock.calls).toMatchSnapshot(); }); - it('should handle smart transactions', async () => { + it('should handle smart transactions and publish QuotesReceived event if quotes are still loading', async () => { + mockMessengerCall.mockImplementationOnce(jest.fn()); // track QuotesReceived event setupEventTrackingMocks(mockMessengerCall); setupBridgeStxMocks(mockMessengerCall); addTransactionBatchFn.mockResolvedValueOnce({ @@ -2573,6 +2574,8 @@ describe('BridgeStatusController', () => { (quoteWithoutApproval.trade as TxData).from, quoteWithoutApproval, true, + true, + ['low_return'], ); controller.stopAllPolling(); diff --git a/packages/bridge-status-controller/src/bridge-status-controller.ts b/packages/bridge-status-controller/src/bridge-status-controller.ts index bf7d613ff31..b4ee5084919 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.ts @@ -1,11 +1,13 @@ import type { AccountsControllerState } from '@metamask/accounts-controller'; import type { StateMetadata } from '@metamask/base-controller'; -import type { - QuoteMetadata, - RequiredEventContextFromClient, - TxData, - QuoteResponse, - Trade, +import type { QuoteWarning } from '@metamask/bridge-controller'; +import { + type QuoteMetadata, + type RequiredEventContextFromClient, + type TxData, + type QuoteResponse, + type Trade, + getQuotesReceivedProperties, } from '@metamask/bridge-controller'; import { formatChainIdToHex, @@ -1031,13 +1033,25 @@ export class BridgeStatusController extends StaticIntervalPollingController & QuoteMetadata, isStxEnabledOnClient: boolean, + isLoading: boolean = false, + warnings: QuoteWarning[] = [], ): Promise> => { + // If trade is submitted before all quotes are loaded, publish QuotesReceived event + if (isLoading) { + this.#trackUnifiedSwapBridgeEvent( + UnifiedSwapBridgeEventName.QuotesReceived, + undefined, + getQuotesReceivedProperties(quoteResponse, warnings), + ); + } this.messenger.call('BridgeController:stopPollingForQuotes'); const selectedAccount = this.#getMultichainSelectedAccount(accountAddress); @@ -1252,7 +1266,8 @@ export class BridgeStatusController extends StaticIntervalPollingController( eventName: T, txMetaId?: string,