Require state to exist when updating finalized checkpoint (#3843)

* require state to exist before saving finalized checkpoint

* Add comment

* gaz

* fix test

* fix test

beacon-chain/blockchain/service_test.go

@@ -358,15 +358,15 @@ func TestChainService_InitializeChainInfo(t *testing.T) {
 	headBlock := &ethpb.BeaconBlock{Slot: finalizedSlot}
 	headState := &pb.BeaconState{Slot: finalizedSlot}
 	headRoot, _ := ssz.SigningRoot(headBlock)
+	if err := db.SaveState(ctx, headState, headRoot); err != nil {
+		t.Fatal(err)
+	}
 	if err := db.SaveFinalizedCheckpoint(ctx, &ethpb.Checkpoint{
 		Epoch: helpers.SlotToEpoch(finalizedSlot),
 		Root:  headRoot[:],
 	}); err != nil {
 		t.Fatal(err)
 	}
-	if err := db.SaveState(ctx, headState, headRoot); err != nil {
-		t.Fatal(err)
-	}
 	if err := db.SaveBlock(ctx, headBlock); err != nil {
 		t.Fatal(err)
 	}

beacon-chain/db/kv/BUILD.bazel

@@ -60,6 +60,7 @@ go_test(
         "//beacon-chain/db/filters:go_default_library",
         "//proto/beacon/p2p/v1:go_default_library",
         "//proto/eth/v1alpha1:go_default_library",
+        "//shared/bytesutil:go_default_library",
         "//shared/testutil:go_default_library",
         "@com_github_ethereum_go_ethereum//common:go_default_library",
         "@com_github_gogo_protobuf//proto:go_default_library",

beacon-chain/db/kv/checkpoint.go

@@ -2,13 +2,17 @@ package kv
 
 import (
 	"context"
+	"errors"
 
 	"github.com/boltdb/bolt"
 	"github.com/gogo/protobuf/proto"
 	ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1"
+	"github.com/prysmaticlabs/prysm/shared/traceutil"
 	"go.opencensus.io/trace"
 )
 
+var errMissingStateForFinalizedCheckpoint = errors.New("no state exists with checkpoint root")
+
 // JustifiedCheckpoint returns the latest justified checkpoint in beacon chain.
 func (k *Store) JustifiedCheckpoint(ctx context.Context) (*ethpb.Checkpoint, error) {
 	ctx, span := trace.StartSpan(ctx, "BeaconDB.JustifiedCheckpoint")
@@ -75,6 +79,13 @@ func (k *Store) SaveFinalizedCheckpoint(ctx context.Context, checkpoint *ethpb.C
 	}
 	return k.db.Update(func(tx *bolt.Tx) error {
 		bucket := tx.Bucket(checkpointBucket)
+		// The corresponding state must exist or there is a risk that the beacondb enters a state
+		// where the finalized beaconState is missing. This would be a fatal condition requiring
+		// a new sync from genesis.
+		if tx.Bucket(stateBucket).Get(checkpoint.Root) == nil {
+			traceutil.AnnotateError(span, errMissingStateForFinalizedCheckpoint)
+			return errMissingStateForFinalizedCheckpoint
+		}
 		return bucket.Put(finalizedCheckpointKey, enc)
 	})
 }

beacon-chain/db/kv/checkpoint_test.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/gogo/protobuf/proto"
 	ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1"
+	pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
+	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 )
 
 func TestStore_JustifiedCheckpoint_CanSaveRetrieve(t *testing.T) {
@@ -34,9 +36,15 @@ func TestStore_FinalizedCheckpoint_CanSaveRetrieve(t *testing.T) {
 	db := setupDB(t)
 	defer teardownDB(t, db)
 	ctx := context.Background()
+	root := bytesutil.ToBytes32([]byte{'B'})
 	cp := &ethpb.Checkpoint{
 		Epoch: 5,
-		Root:  []byte{'B'},
+		Root:  root[:],
+	}
+
+	// a state is required to save checkpoint
+	if err := db.SaveState(ctx, &pb.BeaconState{}, root); err != nil {
+		t.Fatal(err)
 	}
 
 	if err := db.SaveFinalizedCheckpoint(ctx, cp); err != nil {
@@ -91,3 +99,17 @@ func TestStore_FinalizedCheckpoint_DefaultCantBeNil(t *testing.T) {
 		t.Errorf("Wanted %v, received %v", cp, retrieved)
 	}
 }
+
+func TestStore_FinalizedCheckpoint_StateMustExist(t *testing.T) {
+	db := setupDB(t)
+	defer teardownDB(t, db)
+	ctx := context.Background()
+	cp := &ethpb.Checkpoint{
+		Epoch: 5,
+		Root:  []byte{'B'},
+	}
+
+	if err := db.SaveFinalizedCheckpoint(ctx, cp);  err != errMissingStateForFinalizedCheckpoint {
+		t.Fatalf("wanted err %v, got %v", errMissingStateForFinalizedCheckpoint, err)
+	}
+}

beacon-chain/db/kv/state_test.go

@@ -188,14 +188,14 @@ func TestStore_DeleteFinalizedState(t *testing.T) {
 	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)
 	}
+	finalizedCheckpoint := &ethpb.Checkpoint{Root: finalizedBlockRoot[:]}
+	if err := db.SaveFinalizedCheckpoint(ctx, finalizedCheckpoint); 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")