fix(assets-controller): seed native balances after account refresh#9388
fix(assets-controller): seed native balances after account refresh#9388salimtb wants to merge 3 commits into
Conversation
Zero-balancing for new accounts ran before getAssets on account-tree changes, so assetsBalance was often never populated for the new account. Seed native zero balances using the refreshed account list after the fetch.
| | AccountTreeControllerStateChangedEvent | ||
| | ClientControllerStateChangedEvent | ||
| | AccountTreeControllerStateChangeEvent | ||
| | ControllerStateChangedEvent< |
There was a problem hiding this comment.
Can we use the imported version?
Less type issues then
There was a problem hiding this comment.
Same for the other state change events
|
|
||
| // Switching EVM address within the same account group (e.g. Account 1 → Account 2). | ||
| this.messenger.subscribe( | ||
| 'AccountsController:selectedEvmAccountChange', |
There was a problem hiding this comment.
How come this and not the account tree change?
There was a problem hiding this comment.
accountTree change is not fired , that was the issue , now i need to relay on this event
| }); | ||
| this.#subscribeAssets(); | ||
| if (newAccounts.length > 0) { | ||
| await this.#fetchAccountBalancesViaRpc(newAccounts, [ |
There was a problem hiding this comment.
Thoughts on calling the above getAssets with newAccounts?
There was a problem hiding this comment.
hmmmm because getAssets will fetch also quick pipeline and restart all subscription , here i wanted to scope it only on rpc scope to make it better for perf
| * @param chainIds - Chains to fetch on. | ||
| */ | ||
| async #fetchAccountBalancesViaRpc( | ||
| accounts: InternalAccount[], |
There was a problem hiding this comment.
See prev comment, then we can potentially remove this?
| // Subscribe after fetch so WS notifications can recover state | ||
| this.#subscribeAssets(); | ||
|
|
||
| await this.#fetchAccountBalancesViaRpc(accounts, [ |
There was a problem hiding this comment.
Need to call around this. I'm not a fan of even more complexity
|
|
||
| this.#subscribeAssets(); | ||
|
|
||
| await this.#fetchAccountBalancesViaRpc( |
There was a problem hiding this comment.
Not sure I understand why this is needee
| updateMode: 'merge', | ||
| }); | ||
|
|
||
| await this.#fetchAccountBalancesViaRpc(accounts, addedChains); |
| dataTypes: ['balance', 'metadata', 'price'], | ||
| }); | ||
|
|
||
| await this.#fetchAccountBalancesViaRpc(accounts, [selectedChainId]); |
2 participants
Zero-balancing for new accounts ran before getAssets on account-tree changes, so assetsBalance was often never populated for the new account. Seed native zero balances using the refreshed account list after the fetch.
assetsBalancefor newly added accounts on custom/RPC networks (e.g. DXC /eip155:50) whereassetsInfowas present but balances were not fetchedExplanation
References
Checklist
Note
Medium Risk
Changes several hot paths for balance refresh and adds a new messenger subscription that requires host wiring; impact is localized to assets state merging but affects RPC-only and custom networks where API paths may not populate balances.
Overview
Adds a dedicated
#fetchAccountBalancesViaRpcpath that runs RpcDataSource balance middleware after the normalgetAssetsrefresh and merges results into state, limited to chains RPC currently owns. This is wired into account group changes, new accounts on the account tree, newly enabled networks, EVM network picker switches, and a newAccountsController:selectedEvmAccountChangehandler for switching EVM addresses within the same group.The changelog notes that integrators must delegate
AccountsController:selectedEvmAccountChangeto the AssetsController messenger so in-group EVM switches refresh balances. Messenger allowed event types are aligned with*StateChangeEventnaming from related controllers.Reviewed by Cursor Bugbot for commit 01ed1a1. Bugbot is set up for automated code reviews on this repo. Configure here.