Align aggregated att gossip validations

beacon-chain/sync/validate_aggregate_proof.go

@@ -152,10 +152,11 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe
 	}
 
 	// Verify validator index is within the beacon committee.
-	if err := validateIndexInCommittee(ctx, bs, signed.Message.Aggregate, signed.Message.AggregatorIndex); err != nil {
+	result, err := s.validateIndexInCommittee(ctx, bs, signed.Message.Aggregate, signed.Message.AggregatorIndex)
+	if result != pubsub.ValidationAccept {
 		wrappedErr := errors.Wrapf(err, "Could not validate index in committee")
 		tracing.AnnotateError(span, wrappedErr)
-		return pubsub.ValidationReject, wrappedErr
+		return result, wrappedErr
 	}
 
 	// Verify selection proof reflects to the right validator.
@@ -216,15 +217,32 @@ func (s *Service) setAggregatorIndexEpochSeen(epoch primitives.Epoch, aggregator
 	s.seenAggregatedAttestationCache.Add(string(b), true)
 }
 
-// This validates the aggregator's index in state is within the beacon committee.
-func validateIndexInCommittee(ctx context.Context, bs state.ReadOnlyBeaconState, a *ethpb.Attestation, validatorIndex primitives.ValidatorIndex) error {
+// This validates the bitfield is correct and aggregator's index in state is within the beacon committee.
+// It implements the following checks from the consensus spec:
+//   - [REJECT] The committee index is within the expected range -- i.e. `aggregate.data.index < get_committee_count_per_slot(state, aggregate.data.target.epoch)`.
+//   - [REJECT] The number of aggregation bits matches the committee size --
+//     i.e. len(aggregate.aggregation_bits) == len(get_beacon_committee(state, aggregate.data.slot, aggregate.data.index)).
+//   - [REJECT] The aggregate attestation has participants -- that is, len(get_attesting_indices(state, aggregate.data, aggregate.aggregation_bits)) >= 1.
+//   - [REJECT] The aggregator's validator index is within the committee --
+//     i.e. `aggregate_and_proof.aggregator_index in get_beacon_committee(state, aggregate.data.slot, aggregate.data.index)`.
+func (s *Service) validateIndexInCommittee(ctx context.Context, bs state.ReadOnlyBeaconState, a *ethpb.Attestation, validatorIndex primitives.ValidatorIndex) (pubsub.ValidationResult, error) {
 	ctx, span := trace.StartSpan(ctx, "sync.validateIndexInCommittee")
 	defer span.End()
 
-	committee, err := helpers.BeaconCommitteeFromState(ctx, bs, a.Data.Slot, a.Data.CommitteeIndex)
-	if err != nil {
-		return err
+	_, result, err := s.validateCommitteeIndex(ctx, a, bs)
+	if result != pubsub.ValidationAccept {
+		return result, err
+	}
+
+	committee, result, err := s.validateBitLength(ctx, a, bs)
+	if result != pubsub.ValidationAccept {
+		return result, err
 	}
+
+	if a.AggregationBits.Count() == 0 {
+		return pubsub.ValidationReject, errors.New("no attesting indices")
+	}
+
 	var withinCommittee bool
 	for _, i := range committee {
 		if validatorIndex == i {
@@ -233,10 +251,10 @@ func validateIndexInCommittee(ctx context.Context, bs state.ReadOnlyBeaconState,
 		}
 	}
 	if !withinCommittee {
-		return fmt.Errorf("validator index %d is not within the committee: %v",
+		return pubsub.ValidationReject, fmt.Errorf("validator index %d is not within the committee: %v",
 			validatorIndex, committee)
 	}
-	return nil
+	return pubsub.ValidationAccept, nil
 }
 
 // This validates selection proof by validating it's from the correct validator index of the slot.

beacon-chain/sync/validate_aggregate_proof_test.go

@@ -37,6 +37,7 @@ func TestVerifyIndexInCommittee_CanVerify(t *testing.T) {
 	params.SetupTestConfigCleanup(t)
 	params.OverrideBeaconConfig(params.MinimalSpecConfig())
 
+	service := &Service{}
 	validators := uint64(32)
 	s, _ := util.DeterministicGenesisState(t, validators)
 	require.NoError(t, s.SetSlot(params.BeaconConfig().SlotsPerEpoch))
@@ -51,10 +52,14 @@ func TestVerifyIndexInCommittee_CanVerify(t *testing.T) {
 	assert.NoError(t, err)
 	indices, err := attestation.AttestingIndices(att.AggregationBits, committee)
 	require.NoError(t, err)
-	require.NoError(t, validateIndexInCommittee(ctx, s, att, primitives.ValidatorIndex(indices[0])))
+	result, err := service.validateIndexInCommittee(ctx, s, att, primitives.ValidatorIndex(indices[0]))
+	require.NoError(t, err)
+	assert.Equal(t, pubsub.ValidationAccept, result)
 
 	wanted := "validator index 1000 is not within the committee"
-	assert.ErrorContains(t, wanted, validateIndexInCommittee(ctx, s, att, 1000))
+	result, err = service.validateIndexInCommittee(ctx, s, att, 1000)
+	assert.ErrorContains(t, wanted, err)
+	assert.Equal(t, pubsub.ValidationReject, result)
 }
 
 func TestVerifyIndexInCommittee_ExistsInBeaconCommittee(t *testing.T) {
@@ -66,18 +71,40 @@ func TestVerifyIndexInCommittee_ExistsInBeaconCommittee(t *testing.T) {
 	s, _ := util.DeterministicGenesisState(t, validators)
 	require.NoError(t, s.SetSlot(params.BeaconConfig().SlotsPerEpoch))
 
-	bf := []byte{0xff}
 	att := &ethpb.Attestation{Data: &ethpb.AttestationData{
-		Target: &ethpb.Checkpoint{Epoch: 0}},
-		AggregationBits: bf}
+		Target: &ethpb.Checkpoint{Epoch: 0}}}
 
 	committee, err := helpers.BeaconCommitteeFromState(context.Background(), s, att.Data.Slot, att.Data.CommitteeIndex)
 	require.NoError(t, err)
 
-	require.NoError(t, validateIndexInCommittee(ctx, s, att, committee[0]))
+	bl := bitfield.NewBitlist(uint64(len(committee)))
+	att.AggregationBits = bl
+
+	service := &Service{}
+	result, err := service.validateIndexInCommittee(ctx, s, att, committee[0])
+	require.ErrorContains(t, "no attesting indices", err)
+	assert.Equal(t, pubsub.ValidationReject, result)
+
+	att.AggregationBits.SetBitAt(0, true)
+
+	result, err = service.validateIndexInCommittee(ctx, s, att, committee[0])
+	require.NoError(t, err)
+	assert.Equal(t, pubsub.ValidationAccept, result)
 
 	wanted := "validator index 1000 is not within the committee"
-	assert.ErrorContains(t, wanted, validateIndexInCommittee(ctx, s, att, 1000))
+	result, err = service.validateIndexInCommittee(ctx, s, att, 1000)
+	assert.ErrorContains(t, wanted, err)
+	assert.Equal(t, pubsub.ValidationReject, result)
+
+	att.AggregationBits = bitfield.NewBitlist(1)
+	result, err = service.validateIndexInCommittee(ctx, s, att, committee[0])
+	require.ErrorContains(t, "wanted participants bitfield length 4, got: 1", err)
+	assert.Equal(t, pubsub.ValidationReject, result)
+
+	att.Data.CommitteeIndex = 10000
+	result, err = service.validateIndexInCommittee(ctx, s, att, committee[0])
+	require.ErrorContains(t, "committee index 10000 > 2", err)
+	assert.Equal(t, pubsub.ValidationReject, result)
 }
 
 func TestVerifySelection_NotAnAggregator(t *testing.T) {

beacon-chain/sync/validate_beacon_attestation.go

@@ -175,14 +175,9 @@ func (s *Service) validateUnaggregatedAttTopic(ctx context.Context, a *eth.Attes
 	ctx, span := trace.StartSpan(ctx, "sync.validateUnaggregatedAttTopic")
 	defer span.End()
 
-	valCount, err := helpers.ActiveValidatorCount(ctx, bs, slots.ToEpoch(a.Data.Slot))
-	if err != nil {
-		tracing.AnnotateError(span, err)
-		return pubsub.ValidationIgnore, err
-	}
-	count := helpers.SlotCommitteeCount(valCount)
-	if uint64(a.Data.CommitteeIndex) > count {
-		return pubsub.ValidationReject, errors.Errorf("committee index %d > %d", a.Data.CommitteeIndex, count)
+	valCount, result, err := s.validateCommitteeIndex(ctx, a, bs)
+	if result != pubsub.ValidationAccept {
+		return result, err
 	}
 	subnet := helpers.ComputeSubnetForAttestation(valCount, a)
 	format := p2p.GossipTypeMapping[reflect.TypeOf(&eth.Attestation{})]
@@ -198,21 +193,27 @@ func (s *Service) validateUnaggregatedAttTopic(ctx context.Context, a *eth.Attes
 	return pubsub.ValidationAccept, nil
 }
 
+func (s *Service) validateCommitteeIndex(ctx context.Context, a *eth.Attestation, bs state.ReadOnlyBeaconState) (uint64, pubsub.ValidationResult, error) {
+	valCount, err := helpers.ActiveValidatorCount(ctx, bs, slots.ToEpoch(a.Data.Slot))
+	if err != nil {
+		return 0, pubsub.ValidationIgnore, err
+	}
+	count := helpers.SlotCommitteeCount(valCount)
+	if uint64(a.Data.CommitteeIndex) > count {
+		return 0, pubsub.ValidationReject, errors.Errorf("committee index %d > %d", a.Data.CommitteeIndex, count)
+	}
+	return valCount, pubsub.ValidationAccept, nil
+}
+
 // This validates beacon unaggregated attestation using the given state, the validation consists of bitfield length and count consistency
 // and signature verification.
 func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.Attestation, bs state.ReadOnlyBeaconState) (pubsub.ValidationResult, error) {
 	ctx, span := trace.StartSpan(ctx, "sync.validateUnaggregatedAttWithState")
 	defer span.End()
 
-	committee, err := helpers.BeaconCommitteeFromState(ctx, bs, a.Data.Slot, a.Data.CommitteeIndex)
-	if err != nil {
-		tracing.AnnotateError(span, err)
-		return pubsub.ValidationIgnore, err
-	}
-
-	// Verify number of aggregation bits matches the committee size.
-	if err := helpers.VerifyBitfieldLength(a.AggregationBits, uint64(len(committee))); err != nil {
-		return pubsub.ValidationReject, err
+	committee, result, err := s.validateBitLength(ctx, a, bs)
+	if result != pubsub.ValidationAccept {
+		return result, err
 	}
 
 	// Attestation must be unaggregated and the bit index must exist in the range of committee indices.
@@ -231,6 +232,20 @@ func (s *Service) validateUnaggregatedAttWithState(ctx context.Context, a *eth.A
 	return s.validateWithBatchVerifier(ctx, "attestation", set)
 }
 
+func (s *Service) validateBitLength(ctx context.Context, a *eth.Attestation, bs state.ReadOnlyBeaconState) ([]primitives.ValidatorIndex, pubsub.ValidationResult, error) {
+	committee, err := helpers.BeaconCommitteeFromState(ctx, bs, a.Data.Slot, a.Data.CommitteeIndex)
+	if err != nil {
+		return nil, pubsub.ValidationIgnore, err
+	}
+
+	// Verify number of aggregation bits matches the committee size.
+	if err := helpers.VerifyBitfieldLength(a.AggregationBits, uint64(len(committee))); err != nil {
+		return nil, pubsub.ValidationReject, err
+	}
+
+	return committee, pubsub.ValidationAccept, nil
+}
+
 // Returns true if the attestation was already seen for the participating validator for the slot.
 func (s *Service) hasSeenCommitteeIndicesSlot(slot primitives.Slot, committeeID primitives.CommitteeIndex, aggregateBits []byte) bool {
 	s.seenUnAggregatedAttestationLock.RLock()