Push proposer settings every slot: builder #14155
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rkapka
previously approved these changes
Jul 12, 2024
nisdas
reviewed
Jul 22, 2024
validator/client/runner.go
Outdated
| } | ||
| }() | ||
| // account has changed in the middle of an epoch | ||
| if err := v.PushProposerSettings(ctx, km, slot, time.Now().Add(1*time.Minute)); err != nil { |
There was a problem hiding this comment.
why 1 minute instead of 1 slot ?
james-prysm
commented
Jul 22, 2024
| return errors.Wrap(ErrBuilderValidatorRegistration, err.Error()) | ||
| if len(signedRegReqs) > 0 { | ||
| go func() { | ||
| if err := SubmitValidatorRegistrations(ctx, v.validatorClient, signedRegReqs, v.validatorsRegBatchSize); err != nil { |
There was a problem hiding this comment.
this will cancel at the end of the slot, for some groups like dvts should we provide much longer deadlines?
There was a problem hiding this comment.
Our code should assume that it is a single logical validator underneath. Also this simply sends it out to multiple relays which is unrelated to dvts.
nisdas
approved these changes
Jul 30, 2024
james-prysm
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Other
What does this PR do? Why is it needed?
This is a temporary fix/improvement to how often push proposer settings get called until a rework of health tracking and account changes is implemented.
currently, the push proposer settings API is called at the start of every epoch and updates the proposer cache on the beacon node. This may not be ideal as sometimes validators get added in the middle of the epoch and wouldn't correctly account for proposals afterward within the same epoch. Other cases include beacon node upgrades or migrations requiring load balancing, the proposer cache and such may not be updated at the start of the switchover.
Below are some benchmarks for several hundred keys. The majority of the processing time is collecting the current status of keys, and not the API calls themselves.
currently, I believe lighthouse at the very least does something similar.
testing on holesky with about 5k keys yielded the following results-- results have been updated --
with an updated call things changed to 52ms on holesky, 5k keys signed
Which issues(s) does this PR fix?
Fixes #14179
Other notes for review