Update verifyBlkPreState to verify only (#6506)

* Verify should not return the state
* Update tests
* Merge branch 'master' of github.com:prysmaticlabs/prysm into refactor
* Sync with master
* Merge refs/heads/master into refactor
* Fixed error msgs. Thanks @rauljordan
* Merge branch 'master' of github.com:prysmaticlabs/prysm into refactor
* Merge branch 'refactor' of github.com:prysmaticlabs/prysm into refactor
* Merge refs/heads/master into refactor
* Merge refs/heads/master into refactor
* Merge refs/heads/master into refactor
This commit is contained in:
terence tsao
2020-07-07 19:54:55 -07:00
committed by GitHub
parent 1067800430
commit d42d685f78
3 changed files with 27 additions and 32 deletions

View File

@@ -182,10 +182,18 @@ func (s *Service) onBlockInitialSyncStateTransition(ctx context.Context, signed
b := signed.Block
// Retrieve incoming block's pre state.
preState, err := s.verifyBlkPreState(ctx, b)
if err := s.verifyBlkPreState(ctx, b); err != nil {
return err
}
preState, err := s.stateGen.StateByRootInitialSync(ctx, bytesutil.ToBytes32(signed.Block.ParentRoot))
if err != nil {
return err
}
if preState == nil {
return fmt.Errorf("nil pre state for slot %d", b.Slot)
}
// To invalidate cache for parent root because pre state will get mutated.
s.stateGen.DeleteHotStateInCache(bytesutil.ToBytes32(b.ParentRoot))
@@ -220,12 +228,16 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []*ethpb.SignedBeaconBl
b := blks[0].Block
// Retrieve incoming block's pre state.
preState, err := s.verifyBlkPreState(ctx, b)
if err := s.verifyBlkPreState(ctx, b); err != nil {
return nil, nil, nil, err
}
preState, err := s.stateGen.StateByRootInitialSync(ctx, bytesutil.ToBytes32(b.ParentRoot))
if err != nil {
return nil, nil, nil, err
}
// Perform a copy to preserve copy in cache.
preState = preState.Copy()
if preState == nil {
return nil, nil, nil, fmt.Errorf("nil pre state for slot %d", b.Slot)
}
jCheckpoints := make([]*ethpb.Checkpoint, len(blks))
fCheckpoints := make([]*ethpb.Checkpoint, len(blks))

View File

@@ -36,12 +36,11 @@ func (s *Service) getBlockPreState(ctx context.Context, b *ethpb.BeaconBlock) (*
defer span.End()
// Verify incoming block has a valid pre state.
preState, err := s.verifyBlkPreState(ctx, b)
if err != nil {
if err := s.verifyBlkPreState(ctx, b); err != nil {
return nil, err
}
preState, err = s.stateGen.StateByRoot(ctx, bytesutil.ToBytes32(b.ParentRoot))
preState, err := s.stateGen.StateByRoot(ctx, bytesutil.ToBytes32(b.ParentRoot))
if err != nil {
return nil, errors.Wrapf(err, "could not get pre state for slot %d", b.Slot)
}
@@ -68,7 +67,7 @@ func (s *Service) getBlockPreState(ctx context.Context, b *ethpb.BeaconBlock) (*
}
// verifyBlkPreState validates input block has a valid pre-state.
func (s *Service) verifyBlkPreState(ctx context.Context, b *ethpb.BeaconBlock) (*stateTrie.BeaconState, error) {
func (s *Service) verifyBlkPreState(ctx context.Context, b *ethpb.BeaconBlock) error {
ctx, span := trace.StartSpan(ctx, "chainService.verifyBlkPreState")
defer span.End()
@@ -77,23 +76,15 @@ func (s *Service) verifyBlkPreState(ctx context.Context, b *ethpb.BeaconBlock) (
// during initial syncing. There's no risk given a state summary object is just a
// a subset of the block object.
if !s.stateGen.StateSummaryExists(ctx, parentRoot) && !s.beaconDB.HasBlock(ctx, parentRoot) {
return nil, errors.New("could not reconstruct parent state")
return errors.New("could not reconstruct parent state")
}
if !s.stateGen.HasState(ctx, parentRoot) {
if err := s.beaconDB.SaveBlocks(ctx, s.getInitSyncBlocks()); err != nil {
return nil, errors.Wrap(err, "could not save initial sync blocks")
return errors.Wrap(err, "could not save initial sync blocks")
}
s.clearInitSyncBlocks()
}
preState, err := s.stateGen.StateByRootInitialSync(ctx, parentRoot)
if err != nil {
return nil, errors.Wrapf(err, "could not get pre state for slot %d", b.Slot)
}
if preState == nil {
return nil, errors.Wrapf(err, "nil pre state for slot %d", b.Slot)
}
return preState, nil // No copy needed from newly hydrated state gen object.
return nil
}
// verifyBlkDescendant validates input block root is a descendant of the

View File

@@ -2,13 +2,13 @@ package blockchain
import (
"context"
"reflect"
"strings"
"testing"
"time"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"github.com/prysmaticlabs/go-ssz"
"github.com/prysmaticlabs/prysm/beacon-chain/cache"
"github.com/prysmaticlabs/prysm/beacon-chain/core/blocks"
"github.com/prysmaticlabs/prysm/beacon-chain/core/state"
"github.com/prysmaticlabs/prysm/beacon-chain/db"
@@ -313,13 +313,9 @@ func TestCachedPreState_CanGetFromStateSummary(t *testing.T) {
t.Fatal(err)
}
received, err := service.verifyBlkPreState(ctx, b)
if err != nil {
if err := service.verifyBlkPreState(ctx, b); err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(s.InnerStateUnsafe(), received.InnerStateUnsafe()) {
t.Error("cached state not the same")
}
}
func TestCachedPreState_CanGetFromDB(t *testing.T) {
@@ -342,7 +338,7 @@ func TestCachedPreState_CanGetFromDB(t *testing.T) {
b := &ethpb.BeaconBlock{Slot: 1, ParentRoot: r[:]}
service.finalizedCheckpt = &ethpb.Checkpoint{Root: r[:]}
_, err = service.verifyBlkPreState(ctx, b)
err = service.verifyBlkPreState(ctx, b)
wanted := "could not reconstruct parent state"
if err.Error() != wanted {
t.Error("Did not get wanted error")
@@ -359,20 +355,16 @@ func TestCachedPreState_CanGetFromDB(t *testing.T) {
t.Fatal(err)
}
received, err := service.verifyBlkPreState(ctx, b)
if err != nil {
if err := service.verifyBlkPreState(ctx, b); err != nil {
t.Fatal(err)
}
if s.Slot() != received.Slot() {
t.Error("cached state not the same")
}
}
func TestUpdateJustified_CouldUpdateBest(t *testing.T) {
ctx := context.Background()
db, _ := testDB.SetupDB(t)
cfg := &Config{BeaconDB: db}
cfg := &Config{BeaconDB: db, StateGen: stategen.New(db, cache.NewStateSummaryCache())}
service, err := NewService(ctx, cfg)
if err != nil {
t.Fatal(err)