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.
This commit is contained in:
Felix Lange 2024-10-14 21:43:35 +02:00 committed by GitHub
parent 5adc314817
commit add5709cb5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 54 additions and 30 deletions

View File

@ -92,7 +92,7 @@ func (ec *engineClient) updateLoop(headCh <-chan types.ChainHeadEvent) {
} }
func (ec *engineClient) callNewPayload(fork string, event types.ChainHeadEvent) (string, error) { 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 ( var (
method string method string

View File

@ -260,7 +260,15 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H
var requestsHash *common.Hash var requestsHash *common.Hash
if requests != nil { 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 requestsHash = &h
} }
@ -294,7 +302,7 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H
// BlockToExecutableData constructs the ExecutableData structure by filling the // BlockToExecutableData constructs the ExecutableData structure by filling the
// fields from the given block. It assumes the given block is post-merge block. // 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{ data := &ExecutableData{
BlockHash: block.Hash(), BlockHash: block.Hash(),
ParentHash: block.ParentHash(), ParentHash: block.ParentHash(),
@ -315,6 +323,8 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.
ExcessBlobGas: block.ExcessBlobGas(), ExcessBlobGas: block.ExcessBlobGas(),
ExecutionWitness: block.ExecutionWitness(), ExecutionWitness: block.ExecutionWitness(),
} }
// Add blobs.
bundle := BlobsBundleV1{ bundle := BlobsBundleV1{
Commitments: make([]hexutil.Bytes, 0), Commitments: make([]hexutil.Bytes, 0),
Blobs: 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][:])) 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 // ExecutionPayloadBody is used in the response to GetPayloadBodiesByHash and GetPayloadBodiesByRange

View File

@ -1600,7 +1600,7 @@ func TestBlockToPayloadWithBlobs(t *testing.T) {
} }
block := types.NewBlock(&header, &types.Body{Transactions: txs}, nil, trie.NewStackTrie(nil)) 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 var want int
for _, tx := range txs { for _, tx := range txs {
want += len(tx.BlobHashes()) want += len(tx.BlobHashes())

View File

@ -69,26 +69,28 @@ func (args *BuildPayloadArgs) Id() engine.PayloadID {
// the revenue. Therefore, the empty-block here is always available and full-block // the revenue. Therefore, the empty-block here is always available and full-block
// will be set/updated afterwards. // will be set/updated afterwards.
type Payload struct { 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. // 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{ 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) log.Info("Starting work on payload", "id", payload.id)
payload.cond = sync.NewCond(&payload.lock) payload.cond = sync.NewCond(&payload.lock)
@ -143,16 +145,14 @@ func (payload *Payload) Resolve() *engine.ExecutionPayloadEnvelope {
close(payload.stop) close(payload.stop)
} }
if payload.full != nil { 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 { if payload.fullWitness != nil {
envelope.Witness = new(hexutil.Bytes) envelope.Witness = new(hexutil.Bytes)
*envelope.Witness, _ = rlp.EncodeToBytes(payload.fullWitness) // cannot fail *envelope.Witness, _ = rlp.EncodeToBytes(payload.fullWitness) // cannot fail
} }
return envelope 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 { if payload.emptyWitness != nil {
envelope.Witness = new(hexutil.Bytes) envelope.Witness = new(hexutil.Bytes)
*envelope.Witness, _ = rlp.EncodeToBytes(payload.emptyWitness) // cannot fail *envelope.Witness, _ = rlp.EncodeToBytes(payload.emptyWitness) // cannot fail
@ -166,8 +166,7 @@ func (payload *Payload) ResolveEmpty() *engine.ExecutionPayloadEnvelope {
payload.lock.Lock() payload.lock.Lock()
defer payload.lock.Unlock() 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 { if payload.emptyWitness != nil {
envelope.Witness = new(hexutil.Bytes) envelope.Witness = new(hexutil.Bytes)
*envelope.Witness, _ = rlp.EncodeToBytes(payload.emptyWitness) // cannot fail *envelope.Witness, _ = rlp.EncodeToBytes(payload.emptyWitness) // cannot fail
@ -198,8 +197,7 @@ func (payload *Payload) ResolveFull() *engine.ExecutionPayloadEnvelope {
default: default:
close(payload.stop) 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 { if payload.fullWitness != nil {
envelope.Witness = new(hexutil.Bytes) envelope.Witness = new(hexutil.Bytes)
*envelope.Witness, _ = rlp.EncodeToBytes(payload.fullWitness) // cannot fail *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 return nil, empty.err
} }
// Construct a payload object for return. // 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 // Spin up a routine for updating the payload in background. This strategy
// can maximum the revenue for including transactions with highest fee. // can maximum the revenue for including transactions with highest fee.