Address @jtraglia's comments regarding Electra code (#14833)

* change field IDs in `AggregateAttestationAndProofElectra`

* fix typo in `validator.proto`

* correct slashing indices length and shashings length

* check length in indexed attestation's `ToConsensus` method

* use `fieldparams.BLSSignatureLength`

* Add length checks for execution request

* fix typo in `beacon_state.proto`

* fix typo in `ssz_proto_library.bzl`

* fix error messages about incorrect types in block factory

* add Electra case to `BeaconBlockContainerToSignedBeaconBlock`

* move PeerDAS config items to PeerDAS section

* remove redundant params

* rename `PendingDepositLimit` to `PendingDepositsLimit`

* improve requests unmarshaling errors

* rename `get_validator_max_effective_balance` to `get_max_effective_balance`

* fix typo in `consolidations.go`

* rename `index` to `validator_index` in `PendingPartialWithdrawal`

* rename `randomByte` to `randomBytes` in `validators.go`

* fix for version in a comment in `validator.go`

* changelog <3

* Revert "rename `index` to `validator_index` in `PendingPartialWithdrawal`"

This reverts commit 87e4da0.

api/server/structs/BUILD.bazel

@@ -28,6 +28,7 @@ go_library(
         "//api/server:go_default_library",
         "//beacon-chain/state:go_default_library",
         "//config/fieldparams:go_default_library",
+        "//config/params:go_default_library",
         "//consensus-types/interfaces:go_default_library",
         "//consensus-types/primitives:go_default_library",
         "//consensus-types/validator:go_default_library",

api/server/structs/conversions.go

@@ -9,6 +9,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v5/api/server"
 	fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
+	"github.com/prysmaticlabs/prysm/v5/config/params"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/validator"
 	"github.com/prysmaticlabs/prysm/v5/container/slice"
@@ -243,7 +244,7 @@ func (c *ContributionAndProof) ToConsensus() (*eth.ContributionAndProof, error)
 	if err != nil {
 		return nil, server.NewDecodeError(err, "AggregatorIndex")
 	}
-	selectionProof, err := bytesutil.DecodeHexWithLength(c.SelectionProof, 96)
+	selectionProof, err := bytesutil.DecodeHexWithLength(c.SelectionProof, fieldparams.BLSSignatureLength)
 	if err != nil {
 		return nil, server.NewDecodeError(err, "SelectionProof")
 	}
@@ -330,7 +331,7 @@ func (a *AggregateAttestationAndProof) ToConsensus() (*eth.AggregateAttestationA
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Aggregate")
 	}
-	proof, err := bytesutil.DecodeHexWithLength(a.SelectionProof, 96)
+	proof, err := bytesutil.DecodeHexWithLength(a.SelectionProof, fieldparams.BLSSignatureLength)
 	if err != nil {
 		return nil, server.NewDecodeError(err, "SelectionProof")
 	}
@@ -366,7 +367,7 @@ func (a *AggregateAttestationAndProofElectra) ToConsensus() (*eth.AggregateAttes
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Aggregate")
 	}
-	proof, err := bytesutil.DecodeHexWithLength(a.SelectionProof, 96)
+	proof, err := bytesutil.DecodeHexWithLength(a.SelectionProof, fieldparams.BLSSignatureLength)
 	if err != nil {
 		return nil, server.NewDecodeError(err, "SelectionProof")
 	}
@@ -734,6 +735,10 @@ func (s *AttesterSlashingElectra) ToConsensus() (*eth.AttesterSlashingElectra, e
 }
 
 func (a *IndexedAttestation) ToConsensus() (*eth.IndexedAttestation, error) {
+	if err := slice.VerifyMaxLength(a.AttestingIndices, params.BeaconConfig().MaxValidatorsPerCommittee); err != nil {
+		return nil, err
+	}
+
 	indices := make([]uint64, len(a.AttestingIndices))
 	var err error
 	for i, ix := range a.AttestingIndices {
@@ -759,6 +764,13 @@ func (a *IndexedAttestation) ToConsensus() (*eth.IndexedAttestation, error) {
 }
 
 func (a *IndexedAttestationElectra) ToConsensus() (*eth.IndexedAttestationElectra, error) {
+	if err := slice.VerifyMaxLength(
+		a.AttestingIndices,
+		params.BeaconConfig().MaxValidatorsPerCommittee*params.BeaconConfig().MaxCommitteesPerSlot,
+	); err != nil {
+		return nil, err
+	}
+
 	indices := make([]uint64, len(a.AttestingIndices))
 	var err error
 	for i, ix := range a.AttestingIndices {
@@ -1189,7 +1201,7 @@ func AttesterSlashingsElectraToConsensus(src []*AttesterSlashingElectra) ([]*eth
 	if src == nil {
 		return nil, errNilValue
 	}
-	err := slice.VerifyMaxLength(src, 2)
+	err := slice.VerifyMaxLength(src, fieldparams.MaxAttesterSlashingsElectra)
 	if err != nil {
 		return nil, err
 	}
@@ -1210,7 +1222,7 @@ func AttesterSlashingsElectraToConsensus(src []*AttesterSlashingElectra) ([]*eth
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation1.Signature", i))
 		}
-		err = slice.VerifyMaxLength(s.Attestation1.AttestingIndices, 2048)
+		err = slice.VerifyMaxLength(s.Attestation1.AttestingIndices, params.BeaconConfig().MaxValidatorsPerCommittee*params.BeaconConfig().MaxCommitteesPerSlot)
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation1.AttestingIndices", i))
 		}
@@ -1230,7 +1242,7 @@ func AttesterSlashingsElectraToConsensus(src []*AttesterSlashingElectra) ([]*eth
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation2.Signature", i))
 		}
-		err = slice.VerifyMaxLength(s.Attestation2.AttestingIndices, 2048)
+		err = slice.VerifyMaxLength(s.Attestation2.AttestingIndices, params.BeaconConfig().MaxValidatorsPerCommittee*params.BeaconConfig().MaxCommitteesPerSlot)
 		if err != nil {
 			return nil, server.NewDecodeError(err, fmt.Sprintf("[%d].Attestation2.AttestingIndices", i))
 		}

api/server/structs/conversions_block.go

@@ -9,6 +9,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v5/api/server"
 	fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
+	"github.com/prysmaticlabs/prysm/v5/config/params"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
 	"github.com/prysmaticlabs/prysm/v5/container/slice"
@@ -2519,6 +2520,7 @@ func (b *BeaconBlockContentsElectra) ToConsensus() (*eth.BeaconBlockContentsElec
 	}, nil
 }
 
+// nolint:gocognit
 func (b *BeaconBlockElectra) ToConsensus() (*eth.BeaconBlockElectra, error) {
 	if b == nil {
 		return nil, errNilValue
@@ -2706,6 +2708,9 @@ func (b *BeaconBlockElectra) ToConsensus() (*eth.BeaconBlockElectra, error) {
 		return nil, server.NewDecodeError(errors.New("nil execution requests"), "Body.ExequtionRequests")
 	}
 
+	if err = slice.VerifyMaxLength(b.Body.ExecutionRequests.Deposits, params.BeaconConfig().MaxDepositRequestsPerPayload); err != nil {
+		return nil, err
+	}
 	depositRequests := make([]*enginev1.DepositRequest, len(b.Body.ExecutionRequests.Deposits))
 	for i, d := range b.Body.ExecutionRequests.Deposits {
 		depositRequests[i], err = d.ToConsensus()
@@ -2714,6 +2719,9 @@ func (b *BeaconBlockElectra) ToConsensus() (*eth.BeaconBlockElectra, error) {
 		}
 	}
 
+	if err = slice.VerifyMaxLength(b.Body.ExecutionRequests.Withdrawals, params.BeaconConfig().MaxWithdrawalRequestsPerPayload); err != nil {
+		return nil, err
+	}
 	withdrawalRequests := make([]*enginev1.WithdrawalRequest, len(b.Body.ExecutionRequests.Withdrawals))
 	for i, w := range b.Body.ExecutionRequests.Withdrawals {
 		withdrawalRequests[i], err = w.ToConsensus()
@@ -2722,6 +2730,9 @@ func (b *BeaconBlockElectra) ToConsensus() (*eth.BeaconBlockElectra, error) {
 		}
 	}
 
+	if err = slice.VerifyMaxLength(b.Body.ExecutionRequests.Consolidations, params.BeaconConfig().MaxConsolidationsRequestsPerPayload); err != nil {
+		return nil, err
+	}
 	consolidationRequests := make([]*enginev1.ConsolidationRequest, len(b.Body.ExecutionRequests.Consolidations))
 	for i, c := range b.Body.ExecutionRequests.Consolidations {
 		consolidationRequests[i], err = c.ToConsensus()
@@ -3003,9 +3014,14 @@ func (b *BlindedBeaconBlockElectra) ToConsensus() (*eth.BlindedBeaconBlockElectr
 	if err != nil {
 		return nil, server.NewDecodeError(err, "Body.ExecutionPayload.ExcessBlobGas")
 	}
+
 	if b.Body.ExecutionRequests == nil {
 		return nil, server.NewDecodeError(errors.New("nil execution requests"), "Body.ExecutionRequests")
 	}
+
+	if err = slice.VerifyMaxLength(b.Body.ExecutionRequests.Deposits, params.BeaconConfig().MaxDepositRequestsPerPayload); err != nil {
+		return nil, err
+	}
 	depositRequests := make([]*enginev1.DepositRequest, len(b.Body.ExecutionRequests.Deposits))
 	for i, d := range b.Body.ExecutionRequests.Deposits {
 		depositRequests[i], err = d.ToConsensus()
@@ -3014,6 +3030,9 @@ func (b *BlindedBeaconBlockElectra) ToConsensus() (*eth.BlindedBeaconBlockElectr
 		}
 	}
 
+	if err = slice.VerifyMaxLength(b.Body.ExecutionRequests.Withdrawals, params.BeaconConfig().MaxWithdrawalRequestsPerPayload); err != nil {
+		return nil, err
+	}
 	withdrawalRequests := make([]*enginev1.WithdrawalRequest, len(b.Body.ExecutionRequests.Withdrawals))
 	for i, w := range b.Body.ExecutionRequests.Withdrawals {
 		withdrawalRequests[i], err = w.ToConsensus()
@@ -3022,6 +3041,9 @@ func (b *BlindedBeaconBlockElectra) ToConsensus() (*eth.BlindedBeaconBlockElectr
 		}
 	}
 
+	if err = slice.VerifyMaxLength(b.Body.ExecutionRequests.Consolidations, params.BeaconConfig().MaxConsolidationsRequestsPerPayload); err != nil {
+		return nil, err
+	}
 	consolidationRequests := make([]*enginev1.ConsolidationRequest, len(b.Body.ExecutionRequests.Consolidations))
 	for i, c := range b.Body.ExecutionRequests.Consolidations {
 		consolidationRequests[i], err = c.ToConsensus()

beacon-chain/core/electra/consolidations.go

@@ -265,7 +265,7 @@ func ProcessConsolidationRequests(ctx context.Context, st state.BeaconState, req
 		if !helpers.IsActiveValidator(srcV, curEpoch) || !helpers.IsActiveValidatorUsingTrie(tgtV, curEpoch) {
 			continue
 		}
-		// Neither validator are exiting.
+		// Neither validator is exiting.
 		if srcV.ExitEpoch != ffe || tgtV.ExitEpoch() != ffe {
 			continue
 		}

beacon-chain/core/helpers/validator_churn.go

@@ -22,7 +22,7 @@ import (
 func BalanceChurnLimit(activeBalance primitives.Gwei) primitives.Gwei {
 	churn := max(
 		params.BeaconConfig().MinPerEpochChurnLimitElectra,
-		(uint64(activeBalance) / params.BeaconConfig().ChurnLimitQuotient),
+		uint64(activeBalance)/params.BeaconConfig().ChurnLimitQuotient,
 	)
 	return primitives.Gwei(churn - churn%params.BeaconConfig().EffectiveBalanceIncrement)
 }

beacon-chain/core/helpers/validators.go

@@ -393,9 +393,9 @@ func ComputeProposerIndex(bState state.ReadOnlyBeaconState, activeIndices []prim
 		effectiveBal := v.EffectiveBalance()
 		if bState.Version() >= version.Electra {
 			binary.LittleEndian.PutUint64(seedBuffer[len(seed):], i/16)
-			randomByte := hashFunc(seedBuffer)
+			randomBytes := hashFunc(seedBuffer)
 			offset := (i % 16) * 2
-			randomValue := uint64(randomByte[offset]) | uint64(randomByte[offset+1])<<8
+			randomValue := uint64(randomBytes[offset]) | uint64(randomBytes[offset+1])<<8
 
 			if effectiveBal*fieldparams.MaxRandomValueElectra >= beaconConfig.MaxEffectiveBalanceElectra*randomValue {
 				return candidateIndex, nil
@@ -579,7 +579,7 @@ func IsPartiallyWithdrawableValidator(val state.ReadOnlyValidator, balance uint6
 //	"""
 //	Check if ``validator`` is partially withdrawable.
 //	"""
-//	max_effective_balance = get_validator_max_effective_balance(validator)
+//	max_effective_balance = get_max_effective_balance(validator)
 //	has_max_effective_balance = validator.effective_balance == max_effective_balance  # [Modified in Electra:EIP7251]
 //	has_excess_balance = balance > max_effective_balance  # [Modified in Electra:EIP7251]
 //	return (
@@ -619,7 +619,7 @@ func isPartiallyWithdrawableValidatorCapella(val state.ReadOnlyValidator, balanc
 //
 // Spec definition:
 //
-//	def get_validator_max_effective_balance(validator: Validator) -> Gwei:
+//	def get_max_effective_balance(validator: Validator) -> Gwei:
 //	    """
 //	    Get max effective balance for ``validator``.
 //	    """

beacon-chain/core/validators/validator.go

@@ -75,7 +75,7 @@ func InitiateValidatorExit(ctx context.Context, s state.BeaconState, idx primiti
 
 	// Compute exit queue epoch.
 	if s.Version() < version.Electra {
-		// Relevant spec code from deneb:
+		// Relevant spec code from phase0:
 		//
 		//	exit_epochs = [v.exit_epoch for v in state.validators if v.exit_epoch != FAR_FUTURE_EPOCH]
 		//	exit_queue_epoch = max(exit_epochs + [compute_activation_exit_epoch(get_current_epoch(state))])

beacon-chain/rpc/eth/beacon/handlers_pool_test.go

@@ -1559,7 +1559,7 @@ func TestGetAttesterSlashings(t *testing.T) {
 			Signature: bytesutil.PadTo([]byte("signature4"), 96),
 		},
 	}
-	slashing1PostElectra := &ethpbv1alpha1.AttesterSlashingElectra{
+	slashingPostElectra := &ethpbv1alpha1.AttesterSlashingElectra{
 		Attestation_1: &ethpbv1alpha1.IndexedAttestationElectra{
 			AttestingIndices: []uint64{1, 10},
 			Data: &ethpbv1alpha1.AttestationData{
@@ -1595,42 +1595,6 @@ func TestGetAttesterSlashings(t *testing.T) {
 			Signature: bytesutil.PadTo([]byte("signature2"), 96),
 		},
 	}
-	slashing2PostElectra := &ethpbv1alpha1.AttesterSlashingElectra{
-		Attestation_1: &ethpbv1alpha1.IndexedAttestationElectra{
-			AttestingIndices: []uint64{3, 30},
-			Data: &ethpbv1alpha1.AttestationData{
-				Slot:            3,
-				CommitteeIndex:  3,
-				BeaconBlockRoot: bytesutil.PadTo([]byte("blockroot3"), 32),
-				Source: &ethpbv1alpha1.Checkpoint{
-					Epoch: 3,
-					Root:  bytesutil.PadTo([]byte("sourceroot3"), 32),
-				},
-				Target: &ethpbv1alpha1.Checkpoint{
-					Epoch: 30,
-					Root:  bytesutil.PadTo([]byte("targetroot3"), 32),
-				},
-			},
-			Signature: bytesutil.PadTo([]byte("signature3"), 96),
-		},
-		Attestation_2: &ethpbv1alpha1.IndexedAttestationElectra{
-			AttestingIndices: []uint64{4, 40},
-			Data: &ethpbv1alpha1.AttestationData{
-				Slot:            4,
-				CommitteeIndex:  4,
-				BeaconBlockRoot: bytesutil.PadTo([]byte("blockroot4"), 32),
-				Source: &ethpbv1alpha1.Checkpoint{
-					Epoch: 4,
-					Root:  bytesutil.PadTo([]byte("sourceroot4"), 32),
-				},
-				Target: &ethpbv1alpha1.Checkpoint{
-					Epoch: 40,
-					Root:  bytesutil.PadTo([]byte("targetroot4"), 32),
-				},
-			},
-			Signature: bytesutil.PadTo([]byte("signature4"), 96),
-		},
-	}
 
 	t.Run("V1", func(t *testing.T) {
 		t.Run("ok", func(t *testing.T) {
@@ -1702,7 +1666,7 @@ func TestGetAttesterSlashings(t *testing.T) {
 			s := &Server{
 				ChainInfoFetcher: chainService,
 				TimeFetcher:      chainService,
-				SlashingsPool:    &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashing1PostElectra, slashing2PostElectra, slashing1PreElectra}},
+				SlashingsPool:    &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashingPostElectra, slashing1PreElectra}},
 			}
 
 			request := httptest.NewRequest(http.MethodGet, "http://example.com/eth/v2/beacon/pool/attester_slashings", nil)
@@ -1724,8 +1688,7 @@ func TestGetAttesterSlashings(t *testing.T) {
 			ss, err := structs.AttesterSlashingsElectraToConsensus(slashings)
 			require.NoError(t, err)
 
-			require.DeepEqual(t, slashing1PostElectra, ss[0])
-			require.DeepEqual(t, slashing2PostElectra, ss[1])
+			require.DeepEqual(t, slashingPostElectra, ss[0])
 		})
 		t.Run("post-electra-ok", func(t *testing.T) {
 			bs, err := util.NewBeaconStateElectra()
@@ -1741,7 +1704,7 @@ func TestGetAttesterSlashings(t *testing.T) {
 			s := &Server{
 				ChainInfoFetcher: chainService,
 				TimeFetcher:      chainService,
-				SlashingsPool:    &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashing1PostElectra, slashing2PostElectra}},
+				SlashingsPool:    &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashingPostElectra}},
 			}
 
 			request := httptest.NewRequest(http.MethodGet, "http://example.com/eth/v2/beacon/pool/attester_slashings", nil)
@@ -1763,8 +1726,7 @@ func TestGetAttesterSlashings(t *testing.T) {
 			ss, err := structs.AttesterSlashingsElectraToConsensus(slashings)
 			require.NoError(t, err)
 
-			require.DeepEqual(t, slashing1PostElectra, ss[0])
-			require.DeepEqual(t, slashing2PostElectra, ss[1])
+			require.DeepEqual(t, slashingPostElectra, ss[0])
 		})
 		t.Run("pre-electra-ok", func(t *testing.T) {
 			bs, err := util.NewBeaconState()

beacon-chain/rpc/eth/config/handlers_test.go

@@ -140,7 +140,7 @@ func TestGetSpec(t *testing.T) {
 	config.WhistleBlowerRewardQuotientElectra = 79
 	config.PendingPartialWithdrawalsLimit = 80
 	config.MinActivationBalance = 81
-	config.PendingDepositLimit = 82
+	config.PendingDepositsLimit = 82
 	config.MaxPendingPartialsPerWithdrawalsSweep = 83
 	config.PendingConsolidationsLimit = 84
 	config.MaxPartialWithdrawalsPerPayload = 85

beacon-chain/state/state-native/getters_withdrawal.go

@@ -95,7 +95,7 @@ func (b *BeaconState) NextWithdrawalValidatorIndex() (primitives.ValidatorIndex,
 //					index=withdrawal_index,
 //					validator_index=validator_index,
 //					address=ExecutionAddress(validator.withdrawal_credentials[12:]),
-//					amount=balance - get_validator_max_effective_balance(validator),  # [Modified in Electra:EIP7251]
+//					amount=balance - get_max_effective_balance(validator),  # [Modified in Electra:EIP7251]
 //				))
 //				withdrawal_index += WithdrawalIndex(1)
 //			if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD:

changelog/radek_electra-nits.md

@@ -0,0 +1,7 @@
+### Fixed
+
+- Fixed incorrect attester slashing length check.
+
+### Ignored
+
+- Addressed many small suggestions from @jtraglia.