dapphub / dapphub/dapptools

Feature request: Stricter revert checks

Open
#695 0 comments 2 reactions 0 assignees View on GitHub
Dominant language
Haskell
Stars
2.1k
Forks
320
PR merge metrics
No merged PRs in 30d

Description

Copying relevant context from the chat https://dapphub.chat/channel/dev?msg=Rpcaptofwiui5ZHKu

> jo-es June 22, 2021 6:38 AM
> are there any plans on adding the ability to supply an expected revert reason to `testFail`. I'm currently doing the following which works pretty well.
> ```
> try METHOD_WHICH_REVERTS {} catch Error(string memory reason) {
> string memory expected = REVERT_REASON;
> if (keccak256(abi.encodePacked(reason)) == keccak256(abi.encodePacked(expected))) revert("");
> } catch Panic(uint) {}
> ```
> > mds1 7:51 AM
> > Just wanted to +1 on this idea. Similarly, I think `testFail` should only be considered passing if it reverts on the last line of the test. The current implementation seems very prone to bugs where tests are passing from the wrong failure. Specifically I see two categories of this that are both hard to detect:
> >
> > 1. Say my test contains two lines of code. Presumably I want it to fail on the second line (or I wouldn't have that code), but the first line failing will still consider the test passing
> > 2. A call reverting with an unexpected error message will still be considered passing. I might try to test a revert on "insufficient allowance", but accidentally get a passing test that really is reverting with "insufficient balance"

Items 1 and 2 in the above quote are the two forms of stricter revert checks that I think would be useful.

------------------------

Quoting @d-xo on possible implementations:

> david.terry 7:58 AM
> this would be really nice to have. I think we could probably implement it using some magic modifier that would let you pass the expected revert reason to hevm
>
> maybe something like
>
> ```
> testFailSomething()
> public
> revertData(abi.encodeWithSignature(“Error(string)”, “expected revert message”))
> {
> require(false, "expected revert message");
> }
> ```
> > mds1 8:26 AM
> Something like that would be super helpful! There's probably a few other signatures for the `revertData` that'd be useful too, such as just a string, the error name (referring to the new Error type from 0.8.4) with any parameter values, and the error name with specified parameter values

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.