Skip to content

Commit

Permalink
Pass By Value Instead Of Reference (#7710)
Browse files Browse the repository at this point in the history
* change to value from reference

* fix up

* make it a pointer

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
  • Loading branch information
nisdas and rauljordan committed Nov 9, 2020
1 parent 1c5d533 commit 5995d23
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 54 deletions.
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/head.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func (s *Service) cacheJustifiedStateBalances(ctx context.Context, justifiedRoot
epoch := helpers.CurrentEpoch(justifiedState)

justifiedBalances := make([]uint64, justifiedState.NumValidators())
if err := justifiedState.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if err := justifiedState.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if helpers.IsActiveValidatorUsingTrie(val, epoch) {
justifiedBalances[idx] = val.EffectiveBalance()
} else {
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/blocks/attester_slashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func ProcessAttesterSlashings(
currentEpoch := helpers.SlotToEpoch(beaconState.Slot())
var err error
var slashedAny bool
var val *stateTrie.ReadOnlyValidator
var val stateTrie.ReadOnlyValidator
for _, validatorIndex := range slashableIndices {
val, err = beaconState.ValidatorAtIndexReadOnly(validatorIndex)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/blocks/block_operations_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,13 +429,13 @@ func TestFuzzProcessVoluntaryExitsNoVerify_10000(t *testing.T) {
func TestFuzzVerifyExit_10000(t *testing.T) {
fuzzer := fuzz.NewWithSeed(0)
ve := &eth.SignedVoluntaryExit{}
val := &stateTrie.ReadOnlyValidator{}
val := stateTrie.ReadOnlyValidator{}
fork := &pb.Fork{}
var slot uint64

for i := 0; i < 10000; i++ {
fuzzer.Fuzz(ve)
fuzzer.Fuzz(val)
fuzzer.Fuzz(&val)
fuzzer.Fuzz(fork)
fuzzer.Fuzz(&slot)
err := VerifyExitAndSignature(val, slot, fork, ve, params.BeaconConfig().ZeroHash[:])
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/blocks/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func ProcessVoluntaryExitsNoVerifySignature(
// # Verify signature
// domain = get_domain(state, DOMAIN_VOLUNTARY_EXIT, exit.epoch)
// assert bls_verify(validator.pubkey, signing_root(exit), exit.signature, domain)
func VerifyExitAndSignature(validator *stateTrie.ReadOnlyValidator, currentSlot uint64, fork *pb.Fork, signed *ethpb.SignedVoluntaryExit, genesisRoot []byte) error {
func VerifyExitAndSignature(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")
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func VerifyExitAndSignature(validator *stateTrie.ReadOnlyValidator, currentSlot
// assert get_current_epoch(state) >= exit.epoch
// # Verify the validator has been active long enough
// assert get_current_epoch(state) >= validator.activation_epoch + SHARD_COMMITTEE_PERIOD
func verifyExitConditions(validator *stateTrie.ReadOnlyValidator, currentSlot uint64, exit *ethpb.VoluntaryExit) error {
func verifyExitConditions(validator stateTrie.ReadOnlyValidator, currentSlot uint64, exit *ethpb.VoluntaryExit) error {
currentEpoch := helpers.SlotToEpoch(currentSlot)
// Verify the validator is active.
if !helpers.IsActiveValidatorUsingTrie(validator, currentEpoch) {
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/epoch/epoch_processing.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ func UnslashedAttestingIndices(state *stateTrie.BeaconState, atts []*pb.PendingA
if err != nil {
return nil, errors.Wrap(err, "failed to look up validator")
}
if v != nil && v.Slashed() {
if !v.IsNil() && v.Slashed() {
setIndices = append(setIndices[:i], setIndices[i+1:]...)
}
}
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/epoch/precompute/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func New(ctx context.Context, state *stateTrie.BeaconState) ([]*Validator, *Bala
currentEpoch := helpers.CurrentEpoch(state)
prevEpoch := helpers.PrevEpoch(state)

if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if err := state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
// Was validator withdrawable or slashed
withdrawable := prevEpoch+1 >= val.WithdrawableEpoch()
pVal := &Validator{
Expand Down
7 changes: 1 addition & 6 deletions beacon-chain/core/epoch/precompute/slashing.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package precompute

import (
"errors"

ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
Expand All @@ -28,10 +26,7 @@ func ProcessSlashingsPrecompute(state *stateTrie.BeaconState, pBal *Balance) err

var hasSlashing bool
// Iterate through validator list in state, stop until a validator satisfies slashing condition of current epoch.
err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if val == nil {
return errors.New("nil validator in state")
}
err := state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
correctEpoch := epochToWithdraw == val.WithdrawableEpoch()
if val.Slashed() && correctEpoch {
hasSlashing = true
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/helpers/committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func ShuffledIndices(state *stateTrie.BeaconState, epoch uint64) ([]uint64, erro
}

indices := make([]uint64, 0, state.NumValidators())
if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if err := state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if IsActiveValidatorUsingTrie(val, epoch) {
indices = append(indices, uint64(idx))
}
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/helpers/rewards_penalties.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TotalBalance(state *stateTrie.BeaconState, indices []uint64) uint64 {
// return get_total_balance(state, set(get_active_validator_indices(state, get_current_epoch(state))))
func TotalActiveBalance(state *stateTrie.BeaconState) (uint64, error) {
total := uint64(0)
if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if err := state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if IsActiveValidatorUsingTrie(val, SlotToEpoch(state.Slot())) {
total += val.EffectiveBalance()
}
Expand Down
12 changes: 6 additions & 6 deletions beacon-chain/core/helpers/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func IsActiveValidator(validator *ethpb.Validator, epoch uint64) bool {
}

// IsActiveValidatorUsingTrie checks if a read only validator is active.
func IsActiveValidatorUsingTrie(validator *stateTrie.ReadOnlyValidator, epoch uint64) bool {
func IsActiveValidatorUsingTrie(validator stateTrie.ReadOnlyValidator, epoch uint64) bool {
return checkValidatorActiveStatus(validator.ActivationEpoch(), validator.ExitEpoch(), epoch)
}

Expand All @@ -49,7 +49,7 @@ func IsSlashableValidator(activationEpoch, withdrawableEpoch uint64, slashed boo
}

// IsSlashableValidatorUsingTrie checks if a read only validator is slashable.
func IsSlashableValidatorUsingTrie(val *stateTrie.ReadOnlyValidator, epoch uint64) bool {
func IsSlashableValidatorUsingTrie(val stateTrie.ReadOnlyValidator, epoch uint64) bool {
return checkValidatorSlashable(val.ActivationEpoch(), val.WithdrawableEpoch(), val.Slashed(), epoch)
}

Expand Down Expand Up @@ -85,7 +85,7 @@ func ActiveValidatorIndices(state *stateTrie.BeaconState, epoch uint64) ([]uint6
return activeIndices, nil
}
var indices []uint64
if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if err := state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if IsActiveValidatorUsingTrie(val, epoch) {
indices = append(indices, uint64(idx))
}
Expand Down Expand Up @@ -117,7 +117,7 @@ func ActiveValidatorCount(state *stateTrie.BeaconState, epoch uint64) (uint64, e
}

count := uint64(0)
if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
if err := state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if IsActiveValidatorUsingTrie(val, epoch) {
count++
}
Expand Down Expand Up @@ -319,7 +319,7 @@ func IsEligibleForActivationQueue(validator *ethpb.Validator) bool {

// IsEligibleForActivationQueueUsingTrie checks if the read-only validator is eligible to
// be placed into the activation queue.
func IsEligibleForActivationQueueUsingTrie(validator *stateTrie.ReadOnlyValidator) bool {
func IsEligibleForActivationQueueUsingTrie(validator stateTrie.ReadOnlyValidator) bool {
return isEligibileForActivationQueue(validator.ActivationEligibilityEpoch(), validator.EffectiveBalance())
}

Expand Down Expand Up @@ -348,7 +348,7 @@ func IsEligibleForActivation(state *stateTrie.BeaconState, validator *ethpb.Vali
}

// IsEligibleForActivationUsingTrie checks if the validator is eligible for activation.
func IsEligibleForActivationUsingTrie(state *stateTrie.BeaconState, validator *stateTrie.ReadOnlyValidator) bool {
func IsEligibleForActivationUsingTrie(state *stateTrie.BeaconState, validator stateTrie.ReadOnlyValidator) bool {
cpt := state.FinalizedCheckpoint()
if cpt == nil {
return false
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/validators/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func InitiateValidatorExit(state *stateTrie.BeaconState, idx uint64) (*stateTrie
return state, nil
}
var exitEpochs []uint64
err = state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
err = state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if val.ExitEpoch() != params.BeaconConfig().FarFutureEpoch {
exitEpochs = append(exitEpochs, val.ExitEpoch())
}
Expand All @@ -66,7 +66,7 @@ func InitiateValidatorExit(state *stateTrie.BeaconState, idx uint64) (*stateTrie

// We use the exit queue churn to determine if we have passed a churn limit.
exitQueueChurn := uint64(0)
err = state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error {
err = state.ReadFromEveryValidator(func(idx int, val stateTrie.ReadOnlyValidator) error {
if val.ExitEpoch() == exitQueueEpoch {
exitQueueChurn++
}
Expand Down
10 changes: 5 additions & 5 deletions beacon-chain/rpc/beacon/validators_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func (is *infostream) generateValidatorsInfo(pubKeys [][]byte) ([]*ethpb.Validat
}

// generateValidatorInfo generates the validator info for a public key.
func (is *infostream) generateValidatorInfo(pubKey []byte, validator *state.ReadOnlyValidator, headState *state.BeaconState, epoch uint64) (*ethpb.ValidatorInfo, error) {
func (is *infostream) generateValidatorInfo(pubKey []byte, validator state.ReadOnlyValidator, headState *state.BeaconState, epoch uint64) (*ethpb.ValidatorInfo, error) {
info := &ethpb.ValidatorInfo{
PublicKey: pubKey,
Epoch: epoch,
Expand Down Expand Up @@ -384,8 +384,8 @@ func (is *infostream) calculateActivationTimeForPendingValidators(res []*ethpb.V
numAttestingValidators := uint64(0)
pendingValidators := make([]uint64, 0, headState.NumValidators())

err := headState.ReadFromEveryValidator(func(idx int, val *state.ReadOnlyValidator) error {
if val == nil {
err := headState.ReadFromEveryValidator(func(idx int, val state.ReadOnlyValidator) error {
if val.IsNil() {
return errors.New("nil validator in state")
}
if helpers.IsEligibleForActivationUsingTrie(headState, val) {
Expand Down Expand Up @@ -470,10 +470,10 @@ func (s indicesSorter) Less(i, j int) bool {
return s.indices[i] < s.indices[j]
}

func (is *infostream) calculateStatusAndTransition(validator *state.ReadOnlyValidator, currentEpoch uint64) (ethpb.ValidatorStatus, uint64) {
func (is *infostream) calculateStatusAndTransition(validator state.ReadOnlyValidator, currentEpoch uint64) (ethpb.ValidatorStatus, uint64) {
farFutureEpoch := params.BeaconConfig().FarFutureEpoch

if validator == nil {
if validator.IsNil() {
return ethpb.ValidatorStatus_UNKNOWN_STATUS, 0
}

Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/rpc/validator/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func assignmentStatus(beaconState *stateTrie.BeaconState, validatorIdx uint64) e
farFutureEpoch := params.BeaconConfig().FarFutureEpoch
validatorBalance := validator.EffectiveBalance()

if validator == nil {
if validator.IsNil() {
return ethpb.ValidatorStatus_UNKNOWN_STATUS
}
if currentEpoch < validator.ActivationEligibilityEpoch() {
Expand Down
53 changes: 29 additions & 24 deletions beacon-chain/state/getters.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,53 +14,53 @@ import (

// EffectiveBalance returns the effective balance of the
// read only validator.
func (v *ReadOnlyValidator) EffectiveBalance() uint64 {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) EffectiveBalance() uint64 {
if v.IsNil() {
return 0
}
return v.validator.EffectiveBalance
}

// ActivationEligibilityEpoch returns the activation eligibility epoch of the
// read only validator.
func (v *ReadOnlyValidator) ActivationEligibilityEpoch() uint64 {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) ActivationEligibilityEpoch() uint64 {
if v.IsNil() {
return 0
}
return v.validator.ActivationEligibilityEpoch
}

// ActivationEpoch returns the activation epoch of the
// read only validator.
func (v *ReadOnlyValidator) ActivationEpoch() uint64 {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) ActivationEpoch() uint64 {
if v.IsNil() {
return 0
}
return v.validator.ActivationEpoch
}

// WithdrawableEpoch returns the withdrawable epoch of the
// read only validator.
func (v *ReadOnlyValidator) WithdrawableEpoch() uint64 {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) WithdrawableEpoch() uint64 {
if v.IsNil() {
return 0
}
return v.validator.WithdrawableEpoch
}

// ExitEpoch returns the exit epoch of the
// read only validator.
func (v *ReadOnlyValidator) ExitEpoch() uint64 {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) ExitEpoch() uint64 {
if v.IsNil() {
return 0
}
return v.validator.ExitEpoch
}

// PublicKey returns the public key of the
// read only validator.
func (v *ReadOnlyValidator) PublicKey() [48]byte {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) PublicKey() [48]byte {
if v.IsNil() {
return [48]byte{}
}
var pubkey [48]byte
Expand All @@ -70,28 +70,33 @@ func (v *ReadOnlyValidator) PublicKey() [48]byte {

// WithdrawalCredentials returns the withdrawal credentials of the
// read only validator.
func (v *ReadOnlyValidator) WithdrawalCredentials() []byte {
func (v ReadOnlyValidator) WithdrawalCredentials() []byte {
creds := make([]byte, len(v.validator.WithdrawalCredentials))
copy(creds, v.validator.WithdrawalCredentials)
return creds
}

// Slashed returns the read only validator is slashed.
func (v *ReadOnlyValidator) Slashed() bool {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) Slashed() bool {
if v.IsNil() {
return false
}
return v.validator.Slashed
}

// CopyValidator returns the copy of the read only validator.
func (v *ReadOnlyValidator) CopyValidator() *ethpb.Validator {
if v == nil || v.validator == nil {
func (v ReadOnlyValidator) CopyValidator() *ethpb.Validator {
if v.IsNil() {
return nil
}
return CopyValidator(v.validator)
}

// CopyValidator returns the copy of the read only validator.
func (v ReadOnlyValidator) IsNil() bool {
return v.validator == nil
}

// InnerStateUnsafe returns the pointer value of the underlying
// beacon state proto object, bypassing immutability. Use with care.
func (b *BeaconState) InnerStateUnsafe() *pbp2p.BeaconState {
Expand Down Expand Up @@ -606,21 +611,21 @@ func (b *BeaconState) ValidatorAtIndex(idx uint64) (*ethpb.Validator, error) {

// ValidatorAtIndexReadOnly is the validator at the provided index. This method
// doesn't clone the validator.
func (b *BeaconState) ValidatorAtIndexReadOnly(idx uint64) (*ReadOnlyValidator, error) {
func (b *BeaconState) ValidatorAtIndexReadOnly(idx uint64) (ReadOnlyValidator, error) {
if !b.HasInnerState() {
return nil, ErrNilInnerState
return ReadOnlyValidator{}, ErrNilInnerState
}
if b.state.Validators == nil {
return &ReadOnlyValidator{}, nil
return ReadOnlyValidator{}, nil
}
if uint64(len(b.state.Validators)) <= idx {
return nil, fmt.Errorf("index %d out of range", idx)
return ReadOnlyValidator{}, fmt.Errorf("index %d out of range", idx)
}

b.lock.RLock()
defer b.lock.RUnlock()

return &ReadOnlyValidator{b.state.Validators[idx]}, nil
return ReadOnlyValidator{b.state.Validators[idx]}, nil
}

// ValidatorIndexByPubkey returns a given validator by its 48-byte public key.
Expand Down Expand Up @@ -675,7 +680,7 @@ func (b *BeaconState) NumValidators() int {

// ReadFromEveryValidator reads values from every validator and applies it to the provided function.
// Warning: This method is potentially unsafe, as it exposes the actual validator registry.
func (b *BeaconState) ReadFromEveryValidator(f func(idx int, val *ReadOnlyValidator) error) error {
func (b *BeaconState) ReadFromEveryValidator(f func(idx int, val ReadOnlyValidator) error) error {
if !b.HasInnerState() {
return ErrNilInnerState
}
Expand All @@ -687,7 +692,7 @@ func (b *BeaconState) ReadFromEveryValidator(f func(idx int, val *ReadOnlyValida
b.lock.RUnlock()

for i, v := range validators {
err := f(i, &ReadOnlyValidator{validator: v})
err := f(i, ReadOnlyValidator{validator: v})
if err != nil {
return err
}
Expand Down

0 comments on commit 5995d23

Please sign in to comment.