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.
📝 WalkthroughWalkthroughDocker volume naming now dynamically generates names using challenge name slugs instead of fixed identifiers. The container cleanup method gains a prefix parameter for scoped container removal. The ChallengeConfig struct extends to include an optional wasm module metadata field, with corresponding test updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/challenge-orchestrator/src/docker.rs (1)
730-743:⚠️ Potential issue | 🟡 Minor
cache_volume_nameis created but never mounted — orphan volume on every start.The volume
challenge-{slug}-cache(line 732) is created at line 741 but is not referenced in any bind mount. The actual cache mount at line 782 usesdind_cache_volume({slug}-cache). This creates an unused Docker volume on everystart_challengecall.Either remove this dead volume creation or replace the DinD cache volume with it if it was meant to be the canonical cache volume.
🤖 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 730 - 743, The code creates a volume using cache_volume_name and cache_volume_opts but never mounts it, while the actual container bind uses dind_cache_volume, causing an orphaned volume; fix by either removing the unused create_volume call or by using cache_volume_name as the canonical cache volume when building the container binds (replace references to dind_cache_volume with cache_volume_name in the start_challenge container setup), ensuring the created volume is the one mounted into the container and keeping variable names consistent (cache_volume_name, cache_volume_opts, dind_cache_volume).
🤖 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 current slug construction using
config.name.to_lowercase().replace(' ', "-") (used where challenge_slug,
tasks_volume, dind_cache_volume, and evals_volume are built) only replaces
spaces and allows invalid chars into Docker volume names; add a reusable slugify
helper (e.g., slugify(name: &str) -> String) that lowercases, maps
non-alphanumeric chars to '-', collapses/trims leading/trailing '-' as needed,
and then replace the existing direct to_lowercase().replace(...) calls with
slugify(&config.name) when creating challenge_slug and the related volume names
so all non-alphanumeric characters are sanitized.
---
Outside diff comments:
In `@crates/challenge-orchestrator/src/docker.rs`:
- Around line 730-743: The code creates a volume using cache_volume_name and
cache_volume_opts but never mounts it, while the actual container bind uses
dind_cache_volume, causing an orphaned volume; fix by either removing the unused
create_volume call or by using cache_volume_name as the canonical cache volume
when building the container binds (replace references to dind_cache_volume with
cache_volume_name in the start_challenge container setup), ensuring the created
volume is the one mounted into the container and keeping variable names
consistent (cache_volume_name, cache_volume_opts, dind_cache_volume).
| 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.
🛠️ Refactor suggestion | 🟠 Major
Slugification only handles spaces — other special characters will leak into Docker volume names.
config.name.to_lowercase().replace(' ', "-") leaves underscores, dots, colons, and other characters untouched. While Docker volume names are somewhat permissive ([a-zA-Z0-9][a-zA-Z0-9_.-]), challenge names with characters outside this set (e.g., my:challenge!) will produce invalid volume names. This same pattern is repeated at lines 690 and 734.
Consider extracting a proper slugify helper that strips or replaces all non-alphanumeric characters:
Suggested helper
fn slugify(name: &str) -> String {
name.to_lowercase()
.chars()
.map(|c| if c.is_ascii_alphanumeric() { c } else { '-' })
.collect::<String>()
.trim_matches('-')
.to_string()
}🤖 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
current slug construction using config.name.to_lowercase().replace(' ', "-")
(used where challenge_slug, tasks_volume, dind_cache_volume, and evals_volume
are built) only replaces spaces and allows invalid chars into Docker volume
names; add a reusable slugify helper (e.g., slugify(name: &str) -> String) that
lowercases, maps non-alphanumeric chars to '-', collapses/trims leading/trailing
'-' as needed, and then replace the existing direct to_lowercase().replace(...)
calls with slugify(&config.name) when creating challenge_slug and the related
volume names so all non-alphanumeric characters are sanitized.
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.Continuation of closed PR #25.
Summary by CodeRabbit
Refactor
New Features