From ba2fa07cb5c3a471178b2ae5ac206fdba845800f Mon Sep 17 00:00:00 2001 From: kevin <35275952+kaladinlight@users.noreply.github.com> Date: Wed, 22 Nov 2023 13:47:40 -0700 Subject: [PATCH] fix: only fetch transaction history once on load (#5673) --- src/components/Layout/Header/Header.tsx | 4 +- src/context/AppProvider/AppContext.tsx | 84 ++++++++----------- .../TransactionsProvider.tsx | 4 - .../__snapshots__/portfolioSlice.test.ts.snap | 11 +++ .../slices/portfolioSlice/portfolioSlice.ts | 2 +- .../portfolioSlice/portfolioSliceCommon.ts | 1 + src/state/slices/portfolioSlice/selectors.ts | 9 +- src/state/slices/portfolioSlice/utils.ts | 84 ++++++++----------- 8 files changed, 92 insertions(+), 107 deletions(-) diff --git a/src/components/Layout/Header/Header.tsx b/src/components/Layout/Header/Header.tsx index 591d620de3c..09f953adbf9 100644 --- a/src/components/Layout/Header/Header.tsx +++ b/src/components/Layout/Header/Header.tsx @@ -29,7 +29,7 @@ import { useWallet } from 'hooks/useWallet/useWallet' import { portfolio } from 'state/slices/portfolioSlice/portfolioSlice' import { isUtxoAccountId } from 'state/slices/portfolioSlice/utils' import { - selectPortfolioLoadingStatus, + selectPortfolioDegradedState, selectShowSnapsModal, selectWalletAccountIds, selectWalletId, @@ -59,7 +59,7 @@ const hamburgerIcon = export const Header = memo(() => { const { onToggle, isOpen, onClose } = useDisclosure() - const isDegradedState = useSelector(selectPortfolioLoadingStatus) === 'error' + const isDegradedState = useSelector(selectPortfolioDegradedState) const snapModal = useModal('snaps') const isSnapInstalled = useIsSnapInstalled() const previousSnapInstall = usePrevious(isSnapInstalled) diff --git a/src/context/AppProvider/AppContext.tsx b/src/context/AppProvider/AppContext.tsx index 43d6cc28c8e..2d0c52f2d48 100644 --- a/src/context/AppProvider/AppContext.tsx +++ b/src/context/AppProvider/AppContext.tsx @@ -1,5 +1,5 @@ import { useToast } from '@chakra-ui/react' -import type { AccountId, AssetId, ChainId } from '@shapeshiftoss/caip' +import type { AssetId } from '@shapeshiftoss/caip' import { avalancheChainId, bchChainId, @@ -13,7 +13,6 @@ import { } from '@shapeshiftoss/caip' import { DEFAULT_HISTORY_TIMEFRAME } from 'constants/Config' import difference from 'lodash/difference' -import pull from 'lodash/pull' import React, { useEffect } from 'react' import { useTranslate } from 'react-polyglot' import { useSelector } from 'react-redux' @@ -25,8 +24,6 @@ import { useRouteAssetId } from 'hooks/useRouteAssetId/useRouteAssetId' import { useWallet } from 'hooks/useWallet/useWallet' import { walletSupportsChain } from 'hooks/useWalletSupportsChain/useWalletSupportsChain' import { deriveAccountIdsAndMetadata } from 'lib/account/account' -import type { BN } from 'lib/bignumber/bignumber' -import { bnOrZero } from 'lib/bignumber/bignumber' import { setTimeoutAsync } from 'lib/utils' import { nftApi } from 'state/apis/nft/nftApi' import { snapshotApi } from 'state/apis/snapshot/snapshot' @@ -45,6 +42,7 @@ import { } from 'state/slices/opportunitiesSlice/thunks' import { DefiProvider, DefiType } from 'state/slices/opportunitiesSlice/types' import { portfolio, portfolioApi } from 'state/slices/portfolioSlice/portfolioSlice' +import type { AccountMetadataById } from 'state/slices/portfolioSlice/portfolioSliceCommon' import { preferences } from 'state/slices/preferencesSlice/preferencesSlice' import { selectAssetIds, @@ -105,58 +103,53 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { useEffect(() => { if (!wallet) return ;(async () => { - const chainIds = Array.from(supportedChains).filter(chainId => + let chainIds = Array.from(supportedChains).filter(chainId => walletSupportsChain({ chainId, wallet, isSnapInstalled }), ) + + const accountMetadataByAccountId: AccountMetadataById = {} const isMultiAccountWallet = wallet.supportsBip44Accounts() for (let accountNumber = 0; chainIds.length > 0; accountNumber++) { // only some wallets support multi account if (accountNumber > 0 && !isMultiAccountWallet) break + const input = { accountNumber, chainIds, wallet } - const accountMetadataByAccountId = await deriveAccountIdsAndMetadata(input) - const accountIds: AccountId[] = Object.keys(accountMetadataByAccountId) + const accountIdsAndMetadata = await deriveAccountIdsAndMetadata(input) + const accountIds = Object.keys(accountIdsAndMetadata) + + Object.assign(accountMetadataByAccountId, accountIdsAndMetadata) + const { getAccount } = portfolioApi.endpoints - const opts = { forceRefetch: true } - // do *not* upsertOnFetch here - we need to check if the fetched account is empty const accountPromises = accountIds.map(accountId => - dispatch(getAccount.initiate({ accountId }, opts)), + dispatch(getAccount.initiate({ accountId }, { forceRefetch: true })), ) + const accountResults = await Promise.allSettled(accountPromises) - /** - * because UTXO chains can have multiple accounts per number, we need to aggregate - * balance by chain id to see if we fetch the next by accountNumber - */ - const balanceByChainId = accountResults.reduce>((acc, res, idx) => { - if (res.status === 'rejected') return acc + + let chainIdsWithActivity: string[] = [] + accountResults.forEach((res, idx) => { + if (res.status === 'rejected') return + const { data: account } = res.value - if (!account) return acc + if (!account) return + const accountId = accountIds[idx] const { chainId } = fromAccountId(accountId) - const accountBalance = Object.values(account.accountBalances.byId).reduce( - (acc, byAssetId) => { - Object.values(byAssetId).forEach(balance => (acc = acc.plus(bnOrZero(balance)))) - return acc - }, - bnOrZero(0), - ) - acc[chainId] = bnOrZero(acc[chainId]).plus(accountBalance) - // don't upsert empty accounts past account 0 - if (accountNumber > 0 && accountBalance.eq(0)) return acc - const accountMetadata = accountMetadataByAccountId[accountId] - const payload = { [accountId]: accountMetadata } - dispatch(portfolio.actions.upsertAccountMetadata(payload)) + const { hasActivity } = account.accounts.byId[accountId] + + // don't add accounts with no activity past account 0 + if (accountNumber > 0 && !hasActivity) return delete accountMetadataByAccountId[accountId] + + // unique set to handle utxo chains with multiple account types per account + chainIdsWithActivity = Array.from(new Set([...chainIdsWithActivity, chainId])) + dispatch(portfolio.actions.upsertPortfolio(account)) - return acc - }, {}) - - /** - * if the balance for all accounts for the current chainId and accountNumber - * is zero, we've exhausted that chain, don't fetch more of them - */ - Object.entries(balanceByChainId).forEach(([chainId, balance]) => { - if (balance.eq(0)) pull(chainIds, chainId) // pull mutates chainIds, but we want to }) + + chainIds = chainIdsWithActivity } + + dispatch(portfolio.actions.upsertAccountMetadata(accountMetadataByAccountId)) })() }, [dispatch, wallet, supportedChains, isSnapInstalled]) @@ -169,21 +162,12 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { // once portfolio is done loading, fetch all transaction history useEffect(() => { ;(async () => { + if (!requestedAccountIds.length) return if (portfolioLoadingStatus === 'loading') return const { getAllTxHistory } = txHistoryApi.endpoints - try { - await dispatch(getAllTxHistory.initiate(requestedAccountIds)) - } finally { - // add any nft assets detected in the tx history state. - // this will ensure we have all nft assets that have been associated with the account in the assetSlice with parsed metadata. - // additional nft asset upserts will be handled by the transactions websocket subscription. - // NOTE: We currently upsert NFTs in nftApi, which blockbook data currently overwrites, however, said blockbook data is borked - // TODO: remove me or uncomment me when blockbook data is fixed - // const txsById = store.getState().txHistory.txs.byId - // dispatch(assetsSlice.actions.upsertAssets(makeNftAssetsFromTxs(Object.values(txsById)))) - } + await dispatch(getAllTxHistory.initiate(requestedAccountIds)) })() }, [dispatch, requestedAccountIds, portfolioLoadingStatus]) diff --git a/src/context/TransactionsProvider/TransactionsProvider.tsx b/src/context/TransactionsProvider/TransactionsProvider.tsx index f2652361299..2839142a4d7 100644 --- a/src/context/TransactionsProvider/TransactionsProvider.tsx +++ b/src/context/TransactionsProvider/TransactionsProvider.tsx @@ -193,10 +193,6 @@ export const TransactionsProvider: React.FC = ({ chil if (!wallet) return const accountIds = Object.keys(portfolioAccountMetadata) if (!accountIds.length) return - // this looks useless, but prevents attempting to subscribe multiple times - // something further up the tree from this provider is causing renders when the portfolio status changes, - // even though it shouldn't - if (portfolioLoadingStatus === 'loading') return ;(() => { accountIds.forEach(accountId => { const { chainId } = fromAccountId(accountId) diff --git a/src/state/slices/portfolioSlice/__snapshots__/portfolioSlice.test.ts.snap b/src/state/slices/portfolioSlice/__snapshots__/portfolioSlice.test.ts.snap index 931870d23a0..f1c347d50d4 100644 --- a/src/state/slices/portfolioSlice/__snapshots__/portfolioSlice.test.ts.snap +++ b/src/state/slices/portfolioSlice/__snapshots__/portfolioSlice.test.ts.snap @@ -22,6 +22,7 @@ Object { "assetIds": Array [ "bip122:000000000019d6689c085ae165831e93/slip44:0", ], + "hasActivity": true, }, }, "ids": Array [ @@ -61,11 +62,13 @@ Object { "assetIds": Array [ "bip122:000000000019d6689c085ae165831e93/slip44:0", ], + "hasActivity": true, }, "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w": Object { "assetIds": Array [ "bip122:000000000019d6689c085ae165831e93/slip44:0", ], + "hasActivity": true, }, }, "ids": Array [ @@ -116,6 +119,7 @@ Object { "assetIds": Array [ "bip122:000000000019d6689c085ae165831e93/slip44:0", ], + "hasActivity": true, }, "eip155:1:0x9a2d593725045d1727d525dd07a396f9ff079bb1": Object { "assetIds": Array [ @@ -124,6 +128,7 @@ Object { "eip155:1/erc20:0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48", "eip155:1/erc20:0x5f18c75abdae578b483e5f43f12a39cf75b973a9", ], + "hasActivity": true, }, "eip155:1:0xea674fdde714fd979de3edf0f56aa9716b898ec8": Object { "assetIds": Array [ @@ -132,6 +137,7 @@ Object { "eip155:1/erc20:0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48", "eip155:1/erc20:0x5f18c75abdae578b483e5f43f12a39cf75b973a9", ], + "hasActivity": true, }, }, "ids": Array [ @@ -177,6 +183,7 @@ Object { "assetIds": Array [ "bip122:000000000019d6689c085ae165831e93/slip44:0", ], + "hasActivity": true, }, "eip155:1:0x6c8a778ef52e121b7dff1154c553662306a970e9": Object { "assetIds": Array [ @@ -186,6 +193,7 @@ Object { "eip155:1/erc20:0xc770eefad204b5180df6a14ee197d99d808ee52d", "eip155:1/erc20:0xf0939011a9bb95c3b791f0cb546377ed2693a574", ], + "hasActivity": true, }, }, "ids": Array [ @@ -224,6 +232,7 @@ Object { "eip155:1/slip44:60", "eip155:1/erc20:0xc770eefad204b5180df6a14ee197d99d808ee52d", ], + "hasActivity": true, }, }, "ids": Array [ @@ -266,12 +275,14 @@ Object { "eip155:1/slip44:60", "eip155:1/erc20:0xc770eefad204b5180df6a14ee197d99d808ee52d", ], + "hasActivity": true, }, "eip155:1:0xea674fdde714fd979de3edf0f56aa9716b898ec8": Object { "assetIds": Array [ "eip155:1/slip44:60", "eip155:1/erc20:0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48", ], + "hasActivity": true, }, }, "ids": Array [ diff --git a/src/state/slices/portfolioSlice/portfolioSlice.ts b/src/state/slices/portfolioSlice/portfolioSlice.ts index fe00336880b..d5cdd89bbe3 100644 --- a/src/state/slices/portfolioSlice/portfolioSlice.ts +++ b/src/state/slices/portfolioSlice/portfolioSlice.ts @@ -167,7 +167,7 @@ export const portfolioApi = createApi({ console.error(e) const data = cloneDeep(initialState) data.accounts.ids.push(accountId) - data.accounts.byId[accountId] = { assetIds: [] } + data.accounts.byId[accountId] = { assetIds: [], hasActivity: false } dispatch(portfolio.actions.upsertPortfolio(data)) return { data } } diff --git a/src/state/slices/portfolioSlice/portfolioSliceCommon.ts b/src/state/slices/portfolioSlice/portfolioSliceCommon.ts index c6d7fafbe69..6d830af9716 100644 --- a/src/state/slices/portfolioSlice/portfolioSliceCommon.ts +++ b/src/state/slices/portfolioSlice/portfolioSliceCommon.ts @@ -6,6 +6,7 @@ import type { Nominal } from 'types/common' export type PortfolioAccount = { /** The asset ids belonging to an account */ assetIds: AssetId[] + hasActivity?: boolean } export type PortfolioAccounts = { diff --git a/src/state/slices/portfolioSlice/selectors.ts b/src/state/slices/portfolioSlice/selectors.ts index a571cf06615..ef8fa270df4 100644 --- a/src/state/slices/portfolioSlice/selectors.ts +++ b/src/state/slices/portfolioSlice/selectors.ts @@ -138,12 +138,19 @@ export const selectPortfolioLoadingStatus = createSelector( selectPortfolioLoadingStatusGranular, (portfolioLoadingStatusGranular): PortfolioLoadingStatus => { const vals = values(portfolioLoadingStatusGranular) - if (vals.every(val => val === 'loading')) return 'loading' + if (vals.some(val => val === 'loading')) return 'loading' if (vals.some(val => val === 'error')) return 'error' return 'success' }, ) +export const selectPortfolioDegradedState = createSelector( + selectPortfolioLoadingStatusGranular, + (portfolioLoadingStatusGranular): boolean => { + return values(portfolioLoadingStatusGranular).some(val => val === 'error') + }, +) + export const selectPortfolioTotalUserCurrencyBalance = createSelector( selectPortfolioUserCurrencyBalances, (portfolioUserCurrencyBalances): string => diff --git a/src/state/slices/portfolioSlice/utils.ts b/src/state/slices/portfolioSlice/utils.ts index 375c82695dd..4113183f4bc 100644 --- a/src/state/slices/portfolioSlice/utils.ts +++ b/src/state/slices/portfolioSlice/utils.ts @@ -24,7 +24,7 @@ import { toAccountId, toAssetId, } from '@shapeshiftoss/caip' -import type { Account } from '@shapeshiftoss/chain-adapters' +import type { Account, UtxoChainId } from '@shapeshiftoss/chain-adapters' import type { HDWallet } from '@shapeshiftoss/hdwallet-core' import { supportsArbitrum, @@ -176,17 +176,20 @@ export const accountToPortfolio: AccountToPortfolio = args => { const ethAccount = account as Account const { chainId, assetId, pubkey } = account const accountId = toAccountId({ chainId, account: pubkey }) - portfolio.accountBalances.ids.push(accountId) - portfolio.accounts.byId[accountId] = { assetIds: [] } - portfolio.accounts.byId[accountId].assetIds.push(assetId) - portfolio.accounts.ids.push(accountId) + const hasActivity = + bnOrZero(ethAccount.chainSpecific.nonce).gt(0) || bnOrZero(ethAccount.balance).gt(0) - portfolio.accountBalances.byId[accountId] = { - [assetId]: ethAccount.balance, - } + portfolio.accounts.ids.push(accountId) + portfolio.accounts.byId[accountId] = { assetIds: [assetId], hasActivity } + portfolio.accountBalances.ids.push(accountId) + portfolio.accountBalances.byId[accountId] = { [assetId]: account.balance } ethAccount.chainSpecific.tokens?.forEach(token => { + if (bnOrZero(token.balance).gt(0)) { + portfolio.accounts.byId[accountId].hasActivity = true + } + // don't update portfolio if asset is not in the store except for nft assets, // nft assets will be dynamically upserted based on the state of the txHistory slice after the portfolio is loaded if (!isNft(token.assetId) && !args.assetIds.includes(token.assetId)) return @@ -208,39 +211,27 @@ export const accountToPortfolio: AccountToPortfolio = args => { } portfolio.accounts.byId[accountId].assetIds.push(token.assetId) - - portfolio.accountBalances.byId[accountId] = { - ...portfolio.accountBalances.byId[accountId], - [token.assetId]: token.balance, - } + portfolio.accountBalances.byId[accountId][token.assetId] = token.balance }) + break } case CHAIN_NAMESPACE.Utxo: { + const utxoAccount = account as Account const { balance, chainId, assetId, pubkey } = account + // Since btc the pubkeys (address) are base58Check encoded, we don't want to lowercase them and put them in state const accountId = `${chainId}:${pubkey}` - portfolio.accountBalances.ids.push(accountId) - - // initialize this - portfolio.accountBalances.byId[accountId] = {} - - // add the balance from the top level of the account - portfolio.accountBalances.byId[accountId][assetId] = balance + const hasActivity = + bnOrZero(balance).gt(0) || + bnOrZero(utxoAccount.chainSpecific.nextChangeAddressIndex).gt(0) || + bnOrZero(utxoAccount.chainSpecific.nextReceiveAddressIndex).gt(0) - // initialize - if (!portfolio.accounts.byId[accountId]?.assetIds.length) { - portfolio.accounts.byId[accountId] = { - assetIds: [], - } - } - - portfolio.accounts.ids = Array.from(new Set([...portfolio.accounts.ids, accountId])) - - portfolio.accounts.byId[accountId].assetIds = Array.from( - new Set([...portfolio.accounts.byId[accountId].assetIds, assetId]), - ) + portfolio.accounts.ids.push(accountId) + portfolio.accounts.byId[accountId] = { assetIds: [assetId], hasActivity } + portfolio.accountBalances.ids.push(accountId) + portfolio.accountBalances.byId[accountId] = { [assetId]: balance } break } @@ -248,31 +239,26 @@ export const accountToPortfolio: AccountToPortfolio = args => { const cosmosAccount = account as Account const { chainId, assetId } = account const accountId = toAccountId({ chainId, account: _xpubOrAccount }) - portfolio.accountBalances.ids.push(accountId) - portfolio.accounts.byId[accountId] = { - assetIds: [], - } - - portfolio.accounts.byId[accountId].assetIds.push(assetId) + const hasActivity = + bnOrZero(cosmosAccount.chainSpecific.sequence).gt(0) || + bnOrZero(cosmosAccount.balance).gt(0) portfolio.accounts.ids.push(accountId) + portfolio.accounts.byId[accountId] = { assetIds: [assetId], hasActivity } + portfolio.accountBalances.ids.push(accountId) + portfolio.accountBalances.byId[accountId] = { [assetId]: account.balance } cosmosAccount.chainSpecific.assets?.forEach(asset => { - if (!args.assetIds.includes(asset.assetId)) { - return + if (bnOrZero(asset.amount).gt(0)) { + portfolio.accounts.byId[accountId].hasActivity = true } - portfolio.accounts.byId[accountId].assetIds.push(asset.assetId) - portfolio.accountBalances.byId[accountId] = { - ...portfolio.accountBalances.byId[accountId], - [asset.assetId]: asset.amount, - } + if (!args.assetIds.includes(asset.assetId)) return + + portfolio.accounts.byId[accountId].assetIds.push(asset.assetId) + portfolio.accountBalances.byId[accountId][asset.assetId] = asset.amount }) - portfolio.accountBalances.byId[accountId] = { - ...portfolio.accountBalances.byId[accountId], - [assetId]: account.balance, - } break }