Skip to content

Commit

Permalink
Fix validator assignments on slot 0 (#4682)
Browse files Browse the repository at this point in the history
* Fix validator acting upon first slot
* Change log to debug
* Fix roles at slot 0
* Merge branch 'master' of https://github.com/prysmaticlabs/Prysm into fix-val
* Add regression test
* Formatting
* Add slot ticker regression test
  • Loading branch information
0xKiwi committed Jan 29, 2020
1 parent a22c977 commit 3e9bf58
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 5 deletions.
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")
}
}

0 comments on commit 3e9bf58

Please sign in to comment.