chore(pectra): Add Pectra Fork Logging (3/n)

beacon/blockchain/errors.go

@@ -31,6 +31,9 @@ var (
 	ErrNilBlk = errors.New("nil beacon block")
 	// ErrNilBlob is an error for when the BlobSidecars is nil.
 	ErrNilBlob = errors.New("nil blob")
+	// ErrVersionMismatch is an error for when the fork for the block timestamp does not match the fork
+	// for the ABCI timestamp.
+	ErrVersionMismatch = errors.New("ABCI fork version mismatch")
 	// ErrDataNotAvailable indicates that the required data is not available.
 	ErrDataNotAvailable = errors.New("data not available")
 	// ErrSidecarCommitmentMismatch indicates that the BeaconBlockBody commitments do not match the sidecars.

beacon/blockchain/execution_engine.go

@@ -25,7 +25,6 @@ import (
 	"fmt"
 
 	ctypes "github.com/berachain/beacon-kit/consensus-types/types"
-	contypes "github.com/berachain/beacon-kit/consensus/types"
 	engineprimitives "github.com/berachain/beacon-kit/engine-primitives/engine-primitives"
 	statedb "github.com/berachain/beacon-kit/state-transition/core/state"
 )
@@ -38,7 +37,6 @@ import (
 func (s *Service) sendPostBlockFCU(
 	ctx context.Context,
 	st *statedb.StateDB,
-	blk *contypes.ConsensusBlock,
 ) error {
 	lph, err := st.GetLatestExecutionPayloadHeader()
 	if err != nil {
@@ -47,15 +45,14 @@ func (s *Service) sendPostBlockFCU(
 
 	// Send a forkchoice update without payload attributes to notify
 	// EL of the new head.
-	beaconBlk := blk.GetBeaconBlock()
 	// TODO: Switch to New().
 	req := ctypes.BuildForkchoiceUpdateRequestNoAttrs(
 		&engineprimitives.ForkchoiceStateV1{
 			HeadBlockHash:      lph.GetBlockHash(),
 			SafeBlockHash:      lph.GetParentHash(),
 			FinalizedBlockHash: lph.GetParentHash(),
 		},
-		s.chainSpec.ActiveForkVersionForSlot(beaconBlk.GetSlot()),
+		lph.GetForkVersion(),
 	)
 	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req); err != nil {
 		return fmt.Errorf("failed forkchoice update, head %s: %w",

beacon/blockchain/finalize_block.go

@@ -43,7 +43,9 @@ func (s *Service) FinalizeBlock(
 		req,
 		BeaconBlockTxIndex,
 		BlobSidecarsTxIndex,
-		s.chainSpec.ActiveForkVersionForSlot(math.Slot(req.Height))) // #nosec G115
+		// While req.GetTime() and blk.GetTimestamp() may be different, they are guaranteed
+		// to map to the same forkVersion due to checks during ProcessProposal.
+		s.chainSpec.ActiveForkVersionForTimestamp(math.U64(req.GetTime().Unix()))) //#nosec: G115
 	if err != nil {
 		s.logger.Error("Failed to decode block and blobs", "error", err)
 		return nil, fmt.Errorf("failed to decode block and blobs: %w", err)
@@ -121,7 +123,7 @@ func (s *Service) FinalizeBlock(
 		s.logger.Error("failed to processPruning", "error", err)
 	}
 
-	if err = s.sendPostBlockFCU(ctx, st, consensusBlk); err != nil {
+	if err = s.sendPostBlockFCU(ctx, st); err != nil {
 		return nil, fmt.Errorf("sendPostBlockFCU failed: %w", err)
 	}
 

beacon/blockchain/interfaces.go

@@ -67,11 +67,11 @@ type LocalBuilder interface {
 		ctx context.Context,
 		st *statedb.StateDB,
 		slot math.Slot,
-		timestamp uint64,
+		timestamp math.U64,
 		parentBlockRoot common.Root,
 		headEth1BlockHash common.ExecutionHash,
 		finalEth1BlockHash common.ExecutionHash,
-	) (*engineprimitives.PayloadID, error)
+	) (*engineprimitives.PayloadID, common.Version, error)
 }
 
 // StateProcessor defines the interface for processing various state transitions
@@ -167,5 +167,5 @@ type PruningChainSpec interface {
 type ServiceChainSpec interface {
 	PruningChainSpec
 	chain.BlobSpec
-	ActiveForkVersionForSlot(slot math.Slot) common.Version
+	ActiveForkVersionForTimestamp(timestamp math.U64) common.Version
 }

beacon/blockchain/payload.go

@@ -37,19 +37,6 @@ func (s *Service) forceSyncUponProcess(
 	ctx context.Context,
 	st *statedb.StateDB,
 ) {
-	slot, err := st.GetSlot()
-	if err != nil {
-		s.logger.Error(
-			"failed to get slot for force startup head",
-			"error", err,
-		)
-		return
-	}
-
-	// TODO: Verify if the slot number is correct here, I believe in current
-	// form it should be +1'd. Not a big deal until hardforks are in play though.
-	slot++
-
 	lph, err := st.GetLatestExecutionPayloadHeader()
 	if err != nil {
 		s.logger.Error(
@@ -64,7 +51,7 @@ func (s *Service) forceSyncUponProcess(
 		"head_eth1_hash", lph.GetBlockHash(),
 		"safe_eth1_hash", lph.GetParentHash(),
 		"finalized_eth1_hash", lph.GetParentHash(),
-		"for_slot", slot.Base10(),
+		"for_slot", lph.GetNumber(),
 	)
 
 	// Submit the forkchoice update to the execution client.
@@ -74,7 +61,7 @@ func (s *Service) forceSyncUponProcess(
 			SafeBlockHash:      lph.GetParentHash(),
 			FinalizedBlockHash: lph.GetParentHash(),
 		},
-		s.chainSpec.ActiveForkVersionForSlot(slot),
+		s.chainSpec.ActiveForkVersionForTimestamp(lph.GetTimestamp()),
 	)
 	if _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req); err != nil {
 		s.logger.Error(
@@ -117,7 +104,7 @@ func (s *Service) forceSyncUponFinalize(
 			SafeBlockHash:      executionPayload.GetParentHash(),
 			FinalizedBlockHash: executionPayload.GetParentHash(),
 		},
-		s.chainSpec.ActiveForkVersionForSlot(beaconBlock.GetSlot()),
+		s.chainSpec.ActiveForkVersionForTimestamp(executionPayload.GetTimestamp()),
 	)
 
 	switch _, err = s.executionEngine.NotifyForkchoiceUpdate(ctx, req); {
@@ -196,12 +183,12 @@ func (s *Service) rebuildPayloadForRejectedBlock(
 	}
 
 	// Submit a request for a new payload.
-	if _, err = s.localBuilder.RequestPayloadAsync(
+	if _, _, err = s.localBuilder.RequestPayloadAsync(
 		ctx,
 		st,
 		// We are rebuilding for the current slot.
 		stateSlot,
-		nextPayloadTimestamp.Unwrap(),
+		nextPayloadTimestamp,
 		// We set the parent root to the previous block root. The HashTreeRoot
 		// of the header is the same as the HashTreeRoot of the block.
 		latestHeader.HashTreeRoot(),
@@ -264,10 +251,10 @@ func (s *Service) optimisticPayloadBuild(
 
 	// We then trigger a request for the next payload.
 	payload := blk.GetBody().GetExecutionPayload()
-	if _, err := s.localBuilder.RequestPayloadAsync(
+	if _, _, err := s.localBuilder.RequestPayloadAsync(
 		ctx, st,
 		slot,
-		nextPayloadTimestamp.Unwrap(),
+		nextPayloadTimestamp,
 		// The previous block root is simply the root of the block we just
 		// processed.
 		blk.HashTreeRoot(),

beacon/blockchain/process_proposal.go

@@ -37,6 +37,7 @@ import (
 	"github.com/berachain/beacon-kit/primitives/eip4844"
 	"github.com/berachain/beacon-kit/primitives/math"
 	"github.com/berachain/beacon-kit/primitives/transition"
+	"github.com/berachain/beacon-kit/primitives/version"
 	"github.com/berachain/beacon-kit/state-transition/core"
 	statedb "github.com/berachain/beacon-kit/state-transition/core/state"
 	cmtabci "github.com/cometbft/cometbft/abci/types"
@@ -67,12 +68,14 @@ func (s *Service) ProcessProposal(
 		)
 	}
 
+	forkVersion := s.chainSpec.ActiveForkVersionForTimestamp(math.U64(req.GetTime().Unix())) //#nosec: G115
 	// Decode signed block and sidecars.
 	signedBlk, sidecars, err := encoding.ExtractBlobsAndBlockFromRequest(
 		req,
 		BeaconBlockTxIndex,
 		BlobSidecarsTxIndex,
-		s.chainSpec.ActiveForkVersionForSlot(math.Slot(req.Height))) // #nosec G115
+		forkVersion,
+	)
 	if err != nil {
 		return err
 	}
@@ -90,24 +93,41 @@ func (s *Service) ProcessProposal(
 	}
 
 	blk := signedBlk.GetBeaconBlock()
+
+	// There are two different timestamps:
+	//     - The "consensus time" is determined by CometBFT consensus and can be retrieved with `req.GetTime()`
+	//     - The "block time" is determined by beacon-kit consensus and can be retrieved with `blk.GetTimestamp()`
+	// The "consensus time" is what the network agrees the current time is based on CometBFT PBTS.
+	// This "consensus time" is used to constrain the timestamp set as the "block time" by the
+	// beacon-kit app, but they are not always equal in value. The "block time" is used by the
+	// beacon-kit consensus and execution layers to determine the active fork version.
+	//
+	// When unmarshaling the BeaconBlock, we do not yet have access to the "block time", so we
+	// must rely on the "consensus time" as our best estimation of the "block time" needed to
+	// determine the current fork version. Since the two timestamps could be different, we need to
+	// ensure that the fork version for these timestamps are the same. This may result in a failed
+	// proposal or two at the start of the fork.
+	blkVersion := s.chainSpec.ActiveForkVersionForTimestamp(blk.GetTimestamp())
+	if !version.Equals(blkVersion, forkVersion) {
+		return fmt.Errorf("CometBFT version %v, BeaconBlock version %v: %w",
+			forkVersion, blkVersion,
+			ErrVersionMismatch,
+		)
+	}
 	// Make sure we have the right number of BlobSidecars
 	blobKzgCommitments := blk.GetBody().GetBlobKzgCommitments()
 	numCommitments := len(blobKzgCommitments)
 	if numCommitments != len(sidecars) {
-		err = fmt.Errorf("expected %d sidecars, got %d: %w",
+		return fmt.Errorf("expected %d sidecars, got %d: %w",
 			numCommitments, len(sidecars),
 			ErrSidecarCommitmentMismatch,
 		)
-		s.logger.Warn(err.Error())
-		return err
 	}
 	if uint64(numCommitments) > s.chainSpec.MaxBlobsPerBlock() {
-		err = fmt.Errorf("expected less than %d sidecars, got %d: %w",
+		return fmt.Errorf("expected less than %d sidecars, got %d: %w",
 			s.chainSpec.MaxBlobsPerBlock(), numCommitments,
 			core.ErrExceedsBlockBlobLimit,
 		)
-		s.logger.Warn(err.Error())
-		return err
 	}
 
 	// Verify the block and sidecar signatures. We can simply verify the block
@@ -140,7 +160,7 @@ func (s *Service) ProcessProposal(
 		}
 	}
 
-	// Process the block
+	// Process the block.
 	consensusBlk := types.NewConsensusBlock(
 		blk,
 		req.GetProposerAddress(),

beacon/validator/block_builder.go

@@ -62,10 +62,7 @@ func (s *Service) BuildBlockAndSidecars(
 	// and safe block hashes to the execution client.
 	st := s.sb.StateFromContext(ctx)
 
-	// we introduce hard forks with the expectation that the height set for the
-	// hard fork is the first height at which new rules apply. So we need to make
-	// sure that when building blocks, we pick the right height. blkSlots is the
-	// height for the next block, which consensus is requesting BeaconKit to build.
+	// blkSlot is the height for the next block, which consensus is requesting BeaconKit to build.
 	blkSlot := slotData.GetSlot()
 
 	// Prepare the state such that it is ready to build a block for
@@ -74,29 +71,44 @@ func (s *Service) BuildBlockAndSidecars(
 		return nil, nil, err
 	}
 
-	// Build forkdata used for the signing root of the reveal and the sidecars
-	forkData, err := s.buildForkData(st, blkSlot)
+	// Grab parent block root for payload request.
+	parentBlockRoot, err := st.GetBlockRootAtIndex(
+		(blkSlot.Unwrap() - 1) % s.chainSpec.SlotsPerHistoricalRoot(),
+	)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	// Build the reveal for the current slot.
-	// TODO: We can optimize to pre-compute this in parallel?
-	reveal, err := s.buildRandaoReveal(forkData, blkSlot)
+	// Get the payload for the block.
+	envelope, err := s.retrieveExecutionPayload(ctx, st, parentBlockRoot, slotData)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed retrieving execution payload: %w", err)
+	}
+
+	// We introduce hard forks with the expectation that the first block proposed after the
+	// hard fork timestamp is when new rules apply. When building blocks, we provide the Execution
+	// Layer client with a timestamp, and it will create its payload based on that timestamp. We
+	// must use this same timestamp from the payload to build the beacon block. This ensures that
+	// we are building on the same fork version as the Execution Layer.
+	timestamp := envelope.GetExecutionPayload().GetTimestamp()
+
+	// Build forkdata used for the signing root of the reveal and the sidecars
+	forkData, err := s.buildForkData(st, timestamp)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	// Create a new empty block from the current state.
-	blk, err := s.getEmptyBeaconBlockForSlot(st, blkSlot)
+	blk, err := s.getEmptyBeaconBlockForSlot(st, blkSlot, forkData.CurrentVersion, parentBlockRoot)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	// Get the payload for the block.
-	envelope, err := s.retrieveExecutionPayload(ctx, st, blk, slotData)
+	// Build the reveal for the current slot.
+	// TODO: We can optimize to pre-compute this in parallel?
+	reveal, err := s.buildRandaoReveal(forkData, blkSlot)
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed retrieving execution payload: %w", err)
+		return nil, nil, err
 	}
 
 	// We have to assemble the block body prior to producing the sidecars
@@ -150,15 +162,8 @@ func (s *Service) BuildBlockAndSidecars(
 // getEmptyBeaconBlockForSlot creates a new empty block.
 func (s *Service) getEmptyBeaconBlockForSlot(
 	st *statedb.StateDB, requestedSlot math.Slot,
+	forkVersion common.Version, parentBlockRoot common.Root,
 ) (*ctypes.BeaconBlock, error) {
-	// Create a new block.
-	parentBlockRoot, err := st.GetBlockRootAtIndex(
-		(requestedSlot.Unwrap() - 1) % s.chainSpec.SlotsPerHistoricalRoot(),
-	)
-	if err != nil {
-		return nil, err
-	}
-
 	// Get the proposer index for the slot.
 	proposerIndex, err := st.ValidatorIndexByPubkey(
 		s.signer.PublicKey(),
@@ -167,22 +172,23 @@ func (s *Service) getEmptyBeaconBlockForSlot(
 		return nil, err
 	}
 
+	// Create a new block.
 	return ctypes.NewBeaconBlockWithVersion(
 		requestedSlot,
 		proposerIndex,
 		parentBlockRoot,
-		s.chainSpec.ActiveForkVersionForSlot(requestedSlot),
+		forkVersion,
 	)
 }
 
-func (s *Service) buildForkData(st *statedb.StateDB, slot math.Slot) (*ctypes.ForkData, error) {
+func (s *Service) buildForkData(st *statedb.StateDB, timestamp math.U64) (*ctypes.ForkData, error) {
 	genesisValidatorsRoot, err := st.GetGenesisValidatorsRoot()
 	if err != nil {
 		return nil, err
 	}
 
 	return ctypes.NewForkData(
-		s.chainSpec.ActiveForkVersionForSlot(slot),
+		s.chainSpec.ActiveForkVersionForTimestamp(timestamp),
 		genesisValidatorsRoot,
 	), nil
 }
@@ -206,7 +212,7 @@ func (s *Service) buildRandaoReveal(
 func (s *Service) retrieveExecutionPayload(
 	ctx context.Context,
 	st *statedb.StateDB,
-	blk *ctypes.BeaconBlock,
+	parentBlockRoot common.Root,
 	slotData *types.SlotData,
 ) (ctypes.BuiltExecutionPayloadEnv, error) {
 	//
@@ -215,8 +221,8 @@ func (s *Service) retrieveExecutionPayload(
 	// Get the payload for the block.
 	envelope, err := s.localPayloadBuilder.RetrievePayload(
 		ctx,
-		blk.GetSlot(),
-		blk.GetParentBlockRoot(),
+		slotData.GetSlot(),
+		parentBlockRoot,
 	)
 	if err == nil {
 		return envelope, nil
@@ -232,7 +238,7 @@ func (s *Service) retrieveExecutionPayload(
 	// this less confusing.
 
 	s.metrics.failedToRetrievePayload(
-		blk.GetSlot(),
+		slotData.GetSlot(),
 		err,
 	)
 
@@ -246,13 +252,13 @@ func (s *Service) retrieveExecutionPayload(
 	return s.localPayloadBuilder.RequestPayloadSync(
 		ctx,
 		st,
-		blk.GetSlot(),
+		slotData.GetSlot(),
 		payloadtime.Next(
 			slotData.GetConsensusTime(),
 			lph.GetTimestamp(),
 			false, // buildOptimistically
-		).Unwrap(),
-		blk.GetParentBlockRoot(),
+		),
+		parentBlockRoot,
 		lph.GetBlockHash(),
 		lph.GetParentHash(),
 	)