From 1eaa3c174238a2c45bcc5e97dd62ac7a8900f96c Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 15 Mar 2021 21:43:49 +0100 Subject: [PATCH] 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 | 24 ++- .../test_process_sync_committee.py | 184 +++++++++--------- .../test/altair/sanity/test_blocks.py | 14 +- .../pyspec/eth2spec/test/helpers/block.py | 2 +- .../eth2spec/test/helpers/block_processing.py | 60 ++++++ tests/formats/operations/README.md | 17 +- 6 files changed, 188 insertions(+), 113 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/helpers/block_processing.py diff --git a/specs/altair/beacon-chain.md b/specs/altair/beacon-chain.md index 5c266a81c..38090cf11 100644 --- a/specs/altair/beacon-chain.md +++ b/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 - sync_committee_bits: Bitvector[SYNC_COMMITTEE_SIZE] # [New in Altair] - sync_committee_signature: BLSSignature # [New in Altair] + # [New in Altair] + sync_aggregate: SyncAggregate ``` #### `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) 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 430c59b10..58cf48314 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 @@ -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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - committee, # full committee signs + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[index != random_participant for index in committee], + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + committee, # full committee signs + ) ) - - yield 'blocks', [block] - expect_assertion_error(lambda: spec.process_sync_committee(state, block.body)) - yield 'post', None + yield from run_sync_committee_processing(spec, state, block, expect_exception=True) @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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - [index for index in committee if index != random_participant], + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * len(committee), + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + [index for index in committee if index != random_participant], + ) ) - yield 'blocks', [block] - expect_assertion_error(lambda: spec.process_sync_committee(state, block.body)) - yield 'post', None + yield from run_sync_committee_processing(spec, state, block, expect_exception=True) 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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - committee, + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * committee_size, + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + committee, + ) ) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state + yield from run_sync_committee_processing(spec, state, block) 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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - committee, + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * committee_size, + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + committee, + ) ) - - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state + yield from run_sync_committee_processing(spec, state, block) 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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - committee, + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * len(committee), + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + 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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - invalid_block.slot - 2, - committee, + invalid_block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * len(committee), + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + invalid_block.slot - 2, + committee, + ) ) - blocks.append(invalid_block) - expect_assertion_error(lambda: transition_unsigned_block(spec, state, invalid_block)) - - yield 'blocks', blocks - yield 'post', None + yield from run_sync_committee_processing(spec, state, invalid_block, expect_exception=True) @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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - committee, + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * len(committee), + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + committee, + ) ) - yield 'blocks', [block] - expect_assertion_error(lambda: spec.process_sync_committee(state, block.body)) - yield 'post', None + yield from run_sync_committee_processing(spec, state, block, expect_exception=True) @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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - committee_indices, + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[True] * len(committee_indices), + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + committee_indices, + ) ) - signed_block = state_transition_and_sign_block(spec, state, block) - - yield 'blocks', [signed_block] - yield 'post', state + yield from run_sync_committee_processing(spec, state, block) 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 7bd2a91ba..81e0df713 100644 --- a/tests/core/pyspec/eth2spec/test/altair/sanity/test_blocks.py +++ b/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_committee_signature = compute_aggregate_sync_committee_signature( - spec, - state, - block.slot - 1, - participants, + block.body.sync_aggregate = spec.SyncAggregate( + sync_committee_bits=[index in participants for index in committee], + sync_committee_signature=compute_aggregate_sync_committee_signature( + spec, + state, + block.slot - 1, + participants, + ) ) signed_block = state_transition_and_sign_block(spec, state, block) diff --git a/tests/core/pyspec/eth2spec/test/helpers/block.py b/tests/core/pyspec/eth2spec/test/helpers/block.py index 084826d65..6949b54bc 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/block.py +++ b/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 diff --git a/tests/core/pyspec/eth2spec/test/helpers/block_processing.py b/tests/core/pyspec/eth2spec/test/helpers/block_processing.py new file mode 100644 index 000000000..5c8ecb36c --- /dev/null +++ b/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) diff --git a/tests/formats/operations/README.md b/tests/formats/operations/README.md index 51d92a017..033c5fdef 100644 --- a/tests/formats/operations/README.md +++ b/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`* | -|-------------------------|-----------------------|----------------------|--------------------------------------------------------| -| `attestation` | `Attestation` | `attestation` | `process_attestation(state, attestation)` | -| `attester_slashing` | `AttesterSlashing` | `attester_slashing` | `process_attester_slashing(state, attester_slashing)` | -| `block_header` | `BeaconBlock` | **`block`** | `process_block_header(state, block)` | -| `deposit` | `Deposit` | `deposit` | `process_deposit(state, deposit)` | -| `proposer_slashing` | `ProposerSlashing` | `proposer_slashing` | `process_proposer_slashing(state, proposer_slashing)` | -| `voluntary_exit` | `SignedVoluntaryExit` | `voluntary_exit` | `process_voluntary_exit(state, voluntary_exit)` | +| *`operation-name`* | *`operation-object`* | *`input name`* | *`processing call`* | +|-------------------------|-----------------------|----------------------|-----------------------------------------------------------------| +| `attestation` | `Attestation` | `attestation` | `process_attestation(state, attestation)` | +| `attester_slashing` | `AttesterSlashing` | `attester_slashing` | `process_attester_slashing(state, attester_slashing)` | +| `block_header` | `BeaconBlock` | **`block`** | `process_block_header(state, block)` | +| `deposit` | `Deposit` | `deposit` | `process_deposit(state, deposit)` | +| `proposer_slashing` | `ProposerSlashing` | `proposer_slashing` | `process_proposer_slashing(state, proposer_slashing)` | +| `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.