Clean up fork choice (#10226)

* Clean up fork choice

* Update beacon-chain/forkchoice/protoarray/proposer_boost_test.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update beacon-chain/forkchoice/protoarray/store_test.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update beacon-chain/forkchoice/protoarray/store_test.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update store.go

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
This commit is contained in:
terence tsao
2022-02-11 19:33:46 -08:00
committed by GitHub
parent 9dfb385160
commit c9f299b50a
9 changed files with 42 additions and 41 deletions

View File

@@ -113,7 +113,7 @@ func (s *Service) VerifyBlkDescendant(ctx context.Context, root [32]byte) error
}
if !bytes.Equal(bFinalizedRoot, fRoot[:]) {
err := fmt.Errorf("block %#x is not a descendent of the current finalized block slot %d, %#x != %#x",
err := fmt.Errorf("block %#x is not a descendant of the current finalized block slot %d, %#x != %#x",
bytesutil.Trunc(root[:]), finalizedBlk.Slot(), bytesutil.Trunc(bFinalizedRoot),
bytesutil.Trunc(fRoot[:]))
tracing.AnnotateError(span, err)

View File

@@ -100,7 +100,7 @@ func TestStore_OnBlock(t *testing.T) {
return b
}(),
s: st.Copy(),
wantErrString: "is not a descendent of the current finalized block",
wantErrString: "is not a descendant of the current finalized block",
},
{
name: "same slot as finalized block",
@@ -789,7 +789,7 @@ func TestVerifyBlkDescendant(t *testing.T) {
finalizedRoot: r1,
parentRoot: r,
},
wantedErr: "is not a descendent of the current finalized block slot",
wantedErr: "is not a descendant of the current finalized block slot",
},
{
name: "is descendant",

View File

@@ -8,7 +8,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/forkchoice/protoarray"
)
// ForkChoicer represents the full fork choice interface composed of all of the sub-interfaces.
// ForkChoicer represents the full fork choice interface composed of all the sub-interfaces.
type ForkChoicer interface {
HeadRetriever // to compute head.
BlockProcessor // to track new block for fork choice.

View File

@@ -1,7 +1,7 @@
/*
Package protoarray implements proto array fork choice as outlined:
https://github.com/protolambda/lmd-ghost#array-based-stateful-dag-proto_array
This was motivated by the the original implementation by Sigma Prime here:
This was motivated by the original implementation by Sigma Prime here:
https://github.com/sigp/lighthouse/pull/804
*/
package protoarray

View File

@@ -24,7 +24,7 @@ func computeDeltas(
oldBalance := uint64(0)
newBalance := uint64(0)
// Skip if validator has never voted for current root and next root (ie. if the
// Skip if validator has never voted for current root and next root (i.e. if the
// votes are zero hash aka genesis block), there's nothing to compute.
if vote.currentRoot == params.BeaconConfig().ZeroHash && vote.nextRoot == params.BeaconConfig().ZeroHash {
continue

View File

@@ -235,7 +235,7 @@ func TestUpdateSyncTipsWithValidRoots(t *testing.T) {
tests := []struct {
root [32]byte // the root of the new VALID block
tips map[[32]byte]types.Slot // the old synced tips
newtips map[[32]byte]types.Slot // the updated synced tips
newTips map[[32]byte]types.Slot // the updated synced tips
wantedErr error
}{
{
@@ -327,7 +327,7 @@ func TestUpdateSyncTipsWithValidRoots(t *testing.T) {
} else {
require.NoError(t, err)
f.syncedTips.RLock()
require.DeepEqual(t, f.syncedTips.validatedTips, tc.newtips)
require.DeepEqual(t, f.syncedTips.validatedTips, tc.newTips)
f.syncedTips.RUnlock()
}
}
@@ -345,7 +345,7 @@ func TestUpdateSyncTipsWithValidRoots(t *testing.T) {
// J(1) -- K(1) -- L(0)
//
// And every block in the Fork choice is optimistic. Synced_Tips contains a
// single block that is outside of Fork choice. The numbers in parenthesis are
// single block that is outside of Fork choice. The numbers in parentheses are
// the weights of the nodes before removal
//
func TestUpdateSyncTipsWithInvalidRoot(t *testing.T) {
@@ -535,15 +535,16 @@ func TestFindSyncedTip(t *testing.T) {
}
for _, tc := range tests {
f.store.nodesLock.RLock()
defer f.store.nodesLock.RUnlock()
node := f.store.nodes[f.store.nodesIndices[tc.root]]
syncedTips := &optimisticStore{
validatedTips: tc.tips,
}
syncedTips.RLock()
defer syncedTips.RUnlock()
idx, err := f.store.findSyncedTip(ctx, node, syncedTips)
require.NoError(t, err)
require.Equal(t, tc.wanted, f.store.nodes[idx].root)
f.store.nodesLock.RUnlock()
syncedTips.RUnlock()
}
}

View File

@@ -143,7 +143,7 @@ func TestForkChoice_BoostProposerRoot_PreventsExAnteAttack(t *testing.T) {
// Ancestors have the added weights of their children. Genesis is a special exception at 0 weight,
require.Equal(t, f.store.nodes[0].weight, uint64(0))
// Otherwise assuming a block, A, that is not-genesis:
// Otherwise, assuming a block, A, that is not-genesis:
//
// A -> B -> C
//
@@ -160,7 +160,7 @@ func TestForkChoice_BoostProposerRoot_PreventsExAnteAttack(t *testing.T) {
// (A: 54) -> (B: 44) -> (C: 24)
// \_->(D: 10)
//
// So B has its own weight, 10, and the sum of of both C and D thats why we see weight 54 in the
// So B has its own weight, 10, and the sum of both C and D. That's why we see weight 54 in the
// middle instead of the normal progression of (44 -> 34 -> 24).
require.Equal(t, f.store.nodes[1].weight, uint64(54))
require.Equal(t, f.store.nodes[2].weight, uint64(44))

View File

@@ -295,7 +295,7 @@ func (s *Store) head(ctx context.Context, justifiedRoot [32]byte) ([32]byte, err
justifiedNode := s.nodes[justifiedIndex]
bestDescendantIndex := justifiedNode.bestDescendant
// If the justified node doesn't have a best descendent,
// If the justified node doesn't have a best descendant,
// the best node is itself.
if bestDescendantIndex == NonExistentNode {
bestDescendantIndex = justifiedIndex
@@ -378,7 +378,7 @@ func (s *Store) insert(ctx context.Context,
index := uint64(len(s.nodes))
parentIndex, ok := s.nodesIndices[parent]
// Mark genesis block's parent as non existent.
// Mark genesis block's parent as non-existent.
if !ok {
parentIndex = NonExistentNode
}
@@ -398,7 +398,7 @@ func (s *Store) insert(ctx context.Context,
s.nodesIndices[root] = index
s.nodes = append(s.nodes, n)
// Update parent with the best child and descendent only if it's available.
// Update parent with the best child and descendant only if it's available.
if n.parent != NonExistentNode {
if err := s.updateBestChildAndDescendant(parentIndex, index); err != nil {
return err
@@ -414,8 +414,8 @@ func (s *Store) insert(ctx context.Context,
// applyWeightChanges iterates backwards through the nodes in store. It checks all nodes parent
// and its best child. For each node, it updates the weight with input delta and
// back propagate the nodes delta to its parents delta. After scoring changes,
// the best child is then updated along with best descendant.
// back propagate the nodes' delta to its parents' delta. After scoring changes,
// the best child is then updated along with the best descendant.
func (s *Store) applyWeightChanges(
ctx context.Context, justifiedEpoch, finalizedEpoch types.Epoch, newBalances []uint64, delta []int,
) error {
@@ -478,13 +478,13 @@ func (s *Store) applyWeightChanges(
n.weight += uint64(nodeDelta)
}
// Update parent's best child and descendent if the node has a known parent.
// Update parent's best child and descendant if the node has a known parent.
if n.parent != NonExistentNode {
// Protection against node parent index out of bound. This should not happen.
if int(n.parent) >= len(delta) {
return errInvalidParentDelta
}
// Back propagate the nodes delta to its parent.
// Back propagate the nodes' delta to its parent.
delta[n.parent] += nodeDelta
}
}
@@ -510,14 +510,14 @@ func (s *Store) applyWeightChanges(
return nil
}
// updateBestChildAndDescendant updates parent node's best child and descendent.
// updateBestChildAndDescendant updates parent node's best child and descendant.
// It looks at input parent node and input child node and potentially modifies parent's best
// child and best descendent indices.
// child and best descendant indices.
// There are four outcomes:
// 1.) The child is already the best child but it's now invalid due to a FFG change and should be removed.
// 1.) The child is already the best child, but it's now invalid due to a FFG change and should be removed.
// 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.
// 4.) The child is not the best child and does not become the best child.
func (s *Store) updateBestChildAndDescendant(parentIndex, childIndex uint64) error {
// Protection against parent index out of bound, this should not happen.
@@ -552,12 +552,12 @@ func (s *Store) updateBestChildAndDescendant(parentIndex, childIndex uint64) err
if parent.bestChild != NonExistentNode {
if parent.bestChild == childIndex && !childLeadsToViableHead {
// If the child is already the best child of the parent but it's not viable for head,
// If the child is already the best child of the parent, but it's not viable for head,
// we should remove it. (Outcome 1)
newParentChild = changeToNone
} else if parent.bestChild == childIndex {
// If the child is already the best child of the parent, set it again to ensure best
// descendent of the parent is updated. (Outcome 2)
// If the child is already the best child of the parent, set it again to ensure the best
// descendant of the parent is updated. (Outcome 2)
newParentChild = changeToChild
} else {
// Protection against parent's best child going out of bound.
@@ -572,7 +572,7 @@ func (s *Store) updateBestChildAndDescendant(parentIndex, childIndex uint64) err
}
if childLeadsToViableHead && !bestChildLeadsToViableHead {
// The child leads to a viable head, but the current parent's best child doesnt.
// The child leads to a viable head, but the current parent's best child doesn't.
newParentChild = changeToChild
} else if !childLeadsToViableHead && bestChildLeadsToViableHead {
// The child doesn't lead to a viable head, the current parent's best child does.
@@ -670,7 +670,7 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *o
}
s.nodesIndices[finalizedRoot] = uint64(0)
// Recompute best child and descendant for each canonical nodes.
// Recompute the best child and descendant for each canonical nodes.
for _, node := range canonicalNodes {
if node.bestChild != NonExistentNode {
node.bestChild = canonicalNodesMap[node.bestChild]
@@ -686,27 +686,27 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *o
return nil
}
// leadsToViableHead returns true if the node or the best descendent of the node is viable for head.
// leadsToViableHead returns true if the node or the best descendant 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(node *Node) (bool, error) {
var bestDescendentViable bool
bestDescendentIndex := node.bestDescendant
var bestDescendantViable bool
bestDescendantIndex := node.bestDescendant
// If the best descendant is not part of the leaves.
if bestDescendentIndex != NonExistentNode {
// Protection against out of bound, best descendent index can not be
if bestDescendantIndex != NonExistentNode {
// Protection against out of bound, the best descendant index can not be
// exceeds length of nodes list.
if bestDescendentIndex >= uint64(len(s.nodes)) {
if bestDescendantIndex >= uint64(len(s.nodes)) {
return false, errInvalidBestDescendantIndex
}
bestDescendentNode := s.nodes[bestDescendentIndex]
bestDescendentViable = s.viableForHead(bestDescendentNode)
bestDescendantNode := s.nodes[bestDescendantIndex]
bestDescendantViable = s.viableForHead(bestDescendantNode)
}
// The node is viable as long as the best descendent is viable.
return bestDescendentViable || s.viableForHead(node), nil
// The node is viable as long as the best descendant is viable.
return bestDescendantViable || s.viableForHead(node), nil
}
// viableForHead returns true if the node is viable to head.

View File

@@ -134,7 +134,7 @@ func TestStore_Head_BestDescendant(t *testing.T) {
indices := make(map[[32]byte]uint64)
indices[r] = 0
// Since the justified node's best descendent is at index 1 and it's root is `best`,
// Since the justified node's best descendant is at index 1, and its root is `best`,
// the head should be `best`.
s := &Store{nodesIndices: indices, nodes: []*Node{{root: r, bestDescendant: 1}, {root: best}}, canonicalNodes: make(map[[32]byte]bool)}
h, err := s.head(context.Background(), r)
@@ -265,7 +265,7 @@ func TestStore_UpdateBestChildAndDescendant_UpdateDescendant(t *testing.T) {
func TestStore_UpdateBestChildAndDescendant_ChangeChildByViability(t *testing.T) {
// Make parent's best child not equal to child index, child leads to viable index and
// parents best child doesnt lead to viable index.
// parent's best child doesn't lead to viable index.
s := &Store{
justifiedEpoch: 1,
finalizedEpoch: 1,