Skip to content

Commit

Permalink
fix: cheatcode depth fixes (#1215)
Browse files Browse the repository at this point in the history
* fix: correct depth where we check expected emits

* fix: don't reset `tx.origin` too early

Closes #1210
  • Loading branch information
onbjerg authored Apr 7, 2022
1 parent 85907b3 commit 588b05b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 3 deletions.
2 changes: 1 addition & 1 deletion evm/src/executor/inspector/cheatcodes/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ pub fn apply<DB: Database>(
}
HEVMCalls::ExpectEmit(inner) => {
state.expected_emits.push(ExpectedEmit {
depth: data.subroutine.depth(),
depth: data.subroutine.depth() - 1,
checks: [inner.0, inner.1, inner.2, inner.3],
..Default::default()
});
Expand Down
8 changes: 6 additions & 2 deletions evm/src/executor/inspector/cheatcodes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ where

// Clean up pranks
if let Some(prank) = &self.prank {
data.env.tx.caller = prank.prank_origin;
if data.subroutine.depth() == prank.depth {
data.env.tx.caller = prank.prank_origin;
}
if prank.single_call {
std::mem::take(&mut self.prank);
}
Expand Down Expand Up @@ -322,7 +324,9 @@ where
) -> (Return, Option<Address>, Gas, Bytes) {
// Clean up pranks
if let Some(prank) = &self.prank {
data.env.tx.caller = prank.prank_origin;
if data.subroutine.depth() == prank.depth {
data.env.tx.caller = prank.prank_origin;
}
if prank.single_call {
std::mem::take(&mut self.prank);
}
Expand Down
21 changes: 21 additions & 0 deletions testdata/cheats/ExpectEmit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ contract Emitter {
inner.emitEvent(topic1, topic2, topic3, data);
}

/// Ref: issue #1214
function doesNothing() public pure {}

/// Ref: issue #760
function emitSomethingElse(uint256 data) public {
emit SomethingElse(data);
Expand Down Expand Up @@ -228,4 +231,22 @@ contract ExpectEmitTest is DSTest {
// amounts of indexed topics.
emitter.emitSomethingElse(1);
}

/// 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.
///
/// Expected emits should only be checked when the call from which the cheatcode
/// was invoked ends.
///
/// Ref: issue #1214
function testExpectEmitIsCheckedWhenCurrentCallTerminates() public {
cheats.expectEmit(true, true, true, true);
emitter.doesNothing();
emit Something(1, 2, 3, 4);

// This should fail since `SomethingElse` in the test
// and in the `Emitter` contract have differing
// amounts of indexed topics.
emitter.emitEvent(1, 2, 3, 4);
}
}
19 changes: 19 additions & 0 deletions testdata/cheats/Prank.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,23 @@ contract PrankTest is DSTest {

pranker.completePrank(victim);
}

/// Checks that `tx.origin` is set for all subcalls of a `prank`.
///
/// Ref: issue #1210
function testTxOriginInNestedPrank(address sender, address origin) public {
address oldSender = msg.sender;
address oldOrigin = tx.origin;

Victim innerVictim = new Victim();
NestedVictim victim = new NestedVictim(innerVictim);

cheats.prank(sender, origin);
victim.assertCallerAndOrigin(
sender,
"msg.sender was not set correctly",
origin,
"tx.origin was not set correctly"
);
}
}

0 comments on commit 588b05b

Please sign in to comment.