From 2d98902eed23dfa6c09f7fd33ee568a5fa56ccc8 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Wed, 23 Oct 2019 22:50:33 -0700 Subject: [PATCH] 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 | 6 +++--- beacon-chain/db/kv/BUILD.bazel | 1 + beacon-chain/db/kv/checkpoint.go | 11 +++++++++++ beacon-chain/db/kv/checkpoint_test.go | 24 +++++++++++++++++++++++- beacon-chain/db/kv/state_test.go | 8 ++++---- 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/beacon-chain/blockchain/service_test.go b/beacon-chain/blockchain/service_test.go index dff4a733258f..dd6b1720aa66 100644 --- a/beacon-chain/blockchain/service_test.go +++ b/beacon-chain/blockchain/service_test.go @@ -358,15 +358,15 @@ func TestChainService_InitializeChainInfo(t *testing.T) { headBlock := ðpb.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, ðpb.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) } diff --git a/beacon-chain/db/kv/BUILD.bazel b/beacon-chain/db/kv/BUILD.bazel index 0131eaf3fdd7..b6d29c4a3ee6 100644 --- a/beacon-chain/db/kv/BUILD.bazel +++ b/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", diff --git a/beacon-chain/db/kv/checkpoint.go b/beacon-chain/db/kv/checkpoint.go index e5fc9b403ac3..7ad56bf61326 100644 --- a/beacon-chain/db/kv/checkpoint.go +++ b/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) }) } diff --git a/beacon-chain/db/kv/checkpoint_test.go b/beacon-chain/db/kv/checkpoint_test.go index cc195bce365c..50b55d036386 100644 --- a/beacon-chain/db/kv/checkpoint_test.go +++ b/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 := ðpb.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 := ðpb.Checkpoint{ + Epoch: 5, + Root: []byte{'B'}, + } + + if err := db.SaveFinalizedCheckpoint(ctx, cp); err != errMissingStateForFinalizedCheckpoint { + t.Fatalf("wanted err %v, got %v", errMissingStateForFinalizedCheckpoint, err) + } +} diff --git a/beacon-chain/db/kv/state_test.go b/beacon-chain/db/kv/state_test.go index 0dbedbb06139..9d4598e07270 100644 --- a/beacon-chain/db/kv/state_test.go +++ b/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 := ðpb.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 := ðpb.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")