Skip to content

Commit

Permalink
Fail test if no function called after expectRevert (gakonst#274)
Browse files Browse the repository at this point in the history
* first attempt, pushing for review

* cleanup

* prefer is_none of !is_some`

* pushing latest, re feedback from gakonst, test not passing

* use as_deref

* put expected_revert in check_success

* clean up, comment

* chore: fmt

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
  • Loading branch information
wilsoncusack and gakonst authored Dec 20, 2021
1 parent 103f6cf commit 90522a4
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 0 deletions.
4 changes: 4 additions & 0 deletions evm-adapters/src/evmodin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ impl<S: HostExt, Tr: Tracer> Evm<S> for EvmOdin<S, Tr> {
StatusCode::Revert
}

fn expected_revert(&self) -> Option<&[u8]> {
None
}

fn is_success(reason: &Self::ReturnReason) -> bool {
matches!(reason, StatusCode::Success)
}
Expand Down
6 changes: 6 additions & 0 deletions evm-adapters/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ pub trait Evm<State> {
/// Gets the revert reason type
fn revert() -> Self::ReturnReason;

fn expected_revert(&self) -> Option<&[u8]>;

/// Whether a return reason should be considered successful
fn is_success(reason: &Self::ReturnReason) -> bool;
/// Whether a return reason should be considered failing
Expand Down Expand Up @@ -164,6 +166,10 @@ pub trait Evm<State> {
success = !failed;
}
}
// check if there is a remaining expected revert
if self.expected_revert().is_some() {
success = false;
}

// Check Success output: Should Fail vs Success
//
Expand Down
4 changes: 4 additions & 0 deletions evm-adapters/src/sputnik/cheatcodes/cheatcode_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ impl<'a, 'b, B: Backend, P: PrecompileSet> SputnikExecutor<CheatcodeStackState<'
self.handler.state_mut()
}

fn expected_revert(&self) -> Option<&[u8]> {
self.handler.state().expected_revert.as_deref()
}

fn gas_left(&self) -> U256 {
// NB: We do this to avoid `function cannot return without recursing`
U256::from(self.state().metadata().gasometer().gas())
Expand Down
4 changes: 4 additions & 0 deletions evm-adapters/src/sputnik/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ where
ExitReason::Revert(ExitRevert::Reverted)
}

fn expected_revert(&self) -> Option<&[u8]> {
self.executor.expected_revert()
}

fn is_success(reason: &Self::ReturnReason) -> bool {
matches!(reason, ExitReason::Succeed(_))
}
Expand Down
5 changes: 5 additions & 0 deletions evm-adapters/src/sputnik/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub trait SputnikExecutor<S> {
fn config(&self) -> &Config;
fn state(&self) -> &S;
fn state_mut(&mut self) -> &mut S;
fn expected_revert(&self) -> Option<&[u8]>;
fn gas_left(&self) -> U256;
fn transact_call(
&mut self,
Expand Down Expand Up @@ -106,6 +107,10 @@ impl<'a, 'b, S: StackState<'a>, P: PrecompileSet> SputnikExecutor<S>
self.state_mut()
}

fn expected_revert(&self) -> Option<&[u8]> {
None
}

fn gas_left(&self) -> U256 {
// NB: We do this to avoid `function cannot return without recursing`
U256::from(self.state().metadata().gasometer().gas())
Expand Down
6 changes: 6 additions & 0 deletions evm-adapters/testdata/CheatCodes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,12 @@ contract CheatCodes is DSTest {
target.stringErr(99);
}

// Test should fail if nothing is called
// after expectRevert
function testFailExpectRevert3() public {
hevm.expectRevert("revert");
}

function getCode(address who) internal returns (bytes memory o_code) {
assembly {
// retrieve the size of the code, this needs assembly
Expand Down

0 comments on commit 90522a4

Please sign in to comment.