Add in Attestation Validity Check

beacon-chain/core/blocks/attestation.go

@@ -199,60 +199,6 @@ func ProcessAttestationNoVerify(
 	return beaconState, nil
 }
 
-// VerifyIndexedAttestation determines the validity of an indexed attestation.
-//
-// Spec pseudocode definition:
-//  def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: IndexedAttestation) -> bool:
-//    """
-//    Check if ``indexed_attestation`` is not empty, has sorted and unique indices and has a valid aggregate signature.
-//    """
-//    # Verify indices are sorted and unique
-//    indices = indexed_attestation.attesting_indices
-//    if len(indices) == 0 or not indices == sorted(set(indices)):
-//        return False
-//    # Verify aggregate signature
-//    pubkeys = [state.validators[i].pubkey for i in indices]
-//    domain = get_domain(state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)
-//    signing_root = compute_signing_root(indexed_attestation.data, domain)
-//    return bls.FastAggregateVerify(pubkeys, signing_root, indexed_attestation.signature)
-func VerifyIndexedAttestation(ctx context.Context, beaconState *stateTrie.BeaconState, indexedAtt *ethpb.IndexedAttestation) error {
-	ctx, span := trace.StartSpan(ctx, "core.VerifyIndexedAttestation")
-	defer span.End()
-
-	if err := attestationutil.IsValidAttestationIndices(ctx, indexedAtt); err != nil {
-		return err
-	}
-	domain, err := helpers.Domain(beaconState.Fork(), indexedAtt.Data.Target.Epoch, params.BeaconConfig().DomainBeaconAttester, beaconState.GenesisValidatorRoot())
-	if err != nil {
-		return err
-	}
-	indices := indexedAtt.AttestingIndices
-	pubkeys := []bls.PublicKey{}
-	for i := 0; i < len(indices); i++ {
-		pubkeyAtIdx := beaconState.PubkeyAtIndex(indices[i])
-		pk, err := bls.PublicKeyFromBytes(pubkeyAtIdx[:])
-		if err != nil {
-			return errors.Wrap(err, "could not deserialize validator public key")
-		}
-		pubkeys = append(pubkeys, pk)
-	}
-	return attestationutil.VerifyIndexedAttestationSig(ctx, indexedAtt, pubkeys, domain)
-}
-
-// VerifyAttestation converts and attestation into an indexed attestation and verifies
-// the signature in that attestation.
-func VerifyAttestation(ctx context.Context, beaconState *stateTrie.BeaconState, att *ethpb.Attestation) error {
-	if att == nil || att.Data == nil {
-		return fmt.Errorf("nil or missing attestation data: %v", att)
-	}
-	committee, err := helpers.BeaconCommitteeFromState(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
-	if err != nil {
-		return err
-	}
-	indexedAtt := attestationutil.ConvertToIndexed(ctx, att, committee)
-	return VerifyIndexedAttestation(ctx, beaconState, indexedAtt)
-}
-
 // VerifyAttestations will verify the signatures of the provided attestations. This method performs
 // a single BLS verification call to verify the signatures of all of the provided attestations. All
 // of the provided attestations must have valid signatures or this method will return an error.
@@ -306,6 +252,60 @@ func VerifyAttestations(ctx context.Context, beaconState *stateTrie.BeaconState,
 	return verifyAttestationsWithDomain(ctx, beaconState, postForkAtts, currDomain)
 }
 
+// VerifyAttestation converts and attestation into an indexed attestation and verifies
+// the signature in that attestation.
+func VerifyAttestation(ctx context.Context, beaconState *stateTrie.BeaconState, att *ethpb.Attestation) error {
+	if att == nil || att.Data == nil || att.AggregationBits.Count() == 0 {
+		return fmt.Errorf("nil or missing attestation data: %v", att)
+	}
+	committee, err := helpers.BeaconCommitteeFromState(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		return err
+	}
+	indexedAtt := attestationutil.ConvertToIndexed(ctx, att, committee)
+	return VerifyIndexedAttestation(ctx, beaconState, indexedAtt)
+}
+
+// VerifyIndexedAttestation determines the validity of an indexed attestation.
+//
+// Spec pseudocode definition:
+//  def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: IndexedAttestation) -> bool:
+//    """
+//    Check if ``indexed_attestation`` is not empty, has sorted and unique indices and has a valid aggregate signature.
+//    """
+//    # Verify indices are sorted and unique
+//    indices = indexed_attestation.attesting_indices
+//    if len(indices) == 0 or not indices == sorted(set(indices)):
+//        return False
+//    # Verify aggregate signature
+//    pubkeys = [state.validators[i].pubkey for i in indices]
+//    domain = get_domain(state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)
+//    signing_root = compute_signing_root(indexed_attestation.data, domain)
+//    return bls.FastAggregateVerify(pubkeys, signing_root, indexed_attestation.signature)
+func VerifyIndexedAttestation(ctx context.Context, beaconState *stateTrie.BeaconState, indexedAtt *ethpb.IndexedAttestation) error {
+	ctx, span := trace.StartSpan(ctx, "core.VerifyIndexedAttestation")
+	defer span.End()
+
+	if err := attestationutil.IsValidAttestationIndices(ctx, indexedAtt); err != nil {
+		return err
+	}
+	domain, err := helpers.Domain(beaconState.Fork(), indexedAtt.Data.Target.Epoch, params.BeaconConfig().DomainBeaconAttester, beaconState.GenesisValidatorRoot())
+	if err != nil {
+		return err
+	}
+	indices := indexedAtt.AttestingIndices
+	pubkeys := []bls.PublicKey{}
+	for i := 0; i < len(indices); i++ {
+		pubkeyAtIdx := beaconState.PubkeyAtIndex(indices[i])
+		pk, err := bls.PublicKeyFromBytes(pubkeyAtIdx[:])
+		if err != nil {
+			return errors.Wrap(err, "could not deserialize validator public key")
+		}
+		pubkeys = append(pubkeys, pk)
+	}
+	return attestationutil.VerifyIndexedAttestationSig(ctx, indexedAtt, pubkeys, domain)
+}
+
 // Inner method to verify attestations. This abstraction allows for the domain to be provided as an
 // argument.
 func verifyAttestationsWithDomain(ctx context.Context, beaconState *stateTrie.BeaconState, atts []*ethpb.Attestation, domain []byte) error {

beacon-chain/core/blocks/attestation_test.go

@@ -603,6 +603,48 @@ func TestValidateIndexedAttestation_AboveMaxLength(t *testing.T) {
 	assert.ErrorContains(t, want, err)
 }
 
+func TestValidateIndexedAttestation_BadAttestationsSignatureSet(t *testing.T) {
+	beaconState, keys := testutil.DeterministicGenesisState(t, 1000)
+
+	sig := keys[0].Sign([]byte{'t', 'e', 's', 't'})
+	list := bitfield.Bitlist{0b11111111}
+	atts := []*ethpb.Attestation{}
+	for i := uint64(0); i < 1000; i++ {
+		atts = append(atts, &ethpb.Attestation{
+			Data: &ethpb.AttestationData{
+				CommitteeIndex: 1,
+				Slot:           1,
+			},
+			Signature:       sig.Marshal(),
+			AggregationBits: list,
+		})
+	}
+
+	want := "nil or missing indexed attestation data"
+	_, err := blocks.AttestationSignatureSet(context.Background(), beaconState, atts)
+	assert.ErrorContains(t, want, err)
+
+	atts = []*ethpb.Attestation{}
+	list = bitfield.Bitlist{0b00000000}
+	for i := uint64(0); i < 1000; i++ {
+		atts = append(atts, &ethpb.Attestation{
+			Data: &ethpb.AttestationData{
+				CommitteeIndex: 1,
+				Slot:           1,
+				Target: &ethpb.Checkpoint{
+					Root: []byte{},
+				},
+			},
+			Signature:       sig.Marshal(),
+			AggregationBits: list,
+		})
+	}
+
+	want = "expected non-empty attesting indices"
+	_, err = blocks.AttestationSignatureSet(context.Background(), beaconState, atts)
+	assert.ErrorContains(t, want, err)
+}
+
 func TestVerifyAttestations_VerifiesMultipleAttestations(t *testing.T) {
 	ctx := context.Background()
 	numOfValidators := 4 * params.BeaconConfig().SlotsPerEpoch
@@ -634,6 +676,7 @@ func TestVerifyAttestations_VerifiesMultipleAttestations(t *testing.T) {
 		Data: &ethpb.AttestationData{
 			Slot:           1,
 			CommitteeIndex: 0,
+			Target:         new(ethpb.Checkpoint),
 		},
 		Signature: nil,
 	}
@@ -655,6 +698,7 @@ func TestVerifyAttestations_VerifiesMultipleAttestations(t *testing.T) {
 		Data: &ethpb.AttestationData{
 			Slot:           1,
 			CommitteeIndex: 1,
+			Target:         new(ethpb.Checkpoint),
 		},
 		Signature: nil,
 	}
@@ -702,6 +746,7 @@ func TestVerifyAttestations_HandlesPlannedFork(t *testing.T) {
 		Data: &ethpb.AttestationData{
 			Slot:           1,
 			CommitteeIndex: 0,
+			Target:         new(ethpb.Checkpoint),
 		},
 		Signature: nil,
 	}
@@ -723,6 +768,7 @@ func TestVerifyAttestations_HandlesPlannedFork(t *testing.T) {
 		Data: &ethpb.AttestationData{
 			Slot:           1*params.BeaconConfig().SlotsPerEpoch + 1,
 			CommitteeIndex: 1,
+			Target:         new(ethpb.Checkpoint),
 		},
 		Signature: nil,
 	}
@@ -770,6 +816,7 @@ func TestRetrieveAttestationSignatureSet_VerifiesMultipleAttestations(t *testing
 		Data: &ethpb.AttestationData{
 			Slot:           1,
 			CommitteeIndex: 0,
+			Target:         new(ethpb.Checkpoint),
 		},
 		Signature: nil,
 	}
@@ -791,6 +838,7 @@ func TestRetrieveAttestationSignatureSet_VerifiesMultipleAttestations(t *testing
 		Data: &ethpb.AttestationData{
 			Slot:           1,
 			CommitteeIndex: 1,
+			Target:         new(ethpb.Checkpoint),
 		},
 		Signature: nil,
 	}

beacon-chain/core/blocks/signature.go

@@ -144,6 +144,9 @@ func createAttestationSignatureSet(ctx context.Context, beaconState *stateTrie.B
 			return nil, err
 		}
 		ia := attestationutil.ConvertToIndexed(ctx, a, c)
+		if err := attestationutil.IsValidAttestationIndices(ctx, ia); err != nil {
+			return nil, err
+		}
 		indices := ia.AttestingIndices
 		var pk bls.PublicKey
 		for i := 0; i < len(indices); i++ {