Skip to content

refactor(challenge): generalize hardcoded container prefixes and add WASM metadata to ChallengeConfig #42

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.

Sign up for GitHub

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

Closed
echobt wants to merge 1 commit into from
Closed

refactor(challenge): generalize hardcoded container prefixes and add WASM metadata to ChallengeConfig #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions crates/challenge-orchestrator/src/docker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,11 +744,12 @@ impl DockerClient {

// Create named volumes for Docker-in-Docker task sharing
// These volumes are shared between challenge containers and agent containers
let tasks_volume = "term-challenge-tasks";
let dind_cache_volume = "term-challenge-cache";
let evals_volume = "term-challenge-evals";
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);
Comment on lines +747 to +750
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 17, 2026

Choose a reason for hiding this comment

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

🛠️ 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.


for vol_name in [tasks_volume, dind_cache_volume, evals_volume] {
for vol_name in [&tasks_volume, &dind_cache_volume, &evals_volume] {
let vol_opts = CreateVolumeOptions {
name: vol_name.to_string(),
driver: "local".to_string(),
Expand Down Expand Up @@ -778,13 +779,13 @@ impl DockerClient {
tasks_volume, tasks_volume
),
// Cache volume - for downloaded datasets
format!("{}:/root/.cache/term-challenge:rw", dind_cache_volume),
format!("{}:/root/.cache/{}:rw", dind_cache_volume, challenge_slug),
format!(
"{}:/var/lib/docker/volumes/{}/_data:rw",
dind_cache_volume, dind_cache_volume
),
// Evals volume - for evaluation logs
format!("{}:/tmp/term-challenge-evals:rw", evals_volume),
format!("{}:/tmp/{}-evals:rw", evals_volume, challenge_slug),
format!(
"{}:/var/lib/docker/volumes/{}/_data:rw",
evals_volume, evals_volume
Expand Down Expand Up @@ -828,14 +829,11 @@ impl DockerClient {
env.push("PORT=8080".to_string());
// For Docker-in-Docker: use Docker volume paths on host
// The HOST_*_DIR tells the challenge how to map container paths to host paths for DinD
env.push("HOST_TASKS_DIR=/var/lib/docker/volumes/term-challenge-tasks/_data".to_string());
env.push("HOST_CACHE_DIR=/var/lib/docker/volumes/term-challenge-cache/_data".to_string());
env.push("CACHE_DIR=/root/.cache/term-challenge".to_string());
env.push(
"HOST_BENCHMARK_RESULTS_DIR=/var/lib/docker/volumes/term-challenge-evals/_data"
.to_string(),
);
env.push("BENCHMARK_RESULTS_DIR=/tmp/term-challenge-evals".to_string());
env.push(format!("HOST_TASKS_DIR=/var/lib/docker/volumes/{}/_data", tasks_volume));
env.push(format!("HOST_CACHE_DIR=/var/lib/docker/volumes/{}/_data", dind_cache_volume));
env.push(format!("CACHE_DIR=/root/.cache/{}", challenge_slug));
env.push(format!("HOST_BENCHMARK_RESULTS_DIR=/var/lib/docker/volumes/{}/_data", evals_volume));
env.push(format!("BENCHMARK_RESULTS_DIR=/tmp/{}-evals", challenge_slug));
// Pass through DEVELOPMENT_MODE for local image support
if let Ok(dev_mode) = std::env::var("DEVELOPMENT_MODE") {
env.push(format!("DEVELOPMENT_MODE={}", dev_mode));
Expand Down
17 changes: 9 additions & 8 deletions crates/challenge-orchestrator/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,18 +314,19 @@ impl ChallengeOrchestrator {
///
/// Called periodically to prevent Docker from accumulating orphaned containers.
#[deprecated(note = "Docker-based container cleanup is deprecated; prefer WASM-based challenge execution")]
pub async fn cleanup_stale_task_containers(&self) -> anyhow::Result<CleanupResult> {
pub async fn cleanup_stale_task_containers(&self, prefix: &str) -> anyhow::Result<CleanupResult> {
tracing::warn!("Docker-based container cleanup is deprecated; prefer WASM-based challenge execution");
// Clean up term-challenge task containers older than 2 hours
// Clean up task containers matching the given prefix older than 2 hours
// Exclude:
// - challenge-* (main challenge containers managed by orchestrator)
// - challenge-{prefix}* (main challenge containers managed by orchestrator)
// - platform-* (validator, watchtower)
let main_container_exclude = format!("challenge-{}", prefix);
let result = self
.docker
.cleanup_stale_containers(
"term-challenge-",
prefix,
120, // 2 hours old
&["challenge-term-challenge", "platform-"],
&[&main_container_exclude, "platform-"],
)
.await?;

Expand Down Expand Up @@ -696,7 +697,7 @@ mod tests {
let orchestrator = orchestrator_with_mock(docker.clone()).await;

let result = orchestrator
.cleanup_stale_task_containers()
.cleanup_stale_task_containers("my-challenge-")
.await
.expect("cleanup ok");
assert_eq!(result.total_found, 3);
Expand All @@ -706,10 +707,10 @@ mod tests {
let calls = docker.cleanup_calls();
assert_eq!(calls.len(), 1);
let (prefix, max_age, excludes) = &calls[0];
assert_eq!(prefix, "term-challenge-");
assert_eq!(prefix, "my-challenge-");
assert_eq!(*max_age, 120);
let expected: Vec<String> = vec![
"challenge-term-challenge".to_string(),
"challenge-my-challenge-".to_string(),
"platform-".to_string(),
];
assert_eq!(excludes, &expected);
Expand Down
6 changes: 5 additions & 1 deletion crates/p2p-consensus/src/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use crate::messages::{MerkleNode, MerkleProof, SequenceNumber};
use parking_lot::RwLock;
use platform_core::{hash_data, ChallengeId, Hotkey, SignedMessage};
use platform_core::{hash_data, ChallengeId, Hotkey, SignedMessage, WasmModuleMetadata};
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use std::collections::HashMap;
Expand Down Expand Up @@ -80,6 +80,9 @@ pub struct ChallengeConfig {
pub creator: Hotkey,
/// Creation timestamp
pub created_at: i64,
/// WASM module metadata
#[serde(default)]
pub wasm_module_metadata: Option<WasmModuleMetadata>,
}

/// Weight votes for epoch finalization
Expand Down Expand Up @@ -797,6 +800,7 @@ mod tests {
is_active: true,
creator: Hotkey([0u8; 32]),
created_at: chrono::Utc::now().timestamp_millis(),
wasm_module_metadata: None,
};

let id = config.id;
Expand Down
3 changes: 3 additions & 0 deletions tests/blockchain_state_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ fn test_state_hash_unique_per_modification() {
is_active: true,
creator: Hotkey([0u8; 32]),
created_at: chrono::Utc::now().timestamp_millis(),
wasm_module_metadata: None,
};
state.add_challenge(config);
assert!(
Expand Down Expand Up @@ -416,6 +417,7 @@ fn test_full_state_lifecycle_with_block_linking() {
is_active: true,
creator: sudo_keypair.hotkey(),
created_at: chrono::Utc::now().timestamp_millis(),
wasm_module_metadata: None,
};
state.add_challenge(challenge);
assert_eq!(state.challenges.len(), 1);
Expand Down Expand Up @@ -572,6 +574,7 @@ fn test_challenge_removal_changes_hash() {
is_active: true,
creator: Hotkey([0u8; 32]),
created_at: chrono::Utc::now().timestamp_millis(),
wasm_module_metadata: None,
};

state.add_challenge(config);
Expand Down