Description
Fork choice's on_block implementation currently includes this code:
|
# Check if this blocks builds on empty or full parent block |
|
parent_block = store.blocks[block.parent_root] |
|
bid = block.body.signed_execution_payload_bid.message |
|
parent_bid = parent_block.body.signed_execution_payload_bid.message |
|
# Make a copy of the state to avoid mutability issues |
|
if is_parent_node_full(store, block): |
|
assert block.parent_root in store.payload_states |
|
state = copy(store.payload_states[block.parent_root]) |
|
else: |
|
assert bid.parent_block_hash == parent_bid.parent_block_hash |
|
state = copy(store.block_states[block.parent_root]) |
This snippet doesn't make sense if parent_block is a pre-Gloas block because it accesses the Gloas-only field parent.body.signed_execution_payload_bid. I believe this represents a problem at the Gloas fork boundary where the parent is expected to be pre-Gloas.
The function is_parent_node_full called in on_block has the same problem, via its call to get_parent_payload_status:
|
def get_parent_payload_status(store: Store, block: BeaconBlock) -> PayloadStatus: |
|
parent = store.blocks[block.parent_root] |
|
parent_block_hash = block.body.signed_execution_payload_bid.message.parent_block_hash |
|
message_block_hash = parent.body.signed_execution_payload_bid.message.block_hash |
|
return PAYLOAD_STATUS_FULL if parent_block_hash == message_block_hash else PAYLOAD_STATUS_EMPTY |
Proposed Fix
Clients are likely already handling this "correctly", because we have working devnets, but there is the potential for inconsistencies and bugs whenever the spec underspecifies. For example, in Lighthouse I came to the conclusion that I should treat all pre-Gloas blocks as PENDING for the sake of fork choice, but Lodestar treats them as FULL: ethereum/beacon-APIs#590 (comment).
In the case of on_block (and is_parent_node_full), I think pre-Gloas blocks should take the same codepath as PENDING Gloas blocks, as there is no parent payload envelope to check against.
From a testing PoV, I don't think any of the current spec tests cover fork choice over multiple forks? A simple cross-fork test would catch this issue and others like it.
Additional Info
This issue is not solved by deferring payload execution, although any fix will likely touch the same parts of on_block:
Description
Fork choice's
on_blockimplementation currently includes this code:consensus-specs/specs/gloas/fork-choice.md
Lines 743 to 753 in 4d94b9e
This snippet doesn't make sense if
parent_blockis a pre-Gloas block because it accesses the Gloas-only fieldparent.body.signed_execution_payload_bid. I believe this represents a problem at the Gloas fork boundary where the parent is expected to be pre-Gloas.The function
is_parent_node_fullcalled inon_blockhas the same problem, via its call toget_parent_payload_status:consensus-specs/specs/gloas/fork-choice.md
Lines 263 to 267 in 4d94b9e
Proposed Fix
Clients are likely already handling this "correctly", because we have working devnets, but there is the potential for inconsistencies and bugs whenever the spec underspecifies. For example, in Lighthouse I came to the conclusion that I should treat all pre-Gloas blocks as
PENDINGfor the sake of fork choice, but Lodestar treats them asFULL: ethereum/beacon-APIs#590 (comment).In the case of
on_block(andis_parent_node_full), I think pre-Gloas blocks should take the same codepath asPENDINGGloas blocks, as there is no parent payload envelope to check against.From a testing PoV, I don't think any of the current spec tests cover fork choice over multiple forks? A simple cross-fork test would catch this issue and others like it.
Additional Info
This issue is not solved by deferring payload execution, although any fix will likely touch the same parts of
on_block: