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.

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

Deprecate store in blockchain pkg #10903

Merged
merged 34 commits into from
Jun 25, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e06df41
Deprecate store WIP
potuz Jun 17, 2022
a1dc423
Merge remote-tracking branch 'origin/develop' into deprecate_store_4
potuz Jun 17, 2022
19d8a06
fix spectests build
potuz Jun 17, 2022
34aaec5
mock right interface
potuz Jun 17, 2022
950c589
sync tests build
potuz Jun 17, 2022
2b9595c
more tests builds
potuz Jun 17, 2022
e72799e
blockchain tests
potuz Jun 18, 2022
79ae441
Merge remote-tracking branch 'origin/develop' into deprecate_store_4
potuz Jun 19, 2022
fcd384a
more blockchain tests
potuz Jun 19, 2022
ea4eabe
update best justified from genesis
potuz Jun 20, 2022
1761b12
Merge remote-tracking branch 'origin/develop' into deprecate_store_4
potuz Jun 20, 2022
9736f63
deal with nil head on saveHead
potuz Jun 20, 2022
edcfebb
initialize prev justified checkpoint
potuz Jun 20, 2022
e37c835
update finalization correctly
potuz Jun 20, 2022
d0d8b48
update finalization logic
potuz Jun 20, 2022
f6ea696
update finalization logic
potuz Jun 20, 2022
18542f7
Merge branch 'develop' into deprecate_store_4
potuz Jun 20, 2022
968c0b5
track the wall clock on forkchoice spectests
potuz Jun 21, 2022
84a152c
Merge branch 'wall_clock_spectests' into deprecate_store_4
potuz Jun 21, 2022
289a62b
Merge remote-tracking branch 'origin/develop' into deprecate_store_4
potuz Jun 21, 2022
6ad9c0c
export copies of checkpoints from blockchain package
potuz Jun 22, 2022
58be2c9
do not use forkchoice's head on HeadRoot
potuz Jun 22, 2022
081024d
Remove debug remain
potuz Jun 22, 2022
27abeb6
terence's review
potuz Jun 22, 2022
64cec72
Merge branch 'develop' into deprecate_store_4
potuz Jun 22, 2022
7170d79
add forkchoice types deps
potuz Jun 22, 2022
61abba3
wtf
potuz Jun 22, 2022
02191e6
debugging
potuz Jun 23, 2022
64edcad
init-sync: update justified and finalized checkpoints on db
potuz Jun 23, 2022
fe856f3
call updateFinalized instead of only DB
potuz Jun 23, 2022
21b1c7b
remove debug symbols
potuz Jun 23, 2022
caee5eb
Merge branch 'develop' into deprecate_store_4
terencechain Jun 23, 2022
e111a15
safe copy headroot
potuz Jun 25, 2022
1588a3d
Merge remote-tracking branch 'origin/develop' into deprecate_store_4
potuz Jun 25, 2022
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
3 changes: 0 additions & 3 deletions beacon-chain/blockchain/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ go_library(
"log.go",
"merge_ascii_art.go",
"metrics.go",
"new_slot.go",
"options.go",
"pow_block.go",
"process_attestation.go",
Expand All @@ -35,7 +34,6 @@ go_library(
deps = [
"//async:go_default_library",
"//async/event:go_default_library",
"//beacon-chain/blockchain/store:go_default_library",
"//beacon-chain/cache:go_default_library",
"//beacon-chain/cache/depositcache:go_default_library",
"//beacon-chain/core/altair:go_default_library",
Expand Down Expand Up @@ -113,7 +111,6 @@ go_test(
"log_test.go",
"metrics_test.go",
"mock_test.go",
"new_slot_test.go",
"pow_block_test.go",
"process_attestation_test.go",
"process_block_test.go",
Expand Down
66 changes: 24 additions & 42 deletions beacon-chain/blockchain/chain_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"
"time"

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/beacon-chain/blockchain/store"
"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/beacon-chain/forkchoice"
doublylinkedtree "github.com/prysmaticlabs/prysm/beacon-chain/forkchoice/doubly-linked-tree"
Expand Down Expand Up @@ -81,9 +79,9 @@ type CanonicalFetcher interface {
// FinalizationFetcher defines a common interface for methods in blockchain service which
// directly retrieve finalization and justification related data.
type FinalizationFetcher interface {
FinalizedCheckpt() (*ethpb.Checkpoint, error)
CurrentJustifiedCheckpt() (*ethpb.Checkpoint, error)
PreviousJustifiedCheckpt() (*ethpb.Checkpoint, error)
FinalizedCheckpt() *ethpb.Checkpoint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will move all these to *forkchoicetypes.Checkpoint in a future PR

CurrentJustifiedCheckpt() *ethpb.Checkpoint
PreviousJustifiedCheckpt() *ethpb.Checkpoint
VerifyFinalizedBlkDescendant(ctx context.Context, blockRoot [32]byte) error
}

Expand All @@ -94,47 +92,27 @@ type OptimisticModeFetcher interface {
}

// FinalizedCheckpt returns the latest finalized checkpoint from chain store.
func (s *Service) FinalizedCheckpt() (*ethpb.Checkpoint, error) {
cp, err := s.store.FinalizedCheckpt()
if err != nil {
return nil, err
}

return ethpb.CopyCheckpoint(cp), nil
func (s *Service) FinalizedCheckpt() *ethpb.Checkpoint {
cp := s.ForkChoicer().FinalizedCheckpoint()
return &ethpb.Checkpoint{Epoch: cp.Epoch, Root: cp.Root[:]}
}

// CurrentJustifiedCheckpt returns the current justified checkpoint from chain store.
func (s *Service) CurrentJustifiedCheckpt() (*ethpb.Checkpoint, error) {
cp, err := s.store.JustifiedCheckpt()
if err != nil {
return nil, err
}

return ethpb.CopyCheckpoint(cp), nil
// PreviousJustifiedCheckpt returns the current justified checkpoint from chain store.
func (s *Service) PreviousJustifiedCheckpt() *ethpb.Checkpoint {
cp := s.ForkChoicer().PreviousJustifiedCheckpoint()
return &ethpb.Checkpoint{Epoch: cp.Epoch, Root: cp.Root[:]}
}

// PreviousJustifiedCheckpt returns the previous justified checkpoint from chain store.
func (s *Service) PreviousJustifiedCheckpt() (*ethpb.Checkpoint, error) {
cp, err := s.store.PrevJustifiedCheckpt()
if err != nil {
return nil, err
}

return ethpb.CopyCheckpoint(cp), nil
// CurrentJustifiedCheckpt returns the current justified checkpoint from chain store.
func (s *Service) CurrentJustifiedCheckpt() *ethpb.Checkpoint {
cp := s.ForkChoicer().JustifiedCheckpoint()
return &ethpb.Checkpoint{Epoch: cp.Epoch, Root: cp.Root[:]}
}

// BestJustifiedCheckpt returns the best justified checkpoint from store.
func (s *Service) BestJustifiedCheckpt() (*ethpb.Checkpoint, error) {
cp, err := s.store.BestJustifiedCheckpt()
if err != nil {
// If there is no best justified checkpoint, return the checkpoint with root as zeros to be used for genesis cases.
if errors.Is(err, store.ErrNilCheckpoint) {
return &ethpb.Checkpoint{Root: params.BeaconConfig().ZeroHash[:]}, nil
}
return nil, err
}

return ethpb.CopyCheckpoint(cp), nil
func (s *Service) BestJustifiedCheckpt() *ethpb.Checkpoint {
cp := s.ForkChoicer().BestJustifiedCheckpoint()
return &ethpb.Checkpoint{Epoch: cp.Epoch, Root: cp.Root[:]}
}

// HeadSlot returns the slot of the head of the chain.
Expand All @@ -154,9 +132,13 @@ func (s *Service) HeadRoot(ctx context.Context) ([]byte, error) {
s.headLock.RLock()
defer s.headLock.RUnlock()

if s.headRoot() != params.BeaconConfig().ZeroHash {
r := s.headRoot()
return r[:], nil
if s.head != nil && s.head.root != params.BeaconConfig().ZeroHash {
return s.head.root[:], nil
Copy link
Member

Choose a reason for hiding this comment

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

we should return a copy here. By taking the slice, you still reference the root in head

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks this is right I think.

}

root := s.ForkChoicer().CachedHeadRoot()
if root != params.BeaconConfig().ZeroHash {
return root[:], nil
}

b, err := s.cfg.BeaconDB.HeadBlock(ctx)
Expand Down
138 changes: 63 additions & 75 deletions beacon-chain/blockchain/chain_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (
"testing"
"time"

"github.com/prysmaticlabs/prysm/beacon-chain/blockchain/store"
testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing"
doublylinkedtree "github.com/prysmaticlabs/prysm/beacon-chain/forkchoice/doubly-linked-tree"
"github.com/prysmaticlabs/prysm/beacon-chain/forkchoice/protoarray"
forkchoicetypes "github.com/prysmaticlabs/prysm/beacon-chain/forkchoice/types"
"github.com/prysmaticlabs/prysm/beacon-chain/state"
"github.com/prysmaticlabs/prysm/beacon-chain/state/stategen"
v1 "github.com/prysmaticlabs/prysm/beacon-chain/state/v1"
v3 "github.com/prysmaticlabs/prysm/beacon-chain/state/v3"
fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
Expand Down Expand Up @@ -77,81 +78,48 @@ func TestService_ForkChoiceStore(t *testing.T) {
require.Equal(t, types.Epoch(0), p.FinalizedCheckpoint().Epoch)
}

func TestFinalizedCheckpt_CanRetrieve(t *testing.T) {
beaconDB := testDB.SetupDB(t)

cp := &ethpb.Checkpoint{Epoch: 5, Root: bytesutil.PadTo([]byte("foo"), 32)}
c := setupBeaconChain(t, beaconDB)
c.store.SetFinalizedCheckptAndPayloadHash(cp, [32]byte{'a'})

cp, err := c.FinalizedCheckpt()
require.NoError(t, err)
assert.Equal(t, cp.Epoch, cp.Epoch, "Unexpected finalized epoch")
}

func TestFinalizedCheckpt_GenesisRootOk(t *testing.T) {
ctx := context.Background()
beaconDB := testDB.SetupDB(t)

genesisRoot := [32]byte{'A'}
cp := &ethpb.Checkpoint{Root: genesisRoot[:]}
c := setupBeaconChain(t, beaconDB)
c.store.SetFinalizedCheckptAndPayloadHash(cp, [32]byte{'a'})
c.originBlockRoot = genesisRoot
cp, err := c.FinalizedCheckpt()
fcs := protoarray.New()
opts := []Option{
WithDatabase(beaconDB),
WithForkChoiceStore(fcs),
WithStateGen(stategen.New(beaconDB)),
}
service, err := NewService(ctx, opts...)
require.NoError(t, err)
assert.DeepEqual(t, c.originBlockRoot[:], cp.Root)
}

func TestCurrentJustifiedCheckpt_CanRetrieve(t *testing.T) {
beaconDB := testDB.SetupDB(t)

c := setupBeaconChain(t, beaconDB)
_, err := c.CurrentJustifiedCheckpt()
require.ErrorIs(t, err, store.ErrNilCheckpoint)
cp := &ethpb.Checkpoint{Epoch: 6, Root: bytesutil.PadTo([]byte("foo"), 32)}
c.store.SetJustifiedCheckptAndPayloadHash(cp, [32]byte{})
jp, err := c.CurrentJustifiedCheckpt()
gs, _ := util.DeterministicGenesisState(t, 32)
require.NoError(t, service.saveGenesisData(ctx, gs))
cp := service.FinalizedCheckpt()
assert.DeepEqual(t, [32]byte{}, bytesutil.ToBytes32(cp.Root))
cp = service.CurrentJustifiedCheckpt()
assert.DeepEqual(t, [32]byte{}, bytesutil.ToBytes32(cp.Root))
// check that forkchoice has the right genesis root as the node root
root, err := fcs.Head(ctx, []uint64{})
require.NoError(t, err)
assert.Equal(t, cp.Epoch, jp.Epoch, "Unexpected justified epoch")
}
require.Equal(t, service.originBlockRoot, root)

func TestJustifiedCheckpt_GenesisRootOk(t *testing.T) {
beaconDB := testDB.SetupDB(t)

c := setupBeaconChain(t, beaconDB)
genesisRoot := [32]byte{'B'}
cp := &ethpb.Checkpoint{Root: genesisRoot[:]}
c.store.SetJustifiedCheckptAndPayloadHash(cp, [32]byte{})
c.originBlockRoot = genesisRoot
cp, err := c.CurrentJustifiedCheckpt()
require.NoError(t, err)
assert.DeepEqual(t, c.originBlockRoot[:], cp.Root)
}

func TestPreviousJustifiedCheckpt_CanRetrieve(t *testing.T) {
func TestCurrentJustifiedCheckpt_CanRetrieve(t *testing.T) {
ctx := context.Background()
beaconDB := testDB.SetupDB(t)

cp := &ethpb.Checkpoint{Epoch: 7, Root: bytesutil.PadTo([]byte("foo"), 32)}
c := setupBeaconChain(t, beaconDB)
_, err := c.PreviousJustifiedCheckpt()
require.ErrorIs(t, err, store.ErrNilCheckpoint)
c.store.SetPrevJustifiedCheckpt(cp)
pcp, err := c.PreviousJustifiedCheckpt()
fcs := protoarray.New()
opts := []Option{
WithDatabase(beaconDB),
WithForkChoiceStore(fcs),
WithStateGen(stategen.New(beaconDB)),
}
service, err := NewService(ctx, opts...)
require.NoError(t, err)
assert.Equal(t, cp.Epoch, pcp.Epoch, "Unexpected previous justified epoch")
}

func TestPrevJustifiedCheckpt_GenesisRootOk(t *testing.T) {
beaconDB := testDB.SetupDB(t)

genesisRoot := [32]byte{'C'}
cp := &ethpb.Checkpoint{Root: genesisRoot[:]}
c := setupBeaconChain(t, beaconDB)
c.store.SetPrevJustifiedCheckpt(cp)
c.originBlockRoot = genesisRoot
pcp, err := c.PreviousJustifiedCheckpt()
require.NoError(t, err)
assert.DeepEqual(t, c.originBlockRoot[:], pcp.Root)
cp := &forkchoicetypes.Checkpoint{Epoch: 6, Root: [32]byte{'j'}}
require.NoError(t, fcs.UpdateJustifiedCheckpoint(cp))
jp := service.CurrentJustifiedCheckpt()
assert.Equal(t, cp.Epoch, jp.Epoch, "Unexpected justified epoch")
require.Equal(t, cp.Root, bytesutil.ToBytes32(jp.Root))
}

func TestHeadSlot_CanRetrieve(t *testing.T) {
Expand All @@ -163,26 +131,46 @@ func TestHeadSlot_CanRetrieve(t *testing.T) {
}

func TestHeadRoot_CanRetrieve(t *testing.T) {
c := &Service{}
c.head = &head{root: [32]byte{'A'}}
r, err := c.HeadRoot(context.Background())
ctx := context.Background()
beaconDB := testDB.SetupDB(t)
fcs := protoarray.New()
opts := []Option{
WithDatabase(beaconDB),
WithForkChoiceStore(fcs),
WithStateGen(stategen.New(beaconDB)),
}
service, err := NewService(ctx, opts...)
require.NoError(t, err)
gs, _ := util.DeterministicGenesisState(t, 32)
require.NoError(t, service.saveGenesisData(ctx, gs))

r, err := service.HeadRoot(ctx)
require.NoError(t, err)
assert.Equal(t, [32]byte{'A'}, bytesutil.ToBytes32(r))
assert.Equal(t, service.originBlockRoot, bytesutil.ToBytes32(r))
}

func TestHeadRoot_UseDB(t *testing.T) {
ctx := context.Background()
beaconDB := testDB.SetupDB(t)
c := &Service{cfg: &config{BeaconDB: beaconDB}}
c.head = &head{root: params.BeaconConfig().ZeroHash}
fcs := protoarray.New()
opts := []Option{
WithDatabase(beaconDB),
WithForkChoiceStore(fcs),
WithStateGen(stategen.New(beaconDB)),
}
service, err := NewService(ctx, opts...)
require.NoError(t, err)

service.head = &head{root: params.BeaconConfig().ZeroHash}
b := util.NewBeaconBlock()
br, err := b.Block.HashTreeRoot()
require.NoError(t, err)
wsb, err := wrapper.WrappedSignedBeaconBlock(b)
require.NoError(t, err)
require.NoError(t, beaconDB.SaveBlock(context.Background(), wsb))
require.NoError(t, beaconDB.SaveStateSummary(context.Background(), &ethpb.StateSummary{Root: br[:]}))
require.NoError(t, beaconDB.SaveHeadBlockRoot(context.Background(), br))
r, err := c.HeadRoot(context.Background())
require.NoError(t, beaconDB.SaveBlock(ctx, wsb))
require.NoError(t, beaconDB.SaveStateSummary(ctx, &ethpb.StateSummary{Root: br[:]}))
require.NoError(t, beaconDB.SaveHeadBlockRoot(ctx, br))
r, err := service.HeadRoot(ctx)
require.NoError(t, err)
assert.Equal(t, br, bytesutil.ToBytes32(r))
}
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/blockchain/execution_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, arg *notifyForkcho
if err != nil {
return nil, errors.Wrap(err, "could not get execution payload")
}
finalizedHash := s.store.FinalizedPayloadBlockHash()
justifiedHash := s.store.JustifiedPayloadBlockHash()
finalizedHash := s.ForkChoicer().FinalizedPayloadBlockHash()
justifiedHash := s.ForkChoicer().JustifiedPayloadBlockHash()
fcs := &enginev1.ForkchoiceState{
HeadBlockHash: headPayload.BlockHash,
SafeBlockHash: justifiedHash[:],
Expand Down
18 changes: 2 additions & 16 deletions beacon-chain/blockchain/execution_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/prysmaticlabs/prysm/testing/assert"
"github.com/prysmaticlabs/prysm/testing/require"
"github.com/prysmaticlabs/prysm/testing/util"
"github.com/prysmaticlabs/prysm/time/slots"
logTest "github.com/sirupsen/logrus/hooks/test"
)

Expand Down Expand Up @@ -187,9 +186,6 @@ func Test_NotifyForkchoiceUpdate(t *testing.T) {
st, _ := util.DeterministicGenesisState(t, 1)
require.NoError(t, beaconDB.SaveState(ctx, st, tt.finalizedRoot))
require.NoError(t, beaconDB.SaveGenesisBlockRoot(ctx, tt.finalizedRoot))
fc := &ethpb.Checkpoint{Epoch: 0, Root: tt.finalizedRoot[:]}
service.store.SetFinalizedCheckptAndPayloadHash(fc, [32]byte{'a'})
service.store.SetJustifiedCheckptAndPayloadHash(fc, [32]byte{'b'})
arg := &notifyForkchoiceUpdateArg{
headState: st,
headRoot: tt.headRoot,
Expand Down Expand Up @@ -343,9 +339,6 @@ func Test_NotifyForkchoiceUpdateRecursive(t *testing.T) {

require.NoError(t, beaconDB.SaveState(ctx, st, bra))
require.NoError(t, beaconDB.SaveGenesisBlockRoot(ctx, bra))
fc := &ethpb.Checkpoint{Epoch: 0, Root: bra[:]}
service.store.SetFinalizedCheckptAndPayloadHash(fc, [32]byte{'a'})
service.store.SetJustifiedCheckptAndPayloadHash(fc, [32]byte{'b'})
a := &notifyForkchoiceUpdateArg{
headState: st,
headBlock: wbg.Block(),
Expand Down Expand Up @@ -723,14 +716,7 @@ func Test_IsOptimisticCandidateBlock(t *testing.T) {
},
}
for _, tt := range tests {
jRoot, err := tt.justified.Block().HashTreeRoot()
require.NoError(t, err)
require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, tt.justified))
service.store.SetJustifiedCheckptAndPayloadHash(
&ethpb.Checkpoint{
Root: jRoot[:],
Epoch: slots.ToEpoch(tt.justified.Block().Slot()),
}, [32]byte{'a'})
require.NoError(t, service.cfg.BeaconDB.SaveBlock(ctx, wrappedParentBlock))

err = service.optimisticCandidateBlock(ctx, tt.blk)
Expand Down Expand Up @@ -1017,7 +1003,7 @@ func TestService_getPayloadHash(t *testing.T) {
require.NoError(t, err)
wsb, err := wrapper.WrappedSignedBeaconBlock(b)
require.NoError(t, err)
service.saveInitSyncBlock(r, wsb)
require.NoError(t, service.saveInitSyncBlock(ctx, r, wsb))

h, err := service.getPayloadHash(ctx, r[:])
require.NoError(t, err)
Expand All @@ -1030,7 +1016,7 @@ func TestService_getPayloadHash(t *testing.T) {
require.NoError(t, err)
wsb, err = wrapper.WrappedSignedBeaconBlock(bb)
require.NoError(t, err)
service.saveInitSyncBlock(r, wsb)
require.NoError(t, service.saveInitSyncBlock(ctx, r, wsb))

h, err = service.getPayloadHash(ctx, r[:])
require.NoError(t, err)
Expand Down
Loading