From ace832cc772ddbf861dfd69b759349f8962c0128 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Thu, 24 Oct 2024 16:52:00 -0500 Subject: [PATCH 01/19] implementing new decisions around exectuion requests --- proto/engine/v1/electra.go | 92 +++++++++++++++++++++++--------------- 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 90d003ec2a7d..91602eb26e78 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -19,35 +19,39 @@ var ( crSize = crExample.SizeSSZ() ) -const LenExecutionRequestsElectra = 3 +const ( + depositRequestType = 0 + withdrawalRequestType = 1 + consolidationRequestType = 2 +) 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)) - } - - // deposit requests - drs, err := unmarshalItems(ebe.ExecutionRequests[0], drSize, func() *DepositRequest { return &DepositRequest{} }) - if err != nil { - return nil, err - } - requests.Deposits = drs - - // withdrawal requests - wrs, err := unmarshalItems(ebe.ExecutionRequests[1], wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) - if err != nil { - return nil, err - } - requests.Withdrawals = wrs - - // consolidation requests - crs, err := unmarshalItems(ebe.ExecutionRequests[2], crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) - if err != nil { - return nil, err + for i := range ebe.ExecutionRequests { + requestType := ebe.ExecutionRequests[i][0] + requestListInSSZBytes := ebe.ExecutionRequests[i][1:] + switch requestType { + case depositRequestType: + drs, err := unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) + if err != nil { + return nil, err + } + requests.Deposits = drs + case withdrawalRequestType: + wrs, err := unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) + if err != nil { + return nil, err + } + requests.Withdrawals = wrs + case consolidationRequestType: + crs, err := unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) + if err != nil { + return nil, err + } + requests.Consolidations = crs + } } - requests.Consolidations = crs return requests, nil } @@ -57,21 +61,37 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro 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") - } + var requestsData []hexutil.Bytes - wrBytes, err := marshalItems(requests.Withdrawals) - if err != nil { - return nil, errors.Wrap(err, "failed to marshal withdrawal requests") + 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{0} + 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{1} + requestData = append(requestData, wrBytes...) + requestsData = append(requestsData, requestData) } - return []hexutil.Bytes{drBytes, wrBytes, crBytes}, nil + 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{2} + requestData = append(requestData, crBytes...) + requestsData = append(requestsData, requestData) + } + + return requestsData, nil } type sszUnmarshaler interface { From 0884e0e56b9637c28f5b67d5708b9f8f1bc8e38b Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 11:21:11 -0500 Subject: [PATCH 02/19] fixing test fixture --- beacon-chain/execution/engine_client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/execution/engine_client_test.go b/beacon-chain/execution/engine_client_test.go index 0e2ecb48bd2c..8c76162fff56 100644 --- a/beacon-chain/execution/engine_client_test.go +++ b/beacon-chain/execution/engine_client_test.go @@ -1767,7 +1767,7 @@ 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{0}, depositRequestBytes...), append([]byte{1}, withdrawalRequestBytes...), append([]byte{2}, consolidationRequestBytes...)}, } parent := bytesutil.PadTo([]byte("parentHash"), fieldparams.RootLength) sha3Uncles := bytesutil.PadTo([]byte("sha3Uncles"), fieldparams.RootLength) From bd2649407c20ba1ad085b64cfd07d4ffae24ad85 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 13:31:12 -0500 Subject: [PATCH 03/19] adding in more edge case checks and tests --- proto/engine/v1/electra.go | 11 ++++++-- proto/engine/v1/electra_test.go | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 91602eb26e78..135bc2207c00 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -27,9 +27,13 @@ const ( func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) { requests := &ExecutionRequests{} - + var prevTypeNum int for i := range ebe.ExecutionRequests { requestType := ebe.ExecutionRequests[i][0] + if prevTypeNum > int(requestType) { + return nil, errors.New("invalid execution request type order, requests should be in sorted order") + } + prevTypeNum = int(requestType) requestListInSSZBytes := ebe.ExecutionRequests[i][1:] switch requestType { case depositRequestType: @@ -50,6 +54,8 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ return nil, err } requests.Consolidations = crs + default: + return nil, errors.Errorf("unsupported request type %d", requestType) } } @@ -61,8 +67,9 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro return nil, errors.New("invalid execution requests") } - var requestsData []hexutil.Bytes + requestsData := make([]hexutil.Bytes, 0) + // request types MUST be in sorted order starting from 0 if len(requests.Deposits) > 0 { drBytes, err := marshalItems(requests.Deposits) if err != nil { diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index 2d4c9903b98f..e9affec36590 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -11,6 +11,54 @@ import ( var depositRequestsSSZHex = "0x706b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000077630000000000000000000000000000000000000000000000000000000000007b00000000000000736967000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c801000000000000706b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000776300000000000000000000000000000000000000000000000000000000000090010000000000007369670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000" +func TestGetDecodedExecutionRequests(t *testing.T) { + t.Run("Excluded requests still decode successfully", 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{0}, depositRequestBytes...), append([]byte{2}, 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{2}, consolidationRequestBytes...), append([]byte{0}, depositRequestBytes...)}, + } + _, err = ebe.GetDecodedExecutionRequests() + require.ErrorContains(t, "invalid execution request type order", 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) From e0ffd1a7931fe671c0903aab3593ddbbc18b726e Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 13:37:53 -0500 Subject: [PATCH 04/19] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d842d57b40d0..79c154e6e32d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Use read only validator for core processing to avoid unnecessary copying. - Use ROBlock across block processing pipeline. - Added missing Eth-Consensus-Version headers to GetBlockAttestationsV2 and GetAttesterSlashingsV2 endpoints. +- `engine_newPayloadV4`,`engine_getPayloadV4` are changes due to new execution request serialization decisions, [PR](https://github.com/prysmaticlabs/prysm/pull/14580) ### Deprecated From 3c7cbf720c2f836afe378a55cb5f98b7d973b2eb Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 13:48:45 -0500 Subject: [PATCH 05/19] fixing unsafe type cast --- proto/engine/v1/electra.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 135bc2207c00..11d7dc6ab224 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -27,13 +27,13 @@ const ( func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) { requests := &ExecutionRequests{} - var prevTypeNum int + var prevTypeNum uint8 for i := range ebe.ExecutionRequests { requestType := ebe.ExecutionRequests[i][0] - if prevTypeNum > int(requestType) { + if prevTypeNum > requestType { return nil, errors.New("invalid execution request type order, requests should be in sorted order") } - prevTypeNum = int(requestType) + prevTypeNum = requestType requestListInSSZBytes := ebe.ExecutionRequests[i][1:] switch requestType { case depositRequestType: From 2852bccb1e9e84c1a6fd693bf58a5153bf862de2 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:28:05 -0500 Subject: [PATCH 06/19] Update beacon-chain/execution/engine_client_test.go Co-authored-by: Preston Van Loon --- beacon-chain/execution/engine_client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/execution/engine_client_test.go b/beacon-chain/execution/engine_client_test.go index 427a8125f843..cfc49ce2c2a5 100644 --- a/beacon-chain/execution/engine_client_test.go +++ b/beacon-chain/execution/engine_client_test.go @@ -1769,7 +1769,7 @@ func fixturesStruct() *payloadFixtures { Proofs: []hexutil.Bytes{[]byte("proof1"), []byte("proof2")}, Blobs: []hexutil.Bytes{{'a'}, {'b'}}, }, - ExecutionRequests: []hexutil.Bytes{append([]byte{0}, depositRequestBytes...), append([]byte{1}, withdrawalRequestBytes...), append([]byte{2}, consolidationRequestBytes...)}, + ExecutionRequests: []hexutil.Bytes{append([]byte{depositRequestType}, depositRequestBytes...), append([]byte{withdrawalRequestType}, withdrawalRequestBytes...), append([]byte{consolidationRequestType}, consolidationRequestBytes...)}, } parent := bytesutil.PadTo([]byte("parentHash"), fieldparams.RootLength) sha3Uncles := bytesutil.PadTo([]byte("sha3Uncles"), fieldparams.RootLength) From 3cdb4adbeff29a779f95377a3586fe42f7e9b7ab Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:31:55 -0500 Subject: [PATCH 07/19] Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon --- proto/engine/v1/electra.go | 1 + 1 file changed, 1 insertion(+) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 11d7dc6ab224..81cb92fd9fc5 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -30,6 +30,7 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ var prevTypeNum uint8 for i := range ebe.ExecutionRequests { requestType := ebe.ExecutionRequests[i][0] + // Requests must be sorted in ascending order by request type. if prevTypeNum > requestType { return nil, errors.New("invalid execution request type order, requests should be in sorted order") } From 9f7997be08d32cadfb1f63b86c57d49212dd12c6 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:32:01 -0500 Subject: [PATCH 08/19] Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon --- proto/engine/v1/electra.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 81cb92fd9fc5..9d644028171f 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -76,7 +76,7 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro if err != nil { return nil, errors.Wrap(err, "failed to marshal deposit requests") } - requestData := []byte{0} + requestData := []byte{depositRequestType} requestData = append(requestData, drBytes...) requestsData = append(requestsData, requestData) } From 14f5f0ae1c0aab8a3e231152f3b502934af2b5f2 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:32:20 -0500 Subject: [PATCH 09/19] Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon --- proto/engine/v1/electra.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 9d644028171f..01341b0457c5 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -85,7 +85,7 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro if err != nil { return nil, errors.Wrap(err, "failed to marshal withdrawal requests") } - requestData := []byte{1} + requestData := []byte{withdrawalRequestType} requestData = append(requestData, wrBytes...) requestsData = append(requestsData, requestData) } From 4b22e25c83f3ce00c16cf3f04bbb10a3acc0b0b6 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:32:26 -0500 Subject: [PATCH 10/19] Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon --- proto/engine/v1/electra.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 01341b0457c5..18f47e181f9d 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -94,7 +94,7 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro if err != nil { return nil, errors.Wrap(err, "failed to marshal consolidation requests") } - requestData := []byte{2} + requestData := []byte{consolidationRequestType} requestData = append(requestData, crBytes...) requestsData = append(requestsData, requestData) } From e7ba3b10f21c0980ae297fbe9e7f93c2ed021928 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:32:33 -0500 Subject: [PATCH 11/19] Update proto/engine/v1/electra_test.go Co-authored-by: Preston Van Loon --- proto/engine/v1/electra_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index e9affec36590..320af9b674a1 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -23,7 +23,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { "680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") require.NoError(t, err) ebe := &enginev1.ExecutionBundleElectra{ - ExecutionRequests: [][]byte{append([]byte{0}, depositRequestBytes...), append([]byte{2}, consolidationRequestBytes...)}, + ExecutionRequests: [][]byte{append([]byte{depositRequestType}, depositRequestBytes...), append([]byte{consolidationRequestType}, consolidationRequestBytes...)}, } requests, err := ebe.GetDecodedExecutionRequests() require.NoError(t, err) From b077cfd37485bc37e3986ec3b579c4e5a6ca3c13 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:32:39 -0500 Subject: [PATCH 12/19] Update proto/engine/v1/electra_test.go Co-authored-by: Preston Van Loon --- proto/engine/v1/electra_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index 320af9b674a1..c79250e3c861 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -42,7 +42,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { "680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") require.NoError(t, err) ebe := &enginev1.ExecutionBundleElectra{ - ExecutionRequests: [][]byte{append([]byte{2}, consolidationRequestBytes...), append([]byte{0}, depositRequestBytes...)}, + ExecutionRequests: [][]byte{append([]byte{consolidationRequestType}, consolidationRequestBytes...), append([]byte{depositRequestType}, depositRequestBytes...)}, } _, err = ebe.GetDecodedExecutionRequests() require.ErrorContains(t, "invalid execution request type order", err) From 8f62720ec5d1b1731db49e88e6e0329b6177f497 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Mon, 28 Oct 2024 09:31:19 -0500 Subject: [PATCH 13/19] updating based on preston's feedback and adding more tests for edge cases --- proto/engine/v1/electra.go | 24 ++++++++++++++++++++---- proto/engine/v1/electra_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 18f47e181f9d..85cc64539159 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -29,27 +29,37 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ requests := &ExecutionRequests{} var prevTypeNum uint8 for i := range ebe.ExecutionRequests { - requestType := ebe.ExecutionRequests[i][0] - // Requests must be sorted in ascending order by request type. + 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 - requestListInSSZBytes := ebe.ExecutionRequests[i][1:] switch requestType { case depositRequestType: + if len(requestListInSSZBytes) < drSize { + return nil, errors.New("invalid deposit request length, requests should be at least the size of 1 request") + } drs, err := unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) if err != nil { return nil, err } requests.Deposits = drs case withdrawalRequestType: + if len(requestListInSSZBytes) < wrSize { + return nil, errors.New("invalid withdrawal request length, requests should be at least the size of 1 request") + } wrs, err := unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) if err != nil { return nil, err } requests.Withdrawals = wrs case consolidationRequestType: + if len(requestListInSSZBytes) < crSize { + return nil, errors.New("invalid consolidations request length, requests should be at least the size of 1 request") + } crs, err := unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) if err != nil { return nil, err @@ -59,10 +69,16 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ return nil, errors.Errorf("unsupported request type %d", requestType) } } - 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") diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index c79250e3c861..490a50e3f854 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -47,6 +47,33 @@ func TestGetDecodedExecutionRequests(t *testing.T) { _, err = ebe.GetDecodedExecutionRequests() require.ErrorContains(t, "invalid execution request type order", err) }) + t.Run("Requests should error if it's shorter than 1 byte", 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{}, depositRequestBytes...), append([]byte{2}, 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{0}, []byte{}...), append([]byte{2}, consolidationRequestBytes...)}, + } + _, err = ebe.GetDecodedExecutionRequests() + require.ErrorContains(t, "invalid deposit request length", err) + }) } func TestEncodeExecutionRequests(t *testing.T) { From 44de0424d63a6b584d73aecf84c664ef84c8bd47 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Mon, 28 Oct 2024 10:03:36 -0500 Subject: [PATCH 14/19] adding more edgecases, and unit tests --- proto/engine/v1/electra.go | 12 +++++++++++- proto/engine/v1/electra_test.go | 17 ++++++----------- proto/engine/v1/export_test.go | 6 ++++++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 85cc64539159..69b62006391c 100644 --- a/proto/engine/v1/electra.go +++ b/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 @@ -40,7 +41,10 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ switch requestType { case depositRequestType: if len(requestListInSSZBytes) < drSize { - return nil, errors.New("invalid deposit request length, requests should be at least the size of 1 request") + 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) } drs, err := unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) if err != nil { @@ -51,6 +55,9 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ if len(requestListInSSZBytes) < wrSize { return nil, errors.New("invalid withdrawal request length, requests should be at least the size of 1 request") } + 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) + } wrs, err := unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) if err != nil { return nil, err @@ -60,6 +67,9 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ 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) + } crs, err := unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) if err != nil { return nil, err diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index 490a50e3f854..6619edebe839 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -23,7 +23,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { "680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") require.NoError(t, err) ebe := &enginev1.ExecutionBundleElectra{ - ExecutionRequests: [][]byte{append([]byte{depositRequestType}, depositRequestBytes...), append([]byte{consolidationRequestType}, consolidationRequestBytes...)}, + ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)}, } requests, err := ebe.GetDecodedExecutionRequests() require.NoError(t, err) @@ -42,23 +42,18 @@ func TestGetDecodedExecutionRequests(t *testing.T) { "680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") require.NoError(t, err) ebe := &enginev1.ExecutionBundleElectra{ - ExecutionRequests: [][]byte{append([]byte{consolidationRequestType}, consolidationRequestBytes...), append([]byte{depositRequestType}, depositRequestBytes...)}, + 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 it's shorter than 1 byte", func(t *testing.T) { - depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + - "620000000000000000000000000000000000000000000000000000000000000000" + - "4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + - "00000000000000000000000000000000000000000000000000000000000000000000000000000000") - require.NoError(t, 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{}, depositRequestBytes...), append([]byte{2}, consolidationRequestBytes...)}, + 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) @@ -69,10 +64,10 @@ func TestGetDecodedExecutionRequests(t *testing.T) { "680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") require.NoError(t, err) ebe := &enginev1.ExecutionBundleElectra{ - ExecutionRequests: [][]byte{append([]byte{0}, []byte{}...), append([]byte{2}, consolidationRequestBytes...)}, + ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)}, } _, err = ebe.GetDecodedExecutionRequests() - require.ErrorContains(t, "invalid deposit request length", err) + require.ErrorContains(t, "invalid deposit requests length", err) }) } diff --git a/proto/engine/v1/export_test.go b/proto/engine/v1/export_test.go index 2466b7fa44d1..d873a653865a 100644 --- a/proto/engine/v1/export_test.go +++ b/proto/engine/v1/export_test.go @@ -9,3 +9,9 @@ func MarshalItems[T sszMarshaler](items []T) ([]byte, error) { func UnmarshalItems[T sszUnmarshaler](data []byte, itemSize int, newItem func() T) ([]T, error) { return unmarshalItems(data, itemSize, newItem) } + +var ( + DepositRequestType = depositRequestType + WithdrawalRequestType = withdrawalRequestType + ConsolidationRequestType = consolidationRequestType +) From ace1aa5ccc776b02db59819eda4439f0c991b550 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Mon, 28 Oct 2024 10:28:50 -0500 Subject: [PATCH 15/19] fixing tests --- beacon-chain/execution/engine_client_test.go | 4 +++- proto/engine/v1/electra.go | 12 ++++++------ proto/engine/v1/export_test.go | 6 ------ 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/beacon-chain/execution/engine_client_test.go b/beacon-chain/execution/engine_client_test.go index cfc49ce2c2a5..10c2eca45b8d 100644 --- a/beacon-chain/execution/engine_client_test.go +++ b/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{append([]byte{depositRequestType}, depositRequestBytes...), append([]byte{withdrawalRequestType}, withdrawalRequestBytes...), append([]byte{consolidationRequestType}, 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) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 69b62006391c..15f10f5e599d 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -21,9 +21,9 @@ var ( ) const ( - depositRequestType = 0 - withdrawalRequestType = 1 - consolidationRequestType = 2 + DepositRequestType = 0 + WithdrawalRequestType = 1 + ConsolidationRequestType = 2 ) func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) { @@ -39,7 +39,7 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ } prevTypeNum = requestType switch requestType { - case depositRequestType: + case DepositRequestType: if len(requestListInSSZBytes) < drSize { return nil, errors.New("invalid deposit requests length, requests should be at least the size of 1 request") } @@ -51,7 +51,7 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ return nil, err } requests.Deposits = drs - case withdrawalRequestType: + case WithdrawalRequestType: if len(requestListInSSZBytes) < wrSize { return nil, errors.New("invalid withdrawal request length, requests should be at least the size of 1 request") } @@ -63,7 +63,7 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ return nil, err } requests.Withdrawals = wrs - case consolidationRequestType: + case ConsolidationRequestType: if len(requestListInSSZBytes) < crSize { return nil, errors.New("invalid consolidations request length, requests should be at least the size of 1 request") } diff --git a/proto/engine/v1/export_test.go b/proto/engine/v1/export_test.go index d873a653865a..2466b7fa44d1 100644 --- a/proto/engine/v1/export_test.go +++ b/proto/engine/v1/export_test.go @@ -9,9 +9,3 @@ func MarshalItems[T sszMarshaler](items []T) ([]byte, error) { func UnmarshalItems[T sszUnmarshaler](data []byte, itemSize int, newItem func() T) ([]T, error) { return unmarshalItems(data, itemSize, newItem) } - -var ( - DepositRequestType = depositRequestType - WithdrawalRequestType = withdrawalRequestType - ConsolidationRequestType = consolidationRequestType -) From dc7eebd784ec68566e3a2010c244ebead677e24a Mon Sep 17 00:00:00 2001 From: james-prysm Date: Mon, 28 Oct 2024 10:30:56 -0500 Subject: [PATCH 16/19] missed some export changes --- proto/engine/v1/electra.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 15f10f5e599d..985b155260de 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -102,7 +102,7 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro if err != nil { return nil, errors.Wrap(err, "failed to marshal deposit requests") } - requestData := []byte{depositRequestType} + requestData := []byte{DepositRequestType} requestData = append(requestData, drBytes...) requestsData = append(requestsData, requestData) } @@ -111,7 +111,7 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro if err != nil { return nil, errors.Wrap(err, "failed to marshal withdrawal requests") } - requestData := []byte{withdrawalRequestType} + requestData := []byte{WithdrawalRequestType} requestData = append(requestData, wrBytes...) requestsData = append(requestsData, requestData) } @@ -120,7 +120,7 @@ func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, erro if err != nil { return nil, errors.Wrap(err, "failed to marshal consolidation requests") } - requestData := []byte{consolidationRequestType} + requestData := []byte{ConsolidationRequestType} requestData = append(requestData, crBytes...) requestsData = append(requestsData, requestData) } From c69ac6ce3bbe2bc92665a4704c9f6b765865a697 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Mon, 28 Oct 2024 11:54:22 -0500 Subject: [PATCH 17/19] adding more tests --- proto/engine/v1/electra_test.go | 86 ++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index 6619edebe839..e6aa2c783d1b 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -4,6 +4,7 @@ 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" @@ -12,7 +13,31 @@ import ( var depositRequestsSSZHex = "0x706b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000077630000000000000000000000000000000000000000000000000000000000007b00000000000000736967000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c801000000000000706b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000776300000000000000000000000000000000000000000000000000000000000090010000000000007369670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000" func TestGetDecodedExecutionRequests(t *testing.T) { - t.Run("Excluded requests still decode successfully", func(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" + @@ -69,6 +94,65 @@ func TestGetDecodedExecutionRequests(t *testing.T) { _, 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) { From cbdc3ad7090d6fbba5f041137e5201b608d2cd4c Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Mon, 28 Oct 2024 13:30:14 -0500 Subject: [PATCH 18/19] Update proto/engine/v1/electra.go Co-authored-by: Potuz --- proto/engine/v1/electra.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 985b155260de..5bcd9874540b 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -21,9 +21,9 @@ var ( ) const ( - DepositRequestType = 0 - WithdrawalRequestType = 1 - ConsolidationRequestType = 2 + DepositRequestType = iota + WithdrawalRequestType + ConsolidationRequestType ) func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) { From b077638d3d27048e0438746a9e923588abbf7b79 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Mon, 28 Oct 2024 13:33:54 -0500 Subject: [PATCH 19/19] reducing complexity of function based on feedback --- proto/engine/v1/electra.go | 54 +++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index 5bcd9874540b..628373fe1c7b 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -40,37 +40,19 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ prevTypeNum = requestType switch requestType { case DepositRequestType: - 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) - } - drs, err := unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) + drs, err := unmarshalDeposits(requestListInSSZBytes) if err != nil { return nil, err } requests.Deposits = drs case WithdrawalRequestType: - if len(requestListInSSZBytes) < wrSize { - return nil, errors.New("invalid withdrawal request length, requests should be at least the size of 1 request") - } - 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) - } - wrs, err := unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) + wrs, err := unmarshalWithdrawals(requestListInSSZBytes) if err != nil { return nil, err } requests.Withdrawals = wrs case ConsolidationRequestType: - 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) - } - crs, err := unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) + crs, err := unmarshalConsolidations(requestListInSSZBytes) if err != nil { return nil, err } @@ -82,6 +64,36 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ return requests, nil } +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) + } + return unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) +} + +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") + } + 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{} }) +} + +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) + } + return unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) +} + 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")