Allow nodes with and without payload in forkchoice (#14288)

* Allow nodes with and without payload in forkchoice

    This PR takes care of adding nodes to forkchoice that may or may not
    have a corresponding payload. The rationale is as follows

    - The node structure is kept almost the same as today.
    - A zero payload hash is considered as if the node was empty (except for
      the tree root)
    - When inserting a node we check what the right parent node would be
      depending on whether the parent had a payload or not.
    - For pre-epbs forks all nodes are full, no logic changes except a new
      steps to gather the parent hash that is needed for block insertion.

    This PR had to change some core consensus types and interfaces.
    - It removed the ROBlockEPBS interface and added the corresponding ePBS
      fields to the ReadOnlyBeaconBlockBody
    - It moved the setters and getters to epbs dedicated files.

    It also added a checker for `IsParentFull` on forkchoice that simply
    checks for the parent hash of the parent node.

* review

beacon-chain/core/blocks/payload.go

@@ -287,12 +287,45 @@ func ProcessPayloadHeader(st state.BeaconState, header interfaces.ExecutionData)
 // GetBlockPayloadHash returns the hash of the execution payload of the block
 func GetBlockPayloadHash(blk interfaces.ReadOnlyBeaconBlock) ([32]byte, error) {
 	var payloadHash [32]byte
-	if IsPreBellatrixVersion(blk.Version()) {
-		return payloadHash, nil
+	if blk.Version() >= version.EPBS {
+		header, err := blk.Body().SignedExecutionPayloadHeader()
+		if err != nil {
+			return payloadHash, err
+		}
+		if header.Message == nil {
+			return payloadHash, errors.New("nil execution header")
+		}
+		return [32]byte(header.Message.BlockHash), nil
 	}
-	payload, err := blk.Body().Execution()
-	if err != nil {
-		return payloadHash, err
+	if blk.Version() >= version.Bellatrix {
+		payload, err := blk.Body().Execution()
+		if err != nil {
+			return payloadHash, err
+		}
+		return bytesutil.ToBytes32(payload.BlockHash()), nil
+	}
+	return payloadHash, nil
+}
+
+// GetBlockParentHash returns the hash of the parent execution payload
+func GetBlockParentHash(blk interfaces.ReadOnlyBeaconBlock) ([32]byte, error) {
+	var parentHash [32]byte
+	if blk.Version() >= version.EPBS {
+		header, err := blk.Body().SignedExecutionPayloadHeader()
+		if err != nil {
+			return parentHash, err
+		}
+		if header.Message == nil {
+			return parentHash, errors.New("nil execution header")
+		}
+		return [32]byte(header.Message.ParentBlockHash), nil
+	}
+	if blk.Version() >= version.Bellatrix {
+		payload, err := blk.Body().Execution()
+		if err != nil {
+			return parentHash, err
+		}
+		return bytesutil.ToBytes32(payload.ParentHash()), nil
 	}
-	return bytesutil.ToBytes32(payload.BlockHash()), nil
+	return parentHash, nil
 }

beacon-chain/forkchoice/doubly-linked-tree/BUILD.bazel

@@ -4,6 +4,7 @@ go_library(
     name = "go_default_library",
     srcs = [
         "doc.go",
+        "epbs.go",
         "errors.go",
         "forkchoice.go",
         "last_root.go",
@@ -47,6 +48,7 @@ go_library(
 go_test(
     name = "go_default_test",
     srcs = [
+        "epbs_test.go",
         "ffg_update_test.go",
         "forkchoice_test.go",
         "last_root_test.go",

beacon-chain/forkchoice/doubly-linked-tree/epbs.go

@@ -0,0 +1,9 @@
+package doublylinkedtree
+
+func (n *Node) isParentFull() bool {
+	// Finalized checkpoint is considered full
+	if n.parent == nil || n.parent.parent == nil {
+		return true
+	}
+	return n.parent.payloadHash != [32]byte{}
+}

beacon-chain/forkchoice/doubly-linked-tree/epbs_test.go

@@ -0,0 +1,79 @@
+package doublylinkedtree
+
+import (
+	"context"
+	"testing"
+
+	"github.com/prysmaticlabs/prysm/v5/testing/require"
+)
+
+func TestStore_Insert_PayloadContent(t *testing.T) {
+	ctx := context.Background()
+	f := setup(0, 0)
+	s := f.store
+	// The tree root is full
+	fr := [32]byte{}
+	n := s.nodeByRoot[fr]
+	require.Equal(t, true, n.isParentFull())
+
+	// Insert a child with a payload
+	cr := [32]byte{'a'}
+	cp := [32]byte{'p'}
+	n, err := s.insert(ctx, 1, cr, fr, cp, fr, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, true, n.isParentFull())
+	require.Equal(t, s.treeRootNode, n.parent)
+	require.Equal(t, s.nodeByRoot[cr], n)
+
+	// Insert a grandchild without a payload
+	gr := [32]byte{'b'}
+	gn, err := s.insert(ctx, 2, gr, cr, fr, cp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, true, gn.isParentFull())
+	require.Equal(t, n, gn.parent)
+
+	// Insert the payload of the same grandchild
+	gp := [32]byte{'q'}
+	gfn, err := s.insert(ctx, 2, gr, cr, gp, cp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, true, gfn.isParentFull())
+	require.Equal(t, n, gfn.parent)
+
+	// Insert an empty great grandchild based on empty
+	ggr := [32]byte{'c'}
+	ggn, err := s.insert(ctx, 3, ggr, gr, fr, cp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, false, ggn.isParentFull())
+	require.Equal(t, gn, ggn.parent)
+
+	// Insert an empty great grandchild based on full
+	ggfr := [32]byte{'d'}
+	ggfn, err := s.insert(ctx, 3, ggfr, gr, fr, gp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, gfn, ggfn.parent)
+	require.Equal(t, true, ggfn.isParentFull())
+
+	// Insert the payload for the great grandchild based on empty
+	ggp := [32]byte{'r'}
+	n, err = s.insert(ctx, 3, ggr, gr, ggp, cp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, false, n.isParentFull())
+	require.Equal(t, gn, n.parent)
+
+	// Insert the payload for the great grandchild based on full
+	ggfp := [32]byte{'s'}
+	n, err = s.insert(ctx, 3, ggfr, gr, ggfp, gp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, true, n.isParentFull())
+	require.Equal(t, gfn, n.parent)
+
+	// Reinsert an empty node
+	ggfn2, err := s.insert(ctx, 3, ggfr, gr, fr, gp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, ggfn, ggfn2)
+
+	// Reinsert a full node
+	n2, err := s.insert(ctx, 3, ggfr, gr, ggfp, gp, 0, 0)
+	require.NoError(t, err)
+	require.Equal(t, n, n2)
+}

beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go

@@ -115,8 +115,27 @@ func (f *ForkChoice) InsertNode(ctx context.Context, state state.BeaconState, ro
 		return errNilBlockHeader
 	}
 	parentRoot := bytesutil.ToBytes32(bh.ParentRoot)
-	var payloadHash [32]byte
-	if state.Version() >= version.Bellatrix {
+	var payloadHash, parentHash [32]byte
+	if state.Version() >= version.EPBS {
+		slot, err := state.LatestFullSlot()
+		if err != nil {
+			return err
+		}
+		if slot == state.Slot() {
+			latestHeader, err := state.LatestExecutionPayloadHeaderEPBS()
+			if err != nil {
+				return err
+			}
+			copy(payloadHash[:], latestHeader.BlockHash)
+			copy(parentHash[:], latestHeader.ParentBlockHash)
+		} else {
+			latestHash, err := state.LatestBlockHash()
+			if err != nil {
+				return err
+			}
+			copy(parentHash[:], latestHash)
+		}
+	} else if state.Version() >= version.Bellatrix {
 		ph, err := state.LatestExecutionPayloadHeader()
 		if err != nil {
 			return err
@@ -135,7 +154,7 @@ func (f *ForkChoice) InsertNode(ctx context.Context, state state.BeaconState, ro
 		return errInvalidNilCheckpoint
 	}
 	finalizedEpoch := fc.Epoch
-	node, err := f.store.insert(ctx, slot, root, parentRoot, payloadHash, justifiedEpoch, finalizedEpoch)
+	node, err := f.store.insert(ctx, slot, root, parentRoot, payloadHash, parentHash, justifiedEpoch, finalizedEpoch)
 	if err != nil {
 		return err
 	}
@@ -490,8 +509,12 @@ func (f *ForkChoice) InsertChain(ctx context.Context, chain []*forkchoicetypes.B
 		if err != nil {
 			return err
 		}
+		parentHash, err := blocks.GetBlockParentHash(b)
+		if err != nil {
+			return err
+		}
 		if _, err := f.store.insert(ctx,
-			b.Slot(), r, parentRoot, payloadHash,
+			b.Slot(), r, parentRoot, payloadHash, parentHash,
 			chain[i].JustifiedCheckpoint.Epoch, chain[i].FinalizedCheckpoint.Epoch); err != nil {
 			return err
 		}

beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go

@@ -22,7 +22,8 @@ import (
 )
 
 // prepareForkchoiceState prepares a beacon State with the given data to mock
-// insert into forkchoice
+// insert into forkchoice. This method prepares full states and blocks for
+// bellatrix, it cannot be used for ePBS tests.
 func prepareForkchoiceState(
 	_ context.Context,
 	slot primitives.Slot,

beacon-chain/forkchoice/doubly-linked-tree/store.go

@@ -62,20 +62,32 @@ func (s *Store) head(ctx context.Context) ([32]byte, error) {
 
 // insert registers a new block node to the fork choice store's node list.
 // It then updates the new node's parent with the best child and descendant node.
-func (s *Store) insert(ctx context.Context,
+func (s *Store) insert(
+	ctx context.Context,
 	slot primitives.Slot,
-	root, parentRoot, payloadHash [fieldparams.RootLength]byte,
-	justifiedEpoch, finalizedEpoch primitives.Epoch) (*Node, error) {
+	root, parentRoot, payloadHash, parentHash [fieldparams.RootLength]byte,
+	justifiedEpoch, finalizedEpoch primitives.Epoch,
+) (*Node, error) {
 	ctx, span := trace.StartSpan(ctx, "doublyLinkedForkchoice.insert")
 	defer span.End()
 
 	// Return if the block has been inserted into Store before.
-	if n, ok := s.nodeByRoot[root]; ok {
-		return n, nil
+	n, rootPresent := s.nodeByRoot[root]
+	m, hashPresent := s.nodeByPayload[payloadHash]
+	if rootPresent {
+		if payloadHash == [32]byte{} {
+			return n, nil
+		}
+		if hashPresent {
+			return m, nil
+		}
 	}
-
 	parent := s.nodeByRoot[parentRoot]
-	n := &Node{
+	fullParent := s.nodeByPayload[parentHash]
+	if fullParent != nil && parent != nil && fullParent.root == parent.root {
+		parent = fullParent
+	}
+	n = &Node{
 		slot:                     slot,
 		root:                     root,
 		parent:                   parent,
@@ -99,17 +111,22 @@ func (s *Store) insert(ctx context.Context,
 		}
 	}
 
-	s.nodeByPayload[payloadHash] = n
-	s.nodeByRoot[root] = n
 	if parent == nil {
 		if s.treeRootNode == nil {
 			s.treeRootNode = n
 			s.headNode = n
 			s.highestReceivedNode = n
-		} else {
+		} else if s.treeRootNode.root != n.root {
 			return n, errInvalidParentRoot
 		}
-	} else {
+	}
+	if !rootPresent {
+		s.nodeByRoot[root] = n
+	}
+	if !hashPresent {
+		s.nodeByPayload[payloadHash] = n
+	}
+	if parent != nil {
 		parent.children = append(parent.children, n)
 		// Apply proposer boost
 		timeNow := uint64(time.Now().Unix())

beacon-chain/forkchoice/doubly-linked-tree/store_test.go

@@ -133,7 +133,7 @@ func TestStore_Insert(t *testing.T) {
 	fc := &forkchoicetypes.Checkpoint{Epoch: 0}
 	s := &Store{nodeByRoot: nodeByRoot, treeRootNode: treeRootNode, nodeByPayload: nodeByPayload, justifiedCheckpoint: jc, finalizedCheckpoint: fc, highestReceivedNode: &Node{}}
 	payloadHash := [32]byte{'a'}
-	_, err := s.insert(context.Background(), 100, indexToHash(100), indexToHash(0), payloadHash, 1, 1)
+	_, err := s.insert(context.Background(), 100, indexToHash(100), indexToHash(0), payloadHash, payloadHash, 1, 1)
 	require.NoError(t, err)
 	assert.Equal(t, 2, len(s.nodeByRoot), "Did not insert block")
 	assert.Equal(t, (*Node)(nil), treeRootNode.parent, "Incorrect parent")
@@ -327,7 +327,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 
 	// Make sure it doesn't underflow
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-1*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
-	_, err := s.insert(context.Background(), 1, [32]byte{'a'}, b, b, 1, 1)
+	_, err := s.insert(context.Background(), 1, [32]byte{'a'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	count, err := f.ReceivedBlocksLastEpoch()
 	require.NoError(t, err)
@@ -337,7 +337,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 
 	// 64
 	// Received block last epoch is 1
-	_, err = s.insert(context.Background(), 64, [32]byte{'A'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 64, [32]byte{'A'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration((-64*int64(params.BeaconConfig().SecondsPerSlot))-1) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -348,7 +348,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 
 	// 64 65
 	// Received block last epoch is 2
-	_, err = s.insert(context.Background(), 65, [32]byte{'B'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 65, [32]byte{'B'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-66*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -359,7 +359,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 
 	// 64 65 66
 	// Received block last epoch is 3
-	_, err = s.insert(context.Background(), 66, [32]byte{'C'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 66, [32]byte{'C'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-66*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -370,7 +370,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 	// 64 65 66
 	//       98
 	// Received block last epoch is 1
-	_, err = s.insert(context.Background(), 98, [32]byte{'D'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 98, [32]byte{'D'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-98*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -382,7 +382,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 	//       98
 	//              132
 	// Received block last epoch is 1
-	_, err = s.insert(context.Background(), 132, [32]byte{'E'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 132, [32]byte{'E'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-132*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -395,7 +395,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 	//              132
 	//       99
 	// Received block last epoch is still 1. 99 is outside the window
-	_, err = s.insert(context.Background(), 99, [32]byte{'F'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 99, [32]byte{'F'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-132*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -408,7 +408,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 	//              132
 	//       99 100
 	// Received block last epoch is still 1. 100 is at the same position as 132
-	_, err = s.insert(context.Background(), 100, [32]byte{'G'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 100, [32]byte{'G'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-132*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()
@@ -421,7 +421,7 @@ func TestForkChoice_ReceivedBlocksLastEpoch(t *testing.T) {
 	//              132
 	//       99 100 101
 	// Received block last epoch is 2. 101 is within the window
-	_, err = s.insert(context.Background(), 101, [32]byte{'H'}, b, b, 1, 1)
+	_, err = s.insert(context.Background(), 101, [32]byte{'H'}, b, b, b, 1, 1)
 	require.NoError(t, err)
 	s.genesisTime = uint64(time.Now().Add(time.Duration(-132*int64(params.BeaconConfig().SecondsPerSlot)) * time.Second).Unix())
 	count, err = f.ReceivedBlocksLastEpoch()

consensus-types/blocks/BUILD.bazel

@@ -7,11 +7,13 @@ go_library(
         "factory.go",
         "get_payload.go",
         "getters.go",
+        "getters_epbs.go",
         "kzg.go",
         "proto.go",
         "roblob.go",
         "roblock.go",
         "setters.go",
+        "setters_epbs.go",
         "types.go",
     ],
     importpath = "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks",