Skip to content

fix(transaction-controller): use node-authoritative gas estimation - #10245

Open
pedronfigueiredo wants to merge 3 commits into
mainfrom
pnf/node-estimation-conf-1995
Open

pedronfigueiredo wants to merge 3 commits into
mainfrom
pnf/node-estimation-conf-1995

Conversation

@pedronfigueiredo

@pedronfigueiredo pedronfigueiredo commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Explanation

Transaction Controller currently skips eth_estimateGas for ordinary no-data transfers to recipients without code and assigns a fixed 21,000 gas limit. That assumption is not valid across transaction shapes and active fork rules, including EIP-2780 networks, and cannot account for node state such as account creation or delegation resolution.

This change removes the fixed-gas shortcut so transactions without caller-provided gas use the selected network client's existing node/simulation estimation path. Caller-provided gas remains authoritative, and existing percentage/fixed-feature fallbacks plus simulationFails reporting are preserved when estimation fails.

Specifically, this changes gas-limit estimation, not gas-price estimation. For an ordinary transaction without caller-provided gas, Transaction Controller calls eth_estimateGas on the node selected by networkClientId and may apply the existing gas buffer. gasPrice, maxFeePerGas, and maxPriorityFeePerGas continue to come from the existing gas-fee estimation flows; the eventual transaction fee combines those prices with the node-estimated gas limit.

The focused test matrix covers legacy and upgraded node responses for self-transfers, zero-value transfers, existing and new recipients, contract creation, and delegated sender/recipient cases. It also verifies complete transaction-shape forwarding and estimation failure behavior. Existing type-4 composition remains unchanged because it is owned by CONF-1997.

Validation

  • yarn workspace @metamask/transaction-controller run test
  • yarn build
  • yarn workspace @metamask/transaction-controller run build
  • Targeted ESLint and Oxfmt checks
  • yarn workspace @metamask/transaction-controller run changelog:validate

References

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
Changes default gas limits for many wallet-initiated sends, which affects fees and inclusion; wrong estimates could cause reverts, though fallbacks and explicit gas remain.

Overview
Gas limit estimation no longer short-circuits plain no-data transfers to empty recipients with a hard-coded 0x5208 (21,000) limit. The removed requiresFixedGas / eth_getCode path and FIXED_GAS export are gone; when the caller does not set gas, updateGas always goes through estimateGas on the networkClientId client (simulation when enabled, otherwise eth_estimateGas), with existing buffers and failure fallbacks unchanged.

Caller-provided gas is still used as-is. Gas price flows are untouched. Tests and integration mocks now expect eth_estimateGas for simple sends and cover varied shapes (self-transfer, new accounts, contract creation, EIP-7702 delegation). The changelog documents the behavior change.

Reviewed by Cursor Bugbot for commit ab91ca6. Bugbot is set up for automated code reviews on this repo. Configure here.

@pedronfigueiredo
pedronfigueiredo marked this pull request as ready for review September 15, 2026 13:56
@pedronfigueiredo
pedronfigueiredo requested review from a team as code owners September 15, 2026 13:56
@matthewwalsh0
matthewwalsh0 self-requested a review September 15, 2026 14:04
matthewwalsh0
matthewwalsh0 previously approved these changes Sep 15, 2026
@pedronfigueiredo
pedronfigueiredo added this pull request to the merge queue Sep 15, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 15, 2026
@pedronfigueiredo
pedronfigueiredo added this pull request to the merge queue Sep 15, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 15, 2026
…onf-1995

# Conflicts:
#	packages/transaction-controller/CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants