Skip to content

Commit

Permalink
Merge pull request #861 from eth-brownie/feat-dev-revert-context
Browse files Browse the repository at this point in the history
Target dev revert reason in `brownie.reverts`
  • Loading branch information
iamdefinitelyahuman authored Nov 23, 2020
2 parents 1485e7a + a26dbef commit 3b555e8
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 40 deletions.
8 changes: 5 additions & 3 deletions brownie/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,16 @@ def __init__(self, exc: ValueError) -> None:
raise ValueError(exc["message"]) from None

self.txid: str = txid
self.source: str = ""
self.revert_type: str = data["error"]
self.revert_msg: Optional[str] = data.get("reason")
self.pc: Optional[str] = data.get("program_counter")
self.source: str = ""
if self.revert_type == "revert":
self.pc -= 1

self.revert_msg: Optional[str] = data.get("reason")
self.dev_revert_msg = brownie.project.build._get_dev_revert(self.pc)
if self.revert_msg is None and self.revert_type in ("revert", "invalid opcode"):
self.revert_msg = brownie.project.build._get_dev_revert(self.pc)
self.revert_msg = self.dev_revert_msg
else:
raise ValueError(str(exc)) from None

Expand Down
52 changes: 30 additions & 22 deletions brownie/network/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def dev_revert_msg(self) -> Optional[str]:
if self._dev_revert_msg is None:
self._get_trace()

return self._dev_revert_msg
return self._dev_revert_msg or None

@trace_property
def subcalls(self) -> Optional[List]:
Expand Down Expand Up @@ -355,7 +355,7 @@ def _raise_if_reverted(self, exc: Any) -> None:
# if traces are not available, do not attempt to determine the revert reason
raise exc or ValueError("Execution reverted")

if self._revert_msg is None:
if self._dev_revert_msg is None:
# no revert message and unable to check dev string - have to get trace
self._expand_trace()
if self.contract_address:
Expand All @@ -364,7 +364,9 @@ def _raise_if_reverted(self, exc: Any) -> None:
source = self._traceback_string()
else:
source = self._error_string(1)
raise exc._with_attr(source=source, revert_msg=self._revert_msg)
raise exc._with_attr(
source=source, revert_msg=self._revert_msg, dev_revert_msg=self._dev_revert_msg
)

def _await_transaction(self, required_confs: int, is_blocking: bool) -> None:
# await tx showing in mempool
Expand Down Expand Up @@ -592,15 +594,17 @@ def _confirmed_trace(self, trace: Sequence) -> None:
fn = contract.get_method_object(self.input)
self._return_value = fn.decode_output(data)

def _reverted_trace(self, trace: Sequence) -> None:
def _reverted_trace(self, trace: Sequence,) -> None:
self._modified_state = False
# get events from trace
self._events = _decode_trace(trace, str(self.receiver or self.contract_address))
if self.contract_address:
step = next((i for i in trace if i["op"] == "CODECOPY"), None)
if step is not None and int(step["stack"][-3], 16) > 24577:
self._revert_msg = "exceeds EIP-170 size limit"
if self._revert_msg is not None:
self._dev_revert_msg = ""

if self._dev_revert_msg is not None:
return

# iterate over revert instructions in reverse to find revert message
Expand All @@ -612,32 +616,36 @@ def _reverted_trace(self, trace: Sequence) -> None:

elif self.contract_address:
self._revert_msg = "invalid opcode" if step["op"] == "INVALID" else ""
self._dev_revert_msg = ""
return

# check for dev revert string using program counter
dev_revert = build._get_dev_revert(step["pc"])
dev_revert = build._get_dev_revert(step["pc"]) or None
if dev_revert is not None:
self._dev_revert_msg = dev_revert
if self._revert_msg is None:
self._revert_msg = dev_revert
if self._revert_msg is not None:
return
else:
# if none is found, expand the trace and get it from the pcMap
self._expand_trace()
try:
pc_map = state._find_contract(step["address"])._build["pcMap"]
# if this is the function selector revert, check for a jump
if "first_revert" in pc_map[step["pc"]]:
i = trace.index(step) - 4
if trace[i]["pc"] != step["pc"] - 4:
step = trace[i]
self._dev_revert_msg = pc_map[step["pc"]]["dev"]
if self._revert_msg is None:
self._revert_msg = self._dev_revert_msg
return
except (KeyError, AttributeError, TypeError):
pass

# if none is found, expand the trace and get it from the pcMap
self._expand_trace()
try:
pc_map = state._find_contract(step["address"])._build["pcMap"]
# if this is the function selector revert, check for a jump
if "first_revert" in pc_map[step["pc"]]:
i = trace.index(step) - 4
if trace[i]["pc"] != step["pc"] - 4:
step = trace[i]
self._dev_revert_msg = pc_map[step["pc"]]["dev"]
if self._revert_msg is None:
self._revert_msg = self._dev_revert_msg
if self._revert_msg is not None:
if self._dev_revert_msg is None:
self._dev_revert_msg = ""
return
except (KeyError, AttributeError, TypeError):
pass

step = next(i for i in trace[::-1] if i["op"] in ("REVERT", "INVALID"))
self._revert_msg = "invalid opcode" if step["op"] == "INVALID" else ""
Expand Down
23 changes: 13 additions & 10 deletions brownie/test/managers/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ def revert_deprecation(revert_msg=None):


class RevertContextManager:
def __init__(self, revert_msg=None):
def __init__(self, revert_msg=None, dev_revert_msg=None):
self.revert_msg = revert_msg
self.dev_revert_msg = dev_revert_msg
self.always_transact = CONFIG.argv["always_transact"]

if revert_msg is not None and revert_msg.startswith("dev:"):
# run calls as transactins when catching a dev revert string
if revert_msg is not None and (revert_msg.startswith("dev:") or dev_revert_msg):
# run calls as transactinos when catching a dev revert string
CONFIG.argv["always_transact"] = True

def __enter__(self):
Expand All @@ -67,15 +68,17 @@ def __exit__(self, exc_type, exc_value, traceback):
if exc_type is not VirtualMachineError:
raise

if self.revert_msg is None or self.revert_msg == exc_value.revert_msg:
return True
if self.dev_revert_msg is not None and self.dev_revert_msg != exc_value.dev_revert_msg:
raise AssertionError(
f"Unexpected dev revert string '{exc_value.dev_revert_msg}'\n{exc_value.source}"
) from None

if exc_value.revert_msg is None:
raise AssertionError("Transaction reverted, but no revert string was given") from None
if self.revert_msg is not None and self.revert_msg != exc_value.revert_msg:
raise AssertionError(
f"Unexpected revert string '{exc_value.revert_msg}'\n{exc_value.source}"
) from None

raise AssertionError(
f"Unexpected revert string '{exc_value.revert_msg}'\n{exc_value.source}"
) from None
return True


class PytestPrinter:
Expand Down
5 changes: 3 additions & 2 deletions docs/api-test.rst
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,12 @@ RevertContextManager

The ``RevertContextManager`` closely mimics the behaviour of :func:`pytest.raises <pytest.raises>`.

.. py:class:: brownie.test.plugin.RevertContextManager(revert_msg=None)
.. py:class:: brownie.test.plugin.RevertContextManager(revert_msg=None, dev_revert_msg=None)
Context manager used to handle :func:`VirtualMachineError <brownie.exceptions.VirtualMachineError>` exceptions. Raises ``AssertionError`` if no transaction has reverted when the context closes.

* ``revert_msg``: Optional. Raises an ``AssertionError`` if the transaction does not revert with this error string.
* ``revert_msg``: Optional. Raises if the transaction does not revert with this error string.
* ``dev_revert_msg``: Optional. Raises if the transaction does not revert with this :ref:`dev revert string<dev-revert>`.

This class is available as ``brownie.reverts`` when ``pytest`` is active.

Expand Down
41 changes: 39 additions & 2 deletions tests/network/transaction/test_revert_msg.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

@pytest.mark.parametrize("expr", ["revert()", "require(false)"])
@pytest.mark.parametrize("func", REVERT_FUNCTIONS_NO_INPUT)
def test_final_stmt_revert_no_input(console_mode, evmtester, accounts, expr, func):
def test_final_stmt_revert_no_input_no_msg(console_mode, evmtester, accounts, expr, func):

func = func.format(expr)
code = f"""
Expand All @@ -53,11 +53,30 @@ def test_final_stmt_revert_no_input(console_mode, evmtester, accounts, expr, fun
contract = compile_source(code).Foo.deploy({"from": accounts[0]})
tx = contract.foo()
assert tx.revert_msg == "dev: yuss"
assert tx.dev_revert_msg == "dev: yuss"


@pytest.mark.parametrize("expr", ["revert('foo')", "require(false, 'foo')"])
@pytest.mark.parametrize("func", REVERT_FUNCTIONS_NO_INPUT)
def test_final_stmt_revert_no_input(console_mode, evmtester, accounts, expr, func):

func = func.format(expr)
code = f"""
pragma solidity >=0.4.22;
contract Foo {{
{func}
}}
"""

contract = compile_source(code).Foo.deploy({"from": accounts[0]})
tx = contract.foo()
assert tx.revert_msg == "foo"
assert tx.dev_revert_msg == "dev: yuss"


@pytest.mark.parametrize("expr", ["revert()", "require(false)"])
@pytest.mark.parametrize("func", REVERT_FUNCTIONS_INPUT)
def test_final_stmt_revert_input(console_mode, evmtester, accounts, expr, func):
def test_final_stmt_revert_input_no_msg(console_mode, evmtester, accounts, expr, func):

func = func.format(expr)
code = f"""
Expand All @@ -72,6 +91,24 @@ def test_final_stmt_revert_input(console_mode, evmtester, accounts, expr, func):
assert tx.revert_msg == "dev: yuss"


@pytest.mark.parametrize("expr", ["revert('foo')", "require(false, 'foo')"])
@pytest.mark.parametrize("func", REVERT_FUNCTIONS_INPUT)
def test_final_stmt_revert_input(console_mode, evmtester, accounts, expr, func):

func = func.format(expr)
code = f"""
pragma solidity >=0.4.22;
contract Foo {{
{func}
}}
"""

contract = compile_source(code).Foo.deploy({"from": accounts[0]})
tx = contract.foo(4)
assert tx.revert_msg == "foo"
assert tx.dev_revert_msg == "dev: yuss"


def test_revert_msg_via_jump(ext_tester, console_mode):
tx = ext_tester.getCalled(2)
assert tx.revert_msg == "dev: should jump to a revert"
Expand Down
2 changes: 1 addition & 1 deletion tests/test/plugin/test_reverts.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def tester(BrownieTester, accounts):
def test_reverts_success(tester):
with brownie.reverts("zero"):
tester.revertStrings(0)
with brownie.reverts("dev: one"):
with brownie.reverts(dev_revert_msg="dev: one"):
tester.revertStrings(1)
@pytest.mark.xfail(condition=True, reason="", raises=AssertionError, strict=True)
Expand Down

0 comments on commit 3b555e8

Please sign in to comment.