beacon/engine: strip type byte in requests

This change brings geth into compliance with the current engine API specification for the
Prague fork. I have moved the assignment of ExecutionPayloadEnvelope.Requests into
BlockToExecutableData to ensure there is a single place where the type is removed.

While doing so, I noticed that handling of requests in the miner was not quite correct for
the empty payload. It would return `nil` requests for the empty payload even for blocks
after the Prague fork. To fix this, I have added the emptyRequests field in miner.Payload.

beacon/blsync/engineclient.go

@@ -92,7 +92,7 @@ func (ec *engineClient) updateLoop(headCh <-chan types.ChainHeadEvent) {
 }
 
 func (ec *engineClient) callNewPayload(fork string, event types.ChainHeadEvent) (string, error) {
-	execData := engine.BlockToExecutableData(event.Block, nil, nil).ExecutionPayload
+	execData := engine.BlockToExecutableData(event.Block, nil, nil, nil).ExecutionPayload
 
 	var (
 		method string

beacon/engine/types.go

@@ -260,7 +260,15 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H
 
 	var requestsHash *common.Hash
 	if requests != nil {
-		h := types.CalcRequestsHash(requests)
+		// Put back request type byte.
+		typedRequests := make([][]byte, len(requests))
+		for i, reqdata := range requests {
+			typedReqdata := make([]byte, len(reqdata)+1)
+			typedReqdata[0] = byte(i)
+			copy(typedReqdata[1:], reqdata)
+			typedRequests[i] = typedReqdata
+		}
+		h := types.CalcRequestsHash(typedRequests)
 		requestsHash = &h
 	}
 
@@ -294,7 +302,7 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H
 
 // BlockToExecutableData constructs the ExecutableData structure by filling the
 // fields from the given block. It assumes the given block is post-merge block.
-func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.BlobTxSidecar) *ExecutionPayloadEnvelope {
+func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.BlobTxSidecar, requests [][]byte) *ExecutionPayloadEnvelope {
 	data := &ExecutableData{
 		BlockHash:        block.Hash(),
 		ParentHash:       block.ParentHash(),
@@ -315,6 +323,8 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.
 		ExcessBlobGas:    block.ExcessBlobGas(),
 		ExecutionWitness: block.ExecutionWitness(),
 	}
+
+	// Add blobs.
 	bundle := BlobsBundleV1{
 		Commitments: make([]hexutil.Bytes, 0),
 		Blobs:       make([]hexutil.Bytes, 0),
@@ -327,7 +337,23 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.
 			bundle.Proofs = append(bundle.Proofs, hexutil.Bytes(sidecar.Proofs[j][:]))
 		}
 	}
-	return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &bundle, Override: false}
+
+	// Remove type byte in requests.
+	var plainRequests [][]byte
+	if requests != nil {
+		plainRequests = make([][]byte, len(requests))
+		for i, reqdata := range requests {
+			plainRequests[i] = reqdata[1:]
+		}
+	}
+
+	return &ExecutionPayloadEnvelope{
+		ExecutionPayload: data,
+		BlockValue:       fees,
+		BlobsBundle:      &bundle,
+		Requests:         plainRequests,
+		Override:         false,
+	}
 }
 
 // ExecutionPayloadBody is used in the response to GetPayloadBodiesByHash and GetPayloadBodiesByRange

eth/catalyst/api_test.go

@@ -590,10 +590,10 @@ We expect
 (3) If the parent is unavailable, the LVH should not be set.
 
 	CommonAncestor◄─▲── P1 ◄── P2  ◄─ P3  ◄─ ... ◄─ Pn
-	                │
+					│
-	                └── P1' ◄─ P2' ◄─ P3' ◄─ ... ◄─ Pn'
+					└── P1' ◄─ P2' ◄─ P3' ◄─ ... ◄─ Pn'
-	                │
+					│
-	                └── P1''
+					└── P1''
 */
 func TestNewPayloadOnInvalidChain(t *testing.T) {
 	genesis, preMergeBlocks := generateMergeChain(10, false)
@@ -1600,7 +1600,7 @@ func TestBlockToPayloadWithBlobs(t *testing.T) {
 	}
 
 	block := types.NewBlock(&header, &types.Body{Transactions: txs}, nil, trie.NewStackTrie(nil))
-	envelope := engine.BlockToExecutableData(block, nil, sidecars)
+	envelope := engine.BlockToExecutableData(block, nil, sidecars, nil)
 	var want int
 	for _, tx := range txs {
 		want += len(tx.BlobHashes())

miner/payload_building.go

@@ -69,26 +69,28 @@ func (args *BuildPayloadArgs) Id() engine.PayloadID {
 // the revenue. Therefore, the empty-block here is always available and full-block
 // will be set/updated afterwards.
 type Payload struct {
-	id           engine.PayloadID
+	id            engine.PayloadID
-	empty        *types.Block
+	empty         *types.Block
-	emptyWitness *stateless.Witness
+	emptyWitness  *stateless.Witness
-	full         *types.Block
+	full          *types.Block
-	fullWitness  *stateless.Witness
+	fullWitness   *stateless.Witness
-	sidecars     []*types.BlobTxSidecar
+	sidecars      []*types.BlobTxSidecar
-	requests     [][]byte
+	emptyRequests [][]byte
-	fullFees     *big.Int
+	requests      [][]byte
-	stop         chan struct{}
+	fullFees      *big.Int
-	lock         sync.Mutex
+	stop          chan struct{}
-	cond         *sync.Cond
+	lock          sync.Mutex
+	cond          *sync.Cond
 }
 
 // newPayload initializes the payload object.
-func newPayload(empty *types.Block, witness *stateless.Witness, id engine.PayloadID) *Payload {
+func newPayload(empty *types.Block, emptyRequests [][]byte, witness *stateless.Witness, id engine.PayloadID) *Payload {
 	payload := &Payload{
-		id:           id,
+		id:            id,
-		empty:        empty,
+		empty:         empty,
-		emptyWitness: witness,
+		emptyRequests: emptyRequests,
-		stop:         make(chan struct{}),
+		emptyWitness:  witness,
+		stop:          make(chan struct{}),
 	}
 	log.Info("Starting work on payload", "id", payload.id)
 	payload.cond = sync.NewCond(&payload.lock)
@@ -143,16 +145,14 @@ func (payload *Payload) Resolve() *engine.ExecutionPayloadEnvelope {
 		close(payload.stop)
 	}
 	if payload.full != nil {
-		envelope := engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars)
+		envelope := engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars, payload.emptyRequests)
-		envelope.Requests = payload.requests
 		if payload.fullWitness != nil {
 			envelope.Witness = new(hexutil.Bytes)
 			*envelope.Witness, _ = rlp.EncodeToBytes(payload.fullWitness) // cannot fail
 		}
 		return envelope
 	}
-	envelope := engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil)
+	envelope := engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil, payload.emptyRequests)
-	envelope.Requests = payload.requests
 	if payload.emptyWitness != nil {
 		envelope.Witness = new(hexutil.Bytes)
 		*envelope.Witness, _ = rlp.EncodeToBytes(payload.emptyWitness) // cannot fail
@@ -166,8 +166,7 @@ func (payload *Payload) ResolveEmpty() *engine.ExecutionPayloadEnvelope {
 	payload.lock.Lock()
 	defer payload.lock.Unlock()
 
-	envelope := engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil)
+	envelope := engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil, payload.emptyRequests)
-	envelope.Requests = payload.requests
 	if payload.emptyWitness != nil {
 		envelope.Witness = new(hexutil.Bytes)
 		*envelope.Witness, _ = rlp.EncodeToBytes(payload.emptyWitness) // cannot fail
@@ -198,8 +197,7 @@ func (payload *Payload) ResolveFull() *engine.ExecutionPayloadEnvelope {
 	default:
 		close(payload.stop)
 	}
-	envelope := engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars)
+	envelope := engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars, payload.requests)
-	envelope.Requests = payload.requests
 	if payload.fullWitness != nil {
 		envelope.Witness = new(hexutil.Bytes)
 		*envelope.Witness, _ = rlp.EncodeToBytes(payload.fullWitness) // cannot fail
@@ -227,7 +225,7 @@ func (miner *Miner) buildPayload(args *BuildPayloadArgs, witness bool) (*Payload
 		return nil, empty.err
 	}
 	// Construct a payload object for return.
-	payload := newPayload(empty.block, empty.witness, args.Id())
+	payload := newPayload(empty.block, empty.requests, empty.witness, args.Id())
 
 	// Spin up a routine for updating the payload in background. This strategy
 	// can maximum the revenue for including transactions with highest fee.