feat(assets-controller): use AccountsAPI v6 balances#9344
Conversation
3d14604 to
766168b
Compare
c908168 to
dd41aed
Compare
dd41aed to
3804144
Compare
| options.tokenDetectionEnabled ?? ((): boolean => true); | ||
| this.#messenger = options.messenger; | ||
| this.#useBalanceV6 = | ||
| options.useBalanceV6 ?? ((): boolean => this.#isBalanceV6Enabled()); |
There was a problem hiding this comment.
Why have both a parameter (that is not being used) and a default? It's adding code surface that makes it more difficult to find the source and needs to be tested.
If we pass it down as a method there are certain advantages:
- It can be tested in isolation
- It stops requiring passing down the messenger.
| amount: item.balance, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Not blocking for this PR, but I'm noticing that most of the tests for the logic-heavy processV6Balances and processV5Balances are just happy paths with a single account and a single balance.
Maybe it would make sense in a follow-up ticket to move this logic outside and test it against some responses with more edge cases (for v6 only, not point doing it for v5 if we are going to drop it soon). Ideally, we would want to take the opportunity to see if there is a suitable parser in core that we can use to generate a schema for the api response and validate it beforehand to protect ourselves from runtime errors (this could even be done at core-backend level).
That would leave this class just responsible of dealing with a well-known and safe piece of data.
But again, this is more of an afterthought to reduce the cognitive load when trying to work with this data source middleware.
…assignment' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
3804144 to
bb22b96
Compare
Explanation
The
AccountsApiDataSourcecurrently fetches account balances from the Accounts API v5 multi-account endpoint. This PR adds support for the v6 endpoint and lets it run at parity with v5, gated behind theassetsAccountsApiV6remote feature flag (default off → v5).What changed:
assetsAccountsApiV6remote flag istrue, balances are fetched viafetchV6MultiAccountBalances; otherwise the legacy v5 endpoint is used. The flag is read fromRemoteFeatureFlagControlleron every fetch (not cached), so v6 can be rolled out — and instantly rolled back to v5 — remotely without a client release.category: 'token'rows from the v6 response are consumed (DeFi positions are ignored), so token-balance behavior is identical to v5. Unprocessed networks and fetch timeouts still surface as per-chain errors for RPC fallback, exactly as before.includeAssetIds, scoped to chains in the current request. Native (slip44) and non-EVM/off-chain custom assets are filtered out.AccountsApiDataSourcenow takes the sharedAssetsControllermessenger and callsRemoteFeatureFlagController:getState;AssetsControllerpasses the messenger through and extendsAllowedActionsaccordingly. Adds@metamask/remote-feature-flag-controlleras a dependency.useBalanceV6getter onAccountsApiDataSourceConfigoverrides the remote-flag-driven selection (mainly for tests); when omitted, the remote flag is used.Because the default is v5 and v6 produces the same token balances, there is no user-facing behavior change until the flag is enabled.
References
RemoteFeatureFlagController:getStateinto the Assets controller messenger and bumps@metamask/core-backend/@metamask/assets-controller.Checklist
Note
Medium Risk
Changes which backend balances API runs and how responses are normalized; incorrect flag wiring or v6 parsing could affect displayed token balances, though rollout is flag-gated and defaults to v5.
Overview
Accounts API balance fetches can use the v6 multi-account endpoint when the remote flag
assetsAccountsApiV6is true; otherwise behavior stays on v5. The choice is evaluated on each fetch (not cached) so clients can roll out or roll back without redeploying, with an optionaluseBalanceV6getter onAccountsApiDataSourceConfigto override the flag (mainly for tests).AccountsApiDataSourcenow requires the sharedAssetsControllermessenger and callsRemoteFeatureFlagController:getState;AssetsControllerpasses that messenger through and extends its allowed actions accordingly. The package adds@metamask/remote-feature-flag-controlleras a dependency.On the v6 path, balances are loaded via
fetchV6MultiAccountBalances, mapped from the per-account response shape, and onlycategory: 'token'rows are applied (DeFi positions are skipped to match v5). Custom assets are forwarded asincludeAssetIdsfor ERC-20 tokens on chains in the current request (native and off-chain assets filtered out). Unprocessed networks and timeouts still surface as chain errors for RPC fallback, same as v5.Reviewed by Cursor Bugbot for commit 766168b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Medium Risk
Switches which backend API supplies token balances and how v6 payloads are mapped; wrong flag wiring or parsing could affect displayed balances, though the flag defaults off and v6 is intended to match v5 token behavior.
Overview
AccountsApiDataSourcecan fetch balances from the Accounts API v6 multi-account endpoint when remote flagassetsAccountsApiV6is enabled ({ value: true }fromRemoteFeatureFlagController); otherwise it keeps using v5. The flag is read on each fetch (not cached) so rollout and rollback do not require a client release.The data source now requires the shared
AssetsControllermessenger and delegatesRemoteFeatureFlagController:getState;AssetsControllerpasses the messenger through and allows that action. The package adds@metamask/remote-feature-flag-controller(README dependency graph updated).On the v6 path, responses are normalized from the per-account shape via
#processV6Balances, applying onlycategory: 'token'rows (DeFi skipped for v5 parity). Unprocessed networks and fetch failures still mark chains as errors for RPC fallback. v6 calls use the same account-ID arguments as v5 in this diff (tests assert custom assets are not forwarded asincludeAssetIdson v6).Reviewed by Cursor Bugbot for commit bb22b96. Bugbot is set up for automated code reviews on this repo. Configure here.