diff --git a/beacon-chain/blockchain/chain_info.go b/beacon-chain/blockchain/chain_info.go index 29dcb7be5..e2ca447fc 100644 --- a/beacon-chain/blockchain/chain_info.go +++ b/beacon-chain/blockchain/chain_info.go @@ -331,7 +331,7 @@ func (s *Service) IsOptimistic(ctx context.Context) (bool, error) { // IsOptimisticForRoot takes the root and slot as arguments instead of the current head // and returns true if it is optimistic. func (s *Service) IsOptimisticForRoot(ctx context.Context, root [32]byte) (bool, error) { - optimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(ctx, root) + optimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(root) if err == nil { return optimistic, nil } diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index d1adcd215..508060ef0 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -262,7 +262,7 @@ func (s *Service) onBlock(ctx context.Context, signed block.SignedBeaconBlock, b if err := s.cfg.ForkChoiceStore.Prune(ctx, fRoot); err != nil { return errors.Wrap(err, "could not prune proto array fork choice nodes") } - isOptimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(ctx, fRoot) + isOptimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(fRoot) if err != nil { return errors.Wrap(err, "could not check if node is optimistically synced") } diff --git a/beacon-chain/blockchain/process_block_helpers.go b/beacon-chain/blockchain/process_block_helpers.go index fbde2f842..c8601989a 100644 --- a/beacon-chain/blockchain/process_block_helpers.go +++ b/beacon-chain/blockchain/process_block_helpers.go @@ -247,7 +247,7 @@ func (s *Service) updateFinalized(ctx context.Context, cp *ethpb.Checkpoint) err } fRoot := bytesutil.ToBytes32(cp.Root) - optimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(ctx, fRoot) + optimistic, err := s.cfg.ForkChoiceStore.IsOptimistic(fRoot) if err != nil { return err } diff --git a/beacon-chain/forkchoice/doubly-linked-tree/errors.go b/beacon-chain/forkchoice/doubly-linked-tree/errors.go index 3bcd17af7..6898e67d0 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/errors.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/errors.go @@ -8,3 +8,4 @@ var errInvalidProposerBoostRoot = errors.New("invalid proposer boost root") var errUnknownFinalizedRoot = errors.New("unknown finalized root") var errUnknownJustifiedRoot = errors.New("unknown justified root") var errInvalidOptimisticStatus = errors.New("invalid optimistic status") +var errUnknownPayloadHash = errors.New("unknown payload hash") diff --git a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go index b86dafce9..a64ecf212 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go @@ -18,6 +18,7 @@ func New(justifiedEpoch, finalizedEpoch types.Epoch) *ForkChoice { finalizedEpoch: finalizedEpoch, proposerBoostRoot: [32]byte{}, nodeByRoot: make(map[[fieldparams.RootLength]byte]*Node), + nodeByPayload: make(map[[fieldparams.RootLength]byte]*Node), pruneThreshold: defaultPruneThreshold, } @@ -168,7 +169,7 @@ func (f *ForkChoice) IsCanonical(root [32]byte) bool { } // IsOptimistic returns true if the given root has been optimistically synced. -func (f *ForkChoice) IsOptimistic(_ context.Context, root [32]byte) (bool, error) { +func (f *ForkChoice) IsOptimistic(root [32]byte) (bool, error) { f.store.nodesLock.RLock() defer f.store.nodesLock.RUnlock() @@ -302,6 +303,6 @@ func (f *ForkChoice) ForkChoiceNodes() []*pbrpc.ForkChoiceNode { } // SetOptimisticToInvalid removes a block with an invalid execution payload from fork choice store -func (f *ForkChoice) SetOptimisticToInvalid(ctx context.Context, root [fieldparams.RootLength]byte) ([][32]byte, error) { - return f.store.removeNode(ctx, root) +func (f *ForkChoice) SetOptimisticToInvalid(ctx context.Context, root, payloadHash [fieldparams.RootLength]byte) ([][32]byte, error) { + return f.store.setOptimisticToInvalid(ctx, root, payloadHash) } diff --git a/beacon-chain/forkchoice/doubly-linked-tree/node.go b/beacon-chain/forkchoice/doubly-linked-tree/node.go index 0e2025ecd..297cb20e3 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/node.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/node.go @@ -109,7 +109,7 @@ func (n *Node) leadsToViableHead(justifiedEpoch, finalizedEpoch types.Epoch) boo return n.bestDescendant.viableForHead(justifiedEpoch, finalizedEpoch) } -// setNodeAndParentValidated sets the current node and the parent as validated (i.e. non-optimistic). +// setNodeAndParentValidated sets the current node and all the ancestors as validated (i.e. non-optimistic). func (n *Node) setNodeAndParentValidated(ctx context.Context) error { if ctx.Err() != nil { return ctx.Err() diff --git a/beacon-chain/forkchoice/doubly-linked-tree/node_test.go b/beacon-chain/forkchoice/doubly-linked-tree/node_test.go index 10fdd00c9..7d4d8801d 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/node_test.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/node_test.go @@ -180,27 +180,27 @@ func TestNode_SetFullyValidated(t *testing.T) { require.NoError(t, f.InsertOptimisticBlock(ctx, 4, indexToHash(4), indexToHash(3), params.BeaconConfig().ZeroHash, 1, 1)) require.NoError(t, f.InsertOptimisticBlock(ctx, 5, indexToHash(5), indexToHash(1), params.BeaconConfig().ZeroHash, 1, 1)) - opt, err := f.IsOptimistic(ctx, indexToHash(5)) + opt, err := f.IsOptimistic(indexToHash(5)) require.NoError(t, err) require.Equal(t, true, opt) - opt, err = f.IsOptimistic(ctx, indexToHash(4)) + opt, err = f.IsOptimistic(indexToHash(4)) require.NoError(t, err) require.Equal(t, true, opt) require.NoError(t, f.store.nodeByRoot[indexToHash(4)].setNodeAndParentValidated(ctx)) // block 5 should still be optimistic - opt, err = f.IsOptimistic(ctx, indexToHash(5)) + opt, err = f.IsOptimistic(indexToHash(5)) require.NoError(t, err) require.Equal(t, true, opt) // block 4 and 3 should now be valid - opt, err = f.IsOptimistic(ctx, indexToHash(4)) + opt, err = f.IsOptimistic(indexToHash(4)) require.NoError(t, err) require.Equal(t, false, opt) - opt, err = f.IsOptimistic(ctx, indexToHash(3)) + opt, err = f.IsOptimistic(indexToHash(3)) require.NoError(t, err) require.Equal(t, false, opt) } diff --git a/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync.go b/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync.go index 11b62b912..1afba0587 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync.go @@ -2,22 +2,54 @@ package doublylinkedtree import ( "context" + + "github.com/prysmaticlabs/prysm/config/params" ) +func (s *Store) setOptimisticToInvalid(ctx context.Context, root, payloadHash [32]byte) ([][32]byte, error) { + s.nodesLock.Lock() + invalidRoots := make([][32]byte, 0) + node, ok := s.nodeByRoot[root] + if !ok || node == nil { + s.nodesLock.Unlock() + return invalidRoots, ErrNilNode + } + // Check if last valid hash is an ancestor of the passed node. + lastValid, ok := s.nodeByPayload[payloadHash] + if !ok || lastValid == nil { + s.nodesLock.Unlock() + return invalidRoots, errUnknownPayloadHash + } + firstInvalid := node + for ; firstInvalid.parent != nil && firstInvalid.parent.payloadHash != payloadHash; firstInvalid = firstInvalid.parent { + if ctx.Err() != nil { + s.nodesLock.Unlock() + return invalidRoots, ctx.Err() + } + } + // If the last valid payload is in a different fork, we remove only the + // passed node. + if firstInvalid.parent == nil { + firstInvalid = node + } + s.nodesLock.Unlock() + return s.removeNode(ctx, firstInvalid) +} + // removeNode removes the node with the given root and all of its children // from the Fork Choice Store. -func (s *Store) removeNode(ctx context.Context, root [32]byte) ([][32]byte, error) { +func (s *Store) removeNode(ctx context.Context, node *Node) ([][32]byte, error) { s.nodesLock.Lock() defer s.nodesLock.Unlock() invalidRoots := make([][32]byte, 0) - node, ok := s.nodeByRoot[root] - if !ok || node == nil { + if node == nil { return invalidRoots, ErrNilNode } if !node.optimistic || node.parent == nil { return invalidRoots, errInvalidOptimisticStatus } + children := node.parent.children if len(children) == 1 { node.parent.children = []*Node{} @@ -47,6 +79,16 @@ func (s *Store) removeNodeAndChildren(ctx context.Context, node *Node, invalidRo } } invalidRoots = append(invalidRoots, node.root) + s.proposerBoostLock.Lock() + if node.root == s.proposerBoostRoot { + s.proposerBoostRoot = [32]byte{} + } + if node.root == s.previousProposerBoostRoot { + s.previousProposerBoostRoot = params.BeaconConfig().ZeroHash + s.previousProposerBoostScore = 0 + } + s.proposerBoostLock.Unlock() delete(s.nodeByRoot, node.root) + delete(s.nodeByPayload, node.payloadHash) return invalidRoots, nil } diff --git a/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync_test.go b/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync_test.go index 7d6b3b8e9..19f5d6957 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync_test.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/optimistic_sync_test.go @@ -24,32 +24,71 @@ import ( func TestPruneInvalid(t *testing.T) { tests := []struct { root [32]byte // the root of the new INVALID block + payload [32]byte // the last valid hash wantedNodeNumber int wantedRoots [][32]byte }{ { [32]byte{'j'}, + [32]byte{'B'}, 12, [][32]byte{[32]byte{'j'}}, }, { [32]byte{'c'}, + [32]byte{'B'}, 4, [][32]byte{[32]byte{'f'}, [32]byte{'e'}, [32]byte{'i'}, [32]byte{'h'}, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'g'}, [32]byte{'d'}, [32]byte{'c'}}, }, { [32]byte{'i'}, + [32]byte{'H'}, 12, [][32]byte{[32]byte{'i'}}, }, { [32]byte{'h'}, + [32]byte{'G'}, 11, [][32]byte{[32]byte{'i'}, [32]byte{'h'}}, }, { [32]byte{'g'}, + [32]byte{'D'}, + 8, + [][32]byte{[32]byte{'i'}, [32]byte{'h'}, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'g'}}, + }, + { + [32]byte{'i'}, + [32]byte{'D'}, + 8, + [][32]byte{[32]byte{'i'}, [32]byte{'h'}, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'g'}}, + }, + { + [32]byte{'f'}, + [32]byte{'D'}, + 11, + [][32]byte{[32]byte{'f'}, [32]byte{'e'}}, + }, + { + [32]byte{'h'}, + [32]byte{'C'}, + 5, + [][32]byte{ + [32]byte{'f'}, + [32]byte{'e'}, + [32]byte{'i'}, + [32]byte{'h'}, + [32]byte{'l'}, + [32]byte{'k'}, + [32]byte{'g'}, + [32]byte{'d'}, + }, + }, + { + [32]byte{'g'}, + [32]byte{'E'}, 8, [][32]byte{[32]byte{'i'}, [32]byte{'h'}, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'g'}}, }, @@ -58,22 +97,45 @@ func TestPruneInvalid(t *testing.T) { ctx := context.Background() f := setup(1, 1) - require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, params.BeaconConfig().ZeroHash, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, [32]byte{'J'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, [32]byte{'D'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, [32]byte{'E'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, [32]byte{'G'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, [32]byte{'F'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, [32]byte{'H'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, [32]byte{'K'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, [32]byte{'I'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'L'}, 1, 1)) - roots, err := f.store.removeNode(context.Background(), tc.root) + roots, err := f.store.setOptimisticToInvalid(context.Background(), tc.root, tc.payload) require.NoError(t, err) require.DeepEqual(t, tc.wantedRoots, roots) require.Equal(t, tc.wantedNodeNumber, f.NodeCount()) } } + +// This is a regression test (10445) +func TestSetOptimisticToInvalid_ProposerBoost(t *testing.T) { + ctx := context.Background() + f := setup(1, 1) + + require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 1, 1)) + f.store.proposerBoostLock.Lock() + f.store.proposerBoostRoot = [32]byte{'c'} + f.store.previousProposerBoostScore = 10 + f.store.previousProposerBoostRoot = [32]byte{'b'} + f.store.proposerBoostLock.Unlock() + + _, err := f.SetOptimisticToInvalid(ctx, [32]byte{'c'}, [32]byte{'A'}) + require.NoError(t, err) + f.store.proposerBoostLock.RLock() + require.Equal(t, uint64(0), f.store.previousProposerBoostScore) + require.DeepEqual(t, [32]byte{}, f.store.proposerBoostRoot) + require.DeepEqual(t, params.BeaconConfig().ZeroHash, f.store.previousProposerBoostRoot) + f.store.proposerBoostLock.RUnlock() +} diff --git a/beacon-chain/forkchoice/doubly-linked-tree/store.go b/beacon-chain/forkchoice/doubly-linked-tree/store.go index cc0b50560..b8c1bdaba 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/store.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/store.go @@ -121,6 +121,7 @@ func (s *Store) insert(ctx context.Context, payloadHash: payloadHash, } + s.nodeByPayload[payloadHash] = n s.nodeByRoot[root] = n if parent != nil { parent.children = append(parent.children, n) diff --git a/beacon-chain/forkchoice/doubly-linked-tree/store_test.go b/beacon-chain/forkchoice/doubly-linked-tree/store_test.go index 858ee9aa8..806d6c4da 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/store_test.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/store_test.go @@ -107,7 +107,8 @@ func TestStore_Insert(t *testing.T) { // The new node does not have a parent. treeRootNode := &Node{slot: 0, root: indexToHash(0)} nodeByRoot := map[[32]byte]*Node{indexToHash(0): treeRootNode} - s := &Store{nodeByRoot: nodeByRoot, treeRootNode: treeRootNode} + nodeByPayload := map[[32]byte]*Node{indexToHash(0): treeRootNode} + s := &Store{nodeByRoot: nodeByRoot, treeRootNode: treeRootNode, nodeByPayload: nodeByPayload} payloadHash := [32]byte{'a'} require.NoError(t, s.insert(context.Background(), 100, indexToHash(100), indexToHash(0), payloadHash, 1, 1)) assert.Equal(t, 2, len(s.nodeByRoot), "Did not insert block") diff --git a/beacon-chain/forkchoice/doubly-linked-tree/types.go b/beacon-chain/forkchoice/doubly-linked-tree/types.go index 9f3ad0f02..bd82a2db3 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/types.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/types.go @@ -26,6 +26,7 @@ type Store struct { treeRootNode *Node // the root node of the store tree. headNode *Node // last head Node nodeByRoot map[[fieldparams.RootLength]byte]*Node // nodes indexed by roots. + nodeByPayload map[[fieldparams.RootLength]byte]*Node // nodes indexed by payload Hash nodesLock sync.RWMutex proposerBoostLock sync.RWMutex } diff --git a/beacon-chain/forkchoice/interfaces.go b/beacon-chain/forkchoice/interfaces.go index aaeec3d86..35e6817f4 100644 --- a/beacon-chain/forkchoice/interfaces.go +++ b/beacon-chain/forkchoice/interfaces.go @@ -24,7 +24,7 @@ type ForkChoicer interface { type HeadRetriever interface { Head(context.Context, types.Epoch, [32]byte, []uint64, types.Epoch) ([32]byte, error) Tips() ([][32]byte, []types.Slot) - IsOptimistic(ctx context.Context, root [32]byte) (bool, error) + IsOptimistic(root [32]byte) (bool, error) } // BlockProcessor processes the block that's used for accounting fork choice. @@ -71,5 +71,5 @@ type Getter interface { // Setter allows to set forkchoice information type Setter interface { SetOptimisticToValid(context.Context, [fieldparams.RootLength]byte) error - SetOptimisticToInvalid(context.Context, [fieldparams.RootLength]byte) ([][32]byte, error) + SetOptimisticToInvalid(context.Context, [fieldparams.RootLength]byte, [fieldparams.RootLength]byte) ([][32]byte, error) } diff --git a/beacon-chain/forkchoice/protoarray/errors.go b/beacon-chain/forkchoice/protoarray/errors.go index a9e201fff..795d9e43c 100644 --- a/beacon-chain/forkchoice/protoarray/errors.go +++ b/beacon-chain/forkchoice/protoarray/errors.go @@ -5,11 +5,11 @@ import "errors" var errUnknownFinalizedRoot = errors.New("unknown finalized root") var errUnknownJustifiedRoot = errors.New("unknown justified root") var errInvalidNodeIndex = errors.New("node index is invalid") +var errInvalidFinalizedNode = errors.New("invalid finalized block on chain") var ErrUnknownNodeRoot = errors.New("unknown block root") var errInvalidJustifiedIndex = errors.New("justified index is invalid") -var errInvalidBestChildIndex = errors.New("best child index is invalid") var errInvalidBestDescendantIndex = errors.New("best descendant index is invalid") var errInvalidParentDelta = errors.New("parent delta is invalid") var errInvalidNodeDelta = errors.New("node delta is invalid") var errInvalidDeltaLength = errors.New("delta length is invalid") -var errInvalidSyncedTips = errors.New("invalid synced tips") +var errInvalidOptimisticStatus = errors.New("invalid optimistic status") diff --git a/beacon-chain/forkchoice/protoarray/helpers.go b/beacon-chain/forkchoice/protoarray/helpers.go index 307bff069..09e5bbba3 100644 --- a/beacon-chain/forkchoice/protoarray/helpers.go +++ b/beacon-chain/forkchoice/protoarray/helpers.go @@ -85,12 +85,9 @@ func copyNode(node *Node) *Node { return &Node{} } - copiedRoot := [32]byte{} - copy(copiedRoot[:], node.root[:]) - return &Node{ slot: node.slot, - root: copiedRoot, + root: node.root, parent: node.parent, payloadHash: node.payloadHash, justifiedEpoch: node.justifiedEpoch, @@ -98,5 +95,6 @@ func copyNode(node *Node) *Node { weight: node.weight, bestChild: node.bestChild, bestDescendant: node.bestDescendant, + status: node.status, } } diff --git a/beacon-chain/forkchoice/protoarray/metrics.go b/beacon-chain/forkchoice/protoarray/metrics.go index daa2d3e08..a97b4010c 100644 --- a/beacon-chain/forkchoice/protoarray/metrics.go +++ b/beacon-chain/forkchoice/protoarray/metrics.go @@ -48,16 +48,10 @@ var ( Help: "The number of times pruning happened.", }, ) - lastSyncedTipSlot = promauto.NewGauge( - prometheus.GaugeOpts{ - Name: "proto_array_last_synced_tip_slot", - Help: "The slot of the last fully validated block added to the proto array.", - }, - ) - syncedTipsCount = promauto.NewGauge( - prometheus.GaugeOpts{ - Name: "proto_array_synced_tips_count", - Help: "The number of elements in the syncedTips structure.", + validatedNodesCount = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "proto_array_validated_nodes_count", + Help: "The number of nodes that have been fully validated.", }, ) ) diff --git a/beacon-chain/forkchoice/protoarray/node.go b/beacon-chain/forkchoice/protoarray/node.go index 5c49871fa..8a7946dad 100644 --- a/beacon-chain/forkchoice/protoarray/node.go +++ b/beacon-chain/forkchoice/protoarray/node.go @@ -43,8 +43,3 @@ func (n *Node) BestChild() uint64 { func (n *Node) BestDescendant() uint64 { return n.bestDescendant } - -// Graffiti of the fork choice node. -func (n *Node) Graffiti() [32]byte { - return n.graffiti -} diff --git a/beacon-chain/forkchoice/protoarray/node_test.go b/beacon-chain/forkchoice/protoarray/node_test.go index de29d5a1d..5df00f850 100644 --- a/beacon-chain/forkchoice/protoarray/node_test.go +++ b/beacon-chain/forkchoice/protoarray/node_test.go @@ -16,7 +16,6 @@ func TestNode_Getters(t *testing.T) { weight := uint64(10000) bestChild := uint64(5) bestDescendant := uint64(4) - graffiti := [32]byte{'b'} n := &Node{ slot: slot, root: root, @@ -26,7 +25,6 @@ func TestNode_Getters(t *testing.T) { weight: weight, bestChild: bestChild, bestDescendant: bestDescendant, - graffiti: graffiti, } require.Equal(t, slot, n.Slot()) @@ -37,5 +35,4 @@ func TestNode_Getters(t *testing.T) { require.Equal(t, weight, n.Weight()) require.Equal(t, bestChild, n.BestChild()) require.Equal(t, bestDescendant, n.BestDescendant()) - require.Equal(t, graffiti, n.Graffiti()) } diff --git a/beacon-chain/forkchoice/protoarray/optimistic_sync.go b/beacon-chain/forkchoice/protoarray/optimistic_sync.go index a99af3756..0c7bc6053 100644 --- a/beacon-chain/forkchoice/protoarray/optimistic_sync.go +++ b/beacon-chain/forkchoice/protoarray/optimistic_sync.go @@ -3,314 +3,160 @@ package protoarray import ( "context" - types "github.com/prysmaticlabs/eth2-types" "github.com/prysmaticlabs/prysm/config/params" ) -// This returns the minimum and maximum slot of the synced_tips tree -func (f *ForkChoice) boundarySyncedTips() (types.Slot, types.Slot) { - f.syncedTips.RLock() - defer f.syncedTips.RUnlock() - - min := params.BeaconConfig().FarFutureSlot - max := types.Slot(0) - for _, slot := range f.syncedTips.validatedTips { - if slot > max { - max = slot - } - if slot < min { - min = slot - } - } - return min, max -} - // IsOptimistic returns true if this node is optimistically synced // A optimistically synced block is synced as usual, but its // execution payload is not validated, while the EL is still syncing. // This function returns an error if the block is not found in the fork choice // store -func (f *ForkChoice) IsOptimistic(ctx context.Context, root [32]byte) (bool, error) { - if ctx.Err() != nil { - return false, ctx.Err() - } +func (f *ForkChoice) IsOptimistic(root [32]byte) (bool, error) { f.store.nodesLock.RLock() + defer f.store.nodesLock.RUnlock() index, ok := f.store.nodesIndices[root] if !ok { - f.store.nodesLock.RUnlock() return false, ErrUnknownNodeRoot } node := f.store.nodes[index] - slot := node.slot - - // If the node is a synced tip, then it's fully validated - f.syncedTips.RLock() - _, ok = f.syncedTips.validatedTips[root] - if ok { - f.syncedTips.RUnlock() - f.store.nodesLock.RUnlock() - return false, nil - } - f.syncedTips.RUnlock() - - // If the slot is higher than the max synced tip, it's optimistic - min, max := f.boundarySyncedTips() - if slot > max { - f.store.nodesLock.RUnlock() - return true, nil - } - - // If the slot is lower than the min synced tip, it's fully validated - if slot <= min { - f.store.nodesLock.RUnlock() - return false, nil - } - - // if the node is a leaf of the Fork Choice tree, then it's - // optimistic - childIndex := node.BestChild() - if childIndex == NonExistentNode { - f.store.nodesLock.RUnlock() - return true, nil - } - - // recurse to the child - child := f.store.nodes[childIndex] - root = child.root - f.store.nodesLock.RUnlock() - return f.IsOptimistic(ctx, root) -} - -// This function returns the index of sync tip node that's ancestor to the input node. -// In the event of none, `NonExistentNode` is returned. -// This internal method assumes the caller holds a lock on syncedTips and s.nodesLock -func (s *Store) findSyncedTip(ctx context.Context, node *Node, syncedTips *optimisticStore) (uint64, error) { - for { - if ctx.Err() != nil { - return 0, ctx.Err() - } - if _, ok := syncedTips.validatedTips[node.root]; ok { - return s.nodesIndices[node.root], nil - } - if node.parent == NonExistentNode { - return NonExistentNode, nil - } - node = s.nodes[node.parent] - } + return node.status == syncing, nil } // SetOptimisticToValid is called with the root of a block that was returned as -// VALID by the EL. This routine recomputes and updates the synced_tips map to -// account for this new tip. -// WARNING: This method returns an error if the root is not found in forkchoice or -// if the root is not a leaf of the fork choice tree. +// VALID by the EL. +// WARNING: This method returns an error if the root is not found in forkchoice func (f *ForkChoice) SetOptimisticToValid(ctx context.Context, root [32]byte) error { - f.store.nodesLock.RLock() + f.store.nodesLock.Lock() + defer f.store.nodesLock.Unlock() // We can only update if given root is in Fork Choice index, ok := f.store.nodesIndices[root] if !ok { - return errInvalidNodeIndex + return ErrUnknownNodeRoot } - node := f.store.nodes[index] - f.store.nodesLock.RUnlock() - // Stop early if the node is Valid - optimistic, err := f.IsOptimistic(ctx, root) - if err != nil { - return err - } - if !optimistic { - return nil - } - f.store.nodesLock.RLock() - defer f.store.nodesLock.RUnlock() - - // Cache root and slot to validated tips - newTips := make(map[[32]byte]types.Slot) - newValidSlot := node.slot - newTips[root] = newValidSlot - - // Compute the full valid path from the given node to its previous synced tip - // This path will now consist of fully validated blocks. Notice that - // the previous tip may have been outside the Fork Choice store. - // In this case, only one block can be in syncedTips as the whole - // Fork Choice would be a descendant of this block. - validPath := make(map[uint64]bool) - validPath[index] = true - for { + for node := f.store.nodes[index]; node.status == syncing; node = f.store.nodes[index] { if ctx.Err() != nil { return ctx.Err() } - - parentIndex := node.parent - if parentIndex == NonExistentNode { + node.status = valid + index = node.parent + if index == NonExistentNode { break } - if parentIndex >= uint64(len(f.store.nodes)) { - return errInvalidNodeIndex - } - node = f.store.nodes[parentIndex] - _, ok = f.syncedTips.validatedTips[node.root] - if ok { - break - } - validPath[parentIndex] = true + validatedNodesCount.Inc() } - - // Retrieve the list of leaves in the Fork Choice - // These are all the nodes that have NonExistentNode as best child. - leaves, err := f.store.leaves() - if err != nil { - return err - } - - // For each leaf, recompute the new tip. - for _, i := range leaves { - node = f.store.nodes[i] - j := i - for { - if ctx.Err() != nil { - return ctx.Err() - } - // Stop if we reached the previous tip - _, ok = f.syncedTips.validatedTips[node.root] - if ok { - newTips[node.root] = node.slot - break - } - - // Stop if we reach valid path - _, ok = validPath[j] - if ok { - newTips[node.root] = node.slot - break - } - - j = node.parent - if j == NonExistentNode { - break - } - if j >= uint64(len(f.store.nodes)) { - return errInvalidNodeIndex - } - node = f.store.nodes[j] - } - } - - f.syncedTips.validatedTips = newTips - lastSyncedTipSlot.Set(float64(newValidSlot)) - syncedTipsCount.Set(float64(len(newTips))) return nil } -// SetOptimisticToInvalid updates the synced_tips map when the block with the given root becomes INVALID. -func (f *ForkChoice) SetOptimisticToInvalid(ctx context.Context, root [32]byte) ([][32]byte, error) { +// SetOptimisticToInvalid updates the synced_tips map when the block with the given root becomes INVALID. +// It takes two parameters: the root of the INVALID block and the payload Hash +// of the last valid block.s +func (f *ForkChoice) SetOptimisticToInvalid(ctx context.Context, root, payloadHash [32]byte) ([][32]byte, error) { f.store.nodesLock.Lock() defer f.store.nodesLock.Unlock() invalidRoots := make([][32]byte, 0) - idx, ok := f.store.nodesIndices[root] + // We only support setting invalid a node existing in Forkchoice + invalidIndex, ok := f.store.nodesIndices[root] if !ok { - return invalidRoots, errInvalidNodeIndex + return invalidRoots, ErrUnknownNodeRoot } - node := f.store.nodes[idx] - // We only support changing status for the tips in Fork Choice store. - if node.bestChild != NonExistentNode { - return invalidRoots, errInvalidNodeIndex + node := f.store.nodes[invalidIndex] + + lastValidIndex, ok := f.store.payloadIndices[payloadHash] + if !ok || lastValidIndex == NonExistentNode { + return invalidRoots, errInvalidFinalizedNode } - parentIndex := node.parent - // This should not happen - if parentIndex == NonExistentNode { - return invalidRoots, errInvalidNodeIndex + // Check if last valid hash is an ancestor of the passed node + firstInvalidIndex := node.parent + for ; firstInvalidIndex != NonExistentNode && firstInvalidIndex != lastValidIndex; firstInvalidIndex = node.parent { + node = f.store.nodes[firstInvalidIndex] } - // Update the weights of the nodes subtracting the INVALID node's weight + + // if the last valid hash is not an ancestor of the invalid block, we + // just remove the invalid block. + if node.parent != lastValidIndex { + node = f.store.nodes[invalidIndex] + firstInvalidIndex = invalidIndex + lastValidIndex = node.parent + if lastValidIndex == NonExistentNode { + return invalidRoots, errInvalidFinalizedNode + } + } else { + firstInvalidIndex = f.store.nodesIndices[node.root] + } + + // Update the weights of the nodes subtracting the first INVALID node's weight weight := node.weight - node = f.store.nodes[parentIndex] - for { - if ctx.Err() != nil { - return invalidRoots, ctx.Err() - } - node.weight -= weight - if node.parent == NonExistentNode { - break - } - node = f.store.nodes[node.parent] - } - parent := copyNode(f.store.nodes[parentIndex]) - - // delete the invalid node, order is important - f.store.nodes = append(f.store.nodes[:idx], f.store.nodes[idx+1:]...) - delete(f.store.nodesIndices, root) - invalidRoots = append(invalidRoots, root) - // Fix parent and best child for each node - for _, node := range f.store.nodes { - if node.parent == NonExistentNode { - node.parent = NonExistentNode - } else if node.parent > idx { - node.parent -= 1 - } - if node.bestChild == NonExistentNode || node.bestChild == idx { - node.bestChild = NonExistentNode - } else if node.bestChild > idx { - node.bestChild -= 1 - } - if node.bestDescendant == NonExistentNode || node.bestDescendant == idx { - node.bestDescendant = NonExistentNode - } else if node.bestDescendant > idx { - node.bestDescendant -= 1 - } + var validNode *Node + for index := lastValidIndex; index != NonExistentNode; index = validNode.parent { + validNode = f.store.nodes[index] + validNode.weight -= weight } - // Update the parent's best child and best descendant if necessary. - if parent.bestChild == idx || parent.bestDescendant == idx { - for childIndex, child := range f.store.nodes { - if child.parent == parentIndex { - err := f.store.updateBestChildAndDescendant( - parentIndex, uint64(childIndex)) - if err != nil { - return invalidRoots, err - } - break + // Find the current proposer boost (it should be set to zero if an + // INVALID block was boosted) + f.store.proposerBoostLock.RLock() + boostRoot := f.store.proposerBoostRoot + previousBoostRoot := f.store.previousProposerBoostRoot + f.store.proposerBoostLock.RUnlock() + + // Remove the invalid roots from our store maps and adjust their weight + // to zero + boosted := node.root == boostRoot + previouslyBoosted := node.root == previousBoostRoot + + invalidIndices := map[uint64]bool{firstInvalidIndex: true} + node.status = invalid + node.weight = 0 + delete(f.store.nodesIndices, node.root) + delete(f.store.canonicalNodes, node.root) + delete(f.store.payloadIndices, node.payloadHash) + for index := firstInvalidIndex + 1; index < uint64(len(f.store.nodes)); index++ { + invalidNode := f.store.nodes[index] + if _, ok := invalidIndices[invalidNode.parent]; !ok { + continue + } + if invalidNode.status == valid { + return invalidRoots, errInvalidOptimisticStatus + } + if !boosted && invalidNode.root == boostRoot { + boosted = true + } + if !previouslyBoosted && invalidNode.root == previousBoostRoot { + previouslyBoosted = true + } + invalidNode.status = invalid + invalidIndices[index] = true + invalidNode.weight = 0 + delete(f.store.nodesIndices, invalidNode.root) + delete(f.store.canonicalNodes, invalidNode.root) + delete(f.store.payloadIndices, invalidNode.payloadHash) + } + if boosted { + if err := f.ResetBoostedProposerRoot(ctx); err != nil { + return invalidRoots, err + } + } + if previouslyBoosted { + f.store.proposerBoostLock.Lock() + f.store.previousProposerBoostRoot = params.BeaconConfig().ZeroHash + f.store.previousProposerBoostScore = 0 + f.store.proposerBoostLock.Unlock() + } + + for index := range invalidIndices { + invalidRoots = append(invalidRoots, f.store.nodes[index].root) + } + + // Update the best child and descendant + for i := len(f.store.nodes) - 1; i >= 0; i-- { + n := f.store.nodes[i] + if n.parent != NonExistentNode { + if err := f.store.updateBestChildAndDescendant(n.parent, uint64(i)); err != nil { + return invalidRoots, err } } } - - // Return early if the parent is not a synced_tip. - f.syncedTips.Lock() - defer f.syncedTips.Unlock() - parentRoot := parent.root - _, ok = f.syncedTips.validatedTips[parentRoot] - if !ok { - return invalidRoots, nil - } - - leaves, err := f.store.leaves() - if err != nil { - return invalidRoots, err - } - - for _, i := range leaves { - node = f.store.nodes[i] - for { - if ctx.Err() != nil { - return invalidRoots, ctx.Err() - } - - // Return early if the parent is still a synced tip - if node.root == parentRoot { - return invalidRoots, nil - } - _, ok = f.syncedTips.validatedTips[node.root] - if ok { - break - } - if node.parent == NonExistentNode { - break - } - node = f.store.nodes[node.parent] - } - } - delete(f.syncedTips.validatedTips, parentRoot) - syncedTipsCount.Set(float64(len(f.syncedTips.validatedTips))) return invalidRoots, nil } diff --git a/beacon-chain/forkchoice/protoarray/optimistic_sync_test.go b/beacon-chain/forkchoice/protoarray/optimistic_sync_test.go index b9c11586a..08829f146 100644 --- a/beacon-chain/forkchoice/protoarray/optimistic_sync_test.go +++ b/beacon-chain/forkchoice/protoarray/optimistic_sync_test.go @@ -10,116 +10,38 @@ import ( "github.com/prysmaticlabs/prysm/testing/require" ) -// We test the algorithm to check the optimistic status of a node. The -// status for this test is the following branching diagram -// -// -- E -- F -// / -// -- C -- D -// / -// 0 -- 1 -- A -- B -- J -- K -// \ / -// -- G -- H -- I -// -// Here nodes 0, 1, A, B, C, D are fully validated and nodes -// E, F, G, H, J, K are optimistic. -// Synced Tips are nodes B, C, D -// nodes 0 and 1 are outside the Fork Choice Store. +func slicesEqual(a, b [][32]byte) bool { + if len(a) != len(b) { + return false + } -func TestOptimistic(t *testing.T) { + mapA := make(map[[32]byte]bool, len(a)) + for _, root := range a { + mapA[root] = true + } + for _, root := range b { + _, ok := mapA[root] + if !ok { + return false + } + } + return true +} + +func TestOptimistic_Outside_ForkChoice(t *testing.T) { root0 := bytesutil.ToBytes32([]byte("hello0")) - root1 := bytesutil.ToBytes32([]byte("hello1")) nodeA := &Node{ slot: types.Slot(100), root: bytesutil.ToBytes32([]byte("helloA")), bestChild: 1, - } - nodeB := &Node{ - slot: types.Slot(101), - root: bytesutil.ToBytes32([]byte("helloB")), - bestChild: 2, - parent: 0, - } - nodeC := &Node{ - slot: types.Slot(102), - root: bytesutil.ToBytes32([]byte("helloC")), - bestChild: 3, - parent: 1, - } - nodeD := &Node{ - slot: types.Slot(103), - root: bytesutil.ToBytes32([]byte("helloD")), - bestChild: NonExistentNode, - parent: 2, - } - nodeE := &Node{ - slot: types.Slot(103), - root: bytesutil.ToBytes32([]byte("helloE")), - bestChild: 5, - parent: 2, - } - nodeF := &Node{ - slot: types.Slot(104), - root: bytesutil.ToBytes32([]byte("helloF")), - bestChild: NonExistentNode, - parent: 4, - } - nodeG := &Node{ - slot: types.Slot(102), - root: bytesutil.ToBytes32([]byte("helloG")), - bestChild: 7, - parent: 1, - } - nodeH := &Node{ - slot: types.Slot(103), - root: bytesutil.ToBytes32([]byte("helloH")), - bestChild: 8, - parent: 6, - } - nodeI := &Node{ - slot: types.Slot(104), - root: bytesutil.ToBytes32([]byte("helloI")), - bestChild: NonExistentNode, - parent: 7, - } - nodeJ := &Node{ - slot: types.Slot(103), - root: bytesutil.ToBytes32([]byte("helloJ")), - bestChild: 10, - parent: 6, - } - nodeK := &Node{ - slot: types.Slot(104), - root: bytesutil.ToBytes32([]byte("helloK")), - bestChild: NonExistentNode, - parent: 9, + status: valid, } nodes := []*Node{ nodeA, - nodeB, - nodeC, - nodeD, - nodeE, - nodeF, - nodeG, - nodeH, - nodeI, - nodeJ, - nodeK, } ni := map[[32]byte]uint64{ nodeA.root: 0, - nodeB.root: 1, - nodeC.root: 2, - nodeD.root: 3, - nodeE.root: 4, - nodeF.root: 5, - nodeG.root: 6, - nodeH.root: 7, - nodeI.root: 8, - nodeJ.root: 9, - nodeK.root: 10, } s := &Store{ @@ -127,82 +49,14 @@ func TestOptimistic(t *testing.T) { nodesIndices: ni, } - tips := map[[32]byte]types.Slot{ - nodeB.root: nodeB.slot, - nodeC.root: nodeC.slot, - nodeD.root: nodeD.slot, - } - st := &optimisticStore{ - validatedTips: tips, - } f := &ForkChoice{ - store: s, - syncedTips: st, + store: s, } - ctx := context.Background() - // We test the implementation of boundarySyncedTips - min, max := f.boundarySyncedTips() - require.Equal(t, min, types.Slot(101), "minimum tip slot is different") - require.Equal(t, max, types.Slot(103), "maximum tip slot is different") - - // We test first nodes outside the Fork Choice store - _, err := f.IsOptimistic(ctx, root0) + _, err := f.IsOptimistic(root0) require.ErrorIs(t, ErrUnknownNodeRoot, err) - - _, err = f.IsOptimistic(ctx, root1) - require.ErrorIs(t, ErrUnknownNodeRoot, err) - - // We check all nodes in the Fork Choice store. - op, err := f.IsOptimistic(ctx, nodeA.root) - require.NoError(t, err) - require.Equal(t, op, false) - - op, err = f.IsOptimistic(ctx, nodeB.root) - require.NoError(t, err) - require.Equal(t, op, false) - - op, err = f.IsOptimistic(ctx, nodeC.root) - require.NoError(t, err) - require.Equal(t, op, false) - - op, err = f.IsOptimistic(ctx, nodeD.root) - require.NoError(t, err) - require.Equal(t, op, false) - - op, err = f.IsOptimistic(ctx, nodeE.root) - require.NoError(t, err) - require.Equal(t, op, true) - - op, err = f.IsOptimistic(ctx, nodeF.root) - require.NoError(t, err) - require.Equal(t, op, true) - - op, err = f.IsOptimistic(ctx, nodeG.root) - require.NoError(t, err) - require.Equal(t, op, true) - - op, err = f.IsOptimistic(ctx, nodeH.root) - require.NoError(t, err) - require.Equal(t, op, true) - - op, err = f.IsOptimistic(ctx, nodeI.root) - require.NoError(t, err) - require.Equal(t, op, true) - - op, err = f.IsOptimistic(ctx, nodeJ.root) - require.NoError(t, err) - require.Equal(t, op, true) - - op, err = f.IsOptimistic(ctx, nodeK.root) - require.NoError(t, err) - require.Equal(t, op, true) - - // request a write Lock to synced Tips regression #10289 - f.syncedTips.Lock() - defer f.syncedTips.Unlock() } -// This tests the algorithm to update syncedTips +// This tests the algorithm to update optimistic Status // We start with the following diagram // // E -- F @@ -213,165 +67,105 @@ func TestOptimistic(t *testing.T) { // \ \ // J -- K -- L // -// And every block in the Fork choice is optimistic. Synced_Tips contains a -// single block that is outside of Fork choice +// The Chain A -- B -- C -- D -- E is VALID. // func TestSetOptimisticToValid(t *testing.T) { ctx := context.Background() - f := setup(1, 1) - - require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, params.BeaconConfig().ZeroHash, 1, 1)) 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 - wantedErr error + root [32]byte // the root of the new VALID block + testRoot [32]byte // root of the node we will test optimistic status + wantedOptimistic bool // wanted optimistic status for tested node + wantedErr error // wanted error message }{ { [32]byte{'i'}, - map[[32]byte]types.Slot{[32]byte{'z'}: 90}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, + [32]byte{'i'}, + false, nil, }, { [32]byte{'i'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - }, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, + [32]byte{'f'}, + true, nil, }, { [32]byte{'i'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'e'}: 103, - }, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'e'}: 104, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, - nil, - }, - { - [32]byte{'j'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'f'}: 105, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, - map[[32]byte]types.Slot{ - [32]byte{'f'}: 105, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - [32]byte{'j'}: 102, - }, - nil, - }, - { - [32]byte{'g'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'f'}: 105, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'f'}: 105, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, + [32]byte{'b'}, + false, nil, }, { + [32]byte{'i'}, [32]byte{'h'}, - map[[32]byte]types.Slot{ - [32]byte{'z'}: 90, - }, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - [32]byte{'h'}: 105, - }, + false, nil, }, { + [32]byte{'b'}, + [32]byte{'b'}, + false, + nil, + }, + { + [32]byte{'b'}, [32]byte{'h'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, + true, nil, }, { - [32]byte{'g'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'e'}: 104, - }, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'e'}: 104, - [32]byte{'g'}: 104, - }, + [32]byte{'b'}, + [32]byte{'a'}, + false, + nil, + }, + { + [32]byte{'k'}, + [32]byte{'k'}, + false, + nil, + }, + { + [32]byte{'k'}, + [32]byte{'l'}, + true, nil, }, { [32]byte{'p'}, - map[[32]byte]types.Slot{}, - map[[32]byte]types.Slot{}, - errInvalidNodeIndex, + [32]byte{}, + false, + ErrUnknownNodeRoot, }, } for _, tc := range tests { - f.syncedTips.Lock() - f.syncedTips.validatedTips = tc.tips - f.syncedTips.Unlock() - err := f.SetOptimisticToValid(context.Background(), tc.root) + f := setup(1, 1) + + require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, [32]byte{'J'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, [32]byte{'D'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, [32]byte{'E'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, [32]byte{'G'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, [32]byte{'F'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, [32]byte{'H'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, [32]byte{'K'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, [32]byte{'I'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'L'}, 1, 1)) + require.NoError(t, f.SetOptimisticToValid(context.Background(), [32]byte{'e'})) + optimistic, err := f.IsOptimistic([32]byte{'b'}) + require.NoError(t, err) + require.Equal(t, false, optimistic) + + err = f.SetOptimisticToValid(context.Background(), tc.root) if tc.wantedErr != nil { require.ErrorIs(t, err, tc.wantedErr) } else { require.NoError(t, err) - f.syncedTips.RLock() - require.DeepEqual(t, f.syncedTips.validatedTips, tc.newTips) - f.syncedTips.RUnlock() + optimistic, err := f.IsOptimistic(tc.testRoot) + require.NoError(t, err) + require.Equal(t, tc.wantedOptimistic, optimistic) } } } @@ -387,70 +181,81 @@ func TestSetOptimisticToValid(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 parentheses are -// the weights of the nodes before removal +// And the chain A -- B -- C -- D -- E has been fully validated. The numbers in parentheses are +// the weights of the nodes. // func TestSetOptimisticToInvalid(t *testing.T) { tests := []struct { - root [32]byte // the root of the new INVALID block - tips map[[32]byte]types.Slot // the old synced tips - wantedParentTip bool + name string // test description + root [32]byte // the root of the new INVALID block + payload [32]byte // the payload of the last valid hash newBestChild uint64 newBestDescendant uint64 newParentWeight uint64 returnedRoots [][32]byte }{ { + "Remove tip, parent was valid", [32]byte{'j'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - }, - false, + [32]byte{'B'}, 3, - 4, - 8, - [][32]byte{[32]byte{'j'}}, - }, - { - [32]byte{'j'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - }, - true, - 3, - 4, + 12, 8, [][32]byte{[32]byte{'j'}}, }, { + "Remove tip, parent was optimistic", [32]byte{'i'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - [32]byte{'h'}: 105, - }, - true, + [32]byte{'H'}, NonExistentNode, NonExistentNode, 1, [][32]byte{[32]byte{'i'}}, }, { + "Remove tip, lvh is inner and valid", [32]byte{'i'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - }, - false, + [32]byte{'D'}, + 6, + 8, + 3, + [][32]byte{[32]byte{'g'}, [32]byte{'h'}, [32]byte{'k'}, [32]byte{'i'}, [32]byte{'l'}}, + }, + { + "Remove inner, lvh is inner and optimistic", + [32]byte{'h'}, + [32]byte{'G'}, + 10, + 12, + 2, + [][32]byte{[32]byte{'h'}, [32]byte{'i'}}, + }, + { + "Remove tip, lvh is inner and optimistic", + [32]byte{'l'}, + [32]byte{'G'}, + 9, + 11, + 2, + [][32]byte{[32]byte{'k'}, [32]byte{'l'}}, + }, + { + "Remove tip, lvh is not an ancestor", + [32]byte{'j'}, + [32]byte{'C'}, + 5, + 12, + 7, + [][32]byte{[32]byte{'j'}}, + }, + { + "Remove inner, lvh is not an ancestor", + [32]byte{'g'}, + [32]byte{'J'}, NonExistentNode, NonExistentNode, 1, - [][32]byte{[32]byte{'i'}}, + [][32]byte{[32]byte{'g'}, [32]byte{'h'}, [32]byte{'k'}, [32]byte{'i'}, [32]byte{'l'}}, }, } for _, tc := range tests { @@ -458,184 +263,71 @@ func TestSetOptimisticToInvalid(t *testing.T) { f := setup(1, 1) require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, params.BeaconConfig().ZeroHash, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, [32]byte{'J'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, [32]byte{'D'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, [32]byte{'E'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, [32]byte{'G'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, [32]byte{'F'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, [32]byte{'H'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, [32]byte{'K'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, [32]byte{'I'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, [32]byte{'L'}, 1, 1)) weights := []uint64{10, 10, 9, 7, 1, 6, 2, 3, 1, 1, 1, 0, 0} - f.syncedTips.Lock() - f.syncedTips.validatedTips = tc.tips - f.syncedTips.Unlock() f.store.nodesLock.Lock() for i, node := range f.store.nodes { node.weight = weights[i] } - // Make j be the best child and descendant of b - nodeB := f.store.nodes[2] - nodeB.bestChild = 4 - nodeB.bestDescendant = 4 - idx := f.store.nodesIndices[tc.root] - node := f.store.nodes[idx] - parentIndex := node.parent - require.NotEqual(t, NonExistentNode, parentIndex) - parent := f.store.nodes[parentIndex] f.store.nodesLock.Unlock() - roots, err := f.SetOptimisticToInvalid(context.Background(), tc.root) + require.NoError(t, f.SetOptimisticToValid(ctx, [32]byte{'e'})) + roots, err := f.SetOptimisticToInvalid(ctx, tc.root, tc.payload) require.NoError(t, err) - require.DeepEqual(t, tc.returnedRoots, roots) - f.syncedTips.RLock() - _, parentSyncedTip := f.syncedTips.validatedTips[parent.root] - f.syncedTips.RUnlock() - require.Equal(t, tc.wantedParentTip, parentSyncedTip) - require.Equal(t, tc.newBestChild, parent.bestChild) - require.Equal(t, tc.newBestDescendant, parent.bestDescendant) - require.Equal(t, tc.newParentWeight, parent.weight) - } -} - -// This tests the algorithm to find the tip of a given node -// We start with the following diagram -// -// E -- F -// / -// C -- D -// / \ -// A -- B G -- H -- I -// \ \ -// J -- K -- L -// -// -func TestFindSyncedTip(t *testing.T) { - ctx := context.Background() - f := setup(1, 1) - - require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'j'}, [32]byte{'b'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'e'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 104, [32]byte{'g'}, [32]byte{'d'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'h'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, params.BeaconConfig().ZeroHash, 1, 1)) - tests := []struct { - root [32]byte // the root of the block - tips map[[32]byte]types.Slot // the synced tips - wanted [32]byte // the root of expected tip - }{ - { - [32]byte{'i'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 104, - }, - [32]byte{'g'}, - }, - { - [32]byte{'g'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'h'}: 104, - [32]byte{'k'}: 106, - }, - [32]byte{'d'}, - }, - { - [32]byte{'e'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'g'}: 103, - }, - [32]byte{'d'}, - }, - { - [32]byte{'j'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'f'}: 105, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, - [32]byte{'b'}, - }, - { - [32]byte{'g'}, - map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'f'}: 105, - [32]byte{'g'}: 104, - [32]byte{'i'}: 106, - }, - [32]byte{'g'}, - }, - } - for _, tc := range tests { f.store.nodesLock.RLock() - node := f.store.nodes[f.store.nodesIndices[tc.root]] - syncedTips := &optimisticStore{ - validatedTips: tc.tips, - } - syncedTips.RLock() - idx, err := f.store.findSyncedTip(ctx, node, syncedTips) - require.NoError(t, err) - require.Equal(t, tc.wanted, f.store.nodes[idx].root) - + _, ok := f.store.nodesIndices[tc.root] + require.Equal(t, false, ok) + lvh := f.store.nodes[f.store.payloadIndices[tc.payload]] + require.Equal(t, true, slicesEqual(tc.returnedRoots, roots)) + require.Equal(t, tc.newBestChild, lvh.bestChild) + require.Equal(t, tc.newBestDescendant, lvh.bestDescendant) + require.Equal(t, tc.newParentWeight, lvh.weight) + require.Equal(t, syncing, f.store.nodes[8].status /* F */) + require.Equal(t, valid, f.store.nodes[5].status /* E */) f.store.nodesLock.RUnlock() - syncedTips.RUnlock() } } -// This is a regression test (10341) -func TestIsOptimistic_DeadLock(t *testing.T) { +func TestSetOptimisticToInvalid_InvalidRoots(t *testing.T) { ctx := context.Background() f := setup(1, 1) + require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 90, [32]byte{'b'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'c'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 102, [32]byte{'d'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - require.NoError(t, f.InsertOptimisticBlock(ctx, 103, [32]byte{'e'}, params.BeaconConfig().ZeroHash, params.BeaconConfig().ZeroHash, 1, 1)) - tips := map[[32]byte]types.Slot{ - [32]byte{'a'}: 100, - [32]byte{'d'}: 102, - } - f.syncedTips.validatedTips = tips - _, err := f.IsOptimistic(ctx, [32]byte{'a'}) - require.NoError(t, err) - - // Acquire a write lock, this should not hang - f.store.nodesLock.Lock() - f.store.nodesLock.Unlock() - _, err = f.IsOptimistic(ctx, [32]byte{'e'}) - require.NoError(t, err) - - // Acquire a write lock, this should not hang - f.store.nodesLock.Lock() - f.store.nodesLock.Unlock() - _, err = f.IsOptimistic(ctx, [32]byte{'b'}) - require.NoError(t, err) - - // Acquire a write lock, this should not hang - f.store.nodesLock.Lock() - f.store.nodesLock.Unlock() - - _, err = f.IsOptimistic(ctx, [32]byte{'c'}) - require.NoError(t, err) - - // Acquire a write lock, this should not hang - f.store.nodesLock.Lock() - f.store.nodesLock.Unlock() - + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1)) + _, err := f.SetOptimisticToInvalid(ctx, [32]byte{'p'}, [32]byte{'B'}) + require.ErrorIs(t, ErrUnknownNodeRoot, err) + _, err = f.SetOptimisticToInvalid(ctx, [32]byte{'a'}, [32]byte{'p'}) + require.ErrorIs(t, errInvalidFinalizedNode, err) +} + +// This is a regression test (10445) +func TestSetOptimisticToInvalid_ProposerBoost(t *testing.T) { + ctx := context.Background() + f := setup(1, 1) + + require.NoError(t, f.InsertOptimisticBlock(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1)) + require.NoError(t, f.InsertOptimisticBlock(ctx, 101, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 1, 1)) + f.store.proposerBoostLock.Lock() + f.store.proposerBoostRoot = [32]byte{'c'} + f.store.previousProposerBoostScore = 10 + f.store.previousProposerBoostRoot = [32]byte{'b'} + f.store.proposerBoostLock.Unlock() + + _, err := f.SetOptimisticToInvalid(ctx, [32]byte{'c'}, [32]byte{'A'}) + require.NoError(t, err) + f.store.proposerBoostLock.RLock() + require.Equal(t, uint64(0), f.store.previousProposerBoostScore) + require.DeepEqual(t, [32]byte{}, f.store.proposerBoostRoot) + require.DeepEqual(t, params.BeaconConfig().ZeroHash, f.store.previousProposerBoostRoot) + f.store.proposerBoostLock.RUnlock() } diff --git a/beacon-chain/forkchoice/protoarray/store.go b/beacon-chain/forkchoice/protoarray/store.go index b4db29eb3..21ab1cf50 100644 --- a/beacon-chain/forkchoice/protoarray/store.go +++ b/beacon-chain/forkchoice/protoarray/store.go @@ -30,43 +30,14 @@ func New(justifiedEpoch, finalizedEpoch types.Epoch, finalizedRoot [32]byte) *Fo proposerBoostRoot: [32]byte{}, nodes: make([]*Node, 0), nodesIndices: make(map[[32]byte]uint64), + payloadIndices: make(map[[32]byte]uint64), canonicalNodes: make(map[[32]byte]bool), pruneThreshold: defaultPruneThreshold, } b := make([]uint64, 0) v := make([]Vote, 0) - st := &optimisticStore{ - validatedTips: make(map[[32]byte]types.Slot), - } - return &ForkChoice{store: s, balances: b, votes: v, syncedTips: st} -} - -// SetSyncedTips sets the synced and validated tips from the passed map -func (f *ForkChoice) SetSyncedTips(tips map[[32]byte]types.Slot) error { - if len(tips) == 0 { - return errInvalidSyncedTips - } - newTips := make(map[[32]byte]types.Slot, len(tips)) - for k, v := range tips { - newTips[k] = v - } - f.syncedTips.Lock() - defer f.syncedTips.Unlock() - f.syncedTips.validatedTips = newTips - return nil -} - -// SyncedTips returns the synced and validated tips from the fork choice store. -func (f *ForkChoice) SyncedTips() map[[32]byte]types.Slot { - f.syncedTips.RLock() - defer f.syncedTips.RUnlock() - - m := make(map[[32]byte]types.Slot) - for k, v := range f.syncedTips.validatedTips { - m[k] = v - } - return m + return &ForkChoice{store: s, balances: b, votes: v} } // Head returns the head root from fork choice store. @@ -159,7 +130,7 @@ func (f *ForkChoice) InsertOptimisticBlock( // Prune prunes the fork choice store with the new finalized root. The store is only pruned if the input // root is different than the current store finalized root, and the number of the store has met prune threshold. func (f *ForkChoice) Prune(ctx context.Context, finalizedRoot [32]byte) error { - return f.store.prune(ctx, finalizedRoot, f.syncedTips) + return f.store.prune(ctx, finalizedRoot) } // HasNode returns true if the node exists in fork choice store, @@ -375,6 +346,7 @@ func (s *Store) insert(ctx context.Context, } s.nodesIndices[root] = index + s.payloadIndices[payloadHash] = index s.nodes = append(s.nodes, n) // Update parent with the best child and descendant only if it's available. @@ -599,7 +571,7 @@ func (s *Store) updateBestChildAndDescendant(parentIndex, childIndex uint64) err // prune prunes the store with the new finalized root. The tree is only // pruned if the input finalized root are different than the one in stored and // the number of the nodes in store has met prune threshold. -func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *optimisticStore) error { +func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte) error { _, span := trace.StartSpan(ctx, "protoArrayForkChoice.prune") defer span.End() @@ -619,18 +591,9 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *o return nil } - // Traverse through the node list starting from the finalized node at index 0. - // Nodes that are not branching off from the finalized node will be removed. - syncedTips.Lock() - defer syncedTips.Unlock() - canonicalNodesMap := make(map[uint64]uint64, uint64(len(s.nodes))-finalizedIndex) canonicalNodes := make([]*Node, 1, uint64(len(s.nodes))-finalizedIndex) finalizedNode := s.nodes[finalizedIndex] - finalizedTipIndex, err := s.findSyncedTip(ctx, finalizedNode, syncedTips) - if err != nil { - return err - } finalizedNode.parent = NonExistentNode canonicalNodes[0] = finalizedNode canonicalNodesMap[finalizedIndex] = uint64(0) @@ -646,10 +609,6 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *o } else { // Remove node and synced tip that is not part of finalized branch. delete(s.nodesIndices, node.root) - _, ok := syncedTips.validatedTips[node.root] - if ok && idx != finalizedTipIndex { - delete(syncedTips.validatedTips, node.root) - } } } s.nodesIndices[finalizedRoot] = uint64(0) @@ -666,7 +625,6 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *o s.nodes = canonicalNodes prunedCount.Inc() - syncedTipsCount.Set(float64(len(syncedTips.validatedTips))) return nil } @@ -674,6 +632,10 @@ func (s *Store) prune(ctx context.Context, finalizedRoot [32]byte, syncedTips *o // 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) { + if node.status == invalid { + return false, nil + } + var bestDescendantViable bool bestDescendantIndex := node.bestDescendant @@ -705,20 +667,6 @@ func (s *Store) viableForHead(node *Node) bool { return justified && finalized } -// Returns the list of leaves in the Fork Choice store. -// These are all the nodes that have NonExistentNode as best child. -// This internal method assumes that the caller holds a lock in s.nodesLock. -func (s *Store) leaves() ([]uint64, error) { - var leaves []uint64 - for i := uint64(0); i < uint64(len(s.nodes)); i++ { - node := s.nodes[i] - if node.bestChild == NonExistentNode { - leaves = append(leaves, i) - } - } - return leaves, nil -} - // Tips returns all possible chain heads (leaves of fork choice tree). // Heads roots and heads slots are returned. func (f *ForkChoice) Tips() ([][32]byte, []types.Slot) { diff --git a/beacon-chain/forkchoice/protoarray/store_test.go b/beacon-chain/forkchoice/protoarray/store_test.go index 9e570b259..90502da09 100644 --- a/beacon-chain/forkchoice/protoarray/store_test.go +++ b/beacon-chain/forkchoice/protoarray/store_test.go @@ -100,7 +100,7 @@ func TestStore_Head_ContextCancelled(t *testing.T) { func TestStore_Insert_UnknownParent(t *testing.T) { // The new node does not have a parent. - s := &Store{nodesIndices: make(map[[32]byte]uint64)} + s := &Store{nodesIndices: make(map[[32]byte]uint64), payloadIndices: make(map[[32]byte]uint64)} require.NoError(t, s.insert(context.Background(), 100, [32]byte{'A'}, [32]byte{'B'}, params.BeaconConfig().ZeroHash, 1, 1)) assert.Equal(t, 1, len(s.nodes), "Did not insert block") assert.Equal(t, 1, len(s.nodesIndices), "Did not insert block") @@ -113,7 +113,7 @@ func TestStore_Insert_UnknownParent(t *testing.T) { func TestStore_Insert_KnownParent(t *testing.T) { // Similar to UnknownParent test, but this time the new node has a valid parent already in store. // The new node builds on top of the parent. - s := &Store{nodesIndices: make(map[[32]byte]uint64)} + s := &Store{nodesIndices: make(map[[32]byte]uint64), payloadIndices: make(map[[32]byte]uint64)} s.nodes = []*Node{{}} p := [32]byte{'B'} s.nodesIndices[p] = 0 @@ -336,11 +336,10 @@ func TestStore_Prune_LessThanThreshold(t *testing.T) { }) s := &Store{nodes: nodes, nodesIndices: indices, pruneThreshold: 100} - syncedTips := &optimisticStore{} // Finalized root is at index 99 so everything before 99 should be pruned, // but PruneThreshold is at 100 so nothing will be pruned. - require.NoError(t, s.prune(context.Background(), indexToHash(99), syncedTips)) + require.NoError(t, s.prune(context.Background(), indexToHash(99))) assert.Equal(t, 100, len(s.nodes), "Incorrect nodes count") assert.Equal(t, 100, len(s.nodesIndices), "Incorrect node indices count") } @@ -377,10 +376,9 @@ func TestStore_Prune_MoreThanThreshold(t *testing.T) { }) indices[indexToHash(uint64(numOfNodes-1))] = uint64(numOfNodes - 1) s := &Store{nodes: nodes, nodesIndices: indices} - syncedTips := &optimisticStore{} // Finalized root is at index 99 so everything before 99 should be pruned. - require.NoError(t, s.prune(context.Background(), indexToHash(99), syncedTips)) + require.NoError(t, s.prune(context.Background(), indexToHash(99))) assert.Equal(t, 1, len(s.nodes), "Incorrect nodes count") assert.Equal(t, 1, len(s.nodesIndices), "Incorrect node indices count") } @@ -416,15 +414,14 @@ func TestStore_Prune_MoreThanOnce(t *testing.T) { }) s := &Store{nodes: nodes, nodesIndices: indices} - syncedTips := &optimisticStore{} // Finalized root is at index 11 so everything before 11 should be pruned. - require.NoError(t, s.prune(context.Background(), indexToHash(10), syncedTips)) + require.NoError(t, s.prune(context.Background(), indexToHash(10))) assert.Equal(t, 90, len(s.nodes), "Incorrect nodes count") assert.Equal(t, 90, len(s.nodesIndices), "Incorrect node indices count") // One more time. - require.NoError(t, s.prune(context.Background(), indexToHash(20), syncedTips)) + require.NoError(t, s.prune(context.Background(), indexToHash(20))) assert.Equal(t, 80, len(s.nodes), "Incorrect nodes count") assert.Equal(t, 80, len(s.nodesIndices), "Incorrect node indices count") } @@ -460,7 +457,6 @@ func TestStore_Prune_NoDanglingBranch(t *testing.T) { bestDescendant: NonExistentNode, }, } - syncedTips := &optimisticStore{} s := &Store{ pruneThreshold: 0, nodes: nodes, @@ -470,7 +466,7 @@ func TestStore_Prune_NoDanglingBranch(t *testing.T) { indexToHash(uint64(2)): 2, }, } - require.NoError(t, s.prune(context.Background(), indexToHash(uint64(1)), syncedTips)) + require.NoError(t, s.prune(context.Background(), indexToHash(uint64(1)))) require.Equal(t, len(s.nodes), 1) } @@ -486,9 +482,6 @@ func TestStore_Prune_NoDanglingBranch(t *testing.T) { // J -- K -- L // // -// Synced tips are B, D and E. And we finalize F. All that is left in fork -// choice is F, and the only synced tip left is E which is now away from Fork -// Choice. func TestStore_PruneSyncedTips(t *testing.T) { ctx := context.Background() f := setup(1, 1) @@ -505,19 +498,9 @@ func TestStore_PruneSyncedTips(t *testing.T) { require.NoError(t, f.InsertOptimisticBlock(ctx, 105, [32]byte{'k'}, [32]byte{'g'}, params.BeaconConfig().ZeroHash, 1, 1)) require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'i'}, [32]byte{'h'}, params.BeaconConfig().ZeroHash, 1, 1)) require.NoError(t, f.InsertOptimisticBlock(ctx, 106, [32]byte{'l'}, [32]byte{'k'}, params.BeaconConfig().ZeroHash, 1, 1)) - syncedTips := &optimisticStore{ - validatedTips: map[[32]byte]types.Slot{ - [32]byte{'b'}: 101, - [32]byte{'d'}: 103, - [32]byte{'e'}: 104, - }, - } - f.syncedTips = syncedTips f.store.pruneThreshold = 0 require.NoError(t, f.Prune(ctx, [32]byte{'f'})) - require.Equal(t, 1, len(f.syncedTips.validatedTips)) - _, ok := f.syncedTips.validatedTips[[32]byte{'e'}] - require.Equal(t, true, ok) + require.Equal(t, 1, f.NodeCount()) } func TestStore_LeadsToViableHead(t *testing.T) { @@ -546,20 +529,6 @@ func TestStore_LeadsToViableHead(t *testing.T) { } } -func TestStore_SetSyncedTips(t *testing.T) { - f := setup(1, 1) - tips := make(map[[32]byte]types.Slot) - require.ErrorIs(t, errInvalidSyncedTips, f.SetSyncedTips(tips)) - tips[bytesutil.ToBytes32([]byte{'a'})] = 1 - require.NoError(t, f.SetSyncedTips(tips)) - f.syncedTips.RLock() - defer f.syncedTips.RUnlock() - require.Equal(t, 1, len(f.syncedTips.validatedTips)) - slot, ok := f.syncedTips.validatedTips[bytesutil.ToBytes32([]byte{'a'})] - require.Equal(t, true, ok) - require.Equal(t, types.Slot(1), slot) -} - func TestStore_ViableForHead(t *testing.T) { tests := []struct { n *Node diff --git a/beacon-chain/forkchoice/protoarray/types.go b/beacon-chain/forkchoice/protoarray/types.go index 5369c7dd6..d68247387 100644 --- a/beacon-chain/forkchoice/protoarray/types.go +++ b/beacon-chain/forkchoice/protoarray/types.go @@ -9,11 +9,10 @@ import ( // ForkChoice defines the overall fork choice store which includes all block nodes, validator's latest votes and balances. type ForkChoice struct { - store *Store - votes []Vote // tracks individual validator's last vote. - votesLock sync.RWMutex - balances []uint64 // tracks individual validator's last justified balances. - syncedTips *optimisticStore + store *Store + votes []Vote // tracks individual validator's last vote. + votesLock sync.RWMutex + balances []uint64 // tracks individual validator's last justified balances. } // Store defines the fork choice store which includes block nodes and the last view of checkpoint information. @@ -28,6 +27,7 @@ type Store struct { nodes []*Node // list of block nodes, each node is a representation of one block. nodesIndices map[[fieldparams.RootLength]byte]uint64 // the root of block node and the nodes index in the list. canonicalNodes map[[fieldparams.RootLength]byte]bool // the canonical block nodes. + payloadIndices map[[fieldparams.RootLength]byte]uint64 // the payload hash of block node and the index in the list nodesLock sync.RWMutex proposerBoostLock sync.RWMutex } @@ -44,15 +44,17 @@ type Node struct { weight uint64 // weight of this node. bestChild uint64 // bestChild index of this node. bestDescendant uint64 // bestDescendant of this node. - graffiti [fieldparams.RootLength]byte // graffiti of the block node. + status status // optimistic status of this node } -// optimisticStore defines a structure that tracks the tips of the fully -// validated blocks tree. -type optimisticStore struct { - validatedTips map[[32]byte]types.Slot - sync.RWMutex -} +// enum used as optimistic status of a node +type status uint8 + +const ( + syncing status = iota // the node is optimistic + valid //fully validated node + invalid // invalid execution payload +) // Vote defines an individual validator's vote. type Vote struct { diff --git a/beacon-chain/forkchoice/protoarray/vote_test.go b/beacon-chain/forkchoice/protoarray/vote_test.go index 16ed51cbd..f2589c36a 100644 --- a/beacon-chain/forkchoice/protoarray/vote_test.go +++ b/beacon-chain/forkchoice/protoarray/vote_test.go @@ -12,7 +12,6 @@ import ( func TestVotes_CanFindHead(t *testing.T) { balances := []uint64{1, 1} f := setup(1, 1) - syncedTips := &optimisticStore{} // The head should always start at the finalized block. r, err := f.Head(context.Background(), 1, params.BeaconConfig().ZeroHash, balances, 1) @@ -249,7 +248,7 @@ func TestVotes_CanFindHead(t *testing.T) { // Verify pruning below the prune threshold does not affect head. f.store.pruneThreshold = 1000 - require.NoError(t, f.store.prune(context.Background(), indexToHash(5), syncedTips)) + require.NoError(t, f.store.prune(context.Background(), indexToHash(5))) assert.Equal(t, 11, len(f.store.nodes), "Incorrect nodes length after prune") r, err = f.Head(context.Background(), 2, indexToHash(5), balances, 2) @@ -273,7 +272,7 @@ func TestVotes_CanFindHead(t *testing.T) { // / \ // 9 10 f.store.pruneThreshold = 1 - require.NoError(t, f.store.prune(context.Background(), indexToHash(5), syncedTips)) + require.NoError(t, f.store.prune(context.Background(), indexToHash(5))) assert.Equal(t, 5, len(f.store.nodes), "Incorrect nodes length after prune") r, err = f.Head(context.Background(), 2, indexToHash(5), balances, 2)