Introduce an ExecutionPayloadEnvelope to abstract away the fields needed in the EL but not the CL

[potuz]

Up until Electra, the CL clients would not save the full execution payload as this is not needed on the beacon state transition function. However, Electra includes new fields like withdrawal requests, deposit requests and consolidation requests that the consensus client requires in order to perform the state transition function. With the current design the consensus client would need to either

• save locally the requests or
• requests the payload from the EL for each time they want to execute a state transition

I propose to move the BeaconBlockBody to have the following structure on the CL

class BeaconBlockBody(Container):
    randao_reveal:    BLSSignature
    ...
    execution_payload_envelope     ExecutionPayloadEnvelope
    ...
   

class ExecutionPayloadEnvelope(Container)
    deposit_requests: List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD]  # [New in Electra:EIP6110]
    withdrawal_requests: List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD]
    consolidation_requests: List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD]
    execution_payload:    ExecutionPayload
    
class ExecutionPayload(Container):
    parent_hash:  Hash32
    ...

This way the CL can simply replace execution_payload by it's hash tree root and keep in DB a binded_execution_payload_envelope instead of the full envelope and still be able to perform the state transition function.

The exchange with the EL over the Engine API would also reflect this structure. The EL would need to take the requests from the envelope, add them to the ExecutionPayload object in order to compute the block hash.

[etan-status]

Makes sense to me! (with execution_payload moved to the beginning so that it does not get re-indexed as new requests types get added).

The actual DB optimization could also be achieved by replacing the transactions list with its root. Feels more janky though.

How much of this is required for ePBS?

in BeaconState, we only need the execution_payload block_hash?

[potuz]

How much of this is required for ePBS?

In ePBS we already have an envelope (and it's even signed) for a different reason, the payload reveal stage also sends information that is needed for the CL: https://github.com/ethereum/consensus-specs/blob/b082140312aad8248ffb7dafe1bf9c5d0c66c221/specs/_features/eip7732/beacon-chain.md#executionpayloadenvelope

[terencechain]

The actual DB optimization could also be achieved by replacing the transactions list with its root. Feels more janky though.

This may require adding a new payload type, which would be nice to avoid.

[mkalinin]

Alternatively, the structures can be as the following:

class BeaconBlockBody(Container):
    randao_reveal:    BLSSignature
    ...
    execution_payload: ExecutionPayload
    bls_to_execution_changes: List[SignedBLSToExecutionChange, MAX_BLS_TO_EXECUTION_CHANGES]
    blob_kzg_commitments: List[KZGCommitment, MAX_BLOB_COMMITMENTS_PER_BLOCK]
    deposit_requests: List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD]  # [New in Electra:EIP6110]
    withdrawal_requests: List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD]  # [New in Electra:EIP7002]
    consolidation_requests: List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD]  # [New in Electra:EIP7251]

In this case new fields are appended to the end instead of switching the type of the existing execution_payload field. In this case it won’t affect gindex of the payload (at least by Electra).

In this case Engine API has two options assuming all requests are put into a single list: ExecutionPayloadEnvelope or pass requests as param to newPayloadV4 and include requests into response of the getPayloadV4.

I am not sure if this would be future compatible with ePBS and if we even want to keep such compatibility in mind (sorry @potuz).

[terencechain]

Alternatively, the structures can be as the following:

class BeaconBlockBody(Container):

I know the builder API shouldn't affect any decision-making on this, but one thing to note is that if we do this (which I don't see any blockers for), we will have to add all the request types to the builder's bid. I'm not against this but wanted to point it out.

[mkalinin]

Alternatively, the structures can be as the following:

class BeaconBlockBody(Container):

I know the builder API shouldn't affect any decision-making on this, but one thing to note is that if we do this (which I don't see any blockers for), we will have to add all the request types to the builder's bid. I'm not against this but wanted to point it out.

AFAIU, this would require an update to the BuilderBid and BlindedBeaconBlockBody structures in the API:

class BuilderBid(Container):
    header: ExecutionPayloadHeader
    blob_kzg_commitments: List[KZGCommitment, MAX_BLOB_COMMITMENTS_PER_BLOCK]
    deposit_requests: List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD]  # [New in Electra]
    withdrawal_requests: List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD]  # [New in Electra]
    consolidation_requests: List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD]  # [New in Electra]
    value: uint256
    pubkey: BLSPubkey


class BlindedBeaconBlockBody(Container):
    randao_reveal: BLSSignature
    eth1_data: Eth1Data
    graffiti: Bytes32
    proposer_slashings: List[ProposerSlashing, MAX_PROPOSER_SLASHINGS]
    attester_slashings: List[AttesterSlashing, MAX_ATTESTER_SLASHINGS]
    attestations: List[Attestation, MAX_ATTESTATIONS]
    deposits: List[Deposit, MAX_DEPOSITS]
    voluntary_exits: List[SignedVoluntaryExit, MAX_VOLUNTARY_EXITS]
    sync_aggregate: SyncAggregate
    execution_payload_header: ExecutionPayloadHeader
    bls_to_execution_changes: List[SignedBLSToExecutionChange, MAX_BLS_TO_EXECUTION_CHANGES]
    blob_kzg_commitments: List[KZGCommitment, MAX_BLOB_COMMITMENTS_PER_BLOCK]
    deposit_requests: List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD]  # [New in Electra]
    withdrawal_requests: List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD]  # [New in Electra]
    consolidation_requests: List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD]  # [New in Electra]

The above changes look neat to me. Am I missing anything or the changes could be simpler with the ExecutionPayloadEnvelope to your view?

[potuz]

@mkalinin the changes you propose look fine to me.

I am not sure if this would be future compatible with ePBS and if we even want to keep such compatibility in mind (sorry @potuz).

It's not compatible with EIP-7732, but anyway that EIP will change the block type and it would necessarily introduce a signed envelope and change the execution payload type so it doesn't really make it worse. However, if we did change to an envelope today we avoid changing the beacon block body type in the future for any new requests that appear in the Payload, with or without ePBS. I prefer the envelope design as it's more robust and encapsulated, instead of leaking data that is introduced from the EL directly in the CL structure, it stays within the envelope.

[mkalinin]

My only argument that is not in favour of the envelope approach is that it changes gindex of the payload which may affect some consumers while appending requests to the end does not. Once we have stable container such changes should become easier to apply. Other than that, the envelope approach looks good to me.

I don’t get the argument of leaking data because those requests are CL operations emitted by EL and since we move them out of the ExecutionPayload the place where they sit shouldn’t be necessarily tight to the payload, IMO. Each of those request type have a separate handler in the block processing routine and their processing does not depend on the payload. The only dependency is to check whether the given requests are the ones that the EL block is committed to which can be handled on the Engine API level. But I do admit that this will change with ePBS as there are more things introduced that are related to the payload but aren't part of it.

[etan-status]

One thing to keep in mind is that if EIP-7688 SSZ StableContainer gets adopted, generalized indices will get reindexed anyway, so we will have an opportunity to optimize the affected data structures in a subsequent Pectra devnet.

[etan-status]

Should we group the various validator request fields within a sub-Container to match the EL design with a requests_root more closely, and also to be able to group the various requests if there actually is a new one at some future time? see ethereum/execution-apis#565 (comment)

e.g.

class ValidatorRequests(Conainer):
    deposit_requests: List[DepositRequest, MAX_DEPOSIT_REQUESTS_PER_PAYLOAD]  # [New in Electra:EIP6110]
    withdrawal_requests: List[WithdrawalRequest, MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD]
    consolidation_requests: List[ConsolidationRequest, MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD]

class ExecutionPayloadEnvelope(Container)
    execution_payload:    ExecutionPayload
    requests: ValidatorRequests

[g11tech]

I am generally fine with any/either of changes proposed but everything EL responded in envelop sounds good to me for the fact that envelop will be modified in epbs as @potuz mentioned.

it would be nice to also have requests abstracted away in requests field as @etan-status mentioned

[rolfyone]

I agree with @g11tech , generally fine with the envelop approach and that it would be modified, and requests field described by @etan-status seems logical.

[potuz]

+1 for Etan's subtree... Now we need volunteers to write the PR :)

[lucassaldanha]

@potuz I started a PR here: #3875. Still WIP as I review some interactions. Hopefully it has everything you and @etan-status suggested!

One thing I am cautious about is the process_withdrawals function. One of its parameters is the execution_payload, and we get the withdrawals from it to compare with expected_withdrawals (read from the state).

In your example where the CL replaces execution_payload by it's hash tree root (and keep a binded_execution_payload_envelope in DB), does it impact this at all? Sorry if this is a silly question but I am not super familiar with the blinded block flow/spec.

I am curious because in Teku, we did not implement a element-to-element comparison to check the expected withdrawals, we actually compare the withdrawals_root from the execution_payload_header (that we can get from a blinded block body anyway).

TLDR: should we consider updating process_withdrawals to compare the hash root of the expected list of withdrawals with the withdrawal_root from the execution_payload_header in the spec?