Skip to content

feat(platform): add LLM inference, review consensus, enhanced storage, and WASM challenge updates - #51

Merged
echobt merged 10 commits into
mainfrom
feat/llm-inference-review-consensus-wasm
Feb 18, 2026
Merged

feat(platform): add LLM inference, review consensus, enhanced storage, and WASM challenge updates#51
echobt merged 10 commits into
mainfrom
feat/llm-inference-review-consensus-wasm

Conversation

@echobt

@echobt echobt commented Feb 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds LLM inference host functions, P2P review consensus for validator selection, enhanced blockchain storage queries, and updates the term-challenge WASM modules to support the new submission flow with SWE-bench datasets, decay parameters, and challenge-defined RPC routes.

Changes

LLM Inference (crates/challenge-sdk-wasm, crates/wasm-runtime-interface)

  • Add platform_llm WASM import module with llm_chat_completion and llm_is_available host functions
  • Add LlmRequest/LlmResponse/LlmMessage types in new llm_types.rs (no_std compatible)
  • Implement host-side LLM inference via https://llm.chutes.ai/v1/chat/completions (OpenAI-compatible), reading CHUTES_API_KEY from environment
  • Wire up LLM host functions in wasm_executor.rs

P2P Review Consensus (crates/p2p-consensus)

  • Add ReviewAssignment, ReviewDecline, ReviewResult P2P message types with ReviewType enum (LLM/AST)
  • Add deterministic validator selection for review (3 LLM + 3 AST reviewers) using submission hash + epoch as seed
  • Add review tracking state (pending_reviews, review_results) in StateManager
  • Handle review decline with automatic replacement validator selection

Enhanced Blockchain Storage (crates/storage)

  • Add query_by_prefix() for key-prefix scanning on challenge data
  • Add get_at_block() for block-height-based data retrieval
  • Add list_keys() for enumerating all keys in a challenge namespace

Term Challenge WASM Updates (challenges/term-challenge-wasm, challenges/term-challenge)

  • Update Submission type: add signature, epoch, package_zip, basilica_instance, executor_url, executor_token, task_results
  • Update TaskDefinition for SWE-bench: add repo, base_commit, remove Docker fields
  • Add DecayParams with 72h grace period, 24h half-life, 0.0 min multiplier
  • Add dataset.rs module for SWE-bench dataset selection and consensus-based task picking
  • Add routes.rs defining challenge RPC routes (leaderboard, submissions, dataset, decay, stats)
  • Update validate() to enforce epoch rate limiting (1 per 3 epochs), signature verification, and Basilica metadata checks
  • Update evaluate() to use LLM judge scoring via host functions
  • Update scoring.rs with new decay parameters and task-result-based scoring
  • Remove legacy evaluation.rs

Validator Node (bins/validator-node)

  • Add submission handling: signature verification, rate limiting, Basilica health check, P2P relay, 50% approval threshold, review assignment

Breaking Changes

  • Challenge WASM ABI now expects new submission fields (signature, epoch, package_zip, basilica_instance, etc.)
  • TaskDefinition schema changed (SWE-bench fields replace Docker fields)
  • New platform_llm host module required for WASM challenges using LLM inference

Summary by CodeRabbit

Release Notes

  • New Features

    • Comprehensive review system with validator assignment, result tracking, and decline handling
    • LLM-powered evaluation judging with rate limiting and availability checks
    • Dataset-based task configuration and storage management
    • Enhanced storage queries with prefix search and historical block-level retrieval
  • Bug Fixes

    • Enforced size limits on message deserialization and evaluation output
    • Added rate limiting for LLM chat completion requests
  • Refactor

    • Challenge system architecture restructured; version bumped to 4.0.0

…ime-interface

- Add llm_types.rs with LlmRequest, LlmMessage, LlmResponse, LlmUsage types (no_std)
- Add platform_llm extern block with llm_chat_completion and llm_is_available host functions
- Add LlmPolicy, LlmState, LlmHostFunctions, LlmHostStatus to wasm-runtime-interface
- Integrate LlmState into RuntimeState and LlmPolicy into InstanceConfig
- Register LlmHostFunctions in the WASM linker during instantiation
- Add LLM host functions (challenge-sdk-wasm + wasm-runtime-interface):
  llm_types.rs, platform_llm extern block, LlmPolicy, LlmState, LlmHostFunctions
- Add review assignment P2P messages: ReviewType, ReviewAssignmentMessage,
  ReviewDeclineMessage, ReviewResultMessage
- Add deterministic review validator selection via SHA256 seed
- Add ReviewRecord/ReviewResultEntry tracking in ChainState
- Add DynamicStorage: query_by_prefix(), get_at_block(), list_keys()
- Add LlmPolicy/chutes_api_key to validator-node wasm_executor
- Update challenge copies (term-challenge, term-challenge-wasm):
  new types, scoring with decay, dataset/routes/tasks modules
- Delete legacy evaluation.rs from term-challenge
@coderabbitai

coderabbitai Bot commented Feb 18, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR introduces an LLM integration subsystem for WASM challenges, a distributed review framework with three new P2P message types for submission validation, and a comprehensive redesign of the term-challenge with dataset management, epoch-based rate limiting, and LLM-assisted scoring.

Changes

Cohort / File(s) Summary
LLM Host Integration
crates/wasm-runtime-interface/src/llm.rs, crates/challenge-sdk-wasm/src/llm_types.rs, crates/challenge-sdk-wasm/src/host_functions.rs
Adds LLM host functions llm_chat_completion and llm_is_available with policy-based access control, rate limiting, and API key management. Introduces LlmRequest, LlmResponse, LlmMessage, and LlmUsage types for chat-based LLM interactions.
WASM Runtime Integration
crates/wasm-runtime-interface/src/runtime.rs, crates/wasm-runtime-interface/src/lib.rs
Wires LLM state and policy into the WASM runtime initialization; registers LLM host functions in the linker; exposes LLM types at crate root via re-exports.
Review & Consensus System
crates/p2p-consensus/src/messages.rs, crates/p2p-consensus/src/state.rs, crates/p2p-consensus/src/consensus.rs, crates/p2p-consensus/src/network.rs
Introduces three new P2P message variants (ReviewAssignment, ReviewDecline, ReviewResult) with supporting types (ReviewType, ReviewRecord, ReviewResultEntry). Adds review state tracking, validator selection, and message size enforcement to consensus layer.
Term Challenge WASM Redesign
challenges/term-challenge-wasm/src/lib.rs, challenges/term-challenge-wasm/src/types.rs, challenges/term-challenge-wasm/src/tasks.rs, challenges/term-challenge-wasm/src/scoring.rs
Major overhaul introducing dataset management, epoch-based submission rate limiting, LLM-assisted scoring, and validation logic. Adds storage-backed task configuration, per-difficulty statistics, and comprehensive deserialization bounds. Version bumped to 4.0.0.
Term Challenge Non-WASM Sync
challenges/term-challenge/src/lib.rs, challenges/term-challenge/src/types.rs, challenges/term-challenge/src/scoring.rs, challenges/term-challenge/src/tasks.rs, challenges/term-challenge/src/evaluation.rs
Parallel updates to non-WASM term challenge; removes legacy evaluation module, introduces DecayParams and DatasetSelection types, updates scoring model to aggregate-based approach, and synchronizes public API surface with WASM version. Version bumped to 4.0.0.
Validator Node & WASM Executor
bins/validator-node/src/main.rs, bins/validator-node/src/wasm_executor.rs
Adds logging for new review P2P messages; integrates chutes_api_key into WasmExecutorConfig; enforces deserialization bounds via MAX_EVALUATION_OUTPUT_SIZE (64 MiB) and threads LLM policy through WASM execution paths.
Storage Extensions
crates/storage/src/dynamic.rs
Adds prefix-based query, historical block retrieval, and key enumeration methods to both DynamicStorage and ChallengeStorage for flexible data access patterns.
Challenge SDK Updates
crates/challenge-sdk-wasm/src/lib.rs
Re-exports LLM types (LlmMessage, LlmRequest, LlmResponse, LlmUsage) from new llm_types module at crate root.

Sequence Diagram(s)

sequenceDiagram
    actor Val as Validator
    participant WSR as WASM Runtime
    participant Chall as Challenge
    participant Stor as Storage
    participant LLM as LLM Service
    participant Net as P2P Network

    Val->>WSR: Execute Challenge (with llm_policy)
    WSR->>Chall: Instantiate with InstanceConfig (llm_policy)
    Chall->>Stor: Load active dataset
    Stor-->>Chall: TaskDefinitions
    
    Chall->>Chall: validate(submission, params)
    Chall->>Chall: Check epoch rate limit
    
    Chall->>Chall: evaluate(submission, tasks)
    Chall->>Chall: Deserialize with size bounds
    Chall->>Chall: Calculate aggregate score
    
    alt LLM Judging Enabled
        Chall->>LLM: host_llm_chat_completion(request)
        LLM-->>Chall: LlmResponse (score adjustment)
        Chall->>Chall: Clamp adjusted score
    end
    
    Chall->>Stor: set_last_submission_epoch
    Stor-->>Chall: Stored
    
    WSR-->>Val: EvaluationOutput
    
    Val->>Net: Broadcast submission result
    Net->>Net: select_review_validators(submission_id, epoch)
    Net->>Net: Create ReviewAssignment messages
    Net->>Val: ReviewAssignment (to LLM & AST reviewers)
Loading
sequenceDiagram
    actor Rev as Reviewer Validator
    participant Net as P2P Network
    participant Con as Consensus Engine
    participant Stor as Review State
    
    Net->>Rev: Receive ReviewAssignment
    Rev->>Rev: Validate signature & fields
    
    alt Accept Review
        Rev->>Rev: Perform review (LLM or AST)
        Rev->>Net: Send ReviewResult (score, details)
        Net->>Con: Process ReviewResult
        Con->>Stor: add_review_result(submission_id, validator, score)
        Stor-->>Con: Updated review_assignments
    else Decline Review
        Rev->>Net: Send ReviewDecline (reason)
        Net->>Con: Process ReviewDecline
    end
    
    Con->>Con: Query review_status(submission_id)
    Con-->>Net: Review metrics for consensus
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 Hops through datasets, epoch by epoch,
LLM judges whisper from the cloud above,
Review validators chosen by SHA-256 dance,
Submission bounds checked, scores clamped with care,
A challenge reborn, version four awaits!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.72% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the major features added: LLM inference, review consensus, enhanced storage, and WASM challenge updates. It reflects the primary multi-faceted scope of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/llm-inference-review-consensus-wasm

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- LlmPolicy: redact api_key from Debug output, skip from serialization
  to prevent credential leakage to logs/storage/blockchain
- LlmPolicy: enforce allowed_models check before making LLM API calls
- llm.rs: use bounded bincode deserialization for ChatRequest (4MB limit)
- state.rs: validate review scores are finite and in [0.0, 1.0] range
  in add_review_result before storing
- state.rs: add size check in from_bytes (256MB limit)
- messages.rs: add size check in P2PMessage::from_bytes (16MB limit)
- tasks.rs: replace unbounded bincode::deserialize with size-limited
  options (8MB) for dataset storage reads (both challenge variants)
- lib.rs: use bounded bincode options for DatasetSelection in configure()
- types.rs: custom Debug impl for Submission that redacts executor_token
  (both challenge variants)
- wasm_executor.rs: use bounded bincode deserialization for
  EvaluationOutput (64MB limit)
- Delete routes.rs (both crates): get_route_definitions() never called
- Delete dataset.rs (both crates): all 4 functions never called
- Remove scoring::apply_decay() from both crates: never called
- Remove tasks::get_dataset_history() from both crates: never called
- Remove RouteDefinition struct from both types.rs: only used by dead routes.rs
- Remove mod routes and mod dataset declarations from both lib.rs
- Remove unused DecayParams import from scoring.rs

Kept as intentional public API:
- challenge-sdk-wasm LLM host functions and types (SDK surface)
- storage::DynamicStorage query/list/get_at_block methods
- p2p-consensus::ChainState::get_review_status
- AggregateScore::total_tasks() (called by format_summary)
- Remove duplicate 'Signed Message Wrapper' section headers in messages.rs
- Fix silent error swallowing in dynamic.rs query_by_prefix (propagate
  serialization errors instead of unwrap_or_default)
- Extract LLM_REQUEST_TIMEOUT_SECS constant in llm.rs
- Extract RESPONSE_BUF_SMALL/MEDIUM/LARGE constants in host_functions.rs
- Extract MAX_DATASET_HISTORY constant in both tasks.rs files

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
bins/validator-node/src/main.rs (1)

373-381: ⚠️ Potential issue | 🟠 Major

Add CLI configuration for chutes_api_key to enable LLM inference.

The chutes_api_key is hardcoded to None in the WasmExecutorConfig initialization (line 380) with no way to supply a value from CLI or environment. While the infrastructure to use the key exists in wasm_executor.rs and llm.rs, the validator node has no Args field to accept it. Add a field to the Args struct following the established pattern:

#[arg(long, env = "CHUTES_API_KEY")]
chutes_api_key: Option<String>,

Then wire it through to the WasmExecutorConfig initialization to enable LLM functionality when the key is provided.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bins/validator-node/src/main.rs` around lines 373 - 381, Add a new CLI arg to
the Args struct and thread it into the WasmExecutorConfig so chutes_api_key is
populated from CLI/env: add a field to Args with the attribute #[arg(long, env =
"CHUTES_API_KEY")] and type Option<String> (name it chutes_api_key), then pass
that args.chutes_api_key into the WasmExecutorConfig when constructing
WasmChallengeExecutor::new so the chutes_api_key field is no longer hardcoded to
None and LLM inference can be enabled when provided.
bins/validator-node/src/wasm_executor.rs (1)

435-437: ⚠️ Potential issue | 🔴 Critical

execute_get_tasks and execute_configure use ephemeral storage — configure() writes are lost, get_tasks() always returns empty.

Both methods create a fresh InMemoryStorageBackend::new() instead of using Arc::clone(&self.config.storage_backend). This means:

  • configure() calls tasks::store_dataset() → writes to ephemeral storage → instance is dropped → data is lost.
  • get_tasks() calls tasks::get_active_dataset() → reads from empty ephemeral storage → always returns None.

In contrast, execute_evaluation_with_sandbox correctly uses the shared storage backend. These two methods should do the same.

🐛 Proposed fix: use shared storage backend
     // In execute_get_tasks:
     let instance_config = InstanceConfig {
         ...
-        storage_host_config: StorageHostConfig::default(),
-        storage_backend: Arc::new(InMemoryStorageBackend::new()),
+        storage_host_config: StorageHostConfig {
+            allow_direct_writes: true,
+            require_consensus: false,
+            ..self.config.storage_host_config.clone()
+        },
+        storage_backend: Arc::clone(&self.config.storage_backend),
         ...
     };
     // In execute_configure:
     let instance_config = InstanceConfig {
         ...
-        storage_host_config: StorageHostConfig::default(),
-        storage_backend: Arc::new(InMemoryStorageBackend::new()),
+        storage_host_config: StorageHostConfig {
+            allow_direct_writes: true,
+            require_consensus: false,
+            ..self.config.storage_host_config.clone()
+        },
+        storage_backend: Arc::clone(&self.config.storage_backend),
         ...
     };

Also applies to: 518-520

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bins/validator-node/src/wasm_executor.rs` around lines 435 - 437, The bug is
that execute_get_tasks and execute_configure instantiate ephemeral storage with
InMemoryStorageBackend::new() so writes are lost; change both methods to use the
shared storage backend like execute_evaluation_with_sandbox by replacing new()
with Arc::clone(&self.config.storage_backend) (or otherwise reusing
self.config.storage_backend) so tasks::store_dataset and
tasks::get_active_dataset operate on the persistent Arc-backed storage; ensure
any places that currently construct StorageHostConfig or pass storage_backend in
these methods use the same Arc clone instead of creating a fresh
InMemoryStorageBackend.
crates/wasm-runtime-interface/src/runtime.rs (1)

246-264: 🛠️ Refactor suggestion | 🟠 Major

Add reset_llm_counters method to RuntimeState and reset_counters method to LlmState.

LlmState tracks requests_made but lacks a reset mechanism. All other state types (NetworkState, StorageState, ExecState, ContainerState, DataState) provide both a reset_counters() method on the state struct and a corresponding reset_*_counters() method on RuntimeState. Without this for LLM state, request counts will accumulate across evaluations when instances are reused.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/wasm-runtime-interface/src/runtime.rs` around lines 246 - 264, Add a
reset path for LLM counters: implement a reset_counters(&mut self) method on
LlmState that sets its requests_made (and any other counter fields) back to
zero, and add a corresponding reset_llm_counters(&mut self) method on
RuntimeState that calls self.llm_state.reset_counters(). Update any naming to
match existing counterparts (e.g., reset_network_counters,
reset_storage_counters) so the LLM counters are cleared when RuntimeState resets
are invoked.
crates/challenge-sdk-wasm/src/host_functions.rs (1)

82-95: ⚠️ Potential issue | 🟡 Minor

FFI inconsistency: storage_get does not pass buffer length to host.

Unlike http_get, terminal_exec, and llm_chat_completion — which all pass both pointer and length — storage_get passes only the pointer. While the host-side implementation includes bounds checking via write_wasm_memory, the lack of an explicit capacity parameter is inconsistent with other FFI functions and makes the API less safe by design. If a stored value exceeds RESPONSE_BUF_MEDIUM (64 KB), the host will fail to write and return an error rather than truncating gracefully.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/challenge-sdk-wasm/src/host_functions.rs` around lines 82 - 95,
host_storage_get currently calls storage_get with the buffer pointer only, which
is inconsistent with other FFI calls and unsafe if the host tries to write more
than RESPONSE_BUF_MEDIUM; change the call in host_storage_get to pass the buffer
length as the second argument (e.g., value_buf.as_mut_ptr() as i32,
value_buf.len() as i32) and update the host/storage_get ABI accordingly so the
host can bounds-check/truncate safely; also ensure host_storage_get still checks
the returned status and truncates value_buf to status as usize (and treats
status > buffer len as an error or truncation indicator) so behavior matches
http_get/terminal_exec/llm_chat_completion patterns.
🧹 Nitpick comments (12)
crates/storage/src/dynamic.rs (2)

478-538: No unit tests for the three new public methods

query_by_prefix, get_at_block, and list_keys are all untested. At minimum, cover:

  • query_by_prefix: verify prefix filtering, validator-key exclusion, and empty-prefix returns all keys
  • get_at_block: once the version/last_block semantic is fixed, verify the boundary (at-block returns value, after-block returns None)
  • list_keys: verify validator-scoped keys are excluded, expired entries are not returned
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/storage/src/dynamic.rs` around lines 478 - 538, Add unit tests for the
three new public methods: write tests for query_by_prefix to insert several
entries (some with validator set and some without) and assert that filtering by
a non-empty prefix returns only keys without validators that start with the
prefix, that using an empty prefix returns all non-validator keys, and that
validator-scoped keys are excluded; write tests for get_at_block to insert an
entry with a known version/last_block and assert that calling
get_at_block(challenge_id, key, block) returns the value when block >= stored
version and returns None when block < stored version (include the boundary
case); write tests for list_keys to insert multiple keys including
validator-scoped keys and expired/removed entries and assert that
list_keys(challenge_id) returns only currently present non-validator keys; use
the existing helpers/setup used in other tests to create StorageKey/entries and
call Storage::query_by_prefix, Storage::get_at_block, and Storage::list_keys to
validate behavior.

529-538: list_keys: fully deserializes all entry values only to discard them

scan_namespace deserializes every StorageEntry (including potentially large value payloads) for the entire namespace, but list_keys discards all of them — only the key strings are retained. For namespaces with many or large entries this is needlessly expensive.

A key-only iteration over raw sled bytes, only parsing the key string from each row, would avoid the deserialization cost entirely:

♻️ Proposed refactor — key-only scan
 pub fn list_keys(&self, challenge_id: &ChallengeId) -> Result<Vec<String>> {
-    let namespace = challenge_id.0.to_string();
-    let entries = self.scan_namespace(&namespace)?;
-
-    Ok(entries
-        .into_iter()
-        .filter(|(k, _)| k.validator.is_none())
-        .map(|(k, _)| k.key)
-        .collect())
+    let namespace = challenge_id.0.to_string();
+    let prefix = StorageKey::namespace_prefix(&namespace);
+    let mut keys = Vec::new();
+    for item in self.tree.scan_prefix(&prefix) {
+        let (key_bytes, raw) = item.map_err(|e| MiniChainError::Storage(e.to_string()))?;
+        if let Some(sk) = self.parse_key(&key_bytes) {
+            if sk.validator.is_some() { continue; }
+            // Only deserialize to check expiry
+            if let Ok(entry) = bincode::deserialize::<StorageEntry>(&raw) {
+                if entry.is_expired() { continue; }
+            }
+            keys.push(sk.key);
+        }
+    }
+    Ok(keys)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/storage/src/dynamic.rs` around lines 529 - 538, list_keys currently
calls scan_namespace which fully deserializes StorageEntry values (expensive);
instead implement a key-only iterator and use it in list_keys. Add a new helper
(e.g. scan_namespace_keys) that iterates the sled namespace raw rows, decodes
only the StorageKey (or the key prefix) from each row without deserializing
StorageEntry/value, and yields (StorageKey, raw_bytes) or just StorageKey; then
change list_keys to call this new scan_namespace_keys, filter where
StorageKey.validator.is_none(), map to StorageKey.key (String) and collect. Keep
existing scan_namespace and StorageEntry intact for callers that need values.
Ensure symbols referenced: list_keys, scan_namespace, scan_namespace_keys (new),
StorageEntry, StorageKey, ChallengeId.
challenges/term-challenge/src/types.rs (1)

83-95: LlmJudgeResponse::score has no defined bounds.

If score is expected to be in [0.0, 1.0], consider documenting the contract or adding a validation method, since downstream code (to_weight) clamps to this range and mismatched expectations could lead to subtle scoring bugs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/types.rs` around lines 83 - 95, The
LlmJudgeResponse::score field lacks bounds; add a clear contract and enforcement
by documenting that score must be in [0.0, 1.0] and implementing a validation
function (e.g., LlmJudgeResponse::validate or TryFrom) that checks the score
range and returns an error (or clamps) before downstream use (see to_weight
where clamping already occurs); update callers to run validation or use the
validated constructor to ensure no mismatched expectations at runtime.
challenges/term-challenge/src/scoring.rs (1)

1-111: Near-identical duplicate of challenges/term-challenge-wasm/src/scoring.rs.

This file is effectively a copy of the WASM variant's scoring module. Consider extracting the shared AggregateScore, calculate_aggregate, to_weight, and format_summary into a common crate (or shared module) that both term-challenge and term-challenge-wasm depend on, to avoid divergence over time.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/scoring.rs` around lines 1 - 111, This file
duplicates scoring logic found in the WASM variant; extract the shared types and
functions (AggregateScore, calculate_aggregate, to_weight, format_summary and
the DifficultyStats helper shape) into a new common crate or shared module and
have both term-challenge and term-challenge-wasm depend on it; refactor this
file to remove the local definitions and re-export or use the common crate's
symbols, update imports/uses of Difficulty, TaskDefinition, TaskResult to
reference the shared module, and run cargo to fix any path or visibility issues
so both targets compile against the single authoritative implementation.
crates/p2p-consensus/src/consensus.rs (1)

1221-1230: Unbounded loop when validator count is small relative to index collisions.

While the Knuth LCG has a full period of 2^64, the modulo reduction to validators.len() can cause many consecutive collisions when the validator count is small. Consider adding an iteration cap as a safety guard to prevent excessive spinning.

Proposed safeguard
+            let max_iterations = validators.len() * 20; // generous bound
+            let mut iterations = 0;
             while selected.len() < 3 && selected.len() < validators.len() {
                 state = state
                     .wrapping_mul(6364136223846793005)
                     .wrapping_add(1442695040888963407);
                 let idx = (state >> 33) as usize % validators.len();
                 let candidate = &validators[idx];
                 if !selected.contains(candidate) {
                     selected.push(candidate.clone());
                 }
+                iterations += 1;
+                if iterations >= max_iterations {
+                    break;
+                }
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/p2p-consensus/src/consensus.rs` around lines 1221 - 1230, The loop
using Knuth LCG (variables state, validators, selected, idx) can spin
indefinitely when many modulo collisions occur for small validator sets—add a
guarded iteration cap (e.g., max_iters =
validators.len().saturating_mul(10).max(1000)) inside the while loop and
increment a counter each iteration; if the cap is reached, break and
deterministically fill the remaining slots by iterating through validators and
adding the first non-selected entries until selected.len() meets the target,
ensuring the selection logic in this block cannot loop unbounded.
bins/validator-node/src/wasm_executor.rs (1)

195-198: Extract repeated llm_policy construction into a helper.

The same match &self.config.chutes_api_key { ... } pattern appears in 4 places. A small private method would reduce duplication and ensure consistency.

♻️ Proposed helper method
 impl WasmChallengeExecutor {
+    fn llm_policy(&self) -> LlmPolicy {
+        match &self.config.chutes_api_key {
+            Some(key) => LlmPolicy::with_api_key(key.clone()),
+            None => LlmPolicy::default(),
+        }
+    }
+
     // ... then in each InstanceConfig:
-    llm_policy: match &self.config.chutes_api_key {
-        Some(key) => LlmPolicy::with_api_key(key.clone()),
-        None => LlmPolicy::default(),
-    },
+    llm_policy: self.llm_policy(),

Also applies to: 326-329, 440-443, 523-526

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bins/validator-node/src/wasm_executor.rs` around lines 195 - 198, Several
places construct llm_policy with the same match on self.config.chutes_api_key;
extract that logic into a private helper like a method on the current impl
(e.g., fn build_llm_policy(&self) -> LlmPolicy) that returns
LlmPolicy::with_api_key(key.clone()) when Some and LlmPolicy::default() when
None, then replace the four inline matches (including the one constructing
llm_policy) with calls to build_llm_policy(); update all usages referenced by
llm_policy, self.config.chutes_api_key, LlmPolicy::with_api_key, and
LlmPolicy::default to call the new helper to remove duplication and ensure
consistency.
challenges/term-challenge/src/lib.rs (1)

29-48: Same bincode::serialize vs bounded-options asymmetry as in tasks.rs.

bincode_options_submission(), bincode_options_params(), and bincode_options_llm() are used for deserialization, but bincode::serialize is used for serialization (Line 103 in try_llm_judge). While currently compatible due to .with_fixint_encoding(), prefer using the same options object for both serialization and deserialization to prevent future encoding mismatches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/lib.rs` around lines 29 - 48, The three helper
functions bincode_options_submission(), bincode_options_params(), and
bincode_options_llm() create bounded Options used for deserialization but the
code still calls bincode::serialize for serialization (e.g., in try_llm_judge),
creating an encode/decode asymmetry; update the serialization sites to use the
same Options objects used for deserialization (i.e., obtain the appropriate
Options via the matching helper and call the serialization routine that accepts
Options or otherwise use the helper Options to produce the serialized bytes),
ensuring serialize and deserialize use identical bincode options for submission,
params, and llm paths.
challenges/term-challenge/src/tasks.rs (2)

27-27: bincode::serialize vs bincode_options_dataset().deserialize — encoding compatibility check.

store_dataset and append_dataset_history serialize with bincode::serialize (legacy fixint config) but deserialize with bincode_options_dataset() (DefaultOptions::new().with_fixint_encoding()). In bincode 1.3 these happen to use the same integer encoding, so they are compatible. However, this asymmetry is a maintenance trap — if someone replaces bincode::serialize with bincode_options_dataset().serialize, or vice-versa, it could silently break. Consider using bincode_options_dataset().serialize(...) consistently for both paths.

Use consistent bincode options for serialization
 pub fn store_dataset(selection: &DatasetSelection) -> bool {
-    let data = match bincode::serialize(selection) {
+    let data = match bincode_options_dataset().serialize(selection) {
         Ok(d) => d,
         Err(_) => return false,
     };
 fn append_dataset_history(selection: &DatasetSelection) -> bool {
     // ... deserialization ...
-    let data = match bincode::serialize(&history) {
+    let data = match bincode_options_dataset().serialize(&history) {
         Ok(d) => d,
         Err(_) => return false,
     };

Also applies to: 53-53

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/tasks.rs` at line 27, The code uses
bincode::serialize to encode selection but deserializes with
bincode_options_dataset().deserialize, creating an asymmetry; update the
serialization in store_dataset and append_dataset_history to use
bincode_options_dataset().serialize(...) (the same
DefaultOptions::new().with_fixint_encoding() used by bincode_options_dataset())
so encoding/decoding use identical options and avoid future breakage — change
both occurrences where bincode::serialize is called to use
bincode_options_dataset().serialize and keep the existing
bincode_options_dataset().deserialize as-is.

1-58: Near-identical duplication with challenges/term-challenge-wasm/src/tasks.rs.

This entire file (constants, bincode_options_dataset, get_active_dataset, store_dataset, append_dataset_history) is duplicated verbatim in challenges/term-challenge-wasm/src/tasks.rs. Consider extracting the shared logic into a common crate (or reusing types from term-challenge-wasm) to avoid drift between the two copies.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/tasks.rs` around lines 1 - 58, This file
duplicates constants and functions (ACTIVE_DATASET_KEY, DATASET_HISTORY_KEY,
MAX_DATASET_SIZE, MAX_DATASET_HISTORY, bincode_options_dataset,
get_active_dataset, store_dataset, append_dataset_history) that also exist in
challenges/term-challenge-wasm/src/tasks.rs; extract these shared definitions
into a small common crate (e.g., term-challenge-core) and move the constants,
the bincode_options_dataset helper, and the three public functions
(get_active_dataset, store_dataset, append_dataset_history) plus any shared
types into that crate; then update both crates to depend on the new crate and
replace the local implementations with re-exports/uses of the shared symbols to
eliminate duplication and keep a single source of truth.
crates/p2p-consensus/src/state.rs (1)

718-724: No bounds on the number of ReviewRecord entries per submission.

assign_review pushes a new ReviewRecord into the Vec without checking its length. If a submission is repeatedly assigned reviews (e.g., due to repeated declines and replacements), the vector grows without limit, which inflates the serialized state size. Consider capping the number of records per submission.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/p2p-consensus/src/state.rs` around lines 718 - 724, assign_review
currently appends ReviewRecord to review_assignments[submission_id] unbounded;
define a constant like MAX_REVIEWS_PER_SUBMISSION and ensure assign_review
enforces it by capping the Vec for that submission—after pushing the new
ReviewRecord (or before inserting) check the vector's length and if it exceeds
MAX remove the oldest entries (e.g., drop from the front) so the stored history
stays bounded; update any related logic that assumes unlimited history and
reference symbols review_assignments, assign_review, ReviewRecord, and
increment_sequence when making the change.
challenges/term-challenge-wasm/src/lib.rs (1)

159-171: LLM judge silently overrides result.score and result.passed — consider recording the original score.

When the LLM judge returns a score, the original result.score is replaced (line 165) and passed may be flipped to false (line 168). If there's a need to audit or debug scoring discrepancies, consider preserving the original test score alongside the LLM-adjusted score. This is optional and depends on how much observability is needed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge-wasm/src/lib.rs` around lines 159 - 171, The LLM
judge code (params.llm_judge_url loop calling Self::try_llm_judge) overwrites
result.score and result.passed silently; preserve the original by recording it
before assigning the LLM value (e.g., add an Option<f32> field like
original_score or llm_adjusted_score to the Result struct and set
result.original_score = Some(result.score) or result.llm_adjusted_score =
Some(llm_score) before mutating result.score and result.passed), update any
serialization/constructors and tests that create Result instances, and ensure
the new field is populated only when try_llm_judge returns Some so auditing can
compare pre- and post-LLM scores for task.name.
crates/p2p-consensus/src/messages.rs (1)

653-666: ReviewDeclineMessage.reason is unbounded — consider capping its length.

While MAX_P2P_MESSAGE_SIZE (16 MiB) implicitly bounds the overall message, a malicious validator could fill the reason field with megabytes of data. A modest per-field cap (e.g., 1 KiB) at the processing layer would reduce waste in storage and network bandwidth.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/p2p-consensus/src/messages.rs` around lines 653 - 666, Add a modest
per-field cap for ReviewDeclineMessage.reason (e.g., const MAX_REASON_LEN: usize
= 1024) and enforce it at message intake/validation: implement a short
validation check (e.g., impl ReviewDeclineMessage::validate(&self) -> Result<()>
or inline in the handler that deserializes/accepts messages) that returns an
error when reason.len() > MAX_REASON_LEN (or when bytes length > MAX_REASON_LEN
for UTF-8 safety). Make sure the check runs before further processing/storage
and return a clear validation error so oversized messages are rejected early.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@challenges/term-challenge/src/lib.rs`:
- Around line 180-228: validate() currently enforces rate limits by reading
get_last_submission_epoch() but doesn’t advance them, allowing repeated
/validate calls to bypass limits; modify validate() so that after it
deserializes Submission, checks host_consensus_get_epoch() and the rate-limit
condition succeed, it calls set_last_submission_epoch(&submission.miner_hotkey,
current as u64) (or equivalent API) to record the epoch before returning true;
ensure you use the same epoch value logic as evaluate() and keep
get_last_submission_epoch(), host_consensus_get_epoch(), and Submission
references consistent so subsequent validate() calls cannot be used to bypass
rate limiting.

In `@challenges/term-challenge/src/tasks.rs`:
- Around line 18-24: The code currently deserializes ACTIVE_DATASET_KEY as
Vec<TaskDefinition> but store_dataset writes a DatasetSelection; fix by
deserializing a DatasetSelection (use bincode_options_dataset().deserialize to
get a DatasetSelection) and return its .tasks (i.e., change get_active_dataset
to return selection.tasks wrapped in Some or None accordingly), and apply the
same change to the other similar function handling ACTIVE_DATASET_KEY (the block
noted at lines 26-36) so both read the full DatasetSelection and extract .tasks
instead of relying on trailing-bytes behavior.

In `@challenges/term-challenge/src/types.rs`:
- Around line 97-112: DecayParams currently allows half_life_hours == 0 which
will break decay math; change the half_life_hours field to a non-zero type
(std::num::NonZeroU64) and update the Default impl to use
NonZeroU64::new(24).unwrap() (or add a constructor that returns Result to
validate), and update any code using DecayParams::half_life_hours to call .get()
— alternatively, if you prefer not to change the type, add a validate() or new()
constructor on DecayParams that rejects or sanitizes zero half_life_hours and
update Default to set a safe non-zero value (24) so callers and decay functions
(where 2.0_f64.powf(-elapsed / half_life)) are protected from division-by-zero.

In `@crates/p2p-consensus/src/consensus.rs`:
- Around line 1204-1244: The current closure select_n and its independent calls
(llm_reviewers = select_n(0) and ast_reviewers = select_n(8)) can return
overlapping validators; change the selection so the two groups are
deduplicated—either select six distinct validators once (e.g., sample 6 from
validators with the existing PRNG state and then split into llm_reviewers and
ast_reviewers), or modify select_n to accept an exclusion set (pass the first
group's Hotkeys to the second call to skip duplicates); ensure you still pad
with Hotkey([0u8;32]) when validators are fewer than needed and keep the same
RNG logic/state (the state derived from seed[offset..]) to preserve determinism.
- Around line 1197-1202: active_validators() returns a non-deterministic order
(HashMap) so the Vec<Hotkey> assigned to validators must be sorted before
calling select_review_validators(); modify the block that builds validators in
consensus.rs (the variable validators used by select_review_validators) to sort
the Hotkey list deterministically (e.g., by hotkey string/bytes or impl Ord) so
all nodes compute the same reviewer assignment.

In `@crates/p2p-consensus/src/state.rs`:
- Around line 726-758: add_review_result mutates state (inserting into
review.results and calling update_hash()) but does not call
increment_sequence(), so peers may not detect the change; after the successful
insertion and update_hash() in add_review_result (the block that currently
returns true), call self.increment_sequence() (matching assign_review and
add_validator_evaluation behavior) before returning true so the sequence number
is bumped and state-sync will pick up the new review result.
- Around line 726-758: The ReviewResult P2P handler (P2PMessage::ReviewResult in
main.rs) currently only logs and never invokes the state updater; update the
handler to perform cryptographic verification of the ReviewResultMessage
(validate signature against sender hotkey and ensure timestamp is within
acceptable skew to prevent replay) and, only on successful verification, call
State::add_review_result(...) with the message's submission_id, validator
hotkey, score, and details; if verification fails, reject and log appropriately.
Use the existing add_review_result function to record results and ensure you
surface signature/timestamp validation failures in logs so invalid or replayed
ReviewResultMessage instances are not silently dropped. Ensure any helper
signature verification function (or a new
verify_review_result_signature/verify_timestamp) is called from the handler
before add_review_result.

In `@crates/storage/src/dynamic.rs`:
- Around line 479-497: query_by_prefix currently loads the entire namespace via
scan_namespace and deserializes every StorageEntry, then re-serializes
entry.value and returns an unbounded Vec which can OOM; change the API and
implementation to accept an optional limit parameter (e.g., limit:
Option<usize>) and enforce it while collecting results, and add or call a
lower-level scanning helper (e.g., a new scan_namespace_raw or
scan_namespace_keys_values) that yields raw bytes or skips full deserialization
so you can filter keys by prefix without deserializing non-matching entries;
update query_by_prefix to use that raw scanner, serialize only when needed,
respect the limit, and propagate errors using the existing MiniChainError path
(referencing functions/structs query_by_prefix, scan_namespace, StorageEntry and
field value).
- Around line 505-526: get_at_block currently compares block height against the
optimistic-lock counter; add a last_block: u64 field to StorageEntry (and
initialize it in StorageEntry::new and any update() path), set last_block to the
write's block height when writing in set_with_options (use the
StorageChange/block_height available at write time), and change get_at_block to
compare e.last_block <= block instead of e.version <= block; also update any
(de)serialization or migrations that touch StorageEntry to include the new
field.

In `@crates/wasm-runtime-interface/src/llm.rs`:
- Around line 332-337: The code extracts the response content by calling
openai_resp.choices.and_then(|mut c| c.pop()) which returns the last choice
instead of the primary one; change this to take the first choice (e.g., use
.first() or .into_iter().next()) so that the primary completion (choices[0]) is
used reliably even when n > 1, updating the chain around openai_resp.choices ->
choice -> message -> content and preserving the final unwrap_or_default()
behavior.
- Around line 293-330: The code parses response_body and deserializes into
OpenAiResponse without checking http_response.status(); update the
llm_chat_completion flow to inspect http_response.status() immediately after
reading response_body (or before parsing) and if status is not success (e.g.,
!status.is_success()) map error codes to appropriate LlmHostStatus values
(return RateLimited for 429, ApiError for 4xx/5xx otherwise), log the status and
any response text or error detail using the existing warn! pattern (include
error = %err and status = %status), and return early instead of calling
serde_json::from_slice on error responses so openai_resp/choices parsing only
runs for 2xx responses.
- Around line 277-291: Replace per-call creation of reqwest::blocking::Client in
handle_chat_completion with a reusable client stored on LlmState: add a field
(e.g., client: reqwest::blocking::Client) to LlmState, initialize it once when
constructing LlmState, and use that client for the POST request (keeping
existing headers, body, and LLM_REQUEST_TIMEOUT_SECS). Also ensure the blocking
request cannot run on a Tokio async thread by wrapping the blocking HTTP call in
tokio::task::spawn_blocking (or alternatively document/validate that callers
never invoke handle_chat_completion from an async context); update references to
reqwest::blocking::Client and handle_chat_completion accordingly.

---

Outside diff comments:
In `@bins/validator-node/src/main.rs`:
- Around line 373-381: Add a new CLI arg to the Args struct and thread it into
the WasmExecutorConfig so chutes_api_key is populated from CLI/env: add a field
to Args with the attribute #[arg(long, env = "CHUTES_API_KEY")] and type
Option<String> (name it chutes_api_key), then pass that args.chutes_api_key into
the WasmExecutorConfig when constructing WasmChallengeExecutor::new so the
chutes_api_key field is no longer hardcoded to None and LLM inference can be
enabled when provided.

In `@bins/validator-node/src/wasm_executor.rs`:
- Around line 435-437: The bug is that execute_get_tasks and execute_configure
instantiate ephemeral storage with InMemoryStorageBackend::new() so writes are
lost; change both methods to use the shared storage backend like
execute_evaluation_with_sandbox by replacing new() with
Arc::clone(&self.config.storage_backend) (or otherwise reusing
self.config.storage_backend) so tasks::store_dataset and
tasks::get_active_dataset operate on the persistent Arc-backed storage; ensure
any places that currently construct StorageHostConfig or pass storage_backend in
these methods use the same Arc clone instead of creating a fresh
InMemoryStorageBackend.

In `@crates/challenge-sdk-wasm/src/host_functions.rs`:
- Around line 82-95: host_storage_get currently calls storage_get with the
buffer pointer only, which is inconsistent with other FFI calls and unsafe if
the host tries to write more than RESPONSE_BUF_MEDIUM; change the call in
host_storage_get to pass the buffer length as the second argument (e.g.,
value_buf.as_mut_ptr() as i32, value_buf.len() as i32) and update the
host/storage_get ABI accordingly so the host can bounds-check/truncate safely;
also ensure host_storage_get still checks the returned status and truncates
value_buf to status as usize (and treats status > buffer len as an error or
truncation indicator) so behavior matches
http_get/terminal_exec/llm_chat_completion patterns.

In `@crates/wasm-runtime-interface/src/runtime.rs`:
- Around line 246-264: Add a reset path for LLM counters: implement a
reset_counters(&mut self) method on LlmState that sets its requests_made (and
any other counter fields) back to zero, and add a corresponding
reset_llm_counters(&mut self) method on RuntimeState that calls
self.llm_state.reset_counters(). Update any naming to match existing
counterparts (e.g., reset_network_counters, reset_storage_counters) so the LLM
counters are cleared when RuntimeState resets are invoked.

---

Duplicate comments:
In `@challenges/term-challenge-wasm/src/tasks.rs`:
- Around line 1-58: The active dataset handling has inconsistent types and
bincode options: you store a DatasetSelection in store_dataset but
get_active_dataset attempts to deserialize a Vec<TaskDefinition>, and
serialization calls use plain bincode::serialize instead of the constrained
bincode_options_dataset(). Fix by making the stored/read type consistent (either
change get_active_dataset to return Option<DatasetSelection> and deserialize
with bincode_options_dataset(), or change store_dataset to serialize a
Vec<TaskDefinition> if that was intended), update store_dataset and
append_dataset_history to serialize using bincode_options_dataset().serialize
instead of bincode::serialize, and ensure append_dataset_history deserializes
history with bincode_options_dataset() and stores the same DatasetSelection type
under ACTIVE_DATASET_KEY and DATASET_HISTORY_KEY so types align.

---

Nitpick comments:
In `@bins/validator-node/src/wasm_executor.rs`:
- Around line 195-198: Several places construct llm_policy with the same match
on self.config.chutes_api_key; extract that logic into a private helper like a
method on the current impl (e.g., fn build_llm_policy(&self) -> LlmPolicy) that
returns LlmPolicy::with_api_key(key.clone()) when Some and LlmPolicy::default()
when None, then replace the four inline matches (including the one constructing
llm_policy) with calls to build_llm_policy(); update all usages referenced by
llm_policy, self.config.chutes_api_key, LlmPolicy::with_api_key, and
LlmPolicy::default to call the new helper to remove duplication and ensure
consistency.

In `@challenges/term-challenge-wasm/src/lib.rs`:
- Around line 159-171: The LLM judge code (params.llm_judge_url loop calling
Self::try_llm_judge) overwrites result.score and result.passed silently;
preserve the original by recording it before assigning the LLM value (e.g., add
an Option<f32> field like original_score or llm_adjusted_score to the Result
struct and set result.original_score = Some(result.score) or
result.llm_adjusted_score = Some(llm_score) before mutating result.score and
result.passed), update any serialization/constructors and tests that create
Result instances, and ensure the new field is populated only when try_llm_judge
returns Some so auditing can compare pre- and post-LLM scores for task.name.

In `@challenges/term-challenge/src/lib.rs`:
- Around line 29-48: The three helper functions bincode_options_submission(),
bincode_options_params(), and bincode_options_llm() create bounded Options used
for deserialization but the code still calls bincode::serialize for
serialization (e.g., in try_llm_judge), creating an encode/decode asymmetry;
update the serialization sites to use the same Options objects used for
deserialization (i.e., obtain the appropriate Options via the matching helper
and call the serialization routine that accepts Options or otherwise use the
helper Options to produce the serialized bytes), ensuring serialize and
deserialize use identical bincode options for submission, params, and llm paths.

In `@challenges/term-challenge/src/scoring.rs`:
- Around line 1-111: This file duplicates scoring logic found in the WASM
variant; extract the shared types and functions (AggregateScore,
calculate_aggregate, to_weight, format_summary and the DifficultyStats helper
shape) into a new common crate or shared module and have both term-challenge and
term-challenge-wasm depend on it; refactor this file to remove the local
definitions and re-export or use the common crate's symbols, update imports/uses
of Difficulty, TaskDefinition, TaskResult to reference the shared module, and
run cargo to fix any path or visibility issues so both targets compile against
the single authoritative implementation.

In `@challenges/term-challenge/src/tasks.rs`:
- Line 27: The code uses bincode::serialize to encode selection but deserializes
with bincode_options_dataset().deserialize, creating an asymmetry; update the
serialization in store_dataset and append_dataset_history to use
bincode_options_dataset().serialize(...) (the same
DefaultOptions::new().with_fixint_encoding() used by bincode_options_dataset())
so encoding/decoding use identical options and avoid future breakage — change
both occurrences where bincode::serialize is called to use
bincode_options_dataset().serialize and keep the existing
bincode_options_dataset().deserialize as-is.
- Around line 1-58: This file duplicates constants and functions
(ACTIVE_DATASET_KEY, DATASET_HISTORY_KEY, MAX_DATASET_SIZE, MAX_DATASET_HISTORY,
bincode_options_dataset, get_active_dataset, store_dataset,
append_dataset_history) that also exist in
challenges/term-challenge-wasm/src/tasks.rs; extract these shared definitions
into a small common crate (e.g., term-challenge-core) and move the constants,
the bincode_options_dataset helper, and the three public functions
(get_active_dataset, store_dataset, append_dataset_history) plus any shared
types into that crate; then update both crates to depend on the new crate and
replace the local implementations with re-exports/uses of the shared symbols to
eliminate duplication and keep a single source of truth.

In `@challenges/term-challenge/src/types.rs`:
- Around line 83-95: The LlmJudgeResponse::score field lacks bounds; add a clear
contract and enforcement by documenting that score must be in [0.0, 1.0] and
implementing a validation function (e.g., LlmJudgeResponse::validate or TryFrom)
that checks the score range and returns an error (or clamps) before downstream
use (see to_weight where clamping already occurs); update callers to run
validation or use the validated constructor to ensure no mismatched expectations
at runtime.

In `@crates/p2p-consensus/src/consensus.rs`:
- Around line 1221-1230: The loop using Knuth LCG (variables state, validators,
selected, idx) can spin indefinitely when many modulo collisions occur for small
validator sets—add a guarded iteration cap (e.g., max_iters =
validators.len().saturating_mul(10).max(1000)) inside the while loop and
increment a counter each iteration; if the cap is reached, break and
deterministically fill the remaining slots by iterating through validators and
adding the first non-selected entries until selected.len() meets the target,
ensuring the selection logic in this block cannot loop unbounded.

In `@crates/p2p-consensus/src/messages.rs`:
- Around line 653-666: Add a modest per-field cap for
ReviewDeclineMessage.reason (e.g., const MAX_REASON_LEN: usize = 1024) and
enforce it at message intake/validation: implement a short validation check
(e.g., impl ReviewDeclineMessage::validate(&self) -> Result<()> or inline in the
handler that deserializes/accepts messages) that returns an error when
reason.len() > MAX_REASON_LEN (or when bytes length > MAX_REASON_LEN for UTF-8
safety). Make sure the check runs before further processing/storage and return a
clear validation error so oversized messages are rejected early.

In `@crates/p2p-consensus/src/state.rs`:
- Around line 718-724: assign_review currently appends ReviewRecord to
review_assignments[submission_id] unbounded; define a constant like
MAX_REVIEWS_PER_SUBMISSION and ensure assign_review enforces it by capping the
Vec for that submission—after pushing the new ReviewRecord (or before inserting)
check the vector's length and if it exceeds MAX remove the oldest entries (e.g.,
drop from the front) so the stored history stays bounded; update any related
logic that assumes unlimited history and reference symbols review_assignments,
assign_review, ReviewRecord, and increment_sequence when making the change.

In `@crates/storage/src/dynamic.rs`:
- Around line 478-538: Add unit tests for the three new public methods: write
tests for query_by_prefix to insert several entries (some with validator set and
some without) and assert that filtering by a non-empty prefix returns only keys
without validators that start with the prefix, that using an empty prefix
returns all non-validator keys, and that validator-scoped keys are excluded;
write tests for get_at_block to insert an entry with a known version/last_block
and assert that calling get_at_block(challenge_id, key, block) returns the value
when block >= stored version and returns None when block < stored version
(include the boundary case); write tests for list_keys to insert multiple keys
including validator-scoped keys and expired/removed entries and assert that
list_keys(challenge_id) returns only currently present non-validator keys; use
the existing helpers/setup used in other tests to create StorageKey/entries and
call Storage::query_by_prefix, Storage::get_at_block, and Storage::list_keys to
validate behavior.
- Around line 529-538: list_keys currently calls scan_namespace which fully
deserializes StorageEntry values (expensive); instead implement a key-only
iterator and use it in list_keys. Add a new helper (e.g. scan_namespace_keys)
that iterates the sled namespace raw rows, decodes only the StorageKey (or the
key prefix) from each row without deserializing StorageEntry/value, and yields
(StorageKey, raw_bytes) or just StorageKey; then change list_keys to call this
new scan_namespace_keys, filter where StorageKey.validator.is_none(), map to
StorageKey.key (String) and collect. Keep existing scan_namespace and
StorageEntry intact for callers that need values. Ensure symbols referenced:
list_keys, scan_namespace, scan_namespace_keys (new), StorageEntry, StorageKey,
ChallengeId.

Comment on lines 180 to +228
fn validate(&self, input: EvaluationInput) -> bool {
evaluation::validate(&input)
let submission: Submission =
match bincode_options_submission().deserialize(&input.agent_data) {
Ok(s) => s,
Err(_) => return false,
};
let params: ChallengeParams = match bincode_options_params().deserialize(&input.params) {
Ok(p) => p,
Err(_) => return false,
};
if submission.agent_hash.is_empty() || submission.miner_hotkey.is_empty() {
return false;
}
if submission.signature.is_empty() {
return false;
}
if submission.package_zip.is_empty() {
return false;
}
if submission.basilica_instance.is_empty()
|| submission.executor_url.is_empty()
|| submission.executor_token.is_empty()
{
return false;
}
let current_epoch = host_consensus_get_epoch();
if current_epoch >= 0 {
if let Some(last_epoch) = get_last_submission_epoch(&submission.miner_hotkey) {
let current = current_epoch as u64;
if current < last_epoch.saturating_add(EPOCH_RATE_LIMIT) {
return false;
}
}
}
if submission.task_results.is_empty() {
return false;
}
if submission.task_results.len() > MAX_TASKS {
return false;
}
if submission.task_results.len() != params.tasks.len() {
return false;
}
for result in &submission.task_results {
if !validate_task_result(result) {
return false;
}
}
true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how validate() is called in the codebase — is it always paired with evaluate()?
rg -n 'execute_validation|\.validate\(' --type=rs -C3 -g '!target/**'

Repository: PlatformNetwork/platform-v2

Length of output: 97


🏁 Script executed:

#!/bin/bash
# Search for validate() and execute_validation calls in Rust files
rg -n 'execute_validation|\.validate\(' -C3 --glob='*.rs' -g '!target/**'

Repository: PlatformNetwork/platform-v2

Length of output: 21062


🏁 Script executed:

#!/bin/bash
# Search in the term-challenge file for validate() and evaluate() context
cat -n challenges/term-challenge/src/lib.rs | head -250

Repository: PlatformNetwork/platform-v2

Length of output: 9790


🏁 Script executed:

#!/bin/bash
# Get the execute_validation implementation and surrounding context
sed -n '275,320p' bins/validator-node/src/wasm_executor.rs

Repository: PlatformNetwork/platform-v2

Length of output: 1703


🏁 Script executed:

#!/bin/bash
# Search for calls to execute_validation
rg -n 'execute_validation' --glob='*.rs' -C5

Repository: PlatformNetwork/platform-v2

Length of output: 795


🏁 Script executed:

#!/bin/bash
# Search for any actual calls to execute_validation (not just the definition)
rg 'execute_validation\s*\(' --glob='*.rs' -v 'pub fn execute_validation'

Repository: PlatformNetwork/platform-v2

Length of output: 136


🏁 Script executed:

#!/bin/bash
# Search for calls to execute_validation method
rg 'execute_validation' --glob='*.rs' | grep -v 'pub fn execute_validation'

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check the Challenge trait definition in the SDK
find . -name "*.rs" -path "*/platform-challenge-sdk*" | head -20

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Search for the Challenge trait definition
rg 'trait Challenge' --glob='*.rs' -A 20

Repository: PlatformNetwork/platform-v2

Length of output: 7373


🏁 Script executed:

#!/bin/bash
# Search for how validate and evaluate are called from the Challenge trait implementation
rg 'challenge\.validate|challenge\.evaluate' --glob='*.rs' -B2 -A2

Repository: PlatformNetwork/platform-v2

Length of output: 9458


🏁 Script executed:

#!/bin/bash
# Look at the challenge-sdk server to see the route handlers
sed -n '560,580p' crates/challenge-sdk/src/server.rs

Repository: PlatformNetwork/platform-v2

Length of output: 636


🏁 Script executed:

#!/bin/bash
# Look at the evaluate_handler and route setup
sed -n '520,570p' crates/challenge-sdk/src/server.rs

Repository: PlatformNetwork/platform-v2

Length of output: 1676


🏁 Script executed:

#!/bin/bash
# Check the router setup to see if both endpoints are exposed
rg 'Router::new|\.route\(' crates/challenge-sdk/src/server.rs | head -30

Repository: PlatformNetwork/platform-v2

Length of output: 336


🏁 Script executed:

#!/bin/bash
# Check if there are any actual uses of the /validate endpoint or validate() being called from evaluate()
rg '/validate|validate.*evaluate' --glob='*.rs' -B3 -A3 | head -50

Repository: PlatformNetwork/platform-v2

Length of output: 1247


validate() can be called independently and doesn't update the epoch counter, allowing potential rate-limit bypass via repeated validation calls.

The /validate HTTP endpoint is exposed separately from /evaluate, and validate() checks rate limiting via get_last_submission_epoch() but only evaluate() calls set_last_submission_epoch() to advance it. A malicious miner can repeatedly call /validate for the same submission without triggering the epoch update, bypassing the intended rate-limiting mechanism on subsequent validation attempts. Since validate() deserializes data and calls host_consensus_get_epoch() and host_storage_get(), repeated abuse could accumulate costs if these host operations are expensive.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/lib.rs` around lines 180 - 228, validate()
currently enforces rate limits by reading get_last_submission_epoch() but
doesn’t advance them, allowing repeated /validate calls to bypass limits; modify
validate() so that after it deserializes Submission, checks
host_consensus_get_epoch() and the rate-limit condition succeed, it calls
set_last_submission_epoch(&submission.miner_hotkey, current as u64) (or
equivalent API) to record the epoch before returning true; ensure you use the
same epoch value logic as evaluate() and keep get_last_submission_epoch(),
host_consensus_get_epoch(), and Submission references consistent so subsequent
validate() calls cannot be used to bypass rate limiting.

Comment on lines +18 to 24
pub fn get_active_dataset() -> Option<Vec<TaskDefinition>> {
let data = host_storage_get(ACTIVE_DATASET_KEY).ok()?;
if data.is_empty() {
return None;
}
bincode_options_dataset().deserialize(&data).ok()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Serialization format mismatch: store_dataset writes DatasetSelection, get_active_dataset reads Vec<TaskDefinition>.

store_dataset serializes a DatasetSelection (which includes tasks, selected_at_epoch, dataset_hash) into ACTIVE_DATASET_KEY, but get_active_dataset deserializes the same key as Vec<TaskDefinition>. This works only because tasks is the first field in DatasetSelection and allow_trailing_bytes() is set, so bincode reads just the Vec<TaskDefinition> prefix and discards the rest.

This is fragile: reordering fields in DatasetSelection or changing the serialization format will silently break get_active_dataset. Consider either:

  • Storing the full DatasetSelection and extracting .tasks after deserialization, or
  • Serializing only selection.tasks into ACTIVE_DATASET_KEY.
Option A: Deserialize DatasetSelection then extract tasks
 pub fn get_active_dataset() -> Option<Vec<TaskDefinition>> {
     let data = host_storage_get(ACTIVE_DATASET_KEY).ok()?;
     if data.is_empty() {
         return None;
     }
-    bincode_options_dataset().deserialize(&data).ok()
+    let selection: DatasetSelection = bincode_options_dataset().deserialize(&data).ok()?;
+    Some(selection.tasks)
 }

Also applies to: 26-36

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/tasks.rs` around lines 18 - 24, The code
currently deserializes ACTIVE_DATASET_KEY as Vec<TaskDefinition> but
store_dataset writes a DatasetSelection; fix by deserializing a DatasetSelection
(use bincode_options_dataset().deserialize to get a DatasetSelection) and return
its .tasks (i.e., change get_active_dataset to return selection.tasks wrapped in
Some or None accordingly), and apply the same change to the other similar
function handling ACTIVE_DATASET_KEY (the block noted at lines 26-36) so both
read the full DatasetSelection and extract .tasks instead of relying on
trailing-bytes behavior.

Comment on lines 97 to +112
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct EvalMetrics {
pub tasks_passed: u32,
pub tasks_failed: u32,
pub total_tasks: u32,
pub pass_rate: f64,
pub total_execution_time_ms: u64,
pub struct DecayParams {
pub grace_period_hours: u64,
pub half_life_hours: u64,
pub min_multiplier: f64,
}

impl Default for DecayParams {
fn default() -> Self {
Self {
grace_period_hours: 72,
half_life_hours: 24,
min_multiplier: 0.0,
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard against half_life_hours == 0 in decay calculations.

DecayParams allows half_life_hours: 0 which would cause a division-by-zero (or infinite result) in any decay formula like 2^(-elapsed / half_life). Consider either validating at construction or documenting that callers must check for zero.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@challenges/term-challenge/src/types.rs` around lines 97 - 112, DecayParams
currently allows half_life_hours == 0 which will break decay math; change the
half_life_hours field to a non-zero type (std::num::NonZeroU64) and update the
Default impl to use NonZeroU64::new(24).unwrap() (or add a constructor that
returns Result to validate), and update any code using
DecayParams::half_life_hours to call .get() — alternatively, if you prefer not
to change the type, add a validate() or new() constructor on DecayParams that
rejects or sanitizes zero half_life_hours and update Default to set a safe
non-zero value (24) so callers and decay functions (where 2.0_f64.powf(-elapsed
/ half_life)) are protected from division-by-zero.

Comment thread crates/p2p-consensus/src/consensus.rs Outdated
Comment on lines +1197 to +1202
let validators: Vec<Hotkey> = self
.validator_set
.active_validators()
.into_iter()
.map(|r| r.hotkey)
.collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

ast-grep --pattern 'fn active_validators($$$) {
  $$$
}'

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

rg -n "active_validators" --type rust -C 3

Repository: PlatformNetwork/platform-v2

Length of output: 3813


🏁 Script executed:

rg -n "struct ValidatorSet\|impl ValidatorSet" --type rust -A 10

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

rg -n "struct ValidatorSet\|validators:" --type rust -A 3

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

rg -n "pub validators" --type rust -B 2 -A 2 | head -50

Repository: PlatformNetwork/platform-v2

Length of output: 2928


🏁 Script executed:

fd -e rs -x rg -l "HashMap.*validators|BTreeMap.*validators" {} \;

Repository: PlatformNetwork/platform-v2

Length of output: 123


🏁 Script executed:

rg -n "struct ValidatorSet" --type rust -A 10

Repository: PlatformNetwork/platform-v2

Length of output: 974


🏁 Script executed:

rg -n "impl ValidatorSet" --type rust -A 30 | grep -A 30 "active_validators"

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

sed -n '203,210p' crates/p2p-consensus/src/validator.rs

Repository: PlatformNetwork/platform-v2

Length of output: 326


🏁 Script executed:

rg -n "pub fn active_validators" --type rust -A 8

Repository: PlatformNetwork/platform-v2

Length of output: 1265


Deterministic reviewer selection requires sorted validator ordering.

active_validators() iterates over a HashMap (line 93 of crates/p2p-consensus/src/validator.rs), which has non-deterministic iteration order in Rust. This causes different validators to compute different reviewer assignments for the same (submission_id, epoch), breaking network consensus on reviewer selection.

Sort the validators by hotkey before using them in select_review_validators() to ensure all nodes assign reviewers consistently:

Current problematic code
let validators: Vec<Hotkey> = self
    .validator_set
    .active_validators()
    .into_iter()
    .map(|r| r.hotkey)
    .collect();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/p2p-consensus/src/consensus.rs` around lines 1197 - 1202,
active_validators() returns a non-deterministic order (HashMap) so the
Vec<Hotkey> assigned to validators must be sorted before calling
select_review_validators(); modify the block that builds validators in
consensus.rs (the variable validators used by select_review_validators) to sort
the Hotkey list deterministically (e.g., by hotkey string/bytes or impl Ord) so
all nodes compute the same reviewer assignment.

Comment thread crates/p2p-consensus/src/consensus.rs Outdated
Comment on lines +1204 to +1244
let select_n = |offset: usize| -> [Hotkey; 3] {
if validators.is_empty() {
return [Hotkey([0u8; 32]), Hotkey([0u8; 32]), Hotkey([0u8; 32])];
}

let mut selected = Vec::with_capacity(3);
let mut state: u64 = u64::from_le_bytes([
seed[offset],
seed[offset + 1],
seed[offset + 2],
seed[offset + 3],
seed[offset + 4],
seed[offset + 5],
seed[offset + 6],
seed[offset + 7],
]);

while selected.len() < 3 && selected.len() < validators.len() {
state = state
.wrapping_mul(6364136223846793005)
.wrapping_add(1442695040888963407);
let idx = (state >> 33) as usize % validators.len();
let candidate = &validators[idx];
if !selected.contains(candidate) {
selected.push(candidate.clone());
}
}

while selected.len() < 3 {
selected.push(Hotkey([0u8; 32]));
}

[
selected[0].clone(),
selected[1].clone(),
selected[2].clone(),
]
};

let llm_reviewers = select_n(0);
let ast_reviewers = select_n(8);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

LLM and AST reviewer groups can overlap.

The two calls select_n(0) and select_n(8) each independently select 3 validators without cross-group deduplication. A validator could be assigned both LLM and AST review for the same submission. If the intent is 6 distinct reviewers, deduplicate across groups.

Suggested approach

Select all 6 at once from the validator pool and split into two groups, or pass the already-selected set from the first call into the second to exclude them.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/p2p-consensus/src/consensus.rs` around lines 1204 - 1244, The current
closure select_n and its independent calls (llm_reviewers = select_n(0) and
ast_reviewers = select_n(8)) can return overlapping validators; change the
selection so the two groups are deduplicated—either select six distinct
validators once (e.g., sample 6 from validators with the existing PRNG state and
then split into llm_reviewers and ast_reviewers), or modify select_n to accept
an exclusion set (pass the first group's Hotkeys to the second call to skip
duplicates); ensure you still pad with Hotkey([0u8;32]) when validators are
fewer than needed and keep the same RNG logic/state (the state derived from
seed[offset..]) to preserve determinism.

Comment on lines +479 to +497
/// Query entries by prefix within a challenge namespace
pub fn query_by_prefix(
&self,
challenge_id: &ChallengeId,
prefix: &str,
) -> Result<Vec<(String, Vec<u8>)>> {
let namespace = challenge_id.0.to_string();
let entries = self.scan_namespace(&namespace)?;

entries
.into_iter()
.filter(|(k, _)| k.validator.is_none() && k.key.starts_with(prefix))
.map(|(k, entry)| {
let value_bytes = bincode::serialize(&entry.value)
.map_err(|e| MiniChainError::Serialization(e.to_string()))?;
Ok((k.key, value_bytes))
})
.collect()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

query_by_prefix: unbounded result set and redundant serialization round-trip

Two related concerns:

  1. Unbounded results — the method loads the entire namespace into memory, filters, and returns all matches as a Vec. A large or adversarially-filled namespace will exhaust memory with no safeguard. Consider adding a limit: Option<usize> parameter, or at minimum documenting the cost.

  2. Double (de)serializationscan_namespace fully deserializes every StorageEntry in the namespace, then this method immediately re-serializes only entry.value back to bytes. The round-trip is unnecessary; a dedicated lower-level scan (skipping full deserialization for non-matching keys) would be more efficient for hot paths.

♻️ Sketch: avoid full deserialization for unmatched entries
-    let entries = self.scan_namespace(&namespace)?;
-
-    entries
-        .into_iter()
-        .filter(|(k, _)| k.validator.is_none() && k.key.starts_with(prefix))
-        .map(|(k, entry)| {
-            let value_bytes = bincode::serialize(&entry.value)
-                .map_err(|e| MiniChainError::Serialization(e.to_string()))?;
-            Ok((k.key, value_bytes))
-        })
-        .collect()
+    const MAX_RESULTS: usize = 1_000;
+    let ns_prefix = StorageKey::namespace_prefix(&namespace);
+    let mut results = Vec::new();
+    for item in self.tree.scan_prefix(&ns_prefix) {
+        let (key_bytes, raw) = item.map_err(|e| MiniChainError::Storage(e.to_string()))?;
+        let sk = match self.parse_key(&key_bytes) {
+            Some(k) if k.validator.is_none() && k.key.starts_with(prefix) => k,
+            _ => continue,
+        };
+        let entry: StorageEntry = bincode::deserialize(&raw)
+            .map_err(|e| MiniChainError::Serialization(e.to_string()))?;
+        if entry.is_expired() { continue; }
+        let value_bytes = bincode::serialize(&entry.value)
+            .map_err(|e| MiniChainError::Serialization(e.to_string()))?;
+        results.push((sk.key, value_bytes));
+        if results.len() >= MAX_RESULTS { break; }
+    }
+    Ok(results)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/storage/src/dynamic.rs` around lines 479 - 497, query_by_prefix
currently loads the entire namespace via scan_namespace and deserializes every
StorageEntry, then re-serializes entry.value and returns an unbounded Vec which
can OOM; change the API and implementation to accept an optional limit parameter
(e.g., limit: Option<usize>) and enforce it while collecting results, and add or
call a lower-level scanning helper (e.g., a new scan_namespace_raw or
scan_namespace_keys_values) that yields raw bytes or skips full deserialization
so you can filter keys by prefix without deserializing non-matching entries;
update query_by_prefix to use that raw scanner, serialize only when needed,
respect the limit, and propagate errors using the existing MiniChainError path
(referencing functions/structs query_by_prefix, scan_namespace, StorageEntry and
field value).

Comment on lines +505 to +526
pub fn get_at_block(
&self,
challenge_id: &ChallengeId,
key: &str,
block: u64,
) -> Result<Option<Vec<u8>>> {
let storage_key = StorageKey::challenge(challenge_id, key);
let entry = self.get(&storage_key)?;

match entry {
Some(e) => {
if e.version <= block {
let value_bytes = bincode::serialize(&e.value)
.map_err(|err| MiniChainError::Serialization(err.to_string()))?;
Ok(Some(value_bytes))
} else {
Ok(None)
}
}
None => Ok(None),
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the StorageEntry struct definition
find . -name "*.rs" -type f -exec grep -l "struct StorageEntry" {} \;

Repository: PlatformNetwork/platform-v2

Length of output: 100


🏁 Script executed:

# Search for StorageEntry struct and its fields
rg -n "struct StorageEntry" -A 15 --type=rust

Repository: PlatformNetwork/platform-v2

Length of output: 1034


🏁 Script executed:

# Look at the update() and new() methods for StorageEntry
rg -n "impl StorageEntry" -A 30 --type=rust

Repository: PlatformNetwork/platform-v2

Length of output: 1883


🏁 Script executed:

# Examine how version field is used throughout the codebase
rg -n "\.version" --type=rust -B 2 -A 2 | head -100

Repository: PlatformNetwork/platform-v2

Length of output: 6818


🏁 Script executed:

# Check the set_with_options method to see how StorageEntry is created/updated
rg -n "fn set_with_options" -A 30 --type=rust

Repository: PlatformNetwork/platform-v2

Length of output: 2104


🏁 Script executed:

# Examine DynamicStorage struct definition
rg -n "struct DynamicStorage" -A 15 --type=rust

Repository: PlatformNetwork/platform-v2

Length of output: 1082


🏁 Script executed:

# Check for block_height field in DynamicStorage
rg -n "block_height" --type=rust -B 2 -A 2

Repository: PlatformNetwork/platform-v2

Length of output: 50385


🏁 Script executed:

# Examine StorageChange struct
rg -n "struct StorageChange" -A 15 --type=rust

Repository: PlatformNetwork/platform-v2

Length of output: 995


🏁 Script executed:

# View the exact get_at_block method to confirm it matches the review
sed -n '505,526p' crates/storage/src/dynamic.rs

Repository: PlatformNetwork/platform-v2

Length of output: 751


get_at_block: e.version is an optimistic-lock counter, not a block height — the gate is effectively a no-op

StorageEntry.version is incremented on every write (1, 2, 3…) for optimistic concurrency control. Neither StorageEntry::new() nor the update() call in set_with_options receives the current block_height as a parameter. The block height at write time is recorded only in StorageChange, never persisted in StorageEntry itself.

Because of this, e.version <= block will almost always evaluate to true (a write-count of a few versus a block height in the thousands), rendering get_at_block equivalent to "return the current value" regardless of the requested block height. The method cannot implement its documented semantics.

The fix requires recording block height in StorageEntry at write time:

Proposed fix — store block height in StorageEntry

In StorageEntry, add a last_block: u64 field, then in set_with_options:

 let entry = if let Some(mut existing) = old_entry {
-    existing.update(value.clone(), writer);
+    existing.update(value.clone(), writer, *self.block_height.read());
     ...
 } else {
-    let mut e = StorageEntry::new(value.clone(), writer);
+    let mut e = StorageEntry::new(value.clone(), writer, *self.block_height.read());
     ...
 };

Then in get_at_block:

-            if e.version <= block {
+            if e.last_block <= block {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/storage/src/dynamic.rs` around lines 505 - 526, get_at_block currently
compares block height against the optimistic-lock counter; add a last_block: u64
field to StorageEntry (and initialize it in StorageEntry::new and any update()
path), set last_block to the write's block height when writing in
set_with_options (use the StorageChange/block_height available at write time),
and change get_at_block to compare e.last_block <= block instead of e.version <=
block; also update any (de)serialization or migrations that touch StorageEntry
to include the new field.

Comment on lines +277 to +291
let client = reqwest::blocking::Client::new();
let http_response = match client
.post(&endpoint)
.header("Content-Type", "application/json")
.header("Authorization", format!("Bearer {}", api_key))
.body(json_body)
.timeout(std::time::Duration::from_secs(LLM_REQUEST_TIMEOUT_SECS))
.send()
{
Ok(r) => r,
Err(err) => {
warn!(error = %err, "llm_chat_completion: HTTP request failed");
return LlmHostStatus::ApiError.to_i32();
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how the WASM executor invokes modules — is it on an async context?
rg -n 'spawn_blocking|block_in_place|tokio::task' crates/wasm-runtime-interface/src/
echo "---"
rg -n 'async fn.*execute\|async fn.*run\|async fn.*call' crates/wasm-runtime-interface/src/

Repository: PlatformNetwork/platform-v2

Length of output: 74


🏁 Script executed:

#!/bin/bash
# Check the function signature and context of handle_chat_completion
cat -n crates/wasm-runtime-interface/src/llm.rs | head -300 | tail -100

Repository: PlatformNetwork/platform-v2

Length of output: 3634


🏁 Script executed:

#!/bin/bash
# Look for LlmState structure and Client storage
rg -n 'struct LlmState\|pub struct LlmState' crates/wasm-runtime-interface/src/llm.rs -A 20

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check how handle_chat_completion is called
rg -n 'handle_chat_completion' crates/wasm-runtime-interface/src/ -B 2 -A 2

Repository: PlatformNetwork/platform-v2

Length of output: 795


🏁 Script executed:

#!/bin/bash
# Check file structure and imports at top
head -100 crates/wasm-runtime-interface/src/llm.rs

Repository: PlatformNetwork/platform-v2

Length of output: 2748


🏁 Script executed:

#!/bin/bash
# Look for where Linker is used and in what context (async or sync)
rg -n 'Linker' crates/wasm-runtime-interface/src/llm.rs -B 3 -A 3

Repository: PlatformNetwork/platform-v2

Length of output: 547


🏁 Script executed:

#!/bin/bash
# Check parent module and how WASM runtime is initialized
rg -n 'mod llm\|pub mod llm' crates/wasm-runtime-interface/src/
ls -la crates/wasm-runtime-interface/src/

Repository: PlatformNetwork/platform-v2

Length of output: 965


🏁 Script executed:

#!/bin/bash
# Look for LlmState definition more broadly
rg -n 'LlmState' crates/wasm-runtime-interface/src/llm.rs | head -20

Repository: PlatformNetwork/platform-v2

Length of output: 175


🏁 Script executed:

#!/bin/bash
# Check if the executor/runtime is async
rg -n 'async' crates/wasm-runtime-interface/src/lib.rs crates/wasm-runtime-interface/src/mod.rs 2>/dev/null || echo "Files not found, checking structure"
find crates/wasm-runtime-interface/src -name "*.rs" -type f

Repository: PlatformNetwork/platform-v2

Length of output: 678


🏁 Script executed:

#!/bin/bash
# Check runtime.rs to see if executor runs in async context
head -150 crates/wasm-runtime-interface/src/runtime.rs

Repository: PlatformNetwork/platform-v2

Length of output: 5226


🏁 Script executed:

#!/bin/bash
# Look for async fn and tokio usage in runtime
rg -n 'async fn|tokio::|block_on' crates/wasm-runtime-interface/src/runtime.rs | head -30

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check where handle_chat_completion is called from
rg -n 'handle_chat_completion\|func_wrap.*llm' crates/wasm-runtime-interface/src/llm.rs -A 5 -B 2

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check the full context of func_wrap for llm_chat_completion
sed -n '120,145p' crates/wasm-runtime-interface/src/llm.rs

Repository: PlatformNetwork/platform-v2

Length of output: 1048


🏁 Script executed:

#!/bin/bash
# Look for where WASM modules are actually called/executed
rg -n 'call\(|execute\|\.call' crates/wasm-runtime-interface/src/runtime.rs -A 2 -B 2

Repository: PlatformNetwork/platform-v2

Length of output: 2162


🏁 Script executed:

#!/bin/bash
# Check for async context where Store is used
rg -n 'async fn' crates/wasm-runtime-interface/src/runtime.rs

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check lib.rs for executor and async context
rg -n 'async fn|tokio' crates/wasm-runtime-interface/src/lib.rs | head -20

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Look for any block_on or sync execution patterns
rg -n 'block_on|blocking|spawn_blocking' crates/wasm-runtime-interface/src/

Repository: PlatformNetwork/platform-v2

Length of output: 340


🏁 Script executed:

#!/bin/bash
# Check if WasmRuntime is called from async context in parent code
find crates -name "*.rs" -type f ! -path "*/wasm-runtime-interface/*" -exec grep -l "WasmRuntime" {} \;

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Look for async callers of WasmRuntime methods
rg -n 'async fn.*WasmRuntime\|WasmRuntime.*async\|\.call\(' crates --type rust -B 3 | grep -E 'async fn|WasmRuntime|\.call\(' | head -40

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check if handle_chat_completion can be called repeatedly
rg -n 'llm_chat_completion\|call.*llm' crates --type rust -B 2 -A 2 | head -50

Repository: PlatformNetwork/platform-v2

Length of output: 53


🏁 Script executed:

#!/bin/bash
# Check Cargo.toml for wasm-runtime-interface
cat crates/wasm-runtime-interface/Cargo.toml

Repository: PlatformNetwork/platform-v2

Length of output: 610


🏁 Script executed:

#!/bin/bash
# Check if tokio is a dependency anywhere in the workspace
rg -n 'tokio' Cargo.toml crates/*/Cargo.toml 2>/dev/null | head -20

Repository: PlatformNetwork/platform-v2

Length of output: 1649


🏁 Script executed:

#!/bin/bash
# Check the full llm.rs file structure for Client reuse patterns
cat crates/wasm-runtime-interface/src/llm.rs

Repository: PlatformNetwork/platform-v2

Length of output: 13681


Store reqwest::blocking::Client in LlmState to reuse connection pools across requests.

Creating a new Client on every handle_chat_completion invocation (line 277) allocates a fresh connection pool and TLS session. For repeated calls, store the client in LlmState instead.

Additionally, if the WASM executor may run on a Tokio async thread (e.g., when called from rpc-server or challenge-orchestrator), reqwest::blocking will panic or deadlock. Ensure this always runs on a dedicated blocking thread via tokio::task::spawn_blocking, or verify that callers never invoke this from an async context.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/wasm-runtime-interface/src/llm.rs` around lines 277 - 291, Replace
per-call creation of reqwest::blocking::Client in handle_chat_completion with a
reusable client stored on LlmState: add a field (e.g., client:
reqwest::blocking::Client) to LlmState, initialize it once when constructing
LlmState, and use that client for the POST request (keeping existing headers,
body, and LLM_REQUEST_TIMEOUT_SECS). Also ensure the blocking request cannot run
on a Tokio async thread by wrapping the blocking HTTP call in
tokio::task::spawn_blocking (or alternatively document/validate that callers
never invoke handle_chat_completion from an async context); update references to
reqwest::blocking::Client and handle_chat_completion accordingly.

Comment on lines +293 to +330
let response_body = match http_response.bytes() {
Ok(b) => b.to_vec(),
Err(err) => {
warn!(error = %err, "llm_chat_completion: failed to read response body");
return LlmHostStatus::ApiError.to_i32();
}
};

#[derive(Deserialize)]
struct OpenAiResponse {
choices: Option<Vec<OpenAiChoice>>,
usage: Option<OpenAiUsage>,
}

#[derive(Deserialize)]
struct OpenAiChoice {
message: Option<OpenAiRespMessage>,
}

#[derive(Deserialize)]
struct OpenAiRespMessage {
content: Option<String>,
}

#[derive(Deserialize)]
struct OpenAiUsage {
prompt_tokens: Option<u32>,
completion_tokens: Option<u32>,
total_tokens: Option<u32>,
}

let openai_resp: OpenAiResponse = match serde_json::from_slice(&response_body) {
Ok(r) => r,
Err(err) => {
warn!(error = %err, "llm_chat_completion: failed to parse OpenAI response");
return LlmHostStatus::ApiError.to_i32();
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

HTTP response status code is not checked before parsing the body as a successful response.

If the Chutes API returns a 4xx/5xx status (e.g., 401 unauthorized, 429 rate limited, 500 internal error), the body likely contains an error message rather than valid JSON with choices. The serde_json::from_slice will either fail (returning ApiError) or succeed with choices: None, resulting in an empty content string silently returned to the WASM module as a "successful" response.

Consider checking http_response.status() before parsing, and returning a more specific error (e.g., RateLimited for 429).

Proposed fix
+    if !http_response.status().is_success() {
+        warn!(
+            status = %http_response.status(),
+            "llm_chat_completion: API returned error status"
+        );
+        return LlmHostStatus::ApiError.to_i32();
+    }
+
     let response_body = match http_response.bytes() {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let response_body = match http_response.bytes() {
Ok(b) => b.to_vec(),
Err(err) => {
warn!(error = %err, "llm_chat_completion: failed to read response body");
return LlmHostStatus::ApiError.to_i32();
}
};
#[derive(Deserialize)]
struct OpenAiResponse {
choices: Option<Vec<OpenAiChoice>>,
usage: Option<OpenAiUsage>,
}
#[derive(Deserialize)]
struct OpenAiChoice {
message: Option<OpenAiRespMessage>,
}
#[derive(Deserialize)]
struct OpenAiRespMessage {
content: Option<String>,
}
#[derive(Deserialize)]
struct OpenAiUsage {
prompt_tokens: Option<u32>,
completion_tokens: Option<u32>,
total_tokens: Option<u32>,
}
let openai_resp: OpenAiResponse = match serde_json::from_slice(&response_body) {
Ok(r) => r,
Err(err) => {
warn!(error = %err, "llm_chat_completion: failed to parse OpenAI response");
return LlmHostStatus::ApiError.to_i32();
}
};
if !http_response.status().is_success() {
warn!(
status = %http_response.status(),
"llm_chat_completion: API returned error status"
);
return LlmHostStatus::ApiError.to_i32();
}
let response_body = match http_response.bytes() {
Ok(b) => b.to_vec(),
Err(err) => {
warn!(error = %err, "llm_chat_completion: failed to read response body");
return LlmHostStatus::ApiError.to_i32();
}
};
#[derive(Deserialize)]
struct OpenAiResponse {
choices: Option<Vec<OpenAiChoice>>,
usage: Option<OpenAiUsage>,
}
#[derive(Deserialize)]
struct OpenAiChoice {
message: Option<OpenAiRespMessage>,
}
#[derive(Deserialize)]
struct OpenAiRespMessage {
content: Option<String>,
}
#[derive(Deserialize)]
struct OpenAiUsage {
prompt_tokens: Option<u32>,
completion_tokens: Option<u32>,
total_tokens: Option<u32>,
}
let openai_resp: OpenAiResponse = match serde_json::from_slice(&response_body) {
Ok(r) => r,
Err(err) => {
warn!(error = %err, "llm_chat_completion: failed to parse OpenAI response");
return LlmHostStatus::ApiError.to_i32();
}
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/wasm-runtime-interface/src/llm.rs` around lines 293 - 330, The code
parses response_body and deserializes into OpenAiResponse without checking
http_response.status(); update the llm_chat_completion flow to inspect
http_response.status() immediately after reading response_body (or before
parsing) and if status is not success (e.g., !status.is_success()) map error
codes to appropriate LlmHostStatus values (return RateLimited for 429, ApiError
for 4xx/5xx otherwise), log the status and any response text or error detail
using the existing warn! pattern (include error = %err and status = %status),
and return early instead of calling serde_json::from_slice on error responses so
openai_resp/choices parsing only runs for 2xx responses.

Comment on lines +332 to +337
let content = openai_resp
.choices
.and_then(|mut c| c.pop())
.and_then(|c| c.message)
.and_then(|m| m.content)
.unwrap_or_default();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

choices.pop() returns the last choice, not the first.

OpenAI's API returns choices[0] as the primary completion. With n=1 (the default) this works, but if n > 1 is ever passed through, this silently returns the wrong choice. Use .first() or .into_iter().next() instead.

Proposed fix
     let content = openai_resp
         .choices
-        .and_then(|mut c| c.pop())
+        .and_then(|c| c.into_iter().next())
         .and_then(|c| c.message)
         .and_then(|m| m.content)
         .unwrap_or_default();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let content = openai_resp
.choices
.and_then(|mut c| c.pop())
.and_then(|c| c.message)
.and_then(|m| m.content)
.unwrap_or_default();
let content = openai_resp
.choices
.and_then(|c| c.into_iter().next())
.and_then(|c| c.message)
.and_then(|m| m.content)
.unwrap_or_default();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/wasm-runtime-interface/src/llm.rs` around lines 332 - 337, The code
extracts the response content by calling openai_resp.choices.and_then(|mut c|
c.pop()) which returns the last choice instead of the primary one; change this
to take the first choice (e.g., use .first() or .into_iter().next()) so that the
primary completion (choices[0]) is used reliably even when n > 1, updating the
chain around openai_resp.choices -> choice -> message -> content and preserving
the final unwrap_or_default() behavior.

… log injection

- Add bincode size limits to P2P message deserialization (messages.rs, network.rs)
- Add bincode size limits to state deserialization (state.rs)
- Add bincode size limits to storage entry deserialization (dynamic.rs)
- Redact secret_key in Args Debug impl (main.rs)
- Redact chutes_api_key in WasmExecutorConfig Debug impl (wasm_executor.rs)
- Sanitize user-provided ReviewDecline reason before logging (main.rs)
… return value

- Remove select_review_validators from ConsensusEngine: defined but never
  called from any code path (no callers in bins/, crates/, or challenges/)
- Make store_dataset return value explicitly ignored with let _ = in both
  term-challenge and term-challenge-wasm configure() methods

Issues analyzed (10 total, 2 fixed, 8 intentionally kept):
- select_review_validators: REMOVED (orphaned, 67 lines)
- store_dataset ignored return: FIXED (explicit let _ =)
- LLM types (llm_types.rs): kept (SDK API surface for challenge authors)
- host_llm_* functions: kept (WASM FFI bridge, host-side registered)
- ReviewRecord/ReviewResultEntry: kept (ChainState serialization + API)
- assign_review/add_review_result/get_review_status: kept (review feature API)
- DecayParams/active_dataset fields: kept (binary serialization compat)
- query_by_prefix/get_at_block/list_keys: kept (public storage API)
- ReviewType enum: kept (used by message types and state)
- No unused imports found
- Move const after use block in wasm_executor.rs
- Replace .unwrap() with if-let in consensus.rs
- Propagate memory read error instead of silent default in wasm_executor.rs
- Remove dead _sandbox_host_fns assignments in wasm_executor.rs
- Extract SCORE_SCALE_FACTOR and LLM_JUDGE_PASS_THRESHOLD constants
- Extract JOB_TIMEOUT_MS constant in main.rs
- Use bounded bincode options for serialize calls in challenge crates
- Remove incorrect #[allow(dead_code)] on validator_set field
- Remove unused SandboxHostFunctions import
…code compatibility

The host-side ChatRequest struct used Option<u32>/Option<f32> for max_tokens
and temperature, while the guest-side LlmRequest uses plain u32/f32. In
bincode, Option<T> encodes a 1-byte tag prefix before the value, making
the formats binary-incompatible. Deserialization of guest-serialized
LlmRequest bytes would fail on the host.

Changed ChatRequest to use u32/f32 (matching the guest) and wrap values
in Some() when constructing the OpenAI JSON request.
@echobt
echobt merged commit ef45b21 into main Feb 18, 2026
10 checks passed
echobt added a commit that referenced this pull request Feb 18, 2026
- Replace silent error-swallowing (let _ = host_storage_set) with
  proper error logging via host_log in both term-challenge variants
- Remove unused _event_tx_clone variable in p2p network start()
- Extract magic timeout numbers into named constants
  DEFAULT_ROUND_TIMEOUT_MS and DEFAULT_VIEW_CHANGE_TIMEOUT_MS
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.

1 participant