feat(storage): add validated storage with WASM-based consensus - #10
Conversation
Add per-challenge storage system where validators must reach consensus before data is accepted. This prevents abuse by requiring WASM-defined validation logic and cryptographic signatures on all proposals and votes. Core implementation in distributed-storage crate: - ValidatedStorage: main struct managing proposals, votes, and commits - StorageWriteProposal: represents a write request with value hash - StorageWriteVote: validator vote with optional WASM validation result - ConsensusResult: tracks consensus state and commitment status - WasmStorageValidator trait: interface for WASM validation logic - DefaultWasmValidator: passthrough implementation for testing - ValidatedStorageConfig: configurable quorum size and timeouts WASM runtime interface integration: - ChallengeStorage: high-level API for WASM modules to access storage - ChallengeStorageConfig: per-challenge storage configuration - Full async support with proposal/vote/commit lifecycle Key features: - Configurable quorum size for consensus requirements - Proposal expiration with automatic cleanup - Duplicate and conflicting vote detection - Cryptographic hashing of values (SHA256) - Comprehensive test coverage for all operations Dependencies: - Added sha2 to wasm-runtime-interface for hashing
📝 WalkthroughWalkthroughIntroduces a comprehensive WASM-based validated storage system for consensus-driven writes across two crates. The distributed-storage crate gains per-challenge proposal and voting mechanisms with quorum-based consensus tracking. The wasm-runtime-interface crate adds a storage host interface with configurable backends, audit logging, and result encoding for storage operations. Changes
Sequence DiagramsequenceDiagram
participant Proposer as Proposer
participant VS as ValidatedStorage
participant DB as DistributedStore
participant Voters as Voter Pool
Proposer->>VS: propose_write(key, value)
VS->>VS: create StorageWriteProposal
VS->>VS: store proposal state
VS-->>Proposer: proposal created
Proposer->>Voters: broadcast proposal
Voters->>VS: receive_proposal(proposal)
VS->>VS: validate proposal
VS-->>Voters: proposal acknowledged
Note over Voters: Voting phase
Voters->>VS: vote_on_proposal(proposal_id, approved, validator)
VS->>VS: verify voter signature
VS->>VS: detect duplicates/conflicts
VS->>VS: track vote counts
VS-->>Voters: vote recorded
Proposer->>VS: check_consensus(proposal_id)
VS->>VS: count approvals vs quorum
alt Consensus Reached
VS->>VS: create ConsensusResult
VS->>DB: commit_write(key, value)
DB-->>VS: write confirmed
VS-->>Proposer: committed
else Consensus Failed
VS-->>Proposer: consensus pending/failed
end
Proposer->>VS: get(key)
VS->>DB: retrieve if committed
DB-->>VS: return value
VS-->>Proposer: value
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@crates/distributed-storage/src/validated_storage.rs`:
- Around line 327-333: ValidatedStorage currently never invokes
WasmStorageValidator so WASM validation is unused; add a validator field to
ValidatedStorage (e.g., validator: Option<Arc<dyn WasmStorageValidator + Send +
Sync>>) and populate it when building ValidatedStorage based on the
require_wasm_validation flag, then call validator.validate_write(...) inside
vote_on_proposal and receive_proposal (use the same inputs those methods already
have: proposal payload and metadata) before creating/recording a vote or
accepting a proposal; on validation failure return/reject the vote path (or log
and mark proposal as invalid in proposals/committed) so invalid writes do not
proceed.
- Around line 685-699: cleanup_expired currently keeps proposals with
consensus_result.is_some() forever and there is no cleanup for the committed
map, causing memory growth; update cleanup_expired (and/or add a new
cleanup_committed) to evict consensus-reached proposals after a TTL or when
their consensus timestamp is older than config.proposal_timeout_ms (or a new
config like committed_ttl_ms), and add symmetric eviction for the committed map
(the committed field) using the same TTL or a max-size policy so entries in
committed are removed once stale; locate and update the proposals.retain closure
in cleanup_expired and implement a new cleanup path that checks
state.consensus_result.timestamp (or stores a timestamp when setting
consensus_result) and the committed map pruning logic to remove stale entries.
- Around line 148-158: The structs StorageWriteProposal and StorageWriteVote
declare signature: Vec<u8> but signatures are never produced or checked, so
implement end-to-end signing and verification: when creating proposals/votes
(where StorageWriteProposal and StorageWriteVote instances are constructed) sign
the canonical serialized payload with the proposer's/voter's Hotkey private key
and populate signature, and when validating incoming proposals/votes verify the
signature against the Hotkey public key and return the InvalidSignature error
variant on failure; if you intend to defer implementation instead add an
explicit TODO/issue reference in the creation/validation code and comment on
StorageWriteProposal/StorageWriteVote to avoid misleading claims about
cryptographic signatures.
- Around line 127-135: The constructor new currently allows quorum_size == 0
which makes check_consensus treat approving.len() >= quorum_size as true
immediately; update new (the function new that sets the quorum_size field) to
enforce a minimum bound (e.g., require quorum_size >= 1) by either
asserting/validating and panicking on invalid input or normalizing the value
(e.g., quorum_size = std::cmp::max(1, quorum_size)); ensure the change
references quorum_size and that check_consensus/approving.len() behavior is no
longer able to auto-consensus with zero votes.
In `@crates/wasm-runtime-interface/src/storage.rs`:
- Around line 335-354: The propose_write implementation currently inserts values
immediately into the main store (the HashMap under self.data) which bypasses
consensus; change propose_write (InMemoryStorageBackend::propose_write) to only
record the proposal (e.g., store the key/value in a dedicated proposals map or a
proposals entry keyed by the returned proposal id) and return the SHA-256
proposal id, deferring any mutation of the primary data store until a separate
commit method (e.g., commit_write/commit_proposal) applies the proposal;
alternatively, if immediate persistence is intentionally allowed for tests, add
a clear doc comment on InMemoryStorageBackend::propose_write stating it bypasses
consensus semantics and/or gate it behind a test-only flag.
🧹 Nitpick comments (5)
crates/wasm-runtime-interface/src/lib.rs (1)
16-21: Consider merging the twopub use storage::blocks and checking for missing re-exports.There are two separate
pub use storage::{...}blocks (lines 16–21 and 33–36). These can be merged into one for tidiness. Also,NoopStorageAuditLogger,pack_result, andunpack_resultare public instorage.rsbut not re-exported here. If they're intended as internal-only, consider reducing their visibility instorage.rs; otherwise, re-export them for consumers.Also applies to: 33-36
crates/distributed-storage/src/validated_storage.rs (2)
65-65: Blanket#![allow(...)]suppresses useful warnings — scope it down.
#![allow(dead_code, unused_variables, unused_imports)]at the module level masks real issues (e.g., the unusedWasmStorageValidatorintegration, unusedrequire_wasm_validationfield). Consider removing this and addressing the warnings individually, or scoping the allows to specific items that are intentionally stubbed.
519-581:check_consensusacquires a write lock even when only reading.Line 523 takes
self.proposals.write().awaitunconditionally. When consensus is already decided (lines 528–530), only a read is needed. Under contention (many validators polling), this serializes all callers unnecessarily.Consider a two-phase approach: read lock first, then upgrade to write only when consensus is newly reached.
crates/wasm-runtime-interface/src/storage.rs (2)
23-23: Same blanket#![allow(...)]concern as invalidated_storage.rs.Consider removing or scoping this down to specific items.
270-281:StorageBackendimplementations don't enforceStorageHostConfiglimits.
StorageHostConfigdefinesvalidate_keyandvalidate_value, but neitherInMemoryStorageBackendnorNoopStorageBackendcalls these methods. A caller that bypasses the host state layer can store arbitrarily large keys/values. Consider either validating inside the backend methods or documenting that callers must validate externally.Also applies to: 322-367
| pub fn new(challenge_id: &str, quorum_size: usize) -> Self { | ||
| Self { | ||
| challenge_id: challenge_id.to_string(), | ||
| quorum_size, | ||
| proposal_timeout_ms: 30_000, | ||
| namespace_prefix: format!("validated:{}", challenge_id), | ||
| require_wasm_validation: true, | ||
| } | ||
| } |
There was a problem hiding this comment.
quorum_size = 0 allows auto-consensus with zero votes.
No validation on quorum_size in the constructor. Setting it to 0 makes approving.len() >= 0 always true (line 546), so check_consensus would reach consensus before any votes are cast. Add a minimum bound.
Proposed fix
pub fn new(challenge_id: &str, quorum_size: usize) -> Self {
+ assert!(quorum_size >= 1, "quorum_size must be at least 1");
Self {
challenge_id: challenge_id.to_string(),
quorum_size,📝 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.
| pub fn new(challenge_id: &str, quorum_size: usize) -> Self { | |
| Self { | |
| challenge_id: challenge_id.to_string(), | |
| quorum_size, | |
| proposal_timeout_ms: 30_000, | |
| namespace_prefix: format!("validated:{}", challenge_id), | |
| require_wasm_validation: true, | |
| } | |
| } | |
| pub fn new(challenge_id: &str, quorum_size: usize) -> Self { | |
| assert!(quorum_size >= 1, "quorum_size must be at least 1"); | |
| Self { | |
| challenge_id: challenge_id.to_string(), | |
| quorum_size, | |
| proposal_timeout_ms: 30_000, | |
| namespace_prefix: format!("validated:{}", challenge_id), | |
| require_wasm_validation: true, | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@crates/distributed-storage/src/validated_storage.rs` around lines 127 - 135,
The constructor new currently allows quorum_size == 0 which makes
check_consensus treat approving.len() >= quorum_size as true immediately; update
new (the function new that sets the quorum_size field) to enforce a minimum
bound (e.g., require quorum_size >= 1) by either asserting/validating and
panicking on invalid input or normalizing the value (e.g., quorum_size =
std::cmp::max(1, quorum_size)); ensure the change references quorum_size and
that check_consensus/approving.len() behavior is no longer able to
auto-consensus with zero votes.
| #[derive(Clone, Debug, Serialize, Deserialize)] | ||
| pub struct StorageWriteProposal { | ||
| pub proposal_id: [u8; 32], | ||
| pub challenge_id: String, | ||
| pub proposer: Hotkey, | ||
| pub key: Vec<u8>, | ||
| pub value: Vec<u8>, | ||
| pub value_hash: [u8; 32], | ||
| pub timestamp: i64, | ||
| pub signature: Vec<u8>, | ||
| } |
There was a problem hiding this comment.
Signature fields on proposals and votes are never populated or verified.
Both StorageWriteProposal (line 157) and StorageWriteVote (line 227) have signature: Vec<u8> fields that are always initialized to empty vectors. The InvalidSignature error variant exists but is never returned. The PR description claims "cryptographic signatures on proposals and votes," but this is not implemented.
If signatures are planned for a follow-up, consider documenting that explicitly (e.g., a TODO or tracking issue). Otherwise, unsigned proposals/votes undermine the security model since any node can forge votes from arbitrary hotkeys.
Would you like me to open an issue to track the signature implementation?
Also applies to: 220-228
🤖 Prompt for AI Agents
In `@crates/distributed-storage/src/validated_storage.rs` around lines 148 - 158,
The structs StorageWriteProposal and StorageWriteVote declare signature: Vec<u8>
but signatures are never produced or checked, so implement end-to-end signing
and verification: when creating proposals/votes (where StorageWriteProposal and
StorageWriteVote instances are constructed) sign the canonical serialized
payload with the proposer's/voter's Hotkey private key and populate signature,
and when validating incoming proposals/votes verify the signature against the
Hotkey public key and return the InvalidSignature error variant on failure; if
you intend to defer implementation instead add an explicit TODO/issue reference
in the creation/validation code and comment on
StorageWriteProposal/StorageWriteVote to avoid misleading claims about
cryptographic signatures.
| pub struct ValidatedStorage<S: DistributedStore> { | ||
| inner: Arc<S>, | ||
| config: ValidatedStorageConfig, | ||
| local_hotkey: Hotkey, | ||
| proposals: Arc<RwLock<HashMap<[u8; 32], ProposalState>>>, | ||
| committed: Arc<RwLock<HashMap<[u8; 32], ConsensusResult>>>, | ||
| } |
There was a problem hiding this comment.
WasmStorageValidator is defined but never integrated into ValidatedStorage.
The WasmStorageValidator trait (line 702) and the require_wasm_validation config field (line 123) exist, but ValidatedStorage has no field for a validator and never calls validate_write during vote_on_proposal or receive_proposal. This means WASM validation—the core feature described in the PR title—is effectively a no-op.
Consider adding a validator: Arc<dyn WasmStorageValidator> field to ValidatedStorage and calling it during the vote phase:
Proposed structural change
pub struct ValidatedStorage<S: DistributedStore> {
inner: Arc<S>,
config: ValidatedStorageConfig,
local_hotkey: Hotkey,
proposals: Arc<RwLock<HashMap<[u8; 32], ProposalState>>>,
committed: Arc<RwLock<HashMap<[u8; 32], ConsensusResult>>>,
+ validator: Arc<dyn WasmStorageValidator>,
}Also applies to: 702-722
🤖 Prompt for AI Agents
In `@crates/distributed-storage/src/validated_storage.rs` around lines 327 - 333,
ValidatedStorage currently never invokes WasmStorageValidator so WASM validation
is unused; add a validator field to ValidatedStorage (e.g., validator:
Option<Arc<dyn WasmStorageValidator + Send + Sync>>) and populate it when
building ValidatedStorage based on the require_wasm_validation flag, then call
validator.validate_write(...) inside vote_on_proposal and receive_proposal (use
the same inputs those methods already have: proposal payload and metadata)
before creating/recording a vote or accepting a proposal; on validation failure
return/reject the vote path (or log and mark proposal as invalid in
proposals/committed) so invalid writes do not proceed.
| pub async fn cleanup_expired(&self) -> usize { | ||
| let mut proposals = self.proposals.write().await; | ||
| let timeout = self.config.proposal_timeout_ms; | ||
| let before = proposals.len(); | ||
|
|
||
| proposals.retain(|_, state| { | ||
| !state.proposal.is_expired(timeout) || state.consensus_result.is_some() | ||
| }); | ||
|
|
||
| let removed = before - proposals.len(); | ||
| if removed > 0 { | ||
| debug!(removed, "Cleaned up expired proposals"); | ||
| } | ||
| removed | ||
| } |
There was a problem hiding this comment.
committed map and consensus-reached proposals are never cleaned up — potential memory leak.
cleanup_expired (line 690–692) retains proposals where consensus_result.is_some(), and the committed map (line 332) has no cleanup at all. In a long-running node with many challenges, both maps grow without bound. Consider adding a TTL or max-size eviction for committed results.
🤖 Prompt for AI Agents
In `@crates/distributed-storage/src/validated_storage.rs` around lines 685 - 699,
cleanup_expired currently keeps proposals with consensus_result.is_some()
forever and there is no cleanup for the committed map, causing memory growth;
update cleanup_expired (and/or add a new cleanup_committed) to evict
consensus-reached proposals after a TTL or when their consensus timestamp is
older than config.proposal_timeout_ms (or a new config like committed_ttl_ms),
and add symmetric eviction for the committed map (the committed field) using the
same TTL or a max-size policy so entries in committed are removed once stale;
locate and update the proposals.retain closure in cleanup_expired and implement
a new cleanup path that checks state.consensus_result.timestamp (or stores a
timestamp when setting consensus_result) and the committed map pruning logic to
remove stale entries.
| fn propose_write( | ||
| &self, | ||
| challenge_id: &str, | ||
| key: &[u8], | ||
| value: &[u8], | ||
| ) -> Result<[u8; 32], StorageHostError> { | ||
| let mut data = self | ||
| .data | ||
| .write() | ||
| .map_err(|e| StorageHostError::InternalError(format!("lock poisoned: {}", e)))?; | ||
| let challenge_data: &mut HashMap<Vec<u8>, Vec<u8>> = | ||
| data.entry(challenge_id.to_string()).or_default(); | ||
| challenge_data.insert(key.to_vec(), value.to_vec()); | ||
|
|
||
| let mut hasher = Sha256::new(); | ||
| hasher.update(challenge_id.as_bytes()); | ||
| hasher.update(key); | ||
| hasher.update(value); | ||
| Ok(hasher.finalize().into()) | ||
| } |
There was a problem hiding this comment.
InMemoryStorageBackend::propose_write immediately persists data, bypassing consensus semantics.
The method name propose_write (from the StorageBackend trait) implies a proposal that awaits consensus, but this implementation inserts the value directly into the store (line 347). This is semantically inconsistent with the validated-storage design where writes should only succeed after quorum approval.
If this is intentional for testing/development, consider documenting it clearly (e.g., a doc comment on the impl). If not, the backend should only record the proposed key/value and return a proposal ID, deferring actual insertion to a separate commit step.
🤖 Prompt for AI Agents
In `@crates/wasm-runtime-interface/src/storage.rs` around lines 335 - 354, The
propose_write implementation currently inserts values immediately into the main
store (the HashMap under self.data) which bypasses consensus; change
propose_write (InMemoryStorageBackend::propose_write) to only record the
proposal (e.g., store the key/value in a dedicated proposals map or a proposals
entry keyed by the returned proposal id) and return the SHA-256 proposal id,
deferring any mutation of the primary data store until a separate commit method
(e.g., commit_write/commit_proposal) applies the proposal; alternatively, if
immediate persistence is intentionally allowed for tests, add a clear doc
comment on InMemoryStorageBackend::propose_write stating it bypasses consensus
semantics and/or gate it behind a test-only flag.
1 participant
Summary
Introduces a per-challenge validated storage system where validators must reach consensus before data is accepted. WASM code defines validation rules, and writes only succeed when a configurable quorum of validators agree.
Changes
validated_storagemodule todistributed-storagecrate with:ValidatedStoragestruct for per-challenge storage with consensusStorageWriteProposalandStorageWriteVotetypes for the proposal/vote flowWasmStorageValidatortrait andDefaultWasmValidatorimplementationstoragemodule towasm-runtime-interfacecrate with host functions for WASM-based validationsha2dependency for content hashingBreaking Changes
None - this is a new additive feature.
Summary by CodeRabbit
Release Notes