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
2 changes: 2 additions & 0 deletions packages/assets-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ describe('SnapDataSource', () => {
expect(response).toStrictEqual({
assetsBalance: {},
assetsInfo: {},
updateMode: 'full',
updateMode: 'merge',
});

cleanup();
Expand Down Expand Up @@ -470,7 +470,7 @@ describe('SnapDataSource', () => {
expect(response).toStrictEqual({
assetsBalance: {},
assetsInfo: {},
updateMode: 'full',
updateMode: 'merge',
});

cleanup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand All @@ -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(
{
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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);
Expand Down
75 changes: 52 additions & 23 deletions packages/assets-controller/src/middlewares/DetectionMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -47,41 +45,65 @@ 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.
*/
get assetsMiddleware(): Middleware {
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<AccountId, Caip19AssetId[]> = {};

// 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<string, unknown>,
)) {
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;
Comment thread
salimtb marked this conversation as resolved.
}
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) {
Expand All @@ -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;
}
}

Expand Down
Loading