Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/setup-skip-after-caught-revert.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
forge: patch
foundry-evm: patch
foundry-cheatcodes: patch
---

Fixed `vm.skip` in `setUp` being reported as a failure when an earlier revert was caught before the skip.
7 changes: 7 additions & 0 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,12 @@ pub struct Cheatcodes<FEN: FoundryEvmNetwork = EthEvmNetwork> {
/// Test-scoped context holding data that needs to be reset every test run
pub test_context: TestContext,

/// Revert payloads minted by the `skip` cheatcode during the current test call.
///
/// A top-level revert is only classified as a skip when its data byte-equals one of these
/// payloads, so user-crafted `FOUNDRY::SKIP` revert data never skips a test on its own.
pub skip_payloads: Vec<Bytes>,

/// Whether to commit FS changes such as file creations, writes and deletes.
/// Used to prevent duplicate changes file executing non-committing calls.
pub fs_commit: bool,
Expand Down Expand Up @@ -950,6 +956,7 @@ impl<FEN: FoundryEvmNetwork> Cheatcodes<FEN> {
broadcastable_transactions: Default::default(),
access_list: Default::default(),
test_context: Default::default(),
skip_payloads: Default::default(),
serialized_jsons: Default::default(),
eth_deals: Default::default(),
gas_metering: Default::default(),
Expand Down
38 changes: 21 additions & 17 deletions crates/cheatcodes/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::{Cheatcode, Cheatcodes, CheatsCtxt, Result, Vm::*};
use alloy_chains::Chain as AlloyChain;
use alloy_primitives::{Address, U256};
use alloy_primitives::{Address, Bytes, U256};
use alloy_sol_types::SolValue;
use foundry_common::version::SEMVER_VERSION;
use foundry_evm_core::{constants::MAGIC_SKIP, evm::FoundryEvmNetwork};
Expand Down Expand Up @@ -69,28 +69,14 @@ impl Cheatcode for sleepCall {
impl Cheatcode for skip_0Call {
fn apply_stateful<FEN: FoundryEvmNetwork>(&self, ccx: &mut CheatsCtxt<'_, '_, FEN>) -> Result {
let Self { skipTest } = *self;
if skipTest {
// Skip should not work if called deeper than at test level.
// Since we're not returning the magic skip bytes, this will cause a test failure.
ensure!(ccx.ecx.journal().depth() <= 1, "`skip` can only be used at test level");
Err([MAGIC_SKIP, &[]].concat().into())
} else {
Ok(Default::default())
}
skip(ccx, skipTest, "")
}
}

impl Cheatcode for skip_1Call {
fn apply_stateful<FEN: FoundryEvmNetwork>(&self, ccx: &mut CheatsCtxt<'_, '_, FEN>) -> Result {
let Self { skipTest, reason } = self;
if *skipTest {
// Skip should not work if called deeper than at test level.
// Since we're not returning the magic skip bytes, this will cause a test failure.
ensure!(ccx.ecx.journal().depth() <= 1, "`skip` can only be used at test level");
Err([MAGIC_SKIP, reason.as_bytes()].concat().into())
} else {
Ok(Default::default())
}
skip(ccx, *skipTest, reason)
}
}

Expand All @@ -110,6 +96,24 @@ impl Cheatcode for getChain_1Call {
}
}

/// Reverts with the magic skip payload and records it in the state, so that the executor can
/// distinguish this genuine skip from user-crafted revert data carrying the same prefix.
fn skip<FEN: FoundryEvmNetwork>(
ccx: &mut CheatsCtxt<'_, '_, FEN>,
skip_test: bool,
reason: &str,
) -> Result {
if !skip_test {
return Ok(Default::default());
}
// Skip should not work if called deeper than at test level.
// Since we're not returning the magic skip bytes, this will cause a test failure.
ensure!(ccx.ecx.journal().depth() <= 1, "`skip` can only be used at test level");
let payload = Bytes::from([MAGIC_SKIP, reason.as_bytes()].concat());
ccx.state.skip_payloads.push(payload.clone());
Err(payload.into())
}

/// Adds or removes the given breakpoint to the state.
fn breakpoint<FEN: FoundryEvmNetwork>(
state: &mut Cheatcodes<FEN>,
Expand Down
32 changes: 12 additions & 20 deletions crates/evm/evm/src/executors/fuzz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use foundry_common::sh_println;
use foundry_config::FuzzConfig;
use foundry_evm_core::{
Breakpoints,
constants::{CHEATCODE_ADDRESS, MAGIC_ASSUME},
constants::MAGIC_ASSUME,
decode::{RevertDecoder, SkipReason},
evm::FoundryEvmNetwork,
};
Expand Down Expand Up @@ -327,9 +327,7 @@ impl<FEN: FoundryEvmNetwork> FuzzedExecutor<FEN> {
..Default::default()
});
}
if call.reverter == Some(CHEATCODE_ADDRESS)
&& let Some(reason) = SkipReason::decode(&call.result)
{
if let Some(reason) = call.skip_reason() {
return Ok(FuzzTestResult { skipped: true, reason: reason.0, ..Default::default() });
}

Expand Down Expand Up @@ -359,13 +357,10 @@ impl<FEN: FoundryEvmNetwork> FuzzedExecutor<FEN> {
result.logs = call.logs;
result.gas_report_traces.extend(call.traces.into_iter().map(|trace| trace.arena));
} else {
let reason = if call.reverter == Some(CHEATCODE_ADDRESS) {
SkipReason::decode(&call.result)
.map(|reason| reason.to_string())
.or_else(|| rd.maybe_decode(&call.result, call.exit_reason))
} else {
rd.maybe_decode(&call.result, call.exit_reason)
};
let reason = call
.skip_reason()
.map(|reason| reason.to_string())
.or_else(|| rd.maybe_decode(&call.result, call.exit_reason));
result.reason = reason;
let args = tx
.call_details
Expand Down Expand Up @@ -907,15 +902,12 @@ impl<FEN: FoundryEvmNetwork> FuzzedExecutor<FEN> {
}
worker.failure_run = fuzz_run;

// Only classify magic skip payloads when the revert originates from the
// cheatcode address.
let reason = if outcome.1.reverter == Some(CHEATCODE_ADDRESS) {
SkipReason::decode(&outcome.1.result)
.map(|reason| reason.to_string())
.or_else(|| rd.maybe_decode(&outcome.1.result, status))
} else {
rd.maybe_decode(&outcome.1.result, status)
};
// Only classify magic skip payloads minted by the skip cheatcode.
let reason = outcome
.1
.skip_reason()
.map(|reason| reason.to_string())
.or_else(|| rd.maybe_decode(&outcome.1.result, status));
if self.config.show_logs {
worker.logs.extend(outcome.1.logs.clone());
} else {
Expand Down
40 changes: 31 additions & 9 deletions crates/evm/evm/src/executors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,8 @@ impl<FEN: FoundryEvmNetwork> Executor<FEN> {
// Clear broadcastable transactions
cheats.broadcastable_transactions.clear();
cheats.ignored_traces.ignored.clear();
// Skip payloads are scoped to the call they were minted in.
cheats.skip_payloads.clear();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider pinning this lifetime boundary with an integration test: catch vm.skip(..., "x") in setUp and return successfully, then have the test revert with the exact FOUNDRY::SKIPx bytes. It should fail, proving minted payloads cannot authenticate a later executor call.


// if tracing was paused but never unpaused, we should begin next frame with tracing
// still paused
Expand Down Expand Up @@ -1296,6 +1298,11 @@ pub struct RawCallResult<FEN: FoundryEvmNetwork = EthEvmNetwork> {
/// The chisel state
pub chisel_state: Option<(Vec<U256>, Vec<u8>)>,
pub reverter: Option<Address>,
/// Revert payloads minted by the `skip` cheatcode during this call.
///
/// Copied out of the cheatcode state on conversion since `commit` moves that state back into
/// the executor before results are classified.
pub skip_payloads: Vec<Bytes>,
}

impl<FEN: FoundryEvmNetwork> Default for RawCallResult<FEN> {
Expand Down Expand Up @@ -1328,6 +1335,7 @@ impl<FEN: FoundryEvmNetwork> Default for RawCallResult<FEN> {
fork_block_number: None,
chisel_state: None,
reverter: None,
skip_payloads: Vec::new(),
}
}
}
Expand All @@ -1342,11 +1350,20 @@ impl<FEN: FoundryEvmNetwork> RawCallResult<FEN> {
}
}

/// Returns the skip reason if this call reverted with a genuine `vm.skip` payload.
///
/// The revert data must byte-equal a payload recorded by the skip cheatcode during this call;
/// a matching `FOUNDRY::SKIP` prefix alone (user-crafted revert data) does not count.
pub fn skip_reason(&self) -> Option<SkipReason> {
if !self.reverted || !self.skip_payloads.contains(&self.result) {
return None;
}
SkipReason::decode(&self.result)
}

/// Converts the result of the call into an `EvmError`.
pub fn into_evm_error(self, rd: Option<&RevertDecoder>) -> EvmError<FEN> {
if self.reverter == Some(CHEATCODE_ADDRESS)
&& let Some(reason) = SkipReason::decode(&self.result)
{
if let Some(reason) = self.skip_reason() {
return EvmError::Skip(reason);
}
let reason = rd.unwrap_or_default().decode(&self.result, self.exit_reason);
Expand Down Expand Up @@ -1604,6 +1621,7 @@ fn convert_executed_result<FEN: FoundryEvmNetwork>(
.as_ref()
.map(|c| c.broadcastable_transactions.clone())
.filter(|txs| !txs.is_empty());
let skip_payloads = cheatcodes.as_ref().map(|c| c.skip_payloads.clone()).unwrap_or_default();

Ok(RawCallResult {
exit_reason: Some(exit_reason),
Expand Down Expand Up @@ -1633,6 +1651,7 @@ fn convert_executed_result<FEN: FoundryEvmNetwork>(
fork_block_number,
chisel_state,
reverter,
skip_payloads,
})
}

Expand Down Expand Up @@ -1871,8 +1890,9 @@ mod tests {
#[test]
fn cheatcode_skip_payload_is_classified_as_skip() {
let raw = RawCallResult::<EthEvmNetwork> {
reverted: true,
result: Bytes::from_static(b"FOUNDRY::SKIPwith reason"),
reverter: Some(CHEATCODE_ADDRESS),
skip_payloads: vec![Bytes::from_static(b"FOUNDRY::SKIPwith reason")],
..Default::default()
};

Expand All @@ -1881,10 +1901,11 @@ mod tests {
}

#[test]
fn forged_skip_payload_from_non_cheatcode_is_execution_error() {
fn forged_skip_payload_is_execution_error() {
let raw = RawCallResult::<EthEvmNetwork> {
reverted: true,
result: Bytes::from_static(MAGIC_SKIP),
reverter: Some(CALLER),
reverter: Some(CHEATCODE_ADDRESS),
..Default::default()
};

Expand All @@ -1893,10 +1914,11 @@ mod tests {
}

#[test]
fn skip_payload_without_reverter_is_execution_error() {
fn mismatched_skip_payload_is_execution_error() {
let raw = RawCallResult::<EthEvmNetwork> {
result: Bytes::from_static(MAGIC_SKIP),
reverter: None,
reverted: true,
result: Bytes::from_static(b"FOUNDRY::SKIPforged"),
skip_payloads: vec![Bytes::from_static(b"FOUNDRY::SKIPgenuine")],
..Default::default()
};

Expand Down
16 changes: 3 additions & 13 deletions crates/evm/evm/src/executors/showmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@ use alloy_json_abi::Function;
use alloy_primitives::{Address, B256, Selector, hex, keccak256};
use eyre::Result;
use foundry_config::FuzzCorpusConfig;
use foundry_evm_core::{
constants::{CHEATCODE_ADDRESS, MAGIC_ASSUME},
decode::SkipReason,
evm::FoundryEvmNetwork,
};
use foundry_evm_core::{constants::MAGIC_ASSUME, evm::FoundryEvmNetwork};
use foundry_evm_coverage::HitMaps;
use foundry_evm_fuzz::{BasicTxDetails, invariant::FuzzRunIdentifiedContracts};
use std::{
Expand Down Expand Up @@ -324,10 +320,7 @@ pub fn replay_corpus_to_showmap<FEN: FoundryEvmNetwork>(
let fingerprint = snapshot_edge_fingerprint(&call_result);
// `vm.assume` rejects and cheatcode `vm.skip` are discarded by the campaign: the call
// is not committed, checked, or counted toward coverage.
if call_result.result.as_ref() == MAGIC_ASSUME
|| (call_result.reverter == Some(CHEATCODE_ADDRESS)
&& SkipReason::decode(&call_result.result).is_some())
{
if call_result.result.as_ref() == MAGIC_ASSUME || call_result.skip_reason().is_some() {
continue;
}
// Coverage-collection asymmetry across calls within a stateful sequence:
Expand Down Expand Up @@ -521,10 +514,7 @@ pub fn replay_sequence_for_minimization<FEN: FoundryEvmNetwork>(
tx.call_details.calldata.get(..4).map(Selector::from_slice).unwrap_or_default();
let fingerprint = snapshot_edge_fingerprint(&call_result);

if call_result.result.as_ref() == MAGIC_ASSUME
|| (call_result.reverter == Some(CHEATCODE_ADDRESS)
&& SkipReason::decode(&call_result.result).is_some())
{
if call_result.result.as_ref() == MAGIC_ASSUME || call_result.skip_reason().is_some() {
observation.skipped += 1;
continue;
}
Expand Down
6 changes: 2 additions & 4 deletions crates/forge/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use foundry_config::{
Config, FuzzConfig, FuzzCorpusConfig, FuzzDictionaryConfig, InlineConfig, InvariantConfig,
};
use foundry_evm::{
constants::{CALLER, CHEATCODE_ADDRESS, MAGIC_ASSUME},
constants::{CALLER, MAGIC_ASSUME},
core::{backend::DatabaseExt, evm::FoundryEvmNetwork},
decode::{RevertDecoder, SkipReason},
executors::{
Expand Down Expand Up @@ -1745,9 +1745,7 @@ impl<'a, FEN: FoundryEvmNetwork> FunctionRunner<'a, FEN> {
&self,
raw_call_result: &RawCallResult<FEN>,
) -> Result<Option<String>, String> {
if raw_call_result.reverter == Some(CHEATCODE_ADDRESS)
&& let Some(reason) = SkipReason::decode(&raw_call_result.result)
{
if let Some(reason) = raw_call_result.skip_reason() {
return Err(format!("vm.skip during concrete replay: {reason}"));
}

Expand Down
Loading
Loading