EIP-7732: Enshrined Proposer-Builder Separation #3828

potuz · 2024-07-02T12:12:16Z

This PR implements the necessary changes to separate the processing of an Ethereum block into a consensus and an execution part. It also implements an auction mechanism for a consensus proposer to choose the proposer of the execution part of the block (called a builder in the documentation).

The full design notes are included in https://hackmd.io/@potuz/rJ9GCnT1C
Forkchoice annotated spec can be found in https://hackmd.io/@potuz/SJdXM43x0
Validator guide annotated spec can be found in https://hackmd.io/@ttsao/epbs-annotated-validator

Fix epbs consensus spec to be executable

The damned linter complains and there's no problem having it This reverts commit b7461ac.

hwwhww · 2024-07-04T17:01:30Z

specs/_features/eip7732/beacon-chain.md

+    for idx in range(committees_per_slot):
+        beacon_committee = get_beacon_committee(state, slot, CommitteeIndex(idx))
+        validator_indices += beacon_committee[:members_per_committee]
+    return validator_indices


The return type doesn't match.

You can cast in the end:

Suggested change

return validator_indices

return Vector[ValidatorIndex, PTC_SIZE](validator_indices)

or just change the function notation to return Sequence[ValidatorIndex].

hwwhww · 2024-08-06T16:19:42Z

I'm so sorry for keeping you pending 😭 It was mainly because the release cycle got delayed, and I didn't want this huge PR to expand the scale of the v1.5.0-alpha.4.

I wanted to give it a proper review, but now I think it best to have a post-review rather than blocking it.

etan-status · 2024-08-26T08:51:50Z

specs/_features/eip7732/beacon-chain.md

+        state.latest_block_header.state_root = previous_state_root
+
+    # Verify consistency with the beacon block
+    assert envelope.beacon_block_root == hash_tree_root(state.latest_block_header)


This introduces an extra round trip because it means that SignedExecutionPayloadEnvelope can only be published after SignedBeaconBlock is available. For a pure consistency check, the block_hash check may possibly be sufficient.

From the P2P gossip's perspective, I won't be able to import or forward the SignedExecutionPayloadEnvelope until I have seen the beacon block that includes the header referencing the SignedExecutionPayloadEnvelope.

An envelope may also refer to same block hash with different block roots

Why is that a problem? If multiple BeaconBlock refer to the same ExecutionPayload, why do we have to propagate the ExecutionPayload multiple times?

Note it's only possible with equivocation (or heavy branching), because the ExecutionPayload hashes over the timestamp (so is based on slot) and parent_block_root (so is based on the branch)

etan-status · 2024-08-26T08:53:06Z

specs/_features/eip7732/beacon-chain.md

+    assert envelope.builder_index == committed_header.builder_index
+    assert committed_header.blob_kzg_commitments_root == hash_tree_root(envelope.blob_kzg_commitments)
+
+    if not envelope.payload_withheld: 


As this is a cooperative mechanism, wondering how the performance of the system is impacted if the entire message is withheld (as in, no SignedExecutionPayloadEnvelope). If the performance is still okay, not sure if the explicit path to withhold the payload is necessary.

etan-status · 2024-08-26T08:58:20Z

specs/_features/eip7732/beacon-chain.md

+
+    # Verify the state root
+    if verify: 
+        assert envelope.state_root == hash_tree_root(state)


Is this check required? Also, state is already updated, and is not reverted if verification fails.

Without checking, how do we ensure the processing of execution payload is correct? ie latest_block_hash and latest_full_slot

The envelope.payload.state_root is the EL post-state root and used to check that the execution payload processed correctly.

envelope.state_root refers to some CL post-state. It is not linked to the execution payload.

Hm. Here envelope.state_root is the EL post state root used to check that the execution payload processed correctly
envelope is signed_envelope.message

no. the EL post state root is envelope.payload.state_root.

envelope.state_root is hash_tree_root(state) (of the beacon state -- EL does not use htr)

I misspoke. I meant the EL post-state root as the state root after processing the execution payload envelope, but that's not accurate. What I intended to say is that we need to check signed_envelope.message.state_root == hash_tree_root(state) to ensure that the post-state aligns. This is similar to checking block.state_root == hash_tree_root(state) after running the state transition for the beacon state.

etan-status · 2024-08-26T10:33:42Z

specs/_features/eip7732/fork.md

+        # ePBS
+        latest_block_hash=pre.latest_execution_payload_header.block_hash,  # [New in EIP-7732]
+        latest_full_slot=pre.slot,  # [New in EIP-7732]
+        latest_withdrawals_root=Root(),  # [New in EIP-7732]


Won't this trigger bugs lateron in process_execution_payload's withdrawals root check?

etan-status · 2024-08-26T10:56:53Z

specs/_features/eip7732/beacon-chain.md

+        # Verify consistency of the parent hash with respect to the previous execution payload
+        assert payload.parent_hash == state.latest_block_hash
+        # Verify prev_randao
+        assert payload.prev_randao == get_randao_mix(state, get_current_epoch(state))


How does this work?

At slot N, builders build payloads based on prev_randao up through the latest BeaconBlock (so, up through slot N-1).

Then, the proposer picks one of the SignedExecutionPayloadHeader (which has a block_hash based on get_randao_mix up through N-1), and puts it into a BeaconBlock.

Then, validators apply that BeaconBlock, advancing get_randao_mix in the state.

So, later, when the payload is actually revealed, get_randao_mix now includes info up through slot N, but at the time that block_hash was computed it was only up through slot N-1.

I doubt the correctness of the logic here, or maybe need additional clarification on how this is intended.

This is no different than builders built block for mev-boost today no? It's the same check we have process_execution_payload and the builder just takes the head state and uses get_randao_mix(state, get_current_epoch(state))

Before ePBS, process_randao is done after process_execution_payload.

With ePBS, RANDAO mix is already updated at time that execution payload is processed, so validation will fail.

def process_block(state: BeaconState, block: BeaconBlock) -> None: process_block_header(state, block) process_withdrawals(state, block.body.execution_payload) process_execution_payload(state, block.body, EXECUTION_ENGINE) process_randao(state, block.body) process_eth1_data(state, block.body) process_operations(state, block.body) process_sync_aggregate(state, block.body.sync_aggregate)

Ah! I'm convinced you are right. Thanks! I also let potuz know, he is looking at it now

etan-status · 2024-08-27T18:26:35Z

specs/_features/eip7732/beacon-chain.md

+
+#### Modified `get_attesting_indices`
+
+`get_attesting_indices` is modified to ignore PTC votes


Won't this prevent finality in 2 epochs on the minimal preset in tests?

Like, the PTC is still counted in the denominator, just not in the numerator

etan-status · 2024-08-27T18:27:06Z

specs/_features/eip7732/beacon-chain.md

+    # Operations
+    proposer_slashings: List[ProposerSlashing, MAX_PROPOSER_SLASHINGS]
+    attester_slashings: List[AttesterSlashing, MAX_ATTESTER_SLASHINGS]
+    attestations: List[Attestation, MAX_ATTESTATIONS]


Why not MAX_ATTESTATIONS_ELECTRA? Do we need 128 once more?

No, I think this is a bug

etan-status · 2024-08-27T18:51:43Z

presets/mainnet/eip-7732.yaml

+# Execution
+# ---------------------------------------------------------------
+# 2**9 (= 512)
+PTC_SIZE: 512


In the tests, the number of initial validators is SLOTS_PER_EPOCH * 8. so, 32 * 8 = 256 in mainnet. The PTC_SIZE here is double than the amount of validators. This, combined with attestations by the PTC not counting, smells like potential finality issues in tests.

ya, tests will fail for sure, we either decrease the ptc size for minimal or have to sample ptc like we do in sync committee, it's still an open question

should the PTC even influence justification / finality? it seems orthogonal to me, execution has nothing to do with consensus.

as in, why is the PTC even having an effect on finality weighing

From the writing spec's perspective, it's easier to borrow PTC from the beacon attestation committee. 512 is a non-negotiable number, but as you said, this causes issues for minimal

on mainnet preset, the tests run with 256 validators (32*8)

etan-status · 2024-08-27T18:52:51Z

presets/minimal/eip-7732.yaml

+# Execution
+# ---------------------------------------------------------------
+# 2**1(= 2)
+PTC_SIZE: 2


Here we have 2 out of 64 validators. But because validators are split up into SLOTS_PER_EPOCH committees, only 8 of them vote on each slot. So, 25% is PTC which does not count in the numerator but counts in the denominator. If there is some randomness involves, may slow down finality (but not fully break it at least).

etan-status · 2024-08-27T19:08:09Z

specs/_features/eip7732/beacon-chain.md

+
+```python
+def process_withdrawals(state: BeaconState) -> None:
+    # return early if the parent block was empty


This has an interesting scenario which may warrant a test:

Balances are decreased in the CL but are only increased in the EL at a later stage once the payload is revealed.

Assume that revealing fails for a while for some reason, but the CL still continues. Meaning the balances have been decreased in the CL but withdrawals are still pending.

Assume this goes on for like 20 minutes and the chain finalizes.

Someone doing checkpoint sync from the finalized checkpoint will now no longer be able to checkpoint sync, because they do not know the actual withdrawals from get_expected_withdrawals as they have already been processed in the CL. Because they only have data from finalized checkpoint onward, they are now unable to send the expected withdrawals to an EL and will not be able to propose any blocks until someone else who still has the data recovers the situation and manages to properly reveal a payload.

etan-status · 2024-08-27T19:15:35Z

specs/_features/eip7732/fork.md

+        pending_consolidations=pre.pending_consolidations,
+        # ePBS
+        latest_block_hash=pre.latest_execution_payload_header.block_hash,  # [New in EIP-7732]
+        latest_full_slot=pre.slot,  # [New in EIP-7732]


Why pre.slot? No BeaconBlock has been received for this slot yet, and there may have been empty blocks before that.

etan-status · 2024-08-27T19:24:51Z

specs/_features/eip7732/beacon-chain.md

+
+```python
+def is_parent_block_full(state: BeaconState) -> bool:
+    return state.latest_execution_payload_header.block_hash == state.latest_block_hash


In tests, block_hash may be zero hash, and then this may have false positives.

good point!

etan-status · 2024-08-27T19:57:37Z

specs/_features/eip7732/beacon-chain.md

+
+        for_ops(payload.deposit_requests, process_deposit_request)
+        for_ops(payload.withdrawal_requests, process_withdrawal_request)
+        for_ops(payload, process_consolidation_request)


This should probably be payload.consolidation_requests

potuz mentioned this pull request Jul 2, 2024

Add EIP: Enshrined Proposer-Builder Separation ethereum/EIPs#8711

Merged

potuz force-pushed the epbs_cl_repo branch from 9d10fd2 to 78e15ec Compare July 2, 2024 13:19

terencechain mentioned this pull request Jul 2, 2024

Add a project proposal of Jihoon Song eth-protocol-fellows/cohort-five#151

Closed

potuz changed the title ~~Ensrhined Proposer-Builder Separation~~ EIP-7732: Ensrhined Proposer-Builder Separation Jul 3, 2024

potuz force-pushed the epbs_cl_repo branch from 3eefb83 to 4160d2f Compare July 3, 2024 12:59

potuz changed the title ~~EIP-7732: Ensrhined Proposer-Builder Separation~~ EIP-7732: Enshrined Proposer-Builder Separation Jul 3, 2024

potuz and others added 24 commits July 8, 2024 14:36

beacon chain changes

104fbc7

Forkchoice changes

673c75c

Honest validator guide

971c57c

Honest Builder Guide

614e14e

P2P changes

cd7dbef

Fork logic

6d6ef7b

Add comment about attestations and sync committee duties

0d082fd

Fix epbs consensus spec to be executable (#10)

6e02cb4

Fix epbs consensus spec to be executable

modify INTEVALS_PER_SLOT

a01f124

Verify withdrawals only when present

b7b1de9

small typo fixes (#12)

51e47ff

doctoc

25a0fdc

Add gas_limit to the execution payload header

ed7f17e

codespell

95dd081

use boolean instead of bool in forkchoice

752a804

add fake value to KZG indices

4997588

Fix python execution 1

ad8d71d

doctoc

2a43ce5

Fix python execution 2

4500eab

Fix python execution 3

d7a199f

Fix epbs pre fork (#13)

5013e57

add type annotation

181ce5e

use definitive EIP number

a11ffb5

Add deprecation helpers in spec builder

b230a42

potuz force-pushed the epbs_cl_repo branch from e56a199 to 9274a66 Compare July 8, 2024 17:42

potuz added 9 commits July 8, 2024 15:12

use List with limit to hash withdrawals

f97f0b4

Verify kzg-commitments length with the payload

096e564

Fix block_header.state_root and forkchoice filtering

3529b46

doctoc

4b43b96

remove spurious config prefix

5bb25c8

Restore proposer_boost timeliness calculation

8c9514e

Remove block_timeliness from the store

b7461ac

Advance best child with empty until it is higher than every children

d0fad1f

solve ties by block slot first

0c9ca6d

potuz mentioned this pull request Jul 24, 2024

EIP-7732 current fork spectests #3854

Open

potuz added 2 commits July 24, 2024 16:02

Revert "Remove block_timeliness from the store"

b082140

The damned linter complains and there's no problem having it This reverts commit b7461ac.

Typo in validator.md

74124f5

hwwhww approved these changes Aug 6, 2024

View reviewed changes

hwwhww merged commit 8f8ab03 into ethereum:dev Aug 6, 2024
18 checks passed

hwwhww added the general:enhancement New feature or request label Aug 6, 2024

etan-status reviewed Aug 26, 2024

View reviewed changes

etan-status reviewed Aug 27, 2024

View reviewed changes

	return validator_indices
	return Vector[ValidatorIndex, PTC_SIZE](validator_indices)


		#### Modified `get_attesting_indices`

		`get_attesting_indices` is modified to ignore PTC votes

EIP-7732: Enshrined Proposer-Builder Separation #3828

EIP-7732: Enshrined Proposer-Builder Separation #3828

Conversation

potuz commented Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

hwwhww commented Aug 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etan-status Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etan-status Aug 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etan-status Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etan-status Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

potuz commented Jul 2, 2024 •

edited

Loading

etan-status Aug 27, 2024 •

edited

Loading

etan-status Aug 26, 2024 •

edited

Loading

etan-status Aug 27, 2024 •

edited

Loading

etan-status Aug 27, 2024 •

edited

Loading