From 436792fe385134611376a54b03bb7e5f6264eaf7 Mon Sep 17 00:00:00 2001 From: terencechain Date: Thu, 25 Aug 2022 11:24:02 -0700 Subject: [PATCH] Builder: filter header with 0 bid and empty tx root (#11313) * Filter header with 0 bid and empty root * Check nil Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com> --- .../rpc/prysm/v1alpha1/validator/BUILD.bazel | 1 + .../v1alpha1/validator/proposer_bellatrix.go | 22 +++++ .../validator/proposer_bellatrix_test.go | 82 +++++++++++++++++-- 3 files changed, 99 insertions(+), 6 deletions(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel b/beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel index dd1c32ea40..232af306f1 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel @@ -64,6 +64,7 @@ go_library( "//crypto/hash:go_default_library", "//crypto/rand:go_default_library", "//encoding/bytesutil:go_default_library", + "//encoding/ssz:go_default_library", "//monitoring/tracing:go_default_library", "//network/forks:go_default_library", "//proto/engine/v1:go_default_library", diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index d8466f0bcb..481ed39284 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" "time" "github.com/pkg/errors" @@ -19,6 +20,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/consensus-types/interfaces" types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" + "github.com/prysmaticlabs/prysm/v3/encoding/ssz" enginev1 "github.com/prysmaticlabs/prysm/v3/proto/engine/v1" ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v3/runtime/version" @@ -108,6 +110,7 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot types.Sl if blocks.IsPreBellatrixVersion(b.Version()) { return nil, nil } + h, err := b.Block().Body().Execution() if err != nil { return nil, err @@ -120,6 +123,25 @@ func (vs *Server) getPayloadHeaderFromBuilder(ctx context.Context, slot types.Sl if err != nil { return nil, err } + if bid == nil || bid.Message == nil { + return nil, errors.New("builder returned nil bid") + } + + v := bid.Message.Value + + if new(big.Int).SetBytes(bytesutil.ReverseByteOrder(v)).String() == "0" { + return nil, errors.New("builder returned header with 0 bid amount") + } + + emptyRoot, err := ssz.TransactionsRoot([][]byte{}) + if err != nil { + return nil, err + } + + if bytesutil.ToBytes32(bid.Message.Header.TransactionsRoot) == emptyRoot { + return nil, errors.New("builder returned header with an empty tx root") + } + if !bytes.Equal(bid.Message.Header.ParentHash, h.BlockHash()) { return nil, fmt.Errorf("incorrect parent hash %#x != %#x", bid.Message.Header.ParentHash, h.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 72272faff3..4d6822fc03 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -97,6 +97,40 @@ func TestServer_buildHeaderBlock(t *testing.T) { } func TestServer_getPayloadHeader(t *testing.T) { + emptyRoot, err := ssz.TransactionsRoot([][]byte{}) + require.NoError(t, err) + ti, err := slots.ToTime(uint64(time.Now().Unix()), 0) + require.NoError(t, err) + + sk, err := bls.RandKey() + require.NoError(t, err) + bid := ðpb.BuilderBid{ + Header: &v1.ExecutionPayloadHeader{ + FeeRecipient: make([]byte, fieldparams.FeeRecipientLength), + StateRoot: make([]byte, fieldparams.RootLength), + ReceiptsRoot: make([]byte, fieldparams.RootLength), + LogsBloom: make([]byte, fieldparams.LogsBloomLength), + PrevRandao: make([]byte, fieldparams.RootLength), + BaseFeePerGas: make([]byte, fieldparams.RootLength), + BlockHash: make([]byte, fieldparams.RootLength), + TransactionsRoot: bytesutil.PadTo([]byte{1}, fieldparams.RootLength), + ParentHash: params.BeaconConfig().ZeroHash[:], + Timestamp: uint64(ti.Unix()), + }, + Pubkey: sk.PublicKey().Marshal(), + Value: bytesutil.PadTo([]byte{1, 2, 3}, 32), + } + d := params.BeaconConfig().DomainApplicationBuilder + domain, err := signing.ComputeDomain(d, nil, nil) + require.NoError(t, err) + sr, err := signing.ComputeSigningRoot(bid, domain) + require.NoError(t, err) + sBid := ðpb.SignedBuilderBid{ + Message: bid, + Signature: sk.Sign(sr[:]).Marshal(), + } + + require.NoError(t, err) tests := []struct { name string head interfaces.SignedBeaconBlock @@ -131,7 +165,7 @@ func TestServer_getPayloadHeader(t *testing.T) { err: "can't get header", }, { - name: "get header correct", + name: "0 bid", mock: &builderTest.MockBuilderService{ Bid: ðpb.SignedBuilderBid{ Message: ðpb.BuilderBid{ @@ -140,7 +174,6 @@ func TestServer_getPayloadHeader(t *testing.T) { }, }, }, - ErrGetHeader: errors.New("can't get header"), }, fetcher: &blockchainTest.ChainService{ Block: func() interfaces.SignedBeaconBlock { @@ -149,18 +182,55 @@ func TestServer_getPayloadHeader(t *testing.T) { return wb }(), }, - returnedHeader: &v1.ExecutionPayloadHeader{ - BlockNumber: 123, + err: "builder returned header with 0 bid amount", + }, + { + name: "invalid tx root", + mock: &builderTest.MockBuilderService{ + Bid: ðpb.SignedBuilderBid{ + Message: ðpb.BuilderBid{ + Value: []byte{1}, + Header: &v1.ExecutionPayloadHeader{ + BlockNumber: 123, + TransactionsRoot: emptyRoot[:], + }, + }, + }, }, + fetcher: &blockchainTest.ChainService{ + Block: func() interfaces.SignedBeaconBlock { + wb, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockBellatrix()) + require.NoError(t, err) + return wb + }(), + }, + err: "builder returned header with an empty tx root", + }, + { + name: "can get header", + mock: &builderTest.MockBuilderService{ + Bid: sBid, + }, + fetcher: &blockchainTest.ChainService{ + Block: func() interfaces.SignedBeaconBlock { + wb, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockBellatrix()) + require.NoError(t, err) + return wb + }(), + }, + returnedHeader: bid.Header, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - vs := &Server{BlockBuilder: tc.mock, HeadFetcher: tc.fetcher} + vs := &Server{BlockBuilder: tc.mock, HeadFetcher: tc.fetcher, TimeFetcher: &blockchainTest.ChainService{ + Genesis: time.Now(), + }} h, err := vs.getPayloadHeaderFromBuilder(context.Background(), 0, 0) - if err != nil { + if tc.err != "" { require.ErrorContains(t, tc.err, err) } else { + require.NoError(t, err) require.DeepEqual(t, tc.returnedHeader, h) } })