Avoid Panic Retrieving Validator Public Key (#2566)

* exclusive of finalized block

* fixed saveValidatorIdx to skip validator not in state

* fixed test

* tests

* comment

* comment

* fixed test

* comment

beacon-chain/blockchain/block_processing.go

@@ -323,13 +323,24 @@ func (c *ChainService) runStateTransition(
 // validators were activated from current epoch. After it saves, current epoch key
 // is deleted from ActivatedValidators mapping.
 func (c *ChainService) saveValidatorIdx(state *pb.BeaconState) error {
-	activatedValidators := validators.ActivatedValFromEpoch(helpers.CurrentEpoch(state) + 1)
+	nextEpoch := helpers.CurrentEpoch(state) + 1
+	activatedValidators := validators.ActivatedValFromEpoch(nextEpoch)
+	var idxNotInState []uint64
 	for _, idx := range activatedValidators {
+		// If for some reason the activated validator indices is not in state,
+		// we skip them and save them to process for next epoch.
+		if int(idx) >= len(state.ValidatorRegistry) {
+			idxNotInState = append(idxNotInState, idx)
+			continue
+		}
 		pubKey := state.ValidatorRegistry[idx].Pubkey
 		if err := c.beaconDB.SaveValidatorIndex(pubKey, int(idx)); err != nil {
 			return fmt.Errorf("could not save validator index: %v", err)
 		}
 	}
+	// Since we are processing next epoch, save the can't processed validator indices
+	// to the epoch after that.
+	validators.InsertActivatedIndices(nextEpoch+1, idxNotInState)
 	validators.DeleteActivatedVal(helpers.CurrentEpoch(state))
 	return nil
 }

beacon-chain/blockchain/block_processing_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/binary"
 	"math/big"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -773,7 +774,7 @@ func TestDeleteValidatorIdx_DeleteWorks(t *testing.T) {
 	db := internal.SetupDB(t)
 	defer internal.TeardownDB(t, db)
 	epoch := uint64(2)
-	v.InsertActivatedVal(epoch+1, []uint64{0, 1, 2})
+	v.InsertActivatedIndices(epoch+1, []uint64{0, 1, 2})
 	v.InsertExitedVal(epoch+1, []uint64{0, 2})
 	var validators []*pb.Validator
 	for i := 0; i < 3; i++ {
@@ -816,7 +817,7 @@ func TestSaveValidatorIdx_SaveRetrieveWorks(t *testing.T) {
 	db := internal.SetupDB(t)
 	defer internal.TeardownDB(t, db)
 	epoch := uint64(1)
-	v.InsertActivatedVal(epoch+1, []uint64{0, 1, 2})
+	v.InsertActivatedIndices(epoch+1, []uint64{0, 1, 2})
 	var validators []*pb.Validator
 	for i := 0; i < 3; i++ {
 		pubKeyBuf := make([]byte, params.BeaconConfig().BLSPubkeyLength)
@@ -847,3 +848,46 @@ func TestSaveValidatorIdx_SaveRetrieveWorks(t *testing.T) {
 		t.Errorf("Activated validators mapping for epoch %d still there", epoch)
 	}
 }
+
+func TestSaveValidatorIdx_IdxNotInState(t *testing.T) {
+	db := internal.SetupDB(t)
+	defer internal.TeardownDB(t, db)
+	epoch := uint64(100)
+
+	// Tried to insert 5 active indices to DB with only 3 validators in state.
+	v.InsertActivatedIndices(epoch+1, []uint64{0, 1, 2, 3, 4})
+	var validators []*pb.Validator
+	for i := 0; i < 3; i++ {
+		pubKeyBuf := make([]byte, params.BeaconConfig().BLSPubkeyLength)
+		binary.PutUvarint(pubKeyBuf, uint64(i))
+		validators = append(validators, &pb.Validator{
+			Pubkey: pubKeyBuf,
+		})
+	}
+	state := &pb.BeaconState{
+		ValidatorRegistry: validators,
+		Slot:              epoch * params.BeaconConfig().SlotsPerEpoch,
+	}
+	chainService := setupBeaconChain(t, db, nil)
+	if err := chainService.saveValidatorIdx(state); err != nil {
+		t.Fatalf("Could not save validator idx: %v", err)
+	}
+
+	wantedIdx := uint64(2)
+	idx, err := chainService.beaconDB.ValidatorIndex(validators[wantedIdx].Pubkey)
+	if err != nil {
+		t.Fatalf("Could not get validator index: %v", err)
+	}
+	if wantedIdx != idx {
+		t.Errorf("Wanted: %d, got: %d", wantedIdx, idx)
+	}
+
+	if v.ActivatedValFromEpoch(epoch) != nil {
+		t.Errorf("Activated validators mapping for epoch %d still there", epoch)
+	}
+
+	// Verify the skipped validators are included in the next epoch.
+	if !reflect.DeepEqual(v.ActivatedValFromEpoch(epoch+2), []uint64{3, 4}) {
+		t.Error("Did not get wanted validator from activation queue")
+	}
+}

beacon-chain/core/validators/validator.go

@@ -455,16 +455,14 @@ func InitializeValidatorStore(bState *pb.BeaconState) {
 	activeValidatorIndices := helpers.ActiveValidatorIndices(
 		bState.ValidatorRegistry, currentEpoch)
 	vStore.activatedValidators[currentEpoch] = activeValidatorIndices
-
 }
 
-// InsertActivatedVal locks the validator store, inserts the activated validator
-// indices, then unlocks the store again. This method may be used by
-// external services in testing to populate the validator store.
-func InsertActivatedVal(epoch uint64, validators []uint64) {
+// InsertActivatedIndices locks the validator store, inserts the activated validator
+// indices corresponding to their activation epochs.
+func InsertActivatedIndices(epoch uint64, indices []uint64) {
 	vStore.Lock()
 	defer vStore.Unlock()
-	vStore.activatedValidators[epoch] = validators
+	vStore.activatedValidators[epoch] = append(vStore.activatedValidators[epoch], indices...)
 }
 
 // InsertExitedVal locks the validator store, inserts the exited validator

beacon-chain/core/validators/validator_test.go

@@ -671,3 +671,14 @@ func TestInitializeValidatoreStore(t *testing.T) {
 		t.Errorf("Saved active indices are not the same as the one in the validator store, got %v but expected %v", retrievedIndices, indices)
 	}
 }
+
+func TestInsertActivatedIndices_Works(t *testing.T) {
+	InsertActivatedIndices(100, []uint64{1, 2, 3})
+	if !reflect.DeepEqual(vStore.activatedValidators[100], []uint64{1, 2, 3}) {
+		t.Error("Activated validators aren't the same")
+	}
+	InsertActivatedIndices(100, []uint64{100})
+	if !reflect.DeepEqual(vStore.activatedValidators[100], []uint64{1, 2, 3, 100}) {
+		t.Error("Activated validators aren't the same")
+	}
+}