Modify the algorithm of updateFinalizedBlockRoots (#13486)

* rename error var

* new algo

* replay_test

* add comment

* review

* fill out parent root

* handle edge cases

* review

beacon-chain/db/kv/blocks.go

@@ -224,7 +224,7 @@ func (s *Store) DeleteBlock(ctx context.Context, root [32]byte) error {
 	return s.db.Update(func(tx *bolt.Tx) error {
 		bkt := tx.Bucket(finalizedBlockRootsIndexBucket)
 		if b := bkt.Get(root[:]); b != nil {
-			return ErrDeleteJustifiedAndFinalized
+			return ErrDeleteFinalized
 		}
 
 		if err := tx.Bucket(blocksBucket).Delete(root[:]); err != nil {

beacon-chain/db/kv/blocks_test.go

@@ -289,7 +289,7 @@ func TestStore_DeleteBlock(t *testing.T) {
 	require.Equal(t, b, nil)
 	require.Equal(t, false, db.HasStateSummary(ctx, root2))
 
-	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteJustifiedAndFinalized)
+	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteFinalized)
 }
 
 func TestStore_DeleteJustifiedBlock(t *testing.T) {
@@ -309,7 +309,7 @@ func TestStore_DeleteJustifiedBlock(t *testing.T) {
 	require.NoError(t, db.SaveBlock(ctx, blk))
 	require.NoError(t, db.SaveState(ctx, st, root))
 	require.NoError(t, db.SaveJustifiedCheckpoint(ctx, cp))
-	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteJustifiedAndFinalized)
+	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteFinalized)
 }
 
 func TestStore_DeleteFinalizedBlock(t *testing.T) {
@@ -329,7 +329,7 @@ func TestStore_DeleteFinalizedBlock(t *testing.T) {
 	require.NoError(t, db.SaveState(ctx, st, root))
 	require.NoError(t, db.SaveGenesisBlockRoot(ctx, root))
 	require.NoError(t, db.SaveFinalizedCheckpoint(ctx, cp))
-	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteJustifiedAndFinalized)
+	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteFinalized)
 }
 func TestStore_GenesisBlock(t *testing.T) {
 	db := setupDB(t)

beacon-chain/db/kv/error.go

@@ -2,8 +2,8 @@ package kv
 
 import "github.com/pkg/errors"
 
-// ErrDeleteJustifiedAndFinalized is raised when we attempt to delete a finalized block/state
-var ErrDeleteJustifiedAndFinalized = errors.New("cannot delete finalized block or state")
+// ErrDeleteFinalized is raised when we attempt to delete a finalized block/state
+var ErrDeleteFinalized = errors.New("cannot delete finalized block or state")
 
 // ErrNotFound can be used directly, or as a wrapped DBError, whenever a db method needs to
 // indicate that a value couldn't be found.

beacon-chain/db/kv/finalized_block_roots.go

@@ -5,7 +5,6 @@ import (
 	"context"
 
 	"github.com/pkg/errors"
-	"github.com/prysmaticlabs/prysm/v5/beacon-chain/db/filters"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/blocks"
 	"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
 	"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
@@ -29,139 +28,90 @@ var containerFinalizedButNotCanonical = []byte("recent block needs reindexing to
 // beacon block chain using the finalized root alone as this would exclude all other blocks in the
 // finalized epoch from being indexed as "final and canonical".
 //
-// The algorithm for building the index works as follows:
-//   - De-index all finalized beacon block roots from previous_finalized_epoch to
-//     new_finalized_epoch. (I.e. delete these roots from the index, to be re-indexed.)
-//   - Build the canonical finalized chain by walking up the ancestry chain from the finalized block
-//     root until a parent is found in the index, or the parent is genesis or the origin checkpoint.
-//   - Add all block roots in the database where epoch(block.slot) == checkpoint.epoch.
-//
-// This method ensures that all blocks from the current finalized epoch are considered "final" while
-// maintaining only canonical and finalized blocks older than the current finalized epoch.
+// The main part of the algorithm traverses parent->child block relationships in the
+// `blockParentRootIndicesBucket` bucket to find the path between the last finalized checkpoint
+// and the current finalized checkpoint. It relies on the invariant that there is a unique path
+// between two finalized checkpoints.
 func (s *Store) updateFinalizedBlockRoots(ctx context.Context, tx *bolt.Tx, checkpoint *ethpb.Checkpoint) error {
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.updateFinalizedBlockRoots")
 	defer span.End()
 
-	bkt := tx.Bucket(finalizedBlockRootsIndexBucket)
-
-	root := checkpoint.Root
-	var previousRoot []byte
-	genesisRoot := tx.Bucket(blocksBucket).Get(genesisBlockRootKey)
-	initCheckpointRoot := tx.Bucket(blocksBucket).Get(originCheckpointBlockRootKey)
-
-	// De-index recent finalized block roots, to be re-indexed.
+	finalizedBkt := tx.Bucket(finalizedBlockRootsIndexBucket)
 	previousFinalizedCheckpoint := &ethpb.Checkpoint{}
-	if b := bkt.Get(previousFinalizedCheckpointKey); b != nil {
+	if b := finalizedBkt.Get(previousFinalizedCheckpointKey); b != nil {
 		if err := decode(ctx, b, previousFinalizedCheckpoint); err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
 	}
 
-	blockRoots, err := s.BlockRoots(ctx, filters.NewFilter().
-		SetStartEpoch(previousFinalizedCheckpoint.Epoch).
-		SetEndEpoch(checkpoint.Epoch+1),
-	)
-	if err != nil {
-		tracing.AnnotateError(span, err)
-		return err
-	}
-	for _, root := range blockRoots {
-		if err := bkt.Delete(root[:]); err != nil {
-			tracing.AnnotateError(span, err)
-			return err
-		}
-	}
-
-	// Walk up the ancestry chain until we reach a block root present in the finalized block roots
-	// index bucket or genesis block root.
-	for {
-		if bytes.Equal(root, genesisRoot) {
-			break
-		}
-
-		signedBlock, err := s.Block(ctx, bytesutil.ToBytes32(root))
-		if err != nil {
-			tracing.AnnotateError(span, err)
-			return err
-		}
-		if err := blocks.BeaconBlockIsNil(signedBlock); err != nil {
-			tracing.AnnotateError(span, err)
-			return err
-		}
-		block := signedBlock.Block()
-
-		parentRoot := block.ParentRoot()
-		container := &ethpb.FinalizedBlockRootContainer{
-			ParentRoot: parentRoot[:],
-			ChildRoot:  previousRoot,
-		}
-
+	// Handle the case of checkpoint sync.
+	if previousFinalizedCheckpoint.Root == nil && bytes.Equal(checkpoint.Root, tx.Bucket(blocksBucket).Get(originCheckpointBlockRootKey)) {
+		container := &ethpb.FinalizedBlockRootContainer{}
 		enc, err := encode(ctx, container)
 		if err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
-		if err := bkt.Put(root, enc); err != nil {
+		if err = finalizedBkt.Put(checkpoint.Root, enc); err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
+		return updatePrevFinalizedCheckpoint(ctx, span, finalizedBkt, checkpoint)
+	}
 
-		// breaking here allows the initial checkpoint root to be correctly inserted,
-		// but stops the loop from trying to search for its parent.
-		if bytes.Equal(root, initCheckpointRoot) {
-			break
+	var finalized [][]byte
+	if previousFinalizedCheckpoint.Root == nil {
+		genesisRoot := tx.Bucket(blocksBucket).Get(genesisBlockRootKey)
+		_, finalized = pathToFinalizedCheckpoint(ctx, [][]byte{genesisRoot}, checkpoint.Root, tx)
+	} else {
+		if err := updateChildOfPrevFinalizedCheckpoint(
+			ctx,
+			span,
+			finalizedBkt,
+			tx.Bucket(blockParentRootIndicesBucket), previousFinalizedCheckpoint.Root,
+		); err != nil {
+			return err
 		}
+		_, finalized = pathToFinalizedCheckpoint(ctx, [][]byte{previousFinalizedCheckpoint.Root}, checkpoint.Root, tx)
+	}
 
-		// Found parent, loop exit condition.
-		pr := block.ParentRoot()
-		if parentBytes := bkt.Get(pr[:]); parentBytes != nil {
-			parent := &ethpb.FinalizedBlockRootContainer{}
-			if err := decode(ctx, parentBytes, parent); err != nil {
-				tracing.AnnotateError(span, err)
-				return err
+	for i, r := range finalized {
+		var container *ethpb.FinalizedBlockRootContainer
+		switch i {
+		case 0:
+			container = &ethpb.FinalizedBlockRootContainer{
+				ParentRoot: previousFinalizedCheckpoint.Root,
 			}
-			parent.ChildRoot = root
-			enc, err := encode(ctx, parent)
-			if err != nil {
-				tracing.AnnotateError(span, err)
-				return err
+			if len(finalized) > 1 {
+				container.ChildRoot = finalized[i+1]
 			}
-			if err := bkt.Put(pr[:], enc); err != nil {
-				tracing.AnnotateError(span, err)
-				return err
+		case len(finalized) - 1:
+			// We don't know the finalized child of the new finalized checkpoint.
+			// It will be filled out in the next function call.
+			container = &ethpb.FinalizedBlockRootContainer{}
+			if len(finalized) > 1 {
+				container.ParentRoot = finalized[i-1]
+			}
+		default:
+			container = &ethpb.FinalizedBlockRootContainer{
+				ParentRoot: finalized[i-1],
+				ChildRoot:  finalized[i+1],
 			}
-			break
 		}
-		previousRoot = root
-		root = pr[:]
-	}
 
-	// Upsert blocks from the current finalized epoch.
-	roots, err := s.BlockRoots(ctx, filters.NewFilter().SetStartEpoch(checkpoint.Epoch).SetEndEpoch(checkpoint.Epoch+1))
-	if err != nil {
-		tracing.AnnotateError(span, err)
-		return err
-	}
-	for _, root := range roots {
-		root := root[:]
-		if bytes.Equal(root, checkpoint.Root) || bkt.Get(root) != nil {
-			continue
+		enc, err := encode(ctx, container)
+		if err != nil {
+			tracing.AnnotateError(span, err)
+			return err
 		}
-		if err := bkt.Put(root, containerFinalizedButNotCanonical); err != nil {
+		if err = finalizedBkt.Put(r, enc); err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
 	}
 
-	// Update previous checkpoint
-	enc, err := encode(ctx, checkpoint)
-	if err != nil {
-		tracing.AnnotateError(span, err)
-		return err
-	}
-
-	return bkt.Put(previousFinalizedCheckpointKey, enc)
+	return updatePrevFinalizedCheckpoint(ctx, span, finalizedBkt, checkpoint)
 }
 
 // BackfillFinalizedIndex updates the finalized index for a contiguous chain of blocks that are the ancestors of the
@@ -242,8 +192,6 @@ func (s *Store) BackfillFinalizedIndex(ctx context.Context, blocks []blocks.ROBl
 
 // IsFinalizedBlock returns true if the block root is present in the finalized block root index.
 // A beacon block root contained exists in this index if it is considered finalized and canonical.
-// Note: beacon blocks from the latest finalized epoch return true, whether or not they are
-// considered canonical in the "head view" of the beacon node.
 func (s *Store) IsFinalizedBlock(ctx context.Context, blockRoot [32]byte) bool {
 	_, span := trace.StartSpan(ctx, "BeaconDB.IsFinalizedBlock")
 	defer span.End()
@@ -296,3 +244,53 @@ func (s *Store) FinalizedChildBlock(ctx context.Context, blockRoot [32]byte) (in
 	tracing.AnnotateError(span, err)
 	return blk, err
 }
+
+func pathToFinalizedCheckpoint(ctx context.Context, roots [][]byte, checkpointRoot []byte, tx *bolt.Tx) (bool, [][]byte) {
+	if len(roots) == 0 || (len(roots) == 1 && roots[0] == nil) {
+		return false, nil
+	}
+
+	for _, r := range roots {
+		if bytes.Equal(r, checkpointRoot) {
+			return true, [][]byte{r}
+		}
+		children := lookupValuesForIndices(ctx, map[string][]byte{string(blockParentRootIndicesBucket): r}, tx)
+		if len(children) == 0 {
+			children = [][][]byte{nil}
+		}
+		isPath, path := pathToFinalizedCheckpoint(ctx, children[0], checkpointRoot, tx)
+		if isPath {
+			return true, append([][]byte{r}, path...)
+		}
+	}
+
+	return false, nil
+}
+
+func updatePrevFinalizedCheckpoint(ctx context.Context, span *trace.Span, finalizedBkt *bolt.Bucket, checkpoint *ethpb.Checkpoint) error {
+	enc, err := encode(ctx, checkpoint)
+	if err != nil {
+		tracing.AnnotateError(span, err)
+		return err
+	}
+	return finalizedBkt.Put(previousFinalizedCheckpointKey, enc)
+}
+
+func updateChildOfPrevFinalizedCheckpoint(ctx context.Context, span *trace.Span, finalizedBkt, parentBkt *bolt.Bucket, checkpointRoot []byte) error {
+	container := &ethpb.FinalizedBlockRootContainer{}
+	if err := decode(ctx, finalizedBkt.Get(checkpointRoot), container); err != nil {
+		tracing.AnnotateError(span, err)
+		return err
+	}
+	container.ChildRoot = parentBkt.Get(checkpointRoot)
+	enc, err := encode(ctx, container)
+	if err != nil {
+		tracing.AnnotateError(span, err)
+		return err
+	}
+	if err = finalizedBkt.Put(checkpointRoot, enc); err != nil {
+		tracing.AnnotateError(span, err)
+		return err
+	}
+	return nil
+}