Skip to content

chore: make extra_data_size_limit configurable in EthBeaconConsensus#19496

Merged
mattsse merged 10 commits intoparadigmxyz:mainfrom
Peponks9:feat/configurable-extra-data-limit
Nov 12, 2025
Merged

chore: make extra_data_size_limit configurable in EthBeaconConsensus#19496
mattsse merged 10 commits intoparadigmxyz:mainfrom
Peponks9:feat/configurable-extra-data-limit

Conversation

@Peponks9
Copy link
Copy Markdown
Contributor

@Peponks9 Peponks9 commented Nov 4, 2025

Closes #19428

cc @klkvr

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Nov 4, 2025
@mattsse mattsse added C-enhancement New feature or request A-sdk Related to reth's use as a library labels Nov 5, 2025
Copy link
Copy Markdown
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

a few nits

Comment on lines -935 to -937
/// The header is required as an arg, because we might be checking that the header is a fork
/// block before it's in the tree state and before it's in the database.
fn is_fork(&self, target: BlockWithParent) -> ProviderResult<bool> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks like you made these changes on top of your other pr
midn separating this?

Comment thread crates/ethereum/consensus/src/lib.rs Outdated
Comment on lines +41 to +42
/// Maximum allowed extra data size in bytes
pub max_extra_data_size: usize,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this should be private and instead have a getter and a fn with_max_extra_data_size builder style fn

Comment thread crates/ethereum/consensus/src/lib.rs Outdated
/// Create a new instance of [`EthBeaconConsensus`]
pub const fn new(chain_spec: Arc<ChainSpec>) -> Self {
Self { chain_spec }
Self { chain_spec, max_extra_data_size: 32 }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this should use the constant

Comment thread crates/optimism/consensus/src/lib.rs Outdated
Comment on lines +49 to +50
/// Maximum allowed extra data size in bytes
pub max_extra_data_size: usize,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same here

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Nov 5, 2025
@Peponks9 Peponks9 force-pushed the feat/configurable-extra-data-limit branch from 93fbdfc to 3203423 Compare November 8, 2025 05:35
@Peponks9 Peponks9 requested a review from shekhirin as a code owner November 8, 2025 05:51
Copy link
Copy Markdown
Contributor Author

@Peponks9 Peponks9 left a comment

Choose a reason for hiding this comment

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

thanks Matt, changes done. Just investigation on the remaining error

@Peponks9 Peponks9 force-pushed the feat/configurable-extra-data-limit branch from 5d28184 to 901e652 Compare November 10, 2025 17:14
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't think changes here are relevant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, fixing it right away

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed :)

@Peponks9 Peponks9 force-pushed the feat/configurable-extra-data-limit branch from 901e652 to 5d59d09 Compare November 10, 2025 17:38
@Peponks9 Peponks9 requested review from klkvr and mattsse November 11, 2025 15:47
Copy link
Copy Markdown
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

last nits

Comment thread crates/ethereum/consensus/src/lib.rs Outdated
Comment on lines +34 to +35
/// Default maximum extra data size in bytes (32 bytes as per Ethereum spec).
const DEFAULT_MAX_EXTRA_DATA_SIZE: usize = 32;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's use the alloy_consensus constant instead

Comment thread crates/optimism/consensus/src/lib.rs Outdated
Comment on lines +42 to +43
/// Default maximum extra data size in bytes (32 bytes as per Ethereum spec).
const DEFAULT_MAX_EXTRA_DATA_SIZE: usize = 32;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here

Copy link
Copy Markdown
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this lgtm now

@mattsse mattsse added this pull request to the merge queue Nov 12, 2025
Merged via the queue into paradigmxyz:main with commit a7a4c3b Nov 12, 2025
42 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Nov 12, 2025
emhane pushed a commit to op-rs/op-reth that referenced this pull request Nov 24, 2025
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 22, 2026
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-sdk Related to reth's use as a library C-enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Make extra_data_size_limit configurable on EthBeaconConsensus

3 participants