Beacon State: More consistent nil return for state (#4854)

* More consistent nil return for state
* Merge refs/heads/master into nil-state
* Add a check for encode(nil)
* Merge branch 'nil-state' of github.com:prysmaticlabs/prysm into nil-state
* fix test, thanks @rauljordan
* fix tests
* gofmt

beacon-chain/blockchain/process_block_test.go

@@ -42,7 +42,11 @@ func TestStore_OnBlock(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, validGenesisRoot); err != nil {
+	st, err := stateTrie.InitializeFromProtoUnsafe(&pb.BeaconState{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), validGenesisRoot); err != nil {
 		t.Fatal(err)
 	}
 	roots, err := blockTree1(db, validGenesisRoot[:])
@@ -57,11 +61,11 @@ func TestStore_OnBlock(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, randomParentRoot); err != nil {
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), randomParentRoot); err != nil {
 		t.Fatal(err)
 	}
 	randomParentRoot2 := roots[1]
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, bytesutil.ToBytes32(randomParentRoot2)); err != nil {
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), bytesutil.ToBytes32(randomParentRoot2)); err != nil {
 		t.Fatal(err)
 	}
 
@@ -75,25 +79,25 @@ func TestStore_OnBlock(t *testing.T) {
 		{
 			name:          "parent block root does not have a state",
 			blk:           &ethpb.BeaconBlock{},
-			s:             &stateTrie.BeaconState{},
+			s:             st.Copy(),
 			wantErrString: "pre state of slot 0 does not exist",
 		},
 		{
 			name:          "block is from the feature",
 			blk:           &ethpb.BeaconBlock{ParentRoot: randomParentRoot[:], Slot: params.BeaconConfig().FarFutureEpoch},
-			s:             &stateTrie.BeaconState{},
+			s:             st.Copy(),
 			wantErrString: "could not process slot from the future",
 		},
 		{
 			name:          "could not get finalized block",
 			blk:           &ethpb.BeaconBlock{ParentRoot: randomParentRoot[:]},
-			s:             &stateTrie.BeaconState{},
+			s:             st.Copy(),
 			wantErrString: "block from slot 0 is not a descendent of the current finalized block",
 		},
 		{
 			name:          "same slot as finalized block",
 			blk:           &ethpb.BeaconBlock{Slot: 0, ParentRoot: randomParentRoot2},
-			s:             &stateTrie.BeaconState{},
+			s:             st.Copy(),
 			wantErrString: "block is equal or earlier than finalized block, slot 0 < slot 0",
 		},
 	}
@@ -455,8 +459,12 @@ func TestUpdateJustified_CouldUpdateBest(t *testing.T) {
 	}
 	service.justifiedCheckpt = &ethpb.Checkpoint{Root: []byte{'A'}}
 	service.bestJustifiedCheckpt = &ethpb.Checkpoint{Root: []byte{'A'}}
-	service.initSyncState[r] = &stateTrie.BeaconState{}
-	if err := db.SaveState(ctx, &stateTrie.BeaconState{}, r); err != nil {
+	st, err := stateTrie.InitializeFromProtoUnsafe(&pb.BeaconState{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	service.initSyncState[r] = st.Copy()
+	if err := db.SaveState(ctx, st.Copy(), r); err != nil {
 		t.Fatal(err)
 	}
 
@@ -496,10 +504,11 @@ func TestFilterBlockRoots_CanFilter(t *testing.T) {
 	fRoot, _ := ssz.HashTreeRoot(fBlock)
 	hBlock := &ethpb.BeaconBlock{Slot: 1}
 	headRoot, _ := ssz.HashTreeRoot(hBlock)
+	st, _ := stateTrie.InitializeFromProtoUnsafe(&pb.BeaconState{})
 	if err := service.beaconDB.SaveBlock(ctx, &ethpb.SignedBeaconBlock{Block: fBlock}); err != nil {
 		t.Fatal(err)
 	}
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, fRoot); err != nil {
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), fRoot); err != nil {
 		t.Fatal(err)
 	}
 	if err := service.beaconDB.SaveFinalizedCheckpoint(ctx, &ethpb.Checkpoint{Root: fRoot[:]}); err != nil {
@@ -508,7 +517,7 @@ func TestFilterBlockRoots_CanFilter(t *testing.T) {
 	if err := service.beaconDB.SaveBlock(ctx, &ethpb.SignedBeaconBlock{Block: hBlock}); err != nil {
 		t.Fatal(err)
 	}
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, headRoot); err != nil {
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), headRoot); err != nil {
 		t.Fatal(err)
 	}
 	if err := service.beaconDB.SaveHeadBlockRoot(ctx, headRoot); err != nil {
@@ -550,7 +559,8 @@ func TestFillForkChoiceMissingBlocks_CanSave(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, validGenesisRoot); err != nil {
+	st, _ := stateTrie.InitializeFromProtoUnsafe(&pb.BeaconState{})
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), validGenesisRoot); err != nil {
 		t.Fatal(err)
 	}
 	roots, err := blockTree1(db, validGenesisRoot[:])
@@ -603,7 +613,8 @@ func TestFillForkChoiceMissingBlocks_FilterFinalized(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	if err := service.beaconDB.SaveState(ctx, &stateTrie.BeaconState{}, validGenesisRoot); err != nil {
+	st, _ := stateTrie.InitializeFromProtoUnsafe(&pb.BeaconState{})
+	if err := service.beaconDB.SaveState(ctx, st.Copy(), validGenesisRoot); err != nil {
 		t.Fatal(err)
 	}
 
@@ -661,21 +672,25 @@ func blockTree1(db db.Database, genesisRoot []byte) ([][]byte, error) {
 	r7, _ := ssz.HashTreeRoot(b7)
 	b8 := &ethpb.BeaconBlock{Slot: 8, ParentRoot: r6[:]}
 	r8, _ := ssz.HashTreeRoot(b8)
+	st, err := stateTrie.InitializeFromProtoUnsafe(&pb.BeaconState{})
+	if err != nil {
+		return nil, err
+	}
 	for _, b := range []*ethpb.BeaconBlock{b0, b1, b3, b4, b5, b6, b7, b8} {
 		if err := db.SaveBlock(context.Background(), &ethpb.SignedBeaconBlock{Block: b}); err != nil {
 			return nil, err
 		}
-		if err := db.SaveState(context.Background(), &stateTrie.BeaconState{}, bytesutil.ToBytes32(b.ParentRoot)); err != nil {
+		if err := db.SaveState(context.Background(), st.Copy(), bytesutil.ToBytes32(b.ParentRoot)); err != nil {
 			return nil, err
 		}
 	}
-	if err := db.SaveState(context.Background(), &stateTrie.BeaconState{}, r1); err != nil {
+	if err := db.SaveState(context.Background(), st.Copy(), r1); err != nil {
 		return nil, err
 	}
-	if err := db.SaveState(context.Background(), &stateTrie.BeaconState{}, r7); err != nil {
+	if err := db.SaveState(context.Background(), st.Copy(), r7); err != nil {
 		return nil, err
 	}
-	if err := db.SaveState(context.Background(), &stateTrie.BeaconState{}, r8); err != nil {
+	if err := db.SaveState(context.Background(), st.Copy(), r8); err != nil {
 		return nil, err
 	}
 	return [][]byte{r0[:], r1[:], nil, r3[:], r4[:], r5[:], r6[:], r7[:], r8[:]}, nil

beacon-chain/db/kv/BUILD.bazel

@@ -57,6 +57,7 @@ go_test(
         "blocks_test.go",
         "checkpoint_test.go",
         "deposit_contract_test.go",
+        "encoding_test.go",
         "finalized_block_roots_test.go",
         "kv_test.go",
         "operations_test.go",
@@ -69,6 +70,7 @@ go_test(
         "//beacon-chain/db/filters:go_default_library",
         "//beacon-chain/state:go_default_library",
         "//proto/beacon/p2p/v1:go_default_library",
+        "//proto/testing:go_default_library",
         "//shared/bytesutil:go_default_library",
         "//shared/params:go_default_library",
         "//shared/testutil:go_default_library",

beacon-chain/db/kv/encoding.go

@@ -2,6 +2,7 @@ package kv
 
 import (
 	"errors"
+	"reflect"
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/golang/snappy"
@@ -19,7 +20,7 @@ func decode(data []byte, dst proto.Message) error {
 }
 
 func encode(msg proto.Message) ([]byte, error) {
-	if msg == nil {
+	if msg == nil || reflect.ValueOf(msg).IsNil() {
 		return nil, errors.New("cannot encode nil message")
 	}
 	enc, err := proto.Marshal(msg)

beacon-chain/db/kv/encoding_test.go

@@ -0,0 +1,17 @@
+package kv
+
+import (
+	"testing"
+
+	testpb "github.com/prysmaticlabs/prysm/proto/testing"
+)
+
+func Test_encode_handlesNilFromFunction(t *testing.T) {
+	foo := func() *testpb.Puzzle {
+		return nil
+	}
+	_, err := encode(foo())
+	if err == nil || err.Error() != "cannot encode nil message" {
+		t.Fatalf("Wrong error %v", err)
+	}
+}

beacon-chain/rpc/validator/proposer_test.go

@@ -150,7 +150,6 @@ func TestGetBlock_AddsUnaggregatedAtts(t *testing.T) {
 		ExitPool:          voluntaryexits.NewPool(),
 	}
 
-
 	// Generate a bunch of random attestations at slot. These would be considered double votes, but
 	// we don't care for the purpose of this test.
 	var atts []*ethpb.Attestation

beacon-chain/rpc/validator/server_test.go

@@ -39,7 +39,8 @@ func TestValidatorIndex_OK(t *testing.T) {
 	db := dbutil.SetupDB(t)
 	defer dbutil.TeardownDB(t, db)
 	ctx := context.Background()
-	if err := db.SaveState(ctx, &stateTrie.BeaconState{}, [32]byte{}); err != nil {
+	st, _ := stateTrie.InitializeFromProtoUnsafe(&pbp2p.BeaconState{})
+	if err := db.SaveState(ctx, st.Copy(), [32]byte{}); err != nil {
 		t.Fatal(err)
 	}
 

beacon-chain/state/getters.go

@@ -63,15 +63,15 @@ func (v *ReadOnlyValidator) Slashed() bool {
 // InnerStateUnsafe returns the pointer value of the underlying
 // beacon state proto object, bypassing immutability. Use with care.
 func (b *BeaconState) InnerStateUnsafe() *pbp2p.BeaconState {
-	if b.state == nil {
-		return &pbp2p.BeaconState{}
+	if b == nil {
+		return nil
 	}
 	return b.state
 }
 
 // CloneInnerState the beacon state into a protobuf for usage.
 func (b *BeaconState) CloneInnerState() *pbp2p.BeaconState {
-	if b.state == nil {
+	if b == nil || b.state == nil {
 		return nil
 	}
 	return &pbp2p.BeaconState{