Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for terminal block during proposer preparation #3058

Closed
paulhauner opened this issue Mar 3, 2022 · 2 comments
Closed

Check for terminal block during proposer preparation #3058

paulhauner opened this issue Mar 3, 2022 · 2 comments
Assignees
Labels
bellatrix Required to support the Bellatrix Upgrade

Comments

@paulhauner
Copy link
Member

paulhauner commented Mar 3, 2022

Description

The merge transition block is the first post-Bellatrix block to include a non-default ExecutionPayload. The execution_payload.parent_hash value is computed here:

let terminal_pow_block_hash = execution_layer
.get_terminal_pow_block_hash(spec)
.await
.map_err(BlockProductionError::TerminalPoWBlockLookupFailed)?;

According to the validator guide, we're supposed to call forkchoiceUpdated (fcU) before producing that block with the head_block_hash set to the value returned from ExecutionLayer::get_terminal_pow_block_hash.

In #3043, I've added the functionality to give the EL a "heads-up" for payload production via a PayloadAttributes value in a fcU call. However, I haven't handled the edge-case of the transition block.

To handle the edge case, I think we need to call ExecutionLayer::get_terminal_pow_block_hash in BeaconChain::prepare_proposer_async whenever we're post-Bellatrix but the head block does not have execution enabled. If we get a terminal pow block hash, then we should use that value as the head_block_root rather than the value that's presently on the head block.

We might actually be able to do it a bit cleaner, where the call to get_terminal_pow_block_hash is done in BeaconChain::update_execution_engine_forkchoice_async. I'm not sure what the best implementation is, yet.

Importantly, with our current implementation, we'll just fail to give advance notice of the terminal block. However, we'll still produce a block since we'll do a valid fcU call immediately before we try to produce the transition payload.

@paulhauner paulhauner added the bellatrix Required to support the Bellatrix Upgrade label Mar 3, 2022
@ethDreamer
Copy link
Member

I'll take this one

@ethDreamer
Copy link
Member

Diagram of execution flow for reference
FCU_Flow

@ethDreamer ethDreamer self-assigned this Mar 16, 2022
bors bot pushed a commit that referenced this issue Apr 7, 2022
## Issue Addressed

- #3058 

Co-authored-by: Paul Hauner <[email protected]>
paulhauner added a commit to paulhauner/lighthouse that referenced this issue May 6, 2022
## Issue Addressed

- sigp#3058 

Co-authored-by: Paul Hauner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bellatrix Required to support the Bellatrix Upgrade
Projects
None yet
Development

No branches or pull requests

2 participants