From 5653649ca8cc80fb732f68c9e7f4aafb3f3a1ff6 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Tue, 26 Apr 2022 22:32:25 +0200 Subject: [PATCH] Allow light client to verify signatures at period boundary As the sync committee signs the previous block, the situation arises at every sync committee period boundary, that the new sync committee signs a block in the previous sync committee period. The light client cannot reliably detect this condition (e.g., assume that this is the case when it is currently on the last slot of a sync committee period), because the last couple slots of a sync committee period may not have a block. For example, when receiving a `LightClientUpdate` that is constructed as in the following illustration, it is unknown whether `sync_aggregate` was signed by the current or next sync committee at `attested_header`. ``` slot N N + 1 | N + 2 (slot not sent!) | +-----------------+ \ / | +----------------+ | attested_header | <--- X ----|---- | sync_aggregate | +-----------------+ / \ | +----------------+ missed | | sync committee period boundary ``` This patch addresses this edge case by including the slot at which the `sync_aggregate` was created into the `LightClientUpdate` object. Note that the `signature_slot` cannot be trusted beyond the purpose of signature verification, as it could be manipulated to any other slot within the same sync committee period and fork version, without making the `sync_aggregate` invalid. --- specs/altair/sync-protocol.md | 17 ++-- .../altair/unittests/test_sync_protocol.py | 81 +++++++++++++++---- .../eth2spec/test/helpers/light_client.py | 34 +++++--- 3 files changed, 102 insertions(+), 30 deletions(-) diff --git a/specs/altair/sync-protocol.md b/specs/altair/sync-protocol.md index 8751ba6e8..1aa9316d3 100644 --- a/specs/altair/sync-protocol.md +++ b/specs/altair/sync-protocol.md @@ -52,7 +52,7 @@ uses sync committees introduced in [this beacon chain extension](./beacon-chain. | Name | Value | Unit | Duration | | - | - | - | - | -| `MIN_SYNC_COMMITTEE_PARTICIPANTS` | `1` | validators | +| `MIN_SYNC_COMMITTEE_PARTICIPANTS` | `1` | validators | | | `UPDATE_TIMEOUT` | `SLOTS_PER_EPOCH * EPOCHS_PER_SYNC_COMMITTEE_PERIOD` | slots | ~27.3 hours | ## Containers @@ -73,6 +73,8 @@ class LightClientUpdate(Container): sync_aggregate: SyncAggregate # Fork version for the aggregate signature fork_version: Version + # Slot at which the aggregate signature was created (untrusted) + signature_slot: Slot ``` ### `LightClientStore` @@ -162,15 +164,16 @@ def validate_light_client_update(store: LightClientStore, 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.signature_slot > active_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)) - assert update_period in (finalized_period, finalized_period + 1) + signature_period = compute_sync_committee_period(compute_epoch_at_slot(update.signature_slot)) + assert signature_period in (finalized_period, finalized_period + 1) # Verify that the `finalized_header`, if present, actually is the finalized header saved in the - # state of the `attested header` + # state of the `attested_header` if not is_finality_update(update): assert update.finality_branch == [Bytes32() for _ in range(floorlog2(FINALIZED_ROOT_INDEX))] else: @@ -184,10 +187,8 @@ 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 assert is_valid_merkle_branch( leaf=hash_tree_root(update.next_sync_committee), branch=update.next_sync_committee_branch, @@ -202,6 +203,10 @@ def validate_light_client_update(store: LightClientStore, assert sum(sync_aggregate.sync_committee_bits) >= MIN_SYNC_COMMITTEE_PARTICIPANTS # Verify sync committee aggregate signature + if signature_period == finalized_period: + sync_committee = store.current_sync_committee + else: + sync_committee = store.next_sync_committee participant_pubkeys = [ pubkey for (bit, pubkey) in zip(sync_aggregate.sync_committee_bits, sync_committee.pubkeys) if bit 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 04553b3f8..6411ecb6a 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 @@ -39,8 +39,9 @@ def test_process_light_client_update_not_timeout(spec, state): state_root=signed_block.message.state_root, body_root=signed_block.message.body.hash_tree_root(), ) - # Sync committee signing the header - sync_aggregate = get_sync_aggregate(spec, state, block_header, block_root=None) + + # Sync committee signing the block_header + sync_aggregate, fork_version, signature_slot = get_sync_aggregate(spec, state, block_header) next_sync_committee_branch = [spec.Bytes32() for _ in range(spec.floorlog2(spec.NEXT_SYNC_COMMITTEE_INDEX))] # Ensure that finality checkpoint is genesis @@ -56,12 +57,13 @@ def test_process_light_client_update_not_timeout(spec, state): finalized_header=finality_header, finality_branch=finality_branch, sync_aggregate=sync_aggregate, - fork_version=state.fork.current_version, + fork_version=fork_version, + signature_slot=signature_slot, ) pre_store = deepcopy(store) - spec.process_light_client_update(store, update, state.slot, state.genesis_validators_root) + spec.process_light_client_update(store, update, signature_slot, state.genesis_validators_root) assert store.current_max_active_participants > 0 assert store.optimistic_header == update.attested_header @@ -69,6 +71,57 @@ def test_process_light_client_update_not_timeout(spec, state): assert store.best_valid_update == update +@with_altair_and_later +@spec_state_test +@with_presets([MINIMAL], reason="too slow") +def test_process_light_client_update_at_period_boundary(spec, state): + store = initialize_light_client_store(spec, state) + + # Forward to slot before next sync committee period so that next block is final one in period + next_slots(spec, state, spec.UPDATE_TIMEOUT - 2) + snapshot_period = spec.compute_sync_committee_period(spec.compute_epoch_at_slot(store.optimistic_header.slot)) + update_period = spec.compute_sync_committee_period(spec.compute_epoch_at_slot(state.slot)) + assert snapshot_period == update_period + + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + block_header = spec.BeaconBlockHeader( + slot=signed_block.message.slot, + proposer_index=signed_block.message.proposer_index, + parent_root=signed_block.message.parent_root, + state_root=signed_block.message.state_root, + body_root=signed_block.message.body.hash_tree_root(), + ) + + # Sync committee signing the block_header + sync_aggregate, fork_version, signature_slot = get_sync_aggregate(spec, state, block_header) + next_sync_committee_branch = [spec.Bytes32() for _ in range(spec.floorlog2(spec.NEXT_SYNC_COMMITTEE_INDEX))] + + # Finality is unchanged + finality_header = spec.BeaconBlockHeader() + finality_branch = [spec.Bytes32() for _ in range(spec.floorlog2(spec.FINALIZED_ROOT_INDEX))] + + update = spec.LightClientUpdate( + attested_header=block_header, + next_sync_committee=state.next_sync_committee, + next_sync_committee_branch=next_sync_committee_branch, + finalized_header=finality_header, + finality_branch=finality_branch, + sync_aggregate=sync_aggregate, + fork_version=fork_version, + signature_slot=signature_slot, + ) + + pre_store = deepcopy(store) + + spec.process_light_client_update(store, update, signature_slot, state.genesis_validators_root) + + assert store.current_max_active_participants > 0 + assert store.optimistic_header == update.attested_header + assert store.best_valid_update == update + assert store.finalized_header == pre_store.finalized_header + + @with_altair_and_later @spec_state_test @with_presets([MINIMAL], reason="too slow") @@ -91,9 +144,8 @@ def test_process_light_client_update_timeout(spec, state): body_root=signed_block.message.body.hash_tree_root(), ) - # Sync committee signing the finalized_block_header - sync_aggregate = get_sync_aggregate( - spec, state, block_header, block_root=spec.Root(block_header.hash_tree_root())) + # Sync committee signing the block_header + sync_aggregate, fork_version, signature_slot = get_sync_aggregate(spec, state, block_header) # Sync committee is updated next_sync_committee_branch = build_proof(state.get_backing(), spec.NEXT_SYNC_COMMITTEE_INDEX) @@ -108,12 +160,13 @@ def test_process_light_client_update_timeout(spec, state): finalized_header=finality_header, finality_branch=finality_branch, sync_aggregate=sync_aggregate, - fork_version=state.fork.current_version, + fork_version=fork_version, + signature_slot=signature_slot, ) pre_store = deepcopy(store) - spec.process_light_client_update(store, update, state.slot, state.genesis_validators_root) + spec.process_light_client_update(store, update, signature_slot, state.genesis_validators_root) assert store.current_max_active_participants > 0 assert store.optimistic_header == update.attested_header @@ -157,9 +210,8 @@ def test_process_light_client_update_finality_updated(spec, state): body_root=block.body.hash_tree_root(), ) - # Sync committee signing the finalized_block_header - sync_aggregate = get_sync_aggregate( - spec, state, block_header, block_root=spec.Root(block_header.hash_tree_root())) + # Sync committee signing the block_header + sync_aggregate, fork_version, signature_slot = get_sync_aggregate(spec, state, block_header) update = spec.LightClientUpdate( attested_header=block_header, @@ -168,10 +220,11 @@ def test_process_light_client_update_finality_updated(spec, state): finalized_header=finalized_block_header, finality_branch=finality_branch, sync_aggregate=sync_aggregate, - fork_version=state.fork.current_version, + fork_version=fork_version, + signature_slot=signature_slot, ) - spec.process_light_client_update(store, update, state.slot, state.genesis_validators_root) + spec.process_light_client_update(store, update, signature_slot, state.genesis_validators_root) assert store.current_max_active_participants > 0 assert store.optimistic_header == update.attested_header diff --git a/tests/core/pyspec/eth2spec/test/helpers/light_client.py b/tests/core/pyspec/eth2spec/test/helpers/light_client.py index 15c764fc4..b35c6c687 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/light_client.py +++ b/tests/core/pyspec/eth2spec/test/helpers/light_client.py @@ -1,5 +1,9 @@ +from eth2spec.test.helpers.state import ( + transition_to, +) from eth2spec.test.helpers.sync_committee import ( compute_aggregate_sync_committee_signature, + compute_committee_indices, ) @@ -15,21 +19,31 @@ def initialize_light_client_store(spec, state): ) -def get_sync_aggregate(spec, state, block_header, block_root=None, signature_slot=None): +def get_sync_aggregate(spec, state, block_header, signature_slot=None): + # By default, the sync committee signs the previous slot if signature_slot is None: - signature_slot = block_header.slot + signature_slot = block_header.slot + 1 - 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) + # Ensure correct sync committee and fork version are selected + signature_state = state.copy() + transition_to(spec, signature_state, signature_slot) + + # Fetch sync committee + committee_indices = compute_committee_indices(spec, signature_state) + committee_size = len(committee_indices) + + # Compute sync aggregate + sync_committee_bits = [True] * committee_size sync_committee_signature = compute_aggregate_sync_committee_signature( spec, - state, - block_header.slot, - committee, - block_root=block_root, + signature_state, + signature_slot, + committee_indices, + block_root=spec.Root(block_header.hash_tree_root()), ) - return spec.SyncAggregate( + sync_aggregate = spec.SyncAggregate( sync_committee_bits=sync_committee_bits, sync_committee_signature=sync_committee_signature, ) + fork_version = signature_state.fork.current_version + return sync_aggregate, fork_version, signature_slot