ethereum / ethereum/execution-apis

Clarify and limit `engine_forkchoiceUpdated` validation semantics for `safeBlockHash`

Open
#548 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Io
Stars
1.1k
Forks
530
Avg merge
5d 8h
Merged PRs (30d)
9

Description

## Concerns

Consider the following excerpt of the `engine_forkchoiceUpdated` specification:

https://github.com/ethereum/execution-apis/blob/03911ffc053b8b806123f1fc237184b0092a485a/src/engine/paris.md?plain=1#L215

> 5. Client software **MUST** update its forkchoice state if payloads referenced by `forkchoiceState.headBlockHash` and `forkchoiceState.finalizedBlockHash` are `VALID`. The update is specified as follows:

**1. Missing `safeBlockHash`:**

This contains no mention of how and if execution layer clients validate `safeBlockHash`.
As such, it implies that the software **MUST** update its forkchoice state *even if the `safeBlockHash` references an `INVALID` block*, which may be unexpected for client implementations.

I appreciate that a consensus layer client is non-compliant if they provide an invalid `safeBlockHash` thanks to this part of its field definition:
> This value **MUST** be either equal to or an ancestor of `headBlockHash`

However, I believe it's safer to also specify consistent behavior for the EL client, and point 6 allows for the possibility of receiving an out-of-chain `safeBlockHash`.

**2. Missing "belong to the same chain" requirement:**

Point 5 only requires that the finalized and head blocks are `VALID`, not that they are in the same chain.
As such, a fully compliant client would need to update its state even if the final block was in a different chain.

While point 6 includes "belong to the same chain", it only defines how the EL client should respond, not how it should update its state.
> 6. Client software **MUST** return `-38002: Invalid forkchoice state` error ... does not belong to the chain defined by `forkchoiceState.headBlockHash`.

## Current client behavior

### Geth
When receiving a `forkChoiceState` with `VALID` `headBlockHash` and `finalizedBlockHash`, but an `INVALID` or out-of-chain `safeBlockHash`, Geth currently handles this by updating the head and finalized blocks in the chain, but leaving the safe block unchanged.

It also requires that the safe and finalized blocks are part of the same chain as

It still returns `-38002: Invalid forkchoice state` as required by point 6, but the state of the head and finalized blocks has been updated.
It does not roll back these updates.

Safe block validation:
https://github.com/ethereum/go-ethereum/blob/7ed52c949e66ca7874ba39e8d69b451615382edc/eth/catalyst/api.go#L345

Same chain validation:
https://github.com/ethereum/go-ethereum/blob/7ed52c949e66ca7874ba39e8d69b451615382edc/eth/catalyst/api.go#L337-L339

## Recommendation
- Clarify point 5 to include whether EL clients are expected to update the forkchoice state when the `safeBlockHash` is INVALID
- Clarify point 5 to include "same chain" requirement
- Clarify whether partial updates to internal fork choice state are allowed

One solution could look like

> 5. Client software **MUST** update its forkchoice state if *and only if* payloads referenced by `forkchoiceState.headBlockHash`, *`forkchoiceState.safeBlockHash`* and `forkchoiceState.finalizedBlockHash` are `VALID` and *belong to the same chain*. The update is specified as follows:

Note that Geth would not currently abide by this more strict requirement (it doesn't handle *and only if* because it partially updates the state)
Maybe a relaxed version is preferred to maintain backwards compatibility?
I could also discuss *and only if* in a separate issue.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.