Minor fixes (#4716)

* copy state in cache, ensure pre state exists before attepting to process attestation in fork choice * copy state in cache, ensure pre state exists before attepting to process attestation in fork choice * fix test
prysmaticlabs · Feb 2, 2020 · bd334c4 · bd334c4
1 parent 4f38333
commit bd334c4
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 16 deletions.
diff --git a/beacon-chain/blockchain/forkchoice/process_attestation.go b/beacon-chain/blockchain/forkchoice/process_attestation.go
@@ -166,10 +166,9 @@ func (s *Store) getAttPreState(ctx context.Context, c *ethpb.Checkpoint) (*state
 		return nil, fmt.Errorf("pre state of target block %d does not exist", helpers.StartSlot(c.Epoch))
 	}
 
-	stateCopy := baseState.Copy()
 
-	if helpers.StartSlot(c.Epoch) > stateCopy.Slot() {
-		stateCopy, err = state.ProcessSlots(ctx, stateCopy, helpers.StartSlot(c.Epoch))
+	if helpers.StartSlot(c.Epoch) > baseState.Slot() {
+		baseState, err = state.ProcessSlots(ctx, baseState, helpers.StartSlot(c.Epoch))
 		if err != nil {
 			return nil, errors.Wrapf(err, "could not process slots up to %d", helpers.StartSlot(c.Epoch))
 		}
@@ -178,12 +177,12 @@ func (s *Store) getAttPreState(ctx context.Context, c *ethpb.Checkpoint) (*state
 
 	if err := s.checkpointState.AddCheckpointState(&cache.CheckpointState{
 		Checkpoint: c,
-		State:      stateCopy,
+		State:      baseState.Copy(),
 	}); err != nil {
 		return nil, errors.Wrap(err, "could not saved checkpoint state to cache")
 	}
 
-	return stateCopy, nil
+	return baseState, nil
 }
 
 // verifyAttTargetEpoch validates attestation is from the current or previous epoch.

diff --git a/beacon-chain/blockchain/forkchoice/service.go b/beacon-chain/blockchain/forkchoice/service.go
@@ -106,7 +106,7 @@ func (s *Store) GenesisStore(
 
 	if err := s.checkpointState.AddCheckpointState(&cache.CheckpointState{
 		Checkpoint: s.justifiedCheckpt,
-		State:      justifiedState,
+		State:      justifiedState.Copy(),
 	}); err != nil {
 		return errors.Wrap(err, "could not save genesis state in check point cache")
 	}

diff --git a/beacon-chain/blockchain/process_attestation_helpers.go b/beacon-chain/blockchain/process_attestation_helpers.go
@@ -36,23 +36,22 @@ func (s *Service) getAttPreState(ctx context.Context, c *ethpb.Checkpoint) (*sta
 		return nil, fmt.Errorf("pre state of target block %d does not exist", helpers.StartSlot(c.Epoch))
 	}
 
-	stateCopy := baseState.Copy()
 
-	if helpers.StartSlot(c.Epoch) > stateCopy.Slot() {
-		stateCopy, err = state.ProcessSlots(ctx, stateCopy, helpers.StartSlot(c.Epoch))
+	if helpers.StartSlot(c.Epoch) > baseState.Slot() {
+		baseState, err = state.ProcessSlots(ctx, baseState, helpers.StartSlot(c.Epoch))
 		if err != nil {
 			return nil, errors.Wrapf(err, "could not process slots up to %d", helpers.StartSlot(c.Epoch))
 		}
 	}
 
 	if err := s.checkpointState.AddCheckpointState(&cache.CheckpointState{
 		Checkpoint: c,
-		State:      stateCopy,
+		State:      baseState.Copy(),
 	}); err != nil {
 		return nil, errors.Wrap(err, "could not saved checkpoint state to cache")
 	}
 
-	return stateCopy, nil
+	return baseState, nil
 }
 
 // verifyAttTargetEpoch validates attestation is from the current or previous epoch.

diff --git a/beacon-chain/blockchain/receive_attestation.go b/beacon-chain/blockchain/receive_attestation.go
@@ -96,7 +96,7 @@ func (s *Service) processAttestation() {
 			ctx := context.Background()
 			atts := s.attPool.ForkchoiceAttestations()
 			for _, a := range atts {
-				hasState := s.beaconDB.HasState(ctx, bytesutil.ToBytes32(a.Data.BeaconBlockRoot))
+				hasState := s.beaconDB.HasState(ctx, bytesutil.ToBytes32(a.Data.BeaconBlockRoot)) && s.beaconDB.HasState(ctx, bytesutil.ToBytes32(a.Data.Target.Root))
 				hasBlock := s.beaconDB.HasBlock(ctx, bytesutil.ToBytes32(a.Data.BeaconBlockRoot))
 				if !(hasState && hasBlock) {
 					continue

diff --git a/beacon-chain/cache/checkpoint_state.go b/beacon-chain/cache/checkpoint_state.go
@@ -91,7 +91,7 @@ func (c *CheckpointStateCache) StateByCheckpoint(cp *ethpb.Checkpoint) (*stateTr
 		return nil, ErrNotCheckpointState
 	}
 
-	return info.State, nil
+	return info.State.Copy(), nil
 }
 
 // AddCheckpointState adds CheckpointState object to the cache. This method also trims the least

diff --git a/beacon-chain/cache/checkpoint_state_test.go b/beacon-chain/cache/checkpoint_state_test.go
@@ -71,7 +71,7 @@ func TestCheckpointStateCache_StateByCheckpoint(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !reflect.DeepEqual(state, info1.State) {
+	if !reflect.DeepEqual(state.InnerStateUnsafe(), info1.State.InnerStateUnsafe()) {
 		t.Error("incorrectly cached state")
 	}
 
@@ -93,15 +93,15 @@ func TestCheckpointStateCache_StateByCheckpoint(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !reflect.DeepEqual(state, info2.State) {
+	if !reflect.DeepEqual(state.InnerStateUnsafe(), info2.State.InnerStateUnsafe()) {
 		t.Error("incorrectly cached state")
 	}
 
 	state, err = cache.StateByCheckpoint(cp1)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !reflect.DeepEqual(state, info1.State) {
+	if !reflect.DeepEqual(state.InnerStateUnsafe(), info1.State.InnerStateUnsafe()) {
 		t.Error("incorrectly cached state")
 	}
 }