From 8f64eb622e6b47f478910df7571bb916569575aa Mon Sep 17 00:00:00 2001
From: terence tsao <terence@prysmaticlabs.com>
Date: Thu, 8 Oct 2020 16:20:18 -0700
Subject: [PATCH] Optimize `IsValidAttestationIndices` unique sorted indices
 check (#7458)

* Optimize unique sorted indices check

* Add err handling for benchmark

* Gazelle

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
---
 shared/attestationutil/BUILD.bazel            |  1 +
 shared/attestationutil/attestation_utils.go   | 17 ++------
 .../attestationutil/attestation_utils_test.go | 41 +++++++++++++++++++
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/shared/attestationutil/BUILD.bazel b/shared/attestationutil/BUILD.bazel
index 6845502c8..612db677e 100644
--- a/shared/attestationutil/BUILD.bazel
+++ b/shared/attestationutil/BUILD.bazel
@@ -24,6 +24,7 @@ go_test(
     deps = [
         "//shared/params:go_default_library",
         "//shared/testutil/assert:go_default_library",
+        "//shared/testutil/require:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
         "@com_github_prysmaticlabs_go_bitfield//:go_default_library",
     ],
diff --git a/shared/attestationutil/attestation_utils.go b/shared/attestationutil/attestation_utils.go
index 98bf774f8..4a7349caf 100644
--- a/shared/attestationutil/attestation_utils.go
+++ b/shared/attestationutil/attestation_utils.go
@@ -6,7 +6,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"reflect"
 	"sort"
 
 	"github.com/pkg/errors"
@@ -154,20 +153,10 @@ func IsValidAttestationIndices(ctx context.Context, indexedAttestation *ethpb.In
 	if uint64(len(indices)) > params.BeaconConfig().MaxValidatorsPerCommittee {
 		return fmt.Errorf("validator indices count exceeds MAX_VALIDATORS_PER_COMMITTEE, %d > %d", len(indices), params.BeaconConfig().MaxValidatorsPerCommittee)
 	}
-	set := make(map[uint64]bool, len(indices))
-	setIndices := make([]uint64, 0, len(indices))
-	for _, i := range indices {
-		if ok := set[i]; ok {
-			continue
+	for i := 1; i < len(indices); i++ {
+		if indices[i-1] >= indices[i] {
+			return errors.New("attesting indices is not uniquely sorted")
 		}
-		setIndices = append(setIndices, i)
-		set[i] = true
-	}
-	sort.SliceStable(setIndices, func(i, j int) bool {
-		return setIndices[i] < setIndices[j]
-	})
-	if !reflect.DeepEqual(setIndices, indices) {
-		return errors.New("attesting indices is not uniquely sorted")
 	}
 	return nil
 }
diff --git a/shared/attestationutil/attestation_utils_test.go b/shared/attestationutil/attestation_utils_test.go
index dd9ecdcc7..9a9e3bcec 100644
--- a/shared/attestationutil/attestation_utils_test.go
+++ b/shared/attestationutil/attestation_utils_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/prysmaticlabs/prysm/shared/attestationutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/testutil/assert"
+	"github.com/prysmaticlabs/prysm/shared/testutil/require"
 )
 
 func TestAttestingIndices(t *testing.T) {
@@ -95,6 +96,26 @@ func TestIsValidAttestationIndices(t *testing.T) {
 				Signature: make([]byte, 96),
 			},
 		},
+		{
+			name: "Valid indices with length of 2",
+			att: &eth.IndexedAttestation{
+				AttestingIndices: []uint64{1, 2},
+				Data: &eth.AttestationData{
+					Target: &eth.Checkpoint{},
+				},
+				Signature: make([]byte, 96),
+			},
+		},
+		{
+			name: "Valid indices with length of 1",
+			att: &eth.IndexedAttestation{
+				AttestingIndices: []uint64{1},
+				Data: &eth.AttestationData{
+					Target: &eth.Checkpoint{},
+				},
+				Signature: make([]byte, 96),
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -118,6 +139,26 @@ func BenchmarkAttestingIndices_PartialCommittee(b *testing.B) {
 	}
 }
 
+func BenchmarkIsValidAttestationIndices(b *testing.B) {
+	indices := make([]uint64, params.BeaconConfig().MaxValidatorsPerCommittee)
+	for i := 0; i < len(indices); i++ {
+		indices[i] = uint64(i)
+	}
+	att := &eth.IndexedAttestation{
+		AttestingIndices: indices,
+		Data: &eth.AttestationData{
+			Target: &eth.Checkpoint{},
+		},
+		Signature: make([]byte, 96),
+	}
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		if err := attestationutil.IsValidAttestationIndices(context.Background(), att); err != nil {
+			require.NoError(b, err)
+		}
+	}
+}
+
 func TestAttDataIsEqual(t *testing.T) {
 	type test struct {
 		name     string