diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 16c6073711b..f57b047baa3 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Batch `OnAssetConversion` and `OnAssetsMarketData` requests to non-EVM account Snaps ([#6886](https://github.com/MetaMask/core/pull/6886)) + ## [81.0.1] ### Fixed diff --git a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts index d2d013dd557..43c04096ab6 100644 --- a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts +++ b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts @@ -71,6 +71,20 @@ const fakeEvmAccountWithoutMetadata: InternalAccount = { methods: [], }; +const fakeNonEvmAccount2: InternalAccount = { + id: 'account5', + type: 'solana:data-account', + address: '0x123', + metadata: { + name: 'Test Account', + // @ts-expect-error-next-line + snap: { id: 'test-snap-2', enabled: true }, + }, + scopes: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'], + options: {}, + methods: [], +}; + const fakeMarketData = { price: 202.11, priceChange: 0, @@ -127,6 +141,9 @@ const setupController = ({ account1: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], account2: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], account3: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], + account5: [ + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v', + ], }, assetsMetadata: { 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { @@ -136,6 +153,14 @@ const setupController = ({ iconUrl: 'https://example.com/solana.png', units: [{ symbol: 'SOL', name: 'Solana', decimals: 9 }], }, + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': + { + name: 'USDC', + symbol: 'USDC', + fungible: true, + iconUrl: 'https://example.com/usdc.png', + units: [{ symbol: 'USDC', name: 'USDC', decimals: 2 }], + }, }, }), ); @@ -356,8 +381,8 @@ describe('MultichainAssetsRatesController', () => { type: KeyringTypes.snap, }, snap: { - id: 'mock-sol-snap', - name: 'mock-sol-snap', + id: 'mock-sol-snap-1', + name: 'mock-sol-snap-1', enabled: true, }, lastSelected: 0, @@ -377,8 +402,8 @@ describe('MultichainAssetsRatesController', () => { type: KeyringTypes.snap, }, snap: { - id: 'mock-sol-snap', - name: 'mock-sol-snap', + id: 'mock-sol-snap-2', + name: 'mock-sol-snap-2', enabled: true, }, lastSelected: 0, @@ -393,42 +418,49 @@ describe('MultichainAssetsRatesController', () => { accountsAssets: testAccounts, }); - const snapSpy = jest - .fn() - .mockResolvedValueOnce({ - conversionRates: { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - 'swift:0/iso4217:USD': { - rate: '100', - conversionTime: 1738539923277, + const mockResponses = { + onAssetsConversion: [ + { + conversionRates: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + 'swift:0/iso4217:USD': { + rate: '100', + conversionTime: 1738539923277, + }, }, }, }, - }) - .mockResolvedValueOnce({ - marketData: { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - 'swift:0/iso4217:USD': fakeMarketData, + { + conversionRates: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token1:501': { + 'swift:0/iso4217:USD': { + rate: '200', + conversionTime: 1738539923277, + }, + }, }, }, - }) - .mockResolvedValueOnce({ - conversionRates: { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token1:501': { - 'swift:0/iso4217:USD': { - rate: '200', - conversionTime: 1738539923277, - }, + ], + onAssetsMarketData: [ + { + marketData: { + 'swift:0/iso4217:USD': fakeMarketData, }, }, - }) - .mockResolvedValueOnce({ - marketData: { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token1:501': { + { + marketData: { 'swift:0/iso4217:USD': fakeMarketData, }, }, - }); + ], + }; + + const snapSpy = jest.fn().mockImplementation((args) => { + const { handler } = args; + return Promise.resolve( + mockResponses[handler as keyof typeof mockResponses].shift(), + ); + }); messenger.registerActionHandler('SnapController:handleRequest', snapSpy); messenger.publish('MultichainAssetsController:accountAssetListUpdated', { @@ -443,6 +475,7 @@ describe('MultichainAssetsRatesController', () => { }, }, }); + // Wait for the asynchronous subscriber to run. await Promise.resolve(); await advanceTime({ clock, duration: 10 }); @@ -625,7 +658,7 @@ describe('MultichainAssetsRatesController', () => { it('handles mixed success and failure scenarios', async () => { const { controller, messenger } = setupController({ - accountsAssets: [fakeNonEvmAccount, fakeEvmAccount2], + accountsAssets: [fakeNonEvmAccount, fakeNonEvmAccount2], }); const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); @@ -957,7 +990,7 @@ describe('MultichainAssetsRatesController', () => { const snapHandler = jest.fn().mockResolvedValue({ conversionRates: { 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - USD: { + 'swift:0/iso4217:USD': { rate: '100.50', conversionTime: Date.now(), }, diff --git a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts index c25a861087e..0b8054ed01a 100644 --- a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts +++ b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts @@ -31,7 +31,6 @@ import type { import { HandlerType } from '@metamask/snaps-utils'; import { Mutex } from 'async-mutex'; import type { Draft } from 'immer'; -import { cloneDeep } from 'lodash'; import { MAP_CAIP_CURRENCIES } from './constant'; import type { @@ -181,6 +180,15 @@ export type ConversionRatesWithMarketData = { >; }; +/** + * Arguments for a Snap request. + */ +type SnapRequestArgs = { + snapId: SnapId; + handler: HandlerType; + params: T; +}; + /** * Controller that manages multichain token conversion rates. * @@ -320,6 +328,42 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro return accounts.filter((account) => this.#isNonEvmAccount(account)); } + /** + * Adds the assets to a map of Snap ID to assets. + * + * @param snapIdToAssets - The map of Snap ID to assets. + * @param account - The account to add the assets for. + * @param assets - The assets to add. + */ + #addAssetsToSnapIdMap( + snapIdToAssets: Map>, + account: InternalAccount, + assets: CaipAssetType[], + ): void { + // Prevent creating a new set if there are no assets to add. + if (assets.length === 0) { + return; + } + + // FIXME: Instead of using the Snap ID from the account, we should + // select the Snap based on the supported scopes defined in the Snaps' + // manifest. + const snapId = account.metadata.snap?.id as SnapId | undefined; + if (!snapId) { + return; + } + + let snapAssets = snapIdToAssets.get(snapId); + if (!snapAssets) { + snapAssets = new Set(); + snapIdToAssets.set(snapId, snapAssets); + } + + for (const asset of assets) { + snapAssets.add(asset); + } + } + /** * Updates token conversion rates for each non-EVM account. * @@ -332,75 +376,173 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro if (!this.isActive) { return; } - const accounts = this.#listAccounts(); + // Compute the set of unique assets from all accounts. It's important to + // deduplicate assets here to avoid duplicate requests to the Snap. + const accounts = this.#listAccounts(); + const snapIdToAssets = new Map>(); for (const account of accounts) { - const assets = this.#getAssetsForAccount(account.id); - - if (assets?.length === 0) { - continue; - } - - const rates = await this.#getUpdatedRatesFor(account, assets); - // Apply these updated rates to controller state - this.#applyUpdatedRates(rates); + this.#addAssetsToSnapIdMap( + snapIdToAssets, + account, + this.#getAssetsForAccount(account.id), + ); } + + this.#applyUpdatedRates(await this.#getUpdatedRatesFor(snapIdToAssets)); })().finally(() => { releaseLock(); }); } - async #getUpdatedRatesFor( - account: InternalAccount, - assets: CaipAssetType[], - ): Promise< - Record - > { - // Do not attempt to retrieve rates from Snap if there are no assets - if (!assets.length) { + /** + * Returns the CAIP-19 asset type for the current selected currency. Defaults + * to USD if the current selected currency is not supported. + * + * @returns The CAIP-19 asset type for the current selected currency. + */ + #getCaipCurrentCurrency(): CaipAssetType { + return ( + MAP_CAIP_CURRENCIES[this.#currentCurrency] ?? MAP_CAIP_CURRENCIES.usd + ); + } + + /** + * Fetches the conversion rates for the given assets from the given Snap. + * + * @param snapId - The ID of the Snap. + * @param assets - The assets to fetch the conversion rates for. + * @param currency - The currency to fetch the conversion rates for. + * @returns A record of CAIP-19 asset types to conversion rates. + */ + async #getConversionRates( + snapId: SnapId, + assets: Set, + currency: CaipAssetType, + ): Promise> { + // Prevent making a Snap call if there are no assets to fetch. + if (assets.size === 0) { return {}; } - // Build the conversions array - const conversions = this.#buildConversions(assets); - - // Retrieve rates from Snap - const accountRatesResponse = (await this.#handleSnapRequest({ - snapId: account?.metadata.snap?.id as SnapId, + const response = await this.#handleSnapRequest({ + snapId, handler: HandlerType.OnAssetsConversion, - params: conversions, - })) as OnAssetsConversionResponse | null; + params: { + conversions: Array.from(assets).map((asset) => ({ + from: asset, + to: currency, + })), + }, + }); - // If the snap request failed, return empty rates - if (!accountRatesResponse) { + if (!response) { return {}; } - // Prepare assets param for onAssetsMarketData - const currentCurrencyCaip = - MAP_CAIP_CURRENCIES[this.#currentCurrency] ?? MAP_CAIP_CURRENCIES.usd; - const assetsParam = { - assets: assets.map((asset) => ({ asset, unit: currentCurrencyCaip })), - }; + const assetToConversionRate: Record< + CaipAssetType, + AssetConversion | undefined + > = {}; + + for (const asset of assets) { + assetToConversionRate[asset] = + response.conversionRates?.[asset]?.[currency] ?? undefined; + } + + return assetToConversionRate; + } + + /** + * Fetches the market data for the given assets from the given Snap. + * + * @param snapId - The ID of the Snap. + * @param assets - The assets to fetch the market data for. + * @param currency - The currency to fetch the market data for. + * @returns A record of CAIP-19 asset types to market data. + */ + async #getMarketData( + snapId: SnapId, + assets: Set, + currency: CaipAssetType, + ): Promise> { + // Prevent making a Snap call if there are no assets to fetch. + if (assets.size === 0) { + return {}; + } - // Retrieve Market Data from Snap - const marketDataResponse = (await this.#handleSnapRequest({ - snapId: account?.metadata.snap?.id as SnapId, + const response = await this.#handleSnapRequest({ + snapId, handler: HandlerType.OnAssetsMarketData, - params: assetsParam as OnAssetsMarketDataArguments, - })) as OnAssetsMarketDataResponse | null; + params: { + assets: Array.from(assets).map((asset) => ({ + asset, + unit: currency, + })), + }, + }); - // Merge market data into conversion rates if available - const mergedRates = this.#mergeMarketDataIntoConversionRates( - accountRatesResponse, - marketDataResponse, - ); + if (!response) { + return {}; + } - // Flatten nested rates if needed - const flattenedRates = this.#flattenRates(mergedRates); + const assetToMarketData: Record< + CaipAssetType, + FungibleAssetMarketData | undefined + > = {}; - // Build the updatedRates object for these assets - const updatedRates = this.#buildUpdatedRates(assets, flattenedRates); + for (const asset of assets) { + assetToMarketData[asset] = + response.marketData?.[asset]?.[currency] ?? undefined; + } + + return assetToMarketData; + } + + /** + * Fetches the updated rates for the given assets from the given Snaps. + * + * @param snapIdToAssets - A map of Snap ID to CAIP-19 asset types. + * @returns A record of CAIP-19 asset types to unified asset conversions. + */ + async #getUpdatedRatesFor( + snapIdToAssets: Map>, + ): Promise< + Record + > { + const updatedRates: Record< + CaipAssetType, + UnifiedAssetConversion & { currency: CaipAssetType } + > = {}; + + // Keep a local copy to ensure that the currency is always the same for the + // entire loop. + const currency = this.#getCaipCurrentCurrency(); + + // Note: Since the assets come from a 1-to-1 mapping with Snap IDs, we know + // that a given asset will not appear under multiple Snap IDs. + for (const [snapId, assets] of snapIdToAssets.entries()) { + const [rates, marketData] = await Promise.all([ + this.#getConversionRates(snapId, assets, currency), + this.#getMarketData(snapId, assets, currency), + ]); + + for (const asset of assets) { + const assetRate = rates[asset]; + const assetMarketData = marketData[asset]; + + // Rates are mandatory, so skip the asset if not available. + if (!assetRate) { + continue; + } + + updatedRates[asset] = { + currency, + ...assetRate, + ...(assetMarketData && { marketData: assetMarketData }), + }; + } + } return updatedRates; } @@ -500,22 +642,20 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro if (!this.isActive) { return; } - const allNewRates: Record< - string, - UnifiedAssetConversion & { currency: CaipAssetType } - > = {}; - for (const { accountId, assets } of accounts) { - const account = this.#getAccount(accountId); + // First build a map containing all assets that need to be updated per + // Snap ID, this will be used to batch the requests. + const snapIdToAssets = new Map>(); - const rates = await this.#getUpdatedRatesFor(account, assets); - // Track new rates - for (const [asset, rate] of Object.entries(rates)) { - allNewRates[asset] = rate; - } + for (const { accountId, assets } of accounts) { + this.#addAssetsToSnapIdMap( + snapIdToAssets, + this.#getAccount(accountId), + assets, + ); } - this.#applyUpdatedRates(allNewRates); + this.#applyUpdatedRates(await this.#getUpdatedRatesFor(snapIdToAssets)); })().finally(() => { releaseLock(); }); @@ -550,77 +690,6 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro return this.#accountsAssets?.[accountId] ?? []; } - /** - * Builds a conversions array (from each asset → the current currency). - * - * @param assets - The assets to build the conversions for. - * @returns A conversions array. - */ - #buildConversions(assets: CaipAssetType[]): OnAssetsConversionArguments { - const currency = - MAP_CAIP_CURRENCIES[this.#currentCurrency] ?? MAP_CAIP_CURRENCIES.usd; - return { - conversions: assets.map((asset) => ({ - from: asset, - to: currency, - })), - }; - } - - /** - * Flattens any nested structure in the conversion rates returned by Snap. - * - * @param assetsConversionResponse - The conversion rates to flatten. - * @returns A flattened rates object. - */ - #flattenRates( - assetsConversionResponse: ConversionRatesWithMarketData | null, - ): Record { - if (!assetsConversionResponse?.conversionRates) { - return {}; - } - - const { conversionRates } = assetsConversionResponse; - - return Object.fromEntries( - Object.entries(conversionRates).map(([asset, nestedObj]) => { - // e.g., nestedObj might look like: { "swift:0/iso4217:EUR": { rate, conversionTime } } - const singleValue = Object.values(nestedObj)[0]; - return [asset, singleValue]; - }), - ); - } - - /** - * Builds a rates object that covers all given assets, ensuring that - * any asset not returned by Snap is set to null for both `rate` and `conversionTime`. - * - * @param assets - The assets to build the rates for. - * @param flattenedRates - The rates to merge. - * @returns A rates object that covers all given assets. - */ - #buildUpdatedRates( - assets: CaipAssetType[], - flattenedRates: Record, - ): Record { - const updatedRates: Record< - CaipAssetType, - UnifiedAssetConversion & { currency: CaipAssetType } - > = {}; - - for (const asset of assets) { - if (flattenedRates[asset]) { - updatedRates[asset] = { - ...(flattenedRates[asset] as UnifiedAssetConversion), - currency: - MAP_CAIP_CURRENCIES[this.#currentCurrency] ?? - MAP_CAIP_CURRENCIES.usd, - }; - } - } - return updatedRates; - } - /** * Merges the new rates into the controller's state. * @@ -628,7 +697,7 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro */ #applyUpdatedRates( updatedRates: Record< - string, + CaipAssetType, UnifiedAssetConversion & { currency: CaipAssetType } >, ): void { @@ -652,25 +721,22 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro * @param args.params - The asset conversions. * @returns A promise that resolves with the account rates. */ - async #handleSnapRequest({ - snapId, - handler, - params, - }: { - snapId: SnapId; - handler: HandlerType; - params: - | OnAssetsConversionArguments - | OnAssetHistoricalPriceArguments - | OnAssetsMarketDataArguments; - }): Promise< - | OnAssetsConversionResponse - | OnAssetHistoricalPriceResponse - | OnAssetsMarketDataResponse - | undefined - > { + async #handleSnapRequest( + args: SnapRequestArgs, + ): Promise; + + async #handleSnapRequest( + args: SnapRequestArgs, + ): Promise; + + async #handleSnapRequest( + args: SnapRequestArgs, + ): Promise; + + async #handleSnapRequest(args: SnapRequestArgs): Promise { + const { snapId, handler, params } = args; try { - return (await this.messagingSystem.call('SnapController:handleRequest', { + return await this.messagingSystem.call('SnapController:handleRequest', { snapId, origin: 'metamask', handler, @@ -679,11 +745,7 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro method: handler, params, }, - })) as - | OnAssetsConversionResponse - | OnAssetHistoricalPriceResponse - | OnAssetsMarketDataResponse - | undefined; + }); } catch (error) { console.error(`Snap request failed for ${handler}:`, { snapId, @@ -691,49 +753,7 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro message: (error as Error).message, params, }); - // Ignore return undefined; } } - - #mergeMarketDataIntoConversionRates( - accountRatesResponse: OnAssetsConversionResponse, - marketDataResponse: OnAssetsMarketDataResponse | null, - ): ConversionRatesWithMarketData { - // Early return if no market data to merge - if (!marketDataResponse?.marketData) { - return accountRatesResponse; - } - - const result: ConversionRatesWithMarketData = - cloneDeep(accountRatesResponse); - const { conversionRates } = result; - const { marketData } = marketDataResponse; - - // Iterate through each asset in market data - for (const [assetId, currencyData] of Object.entries(marketData)) { - const typedAssetId = assetId as CaipAssetType; - - // Iterate through each currency for this asset - for (const [currency, marketDataForCurrency] of Object.entries( - currencyData, - )) { - const typedCurrency = currency as CaipAssetType; - - // Check if this currency exists in conversion rates for this asset - const existingRate = conversionRates[typedAssetId][typedCurrency]; - if (!existingRate) { - continue; - } - - // Merge market data into the existing conversion rate - conversionRates[typedAssetId][typedCurrency] = { - ...existingRate, - marketData: marketDataForCurrency ?? undefined, - }; - } - } - - return result; - } }