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.

Sign up for GitHub

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

Engine API: extend semantics of executePayload and forkchoiceUpdated methods #165

Merged
merged 7 commits into from
Jan 26, 2022
Merged

Engine API: extend semantics of executePayload and forkchoiceUpdated methods #165

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9b5ff91
Engine API: extend executePayload and forkchoiceUpdated semantics
mkalinin Jan 21, 2022
0324389
Address Danny's feedback
mkalinin Jan 25, 2022
e269920
Engine API: refine semantics changes
mkalinin Jan 26, 2022
c5a7fb3
Fix spellchecker
mkalinin Jan 26, 2022
56efc7b
Fix spellchecker, take 2
mkalinin Jan 26, 2022
3edcc1e
minor copy edits from code review
djrtwo Jan 26, 2022
24996ff
Update src/engine/specification.md
djrtwo Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 60 additions & 24 deletions src/engine/specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ This document specifies the Engine API methods that the Consensus Layer uses to

- [Underlying protocol](#underlying-protocol)
- [Versioning](#versioning)
- [Constants](#constants)
- [Message ordering](#message-ordering)
- [Load-balancing and advanced configurations](#load-balancing-and-advanced-configurations)
- [Errors](#errors)
Expand All @@ -18,10 +17,12 @@ This document specifies the Engine API methods that the Consensus Layer uses to
- [ForkchoiceStateV1](#forkchoicestatev1)
- [PayloadAttributesV1](#payloadattributesv1)
- [Core](#core)
- [engine_executePayloadV1](#engine_executepayloadv1)
- [engine_newPayloadV1](#engine_newpayloadv1)
- [Request](#request)
- [Response](#response)
- [Specification](#specification)
- [Payload validation process](#payload-validation-process)
- [Sync process](#sync-process)
- [engine_forkchoiceUpdatedV1](#engine_forkchoiceupdatedv1)
- [Request](#request-1)
- [Response](#response-1)
Expand Down Expand Up @@ -58,7 +59,7 @@ The versioning of the Engine API is defined as follows:
* a method response value
* a method behavior
* a set of structure fields
* The specification **MAY** reference a method or a structure without the version suffix e.g. `engine_executePayload`. These statements should be read as related to all versions of the referenced method or structure.
* The specification **MAY** reference a method or a structure without the version suffix e.g. `engine_newPayload`. These statements should be read as related to all versions of the referenced method or structure.

## Message ordering

Expand All @@ -76,7 +77,7 @@ Intuitively this is because the Consensus Layer drives the Execution Layer and t
On the other hand, generic many-to-one Consensus Layer to Execution Layer configurations are not supported out-of-the-box.
The Execution Layer, by default, only supports one chain head at a time and thus has undefined behavior when multiple Consensus Layers simultaneously control the head.
The Engine API does work properly, if in such a many-to-one configuration, only one Consensus Layer instantiation is able to *write* to the Execution Layer's chain head and initiate the payload build process (i.e. call `engine_forkchoiceUpdated` ),
while other Consensus Layers can only safely insert payloads (i.e. `engine_executePayload`) and read from the Execution Layer.
while other Consensus Layers can only safely insert payloads (i.e. `engine_newPayload`) and read from the Execution Layer.

## Errors

Expand Down Expand Up @@ -155,35 +156,60 @@ This structure contains the attributes required to initiate a payload build proc

## Core

### engine_executePayloadV1
### engine_newPayloadV1

#### Request

* method: `engine_executePayloadV1`
* method: `engine_newPayloadV1`
* params:
1. [`ExecutionPayloadV1`](#ExecutionPayloadV1)

#### Response

* result: `object`
- `status`: `enum` - `"VALID" | "INVALID" | "SYNCING"`
- `status`: `enum` - `"VALID" | "INVALID" | "SYNCING" | "ACCEPTED" | "INVALID_BLOCK_HASH"`
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
- `latestValidHash`: `DATA|null`, 32 Bytes - the hash of the most recent *valid* block in the branch defined by payload and its ancestors
- `validationError`: `String|null` - a message providing additional details on the validation error if the payload is deemed `INVALID`
* error: code and message set in case an exception happens while executing the payload.
* error: code and message set in case an exception happens while processing the payload.

#### Specification

1. Client software **MUST** validate the payload according to the execution environment rule set with modifications to this rule set defined in the [Block Validity](https://eips.ethereum.org/EIPS/eip-3675#block-validity) section of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#specification) and return the validation result.
* If validation succeeds, return `{status: VALID, latestValidHash: payload.blockHash}`
* If validation fails, return `{status: INVALID, latestValidHash: validHash}` where `validHash` is the block hash of the most recent *valid* ancestor of the invalid payload. That is, the valid ancestor of the payload with the highest `blockNumber`.
1. Client software **MUST** validate `blockHash` value as being equivalent to `Keccak256(RLP(ExecutionBlockHeader))`, where `ExecutionBlockHeader` object is composed of the payload field values and constant values described in the [Block structure](https://eips.ethereum.org/EIPS/eip-3675#block-structure) section of the [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#specification). Additionally:
* Client software **MUST** return `{status: INVALID_BLOCK_HASH, latestValidHash: null, validationError: null}` and discard the payload if this validation fails
* Client software **MUST** run this validation in all cases even if this branch or any other branches of the block tree are in an active sync process.

2. Client software **MUST** discard the payload if it's deemed invalid.
1. Client software **SHOULD** initiate the sync process if requisite data for payload validation is missing. The sync process is specified in the [Sync process](#sync-process) section. Additionally:
* Client software **MUST** return `{status: SYNCING, latestValidHash: null, validationError: null}` if the sync process has been initiated.

3. Client software **MUST** return `{status: SYNCING, latestValidHash: null}` if the sync process is already in progress or if requisite data for payload validation is missing. In the event that requisite data to validate the payload is missing (e.g. does not have payload identified by `parentHash`), the client software **SHOULD** initiate the sync process.
1. Client software **SHOULD** validate the payload if requisite data for payload validation is locally available. The validation process is specified in the [Payload validation process](#payload-validation-process) section. Additionally:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From acd today: this should be made stricter, to not make it implementation-dependent so that clients all lazily choose not to validate any newPayloads at all

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it's more like the following:

MUST validate the payload if extending the canonical head, and MAY validate if on a non-canonical branch

* Client software **MUST** return the result of the payload validation if the validation has been initiated.

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** return `{status: ACCEPTED, latestValidHash: null, validationError: null}` if neither payload validation nor sync process has been initiated.

5. Client software **MUST** return `-32002: Invalid terminal block` error if the parent block is a PoW block that doesn't satisfy terminal block conditions according to [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#definitions). This check maps on the Transition block validity section of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#transition-block-validity).
1. If any of the above fails due to errors unrelated to the normal processing flow of the method, the client software **MUST** return an error.

##### Payload validation process
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

Payload validation process consists of validating a payload with respect to the block header and execution environment rule sets. The process is specified as follows:

1. Client software **MUST** validate the payload according to the block header and execution environment rule set with modifications to these rule sets defined in the [Block Validity](https://eips.ethereum.org/EIPS/eip-3675#block-validity) section of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#specification). Additionally:
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
* If validation succeeds, the response **MUST** contain `{status: VALID, latestValidHash: payload.blockHash}`
* If validation fails, the response **MUST** contain `{status: INVALID, latestValidHash: validHash}` where `validHash` is the block hash of the most recent *valid* ancestor of the invalid payload. That is, the valid ancestor of the payload with the highest `blockNumber`
* Client software **MUST** discard the payload if it's deemed invalid.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

1. Client software **MUST** return `-32002: Invalid terminal block` error if the parent block is locally available and is a PoW block that doesn't satisfy terminal block conditions according to [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#definitions). This check maps on the Transition block validity section of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#transition-block-validity).
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

1. 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. The process of validating a payload on the canonical chain **MUST NOT** be affected by an active sync process on a side branch of the block tree. For example, if side branch `B` is `SYNCING` but the requisite data for validating a payload from canonical branch `A` is available, client software **MUST** initiate the validation process.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client software MUST initiate the validation process.

As stated, this seems a bit circular because we are a the end of the validation process.
Do we mean instead:

client software MUST run full validation of the payload, returning either VALID or INVALID


##### Sync process
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

In the context of this specification, the sync process is understood as the process of obtaining data required to validate the payload. The sync process may consist of the following stages:
1. Pulling data from remote peers in the network.
1. Validating ancestors of the payload and obtaining the parent state.

Each of these stages is optional. Exact behavior of a client software during the sync process is implementation dependent.

### engine_forkchoiceUpdatedV1

Expand All @@ -197,25 +223,35 @@ This structure contains the attributes required to initiate a payload build proc
#### Response

* result: `object`
- `status`: `enum` - `"SUCCESS" | "SYNCING"`
- `status`: `enum` - `"VALID" | "INVALID" | "SYNCING" | "ACCEPTED"`
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
- `latestValidHash`: `DATA|null`, 32 Bytes - the hash of the most recent *valid* block in the branch defined by payload and its ancestors
- `validationError`: `String|null` - a message providing additional details on the validation error if the payload is deemed `INVALID`
- `payloadId`: `DATA|null`, 8 Bytes - identifier of the payload build process or `null`
* error: code and message set in case an exception happens while updating the forkchoice or initiating the payload build process.
* error: code and message set in case an exception happens while validating payload, updating the forkchoice or initiating the payload build process.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

#### Specification

1. The values `(forkchoiceState.headBlockHash, forkchoiceState.finalizedBlockHash)` of this method call map on the `POS_FORKCHOICE_UPDATED` event of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#specification) and **MUST** be processed according to the specification defined in the EIP.
1. Client software **SHOULD** initiate the sync process if `forkchoiceState.headBlockHash` references an unknown payload or a payload that can't be validated because requisite data is missing. The sync process is specified in the [Sync process](#sync-process) section. Additionally:
* Client software **MUST** return `{status: SYNCING, latestValidHash: null, validationError: null, payloadId: null}` if the sync process has been initiated.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

2. All updates to the forkchoice state resulting from this call **MUST** be made atomically.
1. Client software **MAY** skip an update of the forkchoice state and **MAY NOT** begin a payload build process if the payload identified by `forkchoiceState.headBlockHash` hasn't been validated yet. Additionally:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you follow the above suggestion to insert SYNCING logic into payloadValidation, then you need to update this statement to say:

if the payload identified by forkchoiceState.headBlockHash known but has not been validated yet

* Client software **MUST** return `{status: ACCEPTED, latestValidHash: null, validationError: null, payloadId: null}` in this case.

3. Client software **MUST** return `{status: SUCCESS, payloadId: null}` if `payloadAttributes` is `null` and the client is not `SYNCING`.
1. Client software **MAY** skip an update of the forkchoice state and **MAY NOT** begin a payload build process if `forkchoiceState.headBlockHash` references an ancestor of the head of the canonical chain. Additionally:
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
* Client software **MUST** return `{status: VALID, latestValidHash: forkchoiceState.headBlockHash, validationError: null, payloadId: null}` in this case.

4. Client software **MUST** return `{status: SYNCING, payloadId: null}` if the payload identified by either the `forkchoiceState.headBlockHash` or the `forkchoiceState.finalizedBlockHash` is unknown or if the sync process is in progress. In the event that either the `forkchoiceState.headBlockHash` or the `forkchoiceState.finalizedBlockHash` is unknown, the client software **SHOULD** initiate the sync process.
1. Before updating the forkchoice state, client software **MUST** ensure the validity of the payload identified by `forkchoiceState.headBlockHash`, and **MAY** initiate payload validation process if that need be. The validation process is specified in the [Payload validation process](#payload-validation-process) section. Additionally:
* Client software **MUST NOT** update the forkchoice state and immediately respond to the method call with the corresponding result if the payload or any of its ancestors is deemed `INVALID`.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

5. Client software **MUST** return `{status: SUCCESS, payloadId: buildProcessId}` if `payloadAttributes` is not `null` and the client is not `SYNCING`, and **MUST** begin a payload build process building on top of `forkchoiceState.headBlockHash` and identified via `buildProcessId` value. The build process is specified in the [Payload build process](#payload-build-process) section.
1. Client software **MUST** update its forkchoice state if payloads identified by `forkchoiceState.headBlockHash` and `forkchoiceState.finalizedBlockHash` are deemed valid. The update is specified as follows:
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
* The values `(forkchoiceState.headBlockHash, forkchoiceState.finalizedBlockHash)` of this method call map on the `POS_FORKCHOICE_UPDATED` event of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#specification) and **MUST** be processed according to the specification defined in the EIP
* All updates to the forkchoice state resulting from this call **MUST** be made atomically
* Client software **MUST** return `{status: VALID, latestValidHash: forkchoiceState.headBlockHash, validationError: null, payloadId: null}` if `payloadAttributes` is `null`.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

6. Client software **MUST** return `-32002: Invalid terminal block` error if a block referenced by `forkchoiceState.headBlockHash` is a PoW block that doesn't satisfy terminal block conditions according to [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#definitions).
1. Client software **MUST** begin a payload build process building on top of `forkchoiceState.headBlockHash` and identified via `buildProcessId` value if `payloadAttributes` is not `null` and the forkchoice state has been updated successfully. The build process is specified in the [Payload build process](#payload-build-process) section. Additionally:
* Client software **MUST** return `{status: VALID, latestValidHash: forkchoiceState.headBlockHash, validationError: null, payloadId: buildProcessId}`.

7. If any of the above fails due to errors unrelated to the client software's normal `SYNCING` status, the client software **MUST** return an error.
1. If any of the above fails due to errors unrelated to the normal processing flow of the method, the client software **MUST** return an error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So errors in the payload assembling process are returned as JSON_RPC errors directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand your question correctly. But if an exception occurred during initiating a payload build process then the call of this method must be responded with error.

Generally, this point states that in the case when e.g. a payload is INVALID according to EE rules an INVALID status must be in the response, but if exception happened during the validation then the call must be responded with error, not an INVALID status.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should EL return if we receive the wrong timestamp in payload attributes? Because I think INVALID is connected to headBlockHash, not payload attributes. By the wrong timestamp, I mean headBlockHash.Timestamp >= payloadAttributes.Timestamp

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would mean that CL is faulty, EL may respond with error in this case. There is -32602: Invalid params error that fits well into this case


##### Payload build process
The payload build process is specified as follows:
Expand Down