Skip to content

Commit

Permalink
beacon/engine: strip type byte in requests (#30576)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fjl authored and holiman committed Nov 19, 2024
1 parent 7a625d3 commit b755cec
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 30 deletions.
2 changes: 1 addition & 1 deletion beacon/blsync/engineclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 29 additions & 3 deletions beacon/engine/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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(),
Expand All @@ -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),
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion eth/catalyst/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
48 changes: 23 additions & 25 deletions miner/payload_building.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit b755cec

Please sign in to comment.