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

refactor: validator state management refactor #1274

Merged
merged 52 commits into from
Aug 23, 2024

Conversation

rodrigo-o
Copy link
Collaborator

@rodrigo-o rodrigo-o commented Aug 13, 2024

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.

  • ValidatorSet creation
  • Moving the process of new_heads and ticks from Validators to ValidatorSet
  • Simplified the Validator state as much as possible
  • Move Duties to work on a per-epoch-basis, i.e. every duty is calculated once per epoch
  • ValidatorSet selects only the validators with a duty to accomplish in every slot third, and just them do their work.

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:

  • Subnet Info checks (we are not joining anymore to subnets at the duties calculation but everything continue working, this needs to be investigated further)

…ocks and not with libp2p calling the ValidatorManager
@rodrigo-o rodrigo-o marked this pull request as ready for review August 15, 2024 15:08
@rodrigo-o rodrigo-o requested a review from a team as a code owner August 15, 2024 15:08
Copy link
Collaborator

@Arkenan Arkenan left a 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.

lib/lambda_ethereum_consensus/validator/duties.ex Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/validator.ex Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/validator_set.ex Outdated Show resolved Hide resolved
lib/libp2p_port.ex Show resolved Hide resolved
lib/libp2p_port.ex Show resolved Hide resolved
@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Aug 22, 2024

Here is a quick run showing more than one epoch running in Kurtosis with other 64 validators in another clients:

image

@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Aug 23, 2024

The latest commit fixes an issue regarding the start of the node with no Validators that I spotted while testing the maybe_prefetch_committees addition using the make checkpoint-sync start of the local node pointing to mainnet instead of Kurtosis. Now, when there are no active validators, notify_head and notify_tick in the ValidatorSet are effectively a :noop.

@rodrigo-o rodrigo-o enabled auto-merge (squash) August 23, 2024 22:16
@rodrigo-o rodrigo-o merged commit 9835380 into main Aug 23, 2024
13 checks passed
@rodrigo-o rodrigo-o deleted the validator-state-management-refactor branch August 23, 2024 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Share duty calculations and state between validators
2 participants