Fix Gossip Clock Disparity (#6141)

* fix disparity
* Merge refs/heads/master into fixGossipDisparity
* Merge refs/heads/master into fixGossipDisparity
* Merge refs/heads/master into fixGossipDisparity
* Merge refs/heads/master into fixGossipDisparity
* Merge refs/heads/master into fixGossipDisparity
* Merge refs/heads/master into fixGossipDisparity

beacon-chain/blockchain/process_attestation.go

@@ -10,6 +10,7 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/flags"
 	stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
 	"github.com/prysmaticlabs/prysm/shared/bytesutil"
+	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/roughtime"
 	"go.opencensus.io/trace"
 )
@@ -108,7 +109,7 @@ func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]ui
 	}
 
 	// Verify Attestations cannot be from future epochs.
-	if err := helpers.VerifySlotTime(genesisTime, tgtSlot, helpers.TimeShiftTolerance); err != nil {
+	if err := helpers.VerifySlotTime(genesisTime, tgtSlot, params.BeaconNetworkConfig().MaximumGossipClockDisparity); err != nil {
 		return nil, errors.Wrap(err, "could not verify attestation target slot")
 	}
 
@@ -118,7 +119,7 @@ func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]ui
 	}
 
 	// Verify attestations can only affect the fork choice of subsequent slots.
-	if err := helpers.VerifySlotTime(genesisTime, a.Data.Slot+1, helpers.TimeShiftTolerance); err != nil {
+	if err := helpers.VerifySlotTime(genesisTime, a.Data.Slot+1, params.BeaconNetworkConfig().MaximumGossipClockDisparity); err != nil {
 		return nil, err
 	}
 

beacon-chain/blockchain/process_block_helpers.go

@@ -55,7 +55,7 @@ func (s *Service) getBlockPreState(ctx context.Context, b *ethpb.BeaconBlock) (*
 	}
 
 	// Verify block slot time is not from the feature.
-	if err := helpers.VerifySlotTime(preState.GenesisTime(), b.Slot, helpers.TimeShiftTolerance); err != nil {
+	if err := helpers.VerifySlotTime(preState.GenesisTime(), b.Slot, params.BeaconNetworkConfig().MaximumGossipClockDisparity); err != nil {
 		return nil, err
 	}
 

beacon-chain/blockchain/receive_attestation.go

@@ -101,7 +101,7 @@ func (s *Service) processAttestation(subscribedToStateEvents chan struct{}) {
 				// This delays consideration in the fork choice until their slot is in the past.
 				// https://github.com/ethereum/eth2.0-specs/blob/dev/specs/phase0/fork-choice.md#validate_on_attestation
 				nextSlot := a.Data.Slot + 1
-				if err := helpers.VerifySlotTime(uint64(s.genesisTime.Unix()), nextSlot, helpers.TimeShiftTolerance); err != nil {
+				if err := helpers.VerifySlotTime(uint64(s.genesisTime.Unix()), nextSlot, params.BeaconNetworkConfig().MaximumGossipClockDisparity); err != nil {
 					continue
 				}
 

beacon-chain/core/helpers/slot_epoch.go

@@ -90,9 +90,6 @@ func SlotsSinceEpochStarts(slot uint64) uint64 {
 	return slot - StartSlot(SlotToEpoch(slot))
 }
 
-// TimeShiftTolerance specifies the tolerance threshold for slots "from the future".
-const TimeShiftTolerance = 500 * time.Millisecond // ms
-
 // VerifySlotTime validates the input slot is not from the future.
 func VerifySlotTime(genesisTime uint64, slot uint64, timeTolerance time.Duration) error {
 	slotTime, err := SlotToTime(genesisTime, slot)

beacon-chain/sync/subscriber.go

@@ -26,8 +26,6 @@ import (
 
 const pubsubMessageTimeout = 30 * time.Second
 
-var maximumGossipClockDisparity = params.BeaconNetworkConfig().MaximumGossipClockDisparity
-
 // subHandler represents handler for a given subscription.
 type subHandler func(context.Context, proto.Message) error
 

beacon-chain/sync/utils_test.go

@@ -3,9 +3,13 @@ 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) {
@@ -40,3 +44,33 @@ func TestSortedObj_SortBlocksRoots(t *testing.T) {
 		previousSlot = b.Block.Slot
 	}
 }
+
+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,6 +3,7 @@ package sync
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/libp2p/go-libp2p-core/peer"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
@@ -87,7 +88,7 @@ func (r *Service) validateAggregatedAtt(ctx context.Context, signed *ethpb.Signe
 	defer span.End()
 
 	attSlot := signed.Message.Aggregate.Data.Slot
-	if err := validateAggregateAttTime(attSlot, uint64(r.chain.GenesisTime().Unix())); err != nil {
+	if err := validateAggregateAttTime(attSlot, r.chain.GenesisTime()); err != nil {
 		traceutil.AnnotateError(span, err)
 		return pubsub.ValidationIgnore
 	}
@@ -193,18 +194,18 @@ func validateIndexInCommittee(ctx context.Context, s *stateTrie.BeaconState, a *
 }
 
 // Validates that the incoming aggregate attestation is in the desired time range.
-func validateAggregateAttTime(attSlot uint64, genesisTime uint64) error {
-	// in milliseconds
-	attTime := 1000 * (genesisTime + (attSlot * params.BeaconConfig().SecondsPerSlot))
+func validateAggregateAttTime(attSlot uint64, genesisTime time.Time) error {
+	// set expected boundaries for attestations
+	attTime := genesisTime.Add(time.Duration(attSlot*params.BeaconConfig().SecondsPerSlot) * time.Second)
 	attSlotRange := attSlot + params.BeaconNetworkConfig().AttestationPropagationSlotRange
-	attTimeRange := 1000 * (genesisTime + (attSlotRange * params.BeaconConfig().SecondsPerSlot))
-	currentTimeInSec := roughtime.Now().Unix()
-	currentTime := 1000 * currentTimeInSec
+	attTimeRange := genesisTime.Add(time.Duration(attSlotRange*params.BeaconConfig().SecondsPerSlot) * time.Second)
+	currentTime := roughtime.Now()
+	clockDisparity := params.BeaconNetworkConfig().MaximumGossipClockDisparity
 
 	// Verify attestation slot is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots.
-	currentSlot := (uint64(currentTimeInSec) - genesisTime) / params.BeaconConfig().SecondsPerSlot
-	if attTime-uint64(maximumGossipClockDisparity.Milliseconds()) > uint64(currentTime) ||
-		uint64(currentTime-maximumGossipClockDisparity.Milliseconds()) > attTimeRange {
+	currentSlot := helpers.SlotsSince(genesisTime)
+	if attTime.Add(-clockDisparity).After(currentTime) ||
+		currentTime.Add(-clockDisparity).After(attTimeRange) {
 		return fmt.Errorf("attestation slot out of range %d <= %d <= %d", attSlot, currentSlot, attSlot+params.BeaconNetworkConfig().AttestationPropagationSlotRange)
 	}
 	return nil

beacon-chain/sync/validate_beacon_blocks.go

@@ -12,6 +12,7 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
 	"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"
@@ -79,7 +80,7 @@ func (r *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms
 		return pubsub.ValidationIgnore
 	}
 
-	if err := helpers.VerifySlotTime(uint64(r.chain.GenesisTime().Unix()), blk.Block.Slot, maximumGossipClockDisparity); err != nil {
+	if err := helpers.VerifySlotTime(uint64(r.chain.GenesisTime().Unix()), blk.Block.Slot, params.BeaconNetworkConfig().MaximumGossipClockDisparity); err != nil {
 		log.WithError(err).WithField("blockSlot", blk.Block.Slot).Warn("Rejecting incoming block.")
 		return pubsub.ValidationIgnore
 	}

beacon-chain/sync/validate_committee_index_beacon_attestation.go

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