fix(icons): show USDT0's icon, for new and existing users (v5.48.0 backport)

@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;
         }
       }
     }

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({});
+  });
+});

extension/src/background/messageListener/handlers/getCachedAssetIconList.ts

@@ -10,6 +10,52 @@ 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 <img>, 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 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. 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
+    // filter would silently swallow one.
+    //
+    // 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),
+    ),
   };
 };

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 <img> 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);
+  });
+});