refactor: strip non-chunk data types, harden payment, add AlreadyStored optimization#28
Merged
mickvandijke merged 7 commits intomainfrom Mar 18, 2026
Merged
refactor: strip non-chunk data types, harden payment, add AlreadyStored optimization#28mickvandijke merged 7 commits intomainfrom
mickvandijke merged 7 commits intomainfrom
Conversation
…ed optimization Remove all non-chunk data types (GraphEntry, Pointer, Scratchpad) from the protocol layer, storage handler, and client module. Delete the deprecated QuantumClient (891 lines) and self_encrypt wrapper (819 lines) -- their functionality now lives in the saorsa-client crate. Delete the saorsa-cli binary (636 lines) -- replaced by the ant CLI in saorsa-client. Payment hardening: - Remove --disable-payment-verification flag and PaymentConfig::enabled field. Payment verification is now always on -- no production bypass. - Make local_rewards_address required (not Option) on PaymentVerifierConfig. Nodes must know their own rewards address. - Add cache_insert() on PaymentVerifier for test setups (replaces the old disable-EVM test bypass). - Add AlreadyStored variant to ChunkQuoteResponse. When handle_quote() detects a chunk already exists in LMDB, it returns AlreadyStored instead of generating a priced quote -- preventing duplicate payments. Storage additions: - Add put_raw(), put_overwrite(), get_raw() on LmdbStorage for non-content-addressed data (preparing for future mutable data types). Code quality: - Replace direct-indexed xor_distance loop with std::array::from_fn. - Gut E2E tests that depended on deleted QuantumClient. Protocol-level security tests preserved. Client-level tests ported to saorsa-client. 30 files changed, +438 / -5207 lines.
grumbach
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors saorsa-node to focus the protocol/storage layers on chunks only, deletes deprecated client/CLI components now owned by saorsa-client, and hardens the payment path by removing all “disable verification” bypasses and adding an AlreadyStored quote response to prevent duplicate payments.
Changes:
- Remove non-chunk data types and delete the deprecated
QuantumClient,self_encryptwrapper, andsaorsa-clibinary. - Enforce always-on EVM payment verification (remove disable flags/fields, require local rewards address) and add cache preloading helpers for tests.
- Extend LMDB storage with raw/overwrite APIs for future non-content-addressed (mutable) data types, and update test harnesses accordingly.
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/testnet.rs | Removes QuantumClient usage and switches test PUTs to pre-populating the payment cache. |
| tests/e2e/security_attacks.rs | Removes E2E attack scenarios that depended on QuantumClient; keeps protocol-level proof-size/security tests. |
| tests/e2e/payment_flow.rs | Removes QuantumClient-based end-to-end payment flow tests. |
| tests/e2e/mod.rs | Adds clippy allowances to accommodate updated testnet module patterns. |
| tests/e2e/integration_tests.rs | Removes the QuantumClient round-trip integration test. |
| tests/e2e/harness.rs | Adds helper to prepopulate payment cache on a specific peer for test setup. |
| tests/e2e/data_types/scratchpad.rs | Deletes scratchpad E2E tests (non-chunk type removed). |
| tests/e2e/data_types/pointer.rs | Deletes pointer E2E tests (non-chunk type removed). |
| tests/e2e/data_types/mod.rs | Updates data-type test module to chunk-only and removes constants for deleted types. |
| tests/e2e/data_types/graph_entry.rs | Deletes graph-entry E2E tests (non-chunk type removed). |
| tests/e2e/data_types/chunk.rs | Updates protocol creation calls and removes QuantumClient-driven payment tests. |
| tests/e2e/complete_payment_e2e.rs | Removes QuantumClient-based complete payment E2E suite; leaves migration note. |
| src/storage/lmdb.rs | Adds put_raw, put_overwrite, get_raw for non-content-addressed storage preparation. |
| src/storage/handler.rs | Adds AlreadyStored quote path and exposes payment verifier/cache accessors used by tests. |
| src/payment/verifier.rs | Removes enabled toggles and implements always-on verification plus cache_insert() for tests. |
| src/payment/mod.rs | Updates module docs to reflect always-on payment and test-cache strategy. |
| src/node.rs | Removes payment-disable paths and requires a configured rewards address; slightly refactors protocol message handling/logs. |
| src/lib.rs | Removes old client/self-encrypt re-exports; adds saorsa_core re-exports for downstream consumers. |
| src/devnet.rs | Updates devnet payment verifier setup to always-on EVM and requires rewards address. |
| src/config.rs | Removes PaymentConfig::enabled and updates defaults/tests accordingly. |
| src/client/self_encrypt.rs | Deletes the self-encryption wrapper (moved out to saorsa-client). |
| src/client/quantum.rs | Deletes QuantumClient implementation (moved out to saorsa-client). |
| src/client/mod.rs | Reframes module as protocol helpers and rehomes hex_node_id_to_encoded_peer_id. |
| src/client/data_types.rs | Refactors xor_distance using std::array::from_fn. |
| src/bin/saorsa-node/cli.rs | Removes --disable-payment-verification and always configures payment verification. |
| src/bin/saorsa-cli/main.rs | Deletes saorsa-cli binary entrypoint (replaced by ant CLI in saorsa-client). |
| src/bin/saorsa-cli/cli.rs | Deletes saorsa-cli clap definitions/tests. |
| src/ant_protocol/mod.rs | Updates docs to reflect chunk-only protocol support. |
| src/ant_protocol/chunk.rs | Adds ChunkQuoteResponse::AlreadyStored. |
| Cargo.toml | Removes saorsa-cli bin and updates self_encryption dependency source/version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+340
to
+365
|
|
||
| spawn_blocking(move || -> Result<()> { | ||
| let mut wtxn = env | ||
| .write_txn() | ||
| .map_err(|e| Error::Storage(format!("Failed to create write txn: {e}")))?; | ||
|
|
||
| db.put(&mut wtxn, &key, &value) | ||
| .map_err(|e| Error::Storage(format!("Failed to put record: {e}")))?; | ||
| wtxn.commit() | ||
| .map_err(|e| Error::Storage(format!("Failed to commit put: {e}")))?; | ||
| Ok(()) | ||
| }) | ||
| .await | ||
| .map_err(|e| Error::Storage(format!("LMDB put_overwrite task failed: {e}")))??; | ||
|
|
||
| { | ||
| let mut stats = self.stats.write(); | ||
| stats.chunks_stored += 1; | ||
| stats.bytes_stored += data.len() as u64; | ||
| } | ||
|
|
||
| debug!( | ||
| "Overwritten record {} ({} bytes)", | ||
| hex::encode(address), | ||
| data.len() | ||
| ); |
Comment on lines
+355
to
+359
| { | ||
| let mut stats = self.stats.write(); | ||
| stats.chunks_stored += 1; | ||
| stats.bytes_stored += data.len() as u64; | ||
| } |
|
|
||
| /// Get a reference to the payment verifier. | ||
| #[must_use] | ||
| pub fn payment_verifier(&self) -> &PaymentVerifier { |
| /// | ||
| /// This marks the address as already paid, so subsequent `verify_payment` | ||
| /// calls will return `CachedAsVerified` without on-chain verification. | ||
| /// Useful for test setups where real EVM payment is not needed. |
Comment on lines
+357
to
+366
| for node in self.network.nodes() { | ||
| if let Some(ref p2p) = node.p2p_node { | ||
| if p2p.peer_id() == peer_id { | ||
| if let Some(ref protocol) = node.ant_protocol { | ||
| protocol.payment_verifier().cache_insert(*address); | ||
| } | ||
| return; | ||
| } | ||
| } | ||
| } |
Comment on lines
+234
to
+239
| // If the chunk is already stored, no payment is needed | ||
| match self.storage.exists(&request.address) { | ||
| Ok(true) => { | ||
| debug!("Chunk {addr_hex} already stored — returning AlreadyStored"); | ||
| return ChunkQuoteResponse::AlreadyStored; | ||
| } |
Comment on lines
+250
to
+267
| /// Store data at the given address without content-address verification. | ||
| /// | ||
| /// Use this for data types where the address is not `BLAKE3(content)`. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error if the write fails. | ||
| pub async fn put_raw(&self, address: &XorName, data: &[u8]) -> Result<bool> { | ||
| // Fast-path duplicate check | ||
| if self.exists(address)? { | ||
| trace!("Record {} already exists", hex::encode(address)); | ||
| self.stats.write().duplicates += 1; | ||
| return Ok(false); | ||
| } | ||
|
|
||
| let key = *address; | ||
| let value = data.to_vec(); | ||
| let env = self.env.clone(); |
Add MultiAddr, NodeMode, IPDiversityConfig, MlDsa65, NodeIdentity, and P2PEvent to the pub mod core re-exports so saorsa-client can drop its direct saorsa-core dependency.
Greptile SummaryThis PR is a significant cleanup and hardening refactor: it strips ~2,600 lines of deprecated client code ( Key changes:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/ant_protocol/chunk.rs | Adds already_stored: bool field to ChunkQuoteResponse::Success — a backwards-incompatible postcard wire change without a protocol version bump. |
| src/payment/verifier.rs | Removes enabled bypass field, adds cache_insert() for test-only cache pre-population. Security hardening is solid except cache_insert is not #[cfg(test)]-gated. |
| src/storage/handler.rs | Adds AlreadyStored optimisation to handle_quote, payment_verifier() accessor for tests, and put_local test helper. The payment_verifier() accessor is not #[cfg(test)]-gated. |
| src/storage/lmdb.rs | Adds put_raw, put_overwrite, and get_raw for non-content-addressed data. Stats tracking mixes chunk and raw-record counters under chunks_stored. |
| src/node.rs | Rewards address is now required at startup; handler_info pattern prepares for future protocol IDs. Clean and correct. |
| Cargo.toml | Switches self_encryption from a git branch to crates.io 0.35.0 and removes saorsa-cli binary entry. Clean dependency changes. |
Sequence Diagram
sequenceDiagram
participant C as Client
participant N as Node (AntProtocol)
participant L as LmdbStorage
participant Q as QuoteGenerator
participant EVM as Arbitrum EVM
C->>N: ChunkQuoteRequest(address, data_size)
N->>L: exists(address)?
L-->>N: true / false
alt already_stored = true
N->>Q: create_quote(address, …)
Q-->>N: PaymentQuote
N-->>C: ChunkQuoteResponse::Success { quote, already_stored: true }
Note over C: Skip payment & PUT cycle
else already_stored = false
N->>Q: create_quote(address, …)
Q-->>N: PaymentQuote
N-->>C: ChunkQuoteResponse::Success { quote, already_stored: false }
C->>EVM: payForQuotes(quote)
EVM-->>C: payment_proof
C->>N: ChunkPutRequest(address, content, payment_proof)
N->>N: validate size & BLAKE3(content)==address
N->>L: exists(address)?
L-->>N: false
N->>N: PaymentVerifier::verify_payment(address, proof)
N->>EVM: verify_data_payment(digest)
EVM-->>N: ok
N->>L: put(address, content)
L-->>N: stored
N-->>C: ChunkPutResponse::Success { address }
end
Comments Outside Diff (3)
-
src/storage/handler.rs, line 294-298 (link)payment_verifier()accessor is not test-onlypayment_verifier()is an unrestricted public method that exposescache_insert()to any caller — including production code. The PR's stated goal is "no production bypass," but both this accessor andPaymentVerifier::cache_insert(verifier.rs line 249) are compilable and callable in release builds.Marking these
#[cfg(test)]would make the intent explicit and prevent future callers from accidentally (or intentionally) bypassing EVM verification in a production node:The same
#[cfg(test)]guard should be applied toPaymentVerifier::cache_insertinsrc/payment/verifier.rs. -
src/ant_protocol/chunk.rs, line 224-238 (link)Breaking postcard wire format change without a protocol version bump
Adding
already_stored: boolto theChunkQuoteResponse::Successstruct variant is a backwards-incompatible wire protocol change under postcard serialization.Postcard serializes struct fields in order, so:
- Old wire format:
Success=[variant_tag, len(quote), quote_bytes…] - New wire format:
Success=[variant_tag, len(quote), quote_bytes…, already_stored_byte]
A node running the old protocol will fail to decode new
Successmessages (unexpected trailing byte). A new node will fail to decode oldSuccessmessages (missingalready_storedfield). This affects any mixed-version deployment where both old and new nodes coexist.Despite this change,
PROTOCOL_VERSIONremains1andCHUNK_PROTOCOL_IDremains"saorsa/ant/chunk/v1", so peers have no way to negotiate compatibility.Options to address this:
- Bump
PROTOCOL_VERSIONandCHUNK_PROTOCOL_ID(e.g."saorsa/ant/chunk/v2") so old and new nodes can coexist. - Use a separate
AlreadyStoredvariant instead of adding a field toSuccess, and handle old clients seeing an unknown variant gracefully. - If this is intentionally a hard cut-over (no mixed-version rollout planned), document this explicitly in the protocol constants.
- Old wire format:
-
src/storage/lmdb.rs, line 257-324 (link)put_rawupdateschunks_storedstat, conflating chunks and raw recordsput_rawis described as being "for non-content-addressed data types" (mutable records, future Pointers, Scratchpads, etc.), but on a successful write it incrementsstats.chunks_storedandstats.bytes_stored— the same counters used byputfor content-addressed chunks. This will make it impossible to distinguish chunk-storage from raw-record-storage in the stats output atStorageStats.Consider adding
records_stored/bytes_records_storedcounters toStorageStats, or documenting clearly that these counters now cover all entry types rather than chunks only. Similarly, themax_chunkscapacity limit input_rawsilently caps all stored entries regardless of type — the field name makes this surprising to operators.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/storage/handler.rs
Line: 294-298
Comment:
**`payment_verifier()` accessor is not test-only**
`payment_verifier()` is an unrestricted public method that exposes `cache_insert()` to any caller — including production code. The PR's stated goal is "no production bypass," but both this accessor and `PaymentVerifier::cache_insert` (verifier.rs line 249) are compilable and callable in release builds.
Marking these `#[cfg(test)]` would make the intent explicit and prevent future callers from accidentally (or intentionally) bypassing EVM verification in a production node:
```suggestion
/// Get a reference to the payment verifier.
#[must_use]
#[cfg(test)]
pub fn payment_verifier(&self) -> &PaymentVerifier {
&self.payment_verifier
}
```
The same `#[cfg(test)]` guard should be applied to `PaymentVerifier::cache_insert` in `src/payment/verifier.rs`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/ant_protocol/chunk.rs
Line: 224-238
Comment:
**Breaking postcard wire format change without a protocol version bump**
Adding `already_stored: bool` to the `ChunkQuoteResponse::Success` struct variant is a **backwards-incompatible wire protocol change** under postcard serialization.
Postcard serializes struct fields in order, so:
- **Old wire format:** `Success` = `[variant_tag, len(quote), quote_bytes…]`
- **New wire format:** `Success` = `[variant_tag, len(quote), quote_bytes…, already_stored_byte]`
A node running the old protocol will fail to decode new `Success` messages (unexpected trailing byte). A new node will fail to decode old `Success` messages (missing `already_stored` field). This affects any mixed-version deployment where both old and new nodes coexist.
Despite this change, `PROTOCOL_VERSION` remains `1` and `CHUNK_PROTOCOL_ID` remains `"saorsa/ant/chunk/v1"`, so peers have no way to negotiate compatibility.
Options to address this:
1. Bump `PROTOCOL_VERSION` and `CHUNK_PROTOCOL_ID` (e.g. `"saorsa/ant/chunk/v2"`) so old and new nodes can coexist.
2. Use a separate `AlreadyStored` variant instead of adding a field to `Success`, and handle old clients seeing an unknown variant gracefully.
3. If this is intentionally a hard cut-over (no mixed-version rollout planned), document this explicitly in the protocol constants.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/storage/lmdb.rs
Line: 257-324
Comment:
**`put_raw` updates `chunks_stored` stat, conflating chunks and raw records**
`put_raw` is described as being "for non-content-addressed data types" (mutable records, future Pointers, Scratchpads, etc.), but on a successful write it increments `stats.chunks_stored` and `stats.bytes_stored` — the same counters used by `put` for content-addressed chunks. This will make it impossible to distinguish chunk-storage from raw-record-storage in the stats output at `StorageStats`.
Consider adding `records_stored` / `bytes_records_stored` counters to `StorageStats`, or documenting clearly that these counters now cover all entry types rather than chunks only. Similarly, the `max_chunks` capacity limit in `put_raw` silently caps all stored entries regardless of type — the field name makes this surprising to operators.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: replace Already..."
…write stats - ChunkQuoteResponse::AlreadyStored was a new variant that clients matching only Success/Error would miss, causing silent timeouts. Replace with `already_stored: bool` on Success so existing client match arms work unchanged. - handle_quote() checks storage.exists() and sets the flag; quote is still generated normally so the client has full pricing info. - put_overwrite() now reads the old value length before writing and adjusts bytes_stored correctly on updates instead of unconditionally incrementing chunks_stored + bytes_stored.
There was a problem hiding this comment.
Pull request overview
This PR refactors saorsa-node to be chunk-only at the protocol/client/test layers, removes deprecated client/CLI functionality now housed in saorsa-client, and hardens payment verification by making EVM verification unconditional and requiring a configured rewards address.
Changes:
- Remove non-chunk protocol/client/test support and delete deprecated
QuantumClient,self_encryptwrapper, and thesaorsa-clibinary. - Harden payments: eliminate disable-bypasses, require
local_rewards_address, and add test-only cache priming viaPaymentVerifier::cache_insert()+AntProtocol::payment_verifier(). - Add LMDB raw/overwrite APIs and extend quote responses with an
already_storedsignal.
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/testnet.rs | Removes QuantumClient usage and switches tests to cache pre-population for payment acceptance. |
| tests/e2e/security_attacks.rs | Removes E2E flows dependent on QuantumClient / on-chain payment wiring; keeps protocol-level attack tests. |
| tests/e2e/payment_flow.rs | Deletes QuantumClient-based payment E2E flows (migration pending). |
| tests/e2e/mod.rs | Adjusts clippy allowances for the testnet module. |
| tests/e2e/integration_tests.rs | Removes QuantumClient round-trip E2E test now that the client moved out. |
| tests/e2e/harness.rs | Adds helper to prepopulate payment cache for a peer in tests. |
| tests/e2e/data_types/scratchpad.rs | Deletes scratchpad E2E tests (non-chunk types removed). |
| tests/e2e/data_types/pointer.rs | Deletes pointer E2E tests (non-chunk types removed). |
| tests/e2e/data_types/mod.rs | Updates data-types test module to be chunk-only. |
| tests/e2e/data_types/graph_entry.rs | Deletes graph entry E2E tests (non-chunk types removed). |
| tests/e2e/data_types/chunk.rs | Updates chunk E2E tests to prime payment cache instead of disabling verification. |
| tests/e2e/complete_payment_e2e.rs | Removes QuantumClient-based “complete payment” E2E tests and leaves a migration note. |
| src/storage/lmdb.rs | Adds put_raw, put_overwrite, and get_raw for non-content-addressed storage. |
| src/storage/handler.rs | Adds quote “already stored” check and exposes a payment_verifier() accessor for tests. |
| src/payment/verifier.rs | Makes EVM verification always-on; requires local_rewards_address; adds cache_insert(). |
| src/payment/mod.rs | Updates payment module docs for always-on verification and test cache priming. |
| src/node.rs | Removes payment-disable paths; requires rewards address at startup; refactors protocol dispatch loop. |
| src/lib.rs | Removes QuantumClient/self_encrypt re-exports; adds core re-exports and hex_node_id_to_encoded_peer_id. |
| src/devnet.rs | Updates devnet payment verifier initialization for always-on EVM verification. |
| src/config.rs | Removes PaymentConfig::enabled and updates docs/tests accordingly. |
| src/client/self_encrypt.rs | Deletes the in-crate self-encryption wrapper (moved to saorsa-client). |
| src/client/quantum.rs | Deletes deprecated QuantumClient implementation (moved to saorsa-client). |
| src/client/mod.rs | Reorients module as protocol helpers; keeps hex_node_id_to_encoded_peer_id locally. |
| src/client/data_types.rs | Refactors xor_distance to use std::array::from_fn. |
| src/bin/saorsa-node/cli.rs | Removes --disable-payment-verification and updates payment config wiring. |
| src/bin/saorsa-cli/main.rs | Deletes deprecated saorsa-cli binary. |
| src/bin/saorsa-cli/cli.rs | Deletes deprecated saorsa-cli CLI definitions/tests. |
| src/ant_protocol/mod.rs | Updates protocol docs to reflect chunk-only support. |
| src/ant_protocol/chunk.rs | Extends quote response format with already_stored. |
| Cargo.toml | Removes saorsa-cli bin and switches self_encryption to crates.io 0.35.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
223
to
235
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub enum ChunkQuoteResponse { | ||
| /// Quote generated successfully. | ||
| /// | ||
| /// When `already_stored` is `true` the node already holds this chunk and no | ||
| /// payment is required — the client should skip the pay-then-PUT cycle for | ||
| /// this address. The quote is still included for informational purposes. | ||
| Success { | ||
| /// Serialized `PaymentQuote`. | ||
| quote: Vec<u8>, | ||
| /// `true` when the chunk already exists on this node (skip payment). | ||
| already_stored: bool, | ||
| }, |
Comment on lines
+335
to
+339
| pub async fn put_overwrite(&self, address: &XorName, data: &[u8]) -> Result<()> { | ||
| let key = *address; | ||
| let value = data.to_vec(); | ||
| let env = self.env.clone(); | ||
| let db = self.db; |
Comment on lines
+352
to
+366
| pub fn prepopulate_payment_cache_for_peer( | ||
| &self, | ||
| peer_id: &saorsa_core::identity::PeerId, | ||
| address: &XorName, | ||
| ) { | ||
| for node in self.network.nodes() { | ||
| if let Some(ref p2p) = node.p2p_node { | ||
| if p2p.peer_id() == peer_id { | ||
| if let Some(ref protocol) = node.ant_protocol { | ||
| protocol.payment_verifier().cache_insert(*address); | ||
| } | ||
| return; | ||
| } | ||
| } | ||
| } |
Comment on lines
+581
to
586
| let evm_config = EvmVerifierConfig { | ||
| network: config | ||
| .evm_network | ||
| .clone() | ||
| .unwrap_or(EvmNetwork::ArbitrumOne), | ||
| }; |
| }; | ||
|
|
||
| if already_stored { | ||
| debug!("Chunk {addr_hex} already stored — returning quote with already_stored=true"); |
Collaborator
Author
|
@claude @greptile please review |
|
Claude finished @grumbach's task —— View job Review of PR #28Todo List:
OverviewThis is a well-executed refactoring that successfully removes ~5,200 lines of deprecated code while hardening the payment verification system. The PR correctly enforces payment-always-on at the type level and adds sensible optimizations for duplicate storage prevention. High-Level AssessmentStrengths:
Areas of Concern: Critical Issues1.
|
…ct payment_verifier() - Add test_quote_already_stored_flag: verifies already_stored=true for existing chunks and false for new ones in the quote response path. Wires ML-DSA-65 signing in create_test_protocol so quote requests work. - Add test_put_raw_and_get_raw, test_get_raw_not_found, test_put_overwrite_new_record, test_put_overwrite_updates_stats_correctly: full coverage for the new storage methods including stats correctness. - Mark payment_verifier() as #[doc(hidden)] with clear test-only docs. - Remove duplicate trace log in put_raw (race-condition path). - Make prepopulate_payment_cache_for_peer panic on missing peer to catch test configuration errors early.
…vm field - Gate `PaymentVerifier::cache_insert()` and `AntProtocol::payment_verifier()` behind `#[cfg(any(test, feature = "test-utils"))]` so production builds cannot bypass on-chain payment verification. - Add `test-utils` feature to Cargo.toml; E2E tests require it. - Remove dead `DevnetConfig::enable_evm` field (no longer affects behavior since `EvmVerifierConfig::enabled` was removed). - Update CLAUDE.md payment docs to reflect hardened payment policy. - Remove unnecessary `.to_string()` allocation in message dispatch hot path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mickvandijke
approved these changes
Mar 18, 2026
There was a problem hiding this comment.
Pull request overview
This PR refactors saorsa-node to remove deprecated non-chunk protocol/client layers (including QuantumClient, self_encrypt, and the old CLI), hardens payment verification by making EVM verification unconditional and requiring a rewards address, and adds a quote-time optimization to signal when a chunk is already stored.
Changes:
- Remove deprecated client/CLI/self-encryption code paths and update E2E tests to the new testing strategy (cache prepopulation instead of “disable EVM”).
- Enforce “payment verification always on” across config, node startup, devnet, and verifier logic (with new test-only cache insertion helpers).
- Add storage APIs for non-content-addressed records (
put_raw,put_overwrite,get_raw) and introducealready_storedflag on quote success responses.
Reviewed changes
Copilot reviewed 28 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/testnet.rs | Removes QuantumClient usage; updates PUT flows to pre-populate payment cache for tests. |
| tests/e2e/security_attacks.rs | Removes payment E2E attack scenarios dependent on deleted client/bypass paths; keeps protocol-level tests. |
| tests/e2e/payment_flow.rs | Removes QuantumClient-based payment flow tests (migration pending). |
| tests/e2e/mod.rs | Adds clippy allowances for the testnet module. |
| tests/e2e/integration_tests.rs | Removes QuantumClient round-trip E2E test. |
| tests/e2e/harness.rs | Adds helper to prepopulate a node’s payment cache by peer ID. |
| tests/e2e/data_types/mod.rs | Updates data-type E2E module to chunk-only and removes old constants/docs. |
| tests/e2e/data_types/chunk.rs | Updates chunk E2E tests to prepopulate payment cache before unpaid PUTs. |
| tests/e2e/data_types/scratchpad.rs | Deletes scratchpad E2E tests (type removed from protocol layer). |
| tests/e2e/data_types/pointer.rs | Deletes pointer E2E tests (type removed from protocol layer). |
| tests/e2e/data_types/graph_entry.rs | Deletes graph entry E2E tests (type removed from protocol layer). |
| tests/e2e/complete_payment_e2e.rs | Removes old complete payment E2E (pending reimplementation in saorsa-client). |
| src/storage/lmdb.rs | Adds raw record APIs and fixes overwrite stats accounting; adds tests. |
| src/storage/handler.rs | Adds already_stored flag on quote responses; exposes payment_verifier() accessor for tests; updates unit tests to use cache insertion. |
| src/payment/verifier.rs | Removes all “enabled/disable” bypasses; requires local_rewards_address; adds test-only cache_insert. |
| src/payment/mod.rs | Updates documentation to reflect always-on verification and test cache prepopulation. |
| src/node.rs | Removes payment disable paths; makes rewards address required at startup; refactors protocol dispatch. |
| src/lib.rs | Removes deprecated re-exports; re-exports hex_node_id_to_encoded_peer_id and introduces saorsa_node::core re-export module. |
| src/devnet.rs | Removes EVM enable toggle; configures verifier unconditionally and requires rewards address. |
| src/config.rs | Removes PaymentConfig::enabled; updates defaults/tests for always-on verification model. |
| src/client/mod.rs | Removes QuantumClient module; keeps low-level protocol helpers and moves hex_node_id_to_encoded_peer_id here. |
| src/client/data_types.rs | Refactors xor_distance implementation to std::array::from_fn. |
| src/bin/saorsa-node/cli.rs | Removes --disable-payment-verification; updates payment config wiring. |
| src/bin/saorsa-devnet/main.rs | Updates Anvil wiring to set only evm_network (no enable flag). |
| src/bin/saorsa-devnet/cli.rs | Updates --enable-evm help text to match always-on verification behavior. |
| src/bin/saorsa-cli/main.rs | Deletes deprecated saorsa-cli binary. |
| src/bin/saorsa-cli/cli.rs | Deletes deprecated saorsa-cli CLI definitions/tests. |
| src/ant_protocol/mod.rs | Updates protocol docs to reflect chunk-only support. |
| src/ant_protocol/chunk.rs | Adds already_stored field to quote success response and documents semantics. |
| Cargo.toml | Removes saorsa-cli bin; switches self_encryption to crates.io; gates E2E tests behind test-utils. |
| CLAUDE.md | Updates policy documentation to “payment verification always on” and the new testing approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
230
to
+234
| Success { | ||
| /// Serialized `PaymentQuote`. | ||
| quote: Vec<u8>, | ||
| /// `true` when the chunk already exists on this node (skip payment). | ||
| already_stored: bool, |
Comment on lines
+363
to
+367
| if let Some(ref p2p) = node.p2p_node { | ||
| if p2p.peer_id() == peer_id { | ||
| if let Some(ref protocol) = node.ant_protocol { | ||
| protocol.payment_verifier().cache_insert(*address); | ||
| } |
| let ml_dsa = MlDsa65::new(); | ||
| ml_dsa | ||
| .sign(&sk, msg) | ||
| .map_or_else(|_| vec![], |sig| sig.as_bytes().to_vec()) |
Minor version bump reflecting new features and breaking changes since v0.3.1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mickvandijke
added a commit
that referenced
this pull request
Apr 1, 2026
Add unit and e2e tests covering the remaining Section 18 scenarios: Unit tests (32 new): - Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round dual-evidence, #28 dynamic threshold undersized, #33 batched per-key, #34 partial response unresolved, #42 quorum-derived paid-list auth - Admission: #5 unauthorized peer, #7 out-of-range rejected - Config: #18 invalid config rejected, #26 dynamic paid threshold - Scheduling: #8 dedup safety, #8 replica/paid collapse - Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion, #38 snapshot stability mid-join, #39 unreachable removal + slot fill, #40 cooldown peer removed, #41 cycle termination guarantee, consecutive rounds, cycle preserves sync times - Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset on heal, #52 paid/record timestamps independent, #23 entry removal - Audit: #19/#53 partial failure mixed responsibility, #54 all pass, #55 empty failure discard, #56 repair opportunity filter, response count validation, digest uses full record bytes - Types: #13 bootstrap drain, repair opportunity edge cases, terminal state variants - Bootstrap claims: #46 first-seen recorded, #49 cleared on normal E2e tests (4 new): - #2 fresh offer with empty PoP rejected - #5/#37 neighbor sync request returns response - #11 audit challenge multi-key (present + absent) - Fetch not-found for non-existent key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mickvandijke
added a commit
that referenced
this pull request
Apr 1, 2026
Add unit and e2e tests covering the remaining Section 18 scenarios: Unit tests (32 new): - Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round dual-evidence, #28 dynamic threshold undersized, #33 batched per-key, #34 partial response unresolved, #42 quorum-derived paid-list auth - Admission: #5 unauthorized peer, #7 out-of-range rejected - Config: #18 invalid config rejected, #26 dynamic paid threshold - Scheduling: #8 dedup safety, #8 replica/paid collapse - Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion, #38 snapshot stability mid-join, #39 unreachable removal + slot fill, #40 cooldown peer removed, #41 cycle termination guarantee, consecutive rounds, cycle preserves sync times - Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset on heal, #52 paid/record timestamps independent, #23 entry removal - Audit: #19/#53 partial failure mixed responsibility, #54 all pass, #55 empty failure discard, #56 repair opportunity filter, response count validation, digest uses full record bytes - Types: #13 bootstrap drain, repair opportunity edge cases, terminal state variants - Bootstrap claims: #46 first-seen recorded, #49 cleared on normal E2e tests (4 new): - #2 fresh offer with empty PoP rejected - #5/#37 neighbor sync request returns response - #11 audit challenge multi-key (present + absent) - Fetch not-found for non-existent key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove all non-chunk data types (GraphEntry, Pointer, Scratchpad) from the protocol layer, storage handler, and client module. Delete the deprecated
QuantumClient(1121 lines) andself_encryptwrapper (877 lines) — their functionality now lives in thesaorsa-clientcrate. Delete thesaorsa-clibinary (645 lines) — replaced by the CLI insaorsa-client.Payment hardening:
--disable-payment-verificationflag andPaymentConfig::enabledfield. Payment verification is now always on — no production bypass.EvmVerifierConfig::enabledfield. EVM verification is unconditional.local_rewards_addressrequired (notOption) onPaymentVerifierConfig. Nodes must know their own rewards address.rewards_addressrequired inNodeBuilder— node startup fails without one instead of silently using a zero address.cache_insert()onPaymentVerifierfor test setups (replaces the old disable-EVM test bypass).payment_verifier()accessor onAntProtocolso tests can pre-populate the cache.Duplicate-payment prevention:
already_stored: boolfield toChunkQuoteResponse::Success. Whenhandle_quote()detects a chunk already exists in LMDB, it setsalready_stored = trueon the quote response. The quote is still generated normally so clients have full pricing info — the flag simply tells them to skip the pay-then-PUT cycle. No new enum variant; existing client match arms work unchanged.Storage additions:
put_raw(),put_overwrite(),get_raw()onLmdbStoragefor non-content-addressed data (preparing for future mutable data types).put_overwrite()stats: reads old value length before writing and adjustsbytes_storedcorrectly on updates instead of unconditionally incrementingchunks_stored.Public API changes (
lib.rs):QuantumClient,QuantumConfig, and allself_encryptfunctions.hex_node_id_to_encoded_peer_idre-export (moved from deletedquantum.rsintoclient/mod.rs).saorsa_node::coremodule re-exportingPeerId,CoreNodeConfig,NodeMode,P2PNodefromsaorsa-core, so downstream crates can depend onsaorsa-nodealone.Dependency changes:
self_encryptionfrom a git branch dependency to crates.io0.35.0.Message handler refactor (
node.rs):handler_infopattern, preparing for future protocol IDs beyondCHUNK_PROTOCOL_ID.Code quality:
xor_distanceloop withstd::array::from_fn.QuantumClient/ disabled-EVM bypass. Protocol-level security tests preserved. Client-level tests ported tosaorsa-client.verifier.rsandhandler.rsto usecache_insert()instead ofenabled: false.30 files changed, +484 / −5547 lines.