From 6701092003ce12ae819f11efa281c970d551a720 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 2 Sep 2026 16:13:00 -0400 Subject: [PATCH 1/6] fix(icons): show USDT0's icon, for new and existing users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit USDT0 is carried by two of the default asset lists: Soroswap, with a direct https icon, and LOBSTR, with an ipfs.io gateway url. That gateway serves the image to curl but returns 403 to a browser User-Agent, so an can never load it. Two things then went wrong. Icon lookup only broke out of its inner record loop, so the outer loop ran to completion and the LAST matching list won — handing the browser the one url it cannot fetch. The lists arrive in the user's configured order, which is meant to be a priority order, so the first match should win. When that image failed, the retry recorded "no icon" in the cache as a null, and lookup reads a null as "already tried, don't look again". That was only ever meant to last a session, but the cache is on disk, so the verdict outlived it: the asset stayed iconless for good, with no way back — no icon means no , so nothing fires the error handler that would have retried. Reading the cache now skips nulls, which turns them into ordinary cache misses and lets the fresh lookup overwrite the stale entry. Users already stuck recover on first load. The in-session memo is untouched: it lives in Redux, which is where the comment in getAssetIcons always said it lived. Co-Authored-By: Claude Opus 5 (1M context) --- @shared/api/helpers/getIconFromTokenList.ts | 10 +++-- .../__tests__/getCachedAssetIconList.test.ts | 45 +++++++++++++++++++ .../handlers/getCachedAssetIconList.ts | 11 ++++- .../__tests__/getIconFromTokenLists.test.js | 41 ++++++++++++++++- 4 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 extension/src/background/messageListener/handlers/__tests__/getCachedAssetIconList.test.ts diff --git a/@shared/api/helpers/getIconFromTokenList.ts b/@shared/api/helpers/getIconFromTokenList.ts index ae97610db8..aa42c39169 100644 --- a/@shared/api/helpers/getIconFromTokenList.ts +++ b/@shared/api/helpers/getIconFromTokenList.ts @@ -20,7 +20,11 @@ export const getIconFromTokenLists = async ({ }) => { let verifiedToken = {} as AssetListReponseItem; let canonicalAsset = undefined as string | undefined; - for (const data of assetsListsData) { + // The lists arrive in the user's configured order, which is a priority order: + // the first list carrying the asset wins. Without the labeled break below the + // outer loop runs to completion and the LAST matching list silently overwrites + // earlier, higher-priority entries. + listLoop: for (const data of assetsListsData) { const list = data.assets; if (list) { for (const record of list) { @@ -29,7 +33,7 @@ export const getIconFromTokenLists = async ({ if (record.contract && record.contract.match(regex) && record.icon) { verifiedToken = record; canonicalAsset = getCanonicalFromAsset(code, contractId); - break; + break listLoop; } } @@ -42,7 +46,7 @@ export const getIconFromTokenLists = async ({ ) { verifiedToken = record; canonicalAsset = getCanonicalFromAsset(code, issuerId); - break; + break listLoop; } } } diff --git a/extension/src/background/messageListener/handlers/__tests__/getCachedAssetIconList.test.ts b/extension/src/background/messageListener/handlers/__tests__/getCachedAssetIconList.test.ts new file mode 100644 index 0000000000..3dbbf4b402 --- /dev/null +++ b/extension/src/background/messageListener/handlers/__tests__/getCachedAssetIconList.test.ts @@ -0,0 +1,45 @@ +import { getCachedAssetIconList } from "../getCachedAssetIconList"; + +const USDT0 = "USDT0:GATISXX6BZ6NC7IKQBY37CJD4SOZL3CYZJWXEDG6JVIY4WBS6KXJHN6Q"; +const USDC = "USDC:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN"; +const USDC_ICON = "https://centre.io/usdc.png"; + +const makeLocalStore = (assetIconCache: unknown) => + ({ + getItem: jest.fn(async () => assetIconCache), + setItem: jest.fn(), + remove: jest.fn(), + clear: jest.fn(), + }) as any; + +describe("getCachedAssetIconList", () => { + it("returns the icons it has", async () => { + const result = await getCachedAssetIconList({ + localStore: makeLocalStore({ [USDC]: USDC_ICON }), + }); + + expect(result.icons).toEqual({ [USDC]: USDC_ICON }); + }); + + it("leaves out assets recorded as having no icon", async () => { + // A null here means an earlier lookup came up empty, and getAssetIcons + // reads it as "already tried, don't look again". Because this cache is on + // disk that verdict outlived the session, so an asset whose icon failed + // once — USDT0, whose LOBSTR url 403s browsers — stayed iconless forever. + // Dropping nulls turns it back into an ordinary cache miss, and the fresh + // lookup overwrites the stale entry. + const result = await getCachedAssetIconList({ + localStore: makeLocalStore({ [USDC]: USDC_ICON, [USDT0]: null }), + }); + + expect(result.icons).toEqual({ [USDC]: USDC_ICON }); + }); + + it("returns an empty map when nothing is cached", async () => { + const result = await getCachedAssetIconList({ + localStore: makeLocalStore(undefined), + }); + + expect(result.icons).toEqual({}); + }); +}); diff --git a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts index e27161ffb5..57da00c65b 100644 --- a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts +++ b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts @@ -10,6 +10,15 @@ export const getCachedAssetIconList = async ({ (await localStore.getItem(CACHED_ASSET_ICONS_ID)) || {}; return { - icons: assetIconCache, + // A null entry records that a lookup came up empty, and getAssetIcons reads + // one as "already tried, don't look again". That was only ever meant to + // last a session, but this cache is on disk, so the verdict outlived it: an + // asset whose icon failed once stayed iconless for good, with no way back — + // no icon means no , so nothing fires the error handler that would + // have retried. Leaving nulls out makes them ordinary cache misses, and the + // fresh lookup overwrites the stale entry. + icons: Object.fromEntries( + Object.entries(assetIconCache).filter(([, iconUrl]) => iconUrl), + ), }; }; diff --git a/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js b/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js index 49ffe0d371..5974ffb2cb 100644 --- a/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js +++ b/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js @@ -11,7 +11,7 @@ const VERIFIED_TOKEN_ISSUER = validAssetList.assets[0].issuer; const VERIFIED_TOKEN_CODE = validAssetList.assets[0].code; const EXPECTED_ICON_URL = validAssetList.assets[0].icon; -jest +(jest .spyOn(ExtensionMessaging, "sendMessageToBackground") .mockImplementation(() => Promise.resolve({ @@ -20,7 +20,7 @@ jest ), jest .spyOn(TokenListHelpers, "getCombinedAssetListData") - .mockImplementation(() => Promise.resolve([validAssetList])); + .mockImplementation(() => Promise.resolve([validAssetList]))); describe("getIconFromTokenLists", () => { it("should return an icon if an asset is in a token list by contract ID", async () => { @@ -55,3 +55,40 @@ describe("getIconFromTokenLists", () => { expect(canonicalAsset).toBeUndefined(); }); }); + +describe("getIconFromTokenLists list priority", () => { + // USDT0 is on the Soroswap list with a direct https icon and on the LOBSTR + // list with an ipfs.io gateway url. That gateway answers curl but returns 403 + // to a browser User-Agent, so an can never load it. Keeping whichever + // list was read last handed the browser the one url it cannot fetch. + const laterList = { + ...validAssetList, + provider: "Later Provider", + assets: [ + { + ...validAssetList.assets[0], + icon: "https://later-list.example/icon.png", + }, + ], + }; + + it("uses the icon from the first matching list, matched by issuer", async () => { + const { icon } = await getIconFromTokenLists({ + issuerId: VERIFIED_TOKEN_ISSUER, + code: VERIFIED_TOKEN_CODE, + assetsListsData: [validAssetList, laterList], + }); + + expect(icon).toEqual(EXPECTED_ICON_URL); + }); + + it("uses the icon from the first matching list, matched by contract ID", async () => { + const { icon } = await getIconFromTokenLists({ + contractId: VERIFIED_TOKEN_CONTRACT, + code: VERIFIED_TOKEN_CODE, + assetsListsData: [validAssetList, laterList], + }); + + expect(icon).toEqual(EXPECTED_ICON_URL); + }); +}); From bb1a9e93ccd60790cb8d1e7b43a79d7538eb0e3a Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 2 Sep 2026 16:33:38 -0400 Subject: [PATCH 2/6] docs(icons): spell out that the null filter is a workaround, not the fix Records what the real fix is and why it needs three steps in order: stop persisting nulls, migrate the ones already on disk, then drop the filter. Notes that migrating alone would not hold, since every later icon failure writes a fresh null, and that the filter currently rules out a durable negative cache we may well want. Co-Authored-By: Claude Opus 5 (1M context) --- .../handlers/getCachedAssetIconList.ts | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts index 57da00c65b..cab3ccdbe1 100644 --- a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts +++ b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts @@ -11,12 +11,31 @@ export const getCachedAssetIconList = async ({ return { // A null entry records that a lookup came up empty, and getAssetIcons reads - // one as "already tried, don't look again". That was only ever meant to - // last a session, but this cache is on disk, so the verdict outlived it: an - // asset whose icon failed once stayed iconless for good, with no way back — - // no icon means no , so nothing fires the error handler that would - // have retried. Leaving nulls out makes them ordinary cache misses, and the - // fresh lookup overwrites the stale entry. + // one as "already tried, don't look again". That was only ever meant to last + // a session, but this cache is on disk, so the verdict outlived it: an asset + // whose icon failed once stayed iconless for good, with no way back — no + // icon means no , so nothing fires the error handler that would have + // retried. Dropping nulls here turns them back into ordinary cache misses, + // and the fresh lookup overwrites the stale entry. + // + // TODO: this is a read-side workaround, not the real fix. It hides bad data + // rather than stopping it being written, and it permanently rules out ever + // storing a legitimate null here — which we may well want, since an asset + // with genuinely no icon currently re-runs the whole lookup chain (token + // lists, then Horizon, then the issuer's stellar.toml) on every cold popup + // open. A durable negative cache with a TTL is the sane answer to that, and + // this filter would silently swallow it. + // + // To fix properly, in order: + // 1. Stop persisting nulls: retryAssetIcon sends `iconUrl: null` meaning + // "clear this", so cacheAssetIcon should delete the entry rather than + // store the null. + // 2. Add a migration to clear the nulls already on disk. It cannot be + // done by (1) alone, since nothing rewrites an entry the lookup skips. + // 3. Drop this filter, so the read path goes back to being a plain + // accessor and null is free to mean something again. + // Doing (2) without (1) is not enough on its own: every later icon failure + // writes a fresh null and puts that user straight back into the bug. icons: Object.fromEntries( Object.entries(assetIconCache).filter(([, iconUrl]) => iconUrl), ), From 676006d3bcf6c2b8d112420ffbefae94d0937e44 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 2 Sep 2026 16:38:22 -0400 Subject: [PATCH 3/6] fix(icons): consult the session's icon record before looking up again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Account view re-runs the icon hook every time balances change, and the lookup pass was handed an empty cache, so it ignored what the session had already worked out. An asset with no icon anywhere went through the whole chain again on every pass — token lists, then Horizon, then the issuer's toml — rather than once. Passing the session's icon map means the nulls it holds do what they were meant to: mark an asset as settled for this session. Behaviour on a fresh popup is unchanged, since that map starts empty. Raised in review of the persisted-null fix. It is not caused by that change: getAssetIcons records nothing for an asset it skips, so a cached null and an absent entry both left the asset looking unresolved to this hook and both triggered the same repeat lookup. Co-Authored-By: Claude Opus 5 (1M context) --- .../hooks/__tests__/useGetIcons.test.tsx | 75 +++++++++++++++++++ .../popup/views/Account/hooks/useGetIcons.tsx | 10 ++- 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx diff --git a/extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx b/extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx new file mode 100644 index 0000000000..bd1d20225e --- /dev/null +++ b/extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx @@ -0,0 +1,75 @@ +import React from "react"; +import { renderHook, act } from "@testing-library/react"; + +import * as ApiInternal from "@shared/api/internal"; +import * as TokenListHelpers from "@shared/api/helpers/token-list"; +import { TESTNET_NETWORK_DETAILS } from "@shared/constants/stellar"; +import { Wrapper } from "popup/__testHelpers__"; +import { useGetIcons } from "../useGetIcons"; + +const PUBLIC_KEY = "G123"; +const NETWORK = TESTNET_NETWORK_DETAILS.network; +const ICONLESS = "NOICON:GISSUER1"; + +const state = { + settings: { networkDetails: TESTNET_NETWORK_DETAILS, networksList: [] }, + cache: { + balanceData: { + [NETWORK]: { + [PUBLIC_KEY]: { + balances: { + [ICONLESS]: { + token: { code: "NOICON", issuer: { key: "GISSUER1" } }, + }, + }, + }, + }, + }, + // The session already established this asset has no icon anywhere. + icons: { [ICONLESS]: null }, + tokenLists: [{ assets: [] }], + homeDomains: {}, + tokenDetails: {}, + historyData: {}, + tokenPrices: {}, + collections: {}, + popularTokens: {}, + }, +}; + +const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + +); + +describe("useGetIcons", () => { + afterEach(() => jest.restoreAllMocks()); + + it("honours the session's record that an asset has no icon", async () => { + // Account re-runs this hook every time balances change. Without the + // session's own icon map, an asset with no icon anywhere repeats the whole + // chain — token lists, then Horizon, then the issuer's toml — on every one + // of those passes. + jest + .spyOn(ApiInternal, "getAssetIconCache") + .mockResolvedValue({ icons: {} }); + jest + .spyOn(TokenListHelpers, "getCombinedAssetListData") + .mockResolvedValue([]); + const getAssetIcons = jest + .spyOn(ApiInternal, "getAssetIcons") + .mockResolvedValue({}); + + const { result } = renderHook(() => useGetIcons(), { wrapper }); + await act(async () => { + await result.current.fetchData(); + }); + + const lookupCall = getAssetIcons.mock.calls.find( + ([args]) => (args as any).assetsListsData !== undefined, + ); + expect(lookupCall).toBeDefined(); + expect((lookupCall![0] as any).cachedIcons).toEqual({ [ICONLESS]: null }); + }); +}); diff --git a/extension/src/popup/views/Account/hooks/useGetIcons.tsx b/extension/src/popup/views/Account/hooks/useGetIcons.tsx index 09b322654e..3b812ab901 100644 --- a/extension/src/popup/views/Account/hooks/useGetIcons.tsx +++ b/extension/src/popup/views/Account/hooks/useGetIcons.tsx @@ -12,6 +12,7 @@ import { saveIconsForBalances, saveTokenLists, tokensListsSelector, + iconsSelector, } from "popup/ducks/cache"; import { settingsNetworkDetailsSelector, @@ -36,6 +37,9 @@ function useGetIcons() { ); const cachedBalances = useSelector(balancesSelector); const cachedTokenLists = useSelector(tokensListsSelector); + // The session's own record of what has already been resolved — including the + // nulls that mark an asset as having no icon anywhere. + const sessionIcons = useSelector(iconsSelector); const { assetsLists } = useSelector(settingsSelector); const publicKey = useSelector(publicKeySelector); const networkDetails = useSelector(settingsNetworkDetailsSelector); @@ -63,7 +67,11 @@ function useGetIcons() { balances: assetsWithoutIcons, networkDetails, assetsListsData, - cachedIcons: {}, + // Account re-runs this hook on every balances change. Passing the + // session's icons means an asset already found to have no icon + // anywhere is not put through the whole chain again — token lists, + // then Horizon, then the issuer's toml — on each of those passes. + cachedIcons: sessionIcons, }); payload.icons = { ...assetsWithIcons, ...updatedIcons }; reduxDispatch(saveIconsForBalances({ icons: updatedIcons })); From dc29db9cbfff3ae8539c08c2d6b50fbcd235c98f Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 2 Sep 2026 16:48:54 -0400 Subject: [PATCH 4/6] Revert "fix(icons): consult the session's icon record before looking up again" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 676006d3b0f6a1f0e0e5e1e0f8a4f5d5f3c8e7a1. Honouring the session's icon map here is unsafe, because that map is shared with a path that looks less hard. History's getIconUrl consults the token lists and, when they are cached — which the Account view guarantees — never reaches the issuer-toml fallback, then records `iconUrl || null` for whatever it did not find. That null goes into the same Redux map. So an asset whose icon exists only in its issuer's stellar.toml gets marked "no icon" by a lookup that never tried the toml, and this change made the Account view believe it. The asset then renders iconless for the rest of the session, which is a session-scoped version of exactly the latched failure this PR exists to fix. A transient error inside getIconUrlFromIssuer latches the same way. The repeat lookups this was meant to avoid are pre-existing and not introduced by this PR: getAssetIcons records nothing for an asset it skips, so a cached null and an absent entry are indistinguishable to this hook and both already triggered the same lookup. Leaving that as it was. Doing this properly means stopping the history path from recording a negative verdict for a lookup it deliberately skipped, which is a change to another view and belongs outside a hotfix. Co-Authored-By: Claude Opus 5 (1M context) --- .../hooks/__tests__/useGetIcons.test.tsx | 75 ------------------- .../popup/views/Account/hooks/useGetIcons.tsx | 10 +-- 2 files changed, 1 insertion(+), 84 deletions(-) delete mode 100644 extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx diff --git a/extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx b/extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx deleted file mode 100644 index bd1d20225e..0000000000 --- a/extension/src/popup/views/Account/hooks/__tests__/useGetIcons.test.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import React from "react"; -import { renderHook, act } from "@testing-library/react"; - -import * as ApiInternal from "@shared/api/internal"; -import * as TokenListHelpers from "@shared/api/helpers/token-list"; -import { TESTNET_NETWORK_DETAILS } from "@shared/constants/stellar"; -import { Wrapper } from "popup/__testHelpers__"; -import { useGetIcons } from "../useGetIcons"; - -const PUBLIC_KEY = "G123"; -const NETWORK = TESTNET_NETWORK_DETAILS.network; -const ICONLESS = "NOICON:GISSUER1"; - -const state = { - settings: { networkDetails: TESTNET_NETWORK_DETAILS, networksList: [] }, - cache: { - balanceData: { - [NETWORK]: { - [PUBLIC_KEY]: { - balances: { - [ICONLESS]: { - token: { code: "NOICON", issuer: { key: "GISSUER1" } }, - }, - }, - }, - }, - }, - // The session already established this asset has no icon anywhere. - icons: { [ICONLESS]: null }, - tokenLists: [{ assets: [] }], - homeDomains: {}, - tokenDetails: {}, - historyData: {}, - tokenPrices: {}, - collections: {}, - popularTokens: {}, - }, -}; - -const wrapper = ({ children }: { children: React.ReactNode }) => ( - - {children} - -); - -describe("useGetIcons", () => { - afterEach(() => jest.restoreAllMocks()); - - it("honours the session's record that an asset has no icon", async () => { - // Account re-runs this hook every time balances change. Without the - // session's own icon map, an asset with no icon anywhere repeats the whole - // chain — token lists, then Horizon, then the issuer's toml — on every one - // of those passes. - jest - .spyOn(ApiInternal, "getAssetIconCache") - .mockResolvedValue({ icons: {} }); - jest - .spyOn(TokenListHelpers, "getCombinedAssetListData") - .mockResolvedValue([]); - const getAssetIcons = jest - .spyOn(ApiInternal, "getAssetIcons") - .mockResolvedValue({}); - - const { result } = renderHook(() => useGetIcons(), { wrapper }); - await act(async () => { - await result.current.fetchData(); - }); - - const lookupCall = getAssetIcons.mock.calls.find( - ([args]) => (args as any).assetsListsData !== undefined, - ); - expect(lookupCall).toBeDefined(); - expect((lookupCall![0] as any).cachedIcons).toEqual({ [ICONLESS]: null }); - }); -}); diff --git a/extension/src/popup/views/Account/hooks/useGetIcons.tsx b/extension/src/popup/views/Account/hooks/useGetIcons.tsx index 3b812ab901..09b322654e 100644 --- a/extension/src/popup/views/Account/hooks/useGetIcons.tsx +++ b/extension/src/popup/views/Account/hooks/useGetIcons.tsx @@ -12,7 +12,6 @@ import { saveIconsForBalances, saveTokenLists, tokensListsSelector, - iconsSelector, } from "popup/ducks/cache"; import { settingsNetworkDetailsSelector, @@ -37,9 +36,6 @@ function useGetIcons() { ); const cachedBalances = useSelector(balancesSelector); const cachedTokenLists = useSelector(tokensListsSelector); - // The session's own record of what has already been resolved — including the - // nulls that mark an asset as having no icon anywhere. - const sessionIcons = useSelector(iconsSelector); const { assetsLists } = useSelector(settingsSelector); const publicKey = useSelector(publicKeySelector); const networkDetails = useSelector(settingsNetworkDetailsSelector); @@ -67,11 +63,7 @@ function useGetIcons() { balances: assetsWithoutIcons, networkDetails, assetsListsData, - // Account re-runs this hook on every balances change. Passing the - // session's icons means an asset already found to have no icon - // anywhere is not put through the whole chain again — token lists, - // then Horizon, then the issuer's toml — on each of those passes. - cachedIcons: sessionIcons, + cachedIcons: {}, }); payload.icons = { ...assetsWithIcons, ...updatedIcons }; reduxDispatch(saveIconsForBalances({ icons: updatedIcons })); From 8c45234b1d20c844f72a14a0bfacc9ac94f06a03 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 2 Sep 2026 17:34:45 -0400 Subject: [PATCH 5/6] docs(icons): correct what the null filter actually costs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier note claimed the repeat lookup "already happens today via useGetIcons, so filtering here does not add it". That is true of the Account view's icon hook, which passes an empty cache to its lookup pass and so retried either way — and false of the flows that load balances with icons (swap, send, manage assets, history), where a null-marked asset was skipped outright and now is not. Says instead that retrying is the point, since it is how a stuck asset recovers, and is honest that it is newly paid in those flows and never terminates for an asset with no icon anywhere. Names the per-attempt cost, and why a negative cache wants a TTL rather than the latch we just removed. Co-Authored-By: Claude Opus 5 (1M context) --- .../handlers/getCachedAssetIconList.ts | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts index cab3ccdbe1..e7863f7e17 100644 --- a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts +++ b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts @@ -19,12 +19,29 @@ export const getCachedAssetIconList = async ({ // and the fresh lookup overwrites the stale entry. // // TODO: this is a read-side workaround, not the real fix. It hides bad data - // rather than stopping it being written, and it permanently rules out ever - // storing a legitimate null here — which we may well want, since an asset - // with genuinely no icon currently re-runs the whole lookup chain (token - // lists, then Horizon, then the issuer's stellar.toml) on every cold popup - // open. A durable negative cache with a TTL is the sane answer to that, and - // this filter would silently swallow it. + // rather than stopping it being written, and it rules out ever storing a + // meaningful null here. + // + // Retrying is the point: it is how an asset that got stuck gets its icon + // back. But the retry is not free, and for an asset with no icon anywhere + // it repeats forever without ever succeeding. Each attempt costs a + // token-list scan and, when that finds nothing, one batched Horizon call + // covering every such issuer plus a stellar.toml fetch for each issuer that + // publishes a home domain. (USDT0 stops at the Horizon call: its issuer + // publishes no home domain and, with its master key at weight 0, never + // can.) + // + // That cost is newly paid by the flows that load balances with icons — + // swap, send, manage assets, history — which previously skipped a + // null-marked asset outright. The Account view's own icon hook already + // retried regardless, since it passes an empty cache to its lookup pass, so + // nothing changes there. Redux holds the only in-session memo and starts + // empty on every popup open, so nothing suppresses the retry across opens + // either. + // + // A negative cache that survives the popup, with a TTL so it expires rather + // than latching, is the sane way to stop the endless retry — and this + // filter would silently swallow one. // // To fix properly, in order: // 1. Stop persisting nulls: retryAssetIcon sends `iconUrl: null` meaning From 5b8ca7919e2bad7039bffd4523835ed8ed14a145 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 2 Sep 2026 17:35:53 -0400 Subject: [PATCH 6/6] docs(icons): say why the retry is not remembered between popups "Every popup open" only explains itself if you already know the popup is destroyed when it closes. Spell that out: closing it takes Redux with it, and Redux is the only record of what the session had already resolved, so each open starts from nothing. Follows removing "cold popup open", which was invented jargon and read as though there were some warmer kind of open. Co-Authored-By: Claude Opus 5 (1M context) --- .../messageListener/handlers/getCachedAssetIconList.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts index e7863f7e17..000c55f196 100644 --- a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts +++ b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts @@ -35,9 +35,10 @@ export const getCachedAssetIconList = async ({ // swap, send, manage assets, history — which previously skipped a // null-marked asset outright. The Account view's own icon hook already // retried regardless, since it passes an empty cache to its lookup pass, so - // nothing changes there. Redux holds the only in-session memo and starts - // empty on every popup open, so nothing suppresses the retry across opens - // either. + // nothing changes there. And nothing suppresses the retry from one popup to + // the next: closing the popup tears it down, taking Redux — the only record + // of what this session already resolved — with it, so every open starts + // from nothing. // // A negative cache that survives the popup, with a TTL so it expires rather // than latching, is the sane way to stop the endless retry — and this