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.

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

Conversation

etan-status
Copy link

While engine_forkchoiceUpdatedV1 specifies that passing an unknown headBlockHash MAY trigger a sync, EL implementations practically require engine_newPayloadV1 to be called as well.

This is prohibitive for applications where the CL is a light client that does not have the capabilities to obtain full beacon block data. To better support that scenario, a new endpoint is proposed to provide the EL client software with just the ExecutionPayloadHeader, therefore simplifying integration with existing sync logic.

Furthermore, this unlocks additional experiences:

  1. A CL that is syncing optimistically after a prolonged outage can run CL light client sync to obtain the latest ExecutionPayloadHeader. This allows the EL to start syncing to a state close to wall time without having to wait for the CL optimistic sync to catch up.
  2. EL light client implementations that wish to sync only data relevant to a certain use case (e.g., transactions for a specific wallet) can use the ExecutionPayloadHeader to decide what blocks are relevant. The new endpoint enables combined CL/EL deployments.

While `engine_forkchoiceUpdatedV1` specifies that passing an unknown
`headBlockHash` MAY trigger a sync, EL implementations practically
require `engine_newPayloadV1` to be called as well.

This is prohibitive for applications where the CL is a light client
that does not have the capabilities to obtain full beacon block data.
To better support that scenario, a new endpoint is proposed to provide
the EL client software with just the `ExecutionPayloadHeader`, therefore
simplifying integration with existing sync logic.

Furthermore, this unlocks additional experiences:
1. A CL that is syncing optimistically after a prolonged outage can run
   CL light client sync to obtain the latest `ExecutionPayloadHeader`.
   This allows the EL to start syncing to a state close to wall time
   without having to wait for the CL optimistic sync to catch up.
2. EL light client implementations that wish to sync only data relevant
   to a certain use case (e.g., transactions for a specific wallet) can
   use the `ExecutionPayloadHeader` to decide what blocks are relevant.
   The new endpoint enables combined CL/EL deployments.
@@ -174,6 +180,25 @@ 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)

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

@etan-status
Copy link
Author

Updated to use RLP hash instead of SSZ hash, but this would need this PR to go through:
ethereum/consensus-specs#3078
Converted to draft for this reason.

@etan-status etan-status marked this pull request as draft November 4, 2022 23:01

#### Specification

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 and withdrawals autonomously.
Copy link
Collaborator

Choose a reason for hiding this comment

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

EL client can't run payload validation as part of processing this call. Strictly, the only possible status in the response to this call is SYNCING, because body of the payload is missing thus there is not enough data to run validation. The corresponding forkchoiceUpdated may result in validity payload status as EL may pull the body in between calls and run full validation on forkchoiceUpdated.

So, I would strictly specify the way EL handles this particular call.

Copy link
Author

Choose a reason for hiding this comment

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

EL could do the syncing and validation as part of the call as long as it's within timeout.
Likewise, for duplicates it could return cached known VALID/INVALID answers.

Overall, agree that newPayloadHeader would return more SYNCING compared to newPayload, but not solely SYNCING.


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 and withdrawals autonomously.

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
Collaborator

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_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_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
Collaborator

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

@etan-status etan-status changed the title Add engine_newPayloadHeaderV1 Add engine_newPayloadSyncContextV1 Nov 28, 2022
@etan-status etan-status marked this pull request as ready for review November 28, 2022 19:48
@etan-status
Copy link
Author

Superseded by
#354
ethereum/EIPs#6325

Once EL block header is changed to use SSZ format for withdrawals_root and txs_root, can add an API that supplies the full CL ExecutionPayloadHeader in a fully compatible way. That one would be more useful than just the few minimal pieces suggested in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants