Skip to content
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

fix(op-batcher): support new fjord maxRLPBytesPerChannelFjord via rollup chain spec #11169

Merged
merged 3 commits into from
Jul 21, 2024

Conversation

emilianobonassi
Copy link
Contributor

@emilianobonassi emilianobonassi commented Jul 17, 2024

Context:

Fjord bump 10x MaxRLPBytesPerChannel to support higher gas limits.

On derivation the new limit is supported but op-batcher still use the old value.

This can lead to situation where the batcher halt in a chain post-Fjord with high block gas limit when a block bigger than 10M(bytes) is produced (this because a block cannot span today more than 1 channel).

This fixes #10428 using the new value when appropriated and remove legacy SafeMaxRLPBytesPerChannel.

In the comments on the channel builder there a note about the choice of timestamp.

@sebastianst sebastianst self-requested a review July 17, 2024 15:54
@sebastianst sebastianst self-assigned this Jul 17, 2024
Copy link
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

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

Thanks for this fix! I think the pragmatic solution to just use the batch's timestamp as an activation reference works well, as it is fine around the Fjord boundary to activate it a few blocks later than could be, since the max size is increased.

op-e2e/actions/garbage_channel_out.go Outdated Show resolved Hide resolved
op-batcher/batcher/channel_builder_test.go Outdated Show resolved Hide resolved
op-batcher/batcher/channel_builder.go Outdated Show resolved Hide resolved
op-e2e/actions/l2_batcher.go Outdated Show resolved Hide resolved
op-node/benchmarks/batchbuilding_test.go Outdated Show resolved Hide resolved
op-node/rollup/derive/channel_out.go Outdated Show resolved Hide resolved
op-node/rollup/derive/span_channel_out.go Outdated Show resolved Hide resolved
op-node/rollup/derive/channel_out_test.go Outdated Show resolved Hide resolved
Copy link
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ajsutton ajsutton added this pull request to the merge queue Jul 21, 2024
Merged via the queue into ethereum-optimism:develop with commit 6dfe4b0 Jul 21, 2024
63 checks passed
@emilianobonassi
Copy link
Contributor Author

Worth to mention that this fix was possible collaborating with Derive (former Lyra) team which was exploring higher gas block limits (120M).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fjord: Use new large RLP limit in the batcher
4 participants