diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 0100b57b0..729bbcdd5 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -191,20 +191,46 @@ func validateIndexInCommittee(ctx context.Context, bs *stateTrie.BeaconState, a return nil } -// Validates that the incoming aggregate attestation is in the desired time range. +// 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 { - // set expected boundaries for attestations attTime := genesisTime.Add(time.Duration(attSlot*params.BeaconConfig().SecondsPerSlot) * time.Second) - attSlotRange := attSlot + params.BeaconNetworkConfig().AttestationPropagationSlotRange - attTimeRange := genesisTime.Add(time.Duration(attSlotRange*params.BeaconConfig().SecondsPerSlot) * time.Second) - currentTime := roughtime.Now() + 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 - // Verify attestation slot is within the last ATTESTATION_PROPAGATION_SLOT_RANGE slots. - 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) + // 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 } diff --git a/beacon-chain/sync/validate_aggregate_proof_test.go b/beacon-chain/sync/validate_aggregate_proof_test.go index 9b4723227..87051451a 100644 --- a/beacon-chain/sync/validate_aggregate_proof_test.go +++ b/beacon-chain/sync/validate_aggregate_proof_test.go @@ -27,6 +27,7 @@ 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" ) @@ -637,3 +638,89 @@ 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) + } + }) + } +}