Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eip7251: Switch to compounding when consolidating with source==target #3918

Merged
merged 11 commits into from
Oct 3, 2024
73 changes: 56 additions & 17 deletions specs/electra/beacon-chain.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
- [Deposit requests](#deposit-requests)
- [New `process_deposit_request`](#new-process_deposit_request)
- [Execution layer consolidation requests](#execution-layer-consolidation-requests)
- [New `is_valid_switch_to_compounding_request`](#new-is_valid_switch_to_compounding_request)
- [New `process_consolidation_request`](#new-process_consolidation_request)
- [Testing](#testing)

Expand Down Expand Up @@ -683,9 +684,8 @@ def initiate_validator_exit(state: BeaconState, index: ValidatorIndex) -> None:
```python
def switch_to_compounding_validator(state: BeaconState, index: ValidatorIndex) -> None:
validator = state.validators[index]
if has_eth1_withdrawal_credential(validator):
validator.withdrawal_credentials = COMPOUNDING_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:]
queue_excess_active_balance(state, index)
validator.withdrawal_credentials = COMPOUNDING_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:]
queue_excess_active_balance(state, index)
```

#### New `queue_excess_active_balance`
Expand Down Expand Up @@ -928,8 +928,6 @@ def process_pending_consolidations(state: BeaconState) -> None:
if source_validator.withdrawable_epoch > next_epoch:
break

# Churn any target excess active balance of target and raise its max
switch_to_compounding_validator(state, pending_consolidation.target_index)
# Move active balance to target. Excess balance is withdrawable.
active_balance = get_active_balance(state, pending_consolidation.source_index)
decrease_balance(state, pending_consolidation.source_index, active_balance)
Expand Down Expand Up @@ -1225,14 +1223,6 @@ def apply_deposit(state: BeaconState,
state.pending_balance_deposits.append(
PendingBalanceDeposit(index=index, amount=amount)
) # [Modified in Electra:EIP7251]
# Check if valid deposit switch to compounding credentials
if (
is_compounding_withdrawal_credential(withdrawal_credentials)
and has_eth1_withdrawal_credential(state.validators[index])
and is_valid_deposit_signature(pubkey, withdrawal_credentials, amount, signature)
):
switch_to_compounding_validator(state, index)

```

###### New `is_valid_deposit_signature`
Expand Down Expand Up @@ -1404,13 +1394,62 @@ def process_deposit_request(state: BeaconState, deposit_request: DepositRequest)

##### Execution layer consolidation requests

###### New `is_valid_switch_to_compounding_request`

```python
def is_valid_switch_to_compounding_request(
state: BeaconState,
consolidation_request: ConsolidationRequest
) -> bool:
# Switch to compounding requires source and target be equal
if consolidation_request.source_pubkey != consolidation_request.target_pubkey:
return False

# Verify pubkey exists
source_pubkey = consolidation_request.source_pubkey
validator_pubkeys = [v.pubkey for v in state.validators]
if source_pubkey not in validator_pubkeys:
return False

source_validator = state.validators[ValidatorIndex(validator_pubkeys.index(source_pubkey))]

# Verify request has been authorized
if source_validator.withdrawal_credentials[12:] != consolidation_request.source_address:
return False

# Verify source withdrawal credentials
if not has_eth1_withdrawal_credential(source_validator):
return False
jtraglia marked this conversation as resolved.
Show resolved Hide resolved
jtraglia marked this conversation as resolved.
Show resolved Hide resolved

# Verify the source is active
current_epoch = get_current_epoch(state)
if not is_active_validator(source_validator, current_epoch):
return False

# Verify exit for source has not been initiated
if source_validator.exit_epoch != FAR_FUTURE_EPOCH:
return False

return True
```

###### New `process_consolidation_request`

```python
def process_consolidation_request(
state: BeaconState,
consolidation_request: ConsolidationRequest
) -> None:
if is_valid_switch_to_compounding_request(state, consolidation_request):
validator_pubkeys = [v.pubkey for v in state.validators]
request_source_pubkey = consolidation_request.source_pubkey
source_index = ValidatorIndex(validator_pubkeys.index(request_source_pubkey))
switch_to_compounding_validator(state, source_index)
ralexstokes marked this conversation as resolved.
Show resolved Hide resolved
return

# Verify that source != target, so a consolidation cannot be used as an exit.
if consolidation_request.source_pubkey == consolidation_request.target_pubkey:
return
# If the pending consolidations queue is full, consolidation requests are ignored
if len(state.pending_consolidations) == PENDING_CONSOLIDATIONS_LIMIT:
return
Expand All @@ -1431,10 +1470,6 @@ def process_consolidation_request(
source_validator = state.validators[source_index]
target_validator = state.validators[target_index]

# Verify that source != target, so a consolidation cannot be used as an exit.
if source_index == target_index:
return

# Verify source withdrawal credentials
has_correct_credential = has_execution_withdrawal_credential(source_validator)
is_correct_source_address = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets do this later but I think we can also factor out a helper like is_valid_consolidation_request

Expand Down Expand Up @@ -1470,6 +1505,10 @@ def process_consolidation_request(
source_index=source_index,
target_index=target_index
))

# Churn any target excess active balance of target and raise its max
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep this in process_pending_consolidations?

so drop this addition here, and then keep the deletion up above

# Churn any target excess active balance of target and raise its max
switch_to_compounding_validator(state, pending_consolidation.target_index)

seems easier to reason about if the effects are all in one place

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of switching to compounding in one place. So, the consolidation request has an additional semantics that takes an effect disregarding whether it is a genuine consolidation or merely a switch request. The switch also adds pending balance to the activation queue which creates additional dependency between two process epoch routines: process_pending_consolidations and process_pending_balance_deposits, having no additional dependency is cleaner.

if has_eth1_withdrawal_credential(target_validator):
switch_to_compounding_validator(state, target_index)
```

## Testing
Expand Down
Loading