Electra: exclude empty requests in requests list

CHANGELOG.md

@@ -37,6 +37,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
 - Use read only state when computing the active validator list.
 - Simplified `ExitedValidatorIndices`.
 - Simplified `EjectedValidatorIndices`.
+- `engine_newPayloadV4`,`engine_getPayloadV4` are changes due to new execution request serialization decisions, [PR](https://github.com/prysmaticlabs/prysm/pull/14580)
 
 ### Deprecated
 

beacon-chain/execution/engine_client_test.go

@@ -1769,7 +1769,9 @@ func fixturesStruct() *payloadFixtures {
 			Proofs:      []hexutil.Bytes{[]byte("proof1"), []byte("proof2")},
 			Blobs:       []hexutil.Bytes{{'a'}, {'b'}},
 		},
-		ExecutionRequests: []hexutil.Bytes{depositRequestBytes, withdrawalRequestBytes, consolidationRequestBytes},
+		ExecutionRequests: []hexutil.Bytes{append([]byte{pb.DepositRequestType}, depositRequestBytes...),
+			append([]byte{pb.WithdrawalRequestType}, withdrawalRequestBytes...),
+			append([]byte{pb.ConsolidationRequestType}, consolidationRequestBytes...)},
 	}
 	parent := bytesutil.PadTo([]byte("parentHash"), fieldparams.RootLength)
 	sha3Uncles := bytesutil.PadTo([]byte("sha3Uncles"), fieldparams.RootLength)

proto/engine/v1/electra.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/v5/config/params"
 )
 
 type ExecutionPayloadElectra = ExecutionPayloadDeneb
@@ -19,59 +20,124 @@ var (
 	crSize    = crExample.SizeSSZ()
 )
 
-const LenExecutionRequestsElectra = 3
+const (
+	DepositRequestType = iota
+	WithdrawalRequestType
+	ConsolidationRequestType
+)
 
 func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) {
 	requests := &ExecutionRequests{}
-
-	if len(ebe.ExecutionRequests) != LenExecutionRequestsElectra /* types of requests */ {
-		return nil, errors.Errorf("invalid execution request size: %d", len(ebe.ExecutionRequests))
+	var prevTypeNum uint8
+	for i := range ebe.ExecutionRequests {
+		requestType, requestListInSSZBytes, err := decodeExecutionRequest(ebe.ExecutionRequests[i])
+		if err != nil {
+			return nil, err
+		}
+		if prevTypeNum > requestType {
+			return nil, errors.New("invalid execution request type order, requests should be in sorted order")
+		}
+		prevTypeNum = requestType
+		switch requestType {
+		case DepositRequestType:
+			drs, err := unmarshalDeposits(requestListInSSZBytes)
+			if err != nil {
+				return nil, err
+			}
+			requests.Deposits = drs
+		case WithdrawalRequestType:
+			wrs, err := unmarshalWithdrawals(requestListInSSZBytes)
+			if err != nil {
+				return nil, err
+			}
+			requests.Withdrawals = wrs
+		case ConsolidationRequestType:
+			crs, err := unmarshalConsolidations(requestListInSSZBytes)
+			if err != nil {
+				return nil, err
+			}
+			requests.Consolidations = crs
+		default:
+			return nil, errors.Errorf("unsupported request type %d", requestType)
+		}
 	}
+	return requests, nil
+}
 
-	// deposit requests
-	drs, err := unmarshalItems(ebe.ExecutionRequests[0], drSize, func() *DepositRequest { return &DepositRequest{} })
-	if err != nil {
-		return nil, err
+func unmarshalDeposits(requestListInSSZBytes []byte) ([]*DepositRequest, error) {
+	if len(requestListInSSZBytes) < drSize {
+		return nil, errors.New("invalid deposit requests length, requests should be at least the size of 1 request")
+	}
+	if uint64(len(requestListInSSZBytes)) > uint64(drSize)*params.BeaconConfig().MaxDepositRequestsPerPayload {
+		return nil, fmt.Errorf("invalid deposit requests length, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), drSize)
 	}
-	requests.Deposits = drs
+	return unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} })
+}
 
-	// withdrawal requests
-	wrs, err := unmarshalItems(ebe.ExecutionRequests[1], wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} })
-	if err != nil {
-		return nil, err
+func unmarshalWithdrawals(requestListInSSZBytes []byte) ([]*WithdrawalRequest, error) {
+	if len(requestListInSSZBytes) < wrSize {
+		return nil, errors.New("invalid withdrawal request length, requests should be at least the size of 1 request")
 	}
-	requests.Withdrawals = wrs
+	if uint64(len(requestListInSSZBytes)) > uint64(wrSize)*params.BeaconConfig().MaxWithdrawalRequestsPerPayload {
+		return nil, fmt.Errorf("invalid withdrawal requests length, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), wrSize)
+	}
+	return unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} })
+}
 
-	// consolidation requests
-	crs, err := unmarshalItems(ebe.ExecutionRequests[2], crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} })
-	if err != nil {
-		return nil, err
+func unmarshalConsolidations(requestListInSSZBytes []byte) ([]*ConsolidationRequest, error) {
+	if len(requestListInSSZBytes) < crSize {
+		return nil, errors.New("invalid consolidations request length, requests should be at least the size of 1 request")
+	}
+	if uint64(len(requestListInSSZBytes)) > uint64(crSize)*params.BeaconConfig().MaxConsolidationsRequestsPerPayload {
+		return nil, fmt.Errorf("invalid consolidation requests length, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), crSize)
 	}
-	requests.Consolidations = crs
+	return unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} })
+}
 
-	return requests, nil
+func decodeExecutionRequest(req []byte) (typ uint8, data []byte, err error) {
+	if len(req) < 1 {
+		return 0, nil, errors.New("invalid execution request, length less than 1")
+	}
+	return req[0], req[1:], nil
 }
 
 func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, error) {
 	if requests == nil {
 		return nil, errors.New("invalid execution requests")
 	}
 
-	drBytes, err := marshalItems(requests.Deposits)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to marshal deposit requests")
-	}
+	requestsData := make([]hexutil.Bytes, 0)
 
-	wrBytes, err := marshalItems(requests.Withdrawals)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to marshal withdrawal requests")
+	// request types MUST be in sorted order starting from 0
+	if len(requests.Deposits) > 0 {
+		drBytes, err := marshalItems(requests.Deposits)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to marshal deposit requests")
+		}
+		requestData := []byte{DepositRequestType}
+		requestData = append(requestData, drBytes...)
+		requestsData = append(requestsData, requestData)
 	}
-
-	crBytes, err := marshalItems(requests.Consolidations)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to marshal consolidation requests")
+	if len(requests.Withdrawals) > 0 {
+		wrBytes, err := marshalItems(requests.Withdrawals)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to marshal withdrawal requests")
+		}
+		requestData := []byte{WithdrawalRequestType}
+		requestData = append(requestData, wrBytes...)
+		requestsData = append(requestsData, requestData)
+	}
+	if len(requests.Consolidations) > 0 {
+		crBytes, err := marshalItems(requests.Consolidations)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to marshal consolidation requests")
+		}
+		requestData := []byte{ConsolidationRequestType}
+		requestData = append(requestData, crBytes...)
+		requestsData = append(requestsData, requestData)
 	}
-	return []hexutil.Bytes{drBytes, wrBytes, crBytes}, nil
+
+	return requestsData, nil
 }
 
 type sszUnmarshaler interface {

proto/engine/v1/electra_test.go

@@ -4,13 +4,167 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/prysmaticlabs/prysm/v5/config/params"
 	"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
 	enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
 	"github.com/prysmaticlabs/prysm/v5/testing/require"
 )
 
 var depositRequestsSSZHex = "0x706b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000077630000000000000000000000000000000000000000000000000000000000007b00000000000000736967000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c801000000000000706b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000776300000000000000000000000000000000000000000000000000000000000090010000000000007369670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000"
 
+func TestGetDecodedExecutionRequests(t *testing.T) {
+	t.Run("All requests decode successfully", func(t *testing.T) {
+		depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"620000000000000000000000000000000000000000000000000000000000000000" +
+			"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		withdrawalRequestBytes, err := hexutil.Decode("0x6400000000000000000000000000000000000000" +
+			"6500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040597307000000")
+		require.NoError(t, err)
+		consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
+			"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...),
+				append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes...),
+				append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
+		}
+		requests, err := ebe.GetDecodedExecutionRequests()
+		require.NoError(t, err)
+		require.Equal(t, len(requests.Deposits), 1)
+		require.Equal(t, len(requests.Withdrawals), 1)
+		require.Equal(t, len(requests.Consolidations), 1)
+	})
+	t.Run("Excluded requests still decode successfully when one request is missing", func(t *testing.T) {
+		depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"620000000000000000000000000000000000000000000000000000000000000000" +
+			"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
+			"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
+		}
+		requests, err := ebe.GetDecodedExecutionRequests()
+		require.NoError(t, err)
+		require.Equal(t, len(requests.Deposits), 1)
+		require.Equal(t, len(requests.Withdrawals), 0)
+		require.Equal(t, len(requests.Consolidations), 1)
+	})
+	t.Run("Decode execution requests should fail if ordering is not sorted", func(t *testing.T) {
+		depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"620000000000000000000000000000000000000000000000000000000000000000" +
+			"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
+			"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...), append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...)},
+		}
+		_, err = ebe.GetDecodedExecutionRequests()
+		require.ErrorContains(t, "invalid execution request type order", err)
+	})
+	t.Run("Requests should error if the request type is shorter than 1 byte", func(t *testing.T) {
+		consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
+			"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{append([]byte{}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
+		}
+		_, err = ebe.GetDecodedExecutionRequests()
+		require.ErrorContains(t, "invalid execution request, length less than 1", err)
+	})
+	t.Run("If a request type is provided, but the request list is shorter than the ssz of 1 request we error", func(t *testing.T) {
+		consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
+			"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
+			"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
+		}
+		_, err = ebe.GetDecodedExecutionRequests()
+		require.ErrorContains(t, "invalid deposit requests length", err)
+	})
+	t.Run("If deposit requests are over the max allowed per payload then we should error", func(t *testing.T) {
+		requests := make([]*enginev1.DepositRequest, params.BeaconConfig().MaxDepositRequestsPerPayload+1)
+		for i := range requests {
+			requests[i] = &enginev1.DepositRequest{
+				Pubkey:                bytesutil.PadTo([]byte("pk"), 48),
+				WithdrawalCredentials: bytesutil.PadTo([]byte("wc"), 32),
+				Amount:                123,
+				Signature:             bytesutil.PadTo([]byte("sig"), 96),
+				Index:                 456,
+			}
+		}
+		by, err := enginev1.MarshalItems(requests)
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{
+				append([]byte{uint8(enginev1.DepositRequestType)}, by...),
+			},
+		}
+		_, err = ebe.GetDecodedExecutionRequests()
+		require.ErrorContains(t, "invalid deposit requests length, requests should not be more than the max per payload", err)
+	})
+	t.Run("If withdrawal requests are over the max allowed per payload then we should error", func(t *testing.T) {
+		requests := make([]*enginev1.WithdrawalRequest, params.BeaconConfig().MaxWithdrawalRequestsPerPayload+1)
+		for i := range requests {
+			requests[i] = &enginev1.WithdrawalRequest{
+				SourceAddress:   bytesutil.PadTo([]byte("sa"), 20),
+				ValidatorPubkey: bytesutil.PadTo([]byte("pk"), 48),
+				Amount:          55555,
+			}
+		}
+		by, err := enginev1.MarshalItems(requests)
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{
+				append([]byte{uint8(enginev1.WithdrawalRequestType)}, by...),
+			},
+		}
+		_, err = ebe.GetDecodedExecutionRequests()
+		require.ErrorContains(t, "invalid withdrawal requests length, requests should not be more than the max per payload", err)
+	})
+	t.Run("If consolidation requests are over the max allowed per payload then we should error", func(t *testing.T) {
+		requests := make([]*enginev1.ConsolidationRequest, params.BeaconConfig().MaxConsolidationsRequestsPerPayload+1)
+		for i := range requests {
+			requests[i] = &enginev1.ConsolidationRequest{
+				SourceAddress: bytesutil.PadTo([]byte("sa"), 20),
+				SourcePubkey:  bytesutil.PadTo([]byte("pk"), 48),
+				TargetPubkey:  bytesutil.PadTo([]byte("pk"), 48),
+			}
+		}
+		by, err := enginev1.MarshalItems(requests)
+		require.NoError(t, err)
+		ebe := &enginev1.ExecutionBundleElectra{
+			ExecutionRequests: [][]byte{
+				append([]byte{uint8(enginev1.ConsolidationRequestType)}, by...),
+			},
+		}
+		_, err = ebe.GetDecodedExecutionRequests()
+		require.ErrorContains(t, "invalid consolidation requests length, requests should not be more than the max per payload", err)
+	})
+}
+
+func TestEncodeExecutionRequests(t *testing.T) {
+	t.Run("Empty execution requests should return an empty response and not nil", func(t *testing.T) {
+		ebe := &enginev1.ExecutionRequests{}
+		b, err := enginev1.EncodeExecutionRequests(ebe)
+		require.NoError(t, err)
+		require.NotNil(t, b)
+		require.Equal(t, len(b), 0)
+	})
+}
+
 func TestUnmarshalItems_OK(t *testing.T) {
 	drb, err := hexutil.Decode(depositRequestsSSZHex)
 	require.NoError(t, err)