Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/anvil/src/eth/backend/mem/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ where
});
}

#[allow(clippy::redundant_clone)]
fn log(&mut self, ecx: &mut CTX, log: Log) {
call_inspectors!([&mut self.tracer, &mut self.log_collector], |inspector| {
inspector.log(ecx, log.clone());
});
}

#[allow(clippy::redundant_clone)]
fn log_full(&mut self, interp: &mut Interpreter, ecx: &mut CTX, log: Log) {
call_inspectors!([&mut self.tracer, &mut self.log_collector], |inspector| {
Expand Down
35 changes: 27 additions & 8 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1197,19 +1197,27 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for Cheatcodes {
}
}

fn log(&mut self, _ecx: Ecx, log: Log) {
if !self.expected_emits.is_empty()
&& let Some(err) = expect::handle_expect_emit(self, &log, None)
{
// Because we do not have access to the interpreter here, we cannot fail the test
// immediately. In most cases the failure will still be caught on `call_end`.
// In the rare case it is not, we log the error here.
let _ = sh_err!("{err:?}");
}

// `recordLogs`
record_logs(&mut self.recorded_logs, &log);
}

fn log_full(&mut self, interpreter: &mut Interpreter, _ecx: Ecx, log: Log) {
if !self.expected_emits.is_empty() {
expect::handle_expect_emit(self, &log, interpreter);
expect::handle_expect_emit(self, &log, Some(interpreter));
}

// `recordLogs`
if let Some(storage_recorded_logs) = &mut self.recorded_logs {
storage_recorded_logs.push(Vm::Log {
topics: log.data.topics().to_vec(),
data: log.data.data.clone(),
emitter: log.address,
});
}
record_logs(&mut self.recorded_logs, &log);
}

fn call(&mut self, ecx: Ecx, inputs: &mut CallInputs) -> Option<CallOutcome> {
Expand Down Expand Up @@ -2436,6 +2444,17 @@ fn access_is_call(kind: crate::Vm::AccountAccessKind) -> bool {
)
}

/// Records a log into the recorded logs vector, if it exists.
fn record_logs(recorded_logs: &mut Option<Vec<Vm::Log>>, log: &Log) {
if let Some(storage_recorded_logs) = recorded_logs {
storage_recorded_logs.push(Vm::Log {
topics: log.data.topics().to_vec(),
data: log.data.data.clone(),
emitter: log.address,
});
}
}

/// Appends an AccountAccess that resumes the recording of the current context.
fn append_storage_access(
last: &mut Vec<AccountAccess>,
Expand Down
42 changes: 28 additions & 14 deletions crates/cheatcodes/src/test/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,8 +793,12 @@ fn expect_emit(
pub(crate) fn handle_expect_emit(
state: &mut Cheatcodes,
log: &alloy_primitives::Log,
interpreter: &mut Interpreter,
) {
mut interpreter: Option<&mut Interpreter>,
) -> Option<&'static str> {
// This function returns an optional string indicating a failure reason.
// If the string is `Some`, it indicates that the expectation failed with the provided reason.
let mut should_fail = None;

// 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.
Expand All @@ -806,7 +810,7 @@ pub(crate) fn handle_expect_emit(
// 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;
return should_fail;
}

// Check count=0 expectations against this log - fail immediately if violated
Expand All @@ -818,14 +822,19 @@ pub(crate) fn handle_expect_emit(
// Check revert address
&& (expected_emit.address.is_none() || expected_emit.address == Some(log.address))
{
// This event was emitted but we expected it NOT to be (count=0)
// Fail immediately
interpreter.bytecode.set_action(InterpreterAction::new_return(
InstructionResult::Revert,
Error::encode("log emitted 1 time, expected 0"),
interpreter.gas,
));
return;
if let Some(interpreter) = &mut interpreter {
// This event was emitted but we expected it NOT to be (count=0)
// Fail immediately
interpreter.bytecode.set_action(InterpreterAction::new_return(
InstructionResult::Revert,
Error::encode("log emitted but expected 0 times"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nit: log emitted but not expected or smth like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msg shouldnt change here just keep it the same

interpreter.gas,
));
} else {
should_fail = Some("log emitted but expected 0 times");
}

return should_fail;
}
}

Expand All @@ -850,7 +859,7 @@ pub(crate) fn handle_expect_emit(
if !should_fill_logs
&& state.expected_emits.iter().all(|(emit, _)| emit.found || emit.count == 0)
{
return;
return should_fail;
}

let (mut event_to_fill_or_check, mut count_map) = state
Expand All @@ -867,14 +876,17 @@ pub(crate) fn handle_expect_emit(
state
.expected_emits
.insert(index_to_fill_or_check, (event_to_fill_or_check, count_map));
} else {
} else if let Some(interpreter) = &mut interpreter {
interpreter.bytecode.set_action(InterpreterAction::new_return(
InstructionResult::Revert,
Error::encode("use vm.expectEmitAnonymous to match anonymous events"),
interpreter.gas,
));
} else {
should_fail = Some("use vm.expectEmitAnonymous to match anonymous events");
}
return;

return should_fail;
};

// Increment/set `count` for `log.address` and `log.data`
Expand Down Expand Up @@ -953,6 +965,8 @@ pub(crate) fn handle_expect_emit(
// appear.
state.expected_emits.push_front((event_to_fill_or_check, count_map));
}

should_fail
}

/// Handles expected emits specified by the `expectEmit` cheatcodes.
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/evm/src/inspectors/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use revm::{
Inspector,
context::ContextTr,
interpreter::{
CallInputs, CallOutcome, Gas, InstructionResult, Interpreter, InterpreterResult,
CallInputs, CallOutcome, Gas, InstructionResult, InterpreterResult,
interpreter::EthInterpreter,
},
};
Expand Down Expand Up @@ -50,7 +50,7 @@ impl<CTX> Inspector<CTX, EthInterpreter> for LogCollector
where
CTX: ContextTr,
{
fn log_full(&mut self, _interp: &mut Interpreter, _context: &mut CTX, log: Log) {
fn log(&mut self, _context: &mut CTX, log: Log) {
self.logs.push(log);
}

Expand Down
12 changes: 12 additions & 0 deletions crates/evm/evm/src/inspectors/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,14 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStackRefMut<'_>
self.step_end_inlined(interpreter, ecx);
}

#[allow(clippy::redundant_clone)]
fn log(&mut self, ecx: &mut EthEvmContext<&mut dyn DatabaseExt>, log: Log) {
call_inspectors!(
[&mut self.tracer, &mut self.log_collector, &mut self.cheatcodes, &mut self.printer],
|inspector| inspector.log(ecx, log.clone()),
);
}

#[allow(clippy::redundant_clone)]
fn log_full(
&mut self,
Expand Down Expand Up @@ -1195,6 +1203,10 @@ impl Inspector<EthEvmContext<&mut dyn DatabaseExt>> for InspectorStack {
self.as_mut().initialize_interp(interpreter, ecx)
}

fn log(&mut self, ecx: &mut EthEvmContext<&mut dyn DatabaseExt>, log: Log) {
self.as_mut().log(ecx, log)
}

fn log_full(
&mut self,
interpreter: &mut Interpreter,
Expand Down
4 changes: 2 additions & 2 deletions crates/forge/tests/cli/failure_assertions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ Suite result: FAILED. 0 passed; 15 failed; 0 skipped; [ELAPSED]
[FAIL: log != expected log] testShouldFailCountEmitsFromAddress() ([GAS])
[FAIL: log != expected log] testShouldFailCountLessEmits() ([GAS])
[FAIL: log != expected Something] testShouldFailEmitSomethingElse() ([GAS])
[FAIL: log emitted 1 time, expected 0] testShouldFailNoEmit() ([GAS])
[FAIL: log emitted 1 time, expected 0] testShouldFailNoEmitFromAddress() ([GAS])
[FAIL: log emitted but expected 0 times] testShouldFailNoEmit() ([GAS])
[FAIL: log emitted but expected 0 times] testShouldFailNoEmitFromAddress() ([GAS])
Suite result: FAILED. 0 passed; 5 failed; 0 skipped; [ELAPSED]
...
"#,
Expand Down
Loading