-
Notifications
You must be signed in to change notification settings - Fork 37
Modity flashblocks ws bind/port flags #71
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
crates/op-rbuilder/src/args/op.rs
Outdated
|
|
||
| /// flashblock block time in milliseconds | ||
| #[arg( | ||
| long = "rollup.flashblock-block-time", |
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.
We can have a followup issue to deprecate this flags.
| network_port: Option<u16>, | ||
| builder_private_key: Option<String>, | ||
| flashblocks_ws_url: Option<String>, | ||
| flashblocks_port: Option<u16>, |
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.
Using port here since we do not really have any utility to change the bind address during tests.
|
|
||
| if let Some(flashblocks_ws_url) = &self.flashblocks_ws_url { | ||
| if let Some(flashblocks_port) = &self.flashblocks_port { | ||
| cmd.arg("--rollup.enable-flashblocks").arg("true"); |
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.
Wondering if we should also rename rollup.enable-flashblocks to flashblocks.enabled to be more consistent.
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 was worried since Base is already using this code. But I think it should be fine. We do not provide yet any API guarantees. I am going to change the one about the flashblocks block time too.
📝 Summary
Closes #70