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

Jump to bottom

remove preparePayload #83

Merged
merged 5 commits into from
Oct 14, 2021
Merged

remove preparePayload #83

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae7e898
remove prepare payload in favor of combining with forkchoiceupdated
djrtwo Oct 12, 2021
be6356e
use sha256 for payloadId instaed of keccak256
djrtwo Oct 13, 2021
ecee489
clarify note on SYNCING for forkchoiceUpdated
djrtwo Oct 13, 2021
090e499
Update src/engine/specification.md
djrtwo Oct 14, 2021
c7652d6
clarify PayloadAttributes datastructure
djrtwo Oct 14, 2021
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
79 changes: 36 additions & 43 deletions src/engine/specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ The list of error codes introduced by this specification can be found below.

| Code | Possible Return message | Description |
| - | - | - |
| 4 | Unknown header | Should be used when a call refers to an unknown header |
| 5 | Unknown payload | Should be used when the `payloadId` parameter of `engine_getPayload` call refers to a payload building process that is unavailable |
| 5 | Unknown payload | Should be used when the `payloadId` parameter of `engine_getPayload` call refers to a payload build process that is unavailable |

## Structures

Expand All @@ -44,50 +43,14 @@ This structure maps on the [`ExecutionPayload`](https://github.com/ethereum/cons
- `blockHash`: `DATA`, 32 Bytes
- `transactions`: `Array of DATA` - Array of transaction objects, each object is a byte list (`DATA`) representing `TransactionType || TransactionPayload` or `LegacyTransaction` as defined in [EIP-2718](https://eips.ethereum.org/EIPS/eip-2718)

## Core

### engine_preparePayload
### PayloadAttributes

#### Parameters
1. `Object` - The payload attributes:
- `parentHash`: `DATA`, 32 Bytes - hash of the parent block
This structure contains the attributes (in addition to the other parameters contained in `engine_forkchoiceUpdated`) required to initiate a payload build process. The fields are encoded as follows:
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
- `timestamp`: `QUANTITY`, 64 Bits - value for the `timestamp` field of the new payload
- `random`: `DATA`, 32 Bytes - value for the `random` field of the new payload
- `feeRecipient`: `DATA`, 20 Bytes - suggested value for the `coinbase` field of the new payload

#### Returns
`Object|Error` - Either instance of response object or an error. Response object:
1. `payloadId`: `QUANTITY`, 64 Bits - Identifier of the payload building process

#### Specification

1. Given provided field value set client software **SHOULD** build the initial version of the payload which has an empty transaction set.

2. Client software **SHOULD** start the process of updating the payload. The strategy of this process is implementation dependent. The default strategy would be to keep the transaction set up-to-date with the state of local mempool.

3. Client software **SHOULD** stop the updating process either by finishing to serve the [**`engine_getPayload`**](#engine_getPayload) call with the same `payloadId` value or when [`SECONDS_PER_SLOT`](https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#time-parameters-1) (currently set to 12 in the Mainnet configuration) seconds have passed since the point in time identified by the `timestamp` parameter.

4. Client software **MUST** set the payload field values according to the set of parameters passed in the call to this method with exception for the `feeRecipient`. The `coinbase` field value **MAY** deviate from what is specified by the `feeRecipient` parameter.

5. Client software **SHOULD** respond with `2: Action not allowed` error if the sync process is in progress.

6. Client software **SHOULD** respond with `4: Unknown header` error if the parent block is unknown.

### engine_getPayload

#### Parameters
1. `payloadId`: `QUANTITY`, 64 Bits - Identifier of the payload building process

#### Returns
`Object|Error` - Either instance of [`ExecutionPayload`](#ExecutionPayload) or an error

#### Specification

1. Given the `payloadId` client software **MUST** respond with the most recent version of the payload that is available in the corresponding building process at the time of receiving the call.

2. The call **MUST** be responded with `5: Unknown payload` error if the building process identified by the `payloadId` doesn't exist.

3. Client software **MAY** stop the corresponding building process after serving this call.
## Core

### engine_executePayload

Expand Down Expand Up @@ -119,12 +82,42 @@ This structure maps on the [`ExecutionPayload`](https://github.com/ethereum/cons
1. `Object` - The state of the fork choice:
- `headBlockHash`: `DATA`, 32 Bytes - block hash of the head of the canonical chain
- `finalizedBlockHash`: `DATA`, 32 Bytes - block hash of the most recent finalized block
- `payloadAttributes`: `Object|None` - instance of [`PayloadAttributes`](#PayloadAttributes) or `None`

#### Returns
None or an error

`Object|Error` - Either instance of response object or an error. Response object:
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't appear the errors that can be returned are described?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Must we describe all errors? Anything that actually fails on the client during this call might bubble up an error. At interop, the general idea was to have return values not error under normal operation (thus SYNCING instead of "error: block missing") and for errors to actually represent errors in the operation.

Peter made it seem like if any number of things fail in geth, they'd surface that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can have SUCCESS or you could be or enter into SYNCING. In the event that something fails outside of those two, you then send an error.

Can we define all such errors?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's fair. I think the small thing that's bothering me is that Object|Error when Error doesn't feel very well defined. I will take a stab fixing this myself after we merge this though!

1. `status`: `String` - The result of updating the fork choice
- `SUCCESS` - The fork choice is successfully updated, and if requested, the payload build process has successfully started
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is SUCCESS status supposed to mean? Success fo receiving the call or updating the fork choice state? The former doesn't seem valuable as JSON-RPC would send a respond in any case indicating whether it's been successfully received or not. The latter implies the failure scenario, one of such scenarios is the absence of either finalized or head block coming in the method call; and if we have a success scenario signalled in the response then the failure should be signalled as well. Or we may rename SUCCESS to ACK to avoid confusion which won't be useful by itself but would allow for SYNCING to be returned as an alternative.

- `SYNCING` - Sync process is in progress

#### Specification

1. This method call maps 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.

2. Client software **MUST** respond with `4: Unknown block` error if the payload identified by either the `headBlockHash` or the `finalizedBlockHash` is unknown.
2. Client software **MUST** return `SYNCING` status if the payload identified by either the `headBlockHash` or the `finalizedBlockHash` is unknown.
lightclient marked this conversation as resolved.
Show resolved Hide resolved

3. Client software **MUST** begin a payload build process building on top of `headBlockHash` if `payloadAttributes` is not `None` and the client is not `SYNCING`. The build process is specified as:
* The payload build process **MUST** be identifid via `payloadId` where `payloadId` is defined as the first `8` bytes of the `keccak256` hash of concatenation of `headBlockHash`, `payloadAttributes.timestamp`, `payloadAttributes.random`, and `payloadAttributes.feeRecipient` where `payloadAttributes.timestamp` is encoded as big-endian and padded fully to 8 bytes -- i.e. `keccak256(headBlockHash + timestamp + random + feeRecipient)[0:8]`.
lightclient marked this conversation as resolved.
Show resolved Hide resolved
* Client software **MUST** set the payload field values according to the set of parameters passed into this method with exception of the `feeRecipient`. The `coinbase` field value **MAY** deviate from what is specified by the `feeRecipient` parameter.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved
* Client software **SHOULD** build the initial version of the payload which has an empty transaction set.
* Client software **SHOULD** start the process of updating the payload. The strategy of this process is implementation dependent. The default strategy is to keep the transaction set up-to-date with the state of local mempool.
* Client software **SHOULD** stop the updating process when either a call to [**`engine_getPayload`**](#engine_getPayload) with the build process's `payloadId` is made or [`SECONDS_PER_SLOT`](https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#time-parameters-1) (currently set to 12 in the Mainnet configuration) seconds have passed since the point in time identified by the `timestamp` parameter.

4. 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.

### engine_getPayload

#### Parameters
1. `payloadId`: `DATA`, 8 bytes - Identifier of the payload build process

#### Returns
`Object|Error` - Either instance of [`ExecutionPayload`](#ExecutionPayload) or an error
lightclient marked this conversation as resolved.
Show resolved Hide resolved

#### Specification

1. Given the `payloadId` client software **MUST** return the most recent version of the payload that is available in the corresponding build process at the time of receiving the call.

2. The call **MUST** return `5: Unknown payload` error if the build process identified by the `payloadId` does not exist.

3. Client software **MAY** stop the corresponding build process after serving this call.