Merge pull request #3656 from ralexstokes/fix-7251-tests

Fix EIP-7251 tests

Author: ralexstokes

.circleci/config.yml

@@ -168,6 +168,19 @@ jobs:
           command: make citest fork=electra
       - store_test_results:
           path: tests/core/pyspec/test-reports
+  test-eip7251:
+    docker:
+      - image: circleci/python:3.9
+    working_directory: ~/specs-repo
+    steps:
+      - restore_cache:
+          key: v3-specs-repo-{{ .Branch }}-{{ .Revision }}
+      - restore_pyspec_cached_venv
+      - run:
+          name: Run py-tests
+          command: make citest fork=eip7251
+      - store_test_results:
+          path: tests/core/pyspec/test-reports
   test-whisk:
     docker:
       - image: circleci/python:3.9
@@ -317,6 +330,9 @@ workflows:
       - test-electra:
           requires:
             - install_pyspec_test
+      - test-eip7251:
+          requires:
+            - install_pyspec_test
       - test-whisk:
           requires:
             - install_pyspec_test

.github/workflows/run-tests.yml

@@ -71,7 +71,7 @@ jobs:
     needs: [preclear,lint,codespell,table_of_contents]
     strategy:
       matrix:
-        version: ["phase0", "altair", "bellatrix", "capella", "deneb", "electra", "whisk", "eip7594"]
+        version: ["phase0", "altair", "bellatrix", "capella", "deneb", "electra", "eip7251", "whisk", "eip7594"]
     steps:
       - name: Checkout this repo
         uses: actions/[email protected]

Makefile

@@ -35,7 +35,7 @@ MARKDOWN_FILES = $(wildcard $(SPEC_DIR)/*/*.md) \
                  $(wildcard $(SPEC_DIR)/_features/*/*/*.md) \
                  $(wildcard $(SSZ_DIR)/*.md)
 
-ALL_EXECUTABLE_SPEC_NAMES = phase0 altair bellatrix capella deneb electra whisk
+ALL_EXECUTABLE_SPEC_NAMES = phase0 altair bellatrix capella deneb electra eip7251 whisk
 # The parameters for commands. Use `foreach` to avoid listing specs again.
 COVERAGE_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPEC_NAMES), --cov=eth2spec.$S.$(TEST_PRESET_TYPE))
 PYLINT_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPEC_NAMES), ./eth2spec/$S)

specs/_features/eip7251/beacon-chain.md

@@ -509,7 +509,7 @@ def initiate_validator_exit(state: BeaconState, index: ValidatorIndex) -> None:
 def switch_to_compounding_validator(state: BeaconState, index: ValidatorIndex) -> None:
     validator = state.validators[index]
     if has_eth1_withdrawal_credential(validator):
-        validator.withdrawal_credentials[:1] = COMPOUNDING_WITHDRAWAL_PREFIX
+        validator.withdrawal_credentials = COMPOUNDING_WITHDRAWAL_PREFIX + validator.withdrawal_credentials[1:]
         queue_excess_active_balance(state, index)
 ```
 
@@ -1055,7 +1055,7 @@ def process_voluntary_exit(state: BeaconState, signed_voluntary_exit: SignedVolu
     assert get_current_epoch(state) >= voluntary_exit.epoch
     # Verify the validator has been active long enough
     assert get_current_epoch(state) >= validator.activation_epoch + SHARD_COMMITTEE_PERIOD
-    # Only exit validator if it has no pending withdrawals in the queue
+    # [New in EIP-7251] Only exit validator if it has no pending withdrawals in the queue
     assert get_pending_balance_to_withdraw(state, voluntary_exit.validator_index) == 0  # [New in EIP7251]
     # Verify signature
     domain = compute_domain(DOMAIN_VOLUNTARY_EXIT, CAPELLA_FORK_VERSION, state.genesis_validators_root)

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

@@ -2,6 +2,7 @@
     spec_state_test,
     with_capella_and_later,
 )
+from eth2spec.test.helpers.forks import is_post_eip7251
 from eth2spec.test.helpers.state import next_epoch_via_block
 from eth2spec.test.helpers.deposits import (
     prepare_state_and_deposit,
@@ -30,10 +31,25 @@ def test_success_top_up_to_withdrawn_validator(spec, state):
 
     yield from run_deposit_processing(spec, state, deposit, validator_index)
 
-    assert state.balances[validator_index] == amount
-    assert state.validators[validator_index].effective_balance == 0
+    if is_post_eip7251(spec):
+        pending_balance_deposits_len = len(state.pending_balance_deposits)
+        pending_balance_deposit = state.pending_balance_deposits[pending_balance_deposits_len - 1]
+        assert pending_balance_deposit.amount == amount
+        assert pending_balance_deposit.index == validator_index
+    else:
+        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)
+
+    if is_post_eip7251(spec):
+        has_execution_withdrawal = spec.has_execution_withdrawal_credential(validator)
+        is_withdrawable = validator.withdrawable_epoch <= current_epoch
+        has_non_zero_balance = pending_balance_deposit.amount > 0
+        # NOTE: directly compute `is_fully_withdrawable_validator` conditions here
+        # to work around how the epoch processing changed balance updates
+        assert has_execution_withdrawal and is_withdrawable and has_non_zero_balance
+    else:
+        assert spec.is_fully_withdrawable_validator(validator, balance, current_epoch)

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

@@ -19,6 +19,7 @@
     next_slot,
 )
 from eth2spec.test.helpers.withdrawals import (
+    get_expected_withdrawals,
     prepare_expected_withdrawals,
     set_eth1_withdrawal_credential_with_balance,
     set_validator_fully_withdrawable,
@@ -62,7 +63,7 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with
       - post-state ('post').
     If ``valid == False``, run expecting ``AssertionError``
     """
-    expected_withdrawals = spec.get_expected_withdrawals(state)
+    expected_withdrawals = get_expected_withdrawals(spec, state)
     assert len(expected_withdrawals) <= spec.MAX_WITHDRAWALS_PER_PAYLOAD
     if num_expected_withdrawals is not None:
         assert len(expected_withdrawals) == num_expected_withdrawals
@@ -87,7 +88,7 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with
         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
+        assert len(get_expected_withdrawals(spec, 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')
 
@@ -100,7 +101,7 @@ def run_withdrawals_processing(spec, state, execution_payload, num_expected_with
 @with_capella_and_later
 @spec_state_test
 def test_success_zero_expected_withdrawals(spec, state):
-    assert len(spec.get_expected_withdrawals(state)) == 0
+    assert len(get_expected_withdrawals(spec, state)) == 0
 
     next_slot(spec, state)
     execution_payload = build_empty_execution_payload(spec, state)

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

@@ -1,4 +1,5 @@
 from eth2spec.test.helpers.constants import MINIMAL
+from eth2spec.test.helpers.forks import is_post_eip7251
 from eth2spec.test.context import (
     with_capella_and_later,
     spec_state_test,
@@ -20,6 +21,7 @@
     next_epoch_with_attestations,
 )
 from eth2spec.test.helpers.withdrawals import (
+    get_expected_withdrawals,
     set_eth1_withdrawal_credential_with_balance,
     set_validator_fully_withdrawable,
     set_validator_partially_withdrawable,
@@ -202,7 +204,7 @@ def test_full_withdrawal_in_epoch_transition(spec, state):
     index = 0
     current_epoch = spec.get_current_epoch(state)
     set_validator_fully_withdrawable(spec, state, index, current_epoch)
-    assert len(spec.get_expected_withdrawals(state)) == 1
+    assert len(get_expected_withdrawals(spec, state)) == 1
 
     yield 'pre', state
 
@@ -214,7 +216,7 @@ def test_full_withdrawal_in_epoch_transition(spec, state):
     yield 'post', state
 
     assert state.balances[index] == 0
-    assert len(spec.get_expected_withdrawals(state)) == 0
+    assert len(get_expected_withdrawals(spec, state)) == 0
 
 
 @with_capella_and_later
@@ -224,7 +226,7 @@ def test_partial_withdrawal_in_epoch_transition(spec, state):
     set_validator_partially_withdrawable(spec, state, index, excess_balance=1000000000000)
     pre_balance = state.balances[index]
 
-    assert len(spec.get_expected_withdrawals(state)) == 1
+    assert len(get_expected_withdrawals(spec, state)) == 1
 
     yield 'pre', state
 
@@ -238,7 +240,7 @@ def test_partial_withdrawal_in_epoch_transition(spec, state):
     assert state.balances[index] < pre_balance
     # Potentially less than due to sync committee penalty
     assert state.balances[index] <= spec.MAX_EFFECTIVE_BALANCE
-    assert len(spec.get_expected_withdrawals(state)) == 0
+    assert len(get_expected_withdrawals(spec, state)) == 0
 
 
 @with_capella_and_later
@@ -250,7 +252,7 @@ def test_many_partial_withdrawals_in_epoch_transition(spec, state):
         index = (i + state.next_withdrawal_index) % len(state.validators)
         set_validator_partially_withdrawable(spec, state, index, excess_balance=1000000000000)
 
-    assert len(spec.get_expected_withdrawals(state)) == spec.MAX_WITHDRAWALS_PER_PAYLOAD
+    assert (len(get_expected_withdrawals(spec, state)) == spec.MAX_WITHDRAWALS_PER_PAYLOAD)
 
     yield 'pre', state
 
@@ -261,7 +263,7 @@ def test_many_partial_withdrawals_in_epoch_transition(spec, state):
     yield 'blocks', [signed_block]
     yield 'post', state
 
-    assert len(spec.get_expected_withdrawals(state)) == 1
+    assert len(get_expected_withdrawals(spec, state)) == 1
 
 
 def _perform_valid_withdrawal(spec, state):
@@ -272,7 +274,7 @@ def _perform_valid_withdrawal(spec, state):
     next_slot(spec, state)
     pre_next_withdrawal_index = state.next_withdrawal_index
 
-    expected_withdrawals = spec.get_expected_withdrawals(state)
+    expected_withdrawals = get_expected_withdrawals(spec, state)
 
     pre_state = state.copy()
 
@@ -357,7 +359,11 @@ def test_top_up_and_partial_withdrawable_validator(spec, state):
 
     signed_block = state_transition_and_sign_block(spec, state, block)
 
-    yield 'blocks', [signed_block]
+    # ensure we go through an epoch transition, to account for post-EIP-7251 behavior
+    block_in_next_epoch = build_empty_block(spec, state, slot=state.slot + spec.SLOTS_PER_EPOCH)
+    signed_block_in_next_epoch = state_transition_and_sign_block(spec, state, block_in_next_epoch)
+
+    yield 'blocks', [signed_block, signed_block_in_next_epoch]
     yield 'post', state
 
     # Since withdrawals happen before deposits, it becomes partially withdrawable after state transition.
@@ -395,9 +401,13 @@ def test_top_up_to_fully_withdrawn_validator(spec, state):
 
     signed_block_1 = state_transition_and_sign_block(spec, state, block)
 
+    balance = state.balances[validator_index]
+    if is_post_eip7251(spec):
+        balance += state.pending_balance_deposits[0].amount
+
     assert spec.is_fully_withdrawable_validator(
         state.validators[validator_index],
-        state.balances[validator_index],
+        balance,
         spec.get_current_epoch(state)
     )
 

tests/core/pyspec/eth2spec/test/deneb/epoch_processing/test_process_registry_updates.py

@@ -1,4 +1,5 @@
 from eth2spec.test.helpers.keys import pubkeys
+from eth2spec.test.helpers.forks import is_post_eip7251
 from eth2spec.test.helpers.constants import MINIMAL
 from eth2spec.test.context import (
     with_deneb_and_later,
@@ -47,7 +48,9 @@ def run_test_activation_churn_limit(spec, state):
     # Half should churn in first run of registry update
     for i in range(mock_activations):
         index = validator_count_0 + i
-        if index < validator_count_0 + churn_limit_0:
+        # NOTE: activations are gated different after EIP-7251
+        # all eligible validators have been activated
+        if index < validator_count_0 + churn_limit_0 or is_post_eip7251(spec):
             # The eligible validators within the activation churn limit should have been activated
             assert state.validators[index].activation_epoch < spec.FAR_FUTURE_EPOCH
         else: