Use spec attestation time verification in gRPC requests (#6429)

* Re-use attestation time verification

* lint

* fix imports

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

beacon-chain/core/helpers/attestation.go

@@ -2,12 +2,15 @@ package helpers
 
 import (
 	"encoding/binary"
+	"fmt"
+	"time"
 
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
 	"github.com/prysmaticlabs/prysm/shared/bls"
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
+	"github.com/prysmaticlabs/prysm/shared/roughtime"
 )
 
 // SlotSignature returns the signed signature of the hash tree root of input slot.
@@ -108,3 +111,48 @@ func ComputeSubnetFromCommitteeAndSlot(activeValCount, comIdx, attSlot uint64) u
 	computedSubnet := (commsSinceStart + comIdx) % params.BeaconNetworkConfig().AttestationSubnetCount
 	return computedSubnet
 }
+
+// ValidateAttestationTime Validates that the incoming attestation is in the desired time range.
+// An attestation is valid only if received within the last ATTESTATION_PROPAGATION_SLOT_RANGE
+// slots.
+//
+// Example:
+//   ATTESTATION_PROPAGATION_SLOT_RANGE = 5
+//   current_slot = 100
+//   invalid_attestation_slot = 92
+//   invalid_attestation_slot = 101
+//   valid_attestation_slot = 98
+// In the attestation must be within the range of 95 to 100 in the example above.
+func ValidateAttestationTime(attSlot uint64, genesisTime time.Time) error {
+	attTime := genesisTime.Add(time.Duration(attSlot*params.BeaconConfig().SecondsPerSlot) * time.Second)
+	currentSlot := SlotsSince(genesisTime)
+
+	// A clock disparity allows for minor tolerances outside of the expected range. This value is
+	// usually small, less than 1 second.
+	clockDisparity := params.BeaconNetworkConfig().MaximumGossipClockDisparity
+
+	// An attestation cannot be from the future, so the upper bounds is set to now, with a minor
+	// tolerance for peer clock disparity.
+	upperBounds := roughtime.Now().Add(clockDisparity)
+
+	// An attestation cannot be older than the current slot - attestation propagation slot range
+	// with a minor tolerance for peer clock disparity.
+	lowerBoundsSlot := uint64(0)
+	if currentSlot > params.BeaconNetworkConfig().AttestationPropagationSlotRange {
+		lowerBoundsSlot = currentSlot - params.BeaconNetworkConfig().AttestationPropagationSlotRange
+	}
+	lowerBounds := genesisTime.Add(
+		time.Duration(lowerBoundsSlot*params.BeaconConfig().SecondsPerSlot) * time.Second,
+	).Add(-clockDisparity)
+
+	// Verify attestation slot within the time range.
+	if attTime.Before(lowerBounds) || attTime.After(upperBounds) {
+		return fmt.Errorf(
+			"attestation slot %d not within attestation propagation range of %d to %d (current slot)",
+			attSlot,
+			currentSlot-params.BeaconNetworkConfig().AttestationPropagationSlotRange,
+			currentSlot,
+		)
+	}
+	return nil
+}

beacon-chain/core/helpers/attestation_test.go

@@ -3,6 +3,7 @@ package helpers_test
 import (
 	"strconv"
 	"testing"
+	"time"
 
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
@@ -11,6 +12,7 @@ import (
 	"github.com/prysmaticlabs/prysm/shared/bls"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
+	"github.com/prysmaticlabs/prysm/shared/roughtime"
 	"github.com/prysmaticlabs/prysm/shared/testutil"
 )
 
@@ -179,3 +181,89 @@ func TestAttestation_ComputeSubnetForAttestation(t *testing.T) {
 		t.Errorf("Did not get correct subnet for attestation, wanted %d but got %d", 6, sub)
 	}
 }
+
+func Test_ValidateAttestationTime(t *testing.T) {
+	if params.BeaconNetworkConfig().MaximumGossipClockDisparity < 200*time.Millisecond {
+		t.Fatal("This test expects the maximum clock disparity to be at least 200ms")
+	}
+
+	type args struct {
+		attSlot     uint64
+		genesisTime time.Time
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "attestation.slot == current_slot",
+			args: args{
+				attSlot:     15,
+				genesisTime: roughtime.Now().Add(-15 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
+			},
+			wantErr: false,
+		},
+		{
+			name: "attestation.slot == current_slot, received in middle of slot",
+			args: args{
+				attSlot: 15,
+				genesisTime: roughtime.Now().Add(
+					-15 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second,
+				).Add(-(time.Duration(params.BeaconConfig().SecondsPerSlot/2) * time.Second)),
+			},
+			wantErr: false,
+		},
+		{
+			name: "attestation.slot == current_slot, received 200ms early",
+			args: args{
+				attSlot: 16,
+				genesisTime: roughtime.Now().Add(
+					-16 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second,
+				).Add(-200 * time.Millisecond),
+			},
+			wantErr: false,
+		},
+		{
+			name: "attestation.slot > current_slot",
+			args: args{
+				attSlot:     16,
+				genesisTime: roughtime.Now().Add(-15 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
+			},
+			wantErr: true,
+		},
+		{
+			name: "attestation.slot < current_slot-ATTESTATION_PROPAGATION_SLOT_RANGE",
+			args: args{
+				attSlot:     100 - params.BeaconNetworkConfig().AttestationPropagationSlotRange - 1,
+				genesisTime: roughtime.Now().Add(-100 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
+			},
+			wantErr: true,
+		},
+		{
+			name: "attestation.slot = current_slot-ATTESTATION_PROPAGATION_SLOT_RANGE",
+			args: args{
+				attSlot:     100 - params.BeaconNetworkConfig().AttestationPropagationSlotRange,
+				genesisTime: roughtime.Now().Add(-100 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
+			},
+			wantErr: false,
+		},
+		{
+			name: "attestation.slot = current_slot-ATTESTATION_PROPAGATION_SLOT_RANGE, received 200ms late",
+			args: args{
+				attSlot: 100 - params.BeaconNetworkConfig().AttestationPropagationSlotRange,
+				genesisTime: roughtime.Now().Add(
+					-100 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second,
+				).Add(200 * time.Millisecond),
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := helpers.ValidateAttestationTime(tt.args.attSlot, tt.args.genesisTime); (err != nil) != tt.wantErr {
+				t.Errorf("validateAggregateAttTime() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

beacon-chain/rpc/validator/attester.go

@@ -2,6 +2,7 @@ package validator
 
 import (
 	"context"
+	"fmt"
 
 	ptypes "github.com/gogo/protobuf/types"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
@@ -21,8 +22,6 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-const msgInvalidAttestationRequest = "Attestation request must be within current or previous epoch"
-
 // GetAttestationData requests that the beacon node produce an attestation data object,
 // which the validator acting as an attester will then sign.
 func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.AttestationDataRequest) (*ethpb.AttestationData, error) {
@@ -37,9 +36,8 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
 		return nil, status.Errorf(codes.Unavailable, "Syncing to latest head, not ready to respond")
 	}
 
-	currentEpoch := helpers.SlotToEpoch(vs.GenesisTimeFetcher.CurrentSlot())
-	if currentEpoch > 0 && currentEpoch-1 != helpers.SlotToEpoch(req.Slot) && currentEpoch != helpers.SlotToEpoch(req.Slot) {
-		return nil, status.Error(codes.InvalidArgument, msgInvalidAttestationRequest)
+	if err := helpers.ValidateAttestationTime(req.Slot, vs.GenesisTimeFetcher.GenesisTime()); err != nil {
+		return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid request: %v", err))
 	}
 
 	res, err := vs.AttestationCache.Get(ctx, req)

beacon-chain/rpc/validator/attester_test.go

@@ -408,8 +408,8 @@ func TestServer_GetAttestationData_InvalidRequestSlot(t *testing.T) {
 		Slot: 1000000000000,
 	}
 	_, err := attesterServer.GetAttestationData(ctx, req)
-	if s, ok := status.FromError(err); !ok || s.Message() != msgInvalidAttestationRequest {
-		t.Fatalf("Wrong error. Wanted %v, got %v", msgInvalidAttestationRequest, err)
+	if s, ok := status.FromError(err); !ok || !strings.Contains(s.Message(), "invalid request") {
+		t.Fatalf("Wrong error. Wanted error to start with %v, got %v", "invalid request", err)
 	}
 }
 

beacon-chain/sync/utils_test.go

@@ -3,13 +3,9 @@ package sync
 import (
 	"math/rand"
 	"testing"
-	"time"
 
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
-	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
-	"github.com/prysmaticlabs/prysm/shared/params"
-	"github.com/prysmaticlabs/prysm/shared/roughtime"
 )
 
 func TestSortedObj_SortBlocksRoots(t *testing.T) {
@@ -80,33 +76,3 @@ func TestSortedObj_NoDuplicates(t *testing.T) {
 		rootMap[newRoots[i]] = true
 	}
 }
-
-func TestValidateAggregatedTime_ValidatesCorrectly(t *testing.T) {
-	const genesisOffset = 1200
-	genTime := roughtime.Now().Add(-(genesisOffset * time.Second))
-	currSlot := helpers.SlotsSince(genTime)
-	invalidAttSlot := currSlot - params.BeaconNetworkConfig().AttestationPropagationSlotRange - 1
-	err := validateAggregateAttTime(invalidAttSlot, genTime)
-	if err == nil {
-		t.Error("Expected attestation time to be invalid, but it was marked as valid")
-	}
-	timePerSlot := time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second
-	// adjusts the genesis time to allow for the clock disparity to
-	// allow for the attestation to be valid.
-	clockAllowance := params.BeaconNetworkConfig().MaximumGossipClockDisparity * 8 / 10
-	newTime := genTime.Add(timePerSlot - clockAllowance)
-	err = validateAggregateAttTime(invalidAttSlot, newTime)
-	if err != nil {
-		t.Errorf("Expected attestation time to be valid, but it was not: %v", err)
-	}
-	// re-determine the current slot
-	currSlot = helpers.SlotsSince(genTime)
-	err = validateAggregateAttTime(currSlot+1, genTime)
-	if err == nil {
-		t.Error("Expected attestation time to be invalid, but it was marked as valid")
-	}
-	err = validateAggregateAttTime(currSlot-10, genTime)
-	if err != nil {
-		t.Errorf("Expected attestation time to be valid, but it was not: %v", err)
-	}
-}

beacon-chain/sync/validate_aggregate_proof.go

@@ -3,7 +3,6 @@ package sync
 import (
 	"context"
 	"fmt"
-	"time"
 
 	"github.com/libp2p/go-libp2p-core/peer"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
@@ -17,7 +16,6 @@ import (
 	"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"
 )
@@ -87,7 +85,7 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe
 	defer span.End()
 
 	attSlot := signed.Message.Aggregate.Data.Slot
-	if err := validateAggregateAttTime(attSlot, s.chain.GenesisTime()); err != nil {
+	if err := helpers.ValidateAttestationTime(attSlot, s.chain.GenesisTime()); err != nil {
 		traceutil.AnnotateError(span, err)
 		return pubsub.ValidationIgnore
 	}
@@ -191,50 +189,6 @@ func validateIndexInCommittee(ctx context.Context, bs *stateTrie.BeaconState, a
 	return nil
 }
 
-// Validates that the incoming aggregate attestation is in the desired time range. An attestation
-// is valid only if received within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots.
-//
-// Example:
-//   ATTESTATION_PROPAGATION_SLOT_RANGE = 5
-//   current_slot = 100
-//   invalid_attestation_slot = 92
-//   invalid_attestation_slot = 101
-//   valid_attestation_slot = 98
-// In the attestation must be within the range of 95 to 100 in the example above.
-func validateAggregateAttTime(attSlot uint64, genesisTime time.Time) error {
-	attTime := genesisTime.Add(time.Duration(attSlot*params.BeaconConfig().SecondsPerSlot) * time.Second)
-	currentSlot := helpers.SlotsSince(genesisTime)
-
-	// A clock disparity allows for minor tolerances outside of the expected range. This value is
-	// usually small, less than 1 second.
-	clockDisparity := params.BeaconNetworkConfig().MaximumGossipClockDisparity
-
-	// An attestation cannot be from the future, so the upper bounds is set to now, with a minor
-	// tolerance for peer clock disparity.
-	upperBounds := roughtime.Now().Add(clockDisparity)
-
-	// An attestation cannot be older than the current slot - attestation propagation slot range
-	// with a minor tolerance for peer clock disparity.
-	lowerBoundsSlot := uint64(0)
-	if currentSlot > params.BeaconNetworkConfig().AttestationPropagationSlotRange {
-		lowerBoundsSlot = currentSlot - params.BeaconNetworkConfig().AttestationPropagationSlotRange
-	}
-	lowerBounds := genesisTime.Add(
-		time.Duration(lowerBoundsSlot*params.BeaconConfig().SecondsPerSlot) * time.Second,
-	).Add(-clockDisparity)
-
-	// Verify attestation slot within the time range.
-	if attTime.Before(lowerBounds) || attTime.After(upperBounds) {
-		return fmt.Errorf(
-			"attestation slot %d not within attestation propagation range of %d to %d (current slot)",
-			attSlot,
-			currentSlot-params.BeaconNetworkConfig().AttestationPropagationSlotRange,
-			currentSlot,
-		)
-	}
-	return nil
-}
-
 // This validates selection proof by validating it's from the correct validator index of the slot and selection
 // proof is a valid signature.
 func validateSelection(ctx context.Context, bs *stateTrie.BeaconState, data *ethpb.AttestationData, validatorIndex uint64, proof []byte) error {

beacon-chain/sync/validate_aggregate_proof_test.go

@@ -27,7 +27,6 @@ import (
 	"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/testutil"
 )
 
@@ -638,89 +637,3 @@ func TestVerifyIndexInCommittee_SeenAggregatorEpoch(t *testing.T) {
 		t.Fatal("Validated status is true")
 	}
 }
-
-func Test_validateAggregateAttTime(t *testing.T) {
-	if params.BeaconNetworkConfig().MaximumGossipClockDisparity < 200*time.Millisecond {
-		t.Fatal("This test expects the maximum clock disparity to be at least 200ms")
-	}
-
-	type args struct {
-		attSlot     uint64
-		genesisTime time.Time
-	}
-	tests := []struct {
-		name    string
-		args    args
-		wantErr bool
-	}{
-		{
-			name: "attestation.slot == current_slot",
-			args: args{
-				attSlot:     15,
-				genesisTime: roughtime.Now().Add(-15 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
-			},
-			wantErr: false,
-		},
-		{
-			name: "attestation.slot == current_slot, received in middle of slot",
-			args: args{
-				attSlot: 15,
-				genesisTime: roughtime.Now().Add(
-					-15 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second,
-				).Add(-(time.Duration(params.BeaconConfig().SecondsPerSlot/2) * time.Second)),
-			},
-			wantErr: false,
-		},
-		{
-			name: "attestation.slot == current_slot, received 200ms early",
-			args: args{
-				attSlot: 16,
-				genesisTime: roughtime.Now().Add(
-					-16 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second,
-				).Add(-200 * time.Millisecond),
-			},
-			wantErr: false,
-		},
-		{
-			name: "attestation.slot > current_slot",
-			args: args{
-				attSlot:     16,
-				genesisTime: roughtime.Now().Add(-15 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
-			},
-			wantErr: true,
-		},
-		{
-			name: "attestation.slot < current_slot-ATTESTATION_PROPAGATION_SLOT_RANGE",
-			args: args{
-				attSlot:     100 - params.BeaconNetworkConfig().AttestationPropagationSlotRange - 1,
-				genesisTime: roughtime.Now().Add(-100 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
-			},
-			wantErr: true,
-		},
-		{
-			name: "attestation.slot = current_slot-ATTESTATION_PROPAGATION_SLOT_RANGE",
-			args: args{
-				attSlot:     100 - params.BeaconNetworkConfig().AttestationPropagationSlotRange,
-				genesisTime: roughtime.Now().Add(-100 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second),
-			},
-			wantErr: false,
-		},
-		{
-			name: "attestation.slot = current_slot-ATTESTATION_PROPAGATION_SLOT_RANGE, received 200ms late",
-			args: args{
-				attSlot: 100 - params.BeaconNetworkConfig().AttestationPropagationSlotRange,
-				genesisTime: roughtime.Now().Add(
-					-100 * time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second,
-				).Add(200 * time.Millisecond),
-			},
-			wantErr: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if err := validateAggregateAttTime(tt.args.attSlot, tt.args.genesisTime); (err != nil) != tt.wantErr {
-				t.Errorf("validateAggregateAttTime() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}

beacon-chain/sync/validate_beacon_attestation.go

@@ -118,7 +118,7 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p
 	}
 
 	// Attestation's slot is within ATTESTATION_PROPAGATION_SLOT_RANGE.
-	if err := validateAggregateAttTime(att.Data.Slot, s.chain.GenesisTime()); err != nil {
+	if err := helpers.ValidateAttestationTime(att.Data.Slot, s.chain.GenesisTime()); err != nil {
 		traceutil.AnnotateError(span, err)
 		return pubsub.ValidationIgnore
 	}