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

validate EL block hash when running consensus block tests #6406

Open
wants to merge 15 commits into
base: unstable
Choose a base branch
from

Conversation

etan-status
Copy link
Contributor

We currently don't have an easy way to test EL block hash computation. As the EL block hash in consensus-spec-tests is computed correctly, update the test runners that load block from test files to also verify the EL block hash. This increases missing test coverage.

Requires ethereum/consensus-specs#3829

We currently don't have an easy way to test EL block hash computation.
As the EL block hash in consensus-spec-tests is computed correctly,
update the test runners that load block from test files to also verify
the EL block hash. This increases missing test coverage.

Requires ethereum/consensus-specs#3829
Copy link

github-actions bot commented Jul 3, 2024

Unit Test Results

0 files   -        12  0 suites   - 1 814   0s ⏱️ - 54m 43s
0 tests  -   5 290  0 ✔️  -   4 942  0 💤  - 348  0 ±0 
0 runs   - 29 425  0 ✔️  - 28 973  0 💤  - 452  0 ±0 

Results for commit c424901. ± Comparison against base commit a461bb1.

♻️ This comment has been updated with latest results.

@etan-status
Copy link
Contributor Author

Cross-checked against jtraglia/consensus-specs#2 (with #6718)

@etan-status
Copy link
Contributor Author

The hash is incorrect when there are empty txes, see status-im/nim-eth#744 (comment)

As such blocks are invalid anyway as of ethereum/consensus-specs#3885, have excluded that condition from block hash consistency check.

@etan-status
Copy link
Contributor Author

alpha.9 --> used to work but new tests got added after the latest fixes got merged.

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.

1 participant