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

Fetch Initial Assignments at Genesis Slot Post ChainStart #1574

Merged
merged 6 commits into from
Feb 13, 2019

Conversation

rauljordan
Copy link
Contributor

This is part of #1565


Description

Write why you are making the changes in this pull request

Our current validator code does not fetch initial assignments soon after the chain has started to begin its responsibilities. We need this in order for validators to work correctly.

Write a summary of the changes you are making

This PR modifies the update assignments function in the validator client to check to only perform an update if either it is an epoch start or is assignments are nil.

@codecov
Copy link

codecov bot commented Feb 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@f707f14). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1574   +/-   ##
=========================================
  Coverage          ?   71.88%           
=========================================
  Files             ?       98           
  Lines             ?     6673           
  Branches          ?        0           
=========================================
  Hits              ?     4797           
  Misses            ?     1468           
  Partials          ?      408

Copy link
Member

@nisdas nisdas left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -105,8 +105,8 @@ func (v *validator) UpdateAssignments(ctx context.Context, slot uint64) error {
span, ctx := opentracing.StartSpanFromContext(ctx, "validator.UpdateAssignments")
defer span.Finish()

if slot%params.BeaconConfig().EpochLength != 0 {
// Do nothing if not epoch start.
if slot%params.BeaconConfig().EpochLength != 0 && v.assignment != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This should be ||, why wait until an epoch start if you don't have your assignments yet?

Copy link
Member

Choose a reason for hiding this comment

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

Resolved offline, this is the exit condition and I misunderstood

Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

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

One comment

@rauljordan rauljordan merged commit b1f1dff into master Feb 13, 2019
@rauljordan rauljordan deleted the initial-assign branch February 13, 2019 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants