From 8a1c8e22b95ea5809eeb6d20b71c8003eff64c3b Mon Sep 17 00:00:00 2001
From: lightclient <lightclient@protonmail.com>
Date: Mon, 7 Aug 2023 08:19:57 -0600
Subject: [PATCH 1/8] miner: add ability to build block with blobs

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: lightclient <lightclient@protonmail.com>
---
 beacon/engine/types.go    | 19 ++++++++-----
 miner/payload_building.go | 18 +++++++------
 miner/worker.go           | 57 ++++++++++++++++++++++++++++++---------
 miner/worker_test.go      |  4 +--
 4 files changed, 68 insertions(+), 30 deletions(-)

diff --git a/beacon/engine/types.go b/beacon/engine/types.go
index cf06a6333eb..b78444c7bac 100644
--- a/beacon/engine/types.go
+++ b/beacon/engine/types.go
@@ -23,7 +23,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/crypto/kzg4844"
 	"github.com/ethereum/go-ethereum/trie"
 )
 
@@ -237,7 +236,7 @@ func ExecutableDataToBlock(params ExecutableData, versionedHashes []common.Hash)
 
 // 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, blobs []kzg4844.Blob, commitments []kzg4844.Commitment, proofs []kzg4844.Proof) *ExecutionPayloadEnvelope {
+func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.BlobTxSidecar) *ExecutionPayloadEnvelope {
 	data := &ExecutableData{
 		BlockHash:     block.Hash(),
 		ParentHash:    block.ParentHash(),
@@ -258,17 +257,23 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, blobs []kzg4844.Bl
 		ExcessBlobGas: block.ExcessBlobGas(),
 		// TODO BeaconRoot
 	}
-	blobsBundle := BlobsBundleV1{
+	bundle := BlobsBundleV1{
 		Commitments: make([]hexutil.Bytes, 0),
 		Blobs:       make([]hexutil.Bytes, 0),
 		Proofs:      make([]hexutil.Bytes, 0),
 	}
-	for i := range blobs {
-		blobsBundle.Blobs = append(blobsBundle.Blobs, hexutil.Bytes(blobs[i][:]))
-		blobsBundle.Commitments = append(blobsBundle.Commitments, hexutil.Bytes(commitments[i][:]))
-		blobsBundle.Proofs = append(blobsBundle.Proofs, hexutil.Bytes(proofs[i][:]))
+	for _, sidecar := range sidecars {
+		for j := range sidecar.Blobs {
+			bundle.Blobs = append(bundle.Blobs, hexutil.Bytes(sidecar.Blobs[j][:]))
+			bundle.Commitments = append(bundle.Commitments, hexutil.Bytes(sidecar.Commitments[j][:]))
+			bundle.Proofs = append(bundle.Proofs, hexutil.Bytes(sidecar.Proofs[j][:]))
+		}
 	}
+<<<<<<< Updated upstream
 	return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &blobsBundle}
+=======
+	return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &bundle, Override: false}
+>>>>>>> Stashed changes
 }
 
 // ExecutionPayloadBodyV1 is used in the response to GetPayloadBodiesByHashV1 and GetPayloadBodiesByRangeV1
diff --git a/miner/payload_building.go b/miner/payload_building.go
index 299196a3cdf..fe4e284fad8 100644
--- a/miner/payload_building.go
+++ b/miner/payload_building.go
@@ -65,6 +65,7 @@ type Payload struct {
 	id       engine.PayloadID
 	empty    *types.Block
 	full     *types.Block
+	sidecars []*types.BlobTxSidecar
 	fullFees *big.Int
 	stop     chan struct{}
 	lock     sync.Mutex
@@ -84,7 +85,7 @@ func newPayload(empty *types.Block, id engine.PayloadID) *Payload {
 }
 
 // update updates the full-block with latest built version.
-func (payload *Payload) update(block *types.Block, fees *big.Int, elapsed time.Duration) {
+func (payload *Payload) update(block *types.Block, fees *big.Int, sidecars []*types.BlobTxSidecar, elapsed time.Duration) {
 	payload.lock.Lock()
 	defer payload.lock.Unlock()
 
@@ -99,6 +100,7 @@ func (payload *Payload) update(block *types.Block, fees *big.Int, elapsed time.D
 	if payload.full == nil || fees.Cmp(payload.fullFees) > 0 {
 		payload.full = block
 		payload.fullFees = fees
+		payload.sidecars = sidecars
 
 		feesInEther := new(big.Float).Quo(new(big.Float).SetInt(fees), big.NewFloat(params.Ether))
 		log.Info("Updated payload", "id", payload.id, "number", block.NumberU64(), "hash", block.Hash(),
@@ -120,9 +122,9 @@ func (payload *Payload) Resolve() *engine.ExecutionPayloadEnvelope {
 		close(payload.stop)
 	}
 	if payload.full != nil {
-		return engine.BlockToExecutableData(payload.full, payload.fullFees, nil, nil, nil)
+		return engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars)
 	}
-	return engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil, nil, nil)
+	return engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil)
 }
 
 // ResolveEmpty is basically identical to Resolve, but it expects empty block only.
@@ -131,7 +133,7 @@ func (payload *Payload) ResolveEmpty() *engine.ExecutionPayloadEnvelope {
 	payload.lock.Lock()
 	defer payload.lock.Unlock()
 
-	return engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil, nil, nil)
+	return engine.BlockToExecutableData(payload.empty, big.NewInt(0), nil)
 }
 
 // ResolveFull is basically identical to Resolve, but it expects full block only.
@@ -157,7 +159,7 @@ func (payload *Payload) ResolveFull() *engine.ExecutionPayloadEnvelope {
 	default:
 		close(payload.stop)
 	}
-	return engine.BlockToExecutableData(payload.full, payload.fullFees, nil, nil, nil)
+	return engine.BlockToExecutableData(payload.full, payload.fullFees, payload.sidecars)
 }
 
 // buildPayload builds the payload according to the provided parameters.
@@ -165,7 +167,7 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 	// Build the initial version with no transaction included. It should be fast
 	// enough to run. The empty payload can at least make sure there is something
 	// to deliver for not missing slot.
-	empty, _, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, true)
+	empty, _, _, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, true)
 	if err != nil {
 		return nil, err
 	}
@@ -189,9 +191,9 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 			select {
 			case <-timer.C:
 				start := time.Now()
-				block, fees, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, false)
+				block, fees, sidecars, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, false)
 				if err == nil {
-					payload.update(block, fees, time.Since(start))
+					payload.update(block, fees, sidecars, time.Since(start))
 				}
 				timer.Reset(w.recommit)
 			case <-payload.stop:
diff --git a/miner/worker.go b/miner/worker.go
index 9526e817a4f..cd65bae96a9 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -27,6 +27,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/consensus"
 	"github.com/ethereum/go-ethereum/consensus/misc/eip1559"
+	"github.com/ethereum/go-ethereum/consensus/misc/eip4844"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/txpool"
@@ -89,6 +90,8 @@ type environment struct {
 	header   *types.Header
 	txs      []*types.Transaction
 	receipts []*types.Receipt
+	sidecars []*types.BlobTxSidecar
+	blobs    int
 }
 
 // copy creates a deep copy of environment.
@@ -107,6 +110,10 @@ func (env *environment) copy() *environment {
 	}
 	cpy.txs = make([]*types.Transaction, len(env.txs))
 	copy(cpy.txs, env.txs)
+
+	cpy.sidecars = make([]*types.BlobTxSidecar, len(env.sidecars))
+	copy(cpy.sidecars, env.sidecars)
+
 	return cpy
 }
 
@@ -146,6 +153,8 @@ type newPayloadResult struct {
 	err   error
 	block *types.Block
 	fees  *big.Int
+
+	sidecars []*types.BlobTxSidecar
 }
 
 // getWorkReq represents a request for getting a new sealing work with provided parameters.
@@ -516,11 +525,12 @@ func (w *worker) mainLoop() {
 			w.commitWork(req.interrupt, req.timestamp)
 
 		case req := <-w.getWorkCh:
-			block, fees, err := w.generateWork(req.params)
+			block, fees, sidecars, err := w.generateWork(req.params)
 			req.result <- &newPayloadResult{
-				err:   err,
-				block: block,
-				fees:  fees,
+				err:      err,
+				block:    block,
+				fees:     fees,
+				sidecars: sidecars,
 			}
 
 		case ev := <-w.txsCh:
@@ -739,15 +749,24 @@ func (w *worker) commitTransaction(env *environment, tx *types.Transaction) ([]*
 		snap = env.state.Snapshot()
 		gp   = env.gasPool.Gas()
 	)
+	// TODO (MariusVanDerWijden): Move this check
+	if (env.blobs+len(tx.BlobHashes()))*params.BlobTxBlobGasPerBlob > params.BlobTxMaxBlobGasPerBlock {
+		return nil, errors.New("max data blobs reached")
+	}
 	receipt, err := core.ApplyTransaction(w.chainConfig, w.chain, &env.coinbase, env.gasPool, env.state, env.header, tx, &env.header.GasUsed, *w.chain.GetVMConfig())
 	if err != nil {
 		env.state.RevertToSnapshot(snap)
 		env.gasPool.SetGas(gp)
 		return nil, err
 	}
-	env.txs = append(env.txs, tx)
+	env.txs = append(env.txs, tx.WithoutBlobTxSidecar())
 	env.receipts = append(env.receipts, receipt)
 
+	if sc := tx.BlobTxSidecar(); sc != nil {
+		env.sidecars = append(env.sidecars, sc)
+		env.blobs += len(sc.Blobs)
+	}
+
 	return receipt.Logs, nil
 }
 
@@ -895,6 +914,16 @@ func (w *worker) prepareWork(genParams *generateParams) (*environment, error) {
 			header.GasLimit = core.CalcGasLimit(parentGasLimit, w.config.GasCeil)
 		}
 	}
+	if w.chainConfig.IsCancun(header.Number, header.Time) {
+		var excessBlobGas uint64
+		if w.chainConfig.IsCancun(parent.Number, parent.Time) {
+			excessBlobGas = eip4844.CalcExcessBlobGas(*parent.ExcessBlobGas, *parent.BlobGasUsed)
+		} else {
+			// For the first post-fork block, both parent.data_gas_used and parent.excess_data_gas are evaluated as 0
+			excessBlobGas = eip4844.CalcExcessBlobGas(0, 0)
+		}
+		header.ExcessBlobGas = &excessBlobGas
+	}
 	// Run the consensus preparation with the default or customized consensus engine.
 	if err := w.engine.Prepare(w.chain, header); err != nil {
 		log.Error("Failed to prepare header for sealing", "err", err)
@@ -923,6 +952,8 @@ func (w *worker) fillTransactions(interrupt *atomic.Int32, env *environment) err
 	for _, account := range w.eth.TxPool().Locals() {
 		if txs := remoteTxs[account]; len(txs) > 0 {
 			delete(remoteTxs, account)
+			// QQ: this seems to overwrite the local account,
+			// should we instead deduplicate then append?
 			localTxs[account] = txs
 		}
 	}
@@ -942,10 +973,10 @@ func (w *worker) fillTransactions(interrupt *atomic.Int32, env *environment) err
 }
 
 // generateWork generates a sealing block based on the given parameters.
-func (w *worker) generateWork(params *generateParams) (*types.Block, *big.Int, error) {
+func (w *worker) generateWork(params *generateParams) (*types.Block, *big.Int, []*types.BlobTxSidecar, error) {
 	work, err := w.prepareWork(params)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	defer work.discard()
 
@@ -963,9 +994,9 @@ func (w *worker) generateWork(params *generateParams) (*types.Block, *big.Int, e
 	}
 	block, err := w.engine.FinalizeAndAssemble(w.chain, work.header, work.state, work.txs, nil, work.receipts, params.withdrawals)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
-	return block, totalFees(block, work.receipts), nil
+	return block, totalFees(block, work.receipts), work.sidecars, nil
 }
 
 // commitWork generates several new sealing tasks based on the parent block
@@ -1074,7 +1105,7 @@ func (w *worker) commit(env *environment, interval func(), update bool, start ti
 // getSealingBlock generates the sealing block based on the given parameters.
 // The generation result will be passed back via the given channel no matter
 // the generation itself succeeds or not.
-func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, withdrawals types.Withdrawals, noTxs bool) (*types.Block, *big.Int, error) {
+func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, withdrawals types.Withdrawals, noTxs bool) (*types.Block, *big.Int, []*types.BlobTxSidecar, error) {
 	req := &getWorkReq{
 		params: &generateParams{
 			timestamp:   timestamp,
@@ -1091,11 +1122,11 @@ func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase
 	case w.getWorkCh <- req:
 		result := <-req.result
 		if result.err != nil {
-			return nil, nil, result.err
+			return nil, nil, nil, result.err
 		}
-		return result.block, result.fees, nil
+		return result.block, result.fees, result.sidecars, nil
 	case <-w.exitCh:
-		return nil, nil, errors.New("miner closed")
+		return nil, nil, nil, errors.New("miner closed")
 	}
 }
 
diff --git a/miner/worker_test.go b/miner/worker_test.go
index e46061daf19..78ad57314e9 100644
--- a/miner/worker_test.go
+++ b/miner/worker_test.go
@@ -452,7 +452,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co
 
 	// This API should work even when the automatic sealing is not enabled
 	for _, c := range cases {
-		block, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
+		block, _, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
 		if c.expectErr {
 			if err == nil {
 				t.Error("Expect error but get nil")
@@ -468,7 +468,7 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co
 	// This API should work even when the automatic sealing is enabled
 	w.start()
 	for _, c := range cases {
-		block, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
+		block, _, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
 		if c.expectErr {
 			if err == nil {
 				t.Error("Expect error but get nil")

From 79eaffa5d0e6bf02a0e8d062c032903e730cf740 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 01:50:01 +0200
Subject: [PATCH 2/8] beacon/engine: fixup

---
 beacon/engine/types.go | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/beacon/engine/types.go b/beacon/engine/types.go
index b78444c7bac..874f3e90aff 100644
--- a/beacon/engine/types.go
+++ b/beacon/engine/types.go
@@ -269,11 +269,7 @@ func BlockToExecutableData(block *types.Block, fees *big.Int, sidecars []*types.
 			bundle.Proofs = append(bundle.Proofs, hexutil.Bytes(sidecar.Proofs[j][:]))
 		}
 	}
-<<<<<<< Updated upstream
-	return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &blobsBundle}
-=======
-	return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &bundle, Override: false}
->>>>>>> Stashed changes
+	return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &bundle}
 }
 
 // ExecutionPayloadBodyV1 is used in the response to GetPayloadBodiesByHashV1 and GetPayloadBodiesByRangeV1

From d3814eb23e25d95c44b49512987bfabdd7915976 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 02:35:55 +0200
Subject: [PATCH 3/8] params: rename MaxBlobGasPerBlock

---
 consensus/misc/eip4844/eip4844.go | 4 ++--
 core/txpool/blobpool/blobpool.go  | 2 +-
 core/txpool/validation.go         | 4 ++--
 miner/worker.go                   | 2 +-
 params/protocol_params.go         | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/consensus/misc/eip4844/eip4844.go b/consensus/misc/eip4844/eip4844.go
index 583bcdeecd6..2dad9a0cd3d 100644
--- a/consensus/misc/eip4844/eip4844.go
+++ b/consensus/misc/eip4844/eip4844.go
@@ -42,8 +42,8 @@ func VerifyEIP4844Header(parent, header *types.Header) error {
 		return errors.New("header is missing blobGasUsed")
 	}
 	// Verify that the blob gas used remains within reasonable limits.
-	if *header.BlobGasUsed > params.BlobTxMaxBlobGasPerBlock {
-		return fmt.Errorf("blob gas used %d exceeds maximum allowance %d", *header.BlobGasUsed, params.BlobTxMaxBlobGasPerBlock)
+	if *header.BlobGasUsed > params.MaxBlobGasPerBlock {
+		return fmt.Errorf("blob gas used %d exceeds maximum allowance %d", *header.BlobGasUsed, params.MaxBlobGasPerBlock)
 	}
 	if *header.BlobGasUsed%params.BlobTxBlobGasPerBlob != 0 {
 		return fmt.Errorf("blob gas used %d not a multiple of blob gas per blob %d", header.BlobGasUsed, params.BlobTxBlobGasPerBlob)
diff --git a/core/txpool/blobpool/blobpool.go b/core/txpool/blobpool/blobpool.go
index d51d9528077..a7381ac6e79 100644
--- a/core/txpool/blobpool/blobpool.go
+++ b/core/txpool/blobpool/blobpool.go
@@ -53,7 +53,7 @@ const (
 	// maxBlobsPerTransaction is the maximum number of blobs a single transaction
 	// is allowed to contain. Whilst the spec states it's unlimited, the block
 	// data slots are protocol bound, which implicitly also limit this.
-	maxBlobsPerTransaction = params.BlobTxMaxBlobGasPerBlock / params.BlobTxBlobGasPerBlob
+	maxBlobsPerTransaction = params.MaxBlobGasPerBlock / params.BlobTxBlobGasPerBlob
 
 	// txAvgSize is an approximate byte size of a transaction metadata to avoid
 	// tiny overflows causing all txs to move a shelf higher, wasting disk space.
diff --git a/core/txpool/validation.go b/core/txpool/validation.go
index 5451116e0ad..630d5340cfc 100644
--- a/core/txpool/validation.go
+++ b/core/txpool/validation.go
@@ -120,8 +120,8 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types
 		if len(hashes) == 0 {
 			return fmt.Errorf("blobless blob transaction")
 		}
-		if len(hashes) > params.BlobTxMaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob {
-			return fmt.Errorf("too many blobs in transaction: have %d, permitted %d", len(hashes), params.BlobTxMaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob)
+		if len(hashes) > params.MaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob {
+			return fmt.Errorf("too many blobs in transaction: have %d, permitted %d", len(hashes), params.MaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob)
 		}
 		if err := validateBlobSidecar(hashes, sidecar); err != nil {
 			return err
diff --git a/miner/worker.go b/miner/worker.go
index cd65bae96a9..2e8965f09a9 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -750,7 +750,7 @@ func (w *worker) commitTransaction(env *environment, tx *types.Transaction) ([]*
 		gp   = env.gasPool.Gas()
 	)
 	// TODO (MariusVanDerWijden): Move this check
-	if (env.blobs+len(tx.BlobHashes()))*params.BlobTxBlobGasPerBlob > params.BlobTxMaxBlobGasPerBlock {
+	if (env.blobs+len(tx.BlobHashes()))*params.BlobTxBlobGasPerBlob > params.MaxBlobGasPerBlock {
 		return nil, errors.New("max data blobs reached")
 	}
 	receipt, err := core.ApplyTransaction(w.chainConfig, w.chain, &env.coinbase, env.gasPool, env.state, env.header, tx, &env.header.GasUsed, *w.chain.GetVMConfig())
diff --git a/params/protocol_params.go b/params/protocol_params.go
index aab9af2a8a4..701a2fc1d64 100644
--- a/params/protocol_params.go
+++ b/params/protocol_params.go
@@ -167,7 +167,7 @@ const (
 	BlobTxBytesPerFieldElement         = 32      // Size in bytes of a field element
 	BlobTxFieldElementsPerBlob         = 4096    // Number of field elements stored in a single data blob
 	BlobTxHashVersion                  = 0x01    // Version byte of the commitment hash
-	BlobTxMaxBlobGasPerBlock           = 1 << 19 // Maximum consumable blob gas for data blobs per block
+	MaxBlobGasPerBlock                 = 1 << 19 // Maximum consumable blob gas for data blobs per block
 	BlobTxTargetBlobGasPerBlock        = 1 << 18 // Target consumable blob gas for data blobs per block (for 1559-like pricing)
 	BlobTxBlobGasPerBlob               = 1 << 17 // Gas consumption of a single data blob (== blob byte size)
 	BlobTxMinBlobGasprice              = 1       // Minimum gas price for data blobs

From 2179c43eac1ce8cc20ca792596e60a987c97c571 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 02:38:22 +0200
Subject: [PATCH 4/8] miner: improve comment about blob gas limit check

---
 miner/worker.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/miner/worker.go b/miner/worker.go
index 2e8965f09a9..23f7a98f6be 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -749,10 +749,15 @@ func (w *worker) commitTransaction(env *environment, tx *types.Transaction) ([]*
 		snap = env.state.Snapshot()
 		gp   = env.gasPool.Gas()
 	)
-	// TODO (MariusVanDerWijden): Move this check
+
+	// Checking against blob gas limit: It's kind of ugly to perform this check here, but there
+	// isn't really a better place right now. The blob gas limit is checked at block validation time
+	// and not during execution. This means core.ApplyTransaction will not return an error if the
+	// tx has too many blobs. So we have to explicitly check it here.
 	if (env.blobs+len(tx.BlobHashes()))*params.BlobTxBlobGasPerBlob > params.MaxBlobGasPerBlock {
 		return nil, errors.New("max data blobs reached")
 	}
+
 	receipt, err := core.ApplyTransaction(w.chainConfig, w.chain, &env.coinbase, env.gasPool, env.state, env.header, tx, &env.header.GasUsed, *w.chain.GetVMConfig())
 	if err != nil {
 		env.state.RevertToSnapshot(snap)

From 1fdb204bc535cac0880d07637150b287e2d9ce30 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 03:15:57 +0200
Subject: [PATCH 5/8] miner: remove question

---
 miner/worker.go | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/miner/worker.go b/miner/worker.go
index 23f7a98f6be..38c5fd0681b 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -949,19 +949,18 @@ func (w *worker) prepareWork(genParams *generateParams) (*environment, error) {
 // into the given sealing block. The transaction selection and ordering strategy can
 // be customized with the plugin in the future.
 func (w *worker) fillTransactions(interrupt *atomic.Int32, env *environment) error {
-	// Split the pending transactions into locals and remotes
-	// Fill the block with all available pending transactions.
 	pending := w.eth.TxPool().Pending(true)
 
+	// Split the pending transactions into locals and remotes.
 	localTxs, remoteTxs := make(map[common.Address][]*txpool.LazyTransaction), pending
 	for _, account := range w.eth.TxPool().Locals() {
 		if txs := remoteTxs[account]; len(txs) > 0 {
 			delete(remoteTxs, account)
-			// QQ: this seems to overwrite the local account,
-			// should we instead deduplicate then append?
 			localTxs[account] = txs
 		}
 	}
+
+	// Fill the block with all available pending transactions.
 	if len(localTxs) > 0 {
 		txs := newTransactionsByPriceAndNonce(env.signer, localTxs, env.header.BaseFee)
 		if err := w.commitTransactions(env, txs, interrupt); err != nil {

From cea826c243f12d2685556325c3d4d88baa6901bc Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 03:40:50 +0200
Subject: [PATCH 6/8] miner: return newPayloadResult from generateWork

---
 miner/worker.go | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/miner/worker.go b/miner/worker.go
index 38c5fd0681b..66a4e424a94 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -148,13 +148,12 @@ type newWorkReq struct {
 	timestamp int64
 }
 
-// newPayloadResult represents a result struct corresponds to payload generation.
+// newPayloadResult is the result of payload generation.
 type newPayloadResult struct {
-	err   error
-	block *types.Block
-	fees  *big.Int
-
-	sidecars []*types.BlobTxSidecar
+	err      error
+	block    *types.Block
+	fees     *big.Int               // total block fees
+	sidecars []*types.BlobTxSidecar // collected blobs of blob transactions
 }
 
 // getWorkReq represents a request for getting a new sealing work with provided parameters.
@@ -525,13 +524,7 @@ func (w *worker) mainLoop() {
 			w.commitWork(req.interrupt, req.timestamp)
 
 		case req := <-w.getWorkCh:
-			block, fees, sidecars, err := w.generateWork(req.params)
-			req.result <- &newPayloadResult{
-				err:      err,
-				block:    block,
-				fees:     fees,
-				sidecars: sidecars,
-			}
+			req.result <- w.generateWork(req.params)
 
 		case ev := <-w.txsCh:
 			// Apply transactions to the pending state if we're not sealing
@@ -977,10 +970,10 @@ func (w *worker) fillTransactions(interrupt *atomic.Int32, env *environment) err
 }
 
 // generateWork generates a sealing block based on the given parameters.
-func (w *worker) generateWork(params *generateParams) (*types.Block, *big.Int, []*types.BlobTxSidecar, error) {
+func (w *worker) generateWork(params *generateParams) *newPayloadResult {
 	work, err := w.prepareWork(params)
 	if err != nil {
-		return nil, nil, nil, err
+		return &newPayloadResult{err: err}
 	}
 	defer work.discard()
 
@@ -998,9 +991,13 @@ func (w *worker) generateWork(params *generateParams) (*types.Block, *big.Int, [
 	}
 	block, err := w.engine.FinalizeAndAssemble(w.chain, work.header, work.state, work.txs, nil, work.receipts, params.withdrawals)
 	if err != nil {
-		return nil, nil, nil, err
+		return &newPayloadResult{err: err}
+	}
+	return &newPayloadResult{
+		block:    block,
+		fees:     totalFees(block, work.receipts),
+		sidecars: work.sidecars,
 	}
-	return block, totalFees(block, work.receipts), work.sidecars, nil
 }
 
 // commitWork generates several new sealing tasks based on the parent block

From f1a8cc2f283eb92136f95182a90674c628d416d6 Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 04:21:25 +0200
Subject: [PATCH 7/8] miner: return newPayloadResult from getSealingBlock

---
 miner/payload_building.go | 42 +++++++++++++++++++++++----------------
 miner/worker.go           | 10 +++-------
 miner/worker_test.go      | 20 +++++++++----------
 3 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/miner/payload_building.go b/miner/payload_building.go
index fe4e284fad8..74c1eee59f0 100644
--- a/miner/payload_building.go
+++ b/miner/payload_building.go
@@ -85,7 +85,7 @@ func newPayload(empty *types.Block, id engine.PayloadID) *Payload {
 }
 
 // update updates the full-block with latest built version.
-func (payload *Payload) update(block *types.Block, fees *big.Int, sidecars []*types.BlobTxSidecar, elapsed time.Duration) {
+func (payload *Payload) update(r *newPayloadResult, elapsed time.Duration) {
 	payload.lock.Lock()
 	defer payload.lock.Unlock()
 
@@ -97,15 +97,23 @@ func (payload *Payload) update(block *types.Block, fees *big.Int, sidecars []*ty
 	// Ensure the newly provided full block has a higher transaction fee.
 	// In post-merge stage, there is no uncle reward anymore and transaction
 	// fee(apart from the mev revenue) is the only indicator for comparison.
-	if payload.full == nil || fees.Cmp(payload.fullFees) > 0 {
-		payload.full = block
-		payload.fullFees = fees
-		payload.sidecars = sidecars
-
-		feesInEther := new(big.Float).Quo(new(big.Float).SetInt(fees), big.NewFloat(params.Ether))
-		log.Info("Updated payload", "id", payload.id, "number", block.NumberU64(), "hash", block.Hash(),
-			"txs", len(block.Transactions()), "withdrawals", len(block.Withdrawals()), "gas", block.GasUsed(),
-			"fees", feesInEther, "root", block.Root(), "elapsed", common.PrettyDuration(elapsed))
+	if payload.full == nil || r.fees.Cmp(payload.fullFees) > 0 {
+		payload.full = r.block
+		payload.fullFees = r.fees
+		payload.sidecars = r.sidecars
+
+		feesInEther := new(big.Float).Quo(new(big.Float).SetInt(r.fees), big.NewFloat(params.Ether))
+		log.Info("Updated payload",
+			"id", payload.id,
+			"number", r.block.NumberU64(),
+			"hash", r.block.Hash(),
+			"txs", len(r.block.Transactions()),
+			"withdrawals", len(r.block.Withdrawals()),
+			"gas", r.block.GasUsed(),
+			"fees", feesInEther,
+			"root", r.block.Root(),
+			"elapsed", common.PrettyDuration(elapsed),
+		)
 	}
 	payload.cond.Broadcast() // fire signal for notifying full block
 }
@@ -167,12 +175,12 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 	// Build the initial version with no transaction included. It should be fast
 	// enough to run. The empty payload can at least make sure there is something
 	// to deliver for not missing slot.
-	empty, _, _, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, true)
-	if err != nil {
-		return nil, err
+	empty := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, true)
+	if empty.err != nil {
+		return nil, empty.err
 	}
 	// Construct a payload object for return.
-	payload := newPayload(empty, args.Id())
+	payload := newPayload(empty.block, args.Id())
 
 	// Spin up a routine for updating the payload in background. This strategy
 	// can maximum the revenue for including transactions with highest fee.
@@ -191,9 +199,9 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 			select {
 			case <-timer.C:
 				start := time.Now()
-				block, fees, sidecars, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, false)
-				if err == nil {
-					payload.update(block, fees, sidecars, time.Since(start))
+				r := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, false)
+				if r.err == nil {
+					payload.update(r, time.Since(start))
 				}
 				timer.Reset(w.recommit)
 			case <-payload.stop:
diff --git a/miner/worker.go b/miner/worker.go
index 66a4e424a94..3ec8c84dc0b 100644
--- a/miner/worker.go
+++ b/miner/worker.go
@@ -1106,7 +1106,7 @@ func (w *worker) commit(env *environment, interval func(), update bool, start ti
 // getSealingBlock generates the sealing block based on the given parameters.
 // The generation result will be passed back via the given channel no matter
 // the generation itself succeeds or not.
-func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, withdrawals types.Withdrawals, noTxs bool) (*types.Block, *big.Int, []*types.BlobTxSidecar, error) {
+func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase common.Address, random common.Hash, withdrawals types.Withdrawals, noTxs bool) *newPayloadResult {
 	req := &getWorkReq{
 		params: &generateParams{
 			timestamp:   timestamp,
@@ -1121,13 +1121,9 @@ func (w *worker) getSealingBlock(parent common.Hash, timestamp uint64, coinbase
 	}
 	select {
 	case w.getWorkCh <- req:
-		result := <-req.result
-		if result.err != nil {
-			return nil, nil, nil, result.err
-		}
-		return result.block, result.fees, result.sidecars, nil
+		return <-req.result
 	case <-w.exitCh:
-		return nil, nil, nil, errors.New("miner closed")
+		return &newPayloadResult{err: errors.New("miner closed")}
 	}
 }
 
diff --git a/miner/worker_test.go b/miner/worker_test.go
index 78ad57314e9..e504342fabd 100644
--- a/miner/worker_test.go
+++ b/miner/worker_test.go
@@ -452,32 +452,32 @@ func testGetSealingWork(t *testing.T, chainConfig *params.ChainConfig, engine co
 
 	// This API should work even when the automatic sealing is not enabled
 	for _, c := range cases {
-		block, _, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
+		r := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
 		if c.expectErr {
-			if err == nil {
+			if r.err == nil {
 				t.Error("Expect error but get nil")
 			}
 		} else {
-			if err != nil {
-				t.Errorf("Unexpected error %v", err)
+			if r.err != nil {
+				t.Errorf("Unexpected error %v", r.err)
 			}
-			assertBlock(block, c.expectNumber, c.coinbase, c.random)
+			assertBlock(r.block, c.expectNumber, c.coinbase, c.random)
 		}
 	}
 
 	// This API should work even when the automatic sealing is enabled
 	w.start()
 	for _, c := range cases {
-		block, _, _, err := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
+		r := w.getSealingBlock(c.parent, timestamp, c.coinbase, c.random, nil, false)
 		if c.expectErr {
-			if err == nil {
+			if r.err == nil {
 				t.Error("Expect error but get nil")
 			}
 		} else {
-			if err != nil {
-				t.Errorf("Unexpected error %v", err)
+			if r.err != nil {
+				t.Errorf("Unexpected error %v", r.err)
 			}
-			assertBlock(block, c.expectNumber, c.coinbase, c.random)
+			assertBlock(r.block, c.expectNumber, c.coinbase, c.random)
 		}
 	}
 }

From 3bebfce49528cf45b590a9fbd03f65a49759421a Mon Sep 17 00:00:00 2001
From: Felix Lange <fjl@twurst.com>
Date: Wed, 23 Aug 2023 04:32:13 +0200
Subject: [PATCH 8/8] eth/catalyst: fixup invocation of BlockToExecutableData

---
 eth/catalyst/api_test.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go
index c26fbd79fc8..cc0cf8f1285 100644
--- a/eth/catalyst/api_test.go
+++ b/eth/catalyst/api_test.go
@@ -1521,13 +1521,16 @@ func TestBlockToPayloadWithBlobs(t *testing.T) {
 	}
 
 	txs = append(txs, types.NewTx(&inner))
-
-	blobs := make([]kzg4844.Blob, 1)
-	commitments := make([]kzg4844.Commitment, 1)
-	proofs := make([]kzg4844.Proof, 1)
+	sidecars := []*types.BlobTxSidecar{
+		{
+			Blobs:       make([]kzg4844.Blob, 1),
+			Commitments: make([]kzg4844.Commitment, 1),
+			Proofs:      make([]kzg4844.Proof, 1),
+		},
+	}
 
 	block := types.NewBlock(&header, txs, nil, nil, trie.NewStackTrie(nil))
-	envelope := engine.BlockToExecutableData(block, nil, blobs, commitments, proofs)
+	envelope := engine.BlockToExecutableData(block, nil, sidecars)
 	var want int
 	for _, tx := range txs {
 		want += len(tx.BlobHashes())