Skip to content

core,miner: add ability to build block with blobs #27875

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 8 commits into from
Aug 23, 2023
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
17 changes: 9 additions & 8 deletions beacon/engine/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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(),
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions consensus/misc/eip4844/eip4844.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion core/txpool/blobpool/blobpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions core/txpool/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions eth/catalyst/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
50 changes: 30 additions & 20 deletions miner/payload_building.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()

Expand All @@ -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
}
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -157,20 +167,20 @@ 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.
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.
Expand All @@ -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:
Expand Down
76 changes: 52 additions & 24 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

I think it can be treated as block.GasLeft.

Block has a gasPool and we can create a blobGasPool for blobTx. The benefit of this approach is: this check can be used in both (1) block generation and (2) block import.

Right now, we check the blob validity in (v *BlockValidator) ValidateBody when import a new block, and here when generate a new block. They can be in the same place I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Me and @lightclient tried making this change yesterday, adding blobGas into core.GasPool. I think it's not worth it right now. There are too many places in the code where we would need to suddenly care about blobGas. It would be better to refactor the state transition first, to remove duplication across the codebase.

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
}

Expand Down Expand Up @@ -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)
Expand All @@ -915,17 +942,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)
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 {
Expand All @@ -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()

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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")}
}
}

Expand Down
Loading