Guard get_parent_payload_status at fork boundary#5125
Conversation
|
Please review the proposed fix, it breaks post Gloas forks as it can be seen in the failed action: |
get_parent_payload_status at fork boundary
Thanks for the review @brech1. I made a change now. |
|
Reading the discussion in ethereum/beacon-APIs#590 (comment) I'm uncertain on the consensus on what status to pin these blocks to. cc @nflaig |
not sure what the best solution is on the beacon-api but shouldn't the spec just use |
| parent = store.blocks[block.parent_root] | ||
| parent_state = store.block_states[block.parent_root] | ||
| # pre-Gloas parent has no bid field hence, treat as PENDING | ||
| if parent_state.fork.current_version < GLOAS_FORK_VERSION: |
There was a problem hiding this comment.
| if parent_state.fork.current_version < GLOAS_FORK_VERSION: | |
| if compute_epoch_at_slot(block.slot) < GLOAS_FORK_EPOCH: |
There was a problem hiding this comment.
The issue with this suggestion is that our testing framework doesn't work well with it 😢 currently, GLOAS_FORK_EPOCH is FAR_FUTURE_EPOCH so this condition will always be true. And it will always be true in the minimal preset, where we never schedule fork epochs.
get_parent_payload_status(and thereforeis_parent_node_full) was unconditionally accessing the Gloas-only fieldparent.body.signed_execution_payload_bideven for pre-Gloas blocks.This PR adds a guard that checks
parent_state.fork.current_versionand treats any pre-Gloas parent asPAYLOAD_STATUS_PENDING(matching the behavior Lighthouse already implements).No changes were needed to
on_blockoris_parent_node_full.All Gloas tests pass with
make test fork=gloas.Fixes #5096