Skip to content

Commit bc8c0dd

Browse files
feat: query only latest page for incoming transactions (#5983)
## Explanation Query only the latest page of transaction history from the accounts API. In order to minimise load, improve performance, and better align with the transaction state limit. ## References ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 574b8b8 commit bc8c0dd

File tree

8 files changed

+28
-399
lines changed

8 files changed

+28
-399
lines changed

packages/transaction-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414

1515
### Changed
1616

17+
- Query only latest page of transactions from accounts API ([#5983](https://github.com/MetaMask/core/pull/5983))
1718
- Remove incoming transactions when calling `wipeTransactions` ([#5986](https://github.com/MetaMask/core/pull/5986))
1819
- Poll immediately when calling `startIncomingTransactionPolling` ([#5986](https://github.com/MetaMask/core/pull/5986))
1920

packages/transaction-controller/src/TransactionController.test.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4982,24 +4982,6 @@ describe('TransactionController', () => {
49824982
});
49834983
});
49844984

4985-
describe('on incoming transaction helper updateCache call', () => {
4986-
it('updates state', async () => {
4987-
const { controller } = setupController();
4988-
const key = 'testKey';
4989-
const value = 123;
4990-
4991-
incomingTransactionHelperClassMock.mock.calls[0][0].updateCache(
4992-
(cache) => {
4993-
cache[key] = value;
4994-
},
4995-
);
4996-
4997-
expect(controller.state.lastFetchedBlockNumbers).toStrictEqual({
4998-
[key]: value,
4999-
});
5000-
});
5001-
});
5002-
50034985
describe('updateTransactionGasFees', () => {
50044986
it('throws if transaction does not exist', async () => {
50054987
const { controller } = setupController();

packages/transaction-controller/src/TransactionController.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ export type TransactionControllerOptions = {
352352

353353
/** Configuration options for incoming transaction support. */
354354
incomingTransactions?: IncomingTransactionOptions & {
355-
/** API keys to be used for Etherscan requests to prevent rate limiting. */
355+
/** @deprecated Ignored as Etherscan no longer used. */
356356
etherscanApiKeysByChainId?: Record<Hex, string>;
357357
};
358358

@@ -969,25 +969,16 @@ export class TransactionController extends BaseController<
969969
},
970970
);
971971

972-
const updateCache = (fn: (cache: Record<string, unknown>) => void) => {
973-
this.update((_state) => {
974-
fn(_state.lastFetchedBlockNumbers);
975-
});
976-
};
977-
978972
this.#incomingTransactionHelper = new IncomingTransactionHelper({
979973
client: this.#incomingTransactionOptions.client,
980-
getCache: () => this.state.lastFetchedBlockNumbers,
981974
getCurrentAccount: () => this.#getSelectedAccount(),
982975
getLocalTransactions: () => this.state.transactions,
983976
includeTokenTransfers:
984977
this.#incomingTransactionOptions.includeTokenTransfers,
985978
isEnabled: this.#incomingTransactionOptions.isEnabled,
986979
messenger: this.messagingSystem,
987-
queryEntireHistory: this.#incomingTransactionOptions.queryEntireHistory,
988980
remoteTransactionSource: new AccountsApiRemoteTransactionSource(),
989981
trimTransactions: this.#trimTransactionsForState.bind(this),
990-
updateCache,
991982
updateTransactions: this.#incomingTransactionOptions.updateTransactions,
992983
});
993984

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

Lines changed: 1 addition & 168 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,10 @@ jest.useFakeTimers();
1818
const ADDRESS_MOCK = '0x123';
1919
const ONE_DAY_MS = 1000 * 60 * 60 * 24;
2020
const NOW_MOCK = 789000 + ONE_DAY_MS;
21-
const CURSOR_MOCK = 'abcdef';
22-
const CACHED_TIMESTAMP_MOCK = 456;
23-
const INITIAL_TIMESTAMP_MOCK = 789;
2421

2522
const REQUEST_MOCK: RemoteTransactionSourceRequest = {
2623
address: ADDRESS_MOCK,
27-
cache: {},
2824
includeTokenTransfers: true,
29-
queryEntireHistory: true,
30-
updateCache: jest.fn(),
3125
updateTransactions: true,
3226
};
3327

@@ -140,60 +134,10 @@ describe('AccountsApiRemoteTransactionSource', () => {
140134
expect(getAccountTransactionsMock).toHaveBeenCalledWith({
141135
address: ADDRESS_MOCK,
142136
chainIds: SUPPORTED_CHAIN_IDS,
143-
cursor: undefined,
144-
sortDirection: 'ASC',
137+
sortDirection: 'DESC',
145138
});
146139
});
147140

148-
it('queries accounts API with start timestamp if queryEntireHistory is false', async () => {
149-
await new AccountsApiRemoteTransactionSource().fetchTransactions({
150-
...REQUEST_MOCK,
151-
queryEntireHistory: false,
152-
});
153-
154-
expect(getAccountTransactionsMock).toHaveBeenCalledTimes(1);
155-
expect(getAccountTransactionsMock).toHaveBeenCalledWith(
156-
expect.objectContaining({
157-
startTimestamp: INITIAL_TIMESTAMP_MOCK,
158-
}),
159-
);
160-
});
161-
162-
it('queries accounts API with cursor from cache', async () => {
163-
await new AccountsApiRemoteTransactionSource().fetchTransactions({
164-
...REQUEST_MOCK,
165-
cache: {
166-
[`accounts-api#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
167-
CURSOR_MOCK,
168-
},
169-
});
170-
171-
expect(getAccountTransactionsMock).toHaveBeenCalledTimes(1);
172-
expect(getAccountTransactionsMock).toHaveBeenCalledWith(
173-
expect.objectContaining({
174-
cursor: CURSOR_MOCK,
175-
}),
176-
);
177-
});
178-
179-
it('queries accounts API with timestamp from cache', async () => {
180-
await new AccountsApiRemoteTransactionSource().fetchTransactions({
181-
...REQUEST_MOCK,
182-
queryEntireHistory: false,
183-
cache: {
184-
[`accounts-api#timestamp#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
185-
CACHED_TIMESTAMP_MOCK,
186-
},
187-
});
188-
189-
expect(getAccountTransactionsMock).toHaveBeenCalledTimes(1);
190-
expect(getAccountTransactionsMock).toHaveBeenCalledWith(
191-
expect.objectContaining({
192-
startTimestamp: CACHED_TIMESTAMP_MOCK,
193-
}),
194-
);
195-
});
196-
197141
it('returns normalized standard transaction', async () => {
198142
getAccountTransactionsMock.mockResolvedValue({
199143
data: [RESPONSE_STANDARD_MOCK],
@@ -222,117 +166,6 @@ describe('AccountsApiRemoteTransactionSource', () => {
222166
expect(transactions).toStrictEqual([TRANSACTION_TOKEN_TRANSFER_MOCK]);
223167
});
224168

225-
it('queries multiple times if response has next page', async () => {
226-
getAccountTransactionsMock
227-
.mockResolvedValueOnce({
228-
data: [RESPONSE_STANDARD_MOCK],
229-
pageInfo: { hasNextPage: true, count: 1, cursor: CURSOR_MOCK },
230-
})
231-
.mockResolvedValueOnce({
232-
data: [RESPONSE_STANDARD_MOCK],
233-
pageInfo: { hasNextPage: true, count: 1, cursor: CURSOR_MOCK },
234-
});
235-
236-
await new AccountsApiRemoteTransactionSource().fetchTransactions(
237-
REQUEST_MOCK,
238-
);
239-
240-
expect(getAccountTransactionsMock).toHaveBeenCalledTimes(3);
241-
expect(getAccountTransactionsMock).toHaveBeenNthCalledWith(
242-
1,
243-
expect.objectContaining({ cursor: undefined }),
244-
);
245-
expect(getAccountTransactionsMock).toHaveBeenNthCalledWith(
246-
2,
247-
expect.objectContaining({ cursor: CURSOR_MOCK }),
248-
);
249-
expect(getAccountTransactionsMock).toHaveBeenNthCalledWith(
250-
3,
251-
expect.objectContaining({ cursor: CURSOR_MOCK }),
252-
);
253-
});
254-
255-
it('updates cache if response has cursor', async () => {
256-
getAccountTransactionsMock
257-
.mockResolvedValueOnce({
258-
data: [RESPONSE_STANDARD_MOCK],
259-
pageInfo: { hasNextPage: true, count: 1, cursor: CURSOR_MOCK },
260-
})
261-
.mockResolvedValueOnce({
262-
data: [RESPONSE_STANDARD_MOCK],
263-
pageInfo: { hasNextPage: true, count: 1, cursor: CURSOR_MOCK },
264-
});
265-
266-
const cacheMock = {};
267-
268-
const updateCacheMock = jest
269-
.fn()
270-
.mockImplementation((fn) => fn(cacheMock));
271-
272-
await new AccountsApiRemoteTransactionSource().fetchTransactions({
273-
...REQUEST_MOCK,
274-
updateCache: updateCacheMock,
275-
});
276-
277-
expect(updateCacheMock).toHaveBeenCalledTimes(2);
278-
expect(cacheMock).toStrictEqual({
279-
[`accounts-api#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
280-
CURSOR_MOCK,
281-
});
282-
});
283-
284-
it('removes timestamp cache entry if response has cursor', async () => {
285-
getAccountTransactionsMock.mockResolvedValueOnce({
286-
data: [RESPONSE_STANDARD_MOCK],
287-
pageInfo: { hasNextPage: false, count: 1, cursor: CURSOR_MOCK },
288-
});
289-
290-
const cacheMock = {
291-
[`accounts-api#timestamp#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
292-
CACHED_TIMESTAMP_MOCK,
293-
};
294-
295-
const updateCacheMock = jest
296-
.fn()
297-
.mockImplementation((fn) => fn(cacheMock));
298-
299-
await new AccountsApiRemoteTransactionSource().fetchTransactions({
300-
...REQUEST_MOCK,
301-
updateCache: updateCacheMock,
302-
});
303-
304-
expect(updateCacheMock).toHaveBeenCalledTimes(1);
305-
expect(cacheMock).toStrictEqual({
306-
[`accounts-api#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
307-
CURSOR_MOCK,
308-
});
309-
});
310-
311-
it('updates cache with timestamp if response does not have cursor', async () => {
312-
getAccountTransactionsMock.mockResolvedValueOnce({
313-
data: [],
314-
pageInfo: { hasNextPage: false, count: 0, cursor: undefined },
315-
});
316-
317-
const cacheMock = {};
318-
319-
const updateCacheMock = jest
320-
.fn()
321-
.mockImplementation((fn) => fn(cacheMock));
322-
323-
await new AccountsApiRemoteTransactionSource().fetchTransactions({
324-
...REQUEST_MOCK,
325-
queryEntireHistory: false,
326-
updateCache: updateCacheMock,
327-
});
328-
329-
expect(updateCacheMock).toHaveBeenCalledTimes(1);
330-
expect(cacheMock).toStrictEqual({
331-
[`accounts-api#timestamp#${SUPPORTED_CHAIN_IDS.join(',')}#${ADDRESS_MOCK}`]:
332-
INITIAL_TIMESTAMP_MOCK,
333-
});
334-
});
335-
336169
it('ignores outgoing transactions if updateTransactions is false', async () => {
337170
getAccountTransactionsMock.mockResolvedValue({
338171
data: [{ ...RESPONSE_STANDARD_MOCK, to: '0x456' }],

0 commit comments

Comments
 (0)