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

[Bug]: Intrinsic gas too low / Internal JSON-RPC / Intrinsic gas exceeds gas limit for Linea error when sending non-native tokens on all Popular networks #12612

Closed
Unik0rnMaggie opened this issue Dec 9, 2024 · 7 comments · Fixed by #12958 or #12962
Assignees
Labels
regression-main Regression bug that was found on main branch, but not yet present in production regression-RC-7.38.0 Regression bug that was found in release candidate (RC) for release 7.38.0 release-7.39.0 Issue or pull request that will be included in release 7.39.0 release-blocker This bug is blocking the next release Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-assets team-confirmations Push issues to confirmations team type-bug Something isn't working

Comments

@Unik0rnMaggie
Copy link
Contributor

Describe the bug

Error message when sending non-native tokens on Ethereum Mainnet:

Intrinsic gas too low: gas 21000, minimum needed 21620

Expected behavior

No error message

Screenshots/Recordings

Gas.limit.error.mov

Steps to reproduce

  1. Switch to Ethereum Mainnet
  2. send any token except ETH native token
  3. notice gas limit error

Error messages or log output

Detection stage

In beta

Version

7.36.0 (1505)

Build type

Other (please specify exactly where you obtained this build in "Additional Context" section)

Device

Pixel 6 Pro

Operating system

Android

Additional context

Latest 2 QA builds

Severity

No response

@Unik0rnMaggie Unik0rnMaggie added the type-bug Something isn't working label Dec 9, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Dec 9, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Dec 9, 2024
@Unik0rnMaggie Unik0rnMaggie changed the title [Bug]: Intrinsic gas too low error when sending non-native token. on Ethereum Mainnet [Bug]: Intrinsic gas too low error when sending non-native tokens on Ethereum Mainnet Dec 9, 2024
@Unik0rnMaggie Unik0rnMaggie added Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking team-assets portfolio-view Used for PRs and issues related to Q4 2024 portfolio view labels Dec 9, 2024
@Unik0rnMaggie
Copy link
Contributor Author

Also intrinsic gas message on Linea and Internal JSON-RPC error for other networks

Internal.JSON-RPC.error.mov

@Unik0rnMaggie
Copy link
Contributor Author

  • Ethereum Mainnet: when trying to raise the gas limit, the Tx still fails

  • Arbitrum: when trying to raise the gas values, the Tx fails with Internal JSON-RPC error message

Send.non-native.on.Abritrum.mov
Send.non-native.on.Ethereum.Mainnet.gas.limit.50000.mov

@sahar-fehri sahar-fehri added regression-main Regression bug that was found on main branch, but not yet present in production and removed portfolio-view Used for PRs and issues related to Q4 2024 portfolio view labels Dec 9, 2024
@Unik0rnMaggie
Copy link
Contributor Author

Unik0rnMaggie commented Dec 9, 2024

Not present in v7.37.0 RC from here

v7.37.0.Arbitrum.mov
v7.37.0.Ethereum.mov

@sleepytanya sleepytanya added the team-confirmations Push issues to confirmations team label Dec 9, 2024
@jpuri jpuri self-assigned this Dec 11, 2024
@sleepytanya sleepytanya added the regression-RC-7.38.0 Regression bug that was found in release candidate (RC) for release 7.38.0 label Dec 13, 2024
@sleepytanya
Copy link
Contributor

Present in RC 7.38.0 (1514):

7_38.mp4

@sleepytanya sleepytanya changed the title [Bug]: Intrinsic gas too low error when sending non-native tokens on Ethereum Mainnet [Bug]: Intrinsic gas too low / Intrinsic gas exceeds gas limit for Linea error when sending non-native tokens on all Popular networks Dec 13, 2024
@sleepytanya sleepytanya changed the title [Bug]: Intrinsic gas too low / Intrinsic gas exceeds gas limit for Linea error when sending non-native tokens on all Popular networks [Bug]: Intrinsic gas too low / Internal JSON-RPC / Intrinsic gas exceeds gas limit for Linea error when sending non-native tokens on all Popular networks Dec 13, 2024
@sleepytanya
Copy link
Contributor

The bug is present on all Popular networks, Internal JSON-RPC error shown for all Popular networks other than Linea and mainnet. For Linea Intrinsic gas exceeds gas limit error is displayed.

2.mov
Image Image

@Unik0rnMaggie
Copy link
Contributor Author

Reproduced on v7.38.0

v7.38.0.mov

@jpuri jpuri removed their assignment Dec 13, 2024
@Unik0rnMaggie
Copy link
Contributor Author

Present in v7.38.0 (1523) from here for both non-native tokens and ERC721 tokens.

When sending non-native tokens and ERC721 tokens, the error is:

  • intrinsic gas too low: gas 21000, minimum needed 21596 for Ethereum Mainnet
  • intrinsic gas exceeds gas limit for Linea
  • Internal JSON-RPC error for other popular networks
Sending.NFT.error.mov
Sending.non-native.tokens.error.mov

@sleepytanya sleepytanya added Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing and removed Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking labels Jan 10, 2025
@vpintorico vpintorico added the release-blocker This bug is blocking the next release label Jan 13, 2025
@OGPoyraz OGPoyraz self-assigned this Jan 13, 2025
@OGPoyraz OGPoyraz linked a pull request Jan 13, 2025 that will close this issue
7 tasks
github-merge-queue bot pushed a commit that referenced this issue Jan 13, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR aims to add `networkClientId` argument to `estimateGas`
function.

## **Related issues**

Fixes: #12612

## **Manual testing steps**

1. Try sending non-native asset initiated from app
2. See no "Instricted gas too low" error 
3. Expect transaction to be passed

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [X] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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.
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Jan 13, 2025
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Jan 13, 2025
@metamaskbot metamaskbot added the release-7.39.0 Issue or pull request that will be included in release 7.39.0 label Jan 13, 2025
OGPoyraz added a commit that referenced this issue Jan 13, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR aims to add `networkClientId` argument to `estimateGas`
function.

Fixes: #12612

1. Try sending non-native asset initiated from app
2. See no "Instricted gas too low" error
3. Expect transaction to be passed

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<!-- [screenshots/recordings] -->

<!-- [screenshots/recordings] -->

- [X] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] 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.
OGPoyraz added a commit that referenced this issue Jan 13, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR cherry picks
#12958 into
`release/7.38.0`

## **Related issues**

Fixes: #12612

## **Manual testing steps**

N/A

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

N/A

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [X] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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.
nickewansmith pushed a commit that referenced this issue Jan 17, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR aims to add `networkClientId` argument to `estimateGas`
function.

## **Related issues**

Fixes: #12612

## **Manual testing steps**

1. Try sending non-native asset initiated from app
2. See no "Instricted gas too low" error 
3. Expect transaction to be passed

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [X] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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
Labels
regression-main Regression bug that was found on main branch, but not yet present in production regression-RC-7.38.0 Regression bug that was found in release candidate (RC) for release 7.38.0 release-7.39.0 Issue or pull request that will be included in release 7.39.0 release-blocker This bug is blocking the next release Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-assets team-confirmations Push issues to confirmations team type-bug Something isn't working
Projects
Archived in project
Status: Fixed
7 participants