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

chore: add a challenger 'enabled' flag to make its deployment optional #165

Conversation

samlaf
Copy link
Contributor

@samlaf samlaf commented Feb 11, 2025

challenger and its dependency (building the op-program cannon preimage) take a long time to build in op's monorepo.

This change is a first step in being able to disable the challenger when not needed. Will need to figure out a way to also make the cannon preimage building optional from op's repo.

challenger and its dependency (building the op-program cannon preimage) take a long time to build in op's monorepo.

This change is a first step in being able to disable to challenger. Will need to figure out a way to also make the cannon preimage building optional from op's repo
@samlaf
Copy link
Contributor Author

samlaf commented Feb 17, 2025

@janjakubnanista the tests that you asked me to write:
image
are very hard to write. The pattern used for the el_cl_launcher_test doesn't work because the op_challenger.launch function itself is behind the enabled flag. I could move the enabled flag as an arg to that function, but I personally find that more confusing, wdyt?

I still added the test template in 5459f8d, but don't think its that useful (its just checking against a huge sh argument string that I copy-pasted).

I tried adding a similar test for main.run() (which should set all the configs, and then I could query for the challenger's config and see if it's there), but ran into this issue:
image

Btw the above test (running main.run) ran into literally all the bugs that I've identified and fixed in #166, so would recommend we merge that one.

@janjakubnanista janjakubnanista merged commit 5ee6561 into ethpandaops:main Feb 18, 2025
6 checks passed
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