Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

payload electra cloning #14239

Merged
merged 12 commits into from
Jul 19, 2024
6 changes: 5 additions & 1 deletion proto/engine/v1/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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
],
Expand Down Expand Up @@ -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",
Expand Down
92 changes: 92 additions & 0 deletions proto/engine/v1/execution_engine.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package enginev1

import "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"

type copier[T any] interface {
Copy() T
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to approve so you can move on, but my nitpick is that we should name the interface after the method (Cloneable does not follow intuitively from Copy - the reflexive naming here should be something more like Copyer). If you want to address that now I'd appreciate it and give a fast follow-up approval, or you can do it in the follow-up PRs. Also, should the interface be exported?


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),
}
}
30 changes: 30 additions & 0 deletions proto/engine/v1/execution_engine_fuzz_test.go
Original file line number Diff line number Diff line change
@@ -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
}
})
}
3 changes: 3 additions & 0 deletions proto/engine/v1/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package enginev1

type Copier[T any] copier[T]
125 changes: 16 additions & 109 deletions proto/prysm/v1alpha1/cloners.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
47 changes: 0 additions & 47 deletions proto/prysm/v1alpha1/cloners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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()

Expand Down
Loading