Skip to content

Commit

Permalink
changing design a little bit based on feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
james-prysm committed Jul 19, 2024
1 parent 1528b98 commit 115d4c7
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 48 deletions.
14 changes: 4 additions & 10 deletions validator/client/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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) {
Expand Down
34 changes: 0 additions & 34 deletions validator/client/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
12 changes: 9 additions & 3 deletions validator/client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, &ethpb.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
}

Expand Down
7 changes: 6 additions & 1 deletion validator/client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 115d4c7

Please sign in to comment.