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.

Sign up for GitHub

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

Jump to bottom

Refactor validator client #2416

Merged
merged 24 commits into from
Apr 26, 2021
Merged

Refactor validator client #2416

merged 24 commits into from
Apr 26, 2021

Conversation

dapplion
Copy link
Contributor

@dapplion dapplion commented Apr 24, 2021 •
edited
Loading

Motivation

Our validator client worked but was sub-optimal. This PR attempts to solve existing issues from their root, and pave the way for future optimizations

Description

Known fixed issues:

  • Handles pre-genesis properly. Will fetch both proposer and attester duties before genesis. Is able to attest block 0 and propose block 1.
  • Fix bug where the validator will consistently not propose on the first block of the epoch. Closes Validators miss proposing blocks at epoch transition #2380

Improvements:

  • Batch all operations possible. Create a single attestation and aggregate per committee, then sign by everyone and send once.
  • Re-fetch block duties on slot to handle re-orgs safely.
  • Fetch fork only once per epoch and cache the result for future epochs in case of failure.
  • Fetch validator indices in parallel to attester duties. Will refetch duties if it finds new indices.
  • Throttle operations so if an AttesterService slot cycle takes too long, skips the next. This prevents overloading the node continuously.

dapplion added 23 commits April 23, 2021 17:39
@dapplion
Simplify validator API client
9283775 
Simplify AttestationService validator flow

Compute wait times with better precision

Simplify BlockProposingService validator flow

Re-export all util in index

Revert private property accessing

Follow Lighthouse approach for the validator client

Add validatorStore and fork service

Remove wait for block on att service

Reorg duties services

Add dependentRoot functionality

Block service

Clean-up services

Simplify start / stop methods

Remove clock from apiClient

Clean-up apiClient class

Update validator clock tests

Update duties tests

Add more duties tests

Add tests for new validator services

Update validator initialization

Update lodestar tests

Temp patch to pass tests

Fix differenceHex util function

Re-org validator API client

Make validator REST client requests abortable

Log validator indexes in sim tests

Test fork service

Remove runningTasks booleans

Don't log ErrorAborted in validator client

Log time to wait in sim test
@dapplion
Review logic and comments
633a9c8
@dapplion
Use hasVotingPubkey util
d2a4e5b
@dapplion
Review AttestationDutiesService
9ad64b4
@dapplion
Review BlockDutiesService
3e3f8d1
@dapplion
Review ValidatorStore
8541244
@dapplion
Better logging in tests
289d5f2
@dapplion
Update validator unit tests
ad9d19c
@dapplion
Don't fetch duties for slot, epoch < 0
44fab2f
@dapplion
Update sim test timeouts and params
1899c46
@dapplion
Wait a max of SLOT_DIFF for requested slot in validator API
c4310ed
@dapplion
Prioritize the most valuable aggregates
87a2191
@dapplion
Sim test multi-thread, connect nodes before starting validators
e202c07
@dapplion
Don't wait for genesisTime on validator
960b366
@dapplion
Improve handling of genesis duties
5f8f6a5
@dapplion
Fix BlockDuties pre-genesis logic
fdc9a46
@dapplion
Improve error messages in epochContext duties methods
13ac7d7
@dapplion
Consider the node synced before or close to genesis
6127dbb
@dapplion
Improve notWhileSyncing function
dcb0345
@dapplion
Log to debug each time a validator signs and attestation and aggregate
6355e86
@dapplion
In sim test report rendering log undefined as -
fa8d830
@dapplion
Use MAX_API_CLOCK_DISPARITY_MS in validator API
5532e94
@dapplion
Update unit tests
edd29d9
@codeclimate
Copy link

codeclimate bot commented Apr 24, 2021 •
edited
Loading

Code Climate has analyzed commit de90ae1 and detected 8 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 8

View more on Code Climate.

@dapplion dapplion mentioned this pull request Apr 24, 2021
Merged
3xtr4t3rr3str14l
3xtr4t3rr3str14l reviewed Apr 26, 2021
View reviewed changes
Copy link
Contributor

@3xtr4t3rr3str14l 3xtr4t3rr3str14l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a big'un! i only reviewed the packages/lodestar/* half of it (except for the commented function in packages/validator, but will try to look at the validator half later.

packages/lodestar/src/api/impl/beacon/blocks/index.ts Show resolved Hide resolved
packages/lodestar/src/api/impl/beacon/pool/pool.ts Show resolved Hide resolved
packages/lodestar/src/api/impl/validator/validator.ts Show resolved Hide resolved
packages/lodestar/test/sim/singleNodeSingleThread.test.ts Show resolved Hide resolved
packages/validator/src/util/clock.ts Show resolved Hide resolved
@wemeetagain wemeetagain mentioned this pull request Apr 26, 2021
Closed
@wemeetagain wemeetagain merged commit 84a03e0 into master Apr 26, 2021
@wemeetagain wemeetagain deleted the dapplion/review-vc-2 branch April 26, 2021 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@3xtr4t3rr3str14l 3xtr4t3rr3str14l 3xtr4t3rr3str14l left review comments

@wemeetagain wemeetagain wemeetagain approved these changes

@mpetrunic mpetrunic Awaiting requested review from mpetrunic

@twoeths twoeths Awaiting requested review from twoeths

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Validators miss proposing blocks at epoch transition
3 participants
@dapplion @wemeetagain @3xtr4t3rr3str14l