Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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