fix: RpcBalanceFetcher reports a failed native balance read as success:true, value:0 (zeroes real balances)

[gomesalexandre]

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:

// 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.