Node by root mutex fix (#11172)

beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go

@@ -62,7 +62,8 @@ func (f *ForkChoice) Head(
 
 	calledHeadCount.Inc()
 
-	// Using the write lock here because `applyWeightChanges` that gets called subsequently requires a write operation.
+	// Using the write lock here because subsequent calls to `updateBalances`, `applyProposerBoostScore`,
+	// `applyWeightChanges`, `updateBestDescendant`, and `head` require write operations on nodes.
 	f.store.nodesLock.Lock()
 	defer f.store.nodesLock.Unlock()
 
@@ -294,7 +295,7 @@ func (f *ForkChoice) AncestorRoot(ctx context.Context, root [32]byte, slot types
 }
 
 // updateBalances updates the balances that directly voted for each block taking into account the
-// validators' latest votes.
+// validators' latest votes. This function requires a lock in Store.nodesLock.
 func (f *ForkChoice) updateBalances(newBalances []uint64) error {
 	for index, vote := range f.votes {
 		// Skip if validator has been slashed

beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync.go

@@ -8,34 +8,34 @@ import (
 )
 
 func (s *Store) setOptimisticToInvalid(ctx context.Context, root, parentRoot, payloadHash [32]byte) ([][32]byte, error) {
-	s.nodesLock.Lock()
+	s.nodesLock.RLock()
 	invalidRoots := make([][32]byte, 0)
 	node, ok := s.nodeByRoot[root]
 	if !ok {
 		node, ok = s.nodeByRoot[parentRoot]
 		if !ok || node == nil {
-			s.nodesLock.Unlock()
+			s.nodesLock.RUnlock()
 			return invalidRoots, errors.Wrap(ErrNilNode, "could not set node to invalid")
 		}
 		// return early if the parent is LVH
 		if node.payloadHash == payloadHash {
-			s.nodesLock.Unlock()
+			s.nodesLock.RUnlock()
 			return invalidRoots, nil
 		}
 	} else {
 		if node == nil {
-			s.nodesLock.Unlock()
+			s.nodesLock.RUnlock()
 			return invalidRoots, errors.Wrap(ErrNilNode, "could not set node to invalid")
 		}
 		if node.parent.root != parentRoot {
-			s.nodesLock.Unlock()
+			s.nodesLock.RUnlock()
 			return invalidRoots, errInvalidParentRoot
 		}
 	}
 	firstInvalid := node
 	for ; firstInvalid.parent != nil && firstInvalid.parent.payloadHash != payloadHash; firstInvalid = firstInvalid.parent {
 		if ctx.Err() != nil {
-			s.nodesLock.Unlock()
+			s.nodesLock.RUnlock()
 			return invalidRoots, ctx.Err()
 		}
 	}
@@ -44,12 +44,12 @@ func (s *Store) setOptimisticToInvalid(ctx context.Context, root, parentRoot, pa
 	if firstInvalid.parent == nil {
 		// return early if the invalid node was not imported
 		if node.root == parentRoot {
-			s.nodesLock.Unlock()
+			s.nodesLock.RUnlock()
 			return invalidRoots, nil
 		}
 		firstInvalid = node
 	}
-	s.nodesLock.Unlock()
+	s.nodesLock.RUnlock()
 	return s.removeNode(ctx, firstInvalid)
 }
 

beacon-chain/forkchoice/doubly-linked-tree/store.go

@@ -17,7 +17,7 @@ import (
 const defaultPruneThreshold = 256
 
 // applyProposerBoostScore applies the current proposer boost scores to the
-// relevant nodes
+// relevant nodes. This function requires a lock in Store.nodesLock.
 func (s *Store) applyProposerBoostScore(newBalances []uint64) error {
 	s.proposerBoostLock.Lock()
 	defer s.proposerBoostLock.Unlock()
@@ -239,6 +239,10 @@ func (s *Store) prune(ctx context.Context) error {
 func (s *Store) tips() ([][32]byte, []types.Slot) {
 	var roots [][32]byte
 	var slots []types.Slot
+
+	s.nodesLock.RLock()
+	defer s.nodesLock.RUnlock()
+
 	for root, node := range s.nodeByRoot {
 		if len(node.children) == 0 {
 			roots = append(roots, root)