Skip to content

Commit

Permalink
Add config to allow expect revert for internal calls
Browse files Browse the repository at this point in the history
  • Loading branch information
grandizzy committed Dec 12, 2024
1 parent 93aec03 commit df77c40
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 5 deletions.
4 changes: 4 additions & 0 deletions crates/cheatcodes/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub struct CheatsConfig {
pub assertions_revert: bool,
/// Optional seed for the RNG algorithm.
pub seed: Option<U256>,
/// Whether to allow `expectRevert` to work for internal calls.
pub internal_expect_revert: bool,
}

impl CheatsConfig {
Expand Down Expand Up @@ -98,6 +100,7 @@ impl CheatsConfig {
running_version,
assertions_revert: config.assertions_revert,
seed: config.fuzz.seed,
internal_expect_revert: config.allow_internal_expect_revert,
}
}

Expand Down Expand Up @@ -239,6 +242,7 @@ impl Default for CheatsConfig {
running_version: Default::default(),
assertions_revert: true,
seed: None,
internal_expect_revert: false,
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,7 @@ where {
let handler_result = expect::handle_expect_revert(
false,
true,
self.config.internal_expect_revert,
&mut expected_revert,
outcome.result.result,
outcome.result.output.clone(),
Expand Down Expand Up @@ -1339,6 +1340,7 @@ impl Inspector<&mut dyn DatabaseExt> for Cheatcodes {
let handler_result = expect::handle_expect_revert(
cheatcode_call,
false,
self.config.internal_expect_revert,
&mut expected_revert,
outcome.result.result,
outcome.result.output.clone(),
Expand Down
4 changes: 3 additions & 1 deletion crates/cheatcodes/src/test/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,7 @@ fn expect_revert(
pub(crate) fn handle_expect_revert(
is_cheatcode: bool,
is_create: bool,
internal_expect_revert: bool,
expected_revert: &mut ExpectedRevert,
status: InstructionResult,
retdata: Bytes,
Expand All @@ -809,7 +810,8 @@ pub(crate) fn handle_expect_revert(
hex::encode_prefixed(data)
};

if !is_cheatcode {
// Check depths if it's not an expect cheatcode call and if internal expect reverts not enabled.
if !is_cheatcode && !internal_expect_revert {
ensure!(
expected_revert.max_depth > expected_revert.depth,
"call didn't revert at a lower depth than cheatcode call depth"
Expand Down
3 changes: 3 additions & 0 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ pub struct Config {
pub invariant: InvariantConfig,
/// Whether to allow ffi cheatcodes in test
pub ffi: bool,
/// Whether to allow `expectRevert` for internal functions.
pub allow_internal_expect_revert: bool,
/// Use the create 2 factory in all cases including tests and non-broadcasting scripts.
pub always_use_create_2_factory: bool,
/// Sets a timeout in seconds for vm.prompt cheatcodes
Expand Down Expand Up @@ -2310,6 +2312,7 @@ impl Default for Config {
invariant: InvariantConfig::new("cache/invariant".into()),
always_use_create_2_factory: false,
ffi: false,
allow_internal_expect_revert: false,
prompt_timeout: 120,
sender: Self::DEFAULT_SENDER,
tx_origin: Self::DEFAULT_SENDER,
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ forgetest!(can_extract_config_values, |prj, cmd| {
..Default::default()
},
ffi: true,
allow_internal_expect_revert: false,
always_use_create_2_factory: false,
prompt_timeout: 0,
sender: "00a329c0648769A73afAc7F9381D08FB43dBEA72".parse().unwrap(),
Expand Down
1 change: 1 addition & 0 deletions crates/test-utils/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ impl ExtTester {
test_cmd.env("FOUNDRY_FORK_BLOCK_NUMBER", fork_block.to_string());
}
test_cmd.env("FOUNDRY_INVARIANT_DEPTH", "15");
test_cmd.env("FOUNDRY_ALLOW_INTERNAL_EXPECT_REVERT", "true");

test_cmd.assert_success();
}
Expand Down
4 changes: 3 additions & 1 deletion testdata/default/cheats/AttachDelegation.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ contract AttachDelegationTest is DSTest {
assertEq(token.balanceOf(bob), 200);
}

function testFailAttachDelegationRevertInvalidSignature() public {
/// forge-config: default.allow_internal_expect_revert = true
function testAttachDelegationRevertInvalidSignature() public {
Vm.SignedDelegation memory signedDelegation = vm.signDelegation(address(implementation), alice_pk);
// change v from 1 to 0
signedDelegation.v = (signedDelegation.v + 1) % 2;
Expand All @@ -98,6 +99,7 @@ contract AttachDelegationTest is DSTest {

vm.broadcast(alice_pk);
// empty revert because no bytecode was set to Alice's account
vm.expectRevert();
SimpleDelegateContract(alice).execute(calls);
}

Expand Down
10 changes: 8 additions & 2 deletions testdata/default/cheats/MemSafety.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,12 @@ contract MemSafetyTest is DSTest {

/// @dev Tests that expanding memory outside of the range given to `expectSafeMemory`
/// will cause the test to fail while using the `MLOAD` opcode.
function testFailExpectSafeMemory_MLOAD_REVERT() public {
/// forge-config: default.allow_internal_expect_revert = true
function testExpectSafeMemory_MLOAD_REVERT() public {
vm.expectSafeMemory(0x80, 0x100);

vm.expectRevert();

// This should revert. Ugly hack to make sure the mload isn't optimized
// out.
uint256 a;
Expand Down Expand Up @@ -501,8 +505,10 @@ contract MemSafetyTest is DSTest {

/// @dev Tests that expanding memory outside of the range given to `expectSafeMemory`
/// will cause the test to fail while using the `LOG0` opcode.
function testFailExpectSafeMemory_LOG0_REVERT() public {
/// forge-config: default.allow_internal_expect_revert = true
function testExpectSafeMemory_LOG0_REVERT() public {
vm.expectSafeMemory(0x80, 0x100);
vm.expectRevert();
// This should revert.
assembly {
log0(0x100, 0x20)
Expand Down
4 changes: 3 additions & 1 deletion testdata/default/repros/Issue7457.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ contract Issue7457Test is DSTest, ITarget {
target.emitAnonymousEventEmpty();
}

function testFailEmitEventNonIndexedReverts() public {
/// forge-config: default.allow_internal_expect_revert = true
function testEmitEventNonIndexedReverts() public {
vm.expectEmit(false, false, false, true);
vm.expectRevert("use vm.expectEmitAnonymous to match anonymous events");
emit AnonymousEventNonIndexed(1);
}

Expand Down

0 comments on commit df77c40

Please sign in to comment.