Skip to content

beacon/engine: strip type byte in requests #30576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Member

@lightclient lightclient Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "emptyXXX" pattern is kind of weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We keep two blocks here, the 'empty' one without transactions, and one with.

The emptyRequests are needed because there has to be an item for each request type even when there are no requests.

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