Merge pull request #2826 from ethereum/get_sync_aggregate

Minor refactoring for test_sync_protocol.py

specs/altair/sync-protocol.md

@@ -17,12 +17,12 @@
   - [`LightClientStore`](#lightclientstore)
 - [Helper functions](#helper-functions)
   - [`get_subtree_index`](#get_subtree_index)
-  - [`get_active_header`](#get_active_header)
   - [`get_safety_threshold`](#get_safety_threshold)
 - [Light client state updates](#light-client-state-updates)
     - [`process_slot_for_light_client_store`](#process_slot_for_light_client_store)
     - [`validate_light_client_update`](#validate_light_client_update)
-    - [`apply_light_client_update`](#apply_light_client_update)
+    - [`update_sync_committees_from_update`](#update_sync_committees_from_update)
+    - [`update_new_finalized_header`](#update_new_finalized_header)
     - [`process_light_client_update`](#process_light_client_update)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
@@ -62,7 +62,7 @@ uses sync committees introduced in [this beacon chain extension](./beacon-chain.
 class LightClientUpdate(Container):
     # The beacon block header that is attested to by the sync committee
     attested_header: BeaconBlockHeader
-    # Next sync committee corresponding to the active header
+    # Next sync committee corresponding to `attested_header`
     next_sync_committee: SyncCommittee
     next_sync_committee_branch: Vector[Bytes32, floorlog2(NEXT_SYNC_COMMITTEE_INDEX)]
     # The finalized beacon block header attested to by Merkle branch
@@ -102,19 +102,6 @@ def get_subtree_index(generalized_index: GeneralizedIndex) -> uint64:
     return uint64(generalized_index % 2**(floorlog2(generalized_index)))
 ```
 
-### `get_active_header`
-
-```python
-def get_active_header(update: LightClientUpdate) -> BeaconBlockHeader:
-    # The "active header" is the header that the update is trying to convince us
-    # to accept. If a finalized header is present, it's the finalized header,
-    # otherwise it's the attested header
-    if update.finalized_header != BeaconBlockHeader():
-        return update.finalized_header
-    else:
-        return update.attested_header
-```
-
 ### `get_safety_threshold`
 
 ```python
@@ -141,8 +128,8 @@ def process_slot_for_light_client_store(store: LightClientStore, current_slot: S
         and store.best_valid_update is not None
     ):
         # Forced best update when the update timeout has elapsed
-        apply_light_client_update(store, store.best_valid_update)
-        store.best_valid_update = None
+        update_sync_committees_from_update(store, store.best_valid_update)
+        update_new_finalized_header(store, store.best_valid_update.attested_header)
 ```
 
 #### `validate_light_client_update`
@@ -153,12 +140,11 @@ def validate_light_client_update(store: LightClientStore,
                                  current_slot: Slot,
                                  genesis_validators_root: Root) -> None:
     # Verify update slot is larger than slot of current best finalized header
-    active_header = get_active_header(update)
-    assert current_slot >= active_header.slot > store.finalized_header.slot
+    assert current_slot >= update.attested_header.slot >= store.finalized_header.slot
 
     # Verify update does not skip a sync committee period
     finalized_period = compute_sync_committee_period(compute_epoch_at_slot(store.finalized_header.slot))
-    update_period = compute_sync_committee_period(compute_epoch_at_slot(active_header.slot))
+    update_period = compute_sync_committee_period(compute_epoch_at_slot(update.attested_header.slot))
     assert update_period in (finalized_period, finalized_period + 1)
 
     # Verify that the `finalized_header`, if present, actually is the finalized header saved in the
@@ -177,6 +163,7 @@ def validate_light_client_update(store: LightClientStore,
     # Verify update next sync committee if the update period incremented
     if update_period == finalized_period:
         sync_committee = store.current_sync_committee
+
         assert update.next_sync_committee_branch == [Bytes32() for _ in range(floorlog2(NEXT_SYNC_COMMITTEE_INDEX))]
     else:
         sync_committee = store.next_sync_committee
@@ -185,7 +172,7 @@ def validate_light_client_update(store: LightClientStore,
             branch=update.next_sync_committee_branch,
             depth=floorlog2(NEXT_SYNC_COMMITTEE_INDEX),
             index=get_subtree_index(NEXT_SYNC_COMMITTEE_INDEX),
-            root=active_header.state_root,
+            root=update.attested_header.state_root,
         )
 
     sync_aggregate = update.sync_aggregate
@@ -203,17 +190,23 @@ def validate_light_client_update(store: LightClientStore,
     assert bls.FastAggregateVerify(participant_pubkeys, signing_root, sync_aggregate.sync_committee_signature)
 ```
 
-#### `apply_light_client_update`
+#### `update_sync_committees_from_update`
 
 ```python
-def apply_light_client_update(store: LightClientStore, update: LightClientUpdate) -> None:
-    active_header = get_active_header(update)
+def update_sync_committees_from_update(store: LightClientStore, update: LightClientUpdate) -> None:
     finalized_period = compute_sync_committee_period(compute_epoch_at_slot(store.finalized_header.slot))
-    update_period = compute_sync_committee_period(compute_epoch_at_slot(active_header.slot))
+    update_period = compute_sync_committee_period(compute_epoch_at_slot(update.attested_header.slot))
     if update_period == finalized_period + 1:
         store.current_sync_committee = store.next_sync_committee
         store.next_sync_committee = update.next_sync_committee
-    store.finalized_header = active_header
+```
+
+#### `update_new_finalized_header`
+
+```python
+def update_new_finalized_header(store: LightClientStore, header: BeaconBlockHeader) -> None:
+    store.finalized_header = header
+    store.best_valid_update = None
     if store.finalized_header.slot > store.optimistic_header.slot:
         store.optimistic_header = store.finalized_header
 ```
@@ -255,6 +248,6 @@ def process_light_client_update(store: LightClientStore,
         and update.finalized_header != BeaconBlockHeader()
     ):
         # Normal update through 2/3 threshold
-        apply_light_client_update(store, update)
-        store.best_valid_update = None
+        update_sync_committees_from_update(store, update)
+        update_new_finalized_header(store, update.finalized_header)
 ```

tests/core/pyspec/eth2spec/test/altair/unittests/test_sync_protocol.py

@@ -5,38 +5,29 @@
     with_presets,
     with_altair_and_later,
 )
-from eth2spec.test.helpers.attestations import next_epoch_with_attestations
+from eth2spec.test.helpers.attestations import (
+    next_epoch_with_attestations,
+)
 from eth2spec.test.helpers.block import (
     build_empty_block,
     build_empty_block_for_next_slot,
 )
 from eth2spec.test.helpers.constants import MINIMAL
+from eth2spec.test.helpers.light_client import (
+    get_sync_aggregate,
+    initialize_light_client_store,
+)
 from eth2spec.test.helpers.state import (
     next_slots,
     state_transition_and_sign_block,
 )
-from eth2spec.test.helpers.sync_committee import (
-    compute_aggregate_sync_committee_signature,
-)
 from eth2spec.test.helpers.merkle import build_proof
 
 
-def _initialize_light_client_store(spec, state):
-    return spec.LightClientStore(
-        finalized_header=spec.BeaconBlockHeader(),
-        current_sync_committee=state.current_sync_committee,
-        next_sync_committee=state.next_sync_committee,
-        best_valid_update=None,
-        optimistic_header=spec.BeaconBlockHeader(),
-        previous_max_active_participants=0,
-        current_max_active_participants=0,
-    )
-
-
 @with_altair_and_later
 @spec_state_test
 def test_process_light_client_update_not_timeout(spec, state):
-    store = _initialize_light_client_store(spec, state)
+    store = initialize_light_client_store(spec, state)
 
     # Block at slot 1 doesn't increase sync committee period, so it won't force update store.finalized_header
     block = build_empty_block_for_next_slot(spec, state)
@@ -49,19 +40,7 @@ def test_process_light_client_update_not_timeout(spec, state):
         body_root=signed_block.message.body.hash_tree_root(),
     )
     # Sync committee signing the header
-    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,
-        state,
-        block_header.slot,
-        committee,
-    )
-    sync_aggregate = spec.SyncAggregate(
-        sync_committee_bits=sync_committee_bits,
-        sync_committee_signature=sync_committee_signature,
-    )
+    sync_aggregate = get_sync_aggregate(spec, state, block_header, block_root=None)
     next_sync_committee_branch = [spec.Bytes32() for _ in range(spec.floorlog2(spec.NEXT_SYNC_COMMITTEE_INDEX))]
 
     # Ensure that finality checkpoint is genesis
@@ -94,7 +73,7 @@ def test_process_light_client_update_not_timeout(spec, state):
 @spec_state_test
 @with_presets([MINIMAL], reason="too slow")
 def test_process_light_client_update_timeout(spec, state):
-    store = _initialize_light_client_store(spec, state)
+    store = initialize_light_client_store(spec, state)
 
     # Forward to next sync committee period
     next_slots(spec, state, spec.UPDATE_TIMEOUT)
@@ -113,20 +92,8 @@ def test_process_light_client_update_timeout(spec, state):
     )
 
     # Sync committee signing the finalized_block_header
-    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,
-        state,
-        block_header.slot,
-        committee,
-        block_root=spec.Root(block_header.hash_tree_root()),
-    )
-    sync_aggregate = spec.SyncAggregate(
-        sync_committee_bits=sync_committee_bits,
-        sync_committee_signature=sync_committee_signature,
-    )
+    sync_aggregate = get_sync_aggregate(
+        spec, state, block_header, block_root=spec.Root(block_header.hash_tree_root()))
 
     # Sync committee is updated
     next_sync_committee_branch = build_proof(state.get_backing(), spec.NEXT_SYNC_COMMITTEE_INDEX)
@@ -158,7 +125,7 @@ def test_process_light_client_update_timeout(spec, state):
 @spec_state_test
 @with_presets([MINIMAL], reason="too slow")
 def test_process_light_client_update_finality_updated(spec, state):
-    store = _initialize_light_client_store(spec, state)
+    store = initialize_light_client_store(spec, state)
 
     # Change finality
     blocks = []
@@ -191,20 +158,8 @@ def test_process_light_client_update_finality_updated(spec, state):
     )
 
     # Sync committee signing the finalized_block_header
-    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,
-        state,
-        block_header.slot,
-        committee,
-        block_root=spec.Root(block_header.hash_tree_root()),
-    )
-    sync_aggregate = spec.SyncAggregate(
-        sync_committee_bits=sync_committee_bits,
-        sync_committee_signature=sync_committee_signature,
-    )
+    sync_aggregate = get_sync_aggregate(
+        spec, state, block_header, block_root=spec.Root(block_header.hash_tree_root()))
 
     update = spec.LightClientUpdate(
         attested_header=block_header,

tests/core/pyspec/eth2spec/test/helpers/light_client.py

@@ -0,0 +1,35 @@
+from eth2spec.test.helpers.sync_committee import (
+    compute_aggregate_sync_committee_signature,
+)
+
+
+def initialize_light_client_store(spec, state):
+    return spec.LightClientStore(
+        finalized_header=spec.BeaconBlockHeader(),
+        current_sync_committee=state.current_sync_committee,
+        next_sync_committee=state.next_sync_committee,
+        best_valid_update=None,
+        optimistic_header=spec.BeaconBlockHeader(),
+        previous_max_active_participants=0,
+        current_max_active_participants=0,
+    )
+
+
+def get_sync_aggregate(spec, state, block_header, block_root=None, signature_slot=None):
+    if signature_slot is None:
+        signature_slot = block_header.slot
+
+    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,
+        state,
+        block_header.slot,
+        committee,
+        block_root=block_root,
+    )
+    return spec.SyncAggregate(
+        sync_committee_bits=sync_committee_bits,
+        sync_committee_signature=sync_committee_signature,
+    )