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

Remove test_consolidation.py since they are not valid test vectors #3736

Merged
merged 1 commit into from
May 3, 2024

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Apr 30, 2024

Apologized. Leftover of #3724.

Reported by @tersec:

https://github.com/ethereum/consensus-spec-tests/tree/v1.5.0-alpha.1/tests/minimal/electra/sanity/blocks/pyspec_tests/multiple_consolidations_above_churn~ has no pre-state
Also, and caveat, I've not verified this one as closely, but it happens before any actual block processing, so it'd have to be an SSZ reading issue if it is a Nimbus issue at all, the Electra minimal block sanity multiple_consolidations_below_churn, multiple_consolidations_equal_churn, and multiple_consolidations_equal_twice_churn all fail for Nimbus's test fixture on the verify first block, before being able to apply it, on (Nimbus's equivalent of)

It turns out that the original test script used set_eth1_withdrawal_credential_with_balance to manipulate the pre state in the for loop, so it can't be the valid test vectors.

@fradamt once suggested that it's OK to delete these tests. This PR removes these test cases.

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

works for me, we can revisit proper testing over the coming months

@hwwhww hwwhww merged commit 331f1e9 into dev May 3, 2024
28 checks passed
@hwwhww hwwhww deleted the hotfix-consolidation branch May 3, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants