Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: state corruption of import accounts drawer #6817

Merged
merged 3 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 68 additions & 65 deletions src/components/ManageAccountsDrawer/components/ImportAccounts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ import {
} from '@chakra-ui/react'
import type { ChainId } from '@shapeshiftoss/caip'
import { type AccountId, fromAccountId } from '@shapeshiftoss/caip'
import type { AccountMetadata, Asset } from '@shapeshiftoss/types'
import { useIsFetching, useQuery, useQueryClient } from '@tanstack/react-query'
import type { Asset } from '@shapeshiftoss/types'
import { useInfiniteQuery, useQuery } from '@tanstack/react-query'
import { useCallback, useEffect, useMemo, useState } from 'react'
import { useTranslate } from 'react-polyglot'
import { reactQueries } from 'react-queries'
import { accountManagement } from 'react-queries/queries/accountManagement'
import { Amount } from 'components/Amount/Amount'
import { MiddleEllipsis } from 'components/MiddleEllipsis/MiddleEllipsis'
Expand All @@ -34,6 +33,7 @@ import {
} from 'state/slices/selectors'
import { store, useAppDispatch, useAppSelector } from 'state/store'

import { getAccountIdsWithActivityAndMetadata } from '../helpers'
import { DrawerContentWrapper } from './DrawerContent'

// The number of additional empty accounts to include in the initial fetch
Expand Down Expand Up @@ -160,55 +160,58 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
selectHighestAccountNumberForChainId(state, highestAccountNumberForChainIdFilter),
)
const chainNamespaceDisplayName = asset?.networkName ?? ''
const [accounts, setAccounts] = useState<
{ accountId: AccountId; accountMetadata: AccountMetadata; hasActivity: boolean }[][]
>([])
const queryClient = useQueryClient()
const isLoading =
useIsFetching({
predicate: query => {
return (
query.queryKey[0] === 'accountManagement' &&
['accountIdWithActivityAndMetadata', 'firstAccountIdsWithActivityAndMetadata'].some(
str => str === query.queryKey[1],
)
)
},
}) > 0
const [autoFetching, setAutoFetching] = useState(true)
const [toggledActiveAccountIds, setToggledActiveAccountIds] = useState<Set<AccountId>>(new Set())

// reset component state when chainId changes
useEffect(() => {
setAutoFetching(true)
setToggledActiveAccountIds(new Set())
}, [chainId])

// initial fetch to detect the number of accounts based on the "first empty account" heuristic
const { data: allAccountIdsWithActivity } = useQuery(
accountManagement.firstAccountIdsWithActivityAndMetadata(
chainId,
wallet,
walletDeviceId,
// Account numbers are 0-indexed, so we need to add 1 to the highest account number.
// Add additional empty accounts to show more accounts without having to load more.
highestAccountNumber + 1 + NUM_ADDITIONAL_EMPTY_ACCOUNTS,
),
)
const {
data: accounts,
fetchNextPage,
isLoading,
} = useInfiniteQuery({
queryKey: ['accountIdWithActivityAndMetadata', chainId, walletDeviceId, wallet !== null],
queryFn: async ({ pageParam: accountNumber }) => {
return {
accountNumber,
accountIdWithActivityAndMetadata: await getAccountIdsWithActivityAndMetadata(
accountNumber,
chainId,
wallet,
),
}
},
initialPageParam: 0,
getNextPageParam: lastPage => {
return lastPage.accountNumber + 1
},
})

// Handle initial automatic loading
useEffect(() => {
setAccounts(allAccountIdsWithActivity ?? [])
}, [allAccountIdsWithActivity])

const handleLoadMore = useCallback(async () => {
if (!wallet) return
const accountNumber = accounts.length
const accountResults = await queryClient.fetchQuery(
reactQueries.accountManagement.accountIdWithActivityAndMetadata(
accountNumber,
chainId,
wallet,
walletDeviceId,
),
)
if (!accountResults.length) return
setAccounts(previousAccounts => {
return [...previousAccounts, accountResults]
})
}, [accounts, chainId, queryClient, wallet, walletDeviceId])
if (!autoFetching || !accounts) return

// Account numbers are 0-indexed, so we need to add 1 to the highest account number.
// Add additional empty accounts to show more accounts without having to load more.
const numAccountsToLoad = highestAccountNumber + 1 + NUM_ADDITIONAL_EMPTY_ACCOUNTS

if (accounts.pages.length < numAccountsToLoad) {
fetchNextPage()
} else {
// Stop auto-fetching and switch to manual mode
setAutoFetching(false)
}
}, [accounts, highestAccountNumber, fetchNextPage, autoFetching])

const handleLoadMore = useCallback(() => {
if (isLoading || autoFetching) return
fetchNextPage()
}, [autoFetching, isLoading, fetchNextPage])

const handleToggleAccountIds = useCallback((accountIds: AccountId[]) => {
setToggledActiveAccountIds(previousState => {
Expand All @@ -234,10 +237,15 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
await dispatch(portfolioApi.endpoints.getAccount.initiate({ accountId, upsertOnFetch: true }))
}

const accountMetadataByAccountId = accounts.reduce((accumulator, accounts) => {
const obj = accounts.reduce((innerAccumulator, { accountId, accountMetadata }) => {
return { ...innerAccumulator, [accountId]: accountMetadata }
}, {})
if (!accounts) return

const accountMetadataByAccountId = accounts.pages.reduce((accumulator, accounts) => {
const obj = accounts.accountIdWithActivityAndMetadata.reduce(
(innerAccumulator, { accountId, accountMetadata }) => {
return { ...innerAccumulator, [accountId]: accountMetadata }
},
{},
)
return { ...accumulator, ...obj }
}, {})

Expand All @@ -256,12 +264,13 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
}, [toggledActiveAccountIds, accounts, dispatch, onClose, walletDeviceId])

const accountRows = useMemo(() => {
if (!asset) return null
return accounts.map((accountsForAccountNumber, accountNumber) => {
const accountIds = accountsForAccountNumber.map(({ accountId }) => accountId)
if (!asset || !accounts) return null
return accounts.pages.map(({ accountIdWithActivityAndMetadata }, accountNumber) => {
const accountIds = accountIdWithActivityAndMetadata.map(({ accountId }) => accountId)
const key = accountIds.join('-')
return (
<TableRow
key={accountNumber}
key={key}
accountNumber={accountNumber}
accountIds={accountIds}
asset={asset}
Expand All @@ -282,19 +291,13 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
description={translate('accountManagement.importAccounts.description')}
footer={
<>
<Button
colorScheme='gray'
mr={3}
onClick={onClose}
isDisabled={isLoading}
_disabled={disabledProps}
>
<Button colorScheme='gray' mr={3} onClick={onClose}>
{translate('common.cancel')}
</Button>
<Button
colorScheme='blue'
onClick={handleDone}
isDisabled={isLoading}
isDisabled={isLoading || autoFetching}
_disabled={disabledProps}
>
{translate('common.done')}
Expand All @@ -307,14 +310,14 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
<Table variant='simple'>
<Tbody>
{accountRows}
{isLoading && <LoadingRow />}
{(isLoading || autoFetching) && <LoadingRow />}
</Tbody>
</Table>
</TableContainer>
<Button
colorScheme='gray'
onClick={handleLoadMore}
isDisabled={isLoading}
isDisabled={isLoading || autoFetching}
_disabled={disabledProps}
>
{translate('common.loadMore')}
Expand Down
28 changes: 26 additions & 2 deletions src/components/ManageAccountsDrawer/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import type { ChainId } from '@shapeshiftoss/caip'
import { type ChainId, fromAccountId } from '@shapeshiftoss/caip'
import type { HDWallet } from '@shapeshiftoss/hdwallet-core'
import { matchSorter } from 'match-sorter'
import { getChainAdapterManager } from 'context/PluginProvider/chainAdapterSingleton'
import { isSome } from 'lib/utils'
import { deriveAccountIdsAndMetadata } from 'lib/account/account'
import { assertGetChainAdapter, isSome } from 'lib/utils'
import { checkAccountHasActivity } from 'state/slices/portfolioSlice/utils'

export const filterChainIdsBySearchTerm = (search: string, chainIds: ChainId[]) => {
if (!chainIds.length) return []
Expand All @@ -25,3 +28,24 @@ export const filterChainIdsBySearchTerm = (search: string, chainIds: ChainId[])
threshold: matchSorter.rankings.CONTAINS,
}).map(({ chainId }) => chainId)
}

export const getAccountIdsWithActivityAndMetadata = async (
accountNumber: number,
chainId: ChainId,
wallet: HDWallet | null,
) => {
if (!wallet) return []
const input = { accountNumber, chainIds: [chainId], wallet }
const accountIdsAndMetadata = await deriveAccountIdsAndMetadata(input)

return Promise.all(
Object.entries(accountIdsAndMetadata).map(async ([accountId, accountMetadata]) => {
const { account: pubkey } = fromAccountId(accountId)
const adapter = assertGetChainAdapter(chainId)
const account = await adapter.getAccount(pubkey)
const hasActivity = checkAccountHasActivity(account)

return { accountId, accountMetadata, hasActivity }
}),
)
}
84 changes: 1 addition & 83 deletions src/react-queries/queries/accountManagement.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,7 @@
import { createQueryKeys } from '@lukemorales/query-key-factory'
import type { AccountId } from '@shapeshiftoss/caip'
import { type ChainId, fromAccountId } from '@shapeshiftoss/caip'
import type { HDWallet } from '@shapeshiftoss/hdwallet-core'
import type { AccountMetadata } from '@shapeshiftoss/types'
import { deriveAccountIdsAndMetadata } from 'lib/account/account'
import { fromAccountId } from '@shapeshiftoss/caip'
import { assertGetChainAdapter } from 'lib/utils'
import { checkAccountHasActivity } from 'state/slices/portfolioSlice/utils'

const getAccountIdsWithActivityAndMetadata = async (
accountNumber: number,
chainId: ChainId,
wallet: HDWallet,
) => {
const input = { accountNumber, chainIds: [chainId], wallet }
const accountIdsAndMetadata = await deriveAccountIdsAndMetadata(input)

return Promise.all(
Object.entries(accountIdsAndMetadata).map(async ([accountId, accountMetadata]) => {
const { account: pubkey } = fromAccountId(accountId)
const adapter = assertGetChainAdapter(chainId)
const account = await adapter.getAccount(pubkey)
const hasActivity = checkAccountHasActivity(account)

return { accountId, accountMetadata, hasActivity }
}),
)
}

export const accountManagement = createQueryKeys('accountManagement', {
getAccount: (accountId: AccountId) => ({
Expand All @@ -37,62 +13,4 @@ export const accountManagement = createQueryKeys('accountManagement', {
return account
},
}),
accountIdWithActivityAndMetadata: (
accountNumber: number,
chainId: ChainId,
wallet: HDWallet,
walletDeviceId: string,
) => ({
queryKey: ['accountIdWithActivityAndMetadata', chainId, walletDeviceId, accountNumber],
queryFn: () => {
return getAccountIdsWithActivityAndMetadata(accountNumber, chainId, wallet)
},
}),
firstAccountIdsWithActivityAndMetadata: (
chainId: ChainId,
wallet: HDWallet | null,
walletDeviceId: string,
accountNumberLimit: number,
) => ({
queryKey: [
'firstAccountIdsWithActivityAndMetadata',
chainId,
walletDeviceId,
accountNumberLimit,
],
queryFn: async () => {
let accountNumber = 0

const accounts: {
accountId: AccountId
accountMetadata: AccountMetadata
hasActivity: boolean
}[][] = []

if (!wallet) return []

while (true) {
try {
if (accountNumber >= accountNumberLimit) {
break
}

const accountResults = await getAccountIdsWithActivityAndMetadata(
accountNumber,
chainId,
wallet,
)

accounts.push(accountResults)
} catch (error) {
console.error(error)
break
}

accountNumber++
}

return accounts
},
}),
})
Loading