test(assets-controller): websocket price-update integration tests - #10476
Merged
Merged
Conversation
Extract the websocket-update pipeline construction into `buildWsUpdateSources` (mirroring `buildFastFetchSources` for the fast pipeline) so the post-websocket-event enrichment pass can be tested in isolation from `AssetsController.handleAssetsUpdate`. Cover the behavior from #10410 at both layers: - `buildWsUpdateSources.price-updates.integration.test.ts` drives the real `AccountActivityDataSource` + `executeAssetsPipeline` against recorded API responses, asserting detected/existing assets receive Tokens API metadata and Price API spot prices in the same pass. - `AssetsController.ws-price-updates.integration.test.ts` runs the full controller on a closed lifecycle, publishing `AccountActivityService:balanceUpdated` events and asserting state balances, metadata, and spot prices land from the same pipeline pass. Fixtures are captured from the live APIs into `__fixtures__/ws-price-updates/` via `captureWsApiResponses.ts`, mirroring the `scam-token-cleanup` fixture layout.
Resolve the AssetsController.ts conflict: keep the WS-update lane construction (buildWsUpdateSources) from this branch and adopt main's #getUpdatePipelineRequest helper and named source booleans. Adapt the new WS pipeline tests to main's constructor-level getAssetsState API and the renamed AssetsControllerState type, and pass oxlint (new on main).
Match the surrounding code's comment density: drop the inline rationale comments from `#handleAssetsUpdateV5` (the lane ordering rationale lives in `buildWsUpdateSources`' JSDoc, mirroring `buildFastFetchSources`) and cut the capture script's multi-paragraph header (with its shell example) down to a two-line summary — operational details stay in the PR description.
Clean merge: dependency bumps and release changelogs only.
Route `#handleAssetsUpdateV6`'s websocket lane through `buildWsUpdateSources`, mirroring how `buildFastFetchSources` serves the v5 and v6 fast-fetch lanes: the builder now takes `includeCustomAssetGraduation` (true on the v5 lane; the v6 lane never graduates custom assets and instead runs the RPC fallback ahead of detection) and the v6 handler's inline occurrence-filter/RPC-fallback construction is gone. Cover both generations: unit rows for the v6 lane shapes, a v6 pass in the pipeline integration test (RPC fallback passthrough on a successful response), and a controller-level v6 scenario booted with `assetsAccountsApiV6: true` via the fixture's remote feature flags.
Keep one-line summaries and only the required @param/@returns tags; drop the rationale paragraphs, scenario explanations that repeat the test titles, and multi-sentence constant/fixture docs.
Route both update handlers' non-websocket lanes through a sibling builder of `buildWsUpdateSources`: the gating booleans (`includeCustomAssetGraduation`, `includeRpcFallback`) stay at the call sites, so each handler's exact lane composition is preserved. Unit tests cover the five lane shapes.
Prithpal-Sooriya
marked this pull request as draft
September 28, 2026 11:13
Follow the bsc-spam-token integration-test format: an
"Integration Expectation" header, response-surface tables driving
`it.each('$surface - …')` assertions, `describe.each` rows sharing the
v5/v6 lane its, shared fixture builders for the combined event and the
scenario states, and `askedAbout`/`priceOf` helpers instead of inline
set-building and type casts.
…ults Assert the captured spot prices exactly (ETH 2688.8502994319642, USDC 0.999966) instead of numeric ranges, and nest each run's recorded mock output under `mocks` (`priceAPI`, `tokenAPI`, `rpc`) so the pipeline/controller results read apart from what the mocks observed.
One `it` per causal chain: the held-but-unpriced asset is queued, fetched and priced in one test (pipeline and controller level), and the already-priced token's not-queued/not-invoked/not-overwritten checks merge into one. The lane describes keep their per-mechanism `it`s, where each covers the full two-asset set.
Comment on lines
+414
to
+417
| it('does not re-price the already-priced token', () => { | ||
| expect(result.request.assetsForPriceUpdate ?? []).toStrictEqual([]); | ||
| expect(result.mocks.priceAPI.priceBatches).toStrictEqual([]); | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
@Kriys94 Is this a legitimate behaviour or a bug?
If I'm a user that has an ERC20 token (that has balance/metadata/prices).
If a WS event comes through am I:
- Meant to see a price update?
- Not meant to see a price update?
Prithpal-Sooriya
marked this pull request as ready for review
September 28, 2026 13:18
salimtb
approved these changes
Sep 28, 2026
1 of 4 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Code Walkthrough:
https://www.loom.com/share/a4226f00271249e0aa65d002468914f5
Adds integration tests covering the websocket (AccountActivity) balance-update pipeline behavior from #10410 — after a
balanceUpdatedevent, the pipeline now requestsmetadataandpriceenrichment so both existing holdings and tokens first surfaced by the websocket get Tokens API metadata and a Price API spot price in the same pass (instead of waiting for the next price poll).To make that pass testable in isolation (mirroring
buildFastFetchSourcesfor the fast pipeline, which serves both the v5 and v6 fast-fetch lanes), the websocket-update source construction is extracted fromAssetsController#handleAssetsUpdateV5and#handleAssetsUpdateV6intobuildWsUpdateSources. LikebuildFastFetchSources, the builder takes anincludeCustomAssetGraduationoption (trueon the v5 lane; the v6 lane never graduates custom assets and instead runs the RPC fallback ahead of detection), and is tested at three layers:buildWsUpdateSources.test.ts— unit tests of the builder.buildWsUpdateSources.price-updates.integration.test.ts— drives the realAccountActivityDataSourceandexecuteAssetsPipeline(viabuildWsUpdateSources) against recorded API responses:AssetsController.ws-price-updates.integration.test.ts— full-controller integration test: a closed lifecycle,AccountActivityService:balanceUpdatedpublished on the root messenger, asserting state balances, metadata, and spot prices all land from the same pipeline pass — on the default v5 lane and again withassetsAccountsApiV6: true(the v6 lane, where the RPC fallback replaces graduation).API fixtures are captured from the live APIs into
__fixtures__/ws-price-updates/(re-runnable viacaptureWsApiResponses.ts), mirroring thescam-token-cleanupfixture layout. Like that folder,captureWsApiResponses.tsuses the asyncwriteFilehelpers from@metamask/utils/nodeso it passesimport-x/no-nodejs-modules.No consumer-facing behavior changes:
buildWsUpdateSourcesis internal-only (the pipeline module is not re-exported from the package index), so no changelog entry is needed.References
AccountActivityDataSourcemetadata/price enrichment on websocket updates)__fixtures__/scam-token-cleanup/; pipeline extraction mirrorsbuildFastFetchSourcesChecklist
Note
Low Risk
Changes are an internal pipeline refactor plus tests; PR description states no consumer-facing behavior change.
Overview
Refactors AssetsController update enrichment so inline middleware lists in
#handleAssetsUpdateV5/#handleAssetsUpdateV6are replaced bybuildUpdateSources(Accounts API and other non-websocket updates) andbuildWsUpdateSources(AccountActivity websocket updates), mirroring the existingbuildFastFetchSourcespattern. Websocket lanes still differ by v5 vs v6 flags (custom-asset graduation vs RPC fallback, plus occurrence filtering on basic functionality).Adds integration and unit coverage for the websocket balance-update path: metadata and spot prices land in the same pass as a
AccountActivityService:balanceUpdatedevent (new holdings, held-but-unpriced natives, and skipping re-price/refetch for already-enriched tokens), exercised both throughexecuteAssetsPipelineand fullAssetsControllerruns on v5 andassetsAccountsApiV6v6 lanes.Ships recorded API fixtures under
__fixtures__/ws-price-updates/and acaptureWsApiResponses.tsscript to refresh them.Reviewed by Cursor Bugbot for commit 1a942e6. Bugbot is set up for automated code reviews on this repo. Configure here.