specs: Make deneb function is_valid_light_client_header idiomatic #4118
Conversation
…t is idiomatic and coverage friendly.
bshastry
force-pushed
the
deneb-spec-lc-header
branch
from
3c2529e to
40798de
Compare
| if epoch < DENEB_FORK_EPOCH: | ||
| if header.execution.blob_gas_used != uint64(0) or header.execution.excess_blob_gas != uint64(0): | ||
| return False | ||
| if epoch < config.DENEB_FORK_EPOCH: | ||
| return ( | ||
| header.execution.blob_gas_used == uint64(0) | ||
| and header.execution.excess_blob_gas == uint64(0) | ||
| ) |
There was a problem hiding this comment.
I don't think this change is correct. It doesn't behave the same. For pre-Deneb, with unset blob gas fields, it will no longer check if the execution branch is valid. It will simply return true now.
There was a problem hiding this comment.
Yes, the refactor is wrong, and CI also fails.
|
@etan-status, do you mind taking a look at this? |
Thanks for the ping, agree with your observation that the change is incorrect. |
|
Sorry, the change is wrong. My apologies. The original intention was to write a test to cover the line that would return False in the case the blob fields pre deneb are non zero. I'm not sure it is possible to write such a test. Because the fields don't exist pre deneb let alone them being non zero. |
|
Closing this PR. |
refactor: improve light client header validation flow
Change validation logic in
is_valid_light_client_headerto use early return pattern for pre-Deneb blob gas checks. This improves code readability while maintaining identical validation behavior. It increases coverage by one SLoC and decreases partially covered branches by one, for deneb and beyond.