consensus-specs icon indicating copy to clipboard operation
consensus-specs copied to clipboard

Add additional `get_proposer_head` test coverage

Open eserilev opened this issue 2 years ago • 3 comments

Currently clients can potentially pass all get_proposer_head spec tests without having a fully spec compliant implementation. This PR attempts to improve get_proposer_head test coverage. A new test_parent_weak test is added to check for the following scenario:

  • is_head_late, is_shuffling_stable, is_shuffling_stable, is_finalization_ok, is_proposing_on_time is_head_weak all evaluate to true
  • is_parent_strong evaluates to false

In this situation, the parent block doesn't have enough attestation votes and therefore the head root is returned from get_proposer_head. This new test is pretty much a copy paste of test_basic_is_parent_root with a few minor tweaks.

There are several more tests needed to further improve test coverage. This is my first time writing a consensus spec test so I'm looking for some feedback before writing additional tests.

eserilev avatar Mar 25 '24 14:03 eserilev

Thank you @eserilev! Have you run this new test vector with Lighthouse?

hwwhww avatar Mar 28 '24 23:03 hwwhww

@hwwhww yes I have. On Lighthouse unstable this new test vector fails, but on this branch the test passes

eserilev avatar Mar 29 '24 10:03 eserilev

@hwwhww The test passes on LH unstable now so we are happy to merge if you are! Thanks

michaelsproul avatar May 28 '24 06:05 michaelsproul

@hwwhww I think this test case is a nice to have, let me know what you think

eserilev avatar Nov 02 '24 06:11 eserilev

closing as stale

eserilev avatar Jun 02 '25 22:06 eserilev