Skip to content

Commit c154788

Browse files
authored
fix: improve balance fetch and not reset to 0 (#6743)
## Explanation ### Current State and Problem Previously, the `TokenBalancesController` would update token balances, native balances, and staked balances regardless of whether the values had actually changed. This led to several inefficiencies: 1. **Unnecessary state updates**: The controller would trigger state changes even when balance values remained the same 2. **Performance overhead**: Unchanged balances would still go through the entire update pipeline, causing unnecessary re-renders and processing 3. **Reset behavior**: Balances could be reset to zero and then immediately updated with the same value, creating unnecessary intermediate states ### Solution This PR implements value comparison logic to skip updates when balances haven't actually changed: 1. **Token Balance Updates**: Added comparison between `currentBalance` and `newBalance` before updating the state. Only updates are applied when values differ, preventing unnecessary state mutations. 2. **Native Balance Updates**: Enhanced the filter logic to compare existing balances from `AccountTrackerController` state with new balance values. Only balances that have actually changed are included in the update batch sent to `AccountTrackerController:updateNativeBalances`. 3. **Staked Balance Updates**: Similar optimization for staked balances, comparing current staked balance values with new ones before including them in the update batch sent to `AccountTrackerController:updateStakedBalances`. ### Key Improvements - **Reduced State Mutations**: Prevents unnecessary state updates when balance values are identical - **Better Performance**: Eliminates redundant processing and re-renders caused by unchanged balance updates - **Cleaner State Management**: Avoids intermediate states where balances are reset to zero before being set to the same value - **Batch Optimization**: Only sends actual changes to the AccountTracker, reducing the payload size and processing overhead ### Technical Details The implementation uses `isEqual` comparison for the main state update check and direct value comparison (`currentBalance !== newBalance`) for individual balance updates. This ensures that both the overall state update and individual balance updates are optimized to only occur when necessary. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Avoids unnecessary token/native/staked balance updates by comparing existing values and only updating when changed, reducing state mutations and calls. > > - **Fixed: Balance update churn** > - `TokenBalancesController` > - Initialize missing entries without overwriting existing balances; only set balances when values change. > - Filter `updateNativeBalances`/`updateStakedBalances` calls by diffing against `AccountTrackerController:getState` and only send changed entries. > - Add `AccountTrackerController:getState` to allowed actions. > - `AccountTrackerController` > - Optimize `updateNativeBalances` and `updateStakedBalances` to clone state, compare current values, and call `update` only if changes exist. > - **Tests**: Permit and stub `AccountTrackerController:getState`; update messenger setups accordingly. > - **Changelog**: Note fix for unnecessary balance updates to improve performance. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 460183d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 5757b7b commit c154788

File tree

4 files changed

+161
-66
lines changed

4 files changed

+161
-66
lines changed

packages/assets-controllers/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Bump `@metamask/utils` from `^11.8.0` to `^11.8.1` ([#6708](https://github.com/MetaMask/core/pull/6708))
1313

14+
### Fixed
15+
16+
- Fix unnecessary balance updates in `TokenBalancesController` by skipping updates when values haven't changed ([#6743](https://github.com/MetaMask/core/pull/6743))
17+
- Prevents unnecessary state mutations for token balances when values are identical
18+
- Improves performance by reducing redundant processing and re-renders
19+
1420
## [77.0.0]
1521

1622
### Changed

packages/assets-controllers/src/AccountTrackerController.ts

Lines changed: 72 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -702,26 +702,46 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
702702
updateNativeBalances(
703703
balances: { address: string; chainId: Hex; balance: Hex }[],
704704
) {
705-
this.update((state) => {
706-
balances.forEach(({ address, chainId, balance }) => {
707-
const checksumAddress = toChecksumHexAddress(address);
705+
const nextAccountsByChainId = cloneDeep(this.state.accountsByChainId);
706+
let hasChanges = false;
708707

709-
// Ensure the chainId exists in the state
710-
if (!state.accountsByChainId[chainId]) {
711-
state.accountsByChainId[chainId] = {};
712-
}
708+
balances.forEach(({ address, chainId, balance }) => {
709+
const checksumAddress = toChecksumHexAddress(address);
713710

714-
// Ensure the address exists for this chain
715-
if (!state.accountsByChainId[chainId][checksumAddress]) {
716-
state.accountsByChainId[chainId][checksumAddress] = {
717-
balance: '0x0',
718-
};
719-
}
711+
// Ensure the chainId exists in the state
712+
if (!nextAccountsByChainId[chainId]) {
713+
nextAccountsByChainId[chainId] = {};
714+
hasChanges = true;
715+
}
720716

721-
// Update the balance
722-
state.accountsByChainId[chainId][checksumAddress].balance = balance;
723-
});
717+
// Check if the address exists for this chain
718+
const accountExists = Boolean(
719+
nextAccountsByChainId[chainId][checksumAddress],
720+
);
721+
722+
// Ensure the address exists for this chain
723+
if (!accountExists) {
724+
nextAccountsByChainId[chainId][checksumAddress] = {
725+
balance: '0x0',
726+
};
727+
hasChanges = true;
728+
}
729+
730+
// Only update the balance if it has changed, or if this is a new account
731+
const currentBalance =
732+
nextAccountsByChainId[chainId][checksumAddress].balance;
733+
if (!accountExists || currentBalance !== balance) {
734+
nextAccountsByChainId[chainId][checksumAddress].balance = balance;
735+
hasChanges = true;
736+
}
724737
});
738+
739+
// Only call update if there are actual changes
740+
if (hasChanges) {
741+
this.update((state) => {
742+
state.accountsByChainId = nextAccountsByChainId;
743+
});
744+
}
725745
}
726746

727747
/**
@@ -738,27 +758,47 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac
738758
stakedBalance: StakedBalance;
739759
}[],
740760
) {
741-
this.update((state) => {
742-
stakedBalances.forEach(({ address, chainId, stakedBalance }) => {
743-
const checksumAddress = toChecksumHexAddress(address);
761+
const nextAccountsByChainId = cloneDeep(this.state.accountsByChainId);
762+
let hasChanges = false;
744763

745-
// Ensure the chainId exists in the state
746-
if (!state.accountsByChainId[chainId]) {
747-
state.accountsByChainId[chainId] = {};
748-
}
764+
stakedBalances.forEach(({ address, chainId, stakedBalance }) => {
765+
const checksumAddress = toChecksumHexAddress(address);
749766

750-
// Ensure the address exists for this chain
751-
if (!state.accountsByChainId[chainId][checksumAddress]) {
752-
state.accountsByChainId[chainId][checksumAddress] = {
753-
balance: '0x0',
754-
};
755-
}
767+
// Ensure the chainId exists in the state
768+
if (!nextAccountsByChainId[chainId]) {
769+
nextAccountsByChainId[chainId] = {};
770+
hasChanges = true;
771+
}
772+
773+
// Check if the address exists for this chain
774+
const accountExists = Boolean(
775+
nextAccountsByChainId[chainId][checksumAddress],
776+
);
756777

757-
// Update the staked balance
758-
state.accountsByChainId[chainId][checksumAddress].stakedBalance =
778+
// Ensure the address exists for this chain
779+
if (!accountExists) {
780+
nextAccountsByChainId[chainId][checksumAddress] = {
781+
balance: '0x0',
782+
};
783+
hasChanges = true;
784+
}
785+
786+
// Only update the staked balance if it has changed, or if this is a new account
787+
const currentStakedBalance =
788+
nextAccountsByChainId[chainId][checksumAddress].stakedBalance;
789+
if (!accountExists || !isEqual(currentStakedBalance, stakedBalance)) {
790+
nextAccountsByChainId[chainId][checksumAddress].stakedBalance =
759791
stakedBalance;
760-
});
792+
hasChanges = true;
793+
}
761794
});
795+
796+
// Only call update if there are actual changes
797+
if (hasChanges) {
798+
this.update((state) => {
799+
state.accountsByChainId = nextAccountsByChainId;
800+
});
801+
}
762802
}
763803

764804
#registerMessageHandlers() {

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ const setupController = ({
6262
'TokensController:getState',
6363
'AccountsController:getSelectedAccount',
6464
'AccountsController:listAccounts',
65+
'AccountTrackerController:getState',
6566
'AccountTrackerController:updateNativeBalances',
6667
'AccountTrackerController:updateStakedBalances',
6768
],
@@ -111,6 +112,13 @@ const setupController = ({
111112
jest.fn().mockImplementation(() => tokens),
112113
);
113114

115+
messenger.registerActionHandler(
116+
'AccountTrackerController:getState',
117+
jest.fn().mockImplementation(() => ({
118+
accountsByChainId: {},
119+
})),
120+
);
121+
114122
messenger.registerActionHandler(
115123
'AccountTrackerController:updateNativeBalances',
116124
jest.fn(),

packages/assets-controllers/src/TokenBalancesController.ts

Lines changed: 75 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { produce } from 'immer';
3232
import { isEqual } from 'lodash';
3333

3434
import type {
35+
AccountTrackerControllerGetStateAction,
3536
AccountTrackerUpdateNativeBalancesAction,
3637
AccountTrackerUpdateStakedBalancesAction,
3738
} from './AccountTrackerController';
@@ -112,6 +113,7 @@ export type AllowedActions =
112113
| PreferencesControllerGetStateAction
113114
| AccountsControllerGetSelectedAccountAction
114115
| AccountsControllerListAccountsAction
116+
| AccountTrackerControllerGetStateAction
115117
| AccountTrackerUpdateNativeBalancesAction
116118
| AccountTrackerUpdateStakedBalancesAction;
117119

@@ -591,43 +593,55 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
591593

592594
const prev = this.state;
593595
const next = draft(prev, (d) => {
594-
// First, initialize all tokens from allTokens state with balance 0
595-
// for the accounts and chains we're processing
596+
// Initialize account and chain structures if they don't exist, but preserve existing balances
596597
for (const chainId of targetChains) {
597598
for (const account of accountsToProcess) {
598-
// Initialize tokens from allTokens
599+
// Ensure the nested structure exists without overwriting existing balances
600+
d.tokenBalances[account] ??= {};
601+
d.tokenBalances[account][chainId] ??= {};
602+
// Initialize tokens from allTokens only if they don't exist yet
599603
const chainTokens = this.#allTokens[chainId];
600604
if (chainTokens?.[account]) {
601605
Object.values(chainTokens[account]).forEach(
602606
(token: { address: string }) => {
603607
const tokenAddress = checksum(token.address);
604-
((d.tokenBalances[account] ??= {})[chainId] ??= {})[
605-
tokenAddress
606-
] = '0x0';
608+
// Only initialize if the token balance doesn't exist yet
609+
if (!(tokenAddress in d.tokenBalances[account][chainId])) {
610+
d.tokenBalances[account][chainId][tokenAddress] = '0x0';
611+
}
607612
},
608613
);
609614
}
610615

611-
// Initialize tokens from allDetectedTokens
616+
// Initialize tokens from allDetectedTokens only if they don't exist yet
612617
const detectedChainTokens = this.#detectedTokens[chainId];
613618
if (detectedChainTokens?.[account]) {
614619
Object.values(detectedChainTokens[account]).forEach(
615620
(token: { address: string }) => {
616621
const tokenAddress = checksum(token.address);
617-
((d.tokenBalances[account] ??= {})[chainId] ??= {})[
618-
tokenAddress
619-
] = '0x0';
622+
// Only initialize if the token balance doesn't exist yet
623+
if (!(tokenAddress in d.tokenBalances[account][chainId])) {
624+
d.tokenBalances[account][chainId][tokenAddress] = '0x0';
625+
}
620626
},
621627
);
622628
}
623629
}
624630
}
625631

626-
// Then update with actual fetched balances where available
632+
// Update with actual fetched balances only if the value has changed
627633
aggregated.forEach(({ success, value, account, token, chainId }) => {
628634
if (success && value !== undefined) {
629-
((d.tokenBalances[account] ??= {})[chainId] ??= {})[checksum(token)] =
630-
toHex(value);
635+
const newBalance = toHex(value);
636+
const tokenAddress = checksum(token);
637+
const currentBalance =
638+
d.tokenBalances[account]?.[chainId]?.[tokenAddress];
639+
640+
// Only update if the balance has actually changed
641+
if (currentBalance !== newBalance) {
642+
((d.tokenBalances[account] ??= {})[chainId] ??= {})[tokenAddress] =
643+
newBalance;
644+
}
631645
}
632646
});
633647
});
@@ -639,18 +653,34 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
639653
(r) => r.success && r.token === ZERO_ADDRESS,
640654
);
641655

642-
// Update native token balances in a single batch operation for better performance
656+
// Get current AccountTracker state to compare existing balances
657+
const accountTrackerState = this.messagingSystem.call(
658+
'AccountTrackerController:getState',
659+
);
660+
661+
// Update native token balances only if they have changed
643662
if (nativeBalances.length > 0) {
644-
const balanceUpdates = nativeBalances.map((balance) => ({
645-
address: balance.account,
646-
chainId: balance.chainId,
647-
balance: balance.value ? BNToHex(balance.value) : '0x0',
648-
}));
649-
650-
this.messagingSystem.call(
651-
'AccountTrackerController:updateNativeBalances',
652-
balanceUpdates,
653-
);
663+
const balanceUpdates = nativeBalances
664+
.map((balance) => ({
665+
address: balance.account,
666+
chainId: balance.chainId,
667+
balance: balance.value ? BNToHex(balance.value) : '0x0',
668+
}))
669+
.filter((update) => {
670+
const currentBalance =
671+
accountTrackerState.accountsByChainId[update.chainId]?.[
672+
checksum(update.address)
673+
]?.balance;
674+
// Only include if the balance has actually changed
675+
return currentBalance !== update.balance;
676+
});
677+
678+
if (balanceUpdates.length > 0) {
679+
this.messagingSystem.call(
680+
'AccountTrackerController:updateNativeBalances',
681+
balanceUpdates,
682+
);
683+
}
654684
}
655685

656686
// Get staking contract addresses for filtering
@@ -668,16 +698,27 @@ export class TokenBalancesController extends StaticIntervalPollingController<{
668698
});
669699

670700
if (stakedBalances.length > 0) {
671-
const stakedBalanceUpdates = stakedBalances.map((balance) => ({
672-
address: balance.account,
673-
chainId: balance.chainId,
674-
stakedBalance: balance.value ? toHex(balance.value) : '0x0',
675-
}));
676-
677-
this.messagingSystem.call(
678-
'AccountTrackerController:updateStakedBalances',
679-
stakedBalanceUpdates,
680-
);
701+
const stakedBalanceUpdates = stakedBalances
702+
.map((balance) => ({
703+
address: balance.account,
704+
chainId: balance.chainId,
705+
stakedBalance: balance.value ? toHex(balance.value) : '0x0',
706+
}))
707+
.filter((update) => {
708+
const currentStakedBalance =
709+
accountTrackerState.accountsByChainId[update.chainId]?.[
710+
checksum(update.address)
711+
]?.stakedBalance;
712+
// Only include if the staked balance has actually changed
713+
return currentStakedBalance !== update.stakedBalance;
714+
});
715+
716+
if (stakedBalanceUpdates.length > 0) {
717+
this.messagingSystem.call(
718+
'AccountTrackerController:updateStakedBalances',
719+
stakedBalanceUpdates,
720+
);
721+
}
681722
}
682723
}
683724
}

0 commit comments

Comments
 (0)