Skip to content

feat(transaction-pay-controller): Add shared EIP-7702 quote gas estimation#8145

Merged
pedronfigueiredo merged 3 commits intomainfrom
pnf/cor-across-metric
Mar 19, 2026
Merged

feat(transaction-pay-controller): Add shared EIP-7702 quote gas estimation#8145
pedronfigueiredo merged 3 commits intomainfrom
pnf/cor-across-metric

Conversation

@pedronfigueiredo
Copy link
Contributor

@pedronfigueiredo pedronfigueiredo commented Mar 9, 2026

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

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Refactors gas estimation and submission logic for both Relay and Across quotes, including new EIP-7702 batch-handling paths; incorrect limits could impact fee calculation or batch execution behavior.

Overview
Adds a shared estimateQuoteGasLimits utility that centralizes quote-time gas estimation across strategies, using estimateGasBatch for multi-transaction quotes (including EIP-7702 combined single-limit batches) and falling back to per-transaction estimation when appropriate.

Updates Across and Relay quoting to use this shared estimator, introduces a consistent is7702 flag in quote metadata, and reshapes gas limit data (Across now returns an ordered list instead of { approval, swap }). Submission paths are updated to honor is7702 by omitting per-tx gas when using gasLimit7702, with stricter erroring when required limits/fields are missing.

Adds getAcrossOrderedTransactions to reuse the same approval+swap ordering for estimation and submit, and expands test coverage for batch estimation, missing fields, and 7702-specific behaviors.

Written by Cursor Bugbot for commit 845e1c3. This will update automatically on new commits. Configure here.

@pedronfigueiredo pedronfigueiredo self-assigned this Mar 9, 2026
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner March 9, 2026 11:30
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner March 9, 2026 11:32
Copy link
Contributor

@cryptotavares cryptotavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (pass swapTx.chainId and use approval.chainId ?? swapChainId), or
  • Add it at the call site in calculateSourceNetworkCost when building the transactions array

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.

jpuri
jpuri previously approved these changes Mar 11, 2026
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/cor-across-metric branch 2 times, most recently from ec0ec98 to e0c0649 Compare March 11, 2026 18:16
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/cor-across-metric branch 2 times, most recently from 5317383 to 8a2e24a Compare March 11, 2026 18:39
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@pedronfigueiredo pedronfigueiredo force-pushed the pnf/cor-across-metric branch 3 times, most recently from c0225fd to 14f1189 Compare March 16, 2026 12:42
matthewwalsh0
matthewwalsh0 previously approved these changes Mar 17, 2026
requiredParams.from === undefined ||
requiredParams.to === undefined
) {
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, I meant let it throw naturally rather than explicitly checking each field.

const gasEstimate = gasEstimates.gasLimits[index];

if (!gasEstimate) {
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, is this possible since earlier gas estimation would of thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, removed

const batchGasLimit =
is7702 && allParams.length > 1 ? gasLimits[0] : undefined;

if (is7702 && allParams.length > 1 && batchGasLimit === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, is this duplication since we already have is7702 set?

Could we enforce in the type that is7702 guarantees one entry?

matthewwalsh0
matthewwalsh0 previously approved these changes Mar 18, 2026
@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Mar 19, 2026
Merged via the queue into main with commit a7f72f2 Mar 19, 2026
322 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/cor-across-metric branch March 19, 2026 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants