feat: assets controllers package upgrade with fix #36808

bergarces · 2025-10-13T10:51:25Z

Description

Changelog

CHANGELOG entry:

Related issues

Fixes:

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

github-actions · 2025-10-13T10:51:39Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2025-10-13T10:53:50Z

✨ Files requiring CODEOWNER review ✨

✅ @MetaMask/confirmations (2 files, +0 -8)

📁 ui/
- 📁 pages/
  - 📁 confirmations/
    - 📁 confirmation/
      - 📁 stories/
        
        📄 util.js +0 -1
      - 📁 templates/
        
        📄 create-named-snap-account.test.js +0 -7

🧪 @MetaMask/qa (1 files, +0 -14)

📁 test/
- 📁 e2e/
  - 📄 fixture-builder.js +0 -14

bergarces · 2025-10-17T11:38:10Z

app/scripts/controllers/preferences-controller.ts

    useExternalServices: true,
+    // from core PreferencesController
+    isMultiAccountBalancesEnabled: true,
    useMultiAccountBalanceChecker: true,


Putting them together as it'll be easier to remove one when we eventually migrate to the core PreferencesController. For now, and considering this PR is already big enough, we are keeping both.

bergarces · 2025-10-17T11:51:17Z

app/scripts/controller-init/messengers/keyring-controller-messenger.ts

 */
 export function getKeyringControllerMessenger(
-  messenger: Messenger<AllowedActions, AllowedEvents>,
+  messenger: Messenger<never, never>,


It was using the wrong types from a different controller. KeyringController does not require any AllowedActions or AllowedEvents

bergarces · 2025-10-17T11:56:45Z

app/scripts/controller-init/account-tracker-controller-init.ts

+        networkClientId,
+      );
+    },
+    includeStakedAssets: false,


Not part of this PR to include staked assets in the assets list. When we want to enable them, we can do so.

bergarces · 2025-10-17T11:57:39Z

app/scripts/controllers/preferences-controller.test.ts

I just moved the properties together so it's easier to refactor them when the time comes.

bergarces · 2025-10-17T11:58:16Z

app/scripts/controllers/preferences-controller.ts

  setUseMultiAccountBalanceChecker(val: boolean): void {
    this.update((state) => {
      state.useMultiAccountBalanceChecker = val;
+      state.isMultiAccountBalancesEnabled = val;


Making sure isMultiAccountBalancesEnabled gets updated as well. Before this, it was always kept as true and not being used at all.

bergarces · 2025-10-17T12:39:08Z

app/scripts/metamask-controller.js


-      // clear accounts in AccountTrackerController
-      this.accountTrackerController.clearAccounts();
-


This is not being done in mobile.

bergarces · 2025-10-17T12:40:57Z

app/scripts/metamask-controller.js

+    // this.accountTrackerController.updateAccountByAddress({
+    //   address: from,
+    //   networkClientId,
+    // });


There is no equivalent method in this version of the controller to update a specific account for a specific network.

refresh does not work with a specific account, but with the selected account, which is not good here as the transaction could be for any account in the wallet.

syncBalanceWithAddresses only returns the results, but it does not update the state.

We can either do:

Full refresh of all accounts for that network (unnecessarily expensive)

No refresh at all and wait for the balances to update on their own

Make changes to AccountTrackerController to support this feature

bergarces · 2025-10-17T12:45:54Z

app/scripts/metamask-controller.js


-    this.accountTrackerController.syncWithAddresses(addresses);
+    // TODO AccountTrackerController Migration: Review how this works before merging
+    // this.accountTrackerController.syncWithAddresses(addresses);


There is no equivalent method for this currently.

Why do we want KeyringController state changes to update every single address balance? Mobile does not seem to be doing it.

bergarces · 2025-10-17T12:49:00Z

app/scripts/metamask-controller.js

-
-      // Updating accounts in this.accountTrackerController before starting UI syncing ensure that
-      // state has account balance before it is synced with UI
-      await this.accountTrackerController.updateAccountsAllActiveNetworks();


This is a very odd thing to do. The UI should be able to handle the default state of AccountTrackerController. Current testing shows that it does.

Do we want to do a full refresh and wait for the result before entering the UI?

At best, we should manage that within the UI, not force it here.

bergarces · 2025-10-17T12:58:22Z

app/scripts/metamask-controller.js

-            oldAccounts.concat(accounts.map((a) => a.address.toLowerCase())),
-          ),
-        ];
-        this.accountTrackerController.syncWithAddresses(accountsToTrack);


This uses the old addresses state and adds the accounts to the state for balance tracking after they appear in the page and before they've been confirmed.

We do not have an equivalent feature in the new AccountTrackerController.

bergarces · 2025-10-17T13:00:10Z

ui/ducks/metamask/metamask.js


-export function getBlockGasLimit(state) {
-  return state.metamask.currentBlockGasLimit;
-}


Not being used or referenced anywhere.

migrate controller

102687b

metamaskbot added the team-assets label Oct 13, 2025

github-actions bot added the size-XL label Oct 13, 2025

bergarces added 13 commits October 13, 2025 12:35

more adjustments

c130949

progress

d7a8983

Merge branch 'main' into account-tracker-controller-migration

35700fc

remove logs

f7624cf

remnove logs

bf14880

remove logs

025f486

rename file

49f553d

Add missing state

f631510

comments

791b24c

revert

2012727

easy fix for preferences controller

c4a696f

test file

8d2e86c

Merge branch 'main' into account-tracker-controller-migration

eae81ee

bergarces commented Oct 17, 2025

View reviewed changes

bergarces added 4 commits October 17, 2025 12:39

better comment

53290e8

remove keyringcontroller wrong allowed actions and events

a18994c

linting

7e745db

revert changes

970d100

bergarces commented Oct 17, 2025

View reviewed changes

do not include staked assets

5302bed

bergarces commented Oct 17, 2025

View reviewed changes

bergarces added 2 commits October 17, 2025 13:19

remove comment

22cb1c7

revert

4d6060c

bergarces added 2 commits October 17, 2025 13:22

fix

8013943

more comments removed

cdd10c6

bergarces commented Oct 17, 2025

View reviewed changes

more comments

1749a1e

bergarces commented Oct 17, 2025

View reviewed changes

remove hw balance sync

7cea266

bergarces commented Oct 17, 2025

View reviewed changes

remove comment

c3bca42

bergarces commented Oct 17, 2025

View reviewed changes


		// clear accounts in AccountTrackerController
		this.accountTrackerController.clearAccounts();

feat: assets controllers package upgrade with fix #36808

Are you sure you want to change the base?

feat: assets controllers package upgrade with fix #36808

Conversation

bergarces commented Oct 13, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Oct 13, 2025

Uh oh!

metamaskbot commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Files requiring CODEOWNER review ✨

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergarces Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergarces Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergarces Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bergarces commented Oct 13, 2025 •

edited by github-actions bot

Loading

metamaskbot commented Oct 13, 2025 •

edited

Loading

bergarces Oct 17, 2025 •

edited

Loading

bergarces Oct 17, 2025 •

edited

Loading

bergarces Oct 17, 2025 •

edited

Loading