Address review comments

This commit is contained in:
Zahary Karadjov 2021-09-13 19:47:39 +03:00 committed by zah
parent 3e4faaa213
commit a71de3feda
6 changed files with 28 additions and 22 deletions

View File

@ -525,5 +525,5 @@ func shortLog*(v: SyncAggregate): auto =
chronicles.formatIt SyncCommitteeMessage: shortLog(it)
func hash*(x: LightClientUpdate): Hash =
hash(x.header.state_root.data)
template hash*(x: LightClientUpdate): Hash =
hash(x.header)

View File

@ -921,6 +921,9 @@ proc load*(
else:
some(validators[index.int].pubkey.loadValid())
template hash*(header: BeaconBlockHeader): Hash =
hash(header.state_root)
static:
# Sanity checks - these types should be trivial enough to copy with memcpy
doAssert supportsCopyMem(Validator)

View File

@ -236,4 +236,5 @@ func has_flag*(flags: ParticipationFlags, flag_index: int): bool =
# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-beta.3/specs/altair/sync-protocol.md#get_subtree_index
func get_subtree_index*(idx: GeneralizedIndex): uint64 =
doAssert idx > 0
uint64(idx mod (type(idx)(1) shl log2trunc(idx)))

View File

@ -1,16 +1,9 @@
import
std/sets,
stew/bitops2,
stew/[bitops2, objects],
datatypes/altair,
helpers
func branchIsAllZeros(branch: openarray[Eth2Digest]): bool =
for node in branch:
if node != Eth2Digest():
return false
return true
# https://github.com/ethereum/consensus-specs/blob/v1.1.0-beta.3/specs/altair/sync-protocol.md#validate_light_client_update
proc validate_light_client_update*(snapshot: LightClientSnapshot,
update: LightClientUpdate,
@ -27,8 +20,8 @@ proc validate_light_client_update*(snapshot: LightClientSnapshot,
# Verify update header root is the finalized root of the finality header, if specified
# TODO: Use a view type instead of `unsafeAddr`
let signed_header = if update.finality_header == BeaconBlockHeader():
if not branchIsAllZeros(update.finality_branch):
let signed_header = if update.finality_header.isZeroMemory:
if not update.finality_branch.isZeroMemory:
return false
unsafeAddr update.header
else:
@ -43,7 +36,7 @@ proc validate_light_client_update*(snapshot: LightClientSnapshot,
# Verify update next sync committee if the update period incremented
# TODO: Use a view type instead of `unsafeAddr`
let sync_committee = if update_period == snapshot_period:
if not branchIsAllZeros(update.next_sync_committee_branch):
if not update.next_sync_committee_branch.isZeroMemory:
return false
unsafeAddr snapshot.current_sync_committee
else:
@ -55,13 +48,14 @@ proc validate_light_client_update*(snapshot: LightClientSnapshot,
return false
unsafeAddr snapshot.next_sync_committee
let sync_committee_participants_count = countOnes(update.sync_committee_bits)
# Verify sync committee has sufficient participants
if countOnes(update.sync_committee_bits) < MIN_SYNC_COMMITTEE_PARTICIPANTS:
if sync_committee_participants_count < MIN_SYNC_COMMITTEE_PARTICIPANTS:
return false
# Verify sync committee aggregate signature
# participant_pubkeys = [pubkey for (bit, pubkey) in zip(update.sync_committee_bits, sync_committee.pubkeys) if bit]
var participant_pubkeys: seq[ValidatorPubKey]
var participant_pubkeys = newSeqOfCap[ValidatorPubKey](sync_committee_participants_count)
for idx, bit in update.sync_committee_bits:
if bit:
participant_pubkeys.add(sync_committee.pubkeys[idx])
@ -71,7 +65,7 @@ proc validate_light_client_update*(snapshot: LightClientSnapshot,
blsFastAggregateVerify(participant_pubkeys, signing_root.data, update.sync_committee_signature)
# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-beta.3/specs/altair/sync-protocol.md#apply_light_client_update
# https://github.com/ethereum/consensus-specs/blob/v1.1.0-beta.3/specs/altair/sync-protocol.md#apply_light_client_update
proc apply_light_client_update(snapshot: var LightClientSnapshot, update: LightClientUpdate) =
let snapshot_period = compute_epoch_at_slot(snapshot.header.slot) div EPOCHS_PER_SYNC_COMMITTEE_PERIOD
let update_period = compute_epoch_at_slot(update.header.slot) div EPOCHS_PER_SYNC_COMMITTEE_PERIOD
@ -80,7 +74,7 @@ proc apply_light_client_update(snapshot: var LightClientSnapshot, update: LightC
snapshot.next_sync_committee = update.next_sync_committee
snapshot.header = update.header
# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-beta.3/specs/altair/sync-protocol.md#process_light_client_update
# https://github.com/ethereum/consensus-specs/blob/v1.1.0-beta.3/specs/altair/sync-protocol.md#process_light_client_update
proc process_light_client_update(store: var LightClientStore,
update: LightClientUpdate,
current_slot: Slot,
@ -92,7 +86,7 @@ proc process_light_client_update(store: var LightClientStore,
var update_timeout = SLOTS_PER_EPOCH * EPOCHS_PER_SYNC_COMMITTEE_PERIOD
let sync_committee_participants_count = countOnes(update.sync_committee_bits)
if sync_committee_participants_count * 3 >= update.sync_committee_bits.len * 2 and
update.finality_header != BeaconBlockHeader():
not update.finality_header.isZeroMemory:
# Apply update if (1) 2/3 quorum is reached and (2) we have a finality proof.
# Note that (2) means that the current light client design needs finality.
# It may be changed to re-organizable light client design. See the on-going issue eth2.0-specs#2182.
@ -100,6 +94,12 @@ proc process_light_client_update(store: var LightClientStore,
store.valid_updates.clear()
elif current_slot > store.snapshot.header.slot + update_timeout:
var best_update_participants = 0
# TODO:
# Use a view type to avoid the copying when a new best update
# is discovered.
# Alterantively, we could have a `set.max` operation returning
# the best item as a `lent` value. We would need an `OrderedSet`
# type with support for a custom comparison operation.
var best_update: LightClientUpdate
for update in store.valid_updates:
let update_participants = countOnes(update.sync_committee_bits)
@ -110,4 +110,4 @@ proc process_light_client_update(store: var LightClientStore,
# Forced best update when the update timeout has elapsed
apply_light_client_update(store.snapshot, best_update)
store.valid_updates.clear()
return true
true

View File

@ -32,6 +32,8 @@ func binaryTreeHeight*(totalElements: Limit): int =
bitWidth nextPow2(uint64 totalElements)
type
# TODO Figure out what would be the right type for this.
# It probably fits in uint16 for all practical purposes.
GeneralizedIndex* = uint32
SszMerkleizerImpl = object

2
vendor/nim-stew vendored

@ -1 +1 @@
Subproject commit 3c91b8694e15137a81ec7db37c6c58194ec94a6a
Subproject commit 478cc6efdefaabadf0666a3351fb959b78009bcc