EIP-7742: Uncouple blob limits per block across CL and EL

[ralexstokes]

This is the CL part of a change to remove the hard-coded blob count(s) from the execution layer.

Motivation here: https://hackmd.io/@ralexstokes/rJhC_vkSR#Uncertainty-around-deployment-timeline-and-cross-layer-concerns

EIP is here:

• Add EIP: Uncouple blob count between CL and EL EIPs#8735

Engine API PR here:

• changes required for EIP-7742 execution-apis#574

This PR targets Pectra, and I will update to a spec "feature" if we decide against Pectra inclusion.

[g11tech]

strong lgtm from lodestar and ethereumjs team

[mkalinin]

(thanks to @g11tech and @terencechain for suggestion) Add blobGasLimit to execution payload header (as then all data is self-contained)

From my perspective, this looks like the best option with which CL can run assert execution_payload.blob_gas_limit <= MAX_BLOBS_PER_BLOCK * GAS_PER_BLOB check during the payload processing and pass the limit to the build process as a part of payload attributes.

There is another constant TARGET_BLOB_GAS_PER_BLOCK that is explicitly set by the EIP, do we want to turn this constant to be a half of the max value (computable on the fly) or make it externally sent to the block execution?

[dankrad]

There is another constant TARGET_BLOB_GAS_PER_BLOCK that is explicitly set by the EIP, do we want to turn this constant to be a half of the max value (computable on the fly) or make it externally sent to the block execution?

I think I would add the target as well. Otherwise we have to set the target to be 1/2 of the limit, but being able to configure the target independently gives us more flexibility in the future.
In the header, I believe we only need the target, not the max, so it wouldn't add another field to the block header.

[dankrad]

An alternative that may be more elegant and lets us remove some fields from the header: Only add the current blob gas price (to both execution header and the execution API).

The consensus layer already has all the information to compute the correct blob gas price.

[g11tech]

I would favour maxBlobGasLimit (with target half of limit and price calc moved to something like this: https://github.com/ethereum/EIPs/pull/8343/files#diff-41459deea2791d37cd24d9139a73beacd8e7bad2aaaa194db237754a0b8951d6R56) and letting EL compute price using parent's excess blob gas. CL telling blob gas price and effectively moving gas calc to the CL breaks separation of concerns.

Its (CL price) also not that ePBS compatible (it can still be done with extra bookkeeping) where payloads can be "missed" with beacon payloads still being part of chain (because of decoupling between beacon block and payload) and also with some (draft) EIPs which allows for gas compensation for missed blocks so that constant throughput can be targetted.

[mkalinin]

CL telling blob gas price and effectively moving gas calc to the CL breaks separation of concerns

If blobGasPrice computation depends on the blobGasLimit/2 which is moved to the CL it is already breaking the separation of concerns, isn’t it?

I am wondering if it is important for EL to know the blobGasPrice of the next not yet built block? If it is important for any reason then EL must know blobGasLimit and other params involved into the blobGasPrice computation for future blocks before CL sends fcU with the payload attributes to build the next block. If it is not important then I don’t see a reason for why EL can’t get the blobGasPrice from the CL when it is required to build a block.

Regardless of the blobGasPrice calculation, EL block should have blobGasPrice and blobGasLimit to allow EL block validation to be independent from CL. During the optimistic sync there are edge cases when EL does not receive newPayload for each block it is processing -- for such cases EL block validation must be self-contained.

[mkalinin]

It seems to me that max_blobs_per_block is implicitly validated via versioned hashes validation run by EL, isn’t it? So that would mean we don’t need to make blobGasLimit as a part of EL block as the versioned hashes validation is triggered by CL for each payload.

[mkalinin]

We already have the following checks in the process_execution_payload:

# [New in Deneb:EIP4844] Verify commitments are under limit
assert len(body.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK

# Verify the execution payload is valid
# [Modified in Deneb:EIP4844] Pass `versioned_hashes` to Execution Engine
# [Modified in Deneb:EIP4788] Pass `parent_beacon_block_root` to Execution Engine
versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments]
assert execution_engine.verify_and_notify_new_payload(
    NewPayloadRequest(
        execution_payload=payload,
        versioned_hashes=versioned_hashes,
        parent_beacon_block_root=state.latest_block_header.parent_root,
    )
)

So the MAX_BLOBS_PER_BLOCK is already implicitly validated by the CL by the mean of matching the actual versioned_hashes with the expected ones. Why the payload validation process requires MAX_BLOBS_PER_BLOCK to be sent to EL explicitly?

[ralexstokes]

right, ive been thinking about this wrt to how we want to handle this feature

as you point out, the current check for "max blob gas per block" in the EL is redundant when the CL and EL are moving in lock-step via the call to verify_and_notify_new_payload

we still have a concern here around optimistic sync and I think the simplest thing would be to include the blob limits in the EL header so that an execution layer block is self-contained

I intend to bring up on the CL call Thursday, I think we need to agree on the exact semantics of optimistic sync as I could imagine two cases:

1. Make EL blocks self-contained, for verification purposes during syncing
2. Have the CL drive the EL via this check it will already be doing and skip changing the EL block as all (beyond just removing the EL validation to ensure the max/target number of blobs)

[mkalinin]

Versioned hashes check is similar to block hash validation (both checks the block structure), and the spec says that this check must be run instantly even during the sync process as it does not involve the world state. CL clients either send every payload to EL or run versioned hashes and block hash validations on their own if EL is syncing. So this check is executed over each payload, and the MAX_BLOBS_PER_BLOCK shouldn’t be checked by the EL explicitly. So the block building would be the only place where this parameter must be passed to EL.

[ralexstokes]

yep! I ended up reviewing this as well and it seems the maximum check on the EL is currently redundant

I think we remove the check for max from the EL and send over the target

I'll work on tidying up this PR and make an EIP to add the target value to the block header

[ralexstokes]

updated the top comment description with the latest thinking here, along with a link to a WIP EIP; please see comment and the WIP EIP for the latest thinking around this feature

[hwwhww]

@ralexstokes could we merge it now, and then I can change #4023 accordingly?

[g11tech]

lgtm

[jtraglia]

LGTM 👍