feat: integrate merkle batch payments with ML-DSA-65#33
Conversation
|
@claude @greptile please review |
|
Claude finished @grumbach's task —— View job Code Review CompleteI've completed a thorough review of the merkle batch payments integration. Here's my analysis: SummaryThis PR adds merkle batch payment support to enable clients to pay for large chunk batches in a single on-chain transaction using ML-DSA-65 post-quantum cryptography. The implementation is well-structured with comprehensive test coverage (258 tests, 18 new merkle-specific tests). Strengths✅ Excellent adherence to NO PANICS policy - All production code uses proper error handling ( Critical Issues🔴 CRITICAL: Unsafe cast in devnet.rs (Line 218)Location: multihash_bytes.push(u8::try_from(raw.len()).unwrap_or(32));Issue: This uses Impact: Production code that violates safety requirements. Fix: // SAFETY: PeerId is always 32 bytes, safely fits in u8
let len = u8::try_from(raw.len()).unwrap_or(32);
multihash_bytes.push(len);Or better yet, use a proper error path: let len = u8::try_from(raw.len())
.map_err(|_| Error::Payment(format!("Invalid peer ID length: {}", raw.len())))?;
multihash_bytes.push(len);🟡 MODERATE: Potential overflow in pool cache initialization (Line 131-132)Location: let pool_cache_size =
NonZeroUsize::new(DEFAULT_POOL_CACHE_CAPACITY).unwrap_or(NonZeroUsize::MIN);Issue: Uses Context: The comment says "SAFETY: DEFAULT_POOL_CACHE_CAPACITY is a const > 0, so this is always Some", but relying on runtime unwrap for a compile-time guarantee isn't ideal. Fix: // Since DEFAULT_POOL_CACHE_CAPACITY is const, use const assertion
const_assert!(DEFAULT_POOL_CACHE_CAPACITY > 0);
let pool_cache_size = NonZeroUsize::new(DEFAULT_POOL_CACHE_CAPACITY)
.expect("DEFAULT_POOL_CACHE_CAPACITY must be > 0"); // OK in const contextOr simpler: // SAFETY: DEFAULT_POOL_CACHE_CAPACITY (1000) is a const > 0
let pool_cache_size = unsafe { NonZeroUsize::new_unchecked(DEFAULT_POOL_CACHE_CAPACITY) };Note: The current code won't panic since the const is 1,000, but it violates the "no unwrap" guideline even with Security Review✅ ML-DSA-65 Signature VerificationLocation: The code correctly verifies ML-DSA-65 signatures on all 16 candidate nodes before accepting a merkle proof: for candidate in &merkle_proof.winner_pool.candidate_nodes {
if !crate::payment::verify_merkle_candidate_signature(candidate) {
return Err(Error::Payment(format!(
"Invalid ML-DSA-65 signature on merkle candidate node (reward: {})",
candidate.reward_address
)));
}
}Analysis: This is critical for security - without signature verification, a client could fabricate a pool with fake reward addresses. The contract only commits to the bytes submitted, so offline signature verification ensures authenticity. ✅ Address Binding VerificationLocation: if merkle_proof.address.0 != *xorname {
return Err(Error::Payment(format!(
"Merkle proof address mismatch: proof is for {}, but storing {}",
hex::encode(merkle_proof.address.0),
hex::encode(xorname)
)));
}Analysis: Prevents proof replay attacks by ensuring the proof is for the specific XorName being stored. ✅ Merkle Tree ValidationLocation: Uses
Analysis: Comprehensive cryptographic validation of the merkle batch payment. 🟢 Design Decision: No Local-Recipient Check for Merkle ProofsLocation: // Note: unlike single-node payments, merkle proofs are NOT bound to a
// specific storing node. The contract pays `depth` random nodes from the
// winner pool; the storing node is whichever close-group peer the client
// routes the chunk to.Analysis: This is a deliberate design choice. Merkle batch payments pay a random subset of the pool nodes, not a specific recipient. Replay protection comes from the per-address proof binding (each proof is for a specific XorName in the paid tree), not recipient checking. This is architecturally sound. Code Quality✅ Error HandlingAll new code uses proper error handling:
✅ Documentation
✅ Test CoverageNew merkle tests (18 total):
Test quality: Tests verify ML-DSA-65 signatures, timestamp handling, serialization round-trips, and cache behavior. Architecture✅ Proof Type DispatchLocation: Clean enum-based dispatch: pub enum ProofType {
SingleNode,
Merkle,
}
pub fn detect_proof_type(bytes: &[u8]) -> Option<ProofType> {
match bytes.first() {
Some(&PROOF_TAG_SINGLE_NODE) => Some(ProofType::SingleNode),
Some(&PROOF_TAG_MERKLE) => Some(ProofType::Merkle),
_ => None,
}
}Analysis: Type-safe, extensible design that cleanly separates single-node and merkle payment paths. ✅ Pool-Level CachingLocation: let cached_info = {
let mut pool_cache = self.pool_cache.lock();
pool_cache.get(&pool_hash).cloned()
};Analysis: Smart optimization - for a batch of 1,000 chunks sharing the same pool hash, this reduces on-chain queries from 1,000 to 1. The 1,000-entry LRU provides good hit rates for concurrent batch uploads. ✅ Handler RoutingLocation: ChunkMessageBody::MerkleCandidateQuoteRequest(ref req) => {
ChunkMessageBody::MerkleCandidateQuoteResponse(
self.handle_merkle_candidate_quote(req),
)
}Analysis: Clean integration into existing protocol handler with consistent error handling. Minor Observations🟢 MAX_PAYMENT_PROOF_SIZE raised to 256 KBLocation: const MAX_PAYMENT_PROOF_SIZE_BYTES: usize = 262_144;Justification (from PR description):
Analysis: Reasonable limit. Test at line 937-947 verifies real ML-DSA-65 proofs fit within limits. 🟢 PROOF_TAG ConstantsLocation: pub const PROOF_TAG_SINGLE_NODE: u8 = 0x01;
pub const PROOF_TAG_MERKLE: u8 = 0x02;Analysis: Well-documented in PR description, with tests verifying distinct non-zero tags (line 530-536). Forward-compatible design allows adding new proof types without breaking existing code. 🟢 Data Size Validation in HandlerLocation: let Ok(data_size_usize) = usize::try_from(request.data_size) else {
return MerkleCandidateQuoteResponse::Error(ProtocolError::ChunkTooLarge {
size: MAX_CHUNK_SIZE + 1,
max_size: MAX_CHUNK_SIZE,
});
};
if data_size_usize > MAX_CHUNK_SIZE {
return MerkleCandidateQuoteResponse::Error(ProtocolError::ChunkTooLarge {
size: data_size_usize,
max_size: MAX_CHUNK_SIZE,
});
}Analysis: Proper overflow checking with early return. Consistent with existing quote handler pattern. Recommendations1. Fix the unsafe cast in verifier.rs:1218Priority: Critical 2. Address pool_cache_size initializationPriority: Moderate 3. Consider adding integration test for pool cache evictionPriority: Low 4. Document merkle payment flow in CLAUDE.mdPriority: Low ConclusionThis is a high-quality PR with excellent engineering practices:
Blocking issue: One critical violation of the NO PANICS policy in Non-blocking: The pool cache initialization pattern could be improved but won't cause runtime issues. Overall assessment: Approve pending fix for the unsafe cast. The merkle batch payment integration is architecturally sound and ready for production after addressing the panic violation. Job Run | Branch: |
There was a problem hiding this comment.
Pull request overview
This PR extends the node’s payment and quoting pipeline to support Merkle batch payments, enabling clients to pay for many chunks via a single on-chain MerklePaymentVault transaction while switching Merkle candidate signing/verification to ML-DSA-65.
Changes:
- Added Merkle candidate quote request/response to the ANT chunk protocol and handler-side request routing.
- Introduced tagged proof serialization with proof-type detection and added Merkle proof verification flow (incl. pool-hash cache + ML-DSA candidate signature verification).
- Updated devnet manifest/network plumbing to optionally surface a Merkle payments contract address.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/storage/handler.rs |
Handles MerkleCandidateQuoteRequest and returns serialized candidate nodes; adds a handler-level test. |
src/payment/verifier.rs |
Dispatches by proof tag, adds Merkle proof verification path and a pool-hash → on-chain-info LRU cache; expands tests for tagged proofs. |
src/payment/quote.rs |
Adds ML-DSA-65 merkle candidate quote creation and ML-DSA verification helper. |
src/payment/proof.rs |
Adds proof-type tags, tagged (de)serialization helpers for single-node and Merkle proofs, and corresponding unit tests. |
src/payment/mod.rs |
Re-exports new proof/tag and Merkle candidate verification helpers. |
src/node.rs |
Wires the existing ML-DSA signer for both regular quotes and Merkle candidate quotes. |
src/devnet.rs |
Extends DevnetEvmInfo with optional merkle_payments_address. |
src/bin/saorsa-devnet/main.rs |
Populates merkle_payments_address in the devnet manifest for custom networks. |
src/ant_protocol/mod.rs |
Re-exports new Merkle quote types and proof tag constants. |
src/ant_protocol/chunk.rs |
Adds Merkle candidate quote message types and defines proof tag constants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/storage/handler.rs
Outdated
| return MerkleCandidateQuoteResponse::Error(ProtocolError::ChunkTooLarge { | ||
| size: MAX_CHUNK_SIZE + 1, | ||
| max_size: MAX_CHUNK_SIZE, | ||
| }); |
| pub payment_token_address: String, | ||
| /// Data payments contract address. | ||
| pub data_payments_address: String, | ||
| /// Merkle payments contract address (for batch payments). |
| /// Verify a merkle batch payment proof. | ||
| /// | ||
| /// This verification flow: | ||
| /// 1. Deserialize the `MerklePaymentProof` | ||
| /// 2. Check pool cache for previously verified pool hash | ||
| /// 3. If not cached, query on-chain for payment info | ||
| /// 4. Validate the proof against on-chain data | ||
| /// 5. Cache the pool hash for subsequent chunk verifications in the same batch | ||
| #[allow(clippy::too_many_lines)] | ||
| async fn verify_merkle_payment(&self, xorname: &XorName, proof_bytes: &[u8]) -> Result<()> { | ||
| if tracing::enabled!(tracing::Level::DEBUG) { | ||
| debug!("Verifying merkle payment for {}", hex::encode(xorname)); | ||
| } | ||
|
|
| #[test] | ||
| fn test_pool_cache_same_hash_queries_once() { | ||
| use evmlib::merkle_batch_payment::PoolHash; | ||
|
|
||
| // Verify the pool_cache field exists and works correctly. | ||
| // Insert a pool hash, then verify it's present on lookup. | ||
| let verifier = create_test_verifier(); |
src/payment/verifier.rs
Outdated
| "Unknown payment proof type tag: {:?}", | ||
| proof.first() |
|
@greptile please review |
Greptile SummaryThis PR integrates merkle batch payments into Key design decisions worth noting:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| src/payment/verifier.rs | Core merkle payment verification added. The logic is well-structured: address-match guard → pool LRU cache → on-chain query → ML-DSA signature check → upstream merkle proof verification → paid-node index/address validation. One minor inconsistency: payment_info.depth as usize uses a raw cast while the same function converts pna.poolIndex via the safer usize::from(). |
| src/payment/proof.rs | Clean introduction of ProofType enum, detect_proof_type, and tagged serialize/deserialize for both SingleNode and Merkle proofs. The breaking change (legacy untagged proofs now rejected) is intentional and well-tested. |
| src/payment/quote.rs | Adds create_merkle_candidate_quote and verify_merkle_candidate_signature with correct ML-DSA-65 signing/verification, consistent with the existing PaymentQuote path. Tests cover round-trip and tamper detection. |
| src/storage/handler.rs | Routes MerkleCandidateQuoteRequest to handle_merkle_candidate_quote, with proper data_size bounds checking and safe usize::try_from conversion. MerkleCandidateQuoteResponse is also correctly listed in the "unexpected response" arm. |
| src/ant_protocol/chunk.rs | New MerkleCandidateQuoteRequest/Response types and PROOF_TAG constants added cleanly. All new message variants have encode/decode round-trip tests. |
Sequence Diagram
sequenceDiagram
participant Client
participant Node as saorsa-node (handler)
participant QG as QuoteGenerator
participant PV as PaymentVerifier
participant Cache as Pool LRU Cache
participant Chain as MerklePaymentVault (on-chain)
Note over Client,Chain: Phase 1 — Collect 16 ML-DSA-65 candidate quotes
Client->>Node: MerkleCandidateQuoteRequest(address, data_size, ts)
Node->>QG: create_merkle_candidate_quote(size, type, ts)
QG-->>Node: MerklePaymentCandidateNode (ML-DSA-65 signed)
Node-->>Client: MerkleCandidateQuoteResponse::Success{candidate_node}
Note over Client,Chain: Phase 2 — Client pays on-chain, then uploads chunk
Client->>Chain: submitMerklePayment(pool_hash, ...)
Client->>Node: ChunkPutRequest(xorname, data, MerklePaymentProof[tag=0x02])
Note over Node,Chain: Phase 3 — Verify merkle payment
Node->>PV: verify_payment(xorname, proof_bytes)
PV->>PV: detect_proof_type → ProofType::Merkle
PV->>PV: deserialize_merkle_proof (tag check)
PV->>PV: assert proof.address == xorname
PV->>Cache: get(pool_hash)
alt Cache miss
PV->>Chain: get_merkle_payment_info(pool_hash)
Chain-->>PV: OnChainPaymentInfo{depth, ts, paid_nodes}
PV->>Cache: put(pool_hash, info)
else Cache hit
Cache-->>PV: OnChainPaymentInfo (cached)
end
PV->>PV: verify ML-DSA-65 sigs on all 16 candidates
PV->>PV: verify_merkle_proof (address∈tree, midpoint∈tree, roots match, ts valid)
PV->>PV: paid_nodes.len() == depth check
PV->>PV: paid_node[idx].reward_addr == on-chain addr for each entry
PV-->>Node: Ok(())
Node->>Node: cache.insert(xorname)
Node-->>Client: ChunkPutResponse::Success
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/payment/verifier.rs
Line: 581
Comment:
**`depth as usize` inconsistent with fixed `poolIndex` cast**
The same function already addresses the silent-truncation concern by using `usize::from(pna.poolIndex)` (line 517) — the lossless conversion that produces a compile error if the source type is wider than `usize`. The `depth as usize` cast on this line is inconsistent: on a 32-bit target it would silently truncate a `u64` depth value, turning the length check into a false match for any depth ≥ 2³².
Using `try_from` makes the same safety guarantee already applied to `poolIndex`:
```suggestion
if payment_info.paid_node_addresses.len() != usize::try_from(payment_info.depth).map_err(|_| Error::Payment(format!("depth {} overflows usize", payment_info.depth)))? {
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: address Greptil..."
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/payment/verifier.rs
Outdated
| // SAFETY: DEFAULT_POOL_CACHE_CAPACITY is a const > 0, so this is always Some | ||
| let pool_cache_size = | ||
| NonZeroUsize::new(DEFAULT_POOL_CACHE_CAPACITY).unwrap_or(NonZeroUsize::MIN); |
There was a problem hiding this comment.
The comment correctly states DEFAULT_POOL_CACHE_CAPACITY is a non-zero const, but unwrap_or(NonZeroUsize::MIN) silently degrades the pool cache to capacity 1 if that assumption is ever broken (e.g. someone accidentally sets the constant to 0). A pool cache of size 1 would thrash on every batch payment, causing one on-chain RPC call per chunk instead of one per batch — a ~16x regression with no error surfaced.
Using expect gives an immediate, actionable panic at startup instead:
| // SAFETY: DEFAULT_POOL_CACHE_CAPACITY is a const > 0, so this is always Some | |
| let pool_cache_size = | |
| NonZeroUsize::new(DEFAULT_POOL_CACHE_CAPACITY).unwrap_or(NonZeroUsize::MIN); | |
| let pool_cache_size = | |
| NonZeroUsize::new(DEFAULT_POOL_CACHE_CAPACITY).expect("DEFAULT_POOL_CACHE_CAPACITY must be > 0"); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/payment/verifier.rs
Line: 130-132
Comment:
**Silent capacity degradation instead of loud failure**
The comment correctly states `DEFAULT_POOL_CACHE_CAPACITY` is a non-zero const, but `unwrap_or(NonZeroUsize::MIN)` silently degrades the pool cache to capacity **1** if that assumption is ever broken (e.g. someone accidentally sets the constant to `0`). A pool cache of size 1 would thrash on every batch payment, causing one on-chain RPC call per chunk instead of one per batch — a ~16x regression with no error surfaced.
Using `expect` gives an immediate, actionable panic at startup instead:
```suggestion
let pool_cache_size =
NonZeroUsize::new(DEFAULT_POOL_CACHE_CAPACITY).expect("DEFAULT_POOL_CACHE_CAPACITY must be > 0");
```
How can I resolve this? If you propose a fix, please make it concise.
src/payment/verifier.rs
Outdated
| paid_node_addresses: info | ||
| .paidNodeAddresses | ||
| .iter() | ||
| .map(|pna| (pna.rewardsAddress, pna.poolIndex as usize)) | ||
| .collect(), |
There was a problem hiding this comment.
poolIndex as usize silently truncates on 32-bit targets
pna.poolIndex as usize is a widening cast on 64-bit hosts, so it is safe in practice. However, if poolIndex is a u64 or wider type and the code is ever compiled for a 32-bit target, the as cast silently truncates. A truncated index of, say, 2^32 + 3 becomes 3, which is a valid in-bounds pool index — this could allow the address-match check below to validate a different candidate node than the one the contract intended.
A defensive try_from with an explicit bounds error would prevent this:
| paid_node_addresses: info | |
| .paidNodeAddresses | |
| .iter() | |
| .map(|pna| (pna.rewardsAddress, pna.poolIndex as usize)) | |
| .collect(), | |
| .paid_node_addresses: info | |
| .paidNodeAddresses | |
| .iter() | |
| .map(|pna| { | |
| let idx = usize::try_from(pna.poolIndex) | |
| .map_err(|_| format!("poolIndex {} overflows usize", pna.poolIndex))?; | |
| Ok((pna.rewardsAddress, idx)) | |
| }) | |
| .collect::<Result<Vec<_>, String>>() | |
| .map_err(|e| Error::Payment(e))?, |
(This is a multi-hunk change; the paid_node_addresses field type and the for (addr, idx) loop below would need to be adjusted accordingly.)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/payment/verifier.rs
Line: 514-518
Comment:
**`poolIndex as usize` silently truncates on 32-bit targets**
`pna.poolIndex as usize` is a widening cast on 64-bit hosts, so it is safe in practice. However, if `poolIndex` is a `u64` or wider type and the code is ever compiled for a 32-bit target, the `as` cast silently truncates. A truncated index of, say, `2^32 + 3` becomes `3`, which is a valid in-bounds pool index — this could allow the address-match check below to validate a *different* candidate node than the one the contract intended.
A defensive `try_from` with an explicit bounds error would prevent this:
```suggestion
.paid_node_addresses: info
.paidNodeAddresses
.iter()
.map(|pna| {
let idx = usize::try_from(pna.poolIndex)
.map_err(|_| format!("poolIndex {} overflows usize", pna.poolIndex))?;
Ok((pna.rewardsAddress, idx))
})
.collect::<Result<Vec<_>, String>>()
.map_err(|e| Error::Payment(e))?,
```
(This is a multi-hunk change; the `paid_node_addresses` field type and the `for (addr, idx)` loop below would need to be adjusted accordingly.)
How can I resolve this? If you propose a fix, please make it concise.
src/payment/proof.rs
Outdated
| let kp = Keypair::generate_ed25519(); | ||
| MerklePaymentCandidateNode::new( | ||
| &kp, |
There was a problem hiding this comment.
This helper creates candidate nodes with libp2p ed25519 keys. The resulting pub_key field contains raw ed25519 bytes, which have a different byte length from an ML-DSA-65 key.
When the production verifier calls verify_merkle_candidate_signature on each of the 16 candidates, it tries to parse pub_key as an ML-DSA-65 public key — which fails for ed25519 data and causes the function to return false. Any test that routes proofs from this helper through verify_merkle_payment will always fail the ML-DSA signature step, regardless of the proof's structural correctness.
The quote.rs tests show how to produce correctly ML-DSA-65-signed candidates via QuoteGenerator::set_signer (see test_create_merkle_candidate_quote_with_ml_dsa). This helper should use the same approach, or carry an explicit doc comment noting that its candidates are ed25519-signed and must not be used with the production verifier path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/payment/proof.rs
Line: 312-314
Comment:
**ed25519 candidates will fail ML-DSA-65 signature verification**
This helper creates candidate nodes with libp2p ed25519 keys. The resulting `pub_key` field contains raw ed25519 bytes, which have a different byte length from an ML-DSA-65 key.
When the production verifier calls `verify_merkle_candidate_signature` on each of the 16 candidates, it tries to parse `pub_key` as an ML-DSA-65 public key — which fails for ed25519 data and causes the function to return `false`. Any test that routes proofs from this helper through `verify_merkle_payment` will always fail the ML-DSA signature step, regardless of the proof's structural correctness.
The `quote.rs` tests show how to produce correctly ML-DSA-65-signed candidates via `QuoteGenerator::set_signer` (see `test_create_merkle_candidate_quote_with_ml_dsa`). This helper should use the same approach, or carry an explicit doc comment noting that its candidates are ed25519-signed and must not be used with the production verifier path.
How can I resolve this? If you propose a fix, please make it concise.
|
Addressed all review feedback:
@greptile please review |
There was a problem hiding this comment.
Pull request overview
Integrates merkle batch payments into saorsa-node so clients can pay for large chunk batches via a single on-chain MerklePaymentVault transaction, using ML-DSA-65 signatures end-to-end (no ed25519).
Changes:
- Adds new ANT protocol message types for merkle candidate quote request/response and introduces tagged payment-proof type bytes (single-node vs merkle).
- Implements ML-DSA-65 signing + verification for merkle candidate nodes, and adds merkle proof verification flow with an LRU pool-level on-chain info cache.
- Updates devnet configuration/wiring to carry a merkle payments contract address.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/storage/handler.rs | Routes MerkleCandidateQuoteRequest and returns serialized merkle candidate nodes; adds tests for request/response flow. |
| src/payment/verifier.rs | Dispatches verification based on proof tag; adds merkle payment verification and an LRU cache for on-chain pool info; expands proof-size cap and tests. |
| src/payment/quote.rs | Adds ML-DSA-65 merkle candidate quote creation and signature verification helpers; adds unit tests. |
| src/payment/proof.rs | Introduces tagged proof serialization/deserialization and proof-type detection; adds extensive unit tests (single-node + merkle). |
| src/payment/mod.rs | Re-exports new proof and quote APIs (tagged serialization, detection, merkle candidate verification). |
| src/node.rs | Wires the same ML-DSA-65 signer for both regular quotes and merkle candidate quotes; minor initialization refactor. |
| src/devnet.rs | Extends DevnetEvmInfo with optional merkle_payments_address field. |
| src/bin/saorsa-devnet/main.rs | Plumbs merkle_payments_address through devnet network parsing and config output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return MerkleCandidateQuoteResponse::Error(ProtocolError::QuoteFailed(format!( | ||
| "data_size {} overflows usize", | ||
| request.data_size | ||
| ))); |
| let payload = bytes | ||
| .get(1..) | ||
| .ok_or_else(|| "Single-node proof tag present but no payload".to_string())?; | ||
| let proof = rmp_serde::from_slice::<PaymentProof>(payload) | ||
| .map_err(|e| format!("Failed to deserialize single-node proof: {e}"))?; | ||
| Ok((proof.proof_of_payment, proof.tx_hashes)) |
| let payload = bytes | ||
| .get(1..) | ||
| .ok_or_else(|| "Merkle proof tag present but no payload".to_string())?; |
Adds merkle batch payment support for cheaper gas on large uploads. Instead of one on-chain tx per chunk, clients pay for an entire batch in a single MerklePaymentVault transaction. Protocol: MerkleCandidateQuoteRequest/Response, tagged proof serialization. Signing: ML-DSA-65 (not ed25519) via create_merkle_candidate_quote(). Verification: proof type dispatch, ML-DSA sig check on 16 candidates, on-chain query, pool cache, merkle tree proof validation. Infrastructure: merkle_payments_address in DevnetEvmInfo, 256KB proof limit.
- Use QuoteFailed instead of fabricated ChunkTooLarge for usize overflow - Add serde skip_serializing_if on merkle_payments_address Option - Format unknown proof tag as hex byte instead of Some(...)
- Replace ed25519 keypairs in proof.rs test helper with ML-DSA-65 (P1) - Add const assert for pool cache capacity instead of silent degradation - Use usize::from for poolIndex (u8 always fits, no try_from needed) - Rename test_pool_cache_same_hash_queries_once to test_pool_cache_insert_and_lookup - Move test imports to module level (clippy items_after_statements)
Codex review found that the node verifier checked ML-DSA signatures on each candidate but did not verify all candidates share the same merkle_payment_timestamp matching the on-chain value. A malicious client could mix candidates with different timestamps into one pool. Now each candidate's timestamp is checked against the on-chain payment_info.merkle_payment_timestamp.
jacderida
force-pushed
the
merkle_payments
branch
from
440ae11 to
7d1e267
Compare
There was a problem hiding this comment.
Pull request overview
Integrates Merkle batch payment proofs and ML-DSA-65 signing/verification into saorsa-node, allowing clients to pay for many chunks in a single on-chain transaction via tagged proof serialization and MerklePaymentVault verification.
Changes:
- Added Merkle candidate quote request/response protocol messages and proof type tag constants.
- Implemented ML-DSA-65 merkle candidate quote creation + verification and added tagged proof (de)serialization for both single-node and merkle proofs.
- Extended payment verification to dispatch by proof tag, verify merkle batch payments (including on-chain lookups) and added an LRU cache for pool-level on-chain payment info.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/ant_protocol/chunk.rs |
Adds MerkleCandidateQuote message types and defines proof tag constants for proof-type dispatch. |
src/ant_protocol/mod.rs |
Re-exports new protocol types/constants for use across the crate. |
src/storage/handler.rs |
Routes MerkleCandidateQuoteRequest to quote generator and adds validation + serialization for response. |
src/payment/quote.rs |
Adds ML-DSA-65 merkle candidate quote creation and signature verification helper. |
src/payment/proof.rs |
Introduces tagged proof types, proof type detection, and tagged serialize/deserialize for single-node and merkle proofs. |
src/payment/verifier.rs |
Adds proof tag dispatch, merkle payment verification flow, and pool-level LRU cache for on-chain payment info. |
src/payment/mod.rs |
Re-exports new proof and quote APIs (tagged proof functions + merkle candidate verifier). |
src/node.rs |
Wires the same ML-DSA-65 signer for both regular quotes and merkle candidate quotes; minor initialization refactor. |
src/devnet.rs |
Extends devnet manifest EVM info with optional merkle payments contract address. |
src/bin/saorsa-devnet/main.rs |
Plumbs merkle payments contract address into generated devnet manifest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/payment/verifier.rs
Outdated
| use evmlib::contract::payment_vault::error::Error as PaymentVaultError; | ||
| use evmlib::contract::payment_vault::verify_data_payment; |
There was a problem hiding this comment.
PaymentVaultError and verify_data_payment are imported but not used anywhere in this file. Please remove these unused imports to avoid warnings and keep the dependency surface minimal.
| use evmlib::contract::payment_vault::error::Error as PaymentVaultError; | |
| use evmlib::contract::payment_vault::verify_data_payment; |
| let Ok(data_size_usize) = usize::try_from(request.data_size) else { | ||
| return MerkleCandidateQuoteResponse::Error(ProtocolError::QuoteFailed(format!( | ||
| "data_size {} overflows usize", | ||
| request.data_size | ||
| ))); |
There was a problem hiding this comment.
When request.data_size overflows usize, this returns ProtocolError::QuoteFailed, but the non-merkle quote path reports ProtocolError::ChunkTooLarge. Consider returning ChunkTooLarge here as well (or otherwise keeping the error type consistent) so clients can handle oversize requests uniformly.
| let Ok(data_size_usize) = usize::try_from(request.data_size) else { | |
| return MerkleCandidateQuoteResponse::Error(ProtocolError::QuoteFailed(format!( | |
| "data_size {} overflows usize", | |
| request.data_size | |
| ))); | |
| let data_size_usize = match usize::try_from(request.data_size) { | |
| Ok(size) => size, | |
| Err(_) => { | |
| return MerkleCandidateQuoteResponse::Error(ProtocolError::ChunkTooLarge { | |
| size: usize::MAX, | |
| max_size: MAX_CHUNK_SIZE, | |
| }); | |
| } |
The rebase onto main introduced unused imports for PaymentVaultError and verify_data_payment, which were removed on main. This fixes the CI failures caused by -D warnings treating unused imports as errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Integrates merkle batch payments into saorsa-node, enabling clients to pay for large chunk batches in a single on-chain transaction via the MerklePaymentVault contract. Uses ML-DSA-65 post-quantum cryptography throughout (not ed25519).
Changes
Protocol (
src/ant_protocol/chunk.rs)MerkleCandidateQuoteRequest/MerkleCandidateQuoteResponsemessage typesPROOF_TAG_SINGLE_NODE(0x01) /PROOF_TAG_MERKLE(0x02) proof type tagsPayment signing (
src/payment/quote.rs)create_merkle_candidate_quote()— constructsMerklePaymentCandidateNodewith ML-DSA-65 signature (bypasses upstream ant-evm ed25519)verify_merkle_candidate_signature()— ML-DSA-65 verification counterpartPayment verification (
src/payment/verifier.rs)verify_merkle_payment():MerklePaymentVault.getPaymentInfo()MAX_PAYMENT_PROOF_SIZEraised to 256KB (ML-DSA proofs with 16 candidates are ~130KB)Proof serialization (
src/payment/proof.rs)ProofTypeenum,detect_proof_type(), tagged serialize/deserializeHandler (
src/storage/handler.rs)MerkleCandidateQuoteRequestto quote generatorMAX_CHUNK_SIZEcheck on merkle candidate quotesInfrastructure
merkle_payments_addressfield inDevnetEvmInfosaorsa-devnetbinaryTest plan
cfd(cargo fmt + clippy) passes clean