fix/account-tracker-prevent-empty-state-update

packages/assets-controllers/CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Prevented `AccountTrackerController` from updating state with empty or unchanged account balance data during refresh ([#5942](https://github.com/MetaMask/core/pull/5942))
+  - Added guards to skip state updates when fetched balances are empty or identical to existing state
+  - Reduces unnecessary `stateChange` emissions and preserves previously-cached balances under network failure scenarios
+
 ## [68.1.0]
 
 ### Added

packages/assets-controllers/src/AccountTrackerController.ts

@@ -24,7 +24,7 @@ import { StaticIntervalPollingController } from '@metamask/polling-controller';
 import type { PreferencesControllerGetStateAction } from '@metamask/preferences-controller';
 import { assert } from '@metamask/utils';
 import { Mutex } from 'async-mutex';
-import { cloneDeep } from 'lodash';
+import { cloneDeep, isEqual } from 'lodash';
 
 import type {
   AssetsContractController,
@@ -193,13 +193,18 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
 
     this.messagingSystem.subscribe(
       'AccountsController:selectedEvmAccountChange',
-      // TODO: Either fix this lint violation or explain why it's necessary to ignore.
-      // eslint-disable-next-line @typescript-eslint/no-misused-promises
-      () => this.refresh(this.#getNetworkClientIds()),
+      (newAddress, prevAddress) => {
+        if (newAddress !== prevAddress) {
+          // Making an async call for this new event
+          // eslint-disable-next-line @typescript-eslint/no-floating-promises
+          this.refresh(this.#getNetworkClientIds());
+        }
+      },
+      (event): string => event.address,
     );
   }
 
-  private syncAccounts(newChainId: string) {
+  private syncAccounts(newChainIds: string[]) {
     const accountsByChainId = cloneDeep(this.state.accountsByChainId);
     const { selectedNetworkClientId } = this.messagingSystem.call(
       'NetworkController:getState',
@@ -213,12 +218,15 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
 
     const existing = Object.keys(accountsByChainId?.[currentChainId] ?? {});
 
-    if (!accountsByChainId[newChainId]) {
-      accountsByChainId[newChainId] = {};
-      existing.forEach((address) => {
-        accountsByChainId[newChainId][address] = { balance: '0x0' };
-      });
-    }
+    // Initialize new chain IDs if they don't exist
+    newChainIds.forEach((newChainId) => {
+      if (!accountsByChainId[newChainId]) {
+        accountsByChainId[newChainId] = {};
+        existing.forEach((address) => {
+          accountsByChainId[newChainId][address] = { balance: '0x0' };
+        });
+      }
+    });
 
     // Note: The address from the preferences controller are checksummed
     // The addresses from the accounts controller are lowercased
@@ -249,9 +257,11 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
       });
     });
 
-    this.update((state) => {
-      state.accountsByChainId = accountsByChainId;
-    });
+    if (!isEqual(this.state.accountsByChainId, accountsByChainId)) {
+      this.update((state) => {
+        state.accountsByChainId = accountsByChainId;
+      });
+    }
   }
 
   /**
@@ -327,11 +337,17 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
     );
     const releaseLock = await this.#refreshMutex.acquire();
     try {
+      const chainIds = networkClientIds.map((networkClientId) => {
+        const { chainId } = this.#getCorrectNetworkClient(networkClientId);
+        return chainId;
+      });
+
+      this.syncAccounts(chainIds);
+
       // Create an array of promises for each networkClientId
       const updatePromises = networkClientIds.map(async (networkClientId) => {
         const { chainId, ethQuery } =
           this.#getCorrectNetworkClient(networkClientId);
-        this.syncAccounts(chainId);
         const { accountsByChainId } = this.state;
         const { isMultiAccountBalancesEnabled } = this.messagingSystem.call(
           'PreferencesController:getState',
@@ -394,15 +410,28 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
       // Wait for all networkClientId updates to settle in parallel
       const allResults = await Promise.allSettled(updatePromises);
 
-      // Update the state once all networkClientId updates are completed
+      // Build a _copy_ of the current state and track whether anything changed
+      const nextAccountsByChainId: AccountTrackerControllerState['accountsByChainId'] =
+        cloneDeep(this.state.accountsByChainId);
+      let hasChanges = false;
+
       allResults.forEach((result) => {
         if (result.status === 'fulfilled') {
           const { chainId, accountsForChain } = result.value;
-          this.update((state) => {
-            state.accountsByChainId[chainId] = accountsForChain;
-          });
+          // Only mark as changed if the incoming data differs
+          if (!isEqual(nextAccountsByChainId[chainId], accountsForChain)) {
+            nextAccountsByChainId[chainId] = accountsForChain;
+            hasChanges = true;
+          }
         }
       });
+
+      // 👇🏻 call `update` only when something is new / different
+      if (hasChanges) {
+        this.update((state) => {
+          state.accountsByChainId = nextAccountsByChainId;
+        });
+      }
     } finally {
       releaseLock();
     }