Merge pull request #3776 from fradamt/top-ups-exited

Handle top-ups to exiting/exited validators

Author: hwwhww

specs/electra/beacon-chain.md

@@ -811,12 +811,27 @@ def process_pending_balance_deposits(state: BeaconState) -> None:
     available_for_processing = state.deposit_balance_to_consume + get_activation_exit_churn_limit(state)
     processed_amount = 0
     next_deposit_index = 0
+    deposits_to_postpone = []
 
     for deposit in state.pending_balance_deposits:
-        if processed_amount + deposit.amount > available_for_processing:
-            break
-        increase_balance(state, deposit.index, deposit.amount)
-        processed_amount += deposit.amount
+        validator = state.validators[deposit.index]
+        # Validator is exiting, postpone the deposit until after withdrawable epoch
+        if validator.exit_epoch < FAR_FUTURE_EPOCH:
+            if get_current_epoch(state) <= validator.withdrawable_epoch:
+                deposits_to_postpone.append(deposit)
+            # Deposited balance will never become active. Increase balance but do not consume churn
+            else:
+                increase_balance(state, deposit.index, deposit.amount)
+        # Validator is not exiting, attempt to process deposit
+        else:
+            # Deposit does not fit in the churn, no more deposit processing in this epoch.
+            if processed_amount + deposit.amount > available_for_processing:
+                break
+            # Deposit fits in the churn, process it. Increase balance and consume churn.
+            else: 
+                increase_balance(state, deposit.index, deposit.amount)
+                processed_amount += deposit.amount
+        # Regardless of how the deposit was handled, we move on in the queue.
         next_deposit_index += 1
 
     state.pending_balance_deposits = state.pending_balance_deposits[next_deposit_index:]
@@ -825,6 +840,8 @@ def process_pending_balance_deposits(state: BeaconState) -> None:
         state.deposit_balance_to_consume = Gwei(0)
     else:
         state.deposit_balance_to_consume = available_for_processing - processed_amount
+
+    state.pending_balance_deposits += deposits_to_postpone
 ```
 
 #### New `process_pending_consolidations`

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

@@ -5,6 +5,10 @@
 )
 
 
+def run_process_pending_balance_deposits(spec, state):
+    yield from run_epoch_processing_with(spec, state, 'process_pending_balance_deposits')
+
+
 @with_electra_and_later
 @spec_state_test
 def test_pending_deposit_min_activation_balance(spec, state):
@@ -14,9 +18,9 @@ def test_pending_deposit_min_activation_balance(spec, state):
         spec.PendingBalanceDeposit(index=index, amount=amount)
     )
     pre_balance = state.balances[index]
-    yield from run_epoch_processing_with(
-        spec, state, "process_pending_balance_deposits"
-    )
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
     assert state.balances[index] == pre_balance + amount
     # No leftover deposit balance to consume when there are no deposits left to process
     assert state.deposit_balance_to_consume == 0
@@ -32,9 +36,9 @@ def test_pending_deposit_balance_equal_churn(spec, state):
         spec.PendingBalanceDeposit(index=index, amount=amount)
     )
     pre_balance = state.balances[index]
-    yield from run_epoch_processing_with(
-        spec, state, "process_pending_balance_deposits"
-    )
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
     assert state.balances[index] == pre_balance + amount
     assert state.deposit_balance_to_consume == 0
     assert state.pending_balance_deposits == []
@@ -49,9 +53,9 @@ def test_pending_deposit_balance_above_churn(spec, state):
         spec.PendingBalanceDeposit(index=index, amount=amount)
     )
     pre_balance = state.balances[index]
-    yield from run_epoch_processing_with(
-        spec, state, "process_pending_balance_deposits"
-    )
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
     # deposit was above churn, balance hasn't changed
     assert state.balances[index] == pre_balance
     # deposit balance to consume is the full churn limit
@@ -74,9 +78,9 @@ def test_pending_deposit_preexisting_churn(spec, state):
         spec.PendingBalanceDeposit(index=index, amount=amount)
     )
     pre_balance = state.balances[index]
-    yield from run_epoch_processing_with(
-        spec, state, "process_pending_balance_deposits"
-    )
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
     # balance was deposited correctly
     assert state.balances[index] == pre_balance + amount
     # No leftover deposit balance to consume when there are no deposits left to process
@@ -96,9 +100,9 @@ def test_multiple_pending_deposits_below_churn(spec, state):
         spec.PendingBalanceDeposit(index=1, amount=amount)
     )
     pre_balances = state.balances.copy()
-    yield from run_epoch_processing_with(
-        spec, state, "process_pending_balance_deposits"
-    )
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
     for i in [0, 1]:
         assert state.balances[i] == pre_balances[i] + amount
     # No leftover deposit balance to consume when there are no deposits left to process
@@ -116,9 +120,9 @@ def test_multiple_pending_deposits_above_churn(spec, state):
             spec.PendingBalanceDeposit(index=i, amount=amount)
         )
     pre_balances = state.balances.copy()
-    yield from run_epoch_processing_with(
-        spec, state, "process_pending_balance_deposits"
-    )
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
     # First two deposits are processed, third is not because above churn
     for i in [0, 1]:
         assert state.balances[i] == pre_balances[i] + amount
@@ -132,3 +136,143 @@ def test_multiple_pending_deposits_above_churn(spec, state):
     assert state.pending_balance_deposits == [
         spec.PendingBalanceDeposit(index=2, amount=amount)
     ]
+
+
+@with_electra_and_later
+@spec_state_test
+def test_skipped_deposit_exiting_validator(spec, state):
+    index = 0
+    amount = spec.MIN_ACTIVATION_BALANCE
+    state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=index, amount=amount))
+    pre_pending_balance_deposits = state.pending_balance_deposits.copy()
+    pre_balance = state.balances[index]
+    # Initiate the validator's exit
+    spec.initiate_validator_exit(state, index)
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
+    # Deposit is skipped because validator is exiting
+    assert state.balances[index] == pre_balance
+    # All deposits either processed or postponed, no leftover deposit balance to consume
+    assert state.deposit_balance_to_consume == 0
+    # The deposit is still in the queue
+    assert state.pending_balance_deposits == pre_pending_balance_deposits
+
+
+@with_electra_and_later
+@spec_state_test
+def test_multiple_skipped_deposits_exiting_validators(spec, state):
+    amount = spec.EFFECTIVE_BALANCE_INCREMENT
+    for i in [0, 1, 2]:
+        # Append pending deposit for validator i
+        state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=i, amount=amount))
+
+        # Initiate the exit of validator i
+        spec.initiate_validator_exit(state, i)
+    pre_pending_balance_deposits = state.pending_balance_deposits.copy()
+    pre_balances = state.balances.copy()
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
+    # All deposits are postponed, no balance changes
+    assert state.balances == pre_balances
+    # All deposits are postponed, no leftover deposit balance to consume
+    assert state.deposit_balance_to_consume == 0
+    # All deposits still in the queue, in the same order
+    assert state.pending_balance_deposits == pre_pending_balance_deposits
+
+
+@with_electra_and_later
+@spec_state_test
+def test_multiple_pending_one_skipped(spec, state):
+    amount = spec.EFFECTIVE_BALANCE_INCREMENT
+    for i in [0, 1, 2]:
+        state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=i, amount=amount))
+    pre_balances = state.balances.copy()
+    # Initiate the second validator's exit
+    spec.initiate_validator_exit(state, 1)
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
+    # First and last deposit are processed, second is not because of exiting
+    for i in [0, 2]:
+        assert state.balances[i] == pre_balances[i] + amount
+    assert state.balances[1] == pre_balances[1]
+    # All deposits either processed or postponed, no leftover deposit balance to consume
+    assert state.deposit_balance_to_consume == 0
+    # second deposit is still in the queue
+    assert state.pending_balance_deposits == [spec.PendingBalanceDeposit(index=1, amount=amount)]
+
+
+@with_electra_and_later
+@spec_state_test
+def test_mixture_of_skipped_and_above_churn(spec, state):
+    amount01 = spec.EFFECTIVE_BALANCE_INCREMENT
+    amount2 = spec.MAX_EFFECTIVE_BALANCE_ELECTRA
+    # First two validators have small deposit, third validators a large one
+    for i in [0, 1]:
+        state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=i, amount=amount01))
+    state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=2, amount=amount2))
+    pre_balances = state.balances.copy()
+    # Initiate the second validator's exit
+    spec.initiate_validator_exit(state, 1)
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
+    # First deposit is processed
+    assert state.balances[0] == pre_balances[0] + amount01
+    # Second deposit is postponed, third is above churn
+    for i in [1, 2]:
+        assert state.balances[i] == pre_balances[i]
+    # First deposit consumes some deposit balance
+    # Deposit balance to consume is not reset because third deposit is not processed
+    assert state.deposit_balance_to_consume == spec.get_activation_exit_churn_limit(state) - amount01
+    # second and third deposit still in the queue, but second is appended at the end
+    assert state.pending_balance_deposits == [spec.PendingBalanceDeposit(index=2, amount=amount2),
+                                              spec.PendingBalanceDeposit(index=1, amount=amount01)]
+
+
+@with_electra_and_later
+@spec_state_test
+def test_processing_deposit_of_withdrawable_validator(spec, state):
+    index = 0
+    amount = spec.MIN_ACTIVATION_BALANCE
+    state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=index, amount=amount))
+    pre_balance = state.balances[index]
+    # Initiate the validator's exit
+    spec.initiate_validator_exit(state, index)
+    # Set epoch to withdrawable epoch + 1 to allow processing of the deposit
+    state.slot = spec.SLOTS_PER_EPOCH * (state.validators[index].withdrawable_epoch + 1)
+
+    yield from run_process_pending_balance_deposits(spec, state)
+
+    # Deposit is correctly processed
+    assert state.balances[index] == pre_balance + amount
+    # No leftover deposit balance to consume when there are no deposits left to process
+    assert state.deposit_balance_to_consume == 0
+    assert state.pending_balance_deposits == []
+
+
+@with_electra_and_later
+@spec_state_test
+def test_processing_deposit_of_withdrawable_validator_does_not_get_churned(spec, state):
+    amount = spec.MAX_EFFECTIVE_BALANCE_ELECTRA
+    for i in [0, 1]:
+        state.pending_balance_deposits.append(spec.PendingBalanceDeposit(index=i, amount=amount))
+    pre_balances = state.balances.copy()
+    # Initiate the first validator's exit
+    spec.initiate_validator_exit(state, 0)
+    # Set epoch to withdrawable epoch + 1 to allow processing of the deposit
+    state.slot = spec.SLOTS_PER_EPOCH * (state.validators[0].withdrawable_epoch + 1)
+    # Don't use run_epoch_processing_with to avoid penalties being applied
+    yield 'pre', state
+    spec.process_pending_balance_deposits(state)
+    yield 'post', state
+    # First deposit is processed though above churn limit, because validator is withdrawable
+    assert state.balances[0] == pre_balances[0] + amount
+    # Second deposit is not processed because above churn
+    assert state.balances[1] == pre_balances[1]
+    # Second deposit is not processed, so there's leftover deposit balance to consume.
+    # First deposit does not consume any.
+    assert state.deposit_balance_to_consume == spec.get_activation_exit_churn_limit(state)
+    assert state.pending_balance_deposits == [spec.PendingBalanceDeposit(index=1, amount=amount)]