ethereum / consensus-spec-tests

Common tests for the Ethereum proof-of-stake consensus layer
MIT License
69 stars 22 forks source link

Fix `deneb` `include_attestation_from_previous_fork_with_new_range` test #35

Closed ethDreamer closed 11 months ago

ethDreamer commented 12 months ago

In the latest release (v1.4.0-beta.0), there's a new test called include_attestation_from_previous_fork_with_new_range meant to ensure the new attestation inclusion rules under EIP-7045 apply to attestations from the final CAPELLA epoch. This is the only generated test which makes use of the @spec_configured_state_test override.

This override emits a config.yaml file in the generated test directory. There are several problems with this:

1) The sanity test definition does not say to expect a config.yaml 2) The generated config.yaml is invalid:

# this is the mainnet example
ALTAIR_FORK_EPOCH: 74240
...
BELLATRIX_FORK_EPOCH: 144896
...
CAPELLA_FORK_EPOCH: 194048
...
DENEB_FORK_EPOCH: 2

3) The generated config.yaml is missing the following fields which (without modification) prevent lighthouse from ingesting it:

hwwhww commented 12 months ago

Hey @ethDreamer, thank you for reporting it!

I opened a PR to fix this test case: https://github.com/ethereum/consensus-specs/pull/3464

hwwhww commented 11 months ago

closing via https://github.com/ethereum/consensus-specs/pull/3464