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

Fix validator assignments on slot 0 #4682

Merged
merged 7 commits into from Jan 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion shared/slotutil/slotticker.go
Expand Up @@ -64,7 +64,7 @@ func (s *SlotTicker) start(

var nextTickTime time.Time
var slot uint64
if sinceGenesis < 0 {
if sinceGenesis < d {
// Handle when the current time is before the genesis time.
nextTickTime = genesisTime
slot = 0
Expand Down
9 changes: 8 additions & 1 deletion shared/slotutil/slotticker_test.go
Expand Up @@ -43,11 +43,18 @@ func TestSlotTicker(t *testing.T) {
// Tick once.
tick <- time.Now()
slot := <-ticker.C()
if slot != 0 {
t.Fatalf("Expected %d, got %d", 0, slot)
}

// Tick twice.
tick <- time.Now()
slot = <-ticker.C()
if slot != 1 {
t.Fatalf("Expected %d, got %d", 1, slot)
}

// Tick twice.
// Tick thrice.
tick <- time.Now()
slot = <-ticker.C()
if slot != 2 {
Expand Down
1 change: 0 additions & 1 deletion validator/client/runner.go
Expand Up @@ -93,7 +93,6 @@ func run(ctx context.Context, v Validator) {
for id, roles := range allRoles {
wg.Add(1)
go func(roles []pb.ValidatorRole, id [48]byte) {

for _, role := range roles {
switch role {
case pb.ValidatorRole_ATTESTER:
Expand Down
2 changes: 1 addition & 1 deletion validator/client/validator.go
Expand Up @@ -286,7 +286,7 @@ func (v *validator) RolesAt(ctx context.Context, slot uint64) (map[[48]byte][]pb
if duty == nil {
continue
}
if duty.ProposerSlot == slot {
if duty.ProposerSlot > 0 && duty.ProposerSlot == slot {
roles = append(roles, pb.ValidatorRole_PROPOSER)
}
if duty.AttesterSlot == slot {
Expand Down
2 changes: 1 addition & 1 deletion validator/client/validator_propose.go
Expand Up @@ -24,7 +24,7 @@ import (
// sent back to the beacon node for broadcasting.
func (v *validator) ProposeBlock(ctx context.Context, slot uint64, pubKey [48]byte) {
if slot == 0 {
log.Info("Assigned to genesis slot, skipping proposal")
log.Debug("Assigned to genesis slot, skipping proposal")
return
}
ctx, span := trace.StartSpan(ctx, "validator.ProposeBlock")
Expand Down
53 changes: 53 additions & 0 deletions validator/client/validator_test.go
Expand Up @@ -632,3 +632,56 @@ func TestRolesAt_OK(t *testing.T) {
t.Errorf("Unexpected validator role. want: ValidatorRole_AGGREGATOR")
}
}

func TestRolesAt_DoesNotAssignProposer_Slot0(t *testing.T) {
v, m, finish := setup(t)
defer finish()

sks := make([]*bls.SecretKey, 3)
sks[0] = bls.RandKey()
sks[1] = bls.RandKey()
sks[2] = bls.RandKey()
v.keyManager = keymanager.NewDirect(sks)
v.duties = &ethpb.DutiesResponse{
Duties: []*ethpb.DutiesResponse_Duty{
{
CommitteeIndex: 1,
AttesterSlot: 0,
ProposerSlot: 0,
PublicKey: sks[0].PublicKey().Marshal(),
},
{
CommitteeIndex: 2,
AttesterSlot: 4,
ProposerSlot: 0,
PublicKey: sks[1].PublicKey().Marshal(),
},
{
CommitteeIndex: 1,
AttesterSlot: 3,
ProposerSlot: 0,
PublicKey: sks[2].PublicKey().Marshal(),
},
},
}

m.validatorClient.EXPECT().DomainData(
gomock.Any(), // ctx
gomock.Any(), // epoch
).Return(&ethpb.DomainResponse{}, nil /*err*/)

roleMap, err := v.RolesAt(context.Background(), 0)
if err != nil {
t.Fatal(err)
}

if roleMap[bytesutil.ToBytes48(sks[0].PublicKey().Marshal())][0] != pb.ValidatorRole_ATTESTER {
t.Errorf("Unexpected validator role. want: ValidatorRole_PROPOSER")
}
if roleMap[bytesutil.ToBytes48(sks[1].PublicKey().Marshal())][0] != pb.ValidatorRole_UNKNOWN {
t.Errorf("Unexpected validator role. want: ValidatorRole_ATTESTER")
}
if roleMap[bytesutil.ToBytes48(sks[2].PublicKey().Marshal())][0] != pb.ValidatorRole_UNKNOWN {
t.Errorf("Unexpected validator role. want: UNKNOWN")
}
}