Guard against deleting genesis and finalized state in DB (#3842)

* Don't delete boundary state

* Lint

* Test

* Feedback

* Batch and better comment

* Fix test

* zzzzzz

* rmStatesOlderThanLastFinalized

beacon-chain/blockchain/forkchoice/process_block.go

@@ -101,17 +101,18 @@ func (s *Store) OnBlock(ctx context.Context, b *ethpb.BeaconBlock) error {
 	if postState.FinalizedCheckpoint.Epoch > s.finalizedCheckpt.Epoch {
 		s.clearSeenAtts()
 		helpers.ClearAllCaches()
+		if err := s.db.SaveFinalizedCheckpoint(ctx, postState.FinalizedCheckpoint); err != nil {
+			return errors.Wrap(err, "could not save finalized checkpoint")
+		}
+
 		startSlot := helpers.StartSlot(s.finalizedCheckpt.Epoch + 1)
 		endSlot := helpers.StartSlot(postState.FinalizedCheckpoint.Epoch+1) - 1 // Inclusive
-		if err := s.rmStatesBySlots(ctx, startSlot, endSlot); err != nil {
+		if err := s.rmStatesOlderThanLastFinalized(ctx, startSlot, endSlot); err != nil {
 			return errors.Wrapf(err, "could not delete states prior to finalized check point, range: %d, %d",
 				startSlot, endSlot+params.BeaconConfig().SlotsPerEpoch)
 		}
 
 		s.finalizedCheckpt = postState.FinalizedCheckpoint
-		if err := s.db.SaveFinalizedCheckpoint(ctx, postState.FinalizedCheckpoint); err != nil {
-			return errors.Wrap(err, "could not save finalized checkpoint")
-		}
 	}
 
 	// Update validator indices in database as needed.
@@ -186,7 +187,7 @@ func (s *Store) OnBlockNoVerifyStateTransition(ctx context.Context, b *ethpb.Bea
 		helpers.ClearAllCaches()
 		startSlot := helpers.StartSlot(s.finalizedCheckpt.Epoch + 1)
 		endSlot := helpers.StartSlot(postState.FinalizedCheckpoint.Epoch+1) - 1 // Inclusive
-		if err := s.rmStatesBySlots(ctx, startSlot, endSlot); err != nil {
+		if err := s.rmStatesOlderThanLastFinalized(ctx, startSlot, endSlot); err != nil {
 			return errors.Wrapf(err, "could not delete states prior to finalized check point, range: %d, %d",
 				startSlot, endSlot+params.BeaconConfig().SlotsPerEpoch)
 		}
@@ -376,14 +377,14 @@ func (s *Store) clearSeenAtts() {
 	s.seenAtts = make(map[[32]byte]bool)
 }
 
-// rmStatesBySlots deletes the states in db in between the range of slots.
+// rmStatesOlderThanLastFinalized deletes the states in db since last finalized check point.
-func (s *Store) rmStatesBySlots(ctx context.Context, startSlot uint64, endSlot uint64) error {
+func (s *Store) rmStatesOlderThanLastFinalized(ctx context.Context, startSlot uint64, endSlot uint64) error {
 	ctx, span := trace.StartSpan(ctx, "forkchoice.rmStatesBySlots")
 	defer span.End()
 
-	// Do not remove genesis state.
+	// Do not remove genesis state or finalized state at epoch boundary.
-	if startSlot == 0 {
+	if startSlot%params.BeaconConfig().SlotsPerEpoch == 0 {
-		startSlot = 1
+		startSlot++
 	}
 
 	filter := filters.NewFilter().SetStartSlot(startSlot).SetEndSlot(endSlot)

beacon-chain/blockchain/forkchoice/process_block_test.go

@@ -276,7 +276,7 @@ func TestStore_SavesNewBlockAttestations(t *testing.T) {
 	}
 }
 
-func TestRemoveStateBySlots(t *testing.T) {
+func TestRemoveStateSinceLastFinalized(t *testing.T) {
 	ctx := context.Background()
 	db := testDB.SetupDB(t)
 	defer testDB.TeardownDB(t, db)
@@ -309,7 +309,7 @@ func TestRemoveStateBySlots(t *testing.T) {
 	// New finalized epoch: 1
 	finalizedEpoch := uint64(1)
 	endSlot := helpers.StartSlot(finalizedEpoch+1) - 1 // Inclusive
-	if err := store.rmStatesBySlots(ctx, 0, endSlot); err != nil {
+	if err := store.rmStatesOlderThanLastFinalized(ctx, 0, endSlot); err != nil {
 		t.Fatal(err)
 	}
 	for _, r := range blockRoots {
@@ -326,7 +326,7 @@ func TestRemoveStateBySlots(t *testing.T) {
 	// New finalized epoch: 5
 	newFinalizedEpoch := uint64(5)
 	endSlot = helpers.StartSlot(newFinalizedEpoch+1) - 1 // Inclusive
-	if err := store.rmStatesBySlots(ctx, helpers.StartSlot(finalizedEpoch+1), endSlot); err != nil {
+	if err := store.rmStatesOlderThanLastFinalized(ctx, helpers.StartSlot(finalizedEpoch+1), endSlot); err != nil {
 		t.Fatal(err)
 	}
 	for _, r := range blockRoots {
@@ -334,9 +334,12 @@ func TestRemoveStateBySlots(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		// Also verifies genesis state didnt get deleted
+		// Also verifies boundary state didnt get deleted
-		if s != nil && s.Slot != 0 && s.Slot < endSlot {
+		if s != nil {
-			t.Errorf("State with slot %d should not be in DB", s.Slot)
+			isBoundary := s.Slot%params.BeaconConfig().SlotsPerEpoch == 0
+			if !isBoundary && s.Slot < endSlot {
+				t.Errorf("State with slot %d should not be in DB", s.Slot)
+			}
 		}
 	}
 }

beacon-chain/db/kv/state.go

@@ -1,6 +1,7 @@
 package kv
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"strings"
@@ -10,6 +11,7 @@ import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/pkg/errors"
 	pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
+	ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1"
 	"go.opencensus.io/trace"
 )
 
@@ -104,8 +106,26 @@ func (k *Store) SaveState(ctx context.Context, state *pb.BeaconState, blockRoot
 func (k *Store) DeleteState(ctx context.Context, blockRoot [32]byte) error {
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.DeleteState")
 	defer span.End()
+
 	return k.db.Batch(func(tx *bolt.Tx) error {
-		bkt := tx.Bucket(stateBucket)
+		bkt := tx.Bucket(blocksBucket)
+		genesisBlockRoot := bkt.Get(genesisBlockRootKey)
+
+		bkt = tx.Bucket(checkpointBucket)
+		enc := bkt.Get(finalizedCheckpointKey)
+		checkpoint := &ethpb.Checkpoint{}
+		if enc == nil {
+			checkpoint = &ethpb.Checkpoint{Root: genesisBlockRoot}
+		} else {
+			proto.Unmarshal(enc, checkpoint)
+		}
+
+		// Safe guard against deleting genesis or finalized state.
+		if bytes.Equal(blockRoot[:], checkpoint.Root) || bytes.Equal(blockRoot[:], genesisBlockRoot) {
+			return errors.New("could not delete genesis or finalized state")
+		}
+
+		bkt = tx.Bucket(stateBucket)
 		return bkt.Delete(blockRoot[:])
 	})
 }

beacon-chain/db/kv/state_test.go

@@ -162,3 +162,42 @@ func TestStore_StatesBatchDelete(t *testing.T) {
 		}
 	}
 }
+
+func TestStore_DeleteGenesisState(t *testing.T) {
+	db := setupDB(t)
+	defer teardownDB(t, db)
+	ctx := context.Background()
+
+	genesisBlockRoot := [32]byte{'A'}
+	if err := db.SaveGenesisBlockRoot(ctx, genesisBlockRoot); err != nil {
+		t.Fatal(err)
+	}
+	genesisState := &pb.BeaconState{Slot: 100}
+	if err := db.SaveState(ctx, genesisState, genesisBlockRoot); err != nil {
+		t.Fatal(err)
+	}
+	wantedErr := "could not delete genesis or finalized state"
+	if err := db.DeleteState(ctx, genesisBlockRoot); err.Error() != wantedErr {
+		t.Error("Did not receive wanted error")
+	}
+}
+
+func TestStore_DeleteFinalizedState(t *testing.T) {
+	db := setupDB(t)
+	defer teardownDB(t, db)
+	ctx := context.Background()
+
+	finalizedBlockRoot := [32]byte{'A'}
+	finalizedCheckpoint := &ethpb.Checkpoint{Root: finalizedBlockRoot[:]}
+	if err := db.SaveFinalizedCheckpoint(ctx, finalizedCheckpoint); err != nil {
+		t.Fatal(err)
+	}
+	finalizedState := &pb.BeaconState{Slot: 100}
+	if err := db.SaveState(ctx, finalizedState, finalizedBlockRoot); err != nil {
+		t.Fatal(err)
+	}
+	wantedErr := "could not delete genesis or finalized state"
+	if err := db.DeleteState(ctx, finalizedBlockRoot); err.Error() != wantedErr {
+		t.Error("Did not receive wanted error")
+	}
+}

tools/interop/export-genesis/main.go

@@ -10,7 +10,6 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/db"
 )
 
-
 // A basic tool to extract genesis.ssz from existing beaconchain.db.
 // ex:
 //   bazel run //tools/interop/export-genesis:export-genesis -- /tmp/data/beaconchaindata /tmp/genesis.ssz