Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removing explicit deadlines on push proposer settings #14285

Merged
merged 3 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion validator/accounts/testing/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (*Validator) HasProposerSettings() bool {
}

// PushProposerSettings for mocking
func (_ *Validator) PushProposerSettings(_ context.Context, _ keymanager.IKeymanager, _ primitives.Slot, _ time.Time) error {
func (_ *Validator) PushProposerSettings(_ context.Context, _ keymanager.IKeymanager, _ primitives.Slot) error {
panic("implement me")
}

Expand Down
2 changes: 1 addition & 1 deletion validator/client/iface/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Validator interface {
Keymanager() (keymanager.IKeymanager, error)
HandleKeyReload(ctx context.Context, currentKeys [][fieldparams.BLSPubkeyLength]byte) (bool, error)
CheckDoppelGanger(ctx context.Context) error
PushProposerSettings(ctx context.Context, km keymanager.IKeymanager, slot primitives.Slot, deadline time.Time) error
PushProposerSettings(ctx context.Context, km keymanager.IKeymanager, slot primitives.Slot) error
SignValidatorRegistrationRequest(ctx context.Context, signer SigningFunc, newValidatorRegistration *ethpb.ValidatorRegistrationV1) (*ethpb.SignedValidatorRegistrationV1, error)
StartEventStream(ctx context.Context, topics []string, eventsChan chan<- *event.Event)
EventStreamIsRunning() bool
Expand Down
10 changes: 4 additions & 6 deletions validator/client/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ func run(ctx context.Context, v iface.Validator) {
log.Warn("Validator client started without proposer settings such as fee recipient" +
" and will continue to use settings provided in the beacon node.")
}
deadline := time.Now().Add(5 * time.Minute)
if err := v.PushProposerSettings(ctx, km, headSlot, deadline); err != nil {
log.WithError(err).Fatal("Failed to update proposer settings") // allow fatal. skipcq
if err := v.PushProposerSettings(ctx, km, headSlot); err != nil {
log.WithError(err).Fatal("Failed to update proposer settings")
}
for {
ctx, span := trace.StartSpan(ctx, "validator.processSlot")
Expand Down Expand Up @@ -97,7 +96,7 @@ 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
if err := v.PushProposerSettings(ctx, km, slot, deadline); err != nil {
if err := v.PushProposerSettings(ctx, km, slot); err != nil {
log.WithError(err).Warn("Failed to update proposer settings")
}

Expand Down Expand Up @@ -316,8 +315,7 @@ func runHealthCheckRoutine(ctx context.Context, v iface.Validator, eventsChan ch
log.WithError(err).Error("Could not get canonical head slot")
return
}
deadline := time.Now().Add(5 * time.Minute) // Should consider changing to a constant
if err := v.PushProposerSettings(ctx, km, slot, deadline); err != nil {
if err := v.PushProposerSettings(ctx, km, slot); err != nil {
log.WithError(err).Warn("Failed to update proposer settings")
}
}
Expand Down
5 changes: 1 addition & 4 deletions validator/client/testutil/mock_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,7 @@ func (*FakeValidator) HasProposerSettings() bool {
}

// PushProposerSettings for mocking
func (fv *FakeValidator) PushProposerSettings(ctx context.Context, _ keymanager.IKeymanager, _ primitives.Slot, deadline time.Time) error {
nctx, cancel := context.WithDeadline(ctx, deadline)
ctx = nctx
defer cancel()
func (fv *FakeValidator) PushProposerSettings(ctx context.Context, _ keymanager.IKeymanager, _ primitives.Slot) error {
time.Sleep(fv.ProposerSettingWait)
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
log.Error("deadline exceeded")
Expand Down
5 changes: 1 addition & 4 deletions validator/client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1102,16 +1102,13 @@ func (v *validator) SetProposerSettings(ctx context.Context, settings *proposer.
}

// PushProposerSettings calls the prepareBeaconProposer RPC to set the fee recipient and also the register validator API if using a custom builder.
func (v *validator) PushProposerSettings(ctx context.Context, km keymanager.IKeymanager, slot primitives.Slot, deadline time.Time) error {
func (v *validator) PushProposerSettings(ctx context.Context, km keymanager.IKeymanager, slot primitives.Slot) error {
ctx, span := trace.StartSpan(ctx, "validator.PushProposerSettings")
defer span.End()

if km == nil {
return errors.New("keymanager is nil when calling PrepareBeaconProposer")
}
nctx, cancel := context.WithDeadline(ctx, deadline)
ctx = nctx
defer cancel()

pubkeys, err := km.FetchValidatingPublicKeys(ctx)
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions validator/client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2027,8 +2027,7 @@ func TestValidator_PushSettings(t *testing.T) {
require.Equal(t, len(tt.mockExpectedRequests), len(signedRegisterValidatorRequests))
require.Equal(t, len(signedRegisterValidatorRequests), len(v.signedValidatorRegistrations))
}
deadline := time.Now().Add(time.Duration(params.BeaconConfig().SecondsPerSlot) * time.Second)
if err := v.PushProposerSettings(ctx, km, 0, deadline); tt.err != "" {
if err := v.PushProposerSettings(ctx, km, 0); tt.err != "" {
assert.ErrorContains(t, tt.err, err)
}
if len(tt.logMessages) > 0 {
Expand Down
Loading