Skip to content

Commit

Permalink
Check correct fork version in LC sync protocol
Browse files Browse the repository at this point in the history
- Sync committee is determined by signature_slot
- Signature fork version is determined by max(signature_slot, 1) - 1
- Attested block fork version can be anything < signature_slot

Old logic incorrectly derived signature fork version from signature_slot
and did not subtract a slot. Extended tests to check this edge case.
  • Loading branch information
etan-status committed Mar 8, 2023
1 parent ccfe576 commit 43e714e
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 12 deletions.
3 changes: 2 additions & 1 deletion specs/altair/light-client/sync-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ def validate_light_client_update(store: LightClientStore,
pubkey for (bit, pubkey) in zip(sync_aggregate.sync_committee_bits, sync_committee.pubkeys)
if bit
]
fork_version = compute_fork_version(compute_epoch_at_slot(update.signature_slot))
fork_version_slot = max(update.signature_slot, 1) - 1
fork_version = compute_fork_version(compute_epoch_at_slot(fork_version_slot))
domain = compute_domain(DOMAIN_SYNC_COMMITTEE, fork_version, genesis_validators_root)
signing_root = compute_signing_root(update.attested_header.beacon, domain)
assert bls.FastAggregateVerify(participant_pubkeys, signing_root, sync_aggregate.sync_committee_signature)
Expand Down
13 changes: 5 additions & 8 deletions tests/core/pyspec/eth2spec/test/altair/light_client/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -668,10 +668,9 @@ def run_test_single_fork(spec, phases, state, fork):
# Upgrade to post-fork spec, attested block is still before the fork
attested_block = block.copy()
attested_state = state.copy()
state, _ = do_fork(state, spec, phases[fork], fork_epoch, with_block=False)
sync_aggregate, _ = get_sync_aggregate(phases[fork], state)
state, block = do_fork(state, spec, phases[fork], fork_epoch, sync_aggregate=sync_aggregate)
spec = phases[fork]
sync_aggregate, _ = get_sync_aggregate(spec, state)
block = state_transition_with_full_block(spec, state, True, True, sync_aggregate=sync_aggregate)
yield from emit_update(test, spec, state, block, attested_state, attested_block, finalized_block, phases=phases)
assert test.store.finalized_header.beacon.slot == finalized_state.slot
assert test.store.next_sync_committee == finalized_state.next_sync_committee
Expand Down Expand Up @@ -755,18 +754,16 @@ def run_test_multi_fork(spec, phases, state, fork_1, fork_2):
# ..., attested is from `fork_1`, ...
fork_1_epoch = getattr(phases[fork_1].config, fork_1.upper() + '_FORK_EPOCH')
transition_to(spec, state, spec.compute_start_slot_at_epoch(fork_1_epoch) - 1)
state, _ = do_fork(state, spec, phases[fork_1], fork_1_epoch, with_block=False)
state, attested_block = do_fork(state, spec, phases[fork_1], fork_1_epoch)
spec = phases[fork_1]
attested_block = state_transition_with_full_block(spec, state, True, True)
attested_state = state.copy()

# ..., and signature is from `fork_2`
fork_2_epoch = getattr(phases[fork_2].config, fork_2.upper() + '_FORK_EPOCH')
transition_to(spec, state, spec.compute_start_slot_at_epoch(fork_2_epoch) - 1)
state, _ = do_fork(state, spec, phases[fork_2], fork_2_epoch, with_block=False)
sync_aggregate, _ = get_sync_aggregate(phases[fork_2], state)
state, block = do_fork(state, spec, phases[fork_2], fork_2_epoch, sync_aggregate=sync_aggregate)
spec = phases[fork_2]
sync_aggregate, _ = get_sync_aggregate(spec, state)
block = state_transition_with_full_block(spec, state, True, True, sync_aggregate=sync_aggregate)

# Check that update applies
yield from emit_update(test, spec, state, block, attested_state, attested_block, finalized_block, phases=phases)
Expand Down
12 changes: 10 additions & 2 deletions tests/core/pyspec/eth2spec/test/helpers/fork_transition.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def _set_operations_by_dict(block, operation_dict):

def _state_transition_and_sign_block_at_slot(spec,
state,
sync_aggregate=None,
operation_dict=None):
"""
Cribbed from ``transition_unsigned_block`` helper
Expand All @@ -61,6 +62,8 @@ def _state_transition_and_sign_block_at_slot(spec,
Thus use dict to pass operations.
"""
block = build_empty_block(spec, state)
if sync_aggregate is not None:
block.body.sync_aggregate = sync_aggregate

if operation_dict:
_set_operations_by_dict(block, operation_dict)
Expand Down Expand Up @@ -141,7 +144,7 @@ def state_transition_across_slots_with_ignoring_proposers(spec,
next_slot(spec, state)


def do_fork(state, spec, post_spec, fork_epoch, with_block=True, operation_dict=None):
def do_fork(state, spec, post_spec, fork_epoch, with_block=True, sync_aggregate=None, operation_dict=None):
spec.process_slots(state, state.slot + 1)

assert state.slot % spec.SLOTS_PER_EPOCH == 0
Expand Down Expand Up @@ -172,7 +175,12 @@ def do_fork(state, spec, post_spec, fork_epoch, with_block=True, operation_dict=
assert state.fork.current_version == post_spec.config.DENEB_FORK_VERSION

if with_block:
return state, _state_transition_and_sign_block_at_slot(post_spec, state, operation_dict=operation_dict)
return state, _state_transition_and_sign_block_at_slot(
post_spec,
state,
sync_aggregate=sync_aggregate,
operation_dict=operation_dict,
)
else:
return state, None

Expand Down
2 changes: 1 addition & 1 deletion tests/core/pyspec/eth2spec/test/helpers/light_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def get_sync_aggregate(spec, state, num_participants=None, signature_slot=None):
sync_committee_signature = compute_aggregate_sync_committee_signature(
spec,
signature_state,
signature_slot,
max(signature_slot, 1) - 1,
committee_indices[:num_participants],
)
sync_aggregate = spec.SyncAggregate(
Expand Down

0 comments on commit 43e714e

Please sign in to comment.