Skip to content

Conversation

LouisTsai-Csie
Copy link
Collaborator

@LouisTsai-Csie LouisTsai-Csie commented Jun 14, 2025

πŸ—’οΈ Description

Adding benchmark test for the P256VERIFY precompile contract.

πŸ”— Related Issues

Issue: #1734

βœ… Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • 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.

@LouisTsai-Csie LouisTsai-Csie force-pushed the zkevm-p256verify-precompile branch from 4fc2f8d to 15c891d Compare June 16, 2025 09:41
@LouisTsai-Csie LouisTsai-Csie self-assigned this Jun 16, 2025
@LouisTsai-Csie LouisTsai-Csie added feature:zkevm scope:tests Scope: Changes EL client test cases in `./tests` labels Jun 16, 2025
@LouisTsai-Csie LouisTsai-Csie marked this pull request as ready for review June 16, 2025 09:46
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Excellent, just one comment overall.

],
id="bls12_fp_to_g2",
),
pytest.param(
Copy link
Member

Choose a reason for hiding this comment

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

Test is going to pass but the test is only going to test empty account calls because there's no precompile here prior to Prague.

We should test whether precompile_address is in fork.precompiles(), and if not just pytest.skip("Precompile not enabled") at the start of the function.

@LouisTsai-Csie LouisTsai-Csie force-pushed the zkevm-p256verify-precompile branch from 15c891d to 1247180 Compare June 17, 2025 09:05
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for these :)

@marioevz
Copy link
Member

I checked the coverage lost and it makes sense: We are no longer calling empty accounts (the disabled precompiles in prior forks).

@marioevz marioevz merged commit f7ad3a1 into ethereum:main Jun 17, 2025
14 of 15 checks passed
@spencer-tb spencer-tb changed the title feat(zkevm): add p256verify to worst compute tests feat(benchmark): add p256verify to worst compute tests Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature:benchmark scope:tests Scope: Changes EL client test cases in `./tests`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants