-
Notifications
You must be signed in to change notification settings - Fork 0
feat: WASM-based dynamic challenge loading system with term-challenge integration #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements a lightweight rapid consensus protocol for validation results: - Single-round stake-weighted voting for quick finality - Configurable finality threshold and timeout - Confidence scoring based on score variance - Full signature verification and vote tracking - Comprehensive test coverage
…tion - Add WasmRuntime struct with wasmtime engine and module caching - Add SandboxConfig for resource limits (memory, CPU, fuel) - Add ChallengeModule trait and WasmChallengeModule implementation - Add host functions: logging, timestamps, deterministic RNG - Add comprehensive error types for WASM operations - Add ResourceUsage tracking for execution monitoring
- Add ChallengeLoader for loading and managing WASM modules - Add ChallengeRegistry for thread-safe challenge storage - Add VersionManager for version history and rollback - Add ChallengeDiscovery trait with Filesystem and P2P implementations - Add CompositeDiscovery for aggregating multiple sources - Support hot-reload with version preservation - Include comprehensive test suite (74 tests)
Add WASM-compatible types extracted from term-challenge for use in the dynamic challenge loading system: - Difficulty, TermTaskConfig for task configuration - TermTaskResult, TermAggregateScore for evaluation results - TermScoreCalculator for scoring logic - WasmEvalRequest/Response for WASM interface - WasmChallengeInterface trait for challenge modules - TerminalBenchChallenge default implementation
…gnment, and fast consensus - Add platform-wasm-runtime and platform-challenge-loader dependencies - Add CLI arguments for challenge directory and WASM challenges toggle - Initialize ChallengeLoader with configurable challenge directory - Initialize ValidatorAssignment with stake-weighted selection - Initialize FastConsensus for rapid validation finality - Initialize StorageReader for direct metagraph access - Add challenge discovery interval for periodic challenge listing - Update epoch transition handler to refresh assignment seed - Handle evaluation messages in network event handler
📝 WalkthroughWalkthroughIntroduces two new workspace crates—challenge-loader and wasm-runtime—providing infrastructure for discovering, loading, versioning, and executing WASM-based challenge modules. Extends validator-node with challenge discovery, consensus assignment, and fast validation voting. Adds storage reading and on-chain metagraph access via bittensor-integration. Expands core types with terminal benchmark challenge definitions and p2p consensus mechanisms. Changes
Sequence Diagram(s)sequenceDiagram
participant FS as Filesystem
participant CD as Challenge<br/>Discovery
participant CL as Challenge<br/>Loader
participant VN as Validator<br/>Node
participant WR as WASM<br/>Runtime
FS->>CD: scan directory for .wasm files
CD->>CD: parse config (id, version, code_hash)
CD->>CD: emit DiscoveredChallenge with metadata
CD->>VN: broadcast ChallengeUpdate::Added
VN->>CL: start_discovery()
Note over VN,CL: subscribe to updates
VN->>CL: load_challenge(discovered_challenge)
CL->>CL: compute code_hash (SHA-256)
CL->>WR: compile_wasm(bytecode)
WR->>WR: create Engine, Store, Instance
WR-->>CL: return compiled module
CL->>CL: register in ChallengeRegistry
CL->>CL: track version in VersionManager
CL-->>VN: challenge loaded (v1)
Note over VN: Challenge available for use
FS->>CD: detect file modification
CD->>CD: compute new code_hash
CD->>VN: broadcast ChallengeUpdate::Updated
VN->>CL: hot_reload(id, new_wasm)
CL->>WR: compile_wasm(new_bytecode)
WR-->>CL: new module
CL->>CL: update registry, register v2
CL-->>VN: hot reload complete
Note over VN: Challenge v2 now active, v1 preserved
sequenceDiagram
participant V1 as Validator 1<br/>(Primary)
participant V2 as Validator 2
participant V3 as Validator 3
participant FC as FastConsensus<br/>Engine
participant VSet as Validator<br/>Set
V1->>FC: submit_result(ValidationResult)
FC->>FC: compute result_hash (SHA-256)
FC->>FC: create & sign ValidationVote
FC->>FC: start ConsensusRound
FC-->>V1: return signed vote
V2->>FC: handle_vote(vote_from_v1)
FC->>VSet: verify signature
VSet-->>FC: ✓ valid
FC->>FC: record vote in round<br/>total_voted_stake += v1_stake
FC-->>V2: round updated
V3->>FC: handle_vote(vote_from_v2)
FC->>VSet: verify signature
VSet-->>FC: ✓ valid
FC->>FC: record vote<br/>total_voted_stake += v2_stake
FC->>FC: check finality<br/>(67% threshold)
alt Threshold Reached
FC->>FC: compute aggregated_score<br/>(stake-weighted avg)
FC->>FC: compute confidence<br/>(variance-based)
FC->>FC: collect signatures
FC->>FC: create FinalizedResult
FC-->>V3: return FinalizedResult
Note over FC: Result finalized, stored
else Threshold Not Reached
FC-->>V3: return None
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
9-24:⚠️ Potential issue | 🟡 MinorUpdate the WASM runtime note to match the new workspace member.
The note claims the WASM runtime was removed, but it’s now included as a workspace member, which is confusing for maintainers.
📝 Suggested doc tweak
-# Note: WASM runtime removed - updates via git, version checked at handshake +# Note: WASM runtime is now part of this workspace; verify versions at handshake if needed
🤖 Fix all issues with AI agents
In `@bins/validator-node/src/main.rs`:
- Around line 296-307: FastConsensus is instantiated as _fast_consensus but
never used, so Evaluation messages are only logged and no vote aggregation
happens; locate where Evaluation messages are handled (the processing loop or
handler that currently logs evaluations) and wire in the FastConsensus instance
by storing Arc<RwLock<FastConsensus>> (the _fast_consensus you created) into the
surrounding state/context, then replace the simple log path with calls into
FastConsensus methods (e.g., FastConsensus::record_evaluation or
FastConsensus::handle_evaluation — whatever method on FastConsensus aggregates
votes) so incoming Evaluation messages are forwarded to _fast_consensus for vote
aggregation and consensus progression; ensure you clone the Arc when moving into
async handlers and acquire the RwLock appropriately before calling the
aggregation methods.
- Around line 798-808: The current call to
validator_assignment.write().update_config(AssignmentConfig { epoch_seed: seed,
..Default::default() }) replaces the whole AssignmentConfig and resets tuned
fields; instead, read or clone the existing AssignmentConfig from
validator_assignment, modify only its epoch_seed with the new seed, and pass
that updated config into update_config so min_validators, max_validators,
stake_weighted, etc. are preserved (use the existing validator_assignment
read/clone, update epoch_seed, then write().update_config(updated_config)).
- Around line 197-223: The ChallengeLoader is initialized with
enable_p2p_discovery but never started; after constructing the loader in the
Ok(loader) arm (the local variable named loader used to create
Some(Arc::new(loader))), call loader.start_discovery() and handle its Result
(log an error/warn and return None or proceed on Ok) so discovery sources are
registered and list_challenges() will populate; update the Ok branch around
ChallengeLoader::new(...) to invoke start_discovery() before wrapping the loader
in Arc and adjust logging on failure.
In `@crates/challenge-loader/src/lib.rs`:
- Around line 138-161: The test_loader_lifecycle uses invalid WASM bytes
(vec![0u8; 100]) which can mask future validation; update the test to use the
valid minimal WASM provided by the loader module by replacing the placeholder
bytes with a call to the existing sample_wasm_bytes() helper (or the equivalent
function defined in loader.rs) when creating `wasm`, keeping the rest of the
test (ChallengeLoader::default_loader, load_challenge, unload_challenge,
assertions) unchanged.
In `@crates/challenge-loader/src/loader.rs`:
- Around line 346-358: hot_reload() is passing new_wasm.clone() into
registry.update(), causing the registry to store the new version bytes as the
old-version history; capture and pass the actual old WASM bytes instead. Before
calling self.registry.update(...) in hot_reload(), retrieve the current/old wasm
bytes from the loaded challenge state (e.g., add wasm_bytes to LoadedChallenge
and read it) or fetch them from the version_manager/ChallengeVersion record,
then pass that old_bytes variable to registry.update(...). Ensure you still
construct and register the new ChallengeVersion
(register_version/activate_version) using new_wasm after updating the registry.
In `@crates/core/src/term_challenge.rs`:
- Around line 227-273: The calculate_aggregate function currently uses
configs.iter().zip(results.iter()) which silently drops mismatched entries;
change the implementation to build a lookup (HashMap) of TermTaskResult by
task_id, then iterate over configs (TermTaskConfig) and for each config lookup
its result: if found use it, otherwise treat as a failing result (increment
failed, update by_difficulty.total, and do not increment passed or total_score),
and include any execution_time_ms only from actual results; ensure by_difficulty
(DifficultyStats) is updated for every config and total, pass_rate, total_score,
tasks_passed/tasks_failed reflect the full configs length rather than the zipped
length.
In `@crates/p2p-consensus/src/assignment.rs`:
- Around line 125-130: The assignment logic currently checks
active_validators.len() against self.config.min_validators but doesn't account
for max_validators < min_validators; compute available_slots =
min(self.config.max_validators, active_validators.len()) (or otherwise derive
the actual number of slots that will be filled) and validate that
available_slots >= self.config.min_validators before continuing in assign;
update the error branch to return AssignmentError::NotEnoughValidators with
needed = self.config.min_validators and available = available_slots, and apply
the same change to the similar check in the later block (the check around the
lines handling max/min validator constraints).
- Around line 142-146: The stake-weighting math in the weighted_priority branch
can produce zero for small stakes because (v.stake.saturating_add(1) /
1_000_000) yields 0, wiping out the hash-derived priority; update the
computation to ensure a minimum stake factor of 1 before multiplying.
Concretely, replace the current expression in the weighted_priority branch with
a two-step calculation: compute let stake_factor =
v.stake.saturating_div(1_000_000).saturating_add(1) (or use stake_factor =
std::cmp::max(1, (v.stake / 1_000_000) as _)), then compute weighted_priority =
priority.saturating_mul(stake_factor) so low-stake validators retain the
original priority influence while higher-stake validators scale up. Ensure you
modify the code around weighted_priority inside the block guarded by
self.config.stake_weighted and reference v.stake and priority accordingly.
In `@crates/p2p-consensus/src/fast_consensus.rs`:
- Around line 147-169: The verify method must recompute the hash of the embedded
result and ensure it equals the stored result_hash before accepting the
signature: in FastConsensus::verify, after constructing VoteSigningData but
before building signing_bytes/validating the signature, recompute the digest of
self.result (using the same hashing routine used when creating result_hash) and
compare it to self.result_hash; if they differ, return an Err (e.g.,
FastConsensusError::InvalidSignature with a clear message referencing
result/result_hash mismatch) and only then proceed to serialize VoteSigningData
and call validator_set.verify_signature; this prevents a vote whose signature
covers the hash but whose embedded self.result has been tampered with from being
accepted.
- Around line 61-87: The compute_hash method currently serializes the entire
ValidationResult (including timestamp, score and metadata), which is
non-deterministic and prevents validators from agreeing; update
ValidationResult::compute_hash to only include the stable identity fields
(challenge_id, submission_hash, miner) when computing the SHA-256 hash — e.g.
build and serialize a tuple or temporary struct containing just these three
fields inside compute_hash, then hash that serialized bytes and return the
digest so result_hash is deterministic across validators.
- Around line 278-307: submit_result currently unconditionally creates and
inserts a fresh ConsensusRound (using ConsensusRound::new and rounds.insert),
which clobbers any existing round state; remove the creation/insertion of the
new round inside submit_result and instead let handle_vote_internal be
responsible for creating the round if missing. Concretely: delete the
ConsensusRound::new(...) and rounds.insert(...) lines (and any dead/commented
code around them), keep acquiring the mutable rounds write lock and then call
self.handle_vote_internal(&mut rounds, vote.clone()) so existing votes are
preserved and new rounds are only created by handle_vote_internal.
- Around line 321-368: In handle_vote_internal, do not trust vote.stake
(untrusted input); instead look up the voter's stake from the validator set (use
self.validator_set or its accessor) and use/overwrite that canonical stake when
updating round.total_voted_stake and storing the vote in rounds (e.g., replace
use of vote.stake with the stake fetched from ValidatorSet for vote.voter and
store an updated ValidationVote or otherwise record the canonical stake in
ConsensusRound.votes/total_voted_stake); also handle the case where the voter is
unknown in ValidatorSet by returning an appropriate error.
In `@crates/wasm-runtime/src/host_functions.rs`:
- Around line 70-79: The truncation currently uses byte-slicing
(&message[..self.max_log_length]) which can panic on multi-byte UTF‑8
characters; update add_log to truncate by characters instead: check
message.chars().count() against self.max_log_length and, when exceeding, build a
truncated string with
message.chars().take(self.max_log_length).collect::<String>() (then append
"...[truncated]" as before) before pushing to self.logs so slicing never cuts a
UTF‑8 codepoint in half.
- Around line 140-295: Add a helper (e.g., validate_wasm_len or
checked_wasm_len) that takes an i32 and a max cap and returns Result<usize,
wasmtime::Error> (or Err for host_random_bytes' -1 path); use it to reject
negative or overly large lengths before allocating or creating vectors. Apply it
in host_log and host_abort for msg_len/file_len with a 16*1024 cap and return a
wasmtime::Error::msg on invalid input, and in host_random_bytes for len with a
4*1024 cap returning -1 on invalid input; use the returned safe usize for
vec/reads/writes and avoid direct cast from i32 to usize.
In `@crates/wasm-runtime/src/runtime.rs`:
- Around line 243-262: The code reads result_len and then uses result_ptr and
result_len unchecked to allocate and read from memory, allowing negative i32
values to wrap to huge usize allocations; update the logic around the
result_ptr/result_len handling to validate that both are non-negative,
cast-safe, and within memory bounds before any allocation or read (mirror the
checks performed by read_bytes_from_memory()/read_string_from_memory()),
reject/return a WasmError::MemoryError for invalid or out-of-bounds values, and
only then allocate the data buffer and call memory.read for (result_ptr + 4) and
length bytes.
- Around line 64-79: The SandboxConfig memory/table limits are not enforced at
the Store level—modify the create_store functions (in runtime.rs and module.rs)
to build and attach a Store limiter using wasmtime's StoreLimitsBuilder and
StoreLimiter APIs: construct StoreLimitsBuilder with max_memory_bytes (from
max_memory_mb), max_tables, max_table_elements, and max_memories from
SandboxConfig, call build() to create a StoreLimiter, and attach it to the Store
via store.limiter(Some(...)) so the limits are applied per-Store (keep
create_engine as-is for engine-wide config like fuel/stack); note max_globals
cannot be enforced because StoreLimits has no globals quota.
🟡 Minor comments (6)
crates/challenge-loader/src/versioning.rs-242-267 (1)
242-267:⚠️ Potential issue | 🟡 MinorClarify whether the active version can exceed the max retention cap.
prune_versions_internalkeeps an old active version even when it exceedsmax_versions_per_challenge, which contradicts the “maximum” wording. If this behavior is intentional, the docs should say so; otherwise, enforce the cap.📝 Suggested doc clarification
- /// Maximum number of versions to retain per challenge + /// Maximum number of versions to retain per challenge (active version preserved even if older) ... - /// Prune old versions, keeping only the most recent N versions + /// Prune old versions, keeping only the most recent N versions (plus active if older)crates/bittensor-integration/src/storage.rs-172-205 (1)
172-205:⚠️ Potential issue | 🟡 MinorClamp validator UID to avoid silent truncation on large nets.
A raw cast can wrap if
neuron.uidexceedsu16::MAX. Clamping keeps IDs monotonic and avoids unexpected wraparound.🔧 Suggested change
- let validator_info = ValidatorInfo { + let uid = u16::try_from(neuron.uid).unwrap_or(u16::MAX); + + let validator_info = ValidatorInfo { hotkey: hotkey_hex.clone(), coldkey: coldkey_hex.clone(), - uid: neuron.uid as u16, + uid, stake: stake_u64,crates/wasm-runtime/src/runtime.rs-183-190 (1)
183-190:⚠️ Potential issue | 🟡 MinorValidate custom SandboxConfig in execute().
executeaccepts a custom config but never validates it; an invalid config can slip through and cause inconsistent limits.crates/wasm-runtime/src/module.rs-255-257 (1)
255-257:⚠️ Potential issue | 🟡 MinorAdd validation to prevent data length truncation when converting to i32.
data.len()returnsusizewhich can exceedi32::MAX(2,147,483,647). Casting directly withas i32truncates silently, corrupting memory operations. This occurs at lines 256, 270, 274 (call_validate) and 294, 302, 306 (call_calculate_score).Replace unchecked casts with:
let len = i32::try_from(data.len()) .map_err(|_| WasmError::ExecutionError("input too large".into()))?;crates/wasm-runtime/src/sandbox.rs-149-179 (1)
149-179:⚠️ Potential issue | 🟡 MinorAdd validation for
max_cpu_secsand prevent multiplication overflow.The
validate()method doesn't checkmax_cpu_secs, leaving it uninitialized or set to zero without error. Additionally,max_cpu_secs * 1000at line 255 can overflow for large values and should usesaturating_mul()for safety, consistent with defensive programming practices used elsewhere in the codebase.🛡️ Suggested fix
pub fn validate(&self) -> Result<(), ConfigValidationError> { + if self.max_cpu_secs == 0 { + return Err(ConfigValidationError::InvalidValue( + "max_cpu_secs must be greater than 0".to_string(), + )); + } if self.max_memory_mb == 0 { return Err(ConfigValidationError::InvalidValue( "max_memory_mb must be greater than 0".to_string(), )); }- let time_limit_ms = config.max_cpu_secs * 1000; + let time_limit_ms = config.max_cpu_secs.saturating_mul(1000);crates/challenge-loader/src/loader.rs-529-567 (1)
529-567:⚠️ Potential issue | 🟡 MinorJSON config parsing truncates
u64values to smaller types without validation.In
load_config_file(), values likemechanism_idare cast fromu64tou8(line 537-538) without checking for overflow. A JSON value of300would silently truncate to44.🛡️ Proposed fix with validation
let config = ChallengeConfig { mechanism_id: json .get("mechanism_id") .and_then(|v| v.as_u64()) - .map(|v| v as u8) + .and_then(|v| u8::try_from(v).ok()) .unwrap_or(1), // ... similar for other fields that truncate
🧹 Nitpick comments (10)
crates/p2p-consensus/src/fast_consensus.rs (1)
757-807: Test doesn’t actually validate stake‑weighted aggregation.The test creates votes with different
result_hashvalues, so it never exercises the weighted average. Consider using the same result identity for both votes and asserting the expected 0.68 aggregated score.[transformer snippet omitted for brevity; can provide if helpful]
crates/challenge-loader/src/registry.rs (3)
253-305: Version history grows unbounded without any cleanup mechanism.The
update()method appends old versions to the history list without any limit. Over time with frequent hot-reloads, this could lead to memory growth sinceChallengeVersioncontainswasm_bytes: Vec<u8>.Consider adding a configurable limit on version history size (e.g., keep only the last N versions) similar to how
max_challengeslimits total challenges, or document that callers should periodically prune history.♻️ Suggested approach to limit version history
pub struct ChallengeRegistry { challenges: RwLock<HashMap<ChallengeId, LoadedChallenge>>, versions: RwLock<HashMap<ChallengeId, Vec<ChallengeVersion>>>, active_versions: RwLock<HashMap<ChallengeId, u32>>, max_challenges: usize, + max_versions_per_challenge: usize, }Then in
update():let version_record = ChallengeVersion { version: old_version, code_hash: challenge.code_hash.clone(), wasm_bytes, created_at: challenge.loaded_at, is_active: false, }; version_list.push(version_record); + + // Prune old versions if exceeding limit + while version_list.len() > self.max_versions_per_challenge { + version_list.remove(0); + }
196-212: Potential for inconsistent state if version manager removal fails after registry removal.In
unregister(), the challenge is removed fromchallengesfirst. If a concurrent operation or future code change causesversions.write().remove(id)to fail or panic, the registry would be left in an inconsistent state where the challenge is removed but version history remains.Consider restructuring to acquire all locks upfront or use a transaction-like pattern, though given current code this is low risk.
340-347:warn!level may be too severe for intentional clear operations.Using
warn!forclear()could flood logs in scenarios where clearing is intentional (e.g., shutdown, test cleanup). Consider usinginfo!ordebug!level, or adding a parameter to control whether to log.♻️ Suggested change
pub fn clear(&self) { self.challenges.write().clear(); self.versions.write().clear(); self.active_versions.write().clear(); - warn!("Challenge registry cleared"); + info!("Challenge registry cleared"); }crates/challenge-loader/src/loader.rs (2)
127-166: Stub module always returns 0.0 score which could mask issues in integration tests.The
StubWasmModule::evaluate()always returnsOk(0.0), which might not be realistic for testing scenarios where score values matter. Consider documenting this limitation prominently or making the return value configurable.
446-456: Spawned task holds references but discovery is stored after task spawn.The update handler task is spawned at line 450-456, but
compositeis moved intoself.discoveryat line 461 after the task spawn. The task usesloader.clone_for_task()which clones thediscoveryArc, but at spawn timeself.discoveryis stillNone.This means the spawned task's
loaderclone will have the oldNonediscovery reference. While this doesn't directly cause issues since the task only useshandle_discovery_update, it's worth noting this subtle ordering dependency.crates/challenge-loader/src/discovery.rs (4)
202-213: Initial receiver from channel is immediately dropped.In
FilesystemDiscovery::new(), line 205 creates a channel but the receiver is immediately dropped (let (sender, _) = mpsc::channel(100)). This means any sends toself.senderwill fail since there's no receiver.The
senderfield appears unused—broadcast_update()uses thesubscriberslist instead. Consider removing the unusedsenderfield.♻️ Proposed fix
pub struct FilesystemDiscovery { config: FilesystemDiscoveryConfig, - sender: mpsc::Sender<ChallengeUpdate>, subscribers: Arc<RwLock<Vec<mpsc::Sender<ChallengeUpdate>>>>, known_challenges: Arc<RwLock<std::collections::HashMap<PathBuf, String>>>, watching: Arc<RwLock<bool>>, } impl FilesystemDiscovery { pub fn new(config: FilesystemDiscoveryConfig) -> Self { - let (sender, _) = mpsc::channel(100); Self { config, - sender, subscribers: Arc::new(RwLock::new(Vec::new())), known_challenges: Arc::new(RwLock::new(std::collections::HashMap::new())), watching: Arc::new(RwLock::new(false)), } }Also update
clone_inner()accordingly.
332-344: Spawning tasks for each subscriber without tracking or cleanup.
broadcast_update()spawns a new task for each subscriber send operation. If a subscriber is dropped, the error is logged but the dead sender remains in thesubscriberslist. Over time, this could accumulate dead senders.♻️ Suggested approach to clean up dead subscribers
Consider removing failed senders or using a different approach:
fn broadcast_update(&self, update: ChallengeUpdate) { - let subscribers = self.subscribers.read(); - for subscriber in subscribers.iter() { - let update_clone = update.clone(); - let subscriber_clone = subscriber.clone(); - tokio::spawn(async move { - if subscriber_clone.send(update_clone).await.is_err() { - debug!("Subscriber dropped, removing from list"); - } - }); - } + let mut subscribers = self.subscribers.write(); + subscribers.retain(|subscriber| { + // try_send to avoid blocking; if channel is full or closed, remove it + !subscriber.is_closed() + }); + for subscriber in subscribers.iter() { + // Use try_send or spawn; here we keep spawn but filter closed channels above + let update_clone = update.clone(); + let subscriber_clone = subscriber.clone(); + tokio::spawn(async move { + let _ = subscriber_clone.send(update_clone).await; + }); + } }
381-393: Removal detection cannot emitChallengeUpdate::Removeddue to missing ID tracking.The comment at line 390-391 acknowledges that removal notifications are skipped because the challenge ID isn't tracked alongside the path. This means consumers won't be notified when challenge files are deleted from the filesystem.
♻️ Suggested fix to track challenge IDs
Change
known_challengesto store both hash and ID:- known_challenges: Arc<RwLock<std::collections::HashMap<PathBuf, String>>>, + known_challenges: Arc<RwLock<std::collections::HashMap<PathBuf, (String, ChallengeId)>>>,Then update the change detection and removal logic accordingly:
for path in removed_paths { - known.remove(&path); - // We don't have the ID stored, so we'd need to track it differently - // For now, we skip removal notifications - debug!(path = %path.display(), "Challenge file removed"); + if let Some((_, id)) = known.remove(&path) { + updates.push(ChallengeUpdate::Removed(id)); + info!(path = %path.display(), challenge_id = %id, "Challenge file removed"); + } }
599-617: Merged subscription spawns tasks that never terminate if source receivers stay open.In
CompositeDiscovery::subscribe(), tasks are spawned for each source that forward messages to the merged channel. These tasks only exit when the source receiver returnsNoneor the merged sender fails. If sources never close their channels, these tasks run indefinitely.This is acceptable behavior for long-running discovery, but consider adding a mechanism to cancel these forwarding tasks when
stop_watching()is called, or document this as expected behavior.
| // Initialize challenge loader | ||
| let challenge_loader = if args.wasm_challenges { | ||
| let loader_config = LoaderConfig { | ||
| challenges_dir: Some(args.challenges_dir.clone()), | ||
| enable_p2p_discovery: true, | ||
| max_challenges: 100, | ||
| ..Default::default() | ||
| }; | ||
|
|
||
| match ChallengeLoader::new(loader_config) { | ||
| Ok(loader) => { | ||
| info!("Challenge loader initialized"); | ||
| Some(Arc::new(loader)) | ||
| } | ||
| Err(e) => { | ||
| warn!( | ||
| "Failed to initialize challenge loader: {}. WASM challenges disabled.", | ||
| e | ||
| ); | ||
| None | ||
| } | ||
| } | ||
| } else { | ||
| info!("WASM challenges disabled"); | ||
| None | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "start_discovery" --type rsRepository: PlatformNetwork/platform-v2
Length of output: 97
🏁 Script executed:
# Find the ChallengeLoader struct definition and its methods
fd "challenge" --type f -e rs | head -20Repository: PlatformNetwork/platform-v2
Length of output: 289
🏁 Script executed:
# Look for ChallengeLoader implementation
ast-grep --pattern 'impl $_ {
$$$
fn start_discovery($_) {
$$$
}
$$$
}'Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Find challenge-loader crate
fd "challenge-loader" -type dRepository: PlatformNetwork/platform-v2
Length of output: 241
🏁 Script executed:
# Search for start_discovery without file type restriction
rg "start_discovery"Repository: PlatformNetwork/platform-v2
Length of output: 346
🏁 Script executed:
# Find ChallengeLoader struct definition
rg "struct ChallengeLoader" -A 10Repository: PlatformNetwork/platform-v2
Length of output: 857
🏁 Script executed:
# Read the loader.rs file to understand start_discovery implementation
cat crates/challenge-loader/src/loader.rs | head -200Repository: PlatformNetwork/platform-v2
Length of output: 5922
🏁 Script executed:
# Look for the new() method implementation
rg -A 30 "pub fn new\(" crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 1066
🏁 Script executed:
# Check what start_discovery does
rg -A 20 "pub async fn start_discovery" crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 926
🏁 Script executed:
# Get the complete start_discovery implementation
rg -A 50 "pub async fn start_discovery" crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 2006
🏁 Script executed:
# Check if there are any other places where start_discovery is called on ChallengeLoader instances
rg "start_discovery\(\)" -B 2 -A 2Repository: PlatformNetwork/platform-v2
Length of output: 743
🏁 Script executed:
# Check the context of main.rs around lines 197-223
cat -n crates/validator-node/src/main.rs | sed -n '180,240p'Repository: PlatformNetwork/platform-v2
Length of output: 136
🏁 Script executed:
# Look for the function that contains this code
rg -B 20 "Initialize challenge loader" crates/validator-node/src/main.rs | head -40Repository: PlatformNetwork/platform-v2
Length of output: 144
🏁 Script executed:
# Find the validator-node main.rs file
fd "main.rs" | grep -i validatorRepository: PlatformNetwork/platform-v2
Length of output: 102
🏁 Script executed:
# Look for validator-node directory
fd "validator-node" -type dRepository: PlatformNetwork/platform-v2
Length of output: 241
🏁 Script executed:
# Read the validator-node main.rs file around the specified lines
cat -n bins/validator-node/src/main.rs | sed -n '180,240p'Repository: PlatformNetwork/platform-v2
Length of output: 2349
🏁 Script executed:
# Check the complete context of the function containing the challenge loader initialization
rg -B 30 "Initialize challenge loader" bins/validator-node/src/main.rs | head -60Repository: PlatformNetwork/platform-v2
Length of output: 945
🏁 Script executed:
# Look for how the challenge_loader is used after initialization
rg -A 50 "Initialize challenge loader" bins/validator-node/src/main.rs | head -80Repository: PlatformNetwork/platform-v2
Length of output: 1680
🏁 Script executed:
# Check if challenge_loader is used anywhere else in the code
rg "challenge_loader" bins/validator-node/src/main.rs -A 3 -B 1Repository: PlatformNetwork/platform-v2
Length of output: 818
🏁 Script executed:
# Search for any calls to start_discovery on challenge_loader in the entire validator-node
rg "challenge_loader.*start_discovery" bins/validator-node/Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Check what methods are called on challenge_loader
rg "challenge_loader\." bins/validator-node/src/ -A 1Repository: PlatformNetwork/platform-v2
Length of output: 263
🏁 Script executed:
# Check the complete context of the discovery interval loop to see full usage
rg -B 10 -A 10 "challenge_discovery_interval" bins/validator-node/src/main.rsRepository: PlatformNetwork/platform-v2
Length of output: 2017
🏁 Script executed:
# See if there's any initialization after the challenge_loader assignment
cat -n bins/validator-node/src/main.rs | sed -n '197,350p' | grep -A 150 "Initialize challenge loader"Repository: PlatformNetwork/platform-v2
Length of output: 6466
🏁 Script executed:
# Check if list_challenges requires discovery to be running
rg -A 20 "pub fn list_challenges" crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 768
🏁 Script executed:
# Check if there are any other methods that would populate challenges
rg "pub.*fn.*challenge" crates/challenge-loader/src/loader.rs | head -20Repository: PlatformNetwork/platform-v2
Length of output: 489
🏁 Script executed:
# Verify that the registry is empty on initialization
rg -A 10 "pub fn with_capacity" crates/challenge-loader/src/ | grep -A 10 registryRepository: PlatformNetwork/platform-v2
Length of output: 831
🏁 Script executed:
# Check if there's any initialization code that might auto-load challenges
rg "fn new\|impl.*ChallengeRegistry" crates/challenge-loader/src/ -A 15 | head -50Repository: PlatformNetwork/platform-v2
Length of output: 53
Call start_discovery() on the ChallengeLoader after initialization.
The loader is created with enable_p2p_discovery: true and a challenges_dir, but start_discovery() is never invoked. Without this call, the discovery sources are never registered, no initial discovery occurs, and list_challenges() will perpetually return an empty list. The polling in the main loop will never find any challenges.
Suggested fix
match ChallengeLoader::new(loader_config) {
Ok(loader) => {
+ if let Err(e) = loader.start_discovery().await {
+ warn!("Failed to start challenge discovery: {}", e);
+ }
info!("Challenge loader initialized");
Some(Arc::new(loader))
}🤖 Prompt for AI Agents
In `@bins/validator-node/src/main.rs` around lines 197 - 223, The ChallengeLoader
is initialized with enable_p2p_discovery but never started; after constructing
the loader in the Ok(loader) arm (the local variable named loader used to create
Some(Arc::new(loader))), call loader.start_discovery() and handle its Result
(log an error/warn and return None or proceed on Ok) so discovery sources are
registered and list_challenges() will populate; update the Ok branch around
ChallengeLoader::new(...) to invoke start_discovery() before wrapping the loader
in Arc and adjust logging on failure.
| // Initialize fast validation consensus | ||
| let fast_consensus_config = FastConsensusConfig { | ||
| finality_threshold: 0.67, | ||
| vote_timeout: Duration::from_secs(5), | ||
| max_score_variance: 0.1, | ||
| }; | ||
| let _fast_consensus = Arc::new(RwLock::new(FastConsensus::new( | ||
| keypair.clone(), | ||
| validator_set.clone(), | ||
| fast_consensus_config, | ||
| ))); | ||
| info!("Fast validation consensus initialized"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FastConsensus is created but not wired to Evaluation messages.
_fast_consensus is never used and evaluation messages are only logged, so no vote aggregation occurs.
Also applies to: 735-742
🤖 Prompt for AI Agents
In `@bins/validator-node/src/main.rs` around lines 296 - 307, FastConsensus is
instantiated as _fast_consensus but never used, so Evaluation messages are only
logged and no vote aggregation happens; locate where Evaluation messages are
handled (the processing loop or handler that currently logs evaluations) and
wire in the FastConsensus instance by storing Arc<RwLock<FastConsensus>> (the
_fast_consensus you created) into the surrounding state/context, then replace
the simple log path with calls into FastConsensus methods (e.g.,
FastConsensus::record_evaluation or FastConsensus::handle_evaluation — whatever
method on FastConsensus aggregates votes) so incoming Evaluation messages are
forwarded to _fast_consensus for vote aggregation and consensus progression;
ensure you clone the Arc when moving into async handlers and acquire the RwLock
appropriately before calling the aggregation methods.
| // Update validator assignment epoch seed | ||
| { | ||
| let mut seed = [0u8; 32]; | ||
| seed[..8].copy_from_slice(&new_epoch.to_le_bytes()); | ||
| seed[8..16].copy_from_slice(&block.to_le_bytes()); | ||
| validator_assignment | ||
| .write() | ||
| .update_config(AssignmentConfig { | ||
| epoch_seed: seed, | ||
| ..Default::default() | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find AssignmentConfig definition and related code
rg -n "struct AssignmentConfig|impl.*Default.*AssignmentConfig|fn update_config" --type rs -A 10 -B 2Repository: PlatformNetwork/platform-v2
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Find AssignmentConfig definition and related code
rg -n "struct AssignmentConfig|impl.*Default.*AssignmentConfig|fn update_config" crates/ -A 10 -B 2Repository: PlatformNetwork/platform-v2
Length of output: 3475
🏁 Script executed:
#!/bin/bash
# Check the actual code context in the validator-node main.rs file
sed -n '790,820p' bins/validator-node/src/main.rsRepository: PlatformNetwork/platform-v2
Length of output: 1162
Preserve existing validator assignment config during epoch transitions.
Calling update_config(AssignmentConfig { epoch_seed: seed, ..Default::default() }) replaces the entire configuration, resetting min_validators to 3, max_validators to 10, and stake_weighted to true on every epoch transition. This overwrites any previously tuned values. Clone the existing config and update only the epoch_seed field:
Suggested fix
- validator_assignment
- .write()
- .update_config(AssignmentConfig {
- epoch_seed: seed,
- ..Default::default()
- });
+ let mut cfg = validator_assignment.read().config().clone();
+ cfg.epoch_seed = seed;
+ validator_assignment.write().update_config(cfg);📝 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.
| // Update validator assignment epoch seed | |
| { | |
| let mut seed = [0u8; 32]; | |
| seed[..8].copy_from_slice(&new_epoch.to_le_bytes()); | |
| seed[8..16].copy_from_slice(&block.to_le_bytes()); | |
| validator_assignment | |
| .write() | |
| .update_config(AssignmentConfig { | |
| epoch_seed: seed, | |
| ..Default::default() | |
| }); | |
| // Update validator assignment epoch seed | |
| { | |
| let mut seed = [0u8; 32]; | |
| seed[..8].copy_from_slice(&new_epoch.to_le_bytes()); | |
| seed[8..16].copy_from_slice(&block.to_le_bytes()); | |
| let mut cfg = validator_assignment.read().config().clone(); | |
| cfg.epoch_seed = seed; | |
| validator_assignment.write().update_config(cfg); |
🤖 Prompt for AI Agents
In `@bins/validator-node/src/main.rs` around lines 798 - 808, The current call to
validator_assignment.write().update_config(AssignmentConfig { epoch_seed: seed,
..Default::default() }) replaces the whole AssignmentConfig and resets tuned
fields; instead, read or clone the existing AssignmentConfig from
validator_assignment, modify only its epoch_seed with the new seed, and pass
that updated config into update_config so min_validators, max_validators,
stake_weighted, etc. are preserved (use the existing validator_assignment
read/clone, update epoch_seed, then write().update_config(updated_config)).
| #[tokio::test] | ||
| async fn test_loader_lifecycle() { | ||
| use crate::prelude::*; | ||
|
|
||
| // Create loader | ||
| let loader = ChallengeLoader::default_loader().expect("create loader"); | ||
|
|
||
| // Load a challenge | ||
| let id = ChallengeId::new(); | ||
| let wasm = vec![0u8; 100]; // Minimal WASM | ||
|
|
||
| loader | ||
| .load_challenge(id, "test".to_string(), wasm, ChallengeConfig::default()) | ||
| .await | ||
| .expect("load"); | ||
|
|
||
| // Verify loaded | ||
| assert_eq!(loader.challenge_count(), 1); | ||
| assert!(loader.get_challenge(&id).is_some()); | ||
|
|
||
| // Unload | ||
| loader.unload_challenge(&id).await.expect("unload"); | ||
| assert_eq!(loader.challenge_count(), 0); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the load_challenge implementation
rg -n "fn load_challenge" crates -A 20Repository: PlatformNetwork/platform-v2
Length of output: 5565
🏁 Script executed:
#!/bin/bash
# Get the complete load_challenge implementation
sed -n '214,280p' crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 2110
🏁 Script executed:
#!/bin/bash
# Find compile_wasm implementation
rg -n "fn compile_wasm" crates/challenge-loader -A 15Repository: PlatformNetwork/platform-v2
Length of output: 1233
🏁 Script executed:
#!/bin/bash
# Get more context on compile_wasm to confirm it's truly a stub
sed -n '607,650p' crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 1259
🏁 Script executed:
#!/bin/bash
# Check if there are existing WASM fixtures or sample_wasm_bytes implementations
rg -n "sample_wasm_bytes\|wasm_bytes\(\)" crates/challenge-loader/src/lib.rs -A 5Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Look at the test module in loader.rs to see what fixtures exist
sed -n '640,700p' crates/challenge-loader/src/loader.rsRepository: PlatformNetwork/platform-v2
Length of output: 1748
🏁 Script executed:
#!/bin/bash
# Check the actual test_loader_lifecycle in lib.rs
sed -n '138,165p' crates/challenge-loader/src/lib.rsRepository: PlatformNetwork/platform-v2
Length of output: 907
Use valid WASM bytes in the loader lifecycle test.
The test uses vec![0u8; 100], which is not valid WASM (just 100 zero bytes). While the current stub implementation doesn't validate WASM, this test should use proper bytes to ensure it won't mask real validation failures when the module is fully implemented. A valid minimal WASM module header already exists as sample_wasm_bytes() in loader.rs.
Suggested fix
- let wasm = vec![0u8; 100]; // Minimal WASM
+ // Minimal valid WASM module (empty module with header)
+ let wasm = vec![0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00];📝 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.
| #[tokio::test] | |
| async fn test_loader_lifecycle() { | |
| use crate::prelude::*; | |
| // Create loader | |
| let loader = ChallengeLoader::default_loader().expect("create loader"); | |
| // Load a challenge | |
| let id = ChallengeId::new(); | |
| let wasm = vec![0u8; 100]; // Minimal WASM | |
| loader | |
| .load_challenge(id, "test".to_string(), wasm, ChallengeConfig::default()) | |
| .await | |
| .expect("load"); | |
| // Verify loaded | |
| assert_eq!(loader.challenge_count(), 1); | |
| assert!(loader.get_challenge(&id).is_some()); | |
| // Unload | |
| loader.unload_challenge(&id).await.expect("unload"); | |
| assert_eq!(loader.challenge_count(), 0); | |
| } | |
| #[tokio::test] | |
| async fn test_loader_lifecycle() { | |
| use crate::prelude::*; | |
| // Create loader | |
| let loader = ChallengeLoader::default_loader().expect("create loader"); | |
| // Load a challenge | |
| let id = ChallengeId::new(); | |
| // Minimal valid WASM module (empty module with header) | |
| let wasm = vec![0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]; | |
| loader | |
| .load_challenge(id, "test".to_string(), wasm, ChallengeConfig::default()) | |
| .await | |
| .expect("load"); | |
| // Verify loaded | |
| assert_eq!(loader.challenge_count(), 1); | |
| assert!(loader.get_challenge(&id).is_some()); | |
| // Unload | |
| loader.unload_challenge(&id).await.expect("unload"); | |
| assert_eq!(loader.challenge_count(), 0); | |
| } |
🤖 Prompt for AI Agents
In `@crates/challenge-loader/src/lib.rs` around lines 138 - 161, The
test_loader_lifecycle uses invalid WASM bytes (vec![0u8; 100]) which can mask
future validation; update the test to use the valid minimal WASM provided by the
loader module by replacing the placeholder bytes with a call to the existing
sample_wasm_bytes() helper (or the equivalent function defined in loader.rs)
when creating `wasm`, keeping the rest of the test
(ChallengeLoader::default_loader, load_challenge, unload_challenge, assertions)
unchanged.
| // Update registry (stores old version in history) | ||
| let old_version = self.registry.update( | ||
| id, | ||
| new_version, | ||
| new_code_hash.clone(), | ||
| new_module, | ||
| new_wasm.clone(), | ||
| )?; | ||
|
|
||
| // Register new version | ||
| let version_record = ChallengeVersion::new(new_version, new_code_hash.clone(), new_wasm); | ||
| self.version_manager.register_version(*id, version_record)?; | ||
| self.version_manager.activate_version(id, new_version)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Passing new WASM bytes as the old version's history bytes.
In hot_reload(), registry.update() receives new_wasm.clone() as the wasm_bytes parameter. However, looking at registry.rs lines 277-284, this parameter is stored as the old version's wasm_bytes in the version history.
This means when you hot-reload from v1→v2, the history will incorrectly store v2's bytes under v1's version record, making rollback impossible.
🐛 Proposed fix
You need to preserve the old WASM bytes before calling update. However, the current architecture doesn't store the original WASM bytes in LoadedChallenge. Consider one of these approaches:
Option 1: Store wasm_bytes in LoadedChallenge and pass the old bytes:
// Get current challenge
let current = self
.registry
.get(id)
.ok_or_else(|| LoaderError::ChallengeNotFound(format!("Challenge {} not found", id)))?;
+// Get old WASM bytes from version manager for history
+let old_wasm_bytes = self.version_manager
+ .get_version(id, current.version)
+ .map(|v| v.wasm_bytes.clone())
+ .unwrap_or_default();
// Update registry (stores old version in history)
let old_version = self.registry.update(
id,
new_version,
new_code_hash.clone(),
new_module,
- new_wasm.clone(),
+ old_wasm_bytes,
)?;Option 2: Restructure so registry doesn't need the old bytes (version_manager already has them).
🤖 Prompt for AI Agents
In `@crates/challenge-loader/src/loader.rs` around lines 346 - 358, hot_reload()
is passing new_wasm.clone() into registry.update(), causing the registry to
store the new version bytes as the old-version history; capture and pass the
actual old WASM bytes instead. Before calling self.registry.update(...) in
hot_reload(), retrieve the current/old wasm bytes from the loaded challenge
state (e.g., add wasm_bytes to LoadedChallenge and read it) or fetch them from
the version_manager/ChallengeVersion record, then pass that old_bytes variable
to registry.update(...). Ensure you still construct and register the new
ChallengeVersion (register_version/activate_version) using new_wasm after
updating the registry.
| fn handle_vote_internal( | ||
| &self, | ||
| rounds: &mut HashMap<[u8; 32], ConsensusRound>, | ||
| vote: ValidationVote, | ||
| ) -> Result<Option<FinalizedResult>, FastConsensusError> { | ||
| // Verify signature | ||
| if !vote.verify(&self.validator_set)? { | ||
| return Err(FastConsensusError::InvalidSignature(vote.voter.to_hex())); | ||
| } | ||
|
|
||
| // Get or create round | ||
| let round = rounds | ||
| .entry(vote.result_hash) | ||
| .or_insert_with(|| ConsensusRound::new(vote.result_hash)); | ||
|
|
||
| // Already finalized? | ||
| if round.finalized { | ||
| debug!( | ||
| result_hash = hex::encode(vote.result_hash), | ||
| "Round already finalized, returning existing result" | ||
| ); | ||
| return Ok(round.final_result.clone()); | ||
| } | ||
|
|
||
| // Check for timeout | ||
| if round.is_timed_out(self.config.vote_timeout) { | ||
| warn!( | ||
| result_hash = hex::encode(vote.result_hash), | ||
| "Vote received for timed out round" | ||
| ); | ||
| return Err(FastConsensusError::Timeout); | ||
| } | ||
|
|
||
| // Already voted? | ||
| if round.votes.contains_key(&vote.voter) { | ||
| return Err(FastConsensusError::AlreadyVoted(vote.voter.to_hex())); | ||
| } | ||
|
|
||
| debug!( | ||
| voter = vote.voter.to_hex(), | ||
| stake = vote.stake, | ||
| result_hash = hex::encode(vote.result_hash), | ||
| "Processing vote" | ||
| ); | ||
|
|
||
| // Add vote | ||
| round.total_voted_stake = round.total_voted_stake.saturating_add(vote.stake); | ||
| round.votes.insert(vote.voter.clone(), vote); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not trust self‑reported stake in votes.
vote.stake is untrusted input. A validator can inflate its weight unless you derive stake from ValidatorSet and overwrite the vote.
🔧 Suggested change
- fn handle_vote_internal(
- &self,
- rounds: &mut HashMap<[u8; 32], ConsensusRound>,
- vote: ValidationVote,
- ) -> Result<Option<FinalizedResult>, FastConsensusError> {
+ fn handle_vote_internal(
+ &self,
+ rounds: &mut HashMap<[u8; 32], ConsensusRound>,
+ mut vote: ValidationVote,
+ ) -> Result<Option<FinalizedResult>, FastConsensusError> {
// Verify signature
if !vote.verify(&self.validator_set)? {
return Err(FastConsensusError::InvalidSignature(vote.voter.to_hex()));
}
+
+ let actual_stake = self
+ .validator_set
+ .get_validator(&vote.voter)
+ .map(|v| v.stake)
+ .unwrap_or(0);
+ vote.stake = actual_stake;🤖 Prompt for AI Agents
In `@crates/p2p-consensus/src/fast_consensus.rs` around lines 321 - 368, In
handle_vote_internal, do not trust vote.stake (untrusted input); instead look up
the voter's stake from the validator set (use self.validator_set or its
accessor) and use/overwrite that canonical stake when updating
round.total_voted_stake and storing the vote in rounds (e.g., replace use of
vote.stake with the stake fetched from ValidatorSet for vote.voter and store an
updated ValidationVote or otherwise record the canonical stake in
ConsensusRound.votes/total_voted_stake); also handle the case where the voter is
unknown in ValidatorSet by returning an appropriate error.
| /// Add a log message from the WASM module | ||
| pub fn add_log(&mut self, message: String) { | ||
| if self.logs.len() < self.max_log_entries { | ||
| let truncated = if message.len() > self.max_log_length { | ||
| format!("{}...[truncated]", &message[..self.max_log_length]) | ||
| } else { | ||
| message | ||
| }; | ||
| self.logs.push(truncated); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n crates/wasm-runtime/src/host_functions.rs | head -100Repository: PlatformNetwork/platform-v2
Length of output: 3611
🏁 Script executed:
rg -n "max_log_length" crates/wasm-runtime/src/Repository: PlatformNetwork/platform-v2
Length of output: 538
🏁 Script executed:
rg -n "add_log" crates/wasm-runtime/src/Repository: PlatformNetwork/platform-v2
Length of output: 1130
🏁 Script executed:
sed -n '155,165p' crates/wasm-runtime/src/host_functions.rsRepository: PlatformNetwork/platform-v2
Length of output: 468
🏁 Script executed:
sed -n '470,485p' crates/wasm-runtime/src/host_functions.rsRepository: PlatformNetwork/platform-v2
Length of output: 539
🏁 Script executed:
sed -n '140,170p' crates/wasm-runtime/src/host_functions.rsRepository: PlatformNetwork/platform-v2
Length of output: 1312
Fix unsafe UTF‑8 string slicing in log truncation.
The byte-level slicing at line 74 (&message[..self.max_log_length]) panics if max_log_length cuts through a multi-byte UTF-8 character. The message input comes from untrusted WASM data processed via String::from_utf8_lossy(), which can contain arbitrary valid UTF-8 sequences including multi-byte characters (e.g., emoji). Use character-based truncation instead:
Suggested fix
- let truncated = if message.len() > self.max_log_length {
- format!("{}...[truncated]", &message[..self.max_log_length])
+ let truncated = if message.len() > self.max_log_length {
+ let safe: String = message.chars().take(self.max_log_length).collect();
+ format!("{}...[truncated]", safe)
} else {
message
};📝 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.
| /// Add a log message from the WASM module | |
| pub fn add_log(&mut self, message: String) { | |
| if self.logs.len() < self.max_log_entries { | |
| let truncated = if message.len() > self.max_log_length { | |
| format!("{}...[truncated]", &message[..self.max_log_length]) | |
| } else { | |
| message | |
| }; | |
| self.logs.push(truncated); | |
| } | |
| /// Add a log message from the WASM module | |
| pub fn add_log(&mut self, message: String) { | |
| if self.logs.len() < self.max_log_entries { | |
| let truncated = if message.len() > self.max_log_length { | |
| let safe: String = message.chars().take(self.max_log_length).collect(); | |
| format!("{}...[truncated]", safe) | |
| } else { | |
| message | |
| }; | |
| self.logs.push(truncated); | |
| } |
🤖 Prompt for AI Agents
In `@crates/wasm-runtime/src/host_functions.rs` around lines 70 - 79, The
truncation currently uses byte-slicing (&message[..self.max_log_length]) which
can panic on multi-byte UTF‑8 characters; update add_log to truncate by
characters instead: check message.chars().count() against self.max_log_length
and, when exceeding, build a truncated string with
message.chars().take(self.max_log_length).collect::<String>() (then append
"...[truncated]" as before) before pushing to self.logs so slicing never cuts a
UTF‑8 codepoint in half.
| linker | ||
| .func_wrap( | ||
| "env", | ||
| "host_log", | ||
| |mut caller: Caller<'_, SharedHostState>, ptr: i32, len: i32| -> WasmtimeResult<()> { | ||
| let state = caller.data().clone(); | ||
| let memory = caller | ||
| .get_export("memory") | ||
| .and_then(|e| e.into_memory()) | ||
| .ok_or_else(|| wasmtime::Error::msg("failed to find memory export"))?; | ||
|
|
||
| let mut buffer = vec![0u8; len as usize]; | ||
| memory.read(&caller, ptr as usize, &mut buffer)?; | ||
|
|
||
| let message = String::from_utf8_lossy(&buffer).to_string(); | ||
|
|
||
| let mut state_guard = state | ||
| .lock() | ||
| .map_err(|e| wasmtime::Error::msg(format!("failed to lock state: {}", e)))?; | ||
| state_guard.resource_usage_mut().record_host_call(); | ||
| state_guard.add_log(message.clone()); | ||
|
|
||
| debug!(target: "wasm", "WASM log: {}", message); | ||
|
|
||
| Ok(()) | ||
| }, | ||
| ) | ||
| .map_err(|e| WasmError::HostFunctionError(format!("failed to register host_log: {}", e)))?; | ||
|
|
||
| // host_get_timestamp() -> i64 | ||
| // Returns the current Unix timestamp in seconds | ||
| linker | ||
| .func_wrap( | ||
| "env", | ||
| "host_get_timestamp", | ||
| |caller: Caller<'_, SharedHostState>| -> i64 { | ||
| let state = caller.data().clone(); | ||
| if let Ok(mut guard) = state.lock() { | ||
| guard.resource_usage_mut().record_host_call(); | ||
| } | ||
|
|
||
| let timestamp = chrono::Utc::now().timestamp(); | ||
| trace!(target: "wasm", "WASM timestamp request: {}", timestamp); | ||
| timestamp | ||
| }, | ||
| ) | ||
| .map_err(|e| { | ||
| WasmError::HostFunctionError(format!("failed to register host_get_timestamp: {}", e)) | ||
| })?; | ||
|
|
||
| // host_get_timestamp_millis() -> i64 | ||
| // Returns the current Unix timestamp in milliseconds | ||
| linker | ||
| .func_wrap( | ||
| "env", | ||
| "host_get_timestamp_millis", | ||
| |caller: Caller<'_, SharedHostState>| -> i64 { | ||
| let state = caller.data().clone(); | ||
| if let Ok(mut guard) = state.lock() { | ||
| guard.resource_usage_mut().record_host_call(); | ||
| } | ||
|
|
||
| let timestamp = chrono::Utc::now().timestamp_millis(); | ||
| trace!(target: "wasm", "WASM timestamp_millis request: {}", timestamp); | ||
| timestamp | ||
| }, | ||
| ) | ||
| .map_err(|e| { | ||
| WasmError::HostFunctionError(format!( | ||
| "failed to register host_get_timestamp_millis: {}", | ||
| e | ||
| )) | ||
| })?; | ||
|
|
||
| // host_random_bytes(ptr: i32, len: i32) -> i32 | ||
| // Fills the buffer at ptr with len random bytes (deterministic from seed) | ||
| // Returns 0 on success, -1 on error | ||
| linker | ||
| .func_wrap( | ||
| "env", | ||
| "host_random_bytes", | ||
| |mut caller: Caller<'_, SharedHostState>, ptr: i32, len: i32| -> i32 { | ||
| let state = caller.data().clone(); | ||
|
|
||
| let memory = match caller.get_export("memory").and_then(|e| e.into_memory()) { | ||
| Some(m) => m, | ||
| None => return -1, | ||
| }; | ||
|
|
||
| let mut state_guard = match state.lock() { | ||
| Ok(g) => g, | ||
| Err(_) => return -1, | ||
| }; | ||
|
|
||
| state_guard.resource_usage_mut().record_host_call(); | ||
|
|
||
| let bytes = state_guard.random_bytes(len as usize); | ||
|
|
||
| match memory.write(&mut caller, ptr as usize, &bytes) { | ||
| Ok(()) => { | ||
| trace!(target: "wasm", "WASM random_bytes: {} bytes", len); | ||
| 0 | ||
| } | ||
| Err(_) => -1, | ||
| } | ||
| }, | ||
| ) | ||
| .map_err(|e| { | ||
| WasmError::HostFunctionError(format!("failed to register host_random_bytes: {}", e)) | ||
| })?; | ||
|
|
||
| // host_abort(msg_ptr: i32, msg_len: i32, file_ptr: i32, file_len: i32, line: i32, col: i32) | ||
| // Abort execution with an error message (used by AssemblyScript and other languages) | ||
| linker | ||
| .func_wrap( | ||
| "env", | ||
| "host_abort", | ||
| |mut caller: Caller<'_, SharedHostState>, | ||
| msg_ptr: i32, | ||
| msg_len: i32, | ||
| file_ptr: i32, | ||
| file_len: i32, | ||
| line: i32, | ||
| col: i32| | ||
| -> WasmtimeResult<()> { | ||
| let memory = caller | ||
| .get_export("memory") | ||
| .and_then(|e| e.into_memory()) | ||
| .ok_or_else(|| wasmtime::Error::msg("failed to find memory export"))?; | ||
|
|
||
| let mut msg_buffer = vec![0u8; msg_len as usize]; | ||
| memory.read(&caller, msg_ptr as usize, &mut msg_buffer)?; | ||
| let msg = String::from_utf8_lossy(&msg_buffer); | ||
|
|
||
| let mut file_buffer = vec![0u8; file_len as usize]; | ||
| memory.read(&caller, file_ptr as usize, &mut file_buffer)?; | ||
| let file = String::from_utf8_lossy(&file_buffer); | ||
|
|
||
| tracing::error!( | ||
| target: "wasm", | ||
| "WASM abort: {} at {}:{}:{}", | ||
| msg, | ||
| file, | ||
| line, | ||
| col | ||
| ); | ||
|
|
||
| Err(wasmtime::Error::msg(format!( | ||
| "WASM abort: {} at {}:{}:{}", | ||
| msg, file, line, col | ||
| ))) | ||
| }, | ||
| ) | ||
| .map_err(|e| { | ||
| WasmError::HostFunctionError(format!("failed to register host_abort: {}", e)) | ||
| })?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n crates/wasm-runtime/src/host_functions.rs | sed -n '140,300p'Repository: PlatformNetwork/platform-v2
Length of output: 7067
🏁 Script executed:
# Check if there are any existing bounds checks or validation functions
rg "checked_len|MAX_.*BYTES|length.*valid" crates/wasm-runtime/src/host_functions.rsRepository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Look for other potential length-related security issues
rg "vec!\[0u8" crates/wasm-runtime/src/Repository: PlatformNetwork/platform-v2
Length of output: 740
🏁 Script executed:
# Check what types are used for lengths and how they're handled
rg "-> i32|-> i64" crates/wasm-runtime/src/host_functions.rs | head -20Repository: PlatformNetwork/platform-v2
Length of output: 475
🏁 Script executed:
# Find the random_bytes method implementation
rg "fn random_bytes" crates/wasm-runtime/src/Repository: PlatformNetwork/platform-v2
Length of output: 172
🏁 Script executed:
# Check the SharedHostState and state_guard structure
rg "struct SharedHostState|impl SharedHostState" crates/wasm-runtime/src/ -A 5Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Check resource_usage to see if there are caps
rg "resource_usage|record_host_call" crates/wasm-runtime/src/ -B 2 -A 2Repository: PlatformNetwork/platform-v2
Length of output: 7700
🏁 Script executed:
# Verify how casting negative i32 to usize behaves
cat > /tmp/test_cast.rs << 'EOF'
fn main() {
let neg: i32 = -1;
let cast_to_usize = neg as usize;
println!("Casting -1 as i32 to usize: {}", cast_to_usize);
println!("On 64-bit system would be: {}", u64::MAX);
}
EOF
rustc /tmp/test_cast.rs -o /tmp/test_cast && /tmp/test_castRepository: PlatformNetwork/platform-v2
Length of output: 117
🏁 Script executed:
# Find full random_bytes implementation
rg "fn random_bytes" crates/wasm-runtime/src/ -A 10Repository: PlatformNetwork/platform-v2
Length of output: 819
🏁 Script executed:
# Find ResourceUsage structure and any caps
rg "struct ResourceUsage" crates/wasm-runtime/src/ -A 20Repository: PlatformNetwork/platform-v2
Length of output: 1282
🏁 Script executed:
# Check for any memory allocation limits or buffer size checks
rg "MAX.*BYTES|MAX.*BUFFER|buffer.*size|allocation.*limit" crates/wasm-runtime/src/Repository: PlatformNetwork/platform-v2
Length of output: 564
Add bounds checks on untrusted WASM length parameters to prevent OOM.
The len parameters in host_log, host_random_bytes, and host_abort are cast directly to usize without validation. A negative i32 cast to usize becomes a huge value (e.g., -1 → usize::MAX), causing vec![0u8; len] to attempt massive allocation and crash the host process.
Add a validation helper and reasonable caps (16KB for messages, 4KB for RNG):
Suggested fix
+const MAX_HOST_MSG_BYTES: usize = 16 * 1024;
+const MAX_RNG_BYTES: usize = 4 * 1024;
+
+fn checked_len(len: i32, max: usize) -> WasmtimeResult<usize> {
+ if len < 0 {
+ return Err(wasmtime::Error::msg("negative length"));
+ }
+ let len = len as usize;
+ if len > max {
+ return Err(wasmtime::Error::msg("length too large"));
+ }
+ Ok(len)
+}
@@
- let mut buffer = vec![0u8; len as usize];
+ let len = checked_len(len, MAX_HOST_MSG_BYTES)?;
+ let mut buffer = vec![0u8; len];
@@
- let bytes = state_guard.random_bytes(len as usize);
+ let len = match checked_len(len, MAX_RNG_BYTES) {
+ Ok(v) => v,
+ Err(_) => return -1,
+ };
+ let bytes = state_guard.random_bytes(len);
@@
- let mut msg_buffer = vec![0u8; msg_len as usize];
+ let msg_len = checked_len(msg_len, MAX_HOST_MSG_BYTES)?;
+ let mut msg_buffer = vec![0u8; msg_len];
@@
- let mut file_buffer = vec![0u8; file_len as usize];
+ let file_len = checked_len(file_len, MAX_HOST_MSG_BYTES)?;
+ let mut file_buffer = vec![0u8; file_len];📝 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.
| linker | |
| .func_wrap( | |
| "env", | |
| "host_log", | |
| |mut caller: Caller<'_, SharedHostState>, ptr: i32, len: i32| -> WasmtimeResult<()> { | |
| let state = caller.data().clone(); | |
| let memory = caller | |
| .get_export("memory") | |
| .and_then(|e| e.into_memory()) | |
| .ok_or_else(|| wasmtime::Error::msg("failed to find memory export"))?; | |
| let mut buffer = vec![0u8; len as usize]; | |
| memory.read(&caller, ptr as usize, &mut buffer)?; | |
| let message = String::from_utf8_lossy(&buffer).to_string(); | |
| let mut state_guard = state | |
| .lock() | |
| .map_err(|e| wasmtime::Error::msg(format!("failed to lock state: {}", e)))?; | |
| state_guard.resource_usage_mut().record_host_call(); | |
| state_guard.add_log(message.clone()); | |
| debug!(target: "wasm", "WASM log: {}", message); | |
| Ok(()) | |
| }, | |
| ) | |
| .map_err(|e| WasmError::HostFunctionError(format!("failed to register host_log: {}", e)))?; | |
| // host_get_timestamp() -> i64 | |
| // Returns the current Unix timestamp in seconds | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_get_timestamp", | |
| |caller: Caller<'_, SharedHostState>| -> i64 { | |
| let state = caller.data().clone(); | |
| if let Ok(mut guard) = state.lock() { | |
| guard.resource_usage_mut().record_host_call(); | |
| } | |
| let timestamp = chrono::Utc::now().timestamp(); | |
| trace!(target: "wasm", "WASM timestamp request: {}", timestamp); | |
| timestamp | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!("failed to register host_get_timestamp: {}", e)) | |
| })?; | |
| // host_get_timestamp_millis() -> i64 | |
| // Returns the current Unix timestamp in milliseconds | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_get_timestamp_millis", | |
| |caller: Caller<'_, SharedHostState>| -> i64 { | |
| let state = caller.data().clone(); | |
| if let Ok(mut guard) = state.lock() { | |
| guard.resource_usage_mut().record_host_call(); | |
| } | |
| let timestamp = chrono::Utc::now().timestamp_millis(); | |
| trace!(target: "wasm", "WASM timestamp_millis request: {}", timestamp); | |
| timestamp | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!( | |
| "failed to register host_get_timestamp_millis: {}", | |
| e | |
| )) | |
| })?; | |
| // host_random_bytes(ptr: i32, len: i32) -> i32 | |
| // Fills the buffer at ptr with len random bytes (deterministic from seed) | |
| // Returns 0 on success, -1 on error | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_random_bytes", | |
| |mut caller: Caller<'_, SharedHostState>, ptr: i32, len: i32| -> i32 { | |
| let state = caller.data().clone(); | |
| let memory = match caller.get_export("memory").and_then(|e| e.into_memory()) { | |
| Some(m) => m, | |
| None => return -1, | |
| }; | |
| let mut state_guard = match state.lock() { | |
| Ok(g) => g, | |
| Err(_) => return -1, | |
| }; | |
| state_guard.resource_usage_mut().record_host_call(); | |
| let bytes = state_guard.random_bytes(len as usize); | |
| match memory.write(&mut caller, ptr as usize, &bytes) { | |
| Ok(()) => { | |
| trace!(target: "wasm", "WASM random_bytes: {} bytes", len); | |
| 0 | |
| } | |
| Err(_) => -1, | |
| } | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!("failed to register host_random_bytes: {}", e)) | |
| })?; | |
| // host_abort(msg_ptr: i32, msg_len: i32, file_ptr: i32, file_len: i32, line: i32, col: i32) | |
| // Abort execution with an error message (used by AssemblyScript and other languages) | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_abort", | |
| |mut caller: Caller<'_, SharedHostState>, | |
| msg_ptr: i32, | |
| msg_len: i32, | |
| file_ptr: i32, | |
| file_len: i32, | |
| line: i32, | |
| col: i32| | |
| -> WasmtimeResult<()> { | |
| let memory = caller | |
| .get_export("memory") | |
| .and_then(|e| e.into_memory()) | |
| .ok_or_else(|| wasmtime::Error::msg("failed to find memory export"))?; | |
| let mut msg_buffer = vec![0u8; msg_len as usize]; | |
| memory.read(&caller, msg_ptr as usize, &mut msg_buffer)?; | |
| let msg = String::from_utf8_lossy(&msg_buffer); | |
| let mut file_buffer = vec![0u8; file_len as usize]; | |
| memory.read(&caller, file_ptr as usize, &mut file_buffer)?; | |
| let file = String::from_utf8_lossy(&file_buffer); | |
| tracing::error!( | |
| target: "wasm", | |
| "WASM abort: {} at {}:{}:{}", | |
| msg, | |
| file, | |
| line, | |
| col | |
| ); | |
| Err(wasmtime::Error::msg(format!( | |
| "WASM abort: {} at {}:{}:{}", | |
| msg, file, line, col | |
| ))) | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!("failed to register host_abort: {}", e)) | |
| })?; | |
| const MAX_HOST_MSG_BYTES: usize = 16 * 1024; | |
| const MAX_RNG_BYTES: usize = 4 * 1024; | |
| fn checked_len(len: i32, max: usize) -> WasmtimeResult<usize> { | |
| if len < 0 { | |
| return Err(wasmtime::Error::msg("negative length")); | |
| } | |
| let len = len as usize; | |
| if len > max { | |
| return Err(wasmtime::Error::msg("length too large")); | |
| } | |
| Ok(len) | |
| } | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_log", | |
| |mut caller: Caller<'_, SharedHostState>, ptr: i32, len: i32| -> WasmtimeResult<()> { | |
| let state = caller.data().clone(); | |
| let memory = caller | |
| .get_export("memory") | |
| .and_then(|e| e.into_memory()) | |
| .ok_or_else(|| wasmtime::Error::msg("failed to find memory export"))?; | |
| let len = checked_len(len, MAX_HOST_MSG_BYTES)?; | |
| let mut buffer = vec![0u8; len]; | |
| memory.read(&caller, ptr as usize, &mut buffer)?; | |
| let message = String::from_utf8_lossy(&buffer).to_string(); | |
| let mut state_guard = state | |
| .lock() | |
| .map_err(|e| wasmtime::Error::msg(format!("failed to lock state: {}", e)))?; | |
| state_guard.resource_usage_mut().record_host_call(); | |
| state_guard.add_log(message.clone()); | |
| debug!(target: "wasm", "WASM log: {}", message); | |
| Ok(()) | |
| }, | |
| ) | |
| .map_err(|e| WasmError::HostFunctionError(format!("failed to register host_log: {}", e)))?; | |
| // host_get_timestamp() -> i64 | |
| // Returns the current Unix timestamp in seconds | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_get_timestamp", | |
| |caller: Caller<'_, SharedHostState>| -> i64 { | |
| let state = caller.data().clone(); | |
| if let Ok(mut guard) = state.lock() { | |
| guard.resource_usage_mut().record_host_call(); | |
| } | |
| let timestamp = chrono::Utc::now().timestamp(); | |
| trace!(target: "wasm", "WASM timestamp request: {}", timestamp); | |
| timestamp | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!("failed to register host_get_timestamp: {}", e)) | |
| })?; | |
| // host_get_timestamp_millis() -> i64 | |
| // Returns the current Unix timestamp in milliseconds | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_get_timestamp_millis", | |
| |caller: Caller<'_, SharedHostState>| -> i64 { | |
| let state = caller.data().clone(); | |
| if let Ok(mut guard) = state.lock() { | |
| guard.resource_usage_mut().record_host_call(); | |
| } | |
| let timestamp = chrono::Utc::now().timestamp_millis(); | |
| trace!(target: "wasm", "WASM timestamp_millis request: {}", timestamp); | |
| timestamp | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!( | |
| "failed to register host_get_timestamp_millis: {}", | |
| e | |
| )) | |
| })?; | |
| // host_random_bytes(ptr: i32, len: i32) -> i32 | |
| // Fills the buffer at ptr with len random bytes (deterministic from seed) | |
| // Returns 0 on success, -1 on error | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_random_bytes", | |
| |mut caller: Caller<'_, SharedHostState>, ptr: i32, len: i32| -> i32 { | |
| let state = caller.data().clone(); | |
| let memory = match caller.get_export("memory").and_then(|e| e.into_memory()) { | |
| Some(m) => m, | |
| None => return -1, | |
| }; | |
| let mut state_guard = match state.lock() { | |
| Ok(g) => g, | |
| Err(_) => return -1, | |
| }; | |
| state_guard.resource_usage_mut().record_host_call(); | |
| let len = match checked_len(len, MAX_RNG_BYTES) { | |
| Ok(v) => v, | |
| Err(_) => return -1, | |
| }; | |
| let bytes = state_guard.random_bytes(len); | |
| match memory.write(&mut caller, ptr as usize, &bytes) { | |
| Ok(()) => { | |
| trace!(target: "wasm", "WASM random_bytes: {} bytes", len); | |
| 0 | |
| } | |
| Err(_) => -1, | |
| } | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!("failed to register host_random_bytes: {}", e)) | |
| })?; | |
| // host_abort(msg_ptr: i32, msg_len: i32, file_ptr: i32, file_len: i32, line: i32, col: i32) | |
| // Abort execution with an error message (used by AssemblyScript and other languages) | |
| linker | |
| .func_wrap( | |
| "env", | |
| "host_abort", | |
| |mut caller: Caller<'_, SharedHostState>, | |
| msg_ptr: i32, | |
| msg_len: i32, | |
| file_ptr: i32, | |
| file_len: i32, | |
| line: i32, | |
| col: i32| | |
| -> WasmtimeResult<()> { | |
| let memory = caller | |
| .get_export("memory") | |
| .and_then(|e| e.into_memory()) | |
| .ok_or_else(|| wasmtime::Error::msg("failed to find memory export"))?; | |
| let msg_len = checked_len(msg_len, MAX_HOST_MSG_BYTES)?; | |
| let mut msg_buffer = vec![0u8; msg_len]; | |
| memory.read(&caller, msg_ptr as usize, &mut msg_buffer)?; | |
| let msg = String::from_utf8_lossy(&msg_buffer); | |
| let file_len = checked_len(file_len, MAX_HOST_MSG_BYTES)?; | |
| let mut file_buffer = vec![0u8; file_len]; | |
| memory.read(&caller, file_ptr as usize, &mut file_buffer)?; | |
| let file = String::from_utf8_lossy(&file_buffer); | |
| tracing::error!( | |
| target: "wasm", | |
| "WASM abort: {} at {}:{}:{}", | |
| msg, | |
| file, | |
| line, | |
| col | |
| ); | |
| Err(wasmtime::Error::msg(format!( | |
| "WASM abort: {} at {}:{}:{}", | |
| msg, file, line, col | |
| ))) | |
| }, | |
| ) | |
| .map_err(|e| { | |
| WasmError::HostFunctionError(format!("failed to register host_abort: {}", e)) | |
| })?; |
🤖 Prompt for AI Agents
In `@crates/wasm-runtime/src/host_functions.rs` around lines 140 - 295, Add a
helper (e.g., validate_wasm_len or checked_wasm_len) that takes an i32 and a max
cap and returns Result<usize, wasmtime::Error> (or Err for host_random_bytes' -1
path); use it to reject negative or overly large lengths before allocating or
creating vectors. Apply it in host_log and host_abort for msg_len/file_len with
a 16*1024 cap and return a wasmtime::Error::msg on invalid input, and in
host_random_bytes for len with a 4*1024 cap returning -1 on invalid input; use
the returned safe usize for vec/reads/writes and avoid direct cast from i32 to
usize.
| /// Create a wasmtime engine with the given configuration | ||
| fn create_engine(config: &SandboxConfig) -> Result<Engine> { | ||
| let mut engine_config = wasmtime::Config::new(); | ||
|
|
||
| // Enable fuel consumption for CPU limiting | ||
| engine_config.consume_fuel(true); | ||
|
|
||
| // Set stack size limit | ||
| engine_config.max_wasm_stack(config.max_stack_size); | ||
|
|
||
| // Enable parallel compilation for faster module loading | ||
| engine_config.parallel_compilation(true); | ||
|
|
||
| Engine::new(&engine_config) | ||
| .map_err(|e| WasmError::ConfigError(format!("failed to create engine: {}", e))) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find the SandboxConfig definition
find . -type f -name "*.rs" | xargs grep -l "struct SandboxConfig" | head -5Repository: PlatformNetwork/platform-v2
Length of output: 147
🏁 Script executed:
# Check the context around the create_engine function
cat -n crates/wasm-runtime/src/runtime.rs | sed -n '50,100p'Repository: PlatformNetwork/platform-v2
Length of output: 2032
🏁 Script executed:
# Search for where SandboxConfig is used or defined
rg "SandboxConfig" --type rs -A 3 | head -60Repository: PlatformNetwork/platform-v2
Length of output: 97
🌐 Web query:
Wasmtime 27 API memory tables globals limits StoreLimits
💡 Result:
In Wasmtime 27, the built-in “store limits” mechanism is Store::limiter + StoreLimits (built via StoreLimitsBuilder), which implements the ResourceLimiter interface.
What you can limit (Rust API)
StoreLimitsBuilder lets you set:
- Per-memory max size (bytes):
memory_size(limit_bytes)(default: unlimited) - Per-table max elements:
table_elements(limit_elems)(default: unlimited) - Per-store counts:
instances(n),tables(n),memories(n)(each default: 10,000) - Behavior on grow failure:
trap_on_grow_failure(bool)(default:false) [1]
Attach limits to a store with store.limiter(|state| &mut state.limits). Limits apply only to future creation/growth, not retroactively. Also, a default Store already enforces 10,000 for instances/memories/tables unless overridden. [2]
Under the hood, the ResourceLimiter trait limits memory/table growth plus max instances/tables/memories counts (defaults again 10,000). [3]
What you cannot limit with StoreLimits: globals
There is no globals(...) limit in StoreLimitsBuilder, and ResourceLimiter does not include a “globals” quota—so you cannot directly cap the number of globals via StoreLimits in Wasmtime’s store limiter. [1][3]
C API equivalent
In the C API, wasmtime_store_limiter(...) exposes the same knobs: memory_size, table_elements, instances, tables, memories (negative = keep default). [4]
Sources: [1] [2] [3] [4]
🏁 Script executed:
cat -n crates/wasm-runtime/src/sandbox.rsRepository: PlatformNetwork/platform-v2
Length of output: 12740
🏁 Script executed:
# Search for Store creation and limiter usage
rg "Store::new\|limiter\|StoreLimits" --type rs -B 2 -A 2Repository: PlatformNetwork/platform-v2
Length of output: 97
🏁 Script executed:
# Check what fields are in SandboxConfig
rg "struct SandboxConfig|pub.*max_" crates/wasm-runtime/src/sandbox.rs -A 15Repository: PlatformNetwork/platform-v2
Length of output: 2333
🏁 Script executed:
# Search for Store creation and where limits should be applied
rg "Store::new|create_store|StoreLimits|limiter" -B 3 -A 5Repository: PlatformNetwork/platform-v2
Length of output: 12609
🏁 Script executed:
# Look for all references to SandboxConfig fields in the runtime
rg "max_memory_mb|max_tables|max_table_elements|max_memories|max_globals" crates/wasm-runtime/src/runtime.rsRepository: PlatformNetwork/platform-v2
Length of output: 391
🏁 Script executed:
# Check the create_store function in module.rs to see the full implementation
cat -n crates/wasm-runtime/src/module.rs | sed -n '1,150p'Repository: PlatformNetwork/platform-v2
Length of output: 5619
🏁 Script executed:
# Search for any use of StoreLimits or limiter in the codebase
rg "StoreLimits|ResourceLimiter|limiter" --type rsRepository: PlatformNetwork/platform-v2
Length of output: 97
🏁 Script executed:
# Check for StoreLimits or ResourceLimiter usage without specifying file type
grep -r "StoreLimits\|ResourceLimiter\|limiter" crates/wasm-runtime/src/ || echo "No matches found"Repository: PlatformNetwork/platform-v2
Length of output: 87
🏁 Script executed:
# Get more of the module.rs file, specifically the create_store function
cat -n crates/wasm-runtime/src/module.rs | sed -n '150,250p'Repository: PlatformNetwork/platform-v2
Length of output: 4092
Apply SandboxConfig memory/tables/memories limits via Store limiter, not just engine config.
The max_memory_mb, max_tables, max_table_elements, and max_memories fields in SandboxConfig are defined but never enforced. In Wasmtime 27, these limits must be set per-Store using StoreLimitsBuilder + store.limiter(), not at the engine level. Update create_store() in both module.rs and runtime.rs to attach a StoreLimiter with the configured bounds.
Note: max_globals cannot be enforced—Wasmtime's StoreLimits API has no globals quota.
🤖 Prompt for AI Agents
In `@crates/wasm-runtime/src/runtime.rs` around lines 64 - 79, The SandboxConfig
memory/table limits are not enforced at the Store level—modify the create_store
functions (in runtime.rs and module.rs) to build and attach a Store limiter
using wasmtime's StoreLimitsBuilder and StoreLimiter APIs: construct
StoreLimitsBuilder with max_memory_bytes (from max_memory_mb), max_tables,
max_table_elements, and max_memories from SandboxConfig, call build() to create
a StoreLimiter, and attach it to the Store via store.limiter(Some(...)) so the
limits are applied per-Store (keep create_engine as-is for engine-wide config
like fuel/stack); note max_globals cannot be enforced because StoreLimits has no
globals quota.
| // Read result length from first 4 bytes at result_ptr | ||
| let mut len_bytes = [0u8; 4]; | ||
| memory | ||
| .read(&store, result_ptr as usize, &mut len_bytes) | ||
| .map_err(|e| WasmError::MemoryError(format!("failed to read result length: {}", e)))?; | ||
| let result_len = i32::from_le_bytes(len_bytes); | ||
|
|
||
| // Read result data | ||
| let output = if result_len > 0 { | ||
| let mut data = vec![0u8; result_len as usize]; | ||
| memory | ||
| .read(&store, (result_ptr + 4) as usize, &mut data) | ||
| .map_err(|e| { | ||
| WasmError::MemoryError(format!("failed to read result data: {}", e)) | ||
| })?; | ||
| data | ||
| } else { | ||
| Vec::new() | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the exact code at lines 243-262 in the file
cat -n crates/wasm-runtime/src/runtime.rs | sed -n '240,265p'Repository: PlatformNetwork/platform-v2
Length of output: 1346
🏁 Script executed:
# Let's see more context around this area to understand the function
cat -n crates/wasm-runtime/src/runtime.rs | sed -n '200,280p'Repository: PlatformNetwork/platform-v2
Length of output: 3790
🏁 Script executed:
# Let's look for the Memory struct definition and its read method to understand bounds checking
rg "impl.*Memory" crates/wasm-runtime/src/runtime.rs -A 20Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Check if there's a data_size method on Memory
rg "data_size|memory.len|mem_size" crates/wasm-runtime/src/runtime.rs -B 2 -A 2Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Search for Memory type and read method implementation
rg "fn read|impl.*Memory" crates/wasm-runtime/ -B 2 -A 10Repository: PlatformNetwork/platform-v2
Length of output: 1916
🏁 Script executed:
# Look for write_bytes_to_memory to understand the pattern used for memory operations
rg "fn write_bytes_to_memory" crates/wasm-runtime/ -A 20Repository: PlatformNetwork/platform-v2
Length of output: 1423
🏁 Script executed:
# Check what type result_ptr is and if there's any validation
rg "result_ptr" crates/wasm-runtime/src/runtime.rs -B 5 -A 5Repository: PlatformNetwork/platform-v2
Length of output: 1156
🏁 Script executed:
# Look for any other memory safety checks or bounds validation in the file
rg "bounds|overflow|saturating|checked" crates/wasm-runtime/src/runtime.rsRepository: PlatformNetwork/platform-v2
Length of output: 146
🏁 Script executed:
# Let's check the Memory type from wasmtime - look for imports
rg "use.*Memory|from wasmtime" crates/wasm-runtime/src/runtime.rs | head -20Repository: PlatformNetwork/platform-v2
Length of output: 53
🏁 Script executed:
# Let's see the function signature that contains this code to understand the full context
cat -n crates/wasm-runtime/src/runtime.rs | sed -n '160,250p'Repository: PlatformNetwork/platform-v2
Length of output: 4218
🏁 Script executed:
# Check if there are any tests that show expected behavior with invalid inputs
rg "test.*result_ptr|test.*memory" crates/wasm-runtime/src/ -B 2 -A 10Repository: PlatformNetwork/platform-v2
Length of output: 873
Validate result_ptr and result_len before memory access to prevent heap allocation attacks.
The unsafe cast of result_len (an untrusted i32) to usize at line 252 allows negative values to become enormous heap allocations. For example, -1 as i32 becomes usize::MAX when cast, causing OOM or panic. Similarly, result_ptr is not validated for negative values. This pattern conflicts with protective checks already present in read_bytes_from_memory() and read_string_from_memory() in the same codebase.
Add validation to reject negative values and bounds-check against available memory before allocation:
Suggested fix
let result_len = i32::from_le_bytes(len_bytes);
+ if result_ptr < 0 || result_len < 0 {
+ return Err(WasmError::MemoryError(
+ format!("invalid result pointer/length: ptr={}, len={}", result_ptr, result_len)
+ ));
+ }
+
+ let mem_size = memory.data_size(&store);
+ let start = result_ptr as usize;
+ let end = start.saturating_add(4).saturating_add(result_len as usize);
+ if end > mem_size {
+ return Err(WasmError::MemoryError("result out of bounds".into()));
+ }
// Read result data
let output = if result_len > 0 {🤖 Prompt for AI Agents
In `@crates/wasm-runtime/src/runtime.rs` around lines 243 - 262, The code reads
result_len and then uses result_ptr and result_len unchecked to allocate and
read from memory, allowing negative i32 values to wrap to huge usize
allocations; update the logic around the result_ptr/result_len handling to
validate that both are non-negative, cast-safe, and within memory bounds before
any allocation or read (mirror the checks performed by
read_bytes_from_memory()/read_string_from_memory()), reject/return a
WasmError::MemoryError for invalid or out-of-bounds values, and only then
allocate the data buffer and call memory.read for (result_ptr + 4) and length
bytes.
Summary
This PR implements a complete decentralized WASM-based dynamic challenge loading system for platform-v2, with full integration points for term-challenge.
Major Components
1. WASM Challenge Runtime (
platform-wasm-runtime)2. Dynamic Challenge Loading System (
platform-challenge-loader)parking_lot::RwLock3. Validator Assignment Algorithm (
p2p-consensus/assignment)4. Fast Validation Consensus (
p2p-consensus/fast_consensus)5. Bittensor Storage Integration (
bittensor-integration/storage)6. Term-Challenge WASM Types (
core/term_challenge)WasmChallengeInterfacetrait for challenge modules7. Validator Node Integration
Testing
Breaking Changes
None - all changes are additive.
Checklist
Summary by CodeRabbit