Refactor ProcessAttestationNoVerifySignature to VerifyAttestationNoVerifySignature

beacon-chain/core/blocks/attestation.go

@@ -99,14 +99,14 @@ func ProcessAttestationsNoVerifySignature(
 	return beaconState, nil
 }
 
-// ProcessAttestationNoVerifySignature processes the attestation without verifying the attestation signature. This
-// method is used to validate attestations whose signatures have already been verified.
-func ProcessAttestationNoVerifySignature(
+// VerifyAttestationNoVerifySignature verifies the attestation without verifying the attestation signature. This is
+// used before processing attestation with the beacon state.
+func VerifyAttestationNoVerifySignature(
 	ctx context.Context,
 	beaconState *stateTrie.BeaconState,
 	att *ethpb.Attestation,
 ) (*stateTrie.BeaconState, error) {
-	ctx, span := trace.StartSpan(ctx, "core.ProcessAttestationNoVerifySignature")
+	ctx, span := trace.StartSpan(ctx, "core.VerifyAttestationNoVerifySignature")
 	defer span.End()
 
 	if err := helpers.ValidateNilAttestation(att); err != nil {
@@ -159,6 +159,36 @@ func ProcessAttestationNoVerifySignature(
 		return nil, errors.Wrap(err, "could not verify attestation bitfields")
 	}
 
+	// Verify attesting indices are correct.
+	committee, err := helpers.BeaconCommitteeFromState(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		return nil, err
+	}
+	indexedAtt, err := attestationutil.ConvertToIndexed(ctx, att, committee)
+	if err != nil {
+		return nil, err
+	}
+
+	return nil, attestationutil.IsValidAttestationIndices(ctx, indexedAtt)
+}
+
+// ProcessAttestationNoVerifySignature processes the attestation without verifying the attestation signature. This
+// method is used to validate attestations whose signatures have already been verified.
+func ProcessAttestationNoVerifySignature(
+	ctx context.Context,
+	beaconState *stateTrie.BeaconState,
+	att *ethpb.Attestation,
+) (*stateTrie.BeaconState, error) {
+	ctx, span := trace.StartSpan(ctx, "core.ProcessAttestationNoVerifySignature")
+	defer span.End()
+
+	if _, err := VerifyAttestationNoVerifySignature(ctx, beaconState, att); err != nil {
+		return nil, err
+	}
+
+	currEpoch := helpers.CurrentEpoch(beaconState)
+	data := att.Data
+	s := att.Data.Slot
 	proposerIndex, err := helpers.BeaconProposerIndex(beaconState)
 	if err != nil {
 		return nil, err
@@ -186,19 +216,6 @@ func ProcessAttestationNoVerifySignature(
 		}
 	}
 
-	// Verify attesting indices are correct.
-	committee, err := helpers.BeaconCommitteeFromState(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
-	if err != nil {
-		return nil, err
-	}
-	indexedAtt, err := attestationutil.ConvertToIndexed(ctx, att, committee)
-	if err != nil {
-		return nil, err
-	}
-	if err := attestationutil.IsValidAttestationIndices(ctx, indexedAtt); err != nil {
-		return nil, err
-	}
-
 	return beaconState, nil
 }
 

beacon-chain/core/blocks/attestation_test.go

@@ -81,14 +81,13 @@ func TestProcessAttestations_NeitherCurrentNorPrevEpoch(t *testing.T) {
 }
 
 func TestProcessAttestations_CurrentEpochFFGDataMismatches(t *testing.T) {
-	aggBits := bitfield.NewBitlist(3)
 	attestations := []*ethpb.Attestation{
 		{
 			Data: &ethpb.AttestationData{
 				Target: &ethpb.Checkpoint{Epoch: 0, Root: make([]byte, 32)},
 				Source: &ethpb.Checkpoint{Epoch: 1, Root: make([]byte, 32)},
 			},
-			AggregationBits: aggBits,
+			AggregationBits: bitfield.Bitlist{0x09},
 		},
 	}
 	b := testutil.NewBeaconBlock()
@@ -357,7 +356,7 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestProcessAttestationsNoVerify_IncorrectSlotTargetEpoch(t *testing.T) {
+func TestVerifyAttestationNoVerifySignature_IncorrectSlotTargetEpoch(t *testing.T) {
 	beaconState, _ := testutil.DeterministicGenesisState(t, 1)
 
 	att := testutil.HydrateAttestation(&ethpb.Attestation{
@@ -367,7 +366,7 @@ func TestProcessAttestationsNoVerify_IncorrectSlotTargetEpoch(t *testing.T) {
 		},
 	})
 	wanted := "slot 32 does not match target epoch 0"
-	_, err := blocks.ProcessAttestationNoVerifySignature(context.TODO(), beaconState, att)
+	_, err := blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att)
 	assert.ErrorContains(t, wanted, err)
 }
 
@@ -402,7 +401,38 @@ func TestProcessAttestationsNoVerify_OK(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestProcessAttestationsNoVerify_BadAttIdx(t *testing.T) {
+func TestVerifyAttestationNoVerifySignature_OK(t *testing.T) {
+	// Attestation with an empty signature
+
+	beaconState, _ := testutil.DeterministicGenesisState(t, 100)
+
+	aggBits := bitfield.NewBitlist(3)
+	aggBits.SetBitAt(1, true)
+	var mockRoot [32]byte
+	copy(mockRoot[:], "hello-world")
+	att := &ethpb.Attestation{
+		Data: &ethpb.AttestationData{
+			Source: &ethpb.Checkpoint{Epoch: 0, Root: mockRoot[:]},
+			Target: &ethpb.Checkpoint{Epoch: 0, Root: make([]byte, 32)},
+		},
+		AggregationBits: aggBits,
+	}
+
+	zeroSig := [96]byte{}
+	att.Signature = zeroSig[:]
+
+	err := beaconState.SetSlot(beaconState.Slot() + params.BeaconConfig().MinAttestationInclusionDelay)
+	require.NoError(t, err)
+	ckp := beaconState.CurrentJustifiedCheckpoint()
+	copy(ckp.Root, "hello-world")
+	require.NoError(t, beaconState.SetCurrentJustifiedCheckpoint(ckp))
+	require.NoError(t, beaconState.SetCurrentEpochAttestations([]*pb.PendingAttestation{}))
+
+	_, err = blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att)
+	assert.NoError(t, err)
+}
+
+func TestVerifyAttestationNoVerifySignature_BadAttIdx(t *testing.T) {
 	beaconState, _ := testutil.DeterministicGenesisState(t, 100)
 	aggBits := bitfield.NewBitlist(3)
 	aggBits.SetBitAt(1, true)
@@ -423,7 +453,7 @@ func TestProcessAttestationsNoVerify_BadAttIdx(t *testing.T) {
 	copy(ckp.Root, "hello-world")
 	require.NoError(t, beaconState.SetCurrentJustifiedCheckpoint(ckp))
 	require.NoError(t, beaconState.SetCurrentEpochAttestations([]*pb.PendingAttestation{}))
-	_, err := blocks.ProcessAttestationNoVerifySignature(context.TODO(), beaconState, att)
+	_, err := blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att)
 	require.ErrorContains(t, "committee index 100 >= committee count 1", err)
 }