diff --git a/beacon-chain/rpc/eth/validator/validator_test.go b/beacon-chain/rpc/eth/validator/validator_test.go index 872368c06c2d..e7e22f948ecd 100644 --- a/beacon-chain/rpc/eth/validator/validator_test.go +++ b/beacon-chain/rpc/eth/validator/validator_test.go @@ -2727,7 +2727,7 @@ func TestProduceBlindedBlock(t *testing.T) { } id := &enginev1.PayloadIDBytes{0x1} v1Alpha1Server := &v1alpha1validator.Server{ - ExecutionEngineCaller: &mockExecution.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &enginev1.ExecutionPayloadCapella{BlockNumber: 1}, BlockValue: big.NewInt(0)}, + ExecutionEngineCaller: &mockExecution.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &enginev1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: wds}, BlockValue: big.NewInt(0)}, BeaconDB: db, ForkFetcher: &mockChain.ChainService{ForkChoiceStore: doublylinkedtree.New()}, TimeFetcher: &mockChain.ChainService{ diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go index 1bdb5f139ad1..1729e449f82f 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go @@ -13,10 +13,12 @@ import ( "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing" "github.com/prysmaticlabs/prysm/v3/beacon-chain/state" + fieldparams "github.com/prysmaticlabs/prysm/v3/config/fieldparams" "github.com/prysmaticlabs/prysm/v3/config/params" consensusblocks "github.com/prysmaticlabs/prysm/v3/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v3/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives" + "github.com/prysmaticlabs/prysm/v3/crypto/hash" "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v3/encoding/ssz" enginev1 "github.com/prysmaticlabs/prysm/v3/proto/engine/v1" @@ -68,8 +70,13 @@ func (vs *Server) setExecutionData(ctx context.Context, blk interfaces.SignedBea if err != nil { log.WithError(err).Warn("Proposer: failed to get builder payload value") // Default to local if can't get builder value. } + + withdrawalsMatched, err := matchingWithdrawalsRoot(localPayload, builderPayload) + if err != nil { + return errors.Wrap(err, "failed to match withdrawals root") + } // If we can't get the builder value, just use local block. - if err == nil && builderValue.Cmp(localValue) > 0 { // Builder value is higher + if builderValue.Cmp(localValue) > 0 && withdrawalsMatched { // Builder value is higher and withdrawals match. blk.SetBlinded(true) if err := blk.SetExecution(builderPayload); err != nil { log.WithError(err).Warn("Proposer: failed to set builder payload") @@ -87,6 +94,7 @@ func (vs *Server) setExecutionData(ctx context.Context, blk interfaces.SignedBea } } } + } executionData, err := vs.getExecutionPayload(ctx, slot, idx, blk.Block().ParentRoot(), headState) @@ -322,3 +330,27 @@ func validateBuilderSignature(signedBid builder.SignedBid) error { } return signing.VerifySigningRoot(bid, bid.Pubkey(), signedBid.Signature(), d) } + +func matchingWithdrawalsRoot(local, builder interfaces.ExecutionData) (bool, error) { + wds, err := local.Withdrawals() + if err != nil { + return false, errors.Wrap(err, "could not get local withdrawals") + } + br, err := builder.WithdrawalsRoot() + if err != nil { + return false, errors.Wrap(err, "could not get builder withdrawals root") + } + wr, err := ssz.WithdrawalSliceRoot(hash.CustomSHA256Hasher(), wds, fieldparams.MaxWithdrawalsPerPayload) + if err != nil { + return false, errors.Wrap(err, "could not compute local withdrawals root") + } + + if !bytes.Equal(br, wr[:]) { + log.WithFields(logrus.Fields{ + "local": fmt.Sprintf("%#x", wr), + "builder": fmt.Sprintf("%#x", br), + }).Warn("Proposer: withdrawal roots don't match, using local block") + return false, nil + } + return true, 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 26bb5b47c7e9..59ea57bce4c0 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -22,6 +22,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v3/crypto/bls" + "github.com/prysmaticlabs/prysm/v3/crypto/hash" "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v3/encoding/ssz" v1 "github.com/prysmaticlabs/prysm/v3/proto/engine/v1" @@ -53,10 +54,15 @@ func TestServer_setExecutionData(t *testing.T) { })) require.NoError(t, beaconDB.SaveFeeRecipientsByValidatorIDs(context.Background(), []primitives.ValidatorIndex{0}, []common.Address{{}})) - require.NoError(t, err) + withdrawals := []*v1.Withdrawal{{ + Index: 1, + ValidatorIndex: 2, + Address: make([]byte, fieldparams.FeeRecipientLength), + Amount: 3, + }} id := &v1.PayloadIDBytes{0x1} vs := &Server{ - ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 1}, BlockValue: big.NewInt(0)}, + ExecutionEngineCaller: &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 1, Withdrawals: withdrawals}, BlockValue: big.NewInt(0)}, HeadFetcher: &blockchainTest.ChainService{State: capellaTransitionState}, BeaconDB: beaconDB, ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(), @@ -70,8 +76,8 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(1), e.BlockNumber()) // Local block }) - t.Run("Builder configured. Builder Block has higher value", func(t *testing.T) { - blk, err := blocks.NewSignedBeaconBlock(util.NewBlindedBeaconBlockCapella()) + t.Run("Builder configured. Builder Block has higher value. Incorrect withdrawals", func(t *testing.T) { + blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) require.NoError(t, err) require.NoError(t, vs.BeaconDB.SaveRegistrationsByValidatorIDs(ctx, []primitives.ValidatorIndex{blk.Block().ProposerIndex()}, []*ethpb.ValidatorRegistrationV1{{FeeRecipient: make([]byte, fieldparams.FeeRecipientLength), Pubkey: make([]byte, fieldparams.BLSPubkeyLength)}})) @@ -119,6 +125,59 @@ func TestServer_setExecutionData(t *testing.T) { require.NoError(t, vs.setExecutionData(context.Background(), blk, capellaTransitionState)) e, err := blk.Block().Body().Execution() require.NoError(t, err) + require.Equal(t, uint64(1), e.BlockNumber()) // Local block because incorrect withdrawals + }) + t.Run("Builder configured. Builder Block has higher value. Correct withdrawals.", func(t *testing.T) { + blk, err := blocks.NewSignedBeaconBlock(util.NewBlindedBeaconBlockCapella()) + require.NoError(t, err) + require.NoError(t, vs.BeaconDB.SaveRegistrationsByValidatorIDs(ctx, []primitives.ValidatorIndex{blk.Block().ProposerIndex()}, + []*ethpb.ValidatorRegistrationV1{{FeeRecipient: make([]byte, fieldparams.FeeRecipientLength), Pubkey: make([]byte, fieldparams.BLSPubkeyLength)}})) + ti, err := slots.ToTime(uint64(time.Now().Unix()), 0) + require.NoError(t, err) + sk, err := bls.RandKey() + require.NoError(t, err) + wr, err := ssz.WithdrawalSliceRoot(hash.CustomSHA256Hasher(), withdrawals, fieldparams.MaxWithdrawalsPerPayload) + require.NoError(t, err) + bid := ðpb.BuilderBidCapella{ + Header: &v1.ExecutionPayloadHeaderCapella{ + 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()), + BlockNumber: 2, + WithdrawalsRoot: wr[:], + }, + Pubkey: sk.PublicKey().Marshal(), + Value: bytesutil.PadTo([]byte{1}, 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.SignedBuilderBidCapella{ + Message: bid, + Signature: sk.Sign(sr[:]).Marshal(), + } + vs.BlockBuilder = &builderTest.MockBuilderService{ + BidCapella: sBid, + } + wb, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockBellatrix()) + require.NoError(t, err) + chain := &blockchainTest.ChainService{ForkChoiceStore: doublylinkedtree.New(), Genesis: time.Now(), Block: wb} + vs.ForkFetcher = chain + vs.ForkFetcher.ForkChoicer().SetGenesisTime(uint64(time.Now().Unix())) + vs.TimeFetcher = chain + vs.HeadFetcher = chain + require.NoError(t, vs.setExecutionData(context.Background(), blk, capellaTransitionState)) + e, err := blk.Block().Body().Execution() + require.NoError(t, err) require.Equal(t, uint64(2), e.BlockNumber()) // Builder block }) t.Run("Builder configured. Local block has higher value", func(t *testing.T) { @@ -467,3 +526,54 @@ func TestServer_validateBuilderSignature(t *testing.T) { require.NoError(t, err) require.ErrorIs(t, validateBuilderSignature(sBid), signing.ErrSigFailedToVerify) } + +func Test_matchingWithdrawalsRoot(t *testing.T) { + t.Run("could not get local withdrawals", func(t *testing.T) { + local := &v1.ExecutionPayload{} + p, err := blocks.WrappedExecutionPayload(local) + require.NoError(t, err) + _, err = matchingWithdrawalsRoot(p, p) + require.ErrorContains(t, "could not get local withdrawals", err) + }) + t.Run("could not get builder withdrawals root", func(t *testing.T) { + local := &v1.ExecutionPayloadCapella{} + p, err := blocks.WrappedExecutionPayloadCapella(local, big.NewInt(0)) + require.NoError(t, err) + header := &v1.ExecutionPayloadHeader{} + h, err := blocks.WrappedExecutionPayloadHeader(header) + require.NoError(t, err) + _, err = matchingWithdrawalsRoot(p, h) + require.ErrorContains(t, "could not get builder withdrawals root", err) + }) + t.Run("withdrawals mismatch", func(t *testing.T) { + local := &v1.ExecutionPayloadCapella{} + p, err := blocks.WrappedExecutionPayloadCapella(local, big.NewInt(0)) + require.NoError(t, err) + header := &v1.ExecutionPayloadHeaderCapella{} + h, err := blocks.WrappedExecutionPayloadHeaderCapella(header, big.NewInt(0)) + require.NoError(t, err) + matched, err := matchingWithdrawalsRoot(p, h) + require.NoError(t, err) + require.Equal(t, false, matched) + }) + t.Run("withdrawals match", func(t *testing.T) { + wds := []*v1.Withdrawal{{ + Index: 1, + ValidatorIndex: 2, + Address: make([]byte, fieldparams.FeeRecipientLength), + Amount: 3, + }} + local := &v1.ExecutionPayloadCapella{Withdrawals: wds} + p, err := blocks.WrappedExecutionPayloadCapella(local, big.NewInt(0)) + require.NoError(t, err) + header := &v1.ExecutionPayloadHeaderCapella{} + wr, err := ssz.WithdrawalSliceRoot(hash.CustomSHA256Hasher(), wds, fieldparams.MaxWithdrawalsPerPayload) + require.NoError(t, err) + header.WithdrawalsRoot = wr[:] + h, err := blocks.WrappedExecutionPayloadHeaderCapella(header, big.NewInt(0)) + require.NoError(t, err) + matched, err := matchingWithdrawalsRoot(p, h) + require.NoError(t, err) + require.Equal(t, true, matched) + }) +}