From 34a9d389b63ed0b79cc6bbe53d4706662f6a2e20 Mon Sep 17 00:00:00 2001 From: Myroslav Sviderok Date: Tue, 5 Nov 2024 12:42:29 +0200 Subject: [PATCH] fix(TransactionFeedV2): Fix removals and "jumps" of pending transactions (#6206) ### Description This PR fixes an issue when some pending transactions can be removed from the feed for a short time and re-added back again. This was happening because pending and confirmed stand by transactions were split into 2 states which were independently affecting different parts of the final `sections` state. Example: - `pending` transaction appears with a spinning indicator after a Send action - when it is marked as confirmed it changes both `standByTransactions.pending` and `standByTransactions.confirmed` This causes the following update chains: - `standByTransactions.pending` -> `sections` - `standByTransactions.confirmed` -> `updatePaginatedData` -> `paginatedData` -> `confirmedTransactions` -> `sections` So in result `sections` is updated twice with visual feedback in a form of removing and re-adding back transactions. ### Test plan All existing tests pass. https://github.com/user-attachments/assets/1a34e993-9621-4f2d-80e0-d7fcc2069cdb ### Related issues - Relates to RET-1207 ### Backwards compatibility ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added) --- .../feed/TransactionFeedV2.test.tsx | 4 +- src/transactions/feed/TransactionFeedV2.tsx | 110 +++++++----------- src/transactions/selectors.ts | 22 +++- src/transactions/utils.ts | 11 +- 4 files changed, 67 insertions(+), 80 deletions(-) diff --git a/src/transactions/feed/TransactionFeedV2.test.tsx b/src/transactions/feed/TransactionFeedV2.test.tsx index 3666ddefcc3..1e89137ca0e 100644 --- a/src/transactions/feed/TransactionFeedV2.test.tsx +++ b/src/transactions/feed/TransactionFeedV2.test.tsx @@ -490,7 +490,9 @@ describe('TransactionFeedV2', () => { }, }) - expect(tree.getByTestId('TransactionList').props.data[0].data.length).toBe(1) + await waitFor(() => + expect(tree.getByTestId('TransactionList').props.data[0].data.length).toBe(1) + ) await act(() => { const newPendingTransaction = addStandbyTransaction({ diff --git a/src/transactions/feed/TransactionFeedV2.tsx b/src/transactions/feed/TransactionFeedV2.tsx index e8d25a40b84..f6da629fd12 100644 --- a/src/transactions/feed/TransactionFeedV2.tsx +++ b/src/transactions/feed/TransactionFeedV2.tsx @@ -27,7 +27,10 @@ import SwapFeedItem from 'src/transactions/feed/SwapFeedItem' import TokenApprovalFeedItem from 'src/transactions/feed/TokenApprovalFeedItem' import TransferFeedItem from 'src/transactions/feed/TransferFeedItem' import NoActivity from 'src/transactions/NoActivity' -import { allStandbyTransactionsSelector, feedFirstPageSelector } from 'src/transactions/selectors' +import { + feedFirstPageSelector, + formattedStandByTransactionsSelector, +} from 'src/transactions/selectors' import { updateFeedFirstPage } from 'src/transactions/slice' import { FeeType, @@ -37,10 +40,7 @@ import { type TokenExchange, type TokenTransaction, } from 'src/transactions/types' -import { - groupFeedItemsInSections, - standByTransactionToTokenTransaction, -} from 'src/transactions/utils' +import { groupFeedItemsInSections } from 'src/transactions/utils' import Logger from 'src/utils/Logger' import { walletAddressSelector } from 'src/web3/selectors' @@ -152,6 +152,23 @@ function sortTransactions(transactions: TokenTransaction[]): TokenTransaction[] }) } +function categorizeTransactions(transactions: TokenTransaction[]) { + const pending: TokenTransaction[] = [] + const confirmed: TokenTransaction[] = [] + const confirmedHashes: string[] = [] + + for (const tx of transactions) { + if (tx.status === TransactionStatus.Pending) { + pending.push(tx) + } else { + confirmed.push(tx) + confirmedHashes.push(tx.transactionHash) + } + } + + return { pending, confirmed, confirmedHashes } +} + /** * Every page of paginated data includes a limited amount of transactions within a certain period. * In standByTransactions we might have transactions from months ago. Whenever we load a new page @@ -192,7 +209,6 @@ function mergeStandByTransactionsInRange({ return [] } - const allowedNetworks = getAllowedNetworksForTransfers() const max = transactions[0].timestamp const min = transactions.at(-1)!.timestamp @@ -204,48 +220,7 @@ function mergeStandByTransactionsInRange({ }) const deduplicatedTransactions = deduplicateTransactions([...transactions, ...standByInRange]) const sortedTransactions = sortTransactions(deduplicatedTransactions) - const transactionsFromAllowedNetworks = sortedTransactions.filter((tx) => - allowedNetworks.includes(tx.networkId) - ) - - return transactionsFromAllowedNetworks -} - -/** - * Current implementation of standbyTransactionsSelector contains function - * getSupportedNetworkIdsForApprovalTxsInHomefeed in its selectors which triggers a lot of - * unnecessary re-renders. This can be avoided if we join it's result in a string and memoize it, - * similar to how it was done with useAllowedNetworkIdsForTransfers hook from queryHelpers.ts - * - * Not using existing selectors for pending/confirmed stand by transaction only cause they are - * dependant on the un-memoized standbyTransactionsSelector selector which will break the new - * pagination flow. - * - * Implementation of pending is identical to pendingStandbyTransactionsSelector. - * Implementation of confirmed is identical to confirmedStandbyTransactionsSelector. - */ -function useStandByTransactions() { - const standByTransactions = useSelector(allStandbyTransactionsSelector) - const allowedNetworkForTransfers = useAllowedNetworksForTransfers() - - return useMemo(() => { - const transactionsFromAllowedNetworks = standByTransactions.filter((tx) => - allowedNetworkForTransfers.includes(tx.networkId) - ) - - const pending: TokenTransaction[] = [] - const confirmed: TokenTransaction[] = [] - - for (const tx of transactionsFromAllowedNetworks) { - if (tx.status === TransactionStatus.Pending) { - pending.push(standByTransactionToTokenTransaction(tx)) - } else { - confirmed.push(tx) - } - } - - return { pending, confirmed } - }, [standByTransactions, allowedNetworkForTransfers]) + return sortedTransactions } /** @@ -253,9 +228,7 @@ function useStandByTransactions() { * we need to listen to the updates of stand by transactions. Whenever we detect that a completed * transaction was in pending status on previous render - we consider it a newly completed transaction. */ -function useNewlyCompletedTransactions( - standByTransactions: ReturnType -) { +function useNewlyCompletedTransactions(standByTransactions: TokenTransaction[]) { const [{ hasNewlyCompletedTransactions, newlyCompletedCrossChainSwaps }, setPreviousStandBy] = useState({ pending: [] as TokenTransaction[], @@ -267,7 +240,7 @@ function useNewlyCompletedTransactions( useEffect( function updatePrevStandBy() { setPreviousStandBy((prev) => { - const confirmedHashes = standByTransactions.confirmed.map((tx) => tx.transactionHash) + const { pending, confirmed, confirmedHashes } = categorizeTransactions(standByTransactions) const newlyCompleted = prev.pending.filter((tx) => { return confirmedHashes.includes(tx.transactionHash) }) @@ -276,8 +249,8 @@ function useNewlyCompletedTransactions( ) return { - pending: [...standByTransactions.pending], - confirmed: [...standByTransactions.confirmed], + pending, + confirmed, newlyCompletedCrossChainSwaps, hasNewlyCompletedTransactions: !!newlyCompleted.length, } @@ -322,9 +295,10 @@ function renderItem({ item: tx }: { item: TokenTransaction }) { export default function TransactionFeedV2() { const { t } = useTranslation() const dispatch = useDispatch() + const allowedNetworkForTransfers = useAllowedNetworksForTransfers() const address = useSelector(walletAddressSelector) const localCurrencyCode = useSelector(getLocalCurrencyCode) - const standByTransactions = useStandByTransactions() + const standByTransactions = useSelector(formattedStandByTransactionsSelector) const feedFirstPage = useSelector(feedFirstPageSelector) const { hasNewlyCompletedTransactions, newlyCompletedCrossChainSwaps } = useNewlyCompletedTransactions(standByTransactions) @@ -382,7 +356,7 @@ export default function TransactionFeedV2() { if (isFirstPage || pageDataIsAbsent) { const mergedTransactions = mergeStandByTransactionsInRange({ transactions: data.transactions, - standByTransactions: standByTransactions.confirmed, + standByTransactions, currentCursor, isLastPage, }) @@ -393,7 +367,7 @@ export default function TransactionFeedV2() { return prev }) }, - [isFetching, data, standByTransactions.confirmed] + [isFetching, data, standByTransactions] ) useEffect( @@ -439,19 +413,19 @@ export default function TransactionFeedV2() { [paginatedData, data?.pageInfo] ) - const confirmedTransactions = useMemo(() => { + const sections = useMemo(() => { const flattenedPages = Object.values(paginatedData).flat() const deduplicatedTransactions = deduplicateTransactions(flattenedPages) const sortedTransactions = sortTransactions(deduplicatedTransactions) - return sortedTransactions - }, [paginatedData]) + const allowedTransactions = sortedTransactions.filter((tx) => + allowedNetworkForTransfers.includes(tx.networkId) + ) - const sections = useMemo(() => { - const noPendingTransactions = standByTransactions.pending.length === 0 - const noConfirmedTransactions = confirmedTransactions.length === 0 - if (noPendingTransactions && noConfirmedTransactions) return [] - return groupFeedItemsInSections(standByTransactions.pending, confirmedTransactions) - }, [standByTransactions.pending, confirmedTransactions]) + if (allowedTransactions.length === 0) return [] + + const { pending, confirmed } = categorizeTransactions(allowedTransactions) + return groupFeedItemsInSections(pending, confirmed) + }, [paginatedData, allowedNetworkForTransfers]) if (!sections.length) { return getFeatureGate(StatsigFeatureGates.SHOW_GET_STARTED) ? ( @@ -467,7 +441,9 @@ export default function TransactionFeedV2() { return } - const totalTxCount = standByTransactions.pending.length + confirmedTransactions.length + const recentCount = sections[0]?.data.length ?? 0 + const confirmedCount = sections[1]?.data.length ?? 0 + const totalTxCount = recentCount + confirmedCount if (totalTxCount > MIN_NUM_TRANSACTIONS_NECESSARY_FOR_SCROLL) { Toast.showWithGravity(t('noMoreTransactions'), Toast.SHORT, Toast.CENTER) } diff --git a/src/transactions/selectors.ts b/src/transactions/selectors.ts index 60d3ffd4aa8..481dd39fedc 100644 --- a/src/transactions/selectors.ts +++ b/src/transactions/selectors.ts @@ -4,12 +4,12 @@ import { getSupportedNetworkIdsForApprovalTxsInHomefeed } from 'src/tokens/utils import { type ConfirmedStandbyTransaction, type NetworkId, + TokenTransaction, TokenTransactionTypeV2, TransactionStatus, } from 'src/transactions/types' -export const allStandbyTransactionsSelector = (state: RootState) => - state.transactions.standbyTransactions +const allStandbyTransactionsSelector = (state: RootState) => state.transactions.standbyTransactions const standbyTransactionsSelector = createSelector( [allStandbyTransactionsSelector, getSupportedNetworkIdsForApprovalTxsInHomefeed], (standbyTransactions, supportedNetworkIdsForApprovalTxs) => { @@ -22,6 +22,24 @@ const standbyTransactionsSelector = createSelector( } ) +export const formattedStandByTransactionsSelector = createSelector( + [allStandbyTransactionsSelector], + (transactions) => { + return transactions.map((tx): TokenTransaction => { + if (tx.status === TransactionStatus.Pending) { + return { + fees: [], + block: '', + transactionHash: '', + ...tx, // in case the transaction already has the above (e.g. cross chain swaps), use the real values + } + } + + return tx + }) + } +) + export const pendingStandbyTransactionsSelector = createSelector( [standbyTransactionsSelector], (transactions) => { diff --git a/src/transactions/utils.ts b/src/transactions/utils.ts index 9506759348f..221d2941fd1 100644 --- a/src/transactions/utils.ts +++ b/src/transactions/utils.ts @@ -2,7 +2,7 @@ import BigNumber from 'bignumber.js' import { PrefixedTxReceiptProperties, TxReceiptProperties } from 'src/analytics/Properties' import i18n from 'src/i18n' import { TokenBalances } from 'src/tokens/slice' -import { NetworkId, StandbyTransaction, TokenTransaction, TrackedTx } from 'src/transactions/types' +import { NetworkId, TrackedTx } from 'src/transactions/types' import { formatFeedSectionTitle, timeDeltaInDays } from 'src/utils/time' import { getEstimatedGasFee, @@ -120,12 +120,3 @@ export function getPrefixedTxAnalyticsProperties( } return prefixedProperties as Partial> } - -export function standByTransactionToTokenTransaction(tx: StandbyTransaction): TokenTransaction { - return { - fees: [], - block: '', - transactionHash: '', - ...tx, // in case the transaction already has the above (e.g. cross chain swaps), use the real values - } -}