feat(cheatcodes): disallow usage of expectRevert with expectCall and expectEmit

evm/src/executor/inspector/cheatcodes/expect.rs

@@ -45,6 +45,16 @@ fn expect_revert(state: &mut Cheatcodes, reason: Option<Bytes>, depth: u64) -> R
     Ok(Bytes::new())
 }
 
+fn expect_emit(
+    state: &mut Cheatcodes,
+    address: Option<H160>,
+    depth: u64,
+    checks: [bool; 4],
+) -> Result {
+    state.expected_emits.push_back(ExpectedEmit { depth, address, checks, ..Default::default() });
+    Ok(Bytes::new())
+}
+
 #[instrument(skip_all, fields(expected_revert, status, retdata = hex::encode(&retdata)))]
 pub fn handle_expect_revert(
     is_create: bool,
@@ -347,39 +357,26 @@ pub fn apply<DB: DatabaseExt>(
             expect_revert(state, Some(inner.0.into()), data.journaled_state.depth())
         }
         HEVMCalls::ExpectEmit0(_) => {
-            state.expected_emits.push_back(ExpectedEmit {
-                depth: data.journaled_state.depth(),
-                checks: [true, true, true, true],
-                ..Default::default()
-            });
-            Ok(Bytes::new())
-        }
-        HEVMCalls::ExpectEmit1(inner) => {
-            state.expected_emits.push_back(ExpectedEmit {
-                depth: data.journaled_state.depth(),
-                checks: [true, true, true, true],
-                address: Some(inner.0),
-                ..Default::default()
-            });
-            Ok(Bytes::new())
-        }
-        HEVMCalls::ExpectEmit2(inner) => {
-            state.expected_emits.push_back(ExpectedEmit {
-                depth: data.journaled_state.depth(),
-                checks: [inner.0, inner.1, inner.2, inner.3],
-                ..Default::default()
-            });
-            Ok(Bytes::new())
-        }
-        HEVMCalls::ExpectEmit3(inner) => {
-            state.expected_emits.push_back(ExpectedEmit {
-                depth: data.journaled_state.depth(),
-                checks: [inner.0, inner.1, inner.2, inner.3],
-                address: Some(inner.4),
-                ..Default::default()
-            });
-            Ok(Bytes::new())
+            expect_emit(state, None, data.journaled_state.depth(), [true, true, true, true])
         }
+        HEVMCalls::ExpectEmit1(inner) => expect_emit(
+            state,
+            Some(inner.0),
+            data.journaled_state.depth(),
+            [true, true, true, true],
+        ),
+        HEVMCalls::ExpectEmit2(inner) => expect_emit(
+            state,
+            None,
+            data.journaled_state.depth(),
+            [inner.0, inner.1, inner.2, inner.3],
+        ),
+        HEVMCalls::ExpectEmit3(inner) => expect_emit(
+            state,
+            Some(inner.4),
+            data.journaled_state.depth(),
+            [inner.0, inner.1, inner.2, inner.3],
+        ),
         HEVMCalls::ExpectCall0(inner) => expect_call(
             state,
             inner.0,

evm/src/executor/inspector/cheatcodes/mod.rs

@@ -770,6 +770,15 @@ where
 
         // Handle expected reverts
         if let Some(expected_revert) = &self.expected_revert {
+            // Irrespective of whether a revert will be matched or not, disallow having expected
+            // reverts alongside expected emits or calls.
+            if !self.expected_calls.is_empty() || !self.expected_emits.is_empty() {
+                return (
+                    InstructionResult::Revert,
+                    remaining_gas,
+                    "Cannot expect a function to revert while trying to match expected calls or events.".to_string().encode().into(),
+                )
+            }
             if data.journaled_state.depth() == expected_revert.depth {
                 let expected_revert = std::mem::take(&mut self.expected_revert).unwrap();
                 return match handle_expect_revert(

testdata/cheats/ExpectCall.t.sol

@@ -254,6 +254,14 @@ contract ExpectCallTest is DSTest {
         cheats.expectCallMinGas(address(inner), 0, 50_001, abi.encodeWithSelector(inner.add.selector, 1, 1));
         this.exposed_addHardGasLimit(target);
     }
+
+    /// Ensure that you cannot use expectCall with an expectRevert.
+    function testFailExpectCallWithRevertDisallowed() public {
+        Contract target = new Contract();
+        cheats.expectRevert();
+        cheats.expectCall(address(target), abi.encodeWithSelector(target.add.selector));
+        this.exposed_callTargetNTimes(target, 5, 5, 1);
+    }
 }
 
 contract ExpectCallCountTest is DSTest {

testdata/cheats/ExpectEmit.t.sol

@@ -569,6 +569,15 @@ contract ExpectEmitTest is DSTest {
         emitter.emitWindowAndOnTest(this);
     }
 
+    /// We should not be able to expect emits if we're expecting the function reverts, no matter
+    /// if the function reverts or not.
+    function testFailEmitWindowWithRevertDisallowed() public {
+        cheats.expectRevert();
+        cheats.expectEmit(true, false, false, true);
+        emit A(1);
+        emitter.emitWindow();
+    }
+
     /// This test will fail if we check that all expected logs were emitted
     /// after every call from the same depth as the call that invoked the cheatcode.
     ///