fix: force timeout after 30s token detection

packages/assets-controllers/CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Add 30-second timeout protection for Accounts API calls in `TokenDetectionController` to prevent hanging requests ([#7106](https://github.com/MetaMask/core/pull/7106))
+  - Prevents token detection from hanging indefinitely on slow or unresponsive API requests
+  - Automatically falls back to RPC-based token detection when API call times out or fails
+  - Includes error logging for debugging timeout and failure events
+- Handle `unprocessedNetworks` from Accounts API responses to ensure complete token detection coverage ([#7106](https://github.com/MetaMask/core/pull/7106))
+  - When Accounts API returns networks it cannot process, those networks are automatically added to RPC detection
+  - Applies to both `TokenDetectionController` and `TokenBalancesController`
+  - Ensures all requested networks are processed even if API has partial support
+
 ## [88.0.0]
 
 ### Changed

packages/assets-controllers/src/AccountTrackerController.test.ts

@@ -594,6 +594,57 @@ describe('AccountTrackerController', () => {
           },
         );
       });
+
+      it('should create account entry when applying staked balance without native balance (line 743)', async () => {
+        // Mock returning staked balance for ADDRESS_1 and native balance for ADDRESS_2
+        // but NO native balance for ADDRESS_1 - this tests the defensive check on line 743
+        // Use lowercase addresses since queryAllAccounts: true uses lowercase
+        mockedGetTokenBalancesForMultipleAddresses.mockResolvedValueOnce({
+          tokenBalances: {
+            '0x0000000000000000000000000000000000000000': {
+              // Only ADDRESS_2 has native balance, ADDRESS_1 doesn't
+              [ADDRESS_2]: new BN('100', 16),
+            },
+          },
+          stakedBalances: {
+            // ADDRESS_1 has staked balance but no native balance
+            [ADDRESS_1]: new BN('2', 16), // 0x2
+            [ADDRESS_2]: new BN('3', 16), // 0x3
+          },
+        });
+
+        await withController(
+          {
+            options: {
+              includeStakedAssets: true,
+              getStakedBalanceForChain: mockGetStakedBalanceForChain,
+            },
+            isMultiAccountBalancesEnabled: true,
+            selectedAccount: ACCOUNT_1,
+            listAccounts: [ACCOUNT_1, ACCOUNT_2],
+          },
+          async ({ controller, refresh }) => {
+            await refresh(clock, ['mainnet'], true);
+
+            // Line 743 should have created an account entry with balance '0x0' for ADDRESS_1
+            // when applying staked balance without a native balance entry
+            expect(controller.state).toStrictEqual({
+              accountsByChainId: {
+                '0x1': {
+                  [CHECKSUM_ADDRESS_1]: {
+                    balance: '0x0', // Created by line 743 (defensive check)
+                    stakedBalance: '0x2',
+                  },
+                  [CHECKSUM_ADDRESS_2]: {
+                    balance: '0x100',
+                    stakedBalance: '0x3',
+                  },
+                },
+              },
+            });
+          },
+        );
+      });
     });
 
     describe('with networkClientId', () => {

packages/assets-controllers/src/AccountTrackerController.ts

@@ -91,14 +91,19 @@
     },
 
     async fetch(params) {
-      const balances = await rpcBalanceFetcher.fetch(params);
+      const result = await rpcBalanceFetcher.fetch(params);
 
       if (!includeStakedAssets) {
         // Filter out staked balances from the results
-        return balances.filter((balance) => balance.token === ZERO_ADDRESS);
+        return {
+          balances: result.balances.filter(
+            (balance) => balance.token === ZERO_ADDRESS,
+          ),
+          unprocessedChainIds: result.unprocessedChainIds,
+        };
       }
 
-      return balances;
+      return result;
     },
   };
 }
@@ -327,17 +332,17 @@
       (event): string => event.address,
     );
 
     this.messenger.subscribe('NetworkController:networkAdded', async () => {
       await this.refresh(this.#getNetworkClientIds());
     });
 
     this.messenger.subscribe('KeyringController:unlock', async () => {
       await this.refresh(this.#getNetworkClientIds());
     });
 
     this.messenger.subscribe(
       'TransactionController:unapprovedTransactionAdded',
       async (transactionMeta: TransactionMeta) => {
         const addresses = [transactionMeta.txParams.from];
         if (transactionMeta.txParams.to) {
           addresses.push(transactionMeta.txParams.to);
@@ -351,7 +356,7 @@
 
     this.messenger.subscribe(
       'TransactionController:transactionConfirmed',
       async (transactionMeta: TransactionMeta) => {
         const addresses = [transactionMeta.txParams.from];
         if (transactionMeta.txParams.to) {
           addresses.push(transactionMeta.txParams.to);
@@ -630,21 +635,38 @@
         }
 
         try {
-          const balances = await fetcher.fetch({
+          const result = await fetcher.fetch({
             chainIds: supportedChains,
             queryAllAccounts,
             selectedAccount,
             allAccounts,
           });
 
-          if (balances && balances.length > 0) {
-            aggregated.push(...balances);
+          if (result.balances && result.balances.length > 0) {
+            aggregated.push(...result.balances);
             // Remove chains that were successfully processed
-            const processedChains = new Set(balances.map((b) => b.chainId));
+            const processedChains = new Set(
+              result.balances.map((b) => b.chainId),
+            );
             remainingChains = remainingChains.filter(
               (chain) => !processedChains.has(chain),
             );
           }
+
+          // Add unprocessed chains back to remainingChains for next fetcher
+          if (
+            result.unprocessedChainIds &&
+            result.unprocessedChainIds.length > 0
+          ) {
+            // Only add chains that were originally requested and aren't already in remainingChains
+            const currentRemainingChains = remainingChains;
+            const chainsToAdd = result.unprocessedChainIds.filter(
+              (chainId) =>
+                supportedChains.includes(chainId) &&
+                !currentRemainingChains.includes(chainId),
+            );
+            remainingChains.push(...chainsToAdd);
+          }
         } catch (error) {
           console.warn(
             `Balance fetcher failed for chains ${supportedChains.join(', ')}: ${String(error)}`,

packages/assets-controllers/src/CurrencyRateController.test.ts

@@ -885,6 +885,100 @@ describe('CurrencyRateController', () => {
     controller.destroy();
   });
 
+  it('should set conversionDate to null when currency not found in price api response (lines 201-202)', async () => {
+    jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate());
+
+    const messenger = getCurrencyRateControllerMessenger();
+
+    const tokenPricesService = buildMockTokenPricesService();
+
+    // Mock price API response where BNB is not included
+    jest.spyOn(tokenPricesService, 'fetchExchangeRates').mockResolvedValue({
+      eth: {
+        name: 'Ether',
+        ticker: 'eth',
+        value: 1 / 1000,
+        usd: 1 / 3000,
+        currencyType: 'crypto',
+      },
+      // BNB is missing from the response
+    });
+
+    const controller = new CurrencyRateController({
+      messenger,
+      state: { currentCurrency: 'xyz' },
+      tokenPricesService,
+    });
+
+    await controller.updateExchangeRate(['ETH', 'BNB']);
+
+    const conversionDate = getStubbedDate() / 1000;
+    expect(controller.state).toStrictEqual({
+      currentCurrency: 'xyz',
+      currencyRates: {
+        ETH: {
+          conversionDate,
+          conversionRate: 1000,
+          usdConversionRate: 3000,
+        },
+        BNB: {
+          conversionDate: null, // Line 201: rate === undefined
+          conversionRate: null, // Line 202
+          usdConversionRate: null,
+        },
+      },
+    });
+
+    controller.destroy();
+  });
+
+  it('should set conversionDate to null when currency not found in crypto compare response (lines 231-232)', async () => {
+    jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate());
+    const cryptoCompareHost = 'https://min-api.cryptocompare.com';
+    nock(cryptoCompareHost)
+      .get('/data/pricemulti?fsyms=ETH,BNB&tsyms=xyz')
+      .reply(200, {
+        ETH: { XYZ: 4000.42 },
+        // BNB is missing from the response
+      })
+      .persist();
+
+    const messenger = getCurrencyRateControllerMessenger();
+    const tokenPricesService = buildMockTokenPricesService();
+
+    // Make price API fail so it falls back to CryptoCompare
+    jest
+      .spyOn(tokenPricesService, 'fetchExchangeRates')
+      .mockRejectedValue(new Error('Failed to fetch'));
+
+    const controller = new CurrencyRateController({
+      messenger,
+      state: { currentCurrency: 'xyz' },
+      tokenPricesService,
+    });
+
+    await controller.updateExchangeRate(['ETH', 'BNB']);
+
+    const conversionDate = getStubbedDate() / 1000;
+    expect(controller.state).toStrictEqual({
+      currentCurrency: 'xyz',
+      currencyRates: {
+        ETH: {
+          conversionDate,
+          conversionRate: 4000.42,
+          usdConversionRate: null,
+        },
+        BNB: {
+          conversionDate: null, // Line 231: rate === undefined
+          conversionRate: null, // Line 232
+          usdConversionRate: null,
+        },
+      },
+    });
+
+    controller.destroy();
+  });
+
   describe('useExternalServices', () => {
     it('should not fetch exchange rates when useExternalServices is false', async () => {
       const fetchMultiExchangeRateStub = jest.fn();

packages/assets-controllers/src/TokenBalancesController.test.ts

@@ -1459,7 +1459,7 @@ describe('TokenBalancesController', () => {
 
       // Mock the RPC balance fetcher's fetch method to verify the parameter
       const mockRpcFetch = jest.spyOn(RpcBalanceFetcher.prototype, 'fetch');
-      mockRpcFetch.mockResolvedValueOnce([]);
+      mockRpcFetch.mockResolvedValueOnce({ balances: [] });
 
       const { controller } = setupController({
         config: {
@@ -1771,7 +1771,7 @@ describe('TokenBalancesController', () => {
     // Mock empty aggregated results
     const mockFetcher = {
       supports: jest.fn().mockReturnValue(true),
-      fetch: jest.fn().mockResolvedValue([]), // Return empty array
+      fetch: jest.fn().mockResolvedValue({ balances: [] }), // Return empty result
     };
 
     // Replace the balance fetchers with our mock
@@ -4250,7 +4250,7 @@ describe('TokenBalancesController', () => {
 
       // Mock AccountsApiBalanceFetcher to track when line 320 logic is executed
       const mockSupports = jest.fn().mockReturnValue(true);
-      const mockApiFetch = jest.fn().mockResolvedValue([]);
+      const mockApiFetch = jest.fn().mockResolvedValue({ balances: [] });
 
       const apiBalanceFetcher = jest.requireActual(
         './multi-chain-accounts-service/api-balance-fetcher',

packages/assets-controllers/src/TokenBalancesController.ts

@@ -653,21 +653,37 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
       }
 
       try {
-        const balances = await fetcher.fetch({
+        const result = await fetcher.fetch({
           chainIds: supportedChains,
           queryAllAccounts: queryAllAccounts ?? this.#queryAllAccounts,
           selectedAccount: selected as ChecksumAddress,
           allAccounts,
         });
 
-        if (balances && balances.length > 0) {
-          aggregated.push(...balances);
+        if (result.balances && result.balances.length > 0) {
+          aggregated.push(...result.balances);
           // Remove chains that were successfully processed
-          const processedChains = new Set(balances.map((b) => b.chainId));
+          const processedChains = new Set(
+            result.balances.map((b) => b.chainId),
+          );
           remainingChains = remainingChains.filter(
             (chain) => !processedChains.has(chain),
           );
         }
+
+        // Add unprocessed chains back to remainingChains for next fetcher
+        if (
+          result.unprocessedChainIds &&
+          result.unprocessedChainIds.length > 0
+        ) {
+          const currentRemainingChains = remainingChains;
+          const chainsToAdd = result.unprocessedChainIds.filter(
+            (chainId) =>
+              supportedChains.includes(chainId) &&
+              !currentRemainingChains.includes(chainId),
+          );
+          remainingChains.push(...chainsToAdd);
+        }
       } catch (error) {
         console.warn(
           `Balance fetcher failed for chains ${supportedChains.join(', ')}: ${String(error)}`,