From 619e7d8340802f457f5f4e1ada0bff3826a1c27c Mon Sep 17 00:00:00 2001 From: danceratopz Date: Tue, 15 Oct 2024 09:00:58 +0100 Subject: [PATCH] refactor(ethereum_clis): move generic code from `TransitionTool` to `EthereumCLI`#894 --- .flake8 | 4 +- docs/CHANGELOG.md | 1 + docs/library/ethereum_clis.md | 3 + docs/library/evm_transition_tool.md | 3 - docs/library/index.md | 2 +- docs/navigation.md | 4 +- .../__init__.py | 19 ++- src/ethereum_clis/clis/__init__.py | 3 + .../clis}/besu.py | 4 +- .../clis}/ethereumjs.py | 3 +- .../clis}/evmone.py | 3 +- .../clis}/execution_specs.py | 2 +- .../clis}/geth.py | 2 +- .../clis}/nimbus.py | 2 +- src/ethereum_clis/ethereum_cli.py | 155 ++++++++++++++++++ .../file_utils.py | 0 .../py.typed | 0 .../tests/fixtures/1/alloc.json | 0 .../tests/fixtures/1/env.json | 0 .../tests/fixtures/1/exp.json | 0 .../tests/fixtures/1/txs.json | 0 .../tests/fixtures/3/alloc.json | 0 .../tests/fixtures/3/env.json | 0 .../tests/fixtures/3/exp.json | 0 .../tests/fixtures/3/readme.md | 0 .../tests/fixtures/3/txs.json | 0 .../tests/test_evaluate.py | 4 +- .../tests/test_transition_tool.py | 8 +- .../transition_tool.py | 128 +-------------- .../types.py | 0 src/ethereum_test_specs/base.py | 2 +- src/ethereum_test_specs/blockchain.py | 72 ++++---- src/ethereum_test_specs/eof.py | 2 +- src/ethereum_test_specs/state.py | 2 +- src/ethereum_test_specs/tests/test_expect.py | 2 +- .../tests/test_fixtures.py | 2 +- src/ethereum_test_tools/tests/test_code.py | 2 +- src/pytest_plugins/consume/direct/conftest.py | 2 +- .../consume/direct/test_via_direct.py | 2 +- src/pytest_plugins/filler/filler.py | 3 +- .../filler/tests/test_filler.py | 2 +- src/pytest_plugins/forks/forks.py | 2 +- .../eip4895_withdrawals/test_withdrawals.py | 2 +- whitelist.txt | 2 + 44 files changed, 251 insertions(+), 198 deletions(-) create mode 100644 docs/library/ethereum_clis.md delete mode 100644 docs/library/evm_transition_tool.md rename src/{evm_transition_tool => ethereum_clis}/__init__.py (51%) create mode 100644 src/ethereum_clis/clis/__init__.py rename src/{evm_transition_tool => ethereum_clis/clis}/besu.py (97%) rename src/{evm_transition_tool => ethereum_clis/clis}/ethereumjs.py (95%) rename src/{evm_transition_tool => ethereum_clis/clis}/evmone.py (95%) rename src/{evm_transition_tool => ethereum_clis/clis}/execution_specs.py (98%) rename src/{evm_transition_tool => ethereum_clis/clis}/geth.py (98%) rename src/{evm_transition_tool => ethereum_clis/clis}/nimbus.py (97%) create mode 100644 src/ethereum_clis/ethereum_cli.py rename src/{evm_transition_tool => ethereum_clis}/file_utils.py (100%) rename src/{evm_transition_tool => ethereum_clis}/py.typed (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/1/alloc.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/1/env.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/1/exp.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/1/txs.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/3/alloc.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/3/env.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/3/exp.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/3/readme.md (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/fixtures/3/txs.json (100%) rename src/{evm_transition_tool => ethereum_clis}/tests/test_evaluate.py (97%) rename src/{evm_transition_tool => ethereum_clis}/tests/test_transition_tool.py (91%) rename src/{evm_transition_tool => ethereum_clis}/transition_tool.py (79%) rename src/{evm_transition_tool => ethereum_clis}/types.py (100%) diff --git a/.flake8 b/.flake8 index 8ca1a2aeaec..acc500a1de5 100644 --- a/.flake8 +++ b/.flake8 @@ -23,11 +23,11 @@ extend-ignore = E203, D107, D200, D203, D205, # Ignore N806: Variable names with all caps (ALL_CAPS) max-line-length = 99 per-file-ignore = - tests/evm_transition_tool/test_evaluate.py:E501 + tests/ethereum_clis/test_evaluate.py:E501 extend-exclude = setup.py - src/evm_transition_tool/tests/ + src/ethereum_clis/tests/ src/ethereum_test_tools/tests/ src/ethereum_test_forks/tests/ diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 14a10be54d5..805123aca60 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -51,6 +51,7 @@ Test fixtures for use by clients are available for each release on the [Github r ### 🔧 EVM Tools - ✨ Fill test fixtures using EELS by default. EEST now uses the [`ethereum-specs-evm-resolver`](https://github.com/petertdavies/ethereum-spec-evm-resolver) with the EELS daemon ([#792](https://github.com/ethereum/execution-spec-tests/pull/792)). +- 🔀 Move the `evm_transition_tool` package to `ethereum_clis` and derive the transition tool CL interfaces from a shared `EthereumCLI` class that can be reused for other sub-commands ([#894](https://github.com/ethereum/execution-spec-tests/pull/894)). ### 📋 Misc diff --git a/docs/library/ethereum_clis.md b/docs/library/ethereum_clis.md new file mode 100644 index 00000000000..c4595a50afc --- /dev/null +++ b/docs/library/ethereum_clis.md @@ -0,0 +1,3 @@ +# Ethereum CLIs Package + +::: ethereum_clis diff --git a/docs/library/evm_transition_tool.md b/docs/library/evm_transition_tool.md deleted file mode 100644 index b3035547380..00000000000 --- a/docs/library/evm_transition_tool.md +++ /dev/null @@ -1,3 +0,0 @@ -# EVM Transition Tool Package - -::: evm_transition_tool diff --git a/docs/library/index.md b/docs/library/index.md index 1d29d8eded4..de5b5183f1e 100644 --- a/docs/library/index.md +++ b/docs/library/index.md @@ -10,5 +10,5 @@ Execution spec tests consists of several packages that implement helper classes - [`ethereum_test_tools`](./ethereum_test_tools.md) - provides primitives and helpers to test Ethereum execution clients. - [`ethereum_test_types`](./ethereum_test_types.md) - provides Ethereum types built on top of the base types which are used to define test cases and interact with other libraries. - [`ethereum_test_vm`](./ethereum_test_vm.md) - provides definitions for the Ethereum Virtual Machine (EVM) as used to define bytecode in test cases. -- [`evm_transition_tool`](./evm_transition_tool.md) - a wrapper for the transition (`t8n`) tool. +- [`ethereum_clis`](./ethereum_clis.md) - a wrapper for the transition (`t8n`) tool. - [`pytest_plugins`](./pytest_plugins/index.md) - contains pytest customizations that provide additional functionality for generating test fixtures. diff --git a/docs/navigation.md b/docs/navigation.md index ec622fe46e3..33b9de5028a 100644 --- a/docs/navigation.md +++ b/docs/navigation.md @@ -36,7 +36,7 @@ * [Running Github Actions Locally](dev/test_actions_locally.md) * [Changelog](CHANGELOG.md) * [Library Reference](library/index.md) - * [Miscellaneous CLI Tools](library/cli/index.md) + * [EEST CLI Tools](library/cli/index.md) * [Ethereum Test Base Types Package](library/ethereum_test_base_types.md) * [Ethereum Test Exceptions Package](library/ethereum_test_exceptions.md) * [Ethereum Test Fixtures Package](library/ethereum_test_fixtures.md) @@ -45,5 +45,5 @@ * [Ethereum Test Tools Package](library/ethereum_test_tools.md) * [Ethereum Test Types Package](library/ethereum_test_types.md) * [Ethereum Test VM Package](library/ethereum_test_vm.md) - * [EVM Transition Tool Package](library/evm_transition_tool.md) + * [Ethereum CLIs Package](library/ethereum_clis.md) * [Pytest Plugins](library/pytest_plugins/index.md) diff --git a/src/evm_transition_tool/__init__.py b/src/ethereum_clis/__init__.py similarity index 51% rename from src/evm_transition_tool/__init__.py rename to src/ethereum_clis/__init__.py index d2eefd8419f..fdf9f80ae9b 100644 --- a/src/evm_transition_tool/__init__.py +++ b/src/ethereum_clis/__init__.py @@ -2,13 +2,14 @@ Library of Python wrappers for the different implementations of transition tools. """ -from .besu import BesuTransitionTool -from .ethereumjs import EthereumJSTransitionTool -from .evmone import EvmOneTransitionTool -from .execution_specs import ExecutionSpecsTransitionTool -from .geth import GethTransitionTool -from .nimbus import NimbusTransitionTool -from .transition_tool import TransitionTool, TransitionToolNotFoundInPath, UnknownTransitionTool +from .clis.besu import BesuTransitionTool +from .clis.ethereumjs import EthereumJSTransitionTool +from .clis.evmone import EvmOneTransitionTool +from .clis.execution_specs import ExecutionSpecsTransitionTool +from .clis.geth import GethTransitionTool +from .clis.nimbus import NimbusTransitionTool +from .ethereum_cli import CLINotFoundInPath, UnknownCLI +from .transition_tool import TransitionTool from .types import Result, TransitionToolOutput TransitionTool.set_default_tool(ExecutionSpecsTransitionTool) @@ -23,6 +24,6 @@ "Result", "TransitionTool", "TransitionToolOutput", - "TransitionToolNotFoundInPath", - "UnknownTransitionTool", + "CLINotFoundInPath", + "UnknownCLI", ) diff --git a/src/ethereum_clis/clis/__init__.py b/src/ethereum_clis/clis/__init__.py new file mode 100644 index 00000000000..b93d2547b1c --- /dev/null +++ b/src/ethereum_clis/clis/__init__.py @@ -0,0 +1,3 @@ +""" +Package containing concrete implementations of Ethereum CL interfaces. +""" diff --git a/src/evm_transition_tool/besu.py b/src/ethereum_clis/clis/besu.py similarity index 97% rename from src/evm_transition_tool/besu.py rename to src/ethereum_clis/clis/besu.py index ce4b3fc3d6f..12fb502e720 100644 --- a/src/evm_transition_tool/besu.py +++ b/src/ethereum_clis/clis/besu.py @@ -17,8 +17,8 @@ from ethereum_test_forks import Fork from ethereum_test_types import Alloc, Environment, Transaction -from .transition_tool import TransitionTool, dump_files_to_directory, model_dump_config -from .types import TransitionToolInput, TransitionToolOutput +from ..transition_tool import TransitionTool, dump_files_to_directory, model_dump_config +from ..types import TransitionToolInput, TransitionToolOutput class BesuTransitionTool(TransitionTool): diff --git a/src/evm_transition_tool/ethereumjs.py b/src/ethereum_clis/clis/ethereumjs.py similarity index 95% rename from src/evm_transition_tool/ethereumjs.py rename to src/ethereum_clis/clis/ethereumjs.py index 03db217b618..ec3370fee64 100644 --- a/src/evm_transition_tool/ethereumjs.py +++ b/src/ethereum_clis/clis/ethereumjs.py @@ -1,13 +1,14 @@ """ EthereumJS Transition tool interface. """ + from pathlib import Path from re import compile from typing import Optional from ethereum_test_forks import Fork -from .transition_tool import TransitionTool +from ..transition_tool import TransitionTool class EthereumJSTransitionTool(TransitionTool): diff --git a/src/evm_transition_tool/evmone.py b/src/ethereum_clis/clis/evmone.py similarity index 95% rename from src/evm_transition_tool/evmone.py rename to src/ethereum_clis/clis/evmone.py index 568c2d48ccb..c0c834c0ec5 100644 --- a/src/evm_transition_tool/evmone.py +++ b/src/ethereum_clis/clis/evmone.py @@ -1,13 +1,14 @@ """ Evmone Transition tool interface. """ + from pathlib import Path from re import compile from typing import Optional from ethereum_test_forks import Fork -from .transition_tool import TransitionTool +from ..transition_tool import TransitionTool class EvmOneTransitionTool(TransitionTool): diff --git a/src/evm_transition_tool/execution_specs.py b/src/ethereum_clis/clis/execution_specs.py similarity index 98% rename from src/evm_transition_tool/execution_specs.py rename to src/ethereum_clis/clis/execution_specs.py index 8f8a7db5bde..1cebd53dfa9 100644 --- a/src/evm_transition_tool/execution_specs.py +++ b/src/ethereum_clis/clis/execution_specs.py @@ -13,7 +13,7 @@ from ethereum_test_forks import Fork -from .transition_tool import TransitionTool +from ..transition_tool import TransitionTool DAEMON_STARTUP_TIMEOUT_SECONDS = 5 diff --git a/src/evm_transition_tool/geth.py b/src/ethereum_clis/clis/geth.py similarity index 98% rename from src/evm_transition_tool/geth.py rename to src/ethereum_clis/clis/geth.py index 700d3a43d5b..905a782f90b 100644 --- a/src/evm_transition_tool/geth.py +++ b/src/ethereum_clis/clis/geth.py @@ -13,7 +13,7 @@ from ethereum_test_fixtures import BlockchainFixture, StateFixture from ethereum_test_forks import Fork -from .transition_tool import FixtureFormat, TransitionTool, dump_files_to_directory +from ..transition_tool import FixtureFormat, TransitionTool, dump_files_to_directory class GethTransitionTool(TransitionTool): diff --git a/src/evm_transition_tool/nimbus.py b/src/ethereum_clis/clis/nimbus.py similarity index 97% rename from src/evm_transition_tool/nimbus.py rename to src/ethereum_clis/clis/nimbus.py index bb360709297..318a3df18e3 100644 --- a/src/evm_transition_tool/nimbus.py +++ b/src/ethereum_clis/clis/nimbus.py @@ -10,7 +10,7 @@ from ethereum_test_forks import Fork -from .transition_tool import TransitionTool +from ..transition_tool import TransitionTool class NimbusTransitionTool(TransitionTool): diff --git a/src/ethereum_clis/ethereum_cli.py b/src/ethereum_clis/ethereum_cli.py new file mode 100644 index 00000000000..350761bd378 --- /dev/null +++ b/src/ethereum_clis/ethereum_cli.py @@ -0,0 +1,155 @@ +""" +Abstract base class to help create Python interfaces to Ethereum CLIs. +""" + +import os +import shutil +import subprocess +from abc import ABC, abstractmethod +from itertools import groupby +from pathlib import Path +from re import Pattern +from typing import Any, List, Optional, Type + + +class UnknownCLI(Exception): + """Exception raised if an unknown CLI is encountered""" + + pass + + +class CLINotFoundInPath(Exception): + """Exception raised if the specified CLI binary is not found in the path""" + + def __init__(self, message="The CLI binary was not found in the path", binary=None): + if binary: + message = f"{message} ({binary})" + super().__init__(message) + + +class EthereumCLI(ABC): + """ + Abstract base class to help create Python interfaces to Ethereum CLIs. + + This base class helps handle the special case of EVM subcommands, such as + the EVM transition tool `t8n`, which have multiple implementations, one + from each client team. In the case of these tools, this class mainly serves + to help instantiate the correct subclass based on the output of the CLI's + version flag. + """ + + registered_tools: List[Type[Any]] = [] + default_tool: Optional[Type[Any]] = None + default_binary: Path + detect_binary_pattern: Pattern + version_flag: str = "-v" + cached_version: Optional[str] = None + + @abstractmethod + def __init__(self, *, binary: Optional[Path] = None, trace: bool = False): + """ + Abstract initialization method that all subclasses must implement. + """ + if binary is None: + binary = self.default_binary + else: + # improve behavior of which by resolving the path: ~/relative paths don't work + 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 CLINotFoundInPath(binary=binary) + self.binary = Path(binary) + self.trace = trace + + @classmethod + def register_tool(cls, tool_subclass: Type[Any]): + """ + Registers a given subclass as tool option. + """ + cls.registered_tools.append(tool_subclass) # raise NotImplementedError + + @classmethod + def set_default_tool(cls, tool_subclass: Type[Any]): + """ + Registers the default tool subclass. + """ + cls.default_tool = tool_subclass + + @classmethod + def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> Any: + """ + Instantiates the appropriate CLI subclass derived from the CLI's `binary_path`. + + This method will attempt to detect the CLI version and instantiate the appropriate + subclass based on the version output by running hte CLI with the version flag. + """ + assert cls.default_tool is not None, "default CLI implementation was never set" + + if binary_path is None: + return cls.default_tool(binary=binary_path, **kwargs) + + resolved_path = Path(os.path.expanduser(binary_path)).resolve() + if resolved_path.exists(): + binary_path = resolved_path + binary = shutil.which(binary_path) # type: ignore + + if not binary: + raise CLINotFoundInPath(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: + result = subprocess.run( + [binary, version_flag], stdout=subprocess.PIPE, stderr=subprocess.PIPE + ) + if result.returncode != 0: + raise Exception(f"Non-zero return code: {result.returncode}") + + if result.stderr: + raise Exception(f"Tool wrote to stderr: {result.stderr.decode()}") + + binary_output = "" + if result.stdout: + binary_output = result.stdout.decode().strip() + 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 UnknownCLI(f"Unknown CLI: {binary_path}") + + @classmethod + def detect_binary(cls, binary_output: str) -> bool: + """ + Returns True if a CLI's `binary_output` matches the class's expected output. + """ + assert cls.detect_binary_pattern is not None + + return cls.detect_binary_pattern.match(binary_output) is not None + + def version(self) -> str: + """ + Returns the name and version of the CLI as reported by the CLI's version flag. + """ + if self.cached_version is None: + result = subprocess.run( + [str(self.binary), self.version_flag], + 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 diff --git a/src/evm_transition_tool/file_utils.py b/src/ethereum_clis/file_utils.py similarity index 100% rename from src/evm_transition_tool/file_utils.py rename to src/ethereum_clis/file_utils.py diff --git a/src/evm_transition_tool/py.typed b/src/ethereum_clis/py.typed similarity index 100% rename from src/evm_transition_tool/py.typed rename to src/ethereum_clis/py.typed diff --git a/src/evm_transition_tool/tests/fixtures/1/alloc.json b/src/ethereum_clis/tests/fixtures/1/alloc.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/1/alloc.json rename to src/ethereum_clis/tests/fixtures/1/alloc.json diff --git a/src/evm_transition_tool/tests/fixtures/1/env.json b/src/ethereum_clis/tests/fixtures/1/env.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/1/env.json rename to src/ethereum_clis/tests/fixtures/1/env.json diff --git a/src/evm_transition_tool/tests/fixtures/1/exp.json b/src/ethereum_clis/tests/fixtures/1/exp.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/1/exp.json rename to src/ethereum_clis/tests/fixtures/1/exp.json diff --git a/src/evm_transition_tool/tests/fixtures/1/txs.json b/src/ethereum_clis/tests/fixtures/1/txs.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/1/txs.json rename to src/ethereum_clis/tests/fixtures/1/txs.json diff --git a/src/evm_transition_tool/tests/fixtures/3/alloc.json b/src/ethereum_clis/tests/fixtures/3/alloc.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/3/alloc.json rename to src/ethereum_clis/tests/fixtures/3/alloc.json diff --git a/src/evm_transition_tool/tests/fixtures/3/env.json b/src/ethereum_clis/tests/fixtures/3/env.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/3/env.json rename to src/ethereum_clis/tests/fixtures/3/env.json diff --git a/src/evm_transition_tool/tests/fixtures/3/exp.json b/src/ethereum_clis/tests/fixtures/3/exp.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/3/exp.json rename to src/ethereum_clis/tests/fixtures/3/exp.json diff --git a/src/evm_transition_tool/tests/fixtures/3/readme.md b/src/ethereum_clis/tests/fixtures/3/readme.md similarity index 100% rename from src/evm_transition_tool/tests/fixtures/3/readme.md rename to src/ethereum_clis/tests/fixtures/3/readme.md diff --git a/src/evm_transition_tool/tests/fixtures/3/txs.json b/src/ethereum_clis/tests/fixtures/3/txs.json similarity index 100% rename from src/evm_transition_tool/tests/fixtures/3/txs.json rename to src/ethereum_clis/tests/fixtures/3/txs.json diff --git a/src/evm_transition_tool/tests/test_evaluate.py b/src/ethereum_clis/tests/test_evaluate.py similarity index 97% rename from src/evm_transition_tool/tests/test_evaluate.py rename to src/ethereum_clis/tests/test_evaluate.py index 33dfb8cb794..b1affe73f6e 100644 --- a/src/evm_transition_tool/tests/test_evaluate.py +++ b/src/ethereum_clis/tests/test_evaluate.py @@ -8,12 +8,12 @@ import pytest from pydantic import TypeAdapter +from ethereum_clis import ExecutionSpecsTransitionTool, TransitionTool from ethereum_test_base_types import to_json from ethereum_test_forks import Berlin, Fork, Istanbul, London from ethereum_test_types import Alloc, Environment, Transaction -from evm_transition_tool import ExecutionSpecsTransitionTool, TransitionTool -FIXTURES_ROOT = Path(os.path.join("src", "evm_transition_tool", "tests", "fixtures")) +FIXTURES_ROOT = Path(os.path.join("src", "ethereum_clis", "tests", "fixtures")) DEFAULT_EVM_T8N_BINARY_NAME = "ethereum-spec-evm-resolver" diff --git a/src/evm_transition_tool/tests/test_transition_tool.py b/src/ethereum_clis/tests/test_transition_tool.py similarity index 91% rename from src/evm_transition_tool/tests/test_transition_tool.py rename to src/ethereum_clis/tests/test_transition_tool.py index df376d52f10..d4d0f92f84e 100644 --- a/src/evm_transition_tool/tests/test_transition_tool.py +++ b/src/ethereum_clis/tests/test_transition_tool.py @@ -9,13 +9,13 @@ import pytest -from evm_transition_tool import ( +from ethereum_clis import ( + CLINotFoundInPath, EvmOneTransitionTool, ExecutionSpecsTransitionTool, GethTransitionTool, NimbusTransitionTool, TransitionTool, - TransitionToolNotFoundInPath, ) @@ -86,8 +86,8 @@ def mock_run(args, **kwargs): def test_unknown_binary_path(): """ - Test that `from_binary_path` raises `UnknownTransitionTool` for unknown + Test that `from_binary_path` raises `UnknownCLI` for unknown binary paths. """ - with pytest.raises(TransitionToolNotFoundInPath): + with pytest.raises(CLINotFoundInPath): TransitionTool.from_binary_path(binary_path=Path("unknown_binary_path")) diff --git a/src/evm_transition_tool/transition_tool.py b/src/ethereum_clis/transition_tool.py similarity index 79% rename from src/evm_transition_tool/transition_tool.py rename to src/ethereum_clis/transition_tool.py index b86b67fe2f1..58ec2a429a4 100644 --- a/src/evm_transition_tool/transition_tool.py +++ b/src/ethereum_clis/transition_tool.py @@ -10,9 +10,7 @@ import textwrap from abc import abstractmethod from dataclasses import dataclass, field -from itertools import groupby from pathlib import Path -from re import Pattern from typing import Dict, List, Mapping, Optional, Type from requests_unixsocket import Session # type: ignore @@ -21,29 +19,14 @@ from ethereum_test_forks import Fork from ethereum_test_types import Alloc, Environment, Transaction +from .ethereum_cli import EthereumCLI from .file_utils import dump_files_to_directory, write_json_file from .types import TransactionReceipt, TransitionToolInput, TransitionToolOutput - -class UnknownTransitionTool(Exception): - """Exception raised if an unknown t8n is encountered""" - - pass - - -class TransitionToolNotFoundInPath(Exception): - """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: - message = f"{message} ({binary})" - super().__init__(message) - - model_dump_config: Mapping = {"by_alias": True, "exclude_none": True} -class TransitionTool(FixtureVerifier): +class TransitionTool(EthereumCLI, FixtureVerifier): """ Transition tool abstract base class which should be inherited by all transition tool implementations. @@ -53,9 +36,7 @@ class TransitionTool(FixtureVerifier): registered_tools: List[Type["TransitionTool"]] = [] default_tool: Optional[Type["TransitionTool"]] = None - default_binary: Path - detect_binary_pattern: Pattern - version_flag: str = "-v" + t8n_subcommand: Optional[str] = None statetest_subcommand: Optional[str] = None blocktest_subcommand: Optional[str] = None @@ -66,8 +47,6 @@ class TransitionTool(FixtureVerifier): server_url: str process: Optional[subprocess.Popen] = None - # Abstract methods that each tool must implement - @abstractmethod def __init__( self, @@ -78,17 +57,7 @@ def __init__( """ Abstract initialization method that all subclasses must implement. """ - if binary is None: - binary = self.default_binary - else: - # improve behavior of which by resolving the path: ~/relative paths don't work - 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) + super().__init__(binary=binary) self.trace = trace def __init_subclass__(cls): @@ -97,95 +66,6 @@ def __init_subclass__(cls): """ 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[Path], **kwargs) -> "TransitionTool": - """ - 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(binary=binary_path, **kwargs) - - resolved_path = Path(os.path.expanduser(binary_path)).resolve() - if resolved_path.exists(): - binary_path = resolved_path - binary = shutil.which(binary_path) # type: ignore - - 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: - result = subprocess.run( - [binary, version_flag], stdout=subprocess.PIPE, stderr=subprocess.PIPE - ) - if result.returncode != 0: - raise Exception(f"Non-zero return code: {result.returncode}") - - if result.stderr: - raise Exception(f"Tool wrote to stderr: {result.stderr.decode()}") - - binary_output = "" - if result.stdout: - binary_output = result.stdout.decode().strip() - 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}") - - @classmethod - def detect_binary(cls, binary_output: str) -> bool: - """ - Returns True if the binary matches the tool - """ - assert cls.detect_binary_pattern is not None - - return cls.detect_binary_pattern.match(binary_output) is not None - - def version(self) -> str: - """ - Return name and version of tool used to state transition - """ - if self.cached_version is None: - result = subprocess.run( - [str(self.binary), self.version_flag], - 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 - @abstractmethod def is_fork_supported(self, fork: Fork) -> bool: """ diff --git a/src/evm_transition_tool/types.py b/src/ethereum_clis/types.py similarity index 100% rename from src/evm_transition_tool/types.py rename to src/ethereum_clis/types.py diff --git a/src/ethereum_test_specs/base.py b/src/ethereum_test_specs/base.py index a79bf6e8ab2..8c020e1e274 100644 --- a/src/ethereum_test_specs/base.py +++ b/src/ethereum_test_specs/base.py @@ -12,11 +12,11 @@ import pytest from pydantic import BaseModel, Field +from ethereum_clis import Result, TransitionTool from ethereum_test_base_types import to_hex from ethereum_test_fixtures import BaseFixture, FixtureFormat from ethereum_test_forks import Fork from ethereum_test_types import Environment, Transaction, Withdrawal -from evm_transition_tool import Result, TransitionTool class HashMismatchException(Exception): diff --git a/src/ethereum_test_specs/blockchain.py b/src/ethereum_test_specs/blockchain.py index 1e62ef70516..3d732bda437 100644 --- a/src/ethereum_test_specs/blockchain.py +++ b/src/ethereum_test_specs/blockchain.py @@ -8,6 +8,7 @@ import pytest from pydantic import ConfigDict, Field, field_validator +from ethereum_clis import TransitionTool from ethereum_test_base_types import ( Address, Bloom, @@ -53,7 +54,6 @@ Withdrawal, WithdrawalRequest, ) -from evm_transition_tool import TransitionTool from .base import BaseTest, verify_result, verify_transactions from .debugging import print_traces @@ -234,11 +234,11 @@ class Block(Header): rlp_modifier: Header | None = None """ An RLP modifying header which values would be used to override the ones - returned by the `evm_transition_tool`. + returned by the `ethereum_clis.TransitionTool`. """ - exception: List[ - TransactionException | BlockException - ] | TransactionException | BlockException | None = None + exception: ( + List[TransactionException | BlockException] | TransactionException | BlockException | None + ) = None """ If set, the block is expected to be rejected by the client. """ @@ -370,13 +370,13 @@ def make_genesis( base_fee_per_gas=env.base_fee_per_gas, blob_gas_used=env.blob_gas_used, excess_blob_gas=env.excess_blob_gas, - withdrawals_root=Withdrawal.list_root(env.withdrawals) - if env.withdrawals is not None - else None, + withdrawals_root=( + Withdrawal.list_root(env.withdrawals) if env.withdrawals is not None else None + ), parent_beacon_block_root=env.parent_beacon_block_root, - requests_root=Requests(root=[]).trie_root - if fork.header_requests_required(0, 0) - else None, + requests_root=( + Requests(root=[]).trie_root if fork.header_requests_required(0, 0) else None + ), fork=fork, ) @@ -574,27 +574,35 @@ def make_fixture( header=header, txs=[FixtureTransaction.from_transaction(tx) for tx in txs], ommers=[], - withdrawals=[FixtureWithdrawal.from_withdrawal(w) for w in new_env.withdrawals] - if new_env.withdrawals is not None - else None, - deposit_requests=[ - FixtureDepositRequest.from_deposit_request(d) - for d in requests.deposit_requests() - ] - if requests is not None - else None, - withdrawal_requests=[ - FixtureWithdrawalRequest.from_withdrawal_request(w) - for w in requests.withdrawal_requests() - ] - if requests is not None - else None, - consolidation_requests=[ - FixtureConsolidationRequest.from_consolidation_request(c) - for c in requests.consolidation_requests() - ] - if requests is not None - else None, + withdrawals=( + [FixtureWithdrawal.from_withdrawal(w) for w in new_env.withdrawals] + if new_env.withdrawals is not None + else None + ), + deposit_requests=( + [ + FixtureDepositRequest.from_deposit_request(d) + for d in requests.deposit_requests() + ] + if requests is not None + else None + ), + withdrawal_requests=( + [ + FixtureWithdrawalRequest.from_withdrawal_request(w) + for w in requests.withdrawal_requests() + ] + if requests is not None + else None + ), + consolidation_requests=( + [ + FixtureConsolidationRequest.from_consolidation_request(c) + for c in requests.consolidation_requests() + ] + if requests is not None + else None + ), fork=fork, ).with_rlp(txs=txs, requests=requests) if block.exception is None: diff --git a/src/ethereum_test_specs/eof.py b/src/ethereum_test_specs/eof.py index e251eff6f6b..924e6670506 100644 --- a/src/ethereum_test_specs/eof.py +++ b/src/ethereum_test_specs/eof.py @@ -12,6 +12,7 @@ import pytest from pydantic import Field, model_validator +from ethereum_clis import TransitionTool from ethereum_test_base_types import Account, Bytes from ethereum_test_exceptions import EvmoneExceptionMapper from ethereum_test_exceptions.exceptions import EOFExceptionInstanceOrList, to_pipe_str @@ -27,7 +28,6 @@ from ethereum_test_forks import Fork from ethereum_test_types import Alloc, Environment, Transaction from ethereum_test_types.eof.v1 import Container, ContainerKind -from evm_transition_tool import TransitionTool from .base import BaseTest from .state import StateTest diff --git a/src/ethereum_test_specs/state.py b/src/ethereum_test_specs/state.py index 2252393253b..84f30dda74e 100644 --- a/src/ethereum_test_specs/state.py +++ b/src/ethereum_test_specs/state.py @@ -6,6 +6,7 @@ import pytest +from ethereum_clis import TransitionTool from ethereum_test_exceptions import EngineAPIError from ethereum_test_fixtures import ( BaseFixture, @@ -22,7 +23,6 @@ ) from ethereum_test_forks import Fork from ethereum_test_types import Alloc, Environment, Transaction -from evm_transition_tool import TransitionTool from .base import BaseTest from .blockchain import Block, BlockchainTest, Header diff --git a/src/ethereum_test_specs/tests/test_expect.py b/src/ethereum_test_specs/tests/test_expect.py index 36ee92f132d..2e530da2c63 100644 --- a/src/ethereum_test_specs/tests/test_expect.py +++ b/src/ethereum_test_specs/tests/test_expect.py @@ -6,11 +6,11 @@ import pytest +from ethereum_clis import ExecutionSpecsTransitionTool from ethereum_test_base_types import Account, Address from ethereum_test_fixtures import StateFixture from ethereum_test_forks import Fork, get_deployed_forks from ethereum_test_types import Alloc, Environment, Storage, Transaction -from evm_transition_tool import ExecutionSpecsTransitionTool from ..state import StateTest diff --git a/src/ethereum_test_specs/tests/test_fixtures.py b/src/ethereum_test_specs/tests/test_fixtures.py index de2b1d9cde3..551386c2f25 100644 --- a/src/ethereum_test_specs/tests/test_fixtures.py +++ b/src/ethereum_test_specs/tests/test_fixtures.py @@ -10,6 +10,7 @@ from click.testing import CliRunner import cli.check_fixtures +from ethereum_clis import ExecutionSpecsTransitionTool from ethereum_test_base_types import Account, Address, Hash from ethereum_test_exceptions import TransactionException from ethereum_test_fixtures import ( @@ -22,7 +23,6 @@ from ethereum_test_forks import Berlin, Fork, Istanbul, London, Paris, Shanghai from ethereum_test_types import Alloc, Environment, Transaction from ethereum_test_vm import Opcodes as Op -from evm_transition_tool import ExecutionSpecsTransitionTool from ..blockchain import Block, BlockchainTest, Header from ..state import StateTest diff --git a/src/ethereum_test_tools/tests/test_code.py b/src/ethereum_test_tools/tests/test_code.py index 18fd71bec20..72531c510ff 100644 --- a/src/ethereum_test_tools/tests/test_code.py +++ b/src/ethereum_test_tools/tests/test_code.py @@ -8,6 +8,7 @@ import pytest from semver import Version +from ethereum_clis import ExecutionSpecsTransitionTool from ethereum_test_base_types import Account, Address, Bytes, Hash, TestAddress, TestPrivateKey from ethereum_test_fixtures import BlockchainFixture from ethereum_test_forks import ( @@ -22,7 +23,6 @@ from ethereum_test_types import Alloc, Environment, Transaction from ethereum_test_vm import Opcodes as Op from ethereum_test_vm import UndefinedOpcodes -from evm_transition_tool import ExecutionSpecsTransitionTool from ..code import CalldataCase, Case, Conditional, Initcode, Solc, Switch, Yul from .conftest import SOLC_PADDING_VERSION diff --git a/src/pytest_plugins/consume/direct/conftest.py b/src/pytest_plugins/consume/direct/conftest.py index 997adc3a367..894202082e2 100644 --- a/src/pytest_plugins/consume/direct/conftest.py +++ b/src/pytest_plugins/consume/direct/conftest.py @@ -12,10 +12,10 @@ import pytest +from ethereum_clis import TransitionTool from ethereum_test_base_types import to_json from ethereum_test_fixtures.consume import TestCaseIndexFile, TestCaseStream from ethereum_test_fixtures.file import Fixtures -from evm_transition_tool import TransitionTool def pytest_addoption(parser): # noqa: D103 diff --git a/src/pytest_plugins/consume/direct/test_via_direct.py b/src/pytest_plugins/consume/direct/test_via_direct.py index 0bdfe137316..98aec5ac5d5 100644 --- a/src/pytest_plugins/consume/direct/test_via_direct.py +++ b/src/pytest_plugins/consume/direct/test_via_direct.py @@ -9,9 +9,9 @@ import pytest +from ethereum_clis import TransitionTool from ethereum_test_fixtures import BlockchainFixture, StateFixture from ethereum_test_fixtures.consume import TestCaseIndexFile, TestCaseStream -from evm_transition_tool import TransitionTool from ..decorator import fixture_format diff --git a/src/pytest_plugins/filler/filler.py b/src/pytest_plugins/filler/filler.py index 0ad5a7a3cc9..3af694a87eb 100644 --- a/src/pytest_plugins/filler/filler.py +++ b/src/pytest_plugins/filler/filler.py @@ -5,6 +5,7 @@ and that modifies pytest hooks in order to fill test specs for all tests and writes the generated fixtures to file. """ + import configparser import datetime import os @@ -18,6 +19,7 @@ from pytest_metadata.plugin import metadata_key # type: ignore from cli.gen_index import generate_fixtures_index +from ethereum_clis import TransitionTool from ethereum_test_base_types import Alloc, ReferenceSpec from ethereum_test_fixtures import FIXTURE_FORMATS, BaseFixture, FixtureCollector, TestInfo from ethereum_test_forks import ( @@ -31,7 +33,6 @@ generate_github_url, get_current_commit_hash_or_tag, ) -from evm_transition_tool import TransitionTool from pytest_plugins.spec_version_checker.spec_version_checker import EIPSpecTestItem diff --git a/src/pytest_plugins/filler/tests/test_filler.py b/src/pytest_plugins/filler/tests/test_filler.py index 413e60223ca..5dcbed330aa 100644 --- a/src/pytest_plugins/filler/tests/test_filler.py +++ b/src/pytest_plugins/filler/tests/test_filler.py @@ -11,7 +11,7 @@ import pytest -from evm_transition_tool import ExecutionSpecsTransitionTool, TransitionTool +from ethereum_clis import ExecutionSpecsTransitionTool, TransitionTool from pytest_plugins.filler.filler import default_output_directory diff --git a/src/pytest_plugins/forks/forks.py b/src/pytest_plugins/forks/forks.py index 85fe8bcfcf9..f42ccdca4cc 100644 --- a/src/pytest_plugins/forks/forks.py +++ b/src/pytest_plugins/forks/forks.py @@ -13,6 +13,7 @@ from _pytest.mark.structures import ParameterSet from pytest import Metafunc +from ethereum_clis import TransitionTool from ethereum_test_forks import ( Fork, ForkAttribute, @@ -24,7 +25,6 @@ get_transition_forks, transition_fork_to, ) -from evm_transition_tool import TransitionTool def pytest_addoption(parser): diff --git a/tests/shanghai/eip4895_withdrawals/test_withdrawals.py b/tests/shanghai/eip4895_withdrawals/test_withdrawals.py index 2db4a119a66..7fdc82cb1ed 100644 --- a/tests/shanghai/eip4895_withdrawals/test_withdrawals.py +++ b/tests/shanghai/eip4895_withdrawals/test_withdrawals.py @@ -9,6 +9,7 @@ import pytest +from ethereum_clis import TransitionTool from ethereum_test_forks import Cancun, Fork from ethereum_test_tools import ( EOA, @@ -23,7 +24,6 @@ Withdrawal, ) from ethereum_test_tools.vm.opcode import Opcodes as Op -from evm_transition_tool import TransitionTool from .spec import ref_spec_4895 diff --git a/whitelist.txt b/whitelist.txt index 577dea3a1ed..b3b0a689a79 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -69,6 +69,8 @@ changelog chfast classdict cli +clis +CLIs cli2 cmd codeAddr