Switch to compounding when consolidating with source==target

Author: mkalinin

specs/electra/beacon-chain.md

@@ -61,7 +61,6 @@
     - [Modified `get_next_sync_committee_indices`](#modified-get_next_sync_committee_indices)
   - [Beacon state mutators](#beacon-state-mutators)
     - [Modified `initiate_validator_exit`](#modified-initiate_validator_exit)
-    - [New `switch_to_compounding_validator`](#new-switch_to_compounding_validator)
     - [New `queue_excess_active_balance`](#new-queue_excess_active_balance)
     - [New `queue_entire_balance_and_reset_validator`](#new-queue_entire_balance_and_reset_validator)
     - [New `compute_exit_epoch_and_update_churn`](#new-compute_exit_epoch_and_update_churn)
@@ -678,16 +677,6 @@ def initiate_validator_exit(state: BeaconState, index: ValidatorIndex) -> None:
     validator.withdrawable_epoch = Epoch(validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY)
 ```
 
-#### New `switch_to_compounding_validator`
-
-```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)
-```
-
 #### New `queue_excess_active_balance`
 
 ```python
@@ -928,8 +917,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)
@@ -1225,14 +1212,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`
@@ -1431,10 +1410,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 = (
@@ -1459,12 +1434,22 @@ def process_consolidation_request(
     if target_validator.exit_epoch != FAR_FUTURE_EPOCH:
         return
 
+    # Churn any target excess active balance of target and raise its max
+    if has_eth1_withdrawal_credential(target_validator):
+        state.validators[target_index].withdrawal_credentials = (
+            COMPOUNDING_WITHDRAWAL_PREFIX + target_validator.withdrawal_credentials[1:])
+        queue_excess_active_balance(state, target_index)
+
+    # Verify that source != target, so a consolidation cannot be used as an exit.
+    if source_index == target_index:
+        return
+
     # Initiate source validator exit and append pending consolidation
-    source_validator.exit_epoch = compute_consolidation_epoch_and_update_churn(
+    state.validators[source_index].exit_epoch = compute_consolidation_epoch_and_update_churn(
         state, source_validator.effective_balance
     )
-    source_validator.withdrawable_epoch = Epoch(
-        source_validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY
+    state.validators[source_index].withdrawable_epoch = Epoch(
+        state.validators[source_index].exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY
     )
     state.pending_consolidations.append(PendingConsolidation(
         source_index=source_index,

tests/core/pyspec/eth2spec/test/electra/block_processing/test_process_consolidation_request.py

@@ -395,7 +395,37 @@ def test_consolidation_balance_through_two_churn_epochs(spec, state):
     assert state.consolidation_balance_to_consume == expected_balance
 
 
-# Failing tests
+@with_electra_and_later
+@with_presets([MINIMAL], "need sufficient consolidation churn limit")
+@with_custom_state(
+    balances_fn=scaled_churn_balances_exceed_activation_exit_churn_limit,
+    threshold_fn=default_activation_threshold,
+)
+@spec_test
+@single_phase
+def test_source_equals_target_switches_to_compounding(spec, state):
+    current_epoch = spec.get_current_epoch(state)
+    source_index = spec.get_active_validator_indices(state, current_epoch)[0]
+
+    # Set source to eth1 credentials
+    source_address = b"\x22" * 20
+    set_eth1_withdrawal_credential_with_balance(
+        spec, state, source_index, address=source_address
+    )
+    # Make consolidation from source to source
+    consolidation = spec.ConsolidationRequest(
+        source_address=source_address,
+        source_pubkey=state.validators[source_index].pubkey,
+        target_pubkey=state.validators[source_index].pubkey,
+    )
+
+    # Check the the return condition
+    assert consolidation.source_pubkey == consolidation.target_pubkey
+
+    yield from run_consolidation_processing(
+        spec, state, consolidation, success=True
+    )
+
 
 @with_electra_and_later
 @with_presets([MINIMAL], "need sufficient consolidation churn limit")
@@ -405,7 +435,7 @@ def test_consolidation_balance_through_two_churn_epochs(spec, state):
 )
 @spec_test
 @single_phase
-def test_incorrect_source_equals_target(spec, state):
+def test_source_equals_target_switches_to_compounding_with_excess(spec, state):
     current_epoch = spec.get_current_epoch(state)
     source_index = spec.get_active_validator_indices(state, current_epoch)[0]
 
@@ -414,6 +444,8 @@ def test_incorrect_source_equals_target(spec, state):
     set_eth1_withdrawal_credential_with_balance(
         spec, state, source_index, address=source_address
     )
+    # Add excess balance
+    state.balances[source_index] = state.balances[source_index] + spec.EFFECTIVE_BALANCE_INCREMENT
     # Make consolidation from source to source
     consolidation = spec.ConsolidationRequest(
         source_address=source_address,
@@ -425,10 +457,12 @@ def test_incorrect_source_equals_target(spec, state):
     assert consolidation.source_pubkey == consolidation.target_pubkey
 
     yield from run_consolidation_processing(
-        spec, state, consolidation, success=False
+        spec, state, consolidation, success=True
     )
 
 
+# Failing tests
+
 @with_electra_and_later
 @with_presets([MINIMAL], "need sufficient consolidation churn limit")
 @with_custom_state(
@@ -815,36 +849,54 @@ def run_consolidation_processing(spec, state, consolidation, success=True):
         pre_exit_epoch_source = source_validator.exit_epoch
         pre_exit_epoch_target = target_validator.exit_epoch
         pre_pending_consolidations = state.pending_consolidations.copy()
+        pre_target_withdrawal_credentials = target_validator.withdrawal_credentials
+        pre_target_balance = state.balances[target_index]
     else:
         pre_state = state.copy()
 
     yield 'pre', state
     yield 'consolidation_request', consolidation
 
     spec.process_consolidation_request(state, consolidation)
+    # print(state.validators[target_index].withdrawal_credentials)
 
     yield 'post', state
 
     if success:
-        # Check source and target have execution credentials
+        # Check source has execution credentials
         assert spec.has_execution_withdrawal_credential(source_validator)
+        # Check target has compounding credentials
         assert spec.has_execution_withdrawal_credential(target_validator)
         # Check source address in the consolidation fits the withdrawal credentials
         assert source_validator.withdrawal_credentials[12:] == consolidation.source_address
-        # Check source and target are not the same
-        assert source_index != target_index
         # Check source and target were not exiting
         assert pre_exit_epoch_source == spec.FAR_FUTURE_EPOCH
         assert pre_exit_epoch_target == spec.FAR_FUTURE_EPOCH
-        # Check source is now exiting
-        assert state.validators[source_index].exit_epoch < spec.FAR_FUTURE_EPOCH
-        # Check that the exit epoch matches earliest_consolidation_epoch
-        assert state.validators[source_index].exit_epoch == state.earliest_consolidation_epoch
-        # Check that the correct consolidation has been appended
-        expected_new_pending_consolidation = spec.PendingConsolidation(
-            source_index=source_index,
-            target_index=target_index,
-        )
-        assert state.pending_consolidations == pre_pending_consolidations + [expected_new_pending_consolidation]
+        # Check excess balance is queued if the target switched to compounding
+        if pre_target_withdrawal_credentials[:1] == spec.ETH1_ADDRESS_WITHDRAWAL_PREFIX:
+            assert state.validators[target_index].withdrawal_credentials == (
+                spec.COMPOUNDING_WITHDRAWAL_PREFIX + pre_target_withdrawal_credentials[1:])
+            assert state.balances[target_index] == spec.MIN_ACTIVATION_BALANCE
+            if pre_target_balance > spec.MIN_ACTIVATION_BALANCE:
+                assert state.pending_balance_deposits == [spec.PendingBalanceDeposit(
+                    index=target_index, amount=(pre_target_balance - spec.MIN_ACTIVATION_BALANCE))]
+        # If source and target are same, no consolidation must have been initiated
+        if source_index == target_index:
+            assert state.validators[source_index].exit_epoch == spec.FAR_FUTURE_EPOCH
+            assert state.pending_consolidations == []
+        else:
+            # Check source is now exiting
+            assert state.validators[source_index].exit_epoch < spec.FAR_FUTURE_EPOCH
+            # Check that the exit epoch matches earliest_consolidation_epoch
+            assert state.validators[source_index].exit_epoch == state.earliest_consolidation_epoch
+            # Check that the withdrawable_epoch is set correctly
+            assert state.validators[source_index].withdrawable_epoch == (
+                state.validators[source_index].exit_epoch + spec.config.MIN_VALIDATOR_WITHDRAWABILITY_DELAY)
+            # Check that the correct consolidation has been appended
+            expected_new_pending_consolidation = spec.PendingConsolidation(
+                source_index=source_index,
+                target_index=target_index,
+            )
+            assert state.pending_consolidations == pre_pending_consolidations + [expected_new_pending_consolidation]
     else:
         assert pre_state == state

tests/core/pyspec/eth2spec/test/electra/epoch_processing/test_process_pending_consolidations.py

@@ -36,10 +36,6 @@ def test_basic_pending_consolidation(spec, state):
     yield from run_epoch_processing_with(spec, state, "process_pending_consolidations")
 
     # Pending consolidation was successfully processed
-    assert (
-        state.validators[target_index].withdrawal_credentials[:1]
-        == spec.COMPOUNDING_WITHDRAWAL_PREFIX
-    )
     assert state.balances[target_index] == 2 * spec.MIN_ACTIVATION_BALANCE
     assert state.balances[source_index] == 0
     assert state.pending_consolidations == []
@@ -65,21 +61,13 @@ def test_consolidation_not_yet_withdrawable_validator(spec, state):
 
     pre_pending_consolidations = state.pending_consolidations.copy()
     pre_balances = state.balances.copy()
-    pre_target_withdrawal_credential = state.validators[
-        target_index
-    ].withdrawal_credentials[:1]
 
     yield from run_epoch_processing_with(spec, state, "process_pending_consolidations")
 
     # Pending consolidation is not processed
     # Balances are unchanged
     assert state.balances[source_index] == pre_balances[0]
     assert state.balances[target_index] == pre_balances[1]
-    # Target withdrawal credential is unchanged
-    assert (
-        state.validators[target_index].withdrawal_credentials[:1]
-        == pre_target_withdrawal_credential
-    )
     # Pending consolidation is still in the queue
     assert state.pending_consolidations == pre_pending_consolidations
 
@@ -121,17 +109,9 @@ def test_skip_consolidation_when_source_slashed(spec, state):
     # first pending consolidation should not be processed
     assert state.balances[target0_index] == spec.MIN_ACTIVATION_BALANCE
     assert state.balances[source0_index] == spec.MIN_ACTIVATION_BALANCE
-    assert (
-        state.validators[target0_index].withdrawal_credentials[:1]
-        == spec.ETH1_ADDRESS_WITHDRAWAL_PREFIX
-    )
     # second pending consolidation should be processed: first one is skipped and doesn't block the queue
     assert state.balances[target1_index] == 2 * spec.MIN_ACTIVATION_BALANCE
     assert state.balances[source1_index] == 0
-    assert (
-        state.validators[target1_index].withdrawal_credentials[:1]
-        == spec.COMPOUNDING_WITHDRAWAL_PREFIX
-    )
 
 
 @with_electra_and_later
@@ -167,26 +147,14 @@ def test_all_consolidation_cases_together(spec, state):
     spec.initiate_validator_exit(state, 2)
 
     pre_balances = state.balances.copy()
-    pre_target_withdrawal_prefixes = [
-        state.validators[target_index[i]].withdrawal_credentials[:1]
-        for i in [0, 1, 2, 3]
-    ]
     pre_pending_consolidations = state.pending_consolidations.copy()
     yield from run_epoch_processing_with(spec, state, "process_pending_consolidations")
 
     # First consolidation is successfully processed
-    assert (
-        state.validators[target_index[0]].withdrawal_credentials[:1]
-        == spec.COMPOUNDING_WITHDRAWAL_PREFIX
-    )
     assert state.balances[target_index[0]] == 2 * spec.MIN_ACTIVATION_BALANCE
     assert state.balances[source_index[0]] == 0
     # All other consolidations are not processed
     for i in [1, 2, 3]:
-        assert (
-            state.validators[target_index[i]].withdrawal_credentials[:1]
-            == pre_target_withdrawal_prefixes[i]
-        )
         assert state.balances[source_index[i]] == pre_balances[source_index[i]]
         assert state.balances[target_index[i]] == pre_balances[target_index[i]]
     # First consolidation is processed, second is skipped, last two are left in the queue
@@ -226,22 +194,11 @@ def test_pending_consolidation_future_epoch(spec, state):
 
     # Pending consolidation was successfully processed
     expected_source_balance = state_before_consolidation.balances[source_index] - spec.MIN_ACTIVATION_BALANCE
-    assert (
-        state.validators[target_index].withdrawal_credentials[:1]
-        == spec.COMPOUNDING_WITHDRAWAL_PREFIX
-    )
-    assert state.balances[target_index] == 2 * spec.MIN_ACTIVATION_BALANCE
+    expected_target_balance = state_before_consolidation.balances[target_index] + spec.MIN_ACTIVATION_BALANCE
     assert state.balances[source_index] == expected_source_balance
+    assert state.balances[target_index] == expected_target_balance
     assert state.pending_consolidations == []
 
-    # Pending balance deposit to the target is created as part of `switch_to_compounding_validator`.
-    # The excess balance to queue are the rewards accumulated over the previous epoch transitions.
-    expected_pending_balance = state_before_consolidation.balances[target_index] - spec.MIN_ACTIVATION_BALANCE
-    assert len(state.pending_balance_deposits) > 0
-    pending_balance_deposit = state.pending_balance_deposits[len(state.pending_balance_deposits) - 1]
-    assert pending_balance_deposit.index == target_index
-    assert pending_balance_deposit.amount == expected_pending_balance
-
 
 @with_electra_and_later
 @spec_state_test
@@ -280,10 +237,6 @@ def test_pending_consolidation_compounding_creds(spec, state):
     expected_target_balance = (
         state_before_consolidation.balances[source_index] + state_before_consolidation.balances[target_index]
     )
-    assert (
-        state.validators[target_index].withdrawal_credentials[:1]
-        == spec.COMPOUNDING_WITHDRAWAL_PREFIX
-    )
     assert state.balances[target_index] == expected_target_balance
     # All source balance is active and moved to the target,
     # because the source validator has compounding credentials
@@ -336,10 +289,6 @@ def test_pending_consolidation_with_pending_deposit(spec, state):
     expected_target_balance = (
         state_before_consolidation.balances[source_index] + state_before_consolidation.balances[target_index]
     )
-    assert (
-        state.validators[target_index].withdrawal_credentials[:1]
-        == spec.COMPOUNDING_WITHDRAWAL_PREFIX
-    )
     assert state.balances[target_index] == expected_target_balance
     assert state.balances[source_index] == 0
     assert state.pending_consolidations == []