From 5b0c293880b712e51ea8d8e3d094fa5f1750b6d9 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Mon, 20 May 2024 16:54:20 -0500 Subject: [PATCH 01/16] engine GetPayload returns entire response --- beacon-chain/execution/engine_client.go | 73 ++++++------------- beacon-chain/execution/engine_client_test.go | 57 +++++++-------- beacon-chain/execution/testing/BUILD.bazel | 1 - .../execution/testing/mock_engine_client.go | 40 +--------- .../validator/proposer_bellatrix_test.go | 44 ++++++++--- .../validator/proposer_execution_payload.go | 38 +++++----- .../proposer_execution_payload_test.go | 14 +++- .../prysm/v1alpha1/validator/proposer_test.go | 28 ++++--- consensus-types/blocks/BUILD.bazel | 1 + consensus-types/blocks/execution.go | 9 +++ consensus-types/blocks/get_payload.go | 56 ++++++++++++++ consensus-types/blocks/get_payload_test.go | 22 ++++++ consensus-types/primitives/wei.go | 2 +- math/BUILD.bazel | 1 - math/math_helper_test.go | 27 ------- 15 files changed, 227 insertions(+), 186 deletions(-) create mode 100644 consensus-types/blocks/get_payload.go create mode 100644 consensus-types/blocks/get_payload_test.go diff --git a/beacon-chain/execution/engine_client.go b/beacon-chain/execution/engine_client.go index 8ed049bce9fc..312eabfab8d2 100644 --- a/beacon-chain/execution/engine_client.go +++ b/beacon-chain/execution/engine_client.go @@ -118,7 +118,7 @@ type EngineCaller interface { ForkchoiceUpdated( ctx context.Context, state *pb.ForkchoiceState, attrs payloadattribute.Attributer, ) (*pb.PayloadIDBytes, []byte, error) - GetPayload(ctx context.Context, payloadId [8]byte, slot primitives.Slot) (interfaces.ExecutionData, *pb.BlobsBundle, bool, error) + GetPayload(ctx context.Context, payloadId [8]byte, slot primitives.Slot) (*blocks.GetPayloadResponse, error) ExecutionBlockByHash(ctx context.Context, hash common.Hash, withTxs bool) (*pb.ExecutionBlock, error) GetTerminalBlockHash(ctx context.Context, transitionTime uint64) ([]byte, bool, error) } @@ -266,9 +266,23 @@ func (s *Service) ForkchoiceUpdated( } } +func getPayloadMethodAndMessage(slot primitives.Slot) (string, proto.Message) { + pe := slots.ToEpoch(slot) + if pe >= params.BeaconConfig().ElectraForkEpoch { + return GetPayloadMethodV4, &pb.ExecutionPayloadElectraWithValueAndBlobsBundle{} + } + if pe >= params.BeaconConfig().DenebForkEpoch { + return GetPayloadMethodV3, &pb.ExecutionPayloadDenebWithValueAndBlobsBundle{} + } + if pe >= params.BeaconConfig().CapellaForkEpoch { + return GetPayloadMethodV2, &pb.ExecutionPayloadCapellaWithValue{} + } + return GetPayloadMethod, &pb.ExecutionPayload{} +} + // GetPayload calls the engine_getPayloadVX method via JSON-RPC. // It returns the execution data as well as the blobs bundle. -func (s *Service) GetPayload(ctx context.Context, payloadId [8]byte, slot primitives.Slot) (interfaces.ExecutionData, *pb.BlobsBundle, bool, error) { +func (s *Service) GetPayload(ctx context.Context, payloadId [8]byte, slot primitives.Slot) (*blocks.GetPayloadResponse, error) { ctx, span := trace.StartSpan(ctx, "powchain.engine-api-client.GetPayload") defer span.End() start := time.Now() @@ -276,59 +290,16 @@ func (s *Service) GetPayload(ctx context.Context, payloadId [8]byte, slot primit getPayloadLatency.Observe(float64(time.Since(start).Milliseconds())) }() - d := time.Now().Add(defaultEngineTimeout) - ctx, cancel := context.WithDeadline(ctx, d) - defer cancel() - - if slots.ToEpoch(slot) >= params.BeaconConfig().ElectraForkEpoch { - result := &pb.ExecutionPayloadElectraWithValueAndBlobsBundle{} - err := s.rpcClient.CallContext(ctx, result, GetPayloadMethodV4, pb.PayloadIDBytes(payloadId)) - if err != nil { - return nil, nil, false, handleRPCError(err) - } - ed, err := blocks.WrappedExecutionPayloadElectra(result.Payload, blocks.PayloadValueToWei(result.Value)) - if err != nil { - return nil, nil, false, err - } - return ed, result.BlobsBundle, result.ShouldOverrideBuilder, nil - } - - if slots.ToEpoch(slot) >= params.BeaconConfig().DenebForkEpoch { - result := &pb.ExecutionPayloadDenebWithValueAndBlobsBundle{} - err := s.rpcClient.CallContext(ctx, result, GetPayloadMethodV3, pb.PayloadIDBytes(payloadId)) - if err != nil { - return nil, nil, false, handleRPCError(err) - } - ed, err := blocks.WrappedExecutionPayloadDeneb(result.Payload, blocks.PayloadValueToWei(result.Value)) - if err != nil { - return nil, nil, false, err - } - return ed, result.BlobsBundle, result.ShouldOverrideBuilder, nil - } - - if slots.ToEpoch(slot) >= params.BeaconConfig().CapellaForkEpoch { - result := &pb.ExecutionPayloadCapellaWithValue{} - err := s.rpcClient.CallContext(ctx, result, GetPayloadMethodV2, pb.PayloadIDBytes(payloadId)) - if err != nil { - return nil, nil, false, handleRPCError(err) - } - ed, err := blocks.WrappedExecutionPayloadCapella(result.Payload, blocks.PayloadValueToWei(result.Value)) - if err != nil { - return nil, nil, false, err - } - return ed, nil, false, nil - } - - result := &pb.ExecutionPayload{} - err := s.rpcClient.CallContext(ctx, result, GetPayloadMethod, pb.PayloadIDBytes(payloadId)) + method, result := getPayloadMethodAndMessage(slot) + err := s.rpcClient.CallContext(ctx, result, method, pb.PayloadIDBytes(payloadId)) if err != nil { - return nil, nil, false, handleRPCError(err) + return nil, handleRPCError(err) } - ed, err := blocks.WrappedExecutionPayload(result) + res, err := blocks.NewGetPayloadResponse(result) if err != nil { - return nil, nil, false, err + return nil, err } - return ed, nil, false, nil + return res, nil } func (s *Service) ExchangeCapabilities(ctx context.Context) ([]string, error) { diff --git a/beacon-chain/execution/engine_client_test.go b/beacon-chain/execution/engine_client_test.go index 2ac1f8d3c896..4336c6277025 100644 --- a/beacon-chain/execution/engine_client_test.go +++ b/beacon-chain/execution/engine_client_test.go @@ -25,6 +25,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" payloadattribute "github.com/prysmaticlabs/prysm/v5/consensus-types/payload-attribute" + "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" pb "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" "github.com/prysmaticlabs/prysm/v5/runtime/version" @@ -74,10 +75,10 @@ func TestClient_IPC(t *testing.T) { want, ok := fix["ExecutionPayload"].(*pb.ExecutionPayload) require.Equal(t, true, ok) payloadId := [8]byte{1} - resp, _, override, err := srv.GetPayload(ctx, payloadId, 1) + resp, err := srv.GetPayload(ctx, payloadId, 1) require.NoError(t, err) - require.Equal(t, false, override) - pbs := resp.Proto() + require.Equal(t, false, resp.OverrideBuilder) + pbs := resp.ExecutionData.Proto() resPb, ok := pbs.(*pb.ExecutionPayload) require.Equal(t, true, ok) require.NoError(t, err) @@ -87,10 +88,10 @@ func TestClient_IPC(t *testing.T) { want, ok := fix["ExecutionPayloadCapellaWithValue"].(*pb.ExecutionPayloadCapellaWithValue) require.Equal(t, true, ok) payloadId := [8]byte{1} - resp, _, override, err := srv.GetPayload(ctx, payloadId, params.BeaconConfig().SlotsPerEpoch) + resp, err := srv.GetPayload(ctx, payloadId, params.BeaconConfig().SlotsPerEpoch) require.NoError(t, err) - require.Equal(t, false, override) - pbs := resp.Proto() + require.Equal(t, false, resp.OverrideBuilder) + pbs := resp.ExecutionData.Proto() resPb, ok := pbs.(*pb.ExecutionPayloadCapella) require.Equal(t, true, ok) require.DeepEqual(t, want, resPb) @@ -203,10 +204,10 @@ func TestClient_HTTP(t *testing.T) { client.rpcClient = rpcClient // We call the RPC method via HTTP and expect a proper result. - resp, _, override, err := client.GetPayload(ctx, payloadId, 1) + resp, err := client.GetPayload(ctx, payloadId, 1) require.NoError(t, err) - require.Equal(t, false, override) - pbs := resp.Proto() + require.Equal(t, false, resp.OverrideBuilder) + pbs := resp.ExecutionData.Proto() pbStruct, ok := pbs.(*pb.ExecutionPayload) require.Equal(t, true, ok) require.DeepEqual(t, want, pbStruct) @@ -249,10 +250,10 @@ func TestClient_HTTP(t *testing.T) { client.rpcClient = rpcClient // We call the RPC method via HTTP and expect a proper result. - resp, _, override, err := client.GetPayload(ctx, payloadId, params.BeaconConfig().SlotsPerEpoch) + resp, err := client.GetPayload(ctx, payloadId, params.BeaconConfig().SlotsPerEpoch) require.NoError(t, err) - require.Equal(t, false, override) - pbs := resp.Proto() + require.Equal(t, false, resp.OverrideBuilder) + pbs := resp.ExecutionData.Proto() ep, ok := pbs.(*pb.ExecutionPayloadCapella) require.Equal(t, true, ok) require.DeepEqual(t, want.ExecutionPayload.BlockHash.Bytes(), ep.BlockHash) @@ -262,9 +263,7 @@ func TestClient_HTTP(t *testing.T) { require.DeepEqual(t, want.ExecutionPayload.PrevRandao.Bytes(), ep.PrevRandao) require.DeepEqual(t, want.ExecutionPayload.ParentHash.Bytes(), ep.ParentHash) - v, err := resp.ValueInGwei() - require.NoError(t, err) - require.Equal(t, uint64(1236), v) + require.Equal(t, primitives.Gwei(1236), primitives.WeiToGwei(resp.Bid)) }) t.Run(GetPayloadMethodV3, func(t *testing.T) { payloadId := [8]byte{1} @@ -304,22 +303,22 @@ func TestClient_HTTP(t *testing.T) { client.rpcClient = rpcClient // We call the RPC method via HTTP and expect a proper result. - resp, blobsBundle, override, err := client.GetPayload(ctx, payloadId, 2*params.BeaconConfig().SlotsPerEpoch) + resp, err := client.GetPayload(ctx, payloadId, 2*params.BeaconConfig().SlotsPerEpoch) require.NoError(t, err) - require.Equal(t, true, override) - g, err := resp.ExcessBlobGas() + require.Equal(t, true, resp.OverrideBuilder) + g, err := resp.ExecutionData.ExcessBlobGas() require.NoError(t, err) require.DeepEqual(t, uint64(3), g) - g, err = resp.BlobGasUsed() + g, err = resp.ExecutionData.BlobGasUsed() require.NoError(t, err) require.DeepEqual(t, uint64(2), g) commitments := [][]byte{bytesutil.PadTo([]byte("commitment1"), fieldparams.BLSPubkeyLength), bytesutil.PadTo([]byte("commitment2"), fieldparams.BLSPubkeyLength)} - require.DeepEqual(t, commitments, blobsBundle.KzgCommitments) + require.DeepEqual(t, commitments, resp.BlobsBundle.KzgCommitments) proofs := [][]byte{bytesutil.PadTo([]byte("proof1"), fieldparams.BLSPubkeyLength), bytesutil.PadTo([]byte("proof2"), fieldparams.BLSPubkeyLength)} - require.DeepEqual(t, proofs, blobsBundle.Proofs) + require.DeepEqual(t, proofs, resp.BlobsBundle.Proofs) blobs := [][]byte{bytesutil.PadTo([]byte("a"), fieldparams.BlobLength), bytesutil.PadTo([]byte("b"), fieldparams.BlobLength)} - require.DeepEqual(t, blobs, blobsBundle.Blobs) + require.DeepEqual(t, blobs, resp.BlobsBundle.Blobs) }) t.Run(GetPayloadMethodV4, func(t *testing.T) { payloadId := [8]byte{1} @@ -359,22 +358,22 @@ func TestClient_HTTP(t *testing.T) { client.rpcClient = rpcClient // We call the RPC method via HTTP and expect a proper result. - resp, blobsBundle, override, err := client.GetPayload(ctx, payloadId, 2*params.BeaconConfig().SlotsPerEpoch) + resp, err := client.GetPayload(ctx, payloadId, 2*params.BeaconConfig().SlotsPerEpoch) require.NoError(t, err) - require.Equal(t, true, override) - g, err := resp.ExcessBlobGas() + require.Equal(t, true, resp.OverrideBuilder) + g, err := resp.ExecutionData.ExcessBlobGas() require.NoError(t, err) require.DeepEqual(t, uint64(3), g) - g, err = resp.BlobGasUsed() + g, err = resp.ExecutionData.BlobGasUsed() require.NoError(t, err) require.DeepEqual(t, uint64(2), g) commitments := [][]byte{bytesutil.PadTo([]byte("commitment1"), fieldparams.BLSPubkeyLength), bytesutil.PadTo([]byte("commitment2"), fieldparams.BLSPubkeyLength)} - require.DeepEqual(t, commitments, blobsBundle.KzgCommitments) + require.DeepEqual(t, commitments, resp.BlobsBundle.KzgCommitments) proofs := [][]byte{bytesutil.PadTo([]byte("proof1"), fieldparams.BLSPubkeyLength), bytesutil.PadTo([]byte("proof2"), fieldparams.BLSPubkeyLength)} - require.DeepEqual(t, proofs, blobsBundle.Proofs) + require.DeepEqual(t, proofs, resp.BlobsBundle.Proofs) blobs := [][]byte{bytesutil.PadTo([]byte("a"), fieldparams.BlobLength), bytesutil.PadTo([]byte("b"), fieldparams.BlobLength)} - require.DeepEqual(t, blobs, blobsBundle.Blobs) + require.DeepEqual(t, blobs, resp.BlobsBundle.Blobs) }) t.Run(ForkchoiceUpdatedMethod+" VALID status", func(t *testing.T) { forkChoiceState := &pb.ForkchoiceState{ diff --git a/beacon-chain/execution/testing/BUILD.bazel b/beacon-chain/execution/testing/BUILD.bazel index 5f16365d157d..ba5b27f98f6a 100644 --- a/beacon-chain/execution/testing/BUILD.bazel +++ b/beacon-chain/execution/testing/BUILD.bazel @@ -25,7 +25,6 @@ go_library( "//encoding/bytesutil:go_default_library", "//proto/engine/v1:go_default_library", "//proto/prysm/v1alpha1:go_default_library", - "//time/slots:go_default_library", "@com_github_ethereum_go_ethereum//accounts/abi/bind/backends:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library", diff --git a/beacon-chain/execution/testing/mock_engine_client.go b/beacon-chain/execution/testing/mock_engine_client.go index d6ca8b23f74d..5fd5c41bd25c 100644 --- a/beacon-chain/execution/testing/mock_engine_client.go +++ b/beacon-chain/execution/testing/mock_engine_client.go @@ -15,7 +15,6 @@ import ( "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" pb "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" - "github.com/prysmaticlabs/prysm/v5/time/slots" ) // EngineClient -- @@ -23,26 +22,20 @@ type EngineClient struct { NewPayloadResp []byte PayloadIDBytes *pb.PayloadIDBytes ForkChoiceUpdatedResp []byte - ExecutionPayload *pb.ExecutionPayload - ExecutionPayloadCapella *pb.ExecutionPayloadCapella - ExecutionPayloadDeneb *pb.ExecutionPayloadDeneb - ExecutionPayloadElectra *pb.ExecutionPayloadElectra ExecutionBlock *pb.ExecutionBlock Err error ErrLatestExecBlock error ErrExecBlockByHash error ErrForkchoiceUpdated error ErrNewPayload error - ErrGetPayload error ExecutionPayloadByBlockHash map[[32]byte]*pb.ExecutionPayload BlockByHashMap map[[32]byte]*pb.ExecutionBlock NumReconstructedPayloads uint64 TerminalBlockHash []byte TerminalBlockHashExists bool - BuilderOverride bool OverrideValidHash [32]byte - BlockValue uint64 - BlobsBundle *pb.BlobsBundle + GetPayloadResponse *blocks.GetPayloadResponse + ErrGetPayload error } // NewPayload -- @@ -61,33 +54,8 @@ func (e *EngineClient) ForkchoiceUpdated( } // GetPayload -- -func (e *EngineClient) GetPayload(_ context.Context, _ [8]byte, s primitives.Slot) (interfaces.ExecutionData, *pb.BlobsBundle, bool, error) { - if slots.ToEpoch(s) >= params.BeaconConfig().ElectraForkEpoch { - ed, err := blocks.WrappedExecutionPayloadElectra(e.ExecutionPayloadElectra, big.NewInt(int64(e.BlockValue))) - if err != nil { - return nil, nil, false, err - } - return ed, e.BlobsBundle, e.BuilderOverride, nil - } - if slots.ToEpoch(s) >= params.BeaconConfig().DenebForkEpoch { - ed, err := blocks.WrappedExecutionPayloadDeneb(e.ExecutionPayloadDeneb, big.NewInt(int64(e.BlockValue))) - if err != nil { - return nil, nil, false, err - } - return ed, e.BlobsBundle, e.BuilderOverride, nil - } - if slots.ToEpoch(s) >= params.BeaconConfig().CapellaForkEpoch { - ed, err := blocks.WrappedExecutionPayloadCapella(e.ExecutionPayloadCapella, big.NewInt(int64(e.BlockValue))) - if err != nil { - return nil, nil, false, err - } - return ed, nil, e.BuilderOverride, nil - } - p, err := blocks.WrappedExecutionPayload(e.ExecutionPayload) - if err != nil { - return nil, nil, false, err - } - return p, nil, e.BuilderOverride, e.ErrGetPayload +func (e *EngineClient) GetPayload(_ context.Context, _ [8]byte, s primitives.Slot) (*blocks.GetPayloadResponse, error) { + return e.GetPayloadResponse, e.ErrGetPayload } // LatestExecutionBlock -- diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go index 7dae54bfaa89..f441c9e603dd 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -77,8 +77,14 @@ func TestServer_setExecutionData(t *testing.T) { Amount: 3, }} id := &v1.PayloadIDBytes{0x1} + + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: withdrawals}, primitives.ZeroWei) + require.NoError(t, err) vs := &Server{ - ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: withdrawals}, BlockValue: 0}, + ExecutionEngineCaller: &powtesting.EngineClient{ + GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}, + PayloadIDBytes: id, + }, HeadFetcher: &blockchainTest.ChainService{State: capellaTransitionState}, FinalizationFetcher: &blockchainTest.ChainService{}, BeaconDB: beaconDB, @@ -356,7 +362,10 @@ func TestServer_setExecutionData(t *testing.T) { t.Run("Builder configured. Local block has higher value", func(t *testing.T) { blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) - vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 3}, BlockValue: 2 * 1e9} + elBid := primitives.Uint64ToWei(2 * 1e9) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 3}, elBid) + require.NoError(t, err) + vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, Bid: elBid}} b := blk.Block() localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) @@ -377,7 +386,11 @@ func TestServer_setExecutionData(t *testing.T) { blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) - vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 3}, BlockValue: 1 * 1e9} + elBid := primitives.Uint64ToWei(1 * 1e9) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 3}, elBid) + require.NoError(t, err) + + vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, Bid: elBid}} b := blk.Block() localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) @@ -399,7 +412,9 @@ func TestServer_setExecutionData(t *testing.T) { HasConfigured: true, Cfg: &builderTest.Config{BeaconDB: beaconDB}, } - vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 4}, BlockValue: 0} + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 4}, primitives.ZeroWei) + require.NoError(t, err) + vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}} b := blk.Block() localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) @@ -428,11 +443,16 @@ func TestServer_setExecutionData(t *testing.T) { Proofs: [][]byte{{4, 5, 6}}, Blobs: [][]byte{{7, 8, 9}}, } + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadDeneb{BlockNumber: 4}, primitives.ZeroWei) + require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{ - PayloadIDBytes: id, - BlobsBundle: blobsBundle, - ExecutionPayloadDeneb: &v1.ExecutionPayloadDeneb{BlockNumber: 4}, - BlockValue: 0} + PayloadIDBytes: id, + GetPayloadResponse: &blocks.GetPayloadResponse{ + ExecutionData: ed, + BlobsBundle: blobsBundle, + Bid: primitives.ZeroWei, + }, + } blk.SetSlot(primitives.Slot(params.BeaconConfig().DenebForkEpoch) * params.BeaconConfig().SlotsPerEpoch) localPayload, _, err := vs.getLocalPayload(ctx, blk.Block(), capellaTransitionState) require.NoError(t, err) @@ -502,7 +522,13 @@ func TestServer_setExecutionData(t *testing.T) { vs.ForkchoiceFetcher.SetForkChoiceGenesisTime(uint64(time.Now().Unix())) vs.TimeFetcher = chain vs.HeadFetcher = chain - vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadDeneb: &v1.ExecutionPayloadDeneb{BlockNumber: 4, Withdrawals: withdrawals}, BlockValue: 0} + + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadDeneb{BlockNumber: 4, Withdrawals: withdrawals}, primitives.ZeroWei) + require.NoError(t, err) + vs.ExecutionEngineCaller = &powtesting.EngineClient{ + PayloadIDBytes: id, + GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}, + } require.NoError(t, err) blk.SetSlot(primitives.Slot(params.BeaconConfig().DenebForkEpoch) * params.BeaconConfig().SlotsPerEpoch) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go index 8f75edef34b3..efb2b880aa25 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go @@ -77,14 +77,17 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe var pid primitives.PayloadID copy(pid[:], payloadId[:]) payloadIDCacheHit.Inc() - payload, bundle, overrideBuilder, err := vs.ExecutionEngineCaller.GetPayload(ctx, pid, slot) - switch { - case err == nil: + res, err := vs.ExecutionEngineCaller.GetPayload(ctx, pid, slot) + if err == nil { + payload, bundle, overrideBuilder := res.ExecutionData, res.BlobsBundle, res.OverrideBuilder bundleCache.add(slot, bundle) - warnIfFeeRecipientDiffers(payload, val.FeeRecipient) + warnIfFeeRecipientDiffers(payload.FeeRecipient(), val.FeeRecipient[:]) return payload, overrideBuilder, nil - case errors.Is(err, context.DeadlineExceeded): - default: + } + // TODO: TestServer_getExecutionPayloadContextTimeout expects this behavior. + // We need to figure out if it is actually important to "retry" by falling through to the code below when + // we get a timeout when trying to retrieve the cached payload id. + if !errors.Is(err, context.DeadlineExceeded) { return nil, false, errors.Wrap(err, "could not get cached payload from execution client") } } @@ -175,27 +178,26 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe if payloadID == nil { return nil, false, fmt.Errorf("nil payload with block hash: %#x", parentHash) } - payload, bundle, overrideBuilder, err := vs.ExecutionEngineCaller.GetPayload(ctx, *payloadID, slot) + res, err := vs.ExecutionEngineCaller.GetPayload(ctx, *payloadID, slot) if err != nil { return nil, false, err } - bundleCache.add(slot, bundle) - warnIfFeeRecipientDiffers(payload, val.FeeRecipient) - localValueGwei, err := payload.ValueInGwei() - if err == nil { - log.WithField("value", localValueGwei).Debug("received execution payload from local engine") - } - return payload, overrideBuilder, nil + + // TODO: remove bundleCache and use the blobs directly in the payload. + bundleCache.add(slot, res.BlobsBundle) + warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) + log.WithField("value", res.Bid).Debug("received execution payload from local engine") + return res.ExecutionData, res.OverrideBuilder, nil } // warnIfFeeRecipientDiffers logs a warning if the fee recipient in the included payload does not // match the requested one. -func warnIfFeeRecipientDiffers(payload interfaces.ExecutionData, feeRecipient primitives.ExecutionAddress) { +func warnIfFeeRecipientDiffers(payload, val []byte) { // Warn if the fee recipient is not the value we expect. - if payload != nil && !bytes.Equal(payload.FeeRecipient(), feeRecipient[:]) { + if !bytes.Equal(payload, val) { logrus.WithFields(logrus.Fields{ - "wantedFeeRecipient": fmt.Sprintf("%#x", feeRecipient), - "received": fmt.Sprintf("%#x", payload.FeeRecipient()), + "wantedFeeRecipient": fmt.Sprintf("%#x", val), + "received": fmt.Sprintf("%#x", payload), }).Warn("Fee recipient address from execution client is not what was expected. " + "It is possible someone has compromised your client to try and take your transaction fees") } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go index 33369d681fe4..cd18cdddd3db 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go @@ -146,8 +146,10 @@ func TestServer_getExecutionPayload(t *testing.T) { cfg.TerminalBlockHashActivationEpoch = tt.activationEpoch params.OverrideBeaconConfig(cfg) + ed, err := blocks.NewWrappedExecutionData(&pb.ExecutionPayload{}, primitives.ZeroWei) + require.NoError(t, err) vs := &Server{ - ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: tt.payloadID, ErrForkchoiceUpdated: tt.forkchoiceErr, ExecutionPayload: &pb.ExecutionPayload{}, BuilderOverride: tt.override}, + ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: tt.payloadID, ErrForkchoiceUpdated: tt.forkchoiceErr, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, OverrideBuilder: tt.override}}, HeadFetcher: &chainMock.ChainService{State: tt.st}, FinalizationFetcher: &chainMock.ChainService{}, BeaconDB: beaconDB, @@ -194,8 +196,10 @@ func TestServer_getExecutionPayloadContextTimeout(t *testing.T) { cfg.TerminalBlockHashActivationEpoch = 1 params.OverrideBeaconConfig(cfg) + ed, err := blocks.NewWrappedExecutionData(&pb.ExecutionPayload{}, primitives.ZeroWei) + require.NoError(t, err) vs := &Server{ - ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: &pb.PayloadIDBytes{}, ErrGetPayload: context.DeadlineExceeded, ExecutionPayload: &pb.ExecutionPayload{}}, + ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: &pb.PayloadIDBytes{}, ErrGetPayload: context.DeadlineExceeded, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}}, HeadFetcher: &chainMock.ChainService{State: nonTransitionSt}, BeaconDB: beaconDB, PayloadIDCache: cache.NewPayloadIDCache(), @@ -241,10 +245,12 @@ func TestServer_getExecutionPayload_UnexpectedFeeRecipient(t *testing.T) { payloadID := &pb.PayloadIDBytes{0x1} payload := emptyPayload() payload.FeeRecipient = feeRecipient[:] + ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + require.NoError(t, err) vs := &Server{ ExecutionEngineCaller: &powtesting.EngineClient{ - PayloadIDBytes: payloadID, - ExecutionPayload: payload, + PayloadIDBytes: payloadID, + GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}, }, HeadFetcher: &chainMock.ChainService{State: transitionSt}, FinalizationFetcher: &chainMock.ChainService{}, diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go index 6ed6bd95ccf6..6e17499fe27a 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go @@ -277,9 +277,11 @@ func TestServer_GetBeaconBlock_Bellatrix(t *testing.T) { proposerServer := getProposerServer(db, beaconState, parentRoot[:]) proposerServer.Eth1BlockFetcher = c + ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + require.NoError(t, err) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ - PayloadIDBytes: &enginev1.PayloadIDBytes{1}, - ExecutionPayload: payload, + PayloadIDBytes: &enginev1.PayloadIDBytes{1}, + GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}, } randaoReveal, err := util.RandaoReveal(beaconState, 0, privKeys) @@ -400,9 +402,11 @@ func TestServer_GetBeaconBlock_Capella(t *testing.T) { } proposerServer := getProposerServer(db, beaconState, parentRoot[:]) + ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + require.NoError(t, err) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ - PayloadIDBytes: &enginev1.PayloadIDBytes{1}, - ExecutionPayloadCapella: payload, + PayloadIDBytes: &enginev1.PayloadIDBytes{1}, + GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}, } randaoReveal, err := util.RandaoReveal(beaconState, 0, privKeys) @@ -510,6 +514,8 @@ func TestServer_GetBeaconBlock_Deneb(t *testing.T) { BlobGasUsed: 4, ExcessBlobGas: 5, } + ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + require.NoError(t, err) kc := make([][]byte, 0) kc = append(kc, bytesutil.PadTo([]byte("kc"), 48)) @@ -520,9 +526,11 @@ func TestServer_GetBeaconBlock_Deneb(t *testing.T) { bundle := &enginev1.BlobsBundle{KzgCommitments: kc, Proofs: proofs, Blobs: blobs} proposerServer := getProposerServer(db, beaconState, parentRoot[:]) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ - PayloadIDBytes: &enginev1.PayloadIDBytes{1}, - ExecutionPayloadDeneb: payload, - BlobsBundle: bundle, + PayloadIDBytes: &enginev1.PayloadIDBytes{1}, + GetPayloadResponse: &blocks.GetPayloadResponse{ + ExecutionData: ed, + BlobsBundle: bundle, + }, } randaoReveal, err := util.RandaoReveal(beaconState, 0, privKeys) @@ -651,9 +659,11 @@ func TestServer_GetBeaconBlock_Electra(t *testing.T) { } proposerServer := getProposerServer(db, beaconState, parentRoot[:]) + ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + require.NoError(t, err) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ - PayloadIDBytes: &enginev1.PayloadIDBytes{1}, - ExecutionPayloadElectra: payload, + PayloadIDBytes: &enginev1.PayloadIDBytes{1}, + GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}, } randaoReveal, err := util.RandaoReveal(beaconState, 0, privKeys) diff --git a/consensus-types/blocks/BUILD.bazel b/consensus-types/blocks/BUILD.bazel index d56c464ac30e..1988e1a4e9f3 100644 --- a/consensus-types/blocks/BUILD.bazel +++ b/consensus-types/blocks/BUILD.bazel @@ -5,6 +5,7 @@ go_library( srcs = [ "execution.go", "factory.go", + "get_payload.go", "getters.go", "kzg.go", "proto.go", diff --git a/consensus-types/blocks/execution.go b/consensus-types/blocks/execution.go index 13e3905e698b..9be70c2f9085 100644 --- a/consensus-types/blocks/execution.go +++ b/consensus-types/blocks/execution.go @@ -28,15 +28,24 @@ func NewWrappedExecutionData(v proto.Message, weiValue primitives.Wei) (interfac if weiValue == nil { weiValue = new(big.Int).SetInt64(0) } + if v == nil { + return nil, consensus_types.ErrNilObjectWrapped + } switch pbStruct := v.(type) { case *enginev1.ExecutionPayload: return WrappedExecutionPayload(pbStruct) case *enginev1.ExecutionPayloadCapella: return WrappedExecutionPayloadCapella(pbStruct, weiValue) + case *enginev1.ExecutionPayloadCapellaWithValue: + return WrappedExecutionPayloadCapella(pbStruct.Payload, weiValue) case *enginev1.ExecutionPayloadDeneb: return WrappedExecutionPayloadDeneb(pbStruct, weiValue) + case *enginev1.ExecutionPayloadDenebWithValueAndBlobsBundle: + return WrappedExecutionPayloadDeneb(pbStruct.Payload, weiValue) case *enginev1.ExecutionPayloadElectra: return WrappedExecutionPayloadElectra(pbStruct, weiValue) + case *enginev1.ExecutionPayloadElectraWithValueAndBlobsBundle: + return WrappedExecutionPayloadElectra(pbStruct.Payload, weiValue) default: return nil, ErrUnsupportedVersion } diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go new file mode 100644 index 000000000000..cfa174da5a2b --- /dev/null +++ b/consensus-types/blocks/get_payload.go @@ -0,0 +1,56 @@ +package blocks + +import ( + "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" + "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" + enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" + pb "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" + "google.golang.org/protobuf/proto" +) + +// GetPayloadResponse represents the result of unmarshaling an execution engine +// GetPayloadResponseV(1|2|3|4) value. +type GetPayloadResponse struct { + ExecutionData interfaces.ExecutionData + BlobsBundle *enginev1.BlobsBundle + OverrideBuilder bool + Bid primitives.Wei +} + +// bundleGetter is an interface satisfied by get payload responses that have a blobs bundle. +type bundleGetter interface { + GetBlobsBundle() *pb.BlobsBundle +} + +// bidValueGetter is an interface satisfied by get payload responses that have a bid value. +type bidValueGetter interface { + GetValue() []byte +} + +type shouldOverrideBuilderGetter interface { + GetShouldOverrideBuilder() bool +} + +func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { + r := &GetPayloadResponse{} + bundleGetter, hasBundle := msg.(bundleGetter) + if hasBundle { + r.BlobsBundle = bundleGetter.GetBlobsBundle() + } + bidValueGetter, hasBid := msg.(bidValueGetter) + wei := primitives.ZeroWei + if hasBid { + wei = primitives.BigEndianBytesToWei(bidValueGetter.GetValue()) + r.Bid = wei + } + shouldOverride, hasShouldOverride := msg.(shouldOverrideBuilderGetter) + if hasShouldOverride { + r.OverrideBuilder = shouldOverride.GetShouldOverrideBuilder() + } + ed, err := NewWrappedExecutionData(msg, wei) + if err != nil { + return nil, err + } + r.ExecutionData = ed + return r, nil +} diff --git a/consensus-types/blocks/get_payload_test.go b/consensus-types/blocks/get_payload_test.go new file mode 100644 index 000000000000..b40b8f17cd87 --- /dev/null +++ b/consensus-types/blocks/get_payload_test.go @@ -0,0 +1,22 @@ +package blocks + +import ( + "testing" + + pb "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" + "github.com/prysmaticlabs/prysm/v5/testing/require" + "google.golang.org/protobuf/proto" +) + +func TestPayloadGetter(t *testing.T) { + t.Run("pre-capella does not implement payload getter", func(t *testing.T) { + var bellatrix proto.Message = &pb.ExecutionPayload{} + _, ok := bellatrix.(payloadGetter) + require.Equal(t, false, ok) + }) + t.Run("capella implements payload getter", func(t *testing.T) { + var capella interface{} = &pb.ExecutionPayloadCapellaWithValue{} + _, ok := capella.(payloadGetter) + require.Equal(t, true, ok) + }) +} diff --git a/consensus-types/primitives/wei.go b/consensus-types/primitives/wei.go index 8bcee0130899..b74a95e806f2 100644 --- a/consensus-types/primitives/wei.go +++ b/consensus-types/primitives/wei.go @@ -75,8 +75,8 @@ func Uint64ToWei(v uint64) Wei { func BigEndianBytesToWei(value []byte) Wei { v := make([]byte, len(value)) copy(v, value) - slices.Reverse(v) // We have to convert big endian to little endian because the value is coming from the execution layer. + slices.Reverse(v) return big.NewInt(0).SetBytes(v) } diff --git a/math/BUILD.bazel b/math/BUILD.bazel index fac6ee3ab7a0..ffd1f0ba7693 100644 --- a/math/BUILD.bazel +++ b/math/BUILD.bazel @@ -14,7 +14,6 @@ go_test( srcs = ["math_helper_test.go"], deps = [ ":go_default_library", - "//consensus-types/primitives:go_default_library", "//testing/require:go_default_library", ], ) diff --git a/math/math_helper_test.go b/math/math_helper_test.go index 64243a694724..8ee06bbd2ed2 100644 --- a/math/math_helper_test.go +++ b/math/math_helper_test.go @@ -3,10 +3,8 @@ package math_test import ( "fmt" stdmath "math" - "math/big" "testing" - "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/math" "github.com/prysmaticlabs/prysm/v5/testing/require" ) @@ -551,28 +549,3 @@ func TestAddInt(t *testing.T) { }) } } - -func TestWeiToGwei(t *testing.T) { - tests := []struct { - v *big.Int - want primitives.Gwei - }{ - {big.NewInt(1e9 - 1), 0}, - {big.NewInt(1e9), 1}, - {big.NewInt(1e10), 10}, - {big.NewInt(239489233849348394), 239489233}, - } - for _, tt := range tests { - if got := primitives.WeiToGwei(tt.v); got != tt.want { - t.Errorf("WeiToGwei() = %v, want %v", got, tt.want) - } - } -} - -func TestWeiToGwei_CopyOk(t *testing.T) { - v := big.NewInt(1e9) - got := primitives.WeiToGwei(v) - - require.Equal(t, primitives.Gwei(1), got) - require.Equal(t, big.NewInt(1e9).Uint64(), v.Uint64()) -} From f9c7ae9a5ff2572e60e3f3fa550c3edc77b5091f Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Mon, 20 May 2024 16:59:17 -0500 Subject: [PATCH 02/16] deprecate PayloadValueTo(Gwei|Wei) --- api/client/builder/bid.go | 2 +- consensus-types/blocks/execution.go | 13 ------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/api/client/builder/bid.go b/api/client/builder/bid.go index 50e3e21952f2..a62ece939994 100644 --- a/api/client/builder/bid.go +++ b/api/client/builder/bid.go @@ -250,7 +250,7 @@ func (b builderBidDeneb) HashTreeRootWith(hh *ssz.Hasher) error { // Header -- func (b builderBidDeneb) Header() (interfaces.ExecutionData, error) { // We have to convert big endian to little endian because the value is coming from the execution layer. - return blocks.WrappedExecutionPayloadHeaderDeneb(b.p.Header, blocks.PayloadValueToWei(b.p.Value)) + return blocks.WrappedExecutionPayloadHeaderDeneb(b.p.Header, primitives.BigEndianBytesToWei(b.p.Value)) } // BlobKzgCommitments -- diff --git a/consensus-types/blocks/execution.go b/consensus-types/blocks/execution.go index 9be70c2f9085..5e4a36e82ee6 100644 --- a/consensus-types/blocks/execution.go +++ b/consensus-types/blocks/execution.go @@ -1667,16 +1667,3 @@ func (e executionPayloadElectra) WithdrawalRequests() []*enginev1.ExecutionLayer func (e executionPayloadElectra) IsBlinded() bool { return false } - -// PayloadValueToWei returns a Wei value given the payload's value -func PayloadValueToWei(value []byte) primitives.Wei { - // We have to convert big endian to little endian because the value is coming from the execution layer. - return big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(value)) -} - -// PayloadValueToGwei returns a Gwei value given the payload's value -func PayloadValueToGwei(value []byte) primitives.Gwei { - // We have to convert big endian to little endian because the value is coming from the execution layer. - v := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(value)) - return primitives.WeiToGwei(v) -} From 3d5b1a0c43c196d1acbedc7cfd6be87af9407c00 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Tue, 21 May 2024 09:53:36 -0500 Subject: [PATCH 03/16] return entire bid from builder getter --- .../rpc/prysm/v1alpha1/validator/proposer.go | 55 +++++--- .../v1alpha1/validator/proposer_bellatrix.go | 88 ++++++------ .../validator/proposer_bellatrix_test.go | 125 ++++++++++++------ .../validator/proposer_execution_payload.go | 60 ++++----- .../proposer_execution_payload_test.go | 17 ++- consensus-types/blocks/get_payload.go | 3 +- 6 files changed, 205 insertions(+), 143 deletions(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index 1a523edb9ade..c1eebfcd0133 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -40,7 +40,7 @@ var eth1DataNotification bool const ( eth1dataTimeout = 2 * time.Second - defaultBuilderBoostFactor = uint64(100) + defaultBuilderBoostFactor = primitives.Gwei(100) ) // GetBeaconBlock is called by a proposer during its assigned slot to request a block to sign @@ -93,7 +93,7 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) ( builderBoostFactor := defaultBuilderBoostFactor if req.BuilderBoostFactor != nil { - builderBoostFactor = req.BuilderBoostFactor.Value + builderBoostFactor = primitives.Gwei(req.BuilderBoostFactor.Value) } if err = vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost, builderBoostFactor); err != nil { @@ -184,7 +184,7 @@ func (vs *Server) getParentState(ctx context.Context, slot primitives.Slot) (sta return head, parentRoot, err } -func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor uint64) error { +func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor primitives.Gwei) error { // Build consensus fields in background var wg sync.WaitGroup wg.Add(1) @@ -231,25 +231,42 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed vs.setBlsToExecData(sBlk, head) }() - localPayload, overrideBuilder, err := vs.getLocalPayload(ctx, sBlk.Block(), head) - if err != nil { - return status.Errorf(codes.Internal, "Could not get local payload: %v", err) - } - - // There's no reason to try to get a builder bid if local override is true. - var builderPayload interfaces.ExecutionData - var builderKzgCommitments [][]byte - overrideBuilder = overrideBuilder || skipMevBoost // Skip using mev-boost if requested by the caller. - if !overrideBuilder { - builderPayload, builderKzgCommitments, err = vs.getBuilderPayloadAndBlobs(ctx, sBlk.Block().Slot(), sBlk.Block().ProposerIndex()) + blockEpoch := slots.ToEpoch(sBlk.Block().Slot()) + if blockEpoch >= params.BeaconConfig().BellatrixForkEpoch { + local, err := vs.getLocalPayload(ctx, sBlk.Block(), head) if err != nil { - builderGetPayloadMissCount.Inc() - log.WithError(err).Error("Could not get builder payload") + return status.Errorf(codes.Internal, "Could not get local payload: %v", err) + } + + // There's no reason to try to get a builder bid if local override is true. + var builderPayload interfaces.ExecutionData + var builderKzgCommitments [][]byte + if local.OverrideBuilder || skipMevBoost { + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, sBlk.Block().Slot(), sBlk.Block().ProposerIndex()) + if err != nil { + builderGetPayloadMissCount.Inc() + log.WithError(err).Error("Could not get builder payload") + } + // getBuidlerPayloadAndBlobs can return `nil, nil` for the condition where no builder is configured... + if builderBid != nil { + builderPayload, err = builderBid.Header() + if err != nil { + builderGetPayloadMissCount.Inc() + log.WithError(err).Error("Could not get builder payload") + } + if builderBid.Version() > version.Deneb { + builderKzgCommitments, err = builderBid.BlobKzgCommitments() + if err != nil { + builderGetPayloadMissCount.Inc() + log.WithError(err).Error("deneb payload does not have commitments") + } + } + } } - } - if err := setExecutionData(ctx, sBlk, localPayload, builderPayload, builderKzgCommitments, builderBoostFactor); err != nil { - return status.Errorf(codes.Internal, "Could not set execution data: %v", err) + if err := setExecutionData(ctx, sBlk, local, builderPayload, builderKzgCommitments, builderBoostFactor); err != nil { + return status.Errorf(codes.Internal, "Could not set execution data: %v", err) + } } wg.Wait() // Wait until block is built via consensus and execution fields. diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index 907dbadf9132..15915aa071b7 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go @@ -14,6 +14,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/signing" fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" "github.com/prysmaticlabs/prysm/v5/config/params" + "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" @@ -50,7 +51,7 @@ var emptyTransactionsRoot = [32]byte{127, 254, 36, 30, 166, 1, 135, 253, 176, 24 const blockBuilderTimeout = 1 * time.Second // Sets the execution data for the block. Execution data can come from local EL client or remote builder depends on validator registration and circuit breaker conditions. -func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, localPayload, builderPayload interfaces.ExecutionData, builderKzgCommitments [][]byte, builderBoostFactor uint64) error { +func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, local *blocks.GetPayloadResponse, builderPayload interfaces.ExecutionData, builderKzgCommitments [][]byte, builderBoostFactor primitives.Gwei) error { _, span := trace.StartSpan(ctx, "ProposerServer.setExecutionData") defer span.End() @@ -59,38 +60,37 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc return nil } - if localPayload == nil { + if local == nil { return errors.New("local payload is nil") } // Use local payload if builder payload is nil. if builderPayload == nil { - return setLocalExecution(blk, localPayload) + return setLocalExecution(blk, local) } switch { case blk.Version() >= version.Capella: // Compare payload values between local and builder. Default to the local value if it is higher. - localValueGwei, err := localPayload.ValueInGwei() - if err != nil { - return errors.Wrap(err, "failed to get local payload value") - } - builderValueGwei, err := builderPayload.ValueInGwei() + localValueGwei := primitives.WeiToGwei(local.Bid) + // TODO: plumb builder gwei value through the bid instead of getting from ExecutionData. + buildValueUint, err := builderPayload.ValueInGwei() if err != nil { log.WithError(err).Warn("Proposer: failed to get builder payload value") // Default to local if can't get builder value. - return setLocalExecution(blk, localPayload) + return setLocalExecution(blk, local) } + builderValueGwei := primitives.Gwei(buildValueUint) - withdrawalsMatched, err := matchingWithdrawalsRoot(localPayload, builderPayload) + withdrawalsMatched, err := matchingWithdrawalsRoot(local.ExecutionData, builderPayload) if err != nil { tracing.AnnotateError(span, err) log.WithError(err).Warn("Proposer: failed to match withdrawals root") - return setLocalExecution(blk, localPayload) + return setLocalExecution(blk, local) } // Use builder payload if the following in true: // builder_bid_value * builderBoostFactor(default 100) > local_block_value * (local-block-value-boost + 100) - boost := params.BeaconConfig().LocalBlockValueBoost + boost := primitives.Gwei(params.BeaconConfig().LocalBlockValueBoost) higherValueBuilder := builderValueGwei*builderBoostFactor > localValueGwei*(100+boost) if boost > 0 && builderBoostFactor != defaultBuilderBoostFactor { log.WithFields(logrus.Fields{ @@ -107,7 +107,7 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc if higherValueBuilder && withdrawalsMatched { // Builder value is higher and withdrawals match. if err := setBuilderExecution(blk, builderPayload, builderKzgCommitments); err != nil { log.WithError(err).Warn("Proposer: failed to set builder payload") - return setLocalExecution(blk, localPayload) + return setLocalExecution(blk, local) } else { return nil } @@ -127,11 +127,11 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc trace.Int64Attribute("builderGweiValue", int64(builderValueGwei)), // lint:ignore uintcast -- This is OK for tracing. trace.Int64Attribute("builderBoostFactor", int64(builderBoostFactor)), // lint:ignore uintcast -- This is OK for tracing. ) - return setLocalExecution(blk, localPayload) + return setLocalExecution(blk, local) default: // Bellatrix case. if err := setBuilderExecution(blk, builderPayload, builderKzgCommitments); err != nil { log.WithError(err).Warn("Proposer: failed to set builder payload") - return setLocalExecution(blk, localPayload) + return setLocalExecution(blk, local) } else { return nil } @@ -140,26 +140,26 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc // This function retrieves the payload header and kzg commitments given the slot number and the validator index. // It's a no-op if the latest head block is not versioned bellatrix. -func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitives.Slot, idx primitives.ValidatorIndex) (interfaces.ExecutionData, [][]byte, error) { +func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitives.Slot, idx primitives.ValidatorIndex) (builder.Bid, error) { ctx, span := trace.StartSpan(ctx, "ProposerServer.getPayloadHeaderFromBuilder") defer span.End() if slots.ToEpoch(slot) < params.BeaconConfig().BellatrixForkEpoch { - return nil, nil, errors.New("can't get payload header from builder before bellatrix epoch") + return nil, errors.New("can't get payload header from builder before bellatrix epoch") } b, err := vs.HeadFetcher.HeadBlock(ctx) if err != nil { - return nil, nil, err + return nil, err } h, err := b.Block().Body().Execution() if err != nil { - return nil, nil, errors.Wrap(err, "failed to get execution header") + return nil, errors.Wrap(err, "failed to get execution header") } pk, err := vs.HeadFetcher.HeadValidatorIndexToPublicKey(ctx, idx) if err != nil { - return nil, nil, err + return nil, err } ctx, cancel := context.WithTimeout(ctx, blockBuilderTimeout) @@ -167,76 +167,76 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv signedBid, err := vs.BlockBuilder.GetHeader(ctx, slot, bytesutil.ToBytes32(h.BlockHash()), pk) if err != nil { - return nil, nil, err + return nil, err } if signedBid.IsNil() { - return nil, nil, errors.New("builder returned nil bid") + return nil, errors.New("builder returned nil bid") } fork, err := forks.Fork(slots.ToEpoch(slot)) if err != nil { - return nil, nil, errors.Wrap(err, "unable to get fork information") + return nil, errors.Wrap(err, "unable to get fork information") } forkName, ok := params.BeaconConfig().ForkVersionNames[bytesutil.ToBytes4(fork.CurrentVersion)] if !ok { - return nil, nil, errors.New("unable to find current fork in schedule") + return nil, errors.New("unable to find current fork in schedule") } if !strings.EqualFold(version.String(signedBid.Version()), forkName) { - return nil, nil, fmt.Errorf("builder bid response version: %d is different from head block version: %d for epoch %d", signedBid.Version(), b.Version(), slots.ToEpoch(slot)) + return nil, fmt.Errorf("builder bid response version: %d is different from head block version: %d for epoch %d", signedBid.Version(), b.Version(), slots.ToEpoch(slot)) } bid, err := signedBid.Message() if err != nil { - return nil, nil, errors.Wrap(err, "could not get bid") + return nil, errors.Wrap(err, "could not get bid") } if bid.IsNil() { - return nil, nil, errors.New("builder returned nil bid") + return nil, errors.New("builder returned nil bid") } v := bytesutil.LittleEndianBytesToBigInt(bid.Value()) if v.String() == "0" { - return nil, nil, errors.New("builder returned header with 0 bid amount") + return nil, errors.New("builder returned header with 0 bid amount") } header, err := bid.Header() if err != nil { - return nil, nil, errors.Wrap(err, "could not get bid header") + return nil, errors.Wrap(err, "could not get bid header") } txRoot, err := header.TransactionsRoot() if err != nil { - return nil, nil, errors.Wrap(err, "could not get transaction root") + return nil, errors.Wrap(err, "could not get transaction root") } if bytesutil.ToBytes32(txRoot) == emptyTransactionsRoot { - return nil, nil, errors.New("builder returned header with an empty tx root") + return nil, errors.New("builder returned header with an empty tx root") } if !bytes.Equal(header.ParentHash(), h.BlockHash()) { - return nil, nil, fmt.Errorf("incorrect parent hash %#x != %#x", header.ParentHash(), h.BlockHash()) + return nil, fmt.Errorf("incorrect parent hash %#x != %#x", header.ParentHash(), h.BlockHash()) } t, err := slots.ToTime(uint64(vs.TimeFetcher.GenesisTime().Unix()), slot) if err != nil { - return nil, nil, err + return nil, err } if header.Timestamp() != uint64(t.Unix()) { - return nil, nil, fmt.Errorf("incorrect timestamp %d != %d", header.Timestamp(), uint64(t.Unix())) + return nil, fmt.Errorf("incorrect timestamp %d != %d", header.Timestamp(), uint64(t.Unix())) } if err := validateBuilderSignature(signedBid); err != nil { - return nil, nil, errors.Wrap(err, "could not validate builder signature") + return nil, errors.Wrap(err, "could not validate builder signature") } var kzgCommitments [][]byte if bid.Version() >= version.Deneb { kzgCommitments, err = bid.BlobKzgCommitments() if err != nil { - return nil, nil, errors.Wrap(err, "could not get blob kzg commitments") + return nil, errors.Wrap(err, "could not get blob kzg commitments") } if len(kzgCommitments) > fieldparams.MaxBlobsPerBlock { - return nil, nil, fmt.Errorf("builder returned too many kzg commitments: %d", len(kzgCommitments)) + return nil, fmt.Errorf("builder returned too many kzg commitments: %d", len(kzgCommitments)) } for _, c := range kzgCommitments { if len(c) != fieldparams.BLSPubkeyLength { - return nil, nil, fmt.Errorf("builder returned invalid kzg commitment length: %d", len(c)) + return nil, fmt.Errorf("builder returned invalid kzg commitment length: %d", len(c)) } } } @@ -260,7 +260,7 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv trace.StringAttribute("blockHash", fmt.Sprintf("%#x", header.BlockHash())), ) - return header, kzgCommitments, nil + return bid, nil } // Validates builder signature and returns an error if the signature is invalid. @@ -310,13 +310,13 @@ func matchingWithdrawalsRoot(local, builder interfaces.ExecutionData) (bool, err // setLocalExecution sets the execution context for a local beacon block. // It delegates to setExecution for the actual work. -func setLocalExecution(blk interfaces.SignedBeaconBlock, execution interfaces.ExecutionData) error { +func setLocalExecution(blk interfaces.SignedBeaconBlock, local *blocks.GetPayloadResponse) error { var kzgCommitments [][]byte - fullBlobsBundle := bundleCache.get(blk.Block().Slot()) - if fullBlobsBundle != nil { - kzgCommitments = fullBlobsBundle.KzgCommitments + if local.BlobsBundle != nil { + kzgCommitments = local.BlobsBundle.KzgCommitments } - return setExecution(blk, execution, false, kzgCommitments) + // TODO: plumb blobs bundle through here. + return setExecution(blk, local.ExecutionData, false, kzgCommitments) } // setBuilderExecution sets the execution context for a builder's beacon block. diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go index f441c9e603dd..7d654f7a64ad 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -28,6 +28,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/encoding/ssz" v1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" + "github.com/prysmaticlabs/prysm/v5/runtime/version" "github.com/prysmaticlabs/prysm/v5/testing/require" "github.com/prysmaticlabs/prysm/v5/testing/util" "github.com/prysmaticlabs/prysm/v5/time/slots" @@ -98,12 +99,15 @@ func TestServer_setExecutionData(t *testing.T) { blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) + var builderKzgCommitments [][]byte + var builderPayload interfaces.ExecutionData + require.IsNil(t, builderBid) require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block @@ -159,12 +163,18 @@ func TestServer_setExecutionData(t *testing.T) { vs.HeadFetcher = chain b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + builderPayload, err := builderBid.Header() + require.NoError(t, err) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } + require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block because incorrect withdrawals @@ -223,12 +233,18 @@ func TestServer_setExecutionData(t *testing.T) { vs.HeadFetcher = chain b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + builderPayload, err := builderBid.Header() + require.NoError(t, err) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } + require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // Builder block @@ -286,12 +302,18 @@ func TestServer_setExecutionData(t *testing.T) { vs.HeadFetcher = chain b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, math.MaxUint64)) + builderPayload, err := builderBid.Header() + require.NoError(t, err) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } + require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, math.MaxUint64)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // builder block @@ -349,12 +371,18 @@ func TestServer_setExecutionData(t *testing.T) { vs.HeadFetcher = chain b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, 0)) + builderPayload, err := builderBid.Header() + require.NoError(t, err) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } + require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, 0)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // local block @@ -367,12 +395,18 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, Bid: elBid}} b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + builderPayload, err := builderBid.Header() + require.NoError(t, err) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } + require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(3), e.BlockNumber()) // Local block @@ -392,12 +426,18 @@ func TestServer_setExecutionData(t *testing.T) { vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, Bid: elBid}} b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } + builderPayload, err := builderBid.Header() + require.NoError(t, err) + require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(3), e.BlockNumber()) // Local block @@ -416,12 +456,12 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}} b := blk.Block() - localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState) + res, err := vs.getLocalPayload(ctx, b, capellaTransitionState) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.ErrorIs(t, consensus_types.ErrNilObjectWrapped, err) // Builder returns fault. Use local block - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.IsNil(t, builderBid) + require.NoError(t, setExecutionData(context.Background(), blk, res, nil, nil, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(4), e.BlockNumber()) // Local block @@ -454,11 +494,10 @@ func TestServer_setExecutionData(t *testing.T) { }, } blk.SetSlot(primitives.Slot(params.BeaconConfig().DenebForkEpoch) * params.BeaconConfig().SlotsPerEpoch) - localPayload, _, err := vs.getLocalPayload(ctx, blk.Block(), capellaTransitionState) + res, err := vs.getLocalPayload(ctx, blk.Block(), capellaTransitionState) require.NoError(t, err) - require.Equal(t, uint64(4), localPayload.BlockNumber()) - cachedBundle := bundleCache.get(blk.Block().Slot()) - require.DeepEqual(t, cachedBundle, blobsBundle) + require.Equal(t, uint64(4), res.ExecutionData.BlockNumber()) + require.DeepEqual(t, res.BlobsBundle, blobsBundle) }) t.Run("Can get builder payload and blobs in Deneb", func(t *testing.T) { cfg := params.BeaconConfig().Copy() @@ -533,14 +572,20 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) blk.SetSlot(primitives.Slot(params.BeaconConfig().DenebForkEpoch) * params.BeaconConfig().SlotsPerEpoch) require.NoError(t, err) - builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, blk.Block().Slot(), blk.Block().ProposerIndex()) + builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, blk.Block().Slot(), blk.Block().ProposerIndex()) + require.NoError(t, err) + builderPayload, err := builderBid.Header() require.NoError(t, err) + builderKzgCommitments, err := builderBid.BlobKzgCommitments() + if builderBid.Version() >= version.Deneb { + require.NoError(t, err) + } require.DeepEqual(t, bid.BlobKzgCommitments, builderKzgCommitments) require.Equal(t, bid.Header.BlockNumber, builderPayload.BlockNumber()) // header should be the same from block - localPayload, _, err := vs.getLocalPayload(ctx, blk.Block(), denebTransitionState) + res, err := vs.getLocalPayload(ctx, blk.Block(), denebTransitionState) require.NoError(t, err) - require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) got, err := blk.Block().Body().BlobKzgCommitments() require.NoError(t, err) @@ -763,10 +808,12 @@ func TestServer_getPayloadHeader(t *testing.T) { }} hb, err := vs.HeadFetcher.HeadBlock(context.Background()) require.NoError(t, err) - h, _, err := vs.getPayloadHeaderFromBuilder(context.Background(), hb.Block().Slot(), 0) + bid, err := vs.getPayloadHeaderFromBuilder(context.Background(), hb.Block().Slot(), 0) if tc.err != "" { require.ErrorContains(t, tc.err, err) } else { + require.NoError(t, err) + h, err := bid.Header() require.NoError(t, err) if tc.returnedHeader != nil { want, err := blocks.WrappedExecutionPayloadHeader(tc.returnedHeader) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go index efb2b880aa25..559b31ec422c 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prysmaticlabs/prysm/v5/api/client/builder" "github.com/prysmaticlabs/prysm/v5/beacon-chain/cache" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers" @@ -47,12 +48,12 @@ func setFeeRecipientIfBurnAddress(val *cache.TrackedValidator) { } // This returns the local execution payload of a given slot. The function has full awareness of pre and post merge. -func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBeaconBlock, st state.BeaconState) (interfaces.ExecutionData, bool, error) { +func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBeaconBlock, st state.BeaconState) (*consensusblocks.GetPayloadResponse, error) { ctx, span := trace.StartSpan(ctx, "ProposerServer.getLocalPayload") defer span.End() if blk.Version() < version.Bellatrix { - return nil, false, nil + return nil, nil } slot := blk.Slot() @@ -79,35 +80,32 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe payloadIDCacheHit.Inc() res, err := vs.ExecutionEngineCaller.GetPayload(ctx, pid, slot) if err == nil { - payload, bundle, overrideBuilder := res.ExecutionData, res.BlobsBundle, res.OverrideBuilder - bundleCache.add(slot, bundle) - warnIfFeeRecipientDiffers(payload.FeeRecipient(), val.FeeRecipient[:]) - return payload, overrideBuilder, nil + //payload, bundle, overrideBuilder := res.ExecutionData, res.BlobsBundle, res.OverrideBuilder + //bundleCache.add(slot, bundle) + warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) + //return payload, overrideBuilder, nil + return res, nil } // TODO: TestServer_getExecutionPayloadContextTimeout expects this behavior. // We need to figure out if it is actually important to "retry" by falling through to the code below when // we get a timeout when trying to retrieve the cached payload id. if !errors.Is(err, context.DeadlineExceeded) { - return nil, false, errors.Wrap(err, "could not get cached payload from execution client") + return nil, errors.Wrap(err, "could not get cached payload from execution client") } } log.WithFields(logFields).Debug("payload ID cache miss") parentHash, err := vs.getParentBlockHash(ctx, st, slot) switch { case errors.Is(err, errActivationNotReached) || errors.Is(err, errNoTerminalBlockHash): - p, err := consensusblocks.WrappedExecutionPayload(emptyPayload()) - if err != nil { - return nil, false, err - } - return p, false, nil + return consensusblocks.NewGetPayloadResponse(emptyPayload()) case err != nil: - return nil, false, err + return nil, err } payloadIDCacheMiss.Inc() random, err := helpers.RandaoMix(st, time.CurrentEpoch(st)) if err != nil { - return nil, false, err + return nil, err } finalizedBlockHash := [32]byte{} @@ -126,14 +124,14 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe t, err := slots.ToTime(st.GenesisTime(), slot) if err != nil { - return nil, false, err + return nil, err } var attr payloadattribute.Attributer switch st.Version() { case version.Deneb, version.Electra: withdrawals, _, err := st.ExpectedWithdrawals() if err != nil { - return nil, false, err + return nil, err } attr, err = payloadattribute.New(&enginev1.PayloadAttributesV3{ Timestamp: uint64(t.Unix()), @@ -143,12 +141,12 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe ParentBeaconBlockRoot: headRoot[:], }) if err != nil { - return nil, false, err + return nil, err } case version.Capella: withdrawals, _, err := st.ExpectedWithdrawals() if err != nil { - return nil, false, err + return nil, err } attr, err = payloadattribute.New(&enginev1.PayloadAttributesV2{ Timestamp: uint64(t.Unix()), @@ -157,7 +155,7 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe Withdrawals: withdrawals, }) if err != nil { - return nil, false, err + return nil, err } case version.Bellatrix: attr, err = payloadattribute.New(&enginev1.PayloadAttributes{ @@ -166,28 +164,28 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe SuggestedFeeRecipient: val.FeeRecipient[:], }) if err != nil { - return nil, false, err + return nil, err } default: - return nil, false, errors.New("unknown beacon state version") + return nil, errors.New("unknown beacon state version") } payloadID, _, err := vs.ExecutionEngineCaller.ForkchoiceUpdated(ctx, f, attr) if err != nil { - return nil, false, errors.Wrap(err, "could not prepare payload") + return nil, errors.Wrap(err, "could not prepare payload") } if payloadID == nil { - return nil, false, fmt.Errorf("nil payload with block hash: %#x", parentHash) + return nil, fmt.Errorf("nil payload with block hash: %#x", parentHash) } res, err := vs.ExecutionEngineCaller.GetPayload(ctx, *payloadID, slot) if err != nil { - return nil, false, err + return nil, err } - // TODO: remove bundleCache and use the blobs directly in the payload. - bundleCache.add(slot, res.BlobsBundle) + // TODO: remove bundleCache code + //bundleCache.add(slot, res.BlobsBundle) warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) log.WithField("value", res.Bid).Debug("received execution payload from local engine") - return res.ExecutionData, res.OverrideBuilder, nil + return res, nil } // warnIfFeeRecipientDiffers logs a warning if the fee recipient in the included payload does not @@ -236,20 +234,20 @@ func (vs *Server) getTerminalBlockHashIfExists(ctx context.Context, transitionTi func (vs *Server) getBuilderPayloadAndBlobs(ctx context.Context, slot primitives.Slot, - vIdx primitives.ValidatorIndex) (interfaces.ExecutionData, [][]byte, error) { + vIdx primitives.ValidatorIndex) (builder.Bid, error) { ctx, span := trace.StartSpan(ctx, "ProposerServer.getBuilderPayloadAndBlobs") defer span.End() if slots.ToEpoch(slot) < params.BeaconConfig().BellatrixForkEpoch { - return nil, nil, nil + return nil, nil } canUseBuilder, err := vs.canUseBuilder(ctx, slot, vIdx) if err != nil { - return nil, nil, errors.Wrap(err, "failed to check if we can use the builder") + return nil, errors.Wrap(err, "failed to check if we can use the builder") } span.AddAttributes(trace.BoolAttribute("canUseBuilder", canUseBuilder)) if !canUseBuilder { - return nil, nil, nil + return nil, nil } return vs.getPayloadHeaderFromBuilder(ctx, slot, vIdx) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go index cd18cdddd3db..6d3d710085be 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go @@ -166,12 +166,11 @@ func TestServer_getExecutionPayload(t *testing.T) { blk.Block.ParentRoot = bytesutil.PadTo([]byte{'a'}, 32) b, err := blocks.NewSignedBeaconBlock(blk) require.NoError(t, err) - var gotOverride bool - _, gotOverride, err = vs.getLocalPayload(context.Background(), b.Block(), tt.st) + res, err := vs.getLocalPayload(context.Background(), b.Block(), tt.st) if tt.errString != "" { require.ErrorContains(t, tt.errString, err) } else { - require.Equal(t, tt.wantedOverride, gotOverride) + require.Equal(t, tt.wantedOverride, res.OverrideBuilder) require.NoError(t, err) } }) @@ -213,7 +212,7 @@ func TestServer_getExecutionPayloadContextTimeout(t *testing.T) { blk.Block.ParentRoot = bytesutil.PadTo([]byte{'a'}, 32) b, err := blocks.NewSignedBeaconBlock(blk) require.NoError(t, err) - _, _, err = vs.getLocalPayload(context.Background(), b.Block(), nonTransitionSt) + _, err = vs.getLocalPayload(context.Background(), b.Block(), nonTransitionSt) require.NoError(t, err) } @@ -270,10 +269,10 @@ func TestServer_getExecutionPayload_UnexpectedFeeRecipient(t *testing.T) { blk.Block.ParentRoot = bytesutil.PadTo([]byte{}, 32) b, err := blocks.NewSignedBeaconBlock(blk) require.NoError(t, err) - gotPayload, _, err := vs.getLocalPayload(context.Background(), b.Block(), transitionSt) + res, err := vs.getLocalPayload(context.Background(), b.Block(), transitionSt) require.NoError(t, err) - require.NotNil(t, gotPayload) - require.Equal(t, common.Address(gotPayload.FeeRecipient()), feeRecipient) + require.NotNil(t, res) + require.Equal(t, common.Address(res.ExecutionData.FeeRecipient()), feeRecipient) // We should NOT be getting the warning. require.LogsDoNotContain(t, hook, "Fee recipient address from execution client is not what was expected") @@ -283,9 +282,9 @@ func TestServer_getExecutionPayload_UnexpectedFeeRecipient(t *testing.T) { payload.FeeRecipient = evilRecipientAddress[:] vs.PayloadIDCache = cache.NewPayloadIDCache() - gotPayload, _, err = vs.getLocalPayload(context.Background(), b.Block(), transitionSt) + res, err = vs.getLocalPayload(context.Background(), b.Block(), transitionSt) require.NoError(t, err) - require.NotNil(t, gotPayload) + require.NotNil(t, res) // Users should be warned. require.LogsContain(t, hook, "Fee recipient address from execution client is not what was expected") diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index cfa174da5a2b..47ca908c6045 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -14,7 +14,8 @@ type GetPayloadResponse struct { ExecutionData interfaces.ExecutionData BlobsBundle *enginev1.BlobsBundle OverrideBuilder bool - Bid primitives.Wei + // todo: should we convert this to Gwei up front? + Bid primitives.Wei } // bundleGetter is an interface satisfied by get payload responses that have a blobs bundle. From 6d47b8f5096fdf6835450b1cc91b8a25814cbcff Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Tue, 21 May 2024 13:37:18 -0500 Subject: [PATCH 04/16] get bid value from api bid (not ExecutionData) --- api/client/builder/bid.go | 20 +++++++++- api/client/builder/client_test.go | 3 +- api/client/builder/types.go | 10 ++++- .../rpc/prysm/v1alpha1/validator/proposer.go | 26 ++++--------- .../v1alpha1/validator/proposer_bellatrix.go | 35 +++++++++++------- .../validator/proposer_bellatrix_test.go | 37 ++++++++----------- consensus-types/blocks/get_payload.go | 2 +- consensus-types/primitives/wei.go | 6 +-- consensus-types/primitives/wei_test.go | 5 +++ 9 files changed, 81 insertions(+), 63 deletions(-) diff --git a/api/client/builder/bid.go b/api/client/builder/bid.go index a62ece939994..ee9861e5a64b 100644 --- a/api/client/builder/bid.go +++ b/api/client/builder/bid.go @@ -24,6 +24,7 @@ type Bid interface { Header() (interfaces.ExecutionData, error) BlobKzgCommitments() ([][]byte, error) Value() []byte + WeiValue() primitives.Wei Pubkey() []byte Version() int IsNil() bool @@ -130,6 +131,11 @@ func (b builderBid) Value() []byte { return b.p.Value } +// WeiValue -- +func (b builderBid) WeiValue() primitives.Wei { + return primitives.LittleEndianBytesToWei(b.p.Value) +} + // Pubkey -- func (b builderBid) Pubkey() []byte { return b.p.Pubkey @@ -166,7 +172,7 @@ func WrappedBuilderBidCapella(p *ethpb.BuilderBidCapella) (Bid, error) { // Header returns the execution data interface. func (b builderBidCapella) Header() (interfaces.ExecutionData, error) { // We have to convert big endian to little endian because the value is coming from the execution layer. - return blocks.WrappedExecutionPayloadHeaderCapella(b.p.Header, primitives.BigEndianBytesToWei(b.p.Value)) + return blocks.WrappedExecutionPayloadHeaderCapella(b.p.Header, primitives.LittleEndianBytesToWei(b.p.Value)) } // BlobKzgCommitments -- @@ -184,6 +190,11 @@ func (b builderBidCapella) Value() []byte { return b.p.Value } +// WeiValue -- +func (b builderBidCapella) WeiValue() primitives.Wei { + return primitives.LittleEndianBytesToWei(b.p.Value) +} + // Pubkey -- func (b builderBidCapella) Pubkey() []byte { return b.p.Pubkey @@ -227,6 +238,11 @@ func (b builderBidDeneb) Value() []byte { return b.p.Value } +// WeiValue -- +func (b builderBidDeneb) WeiValue() primitives.Wei { + return primitives.LittleEndianBytesToWei(b.p.Value) +} + // Pubkey -- func (b builderBidDeneb) Pubkey() []byte { return b.p.Pubkey @@ -250,7 +266,7 @@ func (b builderBidDeneb) HashTreeRootWith(hh *ssz.Hasher) error { // Header -- func (b builderBidDeneb) Header() (interfaces.ExecutionData, error) { // We have to convert big endian to little endian because the value is coming from the execution layer. - return blocks.WrappedExecutionPayloadHeaderDeneb(b.p.Header, primitives.BigEndianBytesToWei(b.p.Value)) + return blocks.WrappedExecutionPayloadHeaderDeneb(b.p.Header, primitives.LittleEndianBytesToWei(b.p.Value)) } // BlobKzgCommitments -- diff --git a/api/client/builder/client_test.go b/api/client/builder/client_test.go index eaa8f1f53068..2a53bf69bcd0 100644 --- a/api/client/builder/client_test.go +++ b/api/client/builder/client_test.go @@ -16,6 +16,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/api/server/structs" "github.com/prysmaticlabs/prysm/v5/config/params" "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" + "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" types "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" v1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" @@ -200,7 +201,7 @@ func TestClient_GetHeader(t *testing.T) { require.Equal(t, uint64(1), bidHeader.GasUsed()) value, err := stringToUint256("652312848583266388373324160190187140051835877600158453279131187530910662656") require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%#x", value.SSZBytes()), fmt.Sprintf("%#x", bid.Value())) + require.Equal(t, fmt.Sprintf("%#x", value.Bytes()), fmt.Sprintf("%#x", primitives.WeiToBigInt(bid.WeiValue()).Bytes())) bidValue := bytesutil.ReverseByteOrder(bid.Value()) require.DeepEqual(t, bidValue, value.Bytes()) require.DeepEqual(t, big.NewInt(0).SetBytes(bidValue), value.Int) diff --git a/api/client/builder/types.go b/api/client/builder/types.go index 29905096739d..169df6822ebd 100644 --- a/api/client/builder/types.go +++ b/api/client/builder/types.go @@ -156,6 +156,8 @@ func (bb *BuilderBid) ToProto() (*eth.BuilderBid, error) { } return ð.BuilderBid{ Header: header, + // Note that SSZBytes() reverses byte order for the little-endian representation. + // Uint256.Bytes() is big-endian, SSZBytes takes this value and reverses it. Value: bb.Value.SSZBytes(), Pubkey: bb.Pubkey, }, nil @@ -484,6 +486,8 @@ func (bb *BuilderBidCapella) ToProto() (*eth.BuilderBidCapella, error) { } return ð.BuilderBidCapella{ Header: header, + // Note that SSZBytes() reverses byte order for the little-endian representation. + // Uint256.Bytes() is big-endian, SSZBytes takes this value and reverses it. Value: bytesutil.SafeCopyBytes(bb.Value.SSZBytes()), Pubkey: bytesutil.SafeCopyBytes(bb.Pubkey), }, nil @@ -1022,8 +1026,10 @@ func (bb *BuilderBidDeneb) ToProto() (*eth.BuilderBidDeneb, error) { return ð.BuilderBidDeneb{ Header: header, BlobKzgCommitments: kzgCommitments, - Value: bytesutil.SafeCopyBytes(bb.Value.SSZBytes()), - Pubkey: bytesutil.SafeCopyBytes(bb.Pubkey), + // Note that SSZBytes() reverses byte order for the little-endian representation. + // Uint256.Bytes() is big-endian, SSZBytes takes this value and reverses it. + Value: bytesutil.SafeCopyBytes(bb.Value.SSZBytes()), + Pubkey: bytesutil.SafeCopyBytes(bb.Pubkey), }, nil } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index c1eebfcd0133..08d481d60531 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -11,6 +11,7 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" emptypb "github.com/golang/protobuf/ptypes/empty" "github.com/pkg/errors" + builderapi "github.com/prysmaticlabs/prysm/v5/api/client/builder" "github.com/prysmaticlabs/prysm/v5/beacon-chain/blockchain" "github.com/prysmaticlabs/prysm/v5/beacon-chain/builder" "github.com/prysmaticlabs/prysm/v5/beacon-chain/cache" @@ -239,32 +240,19 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed } // There's no reason to try to get a builder bid if local override is true. - var builderPayload interfaces.ExecutionData - var builderKzgCommitments [][]byte + var builderBid builderapi.Bid if local.OverrideBuilder || skipMevBoost { - builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, sBlk.Block().Slot(), sBlk.Block().ProposerIndex()) + builderBid, err = vs.getBuilderPayloadAndBlobs(ctx, sBlk.Block().Slot(), sBlk.Block().ProposerIndex()) if err != nil { builderGetPayloadMissCount.Inc() log.WithError(err).Error("Could not get builder payload") - } - // getBuidlerPayloadAndBlobs can return `nil, nil` for the condition where no builder is configured... - if builderBid != nil { - builderPayload, err = builderBid.Header() - if err != nil { - builderGetPayloadMissCount.Inc() - log.WithError(err).Error("Could not get builder payload") - } - if builderBid.Version() > version.Deneb { - builderKzgCommitments, err = builderBid.BlobKzgCommitments() - if err != nil { - builderGetPayloadMissCount.Inc() - log.WithError(err).Error("deneb payload does not have commitments") - } - } + } else if builderBid == nil { + builderGetPayloadMissCount.Inc() + log.WithError(err).Error("Could not get builder payload") } } - if err := setExecutionData(ctx, sBlk, local, builderPayload, builderKzgCommitments, builderBoostFactor); err != nil { + if err := setExecutionData(ctx, sBlk, local, builderBid, builderBoostFactor); err != nil { return status.Errorf(codes.Internal, "Could not set execution data: %v", err) } } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index 15915aa071b7..728103dfda46 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "math/big" "strings" "time" @@ -51,7 +52,7 @@ var emptyTransactionsRoot = [32]byte{127, 254, 36, 30, 166, 1, 135, 253, 176, 24 const blockBuilderTimeout = 1 * time.Second // Sets the execution data for the block. Execution data can come from local EL client or remote builder depends on validator registration and circuit breaker conditions. -func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, local *blocks.GetPayloadResponse, builderPayload interfaces.ExecutionData, builderKzgCommitments [][]byte, builderBoostFactor primitives.Gwei) error { +func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, local *blocks.GetPayloadResponse, bid builder.Bid, builderBoostFactor primitives.Gwei) error { _, span := trace.StartSpan(ctx, "ProposerServer.setExecutionData") defer span.End() @@ -65,22 +66,25 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc } // Use local payload if builder payload is nil. - if builderPayload == nil { + if bid == nil { return setLocalExecution(blk, local) } - switch { - case blk.Version() >= version.Capella: - // Compare payload values between local and builder. Default to the local value if it is higher. - localValueGwei := primitives.WeiToGwei(local.Bid) - // TODO: plumb builder gwei value through the bid instead of getting from ExecutionData. - buildValueUint, err := builderPayload.ValueInGwei() + var builderKzgCommitments [][]byte + builderPayload, err := bid.Header() + if err != nil { + log.WithError(err).Warn("Proposer: failed to retrieve header from BuilderBid") + return setLocalExecution(blk, local) + } + if bid.Version() >= version.Deneb { + builderKzgCommitments, err = bid.BlobKzgCommitments() if err != nil { - log.WithError(err).Warn("Proposer: failed to get builder payload value") // Default to local if can't get builder value. - return setLocalExecution(blk, local) + log.WithError(err).Warn("Proposer: failed to retrieve kzg commitments from BuilderBid") } - builderValueGwei := primitives.Gwei(buildValueUint) + } + switch { + case blk.Version() >= version.Capella: withdrawalsMatched, err := matchingWithdrawalsRoot(local.ExecutionData, builderPayload) if err != nil { tracing.AnnotateError(span, err) @@ -88,6 +92,9 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc return setLocalExecution(blk, local) } + // Compare payload values between local and builder. Default to the local value if it is higher. + localValueGwei := primitives.WeiToGwei(local.Bid) + builderValueGwei := primitives.WeiToGwei(bid.WeiValue()) // Use builder payload if the following in true: // builder_bid_value * builderBoostFactor(default 100) > local_block_value * (local-block-value-boost + 100) boost := primitives.Gwei(params.BeaconConfig().LocalBlockValueBoost) @@ -192,8 +199,8 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv return nil, errors.New("builder returned nil bid") } - v := bytesutil.LittleEndianBytesToBigInt(bid.Value()) - if v.String() == "0" { + v := bid.WeiValue() + if big.NewInt(0).Cmp(v) == 0 { return nil, errors.New("builder returned header with 0 bid amount") } @@ -255,7 +262,7 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv l.Info("Received header with bid") span.AddAttributes( - trace.StringAttribute("value", v.String()), + trace.StringAttribute("value", primitives.WeiToBigInt(v).String()), trace.StringAttribute("builderPubKey", fmt.Sprintf("%#x", bid.Pubkey())), trace.StringAttribute("blockHash", fmt.Sprintf("%#x", header.BlockHash())), ) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go index 7d654f7a64ad..5c296985d000 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -103,11 +103,8 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - var builderKzgCommitments [][]byte - var builderPayload interfaces.ExecutionData require.IsNil(t, builderBid) - require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block @@ -167,14 +164,14 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - builderPayload, err := builderBid.Header() + _, err = builderBid.Header() require.NoError(t, err) builderKzgCommitments, err := builderBid.BlobKzgCommitments() if builderBid.Version() >= version.Deneb { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block because incorrect withdrawals @@ -237,14 +234,14 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - builderPayload, err := builderBid.Header() + _, err = builderBid.Header() require.NoError(t, err) builderKzgCommitments, err := builderBid.BlobKzgCommitments() if builderBid.Version() >= version.Deneb { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // Builder block @@ -306,14 +303,14 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - builderPayload, err := builderBid.Header() + _, err = builderBid.Header() require.NoError(t, err) builderKzgCommitments, err := builderBid.BlobKzgCommitments() if builderBid.Version() >= version.Deneb { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, math.MaxUint64)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, math.MaxUint64)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // builder block @@ -375,14 +372,14 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - builderPayload, err := builderBid.Header() + _, err = builderBid.Header() require.NoError(t, err) builderKzgCommitments, err := builderBid.BlobKzgCommitments() if builderBid.Version() >= version.Deneb { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, 0)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, 0)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // local block @@ -399,14 +396,14 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) - builderPayload, err := builderBid.Header() + _, err = builderBid.Header() require.NoError(t, err) builderKzgCommitments, err := builderBid.BlobKzgCommitments() if builderBid.Version() >= version.Deneb { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(3), e.BlockNumber()) // Local block @@ -434,10 +431,10 @@ func TestServer_setExecutionData(t *testing.T) { if builderBid.Version() >= version.Deneb { require.NoError(t, err) } - builderPayload, err := builderBid.Header() + _, err = builderBid.Header() require.NoError(t, err) require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(3), e.BlockNumber()) // Local block @@ -461,7 +458,7 @@ func TestServer_setExecutionData(t *testing.T) { builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.ErrorIs(t, consensus_types.ErrNilObjectWrapped, err) // Builder returns fault. Use local block require.IsNil(t, builderBid) - require.NoError(t, setExecutionData(context.Background(), blk, res, nil, nil, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, nil, defaultBuilderBoostFactor)) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(4), e.BlockNumber()) // Local block @@ -577,15 +574,13 @@ func TestServer_setExecutionData(t *testing.T) { builderPayload, err := builderBid.Header() require.NoError(t, err) builderKzgCommitments, err := builderBid.BlobKzgCommitments() - if builderBid.Version() >= version.Deneb { - require.NoError(t, err) - } + require.NoError(t, err) require.DeepEqual(t, bid.BlobKzgCommitments, builderKzgCommitments) require.Equal(t, bid.Header.BlockNumber, builderPayload.BlockNumber()) // header should be the same from block res, err := vs.getLocalPayload(ctx, blk.Block(), denebTransitionState) require.NoError(t, err) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderPayload, builderKzgCommitments, defaultBuilderBoostFactor)) + require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) got, err := blk.Block().Body().BlobKzgCommitments() require.NoError(t, err) diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index 47ca908c6045..9a56df854481 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -41,7 +41,7 @@ func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { bidValueGetter, hasBid := msg.(bidValueGetter) wei := primitives.ZeroWei if hasBid { - wei = primitives.BigEndianBytesToWei(bidValueGetter.GetValue()) + wei = primitives.LittleEndianBytesToWei(bidValueGetter.GetValue()) r.Bid = wei } shouldOverride, hasShouldOverride := msg.(shouldOverrideBuilderGetter) diff --git a/consensus-types/primitives/wei.go b/consensus-types/primitives/wei.go index b74a95e806f2..9dc40046f04a 100644 --- a/consensus-types/primitives/wei.go +++ b/consensus-types/primitives/wei.go @@ -71,11 +71,11 @@ func Uint64ToWei(v uint64) Wei { return big.NewInt(0).SetUint64(v) } -// BigEndianBytesToWei returns a Wei value given a big-endian binary representation (eg engine api payload bids). -func BigEndianBytesToWei(value []byte) Wei { +// LittleEndianBytesToWei returns a Wei value given a little-endian binary representation. +func LittleEndianBytesToWei(value []byte) Wei { v := make([]byte, len(value)) copy(v, value) - // We have to convert big endian to little endian because the value is coming from the execution layer. + // SetBytes expects a big-endian representation of the value, so we reverse the byte slice. slices.Reverse(v) return big.NewInt(0).SetBytes(v) } diff --git a/consensus-types/primitives/wei_test.go b/consensus-types/primitives/wei_test.go index dd9ff308aeaf..c8276968fc62 100644 --- a/consensus-types/primitives/wei_test.go +++ b/consensus-types/primitives/wei_test.go @@ -39,3 +39,8 @@ func TestWeiToGwei_CopyOk(t *testing.T) { require.Equal(t, primitives.Gwei(1), got) require.Equal(t, big.NewInt(1e9).Uint64(), v.Uint64()) } + +func TestZero(t *testing.T) { + z := primitives.ZeroWei + require.Equal(t, 0, big.NewInt(0).Cmp(z)) +} From 62f5acca1166cb47e9f5cd621e12a45ec1b7b077 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Tue, 21 May 2024 20:10:52 -0500 Subject: [PATCH 05/16] plumb bid and bundle through BuildBlockParallel --- .../validator/construct_generic_block.go | 36 +++++++++---------- .../validator/construct_generic_block_test.go | 19 +++++----- .../rpc/prysm/v1alpha1/validator/proposer.go | 21 ++++++----- .../v1alpha1/validator/proposer_bellatrix.go | 23 ++++++------ .../validator/proposer_bellatrix_test.go | 36 ++++++++++++++----- consensus-types/blocks/get_payload.go | 2 +- consensus-types/primitives/wei.go | 3 ++ 7 files changed, 84 insertions(+), 56 deletions(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block.go b/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block.go index e90f3859dde6..a4510e69eead 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block.go @@ -12,7 +12,7 @@ import ( ) // constructGenericBeaconBlock constructs a `GenericBeaconBlock` based on the block version and other parameters. -func (vs *Server) constructGenericBeaconBlock(sBlk interfaces.SignedBeaconBlock, blobsBundle *enginev1.BlobsBundle) (*ethpb.GenericBeaconBlock, error) { +func (vs *Server) constructGenericBeaconBlock(sBlk interfaces.SignedBeaconBlock, blobsBundle *enginev1.BlobsBundle, winningBid primitives.Wei) (*ethpb.GenericBeaconBlock, error) { if sBlk == nil || sBlk.Block() == nil { return nil, fmt.Errorf("block cannot be nil") } @@ -23,17 +23,17 @@ func (vs *Server) constructGenericBeaconBlock(sBlk interfaces.SignedBeaconBlock, } isBlinded := sBlk.IsBlinded() - payloadValue := sBlk.ValueInWei() + bidStr := primitives.WeiToBigInt(winningBid).String() switch sBlk.Version() { case version.Electra: - return vs.constructElectraBlock(blockProto, isBlinded, payloadValue, blobsBundle), nil + return vs.constructElectraBlock(blockProto, isBlinded, bidStr, blobsBundle), nil case version.Deneb: - return vs.constructDenebBlock(blockProto, isBlinded, payloadValue, blobsBundle), nil + return vs.constructDenebBlock(blockProto, isBlinded, bidStr, blobsBundle), nil case version.Capella: - return vs.constructCapellaBlock(blockProto, isBlinded, payloadValue), nil + return vs.constructCapellaBlock(blockProto, isBlinded, bidStr), nil case version.Bellatrix: - return vs.constructBellatrixBlock(blockProto, isBlinded, payloadValue), nil + return vs.constructBellatrixBlock(blockProto, isBlinded, bidStr), nil case version.Altair: return vs.constructAltairBlock(blockProto), nil case version.Phase0: @@ -44,42 +44,42 @@ func (vs *Server) constructGenericBeaconBlock(sBlk interfaces.SignedBeaconBlock, } // Helper functions for constructing blocks for each version -func (vs *Server) constructElectraBlock(blockProto proto.Message, isBlinded bool, payloadValue primitives.Wei, bundle *enginev1.BlobsBundle) *ethpb.GenericBeaconBlock { +func (vs *Server) constructElectraBlock(blockProto proto.Message, isBlinded bool, payloadValue string, bundle *enginev1.BlobsBundle) *ethpb.GenericBeaconBlock { if isBlinded { - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedElectra{BlindedElectra: blockProto.(*ethpb.BlindedBeaconBlockElectra)}, IsBlinded: true, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedElectra{BlindedElectra: blockProto.(*ethpb.BlindedBeaconBlockElectra)}, IsBlinded: true, PayloadValue: payloadValue} } electraContents := ðpb.BeaconBlockContentsElectra{Block: blockProto.(*ethpb.BeaconBlockElectra)} if bundle != nil { electraContents.KzgProofs = bundle.Proofs electraContents.Blobs = bundle.Blobs } - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Electra{Electra: electraContents}, IsBlinded: false, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Electra{Electra: electraContents}, IsBlinded: false, PayloadValue: payloadValue} } -func (vs *Server) constructDenebBlock(blockProto proto.Message, isBlinded bool, payloadValue primitives.Wei, bundle *enginev1.BlobsBundle) *ethpb.GenericBeaconBlock { +func (vs *Server) constructDenebBlock(blockProto proto.Message, isBlinded bool, payloadValue string, bundle *enginev1.BlobsBundle) *ethpb.GenericBeaconBlock { if isBlinded { - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedDeneb{BlindedDeneb: blockProto.(*ethpb.BlindedBeaconBlockDeneb)}, IsBlinded: true, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedDeneb{BlindedDeneb: blockProto.(*ethpb.BlindedBeaconBlockDeneb)}, IsBlinded: true, PayloadValue: payloadValue} } denebContents := ðpb.BeaconBlockContentsDeneb{Block: blockProto.(*ethpb.BeaconBlockDeneb)} if bundle != nil { denebContents.KzgProofs = bundle.Proofs denebContents.Blobs = bundle.Blobs } - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Deneb{Deneb: denebContents}, IsBlinded: false, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Deneb{Deneb: denebContents}, IsBlinded: false, PayloadValue: payloadValue} } -func (vs *Server) constructCapellaBlock(pb proto.Message, isBlinded bool, payloadValue primitives.Wei) *ethpb.GenericBeaconBlock { +func (vs *Server) constructCapellaBlock(pb proto.Message, isBlinded bool, payloadValue string) *ethpb.GenericBeaconBlock { if isBlinded { - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedCapella{BlindedCapella: pb.(*ethpb.BlindedBeaconBlockCapella)}, IsBlinded: true, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedCapella{BlindedCapella: pb.(*ethpb.BlindedBeaconBlockCapella)}, IsBlinded: true, PayloadValue: payloadValue} } - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Capella{Capella: pb.(*ethpb.BeaconBlockCapella)}, IsBlinded: false, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Capella{Capella: pb.(*ethpb.BeaconBlockCapella)}, IsBlinded: false, PayloadValue: payloadValue} } -func (vs *Server) constructBellatrixBlock(pb proto.Message, isBlinded bool, payloadValue primitives.Wei) *ethpb.GenericBeaconBlock { +func (vs *Server) constructBellatrixBlock(pb proto.Message, isBlinded bool, payloadValue string) *ethpb.GenericBeaconBlock { if isBlinded { - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedBellatrix{BlindedBellatrix: pb.(*ethpb.BlindedBeaconBlockBellatrix)}, IsBlinded: true, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_BlindedBellatrix{BlindedBellatrix: pb.(*ethpb.BlindedBeaconBlockBellatrix)}, IsBlinded: true, PayloadValue: payloadValue} } - return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Bellatrix{Bellatrix: pb.(*ethpb.BeaconBlockBellatrix)}, IsBlinded: false, PayloadValue: (*payloadValue).String()} + return ðpb.GenericBeaconBlock{Block: ðpb.GenericBeaconBlock_Bellatrix{Bellatrix: pb.(*ethpb.BeaconBlockBellatrix)}, IsBlinded: false, PayloadValue: payloadValue} } func (vs *Server) constructAltairBlock(pb proto.Message) *ethpb.GenericBeaconBlock { diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block_test.go index 53787b276df9..d3ecf2323e77 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/construct_generic_block_test.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" + "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v5/testing/require" @@ -17,7 +18,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { // Test when sBlk or sBlk.Block() is nil t.Run("NilBlock", func(t *testing.T) { - _, err := vs.constructGenericBeaconBlock(nil, nil) + _, err := vs.constructGenericBeaconBlock(nil, nil, primitives.ZeroWei) require.ErrorContains(t, "block cannot be nil", err) }) @@ -38,7 +39,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { require.NoError(t, err) r1, err := eb.Block.HashTreeRoot() require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, nil) + result, err := vs.constructGenericBeaconBlock(b, nil, primitives.ZeroWei) require.NoError(t, err) r2, err := result.GetElectra().Block.HashTreeRoot() require.NoError(t, err) @@ -70,7 +71,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { contents := ð.BeaconBlockContentsDeneb{Block: eb.Block, KzgProofs: bundle.Proofs, Blobs: bundle.Blobs} r1, err := contents.HashTreeRoot() require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, bundle) + result, err := vs.constructGenericBeaconBlock(b, bundle, primitives.ZeroWei) require.NoError(t, err) r2, err := result.GetDeneb().HashTreeRoot() require.NoError(t, err) @@ -85,7 +86,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { r1, err := b.Block().HashTreeRoot() require.NoError(t, err) scs := &enginev1.BlobsBundle{} - result, err := vs.constructGenericBeaconBlock(b, scs) + result, err := vs.constructGenericBeaconBlock(b, scs, primitives.ZeroWei) require.NoError(t, err) r2, err := result.GetBlindedDeneb().HashTreeRoot() require.NoError(t, err) @@ -98,7 +99,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { t.Run("capella block", func(t *testing.T) { b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, nil) + result, err := vs.constructGenericBeaconBlock(b, nil, primitives.ZeroWei) require.NoError(t, err) r1, err := result.GetCapella().HashTreeRoot() require.NoError(t, err) @@ -112,7 +113,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { t.Run("blind capella block", func(t *testing.T) { b, err := blocks.NewSignedBeaconBlock(util.NewBlindedBeaconBlockCapella()) require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, nil) + result, err := vs.constructGenericBeaconBlock(b, nil, primitives.ZeroWei) require.NoError(t, err) r1, err := result.GetBlindedCapella().HashTreeRoot() require.NoError(t, err) @@ -126,7 +127,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { t.Run("bellatrix block", func(t *testing.T) { b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockBellatrix()) require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, nil) + result, err := vs.constructGenericBeaconBlock(b, nil, primitives.ZeroWei) require.NoError(t, err) r1, err := result.GetBellatrix().HashTreeRoot() require.NoError(t, err) @@ -140,7 +141,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { t.Run("altair block", func(t *testing.T) { b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockAltair()) require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, nil) + result, err := vs.constructGenericBeaconBlock(b, nil, primitives.ZeroWei) require.NoError(t, err) r1, err := result.GetAltair().HashTreeRoot() require.NoError(t, err) @@ -154,7 +155,7 @@ func TestConstructGenericBeaconBlock(t *testing.T) { t.Run("phase0 block", func(t *testing.T) { b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlock()) require.NoError(t, err) - result, err := vs.constructGenericBeaconBlock(b, nil) + result, err := vs.constructGenericBeaconBlock(b, nil, primitives.ZeroWei) require.NoError(t, err) r1, err := result.GetPhase0().HashTreeRoot() require.NoError(t, err) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index 08d481d60531..6934de16a7da 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -26,6 +26,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" + enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v5/runtime/version" "github.com/prysmaticlabs/prysm/v5/time/slots" @@ -97,7 +98,8 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) ( builderBoostFactor = primitives.Gwei(req.BuilderBoostFactor.Value) } - if err = vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost, builderBoostFactor); err != nil { + winningBid, bundle, err := vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost, builderBoostFactor) + if err != nil { return nil, errors.Wrap(err, "could not build block in parallel") } @@ -114,7 +116,7 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) ( }).Info("Finished building block") // Blob cache is updated after BuildBlockParallel - return vs.constructGenericBeaconBlock(sBlk, bundleCache.get(req.Slot)) + return vs.constructGenericBeaconBlock(sBlk, bundle, winningBid) } func (vs *Server) handleSuccesfulReorgAttempt(ctx context.Context, slot primitives.Slot, parentRoot, headRoot [32]byte) (state.BeaconState, error) { @@ -185,7 +187,7 @@ func (vs *Server) getParentState(ctx context.Context, slot primitives.Slot) (sta return head, parentRoot, err } -func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor primitives.Gwei) error { +func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor primitives.Gwei) (primitives.Wei, *enginev1.BlobsBundle, error) { // Build consensus fields in background var wg sync.WaitGroup wg.Add(1) @@ -233,15 +235,17 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed }() blockEpoch := slots.ToEpoch(sBlk.Block().Slot()) + winningBid := primitives.ZeroWei + var bundle *enginev1.BlobsBundle if blockEpoch >= params.BeaconConfig().BellatrixForkEpoch { local, err := vs.getLocalPayload(ctx, sBlk.Block(), head) if err != nil { - return status.Errorf(codes.Internal, "Could not get local payload: %v", err) + return primitives.ZeroWei, nil, status.Errorf(codes.Internal, "Could not get local payload: %v", err) } // There's no reason to try to get a builder bid if local override is true. var builderBid builderapi.Bid - if local.OverrideBuilder || skipMevBoost { + if !(local.OverrideBuilder || skipMevBoost) { builderBid, err = vs.getBuilderPayloadAndBlobs(ctx, sBlk.Block().Slot(), sBlk.Block().ProposerIndex()) if err != nil { builderGetPayloadMissCount.Inc() @@ -252,14 +256,15 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed } } - if err := setExecutionData(ctx, sBlk, local, builderBid, builderBoostFactor); err != nil { - return status.Errorf(codes.Internal, "Could not set execution data: %v", err) + winningBid, bundle, err = setExecutionData(ctx, sBlk, local, builderBid, builderBoostFactor) + if err != nil { + return primitives.ZeroWei, nil, status.Errorf(codes.Internal, "Could not set execution data: %v", err) } } wg.Wait() // Wait until block is built via consensus and execution fields. - return nil + return winningBid, bundle, nil } // ProposeBeaconBlock handles the proposal of beacon blocks. diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index 728103dfda46..6afd343313b7 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go @@ -22,6 +22,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/encoding/ssz" "github.com/prysmaticlabs/prysm/v5/monitoring/tracing" "github.com/prysmaticlabs/prysm/v5/network/forks" + enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" "github.com/prysmaticlabs/prysm/v5/runtime/version" "github.com/prysmaticlabs/prysm/v5/time/slots" "github.com/sirupsen/logrus" @@ -52,29 +53,29 @@ var emptyTransactionsRoot = [32]byte{127, 254, 36, 30, 166, 1, 135, 253, 176, 24 const blockBuilderTimeout = 1 * time.Second // Sets the execution data for the block. Execution data can come from local EL client or remote builder depends on validator registration and circuit breaker conditions. -func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, local *blocks.GetPayloadResponse, bid builder.Bid, builderBoostFactor primitives.Gwei) error { +func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, local *blocks.GetPayloadResponse, bid builder.Bid, builderBoostFactor primitives.Gwei) (primitives.Wei, *enginev1.BlobsBundle, error) { _, span := trace.StartSpan(ctx, "ProposerServer.setExecutionData") defer span.End() slot := blk.Block().Slot() if slots.ToEpoch(slot) < params.BeaconConfig().BellatrixForkEpoch { - return nil + return primitives.ZeroWei, nil, nil } if local == nil { - return errors.New("local payload is nil") + return primitives.ZeroWei, nil, errors.New("local payload is nil") } // Use local payload if builder payload is nil. if bid == nil { - return setLocalExecution(blk, local) + return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } var builderKzgCommitments [][]byte builderPayload, err := bid.Header() if err != nil { log.WithError(err).Warn("Proposer: failed to retrieve header from BuilderBid") - return setLocalExecution(blk, local) + return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } if bid.Version() >= version.Deneb { builderKzgCommitments, err = bid.BlobKzgCommitments() @@ -89,7 +90,7 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc if err != nil { tracing.AnnotateError(span, err) log.WithError(err).Warn("Proposer: failed to match withdrawals root") - return setLocalExecution(blk, local) + return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } // Compare payload values between local and builder. Default to the local value if it is higher. @@ -114,9 +115,9 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc if higherValueBuilder && withdrawalsMatched { // Builder value is higher and withdrawals match. if err := setBuilderExecution(blk, builderPayload, builderKzgCommitments); err != nil { log.WithError(err).Warn("Proposer: failed to set builder payload") - return setLocalExecution(blk, local) + return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } else { - return nil + return bid.WeiValue(), nil, nil } } if !higherValueBuilder { @@ -134,13 +135,13 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc trace.Int64Attribute("builderGweiValue", int64(builderValueGwei)), // lint:ignore uintcast -- This is OK for tracing. trace.Int64Attribute("builderBoostFactor", int64(builderBoostFactor)), // lint:ignore uintcast -- This is OK for tracing. ) - return setLocalExecution(blk, local) + return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) default: // Bellatrix case. if err := setBuilderExecution(blk, builderPayload, builderKzgCommitments); err != nil { log.WithError(err).Warn("Proposer: failed to set builder payload") - return setLocalExecution(blk, local) + return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } else { - return nil + return bid.WeiValue(), nil, nil } } } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go index 5c296985d000..2e1616ee7234 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -104,7 +104,9 @@ func TestServer_setExecutionData(t *testing.T) { builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.NoError(t, err) require.IsNil(t, builderBid) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block @@ -171,7 +173,9 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block because incorrect withdrawals @@ -241,7 +245,9 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // Builder block @@ -310,7 +316,9 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, math.MaxUint64)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, math.MaxUint64) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // builder block @@ -379,7 +387,9 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, 0)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, 0) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // local block @@ -403,7 +413,9 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) } require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(3), e.BlockNumber()) // Local block @@ -434,7 +446,9 @@ func TestServer_setExecutionData(t *testing.T) { _, err = builderBid.Header() require.NoError(t, err) require.DeepEqual(t, [][]uint8{}, builderKzgCommitments) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(3), e.BlockNumber()) // Local block @@ -458,7 +472,9 @@ func TestServer_setExecutionData(t *testing.T) { builderBid, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex()) require.ErrorIs(t, consensus_types.ErrNilObjectWrapped, err) // Builder returns fault. Use local block require.IsNil(t, builderBid) - require.NoError(t, setExecutionData(context.Background(), blk, res, nil, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, nil, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) e, err := blk.Block().Body().Execution() require.NoError(t, err) require.Equal(t, uint64(4), e.BlockNumber()) // Local block @@ -580,7 +596,9 @@ func TestServer_setExecutionData(t *testing.T) { res, err := vs.getLocalPayload(ctx, blk.Block(), denebTransitionState) require.NoError(t, err) - require.NoError(t, setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor)) + _, bundle, err := setExecutionData(context.Background(), blk, res, builderBid, defaultBuilderBoostFactor) + require.NoError(t, err) + require.IsNil(t, bundle) got, err := blk.Block().Body().BlobKzgCommitments() require.NoError(t, err) diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index 9a56df854481..8a2a64091bbf 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -42,8 +42,8 @@ func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { wei := primitives.ZeroWei if hasBid { wei = primitives.LittleEndianBytesToWei(bidValueGetter.GetValue()) - r.Bid = wei } + r.Bid = wei shouldOverride, hasShouldOverride := msg.(shouldOverrideBuilderGetter) if hasShouldOverride { r.OverrideBuilder = shouldOverride.GetShouldOverrideBuilder() diff --git a/consensus-types/primitives/wei.go b/consensus-types/primitives/wei.go index 9dc40046f04a..8d687d69b4a1 100644 --- a/consensus-types/primitives/wei.go +++ b/consensus-types/primitives/wei.go @@ -73,6 +73,9 @@ func Uint64ToWei(v uint64) Wei { // LittleEndianBytesToWei returns a Wei value given a little-endian binary representation. func LittleEndianBytesToWei(value []byte) Wei { + if len(value) == 0 { + return big.NewInt(0) + } v := make([]byte, len(value)) copy(v, value) // SetBytes expects a big-endian representation of the value, so we reverse the byte slice. From 19ec4f26bdc801d1306d27464ee242b3205afbf2 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Tue, 21 May 2024 20:25:49 -0500 Subject: [PATCH 06/16] rm ValueInGwei --- consensus-types/blocks/BUILD.bazel | 1 - consensus-types/blocks/execution.go | 80 ---------------------- consensus-types/blocks/execution_test.go | 26 ------- consensus-types/blocks/get_payload_test.go | 22 ------ consensus-types/blocks/getters.go | 40 ----------- consensus-types/interfaces/beacon_block.go | 4 -- consensus-types/mock/block.go | 8 --- 7 files changed, 181 deletions(-) delete mode 100644 consensus-types/blocks/get_payload_test.go diff --git a/consensus-types/blocks/BUILD.bazel b/consensus-types/blocks/BUILD.bazel index 1988e1a4e9f3..c39b47ed342a 100644 --- a/consensus-types/blocks/BUILD.bazel +++ b/consensus-types/blocks/BUILD.bazel @@ -32,7 +32,6 @@ go_library( "@com_github_pkg_errors//:go_default_library", "@com_github_prysmaticlabs_fastssz//:go_default_library", "@com_github_prysmaticlabs_gohashtree//:go_default_library", - "@com_github_sirupsen_logrus//:go_default_library", "@org_golang_google_protobuf//proto:go_default_library", ], ) diff --git a/consensus-types/blocks/execution.go b/consensus-types/blocks/execution.go index 5e4a36e82ee6..38897009120f 100644 --- a/consensus-types/blocks/execution.go +++ b/consensus-types/blocks/execution.go @@ -202,16 +202,6 @@ func (e executionPayload) ExcessBlobGas() (uint64, error) { return 0, consensus_types.ErrUnsupportedField } -// ValueInWei -- -func (executionPayload) ValueInWei() (primitives.Wei, error) { - return nil, consensus_types.ErrUnsupportedField -} - -// ValueInGwei -- -func (executionPayload) ValueInGwei() (uint64, error) { - return 0, consensus_types.ErrUnsupportedField -} - // executionPayloadHeader is a convenience wrapper around a blinded beacon block body's execution header data structure // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. @@ -370,16 +360,6 @@ func (e executionPayloadHeader) ExcessBlobGas() (uint64, error) { return 0, consensus_types.ErrUnsupportedField } -// ValueInWei -- -func (executionPayloadHeader) ValueInWei() (primitives.Wei, error) { - return nil, consensus_types.ErrUnsupportedField -} - -// ValueInGwei -- -func (executionPayloadHeader) ValueInGwei() (uint64, error) { - return 0, consensus_types.ErrUnsupportedField -} - // PayloadToHeader converts `payload` into execution payload header format. func PayloadToHeader(payload interfaces.ExecutionData) (*enginev1.ExecutionPayloadHeader, error) { txs, err := payload.Transactions() @@ -568,16 +548,6 @@ func (e executionPayloadCapella) ExcessBlobGas() (uint64, error) { return 0, consensus_types.ErrUnsupportedField } -// ValueInWei -- -func (e executionPayloadCapella) ValueInWei() (primitives.Wei, error) { - return e.weiValue, nil -} - -// ValueInGwei -- -func (e executionPayloadCapella) ValueInGwei() (uint64, error) { - return e.gweiValue, nil -} - // executionPayloadHeaderCapella is a convenience wrapper around a blinded beacon block body's execution header data structure // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. @@ -738,16 +708,6 @@ func (e executionPayloadHeaderCapella) ExcessBlobGas() (uint64, error) { return 0, consensus_types.ErrUnsupportedField } -// ValueInWei -- -func (e executionPayloadHeaderCapella) ValueInWei() (primitives.Wei, error) { - return e.weiValue, nil -} - -// ValueInGwei -- -func (e executionPayloadHeaderCapella) ValueInGwei() (uint64, error) { - return e.gweiValue, nil -} - // PayloadToHeaderCapella converts `payload` into execution payload header format. func PayloadToHeaderCapella(payload interfaces.ExecutionData) (*enginev1.ExecutionPayloadHeaderCapella, error) { txs, err := payload.Transactions() @@ -1125,16 +1085,6 @@ func (e executionPayloadHeaderDeneb) ExcessBlobGas() (uint64, error) { return e.p.ExcessBlobGas, nil } -// ValueInWei -- -func (e executionPayloadHeaderDeneb) ValueInWei() (primitives.Wei, error) { - return e.weiValue, nil -} - -// ValueInGwei -- -func (e executionPayloadHeaderDeneb) ValueInGwei() (uint64, error) { - return e.gweiValue, nil -} - // IsBlinded returns true if the underlying data is blinded. func (e executionPayloadHeaderDeneb) IsBlinded() bool { return true @@ -1293,16 +1243,6 @@ func (e executionPayloadDeneb) ExcessBlobGas() (uint64, error) { return e.p.ExcessBlobGas, nil } -// ValueInWei -- -func (e executionPayloadDeneb) ValueInWei() (primitives.Wei, error) { - return e.weiValue, nil -} - -// ValueInGwei -- -func (e executionPayloadDeneb) ValueInGwei() (uint64, error) { - return e.gweiValue, nil -} - // IsBlinded returns true if the underlying data is blinded. func (e executionPayloadDeneb) IsBlinded() bool { return false @@ -1464,16 +1404,6 @@ func (e executionPayloadHeaderElectra) ExcessBlobGas() (uint64, error) { return e.p.ExcessBlobGas, nil } -// ValueInWei -- -func (e executionPayloadHeaderElectra) ValueInWei() (primitives.Wei, error) { - return e.weiValue, nil -} - -// ValueInGwei -- -func (e executionPayloadHeaderElectra) ValueInGwei() (uint64, error) { - return e.gweiValue, nil -} - // DepositReceipts -- func (e executionPayloadHeaderElectra) DepositReceipts() ([]*enginev1.DepositReceipt, error) { return nil, consensus_types.ErrUnsupportedField @@ -1643,16 +1573,6 @@ func (e executionPayloadElectra) ExcessBlobGas() (uint64, error) { return e.p.ExcessBlobGas, nil } -// ValueInWei -- -func (e executionPayloadElectra) ValueInWei() (primitives.Wei, error) { - return e.weiValue, nil -} - -// ValueInGwei -- -func (e executionPayloadElectra) ValueInGwei() (uint64, error) { - return e.gweiValue, nil -} - // DepositReceipts -- func (e executionPayloadElectra) DepositReceipts() []*enginev1.DepositReceipt { return e.p.DepositReceipts diff --git a/consensus-types/blocks/execution_test.go b/consensus-types/blocks/execution_test.go index f9dc7cc5e75e..8424664069f8 100644 --- a/consensus-types/blocks/execution_test.go +++ b/consensus-types/blocks/execution_test.go @@ -108,12 +108,6 @@ func TestWrapExecutionPayloadCapella(t *testing.T) { } payload, err := blocks.WrappedExecutionPayloadCapella(data, big.NewInt(10*1e9)) require.NoError(t, err) - wei, err := payload.ValueInWei() - require.NoError(t, err) - assert.Equal(t, 0, big.NewInt(10*1e9).Cmp(wei)) - gwei, err := payload.ValueInGwei() - require.NoError(t, err) - assert.Equal(t, uint64(10), gwei) assert.DeepEqual(t, data, payload.Proto()) } @@ -139,13 +133,6 @@ func TestWrapExecutionPayloadHeaderCapella(t *testing.T) { payload, err := blocks.WrappedExecutionPayloadHeaderCapella(data, big.NewInt(10*1e9)) require.NoError(t, err) - wei, err := payload.ValueInWei() - require.NoError(t, err) - assert.Equal(t, 0, big.NewInt(10*1e9).Cmp(wei)) - gwei, err := payload.ValueInGwei() - require.NoError(t, err) - assert.Equal(t, uint64(10), gwei) - assert.DeepEqual(t, data, payload.Proto()) txRoot, err := payload.TransactionsRoot() @@ -238,12 +225,6 @@ func TestWrapExecutionPayloadDeneb(t *testing.T) { } payload, err := blocks.WrappedExecutionPayloadDeneb(data, big.NewInt(420*1e9)) require.NoError(t, err) - wei, err := payload.ValueInWei() - require.NoError(t, err) - assert.Equal(t, 0, big.NewInt(420*1e9).Cmp(wei)) - gwei, err := payload.ValueInGwei() - require.NoError(t, err) - assert.Equal(t, uint64(420), gwei) g, err := payload.BlobGasUsed() require.NoError(t, err) @@ -277,13 +258,6 @@ func TestWrapExecutionPayloadHeaderDeneb(t *testing.T) { payload, err := blocks.WrappedExecutionPayloadHeaderDeneb(data, big.NewInt(10*1e9)) require.NoError(t, err) - wei, err := payload.ValueInWei() - require.NoError(t, err) - assert.Equal(t, 0, big.NewInt(10*1e9).Cmp(wei)) - gwei, err := payload.ValueInGwei() - require.NoError(t, err) - assert.Equal(t, uint64(10), gwei) - g, err := payload.BlobGasUsed() require.NoError(t, err) require.DeepEqual(t, uint64(88), g) diff --git a/consensus-types/blocks/get_payload_test.go b/consensus-types/blocks/get_payload_test.go deleted file mode 100644 index b40b8f17cd87..000000000000 --- a/consensus-types/blocks/get_payload_test.go +++ /dev/null @@ -1,22 +0,0 @@ -package blocks - -import ( - "testing" - - pb "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" - "github.com/prysmaticlabs/prysm/v5/testing/require" - "google.golang.org/protobuf/proto" -) - -func TestPayloadGetter(t *testing.T) { - t.Run("pre-capella does not implement payload getter", func(t *testing.T) { - var bellatrix proto.Message = &pb.ExecutionPayload{} - _, ok := bellatrix.(payloadGetter) - require.Equal(t, false, ok) - }) - t.Run("capella implements payload getter", func(t *testing.T) { - var capella interface{} = &pb.ExecutionPayloadCapellaWithValue{} - _, ok := capella.(payloadGetter) - require.Equal(t, true, ok) - }) -} diff --git a/consensus-types/blocks/getters.go b/consensus-types/blocks/getters.go index aae831b685f4..4ba4b5176710 100644 --- a/consensus-types/blocks/getters.go +++ b/consensus-types/blocks/getters.go @@ -2,7 +2,6 @@ package blocks import ( "fmt" - "math/big" "github.com/pkg/errors" ssz "github.com/prysmaticlabs/fastssz" @@ -14,7 +13,6 @@ import ( eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" validatorpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1/validator-client" "github.com/prysmaticlabs/prysm/v5/runtime/version" - log "github.com/sirupsen/logrus" ) // BeaconBlockIsNil checks if any composite field of input signed beacon block is nil. @@ -327,44 +325,6 @@ func (b *SignedBeaconBlock) IsBlinded() bool { return b.version >= version.Bellatrix && b.block.body.executionPayload == nil } -// ValueInWei metadata on the payload value returned by the builder. -func (b *SignedBeaconBlock) ValueInWei() primitives.Wei { - exec, err := b.block.body.Execution() - if err != nil { - if !errors.Is(err, consensus_types.ErrUnsupportedField) { - log.WithError(err).Warn("failed to retrieve execution payload") - } - return big.NewInt(0) - } - val, err := exec.ValueInWei() - if err != nil { - if !errors.Is(err, consensus_types.ErrUnsupportedField) { - log.WithError(err).Warn("failed to retrieve execution payload") - } - return big.NewInt(0) - } - return val -} - -// ValueInGwei metadata on the payload value returned by the builder. -func (b *SignedBeaconBlock) ValueInGwei() uint64 { - exec, err := b.block.body.Execution() - if err != nil { - if !errors.Is(err, consensus_types.ErrUnsupportedField) { - log.WithError(err).Warn("failed to retrieve execution payload") - } - return 0 - } - val, err := exec.ValueInGwei() - if err != nil { - if !errors.Is(err, consensus_types.ErrUnsupportedField) { - log.WithError(err).Warn("failed to retrieve execution payload") - } - return 0 - } - return val -} - // Header converts the underlying protobuf object from blinded block to header format. func (b *SignedBeaconBlock) Header() (*eth.SignedBeaconBlockHeader, error) { if b.IsNil() { diff --git a/consensus-types/interfaces/beacon_block.go b/consensus-types/interfaces/beacon_block.go index 68ce6f661cc6..c08ed34e4edf 100644 --- a/consensus-types/interfaces/beacon_block.go +++ b/consensus-types/interfaces/beacon_block.go @@ -27,8 +27,6 @@ type ReadOnlySignedBeaconBlock interface { ssz.Unmarshaler Version() int IsBlinded() bool - ValueInWei() primitives.Wei - ValueInGwei() uint64 Header() (*ethpb.SignedBeaconBlockHeader, error) } @@ -128,8 +126,6 @@ type ExecutionData interface { TransactionsRoot() ([]byte, error) Withdrawals() ([]*enginev1.Withdrawal, error) WithdrawalsRoot() ([]byte, error) - ValueInWei() (primitives.Wei, error) - ValueInGwei() (uint64, error) } type ExecutionDataElectra interface { diff --git a/consensus-types/mock/block.go b/consensus-types/mock/block.go index e172fd6df832..36fd58671077 100644 --- a/consensus-types/mock/block.go +++ b/consensus-types/mock/block.go @@ -75,14 +75,6 @@ func (SignedBeaconBlock) Header() (*eth.SignedBeaconBlockHeader, error) { panic("implement me") } -func (SignedBeaconBlock) ValueInWei() primitives.Wei { - panic("implement me") -} - -func (SignedBeaconBlock) ValueInGwei() uint64 { - panic("implement me") -} - type BeaconBlock struct { Htr [field_params.RootLength]byte HtrErr error From 0fadc5f12e508ef300186cc39ded969ba419fb9f Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Tue, 21 May 2024 20:40:22 -0500 Subject: [PATCH 07/16] removing wei/gwei fields from the payload wrappers --- api/client/builder/bid.go | 4 +- api/client/builder/client.go | 2 +- beacon-chain/builder/testing/mock.go | 5 +- beacon-chain/core/blocks/payload_test.go | 5 +- beacon-chain/core/blocks/withdrawals_test.go | 8 +-- beacon-chain/db/kv/state_test.go | 7 +- beacon-chain/execution/engine_client.go | 6 +- beacon-chain/execution/engine_client_test.go | 26 +++---- .../validator/proposer_bellatrix_test.go | 28 ++++---- .../proposer_execution_payload_test.go | 9 ++- .../prysm/v1alpha1/validator/proposer_test.go | 8 +-- .../state-native/getters_payload_header.go | 7 +- .../state/state-native/hasher_test.go | 3 +- .../setters_payload_header_test.go | 12 ++-- consensus-types/blocks/execution.go | 67 +++++++------------ consensus-types/blocks/execution_test.go | 25 ++++--- consensus-types/blocks/factory.go | 7 +- consensus-types/blocks/factory_test.go | 3 +- consensus-types/blocks/get_payload.go | 2 +- consensus-types/blocks/getters_test.go | 13 ++-- consensus-types/blocks/proto.go | 14 ++-- consensus-types/blocks/proto_test.go | 9 ++- runtime/interop/premine-state.go | 9 ++- testing/middleware/builder/builder.go | 4 +- .../capella/operations/execution_payload.go | 3 +- .../shared/capella/operations/withdrawals.go | 3 +- .../deneb/operations/execution_payload.go | 3 +- .../shared/deneb/operations/withdrawals.go | 3 +- .../electra/operations/execution_payload.go | 3 +- .../shared/electra/operations/withdrawals.go | 3 +- 30 files changed, 130 insertions(+), 171 deletions(-) diff --git a/api/client/builder/bid.go b/api/client/builder/bid.go index ee9861e5a64b..d2f7c4e77326 100644 --- a/api/client/builder/bid.go +++ b/api/client/builder/bid.go @@ -172,7 +172,7 @@ func WrappedBuilderBidCapella(p *ethpb.BuilderBidCapella) (Bid, error) { // Header returns the execution data interface. func (b builderBidCapella) Header() (interfaces.ExecutionData, error) { // We have to convert big endian to little endian because the value is coming from the execution layer. - return blocks.WrappedExecutionPayloadHeaderCapella(b.p.Header, primitives.LittleEndianBytesToWei(b.p.Value)) + return blocks.WrappedExecutionPayloadHeaderCapella(b.p.Header) } // BlobKzgCommitments -- @@ -266,7 +266,7 @@ func (b builderBidDeneb) HashTreeRootWith(hh *ssz.Hasher) error { // Header -- func (b builderBidDeneb) Header() (interfaces.ExecutionData, error) { // We have to convert big endian to little endian because the value is coming from the execution layer. - return blocks.WrappedExecutionPayloadHeaderDeneb(b.p.Header, primitives.LittleEndianBytesToWei(b.p.Value)) + return blocks.WrappedExecutionPayloadHeaderDeneb(b.p.Header) } // BlobKzgCommitments -- diff --git a/api/client/builder/client.go b/api/client/builder/client.go index ba7c33328f36..97acd7237c58 100644 --- a/api/client/builder/client.go +++ b/api/client/builder/client.go @@ -330,7 +330,7 @@ func (c *Client) SubmitBlindedBlock(ctx context.Context, sb interfaces.ReadOnlyS if err != nil { return nil, nil, err } - ed, err := blocks.NewWrappedExecutionData(pb, nil) + ed, err := blocks.NewWrappedExecutionData(pb) if err != nil { return nil, nil, err } diff --git a/beacon-chain/builder/testing/mock.go b/beacon-chain/builder/testing/mock.go index 55b1aa05efb3..e1c1d913aff8 100644 --- a/beacon-chain/builder/testing/mock.go +++ b/beacon-chain/builder/testing/mock.go @@ -2,7 +2,6 @@ package testing import ( "context" - "math/big" "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/v5/api/client/builder" @@ -55,13 +54,13 @@ func (s *MockBuilderService) SubmitBlindedBlock(_ context.Context, b interfaces. } return w, nil, s.ErrSubmitBlindedBlock case version.Capella: - w, err := blocks.WrappedExecutionPayloadCapella(s.PayloadCapella, big.NewInt(0)) + w, err := blocks.WrappedExecutionPayloadCapella(s.PayloadCapella) if err != nil { return nil, nil, errors.Wrap(err, "could not wrap capella payload") } return w, nil, s.ErrSubmitBlindedBlock case version.Deneb: - w, err := blocks.WrappedExecutionPayloadDeneb(s.PayloadDeneb, big.NewInt(0)) + w, err := blocks.WrappedExecutionPayloadDeneb(s.PayloadDeneb) if err != nil { return nil, nil, errors.Wrap(err, "could not wrap deneb payload") } diff --git a/beacon-chain/core/blocks/payload_test.go b/beacon-chain/core/blocks/payload_test.go index a447e7cbc878..0da3e9b523ff 100644 --- a/beacon-chain/core/blocks/payload_test.go +++ b/beacon-chain/core/blocks/payload_test.go @@ -1,7 +1,6 @@ package blocks_test import ( - "math/big" "testing" "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/blocks" @@ -610,7 +609,7 @@ func Test_ProcessPayloadCapella(t *testing.T) { random, err := helpers.RandaoMix(st, time.CurrentEpoch(st)) require.NoError(t, err) payload.PrevRandao = random - wrapped, err := consensusblocks.WrappedExecutionPayloadCapella(payload, big.NewInt(0)) + wrapped, err := consensusblocks.WrappedExecutionPayloadCapella(payload) require.NoError(t, err) _, err = blocks.ProcessPayload(st, wrapped) require.NoError(t, err) @@ -874,7 +873,7 @@ func emptyPayloadHeaderCapella() (interfaces.ExecutionData, error) { BlockHash: make([]byte, fieldparams.RootLength), TransactionsRoot: make([]byte, fieldparams.RootLength), WithdrawalsRoot: make([]byte, fieldparams.RootLength), - }, big.NewInt(0)) + }) } func emptyPayload() *enginev1.ExecutionPayload { diff --git a/beacon-chain/core/blocks/withdrawals_test.go b/beacon-chain/core/blocks/withdrawals_test.go index 35d917614b2d..6b0f6caf660b 100644 --- a/beacon-chain/core/blocks/withdrawals_test.go +++ b/beacon-chain/core/blocks/withdrawals_test.go @@ -1,7 +1,6 @@ package blocks_test import ( - "math/big" "math/rand" "testing" @@ -643,10 +642,7 @@ func TestProcessBlindWithdrawals(t *testing.T) { require.NoError(t, err) wdRoot, err := ssz.WithdrawalSliceRoot(test.Args.Withdrawals, fieldparams.MaxWithdrawalsPerPayload) require.NoError(t, err) - p, err := consensusblocks.WrappedExecutionPayloadHeaderCapella( - &enginev1.ExecutionPayloadHeaderCapella{WithdrawalsRoot: wdRoot[:]}, - big.NewInt(0), - ) + p, err := consensusblocks.WrappedExecutionPayloadHeaderCapella(&enginev1.ExecutionPayloadHeaderCapella{WithdrawalsRoot: wdRoot[:]}) require.NoError(t, err) post, err := blocks.ProcessWithdrawals(st, p) if test.Control.ExpectedError { @@ -1064,7 +1060,7 @@ func TestProcessWithdrawals(t *testing.T) { } st, err := prepareValidators(spb, test.Args) require.NoError(t, err) - p, err := consensusblocks.WrappedExecutionPayloadCapella(&enginev1.ExecutionPayloadCapella{Withdrawals: test.Args.Withdrawals}, big.NewInt(0)) + p, err := consensusblocks.WrappedExecutionPayloadCapella(&enginev1.ExecutionPayloadCapella{Withdrawals: test.Args.Withdrawals}) require.NoError(t, err) post, err := blocks.ProcessWithdrawals(st, p) if test.Control.ExpectedError { diff --git a/beacon-chain/db/kv/state_test.go b/beacon-chain/db/kv/state_test.go index 6ca53a5e23e3..938b95514599 100644 --- a/beacon-chain/db/kv/state_test.go +++ b/beacon-chain/db/kv/state_test.go @@ -3,7 +3,6 @@ package kv import ( "context" "encoding/binary" - "math/big" "math/rand" "strconv" "testing" @@ -100,7 +99,7 @@ func TestState_CanSaveRetrieve(t *testing.T) { BlockHash: make([]byte, 32), TransactionsRoot: make([]byte, 32), WithdrawalsRoot: make([]byte, 32), - }, big.NewInt(0)) + }) require.NoError(t, err) require.NoError(t, st.SetLatestExecutionPayloadHeader(p)) return st @@ -125,7 +124,7 @@ func TestState_CanSaveRetrieve(t *testing.T) { BlockHash: make([]byte, 32), TransactionsRoot: make([]byte, 32), WithdrawalsRoot: make([]byte, 32), - }, big.NewInt(0)) + }) require.NoError(t, err) require.NoError(t, st.SetLatestExecutionPayloadHeader(p)) return st @@ -152,7 +151,7 @@ func TestState_CanSaveRetrieve(t *testing.T) { WithdrawalsRoot: make([]byte, 32), DepositReceiptsRoot: make([]byte, 32), WithdrawalRequestsRoot: make([]byte, 32), - }, big.NewInt(0)) + }) require.NoError(t, err) require.NoError(t, st.SetLatestExecutionPayloadHeader(p)) return st diff --git a/beacon-chain/execution/engine_client.go b/beacon-chain/execution/engine_client.go index 312eabfab8d2..1566f523fd36 100644 --- a/beacon-chain/execution/engine_client.go +++ b/beacon-chain/execution/engine_client.go @@ -562,7 +562,7 @@ func fullPayloadFromPayloadBody( BlockHash: header.BlockHash(), Transactions: pb.RecastHexutilByteSlice(body.Transactions), Withdrawals: body.Withdrawals, - }, big.NewInt(0)) // We can't get the block value and don't care about the block value for this instance + }) // We can't get the block value and don't care about the block value for this instance case version.Deneb: ebg, err := header.ExcessBlobGas() if err != nil { @@ -591,7 +591,7 @@ func fullPayloadFromPayloadBody( Withdrawals: body.Withdrawals, ExcessBlobGas: ebg, BlobGasUsed: bgu, - }, big.NewInt(0)) // We can't get the block value and don't care about the block value for this instance + }) // We can't get the block value and don't care about the block value for this instance case version.Electra: ebg, err := header.ExcessBlobGas() if err != nil { @@ -630,7 +630,7 @@ func fullPayloadFromPayloadBody( BlobGasUsed: bgu, DepositReceipts: dr, WithdrawalRequests: wr, - }, big.NewInt(0)) // We can't get the block value and don't care about the block value for this instance + }) // We can't get the block value and don't care about the block value for this instance default: return nil, fmt.Errorf("unknown execution block version for payload %d", bVersion) } diff --git a/beacon-chain/execution/engine_client_test.go b/beacon-chain/execution/engine_client_test.go index 4336c6277025..c4255c9f62a8 100644 --- a/beacon-chain/execution/engine_client_test.go +++ b/beacon-chain/execution/engine_client_test.go @@ -132,7 +132,7 @@ func TestClient_IPC(t *testing.T) { require.Equal(t, true, ok) req, ok := fix["ExecutionPayloadCapella"].(*pb.ExecutionPayloadCapella) require.Equal(t, true, ok) - wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(req, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(req) require.NoError(t, err) latestValidHash, err := srv.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{}) require.NoError(t, err) @@ -537,7 +537,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV2Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{}) require.NoError(t, err) @@ -551,7 +551,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV3Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.NoError(t, err) @@ -565,7 +565,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV4Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.NoError(t, err) @@ -593,7 +593,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV2Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{}) require.ErrorIs(t, ErrAcceptedSyncingPayloadStatus, err) @@ -607,7 +607,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV3Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.ErrorIs(t, ErrAcceptedSyncingPayloadStatus, err) @@ -621,7 +621,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV4Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.ErrorIs(t, ErrAcceptedSyncingPayloadStatus, err) @@ -649,7 +649,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV2Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{}) require.ErrorIs(t, ErrInvalidBlockHashPayloadStatus, err) @@ -663,7 +663,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV3Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.ErrorIs(t, ErrInvalidBlockHashPayloadStatus, err) @@ -677,7 +677,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV4Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.ErrorIs(t, ErrInvalidBlockHashPayloadStatus, err) @@ -705,7 +705,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV2Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadCapella(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{}) require.ErrorIs(t, ErrInvalidPayloadStatus, err) @@ -719,7 +719,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV3Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadDeneb(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.ErrorIs(t, ErrInvalidPayloadStatus, err) @@ -733,7 +733,7 @@ func TestClient_HTTP(t *testing.T) { client := newPayloadV4Setup(t, want, execPayload) // We call the RPC method via HTTP and expect a proper result. - wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload, big.NewInt(0)) + wrappedPayload, err := blocks.WrappedExecutionPayloadElectra(execPayload) require.NoError(t, err) resp, err := client.NewPayload(ctx, wrappedPayload, []common.Hash{}, &common.Hash{'a'}) require.ErrorIs(t, ErrInvalidPayloadStatus, err) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go index 2e1616ee7234..3647973d37d2 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -47,7 +47,7 @@ func TestServer_setExecutionData(t *testing.T) { beaconDB := dbTest.SetupDB(t) capellaTransitionState, _ := util.DeterministicGenesisStateCapella(t, 1) - wrappedHeaderCapella, err := blocks.WrappedExecutionPayloadHeaderCapella(&v1.ExecutionPayloadHeaderCapella{BlockNumber: 1}, big.NewInt(0)) + wrappedHeaderCapella, err := blocks.WrappedExecutionPayloadHeaderCapella(&v1.ExecutionPayloadHeaderCapella{BlockNumber: 1}) require.NoError(t, err) require.NoError(t, capellaTransitionState.SetLatestExecutionPayloadHeader(wrappedHeaderCapella)) b2pbCapella := util.NewBeaconBlockCapella() @@ -60,7 +60,7 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, beaconDB.SaveFeeRecipientsByValidatorIDs(context.Background(), []primitives.ValidatorIndex{0}, []common.Address{{}})) denebTransitionState, _ := util.DeterministicGenesisStateDeneb(t, 1) - wrappedHeaderDeneb, err := blocks.WrappedExecutionPayloadHeaderDeneb(&v1.ExecutionPayloadHeaderDeneb{BlockNumber: 2}, big.NewInt(0)) + wrappedHeaderDeneb, err := blocks.WrappedExecutionPayloadHeaderDeneb(&v1.ExecutionPayloadHeaderDeneb{BlockNumber: 2}) require.NoError(t, err) require.NoError(t, denebTransitionState.SetLatestExecutionPayloadHeader(wrappedHeaderDeneb)) b2pbDeneb := util.NewBeaconBlockDeneb() @@ -79,7 +79,7 @@ func TestServer_setExecutionData(t *testing.T) { }} id := &v1.PayloadIDBytes{0x1} - ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: withdrawals}, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: withdrawals}) require.NoError(t, err) vs := &Server{ ExecutionEngineCaller: &powtesting.EngineClient{ @@ -398,7 +398,7 @@ func TestServer_setExecutionData(t *testing.T) { blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) elBid := primitives.Uint64ToWei(2 * 1e9) - ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 3}, elBid) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 3}) require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, Bid: elBid}} b := blk.Block() @@ -430,7 +430,7 @@ func TestServer_setExecutionData(t *testing.T) { blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) elBid := primitives.Uint64ToWei(1 * 1e9) - ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 3}, elBid) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 3}) require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, Bid: elBid}} @@ -463,7 +463,7 @@ func TestServer_setExecutionData(t *testing.T) { HasConfigured: true, Cfg: &builderTest.Config{BeaconDB: beaconDB}, } - ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 4}, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadCapella{BlockNumber: 4}) require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}} b := blk.Block() @@ -496,7 +496,7 @@ func TestServer_setExecutionData(t *testing.T) { Proofs: [][]byte{{4, 5, 6}}, Blobs: [][]byte{{7, 8, 9}}, } - ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadDeneb{BlockNumber: 4}, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadDeneb{BlockNumber: 4}) require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{ PayloadIDBytes: id, @@ -575,7 +575,7 @@ func TestServer_setExecutionData(t *testing.T) { vs.TimeFetcher = chain vs.HeadFetcher = chain - ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadDeneb{BlockNumber: 4, Withdrawals: withdrawals}, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(&v1.ExecutionPayloadDeneb{BlockNumber: 4, Withdrawals: withdrawals}) require.NoError(t, err) vs.ExecutionEngineCaller = &powtesting.EngineClient{ PayloadIDBytes: id, @@ -834,7 +834,7 @@ func TestServer_getPayloadHeader(t *testing.T) { require.DeepEqual(t, want, h) } if tc.returnedHeaderCapella != nil { - want, err := blocks.WrappedExecutionPayloadHeaderCapella(tc.returnedHeaderCapella, big.NewInt(197121)) // value is a mock + want, err := blocks.WrappedExecutionPayloadHeaderCapella(tc.returnedHeaderCapella) // value is a mock require.NoError(t, err) require.DeepEqual(t, want, h) } @@ -891,7 +891,7 @@ func Test_matchingWithdrawalsRoot(t *testing.T) { }) t.Run("could not get builder withdrawals root", func(t *testing.T) { local := &v1.ExecutionPayloadCapella{} - p, err := blocks.WrappedExecutionPayloadCapella(local, big.NewInt(0)) + p, err := blocks.WrappedExecutionPayloadCapella(local) require.NoError(t, err) header := &v1.ExecutionPayloadHeader{} h, err := blocks.WrappedExecutionPayloadHeader(header) @@ -901,10 +901,10 @@ func Test_matchingWithdrawalsRoot(t *testing.T) { }) t.Run("withdrawals mismatch", func(t *testing.T) { local := &v1.ExecutionPayloadCapella{} - p, err := blocks.WrappedExecutionPayloadCapella(local, big.NewInt(0)) + p, err := blocks.WrappedExecutionPayloadCapella(local) require.NoError(t, err) header := &v1.ExecutionPayloadHeaderCapella{} - h, err := blocks.WrappedExecutionPayloadHeaderCapella(header, big.NewInt(0)) + h, err := blocks.WrappedExecutionPayloadHeaderCapella(header) require.NoError(t, err) matched, err := matchingWithdrawalsRoot(p, h) require.NoError(t, err) @@ -918,13 +918,13 @@ func Test_matchingWithdrawalsRoot(t *testing.T) { Amount: 3, }} local := &v1.ExecutionPayloadCapella{Withdrawals: wds} - p, err := blocks.WrappedExecutionPayloadCapella(local, big.NewInt(0)) + p, err := blocks.WrappedExecutionPayloadCapella(local) require.NoError(t, err) header := &v1.ExecutionPayloadHeaderCapella{} wr, err := ssz.WithdrawalSliceRoot(wds, fieldparams.MaxWithdrawalsPerPayload) require.NoError(t, err) header.WithdrawalsRoot = wr[:] - h, err := blocks.WrappedExecutionPayloadHeaderCapella(header, big.NewInt(0)) + h, err := blocks.WrappedExecutionPayloadHeaderCapella(header) require.NoError(t, err) matched, err := matchingWithdrawalsRoot(p, h) require.NoError(t, err) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go index 6d3d710085be..ac52476d166e 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go @@ -3,7 +3,6 @@ package validator import ( "context" "errors" - "math/big" "testing" "github.com/ethereum/go-ethereum/common" @@ -61,7 +60,7 @@ func TestServer_getExecutionPayload(t *testing.T) { })) capellaTransitionState, _ := util.DeterministicGenesisStateCapella(t, 1) - wrappedHeaderCapella, err := blocks.WrappedExecutionPayloadHeaderCapella(&pb.ExecutionPayloadHeaderCapella{BlockNumber: 1}, big.NewInt(0)) + wrappedHeaderCapella, err := blocks.WrappedExecutionPayloadHeaderCapella(&pb.ExecutionPayloadHeaderCapella{BlockNumber: 1}) require.NoError(t, err) require.NoError(t, capellaTransitionState.SetLatestExecutionPayloadHeader(wrappedHeaderCapella)) b2pbCapella := util.NewBeaconBlockCapella() @@ -146,7 +145,7 @@ func TestServer_getExecutionPayload(t *testing.T) { cfg.TerminalBlockHashActivationEpoch = tt.activationEpoch params.OverrideBeaconConfig(cfg) - ed, err := blocks.NewWrappedExecutionData(&pb.ExecutionPayload{}, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(&pb.ExecutionPayload{}) require.NoError(t, err) vs := &Server{ ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: tt.payloadID, ErrForkchoiceUpdated: tt.forkchoiceErr, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed, OverrideBuilder: tt.override}}, @@ -195,7 +194,7 @@ func TestServer_getExecutionPayloadContextTimeout(t *testing.T) { cfg.TerminalBlockHashActivationEpoch = 1 params.OverrideBeaconConfig(cfg) - ed, err := blocks.NewWrappedExecutionData(&pb.ExecutionPayload{}, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(&pb.ExecutionPayload{}) require.NoError(t, err) vs := &Server{ ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: &pb.PayloadIDBytes{}, ErrGetPayload: context.DeadlineExceeded, GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: ed}}, @@ -244,7 +243,7 @@ func TestServer_getExecutionPayload_UnexpectedFeeRecipient(t *testing.T) { payloadID := &pb.PayloadIDBytes{0x1} payload := emptyPayload() payload.FeeRecipient = feeRecipient[:] - ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(payload) require.NoError(t, err) vs := &Server{ ExecutionEngineCaller: &powtesting.EngineClient{ diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go index 6e17499fe27a..108f54eb812f 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go @@ -277,7 +277,7 @@ func TestServer_GetBeaconBlock_Bellatrix(t *testing.T) { proposerServer := getProposerServer(db, beaconState, parentRoot[:]) proposerServer.Eth1BlockFetcher = c - ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(payload) require.NoError(t, err) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ PayloadIDBytes: &enginev1.PayloadIDBytes{1}, @@ -402,7 +402,7 @@ func TestServer_GetBeaconBlock_Capella(t *testing.T) { } proposerServer := getProposerServer(db, beaconState, parentRoot[:]) - ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(payload) require.NoError(t, err) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ PayloadIDBytes: &enginev1.PayloadIDBytes{1}, @@ -514,7 +514,7 @@ func TestServer_GetBeaconBlock_Deneb(t *testing.T) { BlobGasUsed: 4, ExcessBlobGas: 5, } - ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(payload) require.NoError(t, err) kc := make([][]byte, 0) @@ -659,7 +659,7 @@ func TestServer_GetBeaconBlock_Electra(t *testing.T) { } proposerServer := getProposerServer(db, beaconState, parentRoot[:]) - ed, err := blocks.NewWrappedExecutionData(payload, primitives.ZeroWei) + ed, err := blocks.NewWrappedExecutionData(payload) require.NoError(t, err) proposerServer.ExecutionEngineCaller = &mockExecution.EngineClient{ PayloadIDBytes: &enginev1.PayloadIDBytes{1}, diff --git a/beacon-chain/state/state-native/getters_payload_header.go b/beacon-chain/state/state-native/getters_payload_header.go index ba2a204d84a6..ff27b8a7ab32 100644 --- a/beacon-chain/state/state-native/getters_payload_header.go +++ b/beacon-chain/state/state-native/getters_payload_header.go @@ -2,7 +2,6 @@ package state_native import ( "fmt" - "math/big" "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" @@ -24,11 +23,11 @@ func (b *BeaconState) LatestExecutionPayloadHeader() (interfaces.ExecutionData, case version.Bellatrix: return blocks.WrappedExecutionPayloadHeader(b.latestExecutionPayloadHeaderVal()) case version.Capella: - return blocks.WrappedExecutionPayloadHeaderCapella(b.latestExecutionPayloadHeaderCapellaVal(), big.NewInt(0)) + return blocks.WrappedExecutionPayloadHeaderCapella(b.latestExecutionPayloadHeaderCapellaVal()) case version.Deneb: - return blocks.WrappedExecutionPayloadHeaderDeneb(b.latestExecutionPayloadHeaderDenebVal(), big.NewInt(0)) + return blocks.WrappedExecutionPayloadHeaderDeneb(b.latestExecutionPayloadHeaderDenebVal()) case version.Electra: - return blocks.WrappedExecutionPayloadHeaderElectra(b.latestExecutionPayloadHeaderElectraVal(), big.NewInt(0)) + return blocks.WrappedExecutionPayloadHeaderElectra(b.latestExecutionPayloadHeaderElectraVal()) default: return nil, fmt.Errorf("unsupported version (%s) for latest execution payload header", version.String(b.version)) } diff --git a/beacon-chain/state/state-native/hasher_test.go b/beacon-chain/state/state-native/hasher_test.go index 00e28bf76e8b..3f99222a880c 100644 --- a/beacon-chain/state/state-native/hasher_test.go +++ b/beacon-chain/state/state-native/hasher_test.go @@ -2,7 +2,6 @@ package state_native_test import ( "context" - "math/big" "testing" "github.com/prysmaticlabs/go-bitfield" @@ -254,7 +253,7 @@ func TestComputeFieldRootsWithHasher_Capella(t *testing.T) { require.NoError(t, beaconState.SetInactivityScores([]uint64{1, 2, 3})) require.NoError(t, beaconState.SetCurrentSyncCommittee(syncCommittee("current"))) require.NoError(t, beaconState.SetNextSyncCommittee(syncCommittee("next"))) - wrappedHeader, err := blocks.WrappedExecutionPayloadHeaderCapella(executionPayloadHeaderCapella(), big.NewInt(0)) + wrappedHeader, err := blocks.WrappedExecutionPayloadHeaderCapella(executionPayloadHeaderCapella()) require.NoError(t, err) require.NoError(t, beaconState.SetLatestExecutionPayloadHeader(wrappedHeader)) require.NoError(t, beaconState.SetNextWithdrawalIndex(123)) diff --git a/beacon-chain/state/state-native/setters_payload_header_test.go b/beacon-chain/state/state-native/setters_payload_header_test.go index d8d9fb17cbba..8e49ccdfb3ec 100644 --- a/beacon-chain/state/state-native/setters_payload_header_test.go +++ b/beacon-chain/state/state-native/setters_payload_header_test.go @@ -23,19 +23,19 @@ func TestSetLatestExecutionPayloadHeader(t *testing.T) { }(), func() interfaces.ExecutionData { e := util.NewBeaconBlockCapella().Block.Body.ExecutionPayload - ee, err := blocks.WrappedExecutionPayloadCapella(e, nil) + ee, err := blocks.WrappedExecutionPayloadCapella(e) require.NoError(t, err) return ee }(), func() interfaces.ExecutionData { e := util.NewBeaconBlockDeneb().Block.Body.ExecutionPayload - ee, err := blocks.WrappedExecutionPayloadDeneb(e, nil) + ee, err := blocks.WrappedExecutionPayloadDeneb(e) require.NoError(t, err) return ee }(), func() interfaces.ExecutionData { e := util.NewBeaconBlockElectra().Block.Body.ExecutionPayload - ee, err := blocks.WrappedExecutionPayloadElectra(e, nil) + ee, err := blocks.WrappedExecutionPayloadElectra(e) require.NoError(t, err) return ee }(), @@ -50,19 +50,19 @@ func TestSetLatestExecutionPayloadHeader(t *testing.T) { }(), func() interfaces.ExecutionData { e := util.NewBlindedBeaconBlockCapella().Block.Body.ExecutionPayloadHeader - ee, err := blocks.WrappedExecutionPayloadHeaderCapella(e, nil) + ee, err := blocks.WrappedExecutionPayloadHeaderCapella(e) require.NoError(t, err) return ee }(), func() interfaces.ExecutionData { e := util.NewBlindedBeaconBlockDeneb().Message.Body.ExecutionPayloadHeader - ee, err := blocks.WrappedExecutionPayloadHeaderDeneb(e, nil) + ee, err := blocks.WrappedExecutionPayloadHeaderDeneb(e) require.NoError(t, err) return ee }(), func() interfaces.ExecutionData { e := util.NewBlindedBeaconBlockElectra().Message.Body.ExecutionPayloadHeader - ee, err := blocks.WrappedExecutionPayloadHeaderElectra(e, nil) + ee, err := blocks.WrappedExecutionPayloadHeaderElectra(e) require.NoError(t, err) return ee }(), diff --git a/consensus-types/blocks/execution.go b/consensus-types/blocks/execution.go index 38897009120f..bb878a0bec50 100644 --- a/consensus-types/blocks/execution.go +++ b/consensus-types/blocks/execution.go @@ -3,13 +3,11 @@ package blocks import ( "bytes" "errors" - "math/big" fastssz "github.com/prysmaticlabs/fastssz" fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" consensus_types "github.com/prysmaticlabs/prysm/v5/consensus-types" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" - "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v5/encoding/ssz" enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" @@ -24,10 +22,7 @@ type executionPayload struct { } // NewWrappedExecutionData creates an appropriate execution payload wrapper based on the incoming type. -func NewWrappedExecutionData(v proto.Message, weiValue primitives.Wei) (interfaces.ExecutionData, error) { - if weiValue == nil { - weiValue = new(big.Int).SetInt64(0) - } +func NewWrappedExecutionData(v proto.Message) (interfaces.ExecutionData, error) { if v == nil { return nil, consensus_types.ErrNilObjectWrapped } @@ -35,17 +30,17 @@ func NewWrappedExecutionData(v proto.Message, weiValue primitives.Wei) (interfac case *enginev1.ExecutionPayload: return WrappedExecutionPayload(pbStruct) case *enginev1.ExecutionPayloadCapella: - return WrappedExecutionPayloadCapella(pbStruct, weiValue) + return WrappedExecutionPayloadCapella(pbStruct) case *enginev1.ExecutionPayloadCapellaWithValue: - return WrappedExecutionPayloadCapella(pbStruct.Payload, weiValue) + return WrappedExecutionPayloadCapella(pbStruct.Payload) case *enginev1.ExecutionPayloadDeneb: - return WrappedExecutionPayloadDeneb(pbStruct, weiValue) + return WrappedExecutionPayloadDeneb(pbStruct) case *enginev1.ExecutionPayloadDenebWithValueAndBlobsBundle: - return WrappedExecutionPayloadDeneb(pbStruct.Payload, weiValue) + return WrappedExecutionPayloadDeneb(pbStruct.Payload) case *enginev1.ExecutionPayloadElectra: - return WrappedExecutionPayloadElectra(pbStruct, weiValue) + return WrappedExecutionPayloadElectra(pbStruct) case *enginev1.ExecutionPayloadElectraWithValueAndBlobsBundle: - return WrappedExecutionPayloadElectra(pbStruct.Payload, weiValue) + return WrappedExecutionPayloadElectra(pbStruct.Payload) default: return nil, ErrUnsupportedVersion } @@ -392,16 +387,14 @@ func PayloadToHeader(payload interfaces.ExecutionData) (*enginev1.ExecutionPaylo // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. type executionPayloadCapella struct { - p *enginev1.ExecutionPayloadCapella - weiValue primitives.Wei - gweiValue uint64 + p *enginev1.ExecutionPayloadCapella } var _ interfaces.ExecutionData = &executionPayloadCapella{} // WrappedExecutionPayloadCapella is a constructor which wraps a protobuf execution payload into an interface. -func WrappedExecutionPayloadCapella(p *enginev1.ExecutionPayloadCapella, value primitives.Wei) (interfaces.ExecutionData, error) { - w := executionPayloadCapella{p: p, weiValue: value, gweiValue: uint64(primitives.WeiToGwei(value))} +func WrappedExecutionPayloadCapella(p *enginev1.ExecutionPayloadCapella) (interfaces.ExecutionData, error) { + w := executionPayloadCapella{p: p} if w.IsNil() { return nil, consensus_types.ErrNilObjectWrapped } @@ -552,16 +545,14 @@ func (e executionPayloadCapella) ExcessBlobGas() (uint64, error) { // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. type executionPayloadHeaderCapella struct { - p *enginev1.ExecutionPayloadHeaderCapella - weiValue primitives.Wei - gweiValue uint64 + p *enginev1.ExecutionPayloadHeaderCapella } var _ interfaces.ExecutionData = &executionPayloadHeaderCapella{} // WrappedExecutionPayloadHeaderCapella is a constructor which wraps a protobuf execution header into an interface. -func WrappedExecutionPayloadHeaderCapella(p *enginev1.ExecutionPayloadHeaderCapella, value primitives.Wei) (interfaces.ExecutionData, error) { - w := executionPayloadHeaderCapella{p: p, weiValue: value, gweiValue: uint64(primitives.WeiToGwei(value))} +func WrappedExecutionPayloadHeaderCapella(p *enginev1.ExecutionPayloadHeaderCapella) (interfaces.ExecutionData, error) { + w := executionPayloadHeaderCapella{p: p} if w.IsNil() { return nil, consensus_types.ErrNilObjectWrapped } @@ -934,16 +925,14 @@ func IsEmptyExecutionData(data interfaces.ExecutionData) (bool, error) { // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. type executionPayloadHeaderDeneb struct { - p *enginev1.ExecutionPayloadHeaderDeneb - weiValue primitives.Wei - gweiValue uint64 + p *enginev1.ExecutionPayloadHeaderDeneb } var _ interfaces.ExecutionData = &executionPayloadHeaderDeneb{} // WrappedExecutionPayloadHeaderDeneb is a constructor which wraps a protobuf execution header into an interface. -func WrappedExecutionPayloadHeaderDeneb(p *enginev1.ExecutionPayloadHeaderDeneb, value primitives.Wei) (interfaces.ExecutionData, error) { - w := executionPayloadHeaderDeneb{p: p, weiValue: value, gweiValue: uint64(primitives.WeiToGwei(value))} +func WrappedExecutionPayloadHeaderDeneb(p *enginev1.ExecutionPayloadHeaderDeneb) (interfaces.ExecutionData, error) { + w := executionPayloadHeaderDeneb{p: p} if w.IsNil() { return nil, consensus_types.ErrNilObjectWrapped } @@ -1094,16 +1083,14 @@ func (e executionPayloadHeaderDeneb) IsBlinded() bool { // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. type executionPayloadDeneb struct { - p *enginev1.ExecutionPayloadDeneb - weiValue primitives.Wei - gweiValue uint64 + p *enginev1.ExecutionPayloadDeneb } var _ interfaces.ExecutionData = &executionPayloadDeneb{} // WrappedExecutionPayloadDeneb is a constructor which wraps a protobuf execution payload into an interface. -func WrappedExecutionPayloadDeneb(p *enginev1.ExecutionPayloadDeneb, value primitives.Wei) (interfaces.ExecutionData, error) { - w := executionPayloadDeneb{p: p, weiValue: value, gweiValue: uint64(primitives.WeiToGwei(value))} +func WrappedExecutionPayloadDeneb(p *enginev1.ExecutionPayloadDeneb) (interfaces.ExecutionData, error) { + w := executionPayloadDeneb{p: p} if w.IsNil() { return nil, consensus_types.ErrNilObjectWrapped } @@ -1252,17 +1239,15 @@ func (e executionPayloadDeneb) IsBlinded() bool { // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. type executionPayloadHeaderElectra struct { - p *enginev1.ExecutionPayloadHeaderElectra - weiValue primitives.Wei - gweiValue uint64 + p *enginev1.ExecutionPayloadHeaderElectra } var _ interfaces.ExecutionData = &executionPayloadElectra{} var _ interfaces.ExecutionDataElectra = &executionPayloadElectra{} // WrappedExecutionPayloadHeaderElectra is a constructor which wraps a protobuf execution header into an interface. -func WrappedExecutionPayloadHeaderElectra(p *enginev1.ExecutionPayloadHeaderElectra, value primitives.Wei) (interfaces.ExecutionData, error) { - w := executionPayloadHeaderElectra{p: p, weiValue: value, gweiValue: uint64(primitives.WeiToGwei(value))} +func WrappedExecutionPayloadHeaderElectra(p *enginev1.ExecutionPayloadHeaderElectra) (interfaces.ExecutionData, error) { + w := executionPayloadHeaderElectra{p: p} if w.IsNil() { return nil, consensus_types.ErrNilObjectWrapped } @@ -1423,14 +1408,12 @@ func (e executionPayloadHeaderElectra) IsBlinded() bool { // This wrapper allows us to conform to a common interface so that beacon // blocks for future forks can also be applied across Prysm without issues. type executionPayloadElectra struct { - p *enginev1.ExecutionPayloadElectra - weiValue primitives.Wei - gweiValue uint64 + p *enginev1.ExecutionPayloadElectra } // WrappedExecutionPayloadElectra is a constructor which wraps a protobuf execution payload into an interface. -func WrappedExecutionPayloadElectra(p *enginev1.ExecutionPayloadElectra, value primitives.Wei) (interfaces.ExecutionData, error) { - w := executionPayloadElectra{p: p, weiValue: value, gweiValue: uint64(primitives.WeiToGwei(value))} +func WrappedExecutionPayloadElectra(p *enginev1.ExecutionPayloadElectra) (interfaces.ExecutionData, error) { + w := executionPayloadElectra{p: p} if w.IsNil() { return nil, consensus_types.ErrNilObjectWrapped } diff --git a/consensus-types/blocks/execution_test.go b/consensus-types/blocks/execution_test.go index 8424664069f8..d258ce2eae37 100644 --- a/consensus-types/blocks/execution_test.go +++ b/consensus-types/blocks/execution_test.go @@ -1,7 +1,6 @@ package blocks_test import ( - "math/big" "testing" fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" @@ -106,7 +105,7 @@ func TestWrapExecutionPayloadCapella(t *testing.T) { Amount: 77, }}, } - payload, err := blocks.WrappedExecutionPayloadCapella(data, big.NewInt(10*1e9)) + payload, err := blocks.WrappedExecutionPayloadCapella(data) require.NoError(t, err) assert.DeepEqual(t, data, payload.Proto()) @@ -130,7 +129,7 @@ func TestWrapExecutionPayloadHeaderCapella(t *testing.T) { TransactionsRoot: []byte("transactionsroot"), WithdrawalsRoot: []byte("withdrawalsroot"), } - payload, err := blocks.WrappedExecutionPayloadHeaderCapella(data, big.NewInt(10*1e9)) + payload, err := blocks.WrappedExecutionPayloadHeaderCapella(data) require.NoError(t, err) assert.DeepEqual(t, data, payload.Proto()) @@ -145,22 +144,22 @@ func TestWrapExecutionPayloadHeaderCapella(t *testing.T) { } func TestWrapExecutionPayloadCapella_IsNil(t *testing.T) { - _, err := blocks.WrappedExecutionPayloadCapella(nil, big.NewInt(0)) + _, err := blocks.WrappedExecutionPayloadCapella(nil) require.Equal(t, consensus_types.ErrNilObjectWrapped, err) data := &enginev1.ExecutionPayloadCapella{GasUsed: 54} - payload, err := blocks.WrappedExecutionPayloadCapella(data, big.NewInt(0)) + payload, err := blocks.WrappedExecutionPayloadCapella(data) require.NoError(t, err) assert.Equal(t, false, payload.IsNil()) } func TestWrapExecutionPayloadHeaderCapella_IsNil(t *testing.T) { - _, err := blocks.WrappedExecutionPayloadHeaderCapella(nil, big.NewInt(0)) + _, err := blocks.WrappedExecutionPayloadHeaderCapella(nil) require.Equal(t, consensus_types.ErrNilObjectWrapped, err) data := &enginev1.ExecutionPayloadHeaderCapella{GasUsed: 54} - payload, err := blocks.WrappedExecutionPayloadHeaderCapella(data, big.NewInt(0)) + payload, err := blocks.WrappedExecutionPayloadHeaderCapella(data) require.NoError(t, err) assert.Equal(t, false, payload.IsNil()) @@ -223,7 +222,7 @@ func TestWrapExecutionPayloadDeneb(t *testing.T) { BlobGasUsed: 88, ExcessBlobGas: 99, } - payload, err := blocks.WrappedExecutionPayloadDeneb(data, big.NewInt(420*1e9)) + payload, err := blocks.WrappedExecutionPayloadDeneb(data) require.NoError(t, err) g, err := payload.BlobGasUsed() @@ -255,7 +254,7 @@ func TestWrapExecutionPayloadHeaderDeneb(t *testing.T) { BlobGasUsed: 88, ExcessBlobGas: 99, } - payload, err := blocks.WrappedExecutionPayloadHeaderDeneb(data, big.NewInt(10*1e9)) + payload, err := blocks.WrappedExecutionPayloadHeaderDeneb(data) require.NoError(t, err) g, err := payload.BlobGasUsed() @@ -358,7 +357,7 @@ func createWrappedPayloadCapella(t testing.TB) interfaces.ExecutionData { BlockHash: make([]byte, fieldparams.RootLength), Transactions: make([][]byte, 0), Withdrawals: make([]*enginev1.Withdrawal, 0), - }, big.NewInt(0)) + }) require.NoError(t, err) return payload } @@ -380,7 +379,7 @@ func createWrappedPayloadHeaderCapella(t testing.TB) interfaces.ExecutionData { BlockHash: make([]byte, fieldparams.RootLength), TransactionsRoot: make([]byte, fieldparams.RootLength), WithdrawalsRoot: make([]byte, fieldparams.RootLength), - }, big.NewInt(0)) + }) require.NoError(t, err) return payload } @@ -404,7 +403,7 @@ func createWrappedPayloadDeneb(t testing.TB) interfaces.ExecutionData { Withdrawals: make([]*enginev1.Withdrawal, 0), BlobGasUsed: 0, ExcessBlobGas: 0, - }, big.NewInt(0)) + }) require.NoError(t, err) return payload } @@ -428,7 +427,7 @@ func createWrappedPayloadHeaderDeneb(t testing.TB) interfaces.ExecutionData { WithdrawalsRoot: make([]byte, fieldparams.RootLength), BlobGasUsed: 0, ExcessBlobGas: 0, - }, big.NewInt(0)) + }) require.NoError(t, err) return payload } diff --git a/consensus-types/blocks/factory.go b/consensus-types/blocks/factory.go index b7a1c826b118..9e4db86fe6c5 100644 --- a/consensus-types/blocks/factory.go +++ b/consensus-types/blocks/factory.go @@ -2,7 +2,6 @@ package blocks import ( "fmt" - "math/big" "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" @@ -260,11 +259,11 @@ func BuildSignedBeaconBlockFromExecutionPayload(blk interfaces.ReadOnlySignedBea case *enginev1.ExecutionPayload: wrappedPayload, wrapErr = WrappedExecutionPayload(p) case *enginev1.ExecutionPayloadCapella: - wrappedPayload, wrapErr = WrappedExecutionPayloadCapella(p, big.NewInt(0)) + wrappedPayload, wrapErr = WrappedExecutionPayloadCapella(p) case *enginev1.ExecutionPayloadDeneb: - wrappedPayload, wrapErr = WrappedExecutionPayloadDeneb(p, big.NewInt(0)) + wrappedPayload, wrapErr = WrappedExecutionPayloadDeneb(p) case *enginev1.ExecutionPayloadElectra: - wrappedPayload, wrapErr = WrappedExecutionPayloadElectra(p, big.NewInt(0)) + wrappedPayload, wrapErr = WrappedExecutionPayloadElectra(p) default: return nil, fmt.Errorf("%T is not a type of execution payload", p) } diff --git a/consensus-types/blocks/factory_test.go b/consensus-types/blocks/factory_test.go index 80a4a4f4f187..cca68452581e 100644 --- a/consensus-types/blocks/factory_test.go +++ b/consensus-types/blocks/factory_test.go @@ -3,7 +3,6 @@ package blocks import ( "bytes" "errors" - "math/big" "testing" fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" @@ -517,7 +516,7 @@ func TestBuildSignedBeaconBlockFromExecutionPayload(t *testing.T) { ExcessBlobGas: 123, BlobGasUsed: 321, } - wrapped, err := WrappedExecutionPayloadDeneb(payload, big.NewInt(123)) + wrapped, err := WrappedExecutionPayloadDeneb(payload) require.NoError(t, err) header, err := PayloadToHeaderDeneb(wrapped) require.NoError(t, err) diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index 8a2a64091bbf..3618c30674fb 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -48,7 +48,7 @@ func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { if hasShouldOverride { r.OverrideBuilder = shouldOverride.GetShouldOverrideBuilder() } - ed, err := NewWrappedExecutionData(msg, wei) + ed, err := NewWrappedExecutionData(msg) if err != nil { return nil, err } diff --git a/consensus-types/blocks/getters_test.go b/consensus-types/blocks/getters_test.go index 318bd65203d0..826db3bc13e6 100644 --- a/consensus-types/blocks/getters_test.go +++ b/consensus-types/blocks/getters_test.go @@ -1,7 +1,6 @@ package blocks import ( - "math/big" "testing" ssz "github.com/prysmaticlabs/fastssz" @@ -215,9 +214,9 @@ func Test_BeaconBlock_Copy(t *testing.T) { payload := &pb.ExecutionPayloadDeneb{ExcessBlobGas: 123} header := &pb.ExecutionPayloadHeaderDeneb{ExcessBlobGas: 223} - payloadInterface, err := WrappedExecutionPayloadDeneb(payload, big.NewInt(123)) + payloadInterface, err := WrappedExecutionPayloadDeneb(payload) require.NoError(t, err) - headerInterface, err := WrappedExecutionPayloadHeaderDeneb(header, big.NewInt(123)) + headerInterface, err := WrappedExecutionPayloadHeaderDeneb(header) require.NoError(t, err) bb = &BeaconBlockBody{executionPayload: payloadInterface, executionPayloadHeader: headerInterface, randaoReveal: bytesutil.ToBytes96([]byte{246}), graffiti: bytesutil.ToBytes32([]byte("graffiti"))} b = &BeaconBlock{body: bb, slot: 123, proposerIndex: 456, parentRoot: bytesutil.ToBytes32([]byte("parentroot")), stateRoot: bytesutil.ToBytes32([]byte("stateroot"))} @@ -425,7 +424,7 @@ func Test_BeaconBlockBody_Execution(t *testing.T) { assert.DeepEqual(t, result, e) executionCapella := &pb.ExecutionPayloadCapella{BlockNumber: 1} - eCapella, err := WrappedExecutionPayloadCapella(executionCapella, big.NewInt(0)) + eCapella, err := WrappedExecutionPayloadCapella(executionCapella) require.NoError(t, err) bb = &SignedBeaconBlock{version: version.Capella, block: &BeaconBlock{body: &BeaconBlockBody{version: version.Capella}}} require.NoError(t, bb.SetExecution(eCapella)) @@ -434,7 +433,7 @@ func Test_BeaconBlockBody_Execution(t *testing.T) { assert.DeepEqual(t, result, eCapella) executionCapellaHeader := &pb.ExecutionPayloadHeaderCapella{BlockNumber: 1} - eCapellaHeader, err := WrappedExecutionPayloadHeaderCapella(executionCapellaHeader, big.NewInt(0)) + eCapellaHeader, err := WrappedExecutionPayloadHeaderCapella(executionCapellaHeader) require.NoError(t, err) bb = &SignedBeaconBlock{version: version.Capella, block: &BeaconBlock{version: version.Capella, body: &BeaconBlockBody{version: version.Capella}}} require.NoError(t, bb.SetExecution(eCapellaHeader)) @@ -443,7 +442,7 @@ func Test_BeaconBlockBody_Execution(t *testing.T) { assert.DeepEqual(t, result, eCapellaHeader) executionDeneb := &pb.ExecutionPayloadDeneb{BlockNumber: 1, ExcessBlobGas: 123} - eDeneb, err := WrappedExecutionPayloadDeneb(executionDeneb, big.NewInt(0)) + eDeneb, err := WrappedExecutionPayloadDeneb(executionDeneb) require.NoError(t, err) bb = &SignedBeaconBlock{version: version.Deneb, block: &BeaconBlock{body: &BeaconBlockBody{version: version.Deneb}}} require.NoError(t, bb.SetExecution(eDeneb)) @@ -455,7 +454,7 @@ func Test_BeaconBlockBody_Execution(t *testing.T) { require.DeepEqual(t, gas, uint64(123)) executionDenebHeader := &pb.ExecutionPayloadHeaderDeneb{BlockNumber: 1, ExcessBlobGas: 223} - eDenebHeader, err := WrappedExecutionPayloadHeaderDeneb(executionDenebHeader, big.NewInt(0)) + eDenebHeader, err := WrappedExecutionPayloadHeaderDeneb(executionDenebHeader) require.NoError(t, err) bb = &SignedBeaconBlock{version: version.Deneb, block: &BeaconBlock{version: version.Deneb, body: &BeaconBlockBody{version: version.Deneb}}} require.NoError(t, bb.SetExecution(eDenebHeader)) diff --git a/consensus-types/blocks/proto.go b/consensus-types/blocks/proto.go index 314fb0a5e67a..d00e42cdde6a 100644 --- a/consensus-types/blocks/proto.go +++ b/consensus-types/blocks/proto.go @@ -1,8 +1,6 @@ package blocks import ( - "math/big" - "github.com/pkg/errors" consensus_types "github.com/prysmaticlabs/prysm/v5/consensus-types" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" @@ -1031,7 +1029,7 @@ func initBlockBodyFromProtoCapella(pb *eth.BeaconBlockBodyCapella) (*BeaconBlock return nil, errNilBlockBody } - p, err := WrappedExecutionPayloadCapella(pb.ExecutionPayload, big.NewInt(0)) + p, err := WrappedExecutionPayloadCapella(pb.ExecutionPayload) // We allow the payload to be nil if err != nil && err != consensus_types.ErrNilObjectWrapped { return nil, err @@ -1058,7 +1056,7 @@ func initBlindedBlockBodyFromProtoCapella(pb *eth.BlindedBeaconBlockBodyCapella) return nil, errNilBlockBody } - ph, err := WrappedExecutionPayloadHeaderCapella(pb.ExecutionPayloadHeader, big.NewInt(0)) + ph, err := WrappedExecutionPayloadHeaderCapella(pb.ExecutionPayloadHeader) // We allow the payload to be nil if err != nil && err != consensus_types.ErrNilObjectWrapped { return nil, err @@ -1085,7 +1083,7 @@ func initBlockBodyFromProtoDeneb(pb *eth.BeaconBlockBodyDeneb) (*BeaconBlockBody return nil, errNilBlockBody } - p, err := WrappedExecutionPayloadDeneb(pb.ExecutionPayload, big.NewInt(0)) + p, err := WrappedExecutionPayloadDeneb(pb.ExecutionPayload) // We allow the payload to be nil if err != nil && err != consensus_types.ErrNilObjectWrapped { return nil, err @@ -1113,7 +1111,7 @@ func initBlindedBlockBodyFromProtoDeneb(pb *eth.BlindedBeaconBlockBodyDeneb) (*B return nil, errNilBlockBody } - ph, err := WrappedExecutionPayloadHeaderDeneb(pb.ExecutionPayloadHeader, big.NewInt(0)) + ph, err := WrappedExecutionPayloadHeaderDeneb(pb.ExecutionPayloadHeader) // We allow the payload to be nil if err != nil && err != consensus_types.ErrNilObjectWrapped { return nil, err @@ -1141,7 +1139,7 @@ func initBlockBodyFromProtoElectra(pb *eth.BeaconBlockBodyElectra) (*BeaconBlock return nil, errNilBlockBody } - p, err := WrappedExecutionPayloadElectra(pb.ExecutionPayload, big.NewInt(0)) + p, err := WrappedExecutionPayloadElectra(pb.ExecutionPayload) // We allow the payload to be nil if err != nil && err != consensus_types.ErrNilObjectWrapped { return nil, err @@ -1170,7 +1168,7 @@ func initBlindedBlockBodyFromProtoElectra(pb *eth.BlindedBeaconBlockBodyElectra) return nil, errNilBlockBody } - ph, err := WrappedExecutionPayloadHeaderElectra(pb.ExecutionPayloadHeader, big.NewInt(0)) + ph, err := WrappedExecutionPayloadHeaderElectra(pb.ExecutionPayloadHeader) // We allow the payload to be nil if err != nil && err != consensus_types.ErrNilObjectWrapped { return nil, err diff --git a/consensus-types/blocks/proto_test.go b/consensus-types/blocks/proto_test.go index a906687e904b..880edb63db99 100644 --- a/consensus-types/blocks/proto_test.go +++ b/consensus-types/blocks/proto_test.go @@ -1,7 +1,6 @@ package blocks import ( - "math/big" "testing" "github.com/prysmaticlabs/go-bitfield" @@ -1332,7 +1331,7 @@ func bodyBlindedBellatrix(t *testing.T) *BeaconBlockBody { func bodyCapella(t *testing.T) *BeaconBlockBody { f := getFields() - p, err := WrappedExecutionPayloadCapella(f.execPayloadCapella, big.NewInt(0)) + p, err := WrappedExecutionPayloadCapella(f.execPayloadCapella) require.NoError(t, err) return &BeaconBlockBody{ version: version.Capella, @@ -1356,7 +1355,7 @@ func bodyCapella(t *testing.T) *BeaconBlockBody { func bodyBlindedCapella(t *testing.T) *BeaconBlockBody { f := getFields() - ph, err := WrappedExecutionPayloadHeaderCapella(f.execPayloadHeaderCapella, big.NewInt(0)) + ph, err := WrappedExecutionPayloadHeaderCapella(f.execPayloadHeaderCapella) require.NoError(t, err) return &BeaconBlockBody{ version: version.Capella, @@ -1380,7 +1379,7 @@ func bodyBlindedCapella(t *testing.T) *BeaconBlockBody { func bodyDeneb(t *testing.T) *BeaconBlockBody { f := getFields() - p, err := WrappedExecutionPayloadDeneb(f.execPayloadDeneb, big.NewInt(0)) + p, err := WrappedExecutionPayloadDeneb(f.execPayloadDeneb) require.NoError(t, err) return &BeaconBlockBody{ version: version.Deneb, @@ -1405,7 +1404,7 @@ func bodyDeneb(t *testing.T) *BeaconBlockBody { func bodyBlindedDeneb(t *testing.T) *BeaconBlockBody { f := getFields() - ph, err := WrappedExecutionPayloadHeaderDeneb(f.execPayloadHeaderDeneb, big.NewInt(0)) + ph, err := WrappedExecutionPayloadHeaderDeneb(f.execPayloadHeaderDeneb) require.NoError(t, err) return &BeaconBlockBody{ version: version.Deneb, diff --git a/runtime/interop/premine-state.go b/runtime/interop/premine-state.go index 7a9640739101..3eccfaab789f 100644 --- a/runtime/interop/premine-state.go +++ b/runtime/interop/premine-state.go @@ -2,7 +2,6 @@ package interop import ( "context" - "math/big" "github.com/ethereum/go-ethereum/core/types" "github.com/pkg/errors" @@ -596,7 +595,7 @@ func (s *PremineGenesisConfig) setExecutionPayload(g state.BeaconState) error { Transactions: make([][]byte, 0), Withdrawals: make([]*enginev1.Withdrawal, 0), } - wep, err := blocks.WrappedExecutionPayloadCapella(payload, big.NewInt(0)) + wep, err := blocks.WrappedExecutionPayloadCapella(payload) if err != nil { return err } @@ -604,7 +603,7 @@ func (s *PremineGenesisConfig) setExecutionPayload(g state.BeaconState) error { if err != nil { return err } - ed, err = blocks.WrappedExecutionPayloadHeaderCapella(eph, big.NewInt(0)) + ed, err = blocks.WrappedExecutionPayloadHeaderCapella(eph) if err != nil { return err } @@ -628,7 +627,7 @@ func (s *PremineGenesisConfig) setExecutionPayload(g state.BeaconState) error { ExcessBlobGas: *gb.ExcessBlobGas(), BlobGasUsed: *gb.BlobGasUsed(), } - wep, err := blocks.WrappedExecutionPayloadDeneb(payload, big.NewInt(0)) + wep, err := blocks.WrappedExecutionPayloadDeneb(payload) if err != nil { return err } @@ -636,7 +635,7 @@ func (s *PremineGenesisConfig) setExecutionPayload(g state.BeaconState) error { if err != nil { return err } - ed, err = blocks.WrappedExecutionPayloadHeaderDeneb(eph, big.NewInt(0)) + ed, err = blocks.WrappedExecutionPayloadHeaderDeneb(eph) if err != nil { return err } diff --git a/testing/middleware/builder/builder.go b/testing/middleware/builder/builder.go index 1543dd47e905..c0d44760b634 100644 --- a/testing/middleware/builder/builder.go +++ b/testing/middleware/builder/builder.go @@ -427,7 +427,7 @@ func (p *Builder) handleHeaderRequestCapella(w http.ResponseWriter) { weiVal := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(b.Value)) // we set the payload value as twice its actual one so that it always chooses builder payloads vs local payloads weiVal = weiVal.Mul(weiVal, big.NewInt(2)) - wObj, err := blocks.WrappedExecutionPayloadCapella(b.Payload, weiVal) + wObj, err := blocks.WrappedExecutionPayloadCapella(b.Payload) if err != nil { p.cfg.logger.WithError(err).Error("Could not wrap execution payload") http.Error(w, err.Error(), http.StatusInternalServerError) @@ -508,7 +508,7 @@ func (p *Builder) handleHeaderRequestDeneb(w http.ResponseWriter) { weiVal := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(b.Value)) // we set the payload value as twice its actual one so that it always chooses builder payloads vs local payloads weiVal = weiVal.Mul(weiVal, big.NewInt(2)) - wObj, err := blocks.WrappedExecutionPayloadDeneb(b.Payload, weiVal) + wObj, err := blocks.WrappedExecutionPayloadDeneb(b.Payload) if err != nil { p.cfg.logger.WithError(err).Error("Could not wrap execution payload") http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/testing/spectest/shared/capella/operations/execution_payload.go b/testing/spectest/shared/capella/operations/execution_payload.go index 346b72c3ff63..453c265110fc 100644 --- a/testing/spectest/shared/capella/operations/execution_payload.go +++ b/testing/spectest/shared/capella/operations/execution_payload.go @@ -1,7 +1,6 @@ package operations import ( - "math/big" "os" "path" "strings" @@ -54,7 +53,7 @@ func RunExecutionPayloadTest(t *testing.T, config string) { require.NoError(t, err) } - payload, err := blocks2.WrappedExecutionPayloadCapella(block.ExecutionPayload, big.NewInt(0)) + payload, err := blocks2.WrappedExecutionPayloadCapella(block.ExecutionPayload) require.NoError(t, err) file, err := util.BazelFileBytes(testsFolderPath, folder.Name(), "execution.yaml") diff --git a/testing/spectest/shared/capella/operations/withdrawals.go b/testing/spectest/shared/capella/operations/withdrawals.go index 85159b94ae1b..edfb56a8ea61 100644 --- a/testing/spectest/shared/capella/operations/withdrawals.go +++ b/testing/spectest/shared/capella/operations/withdrawals.go @@ -2,7 +2,6 @@ package operations import ( "context" - "math/big" "path" "testing" @@ -44,7 +43,7 @@ func RunWithdrawalsTest(t *testing.T, config string) { if err != nil { return nil, err } - p, err := consensusblocks.WrappedExecutionPayloadCapella(&enginev1.ExecutionPayloadCapella{Withdrawals: withdrawals}, big.NewInt(0)) + p, err := consensusblocks.WrappedExecutionPayloadCapella(&enginev1.ExecutionPayloadCapella{Withdrawals: withdrawals}) require.NoError(t, err) return blocks.ProcessWithdrawals(s, p) }) diff --git a/testing/spectest/shared/deneb/operations/execution_payload.go b/testing/spectest/shared/deneb/operations/execution_payload.go index e708ef271fd4..d1069e4a600f 100644 --- a/testing/spectest/shared/deneb/operations/execution_payload.go +++ b/testing/spectest/shared/deneb/operations/execution_payload.go @@ -1,7 +1,6 @@ package operations import ( - "math/big" "os" "path" "strings" @@ -57,7 +56,7 @@ func RunExecutionPayloadTest(t *testing.T, config string) { require.NoError(t, err) } - payload, err := blocks2.WrappedExecutionPayloadDeneb(body.ExecutionPayload, big.NewInt(0)) + payload, err := blocks2.WrappedExecutionPayloadDeneb(body.ExecutionPayload) require.NoError(t, err) file, err := util.BazelFileBytes(testsFolderPath, folder.Name(), "execution.yaml") diff --git a/testing/spectest/shared/deneb/operations/withdrawals.go b/testing/spectest/shared/deneb/operations/withdrawals.go index d31f3da0aea8..ac8905bc768c 100644 --- a/testing/spectest/shared/deneb/operations/withdrawals.go +++ b/testing/spectest/shared/deneb/operations/withdrawals.go @@ -2,7 +2,6 @@ package operations import ( "context" - "math/big" "path" "testing" @@ -41,7 +40,7 @@ func RunWithdrawalsTest(t *testing.T, config string) { if err != nil { return nil, err } - p, err := consensusblocks.WrappedExecutionPayloadDeneb(&enginev1.ExecutionPayloadDeneb{Withdrawals: withdrawals}, big.NewInt(0)) + p, err := consensusblocks.WrappedExecutionPayloadDeneb(&enginev1.ExecutionPayloadDeneb{Withdrawals: withdrawals}) require.NoError(t, err) return blocks.ProcessWithdrawals(s, p) }) diff --git a/testing/spectest/shared/electra/operations/execution_payload.go b/testing/spectest/shared/electra/operations/execution_payload.go index 69111af82cd5..dc00fc5173eb 100644 --- a/testing/spectest/shared/electra/operations/execution_payload.go +++ b/testing/spectest/shared/electra/operations/execution_payload.go @@ -1,7 +1,6 @@ package operations import ( - "math/big" "os" "path" "strings" @@ -59,7 +58,7 @@ func RunExecutionPayloadTest(t *testing.T, config string) { require.NoError(t, err) } - payload, err := blocks2.WrappedExecutionPayloadElectra(body.ExecutionPayload, big.NewInt(0)) + payload, err := blocks2.WrappedExecutionPayloadElectra(body.ExecutionPayload) require.NoError(t, err) file, err := util.BazelFileBytes(testsFolderPath, folder.Name(), "execution.yaml") diff --git a/testing/spectest/shared/electra/operations/withdrawals.go b/testing/spectest/shared/electra/operations/withdrawals.go index 4c08dfa3d270..866a2de53882 100644 --- a/testing/spectest/shared/electra/operations/withdrawals.go +++ b/testing/spectest/shared/electra/operations/withdrawals.go @@ -2,7 +2,6 @@ package operations import ( "context" - "math/big" "path" "testing" @@ -41,7 +40,7 @@ func RunWithdrawalsTest(t *testing.T, config string) { if err != nil { return nil, err } - p, err := consensusblocks.WrappedExecutionPayloadElectra(&enginev1.ExecutionPayloadElectra{Withdrawals: withdrawals}, big.NewInt(0)) + p, err := consensusblocks.WrappedExecutionPayloadElectra(&enginev1.ExecutionPayloadElectra{Withdrawals: withdrawals}) require.NoError(t, err) return blocks.ProcessWithdrawals(s, p) }) From ae7e532065b0ddfead5887524c1c30e9814fd7c5 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Tue, 21 May 2024 21:08:17 -0500 Subject: [PATCH 08/16] commentary around the little-endian situation --- consensus-types/blocks/get_payload.go | 4 ++++ consensus-types/primitives/wei.go | 3 +++ 2 files changed, 7 insertions(+) diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index 3618c30674fb..81bfda64b0da 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -41,6 +41,10 @@ func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { bidValueGetter, hasBid := msg.(bidValueGetter) wei := primitives.ZeroWei if hasBid { + // The protobuf types that engine api responses unmarshal into store their values in little endian form. + // This is done for consistency with other uint256 values stored in protobufs for SSZ values. + // Long term we should move away from protobuf types for these values and just keep the bid as a big.Int as soon + // as we unmarshal it from the engine api response. wei = primitives.LittleEndianBytesToWei(bidValueGetter.GetValue()) } r.Bid = wei diff --git a/consensus-types/primitives/wei.go b/consensus-types/primitives/wei.go index 8d687d69b4a1..01aa288894c5 100644 --- a/consensus-types/primitives/wei.go +++ b/consensus-types/primitives/wei.go @@ -72,6 +72,9 @@ func Uint64ToWei(v uint64) Wei { } // LittleEndianBytesToWei returns a Wei value given a little-endian binary representation. +// The only places we use this representation are in protobuf types that hold either the +// local execution payload bid or the builder bid. Going forward we should avoid that representation +// so this function being used in new places should be considered a code smell. func LittleEndianBytesToWei(value []byte) Wei { if len(value) == 0 { return big.NewInt(0) From e4ecf63cb86f6c7191ed299d3746e6039316cca8 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 09:04:10 -0500 Subject: [PATCH 09/16] finish the job in BuildBlockParallel --- .../rpc/prysm/v1alpha1/validator/proposer.go | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index 6934de16a7da..6d11c77346af 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -98,25 +98,16 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) ( builderBoostFactor = primitives.Gwei(req.BuilderBoostFactor.Value) } - winningBid, bundle, err := vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost, builderBoostFactor) - if err != nil { - return nil, errors.Wrap(err, "could not build block in parallel") - } - - sr, err := vs.computeStateRoot(ctx, sBlk) - if err != nil { - return nil, status.Errorf(codes.Internal, "Could not compute state root: %v", err) - } - sBlk.SetStateRoot(sr) - + resp, err := vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost, builderBoostFactor) log.WithFields(logrus.Fields{ "slot": req.Slot, "sinceSlotStartTime": time.Since(t), "validator": sBlk.Block().ProposerIndex(), }).Info("Finished building block") - - // Blob cache is updated after BuildBlockParallel - return vs.constructGenericBeaconBlock(sBlk, bundle, winningBid) + if err != nil { + return nil, errors.Wrap(err, "could not build block in parallel") + } + return resp, nil } func (vs *Server) handleSuccesfulReorgAttempt(ctx context.Context, slot primitives.Slot, parentRoot, headRoot [32]byte) (state.BeaconState, error) { @@ -187,7 +178,7 @@ func (vs *Server) getParentState(ctx context.Context, slot primitives.Slot) (sta return head, parentRoot, err } -func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor primitives.Gwei) (primitives.Wei, *enginev1.BlobsBundle, error) { +func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor primitives.Gwei) (*ethpb.GenericBeaconBlock, error) { // Build consensus fields in background var wg sync.WaitGroup wg.Add(1) @@ -234,13 +225,12 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed vs.setBlsToExecData(sBlk, head) }() - blockEpoch := slots.ToEpoch(sBlk.Block().Slot()) winningBid := primitives.ZeroWei var bundle *enginev1.BlobsBundle - if blockEpoch >= params.BeaconConfig().BellatrixForkEpoch { + if slots.ToEpoch(sBlk.Block().Slot()) >= params.BeaconConfig().BellatrixForkEpoch { local, err := vs.getLocalPayload(ctx, sBlk.Block(), head) if err != nil { - return primitives.ZeroWei, nil, status.Errorf(codes.Internal, "Could not get local payload: %v", err) + return nil, status.Errorf(codes.Internal, "Could not get local payload: %v", err) } // There's no reason to try to get a builder bid if local override is true. @@ -258,13 +248,19 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed winningBid, bundle, err = setExecutionData(ctx, sBlk, local, builderBid, builderBoostFactor) if err != nil { - return primitives.ZeroWei, nil, status.Errorf(codes.Internal, "Could not set execution data: %v", err) + return nil, status.Errorf(codes.Internal, "Could not set execution data: %v", err) } } - wg.Wait() // Wait until block is built via consensus and execution fields. + wg.Wait() - return winningBid, bundle, nil + sr, err := vs.computeStateRoot(ctx, sBlk) + if err != nil { + return nil, status.Errorf(codes.Internal, "Could not compute state root: %v", err) + } + sBlk.SetStateRoot(sr) + + return vs.constructGenericBeaconBlock(sBlk, bundle, winningBid) } // ProposeBeaconBlock handles the proposal of beacon blocks. From e2b45397e9b7a74df3bb9414bd214d04fa993f53 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 13:02:24 -0500 Subject: [PATCH 10/16] light self-review cleanup --- beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go | 3 --- .../prysm/v1alpha1/validator/proposer_execution_payload.go | 4 ---- 2 files changed, 7 deletions(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index 6d11c77346af..dd73e282c9ca 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -240,9 +240,6 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed if err != nil { builderGetPayloadMissCount.Inc() log.WithError(err).Error("Could not get builder payload") - } else if builderBid == nil { - builderGetPayloadMissCount.Inc() - log.WithError(err).Error("Could not get builder payload") } } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go index 559b31ec422c..b665548ca7e8 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go @@ -80,8 +80,6 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe payloadIDCacheHit.Inc() res, err := vs.ExecutionEngineCaller.GetPayload(ctx, pid, slot) if err == nil { - //payload, bundle, overrideBuilder := res.ExecutionData, res.BlobsBundle, res.OverrideBuilder - //bundleCache.add(slot, bundle) warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) //return payload, overrideBuilder, nil return res, nil @@ -181,8 +179,6 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe return nil, err } - // TODO: remove bundleCache code - //bundleCache.add(slot, res.BlobsBundle) warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) log.WithField("value", res.Bid).Debug("received execution payload from local engine") return res, nil From 7ebf48ea1c7498c727e16b1cf774987608af0856 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 14:30:34 -0500 Subject: [PATCH 11/16] fix spectest mock --- testing/spectest/shared/common/forkchoice/service.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testing/spectest/shared/common/forkchoice/service.go b/testing/spectest/shared/common/forkchoice/service.go index c02814f94e7e..6395907b9da2 100644 --- a/testing/spectest/shared/common/forkchoice/service.go +++ b/testing/spectest/shared/common/forkchoice/service.go @@ -22,6 +22,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/beacon-chain/startup" "github.com/prysmaticlabs/prysm/v5/beacon-chain/state" "github.com/prysmaticlabs/prysm/v5/beacon-chain/state/stategen" + "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" payloadattribute "github.com/prysmaticlabs/prysm/v5/consensus-types/payload-attribute" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" @@ -92,8 +93,8 @@ type engineMock struct { payloadStatus error } -func (m *engineMock) GetPayload(context.Context, [8]byte, primitives.Slot) (interfaces.ExecutionData, *pb.BlobsBundle, bool, error) { - return nil, nil, false, nil +func (m *engineMock) GetPayload(context.Context, [8]byte, primitives.Slot) (*blocks.GetPayloadResponse, error) { + return nil, nil } func (m *engineMock) GetPayloadV2(context.Context, [8]byte) (*pb.ExecutionPayloadCapella, error) { return nil, nil From 76e3d1b4082e02712ed6f353a4f1c84fb1584284 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 14:38:22 -0500 Subject: [PATCH 12/16] restore engine timeout --- beacon-chain/execution/engine_client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beacon-chain/execution/engine_client.go b/beacon-chain/execution/engine_client.go index 1566f523fd36..cf8a791fc999 100644 --- a/beacon-chain/execution/engine_client.go +++ b/beacon-chain/execution/engine_client.go @@ -289,6 +289,9 @@ func (s *Service) GetPayload(ctx context.Context, payloadId [8]byte, slot primit defer func() { getPayloadLatency.Observe(float64(time.Since(start).Milliseconds())) }() + d := time.Now().Add(defaultEngineTimeout) + ctx, cancel := context.WithDeadline(ctx, d) + defer cancel() method, result := getPayloadMethodAndMessage(slot) err := s.rpcClient.CallContext(ctx, result, method, pb.PayloadIDBytes(payloadId)) From 5cf876339284b5a3f22589021f4c7afb838434cd Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 14:42:34 -0500 Subject: [PATCH 13/16] lint fixes --- testing/middleware/builder/builder.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/testing/middleware/builder/builder.go b/testing/middleware/builder/builder.go index c0d44760b634..592b96eefff7 100644 --- a/testing/middleware/builder/builder.go +++ b/testing/middleware/builder/builder.go @@ -423,10 +423,6 @@ func (p *Builder) handleHeaderRequestCapella(w http.ResponseWriter) { v := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(b.Value)) // we set the payload value as twice its actual one so that it always chooses builder payloads vs local payloads v = v.Mul(v, big.NewInt(2)) - // Is used as the helper modifies the big.Int - weiVal := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(b.Value)) - // we set the payload value as twice its actual one so that it always chooses builder payloads vs local payloads - weiVal = weiVal.Mul(weiVal, big.NewInt(2)) wObj, err := blocks.WrappedExecutionPayloadCapella(b.Payload) if err != nil { p.cfg.logger.WithError(err).Error("Could not wrap execution payload") @@ -504,10 +500,6 @@ func (p *Builder) handleHeaderRequestDeneb(w http.ResponseWriter) { v := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(b.Value)) // we set the payload value as twice its actual one so that it always chooses builder payloads vs local payloads v = v.Mul(v, big.NewInt(2)) - // Is used as the helper modifies the big.Int - weiVal := big.NewInt(0).SetBytes(bytesutil.ReverseByteOrder(b.Value)) - // we set the payload value as twice its actual one so that it always chooses builder payloads vs local payloads - weiVal = weiVal.Mul(weiVal, big.NewInt(2)) wObj, err := blocks.WrappedExecutionPayloadDeneb(b.Payload) if err != nil { p.cfg.logger.WithError(err).Error("Could not wrap execution payload") From e3134dd5ccb0f05e7f3532c93b67a58e8869f257 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 14:58:52 -0500 Subject: [PATCH 14/16] de-duplicate imports --- api/client/builder/client_test.go | 3 +-- consensus-types/blocks/get_payload.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/api/client/builder/client_test.go b/api/client/builder/client_test.go index 2a53bf69bcd0..4e66c98b0a1d 100644 --- a/api/client/builder/client_test.go +++ b/api/client/builder/client_test.go @@ -16,7 +16,6 @@ import ( "github.com/prysmaticlabs/prysm/v5/api/server/structs" "github.com/prysmaticlabs/prysm/v5/config/params" "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" - "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" types "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" v1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" @@ -201,7 +200,7 @@ func TestClient_GetHeader(t *testing.T) { require.Equal(t, uint64(1), bidHeader.GasUsed()) value, err := stringToUint256("652312848583266388373324160190187140051835877600158453279131187530910662656") require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%#x", value.Bytes()), fmt.Sprintf("%#x", primitives.WeiToBigInt(bid.WeiValue()).Bytes())) + require.Equal(t, fmt.Sprintf("%#x", value.Bytes()), fmt.Sprintf("%#x", types.WeiToBigInt(bid.WeiValue()).Bytes())) bidValue := bytesutil.ReverseByteOrder(bid.Value()) require.DeepEqual(t, bidValue, value.Bytes()) require.DeepEqual(t, big.NewInt(0).SetBytes(bidValue), value.Int) diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index 81bfda64b0da..030665e3fa3b 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -3,7 +3,6 @@ package blocks import ( "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" - enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" pb "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" "google.golang.org/protobuf/proto" ) @@ -12,7 +11,7 @@ import ( // GetPayloadResponseV(1|2|3|4) value. type GetPayloadResponse struct { ExecutionData interfaces.ExecutionData - BlobsBundle *enginev1.BlobsBundle + BlobsBundle *pb.BlobsBundle OverrideBuilder bool // todo: should we convert this to Gwei up front? Bid primitives.Wei From 17b05fe57d29f41570b09bc096c90c7dee76c14a Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 15:32:52 -0500 Subject: [PATCH 15/16] remove errant comment --- .../rpc/prysm/v1alpha1/validator/proposer_execution_payload.go | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go index b665548ca7e8..d770eec2e687 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go @@ -81,7 +81,6 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe res, err := vs.ExecutionEngineCaller.GetPayload(ctx, pid, slot) if err == nil { warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) - //return payload, overrideBuilder, nil return res, nil } // TODO: TestServer_getExecutionPayloadContextTimeout expects this behavior. From 79ec647ff2a4613bddc64fc6b7be13caecb028dd Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 22 May 2024 16:54:52 -0500 Subject: [PATCH 16/16] James feedback --- api/client/builder/bid.go | 24 +++----------- api/client/builder/client_test.go | 32 +++++++++---------- .../rpc/prysm/v1alpha1/validator/proposer.go | 2 +- .../v1alpha1/validator/proposer_bellatrix.go | 9 +++--- .../validator/proposer_execution_payload.go | 17 +++++----- 5 files changed, 33 insertions(+), 51 deletions(-) diff --git a/api/client/builder/bid.go b/api/client/builder/bid.go index d2f7c4e77326..f88fd6194eac 100644 --- a/api/client/builder/bid.go +++ b/api/client/builder/bid.go @@ -23,8 +23,7 @@ type SignedBid interface { type Bid interface { Header() (interfaces.ExecutionData, error) BlobKzgCommitments() ([][]byte, error) - Value() []byte - WeiValue() primitives.Wei + Value() primitives.Wei Pubkey() []byte Version() int IsNil() bool @@ -127,12 +126,7 @@ func (b builderBid) Version() int { } // Value -- -func (b builderBid) Value() []byte { - return b.p.Value -} - -// WeiValue -- -func (b builderBid) WeiValue() primitives.Wei { +func (b builderBid) Value() primitives.Wei { return primitives.LittleEndianBytesToWei(b.p.Value) } @@ -186,12 +180,7 @@ func (b builderBidCapella) Version() int { } // Value -- -func (b builderBidCapella) Value() []byte { - return b.p.Value -} - -// WeiValue -- -func (b builderBidCapella) WeiValue() primitives.Wei { +func (b builderBidCapella) Value() primitives.Wei { return primitives.LittleEndianBytesToWei(b.p.Value) } @@ -234,12 +223,7 @@ func (b builderBidDeneb) Version() int { } // Value -- -func (b builderBidDeneb) Value() []byte { - return b.p.Value -} - -// WeiValue -- -func (b builderBidDeneb) WeiValue() primitives.Wei { +func (b builderBidDeneb) Value() primitives.Wei { return primitives.LittleEndianBytesToWei(b.p.Value) } diff --git a/api/client/builder/client_test.go b/api/client/builder/client_test.go index 4e66c98b0a1d..02070acdb3d6 100644 --- a/api/client/builder/client_test.go +++ b/api/client/builder/client_test.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "io" - "math/big" "net/http" "net/url" "strconv" @@ -16,6 +15,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/api/server/structs" "github.com/prysmaticlabs/prysm/v5/config/params" "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" + "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" types "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" v1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1" @@ -198,12 +198,12 @@ func TestClient_GetHeader(t *testing.T) { require.NoError(t, err) require.Equal(t, true, bytes.Equal(expectedTxRoot, withdrawalsRoot)) require.Equal(t, uint64(1), bidHeader.GasUsed()) - value, err := stringToUint256("652312848583266388373324160190187140051835877600158453279131187530910662656") + // this matches the value in the testExampleHeaderResponse + bidStr := "652312848583266388373324160190187140051835877600158453279131187530910662656" + value, err := stringToUint256(bidStr) require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%#x", value.Bytes()), fmt.Sprintf("%#x", types.WeiToBigInt(bid.WeiValue()).Bytes())) - bidValue := bytesutil.ReverseByteOrder(bid.Value()) - require.DeepEqual(t, bidValue, value.Bytes()) - require.DeepEqual(t, big.NewInt(0).SetBytes(bidValue), value.Int) + require.Equal(t, 0, value.Int.Cmp(primitives.WeiToBigInt(bid.Value()))) + require.Equal(t, bidStr, primitives.WeiToBigInt(bid.Value()).String()) }) t.Run("capella", func(t *testing.T) { hc := &http.Client{ @@ -230,12 +230,11 @@ func TestClient_GetHeader(t *testing.T) { withdrawalsRoot, err := bidHeader.WithdrawalsRoot() require.NoError(t, err) require.Equal(t, true, bytes.Equal(expectedWithdrawalsRoot, withdrawalsRoot)) - value, err := stringToUint256("652312848583266388373324160190187140051835877600158453279131187530910662656") + bidStr := "652312848583266388373324160190187140051835877600158453279131187530910662656" + value, err := stringToUint256(bidStr) require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%#x", value.SSZBytes()), fmt.Sprintf("%#x", bid.Value())) - bidValue := bytesutil.ReverseByteOrder(bid.Value()) - require.DeepEqual(t, bidValue, value.Bytes()) - require.DeepEqual(t, big.NewInt(0).SetBytes(bidValue), value.Int) + require.Equal(t, 0, value.Int.Cmp(primitives.WeiToBigInt(bid.Value()))) + require.Equal(t, bidStr, primitives.WeiToBigInt(bid.Value()).String()) }) t.Run("deneb", func(t *testing.T) { hc := &http.Client{ @@ -262,12 +261,13 @@ func TestClient_GetHeader(t *testing.T) { withdrawalsRoot, err := bidHeader.WithdrawalsRoot() require.NoError(t, err) require.Equal(t, true, bytes.Equal(expectedWithdrawalsRoot, withdrawalsRoot)) - value, err := stringToUint256("652312848583266388373324160190187140051835877600158453279131187530910662656") + + bidStr := "652312848583266388373324160190187140051835877600158453279131187530910662656" + value, err := stringToUint256(bidStr) require.NoError(t, err) - require.Equal(t, fmt.Sprintf("%#x", value.SSZBytes()), fmt.Sprintf("%#x", bid.Value())) - bidValue := bytesutil.ReverseByteOrder(bid.Value()) - require.DeepEqual(t, bidValue, value.Bytes()) - require.DeepEqual(t, big.NewInt(0).SetBytes(bidValue), value.Int) + require.Equal(t, 0, value.Int.Cmp(primitives.WeiToBigInt(bid.Value()))) + require.Equal(t, bidStr, primitives.WeiToBigInt(bid.Value()).String()) + kcgCommitments, err := bid.BlobKzgCommitments() require.NoError(t, err) require.Equal(t, len(kcgCommitments) > 0, true) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go index dd73e282c9ca..5a07de13d243 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go @@ -227,7 +227,7 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed winningBid := primitives.ZeroWei var bundle *enginev1.BlobsBundle - if slots.ToEpoch(sBlk.Block().Slot()) >= params.BeaconConfig().BellatrixForkEpoch { + if sBlk.Version() >= version.Bellatrix { local, err := vs.getLocalPayload(ctx, sBlk.Block(), head) if err != nil { return nil, status.Errorf(codes.Internal, "Could not get local payload: %v", err) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index 6afd343313b7..925322ee7e8f 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go @@ -95,7 +95,7 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc // Compare payload values between local and builder. Default to the local value if it is higher. localValueGwei := primitives.WeiToGwei(local.Bid) - builderValueGwei := primitives.WeiToGwei(bid.WeiValue()) + builderValueGwei := primitives.WeiToGwei(bid.Value()) // Use builder payload if the following in true: // builder_bid_value * builderBoostFactor(default 100) > local_block_value * (local-block-value-boost + 100) boost := primitives.Gwei(params.BeaconConfig().LocalBlockValueBoost) @@ -117,7 +117,7 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc log.WithError(err).Warn("Proposer: failed to set builder payload") return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } else { - return bid.WeiValue(), nil, nil + return bid.Value(), nil, nil } } if !higherValueBuilder { @@ -141,7 +141,7 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc log.WithError(err).Warn("Proposer: failed to set builder payload") return local.Bid, local.BlobsBundle, setLocalExecution(blk, local) } else { - return bid.WeiValue(), nil, nil + return bid.Value(), nil, nil } } } @@ -200,7 +200,7 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot primitiv return nil, errors.New("builder returned nil bid") } - v := bid.WeiValue() + v := bid.Value() if big.NewInt(0).Cmp(v) == 0 { return nil, errors.New("builder returned header with 0 bid amount") } @@ -323,7 +323,6 @@ func setLocalExecution(blk interfaces.SignedBeaconBlock, local *blocks.GetPayloa if local.BlobsBundle != nil { kzgCommitments = local.BlobsBundle.KzgCommitments } - // TODO: plumb blobs bundle through here. return setExecution(blk, local.ExecutionData, false, kzgCommitments) } diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go index d770eec2e687..78db18d85c14 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go @@ -80,7 +80,7 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe payloadIDCacheHit.Inc() res, err := vs.ExecutionEngineCaller.GetPayload(ctx, pid, slot) if err == nil { - warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) + warnIfFeeRecipientDiffers(val.FeeRecipient[:], res.ExecutionData.FeeRecipient()) return res, nil } // TODO: TestServer_getExecutionPayloadContextTimeout expects this behavior. @@ -178,19 +178,18 @@ func (vs *Server) getLocalPayload(ctx context.Context, blk interfaces.ReadOnlyBe return nil, err } - warnIfFeeRecipientDiffers(res.ExecutionData.FeeRecipient(), val.FeeRecipient[:]) + warnIfFeeRecipientDiffers(val.FeeRecipient[:], res.ExecutionData.FeeRecipient()) log.WithField("value", res.Bid).Debug("received execution payload from local engine") return res, nil } -// warnIfFeeRecipientDiffers logs a warning if the fee recipient in the included payload does not -// match the requested one. -func warnIfFeeRecipientDiffers(payload, val []byte) { - // Warn if the fee recipient is not the value we expect. - if !bytes.Equal(payload, val) { +// warnIfFeeRecipientDiffers logs a warning if the fee recipient in the payload (eg the EL engine get payload response) does not +// match what was expected (eg the fee recipient previously used to request preparation of the payload). +func warnIfFeeRecipientDiffers(want, got []byte) { + if !bytes.Equal(want, got) { logrus.WithFields(logrus.Fields{ - "wantedFeeRecipient": fmt.Sprintf("%#x", val), - "received": fmt.Sprintf("%#x", payload), + "wantedFeeRecipient": fmt.Sprintf("%#x", want), + "received": fmt.Sprintf("%#x", got), }).Warn("Fee recipient address from execution client is not what was expected. " + "It is possible someone has compromised your client to try and take your transaction fees") }