Separate type for unaggregated network attestations (#14659)

* definitions and gossip

* validator

* broadcast

* broadcast the correct att depending on version

* small updates

* don't check bits after Electra

* nitpick

* tests

* changelog <3

* review

* more review

* review yet again

* try a different design

* fix gossip issues

* cleanup

* tests

* reduce cognitive complexity

* Preston's review

* move changelog entry to unreleased section

* fix pending atts pool issues

* reviews

* Potuz's comments

* test fixes

validator/client/attest.go

@@ -122,23 +122,6 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot primitives.Slot,
 		return
 	}
 
-	var indexInCommittee uint64
-	var found bool
-	for i, vID := range duty.Committee {
-		if vID == duty.ValidatorIndex {
-			indexInCommittee = uint64(i)
-			found = true
-			break
-		}
-	}
-	if !found {
-		log.Errorf("Validator ID %d not found in committee of %v", duty.ValidatorIndex, duty.Committee)
-		if v.emitAccountMetrics {
-			ValidatorAttestFailVec.WithLabelValues(fmtKey).Inc()
-		}
-		return
-	}
-
 	// TODO: Extend to Electra
 	phase0Att, ok := indexedAtt.(*ethpb.IndexedAttestation)
 	if ok {
@@ -153,21 +136,36 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot primitives.Slot,
 		}
 	}
 
-	aggregationBitfield := bitfield.NewBitlist(uint64(len(duty.Committee)))
-	aggregationBitfield.SetBitAt(indexInCommittee, true)
-	committeeBits := primitives.NewAttestationCommitteeBits()
+	var aggregationBitfield bitfield.Bitlist
 
 	var attResp *ethpb.AttestResponse
 	if postElectra {
-		attestation := &ethpb.AttestationElectra{
-			Data:            data,
-			AggregationBits: aggregationBitfield,
-			CommitteeBits:   committeeBits,
-			Signature:       sig,
+		attestation := &ethpb.SingleAttestation{
+			Data:          data,
+			AttesterIndex: duty.ValidatorIndex,
+			CommitteeId:   duty.CommitteeIndex,
+			Signature:     sig,
 		}
-		attestation.CommitteeBits.SetBitAt(uint64(req.CommitteeIndex), true)
 		attResp, err = v.validatorClient.ProposeAttestationElectra(ctx, attestation)
 	} else {
+		var indexInCommittee uint64
+		var found bool
+		for i, vID := range duty.Committee {
+			if vID == duty.ValidatorIndex {
+				indexInCommittee = uint64(i)
+				found = true
+				break
+			}
+		}
+		if !found {
+			log.Errorf("Validator ID %d not found in committee of %v", duty.ValidatorIndex, duty.Committee)
+			if v.emitAccountMetrics {
+				ValidatorAttestFailVec.WithLabelValues(fmtKey).Inc()
+			}
+			return
+		}
+		aggregationBitfield = bitfield.NewBitlist(uint64(len(duty.Committee)))
+		aggregationBitfield.SetBitAt(indexInCommittee, true)
 		attestation := &ethpb.Attestation{
 			Data:            data,
 			AggregationBits: aggregationBitfield,
@@ -199,11 +197,12 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot primitives.Slot,
 		trace.StringAttribute("blockRoot", fmt.Sprintf("%#x", data.BeaconBlockRoot)),
 		trace.Int64Attribute("justifiedEpoch", int64(data.Source.Epoch)),
 		trace.Int64Attribute("targetEpoch", int64(data.Target.Epoch)),
-		trace.StringAttribute("aggregationBitfield", fmt.Sprintf("%#x", aggregationBitfield)),
 	)
 	if postElectra {
-		span.SetAttributes(trace.StringAttribute("committeeBitfield", fmt.Sprintf("%#x", committeeBits)))
+		span.SetAttributes(trace.Int64Attribute("attesterIndex", int64(duty.ValidatorIndex)))
+		span.SetAttributes(trace.Int64Attribute("committeeIndex", int64(duty.CommitteeIndex)))
 	} else {
+		span.SetAttributes(trace.StringAttribute("aggregationBitfield", fmt.Sprintf("%#x", aggregationBitfield)))
 		span.SetAttributes(trace.Int64Attribute("committeeIndex", int64(data.CommitteeIndex)))
 	}
 

validator/client/attest_test.go

@@ -222,11 +222,11 @@ func TestAttestToBlockHead_AttestsCorrectly(t *testing.T) {
 				gomock.Any(), // epoch
 			).Times(2).Return(&ethpb.DomainResponse{SignatureDomain: make([]byte, 32)}, nil /*err*/)
 
-			var generatedAttestation *ethpb.AttestationElectra
+			var generatedAttestation *ethpb.SingleAttestation
 			m.validatorClient.EXPECT().ProposeAttestationElectra(
 				gomock.Any(), // ctx
-				gomock.AssignableToTypeOf(&ethpb.AttestationElectra{}),
-			).Do(func(_ context.Context, att *ethpb.AttestationElectra) {
+				gomock.AssignableToTypeOf(&ethpb.SingleAttestation{}),
+			).Do(func(_ context.Context, att *ethpb.SingleAttestation) {
 				generatedAttestation = att
 			}).Return(&ethpb.AttestResponse{}, nil /* error */)
 
@@ -236,15 +236,15 @@ func TestAttestToBlockHead_AttestsCorrectly(t *testing.T) {
 			aggregationBitfield.SetBitAt(4, true)
 			committeeBits := primitives.NewAttestationCommitteeBits()
 			committeeBits.SetBitAt(5, true)
-			expectedAttestation := &ethpb.AttestationElectra{
+			expectedAttestation := &ethpb.SingleAttestation{
 				Data: &ethpb.AttestationData{
 					BeaconBlockRoot: beaconBlockRoot[:],
 					Target:          &ethpb.Checkpoint{Root: targetRoot[:]},
 					Source:          &ethpb.Checkpoint{Root: sourceRoot[:], Epoch: 3},
 				},
-				AggregationBits: aggregationBitfield,
-				CommitteeBits:   committeeBits,
-				Signature:       make([]byte, 96),
+				AttesterIndex: validatorIndex,
+				CommitteeId:   5,
+				Signature:     make([]byte, 96),
 			}
 
 			root, err := signing.ComputeSigningRoot(expectedAttestation.Data, make([]byte, 32))

validator/client/beacon-api/BUILD.bazel

@@ -122,6 +122,7 @@ go_test(
     deps = [
         "//api:go_default_library",
         "//api/server/structs:go_default_library",
+        "//beacon-chain/core/helpers:go_default_library",
         "//beacon-chain/rpc/eth/shared/testing:go_default_library",
         "//config/params:go_default_library",
         "//consensus-types/primitives:go_default_library",

validator/client/beacon-api/beacon_api_validator_client.go

@@ -154,7 +154,7 @@ func (c *beaconApiValidatorClient) ProposeAttestation(ctx context.Context, in *e
 	})
 }
 
-func (c *beaconApiValidatorClient) ProposeAttestationElectra(ctx context.Context, in *ethpb.AttestationElectra) (*ethpb.AttestResponse, error) {
+func (c *beaconApiValidatorClient) ProposeAttestationElectra(ctx context.Context, in *ethpb.SingleAttestation) (*ethpb.AttestResponse, error) {
 	ctx, span := trace.StartSpan(ctx, "beacon-api.ProposeAttestationElectra")
 	defer span.End()
 

validator/client/beacon-api/beacon_block_json_helpers.go

@@ -51,10 +51,10 @@ func jsonifyAttestations(attestations []*ethpb.Attestation) []*structs.Attestati
 	return jsonAttestations
 }
 
-func jsonifyAttestationsElectra(attestations []*ethpb.AttestationElectra) []*structs.AttestationElectra {
-	jsonAttestations := make([]*structs.AttestationElectra, len(attestations))
+func jsonifySingleAttestations(attestations []*ethpb.SingleAttestation) []*structs.SingleAttestation {
+	jsonAttestations := make([]*structs.SingleAttestation, len(attestations))
 	for index, attestation := range attestations {
-		jsonAttestations[index] = jsonifyAttestationElectra(attestation)
+		jsonAttestations[index] = jsonifySingleAttestation(attestation)
 	}
 	return jsonAttestations
 }
@@ -181,6 +181,15 @@ func jsonifyAttestationElectra(attestation *ethpb.AttestationElectra) *structs.A
 	}
 }
 
+func jsonifySingleAttestation(attestation *ethpb.SingleAttestation) *structs.SingleAttestation {
+	return &structs.SingleAttestation{
+		CommitteeIndex: uint64ToString(attestation.CommitteeId),
+		AttesterIndex:  uint64ToString(attestation.AttesterIndex),
+		Data:           jsonifyAttestationData(attestation.Data),
+		Signature:      hexutil.Encode(attestation.Signature),
+	}
+}
+
 func jsonifySignedAggregateAndProof(signedAggregateAndProof *ethpb.SignedAggregateAttestationAndProof) *structs.SignedAggregateAttestationAndProof {
 	return &structs.SignedAggregateAttestationAndProof{
 		Message: &structs.AggregateAttestationAndProof{

validator/client/beacon-api/mock/json_rest_handler_mock.go

@@ -58,7 +58,7 @@ func (mr *MockJsonRestHandlerMockRecorder) Get(ctx, endpoint, resp any) *gomock.
 // Host mocks base method.
 func (m *MockJsonRestHandler) Host() string {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "HTTPHost")
+	ret := m.ctrl.Call(m, "Host")
 	ret0, _ := ret[0].(string)
 	return ret0
 }
@@ -66,7 +66,7 @@ func (m *MockJsonRestHandler) Host() string {
 // Host indicates an expected call of Host.
 func (mr *MockJsonRestHandlerMockRecorder) Host() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HTTPHost", reflect.TypeOf((*MockJsonRestHandler)(nil).Host))
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Host", reflect.TypeOf((*MockJsonRestHandler)(nil).Host))
 }
 
 // HttpClient mocks base method.

validator/client/beacon-api/propose_attestation.go

@@ -7,13 +7,14 @@ import (
 	"net/http"
 
 	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/v5/network/httputil"
 	ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/v5/runtime/version"
 )
 
 func (c *beaconApiValidatorClient) proposeAttestation(ctx context.Context, attestation *ethpb.Attestation) (*ethpb.AttestResponse, error) {
-	if err := validateNilAttestation(attestation); err != nil {
+	if err := helpers.ValidateNilAttestation(attestation); err != nil {
 		return nil, err
 	}
 	marshalledAttestation, err := json.Marshal(jsonifyAttestations([]*ethpb.Attestation{attestation}))
@@ -58,11 +59,11 @@ func (c *beaconApiValidatorClient) proposeAttestation(ctx context.Context, attes
 	return &ethpb.AttestResponse{AttestationDataRoot: attestationDataRoot[:]}, nil
 }
 
-func (c *beaconApiValidatorClient) proposeAttestationElectra(ctx context.Context, attestation *ethpb.AttestationElectra) (*ethpb.AttestResponse, error) {
-	if err := validateNilAttestation(attestation); err != nil {
+func (c *beaconApiValidatorClient) proposeAttestationElectra(ctx context.Context, attestation *ethpb.SingleAttestation) (*ethpb.AttestResponse, error) {
+	if err := helpers.ValidateNilAttestation(attestation); err != nil {
 		return nil, err
 	}
-	marshalledAttestation, err := json.Marshal(jsonifyAttestationsElectra([]*ethpb.AttestationElectra{attestation}))
+	marshalledAttestation, err := json.Marshal(jsonifySingleAttestations([]*ethpb.SingleAttestation{attestation}))
 	if err != nil {
 		return nil, err
 	}
@@ -84,28 +85,3 @@ func (c *beaconApiValidatorClient) proposeAttestationElectra(ctx context.Context
 
 	return &ethpb.AttestResponse{AttestationDataRoot: attestationDataRoot[:]}, nil
 }
-
-func validateNilAttestation(attestation ethpb.Att) error {
-	if attestation == nil || attestation.IsNil() {
-		return errors.New("attestation can't be nil")
-	}
-	if attestation.GetData().Source == nil {
-		return errors.New("attestation's source can't be nil")
-	}
-	if attestation.GetData().Target == nil {
-		return errors.New("attestation's target can't be nil")
-	}
-	v := attestation.Version()
-	if len(attestation.GetAggregationBits()) == 0 {
-		return errors.New("attestation's bitfield can't be nil")
-	}
-	if len(attestation.GetSignature()) == 0 {
-		return errors.New("attestation signature can't be nil")
-	}
-	if v >= version.Electra {
-		if len(attestation.CommitteeBitsVal().BitIndices()) == 0 {
-			return errors.New("attestation committee bits can't be nil")
-		}
-	}
-	return nil
-}

validator/client/beacon-api/propose_attestation_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/v5/network/httputil"
 	ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/v5/runtime/version"
@@ -51,7 +52,7 @@ func TestProposeAttestation(t *testing.T) {
 		},
 		{
 			name:                 "nil attestation",
-			expectedErrorMessage: "attestation can't be nil",
+			expectedErrorMessage: "attestation is nil",
 		},
 		{
 			name: "nil attestation data",
@@ -59,7 +60,7 @@ func TestProposeAttestation(t *testing.T) {
 				AggregationBits: testhelpers.FillByteSlice(4, 74),
 				Signature:       testhelpers.FillByteSlice(96, 82),
 			},
-			expectedErrorMessage: "attestation can't be nil",
+			expectedErrorMessage: "attestation is nil",
 		},
 		{
 			name: "nil source checkpoint",
@@ -94,17 +95,6 @@ func TestProposeAttestation(t *testing.T) {
 			},
 			expectedErrorMessage: "attestation's bitfield can't be nil",
 		},
-		{
-			name: "nil signature",
-			attestation: &ethpb.Attestation{
-				AggregationBits: testhelpers.FillByteSlice(4, 74),
-				Data: &ethpb.AttestationData{
-					Source: &ethpb.Checkpoint{},
-					Target: &ethpb.Checkpoint{},
-				},
-			},
-			expectedErrorMessage: "attestation signature can't be nil",
-		},
 		{
 			name:                 "bad request",
 			attestation:          attestation,
@@ -120,7 +110,7 @@ func TestProposeAttestation(t *testing.T) {
 			jsonRestHandler := mock.NewMockJsonRestHandler(ctrl)
 
 			var marshalledAttestations []byte
-			if validateNilAttestation(test.attestation) == nil {
+			if helpers.ValidateNilAttestation(test.attestation) == nil {
 				b, err := json.Marshal(jsonifyAttestations([]*ethpb.Attestation{test.attestation}))
 				require.NoError(t, err)
 				marshalledAttestations = b
@@ -181,7 +171,7 @@ func TestProposeAttestationFallBack(t *testing.T) {
 	jsonRestHandler := mock.NewMockJsonRestHandler(ctrl)
 
 	var marshalledAttestations []byte
-	if validateNilAttestation(attestation) == nil {
+	if helpers.ValidateNilAttestation(attestation) == nil {
 		b, err := json.Marshal(jsonifyAttestations([]*ethpb.Attestation{attestation}))
 		require.NoError(t, err)
 		marshalledAttestations = b
@@ -225,8 +215,8 @@ func TestProposeAttestationFallBack(t *testing.T) {
 }
 
 func TestProposeAttestationElectra(t *testing.T) {
-	attestation := &ethpb.AttestationElectra{
-		AggregationBits: testhelpers.FillByteSlice(4, 74),
+	attestation := &ethpb.SingleAttestation{
+		AttesterIndex: 74,
 		Data: &ethpb.AttestationData{
 			Slot:            75,
 			CommitteeIndex:  76,
@@ -240,13 +230,13 @@ func TestProposeAttestationElectra(t *testing.T) {
 				Root:  testhelpers.FillByteSlice(32, 81),
 			},
 		},
-		Signature:     testhelpers.FillByteSlice(96, 82),
-		CommitteeBits: testhelpers.FillByteSlice(8, 83),
+		Signature:   testhelpers.FillByteSlice(96, 82),
+		CommitteeId: 83,
 	}
 
 	tests := []struct {
 		name                 string
-		attestation          *ethpb.AttestationElectra
+		attestation          *ethpb.SingleAttestation
 		expectedErrorMessage string
 		endpointError        error
 		endpointCall         int
@@ -258,86 +248,41 @@ func TestProposeAttestationElectra(t *testing.T) {
 		},
 		{
 			name:                 "nil attestation",
-			expectedErrorMessage: "attestation can't be nil",
+			expectedErrorMessage: "attestation is nil",
 		},
 		{
 			name: "nil attestation data",
-			attestation: &ethpb.AttestationElectra{
-				AggregationBits: testhelpers.FillByteSlice(4, 74),
-				Signature:       testhelpers.FillByteSlice(96, 82),
-				CommitteeBits:   testhelpers.FillByteSlice(8, 83),
+			attestation: &ethpb.SingleAttestation{
+				AttesterIndex: 74,
+				Signature:     testhelpers.FillByteSlice(96, 82),
+				CommitteeId:   83,
 			},
-			expectedErrorMessage: "attestation can't be nil",
+			expectedErrorMessage: "attestation is nil",
 		},
 		{
 			name: "nil source checkpoint",
-			attestation: &ethpb.AttestationElectra{
-				AggregationBits: testhelpers.FillByteSlice(4, 74),
+			attestation: &ethpb.SingleAttestation{
+				AttesterIndex: 74,
 				Data: &ethpb.AttestationData{
 					Target: &ethpb.Checkpoint{},
 				},
-				Signature:     testhelpers.FillByteSlice(96, 82),
-				CommitteeBits: testhelpers.FillByteSlice(8, 83),
+				Signature:   testhelpers.FillByteSlice(96, 82),
+				CommitteeId: 83,
 			},
 			expectedErrorMessage: "attestation's source can't be nil",
 		},
 		{
 			name: "nil target checkpoint",
-			attestation: &ethpb.AttestationElectra{
-				AggregationBits: testhelpers.FillByteSlice(4, 74),
+			attestation: &ethpb.SingleAttestation{
+				AttesterIndex: 74,
 				Data: &ethpb.AttestationData{
 					Source: &ethpb.Checkpoint{},
 				},
-				Signature:     testhelpers.FillByteSlice(96, 82),
-				CommitteeBits: testhelpers.FillByteSlice(8, 83),
+				Signature:   testhelpers.FillByteSlice(96, 82),
+				CommitteeId: 83,
 			},
 			expectedErrorMessage: "attestation's target can't be nil",
 		},
-		{
-			name: "nil aggregation bits",
-			attestation: &ethpb.AttestationElectra{
-				Data: &ethpb.AttestationData{
-					Source: &ethpb.Checkpoint{},
-					Target: &ethpb.Checkpoint{},
-				},
-				Signature:     testhelpers.FillByteSlice(96, 82),
-				CommitteeBits: testhelpers.FillByteSlice(8, 83),
-			},
-			expectedErrorMessage: "attestation's bitfield can't be nil",
-		},
-		{
-			name: "nil signature",
-			attestation: &ethpb.AttestationElectra{
-				AggregationBits: testhelpers.FillByteSlice(4, 74),
-				Data: &ethpb.AttestationData{
-					Source: &ethpb.Checkpoint{},
-					Target: &ethpb.Checkpoint{},
-				},
-				CommitteeBits: testhelpers.FillByteSlice(8, 83),
-			},
-			expectedErrorMessage: "attestation signature can't be nil",
-		},
-		{
-			name: "nil committee bits",
-			attestation: &ethpb.AttestationElectra{
-				AggregationBits: testhelpers.FillByteSlice(4, 74),
-				Data: &ethpb.AttestationData{
-					Slot:            75,
-					CommitteeIndex:  76,
-					BeaconBlockRoot: testhelpers.FillByteSlice(32, 38),
-					Source: &ethpb.Checkpoint{
-						Epoch: 78,
-						Root:  testhelpers.FillByteSlice(32, 79),
-					},
-					Target: &ethpb.Checkpoint{
-						Epoch: 80,
-						Root:  testhelpers.FillByteSlice(32, 81),
-					},
-				},
-				Signature: testhelpers.FillByteSlice(96, 82),
-			},
-			expectedErrorMessage: "attestation committee bits can't be nil",
-		},
 		{
 			name:                 "bad request",
 			attestation:          attestation,
@@ -353,8 +298,8 @@ func TestProposeAttestationElectra(t *testing.T) {
 			jsonRestHandler := mock.NewMockJsonRestHandler(ctrl)
 
 			var marshalledAttestations []byte
-			if validateNilAttestation(test.attestation) == nil {
-				b, err := json.Marshal(jsonifyAttestationsElectra([]*ethpb.AttestationElectra{test.attestation}))
+			if helpers.ValidateNilAttestation(test.attestation) == nil {
+				b, err := json.Marshal(jsonifySingleAttestations([]*ethpb.SingleAttestation{test.attestation}))
 				require.NoError(t, err)
 				marshalledAttestations = b
 			}

validator/client/grpc-api/grpc_validator_client.go

@@ -71,7 +71,7 @@ func (c *grpcValidatorClient) ProposeAttestation(ctx context.Context, in *ethpb.
 	return c.beaconNodeValidatorClient.ProposeAttestation(ctx, in)
 }
 
-func (c *grpcValidatorClient) ProposeAttestationElectra(ctx context.Context, in *ethpb.AttestationElectra) (*ethpb.AttestResponse, error) {
+func (c *grpcValidatorClient) ProposeAttestationElectra(ctx context.Context, in *ethpb.SingleAttestation) (*ethpb.AttestResponse, error) {
 	return c.beaconNodeValidatorClient.ProposeAttestationElectra(ctx, in)
 }
 

validator/client/iface/validator_client.go

@@ -133,7 +133,7 @@ type ValidatorClient interface {
 	FeeRecipientByPubKey(ctx context.Context, in *ethpb.FeeRecipientByPubKeyRequest) (*ethpb.FeeRecipientByPubKeyResponse, error)
 	AttestationData(ctx context.Context, in *ethpb.AttestationDataRequest) (*ethpb.AttestationData, error)
 	ProposeAttestation(ctx context.Context, in *ethpb.Attestation) (*ethpb.AttestResponse, error)
-	ProposeAttestationElectra(ctx context.Context, in *ethpb.AttestationElectra) (*ethpb.AttestResponse, error)
+	ProposeAttestationElectra(ctx context.Context, in *ethpb.SingleAttestation) (*ethpb.AttestResponse, error)
 	SubmitAggregateSelectionProof(ctx context.Context, in *ethpb.AggregateSelectionRequest, index primitives.ValidatorIndex, committeeLength uint64) (*ethpb.AggregateSelectionResponse, error)
 	SubmitAggregateSelectionProofElectra(ctx context.Context, in *ethpb.AggregateSelectionRequest, _ primitives.ValidatorIndex, _ uint64) (*ethpb.AggregateSelectionElectraResponse, error)
 	SubmitSignedAggregateSelectionProof(ctx context.Context, in *ethpb.SignedAggregateSubmitRequest) (*ethpb.SignedAggregateSubmitResponse, error)