From 8364226b689d4bb23a97988cc074fbe872b0aa42 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Fri, 19 Jul 2024 12:07:30 -0500 Subject: [PATCH] payload electra cloning (#14239) * poc for payload electra cloning * partial fixes * fixing build * addressing kasey's comment * forgot to unexport interface * making test more generic * making fuzzing slightly more robust * renaming based on kasey's comment * using fuzz test in same package to avoid exporting interface * fixing build --- proto/engine/v1/BUILD.bazel | 6 +- proto/engine/v1/execution_engine.go | 92 +++++++++++++ proto/engine/v1/execution_engine_fuzz_test.go | 30 +++++ proto/engine/v1/export_test.go | 3 + proto/prysm/v1alpha1/cloners.go | 125 +++--------------- proto/prysm/v1alpha1/cloners_test.go | 47 ------- 6 files changed, 146 insertions(+), 157 deletions(-) create mode 100644 proto/engine/v1/execution_engine.go create mode 100644 proto/engine/v1/execution_engine_fuzz_test.go create mode 100644 proto/engine/v1/export_test.go diff --git a/proto/engine/v1/BUILD.bazel b/proto/engine/v1/BUILD.bazel index bc9138b82dc5..a7c1615015f8 100644 --- a/proto/engine/v1/BUILD.bazel +++ b/proto/engine/v1/BUILD.bazel @@ -75,6 +75,7 @@ go_proto_library( go_library( name = "go_default_library", srcs = [ + "execution_engine.go", "json_marshal_unmarshal.go", ":ssz_generated_files", # keep ], @@ -121,15 +122,18 @@ ssz_proto_files( go_test( name = "go_default_test", srcs = [ + "export_test.go", + "execution_engine_fuzz_test.go", "json_marshal_unmarshal_test.go", ], + embed = [":go_default_library"], deps = [ - ":go_default_library", "//config/fieldparams:go_default_library", "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//encoding/bytesutil:go_default_library", "//testing/require:go_default_library", + "@com_github_google_gofuzz//:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library", "@com_github_ethereum_go_ethereum//core/types:go_default_library", diff --git a/proto/engine/v1/execution_engine.go b/proto/engine/v1/execution_engine.go new file mode 100644 index 000000000000..fb03e21af6ea --- /dev/null +++ b/proto/engine/v1/execution_engine.go @@ -0,0 +1,92 @@ +package enginev1 + +import "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" + +type copier[T any] interface { + Copy() T +} + +func copySlice[T any, C copier[T]](original []C) []T { + // Create a new slice with the same length as the original + newSlice := make([]T, len(original)) + for i := 0; i < len(newSlice); i++ { + newSlice[i] = original[i].Copy() + } + return newSlice +} + +func (w *Withdrawal) Copy() *Withdrawal { + if w == nil { + return nil + } + + return &Withdrawal{ + Index: w.Index, + ValidatorIndex: w.ValidatorIndex, + Address: bytesutil.SafeCopyBytes(w.Address), + Amount: w.Amount, + } +} + +func (d *DepositRequest) Copy() *DepositRequest { + if d == nil { + return nil + } + return &DepositRequest{ + Pubkey: bytesutil.SafeCopyBytes(d.Pubkey), + WithdrawalCredentials: bytesutil.SafeCopyBytes(d.WithdrawalCredentials), + Amount: d.Amount, + Signature: bytesutil.SafeCopyBytes(d.Signature), + Index: d.Index, + } +} + +func (wr *WithdrawalRequest) Copy() *WithdrawalRequest { + if wr == nil { + return nil + } + return &WithdrawalRequest{ + SourceAddress: bytesutil.SafeCopyBytes(wr.SourceAddress), + ValidatorPubkey: bytesutil.SafeCopyBytes(wr.ValidatorPubkey), + Amount: wr.Amount, + } +} + +func (cr *ConsolidationRequest) Copy() *ConsolidationRequest { + if cr == nil { + return nil + } + return &ConsolidationRequest{ + SourceAddress: bytesutil.SafeCopyBytes(cr.SourceAddress), + SourcePubkey: bytesutil.SafeCopyBytes(cr.SourcePubkey), + TargetPubkey: bytesutil.SafeCopyBytes(cr.TargetPubkey), + } +} + +func (payload *ExecutionPayloadElectra) Copy() *ExecutionPayloadElectra { + if payload == nil { + return nil + } + return &ExecutionPayloadElectra{ + ParentHash: bytesutil.SafeCopyBytes(payload.ParentHash), + FeeRecipient: bytesutil.SafeCopyBytes(payload.FeeRecipient), + StateRoot: bytesutil.SafeCopyBytes(payload.StateRoot), + ReceiptsRoot: bytesutil.SafeCopyBytes(payload.ReceiptsRoot), + LogsBloom: bytesutil.SafeCopyBytes(payload.LogsBloom), + PrevRandao: bytesutil.SafeCopyBytes(payload.PrevRandao), + BlockNumber: payload.BlockNumber, + GasLimit: payload.GasLimit, + GasUsed: payload.GasUsed, + Timestamp: payload.Timestamp, + ExtraData: bytesutil.SafeCopyBytes(payload.ExtraData), + BaseFeePerGas: bytesutil.SafeCopyBytes(payload.BaseFeePerGas), + BlockHash: bytesutil.SafeCopyBytes(payload.BlockHash), + Transactions: bytesutil.SafeCopy2dBytes(payload.Transactions), + Withdrawals: copySlice(payload.Withdrawals), + BlobGasUsed: payload.BlobGasUsed, + ExcessBlobGas: payload.ExcessBlobGas, + DepositRequests: copySlice(payload.DepositRequests), + WithdrawalRequests: copySlice(payload.WithdrawalRequests), + ConsolidationRequests: copySlice(payload.ConsolidationRequests), + } +} diff --git a/proto/engine/v1/execution_engine_fuzz_test.go b/proto/engine/v1/execution_engine_fuzz_test.go new file mode 100644 index 000000000000..28d4fb9fa964 --- /dev/null +++ b/proto/engine/v1/execution_engine_fuzz_test.go @@ -0,0 +1,30 @@ +package enginev1_test + +import ( + "fmt" + "testing" + + fuzz "github.com/google/gofuzz" + enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" + "github.com/prysmaticlabs/prysm/v5/testing/require" +) + +func TestCopyExecutionPayload_Fuzz(t *testing.T) { + fuzzCopies(t, &enginev1.ExecutionPayloadElectra{}) +} + +func fuzzCopies[T any, C enginev1.Copier[T]](t *testing.T, obj C) { + fuzzer := fuzz.NewWithSeed(0) + amount := 1000 + t.Run(fmt.Sprintf("%T", obj), func(t *testing.T) { + for i := 0; i < amount; i++ { + fuzzer.Fuzz(obj) // Populate thing with random values + got := obj.Copy() + require.DeepEqual(t, obj, got) + // check shallow copy working + fuzzer.Fuzz(got) + require.DeepNotEqual(t, obj, got) + // TODO: think of deeper not equal fuzzing + } + }) +} diff --git a/proto/engine/v1/export_test.go b/proto/engine/v1/export_test.go new file mode 100644 index 000000000000..17f4cde64607 --- /dev/null +++ b/proto/engine/v1/export_test.go @@ -0,0 +1,3 @@ +package enginev1 + +type Copier[T any] copier[T] diff --git a/proto/prysm/v1alpha1/cloners.go b/proto/prysm/v1alpha1/cloners.go index 1727c0ab0af0..04009820059e 100644 --- a/proto/prysm/v1alpha1/cloners.go +++ b/proto/prysm/v1alpha1/cloners.go @@ -705,7 +705,7 @@ func CopyExecutionPayloadCapella(payload *enginev1.ExecutionPayloadCapella) *eng BaseFeePerGas: bytesutil.SafeCopyBytes(payload.BaseFeePerGas), BlockHash: bytesutil.SafeCopyBytes(payload.BlockHash), Transactions: bytesutil.SafeCopy2dBytes(payload.Transactions), - Withdrawals: CopyWithdrawalSlice(payload.Withdrawals), + Withdrawals: copySlice(payload.Withdrawals), } } @@ -800,33 +800,6 @@ func CopyBlindedBeaconBlockBodyBellatrix(body *BlindedBeaconBlockBodyBellatrix) } } -// CopyWithdrawalSlice copies the provided slice of withdrawals. -func CopyWithdrawalSlice(withdrawals []*enginev1.Withdrawal) []*enginev1.Withdrawal { - if withdrawals == nil { - return nil - } - - res := make([]*enginev1.Withdrawal, len(withdrawals)) - for i := 0; i < len(res); i++ { - res[i] = CopyWithdrawal(withdrawals[i]) - } - return res -} - -// CopyWithdrawal copies the provided withdrawal object. -func CopyWithdrawal(withdrawal *enginev1.Withdrawal) *enginev1.Withdrawal { - if withdrawal == nil { - return nil - } - - return &enginev1.Withdrawal{ - Index: withdrawal.Index, - ValidatorIndex: withdrawal.ValidatorIndex, - Address: bytesutil.SafeCopyBytes(withdrawal.Address), - Amount: withdrawal.Amount, - } -} - func CopyBLSToExecutionChanges(changes []*SignedBLSToExecutionChange) []*SignedBLSToExecutionChange { if changes == nil { return nil @@ -944,7 +917,7 @@ func CopyExecutionPayloadDeneb(payload *enginev1.ExecutionPayloadDeneb) *enginev BaseFeePerGas: bytesutil.SafeCopyBytes(payload.BaseFeePerGas), BlockHash: bytesutil.SafeCopyBytes(payload.BlockHash), Transactions: bytesutil.SafeCopy2dBytes(payload.Transactions), - Withdrawals: CopyWithdrawalSlice(payload.Withdrawals), + Withdrawals: copySlice(payload.Withdrawals), BlobGasUsed: payload.BlobGasUsed, ExcessBlobGas: payload.ExcessBlobGas, } @@ -990,91 +963,12 @@ func CopyBeaconBlockBodyElectra(body *BeaconBlockBodyElectra) *BeaconBlockBodyEl Deposits: CopyDeposits(body.Deposits), VoluntaryExits: CopySignedVoluntaryExits(body.VoluntaryExits), SyncAggregate: CopySyncAggregate(body.SyncAggregate), - ExecutionPayload: CopyExecutionPayloadElectra(body.ExecutionPayload), + ExecutionPayload: body.ExecutionPayload.Copy(), BlsToExecutionChanges: CopyBLSToExecutionChanges(body.BlsToExecutionChanges), BlobKzgCommitments: CopyBlobKZGs(body.BlobKzgCommitments), } } -// CopyExecutionPayloadElectra copies the provided execution payload. -func CopyExecutionPayloadElectra(payload *enginev1.ExecutionPayloadElectra) *enginev1.ExecutionPayloadElectra { - if payload == nil { - return nil - } - return &enginev1.ExecutionPayloadElectra{ - ParentHash: bytesutil.SafeCopyBytes(payload.ParentHash), - FeeRecipient: bytesutil.SafeCopyBytes(payload.FeeRecipient), - StateRoot: bytesutil.SafeCopyBytes(payload.StateRoot), - ReceiptsRoot: bytesutil.SafeCopyBytes(payload.ReceiptsRoot), - LogsBloom: bytesutil.SafeCopyBytes(payload.LogsBloom), - PrevRandao: bytesutil.SafeCopyBytes(payload.PrevRandao), - BlockNumber: payload.BlockNumber, - GasLimit: payload.GasLimit, - GasUsed: payload.GasUsed, - Timestamp: payload.Timestamp, - ExtraData: bytesutil.SafeCopyBytes(payload.ExtraData), - BaseFeePerGas: bytesutil.SafeCopyBytes(payload.BaseFeePerGas), - BlockHash: bytesutil.SafeCopyBytes(payload.BlockHash), - Transactions: bytesutil.SafeCopy2dBytes(payload.Transactions), - Withdrawals: CopyWithdrawalSlice(payload.Withdrawals), - BlobGasUsed: payload.BlobGasUsed, - ExcessBlobGas: payload.ExcessBlobGas, - DepositRequests: CopyDepositRequests(payload.DepositRequests), - WithdrawalRequests: CopyWithdrawalRequests(payload.WithdrawalRequests), - ConsolidationRequests: CopyConsolidationRequests(payload.ConsolidationRequests), - } -} - -func CopyDepositRequests(dr []*enginev1.DepositRequest) []*enginev1.DepositRequest { - if dr == nil { - return nil - } - - newDr := make([]*enginev1.DepositRequest, len(dr)) - for i, d := range dr { - newDr[i] = &enginev1.DepositRequest{ - Pubkey: bytesutil.SafeCopyBytes(d.Pubkey), - WithdrawalCredentials: bytesutil.SafeCopyBytes(d.WithdrawalCredentials), - Amount: d.Amount, - Signature: bytesutil.SafeCopyBytes(d.Signature), - Index: d.Index, - } - } - return newDr -} - -func CopyWithdrawalRequests(wr []*enginev1.WithdrawalRequest) []*enginev1.WithdrawalRequest { - if wr == nil { - return nil - } - newWr := make([]*enginev1.WithdrawalRequest, len(wr)) - for i, w := range wr { - newWr[i] = &enginev1.WithdrawalRequest{ - SourceAddress: bytesutil.SafeCopyBytes(w.SourceAddress), - ValidatorPubkey: bytesutil.SafeCopyBytes(w.ValidatorPubkey), - Amount: w.Amount, - } - } - - return newWr -} - -func CopyConsolidationRequests(cr []*enginev1.ConsolidationRequest) []*enginev1.ConsolidationRequest { - if cr == nil { - return nil - } - newCr := make([]*enginev1.ConsolidationRequest, len(cr)) - for i, w := range cr { - newCr[i] = &enginev1.ConsolidationRequest{ - SourceAddress: bytesutil.SafeCopyBytes(w.SourceAddress), - SourcePubkey: bytesutil.SafeCopyBytes(w.SourcePubkey), - TargetPubkey: bytesutil.SafeCopyBytes(w.TargetPubkey), - } - } - - return newCr -} - func CopyExecutionPayloadHeaderElectra(payload *enginev1.ExecutionPayloadHeaderElectra) *enginev1.ExecutionPayloadHeaderElectra { if payload == nil { return nil @@ -1161,3 +1055,16 @@ func CopyPendingBalanceDeposits(pbd []*PendingBalanceDeposit) []*PendingBalanceD } return newPbd } + +type cloneable[T any] interface { + Copy() T +} + +func copySlice[T any, C cloneable[T]](original []C) []T { + // Create a new slice with the same length as the original + newSlice := make([]T, len(original)) + for i := 0; i < len(newSlice); i++ { + newSlice[i] = original[i].Copy() + } + return newSlice +} diff --git a/proto/prysm/v1alpha1/cloners_test.go b/proto/prysm/v1alpha1/cloners_test.go index 5383970c9dec..22a348f31660 100644 --- a/proto/prysm/v1alpha1/cloners_test.go +++ b/proto/prysm/v1alpha1/cloners_test.go @@ -527,26 +527,6 @@ func bytes(length int) []byte { return b } -func TestCopyWithdrawals(t *testing.T) { - ws := genWithdrawals(10) - - got := v1alpha1.CopyWithdrawalSlice(ws) - if !reflect.DeepEqual(got, ws) { - t.Errorf("TestCopyWithdrawals() = %v, want %v", got, ws) - } - assert.NotEmpty(t, got, "Copied withdrawals have empty fields") -} - -func TestCopyWithdrawal(t *testing.T) { - w := genWithdrawal() - - got := v1alpha1.CopyWithdrawal(w) - if !reflect.DeepEqual(got, w) { - t.Errorf("TestCopyWithdrawal() = %v, want %v", got, w) - } - assert.NotEmpty(t, got, "Copied withdrawal has empty fields") -} - func TestCopyBLSToExecutionChanges(t *testing.T) { changes := genBLSToExecutionChanges(10) @@ -658,33 +638,6 @@ func TestCopyBeaconBlockBodyElectra(t *testing.T) { } } -func TestCopyExecutionPayloadElectra(t *testing.T) { - p := genExecutionPayloadElectra() - - got := v1alpha1.CopyExecutionPayloadElectra(p) - if !reflect.DeepEqual(got, p) { - t.Errorf("TestCopyExecutionPayloadElectra() = %v, want %v", got, p) - } -} - -func TestCopyDepositRequests(t *testing.T) { - drs := genDepositRequests(10) - - got := v1alpha1.CopyDepositRequests(drs) - if !reflect.DeepEqual(got, drs) { - t.Errorf("TestCopyDepositRequests() = %v, want %v", got, drs) - } -} - -func TestCopyWithdrawalRequests(t *testing.T) { - wrs := genWithdrawalRequests(10) - - got := v1alpha1.CopyWithdrawalRequests(wrs) - if !reflect.DeepEqual(got, wrs) { - t.Errorf("TestCopyWithdrawalRequests() = %v, want %v", got, wrs) - } -} - func TestCopyExecutionPayloadHeaderElectra(t *testing.T) { p := genExecutionPayloadHeaderElectra()