diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 714e5df814bb..115e083cb95c 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/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,9 +175,8 @@ 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 @@ -186,7 +184,7 @@ func validateIndexInCommittee(ctx context.Context, s *stateTrie.BeaconState, a * } if !withinCommittee { return fmt.Errorf("validator index %d is not within the committee: %v", - validatorIndex, attestingIndices) + validatorIndex, committee) } return nil } diff --git a/beacon-chain/sync/validate_aggregate_proof_test.go b/beacon-chain/sync/validate_aggregate_proof_test.go index 493e1cd4308e..35b682253457 100644 --- a/beacon-chain/sync/validate_aggregate_proof_test.go +++ b/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 := ðpb.Attestation{Data: ðpb.AttestationData{ + Target: ðpb.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() diff --git a/beacon-chain/sync/validate_beacon_attestation.go b/beacon-chain/sync/validate_beacon_attestation.go index 11b839d64207..7a14d9641f05 100644 --- a/beacon-chain/sync/validate_beacon_attestation.go +++ b/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 } diff --git a/beacon-chain/sync/validate_beacon_attestation_test.go b/beacon-chain/sync/validate_beacon_attestation_test.go index 83045cf6ea06..ba7d599286b4 100644 --- a/beacon-chain/sync/validate_beacon_attestation_test.go +++ b/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 := ðpb.SignedBeaconBlock{ Block: ðpb.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: ðpb.Attestation{ AggregationBits: bitfield.Bitlist{0b1010}, Data: ðpb.AttestationData{ BeaconBlockRoot: validBlockRoot[:], - CommitteeIndex: 1, - Slot: 63, + CommitteeIndex: 0, + Slot: 1, + Target: ðpb.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: ðpb.Attestation{ AggregationBits: bitfield.Bitlist{0b1010}, Data: ðpb.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: ðpb.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: ðpb.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: ðpb.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: ðpb.AttestationData{ BeaconBlockRoot: validBlockRoot[:], CommitteeIndex: 1, - Slot: 63, + Slot: 1, }, }, topic: fmt.Sprintf("/eth2/%x/beacon_attestation_1", digest),