Skip to content

Commit

Permalink
Cleanup of stategen package (#10607)
Browse files Browse the repository at this point in the history 
* powchain and stategen

* revert powchain changes

* rename field to blockRootsOfSavedStates

* rename params to blockRoot

* review feedback

* fix loop

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
  • Loading branch information
@rkapka @rauljordan @prylabs-bulldozer
3 people committed May 11, 2022
1 parent e242439 commit 72e61c5
Show file tree
Hide file tree
Showing 15 changed files with 121 additions and 113 deletions.
1 change: 1 addition & 0 deletions beacon-chain/blockchain/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ func (s *Service) Stop() error {
defer s.cancel()

if s.cfg.StateGen != nil && s.head != nil && s.head.state != nil {
// Save the last finalized state so that starting up in the following run will be much faster.
if err := s.cfg.StateGen.ForceCheckpoint(s.ctx, s.head.state.FinalizedCheckpoint().Root); err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions beacon-chain/state/stategen/cacher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import (
var ErrNotInCache = errors.New("state not found in cache")

type CachedGetter interface {
ByRoot([32]byte) (state.BeaconState, error)
ByBlockRoot([32]byte) (state.BeaconState, error)
}

type CombinedCache struct {
getters []CachedGetter
}

func (c CombinedCache) ByRoot(root [32]byte) (state.BeaconState, error) {
func (c CombinedCache) ByBlockRoot(r [32]byte) (state.BeaconState, error) {
for _, getter := range c.getters {
st, err := getter.ByRoot(root)
st, err := getter.ByBlockRoot(r)
if err == nil {
return st, nil
}
Expand Down
30 changes: 15 additions & 15 deletions beacon-chain/state/stategen/epoch_boundary_state_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ var (

// slotRootInfo specifies the slot root info in the epoch boundary state cache.
type slotRootInfo struct {
slot types.Slot
root [32]byte
slot types.Slot
blockRoot [32]byte
}

// slotKeyFn takes the string representation of the slot to be used as key
Expand Down Expand Up @@ -66,9 +66,9 @@ func newBoundaryStateCache() *epochBoundaryState {
}
}

// ByRoot satisfies the CachedGetter interface
func (e *epochBoundaryState) ByRoot(r [32]byte) (state.BeaconState, error) {
rsi, ok, err := e.getByRoot(r)
// ByBlockRoot satisfies the CachedGetter interface
func (e *epochBoundaryState) ByBlockRoot(r [32]byte) (state.BeaconState, error) {
rsi, ok, err := e.getByBlockRoot(r)
if err != nil {
return nil, err
}
Expand All @@ -79,14 +79,14 @@ func (e *epochBoundaryState) ByRoot(r [32]byte) (state.BeaconState, error) {
}

// get epoch boundary state by its block root. Returns copied state in state info object if exists. Otherwise returns nil.
func (e *epochBoundaryState) getByRoot(r [32]byte) (*rootStateInfo, bool, error) {
func (e *epochBoundaryState) getByBlockRoot(r [32]byte) (*rootStateInfo, bool, error) {
e.lock.RLock()
defer e.lock.RUnlock()

return e.getByRootLockFree(r)
return e.getByBlockRootLockFree(r)
}

func (e *epochBoundaryState) getByRootLockFree(r [32]byte) (*rootStateInfo, bool, error) {
func (e *epochBoundaryState) getByBlockRootLockFree(r [32]byte) (*rootStateInfo, bool, error) {
obj, exists, err := e.rootStateCache.GetByKey(string(r[:]))
if err != nil {
return nil, false, err
Expand Down Expand Up @@ -122,24 +122,24 @@ func (e *epochBoundaryState) getBySlot(s types.Slot) (*rootStateInfo, bool, erro
return nil, false, errNotSlotRootInfo
}

return e.getByRootLockFree(info.root)
return e.getByBlockRootLockFree(info.blockRoot)
}

// put adds a state to the epoch boundary state cache. This method also trims the
// least recently added state info if the cache size has reached the max cache
// size limit.
func (e *epochBoundaryState) put(r [32]byte, s state.BeaconState) error {
func (e *epochBoundaryState) put(blockRoot [32]byte, s state.BeaconState) error {
e.lock.Lock()
defer e.lock.Unlock()

if err := e.slotRootCache.AddIfNotPresent(&slotRootInfo{
slot: s.Slot(),
root: r,
slot: s.Slot(),
blockRoot: blockRoot,
}); err != nil {
return err
}
if err := e.rootStateCache.AddIfNotPresent(&rootStateInfo{
root: r,
root: blockRoot,
state: s.Copy(),
}); err != nil {
return err
Expand All @@ -152,11 +152,11 @@ func (e *epochBoundaryState) put(r [32]byte, s state.BeaconState) error {
}

// delete the state from the epoch boundary state cache.
func (e *epochBoundaryState) delete(r [32]byte) error {
func (e *epochBoundaryState) delete(blockRoot [32]byte) error {
e.lock.Lock()
defer e.lock.Unlock()
return e.rootStateCache.Delete(&rootStateInfo{
root: r,
root: blockRoot,
})
}

Expand Down
6 changes: 3 additions & 3 deletions beacon-chain/state/stategen/epoch_boundary_state_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ func TestEpochBoundaryStateCache_CanSaveAndDelete(t *testing.T) {
r := [32]byte{'a'}
require.NoError(t, e.put(r, s))

got, exists, err := e.getByRoot([32]byte{'b'})
got, exists, err := e.getByBlockRoot([32]byte{'b'})
require.NoError(t, err)
assert.Equal(t, false, exists, "Should not exist")
assert.Equal(t, (*rootStateInfo)(nil), got, "Should not exist")

got, exists, err = e.getByRoot([32]byte{'a'})
got, exists, err = e.getByBlockRoot([32]byte{'a'})
require.NoError(t, err)
assert.Equal(t, true, exists, "Should exist")
assert.DeepSSZEqual(t, s.InnerStateUnsafe(), got.state.InnerStateUnsafe(), "Should have the same state")
Expand All @@ -48,7 +48,7 @@ func TestEpochBoundaryStateCache_CanSaveAndDelete(t *testing.T) {
assert.DeepSSZEqual(t, s.InnerStateUnsafe(), got.state.InnerStateUnsafe(), "Should have the same state")

require.NoError(t, e.delete(r))
got, exists, err = e.getByRoot([32]byte{'b'})
got, exists, err = e.getByBlockRoot([32]byte{'b'})
require.NoError(t, err)
assert.Equal(t, false, exists, "Should not exist")
assert.Equal(t, (*rootStateInfo)(nil), got, "Should not exist")
Expand Down
62 changes: 31 additions & 31 deletions beacon-chain/state/stategen/getter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,31 @@ func (s *State) HasState(ctx context.Context, blockRoot [32]byte) (bool, error)
}

// HasStateInCache returns true if the state exists in cache.
func (s *State) HasStateInCache(ctx context.Context, blockRoot [32]byte) (bool, error) {
func (s *State) HasStateInCache(_ context.Context, blockRoot [32]byte) (bool, error) {
if s.hotStateCache.has(blockRoot) {
return true, nil
}
_, has, err := s.epochBoundaryStateCache.getByRoot(blockRoot)
_, has, err := s.epochBoundaryStateCache.getByBlockRoot(blockRoot)
if err != nil {
return false, err
}
return has, nil
}

// StateByRootIfCachedNoCopy retrieves a state using the input block root only if the state is already in the cache
// StateByRootIfCachedNoCopy retrieves a state using the input block root only if the state is already in the cache.
func (s *State) StateByRootIfCachedNoCopy(blockRoot [32]byte) state.BeaconState {
if !s.hotStateCache.has(blockRoot) {
return nil
}
state := s.hotStateCache.getWithoutCopy(blockRoot)
return state
return s.hotStateCache.getWithoutCopy(blockRoot)
}

// StateByRoot retrieves the state using input block root.
func (s *State) StateByRoot(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) {
ctx, span := trace.StartSpan(ctx, "stateGen.StateByRoot")
defer span.End()

// Genesis case. If block root is zero hash, short circuit to use genesis cachedState stored in DB.
// Genesis case. If block root is zero hash, short circuit to use genesis state stored in DB.
if blockRoot == params.BeaconConfig().ZeroHash {
return s.beaconDB.GenesisState(ctx)
}
Expand All @@ -62,23 +61,25 @@ func (s *State) StateByRoot(ctx context.Context, blockRoot [32]byte) (state.Beac

// StateByRootInitialSync retrieves the state from the DB for the initial syncing phase.
// It assumes initial syncing using a block list rather than a block tree hence the returned
// state is not copied.
// It invalidates cache for parent root because pre state will get mutated.
// Do not use this method for anything other than initial syncing purpose or block tree is applied.
// state is not copied (block batches returned from initial sync are linear).
// It invalidates cache for parent root because pre-state will get mutated.
//
// WARNING: Do not use this method for anything other than initial syncing purpose or block tree is applied.
func (s *State) StateByRootInitialSync(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) {
// Genesis case. If block root is zero hash, short circuit to use genesis state stored in DB.
if blockRoot == params.BeaconConfig().ZeroHash {
return s.beaconDB.GenesisState(ctx)
}

// To invalidate cache for parent root because pre state will get mutated.
// To invalidate cache for parent root because pre-state will get mutated.
// It is a parent root because StateByRootInitialSync is always used to fetch the block's parent state.
defer s.hotStateCache.delete(blockRoot)

if s.hotStateCache.has(blockRoot) {
return s.hotStateCache.getWithoutCopy(blockRoot), nil
}

cachedInfo, ok, err := s.epochBoundaryStateCache.getByRoot(blockRoot)
cachedInfo, ok, err := s.epochBoundaryStateCache.getByBlockRoot(blockRoot)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -113,8 +114,7 @@ func (s *State) StateByRootInitialSync(ctx context.Context, blockRoot [32]byte)
return startState, nil
}

// This returns the state summary object of a given block root, it first checks the cache
// then checks the DB. An error is returned if state summary object is nil.
// This returns the state summary object of a given block root. It first checks the cache, then checks the DB.
func (s *State) stateSummary(ctx context.Context, blockRoot [32]byte) (*ethpb.StateSummary, error) {
var summary *ethpb.StateSummary
var err error
Expand Down Expand Up @@ -152,7 +152,7 @@ func (s *State) DeleteStateFromCaches(_ context.Context, blockRoot [32]byte) err
return s.epochBoundaryStateCache.delete(blockRoot)
}

// This loads a beacon state from either the cache or DB then replay blocks up the requested block root.
// This loads a beacon state from either the cache or DB, then replays blocks up the slot of the requested block root.
func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) {
ctx, span := trace.StartSpan(ctx, "stateGen.loadStateByRoot")
defer span.End()
Expand All @@ -163,16 +163,16 @@ func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (state.
return cachedState, nil
}

// Second, it checks if the state exits in epoch boundary state cache.
cachedInfo, ok, err := s.epochBoundaryStateCache.getByRoot(blockRoot)
// Second, it checks if the state exists in epoch boundary state cache.
cachedInfo, ok, err := s.epochBoundaryStateCache.getByBlockRoot(blockRoot)
if err != nil {
return nil, err
}
if ok {
return cachedInfo.state, nil
}

// Short cut if the cachedState is already in the DB.
// Short circuit if the state is already in the DB.
if s.beaconDB.HasState(ctx, blockRoot) {
return s.beaconDB.State(ctx, blockRoot)
}
Expand All @@ -183,8 +183,8 @@ func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (state.
}
targetSlot := summary.Slot

// Since the requested state is not in caches, start replaying using the last available ancestor state which is
// retrieved using input block's parent root.
// Since the requested state is not in caches or DB, start replaying using the last
// available ancestor state which is retrieved using input block's root.
startState, err := s.LastAncestorState(ctx, blockRoot)
if err != nil {
return nil, errors.Wrap(err, "could not get ancestor state")
Expand All @@ -193,7 +193,6 @@ func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (state.
return nil, errUnknownBoundaryState
}

// Return state early if we are retrieving it from our finalized state cache.
if startState.Slot() == targetSlot {
return startState, nil
}
Expand All @@ -208,23 +207,23 @@ func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (state.
return s.ReplayBlocks(ctx, startState, blks, targetSlot)
}

// This returns the highest available ancestor state of the input block root.
// It recursively look up block's parent until a corresponding state of the block root
// LastAncestorState returns the highest available ancestor state of the input block root.
// It recursively looks up block's parent until a corresponding state of the block root
// is found in the caches or DB.
//
// There's three ways to derive block parent state:
// 1.) block parent state is the last finalized state
// 2.) block parent state is the epoch boundary state and exists in epoch boundary cache.
// 3.) block parent state is in DB.
func (s *State) LastAncestorState(ctx context.Context, root [32]byte) (state.BeaconState, error) {
// 1) block parent state is the last finalized state
// 2) block parent state is the epoch boundary state and exists in epoch boundary cache
// 3) block parent state is in DB
func (s *State) LastAncestorState(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) {
ctx, span := trace.StartSpan(ctx, "stateGen.LastAncestorState")
defer span.End()

if s.isFinalizedRoot(root) && s.finalizedState() != nil {
if s.isFinalizedRoot(blockRoot) && s.finalizedState() != nil {
return s.finalizedState(), nil
}

b, err := s.beaconDB.Block(ctx, root)
b, err := s.beaconDB.Block(ctx, blockRoot)
if err != nil {
return nil, err
}
Expand All @@ -237,13 +236,13 @@ func (s *State) LastAncestorState(ctx context.Context, root [32]byte) (state.Bea
return nil, ctx.Err()
}

// Is the state a genesis state.
// Is the state the genesis state.
parentRoot := bytesutil.ToBytes32(b.Block().ParentRoot())
if parentRoot == params.BeaconConfig().ZeroHash {
return s.beaconDB.GenesisState(ctx)
}

// return an error if slot hasn't been covered by checkpoint sync backfill
// Return an error if slot hasn't been covered by checkpoint sync.
ps := b.Block().Slot() - 1
if !s.slotAvailable(ps) {
return nil, errors.Wrapf(ErrNoDataForSlot, "slot %d not in db due to checkpoint sync", ps)
Expand All @@ -259,7 +258,7 @@ func (s *State) LastAncestorState(ctx context.Context, root [32]byte) (state.Bea
}

// Does the state exist in epoch boundary cache.
cachedInfo, ok, err := s.epochBoundaryStateCache.getByRoot(parentRoot)
cachedInfo, ok, err := s.epochBoundaryStateCache.getByBlockRoot(parentRoot)
if err != nil {
return nil, err
}
Expand All @@ -271,6 +270,7 @@ func (s *State) LastAncestorState(ctx context.Context, root [32]byte) (state.Bea
if s.beaconDB.HasState(ctx, parentRoot) {
return s.beaconDB.State(ctx, parentRoot)
}

b, err = s.beaconDB.Block(ctx, parentRoot)
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions beacon-chain/state/stategen/getter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,22 +174,22 @@ func TestDeleteStateFromCaches(t *testing.T) {
r := [32]byte{'A'}

require.Equal(t, false, service.hotStateCache.has(r))
_, has, err := service.epochBoundaryStateCache.getByRoot(r)
_, has, err := service.epochBoundaryStateCache.getByBlockRoot(r)
require.NoError(t, err)
require.Equal(t, false, has)

service.hotStateCache.put(r, beaconState)
require.NoError(t, service.epochBoundaryStateCache.put(r, beaconState))

require.Equal(t, true, service.hotStateCache.has(r))
_, has, err = service.epochBoundaryStateCache.getByRoot(r)
_, has, err = service.epochBoundaryStateCache.getByBlockRoot(r)
require.NoError(t, err)
require.Equal(t, true, has)

require.NoError(t, service.DeleteStateFromCaches(ctx, r))

require.Equal(t, false, service.hotStateCache.has(r))
_, has, err = service.epochBoundaryStateCache.getByRoot(r)
_, has, err = service.epochBoundaryStateCache.getByBlockRoot(r)
require.NoError(t, err)
require.Equal(t, false, has)
}
Expand Down
10 changes: 5 additions & 5 deletions beacon-chain/state/stategen/history.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ type CanonicalHistory struct {
cache CachedGetter
}

func (ch *CanonicalHistory) ReplayerForSlot(target types.Slot) Replayer {
return &stateReplayer{chainer: ch, method: forSlot, target: target}
func (c *CanonicalHistory) ReplayerForSlot(target types.Slot) Replayer {
return &stateReplayer{chainer: c, method: forSlot, target: target}
}

func (c *CanonicalHistory) BlockForSlot(ctx context.Context, target types.Slot) ([32]byte, interfaces.SignedBeaconBlock, error) {
Expand Down Expand Up @@ -134,17 +134,17 @@ func (c *CanonicalHistory) chainForSlot(ctx context.Context, target types.Slot)
return s, descendants, nil
}

func (c *CanonicalHistory) getState(ctx context.Context, root [32]byte) (state.BeaconState, error) {
func (c *CanonicalHistory) getState(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) {
if c.cache != nil {
st, err := c.cache.ByRoot(root)
st, err := c.cache.ByBlockRoot(blockRoot)
if err == nil {
return st, nil
}
if !errors.Is(err, ErrNotInCache) {
return nil, errors.Wrap(err, "error reading from state cache during state replay")
}
}
return c.h.StateOrError(ctx, root)
return c.h.StateOrError(ctx, blockRoot)
}

// ancestorChain works backwards through the chain lineage, accumulating blocks and checking for a saved state.
Expand Down
Loading

0 comments on commit 72e61c5

Please sign in to comment.