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

@@ -239,12 +239,45 @@ func verifyBlobCommitmentCount(body interfaces.ReadOnlyBeaconBlockBody) error {
 // 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_test.go

@@ -23,7 +23,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

@@ -74,22 +74,33 @@ func (s *Store) insert(ctx context.Context,
 	block := roblock.Block()
 	slot := block.Slot()
 	parentRoot := block.ParentRoot()
-	var payloadHash [32]byte
+	var parentHash, payloadHash [32]byte
 	if block.Version() >= version.Bellatrix {
 		execution, err := block.Body().Execution()
 		if err != nil {
 			return nil, err
 		}
 		copy(payloadHash[:], execution.BlockHash())
+		copy(parentHash[:], execution.ParentHash())
 	}
 
 	// 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,
@@ -113,19 +124,24 @@ 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 {
 			delete(s.nodeByRoot, root)
 			delete(s.nodeByPayload, payloadHash)
 			return nil, 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())

config/params/config.go

@@ -168,6 +168,7 @@ type BeaconChainConfig struct {
 	DenebForkVersion     []byte           `yaml:"DENEB_FORK_VERSION" spec:"true"`     // DenebForkVersion is used to represent the fork version for deneb.
 	DenebForkEpoch       primitives.Epoch `yaml:"DENEB_FORK_EPOCH" spec:"true"`       // DenebForkEpoch is used to represent the assigned fork epoch for deneb.
 	ElectraForkVersion   []byte           `yaml:"ELECTRA_FORK_VERSION" spec:"true"`   // ElectraForkVersion is used to represent the fork version for electra.
+	ElectraForkEpoch     primitives.Epoch `yaml:"ELECTRA_FORK_EPOCH" spec:"true"`     // ElectraForkEpoch is used to represent the assigned fork epoch for electra.
 	EPBSForkVersion      []byte           // EPBSForkVersion is used to represent the fork version for ePBS.
 	EPBSForkEpoch        primitives.Epoch // EPBSForkEpoch is used to represent the assigned fork epoch for ePBS.
 

consensus-types/blocks/BUILD.bazel

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

consensus-types/blocks/getters_epbs.go

@@ -0,0 +1,24 @@
+package blocks
+
+import (
+	consensus_types "github.com/prysmaticlabs/prysm/v5/consensus-types"
+	enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
+	ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
+	"github.com/prysmaticlabs/prysm/v5/runtime/version"
+)
+
+// PayloadAttestations returns the payload attestations in the block.
+func (b *BeaconBlockBody) PayloadAttestations() ([]*ethpb.PayloadAttestation, error) {
+	if b.version < version.EPBS {
+		return nil, consensus_types.ErrNotSupported("PayloadAttestations", b.version)
+	}
+	return b.payloadAttestations, nil
+}
+
+// SignedExecutionPayloadHeader returns the signed execution payload header in the block.
+func (b *BeaconBlockBody) SignedExecutionPayloadHeader() (*enginev1.SignedExecutionPayloadHeader, error) {
+	if b.version < version.EPBS {
+		return nil, consensus_types.ErrNotSupported("SignedExecutionPayloadHeader", b.version)
+	}
+	return b.signedExecutionPayloadHeader, nil
+}

consensus-types/blocks/getters_epbs_test.go

@@ -63,11 +63,14 @@ func Test_EpbsBlock_Copy(t *testing.T) {
 	require.NoError(t, err)
 	copiedEpbsBlock, err := epbsBlock.Copy()
 	require.NoError(t, err)
-	copiedBody, ok := copiedEpbsBlock.Body().(interfaces.ROBlockBodyEpbs)
+	copiedBody, ok := copiedEpbsBlock.Body().(interfaces.ReadOnlyBeaconBlockBody)
 	require.Equal(t, true, ok)
-	require.DeepEqual(t, copiedBody.SignedExecutionPayloadHeader(), signedHeader)
+	copiedHeader, err := copiedBody.SignedExecutionPayloadHeader()
+	require.NoError(t, err)
+	require.DeepEqual(t, copiedHeader, signedHeader)
 
-	copiedPayloadAtts := copiedBody.PayloadAttestations()
+	copiedPayloadAtts, err := copiedBody.PayloadAttestations()
+	require.NoError(t, err)
 	require.DeepEqual(t, copiedPayloadAtts, payloadAttestations)
 }
 
@@ -93,3 +96,11 @@ func Test_EpbsBlock_IsBlinded(t *testing.T) {
 	bd := &BeaconBlockBody{version: version.EPBS}
 	require.Equal(t, false, bd.IsBlinded())
 }
+
+func Test_PreEPBS_Versions(t *testing.T) {
+	bb := &BeaconBlockBody{version: version.Electra}
+	_, err := bb.PayloadAttestations()
+	require.ErrorContains(t, "PayloadAttestations", err)
+	_, err = bb.SignedExecutionPayloadHeader()
+	require.ErrorContains(t, "SignedExecutionPayloadHeader", err)
+}

consensus-types/blocks/setters_epbs.go

@@ -0,0 +1,26 @@
+package blocks
+
+import (
+	consensus_types "github.com/prysmaticlabs/prysm/v5/consensus-types"
+	enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
+	eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
+	"github.com/prysmaticlabs/prysm/v5/runtime/version"
+)
+
+// SetPayloadAttestations sets the payload attestations in the block.
+func (b *SignedBeaconBlock) SetPayloadAttestations(p []*eth.PayloadAttestation) error {
+	if b.version < version.EPBS {
+		return consensus_types.ErrNotSupported("PayloadAttestations", b.version)
+	}
+	b.block.body.payloadAttestations = p
+	return nil
+}
+
+// SetSignedExecutionPayloadHeader sets the signed execution payload header of the block body.
+func (b *SignedBeaconBlock) SetSignedExecutionPayloadHeader(h *enginev1.SignedExecutionPayloadHeader) error {
+	if b.version < version.EPBS {
+		return consensus_types.ErrNotSupported("SetSignedExecutionPayloadHeader", b.version)
+	}
+	b.block.body.signedExecutionPayloadHeader = h
+	return nil
+}

consensus-types/blocks/setters_test.go

@@ -43,7 +43,9 @@ func Test_EpbsBlock_SetPayloadAttestations(t *testing.T) {
 	}
 
 	require.NoError(t, b.SetPayloadAttestations(payloadAttestation))
-	require.DeepEqual(t, b.block.body.PayloadAttestations(), payloadAttestation)
+	expectedPA, err := b.block.body.PayloadAttestations()
+	require.NoError(t, err)
+	require.DeepEqual(t, expectedPA, payloadAttestation)
 }
 
 func Test_EpbsBlock_SetSignedExecutionPayloadHeader(t *testing.T) {
@@ -67,5 +69,7 @@ func Test_EpbsBlock_SetSignedExecutionPayloadHeader(t *testing.T) {
 		Signature: []byte("signature"),
 	}
 	require.NoError(t, b.SetSignedExecutionPayloadHeader(signedExecutionPayloadHeader))
-	require.DeepEqual(t, b.block.body.SignedExecutionPayloadHeader(), signedExecutionPayloadHeader)
+	expectedHeader, err := b.block.body.SignedExecutionPayloadHeader()
+	require.NoError(t, err)
+	require.DeepEqual(t, expectedHeader, signedExecutionPayloadHeader)
 }

consensus-types/interfaces/beacon_block.go

@@ -70,12 +70,8 @@ type ReadOnlyBeaconBlockBody interface {
 	BLSToExecutionChanges() ([]*ethpb.SignedBLSToExecutionChange, error)
 	BlobKzgCommitments() ([][]byte, error)
 	ExecutionRequests() (*enginev1.ExecutionRequests, error)
-}
-
-type ROBlockBodyEpbs interface {
-	ReadOnlyBeaconBlockBody
-	PayloadAttestations() []*ethpb.PayloadAttestation
-	SignedExecutionPayloadHeader() *enginev1.SignedExecutionPayloadHeader
+	PayloadAttestations() ([]*ethpb.PayloadAttestation, error)
+	SignedExecutionPayloadHeader() (*enginev1.SignedExecutionPayloadHeader, error)
 }
 
 type SignedBeaconBlock interface {