feat: close group views in quotes and payment verification by mickvandijke · Pull Request #48 · WithAutonomi/ant-node

mickvandijke · 2026-03-30T16:16:25Z

Summary

Nodes return their local close group view in ChunkQuoteResponse::Success, enabling clients to verify quorum before paying
During payment verification (handle_put), nodes validate that all peers in the proof are current close group members — rejecting proofs containing any unknown peer
Invalid ML-DSA public key derivation in proofs is now a hard error, not a silent log
AntProtocol uses OnceLock<Arc<P2PNode>> with a set_p2p_node() setter so close-group validation works in devnet and testnet (where p2p_node is injected after construction)
DHT lookups deduplicated into AntProtocol::local_close_group()

Motivation

Data is only permanently stored and replicated if accepted by a majority of close group nodes. Without this change, nodes blindly accept any payment proof that includes them, allowing malicious clients to include arbitrary attacker nodes in proofs. The client also had no way to verify it was paying the right set of nodes before spending tokens.

Changes

`src/payment/verifier.rs`

Replace the permissive majority check (3/5 recognized) with strict validation that rejects the proof if any peer is not in the verifying node's current close group
Promote invalid ML-DSA pub_key derivation from a silent debug log to a hard error

`src/storage/handler.rs`

Extract duplicated DHT lookup into AntProtocol::local_close_group()
Fix step-numbering comment in handle_put (5→6 for the store step)
Clarify docs around find_closest_nodes_local semantics (excludes self, CLOSE_GROUP_SIZE count rationale)

`src/ant_protocol/chunk.rs`

Switch AntProtocol.p2p_node to OnceLock<Arc<P2PNode>> with set_p2p_node() setter

`src/devnet.rs` & `tests/e2e/testnet.rs`

Wire set_p2p_node() after node construction so close-group validation is active in devnet and e2e tests

Test plan

All existing unit tests pass (cargo test --lib)
Clippy clean with --all-targets --all-features -- -D warnings
Companion PR: WithAutonomi/ant-client#TBD

🤖 Generated with Claude Code

…yment Nodes now return their local close group view (up to CLOSE_GROUP_SIZE peer IDs) in ChunkQuoteResponse::Success, enabling clients to verify close-group quorum before paying. On the payment verification side, nodes now check that the other peers in a payment proof are known close group members for the content address. The new validate_close_group_membership step requires at least CLOSE_GROUP_MAJORITY proof peers to appear in the node's local close group view (self + DHT peers). This prevents malicious clients from including arbitrary attacker nodes in payment proofs. Key changes: - ChunkQuoteResponse::Success gains a close_group field (Vec<[u8; 32]>) - AntProtocol holds Option<Arc<P2PNode>> for local DHT lookups - handle_quote and handle_put query the local routing table - PaymentVerifier.verify_payment accepts local_close_group for membership checks - PaymentVerifierConfig gains local_peer_id for building the full close group set Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

… DHT lookups Devnet and e2e test nodes were constructed with p2p_node=None and never received it later, silently skipping close-group validation. Switch AntProtocol.p2p_node to OnceLock with a set_p2p_node() setter so the P2P node can be injected after construction. Wire it up in both devnet::start_node and testnet::start_node. Also extracts the duplicated DHT lookup into AntProtocol::local_close_group(), fixes the step-numbering comment in handle_put (5→6 for the store step), and clarifies docs around find_closest_nodes_local semantics (excludes self, CLOSE_GROUP_SIZE count rationale). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Replace the permissive majority check (3/5 recognized) with strict validation that rejects the proof if ANY peer is not in the verifying node's current close group. This prevents malicious clients from including attacker-controlled nodes in payment proofs. Also promotes invalid ML-DSA pub_key derivation from a silent debug log to a hard error — a bad pub_key in a proof is suspicious, not benign. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR strengthens payment-proof validation by tying proofs to the locally known close group for a content address, and exposes each node’s local close-group view in quote responses so clients can verify quorum membership before paying.

Changes:

Add close_group: Vec<[u8; 32]> to ChunkQuoteResponse::Success and populate it via local DHT lookups.
Extend payment verification to validate that peers referenced by a payment proof are members of the locally known close group (plus self).
Plumb local peer identity / P2P node access into the storage protocol handler (constructor arg + post-construction injection for devnet/e2e).

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/e2e/testnet.rs	Adds `local_peer_id` to payment config and injects P2P node into the protocol handler in e2e setup.
tests/e2e/data_types/chunk.rs	Updates test payment config (`local_peer_id`) and `AntProtocol::new(..., None)` signature change.
src/storage/mod.rs	Updates public docs/example for new `AntProtocol::new(..., None)` signature.
src/storage/handler.rs	Adds `OnceLock<Arc<P2PNode>>`, local close-group lookup, async quote handling, and passes close-group into payment verification + quote responses.
src/payment/verifier.rs	Adds `local_peer_id` to config, threads close-group into verification, and implements close-group membership validation for proof peers.
src/node.rs	Wraps P2P node in `Arc` earlier, passes it into `AntProtocol`, and populates `local_peer_id`.
src/devnet.rs	Adds `local_peer_id` and injects P2P node into protocol handler for devnet.
src/ant_protocol/chunk.rs	Extends `ChunkQuoteResponse::Success` with `close_group`.
Cargo.lock	Updates lockfile metadata for `saorsa-core` crate source/checksum.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T18:01:04Z

src/ant_protocol/chunk.rs

    Success {
        /// Serialized `PaymentQuote`.
        quote: Vec<u8>,
        /// `true` when the chunk already exists on this node (skip payment).
        already_stored: bool,
+        /// Up to `CLOSE_GROUP_SIZE` peer IDs (raw 32-byte BLAKE3 hashes) this
+        /// node considers closest to the content address, **excluding itself**
+        /// (the local node is filtered out by the DHT query). Clients combine
+        /// these views from multiple nodes to verify close-group quorum before
+        /// paying.
+        close_group: Vec<[u8; 32]>,
    },


Adding close_group to ChunkQuoteResponse::Success changes the postcard-encoded wire format for this response variant. With CHUNK_PROTOCOL_ID still at /v1 (and PROTOCOL_VERSION unchanged), older clients/nodes will fail to deserialize these responses. Consider bumping the protocol ID/version (e.g., /v2) or introducing a backwards-compatible encoding strategy (e.g., a new response variant) so mixed-version networks can interoperate.

Copilot · 2026-03-30T18:01:04Z

src/payment/verifier.rs

+    /// Verify that **every** peer in the payment proof is a known close group member.
+    ///
+    /// Builds the known set from the current DHT close group plus this node
+    /// itself, then checks that each proof peer (derived via `BLAKE3(pub_key)`)
+    /// appears in that set. Rejects the proof if ANY peer is unrecognized.
+    ///
+    /// Skipped when `local_close_group` is empty (unit tests without DHT).
+    fn validate_close_group_membership(
+        &self,
+        payment: &ProofOfPayment,
+        local_close_group: &[[u8; 32]],
+    ) -> Result<()> {
+        if local_close_group.is_empty() {
+            return Ok(());
+        }
+
+        // Build the known peer set: current DHT close group + this node.
+        let mut known_peers: HashSet<[u8; 32]> = local_close_group.iter().copied().collect();
+        known_peers.insert(self.config.local_peer_id);
+
+        // Every proof peer must be in the known set.
+        for (_encoded_peer_id, quote) in &payment.peer_quotes {
+            let peer_id = peer_id_from_public_key_bytes(&quote.pub_key).map_err(|e| {
+                Error::Payment(format!("Invalid ML-DSA pub_key in proof quote: {e}"))
+            })?;
+
+            if !known_peers.contains(peer_id.as_bytes()) {
+                return Err(Error::Payment(format!(
+                    "Proof peer {} is not in the current close group",
+                    peer_id.to_hex()
+                )));
+            }


This membership check currently rejects the proof if any peer quote is not in the locally known close-group set, but the PR description says nodes should reject proofs with too many unknown peers. If some divergence is expected between nodes’ close-group views, consider allowing a bounded number of unknown peers (e.g., enforce a threshold like <= CLOSE_GROUP_SIZE - CLOSE_GROUP_MAJORITY) rather than failing on the first unknown.

Copilot · 2026-03-30T18:01:04Z

src/payment/verifier.rs

        Self::validate_quote_structure(payment)?;
        Self::validate_quote_content(payment, xorname)?;
        Self::validate_quote_timestamps(payment)?;
        Self::validate_peer_bindings(payment)?;
        self.validate_local_recipient(payment)?;
+        self.validate_close_group_membership(payment, local_close_group)?;



New close-group membership enforcement is introduced here, but there are no unit tests asserting (1) a proof is accepted when all peers are in local_close_group + local_peer_id, and (2) a proof is rejected when at least one peer is outside that set. Adding targeted tests would help prevent regressions in this security-critical validation.

Copilot · 2026-03-30T18:01:05Z

src/storage/handler.rs

+        let lock = OnceLock::new();
+        if let Some(node) = p2p_node {
+            // Fresh OnceLock — set cannot fail.
+            let _ = lock.set(node);


OnceLock::set cannot fail here (fresh lock), but the let _ = ... silently discards the result. Using expect (or debug_assert!(lock.set(node).is_ok())) would make the invariant explicit and avoid hiding future changes where this could fail.

Suggested change

let _ = lock.set(node);

lock.set(node)

.expect("p2p_node OnceLock was just created; set() must succeed");

Copilot · 2026-03-30T18:01:05Z

src/devnet.rs


        if let (Some(ref p2p), Some(ref protocol)) = (&node.p2p_node, &node.ant_protocol) {
+            // Inject P2P node into protocol handler for close-group lookups.
+            let _ = protocol.set_p2p_node(Arc::clone(p2p));


set_p2p_node can return Err if the node was already injected; the result is currently ignored. Consider handling this explicitly (e.g., log/warn on Err or use expect if it should be impossible) to avoid silently running without the intended close-group lookup wiring.

Suggested change

let _ = protocol.set_p2p_node(Arc::clone(p2p));

if let Err(e) = protocol.set_p2p_node(Arc::clone(p2p)) {

warn!(

"Failed to inject P2P node into protocol for devnet node {index}: {e}"

);

}

Copilot · 2026-03-30T18:01:05Z

tests/e2e/testnet.rs

        // Start protocol handler that routes incoming P2P messages to AntProtocol
        if let (Some(ref p2p), Some(ref protocol)) = (&node.p2p_node, &node.ant_protocol) {
+            // Inject P2P node into protocol handler for close-group lookups.
+            let _ = protocol.set_p2p_node(Arc::clone(p2p));


set_p2p_node can return Err if the node was already injected; the result is currently ignored. Consider handling this explicitly (e.g., log/warn on Err or use expect if it should be impossible) so testnet setup fails loudly if the protocol handler isn't wired for close-group lookups.

Suggested change

let _ = protocol.set_p2p_node(Arc::clone(p2p));

protocol

.set_p2p_node(Arc::clone(p2p))

.map_err(|e| {

TestnetError::Startup(format!(

"Failed to inject P2P node into protocol handler for node {}: {e}",

node.index

))

})?;

…node errors Address Copilot review feedback: add 4 unit tests covering the accept, reject, skip, and local-peer-known paths of validate_close_group_membership, and handle set_p2p_node errors explicitly in devnet (warn) and testnet (propagate as TestnetError::Startup). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

mickvandijke mentioned this pull request Mar 30, 2026

feat: client-side close group quorum validation WithAutonomi/ant-client#16

Draft

3 tasks

mickvandijke and others added 2 commits March 30, 2026 19:06

mickvandijke marked this pull request as ready for review March 30, 2026 17:56

Copilot AI review requested due to automatic review settings March 30, 2026 17:56

Copilot started reviewing on behalf of mickvandijke March 30, 2026 17:56 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

mickvandijke marked this pull request as draft March 31, 2026 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: close group views in quotes and payment verification#48

feat: close group views in quotes and payment verification#48
mickvandijke wants to merge 4 commits intomainfrom
feat/close-group-quorum-validation

mickvandijke commented Mar 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	let _ = lock.set(node);
	lock.set(node)
	.expect("p2p_node OnceLock was just created; set() must succeed");

-            let _ = protocol.set_p2p_node(Arc::clone(p2p));
+            if let Err(e) = protocol.set_p2p_node(Arc::clone(p2p)) {
+                warn!(
+                    "Failed to inject P2P node into protocol for devnet node {index}: {e}"
+                );
+            }

Conversation

mickvandijke commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

src/payment/verifier.rs

src/storage/handler.rs

src/ant_protocol/chunk.rs

src/devnet.rs & tests/e2e/testnet.rs

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mickvandijke commented Mar 30, 2026 •

edited

Loading

`src/payment/verifier.rs`

`src/storage/handler.rs`

`src/ant_protocol/chunk.rs`

`src/devnet.rs` & `tests/e2e/testnet.rs`