From 2099a803965b3237283ada745ef9d54a06367f14 Mon Sep 17 00:00:00 2001 From: lightclient <14004106+lightclient@users.noreply.github.com> Date: Wed, 23 Aug 2023 15:16:14 -0600 Subject: [PATCH] miner: add to build block with EIP-4844 blobs (#27875) --------- Co-authored-by: Marius van der Wijden Co-authored-by: Felix Lange --- beacon/engine/types.go | 17 +++---- consensus/misc/eip4844/eip4844.go | 4 +- core/txpool/blobpool/blobpool.go | 2 +- core/txpool/validation.go | 4 +- eth/catalyst/api_test.go | 13 ++++-- miner/payload_building.go | 50 ++++++++++++-------- miner/worker.go | 76 +++++++++++++++++++++---------- miner/worker_test.go | 20 ++++---- params/protocol_params.go | 2 +- 9 files changed, 115 insertions(+), 73 deletions(-) diff --git a/beacon/engine/types.go b/beacon/engine/types.go index cf06a6333eb9..874f3e90aff2 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,19 @@ 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][:])) + } } - return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &blobsBundle} + return &ExecutionPayloadEnvelope{ExecutionPayload: data, BlockValue: fees, BlobsBundle: &bundle} } // ExecutionPayloadBodyV1 is used in the response to GetPayloadBodiesByHashV1 and GetPayloadBodiesByRangeV1 diff --git a/consensus/misc/eip4844/eip4844.go b/consensus/misc/eip4844/eip4844.go index 583bcdeecd6e..2dad9a0cd3de 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 d51d9528077d..a7381ac6e798 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 5451116e0ade..630d5340cfc4 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/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index c26fbd79fc87..cc0cf8f12857 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()) diff --git a/miner/payload_building.go b/miner/payload_building.go index 299196a3cdf3..74c1eee59f0d 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(r *newPayloadResult, elapsed time.Duration) { payload.lock.Lock() defer payload.lock.Unlock() @@ -96,14 +97,23 @@ func (payload *Payload) update(block *types.Block, fees *big.Int, elapsed time.D // 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 - - 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 } @@ -120,9 +130,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 +141,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 +167,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,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. @@ -189,9 +199,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) - if err == nil { - payload.update(block, fees, 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 9526e817a4f8..3ec8c84dc0b2 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 } @@ -141,11 +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 + 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. @@ -516,12 +524,7 @@ func (w *worker) mainLoop() { w.commitWork(req.interrupt, req.timestamp) case req := <-w.getWorkCh: - block, fees, err := w.generateWork(req.params) - req.result <- &newPayloadResult{ - err: err, - block: block, - fees: fees, - } + req.result <- w.generateWork(req.params) case ev := <-w.txsCh: // Apply transactions to the pending state if we're not sealing @@ -739,15 +742,29 @@ func (w *worker) commitTransaction(env *environment, tx *types.Transaction) ([]* snap = env.state.Snapshot() gp = env.gasPool.Gas() ) + + // 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) 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 +912,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) @@ -915,10 +942,9 @@ 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 { @@ -926,6 +952,8 @@ func (w *worker) fillTransactions(interrupt *atomic.Int32, env *environment) err 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 { @@ -942,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, error) { +func (w *worker) generateWork(params *generateParams) *newPayloadResult { work, err := w.prepareWork(params) if err != nil { - return nil, nil, err + return &newPayloadResult{err: err} } defer work.discard() @@ -963,9 +991,13 @@ 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 &newPayloadResult{err: err} + } + return &newPayloadResult{ + block: block, + fees: totalFees(block, work.receipts), + sidecars: work.sidecars, } - return block, totalFees(block, work.receipts), nil } // commitWork generates several new sealing tasks based on the parent block @@ -1074,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, 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, @@ -1089,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, result.err - } - return result.block, result.fees, nil + return <-req.result case <-w.exitCh: - return 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 e46061daf199..e504342fabdf 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) } } } diff --git a/params/protocol_params.go b/params/protocol_params.go index aab9af2a8a4e..701a2fc1d640 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