diff --git a/@shared/api/helpers/getIconFromTokenList.ts b/@shared/api/helpers/getIconFromTokenList.ts index 97a9f6b644..694b315d84 100644 --- a/@shared/api/helpers/getIconFromTokenList.ts +++ b/@shared/api/helpers/getIconFromTokenList.ts @@ -5,6 +5,9 @@ import { import { NetworkDetails } from "@shared/constants/stellar"; import { getCanonicalFromAsset } from "@shared/helpers/stellar"; +import { sendMessageToBackground } from "./extensionMessaging"; +import { SERVICE_TYPES } from "../../constants/services"; + export const getIconFromTokenLists = async ({ issuerId, contractId, @@ -36,6 +39,7 @@ export const getIconFromTokenLists = async ({ issuerId && record.issuer && record.issuer === issuerId && + record.code === code && record.icon ) { verifiedToken = record; @@ -46,6 +50,15 @@ export const getIconFromTokenLists = async ({ } } + if (verifiedToken?.icon) { + await sendMessageToBackground({ + activePublicKey: null, + assetCanonical: `${code}:${contractId || issuerId}`, + iconUrl: verifiedToken?.icon, + type: SERVICE_TYPES.CACHE_ASSET_ICON, + }); + } + return { icon: verifiedToken?.icon, canonicalAsset, diff --git a/@shared/api/helpers/getIconUrlFromIssuer.ts b/@shared/api/helpers/getIconUrlFromIssuer.ts index e46965660a..57eeaa8d63 100644 --- a/@shared/api/helpers/getIconUrlFromIssuer.ts +++ b/@shared/api/helpers/getIconUrlFromIssuer.ts @@ -84,7 +84,8 @@ export const getIconUrlFromIssuer = async ({ if (toml.CURRENCIES) { /* If we find some currencies listed, check to see if they have the currency we're looking for listed */ - toml.CURRENCIES.every(async ({ code: currencyCode, issuer, image }) => { + for (const currency of toml.CURRENCIES) { + const { code: currencyCode, issuer, image } = currency; if (currencyCode === code && issuer === key && image) { /* We found the currency listing in the toml. 3. Get the image url from it */ iconUrl = image; @@ -95,11 +96,11 @@ export const getIconUrlFromIssuer = async ({ iconUrl, type: SERVICE_TYPES.CACHE_ASSET_ICON, }); - return false; + break; } - return true; - }); + } } + /* Return the icon url to the UI, if we found it */ return iconUrl; }; diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index 4e25d74134..ef42005b0e 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -948,7 +948,10 @@ export const getAccountBalances = async ( isMainnet, }); } - return await getAccountIndexerBalances({ publicKey, networkDetails }); + return await getAccountIndexerBalances({ + publicKey, + networkDetails, + }); }; export const getTokenDetails = async ({ @@ -1034,6 +1037,35 @@ export const getTokenDetails = async ({ } }; +export const getAssetIconCache = async ({ + activePublicKey, +}: { + activePublicKey: string | null; +}) => { + let icons = {}; + try { + ({ icons } = await sendMessageToBackground({ + activePublicKey, + type: SERVICE_TYPES.GET_CACHED_ASSET_ICON_LIST, + })); + } catch (e) { + return { icons }; + } + return { icons }; +}; + +/* + getAssetIcons is used to get the icons for the assets in the balances. + It can be used with or without lookup. Lookup is keyed off of passing assetListsData and networkDetails. + + Using this method without lookup is fastest as it just consults the cache and then returns the icons. + This is useful for quickly getting the icons without extra network calls. + + Using this method with lookup is slower as it needs to fetch the token lists and the issuer. + This is useful for getting the icons for the first time. Once we've retrieved an icon, + we'll store it in the background's storage and then use this cache for future lookups. +*/ + export const getAssetIcons = async ({ balances, networkDetails, @@ -1041,17 +1073,18 @@ export const getAssetIcons = async ({ cachedIcons, }: { balances: Balances; - networkDetails: NetworkDetails; - assetsListsData: AssetListResponse[]; - cachedIcons: Record; + networkDetails?: NetworkDetails; + assetsListsData?: AssetListResponse[]; + cachedIcons: Record; }) => { - const assetIcons = {} as { [code: string]: string }; + const assetIcons = {} as { [code: string]: string | null }; + const skipLookup = !assetsListsData || !networkDetails; if (balances) { - let icon = ""; const balanceValues = Object.values(balances); for (let i = 0; i < balanceValues.length; i++) { + let icon = ""; const { token, contractId } = balanceValues[i]; if (token && "issuer" in token) { const { @@ -1066,8 +1099,19 @@ export const getAssetIcons = async ({ continue; } - icon = await getIconUrlFromIssuer({ key, code, networkDetails }); + if (cachedIcon === null) { + // if we've tried to fetch this icon before and it wasn't found, we marked it as null + // don't bother trying to fetch it again + // this null value is only stored in Redux, so we will re-try on next app reload + continue; + } + + if (skipLookup) { + continue; + } + 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, @@ -1078,9 +1122,14 @@ export const getAssetIcons = async ({ if (tokenListIcon.icon && tokenListIcon.canonicalAsset) { icon = tokenListIcon.icon; canonical = tokenListIcon.canonicalAsset; + } else { + // if we still don't have the icon, we try to get it from the issuer + icon = await getIconUrlFromIssuer({ key, code, networkDetails }); } } - assetIcons[canonical] = icon; + + // we assign null here if we checked all sources and still don't have the icon + assetIcons[canonical] = icon || null; } } } @@ -1096,7 +1145,7 @@ export const retryAssetIcon = async ({ }: { key: string; code: string; - assetIcons: { [code: string]: string }; + assetIcons: { [code: string]: string | null }; networkDetails: NetworkDetails; activePublicKey: string | null; }) => { diff --git a/@shared/api/types/message-request.ts b/@shared/api/types/message-request.ts index bd82251919..b7524f367b 100644 --- a/@shared/api/types/message-request.ts +++ b/@shared/api/types/message-request.ts @@ -250,6 +250,10 @@ export interface LoadSettingsMessage extends BaseMessage { type: SERVICE_TYPES.LOAD_SETTINGS; } +export interface GetCachedAssetIconListMessage extends BaseMessage { + type: SERVICE_TYPES.GET_CACHED_ASSET_ICON_LIST; +} + export interface GetCachedAssetIconMessage extends BaseMessage { type: SERVICE_TYPES.GET_CACHED_ASSET_ICON; assetCanonical: string; @@ -381,6 +385,7 @@ export type ServiceMessageRequest = | SaveSettingsMessage | SaveExperimentalFeaturesMessage | LoadSettingsMessage + | GetCachedAssetIconListMessage | GetCachedAssetIconMessage | CacheAssetIconMessage | GetCachedDomainMessage diff --git a/@shared/api/types/types.ts b/@shared/api/types/types.ts index 39f90f0098..c85291e243 100644 --- a/@shared/api/types/types.ts +++ b/@shared/api/types/types.ts @@ -84,6 +84,7 @@ export interface Response { accountName: string; assetCode: string; assetCanonical: string; + icons: {}; iconUrl: string; network: string; networkIndex: number; @@ -214,7 +215,7 @@ export type Settings = { } & Preferences; export interface AssetIcons { - [code: string]: string; + [code: string]: string | null; } export interface AssetDomains { diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index 45c741a7b0..5c3082b959 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -31,6 +31,7 @@ export enum SERVICE_TYPES { SAVE_SETTINGS = "SAVE_SETTINGS", SAVE_EXPERIMENTAL_FEATURES = "SAVE_EXPERIMENTAL_FEATURES", LOAD_SETTINGS = "LOAD_SETTINGS", + GET_CACHED_ASSET_ICON_LIST = "GET_CACHED_ASSET_ICON_LIST", GET_CACHED_ASSET_ICON = "GET_CACHED_ASSET_ICON", CACHE_ASSET_ICON = "CACHE_ASSET_ICON", GET_CACHED_ASSET_DOMAIN = "GET_CACHED_ASSET_DOMAIN", diff --git a/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts new file mode 100644 index 0000000000..e27161ffb5 --- /dev/null +++ b/extension/src/background/messageListener/handlers/getCachedAssetIconList.ts @@ -0,0 +1,15 @@ +import { DataStorageAccess } from "background/helpers/dataStorageAccess"; +import { CACHED_ASSET_ICONS_ID } from "constants/localStorageTypes"; + +export const getCachedAssetIconList = async ({ + localStore, +}: { + localStore: DataStorageAccess; +}) => { + const assetIconCache = + (await localStore.getItem(CACHED_ASSET_ICONS_ID)) || {}; + + return { + icons: assetIconCache, + }; +}; diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index 6349b476e0..90d9b455d1 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -55,6 +55,7 @@ import { saveAllowList } from "./handlers/saveAllowList"; import { saveSettings } from "./handlers/saveSettings"; import { saveExperimentalFeatures } from "./handlers/saveExperimentalFeatures"; import { loadSettings } from "./handlers/loadSettings"; +import { getCachedAssetIconList } from "./handlers/getCachedAssetIconList"; import { getCachedAssetIcon } from "./handlers/getCachedAssetIcons"; import { cacheAssetIcon } from "./handlers/cacheAssetIcon"; import { getCachedAssetDomain } from "./handlers/getCachedDomain"; @@ -359,6 +360,11 @@ export const popupMessageListener = ( localStore, }); } + case SERVICE_TYPES.GET_CACHED_ASSET_ICON_LIST: { + return getCachedAssetIconList({ + localStore, + }); + } case SERVICE_TYPES.GET_CACHED_ASSET_ICON: { return getCachedAssetIcon({ request, diff --git a/extension/src/helpers/__tests__/useGetBalances.test.tsx b/extension/src/helpers/__tests__/useGetBalances.test.tsx index f374ec9f79..468dec60c0 100644 --- a/extension/src/helpers/__tests__/useGetBalances.test.tsx +++ b/extension/src/helpers/__tests__/useGetBalances.test.tsx @@ -137,4 +137,38 @@ describe("useGetBalances (cached path)", () => { expect(getCombinedAssetListData).not.toHaveBeenCalled(); expect(result.current.state.state).toBe(RequestState.SUCCESS); }); + + it("skips trying to lookup an icon that was previously not found", async () => { + const preloadedStateWithNullIcon = { + cache: { + balanceData: { + [TESTNET_NETWORK_DETAILS.network]: { [publicKey]: cachedBalanceData }, + }, + icons: { "XLM:GABCDEF": null }, + tokenLists: tokenListData, + }, + settings: { + assetsLists: [], + }, + }; + const iconStore = makeDummyStore(preloadedStateWithNullIcon); + + const { result } = renderHook( + () => useGetBalances({ showHidden: false, includeIcons: true }), + { wrapper: Wrapper(iconStore) }, + ); + + await act(async () => { + await result.current.fetchData( + publicKey, + true, + TESTNET_NETWORK_DETAILS, + true, + ); + }); + + expect(getCombinedAssetListData).not.toHaveBeenCalled(); + expect(getIconUrlFromIssuer).not.toHaveBeenCalled(); + expect(result.current.state.state).toBe(RequestState.SUCCESS); + }); }); diff --git a/extension/src/helpers/hooks/useGetAssetDomainsWithBalances.ts b/extension/src/helpers/hooks/useGetAssetDomainsWithBalances.ts index 81500cd0b3..ab024bf141 100644 --- a/extension/src/helpers/hooks/useGetAssetDomainsWithBalances.ts +++ b/extension/src/helpers/hooks/useGetAssetDomainsWithBalances.ts @@ -135,7 +135,7 @@ export function useGetAssetDomainsWithBalances(getBalancesOptions: { domains.push({ code, issuer: issuer.key, - image: icons[getCanonicalFromAsset(code, issuer.key)], + image: icons[getCanonicalFromAsset(code, issuer.key)] || null, domain, contract: contractId, isSuspicious: isAssetSuspicious(blockaidData), diff --git a/extension/src/helpers/hooks/useGetBalances.tsx b/extension/src/helpers/hooks/useGetBalances.tsx index 5a5451d1ca..5dc6fc43d2 100644 --- a/extension/src/helpers/hooks/useGetBalances.tsx +++ b/extension/src/helpers/hooks/useGetBalances.tsx @@ -5,6 +5,7 @@ import { getAccountBalances, getAssetIcons, getHiddenAssets, + getAssetIconCache, } from "@shared/api/internal"; import { NetworkDetails } from "@shared/constants/stellar"; import { AssetIcons } from "@shared/api/types"; @@ -99,6 +100,14 @@ function useGetBalances(options: { } as AccountBalances; if (options.includeIcons) { + let cachedIconsFromCache = cachedIcons; + if (!Object.keys(cachedIcons).length) { + const backgroundCachedIcons = await getAssetIconCache({ + activePublicKey: publicKey, + }); + + cachedIconsFromCache = { ...backgroundCachedIcons.icons }; + } const assetsListsData = useCache && cachedTokenLists.length ? cachedTokenLists @@ -111,7 +120,7 @@ function useGetBalances(options: { balances: accountBalances.balances, networkDetails, assetsListsData, - cachedIcons, + cachedIcons: cachedIconsFromCache, }); payload.icons = icons; reduxDispatch(saveTokenLists(assetsListsData)); diff --git a/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx b/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx index 03d5fd7ec6..2d8c1044fe 100644 --- a/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx +++ b/extension/src/popup/components/InternalTransaction/ReviewTransaction/index.tsx @@ -33,9 +33,9 @@ import { CopyValue } from "popup/components/CopyValue"; import "./styles.scss"; interface ReviewTxProps { - assetIcon: string; + assetIcon: string | null; dstAsset?: { - icon: string; + icon: string | null; canonical: string; priceUsd: string | null; amount: string; diff --git a/extension/src/popup/components/account/AccountAssets/index.tsx b/extension/src/popup/components/account/AccountAssets/index.tsx index a0482e7c73..ba43ef9edc 100644 --- a/extension/src/popup/components/account/AccountAssets/index.tsx +++ b/extension/src/popup/components/account/AccountAssets/index.tsx @@ -1,8 +1,9 @@ -import React, { useEffect, useState } from "react"; +import React, { useEffect, useState, memo } from "react"; import { useSelector } from "react-redux"; import isEmpty from "lodash/isEmpty"; import { Asset, Horizon } from "stellar-sdk"; import BigNumber from "bignumber.js"; +import isEqual from "lodash/isEqual"; import { ApiTokenPrices, AssetIcons, Balance } from "@shared/api/types"; import { retryAssetIcon } from "@shared/api/internal"; @@ -36,129 +37,141 @@ export const SorobanTokenIcon = ({ noMargin }: { noMargin?: boolean }) => ( ); -export const AssetIcon = ({ - assetIcons, - code, - issuerKey, - retryAssetIconFetch, - isLPShare = false, - isSorobanToken = false, - icon, - isSuspicious = false, - isModal = false, -}: { +interface AssetIconProps { assetIcons: AssetIcons; code: string; issuerKey: string; retryAssetIconFetch?: (arg: { key: string; code: string }) => void; isLPShare?: boolean; isSorobanToken?: boolean; - icon?: string; + icon?: string | null; isSuspicious?: boolean; isModal?: boolean; -}) => { - /* +} + +const shouldAssetIconSkipUpdate = ( + prevProps: AssetIconProps, + nextProps: AssetIconProps, +) => + isEqual(prevProps.assetIcons, nextProps.assetIcons) && + prevProps.isSuspicious === nextProps.isSuspicious; + +export const AssetIcon = memo( + ({ + assetIcons, + code, + issuerKey, + retryAssetIconFetch, + isLPShare = false, + isSorobanToken = false, + icon, + isSuspicious = false, + isModal = false, + }: AssetIconProps) => { + /* We load asset icons in 2 ways: Method 1. We get an asset's issuer and use that to look up toml info to get the icon path Method 2. We get an icon path directly from an API (like in the trustline flow) and just pass it to this component to render */ - const isXlm = getIsXlm(code); + const isXlm = getIsXlm(code); - // in Method 1, while we wait for the icon path to load, `assetIcons` will be empty until the promise resolves - // This does not apply for XLM as there is no lookup as that logo lives in this codebase - const isFetchingAssetIcons = isEmpty(assetIcons) && !isXlm; + // in Method 1, while we wait for the icon path to load, `assetIcons` will be empty until the promise resolves + // This does not apply for XLM as there is no lookup as that logo lives in this codebase + const isFetchingAssetIcons = isEmpty(assetIcons) && !isXlm; - const [hasError, setHasError] = useState(false); + const [hasError, setHasError] = useState(false); - // For all non-XLM assets (assets where we need to fetch the icon from elsewhere), start by showing a loading state as there is work to do - const [isLoading, setIsLoading] = useState(!isXlm); + // For all non-XLM assets (assets where we need to fetch the icon from elsewhere), start by showing a loading state as there is work to do + const [isLoading, setIsLoading] = useState(true); - const { soroswapTokens } = useSelector(transactionSubmissionSelector); + const { soroswapTokens } = useSelector(transactionSubmissionSelector); - const canonicalAsset = assetIcons[getCanonicalFromAsset(code, issuerKey)]; - let imgSrc = hasError ? ImageMissingIcon : canonicalAsset || ""; - if (icon) { - imgSrc = icon; - } + const canonicalAsset = assetIcons[getCanonicalFromAsset(code, issuerKey)]; + let imgSrc = hasError ? ImageMissingIcon : canonicalAsset || ""; + if (icon) { + imgSrc = icon; + } - const _isSorobanToken = !isSorobanToken - ? issuerKey && isSorobanIssuer(issuerKey) - : isSorobanToken; + const _isSorobanToken = !isSorobanToken + ? issuerKey && isSorobanIssuer(issuerKey) + : isSorobanToken; + + // If an LP share return early w/ hardcoded icon + if (isLPShare) { + return ( +
+ LP +
+ ); + } - // If an LP share return early w/ hardcoded icon - if (isLPShare) { - return ( -
- LP -
- ); - } + // Get icons for Soroban tokens which are not present in assetIcons list + if (_isSorobanToken && !icon && !canonicalAsset) { + const soroswapTokenDetail = soroswapTokens.find( + (token) => token.contract === issuerKey, + ); + // check to see if we have an icon from an external service, like Soroswap + if (soroswapTokenDetail?.icon) { + imgSrc = soroswapTokenDetail?.icon; + } else { + return ; + } + } - // Get icons for Soroban tokens which are not present in assetIcons list - if (_isSorobanToken && !icon && !canonicalAsset) { - const soroswapTokenDetail = soroswapTokens.find( - (token) => token.contract === issuerKey, - ); - // check to see if we have an icon from an external service, like Soroswap - if (soroswapTokenDetail?.icon) { - imgSrc = soroswapTokenDetail?.icon; - } else { - return ; + // If we're waiting on the icon lookup (Method 1), just return the loader until this re-renders with `assetIcons`. We can't do anything until we have it. + if (isFetchingAssetIcons) { + return ( +
+ +
+ ); } - } - // If we're waiting on the icon lookup (Method 1), just return the loader until this re-renders with `assetIcons`. We can't do anything until we have it. - if (isFetchingAssetIcons) { - return ( + // if we have an asset path, start loading the path in an `` + return canonicalAsset || isXlm || imgSrc ? ( +
+ {`${code} { + if (retryAssetIconFetch) { + retryAssetIconFetch({ key: issuerKey, code }); + } + // we tried to load an image path but it failed, so show the broken image icon here + setHasError(true); + }} + onLoad={() => { + // we've sucessfully loaded an icon, end the "loading" state + setIsLoading(false); + }} + /> + +
+ ) : ( + // the image path wasn't found, show a default broken image icon
+
); - } - - // if we have an asset path, start loading the path in an `` - return canonicalAsset || isXlm || imgSrc ? ( -
- {`${code} { - if (retryAssetIconFetch) { - retryAssetIconFetch({ key: issuerKey, code }); - } - // we tried to load an image path but it failed, so show the broken image icon here - setHasError(true); - }} - onLoad={() => { - // we've sucessfully loaded an icon, end the "loading" state - setIsLoading(false); - }} - /> - -
- ) : ( - // the image path wasn't found, show a default broken image icon -
- - -
- ); -}; + }, + shouldAssetIconSkipUpdate, +); interface AccountAssetsProps { assetIcons: AssetIcons; diff --git a/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/SubmitTx/index.tsx b/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/SubmitTx/index.tsx index c37eafa002..daabe0b749 100644 --- a/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/SubmitTx/index.tsx +++ b/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/SubmitTx/index.tsx @@ -34,7 +34,7 @@ interface SubmitTransactionProps { asset: { code: string; issuer: string; - image: string; + image: string | null; domain: string; contract?: string; }; diff --git a/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/hooks/useChangeTrustData.tsx b/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/hooks/useChangeTrustData.tsx index 5fece275aa..f4d63fd37a 100644 --- a/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/hooks/useChangeTrustData.tsx +++ b/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/hooks/useChangeTrustData.tsx @@ -36,7 +36,7 @@ function useGetChangeTrustData({ domain: string; contract?: string; }; - assetImage: string; + assetImage: string | null; networkDetails: NetworkDetails; recommendedFee: string; publicKey: string; diff --git a/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/index.tsx b/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/index.tsx index 2727c67b36..a964c50289 100644 --- a/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/index.tsx +++ b/extension/src/popup/components/manageAssets/ManageAssetRows/ChangeTrustInternal/index.tsx @@ -42,7 +42,7 @@ interface ChangeTrustInternalProps { asset: { code: string; issuer: string; - image: string; + image: string | null; domain: string; contract?: string; }; diff --git a/extension/src/popup/components/manageAssets/ManageAssetRows/ToggleTokenInternal/index.tsx b/extension/src/popup/components/manageAssets/ManageAssetRows/ToggleTokenInternal/index.tsx index 28c1f44d6d..3cce062491 100644 --- a/extension/src/popup/components/manageAssets/ManageAssetRows/ToggleTokenInternal/index.tsx +++ b/extension/src/popup/components/manageAssets/ManageAssetRows/ToggleTokenInternal/index.tsx @@ -22,7 +22,7 @@ interface ToggleTokenInternalProps { asset: { code: string; issuer: string; - image: string; + image: string | null; isTrustlineActive: boolean; domain: string; contract?: string; diff --git a/extension/src/popup/components/manageAssets/ManageAssetRows/index.tsx b/extension/src/popup/components/manageAssets/ManageAssetRows/index.tsx index 27dede5eec..61a2d0728a 100644 --- a/extension/src/popup/components/manageAssets/ManageAssetRows/index.tsx +++ b/extension/src/popup/components/manageAssets/ManageAssetRows/index.tsx @@ -35,7 +35,7 @@ export type ManageAssetCurrency = { decimals?: number; balance?: string; name?: string; - image?: string; + image?: string | null; }; export interface NewAssetFlags { @@ -71,7 +71,7 @@ export const ManageAssetRows = ({ code: string; issuer: string; domain: string; - image: string; + image: string | null; isTrustlineActive: boolean; contract?: string; name?: string; @@ -170,7 +170,7 @@ export const ManageAssetRows = ({ export interface AssetRowData { code?: string; domain: string; - image?: string; + image?: string | null; issuer?: string; isSuspicious?: boolean; name?: string; @@ -204,7 +204,7 @@ const AssetRows = ({ }: { code: string; domain: string; - image: string; + image: string | null; issuer: string; name: string; contract: string; diff --git a/extension/src/popup/ducks/cache.ts b/extension/src/popup/ducks/cache.ts index 55cec41d1b..dd6b0458c9 100644 --- a/extension/src/popup/ducks/cache.ts +++ b/extension/src/popup/ducks/cache.ts @@ -7,7 +7,7 @@ import { TokenDetailsResponse } from "helpers/hooks/useTokenDetails"; type AssetCode = string; type PublicKey = string; -type IconUrl = string; +type IconUrl = string | null; type HomeDomain = string; interface SaveBalancesPayload { diff --git a/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js b/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js index ef0f27d70b..2121979a38 100644 --- a/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js +++ b/extension/src/popup/helpers/__tests__/getIconFromTokenLists.test.js @@ -4,6 +4,7 @@ import { getIconFromTokenLists } from "@shared/api/helpers/getIconFromTokenList" import { TESTNET_NETWORK_DETAILS } from "@shared/constants/stellar"; import { DEFAULT_ASSETS_LISTS } from "@shared/constants/soroban/asset-list"; import { getCanonicalFromAsset } from "helpers/stellar"; +import * as ExtensionMessaging from "@shared/api/helpers/extensionMessaging"; const VERIFIED_TOKEN_CONTRACT = validAssetList.assets[0].contract; const VERIFIED_TOKEN_ISSUER = validAssetList.assets[0].issuer; @@ -11,8 +12,15 @@ const VERIFIED_TOKEN_CODE = validAssetList.assets[0].code; const EXPECTED_ICON_URL = validAssetList.assets[0].icon; jest - .spyOn(TokenListHelpers, "getCombinedAssetListData") - .mockImplementation(() => Promise.resolve([validAssetList])); + .spyOn(ExtensionMessaging, "sendMessageToBackground") + .mockImplementation(() => + Promise.resolve({ + icons: {}, + }), + ), + jest + .spyOn(TokenListHelpers, "getCombinedAssetListData") + .mockImplementation(() => Promise.resolve([validAssetList])); describe("getIconFromTokenLists", () => { it("should return an icon if an asset is in a token list by contract ID", async () => { diff --git a/extension/src/popup/views/Account/hooks/useGetIcons.tsx b/extension/src/popup/views/Account/hooks/useGetIcons.tsx new file mode 100644 index 0000000000..09b322654e --- /dev/null +++ b/extension/src/popup/views/Account/hooks/useGetIcons.tsx @@ -0,0 +1,146 @@ +import { useReducer } from "react"; +import { useDispatch, useSelector } from "react-redux"; +import { captureException } from "@sentry/browser"; + +import { RequestState } from "constants/request"; +import { initialState, reducer } from "helpers/request"; +import { AppDataType, NeedsReRoute } from "helpers/hooks/useGetAppData"; +import { AppDispatch } from "popup/App"; +import { Balances } from "@shared/api/types/backend-api"; +import { + balancesSelector, + saveIconsForBalances, + saveTokenLists, + tokensListsSelector, +} from "popup/ducks/cache"; +import { + settingsNetworkDetailsSelector, + settingsSelector, +} from "popup/ducks/settings"; +import { getCombinedAssetListData } from "@shared/api/helpers/token-list"; +import { getAssetIconCache, getAssetIcons } from "@shared/api/internal"; +import { publicKeySelector } from "popup/ducks/accountServices"; + +interface ResolvedAccountIconsData { + type: AppDataType.RESOLVED; + icons: { [code: string]: string | null }; +} + +export type AccountIconsData = NeedsReRoute | ResolvedAccountIconsData; + +function useGetIcons() { + const reduxDispatch = useDispatch(); + const [state, dispatch] = useReducer( + reducer, + initialState, + ); + const cachedBalances = useSelector(balancesSelector); + const cachedTokenLists = useSelector(tokensListsSelector); + const { assetsLists } = useSelector(settingsSelector); + const publicKey = useSelector(publicKeySelector); + const networkDetails = useSelector(settingsNetworkDetailsSelector); + + const lookupIconData = async ({ + assetsWithoutIcons, + assetsWithIcons, + }: { + assetsWithoutIcons: Balances; + assetsWithIcons: { [code: string]: string | null }; + }) => { + try { + const payload = { + type: AppDataType.RESOLVED, + } as ResolvedAccountIconsData; + + const assetsListsData = cachedTokenLists.length + ? cachedTokenLists + : await getCombinedAssetListData({ + networkDetails, + assetsLists, + }); + + const updatedIcons = await getAssetIcons({ + balances: assetsWithoutIcons, + networkDetails, + assetsListsData, + cachedIcons: {}, + }); + payload.icons = { ...assetsWithIcons, ...updatedIcons }; + reduxDispatch(saveIconsForBalances({ icons: updatedIcons })); + reduxDispatch(saveTokenLists(assetsListsData)); + dispatch({ type: "FETCH_DATA_SUCCESS", payload }); + return payload; + } catch (error) { + captureException(`Error looking up icon data - ${error}`); + return error; + } + }; + + /* + The fetchData flow is not totally dissimilar from the useGetBalances hook with icons. + The main differences: + 1) We don't block the UI with this call, this happens async and loads icons when we have them + 2) We return assets from the cache immediately and then lookup only the missing icons next + */ + + const fetchData = async () => { + dispatch({ type: "FETCH_DATA_START" }); + try { + const payload = { + type: AppDataType.RESOLVED, + } as ResolvedAccountIconsData; + + const backgroundCachedIcons = await getAssetIconCache({ + activePublicKey: publicKey, + }); + + const accountBalances = + cachedBalances[networkDetails.network]?.[publicKey]?.balances; + + const cachedIcons = backgroundCachedIcons.icons || {}; + let icons = {} as { [code: string]: string | null }; + if (Object.keys(cachedIcons).length) { + icons = await getAssetIcons({ + balances: + cachedBalances[networkDetails.network]?.[publicKey]?.balances, + cachedIcons, + }); + + if (Object.keys(icons).length) { + payload.icons = icons; + reduxDispatch(saveIconsForBalances({ icons })); + dispatch({ type: "FETCH_DATA_SUCCESS", payload }); + } + } + + if (accountBalances) { + const assetsWithoutIcons = {} as NonNullable; + + Object.entries(accountBalances).forEach(([asset, balance]) => { + if (!icons[asset] && asset !== "native") { + assetsWithoutIcons[asset] = balance; + } + }); + if (Object.keys(assetsWithoutIcons).length > 0) { + await lookupIconData({ + assetsWithoutIcons, + assetsWithIcons: icons, + }); + } + } + + return payload; + } catch (error) { + dispatch({ type: "FETCH_DATA_ERROR", payload: error }); + captureException(`Error loading icons on Account - ${error}`); + return error; + } + }; + + return { + state, + fetchData, + }; +} + +export { useGetIcons, RequestState }; diff --git a/extension/src/popup/views/Account/index.tsx b/extension/src/popup/views/Account/index.tsx index 35458aa0bb..20d4bb279a 100644 --- a/extension/src/popup/views/Account/index.tsx +++ b/extension/src/popup/views/Account/index.tsx @@ -1,8 +1,9 @@ -import React, { useState, useEffect } from "react"; +import React, { useState, useEffect, useRef } from "react"; import { Navigate, useLocation } from "react-router-dom"; import { useSelector } from "react-redux"; import { Notification } from "@stellar/design-system"; import { useTranslation } from "react-i18next"; +import { isEqual } from "lodash"; import { settingsSorobanSupportedSelector, @@ -26,9 +27,14 @@ import { getTotalUsd } from "popup/helpers/balance"; import { NetworkDetails } from "@shared/constants/stellar"; import { reRouteOnboarding } from "popup/helpers/route"; import { AppDataType } from "helpers/hooks/useGetAppData"; +import { AccountBalances } from "helpers/hooks/useGetBalances"; import { useGetAccountData, RequestState } from "./hooks/useGetAccountData"; import { useGetAccountHistoryData } from "./hooks/useGetAccountHistoryData"; +import { + useGetIcons, + RequestState as IconsRequestState, +} from "./hooks/useGetIcons"; import "popup/metrics/authServices"; import "./styles.scss"; @@ -47,11 +53,15 @@ export const Account = () => { refreshAppData, } = useGetAccountData({ showHidden: false, - includeIcons: true, + includeIcons: false, }); const { state: historyData, fetchData: fetchHistoryData } = useGetAccountHistoryData(); + const { state: iconsData, fetchData: fetchIconsData } = useGetIcons(); + + const previousAccountBalancesRef = useRef(null); + useEffect(() => { const getData = async () => { await fetchData({ useAppDataCache: false }); @@ -77,6 +87,21 @@ export const Account = () => { // eslint-disable-next-line react-hooks/exhaustive-deps }, [accountBalances]); + useEffect(() => { + const getData = async () => { + if ( + accountBalances && + !isEqual(accountBalances, previousAccountBalancesRef.current) + ) { + previousAccountBalancesRef.current = accountBalances; + // unless balances have changed, don't fetch icons; the cache should be hydrated already + await fetchIconsData(); + } + }; + getData(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [accountBalances]); + if ( accountData.state === RequestState.IDLE || accountData.state === RequestState.LOADING @@ -128,6 +153,12 @@ export const Account = () => { } const resolvedData = accountData.data; + const resolvedIcons = + iconsData?.state === IconsRequestState.SUCCESS && + iconsData?.data?.type === AppDataType.RESOLVED + ? iconsData?.data?.icons + : {}; + const tokenPrices = resolvedData?.tokenPrices || {}; const balances = resolvedData?.balances.balances!; const totalBalanceUsd = getTotalUsd(tokenPrices, balances); @@ -228,7 +259,7 @@ export const Account = () => { diff --git a/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx b/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx index b2976da55d..67bcfbf0f2 100644 --- a/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx +++ b/extension/src/popup/views/SignTransaction/hooks/useGetSignTxData.tsx @@ -98,7 +98,7 @@ function useGetSignTxData( ); // Add all icons needed for tx assets - const icons = {} as { [code: string]: string }; + const icons = {} as { [code: string]: string | null }; const assetsListsData = await getCombinedAssetListData({ networkDetails, assetsLists, diff --git a/extension/src/popup/views/__tests__/Account.test.tsx b/extension/src/popup/views/__tests__/Account.test.tsx index 29c1ff4793..d394080b40 100644 --- a/extension/src/popup/views/__tests__/Account.test.tsx +++ b/extension/src/popup/views/__tests__/Account.test.tsx @@ -21,6 +21,8 @@ import { INDEXER_URL } from "@shared/constants/mercury"; import { SERVICE_TYPES } from "@shared/constants/services"; import { HorizonOperation, Response, SettingsState } from "@shared/api/types"; import * as TokenListHelpers from "@shared/api/helpers/token-list"; +import * as GetIconFromTokenList from "@shared/api/helpers/getIconFromTokenList"; +import * as GetIconUrlFromIssuer from "@shared/api/helpers/getIconUrlFromIssuer"; import * as RouteHelpers from "popup/helpers/route"; import { @@ -117,10 +119,6 @@ jest .spyOn(ApiInternal, "getHiddenAssets") .mockImplementation(() => Promise.resolve({ hiddenAssets: {}, error: "" })); -jest - .spyOn(ApiInternal, "getAccountBalances") - .mockImplementation(() => Promise.resolve(mockBalances)); - jest .spyOn(ApiInternal, "getTokenPrices") .mockImplementation(() => Promise.resolve(mockPrices)); @@ -264,6 +262,10 @@ describe("Account view", () => { jest.clearAllMocks(); }); + jest + .spyOn(ApiInternal, "getAccountBalances") + .mockImplementation(() => Promise.resolve(mockBalances)); + it("renders", async () => { render( { expect(screen.getByTestId("account-header")).toBeDefined(); }); + it("displays balances with icons", async () => { + const iconBalances = { + balances: { + ["USDC:GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM"]: { + token: { + code: "USDC", + issuer: { + key: "GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM", + }, + }, + total: new BigNumber("100"), + available: new BigNumber("100"), + blockaidData: {}, + }, + ["FOO:GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM"]: { + token: { + code: "FOO", + issuer: { + key: "GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM", + }, + }, + total: new BigNumber("100"), + available: new BigNumber("100"), + blockaidData: {}, + }, + ["BAZ:GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM"]: { + token: { + code: "BAZ", + issuer: { + key: "GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM", + }, + }, + total: new BigNumber("100"), + available: new BigNumber("100"), + blockaidData: {}, + }, + native: { + token: { type: "native", code: "XLM" }, + total: new BigNumber("50"), + available: new BigNumber("50"), + blockaidData: defaultBlockaidScanAssetResult, + }, + } as any as Balances, + isFunded: true, + subentryCount: 1, + }; + const getIconFromTokenListSpy = jest.spyOn( + GetIconFromTokenList, + "getIconFromTokenLists", + ); + const getIconUrlFromIssuerSpy = jest + .spyOn(GetIconUrlFromIssuer, "getIconUrlFromIssuer") + .mockImplementation(() => Promise.resolve("http://domain.com/baz.png")); + jest + .spyOn(ApiInternal, "getAccountHistory") + .mockImplementationOnce(() => Promise.resolve({ operations: [] } as any)); + jest + .spyOn(ApiInternal, "getAccountBalances") + .mockImplementationOnce(() => Promise.resolve(iconBalances)); + + jest.spyOn(ApiInternal, "getAssetIconCache").mockImplementation(() => + Promise.resolve({ + icons: { + "USDC:GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM": + "http://domain.com/icon.png", + }, + }), + ); + + jest.spyOn(ApiInternal, "getAssetIcons").mockRestore(); + + const assetsListsData = [ + { + assets: [ + { + code: "FOO", + issuer: "GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM", + name: "FOO", + icon: "http://domain.com/foo.png", + }, + ], + }, + ] as any; + jest + .spyOn(TokenListHelpers, "getCombinedAssetListData") + .mockImplementation(() => Promise.resolve(assetsListsData)); + + render( + + + , + ); + await waitFor(() => screen.getByTestId("account-header")); + expect(screen.getByTestId("account-header")).toBeDefined(); + + await waitFor(() => { + const assetNodes = screen.getAllByTestId("account-assets-item"); + expect(assetNodes.length).toEqual(4); + + // fetches and displays icon from background cache + expect( + screen.getByTestId("AccountAssets__asset--loading-USDC"), + ).toContainHTML( + "USDC logo", + ); + + // fetches and displays icon from token list + expect( + screen.getByTestId("AccountAssets__asset--loading-FOO"), + ).toContainHTML("FOO logo"); + expect(getIconFromTokenListSpy).toHaveBeenCalledWith({ + issuerId: "GCK3D3V2XNLLKRFGFFFDEJXA4O2J4X36HET2FE446AV3M4U7DPHO3PEM", + contractId: undefined, + code: "FOO", + assetsListsData: assetsListsData, + networkDetails: TESTNET_NETWORK_DETAILS, + }); + expect(getIconFromTokenListSpy).toHaveBeenCalledTimes(2); + + expect(getIconUrlFromIssuerSpy).toHaveBeenCalledTimes(1); + + // fetches and displays icon from home domain + expect( + screen.getByTestId("AccountAssets__asset--loading-BAZ"), + ).toContainHTML("BAZ logo"); + }); + }); + it("displays balances and scam notifications on Mainnet", async () => { render( { expect( screen.getByTestId("AccountAssets__asset--loading-XLM"), ).not.toContainElement(screen.getByTestId("ScamAssetIcon")); + expect( screen.getByTestId("AccountAssets__asset--loading-USDC"), ).toContainElement(screen.getByTestId("ScamAssetIcon")); diff --git a/extension/src/popup/views/__tests__/AccountHistory.test.tsx b/extension/src/popup/views/__tests__/AccountHistory.test.tsx index 6ce53ec515..b86f08eb62 100644 --- a/extension/src/popup/views/__tests__/AccountHistory.test.tsx +++ b/extension/src/popup/views/__tests__/AccountHistory.test.tsx @@ -22,6 +22,12 @@ import { ROUTES } from "popup/constants/routes"; import { SettingsState } from "@shared/api/types"; import { DEFAULT_ASSETS_LISTS } from "@shared/constants/soroban/asset-list"; +jest.spyOn(ApiInternal, "getAssetIconCache").mockImplementation(() => + Promise.resolve({ + icons: {}, + }), +); + jest.spyOn(ApiInternal, "loadAccount").mockImplementation(() => Promise.resolve({ hasPrivateKey: true,