From 19c38dbac662a74f2cf7464129faeffd6c54aab5 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 13 Dec 2022 17:35:20 +0800 Subject: [PATCH 1/2] Add tests to test sync aggregate's order of operation --- .../test_process_sync_aggregate.py | 68 +++++++++++++++++++ .../eth2spec/test/helpers/sync_committee.py | 26 +++---- 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py b/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py index f593353f97..25dd070a4e 100644 --- a/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py +++ b/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py @@ -201,6 +201,74 @@ def test_sync_committee_rewards_duplicate_committee_full_participation(spec, sta 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): + 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]] = True + committee_bits[positions[1]] = False + + # 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)) + + yield from run_successful_sync_committee_test(spec, state, committee_indices, committee_bits) + + # 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_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)) + + # 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) + + # 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 @spec_state_test @always_bls diff --git a/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py b/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py index cc05b862b3..d68c916acc 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py +++ b/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) From c7e102a5e55ed32525e209d0a01ba5ee7d0ae82a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 14 Dec 2022 16:48:53 +0800 Subject: [PATCH 2/2] PR feedback from @djrtwo and change the duplicate sync committee preconditions --- .../test_process_sync_aggregate.py | 150 +++++++++++------- 1 file changed, 91 insertions(+), 59 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py b/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py index 25dd070a4e..a38727612c 100644 --- a/tests/core/pyspec/eth2spec/test/altair/block_processing/sync_aggregate/test_process_sync_aggregate.py +++ b/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 + 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, + ) - # Set validator's balance to 0 - state.validators[validator_index].effective_balance = 0 - state.balances[validator_index] = 0 + yield from run_successful_sync_committee_test( + spec, state, committee_indices, committee_bits, + skip_reward_validation=skip_reward_validation) - active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) + return validator_index - # 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) +@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,37 +255,50 @@ 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 +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, + ) - # 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 + # The validator gets penalty first (balance is still 0) and then gets reward (balance > 0) + assert state.balances[validator_index] > 0 - # 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))) +@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, + ) - # Preconditions of this test case - assert active_validator_count < spec.SYNC_COMMITTEE_SIZE - assert committee_size > len(set(committee_indices)) + assert state.balances[validator_index] == spec.MAX_EFFECTIVE_BALANCE - # 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) - # 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_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