-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Add EIP: RLP Execution Block Size Limit #9658
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
Conversation
✅ All reviewers have approved. |
The commit 0d57c4e (as a parent of d8272e1) contains errors. |
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.
Some comments, main one I think MiB and KiB should be used instead of MB and KB :) I also don't completely understand the 512 KiB margin, why is this added? If the block fits within the network limit it should be fine, right? Why this extra margin?
EIPS/eip-7934.md
Outdated
--- | ||
eip: 7934 | ||
title: RLP Execution Block Size Limit | ||
description: Introduce a protocol-level cap on the maximum RLP-encoded block size to 10 MB, including a 512 KB margin for beacon block size. |
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.
I think the correct unit to be used in this document is KiB
and MiB
. A MegaByte is 1e6
bytes, and a MebiByte is 2^20 = 1,048,576
bytes. See https://simple.wikipedia.org/wiki/Mebibyte (KiB = 1024 bytes, KB = 1000 bytes)
EIPS/eip-7934.md
Outdated
### Block Size Cap | ||
|
||
- Introduce constants: | ||
- `MAX_BLOCK_SIZE` set to **10 MB (10,485,760 bytes)** |
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.
- `MAX_BLOCK_SIZE` set to **10 MB (10,485,760 bytes)** | |
- `MAX_BLOCK_SIZE` set to **10 MiB (10,485,760 bytes)** |
EIPS/eip-7934.md
Outdated
|
||
- Introduce constants: | ||
- `MAX_BLOCK_SIZE` set to **10 MB (10,485,760 bytes)** | ||
- `MARGIN` set to **512 KB (524,288 bytes)** |
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.
- `MARGIN` set to **512 KB (524,288 bytes)** | |
- `MARGIN` set to **512 KiB (524,288 bytes)** |
EIPS/eip-7934.md
Outdated
--- | ||
eip: 7934 | ||
title: RLP Execution Block Size Limit | ||
description: Introduce a protocol-level cap on the maximum RLP-encoded block size to 10 MB, including a 512 KB margin for beacon block size. |
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.
description: Introduce a protocol-level cap on the maximum RLP-encoded block size to 10 MB, including a 512 KB margin for beacon block size. | |
description: Introduce a protocol-level cap on the maximum RLP-encoded block size to 10 MiB, including a 512 KiB margin for beacon block size. |
|
||
## Motivation | ||
|
||
Currently, Ethereum does not enforce a strict upper limit on the encoded size of blocks. This lack of constraint can result in: |
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.
I think we should carefully define what is a "block" here. Because for EL we could pick the block header, the block itself (header + block body (so this includes transactions and withdrawals for instance)). Or would it be some serialization of whatever gets sent over engine_newPayload
(so some kind of RLP serialization of ExecutionPayloadV_n
?)
Is only EL block the EL can reason about not the EL block with CL wrapper |
Ah right, sorry, I'm a "pure EL" dev so I completely forgot that CL adds extra "fields" in order to propagate the block. Another novice question: can we prove that this safety margin (512 KiB) is sufficient enough? Or are there cases where we use all the reserved block space for the block, and the added CL fields will now grow the data packet over 10 MiB? Is there a worst case scenario where this is possible? (Is it possible to set a safe margin limit such that we can theoretically never go over the 10 MiB) |
It was quoted as worst case of 128KiB in Belatrix https://github.com/ethereum/consensus-specs/blob/dev/specs/bellatrix/p2p-interface.md#gossipsub
Don't know what it is now; hence going for 512KiB Also is an EL devp2p limit of 10MiB (- framing) after which peer will be disconnected if they gossip it https://github.com/ethereum/devp2p/blob/master/caps/eth.md
|
lemme know when good for merging since there are a good amount of open technical comments |
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
I think good for merging |
Thank you for searching out
It really matters to me
…On Sat, Apr 26, 2025, 12:25 AM Giulio rebuffo ***@***.***> wrote:
*Giulio2002* left a comment (ethereum/EIPs#9658)
<#9658 (comment)>
lemme know when good for merging since there are a good amount of open
technical comments
I think good for merging
—
Reply to this email directly, view it on GitHub
<#9658 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/BC6Y7BDGMUR3N2L44WNMMA323KD2NAVCNFSM6AAAAAB3LYWAE6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMZRGI2TOMBXGU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
I have a naive question: with the current gas limit, block sizes typically don't exceed 3 MiB. In what scenario would a block size reach 10 MiB? |
Around 100MGas block where the block was a single tx that was all zeros |
@g11tech ready for merging. addressed everything |
- `MAX_RLP_BLOCK_SIZE` calculated as `MAX_BLOCK_SIZE - MARGIN` | ||
- Any RLP-encoded block exceeding `MAX_RLP_BLOCK_SIZE` must be considered invalid. | ||
|
||
Thus add the following check to the Ethereum protocol: |
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.
should be good idea to list where all these checks should be enforced unless speced out somewhere else
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.
added a nit but still approving, see if you would want to address it in your followup PRs which shouldn't require editor's approval (if in draft)
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.
All Reviewers Have Approved; Performing Automatic Merge...
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.
Are they going for the new ones
ATTENTION: ERC-RELATED PULL REQUESTS NOW OCCUR IN ETHEREUM/ERCS
--
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: