Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/assets-controllers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Fixed `MultichainBalancesController` not retrying balance fetches after unlock, leaving token lists empty; now retries after unlock. ([#8579](https://github.com/MetaMask/core/pull/8579))

### Added

- Expose missing public `CurrencyRateController` methods through its messenger ([#8561](https://github.com/MetaMask/core/pull/8561))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
'AccountsController:accountRemoved',
'AccountsController:accountBalancesUpdated',
'MultichainAssetsController:accountAssetListUpdated',
'KeyringController:stateChange',

Check failure on line 161 in packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts

View workflow job for this annotation

GitHub Actions / Lint, build, and test / Lint (lint:eslint)

Delegating ':stateChange' events is deprecated. Use ':stateChanged' instead
],
});
return multichainBalancesControllerMessenger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
CaipAssetType,
AccountBalancesUpdatedEventPayload,
} from '@metamask/keyring-api';
import type { KeyringControllerGetStateAction } from '@metamask/keyring-controller';
import type {
KeyringControllerGetStateAction,
KeyringControllerStateChangeEvent,
} from '@metamask/keyring-controller';
import type { InternalAccount } from '@metamask/keyring-internal-api';
import { KeyringClient } from '@metamask/keyring-snap-client';
import type { Messenger } from '@metamask/messenger';
Expand Down Expand Up @@ -105,7 +108,8 @@
| AccountsControllerAccountAddedEvent
| AccountsControllerAccountRemovedEvent
| AccountsControllerAccountBalancesUpdatesEvent
| MultichainAssetsControllerAccountAssetListUpdatedEvent;
| MultichainAssetsControllerAccountAssetListUpdatedEvent
| KeyringControllerStateChangeEvent;
/**
* Messenger type for the MultichainBalancesController.
*/
Expand Down Expand Up @@ -190,6 +194,31 @@
await this.#handleOnAccountAssetListUpdated(updatedAccountAssets);
},
);

// When the keyring transitions from locked → unlocked, fetch balances for
// any non-EVM account that had its balance fetch skipped while locked.
let previouslyUnlocked = this.messenger.call(

Check failure on line 200 in packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts

View workflow job for this annotation

GitHub Actions / Lint, build, and test / Lint (lint:eslint)

Do not call messenger actions in the constructor, as this forces clients to instantiate controllers or services in a specific order. Move this call to an init() method instead. Read the controller guidelines for more: https://github.com/MetaMask/core/blob/main/docs/code-guidelines/controller-guidelines.md#do-not-call-messenger-actions-in-the-constructor
'KeyringController:getState',
).isUnlocked;
this.messenger.subscribe(
'KeyringController:stateChange',

Check failure on line 204 in packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts

View workflow job for this annotation

GitHub Actions / Lint, build, and test / Lint (lint:eslint)

Subscribing to ':stateChange' events is deprecated. Use ':stateChanged' instead
({ isUnlocked }) => {
const justUnlocked = isUnlocked && !previouslyUnlocked;
previouslyUnlocked = isUnlocked;
if (!justUnlocked) {
return;
}
for (const account of this.#listAccounts()) {
const hasBalance =
this.state.balances[account.id] &&
Object.keys(this.state.balances[account.id]).length > 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlock handler skips accounts with existing partial balances

Medium Severity

The hasBalance check only triggers updateBalance for accounts with a completely empty balance map. Since balances state is persisted (persist: true), an account can have existing balance entries from a prior session while new assets get added via accountAssetListUpdated while locked. On unlock, the account already has entries so hasBalance is true, and the newly-added assets never get their balances fetched — the exact scenario this PR aims to fix, but only partially addressed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9b21bf2. Configure here.

if (!hasBalance) {
// eslint-disable-next-line no-void
void this.updateBalance(account.id);
}
}
},
);
}

/**
Expand Down Expand Up @@ -392,7 +421,7 @@
this.update((state: Draft<MultichainBalancesControllerState>) => {
Object.entries(balanceUpdate.balances).forEach(
([accountId, assetBalances]) => {
if (accountId in state.balances) {

Check failure on line 424 in packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts

View workflow job for this annotation

GitHub Actions / Lint, build, and test / Lint (lint:eslint)

The "in" operator is not allowed
Object.assign(state.balances[accountId], assetBalances);
}
},
Expand All @@ -406,7 +435,7 @@
* @param accountId - The account ID being removed.
*/
async #handleOnAccountRemoved(accountId: string): Promise<void> {
if (accountId in this.state.balances) {

Check failure on line 438 in packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts

View workflow job for this annotation

GitHub Actions / Lint, build, and test / Lint (lint:eslint)

The "in" operator is not allowed
this.update((state: Draft<MultichainBalancesControllerState>) => {
delete state.balances[accountId];
});
Expand Down
Loading