Rm forkchoice attestation verification (#8301)

* Rm fork choice attestation sig verification

* Unit test

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

beacon-chain/blockchain/process_attestation.go

@@ -39,7 +39,6 @@ var ErrTargetRootNotInDB = errors.New("target root does not exist in db")
 //
 //    # Update latest messages for attesting indices
 //    update_latest_messages(store, indexed_attestation.attesting_indices, attestation)
-// TODO(#6072): This code path is highly untested. Requires comprehensive tests and simpler refactoring.
 func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]uint64, error) {
 	ctx, span := trace.StartSpan(ctx, "blockChain.onAttestation")
 	defer span.End()
@@ -84,14 +83,14 @@ func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]ui
 	}
 
 	// Use the target state to validate attestation and calculate the committees.
-	indexedAtt, err := s.verifyAttestation(ctx, baseState, a)
+	indexedAtt, err := s.verifyAttestationIndices(ctx, baseState, a)
 	if err != nil {
 		return nil, err
 	}
 
-	if indexedAtt.AttestingIndices == nil {
-		return nil, errors.New("nil attesting indices")
-	}
+	// Note that signature verification is ignored here because it was performed in sync's validation pipeline:
+	// validate_aggregate_proof.go and validate_beacon_attestation.go
+	// We assume trusted attestation in this function has verified signature.
 
 	// Update forkchoice store with the new attestation for updating weight.
 	s.forkChoiceStore.ProcessAttestation(ctx, indexedAtt.AttestingIndices, bytesutil.ToBytes32(a.Data.BeaconBlockRoot), a.Data.Target.Epoch)

beacon-chain/blockchain/process_attestation_helpers.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/pkg/errors"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
-	"github.com/prysmaticlabs/prysm/beacon-chain/core/blocks"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/state"
 	stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
@@ -105,8 +104,8 @@ func (s *Service) verifyBeaconBlock(ctx context.Context, data *ethpb.Attestation
 	return nil
 }
 
-// verifyAttestation validates input attestation is valid.
-func (s *Service) verifyAttestation(ctx context.Context, baseState *stateTrie.BeaconState, a *ethpb.Attestation) (*ethpb.IndexedAttestation, error) {
+// verifyAttestationIndices validates input attestation has valid attesting indices.
+func (s *Service) verifyAttestationIndices(ctx context.Context, baseState *stateTrie.BeaconState, a *ethpb.Attestation) (*ethpb.IndexedAttestation, error) {
 	committee, err := helpers.BeaconCommitteeFromState(baseState, a.Data.Slot, a.Data.CommitteeIndex)
 	if err != nil {
 		return nil, err
@@ -115,8 +114,8 @@ func (s *Service) verifyAttestation(ctx context.Context, baseState *stateTrie.Be
 	if err != nil {
 		return nil, err
 	}
-	if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil {
-		return nil, errors.Wrap(err, "could not verify indexed attestation")
+	if err := attestationutil.IsValidAttestationIndices(ctx, indexedAtt); err != nil {
+		return nil, err
 	}
 	return indexedAtt, nil
 }

shared/attestationutil/attestation_utils.go

@@ -149,7 +149,7 @@ func IsValidAttestationIndices(ctx context.Context, indexedAttestation *ethpb.In
 	ctx, span := trace.StartSpan(ctx, "attestationutil.IsValidAttestationIndices")
 	defer span.End()
 
-	if indexedAttestation == nil || indexedAttestation.Data == nil || indexedAttestation.Data.Target == nil {
+	if indexedAttestation == nil || indexedAttestation.Data == nil || indexedAttestation.Data.Target == nil || indexedAttestation.AttestingIndices == nil {
 		return errors.New("nil or missing indexed attestation data")
 	}
 	indices := indexedAttestation.AttestingIndices

shared/attestationutil/attestation_utils_test.go

@@ -67,6 +67,16 @@ func TestIsValidAttestationIndices(t *testing.T) {
 		att       *eth.IndexedAttestation
 		wantedErr string
 	}{
+		{
+			name: "Indices should not be nil",
+			att: &eth.IndexedAttestation{
+				Data: &eth.AttestationData{
+					Target: &eth.Checkpoint{},
+				},
+				Signature: make([]byte, 96),
+			},
+			wantedErr: "nil or missing indexed attestation data",
+		},
 		{
 			name: "Indices should be non-empty",
 			att: &eth.IndexedAttestation{