From add5709cb5a02183df8541505fc8e0f8c569faea Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 14 Oct 2024 21:43:35 +0200 Subject: [PATCH] beacon/engine: strip type byte in requests (#30576) 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 | 2 +- beacon/engine/types.go | 32 ++++++++++++++++++++--- eth/catalyst/api_test.go | 2 +- miner/payload_building.go | 48 +++++++++++++++++------------------ 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/beacon/blsync/engineclient.go b/beacon/blsync/engineclient.go index 97ef6f5cb8..fb8f77f32b 100644 --- a/beacon/blsync/engineclient.go +++ b/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 diff --git a/beacon/engine/types.go b/beacon/engine/types.go index 31f5e6fc2a..e80b6e6633 100644 --- a/beacon/engine/types.go +++ b/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 diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index c3116cb4b6..d4069e50e6 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -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()) diff --git a/miner/payload_building.go b/miner/payload_building.go index ce4836d8a9..1260d839c9 100644 --- a/miner/payload_building.go +++ b/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 - empty *types.Block - emptyWitness *stateless.Witness - full *types.Block - fullWitness *stateless.Witness - sidecars []*types.BlobTxSidecar - requests [][]byte - fullFees *big.Int - stop chan struct{} - lock sync.Mutex - cond *sync.Cond + id engine.PayloadID + empty *types.Block + emptyWitness *stateless.Witness + full *types.Block + fullWitness *stateless.Witness + sidecars []*types.BlobTxSidecar + emptyRequests [][]byte + requests [][]byte + fullFees *big.Int + stop chan struct{} + 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, - empty: empty, - emptyWitness: witness, - stop: make(chan struct{}), + id: id, + empty: empty, + emptyRequests: emptyRequests, + 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.Requests = payload.requests + envelope := engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars, payload.emptyRequests) 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.Requests = payload.requests + envelope := engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil, payload.emptyRequests) 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.Requests = payload.requests + envelope := engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil, payload.emptyRequests) 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.Requests = payload.requests + envelope := engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars, 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.