Skip to content

Commit

Permalink
bound the maximum number of validators considered for withdrawals per…
Browse files Browse the repository at this point in the history
… sweep
  • Loading branch information
ralexstokes committed Dec 5, 2022
1 parent b62c9e8 commit 4431dae
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 26 deletions.
5 changes: 5 additions & 0 deletions presets/mainnet/capella.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions presets/minimal/capella.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
32 changes: 22 additions & 10 deletions specs/capella/beacon-chain.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 |
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand All @@ -323,11 +328,18 @@ 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:
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))
state.next_withdrawal_validator_index = next_validator_index

if len(expected_withdrawals) < MAX_WITHDRAWALS_PER_PAYLOAD:
# advance sweep if there were not a full set of withdrawals
next_index = state.next_withdrawal_validator_index + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
next_validator_index = next_index % len(state.validators)
state.next_withdrawal_validator_index = ValidatorIndex(next_validator_index)
return

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))
state.next_withdrawal_validator_index = next_validator_index
```

#### Modified `process_execution_payload`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,13 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with
yield 'post', state

if len(expected_withdrawals) == 0:
assert state == pre_state
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:
assert len(spec.get_expected_withdrawals(state)) == 0
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')

Expand Down Expand Up @@ -156,8 +160,9 @@ def test_success_max_per_slot(spec, state):
@with_phases([CAPELLA])
@spec_state_test
def test_success_all_fully_withdrawable(spec, state):
withdrawal_count = min(len(state.validators), 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)
Expand All @@ -171,8 +176,9 @@ def test_success_all_fully_withdrawable(spec, state):
@with_phases([CAPELLA])
@spec_state_test
def test_success_all_partially_withdrawable(spec, state):
withdrawal_count = min(len(state.validators), 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)
Expand Down Expand Up @@ -302,8 +308,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)
Expand Down Expand Up @@ -624,7 +630,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)

Expand All @@ -638,7 +644,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)

Expand All @@ -653,7 +659,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)

Expand All @@ -667,7 +673,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)

Expand All @@ -681,7 +687,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)
Expand Down
4 changes: 2 additions & 2 deletions tests/core/pyspec/eth2spec/test/capella/sanity/test_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions tests/core/pyspec/eth2spec/test/helpers/withdrawals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

Expand Down

0 comments on commit 4431dae

Please sign in to comment.