verkle trees: add 2 fields to ExecutionPayload

[gballet]

Description

With the verge, proofs and witness of the pre-state of an (EL) block execution are included in the blocks. In order to build a PoS testnet that provides proofs in blocks, it would be great if it were possible to create a branch with the required fields to be added to the ExecutionPayload.

The ExecutionPayload needs to have two more, optional fields:

• VerkleProof, a byte array of variable length
• VerkleKeyVals, an array of (key, value) pairs, in which the key is a 32-byte vector, and value is a byte vector of length either 0 (missing value) or 32.

At the moment, there is no need to support forking as for this testnet, verkle trees are used since the genesis.

If present, these two fields should be propagated over the network with the rest of the ExecutionPayload so that other clients can inject these fields in their verkle-enabled EL.

[ajsutton]

hmm, I'm not sure I have enough info to actually implement this. ExecutionPayload is an SSZ object so there's no way to add an optional field - there hasn't be some default value (for a list or array that's usually just the empty list), and there has to be a maximum length on every type.

For execution payloads specifically we'd also need to know if these new fields are present on ExecutionPayloadHeader as well or if they're replaced with the hash tree root like transactions is.

[gballet]

Thank you for the feedback. 1. Both lists are arbitraty-length, but limiting them to a maximum of 75K each for now should be fine. 2. It should indeed be the empty list by default. 3. it can be hashed in the header, because without transactions, the proof is of limited use Thanks Guillaume

…

On Mon, 31 Oct 2022, 06:42 Adrian Sutton, ***@***.***> wrote: hmm, I'm not sure I have enough info to actually implement this. ExecutionPayload is an SSZ object so there's no way to add an optional field - there hasn't be some default value (for a list or array that's usually just the empty list), and there has to be a maximum length on every type. For execution payloads specifically we'd also need to know if these new fields are present on ExecutionPayloadHeader as well or if they're replaced with the hash tree root like transactions is. — Reply to this email directly, view it on GitHub <#6346 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAY7ANU3KD4QPNKGKIMZHWDWF5L4VANCNFSM6AAAAAARQEK57A> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ajsutton]

I've put up master...ajsutton:teku:verkle-trees which has this added in. There's nothing to test it with so hard to know if there's something I've missed though.

I wound up including the full verkle proof and verkle key val values in the ExecutionPayloadHeader because blinding and unblinding them would introduce quite a bit more complexity. In terms of actual implementation the key question is likely whether or not the verkle proof and key/vals should wind up in the BeaconState.latestPayloadHeader field which uses the ExecutionPayloadHeader type.

To make this fit within SSZ the two new fields wind up being defined as:

class ExecutionPayload(Container):
# Execution block header fields
    parent_hash: Hash32
    fee_recipient: ExecutionAddress  # 'beneficiary' in the yellow paper
    state_root: Bytes32
    receipts_root: Bytes32
    logs_bloom: ByteVector[BYTES_PER_LOGS_BLOOM]
    prev_randao: Bytes32  # 'difficulty' in the yellow paper
    block_number: uint64  # 'number' in the yellow paper
    gas_limit: uint64
    gas_used: uint64
    timestamp: uint64
    extra_data: ByteList[MAX_EXTRA_DATA_BYTES]
    base_fee_per_gas: uint256
    # Extra payload fields
    block_hash: Hash32  # Hash of execution block
    verkle_proofs: ByteList[75000],                              # New 
    verkle_key_vals, List[VerkleKeyVal, 75000],          # New
    transactions: List[Transaction, MAX_TRANSACTIONS_PER_PAYLOAD]


class VerkleKeyVal:
    key: Bytes32,
    value: ByteList[32]

Notably the value is allowing any length byte array up to 32 bytes because you can't express either 0 or 32 bytes in SSZ without using a union (and unions aren't currently used in the beacon chain so add quite a bit of complexity to support).

[gballet]

@ajsutton thank you for this, there is now a branch to test: gballet/go-ethereum#136. We will add your branch to our tests when they are ready and come back with more feedback.

Incidentally, there has been a proposal for the SSZ encoding a few months ago: https://notes.ethereum.org/Si5tEWlMTYerkhG3fOIAMQ It hasn't been used so far because it requires the post-state, which isn't really practical to provide. The other problem is that it uses Unions. I could update the PR to return the fields needed to build these SSZ structures, but I would like to know your feedback on this, maybe it would be possible to tweak these structures to make it easier for a CL to implement?

[tbenr]

Spec PR
ethereum/consensus-specs#3230

[lucassaldanha]

All the code is on a branch, waiting until it is time to merge it into master.