Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/assets-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/assets-controller/src/AssetsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AssetsControllerState> = {
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(() =>
Expand Down
31 changes: 27 additions & 4 deletions packages/assets-controller/src/AssetsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
58 changes: 48 additions & 10 deletions packages/assets-controller/src/data-sources/PriceDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
@@ -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',
);
});
});
Loading