id comments and tests

proto/prysm/v1alpha1/attestation/BUILD.bazel

@@ -24,7 +24,10 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["attestation_utils_test.go"],
+    srcs = [
+        "attestation_utils_test.go",
+        "id_test.go",
+    ],
     deps = [
         ":go_default_library",
         "//config/fieldparams:go_default_library",
@@ -33,6 +36,7 @@ go_test(
         "//proto/prysm/v1alpha1:go_default_library",
         "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
+        "//testing/util:go_default_library",
         "@com_github_prysmaticlabs_go_bitfield//:go_default_library",
     ],
 )

proto/prysm/v1alpha1/attestation/id.go

@@ -1,6 +1,7 @@
 package attestation
 
 import (
+	"fmt"
 	"strconv"
 
 	"github.com/pkg/errors"
@@ -10,51 +11,57 @@ import (
 	"github.com/prysmaticlabs/prysm/v5/runtime/version"
 )
 
-type DigestSource uint8
+// IdSource represents the part of attestation that will be used to generate the Id.
+type IdSource uint8
 
 const (
-	Full DigestSource = iota
+	// Full generates the Id from the whole attestation.
+	Full IdSource = iota
+	// Data generates the Id from the tuple (slot, committee index, beacon block root, source, target).
 	Data
 )
 
+// Id represents an attestation ID. Its uniqueness depends on the IdSource provided when constructing the Id.
 type Id struct {
 	version int
-	digest  [32]byte
+	hash    [32]byte
 }
 
-func NewId(att ethpb.Att, digest DigestSource) (Id, error) {
+// NewId --
+func NewId(att ethpb.Att, source IdSource) (Id, error) {
 	if err := helpers.ValidateNilAttestation(att); err != nil {
 		return Id{}, err
 	}
 
-	switch digest {
+	switch source {
 	case Full:
-		d, err := att.HashTreeRoot()
+		h, err := att.HashTreeRoot()
 		if err != nil {
 			return Id{}, err
 		}
-		return Id{version: att.Version(), digest: d}, nil
+		return Id{version: att.Version(), hash: h}, nil
 	case Data:
 		data := att.GetData()
 		if att.Version() >= version.Electra {
-			cb := att.CommitteeBitsVal().BitIndices()
-			if len(cb) == 0 {
-				return Id{}, errors.New("no committee bits set")
+			committeeIndices := att.CommitteeBitsVal().BitIndices()
+			if len(committeeIndices) != 1 {
+				return Id{}, fmt.Errorf("%d committee bits are set instead of 1", len(committeeIndices))
 			}
 			dataCopy := ethpb.CopyAttestationData(att.GetData())
-			dataCopy.CommitteeIndex = primitives.CommitteeIndex(cb[0])
+			dataCopy.CommitteeIndex = primitives.CommitteeIndex(committeeIndices[0])
 			data = dataCopy
 		}
-		d, err := data.HashTreeRoot()
+		h, err := data.HashTreeRoot()
 		if err != nil {
 			return Id{}, err
 		}
-		return Id{version: att.Version(), digest: d}, nil
+		return Id{version: att.Version(), hash: h}, nil
 	default:
-		return Id{}, errors.New("invalid digest requested")
+		return Id{}, errors.New("invalid source requested")
 	}
 }
 
+// String --
 func (id Id) String() string {
-	return strconv.Itoa(id.version) + string(id.digest[:])
+	return strconv.Itoa(id.version) + string(id.hash[:])
 }

proto/prysm/v1alpha1/attestation/id_test.go

@@ -0,0 +1,63 @@
+package attestation_test
+
+import (
+	"testing"
+
+	"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
+	ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
+	"github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1/attestation"
+	"github.com/prysmaticlabs/prysm/v5/testing/assert"
+	"github.com/prysmaticlabs/prysm/v5/testing/require"
+	"github.com/prysmaticlabs/prysm/v5/testing/util"
+)
+
+func TestNewId(t *testing.T) {
+	t.Run("full source", func(t *testing.T) {
+		att := util.HydrateAttestation(&ethpb.Attestation{})
+		_, err := attestation.NewId(att, attestation.Full)
+		assert.NoError(t, err)
+	})
+	t.Run("data source Phase 0", func(t *testing.T) {
+		att := util.HydrateAttestation(&ethpb.Attestation{})
+		_, err := attestation.NewId(att, attestation.Data)
+		assert.NoError(t, err)
+	})
+	t.Run("data source Electra", func(t *testing.T) {
+		cb := primitives.NewAttestationCommitteeBits()
+		cb.SetBitAt(0, true)
+		att := util.HydrateAttestationElectra(&ethpb.AttestationElectra{CommitteeBits: cb})
+		_, err := attestation.NewId(att, attestation.Data)
+		assert.NoError(t, err)
+	})
+	t.Run("ID is different between versions", func(t *testing.T) {
+		phase0Att := util.HydrateAttestation(&ethpb.Attestation{})
+		phase0Id, err := attestation.NewId(phase0Att, attestation.Data)
+		require.NoError(t, err)
+		cb := primitives.NewAttestationCommitteeBits()
+		cb.SetBitAt(0, true) // setting committee bit 0 for Electra corresponds to attestation data's committee index 0 for Phase 0
+		electraAtt := util.HydrateAttestationElectra(&ethpb.AttestationElectra{CommitteeBits: cb})
+		electraId, err := attestation.NewId(electraAtt, attestation.Data)
+		require.NoError(t, err)
+
+		assert.NotEqual(t, phase0Id, electraId)
+	})
+	t.Run("invalid source", func(t *testing.T) {
+		att := util.HydrateAttestation(&ethpb.Attestation{})
+		_, err := attestation.NewId(att, 123)
+		assert.ErrorContains(t, "invalid source requested", err)
+	})
+	t.Run("data source Electra - 0 bits set", func(t *testing.T) {
+		cb := primitives.NewAttestationCommitteeBits()
+		att := util.HydrateAttestationElectra(&ethpb.AttestationElectra{CommitteeBits: cb})
+		_, err := attestation.NewId(att, attestation.Data)
+		assert.ErrorContains(t, "0 committee bits are set", err)
+	})
+	t.Run("data source Electra - multiple bits set", func(t *testing.T) {
+		cb := primitives.NewAttestationCommitteeBits()
+		cb.SetBitAt(0, true)
+		cb.SetBitAt(1, true)
+		att := util.HydrateAttestationElectra(&ethpb.AttestationElectra{CommitteeBits: cb})
+		_, err := attestation.NewId(att, attestation.Data)
+		assert.ErrorContains(t, "2 committee bits are set", err)
+	})
+}