Add gasFeeEstimates for batch transactions
#5886
Conversation
…a publish batch hook
Co-authored-by: Matthew Walsh <[email protected]>
…:MetaMask/core into feat/add-batch-transaction-approval-type
…-estimate-gas-batch-transaction
…-estimate-gas-batch-transaction
…-estimate-gas-batch-transaction
…estimate-gas-fee-batch-tx
vinistevam
changed the title
gasFeeEstimates for batch transactionsgasFeeEstimates for batch transactions
| status: TransactionStatus; | ||
|
|
||
| /** When the transaction was created. */ | ||
| time: number; |
There was a problem hiding this comment.
Rather than adding these purely to support the DefaultGasFeeFlow, could we instead just use mock values at the point of calling? To minimise the complexity here as much as possible?
There was a problem hiding this comment.
Sure, I removed the time property, I'm keeping status to filter it in the next PR and filter unapproved transactionBatches in the GasFeePoller.
| }, | ||
| }); | ||
|
|
||
| txBatchMeta.gasFeeEstimates = gasFeeResponse.estimates; |
There was a problem hiding this comment.
The nature of gas fee estimates is they change per block, so should this ticket also update the GasFeePoller to update the batch metadata overtime, or is that a separate dedicated ticket?
There was a problem hiding this comment.
Yeah, we split into two tickets, and I'm currently working on it on a separate PR.
| }); | ||
|
|
||
| const gasFeeResponse = await defaultGasFeeFlow.getGasFees({ | ||
| ethQuery: getEthQuery(networkClientId), |
There was a problem hiding this comment.
Minor, should we define ethQuery as a variable as we'd likely need it in future, and helps readability?
| @@ -1450,6 +1504,8 @@ describe('Batch Utils', () => { | |||
|
|
|||
| const result = await resultPromise; | |||
| expect(result?.batchId).toMatch(/^0x[0-9a-f]{32}$/u); | |||
| expect(simulateGasBatchMock).toHaveBeenCalledTimes(1); | |||
| expect(getGasFeesMock).toHaveBeenCalledTimes(1); | |||
There was a problem hiding this comment.
Should this warrant dedicated separate unit tests to verify that gas limit and gas fees are actually set on the batch metadata in the state?
There was a problem hiding this comment.
I've created a dedicated test for updating gas properties.
vinistevam
dismissed stale reviews from OGPoyraz and pedronfigueiredo
via
316f3d8
Explanation
This PR enhances
TransactionBatchMetaby adding support forgasFeeEstimates,time, andstatus.When both
useHookandrequireApprovalaretrue, the batch approval flow is triggered, andgasFeeEstimatesare populated using theDefaultGasFeeFlow. These estimates are then passed along with the request to be consumed by the client.Changes
TransactionBatchMetato include:gasFeeEstimatesstatusprepareApprovalDataReferences
Changelog
Checklist