ethereum / ethereum/execution-apis
Clarify and limit `engine_forkchoiceUpdated` validation semantics for `safeBlockHash`
- 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
Assessment
This issue has not been assessed yet.