Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/assets-controller/src/data-sources/evm-rpc-services/services/BalanceFetcher.ts
Show resolved
Hide resolved
packages/assets-controller/src/data-sources/evm-rpc-services/services/BalanceFetcher.ts
Outdated
Show resolved
Hide resolved
packages/assets-controller/src/data-sources/evm-rpc-services/services/BalanceFetcher.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The goal here was to pass the assetId alongside the token addresses and the tokenInfo to the balance fetcher.
Since the last two were already being placed in two different arrays of the same length, as well as a setting to return native balance, it's all been simplified by just passing to balance fetcher a single array that contains everything needed:
- AssetId (including the native token)
- Address
- TokenInfo
That way, BalanceFetcher does not need to know how to build the native assetId for every chain.
Also, the token address passed to balance fetcher is always the zero address, regardless of the chain, as that is how BalanceFetcher determines whether to fetch the balance of a token or the balance of the account.
packages/assets-controller/src/data-sources/evm-rpc-services/services/BalanceFetcher.test.ts
Outdated
Show resolved
Hide resolved
| * native assets even when the chain has a native asset with a non-zero address) | ||
| * and optional metadata | ||
| */ | ||
| export type BalanceFetchOptions = { |
There was a problem hiding this comment.
This wasn't being used at all, only to specify that we wanted to include native assets, which was already the default. In any case the native assets need to be added to the array of tokens to fetch now.
| /** Token decimals (omit when unknown — balance fetcher returns raw balance for RpcDataSource to resolve). */ | ||
| decimals?: number; | ||
| /** Token symbol (optional) */ | ||
| symbol?: string; |
There was a problem hiding this comment.
This wasn't being used at all.
| continue; | ||
| } | ||
|
|
||
| const isNative = assetNamespace === 'slip44'; |
There was a problem hiding this comment.
This is not completely right, but this is how we are currently determining that an asset is native everywhere else, so this is consistent.
We already have a ticket and plans to discuss how to handle native tokens (by pushing the definition to an API).
| const tokenMap = new Map<string, TokenFetchInfo>(); | ||
|
|
||
| for (const assetId of Object.keys(accountBalances)) { | ||
| // Only process ERC20 tokens on the current chain |
There was a problem hiding this comment.
We no longer need to filter only erc20 tokens, since we need to include all assets for that chain, including native assets.
This simplifies things as well.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| chain: { reference: chainReference }, | ||
| assetNamespace, | ||
| assetReference, | ||
| } = parseCaipAssetType(assetId); |
There was a problem hiding this comment.
Missing try/catch around parseCaipAssetType in #getAssetsToFetch
Low Severity
The new #getAssetsToFetch calls parseCaipAssetType(assetId) on every key in accountBalances without a try/catch. If any state key is a malformed CAIP-19 string, this throws and crashes the entire polling cycle. The old getTokensToFetch used simple string operations (startsWith, includes) that never throw on bad input. Notably, RpcDataSource.ts already wraps its own parseCaipAssetType calls in try/catch for exactly this reason.
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
1 participant
Explanation
References
Checklist
Note
Medium Risk
Updates the balance-fetching API used by
RpcDataSource/BalanceFetcher, changing how native + custom ERC-20s are identified and batched; regressions could impact displayed balances across chains.Overview
Refactors EVM balance polling/fetching to operate on explicit
AssetFetchEntryobjects (CAIP-19assetId+ on-chainaddress+ optionaldecimals) viafetchBalancesForAssets, replacing the prior token-address + options/tokenInfo approach.RpcDataSource.fetchnow always builds a single native+custom-ERC20 entry list per chain (usingZERO_ADDRESSfor native and pre-populating decimals from existing metadata/token list when available), and removes the helper that previously builtTokenFetchInfofor custom tokens.BalanceFetchernow maps multicall responses back to caller-provided asset IDs, deduplicates by address, and uses a sharedZERO_ADDRESSconstant; tests are updated accordingly andZERO_ADDRESSis centralized inutils/constants.ts(also adopted byMulticallClient).Written by Cursor Bugbot for commit 1189e6b. This will update automatically on new commits. Configure here.