refactor(pytest,fill): removed solc dependency (#1759) #1779

felix314159 · 2025-06-20T09:14:35Z

🗒️ Description

See issue #1759. This PR is still WIP

🔗 Related Issues

✅ Checklist

All: Set appropriate labels for the changes.
All: Considered squashing commits to improve commit history.
All: Added an entry to CHANGELOG.md.
All: Considered updating the online docs in the ./docs/ directory.
Tests: All converted JSON/YML tests from ethereum/tests/tests/static have been assigned @ported_from marker.
Tests: A PR with removal of converted JSON/YML blockchain tests from ethereum/tests have been opened.
Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.

…n in path and use that one if it is 0.8.24 instead of downloading it which does not work on arm

danceratopz

Thanks for tackling this @felix314159!

Just a general remark, I prefer smaller PRs and would have preferred if the test refactor from solc to Opcodes was a separate PR. In this case the task list in the issue could have guided you on this path. But we all do it (I'm guilty of it in #1765; I should have done a preliminary refactor PR first).

I think I wasn't aware that the static tests require solc (it's obvious in hindsight, but I thought they only required lllc).

danceratopz

Ups, was trigger happy with the "Submit Review" button and submitted before I was finished. Here's the rest of the review :)

Docs also need to remove all references to solc-select/yul (installation, installation troubleshooting, contributing?).

danceratopz · 2025-06-23T11:17:46Z

docs/CHANGELOG.md

@@ -26,6 +26,7 @@ Users can select any of the artifacts depending on their testing needs for their
 #### 🔀 Refactoring

 - 🔀 Move `TransactionType` enum from test file to proper module location in `ethereum_test_types.transaction_types` for better code organization and reusability.
+- 🔀 Remove dependency `solc-select` and in CI instead fetch solc 0.8.24 from the official GitHub release. Code changes required involved porting a few Python tests that made use of Yul to our Opcode language ([#1779](https://github.com/ethereum/execution-spec-tests/pull/1779)).


Could you clarify the extent of how much we've removed the solc dependency?

As I understand, this PR removes the dep of solc to fill Python tests, right? But it is still required for the static tests? Which is generally only performed by EEST maintainers or CI). Therefore, we can remove solc-select as a regular test implementer need not be concerned with solc at all. Is this correct?

Looking at the pytest ini files, this is the case. I think the CHANGELOG needs more detailed explanation to help power users understand the changes in this PR.

I think it needs to be much clearer (probably worth a note at the top of the release section in the CHANGELOG) that test implementers should no longer use Yul source in their Python test code and that the Yul and YulCompiler objects are no longer available to do this. Not a breaking change in fixture formats, but a breaking change in test implementing standards :)

Yes exactly, good point I will clarify this in the changelog.

danceratopz · 2025-06-23T11:20:30Z

docs/writing_tests/tutorials/state_transition.md

@@ -18,7 +18,7 @@ The most effective method of learning how to write tests is to study a couple of

 ### Yul Test

-You can find the source code for the Yul test in [tests/homestead/yul/test_yul_example.py](../../tests/homestead/yul/test_yul_example/index.md).
+You can find the source code for the Yul test in `tests/homestead/yul/test_yul_example.py`.


This file no longer exists. We should remove any reference to it in the documentation and ensure that we don't lead test implementers to use Yul in the Python test code.

danceratopz · 2025-06-23T11:20:46Z

pyproject.toml

@@ -50,6 +49,7 @@ dependencies = [
    "pytest-regex>=0.2.0,<0.3",
    "eth-abi>=5.2.0",
    "joblib>=1.4.2",
+    "ckzg>=2.1.1,<3",


Did this unintentionally sneak in here?

Yes sorry, it's from the pending blob pr

danceratopz · 2025-06-23T11:38:30Z

tests/cancun/eip6780_selfdestruct/test_selfdestruct_revert.py

+    #   case 1: selfdestruct_on_outer_call=1
+    #   case 2: selfdestruct_on_outer_call=2
+    #   case 3: selfdestruct_on_outer_call has a different value
+    common_prefix = (


I guess hasher is of no use as the bytecode will not be same. Did or can you diff traces to help verify the port?

I did not diff traces, and I think we need to figure out some kind of way to prove (or disprove) that the new code is functionally equivalent to the Yul code we had before

danceratopz · 2025-06-23T11:39:23Z

tests/homestead/yul/test_yul_example.py

@@ -1,56 +0,0 @@
-"""Test Yul Source Code Examples."""


danceratopz · 2025-06-23T11:40:19Z

tox.ini

 python_source_dirs = src tests .github/scripts

 [testenv:lint]
 description = Lint and code formatting checks (ruff)
 extras = lint
 commands =
    ruff check --no-fix --show-fixes {[testenv]python_source_dirs}
-    ruff format --diff {[testenv]python_source_dirs}
+    ruff format --check {[testenv]python_source_dirs} # use check instead of diff to avoid red print of 'x files already formatted'


Sneaky. Should be a separate PR imo.

AFACK (as far as claude knows) neither --check nor --diff modify files, it's just a matter of whether you see proposed changes or not (apparently with the side-effect of in which color the output is printed). I agree that this change does not have anything to do with solc but it is a low impact change that imo does not deserve its own pr. Printing in danger-red that your codebase is perfectly formatted is very distracting.

I feel like a generally hard problem is to decide when sth deserves its own PR. Both statements are true:

It is easier to revert a PR in case of issues occuring later when it does only one thing

Each PR adds overhead (requires reviews, requires switching between branches, you lose track of all in-progress PRs more quickly, etc)
Maybe we can figure out a set of rules so that we can more objectively rule when sth requires its own pr and when it does not

danceratopz · 2025-06-23T12:07:33Z

tox.ini

+    # Create bin directory
+    mkdir -p {envdir}/bin
+    # if solc 0.8.24 exists already we are done, otherwise download it and verify version equals 0.8.24 (will fail on linux-arm64) 
+    sh -c 'echo "Checking for system solc..."; \


So we still need solc for the framework tests? This will affect regular test implementers (that want to run tox locally). I'd like to find a way to mitigate that. I mean, how does this help ARM users? They copy their custom compiled solc to tox's venv? What's the flow here?

Won't we need to do this for CI releases, too?

This is too heavy for tox.ini imo. Can we add a helper script (preferably in Python), it can be a function in ./src/cli/tox_helpers.py. This could also be used by

I only saw that CI failed until I gave it access to the solc binary. It shouldn't be needed tbh (we don't fill static tests after every push, right?). Maybe we can simplify the tox.ini.

To elaborate what this code currently does: If you have solc version 0.8.24 in your path, then it will not download solc. I figured out a way to re-use your solc binary even in the isolated tox env. So this means it works on arm even now, if you have the correct version (0.8.24) and it is in your path. If you do not have solc or a different version then it will fetch the binary from official sources.

removed solc dependency (ethereum#1759)

c31efcc

felix314159 force-pushed the solc-dep-removed branch from b4a1242 to c31efcc Compare June 20, 2025 13:07

felix314159 added 2 commits June 20, 2025 13:45

improved tox solc checker to be able to find locally installed versio…

54bc906

…n in path and use that one if it is 0.8.24 instead of downloading it which does not work on arm

added changelog entry

67c1bd7

danceratopz reviewed Jun 23, 2025

View reviewed changes

refactor(pytest,fill): removed solc dependency (#1759) #1779

Are you sure you want to change the base?

refactor(pytest,fill): removed solc dependency (#1759) #1779

Uh oh!

Conversation

felix314159 commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

🔗 Related Issues

✅ Checklist

Uh oh!

danceratopz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danceratopz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix314159 Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

felix314159 commented Jun 20, 2025 •

edited

Loading

danceratopz left a comment •

edited

Loading

felix314159 Jun 23, 2025 •

edited

Loading