From 70f90c5296b9ecc95b6f63b8854d103c5d6a18c7 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 19 Sep 2022 11:05:47 -0600 Subject: [PATCH] rmove withdrawn_epoch --- specs/capella/beacon-chain.md | 16 +-- specs/capella/fork.md | 16 +-- .../test_process_bls_to_execution_change.py | 16 ++- .../block_processing/test_process_deposit.py | 30 +++++ .../test_process_full_withdrawals.py | 123 +++++++++++++++--- .../eth2spec/test/helpers/capella/fork.py | 5 +- .../pyspec/eth2spec/test/helpers/deposits.py | 15 ++- .../pyspec/eth2spec/test/helpers/genesis.py | 3 - .../eth2spec/test/helpers/withdrawals.py | 17 +++ .../block_processing/test_process_deposit.py | 46 ++++++- 10 files changed, 228 insertions(+), 59 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_deposit.py create mode 100644 tests/core/pyspec/eth2spec/test/helpers/withdrawals.py diff --git a/specs/capella/beacon-chain.md b/specs/capella/beacon-chain.md index 40592a9bd..25588c1e8 100644 --- a/specs/capella/beacon-chain.md +++ b/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 diff --git a/specs/capella/fork.md b/specs/capella/fork.md index c22387ee7..34d373be0 100644 --- a/specs/capella/fork.md +++ b/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 ``` diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_bls_to_execution_change.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_bls_to_execution_change.py index 4b69e04a6..abcbec0f1 100644 --- a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_bls_to_execution_change.py +++ b/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 diff --git a/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_deposit.py b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_deposit.py new file mode 100644 index 000000000..34c58ef39 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/capella/block_processing/test_process_deposit.py @@ -0,0 +1,30 @@ +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 + + # 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) + + state.balances[validator_index] == amount diff --git a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py b/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py index 1498666bb..35d2968cb 100644 --- a/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py +++ b/tests/core/pyspec/eth2spec/test/capella/epoch_processing/test_process_full_withdrawals.py @@ -1,27 +1,28 @@ +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: @@ -29,11 +30,11 @@ def run_process_full_withdrawals(spec, state, num_expected_withdrawals=None): 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,13 +43,49 @@ 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): @@ -56,7 +93,7 @@ def test_no_withdrawals_but_some_next_epoch(spec, 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)) diff --git a/tests/core/pyspec/eth2spec/test/helpers/capella/fork.py b/tests/core/pyspec/eth2spec/test/helpers/capella/fork.py index 41975904f..8e0aec9c6 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/capella/fork.py +++ b/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 diff --git a/tests/core/pyspec/eth2spec/test/helpers/deposits.py b/tests/core/pyspec/eth2spec/test/helpers/deposits.py index 18929b1d7..4639b22dc 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/deposits.py +++ b/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 diff --git a/tests/core/pyspec/eth2spec/test/helpers/genesis.py b/tests/core/pyspec/eth2spec/test/helpers/genesis.py index b7b941125..e497a0ce2 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/genesis.py +++ b/tests/core/pyspec/eth2spec/test/helpers/genesis.py @@ -20,9 +20,6 @@ def build_mock_validator(spec, i: int, balance: int): effective_balance=min(balance - balance % spec.EFFECTIVE_BALANCE_INCREMENT, spec.MAX_EFFECTIVE_BALANCE) ) - if spec.fork in (CAPELLA): - validator.fully_withdrawn_epoch = spec.FAR_FUTURE_EPOCH - return validator diff --git a/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py b/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py new file mode 100644 index 000000000..739f5eb06 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/helpers/withdrawals.py @@ -0,0 +1,17 @@ +def set_validator_fully_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 + # set exit epoch as well to avoid interactions with other epoch process, e.g. forced ejecions + if validator.exit_epoch > withdrawable_epoch: + validator.exit_epoch = withdrawable_epoch + + if validator.withdrawal_credentials[0:1] == spec.BLS_WITHDRAWAL_PREFIX: + validator.withdrawal_credentials = spec.ETH1_ADDRESS_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:] + + if state.balances[index] == 0: + state.balances[index] = 10000000000 + + assert spec.is_fully_withdrawable_validator(validator, state.balances[index], withdrawable_epoch) diff --git a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_deposit.py b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_deposit.py index 8922032b4..20d8d7d74 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_deposit.py +++ b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_deposit.py @@ -141,13 +141,57 @@ def test_invalid_sig_new_deposit(spec, state): @with_all_phases @spec_state_test -def test_success_top_up(spec, state): +def test_success_top_up__max_effective_balance(spec, state): validator_index = 0 amount = spec.MAX_EFFECTIVE_BALANCE // 4 deposit = prepare_state_and_deposit(spec, state, validator_index, amount, signed=True) + state.balances[validator_index] = spec.MAX_EFFECTIVE_BALANCE + state.validators[validator_index].effective_balance = spec.MAX_EFFECTIVE_BALANCE + yield from run_deposit_processing(spec, state, deposit, validator_index) + assert state.balances[validator_index] == spec.MAX_EFFECTIVE_BALANCE + amount + assert state.validators[validator_index].effective_balance == spec.MAX_EFFECTIVE_BALANCE + + +@with_all_phases +@spec_state_test +def test_success_top_up__less_effective_balance(spec, state): + validator_index = 0 + amount = spec.MAX_EFFECTIVE_BALANCE // 4 + deposit = prepare_state_and_deposit(spec, state, validator_index, amount, signed=True) + + initial_balance = spec.MAX_EFFECTIVE_BALANCE - 1000 + initial_effective_balance = spec.MAX_EFFECTIVE_BALANCE - spec.EFFECTIVE_BALANCE_INCREMENT + state.balances[validator_index] = initial_balance + state.validators[validator_index].effective_balance = initial_effective_balance + + yield from run_deposit_processing(spec, state, deposit, validator_index) + + assert state.balances[validator_index] == initial_balance + amount + # unchanged effective balance + assert state.validators[validator_index].effective_balance == initial_effective_balance + + +@with_all_phases +@spec_state_test +def test_success_top_up__zero_balance(spec, state): + validator_index = 0 + amount = spec.MAX_EFFECTIVE_BALANCE // 4 + deposit = prepare_state_and_deposit(spec, state, validator_index, amount, signed=True) + + initial_balance = 0 + initial_effective_balance = 0 + state.balances[validator_index] = initial_balance + state.validators[validator_index].effective_balance = initial_effective_balance + + yield from run_deposit_processing(spec, state, deposit, validator_index) + + assert state.balances[validator_index] == initial_balance + amount + # unchanged effective balance + assert state.validators[validator_index].effective_balance == initial_effective_balance + @with_all_phases @spec_state_test