From 859a7d743e48b2ac24b43f084af847801fc4e1ce Mon Sep 17 00:00:00 2001 From: Alex Stokes Date: Tue, 11 May 2021 11:18:03 -0700 Subject: [PATCH] Only allow sync committee period calculation at period boundaries --- specs/altair/beacon-chain.md | 12 +++--- specs/altair/fork.md | 6 ++- .../test_process_sync_committee.py | 41 ------------------- .../test_process_sync_committee_updates.py | 3 +- .../test/altair/sanity/test_blocks.py | 3 +- .../altair/unittests/test_sync_protocol.py | 9 ++-- 6 files changed, 18 insertions(+), 56 deletions(-) diff --git a/specs/altair/beacon-chain.md b/specs/altair/beacon-chain.md index 34a33469e..fac8b3064 100644 --- a/specs/altair/beacon-chain.md +++ b/specs/altair/beacon-chain.md @@ -276,16 +276,14 @@ def has_flag(flags: ParticipationFlags, flag_index: int) -> bool: def get_sync_committee_indices(state: BeaconState, epoch: Epoch) -> Sequence[ValidatorIndex]: """ 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. + for a given ``state`` and ``epoch`` at a sync committee period boundary. """ + assert epoch % EPOCHS_PER_SYNC_COMMITTEE_PERIOD == 0 + 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) + active_validator_indices = get_active_validator_indices(state, epoch) active_validator_count = uint64(len(active_validator_indices)) - seed = get_seed(state, base_epoch, DOMAIN_SYNC_COMMITTEE) + seed = get_seed(state, epoch, DOMAIN_SYNC_COMMITTEE) i = 0 sync_committee_indices: List[ValidatorIndex] = [] while len(sync_committee_indices) < SYNC_COMMITTEE_SIZE: diff --git a/specs/altair/fork.md b/specs/altair/fork.md index be562b82b..4498749d3 100644 --- a/specs/altair/fork.md +++ b/specs/altair/fork.md @@ -81,7 +81,9 @@ def upgrade_to_altair(pre: phase0.BeaconState) -> BeaconState: inactivity_scores=[uint64(0) for _ in range(len(pre.validators))], ) # Fill in sync committees - post.current_sync_committee = get_sync_committee(post, get_current_epoch(post)) - post.next_sync_committee = get_sync_committee(post, get_current_epoch(post) + EPOCHS_PER_SYNC_COMMITTEE_PERIOD) + current_period = epoch // EPOCHS_PER_SYNC_COMMITTEE_PERIOD + base_epoch = current_period * EPOCHS_PER_SYNC_COMMITTEE_PERIOD + post.current_sync_committee = get_sync_committee(post, base_epoch) + post.next_sync_committee = get_sync_committee(post, base_epoch + EPOCHS_PER_SYNC_COMMITTEE_PERIOD) return post ``` 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 2e5d62d11..834f51956 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,7 +7,6 @@ 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 ( MAINNET, MINIMAL, @@ -367,43 +366,3 @@ def test_valid_signature_future_committee(spec, state): ) yield from run_sync_committee_processing(spec, state, block) - - -@with_altair_and_later -@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) diff --git a/tests/core/pyspec/eth2spec/test/altair/epoch_processing/test_process_sync_committee_updates.py b/tests/core/pyspec/eth2spec/test/altair/epoch_processing/test_process_sync_committee_updates.py index c909c791c..2910145c9 100644 --- a/tests/core/pyspec/eth2spec/test/altair/epoch_processing/test_process_sync_committee_updates.py +++ b/tests/core/pyspec/eth2spec/test/altair/epoch_processing/test_process_sync_committee_updates.py @@ -39,8 +39,7 @@ def run_sync_committees_progress_test(spec, state): # Can compute the third committee having computed final balances in the last epoch # of this `EPOCHS_PER_SYNC_COMMITTEE_PERIOD` - current_epoch = spec.get_current_epoch(state) - third_sync_committee = spec.get_sync_committee(state, current_epoch + 2 * spec.EPOCHS_PER_SYNC_COMMITTEE_PERIOD) + third_sync_committee = spec.get_sync_committee(state, (next_period + 1) * spec.EPOCHS_PER_SYNC_COMMITTEE_PERIOD) assert state.current_sync_committee == second_sync_committee assert state.next_sync_committee == third_sync_committee diff --git a/tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py index ffe743531..407455a0a 100644 --- a/tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py @@ -18,7 +18,8 @@ from eth2spec.test.context import ( def run_sync_committee_sanity_test(spec, state, fraction_full=1.0): - committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + all_pubkeys = [v.pubkey for v in state.validators] + committee = [all_pubkeys.index(pubkey) for pubkey in state.current_sync_committee.pubkeys] participants = random.sample(committee, int(len(committee) * fraction_full)) yield 'pre', state diff --git a/tests/core/pyspec/eth2spec/test/altair/unittests/test_sync_protocol.py b/tests/core/pyspec/eth2spec/test/altair/unittests/test_sync_protocol.py index 932a46ca5..554cebda8 100644 --- a/tests/core/pyspec/eth2spec/test/altair/unittests/test_sync_protocol.py +++ b/tests/core/pyspec/eth2spec/test/altair/unittests/test_sync_protocol.py @@ -46,7 +46,8 @@ def test_process_light_client_update_not_updated(spec, state): body_root=signed_block.message.body.hash_tree_root(), ) # Sync committee signing the header - committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + all_pubkeys = [v.pubkey for v in state.validators] + committee = [all_pubkeys.index(pubkey) for pubkey in state.current_sync_committee.pubkeys] sync_committee_bits = [True] * len(committee) sync_committee_signature = compute_aggregate_sync_committee_signature( spec, @@ -111,7 +112,8 @@ def test_process_light_client_update_timeout(spec, state): ) # Sync committee signing the finalized_block_header - committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + all_pubkeys = [v.pubkey for v in state.validators] + committee = [all_pubkeys.index(pubkey) for pubkey in state.current_sync_committee.pubkeys] sync_committee_bits = [True] * len(committee) sync_committee_signature = compute_aggregate_sync_committee_signature( spec, @@ -190,7 +192,8 @@ def test_process_light_client_update_finality_updated(spec, state): ) # Sync committee signing the finalized_block_header - committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) + all_pubkeys = [v.pubkey for v in state.validators] + committee = [all_pubkeys.index(pubkey) for pubkey in state.current_sync_committee.pubkeys] sync_committee_bits = [True] * len(committee) sync_committee_signature = compute_aggregate_sync_committee_signature( spec,