Change non-mutating validator accesses to ReadOnly (#5776)

* Change instances of ValidatorAtIndex to ReadOnly where possible

* Use ReadOnly for VerifyExit and Slashings

* Move length check to before lock

* Improve readonly tests

* undo process attester changes

* Fix test

beacon-chain/core/blocks/block_operations.go

@@ -212,7 +212,7 @@ func ProcessBlockHeader(
 
 // VerifyBlockHeaderSignature verifies the proposer signature of a beacon block.
 func VerifyBlockHeaderSignature(beaconState *stateTrie.BeaconState, block *ethpb.SignedBeaconBlock) error {
-	proposer, err := beaconState.ValidatorAtIndex(block.Block.ProposerIndex)
+	proposer, err := beaconState.ValidatorAtIndexReadOnly(block.Block.ProposerIndex)
 	if err != nil {
 		return err
 	}
@@ -222,7 +222,8 @@ func VerifyBlockHeaderSignature(beaconState *stateTrie.BeaconState, block *ethpb
 	if err != nil {
 		return err
 	}
-	return helpers.VerifyBlockSigningRoot(block.Block, proposer.PublicKey, block.Signature, domain)
+	proposerPubKey := proposer.PublicKey()
+	return helpers.VerifyBlockSigningRoot(block.Block, proposerPubKey[:], block.Signature, domain)
 }
 
 // ProcessBlockHeaderNoVerify validates a block by its header but skips proposer
@@ -278,11 +279,11 @@ func ProcessBlockHeaderNoVerify(
 			block.ParentRoot, parentRoot)
 	}
 
-	proposer, err := beaconState.ValidatorAtIndex(idx)
+	proposer, err := beaconState.ValidatorAtIndexReadOnly(idx)
 	if err != nil {
 		return nil, err
 	}
-	if proposer.Slashed {
+	if proposer.Slashed() {
 		return nil, fmt.Errorf("proposer at index %d was previously slashed", idx)
 	}
 
@@ -446,12 +447,12 @@ func VerifyProposerSlashing(
 	if proto.Equal(slashing.Header_1, slashing.Header_2) {
 		return errors.New("expected slashing headers to differ")
 	}
-	proposer, err := beaconState.ValidatorAtIndex(slashing.Header_1.Header.ProposerIndex)
+	proposer, err := beaconState.ValidatorAtIndexReadOnly(slashing.Header_1.Header.ProposerIndex)
 	if err != nil {
 		return err
 	}
-	if !helpers.IsSlashableValidator(proposer, helpers.SlotToEpoch(beaconState.Slot())) {
-		return fmt.Errorf("validator with key %#x is not slashable", proposer.PublicKey)
+	if !helpers.IsSlashableValidatorUsingTrie(proposer, helpers.SlotToEpoch(beaconState.Slot())) {
+		return fmt.Errorf("validator with key %#x is not slashable", proposer.PublicKey())
 	}
 	// Using headerEpoch1 here because both of the headers should have the same epoch.
 	domain, err := helpers.Domain(beaconState.Fork(), helpers.SlotToEpoch(slashing.Header_1.Header.Slot), params.BeaconConfig().DomainBeaconProposer, beaconState.GenesisValidatorRoot())
@@ -460,7 +461,8 @@ func VerifyProposerSlashing(
 	}
 	headers := []*ethpb.SignedBeaconBlockHeader{slashing.Header_1, slashing.Header_2}
 	for _, header := range headers {
-		if err := helpers.VerifySigningRoot(header.Header, proposer.PublicKey, header.Signature, domain); err != nil {
+		proposerPubKey := proposer.PublicKey()
+		if err := helpers.VerifySigningRoot(header.Header, proposerPubKey[:], header.Signature, domain); err != nil {
 			return errors.Wrap(err, "could not verify beacon block header")
 		}
 	}
@@ -1054,7 +1056,7 @@ func ProcessVoluntaryExits(
 				beaconState.NumValidators(),
 			)
 		}
-		val, err := beaconState.ValidatorAtIndex(exit.Exit.ValidatorIndex)
+		val, err := beaconState.ValidatorAtIndexReadOnly(exit.Exit.ValidatorIndex)
 		if err != nil {
 			return nil, err
 		}
@@ -1109,38 +1111,39 @@ func ProcessVoluntaryExitsNoVerify(
 //    # Verify signature
 //    domain = get_domain(state, DOMAIN_VOLUNTARY_EXIT, exit.epoch)
 //    assert bls_verify(validator.pubkey, signing_root(exit), exit.signature, domain)
-func VerifyExit(validator *ethpb.Validator, currentSlot uint64, fork *pb.Fork, signed *ethpb.SignedVoluntaryExit, genesisRoot []byte) error {
+func VerifyExit(validator *stateTrie.ReadOnlyValidator, currentSlot uint64, fork *pb.Fork, signed *ethpb.SignedVoluntaryExit, genesisRoot []byte) error {
 	if signed == nil || signed.Exit == nil {
 		return errors.New("nil exit")
 	}
 
 	exit := signed.Exit
 	currentEpoch := helpers.SlotToEpoch(currentSlot)
 	// Verify the validator is active.
-	if !helpers.IsActiveValidator(validator, currentEpoch) {
+	if !helpers.IsActiveValidatorUsingTrie(validator, currentEpoch) {
 		return errors.New("non-active validator cannot exit")
 	}
 	// Verify the validator has not yet exited.
-	if validator.ExitEpoch != params.BeaconConfig().FarFutureEpoch {
-		return fmt.Errorf("validator has already exited at epoch: %v", validator.ExitEpoch)
+	if validator.ExitEpoch() != params.BeaconConfig().FarFutureEpoch {
+		return fmt.Errorf("validator has already exited at epoch: %v", validator.ExitEpoch())
 	}
 	// Exits must specify an epoch when they become valid; they are not valid before then.
 	if currentEpoch < exit.Epoch {
 		return fmt.Errorf("expected current epoch >= exit epoch, received %d < %d", currentEpoch, exit.Epoch)
 	}
 	// Verify the validator has been active long enough.
-	if currentEpoch < validator.ActivationEpoch+params.BeaconConfig().PersistentCommitteePeriod {
+	if currentEpoch < validator.ActivationEpoch()+params.BeaconConfig().PersistentCommitteePeriod {
 		return fmt.Errorf(
 			"validator has not been active long enough to exit, wanted epoch %d >= %d",
 			currentEpoch,
-			validator.ActivationEpoch+params.BeaconConfig().PersistentCommitteePeriod,
+			validator.ActivationEpoch()+params.BeaconConfig().PersistentCommitteePeriod,
 		)
 	}
 	domain, err := helpers.Domain(fork, exit.Epoch, params.BeaconConfig().DomainVoluntaryExit, genesisRoot)
 	if err != nil {
 		return err
 	}
-	if err := helpers.VerifySigningRoot(exit, validator.PublicKey, signed.Signature, domain); err != nil {
+	valPubKey := validator.PublicKey()
+	if err := helpers.VerifySigningRoot(exit, valPubKey[:], signed.Signature, domain); err != nil {
 		return helpers.ErrSigFailedToVerify
 	}
 	return nil

beacon-chain/core/blocks/block_operations_fuzz_test.go

@@ -410,7 +410,7 @@ func TestFuzzProcessVoluntaryExitsNoVerify_10000(t *testing.T) {
 func TestFuzzVerifyExit_10000(t *testing.T) {
 	fuzzer := fuzz.NewWithSeed(0)
 	ve := &eth.SignedVoluntaryExit{}
-	val := &eth.Validator{}
+	val := &beaconstate.ReadOnlyValidator{}
 	fork := &pb.Fork{}
 	var slot uint64
 

beacon-chain/core/blocks/block_operations_test.go

@@ -549,7 +549,7 @@ func TestProcessProposerSlashings_ValidatorNotSlashable(t *testing.T) {
 	}
 	want := fmt.Sprintf(
 		"validator with key %#x is not slashable",
-		beaconState.Validators()[0].PublicKey,
+		bytesutil.ToBytes48(beaconState.Validators()[0].PublicKey),
 	)
 
 	_, err = blocks.ProcessProposerSlashings(context.Background(), beaconState, block.Body)

beacon-chain/core/epoch/epoch_processing.go

@@ -363,11 +363,11 @@ func unslashedAttestingIndices(state *stateTrie.BeaconState, atts []*pb.PendingA
 	sort.Slice(setIndices, func(i, j int) bool { return setIndices[i] < setIndices[j] })
 	// Remove the slashed validator indices.
 	for i := 0; i < len(setIndices); i++ {
-		v, err := state.ValidatorAtIndex(setIndices[i])
+		v, err := state.ValidatorAtIndexReadOnly(setIndices[i])
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to look up validator")
 		}
-		if v != nil && v.Slashed {
+		if v != nil && v.Slashed() {
 			setIndices = append(setIndices[:i], setIndices[i+1:]...)
 		}
 	}
@@ -391,11 +391,11 @@ func BaseReward(state *stateTrie.BeaconState, index uint64) (uint64, error) {
 	if err != nil {
 		return 0, errors.Wrap(err, "could not calculate active balance")
 	}
-	val, err := state.ValidatorAtIndex(index)
+	val, err := state.ValidatorAtIndexReadOnly(index)
 	if err != nil {
 		return 0, err
 	}
-	effectiveBalance := val.EffectiveBalance
+	effectiveBalance := val.EffectiveBalance()
 	baseReward := effectiveBalance * params.BeaconConfig().BaseRewardFactor /
 		mathutil.IntegerSquareRoot(totalBalance) / params.BeaconConfig().BaseRewardsPerEpoch
 	return baseReward, nil

beacon-chain/core/helpers/validators.go

@@ -42,10 +42,19 @@ func checkValidatorActiveStatus(activationEpoch uint64, exitEpoch uint64, epoch
 //  Check if ``validator`` is slashable.
 //  """
 //  return (not validator.slashed) and (validator.activation_epoch <= epoch < validator.withdrawable_epoch)
-func IsSlashableValidator(validator *ethpb.Validator, epoch uint64) bool {
-	active := validator.ActivationEpoch <= epoch
-	beforeWithdrawable := epoch < validator.WithdrawableEpoch
-	return beforeWithdrawable && active && !validator.Slashed
+func IsSlashableValidator(val *ethpb.Validator, epoch uint64) bool {
+	return checkValidatorSlashable(val.ActivationEpoch, val.WithdrawableEpoch, val.Slashed, epoch)
+}
+
+// IsSlashableValidatorUsingTrie checks if a read only validator is slashable.
+func IsSlashableValidatorUsingTrie(val *stateTrie.ReadOnlyValidator, epoch uint64) bool {
+	return checkValidatorSlashable(val.ActivationEpoch(), val.WithdrawableEpoch(), val.Slashed(), epoch)
+}
+
+func checkValidatorSlashable(activationEpoch uint64, withdrawableEpoch uint64, slashed bool, epoch uint64) bool {
+	active := activationEpoch <= epoch
+	beforeWithdrawable := epoch < withdrawableEpoch
+	return beforeWithdrawable && active && !slashed
 }
 
 // ActiveValidatorIndices filters out active validators based on validator status