Skip to content

Commit e6ffc07

Browse files
james-prysmprestonvanloonpotuz
authored
Electra: exclude empty requests in requests list (#14580)
* implementing new decisions around exectuion requests * fixing test fixture * adding in more edge case checks and tests * changelog * fixing unsafe type cast * Update beacon-chain/execution/engine_client_test.go Co-authored-by: Preston Van Loon <[email protected]> * Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon <[email protected]> * Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon <[email protected]> * Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon <[email protected]> * Update proto/engine/v1/electra.go Co-authored-by: Preston Van Loon <[email protected]> * Update proto/engine/v1/electra_test.go Co-authored-by: Preston Van Loon <[email protected]> * Update proto/engine/v1/electra_test.go Co-authored-by: Preston Van Loon <[email protected]> * updating based on preston's feedback and adding more tests for edge cases * adding more edgecases, and unit tests * fixing tests * missed some export changes * adding more tests * Update proto/engine/v1/electra.go Co-authored-by: Potuz <[email protected]> * reducing complexity of function based on feedback --------- Co-authored-by: Preston Van Loon <[email protected]> Co-authored-by: Potuz <[email protected]>
1 parent 61c296e commit e6ffc07

File tree

4 files changed

+256
-33
lines changed

4 files changed

+256
-33
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
3737
- Use read only state when computing the active validator list.
3838
- Simplified `ExitedValidatorIndices`.
3939
- Simplified `EjectedValidatorIndices`.
40+
- `engine_newPayloadV4`,`engine_getPayloadV4` are changes due to new execution request serialization decisions, [PR](https://github.com/prysmaticlabs/prysm/pull/14580)
4041

4142
### Deprecated
4243

‎beacon-chain/execution/engine_client_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1769,7 +1769,9 @@ func fixturesStruct() *payloadFixtures {
17691769
Proofs: []hexutil.Bytes{[]byte("proof1"), []byte("proof2")},
17701770
Blobs: []hexutil.Bytes{{'a'}, {'b'}},
17711771
},
1772-
ExecutionRequests: []hexutil.Bytes{depositRequestBytes, withdrawalRequestBytes, consolidationRequestBytes},
1772+
ExecutionRequests: []hexutil.Bytes{append([]byte{pb.DepositRequestType}, depositRequestBytes...),
1773+
append([]byte{pb.WithdrawalRequestType}, withdrawalRequestBytes...),
1774+
append([]byte{pb.ConsolidationRequestType}, consolidationRequestBytes...)},
17731775
}
17741776
parent := bytesutil.PadTo([]byte("parentHash"), fieldparams.RootLength)
17751777
sha3Uncles := bytesutil.PadTo([]byte("sha3Uncles"), fieldparams.RootLength)

‎proto/engine/v1/electra.go‎

Lines changed: 98 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/ethereum/go-ethereum/common/hexutil"
77
"github.com/pkg/errors"
8+
"github.com/prysmaticlabs/prysm/v5/config/params"
89
)
910

1011
type ExecutionPayloadElectra = ExecutionPayloadDeneb
@@ -19,59 +20,124 @@ var (
1920
crSize = crExample.SizeSSZ()
2021
)
2122

22-
const LenExecutionRequestsElectra = 3
23+
const (
24+
DepositRequestType = iota
25+
WithdrawalRequestType
26+
ConsolidationRequestType
27+
)
2328

2429
func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) {
2530
requests := &ExecutionRequests{}
26-
27-
if len(ebe.ExecutionRequests) != LenExecutionRequestsElectra /* types of requests */ {
28-
return nil, errors.Errorf("invalid execution request size: %d", len(ebe.ExecutionRequests))
31+
var prevTypeNum uint8
32+
for i := range ebe.ExecutionRequests {
33+
requestType, requestListInSSZBytes, err := decodeExecutionRequest(ebe.ExecutionRequests[i])
34+
if err != nil {
35+
return nil, err
36+
}
37+
if prevTypeNum > requestType {
38+
return nil, errors.New("invalid execution request type order, requests should be in sorted order")
39+
}
40+
prevTypeNum = requestType
41+
switch requestType {
42+
case DepositRequestType:
43+
drs, err := unmarshalDeposits(requestListInSSZBytes)
44+
if err != nil {
45+
return nil, err
46+
}
47+
requests.Deposits = drs
48+
case WithdrawalRequestType:
49+
wrs, err := unmarshalWithdrawals(requestListInSSZBytes)
50+
if err != nil {
51+
return nil, err
52+
}
53+
requests.Withdrawals = wrs
54+
case ConsolidationRequestType:
55+
crs, err := unmarshalConsolidations(requestListInSSZBytes)
56+
if err != nil {
57+
return nil, err
58+
}
59+
requests.Consolidations = crs
60+
default:
61+
return nil, errors.Errorf("unsupported request type %d", requestType)
62+
}
2963
}
64+
return requests, nil
65+
}
3066

31-
// deposit requests
32-
drs, err := unmarshalItems(ebe.ExecutionRequests[0], drSize, func() *DepositRequest { return &DepositRequest{} })
33-
if err != nil {
34-
return nil, err
67+
func unmarshalDeposits(requestListInSSZBytes []byte) ([]*DepositRequest, error) {
68+
if len(requestListInSSZBytes) < drSize {
69+
return nil, errors.New("invalid deposit requests length, requests should be at least the size of 1 request")
70+
}
71+
if uint64(len(requestListInSSZBytes)) > uint64(drSize)*params.BeaconConfig().MaxDepositRequestsPerPayload {
72+
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)
3573
}
36-
requests.Deposits = drs
74+
return unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} })
75+
}
3776

38-
// withdrawal requests
39-
wrs, err := unmarshalItems(ebe.ExecutionRequests[1], wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} })
40-
if err != nil {
41-
return nil, err
77+
func unmarshalWithdrawals(requestListInSSZBytes []byte) ([]*WithdrawalRequest, error) {
78+
if len(requestListInSSZBytes) < wrSize {
79+
return nil, errors.New("invalid withdrawal request length, requests should be at least the size of 1 request")
4280
}
43-
requests.Withdrawals = wrs
81+
if uint64(len(requestListInSSZBytes)) > uint64(wrSize)*params.BeaconConfig().MaxWithdrawalRequestsPerPayload {
82+
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)
83+
}
84+
return unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} })
85+
}
4486

45-
// consolidation requests
46-
crs, err := unmarshalItems(ebe.ExecutionRequests[2], crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} })
47-
if err != nil {
48-
return nil, err
87+
func unmarshalConsolidations(requestListInSSZBytes []byte) ([]*ConsolidationRequest, error) {
88+
if len(requestListInSSZBytes) < crSize {
89+
return nil, errors.New("invalid consolidations request length, requests should be at least the size of 1 request")
90+
}
91+
if uint64(len(requestListInSSZBytes)) > uint64(crSize)*params.BeaconConfig().MaxConsolidationsRequestsPerPayload {
92+
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)
4993
}
50-
requests.Consolidations = crs
94+
return unmarshalItems(requestListInSSZBytes, crSize, func() *ConsolidationRequest { return &ConsolidationRequest{} })
95+
}
5196

52-
return requests, nil
97+
func decodeExecutionRequest(req []byte) (typ uint8, data []byte, err error) {
98+
if len(req) < 1 {
99+
return 0, nil, errors.New("invalid execution request, length less than 1")
100+
}
101+
return req[0], req[1:], nil
53102
}
54103

55104
func EncodeExecutionRequests(requests *ExecutionRequests) ([]hexutil.Bytes, error) {
56105
if requests == nil {
57106
return nil, errors.New("invalid execution requests")
58107
}
59108

60-
drBytes, err := marshalItems(requests.Deposits)
61-
if err != nil {
62-
return nil, errors.Wrap(err, "failed to marshal deposit requests")
63-
}
109+
requestsData := make([]hexutil.Bytes, 0)
64110

65-
wrBytes, err := marshalItems(requests.Withdrawals)
66-
if err != nil {
67-
return nil, errors.Wrap(err, "failed to marshal withdrawal requests")
111+
// request types MUST be in sorted order starting from 0
112+
if len(requests.Deposits) > 0 {
113+
drBytes, err := marshalItems(requests.Deposits)
114+
if err != nil {
115+
return nil, errors.Wrap(err, "failed to marshal deposit requests")
116+
}
117+
requestData := []byte{DepositRequestType}
118+
requestData = append(requestData, drBytes...)
119+
requestsData = append(requestsData, requestData)
68120
}
69-
70-
crBytes, err := marshalItems(requests.Consolidations)
71-
if err != nil {
72-
return nil, errors.Wrap(err, "failed to marshal consolidation requests")
121+
if len(requests.Withdrawals) > 0 {
122+
wrBytes, err := marshalItems(requests.Withdrawals)
123+
if err != nil {
124+
return nil, errors.Wrap(err, "failed to marshal withdrawal requests")
125+
}
126+
requestData := []byte{WithdrawalRequestType}
127+
requestData = append(requestData, wrBytes...)
128+
requestsData = append(requestsData, requestData)
129+
}
130+
if len(requests.Consolidations) > 0 {
131+
crBytes, err := marshalItems(requests.Consolidations)
132+
if err != nil {
133+
return nil, errors.Wrap(err, "failed to marshal consolidation requests")
134+
}
135+
requestData := []byte{ConsolidationRequestType}
136+
requestData = append(requestData, crBytes...)
137+
requestsData = append(requestsData, requestData)
73138
}
74-
return []hexutil.Bytes{drBytes, wrBytes, crBytes}, nil
139+
140+
return requestsData, nil
75141
}
76142

77143
type sszUnmarshaler interface {

‎proto/engine/v1/electra_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,167 @@ import (
44
"testing"
55

66
"github.com/ethereum/go-ethereum/common/hexutil"
7+
"github.com/prysmaticlabs/prysm/v5/config/params"
78
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
89
enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
910
"github.com/prysmaticlabs/prysm/v5/testing/require"
1011
)
1112

1213
var depositRequestsSSZHex = "0x706b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000077630000000000000000000000000000000000000000000000000000000000007b00000000000000736967000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c801000000000000706b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000776300000000000000000000000000000000000000000000000000000000000090010000000000007369670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000"
1314

15+
func TestGetDecodedExecutionRequests(t *testing.T) {
16+
t.Run("All requests decode successfully", func(t *testing.T) {
17+
depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
18+
"620000000000000000000000000000000000000000000000000000000000000000" +
19+
"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
20+
"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
21+
require.NoError(t, err)
22+
withdrawalRequestBytes, err := hexutil.Decode("0x6400000000000000000000000000000000000000" +
23+
"6500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040597307000000")
24+
require.NoError(t, err)
25+
consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
26+
"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
27+
"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
28+
require.NoError(t, err)
29+
ebe := &enginev1.ExecutionBundleElectra{
30+
ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...),
31+
append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes...),
32+
append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
33+
}
34+
requests, err := ebe.GetDecodedExecutionRequests()
35+
require.NoError(t, err)
36+
require.Equal(t, len(requests.Deposits), 1)
37+
require.Equal(t, len(requests.Withdrawals), 1)
38+
require.Equal(t, len(requests.Consolidations), 1)
39+
})
40+
t.Run("Excluded requests still decode successfully when one request is missing", func(t *testing.T) {
41+
depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
42+
"620000000000000000000000000000000000000000000000000000000000000000" +
43+
"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
44+
"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
45+
require.NoError(t, err)
46+
consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
47+
"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
48+
"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
49+
require.NoError(t, err)
50+
ebe := &enginev1.ExecutionBundleElectra{
51+
ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
52+
}
53+
requests, err := ebe.GetDecodedExecutionRequests()
54+
require.NoError(t, err)
55+
require.Equal(t, len(requests.Deposits), 1)
56+
require.Equal(t, len(requests.Withdrawals), 0)
57+
require.Equal(t, len(requests.Consolidations), 1)
58+
})
59+
t.Run("Decode execution requests should fail if ordering is not sorted", func(t *testing.T) {
60+
depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
61+
"620000000000000000000000000000000000000000000000000000000000000000" +
62+
"4059730700000063000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
63+
"00000000000000000000000000000000000000000000000000000000000000000000000000000000")
64+
require.NoError(t, err)
65+
consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
66+
"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
67+
"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
68+
require.NoError(t, err)
69+
ebe := &enginev1.ExecutionBundleElectra{
70+
ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...), append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...)},
71+
}
72+
_, err = ebe.GetDecodedExecutionRequests()
73+
require.ErrorContains(t, "invalid execution request type order", err)
74+
})
75+
t.Run("Requests should error if the request type is shorter than 1 byte", func(t *testing.T) {
76+
consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
77+
"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
78+
"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
79+
require.NoError(t, err)
80+
ebe := &enginev1.ExecutionBundleElectra{
81+
ExecutionRequests: [][]byte{append([]byte{}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
82+
}
83+
_, err = ebe.GetDecodedExecutionRequests()
84+
require.ErrorContains(t, "invalid execution request, length less than 1", err)
85+
})
86+
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) {
87+
consolidationRequestBytes, err := hexutil.Decode("0x6600000000000000000000000000000000000000" +
88+
"670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +
89+
"680000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
90+
require.NoError(t, err)
91+
ebe := &enginev1.ExecutionBundleElectra{
92+
ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)},
93+
}
94+
_, err = ebe.GetDecodedExecutionRequests()
95+
require.ErrorContains(t, "invalid deposit requests length", err)
96+
})
97+
t.Run("If deposit requests are over the max allowed per payload then we should error", func(t *testing.T) {
98+
requests := make([]*enginev1.DepositRequest, params.BeaconConfig().MaxDepositRequestsPerPayload+1)
99+
for i := range requests {
100+
requests[i] = &enginev1.DepositRequest{
101+
Pubkey: bytesutil.PadTo([]byte("pk"), 48),
102+
WithdrawalCredentials: bytesutil.PadTo([]byte("wc"), 32),
103+
Amount: 123,
104+
Signature: bytesutil.PadTo([]byte("sig"), 96),
105+
Index: 456,
106+
}
107+
}
108+
by, err := enginev1.MarshalItems(requests)
109+
require.NoError(t, err)
110+
ebe := &enginev1.ExecutionBundleElectra{
111+
ExecutionRequests: [][]byte{
112+
append([]byte{uint8(enginev1.DepositRequestType)}, by...),
113+
},
114+
}
115+
_, err = ebe.GetDecodedExecutionRequests()
116+
require.ErrorContains(t, "invalid deposit requests length, requests should not be more than the max per payload", err)
117+
})
118+
t.Run("If withdrawal requests are over the max allowed per payload then we should error", func(t *testing.T) {
119+
requests := make([]*enginev1.WithdrawalRequest, params.BeaconConfig().MaxWithdrawalRequestsPerPayload+1)
120+
for i := range requests {
121+
requests[i] = &enginev1.WithdrawalRequest{
122+
SourceAddress: bytesutil.PadTo([]byte("sa"), 20),
123+
ValidatorPubkey: bytesutil.PadTo([]byte("pk"), 48),
124+
Amount: 55555,
125+
}
126+
}
127+
by, err := enginev1.MarshalItems(requests)
128+
require.NoError(t, err)
129+
ebe := &enginev1.ExecutionBundleElectra{
130+
ExecutionRequests: [][]byte{
131+
append([]byte{uint8(enginev1.WithdrawalRequestType)}, by...),
132+
},
133+
}
134+
_, err = ebe.GetDecodedExecutionRequests()
135+
require.ErrorContains(t, "invalid withdrawal requests length, requests should not be more than the max per payload", err)
136+
})
137+
t.Run("If consolidation requests are over the max allowed per payload then we should error", func(t *testing.T) {
138+
requests := make([]*enginev1.ConsolidationRequest, params.BeaconConfig().MaxConsolidationsRequestsPerPayload+1)
139+
for i := range requests {
140+
requests[i] = &enginev1.ConsolidationRequest{
141+
SourceAddress: bytesutil.PadTo([]byte("sa"), 20),
142+
SourcePubkey: bytesutil.PadTo([]byte("pk"), 48),
143+
TargetPubkey: bytesutil.PadTo([]byte("pk"), 48),
144+
}
145+
}
146+
by, err := enginev1.MarshalItems(requests)
147+
require.NoError(t, err)
148+
ebe := &enginev1.ExecutionBundleElectra{
149+
ExecutionRequests: [][]byte{
150+
append([]byte{uint8(enginev1.ConsolidationRequestType)}, by...),
151+
},
152+
}
153+
_, err = ebe.GetDecodedExecutionRequests()
154+
require.ErrorContains(t, "invalid consolidation requests length, requests should not be more than the max per payload", err)
155+
})
156+
}
157+
158+
func TestEncodeExecutionRequests(t *testing.T) {
159+
t.Run("Empty execution requests should return an empty response and not nil", func(t *testing.T) {
160+
ebe := &enginev1.ExecutionRequests{}
161+
b, err := enginev1.EncodeExecutionRequests(ebe)
162+
require.NoError(t, err)
163+
require.NotNil(t, b)
164+
require.Equal(t, len(b), 0)
165+
})
166+
}
167+
14168
func TestUnmarshalItems_OK(t *testing.T) {
15169
drb, err := hexutil.Decode(depositRequestsSSZHex)
16170
require.NoError(t, err)

0 commit comments

Comments
 (0)