Refactor ancestor lookup (#7472)

* Add ancestor root fall back to DB check

* Add a warning message

* Refactor ancestor lookups into seperate functions

* Update error string

* Fix has parent check and add a regression test

* Add a conext error test

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
This commit is contained in:
terence tsao 2020-10-09 17:07:56 -07:00 committed by GitHub
parent f92244d497
commit 8aaa5b6ad0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 113 additions and 9 deletions

View File

@ -258,16 +258,35 @@ func (s *Service) ancestor(ctx context.Context, root []byte, slot uint64) ([]byt
ctx, span := trace.StartSpan(ctx, "forkChoice.ancestor")
defer span.End()
// Stop recursive ancestry lookup if context is cancelled.
if ctx.Err() != nil {
return nil, ctx.Err()
}
r := bytesutil.ToBytes32(root)
// Get ancestor root from fork choice store instead of recursively looking up blocks in DB.
// This is most optimal outcome.
if s.forkChoiceStore.HasParent(r) {
return s.forkChoiceStore.AncestorRoot(ctx, r, slot)
ar, err := s.ancestorByForkChoiceStore(ctx, r, slot)
if err != nil {
// Try getting ancestor root from DB when failed to retrieve from fork choice store.
// This is the second line of defense for retrieving ancestor root.
ar, err = s.ancestorByDB(ctx, r, slot)
if err != nil {
return nil, err
}
}
return ar, nil
}
// This retrieves an ancestor root using fork choice store. The look up is looping through the a flat array structure.
func (s *Service) ancestorByForkChoiceStore(ctx context.Context, r [32]byte, slot uint64) ([]byte, error) {
if !s.forkChoiceStore.HasParent(r) {
return nil, errors.New("could not find root in fork choice store")
}
return s.forkChoiceStore.AncestorRoot(ctx, r, slot)
}
// This retrieves an ancestor root using DB. The look up is recursively looking up DB. Slower than `ancestorByForkChoiceStore`.
func (s *Service) ancestorByDB(ctx context.Context, r [32]byte, slot uint64) ([]byte, error) {
// Stop recursive ancestry lookup if context is cancelled.
if ctx.Err() != nil {
return nil, ctx.Err()
}
signed, err := s.beaconDB.Block(ctx, r)
@ -284,10 +303,10 @@ func (s *Service) ancestor(ctx context.Context, root []byte, slot uint64) ([]byt
}
b := signed.Block
if b.Slot == slot || b.Slot < slot {
return root, nil
return r[:], nil
}
return s.ancestor(ctx, b.ParentRoot, slot)
return s.ancestorByDB(ctx, bytesutil.ToBytes32(b.ParentRoot), slot)
}
// This updates justified check point in store, if the new justified is later than stored justified or

View File

@ -516,6 +516,15 @@ func TestCurrentSlot_HandlesOverflow(t *testing.T) {
slot := svc.CurrentSlot()
require.Equal(t, uint64(0), slot, "Unexpected slot")
}
func TestAncestorByDB_CtxErr(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
service, err := NewService(ctx, &Config{})
require.NoError(t, err)
cancel()
_, err = service.ancestorByDB(ctx, [32]byte{}, 0)
require.ErrorContains(t, "context canceled", err)
}
func TestAncestor_HandleSkipSlot(t *testing.T) {
ctx := context.Background()
@ -562,6 +571,82 @@ func TestAncestor_HandleSkipSlot(t *testing.T) {
}
}
func TestAncestor_CanUseForkchoice(t *testing.T) {
ctx := context.Background()
cfg := &Config{ForkChoiceStore: protoarray.New(0, 0, [32]byte{})}
service, err := NewService(ctx, cfg)
require.NoError(t, err)
b1 := testutil.NewBeaconBlock()
b1.Block.Slot = 1
b1.Block.ParentRoot = bytesutil.PadTo([]byte{'a'}, 32)
r1, err := b1.Block.HashTreeRoot()
require.NoError(t, err)
b100 := testutil.NewBeaconBlock()
b100.Block.Slot = 100
b100.Block.ParentRoot = r1[:]
r100, err := b100.Block.HashTreeRoot()
require.NoError(t, err)
b200 := testutil.NewBeaconBlock()
b200.Block.Slot = 200
b200.Block.ParentRoot = r100[:]
r200, err := b200.Block.HashTreeRoot()
require.NoError(t, err)
for _, b := range []*ethpb.SignedBeaconBlock{b1, b100, b200} {
beaconBlock := testutil.NewBeaconBlock()
beaconBlock.Block.Slot = b.Block.Slot
beaconBlock.Block.ParentRoot = bytesutil.PadTo(b.Block.ParentRoot, 32)
r, err := b.Block.HashTreeRoot()
require.NoError(t, err)
require.NoError(t, service.forkChoiceStore.ProcessBlock(context.Background(), b.Block.Slot, r, bytesutil.ToBytes32(b.Block.ParentRoot), [32]byte{}, 0, 0)) // Saves blocks to fork choice store.
}
r, err := service.ancestor(context.Background(), r200[:], 150)
require.NoError(t, err)
if bytesutil.ToBytes32(r) != r100 {
t.Error("Did not get correct root")
}
}
func TestAncestor_CanUseDB(t *testing.T) {
ctx := context.Background()
db, _ := testDB.SetupDB(t)
cfg := &Config{BeaconDB: db, ForkChoiceStore: protoarray.New(0, 0, [32]byte{})}
service, err := NewService(ctx, cfg)
require.NoError(t, err)
b1 := testutil.NewBeaconBlock()
b1.Block.Slot = 1
b1.Block.ParentRoot = bytesutil.PadTo([]byte{'a'}, 32)
r1, err := b1.Block.HashTreeRoot()
require.NoError(t, err)
b100 := testutil.NewBeaconBlock()
b100.Block.Slot = 100
b100.Block.ParentRoot = r1[:]
r100, err := b100.Block.HashTreeRoot()
require.NoError(t, err)
b200 := testutil.NewBeaconBlock()
b200.Block.Slot = 200
b200.Block.ParentRoot = r100[:]
r200, err := b200.Block.HashTreeRoot()
require.NoError(t, err)
for _, b := range []*ethpb.SignedBeaconBlock{b1, b100, b200} {
beaconBlock := testutil.NewBeaconBlock()
beaconBlock.Block.Slot = b.Block.Slot
beaconBlock.Block.ParentRoot = bytesutil.PadTo(b.Block.ParentRoot, 32)
require.NoError(t, db.SaveBlock(context.Background(), beaconBlock)) // Saves blocks to DB.
}
require.NoError(t, service.forkChoiceStore.ProcessBlock(context.Background(), 200, r200, r200, [32]byte{}, 0, 0))
r, err := service.ancestor(context.Background(), r200[:], 150)
require.NoError(t, err)
if bytesutil.ToBytes32(r) != r100 {
t.Error("Did not get correct root")
}
}
func TestEnsureRootNotZeroHashes(t *testing.T) {
ctx := context.Background()
cfg := &Config{}