fix: RpcBalanceFetcher reports a failed native balance read as success:true, value:0 (zeroes real balances)
- Dominant language
- TypeScript
- Stars
- 413
- Forks
- 308
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 253
Description
What it says on the box: a failed native-balance read and a genuinely-zero native balance are indistinguishable in `RpcBalanceFetcher`, and the failed case wins, wiping the previously-known balance to `0x0`.
## Where
`packages/assets-controllers/src/rpc-service/rpc-balance-fetcher.ts:164-172` (native), `:204-212` (staked). Compare to the ERC-20 branch 12 lines below in the same function, which does it correctly:
```ts
// native, :164-172
allAddressesForNative.forEach((address) => {
const nativeBalance = tokenBalances[ZERO_ADDRESS]?.[address] || null;
chainResults.push({
success: true, // hard-coded, regardless of whether a read happened
value: nativeBalance || new BN('0'), // absent collapses into zero
...
});
});
...
// erc-20, :182 — same function
success: bn !== null,
```
## Why `success: true` is provably wrong here
An absent entry for `ZERO_ADDRESS` in `tokenBalances` can *only* mean the read failed, never a genuine zero:
- `processBalanceResults` (`multicall.ts:674-687`) writes `balanceMap[ZERO_ADDRESS][userAddress]` only inside `if (result.success)`. A genuine on-chain zero still comes back `success: true` from `aggregate3` (`allowFailure: true`) and gets written as `new BN(0)`. Only a failed subcall leaves the key absent.
- `getNativeBalancesFallback` (`multicall.ts:730-765`) records a result only when `status === 'fulfilled'`; a rejected `eth_getBalance` leaves no key.
- `getStakedBalancesForAddresses` (`:983-985`) catches any throw and returns `{}` — one RPC error zeroes every account on that chain.
So the biconditional holds: absent key ⟺ the underlying read failed. The native branch treats "absent" as "confirmed zero" instead.
## Write path (unconditional on `success`)
`TokenBalancesController.ts:1131` filters on `balance.success`, which is hard-coded `true` for native → `AccountTrackerController.updateNativeBalances` (`:996-1041`) sets `accountsByChainId[chainId][addr].balance = '0x0'` and persists it. `isDeprecated` defaults to `() => false` (`AccountTrackerController.ts:302`), so this lane is live by default, not behind a flag.
## Repro
Ported the four functions above verbatim; simulated a custom network → fallback lane where one address's `eth_getBalance` rejects (429):
```
BEFORE Alice balance: 0x4563918244f40000 = 5 ETH
fallback returned keys: [ '0xbbbb…' ] <- Alice absent (her read rejected, not zero)
NativeBalanceUpdate[] : [{"address":"0xaaaa…","chainId":"0x1f4","balance":"0x0"}]
AFTER Alice balance: 0x0 <- 5 ETH wiped, no error surfaced
```
## Reachability
Not exotic — any of these trigger the fallback/failure lane:
- Any user-added custom network with no `MULTICALL_CONTRACT_BY_CHAINID` entry routes every native read through the flaky per-address fallback.
- Any chain where `aggregate3` throws drops to the same fallback (`multicall.ts:1160-1180`).
- Mainnet/hoodi staked-balance reads: one throw in `getStakedBalancesForAddresses` zeroes every account on the chain in one shot.
## Anticipated objection, addressed in advance
`rpc-balance-fetcher.test.ts:910` pins *"should always include native token entry … even when balance is zero"*, so a zero-fill looks intentional at first read. But that premise doesn't apply to native: a *successful* read of a genuine zero already writes a key via the `aggregate3`/`new BN(0)` path shown above. The unconditional zero-fill has no legitimate case to serve on the native side — it only ever converts a failure into a fabricated zero. Precedent for the correct behavior already exists in the sibling fetcher: `api-balance-fetcher.ts:475-477`, *"without overwriting potentially stale balances with zero values"*.
The staked-balance face is genuinely different — `multicall.ts:940` filters `shares.gt(0)`, so an absent key legitimately does mean zero there — so I'm not proposing a change to that path here.
## Suggested direction
Mirror the ERC-20 branch on the native side: `success: nativeBalance !== null` instead of the hard-coded `true`. Filing as an issue rather than a PR first since the fix touches a path with an existing (arguably now-misleading) pinned test, and I'd rather get a maintainer read before reworking it.
Happy to open the PR once there's agreement on direction.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in packages/assets-controllers/src/rpc-service/rpc-balance-fetcher.ts at the native branch around lines 164-172, then compare its result handling with the ERC-20 branch and rpc-balance-fetcher.test.ts around line 910. Read the related paths in multicall.ts and the write flow through TokenBalancesController.ts and AccountTrackerController.ts. Done means failed native reads remain distinguishable from genuine zero balances and the relevant tests cover both cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100