Engine API: Make SYNCING response and according actions fork specific #152
Conversation
|
|
||
| 4. Client software **MAY** provide additional details on the validation error if the payload is deemed `INVALID` by assigning the corresponding message to the `validationError` field. | ||
| 1. Client software **MUST** validate the payload and respond accordingly despite of the sync process being in progress if there is enough data for the payload validation. |
There was a problem hiding this comment.
Might consider a clarifying example
"(e.g. if branch A is SYNCING but a block is inserted the head of branch B which is VALID, this block must be validated)"
| 2. All updates to the forkchoice state resulting from this call **MUST** be made atomically. | ||
| 1. All updates to the forkchoice state resulting from this call **MUST** be made atomically. | ||
|
|
||
| 1. Client software **MUST** return `{status: SUCCESS, payloadId: null}` if `payloadAttributes` is `null`. |
There was a problem hiding this comment.
We should clarify that this isn't returned no matter what if payloadAttribues is null but only if null and successful
| 1. Client software **MUST** return `{status: SUCCESS, payloadId: null}` if `payloadAttributes` is `null`. | |
| 1. Client software **MUST** return `{status: SUCCESS, payloadId: null}` if `payloadAttributes` is `null` and fork choice update is successful. |
There was a problem hiding this comment.
Actually, maybe we should reorder this statement with the next one.
{SYNCING, null}if not enough info to process{SUCCESS, null}if no payload and success{SUCCESS, id}if payload and success
There was a problem hiding this comment.
That way it acts a blocking condition on the next two success conditions
|
|
||
| 3. Client software **MUST** return `{status: SUCCESS, payloadId: null}` if `payloadAttributes` is `null` and the client is not `SYNCING`. | ||
| 1. Client software **MUST** return `{status: SYNCING, payloadId: null}` if the payload identified by either the `forkchoiceState.headBlockHash` or the `forkchoiceState.finalizedBlockHash` is unknown. In the event that either the `forkchoiceState.headBlockHash` or the `forkchoiceState.finalizedBlockHash` is unknown, the client software **SHOULD** initiate the sync process. |
There was a problem hiding this comment.
Should the SHOULD here be a MUST?
It's weird to respond with SYNCING and not actually kick off some sync process
There was a problem hiding this comment.
Data supplied by the forkchoiceUpdated isn't always enough to initiate the sync process. For instance, there is no state root to pull the state for, so, the MUST can't always be satisfied.
Co-authored-by: Danny Ryan <[email protected]>
|
Superseded by #165 |
It is required by an optimistic sync ethereum/consensus-specs#2770 that EL sync process and according responses are fork specific. It means that despite of the sync process that is catching up with the head of any fork an EL client software must still verify payloads, update fork choice state and initiate payload build processes on another fork if data that are requisite for these operations are available for a client software.
An example:
In this example
executePayload(D), whereC -> DMUST NOT be affected by sync process that is catching up withC'. Also,forkchoiceUpdated(finalized: A', head: D)assuming thatA'is finalized ancestor ofAMUST NOT be affected by the aforementioned sync process, and it MUST initiate a payload build process if requested.Also, tired of updating numbers in the list when changing the order of items, thus, utilizing Markdown to maintain ordered lists.