feat(account-tree-controller): persist accountTree#8437
Merged
Conversation
Contributor
|
@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. |
montelaidev
reviewed
Apr 14, 2026
| for (const accountId of group.accounts) { | ||
| // The `AccountsController` also persists its accounts, so we | ||
| // can fetch them too! | ||
| const account = this.messenger.call( |
Contributor
There was a problem hiding this comment.
We can reduce this to one call by getting all the accounts
Contributor
Author
There was a problem hiding this comment.
I initially wanted to use :getAccounts, but didn't want to introduce a breaking change for this
However, I see we still have the old listMultichainAccounts, so we can probably use that + make a Map from it for fast indexing!
Good idea, let me change this!
Contributor
There was a problem hiding this comment.
@montelaidev beat me to it. I would prefer the one getAccounts call as well. I don't think it makes sense to rebuild essentially what is the same map to avoid a breaking change
Contributor
There was a problem hiding this comment.
something like this:
const accounts = this.messenger.call(
'AccountsController:getAccounts',
group.accounts,
);
group.accounts.forEach((accountId, idx) => {
const account = accounts[idx];
let sortOrder = MAX_SORT_ORDER;
if (account) {
sortOrder = ACCOUNT_TYPE_TO_SORT_ORDER[account.type];
}
this.#accountIdToContext.set(accountId, {
walletId,
groupId,
sortOrder,
});
});
Contributor
Author
There was a problem hiding this comment.
I went for :listMultichainAccounts since adding :getAccounts would involve a breaking change for this controller and I'd like to keep it non-breaking for now to avoid the bubbling effect with this change 😅
Though, with :listMultichainAccounts + a Map we can get something pretty similar (I went for that).
Contributor
Author
There was a problem hiding this comment.
We can always change that later, but my goal for now is to unblock the assets team, we can introduce :getAccounts later for sure 👍
montelaidev
approved these changes
Apr 14, 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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Explanation
We used to not persist the account-tree before and we were re-constructing it "fresh" upon
init. However, some other controllers are dependent on the tree and have to wait for it to be built before consuming it.Since the tree should never really change between lock/unlock, that's ok to have a on-disk copy of it to speedup consumers.
We will still try to re-construct it during
initthough (in case rules have changed or that new accounts have appeared that went unnoticed).References
N/A
Checklist
Note
Medium Risk
Persists and rehydrates core controller state, which can affect startup/lock-unlock behavior and introduce stale/mismatched tree data if persisted state diverges from current accounts.
Overview
AccountTreeControllernow persistsaccountTree(state metadatapersist: true) instead of always recomputing it oninit.The controller constructor now pre-initializes internal reverse lookup maps from the persisted tree via
#initTreeContext, allowing methods likegetAccountContext,getAccountGroupObject, andgetAccountsFromSelectedAccountGroupto work beforeinit()runs.Tests and snapshots are updated to cover pre-populated persisted state behavior and the new persisted-state surface, and the changelog documents the new persistence behavior.
Reviewed by Cursor Bugbot for commit 65dedea. Bugbot is set up for automated code reviews on this repo. Configure here.