Feature/move icons to own hook #2308

piyalbasu · 2025-10-15T14:33:14Z

There are 2 main bottlenecks for useGetBalances({ withIcons: true }):

on first load of the app, a user has to fetch all their token lists before they can start to fetch their icons.
we must fetch these tokens lists, fetch their icons (either from cache, asset list, or issuer) before we can show the user any of their balance info

The goal of this PR is to eliminate both of those for the Account view. Here, I'm doing something similar to what I've done with the previous account history pr; I'm moving this icons fetch to a separate hooks that runs after account balances are fetched.

This hook first attempts to retrieve all previously fetched icons from the background's long-lived cache (without loading the token lists at all). This retrieves the icons very quickly and immediately returns them to the UI. For all remaining icons, we go through the familiar process of checking asset lists and the asset issuer. Because this now happens async, this process ca run while the user already has a usable UI.

I am not removing useGetBalances({ withIcons: true }) in other views because this method actually runs quite quickly once the redux store has been hydrated. The main issue is on app first load. If a user does happen to refresh the app on a page that uses useGetBalances({ withIcons: true }), I added a background cache lookup to make it run even faster. (It would be quite an edge case for a user to not have any icons saved in this scenario)

piyalbasu · 2025-10-15T14:34:00Z

@shared/api/helpers/getIconFromTokenList.ts

          issuerId &&
          record.issuer &&
          record.issuer === issuerId &&
+          record.code === code &&


this is a small bug I found while testing - we need to check the asset issuer AND asset code because one G address could be the issuer for multiple assets

wdyt about adding a test that asserts that two assets from the same issuer get the correct icon?

That's actually how I found this bug :) The test is right here: https://github.com/stellar/freighter/pull/2308/files#diff-9943e9b6f3ea7a02af1d671f5ea2a1b9381361230431f521219ea5f36702961cR324

ah nice, ty

piyalbasu · 2025-10-15T14:34:36Z

@shared/api/helpers/getIconFromTokenList.ts

    }
  }

+  if (verifiedToken?.icon) {


if we find an icon, let's cache it in the background so in the future, we don't need to consult the token list at all; we'll already have the url in our background

piyalbasu · 2025-10-15T14:35:17Z

@shared/api/helpers/getIconUrlFromIssuer.ts

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


another small bug I found while testing - Array.every doesn't properly await promises. Changing this to a for loop

should this break out of the loop once the icon has been found?

piyalbasu · 2025-10-15T14:35:52Z

@shared/api/internal.ts

+    return { icons };
+  }
+  return { icons };
+};


I'm adding a method to grab the entire cached icon list. Previously, we were consulting this list one asset at a time

piyalbasu · 2025-10-15T14:42:17Z

@shared/api/internal.ts

    const balanceValues = Object.values(balances);

    for (let i = 0; i < balanceValues.length; i++) {
+      let icon = "";


another small bugfix. This just happened to work, but this icon needs to be reset on every loop

piyalbasu · 2025-10-15T14:43:38Z

extension/src/helpers/hooks/useGetBalances.tsx


      if (options.includeIcons) {
+        let cachedIconsFromCache = cachedIcons;
+        if (!Object.keys(cachedIcons).length) {


if we haven't hydrated redux yet (first load), use background storage's asset cache as the cache

piyalbasu · 2025-10-15T14:44:25Z

extension/src/popup/components/account/AccountAssets/index.tsx

+  isEqual(prevProps.assetIcons, nextProps.assetIcons) &&
+  prevProps.isSuspicious === nextProps.isSuspicious;
+
+export const AssetIcon = memo(


memozing here prevents unnecessary renders/flickers when asset balances poll

extension/src/popup/views/Account/hooks/useGetIcons.tsx

piyalbasu · 2025-10-15T14:46:26Z

extension/src/popup/views/Account/hooks/useGetIcons.tsx

+      payload.icons = { ...assetsWithIcons, ...updatedIcons };
+      reduxDispatch(saveIconsForBalances({ icons: updatedIcons }));
+      reduxDispatch(saveTokenLists(assetsListsData));
+      dispatch({ type: "FETCH_DATA_SUCCESS", payload });


if we find more icons after lookup, dispatch them here

piyalbasu · 2025-10-15T14:48:50Z

@shared/api/internal.ts

  key: string;
  code: string;
-  assetIcons: { [code: string]: string };
+  assetIcons: { [code: string]: string | null };


there's a lot of these type updates in this PR - this just allows us to skip an unnecessary asset issuer lookup if previously in the user's session we weren't able to find the icon

piyalbasu · 2025-10-15T14:57:51Z

@shared/api/internal.ts

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


small adjustment here - look up the icon issuer last because fetching from the fetched token list should be faster. Let's save the most expensive call for last and only if needed

piyalbasu · 2025-10-15T16:04:37Z

This video illustrates the difference in UX between loading the app with and without cached icons

First, we load the app without any cache. Then, the second load shows how the assets load utilizing the cache:

Screen.Recording.2025-10-15.at.10.59.12.AM.mov

piyalbasu · 2025-10-15T16:06:58Z

And here's a video illustrating the difference between what's in production now and what the improvement looks like. By some rough estimating, I'm getting a Largest Contentul Paint reduction of about a full 1 second!

Before:

Screen.Recording.2025-10-15.at.12.02.09.PM.mov

After:

Screen.Recording.2025-10-15.at.12.06.38.PM.mov

aristidesstaffieri · 2025-10-15T18:17:00Z

@shared/api/internal.ts

        }

-        icon = await getIconUrlFromIssuer({ key, code, networkDetails });
+        if (cachedIcon === null) {


So in the case that an image URL didn't resolve temporarily for whatever reasons(network or something), then this cache will prevent the icon from ever being used in the future?

Maybe this cache should have a ttl?

Ah, so something important here (that I can document better) is this null value is only used in Redux. We only ever store an icon in the "hard cache" in the background when we've successfully found an icon. So, if there were to be a networking issue, a user would re-fetch the next time they reload the app

aristidesstaffieri

👏

* Feature/move history fetch to bg (#2273) * upgrade to yarn 2 and use resolutions to block vulnerable package versions (#2239) * upgrade to yarn 2 and use resolutions to block vulnerable package versions * rm deprecated .yarnrc * rm yarnpath * try committing yarn binary to repo * try corepack enable for gha * update run tests cmd * rm yarnpath * rm npm i yarn * update all pipelines * rm superfluous history types * ensure invoke host function tx shows contract parameters (#2243) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * Bugfix/rm auth param names (#2244) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op (#2246) * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * Revert "add issuer for changeTrust op (#2246)" (#2247) This reverts commit 19c8a68. * Bugfix/add issuer for changetrust (#2249) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * cache account balances and poll for updates * fix CI tests * rm `force:true` which was causing action to happen too fast * do a fresh balance fetch on account/network change * first pass at async history * pr comments * allow for history caching * add more sentry tracking for Account and Wallets views (#2268) * add more sentry tracking for Account and Wallets views * adding more sentry reporting * gracefully degrade on errors from Blockaid (#2269) * gracefully degrade on errors from Blockaid * should not be necessary to skip dapp scanning on custom network * rm extra dep * add a test for persisting configurations in the send flow (#2271) * add a test for persisting configurations in the send flow * rm logs and update muxed acct; lower xlm payment * rm slow loading simulation * handle missing scan-tx result; add disabled state for Confirm Anyway (#2272) * handle missing scan-tx result; add disabled state for Confirm Anyway * assertions to show correct confirm button on Blockaid error * add cache for balances to ensure we do a fresh lookup when needed (#2275) * add cache for balances to ensure we do a fresh lookup when needed * add try...catch to token-prices polling * rm log * only dispatch saveBalancesForAccount when fresh data has been fetched * adjust test to wait for UI change * replace yarn setup with just yarn * rm unnecessary return * clear token details on redux clear action * make history row construction async and check for redux state for updates * add tests for assetdetails * increase timeout for flakey test * pr comments * refresh account history every time account balances refresh * check for updated appdata before showing password modal (#2300) * check for updated appdata before showing password modal * update error msg * rm unused redirect logic * stringify errors rather than using `cause` (#2302) * Feature/move icons to own hook (#2308) * move get icons out of critical path; rely on background's cache * add tests and comments * add comment * add comment * only dispatch if we have cached icons * PR comments * skip blockaid scan on first fetch of account-balances (#2310) * skip blockaid scan on first fetch of account-balances * rm stub change * rm more stubs * rm log * add comments and update boolean naming * Dropdown menu option to copy wallet address (#2316) * add button to copy address from dropdown * Added translations * revert translation file changes * revert translation file changes * Added translations * revert changes to translation files * move copy address button to first dropdown position * scroll on long strings; pretty print json (#2320) * scroll on long strings; pretty print json * rm log * add correct snapshot for json message * rm log * finish comment * add error case for JSON * don't use carat for lib * update yarn.lock * move scrollbar to btm of container; reduce json font size * update snapshot * re-searching so should abort any in flight API requests (#2323) * re-searching so should abort any in flight API requests * add comment * fix test name * make test more reliable * add check for correct search results * fix jest locator * [FEATURE] new send/swap navigation flow (#2353) * adds SelectionTile and AddressTile, updates nav flows to match updates. Adds query parameter for default values in send flow * Added translations * adds address tile and uses it in swap flow, tweaks selection tile styles * adds unit tests for new tile components * Added translations * updates swap navigation flow to match updates, updates tests flows to match * updates back icon for send and swap steps, fixes bad test references * tweaks locator in address tile tests * adds store state to asset tile tests, removes asset icon mock * updates SelectionTile prop name, adds isSuspicious prop for AssetTile * adds placeholder value in TokenList for missing token USD value * uses real IdenticonImg in address tile unit tests * adds query param validation for send and swap flow * Update extension/src/popup/views/SendPayment/index.tsx Co-authored-by: Cássio Marcos Goulart <[email protected]> * adds missing import * adds class for tile icon --------- Co-authored-by: Cássio Marcos Goulart <[email protected]> * [FEATURE] adds send and swap buttons to asset detail view (#2351) * adds send and swap buttons to asset detail view * uses secondary button styles * removes run snapshots job (#2355) * release/5.35.4 (#2354) * upgrade to ledger-hq/hw-transport-webhid (#2350) * upgrade to ledger-hq/hw-transport-webhid * add tests * add ledger support for new trustline flow (#2352) * upgrade to ledger-hq/hw-transport-webhid * add ledger support for new trustline flow * only re-fetch balances if we were successful * test for fetching balances on success * add reset spys * adjust spacing at top of hw wallet modal * Now that `Done` button properly shows, click it in tests (#2356) * skip flakey test * skip flakey test * renames local vars to follow convention * adds tests for LP share and tweaks LP title * adds links with query params for asset detail CTAs --------- Co-authored-by: Piyal Basu <[email protected]> * only fetch asset list data if needed (#2369) * only fetch asset list data if needed * correctly show icon loading state * [BUG] SAC token management improvements (#2374) * adds SAC detection when changing trust in the add and remove token flows * updates arg signature for isAssetSac * Feature/cache token prices (#2373) * cache token prices and batch loading wallets * use similar methodology for token price and account balance caching * fix loading state trigger * fix tests * use helper for cache clearing * set isFetchingTokenPrices to false in catch handler * rollback error change * load backend settings async on Account view (#2381) * load backend settings async on Account view * rm console.logs * Feature/use ledger key for home domains (#2363) * use ledger-key/accounts endpoint for home domains * fix tests * create generic ledger key account helper; add tests * rm unneeded data-test prop * rm unused import * [CHORE] git process updates (#2361) * moves the add translations hook to the pre commit stage, removes standalone translations commit * adds script to update app version, removes version update from submit production action, adds update version step to test run action for release branches * removes version input, now uses package version * fetch asset domains in one calls * fix tests * cache home domains while iterating over account history rows * fitler non-G keys * rm .only * fix test param * PR comments --------- Co-authored-by: aristides <[email protected]> * update version numbers for release * rm unnecessary calls to make flows even faster (#2391) * makes send swap buttons stay in the container in full screen mode (#2392) * makes send swap buttons stay in the container in full screen mode * add a pause to make sure flakey e2e test has time to save changes * use redux selector for allAccounts to properly update rename (#2403) * use redux selector for allAccounts to properly update rename * add longer timeout for flakey btn * Add docs for 5.36.0 (#2408) --------- Co-authored-by: leofelix077 <[email protected]> Co-authored-by: aristides <[email protected]> Co-authored-by: Cássio Marcos Goulart <[email protected]>

* move get icons out of critical path; rely on background's cache * add tests and comments * add comment * add comment * only dispatch if we have cached icons * PR comments

* Feature/move icons to own hook (#2308) * move get icons out of critical path; rely on background's cache * add tests and comments * add comment * add comment * only dispatch if we have cached icons * PR comments * [FEATURE] adds send and swap buttons to asset detail view (#2351) * adds send and swap buttons to asset detail view * uses secondary button styles * removes run snapshots job (#2355) * release/5.35.4 (#2354) * upgrade to ledger-hq/hw-transport-webhid (#2350) * upgrade to ledger-hq/hw-transport-webhid * add tests * add ledger support for new trustline flow (#2352) * upgrade to ledger-hq/hw-transport-webhid * add ledger support for new trustline flow * only re-fetch balances if we were successful * test for fetching balances on success * add reset spys * adjust spacing at top of hw wallet modal * Now that `Done` button properly shows, click it in tests (#2356) * skip flakey test * skip flakey test * renames local vars to follow convention * adds tests for LP share and tweaks LP title * adds links with query params for asset detail CTAs --------- Co-authored-by: Piyal Basu <[email protected]> * first pass at showing collectibles in UI * add metadata fetching * add tests for metadata * check for owner of collectibles and test * add empty placeholder and add comments; rm placeholder values * rm captureException * rm duplicated tests added by rebase * pr comments * make non-square nft's cover; only show `collectibles` tab on non-custom network * attempt to clean up flakey e2e tests * rollback parallel testing * removing testing data * update tests --------- Co-authored-by: aristides <[email protected]>

piyalbasu added 2 commits October 14, 2025 12:30

move get icons out of critical path; rely on background's cache

d1df3a8

add tests and comments

7e8e393

piyalbasu commented Oct 15, 2025

View reviewed changes

piyalbasu added 2 commits October 15, 2025 10:40

add comment

f9226f4

add comment

243346e

piyalbasu commented Oct 15, 2025

View reviewed changes

extension/src/popup/views/Account/hooks/useGetIcons.tsx Outdated Show resolved Hide resolved

piyalbasu commented Oct 15, 2025

View reviewed changes

only dispatch if we have cached icons

5158449

piyalbasu marked this pull request as ready for review October 15, 2025 15:53

aristidesstaffieri reviewed Oct 15, 2025

View reviewed changes

PR comments

2ca75c5

aristidesstaffieri approved these changes Oct 15, 2025

View reviewed changes

piyalbasu merged commit cc130fc into release/5.36.0 Oct 15, 2025
3 checks passed

piyalbasu deleted the feature/move-icons-to-own-hook branch October 15, 2025 19:34

piyalbasu added a commit that referenced this pull request Nov 25, 2025

Feature/move icons to own hook (#2308)

52bc64b

* move get icons out of critical path; rely on background's cache * add tests and comments * add comment * add comment * only dispatch if we have cached icons * PR comments

Feature/move icons to own hook #2308

Feature/move icons to own hook #2308

Uh oh!

Conversation

piyalbasu commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyalbasu Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyalbasu Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyalbasu commented Oct 15, 2025

Uh oh!

piyalbasu commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aristidesstaffieri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

piyalbasu commented Oct 15, 2025 •

edited

Loading

piyalbasu Oct 15, 2025 •

edited

Loading

piyalbasu Oct 15, 2025 •

edited

Loading

piyalbasu commented Oct 15, 2025 •

edited

Loading