Skip to content

feat(tests): add test "framework" for BN254 precompiles#1871

Merged
danceratopz merged 3 commits intoethereum:forks/amsterdamfrom
ipsilon:tests/bn254
Dec 16, 2025
Merged

feat(tests): add test "framework" for BN254 precompiles#1871
danceratopz merged 3 commits intoethereum:forks/amsterdamfrom
ipsilon:tests/bn254

Conversation

@chfast
Copy link
Copy Markdown
Member

@chfast chfast commented Dec 9, 2025

🗒️ Description

Add test "framework" for BN254 precompiles: spec.py with some constants and conftest.py with generic fixtures.

🔗 Related Issues or PRs

#1860

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx tox -e static
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered adding an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).
  • 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.
  • Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

🐛

@chfast
Copy link
Copy Markdown
Member Author

chfast commented Dec 9, 2025

I mostly copied the conftest.py from BLS precompiles tests. This works nicely here too because it uses features from Byzantium. So you may want to extract this as a tool for any other precompile testing. However, it will not work for eariler precompiles without modifications.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.26%. Comparing base (dca59f6) to head (590d4a9).
⚠️ Report is 2 commits behind head on forks/amsterdam.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           forks/amsterdam    #1871      +/-   ##
===================================================
+ Coverage            86.15%   86.26%   +0.10%     
===================================================
  Files                  538      538              
  Lines                34557    34557              
  Branches              3222     3222              
===================================================
+ Hits                 29773    29809      +36     
+ Misses                4177     4161      -16     
+ Partials               607      587      -20     
Flag Coverage Δ
unittests 86.26% <ø> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

SamWilsn pushed a commit that referenced this pull request Dec 9, 2025
* type(tests): convert create suicide during init

* Fixes, increase coverage

* fix: Coverage script

* Add coveraged missed reason

---------

Co-authored-by: Mario Vega <marioevz@gmail.com>
@danceratopz danceratopz changed the base branch from forks/osaka to forks/amsterdam December 16, 2025 10:57
Copy link
Copy Markdown
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chfast, very nice!

Re-using the existing BLS pytest fixtures is a good idea. I think we should move these to a central place now in the scope of this PR.

I see two options:

  1. Add them to a new module tests/common/precompile_fixtures.py and import them as required from there - this enables explicit local use of these fixtures and avoids defining them globally for all tests.
  2. Add them to a new fill plugin that is loaded by default (via fill's pytest ini file) and globally scoped.

Although it's a new pattern, I prefer 1. to avoid polluting the test session with fixtures that intended to have a limited scope. Having these fixtures available globally increases the risk of error in unrelated tests that use fixtures that (accidentally) have the same names (the implementer could forget to shadow one or more of the fixtures, leading to unexpected behavior). We could consider using more ambiguous naming, but that's no absolute guarantee that they won't get used by mistake.

If you're happy with 1. Please add it to this PR and state briefly in the new module which type of precompiles could make use of it and why earlier precompiles can't use it. Thanks!

Copy link
Copy Markdown
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! One minor nit!

@@ -0,0 +1,198 @@
"""pytest fixtures for tests executing precompiled contracts."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please improve the docstring here?

state briefly in the new module which type of precompiles could make use of it and why earlier precompiles can't use it. Thanks!

Copy link
Copy Markdown
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@danceratopz danceratopz merged commit d0ce4d7 into ethereum:forks/amsterdam Dec 16, 2025
11 checks passed
@danceratopz danceratopz changed the title enhance(tests): add test "framework" for BN254 precompiles feat(tests): add test "framework" for BN254 precompiles Dec 16, 2025
CPerezz pushed a commit to CPerezz/execution-specs that referenced this pull request Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants