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]: Cannot send a transaction with Hex Data on Optimism #14298

Closed
PeterYinusa opened this issue Mar 31, 2022 · 10 comments
Closed

[Bug]: Cannot send a transaction with Hex Data on Optimism #14298

PeterYinusa opened this issue Mar 31, 2022 · 10 comments
Labels
area-transactions Sev3-low Low severity; minimal to no impact upon users stale issues and PRs marked as stale team-confirmations-planning (only for internal use within Confirmations team) type-bug

Comments

@PeterYinusa
Copy link
Contributor

Describe the bug

If you try to send a transaction that contains Hex Data on Optimism the transaction fails immediately

Optimism.mov

Steps to reproduce

  1. Settings -> Advanced -> Show Hex Data
  2. Create a Send Transaction with Hex Data
  3. Click Next
  4. Click Confirm

Error messages or log output

sentry-install.js:1 [ethjs-query] while formatting outputs from RPC '{"value":{"code":-32603,"data":{"code":-32000,"message":"invalid transaction: intrinsic gas too low"}}}'

Version

10.13.0

Build type

No response

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

You cannot increase the Gas Price anymore, but a Dapp can suggest a higher Gas Price

@PeterYinusa PeterYinusa added rc ✔️ Issues found testing the latest release candidate area-transactions labels Mar 31, 2022
@danjm
Copy link
Contributor

danjm commented Apr 6, 2022

@PeterYinusa I believe this is only a problem if trying to set data to 0x0 (e.g. the error should not occur if setting data to 0x1 or 0xabcdef0123456789), and for that scenario the error will also occur on v10.11.x unless the user edits the gas limit

Can you confirm?

If so, I think it is okay to not fix that at this time because:

  • this only affects optimism
  • This is a very rare edge case. I am not sure there is a use case for sending 0x0 data
  • I believe this is a bug at the rpc provider / ethereum client layer. For us to resolve, we would have to dig in at that layer which could take some time.
  • the only degradation here is that Optimism users can't edit the gas limit to work around the rpc provider / ethereum client bug, but the Optimism team explicitly asked us to prevent editing gas on that network (as doing so usually causes more problems than it solves, and their contract based estimation should provide a correct+optimal estimate)

@PeterYinusa
Copy link
Contributor Author

PeterYinusa commented Apr 7, 2022

@PeterYinusa I believe this is only a problem if trying to set data to 0x0 (e.g. the error should not occur if setting data to 0x1 or 0xabcdef0123456789), and for that scenario the error will also occur on v10.11.x unless the user edits the gas limit

Yes, your observation is correct @danjm

@PeterYinusa PeterYinusa added the Sev3-low Low severity; minimal to no impact upon users label Apr 7, 2022
@PeterYinusa PeterYinusa added this to the v10.13.X milestone Apr 12, 2022
@PeterYinusa PeterYinusa removed the rc ✔️ Issues found testing the latest release candidate label Apr 12, 2022
@PeterYinusa PeterYinusa removed this from the v10.13.X milestone Apr 12, 2022
@nic-github-ux
Copy link

I ran into this problem and don't know how to edit hex data

@seaona
Copy link
Contributor

seaona commented Aug 17, 2022

@PeterYinusa @danjm for me it looks like it fails with all hex data values. I.e. tried with 0x1 or 0xabcdef0123456789 and both failed

@suraneti
Copy link

suraneti commented Sep 5, 2022

This problem also occurred on Hop Protocol and Stargate Finance.

@markusbkoch
Copy link

I also encountered this problem, using 10.18.3
I'm trying to interact with a contract on Optimism through Metamask, so I'm passing some specific data.
The txn value (amount of OETH sent) must be zero. This seems to be causing the gas price to also be set to zero. The transaction is not broadcast and the console outputs:
[ethjs-query] while formatting outputs from RPC '{"value":{"code":-32603,"data":{"code":-32000,"message":"cannot accept 0 gas price transaction"}}}'

@protocolwhisper
Copy link

Same error here!! I already update the data but it don't work at all.
[ethjs-query] while formatting outputs from RPC '{"value":{"code":-32603,"data":{"code":-32000,"message":"cannot accept 0 gas price transaction"}}}'

@protocolwhisper
Copy link

It worked today maybe is a chain issue

@bschorchit bschorchit added the team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead label Mar 9, 2023
@hilvmason hilvmason added team-confirmations-planning (only for internal use within Confirmations team) and removed team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead labels Jun 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2023

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Sep 3, 2023
brad-decker added a commit that referenced this issue Sep 26, 2023
## **Description**
Our fork of ethjs-query has been namespaced to the `@metamask/`
namespace, and updated with mostly development only fixes. There is one
exception which is the reason for this pull request which is the removal
of a try/catch that was catching too broadly and wrapping legitimate
errors in a new Error object that claimed the issue was with the
formatting of the output. In most cases this is incorrect and results in
a wide swath of errors being lumped together inside of sentry. This
change will result in the real errors being surfaced, after which we can
decide where to prioritize efforts to resolve RPC issues.

This PR progresses, and is expected to change the stack trace for the
following issues:
#9317 
#10519 
#10619
#11488 
#11974 
#13395 
#14298 
#14365 
#15250 
#17073 
#17803 
#19697 
#20699 

We are closing these issues opened automatically by sentry-io which are
not fully resolved but should result in better errors and stack traces:

fixes #10552 
fixes #14660 
fixes #14676 
fixes #14730 
fixes #14801
fixes #15065 

## **Manual testing steps**
1. Attempt to reproduce any of the bugs listed, 17073 was the easiest to
reproduce for me. This involves getting test currency from the wemix
faucet as listed in the issue and initiating a transaction between two
accounts you own.
2. On develop you'll see an 'error formatting outputs' error text
similar to what is reported in the issue.
3. On this branch you'll get the original error, without the wrapped
'formatting' error. including a different stack trace.

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
@github-actions
Copy link
Contributor

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2023
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-transactions Sev3-low Low severity; minimal to no impact upon users stale issues and PRs marked as stale team-confirmations-planning (only for internal use within Confirmations team) type-bug
Projects
None yet
Development

No branches or pull requests

10 participants