Define SyncAggregate to bundle sync committee bits and signature, update tests to better isolate the state-change, introduce helper function for future tests, and update test doc

specs/altair/beacon-chain.md

@@ -22,6 +22,7 @@
     - [`BeaconBlockBody`](#beaconblockbody)
     - [`BeaconState`](#beaconstate)
   - [New containers](#new-containers)
+    - [`SyncAggregate`](#syncaggregate)
     - [`SyncCommittee`](#synccommittee)
 - [Helper functions](#helper-functions)
   - [`Predicates`](#predicates)
@@ -146,9 +147,8 @@ class BeaconBlockBody(Container):
     attestations: List[Attestation, MAX_ATTESTATIONS]
     deposits: List[Deposit, MAX_DEPOSITS]
     voluntary_exits: List[SignedVoluntaryExit, MAX_VOLUNTARY_EXITS]
-    # Sync committee aggregate signature
+    # [New in Altair]
-    sync_committee_bits: Bitvector[SYNC_COMMITTEE_SIZE]  # [New in Altair]
+    sync_aggregate: SyncAggregate
-    sync_committee_signature: BLSSignature  # [New in Altair]
 ```
 
 #### `BeaconState`
@@ -193,6 +193,14 @@ class BeaconState(Container):
 
 ### New containers
 
+#### `SyncAggregate`
+
+```python
+class SyncAggregate(Container):
+    sync_committee_bits: Bitvector[SYNC_COMMITTEE_SIZE]
+    sync_committee_signature: BLSSignature
+```
+
 #### `SyncCommittee`
 
 ```python
@@ -409,7 +417,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None:
     process_randao(state, block.body)
     process_eth1_data(state, block.body)
     process_operations(state, block.body)  # [Modified in Altair]
-    process_sync_committee(state, block.body)  # [New in Altair]
+    process_sync_committee(state, block.body.sync_aggregate)  # [New in Altair]
 ```
 
 #### Modified `process_attestation`
@@ -511,16 +519,16 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None:
 #### Sync committee processing
 
 ```python
-def process_sync_committee(state: BeaconState, body: BeaconBlockBody) -> None:
+def process_sync_committee(state: BeaconState, aggregate: SyncAggregate) -> None:
     # Verify sync committee aggregate signature signing over the previous slot block root
     previous_slot = Slot(max(int(state.slot), 1) - 1)
     committee_indices = get_sync_committee_indices(state, get_current_epoch(state))
-    participant_indices = [index for index, bit in zip(committee_indices, body.sync_committee_bits) if bit]
+    participant_indices = [index for index, bit in zip(committee_indices, aggregate.sync_committee_bits) if bit]
     committee_pubkeys = state.current_sync_committee.pubkeys
-    participant_pubkeys = [pubkey for pubkey, bit in zip(committee_pubkeys, body.sync_committee_bits) if bit]
+    participant_pubkeys = [pubkey for pubkey, bit in zip(committee_pubkeys, aggregate.sync_committee_bits) if bit]
     domain = get_domain(state, DOMAIN_SYNC_COMMITTEE, compute_epoch_at_slot(previous_slot))
     signing_root = compute_signing_root(get_block_root_at_slot(state, previous_slot), domain)
-    assert eth2_fast_aggregate_verify(participant_pubkeys, signing_root, body.sync_committee_signature)
+    assert eth2_fast_aggregate_verify(participant_pubkeys, signing_root, aggregate.sync_committee_signature)
 
     # Reward sync committee participants
     proposer_rewards = Gwei(0)

tests/core/pyspec/eth2spec/test/altair/block_processing/test_process_sync_committee.py

@@ -2,8 +2,8 @@ from collections import Counter
 import random
 from eth2spec.test.helpers.block import (
     build_empty_block_for_next_slot,
-    transition_unsigned_block,
 )
+from eth2spec.test.helpers.block_processing import run_block_processing_to
 from eth2spec.test.helpers.state import (
     state_transition_and_sign_block,
     transition_to,
@@ -23,12 +23,29 @@ from eth2spec.test.context import (
 from eth2spec.utils.hash_function import hash
 
 
+def run_sync_committee_processing(spec, state, block, expect_exception=False):
+    """
+    Processes everything up to the sync committee work, then runs the sync committee work in isolation, and
+    produces a pre-state and post-state (None if exception) specifically for sync-committee processing changes.
+    """
+    # process up to the sync committee work
+    call = run_block_processing_to(spec, state, block, 'process_sync_committee')
+    yield 'pre', state
+    yield 'sync_aggregate', block.body.sync_aggregate
+    if expect_exception:
+        expect_assertion_error(lambda: call(state, block))
+        yield 'post', None
+    else:
+        call(state, block)
+        yield 'post', state
+
+
 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
@@ -45,6 +62,7 @@ def get_committee_indices(spec, state, duplicates=False):
 
 @with_all_phases_except([PHASE0, PHASE1])
 @spec_state_test
+@always_bls
 def test_invalid_signature_missing_participant(spec, state):
     committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state))
     random_participant = random.choice(committee)
@@ -53,40 +71,38 @@ 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_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[index != random_participant for index in committee],
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        committee,  # full committee signs
+            block.slot - 1,
+            committee,  # full committee signs
+        )
     )
-
+    yield from run_sync_committee_processing(spec, state, block, expect_exception=True)
-    yield 'blocks', [block]
-    expect_assertion_error(lambda: spec.process_sync_committee(state, block.body))
-    yield 'post', None
 
 
 @with_all_phases_except([PHASE0, PHASE1])
 @spec_state_test
+@always_bls
 def test_invalid_signature_extra_participant(spec, state):
     committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state))
     random_participant = random.choice(committee)
 
-    yield 'pre', 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_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[True] * len(committee),
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        [index for index in committee if index != random_participant],
+            block.slot - 1,
+            [index for index in committee if index != random_participant],
+        )
     )
 
-    yield 'blocks', [block]
+    yield from run_sync_committee_processing(spec, state, block, expect_exception=True)
-    expect_assertion_error(lambda: spec.process_sync_committee(state, block.body))
-    yield 'post', None
 
 
 def compute_sync_committee_participant_reward(spec, state, participant_index, active_validator_count, committee_size):
@@ -113,18 +129,17 @@ def test_sync_committee_rewards_nonduplicate_committee(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_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[True] * committee_size,
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        committee,
+            block.slot - 1,
+            committee,
+        )
     )
 
-    signed_block = state_transition_and_sign_block(spec, state, block)
+    yield from run_sync_committee_processing(spec, state, block)
-
-    yield 'blocks', [signed_block]
-    yield 'post', state
 
     for index in range(len(state.validators)):
         expected_reward = 0
@@ -156,23 +171,18 @@ def test_sync_committee_rewards_duplicate_committee(spec, state):
     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_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[True] * committee_size,
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        committee,
+            block.slot - 1,
+            committee,
+        )
     )
-
+    yield from run_sync_committee_processing(spec, state, block)
-    signed_block = state_transition_and_sign_block(spec, state, block)
-
-    yield 'blocks', [signed_block]
-    yield 'post', state
 
     multiplicities = Counter(committee)
 
@@ -201,19 +211,19 @@ def test_sync_committee_rewards_duplicate_committee(spec, state):
 def test_invalid_signature_past_block(spec, state):
     committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state))
 
-    yield 'pre', state
-
     blocks = []
     for _ in range(2):
         # 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_aggregate = spec.SyncAggregate(
-        block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+            sync_committee_bits=[True] * len(committee),
-            spec,
+            sync_committee_signature=compute_aggregate_sync_committee_signature(
-            state,
+                spec,
-            block.slot - 1,
+                state,
-            committee,
+                block.slot - 1,
+                committee,
+            )
         )
 
         signed_block = state_transition_and_sign_block(spec, state, block)
@@ -221,19 +231,17 @@ 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_aggregate = spec.SyncAggregate(
-    invalid_block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[True] * len(committee),
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        invalid_block.slot - 2,
+            state,
-        committee,
+            invalid_block.slot - 2,
+            committee,
+        )
     )
-    blocks.append(invalid_block)
 
-    expect_assertion_error(lambda: transition_unsigned_block(spec, state, invalid_block))
+    yield from run_sync_committee_processing(spec, state, invalid_block, expect_exception=True)
-
-    yield 'blocks', blocks
-    yield 'post', None
 
 
 @with_all_phases_except([PHASE0, PHASE1])
@@ -253,8 +261,6 @@ def test_invalid_signature_previous_committee(spec, state):
     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)
 
-    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.
@@ -262,21 +268,22 @@ def test_invalid_signature_previous_committee(spec, state):
     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(committee)
+    block.body.sync_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[True] * len(committee),
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        committee,
+            block.slot - 1,
+            committee,
+        )
     )
 
-    yield 'blocks', [block]
+    yield from run_sync_committee_processing(spec, state, block, expect_exception=True)
-    expect_assertion_error(lambda: spec.process_sync_committee(state, block.body))
-    yield 'post', None
 
 
 @with_all_phases_except([PHASE0, PHASE1])
 @spec_state_test
+@always_bls
 def test_valid_signature_future_committee(spec, state):
     # NOTE: the `state` provided is at genesis and the process to select
     # sync committees currently returns the same committee for the first and second
@@ -302,18 +309,15 @@ def test_valid_signature_future_committee(spec, state):
     pubkeys = [validator.pubkey for validator in state.validators]
     committee_indices = [pubkeys.index(pubkey) for pubkey in sync_committee.pubkeys]
 
-    yield 'pre', state
-
     block = build_empty_block_for_next_slot(spec, state)
-    block.body.sync_committee_bits = [True] * len(committee_indices)
+    block.body.sync_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[True] * len(committee_indices),
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        committee_indices,
+            block.slot - 1,
+            committee_indices,
+        )
     )
 
-    signed_block = state_transition_and_sign_block(spec, state, block)
+    yield from run_sync_committee_processing(spec, state, block)
-
-    yield 'blocks', [signed_block]
-    yield 'post', state

tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py

@@ -25,12 +25,14 @@ 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_aggregate = spec.SyncAggregate(
-    block.body.sync_committee_signature = compute_aggregate_sync_committee_signature(
+        sync_committee_bits=[index in participants for index in committee],
-        spec,
+        sync_committee_signature=compute_aggregate_sync_committee_signature(
-        state,
+            spec,
-        block.slot - 1,
+            state,
-        participants,
+            block.slot - 1,
+            participants,
+        )
     )
     signed_block = state_transition_and_sign_block(spec, state, block)
 

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

@@ -92,7 +92,7 @@ def build_empty_block(spec, state, slot=None):
     empty_block.parent_root = parent_block_root
 
     if is_post_altair(spec):
-        empty_block.body.sync_committee_signature = spec.G2_POINT_AT_INFINITY
+        empty_block.body.sync_aggregate.sync_committee_signature = spec.G2_POINT_AT_INFINITY
 
     apply_randao_reveal(spec, state, empty_block)
     return empty_block

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

@@ -0,0 +1,60 @@
+def for_ops(state, operations, fn) -> None:
+    for operation in operations:
+        fn(state, operation)
+
+
+def get_process_calls(spec):
+    return {
+        # PHASE0
+        'process_block_header':
+            lambda state, block: spec.process_block_header(state, block),
+        'process_randao':
+            lambda state, block: spec.process_randao(state, block.body),
+        'process_eth1_data':
+            lambda state, block: spec.process_eth1_data(state, block.body),
+        'process_proposer_slashing':
+            lambda state, block: for_ops(state, block.body.proposer_slashings, spec.process_proposer_slashing),
+        'process_attester_slashing':
+            lambda state, block: for_ops(state, block.body.attester_slashings, spec.process_attester_slashing),
+        'process_attestation':
+            lambda state, block: for_ops(state, block.body.attestations, spec.process_attestation),
+        'process_deposit':
+            lambda state, block: for_ops(state, block.body.deposits, spec.process_deposit),
+        'process_voluntary_exit':
+            lambda state, block: for_ops(state, block.body.voluntary_exits, spec.process_voluntary_exit),
+        # Altair
+        'process_sync_committee':
+            lambda state, block: spec.process_sync_committee(state, block.body.sync_aggregate),
+        # PHASE1
+        'process_custody_game_operations':
+            lambda state, block: spec.process_custody_game_operations(state, block.body),
+        'process_shard_transitions':
+            lambda state, block: spec.process_shard_transitions(
+                state, block.body.shard_transitions, block.body.attestations),
+    }
+
+
+def run_block_processing_to(spec, state, block, process_name: str):
+    """
+    Processes to the block transition, up to, but not including, the sub-transition named ``process_name``.
+    Returns a Callable[[state, block], None] for the remaining ``process_name`` transition.
+
+    Tests should create full blocks to ensure a valid state transition, even if the operation itself is isolated.
+    (e.g. latest_header in the beacon state is up-to-date in a sync-committee test).
+
+    A test prepares a pre-state by calling this function, output the pre-state,
+     and it can then proceed to run the returned callable, and output a post-state.
+    """
+    print(f"state.slot {state.slot} block.slot {block.slot}")
+    # transition state to slot before block state transition
+    if state.slot < block.slot:
+        spec.process_slots(state, block.slot)
+    print(f"state.slot {state.slot} block.slot {block.slot}  A")
+
+    # process components of block transition
+    for name, call in get_process_calls(spec).items():
+        if name == process_name:
+            return call
+        # only run when present. Later phases introduce more to the block-processing.
+        if hasattr(spec, name):
+            call(state, block)

tests/formats/operations/README.md

@@ -33,14 +33,15 @@ This excludes the other parts of the block-transition.
 
 Operations:
 
-| *`operation-name`*      | *`operation-object`*  | *`input name`*       | *`processing call`*                                    |
+| *`operation-name`*      | *`operation-object`*  | *`input name`*       | *`processing call`*                                             |
-|-------------------------|-----------------------|----------------------|--------------------------------------------------------|
+|-------------------------|-----------------------|----------------------|-----------------------------------------------------------------|
-| `attestation`           | `Attestation`         | `attestation`        | `process_attestation(state, attestation)`              |
+| `attestation`           | `Attestation`         | `attestation`        | `process_attestation(state, attestation)`                       |
-| `attester_slashing`     | `AttesterSlashing`    | `attester_slashing`  | `process_attester_slashing(state, attester_slashing)`  |
+| `attester_slashing`     | `AttesterSlashing`    | `attester_slashing`  | `process_attester_slashing(state, attester_slashing)`           |
-| `block_header`          | `BeaconBlock`         | **`block`**          | `process_block_header(state, block)`                   |
+| `block_header`          | `BeaconBlock`         | **`block`**          | `process_block_header(state, block)`                            |
-| `deposit`               | `Deposit`             | `deposit`            | `process_deposit(state, deposit)`                      |
+| `deposit`               | `Deposit`             | `deposit`            | `process_deposit(state, deposit)`                               |
-| `proposer_slashing`     | `ProposerSlashing`    | `proposer_slashing`  | `process_proposer_slashing(state, proposer_slashing)`  |
+| `proposer_slashing`     | `ProposerSlashing`    | `proposer_slashing`  | `process_proposer_slashing(state, proposer_slashing)`           |
-| `voluntary_exit`        | `SignedVoluntaryExit` | `voluntary_exit`     | `process_voluntary_exit(state, voluntary_exit)`        |
+| `voluntary_exit`        | `SignedVoluntaryExit` | `voluntary_exit`     | `process_voluntary_exit(state, voluntary_exit)`                 |
+| `sync_aggregate`        | `SyncAggregate`       | `sync_aggregate`     | `process_sync_committee(state, sync_aggregate)` (new in Altair) |
 
 Note that `block_header` is not strictly an operation (and is a full `Block`), but processed in the same manner, and hence included here.