PR feedback from @djrtwo and change the duplicate sync committee preconditions

tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py

@@ -136,18 +136,22 @@ def test_invalid_signature_extra_participant(spec, state):
     yield from run_sync_committee_processing(spec, state, block, expect_exception=True)
 
 
+def is_duplicate_sync_committee(committee_indices):
+    dup = {v for v in committee_indices if committee_indices.count(v) > 1}
+    return len(dup) > 0
+
+
 @with_altair_and_later
 @with_presets([MINIMAL], reason="to create nonduplicate committee")
 @spec_state_test
 def test_sync_committee_rewards_nonduplicate_committee(spec, state):
     committee_indices = compute_committee_indices(state)
-    committee_size = len(committee_indices)
-    committee_bits = [True] * committee_size
-    active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state)))
 
     # Preconditions of this test case
-    assert active_validator_count > spec.SYNC_COMMITTEE_SIZE
-    assert committee_size == len(set(committee_indices))
+    assert not is_duplicate_sync_committee(committee_indices)
+
+    committee_size = len(committee_indices)
+    committee_bits = [True] * committee_size
 
     yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
 
@@ -157,13 +161,12 @@ def test_sync_committee_rewards_nonduplicate_committee(spec, state):
 @spec_state_test
 def test_sync_committee_rewards_duplicate_committee_no_participation(spec, state):
     committee_indices = compute_committee_indices(state)
-    committee_size = len(committee_indices)
-    committee_bits = [False] * committee_size
-    active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state)))
 
     # Preconditions of this test case
-    assert active_validator_count < spec.SYNC_COMMITTEE_SIZE
-    assert committee_size > len(set(committee_indices))
+    assert is_duplicate_sync_committee(committee_indices)
+
+    committee_size = len(committee_indices)
+    committee_bits = [False] * committee_size
 
     yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
 
@@ -173,14 +176,13 @@ def test_sync_committee_rewards_duplicate_committee_no_participation(spec, state
 @spec_state_test
 def test_sync_committee_rewards_duplicate_committee_half_participation(spec, state):
     committee_indices = compute_committee_indices(state)
+
+    # Preconditions of this test case
+    assert is_duplicate_sync_committee(committee_indices)
+
     committee_size = len(committee_indices)
     committee_bits = [True] * (committee_size // 2) + [False] * (committee_size // 2)
     assert len(committee_bits) == committee_size
-    active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state)))
-
-    # Preconditions of this test case
-    assert active_validator_count < spec.SYNC_COMMITTEE_SIZE
-    assert committee_size > len(set(committee_indices))
 
     yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
 
@@ -190,22 +192,25 @@ def test_sync_committee_rewards_duplicate_committee_half_participation(spec, sta
 @spec_state_test
 def test_sync_committee_rewards_duplicate_committee_full_participation(spec, state):
     committee_indices = compute_committee_indices(state)
-    committee_size = len(committee_indices)
-    committee_bits = [True] * committee_size
-    active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state)))
 
     # Preconditions of this test case
-    assert active_validator_count < spec.SYNC_COMMITTEE_SIZE
-    assert committee_size > len(set(committee_indices))
+    assert is_duplicate_sync_committee(committee_indices)
+
+    committee_size = len(committee_indices)
+    committee_bits = [True] * committee_size
 
     yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
 
 
-@with_altair_and_later
-@with_presets([MAINNET], reason="to create duplicate committee")
-@spec_state_test
-def test_sync_committee_rewards_duplicate_committee_only_participate_first_one(spec, state):
+def _run_sync_committee_selected_twice(
+        spec, state,
+        pre_balance, participate_first_position, participate_second_position,
+        skip_reward_validation=False):
     committee_indices = compute_committee_indices(state)
+
+    # Preconditions of this test case
+    assert is_duplicate_sync_committee(committee_indices)
+
     committee_size = len(committee_indices)
     committee_bits = [False] * committee_size
 
@@ -214,20 +219,34 @@ def test_sync_committee_rewards_duplicate_committee_only_participate_first_one(s
     assert len(dup) > 0
     validator_index = dup.pop()
     positions = [i for i, v in enumerate(committee_indices) if v == validator_index]
-    committee_bits[positions[0]] = True
-    committee_bits[positions[1]] = False
+    committee_bits[positions[0]] = participate_first_position
+    committee_bits[positions[1]] = participate_second_position
 
-    # Set validator's balance to 0
-    state.validators[validator_index].effective_balance = 0
-    state.balances[validator_index] = 0
+    # Set validator's balance
+    state.balances[validator_index] = pre_balance
+    state.validators[validator_index].effective_balance = min(
+        pre_balance - pre_balance % spec.EFFECTIVE_BALANCE_INCREMENT,
+        spec.MAX_EFFECTIVE_BALANCE,
+    )
 
-    active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state)))
+    yield from run_successful_sync_committee_test(
+        spec, state, committee_indices, committee_bits,
+        skip_reward_validation=skip_reward_validation)
 
-    # Preconditions of this test case
-    assert active_validator_count < spec.SYNC_COMMITTEE_SIZE
-    assert committee_size > len(set(committee_indices))
+    return validator_index
 
-    yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
+
+@with_altair_and_later
+@with_presets([MAINNET], reason="to create duplicate committee")
+@spec_state_test
+def test_sync_committee_rewards_duplicate_committee_zero_balance_only_participate_first_one(spec, state):
+    validator_index = yield from _run_sync_committee_selected_twice(
+        spec,
+        state,
+        pre_balance=0,
+        participate_first_position=True,
+        participate_second_position=False,
+    )
 
     # The validator gets reward first (balance > 0) and then gets the same amount of penalty (balance == 0)
     assert state.balances[validator_index] == 0
@@ -236,39 +255,52 @@ def test_sync_committee_rewards_duplicate_committee_only_participate_first_one(s
 @with_altair_and_later
 @with_presets([MAINNET], reason="to create duplicate committee")
 @spec_state_test
-def test_sync_committee_rewards_duplicate_committee_only_participate_second_one(spec, state):
-    committee_indices = compute_committee_indices(state)
-    committee_size = len(committee_indices)
-    committee_bits = [False] * committee_size
-
-    # Find duplicate indices that get selected twice
-    dup = {v for v in committee_indices if committee_indices.count(v) == 2}
-    assert len(dup) > 0
-    validator_index = dup.pop()
-    positions = [i for i, v in enumerate(committee_indices) if v == validator_index]
-    committee_bits[positions[0]] = False
-    committee_bits[positions[1]] = True
-
-    # Set validator's balance to 0
-    state.validators[validator_index].effective_balance = 0
-    state.balances[validator_index] = 0
-
-    active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state)))
-
-    # Preconditions of this test case
-    assert active_validator_count < spec.SYNC_COMMITTEE_SIZE
-    assert committee_size > len(set(committee_indices))
-
+def test_sync_committee_rewards_duplicate_committee_zero_balance_only_participate_second_one(spec, state):
     # Skip `validate_sync_committee_rewards` because it doesn't handle the balance computation order
     # inside the for loop
-    yield from run_successful_sync_committee_test(
-        spec, state, committee_indices, committee_bits,
-        skip_reward_validation=True)
+    validator_index = yield from _run_sync_committee_selected_twice(
+        spec,
+        state,
+        pre_balance=0,
+        participate_first_position=False,
+        participate_second_position=True,
+        skip_reward_validation=True,
+    )
 
     # The validator gets penalty first (balance is still 0) and then gets reward (balance > 0)
     assert state.balances[validator_index] > 0
 
 
+@with_altair_and_later
+@with_presets([MAINNET], reason="to create duplicate committee")
+@spec_state_test
+def test_sync_committee_rewards_duplicate_committee_max_effective_balance_only_participate_first_one(spec, state):
+    validator_index = yield from _run_sync_committee_selected_twice(
+        spec,
+        state,
+        pre_balance=spec.MAX_EFFECTIVE_BALANCE,
+        participate_first_position=True,
+        participate_second_position=False,
+    )
+
+    assert state.balances[validator_index] == spec.MAX_EFFECTIVE_BALANCE
+
+
+@with_altair_and_later
+@with_presets([MAINNET], reason="to create duplicate committee")
+@spec_state_test
+def test_sync_committee_rewards_duplicate_committee_max_effective_balance_only_participate_second_one(spec, state):
+    validator_index = yield from _run_sync_committee_selected_twice(
+        spec,
+        state,
+        pre_balance=spec.MAX_EFFECTIVE_BALANCE,
+        participate_first_position=False,
+        participate_second_position=True,
+    )
+
+    assert state.balances[validator_index] == spec.MAX_EFFECTIVE_BALANCE
+
+
 @with_altair_and_later
 @spec_state_test
 @always_bls