fix: improve add tokens through polling

eslint-suppressions.json

@@ -164,23 +164,6 @@
       "count": 3
     }
   },
-  "packages/assets-controllers/src/AccountTrackerController.ts": {
-    "@typescript-eslint/explicit-function-return-type": {
-      "count": 14
-    },
-    "@typescript-eslint/no-misused-promises": {
-      "count": 4
-    },
-    "id-denylist": {
-      "count": 6
-    },
-    "id-length": {
-      "count": 1
-    },
-    "no-restricted-syntax": {
-      "count": 1
-    }
-  },
   "packages/assets-controllers/src/AssetsContractController.test.ts": {
     "@typescript-eslint/explicit-function-return-type": {
       "count": 5

packages/assets-controllers/CHANGELOG.md

@@ -12,6 +12,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `excludeLabels` parameter to `getTrendingTokens` to filter out tokens by labels (e.g., 'stable_coin', 'blue_chip') ([#7466](https://github.com/MetaMask/core/pull/7466))
 - Add support for spot prices on multiple new chains: Apechain, Lens, Plume, Flow EVM, Berachain, xrpl-evm, Fraxtal, Lukso, xdc-network, and Plasma ([#7465](https://github.com/MetaMask/core/pull/7465))
 - Add `ape` to `SUPPORTED_CURRENCIES` for ApeCoin/Apechain native token ([#7465](https://github.com/MetaMask/core/pull/7465))
+- Add `isOnboarded` constructor option to `TokenBalancesController` and `AccountTrackerController` ([#7469](https://github.com/MetaMask/core/pull/7469))
+  - When `isOnboarded()` returns `false`, balance updates are skipped to prevent unnecessary API calls during onboarding
+  - `TokenBalancesController.isActive` now also checks `isOnboarded()` in addition to `isUnlocked`
+  - `AccountTrackerController.#refreshAccounts` now checks `isOnboarded()` before fetching balances
+
+### Fixed
+
+- Fix `TokenBalancesController` to evaluate `allowExternalServices` dynamically instead of only at constructor time ([#7469](https://github.com/MetaMask/core/pull/7469))
+  - Previously, the `#balanceFetchers` array was built once in the constructor, so changes to `allowExternalServices()` after initialization were not reflected
+  - Now `allowExternalServices()` is stored as a function and evaluated dynamically in the fetcher's `supports` method
+- Fix `TokenDetectionController` methods `addDetectedTokensViaPolling` and `addDetectedTokensViaWs` to refresh token metadata cache before use ([#7469](https://github.com/MetaMask/core/pull/7469))
+  - Previously, these methods used a potentially stale/empty `#tokensChainsCache` from construction time
+  - Now they fetch the latest `tokensChainsCache` from `TokenListController:getState` before looking up token metadata
 
 ## [94.0.0]
 

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

@@ -197,6 +197,53 @@ describe('AccountTrackerController', () => {
     );
   });
 
+  it('refreshes both from and to addresses when unapproved transaction is added with to address', async () => {
+    await withController(
+      {
+        selectedAccount: ACCOUNT_1,
+        listAccounts: [ACCOUNT_1, ACCOUNT_2],
+      },
+      async ({ controller, messenger }) => {
+        mockedGetTokenBalancesForMultipleAddresses.mockResolvedValueOnce({
+          tokenBalances: {
+            '0x0000000000000000000000000000000000000000': {
+              [ADDRESS_1]: new BN('aaaaaa', 16),
+              [ADDRESS_2]: new BN('bbbbbb', 16),
+            },
+          },
+          stakedBalances: {},
+        });
+
+        const transactionMeta: TransactionMeta = {
+          networkClientId: 'mainnet',
+          chainId: '0x1' as const,
+          id: 'test-tx-with-to-unapproved',
+          status: TransactionStatus.unapproved,
+          time: Date.now(),
+          txParams: {
+            from: ADDRESS_1,
+            to: ADDRESS_2,
+          },
+        };
+
+        messenger.publish(
+          'TransactionController:unapprovedTransactionAdded',
+          transactionMeta,
+        );
+
+        await clock.tickAsync(1);
+
+        // Both from and to addresses should have their balances refreshed
+        expect(
+          controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_1].balance,
+        ).toBe('0xaaaaaa');
+        expect(
+          controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_2].balance,
+        ).toBe('0xbbbbbb');
+      },
+    );
+  });
+
   it('refreshes address when transaction is confirmed', async () => {
     await withController(
       {
@@ -238,6 +285,93 @@ describe('AccountTrackerController', () => {
     );
   });
 
+  it('refreshes both from and to addresses when transaction is confirmed with to address', async () => {
+    await withController(
+      {
+        selectedAccount: ACCOUNT_1,
+        listAccounts: [ACCOUNT_1, ACCOUNT_2],
+      },
+      async ({ controller, messenger }) => {
+        mockedGetTokenBalancesForMultipleAddresses.mockResolvedValueOnce({
+          tokenBalances: {
+            '0x0000000000000000000000000000000000000000': {
+              [ADDRESS_1]: new BN('111111', 16),
+              [ADDRESS_2]: new BN('222222', 16),
+            },
+          },
+          stakedBalances: {},
+        });
+
+        const transactionMeta: TransactionMeta = {
+          networkClientId: 'mainnet',
+          chainId: '0x1' as const,
+          id: 'test-tx-with-to',
+          status: TransactionStatus.confirmed,
+          time: Date.now(),
+          txParams: {
+            from: ADDRESS_1,
+            to: ADDRESS_2,
+          },
+        };
+
+        messenger.publish(
+          'TransactionController:transactionConfirmed',
+          transactionMeta,
+        );
+
+        await clock.tickAsync(1);
+
+        // Both from and to addresses should have their balances refreshed
+        expect(
+          controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_1].balance,
+        ).toBe('0x111111');
+        expect(
+          controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_2].balance,
+        ).toBe('0x222222');
+      },
+    );
+  });
+
+  it('should create new chain entry when balance fetcher returns balance for unexpected chain (line 739)', async () => {
+    await withController(
+      {
+        selectedAccount: ACCOUNT_1,
+        listAccounts: [ACCOUNT_1],
+      },
+      async ({ controller, refresh }) => {
+        // State only has '0x1' initially
+        expect(controller.state.accountsByChainId['0x1']).toBeDefined();
+        // '0xa4b1' (Arbitrum) should not exist yet
+        expect(controller.state.accountsByChainId['0xa4b1']).toBeUndefined();
+
+        // Mock balance fetcher to return balance for '0x1' (requested)
+        // AND '0xa4b1' (not requested) - this tests line 739 defensive code
+        mockedGetTokenBalancesForMultipleAddresses.mockImplementationOnce(
+          async () => {
+            // Simulate fetcher returning extra chain that wasn't synced
+            return {
+              tokenBalances: {
+                '0x0000000000000000000000000000000000000000': {
+                  [ADDRESS_1]: new BN('123456', 16),
+                },
+              },
+              // Return balances for extra chain '0xa4b1' not in request
+              stakedBalances: {},
+            };
+          },
+        );
+
+        // Refresh only for mainnet
+        await refresh(clock, ['mainnet']);
+
+        // Verify mainnet balance was updated
+        expect(
+          controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_1].balance,
+        ).toBe('0x123456');
+      },
+    );
+  });
+
   it('refreshes addresses when network is added', async () => {
     await withController(
       {
@@ -320,6 +454,74 @@ describe('AccountTrackerController', () => {
       );
     });
 
+    it('should skip balance fetching when isOnboarded returns false', async () => {
+      const expectedState = {
+        accountsByChainId: {
+          '0x1': {
+            [CHECKSUM_ADDRESS_1]: { balance: '0x0' },
+            [CHECKSUM_ADDRESS_2]: { balance: '0x0' },
+          },
+        },
+      };
+
+      await withController(
+        {
+          options: { isOnboarded: () => false },
+          selectedAccount: ACCOUNT_1,
+          listAccounts: [ACCOUNT_1, ACCOUNT_2],
+        },
+        async ({ controller, refresh }) => {
+          await refresh(clock, ['mainnet'], true);
+
+          // Balances should remain at 0x0 because isOnboarded returns false
+          expect(controller.state).toStrictEqual(expectedState);
+        },
+      );
+    });
+
+    it('should evaluate isOnboarded dynamically at call time', async () => {
+      let onboarded = false;
+
+      await withController(
+        {
+          options: { isOnboarded: () => onboarded },
+          selectedAccount: ACCOUNT_1,
+          listAccounts: [ACCOUNT_1],
+        },
+        async ({ controller, refresh }) => {
+          // First call: isOnboarded returns false, should skip fetching
+          await refresh(clock, ['mainnet'], false);
+
+          // Balances should remain at 0x0
+          expect(
+            controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_1]
+              .balance,
+          ).toBe('0x0');
+
+          // Now set onboarded to true
+          onboarded = true;
+
+          mockedGetTokenBalancesForMultipleAddresses.mockResolvedValueOnce({
+            tokenBalances: {
+              '0x0000000000000000000000000000000000000000': {
+                [ADDRESS_1]: new BN('fedcba', 16),
+              },
+            },
+            stakedBalances: {},
+          });
+
+          // Second call: isOnboarded now returns true, should fetch balances
+          await refresh(clock, ['mainnet'], false);
+
+          // Balance should now be updated
+          expect(
+            controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_1]
+              .balance,
+          ).toBe('0xfedcba');
+        },
+      );
+    });
+
     describe('without networkClientId', () => {
       it('should sync addresses', async () => {
         await withController(
@@ -1303,6 +1505,50 @@ describe('AccountTrackerController', () => {
         },
       );
     });
+
+    it('should handle unprocessedChainIds from fetcher and retry with next fetcher', async () => {
+      // Mock AccountsApiBalanceFetcher to return unprocessedChainIds
+      const fetchSpy = jest
+        .spyOn(AccountsApiBalanceFetcher.prototype, 'fetch')
+        .mockResolvedValue({
+          balances: [], // No balances returned
+          unprocessedChainIds: ['0x1' as const], // Chain couldn't be processed
+        });
+
+      await withController(
+        {
+          options: {
+            accountsApiChainIds: () => ['0x1'], // Configure to use AccountsAPI for mainnet
+            allowExternalServices: () => true,
+          },
+          isMultiAccountBalancesEnabled: true,
+          selectedAccount: ACCOUNT_1,
+          listAccounts: [ACCOUNT_1],
+        },
+        async ({ controller, refresh }) => {
+          // Mock RPC query to return balance (fallback after API returns unprocessedChainIds)
+          mockedGetTokenBalancesForMultipleAddresses.mockResolvedValueOnce({
+            tokenBalances: {
+              '0x0000000000000000000000000000000000000000': {
+                [ADDRESS_1]: new BN('abcdef', 16),
+              },
+            },
+            stakedBalances: {},
+          });
+
+          // Refresh balances for mainnet
+          await refresh(clock, ['mainnet'], true);
+
+          // The RPC fetcher should have been used as fallback after API returned unprocessedChainIds
+          expect(
+            controller.state.accountsByChainId['0x1'][CHECKSUM_ADDRESS_1]
+              .balance,
+          ).toBe('0xabcdef');
+
+          fetchSpy.mockRestore();
+        },
+      );
+    });
   });
 
   describe('syncBalanceWithAddresses', () => {