Revert "Modify the algorithm of updateFinalizedBlockRoots (#13486)"

This reverts commit 32fb183.

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 ErrDeleteFinalized
+			return ErrDeleteJustifiedAndFinalized
 		}
 
 		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), ErrDeleteFinalized)
+	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteJustifiedAndFinalized)
 }
 
 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), ErrDeleteFinalized)
+	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteJustifiedAndFinalized)
 }
 
 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), ErrDeleteFinalized)
+	require.ErrorIs(t, db.DeleteBlock(ctx, root), ErrDeleteJustifiedAndFinalized)
 }
 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"
 
-// ErrDeleteFinalized is raised when we attempt to delete a finalized block/state
-var ErrDeleteFinalized = errors.New("cannot delete finalized block or state")
+// ErrDeleteJustifiedAndFinalized is raised when we attempt to delete a finalized block/state
+var ErrDeleteJustifiedAndFinalized = 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,6 +5,7 @@ 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"
@@ -28,90 +29,139 @@ 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 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.
+// 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.
 func (s *Store) updateFinalizedBlockRoots(ctx context.Context, tx *bolt.Tx, checkpoint *ethpb.Checkpoint) error {
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.updateFinalizedBlockRoots")
 	defer span.End()
 
-	finalizedBkt := tx.Bucket(finalizedBlockRootsIndexBucket)
+	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.
 	previousFinalizedCheckpoint := &ethpb.Checkpoint{}
-	if b := finalizedBkt.Get(previousFinalizedCheckpointKey); b != nil {
+	if b := bkt.Get(previousFinalizedCheckpointKey); b != nil {
 		if err := decode(ctx, b, previousFinalizedCheckpoint); err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
 	}
 
-	// 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)
+	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 = finalizedBkt.Put(checkpoint.Root, enc); err != nil {
+		if err := blocks.BeaconBlockIsNil(signedBlock); err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
-		return updatePrevFinalizedCheckpoint(ctx, span, finalizedBkt, checkpoint)
-	}
+		block := signedBlock.Block()
+
+		parentRoot := block.ParentRoot()
+		container := &ethpb.FinalizedBlockRootContainer{
+			ParentRoot: parentRoot[:],
+			ChildRoot:  previousRoot,
+		}
 
-	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 {
+		enc, err := encode(ctx, container)
+		if err != nil {
+			tracing.AnnotateError(span, err)
+			return err
+		}
+		if err := bkt.Put(root, enc); err != nil {
+			tracing.AnnotateError(span, err)
 			return err
 		}
-		_, finalized = pathToFinalizedCheckpoint(ctx, [][]byte{previousFinalizedCheckpoint.Root}, checkpoint.Root, tx)
-	}
 
-	for i, r := range finalized {
-		var container *ethpb.FinalizedBlockRootContainer
-		switch i {
-		case 0:
-			container = &ethpb.FinalizedBlockRootContainer{
-				ParentRoot: previousFinalizedCheckpoint.Root,
-			}
-			if len(finalized) > 1 {
-				container.ChildRoot = finalized[i+1]
+		// 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
+		}
+
+		// 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
 			}
-		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]
+			parent.ChildRoot = root
+			enc, err := encode(ctx, parent)
+			if err != nil {
+				tracing.AnnotateError(span, err)
+				return err
 			}
-		default:
-			container = &ethpb.FinalizedBlockRootContainer{
-				ParentRoot: finalized[i-1],
-				ChildRoot:  finalized[i+1],
+			if err := bkt.Put(pr[:], enc); err != nil {
+				tracing.AnnotateError(span, err)
+				return err
 			}
+			break
 		}
+		previousRoot = root
+		root = pr[:]
+	}
 
-		enc, err := encode(ctx, container)
-		if err != nil {
-			tracing.AnnotateError(span, err)
-			return err
+	// 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
 		}
-		if err = finalizedBkt.Put(r, enc); err != nil {
+		if err := bkt.Put(root, containerFinalizedButNotCanonical); err != nil {
 			tracing.AnnotateError(span, err)
 			return err
 		}
 	}
 
-	return updatePrevFinalizedCheckpoint(ctx, span, finalizedBkt, checkpoint)
+	// Update previous checkpoint
+	enc, err := encode(ctx, checkpoint)
+	if err != nil {
+		tracing.AnnotateError(span, err)
+		return err
+	}
+
+	return bkt.Put(previousFinalizedCheckpointKey, enc)
 }
 
 // BackfillFinalizedIndex updates the finalized index for a contiguous chain of blocks that are the ancestors of the
@@ -192,6 +242,8 @@ 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()
@@ -244,53 +296,3 @@ 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
-}