Skip to content

Commit ecb4c2a

Browse files
mkalininmkalinin
authored
eip7251: Fix partial withdrawals count (#3943)
1 parent 0c8645e commit ecb4c2a

File tree

5 files changed

+267
-79
lines changed

5 files changed

+267
-79
lines changed

‎presets/minimal/electra.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,5 @@ MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD: 2
4141

4242
# Withdrawals processing
4343
# ---------------------------------------------------------------
44-
# 2**0 ( = 1) pending withdrawals
45-
MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP: 1
44+
# 2**1 ( = 2) pending withdrawals
45+
MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP: 2

‎specs/electra/beacon-chain.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,7 @@ def get_expected_withdrawals(state: BeaconState) -> Tuple[Sequence[Withdrawal],
999999
withdrawal_index = state.next_withdrawal_index
10001000
validator_index = state.next_withdrawal_validator_index
10011001
withdrawals: List[Withdrawal] = []
1002+
partial_withdrawals_count = 0
10021003

10031004
# [New in Electra:EIP7251] Consume pending partial withdrawals
10041005
for withdrawal in state.pending_partial_withdrawals:
@@ -1018,7 +1019,7 @@ def get_expected_withdrawals(state: BeaconState) -> Tuple[Sequence[Withdrawal],
10181019
))
10191020
withdrawal_index += WithdrawalIndex(1)
10201021

1021-
partial_withdrawals_count = len(withdrawals)
1022+
partial_withdrawals_count += 1
10221023

10231024
# Sweep for remaining.
10241025
bound = min(len(state.validators), MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP)

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

Lines changed: 1 addition & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from eth2spec.test.context import (
44
spec_state_test,
5-
expect_assertion_error,
65
with_presets,
76
with_capella_and_later,
87
)
@@ -24,80 +23,10 @@
2423
set_eth1_withdrawal_credential_with_balance,
2524
set_validator_fully_withdrawable,
2625
set_validator_partially_withdrawable,
26+
run_withdrawals_processing,
2727
)
2828

2929

30-
def verify_post_state(state, spec, expected_withdrawals,
31-
fully_withdrawable_indices, partial_withdrawals_indices):
32-
# Consider verifying also the condition when no withdrawals are expected.
33-
if len(expected_withdrawals) == 0:
34-
return
35-
36-
expected_withdrawals_validator_indices = [withdrawal.validator_index for withdrawal in expected_withdrawals]
37-
assert state.next_withdrawal_index == expected_withdrawals[-1].index + 1
38-
39-
if len(expected_withdrawals) == spec.MAX_WITHDRAWALS_PER_PAYLOAD:
40-
# NOTE: ideally we would also check in the case with
41-
# fewer than maximum withdrawals but that requires the pre-state info
42-
next_withdrawal_validator_index = (expected_withdrawals_validator_indices[-1] + 1) % len(state.validators)
43-
assert state.next_withdrawal_validator_index == next_withdrawal_validator_index
44-
45-
for index in fully_withdrawable_indices:
46-
if index in expected_withdrawals_validator_indices:
47-
assert state.balances[index] == 0
48-
else:
49-
assert state.balances[index] > 0
50-
for index in partial_withdrawals_indices:
51-
if index in expected_withdrawals_validator_indices:
52-
assert state.balances[index] == spec.MAX_EFFECTIVE_BALANCE
53-
else:
54-
assert state.balances[index] > spec.MAX_EFFECTIVE_BALANCE
55-
56-
57-
def run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=None,
58-
fully_withdrawable_indices=None, partial_withdrawals_indices=None, valid=True):
59-
"""
60-
Run ``process_withdrawals``, yielding:
61-
- pre-state ('pre')
62-
- execution payload ('execution_payload')
63-
- post-state ('post').
64-
If ``valid == False``, run expecting ``AssertionError``
65-
"""
66-
expected_withdrawals = get_expected_withdrawals(spec, state)
67-
assert len(expected_withdrawals) <= spec.MAX_WITHDRAWALS_PER_PAYLOAD
68-
if num_expected_withdrawals is not None:
69-
assert len(expected_withdrawals) == num_expected_withdrawals
70-
71-
pre_state = state.copy()
72-
yield 'pre', state
73-
yield 'execution_payload', execution_payload
74-
75-
if not valid:
76-
expect_assertion_error(lambda: spec.process_withdrawals(state, execution_payload))
77-
yield 'post', None
78-
return
79-
80-
spec.process_withdrawals(state, execution_payload)
81-
82-
yield 'post', state
83-
84-
if len(expected_withdrawals) == 0:
85-
next_withdrawal_validator_index = (
86-
pre_state.next_withdrawal_validator_index + spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP
87-
)
88-
assert state.next_withdrawal_validator_index == next_withdrawal_validator_index % len(state.validators)
89-
elif len(expected_withdrawals) <= spec.MAX_WITHDRAWALS_PER_PAYLOAD:
90-
bound = min(spec.MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP, spec.MAX_WITHDRAWALS_PER_PAYLOAD)
91-
assert len(get_expected_withdrawals(spec, state)) <= bound
92-
elif len(expected_withdrawals) > spec.MAX_WITHDRAWALS_PER_PAYLOAD:
93-
raise ValueError('len(expected_withdrawals) should not be greater than MAX_WITHDRAWALS_PER_PAYLOAD')
94-
95-
if fully_withdrawable_indices is not None or partial_withdrawals_indices is not None:
96-
verify_post_state(state, spec, expected_withdrawals, fully_withdrawable_indices, partial_withdrawals_indices)
97-
98-
return expected_withdrawals
99-
100-
10130
@with_capella_and_later
10231
@spec_state_test
10332
def test_success_zero_expected_withdrawals(spec, state):

‎tests/core/pyspec/eth2spec/test/electra/block_processing/test_process_withdrawals.py‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import random
2+
3+
from eth2spec.test.context import (
4+
spec_state_test,
5+
with_electra_and_later,
6+
)
7+
from eth2spec.test.helpers.execution_payload import (
8+
build_empty_execution_payload,
9+
)
10+
from eth2spec.test.helpers.state import (
11+
next_slot,
12+
)
13+
from eth2spec.test.helpers.withdrawals import (
14+
prepare_expected_withdrawals_compounding,
15+
run_withdrawals_processing,
16+
set_compounding_withdrawal_credential_with_balance,
17+
prepare_pending_withdrawal,
18+
)
19+
20+
21+
@with_electra_and_later
22+
@spec_state_test
23+
def test_success_mixed_fully_and_partial_withdrawable_compounding(spec, state):
24+
num_full_withdrawals = spec.MAX_WITHDRAWALS_PER_PAYLOAD // 2
25+
num_partial_withdrawals = spec.MAX_WITHDRAWALS_PER_PAYLOAD - num_full_withdrawals
26+
fully_withdrawable_indices, partial_withdrawals_indices = prepare_expected_withdrawals_compounding(
27+
spec, state,
28+
rng=random.Random(42),
29+
num_full_withdrawals=num_full_withdrawals,
30+
num_partial_withdrawals_sweep=num_partial_withdrawals,
31+
)
32+
33+
next_slot(spec, state)
34+
execution_payload = build_empty_execution_payload(spec, state)
35+
36+
yield from run_withdrawals_processing(
37+
spec, state, execution_payload,
38+
fully_withdrawable_indices=fully_withdrawable_indices,
39+
partial_withdrawals_indices=partial_withdrawals_indices)
40+
41+
42+
@with_electra_and_later
43+
@spec_state_test
44+
def test_success_no_max_effective_balance_compounding(spec, state):
45+
validator_index = len(state.validators) // 2
46+
# To be partially withdrawable, the validator's effective balance must be maxed out
47+
effective_balance = spec.MAX_EFFECTIVE_BALANCE_ELECTRA - spec.EFFECTIVE_BALANCE_INCREMENT
48+
set_compounding_withdrawal_credential_with_balance(spec, state, validator_index, effective_balance)
49+
50+
validator = state.validators[validator_index]
51+
assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index])
52+
53+
execution_payload = build_empty_execution_payload(spec, state)
54+
55+
yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0)
56+
57+
58+
@with_electra_and_later
59+
@spec_state_test
60+
def test_success_no_excess_balance_compounding(spec, state):
61+
validator_index = len(state.validators) // 2
62+
# To be partially withdrawable, the validator needs an excess balance
63+
effective_balance = spec.MAX_EFFECTIVE_BALANCE_ELECTRA
64+
set_compounding_withdrawal_credential_with_balance(spec, state, validator_index, effective_balance)
65+
66+
validator = state.validators[validator_index]
67+
assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index])
68+
69+
execution_payload = build_empty_execution_payload(spec, state)
70+
71+
yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0)
72+
73+
74+
@with_electra_and_later
75+
@spec_state_test
76+
def test_success_excess_balance_but_no_max_effective_balance_compounding(spec, state):
77+
validator_index = len(state.validators) // 2
78+
# To be partially withdrawable, the validator needs both a maxed out effective balance and an excess balance
79+
effective_balance = spec.MAX_EFFECTIVE_BALANCE_ELECTRA - spec.EFFECTIVE_BALANCE_INCREMENT
80+
balance = spec.MAX_EFFECTIVE_BALANCE_ELECTRA + spec.EFFECTIVE_BALANCE_INCREMENT
81+
set_compounding_withdrawal_credential_with_balance(spec, state, validator_index, effective_balance, balance)
82+
83+
validator = state.validators[validator_index]
84+
assert not spec.is_partially_withdrawable_validator(validator, state.balances[validator_index])
85+
86+
execution_payload = build_empty_execution_payload(spec, state)
87+
88+
yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=0)
89+
90+
91+
@with_electra_and_later
92+
@spec_state_test
93+
def test_pending_withdrawals_one_skipped_one_effective(spec, state):
94+
index_0 = 3
95+
index_1 = 5
96+
97+
withdrawal_0 = prepare_pending_withdrawal(spec, state, index_0)
98+
withdrawal_1 = prepare_pending_withdrawal(spec, state, index_1)
99+
100+
# If validator doesn't have an excess balance pending withdrawal is skipped
101+
state.balances[index_0] = spec.MIN_ACTIVATION_BALANCE
102+
103+
execution_payload = build_empty_execution_payload(spec, state)
104+
assert state.pending_partial_withdrawals == [withdrawal_0, withdrawal_1]
105+
yield from run_withdrawals_processing(spec, state, execution_payload, num_expected_withdrawals=1)
106+
107+
assert state.pending_partial_withdrawals == []

0 commit comments

Comments
 (0)