chore(TransactionFeedV2): Cleanup stand by transactions in Transactio…

…n Feed V2 (#6146)

### Description
4th PR of RET-1207. This implements the cleanup of confirmed stand by
transactions that are already present in the feed. Currently, we keep
confirmed transactions in `standByTransactions` even if they are present
in the pagination data. This is unnecessary as it takes extra space from
the persisted storage.

### Test plan
- changed the test that checks merging of pages with stand by
transactions
- added test to check if the stand by transaction was correctly removed
if it appeared in the pagination data

### Related issues

- Relates to RET-1207

### Backwards compatibility
Yes

### 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)

src/transactions/actions.ts

@@ -19,6 +19,7 @@ export enum Actions {
   TRANSACTION_CONFIRMED = 'TRANSACTIONS/TRANSACTION_CONFIRMED',
   REFRESH_RECENT_TX_RECIPIENTS = 'TRANSACTIONS/REFRESH_RECENT_TX_RECIPIENTS',
   UPDATE_TRANSACTIONS = 'TRANSACTIONS/UPDATE_TRANSACTIONS',
+  REMOVE_DUPLICATED_STANDBY_TRANSACTIONS = 'TRANSACTIONS/REMOVE_DUPLICATED_STANDBY_TRANSACTIONS',
 }
 
 type BaseStandbyTransactionType<T> = Omit<PendingStandbyTransaction<T>, 'timestamp' | 'status'>
@@ -59,10 +60,16 @@ export interface UpdateTransactionsAction {
   networkId: NetworkId
 }
 
+interface RemoveDuplicatedStandByTransactionsAction {
+  type: Actions.REMOVE_DUPLICATED_STANDBY_TRANSACTIONS
+  newPageTransactions: TokenTransaction[]
+}
+
 export type ActionTypes =
   | AddStandbyTransactionAction
   | UpdateTransactionsAction
   | TransactionConfirmedAction
+  | RemoveDuplicatedStandByTransactionsAction
 
 export const addStandbyTransaction = (
   transaction: BaseStandbyTransaction
@@ -90,3 +97,10 @@ export const updateTransactions = (
   networkId,
   transactions,
 })
+
+export const removeDuplicatedStandByTransactions = (
+  newPageTransactions: TokenTransaction[]
+): RemoveDuplicatedStandByTransactionsAction => ({
+  type: Actions.REMOVE_DUPLICATED_STANDBY_TRANSACTIONS,
+  newPageTransactions,
+})

src/transactions/feed/TransactionFeedV2.test.tsx

@@ -218,26 +218,6 @@ describe('TransactionFeedV2', () => {
     expect(getNumTransactionItems(tree.getByTestId('TransactionList'))).toBe(2)
   })
 
-  it('tries to fetch a page of transactions, and stores empty pages', async () => {
-    mockFetch
-      .mockResponseOnce(typedResponse({ transactions: [mockTransaction()] }))
-      .mockResponseOnce(typedResponse({ transactions: [] }))
-
-    const { store, ...tree } = renderScreen()
-
-    await store.dispatch(
-      transactionFeedV2Api.endpoints.transactionFeedV2.initiate({ address: '0x00', endCursor: 0 })
-    )
-    await store.dispatch(
-      transactionFeedV2Api.endpoints.transactionFeedV2.initiate({ address: '0x00', endCursor: 123 })
-    )
-
-    await waitFor(() => tree.getByTestId('TransactionList'))
-
-    await waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2))
-    expect(getNumTransactionItems(tree.getByTestId('TransactionList'))).toBe(1)
-  })
-
   it('renders GetStarted if SHOW_GET_STARTED is enabled and transaction feed is empty', async () => {
     jest.mocked(getFeatureGate).mockReturnValue(true)
     const tree = renderScreen()
@@ -275,10 +255,6 @@ describe('TransactionFeedV2', () => {
       },
     })
 
-    await store.dispatch(
-      transactionFeedV2Api.endpoints.transactionFeedV2.initiate({ address: '0x00', endCursor: 0 })
-    )
-
     await waitFor(() => {
       expect(tree.getByTestId('TransactionList').props.data.length).toBe(2)
     })
@@ -308,16 +284,32 @@ describe('TransactionFeedV2', () => {
           mockTransaction({ transactionHash: '0x20', timestamp: 20 }), // not in scope
           mockTransaction({ transactionHash: '0x30', timestamp: 30 }), // in scope
           mockTransaction({ transactionHash: '0x40', timestamp: 40 }), // in scope
-          mockTransaction({ transactionHash: '0x50', timestamp: 50 }), // not in scope
+          /**
+           * this is the latest transactions which means that it will be outside of the scope
+           * of the max timestamp of the first page. But if it is the first page of the feed -
+           * it should also be merged in as zerion still might not include it in the response
+           * for some time.
+           */
+          mockTransaction({ transactionHash: '0x50', timestamp: 50 }), // in scope
         ],
       },
     })
 
-    await store.dispatch(
-      transactionFeedV2Api.endpoints.transactionFeedV2.initiate({ address: '0x00', endCursor: 0 })
-    )
-
     await waitFor(() => tree.getByTestId('TransactionList'))
-    expect(getNumTransactionItems(tree.getByTestId('TransactionList'))).toBe(6)
+    expect(getNumTransactionItems(tree.getByTestId('TransactionList'))).toBe(7)
+  })
+
+  it('cleanup is triggered for confirmed stand by transactions', async () => {
+    mockFetch.mockResponse(typedResponse({ transactions: [mockTransaction()] }))
+    const { store } = renderScreen({
+      transactions: { standbyTransactions: [mockTransaction()] },
+    })
+
+    /**
+     * For now, there's no way to check for dispatched actions via getActions as we usually do
+     * as the current setupApiStore doesn't return it. But at least we can make sure that the
+     * transaction gets removed.
+     */
+    await waitFor(() => expect(store.getState().transactions.standbyTransactions.length).toBe(0))
   })
 })

src/transactions/feed/TransactionFeedV2.tsx

@@ -2,12 +2,13 @@ import React, { useEffect, useMemo, useState } from 'react'
 import { ActivityIndicator, SectionList, StyleSheet, View } from 'react-native'
 import SectionHead from 'src/components/SectionHead'
 import GetStarted from 'src/home/GetStarted'
-import { useSelector } from 'src/redux/hooks'
+import { useDispatch, useSelector } from 'src/redux/hooks'
 import { getFeatureGate, getMultichainFeatures } from 'src/statsig'
 import { StatsigFeatureGates } from 'src/statsig/types'
 import colors from 'src/styles/colors'
 import { Spacing } from 'src/styles/styles'
 import NoActivity from 'src/transactions/NoActivity'
+import { removeDuplicatedStandByTransactions } from 'src/transactions/actions'
 import { useTransactionFeedV2Query } from 'src/transactions/api'
 import EarnFeedItem from 'src/transactions/feed/EarnFeedItem'
 import NftFeedItem from 'src/transactions/feed/NftFeedItem'
@@ -95,18 +96,32 @@ function sortTransactions(transactions: TokenTransaction[]): TokenTransaction[]
  * Otherwise, if we merge all the stand by transactins into the page it will cause more late transactions
  * that were already merged to be removed from the top of the list and move them to the bottom.
  * This will cause the screen to "shift", which we're trying to avoid.
+ *
+ * Note: when merging the first page – stand by transactions might include some new pending transaction.
+ * In order to include them in the merged list we need to also check if the stand by transaction is newer
+ * than the max timestamp from the page. But this must only happen for the first page as otherwise any
+ * following page would include stand by transactions from previous pages.
  */
-function mergeStandByTransactionsInRange(
-  transactions: TokenTransaction[],
-  standBy: TokenTransaction[]
-): TokenTransaction[] {
+function mergeStandByTransactionsInRange({
+  transactions,
+  standByTransactions,
+  currentCursor,
+}: {
+  transactions: TokenTransaction[]
+  standByTransactions: TokenTransaction[]
+  currentCursor?: number
+}): TokenTransaction[] {
   if (transactions.length === 0) return []
 
   const allowedNetworks = getAllowedNetworksForTransfers()
   const max = transactions[0].timestamp
   const min = transactions.at(-1)!.timestamp
 
-  const standByInRange = standBy.filter((tx) => tx.timestamp >= min && tx.timestamp <= max)
+  const standByInRange = standByTransactions.filter((tx) => {
+    const inRange = tx.timestamp >= min && tx.timestamp <= max
+    const newTransaction = currentCursor === FIRST_PAGE_TIMESTAMP && tx.timestamp > max
+    return inRange || newTransaction
+  })
   const deduplicatedTransactions = deduplicateTransactions([...transactions, ...standByInRange])
   const transactionsFromAllowedNetworks = deduplicatedTransactions.filter((tx) =>
     allowedNetworks.includes(tx.networkId)
@@ -116,8 +131,8 @@ function mergeStandByTransactionsInRange(
 }
 
 /**
- * Current implementation of allStandbyTransactionsSelector contains function
- * getSupportedNetworkIdsForApprovalTxsInHomefeed in its selectors list which triggers a lot of
+ * 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
  *
@@ -175,6 +190,7 @@ function renderItem({ item: tx }: { item: TokenTransaction }) {
 }
 
 export default function TransactionFeedV2() {
+  const dispatch = useDispatch()
   const address = useSelector(walletAddressSelector)
   const standByTransactions = useStandByTransactions()
   const [endCursor, setEndCursor] = useState(FIRST_PAGE_TIMESTAMP)
@@ -248,10 +264,11 @@ export default function TransactionFeedV2() {
           prev[currentCursor] === undefined // data for this page wasn't stored yet
 
         if (isFirstPage || pageDataIsAbsent) {
-          const mergedTransactions = mergeStandByTransactionsInRange(
+          const mergedTransactions = mergeStandByTransactionsInRange({
             transactions,
-            standByTransactions.confirmed
-          )
+            standByTransactions: standByTransactions.confirmed,
+            currentCursor,
+          })
 
           return { ...prev, [currentCursor!]: mergedTransactions }
         }
@@ -262,6 +279,22 @@ export default function TransactionFeedV2() {
     [isFetching, data?.transactions, originalArgs?.endCursor, standByTransactions.confirmed]
   )
 
+  /**
+   * In order to avoid bloating stand by transactions with confirmed transactions that are already
+   * present in the feed via pagination – we need to cleanup them up. This must run for every page
+   * as standByTransaction selector might include very old transactions. We should use the chance
+   * whenever the user managed to scroll to those old transactions and remove them from persisted
+   * storage. Maybe there is a better way to keep it clean with another saga watcher?
+   */
+  useEffect(
+    function cleanupStandByTransactions() {
+      if (data?.transactions.length) {
+        dispatch(removeDuplicatedStandByTransactions(data.transactions))
+      }
+    },
+    [data?.transactions]
+  )
+
   const confirmedTransactions = useMemo(() => {
     const flattenedPages = Object.values(paginatedData).flat()
     const deduplicatedTransactions = deduplicateTransactions(flattenedPages)

src/transactions/reducer.ts

@@ -162,6 +162,26 @@ export const reducer = (
         },
         standbyTransactions: updatedStandbyTransactions,
       }
+
+    case Actions.REMOVE_DUPLICATED_STANDBY_TRANSACTIONS:
+      const confirmedTransactionsFromNewPage = action.newPageTransactions
+        .filter((tx) => tx.status !== TransactionStatus.Pending)
+        .map((tx) => tx.transactionHash)
+
+      return {
+        ...state,
+        standbyTransactions: state.standbyTransactions.filter((tx) => {
+          /**
+           * - ignore empty hashes as there's no way to compare them
+           * - ignore pending as it should only affect confirmed transactions that are already
+           *   present in the paginated data
+           */
+          if (!tx.transactionHash || tx.status === TransactionStatus.Pending) return true
+
+          return !confirmedTransactionsFromNewPage.includes(tx.transactionHash)
+        }),
+      }
+
     default:
       return state
   }
@@ -206,7 +226,7 @@ export const confirmedStandbyTransactionsSelector = createSelector(
   }
 )
 
-export const transactionsByNetworkIdSelector = (state: RootState) =>
+const transactionsByNetworkIdSelector = (state: RootState) =>
   state.transactions.transactionsByNetworkId
 
 export const transactionsSelector = createSelector(