QSP10, QSP11 - Add proper locking (#6350)

* Proper locking based on audit feedbacks * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Preston's feedback * Merge branch 'qsp10-11' of github.com:prysmaticlabs/prysm into qsp10-11 * Unlock in invalid index branch * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge branch 'master' into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11
prysmaticlabs · Jun 23, 2020 · c3adde3 · c3adde3
1 parent 2bdb825
commit c3adde3
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 5 deletions.
diff --git a/beacon-chain/forkchoice/protoarray/store.go b/beacon-chain/forkchoice/protoarray/store.go
@@ -101,6 +101,9 @@ func (f *ForkChoice) Prune(ctx context.Context, finalizedRoot [32]byte) error {
 
 // Nodes returns the copied list of block nodes in the fork choice store.
 func (f *ForkChoice) Nodes() []*Node {
+	f.store.nodeIndicesLock.RLock()
+	defer f.store.nodeIndicesLock.RUnlock()
+
 	cpy := make([]*Node, len(f.store.Nodes))
 	copy(cpy, f.store.Nodes)
 	return cpy

diff --git a/beacon-chain/state/getters.go b/beacon-chain/state/getters.go
@@ -237,6 +237,9 @@ func (b *BeaconState) ParentRoot() [32]byte {
 	if !b.HasInnerState() {
 		return [32]byte{}
 	}
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+
 	parentRoot := [32]byte{}
 	copy(parentRoot[:], b.state.LatestBlockHeader.ParentRoot)
 	return parentRoot
@@ -330,9 +333,14 @@ func (b *BeaconState) Eth1Data() *ethpb.Eth1Data {
 	if !b.HasInnerState() {
 		return nil
 	}
+
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+
 	if b.state.Eth1Data == nil {
 		return nil
 	}
+
 	return CopyETH1Data(b.state.Eth1Data)
 }
 
@@ -342,9 +350,14 @@ func (b *BeaconState) Eth1DataVotes() []*ethpb.Eth1Data {
 	if !b.HasInnerState() {
 		return nil
 	}
+
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+
 	if b.state.Eth1DataVotes == nil {
 		return nil
 	}
+
 	res := make([]*ethpb.Eth1Data, len(b.state.Eth1DataVotes))
 	for i := 0; i < len(res); i++ {
 		res[i] = CopyETH1Data(b.state.Eth1DataVotes[i])
@@ -358,6 +371,10 @@ func (b *BeaconState) Eth1DepositIndex() uint64 {
 	if !b.HasInnerState() {
 		return 0
 	}
+
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+
 	return b.state.Eth1DepositIndex
 }
 
@@ -410,16 +427,17 @@ func (b *BeaconState) ValidatorAtIndex(idx uint64) (*ethpb.Validator, error) {
 	if !b.HasInnerState() {
 		return nil, ErrNilInnerState
 	}
+
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+
 	if b.state.Validators == nil {
 		return &ethpb.Validator{}, nil
 	}
+
 	if uint64(len(b.state.Validators)) <= idx {
 		return nil, fmt.Errorf("index %d out of range", idx)
 	}
-
-	b.lock.RLock()
-	defer b.lock.RUnlock()
-
 	val := b.state.Validators[idx]
 	pubKey := make([]byte, len(val.PublicKey))
 	copy(pubKey, val.PublicKey)
@@ -502,6 +520,9 @@ func (b *BeaconState) NumValidators() int {
 	if !b.HasInnerState() {
 		return 0
 	}
+	b.lock.RLock()
+	defer b.lock.RUnlock()
+
 	return len(b.state.Validators)
 }
 

diff --git a/beacon-chain/state/setters.go b/beacon-chain/state/setters.go
@@ -155,11 +155,12 @@ func (b *BeaconState) UpdateStateRootAtIndex(idx uint64, stateRoot [32]byte) err
 	if !b.HasInnerState() {
 		return ErrNilInnerState
 	}
+	b.lock.RLock()
 	if len(b.state.StateRoots) <= int(idx) {
+		b.lock.RUnlock()
 		return errors.Errorf("invalid index provided %d", idx)
 	}
 
-	b.lock.RLock()
 	// Check if we hold the only reference to the shared state roots slice.
 	r := b.state.StateRoots
 	if ref := b.sharedFieldReferences[stateRoots]; ref.Refs() > 1 {