refactor: bridge QuoteMetadata read/write

packages/bridge-controller/CHANGELOG.md

@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **BREAKING:** Make `QuoteMetadata` fields optional, remove `0` and `null` amount fallbacks ([#9507](https://github.com/MetaMask/core/pull/9507))
+- Refactor quoteMetadata calculation and data access to prepare for metadata migration ([#9507](https://github.com/MetaMask/core/pull/9507))
+  - Extract `QuoteMetadata` type and calculation to a new file
+  - Implement `mergeQuoteMetadata` util which appends QuoteMetadata to QuoteResponse
 - Bump `@metamask/assets-controller` from `^10.2.1` to `^11.0.0` ([#9485](https://github.com/MetaMask/core/pull/9485))
 
 ### Fixed

packages/bridge-controller/src/index.ts

@@ -37,19 +37,20 @@ export {
   getQuotesReceivedProperties,
 } from './utils/metrics/properties';
 
+export type { QuoteMetadata, TokenAmountValues } from './utils/quote-metadata';
+export { mergeQuoteMetadata } from './utils/quote-metadata';
+
 export type {
   ChainConfiguration,
   L1GasFees,
   NonEvmFees,
-  QuoteMetadata,
   GasMultiplierByChainId,
   FeatureFlagResponse,
   BridgeAsset,
   GenericQuoteRequest,
   BatchSellTradesResponse,
   GaslessProperties,
   SimulatedGasFeeLimits,
-  TokenAmountValues,
   Step,
   RefuelData,
   FeeData,

packages/bridge-controller/src/selectors.test.ts

@@ -33,10 +33,10 @@ import { getNativeAssetForChainId, isNativeAddress } from './utils/bridge';
 import {
   formatAddressToAssetId,
   formatAddressToCaipReference,
-  formatChainIdToCaip,
   formatChainIdToDec,
   formatChainIdToHex,
 } from './utils/caip-formatters';
+import { mergeQuoteMetadata } from './utils/quote-metadata';
 import { BatchSellTransactionType } from './validators/batch-sell';
 import type { QuoteResponseV1 } from './validators/quote-response-v1';
 import { validateQuoteResponseV1 } from './validators/quote-response-v1';
@@ -412,7 +412,6 @@ describe('Bridge Selectors', () => {
       stateOverrides?: Partial<BridgeAppState>,
     ): BridgeAppState => {
       const decChainId = formatChainIdToDec(chainId);
-      const caipChainId = formatChainIdToCaip(chainId);
 
       const mockQuote = {
         quote: {
@@ -426,16 +425,15 @@ describe('Bridge Selectors', () => {
             chainId: decChainId,
             address: '0x0000000000000000000000000000000000000000',
             decimals: 18,
-            assetId: `${caipChainId}/erc20:0x0000000000000000000000000000000000000000`,
+            assetId: getNativeAssetForChainId(chainId).assetId.toLowerCase(),
             symbol: 'ETH',
             name: 'Ethereum',
           },
           destAsset: {
             chainId: 137,
             address: '0x0000000000000000000000000000000000000000',
             decimals: 18,
-            assetId:
-              'eip155:137/erc20:0x0000000000000000000000000000000000000000',
+            assetId: getNativeAssetForChainId(137).assetId.toLowerCase(),
             symbol: 'POL',
             name: 'Polygon',
           },
@@ -451,7 +449,8 @@ describe('Bridge Selectors', () => {
                 decimals: 18,
                 symbol: 'ETH',
                 name: 'Ethereum',
-                assetId: `${caipChainId}/erc20:0x0000000000000000000000000000000000000000`,
+                assetId:
+                  getNativeAssetForChainId(chainId).assetId.toLowerCase(),
               },
             },
           },
@@ -476,17 +475,10 @@ describe('Bridge Selectors', () => {
           value: '0x0',
         },
       };
-      validateQuoteResponseV1(mockQuote);
 
       return {
         quotes: [
-          merge(
-            {},
-            {
-              ...mockQuote,
-            },
-            quoteOverrides,
-          ),
+          merge({}, mockQuote, quoteOverrides),
           merge(
             {},
             {
@@ -575,17 +567,21 @@ describe('Bridge Selectors', () => {
           {
             ...mockState,
             assetExchangeRates: {
-              [formatAddressToAssetId(
+              [mockQuote.quote.srcAsset.assetId.toLowerCase() ??
+              formatAddressToAssetId(
                 mockQuote.quote.srcAsset.address,
                 mockQuote.quote.srcChainId,
-              ) ?? '']: {
+              ) ??
+              '']: {
                 exchangeRate: '1980',
                 usdExchangeRate: '10',
               },
-              [formatAddressToAssetId(
+              [mockQuote.quote.destAsset.assetId.toLowerCase() ??
+              formatAddressToAssetId(
                 mockQuote.quote.destAsset.address,
                 mockQuote.quote.destChainId,
-              ) ?? '']: {
+              ) ??
+              '']: {
                 exchangeRate: '200',
                 usdExchangeRate: '1',
               },
@@ -659,6 +655,58 @@ describe('Bridge Selectors', () => {
       expect(result.sortedQuotes[0].cost?.valueInCurrency).toBe('-419.985546');
     });
 
+    it('should return sorted quotes with metadata (no assetId)', () => {
+      const mockState = getMockState(1, {
+        quote: {
+          srcAsset: {
+            chainId: 1,
+            address: '0x0000000000000000000000000000000000000000',
+            decimals: 18,
+            assetId: null,
+            symbol: 'ETH',
+            name: 'Ethereum',
+          },
+        } as never,
+      });
+      const mockQuote = mockState.quotes[0];
+      const { quotesInitialLoadTimeMs, quotesLastFetchedMs, ...result } =
+        selectBridgeQuotes(
+          {
+            ...mockState,
+            assetExchangeRates: {
+              [mockQuote.quote.srcAsset.assetId?.toLowerCase() ??
+              formatAddressToAssetId(
+                mockQuote.quote.srcAsset.address,
+                mockQuote.quote.srcChainId,
+              ) ??
+              '']: {
+                exchangeRate: '1980',
+                usdExchangeRate: '10',
+              },
+              [mockQuote.quote.destAsset.assetId?.toLowerCase() ??
+              formatAddressToAssetId(
+                mockQuote.quote.destAsset.address,
+                mockQuote.quote.destChainId,
+              ) ??
+              '']: {
+                exchangeRate: '200',
+                usdExchangeRate: '1',
+              },
+            },
+          },
+          mockClientParams,
+        );
+
+      const quoteResponseV1 = {
+        ...mockState.quotes[1],
+      };
+
+      expect(result.sortedQuotes[0]).toStrictEqual(
+        expect.objectContaining(quoteResponseV1),
+      );
+      expect(result.sortedQuotes[0].cost?.valueInCurrency).toBe('-419.985546');
+    });
+
     it('should return metadata when quotes are empty', () => {
       const mockState = getMockState(1);
       const mockQuote = mockState.quotes[0];
@@ -1845,7 +1893,10 @@ describe('Bridge Selectors', () => {
         },
       };
       const solanaQuote = solanaState.quotes[1];
-      const quoteResponseV1 = { ...solanaQuote, ...expectedQuoteMetadata };
+      const quoteResponseV1 = mergeQuoteMetadata(
+        solanaQuote,
+        expectedQuoteMetadata,
+      );
       validateQuoteResponseV1(quoteResponseV1);
       expect(result.recommendedQuote).toStrictEqual(quoteResponseV1);
     });
@@ -1981,7 +2032,7 @@ describe('Bridge Selectors', () => {
         ]
       `);
       expect(
-        recommendedQuotes.map((quote) => quote?.sentAmount.usd),
+        recommendedQuotes.map((quote) => quote?.sentAmount?.usd),
       ).toStrictEqual(['18', '140']);
     });
 

packages/bridge-controller/src/selectors.ts

@@ -19,16 +19,10 @@ import {
 } from 'reselect';
 
 import { BRIDGE_PREFERRED_GAS_ESTIMATE } from './constants/bridge';
-import type {
-  BridgeControllerState,
-  ExchangeRate,
-  QuoteMetadata,
-  TokenAmountValues,
-} from './types';
+import type { BridgeControllerState, ExchangeRate } from './types';
 import { RequestStatus, SortOrder } from './types';
 import {
   getNativeAssetForChainId,
-  isEvmQuoteResponse,
   isNativeAddress,
   isNonEvmChainId,
 } from './utils/bridge';
@@ -39,20 +33,10 @@ import {
   formatChainIdToHex,
 } from './utils/caip-formatters';
 import { processFeatureFlags } from './utils/feature-flags';
-import {
-  calcAdjustedReturn,
-  calcCost,
-  calcEstimatedAndMaxTotalGasFee,
-  calcIncludedTxFees,
-  calcRelayerFee,
-  calcSentAmount,
-  calcNonEvmTotalNetworkFee,
-  calcSwapRate,
-  calcToAmount,
-  calcTotalEstimatedNetworkFee,
-  calcTotalMaxNetworkFee,
-  calcBatchFees,
-} from './utils/quote';
+import { calcBatchFees } from './utils/quote';
+import type { TokenAmountValues } from './utils/quote-metadata';
+import { calcQuoteMetadata, mergeQuoteMetadata } from './utils/quote-metadata';
+import type { QuoteMetadata } from './utils/quote-metadata';
 import { getDefaultSlippagePercentage } from './utils/slippage';
 import type { QuoteResponseV1 } from './validators/quote-response-v1';
 
@@ -358,100 +342,28 @@ const selectBridgeQuotesWithMetadata = createBridgeSelector(
     destTokenExchangeRate,
     nativeExchangeRate,
   ) => {
-    const newQuotes = quotes.map((quote) => {
+    return quotes.map((quote) => {
+      // This is a fallback for client unit tests
+      const fallbackSourceAssetId = formatAddressToAssetId(
+        quote.quote.srcAsset.address,
+        quote.quote.srcChainId,
+      );
       const sourceAssetId =
-        formatAddressToAssetId(
-          quote.quote.srcAsset.address,
-          quote.quote.srcChainId,
-        ) ?? quote.quote.srcAsset.assetId;
+        quote.quote.srcAsset.assetId ??
+        /* c8 ignore next */
+        fallbackSourceAssetId;
       const srcTokenExchangeRate = selectExchangeRateByAssetId(
         exchangeRateSources,
         sourceAssetId,
       );
-      const sentAmount = calcSentAmount(quote.quote, srcTokenExchangeRate);
-      const toTokenAmount = calcToAmount(
-        quote.quote.destTokenAmount,
-        quote.quote.destAsset,
-        destTokenExchangeRate,
-      );
-      const minToTokenAmount = calcToAmount(
-        quote.quote.minDestTokenAmount,
-        quote.quote.destAsset,
-        destTokenExchangeRate,
-      );
-
-      const includedTxFees = calcIncludedTxFees(
-        quote.quote,
+      const quoteMetadata = calcQuoteMetadata(quote, {
         srcTokenExchangeRate,
+        bridgeFeesPerGas,
         destTokenExchangeRate,
-      );
-
-      let totalEstimatedNetworkFee,
-        totalMaxNetworkFee,
-        relayerFee,
-        gasFee: QuoteMetadata['gasFee'];
-
-      if (isEvmQuoteResponse(quote)) {
-        relayerFee = calcRelayerFee(quote, nativeExchangeRate);
-        gasFee = calcEstimatedAndMaxTotalGasFee({
-          bridgeQuote: quote,
-          ...bridgeFeesPerGas,
-          ...nativeExchangeRate,
-        });
-        // Uses effectiveGasFee to calculate the total estimated network fee
-        totalEstimatedNetworkFee = calcTotalEstimatedNetworkFee(
-          gasFee,
-          relayerFee,
-        );
-        totalMaxNetworkFee = calcTotalMaxNetworkFee(gasFee, relayerFee);
-      } else {
-        // Use the new generic function for all non-EVM chains
-        totalEstimatedNetworkFee = calcNonEvmTotalNetworkFee(
-          quote,
-          nativeExchangeRate,
-        );
-        gasFee = {
-          effective: totalEstimatedNetworkFee,
-          total: totalEstimatedNetworkFee,
-          max: totalEstimatedNetworkFee,
-        };
-        totalMaxNetworkFee = totalEstimatedNetworkFee;
-      }
-
-      const adjustedReturn = calcAdjustedReturn(
-        toTokenAmount,
-        totalEstimatedNetworkFee,
-        quote.quote,
-      );
-      const cost = calcCost(adjustedReturn, sentAmount);
-
-      return {
-        ...quote,
-        // QuoteMetadata fields
-        sentAmount,
-        toTokenAmount,
-        minToTokenAmount,
-        swapRate: calcSwapRate(sentAmount.amount, toTokenAmount.amount),
-        /**
-        This is the amount required to submit all the transactions.
-        Includes the relayer fee or other native fees.
-        Should be used for balance checks and tx submission.
-         */
-        totalNetworkFee: totalEstimatedNetworkFee,
-        totalMaxNetworkFee,
-        /**
-        This contains gas fee estimates for the bridge transaction
-        Does not include the relayer fee (if needed), just the gasLimit and effectiveGas returned by the bridge API.
-        Should only be used for display purposes.
-         */
-        gasFee,
-        adjustedReturn,
-        cost,
-        includedTxFees,
-      };
+        nativeExchangeRate,
+      });
+      return mergeQuoteMetadata(quote, quoteMetadata);
     });
-
-    return newQuotes;
   },
 );
 
@@ -469,10 +381,10 @@ const selectSortedBridgeQuotes = createBridgeSelector(
           'asc',
         );
       default:
-        if (quotesWithMetadata.every((quote) => quote.cost.valueInCurrency)) {
+        if (quotesWithMetadata.every((quote) => quote?.cost?.valueInCurrency)) {
           return orderBy(
             quotesWithMetadata,
-            ({ cost }) => Number(cost.valueInCurrency),
+            ({ cost }) => Number(cost?.valueInCurrency),
             'asc',
           );
         }