Skip to content

chore: Support multiple supervisors: Decouple challengers from networks [4/N]#243

Merged
janjakubnanista merged 2 commits intomainfrom
jan/challengers-fix--004
May 1, 2025
Merged

chore: Support multiple supervisors: Decouple challengers from networks [4/N]#243
janjakubnanista merged 2 commits intomainfrom
jan/challengers-fix--004

Conversation

@janjakubnanista
Copy link
Copy Markdown
Collaborator

@janjakubnanista janjakubnanista commented Apr 30, 2025

Description

The challenger instances are decoupled from L2 networks and their configuration is moving to the top-level configuration.

Notes

There are several larger motions in this PR aimed at laying down the groundwork for further improvements:

  • The intermediate step of parsing the input to dict, then reformatting it to a struct has been removed. As it is, it adds no value and only creates room for error
  • The input parser for challenger has been moved to its own file
  • Challenger configuration is structured as a dictionary in YAML - this enforces unique and YAML-valid (no whitespace, etc) names, which removes the need for extra validation logic. The dictionary is then parsed into a list for ease of enumeration, and the names are added to the config objects themselves
  • There is no default configuration for challenger (in the sense that there is no challengers.default_params object. Instead, YAML anchors can be utilised to share pieces of configuration. This removes the need for extra parsing logic and keeps things as lean as possible
  • The config objects are enriched with service_name property. This will become important when it comes to resolving circular dependencies later on, as URLs will be possible to construct without having an instance of Service
  • The ubiquitous mkdir && combined with sh -c entrypoint has been replaced (only for op-challenger) with a persistent directory.

Questions

@sigma there will be more objects where something points to a list of networks. I named the field participants so that later on we might expand it to cover both networks and interop sets (with a heterogeneous list of some sort, TBD). That okay? I want to avoid confusion with participants in chains that signify nodes rather than networks.

Related to ethereum-optimism/optimism#15611

@janjakubnanista janjakubnanista marked this pull request as draft April 30, 2025 18:14
@janjakubnanista janjakubnanista force-pushed the jan/challengers-fix--004 branch from 656dcc3 to 45affae Compare April 30, 2025 18:15
@janjakubnanista janjakubnanista marked this pull request as ready for review April 30, 2025 18:15
@janjakubnanista janjakubnanista requested a review from sigma April 30, 2025 18:15
@janjakubnanista janjakubnanista self-assigned this Apr 30, 2025
@janjakubnanista janjakubnanista force-pushed the jan/challengers-fix--004 branch 6 times, most recently from 8770dec to 3e0e845 Compare May 1, 2025 05:48
@sigma
Copy link
Copy Markdown
Collaborator

sigma commented May 1, 2025

@sigma there will be more objects where something points to a list of networks. I named the field participants so that later on we might expand it to cover both networks and interop sets (with a heterogeneous list of some sort, TBD). That okay? I want to avoid confusion with participants in chains that signify nodes rather than networks.

Yeah that seems fine to me. I guess fundamentally what we would like to write is some sort of predicate to select the right items in the overall topology, but yaml being yaml it's gonna look a little bit uglier than that :)

@janjakubnanista janjakubnanista force-pushed the jan/challengers-fix--004 branch from 3e0e845 to 3a562e9 Compare May 1, 2025 15:31
@janjakubnanista janjakubnanista merged commit 402388c into main May 1, 2025
7 checks passed
@janjakubnanista janjakubnanista deleted the jan/challengers-fix--004 branch May 1, 2025 15:52
@samlaf
Copy link
Copy Markdown
Contributor

samlaf commented May 5, 2025

Yeah that seems fine to me. I guess fundamentally what we would like to write is some sort of predicate to select the right items in the overall topology, but yaml being yaml it's gonna look a little bit uglier than that :)

@sigma you could use tags like kubernetes for this no? Make sure all the chains you want challenger to run against are tagged with a challenger-target tag or something. Not sure this is any cleaner given that kurtosis is a single-config deployment pattern actually though.. Curious what did you have in mind with your predicates then?

@samlaf
Copy link
Copy Markdown
Contributor

samlaf commented May 5, 2025

@janjakubnanista just looked at the new config in the README, its a bit ambiguous how to set this off to turn off all challengers.
image

Aka what's the default behavior if I don't add a challengers: section? What if I do but it has no internal fields? Assuming no challengers would be run but would be good to indicate that.

samlaf added a commit to Layr-Labs/hokulea that referenced this pull request May 5, 2025
samlaf added a commit to Layr-Labs/hokulea that referenced this pull request May 5, 2025
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