Skip to content

Commit 8221a0d

Browse files
vinistevampedronfigueiredomatthewwalsh0OGPoyraz
authored
Extends GasFeePoller to update gas properties for unapproved transaction batches (#5950)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> This PR extends the `GasFeePoller` to track/update gas properties for unapproved `transactionBatches`. ## Changes The GasFeePoller now: - Detects unapproved `transactionBatches` via `#getUnapprovedTransactionBatches`. - Fetches gas estimates using `DefaultGasFeeFlow`. - Emits `transaction-batch-updated` events with updated `gasFeeEstimates`. Updates are applied to the `TransactionBatchMeta` for each batch, mirroring the behaviour already in place for single transactions. This enhancement ensures that unapproved transaction batches receive timely gas updates similar to individual transactions, improving consistency across transaction types. ## References Fixes MetaMask/MetaMask-planning#5090 ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## 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 - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --------- Co-authored-by: Pedro Figueiredo <[email protected]> Co-authored-by: Matthew Walsh <[email protected]> Co-authored-by: OGPoyraz <[email protected]>
1 parent 2454229 commit 8221a0d

File tree

6 files changed

+374
-10
lines changed

6 files changed

+374
-10
lines changed

‎packages/transaction-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
- Query only latest page of transactions from accounts API ([#5983](https://github.com/MetaMask/core/pull/5983))
1919
- Remove incoming transactions when calling `wipeTransactions` ([#5986](https://github.com/MetaMask/core/pull/5986))
2020
- Poll immediately when calling `startIncomingTransactionPolling` ([#5986](https://github.com/MetaMask/core/pull/5986))
21+
- Extend `GasFeePoller` to support gas updates for unapproved `transactionBatches`, emitting `transaction-batch-updated` with `gasFeeEstimates`. ([#5950](https://github.com/MetaMask/core/pull/5950))
2122

2223
## [58.0.0]
2324

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,7 @@ export class TransactionController extends BaseController<
941941
getGasFeeControllerEstimates: this.#getGasFeeEstimates,
942942
getProvider: (networkClientId) => this.#getProvider({ networkClientId }),
943943
getTransactions: () => this.state.transactions,
944+
getTransactionBatches: () => this.state.transactionBatches,
944945
layer1GasFeeFlows: this.#layer1GasFeeFlows,
945946
messenger: this.messagingSystem,
946947
onStateChange: (listener) => {
@@ -956,6 +957,11 @@ export class TransactionController extends BaseController<
956957
this.#onGasFeePollerTransactionUpdate.bind(this),
957958
);
958959

960+
gasFeePoller.hub.on(
961+
'transaction-batch-updated',
962+
this.#onGasFeePollerTransactionBatchUpdate.bind(this),
963+
);
964+
959965
this.#methodDataHelper = new MethodDataHelper({
960966
getProvider: (networkClientId) => this.#getProvider({ networkClientId }),
961967
getState: () => this.state.methodData,
@@ -4234,6 +4240,36 @@ export class TransactionController extends BaseController<
42344240
);
42354241
}
42364242

4243+
#onGasFeePollerTransactionBatchUpdate({
4244+
transactionBatchId,
4245+
gasFeeEstimates,
4246+
}: {
4247+
transactionBatchId: Hex;
4248+
gasFeeEstimates?: GasFeeEstimates;
4249+
}) {
4250+
this.#updateTransactionBatch(transactionBatchId, (batch) => {
4251+
return { ...batch, gasFeeEstimates };
4252+
});
4253+
}
4254+
4255+
#updateTransactionBatch(
4256+
batchId: string,
4257+
callback: (batch: TransactionBatchMeta) => TransactionBatchMeta | void,
4258+
): void {
4259+
this.update((state) => {
4260+
const index = state.transactionBatches.findIndex((b) => b.id === batchId);
4261+
4262+
if (index === -1) {
4263+
throw new Error(`Cannot update batch, ID not found - ${batchId}`);
4264+
}
4265+
4266+
const batch = state.transactionBatches[index];
4267+
const updated = callback(batch);
4268+
4269+
state.transactionBatches[index] = updated ?? batch;
4270+
});
4271+
}
4272+
42374273
#getSelectedAccount() {
42384274
return this.messagingSystem.call('AccountsController:getSelectedAccount');
42394275
}

‎packages/transaction-controller/src/helpers/GasFeePoller.test.ts

Lines changed: 184 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import EthQuery from '@metamask/eth-query';
12
import type { Provider } from '@metamask/network-controller';
23
import type { Hex } from '@metamask/utils';
34

@@ -7,8 +8,13 @@ import {
78
updateTransactionGasEstimates,
89
} from './GasFeePoller';
910
import { flushPromises } from '../../../../tests/helpers';
11+
import { DefaultGasFeeFlow } from '../gas-flows/DefaultGasFeeFlow';
1012
import type { TransactionControllerMessenger } from '../TransactionController';
11-
import type { GasFeeFlowResponse, Layer1GasFeeFlow } from '../types';
13+
import type {
14+
GasFeeFlowResponse,
15+
Layer1GasFeeFlow,
16+
TransactionBatchMeta,
17+
} from '../types';
1218
import {
1319
GasFeeEstimateLevel,
1420
GasFeeEstimateType,
@@ -44,6 +50,23 @@ const TRANSACTION_META_MOCK: TransactionMeta = {
4450
},
4551
};
4652

53+
const TRANSACTION_BATCH_META_MOCK: TransactionBatchMeta = {
54+
id: 'batch1',
55+
chainId: CHAIN_ID_MOCK,
56+
networkClientId: NETWORK_CLIENT_ID_MOCK,
57+
status: TransactionStatus.unapproved,
58+
transactions: [
59+
{
60+
gas: '0x5208',
61+
},
62+
{
63+
gas: '0x5208',
64+
},
65+
],
66+
gas: '0x10000',
67+
from: '0x123',
68+
};
69+
4770
const FEE_MARKET_GAS_FEE_ESTIMATES_MOCK = {
4871
type: GasFeeEstimateType.FeeMarket,
4972
[GasFeeEstimateLevel.Low]: {
@@ -93,6 +116,9 @@ describe('GasFeePoller', () => {
93116
let gasFeeFlowMock: jest.Mocked<GasFeeFlow>;
94117
let triggerOnStateChange: () => void;
95118
let getTransactionsMock: jest.MockedFunction<() => TransactionMeta[]>;
119+
let getTransactionBatchesMock: jest.MockedFunction<
120+
() => TransactionBatchMeta[]
121+
>;
96122
const getTransactionLayer1GasFeeMock = jest.mocked(
97123
getTransactionLayer1GasFee,
98124
);
@@ -113,13 +139,19 @@ describe('GasFeePoller', () => {
113139
getTransactionsMock = jest.fn();
114140
getTransactionsMock.mockReturnValue([{ ...TRANSACTION_META_MOCK }]);
115141

142+
getTransactionBatchesMock = jest.fn();
143+
getTransactionBatchesMock.mockReturnValue([
144+
{ ...TRANSACTION_BATCH_META_MOCK },
145+
]);
146+
116147
getTransactionLayer1GasFeeMock.mockResolvedValue(LAYER1_GAS_FEE_MOCK);
117148

118149
constructorOptions = {
119150
findNetworkClientIdByChainId: findNetworkClientIdByChainIdMock,
120151
gasFeeFlows: [gasFeeFlowMock],
121152
getGasFeeControllerEstimates: getGasFeeControllerEstimatesMock,
122153
getTransactions: getTransactionsMock,
154+
getTransactionBatches: getTransactionBatchesMock,
123155
layer1GasFeeFlows: layer1GasFeeFlowsMock,
124156
messenger: messengerMock,
125157
onStateChange: (listener: () => void) => {
@@ -212,6 +244,7 @@ describe('GasFeePoller', () => {
212244

213245
getTransactionsMock.mockReturnValueOnce([{ ...TRANSACTION_META_MOCK }]);
214246
getTransactionsMock.mockReturnValueOnce([]);
247+
getTransactionBatchesMock.mockReturnValue([]);
215248

216249
const gasFeePoller = new GasFeePoller(constructorOptions);
217250
gasFeePoller.hub.on('transaction-updated', listener);
@@ -254,6 +287,155 @@ describe('GasFeePoller', () => {
254287
triggerOnStateChange();
255288
await flushPromises();
256289

290+
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledTimes(4);
291+
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledWith({
292+
networkClientId: 'networkClientId1',
293+
});
294+
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledWith({
295+
networkClientId: 'networkClientId2',
296+
});
297+
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledWith({
298+
networkClientId: 'networkClientId4',
299+
});
300+
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledWith({
301+
networkClientId: NETWORK_CLIENT_ID_MOCK,
302+
});
303+
});
304+
});
305+
});
306+
307+
describe('if unapproved transaction batches', () => {
308+
let getGasFeesMock: jest.Mock;
309+
beforeEach(() => {
310+
getGasFeesMock = jest.fn().mockResolvedValue({
311+
estimates: FEE_MARKET_GAS_FEE_ESTIMATES_MOCK,
312+
});
313+
jest
314+
.spyOn(DefaultGasFeeFlow.prototype, 'getGasFees')
315+
.mockImplementation(getGasFeesMock);
316+
});
317+
318+
it('emits batch updated event', async () => {
319+
const listener = jest.fn();
320+
getTransactionsMock.mockReturnValue([]);
321+
const gasFeePoller = new GasFeePoller(constructorOptions);
322+
gasFeePoller.hub.on('transaction-batch-updated', listener);
323+
324+
triggerOnStateChange();
325+
await flushPromises();
326+
327+
expect(listener).toHaveBeenCalledTimes(1);
328+
expect(listener).toHaveBeenCalledWith({
329+
transactionBatchId: TRANSACTION_BATCH_META_MOCK.id,
330+
gasFeeEstimates: GAS_FEE_FLOW_RESPONSE_MOCK.estimates,
331+
gasFeeEstimatesLoaded: true,
332+
});
333+
});
334+
335+
it('calls gas fee flow for batches', async () => {
336+
getGasFeeControllerEstimatesMock.mockResolvedValue({});
337+
338+
new GasFeePoller(constructorOptions);
339+
340+
triggerOnStateChange();
341+
await flushPromises();
342+
343+
expect(gasFeeFlowMock.getGasFees).toHaveBeenCalledTimes(1);
344+
expect(gasFeeFlowMock.getGasFees).toHaveBeenCalledWith({
345+
ethQuery: expect.any(EthQuery),
346+
gasFeeControllerData: expect.any(Object),
347+
messenger: expect.any(Function),
348+
transactionMeta: {
349+
id: '1',
350+
chainId: TRANSACTION_BATCH_META_MOCK.chainId,
351+
networkClientId: TRANSACTION_BATCH_META_MOCK.networkClientId,
352+
status: TRANSACTION_BATCH_META_MOCK.status,
353+
time: expect.any(Number),
354+
txParams: {
355+
from: TRANSACTION_BATCH_META_MOCK.from,
356+
type: TransactionEnvelopeType.feeMarket,
357+
},
358+
},
359+
});
360+
});
361+
362+
it('creates polling timeout for batches', async () => {
363+
new GasFeePoller(constructorOptions);
364+
365+
triggerOnStateChange();
366+
await flushPromises();
367+
368+
expect(jest.getTimerCount()).toBe(1);
369+
370+
jest.runOnlyPendingTimers();
371+
await flushPromises();
372+
373+
expect(gasFeeFlowMock.getGasFees).toHaveBeenCalledTimes(2);
374+
});
375+
376+
it('does not create additional polling timeout on subsequent state changes', async () => {
377+
new GasFeePoller(constructorOptions);
378+
379+
triggerOnStateChange();
380+
await flushPromises();
381+
382+
triggerOnStateChange();
383+
await flushPromises();
384+
385+
expect(jest.getTimerCount()).toBe(1);
386+
});
387+
388+
it('does nothing if no transaction batches', async () => {
389+
const listener = jest.fn();
390+
391+
getTransactionsMock.mockReturnValue([]);
392+
getTransactionBatchesMock.mockReturnValueOnce([
393+
{ ...TRANSACTION_BATCH_META_MOCK },
394+
]);
395+
getTransactionBatchesMock.mockReturnValueOnce([]);
396+
397+
const gasFeePoller = new GasFeePoller(constructorOptions);
398+
gasFeePoller.hub.on('transaction-batch-updated', listener);
399+
400+
triggerOnStateChange();
401+
await flushPromises();
402+
403+
expect(listener).toHaveBeenCalledTimes(0);
404+
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledTimes(0);
405+
expect(gasFeeFlowMock.getGasFees).toHaveBeenCalledTimes(0);
406+
});
407+
408+
describe('fetches GasFeeController data for batches', () => {
409+
it('for each unique chain ID in batches', async () => {
410+
getTransactionsMock.mockReturnValue([]);
411+
getTransactionBatchesMock.mockReturnValue([
412+
{
413+
...TRANSACTION_BATCH_META_MOCK,
414+
chainId: '0x1',
415+
networkClientId: 'networkClientId1',
416+
},
417+
{
418+
...TRANSACTION_BATCH_META_MOCK,
419+
chainId: '0x2',
420+
networkClientId: 'networkClientId2',
421+
},
422+
{
423+
...TRANSACTION_BATCH_META_MOCK,
424+
chainId: '0x2',
425+
networkClientId: 'networkClientId3',
426+
},
427+
{
428+
...TRANSACTION_BATCH_META_MOCK,
429+
chainId: '0x3',
430+
networkClientId: 'networkClientId4',
431+
},
432+
]);
433+
434+
new GasFeePoller(constructorOptions);
435+
436+
triggerOnStateChange();
437+
await flushPromises();
438+
257439
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledTimes(3);
258440
expect(getGasFeeControllerEstimatesMock).toHaveBeenCalledWith({
259441
networkClientId: 'networkClientId1',
@@ -348,6 +530,7 @@ describe('GasFeePoller', () => {
348530
await flushPromises();
349531

350532
getTransactionsMock.mockReturnValue([]);
533+
getTransactionBatchesMock.mockReturnValue([]);
351534

352535
triggerOnStateChange();
353536
await flushPromises();

0 commit comments

Comments
 (0)