Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover State Summary Correctly #12214

Merged
merged 3 commits into from
Mar 30, 2023
Merged

Recover State Summary Correctly #12214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f47bc2
fix it
nisdas Mar 30, 2023
78b47f2
add zerohash checks
nisdas Mar 30, 2023
b46c7dc
preston's review
nisdas Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions beacon-chain/blockchain/chain_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,10 @@ func (s *Service) IsOptimisticForRoot(ctx context.Context, root [32]byte) (bool,
}

if ss == nil {
return true, errInvalidNilSummary
ss, err = s.recoverStateSummary(ctx, root)
if err != nil {
return true, err
}
}
validatedCheckpoint, err := s.cfg.BeaconDB.LastValidatedCheckpoint(ctx)
if err != nil {
Expand All @@ -428,7 +431,10 @@ func (s *Service) IsOptimisticForRoot(ctx context.Context, root [32]byte) (bool,
return false, err
}
if lastValidated == nil {
return false, errInvalidNilSummary
lastValidated, err = s.recoverStateSummary(ctx, root)
if err != nil {
return false, err
}
}

if ss.Slot > lastValidated.Slot {
Expand Down Expand Up @@ -476,3 +482,18 @@ func (s *Service) Ancestor(ctx context.Context, root []byte, slot primitives.Slo
func (s *Service) SetGenesisTime(t time.Time) {
s.genesisTime = t
}

func (s *Service) recoverStateSummary(ctx context.Context, blockRoot [32]byte) (*ethpb.StateSummary, error) {
if s.cfg.BeaconDB.HasBlock(ctx, blockRoot) {
b, err := s.cfg.BeaconDB.Block(ctx, blockRoot)
if err != nil {
return nil, err
}
summary := &ethpb.StateSummary{Slot: b.Block().Slot(), Root: blockRoot[:]}
if err := s.cfg.BeaconDB.SaveStateSummary(ctx, summary); err != nil {
return nil, err
}
return summary, nil
}
return nil, errBlockDoesNotExist
}
Comment on lines +486 to +499
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to add a test for this method?

24 changes: 19 additions & 5 deletions beacon-chain/blockchain/chain_info_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,15 +518,10 @@ func TestService_IsOptimisticForRoot_DB(t *testing.T) {
validatedCheckpoint := &ethpb.Checkpoint{Root: br[:]}
require.NoError(t, beaconDB.SaveLastValidatedCheckpoint(ctx, validatedCheckpoint))

_, err = c.IsOptimisticForRoot(ctx, optimisticRoot)
require.ErrorContains(t, "nil summary returned from the DB", err)

require.NoError(t, beaconDB.SaveStateSummary(context.Background(), &ethpb.StateSummary{Root: optimisticRoot[:], Slot: 11}))
optimistic, err := c.IsOptimisticForRoot(ctx, optimisticRoot)
require.NoError(t, err)
require.Equal(t, true, optimistic)

require.NoError(t, beaconDB.SaveStateSummary(context.Background(), &ethpb.StateSummary{Root: validatedRoot[:], Slot: 9}))
cp := &ethpb.Checkpoint{
Epoch: 1,
Comment on lines 520 to 526
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some kind of regression test? This might be the test already, but you can assert that = BeaconDB.StateSummary returns nil and returns non-nil after calling IsOptimisticForRoot

Root: validatedRoot[:],
Expand Down Expand Up @@ -588,6 +583,25 @@ func TestService_IsOptimisticForRoot_DB_non_canonical(t *testing.T) {

}

func TestService_IsOptimisticForRoot_StateSummaryRecovered(t *testing.T) {
beaconDB := testDB.SetupDB(t)
ctx := context.Background()
c := &Service{cfg: &config{BeaconDB: beaconDB, ForkChoiceStore: doublylinkedtree.New()}, head: &head{root: [32]byte{'b'}}}
c.head = &head{root: params.BeaconConfig().ZeroHash}
b := util.NewBeaconBlock()
b.Block.Slot = 10
br, err := b.Block.HashTreeRoot()
require.NoError(t, err)
util.SaveBlock(t, context.Background(), beaconDB, b)
_, err = c.IsOptimisticForRoot(ctx, br)
assert.NoError(t, err)
summ, err := beaconDB.StateSummary(ctx, br)
assert.NoError(t, err)
assert.NotNil(t, summ)
assert.Equal(t, 10, int(summ.Slot))
assert.DeepEqual(t, br[:], summ.Root)
}

func TestService_IsFinalized(t *testing.T) {
beaconDB := testDB.SetupDB(t)
ctx := context.Background()
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/blockchain/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ var (
errNilFinalizedCheckpoint = errors.New("nil finalized checkpoint returned from state")
// errNilJustifiedCheckpoint is returned when a nil justified checkpt is returned from a state.
errNilJustifiedCheckpoint = errors.New("nil justified checkpoint returned from state")
// errInvalidNilSummary is returned when a nil summary is returned from the DB.
errInvalidNilSummary = errors.New("nil summary returned from the DB")
// errBlockDoesNotExist is returned when a block does not exist for a particular state summary.
errBlockDoesNotExist = errors.New("could not find block in DB")
// errWrongBlockCount is returned when the wrong number of blocks or block roots is used
errWrongBlockCount = errors.New("wrong number of blocks or block roots")
// errBlockNotFoundInCacheOrDB is returned when a block is not found in the cache or DB.
Expand Down
10 changes: 10 additions & 0 deletions beacon-chain/rpc/eth/helpers/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package helpers

import (
"bytes"
"context"
"strconv"
"strings"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/prysmaticlabs/prysm/v4/beacon-chain/db"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/rpc/lookup"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/sync"
"github.com/prysmaticlabs/prysm/v4/config/params"
"github.com/prysmaticlabs/prysm/v4/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v4/time/slots"
Expand Down Expand Up @@ -76,12 +78,20 @@ func IsOptimistic(
if fcp == nil {
return true, errors.New("received nil finalized checkpoint")
}
// Special genesis case in the event our checkpoint root is a zerohash.
if bytes.Equal(fcp.Root, params.BeaconConfig().ZeroHash[:]) {
return false, nil
}
return optimisticModeFetcher.IsOptimisticForRoot(ctx, bytesutil.ToBytes32(fcp.Root))
case "justified":
jcp := chainInfo.CurrentJustifiedCheckpt()
if jcp == nil {
return true, errors.New("received nil justified checkpoint")
}
// Special genesis case in the event our checkpoint root is a zerohash.
if bytes.Equal(jcp.Root, params.BeaconConfig().ZeroHash[:]) {
return false, nil
}
return optimisticModeFetcher.IsOptimisticForRoot(ctx, bytesutil.ToBytes32(jcp.Root))
default:
if len(stateId) == 32 {
Expand Down