Clean up verify attestation and better error log (#4729)

beacon-chain/blockchain/forkchoice/process_attestation.go

@@ -226,29 +226,9 @@ func (s *Store) verifyAttestation(ctx context.Context, baseState *stateTrie.Beac
 	}
 
 	if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil {
-
-		// TODO(3603): Delete the following signature verify fallback when issue 3603 closes.
-		// When signature fails to verify with committee cache enabled at run time,
-		// the following re-runs the same signature verify routine without cache in play.
-		// This provides extra assurance that committee cache can't break run time.
-		if err == blocks.ErrSigFailedToVerify {
-			committee, err = helpers.BeaconCommitteeWithoutCache(baseState, a.Data.Slot, a.Data.CommitteeIndex)
-			if err != nil {
-				return nil, errors.Wrap(err, "could not convert attestation to indexed attestation without cache")
-			}
-			indexedAtt, err = attestationutil.ConvertToIndexed(ctx, a, committee)
-			if err != nil {
-				return nil, errors.Wrap(err, "could not convert attestation to indexed attestation")
-			}
-			if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil {
-				return nil, errors.Wrap(err, "could not verify indexed attestation without cache")
-			}
-			sigFailsToVerify.Inc()
-			return indexedAtt, nil
-		}
-
 		return nil, errors.Wrap(err, "could not verify indexed attestation")
 	}
+
 	return indexedAtt, nil
 }
 

beacon-chain/blockchain/process_attestation_helpers.go

@@ -95,27 +95,6 @@ func (s *Service) verifyAttestation(ctx context.Context, baseState *stateTrie.Be
 	}
 
 	if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil {
-
-		// TODO(3603): Delete the following signature verify fallback when issue 3603 closes.
-		// When signature fails to verify with committee cache enabled at run time,
-		// the following re-runs the same signature verify routine without cache in play.
-		// This provides extra assurance that committee cache can't break run time.
-		if err == blocks.ErrSigFailedToVerify {
-			committee, err = helpers.BeaconCommitteeWithoutCache(baseState, a.Data.Slot, a.Data.CommitteeIndex)
-			if err != nil {
-				return nil, errors.Wrap(err, "could not convert attestation to indexed attestation without cache")
-			}
-			indexedAtt, err = attestationutil.ConvertToIndexed(ctx, a, committee)
-			if err != nil {
-				return nil, errors.Wrap(err, "could not convert attestation to indexed attestation")
-			}
-			if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil {
-				return nil, errors.Wrap(err, "could not verify indexed attestation without cache")
-			}
-			sigFailsToVerify.Inc()
-			return indexedAtt, nil
-		}
-
 		return nil, errors.Wrap(err, "could not verify indexed attestation")
 	}
 

beacon-chain/blockchain/receive_attestation.go

@@ -112,7 +112,11 @@ func (s *Service) processAttestation() {
 
 				if err := s.ReceiveAttestationNoPubsub(ctx, a); err != nil {
 					log.WithFields(logrus.Fields{
-						"targetRoot": fmt.Sprintf("%#x", a.Data.Target.Root),
+						"slot": a.Data.Slot,
+						"committeeIndex": a.Data.CommitteeIndex,
+						"beaconBlockRoot": fmt.Sprintf("%#x", bytesutil.Trunc(a.Data.BeaconBlockRoot)),
+						"targetRoot": fmt.Sprintf("%#x", bytesutil.Trunc(a.Data.Target.Root)),
+						"aggregationCount": a.AggregationBits.Count(),
 					}).WithError(err).Error("Could not receive attestation in chain service")
 				}
 			}

beacon-chain/core/helpers/committee.go

@@ -109,27 +109,6 @@ func BeaconCommittee(validatorIndices []uint64, seed [32]byte, slot uint64, comm
 	return ComputeCommittee(validatorIndices, seed, epochOffset, count)
 }
 
-// BeaconCommitteeWithoutCache returns the crosslink committee of a given slot and committee index without the
-// usage of committee cache.
-// TODO(3603): Delete this function when issue 3603 closes.
-func BeaconCommitteeWithoutCache(state *stateTrie.BeaconState, slot uint64, index uint64) ([]uint64, error) {
-	epoch := SlotToEpoch(slot)
-	indices, err := ActiveValidatorIndices(state, epoch)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get active indices")
-	}
-	committeesPerSlot := SlotCommitteeCount(uint64(len(indices)))
-	epochOffset := index + (slot%params.BeaconConfig().SlotsPerEpoch)*committeesPerSlot
-	count := committeesPerSlot * params.BeaconConfig().SlotsPerEpoch
-
-	seed, err := Seed(state, epoch, params.BeaconConfig().DomainBeaconAttester)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get seed")
-	}
-
-	return ComputeCommittee(indices, seed, epochOffset, count)
-}
-
 // ComputeCommittee returns the requested shuffled committee out of the total committees using
 // validator indices and seed.
 //

beacon-chain/core/helpers/committee_test.go

@@ -149,67 +149,6 @@ func TestAttestationParticipants_NoCommitteeCache(t *testing.T) {
 	}
 }
 
-func TestAttestingIndicesWithBeaconCommitteeWithoutCache_Ok(t *testing.T) {
-	committeeSize := uint64(16)
-	validators := make([]*ethpb.Validator, committeeSize*params.BeaconConfig().SlotsPerEpoch)
-	for i := 0; i < len(validators); i++ {
-		validators[i] = &ethpb.Validator{
-			ExitEpoch: params.BeaconConfig().FarFutureEpoch,
-		}
-	}
-
-	state, _ := beaconstate.InitializeFromProto(&pb.BeaconState{
-		Slot:        params.BeaconConfig().SlotsPerEpoch,
-		Validators:  validators,
-		RandaoMixes: make([][]byte, params.BeaconConfig().EpochsPerHistoricalVector),
-	})
-
-	attestationData := &ethpb.AttestationData{}
-
-	tests := []struct {
-		attestationSlot uint64
-		bitfield        bitfield.Bitlist
-		wanted          []uint64
-	}{
-		{
-			attestationSlot: 3,
-			bitfield:        bitfield.Bitlist{0x07},
-			wanted:          []uint64{344, 221},
-		},
-		{
-			attestationSlot: 2,
-			bitfield:        bitfield.Bitlist{0x05},
-			wanted:          []uint64{207},
-		},
-		{
-			attestationSlot: 11,
-			bitfield:        bitfield.Bitlist{0x07},
-			wanted:          []uint64{409, 213},
-		},
-	}
-
-	for _, tt := range tests {
-		attestationData.Target = &ethpb.Checkpoint{Epoch: 0}
-		attestationData.Slot = tt.attestationSlot
-		committee, err := BeaconCommitteeWithoutCache(state, tt.attestationSlot, 0 /* committee index */)
-		if err != nil {
-			t.Error(err)
-		}
-		result, err := attestationutil.AttestingIndices(tt.bitfield, committee)
-		if err != nil {
-			t.Errorf("Failed to get attestation participants: %v", err)
-		}
-
-		if !reflect.DeepEqual(tt.wanted, result) {
-			t.Errorf(
-				"Result indices was an unexpected value. Wanted %d, got %d",
-				tt.wanted,
-				result,
-			)
-		}
-	}
-}
-
 func TestAttestationParticipants_EmptyBitfield(t *testing.T) {
 	validators := make([]*ethpb.Validator, params.BeaconConfig().MinGenesisActiveValidatorCount)
 	for i := 0; i < len(validators); i++ {