-
Notifications
You must be signed in to change notification settings - Fork 20.9k
core,miner,params: implement EIP-7934 - RLP Execution Block Size Limit #31990
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
base: master
Are you sure you want to change the base?
Changes from all commits
1302578
c690d88
3a68e26
d91afe4
abf7707
b92396b
5f12e63
2bb5c34
80f88d7
98c9efe
66cf1f7
4267de5
669986b
e43cb0a
71a9b4e
627133d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,7 @@ type environment struct { | |
signer types.Signer | ||
state *state.StateDB // apply state changes here | ||
tcount int // tx count in cycle | ||
size uint64 // size of the block we are building | ||
gasPool *core.GasPool // available gas used to pack transactions | ||
coinbase common.Address | ||
evm *vm.EVM | ||
|
@@ -68,6 +69,11 @@ const ( | |
commitInterruptNewHead | ||
commitInterruptResubmit | ||
commitInterruptTimeout | ||
|
||
// cap the size of blocks we will produce below the max allowed by | ||
// EIP-7934. This gives us buffer room if the estimated size of the | ||
// block we are building is off from the actual encoded size. | ||
blockRLPSizeCapBuffer = 1_000_000 | ||
) | ||
|
||
// newPayloadResult is the result of payload generation. | ||
|
@@ -95,12 +101,37 @@ type generateParams struct { | |
} | ||
|
||
// generateWork generates a sealing block based on the given parameters. | ||
func (miner *Miner) generateWork(params *generateParams, witness bool) *newPayloadResult { | ||
work, err := miner.prepareWork(params, witness) | ||
func (miner *Miner) generateWork(genParam *generateParams, witness bool) *newPayloadResult { | ||
work, err := miner.prepareWork(genParam, witness) | ||
if err != nil { | ||
return &newPayloadResult{err: err} | ||
} | ||
if !params.noTxs { | ||
var includedWithdrawals types.Withdrawals | ||
|
||
// If we are post-osaka, incorporate the requested withdrawals into the | ||
// block size calculation up-front to ensure that all requested withdrawals | ||
// can be included even if we hit the size cap when filling the block with | ||
// txs. | ||
// | ||
// Also, ensure that including all requested withdrawals wouldn't bring us | ||
// over the block size cap limit. The withdrawal cap ensures that this can't | ||
// actually happen right now, but it doesn't hurt to make this code | ||
// future-proof for a situation where the withdrawal cap is lifted. | ||
if miner.chainConfig.IsOsaka(work.header.Number, work.header.Time) { | ||
maxBlockSize := params.BlockRLPSizeCap - blockRLPSizeCapBuffer | ||
|
||
for _, withdrawal := range genParam.withdrawals { | ||
if int(work.size)+params.WithdrawalSize > maxBlockSize { | ||
break | ||
} | ||
work.size += params.WithdrawalSize | ||
includedWithdrawals = append(includedWithdrawals, withdrawal) | ||
} | ||
} else { | ||
includedWithdrawals = genParam.withdrawals | ||
rjl493456442 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if !genParam.noTxs { | ||
interrupt := new(atomic.Int32) | ||
timer := time.AfterFunc(miner.config.Recommit, func() { | ||
interrupt.Store(commitInterruptTimeout) | ||
|
@@ -112,8 +143,8 @@ func (miner *Miner) generateWork(params *generateParams, witness bool) *newPaylo | |
log.Warn("Block building is interrupted", "allowance", common.PrettyDuration(miner.config.Recommit)) | ||
} | ||
} | ||
body := types.Body{Transactions: work.txs, Withdrawals: includedWithdrawals} | ||
|
||
body := types.Body{Transactions: work.txs, Withdrawals: params.withdrawals} | ||
allLogs := make([]*types.Log, 0) | ||
for _, r := range work.receipts { | ||
allLogs = append(allLogs, r.Logs...) | ||
|
@@ -256,6 +287,7 @@ func (miner *Miner) makeEnv(parent *types.Header, header *types.Header, coinbase | |
return &environment{ | ||
signer: types.MakeSigner(miner.chainConfig, header.Number, header.Time), | ||
state: state, | ||
size: uint64(header.Size()), | ||
coinbase: coinbase, | ||
header: header, | ||
witness: state.Witness(), | ||
|
@@ -273,6 +305,7 @@ func (miner *Miner) commitTransaction(env *environment, tx *types.Transaction) e | |
} | ||
env.txs = append(env.txs, tx) | ||
env.receipts = append(env.receipts, receipt) | ||
env.size += tx.Size() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It only applies the non-blob-tx, but for the blob-tx, the tx withouth sidecar should also be considered for the inclusion size. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think blobs don't need to be included here, since they do not become a part of the block. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gary's point was that his path wasn't called for blob transactions. We need to factor the blob transaction without the sidecar into the size calc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i've since updated this pr. |
||
env.tcount++ | ||
return nil | ||
} | ||
|
@@ -298,6 +331,7 @@ func (miner *Miner) commitBlobTransaction(env *environment, tx *types.Transactio | |
env.receipts = append(env.receipts, receipt) | ||
env.sidecars = append(env.sidecars, sc) | ||
env.blobs += len(sc.Blobs) | ||
env.size += tx.WithoutBlobTxSidecar().Size() | ||
*env.header.BlobGasUsed += receipt.BlobGasUsed | ||
env.tcount++ | ||
return nil | ||
|
@@ -318,7 +352,11 @@ func (miner *Miner) applyTransaction(env *environment, tx *types.Transaction) (* | |
} | ||
|
||
func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *transactionsByPriceAndNonce, interrupt *atomic.Int32) error { | ||
gasLimit := env.header.GasLimit | ||
var ( | ||
isOsaka = miner.chainConfig.IsOsaka(env.header.Number, env.header.Time) | ||
isCancun = miner.chainConfig.IsCancun(env.header.Number, env.header.Time) | ||
gasLimit = env.header.GasLimit | ||
) | ||
if env.gasPool == nil { | ||
env.gasPool = new(core.GasPool).AddGas(gasLimit) | ||
} | ||
|
@@ -374,7 +412,7 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran | |
// Most of the blob gas logic here is agnostic as to if the chain supports | ||
// blobs or not, however the max check panics when called on a chain without | ||
// a defined schedule, so we need to verify it's safe to call. | ||
if miner.chainConfig.IsCancun(env.header.Number, env.header.Time) { | ||
if isCancun { | ||
left := eip4844.MaxBlobsPerBlock(miner.chainConfig, env.header.Time) - env.blobs | ||
if left < int(ltx.BlobGas/params.BlobTxBlobGasPerBlob) { | ||
log.Trace("Not enough blob space left for transaction", "hash", ltx.Hash, "left", left, "needed", ltx.BlobGas/params.BlobTxBlobGasPerBlob) | ||
|
@@ -391,8 +429,14 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran | |
continue | ||
} | ||
|
||
// if inclusion of the transaction would put the block size over the | ||
// maximum we allow, don't add any more txs to the payload. | ||
if isOsaka && env.size+tx.Size() > params.BlockRLPSizeCap-blockRLPSizeCapBuffer { | ||
break | ||
} | ||
|
||
// Make sure all transactions after osaka have cell proofs | ||
if miner.chainConfig.IsOsaka(env.header.Number, env.header.Time) { | ||
if isOsaka { | ||
if sidecar := tx.BlobTxSidecar(); sidecar != nil { | ||
if sidecar.Version == 0 { | ||
log.Info("Including blob tx with v0 sidecar, recomputing proofs", "hash", ltx.Hash) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the CL team:
Probably we need to put a note here, at least adding a warning log that a part of withdrawals specified by the CL are discarded due to the size restriction. In practice, it's impossible to occur.