Use justified blocks execution payload hash as safeBlockHash in engine fcU #4080
Conversation
| @@ -284,8 +285,9 @@ async function maybeIssueNextProposerEngineFcU( | |||
| const proposerIndex = prepareState.epochCtx.getBeaconProposer(prepareSlot); | |||
| const feeRecipient = chain.beaconProposerCache.get(proposerIndex); | |||
| if (feeRecipient) { | |||
| const safeBlockHash = chain.forkChoice.getJustifiedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX; | |||
There was a problem hiding this comment.
I prefer to have a separate method like this in forkchoice
def get_safe_execution_payload_hash(store: Store) -> Hash32:
safe_block_root = get_safe_beacon_block_root(store)
safe_block = store.blocks[safe_block_root]
# Return Hash32() if no payload is yet justified
if compute_epoch_at_slot(safe_block.slot) >= BELLATRIX_FORK_EPOCH:
return safe_block.body.execution_payload.block_hash
else:
return Hash32()and use it everywhere to make it clear
There was a problem hiding this comment.
ummm justified iproto block in forkchoice will only have executionPayloadBlockHash if its post merge i.e. >= BELLATRIX_FORK_EPOCH.
It is similar to the way we extract finalizedBlockHash, I could add comments for better clarity if you think that would help understanding whats going on? 🤔
There was a problem hiding this comment.
if you think safeBlockHash() will not changed in the future then we're good 👍
There was a problem hiding this comment.
yes its more or less fixed, its the EL counterpart of the justified block, which they can use to tag, prune, throw reorg errors etc may be, earlier it headBlockHash was used as a stub for it, which the ELs were totally ignoring
The specs (v1.2.0 rc) have been updated on what to pass as
safeBlockHashin engine fcU call (https://github.com/ethereum/consensus-specs/pull/2876/files, https://github.com/ethereum/consensus-specs/pull/2851/files, https://github.com/ethereum/consensus-specs/pull/2858/files)Motivation
This PR implements the same in lodestar and does some comment cleanup.