Skip to content

Commit e858033

Browse files
committed
fix: audit round 2 - critical consensus, storage, RPC, and epoch bugs
Critical: - C1: Re-add consensus guard in handle_storage_set - C2: Reject unsigned votes in StateRootConsensus and ValidatedStorage - C3: Log error when valid_voters is empty - C4: Deduplicate view change voters in handle_new_view - C5: Verify prepared_proof signatures before restoring round High: - H1: Wrap block_on with block_in_place in ChallengeStorageBackend - H2: Block cross-challenge storage reads - H3: Fix body hash in challenge_call JSON-RPC - H4: Enforce auth on requires_auth routes - H5: Require validator_hotkey in webhook - H6: Extract mechanism_id with TODO Medium: - M1: BFT quorum for storage proposal voting - M2: Route matching for path params in challenge_call - M6: Remove expired pending_ops from sled - M8: saturating_sub in cleanup - M9: flush in write_direct - M12: Averaged weights on divergence - M13: Hash verification for AgentLogProposal
1 parent 739d4da commit e858033

File tree

13 files changed

+320
-65
lines changed

13 files changed

+320
-65
lines changed

bins/validator-node/src/challenge_storage.rs

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ impl ChallengeStorageBackend {
1919
impl StorageBackend for ChallengeStorageBackend {
2020
fn get(&self, challenge_id: &str, key: &[u8]) -> Result<Option<Vec<u8>>, StorageHostError> {
2121
let storage_key = DStorageKey::new(challenge_id, hex::encode(key));
22-
let result = tokio::runtime::Handle::current()
23-
.block_on(self.storage.get(&storage_key, DGetOptions::default()))
24-
.map_err(|e| StorageHostError::StorageError(e.to_string()))?;
22+
let result = tokio::task::block_in_place(|| {
23+
tokio::runtime::Handle::current()
24+
.block_on(self.storage.get(&storage_key, DGetOptions::default()))
25+
})
26+
.map_err(|e| StorageHostError::StorageError(e.to_string()))?;
2527
Ok(result.map(|v| v.data))
2628
}
2729

@@ -32,12 +34,14 @@ impl StorageBackend for ChallengeStorageBackend {
3234
value: &[u8],
3335
) -> Result<[u8; 32], StorageHostError> {
3436
let storage_key = DStorageKey::new(challenge_id, hex::encode(key));
35-
tokio::runtime::Handle::current()
36-
.block_on(
37-
self.storage
38-
.put(storage_key, value.to_vec(), DPutOptions::default()),
39-
)
40-
.map_err(|e| StorageHostError::StorageError(e.to_string()))?;
37+
tokio::task::block_in_place(|| {
38+
tokio::runtime::Handle::current().block_on(self.storage.put(
39+
storage_key,
40+
value.to_vec(),
41+
DPutOptions::default(),
42+
))
43+
})
44+
.map_err(|e| StorageHostError::StorageError(e.to_string()))?;
4145

4246
let mut hasher = Sha256::new();
4347
hasher.update(challenge_id.as_bytes());
@@ -48,16 +52,23 @@ impl StorageBackend for ChallengeStorageBackend {
4852

4953
fn delete(&self, challenge_id: &str, key: &[u8]) -> Result<bool, StorageHostError> {
5054
let storage_key = DStorageKey::new(challenge_id, hex::encode(key));
51-
tokio::runtime::Handle::current()
52-
.block_on(self.storage.delete(&storage_key))
53-
.map_err(|e| StorageHostError::StorageError(e.to_string()))
55+
tokio::task::block_in_place(|| {
56+
tokio::runtime::Handle::current().block_on(self.storage.delete(&storage_key))
57+
})
58+
.map_err(|e| StorageHostError::StorageError(e.to_string()))
5459
}
5560

5661
fn get_cross(
5762
&self,
5863
challenge_id: &str,
5964
key: &[u8],
6065
) -> Result<Option<Vec<u8>>, StorageHostError> {
61-
self.get(challenge_id, key)
66+
let storage_key = DStorageKey::new(challenge_id, hex::encode(key));
67+
let result = tokio::task::block_in_place(|| {
68+
tokio::runtime::Handle::current()
69+
.block_on(self.storage.get(&storage_key, DGetOptions::default()))
70+
})
71+
.map_err(|e| StorageHostError::StorageError(e.to_string()))?;
72+
Ok(result.map(|v| v.data))
6273
}
6374
}

bins/validator-node/src/main.rs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2076,11 +2076,25 @@ async fn handle_network_event(
20762076
);
20772077
}
20782078
P2PMessage::AgentLogProposal(msg) => {
2079-
debug!(
2080-
submission_id = %msg.submission_id,
2081-
validator = %msg.validator_hotkey.to_hex(),
2082-
"Received agent log proposal"
2083-
);
2079+
// Verify logs hash matches data
2080+
let computed_hash = platform_core::crypto::hash(&msg.logs_data);
2081+
if computed_hash != msg.logs_hash {
2082+
warn!("Agent log proposal hash mismatch, ignoring");
2083+
} else {
2084+
debug!(
2085+
submission_id = %msg.submission_id,
2086+
validator = %msg.validator_hotkey.to_hex(),
2087+
"Received valid agent log proposal"
2088+
);
2089+
// Store in chain state for consensus
2090+
state_manager.apply(|state| {
2091+
state
2092+
.agent_log_proposals
2093+
.entry(msg.submission_id.clone())
2094+
.or_default()
2095+
.insert(msg.validator_hotkey.clone(), msg.logs_data.clone());
2096+
});
2097+
}
20842098
}
20852099
P2PMessage::StorageRootSync(msg) => {
20862100
if !validator_set.is_validator(&msg.validator) {
@@ -2276,6 +2290,7 @@ async fn handle_block_event(
22762290

22772291
// Get weights from decentralized state
22782292
if let (Some(st), Some(sig)) = (subtensor.as_ref(), signer.as_ref()) {
2293+
// TODO: Call detect_suspicious_validators and apply_penalties with excluded validators
22792294
let final_weights = state_manager.apply(|state| state.finalize_weights());
22802295

22812296
match final_weights {
@@ -2285,11 +2300,16 @@ async fn handle_block_event(
22852300
let vals: Vec<u16> = weights.iter().map(|(_, w)| *w).collect();
22862301

22872302
info!("Submitting weights for {} UIDs", uids.len());
2303+
// TODO(H6): mechanism_id is hardcoded to 0. Should be derived from
2304+
// challenge config (e.g. mechanism_configs) or subnet hyperparameters.
2305+
// Each subnet mechanism may have a different ID; using 0 assumes a
2306+
// single-mechanism subnet.
2307+
let mechanism_id = 0u8;
22882308
match st
22892309
.set_mechanism_weights(
22902310
sig,
22912311
netuid,
2292-
0,
2312+
mechanism_id,
22932313
&uids,
22942314
&vals,
22952315
version_key,
@@ -2306,11 +2326,13 @@ async fn handle_block_event(
23062326
}
23072327
_ => {
23082328
info!("No challenge weights for epoch {} - submitting burn weights (100% to UID 0)", epoch);
2329+
// TODO(H6): mechanism_id is hardcoded to 0 (see note above)
2330+
let mechanism_id = 0u8;
23092331
match st
23102332
.set_mechanism_weights(
23112333
sig,
23122334
netuid,
2313-
0,
2335+
mechanism_id,
23142336
&[0u16],
23152337
&[65535u16],
23162338
version_key,

bins/validator-node/src/wasm_executor.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ impl WasmChallengeExecutor {
201201
restart_id: String::new(),
202202
config_version: 0,
203203
storage_host_config: StorageHostConfig {
204-
allow_direct_writes: false,
204+
allow_direct_writes: true,
205205
require_consensus: true,
206206
..self.config.storage_host_config.clone()
207207
},
@@ -329,7 +329,7 @@ impl WasmChallengeExecutor {
329329
restart_id: String::new(),
330330
config_version: 0,
331331
storage_host_config: StorageHostConfig {
332-
allow_direct_writes: false,
332+
allow_direct_writes: true,
333333
require_consensus: true,
334334
..self.config.storage_host_config.clone()
335335
},
@@ -808,7 +808,7 @@ impl WasmChallengeExecutor {
808808
restart_id: String::new(),
809809
config_version: 0,
810810
storage_host_config: StorageHostConfig {
811-
allow_direct_writes: false,
811+
allow_direct_writes: true,
812812
require_consensus: true,
813813
..self.config.storage_host_config.clone()
814814
},
@@ -966,7 +966,7 @@ impl WasmChallengeExecutor {
966966
challenge_id: module_path.to_string(),
967967
validator_id: "validator".to_string(),
968968
storage_host_config: StorageHostConfig {
969-
allow_direct_writes: false,
969+
allow_direct_writes: true,
970970
require_consensus: true,
971971
..self.config.storage_host_config.clone()
972972
},

crates/distributed-storage/src/state_consensus.rs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -621,15 +621,28 @@ impl StateRootConsensus {
621621
})?;
622622

623623
// Verify voter is a known validator
624+
if self.valid_voters.is_empty() {
625+
tracing::error!(
626+
"valid_voters is empty - voter authentication disabled! Call set_valid_voters()"
627+
);
628+
}
624629
if !self.valid_voters.is_empty() && !self.valid_voters.contains(&vote.voter) {
625630
return Err(StateRootConsensusError::InternalError(format!(
626631
"Vote from unknown validator: {}",
627632
vote.voter.to_hex()
628633
)));
629634
}
630635

631-
// Verify vote signature if present
632-
if !vote.signature.is_empty() {
636+
// Reject votes without signatures
637+
if vote.signature.is_empty() {
638+
return Err(StateRootConsensusError::InternalError(format!(
639+
"Vote from {} has no signature",
640+
vote.voter.to_hex()
641+
)));
642+
}
643+
644+
// Verify vote signature cryptographically when valid_voters is configured
645+
if !self.valid_voters.is_empty() {
633646
let vote_hash = vote.compute_hash();
634647
let signed_msg = platform_core::SignedMessage {
635648
message: vote_hash.to_vec(),
@@ -652,11 +665,6 @@ impl StateRootConsensus {
652665
)));
653666
}
654667
}
655-
} else {
656-
warn!(
657-
voter = vote.voter.to_hex(),
658-
"Vote received without signature"
659-
);
660668
}
661669

662670
// Verify vote is for current proposal
@@ -1292,14 +1300,15 @@ mod tests {
12921300
assert!(consensus.check_consensus().is_none()); // Not enough yet
12931301

12941302
// Second vote from another validator
1295-
let vote2 = StateRootVote {
1303+
let mut vote2 = StateRootVote {
12961304
block_number: 100,
12971305
voter: create_test_hotkey(2),
12981306
state_root: global_root,
12991307
agrees_with_proposal: true,
13001308
timestamp: chrono::Utc::now().timestamp_millis(),
13011309
signature: Vec::new(),
13021310
};
1311+
vote2.signature = vec![0u8; 64];
13031312

13041313
let result = consensus.receive_vote(vote2).expect("Should accept vote");
13051314
assert!(result.is_some()); // Should have consensus now
@@ -1322,25 +1331,27 @@ mod tests {
13221331
let _proposal = consensus.propose_state_root(100, global_root, challenge_roots);
13231332

13241333
// First vote from validator 2
1325-
let vote1 = StateRootVote {
1334+
let mut vote1 = StateRootVote {
13261335
block_number: 100,
13271336
voter: create_test_hotkey(2),
13281337
state_root: global_root,
13291338
agrees_with_proposal: true,
13301339
timestamp: chrono::Utc::now().timestamp_millis(),
13311340
signature: Vec::new(),
13321341
};
1342+
vote1.signature = vec![0u8; 64];
13331343
consensus.receive_vote(vote1).expect("Should accept vote");
13341344

13351345
// Conflicting vote from same validator
1336-
let vote2 = StateRootVote {
1346+
let mut vote2 = StateRootVote {
13371347
block_number: 100,
13381348
voter: create_test_hotkey(2),
13391349
state_root: [99u8; 32], // Different root!
13401350
agrees_with_proposal: false,
13411351
timestamp: chrono::Utc::now().timestamp_millis(),
13421352
signature: Vec::new(),
13431353
};
1354+
vote2.signature = vec![0u8; 64];
13441355

13451356
let result = consensus.receive_vote(vote2);
13461357
assert!(result.is_err());
@@ -1507,14 +1518,15 @@ mod tests {
15071518
let hotkey = create_test_hotkey(1);
15081519
let mut consensus = StateRootConsensus::new(hotkey, 3);
15091520

1510-
let vote = StateRootVote {
1521+
let mut vote = StateRootVote {
15111522
block_number: 100,
15121523
voter: create_test_hotkey(2),
15131524
state_root: [42u8; 32],
15141525
agrees_with_proposal: true,
15151526
timestamp: chrono::Utc::now().timestamp_millis(),
15161527
signature: Vec::new(),
15171528
};
1529+
vote.signature = vec![0u8; 64];
15181530

15191531
let result = consensus.receive_vote(vote);
15201532
assert!(result.is_err());
@@ -1535,14 +1547,15 @@ mod tests {
15351547

15361548
let _proposal = consensus.propose_state_root(100, global_root, challenge_roots);
15371549

1538-
let vote = StateRootVote {
1550+
let mut vote = StateRootVote {
15391551
block_number: 999, // Wrong block!
15401552
voter: create_test_hotkey(2),
15411553
state_root: global_root,
15421554
agrees_with_proposal: true,
15431555
timestamp: chrono::Utc::now().timestamp_millis(),
15441556
signature: Vec::new(),
15451557
};
1558+
vote.signature = vec![0u8; 64];
15461559

15471560
let result = consensus.receive_vote(vote);
15481561
assert!(result.is_err());

crates/distributed-storage/src/validated_storage.rs

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,9 @@ impl<S: DistributedStore + 'static> ValidatedStorage<S> {
494494
// Verify voter is a known validator
495495
{
496496
let voters = self.valid_voters.read().await;
497+
if voters.is_empty() {
498+
tracing::error!("valid_voters is empty - voter authentication disabled! Call set_valid_voters()");
499+
}
497500
if !voters.is_empty() && !voters.contains(&vote.voter) {
498501
return Err(ValidatedStorageError::Storage(format!(
499502
"Vote from unknown validator: {}",
@@ -502,25 +505,34 @@ impl<S: DistributedStore + 'static> ValidatedStorage<S> {
502505
}
503506
}
504507

505-
// Verify vote signature if present
506-
if !vote.signature.is_empty() {
507-
let vote_hash = vote.compute_hash();
508-
let signed_msg = platform_core::SignedMessage {
509-
message: vote_hash.to_vec(),
510-
signature: vote.signature.clone(),
511-
signer: vote.voter.clone(),
512-
};
513-
match signed_msg.verify() {
514-
Ok(true) => {}
515-
_ => {
516-
return Err(ValidatedStorageError::Storage(format!(
517-
"Invalid signature from voter: {}",
518-
vote.voter.to_hex()
519-
)));
508+
// Reject votes without signatures
509+
if vote.signature.is_empty() {
510+
return Err(ValidatedStorageError::Storage(format!(
511+
"Vote from {} has no signature",
512+
vote.voter.to_hex()
513+
)));
514+
}
515+
516+
// Verify vote signature cryptographically when valid_voters is configured
517+
{
518+
let voters = self.valid_voters.read().await;
519+
if !voters.is_empty() {
520+
let vote_hash = vote.compute_hash();
521+
let signed_msg = platform_core::SignedMessage {
522+
message: vote_hash.to_vec(),
523+
signature: vote.signature.clone(),
524+
signer: vote.voter.clone(),
525+
};
526+
match signed_msg.verify() {
527+
Ok(true) => {}
528+
_ => {
529+
return Err(ValidatedStorageError::Storage(format!(
530+
"Invalid signature from voter: {}",
531+
vote.voter.to_hex()
532+
)));
533+
}
520534
}
521535
}
522-
} else {
523-
tracing::warn!(voter = %vote.voter.to_hex(), "Vote received without signature");
524536
}
525537

526538
let proposal_id = vote.proposal_id;
@@ -886,7 +898,9 @@ mod tests {
886898
.expect("Check should succeed");
887899
assert!(result.is_none());
888900

889-
let vote2 = StorageWriteVote::new(proposal.proposal_id, create_test_hotkey(2), true, None);
901+
let mut vote2 =
902+
StorageWriteVote::new(proposal.proposal_id, create_test_hotkey(2), true, None);
903+
vote2.signature = vec![0u8; 64];
890904
let result = validated
891905
.receive_vote(vote2)
892906
.await
@@ -916,7 +930,9 @@ mod tests {
916930
.await
917931
.expect("Vote should succeed");
918932

919-
let vote2 = StorageWriteVote::new(proposal.proposal_id, create_test_hotkey(2), true, None);
933+
let mut vote2 =
934+
StorageWriteVote::new(proposal.proposal_id, create_test_hotkey(2), true, None);
935+
vote2.signature = vec![0u8; 64];
920936
validated
921937
.receive_vote(vote2)
922938
.await
@@ -1031,13 +1047,15 @@ mod tests {
10311047
let proposal = validated.propose_write(b"test-key", b"test-value").await;
10321048

10331049
let voter = create_test_hotkey(2);
1034-
let vote1 = StorageWriteVote::new(proposal.proposal_id, voter.clone(), true, None);
1050+
let mut vote1 = StorageWriteVote::new(proposal.proposal_id, voter.clone(), true, None);
1051+
vote1.signature = vec![0u8; 64];
10351052
validated
10361053
.receive_vote(vote1)
10371054
.await
10381055
.expect("First vote should succeed");
10391056

1040-
let vote2 = StorageWriteVote::new(proposal.proposal_id, voter, false, None);
1057+
let mut vote2 = StorageWriteVote::new(proposal.proposal_id, voter, false, None);
1058+
vote2.signature = vec![0u8; 64];
10411059
let result = validated.receive_vote(vote2).await;
10421060

10431061
assert!(matches!(

0 commit comments

Comments
 (0)