Skip to content

Conversation

@maoueh
Copy link
Contributor

@maoueh maoueh commented Dec 2, 2025

This PR introducts a --block-time=1s on builder-playground cook l1 so it's possible to product blocks faster on the launched node.

I took the libery to rename the op block time flag to be unexported and to have the InSeconds suffix. I also used time.Duration flag type for the --block-time implementation on L1 recipe, let me know if you would like this to be applied to op block-time too, or if in the opposite, would you prefer keeping the old way for both flag.

This PR introducts a `--block-time=1s` on `builder-playground cook l1` so it's possible to product blocks faster on the launched node.

I took the libery to rename the op block time flag to be unexported and to have the `InSeconds` suffix. I also used `time.Duration` flag type for the `--block-time` implementation on L1 recipe, let me know if you would like this to be applied to op block-time too, or if in the opposite, would you prefer keeping the old way for both flag.
Copilot AI review requested due to automatic review settings December 2, 2025 17:01
Copy link

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

This PR adds the ability to configure L1 block time via a new --block-time flag for the builder-playground cook l1 command, enabling faster block production on the launched node.

Key changes:

  • Added --block-time flag to L1 recipe accepting duration format (default: 12s)
  • Refactored field naming in ArtifactsBuilder for consistency, renaming OpblockTime to opBlockTimeInSeconds and adding l1BlockTimeInSeconds
  • Updated config template to use dynamic SecondsPerSlot value instead of hardcoded 12

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
playground/recipe_l1.go Adds blockTime field and --block-time flag, calls new L1BlockTime() method
playground/artifacts.go Adds l1BlockTimeInSeconds field, renames OpblockTime to opBlockTimeInSeconds, implements L1BlockTime() method, and updates template substitution
playground/config.yaml.tmpl Replaces hardcoded SECONDS_PER_SLOT: 12 with template variable {{.SecondsPerSlot}}
README.md Documents the new --block-time flag with usage example

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ferranbt
Copy link
Collaborator

ferranbt commented Dec 5, 2025

Hey, have you tried this setup? I believe (though I am not sure right away) there were some problems changing seconds per slot since some clients had those values hardcoded from the preset of the config file.

@maoueh
Copy link
Contributor Author

maoueh commented Dec 5, 2025

Hey, have you tried this setup? I believe (though I am not sure right away) there were some problems changing seconds per slot since some clients had those values hardcoded from the preset of the config file.

Yes we tested the 1s value using cook l1 and syncing another geth execution client from outside directly as well as a Besu EL. Both seemed to have worked properly. I wasn't sure either if it would work actually :) So yes we tested a bit changing the hard-coded value directly in code to see. It seemed to have worked properly yes.

I need to fix the wrong logic of the min(...) as it's right, it always 1 second, but seems to work properly. I needed to check also if affected in any the opstack recipe, since it uses some polling internal set at 12s for some parameters. But I think this PR touches only cook l1 so cook opstack shouldn't be affected. I'll make sure it's the case next week.

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.

2 participants