From a0b2b85861b7115a6846d2cbcd8745da485d31d9 Mon Sep 17 00:00:00 2001 From: Nicholas Gambino Date: Fri, 22 Nov 2024 11:43:57 -0800 Subject: [PATCH 1/5] fix: Temporarily disable isTokenNetworkFilterEqualCurrentNetwork check to ensure cross-chain balances appear as expected --- ui/selectors/selectors.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index b6df07bd2bd0..1724d42843b4 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -2371,11 +2371,12 @@ export const getChainIdsToPoll = createDeepEqualSelector( ( networkConfigurations, currentChainId, - isTokenNetworkFilterEqualCurrentNetwork, + // isTokenNetworkFilterEqualCurrentNetwork, ) => { if ( - !process.env.PORTFOLIO_VIEW || - isTokenNetworkFilterEqualCurrentNetwork + !process.env.PORTFOLIO_VIEW + // !process.env.PORTFOLIO_VIEW || + // isTokenNetworkFilterEqualCurrentNetwork ) { return [currentChainId]; } From 711e921b7dfe3f6cc66dcd92009aee7c62fb51a6 Mon Sep 17 00:00:00 2001 From: Nicholas Gambino Date: Fri, 22 Nov 2024 11:54:16 -0800 Subject: [PATCH 2/5] chore: Add todo comment --- ui/selectors/selectors.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 1724d42843b4..79c29f66e622 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1082,6 +1082,7 @@ export function getIsTokenNetworkFilterEqualCurrentNetwork(state) { const chainId = getCurrentChainId(state); const { tokenNetworkFilter: tokenNetworkFilterValue } = getPreferences(state); const tokenNetworkFilter = tokenNetworkFilterValue || {}; + console.log(chainId, tokenNetworkFilterValue, tokenNetworkFilter); if ( Object.keys(tokenNetworkFilter).length === 1 && Object.keys(tokenNetworkFilter)[0] === chainId @@ -2375,6 +2376,8 @@ export const getChainIdsToPoll = createDeepEqualSelector( ) => { if ( !process.env.PORTFOLIO_VIEW + // TODO: We need to poll across allchains in order to calculate the aggregate balances in the main balance as well as in all networks + // If we scope this to the currently filtered chain, we won't be able to compute the balances for the chains outside of the filtered one // !process.env.PORTFOLIO_VIEW || // isTokenNetworkFilterEqualCurrentNetwork ) { From 77a79e215ae46a810ccab07cc12b5be2b8ccc33d Mon Sep 17 00:00:00 2001 From: Nicholas Gambino Date: Fri, 22 Nov 2024 12:03:18 -0800 Subject: [PATCH 3/5] fix: Remove log --- ui/selectors/selectors.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 79c29f66e622..986c316b0bb0 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1082,7 +1082,6 @@ export function getIsTokenNetworkFilterEqualCurrentNetwork(state) { const chainId = getCurrentChainId(state); const { tokenNetworkFilter: tokenNetworkFilterValue } = getPreferences(state); const tokenNetworkFilter = tokenNetworkFilterValue || {}; - console.log(chainId, tokenNetworkFilterValue, tokenNetworkFilter); if ( Object.keys(tokenNetworkFilter).length === 1 && Object.keys(tokenNetworkFilter)[0] === chainId From 57c1412ceb05176482b3f49573a060bf5b3de1e3 Mon Sep 17 00:00:00 2001 From: Nicholas Gambino Date: Mon, 25 Nov 2024 11:45:10 -0800 Subject: [PATCH 4/5] chore: Add additional selector without network filter constraint --- .../network-filter/network-filter.tsx | 4 +-- ui/selectors/selectors.js | 29 +++++++++++++++---- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/ui/components/app/assets/asset-list/network-filter/network-filter.tsx b/ui/components/app/assets/asset-list/network-filter/network-filter.tsx index 4e9aa14eea25..f7f868b4a8b1 100644 --- a/ui/components/app/assets/asset-list/network-filter/network-filter.tsx +++ b/ui/components/app/assets/asset-list/network-filter/network-filter.tsx @@ -5,9 +5,9 @@ import { getCurrentChainId, getCurrentNetwork, getPreferences, - getChainIdsToPoll, getShouldHideZeroBalanceTokens, getSelectedAccount, + getAllChainsToPoll, } from '../../../../../selectors'; import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; @@ -49,7 +49,7 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => { const shouldHideZeroBalanceTokens = useSelector( getShouldHideZeroBalanceTokens, ); - const allChainIDs = useSelector(getChainIdsToPoll); + const allChainIDs = useSelector(getAllChainsToPoll); const { formattedTokensWithBalancesPerChain } = useGetFormattedTokensPerChain( selectedAccount, shouldHideZeroBalanceTokens, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 986c316b0bb0..64bdb8cd3bb7 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -2364,6 +2364,26 @@ export const getAllEnabledNetworks = createDeepEqualSelector( ), ); +// USE THIS WITH CAUTION +// Only use this selector if you are absolutely sure that your UI component needs data from _all chains_ to compute a value. Else, use getChainsIdsToPoll +// An example of a component that should _not_ use this selector: the token list only needs to poll for chains based on the network filter, (potentially only one chain). In this case you would want to use getChainIdsToPoll +// An example of a component that should _need_ to use this selector: Aggregated balance that needs to display regardless of network filter selection (always needs to display aggregated balance regardless of chains that are selected) +// Leveraging this hook can cause expensive computation that is not needed in all cases, and should be optimized, where possible, to use getChainIdsToPoll instead +export const getAllChainsToPoll = createDeepEqualSelector( + getNetworkConfigurationsByChainId, + getCurrentChainId, + getIsTokenNetworkFilterEqualCurrentNetwork, + (networkConfigurations, currentChainId) => { + if (!process.env.PORTFOLIO_VIEW) { + return [currentChainId]; + } + + return Object.keys(networkConfigurations).filter( + (chainId) => chainId === currentChainId || !TEST_CHAINS.includes(chainId), + ); + }, +); + export const getChainIdsToPoll = createDeepEqualSelector( getNetworkConfigurationsByChainId, getCurrentChainId, @@ -2371,14 +2391,11 @@ export const getChainIdsToPoll = createDeepEqualSelector( ( networkConfigurations, currentChainId, - // isTokenNetworkFilterEqualCurrentNetwork, + isTokenNetworkFilterEqualCurrentNetwork, ) => { if ( - !process.env.PORTFOLIO_VIEW - // TODO: We need to poll across allchains in order to calculate the aggregate balances in the main balance as well as in all networks - // If we scope this to the currently filtered chain, we won't be able to compute the balances for the chains outside of the filtered one - // !process.env.PORTFOLIO_VIEW || - // isTokenNetworkFilterEqualCurrentNetwork + !process.env.PORTFOLIO_VIEW || + isTokenNetworkFilterEqualCurrentNetwork ) { return [currentChainId]; } From 8d8c222b0693d641dd68b9bdcb4aa8d3c2678d08 Mon Sep 17 00:00:00 2001 From: Nicholas Gambino Date: Tue, 26 Nov 2024 09:15:22 -0800 Subject: [PATCH 5/5] chore: Cleanup and update comment for clarity --- ui/selectors/selectors.js | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 64bdb8cd3bb7..b5f52f42dcc8 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -2364,15 +2364,31 @@ export const getAllEnabledNetworks = createDeepEqualSelector( ), ); -// USE THIS WITH CAUTION -// Only use this selector if you are absolutely sure that your UI component needs data from _all chains_ to compute a value. Else, use getChainsIdsToPoll -// An example of a component that should _not_ use this selector: the token list only needs to poll for chains based on the network filter, (potentially only one chain). In this case you would want to use getChainIdsToPoll -// An example of a component that should _need_ to use this selector: Aggregated balance that needs to display regardless of network filter selection (always needs to display aggregated balance regardless of chains that are selected) -// Leveraging this hook can cause expensive computation that is not needed in all cases, and should be optimized, where possible, to use getChainIdsToPoll instead +/* + * USE THIS WITH CAUTION + * + * Only use this selector if you are absolutely sure that your UI component needs + * data from _all chains_ to compute a value. Else, use `getChainIdsToPoll`. + * + * Examples: + * - Components that should NOT use this selector: + * - Token list: This only needs to poll for chains based on the network filter + * (potentially only one chain). In this case, use `getChainIdsToPoll`. + * - Components that SHOULD use this selector: + * - Aggregated balance: This needs to display data regardless of network filter + * selection (always showing aggregated balances across all chains). + * + * Key Considerations: + * - This selector can cause expensive computations. It should only be used when + * necessary, and where possible, optimized to use `getChainIdsToPoll` instead. + * - Logic Overview: + * - If `PORTFOLIO_VIEW` is not enabled, the selector returns only the `currentChainId`. + * - Otherwise, it includes all chains from `networkConfigurations`, excluding + * `TEST_CHAINS`, while ensuring the `currentChainId` is included. + */ export const getAllChainsToPoll = createDeepEqualSelector( getNetworkConfigurationsByChainId, getCurrentChainId, - getIsTokenNetworkFilterEqualCurrentNetwork, (networkConfigurations, currentChainId) => { if (!process.env.PORTFOLIO_VIEW) { return [currentChainId];