fix: audit fixes - signature verification, dedup broadcast, state hash completeness, sequence tracking

Author: echobt

bins/validator-node/src/challenge_storage.rs

@@ -297,7 +297,9 @@ impl StorageBackend for ChallengeStorageBackend {
             }
         }
 
-        let result: Vec<(Vec<u8>, Vec<u8>)> = items.into_iter().collect();
+        // Sort by key for deterministic ordering across calls
+        let mut result: Vec<(Vec<u8>, Vec<u8>)> = items.into_iter().collect();
+        result.sort_by(|a, b| a.0.cmp(&b.0));
         let limited = if result.len() > limit as usize {
             result[..limit as usize].to_vec()
         } else {

bins/validator-node/src/main.rs

@@ -72,6 +72,19 @@ fn sanitize_for_log(s: &str) -> String {
         .collect()
 }
 
+/// Verify an sr25519 signature from a Hotkey over the given data.
+fn verify_signature(signer: &Hotkey, data: &[u8], signature: &[u8]) -> bool {
+    use sp_core::crypto::Pair as _;
+    if signature.len() != 64 {
+        return false;
+    }
+    let mut sig_bytes = [0u8; 64];
+    sig_bytes.copy_from_slice(signature);
+    let sig = sp_core::sr25519::Signature::from_raw(sig_bytes);
+    let pubkey = sp_core::sr25519::Public::from_raw(signer.0);
+    sp_core::sr25519::Pair::verify(&sig, data, &pubkey)
+}
+
 /// Helper to mutate ChainState and automatically persist changes.
 /// This ensures we never forget to persist after mutations.
 async fn mutate_and_persist<F, R>(
@@ -1789,36 +1802,46 @@ async fn main() -> Result<()> {
                             if let Some(ref executor) = wasm_executor {
                                 match executor.execute_sync_with_block(&module_path, current_block, current_epoch) {
                                     Ok(sync_result) => {
-                                        info!(
-                                            challenge_id = %challenge_id,
-                                            block = current_block,
-                                            total_users = sync_result.total_users,
-                                            "Challenge sync completed, broadcasting proposal"
-                                        );
+                                        // Skip broadcasting if sync was deduped (zeroed hash
+                                        // from DedupFlags guard). Broadcasting a zeroed hash
+                                        // would poison P2P consensus.
+                                        if sync_result.leaderboard_hash == [0u8; 32] && sync_result.total_users == 0 {
+                                            debug!(
+                                                challenge_id = %challenge_id,
+                                                "Sync deduped (zeroed result), skipping broadcast"
+                                            );
+                                        } else {
+                                            info!(
+                                                challenge_id = %challenge_id,
+                                                block = current_block,
+                                                total_users = sync_result.total_users,
+                                                "Challenge sync completed, broadcasting proposal"
+                                            );
 
-                                        // Broadcast sync proposal
-                                        let timestamp = chrono::Utc::now().timestamp_millis();
-                                        let proposal_data = bincode::serialize(&(
-                                            &challenge_id,
-                                            &sync_result.leaderboard_hash,
-                                            current_block,
-                                            timestamp
-                                        )).unwrap_or_default();
-                                        let signature = keypair.sign_bytes(&proposal_data).unwrap_or_default();
-
-                                        let proposal_msg = P2PMessage::ChallengeSyncProposal(
-                                            platform_p2p_consensus::ChallengeSyncProposalMessage {
-                                                challenge_id,
-                                                sync_result_hash: sync_result.leaderboard_hash,
-                                                proposer: keypair.hotkey(),
-                                                block_number: current_block,
-                                                timestamp,
-                                                signature,
-                                            }
-                                        );
+                                            // Broadcast sync proposal
+                                            let timestamp = chrono::Utc::now().timestamp_millis();
+                                            let proposal_data = bincode::serialize(&(
+                                                &challenge_id,
+                                                &sync_result.leaderboard_hash,
+                                                current_block,
+                                                timestamp
+                                            )).unwrap_or_default();
+                                            let signature = keypair.sign_bytes(&proposal_data).unwrap_or_default();
+
+                                            let proposal_msg = P2PMessage::ChallengeSyncProposal(
+                                                platform_p2p_consensus::ChallengeSyncProposalMessage {
+                                                    challenge_id,
+                                                    sync_result_hash: sync_result.leaderboard_hash,
+                                                    proposer: keypair.hotkey(),
+                                                    block_number: current_block,
+                                                    timestamp,
+                                                    signature,
+                                                }
+                                            );
 
-                                        if let Err(e) = p2p_broadcast_tx.send(platform_p2p_consensus::P2PCommand::Broadcast(proposal_msg)).await {
-                                            warn!(error = %e, "Failed to broadcast sync proposal");
+                                            if let Err(e) = p2p_broadcast_tx.send(platform_p2p_consensus::P2PCommand::Broadcast(proposal_msg)).await {
+                                                warn!(error = %e, "Failed to broadcast sync proposal");
+                                            }
                                         }
                                     }
                                     Err(e) => {
@@ -3015,11 +3038,30 @@ async fn handle_network_event(
                 // Verify proposer is a known validator
                 let proposer_valid = validator_set.is_validator(&proposal.proposer);
 
+                // Verify cryptographic signature on proposal
+                let sig_valid = if proposer_valid {
+                    let sign_data = bincode::serialize(&(
+                        &proposal.proposal_id,
+                        proposal.challenge_id.to_string(),
+                        proposal.timestamp,
+                    ))
+                    .unwrap_or_default();
+                    verify_signature(&proposal.proposer, &sign_data, &proposal.signature)
+                } else {
+                    false
+                };
+
                 if !proposer_valid {
                     warn!(
                         proposer = %proposal.proposer.to_ss58(),
                         "Storage proposal from unknown validator, ignoring"
                     );
+                } else if !sig_valid {
+                    warn!(
+                        proposal_id = %hex::encode(&proposal.proposal_id[..8]),
+                        proposer = %proposal.proposer.to_ss58(),
+                        "Storage proposal signature verification failed, ignoring"
+                    );
                 } else {
                     // Add proposal to state
                     let storage_proposal = StorageProposal {
@@ -3151,8 +3193,25 @@ async fn handle_network_event(
                 );
 
                 // Verify voter is a known validator
-                if !validator_set.is_validator(&vote.voter) {
+                // Verify cryptographic signature on vote
+                let voter_known = validator_set.is_validator(&vote.voter);
+                let vote_sig_valid = if voter_known {
+                    let vote_sign_data =
+                        bincode::serialize(&(&vote.proposal_id, vote.approve, vote.timestamp))
+                            .unwrap_or_default();
+                    verify_signature(&vote.voter, &vote_sign_data, &vote.signature)
+                } else {
+                    false
+                };
+
+                if !voter_known {
                     warn!(voter = %vote.voter.to_ss58(), "Vote from unknown validator");
+                } else if !vote_sig_valid {
+                    warn!(
+                        proposal_id = %hex::encode(&vote.proposal_id[..8]),
+                        voter = %vote.voter.to_ss58(),
+                        "Storage vote signature verification failed, ignoring"
+                    );
                 } else {
                     // Add vote to proposal
                     let consensus_result = state_manager.apply(|state| {

bins/validator-node/src/wasm_executor.rs

@@ -1195,7 +1195,7 @@ impl WasmChallengeExecutor {
                 ..self.config.storage_host_config.clone()
             },
             storage_backend: Arc::clone(&self.config.storage_backend),
-            consensus_policy: ConsensusPolicy::read_only(),
+            consensus_policy: ConsensusPolicy::default(),
             network_policy: NetworkPolicy::development(),
             time_policy: TimePolicy::deterministic(real_now_ms),
             block_height,

crates/p2p-consensus/src/state.rs

@@ -478,6 +478,10 @@ impl ChainState {
             pending_ids: Vec<String>,
             weight_voter_count: usize,
             historical_weight_epochs: Vec<u64>,
+            completed_eval_count: usize,
+            leaderboard_challenges: Vec<String>,
+            active_job_ids: Vec<String>,
+            task_progress_ids: Vec<String>,
         }
 
         let mut validators: Vec<(String, u64)> = self
@@ -503,6 +507,19 @@ impl ChainState {
             self.historical_weights.keys().copied().collect();
         historical_weight_epochs.sort();
 
+        let completed_eval_count: usize =
+            self.completed_evaluations.values().map(|v| v.len()).sum();
+
+        let mut leaderboard_challenges: Vec<String> =
+            self.leaderboard.keys().map(|c| c.to_string()).collect();
+        leaderboard_challenges.sort();
+
+        let mut active_job_ids: Vec<String> = self.active_jobs.keys().cloned().collect();
+        active_job_ids.sort();
+
+        let mut task_progress_ids: Vec<String> = self.task_progress.keys().cloned().collect();
+        task_progress_ids.sort();
+
         let input = HashInput {
             sequence: self.sequence,
             epoch: self.epoch,
@@ -514,6 +531,10 @@ impl ChainState {
             pending_ids,
             weight_voter_count,
             historical_weight_epochs,
+            completed_eval_count,
+            leaderboard_challenges,
+            active_job_ids,
+            task_progress_ids,
         };
 
         self.state_hash = hash_data(&input).unwrap_or([0u8; 32]);
@@ -911,7 +932,7 @@ impl ChainState {
 
         if let Some(record) = self.pending_evaluations.get_mut(submission_id) {
             record.evaluations.insert(validator, evaluation);
-            self.update_hash();
+            self.increment_sequence();
             Ok(())
         } else {
             Err(StateError::ChallengeNotFound(submission_id.to_string()))
@@ -1360,7 +1381,7 @@ impl ChainState {
     pub fn update_task_progress(&mut self, record: TaskProgressRecord) {
         let key = format!("{}:{}", record.submission_id, record.validator.to_hex());
         self.task_progress.insert(key, record);
-        self.update_hash();
+        self.increment_sequence();
     }
 
     pub fn update_challenge_storage_root(&mut self, challenge_id: ChallengeId, root: [u8; 32]) {
@@ -1473,7 +1494,7 @@ impl ChainState {
                             timestamp: chrono::Utc::now().timestamp_millis(),
                         },
                     );
-                    self.update_hash();
+                    self.increment_sequence();
                     return true;
                 }
             }
@@ -1522,7 +1543,7 @@ impl ChainState {
             .entry(submission_id.to_string())
             .or_default()
             .insert(validator, logs_data);
-        self.update_hash();
+        self.increment_sequence();
         true
     }