fix(icons): keep the asset icon that actually loads, not the first one listed

@shared/api/__tests__/internal.test.ts

@@ -5,6 +5,8 @@ import {
   TESTNET_NETWORK_DETAILS,
 } from "@shared/constants/stellar";
 import * as GetLedgerKeyAccounts from "../helpers/getLedgerKeyAccounts";
+import * as GetIconUrlFromIssuer from "../helpers/getIconUrlFromIssuer";
+import * as IconProbe from "../helpers/iconProbe";
 import * as internalApi from "../internal";
 import { sendMessageToBackground } from "@shared/api/helpers/extensionMessaging";
 import { SERVICE_TYPES } from "@shared/constants/services";
@@ -392,4 +394,231 @@ describe("internalApi", () => {
       });
     });
   });
+  describe("retryAssetIcon", () => {
+    const KEY = "GATISXX6BZ6NC7IKQBY37CJD4SOZL3CYZJWXEDG6JVIY4WBS6KXJHN6Q";
+    const CODE = "USDT0";
+    const CANONICAL = `${CODE}:${KEY}`;
+    const FAILED_ICON = "https://ipfs.io/ipfs/bafkreidead";
+    const OTHER_DEAD_ICON = "https://also-dead.example/icon.png";
+    const LIVE_ICON = "https://docs.usdt0.to/downloads/usdt0/icon.png";
+    const TOML_ICON = "https://usdt0.to/icon.png";
+
+    const listWith = (icon: string, provider: string) =>
+      ({
+        name: "Test list",
+        description: "",
+        network: "public",
+        version: "1.0",
+        provider,
+        assets: [
+          {
+            code: CODE,
+            issuer: KEY,
+            name: CODE,
+            org: CODE,
+            domain: "usdt0.to",
+            icon,
+            decimals: 7,
+          },
+        ],
+      }) as any;
+
+    /** Stands in for the browser: only the named urls render. */
+    const onlyLoads = (...loadable: string[]) =>
+      jest
+        .spyOn(IconProbe, "firstLoadableIconUrl")
+        .mockImplementation(async (urls: string[]) =>
+          urls.find((url) => loadable.includes(url)),
+        );
+
+    const retry = (
+      assetsListsData: unknown[],
+      failedIcon: string = FAILED_ICON,
+    ) =>
+      internalApi.retryAssetIcon({
+        activePublicKey: null,
+        key: KEY,
+        code: CODE,
+        assetIcons: { [CANONICAL]: failedIcon },
+        networkDetails: MAINNET_NETWORK_DETAILS,
+        assetsListsData: assetsListsData as any,
+      });
+
+    it("settles on a candidate that loads", async () => {
+      onlyLoads(LIVE_ICON);
+
+      const result = await retry([
+        listWith(OTHER_DEAD_ICON, "A"),
+        listWith(LIVE_ICON, "B"),
+      ]);
+
+      expect(result[CANONICAL]).toEqual(LIVE_ICON);
+    });
+
+    it("never re-offers the url that just failed, even if it would load", async () => {
+      // The url rendered once and is cached; it is failing now. Handing it
+      // back would just re-render the same broken image.
+      onlyLoads(FAILED_ICON, LIVE_ICON);
+
+      const result = await retry([
+        listWith(FAILED_ICON, "A"),
+        listWith(LIVE_ICON, "B"),
+      ]);
+
+      expect(result[CANONICAL]).toEqual(LIVE_ICON);
+    });
+
+    it("falls back to the issuer TOML when the lists offer nothing else", async () => {
+      onlyLoads(TOML_ICON);
+      jest
+        .spyOn(GetIconUrlFromIssuer, "getIconUrlFromIssuer")
+        .mockResolvedValue(TOML_ICON);
+
+      const result = await retry([listWith(FAILED_ICON, "A")]);
+
+      expect(result[CANONICAL]).toEqual(TOML_ICON);
+    });
+
+    it("clears the icon when no source offers anything that loads", async () => {
+      onlyLoads();
+      jest
+        .spyOn(GetIconUrlFromIssuer, "getIconUrlFromIssuer")
+        .mockResolvedValue(TOML_ICON);
+
+      const result = await retry([listWith(FAILED_ICON, "A")]);
+
+      expect(result[CANONICAL]).toEqual("");
+    });
+  });
+
+  describe("getAssetIcons", () => {
+    const KEY = "GATISXX6BZ6NC7IKQBY37CJD4SOZL3CYZJWXEDG6JVIY4WBS6KXJHN6Q";
+    const CODE = "USDT0";
+    const CANONICAL = `${CODE}:${KEY}`;
+    const DEAD_ICON = "https://ipfs.io/ipfs/bafkreidead";
+    const LIVE_ICON = "https://docs.usdt0.to/downloads/usdt0/icon.png";
+    const TOML_ICON = "https://usdt0.to/icon.png";
+
+    const balances = {
+      [CANONICAL]: {
+        token: { code: CODE, issuer: { key: KEY } },
+      },
+    } as any;
+
+    const listWith = (icon: string, provider: string) =>
+      ({
+        name: "Test list",
+        description: "",
+        network: "public",
+        version: "1.0",
+        provider,
+        assets: [
+          {
+            code: CODE,
+            issuer: KEY,
+            name: CODE,
+            org: CODE,
+            domain: "usdt0.to",
+            icon,
+            decimals: 7,
+          },
+        ],
+      }) as any;
+
+    /** Stands in for the browser: only `loadable` renders. */
+    const onlyLoads = (loadable: string) =>
+      jest
+        .spyOn(IconProbe, "firstLoadableIconUrl")
+        .mockImplementation(async (urls: string[]) =>
+          urls.find((url) => url === loadable),
+        );
+
+    it("uses the candidate that loads rather than the one listed first", async () => {
+      onlyLoads(LIVE_ICON);
+
+      const icons = await internalApi.getAssetIcons({
+        balances,
+        networkDetails: MAINNET_NETWORK_DETAILS,
+        assetsListsData: [listWith(DEAD_ICON, "A"), listWith(LIVE_ICON, "B")],
+        cachedIcons: {},
+      });
+
+      expect(icons[CANONICAL]).toEqual(LIVE_ICON);
+    });
+
+    it("caches the icon it settled on, not the one it rejected", async () => {
+      onlyLoads(LIVE_ICON);
+
+      await internalApi.getAssetIcons({
+        balances,
+        networkDetails: MAINNET_NETWORK_DETAILS,
+        assetsListsData: [listWith(DEAD_ICON, "A"), listWith(LIVE_ICON, "B")],
+        cachedIcons: {},
+      });
+
+      expect(mockedSend).toHaveBeenCalledWith({
+        activePublicKey: null,
+        assetCanonical: CANONICAL,
+        iconUrl: LIVE_ICON,
+        type: SERVICE_TYPES.CACHE_ASSET_ICON,
+      });
+    });
+
+    it("falls back to the issuer TOML when no list candidate loads", async () => {
+      onlyLoads(TOML_ICON);
+      jest
+        .spyOn(GetLedgerKeyAccounts, "getLedgerKeyAccounts")
+        .mockResolvedValue({
+          [KEY]: { home_domain: "usdt0.to" },
+        } as any);
+      jest
+        .spyOn(GetIconUrlFromIssuer, "getIconUrlFromIssuer")
+        .mockResolvedValue(TOML_ICON);
+
+      const icons = await internalApi.getAssetIcons({
+        balances,
+        networkDetails: MAINNET_NETWORK_DETAILS,
+        assetsListsData: [listWith(DEAD_ICON, "A")],
+        cachedIcons: {},
+      });
+
+      expect(icons[CANONICAL]).toEqual(TOML_ICON);
+    });
+
+    it("discards an issuer TOML icon that does not load", async () => {
+      // Nothing renders, including the toml's own url.
+      onlyLoads("https://nothing-loads.example/icon.png");
+      jest
+        .spyOn(GetLedgerKeyAccounts, "getLedgerKeyAccounts")
+        .mockResolvedValue({
+          [KEY]: { home_domain: "usdt0.to" },
+        } as any);
+      jest
+        .spyOn(GetIconUrlFromIssuer, "getIconUrlFromIssuer")
+        .mockResolvedValue(TOML_ICON);
+
+      const icons = await internalApi.getAssetIcons({
+        balances,
+        networkDetails: MAINNET_NETWORK_DETAILS,
+        assetsListsData: [listWith(DEAD_ICON, "A")],
+        cachedIcons: {},
+      });
+
+      expect(icons[CANONICAL]).toBeNull();
+    });
+
+    it("trusts an already-cached icon without re-probing it", async () => {
+      const probe = jest.spyOn(IconProbe, "firstLoadableIconUrl");
+
+      const icons = await internalApi.getAssetIcons({
+        balances,
+        networkDetails: MAINNET_NETWORK_DETAILS,
+        assetsListsData: [listWith(LIVE_ICON, "A")],
+        cachedIcons: { [CANONICAL]: LIVE_ICON },
+      });
+
+      expect(icons[CANONICAL]).toEqual(LIVE_ICON);
+      expect(probe).not.toHaveBeenCalled();
+    });
+  });
 });

@shared/api/helpers/__tests__/iconProbe.test.ts

@@ -0,0 +1,140 @@
+import {
+  ICON_LOAD_BUDGET_MS,
+  canLoadIcon,
+  firstLoadableIconUrl,
+} from "../iconProbe";
+
+const A = "https://a.example/icon.png";
+const B = "https://b.example/icon.png";
+const C = "https://c.example/icon.png";
+
+/** Probe stub that reports success only for the urls named as loadable. */
+const probeThatLoads = (...loadable: string[]) =>
+  jest.fn(async (url: string) => loadable.includes(url));
+
+describe("firstLoadableIconUrl", () => {
+  it("returns the first candidate that loads", async () => {
+    const probe = probeThatLoads(B);
+
+    const result = await firstLoadableIconUrl([A, B, C], { probe });
+
+    expect(result).toEqual(B);
+  });
+
+  it("stops probing once a candidate loads", async () => {
+    const probe = probeThatLoads(A, B, C);
+
+    await firstLoadableIconUrl([A, B, C], { probe });
+
+    expect(probe).toHaveBeenCalledTimes(1);
+    expect(probe).toHaveBeenCalledWith(A, expect.any(Number));
+  });
+
+  it("returns undefined when no candidate loads", async () => {
+    const result = await firstLoadableIconUrl([A, B], {
+      probe: probeThatLoads(),
+    });
+
+    expect(result).toBeUndefined();
+  });
+
+  it("returns undefined for an empty candidate list without probing", async () => {
+    const probe = probeThatLoads(A);
+
+    const result = await firstLoadableIconUrl([], { probe });
+
+    expect(result).toBeUndefined();
+    expect(probe).not.toHaveBeenCalled();
+  });
+
+  it("spends one shared budget across candidates rather than one per candidate", async () => {
+    // Each probe burns more than the whole budget, so only the first candidate
+    // should ever be attempted no matter how many are queued.
+    const probe = jest.fn(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 30));
+      return false;
+    });
+
+    const result = await firstLoadableIconUrl([A, B, C], {
+      budgetMs: 20,
+      probe,
+    });
+
+    expect(result).toBeUndefined();
+    expect(probe).toHaveBeenCalledTimes(1);
+  });
+
+  it("hands each probe only the budget that is left", async () => {
+    const probe = jest.fn(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 20));
+      return false;
+    });
+
+    await firstLoadableIconUrl([A, B], { budgetMs: 200, probe });
+
+    const [, firstTimeout] = probe.mock.calls[0];
+    const [, secondTimeout] = probe.mock.calls[1];
+    expect(secondTimeout).toBeLessThan(firstTimeout as number);
+  });
+});
+
+describe("canLoadIcon", () => {
+  const originalImage = global.Image;
+
+  /**
+   * Stands in for the browser's HTMLImageElement: records the assigned src and
+   * lets each test decide whether that src "loads", so no network is involved.
+   */
+  const stubImage = (outcome: "load" | "error" | "never") => {
+    const instances: any[] = [];
+    (global as any).Image = class {
+      onload: (() => void) | null = null;
+      onerror: (() => void) | null = null;
+      set src(_value: string) {
+        instances.push(this);
+        if (outcome === "never") {
+          return;
+        }
+        setTimeout(() => {
+          if (outcome === "load") {
+            this.onload?.();
+          } else {
+            this.onerror?.();
+          }
+        }, 0);
+      }
+    };
+    return instances;
+  };
+
+  afterEach(() => {
+    (global as any).Image = originalImage;
+  });
+
+  it("resolves true when the image loads", async () => {
+    stubImage("load");
+
+    await expect(canLoadIcon(A, 100)).resolves.toBe(true);
+  });
+
+  it("resolves false when the image errors", async () => {
+    stubImage("error");
+
+    await expect(canLoadIcon(A, 100)).resolves.toBe(false);
+  });
+
+  it("resolves false when the image never settles within the timeout", async () => {
+    stubImage("never");
+
+    await expect(canLoadIcon(A, 20)).resolves.toBe(false);
+  });
+});
+
+describe("ICON_LOAD_BUDGET_MS", () => {
+  it("leaves headroom over a typical icon fetch without being a visible stall", () => {
+    // Measured: both real USDT0 candidates fully load in 130-300ms including
+    // cold DNS + TLS for a ~8KB png.
+    expect(ICON_LOAD_BUDGET_MS).toBeGreaterThanOrEqual(1000);
+    expect(ICON_LOAD_BUDGET_MS).toBeLessThanOrEqual(1500);
+  });
+});