From 50765edc300d9d06aad35d5428157dc961d9090e Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 13 Jan 2021 15:11:24 +0800 Subject: [PATCH 01/10] Set minimal config's `EPOCHS_PER_SYNC_COMMITTEE_PERIOD` to 8 --- configs/minimal/lightclient_patch.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configs/minimal/lightclient_patch.yaml b/configs/minimal/lightclient_patch.yaml index ba1179a2b..0ee39a30b 100644 --- a/configs/minimal/lightclient_patch.yaml +++ b/configs/minimal/lightclient_patch.yaml @@ -12,8 +12,8 @@ SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE: 16 # Time parameters # --------------------------------------------------------------- -# 2**8 (= 256) -EPOCHS_PER_SYNC_COMMITTEE_PERIOD: 256 +# [customized] +EPOCHS_PER_SYNC_COMMITTEE_PERIOD: 8 # Signature domains From 002dfaa89192f29ce5e742962cf2c41f4927e78c Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 13 Jan 2021 17:29:33 +0800 Subject: [PATCH 02/10] Set minimal config's `SYNC_COMMITTEE_SIZE` to 32 --- configs/minimal/lightclient_patch.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/minimal/lightclient_patch.yaml b/configs/minimal/lightclient_patch.yaml index 0ee39a30b..afe7d897e 100644 --- a/configs/minimal/lightclient_patch.yaml +++ b/configs/minimal/lightclient_patch.yaml @@ -5,7 +5,7 @@ CONFIG_NAME: "minimal" # Misc # --------------------------------------------------------------- # [customized] -SYNC_COMMITTEE_SIZE: 64 +SYNC_COMMITTEE_SIZE: 32 # [customized] SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE: 16 From b2658f1091482a3993b311696d5c50b744bde51f Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 13 Jan 2021 17:28:23 +0800 Subject: [PATCH 03/10] Fix SyncCommittee 1. Make `get_sync_committee_indices` do not return duplicate indices 2. Pad default values to Vectors --- specs/lightclient/beacon-chain.md | 17 +++++++++++++++-- .../eth2spec/test/helpers/sync_committee.py | 6 ++++++ .../test_process_sync_committee.py | 17 ++++++++++------- .../lightclient_patch/sanity/test_blocks.py | 5 ++++- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 2fbe190e4..1b434faf4 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -50,6 +50,7 @@ This is a standalone beacon chain patch adding light client support via sync com | Name | Value | | - | - | +| `G1_POINT_AT_INFINITY` | `BLSPubkey(b'\xc0' + b'\x00' * 47)` | | `G2_POINT_AT_INFINITY` | `BLSSignature(b'\xc0' + b'\x00' * 95)` | ### Misc @@ -138,12 +139,19 @@ def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[Val seed = get_seed(state, base_epoch, DOMAIN_SYNC_COMMITTEE) i = 0 sync_committee_indices: List[ValidatorIndex] = [] - while len(sync_committee_indices) < SYNC_COMMITTEE_SIZE: + if len(active_validator_indices) < SYNC_COMMITTEE_SIZE: + committee_size = len(active_validator_indices) + else: + committee_size = SYNC_COMMITTEE_SIZE + while len(sync_committee_indices) < committee_size: shuffled_index = compute_shuffled_index(uint64(i % active_validator_count), active_validator_count, seed) candidate_index = active_validator_indices[shuffled_index] random_byte = hash(seed + uint_to_bytes(uint64(i // 32)))[i % 32] effective_balance = state.validators[candidate_index].effective_balance - if effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte: + if ( + effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte + and candidate_index not in sync_committee_indices + ): sync_committee_indices.append(candidate_index) i += 1 return sync_committee_indices @@ -163,6 +171,11 @@ def get_sync_committee(state: BeaconState, epoch: Epoch) -> SyncCommittee: bls.AggregatePKs(pubkeys[i:i + SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE]) for i in range(0, len(pubkeys), SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE) ] + # Pad G1_POINT_AT_INFINITY to the BLSPubkey Vectors + if len(pubkeys) < SYNC_COMMITTEE_SIZE: + pubkeys += [G1_POINT_AT_INFINITY] * (SYNC_COMMITTEE_SIZE - len(pubkeys)) + aggregates_length = SYNC_COMMITTEE_SIZE // SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE + aggregates += [G1_POINT_AT_INFINITY] * (aggregates_length - len(aggregates)) return SyncCommittee(pubkeys=pubkeys, pubkey_aggregates=aggregates) ``` diff --git a/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py b/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py index b7b2381e3..21557dedd 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py @@ -31,3 +31,9 @@ def compute_aggregate_sync_committee_signature(spec, state, slot, participants): ) ) return bls.Aggregate(signatures) + + +def get_padded_sync_committee_bits(spec, sync_committee_bits): + if len(sync_committee_bits) < spec.SYNC_COMMITTEE_SIZE: + return sync_committee_bits + [False] * (spec.SYNC_COMMITTEE_SIZE - len(sync_committee_bits)) + return sync_committee_bits diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index 37cd0992b..14f3a9f0a 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -9,6 +9,7 @@ from eth2spec.test.helpers.state import ( ) from eth2spec.test.helpers.sync_committee import ( compute_aggregate_sync_committee_signature, + get_padded_sync_committee_bits, ) from eth2spec.test.context import ( PHASE0, PHASE1, @@ -28,7 +29,9 @@ def test_invalid_signature_missing_participant(spec, state): block = build_empty_block_for_next_slot(spec, state) # Exclude one participant whose signature was included. - block.body.sync_committee_bits = [index != random_participant for index in committee] + block.body.sync_committee_bits = get_padded_sync_committee_bits( + spec, [index != random_participant for index in committee] + ) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -51,7 +54,7 @@ def test_invalid_signature_extra_participant(spec, state): block = build_empty_block_for_next_slot(spec, state) # Exclude one signature even though the block claims the entire committee participated. - block.body.sync_committee_bits = [True] * len(committee) + block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -83,7 +86,7 @@ def test_sync_committee_rewards(spec, state): pre_balances = state.balances.copy() block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = [True] * committee_size + block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * committee_size) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -126,7 +129,7 @@ def test_invalid_signature_past_block(spec, state): # NOTE: need to transition twice to move beyond the degenerate case at genesis block = build_empty_block_for_next_slot(spec, state) # Valid sync committee signature here... - block.body.sync_committee_bits = [True] * len(committee) + block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -139,7 +142,7 @@ def test_invalid_signature_past_block(spec, state): invalid_block = build_empty_block_for_next_slot(spec, state) # Invalid signature from a slot other than the previous - invalid_block.body.sync_committee_bits = [True] * len(committee) + invalid_block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) invalid_block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -175,7 +178,7 @@ def test_invalid_signature_previous_committee(spec, state): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = [True] * len(committee) + block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -218,7 +221,7 @@ def test_valid_signature_future_committee(spec, state): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = [True] * len(committee_indices) + block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee_indices)) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py index 9033a0f15..93f8cd3ee 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py @@ -8,6 +8,7 @@ from eth2spec.test.helpers.block import ( ) from eth2spec.test.helpers.sync_committee import ( compute_aggregate_sync_committee_signature, + get_padded_sync_committee_bits, ) from eth2spec.test.context import ( PHASE0, PHASE1, @@ -23,7 +24,9 @@ def run_sync_committee_sanity_test(spec, state, fraction_full=1.0): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = [index in participants for index in committee] + block.body.sync_committee_bits = get_padded_sync_committee_bits( + spec, [index in participants for index in committee] + ) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, From 2a6699290f14bccddc9992b4a1e49eb71a6eb52b Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 14 Jan 2021 01:47:40 +0800 Subject: [PATCH 04/10] Revert "Fix SyncCommittee" This reverts commit b2658f1091482a3993b311696d5c50b744bde51f. --- specs/lightclient/beacon-chain.md | 17 ++--------------- .../eth2spec/test/helpers/sync_committee.py | 6 ------ .../test_process_sync_committee.py | 17 +++++++---------- .../lightclient_patch/sanity/test_blocks.py | 5 +---- 4 files changed, 10 insertions(+), 35 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 1b434faf4..2fbe190e4 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -50,7 +50,6 @@ This is a standalone beacon chain patch adding light client support via sync com | Name | Value | | - | - | -| `G1_POINT_AT_INFINITY` | `BLSPubkey(b'\xc0' + b'\x00' * 47)` | | `G2_POINT_AT_INFINITY` | `BLSSignature(b'\xc0' + b'\x00' * 95)` | ### Misc @@ -139,19 +138,12 @@ def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[Val seed = get_seed(state, base_epoch, DOMAIN_SYNC_COMMITTEE) i = 0 sync_committee_indices: List[ValidatorIndex] = [] - if len(active_validator_indices) < SYNC_COMMITTEE_SIZE: - committee_size = len(active_validator_indices) - else: - committee_size = SYNC_COMMITTEE_SIZE - while len(sync_committee_indices) < committee_size: + while len(sync_committee_indices) < SYNC_COMMITTEE_SIZE: shuffled_index = compute_shuffled_index(uint64(i % active_validator_count), active_validator_count, seed) candidate_index = active_validator_indices[shuffled_index] random_byte = hash(seed + uint_to_bytes(uint64(i // 32)))[i % 32] effective_balance = state.validators[candidate_index].effective_balance - if ( - effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte - and candidate_index not in sync_committee_indices - ): + if effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte: sync_committee_indices.append(candidate_index) i += 1 return sync_committee_indices @@ -171,11 +163,6 @@ def get_sync_committee(state: BeaconState, epoch: Epoch) -> SyncCommittee: bls.AggregatePKs(pubkeys[i:i + SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE]) for i in range(0, len(pubkeys), SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE) ] - # Pad G1_POINT_AT_INFINITY to the BLSPubkey Vectors - if len(pubkeys) < SYNC_COMMITTEE_SIZE: - pubkeys += [G1_POINT_AT_INFINITY] * (SYNC_COMMITTEE_SIZE - len(pubkeys)) - aggregates_length = SYNC_COMMITTEE_SIZE // SYNC_COMMITTEE_PUBKEY_AGGREGATES_SIZE - aggregates += [G1_POINT_AT_INFINITY] * (aggregates_length - len(aggregates)) return SyncCommittee(pubkeys=pubkeys, pubkey_aggregates=aggregates) ``` diff --git a/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py b/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py index 21557dedd..b7b2381e3 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/helpers/sync_committee.py @@ -31,9 +31,3 @@ def compute_aggregate_sync_committee_signature(spec, state, slot, participants): ) ) return bls.Aggregate(signatures) - - -def get_padded_sync_committee_bits(spec, sync_committee_bits): - if len(sync_committee_bits) < spec.SYNC_COMMITTEE_SIZE: - return sync_committee_bits + [False] * (spec.SYNC_COMMITTEE_SIZE - len(sync_committee_bits)) - return sync_committee_bits diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index 14f3a9f0a..37cd0992b 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -9,7 +9,6 @@ from eth2spec.test.helpers.state import ( ) from eth2spec.test.helpers.sync_committee import ( compute_aggregate_sync_committee_signature, - get_padded_sync_committee_bits, ) from eth2spec.test.context import ( PHASE0, PHASE1, @@ -29,9 +28,7 @@ def test_invalid_signature_missing_participant(spec, state): block = build_empty_block_for_next_slot(spec, state) # Exclude one participant whose signature was included. - block.body.sync_committee_bits = get_padded_sync_committee_bits( - spec, [index != random_participant for index in committee] - ) + block.body.sync_committee_bits = [index != random_participant for index in committee] block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -54,7 +51,7 @@ def test_invalid_signature_extra_participant(spec, state): block = build_empty_block_for_next_slot(spec, state) # Exclude one signature even though the block claims the entire committee participated. - block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) + block.body.sync_committee_bits = [True] * len(committee) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -86,7 +83,7 @@ def test_sync_committee_rewards(spec, state): pre_balances = state.balances.copy() block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * committee_size) + block.body.sync_committee_bits = [True] * committee_size block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -129,7 +126,7 @@ def test_invalid_signature_past_block(spec, state): # NOTE: need to transition twice to move beyond the degenerate case at genesis block = build_empty_block_for_next_slot(spec, state) # Valid sync committee signature here... - block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) + block.body.sync_committee_bits = [True] * len(committee) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -142,7 +139,7 @@ def test_invalid_signature_past_block(spec, state): invalid_block = build_empty_block_for_next_slot(spec, state) # Invalid signature from a slot other than the previous - invalid_block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) + invalid_block.body.sync_committee_bits = [True] * len(committee) invalid_block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -178,7 +175,7 @@ def test_invalid_signature_previous_committee(spec, state): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee)) + block.body.sync_committee_bits = [True] * len(committee) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, @@ -221,7 +218,7 @@ def test_valid_signature_future_committee(spec, state): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = get_padded_sync_committee_bits(spec, [True] * len(committee_indices)) + block.body.sync_committee_bits = [True] * len(committee_indices) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py index 93f8cd3ee..9033a0f15 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/sanity/test_blocks.py @@ -8,7 +8,6 @@ from eth2spec.test.helpers.block import ( ) from eth2spec.test.helpers.sync_committee import ( compute_aggregate_sync_committee_signature, - get_padded_sync_committee_bits, ) from eth2spec.test.context import ( PHASE0, PHASE1, @@ -24,9 +23,7 @@ def run_sync_committee_sanity_test(spec, state, fraction_full=1.0): yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = get_padded_sync_committee_bits( - spec, [index in participants for index in committee] - ) + block.body.sync_committee_bits = [index in participants for index in committee] block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, From c5d9aa2502d57fe264702c2ad5a396ffaf1a3b5d Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 19 Jan 2021 20:00:43 +0800 Subject: [PATCH 05/10] Fix test cases for minimal and mainnet configs --- .../test_process_sync_committee.py | 81 +++++++++++++++++-- 1 file changed, 76 insertions(+), 5 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index 37cd0992b..fb44e855e 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -12,8 +12,10 @@ from eth2spec.test.helpers.sync_committee import ( ) from eth2spec.test.context import ( PHASE0, PHASE1, + MAINNET, MINIMAL, expect_assertion_error, with_all_phases_except, + with_configs, spec_state_test, ) @@ -72,12 +74,18 @@ def compute_sync_committee_participant_reward(spec, state, participant_index, ac @with_all_phases_except([PHASE0, PHASE1]) +@with_configs([MINIMAL], reason="to create nonduplicate committee") @spec_state_test -def test_sync_committee_rewards(spec, state): +def test_sync_committee_rewards_nonduplicate_committee(spec, state): committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) committee_size = len(committee) active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) + # Preconditions of this test case + # Note that the committee members MAY still be duplicate even with enough active validator count probabilistically. + assert active_validator_count >= spec.SYNC_COMMITTEE_SIZE + assert committee_size == len(set(committee)) + yield 'pre', state pre_balances = state.balances.copy() @@ -114,6 +122,67 @@ def test_sync_committee_rewards(spec, state): assert state.balances[index] == pre_balances[index] + expected_reward +@with_all_phases_except([PHASE0, PHASE1]) +@with_configs([MAINNET], reason="to create duplicate committee") +@spec_state_test +def test_sync_committee_rewards_duplicate_committee(spec, state): + committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + committee_size = len(committee) + active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) + + # Preconditions of this test case + # With mainnet config, where active validators are less than SYNC_COMMITTEE_SIZE, + # the committee members SHOULD be duplicate. + assert active_validator_count < spec.SYNC_COMMITTEE_SIZE + assert committee_size > len(set(committee)) + + yield 'pre', state + + pre_balances = state.balances.copy() + + block = build_empty_block_for_next_slot(spec, state) + block.body.sync_committee_bits = [True] * committee_size + block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + committee, + ) + + signed_block = state_transition_and_sign_block(spec, state, block) + + yield 'blocks', [signed_block] + yield 'post', state + + duplicate_count = {} + for i, x in enumerate(committee): + if i != committee.index(x): + if x not in duplicate_count: + duplicate_count[x] = 1 + duplicate_count[x] += 1 + + for index in range(len(state.validators)): + expected_reward = 0 + + if index == block.proposer_index: + expected_reward += sum([spec.get_proposer_reward(state, index) for index in committee]) + + if index in committee: + reward = compute_sync_committee_participant_reward( + spec, + state, + index, + active_validator_count, + committee_size, + ) + if index not in duplicate_count: + expected_reward += reward + else: + expected_reward += reward * duplicate_count[index] + + assert state.balances[index] == pre_balances[index] + expected_reward + + @with_all_phases_except([PHASE0, PHASE1]) @spec_state_test def test_invalid_signature_past_block(spec, state): @@ -163,24 +232,26 @@ def test_invalid_signature_previous_committee(spec, state): # To get a distinct committee so we can generate an "old" signature, we need to advance # 2 EPOCHS_PER_SYNC_COMMITTEE_PERIOD periods. current_epoch = spec.get_current_epoch(state) - previous_committee = state.next_sync_committee epoch_in_future_sync_commitee_period = current_epoch + 2 * spec.EPOCHS_PER_SYNC_COMMITTEE_PERIOD slot_in_future_sync_committee_period = epoch_in_future_sync_commitee_period * spec.SLOTS_PER_EPOCH transition_to(spec, state, slot_in_future_sync_committee_period) + # Create incorrect_committee for generating invalid signature. pubkeys = [validator.pubkey for validator in state.validators] - committee = [pubkeys.index(pubkey) for pubkey in previous_committee.pubkeys] + correct_committee = [pubkeys.index(pubkey) for pubkey in state.current_sync_committee.pubkeys] + incorrect_committee = [(correct_committee[0] + 1) % len(pubkeys)] + correct_committee[1:] + assert correct_committee != incorrect_committee yield 'pre', state block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = [True] * len(committee) + block.body.sync_committee_bits = [True] * len(incorrect_committee) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, block.slot - 1, - committee, + incorrect_committee, ) yield 'blocks', [block] From c877d142bd30fb8325e2f5a99ee9fdaa2aef4111 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 19 Jan 2021 20:24:25 +0800 Subject: [PATCH 06/10] Add duplicate elements warning to the docstring --- specs/lightclient/beacon-chain.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 2fbe190e4..8f0c514f0 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -130,7 +130,8 @@ def eth2_fast_aggregate_verify(pubkeys: Sequence[BLSPubkey], message: Bytes32, s def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[ValidatorIndex]: """ Return the sync committee indices for a given state and epoch. - """ + Note that there may be duplicate indices in the resulting list. + """ MAX_RANDOM_BYTE = 2**8 - 1 base_epoch = Epoch((max(epoch // EPOCHS_PER_SYNC_COMMITTEE_PERIOD, 1) - 1) * EPOCHS_PER_SYNC_COMMITTEE_PERIOD) active_validator_indices = get_active_validator_indices(state, base_epoch) From 12593e8782ef4d66f557377d537aa843d68f86d0 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 19 Jan 2021 12:52:40 +0000 Subject: [PATCH 07/10] Update comments --- specs/lightclient/beacon-chain.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 8f0c514f0..54006325e 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -129,8 +129,7 @@ def eth2_fast_aggregate_verify(pubkeys: Sequence[BLSPubkey], message: Bytes32, s ```python def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[ValidatorIndex]: """ - Return the sync committee indices for a given state and epoch. - Note that there may be duplicate indices in the resulting list. + Return the sequence of sync committee indices (which may include duplicate indices) for a given state and epoch. """ MAX_RANDOM_BYTE = 2**8 - 1 base_epoch = Epoch((max(epoch // EPOCHS_PER_SYNC_COMMITTEE_PERIOD, 1) - 1) * EPOCHS_PER_SYNC_COMMITTEE_PERIOD) @@ -144,7 +143,7 @@ def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[Val candidate_index = active_validator_indices[shuffled_index] random_byte = hash(seed + uint_to_bytes(uint64(i // 32)))[i % 32] effective_balance = state.validators[candidate_index].effective_balance - if effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte: + if effective_balance * MAX_RANDOM_BYTE >= MAX_EFFECTIVE_BALANCE * random_byte: # Sample with replacement sync_committee_indices.append(candidate_index) i += 1 return sync_committee_indices From 17a04c2728c67410941b064cbc9897545c3e2acc Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 21 Jan 2021 23:02:22 +0800 Subject: [PATCH 08/10] PR feedback from @ralexstokes --- .../test_process_sync_committee.py | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index fb44e855e..cec3a1a65 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -1,3 +1,4 @@ +from collections import Counter import random from eth2spec.test.helpers.block import ( build_empty_block_for_next_slot, @@ -154,12 +155,7 @@ def test_sync_committee_rewards_duplicate_committee(spec, state): yield 'blocks', [signed_block] yield 'post', state - duplicate_count = {} - for i, x in enumerate(committee): - if i != committee.index(x): - if x not in duplicate_count: - duplicate_count[x] = 1 - duplicate_count[x] += 1 + multiplicities = Counter(committee) for index in range(len(state.validators)): expected_reward = 0 @@ -175,10 +171,7 @@ def test_sync_committee_rewards_duplicate_committee(spec, state): active_validator_count, committee_size, ) - if index not in duplicate_count: - expected_reward += reward - else: - expected_reward += reward * duplicate_count[index] + expected_reward += reward * multiplicities[index] assert state.balances[index] == pre_balances[index] + expected_reward @@ -224,6 +217,7 @@ def test_invalid_signature_past_block(spec, state): @with_all_phases_except([PHASE0, PHASE1]) +@with_configs([MINIMAL], reason="to produce different committee sets") @spec_state_test def test_invalid_signature_previous_committee(spec, state): # NOTE: the `state` provided is at genesis and the process to select @@ -232,26 +226,27 @@ def test_invalid_signature_previous_committee(spec, state): # To get a distinct committee so we can generate an "old" signature, we need to advance # 2 EPOCHS_PER_SYNC_COMMITTEE_PERIOD periods. current_epoch = spec.get_current_epoch(state) + old_sync_committee = state.next_sync_committee epoch_in_future_sync_commitee_period = current_epoch + 2 * spec.EPOCHS_PER_SYNC_COMMITTEE_PERIOD slot_in_future_sync_committee_period = epoch_in_future_sync_commitee_period * spec.SLOTS_PER_EPOCH transition_to(spec, state, slot_in_future_sync_committee_period) - # Create incorrect_committee for generating invalid signature. - pubkeys = [validator.pubkey for validator in state.validators] - correct_committee = [pubkeys.index(pubkey) for pubkey in state.current_sync_committee.pubkeys] - incorrect_committee = [(correct_committee[0] + 1) % len(pubkeys)] + correct_committee[1:] - assert correct_committee != incorrect_committee - yield 'pre', state + # Use the previous sync committee to produce the signature. + pubkeys = [validator.pubkey for validator in state.validators] + # Ensure that the pubkey sets are different. + assert set(old_sync_committee.pubkeys) != set(state.current_sync_committee.pubkeys) + committee = [pubkeys.index(pubkey) for pubkey in old_sync_committee.pubkeys] + block = build_empty_block_for_next_slot(spec, state) - block.body.sync_committee_bits = [True] * len(incorrect_committee) + block.body.sync_committee_bits = [True] * len(committee) block.body.sync_committee_signature = compute_aggregate_sync_committee_signature( spec, state, block.slot - 1, - incorrect_committee, + committee, ) yield 'blocks', [block] From 3847e425b1cae8292a3fb833730068db7c7269e2 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Wed, 20 Jan 2021 16:59:27 -0800 Subject: [PATCH 09/10] refactor to use helper for duplicates in light client committees, rather than config changes --- .../test_process_sync_committee.py | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index fb44e855e..5ef04ef40 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -18,6 +18,27 @@ from eth2spec.test.context import ( with_configs, spec_state_test, ) +from eth2spec.utils.hash_function import hash + + +def get_committee_indices(spec, state, duplicates=False): + ''' + This utility function allows the caller to ensure there are or are not + duplicate validator indices in the returned committee based on + the boolean ``duplicates``. + ''' + state = state.copy() + current_epoch = spec.get_current_epoch(state) + randao_index = current_epoch % spec.EPOCHS_PER_HISTORICAL_VECTOR + while True: + committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + if duplicates: + if len(committee) != len(set(committee)): + return committee + else: + if len(committee) == len(set(committee)): + return committee + state.randao_mixes[randao_index] = hash(state.randao_mixes[randao_index]) @with_all_phases_except([PHASE0, PHASE1]) @@ -77,7 +98,7 @@ def compute_sync_committee_participant_reward(spec, state, participant_index, ac @with_configs([MINIMAL], reason="to create nonduplicate committee") @spec_state_test def test_sync_committee_rewards_nonduplicate_committee(spec, state): - committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + committee = get_committee_indices(spec, state, duplicates=False) committee_size = len(committee) active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) @@ -126,7 +147,7 @@ def test_sync_committee_rewards_nonduplicate_committee(spec, state): @with_configs([MAINNET], reason="to create duplicate committee") @spec_state_test def test_sync_committee_rewards_duplicate_committee(spec, state): - committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + committee = get_committee_indices(spec, state, duplicates=True) committee_size = len(committee) active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) From 0e415fe7c7d04ce1c1bacdee1288d40b6158c0b8 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Thu, 21 Jan 2021 15:33:43 -0800 Subject: [PATCH 10/10] comments no longer apply --- .../block_processing/test_process_sync_committee.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index 5ef04ef40..98db7b7ee 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -103,7 +103,6 @@ def test_sync_committee_rewards_nonduplicate_committee(spec, state): active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) # Preconditions of this test case - # Note that the committee members MAY still be duplicate even with enough active validator count probabilistically. assert active_validator_count >= spec.SYNC_COMMITTEE_SIZE assert committee_size == len(set(committee)) @@ -152,8 +151,6 @@ def test_sync_committee_rewards_duplicate_committee(spec, state): active_validator_count = len(spec.get_active_validator_indices(state, spec.get_current_epoch(state))) # Preconditions of this test case - # With mainnet config, where active validators are less than SYNC_COMMITTEE_SIZE, - # the committee members SHOULD be duplicate. assert active_validator_count < spec.SYNC_COMMITTEE_SIZE assert committee_size > len(set(committee))