Skip to content

Commit e0a2c13

Browse files
committed
Fixing assert failure checks in test mode
require(bool,string) was also considered a FAIL, and revert(string) also, etc. Also added a number of tests for all of these.
1 parent 8c36e83 commit e0a2c13

File tree

9 files changed

+61
-223
lines changed

9 files changed

+61
-223
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
8282
instead of crashing.
8383
- We no longer produce duplicate SMT assertions regarding concrete keccak values.
8484
- Ord is now correctly implemented for Prop.
85+
- We only report FAIL in `test` mode for assertion failures that match the
86+
string prefix "assertion failed", or match function selector Panic(uint256)
87+
with a parameter 0x1. Previously, `require(a==b, "reason")` was a cause for
88+
FAIL in case a!=b was possible. This has now been fixed.
8589

8690
## Changed
8791
- Warnings now lead printing FAIL. This way, users don't accidentally think that

src/EVM/UnitTest.hs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,10 @@ symRun opts@UnitTestOptions{..} vm (Sig testName types) = do
210210
Success _ _ _ store -> if opts.checkFailBit then PNeg (failed store) else PBool True
211211
Failure _ _ (Revert msg) -> case msg of
212212
ConcreteBuf b ->
213-
if (BS.isPrefixOf (selector "Error(string)") b) || b == panicMsg 0x01 then PBool False
213+
-- We need to drop the selector (4B), the offset value (aligned to 32B), and the length of the string (aligned to 32B)
214+
-- NOTE: assertTrue/assertFalse does not have the double colon after "assertion failed"
215+
let assertFail = selector "Error(string)" `BS.isPrefixOf` b && "assertion failed" `BS.isPrefixOf` (BS.drop (4+32+32) b)
216+
in if assertFail || b == panicMsg 0x01 then PBool False
214217
else PBool True
215218
b -> b ./= ConcreteBuf (panicMsg 0x01)
216219
Failure _ _ _ -> PBool True

test/contracts/fail/10_BadVault.sol

Lines changed: 0 additions & 214 deletions
This file was deleted.

test/contracts/fail/assertEq.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import {Test} from "forge-std/Test.sol";
2+
3+
contract SolidityTest is Test {
4+
function prove_assert_equal(uint stuff) public {
5+
assertEq(stuff, 0);
6+
}
7+
}

test/contracts/pass/requireEmpty.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import {Test} from "forge-std/Test.sol";
2+
3+
contract SolidityTest is Test {
4+
function prove_require_emtpy(uint stuff, uint a) public returns (uint) {
5+
if (stuff == 0) {
6+
require(a == 0);
7+
return a;
8+
}
9+
return a;
10+
}
11+
}

test/contracts/pass/requireString.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import {Test} from "forge-std/Test.sol";
2+
3+
contract SolidityTest is Test {
4+
function prove_require_string(uint stuff, uint a) public returns (uint) {
5+
if (stuff == 0) {
6+
require(a == 0, "must be 0");
7+
return a;
8+
}
9+
return a;
10+
}
11+
}

test/contracts/pass/revertEmpty.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import {Test} from "forge-std/Test.sol";
2+
3+
contract SolidityTest is Test {
4+
function prove_revert_empty(uint stuff) public {
5+
if (stuff == 0) revert();
6+
}
7+
}

test/contracts/pass/revertString.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import {Test} from "forge-std/Test.sol";
2+
3+
contract SolidityTest is Test {
4+
function prove_revert_string(uint stuff) public {
5+
if (stuff == 0) revert("stuff");
6+
}
7+
}

test/test.hs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,11 +1763,17 @@ tests = testGroup "hevm"
17631763
runSolidityTest testFile ".*" >>= assertEqualM "test result" (True, True)
17641764
, test "Foundry" $ do
17651765
-- quick smokecheck to make sure that we can parse ForgeStdLib style build outputs
1766+
-- return is a pair of (No Cex, No Warnings)
17661767
let cases =
1767-
[ ("test/contracts/pass/trivial.sol", ".*", (True, True))
1768-
, ("test/contracts/pass/dsProvePass.sol", "proveEasy", (True, True))
1769-
, ("test/contracts/fail/trivial.sol", ".*", (False, False))
1770-
, ("test/contracts/fail/dsProveFail.sol", "prove_add", (False, True))
1768+
[ ("test/contracts/pass/trivial.sol", ".*", (True, True))
1769+
, ("test/contracts/pass/dsProvePass.sol", ".*", (True, True))
1770+
, ("test/contracts/pass/revertEmpty.sol", ".*", (True, True))
1771+
, ("test/contracts/pass/revertString.sol", ".*", (True, True))
1772+
, ("test/contracts/pass/requireEmpty.sol", ".*", (True, True))
1773+
, ("test/contracts/pass/requireString.sol", ".*", (True, True))
1774+
, ("test/contracts/fail/trivial.sol", ".*", (False, False))
1775+
, ("test/contracts/fail/dsProveFail.sol", ".*", (False, True))
1776+
, ("test/contracts/fail/assertEq.sol", ".*", (False, True))
17711777
]
17721778
forM_ cases $ \(testFile, match, expected) -> do
17731779
actual <- runSolidityTestCustom testFile match Nothing Nothing False Nothing Foundry
@@ -1794,10 +1800,6 @@ tests = testGroup "hevm"
17941800
, test "transfer-dapp" $ do
17951801
let testFile = "test/contracts/pass/transfer.sol"
17961802
runSolidityTest testFile "prove_transfer" >>= assertEqualM "should prove transfer" (True, True)
1797-
, test "badvault-sym-branch" $ do
1798-
let testFile = "test/contracts/fail/10_BadVault.sol"
1799-
runSolidityTestCustom testFile "prove_BadVault_usingExploitLaunchPad" Nothing Nothing True Nothing Foundry >>=
1800-
assertEqualM "Must find counterexample" (False, True)
18011803
, test "Prove-Tests-Fail" $ do
18021804
let testFile = "test/contracts/fail/dsProveFail.sol"
18031805
runSolidityTest testFile "prove_trivial" >>= assertEqualM "prove_trivial" (False, False)

0 commit comments

Comments
 (0)