diff --git a/beacon-chain/core/blocks/attestation.go b/beacon-chain/core/blocks/attestation.go index e916e6cf4..443f92980 100644 --- a/beacon-chain/core/blocks/attestation.go +++ b/beacon-chain/core/blocks/attestation.go @@ -103,35 +103,46 @@ func ProcessAttestationsNoVerifySignature( // used before processing attestation with the beacon state. func VerifyAttestationNoVerifySignature( ctx context.Context, - beaconState iface.BeaconState, + beaconState iface.ReadOnlyBeaconState, att *ethpb.Attestation, -) (iface.BeaconState, error) { +) error { ctx, span := trace.StartSpan(ctx, "core.VerifyAttestationNoVerifySignature") defer span.End() if err := helpers.ValidateNilAttestation(att); err != nil { - return nil, err + return err } currEpoch := helpers.CurrentEpoch(beaconState) prevEpoch := helpers.PrevEpoch(beaconState) data := att.Data if data.Target.Epoch != prevEpoch && data.Target.Epoch != currEpoch { - return nil, fmt.Errorf( + return fmt.Errorf( "expected target epoch (%d) to be the previous epoch (%d) or the current epoch (%d)", data.Target.Epoch, prevEpoch, currEpoch, ) } + + if data.Target.Epoch == currEpoch { + if !beaconState.MatchCurrentJustifiedCheckpoint(data.Source) { + return errors.New("source check point not equal to current justified checkpoint") + } + } else { + if !beaconState.MatchPreviousJustifiedCheckpoint(data.Source) { + return errors.New("source check point not equal to previous justified checkpoint") + } + } + if err := helpers.ValidateSlotTargetEpoch(att.Data); err != nil { - return nil, err + return err } s := att.Data.Slot minInclusionCheck := s+params.BeaconConfig().MinAttestationInclusionDelay <= beaconState.Slot() epochInclusionCheck := beaconState.Slot() <= s+params.BeaconConfig().SlotsPerEpoch if !minInclusionCheck { - return nil, fmt.Errorf( + return fmt.Errorf( "attestation slot %d + inclusion delay %d > state slot %d", s, params.BeaconConfig().MinAttestationInclusionDelay, @@ -139,7 +150,7 @@ func VerifyAttestationNoVerifySignature( ) } if !epochInclusionCheck { - return nil, fmt.Errorf( + return fmt.Errorf( "state slot %d > attestation slot %d + SLOTS_PER_EPOCH %d", beaconState.Slot(), s, @@ -148,28 +159,28 @@ func VerifyAttestationNoVerifySignature( } activeValidatorCount, err := helpers.ActiveValidatorCount(beaconState, att.Data.Target.Epoch) if err != nil { - return nil, err + return err } c := helpers.SlotCommitteeCount(activeValidatorCount) if uint64(att.Data.CommitteeIndex) >= c { - return nil, fmt.Errorf("committee index %d >= committee count %d", att.Data.CommitteeIndex, c) + return fmt.Errorf("committee index %d >= committee count %d", att.Data.CommitteeIndex, c) } if err := helpers.VerifyAttestationBitfieldLengths(beaconState, att); err != nil { - return nil, errors.Wrap(err, "could not verify attestation bitfields") + return 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 + return err } indexedAtt, err := attestationutil.ConvertToIndexed(ctx, att, committee) if err != nil { - return nil, err + return err } - return nil, attestationutil.IsValidAttestationIndices(ctx, indexedAtt) + return attestationutil.IsValidAttestationIndices(ctx, indexedAtt) } // ProcessAttestationNoVerifySignature processes the attestation without verifying the attestation signature. This @@ -182,7 +193,7 @@ func ProcessAttestationNoVerifySignature( ctx, span := trace.StartSpan(ctx, "core.ProcessAttestationNoVerifySignature") defer span.End() - if _, err := VerifyAttestationNoVerifySignature(ctx, beaconState, att); err != nil { + if err := VerifyAttestationNoVerifySignature(ctx, beaconState, att); err != nil { return nil, err } @@ -201,16 +212,10 @@ func ProcessAttestationNoVerifySignature( } if data.Target.Epoch == currEpoch { - if !beaconState.MatchCurrentJustifiedCheckpoint(data.Source) { - return nil, errors.New("source check point not equal to current justified checkpoint") - } if err := beaconState.AppendCurrentEpochAttestations(pendingAtt); err != nil { return nil, err } } else { - if !beaconState.MatchPreviousJustifiedCheckpoint(data.Source) { - return nil, errors.New("source check point not equal to previous justified checkpoint") - } if err := beaconState.AppendPreviousEpochAttestations(pendingAtt); err != nil { return nil, err } diff --git a/beacon-chain/core/blocks/attestation_regression_test.go b/beacon-chain/core/blocks/attestation_regression_test.go index f3232c72a..77bf9833b 100644 --- a/beacon-chain/core/blocks/attestation_regression_test.go +++ b/beacon-chain/core/blocks/attestation_regression_test.go @@ -6,9 +6,13 @@ import ( "testing" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" + "github.com/prysmaticlabs/go-bitfield" "github.com/prysmaticlabs/prysm/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/beacon-chain/state" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/testutil" + "github.com/prysmaticlabs/prysm/shared/testutil/assert" "github.com/prysmaticlabs/prysm/shared/testutil/require" ) @@ -42,3 +46,35 @@ func TestProcessAttestationNoVerifySignature_BeaconFuzzIssue78(t *testing.T) { _, err = blocks.ProcessAttestationNoVerifySignature(ctx, st, att) require.ErrorContains(t, "committee index 1 >= committee count 1", err) } + +// Regression introduced in https://github.com/prysmaticlabs/prysm/pull/8566. +func TestVerifyAttestationNoVerifySignature_IncorrectSourceEpoch(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 := ðpb.Attestation{ + Data: ðpb.AttestationData{ + Source: ðpb.Checkpoint{Epoch: 99, Root: mockRoot[:]}, + Target: ðpb.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.NotEqual(t, nil, err) +} diff --git a/beacon-chain/core/blocks/attestation_test.go b/beacon-chain/core/blocks/attestation_test.go index 58fbf8ccc..99bf297d3 100644 --- a/beacon-chain/core/blocks/attestation_test.go +++ b/beacon-chain/core/blocks/attestation_test.go @@ -366,7 +366,7 @@ func TestVerifyAttestationNoVerifySignature_IncorrectSlotTargetEpoch(t *testing. }, }) wanted := "slot 32 does not match target epoch 0" - _, err := blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att) + err := blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att) assert.ErrorContains(t, wanted, err) } @@ -428,7 +428,7 @@ func TestVerifyAttestationNoVerifySignature_OK(t *testing.T) { require.NoError(t, beaconState.SetCurrentJustifiedCheckpoint(ckp)) require.NoError(t, beaconState.SetCurrentEpochAttestations([]*pb.PendingAttestation{})) - _, err = blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att) + err = blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att) assert.NoError(t, err) } @@ -453,7 +453,7 @@ func TestVerifyAttestationNoVerifySignature_BadAttIdx(t *testing.T) { 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) + err := blocks.VerifyAttestationNoVerifySignature(context.TODO(), beaconState, att) require.ErrorContains(t, "committee index 100 >= committee count 1", err) } diff --git a/beacon-chain/rpc/beaconv1/pool.go b/beacon-chain/rpc/beaconv1/pool.go index 30127afcd..3325b3748 100644 --- a/beacon-chain/rpc/beaconv1/pool.go +++ b/beacon-chain/rpc/beaconv1/pool.go @@ -35,7 +35,7 @@ func (bs *Server) SubmitAttestations(ctx context.Context, req *ethpb.SubmitAttes var validAttestations []*ethpb_alpha.Attestation for _, sourceAtt := range req.Data { att := migration.V1AttToV1Alpha1(sourceAtt) - _, err = blocks.VerifyAttestationNoVerifySignature(ctx, headState, att) + err = blocks.VerifyAttestationNoVerifySignature(ctx, headState, att) if err != nil { continue }