Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: gas estimation for type 4 transactions #5519

Merged
merged 5 commits into from
Mar 24, 2025

Conversation

matthewwalsh0
Copy link
Member

@matthewwalsh0 matthewwalsh0 commented Mar 20, 2025

Explanation

The gas for type 4 transactions that include data to the upgraded account can only be estimated using eth_estimateGas if the real signature properties are included, otherwise the upgraded address cannot be known since it is derived from the signature itself.

As we don't want to sign an authorisation until the user approves the transaction, we will instead:

  1. Estimate only the upgrade, with no data and a dummy signature, using eth_estimateGas.
  2. Estimate the data only on the resulting upgraded EOA using the simulation API to override the account code.
  3. Add the two values together, and subtract the intrinsic gas cost (21000).

References

Fixes #31140

Changelog

See CHANGELOG.md.

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 highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@matthewwalsh0
Copy link
Member Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "27.0.0-preview-3e15260f",
  "@metamask-previews/address-book-controller": "6.0.3-preview-3e15260f",
  "@metamask-previews/announcement-controller": "7.0.3-preview-3e15260f",
  "@metamask-previews/approval-controller": "7.1.3-preview-3e15260f",
  "@metamask-previews/assets-controllers": "55.0.0-preview-3e15260f",
  "@metamask-previews/base-controller": "8.0.0-preview-3e15260f",
  "@metamask-previews/bridge-controller": "9.0.0-preview-3e15260f",
  "@metamask-previews/bridge-status-controller": "9.0.0-preview-3e15260f",
  "@metamask-previews/build-utils": "3.0.3-preview-3e15260f",
  "@metamask-previews/chain-agnostic-permission": "0.1.0-preview-3e15260f",
  "@metamask-previews/composable-controller": "11.0.0-preview-3e15260f",
  "@metamask-previews/controller-utils": "11.6.0-preview-3e15260f",
  "@metamask-previews/earn-controller": "0.9.0-preview-3e15260f",
  "@metamask-previews/eip1193-permission-middleware": "0.1.0-preview-3e15260f",
  "@metamask-previews/ens-controller": "16.0.0-preview-3e15260f",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-3e15260f",
  "@metamask-previews/gas-fee-controller": "23.0.0-preview-3e15260f",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-3e15260f",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-3e15260f",
  "@metamask-previews/keyring-controller": "21.0.0-preview-3e15260f",
  "@metamask-previews/logging-controller": "6.0.4-preview-3e15260f",
  "@metamask-previews/message-manager": "12.0.1-preview-3e15260f",
  "@metamask-previews/multichain": "4.0.0-preview-3e15260f",
  "@metamask-previews/multichain-api-middleware": "0.1.0-preview-3e15260f",
  "@metamask-previews/multichain-network-controller": "0.3.0-preview-3e15260f",
  "@metamask-previews/multichain-transactions-controller": "0.8.0-preview-3e15260f",
  "@metamask-previews/name-controller": "8.0.3-preview-3e15260f",
  "@metamask-previews/network-controller": "23.0.0-preview-3e15260f",
  "@metamask-previews/notification-services-controller": "5.0.0-preview-3e15260f",
  "@metamask-previews/permission-controller": "11.0.6-preview-3e15260f",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-3e15260f",
  "@metamask-previews/phishing-controller": "12.4.1-preview-3e15260f",
  "@metamask-previews/polling-controller": "13.0.0-preview-3e15260f",
  "@metamask-previews/preferences-controller": "17.0.0-preview-3e15260f",
  "@metamask-previews/profile-sync-controller": "11.0.0-preview-3e15260f",
  "@metamask-previews/queued-request-controller": "10.0.0-preview-3e15260f",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-3e15260f",
  "@metamask-previews/remote-feature-flag-controller": "1.6.0-preview-3e15260f",
  "@metamask-previews/sample-controllers": "0.0.0-preview-3e15260f",
  "@metamask-previews/selected-network-controller": "22.0.0-preview-3e15260f",
  "@metamask-previews/signature-controller": "27.0.0-preview-3e15260f",
  "@metamask-previews/token-search-discovery-controller": "2.1.0-preview-3e15260f",
  "@metamask-previews/transaction-controller": "51.0.0-preview-3e15260f",
  "@metamask-previews/user-operation-controller": "30.0.0-preview-3e15260f"
}

@matthewwalsh0
Copy link
Member Author

@metamaskbot publish-preview

@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review March 21, 2025 11:16
@matthewwalsh0 matthewwalsh0 requested review from a team as code owners March 21, 2025 11:16
@matthewwalsh0 matthewwalsh0 force-pushed the fix/type-4-gas-estimation branch from fcc3894 to e3c959f Compare March 21, 2025 11:20
@matthewwalsh0
Copy link
Member Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "27.0.0-preview-3e231aec",
  "@metamask-previews/address-book-controller": "6.0.3-preview-3e231aec",
  "@metamask-previews/announcement-controller": "7.0.3-preview-3e231aec",
  "@metamask-previews/approval-controller": "7.1.3-preview-3e231aec",
  "@metamask-previews/assets-controllers": "55.0.0-preview-3e231aec",
  "@metamask-previews/base-controller": "8.0.0-preview-3e231aec",
  "@metamask-previews/bridge-controller": "9.0.0-preview-3e231aec",
  "@metamask-previews/bridge-status-controller": "9.0.0-preview-3e231aec",
  "@metamask-previews/build-utils": "3.0.3-preview-3e231aec",
  "@metamask-previews/chain-agnostic-permission": "0.1.0-preview-3e231aec",
  "@metamask-previews/composable-controller": "11.0.0-preview-3e231aec",
  "@metamask-previews/controller-utils": "11.6.0-preview-3e231aec",
  "@metamask-previews/earn-controller": "0.9.0-preview-3e231aec",
  "@metamask-previews/eip1193-permission-middleware": "0.1.0-preview-3e231aec",
  "@metamask-previews/ens-controller": "16.0.0-preview-3e231aec",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-3e231aec",
  "@metamask-previews/gas-fee-controller": "23.0.0-preview-3e231aec",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-3e231aec",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-3e231aec",
  "@metamask-previews/keyring-controller": "21.0.0-preview-3e231aec",
  "@metamask-previews/logging-controller": "6.0.4-preview-3e231aec",
  "@metamask-previews/message-manager": "12.0.1-preview-3e231aec",
  "@metamask-previews/multichain": "4.0.0-preview-3e231aec",
  "@metamask-previews/multichain-api-middleware": "0.1.0-preview-3e231aec",
  "@metamask-previews/multichain-network-controller": "0.3.0-preview-3e231aec",
  "@metamask-previews/multichain-transactions-controller": "0.8.0-preview-3e231aec",
  "@metamask-previews/name-controller": "8.0.3-preview-3e231aec",
  "@metamask-previews/network-controller": "23.0.0-preview-3e231aec",
  "@metamask-previews/notification-services-controller": "5.0.0-preview-3e231aec",
  "@metamask-previews/permission-controller": "11.0.6-preview-3e231aec",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-3e231aec",
  "@metamask-previews/phishing-controller": "12.4.1-preview-3e231aec",
  "@metamask-previews/polling-controller": "13.0.0-preview-3e231aec",
  "@metamask-previews/preferences-controller": "17.0.0-preview-3e231aec",
  "@metamask-previews/profile-sync-controller": "11.0.0-preview-3e231aec",
  "@metamask-previews/queued-request-controller": "10.0.0-preview-3e231aec",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-3e231aec",
  "@metamask-previews/remote-feature-flag-controller": "1.6.0-preview-3e231aec",
  "@metamask-previews/sample-controllers": "0.0.0-preview-3e231aec",
  "@metamask-previews/selected-network-controller": "22.0.0-preview-3e231aec",
  "@metamask-previews/signature-controller": "27.0.0-preview-3e231aec",
  "@metamask-previews/token-search-discovery-controller": "2.1.0-preview-3e231aec",
  "@metamask-previews/transaction-controller": "51.0.0-preview-3e231aec",
  "@metamask-previews/user-operation-controller": "30.0.0-preview-3e231aec"
}

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "27.0.0-preview-e3c959f",
  "@metamask-previews/address-book-controller": "6.0.3-preview-e3c959f",
  "@metamask-previews/announcement-controller": "7.0.3-preview-e3c959f",
  "@metamask-previews/approval-controller": "7.1.3-preview-e3c959f",
  "@metamask-previews/assets-controllers": "55.0.1-preview-e3c959f",
  "@metamask-previews/base-controller": "8.0.0-preview-e3c959f",
  "@metamask-previews/bridge-controller": "10.0.0-preview-e3c959f",
  "@metamask-previews/bridge-status-controller": "10.0.0-preview-e3c959f",
  "@metamask-previews/build-utils": "3.0.3-preview-e3c959f",
  "@metamask-previews/chain-agnostic-permission": "0.1.0-preview-e3c959f",
  "@metamask-previews/composable-controller": "11.0.0-preview-e3c959f",
  "@metamask-previews/controller-utils": "11.6.0-preview-e3c959f",
  "@metamask-previews/earn-controller": "0.9.0-preview-e3c959f",
  "@metamask-previews/eip1193-permission-middleware": "0.1.0-preview-e3c959f",
  "@metamask-previews/ens-controller": "16.0.0-preview-e3c959f",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-e3c959f",
  "@metamask-previews/gas-fee-controller": "23.0.0-preview-e3c959f",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-e3c959f",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-e3c959f",
  "@metamask-previews/keyring-controller": "21.0.0-preview-e3c959f",
  "@metamask-previews/logging-controller": "6.0.4-preview-e3c959f",
  "@metamask-previews/message-manager": "12.0.1-preview-e3c959f",
  "@metamask-previews/multichain": "4.0.0-preview-e3c959f",
  "@metamask-previews/multichain-api-middleware": "0.1.0-preview-e3c959f",
  "@metamask-previews/multichain-network-controller": "0.3.0-preview-e3c959f",
  "@metamask-previews/multichain-transactions-controller": "0.8.0-preview-e3c959f",
  "@metamask-previews/name-controller": "8.0.3-preview-e3c959f",
  "@metamask-previews/network-controller": "23.0.0-preview-e3c959f",
  "@metamask-previews/notification-services-controller": "5.0.0-preview-e3c959f",
  "@metamask-previews/permission-controller": "11.0.6-preview-e3c959f",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-e3c959f",
  "@metamask-previews/phishing-controller": "12.4.1-preview-e3c959f",
  "@metamask-previews/polling-controller": "13.0.0-preview-e3c959f",
  "@metamask-previews/preferences-controller": "17.0.0-preview-e3c959f",
  "@metamask-previews/profile-sync-controller": "11.0.0-preview-e3c959f",
  "@metamask-previews/queued-request-controller": "10.0.0-preview-e3c959f",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-e3c959f",
  "@metamask-previews/remote-feature-flag-controller": "1.6.0-preview-e3c959f",
  "@metamask-previews/sample-controllers": "0.0.0-preview-e3c959f",
  "@metamask-previews/selected-network-controller": "22.0.0-preview-e3c959f",
  "@metamask-previews/signature-controller": "27.0.0-preview-e3c959f",
  "@metamask-previews/token-search-discovery-controller": "2.1.0-preview-e3c959f",
  "@metamask-previews/transaction-controller": "52.0.0-preview-e3c959f",
  "@metamask-previews/user-operation-controller": "31.0.0-preview-e3c959f"
}

@matthewwalsh0 matthewwalsh0 requested a review from OGPoyraz March 21, 2025 16:19
@matthewwalsh0 matthewwalsh0 enabled auto-merge (squash) March 24, 2025 08:07
@matthewwalsh0 matthewwalsh0 force-pushed the fix/type-4-gas-estimation branch from e3c959f to 4d533bc Compare March 24, 2025 08:07
@matthewwalsh0 matthewwalsh0 merged commit 3b5b35b into main Mar 24, 2025
193 checks passed
@matthewwalsh0 matthewwalsh0 deleted the fix/type-4-gas-estimation branch March 24, 2025 08:12
matthewwalsh0 added a commit that referenced this pull request Mar 25, 2025
The gas for type 4 transactions that include `data` to the upgraded
account can only be estimated using `eth_estimateGas` if the real
signature properties are included, otherwise the upgraded address cannot
be known since it is derived from the signature itself.

As we don't want to sign an authorisation until the user approves the
transaction, we will instead:

1. Estimate only the upgrade, with no data and a dummy signature, using
`eth_estimateGas`.
2. Estimate the data only on the resulting upgraded EOA using the
simulation API to override the account code.
3. Add the two values together, and subtract the intrinsic gas cost
(`21000`).

Fixes
[#31140](MetaMask/metamask-extension#31140)

See `CHANGELOG.md`.

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
matthewwalsh0 added a commit to MetaMask/metamask-extension that referenced this pull request Mar 25, 2025
## **Description**

Cherry-pick of #31250 for `12.15.0`.

Using patch due to unrelated changes in
`@metamask/transaction-controller` release.

Patch is an unchanged cherry-pick of
[#5519](MetaMask/core#5519).

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31264?quickstart=1)

## **Related issues**

## **Manual testing steps**

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
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.

[Bug]: Gas estimation fails for EIP-7702 upgrades
3 participants