Skip to content

Conversation

@manuelmauro
Copy link
Contributor

Description

Adds support for EIP-7623

@RomarQ
Copy link
Contributor

RomarQ commented Sep 5, 2025

For running the tests for pectra, you will need to add Fork::Pectra => Config::pectra() in

let mut config = match test.fork {

If possibly only running the tests for EIP-7623

@sorpaas
Copy link
Member

sorpaas commented Sep 5, 2025

If possibly only running the tests for EIP-7623

Not sure if this is possible. It passed everything so I didn't yet added any skip_tests functionality.
As long as it doesn't break any past tests, we can wait till all Pectra is implemented and then test them together.

@manuelmauro manuelmauro marked this pull request as ready for review September 5, 2025 15:17
@manuelmauro
Copy link
Contributor Author

For running the tests for pectra, you will need to add Fork::Pectra => Config::pectra() in

let mut config = match test.fork {

If possibly only running the tests for EIP-7623

Added in c1f288a

Fork::London => Config::london(),
Fork::Shanghai => Config::shanghai(),
Fork::Cancun => Config::cancun(),
Fork::Prague => Config::prague(),
Copy link
Member

Choose a reason for hiding this comment

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

Hmm why is this tests passing?

Copy link
Contributor Author

@manuelmauro manuelmauro Sep 8, 2025

Choose a reason for hiding this comment

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

Could it be because the EIP-7702 testing is mostly implemented in this framework https://github.com/ethereum/execution-spec-tests (rather then https://github.com/ethereum/tests)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the https://github.com/ethereum/tests/tree/428f218d7d6f4a52544e12684afbfe6e2882ffbf submodule is pointing to a commit from 2 years ago.

Copy link
Member

@sorpaas sorpaas Sep 8, 2025

Choose a reason for hiding this comment

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

Also the https://github.com/ethereum/tests/tree/428f218d7d6f4a52544e12684afbfe6e2882ffbf submodule is pointing to a commit from 2 years ago.

Yeah this is expected. At some point last year, legacy tests are removed from ethereum/tests and replaced by legacytests. Then EIP-7610 is implemented (for all hard forks). The tests of oldethtests and legacytests are actualy redundant to each other. legacytests is simply a newer version of oldethtests, with the only difference of whether it implements EIP-7610.

Mainnet clients don't care, but we still want to support non-EIP-7610 situations, so we still keep oldethtests with a commit from 2 years ago.

@manuelmauro
Copy link
Contributor Author

For running the tests for pectra, you will need to add Fork::Pectra => Config::pectra() in

let mut config = match test.fork {

If possibly only running the tests for EIP-7623

Added in c1f288a

Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

Added some remarks, we should use saturating math operations

Paris,
Berlin,
Cancun,
Prague,
Copy link
Member

Choose a reason for hiding this comment

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

Ah I think I know -- The commit of legacytests doesn't yet contain any Prague tests. It's up to the last hard fork.

To test Prague we need to update legacytests or add ethereum-spec-tests.

As said in previous comment, on the other hand, oldethtests is used only for EIP-7610 and it should never be updated.

@sorpaas sorpaas added this pull request to the merge queue Sep 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 8, 2025
@sorpaas sorpaas added this pull request to the merge queue Sep 8, 2025
@RomarQ
Copy link
Contributor

RomarQ commented Sep 8, 2025

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 8, 2025
@sorpaas
Copy link
Member

sorpaas commented Sep 8, 2025

@manuelmauro Please pull master and then we can merge.

@sorpaas sorpaas added this pull request to the merge queue Sep 9, 2025
Merged via the queue into rust-ethereum:master with commit 9017c0e Sep 9, 2025
4 checks passed
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.

3 participants