Remove verify unaggregated attestation when aggregating (#4347)

beacon-chain/rpc/aggregator/BUILD.bazel

@@ -7,7 +7,6 @@ go_library(
     visibility = ["//beacon-chain:__subpackages__"],
     deps = [
         "//beacon-chain/blockchain:go_default_library",
-        "//beacon-chain/core/blocks:go_default_library",
         "//beacon-chain/core/helpers:go_default_library",
         "//beacon-chain/db:go_default_library",
         "//beacon-chain/operations/attestations:go_default_library",

beacon-chain/rpc/aggregator/server.go

@@ -5,7 +5,6 @@ import (
 
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/beacon-chain/blockchain"
-	"github.com/prysmaticlabs/prysm/beacon-chain/core/blocks"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/beacon-chain/db"
 	"github.com/prysmaticlabs/prysm/beacon-chain/operations/attestations"
@@ -79,24 +78,8 @@ func (as *Server) SubmitAggregateAndProof(ctx context.Context, req *pb.Aggregati
 	// Retrieve the unaggregated attestation from pool
 	atts := as.AttPool.UnaggregatedAttestationsBySlotIndex(req.Slot, req.CommitteeIndex)
 
-	headState, err := as.HeadFetcher.HeadState(ctx)
-	if err != nil {
-		return nil, err
-	}
-	// Verify attestations are valid before aggregating and broadcasting them out.
-	validAtts := make([]*ethpb.Attestation, 0, len(atts))
-	for _, att := range atts {
-		if err := blocks.VerifyAttestation(ctx, headState, att); err != nil {
-			if err := as.AttPool.DeleteUnaggregatedAttestation(att); err != nil {
-				return nil, status.Errorf(codes.Internal, "Could not delete invalid attestation: %v", err)
-			}
-			continue
-		}
-		validAtts = append(validAtts, att)
-	}
-
 	// Aggregate the attestations and broadcast them.
-	aggregatedAtts, err := helpers.AggregateAttestations(validAtts)
+	aggregatedAtts, err := helpers.AggregateAttestations(atts)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not aggregate attestations: %v", err)
 	}

beacon-chain/rpc/validator/BUILD.bazel

@@ -32,6 +32,7 @@ go_library(
         "//beacon-chain/sync:go_default_library",
         "//proto/beacon/p2p/v1:go_default_library",
         "//proto/beacon/rpc/v1:go_default_library",
+        "//shared/bls:go_default_library",
         "//shared/bytesutil:go_default_library",
         "//shared/featureconfig:go_default_library",
         "//shared/hashutil:go_default_library",

beacon-chain/rpc/validator/attester.go

@@ -9,6 +9,7 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/cache"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/state"
+	"github.com/prysmaticlabs/prysm/shared/bls"
 	"go.opencensus.io/trace"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -98,6 +99,10 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
 // ProposeAttestation is a function called by an attester to vote
 // on a block via an attestation object as defined in the Ethereum Serenity specification.
 func (vs *Server) ProposeAttestation(ctx context.Context, att *ethpb.Attestation) (*ethpb.AttestResponse, error) {
+	if _, err := bls.SignatureFromBytes(att.Signature); err != nil {
+		return nil, status.Error(codes.InvalidArgument, "Incorrect attestation signature")
+	}
+
 	root, err := ssz.HashTreeRoot(att.Data)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not tree hash attestation: %v", err)

beacon-chain/rpc/validator/attester_test.go

@@ -17,6 +17,7 @@ import (
 	mockp2p "github.com/prysmaticlabs/prysm/beacon-chain/p2p/testing"
 	mockSync "github.com/prysmaticlabs/prysm/beacon-chain/sync/initial-sync/testing"
 	pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
+	"github.com/prysmaticlabs/prysm/shared/bls"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/params"
 )
@@ -71,7 +72,10 @@ func TestProposeAttestation_OK(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	sk := bls.RandKey()
+	sig := sk.Sign([]byte("dummy_test_data"), 0 /*domain*/)
 	req := &ethpb.Attestation{
+		Signature: sig.Marshal(),
 		Data: &ethpb.AttestationData{
 			BeaconBlockRoot: root[:],
 			Source:          &ethpb.Checkpoint{},
@@ -83,6 +87,30 @@ func TestProposeAttestation_OK(t *testing.T) {
 	}
 }
 
+func TestProposeAttestation_IncorrectSignature(t *testing.T) {
+	db := dbutil.SetupDB(t)
+	defer dbutil.TeardownDB(t, db)
+
+	attesterServer := &Server{
+		HeadFetcher:      &mock.ChainService{},
+		P2P:              &mockp2p.MockBroadcaster{},
+		BeaconDB:         db,
+		AttestationCache: cache.NewAttestationCache(),
+		AttPool:          attestations.NewPool(),
+	}
+
+	req := &ethpb.Attestation{
+		Data: &ethpb.AttestationData{
+			Source:          &ethpb.Checkpoint{},
+			Target:          &ethpb.Checkpoint{},
+		},
+	}
+	wanted := "Incorrect attestation signature"
+	if _, err := attesterServer.ProposeAttestation(context.Background(), req); !strings.Contains(err.Error(), wanted) {
+		t.Errorf("Did not get wanted error")
+	}
+}
+
 func TestGetAttestationData_OK(t *testing.T) {
 	block := &ethpb.BeaconBlock{
 		Slot: 3*params.BeaconConfig().SlotsPerEpoch + 1,