fix withdrawals, check only in the CL

specs/_features/epbs/beacon-chain.md

@@ -171,7 +171,6 @@ class BeaconBlockBody(Container):
     signed_execution_payload_header: SignedExecutionPayloadHeader  # [New in ePBS]
     bls_to_execution_changes: List[SignedBLSToExecutionChange, MAX_BLS_TO_EXECUTION_CHANGES]
     blob_kzg_commitments: List[KZGCommitment, MAX_BLOB_COMMITMENTS_PER_BLOCK]
-    withdrawals: List[Withdrawal, MAX_WITHDRAWALS_PER_PAYLOAD]  # [New in ePBS]
 ```
 
 #### `ExecutionPayload`
@@ -280,6 +279,7 @@ class BeaconState(Container):
     historical_summaries: List[HistoricalSummary, HISTORICAL_ROOTS_LIMIT]
     # PBS
     current_signed_execution_payload_header: SignedExecutionPayloadHeader # [New in ePBS]
+    last_withdrawals: List[Withdrawal, MAX_WITHDRAWALS_PER_PAYLOAD] # [New in ePBS]
 ```
 
 ## Helper functions
@@ -349,19 +349,6 @@ class NewInclusionListRequest(object):
 
 #### Engine APIs
 
-#### New `notify_withdrawals` 
-
-TODO: Can we send this with FCU as parameters instead of a new engine method reorg resistant? We need to remove withdrawals from the payload attributes now. 
-
-```python
-def notify_withdrawals(self: ExecutionEngine, withdrawals: NewWithdrawalsRequest) -> None
-    """
-    This call informs the EL that the next payload which is a grandchild of the current ``parent_block_hash``
-    needs to include the listed withdrawals that have been already fulfilled in the CL
-    """
-    ...
-```
-
 #### New `notify_new_inclusion_list`
 
 ```python
@@ -384,84 +371,42 @@ def notify_new_inclusion_list(self: ExecutionEngine,
 ```python
 def process_block(state: BeaconState, block: BeaconBlock) -> None:
     process_block_header(state, block)
-    process_withdrawals(state, block.body) [Modified in ePBS]
+    process_withdrawals(state) [Modified in ePBS]
     process_execution_payload_header(state, block.body.execution_payload_header) # [Modified in ePBS]
     process_randao(state, block.body)
     process_eth1_data(state, block.body)
     process_operations(state, block.body)  # [Modified in ePBS]
     process_sync_aggregate(state, block.body.sync_aggregate)
 ```
 
-#### Modified `get_expected_withdrawals`
-**Note:** the function `get_expected_withdrawals` is modified to return no withdrawals if the parent block was empty.
-TODO: Still need to include the MaxEB changes
+#### Modified `process_withdrawals`
+**Note:** TODO: This is modified to take only the State as parameter as they are deterministic. Still need to include the MaxEB changes
 
 ```python
-def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]:
+def process_withdrawals(state: BeaconState) -> None:
     ## return early if the parent block was empty
-    withdrawals: List[Withdrawal] = []
     if state.current_signed_execution_payload_header.message != state.latest_execution_payload_header:
-        return withdrawals
-    epoch = get_current_epoch(state)
-    withdrawal_index = state.next_withdrawal_index
-    validator_index = state.next_withdrawal_validator_index
-    bound = min(len(state.validators), MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP)
-    for _ in range(bound):
-        validator = state.validators[validator_index]
-        balance = state.balances[validator_index]
-        if is_fully_withdrawable_validator(validator, balance, epoch):
-            withdrawals.append(Withdrawal(
-                index=withdrawal_index,
-                validator_index=validator_index,
-                address=ExecutionAddress(validator.withdrawal_credentials[12:]),
-                amount=balance,
-            ))
-            withdrawal_index += WithdrawalIndex(1)
-        elif is_partially_withdrawable_validator(validator, balance):
-            withdrawals.append(Withdrawal(
-                index=withdrawal_index,
-                validator_index=validator_index,
-                address=ExecutionAddress(validator.withdrawal_credentials[12:]),
-                amount=balance - MAX_EFFECTIVE_BALANCE,
-            ))
-            withdrawal_index += WithdrawalIndex(1)
-        if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD:
-            break
-        validator_index = ValidatorIndex((validator_index + 1) % len(state.validators))
-    return withdrawals
-```
-
-#### Modified `process_withdrawals`
-**Note:** TODO: This is modified to take a `BeaconBlockBody`. Still need to include the MaxEB changes
-
-```python
-def process_withdrawals(state: BeaconState, body: BeaconBlockBody) -> None:
-    withdrawals = body.withdrawals
-    expected_withdrawals = get_expected_withdrawals(state)
-    assert len(withdrawals) == len(expected_withdrawals)
-
-    for expected_withdrawal, withdrawal in zip(expected_withdrawals, withdrawals):
-        assert withdrawal == expected_withdrawal
+        return 
+    withdrawals = get_expected_withdrawals(state)
+    state.last_withdrawals = withdrawals
+    for withdrawal in withdrawals:
         decrease_balance(state, withdrawal.validator_index, withdrawal.amount)
 
     # Update the next withdrawal index if this block contained withdrawals
-    if len(expected_withdrawals) != 0:
-        latest_withdrawal = expected_withdrawals[-1]
+    if len(withdrawals) != 0:
+        latest_withdrawal = withdrawals[-1]
         state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1)
 
     # Update the next validator index to start the next withdrawal sweep
-    if len(expected_withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD:
+    if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD:
         # Next sweep starts after the latest withdrawal's validator index
-        next_validator_index = ValidatorIndex((expected_withdrawals[-1].validator_index + 1) % len(state.validators))
+        next_validator_index = ValidatorIndex((withdrawals[-1].validator_index + 1) % len(state.validators))
         state.next_withdrawal_validator_index = next_validator_index
     else:
         # Advance sweep by the max length of the sweep if there was not a full set of withdrawals
         next_index = state.next_withdrawal_validator_index + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
         next_validator_index = ValidatorIndex(next_index % len(state.validators))
         state.next_withdrawal_validator_index = next_validator_index
-    # Inform the EL of the processed withdrawals
-    hash = body.signed_execution_payload_header.message.parent_block_hash
-    execution_engine.notify_withdrawals(NewWithdrawalsRequest(withdrawals=withdrawals, parent_block_hash = hash))
 ```
 
 #### New `verify_execution_payload_header_signature`
@@ -517,6 +462,10 @@ def process_execution_payload(state: BeaconState, signed_envelope: SignedExecuti
     hash = hash_tree_root(payload)
     previous_hash = hash_tree_root(state.current_signed_execution_payload_header.message)
     assert hash == previous_hash
+    # Verify the withdrawals
+    assert len(state.last_withrawals) == len(payload.withdrawals)
+    for withdrawal, payload_withdrawal in zip(state.last_withdrawals, payload.withdrawals):
+        assert withdrawal == payload_withdrawal 
     # Verify the execution payload is valid
     versioned_hashes = [kzg_commitment_to_versioned_hash(commitment) for commitment in body.blob_kzg_commitments]
     assert execution_engine.verify_and_notify_new_payload(

specs/_features/epbs/design.md

@@ -28,14 +28,6 @@ Payments are processed unconditionally when processing the signed execution payl
 
 ## Withdrawals
 
-Withdrawals cannot be fulfilled at the same time in the CL and the EL now: if they are included in the Execution payload for slot N, the consensus block for the same slot may have invalidated the validator balances. The new mechanism is as follows
-- Proposer for block N includes the withdrawals in his CL block and they are immediately processed in the CL
-- The execution payload for N mints ETH in the EL for the already burnt ETH in the CL during slot N-1. 
-- If the slot N-1 was empty, the proposer for N does not advance withdrawals and does not include any new withdrawals, so that each EL block mints a maximum of `MAX_WITHDRAWALS_PER_PAYLOAD` withdrawals. 
-- There is a new engine endpoint that notifies the EL of the next withdrawals that need to be included. The EL needs to verify the validity that the execution payload includes those withdrawals in the next blockhash.
+Withdrawals are deterministic on the beacon state, so on a consensus layer block processing, they are immediately processed, then later when the payload appears we verify that the withdrawals in the payload agree with the already fulfilled withdrawals in the CL.  
 
-### Examples
-
-(N-2, Full) -- (N-1: Full, withdrawals 0--15) -- (N: Empty, withdrawals 16--31) -- (N+1, does not send withdrawals, Full: excecutes withdrawals 0--15 since it is a child of the blockhash from N-1, thus grandchild of the blockhash of N-2) -- (N+2, withdrawals 32--47, Full: excecutes withdrawals 16--31) 
-
-(N-2, Full) - (N-1: Full withdrawals 0--15) -- (N: Empty, withdrawals 16--31) -- (N+1: Empty, no withdrawals) -- (N+2: no withdrawals, Full: executes withdrawals 0--15). 
+So when importing the CL block for slot N, we process the expected withdrawals at that slot. We save the list of paid withdrawals to the beacon state. When the payload for slot N appears, we check that the withdrawals correspond to the saved withdrawals. If the payload does not appear, the saved withdrawals remain, so any future payload has to include those.