fix(cheatcodes): do not account already matched emit events when fill or check (#8824)

* fix(6643): do not account already matched events when fill or check

* Move test as repro test

Author: grandizzy

crates/cheatcodes/src/test/expect.rs

@@ -464,14 +464,16 @@ fn expect_emit(
     address: Option<Address>,
     anonymous: bool,
 ) -> Result {
-    state.expected_emits.push_back(ExpectedEmit {
-        depth,
-        checks,
-        address,
-        found: false,
-        log: None,
-        anonymous,
-    });
+    let expected_emit = ExpectedEmit { depth, checks, address, found: false, log: None, anonymous };
+    if let Some(found_emit_pos) = state.expected_emits.iter().position(|emit| emit.found) {
+        // The order of emits already found (back of queue) should not be modified, hence push any
+        // new emit before first found emit.
+        state.expected_emits.insert(found_emit_pos, expected_emit);
+    } else {
+        // If no expected emits then push new one at the back of queue.
+        state.expected_emits.push_back(expected_emit);
+    }
+
     Ok(Default::default())
 }
 
@@ -494,23 +496,34 @@ pub(crate) fn handle_expect_emit(
         return
     }
 
-    // If there's anything to fill, we need to pop back.
-    // Otherwise, if there are any events that are unmatched, we try to match to match them
-    // in the order declared, so we start popping from the front (like a queue).
-    let mut event_to_fill_or_check =
-        if state.expected_emits.iter().any(|expected| expected.log.is_none()) {
-            state.expected_emits.pop_back()
-        } else {
-            state.expected_emits.pop_front()
-        }
+    let should_fill_logs = state.expected_emits.iter().any(|expected| expected.log.is_none());
+    let index_to_fill_or_check = if should_fill_logs {
+        // If there's anything to fill, we start with the last event to match in the queue
+        // (without taking into account events already matched).
+        state
+            .expected_emits
+            .iter()
+            .position(|emit| emit.found)
+            .unwrap_or(state.expected_emits.len())
+            .saturating_sub(1)
+    } else {
+        // Otherwise, if all expected logs are filled, we start to check any unmatched event
+        // in the declared order, so we start from the front (like a queue).
+        0
+    };
+
+    let mut event_to_fill_or_check = state
+        .expected_emits
+        .remove(index_to_fill_or_check)
         .expect("we should have an emit to fill or check");
 
     let Some(expected) = &event_to_fill_or_check.log else {
         // Unless the caller is trying to match an anonymous event, the first topic must be
         // filled.
         if event_to_fill_or_check.anonymous || log.topics().first().is_some() {
             event_to_fill_or_check.log = Some(log.data.clone());
-            state.expected_emits.push_back(event_to_fill_or_check);
+            // If we only filled the expected log then we put it back at the same position.
+            state.expected_emits.insert(index_to_fill_or_check, event_to_fill_or_check);
         } else {
             interpreter.instruction_result = InstructionResult::Revert;
             interpreter.next_action = InterpreterAction::Return {

crates/forge/tests/it/repros.rs

@@ -378,3 +378,6 @@ test_repro!(8383, false, None, |res| {
 
 // https://github.com/foundry-rs/foundry/issues/1543
 test_repro!(1543);
+
+// https://github.com/foundry-rs/foundry/issues/6643
+test_repro!(6643);

testdata/default/repros/Issue6643.t.sol

@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: MIT OR Apache-2.0
+pragma solidity 0.8.18;
+
+import "ds-test/test.sol";
+import "cheats/Vm.sol";
+
+contract Counter {
+    event TestEvent(uint256 n);
+    event AnotherTestEvent(uint256 n);
+
+    constructor() {
+        emit TestEvent(1);
+    }
+
+    function f() external {
+        emit TestEvent(2);
+    }
+
+    function g() external {
+        emit AnotherTestEvent(1);
+        this.f();
+        emit AnotherTestEvent(2);
+    }
+}
+
+// https://github.com/foundry-rs/foundry/issues/6643
+contract Issue6643Test is DSTest {
+    Vm constant vm = Vm(HEVM_ADDRESS);
+    Counter public counter;
+
+    event TestEvent(uint256 n);
+    event AnotherTestEvent(uint256 n);
+
+    function setUp() public {
+        counter = new Counter();
+    }
+
+    function test_Bug1() public {
+        // part1
+        vm.expectEmit();
+        emit TestEvent(1);
+        new Counter();
+        // part2
+        vm.expectEmit();
+        emit TestEvent(2);
+        counter.f();
+        // part3
+        vm.expectEmit();
+        emit AnotherTestEvent(1);
+        vm.expectEmit();
+        emit TestEvent(2);
+        vm.expectEmit();
+        emit AnotherTestEvent(2);
+        counter.g();
+    }
+
+    function test_Bug2() public {
+        vm.expectEmit();
+        emit TestEvent(1);
+        new Counter();
+        vm.expectEmit();
+        emit TestEvent(1);
+        new Counter();
+    }
+}