diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 45acf6cd067..b02de7e76a5 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/transaction-controller` from `^69.6.1` to `^69.7.0` ([#10046](https://github.com/MetaMask/core/pull/10046)) +### Fixed + +- Price a staked position at parity with its chain's native currency instead of `$0`, in both the aggregated-balance selectors and the public asset getters (`getAccountAssetByID`, `getAccountAssetsByIDs`, `getAccountAssetsByScope`, `getAssets`, `getAssetsPrice`); price-refresh queuing (`assetsMiddleware`'s detected-asset queuing, `DetectionMiddleware`, and the force-refresh path) no longer lets a stale price recorded under the staking-vault's own asset ID suppress fetching the correct native price ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) + ## [14.0.3] ### Changed diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 59e80e44428..87a063bf114 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1013,6 +1013,45 @@ describe('AssetsController', () => { }); }); + it('prices a staked position from the chain native asset, not the vault contract (public getter)', async () => { + // Regression test: this is the public getter surface, separate from + // the aggregated-balance selector — the vault contract's own asset ID + // never has a Price API entry, so it must be aliased to the chain's + // native asset price here too, not just in the balance selector. + // State is keyed by the checksummed form, matching how normalizeAssetId + // stores every asset ID (see the "normalizes a lowercase EVM asset ID" + // test above) — STAKING_CONTRACT_ADDRESS_BY_CHAINID itself stores the + // address lowercase since it only ever compares case-insensitively. + const stakedAssetId = + 'eip155:1/erc20:0x4FEF9D741011476750A243aC70b9789a63dd47Df' as Caip19AssetId; + const stakedMetadata = { + type: 'erc20' as const, + symbol: 'stETH', + name: 'Staked ETH', + decimals: 18, + }; + const initialState: Partial = { + assetsInfo: { [stakedAssetId]: stakedMetadata }, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { [stakedAssetId]: { amount: '2' } }, + }, + // Only the native asset has a price entry, matching the real API. + assetsPrice: { + [MOCK_NATIVE_ASSET_ID]: { price: 2000, lastUpdated: 123 }, + }, + }; + + await withController({ state: initialState }, ({ controller }) => { + const asset = controller.getAccountAssetByID( + MOCK_ACCOUNT_ID, + stakedAssetId, + ); + + expect(asset?.price).toStrictEqual({ price: 2000, lastUpdated: 123 }); + expect(asset?.fiatValue).toBe(4000); + }); + }); + it('throws when accountId is empty', async () => { await withController(({ controller }) => { expect(() => diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 8754c27d3d0..9b181e82f0c 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -85,6 +85,7 @@ import type { AccountsApiDataSourceConfig } from './data-sources/AccountsApiData import { AccountsApiDataSource } from './data-sources/AccountsApiDataSource.js'; import { isStakingContractAssetId } from './data-sources/evm-rpc-services/index.js'; import { shouldSkipNativeForCaipChainId } from './data-sources/evm-rpc-services/utils/assets.js'; +import { resolvePriceLookupAssetId } from './data-sources/evm-rpc-services/utils/index.js'; import type { PriceDataSourceConfig } from './data-sources/PriceDataSource.js'; import { isPriceableAsset, @@ -2378,13 +2379,29 @@ export class AssetsController extends BaseController< continue; } const normalizedAssetId = normalizeAssetId(assetId as Caip19AssetId); - if (prices[normalizedAssetId] ?? prices[assetId]) { + // Resolve BEFORE checking presence/priceability: a staked position's + // own asset ID never has (and, since it's non-priceable, never will + // have) a legitimate price entry, and `isPriceableAsset` now rejects + // it outright — checking against the raw ID would silently skip it + // every time instead of enqueueing its resolved native asset's + // price. For a staking asset, a price recorded under the raw vault + // key is stale/foreign noise (e.g. from before this alias existed) + // and must NOT count as "already priced" — only the resolved + // (native) key's presence is authoritative there. + const priceLookupAssetId = resolvePriceLookupAssetId( + normalizedAssetId, + ) as Caip19AssetId; + const isStakedPosition = priceLookupAssetId !== normalizedAssetId; + const alreadyHasPrice = isStakedPosition + ? Boolean(prices[priceLookupAssetId]) + : Boolean(prices[normalizedAssetId] ?? prices[assetId]); + if (alreadyHasPrice) { continue; } - if (!isPriceableAsset(normalizedAssetId)) { + if (!isPriceableAsset(priceLookupAssetId)) { continue; } - assetsForPriceUpdate.push(normalizedAssetId); + assetsForPriceUpdate.push(priceLookupAssetId); } } @@ -3065,7 +3082,13 @@ export class AssetsController extends BaseController< } } - const priceRaw = this.state.assetsPrice[assetId]; + // Staked positions are priced under the chain's native asset instead of + // the vault contract's own asset ID — the Price API never has data for + // the vault itself. See `resolvePriceLookupAssetId`. + const priceRaw = + this.state.assetsPrice[ + resolvePriceLookupAssetId(assetId) as Caip19AssetId + ]; const price: AssetPrice = priceRaw ?? { price: 0, lastUpdated: 0, diff --git a/packages/assets-controller/src/data-sources/PriceDataSource.test.ts b/packages/assets-controller/src/data-sources/PriceDataSource.test.ts index c2fb5b1a61a..e02b1d8323e 100644 --- a/packages/assets-controller/src/data-sources/PriceDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/PriceDataSource.test.ts @@ -1315,4 +1315,66 @@ describe('PriceDataSource', () => { controller.destroy(); }, ); + + it('requests the chain native asset price for a staked-position balance, not the vault contract address', async () => { + // Regression test: the vault contract itself is never a priced token — + // requesting it would always return null — so a staked-position balance + // must be mapped to its chain's native asset before the price fetch, + // not merely dropped by the priceable-asset filter. + const MAINNET_STAKING_VAULT = + 'eip155:1/erc20:0x4fef9d741011476750a243ac70b9789a63dd47df' as Caip19AssetId; + const { controller, apiClient, getAssetsState } = setupController({ + balanceState: { + 'mock-account-id': { + [MAINNET_STAKING_VAULT]: { amount: '2' }, + }, + }, + priceResponse: { + [MOCK_NATIVE_ASSET]: createMockPriceData(2000), + }, + }); + + const response = await controller.fetch( + createDataRequest(), + getAssetsState, + ); + + expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledWith( + [MOCK_NATIVE_ASSET], + { currency: 'usd', includeMarketData: true }, + ); + // The vault's own asset ID must never appear as a requested key. + expect(apiClient.prices.fetchV3SpotPrices.mock.calls[0][0]).not.toContain( + MAINNET_STAKING_VAULT, + ); + expect(response.assetsPrice?.[MOCK_NATIVE_ASSET]?.price).toBe(2000); + + controller.destroy(); + }); + + it('requests the native asset only once when both a native and a staked balance are held on the same chain', async () => { + // A holder with both liquid and staked ETH must not cause a duplicate + // request for the native asset. + const MAINNET_STAKING_VAULT = + 'eip155:1/erc20:0x4fef9d741011476750a243ac70b9789a63dd47df' as Caip19AssetId; + const { controller, apiClient, getAssetsState } = setupController({ + balanceState: { + 'mock-account-id': { + [MOCK_NATIVE_ASSET]: { amount: '1' }, + [MAINNET_STAKING_VAULT]: { amount: '2' }, + }, + }, + priceResponse: { + [MOCK_NATIVE_ASSET]: createMockPriceData(2000), + }, + }); + + await controller.fetch(createDataRequest(), getAssetsState); + + const requested = apiClient.prices.fetchV3SpotPrices.mock + .calls[0][0] as Caip19AssetId[]; + expect(requested.filter((id) => id === MOCK_NATIVE_ASSET)).toHaveLength(1); + + controller.destroy(); + }); }); diff --git a/packages/assets-controller/src/data-sources/PriceDataSource.ts b/packages/assets-controller/src/data-sources/PriceDataSource.ts index 56a263b4294..d57b7d2f9ad 100644 --- a/packages/assets-controller/src/data-sources/PriceDataSource.ts +++ b/packages/assets-controller/src/data-sources/PriceDataSource.ts @@ -18,7 +18,11 @@ import type { import { DedupingBatchFetcher } from '../utils/dedupingBatchFetcher.js'; import { fetchWithTimeout, normalizeAssetId } from '../utils/index.js'; import type { SubscriptionRequest } from './AbstractDataSource.js'; -import { reduceInBatchesSerially } from './evm-rpc-services/index.js'; +import { + isStakingContractAssetId, + reduceInBatchesSerially, +} from './evm-rpc-services/index.js'; +import { resolvePriceLookupAssetId } from './evm-rpc-services/utils/index.js'; // ============================================================================ // CONSTANTS @@ -100,6 +104,13 @@ const NON_PRICEABLE_ASSET_PATTERNS = [ * @returns True if the asset has market price data. */ export function isPriceableAsset(assetId: Caip19AssetId): boolean { + // Staking vault contracts aren't real priced tokens — the Price API always + // returns null for them — so requesting them wastes a batch slot for a + // guaranteed miss. Their fiat value is derived from the chain's native + // currency instead (see `getNativeAssetIdForStakedAsset`). + if (isStakingContractAssetId(assetId)) { + return false; + } return !NON_PRICEABLE_ASSET_PATTERNS.some((pattern) => pattern.test(assetId)); } @@ -232,11 +243,20 @@ export class PriceDataSource { (queuedId) => queuedId === assetId || queuedId === normalizedAssetId, ); - if ( - statePrices[assetId] === undefined && - statePrices[normalizedAssetId] === undefined && - !alreadyQueued - ) { + // For a staking asset, only the RESOLVED (native) key's presence + // is authoritative — a price recorded under the raw vault key is + // stale/foreign noise (e.g. left over from before this alias + // existed) and must never count as "already priced", since the + // vault's own key never gets a fresh write to age it out. + const priceLookupAssetId = resolvePriceLookupAssetId( + normalizedAssetId, + ) as Caip19AssetId; + const isStakedPosition = priceLookupAssetId !== normalizedAssetId; + const alreadyHasPrice = isStakedPosition + ? statePrices[priceLookupAssetId] !== undefined + : statePrices[normalizedAssetId] !== undefined || + statePrices[assetId] !== undefined; + if (!alreadyHasPrice && !alreadyQueued) { assetIds.add(normalizedAssetId); } } @@ -246,8 +266,18 @@ export class PriceDataSource { return next(ctx); } - // Filter to only priceable assets - const priceableAssetIds = [...assetIds].filter(isPriceableAsset); + // Staking-vault positions have no price of their own — request their + // chain's native asset instead so the fetch that actually satisfies + // their price is guaranteed to happen, not merely not-dropped. + // Dedupe afterwards since multiple staking assets can map to the same + // native asset. + const priceableAssetIds = [ + ...new Set( + [...assetIds] + .map((id) => resolvePriceLookupAssetId(id)) + .filter(isPriceableAsset), + ), + ] as Caip19AssetId[]; if (priceableAssetIds.length === 0) { return next(ctx); @@ -490,8 +520,16 @@ export class PriceDataSource { getAssetsState, ); - // Filter out non-priceable assets (e.g., Tron bandwidth/energy resources) - const assetIds = rawAssetIds.filter(isPriceableAsset); + // Filter out non-priceable assets (e.g., Tron bandwidth/energy resources). + // Staking-vault positions are mapped to their chain's native asset first + // so the fetch that actually satisfies their price is guaranteed to run. + const assetIds = [ + ...new Set( + rawAssetIds + .map((id) => resolvePriceLookupAssetId(id)) + .filter(isPriceableAsset), + ), + ] as Caip19AssetId[]; if (assetIds.length === 0) { return response; diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/utils/index.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/utils/index.ts index 55367744a83..4689513cabb 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/utils/index.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/utils/index.ts @@ -1,8 +1,10 @@ export { divideIntoBatches, reduceInBatchesSerially } from './batch.js'; export { chainIdToHex, weiToHumanReadable } from './parsing.js'; export { + getNativeAssetIdForStakedAsset, getStakingContractAddress, getSupportedStakingChainIds, isStakingContractAssetId, + resolvePriceLookupAssetId, STAKING_CONTRACT_ADDRESS_BY_CHAINID, } from './staking-contracts.js'; diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.test.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.test.ts new file mode 100644 index 00000000000..d1b8de9b3be --- /dev/null +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.test.ts @@ -0,0 +1,79 @@ +import { + getNativeAssetIdForStakedAsset, + resolvePriceLookupAssetId, + STAKING_CONTRACT_ADDRESS_BY_CHAINID, +} from './staking-contracts.js'; + +describe('getNativeAssetIdForStakedAsset', () => { + it('resolves the mainnet staking vault to the mainnet native asset', () => { + expect( + getNativeAssetIdForStakedAsset( + `eip155:1/erc20:${STAKING_CONTRACT_ADDRESS_BY_CHAINID['eip155:1']}`, + ), + ).toBe('eip155:1/slip44:60'); + }); + + it('resolves the Hoodi staking vault to the Hoodi native asset', () => { + expect( + getNativeAssetIdForStakedAsset( + `eip155:560048/erc20:${STAKING_CONTRACT_ADDRESS_BY_CHAINID['eip155:560048']}`, + ), + ).toBe('eip155:560048/slip44:60'); + }); + + it('is case-insensitive on the contract address', () => { + const upper = STAKING_CONTRACT_ADDRESS_BY_CHAINID['eip155:1'].toUpperCase(); + expect(getNativeAssetIdForStakedAsset(`eip155:1/erc20:${upper}`)).toBe( + 'eip155:1/slip44:60', + ); + }); + + it('returns undefined for the mainnet staking address on an unrelated chain', () => { + // Same contract address, wrong chain — must not resolve. + expect( + getNativeAssetIdForStakedAsset( + `eip155:137/erc20:${STAKING_CONTRACT_ADDRESS_BY_CHAINID['eip155:1']}`, + ), + ).toBeUndefined(); + }); + + it('returns undefined for an unrelated erc20 on a known staking chain', () => { + expect( + getNativeAssetIdForStakedAsset( + 'eip155:1/erc20:0x9999999999999999999999999999999999999999', + ), + ).toBeUndefined(); + }); + + it('returns undefined for a non-erc20 asset (e.g. native)', () => { + expect( + getNativeAssetIdForStakedAsset('eip155:1/slip44:60'), + ).toBeUndefined(); + }); + + it('returns undefined for a malformed asset ID', () => { + expect(getNativeAssetIdForStakedAsset('not-a-caip-id')).toBeUndefined(); + expect(getNativeAssetIdForStakedAsset('')).toBeUndefined(); + }); +}); + +describe('resolvePriceLookupAssetId', () => { + it('resolves a staking vault asset ID to its native asset ID', () => { + expect( + resolvePriceLookupAssetId( + `eip155:1/erc20:${STAKING_CONTRACT_ADDRESS_BY_CHAINID['eip155:1']}`, + ), + ).toBe('eip155:1/slip44:60'); + }); + + it('returns the input unchanged for a non-staking asset', () => { + const assetId = 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48'; + expect(resolvePriceLookupAssetId(assetId)).toBe(assetId); + }); + + it('returns the input unchanged for the native asset itself', () => { + expect(resolvePriceLookupAssetId('eip155:1/slip44:60')).toBe( + 'eip155:1/slip44:60', + ); + }); +}); diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.ts index e988b0693ab..7908bb381b1 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/utils/staking-contracts.ts @@ -8,12 +8,49 @@ import { toCaipChainId, } from '@metamask/utils'; -/** Staking contract addresses by CAIP-2 chain ID (e.g. "eip155:1"). */ -export const STAKING_CONTRACT_ADDRESS_BY_CHAINID: Record = { - 'eip155:1': '0x4fef9d741011476750a243ac70b9789a63dd47df', // Mainnet - 'eip155:560048': '0xe96ac18cfe5a7af8fe1fe7bc37ff110d88bc67ff', // Hoodi (0x88bb0) +/** + * Single source of truth for every staking-supported chain: its vault + * contract address AND its native-currency SLIP-44 coin type, together. A + * staked position has no market price of its own (the vault contract isn't a + * priced token) — its value tracks the chain's native currency 1:1, so + * callers alias the staked asset to `nativeSlip44` to price it. + * + * This used to be two separate maps (address-by-chain, slip44-by-chain) with + * an attempted type-level check that a chain key present in one must be + * present in the other. That check was ineffective — widening either map to + * `Record` collapses `keyof typeof` back down to plain `string`, + * so nothing actually caught a chain added to one map and not the other. + * Keeping both pieces of a chain's staking config in ONE entry makes that + * class of drift structurally impossible rather than merely type-checked. + */ +const STAKING_CHAIN_CONFIG: Record< + string, + { address: string; nativeSlip44: number } +> = { + 'eip155:1': { + address: '0x4fef9d741011476750a243ac70b9789a63dd47df', // Mainnet + nativeSlip44: 60, + }, + 'eip155:560048': { + address: '0xe96ac18cfe5a7af8fe1fe7bc37ff110d88bc67ff', // Hoodi (0x88bb0) + nativeSlip44: 60, + }, }; +/** + * Staking contract addresses by CAIP-2 chain ID (e.g. "eip155:1"). Derived + * from {@link STAKING_CHAIN_CONFIG}; kept as its own export since existing + * callers (`getStakingContractAddress`, `getSupportedStakingChainIds`, and + * direct importers such as `StakedBalanceFetcher`) only need the address. + */ +export const STAKING_CONTRACT_ADDRESS_BY_CHAINID: Record = + Object.fromEntries( + Object.entries(STAKING_CHAIN_CONFIG).map(([chainId, config]) => [ + chainId, + config.address, + ]), + ); + /** * Normalize chain ID to CAIP-2 for lookup (e.g. "0x1" -> "eip155:1"). * Uses @metamask/utils for CAIP parsing. @@ -73,3 +110,47 @@ export function isStakingContractAssetId(assetId: string): boolean { )?.toLowerCase(); return stakingAddress !== undefined && address === stakingAddress; } + +/** + * Returns the CAIP-19 native-asset ID a staked position's price should be + * looked up under (e.g. "eip155:1/slip44:60" for mainnet ETH staking). The + * vault contract itself is never a priced token — the Price API returns + * `null` for it — so a staked balance is valued at parity with the chain's + * native currency. + * + * @param assetId - CAIP-19 asset ID to check (e.g. a staked-position asset). + * @returns The chain's native CAIP-19 asset ID, or `undefined` if `assetId` + * is not a known staking contract. + */ +export function getNativeAssetIdForStakedAsset( + assetId: string, +): string | undefined { + if (!isStakingContractAssetId(assetId)) { + return undefined; + } + // `isStakingContractAssetId` above already confirmed `chainId` is a key of + // `STAKING_CHAIN_CONFIG` (via `getStakingContractAddress`), which is the + // single source of truth for both the address and the native coin type — + // there is no second map this lookup could miss against. + const { chainId } = parseCaipAssetType(assetId); + const { nativeSlip44 } = STAKING_CHAIN_CONFIG[chainId]; + return `${chainId}/slip44:${nativeSlip44}`; +} + +/** + * Resolves the CAIP-19 asset ID that should actually be used to look up a + * price for `assetId` — the chain's native asset if `assetId` is a known + * staking-vault position, otherwise `assetId` unchanged. Staked balances + * track their chain's native currency 1:1 and are unconditionally priced + * that way: a stale or missing price object recorded directly under the + * vault's own asset ID (e.g. from a persisted pre-fix state, or a currency + * switch that only partially refreshed) must never win over the native + * price, so callers should route ALL staking-asset price lookups through + * this resolver rather than only falling back on a missing entry. + * + * @param assetId - CAIP-19 asset ID to resolve a price-lookup key for. + * @returns The CAIP-19 asset ID to use as the `assetsPrice` lookup key. + */ +export function resolvePriceLookupAssetId(assetId: string): string { + return getNativeAssetIdForStakedAsset(assetId) ?? assetId; +} diff --git a/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts b/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts index e876e47f946..40053e15762 100644 --- a/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts +++ b/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts @@ -409,6 +409,42 @@ describe('DetectionMiddleware', () => { expect(next).toHaveBeenCalledWith(context); }); + it('queues assetsForPriceUpdate for a staked position even when a stale price exists under the vault asset ID itself', async () => { + // Regression test: before staking positions were priced via their native + // asset alias, a price entry could exist directly under the vault's own + // asset ID (from an old poll, or persisted pre-fix state). Checking + // presence against only that raw key would suppress queuing a re-fetch + // forever, since the vault key never gets a fresh write to age it out — + // presence must be checked against the RESOLVED (native) key instead. + const MAINNET_STAKING_VAULT = + 'eip155:1/erc20:0x4fef9d741011476750a243ac70b9789a63dd47df' as Caip19AssetId; + const context = createMiddlewareContext( + { + response: { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MAINNET_STAKING_VAULT]: { amount: '2' }, + }, + }, + }, + }, + [MAINNET_STAKING_VAULT], + // A stale price is recorded directly under the vault's own asset ID — + // the native asset (eip155:1/slip44:60) has no price entry at all. + [MAINNET_STAKING_VAULT], + ); + const { middleware } = setupController(); + const next = jest.fn().mockImplementation((ctx) => Promise.resolve(ctx)); + + await middleware.assetsMiddleware(context, next); + + expect(context.response.detectedAssets).toBeUndefined(); + expect(context.request.assetsForPriceUpdate).toStrictEqual([ + normalizeAssetId(MAINNET_STAKING_VAULT), + ]); + expect(next).toHaveBeenCalledWith(context); + }); + it('queues assetsForPriceUpdate for known balance assets that still lack a price', async () => { const { middleware } = setupController(); // Asset already has metadata (known / seeded) but no price yet. diff --git a/packages/assets-controller/src/middlewares/DetectionMiddleware.ts b/packages/assets-controller/src/middlewares/DetectionMiddleware.ts index 805c59143e1..8dc3115f184 100644 --- a/packages/assets-controller/src/middlewares/DetectionMiddleware.ts +++ b/packages/assets-controller/src/middlewares/DetectionMiddleware.ts @@ -1,3 +1,4 @@ +import { resolvePriceLookupAssetId } from '../data-sources/evm-rpc-services/utils/index.js'; import { projectLogger, createModuleLogger } from '../logger.js'; import { forDataTypes } from '../types.js'; import type { AccountId, Caip19AssetId, Middleware } from '../types.js'; @@ -144,16 +145,27 @@ export class DetectionMiddleware { // 1) newly detected assets missing a price, and // 2) assets in this balance response that already exist in state but still // lack a price (e.g. natives seeded before the first price poll). - // PriceDataSource filters non-priceable IDs before calling the API. + // PriceDataSource filters non-priceable IDs (and resolves staking-vault + // IDs to their chain's native asset) before calling the API. const prices = stateAssetsPrice as Record; const missingPriceAssets = new Set(); const maybeQueue = (assetId: Caip19AssetId): void => { const normalizedAssetId = normalizeAssetId(assetId); - if ( - prices[normalizedAssetId] === undefined && - prices[assetId] === undefined - ) { + // For a staking asset, only the RESOLVED (native) key's presence is + // authoritative — a price recorded under the raw vault key is + // stale/foreign noise (e.g. left over from before this alias + // existed) and must never count as "already priced", since the + // vault's own key never gets a fresh write to age it out. + const priceLookupAssetId = resolvePriceLookupAssetId( + normalizedAssetId, + ) as Caip19AssetId; + const isStakedPosition = priceLookupAssetId !== normalizedAssetId; + const alreadyHasPrice = isStakedPosition + ? prices[priceLookupAssetId] !== undefined + : prices[normalizedAssetId] !== undefined || + prices[assetId] !== undefined; + if (!alreadyHasPrice) { missingPriceAssets.add(normalizedAssetId); } }; diff --git a/packages/assets-controller/src/selectors/balance.test.ts b/packages/assets-controller/src/selectors/balance.test.ts index 4a07eed4e7e..8d9bb09ca4b 100644 --- a/packages/assets-controller/src/selectors/balance.test.ts +++ b/packages/assets-controller/src/selectors/balance.test.ts @@ -93,6 +93,9 @@ describe('balance selectors', () => { const assetPolygon = 'eip155:137/slip44:966' as Caip19AssetId; const assetTangYuan = 'eip155:56/erc20:0x1111111111111111111111111111111111111111' as Caip19AssetId; + // Real mainnet staking vault contract address (STAKING_CONTRACT_ADDRESS_BY_CHAINID). + const assetStakedEth = + 'eip155:1/erc20:0x4fef9d741011476750a243ac70b9789a63dd47df' as Caip19AssetId; const assetInfoEth = { type: 'native' as const, @@ -122,6 +125,13 @@ describe('balance selectors', () => { decimals: 9, }; + const assetInfoStakedEth = { + type: 'erc20' as const, + symbol: 'stETH', + name: 'Staked ETH', + decimals: 18, + }; + function testFungibleAssetPrice( price: number, pricePercentChange1d = 0, @@ -481,6 +491,102 @@ describe('balance selectors', () => { expect(result.entries).toHaveLength(2); }); + + it('prices a staked position at parity with the chain native currency, not zero', () => { + // Regression test: the staking vault contract (assetStakedEth) is never + // itself a priced token — the Price API always returns null for it — so + // it must be aliased to the native asset's price (assetEth) rather than + // silently valued at zero. Mirrors a real mainnet ETH + staked-ETH + // portfolio. + const state = arrangeAssetsControllerState({ + assetsBalance: { + [accountId1]: { + [assetEth]: { amount: '1' }, + [assetStakedEth]: { amount: '2' }, + }, + }, + assetsInfo: { + [assetEth]: assetInfoEth, + [assetStakedEth]: assetInfoStakedEth, + }, + assetsPrice: { + // Only the native asset has a price entry — matching the real API, + // which never returns price data for the vault contract address. + [assetEth]: testFungibleAssetPrice(2000, 5), + }, + }); + + const result = getAggregatedBalanceForAccount(state, selectedAccount); + + expect(result.entries).toHaveLength(2); + const stakedEntry = result.entries.find( + (entry) => entry.assetId === assetStakedEth, + ); + expect(stakedEntry).toMatchObject({ amount: '2' }); + // 1 ETH liquid + 2 ETH staked, both priced at 2000 -> 3 * 2000 = 6000. + // Before the fix this was 2000 (the staked leg silently priced at 0). + expect(result.totalBalanceInFiat).toBe(6000); + expect(result.pricePercentChange1d).toBeCloseTo(5, 10); + }); + + it('prices an unrelated erc20 at zero (an address that merely resembles a vault, on a chain that DOES have one)', () => { + // Sanity check for the fix's boundary: mainnet (eip155:1) is a known + // staking chain, but this address is NOT its staking contract — only + // the address is unrelated, not the chain. It must not be aliased to + // anything; it has no price entry and no native-currency fallback + // applies, so it stays at zero as before. + const unrelatedErc20 = + 'eip155:1/erc20:0x9999999999999999999999999999999999999999' as Caip19AssetId; + const state = arrangeAssetsControllerState({ + assetsBalance: { + [accountId1]: { + [unrelatedErc20]: { amount: '10' }, + }, + }, + assetsInfo: { + [unrelatedErc20]: assetInfoUsdc, + }, + assetsPrice: { + [assetEth]: testFungibleAssetPrice(2000, 5), + }, + }); + + const result = getAggregatedBalanceForAccount(state, selectedAccount); + + expect(result.entries).toHaveLength(1); + expect(result.totalBalanceInFiat).toBe(0); + }); + + it('prices a staked position from the native entry even when a stale/wrong price is also recorded under the vault asset ID itself', () => { + // Regression test for a second failure mode Codex review caught: a + // fallback that only fires when the vault's own price entry is MISSING + // still lets a persisted-but-stale (or wrong-currency) vault price win + // over the correct native price. The native price must be used + // unconditionally for a recognized staking asset, never merely as a + // fallback-on-absence. + const state = arrangeAssetsControllerState({ + assetsBalance: { + [accountId1]: { + [assetStakedEth]: { amount: '2' }, + }, + }, + assetsInfo: { + [assetStakedEth]: assetInfoStakedEth, + }, + assetsPrice: { + [assetEth]: testFungibleAssetPrice(2000, 5), + // A stale/wrong price recorded directly under the vault's own + // asset ID (e.g. left over from before this fix, or a partial + // currency-switch refresh) — must NOT be used. + [assetStakedEth]: testFungibleAssetPrice(0, 0), + }, + }); + + const result = getAggregatedBalanceForAccount(state, selectedAccount); + + // 2 ETH staked at the NATIVE price (2000), not the stale vault price (0). + expect(result.totalBalanceInFiat).toBe(4000); + }); }); describe('getAccountIdsForGroup', () => { diff --git a/packages/assets-controller/src/selectors/balance.ts b/packages/assets-controller/src/selectors/balance.ts index 752746c8517..78167378962 100644 --- a/packages/assets-controller/src/selectors/balance.ts +++ b/packages/assets-controller/src/selectors/balance.ts @@ -12,6 +12,7 @@ import { import BigNumberJS from 'bignumber.js'; import type { AssetsControllerState } from '../AssetsController.js'; +import { resolvePriceLookupAssetId } from '../data-sources/evm-rpc-services/utils/index.js'; import type { AccountId, AssetBalance, @@ -110,7 +111,8 @@ const getPriceDatumFast = ( assetsPrice: AssetsControllerState['assetsPrice'] | undefined, assetId: Caip19AssetId, ): PriceDatum => { - const raw = assetsPrice?.[assetId]; + const raw = + assetsPrice?.[resolvePriceLookupAssetId(assetId) as Caip19AssetId]; if (!raw || typeof raw !== 'object') { return ZERO_PRICE; }