Skip to content

fix: fix account api cache cp-8.8.0 - #34748

Open
salimtb wants to merge 5 commits into
mainfrom
fix/fix-account-api-cache
Open

fix: fix account api cache cp-8.8.0#34748
salimtb wants to merge 5 commits into
mainfrom
fix/fix-account-api-cache

Conversation

@salimtb

@salimtb salimtb commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

This PR removes cache usage when requesting the Account API to fetch balances, ensuring that forceUpdate calls result in a real network request rather than a near-immediate cache hit.

Currently, AccountsApiDataSource uses { staleTime: 100, gcTime: 100 } for all fetches — including forced ones. This means that even when forceUpdate: true is set (e.g. via AssetsController.getAssets), React Query may return stale cached data within the 100ms window instead of hitting the network. This is incorrect behavior for an explicit force-refresh path.

The fix patches @metamask/assets-controller@13.1.3 so that when request.forceUpdate is set, fetchOptions uses { staleTime: 0, gcTime: 0 } — bypassing the cache entirely and guaranteeing a fresh network fetch. Both .cjs and .mjs builds are patched, and package.json (resolutions + dependencies) and yarn.lock are updated accordingly.

Core PR: MetaMask/core#9870

Changelog

CHANGELOG entry: remove account api cache

Related issues

Fixes:

Manual testing steps

Feature: Force-refresh account balances bypass cache

  Scenario: User triggers a forced balance refresh
    Given the app is open with at least one account with token balances
    And the account API has been called at least once (cache is warm)

    When a force-refresh is triggered (e.g. pull-to-refresh or returning to the wallet tab)
    Then the app fetches fresh balances from the network
    And the balance displayed reflects the latest on-chain state, not a cached value

Screenshots/Recordings

Before

ScreenRecording_08-12-2026.15-09-45_1.MP4

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Low Risk
Small behavioral change in dependency caching for explicit force-refresh only; slightly more API traffic on those paths but no auth or data-model changes.

Overview
Patches @metamask/assets-controller@13.1.3 so forced account/asset fetches actually skip the React Query cache instead of reusing data for ~100ms.

In AccountsApiDataSource, when request.forceUpdate is set, fetchOptions changes from { staleTime: 100, gcTime: 100 } to { staleTime: 0, gcTime: 0 } (both .cjs and .mjs builds). That aligns “force update” paths—such as AssetsController.getAssets with forceUpdate: true—with a real network refresh rather than a near-immediate cache hit.

Wiring is updated in package.json (resolutions + dependencies) and yarn.lock so the app resolves the patched package instead of the stock npm release.

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

@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamask-ci

metamask-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.
  • Pre-merge author checklist has unchecked items (e.g. "I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards."). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

Match core revert of #9591: forceUpdate balance fetches use
staleTime/gcTime 0 again instead of the 100ms window.
Align with MetaMask/core#9867: invalidate balance queries and fetch
with staleTime/gcTime 0 on every AccountsApiDataSource fetch.
Revert forceUpdate balance cache window from 100ms to staleTime/gcTime
0 so forced refreshes bypass TanStack cache (MetaMask/core#9870).
@salimtb
salimtb marked this pull request as ready for review August 14, 2026 09:40
@salimtb
salimtb requested a review from a team as a code owner August 14, 2026 09:40
@salimtb salimtb changed the title fix: fix account api cache fix: fix account api cache cp-8.8.0 Aug 14, 2026
Retarget assets-controller forceUpdate cache patch from 13.1.2 to 13.1.3
(staleTime/gcTime 0 on forceUpdate) after main bump.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added the risk:high AI analysis: high risk label Aug 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps, SmokeMMConnect
  • Selected Performance tags: @PerformanceAssetLoading, @PerformanceAccountList
  • Risk Level: high
  • AI Confidence: 100%
click to see 🤖 AI reasoning details

E2E Test Selection:
Hard rule (controller-version-update): @MetaMask controller package version updated in package.json: @metamask/assets-controller@npm:^13.1.3, @metamask/assets-controller. Running all tests.

Performance Test Selection:
The patch changes cache timing in AccountsApiDataSource from staleTime/gcTime of 100ms to 0ms for forceUpdate requests. This could affect asset loading performance since force-update requests will now always hit the network rather than potentially using a 100ms cache window. @PerformanceAssetLoading directly measures token list rendering and balance fetching performance, which is the primary area affected. @PerformanceAccountList measures account selector performance which includes token load impact — also relevant since account asset data is fetched via this data source.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

All tests passed · 4 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

✅ Passed Tests (4)
Test Platform Device Duration Team Recording
Account creation after fresh install Android Google Pixel 8 Pro (v14.0) 1.06s @metamask-onboarding-team 📹 Watch
Asset View, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 2.33s @assets-dev-team 📹 Watch
Aggregated Balance Loading Time, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 8.27s @assets-dev-team 📹 Watch
Import SRP with +50 accounts, SRP 1, SRP 2, SRP 3 Android Google Pixel 8 Pro (v14.0) 3.98s @Accounts-team 📹 Watch

Branch: fix/fix-account-api-cache · Build: E2E · Commit: aeeb511 · View full run

pull Bot pushed a commit to dmrazzy/core that referenced this pull request Aug 14, 2026
…ts api request (MetaMask#9591) (MetaMask#9870)

This reverts commit 1b082f7.

Extension PR: MetaMask/metamask-extension#45494
Mobile PR: MetaMask/metamask-mobile#34748

## Explanation

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] 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)
- [ ] 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

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches balance refresh behavior on user-visible paths (force
refresh); tradeoff is fresher balances vs slightly more Accounts API
traffic, with low security impact.
> 
> **Overview**
> Reverts [MetaMask#9591](MetaMask#9591): when
`AccountsApiDataSource` handles a balance fetch with **`forceUpdate:
true`**, TanStack Query options go back to **`staleTime: 0` and `gcTime:
0`** instead of a 100ms window.
> 
> Forced refreshes (e.g. after unlock, account/network changes, or
explicit refresh) are meant to hit the Accounts API with a fresh
request; the short cache window could reuse a just-fetched entry and
leave balances stale. The unit test is renamed/updated to assert cache
bypass on `forceUpdate`.
> 
> The **Unreleased** changelog documents this revert; it also
consolidates the Arc native USDC default-tracking note under Unreleased
and drops a duplicate entry from the 13.1.3 section.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
ed2395f. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Salim TOUBAL <salimtb@users.noreply.github.com>
@tommasini

Copy link
Copy Markdown
Contributor

Can we have a PRdescription please?

@tommasini

Copy link
Copy Markdown
Contributor

Also it would be nice to link the core PR with that change ongoing

@salimtb

salimtb commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

@tommasini done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants