-
Notifications
You must be signed in to change notification settings - Fork 344
docs: update CONTRIBUTING.md for uv
and ruff
#1396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: forks/osaka
Are you sure you want to change the base?
Changes from all commits
72bd744
006d0f6
c52604c
42bc272
aba2239
d07de8d
980aa82
5ecd344
7396ac7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,14 +4,12 @@ Help is always welcome and there are plenty of options to contribute to the Ethe | |||||
|
||||||
In particular, we appreciate support in the following areas: | ||||||
|
||||||
- Reporting issues | ||||||
- Reporting issues. | ||||||
- Fixing and responding to [issues](https://github.com/ethereum/execution-specs/issues), especially those tagged as [E-easy](https://github.com/ethereum/execution-specs/labels/E-easy) which are meant as introductory issues for external contributors. | ||||||
- Improving the documentation. | ||||||
|
||||||
|
||||||
For details about EELS usage and building, please refer the [README](https://github.com/ethereum/execution-specs/blob/master/README.md#usage) | ||||||
|
||||||
|
||||||
## Contribution Guidelines | ||||||
|
||||||
This specification aims to be: | ||||||
|
@@ -24,7 +22,7 @@ This specification aims to be: | |||||
|
||||||
- Attempt to use descriptive English words (or _very common_ abbreviations) in documentation and identifiers. | ||||||
- Avoid using EIP numbers in identifiers. | ||||||
- If necessary, there is a custom dictionary `whitelist.txt`. | ||||||
- If necessary, there is a custom dictionary `whitelist.txt`. | ||||||
|
||||||
### Changes across various Forks | ||||||
|
||||||
|
@@ -36,131 +34,217 @@ When creating pull requests affecting multiple forks, we recommended submitting | |||||
2. Apply the changes across the other forks, push them, and mark the pull request as ready for review. | ||||||
|
||||||
This saves you having to apply code review feedback repeatedly for each fork. | ||||||
|
||||||
### Development | ||||||
|
||||||
Running the tests necessary to merge into the repository requires: | ||||||
|
||||||
* Python 3.11.x, and | ||||||
* [PyPy](https://www.pypy.org/) [7.3.19](https://downloads.python.org/pypy/) or later. | ||||||
* `geth` installed and present in `$PATH` | ||||||
- [`uv`](https://docs.astral.sh/uv/) package manager, | ||||||
- Python 3.11.x, | ||||||
- [PyPy](https://www.pypy.org/) [7.3.19](https://downloads.python.org/pypy/) or later (typically only required by execution-specs maintainers), | ||||||
- `geth` installed and present in `$PATH` (only required by users who would like to sync and verify mainnet blocks using EELS; not required for typical specification or test development). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
|
||||||
#### Installing `uv` | ||||||
|
||||||
`uv` can be installed via `curl`, some package managers or `pip`. It is recommended to use `curl` or a package manager to enable easier updates (`curl`-installs can run `uv self update`) and install and manage Python installations. See `uv`'s [installation docs](https://docs.astral.sh/uv/getting-started/installation/) for package manager support and other information. | ||||||
|
||||||
```bash | ||||||
curl -LsSf https://astral.sh/uv/install.sh | sh | ||||||
``` | ||||||
|
||||||
Carsons-Eels marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
#### Installing Python and PyPy with `uv` | ||||||
|
||||||
`uv` can be used to install the required version of Python, if not already available: | ||||||
|
||||||
```bash | ||||||
uv python install 3.11 | ||||||
uv python pin 3.11 | ||||||
``` | ||||||
|
||||||
For [PyPy](ttps://www.pypy.org/) (this is typically only required by execution-specs maintainers): | ||||||
|
||||||
```bash | ||||||
uv python install pypy-3.11.13-linux-x86_64-gnu | ||||||
``` | ||||||
|
||||||
or pick the most recent PyPy 3.11 version from this list: | ||||||
|
||||||
```bash | ||||||
uv python list | ||||||
``` | ||||||
|
||||||
#### Development Setup | ||||||
|
||||||
Clone the repository and set up your development environment: | ||||||
|
||||||
```bash | ||||||
git clone https://github.com/ethereum/execution-specs.git | ||||||
cd execution-specs | ||||||
uv python install 3.11 | ||||||
uv python pin 3.11 | ||||||
uv sync --all-extras | ||||||
``` | ||||||
|
||||||
The repository includes several tox environments for different testing scenarios: | ||||||
|
||||||
- `static` - Linting, type checking and spell checking. | ||||||
- `json_infra` - Run specific execution spec test releases. | ||||||
- `py3` - Run the local version of execution spec tests using CPython. | ||||||
- `pypy3` - Run the local version of execution spec tests using PyPy. | ||||||
|
||||||
It is recommended to run the `static` checks locally before pushing changes to a pull request: | ||||||
|
||||||
`execution-specs` depends on a submodule that contains common tests that are run across all clients, so we need to clone the repo with the --recursive flag. Example: | ||||||
```bash | ||||||
$ git clone --recursive https://github.com/ethereum/execution-specs.git | ||||||
uvx tox -e static # Run linting and type checking. | ||||||
``` | ||||||
|
||||||
Or, if you've already cloned the repository, you can fetch the submodules with: | ||||||
All environments can be executed using (very slow, not recommended, even in parallel): | ||||||
|
||||||
```bash | ||||||
$ git submodule update --init --recursive | ||||||
uvx tox | ||||||
``` | ||||||
Comment on lines
+95
to
105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should tell people not to run the full |
||||||
|
||||||
The tests can be run with: | ||||||
Environments can be executed in parallel with (still very slow): | ||||||
|
||||||
```bash | ||||||
$ tox | ||||||
uvx tox run-parallel | ||||||
``` | ||||||
Comment on lines
+107
to
111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a safe recommendation? The PyPy environment OOM'd in CI with 32GiB of memory. |
||||||
|
||||||
The development tools can also be run outside of `tox`, and can automatically reformat the code: | ||||||
#### Development Tools | ||||||
|
||||||
The development tools can also be run directly with `uv`, which handles the virtual environment automatically: | ||||||
|
||||||
```bash | ||||||
$ pip install -e ".[doc,lint,test,fill]" # Installs ethereum, and development tools. | ||||||
$ isort src # Organizes imports. | ||||||
$ black src # Formats code. | ||||||
$ flake8 # Reports style/spelling/documentation errors. | ||||||
$ mypy src # Verifies type annotations. | ||||||
$ pytest -n 4 # Runs tests parallelly. | ||||||
$ pytest -m "not slow" # Runs tests which execute quickly. | ||||||
uv run codespell -I whitelist.txt {tox_root}/src --skip '{tox_root}/src/ethereum_spec_tests' # Spellcheck source | ||||||
uv run ruff check src --exclude src/ethereum_spec_tests # Linting | ||||||
uv run mypy src --exclude "src/ethereum_spec_tests/*" --namespace-packages # Static type checking | ||||||
``` | ||||||
|
||||||
It is recommended to use a [virtual environment](https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/#creating-a-virtual-environment) to keep your system Python installation clean. | ||||||
#### EEST / Spec-Tests | ||||||
|
||||||
A subset of the spec-tests (specified via a substring match to `-k`) can be run directly via: | ||||||
|
||||||
A trace of the EVM execution for any test case can be obtained by providing the `--evm-trace` argument to pytest. | ||||||
```bash | ||||||
uv run ethereum-spec-fill tests/eest --clean --fork Osaka -k eip7939 | ||||||
``` | ||||||
|
||||||
#### Spec Unit Tests | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I might be missing the point of the subsection? |
||||||
|
||||||
A trace of the EVM execution for any unit test case can be obtained by providing the `--evm-trace` argument to pytest. | ||||||
Note: Make sure to run the EVM trace on a small number of tests at a time. The log might otherwise get very big. | ||||||
Below is an example. | ||||||
|
||||||
```bash | ||||||
pytest tests/frontier/test_state_transition.py -k 'test_general_state_tests_new' --evm-trace | ||||||
uv run pytest tests/frontier/test_state_transition.py -k 'test_general_state_tests_new' --evm-trace | ||||||
gurukamath marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
``` | ||||||
|
||||||
## Code Standards | ||||||
|
||||||
The codebase follows specific formatting and linting standards enforced by `ruff` (which combines similar functionality to that of `black`, `isort`, and `flake8`): | ||||||
|
||||||
- **Line Length**: 79 characters maximum. | ||||||
- **Formatting**: Enforced by `ruff format` (similar to `black`). | ||||||
- **Import Sorting**: Handled by `ruff check` (similar to `isort`). | ||||||
- **Linting**: Code quality checks via `ruff check`. | ||||||
- **Type Checking**: Enforced by `mypy` for type annotations. | ||||||
- **Spell Checking**: Documentation and comments checked by `codespell`. | ||||||
|
||||||
All these standards are automatically checked in CI via the `static` tox environment (see above). | ||||||
|
||||||
## Integration with execution-spec-tests | ||||||
|
||||||
**Note: This integration is a work in progress (WIP).** | ||||||
|
||||||
The execution-specs repository has integrated code from the [ethereum/execution-spec-tests](https://github.com/ethereum/execution-spec-tests) repository using git subtrees. This integration includes: | ||||||
|
||||||
- **`src/ethereum_spec_tests/`** - The test framework, CLI tools, and utilities from execution-spec-tests, | ||||||
- **`tests/eest/`** - The actual EVM test cases from execution-spec-tests. | ||||||
|
||||||
### Important: Read-Only Subtree Directories | ||||||
|
||||||
The subtree directories (`src/ethereum_spec_tests/` and `tests/eest/`) should be treated as **read-only** in this repository. Any changes to code in these directories should be made via pull requests to the upstream [ethereum/execution-spec-tests](https://github.com/ethereum/execution-spec-tests) repository, not directly in execution-specs. | ||||||
|
||||||
If you need to modify test framework code or test cases, please: | ||||||
|
||||||
1. Fork and submit PRs to [ethereum/execution-spec-tests](https://github.com/ethereum/execution-spec-tests). | ||||||
2. Once merged upstream, the subtrees in execution-specs will be updated accordingly. | ||||||
|
||||||
## CLI Utilities `ethereum_spec_tools` | ||||||
|
||||||
The EELS repository has various CLI utilities that can help in the development process. | ||||||
|
||||||
### New Fork Tool | ||||||
----------------- | ||||||
|
||||||
This tool can be used to create the base code for a new fork by using the existing code from a given fork. | ||||||
|
||||||
The command takes 4 arguments, 2 of which are optional | ||||||
* from_fork: The fork name from which the code is to be duplicated. Eg. - "Tangerine Whistle" | ||||||
* to_fork: The fork name of the new fork Eg - "Spurious Dragon" | ||||||
* from_test (Optional): Name of the from fork within the test fixtures in case it is different from fork name. Eg. - "EIP150" | ||||||
* to_test (Optional): Name of the to fork within the test fixtures in case it is different from fork name Eg - "EIP158" | ||||||
|
||||||
- from_fork: The fork name from which the code is to be duplicated. Eg. - "Tangerine Whistle" | ||||||
- to_fork: The fork name of the new fork Eg - "Spurious Dragon" | ||||||
- from_test (Optional): Name of the from fork within the test fixtures in case it is different from fork name. Eg. - "EIP150" | ||||||
- to_test (Optional): Name of the to fork within the test fixtures in case it is different from fork name Eg - "EIP158" | ||||||
|
||||||
As an example, if one wants to create baseline code for the `Spurious Dragon` fork from the `Tangerine Whistle` one | ||||||
|
||||||
```bash | ||||||
ethereum-spec-new-fork --from_fork="Tangerine Whistle" --to_fork="Spurious Dragon" --from_test=EIP150 --to_test=EIP158 | ||||||
uv run ethereum-spec-new-fork --from_fork="Tangerine Whistle" --to_fork="Spurious Dragon" --from_test=EIP150 --to_test=EIP158 | ||||||
``` | ||||||
|
||||||
The following will have to however, be updated manually | ||||||
|
||||||
1. The fork number and `MAINNET_FORK_BLOCK` in `__init__.py`. If you are proposing a new EIP, please set `MAINNET_FORK_BLOCK` to `None`. | ||||||
2. Any absolute package imports from other forks eg. in `trie.py` | ||||||
3. Package names under `setup.cfg` | ||||||
4. Add the new fork to the `monkey_patch()` function in `src/ethereum_optimized/__init__.py` | ||||||
5. Adjust the underline in `fork/__init__.py` | ||||||
Comment on lines
176
to
199
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This'll all get replaced when #1372 is merged. |
||||||
|
||||||
|
||||||
### Sync Tool | ||||||
------------- | ||||||
|
||||||
The sync tool allows one to use an RPC provider to fetch and validate blocks against EELS. | ||||||
The state can also be stored in a local DB after validation. Since syncing directly with the specs can be | ||||||
very slow, one can also leverage the optimized module. This contains alternative implementations of routines | ||||||
in EELS that have been optimized for speed rather than clarity/readability. | ||||||
|
||||||
The tool can be called using the `uv run ethereum-spec-sync` command which takes the following arguments | ||||||
|
||||||
The tool can be called using the `ethereum-spec-sync` command which takes the following arguments | ||||||
* rpc-url: Endpoint providing the Ethereum RPC API. Defaults to `http://localhost:8545/` | ||||||
* unoptimized: Don't use the optimized state/ethash (this can be extremely slow) | ||||||
* persist: Store the state in a db in this file | ||||||
* geth: Use geth specific RPC endpoints while fetching blocks | ||||||
* reset: Delete the db and start from scratch | ||||||
* gas-per-commit: Commit to db each time this much gas is consumed. Defaults to 1_000_000_000 | ||||||
* initial-state: Start from the state in this db, rather than genesis | ||||||
* stop-at: After syncing this block, exit successfully | ||||||
- rpc-url: Endpoint providing the Ethereum RPC API. Defaults to `http://localhost:8545/` | ||||||
- unoptimized: Don't use the optimized state/ethash (this can be extremely slow) | ||||||
- persist: Store the state in a db in this file | ||||||
- geth: Use geth specific RPC endpoints while fetching blocks | ||||||
- reset: Delete the db and start from scratch | ||||||
- gas-per-commit: Commit to db each time this much gas is consumed. Defaults to 1_000_000_000 | ||||||
- initial-state: Start from the state in this db, rather than genesis | ||||||
- stop-at: After syncing this block, exit successfully | ||||||
|
||||||
- The following options are not supported WITH `--unoptimized` -> `--persist`, `--initial-state`, `--reset` | ||||||
- The following options are not supported WITHOUT `--persist` -> `--initial_state`, `--reset` | ||||||
|
||||||
|
||||||
### Patch Tool | ||||||
-------------- | ||||||
|
||||||
This tool can be used to apply the unstaged changes in `SOURCE_FORK` to each of the `TARGET_FORKS`. If some | ||||||
of the change didn't apply, '.rej' files listing the unapplied changes will be left in the `TARGET_FORK`. | ||||||
|
||||||
|
||||||
The tool takes the following command line arguments | ||||||
* The fork name where the changes have been made. Eg:- `frontier` (only a single fork name) | ||||||
* The fork names where the changes have to be applied. Eg:- `homestead` (multiple values can be provided separated by space) | ||||||
* optimized: Patch the optimized code instead | ||||||
* tests: Patch the tests instead | ||||||
|
||||||
- The fork name where the changes have been made. Eg:- `frontier` (only a single fork name) | ||||||
- The fork names where the changes have to be applied. Eg:- `homestead` (multiple values can be provided separated by space) | ||||||
- optimized: Patch the optimized code instead | ||||||
- tests: Patch the tests instead | ||||||
|
||||||
As an example, if one wants to apply changes made in `Frontier` fork to `Homestead` and `Tangerine Whistle` | ||||||
|
||||||
```bash | ||||||
python src/ethereum_spec_tools/patch_tool.py frontier homestead tangerine_whistle | ||||||
uv run ethereum-spec-patch frontier homestead tangerine_whistle | ||||||
``` | ||||||
|
||||||
### Lint Tool | ||||||
------------- | ||||||
|
||||||
This tool checks for style and formatting issues specific to EELS and emits diagnostics | ||||||
when issues are found | ||||||
|
||||||
The tool currently performs the following checks | ||||||
|
||||||
- The order of the identifiers between each hardfork is consistent. | ||||||
- Import statements follow the relevant import rules in modules. | ||||||
|
||||||
The command to run the tool is `ethereum-spec-lint` | ||||||
The command to run the tool is `uv run ethereum-spec-lint` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyone running
tox
locally will have to have PyPy installed.