Skip to content

test(assets-controller): add stale-balance scenario integration tests for accounts API v6 - #10535

Closed
Prithpal-Sooriya wants to merge 8 commits into
MetaMask:mainfrom
Prithpal-Sooriya:feature/use-memory-see-this-sp0
Closed

Prithpal-Sooriya wants to merge 8 commits into
MetaMask:mainfrom
Prithpal-Sooriya:feature/use-memory-see-this-sp0

Conversation

@Prithpal-Sooriya

@Prithpal-Sooriya Prithpal-Sooriya commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

Adds AssetsController.stale-balance-scenarios.integration.test.ts, an AssetsController-level integration suite covering all 12 stale-balance scenarios from the accounts API v6 integration decision doc (full and merge tables).

The suite boots the real AssetsController (with assetsAccountsApiV6: true) and mocks only the external boundaries, following the pattern of AssetsController.bsc-spam-token-filtering.integration.test.ts:

  • HTTP APIs (Accounts API v6 + v2-supportedNetworks, tokens /v3/assets, prices /v3/spot-prices) are nocked from real captured responses in src/__fixtures__/stale-balance/api-responses/. Every fixture mutation is documented inline (balance doubling to model wallet movement between passes, occurrences stamping, chain omissions for RPC fallback).
  • RPC / snap / lifecycle are exercised through mock messenger action handlers (NetworkController, SnapController, KeyringController, staking reads) and live events (AccountActivityService:balanceUpdated, AccountsController:accountBalancesUpdated, TransactionController:transactionConfirmed, KeyringController:unlock).

Scenario coverage (7 full + 5 merge): snap failure carry-over, skipped-asset last-amount copy, zero-seeding for never-seen visible assets, RPC fallback carry-over on failing balanceOf reads, disappearing zero-balance seeds, failed-fetch carry-over, fresh amounts on healthy fallback, event overlays, malformed-row handling, wrong-amount correction at the next full poll, snap-event overlay + refresh, and staked ETH on mainnet + Hoodi across an unlock lifecycle with a staking deposit.

Test plan

  • yarn workspace @metamask/assets-controller run test
    • New suite: 12/12 scenarios pass
    • Full package suite: 39 suites / 1172 tests pass
  • yarn lint checks on the new files: oxlint and oxfmt clean
  • yarn build (root, full monorepo type-check) passes

Notes for reviewers

  • Fixture balance-row shape includes unit, required by the keyring API's Balance struct via strictMask (@metamask/keyring-api); the controller discards it downstream.
  • Native ETH on Hoodi is keyed under the zero-address ERC-20 encoding (eip155:560048/erc20:0x0000000000000000000000000000000000000000) — the controller's documented fallback for EVM chains absent from its native-asset seed map, which excludes testnets.
  • The Multicall3 aggregate3 fixture encodes the contract's real return shape — Result[] only, with no leading blockNumber (only aggregate/tryAggregate return one). An earlier draft of the fixture encoded (uint256, Result[]), which made every batch decode throw and silently fall through to single balanceOf calls; caught in review and fixed, so the scenarios exercise the batched read path. MulticallClient's decoder is correct as shipped — no product bug here.

Note

Low Risk
Test and fixture additions only; no runtime balance or merge logic changes in production code.

Overview
Adds end-to-end integration coverage for all 12 stale-balance scenarios from the accounts API v6 decision doc (full and merge tables), without changing AssetsController production code.

The new suite boots the real controller with assetsAccountsApiV6: true and replaces external boundaries only: nock interceptors serve captured Accounts/Tokens/Prices API responses (with scenario knobs like unprocessedNetworks, balance overrides, and stamped token occurrences so spam filtering does not dominate), while messenger mocks supply BNB/mainnet/Hoodi RPC (Multicall3 + staking reads), the Solana keyring snap, wallet lifecycle, and live events (AccountActivityService:balanceUpdated, snap balance updates, staking transactionConfirmed).

Twelve tests assert last-known vs fresh behavior when the snap fails or omits assets, BNB Chain falls back to RPC (including partial failures and zero-token drops), activity events merge or are ignored when malformed, and staked ETH on mainnet/Hoodi survives token polls after a vault deposit.

Reviewed by Cursor Bugbot for commit d3c9d4f. Bugbot is set up for automated code reviews on this repo. Configure here.

… for accounts API v6

Cover all 12 stale-balance scenarios documented in the accounts API v6
integration decision doc (7 `full` + 5 `merge`), booting the real
AssetsController with mocked external boundaries: nock for the Accounts /
tokens / prices HTTP APIs, and mock messenger action handlers for RPC,
snap, and lifecycle events. Fixtures build on real API captures, with
every fixture mutation documented inline.
@Prithpal-Sooriya
Prithpal-Sooriya requested a review from a team as a code owner September 28, 2026 14:31
…-balance suite

Follow the shared-utils schema of the other AssetsController integration
suites: `getIgnoringCase` moves to `__fixtures__/test-utils.ts` instead of
a per-suite copy, and the account builders in
`__fixtures__/stale-balance/staleBalanceWallet.ts` delegate to the shared
`createMockInternalAccount` (as `AssetsController.spam-cleanup.integration
test.ts` does) rather than hand-rolling `InternalAccount` boilerplate.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit d3c9d4f. Configure here.

@Prithpal-Sooriya
Prithpal-Sooriya marked this pull request as draft September 28, 2026 20:35
…teSurface assertion utils

- The Multicall3 `aggregate3` fixture no longer prefixes a `blockNumber`
  `uint256` before the results array: the real contract returns `Result[]`
  only (only `aggregate`/`tryAggregate` return a block number), so the
  batched read path now decodes instead of silently falling back to
  single `balanceOf` calls (review finding).
- Amount assertions now go through `StateSurface`-style utils
  (`buildBalancesSurface` + per-account `SOLANA_BALANCES`/`BSC_BALANCES`/
  `MAINNET_BALANCES`), matching the other AssetsController integration
  suites.
…cenario describe

Adopt the exact `StateSurface` pattern of the bsc-spam integration
suite: a bare `type StateSurface` and a `const BALANCES: StateSurface`
whose lookUp binds the account's balances record — copied verbatim into
each scenario describe (the wallet spans three accounts, so each
describe binds its own), replacing the `buildBalancesSurface` factory.
…fix, and verbatim StateSurface utils

The three commits below (d3c9d4f, 48b3abd, 1c100ff) landed
empty — their changes were never staged before the plumbing commit, so
only the SHAs moved. This commit actually lands their changes:

- Reuse the shared integration-test utils: consolidate `getIgnoringCase`
  into `__fixtures__/test-utils.ts` and delegate the account builders in
  `staleBalanceWallet.ts` to the shared `createMockInternalAccount`.
- Fix the Multicall3 `aggregate3` fixture's wire shape: drop the
  `blockNumber` `uint256` prefix — the real contract returns `Result[]`
  only, so the batched read path decodes instead of silently falling
  back to single `balanceOf` calls.
- Copy the bsc-spam suite's `StateSurface` utils verbatim: bare
  `type StateSurface` and a `const BALANCES: StateSurface` per scenario
  describe (this wallet spans three accounts), with all amount
  assertions reading `surface.lookUp`.
Assert with the bsc-spam-token exemplar dialect (direct
BALANCES.lookUp(...).toMatchObject, inline key-set checks) instead of the
invented expectAmount/expectedAmount/lowerKeySet helpers; carry-over states
inline via buildEmptyStaleBalanceState. Drop all banner separators and
rationale comments from the suite and the __fixtures__/stale-balance helpers,
keeping one-to-two-line summaries only; remove the dead
USDT_ASSET_ID_LOWERCASE constant.
…e-this-sp0

# Conflicts:
#	packages/assets-controller/src/__fixtures__/test-utils.ts
Per review: add getAssetIds and lookupAmount to each describe's BALANCES
surface so the repeated lower-cased key-set checks and amount assertions
read expect(BALANCES.getAssetIds(state)).toStrictEqual(...) and
expect(BALANCES.lookupAmount(state, assetId)).toBe(...) across every
scenario. Drop the scenario-3 key-set check (redundant with its balance
assertions) and replace scenario-11's with a direct USDC amount
assertion; S5/S8/S9/S10 key-sets now compare surfaces of captured states.
@Prithpal-Sooriya

Copy link
Copy Markdown
Contributor Author

Note on the failing "Validate changelog diffs" check: this failure is a known CI limitation for fork PRs, not an issue with this PR's changes.

  • The job log shows it failed at ref resolution, before any changelog validation ran: fatal: Not a valid object name origin/feature/use-memory-see-this-sp0 (exit 128).
  • This PR is opened from a fork, so its head branch does not exist under origin/* in the base-repo clone that the job checks out.
  • All other lint/build/test jobs pass on the head commit, including lint:oxlint, lint:misc:check, and the per-package Validate changelog jobs. No CHANGELOG.md files are modified in this diff (the PR carries the no-changelog label), so once the upstream fix for the action lands, a CI re-run will pass this job.

Upstream fix in flight: dbced1f5f2e — "ci: resolve changelog checks against the pull request ref" (branch fix/changelog-checks-fork-prs), which resolves the PR head via refs/pull/<number>/head instead of origin/<branch>.

cryptodev-2s pushed a commit to cryptodev-2s/core that referenced this pull request Sep 30, 2026
… for accounts API v6 (MetaMask#10584)

## Explanation

Adds `AssetsController.stale-balance-scenarios.integration.test.ts`, an
AssetsController-level integration suite covering **all 12 stale-balance
scenarios** from the accounts API v6 integration decision doc
([`full`](https://github.com/MetaMask/core/blob/main/packages/assets-controller/docs/decisions/2026-09-23-accounts-api-v6-integration.md#stale-balance-scenarios-full)
and
[`merge`](https://github.com/MetaMask/core/blob/main/packages/assets-controller/docs/decisions/2026-09-23-accounts-api-v6-integration.md#stale-balance-scenarios-full)
tables).

The suite boots the real `AssetsController` (with `assetsAccountsApiV6:
true`) and mocks only the external boundaries, following the pattern of
`AssetsController.bsc-spam-token-filtering.integration.test.ts`:

- **HTTP APIs** (`Accounts API` v6 + v2-supportedNetworks, tokens
`/v3/assets`, prices `/v3/spot-prices`) are nocked from real captured
responses in `src/__fixtures__/stale-balance/api-responses/`. Every
fixture mutation is documented inline (balance doubling to model wallet
movement between passes, `occurrences` stamping, chain omissions for RPC
fallback).
- **RPC / snap / lifecycle** are exercised through mock messenger action
handlers (`NetworkController`, `SnapController`, `KeyringController`,
staking reads) and live events (`AccountActivityService:balanceUpdated`,
`AccountsController:accountBalancesUpdated`,
`TransactionController:transactionConfirmed`,
`KeyringController:unlock`).

Scenario coverage (7 `full` + 5 `merge`): snap failure carry-over,
skipped-asset last-amount copy, zero-seeding for never-seen visible
assets, RPC fallback carry-over on failing `balanceOf` reads,
disappearing zero-balance seeds, failed-fetch carry-over, fresh amounts
on healthy fallback, event overlays, malformed-row handling,
wrong-amount correction at the next full poll, snap-event overlay +
refresh, and staked ETH on mainnet + Hoodi across an unlock lifecycle
with a staking deposit.

### Test plan

- `yarn workspace @metamask/assets-controller run test`
  - New suite: 12/12 scenarios pass
  - Full package suite passes
- `yarn lint` checks on the new files: oxlint and oxfmt clean
- `yarn build` (root, full monorepo type-check) passes

### Notes for reviewers

- Fixture balance-row shape includes `unit`, required by the keyring
API's `Balance` struct via `strictMask` (`@metamask/keyring-api`); the
controller discards it downstream.
- Native ETH on Hoodi is keyed under the zero-address ERC-20 encoding
(`eip155:560048/erc20:0x0000000000000000000000000000000000000000`) — the
controller's documented fallback for EVM chains absent from its
native-asset seed map, which excludes testnets.
- The Multicall3 `aggregate3` fixture encodes the contract's real return
shape — `Result[]` only, with no leading `blockNumber` (only
`aggregate`/`tryAggregate` return one). An earlier draft of the fixture
encoded `(uint256, Result[])`, which made every batch decode throw and
silently fall through to single `balanceOf` calls; caught in review and
fixed, so the scenarios exercise the batched read path.
`MulticallClient`'s decoder is correct as shipped — no product bug here.

## References

- Recreates MetaMask#10535, which was accidentally opened from a personal fork
that has since been deleted (deleting the fork auto-closed that PR). The
changes are identical; this time the head branch lives on this
repository so the `Validate changelog diffs` job can resolve
`origin/<head-ref>` (the fork-PR limitation documented in that PR).

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
— no entries needed: test-only, non-consumer-facing changes
(`no-changelog` label applied)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them — N/A, no breaking changes

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Test-only additions with mocked boundaries; no changes to runtime
balance merge or fetch behavior shipped to consumers.
> 
> **Overview**
> Adds
**`AssetsController.stale-balance-scenarios.integration.test.ts`**, an
integration suite that runs the real **`AssetsController`** with
**`assetsAccountsApiV6: true`** and exercises all **12 stale-balance
scenarios** from the accounts API v6 decision doc (Solana snap
failures/skips, BNB Chain RPC fallback when v6 marks a chain
unprocessed, Account Activity live overlays/malformed rows, and staked
ETH across mainnet + Hoodi after unlock and a staking tx).
> 
> Supporting changes introduce a **`__fixtures__/stale-balance/`** tree:
real HTTP captures for Accounts/Tokens/Prices APIs wired through
**`mockStaleBalanceApis`**, mutable mock EVM RPC (Multicall3
**`aggregate3`**, failing tokens, staking reads), Solana keyring snap
handlers, wallet/account builders, and messenger registration for
BSC/mainnet/Hoodi networks. **`getIgnoringCase`** is added to
**`test-utils`** for case-insensitive balance assertions, and
**`tsc-suppressions.json`** picks up suppressions for fixture/test
typing.
> 
> There is **no production logic change** in this PR—only tests,
fixtures, and type suppressions.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
245e36c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant