From e0e735470809df29c5404f64102ffbae5a574e0a Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Thu, 20 Feb 2025 08:31:02 -0600 Subject: [PATCH] improving the error messages for execution request deserialization (#14962) * improving the error messages for execution request deserialization * changelog --- ...-prysm_improve-execution-request-errors.md | 3 +++ proto/engine/v1/electra.go | 21 +++++++++++-------- proto/engine/v1/electra_test.go | 8 +++---- 3 files changed, 19 insertions(+), 13 deletions(-) create mode 100644 changelog/james-prysm_improve-execution-request-errors.md diff --git a/changelog/james-prysm_improve-execution-request-errors.md b/changelog/james-prysm_improve-execution-request-errors.md new file mode 100644 index 0000000000..ab35eeb9b6 --- /dev/null +++ b/changelog/james-prysm_improve-execution-request-errors.md @@ -0,0 +1,3 @@ +### Changed + +- execution requests errors on ssz length have been improved \ No newline at end of file diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index f46a356a31..24b81dc053 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -63,30 +63,33 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ func unmarshalDeposits(requestListInSSZBytes []byte) ([]*DepositRequest, error) { if len(requestListInSSZBytes) < drSize { - return nil, fmt.Errorf("invalid deposit requests length, requests should be at least the size of %d", drSize) + return nil, fmt.Errorf("invalid deposit requests SSZ size, got %d expected at least %d", len(requestListInSSZBytes), drSize) } - 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) + maxSSZsize := uint64(drSize) * params.BeaconConfig().MaxDepositRequestsPerPayload + if uint64(len(requestListInSSZBytes)) > maxSSZsize { + return nil, fmt.Errorf("invalid deposit requests SSZ size, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), maxSSZsize) } return unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) } func unmarshalWithdrawals(requestListInSSZBytes []byte) ([]*WithdrawalRequest, error) { if len(requestListInSSZBytes) < wrSize { - return nil, fmt.Errorf("invalid withdrawal requests length, requests should be at least the size of %d", wrSize) + return nil, fmt.Errorf("invalid withdrawal requests SSZ size, got %d expected at least %d", len(requestListInSSZBytes), wrSize) } - 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) + maxSSZsize := uint64(wrSize) * params.BeaconConfig().MaxWithdrawalRequestsPerPayload + if uint64(len(requestListInSSZBytes)) > maxSSZsize { + return nil, fmt.Errorf("invalid withdrawal requests SSZ size, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), maxSSZsize) } return unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) } func unmarshalConsolidations(requestListInSSZBytes []byte) ([]*ConsolidationRequest, error) { if len(requestListInSSZBytes) < crSize { - return nil, fmt.Errorf("invalid consolidation requests length, requests should be at least the size of %d", crSize) + return nil, fmt.Errorf("invalid consolidation requests SSZ size, got %d expected at least %d", len(requestListInSSZBytes), crSize) } - 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) + maxSSZsize := uint64(crSize) * params.BeaconConfig().MaxConsolidationsRequestsPerPayload + if uint64(len(requestListInSSZBytes)) > maxSSZsize { + return nil, fmt.Errorf("invalid consolidation requests SSZ size, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), maxSSZsize) } return unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} }) } diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index bab9f3912b..f44aaca6d5 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -122,7 +122,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { 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) + require.ErrorContains(t, "invalid deposit requests SSZ size", 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) @@ -143,7 +143,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { }, } _, err = ebe.GetDecodedExecutionRequests() - require.ErrorContains(t, "invalid deposit requests length, requests should not be more than the max per payload", err) + require.ErrorContains(t, "invalid deposit requests SSZ size, 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) @@ -162,7 +162,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { }, } _, err = ebe.GetDecodedExecutionRequests() - require.ErrorContains(t, "invalid withdrawal requests length, requests should not be more than the max per payload", err) + require.ErrorContains(t, "invalid withdrawal requests SSZ size, 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) @@ -181,7 +181,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { }, } _, err = ebe.GetDecodedExecutionRequests() - require.ErrorContains(t, "invalid consolidation requests length, requests should not be more than the max per payload", err) + require.ErrorContains(t, "invalid consolidation requests SSZ size, requests should not be more than the max per payload", err) }) }