From e7b7f9af8ade4d032e89d7a65aae588e3598c07c Mon Sep 17 00:00:00 2001 From: spencer-tb Date: Tue, 14 Mar 2023 15:29:41 -0600 Subject: [PATCH 01/29] Initial integration of evmone-t8n. Update default evm to create txs json file from rlp. For evmone-t8n, add sender to all txs, make sure all inputs are hex. Start on generating v, r & s. Update evm integration after new updates to evmone-t8n. evmone-t8n works with b11r Test evm vs evmone output. t8n: evm one improvements pytest: test-filler plugin: detect t8n tool --- src/evm_transition_tool/__init__.py | 145 +++++++++++++++++- src/pytest_plugins/forks/forks.py | 10 +- src/pytest_plugins/test_filler/test_filler.py | 25 ++- whitelist.txt | 1 + 4 files changed, 171 insertions(+), 10 deletions(-) diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index 0fd9cf5bec..8d76f6c7b2 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -140,6 +140,13 @@ def calc_withdrawals_root(self, withdrawals: Any, fork: Fork) -> bytes: ) return bytes.fromhex(withdrawals_root[2:]) + def write_json_file(self, data: Dict[str, Any], file_path: str) -> None: + """ + Write a JSON file to the given path. + """ + with open(file_path, "w") as f: + json.dump(data, f, ensure_ascii=False, indent=4) + class EvmTransitionTool(TransitionTool): """ @@ -155,8 +162,10 @@ def __init__( binary: Optional[Path | str] = None, trace: bool = False, ): - if binary is None: - which_path = which("evm") + if binary is None or type(binary) is str: + if binary is None: + binary = "evm" + which_path = which(binary) if which_path is not None: binary = Path(which_path) if binary is None or not Path(binary).exists(): @@ -276,3 +285,135 @@ def is_fork_supported(self, fork: Fork) -> bool: Returns True if the fork is supported by the tool """ return fork().name() in self.help_string + + +class EvmOneTransitionTool(TransitionTool): + """ + Evmone `evmone-t8n` Transition tool frontend. + """ + + binary: Path + cached_version: Optional[str] = None + trace: bool + + def __init__( + self, + binary: Optional[Path | str] = None, + trace: bool = False, + ): + if binary is None or type(binary) is str: + if binary is None: + binary = "evm" + which_path = which(binary) + if which_path is not None: + binary = Path(which_path) + if binary is None or not Path(binary).exists(): + raise Exception("""`evmone-t8n` binary executable is not accessible""") + self.binary = Path(binary) + self.trace = trace + + def evaluate( + self, + alloc: Any, + txs: Any, + env: Any, + fork: Fork, + chain_id: int = 1, + reward: int = 0, + eips: Optional[List[int]] = None, + ) -> Tuple[Dict[str, Any], Dict[str, Any]]: + """ + Executes `evmone-t8n` with the specified arguments. + """ + fork_name = fork.name() + if eips is not None: + fork_name = "+".join([fork_name] + [str(eip) for eip in eips]) + + temp_dir = tempfile.TemporaryDirectory() + + input_contents = { + "alloc": alloc, + "env": env, + "txs": txs, + } + input_paths = { + k: os.path.join(temp_dir.name, f"input_{k}.json") for k in input_contents.keys() + } + for key, val in input_contents.items(): + file_path = os.path.join(temp_dir.name, f"input_{key}.json") + self.write_json_file(val, file_path) + + # Construct args for evmone-t8n binary + args = [ + str(self.binary), + "--state.fork", + fork_name, + "--input.alloc", + input_paths["alloc"], + "--input.env", + input_paths["env"], + "--input.txs", + input_paths["txs"], + "--output.basedir", + temp_dir.name, + "--output.result", + "output_result.json", + "--output.alloc", + "output_alloc.json", + "--output.body", + "txs.rlp", + "--state.reward", + str(reward), + "--state.chainid", + str(chain_id), + ] + result = subprocess.run( + args, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + + if result.returncode != 0: + raise Exception("failed to evaluate: " + result.stderr.decode()) + + output_paths = { + "alloc": os.path.join(temp_dir.name, "output_alloc.json"), + "result": os.path.join(temp_dir.name, "output_result.json"), + } + + output_contents = {} + for key, file_path in output_paths.items(): + with open(file_path, "r+") as file: + contents = json.load(file) + file.seek(0) + json.dump(contents, file, ensure_ascii=False, indent=4) + file.truncate() + output_contents[key] = contents + + temp_dir.cleanup() + + return output_contents["alloc"], output_contents["result"] + + def version(self) -> str: + """ + Gets `evmone-t8n` binary version. + """ + if self.cached_version is None: + result = subprocess.run( + [str(self.binary), "-v"], + stdout=subprocess.PIPE, + ) + + if result.returncode != 0: + raise Exception("failed to evaluate: " + result.stderr.decode()) + + self.cached_version = result.stdout.decode().strip() + + return self.cached_version + + def is_fork_supported(self, fork: Fork) -> bool: + """ + Returns True if the fork is supported by the tool. + Currently, evmone-t8n provides no way to determine supported forks. + """ + return True diff --git a/src/pytest_plugins/forks/forks.py b/src/pytest_plugins/forks/forks.py index c09b148fbb..9397ef4924 100644 --- a/src/pytest_plugins/forks/forks.py +++ b/src/pytest_plugins/forks/forks.py @@ -14,7 +14,7 @@ get_transition_forks, transition_fork_to, ) -from evm_transition_tool import EvmTransitionTool +from evm_transition_tool import EvmTransitionTool, EvmOneTransitionTool def pytest_addoption(parser): @@ -157,7 +157,13 @@ def pytest_configure(config): # with --collect-only, we don't have access to these config options if config.option.collectonly: return - t8n = EvmTransitionTool( + + transitionToolCls = EvmTransitionTool + evm_bin = config.getoption("evm_bin") + if evm_bin: + if evm_bin == "evmone-t8n": + transitionToolCls = EvmOneTransitionTool + t8n = transitionToolCls( binary=config.getoption("evm_bin"), trace=config.getoption("evm_collect_traces"), ) diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index ec2bd0fcad..0b0f444f77 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -24,7 +24,7 @@ Yul, fill_test, ) -from evm_transition_tool import EvmTransitionTool +from evm_transition_tool import EvmOneTransitionTool, EvmTransitionTool from pytest_plugins.spec_version_checker.spec_version_checker import EIPSpecTestItem @@ -112,12 +112,26 @@ def pytest_configure(config): ) +def get_transition_tool_from_evm_bin(*, binary, collect_traces): + """ + Returns an EvmTransitionTool instance from the evm_bin path. + """ + transitionToolCls = EvmTransitionTool + if binary and binary == "evmone-t8n": + transitionToolCls = EvmOneTransitionTool + + return transitionToolCls( + binary=binary, + trace=collect_traces, + ) + + @pytest.hookimpl(trylast=True) def pytest_report_header(config, start_path): """Add lines to pytest's console output header""" - t8n = EvmTransitionTool( + t8n = get_transition_tool_from_evm_bin( binary=config.getoption("evm_bin"), - trace=config.getoption("evm_collect_traces"), + collect_traces=config.getoption("evm_collect_traces"), ) solc_version_string = Yul("", binary=config.getoption("solc_bin")).version() return [f"{t8n.version()}, solc version {solc_version_string}"] @@ -144,9 +158,8 @@ def t8n(request, evm_bin): """ Returns the configured transition tool. """ - t8n = EvmTransitionTool( - binary=evm_bin, - trace=request.config.getoption("evm_collect_traces"), + t8n = get_transition_tool_from_evm_bin( + binary=evm_bin, collect_traces=request.config.getoption("evm_collect_traces") ) return t8n diff --git a/whitelist.txt b/whitelist.txt index 504e853bb7..0d0b786089 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -20,6 +20,7 @@ eth ethash ethereum evm +evmone fn geth hash32 From f67e477e1a256f8585af96487a29b0d78eb91844 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Mon, 3 Jul 2023 17:28:52 +0000 Subject: [PATCH 02/29] t8n: Add static method to detect tool type --- src/evm_transition_tool/__init__.py | 32 +++++++++++++- src/pytest_plugins/forks/forks.py | 9 ++-- src/pytest_plugins/test_filler/test_filler.py | 42 ++++++++++--------- 3 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index 8d76f6c7b2..dd952ae4ba 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -9,7 +9,7 @@ from abc import abstractmethod from pathlib import Path from shutil import which -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple, Type from ethereum_test_forks import Fork @@ -21,6 +21,32 @@ class TransitionTool: traces: List[List[List[Dict]]] | None = None + # Static methods + + @staticmethod + def from_binary_path(*, binary_path: Optional[str]) -> Type["TransitionTool"]: + """ + Returns the appropriate TransitionTool subclass derived from the binary path. + """ + if binary_path and "evmone-t8n" in binary_path: + return EvmOneTransitionTool + + return EvmTransitionTool + + # Abstract methods that each tool must implement + + @abstractmethod + def __init__( + self, + *, + binary: Optional[Path | str] = None, + trace: bool = False, + ): + """ + Abstract initialization method that all subclasses must implement. + """ + pass + @abstractmethod def evaluate( self, @@ -159,6 +185,7 @@ class EvmTransitionTool(TransitionTool): def __init__( self, + *, binary: Optional[Path | str] = None, trace: bool = False, ): @@ -298,12 +325,13 @@ class EvmOneTransitionTool(TransitionTool): def __init__( self, + *, binary: Optional[Path | str] = None, trace: bool = False, ): if binary is None or type(binary) is str: if binary is None: - binary = "evm" + binary = "evmone-t8n" which_path = which(binary) if which_path is not None: binary = Path(which_path) diff --git a/src/pytest_plugins/forks/forks.py b/src/pytest_plugins/forks/forks.py index 9397ef4924..1d53890bdf 100644 --- a/src/pytest_plugins/forks/forks.py +++ b/src/pytest_plugins/forks/forks.py @@ -14,7 +14,7 @@ get_transition_forks, transition_fork_to, ) -from evm_transition_tool import EvmTransitionTool, EvmOneTransitionTool +from evm_transition_tool import TransitionTool def pytest_addoption(parser): @@ -157,12 +157,9 @@ def pytest_configure(config): # with --collect-only, we don't have access to these config options if config.option.collectonly: return - - transitionToolCls = EvmTransitionTool + evm_bin = config.getoption("evm_bin") - if evm_bin: - if evm_bin == "evmone-t8n": - transitionToolCls = EvmOneTransitionTool + transitionToolCls = TransitionTool.from_binary_path(binary_path=evm_bin) t8n = transitionToolCls( binary=config.getoption("evm_bin"), trace=config.getoption("evm_collect_traces"), diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index 0b0f444f77..59ac1edf52 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -24,7 +24,7 @@ Yul, fill_test, ) -from evm_transition_tool import EvmOneTransitionTool, EvmTransitionTool +from evm_transition_tool import TransitionTool from pytest_plugins.spec_version_checker.spec_version_checker import EIPSpecTestItem @@ -112,27 +112,14 @@ def pytest_configure(config): ) -def get_transition_tool_from_evm_bin(*, binary, collect_traces): - """ - Returns an EvmTransitionTool instance from the evm_bin path. - """ - transitionToolCls = EvmTransitionTool - if binary and binary == "evmone-t8n": - transitionToolCls = EvmOneTransitionTool - - return transitionToolCls( - binary=binary, - trace=collect_traces, - ) - - @pytest.hookimpl(trylast=True) def pytest_report_header(config, start_path): """Add lines to pytest's console output header""" - t8n = get_transition_tool_from_evm_bin( - binary=config.getoption("evm_bin"), - collect_traces=config.getoption("evm_collect_traces"), + binary_path = config.getoption("evm_bin") + t8nCls = TransitionTool.from_binary_path( + binary_path=binary_path, ) + t8n = t8nCls(binary=binary_path) solc_version_string = Yul("", binary=config.getoption("solc_bin")).version() return [f"{t8n.version()}, solc version {solc_version_string}"] @@ -158,9 +145,10 @@ def t8n(request, evm_bin): """ Returns the configured transition tool. """ - t8n = get_transition_tool_from_evm_bin( - binary=evm_bin, collect_traces=request.config.getoption("evm_collect_traces") + t8nCls = TransitionTool.from_binary_path( + binary_path=evm_bin, ) + t8n = t8nCls(binary=evm_bin, trace=request.config.getoption("evm_collect_traces")) return t8n @@ -337,6 +325,13 @@ def state_test( class StateTestWrapper(StateTest): def __init__(self, *args, **kwargs): + if ( + "env" not in kwargs + or "pre" not in kwargs + or "post" not in kwargs + or "txs" not in kwargs + ): + raise Exception("Insufficient arguments for StateTestWrapper") super(StateTestWrapper, self).__init__(*args, **kwargs) fixture_collector.add_fixture( request.node, @@ -365,6 +360,13 @@ def blockchain_test( class BlockchainTestWrapper(BlockchainTest): def __init__(self, *args, **kwargs): + if ( + "genesis_environment" not in kwargs + or "pre" not in kwargs + or "post" not in kwargs + or "blocks" not in kwargs + ): + raise Exception("Insufficient arguments for BlockchainTestWrapper") super(BlockchainTestWrapper, self).__init__(*args, **kwargs) fixture_collector.add_fixture( request.node, From ccfb102d5e0bf459c068e4b32bcfd9bcd108545e Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Mon, 3 Jul 2023 17:35:25 +0000 Subject: [PATCH 03/29] t8n: nit method change --- src/evm_transition_tool/__init__.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index dd952ae4ba..960ffc5792 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -166,12 +166,13 @@ def calc_withdrawals_root(self, withdrawals: Any, fork: Fork) -> bytes: ) return bytes.fromhex(withdrawals_root[2:]) - def write_json_file(self, data: Dict[str, Any], file_path: str) -> None: - """ - Write a JSON file to the given path. - """ - with open(file_path, "w") as f: - json.dump(data, f, ensure_ascii=False, indent=4) + +def write_json_file(data: Dict[str, Any], file_path: str) -> None: + """ + Write a JSON file to the given path. + """ + with open(file_path, "w") as f: + json.dump(data, f, ensure_ascii=False, indent=4) class EvmTransitionTool(TransitionTool): @@ -369,7 +370,7 @@ def evaluate( } for key, val in input_contents.items(): file_path = os.path.join(temp_dir.name, f"input_{key}.json") - self.write_json_file(val, file_path) + write_json_file(val, file_path) # Construct args for evmone-t8n binary args = [ From 360233accecee9dbd8f4cc1513e26edc4d496727 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Mon, 3 Jul 2023 18:11:46 +0000 Subject: [PATCH 04/29] test_filler: rollback check for spec fixture parameters --- src/pytest_plugins/test_filler/test_filler.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index 59ac1edf52..7523416ae0 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -325,13 +325,6 @@ def state_test( class StateTestWrapper(StateTest): def __init__(self, *args, **kwargs): - if ( - "env" not in kwargs - or "pre" not in kwargs - or "post" not in kwargs - or "txs" not in kwargs - ): - raise Exception("Insufficient arguments for StateTestWrapper") super(StateTestWrapper, self).__init__(*args, **kwargs) fixture_collector.add_fixture( request.node, @@ -360,13 +353,6 @@ def blockchain_test( class BlockchainTestWrapper(BlockchainTest): def __init__(self, *args, **kwargs): - if ( - "genesis_environment" not in kwargs - or "pre" not in kwargs - or "post" not in kwargs - or "blocks" not in kwargs - ): - raise Exception("Insufficient arguments for BlockchainTestWrapper") super(BlockchainTestWrapper, self).__init__(*args, **kwargs) fixture_collector.add_fixture( request.node, From a38de3212d6bc72fdcb25d48e0610a83a3ece2ab Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 01:17:46 +0000 Subject: [PATCH 05/29] evm_transition_tool: refactor each tool to separate files --- src/ethereum_test_tools/tests/test_filler.py | 10 +- src/evm_transition_tool/__init__.py | 453 +----------------- src/evm_transition_tool/evmone.py | 162 +++++++ src/evm_transition_tool/geth.py | 162 +++++++ .../tests/test_evaluate.py | 8 +- src/evm_transition_tool/transition_tool.py | 198 ++++++++ src/pytest_plugins/test_filler/test_filler.py | 8 +- 7 files changed, 545 insertions(+), 456 deletions(-) create mode 100644 src/evm_transition_tool/evmone.py create mode 100644 src/evm_transition_tool/geth.py create mode 100644 src/evm_transition_tool/transition_tool.py diff --git a/src/ethereum_test_tools/tests/test_filler.py b/src/ethereum_test_tools/tests/test_filler.py index 49dd5f559a..d5343e3f83 100644 --- a/src/ethereum_test_tools/tests/test_filler.py +++ b/src/ethereum_test_tools/tests/test_filler.py @@ -9,7 +9,7 @@ import pytest from ethereum_test_forks import Berlin, Fork, Istanbul, London -from evm_transition_tool import EvmTransitionTool +from evm_transition_tool import GethTransitionTool from ..code import Yul from ..common import Account, Block, Environment, JSONEncoder, TestAddress, Transaction @@ -59,7 +59,7 @@ def test_make_genesis(fork: Fork, hash: bytes): TestAddress: Account(balance=0x0BA1A9CE0BA1A9CE), } - t8n = EvmTransitionTool() + t8n = GethTransitionTool() _, genesis = StateTest(env=env, pre=pre, post={}, txs=[], tag="some_state_test").make_genesis( t8n, @@ -111,7 +111,7 @@ def test_fill_state_test(fork: Fork, expected_json_file: str): state_test = StateTest(env=env, pre=pre, post=post, txs=[tx], tag="my_chain_id_test") - t8n = EvmTransitionTool() + t8n = GethTransitionTool() fixture = { f"000/my_chain_id_test/{fork}": fill_test( @@ -398,7 +398,7 @@ def test_fill_london_blockchain_test_valid_txs(fork: Fork): tag="fill_london_blockchain_test_valid_txs", ) - t8n = EvmTransitionTool() + t8n = GethTransitionTool() fixture = { f"000/my_blockchain_test/{fork.name()}": fill_test( @@ -733,7 +733,7 @@ def test_fill_london_blockchain_test_invalid_txs(fork): tag="fill_london_blockchain_test_invalid_txs", ) - t8n = EvmTransitionTool() + t8n = GethTransitionTool() fixture = { f"000/my_blockchain_test/{fork.name()}": fill_test( diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index 960ffc5792..d458b96731 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -1,448 +1,15 @@ """ -Python wrapper for the `evm t8n` tool. +Library of Python wrappers for the different implementations of transition tools. """ -import json -import os -import subprocess -import tempfile -from abc import abstractmethod -from pathlib import Path -from shutil import which -from typing import Any, Dict, List, Optional, Tuple, Type +from .evmone import EvmOneTransitionTool +from .geth import GethTransitionTool +from .transition_tool import TransitionTool -from ethereum_test_forks import Fork +TransitionTool.set_default_tool(GethTransitionTool) - -class TransitionTool: - """ - Transition tool frontend. - """ - - traces: List[List[List[Dict]]] | None = None - - # Static methods - - @staticmethod - def from_binary_path(*, binary_path: Optional[str]) -> Type["TransitionTool"]: - """ - Returns the appropriate TransitionTool subclass derived from the binary path. - """ - if binary_path and "evmone-t8n" in binary_path: - return EvmOneTransitionTool - - return EvmTransitionTool - - # Abstract methods that each tool must implement - - @abstractmethod - def __init__( - self, - *, - binary: Optional[Path | str] = None, - trace: bool = False, - ): - """ - Abstract initialization method that all subclasses must implement. - """ - pass - - @abstractmethod - def evaluate( - self, - alloc: Any, - txs: Any, - env: Any, - fork: Fork, - chain_id: int = 1, - reward: int = 0, - eips: Optional[List[int]] = None, - ) -> Tuple[Dict[str, Any], Dict[str, Any]]: - """ - Simulate a state transition with specified parameters - """ - pass - - @abstractmethod - def version(self) -> str: - """ - Return name and version of tool used to state transition - """ - pass - - @abstractmethod - def is_fork_supported(self, fork: Fork) -> bool: - """ - Returns True if the fork is supported by the tool - """ - pass - - def reset_traces(self): - """ - Resets the internal trace storage for a new test to begin - """ - self.traces = None - - def append_traces(self, new_traces: List[List[Dict]]): - """ - Appends a list of traces of a state transition to the current list - """ - if self.traces is None: - self.traces = [] - self.traces.append(new_traces) - - def get_traces(self) -> List[List[List[Dict]]] | None: - """ - Returns the accumulated traces - """ - return self.traces - - def calc_state_root(self, alloc: Any, fork: Fork) -> bytes: - """ - Calculate the state root for the given `alloc`. - """ - env: Dict[str, Any] = { - "currentCoinbase": "0x0000000000000000000000000000000000000000", - "currentDifficulty": "0x0", - "currentGasLimit": "0x0", - "currentNumber": "0", - "currentTimestamp": "0", - } - - if fork.header_base_fee_required(0, 0): - env["currentBaseFee"] = "7" - - if fork.header_prev_randao_required(0, 0): - env["currentRandom"] = "0" - - if fork.header_withdrawals_required(0, 0): - env["withdrawals"] = [] - - _, result = self.evaluate(alloc, [], env, fork) - state_root = result.get("stateRoot") - if state_root is None or not isinstance(state_root, str): - raise Exception("Unable to calculate state root") - return bytes.fromhex(state_root[2:]) - - def calc_withdrawals_root(self, withdrawals: Any, fork: Fork) -> bytes: - """ - Calculate the state root for the given `alloc`. - """ - if type(withdrawals) is list and len(withdrawals) == 0: - # Optimize returning the empty root immediately - return bytes.fromhex( - "56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421" - ) - - env: Dict[str, Any] = { - "currentCoinbase": "0x0000000000000000000000000000000000000000", - "currentDifficulty": "0x0", - "currentGasLimit": "0x0", - "currentNumber": "0", - "currentTimestamp": "0", - "withdrawals": withdrawals, - } - - if fork.header_base_fee_required(0, 0): - env["currentBaseFee"] = "7" - - if fork.header_prev_randao_required(0, 0): - env["currentRandom"] = "0" - - if fork.header_excess_data_gas_required(0, 0): - env["currentExcessDataGas"] = "0" - - _, result = self.evaluate({}, [], env, fork) - withdrawals_root = result.get("withdrawalsRoot") - if withdrawals_root is None: - raise Exception( - "Unable to calculate withdrawals root: no value returned from transition tool" - ) - if type(withdrawals_root) is not str: - raise Exception( - "Unable to calculate withdrawals root: " - + "incorrect type returned from transition tool: " - + f"{withdrawals_root}" - ) - return bytes.fromhex(withdrawals_root[2:]) - - -def write_json_file(data: Dict[str, Any], file_path: str) -> None: - """ - Write a JSON file to the given path. - """ - with open(file_path, "w") as f: - json.dump(data, f, ensure_ascii=False, indent=4) - - -class EvmTransitionTool(TransitionTool): - """ - Go-ethereum `evm` Transition tool frontend. - """ - - binary: Path - cached_version: Optional[str] = None - trace: bool - - def __init__( - self, - *, - binary: Optional[Path | str] = None, - trace: bool = False, - ): - if binary is None or type(binary) is str: - if binary is None: - binary = "evm" - which_path = which(binary) - if which_path is not None: - binary = Path(which_path) - if binary is None or not Path(binary).exists(): - raise Exception( - """`evm` binary executable is not accessible, please refer to - https://github.com/ethereum/go-ethereum on how to compile and - install the full suite of utilities including the `evm` tool""" - ) - self.binary = Path(binary) - self.trace = trace - args = [str(self.binary), "t8n", "--help"] - try: - result = subprocess.run(args, capture_output=True, text=True) - except subprocess.CalledProcessError as e: - raise Exception("evm process unexpectedly returned a non-zero status code: " f"{e}.") - except Exception as e: - raise Exception(f"Unexpected exception calling evm tool: {e}.") - self.help_string = result.stdout - - def evaluate( - self, - alloc: Any, - txs: Any, - env: Any, - fork: Fork, - chain_id: int = 1, - reward: int = 0, - eips: Optional[List[int]] = None, - ) -> Tuple[Dict[str, Any], Dict[str, Any]]: - """ - Executes `evm t8n` with the specified arguments. - """ - fork_name = fork.name() - if eips is not None: - fork_name = "+".join([fork_name] + [str(eip) for eip in eips]) - - temp_dir = tempfile.TemporaryDirectory() - - if int(env["currentNumber"], 0) == 0: - reward = -1 - args = [ - str(self.binary), - "t8n", - "--input.alloc=stdin", - "--input.txs=stdin", - "--input.env=stdin", - "--output.result=stdout", - "--output.alloc=stdout", - "--output.body=txs.rlp", - f"--output.basedir={temp_dir.name}", - f"--state.fork={fork_name}", - f"--state.chainid={chain_id}", - f"--state.reward={reward}", - ] - - if self.trace: - args.append("--trace") - - stdin = { - "alloc": alloc, - "txs": txs, - "env": env, - } - - encoded_input = str.encode(json.dumps(stdin)) - result = subprocess.run( - args, - input=encoded_input, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - - if result.returncode != 0: - raise Exception("failed to evaluate: " + result.stderr.decode()) - - output = json.loads(result.stdout) - - if "alloc" not in output or "result" not in output: - raise Exception("malformed result") - - if self.trace: - receipts: List[Any] = output["result"]["receipts"] - traces: List[List[Dict]] = [] - for i, r in enumerate(receipts): - h = r["transactionHash"] - trace_file_name = f"trace-{i}-{h}.jsonl" - with open(os.path.join(temp_dir.name, trace_file_name), "r") as trace_file: - tx_traces: List[Dict] = [] - for trace_line in trace_file.readlines(): - tx_traces.append(json.loads(trace_line)) - traces.append(tx_traces) - self.append_traces(traces) - - temp_dir.cleanup() - - return output["alloc"], output["result"] - - def version(self) -> str: - """ - Gets `evm` binary version. - """ - if self.cached_version is None: - result = subprocess.run( - [str(self.binary), "-v"], - stdout=subprocess.PIPE, - ) - - if result.returncode != 0: - raise Exception("failed to evaluate: " + result.stderr.decode()) - - self.cached_version = result.stdout.decode().strip() - - return self.cached_version - - def is_fork_supported(self, fork: Fork) -> bool: - """ - Returns True if the fork is supported by the tool - """ - return fork().name() in self.help_string - - -class EvmOneTransitionTool(TransitionTool): - """ - Evmone `evmone-t8n` Transition tool frontend. - """ - - binary: Path - cached_version: Optional[str] = None - trace: bool - - def __init__( - self, - *, - binary: Optional[Path | str] = None, - trace: bool = False, - ): - if binary is None or type(binary) is str: - if binary is None: - binary = "evmone-t8n" - which_path = which(binary) - if which_path is not None: - binary = Path(which_path) - if binary is None or not Path(binary).exists(): - raise Exception("""`evmone-t8n` binary executable is not accessible""") - self.binary = Path(binary) - self.trace = trace - - def evaluate( - self, - alloc: Any, - txs: Any, - env: Any, - fork: Fork, - chain_id: int = 1, - reward: int = 0, - eips: Optional[List[int]] = None, - ) -> Tuple[Dict[str, Any], Dict[str, Any]]: - """ - Executes `evmone-t8n` with the specified arguments. - """ - fork_name = fork.name() - if eips is not None: - fork_name = "+".join([fork_name] + [str(eip) for eip in eips]) - - temp_dir = tempfile.TemporaryDirectory() - - input_contents = { - "alloc": alloc, - "env": env, - "txs": txs, - } - input_paths = { - k: os.path.join(temp_dir.name, f"input_{k}.json") for k in input_contents.keys() - } - for key, val in input_contents.items(): - file_path = os.path.join(temp_dir.name, f"input_{key}.json") - write_json_file(val, file_path) - - # Construct args for evmone-t8n binary - args = [ - str(self.binary), - "--state.fork", - fork_name, - "--input.alloc", - input_paths["alloc"], - "--input.env", - input_paths["env"], - "--input.txs", - input_paths["txs"], - "--output.basedir", - temp_dir.name, - "--output.result", - "output_result.json", - "--output.alloc", - "output_alloc.json", - "--output.body", - "txs.rlp", - "--state.reward", - str(reward), - "--state.chainid", - str(chain_id), - ] - result = subprocess.run( - args, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - - if result.returncode != 0: - raise Exception("failed to evaluate: " + result.stderr.decode()) - - output_paths = { - "alloc": os.path.join(temp_dir.name, "output_alloc.json"), - "result": os.path.join(temp_dir.name, "output_result.json"), - } - - output_contents = {} - for key, file_path in output_paths.items(): - with open(file_path, "r+") as file: - contents = json.load(file) - file.seek(0) - json.dump(contents, file, ensure_ascii=False, indent=4) - file.truncate() - output_contents[key] = contents - - temp_dir.cleanup() - - return output_contents["alloc"], output_contents["result"] - - def version(self) -> str: - """ - Gets `evmone-t8n` binary version. - """ - if self.cached_version is None: - result = subprocess.run( - [str(self.binary), "-v"], - stdout=subprocess.PIPE, - ) - - if result.returncode != 0: - raise Exception("failed to evaluate: " + result.stderr.decode()) - - self.cached_version = result.stdout.decode().strip() - - return self.cached_version - - def is_fork_supported(self, fork: Fork) -> bool: - """ - Returns True if the fork is supported by the tool. - Currently, evmone-t8n provides no way to determine supported forks. - """ - return True +__all__ = ( + "EvmOneTransitionTool", + "GethTransitionTool", + "TransitionTool", +) diff --git a/src/evm_transition_tool/evmone.py b/src/evm_transition_tool/evmone.py new file mode 100644 index 0000000000..6c8359b763 --- /dev/null +++ b/src/evm_transition_tool/evmone.py @@ -0,0 +1,162 @@ +""" +Evmone Transition tool frontend. +""" +import json +import os +import subprocess +import tempfile +from pathlib import Path +from shutil import which +from typing import Any, Dict, List, Optional, Tuple + +from ethereum_test_forks import Fork + +from .transition_tool import TransitionTool + + +def write_json_file(data: Dict[str, Any], file_path: str) -> None: + """ + Write a JSON file to the given path. + """ + with open(file_path, "w") as f: + json.dump(data, f, ensure_ascii=False, indent=4) + + +class EvmOneTransitionTool(TransitionTool): + """ + Evmone `evmone-t8n` Transition tool frontend wrapper class. + """ + + binary: Path + cached_version: Optional[str] = None + trace: bool + + def __init__( + self, + *, + binary: Optional[Path | str] = None, + trace: bool = False, + ): + if binary is None or type(binary) is str: + if binary is None: + binary = "evmone-t8n" + which_path = which(binary) + if which_path is not None: + binary = Path(which_path) + if binary is None or not Path(binary).exists(): + raise Exception("""`evmone-t8n` binary executable is not accessible""") + self.binary = Path(binary) + self.trace = trace + + @staticmethod + def matches_binary_path(binary_path: str) -> bool: + """ + Returns True if the binary path matches the tool + """ + return os.path.basename(binary_path) == "evmone-t8n" + + def evaluate( + self, + alloc: Any, + txs: Any, + env: Any, + fork: Fork, + chain_id: int = 1, + reward: int = 0, + eips: Optional[List[int]] = None, + ) -> Tuple[Dict[str, Any], Dict[str, Any]]: + """ + Executes `evmone-t8n` with the specified arguments. + """ + fork_name = fork.name() + if eips is not None: + fork_name = "+".join([fork_name] + [str(eip) for eip in eips]) + + temp_dir = tempfile.TemporaryDirectory() + + input_contents = { + "alloc": alloc, + "env": env, + "txs": txs, + } + input_paths = { + k: os.path.join(temp_dir.name, f"input_{k}.json") for k in input_contents.keys() + } + for key, val in input_contents.items(): + file_path = os.path.join(temp_dir.name, f"input_{key}.json") + write_json_file(val, file_path) + + # Construct args for evmone-t8n binary + args = [ + str(self.binary), + "--state.fork", + fork_name, + "--input.alloc", + input_paths["alloc"], + "--input.env", + input_paths["env"], + "--input.txs", + input_paths["txs"], + "--output.basedir", + temp_dir.name, + "--output.result", + "output_result.json", + "--output.alloc", + "output_alloc.json", + "--output.body", + "txs.rlp", + "--state.reward", + str(reward), + "--state.chainid", + str(chain_id), + ] + result = subprocess.run( + args, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + + if result.returncode != 0: + raise Exception("failed to evaluate: " + result.stderr.decode()) + + output_paths = { + "alloc": os.path.join(temp_dir.name, "output_alloc.json"), + "result": os.path.join(temp_dir.name, "output_result.json"), + } + + output_contents = {} + for key, file_path in output_paths.items(): + with open(file_path, "r+") as file: + contents = json.load(file) + file.seek(0) + json.dump(contents, file, ensure_ascii=False, indent=4) + file.truncate() + output_contents[key] = contents + + temp_dir.cleanup() + + return output_contents["alloc"], output_contents["result"] + + def version(self) -> str: + """ + Gets `evmone-t8n` binary version. + """ + if self.cached_version is None: + result = subprocess.run( + [str(self.binary), "-v"], + stdout=subprocess.PIPE, + ) + + if result.returncode != 0: + raise Exception("failed to evaluate: " + result.stderr.decode()) + + self.cached_version = result.stdout.decode().strip() + + return self.cached_version + + def is_fork_supported(self, fork: Fork) -> bool: + """ + Returns True if the fork is supported by the tool. + Currently, evmone-t8n provides no way to determine supported forks. + """ + return True diff --git a/src/evm_transition_tool/geth.py b/src/evm_transition_tool/geth.py new file mode 100644 index 0000000000..f64ae05c42 --- /dev/null +++ b/src/evm_transition_tool/geth.py @@ -0,0 +1,162 @@ +""" +Go-ethereum Transition tool frontend. +""" + +import json +import os +import subprocess +import tempfile +from pathlib import Path +from shutil import which +from typing import Any, Dict, List, Optional, Tuple + +from ethereum_test_forks import Fork + +from .transition_tool import TransitionTool + + +class GethTransitionTool(TransitionTool): + """ + Go-ethereum `evm` Transition tool frontend wrapper class. + """ + + binary: Path + cached_version: Optional[str] = None + trace: bool + + def __init__( + self, + *, + binary: Optional[Path | str] = None, + trace: bool = False, + ): + if binary is None or type(binary) is str: + if binary is None: + binary = "evm" + which_path = which(binary) + if which_path is not None: + binary = Path(which_path) + if binary is None or not Path(binary).exists(): + raise Exception( + """`evm` binary executable is not accessible, please refer to + https://github.com/ethereum/go-ethereum on how to compile and + install the full suite of utilities including the `evm` tool""" + ) + self.binary = Path(binary) + self.trace = trace + args = [str(self.binary), "t8n", "--help"] + try: + result = subprocess.run(args, capture_output=True, text=True) + except subprocess.CalledProcessError as e: + raise Exception("evm process unexpectedly returned a non-zero status code: " f"{e}.") + except Exception as e: + raise Exception(f"Unexpected exception calling evm tool: {e}.") + self.help_string = result.stdout + + @staticmethod + def matches_binary_path(binary_path: str) -> bool: + """ + Returns True if the binary path matches the tool + """ + return os.path.basename(binary_path) == "evm" + + def evaluate( + self, + alloc: Any, + txs: Any, + env: Any, + fork: Fork, + chain_id: int = 1, + reward: int = 0, + eips: Optional[List[int]] = None, + ) -> Tuple[Dict[str, Any], Dict[str, Any]]: + """ + Executes `evm t8n` with the specified arguments. + """ + fork_name = fork.name() + if eips is not None: + fork_name = "+".join([fork_name] + [str(eip) for eip in eips]) + + temp_dir = tempfile.TemporaryDirectory() + + if int(env["currentNumber"], 0) == 0: + reward = -1 + args = [ + str(self.binary), + "t8n", + "--input.alloc=stdin", + "--input.txs=stdin", + "--input.env=stdin", + "--output.result=stdout", + "--output.alloc=stdout", + "--output.body=txs.rlp", + f"--output.basedir={temp_dir.name}", + f"--state.fork={fork_name}", + f"--state.chainid={chain_id}", + f"--state.reward={reward}", + ] + + if self.trace: + args.append("--trace") + + stdin = { + "alloc": alloc, + "txs": txs, + "env": env, + } + + encoded_input = str.encode(json.dumps(stdin)) + result = subprocess.run( + args, + input=encoded_input, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + + if result.returncode != 0: + raise Exception("failed to evaluate: " + result.stderr.decode()) + + output = json.loads(result.stdout) + + if "alloc" not in output or "result" not in output: + raise Exception("malformed result") + + if self.trace: + receipts: List[Any] = output["result"]["receipts"] + traces: List[List[Dict]] = [] + for i, r in enumerate(receipts): + h = r["transactionHash"] + trace_file_name = f"trace-{i}-{h}.jsonl" + with open(os.path.join(temp_dir.name, trace_file_name), "r") as trace_file: + tx_traces: List[Dict] = [] + for trace_line in trace_file.readlines(): + tx_traces.append(json.loads(trace_line)) + traces.append(tx_traces) + self.append_traces(traces) + + temp_dir.cleanup() + + return output["alloc"], output["result"] + + def version(self) -> str: + """ + Gets `evm` binary version. + """ + if self.cached_version is None: + result = subprocess.run( + [str(self.binary), "-v"], + stdout=subprocess.PIPE, + ) + + if result.returncode != 0: + raise Exception("failed to evaluate: " + result.stderr.decode()) + + self.cached_version = result.stdout.decode().strip() + + return self.cached_version + + def is_fork_supported(self, fork: Fork) -> bool: + """ + Returns True if the fork is supported by the tool + """ + return fork().name() in self.help_string diff --git a/src/evm_transition_tool/tests/test_evaluate.py b/src/evm_transition_tool/tests/test_evaluate.py index dda709ebfe..e1fb07e9ea 100644 --- a/src/evm_transition_tool/tests/test_evaluate.py +++ b/src/evm_transition_tool/tests/test_evaluate.py @@ -7,12 +7,12 @@ import pytest from ethereum_test_forks import Berlin, Fork, Istanbul, London -from evm_transition_tool import EvmTransitionTool, TransitionTool +from evm_transition_tool import GethTransitionTool, TransitionTool FIXTURES_ROOT = Path(os.path.join("src", "evm_transition_tool", "tests", "fixtures")) -@pytest.mark.parametrize("t8n", [EvmTransitionTool()]) +@pytest.mark.parametrize("t8n", [GethTransitionTool()]) @pytest.mark.parametrize("fork", [London, Istanbul]) @pytest.mark.parametrize( "alloc,base_fee,hash", @@ -79,7 +79,7 @@ class TestEnv: assert t8n.calc_state_root(alloc, fork).startswith(hash) -@pytest.mark.parametrize("evm_tool", [EvmTransitionTool]) +@pytest.mark.parametrize("evm_tool", [GethTransitionTool]) @pytest.mark.parametrize("binary_arg", ["no_binary_arg", "path_type", "str_type"]) def test_evm_tool_binary_arg(evm_tool, binary_arg): if binary_arg == "no_binary_arg": @@ -98,7 +98,7 @@ def test_evm_tool_binary_arg(evm_tool, binary_arg): raise Exception("unknown test parameter") -@pytest.mark.parametrize("t8n", [EvmTransitionTool()]) +@pytest.mark.parametrize("t8n", [GethTransitionTool()]) @pytest.mark.parametrize("test_dir", os.listdir(path=FIXTURES_ROOT)) def test_evm_t8n(t8n: TransitionTool, test_dir: str) -> None: alloc_path = Path(FIXTURES_ROOT, test_dir, "alloc.json") diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py new file mode 100644 index 0000000000..99bb360201 --- /dev/null +++ b/src/evm_transition_tool/transition_tool.py @@ -0,0 +1,198 @@ +""" +Transition tool abstract class. +""" + +from abc import abstractmethod +from pathlib import Path +from typing import Any, Dict, List, Optional, Tuple, Type + +from ethereum_test_forks import Fork + + +class TransitionTool: + """ + Transition tool abstract base class which should be inherited by all transition tool + implementations. + """ + + traces: List[List[List[Dict]]] | None = None + + registered_tools: List[Type["TransitionTool"]] = [] + default_tool: Optional[Type["TransitionTool"]] = None + + # Abstract methods that each tool must implement + + @abstractmethod + def __init__( + self, + *, + binary: Optional[Path | str] = None, + trace: bool = False, + ): + """ + Abstract initialization method that all subclasses must implement. + """ + pass + + def __init_subclass__(cls): + """ + Registers all subclasses of TransitionTool as possible tools. + """ + TransitionTool.register_tool(cls) + + @classmethod + def register_tool(cls, tool_subclass: Type["TransitionTool"]): + """ + Registers a given subclass as tool option. + """ + cls.registered_tools.append(tool_subclass) + + @classmethod + def set_default_tool(cls, tool_subclass: Type["TransitionTool"]): + """ + Registers the default tool subclass. + """ + cls.default_tool = tool_subclass + + @classmethod + def from_binary_path(cls, *, binary_path: Optional[str]) -> Type["TransitionTool"]: + """ + Returns the appropriate TransitionTool subclass derived from the binary path. + """ + assert cls.default_tool is not None, "default transition tool was never set" + + if binary_path is None: + return cls.default_tool + + for subclass in cls.registered_tools: + if subclass.matches_binary_path(binary_path): + return subclass + + return cls.default_tool + + @staticmethod + @abstractmethod + def matches_binary_path(binary_path: str) -> bool: + """ + Returns True if the binary path matches the tool + """ + pass + + @abstractmethod + def evaluate( + self, + alloc: Any, + txs: Any, + env: Any, + fork: Fork, + chain_id: int = 1, + reward: int = 0, + eips: Optional[List[int]] = None, + ) -> Tuple[Dict[str, Any], Dict[str, Any]]: + """ + Simulate a state transition with specified parameters + """ + pass + + @abstractmethod + def version(self) -> str: + """ + Return name and version of tool used to state transition + """ + pass + + @abstractmethod + def is_fork_supported(self, fork: Fork) -> bool: + """ + Returns True if the fork is supported by the tool + """ + pass + + def reset_traces(self): + """ + Resets the internal trace storage for a new test to begin + """ + self.traces = None + + def append_traces(self, new_traces: List[List[Dict]]): + """ + Appends a list of traces of a state transition to the current list + """ + if self.traces is None: + self.traces = [] + self.traces.append(new_traces) + + def get_traces(self) -> List[List[List[Dict]]] | None: + """ + Returns the accumulated traces + """ + return self.traces + + def calc_state_root(self, alloc: Any, fork: Fork) -> bytes: + """ + Calculate the state root for the given `alloc`. + """ + env: Dict[str, Any] = { + "currentCoinbase": "0x0000000000000000000000000000000000000000", + "currentDifficulty": "0x0", + "currentGasLimit": "0x0", + "currentNumber": "0", + "currentTimestamp": "0", + } + + if fork.header_base_fee_required(0, 0): + env["currentBaseFee"] = "7" + + if fork.header_prev_randao_required(0, 0): + env["currentRandom"] = "0" + + if fork.header_withdrawals_required(0, 0): + env["withdrawals"] = [] + + _, result = self.evaluate(alloc, [], env, fork) + state_root = result.get("stateRoot") + if state_root is None or not isinstance(state_root, str): + raise Exception("Unable to calculate state root") + return bytes.fromhex(state_root[2:]) + + def calc_withdrawals_root(self, withdrawals: Any, fork: Fork) -> bytes: + """ + Calculate the state root for the given `alloc`. + """ + if type(withdrawals) is list and len(withdrawals) == 0: + # Optimize returning the empty root immediately + return bytes.fromhex( + "56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421" + ) + + env: Dict[str, Any] = { + "currentCoinbase": "0x0000000000000000000000000000000000000000", + "currentDifficulty": "0x0", + "currentGasLimit": "0x0", + "currentNumber": "0", + "currentTimestamp": "0", + "withdrawals": withdrawals, + } + + if fork.header_base_fee_required(0, 0): + env["currentBaseFee"] = "7" + + if fork.header_prev_randao_required(0, 0): + env["currentRandom"] = "0" + + if fork.header_excess_data_gas_required(0, 0): + env["currentExcessDataGas"] = "0" + + _, result = self.evaluate({}, [], env, fork) + withdrawals_root = result.get("withdrawalsRoot") + if withdrawals_root is None: + raise Exception( + "Unable to calculate withdrawals root: no value returned from transition tool" + ) + if type(withdrawals_root) is not str: + raise Exception( + "Unable to calculate withdrawals root: " + + "incorrect type returned from transition tool: " + + f"{withdrawals_root}" + ) + return bytes.fromhex(withdrawals_root[2:]) diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index 7523416ae0..b629566582 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -116,10 +116,10 @@ def pytest_configure(config): def pytest_report_header(config, start_path): """Add lines to pytest's console output header""" binary_path = config.getoption("evm_bin") - t8nCls = TransitionTool.from_binary_path( + t8n_tool_class = TransitionTool.from_binary_path( binary_path=binary_path, ) - t8n = t8nCls(binary=binary_path) + t8n = t8n_tool_class(binary=binary_path) solc_version_string = Yul("", binary=config.getoption("solc_bin")).version() return [f"{t8n.version()}, solc version {solc_version_string}"] @@ -145,10 +145,10 @@ def t8n(request, evm_bin): """ Returns the configured transition tool. """ - t8nCls = TransitionTool.from_binary_path( + t8n_tool_class = TransitionTool.from_binary_path( binary_path=evm_bin, ) - t8n = t8nCls(binary=evm_bin, trace=request.config.getoption("evm_collect_traces")) + t8n = t8n_tool_class(binary=evm_bin, trace=request.config.getoption("evm_collect_traces")) return t8n From 3a28c9c50d3c905d39add0c7c2e12974d0043d22 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 01:20:24 +0000 Subject: [PATCH 06/29] evm_transition_tool/evmone: clarify tracing --- src/evm_transition_tool/evmone.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/evm_transition_tool/evmone.py b/src/evm_transition_tool/evmone.py index 6c8359b763..d47691a916 100644 --- a/src/evm_transition_tool/evmone.py +++ b/src/evm_transition_tool/evmone.py @@ -46,6 +46,8 @@ def __init__( if binary is None or not Path(binary).exists(): raise Exception("""`evmone-t8n` binary executable is not accessible""") self.binary = Path(binary) + if trace: + raise Exception("`evmone-t8n` does not support tracing.") self.trace = trace @staticmethod From eb01d57cde4dad27c95d0be8ffb9b1f069c2dcd8 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Wed, 5 Jul 2023 10:41:36 +0200 Subject: [PATCH 07/29] refactor: return instantiated t8n tool object instead of subclass def --- src/evm_transition_tool/__init__.py | 3 +- .../tests/test_transition_tool.py | 49 +++++++++++++++++++ src/evm_transition_tool/transition_tool.py | 17 +++++-- src/pytest_plugins/forks/forks.py | 6 +-- src/pytest_plugins/test_filler/test_filler.py | 11 ++--- whitelist.txt | 3 +- 6 files changed, 69 insertions(+), 20 deletions(-) create mode 100644 src/evm_transition_tool/tests/test_transition_tool.py diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index d458b96731..3bb2251575 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -4,7 +4,7 @@ from .evmone import EvmOneTransitionTool from .geth import GethTransitionTool -from .transition_tool import TransitionTool +from .transition_tool import TransitionTool, UnknownTransitionToolError TransitionTool.set_default_tool(GethTransitionTool) @@ -12,4 +12,5 @@ "EvmOneTransitionTool", "GethTransitionTool", "TransitionTool", + "UnknownTransitionToolError", ) diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/evm_transition_tool/tests/test_transition_tool.py new file mode 100644 index 0000000000..d2be78b3b5 --- /dev/null +++ b/src/evm_transition_tool/tests/test_transition_tool.py @@ -0,0 +1,49 @@ +""" +Test the transition tool and subclasses. +""" + +from pathlib import Path + +import pytest + +from evm_transition_tool import ( + EvmOneTransitionTool, + GethTransitionTool, + TransitionTool, + UnknownTransitionToolError, +) + + +def test_default_tool(): + """ + Tests that the default t8n tool is set. + """ + assert TransitionTool.default_tool is GethTransitionTool + + +def test_from_binary(monkeypatch): + """ + Test that `from_binary` instantiates the correct subclass. + """ + + def mock_exists(self): + return True + + # monkeypatch the exists method: the transition tools constructor raises + # an exception if the binary path does not exist + monkeypatch.setattr(Path, "exists", mock_exists) + + assert isinstance(TransitionTool.from_binary_path(binary_path=None), GethTransitionTool) + assert isinstance(TransitionTool.from_binary_path(binary_path="evm"), GethTransitionTool) + assert isinstance( + TransitionTool.from_binary_path(binary_path="evmone-t8n"), EvmOneTransitionTool + ) + + +def test_unknown_binary_path(): + """ + Test that `from_binary_path` raises `UnknownTransitionToolError` for unknown + binary paths. + """ + with pytest.raises(UnknownTransitionToolError): + TransitionTool.from_binary_path(binary_path="unknown_binary_path") diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 99bb360201..c195602d82 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -9,6 +9,12 @@ from ethereum_test_forks import Fork +class UnknownTransitionToolError(Exception): + """Exception raised if an unknown t8n is encountered""" + + pass + + class TransitionTool: """ Transition tool abstract base class which should be inherited by all transition tool @@ -55,20 +61,21 @@ def set_default_tool(cls, tool_subclass: Type["TransitionTool"]): cls.default_tool = tool_subclass @classmethod - def from_binary_path(cls, *, binary_path: Optional[str]) -> Type["TransitionTool"]: + def from_binary_path(cls, *, binary_path: Optional[str], **kwargs) -> "TransitionTool": """ - Returns the appropriate TransitionTool subclass derived from the binary path. + Instantiates the appropriate TransitionTool subclass derived from the + tool's binary path. """ assert cls.default_tool is not None, "default transition tool was never set" if binary_path is None: - return cls.default_tool + return cls.default_tool(binary=binary_path, **kwargs) for subclass in cls.registered_tools: if subclass.matches_binary_path(binary_path): - return subclass + return subclass(binary=binary_path, **kwargs) - return cls.default_tool + raise UnknownTransitionToolError(f"Unknown transition tool binary: {binary_path}") @staticmethod @abstractmethod diff --git a/src/pytest_plugins/forks/forks.py b/src/pytest_plugins/forks/forks.py index 1d53890bdf..3b7bbf5b81 100644 --- a/src/pytest_plugins/forks/forks.py +++ b/src/pytest_plugins/forks/forks.py @@ -159,11 +159,7 @@ def pytest_configure(config): return evm_bin = config.getoption("evm_bin") - transitionToolCls = TransitionTool.from_binary_path(binary_path=evm_bin) - t8n = transitionToolCls( - binary=config.getoption("evm_bin"), - trace=config.getoption("evm_collect_traces"), - ) + t8n = TransitionTool.from_binary_path(binary_path=evm_bin) unsupported_forks = [ fork for fork in config.fork_range if not t8n.is_fork_supported(config.fork_map[fork]) ] diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index b629566582..9818053f93 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -116,10 +116,7 @@ def pytest_configure(config): def pytest_report_header(config, start_path): """Add lines to pytest's console output header""" binary_path = config.getoption("evm_bin") - t8n_tool_class = TransitionTool.from_binary_path( - binary_path=binary_path, - ) - t8n = t8n_tool_class(binary=binary_path) + t8n = TransitionTool.from_binary_path(binary_path=binary_path) solc_version_string = Yul("", binary=config.getoption("solc_bin")).version() return [f"{t8n.version()}, solc version {solc_version_string}"] @@ -145,11 +142,9 @@ def t8n(request, evm_bin): """ Returns the configured transition tool. """ - t8n_tool_class = TransitionTool.from_binary_path( - binary_path=evm_bin, + return TransitionTool.from_binary_path( + binary_path=evm_bin, trace=request.config.getoption("evm_collect_traces") ) - t8n = t8n_tool_class(binary=evm_bin, trace=request.config.getoption("evm_collect_traces")) - return t8n def strip_test_prefix(name: str) -> str: diff --git a/whitelist.txt b/whitelist.txt index 0d0b786089..f04ef7a587 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -299,4 +299,5 @@ evaluatable GHSA -Pomerantz \ No newline at end of file +Pomerantz +instantiation From 6c60144b67ae16c8fd277c61efdfeb497b917321 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Wed, 5 Jul 2023 12:48:07 +0200 Subject: [PATCH 08/29] refactor: change evm-bin command-line argument to type Path Then simplify transition tool constructors by removing type str for binary argument. --- src/evm_transition_tool/evmone.py | 24 +++++++-------- src/evm_transition_tool/geth.py | 30 ++++++++----------- .../tests/test_transition_tool.py | 17 ++++++----- src/evm_transition_tool/transition_tool.py | 15 ++++++++-- src/pytest_plugins/test_filler/test_filler.py | 6 ++-- 5 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/evm_transition_tool/evmone.py b/src/evm_transition_tool/evmone.py index d47691a916..357f48a3a4 100644 --- a/src/evm_transition_tool/evmone.py +++ b/src/evm_transition_tool/evmone.py @@ -3,15 +3,15 @@ """ import json import os +import shutil import subprocess import tempfile from pathlib import Path -from shutil import which from typing import Any, Dict, List, Optional, Tuple from ethereum_test_forks import Fork -from .transition_tool import TransitionTool +from .transition_tool import TransitionTool, TransitionToolNotFoundInPath def write_json_file(data: Dict[str, Any], file_path: str) -> None: @@ -27,6 +27,7 @@ class EvmOneTransitionTool(TransitionTool): Evmone `evmone-t8n` Transition tool frontend wrapper class. """ + default_binary = Path("evmone-t8n") binary: Path cached_version: Optional[str] = None trace: bool @@ -34,28 +35,25 @@ class EvmOneTransitionTool(TransitionTool): def __init__( self, *, - binary: Optional[Path | str] = None, + binary: Optional[Path] = None, trace: bool = False, ): - if binary is None or type(binary) is str: - if binary is None: - binary = "evmone-t8n" - which_path = which(binary) - if which_path is not None: - binary = Path(which_path) - if binary is None or not Path(binary).exists(): - raise Exception("""`evmone-t8n` binary executable is not accessible""") + if binary is None: + binary = self.default_binary + binary = shutil.which(os.path.expanduser(binary)) # type: ignore + if not binary: + raise TransitionToolNotFoundInPath(binary=binary) self.binary = Path(binary) if trace: raise Exception("`evmone-t8n` does not support tracing.") self.trace = trace @staticmethod - def matches_binary_path(binary_path: str) -> bool: + def matches_binary_path(binary_path: Path) -> bool: """ Returns True if the binary path matches the tool """ - return os.path.basename(binary_path) == "evmone-t8n" + return binary_path.name == "evmone-t8n" def evaluate( self, diff --git a/src/evm_transition_tool/geth.py b/src/evm_transition_tool/geth.py index f64ae05c42..f702fc0e7f 100644 --- a/src/evm_transition_tool/geth.py +++ b/src/evm_transition_tool/geth.py @@ -4,15 +4,15 @@ import json import os +import shutil import subprocess import tempfile from pathlib import Path -from shutil import which from typing import Any, Dict, List, Optional, Tuple from ethereum_test_forks import Fork -from .transition_tool import TransitionTool +from .transition_tool import TransitionTool, TransitionToolNotFoundInPath class GethTransitionTool(TransitionTool): @@ -20,28 +20,22 @@ class GethTransitionTool(TransitionTool): Go-ethereum `evm` Transition tool frontend wrapper class. """ - binary: Path + default_binary = Path("evm") + binary: Path | None cached_version: Optional[str] = None trace: bool def __init__( self, *, - binary: Optional[Path | str] = None, + binary: Optional[Path] = None, trace: bool = False, ): - if binary is None or type(binary) is str: - if binary is None: - binary = "evm" - which_path = which(binary) - if which_path is not None: - binary = Path(which_path) - if binary is None or not Path(binary).exists(): - raise Exception( - """`evm` binary executable is not accessible, please refer to - https://github.com/ethereum/go-ethereum on how to compile and - install the full suite of utilities including the `evm` tool""" - ) + if binary is None: + binary = self.default_binary + binary = shutil.which(os.path.expanduser(binary)) # type: ignore + if not binary: + raise TransitionToolNotFoundInPath(binary=binary) self.binary = Path(binary) self.trace = trace args = [str(self.binary), "t8n", "--help"] @@ -54,11 +48,11 @@ def __init__( self.help_string = result.stdout @staticmethod - def matches_binary_path(binary_path: str) -> bool: + def matches_binary_path(binary_path: Path) -> bool: """ Returns True if the binary path matches the tool """ - return os.path.basename(binary_path) == "evm" + return binary_path.name == "evm" def evaluate( self, diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/evm_transition_tool/tests/test_transition_tool.py index d2be78b3b5..4aeebc66c7 100644 --- a/src/evm_transition_tool/tests/test_transition_tool.py +++ b/src/evm_transition_tool/tests/test_transition_tool.py @@ -2,6 +2,7 @@ Test the transition tool and subclasses. """ +import shutil from pathlib import Path import pytest @@ -26,17 +27,17 @@ def test_from_binary(monkeypatch): Test that `from_binary` instantiates the correct subclass. """ - def mock_exists(self): - return True + def mock_which(self): + return "evm" - # monkeypatch the exists method: the transition tools constructor raises - # an exception if the binary path does not exist - monkeypatch.setattr(Path, "exists", mock_exists) + # monkeypatch: the transition tools constructor raises an exception if the binary path does + # not exist + monkeypatch.setattr(shutil, "which", mock_which) assert isinstance(TransitionTool.from_binary_path(binary_path=None), GethTransitionTool) - assert isinstance(TransitionTool.from_binary_path(binary_path="evm"), GethTransitionTool) + assert isinstance(TransitionTool.from_binary_path(binary_path=Path("evm")), GethTransitionTool) assert isinstance( - TransitionTool.from_binary_path(binary_path="evmone-t8n"), EvmOneTransitionTool + TransitionTool.from_binary_path(binary_path=Path("evmone-t8n")), EvmOneTransitionTool ) @@ -46,4 +47,4 @@ def test_unknown_binary_path(): binary paths. """ with pytest.raises(UnknownTransitionToolError): - TransitionTool.from_binary_path(binary_path="unknown_binary_path") + TransitionTool.from_binary_path(binary_path=Path("unknown_binary_path")) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index c195602d82..788ad72cbb 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -15,6 +15,15 @@ class UnknownTransitionToolError(Exception): pass +class TransitionToolNotFoundInPath(Exception): + """Exception raised if an unknown t8n is encountered""" + + def __init__(self, message="The transition tool was not found in the path", binary=None): + if binary: + message = f"{message (binary)}" + super().__init__(message) + + class TransitionTool: """ Transition tool abstract base class which should be inherited by all transition tool @@ -32,7 +41,7 @@ class TransitionTool: def __init__( self, *, - binary: Optional[Path | str] = None, + binary: Optional[Path] = None, trace: bool = False, ): """ @@ -61,7 +70,7 @@ def set_default_tool(cls, tool_subclass: Type["TransitionTool"]): cls.default_tool = tool_subclass @classmethod - def from_binary_path(cls, *, binary_path: Optional[str], **kwargs) -> "TransitionTool": + def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "TransitionTool": """ Instantiates the appropriate TransitionTool subclass derived from the tool's binary path. @@ -79,7 +88,7 @@ def from_binary_path(cls, *, binary_path: Optional[str], **kwargs) -> "Transitio @staticmethod @abstractmethod - def matches_binary_path(binary_path: str) -> bool: + def matches_binary_path(binary_path: Path) -> bool: """ Returns True if the binary path matches the tool """ diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index 9818053f93..f6b382ec4c 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -8,6 +8,7 @@ import json import os import re +from pathlib import Path from typing import Any, Dict, List, Tuple, Type import pytest @@ -37,6 +38,7 @@ def pytest_addoption(parser): "--evm-bin", action="store", dest="evm_bin", + type=Path, default=None, help=( "Path to an evm executable that provides `t8n`. " "Default: First 'evm' entry in PATH" @@ -122,7 +124,7 @@ def pytest_report_header(config, start_path): @pytest.fixture(autouse=True, scope="session") -def evm_bin(request): +def evm_bin(request) -> Path: """ Returns the configured evm tool binary path. """ @@ -138,7 +140,7 @@ def solc_bin(request): @pytest.fixture(autouse=True, scope="session") -def t8n(request, evm_bin): +def t8n(request, evm_bin: Path) -> TransitionTool: """ Returns the configured transition tool. """ From 34cc32dd579dd14e339d31cbb20e1e4dfbb0a884 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Wed, 5 Jul 2023 12:59:45 +0200 Subject: [PATCH 09/29] refactor: move common constructor code to base class --- src/evm_transition_tool/evmone.py | 13 +++---------- src/evm_transition_tool/geth.py | 13 +++---------- src/evm_transition_tool/transition_tool.py | 12 +++++++++++- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/evm_transition_tool/evmone.py b/src/evm_transition_tool/evmone.py index 357f48a3a4..121b0665a1 100644 --- a/src/evm_transition_tool/evmone.py +++ b/src/evm_transition_tool/evmone.py @@ -3,7 +3,6 @@ """ import json import os -import shutil import subprocess import tempfile from pathlib import Path @@ -11,7 +10,7 @@ from ethereum_test_forks import Fork -from .transition_tool import TransitionTool, TransitionToolNotFoundInPath +from .transition_tool import TransitionTool def write_json_file(data: Dict[str, Any], file_path: str) -> None: @@ -38,15 +37,9 @@ def __init__( binary: Optional[Path] = None, trace: bool = False, ): - if binary is None: - binary = self.default_binary - binary = shutil.which(os.path.expanduser(binary)) # type: ignore - if not binary: - raise TransitionToolNotFoundInPath(binary=binary) - self.binary = Path(binary) - if trace: + super().__init__(binary=binary, trace=trace) + if self.trace: raise Exception("`evmone-t8n` does not support tracing.") - self.trace = trace @staticmethod def matches_binary_path(binary_path: Path) -> bool: diff --git a/src/evm_transition_tool/geth.py b/src/evm_transition_tool/geth.py index f702fc0e7f..77b41f4f1a 100644 --- a/src/evm_transition_tool/geth.py +++ b/src/evm_transition_tool/geth.py @@ -4,7 +4,6 @@ import json import os -import shutil import subprocess import tempfile from pathlib import Path @@ -12,7 +11,7 @@ from ethereum_test_forks import Fork -from .transition_tool import TransitionTool, TransitionToolNotFoundInPath +from .transition_tool import TransitionTool class GethTransitionTool(TransitionTool): @@ -21,7 +20,7 @@ class GethTransitionTool(TransitionTool): """ default_binary = Path("evm") - binary: Path | None + binary: Path cached_version: Optional[str] = None trace: bool @@ -31,13 +30,7 @@ def __init__( binary: Optional[Path] = None, trace: bool = False, ): - if binary is None: - binary = self.default_binary - binary = shutil.which(os.path.expanduser(binary)) # type: ignore - if not binary: - raise TransitionToolNotFoundInPath(binary=binary) - self.binary = Path(binary) - self.trace = trace + super().__init__(binary=binary, trace=trace) args = [str(self.binary), "t8n", "--help"] try: result = subprocess.run(args, capture_output=True, text=True) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 788ad72cbb..a48e406e9d 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -2,6 +2,8 @@ Transition tool abstract class. """ +import os +import shutil from abc import abstractmethod from pathlib import Path from typing import Any, Dict, List, Optional, Tuple, Type @@ -34,6 +36,7 @@ class TransitionTool: registered_tools: List[Type["TransitionTool"]] = [] default_tool: Optional[Type["TransitionTool"]] = None + default_binary: Path | None = None # Abstract methods that each tool must implement @@ -47,7 +50,14 @@ def __init__( """ Abstract initialization method that all subclasses must implement. """ - pass + if binary is None: + binary = self.default_binary + # expanduser: tilde does not get expanded by shutil.which + binary = shutil.which(os.path.expanduser(binary)) # type: ignore + if not binary: + raise TransitionToolNotFoundInPath(binary=binary) + self.binary = Path(binary) + self.trace = trace def __init_subclass__(cls): """ From 7915b58c8335578a37d8652ea544b7c471146ddf Mon Sep 17 00:00:00 2001 From: danceratopz Date: Wed, 5 Jul 2023 13:23:28 +0200 Subject: [PATCH 10/29] refactor: move matches_binary_path to a classmethod in base class --- src/evm_transition_tool/evmone.py | 7 ------- src/evm_transition_tool/geth.py | 7 ------- src/evm_transition_tool/transition_tool.py | 9 ++++----- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/src/evm_transition_tool/evmone.py b/src/evm_transition_tool/evmone.py index 121b0665a1..16c9f70097 100644 --- a/src/evm_transition_tool/evmone.py +++ b/src/evm_transition_tool/evmone.py @@ -41,13 +41,6 @@ def __init__( if self.trace: raise Exception("`evmone-t8n` does not support tracing.") - @staticmethod - def matches_binary_path(binary_path: Path) -> bool: - """ - Returns True if the binary path matches the tool - """ - return binary_path.name == "evmone-t8n" - def evaluate( self, alloc: Any, diff --git a/src/evm_transition_tool/geth.py b/src/evm_transition_tool/geth.py index 77b41f4f1a..c945e986a7 100644 --- a/src/evm_transition_tool/geth.py +++ b/src/evm_transition_tool/geth.py @@ -40,13 +40,6 @@ def __init__( raise Exception(f"Unexpected exception calling evm tool: {e}.") self.help_string = result.stdout - @staticmethod - def matches_binary_path(binary_path: Path) -> bool: - """ - Returns True if the binary path matches the tool - """ - return binary_path.name == "evm" - def evaluate( self, alloc: Any, diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index a48e406e9d..280e9dabb0 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -36,7 +36,7 @@ class TransitionTool: registered_tools: List[Type["TransitionTool"]] = [] default_tool: Optional[Type["TransitionTool"]] = None - default_binary: Path | None = None + default_binary: Path # Abstract methods that each tool must implement @@ -96,13 +96,12 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti raise UnknownTransitionToolError(f"Unknown transition tool binary: {binary_path}") - @staticmethod - @abstractmethod - def matches_binary_path(binary_path: Path) -> bool: + @classmethod + def matches_binary_path(cls, binary_path: Path) -> bool: """ Returns True if the binary path matches the tool """ - pass + return binary_path.name == cls.default_binary.name @abstractmethod def evaluate( From 2fbf1cf2a7806a2091405843677e88eca06ce35f Mon Sep 17 00:00:00 2001 From: danceratopz Date: Wed, 5 Jul 2023 13:41:00 +0200 Subject: [PATCH 11/29] fix: fix --evm-bin behaviour with relative paths and tilde --- src/evm_transition_tool/transition_tool.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 280e9dabb0..3e80cd41fb 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -52,8 +52,10 @@ def __init__( """ if binary is None: binary = self.default_binary - # expanduser: tilde does not get expanded by shutil.which - binary = shutil.which(os.path.expanduser(binary)) # type: ignore + else: + # improve behavior of which by resolving the path: ~/relative paths don't work + binary = Path(os.path.expanduser(binary)).resolve() + binary = shutil.which(binary) # type: ignore if not binary: raise TransitionToolNotFoundInPath(binary=binary) self.binary = Path(binary) From 2ccb377dab8106c6ef8584d3e32ad764a8f418f4 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Wed, 5 Jul 2023 13:50:35 +0200 Subject: [PATCH 12/29] chore: raise an error only once if evm bin/tracing options are invalid Otherwise, the error is raised in every test. --- src/pytest_plugins/test_filler/test_filler.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index f6b382ec4c..7785281f4c 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -112,6 +112,11 @@ def pytest_configure(config): "markers", "compile_yul_with(fork): Always compile Yul source using the corresponding evm version.", ) + # Instantiate the transition tool here to check that the binary path/trace option is valid. + # This ensures we only raise an error once, if appropriate, instead of for every test. + TransitionTool.from_binary_path( + binary_path=config.getoption("evm_bin"), trace=config.getoption("evm_collect_traces") + ) @pytest.hookimpl(trylast=True) From 953e2b522c9fe1ed67f4c4f76e6cfc0ed087c6a7 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 18:22:54 +0000 Subject: [PATCH 13/29] fix: error message --- src/evm_transition_tool/transition_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 3e80cd41fb..1a2b8b53d7 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -22,7 +22,7 @@ class TransitionToolNotFoundInPath(Exception): def __init__(self, message="The transition tool was not found in the path", binary=None): if binary: - message = f"{message (binary)}" + message = f"{message} ({binary})" super().__init__(message) From 3952bb61986539d780ae1938bb1136605057c81a Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 18:44:54 +0000 Subject: [PATCH 14/29] fix: change error name --- src/evm_transition_tool/__init__.py | 4 ++-- src/evm_transition_tool/tests/test_transition_tool.py | 6 +++--- src/evm_transition_tool/transition_tool.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index 3bb2251575..117c75f18f 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -4,7 +4,7 @@ from .evmone import EvmOneTransitionTool from .geth import GethTransitionTool -from .transition_tool import TransitionTool, UnknownTransitionToolError +from .transition_tool import TransitionTool, UnknownTransitionTool TransitionTool.set_default_tool(GethTransitionTool) @@ -12,5 +12,5 @@ "EvmOneTransitionTool", "GethTransitionTool", "TransitionTool", - "UnknownTransitionToolError", + "UnknownTransitionTool", ) diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/evm_transition_tool/tests/test_transition_tool.py index 4aeebc66c7..52229a73c0 100644 --- a/src/evm_transition_tool/tests/test_transition_tool.py +++ b/src/evm_transition_tool/tests/test_transition_tool.py @@ -11,7 +11,7 @@ EvmOneTransitionTool, GethTransitionTool, TransitionTool, - UnknownTransitionToolError, + UnknownTransitionTool, ) @@ -43,8 +43,8 @@ def mock_which(self): def test_unknown_binary_path(): """ - Test that `from_binary_path` raises `UnknownTransitionToolError` for unknown + Test that `from_binary_path` raises `UnknownTransitionTool` for unknown binary paths. """ - with pytest.raises(UnknownTransitionToolError): + with pytest.raises(UnknownTransitionTool): TransitionTool.from_binary_path(binary_path=Path("unknown_binary_path")) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 1a2b8b53d7..73091436db 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -11,7 +11,7 @@ from ethereum_test_forks import Fork -class UnknownTransitionToolError(Exception): +class UnknownTransitionTool(Exception): """Exception raised if an unknown t8n is encountered""" pass @@ -96,7 +96,7 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti if subclass.matches_binary_path(binary_path): return subclass(binary=binary_path, **kwargs) - raise UnknownTransitionToolError(f"Unknown transition tool binary: {binary_path}") + raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") @classmethod def matches_binary_path(cls, binary_path: Path) -> bool: From 622f181552642c2cb1fb48f9c0552644b81cb974 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 19:26:05 +0000 Subject: [PATCH 15/29] improvement: harden t8n tool detection --- src/evm_transition_tool/evmone.py | 3 ++ src/evm_transition_tool/geth.py | 3 ++ .../tests/test_transition_tool.py | 54 ++++++++++++++++--- src/evm_transition_tool/transition_tool.py | 22 +++++--- whitelist.txt | 1 + 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/evm_transition_tool/evmone.py b/src/evm_transition_tool/evmone.py index 16c9f70097..9c0315ff92 100644 --- a/src/evm_transition_tool/evmone.py +++ b/src/evm_transition_tool/evmone.py @@ -6,6 +6,7 @@ import subprocess import tempfile from pathlib import Path +from re import compile from typing import Any, Dict, List, Optional, Tuple from ethereum_test_forks import Fork @@ -27,6 +28,8 @@ class EvmOneTransitionTool(TransitionTool): """ default_binary = Path("evmone-t8n") + detect_binary_pattern = compile(r"^evmone-t8n\b") + binary: Path cached_version: Optional[str] = None trace: bool diff --git a/src/evm_transition_tool/geth.py b/src/evm_transition_tool/geth.py index c945e986a7..ca8ba0de0d 100644 --- a/src/evm_transition_tool/geth.py +++ b/src/evm_transition_tool/geth.py @@ -7,6 +7,7 @@ import subprocess import tempfile from pathlib import Path +from re import compile from typing import Any, Dict, List, Optional, Tuple from ethereum_test_forks import Fork @@ -20,6 +21,8 @@ class GethTransitionTool(TransitionTool): """ default_binary = Path("evm") + detect_binary_pattern = compile(r"^evm version\b") + binary: Path cached_version: Optional[str] = None trace: bool diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/evm_transition_tool/tests/test_transition_tool.py index 52229a73c0..bf8deca025 100644 --- a/src/evm_transition_tool/tests/test_transition_tool.py +++ b/src/evm_transition_tool/tests/test_transition_tool.py @@ -2,8 +2,10 @@ Test the transition tool and subclasses. """ +import os import shutil from pathlib import Path +from typing import Type import pytest @@ -22,23 +24,61 @@ def test_default_tool(): assert TransitionTool.default_tool is GethTransitionTool -def test_from_binary(monkeypatch): +@pytest.mark.parametrize( + "binary_path,which_result,read_result,expected_class", + [ + ( + Path("evm"), + "evm", + "evm version 1.12.1-unstable-c7b099b2-20230627", + GethTransitionTool, + ), + ( + Path("evmone-t8n"), + "evmone-t8n", + "evmone-t8n 0.11.0-dev+commit.93997506", + EvmOneTransitionTool, + ), + ( + None, + "evm", + "evm version 1.12.1-unstable-c7b099b2-20230627", + GethTransitionTool, + ), + ], +) +def test_from_binary( + monkeypatch, + binary_path: Path | None, + which_result: str, + read_result: str, + expected_class: Type[TransitionTool], +): """ Test that `from_binary` instantiates the correct subclass. """ def mock_which(self): - return "evm" + return which_result + + class ReadResult: + read_result: str + + def __init__(self, read_result): + self.read_result = read_result + + def read(self): + return self.read_result + + def mock_popen(path): + return ReadResult(read_result) # monkeypatch: the transition tools constructor raises an exception if the binary path does # not exist monkeypatch.setattr(shutil, "which", mock_which) + monkeypatch.setattr(os, "popen", mock_popen) - assert isinstance(TransitionTool.from_binary_path(binary_path=None), GethTransitionTool) - assert isinstance(TransitionTool.from_binary_path(binary_path=Path("evm")), GethTransitionTool) - assert isinstance( - TransitionTool.from_binary_path(binary_path=Path("evmone-t8n")), EvmOneTransitionTool - ) + assert isinstance(TransitionTool.from_binary_path(binary_path=binary_path), expected_class) def test_unknown_binary_path(): diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 73091436db..bed7b65345 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -6,6 +6,7 @@ import shutil from abc import abstractmethod from pathlib import Path +from re import Pattern from typing import Any, Dict, List, Optional, Tuple, Type from ethereum_test_forks import Fork @@ -37,6 +38,8 @@ class TransitionTool: registered_tools: List[Type["TransitionTool"]] = [] default_tool: Optional[Type["TransitionTool"]] = None default_binary: Path + detect_binary_pattern: Pattern + version_flag: str = "-v" # Abstract methods that each tool must implement @@ -92,18 +95,25 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti if binary_path is None: return cls.default_tool(binary=binary_path, **kwargs) - for subclass in cls.registered_tools: - if subclass.matches_binary_path(binary_path): - return subclass(binary=binary_path, **kwargs) + binary = shutil.which(Path(os.path.expanduser(binary_path)).resolve()) + if binary: + for subclass in cls.registered_tools: + if subclass.detect_binary(binary_path): + return subclass(binary=binary_path, **kwargs) raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") @classmethod - def matches_binary_path(cls, binary_path: Path) -> bool: + def detect_binary(cls, binary_path: Path) -> bool: """ - Returns True if the binary path matches the tool + Returns True if the binary matches the tool """ - return binary_path.name == cls.default_binary.name + assert cls.detect_binary_pattern is not None + + # Run the binary with the appropriate version flag + version = os.popen(f"{binary_path} {cls.version_flag}").read() + + return cls.detect_binary_pattern.match(version) is not None @abstractmethod def evaluate( diff --git a/whitelist.txt b/whitelist.txt index f04ef7a587..58b3530c8f 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -276,6 +276,7 @@ parametrized param params parametrize +popen pytester pytestmark regexes From a35fe2cd5ceefda4cffb7ab73a48a390634f0396 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 20:36:04 +0000 Subject: [PATCH 16/29] improvement: efficient tool detection --- src/evm_transition_tool/transition_tool.py | 25 +++++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index bed7b65345..be62bdd8f5 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -5,6 +5,7 @@ import os import shutil from abc import abstractmethod +from itertools import groupby from pathlib import Path from re import Pattern from typing import Any, Dict, List, Optional, Tuple, Type @@ -97,23 +98,31 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti binary = shutil.which(Path(os.path.expanduser(binary_path)).resolve()) if binary: - for subclass in cls.registered_tools: - if subclass.detect_binary(binary_path): - return subclass(binary=binary_path, **kwargs) + # Group the tools by version flag, so we only have to call the tool once for all the + # classes that share the same version flag + for version_flag, subclasses in groupby( + cls.registered_tools, key=lambda x: x.version_flag + ): + try: + binary_output = os.popen(f"{binary_path} {version_flag}").read() + except Exception: + # If the tool doesn't support the version flag, + # we'll get an non-zero exit code. + continue + for subclass in subclasses: + if subclass.detect_binary(binary_output): + return subclass(binary=binary_path, **kwargs) raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") @classmethod - def detect_binary(cls, binary_path: Path) -> bool: + def detect_binary(cls, binary_output: str) -> bool: """ Returns True if the binary matches the tool """ assert cls.detect_binary_pattern is not None - # Run the binary with the appropriate version flag - version = os.popen(f"{binary_path} {cls.version_flag}").read() - - return cls.detect_binary_pattern.match(version) is not None + return cls.detect_binary_pattern.match(binary_output) is not None @abstractmethod def evaluate( From 78454bec678a12ad6d3ed12bcb4e13492540123e Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 20:36:27 +0000 Subject: [PATCH 17/29] tox: whitelist --- whitelist.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/whitelist.txt b/whitelist.txt index 58b3530c8f..fcd051327a 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -264,6 +264,7 @@ fspath funcargs getgroup getoption +groupby hookimpl hookwrapper IGNORECASE @@ -284,6 +285,7 @@ reportinfo ret runpytest runtest +subclasses tryfirst trylast usefixtures From 0aaf20fd86aa3f8484ca16b251ad076b2713d7a0 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 21:38:56 +0000 Subject: [PATCH 18/29] fix: shutil.which usage --- src/evm_transition_tool/transition_tool.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index be62bdd8f5..15a5992f1b 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -58,8 +58,10 @@ def __init__( binary = self.default_binary else: # improve behavior of which by resolving the path: ~/relative paths don't work - binary = Path(os.path.expanduser(binary)).resolve() - binary = shutil.which(binary) # type: ignore + try: + binary = Path(os.path.expanduser(binary)).resolve(strict=True) + except FileNotFoundError: + binary = shutil.which(binary) # type: ignore if not binary: raise TransitionToolNotFoundInPath(binary=binary) self.binary = Path(binary) @@ -95,23 +97,27 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti if binary_path is None: return cls.default_tool(binary=binary_path, **kwargs) + try: + binary = Path(os.path.expanduser(binary_path)).resolve(strict=True) + except FileNotFoundError: + binary = shutil.which(binary_path) # type: ignore - binary = shutil.which(Path(os.path.expanduser(binary_path)).resolve()) - if binary: + if binary is not None: + binary = Path(binary) # Group the tools by version flag, so we only have to call the tool once for all the # classes that share the same version flag for version_flag, subclasses in groupby( cls.registered_tools, key=lambda x: x.version_flag ): try: - binary_output = os.popen(f"{binary_path} {version_flag}").read() + binary_output = os.popen(f"{binary} {version_flag}").read() except Exception: # If the tool doesn't support the version flag, # we'll get an non-zero exit code. continue for subclass in subclasses: if subclass.detect_binary(binary_output): - return subclass(binary=binary_path, **kwargs) + return subclass(binary=binary, **kwargs) raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") From b9d86b5064244df4d2badc00daedb83d934944e8 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 5 Jul 2023 22:23:03 +0000 Subject: [PATCH 19/29] fix: remove try-catch --- src/evm_transition_tool/transition_tool.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 15a5992f1b..3486974f1d 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -58,10 +58,10 @@ def __init__( binary = self.default_binary else: # improve behavior of which by resolving the path: ~/relative paths don't work - try: - binary = Path(os.path.expanduser(binary)).resolve(strict=True) - except FileNotFoundError: - binary = shutil.which(binary) # type: ignore + resolved_path = Path(os.path.expanduser(binary)).resolve() + if resolved_path.exists(): + binary = resolved_path + binary = shutil.which(binary) # type: ignore if not binary: raise TransitionToolNotFoundInPath(binary=binary) self.binary = Path(binary) @@ -97,10 +97,11 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti if binary_path is None: return cls.default_tool(binary=binary_path, **kwargs) - try: - binary = Path(os.path.expanduser(binary_path)).resolve(strict=True) - except FileNotFoundError: - binary = shutil.which(binary_path) # type: ignore + + resolved_path = Path(os.path.expanduser(binary_path)).resolve() + if resolved_path.exists(): + binary = resolved_path + binary = shutil.which(binary_path) # type: ignore if binary is not None: binary = Path(binary) From 4e56dff52277939213af4974ef5aa6f433233411 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 6 Jul 2023 07:53:15 -0600 Subject: [PATCH 20/29] Separate binary not found and unknown binary errors Co-authored-by: Dan --- src/evm_transition_tool/transition_tool.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 3486974f1d..2f6f37aa6e 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -119,8 +119,9 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti for subclass in subclasses: if subclass.detect_binary(binary_output): return subclass(binary=binary, **kwargs) + raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") - raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") + raise TransitionToolNotFoundInPath(binary=binary) @classmethod def detect_binary(cls, binary_output: str) -> bool: From a64b028af55e9d4fb20457c18d6868dfbd449d3b Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 6 Jul 2023 13:56:20 +0000 Subject: [PATCH 21/29] nit: indentation --- src/evm_transition_tool/transition_tool.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 2f6f37aa6e..8053a98aca 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -119,7 +119,8 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti for subclass in subclasses: if subclass.detect_binary(binary_output): return subclass(binary=binary, **kwargs) - raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") + + raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") raise TransitionToolNotFoundInPath(binary=binary) From 9ff21508790118861df985dbd8173261c2146892 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 6 Jul 2023 14:02:08 +0000 Subject: [PATCH 22/29] fix: evm_transition_tool tests --- src/evm_transition_tool/__init__.py | 3 ++- src/evm_transition_tool/tests/test_transition_tool.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/evm_transition_tool/__init__.py b/src/evm_transition_tool/__init__.py index 117c75f18f..531e81996c 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/evm_transition_tool/__init__.py @@ -4,7 +4,7 @@ from .evmone import EvmOneTransitionTool from .geth import GethTransitionTool -from .transition_tool import TransitionTool, UnknownTransitionTool +from .transition_tool import TransitionTool, TransitionToolNotFoundInPath, UnknownTransitionTool TransitionTool.set_default_tool(GethTransitionTool) @@ -12,5 +12,6 @@ "EvmOneTransitionTool", "GethTransitionTool", "TransitionTool", + "TransitionToolNotFoundInPath", "UnknownTransitionTool", ) diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/evm_transition_tool/tests/test_transition_tool.py index bf8deca025..20297a67a1 100644 --- a/src/evm_transition_tool/tests/test_transition_tool.py +++ b/src/evm_transition_tool/tests/test_transition_tool.py @@ -13,7 +13,7 @@ EvmOneTransitionTool, GethTransitionTool, TransitionTool, - UnknownTransitionTool, + TransitionToolNotFoundInPath, ) @@ -86,5 +86,5 @@ def test_unknown_binary_path(): Test that `from_binary_path` raises `UnknownTransitionTool` for unknown binary paths. """ - with pytest.raises(UnknownTransitionTool): + with pytest.raises(TransitionToolNotFoundInPath): TransitionTool.from_binary_path(binary_path=Path("unknown_binary_path")) From 2c7eb0d0d187916dc02b1dd541b91b483fd702a8 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Thu, 6 Jul 2023 16:58:48 +0200 Subject: [PATCH 23/29] fix: save the resolved path to binary_path This broke --collect-only with --evm-bin=~/bin/evm as the result of expanduser result was not saved. --- src/evm_transition_tool/transition_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 8053a98aca..f20e9ed2b3 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -100,7 +100,7 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti resolved_path = Path(os.path.expanduser(binary_path)).resolve() if resolved_path.exists(): - binary = resolved_path + binary_path = resolved_path binary = shutil.which(binary_path) # type: ignore if binary is not None: From 2198a3596b84e83cd68ff32744dea9b773d74933 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Thu, 6 Jul 2023 17:01:12 +0200 Subject: [PATCH 24/29] fix: don't try to access the evm binary for collect-only There's no need to run error-checking or report versions in the header when running `fill --collect-only`. --- src/pytest_plugins/test_filler/test_filler.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/pytest_plugins/test_filler/test_filler.py b/src/pytest_plugins/test_filler/test_filler.py index 7785281f4c..3fd88b4c7e 100644 --- a/src/pytest_plugins/test_filler/test_filler.py +++ b/src/pytest_plugins/test_filler/test_filler.py @@ -112,6 +112,8 @@ def pytest_configure(config): "markers", "compile_yul_with(fork): Always compile Yul source using the corresponding evm version.", ) + if config.option.collectonly: + return # Instantiate the transition tool here to check that the binary path/trace option is valid. # This ensures we only raise an error once, if appropriate, instead of for every test. TransitionTool.from_binary_path( @@ -122,6 +124,8 @@ def pytest_configure(config): @pytest.hookimpl(trylast=True) def pytest_report_header(config, start_path): """Add lines to pytest's console output header""" + if config.option.collectonly: + return binary_path = config.getoption("evm_bin") t8n = TransitionTool.from_binary_path(binary_path=binary_path) solc_version_string = Yul("", binary=config.getoption("solc_bin")).version() From 53fe032aec0bd04c2f3d1493ee80461eb25d5310 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Thu, 6 Jul 2023 17:04:21 +0200 Subject: [PATCH 25/29] fix: use popen as a context manager Ensure the file opened by popen is closed. --- src/evm_transition_tool/transition_tool.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index f20e9ed2b3..72511f2765 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -111,7 +111,8 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti cls.registered_tools, key=lambda x: x.version_flag ): try: - binary_output = os.popen(f"{binary} {version_flag}").read() + with os.popen(f"{binary} {version_flag}") as f: + binary_output = f.read() except Exception: # If the tool doesn't support the version flag, # we'll get an non-zero exit code. From 60f4a1831f4ccbb784b3ffcfae8916a649edfbbf Mon Sep 17 00:00:00 2001 From: danceratopz Date: Thu, 6 Jul 2023 17:07:20 +0200 Subject: [PATCH 26/29] chore: fix docstring in exception class --- src/evm_transition_tool/transition_tool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 72511f2765..bf449f042a 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -20,7 +20,7 @@ class UnknownTransitionTool(Exception): class TransitionToolNotFoundInPath(Exception): - """Exception raised if an unknown t8n is encountered""" + """Exception raised if the specified t8n tool is not found in the path""" def __init__(self, message="The transition tool was not found in the path", binary=None): if binary: From fdf2ea63e5be2458dfbcd55129f9078b83267fb4 Mon Sep 17 00:00:00 2001 From: danceratopz Date: Thu, 6 Jul 2023 17:10:51 +0200 Subject: [PATCH 27/29] fix: distinguish between not found in path & unknown t8n tool errors --- src/evm_transition_tool/transition_tool.py | 37 ++++++++++++---------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index bf449f042a..253a0e4312 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -103,23 +103,26 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti binary_path = resolved_path binary = shutil.which(binary_path) # type: ignore - if binary is not None: - binary = Path(binary) - # Group the tools by version flag, so we only have to call the tool once for all the - # classes that share the same version flag - for version_flag, subclasses in groupby( - cls.registered_tools, key=lambda x: x.version_flag - ): - try: - with os.popen(f"{binary} {version_flag}") as f: - binary_output = f.read() - except Exception: - # If the tool doesn't support the version flag, - # we'll get an non-zero exit code. - continue - for subclass in subclasses: - if subclass.detect_binary(binary_output): - return subclass(binary=binary, **kwargs) + if not binary: + raise TransitionToolNotFoundInPath(binary=binary) + + binary = Path(binary) + + # Group the tools by version flag, so we only have to call the tool once for all the + # classes that share the same version flag + for version_flag, subclasses in groupby( + cls.registered_tools, key=lambda x: x.version_flag + ): + try: + with os.popen(f"{binary} {version_flag}") as f: + binary_output = f.read() + except Exception: + # If the tool doesn't support the version flag, + # we'll get an non-zero exit code. + continue + for subclass in subclasses: + if subclass.detect_binary(binary_output): + return subclass(binary=binary, **kwargs) raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") From f4fd9b6f9ba6e591c6eef1213bd359a05e88f716 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 6 Jul 2023 15:17:31 +0000 Subject: [PATCH 28/29] fix: error indentation --- src/evm_transition_tool/transition_tool.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/evm_transition_tool/transition_tool.py b/src/evm_transition_tool/transition_tool.py index 253a0e4312..f79b461489 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/evm_transition_tool/transition_tool.py @@ -124,9 +124,7 @@ def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> "Transiti if subclass.detect_binary(binary_output): return subclass(binary=binary, **kwargs) - raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") - - raise TransitionToolNotFoundInPath(binary=binary) + raise UnknownTransitionTool(f"Unknown transition tool binary: {binary_path}") @classmethod def detect_binary(cls, binary_output: str) -> bool: From 2a13c112aee02e429f189f5c5c9406463367fb64 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 6 Jul 2023 15:55:09 +0000 Subject: [PATCH 29/29] fix: evm_transition_tool tests --- src/evm_transition_tool/tests/test_transition_tool.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/evm_transition_tool/tests/test_transition_tool.py index 20297a67a1..6193bbaf90 100644 --- a/src/evm_transition_tool/tests/test_transition_tool.py +++ b/src/evm_transition_tool/tests/test_transition_tool.py @@ -70,6 +70,12 @@ def __init__(self, read_result): def read(self): return self.read_result + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + pass + def mock_popen(path): return ReadResult(read_result)