Skip to content

Conversation

@tomip01
Copy link
Contributor

@tomip01 tomip01 commented Oct 15, 2025

Motivation

When running ethrex l2 --dev the sequencer flags are being ignored. We want to be able to configure the L2

Description

  • Add a new populate_with_defaults function for L2 options. This function assigns default values to any Option<T> flags that are not provided when running l2 --dev, instead of leaving them as None.
  • Update default implementation for ProofCoordinatorOptions to set proof_coordinator_qpl_tool_path as the one in the cli

Copilot AI review requested due to automatic review settings October 15, 2025 12:44
@tomip01 tomip01 requested a review from a team as a code owner October 15, 2025 12:44
@github-actions github-actions bot added the L2 Rollup client label Oct 15, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Enable honoring CLI flags for l2 --dev by hydrating missing Option fields from Defaults and setting a default QPL tool path for the proof coordinator.

  • Introduce populate_with_defaults across L2 option structs to apply defaults only for unset fields
  • Set a default proof_coordinator_qpl_tool_path and wire population in the l2 --dev flow

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
cmd/ethrex/l2/options.rs Adds populate_with_defaults for Options and nested structs; introduces DEFAULT_PROOF_COORDINATOR_QPL_TOOL_PATH; updates Default for ProofCoordinatorOptions.
cmd/ethrex/l2/command.rs Calls populate_with_defaults in the --dev path after setting node_opts to default_l2.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link

github-actions bot commented Oct 15, 2025

Lines of code report

Total lines added: 124
Total lines removed: 2
Total lines changed: 126

Detailed view
+---------------------------------+-------+------+
| File                            | Lines | Diff |
+---------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/command.rs | 647   | -2   |
+---------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/options.rs | 988   | +124 |
+---------------------------------+-------+------+

Comment on lines 30 to 32
/// Sets the value of an Option<T> to a default if it is currently None.
/// If the target already contains a value, it is left unchanged.
/// Useful for populating option fields with defaults when not specified by the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a Option::or()

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, nice catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done here 7c1b09c

@ManuelBilbao ManuelBilbao added this pull request to the merge queue Oct 15, 2025
Merged via the queue into main with commit c52eaf0 Oct 15, 2025
43 checks passed
@ManuelBilbao ManuelBilbao deleted the enable_flags_l2_dev branch October 15, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L2 Rollup client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants