Skip to content

Commit 5907d59

Browse files
committed
fix: set submittedTime at pay publish hook start for accurate time-to-complete metrics
1 parent 231dd13 commit 5907d59

File tree

6 files changed

+115
-2
lines changed

6 files changed

+115
-2
lines changed

packages/transaction-controller/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Add optional `atomic` property to `TransactionBatchRequest` to configure whether EIP-7702 batch calls revert together or can fail independently ([#8320](https://github.com/MetaMask/core/pull/8320))
1313

14+
### Changed
15+
16+
- Preserve `submittedTime` if already set by a publish hook instead of overwriting it ([#8439](https://github.com/MetaMask/core/pull/8439))
17+
1418
## [64.1.0]
1519

1620
### Added

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2903,6 +2903,51 @@ describe('TransactionController', () => {
29032903
});
29042904
});
29052905

2906+
it('preserves submittedTime if already set by publish hook', async () => {
2907+
const presetSubmittedTime = 1000000;
2908+
2909+
const { controller } = setupController({
2910+
options: {
2911+
hooks: {
2912+
publish: async (txMeta) => {
2913+
const existing = controller.state.transactions.find(
2914+
(transaction) => transaction.id === txMeta.id,
2915+
);
2916+
2917+
if (existing) {
2918+
controller.updateTransaction(
2919+
{ ...existing, submittedTime: presetSubmittedTime },
2920+
'Set submittedTime in publish hook',
2921+
);
2922+
}
2923+
return { transactionHash: '0x123' };
2924+
},
2925+
},
2926+
},
2927+
messengerOptions: {
2928+
addTransactionApprovalRequest: {
2929+
state: 'approved',
2930+
},
2931+
},
2932+
});
2933+
2934+
const { result } = await controller.addTransaction(
2935+
{
2936+
from: ACCOUNT_MOCK,
2937+
to: ACCOUNT_MOCK,
2938+
},
2939+
{
2940+
networkClientId: NETWORK_CLIENT_ID_MOCK,
2941+
},
2942+
);
2943+
2944+
await result;
2945+
2946+
expect(controller.state.transactions[0].submittedTime).toBe(
2947+
presetSubmittedTime,
2948+
);
2949+
});
2950+
29062951
it('throws with data message if publish fails', async () => {
29072952
const { controller } = setupController({
29082953
messengerOptions: {

packages/transaction-controller/src/TransactionController.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3291,7 +3291,7 @@ export class TransactionController extends BaseController<
32913291
(draftTxMeta) => {
32923292
draftTxMeta.hash = hash;
32933293
draftTxMeta.status = TransactionStatus.submitted;
3294-
draftTxMeta.submittedTime = new Date().getTime();
3294+
draftTxMeta.submittedTime ??= new Date().getTime();
32953295
if (shouldUpdatePreTxBalance) {
32963296
draftTxMeta.preTxBalance = preTxBalance;
32973297
log('Updated pre-transaction balance', preTxBalance);

packages/transaction-pay-controller/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Bump `@metamask/ramps-controller` from `^13.0.0` to `^13.1.0` ([#8407](https://github.com/MetaMask/core/pull/8407))
1313
- Bump `@metamask/transaction-controller` from `^64.0.0` to `^64.1.0` ([#8432](https://github.com/MetaMask/core/pull/8432))
1414

15+
### Fixed
16+
17+
- Set `submittedTime` at the start of `TransactionPayPublishHook` before strategy execution for accurate `mm_pay_time_to_complete_s` metrics in intent-based flows ([#8439](https://github.com/MetaMask/core/pull/8439))
18+
1519
## [19.1.0]
1620

1721
### Added

packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,12 @@ describe('TransactionPayPublishHook', () => {
3030
const executeMock = jest.fn();
3131
const getStrategyByNameMock = jest.mocked(getStrategyByName);
3232

33-
const { messenger, getControllerStateMock } = getMessengerMock();
33+
const {
34+
messenger,
35+
getControllerStateMock,
36+
getTransactionControllerStateMock,
37+
updateTransactionMock,
38+
} = getMessengerMock();
3439

3540
let hook: TransactionPayPublishHook;
3641

@@ -65,6 +70,10 @@ describe('TransactionPayPublishHook', () => {
6570
},
6671
},
6772
} as TransactionPayControllerState);
73+
74+
getTransactionControllerStateMock.mockReturnValue({
75+
transactions: [TRANSACTION_META_MOCK],
76+
});
6877
});
6978

7079
it('executes strategy with quotes', async () => {
@@ -93,6 +102,45 @@ describe('TransactionPayPublishHook', () => {
93102
expect(executeMock).not.toHaveBeenCalled();
94103
});
95104

105+
it('sets submittedTime on the transaction before executing strategy', async () => {
106+
await runHook();
107+
108+
expect(updateTransactionMock).toHaveBeenCalledWith(
109+
expect.objectContaining({
110+
id: TRANSACTION_META_MOCK.id,
111+
submittedTime: expect.any(Number),
112+
}),
113+
'Set submittedTime at pay publish hook start',
114+
);
115+
});
116+
117+
it('sets submittedTime before strategy.execute is called', async () => {
118+
const callOrder: string[] = [];
119+
120+
updateTransactionMock.mockImplementation(() => {
121+
callOrder.push('updateTransaction');
122+
});
123+
124+
executeMock.mockImplementation(() => {
125+
callOrder.push('execute');
126+
return { transactionHash: '0x123' };
127+
});
128+
129+
await runHook();
130+
131+
expect(callOrder).toStrictEqual(['updateTransaction', 'execute']);
132+
});
133+
134+
it('does not set submittedTime if no quotes', async () => {
135+
getControllerStateMock.mockReturnValue({
136+
transactionData: {},
137+
});
138+
139+
await runHook();
140+
141+
expect(updateTransactionMock).not.toHaveBeenCalled();
142+
});
143+
96144
it('throws errors from submit', async () => {
97145
executeMock.mockRejectedValue(new Error('Test error'));
98146

packages/transaction-pay-controller/src/helpers/TransactionPayPublishHook.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
TransactionPayQuote,
1111
} from '../types';
1212
import { getStrategyByName } from '../utils/strategy';
13+
import { updateTransaction } from '../utils/transaction';
1314

1415
const log = createModuleLogger(projectLogger, 'pay-publish-hook');
1516

@@ -68,6 +69,17 @@ export class TransactionPayPublishHook {
6869
return EMPTY_RESULT;
6970
}
7071

72+
updateTransaction(
73+
{
74+
transactionId,
75+
messenger: this.#messenger,
76+
note: 'Set submittedTime at pay publish hook start',
77+
},
78+
(tx) => {
79+
tx.submittedTime = new Date().getTime();
80+
},
81+
);
82+
7183
const strategy = getStrategyByName(quotes[0].strategy);
7284

7385
return await strategy.execute({

0 commit comments

Comments
 (0)