feat(core,miner): implement EIP-7934 - RLP execution block size limit #31990#2237
feat(core,miner): implement EIP-7934 - RLP execution block size limit #31990#2237gzliudan wants to merge 1 commit intoXinFinOrg:dev-upgradefrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
Implements the EIP-7934 RLP-encoded block size cap (activated at Osaka) by introducing a protocol constant, enforcing the cap during block validation, and adding miner-side packing heuristics plus a regression test.
Changes:
- Add
params.MaxBlockSizeprotocol constant for the maximum RLP-encoded block size. - Enforce oversized block rejection in
core.BlockValidator.ValidateBody(Osaka+). - Add miner-side transaction packing limit and a unit test covering oversized block validation at the Osaka fork boundary.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| params/protocol_params.go | Adds MaxBlockSize protocol constant used for the new cap. |
| core/error.go | Introduces ErrBlockOversized for block-size validation failures. |
| core/block_validator.go | Enforces the RLP block size cap from Osaka onward. |
| core/block_validator_test.go | Adds a test ensuring oversized blocks are rejected only post-Osaka. |
| miner/worker.go | Tracks approximate assembled size and stops including txs when near the cap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
miner/worker.go
Outdated
| // Stop packing when the next tx would exceed the miner-side block size | ||
| // threshold. | ||
| if !w.txFitsSize(tx) { | ||
| // Stop considering this tx if it would exceed the miner-side block size | ||
| // threshold, and move on to the next candidate. | ||
| if !w.txFitsSize(tx) { | ||
| log.Debug("Skipping oversized transaction", "hash", tx.Hash(), "size", tx.Size()) | ||
| txs.Pop() | ||
| continue | ||
| } |
There was a problem hiding this comment.
The new oversized-tx guard block has a duplicated if !w.txFitsSize(tx) and mismatched indentation/bracing (nested if without closing the first). As written, this section won’t compile and the intended control-flow (skip/pop and continue) is unclear. Please collapse to a single size check with correct braces and indentation.
miner/worker.go
Outdated
| // txFits reports whether the transaction fits into the block size limit. | ||
| func (w *Work) txFitsSize(tx *types.Transaction) bool { | ||
| // this Osaka-specific cap is not enforced pre-fork | ||
| if w.config.IsOsaka(w.header.Number) { | ||
| return w.size+tx.Size() < params.MaxBlockSize-maxBlockSizeBufferZone | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
Work.size is used to enforce an RLP-encoded block size cap, but it’s initialized from header.Size(), which (per its doc) is an approximate memory usage metric (common.StorageSize), not the header’s RLP-encoded byte length. Mixing that with tx.Size() (RLP size) makes the size accounting misleading and may be either overly conservative or inaccurate. Consider initializing/tracking size using RLP-encoded sizes (e.g., encode header once to get its RLP length, or track only tx-bytes and rename the field to reflect that it’s an estimate).
| for _, tx := range specialTxs { | ||
| if !w.txFitsSize(tx) { | ||
| log.Debug("Skipping oversized transaction", "hash", tx.Hash(), "size", tx.Size()) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Miner-side enforcement of the new Osaka block-size cap isn’t covered by tests in this PR. Since miner/worker_test.go exists and this change affects transaction selection behavior, please add a unit test that activates Osaka and asserts oversized transactions are skipped (and that normal-sized transactions are still included).
gzliudan
force-pushed
the
EIP-7934
branch
3 times, most recently
from
c1dab06 to
2819ebc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes
Ref: ethereum#31990
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that