From 3e9bf58d81a986881de9d2d0fcbe565b3fe83ef2 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Tue, 28 Jan 2020 23:30:30 -0500 Subject: [PATCH] Fix validator assignments on slot 0 (#4682) * 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 --- shared/slotutil/slotticker.go | 2 +- shared/slotutil/slotticker_test.go | 9 ++++- validator/client/runner.go | 1 - validator/client/validator.go | 2 +- validator/client/validator_propose.go | 2 +- validator/client/validator_test.go | 53 +++++++++++++++++++++++++++ 6 files changed, 64 insertions(+), 5 deletions(-) diff --git a/shared/slotutil/slotticker.go b/shared/slotutil/slotticker.go index 1cee3f53074..70d7427e83d 100644 --- a/shared/slotutil/slotticker.go +++ b/shared/slotutil/slotticker.go @@ -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 diff --git a/shared/slotutil/slotticker_test.go b/shared/slotutil/slotticker_test.go index 8eabef00877..63d368f18dd 100644 --- a/shared/slotutil/slotticker_test.go +++ b/shared/slotutil/slotticker_test.go @@ -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 { diff --git a/validator/client/runner.go b/validator/client/runner.go index 3b5623a562c..62a673bd983 100644 --- a/validator/client/runner.go +++ b/validator/client/runner.go @@ -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: diff --git a/validator/client/validator.go b/validator/client/validator.go index 708fd29ff98..7d6f828f410 100644 --- a/validator/client/validator.go +++ b/validator/client/validator.go @@ -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 { diff --git a/validator/client/validator_propose.go b/validator/client/validator_propose.go index 010383a0034..ebb06a86d1a 100644 --- a/validator/client/validator_propose.go +++ b/validator/client/validator_propose.go @@ -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") diff --git a/validator/client/validator_test.go b/validator/client/validator_test.go index e2af5416418..5f3445c652a 100644 --- a/validator/client/validator_test.go +++ b/validator/client/validator_test.go @@ -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 = ðpb.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(ðpb.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") + } +}