Merge pull request #357 from iamdefinitelyahuman/eip170

Eip170 checks, v1.6.3

CHANGELOG.md

@@ -6,11 +6,18 @@ This changelog format is based on [Keep a Changelog](https://keepachangelog.com/
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased](https://github.com/iamdefinitelyahuman/brownie)
+
+## [1.6.3](https://github.com/iamdefinitelyahuman/brownie/tree/v1.6.3) - 2020-02-09
 ### Added
 - `--stateful` flag to only run or skip stateful test cases
+- [EIP-170](https://github.com/ethereum/EIPs/issues/170) size limits: warn on compile, give useful error message on failed deployment
+
+### Changed
+- unexpanded transaction trace is available for deployment transactions
 
 ### Fixed
 - Warn instead of raising when an import spec cannot be found
+- Handle `REVERT` outside of function when generating revert map
 
 ## [1.6.2](https://github.com/iamdefinitelyahuman/brownie/tree/v1.6.2) - 2020-02-05
 ### Fixed

brownie/_cli/__main__.py

@@ -10,7 +10,7 @@
 from brownie.utils import color, notify
 from brownie.utils.docopt import docopt, levenshtein_norm
 
-__version__ = "1.6.2"
+__version__ = "1.6.3"
 
 __doc__ = """Usage:  brownie <command> [<args>...] [options <args>]
 

brownie/network/transaction.py

@@ -39,6 +39,19 @@ def wrapper(self: "TransactionReceipt") -> Any:
     return wrapper
 
 
+def trace_inspection(fn: Callable) -> Any:
+    def wrapper(self: "TransactionReceipt", *args: Any, **kwargs: Any) -> Any:
+        if self.contract_address:
+            raise NotImplementedError(
+                "Trace inspection methods are not available for deployment transactions."
+            )
+        if self.input == "0x" and self.gas_used == 21000:
+            return None
+        return fn(self, *args, **kwargs)
+
+    return wrapper
+
+
 class TransactionReceipt:
 
     """Attributes and methods relating to a broadcasted transaction.
@@ -175,14 +188,17 @@ def __init__(
                 if self._revert_msg is None:
                     # no revert message and unable to check dev string - have to get trace
                     self._expand_trace()
-                source = self._traceback_string() if ARGV["revert"] else self._error_string(1)
+                if self.contract_address:
+                    source = ""
+                else:
+                    source = self._traceback_string() if ARGV["revert"] else self._error_string(1)
                 raise VirtualMachineError({"message": self._revert_msg or "", "source": source})
         except KeyboardInterrupt:
             if ARGV["cli"] != "console":
                 raise
 
     def __repr__(self) -> str:
-        c = {-1: "pending", 0: "error", 1: None}
+        c = {-1: "bright yellow", 0: "bright red", 1: None}
         return f"<Transaction '{color(c[self.status])}{self.txid}{color}'>"
 
     def __hash__(self) -> int:
@@ -234,7 +250,11 @@ def return_value(self) -> Optional[str]:
 
     @trace_property
     def revert_msg(self) -> Optional[str]:
-        if not self.status and self._revert_msg is None:
+        if self.status:
+            return None
+        if self._revert_msg is None:
+            self._get_trace()
+        elif self.contract_address and self._revert_msg == "out of gas":
             self._get_trace()
         return self._revert_msg
 
@@ -331,8 +351,8 @@ def _get_trace(self) -> None:
         if self._raw_trace is not None:
             return
         self._raw_trace = []
-        if (self.input == "0x" and self.gas_used == 21000) or self.contract_address:
-            self._modified_state = bool(self.contract_address)
+        if self.input == "0x" and self.gas_used == 21000:
+            self._modified_state = False
             self._trace = []
             return
 
@@ -361,7 +381,7 @@ def _get_trace(self) -> None:
     def _confirmed_trace(self, trace: Sequence) -> None:
         self._modified_state = next((True for i in trace if i["op"] == "SSTORE"), False)
 
-        if trace[-1]["op"] != "RETURN":
+        if trace[-1]["op"] != "RETURN" or self.contract_address:
             return
         contract = _find_contract(self.receiver)
         if contract:
@@ -373,6 +393,10 @@ def _reverted_trace(self, trace: Sequence) -> None:
         self._modified_state = False
         # get events from trace
         self._events = _decode_trace(trace)
+        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:
             return
         # get revert message
@@ -382,6 +406,9 @@ def _reverted_trace(self, trace: Sequence) -> None:
             data = _get_memory(step, -1)[4:]
             self._revert_msg = decode_abi(["string"], data)[0]
             return
+        if self.contract_address:
+            self._revert_msg = "invalid opcode" if step["op"] == "INVALID" else ""
+            return
         # check for dev revert string using program counter
         self._revert_msg = build._get_dev_revert(step["pc"])
         if self._revert_msg is not None:
@@ -419,7 +446,7 @@ def _expand_trace(self) -> None:
         self._trace = trace = self._raw_trace
         self._new_contracts = []
         self._internal_transfers = []
-        if not trace:
+        if self.contract_address or not trace:
             coverage._add_transaction(self.coverage_hash, {})
             return
         if "fn" in trace[0]:
@@ -562,18 +589,15 @@ def info(self) -> None:
                     result += f"\n      {key}: {color('bright blue')}{value}{color}"
         print(result)
 
+    @trace_inspection
     def call_trace(self) -> None:
         """Displays the complete sequence of contracts and methods called during
         the transaction, and the range of trace step indexes for each method.
 
         Lines highlighed in red ended with a revert.
         """
-        trace = self.trace
-        if not trace:
-            if not self.contract_address:
-                return
-            raise NotImplementedError("Call trace is not available for deployment transactions.")
 
+        trace = self.trace
         result = f"Call trace for '{color('bright blue')}{self.txid}{color}':"
         result += _step_print(trace[0], trace[-1], None, 0, len(trace))
         indent = {0: 0}
@@ -613,17 +637,14 @@ def call_trace(self) -> None:
         print(result)
 
     def traceback(self) -> None:
-        print(self._traceback_string())
+        print(self._traceback_string() or "")
 
+    @trace_inspection
     def _traceback_string(self) -> str:
         """Returns an error traceback for the transaction."""
         if self.status == 1:
             return ""
         trace = self.trace
-        if not trace:
-            if not self.contract_address:
-                return ""
-            raise NotImplementedError("Traceback is not available for deployment transactions.")
 
         try:
             idx = next(i for i in range(len(trace)) if trace[i]["op"] in ("REVERT", "INVALID"))
@@ -651,8 +672,9 @@ def _traceback_string(self) -> str:
         )
 
     def error(self, pad: int = 3) -> None:
-        print(self._error_string(pad))
+        print(self._error_string(pad) or "")
 
+    @trace_inspection
     def _error_string(self, pad: int = 3) -> str:
         """Returns the source code that caused the transaction to revert.
 
@@ -682,8 +704,9 @@ def _error_string(self, pad: int = 3) -> str:
             return ""
 
     def source(self, idx: int, pad: int = 3) -> None:
-        print(self._source_string(idx, pad))
+        print(self._source_string(idx, pad) or "")
 
+    @trace_inspection
     def _source_string(self, idx: int, pad: int) -> str:
         """Displays the associated source code for a given stack trace step.
 
@@ -694,7 +717,7 @@ def _source_string(self, idx: int, pad: int) -> str:
         Returns: source code string
         """
         trace = self.trace[idx]
-        if not trace["source"]:
+        if not trace.get("source", None):
             return ""
         contract = _find_contract(self.trace[idx]["address"])
         source, linenos = highlight_source(

brownie/project/compiler/__init__.py

@@ -5,6 +5,7 @@
 from pathlib import Path
 from typing import Dict, Optional, Union
 
+from eth_utils import remove_0x_prefix
 from semantic_version import Version
 
 from brownie.exceptions import UnsupportedLanguage
@@ -15,6 +16,7 @@
     install_solc,
     set_solc_version,
 )
+from brownie.utils import notify
 
 from . import solidity, vyper
 
@@ -279,6 +281,12 @@ def generate_build_json(
                 "sourcePath": path_str,
             }
         )
+        size = len(remove_0x_prefix(output_evm["deployedBytecode"]["object"])) / 2  # type: ignore
+        if size > 24577:
+            notify(
+                "WARNING",
+                f"deployed size of {contract_name} is {size} bytes, exceeds EIP-170 limit of 24577",
+            )
 
     if not silent:
         print("")

docs/conf.py

@@ -37,7 +37,7 @@ def setup(sphinx):
 # The short X.Y version
 version = ""
 # The full version, including alpha/beta/rc tags
-release = "v1.6.2"
+release = "v1.6.3"
 
 
 # -- General configuration ---------------------------------------------------

setup.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 1.6.2
+current_version = 1.6.3
 
 [bumpversion:file:setup.py]
 

setup.py

@@ -11,7 +11,7 @@
 setup(
     name="eth-brownie",
     packages=find_packages(),
-    version="1.6.2",  # don't change this manually, use bumpversion instead
+    version="1.6.3",  # don't change this manually, use bumpversion instead
     license="MIT",
     description="A Python framework for Ethereum smart contract deployment, testing and interaction.",  # noqa: E501
     long_description=long_description,

tests/conftest.py

@@ -96,7 +96,16 @@ def pytest_sessionstart():
     monkeypatch_session = MonkeyPatch()
     monkeypatch_session.setattr(
         "solcx.get_available_solc_versions",
-        lambda: ["v0.6.2", "v0.5.15", "v0.5.8", "v0.5.7", "v0.4.25", "v0.4.24", "v0.4.22"],
+        lambda: [
+            "v0.6.2",
+            "v0.5.15",
+            "v0.5.8",
+            "v0.5.7",
+            "v0.5.0",
+            "v0.4.25",
+            "v0.4.24",
+            "v0.4.22",
+        ],
     )
 
 

tests/network/transaction/test_revert_msg.py

@@ -3,6 +3,7 @@
 import pytest
 
 from brownie.exceptions import VirtualMachineError
+from brownie.project import compile_source
 
 
 def test_revert_msg_via_jump(ext_tester, console_mode):
@@ -76,3 +77,9 @@ def test_vyper_revert_reasons(vypertester, console_mode):
     assert tx.revert_msg == "Modulo by zero"
     tx = vypertester.overflow(0, 0, {"value": 31337})
     assert tx.revert_msg == "Cannot send ether to nonpayable function"
+
+
+def test_deployment_size_limit(accounts, console_mode):
+    code = f"@public\ndef baz():\n    assert msg.sender != ZERO_ADDRESS, '{'blah'*10000}'"
+    tx = compile_source(code).Vyper.deploy({"from": accounts[0]})
+    assert tx.revert_msg == "exceeds EIP-170 size limit"

tests/network/transaction/test_trace.py

@@ -158,8 +158,9 @@ def test_call_trace(console_mode, tester):
 
 
 def test_trace_deploy(tester):
-    """trace is not calculated for deploying contracts"""
-    assert not tester.tx.trace
+    """trace is calculated for deploying contracts but not expanded"""
+    assert tester.tx.trace
+    assert "fn" not in tester.tx.trace[0]
 
 
 def test_trace_transfer(accounts):

tests/network/transaction/test_verbosity.py

@@ -50,9 +50,11 @@ def test_error(tx, reverted_tx, capfd):
 
 def test_deploy_reverts(BrownieTester, accounts, console_mode):
     tx = BrownieTester.deploy(True, {"from": accounts[0]}).tx
-    tx.traceback()
+    with pytest.raises(NotImplementedError):
+        tx.traceback()
     with pytest.raises(NotImplementedError):
         tx.call_trace()
+
     revertingtx = BrownieTester.deploy(False, {"from": accounts[0]})
     with pytest.raises(NotImplementedError):
         revertingtx.call_trace()

tests/project/compiler/test_solidity.py

@@ -222,3 +222,13 @@ def test_get_abi():
             "type": "function",
         }
     ]
+
+
+def test_size_limit(capfd):
+    code = f"""
+pragma solidity 0.6.2;
+contract Foo {{ function foo() external returns (bool) {{
+    require(msg.sender != address(0), "{"blah"*10000}"); }}
+}}"""
+    compiler.compile_and_format({"foo.sol": code})
+    assert "exceeds EIP-170 limit of 24577" in capfd.readouterr()[0]

tests/project/compiler/test_vyper.py

@@ -83,3 +83,9 @@ def test_get_abi():
             "gas": 351,
         }
     ]
+
+
+def test_size_limit(capfd):
+    code = f"@public\ndef baz():\n    assert msg.sender != ZERO_ADDRESS, '{'blah'*10000}'"
+    compiler.compile_and_format({"foo.vy": code})
+    assert "exceeds EIP-170 limit of 24577" in capfd.readouterr()[0]