diff --git a/README.md b/README.md index ed7f97862c..e9031557fc 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,7 @@ linkStyle default opacity:0.5 assets_controllers --> accounts_controller; assets_controllers --> approval_controller; assets_controllers --> base_controller; + assets_controllers --> config_registry_controller; assets_controllers --> controller_utils; assets_controllers --> core_backend; assets_controllers --> keyring_controller; diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 72b4b28514..92ec5143f5 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `@metamask/config-registry-controller` as a dependency ([#9789](https://github.com/MetaMask/core/pull/9789)) + +### Changed + +- **BREAKING:** `TokenRatesControllerMessenger` now requires the `ConfigRegistryController:getNetworkConfigByCaip2ChainId` action to be delegated ([#9789](https://github.com/MetaMask/core/pull/9789)) + - `getAssetId`/`CodefiTokenPricesServiceV2` now resolve native asset CAIP-19 IDs from the config registry's `assets.native.assetId` before falling back to the hardcoded `SPOT_PRICES_SUPPORT_INFO` map, then to `NetworkEnablementController`'s `nativeAssetIdentifiers`. This lets new chains get correct native-asset pricing without a `SPOT_PRICES_SUPPORT_INFO` release. `TokenRatesController` seeds this per chain, right before pricing that chain's assets, via `ConfigRegistryController`'s per-chain lookup action, rather than mirroring its entire network map. + ## [111.0.0] ### Changed diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index d5fcb6a57c..0cfbe58e37 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -66,6 +66,7 @@ "@metamask/accounts-controller": "^39.0.6", "@metamask/approval-controller": "^9.0.2", "@metamask/base-controller": "^9.1.0", + "@metamask/config-registry-controller": "^2.0.1", "@metamask/contract-metadata": "^2.4.0", "@metamask/controller-utils": "^12.3.0", "@metamask/core-backend": "^8.1.1", diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index 818da8ac73..6a0652ef5f 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -1,4 +1,5 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; +import type { RegistryNetworkConfig } from '@metamask/config-registry-controller'; import { ChainId, toChecksumHexAddress } from '@metamask/controller-utils'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { @@ -23,7 +24,11 @@ import type { AbstractTokenPricesService, EvmAssetWithMarketData, } from './token-prices-service/abstract-token-prices-service.js'; -import { ZERO_ADDRESS } from './token-prices-service/codefi-v2.js'; +import { + getAssetId, + resetNetworkConfigsCache, + ZERO_ADDRESS, +} from './token-prices-service/codefi-v2.js'; import { controllerName, TokenRatesController, @@ -75,6 +80,7 @@ function buildTokenRatesControllerMessenger( 'TokensController:getState', 'NetworkController:getState', 'NetworkEnablementController:getState', + 'ConfigRegistryController:getNetworkConfigByCaip2ChainId', ], events: ['TokensController:stateChange', 'NetworkController:stateChange'], }); @@ -82,6 +88,13 @@ function buildTokenRatesControllerMessenger( } describe('TokenRatesController', () => { + afterEach(() => { + // getAssetId's config registry cache is a module-level singleton (shared + // across the whole process, like getSupportedNetworks); reset it so + // tests don't leak state into each other. + resetNetworkConfigsCache(); + }); + describe('constructor', () => { it('should set default state', async () => { await withController(async ({ controller }) => { @@ -232,6 +245,50 @@ describe('TokenRatesController', () => { ); }); + it('seeds the config registry cache used by getAssetId for the chains being priced', async () => { + const mockGetNetworkConfigByCaip2ChainId = jest + .fn() + .mockImplementation((caipChainId: string) => + caipChainId === 'eip155:1' + ? { assets: { native: { assetId: 'eip155:1/slip44:61' } } } + : undefined, + ); + + await withController( + { mockGetNetworkConfigByCaip2ChainId }, + async ({ controller }) => { + await controller.updateExchangeRates([ + { chainId: '0x1', nativeCurrency: 'ETH' }, + ]); + + expect(mockGetNetworkConfigByCaip2ChainId).toHaveBeenCalledWith( + 'eip155:1', + ); + // getAssetId is the function extracted for client parity with + // fetchTokenPrices; it must reflect the same registry data + // updateExchangeRates just seeded, with no params beyond chain/token. + expect( + getAssetId({ chainId: '0x1', tokenAddress: ZERO_ADDRESS }), + ).toBe('eip155:1/slip44:61'); + }, + ); + }); + + it('does not seed the cache for chains outside the current batch', async () => { + await withController(async ({ controller }) => { + await controller.updateExchangeRates([ + { chainId: '0x89', nativeCurrency: 'MATIC' }, + ]); + + // 0x1 was never part of a priced batch, so it falls back to the + // hardcoded SPOT_PRICES_SUPPORT_INFO entry rather than picking up + // stale or unrelated registry data. + expect(getAssetId({ chainId: '0x1', tokenAddress: ZERO_ADDRESS })).toBe( + 'eip155:1/slip44:60', + ); + }); + }); + it('clears stale marketData when isDeprecated toggles to true at runtime', async () => { const tokenPricesService = buildMockTokenPricesService(); jest.spyOn(tokenPricesService, 'fetchTokenPrices'); @@ -1458,6 +1515,9 @@ type WithControllerOptions = { >; mockTokensControllerState?: Partial; mockNetworkState?: Partial; + mockGetNetworkConfigByCaip2ChainId?: ( + caipChainId: string, + ) => RegistryNetworkConfig | undefined; }; type WithControllerArgs = @@ -1477,7 +1537,12 @@ async function withController( ...args: WithControllerArgs ): Promise { const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; - const { options, mockTokensControllerState, mockNetworkState } = rest; + const { + options, + mockTokensControllerState, + mockNetworkState, + mockGetNetworkConfigByCaip2ChainId, + } = rest; const messenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, }); @@ -1512,6 +1577,18 @@ async function withController( }), ); + // Register ConfigRegistryController:getNetworkConfigByCaip2ChainId handler + const defaultGetNetworkConfigByCaip2ChainId = (): undefined => undefined; + messenger.registerActionHandler( + 'ConfigRegistryController:getNetworkConfigByCaip2ChainId', + jest + .fn() + .mockImplementation( + mockGetNetworkConfigByCaip2ChainId ?? + defaultGetNetworkConfigByCaip2ChainId, + ), + ); + const controller = new TokenRatesController({ tokenPricesService: buildMockTokenPricesService(), messenger: buildTokenRatesControllerMessenger(messenger), diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index a27105de66..4faf7b2e09 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -3,6 +3,7 @@ import type { ControllerStateChangeEvent, StateMetadata, } from '@metamask/base-controller'; +import type { ConfigRegistryControllerGetNetworkConfigByCaip2ChainIdAction } from '@metamask/config-registry-controller'; import { toChecksumHexAddress } from '@metamask/controller-utils'; import type { Messenger } from '@metamask/messenger'; import type { @@ -12,6 +13,11 @@ import type { import type { NetworkEnablementControllerGetStateAction } from '@metamask/network-enablement-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; +import { + hexToNumber, + KnownCaipNamespace, + toCaipChainId, +} from '@metamask/utils'; import { isEqual } from 'lodash'; import { @@ -19,7 +25,10 @@ import { TOKEN_PRICES_BATCH_SIZE, } from './assetsUtil.js'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service.js'; -import { getNativeTokenAddress } from './token-prices-service/codefi-v2.js'; +import { + getNativeTokenAddress, + setNetworkConfig, +} from './token-prices-service/codefi-v2.js'; import { TokenRwaData } from './token-service.js'; import type { TokensControllerGetStateAction, @@ -98,7 +107,8 @@ type ChainIdAndNativeCurrency = { export type AllowedActions = | TokensControllerGetStateAction | NetworkControllerGetStateAction - | NetworkEnablementControllerGetStateAction; + | NetworkEnablementControllerGetStateAction + | ConfigRegistryControllerGetNetworkConfigByCaip2ChainIdAction; /** * The external events available to the {@link TokenRatesController}. @@ -380,6 +390,32 @@ export class TokenRatesController extends StaticIntervalPollingController): void { + for (const chainId of new Set(chainIds)) { + const caipChainId = toCaipChainId( + KnownCaipNamespace.Eip155, + hexToNumber(chainId).toString(), + ); + setNetworkConfig( + caipChainId, + this.messenger.call( + 'ConfigRegistryController:getNetworkConfigByCaip2ChainId', + caipChainId, + ), + ); + } + } + /** * Get the tokens for the given chain. * @@ -448,6 +484,10 @@ export class TokenRatesController extends StaticIntervalPollingController chainId), + ); + const marketData: Record> = {}; const assetsByNativeCurrency: Record< string, diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts index f81cf6879e..629a9e08a5 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts @@ -1,3 +1,4 @@ +import type { RegistryNetworkConfig } from '@metamask/config-registry-controller'; import { KnownCaipNamespace } from '@metamask/utils'; import type { Hex } from '@metamask/utils'; import nock, { isDone } from 'nock'; @@ -16,6 +17,8 @@ import { getSupportedNetworks, resetSupportedNetworksCache, getAssetId, + setNetworkConfig, + resetNetworkConfigsCache, } from './codefi-v2.js'; // We're not customizing the default max delay @@ -2144,6 +2147,72 @@ describe('CodefiTokenPricesServiceV2', () => { }); }); + describe('setNetworkConfig', () => { + afterEach(() => { + resetSupportedNetworksCache(); + resetNetworkConfigsCache(); + }); + + it('uses the config registry asset id in fetchTokenPrices, taking priority over the hardcoded entry', async () => { + const mockNetworksResponse = { + fullSupport: ['eip155:1'], + partialSupport: { + spotPricesV2: [], + spotPricesV3: ['eip155:1'], + }, + }; + nock('https://price.api.cx.metamask.io') + .get('/v2/supportedNetworks') + .reply(200, mockNetworksResponse) + .persist(); + + await fetchSupportedNetworks(); + + const registryAssetId = 'eip155:1/slip44:61'; + + nock('https://price.api.cx.metamask.io') + .get('/v3/spot-prices') + .query(true) + .reply(200, { + [registryAssetId]: { + price: 3000, + currency: 'USD', + pricePercentChange1d: 1, + priceChange1d: 1, + marketCap: 1000000, + allTimeHigh: 5000, + allTimeLow: 100, + totalVolume: 50000, + high1d: 2100, + low1d: 1900, + circulatingSupply: 1000000, + dilutedMarketCap: 2000000, + marketCapPercentChange1d: 0.5, + pricePercentChange1h: 0.1, + pricePercentChange7d: 5, + pricePercentChange14d: 10, + pricePercentChange30d: 15, + pricePercentChange200d: 50, + pricePercentChange1y: 100, + }, + }); + + setNetworkConfig('eip155:1', { + assets: { native: { assetId: registryAssetId } }, + } as RegistryNetworkConfig); + + const service = new CodefiTokenPricesServiceV2(); + + const result = await service.fetchTokenPrices({ + assets: [{ chainId: '0x1', tokenAddress: ZERO_ADDRESS }], + currency: 'USD', + }); + + expect(result).toHaveLength(1); + expect(result[0].price).toBe(3000); + }); + }); + describe('validateChainIdSupported with dynamic networks', () => { afterEach(() => { resetSupportedNetworksCache(); @@ -2186,6 +2255,10 @@ describe('CodefiTokenPricesServiceV2', () => { }); describe('getAssetId', () => { + afterEach(() => { + resetNetworkConfigsCache(); + }); + it('returns a CAIP-19 erc20 id with a lowercased address for ERC20 tokens', () => { expect(getAssetId({ chainId: '0x1', tokenAddress: '0xABCDEF' })).toBe( 'eip155:1/erc20:0xabcdef', @@ -2239,6 +2312,51 @@ describe('CodefiTokenPricesServiceV2', () => { ).toBe('eip155:1/slip44:60'); }); + it('prefers the config registry entry over the hardcoded entry', () => { + setNetworkConfig('eip155:1', { + assets: { native: { assetId: 'eip155:1/slip44:61' } }, + } as RegistryNetworkConfig); + + expect(getAssetId({ chainId: '0x1', tokenAddress: ZERO_ADDRESS })).toBe( + 'eip155:1/slip44:61', + ); + }); + + it('prefers the config registry entry over nativeAssetIdentifiers when there is no hardcoded entry', () => { + // 0x42 (OKXChain) is not in SPOT_PRICES_SUPPORT_INFO + setNetworkConfig('eip155:66', { + assets: { native: { assetId: 'eip155:66/erc20:0xregistry' } }, + } as RegistryNetworkConfig); + + expect( + getAssetId({ + chainId: '0x42', + tokenAddress: ZERO_ADDRESS, + nativeAssetIdentifiers: { 'eip155:66': 'eip155:66/slip44:996' }, + }), + ).toBe('eip155:66/erc20:0xregistry'); + }); + + it('falls back to the hardcoded entry when the config registry has no entry for the chain', () => { + expect(getAssetId({ chainId: '0x1', tokenAddress: ZERO_ADDRESS })).toBe( + 'eip155:1/slip44:60', + ); + }); + + it('clears a chain from the config registry cache when set to undefined', () => { + setNetworkConfig('eip155:1', { + assets: { native: { assetId: 'eip155:1/slip44:61' } }, + } as RegistryNetworkConfig); + expect(getAssetId({ chainId: '0x1', tokenAddress: ZERO_ADDRESS })).toBe( + 'eip155:1/slip44:61', + ); + + setNetworkConfig('eip155:1', undefined); + expect(getAssetId({ chainId: '0x1', tokenAddress: ZERO_ADDRESS })).toBe( + 'eip155:1/slip44:60', + ); + }); + it('returns undefined for a native token with no hardcoded entry and no identifier', () => { expect( getAssetId({ chainId: '0x42', tokenAddress: ZERO_ADDRESS }), diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts index cd74cc5743..2cbe05899b 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts @@ -1,3 +1,4 @@ +import type { RegistryNetworkConfig } from '@metamask/config-registry-controller'; import { createServicePolicy, DEFAULT_CIRCUIT_BREAK_DURATION, @@ -7,7 +8,7 @@ import { handleFetch, } from '@metamask/controller-utils'; import type { ServicePolicy } from '@metamask/controller-utils'; -import type { CaipAssetType, Hex } from '@metamask/utils'; +import type { CaipAssetType, CaipChainId, Hex } from '@metamask/utils'; import { hexToNumber, KnownCaipNamespace, @@ -570,14 +571,65 @@ export function resetSupportedCurrenciesCache(): void { lastFetchedCurrencies = null; } +/** + * In-memory cache of the config registry's network configurations, keyed by + * CAIP-2 chain ID. Populated per-chain via {@link setNetworkConfig}, typically + * by TokenRatesController using ConfigRegistryController's + * `getNetworkConfigByCaip2ChainId` action for whichever chains are about to + * be priced — so the cache never holds more than what's actually been + * resolved, and always reflects the registry's current state (no bulk + * snapshot to go stale between polls). + * + * This is a module-level cache (like {@link lastFetchedSupportedNetworks}) + * rather than instance state so that {@link getAssetId} gives the same + * answer whether it's called internally by {@link fetchTokenPrices} or + * directly by external callers — there is only one source of truth for a + * given chain's native asset ID, not one per service instance. + */ +let cachedNetworkConfigs: Record = {}; + +/** + * Updates the config registry network configuration used by + * {@link getAssetId} to resolve a chain's native asset CAIP-19 ID. Should be + * called with the result of ConfigRegistryController's + * `getNetworkConfigByCaip2ChainId` action, typically right before pricing + * that chain's assets, so the cache stays current without needing to mirror + * the registry's entire network map. + * + * @param caipChainId - The CAIP-2 chain ID the config belongs to. + * @param networkConfig - The registry's network configuration for this + * chain, or undefined if the registry has no entry for it (clears any + * previously cached entry). + */ +export function setNetworkConfig( + caipChainId: CaipChainId, + networkConfig: RegistryNetworkConfig | undefined, +): void { + if (networkConfig) { + cachedNetworkConfigs[caipChainId] = networkConfig; + } else { + delete cachedNetworkConfigs[caipChainId]; + } +} + +/** + * Resets the config registry network configurations cache. + * This is primarily intended for testing purposes. + */ +export function resetNetworkConfigsCache(): void { + cachedNetworkConfigs = {}; +} + /** * Derives the CAIP-19 asset ID used to query the Price API for a token on a * given chain. * - * For native tokens, uses the hardcoded {@link SPOT_PRICES_SUPPORT_INFO} entry - * when defined, otherwise falls back to the provided native asset identifiers - * (sourced from NetworkEnablementController). For ERC20 tokens, constructs the - * CAIP-19 ID dynamically. + * For native tokens, prefers the CAIP-19 ID from the config registry cache + * (`assets.native.assetId`, see {@link setNetworkConfig}) when available, + * then the hardcoded {@link SPOT_PRICES_SUPPORT_INFO} entry, and finally + * falls back to the provided native asset identifiers (sourced from + * NetworkEnablementController). For ERC20 tokens, constructs the CAIP-19 ID + * dynamically. * * @param args - The arguments to this function. * @param args.chainId - The hexadecimal chain ID the token lives on. @@ -606,12 +658,14 @@ export function getAssetId({ nativeAddress.toLowerCase() === tokenAddress.toLowerCase(); if (isNativeToken) { + const registryAssetId = cachedNetworkConfigs[caipChainId]?.assets?.native + ?.assetId as CaipAssetType | undefined; const hardcodedId = ( SPOT_PRICES_SUPPORT_INFO as Partial> )[chainId]; - return (hardcodedId ?? nativeAssetIdentifiers[caipChainId]) as - | CaipAssetType - | undefined; + return (registryAssetId ?? + hardcodedId ?? + nativeAssetIdentifiers[caipChainId]) as CaipAssetType | undefined; } return `${caipChainId}/erc20:${tokenAddress.toLowerCase()}` as CaipAssetType; diff --git a/packages/assets-controllers/src/token-prices-service/index.test.ts b/packages/assets-controllers/src/token-prices-service/index.test.ts index 250d755a26..c9d626e2d6 100644 --- a/packages/assets-controllers/src/token-prices-service/index.test.ts +++ b/packages/assets-controllers/src/token-prices-service/index.test.ts @@ -12,6 +12,8 @@ describe('token-prices-service', () => { "resetSupportedNetworksCache", "SPOT_PRICES_SUPPORT_INFO", "getAssetId", + "setNetworkConfig", + "resetNetworkConfigsCache", ] `); }); diff --git a/packages/assets-controllers/src/token-prices-service/index.ts b/packages/assets-controllers/src/token-prices-service/index.ts index 8361bb68af..5853e08fe9 100644 --- a/packages/assets-controllers/src/token-prices-service/index.ts +++ b/packages/assets-controllers/src/token-prices-service/index.ts @@ -11,4 +11,6 @@ export { resetSupportedNetworksCache, SPOT_PRICES_SUPPORT_INFO, getAssetId, + setNetworkConfig, + resetNetworkConfigsCache, } from './codefi-v2.js'; diff --git a/packages/assets-controllers/tsconfig.build.json b/packages/assets-controllers/tsconfig.build.json index 6e3978c145..9df9b96ffb 100644 --- a/packages/assets-controllers/tsconfig.build.json +++ b/packages/assets-controllers/tsconfig.build.json @@ -62,6 +62,9 @@ }, { "path": "../remote-feature-flag-controller/tsconfig.build.json" + }, + { + "path": "../config-registry-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"], diff --git a/packages/assets-controllers/tsconfig.json b/packages/assets-controllers/tsconfig.json index 7a5d296a79..711ab71e8a 100644 --- a/packages/assets-controllers/tsconfig.json +++ b/packages/assets-controllers/tsconfig.json @@ -61,6 +61,9 @@ }, { "path": "../remote-feature-flag-controller" + }, + { + "path": "../config-registry-controller" } ], "include": ["../../types", "./src", "../../tests"] diff --git a/yarn.lock b/yarn.lock index b0c5de5280..8ae2d518e3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6045,6 +6045,7 @@ __metadata: "@metamask/approval-controller": "npm:^9.0.2" "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/base-controller": "npm:^9.1.0" + "@metamask/config-registry-controller": "npm:^2.0.1" "@metamask/contract-metadata": "npm:^2.4.0" "@metamask/controller-utils": "npm:^12.3.0" "@metamask/core-backend": "npm:^8.1.1"