Merge pull request #1535 from input-output-hk/fix/transaction-tracker-process-transactions-in-correct-order

fix(wallet): transaction tracker now process transaction in chronological order

Author: rhyslbw

packages/wallet/src/services/TransactionsTracker.ts

@@ -77,6 +77,14 @@ export interface TransactionsTrackerInternalsProps {
 // Temporarily hardcoded. Will be replaced with ChainHistoryProvider 'maxPageSize' value once ADP-2249 is implemented
 export const PAGE_SIZE = 25;
 
+/**
+ * Sorts the given HydratedTx by slot.
+ *
+ * @param lhs The left-hand side of the comparison operation.
+ * @param rhs The left-hand side of the comparison operation.
+ */
+const sortTxBySlot = (lhs: Cardano.HydratedTx, rhs: Cardano.HydratedTx) => lhs.blockHeader.slot - rhs.blockHeader.slot;
+
 const allTransactionsByAddresses = async (
   chainHistoryProvider: ChainHistoryProvider,
   { addresses, blockRange }: { addresses: Cardano.PaymentAddress[]; blockRange: Range<Cardano.BlockNo> }
@@ -102,7 +110,7 @@ const allTransactionsByAddresses = async (
     } while (pageResults.length === PAGE_SIZE);
   }
 
-  return response;
+  return response.sort(sortTxBySlot);
 };
 
 const getLastTransactionsAtBlock = (
@@ -120,7 +128,9 @@ const getLastTransactionsAtBlock = (
     }
   }
 
-  return txsFromSameBlock;
+  // Since we are traversing the array in reverse to find the transactions for this block,
+  // we must reverse the result.
+  return txsFromSameBlock.reverse();
 };
 
 export const revertLastBlock = (
@@ -176,7 +186,7 @@ const findIntersectionAndUpdateTxStore = ({
     combinator: exhaustMap,
     equals: transactionsEquals,
     onFatalError,
-    // eslint-disable-next-line sonarjs/cognitive-complexity
+    // eslint-disable-next-line sonarjs/cognitive-complexity,complexity
     provider: async () => {
       let rollbackOcurred = false;
       // eslint-disable-next-line no-constant-condition
@@ -219,12 +229,13 @@ const findIntersectionAndUpdateTxStore = ({
 
         const localTxsFromSameBlock = getLastTransactionsAtBlock(localTransactions, lowerBound);
         const firstSegmentOfNewTransactions = newTransactions.slice(0, localTxsFromSameBlock.length);
+        const hasSameLength = localTxsFromSameBlock.length === firstSegmentOfNewTransactions.length;
 
         // The first segment of new transaction should match exactly (same txs and same order) our last know TXs. Otherwise
         // roll them back and re-apply in new order.
-        const sameTxAndOrder = localTxsFromSameBlock.every(
-          (tx, index) => tx.id === firstSegmentOfNewTransactions[index].id
-        );
+        const sameTxAndOrder =
+          hasSameLength &&
+          localTxsFromSameBlock.every((tx, index) => tx.id === firstSegmentOfNewTransactions[index].id);
 
         if (!sameTxAndOrder) {
           localTransactions = revertLastBlock(localTransactions, lowerBound, rollback$, newTransactions, logger);

packages/wallet/test/services/TransactionsTracker.test.ts

@@ -31,7 +31,7 @@ const { generateTxAlonzo, mockChainHistoryProvider, queryTransactionsResult, que
 const updateTransactionsBlockNo = (transactions: Cardano.HydratedTx[], blockNo = Cardano.BlockNo(10_050)) =>
   transactions.map((tx) => ({
     ...tx,
-    blockHeader: { ...tx.blockHeader, blockNo }
+    blockHeader: { ...tx.blockHeader, blockNo, slot: Cardano.Slot(0) }
   }));
 
 const updateTransactionIds = (transactions: Cardano.HydratedTx[], tailPattern = 'aaa') =>
@@ -343,6 +343,13 @@ describe('TransactionsTracker', () => {
           Cardano.BlockNo(10_051)
         );
 
+        txId1.blockHeader.slot = Cardano.Slot(10_050);
+        txId2.blockHeader.slot = Cardano.Slot(10_051);
+        txId3.blockHeader.slot = Cardano.Slot(10_052);
+
+        txId1OtherBlock.blockHeader.slot = Cardano.Slot(10_050);
+        txId2OtherBlock.blockHeader.slot = Cardano.Slot(10_051);
+
         await firstValueFrom(store.setAll([txId1, txId2]));
 
         chainHistoryProvider.transactionsByAddresses = jest.fn(() => ({
@@ -364,11 +371,11 @@ describe('TransactionsTracker', () => {
 
         expect(await firstValueFrom(provider$.pipe(bufferCount(2)))).toEqual([
           [txId1, txId2], // from store
-          [txId1OtherBlock, txId2OtherBlock, txId3] // chain history
+          [txId1, txId2, txId3] // chain history
         ]);
         expect(rollbacks.length).toBe(0);
         expect(store.setAll).toBeCalledTimes(2);
-        expect(store.setAll).nthCalledWith(2, [txId1OtherBlock, txId2OtherBlock, txId3]);
+        expect(store.setAll).nthCalledWith(2, [txId1, txId2, txId3]);
       });
 
       // latestStoredBlock  <1 2 3>
@@ -380,8 +387,16 @@ describe('TransactionsTracker', () => {
           queryTransactionsResult2.pageResults,
           Cardano.BlockNo(10_050)
         );
+
+        txId1.blockHeader.slot = Cardano.Slot(10_050);
+        txId2.blockHeader.slot = Cardano.Slot(10_051);
+        txId3.blockHeader.slot = Cardano.Slot(10_052);
+
         const [txId1OtherBlock, txId2OtherBlock] = updateTransactionsBlockNo([txId1, txId2], Cardano.BlockNo(10_051));
 
+        txId1OtherBlock.blockHeader.slot = Cardano.Slot(10_051);
+        txId2OtherBlock.blockHeader.slot = Cardano.Slot(10_052);
+
         await firstValueFrom(store.setAll([txId1, txId2, txId3]));
 
         chainHistoryProvider.transactionsByAddresses = jest.fn(() => ({
@@ -496,6 +511,43 @@ describe('TransactionsTracker', () => {
         expect(store.setAll).nthCalledWith(2, [txId3OtherBlock, txId2OtherBlock, txId1OtherBlock]);
       });
 
+      it('process transactions in the right order (sorted by slot ASC) regardless of transaction order in the backend response', async () => {
+        const [txId1, txId2, txId3] = updateTransactionsBlockNo(
+          queryTransactionsResult2.pageResults,
+          Cardano.BlockNo(10_000)
+        );
+
+        txId1.blockHeader.slot = Cardano.Slot(10_000);
+        txId2.blockHeader.slot = Cardano.Slot(10_001);
+        txId3.blockHeader.slot = Cardano.Slot(10_002);
+
+        await firstValueFrom(store.setAll([txId1, txId2, txId3]));
+
+        chainHistoryProvider.transactionsByAddresses = jest.fn(() => ({
+          pageResults: [txId3, txId2, txId1],
+          totalResultCount: 3
+        }));
+
+        const { transactionsSource$: provider$, rollback$ } = createAddressTransactionsProvider({
+          addresses$: of(addresses),
+          chainHistoryProvider,
+          logger,
+          retryBackoffConfig,
+          store,
+          tipBlockHeight$
+        });
+
+        const rollbacks: Cardano.HydratedTx[] = [];
+        rollback$.subscribe((tx) => rollbacks.push(tx));
+
+        expect(await firstValueFrom(provider$.pipe(bufferCount(1)))).toEqual([
+          [txId1, txId2, txId3] // chain history
+        ]);
+        expect(rollbacks.length).toBe(0);
+        expect(store.setAll).toBeCalledTimes(1);
+        expect(store.setAll).nthCalledWith(1, [txId1, txId2, txId3]);
+      });
+
       // latestStoredBlock  <1 2 3>
       // newBlock           <1 2 3>
       // rollback$          none