From 0c32eb5c03a3151d59ff69710c2b1fb1f8e92cc5 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Tue, 23 Aug 2022 12:26:43 -0500 Subject: [PATCH] Beacon API: skip updating fee recipient if it's the same (#11296) * adding in redudant check * adding unit tests * fixing linting Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com> --- beacon-chain/rpc/eth/validator/BUILD.bazel | 2 + beacon-chain/rpc/eth/validator/validator.go | 20 ++++- .../rpc/eth/validator/validator_test.go | 84 +++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/beacon-chain/rpc/eth/validator/BUILD.bazel b/beacon-chain/rpc/eth/validator/BUILD.bazel index 77bf7f0f8..8cf3e5869 100644 --- a/beacon-chain/rpc/eth/validator/BUILD.bazel +++ b/beacon-chain/rpc/eth/validator/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//beacon-chain/cache:go_default_library", "//beacon-chain/core/helpers:go_default_library", "//beacon-chain/core/transition:go_default_library", + "//beacon-chain/db/kv:go_default_library", "//beacon-chain/operations/attestations:go_default_library", "//beacon-chain/operations/synccommittee:go_default_library", "//beacon-chain/p2p:go_default_library", @@ -86,6 +87,7 @@ go_test( "//time/slots:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", + "@com_github_sirupsen_logrus//hooks/test:go_default_library", "@org_golang_google_protobuf//proto:go_default_library", ], ) diff --git a/beacon-chain/rpc/eth/validator/validator.go b/beacon-chain/rpc/eth/validator/validator.go index 93c2cca0a..a7ce8f45d 100644 --- a/beacon-chain/rpc/eth/validator/validator.go +++ b/beacon-chain/rpc/eth/validator/validator.go @@ -16,6 +16,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/beacon-chain/cache" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/transition" + "github.com/prysmaticlabs/prysm/v3/beacon-chain/db/kv" rpchelpers "github.com/prysmaticlabs/prysm/v3/beacon-chain/rpc/eth/helpers" "github.com/prysmaticlabs/prysm/v3/beacon-chain/state" statev1 "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/v1" @@ -546,7 +547,24 @@ func (vs *Server) PrepareBeaconProposer( defer span.End() var feeRecipients []common.Address var validatorIndices []types.ValidatorIndex - for _, recipientContainer := range request.Recipients { + newRecipients := make([]*ethpbv1.PrepareBeaconProposerRequest_FeeRecipientContainer, 0, len(request.Recipients)) + for _, r := range request.Recipients { + f, err := vs.V1Alpha1Server.BeaconDB.FeeRecipientByValidatorID(ctx, r.ValidatorIndex) + switch { + case errors.Is(err, kv.ErrNotFoundFeeRecipient): + newRecipients = append(newRecipients, r) + case err != nil: + return nil, status.Errorf(codes.Internal, "Could not get fee recipient by validator index: %v", err) + default: + } + if common.BytesToAddress(r.FeeRecipient) != f { + newRecipients = append(newRecipients, r) + } + } + if len(newRecipients) == 0 { + return &emptypb.Empty{}, nil + } + for _, recipientContainer := range newRecipients { recipient := hexutil.Encode(recipientContainer.FeeRecipient) if !common.IsHexAddress(recipient) { return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("Invalid fee recipient address: %v", recipient)) diff --git a/beacon-chain/rpc/eth/validator/validator_test.go b/beacon-chain/rpc/eth/validator/validator_test.go index 0258ee7dd..449a02df7 100644 --- a/beacon-chain/rpc/eth/validator/validator_test.go +++ b/beacon-chain/rpc/eth/validator/validator_test.go @@ -44,6 +44,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/testing/require" "github.com/prysmaticlabs/prysm/v3/testing/util" "github.com/prysmaticlabs/prysm/v3/time/slots" + logTest "github.com/sirupsen/logrus/hooks/test" "google.golang.org/protobuf/proto" ) @@ -3621,6 +3622,89 @@ func TestPrepareBeaconProposer(t *testing.T) { }) } } +func TestProposer_PrepareBeaconProposerOverlapping(t *testing.T) { + hook := logTest.NewGlobal() + db := dbutil.SetupDB(t) + ctx := context.Background() + v1Server := &v1alpha1validator.Server{ + BeaconDB: db, + } + proposerServer := &Server{V1Alpha1Server: v1Server} + + // New validator + f := bytesutil.PadTo([]byte{0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF}, fieldparams.FeeRecipientLength) + req := ðpbv1.PrepareBeaconProposerRequest{ + Recipients: []*ethpbv1.PrepareBeaconProposerRequest_FeeRecipientContainer{ + {FeeRecipient: f, ValidatorIndex: 1}, + }, + } + _, err := proposerServer.PrepareBeaconProposer(ctx, req) + require.NoError(t, err) + require.LogsContain(t, hook, "Updated fee recipient addresses for validator indices") + + // Same validator + hook.Reset() + _, err = proposerServer.PrepareBeaconProposer(ctx, req) + require.NoError(t, err) + require.LogsDoNotContain(t, hook, "Updated fee recipient addresses for validator indices") + + // Same validator with different fee recipient + hook.Reset() + f = bytesutil.PadTo([]byte{0x01, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF}, fieldparams.FeeRecipientLength) + req = ðpbv1.PrepareBeaconProposerRequest{ + Recipients: []*ethpbv1.PrepareBeaconProposerRequest_FeeRecipientContainer{ + {FeeRecipient: f, ValidatorIndex: 1}, + }, + } + _, err = proposerServer.PrepareBeaconProposer(ctx, req) + require.NoError(t, err) + require.LogsContain(t, hook, "Updated fee recipient addresses for validator indices") + + // More than one validator + hook.Reset() + f = bytesutil.PadTo([]byte{0x01, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF}, fieldparams.FeeRecipientLength) + req = ðpbv1.PrepareBeaconProposerRequest{ + Recipients: []*ethpbv1.PrepareBeaconProposerRequest_FeeRecipientContainer{ + {FeeRecipient: f, ValidatorIndex: 1}, + {FeeRecipient: f, ValidatorIndex: 2}, + }, + } + _, err = proposerServer.PrepareBeaconProposer(ctx, req) + require.NoError(t, err) + require.LogsContain(t, hook, "Updated fee recipient addresses for validator indices") + + // Same validators + hook.Reset() + _, err = proposerServer.PrepareBeaconProposer(ctx, req) + require.NoError(t, err) + require.LogsDoNotContain(t, hook, "Updated fee recipient addresses for validator indices") +} + +func BenchmarkServer_PrepareBeaconProposer(b *testing.B) { + db := dbutil.SetupDB(b) + ctx := context.Background() + v1Server := &v1alpha1validator.Server{ + BeaconDB: db, + } + proposerServer := &Server{V1Alpha1Server: v1Server} + + f := bytesutil.PadTo([]byte{0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF, 0x01, 0xFF}, fieldparams.FeeRecipientLength) + recipients := make([]*ethpbv1.PrepareBeaconProposerRequest_FeeRecipientContainer, 0) + for i := 0; i < 10000; i++ { + recipients = append(recipients, ðpbv1.PrepareBeaconProposerRequest_FeeRecipientContainer{FeeRecipient: f, ValidatorIndex: types.ValidatorIndex(i)}) + } + + req := ðpbv1.PrepareBeaconProposerRequest{ + Recipients: recipients, + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, err := proposerServer.PrepareBeaconProposer(ctx, req) + if err != nil { + b.Fatal(err) + } + } +} func TestServer_SubmitValidatorRegistrations(t *testing.T) { type args struct {