From ed112a72f566f38ad300e1a5a860deef5a347e15 Mon Sep 17 00:00:00 2001 From: "emo.eth" Date: Tue, 5 Nov 2024 10:15:41 -0800 Subject: [PATCH] remove assumeNoPartialRevert; update assumeNoPartialRevert --- crates/cheatcodes/assets/cheatcodes.json | 48 ++----------------- crates/cheatcodes/spec/src/vm.rs | 16 ++----- crates/cheatcodes/src/test/assume.rs | 39 ++------------- crates/cheatcodes/src/test/revert_handlers.rs | 1 + crates/forge/tests/cli/test_cmd.rs | 17 +++---- testdata/cheats/Vm.sol | 2 - testdata/default/cheats/AssumeNoRevert.t.sol | 8 ++-- 7 files changed, 27 insertions(+), 104 deletions(-) diff --git a/crates/cheatcodes/assets/cheatcodes.json b/crates/cheatcodes/assets/cheatcodes.json index 398ab88f46d34..414fc383ff9d9 100644 --- a/crates/cheatcodes/assets/cheatcodes.json +++ b/crates/cheatcodes/assets/cheatcodes.json @@ -3056,46 +3056,6 @@ "status": "stable", "safety": "safe" }, - { - "func": { - "id": "assumeNoPartialRevert_0", - "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoRevert`.", - "declaration": "function assumeNoPartialRevert(bytes4 revertData) external pure;", - "visibility": "external", - "mutability": "pure", - "signature": "assumeNoPartialRevert(bytes4)", - "selector": "0xba286427", - "selectorBytes": [ - 186, - 40, - 100, - 39 - ] - }, - "group": "testing", - "status": "stable", - "safety": "safe" - }, - { - "func": { - "id": "assumeNoPartialRevert_1", - "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoRevert`.", - "declaration": "function assumeNoPartialRevert(bytes4 revertData, address reverter) external pure;", - "visibility": "external", - "mutability": "pure", - "signature": "assumeNoPartialRevert(bytes4,address)", - "selector": "0x3d5c537e", - "selectorBytes": [ - 61, - 92, - 83, - 126 - ] - }, - "group": "testing", - "status": "stable", - "safety": "safe" - }, { "func": { "id": "assumeNoRevert_0", @@ -3119,7 +3079,7 @@ { "func": { "id": "assumeNoRevert_1", - "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoPartialRevert`.", + "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons.", "declaration": "function assumeNoRevert(bytes4 revertData) external pure;", "visibility": "external", "mutability": "pure", @@ -3139,7 +3099,7 @@ { "func": { "id": "assumeNoRevert_2", - "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoPartialRevert`.", + "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons.", "declaration": "function assumeNoRevert(bytes calldata revertData) external pure;", "visibility": "external", "mutability": "pure", @@ -3159,7 +3119,7 @@ { "func": { "id": "assumeNoRevert_3", - "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoRevert`.", + "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons.", "declaration": "function assumeNoRevert(bytes4 revertData, address reverter) external pure;", "visibility": "external", "mutability": "pure", @@ -3179,7 +3139,7 @@ { "func": { "id": "assumeNoRevert_4", - "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoPartialRevert`.", + "description": "Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons.", "declaration": "function assumeNoRevert(bytes calldata revertData, address reverter) external pure;", "visibility": "external", "mutability": "pure", diff --git a/crates/cheatcodes/spec/src/vm.rs b/crates/cheatcodes/spec/src/vm.rs index 8b8051931d9dc..a06fa097fe517 100644 --- a/crates/cheatcodes/spec/src/vm.rs +++ b/crates/cheatcodes/spec/src/vm.rs @@ -825,30 +825,22 @@ interface Vm { #[cheatcode(group = Testing, safety = Safe)] function assumeNoRevert() external pure; - /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoPartialRevert`. + /// Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons. #[cheatcode(group = Testing, safety = Safe)] function assumeNoRevert(bytes4 revertData) external pure; - /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoPartialRevert`. + /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. #[cheatcode(group = Testing, safety = Safe)] function assumeNoRevert(bytes calldata revertData) external pure; - /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoRevert`. + /// Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons. #[cheatcode(group = Testing, safety = Safe)] function assumeNoRevert(bytes4 revertData, address reverter) external pure; - /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoPartialRevert`. + /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons. #[cheatcode(group = Testing, safety = Safe)] function assumeNoRevert(bytes calldata revertData, address reverter) external pure; - /// Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoRevert`. - #[cheatcode(group = Testing, safety = Safe)] - function assumeNoPartialRevert(bytes4 revertData) external pure; - - /// Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons. Can be combined with `assumeNoRevert`. - #[cheatcode(group = Testing, safety = Safe)] - function assumeNoPartialRevert(bytes4 revertData, address reverter) external pure; - /// Writes a breakpoint to jump to in the debugger. #[cheatcode(group = Testing, safety = Safe)] function breakpoint(string calldata char) external pure; diff --git a/crates/cheatcodes/src/test/assume.rs b/crates/cheatcodes/src/test/assume.rs index d0ebdff5d96dc..bf256a9ee9f47 100644 --- a/crates/cheatcodes/src/test/assume.rs +++ b/crates/cheatcodes/src/test/assume.rs @@ -2,8 +2,8 @@ use crate::{Cheatcode, Cheatcodes, CheatsCtxt, Error, Result}; use alloy_primitives::Address; use foundry_evm_core::constants::MAGIC_ASSUME; use spec::Vm::{ - assumeCall, assumeNoPartialRevert_0Call, assumeNoPartialRevert_1Call, assumeNoRevert_0Call, - assumeNoRevert_1Call, assumeNoRevert_2Call, assumeNoRevert_3Call, assumeNoRevert_4Call, + assumeCall, assumeNoRevert_0Call, assumeNoRevert_1Call, assumeNoRevert_2Call, + assumeNoRevert_3Call, assumeNoRevert_4Call, }; use std::fmt::Debug; @@ -47,7 +47,7 @@ impl Cheatcode for assumeCall { impl Cheatcode for assumeNoRevert_0Call { fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result { - assume_no_revert(ccx.state, ccx.ecx.journaled_state.depth(), None, false, None) + assume_no_revert(ccx.state, ccx.ecx.journaled_state.depth(), None, None) } } @@ -58,7 +58,6 @@ impl Cheatcode for assumeNoRevert_1Call { ccx.state, ccx.ecx.journaled_state.depth(), Some(revertData.to_vec()), - false, None, ) } @@ -70,7 +69,6 @@ impl Cheatcode for assumeNoRevert_2Call { ccx.state, ccx.ecx.journaled_state.depth(), Some(revertData.to_vec()), - false, None, ) } @@ -82,7 +80,6 @@ impl Cheatcode for assumeNoRevert_3Call { ccx.state, ccx.ecx.journaled_state.depth(), Some(revertData.to_vec()), - false, Some(*reverter), ) } @@ -94,33 +91,6 @@ impl Cheatcode for assumeNoRevert_4Call { ccx.state, ccx.ecx.journaled_state.depth(), Some(revertData.to_vec()), - false, - Some(*reverter), - ) - } -} - -impl Cheatcode for assumeNoPartialRevert_0Call { - fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result { - let Self { revertData } = self; - assume_no_revert( - ccx.state, - ccx.ecx.journaled_state.depth(), - Some(revertData.to_vec()), - true, - None, - ) - } -} - -impl Cheatcode for assumeNoPartialRevert_1Call { - fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result { - let Self { revertData, reverter } = self; - assume_no_revert( - ccx.state, - ccx.ecx.journaled_state.depth(), - Some(revertData.to_vec()), - true, Some(*reverter), ) } @@ -130,11 +100,12 @@ fn assume_no_revert( state: &mut Cheatcodes, depth: u64, reason: Option>, - partial_match: bool, reverter: Option
, ) -> Result { ensure!(state.expected_revert.is_none(), ASSUME_EXPECT_REJECT_MAGIC); + // if reason is not none, check if it is 4 bytes; if so, allow partial matches + let partial_match = reason.is_some() && reason.as_ref().unwrap().len() == 4; // if assume_no_revert is not set, set it if state.assume_no_revert.is_none() { state.assume_no_revert = Some(AssumeNoRevert { depth, reasons: vec![], reverted_by: None }); diff --git a/crates/cheatcodes/src/test/revert_handlers.rs b/crates/cheatcodes/src/test/revert_handlers.rs index 7c378b0a09a30..1788709cd9d36 100644 --- a/crates/cheatcodes/src/test/revert_handlers.rs +++ b/crates/cheatcodes/src/test/revert_handlers.rs @@ -28,6 +28,7 @@ pub(crate) trait RevertParameters { fn reason(&self) -> Option<&[u8]>; fn partial_match(&self) -> bool; } + impl RevertParameters for AcceptableRevertParameters { fn reverter(&self) -> Option
{ self.reverter diff --git a/crates/forge/tests/cli/test_cmd.rs b/crates/forge/tests/cli/test_cmd.rs index 6ab7f309a1026..ab66e0a0204a3 100644 --- a/crates/forge/tests/cli/test_cmd.rs +++ b/crates/forge/tests/cli/test_cmd.rs @@ -2500,6 +2500,15 @@ forgetest_init!(test_assume_no_revert_with_data, |prj, cmd| { _vm.assumeNoRevert(abi.encodeWithSelector(Reverter.RevertWithData.selector, 4), address(reverter)); reverter.twoPossibleReverts(x); } + + /// @dev Test that `assumeNoRevert` assumptions are cleared after the first non-cheatcode external call + function testMultipleAssumesClearAfterCall_fails(uint256 x) public view { + _vm.assumeNoRevert(Reverter.MyRevert.selector); + _vm.assumeNoRevert(Reverter.RevertWithData.selector, address(reverter)); + reverter.twoPossibleReverts(x); + + reverter.twoPossibleReverts(2); + } /// @dev Test that `assumeNoRevert` correctly rejects a generic assumeNoRevert call after any specific reason is provided function testMultipleAssumes_ThrowOnGenericNoRevert_AfterSpecific_fails(bytes4 selector) public view { @@ -2560,14 +2569,6 @@ forgetest_init!(test_assume_no_revert_with_data, |prj, cmd| { "testMultipleAssumesClearAfterCall_fails", "FAIL: MyRevert(); counterexample:", ); - // need a better way to handle cheatcodes reverting; currently their messages get abi-encoded - // and treated like normal evm data, which makes them hard (and inefficient) to match in the - // handler - // assert_failure_contains( - // &mut cmd, - // "testMultipleAssumes_ThrowOnGenericNoRevert_fails", - // "FAIL: vm.assumeNoRevert: cannot combine a generic assumeNoRevert with specific - // assumeNoRevert reasons;", ); assert_failure_contains( &mut cmd, "testMultipleAssumes_ThrowOnGenericNoRevert_AfterSpecific_fails", diff --git a/testdata/cheats/Vm.sol b/testdata/cheats/Vm.sol index ff2518dd40207..3db40fff62e25 100644 --- a/testdata/cheats/Vm.sol +++ b/testdata/cheats/Vm.sol @@ -147,8 +147,6 @@ interface Vm { function assertTrue(bool condition) external pure; function assertTrue(bool condition, string calldata error) external pure; function assume(bool condition) external pure; - function assumeNoPartialRevert(bytes4 revertData) external pure; - function assumeNoPartialRevert(bytes4 revertData, address reverter) external pure; function assumeNoRevert() external pure; function assumeNoRevert(bytes4 revertData) external pure; function assumeNoRevert(bytes calldata revertData) external pure; diff --git a/testdata/default/cheats/AssumeNoRevert.t.sol b/testdata/default/cheats/AssumeNoRevert.t.sol index 393cfb9a9dd12..49d6b19c158bc 100644 --- a/testdata/default/cheats/AssumeNoRevert.t.sol +++ b/testdata/default/cheats/AssumeNoRevert.t.sol @@ -80,9 +80,9 @@ contract ReverterTest is Test { reverter.revertWithDataIf2(x); } - /// @dev Test that `assumeNoPartialRevert` anticipates and correctly rejects a specific error selector with any extra data + /// @dev Test that `assumeNoRevert` anticipates and correctly rejects a specific error selector with any extra data (ie providing selector allows for arbitrary extra data) function testAssumeWithDataPartial(uint256 x) public view { - _vm.assumeNoPartialRevert(Reverter.RevertWithData.selector); + _vm.assumeNoRevert(Reverter.RevertWithData.selector); reverter.revertWithDataIf2(x); } @@ -100,9 +100,9 @@ contract ReverterTest is Test { reverter.twoPossibleReverts(x); } - /// @dev Test that `assumeNoPartialRevert` correctly interacts with `assumeNoRevert` + /// @dev Test that `assumeNoRevert` correctly interacts with itself when partially matching on the error selector function testMultipleAssumes_Partial(uint256 x) public view { - _vm.assumeNoPartialRevert(Reverter.RevertWithData.selector); + _vm.assumeNoRevert(Reverter.RevertWithData.selector); _vm.assumeNoRevert(Reverter.MyRevert.selector); reverter.twoPossibleReverts(x); }