Add a TTL to the cachedAssetIconsId storage cache

[aristidesstaffieri]

Problem

cachedAssetIconsId in extension local storage maps CODE:ISSUER → icon URL. Entries are written on first resolution and never invalidated: no timestamp, no TTL, no expiry path.

getAssetIcons gates on the cache upstream of the token list data:

const cachedIcon = cachedIcons[canonical];
if (cachedIcon) { assetIcons[canonical] = cachedIcon; continue; }
// ...
const tokenListIcon = await getIconFromTokenLists({ ... });

— @shared/api/internal.ts:1345

Because the gate is per-asset and sits before assetsListsData is read, re-fetching the token lists has no effect on an asset that resolved once. The write in getIconFromTokenLists (@shared/api/helpers/getIconFromTokenList.ts:52) is write-on-miss, not write-through-on-refresh.

Rationale

Token lists are mutable; we cache them as if they were immutable. List maintainers and issuers rotate icon URLs for rehosting, CDN migrations, and rebrands. Every rotation is invisible to any user who resolved that asset beforehand. Cache lifetime is effectively install lifetime.

There is no remedy for the user or for support. The only code that clears the key is localStore.clear() in resetExperimentalData, reachable only from the DEV_SERVER-gated /integration-test route. cache.clearAll has no callers. The only advice we can give today is "clear extension storage" or reinstall.

Failures latch permanently. retryAssetIcon fires only on <img> load error and persists iconUrl: null; getAssetIcons then treats null as do-not-retry. The comment there claims the null lives only in Redux, but cacheAssetIcon writes it to local storage and GET_CACHED_ASSET_ICON_LIST returns it verbatim, so one transient image failure suppresses that icon indefinitely. A TTL expires these nulls as a side effect.

The staleness is user-visible and self-contradictory. The Swap and Add-Asset pickers render record.icon straight from freshly fetched lists (popup/helpers/searchAsset.ts), bypassing this cache — so one session can show the new icon in the picker and the old one on the account list.

The map only grows. Entries for assets the user no longer holds are never pruned.

Proposed change

Expire the cache on a staleness window, reusing the <storageKey>_date convention already in background/helpers/cachedFetch.ts:

• Write cachedAssetIconsId_date (epoch ms) alongside the map. On read, if it is older than the TTL, drop the map and let the existing write-on-miss path repopulate from the current lists.
• Put the check in the background read handlers (getCachedAssetIcons.ts, getCachedAssetIconList.ts) so every consumer inherits it.
• Map-level expiry needs no entry-shape migration, and an absent _date already reads as stale (Number(undefined || "") → 0), so existing installs expire on first read. Per-entry { url, updatedAt } is the alternative if we want staggered refresh, at the cost of a migration plus tolerant reads.
• Suggested TTL: 7 days, matching cachedFetch. Icons change rarely and a miss costs one token list lookup we already perform.

Out of scope

getIconFromTokenLists has no outer-loop break, so the last matching list wins rather than the first — a separate correctness question worth its own issue.

[piyalbasu]

#3004 takes on the staleness problem described here, and I think it supersedes
this issue — flagging rather than closing, since it's your call.

The diagnosis here is right and #3004 leans on it: the cache gate sits upstream
of the list read, so an asset that resolved once never consults list data again,
and a rotated icon URL is invisible for the life of the install.

Where it lands differently is the mechanism. Rather than dropping the map when
it goes stale, #3004 compares each cached URL against the current candidates and
only touches entries that actually disagree. Same trigger, same 7-day window,
but nothing correct is discarded — no icon blanks while it re-resolves, and an
entry the lists still agree with is never rewritten. It also picks up the part a
TTL can't reach on its own: a URL that never rendered in the first place, which
re-resolving on a timer would just fetch again.

One note on the rationale above — the "failures latch permanently" paragraph is
now out of date. #2994 filters persisted nulls on read, so a transient image
failure no longer suppresses an icon indefinitely, and a TTL is no longer needed
to clear them.

The map-only-grows point isn't carried over into #3004; if pruning entries for
assets the user no longer holds is worth keeping, that's a good reason to narrow
this issue to that rather than close it.

[CassioMG]

Closing in favor or #3004