From 86104ea361921cd6b952bd13a22c6a198648404a Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Fri, 7 May 2021 09:55:21 -0700 Subject: [PATCH 1/4] Use stable sync committee indices when processing block rewards --- specs/altair/beacon-chain.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/specs/altair/beacon-chain.md b/specs/altair/beacon-chain.md index b2ee93557..59b11c8e4 100644 --- a/specs/altair/beacon-chain.md +++ b/specs/altair/beacon-chain.md @@ -571,7 +571,11 @@ def process_sync_committee(state: BeaconState, aggregate: SyncAggregate) -> None proposer_reward = Gwei(participant_reward * PROPOSER_WEIGHT // (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)) # Apply participant and proposer rewards - committee_indices = get_sync_committee_indices(state, get_current_epoch(state)) + committee_indices = [] + pubkeys = [v.pubkey for v in state.validators] + for pubkey in state.current_sync_committee.pubkeys: + index = pubkeys.index(pubkey) + committee_indices.append(ValidatorIndex(index)) participant_indices = [index for index, bit in zip(committee_indices, aggregate.sync_committee_bits) if bit] for participant_index in participant_indices: increase_balance(state, participant_index, participant_reward) From 04a9595415678013442460ef4a0ee90d0799924b Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Fri, 7 May 2021 10:06:44 -0700 Subject: [PATCH 2/4] Add notes about sync committee stability --- specs/altair/beacon-chain.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/specs/altair/beacon-chain.md b/specs/altair/beacon-chain.md index 59b11c8e4..0855d6f11 100644 --- a/specs/altair/beacon-chain.md +++ b/specs/altair/beacon-chain.md @@ -277,6 +277,9 @@ def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[Val """ Return the sequence of sync committee indices (which may include duplicate indices) for a given ``state`` and ``epoch``. + + Note: This function is not stable during a sync committee period as + a validator's effective balance may change enough to affect the sampling. """ MAX_RANDOM_BYTE = 2**8 - 1 base_epoch = Epoch((max(epoch // EPOCHS_PER_SYNC_COMMITTEE_PERIOD, 1) - 1) * EPOCHS_PER_SYNC_COMMITTEE_PERIOD) @@ -310,6 +313,9 @@ def get_sync_committee(state: BeaconState, epoch: Epoch) -> SyncCommittee: ``SyncCommittee`` can also contain duplicate pubkeys, when ``get_sync_committee_indices`` returns duplicate indices. Implementations must take care when handling optimizations relating to aggregation and verification in the presence of duplicates. + + Note: This function should only be called at sync committee period boundaries, as + ``get_sync_committee_indices`` is not stable within a given period. """ indices = get_sync_committee_indices(state, epoch) pubkeys = [state.validators[index].pubkey for index in indices] From b336b710e9bd08594af87a16f53932bbe4672fe9 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Fri, 7 May 2021 15:54:01 -0700 Subject: [PATCH 3/4] Update specs/altair/beacon-chain.md Co-authored-by: vbuterin --- specs/altair/beacon-chain.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/specs/altair/beacon-chain.md b/specs/altair/beacon-chain.md index 0855d6f11..ac4457b22 100644 --- a/specs/altair/beacon-chain.md +++ b/specs/altair/beacon-chain.md @@ -577,11 +577,8 @@ def process_sync_committee(state: BeaconState, aggregate: SyncAggregate) -> None proposer_reward = Gwei(participant_reward * PROPOSER_WEIGHT // (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)) # Apply participant and proposer rewards - committee_indices = [] - pubkeys = [v.pubkey for v in state.validators] - for pubkey in state.current_sync_committee.pubkeys: - index = pubkeys.index(pubkey) - committee_indices.append(ValidatorIndex(index)) + all_pubkeys = [v.pubkey for v in state.validators] + committee_indices = [ValidatorIndex(all_pubkeys.index(pubkey)) for pubkey in state.current_sync_committee.pubkeys] participant_indices = [index for index, bit in zip(committee_indices, aggregate.sync_committee_bits) if bit] for participant_index in participant_indices: increase_balance(state, participant_index, participant_reward) From 72a4ff803b3be39da3a29730a273b04ec6f35a87 Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Fri, 7 May 2021 17:02:52 -0700 Subject: [PATCH 4/4] add test to ensure sync committees are referenced from the state --- .../test_process_sync_committee.py | 55 ++++++++++++++++--- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_sync_committee.py index e0faf3d0d..3e4d77c0f 100644 --- a/tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_sync_committee.py @@ -7,6 +7,7 @@ from eth2spec.test.helpers.block_processing import run_block_processing_to from eth2spec.test.helpers.state import ( state_transition_and_sign_block, transition_to, + next_epoch, ) from eth2spec.test.helpers.constants import ( PHASE0, @@ -160,13 +161,13 @@ def validate_sync_committee_rewards(spec, pre_state, post_state, committee, comm committee_bits, ) - if proposer_index == index: - reward += compute_sync_committee_proposer_reward( - spec, - pre_state, - committee, - committee_bits, - ) + if proposer_index == index: + reward += compute_sync_committee_proposer_reward( + spec, + pre_state, + committee, + committee_bits, + ) assert post_state.balances[index] == pre_state.balances[index] + reward @@ -367,3 +368,43 @@ def test_valid_signature_future_committee(spec, state): ) yield from run_sync_committee_processing(spec, state, block) + + +@with_all_phases_except([PHASE0]) +@spec_state_test +def test_sync_committee_is_only_computed_at_epoch_boundary(spec, state): + """ + Sync committees can only be computed at sync committee period boundaries. + Ensure a client respects the committee in the state (assumed to be derived + in the correct way). + """ + current_epoch = spec.get_current_epoch(state) + + # use a "synthetic" committee to simulate the situation + # where ``spec.get_sync_committee`` at the sync committee + # period epoch boundary would have diverged some epochs into the + # period; ``aggregate_pubkey`` is not relevant to this test + pubkeys = [] + committee_indices = [] + i = 0 + active_validator_count = len(spec.get_active_validator_indices(state, current_epoch)) + while len(pubkeys) < spec.SYNC_COMMITTEE_SIZE: + v = state.validators[i % active_validator_count] + if spec.is_active_validator(v, current_epoch): + pubkeys.append(v.pubkey) + committee_indices.append(i) + i += 1 + + synthetic_committee = spec.SyncCommittee(pubkeys=pubkeys, aggregate_pubkey=spec.BLSPubkey()) + state.current_sync_committee = synthetic_committee + + assert spec.EPOCHS_PER_SYNC_COMMITTEE_PERIOD > 3 + for _ in range(3): + next_epoch(spec, state) + + committee = get_committee_indices(spec, state) + assert 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)