Skip to content

Commit

Permalink
fix(TransactionFeedV2): Fix removals and "jumps" of pending transacti…
Browse files Browse the repository at this point in the history
…ons (#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

<!-- Brief explanation of why these changes are/are not backwards
compatible. -->

### 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)
  • Loading branch information
sviderock authored Nov 5, 2024
1 parent 52fb491 commit 34a9d38
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 80 deletions.
4 changes: 3 additions & 1 deletion src/transactions/feed/TransactionFeedV2.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
110 changes: 43 additions & 67 deletions src/transactions/feed/TransactionFeedV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -192,7 +209,6 @@ function mergeStandByTransactionsInRange({
return []
}

const allowedNetworks = getAllowedNetworksForTransfers()
const max = transactions[0].timestamp
const min = transactions.at(-1)!.timestamp

Expand All @@ -204,58 +220,15 @@ 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
}

/**
* In order to properly detect if any of the existing pending transactions turned into completed
* 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<typeof useStandByTransactions>
) {
function useNewlyCompletedTransactions(standByTransactions: TokenTransaction[]) {
const [{ hasNewlyCompletedTransactions, newlyCompletedCrossChainSwaps }, setPreviousStandBy] =
useState({
pending: [] as TokenTransaction[],
Expand All @@ -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)
})
Expand All @@ -276,8 +249,8 @@ function useNewlyCompletedTransactions(
)

return {
pending: [...standByTransactions.pending],
confirmed: [...standByTransactions.confirmed],
pending,
confirmed,
newlyCompletedCrossChainSwaps,
hasNewlyCompletedTransactions: !!newlyCompleted.length,
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -382,7 +356,7 @@ export default function TransactionFeedV2() {
if (isFirstPage || pageDataIsAbsent) {
const mergedTransactions = mergeStandByTransactionsInRange({
transactions: data.transactions,
standByTransactions: standByTransactions.confirmed,
standByTransactions,
currentCursor,
isLastPage,
})
Expand All @@ -393,7 +367,7 @@ export default function TransactionFeedV2() {
return prev
})
},
[isFetching, data, standByTransactions.confirmed]
[isFetching, data, standByTransactions]
)

useEffect(
Expand Down Expand Up @@ -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) ? (
Expand All @@ -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)
}
Expand Down
22 changes: 20 additions & 2 deletions src/transactions/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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) => {
Expand Down
11 changes: 1 addition & 10 deletions src/transactions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -120,12 +120,3 @@ export function getPrefixedTxAnalyticsProperties<Prefix extends string>(
}
return prefixedProperties as Partial<PrefixedTxReceiptProperties<Prefix>>
}

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

0 comments on commit 34a9d38

Please sign in to comment.