Skip to content

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Apr 8, 2025

Changes

  • Don't build blocks larger than 9728.KiB (Configurable)

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
image

So we shouldn't build passed this size

Types of changes

What types of changes does your code introduce?

  • New feature (a non-breaking change that adds functionality)

Testing

Requires testing

  • No

@benaadams benaadams marked this pull request as ready for review April 8, 2025 15:26
@benaadams benaadams requested a review from rubo as a code owner April 8, 2025 15:26
Copy link
Contributor

@MarekM25 MarekM25 left a 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;
Copy link
Contributor

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?

Copy link
Member Author

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

Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

use named const

@benaadams benaadams closed this Apr 8, 2025
@benaadams benaadams reopened this Apr 8, 2025
@benaadams benaadams merged commit d958861 into master Apr 9, 2025
80 checks passed
@benaadams benaadams deleted the max-block-build branch April 9, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants