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

feat(cheatcodes): add vm.assumeNoRevert for fuzz tests #8780

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Aug 30, 2024

Motivation

Closes #4090
Closes #7246

Similar with assume cheatcode, but for discarding inputs in fuzz runs that reverts:

    /// Discard this run's fuzz inputs and generate new ones if next call reverted.
    function assumeNoRevert() external pure;

Solution

  • split assume* cheatcodes in separate mod
  • in call_end, if assumeNoRevert cheatcode set, discard run if we're at the same depth as cheatcode and call reverted by returning result with MAGIC_ASSUME output (caught by fuzz and invariant tests and discarded). If call didn't revert then reset chetcode state and continue test
  • unit tests

@klkvr
Copy link
Member

klkvr commented Sep 1, 2024

should this only re-run if next call reverts? went through issues and seems like this is the common usecase, current version might result in footguns, especially given that sometimes it's hard to craft an array with addresses whitelist

@grandizzy
Copy link
Collaborator Author

grandizzy commented Sep 2, 2024

should this only re-run if next call reverts? went through issues and seems like this is the common usecase, current version might result in footguns, especially given that sometimes it's hard to craft an array with addresses whitelist

I think this makes sense, was looking at #7246 (comment) comment and thought we may want to provide same behavior as when setting fail_on_revert=false but since this is a cheatcode is better to have the next call consistency. I reworked PR to reflect such, please check.

@mds1
Copy link
Collaborator

mds1 commented Sep 3, 2024

Spec sgtm, and do we also increment the rejects counter the same way? A single test rejects counter for this + vm.assume seems like the best UX, to keep it simple

@grandizzy
Copy link
Collaborator Author

grandizzy commented Sep 3, 2024

Spec sgtm, and do we also increment the rejects counter the same way? A single test rejects counter for this + vm.assume seems like the best UX, to keep it simple

yep, the rejects counter is incremented same way and shared by both assumes

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm, pending @klkvr

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