-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feature(invariant) - persist and replay failure #7899
Merged
grandizzy
merged 6 commits into
foundry-rs:master
from
grandizzy:invariant-failure-persistence
May 21, 2024
Merged
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
9b9ccf8
feature(invariant) - persist and replay failure
grandizzy f6e1eef
Fix unit test
grandizzy 9b22080
Changes after review:
grandizzy db1b619
Changes after review: display proper message if replayed sequence rev…
grandizzy 7686c03
Changes after review: simplify check sequence logic
grandizzy 69c523d
Merge remote-tracking branch 'origin' into invariant-failure-persistence
grandizzy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
use crate::executors::{invariant::error::FailedInvariantCaseData, Executor}; | ||
use alloy_primitives::U256; | ||
use alloy_primitives::{Address, Bytes, U256}; | ||
use foundry_evm_core::constants::CALLER; | ||
use foundry_evm_fuzz::invariant::BasicTxDetails; | ||
use proptest::bits::{BitSetLike, VarBitSet}; | ||
|
@@ -97,7 +97,14 @@ pub(crate) fn shrink_sequence( | |
let mut shrinker = CallSequenceShrinker::new(calls.len()); | ||
for _ in 0..failed_case.shrink_run_limit { | ||
// Check candidate sequence result. | ||
match check_sequence(failed_case, executor.clone(), calls, shrinker.current().collect()) { | ||
match check_sequence( | ||
executor.clone(), | ||
calls, | ||
shrinker.current().collect(), | ||
failed_case.addr, | ||
failed_case.func.clone(), | ||
failed_case.fail_on_revert, | ||
) { | ||
// If candidate sequence still fails then shrink more if possible. | ||
Ok(false) if !shrinker.simplify() => break, | ||
// If candidate sequence pass then restore last removed call and shrink other | ||
|
@@ -110,41 +117,39 @@ pub(crate) fn shrink_sequence( | |
Ok(shrinker.current().map(|idx| &calls[idx]).cloned().collect()) | ||
} | ||
|
||
/// Checks if the shrinked sequence fails test, if it does then we can try simplifying more. | ||
fn check_sequence( | ||
failed_case: &FailedInvariantCaseData, | ||
/// Checks if the given call sequence breaks the invariant. | ||
/// Used in shrinking phase for checking candidate sequences and in replay failures phase to test | ||
/// persisted failures. | ||
pub fn check_sequence( | ||
mut executor: Executor, | ||
calls: &[BasicTxDetails], | ||
sequence: Vec<usize>, | ||
test_address: Address, | ||
test_function: Bytes, | ||
fail_on_revert: bool, | ||
) -> eyre::Result<bool> { | ||
let mut sequence_failed = false; | ||
// Apply the shrinked candidate sequence. | ||
// Apply the call sequence. | ||
for call_index in sequence { | ||
let (sender, (addr, bytes)) = &calls[call_index]; | ||
let call_result = | ||
executor.call_raw_committing(*sender, *addr, bytes.clone(), U256::ZERO)?; | ||
if call_result.reverted && failed_case.fail_on_revert { | ||
if call_result.reverted && fail_on_revert { | ||
// Candidate sequence fails test. | ||
// We don't have to apply remaining calls to check sequence. | ||
sequence_failed = true; | ||
break; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we just return here instead of managing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep yep, nice cleanup! 7686c03 |
||
} | ||
} | ||
// Return without checking the invariant if we already have failing sequence. | ||
// Return without checking the invariant if we already have a failing sequence. | ||
if sequence_failed { | ||
return Ok(false); | ||
}; | ||
|
||
// Check the invariant for candidate sequence. | ||
// If sequence fails then we can continue with shrinking - the removed call does not affect | ||
// failure. | ||
// | ||
// If sequence doesn't fail then we have to restore last removed call and continue with next | ||
// call - removed call is a required step for reproducing the failure. | ||
let mut call_result = | ||
executor.call_raw(CALLER, failed_case.addr, failed_case.func.clone(), U256::ZERO)?; | ||
// Check the invariant for call sequence. | ||
let mut call_result = executor.call_raw(CALLER, test_address, test_function, U256::ZERO)?; | ||
Ok(executor.is_raw_call_success( | ||
failed_case.addr, | ||
test_address, | ||
Cow::Owned(call_result.state_changeset.take().unwrap()), | ||
&call_result, | ||
false, | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we convert this into either a private function or closure instead?
not a fan of using private macros for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed in 9b22080 I opted not to throw the SolcIoError anymore (as is not really solc related) and just ignore, can readd it if you think it should be there