-
-
Notifications
You must be signed in to change notification settings - Fork 252
fix(account-tree-controller)!: automatically removes "outdated" accounts from multichain account groups #7038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@metamaskbot publish-previews |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
8cb8a9e to
60778d9
Compare
|
@metamaskbot publish-previews |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
|
@metamaskbot publish-previews |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
| this.messenger.subscribe( | ||
| 'MultichainAccountService:multichainAccountGroupCreated', | ||
| (group) => { | ||
| this.#handleMultichainAccountWalletGroupCreatedOrUpdated(group); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since one should really be useful, but I added it in to avoid having another breaking change with another PR that should replace this one someday:
|
@metamaskbot publish-previews |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
Explanation
Multichain account groups can "disable" accounts using
AccountProviderWrappers. This is mostly used in combination with remote feature flags or with the "Basic Functionality" flags.Though, since the account tree is not "consuming" multichain account groups directly, the account list from those groups was not properly updated (since not
:account{Added,Removed}are used in this case), resulting in a de-sync between the groups from the tree and the groups from the service.Note
This PR aims to fix this de-sync in a simple way without involving too many architecture changes (so it's less risky to release this first), but the real fix for this is there:
multichain-account-serviceto build multichain account wallet/group nodes #6646References
N/A
Checklist
Note
AccountTreeController now skips and removes “outdated” BIP‑44 accounts using MultichainAccountService actions/events, MultichainAccountService simplifies provider creation/error handling, and repo scripts/configs are cleaned up (drop build:all, adjust ts-bridge, prettier/yarn).
MultichainAccountService:getMultichainAccountWallets.MultichainAccountService:multichainAccountGroup{Created,Updated}to remove accounts no longer in a group.createMultichainAccountGroupto either await all providers or just EVM, running others in background with logging; aggregate errors when awaiting all.utils.build:allacross packages and relax Yarn constraints; reorderbuild:docswhere needed.@ts-bridge/clito^0.6.1; updateyarn.lock.Written by Cursor Bugbot for commit 3c46556. This will update automatically on new commits. Configure here.