bound the maximum number of validators considered for withdrawals per…

… sweep

presets/mainnet/capella.yaml

@@ -10,3 +10,8 @@ MAX_BLS_TO_EXECUTION_CHANGES: 16
 # ---------------------------------------------------------------
 # 2**4 (= 16) withdrawals
 MAX_WITHDRAWALS_PER_PAYLOAD: 16
+
+# Withdrawals processing
+# ---------------------------------------------------------------
+# 2**10 (= 1024) validators
+MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP: 1024

presets/minimal/capella.yaml

@@ -10,3 +10,8 @@ MAX_BLS_TO_EXECUTION_CHANGES: 16
 # ---------------------------------------------------------------
 # [customized] 2**2 (= 4)
 MAX_WITHDRAWALS_PER_PAYLOAD: 4
+
+# Withdrawals processing
+# ---------------------------------------------------------------
+# [customized] 2**4 (= 16) validators
+MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP: 16

specs/capella/beacon-chain.md

@@ -8,11 +8,11 @@
 
 - [Introduction](#introduction)
 - [Custom types](#custom-types)
-- [Constants](#constants)
   - [Domain types](#domain-types)
 - [Preset](#preset)
   - [Max operations per block](#max-operations-per-block)
   - [Execution](#execution)
+  - [Withdrawals processing](#withdrawals-processing)
 - [Containers](#containers)
   - [New containers](#new-containers)
     - [`Withdrawal`](#withdrawal)
@@ -58,8 +58,6 @@ We define the following Python custom types for type hinting and readability:
 | - | - | - |
 | `WithdrawalIndex` | `uint64` | an index of a `Withdrawal` |
 
-## Constants
-
 ### Domain types
 
 | Name | Value |
@@ -80,6 +78,12 @@ We define the following Python custom types for type hinting and readability:
 | - | - | - |
 | `MAX_WITHDRAWALS_PER_PAYLOAD` | `uint64(2**4)` (= 16) | Maximum amount of withdrawals allowed in each payload |
 
+### Withdrawals processing
+
+| Name | Value |
+| - | - |
+| `MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP` | `1024` (= 2**10 ) |
+
 ## Containers
 
 ### New containers
@@ -288,7 +292,8 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]:
     withdrawal_index = state.next_withdrawal_index
     validator_index = state.next_withdrawal_validator_index
     withdrawals: List[Withdrawal] = []
-    for _ in range(len(state.validators)):
+    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):
@@ -312,7 +317,7 @@ def get_expected_withdrawals(state: BeaconState) -> Sequence[Withdrawal]:
         validator_index = ValidatorIndex((validator_index + 1) % len(state.validators))
     return withdrawals
 ```
-        
+
 #### New `process_withdrawals`
 
 ```python
@@ -323,10 +328,21 @@ def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None:
     for expected_withdrawal, withdrawal in zip(expected_withdrawals, payload.withdrawals):
         assert withdrawal == expected_withdrawal
         decrease_balance(state, withdrawal.validator_index, withdrawal.amount)
-    if len(expected_withdrawals) > 0:
+
+    # Update the next withdrawal index if this block contained withdrawals
+    if len(expected_withdrawals) != 0:
         latest_withdrawal = expected_withdrawals[-1]
         state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1)
-        next_validator_index = ValidatorIndex((latest_withdrawal.validator_index + 1) % len(state.validators))
+
+    # Update the next validator index to start the next withdrawal sweep
+    if len(expected_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))
+        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
 ```
 

tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_withdrawals.py

@@ -6,7 +6,7 @@
     with_presets,
     with_phases,
 )
-from eth2spec.test.helpers.constants import MINIMAL, CAPELLA
+from eth2spec.test.helpers.constants import MAINNET, MINIMAL, CAPELLA
 from eth2spec.test.helpers.execution_payload import (
     build_empty_execution_payload,
 )
@@ -33,8 +33,13 @@ def verify_post_state(state, spec, expected_withdrawals,
 
     expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals]
     assert state.next_withdrawal_index == expected_withdrawals[-1].index + 1
-    next_withdrawal_validator_index = (expected_withdrawals_validator_indices[-1] + 1) % len(state.validators)
-    assert state.next_withdrawal_validator_index == next_withdrawal_validator_index
+
+    if len(expected_withdrawals) == spec.MAX_WITHDRAWALS_PER_PAYLOAD:
+        # NOTE: ideally we would also check in the case with
+        # fewer than maximum withdrawals but that requires the pre-state info
+        next_withdrawal_validator_index = (expected_withdrawals_validator_indices[-1] + 1) % len(state.validators)
+        assert state.next_withdrawal_validator_index == next_withdrawal_validator_index
+
     for index in fully_withdrawable_indices:
         if index in expected_withdrawals_validator_indices:
             assert state.balances[index] == 0
@@ -75,9 +80,13 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with
     yield 'post', state
 
     if len(expected_withdrawals) == 0:
-        assert state == pre_state
-    elif len(expected_withdrawals) < spec.MAX_WITHDRAWALS_PER_PAYLOAD:
-        assert len(spec.get_expected_withdrawals(state)) == 0
+        next_withdrawal_validator_index = (
+            pre_state.next_withdrawal_validator_index + spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
+        )
+        assert state.next_withdrawal_validator_index == next_withdrawal_validator_index % len(state.validators)
+    elif len(expected_withdrawals) <= spec.MAX_WITHDRAWALS_PER_PAYLOAD:
+        bound = min(spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP, spec.MAX_WITHDRAWALS_PER_PAYLOAD)
+        assert len(spec.get_expected_withdrawals(state)) <= bound
     elif len(expected_withdrawals) > spec.MAX_WITHDRAWALS_PER_PAYLOAD:
         raise ValueError('len(expected_withdrawals) should not be greater than MAX_WITHDRAWALS_PER_PAYLOAD')
 
@@ -154,10 +163,32 @@ def test_success_max_per_slot(spec, state):
 
 
 @with_phases([CAPELLA])
+@with_presets([MAINNET], reason="too few validators with minimal config")
+@spec_state_test
+def test_success_all_fully_withdrawable_in_one_sweep(spec, state):
+    assert len(state.validators) <= spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
+
+    withdrawal_count = len(state.validators)
+    fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals(
+        spec, state, num_full_withdrawals=withdrawal_count)
+
+    next_slot(spec, state)
+    execution_payload = build_empty_execution_payload(spec, state)
+
+    yield from run_withdrawals_processing(
+        spec, state, execution_payload,
+        fully_withdrawable_indices=fully_withdrawable_indices,
+        partial_withdrawals_indices=partial_withdrawals_indices)
+
+@with_phases([CAPELLA])
+@with_presets([MINIMAL], reason="too many validators with mainnet config")
 @spec_state_test
 def test_success_all_fully_withdrawable(spec, state):
+    assert len(state.validators) > spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
+
+    withdrawal_count = spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
     fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals(
-        spec, state, num_full_withdrawals=len(state.validators))
+        spec, state, num_full_withdrawals=withdrawal_count)
 
     next_slot(spec, state)
     execution_payload = build_empty_execution_payload(spec, state)
@@ -169,10 +200,32 @@ def test_success_all_fully_withdrawable(spec, state):
 
 
 @with_phases([CAPELLA])
+@with_presets([MAINNET], reason="too few validators with minimal config")
+@spec_state_test
+def test_success_all_partially_withdrawable_in_one_sweep(spec, state):
+    assert len(state.validators) <= spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
+
+    withdrawal_count = len(state.validators)
+    fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals(
+        spec, state, num_partial_withdrawals=withdrawal_count)
+
+    next_slot(spec, state)
+    execution_payload = build_empty_execution_payload(spec, state)
+
+    yield from run_withdrawals_processing(
+        spec, state, execution_payload,
+        fully_withdrawable_indices=fully_withdrawable_indices,
+        partial_withdrawals_indices=partial_withdrawals_indices)
+
+@with_phases([CAPELLA])
+@with_presets([MINIMAL], reason="too many validators with mainnet config")
 @spec_state_test
 def test_success_all_partially_withdrawable(spec, state):
+    assert len(state.validators) > spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
+
+    withdrawal_count = spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
     fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals(
-        spec, state, num_partial_withdrawals=len(state.validators))
+        spec, state, num_partial_withdrawals=withdrawal_count)
 
     next_slot(spec, state)
     execution_payload = build_empty_execution_payload(spec, state)
@@ -302,8 +355,8 @@ def test_fail_a_lot_partially_withdrawable_too_few_in_withdrawals(spec, state):
 @with_phases([CAPELLA])
 @spec_state_test
 def test_fail_a_lot_mixed_withdrawable_in_queue_too_few_in_withdrawals(spec, state):
-    prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4,
-                                 num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4)
+    prepare_expected_withdrawals(spec, state, num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD,
+                                 num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD)
 
     next_slot(spec, state)
     execution_payload = build_empty_execution_payload(spec, state)
@@ -624,7 +677,7 @@ def test_success_excess_balance_but_no_max_effective_balance(spec, state):
 @with_phases([CAPELLA])
 @spec_state_test
 def test_success_one_partial_withdrawable_not_yet_active(spec, state):
-    validator_index = len(state.validators) // 2
+    validator_index = min(len(state.validators) // 2, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1)
     state.validators[validator_index].activation_epoch += 4
     set_validator_partially_withdrawable(spec, state, validator_index)
 
@@ -638,7 +691,7 @@ def test_success_one_partial_withdrawable_not_yet_active(spec, state):
 @with_phases([CAPELLA])
 @spec_state_test
 def test_success_one_partial_withdrawable_in_exit_queue(spec, state):
-    validator_index = len(state.validators) // 2
+    validator_index = min(len(state.validators) // 2, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1)
     state.validators[validator_index].exit_epoch = spec.get_current_epoch(state) + 1
     set_validator_partially_withdrawable(spec, state, validator_index)
 
@@ -653,7 +706,7 @@ def test_success_one_partial_withdrawable_in_exit_queue(spec, state):
 @with_phases([CAPELLA])
 @spec_state_test
 def test_success_one_partial_withdrawable_exited(spec, state):
-    validator_index = len(state.validators) // 2
+    validator_index = min(len(state.validators) // 2, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1)
     state.validators[validator_index].exit_epoch = spec.get_current_epoch(state)
     set_validator_partially_withdrawable(spec, state, validator_index)
 
@@ -667,7 +720,7 @@ def test_success_one_partial_withdrawable_exited(spec, state):
 @with_phases([CAPELLA])
 @spec_state_test
 def test_success_one_partial_withdrawable_active_and_slashed(spec, state):
-    validator_index = len(state.validators) // 2
+    validator_index = min(len(state.validators) // 2, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1)
     state.validators[validator_index].slashed = True
     set_validator_partially_withdrawable(spec, state, validator_index)
 
@@ -681,7 +734,7 @@ def test_success_one_partial_withdrawable_active_and_slashed(spec, state):
 @with_phases([CAPELLA])
 @spec_state_test
 def test_success_one_partial_withdrawable_exited_and_slashed(spec, state):
-    validator_index = len(state.validators) // 2
+    validator_index = min(len(state.validators) // 2, spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP - 1)
     state.validators[validator_index].slashed = True
     state.validators[validator_index].exit_epoch = spec.get_current_epoch(state)
     set_validator_partially_withdrawable(spec, state, validator_index)

tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py

@@ -194,8 +194,8 @@ def test_many_partial_withdrawals_in_epoch_transition(spec, state):
 
 def _perform_valid_withdrawal(spec, state):
     fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals(
-        spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4,
-        num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 4)
+        spec, state, num_partial_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 2,
+        num_full_withdrawals=spec.MAX_WITHDRAWALS_PER_PAYLOAD * 2)
 
     next_slot(spec, state)
     pre_next_withdrawal_index = state.next_withdrawal_index

tests/core/pyspec/eth2spec/test/helpers/withdrawals.py

@@ -36,9 +36,10 @@ def set_validator_partially_withdrawable(spec, state, index, excess_balance=1000
 
 def prepare_expected_withdrawals(spec, state,
                                  num_full_withdrawals=0, num_partial_withdrawals=0, rng=random.Random(5566)):
-    assert num_full_withdrawals + num_partial_withdrawals <= len(state.validators)
-    all_validator_indices = list(range(len(state.validators)))
-    sampled_indices = rng.sample(all_validator_indices, num_full_withdrawals + num_partial_withdrawals)
+    bound = min(len(state.validators), spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP)
+    assert num_full_withdrawals + num_partial_withdrawals <= bound
+    eligible_validator_indices = list(range(bound))
+    sampled_indices = rng.sample(eligible_validator_indices, num_full_withdrawals + num_partial_withdrawals)
     fully_withdrawable_indices = rng.sample(sampled_indices, num_full_withdrawals)
     partial_withdrawals_indices = list(set(sampled_indices).difference(set(fully_withdrawable_indices)))