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

[DRAFT] Cheatcodes as per new std-forge library #567

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

msooseth
Copy link
Collaborator

@msooseth msooseth commented Sep 23, 2024

Goal

The way forge-std is doing things has changed. It no longer sets a value in ds-test, but instead Revert-s with a specific message. So we have to match:

forge test --match-test "test_set_get()" -vvvv
[...]
  [31205] MateFailContractTest::test_set_get()
    ├─ [22216] MateFailContract::set_mate()
    │   └─ ← [Stop]
    ├─ [268] MateFailContract::get_mate() [staticcall]
    │   └─ ← [Return] 3
    ├─ [0] VM::assertEq(2, 3) [staticcall]
    │   └─ ← [Revert] assertion failed: 3 != 2
    └─ ← [Revert] assertion failed: 3 != 2

Instead of the current:

Traces:
  [42786] MateFailContractTest::test_set_get()
    ├─ [22216] MateFailContract::set_mate()
    │   └─ ← [Stop]
    ├─ [268] MateFailContract::get_mate() [staticcall]
    │   └─ ← [Return] 3
    ├─ emit log(val: "Error: a == b not satisfied [uint]")
    ├─ emit log_named_uint(key: "      Left", val: 3)
    ├─ emit log_named_uint(key: "     Right", val: 2)
    ├─ [0] VM::store(VM: [0x7109709ECfa91a80626fF3989D68f67F5b1DD12D], 0x6661696c65640000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000001)
    │   └─ ← [Return]
    └─ ← [Stop]

Implementation

Emilio has changed things around already in EVM.hs to have a separate frame, which allows this to be coded in a relatively compact way. I have now added a number of required (but not all) cheatcodes.

Future work

I haven't added:

  • assertEq(T[], T[]) type asserts
  • assertNotEq(T[], T[]) type asserts
  • assertXXDecimal
  • `assertApproxXX
  • assertEqCall`

I need to cut the cheatcodes into a separate file. I'll do these in the next PR, this was it's a bit easier to review this PR.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@msooseth msooseth force-pushed the implement_new_cheatcodes branch 2 times, most recently from e78213a to c0cca47 Compare October 28, 2024 11:21
@msooseth msooseth changed the title [TODO] Cheatcodes TODOs Cheatcodes as per new std-forge library Oct 29, 2024
@msooseth msooseth changed the title Cheatcodes as per new std-forge library [DRAFT] Cheatcodes as per new std-forge library Oct 29, 2024
@msooseth msooseth force-pushed the implement_new_cheatcodes branch 7 times, most recently from fd7745f to 527025f Compare October 29, 2024 17:46
Comment on lines -290 to -297
checkSymFailures :: VMOps t => UnitTestOptions RealWorld -> Stepper t RealWorld (VM t RealWorld)
checkSymFailures UnitTestOptions { .. } = do
-- Ask whether any assertions failed
Stepper.evm $ do
popTrace
abiCall testParams (Left ("failed()", emptyAbi))
Stepper.runFully

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used anywhere, so I'm removing it

") parameter decoding failed. Error: " <> show abivals
revertErr a b comp = frameRevert $ "assertion failed: " <>
BS8.pack (show a) <> " " <> comp <> " " <> BS8.pack (show b)
assertEq abitype sig input = do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super bothered by the duplication but if you wanted to cut some lines you could probably merge the implementations into one function that is parametrised on the comparison function and string used in the revert error?

src/EVM.hs Outdated
revertContracts
revertSubstate
assign (#state % #returndata) mempty
assign (#state % #returndata) (ConcreteBuf (BS8.pack $ show e))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to do this for non reverting errors?

@@ -224,6 +227,12 @@ symRun opts@UnitTestOptions{..} vm (Sig testName types) = do
-- check postconditions against vm
(e, results) <- verify solvers (makeVeriOpts opts) (symbolify vm') (Just postcondition)
let allReverts = not . (any Expr.isSuccess) . flattenExpr $ e
let fails = filter (Expr.isFailure) $ flattenExpr e
unless (null fails) $ liftIO $ do
putStrLn $ " \x1b[33mWARNING\x1b[0m: hevm was only able to partially explore the test " <> Text.unpack testName <> " due to: ";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are failures the same as partial executions?

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.

2 participants