diff --git a/beacon-chain/blockchain/process_attestation_helpers.go b/beacon-chain/blockchain/process_attestation_helpers.go index ba5a83964f..4069015768 100644 --- a/beacon-chain/blockchain/process_attestation_helpers.go +++ b/beacon-chain/blockchain/process_attestation_helpers.go @@ -121,11 +121,7 @@ func (s *Service) verifyAttestation(ctx context.Context, baseState *stateTrie.Be if err != nil { return nil, err } - indexedAtt, err := attestationutil.ConvertToIndexed(ctx, a, committee) - if err != nil { - return nil, errors.Wrap(err, "could not convert attestation to indexed attestation") - } - + indexedAtt := attestationutil.ConvertToIndexed(ctx, a, committee) if err := blocks.VerifyIndexedAttestation(ctx, baseState, indexedAtt); err != nil { if err == blocks.ErrSigFailedToVerify { // When sig fails to verify, check if there's a differences in committees due to diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index f7dfc41937..a8d77726f1 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -349,10 +349,7 @@ func (s *Service) insertBlockToForkChoiceStore(ctx context.Context, blk *ethpb.B if err != nil { return err } - indices, err := attestationutil.AttestingIndices(a.AggregationBits, committee) - if err != nil { - return err - } + indices := attestationutil.AttestingIndices(a.AggregationBits, committee) s.forkChoiceStore.ProcessAttestation(ctx, indices, bytesutil.ToBytes32(a.Data.BeaconBlockRoot), a.Data.Target.Epoch) } diff --git a/beacon-chain/core/blocks/block_operations.go b/beacon-chain/core/blocks/block_operations.go index 16ddc831c6..052776a945 100644 --- a/beacon-chain/core/blocks/block_operations.go +++ b/beacon-chain/core/blocks/block_operations.go @@ -856,10 +856,7 @@ func VerifyAttestation(ctx context.Context, beaconState *stateTrie.BeaconState, if err != nil { return err } - indexedAtt, err := attestationutil.ConvertToIndexed(ctx, att, committee) - if err != nil { - return errors.Wrap(err, "could not convert to indexed attestation") - } + indexedAtt := attestationutil.ConvertToIndexed(ctx, att, committee) return VerifyIndexedAttestation(ctx, beaconState, indexedAtt) } diff --git a/beacon-chain/core/blocks/block_operations_test.go b/beacon-chain/core/blocks/block_operations_test.go index aa11659692..c4a115a11a 100644 --- a/beacon-chain/core/blocks/block_operations_test.go +++ b/beacon-chain/core/blocks/block_operations_test.go @@ -943,7 +943,7 @@ func TestProcessAttestations_OK(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(att.AggregationBits, committee) if err != nil { t.Error(err) } @@ -1004,7 +1004,7 @@ func TestProcessAggregatedAttestation_OverlappingBits(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices1, err := attestationutil.AttestingIndices(att1.AggregationBits, committee) + attestingIndices1 := attestationutil.AttestingIndices(att1.AggregationBits, committee) if err != nil { t.Fatal(err) } @@ -1032,7 +1032,7 @@ func TestProcessAggregatedAttestation_OverlappingBits(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices2, err := attestationutil.AttestingIndices(att2.AggregationBits, committee) + attestingIndices2 := attestationutil.AttestingIndices(att2.AggregationBits, committee) if err != nil { t.Fatal(err) } @@ -1082,7 +1082,7 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices1, err := attestationutil.AttestingIndices(att1.AggregationBits, committee) + attestingIndices1 := attestationutil.AttestingIndices(att1.AggregationBits, committee) if err != nil { t.Fatal(err) } @@ -1109,7 +1109,7 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices2, err := attestationutil.AttestingIndices(att2.AggregationBits, committee) + attestingIndices2 := attestationutil.AttestingIndices(att2.AggregationBits, committee) if err != nil { t.Fatal(err) } @@ -1240,10 +1240,7 @@ func TestConvertToIndexed_OK(t *testing.T) { if err != nil { t.Error(err) } - ia, err := attestationutil.ConvertToIndexed(context.Background(), attestation, committee) - if err != nil { - t.Errorf("failed to convert attestation to indexed attestation: %v", err) - } + ia := attestationutil.ConvertToIndexed(context.Background(), attestation, committee) if !reflect.DeepEqual(wanted, ia) { diff, _ := messagediff.PrettyDiff(ia, wanted) t.Log(diff) diff --git a/beacon-chain/core/epoch/epoch_processing.go b/beacon-chain/core/epoch/epoch_processing.go index c3e489231c..e2347c83e7 100644 --- a/beacon-chain/core/epoch/epoch_processing.go +++ b/beacon-chain/core/epoch/epoch_processing.go @@ -336,10 +336,7 @@ func unslashedAttestingIndices(state *stateTrie.BeaconState, atts []*pb.PendingA if err != nil { return nil, err } - attestingIndices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) - if err != nil { - return nil, errors.Wrap(err, "could not get attester indices") - } + attestingIndices := attestationutil.AttestingIndices(att.AggregationBits, committee) // Create a set for attesting indices set := make([]uint64, 0, len(attestingIndices)) for _, index := range attestingIndices { diff --git a/beacon-chain/core/epoch/precompute/attestation.go b/beacon-chain/core/epoch/precompute/attestation.go index 00c3690b85..025b8ef562 100644 --- a/beacon-chain/core/epoch/precompute/attestation.go +++ b/beacon-chain/core/epoch/precompute/attestation.go @@ -47,10 +47,7 @@ func ProcessAttestations( if err != nil { return nil, nil, err } - indices, err := attestationutil.AttestingIndices(a.AggregationBits, committee) - if err != nil { - return nil, nil, err - } + indices := attestationutil.AttestingIndices(a.AggregationBits, committee) vp = UpdateValidator(vp, v, indices, a, a.Data.Slot) } diff --git a/beacon-chain/core/epoch/precompute/attestation_test.go b/beacon-chain/core/epoch/precompute/attestation_test.go index b0ab45ae39..56cc2654e8 100644 --- a/beacon-chain/core/epoch/precompute/attestation_test.go +++ b/beacon-chain/core/epoch/precompute/attestation_test.go @@ -219,7 +219,7 @@ func TestProcessAttestations(t *testing.T) { if err != nil { t.Error(err) } - indices, _ := attestationutil.AttestingIndices(att1.AggregationBits, committee) + indices := attestationutil.AttestingIndices(att1.AggregationBits, committee) for _, i := range indices { if !vp[i].IsPrevEpochAttester { t.Error("Not a prev epoch attester") @@ -229,7 +229,7 @@ func TestProcessAttestations(t *testing.T) { if err != nil { t.Error(err) } - indices, _ = attestationutil.AttestingIndices(att2.AggregationBits, committee) + indices = attestationutil.AttestingIndices(att2.AggregationBits, committee) for _, i := range indices { if !vp[i].IsPrevEpochAttester { t.Error("Not a prev epoch attester") diff --git a/beacon-chain/core/helpers/committee_test.go b/beacon-chain/core/helpers/committee_test.go index 1955c7f93f..548034fa47 100644 --- a/beacon-chain/core/helpers/committee_test.go +++ b/beacon-chain/core/helpers/committee_test.go @@ -134,7 +134,7 @@ func TestAttestationParticipants_NoCommitteeCache(t *testing.T) { if err != nil { t.Error(err) } - result, err := attestationutil.AttestingIndices(tt.bitfield, committee) + result := attestationutil.AttestingIndices(tt.bitfield, committee) if err != nil { t.Errorf("Failed to get attestation participants: %v", err) } @@ -167,7 +167,7 @@ func TestAttestationParticipants_EmptyBitfield(t *testing.T) { if err != nil { t.Fatal(err) } - indices, err := attestationutil.AttestingIndices(bitfield.NewBitlist(128), committee) + indices := attestationutil.AttestingIndices(bitfield.NewBitlist(128), committee) if err != nil { t.Fatalf("attesting indices failed: %v", err) } diff --git a/beacon-chain/core/state/transition_test.go b/beacon-chain/core/state/transition_test.go index 96a272e13e..c45697179c 100644 --- a/beacon-chain/core/state/transition_test.go +++ b/beacon-chain/core/state/transition_test.go @@ -428,7 +428,7 @@ func TestProcessBlock_PassesProcessingConditions(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices, err := attestationutil.AttestingIndices(blockAtt.AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(blockAtt.AggregationBits, committee) if err != nil { t.Error(err) } @@ -743,7 +743,7 @@ func TestProcessBlk_AttsBasedOnValidatorCount(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(att.AggregationBits, committee) if err != nil { t.Error(err) } diff --git a/beacon-chain/rpc/beacon/attestations.go b/beacon-chain/rpc/beacon/attestations.go index 9468268187..6e34370ba8 100644 --- a/beacon-chain/rpc/beacon/attestations.go +++ b/beacon-chain/rpc/beacon/attestations.go @@ -185,15 +185,7 @@ func (bs *Server) ListIndexedAttestations( continue } committee := committeesBySlot[att.Data.Slot].Committees[att.Data.CommitteeIndex] - idxAtt, err := attestationutil.ConvertToIndexed(ctx, atts[i], committee.ValidatorIndices) - if err != nil { - return nil, status.Errorf( - codes.Internal, - "Could not convert attestation with slot %d to indexed form: %v", - att.Data.Slot, - err, - ) - } + idxAtt := attestationutil.ConvertToIndexed(ctx, atts[i], committee.ValidatorIndices) indexedAtts[i] = idxAtt } @@ -311,15 +303,7 @@ func (bs *Server) StreamIndexedAttestations( continue } committee := committeesForSlot.Committees[att.Data.CommitteeIndex] - idxAtt, err := attestationutil.ConvertToIndexed(stream.Context(), att, committee.ValidatorIndices) - if err != nil { - return status.Errorf( - codes.Internal, - "Could not convert attestation with slot %d to indexed form: %v", - att.Data.Slot, - err, - ) - } + idxAtt := attestationutil.ConvertToIndexed(stream.Context(), att, committee.ValidatorIndices) if err := stream.Send(idxAtt); err != nil { return status.Errorf(codes.Unavailable, "Could not send over stream: %v", err) } diff --git a/beacon-chain/rpc/beacon/attestations_test.go b/beacon-chain/rpc/beacon/attestations_test.go index fbf9a09b86..d01979b890 100644 --- a/beacon-chain/rpc/beacon/attestations_test.go +++ b/beacon-chain/rpc/beacon/attestations_test.go @@ -600,7 +600,7 @@ func TestServer_ListIndexedAttestations_GenesisEpoch(t *testing.T) { for i := 0; i < len(indexedAtts); i++ { att := atts[i] committee := committees[att.Data.Slot].Committees[att.Data.CommitteeIndex] - idxAtt, err := attestationutil.ConvertToIndexed(ctx, atts[i], committee.ValidatorIndices) + idxAtt := attestationutil.ConvertToIndexed(ctx, atts[i], committee.ValidatorIndices) if err != nil { t.Fatalf("Could not convert attestation to indexed: %v", err) } @@ -697,7 +697,7 @@ func TestServer_ListIndexedAttestations_ArchivedEpoch(t *testing.T) { for i := 0; i < len(indexedAtts); i++ { att := atts[i] committee := committees[att.Data.Slot].Committees[att.Data.CommitteeIndex] - idxAtt, err := attestationutil.ConvertToIndexed(ctx, atts[i], committee.ValidatorIndices) + idxAtt := attestationutil.ConvertToIndexed(ctx, atts[i], committee.ValidatorIndices) if err != nil { t.Fatalf("Could not convert attestation to indexed: %v", err) } @@ -989,7 +989,7 @@ func TestServer_StreamIndexedAttestations_OK(t *testing.T) { for i := 0; i < len(indexedAtts); i++ { att := aggAtts[i] committee := committees[att.Data.Slot].Committees[att.Data.CommitteeIndex] - idxAtt, err := attestationutil.ConvertToIndexed(ctx, att, committee.ValidatorIndices) + idxAtt := attestationutil.ConvertToIndexed(ctx, att, committee.ValidatorIndices) if err != nil { t.Fatalf("Could not convert attestation to indexed: %v", err) } diff --git a/beacon-chain/rpc/validator/aggregator_test.go b/beacon-chain/rpc/validator/aggregator_test.go index 8ad0c0bbe6..6757bed8f6 100644 --- a/beacon-chain/rpc/validator/aggregator_test.go +++ b/beacon-chain/rpc/validator/aggregator_test.go @@ -221,7 +221,7 @@ func generateAtt(state *beaconstate.BeaconState, index uint64, privKeys []*bls.S AggregationBits: aggBits, } committee, _ := helpers.BeaconCommitteeFromState(state, att.Data.Slot, att.Data.CommitteeIndex) - attestingIndices, _ := attestationutil.AttestingIndices(att.AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(att.AggregationBits, committee) domain, err := helpers.Domain(state.Fork(), 0, params.BeaconConfig().DomainBeaconAttester) if err != nil { return nil, err diff --git a/beacon-chain/rpc/validator/proposer_test.go b/beacon-chain/rpc/validator/proposer_test.go index f017ad618b..490f2f6a5b 100644 --- a/beacon-chain/rpc/validator/proposer_test.go +++ b/beacon-chain/rpc/validator/proposer_test.go @@ -1307,7 +1307,7 @@ func TestFilterAttestation_OK(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices, err := attestationutil.AttestingIndices(atts[i].AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(atts[i].AggregationBits, committee) if err != nil { t.Error(err) } diff --git a/beacon-chain/sync/pending_attestations_queue_test.go b/beacon-chain/sync/pending_attestations_queue_test.go index 4635c29da8..9c9dad25b9 100644 --- a/beacon-chain/sync/pending_attestations_queue_test.go +++ b/beacon-chain/sync/pending_attestations_queue_test.go @@ -128,7 +128,7 @@ func TestProcessPendingAtts_HasBlockSaveAggregatedAtt(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(att.AggregationBits, committee) if err != nil { t.Error(err) } diff --git a/beacon-chain/sync/validate_aggregate_proof.go b/beacon-chain/sync/validate_aggregate_proof.go index 0c9d325a0a..44aa027691 100644 --- a/beacon-chain/sync/validate_aggregate_proof.go +++ b/beacon-chain/sync/validate_aggregate_proof.go @@ -148,10 +148,7 @@ func validateIndexInCommittee(ctx context.Context, s *stateTrie.BeaconState, a * if err != nil { return err } - attestingIndices, err := attestationutil.AttestingIndices(a.AggregationBits, committee) - if err != nil { - return err - } + attestingIndices := attestationutil.AttestingIndices(a.AggregationBits, committee) var withinCommittee bool for _, i := range attestingIndices { if validatorIndex == i { diff --git a/beacon-chain/sync/validate_aggregate_proof_test.go b/beacon-chain/sync/validate_aggregate_proof_test.go index 4598acee10..88f4d4f9b5 100644 --- a/beacon-chain/sync/validate_aggregate_proof_test.go +++ b/beacon-chain/sync/validate_aggregate_proof_test.go @@ -48,7 +48,7 @@ func TestVerifyIndexInCommittee_CanVerify(t *testing.T) { if err != nil { t.Error(err) } - indices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) + indices := attestationutil.AttestingIndices(att.AggregationBits, committee) if err != nil { t.Fatal(err) } @@ -330,7 +330,7 @@ func TestValidateAggregateAndProof_CanValidate(t *testing.T) { if err != nil { t.Error(err) } - attestingIndices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) + attestingIndices := attestationutil.AttestingIndices(att.AggregationBits, committee) if err != nil { t.Error(err) } diff --git a/shared/attestationutil/BUILD.bazel b/shared/attestationutil/BUILD.bazel index d50b8b878f..311345ce33 100644 --- a/shared/attestationutil/BUILD.bazel +++ b/shared/attestationutil/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -6,9 +6,15 @@ go_library( importpath = "github.com/prysmaticlabs/prysm/shared/attestationutil", visibility = ["//visibility:public"], deps = [ - "@com_github_pkg_errors//:go_default_library", "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", "@io_opencensus_go//trace:go_default_library", ], ) + +go_test( + name = "go_default_test", + srcs = ["attestation_utils_test.go"], + embed = [":go_default_library"], + deps = ["@com_github_prysmaticlabs_go_bitfield//:go_default_library"], +) diff --git a/shared/attestationutil/attestation_utils.go b/shared/attestationutil/attestation_utils.go index 0d95bb36d3..72de7278a5 100644 --- a/shared/attestationutil/attestation_utils.go +++ b/shared/attestationutil/attestation_utils.go @@ -4,7 +4,6 @@ import ( "context" "sort" - "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/go-bitfield" "go.opencensus.io/trace" @@ -28,14 +27,11 @@ import ( // data=attestation.data, // signature=attestation.signature, // ) -func ConvertToIndexed(ctx context.Context, attestation *ethpb.Attestation, committee []uint64) (*ethpb.IndexedAttestation, error) { +func ConvertToIndexed(ctx context.Context, attestation *ethpb.Attestation, committee []uint64) *ethpb.IndexedAttestation { ctx, span := trace.StartSpan(ctx, "core.ConvertToIndexed") defer span.End() - attIndices, err := AttestingIndices(attestation.AggregationBits, committee) - if err != nil { - return nil, errors.Wrap(err, "could not get attesting indices") - } + attIndices := AttestingIndices(attestation.AggregationBits, committee) sort.Slice(attIndices, func(i, j int) bool { return attIndices[i] < attIndices[j] @@ -45,7 +41,7 @@ func ConvertToIndexed(ctx context.Context, attestation *ethpb.Attestation, commi Signature: attestation.Signature, AttestingIndices: attIndices, } - return inAtt, nil + return inAtt } // AttestingIndices returns the attesting participants indices from the attestation data. The @@ -61,7 +57,7 @@ func ConvertToIndexed(ctx context.Context, attestation *ethpb.Attestation, commi // """ // committee = get_beacon_committee(state, data.slot, data.index) // return set(index for i, index in enumerate(committee) if bits[i]) -func AttestingIndices(bf bitfield.Bitfield, committee []uint64) ([]uint64, error) { +func AttestingIndices(bf bitfield.Bitfield, committee []uint64) []uint64 { indices := make([]uint64, 0, len(committee)) indicesSet := make(map[uint64]bool, len(committee)) for i, idx := range committee { @@ -72,5 +68,5 @@ func AttestingIndices(bf bitfield.Bitfield, committee []uint64) ([]uint64, error } indicesSet[idx] = true } - return indices, nil + return indices } diff --git a/shared/attestationutil/attestation_utils_test.go b/shared/attestationutil/attestation_utils_test.go new file mode 100644 index 0000000000..e041414303 --- /dev/null +++ b/shared/attestationutil/attestation_utils_test.go @@ -0,0 +1,46 @@ +package attestationutil_test + +import ( + "reflect" + "testing" + + "github.com/prysmaticlabs/go-bitfield" + "github.com/prysmaticlabs/prysm/shared/attestationutil" +) + +func TestAttestingIndices(t *testing.T) { + type args struct { + bf bitfield.Bitfield + committee []uint64 + } + tests := []struct { + name string + args args + want []uint64 + }{ + { + name: "Full committee attested", + args: args{ + bf: bitfield.Bitlist{0b1111}, + committee: []uint64{0, 1, 2}, + }, + want: []uint64{0, 1, 2}, + }, + { + name: "Partial committee attested", + args: args{ + bf: bitfield.Bitlist{0b1101}, + committee: []uint64{0, 1, 2}, + }, + want: []uint64{0, 2}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := attestationutil.AttestingIndices(tt.args.bf, tt.args.committee) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("AttestingIndices() got = %v, want %v", got, tt.want) + } + }) + } +} \ No newline at end of file diff --git a/tools/blocktree/main.go b/tools/blocktree/main.go index edac503053..11c00f351c 100644 --- a/tools/blocktree/main.go +++ b/tools/blocktree/main.go @@ -100,10 +100,7 @@ func main() { if err != nil { panic(err) } - indices, err := attestationutil.AttestingIndices(att.AggregationBits, committee) - if err != nil { - panic(err) - } + indices := attestationutil.AttestingIndices(att.AggregationBits, committee) for _, i := range indices { m[r].score[i] = true }