fix(assets-controller): Coalesce assets price and fix middleware

packages/assets-controller/src/AssetsController.ts

@@ -1204,26 +1204,12 @@ export class AssetsController extends BaseController<
     const removedChains = previous.filter((ch) => !activeChains.includes(ch));
 
     if (addedChains.length > 0 || removedChains.length > 0) {
-      // Refresh subscriptions to use updated data source availability
+      // Refresh subscriptions to use updated data source availability.
+      // No one-time fetch needed here — #handleEnabledNetworksChanged
+      // handles fetches when the user enables a network, and
+      // #subscribeAssets re-subscribes with the correct chain assignment.
       this.#subscribeAssets();
     }
-
-    // If chains were added and we have selected accounts, do one-time fetch
-    if (addedChains.length > 0 && this.#getSelectedAccounts().length > 0) {
-      const addedEnabledChains = addedChains.filter((chain) =>
-        this.#enabledChains.has(chain),
-      );
-      if (addedEnabledChains.length > 0) {
-        log('Fetching balances for newly added chains', { addedEnabledChains });
-        this.getAssets(this.#getSelectedAccounts(), {
-          chainIds: addedEnabledChains,
-          forceUpdate: true,
-          updateMode: 'merge',
-        }).catch((error) => {
-          log('Failed to fetch balance for added chains', { error });
-        });
-      }
-    }
   }
 
   /**
@@ -1829,6 +1815,9 @@ export class AssetsController extends BaseController<
       return;
     }
 
+    // Currency changed — old cached prices are in the wrong currency.
+    this.#priceDataSource.invalidatePriceCache();
+
     this.getAssets(this.#getSelectedAccounts(), {
       forceUpdate: true,
       dataTypes: ['price'],

packages/assets-controller/src/data-sources/PriceDataSource.test.ts

@@ -556,13 +556,12 @@ describe('PriceDataSource', () => {
 
     expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
 
-    jest.advanceTimersByTime(10000);
-    await Promise.resolve();
-
-    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(2);
-
-    jest.advanceTimersByTime(50000);
-    await Promise.resolve();
+    // Advance one tick at a time, flushing microtasks between each so the
+    // async pollFn completes and inflight promises settle before the next tick.
+    for (let i = 2; i <= 7; i++) {
+      jest.advanceTimersByTime(10000);
+      await jest.advanceTimersByTimeAsync(0);
+    }
 
     expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(7);
 
@@ -857,6 +856,222 @@ describe('PriceDataSource', () => {
     controller.destroy();
   });
 
+  it('skips fetching prices for assets fetched within the freshness TTL', async () => {
+    const { controller, apiClient, getAssetsState } = setupController({
+      balanceState: {
+        'mock-account-id': {
+          [MOCK_NATIVE_ASSET]: { amount: '1000000000000000000' },
+        },
+      },
+      priceResponse: {
+        [MOCK_NATIVE_ASSET]: createMockPriceData(2500),
+      },
+    });
+
+    // First fetch — asset is stale, API is called
+    await controller.fetch(createDataRequest(), getAssetsState);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    // Second fetch immediately after — asset is fresh, API is NOT called again
+    await controller.fetch(createDataRequest(), getAssetsState);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    controller.destroy();
+  });
+
+  it('re-fetches prices after the freshness TTL expires', async () => {
+    const { controller, apiClient, getAssetsState } = setupController({
+      pollInterval: 10_000,
+      balanceState: {
+        'mock-account-id': {
+          [MOCK_NATIVE_ASSET]: { amount: '1000000000000000000' },
+        },
+      },
+      priceResponse: {
+        [MOCK_NATIVE_ASSET]: createMockPriceData(2500),
+      },
+    });
+
+    await controller.fetch(createDataRequest(), getAssetsState);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    // Advance past the TTL (pollInterval = 10s is used as freshness TTL)
+    jest.advanceTimersByTime(11_000);
+
+    await controller.fetch(createDataRequest(), getAssetsState);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(2);
+
+    controller.destroy();
+  });
+
+  it('invalidatePriceCache allows re-fetching assets that were fresh', async () => {
+    const { controller, apiClient } = setupController({
+      priceResponse: {
+        [MOCK_TOKEN_ASSET]: createMockPriceData(1.0),
+      },
+    });
+
+    const next = jest.fn().mockResolvedValue(undefined);
+
+    // First call — populates freshness cache
+    const context1 = createMiddlewareContext({
+      request: createDataRequest({ assetsForPriceUpdate: [MOCK_TOKEN_ASSET] }),
+      response: {},
+    });
+    await controller.assetsMiddleware(context1, next);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    // Second call — skipped (fresh)
+    const context2 = createMiddlewareContext({
+      request: createDataRequest({ assetsForPriceUpdate: [MOCK_TOKEN_ASSET] }),
+      response: {},
+    });
+    await controller.assetsMiddleware(context2, next);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    // Invalidate cache, then fetch again — API is called
+    controller.invalidatePriceCache();
+    const context3 = createMiddlewareContext({
+      request: createDataRequest({ assetsForPriceUpdate: [MOCK_TOKEN_ASSET] }),
+      response: {},
+    });
+    await controller.assetsMiddleware(context3, next);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(2);
+
+    controller.destroy();
+  });
+
+  it('coalesces parallel fetches for the same asset into a single API call', async () => {
+    let resolveApi: ((value: Record<string, unknown>) => void) | undefined;
+    const apiPromise = new Promise<Record<string, unknown>>((resolve) => {
+      resolveApi = resolve;
+    });
+
+    const { controller, apiClient } = setupController({
+      priceResponse: {
+        [MOCK_TOKEN_ASSET]: createMockPriceData(1.0),
+      },
+    });
+
+    // Make the API call hang until we resolve manually
+    apiClient.prices.fetchV3SpotPrices.mockReturnValue(apiPromise);
+
+    const next = jest.fn().mockResolvedValue(undefined);
+
+    // Fire two parallel middleware calls for the same asset
+    const context1 = createMiddlewareContext({
+      request: createDataRequest({ assetsForPriceUpdate: [MOCK_TOKEN_ASSET] }),
+      response: {},
+    });
+    const context2 = createMiddlewareContext({
+      request: createDataRequest({ assetsForPriceUpdate: [MOCK_TOKEN_ASSET] }),
+      response: {},
+    });
+
+    const promise1 = controller.assetsMiddleware(context1, next);
+    const promise2 = controller.assetsMiddleware(context2, next);
+
+    // Only ONE API call should have been made (second call joins inflight)
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    // Resolve the API
+    expect(resolveApi).toBeDefined();
+    if (resolveApi) {
+      resolveApi({ [MOCK_TOKEN_ASSET]: createMockPriceData(1.0) });
+    }
+    await Promise.all([promise1, promise2]);
+
+    // Still only one API call total
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    // Both contexts received the price
+    expect(context1.response.assetsPrice?.[MOCK_TOKEN_ASSET]).toBeDefined();
+    expect(context2.response.assetsPrice?.[MOCK_TOKEN_ASSET]).toBeDefined();
+
+    controller.destroy();
+  });
+
+  it('freshness is per-asset — stale assets are fetched while fresh ones are skipped', async () => {
+    const { controller, apiClient, getAssetsState } = setupController({
+      balanceState: {
+        'mock-account-id': {
+          [MOCK_NATIVE_ASSET]: { amount: '1000000000000000000' },
+        },
+      },
+      priceResponse: {
+        [MOCK_NATIVE_ASSET]: createMockPriceData(2500),
+        [MOCK_TOKEN_ASSET]: createMockPriceData(1.0),
+      },
+    });
+
+    // Fetch only MOCK_NATIVE_ASSET via balance state
+    await controller.fetch(createDataRequest(), getAssetsState);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledWith(
+      [MOCK_NATIVE_ASSET],
+      expect.anything(),
+    );
+
+    // Now middleware requests MOCK_TOKEN_ASSET (not yet fetched) and MOCK_NATIVE_ASSET (fresh)
+    const next = jest.fn().mockResolvedValue(undefined);
+    const context = createMiddlewareContext({
+      request: createDataRequest({
+        assetsForPriceUpdate: [MOCK_TOKEN_ASSET, MOCK_NATIVE_ASSET],
+      }),
+      response: {},
+    });
+    await controller.assetsMiddleware(context, next);
+
+    // Only MOCK_TOKEN_ASSET should be sent to the API (MOCK_NATIVE_ASSET is fresh)
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(2);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenLastCalledWith(
+      [MOCK_TOKEN_ASSET],
+      expect.anything(),
+    );
+
+    controller.destroy();
+  });
+
+  it('destroy clears the freshness cache', async () => {
+    const { controller, apiClient, getAssetsState } = setupController({
+      balanceState: {
+        'mock-account-id': {
+          [MOCK_NATIVE_ASSET]: { amount: '1000000000000000000' },
+        },
+      },
+      priceResponse: {
+        [MOCK_NATIVE_ASSET]: createMockPriceData(2500),
+      },
+    });
+
+    await controller.fetch(createDataRequest(), getAssetsState);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(1);
+
+    controller.destroy();
+
+    // Re-create a new controller instance to verify state is gone
+    // (destroy clears the priceFetchedAt map — for the same instance it won't poll after destroy)
+    const controller2 = new PriceDataSource({
+      queryApiClient:
+        apiClient as unknown as PriceDataSourceOptions['queryApiClient'],
+      getSelectedCurrency: (): SupportedCurrency => 'usd',
+    });
+
+    const getAssetsState2 = (): AssetsControllerStateInternal =>
+      ({
+        assetsBalance: {
+          'mock-account-id': {
+            [MOCK_NATIVE_ASSET]: { amount: '1000000000000000000' },
+          },
+        },
+      }) as unknown as AssetsControllerStateInternal;
+
+    await controller2.fetch(createDataRequest(), getAssetsState2);
+    expect(apiClient.prices.fetchV3SpotPrices).toHaveBeenCalledTimes(2);
+
+    controller2.destroy();
+  });
+
   it('destroy cleans up all subscriptions', async () => {
     const polygonAsset =
       'eip155:137/erc20:0x0000000000000000000000000000000000001010' as Caip19AssetId;