Merge pull request #2998 from ethereum/remove-withdrawn-epoch

remove withdrawn_epoch

specs/capella/beacon-chain.md

@@ -193,7 +193,6 @@ class Validator(Container):
     activation_epoch: Epoch
     exit_epoch: Epoch
     withdrawable_epoch: Epoch  # When validator can withdraw funds
-    fully_withdrawn_epoch: Epoch  # [New in Capella]
 ```
 
 #### `BeaconBlockBody`
@@ -297,13 +296,14 @@ def has_eth1_withdrawal_credential(validator: Validator) -> bool:
 #### `is_fully_withdrawable_validator`
 
 ```python
-def is_fully_withdrawable_validator(validator: Validator, epoch: Epoch) -> bool:
+def is_fully_withdrawable_validator(validator: Validator, balance: Gwei, epoch: Epoch) -> bool:
     """
     Check if ``validator`` is fully withdrawable.
     """
     return (
         has_eth1_withdrawal_credential(validator)
-        and validator.withdrawable_epoch <= epoch < validator.fully_withdrawn_epoch
+        and validator.withdrawable_epoch <= epoch
+        and balance > 0
     )
 ```
 
@@ -349,11 +349,11 @@ def process_epoch(state: BeaconState) -> None:
 ```python
 def process_full_withdrawals(state: BeaconState) -> None:
     current_epoch = get_current_epoch(state)
-    for index, validator in enumerate(state.validators):
-        if is_fully_withdrawable_validator(validator, current_epoch):
-            # TODO, consider the zero-balance case
-            withdraw_balance(state, ValidatorIndex(index), state.balances[index])
-            validator.fully_withdrawn_epoch = current_epoch
+    for index in range(len(state.validators)):
+        balance = state.balances[index]
+        validator = state.validators[index]
+        if is_fully_withdrawable_validator(validator, balance, current_epoch):
+            withdraw_balance(state, ValidatorIndex(index), balance)
 ```
 
 #### Partial withdrawals

specs/capella/fork.md

@@ -90,7 +90,7 @@ def upgrade_to_capella(pre: bellatrix.BeaconState) -> BeaconState:
         eth1_data_votes=pre.eth1_data_votes,
         eth1_deposit_index=pre.eth1_deposit_index,
         # Registry
-        validators=[],
+        validators=pre.validators,
         balances=pre.balances,
         # Randomness
         randao_mixes=pre.randao_mixes,
@@ -117,19 +117,5 @@ def upgrade_to_capella(pre: bellatrix.BeaconState) -> BeaconState:
         next_partial_withdrawal_validator_index=ValidatorIndex(0),
     )
 
-    for pre_validator in pre.validators:
-        post_validator = Validator(
-            pubkey=pre_validator.pubkey,
-            withdrawal_credentials=pre_validator.withdrawal_credentials,
-            effective_balance=pre_validator.effective_balance,
-            slashed=pre_validator.slashed,
-            activation_eligibility_epoch=pre_validator.activation_eligibility_epoch,
-            activation_epoch=pre_validator.activation_epoch,
-            exit_epoch=pre_validator.exit_epoch,
-            withdrawable_epoch=pre_validator.withdrawable_epoch,
-            fully_withdrawn_epoch=FAR_FUTURE_EPOCH,
-        )
-        post.validators.append(post_validator)
-
     return post
 ```

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

@@ -82,7 +82,9 @@ def test_success_not_activated(spec, state):
     signed_address_change = get_signed_address_change(spec, state)
     yield from run_bls_to_execution_change_processing(spec, state, signed_address_change)
 
-    assert not spec.is_fully_withdrawable_validator(state.validators[validator_index], spec.get_current_epoch(state))
+    validator = state.validators[validator_index]
+    balance = state.balances[validator_index]
+    assert not spec.is_fully_withdrawable_validator(validator, balance, spec.get_current_epoch(state))
 
 
 @with_capella_and_later
@@ -98,7 +100,9 @@ def test_success_in_activation_queue(spec, state):
     signed_address_change = get_signed_address_change(spec, state)
     yield from run_bls_to_execution_change_processing(spec, state, signed_address_change)
 
-    assert not spec.is_fully_withdrawable_validator(state.validators[validator_index], spec.get_current_epoch(state))
+    validator = state.validators[validator_index]
+    balance = state.balances[validator_index]
+    assert not spec.is_fully_withdrawable_validator(validator, balance, spec.get_current_epoch(state))
 
 
 @with_capella_and_later
@@ -126,7 +130,9 @@ def test_success_exited(spec, state):
     signed_address_change = get_signed_address_change(spec, state, validator_index=validator_index)
     yield from run_bls_to_execution_change_processing(spec, state, signed_address_change)
 
-    assert not spec.is_fully_withdrawable_validator(state.validators[validator_index], spec.get_current_epoch(state))
+    validator = state.validators[validator_index]
+    balance = state.balances[validator_index]
+    assert not spec.is_fully_withdrawable_validator(validator, balance, spec.get_current_epoch(state))
 
 
 @with_capella_and_later
@@ -142,7 +148,9 @@ def test_success_withdrawable(spec, state):
     signed_address_change = get_signed_address_change(spec, state, validator_index=validator_index)
     yield from run_bls_to_execution_change_processing(spec, state, signed_address_change)
 
-    assert spec.is_fully_withdrawable_validator(state.validators[validator_index], spec.get_current_epoch(state))
+    validator = state.validators[validator_index]
+    balance = state.balances[validator_index]
+    assert spec.is_fully_withdrawable_validator(validator, balance, spec.get_current_epoch(state))
 
 
 @with_capella_and_later

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

@@ -0,0 +1,39 @@
+from eth2spec.test.context import (
+    spec_state_test,
+    with_capella_and_later,
+)
+from eth2spec.test.helpers.state import next_epoch_via_block
+from eth2spec.test.helpers.deposits import (
+    prepare_state_and_deposit,
+    run_deposit_processing,
+)
+from eth2spec.test.helpers.withdrawals import set_validator_fully_withdrawable
+
+
+@with_capella_and_later
+@spec_state_test
+def test_success_top_up_to_withdrawn_validator(spec, state):
+    validator_index = 0
+
+    # Fully withdraw validator
+    set_validator_fully_withdrawable(spec, state, validator_index)
+    assert state.balances[validator_index] > 0
+    next_epoch_via_block(spec, state)
+    assert state.balances[validator_index] == 0
+    assert state.validators[validator_index].effective_balance > 0
+    next_epoch_via_block(spec, state)
+    assert state.validators[validator_index].effective_balance == 0
+
+    # Make a top-up balance to validator
+    amount = spec.MAX_EFFECTIVE_BALANCE // 4
+    deposit = prepare_state_and_deposit(spec, state, validator_index, amount, signed=True)
+
+    yield from run_deposit_processing(spec, state, deposit, validator_index)
+
+    assert state.balances[validator_index] == amount
+    assert state.validators[validator_index].effective_balance == 0
+
+    validator = state.validators[validator_index]
+    balance = state.balances[validator_index]
+    current_epoch = spec.get_current_epoch(state)
+    assert spec.is_fully_withdrawable_validator(validator, balance, current_epoch)

tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py

@@ -1,39 +1,40 @@
+from random import Random
+
 from eth2spec.test.context import (
     with_capella_and_later,
     spec_state_test,
 )
-from eth2spec.test.helpers.epoch_processing import run_epoch_processing_with
-
-
-def set_validator_withdrawable(spec, state, index, withdrawable_epoch=None):
-    if withdrawable_epoch is None:
-        withdrawable_epoch = spec.get_current_epoch(state)
-
-    validator = state.validators[index]
-    validator.withdrawable_epoch = withdrawable_epoch
-    validator.withdrawal_credentials = spec.ETH1_ADDRESS_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:]
-
-    assert spec.is_fully_withdrawable_validator(validator, withdrawable_epoch)
+from eth2spec.test.helpers.random import (
+    randomize_state,
+)
+from eth2spec.test.helpers.epoch_processing import (
+    run_epoch_processing_to,
+)
+from eth2spec.test.helpers.withdrawals import (
+    set_validator_fully_withdrawable,
+)
 
 
 def run_process_full_withdrawals(spec, state, num_expected_withdrawals=None):
+    run_epoch_processing_to(spec, state, 'process_full_withdrawals')
+
     pre_next_withdrawal_index = state.next_withdrawal_index
     pre_withdrawal_queue = state.withdrawal_queue.copy()
     to_be_withdrawn_indices = [
         index for index, validator in enumerate(state.validators)
-        if spec.is_fully_withdrawable_validator(validator, spec.get_current_epoch(state))
+        if spec.is_fully_withdrawable_validator(validator, state.balances[index], spec.get_current_epoch(state))
     ]
 
     if num_expected_withdrawals is not None:
         assert len(to_be_withdrawn_indices) == num_expected_withdrawals
     else:
         num_expected_withdrawals = len(to_be_withdrawn_indices)
 
-    yield from run_epoch_processing_with(spec, state, 'process_full_withdrawals')
+    yield 'pre', state
+    spec.process_full_withdrawals(state)
+    yield 'post', state
 
     for index in to_be_withdrawn_indices:
-        validator = state.validators[index]
-        assert validator.fully_withdrawn_epoch == spec.get_current_epoch(state)
         assert state.balances[index] == 0
 
     assert len(state.withdrawal_queue) == len(pre_withdrawal_queue) + num_expected_withdrawals
@@ -42,21 +43,57 @@ def run_process_full_withdrawals(spec, state, num_expected_withdrawals=None):
 
 @with_capella_and_later
 @spec_state_test
-def test_no_withdrawals(spec, state):
+def test_no_withdrawable_validators(spec, state):
     pre_validators = state.validators.copy()
     yield from run_process_full_withdrawals(spec, state, 0)
 
     assert pre_validators == state.validators
 
 
+@with_capella_and_later
+@spec_state_test
+def test_withdrawable_epoch_but_0_balance(spec, state):
+    current_epoch = spec.get_current_epoch(state)
+    set_validator_fully_withdrawable(spec, state, 0, current_epoch)
+
+    state.validators[0].effective_balance = 10000000000
+    state.balances[0] = 0
+
+    yield from run_process_full_withdrawals(spec, state, 0)
+
+
+@with_capella_and_later
+@spec_state_test
+def test_withdrawable_epoch_but_0_effective_balance_0_balance(spec, state):
+    current_epoch = spec.get_current_epoch(state)
+    set_validator_fully_withdrawable(spec, state, 0, current_epoch)
+
+    state.validators[0].effective_balance = 0
+    state.balances[0] = 0
+
+    yield from run_process_full_withdrawals(spec, state, 0)
+
+
+@with_capella_and_later
+@spec_state_test
+def test_withdrawable_epoch_but_0_effective_balance_nonzero_balance(spec, state):
+    current_epoch = spec.get_current_epoch(state)
+    set_validator_fully_withdrawable(spec, state, 0, current_epoch)
+
+    state.validators[0].effective_balance = 0
+    state.balances[0] = 100000000
+
+    yield from run_process_full_withdrawals(spec, state, 1)
+
+
 @with_capella_and_later
 @spec_state_test
 def test_no_withdrawals_but_some_next_epoch(spec, state):
     current_epoch = spec.get_current_epoch(state)
 
     # Make a few validators withdrawable at the *next* epoch
     for index in range(3):
-        set_validator_withdrawable(spec, state, index, current_epoch + 1)
+        set_validator_fully_withdrawable(spec, state, index, current_epoch + 1)
 
     yield from run_process_full_withdrawals(spec, state, 0)
 
@@ -65,7 +102,7 @@ def test_no_withdrawals_but_some_next_epoch(spec, state):
 @spec_state_test
 def test_single_withdrawal(spec, state):
     # Make one validator withdrawable
-    set_validator_withdrawable(spec, state, 0)
+    set_validator_fully_withdrawable(spec, state, 0)
 
     assert state.next_withdrawal_index == 0
     yield from run_process_full_withdrawals(spec, state, 1)
@@ -78,7 +115,7 @@ def test_single_withdrawal(spec, state):
 def test_multi_withdrawal(spec, state):
     # Make a few validators withdrawable
     for index in range(3):
-        set_validator_withdrawable(spec, state, index)
+        set_validator_fully_withdrawable(spec, state, index)
 
     yield from run_process_full_withdrawals(spec, state, 3)
 
@@ -88,6 +125,50 @@ def test_multi_withdrawal(spec, state):
 def test_all_withdrawal(spec, state):
     # Make all validators withdrawable
     for index in range(len(state.validators)):
-        set_validator_withdrawable(spec, state, index)
+        set_validator_fully_withdrawable(spec, state, index)
 
     yield from run_process_full_withdrawals(spec, state, len(state.validators))
+
+
+def run_random_full_withdrawals_test(spec, state, rng):
+    randomize_state(spec, state, rng)
+    for index in range(len(state.validators)):
+        # 50% withdrawable
+        if rng.choice([True, False]):
+            set_validator_fully_withdrawable(spec, state, index)
+            validator = state.validators[index]
+            # 12.5% unset credentials
+            if rng.randint(0, 7) == 0:
+                validator.withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:]
+            # 12.5% not enough balance
+            if rng.randint(0, 7) == 0:
+                state.balances[index] = 0
+            # 12.5% not close enough epoch
+            if rng.randint(0, 7) == 0:
+                validator.withdrawable_epoch += 1
+
+    yield from run_process_full_withdrawals(spec, state, None)
+
+
+@with_capella_and_later
+@spec_state_test
+def test_random_withdrawals_0(spec, state):
+    yield from run_random_full_withdrawals_test(spec, state, Random(444))
+
+
+@with_capella_and_later
+@spec_state_test
+def test_random_withdrawals_1(spec, state):
+    yield from run_random_full_withdrawals_test(spec, state, Random(420))
+
+
+@with_capella_and_later
+@spec_state_test
+def test_random_withdrawals_2(spec, state):
+    yield from run_random_full_withdrawals_test(spec, state, Random(200))
+
+
+@with_capella_and_later
+@spec_state_test
+def test_random_withdrawals_3(spec, state):
+    yield from run_random_full_withdrawals_test(spec, state, Random(2000000))

tests/core/pyspec/eth2spec/test/helpers/capella/fork.py

@@ -16,7 +16,7 @@ def run_fork_test(post_spec, pre_state):
         # Eth1
         'eth1_data', 'eth1_data_votes', 'eth1_deposit_index',
         # Registry
-        'balances',
+        'validators', 'balances',
         # Randomness
         'randao_mixes',
         # Slashings
@@ -36,7 +36,7 @@ def run_fork_test(post_spec, pre_state):
         assert getattr(pre_state, field) == getattr(post_state, field)
 
     # Modified fields
-    modified_fields = ['fork', 'validators']
+    modified_fields = ['fork']
     for field in modified_fields:
         assert getattr(pre_state, field) != getattr(post_state, field)
 
@@ -50,7 +50,6 @@ def run_fork_test(post_spec, pre_state):
         ]
         for field in stable_validator_fields:
             assert getattr(pre_validator, field) == getattr(post_validator, field)
-        assert post_validator.fully_withdrawn_epoch == post_spec.FAR_FUTURE_EPOCH
 
     assert pre_state.fork.current_version == post_state.fork.previous_version
     assert post_state.fork.current_version == post_spec.config.CAPELLA_FORK_VERSION

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

@@ -188,8 +188,12 @@ def run_deposit_processing(spec, state, deposit, validator_index, valid=True, ef
     """
     pre_validator_count = len(state.validators)
     pre_balance = 0
+    is_top_up = False
+    # is a top-up
     if validator_index < pre_validator_count:
+        is_top_up = True
         pre_balance = get_balance(state, validator_index)
+        pre_effective_balance = state.validators[validator_index].effective_balance
 
     yield 'pre', state
     yield 'deposit', deposit
@@ -219,9 +223,12 @@ def run_deposit_processing(spec, state, deposit, validator_index, valid=True, ef
             assert len(state.balances) == pre_validator_count + 1
         assert get_balance(state, validator_index) == pre_balance + deposit.data.amount
 
-        effective = min(spec.MAX_EFFECTIVE_BALANCE,
-                        pre_balance + deposit.data.amount)
-        effective -= effective % spec.EFFECTIVE_BALANCE_INCREMENT
-        assert state.validators[validator_index].effective_balance == effective
+        if is_top_up:
+            # Top-ups do not change effective balance
+            assert state.validators[validator_index].effective_balance == pre_effective_balance
+        else:
+            effective_balance = min(spec.MAX_EFFECTIVE_BALANCE, deposit.data.amount)
+            effective_balance -= effective_balance % spec.EFFECTIVE_BALANCE_INCREMENT
+            assert state.validators[validator_index].effective_balance == effective_balance
 
     assert state.eth1_deposit_index == state.eth1_data.deposit_count