Skip to content

Commit 6701092

Browse files
piyalbasuclaude
andcommitted
fix(icons): show USDT0's icon, for new and existing users
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 <img> 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 <img>, 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) <noreply@anthropic.com>
1 parent 6c30f39 commit 6701092

4 files changed

Lines changed: 101 additions & 6 deletions

File tree

@shared/api/helpers/getIconFromTokenList.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ export const getIconFromTokenLists = async ({
2020
}) => {
2121
let verifiedToken = {} as AssetListReponseItem;
2222
let canonicalAsset = undefined as string | undefined;
23-
for (const data of assetsListsData) {
23+
// The lists arrive in the user's configured order, which is a priority order:
24+
// the first list carrying the asset wins. Without the labeled break below the
25+
// outer loop runs to completion and the LAST matching list silently overwrites
26+
// earlier, higher-priority entries.
27+
listLoop: for (const data of assetsListsData) {
2428
const list = data.assets;
2529
if (list) {
2630
for (const record of list) {
@@ -29,7 +33,7 @@ export const getIconFromTokenLists = async ({
2933
if (record.contract && record.contract.match(regex) && record.icon) {
3034
verifiedToken = record;
3135
canonicalAsset = getCanonicalFromAsset(code, contractId);
32-
break;
36+
break listLoop;
3337
}
3438
}
3539

@@ -42,7 +46,7 @@ export const getIconFromTokenLists = async ({
4246
) {
4347
verifiedToken = record;
4448
canonicalAsset = getCanonicalFromAsset(code, issuerId);
45-
break;
49+
break listLoop;
4650
}
4751
}
4852
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { getCachedAssetIconList } from "../getCachedAssetIconList";
2+
3+
const USDT0 = "USDT0:GATISXX6BZ6NC7IKQBY37CJD4SOZL3CYZJWXEDG6JVIY4WBS6KXJHN6Q";
4+
const USDC = "USDC:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN";
5+
const USDC_ICON = "https://centre.io/usdc.png";
6+
7+
const makeLocalStore = (assetIconCache: unknown) =>
8+
({
9+
getItem: jest.fn(async () => assetIconCache),
10+
setItem: jest.fn(),
11+
remove: jest.fn(),
12+
clear: jest.fn(),
13+
}) as any;
14+
15+
describe("getCachedAssetIconList", () => {
16+
it("returns the icons it has", async () => {
17+
const result = await getCachedAssetIconList({
18+
localStore: makeLocalStore({ [USDC]: USDC_ICON }),
19+
});
20+
21+
expect(result.icons).toEqual({ [USDC]: USDC_ICON });
22+
});
23+
24+
it("leaves out assets recorded as having no icon", async () => {
25+
// A null here means an earlier lookup came up empty, and getAssetIcons
26+
// reads it as "already tried, don't look again". Because this cache is on
27+
// disk that verdict outlived the session, so an asset whose icon failed
28+
// once — USDT0, whose LOBSTR url 403s browsers — stayed iconless forever.
29+
// Dropping nulls turns it back into an ordinary cache miss, and the fresh
30+
// lookup overwrites the stale entry.
31+
const result = await getCachedAssetIconList({
32+
localStore: makeLocalStore({ [USDC]: USDC_ICON, [USDT0]: null }),
33+
});
34+
35+
expect(result.icons).toEqual({ [USDC]: USDC_ICON });
36+
});
37+
38+
it("returns an empty map when nothing is cached", async () => {
39+
const result = await getCachedAssetIconList({
40+
localStore: makeLocalStore(undefined),
41+
});
42+
43+
expect(result.icons).toEqual({});
44+
});
45+
});

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ export const getCachedAssetIconList = async ({
1010
(await localStore.getItem(CACHED_ASSET_ICONS_ID)) || {};
1111

1212
return {
13-
icons: assetIconCache,
13+
// A null entry records that a lookup came up empty, and getAssetIcons reads
14+
// one as "already tried, don't look again". That was only ever meant to
15+
// last a session, but this cache is on disk, so the verdict outlived it: an
16+
// asset whose icon failed once stayed iconless for good, with no way back —
17+
// no icon means no <img>, so nothing fires the error handler that would
18+
// have retried. Leaving nulls out makes them ordinary cache misses, and the
19+
// fresh lookup overwrites the stale entry.
20+
icons: Object.fromEntries(
21+
Object.entries(assetIconCache).filter(([, iconUrl]) => iconUrl),
22+
),
1423
};
1524
};

extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const VERIFIED_TOKEN_ISSUER = validAssetList.assets[0].issuer;
1111
const VERIFIED_TOKEN_CODE = validAssetList.assets[0].code;
1212
const EXPECTED_ICON_URL = validAssetList.assets[0].icon;
1313

14-
jest
14+
(jest
1515
.spyOn(ExtensionMessaging, "sendMessageToBackground")
1616
.mockImplementation(() =>
1717
Promise.resolve({
@@ -20,7 +20,7 @@ jest
2020
),
2121
jest
2222
.spyOn(TokenListHelpers, "getCombinedAssetListData")
23-
.mockImplementation(() => Promise.resolve([validAssetList]));
23+
.mockImplementation(() => Promise.resolve([validAssetList])));
2424

2525
describe("getIconFromTokenLists", () => {
2626
it("should return an icon if an asset is in a token list by contract ID", async () => {
@@ -55,3 +55,40 @@ describe("getIconFromTokenLists", () => {
5555
expect(canonicalAsset).toBeUndefined();
5656
});
5757
});
58+
59+
describe("getIconFromTokenLists list priority", () => {
60+
// USDT0 is on the Soroswap list with a direct https icon and on the LOBSTR
61+
// list with an ipfs.io gateway url. That gateway answers curl but returns 403
62+
// to a browser User-Agent, so an <img> can never load it. Keeping whichever
63+
// list was read last handed the browser the one url it cannot fetch.
64+
const laterList = {
65+
...validAssetList,
66+
provider: "Later Provider",
67+
assets: [
68+
{
69+
...validAssetList.assets[0],
70+
icon: "https://later-list.example/icon.png",
71+
},
72+
],
73+
};
74+
75+
it("uses the icon from the first matching list, matched by issuer", async () => {
76+
const { icon } = await getIconFromTokenLists({
77+
issuerId: VERIFIED_TOKEN_ISSUER,
78+
code: VERIFIED_TOKEN_CODE,
79+
assetsListsData: [validAssetList, laterList],
80+
});
81+
82+
expect(icon).toEqual(EXPECTED_ICON_URL);
83+
});
84+
85+
it("uses the icon from the first matching list, matched by contract ID", async () => {
86+
const { icon } = await getIconFromTokenLists({
87+
contractId: VERIFIED_TOKEN_CONTRACT,
88+
code: VERIFIED_TOKEN_CODE,
89+
assetsListsData: [validAssetList, laterList],
90+
});
91+
92+
expect(icon).toEqual(EXPECTED_ICON_URL);
93+
});
94+
});

0 commit comments

Comments
 (0)