From 551e3c3bdcbdd50f6e373eb48541638aea094681 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 14:07:38 +0100 Subject: [PATCH 01/36] feat: defi controller v2 --- defi-visualizer.html | 714 ++++++++++++++++++ ...sitionsControllerV2-method-action-types.ts | 27 + .../DeFiPositionsControllerV2.ts | 262 +++++++ .../build-defi-balances-query.ts | 141 ++++ .../group-defi-positions-v6.ts | 340 +++++++++ packages/assets-controllers/src/index.ts | 23 + 6 files changed, 1507 insertions(+) create mode 100644 defi-visualizer.html create mode 100644 packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts create mode 100644 packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts create mode 100644 packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts create mode 100644 packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts diff --git a/defi-visualizer.html b/defi-visualizer.html new file mode 100644 index 00000000000..82611c6284b --- /dev/null +++ b/defi-visualizer.html @@ -0,0 +1,714 @@ + + + + + +DeFi Balances Visualizer + + + +
+

DeFi Balances Visualizer

+

MetaMask multiaccount balances API · DeFi positions across 7 EVM chains

+
+ + + +
+
Fetches Mainnet, Linea, Base, Arbitrum, BNB Chain, Optimism & Polygon. Only defi balances are shown; all data is preserved in the raw response.
+
+ +
+ + + +
+ +
+ + + + diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts new file mode 100644 index 00000000000..ebc596f4e98 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -0,0 +1,27 @@ +/** + * This file is auto generated. + * Do not edit manually. + */ + +import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; + +/** + * Fetches DeFi positions for the selected account group and stores them, + * shaped for direct client consumption. Everything happens behind this + * method: resolving the accounts, calling the Accounts API, transforming the + * response, and updating state. + * + * Throttled per set of accounts by an in-memory minimum interval, so repeated + * calls within the window are no-ops. Disabled controllers and empty account + * groups return without fetching. + */ +export type DeFiPositionsControllerV2FetchDeFiPositionsAction = { + type: `DeFiPositionsControllerV2:fetchDeFiPositions`; + handler: DeFiPositionsControllerV2['fetchDeFiPositions']; +}; + +/** + * Union of all DeFiPositionsControllerV2 action types. + */ +export type DeFiPositionsControllerV2MethodActions = + DeFiPositionsControllerV2FetchDeFiPositionsAction; diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts new file mode 100644 index 00000000000..d5a50994b0b --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -0,0 +1,262 @@ +import type { AccountTreeControllerGetAccountsFromSelectedAccountGroupAction } from '@metamask/account-tree-controller'; +import { BaseController } from '@metamask/base-controller'; +import type { + ControllerGetStateAction, + ControllerStateChangedEvent, + StateMetadata, +} from '@metamask/base-controller'; +import type { ApiPlatformClient } from '@metamask/core-backend'; +import type { Messenger } from '@metamask/messenger'; + +import { + buildDeFiBalancesQuery, + DEFI_BALANCES_V6_REQUEST_OPTIONS, + normalizeCaipAccountId, +} from './build-defi-balances-query'; +import type { DeFiPositionsByAccount } from './group-defi-positions-v6'; +import { groupDeFiPositionsV6 } from './group-defi-positions-v6'; +import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; + +const controllerName = 'DeFiPositionsControllerV2'; + +const ONE_MINUTE_IN_MS = 60_000; + +const MESSENGER_EXPOSED_METHODS = ['fetchDeFiPositions'] as const; + +export type { + DeFiPositionsByAccount, + DeFiProtocolPositionGroup, + DeFiPositionDetailsSection, + DeFiPositionPoolGroup, + DeFiUnderlyingPosition, + DeFiPositionIconGroupItem, +} from './group-defi-positions-v6'; + +export type DeFiPositionsControllerV2State = { + /** + * DeFi positions keyed by internal MetaMask account ID (`InternalAccount.id`, + * the same key AssetsController uses). Each account maps to a flat list of + * protocol groups shown in the DeFi tab, each carrying its own `chainId` for + * filtering plus the details-page sections embedded inside it. This is + * exactly the shape the client consumes, so no further transformation is + * needed on read. + */ + allDeFiPositions: DeFiPositionsByAccount; +}; + +const controllerMetadata: StateMetadata = { + allDeFiPositions: { + includeInStateLogs: false, + persist: true, + includeInDebugSnapshot: false, + usedInUi: true, + }, +}; + +export const getDefaultDeFiPositionsControllerV2State = + (): DeFiPositionsControllerV2State => { + return { + allDeFiPositions: {}, + }; + }; + +export type DeFiPositionsControllerV2GetStateAction = ControllerGetStateAction< + typeof controllerName, + DeFiPositionsControllerV2State +>; + +export type DeFiPositionsControllerV2Actions = + | DeFiPositionsControllerV2GetStateAction + | DeFiPositionsControllerV2MethodActions; + +export type DeFiPositionsControllerV2StateChangedEvent = + ControllerStateChangedEvent< + typeof controllerName, + DeFiPositionsControllerV2State + >; + +export type DeFiPositionsControllerV2Events = + DeFiPositionsControllerV2StateChangedEvent; + +/** + * The external actions available to the {@link DeFiPositionsControllerV2}. + */ +export type AllowedActions = + AccountTreeControllerGetAccountsFromSelectedAccountGroupAction; + +/** + * The external events available to the {@link DeFiPositionsControllerV2}. + * + * None for now. When wiring the controller into a client, events such as + * `KeyringController:lock`, `TransactionController:transactionConfirmed`, and + * `AccountTreeController:selectedAccountGroupChange` can be added here and + * subscribed to in order to trigger/clear fetches. + */ +export type AllowedEvents = never; + +export type DeFiPositionsControllerV2Messenger = Messenger< + typeof controllerName, + DeFiPositionsControllerV2Actions | AllowedActions, + DeFiPositionsControllerV2Events | AllowedEvents +>; + +/** + * Controller that fetches DeFi positions for the selected account group from + * the Accounts API (v6 multiaccount balances) and stores them in the shape the + * client consumes directly. + */ +export class DeFiPositionsControllerV2 extends BaseController< + typeof controllerName, + DeFiPositionsControllerV2State, + DeFiPositionsControllerV2Messenger +> { + readonly #apiClient: ApiPlatformClient; + + readonly #isEnabled: () => boolean; + + readonly #getVsCurrency: () => string; + + readonly #minimumFetchIntervalMs: number; + + /** + * In-memory timestamp (ms) of the last fetch per query, keyed by the sorted + * account IDs. Intentionally not persisted: it resets on restart, so the + * first fetch after a restart always goes through. + */ + readonly #lastFetchByKey = new Map(); + + /** + * @param options - Constructor options. + * @param options.messenger - The controller messenger. + * @param options.apiClient - Accounts API client used to fetch balances/positions. Auth is handled by the client. + * @param options.isEnabled - Returns whether fetching is enabled (default: () => false). + * @param options.getVsCurrency - Returns the fiat currency for prices (default: () => 'usd'). + * @param options.minimumFetchIntervalMs - Minimum time between fetches for the same accounts (default: 1 minute). + * @param options.state - Initial controller state. + */ + constructor({ + messenger, + apiClient, + isEnabled = (): boolean => false, + getVsCurrency = (): string => DEFI_BALANCES_V6_REQUEST_OPTIONS.vsCurrency, + minimumFetchIntervalMs = ONE_MINUTE_IN_MS, + state, + }: { + messenger: DeFiPositionsControllerV2Messenger; + apiClient: ApiPlatformClient; + isEnabled?: () => boolean; + getVsCurrency?: () => string; + minimumFetchIntervalMs?: number; + state?: Partial; + }) { + super({ + name: controllerName, + metadata: controllerMetadata, + messenger, + state: { + ...getDefaultDeFiPositionsControllerV2State(), + ...state, + }, + }); + + this.#apiClient = apiClient; + this.#isEnabled = isEnabled; + this.#getVsCurrency = getVsCurrency; + this.#minimumFetchIntervalMs = minimumFetchIntervalMs; + + this.messenger.registerMethodActionHandlers( + this, + MESSENGER_EXPOSED_METHODS, + ); + } + + /** + * Fetches DeFi positions for the selected account group and stores them, + * shaped for direct client consumption. Everything happens behind this + * method: resolving the accounts, calling the Accounts API, transforming the + * response, and updating state. + * + * Throttled per set of accounts by an in-memory minimum interval, so repeated + * calls within the window are no-ops. Disabled controllers and empty account + * groups return without fetching. + */ + async fetchDeFiPositions(): Promise { + if (!this.#isEnabled()) { + return; + } + + const accounts = this.messenger.call( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + ); + + const { + accounts: accountQueries, + accountIds, + networks, + } = buildDeFiBalancesQuery(accounts); + + if (accountIds.length === 0 || networks.length === 0) { + return; + } + + // The v6 response echoes the CAIP-10 IDs we sent; map them back to the + // internal account IDs used to key state. + const internalAccountIdByCaip = new Map( + accountQueries.map((account) => [ + normalizeCaipAccountId(account.caipAccountId), + account.internalAccountId, + ]), + ); + const resolveAccountId = (responseAccountId: string): string => + internalAccountIdByCaip.get(normalizeCaipAccountId(responseAccountId)) ?? + responseAccountId; + + const throttleKey = [...accountIds].sort().join(','); + const now = Date.now(); + const lastFetchedAt = this.#lastFetchByKey.get(throttleKey); + if ( + lastFetchedAt !== undefined && + now - lastFetchedAt < this.#minimumFetchIntervalMs + ) { + return; + } + // Mark before awaiting so concurrent calls within the window are throttled. + this.#lastFetchByKey.set(throttleKey, now); + + try { + const response = await this.#apiClient.accounts.fetchV6MultiAccountBalances( + accountIds, + { + networks, + includeDeFiBalances: + DEFI_BALANCES_V6_REQUEST_OPTIONS.includeDeFiBalances, + forceFetchDeFiPositions: + DEFI_BALANCES_V6_REQUEST_OPTIONS.forceFetchDeFiPositions, + includePrices: DEFI_BALANCES_V6_REQUEST_OPTIONS.includePrices, + vsCurrency: this.#getVsCurrency().toLowerCase(), + }, + ); + + const positionsByAccount = groupDeFiPositionsV6( + response, + resolveAccountId, + ); + + this.update((state) => { + for (const [accountId, positions] of Object.entries( + positionsByAccount, + )) { + state.allDeFiPositions[accountId] = positions; + } + }); + } catch (error) { + // Allow a retry before the interval elapses when a fetch fails. + this.#lastFetchByKey.delete(throttleKey); + console.error('Failed to fetch DeFi positions', error); + } + + // TODO: The previous controller emitted position-count analytics via a + // `trackEvent` hook (see calculate-defi-metrics). Deliberately dropped here; + // confirm with the analytics owners what metrics V2 needs before re-adding. + } +} diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts new file mode 100644 index 00000000000..8ff42b9d958 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -0,0 +1,141 @@ +import { isEvmAccountType, SolAccountType, SolScope } from '@metamask/keyring-api'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; +import type { CaipAccountId, CaipChainId } from '@metamask/utils'; +import { KnownCaipNamespace, toCaipAccountId } from '@metamask/utils'; + +/** + * Networks the DeFi balances (v6 multiaccount) endpoint supports. + */ +export const DEFI_SUPPORTED_NETWORKS = [ + 'eip155:1', + 'eip155:137', + 'eip155:56', + 'eip155:1329', + 'eip155:43114', + 'eip155:59144', + 'eip155:8453', + 'eip155:10', + 'eip155:42161', + 'eip155:143', + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + 'eip155:999', + 'eip155:5042', +] as const satisfies readonly CaipChainId[]; + +const SOLANA_MAINNET_CAIP_CHAIN_ID = SolScope.Mainnet as CaipChainId; + +/** + * Fixed request flags used against the v6 multiaccount balances endpoint so the + * response only carries what the DeFi views need (positions + prices). + */ +export const DEFI_BALANCES_V6_REQUEST_OPTIONS = { + includeDeFiBalances: true, + forceFetchDeFiPositions: true, + includePrices: true, + vsCurrency: 'usd', +} as const; + +/** + * A single account to query, pairing the CAIP-10 ID sent to the API with the + * internal MetaMask account ID (`InternalAccount.id`) used to key state. + */ +export type DeFiBalanceAccountQuery = { + caipAccountId: CaipAccountId; + internalAccountId: string; +}; + +export type DeFiBalancesQuery = { + /** Per-account entries linking CAIP-10 IDs to internal account IDs. */ + accounts: DeFiBalanceAccountQuery[]; + /** CAIP-10 account IDs to query (EVM and/or Solana). */ + accountIds: CaipAccountId[]; + /** CAIP-2 networks to query, deduped across accounts. */ + networks: CaipChainId[]; +}; + +/** + * Builds an EVM CAIP-10 account ID that spans every EVM chain (reference `0`). + * + * @param address - The EVM account address. + * @returns The CAIP-10 account ID for the address. + */ +function toEvmCaipAccountId(address: string): CaipAccountId { + return toCaipAccountId(KnownCaipNamespace.Eip155, '0', address); +} + +/** + * Normalizes a CAIP-10 account ID for case-insensitive matching. EVM addresses + * are case-insensitive, so `eip155:*` IDs are lowercased; other namespaces + * (e.g. Solana, whose base58 addresses are case-sensitive) are left as-is. Used + * to match the CAIP IDs the v6 API echoes back to the ones we sent. + * + * @param caipAccountId - The CAIP-10 account ID. + * @returns The normalized account ID. + */ +export function normalizeCaipAccountId(caipAccountId: string): string { + return caipAccountId.startsWith(`${KnownCaipNamespace.Eip155}:`) + ? caipAccountId.toLowerCase() + : caipAccountId; +} + +/** + * Builds the account IDs and networks to request DeFi positions for, from the + * accounts in the selected account group. + * + * Picks the group's EVM account (queried across all supported EVM chains) and + * its Solana account (queried on supported Solana chains). Enabled-network + * filtering is intentionally omitted here: positions are stored per chain, so + * the client can filter by enabled networks when reading state. + * + * @param internalAccounts - Accounts belonging to the selected account group. + * @param supportedNetworks - Networks supported by the DeFi balances API. + * @returns Account IDs and networks for the v6 multiaccount balances request. + */ +export function buildDeFiBalancesQuery( + internalAccounts: InternalAccount[], + supportedNetworks: readonly CaipChainId[] = DEFI_SUPPORTED_NETWORKS, +): DeFiBalancesQuery { + const evmNetworks = supportedNetworks.filter((network) => + network.startsWith(`${KnownCaipNamespace.Eip155}:`), + ); + const solanaNetworks = supportedNetworks.filter((network) => + network.startsWith(`${KnownCaipNamespace.Solana}:`), + ); + + const accounts: DeFiBalanceAccountQuery[] = []; + const networks: CaipChainId[] = []; + + const evmAccount = internalAccounts.find((account) => + isEvmAccountType(account.type), + ); + if (evmAccount && evmNetworks.length > 0) { + accounts.push({ + caipAccountId: toEvmCaipAccountId(evmAccount.address), + internalAccountId: evmAccount.id, + }); + networks.push(...evmNetworks); + } + + const solanaAccount = internalAccounts.find( + (account) => account.type === SolAccountType.DataAccount, + ); + if (solanaAccount && solanaNetworks.length > 0) { + const [, solanaReference] = SOLANA_MAINNET_CAIP_CHAIN_ID.split(':'); + + accounts.push({ + caipAccountId: toCaipAccountId( + KnownCaipNamespace.Solana, + solanaReference, + solanaAccount.address, + ), + internalAccountId: solanaAccount.id, + }); + networks.push(...solanaNetworks); + } + + return { + accounts, + accountIds: accounts.map((account) => account.caipAccountId), + networks: [...new Set(networks)] as CaipChainId[], + }; +} diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts new file mode 100644 index 00000000000..2c017076a35 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -0,0 +1,340 @@ +import type { + V6BalanceItem, + V6BalanceMetadata, + V6BalancesResponse, +} from '@metamask/core-backend'; +import type { CaipAssetType, CaipChainId } from '@metamask/utils'; +import { + KnownCaipNamespace, + parseCaipAssetType, + parseCaipChainId, +} from '@metamask/utils'; + +// TODO: The extension prototype derived token icons via +// `getCaipAssetImageUrl`/`getAssetImageUrl` (shared/lib/asset-utils). Core has +// no shared equivalent yet, so the minimal builder below is inlined. Replace it +// with a shared helper if/when one lands in core. +const STATIC_METAMASK_BASE_URL = 'https://static.cx.metamask.io'; + +/** + * An icon-group entry shown next to a protocol in the DeFi tab list. + */ +export type DeFiPositionIconGroupItem = { + avatarValue: string; + symbol: string; +}; + +/** + * A single underlying position row shown on the DeFi details page. + */ +export type DeFiUnderlyingPosition = { + assetId: CaipAssetType; + chainId: CaipChainId; + symbol: string; + name: string; + /** Raw balance string as returned by the API. */ + balance: string; + /** Parsed balance amount, or 0 when invalid. */ + normalizedBalance: number; + decimals: number; + /** Fiat market value in the requested currency. */ + marketValue: number; + /** Position type from protocol metadata (e.g. supply, borrow, stake, reward). */ + positionType: string; + poolAddress: string; + tokenImage: string; +}; + +/** + * A group of underlying positions that share a pool address. + */ +export type DeFiPositionPoolGroup = { + poolAddress: string; + positions: DeFiUnderlyingPosition[]; +}; + +/** + * A section of the details page, grouping pools by protocol name. + */ +export type DeFiPositionDetailsSection = { + protocolName: string; + poolGroups: DeFiPositionPoolGroup[]; +}; + +/** + * One row in the DeFi tab list (a protocol on a given chain), with the details + * needed to render the details page embedded directly inside it. + */ +export type DeFiProtocolPositionGroup = { + protocolId: string; + protocolName: string; + protocolIconUrl: string; + chainId: CaipChainId; + /** Aggregated fiat market value across all positions in the group. */ + marketValue: number; + /** Symbols of the underlying tokens, ordered for display. */ + underlyingSymbols: string[]; + /** Icon-group entries for the list row. */ + iconGroup: DeFiPositionIconGroupItem[]; + /** Detail sections consumed by the details page. */ + sections: DeFiPositionDetailsSection[]; +}; + +/** + * DeFi positions for every queried account, keyed by the internal MetaMask + * account ID (`InternalAccount.id` UUID), the same key AssetsController uses. + * Each account maps to a flat list of protocol groups; filter by each group's + * `chainId` rather than digging through a nested chain map. + */ +export type DeFiPositionsByAccount = { + [accountId: string]: DeFiProtocolPositionGroup[]; +}; + +const SYMBOL_PRIORITY = ['ETH', 'WETH']; + +type DefiBalanceWithMetadata = V6BalanceItem & { metadata: V6BalanceMetadata }; + +/** + * Builds a static token icon URL for a CAIP asset ID. + * + * @param assetId - The CAIP-19 asset ID. + * @returns The token icon URL, or an empty string when it cannot be built. + */ +function getDefiTokenImageUrl(assetId: CaipAssetType): string { + try { + const { chainId } = parseCaipAssetType(assetId); + const { namespace } = parseCaipChainId(chainId); + const isEvm = namespace === KnownCaipNamespace.Eip155; + const normalizedAssetId = (isEvm ? assetId.toLowerCase() : assetId).replace( + /:/gu, + '/', + ); + + return `${STATIC_METAMASK_BASE_URL}/api/v2/tokenIcons/assets/${normalizedAssetId}.png`; + } catch { + return ''; + } +} + +/** + * Returns whether a balance row is a DeFi position carrying protocol metadata. + * + * @param balance - A balance row from the v6 API. + * @returns True when the row is a `category: defi` row with protocol metadata. + */ +function isDefiBalanceWithMetadata( + balance: V6BalanceItem, +): balance is DefiBalanceWithMetadata { + return ( + balance.category === 'defi' && + balance.metadata !== undefined && + (balance.metadata as Partial).protocolId !== undefined + ); +} + +/** + * Returns the parsed balance amount for a v6 balance row. + * + * @param balance - A balance row from the v6 API. + * @returns The parsed balance, or 0 when invalid. + */ +function getNormalizedBalance(balance: V6BalanceItem): number { + const normalizedBalance = Number.parseFloat(balance.balance); + + return Number.isFinite(normalizedBalance) ? normalizedBalance : 0; +} + +/** + * Returns the fiat market value for a v6 DeFi balance row. + * + * @param balance - A balance row from the v6 API. + * @returns The fiat value, or 0 when unavailable. + */ +function getMarketValue(balance: V6BalanceItem): number { + const normalizedBalance = getNormalizedBalance(balance); + const price = Number.parseFloat(balance.price ?? '0'); + + if (!Number.isFinite(price)) { + return 0; + } + + return normalizedBalance * price; +} + +/** + * Moves a priority symbol (ETH/WETH) to the front of the icon group. + * + * @param iconGroup - The icon-group entries to order. + * @returns The ordered icon-group entries. + */ +function orderIconGroup( + iconGroup: DeFiPositionIconGroupItem[], +): DeFiPositionIconGroupItem[] { + const orderedIcons = [...iconGroup]; + const priorityIndex = orderedIcons.findIndex((item) => + SYMBOL_PRIORITY.includes(item.symbol), + ); + + if (priorityIndex > 0) { + const [priorityIcon] = orderedIcons.splice(priorityIndex, 1); + orderedIcons.unshift(priorityIcon); + } + + return orderedIcons; +} + +/** + * Maps a DeFi balance row to a details-page underlying position. + * + * @param balance - A DeFi balance row with protocol metadata. + * @returns The underlying position for the details page. + */ +function toUnderlyingPosition( + balance: DefiBalanceWithMetadata, +): DeFiUnderlyingPosition { + const assetId = balance.assetId as CaipAssetType; + const { chainId } = parseCaipAssetType(assetId); + const { positionType, poolAddress, protocolIconUrl } = balance.metadata; + + return { + assetId, + chainId, + symbol: balance.symbol, + name: balance.name, + balance: balance.balance, + normalizedBalance: getNormalizedBalance(balance), + decimals: balance.decimals, + marketValue: getMarketValue(balance), + positionType, + poolAddress, + tokenImage: getDefiTokenImageUrl(assetId) || protocolIconUrl, + }; +} + +/** + * Mutable accumulator used while grouping a single protocol's positions. + */ +type MutableProtocolGroup = { + protocolId: string; + protocolName: string; + protocolIconUrl: string; + chainId: CaipChainId; + marketValue: number; + /** Underlying token symbol -> icon-group entry, deduped by symbol. */ + iconBySymbol: Map; + /** protocolName -> (poolAddress -> positions), for details-page sections. */ + sectionByName: Map>; +}; + +/** + * Finalizes a mutable protocol group into its stored, client-ready shape. + * + * @param group - The accumulated protocol group. + * @returns The finalized protocol position group. + */ +function finalizeGroup( + group: MutableProtocolGroup, +): DeFiProtocolPositionGroup { + const iconGroup = orderIconGroup([...group.iconBySymbol.values()]); + + const sections: DeFiPositionDetailsSection[] = [ + ...group.sectionByName.entries(), + ].map(([protocolName, poolGroups]) => ({ + protocolName, + poolGroups: [...poolGroups.entries()].map(([poolAddress, positions]) => ({ + poolAddress, + positions, + })), + })); + + return { + protocolId: group.protocolId, + protocolName: group.protocolName, + protocolIconUrl: group.protocolIconUrl, + chainId: group.chainId, + marketValue: group.marketValue, + underlyingSymbols: iconGroup.map(({ symbol }) => symbol), + iconGroup, + sections, + }; +} + +/** + * Transforms a v6 multiaccount balances response into the stored DeFi state: + * positions keyed by internal account ID, each mapping to a flat list of + * protocol groups. Every group carries its own `chainId` (so the client can + * filter without a nested chain map) plus both the DeFi-tab summary and the + * details-page sections. Accounts present in the response but with no DeFi + * positions are included with an empty list so stale data is cleared. + * + * The v6 response keys accounts by the CAIP-10 ID sent to the API, so + * `resolveAccountId` maps that back to the internal MetaMask account ID used to + * key state. It defaults to the identity function (leaving the response ID) for + * callers that don't need the mapping (e.g. tests). + * + * @param response - The v6 multiaccount balances response. + * @param resolveAccountId - Maps a response (CAIP-10) account ID to the internal account ID. + * @returns DeFi positions keyed by internal account ID and chain. + */ +export function groupDeFiPositionsV6( + response: V6BalancesResponse, + resolveAccountId: (responseAccountId: string) => string = (id) => id, +): DeFiPositionsByAccount { + const result: DeFiPositionsByAccount = {}; + + for (const account of response.accounts) { + // Seed every queried account so accounts that no longer hold positions + // overwrite (clear) any previously stored data. + const groupsByKey = new Map(); + + for (const balance of account.balances) { + if (!isDefiBalanceWithMetadata(balance)) { + continue; + } + + const assetId = balance.assetId as CaipAssetType; + const { chainId } = parseCaipAssetType(assetId); + const { protocolId, protocolName, protocolIconUrl, poolAddress } = + balance.metadata; + const groupKey = `${chainId}#${protocolId}`; + const marketValue = getMarketValue(balance); + const iconEntry: DeFiPositionIconGroupItem = { + symbol: balance.symbol, + avatarValue: getDefiTokenImageUrl(assetId), + }; + const position = toUnderlyingPosition(balance); + + let group = groupsByKey.get(groupKey); + if (!group) { + group = { + protocolId, + protocolName, + protocolIconUrl, + chainId, + marketValue: 0, + iconBySymbol: new Map(), + sectionByName: new Map(), + }; + groupsByKey.set(groupKey, group); + } + + group.marketValue += marketValue; + group.iconBySymbol.set(balance.symbol, iconEntry); + + let poolGroups = group.sectionByName.get(protocolName); + if (!poolGroups) { + poolGroups = new Map(); + group.sectionByName.set(protocolName, poolGroups); + } + const poolPositions = poolGroups.get(poolAddress) ?? []; + poolPositions.push(position); + poolGroups.set(poolAddress, poolPositions); + } + + result[resolveAccountId(account.accountId)] = [ + ...groupsByKey.values(), + ].map(finalizeGroup); + } + + return result; +} diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 70cadfb0156..6f446820409 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -264,6 +264,29 @@ export type { DeFiPositionsControllerMessenger, } from './DeFiPositionsController/DeFiPositionsController'; export type { GroupedDeFiPositions } from './DeFiPositionsController/group-defi-positions'; +export { + DeFiPositionsControllerV2, + getDefaultDeFiPositionsControllerV2State, +} from './DeFiPositionsController/DeFiPositionsControllerV2'; +export type { + DeFiPositionsControllerV2State, + DeFiPositionsControllerV2Actions, + DeFiPositionsControllerV2Events, + DeFiPositionsControllerV2GetStateAction, + DeFiPositionsControllerV2StateChangedEvent, + DeFiPositionsControllerV2Messenger, + DeFiPositionsByAccount, + DeFiProtocolPositionGroup, + DeFiPositionDetailsSection, + DeFiPositionPoolGroup, + DeFiUnderlyingPosition, + DeFiPositionIconGroupItem, +} from './DeFiPositionsController/DeFiPositionsControllerV2'; +export type { DeFiPositionsControllerV2FetchDeFiPositionsAction } from './DeFiPositionsController/DeFiPositionsControllerV2-method-action-types'; +export { + DEFI_SUPPORTED_NETWORKS, + buildDeFiBalancesQuery, +} from './DeFiPositionsController/build-defi-balances-query'; export type { AccountGroupBalance, WalletBalance, From e2afb4d3835bbd5d406fadd83ff7f436dc369624 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 14:08:18 +0100 Subject: [PATCH 02/36] move file --- .../src/DeFiPositionsController/defi-visualizer.html | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename defi-visualizer.html => packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html (100%) diff --git a/defi-visualizer.html b/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html similarity index 100% rename from defi-visualizer.html rename to packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html From 7e679831d1d4d28aa6735e74100346708a208d06 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 14:18:56 +0100 Subject: [PATCH 03/36] linting --- .../DeFiPositionsControllerV2.ts | 10 +- .../build-defi-balances-query.ts | 6 +- .../defi-visualizer.html | 1691 ++++++++++------- .../group-defi-positions-v6.ts | 10 +- 4 files changed, 994 insertions(+), 723 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index d5a50994b0b..37be994d218 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -13,9 +13,9 @@ import { DEFI_BALANCES_V6_REQUEST_OPTIONS, normalizeCaipAccountId, } from './build-defi-balances-query'; +import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; import type { DeFiPositionsByAccount } from './group-defi-positions-v6'; import { groupDeFiPositionsV6 } from './group-defi-positions-v6'; -import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; const controllerName = 'DeFiPositionsControllerV2'; @@ -224,9 +224,8 @@ export class DeFiPositionsControllerV2 extends BaseController< this.#lastFetchByKey.set(throttleKey, now); try { - const response = await this.#apiClient.accounts.fetchV6MultiAccountBalances( - accountIds, - { + const response = + await this.#apiClient.accounts.fetchV6MultiAccountBalances(accountIds, { networks, includeDeFiBalances: DEFI_BALANCES_V6_REQUEST_OPTIONS.includeDeFiBalances, @@ -234,8 +233,7 @@ export class DeFiPositionsControllerV2 extends BaseController< DEFI_BALANCES_V6_REQUEST_OPTIONS.forceFetchDeFiPositions, includePrices: DEFI_BALANCES_V6_REQUEST_OPTIONS.includePrices, vsCurrency: this.#getVsCurrency().toLowerCase(), - }, - ); + }); const positionsByAccount = groupDeFiPositionsV6( response, diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index 8ff42b9d958..9eab44e6fed 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -1,4 +1,8 @@ -import { isEvmAccountType, SolAccountType, SolScope } from '@metamask/keyring-api'; +import { + isEvmAccountType, + SolAccountType, + SolScope, +} from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { CaipAccountId, CaipChainId } from '@metamask/utils'; import { KnownCaipNamespace, toCaipAccountId } from '@metamask/utils'; diff --git a/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html b/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html index 82611c6284b..20eca47f1cd 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html +++ b/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html @@ -1,714 +1,985 @@ - + - - - -DeFi Balances Visualizer - - - -
-

DeFi Balances Visualizer

-

MetaMask multiaccount balances API · DeFi positions across 7 EVM chains

-
- - - -
-
Fetches Mainnet, Linea, Base, Arbitrum, BNB Chain, Optimism & Polygon. Only defi balances are shown; all data is preserved in the raw response.
-
- -
- - - -
- -
- - - + })(); + + diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index 2c017076a35..7c35fd80964 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -232,9 +232,7 @@ type MutableProtocolGroup = { * @param group - The accumulated protocol group. * @returns The finalized protocol position group. */ -function finalizeGroup( - group: MutableProtocolGroup, -): DeFiProtocolPositionGroup { +function finalizeGroup(group: MutableProtocolGroup): DeFiProtocolPositionGroup { const iconGroup = orderIconGroup([...group.iconBySymbol.values()]); const sections: DeFiPositionDetailsSection[] = [ @@ -331,9 +329,9 @@ export function groupDeFiPositionsV6( poolGroups.set(poolAddress, poolPositions); } - result[resolveAccountId(account.accountId)] = [ - ...groupsByKey.values(), - ].map(finalizeGroup); + result[resolveAccountId(account.accountId)] = [...groupsByKey.values()].map( + finalizeGroup, + ); } return result; From 052b1389a94fa3a3740e3fcf1347332e386a3483 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 15:17:50 +0100 Subject: [PATCH 04/36] progress --- .../DeFiPositionsControllerV2.ts | 66 +++++++++-------- .../build-defi-balances-query.ts | 71 +++++++++---------- .../group-defi-positions-v6.ts | 21 ++++-- 3 files changed, 83 insertions(+), 75 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 37be994d218..948080ad83e 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -119,9 +119,16 @@ export class DeFiPositionsControllerV2 extends BaseController< readonly #minimumFetchIntervalMs: number; /** - * In-memory timestamp (ms) of the last fetch per query, keyed by the sorted - * account IDs. Intentionally not persisted: it resets on restart, so the - * first fetch after a restart always goes through. + * In-memory timestamp (ms) of the last fetch per set of accounts. + * + * This is a controller-level gate, separate from TanStack's `staleTime` inside + * `fetchV6MultiAccountBalances`: when the interval has not elapsed we + * early-return without regrouping or writing state. TanStack still dedupes + * in-flight HTTP for identical query keys; this Map skips that work entirely. + * + * Keyed by sorted CAIP account IDs only (not networks / vsCurrency). + * Intentionally not persisted: resets on restart, so the first fetch after a + * restart always goes through. */ readonly #lastFetchByKey = new Map(); @@ -137,15 +144,15 @@ export class DeFiPositionsControllerV2 extends BaseController< constructor({ messenger, apiClient, - isEnabled = (): boolean => false, - getVsCurrency = (): string => DEFI_BALANCES_V6_REQUEST_OPTIONS.vsCurrency, + isEnabled, + getVsCurrency, minimumFetchIntervalMs = ONE_MINUTE_IN_MS, state, }: { messenger: DeFiPositionsControllerV2Messenger; apiClient: ApiPlatformClient; - isEnabled?: () => boolean; - getVsCurrency?: () => string; + isEnabled: () => boolean; + getVsCurrency: () => string; minimumFetchIntervalMs?: number; state?: Partial; }) { @@ -177,40 +184,28 @@ export class DeFiPositionsControllerV2 extends BaseController< * response, and updating state. * * Throttled per set of accounts by an in-memory minimum interval, so repeated - * calls within the window are no-ops. Disabled controllers and empty account - * groups return without fetching. + * calls within the window are no-ops (no HTTP, no regroup, no state write). + * Disabled controllers and empty account groups return without fetching. */ async fetchDeFiPositions(): Promise { if (!this.#isEnabled()) { return; } - const accounts = this.messenger.call( + const selectedAccounts = this.messenger.call( 'AccountTreeController:getAccountsFromSelectedAccountGroup', ); - const { - accounts: accountQueries, - accountIds, - networks, - } = buildDeFiBalancesQuery(accounts); + const { networks, internalAccountIdByCaip } = + buildDeFiBalancesQuery(selectedAccounts); - if (accountIds.length === 0 || networks.length === 0) { + if (internalAccountIdByCaip.size === 0 || networks.length === 0) { return; } - // The v6 response echoes the CAIP-10 IDs we sent; map them back to the - // internal account IDs used to key state. - const internalAccountIdByCaip = new Map( - accountQueries.map((account) => [ - normalizeCaipAccountId(account.caipAccountId), - account.internalAccountId, - ]), - ); - const resolveAccountId = (responseAccountId: string): string => - internalAccountIdByCaip.get(normalizeCaipAccountId(responseAccountId)) ?? - responseAccountId; - + const accountIds = [...internalAccountIdByCaip.keys()]; + // Stable key so the same account set throttles together regardless of map + // iteration order. const throttleKey = [...accountIds].sort().join(','); const now = Date.now(); const lastFetchedAt = this.#lastFetchByKey.get(throttleKey); @@ -220,7 +215,9 @@ export class DeFiPositionsControllerV2 extends BaseController< ) { return; } - // Mark before awaiting so concurrent calls within the window are throttled. + // Claim the slot before awaiting so a second call that arrives while the + // first is in flight is also dropped (TanStack would share that promise; + // we intentionally skip instead). this.#lastFetchByKey.set(throttleKey, now); try { @@ -235,6 +232,13 @@ export class DeFiPositionsControllerV2 extends BaseController< vsCurrency: this.#getVsCurrency().toLowerCase(), }); + // The v6 response echoes the CAIP-10 IDs we sent; map them back to the + // internal account IDs used to key state. Unmatched accounts are skipped. + const resolveAccountId = ( + responseAccountId: string, + ): string | undefined => + internalAccountIdByCaip.get(normalizeCaipAccountId(responseAccountId)); + const positionsByAccount = groupDeFiPositionsV6( response, resolveAccountId, @@ -248,13 +252,13 @@ export class DeFiPositionsControllerV2 extends BaseController< } }); } catch (error) { - // Allow a retry before the interval elapses when a fetch fails. + // Clear the claim so a failed fetch does not burn the throttle window. this.#lastFetchByKey.delete(throttleKey); console.error('Failed to fetch DeFi positions', error); } // TODO: The previous controller emitted position-count analytics via a // `trackEvent` hook (see calculate-defi-metrics). Deliberately dropped here; - // confirm with the analytics owners what metrics V2 needs before re-adding. + // confirm what analytics will be needed before re-adding. } } diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index 9eab44e6fed..1c303b3c3ea 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -9,8 +9,11 @@ import { KnownCaipNamespace, toCaipAccountId } from '@metamask/utils'; /** * Networks the DeFi balances (v6 multiaccount) endpoint supports. + * Cross-section of the supported chains from: + * https://developers.zerion.io/supported-blockchains + * https://accounts.api.cx.metamask.io/v2/supportedNetworks */ -export const DEFI_SUPPORTED_NETWORKS = [ +export const DEFI_SUPPORTED_NETWORKS: readonly CaipChainId[] = [ 'eip155:1', 'eip155:137', 'eip155:56', @@ -24,37 +27,32 @@ export const DEFI_SUPPORTED_NETWORKS = [ 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', 'eip155:999', 'eip155:5042', -] as const satisfies readonly CaipChainId[]; +]; -const SOLANA_MAINNET_CAIP_CHAIN_ID = SolScope.Mainnet as CaipChainId; +const SOLANA_MAINNET_CAIP_CHAIN_ID: CaipChainId = SolScope.Mainnet; /** * Fixed request flags used against the v6 multiaccount balances endpoint so the * response only carries what the DeFi views need (positions + prices). + * Fiat currency (`vsCurrency`) is not fixed here — it comes from + * AssetsController `selectedCurrency` at fetch time. */ export const DEFI_BALANCES_V6_REQUEST_OPTIONS = { includeDeFiBalances: true, forceFetchDeFiPositions: true, includePrices: true, - vsCurrency: 'usd', } as const; -/** - * A single account to query, pairing the CAIP-10 ID sent to the API with the - * internal MetaMask account ID (`InternalAccount.id`) used to key state. - */ -export type DeFiBalanceAccountQuery = { - caipAccountId: CaipAccountId; - internalAccountId: string; -}; - export type DeFiBalancesQuery = { - /** Per-account entries linking CAIP-10 IDs to internal account IDs. */ - accounts: DeFiBalanceAccountQuery[]; - /** CAIP-10 account IDs to query (EVM and/or Solana). */ - accountIds: CaipAccountId[]; /** CAIP-2 networks to query, deduped across accounts. */ networks: CaipChainId[]; + /** + * Normalized CAIP-10 account IDs → internal MetaMask account IDs + * (`InternalAccount.id`). Keys are normalized via + * {@link normalizeCaipAccountId} so they can be sent as the v6 request + * account IDs and used to match response IDs case-insensitively for EVM. + */ + internalAccountIdByCaip: Map; }; /** @@ -92,31 +90,30 @@ export function normalizeCaipAccountId(caipAccountId: string): string { * the client can filter by enabled networks when reading state. * * @param internalAccounts - Accounts belonging to the selected account group. - * @param supportedNetworks - Networks supported by the DeFi balances API. - * @returns Account IDs and networks for the v6 multiaccount balances request. + * @returns Networks and a CAIP→internal account ID map for the v6 multiaccount + * balances request. Map keys are the CAIP account IDs to query. */ export function buildDeFiBalancesQuery( internalAccounts: InternalAccount[], - supportedNetworks: readonly CaipChainId[] = DEFI_SUPPORTED_NETWORKS, ): DeFiBalancesQuery { - const evmNetworks = supportedNetworks.filter((network) => + const evmNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => network.startsWith(`${KnownCaipNamespace.Eip155}:`), ); - const solanaNetworks = supportedNetworks.filter((network) => + const solanaNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => network.startsWith(`${KnownCaipNamespace.Solana}:`), ); - const accounts: DeFiBalanceAccountQuery[] = []; const networks: CaipChainId[] = []; + const internalAccountIdByCaip = new Map(); const evmAccount = internalAccounts.find((account) => isEvmAccountType(account.type), ); if (evmAccount && evmNetworks.length > 0) { - accounts.push({ - caipAccountId: toEvmCaipAccountId(evmAccount.address), - internalAccountId: evmAccount.id, - }); + internalAccountIdByCaip.set( + normalizeCaipAccountId(toEvmCaipAccountId(evmAccount.address)), + evmAccount.id, + ); networks.push(...evmNetworks); } @@ -125,21 +122,21 @@ export function buildDeFiBalancesQuery( ); if (solanaAccount && solanaNetworks.length > 0) { const [, solanaReference] = SOLANA_MAINNET_CAIP_CHAIN_ID.split(':'); - - accounts.push({ - caipAccountId: toCaipAccountId( - KnownCaipNamespace.Solana, - solanaReference, - solanaAccount.address, + internalAccountIdByCaip.set( + normalizeCaipAccountId( + toCaipAccountId( + KnownCaipNamespace.Solana, + solanaReference, + solanaAccount.address, + ), ), - internalAccountId: solanaAccount.id, - }); + solanaAccount.id, + ); networks.push(...solanaNetworks); } return { - accounts, - accountIds: accounts.map((account) => account.caipAccountId), networks: [...new Set(networks)] as CaipChainId[], + internalAccountIdByCaip, }; } diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index 7c35fd80964..b09fe30b106 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -267,20 +267,29 @@ function finalizeGroup(group: MutableProtocolGroup): DeFiProtocolPositionGroup { * * The v6 response keys accounts by the CAIP-10 ID sent to the API, so * `resolveAccountId` maps that back to the internal MetaMask account ID used to - * key state. It defaults to the identity function (leaving the response ID) for - * callers that don't need the mapping (e.g. tests). + * key state. Accounts that do not resolve are skipped. It defaults to the + * identity function (leaving the response ID) for callers that don't need the + * mapping (e.g. tests). * * @param response - The v6 multiaccount balances response. - * @param resolveAccountId - Maps a response (CAIP-10) account ID to the internal account ID. + * @param resolveAccountId - Maps a response (CAIP-10) account ID to the internal + * account ID, or `undefined` to skip the account. * @returns DeFi positions keyed by internal account ID and chain. */ export function groupDeFiPositionsV6( response: V6BalancesResponse, - resolveAccountId: (responseAccountId: string) => string = (id) => id, + resolveAccountId: ( + responseAccountId: string, + ) => string | undefined = (id) => id, ): DeFiPositionsByAccount { const result: DeFiPositionsByAccount = {}; for (const account of response.accounts) { + const accountId = resolveAccountId(account.accountId); + if (accountId === undefined) { + continue; + } + // Seed every queried account so accounts that no longer hold positions // overwrite (clear) any previously stored data. const groupsByKey = new Map(); @@ -329,9 +338,7 @@ export function groupDeFiPositionsV6( poolGroups.set(poolAddress, poolPositions); } - result[resolveAccountId(account.accountId)] = [...groupsByKey.values()].map( - finalizeGroup, - ); + result[accountId] = [...groupsByKey.values()].map(finalizeGroup); } return result; From fa243223a92ac0d1204a43f227f42f46fec3339f Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 15:20:50 +0100 Subject: [PATCH 05/36] linting --- .../src/DeFiPositionsController/group-defi-positions-v6.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index b09fe30b106..3c63dcdd2fa 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -278,9 +278,8 @@ function finalizeGroup(group: MutableProtocolGroup): DeFiProtocolPositionGroup { */ export function groupDeFiPositionsV6( response: V6BalancesResponse, - resolveAccountId: ( - responseAccountId: string, - ) => string | undefined = (id) => id, + resolveAccountId: (responseAccountId: string) => string | undefined = (id) => + id, ): DeFiPositionsByAccount { const result: DeFiPositionsByAccount = {}; From 4e3104f4a3fed51884f6cdefaf9bcc9950cbad1e Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 15:50:13 +0100 Subject: [PATCH 06/36] refactors --- .../DeFiPositionsControllerV2.ts | 1 - .../group-defi-positions-v6.ts | 101 +++++++----------- packages/assets-controllers/src/index.ts | 1 - 3 files changed, 39 insertions(+), 64 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 948080ad83e..3913b22fa46 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -27,7 +27,6 @@ export type { DeFiPositionsByAccount, DeFiProtocolPositionGroup, DeFiPositionDetailsSection, - DeFiPositionPoolGroup, DeFiUnderlyingPosition, DeFiPositionIconGroupItem, } from './group-defi-positions-v6'; diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index 3c63dcdd2fa..1cb7c530821 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -20,7 +20,8 @@ const STATIC_METAMASK_BASE_URL = 'https://static.cx.metamask.io'; * An icon-group entry shown next to a protocol in the DeFi tab list. */ export type DeFiPositionIconGroupItem = { - avatarValue: string; + /** Token icon URL, when one can be built for the asset. */ + avatarValue?: string; symbol: string; }; @@ -32,33 +33,25 @@ export type DeFiUnderlyingPosition = { chainId: CaipChainId; symbol: string; name: string; + decimals: number; /** Raw balance string as returned by the API. */ balance: string; - /** Parsed balance amount, or 0 when invalid. */ - normalizedBalance: number; - decimals: number; - /** Fiat market value in the requested currency. */ - marketValue: number; + /** Fiat market value in the requested currency, when a price is available. */ + marketValue?: number; /** Position type from protocol metadata (e.g. supply, borrow, stake, reward). */ positionType: string; + /** Address of the pool this position belongs to. */ poolAddress: string; - tokenImage: string; -}; - -/** - * A group of underlying positions that share a pool address. - */ -export type DeFiPositionPoolGroup = { - poolAddress: string; - positions: DeFiUnderlyingPosition[]; + /** Token icon URL, when one can be built for the asset. */ + tokenImage?: string; }; /** - * A section of the details page, grouping pools by protocol name. + * A section of the details page, grouping positions by protocol name. */ export type DeFiPositionDetailsSection = { protocolName: string; - poolGroups: DeFiPositionPoolGroup[]; + positions: DeFiUnderlyingPosition[]; }; /** @@ -72,8 +65,6 @@ export type DeFiProtocolPositionGroup = { chainId: CaipChainId; /** Aggregated fiat market value across all positions in the group. */ marketValue: number; - /** Symbols of the underlying tokens, ordered for display. */ - underlyingSymbols: string[]; /** Icon-group entries for the list row. */ iconGroup: DeFiPositionIconGroupItem[]; /** Detail sections consumed by the details page. */ @@ -90,6 +81,8 @@ export type DeFiPositionsByAccount = { [accountId: string]: DeFiProtocolPositionGroup[]; }; +// Prefer ETH/WETH first in the list-row icon stack when a protocol has multiple +// underlyings. Display-only. const SYMBOL_PRIORITY = ['ETH', 'WETH']; type DefiBalanceWithMetadata = V6BalanceItem & { metadata: V6BalanceMetadata }; @@ -98,9 +91,9 @@ type DefiBalanceWithMetadata = V6BalanceItem & { metadata: V6BalanceMetadata }; * Builds a static token icon URL for a CAIP asset ID. * * @param assetId - The CAIP-19 asset ID. - * @returns The token icon URL, or an empty string when it cannot be built. + * @returns The token icon URL, or `undefined` when it cannot be built. */ -function getDefiTokenImageUrl(assetId: CaipAssetType): string { +function getDefiTokenImageUrl(assetId: CaipAssetType): string | undefined { try { const { chainId } = parseCaipAssetType(assetId); const { namespace } = parseCaipChainId(chainId); @@ -112,7 +105,7 @@ function getDefiTokenImageUrl(assetId: CaipAssetType): string { return `${STATIC_METAMASK_BASE_URL}/api/v2/tokenIcons/assets/${normalizedAssetId}.png`; } catch { - return ''; + return undefined; } } @@ -132,30 +125,23 @@ function isDefiBalanceWithMetadata( ); } -/** - * Returns the parsed balance amount for a v6 balance row. - * - * @param balance - A balance row from the v6 API. - * @returns The parsed balance, or 0 when invalid. - */ -function getNormalizedBalance(balance: V6BalanceItem): number { - const normalizedBalance = Number.parseFloat(balance.balance); - - return Number.isFinite(normalizedBalance) ? normalizedBalance : 0; -} - /** * Returns the fiat market value for a v6 DeFi balance row. * * @param balance - A balance row from the v6 API. - * @returns The fiat value, or 0 when unavailable. + * @returns The fiat value, or `undefined` when price is missing or the + * balance/price is invalid. */ -function getMarketValue(balance: V6BalanceItem): number { - const normalizedBalance = getNormalizedBalance(balance); - const price = Number.parseFloat(balance.price ?? '0'); +function getMarketValue(balance: V6BalanceItem): number | undefined { + if (balance.price === undefined) { + return undefined; + } + + const normalizedBalance = Number.parseFloat(balance.balance); + const price = Number.parseFloat(balance.price); - if (!Number.isFinite(price)) { - return 0; + if (!Number.isFinite(normalizedBalance) || !Number.isFinite(price)) { + return undefined; } return normalizedBalance * price; @@ -194,7 +180,7 @@ function toUnderlyingPosition( ): DeFiUnderlyingPosition { const assetId = balance.assetId as CaipAssetType; const { chainId } = parseCaipAssetType(assetId); - const { positionType, poolAddress, protocolIconUrl } = balance.metadata; + const { positionType, poolAddress } = balance.metadata; return { assetId, @@ -202,12 +188,11 @@ function toUnderlyingPosition( symbol: balance.symbol, name: balance.name, balance: balance.balance, - normalizedBalance: getNormalizedBalance(balance), decimals: balance.decimals, marketValue: getMarketValue(balance), positionType, poolAddress, - tokenImage: getDefiTokenImageUrl(assetId) || protocolIconUrl, + tokenImage: getDefiTokenImageUrl(assetId), }; } @@ -222,8 +207,8 @@ type MutableProtocolGroup = { marketValue: number; /** Underlying token symbol -> icon-group entry, deduped by symbol. */ iconBySymbol: Map; - /** protocolName -> (poolAddress -> positions), for details-page sections. */ - sectionByName: Map>; + /** protocolName -> positions, for details-page sections. */ + sectionByName: Map; }; /** @@ -237,12 +222,9 @@ function finalizeGroup(group: MutableProtocolGroup): DeFiProtocolPositionGroup { const sections: DeFiPositionDetailsSection[] = [ ...group.sectionByName.entries(), - ].map(([protocolName, poolGroups]) => ({ + ].map(([protocolName, positions]) => ({ protocolName, - poolGroups: [...poolGroups.entries()].map(([poolAddress, positions]) => ({ - poolAddress, - positions, - })), + positions, })); return { @@ -251,7 +233,6 @@ function finalizeGroup(group: MutableProtocolGroup): DeFiProtocolPositionGroup { protocolIconUrl: group.protocolIconUrl, chainId: group.chainId, marketValue: group.marketValue, - underlyingSymbols: iconGroup.map(({ symbol }) => symbol), iconGroup, sections, }; @@ -300,8 +281,7 @@ export function groupDeFiPositionsV6( const assetId = balance.assetId as CaipAssetType; const { chainId } = parseCaipAssetType(assetId); - const { protocolId, protocolName, protocolIconUrl, poolAddress } = - balance.metadata; + const { protocolId, protocolName, protocolIconUrl } = balance.metadata; const groupKey = `${chainId}#${protocolId}`; const marketValue = getMarketValue(balance); const iconEntry: DeFiPositionIconGroupItem = { @@ -324,17 +304,14 @@ export function groupDeFiPositionsV6( groupsByKey.set(groupKey, group); } - group.marketValue += marketValue; + if (marketValue !== undefined) { + group.marketValue += marketValue; + } group.iconBySymbol.set(balance.symbol, iconEntry); - let poolGroups = group.sectionByName.get(protocolName); - if (!poolGroups) { - poolGroups = new Map(); - group.sectionByName.set(protocolName, poolGroups); - } - const poolPositions = poolGroups.get(poolAddress) ?? []; - poolPositions.push(position); - poolGroups.set(poolAddress, poolPositions); + const sectionPositions = group.sectionByName.get(protocolName) ?? []; + sectionPositions.push(position); + group.sectionByName.set(protocolName, sectionPositions); } result[accountId] = [...groupsByKey.values()].map(finalizeGroup); diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 6f446820409..950c530f77c 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -278,7 +278,6 @@ export type { DeFiPositionsByAccount, DeFiProtocolPositionGroup, DeFiPositionDetailsSection, - DeFiPositionPoolGroup, DeFiUnderlyingPosition, DeFiPositionIconGroupItem, } from './DeFiPositionsController/DeFiPositionsControllerV2'; From f6479932447fc97bfef4bbcd5020dd1508a6d5ae Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 16:07:49 +0100 Subject: [PATCH 07/36] refactor for grouping --- .../group-defi-positions-v6.ts | 120 +++++++----------- 1 file changed, 44 insertions(+), 76 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index 1cb7c530821..9d190c2b9f5 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -47,9 +47,13 @@ export type DeFiUnderlyingPosition = { }; /** - * A section of the details page, grouping positions by protocol name. + * A section of the details page, grouping positions that share the same + * API `protocolName`. A single `protocolId` can have multiple names + * (different pools/products under one protocol), so a group may contain + * several sections. */ export type DeFiPositionDetailsSection = { + /** Section label from the API (`metadata.protocolName`). */ protocolName: string; positions: DeFiUnderlyingPosition[]; }; @@ -60,6 +64,7 @@ export type DeFiPositionDetailsSection = { */ export type DeFiProtocolPositionGroup = { protocolId: string; + /** Display name from the first position seen for this protocol. */ protocolName: string; protocolIconUrl: string; chainId: CaipChainId; @@ -67,7 +72,10 @@ export type DeFiProtocolPositionGroup = { marketValue: number; /** Icon-group entries for the list row. */ iconGroup: DeFiPositionIconGroupItem[]; - /** Detail sections consumed by the details page. */ + /** + * Detail sections consumed by the details page, one per distinct API + * `protocolName` under this `protocolId`. + */ sections: DeFiPositionDetailsSection[]; }; @@ -148,25 +156,19 @@ function getMarketValue(balance: V6BalanceItem): number | undefined { } /** - * Moves a priority symbol (ETH/WETH) to the front of the icon group. + * Moves a priority symbol (ETH/WETH) to the front of the icon group, in place. * - * @param iconGroup - The icon-group entries to order. - * @returns The ordered icon-group entries. + * @param iconGroup - The icon-group entries to reorder. */ -function orderIconGroup( - iconGroup: DeFiPositionIconGroupItem[], -): DeFiPositionIconGroupItem[] { - const orderedIcons = [...iconGroup]; - const priorityIndex = orderedIcons.findIndex((item) => +function orderIconGroup(iconGroup: DeFiPositionIconGroupItem[]): void { + const priorityIndex = iconGroup.findIndex((item) => SYMBOL_PRIORITY.includes(item.symbol), ); if (priorityIndex > 0) { - const [priorityIcon] = orderedIcons.splice(priorityIndex, 1); - orderedIcons.unshift(priorityIcon); + const [priorityIcon] = iconGroup.splice(priorityIndex, 1); + iconGroup.unshift(priorityIcon); } - - return orderedIcons; } /** @@ -196,48 +198,6 @@ function toUnderlyingPosition( }; } -/** - * Mutable accumulator used while grouping a single protocol's positions. - */ -type MutableProtocolGroup = { - protocolId: string; - protocolName: string; - protocolIconUrl: string; - chainId: CaipChainId; - marketValue: number; - /** Underlying token symbol -> icon-group entry, deduped by symbol. */ - iconBySymbol: Map; - /** protocolName -> positions, for details-page sections. */ - sectionByName: Map; -}; - -/** - * Finalizes a mutable protocol group into its stored, client-ready shape. - * - * @param group - The accumulated protocol group. - * @returns The finalized protocol position group. - */ -function finalizeGroup(group: MutableProtocolGroup): DeFiProtocolPositionGroup { - const iconGroup = orderIconGroup([...group.iconBySymbol.values()]); - - const sections: DeFiPositionDetailsSection[] = [ - ...group.sectionByName.entries(), - ].map(([protocolName, positions]) => ({ - protocolName, - positions, - })); - - return { - protocolId: group.protocolId, - protocolName: group.protocolName, - protocolIconUrl: group.protocolIconUrl, - chainId: group.chainId, - marketValue: group.marketValue, - iconGroup, - sections, - }; -} - /** * Transforms a v6 multiaccount balances response into the stored DeFi state: * positions keyed by internal account ID, each mapping to a flat list of @@ -272,23 +232,16 @@ export function groupDeFiPositionsV6( // Seed every queried account so accounts that no longer hold positions // overwrite (clear) any previously stored data. - const groupsByKey = new Map(); + const groupsByKey = new Map(); for (const balance of account.balances) { if (!isDefiBalanceWithMetadata(balance)) { continue; } - const assetId = balance.assetId as CaipAssetType; - const { chainId } = parseCaipAssetType(assetId); - const { protocolId, protocolName, protocolIconUrl } = balance.metadata; - const groupKey = `${chainId}#${protocolId}`; - const marketValue = getMarketValue(balance); - const iconEntry: DeFiPositionIconGroupItem = { - symbol: balance.symbol, - avatarValue: getDefiTokenImageUrl(assetId), - }; const position = toUnderlyingPosition(balance); + const { protocolId, protocolName, protocolIconUrl } = balance.metadata; + const groupKey = `${position.chainId}#${protocolId}`; let group = groupsByKey.get(groupKey); if (!group) { @@ -296,25 +249,40 @@ export function groupDeFiPositionsV6( protocolId, protocolName, protocolIconUrl, - chainId, + chainId: position.chainId, marketValue: 0, - iconBySymbol: new Map(), - sectionByName: new Map(), + iconGroup: [], + sections: [], }; groupsByKey.set(groupKey, group); } - if (marketValue !== undefined) { - group.marketValue += marketValue; + if (position.marketValue !== undefined) { + group.marketValue += position.marketValue; + } + + if (!group.iconGroup.some((item) => item.symbol === position.symbol)) { + group.iconGroup.push({ + symbol: position.symbol, + avatarValue: position.tokenImage, + }); } - group.iconBySymbol.set(balance.symbol, iconEntry); - const sectionPositions = group.sectionByName.get(protocolName) ?? []; - sectionPositions.push(position); - group.sectionByName.set(protocolName, sectionPositions); + let section = group.sections.find( + (item) => item.protocolName === protocolName, + ); + if (!section) { + section = { protocolName, positions: [] }; + group.sections.push(section); + } + section.positions.push(position); } - result[accountId] = [...groupsByKey.values()].map(finalizeGroup); + const groups = [...groupsByKey.values()]; + for (const group of groups) { + orderIconGroup(group.iconGroup); + } + result[accountId] = groups; } return result; From 9d7c4358f224a56fad1af26052897f3c41328662 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 16:26:06 +0100 Subject: [PATCH 08/36] rename state field --- .../DeFiPositionsControllerV2.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 3913b22fa46..080ddb2bcf4 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -39,12 +39,17 @@ export type DeFiPositionsControllerV2State = { * filtering plus the details-page sections embedded inside it. This is * exactly the shape the client consumes, so no further transformation is * needed on read. + * + * Named `allDeFiPositionsV2` (rather than `allDeFiPositions`) so it can live + * alongside the legacy `DeFiPositionsController` in clients that flatten every + * controller's state into a single object (e.g. the extension background), + * without colliding on the shared `allDeFiPositions` key. */ - allDeFiPositions: DeFiPositionsByAccount; + allDeFiPositionsV2: DeFiPositionsByAccount; }; const controllerMetadata: StateMetadata = { - allDeFiPositions: { + allDeFiPositionsV2: { includeInStateLogs: false, persist: true, includeInDebugSnapshot: false, @@ -55,7 +60,7 @@ const controllerMetadata: StateMetadata = { export const getDefaultDeFiPositionsControllerV2State = (): DeFiPositionsControllerV2State => { return { - allDeFiPositions: {}, + allDeFiPositionsV2: {}, }; }; @@ -247,7 +252,7 @@ export class DeFiPositionsControllerV2 extends BaseController< for (const [accountId, positions] of Object.entries( positionsByAccount, )) { - state.allDeFiPositions[accountId] = positions; + state.allDeFiPositionsV2[accountId] = positions; } }); } catch (error) { From 8a2695555aa2a558e505a60f1913765efc2ebf4f Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 16:30:20 +0100 Subject: [PATCH 09/36] tests --- .../DeFiPositionsControllerV2.test.ts | 482 ++++++++++++++++++ .../build-defi-balances-query.test.ts | 182 +++++++ .../group-defi-positions-v6.test.ts | 415 +++++++++++++++ 3 files changed, 1079 insertions(+) create mode 100644 packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts create mode 100644 packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts create mode 100644 packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts new file mode 100644 index 00000000000..6f999f40a57 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -0,0 +1,482 @@ +import { deriveStateFromMetadata } from '@metamask/base-controller'; +import type { ApiPlatformClient, V6BalancesResponse } from '@metamask/core-backend'; +import { + BtcAccountType, + EthAccountType, + SolAccountType, + SolMethod, + SolScope, +} from '@metamask/keyring-api'; +import { KeyringTypes } from '@metamask/keyring-controller'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; +import { MOCK_ANY_NAMESPACE, Messenger } from '@metamask/messenger'; +import type { + MessengerActions, + MessengerEvents, + MockAnyNamespace, +} from '@metamask/messenger'; + +import { createMockInternalAccount } from '../../../accounts-controller/tests/mocks'; +import { + DEFI_BALANCES_V6_REQUEST_OPTIONS, + DEFI_SUPPORTED_NETWORKS, +} from './build-defi-balances-query'; +import type { DeFiPositionsControllerV2Messenger } from './DeFiPositionsControllerV2'; +import { + DeFiPositionsControllerV2, + getDefaultDeFiPositionsControllerV2State, +} from './DeFiPositionsControllerV2'; + +const EVM_ADDRESS = '0x0000000000000000000000000000000000000001'; +const SOLANA_ADDRESS = 'So11111111111111111111111111111111111111112'; + +const GROUP_ACCOUNTS = [ + createMockInternalAccount({ + id: 'evm-account-id', + address: EVM_ADDRESS, + type: EthAccountType.Eoa, + }), + createMockInternalAccount({ + id: 'btc-account-id', + type: BtcAccountType.P2wpkh, + }), +]; + +const GROUP_ACCOUNTS_WITH_SOLANA: InternalAccount[] = [ + ...GROUP_ACCOUNTS, + { + id: 'solana-account-id', + address: SOLANA_ADDRESS, + options: {}, + methods: [SolMethod.SendAndConfirmTransaction], + scopes: [SolScope.Mainnet], + type: SolAccountType.DataAccount, + metadata: { + name: 'Solana Account', + keyring: { type: KeyringTypes.snap }, + importTime: Date.now(), + lastSelected: Date.now(), + snap: { + id: 'mock-sol-snap', + name: 'mock-sol-snap', + enabled: true, + }, + }, + }, +]; + +const GROUP_ACCOUNTS_NO_SUPPORTED = [ + createMockInternalAccount({ + id: 'btc-account-id', + type: BtcAccountType.P2wpkh, + }), +]; + +type AllDeFiPositionsControllerV2Actions = + MessengerActions; + +type AllDeFiPositionsControllerV2Events = + MessengerEvents; + +type RootMessenger = Messenger< + MockAnyNamespace, + AllDeFiPositionsControllerV2Actions, + AllDeFiPositionsControllerV2Events +>; + +/** + * Builds a minimal successful v6 balances response for the EVM account. + * + * @param overrides - Optional response overrides. + * @returns A v6 balances response. + */ +function buildMockBalancesResponse( + overrides?: Partial, +): V6BalancesResponse { + return { + unprocessedNetworks: [], + unprocessedIncludeAssetIds: [], + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS}`, + balances: [ + { + category: 'defi', + assetId: 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: { + protocolId: 'aave-v3', + protocolName: 'Aave V3', + description: 'Aave V3 on ethereum', + protocolUrl: 'https://aave.com/', + protocolIconUrl: 'https://example.com/aave.png', + positionType: 'supply', + poolAddress: '0xpool', + }, + }, + ], + }, + ], + ...overrides, + }; +} + +/** + * Sets up the V2 controller with the given configuration. + * + * @param config - Configuration for the mock setup. + * @param config.isEnabled - Whether the controller is enabled. + * @param config.getVsCurrency - Fiat currency getter. + * @param config.minimumFetchIntervalMs - Minimum fetch interval. + * @param config.mockGroupAccounts - Accounts returned for the selected group. + * @param config.mockFetchV6MultiAccountBalances - Mock API fetch function. + * @param config.state - Initial controller state. + * @returns The controller instance and mocks. + */ +function setupController({ + isEnabled = () => true, + getVsCurrency = () => 'USD', + minimumFetchIntervalMs, + mockGroupAccounts = GROUP_ACCOUNTS, + mockFetchV6MultiAccountBalances = jest + .fn() + .mockResolvedValue(buildMockBalancesResponse()), + state, +}: { + isEnabled?: () => boolean; + getVsCurrency?: () => string; + minimumFetchIntervalMs?: number; + mockGroupAccounts?: InternalAccount[]; + mockFetchV6MultiAccountBalances?: jest.Mock; + state?: Partial< + ReturnType + >; +} = {}) { + const messenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + + messenger.registerActionHandler( + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => mockGroupAccounts, + ); + + const controllerMessenger = new Messenger< + 'DeFiPositionsControllerV2', + AllDeFiPositionsControllerV2Actions, + AllDeFiPositionsControllerV2Events, + RootMessenger + >({ + namespace: 'DeFiPositionsControllerV2', + parent: messenger, + }); + messenger.delegate({ + messenger: controllerMessenger, + actions: ['AccountTreeController:getAccountsFromSelectedAccountGroup'], + }); + + const apiClient = { + accounts: { + fetchV6MultiAccountBalances: mockFetchV6MultiAccountBalances, + }, + } as unknown as ApiPlatformClient; + + const controller = new DeFiPositionsControllerV2({ + messenger: controllerMessenger, + apiClient, + isEnabled, + getVsCurrency, + minimumFetchIntervalMs, + state, + }); + + return { + controller, + controllerMessenger, + mockFetchV6MultiAccountBalances, + }; +} + +describe('DeFiPositionsControllerV2', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + it('sets default state', () => { + const { controller } = setupController(); + + expect(controller.state).toStrictEqual( + getDefaultDeFiPositionsControllerV2State(), + ); + }); + + it('does not fetch when the controller is disabled', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + isEnabled: () => false, + }); + + await controller.fetchDeFiPositions(); + + expect(mockFetchV6MultiAccountBalances).not.toHaveBeenCalled(); + expect(controller.state).toStrictEqual( + getDefaultDeFiPositionsControllerV2State(), + ); + }); + + it('does not fetch when the selected group has no supported accounts', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + mockGroupAccounts: GROUP_ACCOUNTS_NO_SUPPORTED, + }); + + await controller.fetchDeFiPositions(); + + expect(mockFetchV6MultiAccountBalances).not.toHaveBeenCalled(); + expect(controller.state).toStrictEqual( + getDefaultDeFiPositionsControllerV2State(), + ); + }); + + it('fetches positions and stores them keyed by internal account ID', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController(); + + await controller.fetchDeFiPositions(); + + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledWith( + [`eip155:0:${EVM_ADDRESS.toLowerCase()}`], + { + networks: DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('eip155:'), + ), + ...DEFI_BALANCES_V6_REQUEST_OPTIONS, + vsCurrency: 'usd', + }, + ); + + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, + ); + expect( + controller.state.allDeFiPositionsV2['evm-account-id'][0], + ).toMatchObject({ + protocolId: 'aave-v3', + protocolName: 'Aave V3', + chainId: 'eip155:1', + marketValue: 2000, + }); + }); + + it('maps mixed-case EVM response account IDs back to internal IDs', async () => { + const mockFetchV6MultiAccountBalances = jest.fn().mockResolvedValue( + buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS.toUpperCase()}`, + balances: [ + { + category: 'defi', + assetId: + 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: { + protocolId: 'aave-v3', + protocolName: 'Aave V3', + description: 'Aave V3 on ethereum', + protocolUrl: 'https://aave.com/', + protocolIconUrl: 'https://example.com/aave.png', + positionType: 'supply', + poolAddress: '0xpool', + }, + }, + ], + }, + ], + }), + ); + + const { controller } = setupController({ + mockFetchV6MultiAccountBalances, + }); + + await controller.fetchDeFiPositions(); + + expect(controller.state.allDeFiPositionsV2).toHaveProperty('evm-account-id'); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, + ); + }); + + it('requests Solana and EVM networks when both accounts are present', async () => { + const mockFetchV6MultiAccountBalances = jest.fn().mockResolvedValue( + buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS}`, + balances: [], + }, + { + accountId: `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`, + balances: [], + }, + ], + }), + ); + + const { controller, mockFetchV6MultiAccountBalances: mockFetch } = + setupController({ + mockGroupAccounts: GROUP_ACCOUNTS_WITH_SOLANA, + mockFetchV6MultiAccountBalances, + }); + + await controller.fetchDeFiPositions(); + + const expectedEvmNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('eip155:'), + ); + const expectedSolanaNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('solana:'), + ); + + expect(mockFetch).toHaveBeenCalledWith( + [ + `eip155:0:${EVM_ADDRESS.toLowerCase()}`, + `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`, + ], + { + networks: [...expectedEvmNetworks, ...expectedSolanaNetworks], + ...DEFI_BALANCES_V6_REQUEST_OPTIONS, + vsCurrency: 'usd', + }, + ); + expect(controller.state.allDeFiPositionsV2).toStrictEqual({ + 'evm-account-id': [], + 'solana-account-id': [], + }); + }); + + it('throttles repeated fetches for the same accounts within the interval', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + minimumFetchIntervalMs: 60_000, + }); + + await controller.fetchDeFiPositions(); + await controller.fetchDeFiPositions(); + + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(60_000); + await controller.fetchDeFiPositions(); + + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + }); + + it('clears the throttle claim when a fetch fails so retries are allowed', async () => { + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + const mockFetchV6MultiAccountBalances = jest + .fn() + .mockRejectedValueOnce(new Error('network error')) + .mockResolvedValueOnce(buildMockBalancesResponse()); + + const { controller } = setupController({ + mockFetchV6MultiAccountBalances, + }); + + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); + expect(controller.state.allDeFiPositionsV2).toStrictEqual({}); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to fetch DeFi positions', + expect.any(Error), + ); + + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, + ); + }); + + it('exposes fetchDeFiPositions via the messenger', async () => { + const { controllerMessenger, mockFetchV6MultiAccountBalances } = + setupController(); + + await controllerMessenger.call( + 'DeFiPositionsControllerV2:fetchDeFiPositions', + ); + + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); + }); + + describe('metadata', () => { + it('includes expected state in debug snapshots', () => { + const { controller } = setupController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInDebugSnapshot', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('includes expected state in state logs', () => { + const { controller } = setupController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(`{}`); + }); + + it('persists expected state', () => { + const { controller } = setupController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(` + { + "allDeFiPositionsV2": {}, + } + `); + }); + + it('exposes expected state to UI', () => { + const { controller } = setupController(); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(` + { + "allDeFiPositionsV2": {}, + } + `); + }); + }); +}); diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts new file mode 100644 index 00000000000..4ff039434b9 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts @@ -0,0 +1,182 @@ +import { + BtcAccountType, + EthAccountType, + EthMethod, + EthScope, + SolAccountType, + SolMethod, + SolScope, +} from '@metamask/keyring-api'; +import { KeyringTypes } from '@metamask/keyring-controller'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; + +import { createMockInternalAccount } from '../../../accounts-controller/tests/mocks'; +import { + buildDeFiBalancesQuery, + DEFI_BALANCES_V6_REQUEST_OPTIONS, + DEFI_SUPPORTED_NETWORKS, + normalizeCaipAccountId, +} from './build-defi-balances-query'; + +const EVM_ADDRESS = '0x0000000000000000000000000000000000000001'; +const SOLANA_ADDRESS = 'So11111111111111111111111111111111111111112'; + +const mockEvmAccount = createMockInternalAccount({ + id: 'evm-account-id', + address: EVM_ADDRESS, + type: EthAccountType.Eoa, +}); + +const mockSolanaAccount: InternalAccount = { + id: 'solana-account-id', + address: SOLANA_ADDRESS, + options: {}, + methods: [SolMethod.SendAndConfirmTransaction], + scopes: [SolScope.Mainnet], + type: SolAccountType.DataAccount, + metadata: { + name: 'Solana Account', + keyring: { type: KeyringTypes.snap }, + importTime: Date.now(), + lastSelected: Date.now(), + snap: { + id: 'mock-sol-snap', + name: 'mock-sol-snap', + enabled: true, + }, + }, +}; + +const mockBtcAccount = createMockInternalAccount({ + id: 'btc-account-id', + type: BtcAccountType.P2wpkh, +}); + +describe('normalizeCaipAccountId', () => { + it('lowercases EVM CAIP account IDs', () => { + expect( + normalizeCaipAccountId(`eip155:0:${EVM_ADDRESS.toUpperCase()}`), + ).toBe(`eip155:0:${EVM_ADDRESS.toLowerCase()}`); + }); + + it('leaves non-EVM CAIP account IDs unchanged', () => { + const solanaCaipAccountId = `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`; + + expect(normalizeCaipAccountId(solanaCaipAccountId)).toBe( + solanaCaipAccountId, + ); + }); +}); + +describe('buildDeFiBalancesQuery', () => { + it('exports the fixed v6 request options used by the controller', () => { + expect(DEFI_BALANCES_V6_REQUEST_OPTIONS).toStrictEqual({ + includeDeFiBalances: true, + forceFetchDeFiPositions: true, + includePrices: true, + }); + }); + + it('builds an EVM CAIP account spanning all supported EVM networks', () => { + const result = buildDeFiBalancesQuery([mockEvmAccount, mockBtcAccount]); + + const expectedEvmNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('eip155:'), + ); + + expect(result.networks).toStrictEqual(expectedEvmNetworks); + expect([...result.internalAccountIdByCaip.entries()]).toStrictEqual([ + [`eip155:0:${EVM_ADDRESS.toLowerCase()}`, 'evm-account-id'], + ]); + }); + + it('builds a Solana CAIP account for supported Solana networks', () => { + const result = buildDeFiBalancesQuery([mockSolanaAccount, mockBtcAccount]); + + const expectedSolanaNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('solana:'), + ); + const [, solanaReference] = SolScope.Mainnet.split(':'); + + expect(result.networks).toStrictEqual(expectedSolanaNetworks); + expect([...result.internalAccountIdByCaip.entries()]).toStrictEqual([ + [`solana:${solanaReference}:${SOLANA_ADDRESS}`, 'solana-account-id'], + ]); + }); + + it('combines EVM and Solana accounts from the selected group', () => { + const result = buildDeFiBalancesQuery([ + mockEvmAccount, + mockSolanaAccount, + mockBtcAccount, + ]); + + const expectedEvmNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('eip155:'), + ); + const expectedSolanaNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => + network.startsWith('solana:'), + ); + + // EVM networks are added first, then Solana — not the literal + // DEFI_SUPPORTED_NETWORKS order (where Solana sits mid-list). + expect(result.networks).toStrictEqual([ + ...expectedEvmNetworks, + ...expectedSolanaNetworks, + ]); + expect(result.internalAccountIdByCaip.size).toBe(2); + expect( + result.internalAccountIdByCaip.get( + `eip155:0:${EVM_ADDRESS.toLowerCase()}`, + ), + ).toBe('evm-account-id'); + expect( + result.internalAccountIdByCaip.get( + `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`, + ), + ).toBe('solana-account-id'); + }); + + it('returns empty networks and map when there are no supported accounts', () => { + const result = buildDeFiBalancesQuery([mockBtcAccount]); + + expect(result).toStrictEqual({ + networks: [], + internalAccountIdByCaip: new Map(), + }); + }); + + it('uses only the first EVM and first Solana account in the group', () => { + const secondEvmAccount = createMockInternalAccount({ + id: 'evm-account-id-2', + address: '0x0000000000000000000000000000000000000002', + type: EthAccountType.Eoa, + methods: [EthMethod.SignTransaction], + scopes: [EthScope.Eoa], + }); + const secondSolanaAccount: InternalAccount = { + ...mockSolanaAccount, + id: 'solana-account-id-2', + address: 'So22222222222222222222222222222222222222222', + }; + + const result = buildDeFiBalancesQuery([ + mockEvmAccount, + secondEvmAccount, + mockSolanaAccount, + secondSolanaAccount, + ]); + + expect(result.internalAccountIdByCaip.size).toBe(2); + expect( + result.internalAccountIdByCaip.get( + `eip155:0:${EVM_ADDRESS.toLowerCase()}`, + ), + ).toBe('evm-account-id'); + expect( + result.internalAccountIdByCaip.get( + `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`, + ), + ).toBe('solana-account-id'); + }); +}); diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts new file mode 100644 index 00000000000..dacc6205d6d --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts @@ -0,0 +1,415 @@ +import type { V6BalancesResponse } from '@metamask/core-backend'; +import type { CaipAssetType } from '@metamask/utils'; + +import { groupDeFiPositionsV6 } from './group-defi-positions-v6'; + +const AAVE_METADATA = { + protocolId: 'aave-v3', + protocolName: 'Aave V3', + description: 'Aave V3 on ethereum', + protocolUrl: 'https://aave.com/', + protocolIconUrl: 'https://example.com/aave.png', + positionType: 'supply', + poolAddress: '0xpool', +}; + +const WETH_ASSET_ID = + 'eip155:1/erc20:0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2' as CaipAssetType; +const USDC_ASSET_ID = + 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' as CaipAssetType; +const USDT_ASSET_ID = + 'eip155:1/erc20:0xdAC17F958D2ee523a2206206994597C13D831ec7' as CaipAssetType; +const BASE_WETH_ASSET_ID = + 'eip155:8453/erc20:0x4200000000000000000000000000000000000006' as CaipAssetType; + +/** + * Builds a minimal v6 balances response for tests. + * + * @param accounts - Account entries to include. + * @returns A v6 balances response. + */ +function buildResponse( + accounts: V6BalancesResponse['accounts'], +): V6BalancesResponse { + return { + unprocessedNetworks: [], + unprocessedIncludeAssetIds: [], + accounts, + }; +} + +describe('groupDeFiPositionsV6', () => { + it('groups DeFi positions by chain and protocolId', () => { + const response = buildResponse([ + { + accountId: 'eip155:0:0xabc', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: AAVE_METADATA, + }, + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '100', + price: '1', + metadata: { + ...AAVE_METADATA, + positionType: 'supply', + poolAddress: '0xpool2', + }, + }, + { + category: 'defi', + assetId: BASE_WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '2', + price: '2000', + metadata: AAVE_METADATA, + }, + ], + }, + ]); + + const result = groupDeFiPositionsV6(response); + + expect(Object.keys(result)).toStrictEqual(['eip155:0:0xabc']); + expect(result['eip155:0:0xabc']).toHaveLength(2); + + const ethGroup = result['eip155:0:0xabc'].find( + (group) => group.chainId === 'eip155:1', + ); + const baseGroup = result['eip155:0:0xabc'].find( + (group) => group.chainId === 'eip155:8453', + ); + + expect(ethGroup).toMatchObject({ + protocolId: 'aave-v3', + protocolName: 'Aave V3', + protocolIconUrl: 'https://example.com/aave.png', + chainId: 'eip155:1', + marketValue: 2100, + }); + expect(ethGroup?.sections).toHaveLength(1); + expect(ethGroup?.sections[0].positions).toHaveLength(2); + + expect(baseGroup).toMatchObject({ + protocolId: 'aave-v3', + chainId: 'eip155:8453', + marketValue: 4000, + }); + }); + + it('ignores token rows and defi rows without protocol metadata', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'token', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + }, + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '100', + price: '1', + }, + { + category: 'defi', + assetId: USDT_ASSET_ID, + name: 'Tether', + symbol: 'USDT', + decimals: 6, + balance: '50', + price: '1', + metadata: { + limit: '1', + }, + }, + ], + }, + ]); + + const result = groupDeFiPositionsV6(response); + + expect(result['account-1']).toStrictEqual([]); + }); + + it('seeds an empty list for accounts with no DeFi positions', () => { + const response = buildResponse([ + { + accountId: 'account-empty', + balances: [], + }, + ]); + + expect(groupDeFiPositionsV6(response)).toStrictEqual({ + 'account-empty': [], + }); + }); + + it('skips accounts that do not resolve to an internal account ID', () => { + const response = buildResponse([ + { + accountId: 'eip155:0:0xunknown', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: AAVE_METADATA, + }, + ], + }, + { + accountId: 'eip155:0:0xknown', + balances: [ + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '10', + price: '1', + metadata: AAVE_METADATA, + }, + ], + }, + ]); + + const result = groupDeFiPositionsV6(response, (responseAccountId) => + responseAccountId === 'eip155:0:0xknown' ? 'internal-1' : undefined, + ); + + expect(Object.keys(result)).toStrictEqual(['internal-1']); + expect(result['internal-1']).toHaveLength(1); + }); + + it('omits market value when price is missing or invalid', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + metadata: AAVE_METADATA, + }, + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: 'not-a-number', + price: '1', + metadata: { + ...AAVE_METADATA, + protocolName: 'Aave V3 USDC', + }, + }, + { + category: 'defi', + assetId: USDT_ASSET_ID, + name: 'Tether', + symbol: 'USDT', + decimals: 6, + balance: '5', + price: '1', + metadata: { + ...AAVE_METADATA, + protocolName: 'Aave V3 USDT', + }, + }, + ], + }, + ]); + + const [group] = groupDeFiPositionsV6(response)['account-1']; + + expect(group.marketValue).toBe(5); + expect(group.sections[0].positions[0].marketValue).toBeUndefined(); + expect(group.sections[1].positions[0].marketValue).toBeUndefined(); + expect(group.sections[2].positions[0].marketValue).toBe(5); + }); + + it('creates separate detail sections per protocolName under one protocolId', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: { + ...AAVE_METADATA, + protocolName: 'Aave V3 Supply', + }, + }, + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '100', + price: '1', + metadata: { + ...AAVE_METADATA, + protocolName: 'Aave V3 Borrow', + positionType: 'borrow', + }, + }, + ], + }, + ]); + + const [group] = groupDeFiPositionsV6(response)['account-1']; + + expect(group.protocolName).toBe('Aave V3 Supply'); + expect(group.sections).toStrictEqual([ + { + protocolName: 'Aave V3 Supply', + positions: [ + expect.objectContaining({ + symbol: 'WETH', + positionType: 'supply', + }), + ], + }, + { + protocolName: 'Aave V3 Borrow', + positions: [ + expect.objectContaining({ + symbol: 'USDC', + positionType: 'borrow', + }), + ], + }, + ]); + }); + + it('dedupes icon-group symbols and moves ETH/WETH to the front', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '100', + price: '1', + metadata: AAVE_METADATA, + }, + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: AAVE_METADATA, + }, + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '0.5', + price: '2000', + metadata: { + ...AAVE_METADATA, + positionType: 'reward', + }, + }, + ], + }, + ]); + + const [group] = groupDeFiPositionsV6(response)['account-1']; + + expect(group.iconGroup.map((item) => item.symbol)).toStrictEqual([ + 'WETH', + 'USDC', + ]); + expect(group.iconGroup[0].avatarValue).toBe( + 'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/erc20/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2.png', + ); + }); + + it('builds underlying positions with token images and chain IDs', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1.5', + price: '2000', + metadata: AAVE_METADATA, + }, + ], + }, + ]); + + const [group] = groupDeFiPositionsV6(response)['account-1']; + const [position] = group.sections[0].positions; + + expect(position).toStrictEqual({ + assetId: WETH_ASSET_ID, + chainId: 'eip155:1', + symbol: 'WETH', + name: 'Wrapped Ether', + decimals: 18, + balance: '1.5', + marketValue: 3000, + positionType: 'supply', + poolAddress: '0xpool', + tokenImage: + 'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/erc20/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2.png', + }); + }); +}); From 057ff96708b6c8aaaa53228f689fc5a13bd6ecd5 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 16:30:40 +0100 Subject: [PATCH 10/36] lint --- .../DeFiPositionsControllerV2.test.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 6f999f40a57..e78aee5a205 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -1,5 +1,8 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; -import type { ApiPlatformClient, V6BalancesResponse } from '@metamask/core-backend'; +import type { + ApiPlatformClient, + V6BalancesResponse, +} from '@metamask/core-backend'; import { BtcAccountType, EthAccountType, @@ -102,7 +105,8 @@ function buildMockBalancesResponse( balances: [ { category: 'defi', - assetId: 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', + assetId: + 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', name: 'Wrapped Ether', symbol: 'WETH', decimals: 18, @@ -152,9 +156,7 @@ function setupController({ minimumFetchIntervalMs?: number; mockGroupAccounts?: InternalAccount[]; mockFetchV6MultiAccountBalances?: jest.Mock; - state?: Partial< - ReturnType - >; + state?: Partial>; } = {}) { const messenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, @@ -313,7 +315,9 @@ describe('DeFiPositionsControllerV2', () => { await controller.fetchDeFiPositions(); - expect(controller.state.allDeFiPositionsV2).toHaveProperty('evm-account-id'); + expect(controller.state.allDeFiPositionsV2).toHaveProperty( + 'evm-account-id', + ); expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( 1, ); From f8cde807eef217189ff340159a02d8e8547f0458 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 16:45:29 +0100 Subject: [PATCH 11/36] changelog --- packages/assets-controllers/CHANGELOG.md | 4 ++++ .../DeFiPositionsControllerV2-method-action-types.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index fc7a60d3c7d..0b0432081cc 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `DeFiPositionsControllerV2`, which fetches DeFi positions from the Accounts API v6 multiaccount balances endpoint and stores them in a client-ready shape under `allDeFiPositionsV2` ([#9503](https://github.com/MetaMask/core/pull/9503)) + - Export `DeFiPositionsControllerV2` and `getDefaultDeFiPositionsControllerV2State` + - Export types including `DeFiPositionsControllerV2State`, messenger/action/event types, `DeFiPositionsByAccount`, and related position group types + - Export `DeFiPositionsControllerV2FetchDeFiPositionsAction` and `buildDeFiBalancesQuery` helpers - Add Robinhood Chain (`4663`/`0x1237`) BalanceFetcher support for legacy single-call token detection ([#9473](https://github.com/MetaMask/core/pull/9473)) - Add `Robinhood` in `SupportedTokenDetectionNetworks` - Add Robinhood BalanceFetcher address in `SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID` diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index ebc596f4e98..1077d1da23c 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -12,8 +12,8 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; * response, and updating state. * * Throttled per set of accounts by an in-memory minimum interval, so repeated - * calls within the window are no-ops. Disabled controllers and empty account - * groups return without fetching. + * calls within the window are no-ops (no HTTP, no regroup, no state write). + * Disabled controllers and empty account groups return without fetching. */ export type DeFiPositionsControllerV2FetchDeFiPositionsAction = { type: `DeFiPositionsControllerV2:fetchDeFiPositions`; From 63a75da3435a039ea36c0a9daf3d8cb1d8a17cef Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 17:49:06 +0100 Subject: [PATCH 12/36] refactors --- .../DeFiPositionsControllerV2.ts | 16 +++++++-- .../build-defi-balances-query.ts | 36 ++++++++++++++++++- .../group-defi-positions-v6.ts | 18 ++++++++-- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 080ddb2bcf4..0f05cd7cb27 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -11,7 +11,7 @@ import type { Messenger } from '@metamask/messenger'; import { buildDeFiBalancesQuery, DEFI_BALANCES_V6_REQUEST_OPTIONS, - normalizeCaipAccountId, + toAccountMatchKey, } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; import type { DeFiPositionsByAccount } from './group-defi-positions-v6'; @@ -236,12 +236,22 @@ export class DeFiPositionsControllerV2 extends BaseController< vsCurrency: this.#getVsCurrency().toLowerCase(), }); - // The v6 response echoes the CAIP-10 IDs we sent; map them back to the + // The v6 response echoes a per-chain CAIP-10 ID for every chain + // (`eip155:1:`, `eip155:137:`, ...), while we requested with + // the all-chains reference (`eip155:0:`). Match on namespace + + // address (ignoring the chain reference) so responses map back to the // internal account IDs used to key state. Unmatched accounts are skipped. + const internalAccountIdByMatchKey = new Map(); + for (const [caipAccountId, internalId] of internalAccountIdByCaip) { + internalAccountIdByMatchKey.set( + toAccountMatchKey(caipAccountId), + internalId, + ); + } const resolveAccountId = ( responseAccountId: string, ): string | undefined => - internalAccountIdByCaip.get(normalizeCaipAccountId(responseAccountId)); + internalAccountIdByMatchKey.get(toAccountMatchKey(responseAccountId)); const positionsByAccount = groupDeFiPositionsV6( response, diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index 1c303b3c3ea..a0f588be467 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -5,7 +5,11 @@ import { } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { CaipAccountId, CaipChainId } from '@metamask/utils'; -import { KnownCaipNamespace, toCaipAccountId } from '@metamask/utils'; +import { + KnownCaipNamespace, + parseCaipAccountId, + toCaipAccountId, +} from '@metamask/utils'; /** * Networks the DeFi balances (v6 multiaccount) endpoint supports. @@ -80,6 +84,36 @@ export function normalizeCaipAccountId(caipAccountId: string): string { : caipAccountId; } +/** + * Builds a chain-reference-agnostic key (`namespace:address`) for matching the + * CAIP-10 account IDs we send against the ones the v6 API echoes back. + * + * We request EVM balances with the all-chains reference (`eip155:0:
`), + * but the response echoes a separate per-chain ID for every chain + * (`eip155:1:
`, `eip155:137:
`, ...). Matching on the full + * CAIP-10 string therefore fails, so we drop the reference and match on + * namespace + address instead. EVM addresses are lowercased (case-insensitive); + * other namespaces (e.g. Solana) keep their case. + * + * @param caipAccountId - A CAIP-10 account ID. + * @returns The match key, or the normalized ID if it cannot be parsed. + */ +export function toAccountMatchKey(caipAccountId: string): string { + try { + const { + chain: { namespace }, + address, + } = parseCaipAccountId(caipAccountId as CaipAccountId); + const normalizedAddress = + namespace === KnownCaipNamespace.Eip155 + ? address.toLowerCase() + : address; + return `${namespace}:${normalizedAddress}`; + } catch { + return normalizeCaipAccountId(caipAccountId); + } +} + /** * Builds the account IDs and networks to request DeFi positions for, from the * accounts in the selected account group. diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index 9d190c2b9f5..7bb8b5a179b 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -222,7 +222,14 @@ export function groupDeFiPositionsV6( resolveAccountId: (responseAccountId: string) => string | undefined = (id) => id, ): DeFiPositionsByAccount { - const result: DeFiPositionsByAccount = {}; + // Accumulate groups per resolved internal account ID. The v6 response returns + // a separate entry per chain (e.g. `eip155:1:`, `eip155:137:`), + // and several of them can resolve to the same internal account ID, so we must + // merge across all of them rather than overwrite per response account. + const groupsByAccountKey = new Map< + string, + Map + >(); for (const account of response.accounts) { const accountId = resolveAccountId(account.accountId); @@ -232,7 +239,11 @@ export function groupDeFiPositionsV6( // Seed every queried account so accounts that no longer hold positions // overwrite (clear) any previously stored data. - const groupsByKey = new Map(); + let groupsByKey = groupsByAccountKey.get(accountId); + if (!groupsByKey) { + groupsByKey = new Map(); + groupsByAccountKey.set(accountId, groupsByKey); + } for (const balance of account.balances) { if (!isDefiBalanceWithMetadata(balance)) { @@ -277,7 +288,10 @@ export function groupDeFiPositionsV6( } section.positions.push(position); } + } + const result: DeFiPositionsByAccount = {}; + for (const [accountId, groupsByKey] of groupsByAccountKey) { const groups = [...groupsByKey.values()]; for (const group of groups) { orderIconGroup(group.iconGroup); From 06e53d3315b5ecdc2da2944db5194cd63fbd684d Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 17:49:26 +0100 Subject: [PATCH 13/36] use fake account --- .../DeFiPositionsControllerV2.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 0f05cd7cb27..68e9e001333 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -11,6 +11,7 @@ import type { Messenger } from '@metamask/messenger'; import { buildDeFiBalancesQuery, DEFI_BALANCES_V6_REQUEST_OPTIONS, + normalizeCaipAccountId, toAccountMatchKey, } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; @@ -207,6 +208,17 @@ export class DeFiPositionsControllerV2 extends BaseController< return; } + // TEMPORARY: always fetch this account for testing, regardless of selection. + // Positions are still stored under the selected account's internal ID. + const TEST_CAIP_ACCOUNT_ID = + 'eip155:0:0x3e8734ec146c981e3ed1f6b582d447dde701d90c'; + const selectedInternalAccountId = [...internalAccountIdByCaip.values()][0]; + internalAccountIdByCaip.clear(); + internalAccountIdByCaip.set( + normalizeCaipAccountId(TEST_CAIP_ACCOUNT_ID), + selectedInternalAccountId, + ); + const accountIds = [...internalAccountIdByCaip.keys()]; // Stable key so the same account set throttles together regardless of map // iteration order. From 72c07fa30d450b3a16d5c710c5d87f3426064231 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 17:51:13 +0100 Subject: [PATCH 14/36] linting --- .../src/DeFiPositionsController/build-defi-balances-query.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index a0f588be467..5d35c1a2a4b 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -105,9 +105,7 @@ export function toAccountMatchKey(caipAccountId: string): string { address, } = parseCaipAccountId(caipAccountId as CaipAccountId); const normalizedAddress = - namespace === KnownCaipNamespace.Eip155 - ? address.toLowerCase() - : address; + namespace === KnownCaipNamespace.Eip155 ? address.toLowerCase() : address; return `${namespace}:${normalizedAddress}`; } catch { return normalizeCaipAccountId(caipAccountId); From c3e3b0bc416df10486494420b1cfd8bd597afc7c Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 18:06:37 +0100 Subject: [PATCH 15/36] position types --- packages/assets-controllers/CHANGELOG.md | 1 + .../DeFiPositionsControllerV2.test.ts | 4 +- .../DeFiPositionsControllerV2.ts | 5 ++ .../group-defi-positions-v6.test.ts | 59 ++++++++++++++++--- .../group-defi-positions-v6.ts | 44 ++++++++++++-- packages/assets-controllers/src/index.ts | 3 + packages/core-backend/CHANGELOG.md | 4 ++ .../core-backend/src/api/accounts/index.ts | 2 + .../core-backend/src/api/accounts/types.ts | 27 ++++++++- packages/core-backend/src/api/index.ts | 3 +- packages/core-backend/src/index.ts | 2 + 11 files changed, 139 insertions(+), 15 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 0b0432081cc..1f7d0ece40e 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `DeFiPositionsControllerV2`, which fetches DeFi positions from the Accounts API v6 multiaccount balances endpoint and stores them in a client-ready shape under `allDeFiPositionsV2` ([#9503](https://github.com/MetaMask/core/pull/9503)) - Export `DeFiPositionsControllerV2` and `getDefaultDeFiPositionsControllerV2State` - Export types including `DeFiPositionsControllerV2State`, messenger/action/event types, `DeFiPositionsByAccount`, and related position group types + - Export `DeFiPositionType`, `DEFI_POSITION_TYPES`, and `DEFI_POSITION_LIABILITY_TYPES` (`lending` is subtracted from protocol `marketValue`) - Export `DeFiPositionsControllerV2FetchDeFiPositionsAction` and `buildDeFiBalancesQuery` helpers - Add Robinhood Chain (`4663`/`0x1237`) BalanceFetcher support for legacy single-call token detection ([#9473](https://github.com/MetaMask/core/pull/9473)) - Add `Robinhood` in `SupportedTokenDetectionNetworks` diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index e78aee5a205..387d9d4cf21 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -118,7 +118,7 @@ function buildMockBalancesResponse( description: 'Aave V3 on ethereum', protocolUrl: 'https://aave.com/', protocolIconUrl: 'https://example.com/aave.png', - positionType: 'supply', + positionType: 'deposit', poolAddress: '0xpool', }, }, @@ -299,7 +299,7 @@ describe('DeFiPositionsControllerV2', () => { description: 'Aave V3 on ethereum', protocolUrl: 'https://aave.com/', protocolIconUrl: 'https://example.com/aave.png', - positionType: 'supply', + positionType: 'deposit', poolAddress: '0xpool', }, }, diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 68e9e001333..e271bf63fcc 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -30,6 +30,11 @@ export type { DeFiPositionDetailsSection, DeFiUnderlyingPosition, DeFiPositionIconGroupItem, + DeFiPositionType, +} from './group-defi-positions-v6'; +export { + DEFI_POSITION_TYPES, + DEFI_POSITION_LIABILITY_TYPES, } from './group-defi-positions-v6'; export type DeFiPositionsControllerV2State = { diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts index dacc6205d6d..18581a88541 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts @@ -9,7 +9,7 @@ const AAVE_METADATA = { description: 'Aave V3 on ethereum', protocolUrl: 'https://aave.com/', protocolIconUrl: 'https://example.com/aave.png', - positionType: 'supply', + positionType: 'deposit', poolAddress: '0xpool', }; @@ -64,7 +64,7 @@ describe('groupDeFiPositionsV6', () => { price: '1', metadata: { ...AAVE_METADATA, - positionType: 'supply', + positionType: 'deposit', poolAddress: '0xpool2', }, }, @@ -262,6 +262,51 @@ describe('groupDeFiPositionsV6', () => { expect(group.sections[2].positions[0].marketValue).toBe(5); }); + it('subtracts lending positions from the protocol market value', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: { + ...AAVE_METADATA, + protocolName: 'Aave V3 Supply', + positionType: 'deposit', + }, + }, + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '500', + price: '1', + metadata: { + ...AAVE_METADATA, + protocolName: 'Aave V3 Borrow', + positionType: 'lending', + }, + }, + ], + }, + ]); + + const [group] = groupDeFiPositionsV6(response)['account-1']; + + expect(group.marketValue).toBe(1500); + expect(group.sections[0].positions[0].marketValue).toBe(2000); + expect(group.sections[1].positions[0].marketValue).toBe(500); + expect(group.sections[1].positions[0].positionType).toBe('lending'); + }); + it('creates separate detail sections per protocolName under one protocolId', () => { const response = buildResponse([ { @@ -291,7 +336,7 @@ describe('groupDeFiPositionsV6', () => { metadata: { ...AAVE_METADATA, protocolName: 'Aave V3 Borrow', - positionType: 'borrow', + positionType: 'lending', }, }, ], @@ -307,7 +352,7 @@ describe('groupDeFiPositionsV6', () => { positions: [ expect.objectContaining({ symbol: 'WETH', - positionType: 'supply', + positionType: 'deposit', }), ], }, @@ -316,7 +361,7 @@ describe('groupDeFiPositionsV6', () => { positions: [ expect.objectContaining({ symbol: 'USDC', - positionType: 'borrow', + positionType: 'lending', }), ], }, @@ -358,7 +403,7 @@ describe('groupDeFiPositionsV6', () => { price: '2000', metadata: { ...AAVE_METADATA, - positionType: 'reward', + positionType: 'rewards', }, }, ], @@ -406,7 +451,7 @@ describe('groupDeFiPositionsV6', () => { decimals: 18, balance: '1.5', marketValue: 3000, - positionType: 'supply', + positionType: 'deposit', poolAddress: '0xpool', tokenImage: 'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/erc20/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2.png', diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index 7bb8b5a179b..e3a7f9cbb24 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -1,7 +1,9 @@ +import { V6_DEFI_POSITION_TYPES } from '@metamask/core-backend'; import type { V6BalanceItem, V6BalanceMetadata, V6BalancesResponse, + V6DeFiPositionType, } from '@metamask/core-backend'; import type { CaipAssetType, CaipChainId } from '@metamask/utils'; import { @@ -16,6 +18,25 @@ import { // with a shared helper if/when one lands in core. const STATIC_METAMASK_BASE_URL = 'https://static.cx.metamask.io'; +/** + * Possible `positionType` values from Accounts API v6 DeFi metadata. + * Re-export of {@link V6_DEFI_POSITION_TYPES} from `@metamask/core-backend`. + */ +export const DEFI_POSITION_TYPES = V6_DEFI_POSITION_TYPES; + +/** + * The specific module or functionality within a DeFi protocol where a position + * is held. Alias of {@link V6DeFiPositionType} from `@metamask/core-backend`. + */ +export type DeFiPositionType = V6DeFiPositionType; + +/** + * Position types whose fiat value is a liability and is subtracted from the + * protocol group's aggregated `marketValue`. + */ +export const DEFI_POSITION_LIABILITY_TYPES: ReadonlySet = + new Set(['lending']); + /** * An icon-group entry shown next to a protocol in the DeFi tab list. */ @@ -38,8 +59,8 @@ export type DeFiUnderlyingPosition = { balance: string; /** Fiat market value in the requested currency, when a price is available. */ marketValue?: number; - /** Position type from protocol metadata (e.g. supply, borrow, stake, reward). */ - positionType: string; + /** Position type from protocol metadata. */ + positionType: DeFiPositionType; /** Address of the pool this position belongs to. */ poolAddress: string; /** Token icon URL, when one can be built for the asset. */ @@ -68,7 +89,10 @@ export type DeFiProtocolPositionGroup = { protocolName: string; protocolIconUrl: string; chainId: CaipChainId; - /** Aggregated fiat market value across all positions in the group. */ + /** + * Aggregated fiat market value across all positions in the group. + * `lending` positions are subtracted; all other types are added. + */ marketValue: number; /** Icon-group entries for the list row. */ iconGroup: DeFiPositionIconGroupItem[]; @@ -155,6 +179,17 @@ function getMarketValue(balance: V6BalanceItem): number | undefined { return normalizedBalance * price; } +/** + * Returns the sign used when rolling a position's market value into a protocol + * group total. Liability types (currently `lending`) subtract; all others add. + * + * @param positionType - The position's protocol module type. + * @returns `-1` for liabilities, `1` otherwise. + */ +function getMarketValueSign(positionType: DeFiPositionType): 1 | -1 { + return DEFI_POSITION_LIABILITY_TYPES.has(positionType) ? -1 : 1; +} + /** * Moves a priority symbol (ETH/WETH) to the front of the icon group, in place. * @@ -269,7 +304,8 @@ export function groupDeFiPositionsV6( } if (position.marketValue !== undefined) { - group.marketValue += position.marketValue; + group.marketValue += + position.marketValue * getMarketValueSign(position.positionType); } if (!group.iconGroup.some((item) => item.symbol === position.symbol)) { diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 950c530f77c..c0f9e86f6a8 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -267,6 +267,8 @@ export type { GroupedDeFiPositions } from './DeFiPositionsController/group-defi- export { DeFiPositionsControllerV2, getDefaultDeFiPositionsControllerV2State, + DEFI_POSITION_TYPES, + DEFI_POSITION_LIABILITY_TYPES, } from './DeFiPositionsController/DeFiPositionsControllerV2'; export type { DeFiPositionsControllerV2State, @@ -280,6 +282,7 @@ export type { DeFiPositionDetailsSection, DeFiUnderlyingPosition, DeFiPositionIconGroupItem, + DeFiPositionType, } from './DeFiPositionsController/DeFiPositionsControllerV2'; export type { DeFiPositionsControllerV2FetchDeFiPositionsAction } from './DeFiPositionsController/DeFiPositionsControllerV2-method-action-types'; export { diff --git a/packages/core-backend/CHANGELOG.md b/packages/core-backend/CHANGELOG.md index c0e87c8bada..c8467ac734d 100644 --- a/packages/core-backend/CHANGELOG.md +++ b/packages/core-backend/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Export `V6_DEFI_POSITION_TYPES` and inferred `V6DeFiPositionType`, and type `V6BalanceMetadata.positionType` with the Accounts API v6 DeFi position module values (`deposit`, `lending`, `yield`, `liquidity_pool`, `staked`, `leveraged_farming`, `nft_staked`, `farming`, `locked`, `vesting`, `rewards`, `investment`) + ### Changed - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) diff --git a/packages/core-backend/src/api/accounts/index.ts b/packages/core-backend/src/api/accounts/index.ts index 3f08819c3fa..1242b951f21 100644 --- a/packages/core-backend/src/api/accounts/index.ts +++ b/packages/core-backend/src/api/accounts/index.ts @@ -3,6 +3,7 @@ */ export { AccountsApiClient } from './client'; +export { V6_DEFI_POSITION_TYPES } from './types'; export type { V5BalanceItem, V5BalancesResponse, @@ -10,6 +11,7 @@ export type { V2BalancesResponse, V4BalancesResponse, V6VsCurrency, + V6DeFiPositionType, V6BalanceMetadata, V6TokenMetadata, V6BalanceItem, diff --git a/packages/core-backend/src/api/accounts/types.ts b/packages/core-backend/src/api/accounts/types.ts index 41eeb031767..322477e198a 100644 --- a/packages/core-backend/src/api/accounts/types.ts +++ b/packages/core-backend/src/api/accounts/types.ts @@ -63,6 +63,31 @@ export type V4BalancesResponse = { */ export type V6VsCurrency = string; +/** + * Possible `positionType` values on DeFi rows in the v6 balances response. + * Categorizes the protocol module where the position is held. + */ +export const V6_DEFI_POSITION_TYPES = [ + 'deposit', + 'lending', + 'yield', + 'liquidity_pool', + 'staked', + 'leveraged_farming', + 'nft_staked', + 'farming', + 'locked', + 'vesting', + 'rewards', + 'investment', +] as const; + +/** + * The specific module or functionality within a DeFi protocol where a position + * is held. + */ +export type V6DeFiPositionType = (typeof V6_DEFI_POSITION_TYPES)[number]; + /** * DeFi protocol metadata attached to a `category: defi` row in the v6 balances * response (`BalanceMetadataV3ResponseDto`). @@ -73,7 +98,7 @@ export type V6BalanceMetadata = { description: string; protocolUrl: string; protocolIconUrl: string; - positionType: string; + positionType: V6DeFiPositionType; poolAddress: string; }; diff --git a/packages/core-backend/src/api/index.ts b/packages/core-backend/src/api/index.ts index 02c01b9838e..9f30f023979 100644 --- a/packages/core-backend/src/api/index.ts +++ b/packages/core-backend/src/api/index.ts @@ -23,7 +23,7 @@ export { } from './shared-types'; // Accounts API -export { AccountsApiClient } from './accounts'; +export { AccountsApiClient, V6_DEFI_POSITION_TYPES } from './accounts'; export type { V5BalanceItem, V5BalancesResponse, @@ -31,6 +31,7 @@ export type { V2BalancesResponse, V4BalancesResponse, V6VsCurrency, + V6DeFiPositionType, V6BalanceMetadata, V6TokenMetadata, V6BalanceItem, diff --git a/packages/core-backend/src/index.ts b/packages/core-backend/src/index.ts index f88d719373e..b58ac3abc54 100644 --- a/packages/core-backend/src/index.ts +++ b/packages/core-backend/src/index.ts @@ -121,6 +121,7 @@ export { API_URLS, STALE_TIMES, GC_TIMES, + V6_DEFI_POSITION_TYPES, // Helpers calculateRetryDelay, getQueryOptionsOverrides, @@ -148,6 +149,7 @@ export type { V2BalancesResponse, V4BalancesResponse, V6VsCurrency, + V6DeFiPositionType, V6BalanceMetadata, V6TokenMetadata, V6BalanceItem, From f6587f69d30e1c764bbc8dfc5962910faa803b26 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 14 Jul 2026 18:08:15 +0100 Subject: [PATCH 16/36] fix tests --- .../DeFiPositionsControllerV2.test.ts | 17 ++++++++++++----- .../DeFiPositionsControllerV2.ts | 12 ------------ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 387d9d4cf21..931cf6103a3 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -61,8 +61,6 @@ const GROUP_ACCOUNTS_WITH_SOLANA: InternalAccount[] = [ lastSelected: Date.now(), snap: { id: 'mock-sol-snap', - name: 'mock-sol-snap', - enabled: true, }, }, }, @@ -142,8 +140,8 @@ function buildMockBalancesResponse( * @returns The controller instance and mocks. */ function setupController({ - isEnabled = () => true, - getVsCurrency = () => 'USD', + isEnabled = (): boolean => true, + getVsCurrency = (): string => 'USD', minimumFetchIntervalMs, mockGroupAccounts = GROUP_ACCOUNTS, mockFetchV6MultiAccountBalances = jest @@ -157,7 +155,16 @@ function setupController({ mockGroupAccounts?: InternalAccount[]; mockFetchV6MultiAccountBalances?: jest.Mock; state?: Partial>; -} = {}) { +} = {}): { + controller: DeFiPositionsControllerV2; + controllerMessenger: Messenger< + 'DeFiPositionsControllerV2', + AllDeFiPositionsControllerV2Actions, + AllDeFiPositionsControllerV2Events, + RootMessenger + >; + mockFetchV6MultiAccountBalances: jest.Mock; +} { const messenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, }); diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index e271bf63fcc..71066947830 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -11,7 +11,6 @@ import type { Messenger } from '@metamask/messenger'; import { buildDeFiBalancesQuery, DEFI_BALANCES_V6_REQUEST_OPTIONS, - normalizeCaipAccountId, toAccountMatchKey, } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; @@ -213,17 +212,6 @@ export class DeFiPositionsControllerV2 extends BaseController< return; } - // TEMPORARY: always fetch this account for testing, regardless of selection. - // Positions are still stored under the selected account's internal ID. - const TEST_CAIP_ACCOUNT_ID = - 'eip155:0:0x3e8734ec146c981e3ed1f6b582d447dde701d90c'; - const selectedInternalAccountId = [...internalAccountIdByCaip.values()][0]; - internalAccountIdByCaip.clear(); - internalAccountIdByCaip.set( - normalizeCaipAccountId(TEST_CAIP_ACCOUNT_ID), - selectedInternalAccountId, - ); - const accountIds = [...internalAccountIdByCaip.keys()]; // Stable key so the same account set throttles together regardless of map // iteration order. From 8770d266b1928cdb6623f21553a06129e61f4853 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Wed, 15 Jul 2026 09:47:38 +0100 Subject: [PATCH 17/36] changelog --- packages/core-backend/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-backend/CHANGELOG.md b/packages/core-backend/CHANGELOG.md index c8467ac734d..ca5ca5f8bae 100644 --- a/packages/core-backend/CHANGELOG.md +++ b/packages/core-backend/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Export `V6_DEFI_POSITION_TYPES` and inferred `V6DeFiPositionType`, and type `V6BalanceMetadata.positionType` with the Accounts API v6 DeFi position module values (`deposit`, `lending`, `yield`, `liquidity_pool`, `staked`, `leveraged_farming`, `nft_staked`, `farming`, `locked`, `vesting`, `rewards`, `investment`) +- Export `V6_DEFI_POSITION_TYPES` and inferred `V6DeFiPositionType`, and type `V6BalanceMetadata.positionType` with the Accounts API v6 DeFi position module values (`deposit`, `lending`, `yield`, `liquidity_pool`, `staked`, `leveraged_farming`, `nft_staked`, `farming`, `locked`, `vesting`, `rewards`, `investment`) ([#9503](https://github.com/MetaMask/core/pull/9503)) ### Changed From 72f0609be8e04ce5c1c2f7cfa90e6b05293e7979 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Thu, 16 Jul 2026 14:01:00 +0100 Subject: [PATCH 18/36] fixes to types --- packages/assets-controllers/CHANGELOG.md | 1 + .../DeFiPositionsControllerV2.test.ts | 8 +- .../defi-visualizer.html | 4 +- .../group-defi-positions-v6.test.ts | 74 ++++++++++++++++--- .../group-defi-positions-v6.ts | 36 ++++++--- packages/core-backend/CHANGELOG.md | 2 + .../src/api/accounts/client.test.ts | 3 +- .../core-backend/src/api/accounts/types.ts | 3 +- 8 files changed, 100 insertions(+), 31 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 1f7d0ece40e..8fd4487c8ae 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `DeFiPositionsControllerV2`, which fetches DeFi positions from the Accounts API v6 multiaccount balances endpoint and stores them in a client-ready shape under `allDeFiPositionsV2` ([#9503](https://github.com/MetaMask/core/pull/9503)) - Export `DeFiPositionsControllerV2` and `getDefaultDeFiPositionsControllerV2State` - Export types including `DeFiPositionsControllerV2State`, messenger/action/event types, `DeFiPositionsByAccount`, and related position group types + - Group detail sections by Accounts API `productName`; keep per-position `groupId` from metadata - Export `DeFiPositionType`, `DEFI_POSITION_TYPES`, and `DEFI_POSITION_LIABILITY_TYPES` (`lending` is subtracted from protocol `marketValue`) - Export `DeFiPositionsControllerV2FetchDeFiPositionsAction` and `buildDeFiBalancesQuery` helpers - Add Robinhood Chain (`4663`/`0x1237`) BalanceFetcher support for legacy single-call token detection ([#9473](https://github.com/MetaMask/core/pull/9473)) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 931cf6103a3..f8bfd293f21 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -112,12 +112,13 @@ function buildMockBalancesResponse( price: '2000', metadata: { protocolId: 'aave-v3', - protocolName: 'Aave V3', + productName: 'Aave V3', description: 'Aave V3 on ethereum', protocolUrl: 'https://aave.com/', protocolIconUrl: 'https://example.com/aave.png', positionType: 'deposit', poolAddress: '0xpool', + groupId: 'group-aave-1', }, }, ], @@ -278,7 +279,7 @@ describe('DeFiPositionsControllerV2', () => { controller.state.allDeFiPositionsV2['evm-account-id'][0], ).toMatchObject({ protocolId: 'aave-v3', - protocolName: 'Aave V3', + productName: 'Aave V3', chainId: 'eip155:1', marketValue: 2000, }); @@ -302,12 +303,13 @@ describe('DeFiPositionsControllerV2', () => { price: '2000', metadata: { protocolId: 'aave-v3', - protocolName: 'Aave V3', + productName: 'Aave V3', description: 'Aave V3 on ethereum', protocolUrl: 'https://aave.com/', protocolIconUrl: 'https://example.com/aave.png', positionType: 'deposit', poolAddress: '0xpool', + groupId: 'group-aave-1', }, }, ], diff --git a/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html b/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html index 20eca47f1cd..ccf6b204982 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html +++ b/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html @@ -504,12 +504,12 @@

Raw JSON response

var NO_POOL = '(no pool address)'; function group(entries) { - // protocolId -> protocolName -> poolAddress -> positionType -> [entries] + // protocolId -> productName -> poolAddress -> positionType -> [entries] var tree = {}; entries.forEach(function (e) { var m = e.raw.metadata || {}; var pid = m.protocolId || '(no protocolId)'; - var pname = m.protocolName || '(no protocolName)'; + var pname = m.productName || '(no productName)'; var pool = m.poolAddress || NO_POOL; var ptype = m.positionType || '(no positionType)'; tree[pid] = tree[pid] || {}; diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts index 18581a88541..de095116137 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts @@ -5,12 +5,13 @@ import { groupDeFiPositionsV6 } from './group-defi-positions-v6'; const AAVE_METADATA = { protocolId: 'aave-v3', - protocolName: 'Aave V3', + productName: 'Aave V3', description: 'Aave V3 on ethereum', protocolUrl: 'https://aave.com/', protocolIconUrl: 'https://example.com/aave.png', positionType: 'deposit', poolAddress: '0xpool', + groupId: 'group-aave-1', }; const WETH_ASSET_ID = @@ -96,7 +97,7 @@ describe('groupDeFiPositionsV6', () => { expect(ethGroup).toMatchObject({ protocolId: 'aave-v3', - protocolName: 'Aave V3', + productName: 'Aave V3', protocolIconUrl: 'https://example.com/aave.png', chainId: 'eip155:1', marketValue: 2100, @@ -234,7 +235,7 @@ describe('groupDeFiPositionsV6', () => { price: '1', metadata: { ...AAVE_METADATA, - protocolName: 'Aave V3 USDC', + productName: 'Aave V3 USDC', }, }, { @@ -247,7 +248,7 @@ describe('groupDeFiPositionsV6', () => { price: '1', metadata: { ...AAVE_METADATA, - protocolName: 'Aave V3 USDT', + productName: 'Aave V3 USDT', }, }, ], @@ -277,7 +278,7 @@ describe('groupDeFiPositionsV6', () => { price: '2000', metadata: { ...AAVE_METADATA, - protocolName: 'Aave V3 Supply', + productName: 'Aave V3 Supply', positionType: 'deposit', }, }, @@ -291,7 +292,7 @@ describe('groupDeFiPositionsV6', () => { price: '1', metadata: { ...AAVE_METADATA, - protocolName: 'Aave V3 Borrow', + productName: 'Aave V3 Borrow', positionType: 'lending', }, }, @@ -307,7 +308,7 @@ describe('groupDeFiPositionsV6', () => { expect(group.sections[1].positions[0].positionType).toBe('lending'); }); - it('creates separate detail sections per protocolName under one protocolId', () => { + it('creates separate detail sections per productName under one protocolId', () => { const response = buildResponse([ { accountId: 'account-1', @@ -322,7 +323,7 @@ describe('groupDeFiPositionsV6', () => { price: '2000', metadata: { ...AAVE_METADATA, - protocolName: 'Aave V3 Supply', + productName: 'Aave V3 Supply', }, }, { @@ -335,7 +336,7 @@ describe('groupDeFiPositionsV6', () => { price: '1', metadata: { ...AAVE_METADATA, - protocolName: 'Aave V3 Borrow', + productName: 'Aave V3 Borrow', positionType: 'lending', }, }, @@ -345,10 +346,10 @@ describe('groupDeFiPositionsV6', () => { const [group] = groupDeFiPositionsV6(response)['account-1']; - expect(group.protocolName).toBe('Aave V3 Supply'); + expect(group.productName).toBe('Aave V3 Supply'); expect(group.sections).toStrictEqual([ { - protocolName: 'Aave V3 Supply', + productName: 'Aave V3 Supply', positions: [ expect.objectContaining({ symbol: 'WETH', @@ -357,7 +358,7 @@ describe('groupDeFiPositionsV6', () => { ], }, { - protocolName: 'Aave V3 Borrow', + productName: 'Aave V3 Borrow', positions: [ expect.objectContaining({ symbol: 'USDC', @@ -453,8 +454,57 @@ describe('groupDeFiPositionsV6', () => { marketValue: 3000, positionType: 'deposit', poolAddress: '0xpool', + groupId: 'group-aave-1', tokenImage: 'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/erc20/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2.png', }); }); + + it('keeps distinct groupIds on positions that share a productName', () => { + const response = buildResponse([ + { + accountId: 'account-1', + balances: [ + { + category: 'defi', + assetId: WETH_ASSET_ID, + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '2000', + metadata: { + ...AAVE_METADATA, + productName: 'Pendle YT', + groupId: 'group-yt-1', + }, + }, + { + category: 'defi', + assetId: USDC_ASSET_ID, + name: 'USD Coin', + symbol: 'USDC', + decimals: 6, + balance: '100', + price: '1', + metadata: { + ...AAVE_METADATA, + productName: 'Pendle YT', + poolAddress: '0xpool2', + groupId: 'group-yt-2', + }, + }, + ], + }, + ]); + + const [group] = groupDeFiPositionsV6(response)['account-1']; + + expect(group.sections).toHaveLength(1); + expect(group.sections[0].productName).toBe('Pendle YT'); + expect(group.sections[0].positions.map((p) => p.groupId)).toStrictEqual([ + 'group-yt-1', + 'group-yt-2', + ]); + }); }); diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index e3a7f9cbb24..efc3572517f 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -63,19 +63,24 @@ export type DeFiUnderlyingPosition = { positionType: DeFiPositionType; /** Address of the pool this position belongs to. */ poolAddress: string; + /** + * Upstream grouping id from the API. Rows that share a `productName` can + * still carry distinct `groupId`s (e.g. multiple Pendle YT markets). + */ + groupId: string; /** Token icon URL, when one can be built for the asset. */ tokenImage?: string; }; /** * A section of the details page, grouping positions that share the same - * API `protocolName`. A single `protocolId` can have multiple names - * (different pools/products under one protocol), so a group may contain + * API `productName`. A single `protocolId` can have multiple products + * (different pools/markets under one protocol), so a group may contain * several sections. */ export type DeFiPositionDetailsSection = { - /** Section label from the API (`metadata.protocolName`). */ - protocolName: string; + /** Section label from the API (`metadata.productName`). */ + productName: string; positions: DeFiUnderlyingPosition[]; }; @@ -85,8 +90,12 @@ export type DeFiPositionDetailsSection = { */ export type DeFiProtocolPositionGroup = { protocolId: string; - /** Display name from the first position seen for this protocol. */ - protocolName: string; + /** + * Product name from the first position seen for this protocol. Prefer + * `protocolId` for the list-row title; use section `productName`s for + * per-product detail headings. + */ + productName: string; protocolIconUrl: string; chainId: CaipChainId; /** @@ -98,7 +107,7 @@ export type DeFiProtocolPositionGroup = { iconGroup: DeFiPositionIconGroupItem[]; /** * Detail sections consumed by the details page, one per distinct API - * `protocolName` under this `protocolId`. + * `productName` under this `protocolId`. */ sections: DeFiPositionDetailsSection[]; }; @@ -217,7 +226,7 @@ function toUnderlyingPosition( ): DeFiUnderlyingPosition { const assetId = balance.assetId as CaipAssetType; const { chainId } = parseCaipAssetType(assetId); - const { positionType, poolAddress } = balance.metadata; + const { positionType, poolAddress, groupId } = balance.metadata; return { assetId, @@ -229,6 +238,7 @@ function toUnderlyingPosition( marketValue: getMarketValue(balance), positionType, poolAddress, + groupId, tokenImage: getDefiTokenImageUrl(assetId), }; } @@ -286,14 +296,14 @@ export function groupDeFiPositionsV6( } const position = toUnderlyingPosition(balance); - const { protocolId, protocolName, protocolIconUrl } = balance.metadata; + const { protocolId, productName, protocolIconUrl } = balance.metadata; const groupKey = `${position.chainId}#${protocolId}`; let group = groupsByKey.get(groupKey); if (!group) { group = { protocolId, - protocolName, + productName, protocolIconUrl, chainId: position.chainId, marketValue: 0, @@ -315,11 +325,13 @@ export function groupDeFiPositionsV6( }); } + // Sections are keyed by productName; distinct groupIds under the same + // product remain available on each underlying position. let section = group.sections.find( - (item) => item.protocolName === protocolName, + (item) => item.productName === productName, ); if (!section) { - section = { protocolName, positions: [] }; + section = { productName, positions: [] }; group.sections.push(section); } section.positions.push(position); diff --git a/packages/core-backend/CHANGELOG.md b/packages/core-backend/CHANGELOG.md index ca5ca5f8bae..7f956df3302 100644 --- a/packages/core-backend/CHANGELOG.md +++ b/packages/core-backend/CHANGELOG.md @@ -10,9 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Export `V6_DEFI_POSITION_TYPES` and inferred `V6DeFiPositionType`, and type `V6BalanceMetadata.positionType` with the Accounts API v6 DeFi position module values (`deposit`, `lending`, `yield`, `liquidity_pool`, `staked`, `leveraged_farming`, `nft_staked`, `farming`, `locked`, `vesting`, `rewards`, `investment`) ([#9503](https://github.com/MetaMask/core/pull/9503)) +- Add `groupId` to `V6BalanceMetadata` to match Accounts API v6 DeFi metadata ([#9503](https://github.com/MetaMask/core/pull/9503)) ### Changed +- **BREAKING:** Rename `V6BalanceMetadata.protocolName` to `productName` to match the Accounts API v6 DeFi metadata field ([#9503](https://github.com/MetaMask/core/pull/9503)) - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392)) - Bump `@metamask/profile-sync-controller` from `^28.2.0` to `^28.3.0` ([#9463](https://github.com/MetaMask/core/pull/9463)) - Bump `@metamask/accounts-controller` from `^39.0.4` to `^39.0.5` ([#9470](https://github.com/MetaMask/core/pull/9470)) diff --git a/packages/core-backend/src/api/accounts/client.test.ts b/packages/core-backend/src/api/accounts/client.test.ts index bdd5ec32367..df1fb81eabc 100644 --- a/packages/core-backend/src/api/accounts/client.test.ts +++ b/packages/core-backend/src/api/accounts/client.test.ts @@ -292,7 +292,8 @@ describe('AccountsApiClient', () => { balance: '1.0', metadata: { protocolId: 'metamask', - protocolName: 'MetaMask Swaps', + productName: 'MetaMask Swaps', + groupId: 'group-1', description: 'MetaMask Swaps on ethereum', protocolUrl: 'https://metamask.io/', protocolIconUrl: 'https://example.com/icon.jpg', diff --git a/packages/core-backend/src/api/accounts/types.ts b/packages/core-backend/src/api/accounts/types.ts index 322477e198a..f5f955810ba 100644 --- a/packages/core-backend/src/api/accounts/types.ts +++ b/packages/core-backend/src/api/accounts/types.ts @@ -94,12 +94,13 @@ export type V6DeFiPositionType = (typeof V6_DEFI_POSITION_TYPES)[number]; */ export type V6BalanceMetadata = { protocolId: string; - protocolName: string; + productName: string; description: string; protocolUrl: string; protocolIconUrl: string; positionType: V6DeFiPositionType; poolAddress: string; + groupId: string; }; /** From 88c590582f73a6834ad8854715a6a5923c90e341 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Thu, 16 Jul 2026 16:07:41 +0100 Subject: [PATCH 19/36] empty From 2a1338c880f97957411e1d81165642ac24d1d7a8 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Thu, 16 Jul 2026 16:11:49 +0100 Subject: [PATCH 20/36] force address --- .../DeFiPositionsControllerV2.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 71066947830..e271bf63fcc 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -11,6 +11,7 @@ import type { Messenger } from '@metamask/messenger'; import { buildDeFiBalancesQuery, DEFI_BALANCES_V6_REQUEST_OPTIONS, + normalizeCaipAccountId, toAccountMatchKey, } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; @@ -212,6 +213,17 @@ export class DeFiPositionsControllerV2 extends BaseController< return; } + // TEMPORARY: always fetch this account for testing, regardless of selection. + // Positions are still stored under the selected account's internal ID. + const TEST_CAIP_ACCOUNT_ID = + 'eip155:0:0x3e8734ec146c981e3ed1f6b582d447dde701d90c'; + const selectedInternalAccountId = [...internalAccountIdByCaip.values()][0]; + internalAccountIdByCaip.clear(); + internalAccountIdByCaip.set( + normalizeCaipAccountId(TEST_CAIP_ACCOUNT_ID), + selectedInternalAccountId, + ); + const accountIds = [...internalAccountIdByCaip.keys()]; // Stable key so the same account set throttles together regardless of map // iteration order. From 3401cedc38eb899e6a335ec35f417f67773fce43 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Mon, 20 Jul 2026 13:12:01 +0100 Subject: [PATCH 21/36] move common code --- .../merge-positions-for-accounts.test.ts | 209 ++++++++++++++++++ .../merge-positions-for-accounts.ts | 89 ++++++++ packages/assets-controllers/src/index.ts | 4 + 3 files changed, 302 insertions(+) create mode 100644 packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts create mode 100644 packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts new file mode 100644 index 00000000000..b2fcfad89d3 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts @@ -0,0 +1,209 @@ +import type { CaipAssetType, CaipChainId } from '@metamask/utils'; + +import type { + DeFiProtocolPositionGroup, + DeFiUnderlyingPosition, +} from './group-defi-positions-v6'; +import { + mergePositionsForAccounts, + mergeSections, +} from './merge-positions-for-accounts'; + +const ETH_MAINNET = 'eip155:1' as CaipChainId; +const BASE = 'eip155:8453' as CaipChainId; + +const WETH_ASSET_ID = + 'eip155:1/erc20:0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2' as CaipAssetType; +const USDC_ASSET_ID = + 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' as CaipAssetType; + +/** + * Builds an underlying position for tests. + * + * @param overrides - Fields to override on the default position. + * @returns An underlying position. + */ +function buildPosition( + overrides: Partial = {}, +): DeFiUnderlyingPosition { + return { + assetId: WETH_ASSET_ID, + chainId: ETH_MAINNET, + symbol: 'WETH', + name: 'Wrapped Ether', + decimals: 18, + balance: '1', + marketValue: 2000, + positionType: 'deposit', + poolAddress: '0xpool', + groupId: 'group-1', + tokenImage: 'https://example.com/weth.png', + ...overrides, + }; +} + +/** + * Builds a protocol position group for tests. + * + * @param overrides - Fields to override on the default group. + * @returns A protocol position group. + */ +function buildGroup( + overrides: Partial = {}, +): DeFiProtocolPositionGroup { + return { + protocolId: 'aave-v3', + productName: 'Aave V3', + protocolIconUrl: 'https://example.com/aave.png', + chainId: ETH_MAINNET, + marketValue: 2000, + iconGroup: [{ symbol: 'WETH', avatarValue: 'https://example.com/weth.png' }], + sections: [{ productName: 'Aave V3', positions: [buildPosition()] }], + ...overrides, + }; +} + +describe('mergePositionsForAccounts', () => { + it('returns an empty list when no accounts have positions', () => { + expect(mergePositionsForAccounts({}, ['account-1'])).toStrictEqual([]); + }); + + it('returns a single account’s groups unchanged in content', () => { + const group = buildGroup(); + + const result = mergePositionsForAccounts({ 'account-1': [group] }, [ + 'account-1', + ]); + + expect(result).toStrictEqual([group]); + }); + + it('does not mutate the source groups held in state', () => { + const group = buildGroup(); + const positionsByAccount = { 'account-1': [group] }; + + const result = mergePositionsForAccounts(positionsByAccount, ['account-1']); + result[0].marketValue = 999; + result[0].iconGroup.push({ symbol: 'HACK' }); + + expect(group.marketValue).toBe(2000); + expect(group.iconGroup).toHaveLength(1); + }); + + it('keeps groups on the same protocol but different chains separate', () => { + const ethGroup = buildGroup({ chainId: ETH_MAINNET }); + const baseGroup = buildGroup({ chainId: BASE }); + + const result = mergePositionsForAccounts( + { 'account-1': [ethGroup], 'account-2': [baseGroup] }, + ['account-1', 'account-2'], + ); + + expect(result).toHaveLength(2); + expect(result.map((group) => group.chainId)).toStrictEqual([ + ETH_MAINNET, + BASE, + ]); + }); + + it('merges groups that share chain and protocol across accounts', () => { + const groupA = buildGroup({ + marketValue: 2000, + iconGroup: [{ symbol: 'WETH' }], + sections: [ + { productName: 'Aave V3', positions: [buildPosition({ symbol: 'WETH' })] }, + ], + }); + const groupB = buildGroup({ + marketValue: 500, + iconGroup: [{ symbol: 'USDC' }], + sections: [ + { + productName: 'Aave V3', + positions: [buildPosition({ symbol: 'USDC', assetId: USDC_ASSET_ID })], + }, + ], + }); + + const result = mergePositionsForAccounts( + { 'account-1': [groupA], 'account-2': [groupB] }, + ['account-1', 'account-2'], + ); + + expect(result).toHaveLength(1); + expect(result[0].marketValue).toBe(2500); + expect(result[0].iconGroup.map((icon) => icon.symbol)).toStrictEqual([ + 'WETH', + 'USDC', + ]); + expect(result[0].sections).toHaveLength(1); + expect(result[0].sections[0].positions).toHaveLength(2); + }); + + it('deduplicates icon entries that share a symbol when merging', () => { + const groupA = buildGroup({ iconGroup: [{ symbol: 'WETH' }] }); + const groupB = buildGroup({ iconGroup: [{ symbol: 'WETH' }] }); + + const result = mergePositionsForAccounts( + { 'account-1': [groupA], 'account-2': [groupB] }, + ['account-1', 'account-2'], + ); + + expect(result[0].iconGroup).toHaveLength(1); + }); + + it('ignores account IDs that are not in the selected group', () => { + const result = mergePositionsForAccounts( + { 'account-1': [buildGroup()], 'account-2': [buildGroup()] }, + ['account-1'], + ); + + expect(result).toHaveLength(1); + }); +}); + +describe('mergeSections', () => { + it('appends positions to the section that shares a productName', () => { + const existing = [ + { productName: 'Aave V3', positions: [buildPosition({ symbol: 'WETH' })] }, + ]; + const incoming = [ + { productName: 'Aave V3', positions: [buildPosition({ symbol: 'USDC' })] }, + ]; + + const result = mergeSections(existing, incoming); + + expect(result).toHaveLength(1); + expect(result[0].positions).toHaveLength(2); + }); + + it('keeps sections with distinct productNames separate', () => { + const existing = [ + { productName: 'Aave V3', positions: [buildPosition()] }, + ]; + const incoming = [ + { productName: 'Pendle', positions: [buildPosition()] }, + ]; + + const result = mergeSections(existing, incoming); + + expect(result.map((section) => section.productName)).toStrictEqual([ + 'Aave V3', + 'Pendle', + ]); + }); + + it('does not mutate the input sections', () => { + const existing = [ + { productName: 'Aave V3', positions: [buildPosition()] }, + ]; + const incoming = [ + { productName: 'Aave V3', positions: [buildPosition()] }, + ]; + + mergeSections(existing, incoming); + + expect(existing[0].positions).toHaveLength(1); + expect(incoming[0].positions).toHaveLength(1); + }); +}); diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts new file mode 100644 index 00000000000..39bdacf5627 --- /dev/null +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts @@ -0,0 +1,89 @@ +import type { + DeFiPositionDetailsSection, + DeFiPositionsByAccount, + DeFiProtocolPositionGroup, +} from './group-defi-positions-v6'; + +/** + * Merges details-page sections that share the same `productName`, appending + * positions rather than keeping them as separate adjacent sections. + * + * @param existingSections - Sections already collected for this protocol group. + * @param incomingSections - Sections from another account holding the same + * protocol, to be merged in. + * @returns The merged sections, one per distinct `productName`. + */ +export function mergeSections( + existingSections: DeFiPositionDetailsSection[], + incomingSections: DeFiPositionDetailsSection[], +): DeFiPositionDetailsSection[] { + const byProductName = new Map( + existingSections.map((section) => [ + section.productName, + { ...section, positions: [...section.positions] }, + ]), + ); + + for (const section of incomingSections) { + const existing = byProductName.get(section.productName); + + if (!existing) { + byProductName.set(section.productName, { + ...section, + positions: [...section.positions], + }); + continue; + } + + existing.positions.push(...section.positions); + } + + return [...byProductName.values()]; +} + +/** + * Merges the protocol groups of every account in the selected group into a + * single flat list, combining groups that share the same chain and protocol. + * + * The controller stores DeFi positions keyed per internal account, but every + * client surface consumes the selected account group as a single merged list. + * This helper is exported so both clients share one implementation rather than + * each maintaining a copy. + * + * @param positionsByAccount - DeFi positions keyed by internal account ID. + * @param accountIds - Internal account IDs in the selected account group. + * @returns The merged protocol groups. + */ +export function mergePositionsForAccounts( + positionsByAccount: DeFiPositionsByAccount, + accountIds: string[], +): DeFiProtocolPositionGroup[] { + const byKey = new Map(); + + for (const accountId of accountIds) { + for (const group of positionsByAccount[accountId] ?? []) { + const key = `${group.chainId}#${group.protocolId}`; + const existing = byKey.get(key); + + if (!existing) { + // Clone so we never mutate the object held in client state. + byKey.set(key, { + ...group, + iconGroup: [...group.iconGroup], + sections: [...group.sections], + }); + continue; + } + + existing.marketValue += group.marketValue; + for (const icon of group.iconGroup) { + if (!existing.iconGroup.some((item) => item.symbol === icon.symbol)) { + existing.iconGroup.push(icon); + } + } + existing.sections = mergeSections(existing.sections, group.sections); + } + } + + return [...byKey.values()]; +} diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index c0f9e86f6a8..fab37fb61b3 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -285,6 +285,10 @@ export type { DeFiPositionType, } from './DeFiPositionsController/DeFiPositionsControllerV2'; export type { DeFiPositionsControllerV2FetchDeFiPositionsAction } from './DeFiPositionsController/DeFiPositionsControllerV2-method-action-types'; +export { + mergePositionsForAccounts, + mergeSections, +} from './DeFiPositionsController/merge-positions-for-accounts'; export { DEFI_SUPPORTED_NETWORKS, buildDeFiBalancesQuery, From 5513f5df1175f49403f8cdb6255d3905e9b65fa1 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 11:28:00 +0100 Subject: [PATCH 22/36] force refresh option --- ...ositionsControllerV2-method-action-types.ts | 7 ++++++- .../DeFiPositionsControllerV2.test.ts | 16 ++++++++++++++++ .../DeFiPositionsControllerV2.ts | 18 +++++++++++++++--- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index 1077d1da23c..3a5d3c607fd 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -13,7 +13,12 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; * * Throttled per set of accounts by an in-memory minimum interval, so repeated * calls within the window are no-ops (no HTTP, no regroup, no state write). - * Disabled controllers and empty account groups return without fetching. + * Pass `{ forceRefresh: true }` to bypass that throttle. Disabled controllers + * and empty account groups return without fetching. + * + * @param options - Optional fetch modifiers. + * @param options.forceRefresh - When true, bypass the minimum-interval + * throttle and fetch immediately. */ export type DeFiPositionsControllerV2FetchDeFiPositionsAction = { type: `DeFiPositionsControllerV2:fetchDeFiPositions`; diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index f8bfd293f21..581fc601899 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -396,6 +396,22 @@ describe('DeFiPositionsControllerV2', () => { expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); + it('bypasses the throttle when forceRefresh is true', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + minimumFetchIntervalMs: 60_000, + }); + + await controller.fetchDeFiPositions(); + await controller.fetchDeFiPositions({ forceRefresh: true }); + + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + + // A subsequent non-forced call within the interval is still throttled, + // keyed from the forceRefresh fetch timestamp. + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + }); + it('clears the throttle claim when a fetch fails so retries are allowed', async () => { const consoleErrorSpy = jest .spyOn(console, 'error') diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index e271bf63fcc..3006e4712eb 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -195,9 +195,19 @@ export class DeFiPositionsControllerV2 extends BaseController< * * Throttled per set of accounts by an in-memory minimum interval, so repeated * calls within the window are no-ops (no HTTP, no regroup, no state write). - * Disabled controllers and empty account groups return without fetching. + * Pass `{ forceRefresh: true }` to bypass that throttle (e.g. pull-to-refresh + * or after a confirmed transaction). The successful/forced fetch still + * updates the throttle timestamp, so subsequent non-forced calls remain + * gated. Disabled controllers and empty account groups return without + * fetching. + * + * @param options - Optional fetch modifiers. + * @param options.forceRefresh - When true, bypass the minimum-interval + * throttle and fetch immediately. */ - async fetchDeFiPositions(): Promise { + async fetchDeFiPositions(options?: { + forceRefresh?: boolean; + }): Promise { if (!this.#isEnabled()) { return; } @@ -231,6 +241,7 @@ export class DeFiPositionsControllerV2 extends BaseController< const now = Date.now(); const lastFetchedAt = this.#lastFetchByKey.get(throttleKey); if ( + !options?.forceRefresh && lastFetchedAt !== undefined && now - lastFetchedAt < this.#minimumFetchIntervalMs ) { @@ -238,7 +249,8 @@ export class DeFiPositionsControllerV2 extends BaseController< } // Claim the slot before awaiting so a second call that arrives while the // first is in flight is also dropped (TanStack would share that promise; - // we intentionally skip instead). + // we intentionally skip instead). forceRefresh also claims the slot so + // follow-up non-forced calls within the interval stay throttled. this.#lastFetchByKey.set(throttleKey, now); try { From 29793f7b0827468e929e59735ef9a57af582b8dc Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 11:28:56 +0100 Subject: [PATCH 23/36] linting --- .../merge-positions-for-accounts.test.ts | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts index b2fcfad89d3..cb19cb4e769 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts @@ -57,7 +57,9 @@ function buildGroup( protocolIconUrl: 'https://example.com/aave.png', chainId: ETH_MAINNET, marketValue: 2000, - iconGroup: [{ symbol: 'WETH', avatarValue: 'https://example.com/weth.png' }], + iconGroup: [ + { symbol: 'WETH', avatarValue: 'https://example.com/weth.png' }, + ], sections: [{ productName: 'Aave V3', positions: [buildPosition()] }], ...overrides, }; @@ -111,7 +113,10 @@ describe('mergePositionsForAccounts', () => { marketValue: 2000, iconGroup: [{ symbol: 'WETH' }], sections: [ - { productName: 'Aave V3', positions: [buildPosition({ symbol: 'WETH' })] }, + { + productName: 'Aave V3', + positions: [buildPosition({ symbol: 'WETH' })], + }, ], }); const groupB = buildGroup({ @@ -120,7 +125,9 @@ describe('mergePositionsForAccounts', () => { sections: [ { productName: 'Aave V3', - positions: [buildPosition({ symbol: 'USDC', assetId: USDC_ASSET_ID })], + positions: [ + buildPosition({ symbol: 'USDC', assetId: USDC_ASSET_ID }), + ], }, ], }); @@ -165,10 +172,16 @@ describe('mergePositionsForAccounts', () => { describe('mergeSections', () => { it('appends positions to the section that shares a productName', () => { const existing = [ - { productName: 'Aave V3', positions: [buildPosition({ symbol: 'WETH' })] }, + { + productName: 'Aave V3', + positions: [buildPosition({ symbol: 'WETH' })], + }, ]; const incoming = [ - { productName: 'Aave V3', positions: [buildPosition({ symbol: 'USDC' })] }, + { + productName: 'Aave V3', + positions: [buildPosition({ symbol: 'USDC' })], + }, ]; const result = mergeSections(existing, incoming); @@ -178,12 +191,8 @@ describe('mergeSections', () => { }); it('keeps sections with distinct productNames separate', () => { - const existing = [ - { productName: 'Aave V3', positions: [buildPosition()] }, - ]; - const incoming = [ - { productName: 'Pendle', positions: [buildPosition()] }, - ]; + const existing = [{ productName: 'Aave V3', positions: [buildPosition()] }]; + const incoming = [{ productName: 'Pendle', positions: [buildPosition()] }]; const result = mergeSections(existing, incoming); @@ -194,12 +203,8 @@ describe('mergeSections', () => { }); it('does not mutate the input sections', () => { - const existing = [ - { productName: 'Aave V3', positions: [buildPosition()] }, - ]; - const incoming = [ - { productName: 'Aave V3', positions: [buildPosition()] }, - ]; + const existing = [{ productName: 'Aave V3', positions: [buildPosition()] }]; + const incoming = [{ productName: 'Aave V3', positions: [buildPosition()] }]; mergeSections(existing, incoming); From 924c21f38cfd143d1eb062e9fcf45cccb9426200 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 11:36:47 +0100 Subject: [PATCH 24/36] fixes --- .../DeFiPositionsControllerV2.ts | 12 ------------ .../merge-positions-for-accounts.test.ts | 4 ++++ .../merge-positions-for-accounts.ts | 2 +- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 3006e4712eb..cc6cbb73090 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -11,7 +11,6 @@ import type { Messenger } from '@metamask/messenger'; import { buildDeFiBalancesQuery, DEFI_BALANCES_V6_REQUEST_OPTIONS, - normalizeCaipAccountId, toAccountMatchKey, } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; @@ -223,17 +222,6 @@ export class DeFiPositionsControllerV2 extends BaseController< return; } - // TEMPORARY: always fetch this account for testing, regardless of selection. - // Positions are still stored under the selected account's internal ID. - const TEST_CAIP_ACCOUNT_ID = - 'eip155:0:0x3e8734ec146c981e3ed1f6b582d447dde701d90c'; - const selectedInternalAccountId = [...internalAccountIdByCaip.values()][0]; - internalAccountIdByCaip.clear(); - internalAccountIdByCaip.set( - normalizeCaipAccountId(TEST_CAIP_ACCOUNT_ID), - selectedInternalAccountId, - ); - const accountIds = [...internalAccountIdByCaip.keys()]; // Stable key so the same account set throttles together regardless of map // iteration order. diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts index cb19cb4e769..615db5c2960 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts @@ -87,9 +87,13 @@ describe('mergePositionsForAccounts', () => { const result = mergePositionsForAccounts(positionsByAccount, ['account-1']); result[0].marketValue = 999; result[0].iconGroup.push({ symbol: 'HACK' }); + result[0].sections.push({ productName: 'HACK', positions: [] }); + result[0].sections[0].positions.push(buildPosition({ symbol: 'HACK' })); expect(group.marketValue).toBe(2000); expect(group.iconGroup).toHaveLength(1); + expect(group.sections).toHaveLength(1); + expect(group.sections[0].positions).toHaveLength(1); }); it('keeps groups on the same protocol but different chains separate', () => { diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts index 39bdacf5627..5395caf4044 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts @@ -70,7 +70,7 @@ export function mergePositionsForAccounts( byKey.set(key, { ...group, iconGroup: [...group.iconGroup], - sections: [...group.sections], + sections: mergeSections([], group.sections), }); continue; } From 0ce0de4f0a648412f90f08c08c41d18f63f78c43 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 11:45:20 +0100 Subject: [PATCH 25/36] type --- .../DeFiPositionsControllerV2-method-action-types.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index 3a5d3c607fd..815f62b89ec 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -13,8 +13,11 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; * * Throttled per set of accounts by an in-memory minimum interval, so repeated * calls within the window are no-ops (no HTTP, no regroup, no state write). - * Pass `{ forceRefresh: true }` to bypass that throttle. Disabled controllers - * and empty account groups return without fetching. + * Pass `{ forceRefresh: true }` to bypass that throttle (e.g. pull-to-refresh + * or after a confirmed transaction). The successful/forced fetch still + * updates the throttle timestamp, so subsequent non-forced calls remain + * gated. Disabled controllers and empty account groups return without + * fetching. * * @param options - Optional fetch modifiers. * @param options.forceRefresh - When true, bypass the minimum-interval From 48d0b947fd34a7cbf0da88991326ed4159b92558 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 12:11:59 +0100 Subject: [PATCH 26/36] lint --- .../DeFiPositionsController/group-defi-positions-v6.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts index de095116137..b862053f086 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts @@ -502,7 +502,9 @@ describe('groupDeFiPositionsV6', () => { expect(group.sections).toHaveLength(1); expect(group.sections[0].productName).toBe('Pendle YT'); - expect(group.sections[0].positions.map((p) => p.groupId)).toStrictEqual([ + expect( + group.sections[0].positions.map((position) => position.groupId), + ).toStrictEqual([ 'group-yt-1', 'group-yt-2', ]); From 9f7c44f4e60e8a5998441fa5ab42172a01ddfbfa Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 12:58:19 +0100 Subject: [PATCH 27/36] linting --- .../DeFiPositionsController/group-defi-positions-v6.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts index b862053f086..c81cc840578 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts @@ -504,9 +504,6 @@ describe('groupDeFiPositionsV6', () => { expect(group.sections[0].productName).toBe('Pendle YT'); expect( group.sections[0].positions.map((position) => position.groupId), - ).toStrictEqual([ - 'group-yt-1', - 'group-yt-2', - ]); + ).toStrictEqual(['group-yt-1', 'group-yt-2']); }); }); From 11df970b2757cb9b880ef6a665bb3d2956da3ad0 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:11:48 +0100 Subject: [PATCH 28/36] move visualizer --- .../defi-balances-visualizer.html} | 6 ++++++ 1 file changed, 6 insertions(+) rename packages/assets-controllers/{src/DeFiPositionsController/defi-visualizer.html => devtools/defi-balances-visualizer.html} (99%) diff --git a/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html b/packages/assets-controllers/devtools/defi-balances-visualizer.html similarity index 99% rename from packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html rename to packages/assets-controllers/devtools/defi-balances-visualizer.html index ccf6b204982..1953ac24856 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/defi-visualizer.html +++ b/packages/assets-controllers/devtools/defi-balances-visualizer.html @@ -1,4 +1,10 @@ + From e0918cbb6ea5fca76016b6da90026302bb111ff0 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:20:31 +0100 Subject: [PATCH 29/36] better throttle --- ...sitionsControllerV2-method-action-types.ts | 16 ++- .../DeFiPositionsControllerV2.test.ts | 136 ++++++++++++++++++ .../DeFiPositionsControllerV2.ts | 82 +++++++---- 3 files changed, 200 insertions(+), 34 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index 815f62b89ec..445ce7f6b60 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -11,13 +11,15 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; * method: resolving the accounts, calling the Accounts API, transforming the * response, and updating state. * - * Throttled per set of accounts by an in-memory minimum interval, so repeated - * calls within the window are no-ops (no HTTP, no regroup, no state write). - * Pass `{ forceRefresh: true }` to bypass that throttle (e.g. pull-to-refresh - * or after a confirmed transaction). The successful/forced fetch still - * updates the throttle timestamp, so subsequent non-forced calls remain - * gated. Disabled controllers and empty account groups return without - * fetching. + * Throttled per set of accounts + vsCurrency by an in-memory minimum + * interval, so repeated calls within the window are no-ops (no HTTP, no + * regroup, no state write). A change in fiat currency bypasses that window + * so tab-focus refetches pick up new prices without `forceRefresh`. Pass + * `{ forceRefresh: true }` to bypass the throttle entirely (e.g. + * pull-to-refresh or after a confirmed transaction). The successful/forced + * fetch still updates the throttle claim, so subsequent non-forced calls for + * the same accounts and currency remain gated. Disabled controllers and + * empty account groups return without fetching. * * @param options - Optional fetch modifiers. * @param options.forceRefresh - When true, bypass the minimum-interval diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 581fc601899..976ea620b53 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -412,6 +412,34 @@ describe('DeFiPositionsControllerV2', () => { expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); + it('refetches when vsCurrency changes even without forceRefresh', async () => { + let vsCurrency = 'USD'; + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + minimumFetchIntervalMs: 60_000, + getVsCurrency: () => vsCurrency, + }); + + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); + expect(mockFetchV6MultiAccountBalances).toHaveBeenLastCalledWith( + expect.any(Array), + expect.objectContaining({ vsCurrency: 'usd' }), + ); + + vsCurrency = 'EUR'; + await controller.fetchDeFiPositions(); + + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + expect(mockFetchV6MultiAccountBalances).toHaveBeenLastCalledWith( + expect.any(Array), + expect.objectContaining({ vsCurrency: 'eur' }), + ); + + // Same currency again within the interval stays throttled. + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + }); + it('clears the throttle claim when a fetch fails so retries are allowed', async () => { const consoleErrorSpy = jest .spyOn(console, 'error') @@ -440,6 +468,114 @@ describe('DeFiPositionsControllerV2', () => { ); }); + it('does not let a slower older response overwrite a newer forceRefresh result', async () => { + let resolveFirst!: (value: V6BalancesResponse) => void; + let resolveSecond!: (value: V6BalancesResponse) => void; + const firstPending = new Promise((resolve) => { + resolveFirst = resolve; + }); + const secondPending = new Promise((resolve) => { + resolveSecond = resolve; + }); + + const staleResponse = buildMockBalancesResponse(); + const freshResponse = buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS}`, + balances: [ + { + category: 'defi', + assetId: + 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', + name: 'Wrapped Ether', + symbol: 'WETH', + decimals: 18, + balance: '1', + price: '3000', + metadata: { + protocolId: 'aave-v3', + productName: 'Aave V3', + description: 'Aave V3 on ethereum', + protocolUrl: 'https://aave.com/', + protocolIconUrl: 'https://example.com/aave.png', + positionType: 'deposit', + poolAddress: '0xpool', + groupId: 'group-aave-1', + }, + }, + ], + }, + ], + }); + + const mockFetchV6MultiAccountBalances = jest + .fn() + .mockReturnValueOnce(firstPending) + .mockReturnValueOnce(secondPending); + + const { controller } = setupController({ + mockFetchV6MultiAccountBalances, + }); + + const firstFetch = controller.fetchDeFiPositions(); + const secondFetch = controller.fetchDeFiPositions({ forceRefresh: true }); + + resolveSecond(freshResponse); + await secondFetch; + expect( + controller.state.allDeFiPositionsV2['evm-account-id'][0], + ).toMatchObject({ marketValue: 3000 }); + + resolveFirst(staleResponse); + await firstFetch; + expect( + controller.state.allDeFiPositionsV2['evm-account-id'][0], + ).toMatchObject({ marketValue: 3000 }); + }); + + it('does not clear a newer fetch throttle claim when an older overlapping fetch fails', async () => { + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + let rejectFirst!: (reason?: unknown) => void; + let resolveSecond!: (value: V6BalancesResponse) => void; + const firstPending = new Promise((_resolve, reject) => { + rejectFirst = reject; + }); + const secondPending = new Promise((resolve) => { + resolveSecond = resolve; + }); + + const mockFetchV6MultiAccountBalances = jest + .fn() + .mockReturnValueOnce(firstPending) + .mockReturnValueOnce(secondPending); + + const { controller } = setupController({ + mockFetchV6MultiAccountBalances, + minimumFetchIntervalMs: 60_000, + }); + + const firstFetch = controller.fetchDeFiPositions(); + const secondFetch = controller.fetchDeFiPositions({ forceRefresh: true }); + + rejectFirst(new Error('network error')); + await firstFetch; + + resolveSecond(buildMockBalancesResponse()); + await secondFetch; + + // Older failure must not wipe the newer claim, or this would fetch again. + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to fetch DeFi positions', + expect.any(Error), + ); + }); + it('exposes fetchDeFiPositions via the messenger', async () => { const { controllerMessenger, mockFetchV6MultiAccountBalances } = setupController(); diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index cc6cbb73090..63aae8287fe 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -128,18 +128,28 @@ export class DeFiPositionsControllerV2 extends BaseController< readonly #minimumFetchIntervalMs: number; /** - * In-memory timestamp (ms) of the last fetch per set of accounts. + * In-memory fetch claim per set of accounts. * - * This is a controller-level gate, separate from TanStack's `staleTime` inside - * `fetchV6MultiAccountBalances`: when the interval has not elapsed we - * early-return without regrouping or writing state. TanStack still dedupes - * in-flight HTTP for identical query keys; this Map skips that work entirely. + * Controller-level gate, separate from TanStack's `staleTime` inside + * `fetchV6MultiAccountBalances`: when the interval has not elapsed for the + * same accounts *and* currency we early-return without regrouping or writing + * state. TanStack still dedupes in-flight HTTP for identical query keys; this + * Map skips that work entirely. * - * Keyed by sorted CAIP account IDs only (not networks / vsCurrency). - * Intentionally not persisted: resets on restart, so the first fetch after a - * restart always goes through. + * Keyed by sorted CAIP account IDs only (not networks). A currency change + * invalidates the throttle for that account set so a plain + * `fetchDeFiPositions()` (e.g. returning to the DeFi tab) refetches prices + * without needing `forceRefresh`. + * + * `generation` identifies the latest claim for a key so overlapping + * `forceRefresh` calls discard stale responses (and older failures do not + * clear a newer claim). Intentionally not persisted: resets on restart, so + * the first fetch after a restart always goes through. */ - readonly #lastFetchByKey = new Map(); + readonly #lastFetchByKey = new Map< + string, + { fetchedAt: number; vsCurrency: string; generation: number } + >(); /** * @param options - Constructor options. @@ -192,13 +202,15 @@ export class DeFiPositionsControllerV2 extends BaseController< * method: resolving the accounts, calling the Accounts API, transforming the * response, and updating state. * - * Throttled per set of accounts by an in-memory minimum interval, so repeated - * calls within the window are no-ops (no HTTP, no regroup, no state write). - * Pass `{ forceRefresh: true }` to bypass that throttle (e.g. pull-to-refresh - * or after a confirmed transaction). The successful/forced fetch still - * updates the throttle timestamp, so subsequent non-forced calls remain - * gated. Disabled controllers and empty account groups return without - * fetching. + * Throttled per set of accounts + vsCurrency by an in-memory minimum + * interval, so repeated calls within the window are no-ops (no HTTP, no + * regroup, no state write). A change in fiat currency bypasses that window + * so tab-focus refetches pick up new prices without `forceRefresh`. Pass + * `{ forceRefresh: true }` to bypass the throttle entirely (e.g. + * pull-to-refresh or after a confirmed transaction). The successful/forced + * fetch still updates the throttle claim, so subsequent non-forced calls for + * the same accounts and currency remain gated. Disabled controllers and + * empty account groups return without fetching. * * @param options - Optional fetch modifiers. * @param options.forceRefresh - When true, bypass the minimum-interval @@ -226,20 +238,28 @@ export class DeFiPositionsControllerV2 extends BaseController< // Stable key so the same account set throttles together regardless of map // iteration order. const throttleKey = [...accountIds].sort().join(','); + const vsCurrency = this.#getVsCurrency().toLowerCase(); const now = Date.now(); - const lastFetchedAt = this.#lastFetchByKey.get(throttleKey); + const lastFetch = this.#lastFetchByKey.get(throttleKey); if ( !options?.forceRefresh && - lastFetchedAt !== undefined && - now - lastFetchedAt < this.#minimumFetchIntervalMs + lastFetch?.vsCurrency === vsCurrency && + now - lastFetch.fetchedAt < this.#minimumFetchIntervalMs ) { return; } - // Claim the slot before awaiting so a second call that arrives while the - // first is in flight is also dropped (TanStack would share that promise; - // we intentionally skip instead). forceRefresh also claims the slot so - // follow-up non-forced calls within the interval stay throttled. - this.#lastFetchByKey.set(throttleKey, now); + // Claim the slot before awaiting so a second non-forced call that arrives + // while the first is in flight is also dropped (TanStack would share that + // promise; we intentionally skip instead). forceRefresh also claims the + // slot so follow-up non-forced calls within the interval stay throttled. + // Bump generation so overlapping forceRefresh calls can discard stale + // responses that finish out of order. + const fetchGeneration = (lastFetch?.generation ?? 0) + 1; + this.#lastFetchByKey.set(throttleKey, { + fetchedAt: now, + vsCurrency, + generation: fetchGeneration, + }); try { const response = @@ -250,9 +270,14 @@ export class DeFiPositionsControllerV2 extends BaseController< forceFetchDeFiPositions: DEFI_BALANCES_V6_REQUEST_OPTIONS.forceFetchDeFiPositions, includePrices: DEFI_BALANCES_V6_REQUEST_OPTIONS.includePrices, - vsCurrency: this.#getVsCurrency().toLowerCase(), + vsCurrency, }); + if (this.#lastFetchByKey.get(throttleKey)?.generation !== fetchGeneration) { + // A newer fetch for this account set has already claimed the slot. + return; + } + // The v6 response echoes a per-chain CAIP-10 ID for every chain // (`eip155:1:`, `eip155:137:`, ...), while we requested with // the all-chains reference (`eip155:0:`). Match on namespace + @@ -283,8 +308,11 @@ export class DeFiPositionsControllerV2 extends BaseController< } }); } catch (error) { - // Clear the claim so a failed fetch does not burn the throttle window. - this.#lastFetchByKey.delete(throttleKey); + // Only the latest attempt may clear the claim; an older failure must not + // reopen the throttle window for a newer in-flight or completed fetch. + if (this.#lastFetchByKey.get(throttleKey)?.generation === fetchGeneration) { + this.#lastFetchByKey.delete(throttleKey); + } console.error('Failed to fetch DeFi positions', error); } From b2e379e80c8f07c3d842c19a326d0c7e65e56584 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:21:21 +0100 Subject: [PATCH 30/36] linting --- .../DeFiPositionsController/DeFiPositionsControllerV2.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 63aae8287fe..efe448c6e42 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -273,7 +273,9 @@ export class DeFiPositionsControllerV2 extends BaseController< vsCurrency, }); - if (this.#lastFetchByKey.get(throttleKey)?.generation !== fetchGeneration) { + if ( + this.#lastFetchByKey.get(throttleKey)?.generation !== fetchGeneration + ) { // A newer fetch for this account set has already claimed the slot. return; } @@ -310,7 +312,9 @@ export class DeFiPositionsControllerV2 extends BaseController< } catch (error) { // Only the latest attempt may clear the claim; an older failure must not // reopen the throttle window for a newer in-flight or completed fetch. - if (this.#lastFetchByKey.get(throttleKey)?.generation === fetchGeneration) { + if ( + this.#lastFetchByKey.get(throttleKey)?.generation === fetchGeneration + ) { this.#lastFetchByKey.delete(throttleKey); } console.error('Failed to fetch DeFi positions', error); From aa9ab2a7521f24e2792a93d894da3643e95f1b30 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:28:25 +0100 Subject: [PATCH 31/36] remove settings constant --- .../DeFiPositionsControllerV2.test.ts | 13 +++++++------ .../DeFiPositionsControllerV2.ts | 9 +++------ .../build-defi-balances-query.test.ts | 9 --------- .../build-defi-balances-query.ts | 12 ------------ 4 files changed, 10 insertions(+), 33 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 976ea620b53..fff23154b29 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -20,10 +20,7 @@ import type { } from '@metamask/messenger'; import { createMockInternalAccount } from '../../../accounts-controller/tests/mocks'; -import { - DEFI_BALANCES_V6_REQUEST_OPTIONS, - DEFI_SUPPORTED_NETWORKS, -} from './build-defi-balances-query'; +import { DEFI_SUPPORTED_NETWORKS } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2Messenger } from './DeFiPositionsControllerV2'; import { DeFiPositionsControllerV2, @@ -267,7 +264,9 @@ describe('DeFiPositionsControllerV2', () => { networks: DEFI_SUPPORTED_NETWORKS.filter((network) => network.startsWith('eip155:'), ), - ...DEFI_BALANCES_V6_REQUEST_OPTIONS, + includeDeFiBalances: true, + forceFetchDeFiPositions: true, + includePrices: true, vsCurrency: 'usd', }, ); @@ -370,7 +369,9 @@ describe('DeFiPositionsControllerV2', () => { ], { networks: [...expectedEvmNetworks, ...expectedSolanaNetworks], - ...DEFI_BALANCES_V6_REQUEST_OPTIONS, + includeDeFiBalances: true, + forceFetchDeFiPositions: true, + includePrices: true, vsCurrency: 'usd', }, ); diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index efe448c6e42..df46c24df84 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -10,7 +10,6 @@ import type { Messenger } from '@metamask/messenger'; import { buildDeFiBalancesQuery, - DEFI_BALANCES_V6_REQUEST_OPTIONS, toAccountMatchKey, } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; @@ -265,11 +264,9 @@ export class DeFiPositionsControllerV2 extends BaseController< const response = await this.#apiClient.accounts.fetchV6MultiAccountBalances(accountIds, { networks, - includeDeFiBalances: - DEFI_BALANCES_V6_REQUEST_OPTIONS.includeDeFiBalances, - forceFetchDeFiPositions: - DEFI_BALANCES_V6_REQUEST_OPTIONS.forceFetchDeFiPositions, - includePrices: DEFI_BALANCES_V6_REQUEST_OPTIONS.includePrices, + includeDeFiBalances: true, + forceFetchDeFiPositions: true, + includePrices: true, vsCurrency, }); diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts index 4ff039434b9..6cedffe8348 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts @@ -13,7 +13,6 @@ import type { InternalAccount } from '@metamask/keyring-internal-api'; import { createMockInternalAccount } from '../../../accounts-controller/tests/mocks'; import { buildDeFiBalancesQuery, - DEFI_BALANCES_V6_REQUEST_OPTIONS, DEFI_SUPPORTED_NETWORKS, normalizeCaipAccountId, } from './build-defi-balances-query'; @@ -69,14 +68,6 @@ describe('normalizeCaipAccountId', () => { }); describe('buildDeFiBalancesQuery', () => { - it('exports the fixed v6 request options used by the controller', () => { - expect(DEFI_BALANCES_V6_REQUEST_OPTIONS).toStrictEqual({ - includeDeFiBalances: true, - forceFetchDeFiPositions: true, - includePrices: true, - }); - }); - it('builds an EVM CAIP account spanning all supported EVM networks', () => { const result = buildDeFiBalancesQuery([mockEvmAccount, mockBtcAccount]); diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index 5d35c1a2a4b..d6f66d33566 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -35,18 +35,6 @@ export const DEFI_SUPPORTED_NETWORKS: readonly CaipChainId[] = [ const SOLANA_MAINNET_CAIP_CHAIN_ID: CaipChainId = SolScope.Mainnet; -/** - * Fixed request flags used against the v6 multiaccount balances endpoint so the - * response only carries what the DeFi views need (positions + prices). - * Fiat currency (`vsCurrency`) is not fixed here — it comes from - * AssetsController `selectedCurrency` at fetch time. - */ -export const DEFI_BALANCES_V6_REQUEST_OPTIONS = { - includeDeFiBalances: true, - forceFetchDeFiPositions: true, - includePrices: true, -} as const; - export type DeFiBalancesQuery = { /** CAIP-2 networks to query, deduped across accounts. */ networks: CaipChainId[]; From e0593b4eb45eaa763ba639967cbf66dc54699a5a Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:38:42 +0100 Subject: [PATCH 32/36] refactors --- .../DeFiPositionsControllerV2.test.ts | 34 +++++++- .../DeFiPositionsControllerV2.ts | 84 +++++-------------- .../build-defi-balances-query.test.ts | 27 ++---- .../build-defi-balances-query.ts | 75 ++++------------- .../group-defi-positions-v6.test.ts | 13 +-- .../group-defi-positions-v6.ts | 64 +++++++++++--- .../merge-positions-for-accounts.test.ts | 70 ++++++---------- .../merge-positions-for-accounts.ts | 2 +- packages/assets-controllers/src/index.ts | 17 ++-- 9 files changed, 173 insertions(+), 213 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index fff23154b29..84cff92a3f9 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -133,6 +133,8 @@ function buildMockBalancesResponse( * @param config.getVsCurrency - Fiat currency getter. * @param config.minimumFetchIntervalMs - Minimum fetch interval. * @param config.mockGroupAccounts - Accounts returned for the selected group. + * @param config.getGroupAccounts - Getter for the selected group accounts + * (preferred when the selection changes between fetches). * @param config.mockFetchV6MultiAccountBalances - Mock API fetch function. * @param config.state - Initial controller state. * @returns The controller instance and mocks. @@ -142,6 +144,7 @@ function setupController({ getVsCurrency = (): string => 'USD', minimumFetchIntervalMs, mockGroupAccounts = GROUP_ACCOUNTS, + getGroupAccounts, mockFetchV6MultiAccountBalances = jest .fn() .mockResolvedValue(buildMockBalancesResponse()), @@ -151,6 +154,7 @@ function setupController({ getVsCurrency?: () => string; minimumFetchIntervalMs?: number; mockGroupAccounts?: InternalAccount[]; + getGroupAccounts?: () => InternalAccount[]; mockFetchV6MultiAccountBalances?: jest.Mock; state?: Partial>; } = {}): { @@ -169,7 +173,7 @@ function setupController({ messenger.registerActionHandler( 'AccountTreeController:getAccountsFromSelectedAccountGroup', - () => mockGroupAccounts, + () => getGroupAccounts?.() ?? mockGroupAccounts, ); const controllerMessenger = new Messenger< @@ -397,6 +401,34 @@ describe('DeFiPositionsControllerV2', () => { expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); + it('keeps an independent throttle TTL per account set', async () => { + const otherEvmAccount = createMockInternalAccount({ + id: 'evm-account-id-2', + address: '0x0000000000000000000000000000000000000002', + type: EthAccountType.Eoa, + }); + let groupAccounts: InternalAccount[] = GROUP_ACCOUNTS; + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + minimumFetchIntervalMs: 60_000, + getGroupAccounts: () => groupAccounts, + mockFetchV6MultiAccountBalances: jest + .fn() + .mockResolvedValue(buildMockBalancesResponse()), + }); + + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); + + groupAccounts = [otherEvmAccount]; + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + + // Returning to the first group within the window reuses its TTL. + groupAccounts = GROUP_ACCOUNTS; + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + }); + it('bypasses the throttle when forceRefresh is true', async () => { const { controller, mockFetchV6MultiAccountBalances } = setupController({ minimumFetchIntervalMs: 60_000, diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index df46c24df84..df61cf3203b 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -8,10 +8,7 @@ import type { import type { ApiPlatformClient } from '@metamask/core-backend'; import type { Messenger } from '@metamask/messenger'; -import { - buildDeFiBalancesQuery, - toAccountMatchKey, -} from './build-defi-balances-query'; +import { buildDeFiBalancesQuery } from './build-defi-balances-query'; import type { DeFiPositionsControllerV2MethodActions } from './DeFiPositionsControllerV2-method-action-types'; import type { DeFiPositionsByAccount } from './group-defi-positions-v6'; import { groupDeFiPositionsV6 } from './group-defi-positions-v6'; @@ -22,19 +19,6 @@ const ONE_MINUTE_IN_MS = 60_000; const MESSENGER_EXPOSED_METHODS = ['fetchDeFiPositions'] as const; -export type { - DeFiPositionsByAccount, - DeFiProtocolPositionGroup, - DeFiPositionDetailsSection, - DeFiUnderlyingPosition, - DeFiPositionIconGroupItem, - DeFiPositionType, -} from './group-defi-positions-v6'; -export { - DEFI_POSITION_TYPES, - DEFI_POSITION_LIABILITY_TYPES, -} from './group-defi-positions-v6'; - export type DeFiPositionsControllerV2State = { /** * DeFi positions keyed by internal MetaMask account ID (`InternalAccount.id`, @@ -127,23 +111,18 @@ export class DeFiPositionsControllerV2 extends BaseController< readonly #minimumFetchIntervalMs: number; /** - * In-memory fetch claim per set of accounts. + * In-memory fetch claim per account set. * * Controller-level gate, separate from TanStack's `staleTime` inside * `fetchV6MultiAccountBalances`: when the interval has not elapsed for the - * same accounts *and* currency we early-return without regrouping or writing - * state. TanStack still dedupes in-flight HTTP for identical query keys; this - * Map skips that work entirely. - * - * Keyed by sorted CAIP account IDs only (not networks). A currency change - * invalidates the throttle for that account set so a plain - * `fetchDeFiPositions()` (e.g. returning to the DeFi tab) refetches prices - * without needing `forceRefresh`. + * same accounts and currency we early-return without regrouping or writing + * state. Each account set keeps its own TTL so switching groups and back + * within the window can reuse already-fetched state. * - * `generation` identifies the latest claim for a key so overlapping - * `forceRefresh` calls discard stale responses (and older failures do not - * clear a newer claim). Intentionally not persisted: resets on restart, so - * the first fetch after a restart always goes through. + * A currency change for a given account set invalidates that claim so a + * plain `fetchDeFiPositions()` refetches prices without `forceRefresh`. + * `generation` lets overlapping `forceRefresh` calls for the same key + * discard stale responses. Not persisted: resets on restart. */ readonly #lastFetchByKey = new Map< string, @@ -236,10 +215,10 @@ export class DeFiPositionsControllerV2 extends BaseController< const accountIds = [...internalAccountIdByCaip.keys()]; // Stable key so the same account set throttles together regardless of map // iteration order. - const throttleKey = [...accountIds].sort().join(','); + const accountsKey = [...accountIds].sort().join(','); const vsCurrency = this.#getVsCurrency().toLowerCase(); const now = Date.now(); - const lastFetch = this.#lastFetchByKey.get(throttleKey); + const lastFetch = this.#lastFetchByKey.get(accountsKey); if ( !options?.forceRefresh && lastFetch?.vsCurrency === vsCurrency && @@ -247,14 +226,12 @@ export class DeFiPositionsControllerV2 extends BaseController< ) { return; } - // Claim the slot before awaiting so a second non-forced call that arrives - // while the first is in flight is also dropped (TanStack would share that - // promise; we intentionally skip instead). forceRefresh also claims the - // slot so follow-up non-forced calls within the interval stay throttled. - // Bump generation so overlapping forceRefresh calls can discard stale - // responses that finish out of order. + // Claim before awaiting so a second non-forced call while in flight is + // dropped. forceRefresh also claims so follow-up non-forced calls stay + // throttled. Bump generation so overlapping forceRefresh calls discard + // stale responses that finish out of order. const fetchGeneration = (lastFetch?.generation ?? 0) + 1; - this.#lastFetchByKey.set(throttleKey, { + this.#lastFetchByKey.set(accountsKey, { fetchedAt: now, vsCurrency, generation: fetchGeneration, @@ -270,33 +247,14 @@ export class DeFiPositionsControllerV2 extends BaseController< vsCurrency, }); - if ( - this.#lastFetchByKey.get(throttleKey)?.generation !== fetchGeneration - ) { + if (this.#lastFetchByKey.get(accountsKey)?.generation !== fetchGeneration) { // A newer fetch for this account set has already claimed the slot. return; } - // The v6 response echoes a per-chain CAIP-10 ID for every chain - // (`eip155:1:`, `eip155:137:`, ...), while we requested with - // the all-chains reference (`eip155:0:`). Match on namespace + - // address (ignoring the chain reference) so responses map back to the - // internal account IDs used to key state. Unmatched accounts are skipped. - const internalAccountIdByMatchKey = new Map(); - for (const [caipAccountId, internalId] of internalAccountIdByCaip) { - internalAccountIdByMatchKey.set( - toAccountMatchKey(caipAccountId), - internalId, - ); - } - const resolveAccountId = ( - responseAccountId: string, - ): string | undefined => - internalAccountIdByMatchKey.get(toAccountMatchKey(responseAccountId)); - const positionsByAccount = groupDeFiPositionsV6( response, - resolveAccountId, + internalAccountIdByCaip, ); this.update((state) => { @@ -309,10 +267,8 @@ export class DeFiPositionsControllerV2 extends BaseController< } catch (error) { // Only the latest attempt may clear the claim; an older failure must not // reopen the throttle window for a newer in-flight or completed fetch. - if ( - this.#lastFetchByKey.get(throttleKey)?.generation === fetchGeneration - ) { - this.#lastFetchByKey.delete(throttleKey); + if (this.#lastFetchByKey.get(accountsKey)?.generation === fetchGeneration) { + this.#lastFetchByKey.delete(accountsKey); } console.error('Failed to fetch DeFi positions', error); } diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts index 6cedffe8348..7446bfbeb94 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.test.ts @@ -14,7 +14,6 @@ import { createMockInternalAccount } from '../../../accounts-controller/tests/mo import { buildDeFiBalancesQuery, DEFI_SUPPORTED_NETWORKS, - normalizeCaipAccountId, } from './build-defi-balances-query'; const EVM_ADDRESS = '0x0000000000000000000000000000000000000001'; @@ -51,25 +50,17 @@ const mockBtcAccount = createMockInternalAccount({ type: BtcAccountType.P2wpkh, }); -describe('normalizeCaipAccountId', () => { - it('lowercases EVM CAIP account IDs', () => { - expect( - normalizeCaipAccountId(`eip155:0:${EVM_ADDRESS.toUpperCase()}`), - ).toBe(`eip155:0:${EVM_ADDRESS.toLowerCase()}`); - }); - - it('leaves non-EVM CAIP account IDs unchanged', () => { - const solanaCaipAccountId = `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`; - - expect(normalizeCaipAccountId(solanaCaipAccountId)).toBe( - solanaCaipAccountId, - ); - }); -}); - describe('buildDeFiBalancesQuery', () => { it('builds an EVM CAIP account spanning all supported EVM networks', () => { - const result = buildDeFiBalancesQuery([mockEvmAccount, mockBtcAccount]); + const mixedCaseEvmAccount = createMockInternalAccount({ + id: 'evm-account-id', + address: EVM_ADDRESS.toUpperCase(), + type: EthAccountType.Eoa, + }); + const result = buildDeFiBalancesQuery([ + mixedCaseEvmAccount, + mockBtcAccount, + ]); const expectedEvmNetworks = DEFI_SUPPORTED_NETWORKS.filter((network) => network.startsWith('eip155:'), diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index d6f66d33566..cca48a9d941 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -5,11 +5,7 @@ import { } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { CaipAccountId, CaipChainId } from '@metamask/utils'; -import { - KnownCaipNamespace, - parseCaipAccountId, - toCaipAccountId, -} from '@metamask/utils'; +import { KnownCaipNamespace, toCaipAccountId } from '@metamask/utils'; /** * Networks the DeFi balances (v6 multiaccount) endpoint supports. @@ -39,65 +35,26 @@ export type DeFiBalancesQuery = { /** CAIP-2 networks to query, deduped across accounts. */ networks: CaipChainId[]; /** - * Normalized CAIP-10 account IDs → internal MetaMask account IDs - * (`InternalAccount.id`). Keys are normalized via - * {@link normalizeCaipAccountId} so they can be sent as the v6 request - * account IDs and used to match response IDs case-insensitively for EVM. + * Request CAIP-10 account IDs → internal MetaMask account IDs + * (`InternalAccount.id`). EVM keys use the all-chains reference and a + * lowercased address; Solana keys keep address case. */ internalAccountIdByCaip: Map; }; /** * Builds an EVM CAIP-10 account ID that spans every EVM chain (reference `0`). + * Addresses are lowercased because EVM addresses are case-insensitive. * * @param address - The EVM account address. * @returns The CAIP-10 account ID for the address. */ function toEvmCaipAccountId(address: string): CaipAccountId { - return toCaipAccountId(KnownCaipNamespace.Eip155, '0', address); -} - -/** - * Normalizes a CAIP-10 account ID for case-insensitive matching. EVM addresses - * are case-insensitive, so `eip155:*` IDs are lowercased; other namespaces - * (e.g. Solana, whose base58 addresses are case-sensitive) are left as-is. Used - * to match the CAIP IDs the v6 API echoes back to the ones we sent. - * - * @param caipAccountId - The CAIP-10 account ID. - * @returns The normalized account ID. - */ -export function normalizeCaipAccountId(caipAccountId: string): string { - return caipAccountId.startsWith(`${KnownCaipNamespace.Eip155}:`) - ? caipAccountId.toLowerCase() - : caipAccountId; -} - -/** - * Builds a chain-reference-agnostic key (`namespace:address`) for matching the - * CAIP-10 account IDs we send against the ones the v6 API echoes back. - * - * We request EVM balances with the all-chains reference (`eip155:0:
`), - * but the response echoes a separate per-chain ID for every chain - * (`eip155:1:
`, `eip155:137:
`, ...). Matching on the full - * CAIP-10 string therefore fails, so we drop the reference and match on - * namespace + address instead. EVM addresses are lowercased (case-insensitive); - * other namespaces (e.g. Solana) keep their case. - * - * @param caipAccountId - A CAIP-10 account ID. - * @returns The match key, or the normalized ID if it cannot be parsed. - */ -export function toAccountMatchKey(caipAccountId: string): string { - try { - const { - chain: { namespace }, - address, - } = parseCaipAccountId(caipAccountId as CaipAccountId); - const normalizedAddress = - namespace === KnownCaipNamespace.Eip155 ? address.toLowerCase() : address; - return `${namespace}:${normalizedAddress}`; - } catch { - return normalizeCaipAccountId(caipAccountId); - } + return toCaipAccountId( + KnownCaipNamespace.Eip155, + '0', + address.toLowerCase(), + ); } /** @@ -131,7 +88,7 @@ export function buildDeFiBalancesQuery( ); if (evmAccount && evmNetworks.length > 0) { internalAccountIdByCaip.set( - normalizeCaipAccountId(toEvmCaipAccountId(evmAccount.address)), + toEvmCaipAccountId(evmAccount.address), evmAccount.id, ); networks.push(...evmNetworks); @@ -143,12 +100,10 @@ export function buildDeFiBalancesQuery( if (solanaAccount && solanaNetworks.length > 0) { const [, solanaReference] = SOLANA_MAINNET_CAIP_CHAIN_ID.split(':'); internalAccountIdByCaip.set( - normalizeCaipAccountId( - toCaipAccountId( - KnownCaipNamespace.Solana, - solanaReference, - solanaAccount.address, - ), + toCaipAccountId( + KnownCaipNamespace.Solana, + solanaReference, + solanaAccount.address, ), solanaAccount.id, ); diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts index c81cc840578..59f3d6c8e8c 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.test.ts @@ -169,10 +169,10 @@ describe('groupDeFiPositionsV6', () => { }); }); - it('skips accounts that do not resolve to an internal account ID', () => { + it('maps response accounts to internal IDs and skips unmatched ones', () => { const response = buildResponse([ { - accountId: 'eip155:0:0xunknown', + accountId: 'eip155:1:0xUnknown', balances: [ { category: 'defi', @@ -187,7 +187,9 @@ describe('groupDeFiPositionsV6', () => { ], }, { - accountId: 'eip155:0:0xknown', + // Response uses a per-chain reference + mixed case; request map uses + // the all-chains reference (`eip155:0:...`). + accountId: 'eip155:1:0xKnown', balances: [ { category: 'defi', @@ -203,8 +205,9 @@ describe('groupDeFiPositionsV6', () => { }, ]); - const result = groupDeFiPositionsV6(response, (responseAccountId) => - responseAccountId === 'eip155:0:0xknown' ? 'internal-1' : undefined, + const result = groupDeFiPositionsV6( + response, + new Map([['eip155:0:0xknown', 'internal-1']]), ); expect(Object.keys(result)).toStrictEqual(['internal-1']); diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index efc3572517f..cff05af37fe 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -5,9 +5,10 @@ import type { V6BalancesResponse, V6DeFiPositionType, } from '@metamask/core-backend'; -import type { CaipAssetType, CaipChainId } from '@metamask/utils'; +import type { CaipAccountId, CaipAssetType, CaipChainId } from '@metamask/utils'; import { KnownCaipNamespace, + parseCaipAccountId, parseCaipAssetType, parseCaipChainId, } from '@metamask/utils'; @@ -243,6 +244,36 @@ function toUnderlyingPosition( }; } +/** + * Builds a chain-reference-agnostic key (`namespace:address`) for matching the + * CAIP-10 account IDs we request against the ones the v6 API echoes back. + * + * We request EVM balances with the all-chains reference (`eip155:0:
`), + * but the response echoes a separate per-chain ID for every chain + * (`eip155:1:
`, `eip155:137:
`, ...). Matching on the full + * CAIP-10 string therefore fails, so we drop the reference and match on + * namespace + address instead. EVM addresses are lowercased; other namespaces + * keep their case. + * + * @param caipAccountId - A CAIP-10 account ID. + * @returns The match key, or a case-normalized fallback if parsing fails. + */ +function toAccountMatchKey(caipAccountId: string): string { + try { + const { + chain: { namespace }, + address, + } = parseCaipAccountId(caipAccountId as CaipAccountId); + const normalizedAddress = + namespace === KnownCaipNamespace.Eip155 ? address.toLowerCase() : address; + return `${namespace}:${normalizedAddress}`; + } catch { + return caipAccountId.startsWith(`${KnownCaipNamespace.Eip155}:`) + ? caipAccountId.toLowerCase() + : caipAccountId; + } +} + /** * Transforms a v6 multiaccount balances response into the stored DeFi state: * positions keyed by internal account ID, each mapping to a flat list of @@ -251,22 +282,29 @@ function toUnderlyingPosition( * details-page sections. Accounts present in the response but with no DeFi * positions are included with an empty list so stale data is cleared. * - * The v6 response keys accounts by the CAIP-10 ID sent to the API, so - * `resolveAccountId` maps that back to the internal MetaMask account ID used to - * key state. Accounts that do not resolve are skipped. It defaults to the - * identity function (leaving the response ID) for callers that don't need the - * mapping (e.g. tests). + * When `internalAccountIdByCaip` is provided, response account IDs are matched + * to internal MetaMask account IDs via namespace + address (ignoring chain + * reference and EVM case). Unmatched accounts are skipped. When omitted, the + * response account ID is used as-is (handy for unit tests). * * @param response - The v6 multiaccount balances response. - * @param resolveAccountId - Maps a response (CAIP-10) account ID to the internal - * account ID, or `undefined` to skip the account. - * @returns DeFi positions keyed by internal account ID and chain. + * @param internalAccountIdByCaip - Optional map of request CAIP-10 account IDs + * to internal MetaMask account IDs. + * @returns DeFi positions keyed by internal account ID. */ export function groupDeFiPositionsV6( response: V6BalancesResponse, - resolveAccountId: (responseAccountId: string) => string | undefined = (id) => - id, + internalAccountIdByCaip?: Map, ): DeFiPositionsByAccount { + const internalAccountIdByMatchKey = internalAccountIdByCaip + ? new Map( + [...internalAccountIdByCaip].map(([caipAccountId, internalId]) => [ + toAccountMatchKey(caipAccountId), + internalId, + ]), + ) + : undefined; + // Accumulate groups per resolved internal account ID. The v6 response returns // a separate entry per chain (e.g. `eip155:1:`, `eip155:137:`), // and several of them can resolve to the same internal account ID, so we must @@ -277,7 +315,9 @@ export function groupDeFiPositionsV6( >(); for (const account of response.accounts) { - const accountId = resolveAccountId(account.accountId); + const accountId = internalAccountIdByMatchKey + ? internalAccountIdByMatchKey.get(toAccountMatchKey(account.accountId)) + : account.accountId; if (accountId === undefined) { continue; } diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts index 615db5c2960..b0e5cc0b912 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts @@ -4,10 +4,7 @@ import type { DeFiProtocolPositionGroup, DeFiUnderlyingPosition, } from './group-defi-positions-v6'; -import { - mergePositionsForAccounts, - mergeSections, -} from './merge-positions-for-accounts'; +import { mergePositionsForAccounts } from './merge-positions-for-accounts'; const ETH_MAINNET = 'eip155:1' as CaipChainId; const BASE = 'eip155:8453' as CaipChainId; @@ -171,48 +168,33 @@ describe('mergePositionsForAccounts', () => { expect(result).toHaveLength(1); }); -}); - -describe('mergeSections', () => { - it('appends positions to the section that shares a productName', () => { - const existing = [ - { - productName: 'Aave V3', - positions: [buildPosition({ symbol: 'WETH' })], - }, - ]; - const incoming = [ - { - productName: 'Aave V3', - positions: [buildPosition({ symbol: 'USDC' })], - }, - ]; - - const result = mergeSections(existing, incoming); - - expect(result).toHaveLength(1); - expect(result[0].positions).toHaveLength(2); - }); - - it('keeps sections with distinct productNames separate', () => { - const existing = [{ productName: 'Aave V3', positions: [buildPosition()] }]; - const incoming = [{ productName: 'Pendle', positions: [buildPosition()] }]; - const result = mergeSections(existing, incoming); - - expect(result.map((section) => section.productName)).toStrictEqual([ - 'Aave V3', - 'Pendle', - ]); - }); - - it('does not mutate the input sections', () => { - const existing = [{ productName: 'Aave V3', positions: [buildPosition()] }]; - const incoming = [{ productName: 'Aave V3', positions: [buildPosition()] }]; + it('keeps sections with distinct productNames separate when merging', () => { + const groupA = buildGroup({ + sections: [ + { + productName: 'Aave V3', + positions: [buildPosition({ symbol: 'WETH' })], + }, + ], + }); + const groupB = buildGroup({ + sections: [ + { + productName: 'Pendle', + positions: [buildPosition({ symbol: 'USDC', assetId: USDC_ASSET_ID })], + }, + ], + }); - mergeSections(existing, incoming); + const result = mergePositionsForAccounts( + { 'account-1': [groupA], 'account-2': [groupB] }, + ['account-1', 'account-2'], + ); - expect(existing[0].positions).toHaveLength(1); - expect(incoming[0].positions).toHaveLength(1); + expect(result).toHaveLength(1); + expect(result[0].sections.map((section) => section.productName)).toStrictEqual( + ['Aave V3', 'Pendle'], + ); }); }); diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts index 5395caf4044..dac70047a49 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.ts @@ -13,7 +13,7 @@ import type { * protocol, to be merged in. * @returns The merged sections, one per distinct `productName`. */ -export function mergeSections( +function mergeSections( existingSections: DeFiPositionDetailsSection[], incomingSections: DeFiPositionDetailsSection[], ): DeFiPositionDetailsSection[] { diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index cedc838b7ca..9260bd312cb 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -268,8 +268,6 @@ export type { GroupedDeFiPositions } from './DeFiPositionsController/group-defi- export { DeFiPositionsControllerV2, getDefaultDeFiPositionsControllerV2State, - DEFI_POSITION_TYPES, - DEFI_POSITION_LIABILITY_TYPES, } from './DeFiPositionsController/DeFiPositionsControllerV2'; export type { DeFiPositionsControllerV2State, @@ -278,18 +276,21 @@ export type { DeFiPositionsControllerV2GetStateAction, DeFiPositionsControllerV2StateChangedEvent, DeFiPositionsControllerV2Messenger, +} from './DeFiPositionsController/DeFiPositionsControllerV2'; +export type { DeFiPositionsControllerV2FetchDeFiPositionsAction } from './DeFiPositionsController/DeFiPositionsControllerV2-method-action-types'; +export { + DEFI_POSITION_TYPES, + DEFI_POSITION_LIABILITY_TYPES, +} from './DeFiPositionsController/group-defi-positions-v6'; +export type { DeFiPositionsByAccount, DeFiProtocolPositionGroup, DeFiPositionDetailsSection, DeFiUnderlyingPosition, DeFiPositionIconGroupItem, DeFiPositionType, -} from './DeFiPositionsController/DeFiPositionsControllerV2'; -export type { DeFiPositionsControllerV2FetchDeFiPositionsAction } from './DeFiPositionsController/DeFiPositionsControllerV2-method-action-types'; -export { - mergePositionsForAccounts, - mergeSections, -} from './DeFiPositionsController/merge-positions-for-accounts'; +} from './DeFiPositionsController/group-defi-positions-v6'; +export { mergePositionsForAccounts } from './DeFiPositionsController/merge-positions-for-accounts'; export { DEFI_SUPPORTED_NETWORKS, buildDeFiBalancesQuery, From 5427744897f6305f27814ea3680dc58af1c803b0 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:41:23 +0100 Subject: [PATCH 33/36] refactors --- ...sitionsControllerV2-method-action-types.ts | 21 +++----- .../DeFiPositionsControllerV2.test.ts | 42 ++++++++++++++++ .../DeFiPositionsControllerV2.ts | 49 ++++++------------- .../group-defi-positions-v6.ts | 5 +- packages/assets-controllers/src/index.ts | 4 -- 5 files changed, 65 insertions(+), 56 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index 445ce7f6b60..05dd7167c2f 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -6,20 +6,13 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; /** - * Fetches DeFi positions for the selected account group and stores them, - * shaped for direct client consumption. Everything happens behind this - * method: resolving the accounts, calling the Accounts API, transforming the - * response, and updating state. - * - * Throttled per set of accounts + vsCurrency by an in-memory minimum - * interval, so repeated calls within the window are no-ops (no HTTP, no - * regroup, no state write). A change in fiat currency bypasses that window - * so tab-focus refetches pick up new prices without `forceRefresh`. Pass - * `{ forceRefresh: true }` to bypass the throttle entirely (e.g. - * pull-to-refresh or after a confirmed transaction). The successful/forced - * fetch still updates the throttle claim, so subsequent non-forced calls for - * the same accounts and currency remain gated. Disabled controllers and - * empty account groups return without fetching. + * Fetches DeFi positions for the selected account group and merges them into + * `allDeFiPositionsV2` (other accounts' cached entries are kept so group + * switches can reuse TTL'd state). No-ops when disabled, when the group has + * no supported accounts, or when the same accounts + `vsCurrency` were + * fetched within `minimumFetchIntervalMs`. Pass `{ forceRefresh: true }` to + * bypass the throttle (e.g. pull-to-refresh). A `vsCurrency` change for the + * same accounts also bypasses it. * * @param options - Optional fetch modifiers. * @param options.forceRefresh - When true, bypass the minimum-interval diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 84cff92a3f9..c483f2344e9 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -429,6 +429,48 @@ describe('DeFiPositionsControllerV2', () => { expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); + it('merges fetched accounts into state without clearing other accounts', async () => { + const otherEvmAddress = '0x0000000000000000000000000000000000000002'; + const otherEvmAccount = createMockInternalAccount({ + id: 'evm-account-id-2', + address: otherEvmAddress, + type: EthAccountType.Eoa, + }); + let groupAccounts: InternalAccount[] = GROUP_ACCOUNTS; + const { controller } = setupController({ + getGroupAccounts: () => groupAccounts, + mockFetchV6MultiAccountBalances: jest + .fn() + .mockResolvedValueOnce(buildMockBalancesResponse()) + .mockResolvedValueOnce( + buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${otherEvmAddress}`, + balances: [], + }, + ], + }), + ), + }); + + await controller.fetchDeFiPositions(); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, + ); + + groupAccounts = [otherEvmAccount]; + await controller.fetchDeFiPositions(); + + expect(controller.state.allDeFiPositionsV2).toStrictEqual({ + 'evm-account-id': expect.any(Array), + 'evm-account-id-2': [], + }); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, + ); + }); + it('bypasses the throttle when forceRefresh is true', async () => { const { controller, mockFetchV6MultiAccountBalances } = setupController({ minimumFetchIntervalMs: 60_000, diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index df61cf3203b..1ab726da2dc 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -79,10 +79,10 @@ export type AllowedActions = /** * The external events available to the {@link DeFiPositionsControllerV2}. * - * None for now. When wiring the controller into a client, events such as - * `KeyringController:lock`, `TransactionController:transactionConfirmed`, and - * `AccountTreeController:selectedAccountGroupChange` can be added here and - * subscribed to in order to trigger/clear fetches. + * None yet — clients must call `fetchDeFiPositions` (and optionally + * `{ forceRefresh: true }`) on their own triggers. Likely future subscriptions: + * `AccountTreeController:selectedAccountGroupChange`, + * `TransactionController:transactionConfirmed`, and `KeyringController:lock`. */ export type AllowedEvents = never; @@ -111,18 +111,8 @@ export class DeFiPositionsControllerV2 extends BaseController< readonly #minimumFetchIntervalMs: number; /** - * In-memory fetch claim per account set. - * - * Controller-level gate, separate from TanStack's `staleTime` inside - * `fetchV6MultiAccountBalances`: when the interval has not elapsed for the - * same accounts and currency we early-return without regrouping or writing - * state. Each account set keeps its own TTL so switching groups and back - * within the window can reuse already-fetched state. - * - * A currency change for a given account set invalidates that claim so a - * plain `fetchDeFiPositions()` refetches prices without `forceRefresh`. - * `generation` lets overlapping `forceRefresh` calls for the same key - * discard stale responses. Not persisted: resets on restart. + * In-memory per-account-set fetch claim (`fetchedAt`, `vsCurrency`, + * `generation`). Not persisted. See {@link fetchDeFiPositions}. */ readonly #lastFetchByKey = new Map< string, @@ -175,20 +165,13 @@ export class DeFiPositionsControllerV2 extends BaseController< } /** - * Fetches DeFi positions for the selected account group and stores them, - * shaped for direct client consumption. Everything happens behind this - * method: resolving the accounts, calling the Accounts API, transforming the - * response, and updating state. - * - * Throttled per set of accounts + vsCurrency by an in-memory minimum - * interval, so repeated calls within the window are no-ops (no HTTP, no - * regroup, no state write). A change in fiat currency bypasses that window - * so tab-focus refetches pick up new prices without `forceRefresh`. Pass - * `{ forceRefresh: true }` to bypass the throttle entirely (e.g. - * pull-to-refresh or after a confirmed transaction). The successful/forced - * fetch still updates the throttle claim, so subsequent non-forced calls for - * the same accounts and currency remain gated. Disabled controllers and - * empty account groups return without fetching. + * Fetches DeFi positions for the selected account group and merges them into + * `allDeFiPositionsV2` (other accounts' cached entries are kept so group + * switches can reuse TTL'd state). No-ops when disabled, when the group has + * no supported accounts, or when the same accounts + `vsCurrency` were + * fetched within `minimumFetchIntervalMs`. Pass `{ forceRefresh: true }` to + * bypass the throttle (e.g. pull-to-refresh). A `vsCurrency` change for the + * same accounts also bypasses it. * * @param options - Optional fetch modifiers. * @param options.forceRefresh - When true, bypass the minimum-interval @@ -257,6 +240,8 @@ export class DeFiPositionsControllerV2 extends BaseController< internalAccountIdByCaip, ); + // Merge by account: replace keys present in this response (including + // empty lists that clear stale positions) but leave other accounts alone. this.update((state) => { for (const [accountId, positions] of Object.entries( positionsByAccount, @@ -272,9 +257,5 @@ export class DeFiPositionsControllerV2 extends BaseController< } console.error('Failed to fetch DeFi positions', error); } - - // TODO: The previous controller emitted position-count analytics via a - // `trackEvent` hook (see calculate-defi-metrics). Deliberately dropped here; - // confirm what analytics will be needed before re-adding. } } diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index cff05af37fe..fa26f7f7738 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -13,10 +13,7 @@ import { parseCaipChainId, } from '@metamask/utils'; -// TODO: The extension prototype derived token icons via -// `getCaipAssetImageUrl`/`getAssetImageUrl` (shared/lib/asset-utils). Core has -// no shared equivalent yet, so the minimal builder below is inlined. Replace it -// with a shared helper if/when one lands in core. +/** Static.cx host used to build CAIP-19 token icon URLs for DeFi positions. */ const STATIC_METAMASK_BASE_URL = 'https://static.cx.metamask.io'; /** diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 9260bd312cb..e6d8103df00 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -291,10 +291,6 @@ export type { DeFiPositionType, } from './DeFiPositionsController/group-defi-positions-v6'; export { mergePositionsForAccounts } from './DeFiPositionsController/merge-positions-for-accounts'; -export { - DEFI_SUPPORTED_NETWORKS, - buildDeFiBalancesQuery, -} from './DeFiPositionsController/build-defi-balances-query'; export type { AccountGroupBalance, WalletBalance, From c164709e8b3216840de2d27099c9d65f36a02ab8 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:42:04 +0100 Subject: [PATCH 34/36] linting --- .../DeFiPositionsControllerV2.ts | 8 ++++++-- .../build-defi-balances-query.ts | 6 +----- .../DeFiPositionsController/group-defi-positions-v6.ts | 6 +++++- .../merge-positions-for-accounts.test.ts | 10 ++++++---- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 1ab726da2dc..fa7a129c122 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -230,7 +230,9 @@ export class DeFiPositionsControllerV2 extends BaseController< vsCurrency, }); - if (this.#lastFetchByKey.get(accountsKey)?.generation !== fetchGeneration) { + if ( + this.#lastFetchByKey.get(accountsKey)?.generation !== fetchGeneration + ) { // A newer fetch for this account set has already claimed the slot. return; } @@ -252,7 +254,9 @@ export class DeFiPositionsControllerV2 extends BaseController< } catch (error) { // Only the latest attempt may clear the claim; an older failure must not // reopen the throttle window for a newer in-flight or completed fetch. - if (this.#lastFetchByKey.get(accountsKey)?.generation === fetchGeneration) { + if ( + this.#lastFetchByKey.get(accountsKey)?.generation === fetchGeneration + ) { this.#lastFetchByKey.delete(accountsKey); } console.error('Failed to fetch DeFi positions', error); diff --git a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts index cca48a9d941..cde9b786125 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/build-defi-balances-query.ts @@ -50,11 +50,7 @@ export type DeFiBalancesQuery = { * @returns The CAIP-10 account ID for the address. */ function toEvmCaipAccountId(address: string): CaipAccountId { - return toCaipAccountId( - KnownCaipNamespace.Eip155, - '0', - address.toLowerCase(), - ); + return toCaipAccountId(KnownCaipNamespace.Eip155, '0', address.toLowerCase()); } /** diff --git a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts index fa26f7f7738..9ea4de4dfb3 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/group-defi-positions-v6.ts @@ -5,7 +5,11 @@ import type { V6BalancesResponse, V6DeFiPositionType, } from '@metamask/core-backend'; -import type { CaipAccountId, CaipAssetType, CaipChainId } from '@metamask/utils'; +import type { + CaipAccountId, + CaipAssetType, + CaipChainId, +} from '@metamask/utils'; import { KnownCaipNamespace, parseCaipAccountId, diff --git a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts index b0e5cc0b912..7b8ff135935 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/merge-positions-for-accounts.test.ts @@ -182,7 +182,9 @@ describe('mergePositionsForAccounts', () => { sections: [ { productName: 'Pendle', - positions: [buildPosition({ symbol: 'USDC', assetId: USDC_ASSET_ID })], + positions: [ + buildPosition({ symbol: 'USDC', assetId: USDC_ASSET_ID }), + ], }, ], }); @@ -193,8 +195,8 @@ describe('mergePositionsForAccounts', () => { ); expect(result).toHaveLength(1); - expect(result[0].sections.map((section) => section.productName)).toStrictEqual( - ['Aave V3', 'Pendle'], - ); + expect( + result[0].sections.map((section) => section.productName), + ).toStrictEqual(['Aave V3', 'Pendle']); }); }); From 7d07edfefd8475881cd29bcb2ce2b0d5720f1d7d Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 14:53:48 +0100 Subject: [PATCH 35/36] fixes --- ...sitionsControllerV2-method-action-types.ts | 9 ++--- .../DeFiPositionsControllerV2.test.ts | 35 +++++++++++++++++++ .../DeFiPositionsControllerV2.ts | 21 +++++++---- 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index 05dd7167c2f..75206135c01 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -6,10 +6,11 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; /** - * Fetches DeFi positions for the selected account group and merges them into - * `allDeFiPositionsV2` (other accounts' cached entries are kept so group - * switches can reuse TTL'd state). No-ops when disabled, when the group has - * no supported accounts, or when the same accounts + `vsCurrency` were + * Fetches DeFi positions for the selected account group. Each account key in + * a valid response replaces that account's state (other accounts stay). If + * any account is still indexing (`processingDefiPositions`), the response is + * discarded and prior state is kept. No-ops when disabled, when the group + * has no supported accounts, or when the same accounts + `vsCurrency` were * fetched within `minimumFetchIntervalMs`. Pass `{ forceRefresh: true }` to * bypass the throttle (e.g. pull-to-refresh). A `vsCurrency` change for the * same accounts also bypasses it. diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index c483f2344e9..2798f9bcece 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -429,6 +429,41 @@ describe('DeFiPositionsControllerV2', () => { expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); + it('keeps the last valid response when DeFi indexing is still processing', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController({ + minimumFetchIntervalMs: 60_000, + mockFetchV6MultiAccountBalances: jest + .fn() + .mockResolvedValueOnce(buildMockBalancesResponse()) + .mockResolvedValueOnce( + buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS}`, + processingDefiPositions: true, + balances: [], + }, + ], + }), + ) + .mockResolvedValueOnce(buildMockBalancesResponse()), + }); + + await controller.fetchDeFiPositions(); + const cached = controller.state.allDeFiPositionsV2['evm-account-id']; + expect(cached).toHaveLength(1); + + await controller.fetchDeFiPositions({ forceRefresh: true }); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toBe(cached); + + // Invalid response drops the throttle claim so a retry can land. + await controller.fetchDeFiPositions(); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(3); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, + ); + }); + it('merges fetched accounts into state without clearing other accounts', async () => { const otherEvmAddress = '0x0000000000000000000000000000000000000002'; const otherEvmAccount = createMockInternalAccount({ diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index fa7a129c122..726ec9c16a8 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -165,10 +165,11 @@ export class DeFiPositionsControllerV2 extends BaseController< } /** - * Fetches DeFi positions for the selected account group and merges them into - * `allDeFiPositionsV2` (other accounts' cached entries are kept so group - * switches can reuse TTL'd state). No-ops when disabled, when the group has - * no supported accounts, or when the same accounts + `vsCurrency` were + * Fetches DeFi positions for the selected account group. Each account key in + * a valid response replaces that account's state (other accounts stay). If + * any account is still indexing (`processingDefiPositions`), the response is + * discarded and prior state is kept. No-ops when disabled, when the group + * has no supported accounts, or when the same accounts + `vsCurrency` were * fetched within `minimumFetchIntervalMs`. Pass `{ forceRefresh: true }` to * bypass the throttle (e.g. pull-to-refresh). A `vsCurrency` change for the * same accounts also bypasses it. @@ -237,13 +238,21 @@ export class DeFiPositionsControllerV2 extends BaseController< return; } + // Incomplete indexing is not a valid snapshot — keep prior state and + // drop the throttle claim so the next call can retry. + if ( + response.accounts.some((account) => account.processingDefiPositions) + ) { + this.#lastFetchByKey.delete(accountsKey); + return; + } + const positionsByAccount = groupDeFiPositionsV6( response, internalAccountIdByCaip, ); - // Merge by account: replace keys present in this response (including - // empty lists that clear stale positions) but leave other accounts alone. + // Last valid response wins per account; other accounts stay untouched. this.update((state) => { for (const [accountId, positions] of Object.entries( positionsByAccount, From 7144a6308571d79e20fd2ab0564313e5d21ea5bd Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Tue, 21 Jul 2026 15:07:18 +0100 Subject: [PATCH 36/36] remove throttling --- ...sitionsControllerV2-method-action-types.ts | 18 +- .../DeFiPositionsControllerV2.test.ts | 276 ++++++------------ .../DeFiPositionsControllerV2.ts | 114 +++----- 3 files changed, 133 insertions(+), 275 deletions(-) diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts index 75206135c01..75e687596fd 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2-method-action-types.ts @@ -7,17 +7,17 @@ import type { DeFiPositionsControllerV2 } from './DeFiPositionsControllerV2'; /** * Fetches DeFi positions for the selected account group. Each account key in - * a valid response replaces that account's state (other accounts stay). If - * any account is still indexing (`processingDefiPositions`), the response is - * discarded and prior state is kept. No-ops when disabled, when the group - * has no supported accounts, or when the same accounts + `vsCurrency` were - * fetched within `minimumFetchIntervalMs`. Pass `{ forceRefresh: true }` to - * bypass the throttle (e.g. pull-to-refresh). A `vsCurrency` change for the - * same accounts also bypasses it. + * a ready response replaces that account's state (other accounts stay). + * Accounts still indexing (`processingDefiPositions`) are skipped so prior + * state is kept for them. No-ops when disabled or when the group has no + * supported accounts. Caching / spam prevention is handled by the apiClient + * TanStack Query cache (keyed by accounts + query options including + * `vsCurrency`). Pass `{ forceRefresh: true }` to bypass the cache (e.g. + * pull-to-refresh). * * @param options - Optional fetch modifiers. - * @param options.forceRefresh - When true, bypass the minimum-interval - * throttle and fetch immediately. + * @param options.forceRefresh - When true, bypass the apiClient cache and + * fetch immediately. */ export type DeFiPositionsControllerV2FetchDeFiPositionsAction = { type: `DeFiPositionsControllerV2:fetchDeFiPositions`; diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts index 2798f9bcece..b0b10061e49 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.test.ts @@ -131,7 +131,6 @@ function buildMockBalancesResponse( * @param config - Configuration for the mock setup. * @param config.isEnabled - Whether the controller is enabled. * @param config.getVsCurrency - Fiat currency getter. - * @param config.minimumFetchIntervalMs - Minimum fetch interval. * @param config.mockGroupAccounts - Accounts returned for the selected group. * @param config.getGroupAccounts - Getter for the selected group accounts * (preferred when the selection changes between fetches). @@ -142,7 +141,6 @@ function buildMockBalancesResponse( function setupController({ isEnabled = (): boolean => true, getVsCurrency = (): string => 'USD', - minimumFetchIntervalMs, mockGroupAccounts = GROUP_ACCOUNTS, getGroupAccounts, mockFetchV6MultiAccountBalances = jest @@ -152,7 +150,6 @@ function setupController({ }: { isEnabled?: () => boolean; getVsCurrency?: () => string; - minimumFetchIntervalMs?: number; mockGroupAccounts?: InternalAccount[]; getGroupAccounts?: () => InternalAccount[]; mockFetchV6MultiAccountBalances?: jest.Mock; @@ -201,7 +198,6 @@ function setupController({ apiClient, isEnabled, getVsCurrency, - minimumFetchIntervalMs, state, }); @@ -213,12 +209,7 @@ function setupController({ } describe('DeFiPositionsControllerV2', () => { - beforeEach(() => { - jest.useFakeTimers(); - }); - afterEach(() => { - jest.useRealTimers(); jest.restoreAllMocks(); }); @@ -273,6 +264,7 @@ describe('DeFiPositionsControllerV2', () => { includePrices: true, vsCurrency: 'usd', }, + {}, ); expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( @@ -378,6 +370,7 @@ describe('DeFiPositionsControllerV2', () => { includePrices: true, vsCurrency: 'usd', }, + {}, ); expect(controller.state.allDeFiPositionsV2).toStrictEqual({ 'evm-account-id': [], @@ -385,56 +378,73 @@ describe('DeFiPositionsControllerV2', () => { }); }); - it('throttles repeated fetches for the same accounts within the interval', async () => { + it('keeps prior state for accounts still indexing DeFi positions', async () => { const { controller, mockFetchV6MultiAccountBalances } = setupController({ - minimumFetchIntervalMs: 60_000, - }); - - await controller.fetchDeFiPositions(); - await controller.fetchDeFiPositions(); - - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); - - jest.advanceTimersByTime(60_000); - await controller.fetchDeFiPositions(); - - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); - }); - - it('keeps an independent throttle TTL per account set', async () => { - const otherEvmAccount = createMockInternalAccount({ - id: 'evm-account-id-2', - address: '0x0000000000000000000000000000000000000002', - type: EthAccountType.Eoa, - }); - let groupAccounts: InternalAccount[] = GROUP_ACCOUNTS; - const { controller, mockFetchV6MultiAccountBalances } = setupController({ - minimumFetchIntervalMs: 60_000, - getGroupAccounts: () => groupAccounts, mockFetchV6MultiAccountBalances: jest .fn() - .mockResolvedValue(buildMockBalancesResponse()), + .mockResolvedValueOnce(buildMockBalancesResponse()) + .mockResolvedValueOnce( + buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS}`, + processingDefiPositions: true, + balances: [], + }, + ], + }), + ), }); await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); - - groupAccounts = [otherEvmAccount]; - await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + const cached = controller.state.allDeFiPositionsV2['evm-account-id']; + expect(cached).toHaveLength(1); - // Returning to the first group within the window reuses its TTL. - groupAccounts = GROUP_ACCOUNTS; - await controller.fetchDeFiPositions(); + await controller.fetchDeFiPositions({ forceRefresh: true }); + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toBe(cached); expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); - it('keeps the last valid response when DeFi indexing is still processing', async () => { - const { controller, mockFetchV6MultiAccountBalances } = setupController({ - minimumFetchIntervalMs: 60_000, + it('updates ready accounts while skipping ones still indexing', async () => { + const solanaAccountId = `solana:${SolScope.Mainnet.split(':')[1]}:${SOLANA_ADDRESS}`; + const { controller } = setupController({ + mockGroupAccounts: GROUP_ACCOUNTS_WITH_SOLANA, mockFetchV6MultiAccountBalances: jest .fn() - .mockResolvedValueOnce(buildMockBalancesResponse()) + .mockResolvedValueOnce( + buildMockBalancesResponse({ + accounts: [ + { + accountId: `eip155:0:${EVM_ADDRESS}`, + balances: buildMockBalancesResponse().accounts[0].balances, + }, + { + accountId: solanaAccountId, + balances: [ + { + category: 'defi', + assetId: `${SolScope.Mainnet}/token:${SOLANA_ADDRESS}`, + name: 'Wrapped SOL', + symbol: 'WSOL', + decimals: 9, + balance: '1', + price: '100', + metadata: { + protocolId: 'marinade', + productName: 'Marinade', + description: 'Marinade on solana', + protocolUrl: 'https://marinade.finance/', + protocolIconUrl: 'https://example.com/marinade.png', + positionType: 'stake', + poolAddress: 'pool', + groupId: 'group-marinade-1', + }, + }, + ], + }, + ], + }), + ) .mockResolvedValueOnce( buildMockBalancesResponse({ accounts: [ @@ -443,25 +453,31 @@ describe('DeFiPositionsControllerV2', () => { processingDefiPositions: true, balances: [], }, + { + accountId: solanaAccountId, + balances: [], + }, ], }), - ) - .mockResolvedValueOnce(buildMockBalancesResponse()), + ), }); await controller.fetchDeFiPositions(); - const cached = controller.state.allDeFiPositionsV2['evm-account-id']; - expect(cached).toHaveLength(1); + const evmPositions = controller.state.allDeFiPositionsV2['evm-account-id']; + expect(evmPositions).toHaveLength(1); + expect( + controller.state.allDeFiPositionsV2['solana-account-id'], + ).toHaveLength(1); await controller.fetchDeFiPositions({ forceRefresh: true }); - expect(controller.state.allDeFiPositionsV2['evm-account-id']).toBe(cached); - // Invalid response drops the throttle claim so a retry can land. - await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(3); - expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( - 1, + // Still-indexing EVM account keeps prior positions; ready Solana clears. + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toBe( + evmPositions, ); + expect( + controller.state.allDeFiPositionsV2['solana-account-id'], + ).toStrictEqual([]); }); it('merges fetched accounts into state without clearing other accounts', async () => { @@ -506,69 +522,57 @@ describe('DeFiPositionsControllerV2', () => { ); }); - it('bypasses the throttle when forceRefresh is true', async () => { - const { controller, mockFetchV6MultiAccountBalances } = setupController({ - minimumFetchIntervalMs: 60_000, - }); + it('passes staleTime: 0 to the apiClient when forceRefresh is true', async () => { + const { controller, mockFetchV6MultiAccountBalances } = setupController(); - await controller.fetchDeFiPositions(); await controller.fetchDeFiPositions({ forceRefresh: true }); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); - - // A subsequent non-forced call within the interval is still throttled, - // keyed from the forceRefresh fetch timestamp. - await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); + expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledWith( + expect.any(Array), + expect.objectContaining({ vsCurrency: 'usd' }), + { staleTime: 0 }, + ); }); - it('refetches when vsCurrency changes even without forceRefresh', async () => { + it('passes the current vsCurrency to the apiClient', async () => { let vsCurrency = 'USD'; const { controller, mockFetchV6MultiAccountBalances } = setupController({ - minimumFetchIntervalMs: 60_000, getVsCurrency: () => vsCurrency, }); await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); expect(mockFetchV6MultiAccountBalances).toHaveBeenLastCalledWith( expect.any(Array), expect.objectContaining({ vsCurrency: 'usd' }), + {}, ); vsCurrency = 'EUR'; await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); expect(mockFetchV6MultiAccountBalances).toHaveBeenLastCalledWith( expect.any(Array), expect.objectContaining({ vsCurrency: 'eur' }), + {}, ); - - // Same currency again within the interval stays throttled. - await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); }); - it('clears the throttle claim when a fetch fails so retries are allowed', async () => { + it('keeps prior state when a fetch fails', async () => { const consoleErrorSpy = jest .spyOn(console, 'error') .mockImplementation(() => undefined); const mockFetchV6MultiAccountBalances = jest .fn() - .mockRejectedValueOnce(new Error('network error')) - .mockResolvedValueOnce(buildMockBalancesResponse()); + .mockResolvedValueOnce(buildMockBalancesResponse()) + .mockRejectedValueOnce(new Error('network error')); const { controller } = setupController({ mockFetchV6MultiAccountBalances, }); await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(1); - expect(controller.state.allDeFiPositionsV2).toStrictEqual({}); - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Failed to fetch DeFi positions', - expect.any(Error), + expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( + 1, ); await controller.fetchDeFiPositions(); @@ -576,110 +580,6 @@ describe('DeFiPositionsControllerV2', () => { expect(controller.state.allDeFiPositionsV2['evm-account-id']).toHaveLength( 1, ); - }); - - it('does not let a slower older response overwrite a newer forceRefresh result', async () => { - let resolveFirst!: (value: V6BalancesResponse) => void; - let resolveSecond!: (value: V6BalancesResponse) => void; - const firstPending = new Promise((resolve) => { - resolveFirst = resolve; - }); - const secondPending = new Promise((resolve) => { - resolveSecond = resolve; - }); - - const staleResponse = buildMockBalancesResponse(); - const freshResponse = buildMockBalancesResponse({ - accounts: [ - { - accountId: `eip155:0:${EVM_ADDRESS}`, - balances: [ - { - category: 'defi', - assetId: - 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', - name: 'Wrapped Ether', - symbol: 'WETH', - decimals: 18, - balance: '1', - price: '3000', - metadata: { - protocolId: 'aave-v3', - productName: 'Aave V3', - description: 'Aave V3 on ethereum', - protocolUrl: 'https://aave.com/', - protocolIconUrl: 'https://example.com/aave.png', - positionType: 'deposit', - poolAddress: '0xpool', - groupId: 'group-aave-1', - }, - }, - ], - }, - ], - }); - - const mockFetchV6MultiAccountBalances = jest - .fn() - .mockReturnValueOnce(firstPending) - .mockReturnValueOnce(secondPending); - - const { controller } = setupController({ - mockFetchV6MultiAccountBalances, - }); - - const firstFetch = controller.fetchDeFiPositions(); - const secondFetch = controller.fetchDeFiPositions({ forceRefresh: true }); - - resolveSecond(freshResponse); - await secondFetch; - expect( - controller.state.allDeFiPositionsV2['evm-account-id'][0], - ).toMatchObject({ marketValue: 3000 }); - - resolveFirst(staleResponse); - await firstFetch; - expect( - controller.state.allDeFiPositionsV2['evm-account-id'][0], - ).toMatchObject({ marketValue: 3000 }); - }); - - it('does not clear a newer fetch throttle claim when an older overlapping fetch fails', async () => { - const consoleErrorSpy = jest - .spyOn(console, 'error') - .mockImplementation(() => undefined); - - let rejectFirst!: (reason?: unknown) => void; - let resolveSecond!: (value: V6BalancesResponse) => void; - const firstPending = new Promise((_resolve, reject) => { - rejectFirst = reject; - }); - const secondPending = new Promise((resolve) => { - resolveSecond = resolve; - }); - - const mockFetchV6MultiAccountBalances = jest - .fn() - .mockReturnValueOnce(firstPending) - .mockReturnValueOnce(secondPending); - - const { controller } = setupController({ - mockFetchV6MultiAccountBalances, - minimumFetchIntervalMs: 60_000, - }); - - const firstFetch = controller.fetchDeFiPositions(); - const secondFetch = controller.fetchDeFiPositions({ forceRefresh: true }); - - rejectFirst(new Error('network error')); - await firstFetch; - - resolveSecond(buildMockBalancesResponse()); - await secondFetch; - - // Older failure must not wipe the newer claim, or this would fetch again. - await controller.fetchDeFiPositions(); - expect(mockFetchV6MultiAccountBalances).toHaveBeenCalledTimes(2); expect(consoleErrorSpy).toHaveBeenCalledWith( 'Failed to fetch DeFi positions', expect.any(Error), diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts index 726ec9c16a8..2030e6e0075 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsControllerV2.ts @@ -15,8 +15,6 @@ import { groupDeFiPositionsV6 } from './group-defi-positions-v6'; const controllerName = 'DeFiPositionsControllerV2'; -const ONE_MINUTE_IN_MS = 60_000; - const MESSENGER_EXPOSED_METHODS = ['fetchDeFiPositions'] as const; export type DeFiPositionsControllerV2State = { @@ -96,6 +94,10 @@ export type DeFiPositionsControllerV2Messenger = Messenger< * Controller that fetches DeFi positions for the selected account group from * the Accounts API (v6 multiaccount balances) and stores them in the shape the * client consumes directly. + * + * Deduplication and freshness are handled by the shared TanStack Query cache on + * {@link ApiPlatformClient} (balances default `staleTime` is 1 minute). Pass + * `{ forceRefresh: true }` to bypass that cache for pull-to-refresh. */ export class DeFiPositionsControllerV2 extends BaseController< typeof controllerName, @@ -108,24 +110,12 @@ export class DeFiPositionsControllerV2 extends BaseController< readonly #getVsCurrency: () => string; - readonly #minimumFetchIntervalMs: number; - - /** - * In-memory per-account-set fetch claim (`fetchedAt`, `vsCurrency`, - * `generation`). Not persisted. See {@link fetchDeFiPositions}. - */ - readonly #lastFetchByKey = new Map< - string, - { fetchedAt: number; vsCurrency: string; generation: number } - >(); - /** * @param options - Constructor options. * @param options.messenger - The controller messenger. * @param options.apiClient - Accounts API client used to fetch balances/positions. Auth is handled by the client. * @param options.isEnabled - Returns whether fetching is enabled (default: () => false). * @param options.getVsCurrency - Returns the fiat currency for prices (default: () => 'usd'). - * @param options.minimumFetchIntervalMs - Minimum time between fetches for the same accounts (default: 1 minute). * @param options.state - Initial controller state. */ constructor({ @@ -133,14 +123,12 @@ export class DeFiPositionsControllerV2 extends BaseController< apiClient, isEnabled, getVsCurrency, - minimumFetchIntervalMs = ONE_MINUTE_IN_MS, state, }: { messenger: DeFiPositionsControllerV2Messenger; apiClient: ApiPlatformClient; isEnabled: () => boolean; getVsCurrency: () => string; - minimumFetchIntervalMs?: number; state?: Partial; }) { super({ @@ -156,7 +144,6 @@ export class DeFiPositionsControllerV2 extends BaseController< this.#apiClient = apiClient; this.#isEnabled = isEnabled; this.#getVsCurrency = getVsCurrency; - this.#minimumFetchIntervalMs = minimumFetchIntervalMs; this.messenger.registerMethodActionHandlers( this, @@ -166,17 +153,17 @@ export class DeFiPositionsControllerV2 extends BaseController< /** * Fetches DeFi positions for the selected account group. Each account key in - * a valid response replaces that account's state (other accounts stay). If - * any account is still indexing (`processingDefiPositions`), the response is - * discarded and prior state is kept. No-ops when disabled, when the group - * has no supported accounts, or when the same accounts + `vsCurrency` were - * fetched within `minimumFetchIntervalMs`. Pass `{ forceRefresh: true }` to - * bypass the throttle (e.g. pull-to-refresh). A `vsCurrency` change for the - * same accounts also bypasses it. + * a ready response replaces that account's state (other accounts stay). + * Accounts still indexing (`processingDefiPositions`) are skipped so prior + * state is kept for them. No-ops when disabled or when the group has no + * supported accounts. Caching / spam prevention is handled by the apiClient + * TanStack Query cache (keyed by accounts + query options including + * `vsCurrency`). Pass `{ forceRefresh: true }` to bypass the cache (e.g. + * pull-to-refresh). * * @param options - Optional fetch modifiers. - * @param options.forceRefresh - When true, bypass the minimum-interval - * throttle and fetch immediately. + * @param options.forceRefresh - When true, bypass the apiClient cache and + * fetch immediately. */ async fetchDeFiPositions(options?: { forceRefresh?: boolean; @@ -197,62 +184,40 @@ export class DeFiPositionsControllerV2 extends BaseController< } const accountIds = [...internalAccountIdByCaip.keys()]; - // Stable key so the same account set throttles together regardless of map - // iteration order. - const accountsKey = [...accountIds].sort().join(','); const vsCurrency = this.#getVsCurrency().toLowerCase(); - const now = Date.now(); - const lastFetch = this.#lastFetchByKey.get(accountsKey); - if ( - !options?.forceRefresh && - lastFetch?.vsCurrency === vsCurrency && - now - lastFetch.fetchedAt < this.#minimumFetchIntervalMs - ) { - return; - } - // Claim before awaiting so a second non-forced call while in flight is - // dropped. forceRefresh also claims so follow-up non-forced calls stay - // throttled. Bump generation so overlapping forceRefresh calls discard - // stale responses that finish out of order. - const fetchGeneration = (lastFetch?.generation ?? 0) + 1; - this.#lastFetchByKey.set(accountsKey, { - fetchedAt: now, - vsCurrency, - generation: fetchGeneration, - }); try { const response = - await this.#apiClient.accounts.fetchV6MultiAccountBalances(accountIds, { - networks, - includeDeFiBalances: true, - forceFetchDeFiPositions: true, - includePrices: true, - vsCurrency, - }); - - if ( - this.#lastFetchByKey.get(accountsKey)?.generation !== fetchGeneration - ) { - // A newer fetch for this account set has already claimed the slot. - return; - } - - // Incomplete indexing is not a valid snapshot — keep prior state and - // drop the throttle claim so the next call can retry. - if ( - response.accounts.some((account) => account.processingDefiPositions) - ) { - this.#lastFetchByKey.delete(accountsKey); + await this.#apiClient.accounts.fetchV6MultiAccountBalances( + accountIds, + { + networks, + includeDeFiBalances: true, + forceFetchDeFiPositions: true, + includePrices: true, + vsCurrency, + }, + { + // staleTime: 0 makes TanStack treat the cache as stale for this call. + ...(options?.forceRefresh ? { staleTime: 0 } : {}), + }, + ); + + // Skip accounts still indexing — their balances are not a valid snapshot. + const readyAccounts = response.accounts.filter( + (account) => !account.processingDefiPositions, + ); + if (readyAccounts.length === 0) { return; } const positionsByAccount = groupDeFiPositionsV6( - response, + { ...response, accounts: readyAccounts }, internalAccountIdByCaip, ); - // Last valid response wins per account; other accounts stay untouched. + // Last valid response wins per ready account; processing / other accounts + // stay untouched. this.update((state) => { for (const [accountId, positions] of Object.entries( positionsByAccount, @@ -261,13 +226,6 @@ export class DeFiPositionsControllerV2 extends BaseController< } }); } catch (error) { - // Only the latest attempt may clear the claim; an older failure must not - // reopen the throttle window for a newer in-flight or completed fetch. - if ( - this.#lastFetchByKey.get(accountsKey)?.generation === fetchGeneration - ) { - this.#lastFetchByKey.delete(accountsKey); - } console.error('Failed to fetch DeFi positions', error); } }