Skip to content
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

EIP-2537: Update test files to latest gas schedule #9217

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jochem-brouwer
Copy link
Member

This PR updates the test files linked in EIP-2537 to reflect:

  • The G1MUL/G2MUL precompiles are removed
  • The gas schedule of (nearly) all precompiles is updated

These files pass the tests on EthereumJS ethereumjs/ethereumjs-monorepo#3807 and the BLS precompiles also pass the latest Pectra tests: https://github.com/ethereum/execution-spec-tests/releases/tag/pectra-devnet-5%40v1.1.0

If another client could double-check that the modified files are correct, that would be great 😄 👍

@eth-bot
Copy link
Collaborator

eth-bot commented Jan 6, 2025

File EIPS/eip-2537.md

Requires 1 more reviewers from @g11tech, @lightclient, @SamWilsn, @xinbenlv

@eth-bot eth-bot added the e-consensus Waiting on editor consensus label Jan 6, 2025
@g11tech
Copy link
Contributor

g11tech commented Jan 8, 2025

@jochem-brouwer could you have any of @marioevz / @lightclient / @MarekM25 (or whoever is best informed on this) give it a review/approve before I merge it?

@jochem-brouwer
Copy link
Member Author

If one of the clients could check if the changed test files match their implementation that would be great 😄 👍

@marioevz
Copy link
Member

marioevz commented Jan 8, 2025

I checked the differences and found a couple of things:

  • The gas in our JSON files was outdated, but this was a no-op because EEST has the formulas built-in and disregarded the value in the JSON.
  • The mul_ files were not deleted from the EEST repository, rather they were redirected to be executed by the MSM precompile.

I updated the JSON files in EEST with the ones in this PR and the generated files were identical.

Nevertheless I will open a PR in EEST to update them just for them to be up to date, although we will keep the mul_ JSON files.

@jochem-brouwer
Copy link
Member Author

It is actually a good point regarding the mul files to redirect those to MSM. I should do so as well, but for clarity I think it is better (in here) to put those inputs/outputs into the MSM file to make it clear it should be ran by that precompile? (Since the mul precompile does not exist anymore)

@jochem-brouwer jochem-brouwer marked this pull request as draft January 9, 2025 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e-consensus Waiting on editor consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants