Skip to content

Fix: assets controller startup empty accounts#8412

Merged
salimtb merged 4 commits intomainfrom
fix/assets-controller-startup-empty-accounts
Apr 10, 2026
Merged

Fix: assets controller startup empty accounts#8412
salimtb merged 4 commits intomainfrom
fix/assets-controller-startup-empty-accounts

Conversation

@salimtb
Copy link
Copy Markdown
Contributor

@salimtb salimtb commented Apr 9, 2026

Explanation

Current state: On returning users (those with persisted accounts), AssetsController.#start() was called at keyring unlock time before AccountTreeController.init() had completed building the account tree. At that point, #selectedAccounts returned an empty array, so no subscriptions were set up and no asset fetch was triggered. The controller would then sit idle because:

  • AccountTreeController:selectedAccountGroupChange only fires when the selected group changes — on returning users the persisted group is unchanged, so this event is skipped.
  • AccountTreeController:accountTreeChange is not published by init() — it only fires when accounts are added/removed after initialization.

This meant asset balances, metadata, and prices were never loaded until the user manually switched accounts or networks.

Solution: Two changes working together:

  1. Subscribe to AccountTreeController:stateChange — the base-controller event that is guaranteed to fire when init() calls this.update() to build the tree. When this fires, #updateActive() is called, which calls #start() now that accounts are available.

  2. Make #start() idempotent — it now returns early if accounts/chains are not yet available (so the initial unlock call is a safe no-op), and also returns early if subscriptions are already active (so multiple subsequent stateChange firings — e.g. from snap accounts being added — don't trigger redundant fetches).

This follows the same pattern used by DeFiPositionsController, which also gets accounts lazily from AccountTreeController rather than eagerly at unlock time.

References

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
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Touches AssetsController startup/lifecycle logic and subscription gating, which could change when/ how often asset fetches run if the new idempotency checks miss an edge case.

Overview
Fixes a startup race where AssetsController could remain idle for returning users by additionally reacting to AccountTreeController:stateChange and re-evaluating active tracking once the account tree is initialized.

Makes #start() idempotent by returning early when no accounts/chains are available and when subscriptions are already active, preventing duplicate subscriptions/fetches from repeated events; adds a regression test for the unlock-then-tree-init flow and documents the fix in the assets-controller changelog.

Reviewed by Cursor Bugbot for commit d5ad3c3. Bugbot is set up for automated code reviews on this repo. Configure here.

@salimtb salimtb changed the title Fix/assets controller startup empty accounts Fix: assets controller startup empty accounts Apr 9, 2026
@salimtb salimtb force-pushed the fix/assets-controller-startup-empty-accounts branch from 45dcaf9 to bba7020 Compare April 9, 2026 10:19
@salimtb salimtb marked this pull request as ready for review April 9, 2026 10:29
@salimtb salimtb requested review from a team as code owners April 9, 2026 10:29
@salimtb salimtb added this pull request to the merge queue Apr 10, 2026
// when init() calls this.update(). #start() is idempotent so
// repeated fires are safe.
this.messenger.subscribe('AccountTreeController:stateChange', () => {
this.#updateActive();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use the state change selector (3rd param of the subscribe func)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a blocker, we'll receive a new event by accounts team which should remove the need for this state change event

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes it's just a temp solution for now

@salimtb salimtb removed this pull request from the merge queue due to a manual request Apr 10, 2026
@salimtb salimtb added this pull request to the merge queue Apr 10, 2026
Merged via the queue into main with commit 1afc2d2 Apr 10, 2026
341 checks passed
@salimtb salimtb deleted the fix/assets-controller-startup-empty-accounts branch April 10, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants