-
Notifications
You must be signed in to change notification settings - Fork 34
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
refactor: validator state management refactor #1274
Conversation
…ocks and not with libp2p calling the ValidatorManager
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR rodri, left a bunch of comments. None of them are blocking, feel free to merge as is. Most of them are nits. The only one that I'm doubtful about is the setting of false instead of true.
The latest commit fixes an issue regarding the start of the node with no Validators that I spotted while testing the |
Motivation
After the latest refactors regarding Genserver removal, the state management of validators became more complex than it needed, before it made sense because of everyone beign and independen Genserver, but now most of the state could be shared by a
ValidatorSet
, this PR deals with that.Description
This PR introduces a new
ValidatorSet
module that manage Validators and Duties. It process both new_head notifications as well as ticks throughout the slot lifecycle and select just the validators that need to be involved in every step of the process. The logic is maintained as it were previously implemented, this PR aims to reduce complexity, duplication and enhance clarity but it doesn't go in depth in the current logic, it tries to reuse current functions when available and simplify the ones that are intuitive to do so.Note: The long commit history is because this branch started out of
validator-manager-genserver-removal
(#1224) before merging it to main.Resolves #1255
NEXT STEPS: