diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 10c4f7edb6..a558452972 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `AssetsController` reconciliate and self-heals stale assetInfo metadata types ([#9099](https://github.com/MetaMask/core/pull/9099)) +- `DetectionMiddleware` no longer triggers redundant metadata and price fetches for assets already present in `assetsBalance` or `assetsInfo` state ([#9215](https://github.com/MetaMask/core/pull/9215)) +- `AccountsApiDataSource` and `SnapDataSource` now use `merge` update mode instead of `full`, preventing assets on custom or partially-supported networks from being incorrectly wiped on each poll ([#9215](https://github.com/MetaMask/core/pull/9215)) ## [9.0.1] diff --git a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts index c6b9853c4a..57200f79b7 100644 --- a/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts +++ b/packages/assets-controller/src/data-sources/AccountsApiDataSource.ts @@ -342,7 +342,7 @@ export class AccountsApiDataSource extends AbstractDataSource< ); response.assetsBalance = assetsBalance; - response.updateMode = 'full'; + response.updateMode = 'merge'; } catch (error) { log('Fetch FAILED', { error, chains: chainsToFetch }); diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts index 638f12545e..8518b3d694 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.test.ts @@ -435,7 +435,7 @@ describe('SnapDataSource', () => { expect(response).toStrictEqual({ assetsBalance: {}, assetsInfo: {}, - updateMode: 'full', + updateMode: 'merge', }); cleanup(); @@ -470,7 +470,7 @@ describe('SnapDataSource', () => { expect(response).toStrictEqual({ assetsBalance: {}, assetsInfo: {}, - updateMode: 'full', + updateMode: 'merge', }); cleanup(); diff --git a/packages/assets-controller/src/data-sources/SnapDataSource.ts b/packages/assets-controller/src/data-sources/SnapDataSource.ts index c1925e5355..17da7b0b5e 100644 --- a/packages/assets-controller/src/data-sources/SnapDataSource.ts +++ b/packages/assets-controller/src/data-sources/SnapDataSource.ts @@ -445,13 +445,13 @@ export class SnapDataSource extends AbstractDataSource< return {}; } if (!request?.accountsWithSupportedChains?.length) { - return { assetsBalance: {}, assetsInfo: {}, updateMode: 'full' }; + return { assetsBalance: {}, assetsInfo: {}, updateMode: 'merge' }; } const results: DataResponse = { assetsBalance: {}, assetsInfo: {}, - updateMode: 'full', + updateMode: 'merge', }; // Fetch balances for each account using its snap ID from metadata diff --git a/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts b/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts index 084759b59e..5838adcd73 100644 --- a/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts +++ b/packages/assets-controller/src/middlewares/DetectionMiddleware.test.ts @@ -151,7 +151,7 @@ describe('DetectionMiddleware', () => { expect(next).toHaveBeenCalledWith(context); }); - it('includes all balance assets in detectedAssets even when they have metadata', async () => { + it('skips balance assets that already have metadata in state', async () => { const { middleware } = setupController(); const context = createMiddlewareContext( { @@ -170,14 +170,12 @@ describe('DetectionMiddleware', () => { await middleware.assetsMiddleware(context, next); - // All assets in balance are included so prices (and metadata when needed) are fetched - expect(context.response.detectedAssets).toStrictEqual({ - [MOCK_ACCOUNT_ID]: [MOCK_ASSET_1, MOCK_NATIVE_ASSET], - }); + // Both assets are already in state.assetsInfo → nothing new to detect + expect(context.response.detectedAssets).toBeUndefined(); expect(next).toHaveBeenCalledWith(context); }); - it('includes all balance assets in mixed scenario (metadata presence is ignored)', async () => { + it('only detects assets not already in state (mixed scenario)', async () => { const { middleware } = setupController(); const context = createMiddlewareContext( { @@ -197,13 +195,14 @@ describe('DetectionMiddleware', () => { await middleware.assetsMiddleware(context, next); + // MOCK_ASSET_1 is already in state.assetsInfo → skipped; the other two are new expect(context.response.detectedAssets).toStrictEqual({ - [MOCK_ACCOUNT_ID]: [MOCK_ASSET_1, MOCK_ASSET_2, MOCK_NATIVE_ASSET], + [MOCK_ACCOUNT_ID]: [MOCK_ASSET_2, MOCK_NATIVE_ASSET], }); expect(next).toHaveBeenCalledWith(context); }); - it('handles multiple accounts', async () => { + it('handles multiple accounts, skipping assets already in state per account', async () => { const { middleware } = setupController(); const account2Id = 'account-2-id'; const context = createMiddlewareContext( @@ -226,14 +225,15 @@ describe('DetectionMiddleware', () => { await middleware.assetsMiddleware(context, next); + // MOCK_NATIVE_ASSET is in state.assetsInfo → skipped for account2; MOCK_ASSET_1 and MOCK_ASSET_2 are new expect(context.response.detectedAssets).toStrictEqual({ [MOCK_ACCOUNT_ID]: [MOCK_ASSET_1], - [account2Id]: [MOCK_ASSET_2, MOCK_NATIVE_ASSET], + [account2Id]: [MOCK_ASSET_2], }); expect(next).toHaveBeenCalledWith(context); }); - it('includes all balance assets per account regardless of metadata', async () => { + it('skips an account entirely when all its balance assets are already in state', async () => { const { middleware } = setupController(); const account2Id = 'account-2-id'; const context = createMiddlewareContext( @@ -255,9 +255,9 @@ describe('DetectionMiddleware', () => { await middleware.assetsMiddleware(context, next); - // Both accounts get their balance assets so prices/metadata can be fetched + // MOCK_ASSET_1 is already in state.assetsInfo → MOCK_ACCOUNT_ID produces no new assets; + // MOCK_ASSET_2 is new → account2 is included expect(context.response.detectedAssets).toStrictEqual({ - [MOCK_ACCOUNT_ID]: [MOCK_ASSET_1], [account2Id]: [MOCK_ASSET_2], }); expect(next).toHaveBeenCalledWith(context); diff --git a/packages/assets-controller/src/middlewares/DetectionMiddleware.ts b/packages/assets-controller/src/middlewares/DetectionMiddleware.ts index f5827ea3d0..9ca75db6ab 100644 --- a/packages/assets-controller/src/middlewares/DetectionMiddleware.ts +++ b/packages/assets-controller/src/middlewares/DetectionMiddleware.ts @@ -19,16 +19,14 @@ createModuleLogger(projectLogger, CONTROLLER_NAME); * DetectionMiddleware builds the set of assets that downstream sources use for * metadata and price fetching. * - * This middleware: - * - Includes every asset that appears in response.assetsBalance (so prices and - * metadata are fetched for existing assets as well as new ones) - * - Includes each account's custom assets from state (so custom tokens get - * metadata and prices even when they have no balance yet) - * - Fills response.detectedAssets with these asset IDs per account - * - * TokenDataSource and PriceDataSource both key off detectedAssets. TokenDataSource - * then filters to only fetch metadata for assets that lack it; PriceDataSource - * fetches prices for all detected assets. + * An asset is included in `detectedAssets` only when it is genuinely new: + * - Assets from `response.assetsBalance` are included only if they are absent + * from BOTH `state.assetsBalance` (never tracked before) AND `state.assetsInfo` + * (no metadata yet). Assets already present in either collection are considered + * known and are intentionally excluded — PriceDataSource's own subscription + * handles periodic refreshes for those. + * - Each account's custom assets from state are always included because they + * may have no balance yet and are explicitly managed by the user. * * Usage: * ```typescript @@ -47,9 +45,10 @@ export class DetectionMiddleware { * Get the middleware that builds detectedAssets for metadata and price fetching. * * This middleware: - * 1. Includes all assets from response.assetsBalance (so prices are fetched for existing assets too) - * 2. Merges each account's custom assets from state - * 3. Fills response.detectedAssets with these asset IDs per account + * 1. Includes assets from response.assetsBalance that are absent from both + * state.assetsBalance and state.assetsInfo (brand-new assets only) + * 2. Always includes each account's custom assets from state + * 3. Fills response.detectedAssets with the resulting asset IDs per account * * @returns The middleware function for the assets pipeline. */ @@ -57,31 +56,54 @@ export class DetectionMiddleware { return forDataTypes(['balance'], async (ctx, next) => { const { request, response } = ctx; - // Get state for custom assets + // Get state for custom assets, existing balances, and existing metadata const state = ctx.getAssetsState(); - const { customAssets: stateCustomAssets } = state; + const { + customAssets: stateCustomAssets, + assetsBalance: stateAssetsBalance, + assetsInfo: stateAssetsInfo, + } = state; const detectedAssets: Record = {}; - // 1. From balance response: include every asset with balance (so prices + metadata path include existing assets) + // 1. From balance response: only include assets that are genuinely new — + // not already present in state.assetsBalance or state.assetsInfo. if (response.assetsBalance) { for (const [accountId, accountBalances] of Object.entries( response.assetsBalance, )) { const detected: Caip19AssetId[] = []; + const stateAccountBalances = stateAssetsBalance[accountId] ?? {}; + for (const assetId of Object.keys( accountBalances as Record, )) { - detected.push(assetId as Caip19AssetId); + const caipAssetId = assetId as Caip19AssetId; + // Skip if already tracked in state balances or already has metadata + if ( + stateAccountBalances[caipAssetId] !== undefined || + stateAssetsInfo[caipAssetId] !== undefined + ) { + continue; + } + detected.push(caipAssetId); } - // Merge this account's custom assets from state + // Merge custom assets for this account, applying the same filter: + // skip if already in state balance or already has metadata. const customForAccount = stateCustomAssets?.[accountId] ?? []; for (const assetId of customForAccount) { - if (!detected.includes(assetId)) { - detected.push(assetId); + if (detected.includes(assetId)) { + continue; + } + if ( + stateAccountBalances[assetId] !== undefined || + stateAssetsInfo[assetId] !== undefined + ) { + continue; } + detected.push(assetId); } if (detected.length > 0) { @@ -90,15 +112,22 @@ export class DetectionMiddleware { } } - // 2. Accounts in request that weren't in balance response: include their custom assets + // 2. Accounts in request that weren't in balance response: include their + // custom assets that are not yet in state. for (const { account } of request.accountsWithSupportedChains) { const accountId = account.id; if (detectedAssets[accountId]) { continue; } + const stateAccountBalances = stateAssetsBalance[accountId] ?? {}; const customForAccount = stateCustomAssets?.[accountId] ?? []; - if (customForAccount.length > 0) { - detectedAssets[accountId] = customForAccount; + const newCustomAssets = customForAccount.filter((assetId) => { + const inBalance = stateAccountBalances[assetId] !== undefined; + const inInfo = stateAssetsInfo[assetId] !== undefined; + return !inBalance && !inInfo; + }); + if (newCustomAssets.length > 0) { + detectedAssets[accountId] = newCustomAssets; } }