Uh oh!
There was an error while loading. Please reload this page.
feat: start tracking on AccountTreeController:initialized - #9892
Merged
Conversation
Subscribe to AccountTreeController:initialized so asset tracking starts only after the tree is fully built, instead of on intermediate stateChange events during init. Hosts must now also delegate the new event through the AssetsController messenger.
salimtb
commented
Aug 17, 2026
ContributorAuthor
@metamaskbot publish-preview |
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e0e5a85. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
| this.#ensureNativeBalancesDefaultZero(); | ||
| this.#ensureDefaultTrackedAssetsSeeded(); | ||
| this.#subscribeAssets(); | ||
| this.#subscribeAssets({ skipInitialFetch: true }); |
There was a problem hiding this comment.
Skip poll after failed fetch
Medium Severity
On startup and tree-refresh failure paths, #subscribeAssets is still called with skipInitialFetch: true even though the forced getAssets did not succeed. Accounts API then skips its subscribe-time poll, so balances stay empty until the next interval instead of recovering immediately.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e0e5a85. Configure here.
salimtb
commented
Aug 17, 2026
ContributorAuthor
@metamaskbot publish-preview |
salimtb
commented
Aug 17, 2026
ContributorAuthor
@metamaskbot publish-preview |
Contributor
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
salimtb
commented
Aug 17, 2026
| | AccountTreeControllerSelectedAccountGroupChangeEvent | ||
| | AccountTreeControllerStateChangeEvent | ||
| | AccountTreeControllerInitializedEvent | ||
| | AccountTreeControllerUninitializedEvent |
ContributorAuthor
There was a problem hiding this comment.
Hosts that filter messenger events must now also allow :initialized / :uninitialized. These are the tree-ready / torn-down signals. Startup used to ride on :stateChange during AccountTreeController.init(), which fired many times and spammed the Accounts API.
| * Whether `AccountTreeController` has finished `init()`. Unlock / UI-open | ||
| * alone must not start fetches — the tree can still be mid-build. | ||
| */ | ||
| #accountTreeInitialized = false; |
ContributorAuthor
There was a problem hiding this comment.
New flag. Unlock and UI-open can happen before AccountTreeController.init() finishes; without it #updateActive() could start fetches on a half-built tree. Tracking now runs only when UI is open and unlocked and the tree is initialized.
| 'AccountTreeController:selectedAccountGroupChange', | ||
| (groupId) => { | ||
| this.#handleAccountGroupChanged(groupId).catch(console.error); | ||
| (groupId, previousGroupId) => { |
ContributorAuthor
There was a problem hiding this comment.
Thread previousGroupId through. ATC re-emits selectedAccountGroupChange on every init() even when the group didn't change, so we need the previous id to tell real user switches from init noise.
| // Start tracking only after the account tree is fully built. Unlock can | ||
| // happen before `AccountTreeController.init()`, and `:stateChange` fires | ||
| // for intermediate mutations during that build. | ||
| this.messenger.subscribe('AccountTreeController:initialized', () => { |
ContributorAuthor
There was a problem hiding this comment.
:initialized flips the flag and calls #updateActive() to start tracking once; :uninitialized clears it and stops tracking (wallet reset / clearState) so we don't keep polling stale accounts.
| #updateActive(): void { | ||
| const shouldRun = this.#uiOpen && this.#keyringUnlocked; | ||
| const shouldRun = | ||
| this.#uiOpen && this.#keyringUnlocked && this.#accountTreeInitialized; |
ContributorAuthor
There was a problem hiding this comment.
Single start/stop gate now also requires #accountTreeInitialized. Unlock alone is no longer enough — this prevents the unlock-before-init Accounts API burst.
| // Subscribe after seed so the price poll sees natives / defaults. | ||
| this.#subscribeAssets(); | ||
| // Balances were just force-fetched — skip AccountsApi's subscribe-time poll. | ||
| this.#subscribeAssets({ skipInitialFetch: true }); |
ContributorAuthor
There was a problem hiding this comment.
Group switch already force-fetched via getAssets, so skip AccountsApi's subscribe-time poll here too.
| * is created. Used after the controller has already force-fetched balances | ||
| * for the same accounts/chains so we do not immediately hit the API again. | ||
| */ | ||
| skipInitialFetch?: boolean; |
ContributorAuthor
There was a problem hiding this comment.
Optional subscribe flag so AssetsController can tell AccountsApi that balances for this scope are already fresh. Non-breaking — sources that don't read it behave as before.
| const { request } = context; | ||
| // Price/metadata-only requests must not hit the Accounts API. | ||
| if (!request.dataTypes.includes('balance')) { |
ContributorAuthor
There was a problem hiding this comment.
Skip the Accounts API balance middleware for price-only refreshes (currency change, missing-price backfill, etc.). Balances aren't needed there, so pass through to the next middleware without the HTTP call.
| // Interval above still polls on the normal cadence. This only skips the | ||
| // one-shot fetch at subscribe time when the controller already ran a | ||
| // force getAssets for the same scope (startup / group refresh). | ||
| if (!skipInitialFetch) { |
ContributorAuthor
There was a problem hiding this comment.
setInterval is still armed , polling is not disabled. We only skip the immediate one-shot fetch when the controller already force-fetched the same accounts/chains; the next tick still runs on pollInterval.
| // AssetsController | ||
| 'AccountTreeController:selectedAccountGroupChange', | ||
| 'AccountTreeController:stateChange', | ||
| 'AccountTreeController:initialized', |
ContributorAuthor
There was a problem hiding this comment.
Mirror the host messenger allowlist so the fixture can publish the new :initialized / :uninitialized lifecycle events.
juanmigdr
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Subscribe to AccountTreeController:initialized so asset tracking starts only after the tree is fully built, instead of on intermediate stateChange events during init. Hosts must now also delegate the new event through the AssetsController messenger.
Extension PR: MetaMask/metamask-extension#45579
Mobile PR: MetaMask/metamask-mobile#34866
Explanation
References
Checklist
Note
Cursor Bugbot is generating a summary for commit 22007ec. Configure here.