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

expectRevert should support panic codes #149

Open
axic opened this issue Dec 29, 2020 · 4 comments
Open

expectRevert should support panic codes #149

axic opened this issue Dec 29, 2020 · 4 comments
Labels

Comments

@axic
Copy link

axic commented Dec 29, 2020

Since right now the second argument is expected to be a string (which assumes the signature of Error(string)), I think the easiest way to extend this would be if a number is found, treat it s a panic code (with the signature of Panic(uint256)).

See https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require for more information.

This would allow testing for specific abort reasons.

@abcoathup
Copy link
Contributor

Hi @axic! Thanks for the suggestion, it is really appreciated.

The project owner will review your suggestion as soon as they can.

Please wait until we have discussed this idea before writing any code or submitting a Pull Request, so we can go through the design beforehand. We don’t want you to waste your time!

@frangio
Copy link
Contributor

frangio commented Jan 5, 2021

Yes this would be great. I don't know how we would implement it though. We rely on the error message that Truffle emits. What error message does Truffle emit on a Panic(uint256) error?

@axic
Copy link
Author

axic commented Jan 29, 2021

What error message does Truffle emit on a Panic(uint256) error?

Hm, without looking, I hoped this would be based on examining the exact returndata. But it seems that ethereumjs/ganache does some processing.

ethereumjs-vm returns the data unmodified. ganache looks up revert strings, but drops anything else.

Created an issue upstream: trufflesuite/ganache#758

@frangio
Copy link
Contributor

frangio commented Jan 29, 2021

Thanks for looking into this. From a quick test Hardhat doesn't support Panic either, so I opened an issue there too. NomicFoundation/hardhat#1209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants