fix: cache incoming transaction timestamps

packages/transaction-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix incoming transaction support with `queryEntireHistory` set to `false` ([#5582](https://github.com/MetaMask/core/pull/5582))
+
 ## [54.0.0]
 
 ### Added

packages/transaction-controller/src/helpers/AccountsApiRemoteTransactionSource.test.ts

@@ -14,8 +14,11 @@ jest.mock('../api/accounts-api');
 jest.useFakeTimers();
 
 const ADDRESS_MOCK = '0x123';
-const NOW_MOCK = 789000;
+const ONE_DAY_MS = 1000 * 60 * 60 * 24;
+const NOW_MOCK = 789000 + ONE_DAY_MS;
 const CURSOR_MOCK = 'abcdef';
+const CACHED_TIMESTAMP_MOCK = 456;
+const INITIAL_TIMESTAMP_MOCK = 789;
 
 const REQUEST_MOCK: RemoteTransactionSourceRequest = {
   address: ADDRESS_MOCK,
@@ -141,7 +144,7 @@ describe('AccountsApiRemoteTransactionSource', () => {
       expect(getAccountTransactionsMock).toHaveBeenCalledTimes(1);
       expect(getAccountTransactionsMock).toHaveBeenCalledWith(
         expect.objectContaining({
-          startTimestamp: 789,
+          startTimestamp: INITIAL_TIMESTAMP_MOCK,
         }),
       );
     });
@@ -163,6 +166,24 @@ describe('AccountsApiRemoteTransactionSource', () => {
       );
     });
 
+    it('queries accounts API with timestamp from cache', async () => {
+      await new AccountsApiRemoteTransactionSource().fetchTransactions({
+        ...REQUEST_MOCK,
+        queryEntireHistory: false,
+        cache: {
+          [`accounts-api#timestamp#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
+            CACHED_TIMESTAMP_MOCK,
+        },
+      });
+
+      expect(getAccountTransactionsMock).toHaveBeenCalledTimes(1);
+      expect(getAccountTransactionsMock).toHaveBeenCalledWith(
+        expect.objectContaining({
+          startTimestamp: CACHED_TIMESTAMP_MOCK,
+        }),
+      );
+    });
+
     it('returns normalized standard transaction', async () => {
       getAccountTransactionsMock.mockResolvedValue({
         data: [RESPONSE_STANDARD_MOCK],
@@ -250,6 +271,58 @@ describe('AccountsApiRemoteTransactionSource', () => {
       });
     });
 
+    it('removes timestamp cache entry if response has cursor', async () => {
+      getAccountTransactionsMock.mockResolvedValueOnce({
+        data: [RESPONSE_STANDARD_MOCK],
+        pageInfo: { hasNextPage: false, count: 1, cursor: CURSOR_MOCK },
+      });
+
+      const cacheMock = {
+        [`accounts-api#timestamp#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
+          CACHED_TIMESTAMP_MOCK,
+      };
+
+      const updateCacheMock = jest
+        .fn()
+        .mockImplementation((fn) => fn(cacheMock));
+
+      await new AccountsApiRemoteTransactionSource().fetchTransactions({
+        ...REQUEST_MOCK,
+        updateCache: updateCacheMock,
+      });
+
+      expect(updateCacheMock).toHaveBeenCalledTimes(1);
+      expect(cacheMock).toStrictEqual({
+        [`accounts-api#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
+          CURSOR_MOCK,
+      });
+    });
+
+    it('updates cache with timestamp if response does not have cursor', async () => {
+      getAccountTransactionsMock.mockResolvedValueOnce({
+        data: [],
+        pageInfo: { hasNextPage: false, count: 0, cursor: undefined },
+      });
+
+      const cacheMock = {};
+
+      const updateCacheMock = jest
+        .fn()
+        .mockImplementation((fn) => fn(cacheMock));
+
+      await new AccountsApiRemoteTransactionSource().fetchTransactions({
+        ...REQUEST_MOCK,
+        queryEntireHistory: false,
+        updateCache: updateCacheMock,
+      });
+
+      expect(updateCacheMock).toHaveBeenCalledTimes(1);
+      expect(cacheMock).toStrictEqual({
+        [`accounts-api#timestamp#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
+          INITIAL_TIMESTAMP_MOCK,
+      });
+    });
+
     it('ignores outgoing transactions if updateTransactions is false', async () => {
       getAccountTransactionsMock.mockResolvedValue({
         data: [{ ...RESPONSE_STANDARD_MOCK, to: '0x456' }],

packages/transaction-controller/src/helpers/AccountsApiRemoteTransactionSource.ts

@@ -18,6 +18,8 @@ import type {
 } from '../types';
 import { TransactionStatus, TransactionType } from '../types';
 
+const RECENT_HISTORY_DURATION_MS = 1000 * 60 * 60 * 24; // 1 Day
+
 export const SUPPORTED_CHAIN_IDS: Hex[] = [
   CHAIN_IDS.MAINNET,
   CHAIN_IDS.POLYGON,
@@ -84,32 +86,49 @@ export class AccountsApiRemoteTransactionSource
 
     const cursor = this.#getCacheCursor(cache, SUPPORTED_CHAIN_IDS, address);
 
+    const timestamp = this.#getCacheTimestamp(
+      cache,
+      SUPPORTED_CHAIN_IDS,
+      address,
+    );
+
     if (cursor) {
       log('Using cached cursor', cursor);
+    } else if (timestamp) {
+      log('Using cached timestamp', timestamp);
+    } else {
+      log('No cached cursor or timestamp found');
     }
 
-    return await this.#queryTransactions(request, SUPPORTED_CHAIN_IDS, cursor);
+    return await this.#queryTransactions(
+      request,
+      SUPPORTED_CHAIN_IDS,
+      cursor,
+      timestamp,
+    );
   }
 
   async #queryTransactions(
     request: RemoteTransactionSourceRequest,
     chainIds: Hex[],
     cursor?: string,
+    timestamp?: number,
   ): Promise<TransactionResponse[]> {
-    const { address, queryEntireHistory, updateCache } = request;
+    const { address, queryEntireHistory } = request;
     const transactions: TransactionResponse[] = [];
 
     let hasNextPage = true;
     let currentCursor = cursor;
     let pageCount = 0;
 
-    const startTimestamp =
-      queryEntireHistory || cursor
-        ? undefined
-        : this.#getTimestampSeconds(Date.now());
-
     while (hasNextPage) {
       try {
+        const startTimestamp = this.#getStartTimestamp({
+          cursor: currentCursor,
+          queryEntireHistory,
+          timestamp,
+        });
+
         const response = await getAccountTransactions({
           address,
           chainIds,
@@ -127,15 +146,12 @@ export class AccountsApiRemoteTransactionSource
         hasNextPage = response?.pageInfo?.hasNextPage;
         currentCursor = response?.pageInfo?.cursor;
 
-        if (currentCursor) {
-          // eslint-disable-next-line no-loop-func
-          updateCache((cache) => {
-            const key = this.#getCacheKey(chainIds, address);
-            cache[key] = currentCursor;
-
-            log('Updated cache', { key, newCursor: currentCursor });
-          });
-        }
+        this.#updateCache({
+          chainIds,
+          cursor: currentCursor,
+          request,
+          startTimestamp,
+        });
       } catch (error) {
         log('Error while fetching transactions', error);
         break;
@@ -248,7 +264,64 @@ export class AccountsApiRemoteTransactionSource
     };
   }
 
-  #getCacheKey(chainIds: Hex[], address: Hex): string {
+  #updateCache({
+    chainIds,
+    cursor,
+    request,
+    startTimestamp,
+  }: {
+    chainIds: Hex[];
+    cursor?: string;
+    request: RemoteTransactionSourceRequest;
+    startTimestamp?: number;
+  }) {
+    if (!cursor && !startTimestamp) {
+      log('Cache not updated');
+      return;
+    }
+
+    const { address, updateCache } = request;
+    const cursorCacheKey = this.#getCursorCacheKey(chainIds, address);
+    const timestampCacheKey = this.#getTimestampCacheKey(chainIds, address);
+
+    updateCache((cache) => {
+      if (cursor) {
+        cache[cursorCacheKey] = cursor;
+        delete cache[timestampCacheKey];
+
+        log('Updated cursor in cache', { cursorCacheKey, newCursor: cursor });
+      } else {
+        cache[timestampCacheKey] = startTimestamp;
+
+        log('Updated timestamp in cache', {
+          timestampCacheKey,
+          newTimestamp: startTimestamp,
+        });
+      }
+    });
+  }
+
+  #getStartTimestamp({
+    cursor,
+    queryEntireHistory,
+    timestamp,
+  }: {
+    cursor?: string;
+    queryEntireHistory: boolean;
+    timestamp?: number;
+  }): number | undefined {
+    if (queryEntireHistory || cursor) {
+      return undefined;
+    }
+
+    if (timestamp) {
+      return timestamp;
+    }
+
+    return this.#getTimestampSeconds(Date.now() - RECENT_HISTORY_DURATION_MS);
+  }
+
+  #getCursorCacheKey(chainIds: Hex[], address: Hex): string {
     return `accounts-api#${chainIds.join(',')}#${address}`;
   }
 
@@ -257,10 +330,23 @@ export class AccountsApiRemoteTransactionSource
     chainIds: Hex[],
     address: Hex,
   ): string | undefined {
-    const key = this.#getCacheKey(chainIds, address);
+    const key = this.#getCursorCacheKey(chainIds, address);
     return cache[key] as string | undefined;
   }
 
+  #getTimestampCacheKey(chainIds: Hex[], address: Hex): string {
+    return `accounts-api#timestamp#${chainIds.join(',')}#${address}`;
+  }
+
+  #getCacheTimestamp(
+    cache: Record<string, unknown>,
+    chainIds: Hex[],
+    address: Hex,
+  ): number | undefined {
+    const key = this.#getTimestampCacheKey(chainIds, address);
+    return cache[key] as number | undefined;
+  }
+
   #getTimestampSeconds(timestampMs: number): number {
     return Math.floor(timestampMs / 1000);
   }

packages/transaction-controller/src/types.ts

@@ -922,7 +922,7 @@ export interface RemoteTransactionSourceRequest {
   address: Hex;
 
   /**
-   * Numerical cache to optimize fetching transactions.
+   * Cache to optimize fetching transactions.
    */
   cache: Record<string, unknown>;