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

Add value for MaxBuilderEpochMissedSlots #14334

Conversation

bnovil
Copy link
Contributor

@bnovil bnovil commented Aug 12, 2024

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?
Add value for MaxBuilderEpochMissedSlots to make it consistent with the documentation, https://docs.prylabs.network/docs/advanced/builder

Circuit breaker
The circuit breaker is a safety feature for falling back to local execution when using a builder. This occurs when the builder or client using the builder endpoints encounters issues that cause missed blocks. By default, the circuit breaker will be triggered after 3 slots are consecutively missed or 5 slots are missed in an epoch, but this can be configured through the --max-builder-consecutive-missed-slots and max-builder-epoch-missed-slots flags.

Which issues(s) does this PR fix?

Fixes #14275

Other notes for review

@bnovil bnovil requested a review from a team as a code owner August 12, 2024 08:34
@prestonvanloon
Copy link
Member

prestonvanloon commented Aug 12, 2024

@terencechain please review this. Why is there no default flag value and is it safe to add one?

edit: introduced in #11215

@prestonvanloon
Copy link
Member

@rkapka Please review also. Looks like the default value was removed in #13186?

@potuz
Copy link
Contributor

potuz commented Aug 12, 2024

@terencechain please review this. Why is there no default flag value and is it safe to add one?

edit: introduced in #11215

I thought this was covered here
https://github.com/prysmaticlabs/prysm/blob/develop/config/params/mainnet_config.go#L265

It needs different values per chain, I'd close this one @prestonvanloon

@prestonvanloon
Copy link
Member

Thanks @potuz.

We can either close this entirely or merge with a comment mentioning that the defaults are on a network basis and we can provide the default value for mainnet in the description such that users can understand from the help text.

@bnovil What do you think?

@bnovil
Copy link
Contributor Author

bnovil commented Aug 14, 2024

Thanks @potuz.

We can either close this entirely or merge with a comment mentioning that the defaults are on a network basis and we can provide the default value for mainnet in the description such that users can understand from the help text.

@bnovil What do you think?

I see. I'd like to update the description to make it easier to understand. Thanks for your explanation.

@prestonvanloon prestonvanloon added this pull request to the merge queue Aug 30, 2024
Merged via the queue into prysmaticlabs:develop with commit 342bb0f Aug 30, 2024
17 of 18 checks passed
james-prysm added a commit that referenced this pull request Sep 6, 2024
* add value for MaxBuilderEpochMissedSlots

* make usage for MaxBuilderEpochMisedSlots more friendly

---------

Co-authored-by: Preston Van Loon <[email protected]>
Co-authored-by: james-prysm <[email protected]>
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.

Update differences between code and help commands
4 participants