Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reorder gossip validation checks #5610

Merged
merged 2 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion beacon_chain/consensus_object_pools/spec_cache.nim
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,19 @@ func get_beacon_committee_len*(
)

# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/specs/phase0/beacon-chain.md#get_attesting_indices
func compatible_with_shuffling*(
bits: CommitteeValidatorsBits,
shufflingRef: ShufflingRef,
slot: Slot,
committee_index: CommitteeIndex): bool =
bits.lenu64 == get_beacon_committee_len(shufflingRef, slot, committee_index)

iterator get_attesting_indices*(shufflingRef: ShufflingRef,
slot: Slot,
committee_index: CommitteeIndex,
bits: CommitteeValidatorsBits):
ValidatorIndex =
if bits.lenu64 != get_beacon_committee_len(shufflingRef, slot, committee_index):
if not bits.compatible_with_shuffling(shufflingRef, slot, committee_index):
trace "get_attesting_indices: inconsistent aggregation and committee length"
else:
for index_in_committee, validator_index in get_beacon_committee(
Expand Down
24 changes: 14 additions & 10 deletions beacon_chain/gossip_processing/gossip_validation.nim
Original file line number Diff line number Diff line change
Expand Up @@ -724,8 +724,8 @@ proc validateAttestation*(
# This uses the same epochRef as data.target.epoch, because the attestation's
# epoch matches its target and attestation.data.target.root is an ancestor of
# attestation.data.beacon_block_root.
if not (attestation.aggregation_bits.lenu64 == get_beacon_committee_len(
shufflingRef, attestation.data.slot, committee_index)):
if not attestation.aggregation_bits.compatible_with_shuffling(
shufflingRef, slot, committee_index):
return pool.checkedReject(
"Attestation: number of aggregation bits and committee size mismatch")

Expand Down Expand Up @@ -872,14 +872,6 @@ proc validateAggregate*(
return pool.checkedResult(v.error)
v.get()

if checkCover and
pool[].covers(aggregate.data, aggregate.aggregation_bits):
# [IGNORE] A valid aggregate attestation defined by
# `hash_tree_root(aggregate.data)` whose `aggregation_bits` is a non-strict
# superset has _not_ already been seen.
# https://github.com/ethereum/consensus-specs/pull/2847
return errIgnore("Aggregate already covered")

let
shufflingRef =
pool.dag.getShufflingRef(target.blck, target.slot.epoch, false).valueOr:
Expand All @@ -896,6 +888,18 @@ proc validateAggregate*(
return pool.checkedReject(
"Attestation: committee index not within expected range")
idx.get()
if not aggregate.aggregation_bits.compatible_with_shuffling(
shufflingRef, slot, committee_index):
return pool.checkedReject(
"Aggregate: number of aggregation bits and committee size mismatch")

if checkCover and
pool[].covers(aggregate.data, aggregate.aggregation_bits):
# [IGNORE] A valid aggregate attestation defined by
# `hash_tree_root(aggregate.data)` whose `aggregation_bits` is a non-strict
# superset has _not_ already been seen.
# https://github.com/ethereum/consensus-specs/pull/2847
return errIgnore("Aggregate already covered")

# [REJECT] aggregate_and_proof.selection_proof selects the validator as an
# aggregator for the slot -- i.e. is_aggregator(state, aggregate.data.slot,
Expand Down
Loading