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

Possible invalid gas check #2381

Closed
gabmontes opened this issue Feb 15, 2019 · 1 comment · Fixed by #2387
Closed

Possible invalid gas check #2381

gabmontes opened this issue Feb 15, 2019 · 1 comment · Fixed by #2387
Labels
Bug Addressing a bug

Comments

@gabmontes
Copy link
Contributor

gabmontes commented Feb 15, 2019

Up to version beta.37, there was an issue with checking the receipt for transaction execution status:

https://github.com/ethereum/web3.js/blob/1d9f6c0889c7f551b4ec1041cece0d50bc3ff2c8/packages/web3-core-method/src/index.js#L351

While gasProvided is a hex string, receipt.gasUsed is a number, making the condition always fail -and therefore failing to do any check-. Proper comparison would be:

gasProvided !== utils.numberToHex(receipt.gasUsed)

It looks like after the refactor in version beta.38 and up, the condition is the same:

https://github.com/ethereum/web3.js/blob/599851753b4a35b5c11f902c7c592b111e190073/packages/web3-core-method/src/validators/TransactionReceiptValidator.js#L80

I was unable to check the latests versions yet as will remain in beta.37 for now but it is possible that the error persist.

gabmontes added a commit to autonomoussoftware/metronome-wallet-core that referenced this issue Feb 15, 2019
@nivida
Copy link
Contributor

nivida commented Feb 15, 2019

Thanks for submitting this issue. Will fix it next week after my vacation 👌🏻

@nivida nivida added the Bug Addressing a bug label Feb 15, 2019
nivida referenced this issue Feb 19, 2019
Validation of the gas usage fixed in TransactionReceiptValidator
@nivida nivida mentioned this issue Oct 21, 2019
11 tasks
@gabmontes gabmontes mentioned this issue Nov 18, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants