fix(assets-controller): price deduper drops erc20 prices for inflight joiners and can mix currencies

[gomesalexandre]

Explanation

Two coupled bugs in PriceDataSource's price-fetching deduper (DedupingBatchFetcher + #executeBatchFetch/#fetchSpotPrices):

Bug A — inflight joiners silently lose ERC-20 prices

assetsMiddleware requests/stores CAIP-19 asset IDs in checksummed form (normalizeAssetId), but the Price API's response echoes back ERC-20 addresses lowercase. The deduper's inflight-joiner path (dedupingBatchFetcher.ts:186) looks up the batch's raw response by the requested key. A caller starting the batch happened to be rescued downstream (AssetsController's own response-key re-normalization at merge time), but a caller joining the same fetch as an in-flight promise looked its checksummed key up directly against the raw (lowercase) response and got undefined — the price silently vanished, self-healing only after the freshness TTL expired.

Bug B — currency-blind cache key

The deduper's cache/inflight key was just the asset ID, with no currency component. invalidate()/invalidateKeys() (called on a currency change) are documented to leave in-flight promises alone. So a currency switch racing an in-flight fetch could join — or, worse, later receive on settlement — a price fetched under the previously-selected currency.

Fix

• Introduce a composite currency:assetId deduper key (PriceDeduperKey).
• Normalize asset IDs consistently at the point a key is built and at the point the API response is matched back against it, using safeNormalizeAssetId throughout — response data is untrusted, and one malformed key must not poison an otherwise-valid batch (a real gap in my first draft, caught in review — see receipts).
• Decode the currency from the key inside #executeBatchFetch instead of re-reading the live selected currency at execution time, which is itself a second, independent source of the same race (the currency could move on between when a batch is queued and when its async callback actually runs).
• Composite keys alone stop a new request from joining a stale-currency fetch, but a slow stale-currency fetch that's already independently in flight would otherwise still complete and let its now-superseded values reach the caller. #executeBatchFetch now discards its own result if the selected currency has moved on by the time the batch settles — closing the other half of the race.
• Defensive invariant: #executeBatchFetch throws if a batch is ever handed keys spanning more than one currency (currently unreachable given how keys are built, but cheap to make loud rather than silently wrong if that invariant is ever broken by a future change).

References

None — found via independent code-review/testing during this session, not tied to a filed issue.

Changelog

### Fixed
- Fix `PriceDataSource` losing ERC-20 prices for callers that joined an in-flight fetch, caused by the deduper matching the Price API's response (lowercase-cased addresses) against the caller's checksummed request key
- Fix a currency switch racing an in-flight price fetch, which could let a caller join (or receive) a price fetched under the previously-selected currency

receipts

Real, independently-reproduced repros (not just reasoning from source) for both bugs, plus genuine red-before/green-after on every new test:

$ NODE_OPTIONS=--experimental-vm-modules yarn jest src/data-sources/PriceDataSource.test.ts --coverage=false
Test Suites: 1 passed, 1 total
Tests:       49 passed, 49 total

New regression describe blocks:

• regression: checksummed vs lowercase asset ID casing — an inflight joiner receiving undefined for a checksummed key against a lowercase-keyed response, plus a lowercase-vs-checksummed caller-coalescing test (2 API calls → 1)
• regression: forceUpdate invalidation uses the normalized deduper key — proves invalidateKeys targets the same normalized key the fetch populated, not a silent no-op
• regression: currency switch racing an inflight fetch — a USD fetch hangs, currency switches to EUR mid-flight, the EUR request gets the EUR price (not the stale USD one), and the stale USD result is discarded on settlement rather than delivered to anyone

Genuine red-before/green-after — stashed just the source fix (kept the new tests), reran:

$ git stash push -- packages/assets-controller/src/data-sources/PriceDataSource.ts
$ NODE_OPTIONS=--experimental-vm-modules yarn jest src/data-sources/PriceDataSource.test.ts --coverage=false
Tests:       7 failed, 40 passed, 47 total
  ✕ an inflight joiner receives the price when the API response key casing differs...  (Received: undefined)
  ✕ a request under a new currency does not join an inflight fetch...                  (Exceeded timeout of 5000ms — deadlocked joining the stale hanging promise)
  ✕ a caller requesting the lowercase form coalesces onto an inflight fetch...          (2 calls, expected 1)
  + 4 pre-existing assertions I updated to expect normalization, correctly red against un-normalized old code
$ git stash pop
$ NODE_OPTIONS=--experimental-vm-modules yarn jest src/data-sources/PriceDataSource.test.ts --coverage=false
Tests:       49 passed, 49 total

Full package suite (all consumers, incl. AssetsController.test.ts):

$ NODE_OPTIONS=--experimental-vm-modules yarn jest --coverage=false
Test Suites: 33 passed, 33 total
Tests:       985 passed, 985 total

Real repo-wide typecheck (the actual CI command, not a scoped tsc --noEmit that hits unrelated project-reference build-order noise):

$ yarn lint:tsc
$ echo $?
0

Lint clean:

$ yarn eslint packages/assets-controller/src/data-sources/PriceDataSource.ts packages/assets-controller/src/data-sources/PriceDataSource.test.ts
(no output, exit 0)

Changelog validated:

$ yarn changelog:validate
$ echo $?
0

Adversarial review (Codex, synchronous)

Ran Codex against the diff before opening this PR. It found real, substantive gaps in my first draft, all fixed before this PR was opened:

1. High — a slow stale-currency batch could still complete and overwrite a fresher currency's data in state, even with composite-key isolation preventing the join. → added the settlement-time currency check described above.
2. Medium — the USD companion-price lookup used the raw (non-normalized) key, so if the currency-specific and USD-baseline responses ever used different address casing for the same asset, a valid price could be silently discarded. → normalize the USD companion response too before joining.
3. Medium — normalizeAssetId (which can throw on malformed input) was used unguarded in three places touching untrusted/batch data, so one malformed asset ID could poison an entire otherwise-valid batch. → switched to safeNormalizeAssetId at all three sites.
4. Low — no test proved forceUpdate/invalidateKeys still worked correctly against the new composite key, and no test proved a genuinely different-cased caller (not just a differently-cased API response) coalesces correctly. → added both.

Checklist

• I've updated the test suite for new or updated code as needed
• I've updated documentation (JSDoc, README, e.g.) for new or updated code as needed
• I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as needed
• I've prepared draft PRs for clients to resolve any breaking changes
• I've highlighted TypeScript type changes that may be missed during release

---

Note

Medium Risk
Changes core price-fetch caching and coalescing used across the assets pipeline; behavior is well-covered by new regression tests but incorrect keying could still affect displayed fiat values after currency switches or concurrent fetches.

Overview
Fixes two bugs in PriceDataSource’s price-fetch deduper: inflight joiners could miss ERC-20 prices when API response keys were lowercase but request keys were checksummed, and a currency change could coalesce onto or apply stale in-flight prices because cache keys were asset-only.

The deduper now uses currency:normalizedAssetId keys, maps API responses back through safeNormalizeAssetId (including the USD companion batch), forceUpdate invalidates those composite keys, and #executeBatchFetch drops results if the selected currency moved on while the request was in flight. Regression tests cover address casing, forced refresh, and currency-switch races; the package changelog documents the fixes.

^{Reviewed by Cursor Bugbot for commit 363f6cb. Bugbot is set up for automated code reviews on this repo. Configure here.}