Skip to content

Conversation

@DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Aug 19, 2023

Motivation

Most of the invariant code is written as if it handles multiple functions per call (e.g. argument is &[Function] instead of &Function etc.).

This is fine, except we call these functions with vec![function] in a loop over the functions...

Recommended to review with "Hide whitespace" since this gets rid of a lot of indentation

Solution

Rewrite invariants to run one function at a time, and parallelize the top level loop

Comment on lines -454 to -457
let Ok(InvariantFuzzTestResult { invariants, cases, reverts, last_run_inputs }) =
evm.invariant_fuzz(&invariant_contract)
else {
return vec![]
Copy link
Member Author

Choose a reason for hiding this comment

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

This error was just completely ignored for some reason. This gets thrown by prepare_fuzzing

Copy link
Member

@Evalir Evalir Aug 19, 2023

Choose a reason for hiding this comment

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

Yeah, I think there's a few more cases of this happening across the test runner. I have a pending refactor of another error being ignored on invariants

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

I love this—there's a ton we can refactor here but this is great to reduce performance bottlenecks 😄

Let's just make sure we test this a ton before merge!

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

I love this—there's a ton we can refactor here but this is great to reduce performance bottlenecks 😄

Let's just make sure we test this a ton before merge!

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.

amazing

@mattsse mattsse merged commit 18388a8 into foundry-rs:master Aug 20, 2023
@DaniPopes DaniPopes deleted the parallel-invariants branch August 20, 2023 16:16
Comment on lines -625 to -631
// Hacky to provide the full error to the user.
for invariant in invariant_contract.invariant_functions.iter() {
failures.failed_invariants.insert(
invariant.name.clone(),
(Some(error.clone()), invariant.to_owned().clone()),
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only one invariant is executed at a time now so there can't be multiple fails in one call to this function

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you take a look at #5726 pls? I think a bug was introduced but I can't manage to find when or where

mikelodder7 pushed a commit to LIT-Protocol/foundry that referenced this pull request Sep 12, 2023
* refactor: parallelize invariants

* chore: clippy

* collect before
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.

4 participants