Merge pull request #2394 from ralexstokes/fix-sync-committee-iteration

Use stable sync committee indices when processing block rewards

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]
@@ -571,7 +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 = get_sync_committee_indices(state, get_current_epoch(state))
+    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)

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:
+        if proposer_index == index:
-                reward += compute_sync_committee_proposer_reward(
+            reward += compute_sync_committee_proposer_reward(
-                    spec,
+                spec,
-                    pre_state,
+                pre_state,
-                    committee,
+                committee,
-                    committee_bits,
+                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)