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

Disable watchdog if watchdog_timeout_sec config is 0 #263

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

ferranbt
Copy link
Contributor

@ferranbt ferranbt commented Dec 3, 2024

πŸ“ Summary

This PR disables the watchdog if the value from config is zero.


βœ… I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

Copy link

github-actions bot commented Dec 3, 2024

Benchmark results for b2b8d3b

Report: https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/benchmark/b2b8d3b-af6f94a/report/index.html

Date (UTC) 2024-12-03T14:50:30+00:00
Commit b2b8d3b57d404a0e5df8462be3a33d21fced0913
Base SHA af6f94ab6f97bea9ce19377c1df255d3b9b73004

Significant changes

Benchmark Mean Status
MEV-Boost SubmitBlock serialization/JSON encoding -4.54% Performance has improved.

Copy link
Contributor

@ZanCorDX ZanCorDX left a comment

Choose a reason for hiding this comment

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

I don't like hacky constants like 0. I would prefeer to make watchdog_timeout_sec Option in the cfg. The only caveat would be that it would not be fully bacwards compatible since before not having watchdog_timeout_sec would default to 3 min but we don' really use that.

@ZanCorDX
Copy link
Contributor

ZanCorDX commented Dec 3, 2024

null object pattern would be nice although for this case, it may be too much (I'll have to copy/paste your changes into multioperator for the other use of the watchdog).

@ferranbt ferranbt merged commit 921db00 into develop Dec 3, 2024
6 checks passed
@ferranbt ferranbt deleted the ferranbt/disable-watchdog-with-config branch December 3, 2024 21:05
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.

3 participants