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

reason-string rule incorrectly handles revert #157

Closed
kevsul opened this issue Sep 9, 2019 · 2 comments
Closed

reason-string rule incorrectly handles revert #157

kevsul opened this issue Sep 9, 2019 · 2 comments

Comments

@kevsul
Copy link
Contributor

kevsul commented Sep 9, 2019

The reason-string rule checks that calls to require and revert include an optional message string. It works fine for require calls, but not for revert.

The issue is that the rule incorrectly treats both calls as if they took 2 arguments, whereas revert only takes one (the reason string). Solidity docs

So this works as expected:

require(condition, "Reason string");

but this causes a warning when it shouldn't:

revert("Reason string");

and this passes solhint, but is actually a compile error:

revert(false, "Reason string");
@kevsul
Copy link
Contributor Author

kevsul commented Sep 9, 2019

PR here: #158

@fvictorio
Copy link
Contributor

Closed by #158.

boolafish added a commit to omgnetwork/plasma-contracts that referenced this issue Oct 2, 2019
Remove this comment: "// solhint-disable-next-line reason-string" as the issue is solved.
solhint bug: protofire/solhint#157
boolafish added a commit to omgnetwork/plasma-contracts that referenced this issue Oct 2, 2019
Remove this comment: "// solhint-disable-next-line reason-string" as the issue is solved.
solhint bug: protofire/solhint#157
boolafish added a commit to omgnetwork/plasma-contracts that referenced this issue Oct 4, 2019
Remove this comment: "// solhint-disable-next-line reason-string" as the issue is solved.
solhint bug: protofire/solhint#157
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

No branches or pull requests

2 participants