Loosen Restrictions for Certain P2P Attestation Validation Conditions

beacon-chain/sync/validate_aggregate_proof.go

@@ -8,18 +8,17 @@ import (
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
 	"github.com/pkg/errors"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
+	"go.opencensus.io/trace"
 	"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"
-	"github.com/prysmaticlabs/prysm/shared/attestationutil"
 	"github.com/prysmaticlabs/prysm/shared/bls"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/roughtime"
 	"github.com/prysmaticlabs/prysm/shared/traceutil"
-	"go.opencensus.io/trace"
 )
 
 // validateAggregateAndProof verifies the aggregated signature and the selection proof is valid before forwarding to the
@@ -106,7 +105,7 @@ func (r *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe
 		}
 	}
 
-	// Verify validator index is within the aggregate's committee.
+	// Verify validator index is within the beacon committee.
 	if err := validateIndexInCommittee(ctx, s, signed.Message.Aggregate, signed.Message.AggregatorIndex); err != nil {
 		traceutil.AnnotateError(span, errors.Wrapf(err, "Could not validate index in committee"))
 		return false
@@ -167,7 +166,7 @@ func (r *Service) setAggregatorIndexEpochSeen(epoch uint64, aggregatorIndex uint
 	r.seenAttestationCache.Add(string(b), true)
 }
 
-// This validates the aggregator's index in state is within the attesting indices of the attestation.
+// This validates the aggregator's index in state is within the beacon committee.
 func validateIndexInCommittee(ctx context.Context, s *stateTrie.BeaconState, a *ethpb.Attestation, validatorIndex uint64) error {
 	ctx, span := trace.StartSpan(ctx, "sync.validateIndexInCommittee")
 	defer span.End()
@@ -176,17 +175,16 @@ func validateIndexInCommittee(ctx context.Context, s *stateTrie.BeaconState, a *
 	if err != nil {
 		return err
 	}
-	attestingIndices := attestationutil.AttestingIndices(a.AggregationBits, committee)
 	var withinCommittee bool
-	for _, i := range attestingIndices {
+	for _, i := range committee {
 		if validatorIndex == i {
 			withinCommittee = true
 			break
 		}
 	}
 	if !withinCommittee {
 		return fmt.Errorf("validator index %d is not within the committee: %v",
-			validatorIndex, attestingIndices)
+			validatorIndex, committee)
 	}
 	return nil
 }

beacon-chain/sync/validate_aggregate_proof_test.go

@@ -65,6 +65,37 @@ func TestVerifyIndexInCommittee_CanVerify(t *testing.T) {
 	}
 }
 
+func TestVerifyIndexInCommittee_ExistsInBeaconCommittee(t *testing.T) {
+	ctx := context.Background()
+	params.UseMinimalConfig()
+	defer params.UseMainnetConfig()
+
+	validators := uint64(64)
+	s, _ := testutil.DeterministicGenesisState(t, validators)
+	if err := s.SetSlot(params.BeaconConfig().SlotsPerEpoch); err != nil {
+		t.Fatal(err)
+	}
+
+	bf := []byte{0xff}
+	att := &ethpb.Attestation{Data: &ethpb.AttestationData{
+		Target: &ethpb.Checkpoint{Epoch: 0}},
+		AggregationBits: bf}
+
+	committee, err := helpers.BeaconCommitteeFromState(s, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if err := validateIndexInCommittee(ctx, s, att, committee[0]); err != nil {
+		t.Fatal(err)
+	}
+
+	wanted := "validator index 1000 is not within the committee"
+	if err := validateIndexInCommittee(ctx, s, att, 1000); err == nil || !strings.Contains(err.Error(), wanted) {
+		t.Error("Did not receive wanted error")
+	}
+}
+
 func TestVerifySelection_NotAnAggregator(t *testing.T) {
 	ctx := context.Background()
 	params.UseMinimalConfig()

beacon-chain/sync/validate_beacon_attestation.go

@@ -9,7 +9,9 @@ import (
 	"github.com/libp2p/go-libp2p-core/peer"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
 	eth "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
+	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/beacon-chain/p2p"
+	"github.com/prysmaticlabs/prysm/shared/attestationutil"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
 	"github.com/prysmaticlabs/prysm/shared/traceutil"
@@ -18,7 +20,7 @@ import (
 
 // Validation
 // - The attestation's committee index (attestation.data.index) is for the correct subnet.
-// - The attestation is unaggregated -- that is, it has exactly one participating validator (len([bit for bit in attestation.aggregation_bits if bit == 0b1]) == 1).
+// - The attestation is unaggregated -- that is, it has exactly one participating validator (len(get_attesting_indices(state, attestation.data, attestation.aggregation_bits)) == 1).
 // - The block being voted for (attestation.data.beacon_block_root) passes validation.
 // - attestation.data.slot is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots (attestation.data.slot + ATTESTATION_PROPAGATION_SLOT_RANGE >= current_slot >= attestation.data.slot).
 // - The signature of attestation is valid.
@@ -56,6 +58,7 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p
 	if att.Data == nil {
 		return false
 	}
+
 	// Verify this the first attestation received for the participating validator for the slot.
 	if s.hasSeenCommitteeIndicesSlot(att.Data.Slot, att.Data.CommitteeIndex, att.AggregationBits) {
 		return false
@@ -72,8 +75,23 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p
 		return false
 	}
 
+	// Attestation aggregation bits must exist.
+	if att.AggregationBits == nil {
+		return false
+	}
+	st, err := s.chain.AttestationPreState(ctx, att)
+	if err != nil {
+		traceutil.AnnotateError(span, err)
+		return false
+	}
+	committee, err := helpers.BeaconCommitteeFromState(st, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		traceutil.AnnotateError(span, err)
+		return false
+	}
+
 	// Attestation must be unaggregated.
-	if att.AggregationBits == nil || att.AggregationBits.Count() != 1 {
+	if len(attestationutil.AttestingIndices(att.AggregationBits, committee)) != 1 {
 		return false
 	}
 

beacon-chain/sync/validate_beacon_attestation_test.go

@@ -28,7 +28,8 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 	p := p2ptest.NewTestP2P(t)
 	db := dbtest.SetupDB(t)
 	chain := &mockChain.ChainService{
-		Genesis:          time.Now().Add(time.Duration(-64*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second), // 64 slots ago
+		// 1 slot ago.
+		Genesis:          time.Now().Add(time.Duration(-1*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second),
 		ValidatorsRoot:   [32]byte{'A'},
 		ValidAttestation: true,
 	}
@@ -53,7 +54,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 
 	blk := &ethpb.SignedBeaconBlock{
 		Block: &ethpb.BeaconBlock{
-			Slot: 55,
+			Slot: 1,
 		},
 	}
 	if err := db.SaveBlock(ctx, blk); err != nil {
@@ -65,10 +66,15 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	savedState := testutil.NewBeaconState()
+	validators := uint64(64)
+	savedState, _ := testutil.DeterministicGenesisState(t, validators)
+	if err := savedState.SetSlot(1); err != nil {
+		t.Fatal(err)
+	}
 	if err := db.SaveState(context.Background(), savedState, validBlockRoot); err != nil {
 		t.Fatal(err)
 	}
+	chain.State = savedState
 
 	tests := []struct {
 		name                      string
@@ -78,27 +84,31 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 		want                      bool
 	}{
 		{
-			name: "validAttestationSignature",
+			name: "valid attestation signature",
 			msg: &ethpb.Attestation{
 				AggregationBits: bitfield.Bitlist{0b1010},
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: validBlockRoot[:],
-					CommitteeIndex:  1,
-					Slot:            63,
+					CommitteeIndex:  0,
+					Slot:            1,
+					Target: &ethpb.Checkpoint{
+						Epoch: 0,
+						Root:  validBlockRoot[:],
+					},
 				},
 			},
-			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_1", digest),
+			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_0", digest),
 			validAttestationSignature: true,
 			want:                      true,
 		},
 		{
-			name: "alreadySeen",
+			name: "already seen",
 			msg: &ethpb.Attestation{
 				AggregationBits: bitfield.Bitlist{0b1010},
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: validBlockRoot[:],
-					CommitteeIndex:  1,
-					Slot:            63,
+					CommitteeIndex:  0,
+					Slot:            1,
 				},
 			},
 			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_1", digest),
@@ -112,7 +122,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: validBlockRoot[:],
 					CommitteeIndex:  2,
-					Slot:            63,
+					Slot:            1,
 				},
 			},
 			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_3", digest),
@@ -126,7 +136,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: validBlockRoot[:],
 					CommitteeIndex:  1,
-					Slot:            63,
+					Slot:            1,
 				},
 			},
 			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_1", digest),
@@ -140,7 +150,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: bytesutil.PadTo([]byte("missing"), 32),
 					CommitteeIndex:  1,
-					Slot:            63,
+					Slot:            1,
 				},
 			},
 			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_1", digest),
@@ -154,7 +164,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: validBlockRoot[:],
 					CommitteeIndex:  1,
-					Slot:            63,
+					Slot:            1,
 				},
 			},
 			topic:                     fmt.Sprintf("/eth2/%x/beacon_attestation_1", digest),