From 115d4c796139b06ea7906d035c2d93d0e47054c2 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 19 Jul 2024 15:31:58 -0500 Subject: [PATCH] changing design a little bit based on feedback --- validator/client/runner.go | 14 ++++-------- validator/client/runner_test.go | 34 ------------------------------ validator/client/validator.go | 12 ++++++++--- validator/client/validator_test.go | 7 +++++- 4 files changed, 19 insertions(+), 48 deletions(-) diff --git a/validator/client/runner.go b/validator/client/runner.go index dbf3009c3d59..14cdf5cfe2b6 100644 --- a/validator/client/runner.go +++ b/validator/client/runner.go @@ -63,11 +63,7 @@ func run(ctx context.Context, v iface.Validator) { } deadline := time.Now().Add(5 * time.Minute) if err := v.PushProposerSettings(ctx, km, headSlot, deadline); err != nil { - if errors.Is(err, ErrBuilderValidatorRegistration) { - log.WithError(err).Warn("Push proposer settings error") - } else { - log.WithError(err).Fatal("Failed to update proposer settings") // allow fatal. skipcq - } + log.WithError(err).Fatal("Failed to update proposer settings") // allow fatal. skipcq } for { ctx, span := trace.StartSpan(ctx, "validator.processSlot") @@ -101,11 +97,9 @@ func run(ctx context.Context, v iface.Validator) { // call push proposer settings often to account for the following edge cases: // proposer is activated at the start of epoch and tries to propose immediately // account has changed in the middle of an epoch - go func() { - if err := v.PushProposerSettings(ctx, km, slot, time.Now().Add(1*time.Minute)); err != nil { - log.WithError(err).Warn("Failed to update proposer settings") - } - }() + if err := v.PushProposerSettings(ctx, km, slot, time.Now().Add(1*time.Minute)); err != nil { + log.WithError(err).Warn("Failed to update proposer settings") + } // Start fetching domain data for the next epoch. if slots.IsEpochEnd(slot) { diff --git a/validator/client/runner_test.go b/validator/client/runner_test.go index dffadbabcc82..aee9800da4a9 100644 --- a/validator/client/runner_test.go +++ b/validator/client/runner_test.go @@ -370,37 +370,3 @@ func TestUpdateProposerSettingsAt_EpochEndOk(t *testing.T) { // can't test "Failed to update proposer settings" because of log.fatal assert.LogsContain(t, hook, "Mock updated proposer settings") } - -func TestUpdateProposerSettings_ContinuesAfterValidatorRegistrationFails(t *testing.T) { - errSomeOtherError := errors.New("some internal error") - ctrl := gomock.NewController(t) - defer ctrl.Finish() - node := healthTesting.NewMockHealthClient(ctrl) - tracker := beacon.NewNodeHealthTracker(node) - node.EXPECT().IsHealthy(gomock.Any()).Return(true).AnyTimes() - v := &testutil.FakeValidator{ - ProposerSettingsErr: errors.Wrap(ErrBuilderValidatorRegistration, errSomeOtherError.Error()), - Km: &mockKeymanager{accountsChangedFeed: &event.Feed{}}, - Tracker: tracker, - } - err := v.SetProposerSettings(context.Background(), &proposer.Settings{ - DefaultConfig: &proposer.Option{ - FeeRecipientConfig: &proposer.FeeRecipientConfig{ - FeeRecipient: common.HexToAddress("0x046Fb65722E7b2455012BFEBf6177F1D2e9738D9"), - }, - }, - }) - require.NoError(t, err) - ctx, cancel := context.WithCancel(context.Background()) - hook := logTest.NewGlobal() - slot := params.BeaconConfig().SlotsPerEpoch - ticker := make(chan primitives.Slot) - v.NextSlotRet = ticker - go func() { - ticker <- slot - - cancel() - }() - run(ctx, v) - assert.LogsContain(t, hook, ErrBuilderValidatorRegistration.Error()) -} diff --git a/validator/client/validator.go b/validator/client/validator.go index 4d8fbc668061..9423cfaad92e 100644 --- a/validator/client/validator.go +++ b/validator/client/validator.go @@ -1125,6 +1125,7 @@ func (v *validator) PushProposerSettings(ctx context.Context, km keymanager.IKey if err != nil { return err } + proposerReqs, err := v.buildPrepProposerReqs(filteredKeys) if err != nil { return err @@ -1139,16 +1140,21 @@ func (v *validator) PushProposerSettings(ctx context.Context, km keymanager.IKey "proposerSettingsRequestCount": len(proposerReqs), }).Debugln("Request count did not match included validator count. Only keys that have been activated will be included in the request.") } + if _, err := v.validatorClient.PrepareBeaconProposer(ctx, ðpb.PrepareBeaconProposerRequest{ Recipients: proposerReqs, }); err != nil { return err } - signedRegReqs := v.buildSignedRegReqs(ctx, filteredKeys, km.Sign) - if err := SubmitValidatorRegistrations(ctx, v.validatorClient, signedRegReqs, v.validatorsRegBatchSize); err != nil { - return errors.Wrap(ErrBuilderValidatorRegistration, err.Error()) + if len(signedRegReqs) > 0 { + go func() { + if err := SubmitValidatorRegistrations(ctx, v.validatorClient, signedRegReqs, v.validatorsRegBatchSize); err != nil { + log.WithError(errors.Wrap(ErrBuilderValidatorRegistration, err.Error())).Warn("failed to register validator on builder") + } + }() } + return nil } diff --git a/validator/client/validator_test.go b/validator/client/validator_test.go index 734dfbd40702..69af50cf72e7 100644 --- a/validator/client/validator_test.go +++ b/validator/client/validator_test.go @@ -1477,6 +1477,7 @@ func TestValidator_PushSettings(t *testing.T) { feeRecipientMap map[primitives.ValidatorIndex]string mockExpectedRequests []ExpectedValidatorRegistration err string + logDelay time.Duration logMessages []string doesntContainLogs bool }{ @@ -1993,7 +1994,8 @@ func TestValidator_PushSettings(t *testing.T) { ).Return(&empty.Empty{}, errors.New("request failed")) return &v }, - err: "could not submit signed registrations to beacon node", + logMessages: []string{"request failed"}, + logDelay: 1 * time.Second, }, } for _, tt := range tests { @@ -2030,6 +2032,9 @@ func TestValidator_PushSettings(t *testing.T) { assert.ErrorContains(t, tt.err, err) } if len(tt.logMessages) > 0 { + if tt.logDelay > 0 { + time.Sleep(tt.logDelay) + } for _, message := range tt.logMessages { if tt.doesntContainLogs { assert.LogsDoNotContain(t, hook, message)