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

lock forkchoice on calls to Ancestor #12162

Merged
merged 2 commits into from Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions beacon-chain/blockchain/chain_info.go
Expand Up @@ -28,6 +28,15 @@ type ChainInfoFetcher interface {
CanonicalFetcher
ForkFetcher
HeadDomainFetcher
ForkchoiceFetcher
}

// ForkchoiceFetcher defines a common interface for methods that access directly
// forkchoice information. These typically require a lock and external callers
// are requested to call methods within this blockchain package that takes care
// of locking forkchoice
type ForkchoiceFetcher interface {
Ancestor(context.Context, []byte, primitives.Slot) ([]byte, error)
}

// HeadUpdater defines a common interface for methods in blockchain service
Expand Down Expand Up @@ -436,6 +445,41 @@ func (s *Service) IsOptimisticForRoot(ctx context.Context, root [32]byte) (bool,
return !isCanonical, nil
}

// Ancestor returns the block root of an ancestry block from the input block root.
//
// Spec pseudocode definition:
//
// def get_ancestor(store: Store, root: Root, slot: Slot) -> Root:
// block = store.blocks[root]
// if block.slot > slot:
// return get_ancestor(store, block.parent_root, slot)
// elif block.slot == slot:
// return root
// else:
// # root is older than queried slot, thus a skip slot. Return most recent root prior to slot
// return root
func (s *Service) Ancestor(ctx context.Context, root []byte, slot primitives.Slot) ([]byte, error) {
ctx, span := trace.StartSpan(ctx, "blockChain.ancestor")
defer span.End()

r := bytesutil.ToBytes32(root)
// Get ancestor root from fork choice store instead of recursively looking up blocks in DB.
// This is most optimal outcome.
s.ForkChoicer().RLock()
ar, err := s.cfg.ForkChoiceStore.AncestorRoot(ctx, r, slot)
s.ForkChoicer().RUnlock()
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
}

// SetGenesisTime sets the genesis time of beacon chain.
func (s *Service) SetGenesisTime(t time.Time) {
s.genesisTime = t
Expand Down
33 changes: 0 additions & 33 deletions beacon-chain/blockchain/process_block_helpers.go
Expand Up @@ -146,39 +146,6 @@ func (s *Service) updateFinalized(ctx context.Context, cp *ethpb.Checkpoint) err
return nil
}

// ancestor returns the block root of an ancestry block from the input block root.
//
// Spec pseudocode definition:
//
// def get_ancestor(store: Store, root: Root, slot: Slot) -> Root:
// block = store.blocks[root]
// if block.slot > slot:
// return get_ancestor(store, block.parent_root, slot)
// elif block.slot == slot:
// return root
// else:
// # root is older than queried slot, thus a skip slot. Return most recent root prior to slot
// return root
func (s *Service) ancestor(ctx context.Context, root []byte, slot primitives.Slot) ([]byte, error) {
ctx, span := trace.StartSpan(ctx, "blockChain.ancestor")
defer span.End()

r := bytesutil.ToBytes32(root)
// Get ancestor root from fork choice store instead of recursively looking up blocks in DB.
// This is most optimal outcome.
ar, err := s.cfg.ForkChoiceStore.AncestorRoot(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 DB. The look up is recursively looking up DB. Slower than `ancestorByForkChoiceStore`.
func (s *Service) ancestorByDB(ctx context.Context, r [32]byte, slot primitives.Slot) (root [32]byte, err error) {
ctx, span := trace.StartSpan(ctx, "blockChain.ancestorByDB")
Expand Down
8 changes: 4 additions & 4 deletions beacon-chain/blockchain/process_block_test.go
Expand Up @@ -600,14 +600,14 @@ func TestAncestor_HandleSkipSlot(t *testing.T) {
}

// Slots 100 to 200 are skip slots. Requesting root at 150 will yield root at 100. The last physical block.
r, err := service.ancestor(context.Background(), r200[:], 150)
r, err := service.Ancestor(context.Background(), r200[:], 150)
require.NoError(t, err)
if bytesutil.ToBytes32(r) != r100 {
t.Error("Did not get correct root")
}

// Slots 1 to 100 are skip slots. Requesting root at 50 will yield root at 1. The last physical block.
r, err = service.ancestor(context.Background(), r200[:], 50)
r, err = service.Ancestor(context.Background(), r200[:], 50)
require.NoError(t, err)
if bytesutil.ToBytes32(r) != r1 {
t.Error("Did not get correct root")
Expand Down Expand Up @@ -648,7 +648,7 @@ func TestAncestor_CanUseForkchoice(t *testing.T) {
require.NoError(t, service.cfg.ForkChoiceStore.InsertNode(ctx, st, blkRoot))
}

r, err := service.ancestor(context.Background(), r200[:], 150)
r, err := service.Ancestor(context.Background(), r200[:], 150)
require.NoError(t, err)
if bytesutil.ToBytes32(r) != r100 {
t.Error("Did not get correct root")
Expand Down Expand Up @@ -696,7 +696,7 @@ func TestAncestor_CanUseDB(t *testing.T) {
require.NoError(t, err)
require.NoError(t, service.cfg.ForkChoiceStore.InsertNode(ctx, st, blkRoot))

r, err := service.ancestor(context.Background(), r200[:], 150)
r, err := service.Ancestor(context.Background(), r200[:], 150)
require.NoError(t, err)
if bytesutil.ToBytes32(r) != r100 {
t.Error("Did not get correct root")
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/receive_attestation.go
Expand Up @@ -56,7 +56,7 @@ func (s *Service) VerifyLmdFfgConsistency(ctx context.Context, a *ethpb.Attestat
if err != nil {
return err
}
r, err := s.ancestor(ctx, a.Data.BeaconBlockRoot, targetSlot)
r, err := s.Ancestor(ctx, a.Data.BeaconBlockRoot, targetSlot)
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions beacon-chain/blockchain/testing/mock.go
Expand Up @@ -74,6 +74,10 @@ func (s *ChainService) ForkChoicer() forkchoice.ForkChoicer {
return s.ForkChoiceStore
}

func (s *ChainService) Ancestor(_ context.Context, _ []byte, _ primitives.Slot) ([]byte, error) {
return nil, nil
}

// StateNotifier mocks the same method in the chain service.
func (s *ChainService) StateNotifier() statefeed.Notifier {
if s.stateNotifier == nil {
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/sync/service.go
Expand Up @@ -100,6 +100,7 @@ type blockchainService interface {
blockchain.CanonicalFetcher
blockchain.OptimisticModeFetcher
blockchain.SlashingReceiver
blockchain.ForkchoiceFetcher
}

// Service is responsible for handling all run time p2p related operations as the
Expand Down