feat(transaction-pay-controller): Add shared EIP-7702 quote gas estimation#8145
feat(transaction-pay-controller): Add shared EIP-7702 quote gas estimation#8145pedronfigueiredo wants to merge 1 commit intomainfrom
Conversation
c658ed5 to
7b400b2
Compare
7b400b2 to
49aa199
Compare
cryptotavares
left a comment
There was a problem hiding this comment.
Reviewing as @cryptotavares's AI assistant
Overall this is a clean refactor — the shared estimator logic is well-structured, fallbacks are correctly wired, and the batch/individual split in estimateQuoteGasLimits is sound. A few things worth addressing before merge:
Bug: Missing chainId fallback in getAcrossOrderedTransactions
File: src/strategy/across/transactions.ts + src/strategy/across/across-quotes.ts
getAcrossOrderedTransactions spreads approval transactions directly from the Across API response:
const approvalTransactions = (quote.approvalTxns ?? []).map((approval) => ({
...approval,
kind: 'approval' as const,
type: TransactionType.tokenMethodApprove,
}));If the Across API omits chainId on an approval transaction (which the old code explicitly guarded against with quote.approvalTxns?.[index]?.chainId ?? swapTx.chainId), then transaction.chainId will be undefined at runtime despite the type saying number. The gas estimation call in calculateSourceNetworkCost then does:
chainId: toHex(transaction.chainId), // toHex(undefined) → likely '0x0'This would target the wrong chain for gas estimation. The cost calculation later in calculateSourceNetworkCost still has the correct fallback (quote.approvalTxns?.[index]?.chainId ?? swapTx.chainId), but that doesn't help if estimation ran against chain 0x0.
The old fallback was intentional. Either:
- Add the fallback inside
getAcrossOrderedTransactions(passswapTx.chainIdand useapproval.chainId ?? swapChainId), or - Add it at the call site in
calculateSourceNetworkCostwhen building thetransactionsarray
Logic concern: useBuffer skips buffer when batch API echoes provided gas
File: src/utils/quote-gas.ts (~L105)
const useBuffer =
gasLimits.length === 1 || paramGasLimits[index] !== gasLimit;When estimateGasBatch returns per-transaction results (gasLimits.length === transactions.length), useBuffer is false if the batch API returns exactly the value that was passed in via transaction.gas. The intent seems to be "if the chain just echoed back our provided value, don't add a buffer on top." That's a reasonable interpretation, but paramGasLimits[index] is the parsed input gas; if the batch API happens to estimate the same number that was provided (not just echo it), buffer would also be skipped. Is that intentional? Worth adding a comment to clarify.
Minor: combinePostQuoteGas EIP-7702 detection heuristic
File: src/strategy/relay/relay-quotes.ts (~L480)
// TODO: Test EIP-7702 support on the chain as well before assuming single gas limit.
const isEIP7702 = gasLimits.length === 1;A single-step Relay quote (one transaction, individual estimation) also produces gasLimits.length === 1, so this misidentifies it as a 7702 batch and combines the original tx gas into a single limit. I see this has a TODO already — just flagging it as something that becomes more load-bearing now that the shared estimator is in use for both strategies.
One merge blocker (the chainId fallback regression), one question (buffer logic intent), one pre-existing TODO that's worth tracking. Happy to look at a follow-up once the chainId question is resolved.
Explanation
Add a shared quote gas estimator that switches between per-transaction and EIP-7702 batch estimation, reuses Across’s ordered submission transaction list at quote time, and falls back to the single-transaction path when batch estimation is unsupported or fails.
References
https://github.com/MetaMask/MetaMask-planning/issues/7098
Checklist
Note
Medium Risk
Touches gas estimation and transaction submission paths (including optional EIP-7702 batching), which can affect quoted fees and on-chain execution if limits are miscomputed. Mitigated by fallbacks to per-transaction estimation and expanded test coverage for batch/failed/partial-data scenarios.
Overview
Adds a shared quote gas estimator (
estimateQuoteGasLimits) that chooses between per-transaction estimation and EIP-7702estimateGasBatchon supported chains, applies gas buffers, preserves provided gas limits, and falls back when batching is unsupported, fails, or returns unexpected results.Across strategy now reuses this estimator by building an ordered approval+swap transaction list (
getAcrossOrderedTransactions), emitting either a single combinedmetamask.gasLimits.batch(for 7702 batching) or per-transactionapproval/swaplimits, and tightening error handling when swap gas is missing.Across submit supports 7702 batches when quotes include a combined batch gas limit (omitting per-tx
gasand passinggasLimit7702/batch flags), and transaction ordering/type assignment is unified via the shared ordered-transaction helper.Relay quoting switches to the shared estimator and adds defensive handling for incomplete relay item params by reusing/placeholdering missing fields to keep estimation working without enabling batching.
Written by Cursor Bugbot for commit 49aa199. This will update automatically on new commits. Configure here.