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

handleRevert implementation broken when no value is specified for gasPrice on eth_sendRawTransaction #4902

Closed
1 task done
drub0y opened this issue Mar 29, 2022 · 2 comments · Fixed by #4903
Closed
1 task done
Labels
1.x 1.0 related issues Bug Addressing a bug Investigate

Comments

@drub0y
Copy link

drub0y commented Mar 29, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

checkConfirmation in web3-core-method/src/index.js on line 458 (here) throws an internal error because it blindly accesses parsedTx.gasPrice to call its .toHexString(), but this property is undefined for all EIP-1559 transactions. This is then captured by an internal try/catch, swallowed and turned into TransactionRevertedWithoutReasonError and the app receives no reason.

Expected Behavior

I expect the logic to be aware of the fact that gasPrice will be undefined for EIP-1559 transactions and not call .toHextString() on it in that case so that it doesn't cause an internal error that stops fetching the reason for the failure from the RPC node with eth_call.

Steps to Reproduce

  1. Set web3.eth.handleRevert = true
  2. Send a transaction that should return a revert reason through the eth_sendRawTransaction
  3. Catch the error
  4. Notice it has no reason when it should

Web3.js Version

1.7.1

Environment

Shouldn't matter, but I'm on Node.js Version 16.14.0.

Anything Else?

Simply adding the null propagation operator like so is all that is needed to fix:

gasPrice: parsedTx.gasPrice?.toHexString(),

I have tested this by manually editing the version in lib locally and everything works as expected then. If trying to be backwards compatible and not use null propagation operator the following fixes as well:

gasPrice: parsedTx.gasPrice ? parsedTx.gasPrice.toHexString() : undefined,
@drub0y drub0y added the Bug Addressing a bug label Mar 29, 2022
@drub0y drub0y changed the title handleRevert implementation broken when no value is specified for gasPrice on transaction handleRevert implementation broken when no value is specified for gasPrice on eth_sendRawTransaction Mar 29, 2022
drub0y added a commit to drub0y/web3.js that referenced this issue Mar 29, 2022
drub0y added a commit to drub0y/web3.js that referenced this issue Mar 29, 2022
@nikoulai nikoulai added the 1.x 1.0 related issues label Mar 30, 2022
@nazarhussain
Copy link
Contributor

@drub0y Thanks for reaching back to us. We will investigate this issue and try to get it fixed as soon we have some bandwidth. Currently all our resources are focused towards 4.x rewrite.

@drub0y
Copy link
Author

drub0y commented Mar 31, 2022

@nazarhussain No problem! Fix is already PR'd actually and looks to be slated for inclusion into 1.7.3. 🤞

jdevcs pushed a commit that referenced this issue Apr 13, 2022
* Fixes calling toHexString on undefined gasPrice

Fixes #4902
@jdevcs jdevcs closed this as completed in a1a7679 Apr 13, 2022
@jdevcs jdevcs mentioned this issue May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug Investigate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants