Merge pull request #3157 from ethereum/balance-op

Add tests to test sync aggregate's order of balance operation

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,15 +192,113 @@ 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)
+
+    # Preconditions of this test case
+    assert is_duplicate_sync_committee(committee_indices)
+
     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)))
+
+    yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
+
+
+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 active_validator_count < spec.SYNC_COMMITTEE_SIZE
-    assert committee_size > len(set(committee_indices))
+    assert is_duplicate_sync_committee(committee_indices)
 
-    yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits)
+    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]] = participate_first_position
+    committee_bits[positions[1]] = participate_second_position
+
+    # 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,
+    )
+
+    yield from run_successful_sync_committee_test(
+        spec, state, committee_indices, committee_bits,
+        skip_reward_validation=skip_reward_validation)
+
+    return validator_index
+
+
+@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
+
+
+@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_second_one(spec, state):
+    # Skip `validate_sync_committee_rewards` because it doesn't handle the balance computation order
+    # inside the for loop
+    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

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

@@ -107,10 +107,11 @@ def validate_sync_committee_rewards(spec, pre_state, post_state, committee_indic
                 committee_bits,
             )
 
-        assert post_state.balances[index] == pre_state.balances[index] + reward - penalty
+        balance = pre_state.balances[index] + reward
+        assert post_state.balances[index] == (0 if balance < penalty else balance - penalty)
 
 
-def run_sync_committee_processing(spec, state, block, expect_exception=False):
+def run_sync_committee_processing(spec, state, block, expect_exception=False, skip_reward_validation=False):
     """
     Processes everything up to the sync committee work, then runs the sync committee work in isolation, and
     produces a pre-state and post-state (None if exception) specifically for sync-committee processing changes.
@@ -131,14 +132,15 @@ def run_sync_committee_processing(spec, state, block, expect_exception=False):
     else:
         committee_indices = compute_committee_indices(state, state.current_sync_committee)
         committee_bits = block.body.sync_aggregate.sync_committee_bits
-        validate_sync_committee_rewards(
-            spec,
-            pre_state,
-            state,
-            committee_indices,
-            committee_bits,
-            block.proposer_index
-        )
+        if not skip_reward_validation:
+            validate_sync_committee_rewards(
+                spec,
+                pre_state,
+                state,
+                committee_indices,
+                committee_bits,
+                block.proposer_index
+            )
 
 
 def _build_block_for_next_slot_with_sync_participation(spec, state, committee_indices, committee_bits):
@@ -156,6 +158,6 @@ def _build_block_for_next_slot_with_sync_participation(spec, state, committee_in
     return block
 
 
-def run_successful_sync_committee_test(spec, state, committee_indices, committee_bits):
+def run_successful_sync_committee_test(spec, state, committee_indices, committee_bits, skip_reward_validation=False):
     block = _build_block_for_next_slot_with_sync_participation(spec, state, committee_indices, committee_bits)
-    yield from run_sync_committee_processing(spec, state, block)
+    yield from run_sync_committee_processing(spec, state, block, skip_reward_validation=skip_reward_validation)