Forkchoice interface cleanup (#12010)

* remove AllTipsAreInvalid

* rename InsertOptimisticChain

* remove proposer boost interface

* remove HasParent

beacon-chain/blockchain/chain_info.go

@@ -330,9 +330,6 @@ func (s *Service) IsOptimistic(ctx context.Context) (bool, error) {
 	headRoot := s.head.root
 	s.headLock.RUnlock()
 
-	if s.cfg.ForkChoiceStore.AllTipsAreInvalid() {
-		return true, nil
-	}
 	optimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(headRoot)
 	if err == nil {
 		return optimistic, nil

beacon-chain/blockchain/process_block.go

@@ -443,7 +443,7 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []interfaces.ReadOnlySi
 		}
 	}
 	// Insert all nodes but the last one to forkchoice
-	if err := s.cfg.ForkChoiceStore.InsertOptimisticChain(ctx, pendingNodes); err != nil {
+	if err := s.cfg.ForkChoiceStore.InsertChain(ctx, pendingNodes); err != nil {
 		return errors.Wrap(err, "could not insert batch to forkchoice")
 	}
 	// Insert the last block to forkchoice

beacon-chain/blockchain/process_block_helpers.go

@@ -201,7 +201,7 @@ func (s *Service) ancestor(ctx context.Context, root []byte, slot primitives.Slo
 	r := bytesutil.ToBytes32(root)
 	// Get ancestor root from fork choice store instead of recursively looking up blocks in DB.
 	// This is most optimal outcome.
-	ar, err := s.ancestorByForkChoiceStore(ctx, r, slot)
+	ar, err := s.cfg.ForkChoiceStore.AncestorRoot(ctx, r, slot)
 	if err != nil {
 		// Try getting ancestor root from DB when failed to retrieve from fork choice store.
 		// This is the second line of defense for retrieving ancestor root.
@@ -211,38 +211,28 @@ func (s *Service) ancestor(ctx context.Context, root []byte, slot primitives.Slo
 		}
 	}
 
-	return ar, nil
-}
-
-// This retrieves an ancestor root using fork choice store. The look up is looping through the a flat array structure.
-func (s *Service) ancestorByForkChoiceStore(ctx context.Context, r [32]byte, slot primitives.Slot) ([]byte, error) {
-	ctx, span := trace.StartSpan(ctx, "blockChain.ancestorByForkChoiceStore")
-	defer span.End()
-
-	if !s.cfg.ForkChoiceStore.HasParent(r) {
-		return nil, errors.New("could not find root in fork choice store")
-	}
-	root, err := s.cfg.ForkChoiceStore.AncestorRoot(ctx, r, slot)
-	return root[:], err
+	return ar[:], nil
 }
 
 // This retrieves an ancestor root using DB. The look up is recursively looking up DB. Slower than `ancestorByForkChoiceStore`.
-func (s *Service) ancestorByDB(ctx context.Context, r [32]byte, slot primitives.Slot) ([]byte, error) {
+func (s *Service) ancestorByDB(ctx context.Context, r [32]byte, slot primitives.Slot) (root [32]byte, err error) {
 	ctx, span := trace.StartSpan(ctx, "blockChain.ancestorByDB")
 	defer span.End()
 
+	root = [32]byte{}
 	// Stop recursive ancestry lookup if context is cancelled.
 	if ctx.Err() != nil {
-		return nil, ctx.Err()
+		err = ctx.Err()
+		return
 	}
 
 	signed, err := s.getBlock(ctx, r)
 	if err != nil {
-		return nil, err
+		return root, err
 	}
 	b := signed.Block()
 	if b.Slot() == slot || b.Slot() < slot {
-		return r[:], nil
+		return r, nil
 	}
 
 	return s.ancestorByDB(ctx, b.ParentRoot(), slot)
@@ -284,7 +274,7 @@ func (s *Service) fillInForkChoiceMissingBlocks(ctx context.Context, blk interfa
 	if root != s.ensureRootNotZeros(finalized.Root) && !s.ForkChoicer().HasNode(root) {
 		return errNotDescendantOfFinalized
 	}
-	return s.cfg.ForkChoiceStore.InsertOptimisticChain(ctx, pendingNodes)
+	return s.cfg.ForkChoiceStore.InsertChain(ctx, pendingNodes)
 }
 
 // inserts finalized deposits into our finalized deposit trie.

beacon-chain/blockchain/process_block_test.go

@@ -2124,7 +2124,6 @@ func TestNoViableHead_Reboot(t *testing.T) {
 	optimistic, err := service.IsOptimistic(ctx)
 	require.NoError(t, err)
 	require.Equal(t, false, optimistic)
-	require.Equal(t, false, service.ForkChoicer().AllTipsAreInvalid())
 
 	// Check that the node's justified checkpoint does not agree with the
 	// last valid state's justified checkpoint
@@ -2151,7 +2150,6 @@ func TestNoViableHead_Reboot(t *testing.T) {
 	optimistic, err = service.IsOptimistic(ctx)
 	require.NoError(t, err)
 	require.Equal(t, true, optimistic)
-	require.Equal(t, true, service.ForkChoicer().AllTipsAreInvalid())
 	st, err = service.cfg.StateGen.StateByRoot(ctx, root)
 	require.NoError(t, err)
 
@@ -2179,7 +2177,6 @@ func TestNoViableHead_Reboot(t *testing.T) {
 	optimistic, err = service.IsOptimistic(ctx)
 	require.NoError(t, err)
 	require.Equal(t, true, optimistic)
-	require.Equal(t, true, service.ForkChoicer().AllTipsAreInvalid())
 
 	// Import block 24, it should justify Epoch 3 and become HEAD, the node
 	// recovers
@@ -2203,7 +2200,6 @@ func TestNoViableHead_Reboot(t *testing.T) {
 	optimistic, err = service.IsOptimistic(ctx)
 	require.NoError(t, err)
 	require.Equal(t, false, optimistic)
-	require.Equal(t, false, service.ForkChoicer().AllTipsAreInvalid())
 }
 
 func TestOnBlock_HandleBlockAttestations(t *testing.T) {

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

@@ -249,20 +249,6 @@ func (f *ForkChoice) HasNode(root [32]byte) bool {
 	return ok
 }
 
-// HasParent returns true if the node parent exists in fork choice store,
-// false else wise.
-func (f *ForkChoice) HasParent(root [32]byte) bool {
-	f.store.nodesLock.RLock()
-	defer f.store.nodesLock.RUnlock()
-
-	node, ok := f.store.nodeByRoot[root]
-	if !ok || node == nil {
-		return false
-	}
-
-	return node.parent != nil
-}
-
 // IsCanonical returns true if the given root is part of the canonical chain.
 func (f *ForkChoice) IsCanonical(root [32]byte) bool {
 	f.store.nodesLock.RLock()
@@ -290,6 +276,10 @@ func (f *ForkChoice) IsOptimistic(root [32]byte) (bool, error) {
 	f.store.nodesLock.RLock()
 	defer f.store.nodesLock.RUnlock()
 
+	if f.store.allTipsAreInvalid {
+		return true, nil
+	}
+
 	node, ok := f.store.nodeByRoot[root]
 	if !ok || node == nil {
 		return true, ErrNilNode
@@ -575,7 +565,7 @@ func (f *ForkChoice) CommonAncestor(ctx context.Context, r1 [32]byte, r2 [32]byt
 // number). All blocks are assumed to be a strict chain
 // where blocks[i].Parent = blocks[i+1]. Also we assume that the parent of the
 // last block in this list is already included in forkchoice store.
-func (f *ForkChoice) InsertOptimisticChain(ctx context.Context, chain []*forkchoicetypes.BlockAndCheckpoints) error {
+func (f *ForkChoice) InsertChain(ctx context.Context, chain []*forkchoicetypes.BlockAndCheckpoints) error {
 	if len(chain) == 0 {
 		return nil
 	}

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

@@ -580,7 +580,7 @@ func TestStore_CommonAncestor(t *testing.T) {
 	require.ErrorIs(t, err, forkchoice.ErrUnknownCommonAncestor)
 }
 
-func TestStore_InsertOptimisticChain(t *testing.T) {
+func TestStore_InsertChain(t *testing.T) {
 	f := setup(1, 1)
 	blks := make([]*forkchoicetypes.BlockAndCheckpoints, 0)
 	blk := util.NewBeaconBlock()
@@ -613,10 +613,10 @@ func TestStore_InsertOptimisticChain(t *testing.T) {
 	for i := 0; i < len(blks); i++ {
 		args[i] = blks[10-i-1]
 	}
-	require.NoError(t, f.InsertOptimisticChain(context.Background(), args))
+	require.NoError(t, f.InsertChain(context.Background(), args))
 
 	f = setup(1, 1)
-	require.NoError(t, f.InsertOptimisticChain(context.Background(), args[2:]))
+	require.NoError(t, f.InsertChain(context.Background(), args[2:]))
 }
 
 func TestForkChoice_UpdateCheckpoints(t *testing.T) {

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

@@ -32,7 +32,7 @@ import (
 //	        store.justified_checkpoint = store.best_justified_checkpoint
 func (f *ForkChoice) NewSlot(ctx context.Context, slot primitives.Slot) error {
 	// Reset proposer boost root
-	if err := f.ResetBoostedProposerRoot(ctx); err != nil {
+	if err := f.resetBoostedProposerRoot(ctx); err != nil {
 		return errors.Wrap(err, "could not reset boosted proposer root in fork choice")
 	}
 

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

@@ -109,10 +109,3 @@ func (s *Store) removeNodeAndChildren(ctx context.Context, node *Node, invalidRo
 	delete(s.nodeByPayload, node.payloadHash)
 	return invalidRoots, nil
 }
-
-// AllTipsAreInvalid returns true if no forkchoice tip is viable for head
-func (f *ForkChoice) AllTipsAreInvalid() bool {
-	f.store.nodesLock.RLock()
-	defer f.store.nodesLock.RUnlock()
-	return f.store.allTipsAreInvalid
-}

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

@@ -7,8 +7,8 @@ import (
 	"github.com/prysmaticlabs/prysm/v3/config/params"
 )
 
-// ResetBoostedProposerRoot sets the value of the proposer boosted root to zeros.
-func (f *ForkChoice) ResetBoostedProposerRoot(_ context.Context) error {
+// resetBoostedProposerRoot sets the value of the proposer boosted root to zeros.
+func (f *ForkChoice) resetBoostedProposerRoot(_ context.Context) error {
 	f.store.proposerBoostLock.Lock()
 	f.store.proposerBoostRoot = [32]byte{}
 	f.store.proposerBoostLock.Unlock()

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

@@ -320,25 +320,6 @@ func TestStore_PruneMapsNodes(t *testing.T) {
 
 }
 
-func TestStore_HasParent(t *testing.T) {
-	f := setup(1, 1)
-	ctx := context.Background()
-	state, blkRoot, err := prepareForkchoiceState(ctx, 1, indexToHash(1), params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)
-	require.NoError(t, err)
-	require.NoError(t, f.InsertNode(ctx, state, blkRoot))
-	state, blkRoot, err = prepareForkchoiceState(ctx, 2, indexToHash(2), indexToHash(1), params.BeaconConfig().ZeroHash, 1, 1)
-	require.NoError(t, err)
-	require.NoError(t, f.InsertNode(ctx, state, blkRoot))
-	state, blkRoot, err = prepareForkchoiceState(ctx, 3, indexToHash(3), indexToHash(2), params.BeaconConfig().ZeroHash, 1, 1)
-	require.NoError(t, err)
-	require.NoError(t, f.InsertNode(ctx, state, blkRoot))
-	require.Equal(t, false, f.HasParent(params.BeaconConfig().ZeroHash))
-	require.Equal(t, true, f.HasParent(indexToHash(1)))
-	require.Equal(t, true, f.HasParent(indexToHash(2)))
-	require.Equal(t, true, f.HasParent(indexToHash(3)))
-	require.Equal(t, false, f.HasParent(indexToHash(4)))
-}
-
 func TestForkChoice_HighestReceivedBlockSlotRoot(t *testing.T) {
 	f := setup(1, 1)
 	s := f.store

beacon-chain/forkchoice/interfaces.go

@@ -21,7 +21,6 @@ type ForkChoicer interface {
 	AttestationProcessor // to track new attestation for fork choice.
 	Getter               // to retrieve fork choice information.
 	Setter               // to set fork choice information.
-	ProposerBooster      // ability to boost timely-proposed block roots.
 }
 
 // HeadRetriever retrieves head root and optimistic info of the current chain.
@@ -30,13 +29,12 @@ type HeadRetriever interface {
 	CachedHeadRoot() [32]byte
 	Tips() ([][32]byte, []primitives.Slot)
 	IsOptimistic(root [32]byte) (bool, error)
-	AllTipsAreInvalid() bool
 }
 
 // BlockProcessor processes the block that's used for accounting fork choice.
 type BlockProcessor interface {
 	InsertNode(context.Context, state.BeaconState, [32]byte) error
-	InsertOptimisticChain(context.Context, []*forkchoicetypes.BlockAndCheckpoints) error
+	InsertChain(context.Context, []*forkchoicetypes.BlockAndCheckpoints) error
 }
 
 // AttestationProcessor processes the attestation that's used for accounting fork choice.
@@ -45,16 +43,10 @@ type AttestationProcessor interface {
 	InsertSlashedIndex(context.Context, primitives.ValidatorIndex)
 }
 
-// ProposerBooster is able to boost the proposer's root score during fork choice.
-type ProposerBooster interface {
-	ResetBoostedProposerRoot(ctx context.Context) error
-}
-
 // Getter returns fork choice related information.
 type Getter interface {
 	HasNode([32]byte) bool
 	ProposerBoost() [fieldparams.RootLength]byte
-	HasParent(root [32]byte) bool
 	AncestorRoot(ctx context.Context, root [32]byte, slot primitives.Slot) ([32]byte, error)
 	CommonAncestor(ctx context.Context, root1 [32]byte, root2 [32]byte) ([32]byte, primitives.Slot, error)
 	IsCanonical(root [32]byte) bool