feat(cheatcodes): Make expectEmit only work for the next call

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

@@ -123,46 +123,79 @@ pub struct ExpectedEmit {
 }
 
 pub fn handle_expect_emit(state: &mut Cheatcodes, log: RawLog, address: &Address) {
-    // Fill or check the expected emits
-    if let Some(next_expect_to_fill) =
-        state.expected_emits.iter_mut().find(|expect| expect.log.is_none())
-    {
-        // We have unfilled expects, so we fill the first one
-        next_expect_to_fill.log = Some(log);
-    } else if let Some(next_expect) = state.expected_emits.iter_mut().find(|expect| !expect.found) {
-        // We do not have unfilled expects, so we try to match this log with the first unfound
-        // log that we expect
-        let expected =
-            next_expect.log.as_ref().expect("we should have a log to compare against here");
-
-        let expected_topic_0 = expected.topics.get(0);
-        let log_topic_0 = log.topics.get(0);
-
-        // same topic0 and equal number of topics should be verified further, others are a no
-        // match
-        if expected_topic_0
-            .zip(log_topic_0)
-            .map_or(false, |(a, b)| a == b && expected.topics.len() == log.topics.len())
-        {
-            // Match topics
-            next_expect.found = log
-                .topics
-                .iter()
-                .skip(1)
-                .enumerate()
-                .filter(|(i, _)| next_expect.checks[*i])
-                .all(|(i, topic)| topic == &expected.topics[i + 1]);
-
-            // Maybe match source address
-            if let Some(addr) = next_expect.address {
-                next_expect.found &= addr == *address;
+    // Fill or check the expected emits.
+    // We expect for emit checks to be filled as they're declared (from oldest to newest),
+    // so we fill them and push them to the back of the queue.
+    // If the user has properly filled all the emits, they'll end up in their original order.
+    // If not, the queue will not be in the order the events will be intended to be filled,
+    // and we'll be able to later detect this and bail.
+
+    // First, we can return early if all events have been matched.
+    // This allows a contract to arbitrarily emit more events than expected (additive behavior),
+    // as long as all the previous events were matched in the order they were expected to be.
+    if state.expected_emits.iter().all(|expected| expected.found) {
+        return
+    }
+
+    // if there's anything to fill, we need to pop back.
+    let event_to_fill_or_check =
+        if state.expected_emits.iter().any(|expected| expected.log.is_none()) {
+            state.expected_emits.pop_back()
+        // Else, 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).
+        } else {
+            state.expected_emits.pop_front()
+        };
+
+    let mut event_to_fill_or_check =
+        event_to_fill_or_check.expect("We should have an emit to fill or check. This is a bug");
+
+    match event_to_fill_or_check.log {
+        Some(ref expected) => {
+            let expected_topic_0 = expected.topics.get(0);
+            let log_topic_0 = log.topics.get(0);
+
+            // same topic0 and equal number of topics should be verified further, others are a no
+            // match
+            if expected_topic_0
+                .zip(log_topic_0)
+                .map_or(false, |(a, b)| a == b && expected.topics.len() == log.topics.len())
+            {
+                // Match topics
+                event_to_fill_or_check.found = log
+                    .topics
+                    .iter()
+                    .skip(1)
+                    .enumerate()
+                    .filter(|(i, _)| event_to_fill_or_check.checks[*i])
+                    .all(|(i, topic)| topic == &expected.topics[i + 1]);
+
+                // Maybe match source address
+                if let Some(addr) = event_to_fill_or_check.address {
+                    event_to_fill_or_check.found &= addr == *address;
+                }
+
+                // Maybe match data
+                if event_to_fill_or_check.checks[3] {
+                    event_to_fill_or_check.found &= expected.data == log.data;
+                }
             }
 
-            // Maybe match data
-            if next_expect.checks[3] {
-                next_expect.found &= expected.data == log.data;
+            // If we found the event, we can push it to the back of the queue
+            // and begin expecting the next event.
+            if event_to_fill_or_check.found {
+                state.expected_emits.push_back(event_to_fill_or_check);
+            } else {
+                // We did not match this event, so we need to keep waiting for the right one to
+                // appear.
+                state.expected_emits.push_front(event_to_fill_or_check);
             }
         }
+        // Fill the event.
+        None => {
+            event_to_fill_or_check.log = Some(log);
+            state.expected_emits.push_back(event_to_fill_or_check);
+        }
     }
 }
 
@@ -234,33 +267,33 @@ pub fn apply<DB: DatabaseExt>(
             expect_revert(state, Some(inner.0.into()), data.journaled_state.depth())
         }
         HEVMCalls::ExpectEmit0(_) => {
-            state.expected_emits.push(ExpectedEmit {
-                depth: data.journaled_state.depth() - 1,
+            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(ExpectedEmit {
-                depth: data.journaled_state.depth() - 1,
+            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(ExpectedEmit {
-                depth: data.journaled_state.depth() - 1,
+            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(ExpectedEmit {
-                depth: data.journaled_state.depth() - 1,
+            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()

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

@@ -29,7 +29,7 @@ use revm::{
 };
 use serde_json::Value;
 use std::{
-    collections::{BTreeMap, HashMap},
+    collections::{BTreeMap, HashMap, VecDeque},
     fs::File,
     io::BufReader,
     ops::Range,
@@ -129,7 +129,7 @@ pub struct Cheatcodes {
     pub expected_calls: BTreeMap<Address, Vec<(ExpectedCallData, u64)>>,
 
     /// Expected emits
-    pub expected_emits: Vec<ExpectedEmit>,
+    pub expected_emits: VecDeque<ExpectedEmit>,
 
     /// Map of context depths to memory offset ranges that may be written to within the call depth.
     pub allowed_mem_writes: BTreeMap<u64, Vec<Range<u64>>>,
@@ -527,7 +527,6 @@ where
         topics: &[B256],
         data: &bytes::Bytes,
     ) {
-        // Match logs if `expectEmit` has been called
         if !self.expected_emits.is_empty() {
             handle_expect_emit(
                 self,
@@ -753,21 +752,38 @@ where
             }
         }
 
-        // Handle expected emits at current depth
-        if !self
+        // At the end of the call,
+        // we need to check if we've found all the emits.
+        // We know we've found all the expected emits in the right order
+        // if the queue is fully matched.
+        // If it's not fully matched, then either:
+        // 1. Not enough events were emitted (we'll know this because the amount of times we
+        // inspected events will be less than the size of the queue) 2. The wrong events
+        // were emitted (The inspected events should match the size of the queue, but still some
+        // events will not be matched)
+
+        // First, check that we're at the call depth where the emits were declared from.
+        let should_check_emits = self
             .expected_emits
             .iter()
-            .filter(|expected| expected.depth == data.journaled_state.depth())
-            .all(|expected| expected.found)
-        {
-            return (
-                InstructionResult::Revert,
-                remaining_gas,
-                "Log != expected log".to_string().encode().into(),
-            )
-        } else {
-            // Clear the emits we expected at this depth that have been found
-            self.expected_emits.retain(|expected| !expected.found)
+            .any(|expected| expected.depth == data.journaled_state.depth()) &&
+            // Ignore staticcalls
+            !call.is_static;
+        // If so, check the emits
+        if should_check_emits {
+            // Not all emits were matched.
+            if self.expected_emits.iter().any(|expected| !expected.found) {
+                return (
+                    InstructionResult::Revert,
+                    remaining_gas,
+                    "Log != expected log".to_string().encode().into(),
+                )
+            } else {
+                // All emits were found, we're good.
+                // Clear the queue, as we expect the user to declare more events for the next call
+                // if they wanna match further events.
+                self.expected_emits.clear()
+            }
         }
 
         // If the depth is 0, then this is the root call terminating
@@ -814,11 +830,14 @@ where
             }
 
             // Check if we have any leftover expected emits
+            // First, if any emits were found at the root call, then we its ok and we remove them.
+            self.expected_emits.retain(|expected| !expected.found);
+            // If not empty, we got mismatched emits
             if !self.expected_emits.is_empty() {
                 return (
                     InstructionResult::Revert,
                     remaining_gas,
-                    "Expected an emit, but no logs were emitted afterward"
+                    "Expected an emit, but no logs were emitted afterward. You might have mismatched events or not enough events were emitted."
                         .to_string()
                         .encode()
                         .into(),