diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go index 23e99841e..87b440edc 100644 --- a/beacon-chain/blockchain/process_block_helpers.go +++ b/beacon-chain/blockchain/process_block_helpers.go @@ -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 diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index 44ccd21fa..a8493e22c 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -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{}