diff --git a/.gitignore b/.gitignore index fd08eaf32b..a7df4ce5e2 100644 --- a/.gitignore +++ b/.gitignore @@ -47,7 +47,6 @@ pip-delete-this-directory.txt .tox/ /doc/_autosummary -/docs .coverage .coverage.* @@ -60,3 +59,4 @@ tests/fixtures tests/t8n_testdata fixtures +logs/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5cb1e62462..56917c0a9f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,6 +26,8 @@ This specification aims to be: - Avoid using EIP numbers in identifiers. - If necessary, there is a custom dictionary `whitelist.txt`. +See [docs/STYLE_GUIDE.md](docs/STYLE_GUIDE.md) for rules not covered by Black (naming, ensure/assert, and typing ignore policy). + ### Changes across various Forks Many contributions require changes across multiple forks, organized under `src/ethereum/*`. When making such changes, please ensure that differences between the forks are minimal and consist only of necessary differences. This will help with getting cleaner [diff outputs](https://ethereum.github.io/execution-specs/diffs/index.html). diff --git a/docs/STYLE_GUIDE.md b/docs/STYLE_GUIDE.md new file mode 100644 index 0000000000..131f184878 --- /dev/null +++ b/docs/STYLE_GUIDE.md @@ -0,0 +1,111 @@ +# EELS Style Guide + +This document outlines style conventions for the Ethereum Execution Layer Specifications (EELS) that are not automatically enforced by Black formatter. These rules promote code clarity, consistency, and maintainability across the codebase. + +## Function Naming + +Function names should be **short, imperative verbs** in `snake_case` format. Follow [PEP 8](https://peps.python.org/pep-0008/#function-and-variable-names) naming conventions. + +**Good:** +```python +def fetch_state(address: Address) -> Account: + """Fetch account state from storage.""" + +def validate_block(block: Block) -> None: + """Validate block structure and contents.""" + +def compute_hash(data: bytes) -> Hash32: + """Compute hash of input data.""" +``` + +**Avoid:** +```python +def do_fetch(address: Address) -> Account: # Avoid "do_" prefixes +def get_the_state_data(address: Address) -> Account: # Too verbose +def validateBlock(block: Block) -> None: # Use snake_case, not camelCase +``` + +## Error Handling + +Use **exceptions** for protocol-handled errors. Avoid sentinel return values for error conditions. + +**Good:** +```python +def execute_transaction(tx: Transaction) -> Receipt: + if tx.gas_limit < INTRINSIC_GAS: + raise InsufficientGas("Gas limit too low") + # ... execution logic + return receipt + +def withdraw_funds(account: Account, amount: Uint256) -> None: + if account.balance < amount: + raise InsufficientFunds(f"Balance {account.balance} < {amount}") + # ... withdrawal logic +``` + +**Avoid:** +```python +def execute_transaction(tx: Transaction) -> Optional[Receipt]: + if tx.gas_limit < INTRINSIC_GAS: + return None # Don't use None for errors + # ... execution logic + return receipt +``` + +## Runtime Checks + +- Use `ensure(...)` for runtime condition validation +- Use `assert` **only** for: + - Invariants and type assumptions + - Bug detection and debugging + - **Never** for control flow or user-facing validation + +**Good:** +```python +def transfer(sender: Account, recipient: Account, amount: Uint256) -> None: + ensure(sender.balance >= amount, "Insufficient balance") # Runtime validation + ensure(amount > 0, "Transfer amount must be positive") # Runtime validation + + # Internal invariant checks + assert isinstance(sender.balance, Uint256) # Type assumption + assert sender.address != recipient.address # Invariant check +``` + +**Avoid:** +```python +def transfer(sender: Account, recipient: Account, amount: Uint256) -> None: + assert sender.balance >= amount # Don't use assert for user validation + assert amount > 0 # Don't use assert for control flow +``` + +## Type Annotations and Ignores + +- **Never** use blanket `# type: ignore` comments +- If type ignoring is absolutely necessary: + - Include the specific error code: `# type: ignore[error-code]` + - Add a brief one-line comment explaining why + +**Good:** +```python +# Specific error code with explanation +result = unsafe_cast(data) # type: ignore[attr-defined] # Legacy API compatibility + +# Type checker limitation workaround +value = getattr(obj, name, default) # type: ignore[misc] # Dynamic attribute access +``` + +**Avoid:** +```python +result = some_function() # type: ignore # Too broad, no explanation +data = process(input) # type: ignore # No error code specified +``` + +## Enforcement + +These style rules are enforced through: + +- **mypy** with strict configuration for type checking and ignore validation +- **Ruff** linting rules (including PGH003 for blanket type ignore detection) +- Code review processes + +The CI pipeline will reject code that violates these guidelines. \ No newline at end of file diff --git a/mypy.ini b/mypy.ini index 62ad9c2438..a19f3cb194 100644 --- a/mypy.ini +++ b/mypy.ini @@ -9,5 +9,7 @@ strict_bytes = True warn_unused_ignores = True warn_unused_configs = True warn_redundant_casts = True +show_error_codes = True +enable_error_code = ignore-without-code ignore_missing_imports = False diff --git a/pyproject.toml b/pyproject.toml index 69eb8378b2..2c8ba14459 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -441,6 +441,7 @@ select = [ "D", # pydocstyle "C4", # flake8-comprehensions "ARG", # flake8-unused-arguments + "PGH003", # blanket-type-ignore ] fixable = [ "E", # pycodestyle errors diff --git a/src/ethereum_spec_tools/docc.py b/src/ethereum_spec_tools/docc.py index e934ea79df..28e7c0f9dc 100644 --- a/src/ethereum_spec_tools/docc.py +++ b/src/ethereum_spec_tools/docc.py @@ -51,8 +51,12 @@ from docc.transform import Transform from fladrif.apply import Apply from fladrif.treediff import Adapter, Operation, TreeMatcher -from mistletoe import block_token as blocks # type: ignore -from mistletoe import span_token as spans +from mistletoe import ( # type: ignore[import-untyped] # Third-party library without types + block_token as blocks, +) +from mistletoe import ( + span_token as spans, +) from typing_extensions import assert_never, override from .forks import Hardfork diff --git a/src/ethereum_spec_tools/forks.py b/src/ethereum_spec_tools/forks.py index 9be8f2317c..e0f97e19e7 100644 --- a/src/ethereum_spec_tools/forks.py +++ b/src/ethereum_spec_tools/forks.py @@ -130,7 +130,7 @@ def load(cls: Type[H], config_dict: Dict["ForkCriteria", str]) -> List[H]: for criteria, name in config: mod = importlib.import_module("ethereum." + name) - mod.FORK_CRITERIA = criteria # type: ignore + mod.FORK_CRITERIA = criteria # type: ignore[attr-defined] # Dynamic module attribute assignment forks.append(cls(mod)) return forks