-
Notifications
You must be signed in to change notification settings - Fork 548
Cap block build size in bytes #8486
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
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.
This is a good PR, but it will still have attack vectors with higher gas-limit. Of course, next improvements can be done in a seperate PRs. Let's discuss
@@ -58,5 +58,7 @@ public byte[] GetExtraDataBytes() | |||
public string GasToken { get => GasTokenTicker; set => GasTokenTicker = value; } | |||
|
|||
public static string GasTokenTicker { get; set; } = "ETH"; | |||
|
|||
public long BlockProductionMaxTxKilobytes { get; set; } = 9728; |
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 you sure it is correct value? how much CL can have on worst-case?
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.
Docs say 127kB in worse case; but not sure they are outdated so increased to 512kB (as also EL payload framing)
1024*10-512 = 9728
...ethermind/Nethermind.Consensus/Processing/BlockProcessor.BlockProductionTransactionPicker.cs
Outdated
Show resolved
Hide resolved
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.
use named const
...ethermind/Nethermind.Consensus/Processing/BlockProcessor.BlockProductionTransactionPicker.cs
Outdated
Show resolved
Hide resolved
928238a
to
d7bdd88
Compare
Changes
10MB is the max L1 CL uncompress block gossip size and also includes the block headers etc https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/p2p-interface.md#max_message_size

So we shouldn't build passed this size
Types of changes
What types of changes does your code introduce?
Testing
Requires testing