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

Return full REVERT error message #6030

Merged
merged 9 commits into from
Aug 28, 2023
Merged

Return full REVERT error message #6030

merged 9 commits into from
Aug 28, 2023

Conversation

emlautarom1
Copy link
Contributor

Fixes #6024

Changes

  • Return the full error message on REVERT when parsing fails

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

Remarks

This fix only results in compatibility with other clients like geth and erigon.

@emlautarom1
Copy link
Contributor Author

The example on the referenced ticket (#6024) should result most likely in Reverted 0x41413231206469646e2774207061792070726566756e64, but several clients (erigon, Geth) return the full output:

0x220266b600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000001741413231206469646e2774207061792070726566756e64000000000000000000

I say "most likely" since 0x41413231206469646e2774207061792070726566756e64 when decoded as ASCII is "AA21 didn't pay prefund", which matches the REVERT associated message in the source contract: https://etherscan.io/address/0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789#code#F1#L422

@emlautarom1
Copy link
Contributor Author

It's also worth noting that Solidity has a convention starting from 0.8 regarding error messages: https://soliditylang.org/blog/2020/12/16/solidity-v0.8.0-release-announcement/.

Nevertheless, for the referenced ticket, the output value is:

byte[] data = {
    0x22, 0x02, 0x66, 0xb6, // Keccak of 'FailedOp(uint256,string)' == 0x220266b6
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x17,
    0x41, 0x41, 0x32, 0x31, 0x20, 0x64, 0x69, 0x64, 0x6e, 0x27, 0x74, 0x20, 0x70, 0x61, 0x79, 0x20, 0x70, 0x72, 0x65, 0x66, 0x75, 0x6e, 0x64, // "AA21 didn't pay prefund"
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

which does not match any of our expectations regarding error messages.

@emlautarom1 emlautarom1 self-assigned this Aug 22, 2023
@emlautarom1
Copy link
Contributor Author

Associated ticket related to error messages in the spec: ethereum/execution-apis#463

@emlautarom1 emlautarom1 merged commit 77bbdf0 into master Aug 28, 2023
61 checks passed
@emlautarom1 emlautarom1 deleted the fix/revert-message branch August 28, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Receiving "Reverted 0x" on reverts instead of the actual error
3 participants