Conversation
…WASM metadata to ChallengeConfig Replace hardcoded "term-challenge-" Docker container name prefixes with dynamic names derived from the challenge config name. This allows multiple challenges to coexist without volume/container name collisions. In challenge-orchestrator/docker.rs, volume names (tasks, cache, evals) and their corresponding environment variables are now generated from a slug of config.name instead of being hardcoded to "term-challenge-*". In challenge-orchestrator/lib.rs, cleanup_stale_task_containers() now accepts a prefix parameter so callers specify which challenge containers to clean up, replacing the hardcoded "term-challenge-" prefix. The exclude list is also dynamically constructed from the prefix. In p2p-consensus/state.rs, ChallengeConfig gains an optional wasm_module_metadata field (using WasmModuleMetadata from platform_core) to support WASM-only challenges without requiring Docker configuration. The field defaults to None via serde for backward compatibility. Tests in blockchain_state_tests.rs and the orchestrator unit tests are updated to match the new signatures and struct fields.
📝 WalkthroughWalkthroughThis pull request introduces challenge-specific slug-based volume naming in the Docker orchestrator, updates the cleanup API to accept a prefix parameter, and extends the ChallengeConfig data structure with a new optional WasmModuleMetadata field. Related tests are updated accordingly. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/challenge-orchestrator/src/docker.rs (1)
732-735:cache_volume_nameanddind_cache_volumemay collide or cause confusion.
cache_volume_name(line 732) is"challenge-{slug}-cache"whiledind_cache_volume(line 749) is"{slug}-cache". Both are created but serve different purposes. If someone names a challenge"challenge-foo", the cache volume becomes"challenge-challenge-foo-cache"and the DinD cache becomes"challenge-foo-cache"— these could overlap with another challenge named"foo". Consider using a consistent naming scheme to avoid ambiguity.Also applies to: 747-750
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/challenge-orchestrator/src/docker.rs` around lines 732 - 735, The cache volume naming is inconsistent and can collide: compute a single canonical slug once (e.g., let slug = config.name.to_lowercase().replace(' ', '-')) and then derive both names from it with distinct, explicit prefixes so their intent is clear (e.g., "challenge-{slug}-cache" for challenge cache and "dind-{slug}-cache" for the DinD cache); update the variables cache_volume_name and dind_cache_volume (and any place that creates or refers to them) to use that shared slug and the distinct prefixes to avoid collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/challenge-orchestrator/src/docker.rs`:
- Around line 747-750: The duplicated fragile slug logic
(config.name.to_lowercase().replace(' ', "-")) should be replaced by a single
helper (e.g., slugify) that normalizes the string once at the top of
start_challenge: lowercases, removes or replaces all characters not allowed in
Docker names (match [a-zA-Z0-9][a-zA-Z0-9_.-]*), collapses runs of invalid chars
to single hyphens, and trims leading/trailing separators; compute challenge_slug
= slugify(config.name) and then reuse that variable for container names and
volumes (tasks_volume, dind_cache_volume, evals_volume and any mounts/envs)
instead of repeating the inline replace.
---
Nitpick comments:
In `@crates/challenge-orchestrator/src/docker.rs`:
- Around line 732-735: The cache volume naming is inconsistent and can collide:
compute a single canonical slug once (e.g., let slug =
config.name.to_lowercase().replace(' ', '-')) and then derive both names from it
with distinct, explicit prefixes so their intent is clear (e.g.,
"challenge-{slug}-cache" for challenge cache and "dind-{slug}-cache" for the
DinD cache); update the variables cache_volume_name and dind_cache_volume (and
any place that creates or refers to them) to use that shared slug and the
distinct prefixes to avoid collisions.
| let challenge_slug = config.name.to_lowercase().replace(' ', "-"); | ||
| let tasks_volume = format!("{}-tasks", challenge_slug); | ||
| let dind_cache_volume = format!("{}-cache", challenge_slug); | ||
| let evals_volume = format!("{}-evals", challenge_slug); |
There was a problem hiding this comment.
Slug generation is fragile — only spaces are replaced, and the logic is duplicated.
The slug config.name.to_lowercase().replace(' ', "-") appears at lines 690, 734, and 747. Characters like /, .., \, or non-ASCII could produce invalid Docker volume names or introduce path traversal in mount paths (e.g., lines 782, 788, 834-836). Docker volume names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*.
Consider:
- Extract a single
slugifyhelper that strips or replaces all non-alphanumeric characters. - Compute the slug once at the top of
start_challengeand reuse it.
Proposed approach
+ /// Produce a Docker-safe slug from a challenge name.
+ fn slugify(name: &str) -> String {
+ name.to_lowercase()
+ .chars()
+ .map(|c| if c.is_ascii_alphanumeric() { c } else { '-' })
+ .collect::<String>()
+ .trim_matches('-')
+ .to_string()
+ }Then at the top of start_challenge:
+ let challenge_slug = Self::slugify(&config.name);And reuse challenge_slug everywhere (container name, cache volume, DinD volumes, env vars).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/challenge-orchestrator/src/docker.rs` around lines 747 - 750, The
duplicated fragile slug logic (config.name.to_lowercase().replace(' ', "-"))
should be replaced by a single helper (e.g., slugify) that normalizes the string
once at the top of start_challenge: lowercases, removes or replaces all
characters not allowed in Docker names (match [a-zA-Z0-9][a-zA-Z0-9_.-]*),
collapses runs of invalid chars to single hyphens, and trims leading/trailing
separators; compute challenge_slug = slugify(config.name) and then reuse that
variable for container names and volumes (tasks_volume, dind_cache_volume,
evals_volume and any mounts/envs) instead of repeating the inline replace.
1 participant
Summary
Removes hardcoded
term-challenge-prefixes from Docker container/volume naming in the challenge orchestrator and adds optional WASM module metadata toChallengeConfigin the consensus state, preparing for WASM-based challenge execution.Changes
Challenge Orchestrator (
crates/challenge-orchestrator)docker.rs: Replace all hardcodedterm-challenge-volume names and mount paths with dynamically generated names derived fromconfig.name(slugified). This includes task volumes, cache volumes, eval volumes, and their corresponding environment variables (HOST_TASKS_DIR,HOST_CACHE_DIR,CACHE_DIR,HOST_BENCHMARK_RESULTS_DIR,BENCHMARK_RESULTS_DIR).lib.rs: Parameterizecleanup_stale_task_containersto accept aprefix: &strargument instead of using the hardcodedterm-challenge-prefix. Update the container exclusion list to use the dynamic prefix. Update tests accordingly.P2P Consensus (
crates/p2p-consensus)state.rs: Add optionalwasm_module_metadata: Option<WasmModuleMetadata>field toChallengeConfig, with#[serde(default)]for backward compatibility. ImportWasmModuleMetadatafromplatform_core.Integration Tests
blockchain_state_tests.rsto includewasm_module_metadata: Nonein allChallengeConfiginstantiations.Breaking Changes
cleanup_stale_task_containersnow requires aprefixargument (previously took no arguments). Callers must be updated.Summary by CodeRabbit
New Features
Infrastructure Improvements