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

Add engine_newPayloadSyncContextV1 #318

Closed
wants to merge 15 commits into from
Closed

Add engine_newPayloadSyncContextV1 #318

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
81825fd
Add `engine_newPayloadHeaderV1`
etan-status Oct 31, 2022
029af74
Merge branch 'main' into lc-eph
etan-status Nov 4, 2022
1d42525
Satisfy spellcheck
etan-status Nov 4, 2022
d794b90
Change from SSZ to RLP hash
etan-status Nov 4, 2022
08c0030
Cleanup
etan-status Nov 4, 2022
834b36b
Improve wording
etan-status Nov 4, 2022
6054dfb
Synchronize layout
etan-status Nov 4, 2022
c86db03
Consistent format to refer to newPayload response
etan-status Nov 4, 2022
53beebe
Add new fields to nullability check
etan-status Nov 4, 2022
baa849c
Extend requirement for withdrawals
etan-status Nov 4, 2022
c206bc8
Only pass minimal sync context information
etan-status Nov 28, 2022
d7be255
Typo
etan-status Nov 28, 2022
17b4222
Always return `SYNCING` for `newPayloadSyncContext`
etan-status Dec 7, 2022
4a09624
Document error object
etan-status Dec 7, 2022
b05cb74
Improve grammar
etan-status Dec 7, 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
Prev Previous commit
Next Next commit
Merge branch 'main' into lc-eph
  • Loading branch information
@etan-status
etan-status committed Nov 4, 2022
commit 029af74d8eb70596cbe34c9a890b022cfb751ddf
61 changes: 54 additions & 7 deletions src/engine/specification.md
View file
Original file line number Diff line number Diff line change
@@ -16,6 +16,8 @@ This document specifies the Engine API methods that the Consensus Layer uses to
- [Timeouts](#timeouts)
- [Structures](#structures)
- [ExecutionPayloadV1](#executionpayloadv1)
- [WithdrawalV1](#withdrawalv1)
- [ExecutionPayloadV2](#executionpayloadv2)
- [ExecutionPayloadHeaderV1](#executionpayloadheaderv1)
- [ForkchoiceStateV1](#forkchoicestatev1)
- [PayloadAttributesV1](#payloadattributesv1)
@@ -43,11 +45,23 @@ This document specifies the Engine API methods that the Consensus Layer uses to
- [Request](#request-3)
- [Response](#response-3)
- [Specification](#specification-3)
- [Optional endpoints](#optional-endpoints)
- [engine_newPayloadHeaderV1](#engine_newpayloadheaderv1)
- [engine_getPayloadV1](#engine_getpayloadv1)
- [Request](#request-4)
- [Response](#response-4)
- [Specification](#specification-4)
- [engine_getPayloadV2](#engine_getpayloadv2)
- [Request](#request-5)
- [Response](#response-5)
- [Specification](#specification-5)
- [engine_exchangeTransitionConfigurationV1](#engine_exchangetransitionconfigurationv1)
- [Request](#request-6)
- [Response](#response-6)
- [Specification](#specification-6)
- [Optional endpoints](#optional-endpoints)
- [engine_newPayloadHeaderV1](#engine_newpayloadheaderv1)
- [Request](#request-7)
- [Response](#response-7)
- [Specification](#specification-7)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

@@ -181,9 +195,41 @@ 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)

### WithdrawalV1

This structure maps onto the validator withdrawal object from the beacon chain spec.
The fields are encoded as follows:

- `index`: `QUANTITY`, 64 Bits
- `validatorIndex`: `QUANTITY`, 64 Bits
- `address`: `DATA`, 20 Bytes
- `amount`: `QUANTITY`, 256 Bits

*Note*: the `amount` value is represented on the beacon chain as a little-endian value in units of Gwei, whereas the `amount` in this structure *MUST* be converted to a big-endian value in units of Wei.

### ExecutionPayloadV2

This structure has the syntax of `ExecutionPayloadV1` and appends a single field: `withdrawals`.

- `parentHash`: `DATA`, 32 Bytes
- `feeRecipient`: `DATA`, 20 Bytes
- `stateRoot`: `DATA`, 32 Bytes
- `receiptsRoot`: `DATA`, 32 Bytes
- `logsBloom`: `DATA`, 256 Bytes
- `prevRandao`: `DATA`, 32 Bytes
- `blockNumber`: `QUANTITY`, 64 Bits
- `gasLimit`: `QUANTITY`, 64 Bits
- `gasUsed`: `QUANTITY`, 64 Bits
- `timestamp`: `QUANTITY`, 64 Bits
- `extraData`: `DATA`, 0 to 32 Bytes
- `baseFeePerGas`: `QUANTITY`, 256 Bits
- `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)
- `withdrawals`: `Array of WithdrawalV1` - Array of withdrawals, each object is an `OBJECT` containing the fields of a `WithdrawalV1` structure.

### ExecutionPayloadHeaderV1

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think a full ExecutionPayloadHeader is required to trigger the beacon sync, only blockNumber and blockHash are sufficient and rest EL clients can fetch off the wire, since they would anyway need to fetch full block body for transactions to eventually execute the block (and if we are being liberal we can include parentHash as well)
So may be we can call it ExecutionPayloadHeaderSlim or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For LES use case, having access to logs_bloom is also useful to determine whether a block contains useful information.

The one truly questionable field is the transactionsRoot though – it being an SSZ root may make it difficult to get a VALID / INVALID classification from the EL without it also knowing how to create and verify those. For that one, I see three options:

  1. transactionsRoot is not useful for any use case, in this case just drop it from the engine API structure, diverging from the beacon chain structure.
  2. transactionsRoot would be useful as an RLP hash for some use cases, in this case extend the beacon chain structure with RLP transactionsHash that lives next to the transactions in both ExecutionPayload and ExecutionPayloadHeader.
  3. transactionsRoot is still useful in SSZ format, in this case just keep as is. Maybe for MEV blinded blocks to prove inclusion of particular tx against censoring, but those proofs could also be rooted in stateRoot I guess?

On CL light client front, the extra size for full header is not substantially different from including just a couple of the fields, as proof size goes up for individual fields. So, full ExecutionPayloadHeader can be expected to be available. Any subset is alright for the engine API.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, so...

for execution nodes running full protocol, it won't matter what kind of transactionsRoot we provide since they would still need to download the block off the wire (as you observed) using their own peers as they do in normal beacon sync when they don't have parents.

Now for les we could make it easy by having the RLP transactions hash that the blockhash lines up and they don't have to download anything off wire (unless there is a missing header update in which case they will again have to)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ethereum/consensus-specs#3078
Opened a proposal here to add those RLP hashes to the ExecutionPayload / ExecutionPayloadHeader as well, from Capella onward. This would make this much easier, removing the need for RLP computations in CL, or for SSZ computations in EL.

This structure maps on the [`ExecutionPayloadHeader`](https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/beacon-chain.md#executionpayloadheader) structure of the beacon chain spec. The fields are encoded as follows:
This structure maps on the [`ExecutionPayloadHeader`](https://github.com/ethereum/consensus-specs/blob/dev/specs/capella/beacon-chain.md#executionpayloadheader) structure of the Capella beacon chain spec. The fields are encoded as follows:

- `parentHash`: `DATA`, 32 Bytes
- `feeRecipient`: `DATA`, 20 Bytes
@@ -199,6 +245,7 @@ This structure maps on the [`ExecutionPayloadHeader`](https://github.com/ethereu
- `baseFeePerGas`: `QUANTITY`, 256 Bits
- `blockHash`: `DATA`, 32 Bytes
- `transactionsRoot`: `DATA`, 32 Bytes - `hash_tree_root(transactions)`, not the keccak256 hash but instead the Consensus Layer [SSZ merkle root](https://github.com/ethereum/consensus-specs/blob/dev/ssz/simple-serialize.md)
- `withdrawalsRoot`: `DATA`, 32 Bytes - `hash_tree_root(withdrawals)`, not the keccak256 hash but instead the Consensus Layer [SSZ merkle root](https://github.com/ethereum/consensus-specs/blob/dev/ssz/simple-serialize.md)

### ForkchoiceStateV1

@@ -509,12 +556,12 @@ Refer to the specification for [`engine_getPayloadV1`](#engine_getpayloadv1).

#### Specification

1. Execution Layer client software **MUST** handle calls to this endpoint the same as [`engine_newPayloadV1`](#engine_getpayloadv1) and provide compatible responses. When needed, Execution Layer client software **MUST** obtain the block's transactions autonomously.
1. Execution Layer client software **MUST** handle calls to this endpoint the same as [`engine_newPayloadV2`](#engine_getpayloadv2) and provide compatible responses. When needed, Execution Layer client software **MUST** obtain the block's transactions autonomously.

2. Consensus Layer client software **SHOULD NOT** use this endpoint for validator duties. Instead, the [`engine_newPayloadV1`](#engine_getpayloadv1) endpoint **SHOULD** be used to reduce sync latency and maximize validator rewards.
2. Consensus Layer client software **SHOULD NOT** use this endpoint for validator duties. Instead, the [`engine_newPayloadV2`](#engine_getpayloadv2) endpoint **SHOULD** be used to reduce sync latency and maximize validator rewards.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2. Consensus Layer client software **SHOULD NOT** use this endpoint for validator duties. Instead, the [`engine_newPayloadV2`](#engine_getpayloadv2) endpoint **SHOULD** be used to reduce sync latency and maximize validator rewards.
2. Consensus Layer client software **MUST NOT** use this endpoint for validator duties. Instead, the [`engine_newPayloadV2`](#engine_getpayloadv2) endpoint **MUST** be used to reduce sync latency and maximize validator rewards.

Validators MUST always run full nodes, not only because of latency and rewards issues, but also because of LC protocol security assumptions which guarantees are based on full nodes validating the chain.


3. Consensus Layer client software **MAY** use this endpoint during [optimistic sync](https://github.com/ethereum/consensus-specs/blob/dev/sync/optimistic.md) to inform Execution Layer client software about blocks far in the future. Execution Layer client software **MUST** support switching to this future block if requested to do so with [`engine_forkchoiceUpdatedV1`](#engine_forkchoiceupdatedv1). This allows the Execution Layer client software to sync close to current wall time without having to wait for optimistic sync to catch up.
3. Consensus Layer client software **MAY** use this endpoint during [optimistic sync](https://github.com/ethereum/consensus-specs/blob/dev/sync/optimistic.md) to inform Execution Layer client software about blocks far in the future. Execution Layer client software **MUST** support switching to this future block if requested to do so with `engine_forkchoiceUpdated`. This allows the Execution Layer client software to sync close to current wall time without having to wait for optimistic sync to catch up.

4. Consensus Layer [light clients](https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md) **MAY** use this endpoint together with [`engine_forkchoiceUpdatedV1`](#engine_forkchoiceupdatedv1) to sync Execution Layer client software. Execution Layer client software **MUST** support syncing with only those two endpoints. Notably, syncing **MUST NOT** require [`engine_newPayloadV1`](#engine_getpayloadv1) calls. Furthermore, Execution Layer client software **MAY** also support syncing with solely `engine_forkchoiceUpdatedV1` calls.
4. Consensus Layer [light clients](https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md) **MAY** use this endpoint together with `engine_forkchoiceUpdated` to sync Execution Layer client software. Execution Layer client software **MUST** support syncing with only those two endpoints. Notably, syncing **MUST NOT** require `engine_newPayload` calls. Furthermore, Execution Layer client software **MAY** also support syncing with solely `engine_forkchoiceUpdated` calls.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about moving "syncing" requirement into a separate point? Something like EL client software running a full node MAY initiate the sync process, otherwise MUST defer the sync start to the corresponding forkchoiceUpdated message? Also, a separate statement may relate to EL client software running a light node as it is already done by (5).

IMO, this statement made before (3) and (4) would bring more clarity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified using Execution Layer client **MUST** support syncing solely based on calls to this endpoint and engine_forkchoiceUpdated`. How the EL splits up the work to trigger the sync start is an implementation detail.


5. Client software **MAY** offer configuration options to limit the sync scope to use case dependent data (e.g., only sync transactions relating to a certain wallet). This enables combined Consensus Layer / Execution Layer light client experiences.
You are viewing a condensed version of this merge commit. You can view the full changes here.