-
Notifications
You must be signed in to change notification settings - Fork 111
feat(l1)!: allow setting a target gas limit through CLI #4973
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.
Pull Request Overview
This PR adds CLI support for configuring the target gas limit in block production, addressing the limitation where localnets with only ethrex nodes were restricted to the default 30M gas limit.
- Adds a new
--block-producer.gas-limitCLI flag to allow custom gas limit configuration - Refactors
init_rpc_apito accept gas limit and extra data from the main options object instead of as separate parameters - Updates lint annotation from
allowtoexpectfor thetoo_many_argumentsclippy warning
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/ethrex/cli.rs | Adds new gas_limit CLI option field with appropriate flag configuration and default value |
| cmd/ethrex/initializers.rs | Refactors function signature to use options object for gas limit and extra data, removes hardcoded None value |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Lines of code reportTotal lines added: Detailed view |
| pub extra_data: String, | ||
| #[arg( | ||
| long = "block-producer.gas-limit", | ||
| default_value_t = DEFAULT_BUILDER_GAS_CEIL, |
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 don't understand this? Isn't the default gas ceil different depending on the fork? I understand if you want to overwrite it but, isnt the default different for each fork?
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.
No. The default gas limit is set by each client independently of the network. For example, geth currently has a default of 60M:
I think we should bump this default to 60M, but I didn't change that in this PR since it's important enough to merit its own PR and discussion.
| } | ||
|
|
||
| #[allow(clippy::too_many_arguments)] | ||
| #[expect(clippy::too_many_arguments)] |
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.
:agite:
Motivation
When spinning up localnets made up only of ethrex nodes, we can't use gas limits apart from the default one of 30M. We want to be able to set it through a CLI flag in those cases.
Description
This PR adds a flag for setting the gas limit:
--builder.gas-limit. It also renames the flag--block-producer.extra-datato--builder.extra-data.Closes #4482