Feature/move icons to own hook

@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,

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

@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,24 +1037,54 @@ 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,
   assetsListsData,
   cachedIcons,
 }: {
   balances: Balances;
-  networkDetails: NetworkDetails;
-  assetsListsData: AssetListResponse[];
-  cachedIcons: Record<string, string>;
+  networkDetails?: NetworkDetails;
+  assetsListsData?: AssetListResponse[];
+  cachedIcons: Record<string, string | null>;
 }) => {
-  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;
 }) => {

@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

@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 {

@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",

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,
+  };
+};

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,

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>(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>(RequestState.SUCCESS);
+  });
 });

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),

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));

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;