diff --git a/@shared/api/helpers/__tests__/token-list.test.ts b/@shared/api/helpers/__tests__/token-list.test.ts new file mode 100644 index 0000000000..1838734e19 --- /dev/null +++ b/@shared/api/helpers/__tests__/token-list.test.ts @@ -0,0 +1,345 @@ +import { getCombinedAssetListData } from "../token-list"; +import { + MAINNET_NETWORK_DETAILS, + TESTNET_NETWORK_DETAILS, + NETWORKS, +} from "@shared/constants/stellar"; +import { + DEFAULT_ASSETS_LISTS, + AssetsListItem, +} from "@shared/constants/soroban/asset-list"; + +// Mock Sentry captureException +jest.mock("@sentry/browser", () => ({ + captureException: jest.fn(), +})); + +import { captureException } from "@sentry/browser"; + +describe("getCombinedAssetListData", () => { + beforeEach(() => { + jest.clearAllMocks(); + // Mock fetch globally + global.fetch = jest.fn() as jest.Mock; + }); + + it("should return cached asset lists when provided and not empty", async () => { + const cachedAssetLists = [ + { + name: "Cached Asset List", + provider: "Cached Provider", + description: "Cached Description", + version: "1.0.0", + network: "mainnet", + assets: [ + { + code: "CACHED", + issuer: "GCACHED123", + contract: "CCACHED456", + domain: "cached.com", + icon: "https://cached.com/icon.png", + decimals: 7, + }, + ], + }, + ]; + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + cachedAssetLists, + }); + + // Should return cached lists without fetching + expect(result).toEqual(cachedAssetLists); + expect(result.length).toBe(1); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("should fetch asset lists when cachedAssetLists is empty", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [], + }; + + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + cachedAssetLists: [], + }); + + // Should fetch lists when cache is empty + expect(global.fetch).toHaveBeenCalled(); + expect(result.length).toBeGreaterThan(0); + }); + + it("should fetch asset lists when cachedAssetLists is undefined", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [], + }; + + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + cachedAssetLists: undefined, + }); + + // Should fetch lists when cache is undefined + expect(global.fetch).toHaveBeenCalled(); + expect(result.length).toBeGreaterThan(0); + }); + + it("should handle 429 rate limit error gracefully and continue with other lists", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [ + { + code: "TEST", + issuer: "GABCDEF123", + contract: "C123456", + domain: "test.com", + icon: "https://test.com/icon.png", + decimals: 7, + }, + ], + }; + + // Mock fetch to return 429 for Soroswap URL and success for others + (global.fetch as jest.Mock).mockImplementation((url: string) => { + if (url.includes("soroswap/token-list")) { + return Promise.resolve({ + ok: false, + status: 429, + statusText: "Too Many Requests", + }); + } + // Return success for other URLs + return Promise.resolve({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + }); + + // Should return successful lists (excluding the 429 one) + expect(result.length).toBeGreaterThan(0); + expect(result).not.toContain(null); + expect(result).not.toContain(undefined); + + // Verify that captureException was called for the 429 error + expect(captureException).toHaveBeenCalledWith( + expect.stringContaining("Failed to load asset list"), + ); + expect(captureException).toHaveBeenCalledWith( + expect.stringContaining("(429)"), + ); + }); + + it("should handle network errors gracefully", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [], + }; + + // Mock fetch to throw network error for Soroswap URL + (global.fetch as jest.Mock).mockImplementation((url: string) => { + if (url.includes("soroswap/token-list")) { + return Promise.reject(new Error("Network error")); + } + return Promise.resolve({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + }); + + // Should return successful lists (excluding the failed one) + expect(result.length).toBeGreaterThan(0); + expect(result).not.toContain(null); + + // Verify that captureException was called for the network error + expect(captureException).toHaveBeenCalledWith( + expect.stringContaining("Failed to load asset list"), + ); + }); + + it("should handle 500 server errors gracefully", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [], + }; + + // Mock fetch to return 500 for Soroswap URL + (global.fetch as jest.Mock).mockImplementation((url: string) => { + if (url.includes("soroswap/token-list")) { + return Promise.resolve({ + ok: false, + status: 500, + statusText: "Internal Server Error", + }); + } + return Promise.resolve({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + }); + + // Should return successful lists (excluding the 500 one) + expect(result.length).toBeGreaterThan(0); + expect(result).not.toContain(null); + + // Verify that captureException was called + expect(captureException).toHaveBeenCalledWith( + expect.stringContaining("Failed to load asset list"), + ); + expect(captureException).toHaveBeenCalledWith( + expect.stringContaining("(500)"), + ); + }); + + it("should handle JSON parsing errors gracefully", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [], + }; + + // Mock fetch to return invalid JSON for Soroswap URL + (global.fetch as jest.Mock).mockImplementation((url: string) => { + if (url.includes("soroswap/token-list")) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => { + throw new Error("Invalid JSON"); + }, + }); + } + return Promise.resolve({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + }); + + // Should return successful lists (excluding the invalid JSON one) + expect(result.length).toBeGreaterThan(0); + expect(result).not.toContain(null); + + // Verify that captureException was called for JSON parsing error + expect(captureException).toHaveBeenCalledWith( + expect.stringContaining("Failed to parse asset list JSON"), + ); + }); + + it("should return all successful lists when all requests succeed", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "mainnet", + assets: [], + }; + + // Mock all fetches to succeed + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + + const result = await getCombinedAssetListData({ + networkDetails: MAINNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + }); + + // Should return all enabled lists + const enabledLists = DEFAULT_ASSETS_LISTS[NETWORKS.PUBLIC].filter( + (list: AssetsListItem) => list.isEnabled, + ); + expect(result.length).toBe(enabledLists.length); + expect(captureException).not.toHaveBeenCalled(); + }); + + it("should handle testnet network correctly", async () => { + const mockSuccessResponse = { + name: "Test Asset List", + provider: "Test Provider", + description: "Test Description", + version: "1.0.0", + network: "testnet", + assets: [], + }; + + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + json: async () => mockSuccessResponse, + }); + + const result = await getCombinedAssetListData({ + networkDetails: TESTNET_NETWORK_DETAILS, + assetsLists: DEFAULT_ASSETS_LISTS, + }); + + expect(result.length).toBeGreaterThan(0); + }); +}); diff --git a/@shared/api/helpers/getIconFromTokenList.ts b/@shared/api/helpers/getIconFromTokenList.ts index 694b315d84..ae97610db8 100644 --- a/@shared/api/helpers/getIconFromTokenList.ts +++ b/@shared/api/helpers/getIconFromTokenList.ts @@ -2,7 +2,6 @@ import { AssetListReponseItem, AssetListResponse, } from "@shared/constants/soroban/asset-list"; -import { NetworkDetails } from "@shared/constants/stellar"; import { getCanonicalFromAsset } from "@shared/helpers/stellar"; import { sendMessageToBackground } from "./extensionMessaging"; @@ -14,7 +13,6 @@ export const getIconFromTokenLists = async ({ code, assetsListsData, }: { - networkDetails: NetworkDetails; issuerId?: string; contractId?: string; code: string; diff --git a/@shared/api/helpers/token-list.ts b/@shared/api/helpers/token-list.ts index cc87ce6d78..259605e861 100644 --- a/@shared/api/helpers/token-list.ts +++ b/@shared/api/helpers/token-list.ts @@ -53,10 +53,17 @@ export const schemaValidatedAssetList = async ( export const getCombinedAssetListData = async ({ networkDetails, assetsLists, + cachedAssetLists, }: { networkDetails: NetworkDetails; assetsLists: AssetsLists; + cachedAssetLists?: AssetListResponse[]; }) => { + // If cached asset lists are provided and not empty, use them instead of fetching + if (cachedAssetLists?.length) { + return cachedAssetLists; + } + let network = networkDetails.network; if (network === CUSTOM_NETWORK) { @@ -86,9 +93,23 @@ export const getCombinedAssetListData = async ({ res = await fetch(url); } catch (e) { captureException(`Failed to load asset list: ${url}`); + return null; + } + + if (!res || !res.ok) { + const statusText = res?.status ? ` (${res.status})` : ""; + captureException(`Failed to load asset list: ${url}${statusText}`); + return null; } - return res?.json(); + try { + return await res.json(); + } catch (e) { + captureException( + `Failed to parse asset list JSON: ${url} - ${JSON.stringify(e)}`, + ); + return null; + } }; promiseArr.push(fetchAndParse()); @@ -96,11 +117,11 @@ export const getCombinedAssetListData = async ({ } const promiseRes = - await Promise.allSettled>(promiseArr); + await Promise.allSettled>(promiseArr); const assetListsData = []; for (const p of promiseRes) { - if (p.status === "fulfilled") { + if (p.status === "fulfilled" && p.value) { assetListsData.push(p.value); } } diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index 40441f5c89..c3fd0f775b 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -1129,7 +1129,6 @@ export const getAssetIcons = async ({ if (!icon) { // if we don't have the icon, we try to get it from the token lists const tokenListIcon = await getIconFromTokenLists({ - networkDetails, issuerId: key, contractId, code, diff --git a/extension/package.json b/extension/package.json index 59b667e659..9e50bfba07 100644 --- a/extension/package.json +++ b/extension/package.json @@ -37,7 +37,6 @@ "@types/react-redux": "7.1.34", "@types/react-router-dom": "5.3.3", "@types/redux": "3.6.31", - "@types/testing-library__jest-dom": "6.0.0", "@types/webextension-polyfill": "0.12.0", "@types/yup": "0.32.0", "bignumber.js": "9.3.0", diff --git a/extension/src/popup/components/manageAssets/AddAsset/index.tsx b/extension/src/popup/components/manageAssets/AddAsset/index.tsx index 5b293c7e0b..aef4b2fc6f 100644 --- a/extension/src/popup/components/manageAssets/AddAsset/index.tsx +++ b/extension/src/popup/components/manageAssets/AddAsset/index.tsx @@ -11,6 +11,7 @@ import { isSacContractExecutable } from "@shared/helpers/soroban/token"; import { FormRows } from "popup/basics/Forms"; import { settingsSelector } from "popup/ducks/settings"; +import { tokensListsSelector } from "popup/ducks/cache"; import { getNativeContractDetails } from "popup/helpers/searchAsset"; import { getAssetListsForAsset, @@ -68,6 +69,7 @@ export const AddAsset = () => { useState(false); const [verifiedLists, setVerifiedLists] = useState([] as string[]); const { assetsLists } = useSelector(settingsSelector); + const cachedTokenLists = useSelector(tokensListsSelector); const navigate = useNavigate(); const { state, fetchData } = useGetAddAssetData({ @@ -167,6 +169,7 @@ export const AddAsset = () => { networkDetails, assets: [token], assetsListsDetails: assetsLists, + cachedAssetLists: cachedTokenLists, }); setVerifiedAssetRows(verifiedAssets); setUnverifiedAssetRows(unverifiedAssets); @@ -175,6 +178,7 @@ export const AddAsset = () => { asset: token, assetsListsDetails: assetsLists, networkDetails, + cachedAssetLists: cachedTokenLists, }); setVerifiedLists(assetListsForToken); if (assetListsForToken.length) { @@ -249,6 +253,7 @@ export const AddAsset = () => { networkDetails, assets: scannedAssetRows, assetsListsDetails: assetsLists, + cachedAssetLists: cachedTokenLists, }); setVerifiedAssetRows(verifiedAssets); setUnverifiedAssetRows(unverifiedAssets); diff --git a/extension/src/popup/components/manageAssets/SearchAsset/hooks/useAssetLookup.ts b/extension/src/popup/components/manageAssets/SearchAsset/hooks/useAssetLookup.ts index 59810ce0f0..1e5008cdaa 100644 --- a/extension/src/popup/components/manageAssets/SearchAsset/hooks/useAssetLookup.ts +++ b/extension/src/popup/components/manageAssets/SearchAsset/hooks/useAssetLookup.ts @@ -1,5 +1,5 @@ import { useReducer, useRef } from "react"; -import { useSelector } from "react-redux"; +import { useDispatch, useSelector } from "react-redux"; import { captureException } from "@sentry/browser"; import { getTokenDetails } from "@shared/api/internal"; @@ -7,9 +7,9 @@ import { isSacContractExecutable } from "@shared/helpers/soroban/token"; import { INDEXER_URL } from "@shared/constants/mercury"; import { BlockAidScanAssetResult } from "@shared/api/types"; import { - getVerifiedTokens, getNativeContractDetails, searchAsset, + getVerifiedTokens, VerifiedTokenRecord, } from "popup/helpers/searchAsset"; import { splitVerifiedAssetCurrency } from "popup/helpers/assetList"; @@ -22,6 +22,9 @@ import { ManageAssetCurrency } from "popup/components/manageAssets/ManageAssetRo import { NetworkDetails } from "@shared/constants/stellar"; import { getCombinedAssetListData } from "@shared/api/helpers/token-list"; import { getIconFromTokenLists } from "@shared/api/helpers/getIconFromTokenList"; +import { tokensListsSelector, saveTokenLists } from "popup/ducks/cache"; +import { AssetListResponse } from "@shared/constants/soroban/asset-list"; +import { AppDispatch, store } from "popup/App"; interface AssetRecord { asset: string; @@ -55,6 +58,7 @@ const useAssetLookup = () => { let verifiedLists = [] as string[]; const abortControllerRef = useRef(null); + const reduxDispatch = useDispatch(); const { assetsLists } = useSelector(settingsSelector); const MAX_ASSETS_TO_SCAN = 10; @@ -139,6 +143,7 @@ const useAssetLookup = () => { contractId: string, isAllowListVerificationEnabled: boolean, networkDetails: NetworkDetails, + assetsListsData: AssetListResponse[], signal: AbortSignal, ) => { let assetRows = [] as ManageAssetCurrency[]; @@ -192,8 +197,9 @@ const useAssetLookup = () => { signal, ).catch(() => { if (signal.aborted) { - return undefined; + return {} as BlockAidScanAssetResult; } + return {} as BlockAidScanAssetResult; }); assetRows = [ { @@ -210,10 +216,12 @@ const useAssetLookup = () => { // If we're using asset lists, we can retrieve asset info from the list if (isAllowListVerificationEnabled) { + // getVerifiedTokens will use cached data if provided, avoiding re-fetching verifiedTokens = await getVerifiedTokens({ networkDetails, contractId, assetsLists, + cachedAssetLists: assetsListsData, }); try { @@ -268,8 +276,9 @@ const useAssetLookup = () => { signal, }).catch(() => { if (signal.aborted) { - return; + return { _embedded: { records: [] } }; } + throw new Error("Failed to fetch Stellar Expert data"); }); return resJson._embedded.records.map((record: AssetRecord) => ({ @@ -322,6 +331,29 @@ const useAssetLookup = () => { let assetRows: ManageAssetCurrency[] = []; const blockaidScanResults: { [key: string]: BlockAidScanAssetResult } = {}; + // Use cached data if available, otherwise fetch once and reuse + // Read cache directly from store to get the latest value + const currentCache = tokensListsSelector(store.getState()); + let assetsListsData: AssetListResponse[] = []; + + // Check cache first - use it if available + if (currentCache?.length) { + // Use cached data - no network calls + assetsListsData = currentCache; + } else { + // Fetch only if cache is empty - fetch sequentially + assetsListsData = await getCombinedAssetListData({ + networkDetails, + assetsLists, + cachedAssetLists: [], // Pass empty array to indicate no cache + }); + + // Save to cache for future use - this will persist for the session + if (assetsListsData.length > 0) { + reduxDispatch(saveTokenLists(assetsListsData)); + } + } + if (isContractId(asset)) { // for custom tokens, try to go down the the custom token flow try { @@ -330,6 +362,7 @@ const useAssetLookup = () => { asset, isAllowListVerificationEnabled, networkDetails, + assetsListsData, signal, ); } catch (e) { @@ -358,17 +391,11 @@ const useAssetLookup = () => { } } - const assetsListsData = await getCombinedAssetListData({ - networkDetails, - assetsLists, - }); - for (const assetRow of assetRows) { const key = assetRow.issuer!; const code = assetRow.code!; if (!assetRow.image) { const tokenListIcon = await getIconFromTokenLists({ - networkDetails, issuerId: key, contractId: assetRow.contract, code, @@ -385,6 +412,7 @@ const useAssetLookup = () => { networkDetails, assets: assetRows, assetsListsDetails: assetsLists, + cachedAssetLists: assetsListsData, // Use the fetched/cached data we already have }); const payload = { diff --git a/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js b/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js index 2121979a38..49ffe0d371 100644 --- a/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js +++ b/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js @@ -25,7 +25,6 @@ jest describe("getIconFromTokenLists", () => { it("should return an icon if an asset is in a token list by contract ID", async () => { const { icon, canonicalAsset } = await getIconFromTokenLists({ - networkDetails: TESTNET_NETWORK_DETAILS, contractId: VERIFIED_TOKEN_CONTRACT, code: VERIFIED_TOKEN_CODE, assetsListsData: [validAssetList], @@ -37,7 +36,6 @@ describe("getIconFromTokenLists", () => { }); it("should return an icon if an asset is in a token list by issuer", async () => { const { icon, canonicalAsset } = await getIconFromTokenLists({ - networkDetails: TESTNET_NETWORK_DETAILS, issuerId: VERIFIED_TOKEN_ISSUER, code: VERIFIED_TOKEN_CODE, assetsListsData: [validAssetList], @@ -49,7 +47,6 @@ describe("getIconFromTokenLists", () => { }); it("should return undefined if an asset is not on the token list", async () => { const { icon, canonicalAsset } = await getIconFromTokenLists({ - networkDetails: TESTNET_NETWORK_DETAILS, issuerId: TEST_PUBLIC_KEY, code: VERIFIED_TOKEN_CODE, assetsListsData: [validAssetList], diff --git a/extension/src/popup/helpers/__tests__/searchAsset.test.js b/extension/src/popup/helpers/__tests__/searchAsset.test.js index 0fca20de74..75c9d4dffb 100644 --- a/extension/src/popup/helpers/__tests__/searchAsset.test.js +++ b/extension/src/popup/helpers/__tests__/searchAsset.test.js @@ -2,6 +2,40 @@ import { validAssetList } from "popup/__testHelpers__"; import * as SearchAsset from "../searchAsset"; import { schemaValidatedAssetList } from "@shared/api/helpers/token-list"; +/** + * SEP-0042 Asset List JSON Schema + * This schema is used for testing schema validation in tests. + * @see https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0042.md + */ +const SEP_0042_ASSET_LIST_SCHEMA = { + type: "object", + properties: { + name: { type: "string" }, + provider: { type: "string" }, + description: { type: "string" }, + version: { type: "string" }, + network: { type: "string" }, + assets: { + type: "array", + items: { + type: "object", + properties: { + code: { type: "string" }, + issuer: { type: "string" }, + contract: { type: "string" }, + name: { type: "string" }, + org: { type: "string" }, + domain: { type: "string" }, + icon: { type: "string" }, + decimals: { type: "number" }, + }, + required: ["code", "issuer", "contract", "domain", "icon", "decimals"], + }, + }, + }, + required: ["name", "provider", "description", "version", "network", "assets"], +}; + describe("searchAsset", () => { beforeEach(() => { jest.restoreAllMocks(); @@ -47,6 +81,17 @@ describe("searchAsset", () => { }); }); it("schemaValidatedAssetList should return list if valid", async () => { + // Mock the schema fetch + jest.spyOn(global, "fetch").mockImplementation((url) => { + if (url.includes("assetlist.schema.json")) { + return Promise.resolve({ + ok: true, + json: async () => SEP_0042_ASSET_LIST_SCHEMA, + }); + } + return Promise.reject(new Error("Unexpected fetch")); + }); + const { assets } = await schemaValidatedAssetList(validAssetList); expect(assets).toStrictEqual(validAssetList.assets); }); @@ -60,10 +105,23 @@ describe("searchAsset", () => { expect(assets).toStrictEqual([]); }); it("schemaValidatedAssetList should return empty list and errors if validation fails", async () => { + // Mock the schema fetch + jest.spyOn(global, "fetch").mockImplementation((url) => { + if (url.includes("assetlist.schema.json")) { + return Promise.resolve({ + ok: true, + json: async () => ({ + ...SEP_0042_ASSET_LIST_SCHEMA, + additionalProperties: false, + }), + }); + } + return Promise.reject(new Error("Unexpected fetch")); + }); + const { assets, errors } = await schemaValidatedAssetList({ // incorrect key title: "PiyalBasu Top 50", - provider: "PiyalBasu", description: "Test asset list schema", version: "1.0", @@ -84,7 +142,7 @@ describe("searchAsset", () => { }); expect(assets).toStrictEqual([]); - // error for missing `name` and error for additional key `title` - expect(errors).toHaveLength(2); + // error for missing `name` and error for additional key `title` and error for 'feedback' + expect(errors).toHaveLength(3); }); }); diff --git a/extension/src/popup/helpers/assetList.ts b/extension/src/popup/helpers/assetList.ts index 1dd396ae5a..4d4530e64a 100644 --- a/extension/src/popup/helpers/assetList.ts +++ b/extension/src/popup/helpers/assetList.ts @@ -2,6 +2,7 @@ import { NetworkDetails } from "@shared/constants/stellar"; import { AssetsLists, AssetListReponseItem, + AssetListResponse, } from "@shared/constants/soroban/asset-list"; import { getNativeContractDetails } from "popup/helpers/searchAsset"; @@ -13,14 +14,17 @@ export const splitVerifiedAssetCurrency = async ({ networkDetails, assets, assetsListsDetails, + cachedAssetLists, }: { networkDetails: NetworkDetails; assets: ManageAssetCurrency[]; assetsListsDetails: AssetsLists; + cachedAssetLists?: AssetListResponse[]; }) => { const settledResponses = await getAssetLists({ assetsListsDetails, networkDetails, + cachedAssetLists, }); const nativeContractDetails = getNativeContractDetails(networkDetails); @@ -71,18 +75,23 @@ export const splitVerifiedAssetCurrency = async ({ }; }; +interface GetAssetListsForAssetParams { + asset: ManageAssetCurrency; + assetsListsDetails: AssetsLists; + networkDetails: NetworkDetails; + cachedAssetLists?: AssetListResponse[]; +} + export const getAssetListsForAsset = async ({ asset, assetsListsDetails, networkDetails, -}: { - asset: ManageAssetCurrency; - assetsListsDetails: AssetsLists; - networkDetails: NetworkDetails; -}) => { + cachedAssetLists, +}: GetAssetListsForAssetParams) => { const settledResponses = await getAssetLists({ assetsListsDetails, networkDetails, + cachedAssetLists, }); const validatedAssets = {} as Record; diff --git a/extension/src/popup/helpers/searchAsset.ts b/extension/src/popup/helpers/searchAsset.ts index d6c21be10b..725ac55306 100644 --- a/extension/src/popup/helpers/searchAsset.ts +++ b/extension/src/popup/helpers/searchAsset.ts @@ -16,7 +16,7 @@ export const searchAsset = async ({ }: { asset: any; networkDetails: NetworkDetails; - signal: AbortSignal; + signal?: AbortSignal; }) => { const res = await fetch( `${getApiStellarExpertUrl(networkDetails)}/asset?search=${asset}`, @@ -58,33 +58,44 @@ export type VerifiedTokenRecord = AssetListReponseItem & { export const getAssetLists = async ({ assetsListsDetails, networkDetails, + cachedAssetLists, }: { assetsListsDetails: AssetsLists; networkDetails: NetworkDetails; + cachedAssetLists?: AssetListResponse[]; }) => { + // If cached asset lists are provided and not empty, use them instead of fetching + if (cachedAssetLists?.length) { + // Convert cached data to the expected Promise.allSettled format + return cachedAssetLists.map((assetList) => ({ + status: "fulfilled" as const, + value: assetList, + })); + } + const network = networkDetails.network; const assetsListsDetailsByNetwork = assetsListsDetails[network as AssetsListKey]; - const assetListsResponses = [] as AssetListResponse[]; - for (const networkList of assetsListsDetailsByNetwork) { - const { url, isEnabled } = networkList; + const promiseArr = []; + for (const { url, isEnabled } of assetsListsDetailsByNetwork) { + if (!isEnabled) continue; - if (isEnabled) { - const fetchAndParse = async (): Promise => { - const res = await fetch(url); - if (!res.ok) { - throw new Error(res.statusText); - } - return res.json(); - }; + const fetchAndParse = async (): Promise => { + const res = await fetch(url); + if (!res.ok) { + throw new Error(res.statusText); + } + return res.json(); + }; - assetListsResponses.push(await fetchAndParse()); - } + promiseArr.push(fetchAndParse()); } - const settledResponses = await Promise.allSettled(assetListsResponses); - return settledResponses; + const promiseRes = + await Promise.allSettled>(promiseArr); + + return promiseRes; }; export const getVerifiedTokens = async ({ @@ -92,15 +103,18 @@ export const getVerifiedTokens = async ({ contractId, setIsSearching, assetsLists, + cachedAssetLists, }: { networkDetails: NetworkDetails; contractId: string; setIsSearching?: (isSearching: boolean) => void; assetsLists: AssetsLists; + cachedAssetLists?: AssetListResponse[]; }) => { - const assetListsData = await getCombinedAssetListData({ + const assetListsData: AssetListResponse[] = await getCombinedAssetListData({ networkDetails, assetsLists, + cachedAssetLists, }); const nativeContract = getNativeContractDetails(networkDetails); diff --git a/extension/src/popup/helpers/useTokenLookup.ts b/extension/src/popup/helpers/useTokenLookup.ts index e0814937f4..1732342523 100644 --- a/extension/src/popup/helpers/useTokenLookup.ts +++ b/extension/src/popup/helpers/useTokenLookup.ts @@ -19,6 +19,7 @@ import { } from "popup/helpers/searchAsset"; import { isAssetSuspicious, scanAsset } from "popup/helpers/blockaid"; import { ManageAssetCurrency } from "popup/components/manageAssets/ManageAssetRows"; +import { tokensListsSelector } from "popup/ducks/cache"; export const useTokenLookup = ({ setAssetRows, @@ -38,6 +39,7 @@ export const useTokenLookup = ({ const publicKey = useSelector(publicKeySelector); const networkDetails = useSelector(settingsNetworkDetailsSelector); const { assetsLists } = useSelector(settingsSelector); + const cachedTokenLists = useSelector(tokensListsSelector); const isAllowListVerificationEnabled = isMainnet(networkDetails) || isTestnet(networkDetails); @@ -119,6 +121,7 @@ export const useTokenLookup = ({ networkDetails, contractId, assetsLists, + cachedAssetLists: cachedTokenLists, }); try { @@ -154,6 +157,7 @@ export const useTokenLookup = ({ }, [ assetsLists, + cachedTokenLists, isAllowListVerificationEnabled, networkDetails, publicKey, diff --git a/extension/src/popup/views/SignTransaction/hooks/__tests__/useGetSignTxData.test.tsx b/extension/src/popup/views/SignTransaction/hooks/__tests__/useGetSignTxData.test.tsx index 7c5828f023..a4262ea5c4 100644 --- a/extension/src/popup/views/SignTransaction/hooks/__tests__/useGetSignTxData.test.tsx +++ b/extension/src/popup/views/SignTransaction/hooks/__tests__/useGetSignTxData.test.tsx @@ -55,9 +55,16 @@ jest .spyOn(AccountHelpers, "signFlowAccountSelector") .mockReturnValue(mockAccounts[0]); -const GetCombinedAssetListDataSpy = jest - .spyOn(TokenListHelpers, "getCombinedAssetListData") - .mockResolvedValue([]); +jest.spyOn(TokenListHelpers, "getCombinedAssetListData").mockResolvedValue([ + { + name: "Test Asset List", + description: "Test description", + network: "testnet", + version: "1.0.0", + provider: "test", + assets: [], + }, +]); jest.mock("react-router-dom", () => ({ ...jest.requireActual("react-router-dom"), @@ -80,7 +87,16 @@ describe("useGetSignTxData", () => { balances: mockBalances.balances, }; - const tokenListData = [{ id: "example-token-list" }]; + const tokenListData = [ + { + name: "Example Asset List", + description: "Example description", + network: "testnet", + version: "1.0.0", + provider: "example", + assets: [], + }, + ]; const canonicalKey = "XLM:GABCDEF"; const cachedIcons = { [canonicalKey]: "https://cached/icon/url.png", @@ -176,7 +192,7 @@ describe("useGetSignTxData", () => { await result.current.fetchData(); }); - expect(GetCombinedAssetListDataSpy).toHaveBeenCalled(); + // Verify that asset lists data was used (icons populated indicates asset lists data length > 0) // @ts-ignore expect(result.current.state.data?.icons).toEqual({ [TEST_CANONICAL]: "https://example.com/icon.png", @@ -219,7 +235,7 @@ describe("useGetSignTxData", () => { await result.current.fetchData(); }); - expect(GetCombinedAssetListDataSpy).not.toHaveBeenCalled(); + // Verify that asset lists data was not used (no icons indicates asset lists data length = 0 or not fetched) // @ts-ignore expect(result.current.state.data?.icons).toEqual({}); // @ts-ignore @@ -255,11 +271,22 @@ describe("useGetSignTxData", () => { { wrapper: Wrapper(store) }, ); + jest + .spyOn(GetIconUrlFromIssuerHelpers, "getIconUrlFromIssuer") + .mockResolvedValue(""); + + jest + .spyOn(GetIconFromTokenListHelpers, "getIconFromTokenLists") + .mockResolvedValue({ + icon: "https://example.com/icon.png", + canonicalAsset: TEST_CANONICAL, + }); + await act(async () => { await result.current.fetchData(); }); - expect(GetCombinedAssetListDataSpy).toHaveBeenCalledTimes(1); + // Verify that asset lists data was used (icons populated indicates asset lists data length > 0) // @ts-ignore expect(result.current.state.data?.icons).toEqual({ [TEST_CANONICAL]: "https://example.com/icon.png", diff --git a/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx b/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx index 69ea71a399..c19675fcd4 100644 --- a/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx +++ b/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx @@ -16,7 +16,7 @@ import { makeAccountActive } from "popup/ducks/accountServices"; import { useDispatch, useSelector } from "react-redux"; import { AppDispatch } from "popup/App"; import { signFlowAccountSelector } from "popup/helpers/account"; -import { iconsSelector } from "popup/ducks/cache"; +import { iconsSelector, tokensListsSelector } from "popup/ducks/cache"; import { getIconUrlFromIssuer } from "@shared/api/helpers/getIconUrlFromIssuer"; import { getIconFromTokenLists } from "@shared/api/helpers/getIconFromTokenList"; import { isContractId } from "popup/helpers/soroban"; @@ -62,6 +62,7 @@ function useGetSignTxData( const { fetchData: fetchAppData } = useGetAppData(); const { fetchData: fetchBalances } = useGetBalances(balanceOptions); const cachedIcons = useSelector(iconsSelector); + const cachedTokenLists = useSelector(tokensListsSelector); const { assetsLists } = useSelector(settingsSelector); const { scanTx } = useScanTx(); const [accountNotFound, setAccountNotFound] = useState(false); @@ -130,10 +131,14 @@ function useGetSignTxData( dispatch({ type: "FETCH_DATA_SUCCESS", payload: firstRenderPayload }); // Add all icons needed for tx assets - const icons = {} as { [code: string]: string | null }; - let assetsListsData: AssetListResponse[] = []; + const icons = {} as { [code: string]: string }; + // Initialize with cached lists, but we'll fetch fresh data when needed for icon lookups + let assetsListsData: AssetListResponse[] = cachedTokenLists || []; + let hasFetchedAssetLists = assetsListsData.length > 0; + // Fetch icons for asset diffs only if includeIcons is true if ( + balanceOptions.includeIcons && scanResult && "simulation" in scanResult && scanResult.simulation && @@ -152,8 +157,9 @@ function useGetSignTxData( const key = diff.asset.issuer; const code = diff.asset.code; let canonical = getCanonicalFromAsset(code, key); - if (cachedIcons[canonical]) { - icons[canonical] = cachedIcons[canonical]; + const cachedIcon = cachedIcons[canonical]; + if (cachedIcon) { + icons[canonical] = cachedIcon; } else { let icon = await getIconUrlFromIssuer({ key, @@ -161,14 +167,15 @@ function useGetSignTxData( networkDetails, }); if (!icon) { - if (!assetsListsData.length) { + if (!hasFetchedAssetLists) { assetsListsData = await getCombinedAssetListData({ networkDetails, assetsLists, + cachedAssetLists: cachedTokenLists, }); + hasFetchedAssetLists = true; } const tokenListIcon = await getIconFromTokenLists({ - networkDetails, issuerId: key, contractId: isContractId(key) ? key : undefined, code, @@ -179,12 +186,15 @@ function useGetSignTxData( canonical = tokenListIcon.canonicalAsset; } } - icons[canonical] = icon; + if (icon) { + icons[canonical] = icon; + } } } } } + // Always fetch icons for changeTrust operations (regardless of includeIcons flag) const transaction = TransactionBuilder.fromXDR( scanOptions.xdr, networkDetails.networkPassphrase, @@ -197,8 +207,9 @@ function useGetSignTxData( if ("code" in trustChange.line) { const { code, issuer } = trustChange.line; let canonical = getCanonicalFromAsset(code, issuer); - if (cachedIcons[canonical]) { - icons[canonical] = cachedIcons[canonical]; + const cachedIcon = cachedIcons[canonical]; + if (cachedIcon) { + icons[canonical] = cachedIcon; } else { let icon = await getIconUrlFromIssuer({ key: issuer, @@ -206,14 +217,15 @@ function useGetSignTxData( networkDetails, }); if (!icon) { - if (!assetsListsData.length) { + if (!hasFetchedAssetLists) { assetsListsData = await getCombinedAssetListData({ networkDetails, assetsLists, + cachedAssetLists: cachedTokenLists, }); + hasFetchedAssetLists = true; } const tokenListIcon = await getIconFromTokenLists({ - networkDetails, issuerId: issuer, contractId: isContractId(issuer) ? issuer : undefined, code, @@ -224,7 +236,9 @@ function useGetSignTxData( canonical = tokenListIcon.canonicalAsset; } } - icons[canonical] = icon; + if (icon) { + icons[canonical] = icon; + } } } } diff --git a/extension/src/popup/views/Wallets/index.tsx b/extension/src/popup/views/Wallets/index.tsx index 00907e0d13..c0ef30dd98 100644 --- a/extension/src/popup/views/Wallets/index.tsx +++ b/extension/src/popup/views/Wallets/index.tsx @@ -402,7 +402,6 @@ export const Wallets = () => { return ( <> { contractId: undefined, code: "FOO", assetsListsData: assetsListsData, - networkDetails: TESTNET_NETWORK_DETAILS, }); expect(getIconFromTokenListSpy).toHaveBeenCalledTimes(2); diff --git a/yarn.lock b/yarn.lock index 06df198f42..72f7244c56 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5164,20 +5164,6 @@ __metadata: languageName: node linkType: hard -"@testing-library/jest-dom@npm:*": - version: 6.8.0 - resolution: "@testing-library/jest-dom@npm:6.8.0" - dependencies: - "@adobe/css-tools": "npm:^4.4.0" - aria-query: "npm:^5.0.0" - css.escape: "npm:^1.5.1" - dom-accessibility-api: "npm:^0.6.3" - picocolors: "npm:^1.1.1" - redent: "npm:^3.0.0" - checksum: 10c0/4c5b8b433e0339e0399b940ae901a99ae00f1d5ffb7cbb295460b2c44aaad0bc7befcca7b06ceed7aa68a524970077468046c9fe52836ee26f45b807c80a7ff1 - languageName: node - linkType: hard - "@testing-library/jest-dom@npm:6.6.3": version: 6.6.3 resolution: "@testing-library/jest-dom@npm:6.6.3" @@ -5985,15 +5971,6 @@ __metadata: languageName: node linkType: hard -"@types/testing-library__jest-dom@npm:6.0.0": - version: 6.0.0 - resolution: "@types/testing-library__jest-dom@npm:6.0.0" - dependencies: - "@testing-library/jest-dom": "npm:*" - checksum: 10c0/824950dc82752ddb656fa7ca851bf454804332f7a7a8571231abfc3553902198c6b96de209bb3dd1f1a15ee6a269a1efa56866041f5692ee0129308772e658e0 - languageName: node - linkType: hard - "@types/tough-cookie@npm:*": version: 4.0.5 resolution: "@types/tough-cookie@npm:4.0.5" @@ -10690,7 +10667,6 @@ __metadata: "@types/react-router-dom": "npm:5.3.3" "@types/redux": "npm:3.6.31" "@types/semver": "npm:7.5.8" - "@types/testing-library__jest-dom": "npm:6.0.0" "@types/webextension-polyfill": "npm:0.12.0" "@types/yup": "npm:0.32.0" bignumber.js: "npm:9.3.0"