Skip to content

Commit

Permalink
Removed spans for fork choice helpers (#4808)
Browse files Browse the repository at this point in the history 
* Removed spans
  • Loading branch information
@terencechain
terencechain committed Feb 9, 2020
1 parent 8a02003 commit 5be4fee
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 29 deletions.
29 changes: 10 additions & 19 deletions beacon-chain/forkchoice/protoarray/nodes.go
View file
Expand Up @@ -39,7 +39,7 @@ func (s *Store) head(ctx context.Context, justifiedRoot [32]byte) ([32]byte, err

bestNode := s.nodes[bestDescendantIndex]

if !s.viableForHead(ctx, bestNode) {
if !s.viableForHead(bestNode) {
return [32]byte{}, fmt.Errorf("head at slot %d with weight %d is not eligible, finalizedEpoch %d != %d, justifiedEpoch %d != %d",
bestNode.Slot, bestNode.Weight/10e9, bestNode.finalizedEpoch, s.finalizedEpoch, bestNode.justifiedEpoch, s.justifiedEpoch)
}
Expand Down Expand Up @@ -95,7 +95,7 @@ func (s *Store) insert(ctx context.Context,

// Update parent with the best child and descendent only if it's available.
if n.Parent != nonExistentNode {
if err := s.updateBestChildAndDescendant(ctx, parentIndex, uint64(index)); err != nil {
if err := s.updateBestChildAndDescendant(parentIndex, uint64(index)); err != nil {
return err
}
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func (s *Store) applyWeightChanges(ctx context.Context, justifiedEpoch uint64, f
}
// Back propagate the nodes delta to its parent.
delta[n.Parent] += nodeDelta
if err := s.updateBestChildAndDescendant(ctx, n.Parent, uint64(i)); err != nil {
if err := s.updateBestChildAndDescendant(n.Parent, uint64(i)); err != nil {
return err
}
}
Expand All @@ -178,10 +178,7 @@ func (s *Store) applyWeightChanges(ctx context.Context, justifiedEpoch uint64, f
// 2.) The child is already the best child and the parent is updated with the new best descendant.
// 3.) The child is not the best child but becomes the best child.
// 4.) The child is not the best child and does not become best child.
func (s *Store) updateBestChildAndDescendant(ctx context.Context, parentIndex uint64, childIndex uint64) error {
ctx, span := trace.StartSpan(ctx, "protoArrayForkChoice.updateBestChildAndDescendant")
defer span.End()

func (s *Store) updateBestChildAndDescendant(parentIndex uint64, childIndex uint64) error {
// Protection against parent index out of bound, this should not happen.
if parentIndex >= uint64(len(s.nodes)) {
return errInvalidNodeIndex
Expand All @@ -195,7 +192,7 @@ func (s *Store) updateBestChildAndDescendant(ctx context.Context, parentIndex ui
child := s.nodes[childIndex]

// Is the child viable to become head? Based on justification and finalization rules.
childLeadsToViableHead, err := s.leadsToViableHead(ctx, child)
childLeadsToViableHead, err := s.leadsToViableHead(child)
if err != nil {
return err
}
Expand Down Expand Up @@ -228,7 +225,7 @@ func (s *Store) updateBestChildAndDescendant(ctx context.Context, parentIndex ui
}
bestChild := s.nodes[parent.bestChild]
// Is current parent's best child viable to be head? Based on justification and finalization rules.
bestChildLeadsToViableHead, err := s.leadsToViableHead(ctx, bestChild)
bestChildLeadsToViableHead, err := s.leadsToViableHead(bestChild)
if err != nil {
return err
}
Expand Down Expand Up @@ -351,10 +348,7 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte) error {
// leadsToViableHead returns true if the node or the best descendent of the node is viable for head.
// Any node with diff finalized or justified epoch than the ones in fork choice store
// should not be viable to head.
func (s *Store) leadsToViableHead(ctx context.Context, node *Node) (bool, error) {
ctx, span := trace.StartSpan(ctx, "protoArrayForkChoice.leadsToViableHead")
defer span.End()

func (s *Store) leadsToViableHead(node *Node) (bool, error) {
var bestDescendentViable bool
bestDescendentIndex := node.BestDescendent

Expand All @@ -367,20 +361,17 @@ func (s *Store) leadsToViableHead(ctx context.Context, node *Node) (bool, error)
}

bestDescendentNode := s.nodes[bestDescendentIndex]
bestDescendentViable = s.viableForHead(ctx, bestDescendentNode)
bestDescendentViable = s.viableForHead(bestDescendentNode)
}

// The node is viable as long as the best descendent is viable.
return bestDescendentViable || s.viableForHead(ctx, node), nil
return bestDescendentViable || s.viableForHead(node), nil
}

// viableForHead returns true if the node is viable to head.
// Any node with diff finalized or justified epoch than the ones in fork choice store
// should not be viable to head.
func (s *Store) viableForHead(ctx context.Context, node *Node) bool {
ctx, span := trace.StartSpan(ctx, "protoArrayForkChoice.viableForHead")
defer span.End()

func (s *Store) viableForHead(node *Node) bool {
// `node` is viable if its justified epoch and finalized epoch are the same as the one in `Store`.
// It's also viable if we are in genesis epoch.
justified := s.justifiedEpoch == node.justifiedEpoch || s.justifiedEpoch == 0
Expand Down
20 changes: 10 additions & 10 deletions beacon-chain/forkchoice/protoarray/nodes_test.go
View file
Expand Up @@ -219,7 +219,7 @@ func TestStore_UpdateBestChildAndDescendant_RemoveChild(t *testing.T) {
// Make parent's best child equal's to input child index and child is not viable.
s := &Store{nodes: []*Node{{bestChild: 1}, {}}, justifiedEpoch: 1, finalizedEpoch: 1}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 1); err != nil {
if err := s.updateBestChildAndDescendant(0, 1); err != nil {
t.Fatal(err)
}

Expand All @@ -236,7 +236,7 @@ func TestStore_UpdateBestChildAndDescendant_UpdateDescendant(t *testing.T) {
// Make parent's best child equal to child index and child is viable.
s := &Store{nodes: []*Node{{bestChild: 1}, {BestDescendent: nonExistentNode}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 1); err != nil {
if err := s.updateBestChildAndDescendant(0, 1); err != nil {
t.Fatal(err)
}

Expand All @@ -259,7 +259,7 @@ func TestStore_UpdateBestChildAndDescendant_ChangeChildByViability(t *testing.T)
{BestDescendent: nonExistentNode},
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 2); err != nil {
if err := s.updateBestChildAndDescendant(0, 2); err != nil {
t.Fatal(err)
}

Expand All @@ -282,7 +282,7 @@ func TestStore_UpdateBestChildAndDescendant_ChangeChildByWeight(t *testing.T) {
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1},
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1, Weight: 1}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 2); err != nil {
if err := s.updateBestChildAndDescendant(0, 2); err != nil {
t.Fatal(err)
}

Expand All @@ -304,7 +304,7 @@ func TestStore_UpdateBestChildAndDescendant_ChangeChildAtLeaf(t *testing.T) {
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1},
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 2); err != nil {
if err := s.updateBestChildAndDescendant(0, 2); err != nil {
t.Fatal(err)
}

Expand All @@ -327,7 +327,7 @@ func TestStore_UpdateBestChildAndDescendant_NoChangeByViability(t *testing.T) {
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1},
{BestDescendent: nonExistentNode}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 2); err != nil {
if err := s.updateBestChildAndDescendant(0, 2); err != nil {
t.Fatal(err)
}

Expand All @@ -350,7 +350,7 @@ func TestStore_UpdateBestChildAndDescendant_NoChangeByWeight(t *testing.T) {
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1, Weight: 1},
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 2); err != nil {
if err := s.updateBestChildAndDescendant(0, 2); err != nil {
t.Fatal(err)
}

Expand All @@ -372,7 +372,7 @@ func TestStore_UpdateBestChildAndDescendant_NoChangeAtLeaf(t *testing.T) {
{BestDescendent: nonExistentNode, justifiedEpoch: 1, finalizedEpoch: 1},
{BestDescendent: nonExistentNode}}}

if err := s.updateBestChildAndDescendant(context.Background(), 0, 2); err != nil {
if err := s.updateBestChildAndDescendant(0, 2); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -495,7 +495,7 @@ func TestStore_LeadsToViableHead(t *testing.T) {
finalizedEpoch: tc.finalizedEpoch,
nodes: []*Node{tc.n},
}
got, err := s.leadsToViableHead(context.Background(), tc.n)
got, err := s.leadsToViableHead(tc.n)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -524,7 +524,7 @@ func TestStore_ViableForHead(t *testing.T) {
justifiedEpoch: tc.justifiedEpoch,
finalizedEpoch: tc.finalizedEpoch,
}
if got := s.viableForHead(context.Background(), tc.n); got != tc.want {
if got := s.viableForHead(tc.n); got != tc.want {
t.Errorf("viableForHead() = %v, want %v", got, tc.want)
}
}
Expand Down

0 comments on commit 5be4fee

Please sign in to comment.