Test case naming clean up (#3143)

* Add @description decorator

* Unify test case naming style

* more clean ups

* Altair tests cleanup

* Clean up Altair and Bellatrix `process_deposit` tests

* Clean up Bellatrix tests

* Clean up Capella tests

* PR feedback from @ralexstokes

* Add comments on the deposit fork version tests

* Remove `test_incorrect_sig_other_version` since it is duplicate to `test_ineffective_deposit_with_bad_fork_version`

* Add `test_ineffective_deposit_with_current_fork_version`

specs/altair/beacon-chain.md

@@ -523,6 +523,7 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None:
         if bls.Verify(pubkey, signing_root, deposit.data.signature):
             state.validators.append(get_validator_from_deposit(deposit))
             state.balances.append(amount)
+            # [New in Altair]
             state.previous_epoch_participation.append(ParticipationFlags(0b0000_0000))
             state.current_epoch_participation.append(ParticipationFlags(0b0000_0000))
             state.inactivity_scores.append(uint64(0))

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

@@ -0,0 +1,41 @@
+from eth2spec.test.context import (
+    spec_state_test,
+    always_bls,
+    with_phases,
+    with_altair_and_later,
+)
+from eth2spec.test.helpers.constants import (
+    ALTAIR,
+)
+
+
+from eth2spec.test.helpers.deposits import (
+    run_deposit_processing_with_specific_fork_version,
+)
+
+
+@with_phases([ALTAIR])
+@spec_state_test
+@always_bls
+def test_effective_deposit_with_previous_fork_version(spec, state):
+    assert state.fork.previous_version != state.fork.current_version
+
+    # It's only effective in Altair because the default `fork_version` of `compute_domain` is `GENESIS_FORK_VERSION`.
+    # Therefore it's just a normal `DepositMessage`.
+    yield from run_deposit_processing_with_specific_fork_version(
+        spec,
+        state,
+        fork_version=state.fork.previous_version,
+    )
+
+
+@with_altair_and_later
+@spec_state_test
+@always_bls
+def test_ineffective_deposit_with_current_fork_version(spec, state):
+    yield from run_deposit_processing_with_specific_fork_version(
+        spec,
+        state,
+        fork_version=state.fork.current_version,
+        effective=False,
+    )

tests/core/pyspec/eth2spec/test/altair/epoch_processing/test_process_sync_committee_updates.py

@@ -110,7 +110,7 @@ def test_sync_committees_progress_misc_balances_not_genesis(spec, state):
 @spec_state_test
 @always_bls
 @with_presets([MINIMAL], reason="too slow")
-def test_sync_committees_no_progress_not_boundary(spec, state):
+def test_sync_committees_no_progress_not_at_period_boundary(spec, state):
     assert spec.get_current_epoch(state) == spec.GENESIS_EPOCH
     slot_not_at_period_boundary = state.slot + spec.SLOTS_PER_EPOCH
     transition_to(spec, state, slot_not_at_period_boundary)

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

@@ -51,40 +51,40 @@ def run_sync_committee_sanity_test(spec, state, fraction_full=1.0, rng=Random(45
 
 @with_altair_and_later
 @spec_state_test
-def test_full_sync_committee_committee(spec, state):
+def test_sync_committee_committee__full(spec, state):
     next_epoch(spec, state)
     yield from run_sync_committee_sanity_test(spec, state, fraction_full=1.0)
 
 
 @with_altair_and_later
 @spec_state_test
-def test_half_sync_committee_committee(spec, state):
+def test_sync_committee_committee__half(spec, state):
     next_epoch(spec, state)
     yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.5, rng=Random(1212))
 
 
 @with_altair_and_later
 @spec_state_test
-def test_empty_sync_committee_committee(spec, state):
+def test_sync_committee_committee__empty(spec, state):
     next_epoch(spec, state)
     yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.0)
 
 
 @with_altair_and_later
 @spec_state_test
-def test_full_sync_committee_committee_genesis(spec, state):
+def test_sync_committee_committee_genesis__full(spec, state):
     yield from run_sync_committee_sanity_test(spec, state, fraction_full=1.0)
 
 
 @with_altair_and_later
 @spec_state_test
-def test_half_sync_committee_committee_genesis(spec, state):
+def test_sync_committee_committee_genesis__half(spec, state):
     yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.5, rng=Random(2323))
 
 
 @with_altair_and_later
 @spec_state_test
-def test_empty_sync_committee_committee_genesis(spec, state):
+def test_sync_committee_committee_genesis__empty(spec, state):
     yield from run_sync_committee_sanity_test(spec, state, fraction_full=0.0)
 
 

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

@@ -4,82 +4,35 @@
     with_bellatrix_and_later,
 )
 from eth2spec.test.helpers.deposits import (
-    deposit_from_context,
-    run_deposit_processing,
+    run_deposit_processing_with_specific_fork_version,
 )
-from eth2spec.test.helpers.keys import (
-    privkeys,
-    pubkeys,
-)
-from eth2spec.utils import bls
-
-
-def _run_deposit_processing_with_specific_fork_version(
-        spec,
-        state,
-        fork_version,
-        valid,
-        effective):
-    validator_index = len(state.validators)
-    amount = spec.MAX_EFFECTIVE_BALANCE
-
-    pubkey = pubkeys[validator_index]
-    privkey = privkeys[validator_index]
-    withdrawal_credentials = spec.BLS_WITHDRAWAL_PREFIX + spec.hash(pubkey)[1:]
-
-    deposit_message = spec.DepositMessage(pubkey=pubkey, withdrawal_credentials=withdrawal_credentials, amount=amount)
-    domain = spec.compute_domain(domain_type=spec.DOMAIN_DEPOSIT, fork_version=fork_version)
-    deposit_data = spec.DepositData(
-        pubkey=pubkey, withdrawal_credentials=withdrawal_credentials, amount=amount,
-        signature=bls.Sign(privkey, spec.compute_signing_root(deposit_message, domain))
-    )
-    deposit, root, _ = deposit_from_context(spec, [deposit_data], 0)
-
-    state.eth1_deposit_index = 0
-    state.eth1_data.deposit_root = root
-    state.eth1_data.deposit_count = 1
-
-    yield from run_deposit_processing(spec, state, deposit, validator_index, valid=valid, effective=effective)
 
 
 @with_bellatrix_and_later
 @spec_state_test
 @always_bls
-def test_deposit_with_previous_fork_version__valid_ineffective(spec, state):
+def test_ineffective_deposit_with_previous_fork_version(spec, state):
+    # Since deposits are valid across forks, the domain is always set with `GENESIS_FORK_VERSION`.
+    # It's an ineffective deposit because it fails at BLS sig verification.
+    # NOTE: it was effective in Altair.
     assert state.fork.previous_version != state.fork.current_version
 
-    yield from _run_deposit_processing_with_specific_fork_version(
+    yield from run_deposit_processing_with_specific_fork_version(
         spec,
         state,
         fork_version=state.fork.previous_version,
-        valid=True,
         effective=False,
     )
 
 
 @with_bellatrix_and_later
 @spec_state_test
 @always_bls
-def test_deposit_with_genesis_fork_version__valid_effective(spec, state):
+def test_effective_deposit_with_genesis_fork_version(spec, state):
     assert spec.config.GENESIS_FORK_VERSION not in (state.fork.previous_version, state.fork.current_version)
 
-    yield from _run_deposit_processing_with_specific_fork_version(
+    yield from run_deposit_processing_with_specific_fork_version(
         spec,
         state,
         fork_version=spec.config.GENESIS_FORK_VERSION,
-        valid=True,
-        effective=True,
-    )
-
-
-@with_bellatrix_and_later
-@spec_state_test
-@always_bls
-def test_deposit_with_bad_fork_version__valid_ineffective(spec, state):
-    yield from _run_deposit_processing_with_specific_fork_version(
-        spec,
-        state,
-        fork_version=spec.Version('0xAaBbCcDd'),
-        valid=True,
-        effective=False,
     )

tests/core/pyspec/eth2spec/test/bellatrix/block_processing/test_process_execution_payload.py

@@ -104,14 +104,14 @@ def run_bad_execution_test(spec, state):
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_execution_first_payload(spec, state):
+def test_invalid_bad_execution_first_payload(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_bad_execution_test(spec, state)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_execution_regular_payload(spec, state):
+def test_invalid_bad_execution_regular_payload(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_bad_execution_test(spec, state)
 
@@ -125,12 +125,12 @@ def test_bad_parent_hash_first_payload(spec, state):
     execution_payload = build_empty_execution_payload(spec, state)
     execution_payload.parent_hash = b'\x55' * 32
 
-    yield from run_execution_payload_processing(spec, state, execution_payload, valid=True)
+    yield from run_execution_payload_processing(spec, state, execution_payload)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_parent_hash_regular_payload(spec, state):
+def test_invalid_bad_parent_hash_regular_payload(spec, state):
     state = build_state_with_complete_transition(spec, state)
     next_slot(spec, state)
 
@@ -151,14 +151,14 @@ def run_bad_prev_randao_test(spec, state):
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_prev_randao_first_payload(spec, state):
+def test_invalid_bad_prev_randao_first_payload(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_bad_prev_randao_test(spec, state)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_pre_randao_regular_payload(spec, state):
+def test_invalid_bad_pre_randao_regular_payload(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_bad_prev_randao_test(spec, state)
 
@@ -176,14 +176,14 @@ def run_bad_everything_test(spec, state):
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_everything_first_payload(spec, state):
+def test_invalid_bad_everything_first_payload(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_bad_everything_test(spec, state)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_bad_everything_regular_payload(spec, state):
+def test_invalid_bad_everything_regular_payload(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_bad_everything_test(spec, state)
 
@@ -204,28 +204,28 @@ def run_bad_timestamp_test(spec, state, is_future):
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_future_timestamp_first_payload(spec, state):
+def test_invalid_future_timestamp_first_payload(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_bad_timestamp_test(spec, state, is_future=True)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_future_timestamp_regular_payload(spec, state):
+def test_invalid_future_timestamp_regular_payload(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_bad_timestamp_test(spec, state, is_future=True)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_past_timestamp_first_payload(spec, state):
+def test_invalid_past_timestamp_first_payload(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_bad_timestamp_test(spec, state, is_future=False)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_past_timestamp_regular_payload(spec, state):
+def test_invalid_past_timestamp_regular_payload(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_bad_timestamp_test(spec, state, is_future=False)
 
@@ -320,27 +320,27 @@ def run_randomized_non_validated_execution_fields_test(spec, state, execution_va
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_randomized_non_validated_execution_fields_first_payload__valid(spec, state):
+def test_randomized_non_validated_execution_fields_first_payload__execution_valid(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_randomized_non_validated_execution_fields_test(spec, state)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_randomized_non_validated_execution_fields_regular_payload__valid(spec, state):
+def test_randomized_non_validated_execution_fields_regular_payload__execution_valid(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_randomized_non_validated_execution_fields_test(spec, state)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_randomized_non_validated_execution_fields_first_payload__invalid(spec, state):
+def test_invalid_randomized_non_validated_execution_fields_first_payload__execution_invalid(spec, state):
     state = build_state_with_incomplete_transition(spec, state)
     yield from run_randomized_non_validated_execution_fields_test(spec, state, execution_valid=False)
 
 
 @with_bellatrix_and_later
 @spec_state_test
-def test_randomized_non_validated_execution_fields_regular_payload__invalid(spec, state):
+def test_invalid_randomized_non_validated_execution_fields_regular_payload__execution_invalid(spec, state):
     state = build_state_with_complete_transition(spec, state)
     yield from run_randomized_non_validated_execution_fields_test(spec, state, execution_valid=False)