From ae7b7ecdc2443406ff65a9364547e59c9dd59d31 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Wed, 7 Oct 2026 14:28:11 +0200 Subject: [PATCH 01/11] docs(adr): ADR-0020 chunk GET returns the chunk or closer peers Kademlia's FIND_VALUE for chunks. A new get-or-closer request is answered with the chunk by a holder, and otherwise with the closer peers its FIND_NODE answer would hold, encoded and checked by saorsa-core. Native clients run their own iterative lookup with it. Peers that do not announce `get-or-closer/1` in their user agent get FIND_NODE plus a GET instead, so a new client reads at full speed from today's nodes. When the lookup finds nothing, the client falls back to today's read path, so the absence rule is unchanged. Measured on production: native p50 5.95 s to about 1 s, the whole file 3.7 times faster, at 2.5-3.3 chunk copies per read. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- .../ADR-0020-kademlia-chunk-get-or-closer.md | 256 ++++++++++++++++++ docs/adr/README.md | 1 + 2 files changed, 257 insertions(+) create mode 100644 docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md diff --git a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md new file mode 100644 index 00000000..d4439c2a --- /dev/null +++ b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md @@ -0,0 +1,256 @@ +# ADR-0020: Kademlia-style chunk GET: return the chunk or closer peers + +- **Status:** Proposed +- **Date:** 2026-10-07 +- **Decision owners:** +- **Reviewers:** +- **Supersedes:** none +- **Superseded by:** none +- **Related:** Linear V2-1358; ant-client investigation + `docs/investigations/2026-10-06-kademlia-chunk-get/`; + [ADR-0015](ADR-0015-direct-browser-clients-over-webrtc-direct.md) (browser + capabilities); saorsa-core, ant-protocol and ant-client branches + `mickvandijke/v2-1358-kad-chunk-get` + +## Context + +A native chunk read runs two separate steps today: + +1. saorsa-core's iterative closest-peers lookup runs until it converges on the + target's close group. +2. GETs are then sent one at a time, in XOR order. + +While the lookup runs, early GETs go to peers that were already connected. +A `ChunkGetResponse::NotFound` carries no peers, so a GET never helps +discovery. + +Measured on the production network, reading 150 chunks of one 612 MB file: + +- **Native reads spend most of their time finding a holder.** Discovery takes + 5.1 s at p50 and transfer 0.8 s, so 82% of read time is discovery. +- **The lookup runs past the point where it could stop.** It needs about 6 s to + converge, but the round that first reaches the close group is usually round + 2, about 0.3 s in. +- **Sending a GET beside every FIND_NODE removes almost all of that wait.** The + read stops at the first verified chunk. In the measurements: + + | | p50 | p90 | + | --- | ---: | ---: | + | Today | 5.95 s | 13.0 s | + | GET beside every FIND_NODE | 0.97 s | 1.9 s | + + The whole file then reads in 49–55 s instead of 199–205 s. +- **That speed costs bytes.** The round that reaches the close group queries + up to alpha holders at once, so the read downloads 2.5–3.3 copies of a chunk + on average. + +Kademlia's FIND_VALUE folds the value into the lookup: a node that holds the +key returns it; one that doesn't returns closer peers. This network cannot +just switch to that, for three reasons: + +- **Old peers.** Nodes on 0.20 and 0.21 drop an unknown `ChunkMessageBody` + variant. Natively they send no reply, so the client waits out its timeout. + Over WebRTC Direct, an undecodable request closes the browser's data channel. +- **No native capability exchange.** A connected node's user agent + (`node/0.21.0 migration/files`) is the only signal of what it supports. +- **Peer validation lives in saorsa-core.** FIND_NODE answers are checked + there: owner-signed address records, publish sequences, LAN filtering and + dial plans. A chunk-protocol message that carries peers must not bypass + those checks. + +## Decision Drivers + +- Cut native read latency to about one round trip per lookup step, and stop + the lookup as soon as a verified chunk arrives. +- Stay compatible in both directions: + - Old clients work unchanged with new nodes. + - New clients work with old nodes at full speed, not after timeouts. + - Rollout can happen in any order. +- Keep peer encoding and validation in saorsa-core, so a peer learned from a + chunk response is trusted exactly as far as one learned from FIND_NODE. +- Leave the chunk-absence rule unchanged: a completed lookup and a majority of + `NotFound`s. +- Give the client one place to evolve read policy without further wire + changes. + +## Considered Options + +1. **Status quo.** Native discovery stays at about 5 s per chunk. +2. **Lookup progress feeds the early GETs (client only).** Measured native p50 + 1.8 s, p90 4.9 s, one copy. No wire change, but more than 0.8 s slower at + p50 than options 3 and 5. +3. **GET beside each FIND_NODE (client only).** The same latency as option 5, + working with today's nodes. It sends two requests per peer, and the GET + carries nothing useful when the peer doesn't hold the chunk. It remains the + per-peer fallback under option 5. +4. **"Do you hold it?" request, then one fetch.** Measured native p50 1.16 s + with 1.07 copies. Keeps bytes down at the cost of an extra round trip + before the transfer. Deferred: it can be added later as another appended + variant. +5. **Chunk-or-closer-peers request in ant-protocol, with the closer peers in + saorsa-core's FIND_NODE answer format (chosen).** One request per lookup + step to upgraded nodes, with per-peer fallback to option 3 for older ones. +6. **FIND_VALUE inside saorsa-core's DHT.** Rejected: + - DHT messages are capped at 64 KiB, too small for a 4 MiB chunk. + - The DHT layer would have to know about chunk storage. + +## Decision + +We will add a chunk-or-closer-peers request to the chunk protocol. Native +clients will run their own iterative lookup with it, and fall back per peer to +FIND_NODE plus GET. + +### Wire (ant-protocol, additive) + +`ChunkMessageBody` gains two variants, appended after every existing one, so +existing discriminants keep their wire values: + +```rust +GetOrCloserRequest(ChunkGetOrCloserRequest { address: XorName }) +GetOrCloserResponse(ChunkGetOrCloserResponse) + +enum ChunkGetOrCloserResponse { + /// The responder holds the chunk. + Found { address: XorName, content: Bytes }, + /// The responder does not hold the chunk; `peers` are its closest known + /// peers to `address`, encoded by saorsa-core. + Closer { address: XorName, peers: Bytes }, + Error(ProtocolError), +} +``` + +`peers` is opaque to ant-protocol. saorsa-core produces and reads it with two +public calls: + +- **`DhtNetworkManager::encode_closer_peers(key, requester)`** builds exactly + what a FIND_NODE answer to that requester would hold: + - the K closest routing-table peers, without the requester + - QUIC addresses with publish sequences + - owner-signed address records, within the FIND_NODE size limit +- **`DhtNetworkManager::decode_closer_peers(responder, key, payload, + transport_source)`** validates the payload as a FIND_NODE answer from that + responder. It goes through the same signed-record verification, owner-view + protection, LAN filtering and dial-plan checks. + +The peer format is therefore versioned in one place, saorsa-core, as it is for +FIND_NODE today. + +### Node (ant-node) + +- **If the node holds the chunk,** it answers `Found`, read from storage like a + GET. +- **Otherwise it answers `Closer`,** using `encode_closer_peers` with the + authenticated sender as the requester. +- **It advertises support** with the user-agent token `get-or-closer/1` + (`ant_protocol::GET_OR_CLOSER_AGENT_TOKEN`), after the existing migration + token. The `node/` prefix that saorsa-core gates DHT membership on is kept. +- **It does not advertise support to browsers yet.** Browsers cannot decode the + native peer format. The WebRTC Direct listener keeps refusing the request, + and HELLO does not list it. + +### Native client (ant-client) + +A chunk read runs a client-driven iterative lookup on saorsa-core's public +lookup engine (`IterativeLookup`). The parameters match today's native lookup: +alpha 3, K, a 5 s grace period per round, and a 120 s deadline. + +For each peer the lookup queries, the client: + +1. **Connects to it.** It uses `DhtNetworkManager::connect_lookup_peer`, which + the lookup's own request would need anyway. It then reads the peer's user + agent. +2. **If the peer advertises `get-or-closer/1`,** sends `GetOrCloserRequest`: + - `Found`: verify the content hash, then stop the read. + - `Closer`: `decode_closer_peers` supplies the lookup candidates. +3. **Otherwise,** sends FIND_NODE (`DhtNetworkManager::find_node_on_peer`) and + a GET at the same time. FIND_NODE supplies the candidates, and a `Success` + from the GET stops the read. +4. **If the connection fails,** marks the peer failed in the lookup, as a + failed FIND_NODE is marked today. + +The read driver owns every request, not the lookup round that triggered it. +The round's grace window therefore never cancels a chunk transfer that is +still arriving. Conflicting reports about a peer are resolved with +`client_routing::compute_winner`, as in both existing lookups. + +- **When the lookup ends without a chunk,** or fails, the client runs today's + read path (`retrieve_progressive`) unchanged. Its absence rule and retry + round are unchanged. +- **Browser clients keep today's read path.** Supporting this request there + needs a portable decoder for the peer format and a HELLO capability. + +## Consequences + +### Positive + +- **Native read latency drops to about one round trip per lookup step.** + Measured with the per-peer fallback that this decision keeps for old nodes: + p50 5.95 s to about 1 s, p90 13 s to about 2 s, and the whole file 3.7 times + faster. +- **Upgraded nodes answer each lookup step with one request** instead of + FIND_NODE plus GET. The chunk travels in that same reply. +- **Compatible in both directions.** Old clients never send the new variant. + New clients send it only to nodes that advertise it, and treat every other + peer as before. +- **Peer trust is unchanged.** Peers learned from `Closer` pass the same + saorsa-core validation as FIND_NODE answers. +- **The absence rule, the retry policy and the browser path are untouched.** + +### Negative / Trade-offs + +- **Duplicate downloads.** Up to alpha holders can return the chunk in the + same round: measured at 2.5–3.3 copies per chunk natively. That costs bytes + for the client and egress for the nodes. A "have it" or inline-flag variant + can bound this later without changing the request defined here. +- **The native client runs its own lookup**, beside the one in saorsa-core. + Lookup parameters and peer policy must be kept in step. +- **Capability is a user-agent token**, and is known only once a peer is + connected. Every queried peer has to be connected before the client can + choose what to send it. The lookup's own request would have needed that + connection anyway. +- **A second chunk response shape** to maintain, with its own traffic + accounting. + +### Neutral / Operational + +- **Version bumps.** ant-protocol takes a minor version bump for the appended + variants. saorsa-core gains public API with no wire change. +- **Node user agents gain a token.** Any tooling that parses them must split + on whitespace. +- **Rollout order is free.** Nodes are a no-op until clients use the request, + and clients fall back per peer. + +## Validation + +- **ant-protocol** + - Round-trip tests for the new messages. + - Every existing discriminant keeps its wire value. +- **saorsa-core** + - `encode_closer_peers` / `decode_closer_peers` return the same trusted + candidates as a FIND_NODE answer for the same key. + - `find_node_on_peer` and `connect_lookup_peer` behave against a live peer. +- **ant-node** + - The handler answers `Found` for a held chunk and `Closer`, with decodable + peers, otherwise. + - The user agent carries the token and keeps the `node/` prefix. +- **ant-client** + - The read driver is tested on a scripted network. It must: + - stop at the first verified chunk + - fall back per peer for legacy peers + - handle a mixed network + - treat a hash mismatch as an error + - keep a transfer alive past its lookup round's grace window + - fall back to today's path when the lookup finds nothing + - Live: a new client against today's nodes takes the legacy route for every + peer and stays within the measured latency of option 3. A local devnet of + upgraded nodes takes the `GetOrCloserRequest` route. +- **Review triggers** + - Node egress for reads rises enough to matter: add the "have it" variant. + - Browser reads need this request: add the portable decoder and a HELLO + capability. + - saorsa-core changes its native lookup parameters: mirror them in the + client lookup. + +## Notes for AI-assisted work + +AI tools may help draft this ADR, but **must not mark it Accepted without human review**. Accepted ADRs are immutable: create a new superseding ADR rather than editing an Accepted ADR. diff --git a/docs/adr/README.md b/docs/adr/README.md index 43cc2028..c490a544 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -39,5 +39,6 @@ See [`TOOLING.md`](./TOOLING.md) for `adrs`, `adr-kit`, and AI harness setup. - [ADR-0017: Bounded fresh-replication offers and copy-free message sends](./ADR-0017-bounded-fresh-offers-and-copy-free-sends.md) - [ADR-0018: Pointer ownership transfer by final redirection](./ADR-0018-pointer-transfer-by-final-redirection.md) - [ADR-0019: Pointer audits serve the record round 1 bound](./ADR-0019-pointer-audits-serve-the-record-round-one-bound.md) +- [ADR-0020: Kademlia-style chunk GET: return the chunk or closer peers](./ADR-0020-kademlia-chunk-get-or-closer.md) - [ADR-0022: Remove the LMDB Chunk Store](./ADR-0022-remove-the-lmdb-chunk-store.md) - [ADR-0023: Spread hot-record reads across the close group, and refuse full browser sessions explicitly](./ADR-0023-spread-hot-reads-and-refuse-full-browser-sessions.md) From c59bcf393d63fb27f8762c8852116e8e4ad5d2c1 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:19:56 +0100 Subject: [PATCH 02/11] feat(storage): answer get-or-closer requests ADR-0020. A node that holds the requested chunk answers `Found` with it. Otherwise it answers `Closer` with saorsa-core's encoding of the FIND_NODE answer it would give the authenticated requester, so clients check those peers exactly as they check FIND_NODE answers. A node with storage enabled announces `get-or-closer/1` in its user agent; one with storage off never answers chunk requests, so it does not. Browsers are not offered the request yet. Get-or-closer traffic gets its own rx and tx rows in the chunk traffic summary (group 4), and each answer is logged as `get_or_closer_rpc`. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- src/ant_protocol/mod.rs | 7 +- src/devnet.rs | 9 +- src/node.rs | 43 ++++++--- src/storage/handler.rs | 170 +++++++++++++++++++++++++++++++++- src/storage/traffic.rs | 54 +++++++++-- tests/e2e/data_types/chunk.rs | 73 +++++++++++++++ tests/e2e/testnet.rs | 45 ++++++++- 7 files changed, 367 insertions(+), 34 deletions(-) diff --git a/src/ant_protocol/mod.rs b/src/ant_protocol/mod.rs index 74921117..c770301e 100644 --- a/src/ant_protocol/mod.rs +++ b/src/ant_protocol/mod.rs @@ -17,11 +17,12 @@ pub use ::ant_protocol::chunk; pub use ::ant_protocol::chunk::{ - settlement_compatibility, ChunkGetRequest, ChunkGetResponse, ChunkMessage, ChunkMessageBody, + advertises_get_or_closer, settlement_compatibility, ChunkGetOrCloserRequest, + ChunkGetOrCloserResponse, ChunkGetRequest, ChunkGetResponse, ChunkMessage, ChunkMessageBody, ChunkPutRequest, ChunkPutResponse, ChunkQuoteRequest, ChunkQuoteRequestV2, ChunkQuoteResponse, MerkleCandidateQuoteRequest, MerkleCandidateQuoteRequestV2, MerkleCandidateQuoteResponse, ProtocolError, SettlementCompatibility, XorName, CHUNK_PROTOCOL_ID, CLOSE_GROUP_MAJORITY, - CLOSE_GROUP_SIZE, CURRENT_SETTLEMENT_VERSION, DATA_TYPE_CHUNK, MAX_CHUNK_SIZE, - MAX_WIRE_MESSAGE_SIZE, MIN_SUPPORTED_SETTLEMENT_VERSION, PROOF_TAG_MERKLE, + CLOSE_GROUP_SIZE, CURRENT_SETTLEMENT_VERSION, DATA_TYPE_CHUNK, GET_OR_CLOSER_AGENT_TOKEN, + MAX_CHUNK_SIZE, MAX_WIRE_MESSAGE_SIZE, MIN_SUPPORTED_SETTLEMENT_VERSION, PROOF_TAG_MERKLE, PROOF_TAG_SINGLE_NODE, PROTOCOL_VERSION, XORNAME_LEN, }; diff --git a/src/devnet.rs b/src/devnet.rs index 18e459bb..9b16645f 100644 --- a/src/devnet.rs +++ b/src/devnet.rs @@ -106,11 +106,10 @@ async fn handle_chunk_message( let handled = protocol .try_handle_request_with_context( &data, - Some(ChunkRequestContext::new( - source.to_string(), - received_at.into(), - Duration::ZERO, - )), + Some( + ChunkRequestContext::new(source.to_string(), received_at.into(), Duration::ZERO) + .with_requester(source), + ), ) .await; let telemetry = handled.get_telemetry; diff --git a/src/node.rs b/src/node.rs index ac1f3588..0459da13 100644 --- a/src/node.rs +++ b/src/node.rs @@ -1,6 +1,6 @@ //! Node implementation - thin wrapper around saorsa-core's `P2PNode`. -use crate::ant_protocol::CHUNK_PROTOCOL_ID; +use crate::ant_protocol::{CHUNK_PROTOCOL_ID, GET_OR_CLOSER_AGENT_TOKEN}; use crate::config::{ default_nodes_dir, default_root_dir, NetworkMode, NodeConfig, NODE_IDENTITY_FILENAME, }; @@ -53,8 +53,18 @@ const PROTOCOL_DRAIN_GRACE: std::time::Duration = std::time::Duration::from_secs /// that lost the prefix would stop being routed to. The version is this build's rather than /// the transport's, because it is the one a peer needs in order to tell which release it is /// talking to. -fn user_agent() -> String { - format!("node/{}", env!("CARGO_PKG_VERSION")) +/// +/// A node that serves chunks also carries the get-or-closer token (ADR-0020). Native peers +/// exchange no capabilities, and a node built before that request drops it without a reply, +/// so this token is how a client knows it may send one. A node with storage switched off +/// never answers chunk requests, so it does not announce it. +fn user_agent(serves_chunks: bool) -> String { + let agent = format!("node/{}", env!("CARGO_PKG_VERSION")); + if serves_chunks { + format!("{agent} {GET_OR_CLOSER_AGENT_TOKEN}") + } else { + agent + } } /// Builder for constructing an Ant node. @@ -334,7 +344,7 @@ impl NodeBuilder { } } - core_config.custom_user_agent = Some(user_agent()); + core_config.custom_user_agent = Some(user_agent(config.storage.enabled)); // Persist close group peers + trust scores across restarts. // Default to root_dir (alongside node_identity.key) when not explicitly set. @@ -985,11 +995,10 @@ impl RunningNode { let handled = protocol .try_handle_request_with_context( data, - Some(ChunkRequestContext::new( - source.to_string(), - received_at, - queue_wait, - )), + Some( + ChunkRequestContext::new(source.to_string(), received_at, queue_wait) + .with_requester(*source), + ), ) .await; let telemetry = handled.get_telemetry; @@ -1615,13 +1624,23 @@ mod tests { } /// The prefix is what keeps this node in its peers' routing tables, and the version is - /// this build's, with nothing after it. + /// this build's, with nothing after it on a node that does not serve chunks. #[test] fn the_user_agent_is_this_builds_version_under_the_node_prefix() { assert_eq!( - super::user_agent(), + super::user_agent(false), format!("node/{}", env!("CARGO_PKG_VERSION")) ); - assert!(super::user_agent().starts_with("node/")); + assert!(super::user_agent(true).starts_with("node/")); + } + + #[test] + fn user_agent_advertises_get_or_closer_only_when_serving_chunks() { + assert!(crate::ant_protocol::advertises_get_or_closer( + &super::user_agent(true) + )); + assert!(!crate::ant_protocol::advertises_get_or_closer( + &super::user_agent(false) + )); } } diff --git a/src/storage/handler.rs b/src/storage/handler.rs index 62e2db75..156dbcac 100644 --- a/src/storage/handler.rs +++ b/src/storage/handler.rs @@ -29,10 +29,11 @@ use crate::ant_protocol::DATA_TYPE_CHUNK; use crate::ant_protocol::{ - settlement_compatibility, ChunkGetRequest, ChunkGetResponse, ChunkMessage, ChunkMessageBody, - ChunkPutRequest, ChunkPutResponse, ChunkQuoteRequest, ChunkQuoteRequestV2, ChunkQuoteResponse, - MerkleCandidateQuoteRequest, MerkleCandidateQuoteRequestV2, MerkleCandidateQuoteResponse, - ProtocolError, SettlementCompatibility, XorName, CHUNK_PROTOCOL_ID, CURRENT_SETTLEMENT_VERSION, + settlement_compatibility, ChunkGetOrCloserRequest, ChunkGetOrCloserResponse, ChunkGetRequest, + ChunkGetResponse, ChunkMessage, ChunkMessageBody, ChunkPutRequest, ChunkPutResponse, + ChunkQuoteRequest, ChunkQuoteRequestV2, ChunkQuoteResponse, MerkleCandidateQuoteRequest, + MerkleCandidateQuoteRequestV2, MerkleCandidateQuoteResponse, ProtocolError, + SettlementCompatibility, XorName, CHUNK_PROTOCOL_ID, CURRENT_SETTLEMENT_VERSION, MAX_CHUNK_SIZE, MIN_SUPPORTED_SETTLEMENT_VERSION, }; use crate::client::compute_address; @@ -48,7 +49,7 @@ use crate::storage::ChunkStore; use ant_protocol::chunk::{PointerGetResponse, PointerPutResponse}; use bytes::Bytes; use parking_lot::RwLock; -use saorsa_core::P2PNode; +use saorsa_core::{P2PNode, PeerId}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -76,6 +77,8 @@ fn duration_ms(duration: Duration) -> u64 { /// encoding, and the response hand-off to the transport. pub struct ChunkRequestContext { source_peer: String, + /// The authenticated sender, when the router knows it. + requester: Option, received_at: Instant, queue_wait_ms: u64, } @@ -85,10 +88,19 @@ impl ChunkRequestContext { pub(crate) fn new(source_peer: String, received_at: Instant, queue_wait: Duration) -> Self { Self { source_peer, + requester: None, received_at, queue_wait_ms: duration_ms(queue_wait), } } + + /// Record the authenticated sender, so a get-or-closer answer can leave + /// it out of the closer peers, as a `FIND_NODE` answer does. + #[must_use] + pub(crate) fn with_requester(mut self, requester: PeerId) -> Self { + self.requester = Some(requester); + self + } } /// A decoded protocol request and, for GETs, its pending one-shot telemetry. @@ -256,6 +268,28 @@ fn log_pointer_get_rpc(elapsed: Duration, address: &XorName, response: &PointerG ); } +fn log_get_or_closer_rpc( + elapsed: Duration, + address: &XorName, + response: &ChunkGetOrCloserResponse, +) { + let duration_ms = duration_ms(elapsed); + let outcome: &'static str = match response { + ChunkGetOrCloserResponse::Found { .. } => "found", + ChunkGetOrCloserResponse::Closer { .. } => "closer", + ChunkGetOrCloserResponse::Error(_) => "error", + _ => "unknown", + }; + let addr = hex::encode(address); + info!( + target: "ant_node::storage::rpc_latency", + duration_ms, + outcome, + addr = %addr, + "get_or_closer_rpc" + ); +} + /// How many unversioned quote requests to receive between adoption log lines. /// /// One line per request would drown the log at production quote rates, and one @@ -659,6 +693,15 @@ impl AntProtocol { let key = ChunkResponseKey::of_pointer_put(&response); (ChunkMessageBody::PointerPutResponse(response), key) } + ChunkMessageBody::GetOrCloserRequest(req) => { + let started = Instant::now(); + let address = req.address; + let requester = context.as_ref().and_then(|context| context.requester); + let response = self.handle_get_or_closer(req, requester).await; + log_get_or_closer_rpc(started.elapsed(), &address, &response); + let key = ChunkResponseKey::of_get_or_closer(&response); + (ChunkMessageBody::GetOrCloserResponse(response), key) + } ChunkMessageBody::PointerGetRequest(req) => { let started = Instant::now(); let address = req.address; @@ -922,6 +965,67 @@ impl AntProtocol { } } + /// Answer a get-or-closer request (ADR-0020): the chunk when this node + /// holds it, otherwise the `FIND_NODE` answer it would give `requester` for + /// the address. + async fn handle_get_or_closer( + &self, + request: ChunkGetOrCloserRequest, + requester: Option, + ) -> ChunkGetOrCloserResponse { + let address = request.address; + match self.storage.get(&address).await { + Ok(Some(content)) => ChunkGetOrCloserResponse::Found { + address, + content: Bytes::from(content), + }, + Ok(None) => self.closer_peers(address, requester).await, + Err(e) => { + warn!( + "Failed to read chunk {} for get-or-closer: {e}", + hex::encode(address) + ); + ChunkGetOrCloserResponse::Error(ProtocolError::StorageFailed(e.to_string())) + } + } + } + + /// This node's closest known peers to `address`, encoded by saorsa-core + /// exactly as its `FIND_NODE` answer to `requester` would hold them. + async fn closer_peers( + &self, + address: XorName, + requester: Option, + ) -> ChunkGetOrCloserResponse { + // Bind the handle out of the lock so no guard is held across `.await`. + let attached = self.p2p_node.read().as_ref().map(Arc::clone); + let Some(p2p) = attached else { + return ChunkGetOrCloserResponse::Error(ProtocolError::Internal( + "this node has no routing table to answer from".to_string(), + )); + }; + // Without a known requester there is no one to leave out; the local + // node never appears in its own answer. + let requester = requester.unwrap_or_else(|| *p2p.peer_id()); + match p2p + .dht_manager() + .encode_closer_peers(&address, &requester) + .await + { + Ok(peers) => ChunkGetOrCloserResponse::Closer { + address, + peers: Bytes::from(peers), + }, + Err(e) => { + warn!( + "Failed to encode closer peers for {}: {e}", + hex::encode(address) + ); + ChunkGetOrCloserResponse::Error(ProtocolError::Internal(e.to_string())) + } + } + } + /// Resync the quoting metric to the authoritative count of records the node /// actually holds. /// @@ -1451,6 +1555,62 @@ mod tests { } } + #[tokio::test] + async fn test_get_or_closer_returns_a_held_chunk() { + let (protocol, _temp) = create_test_protocol().await; + let content = b"get-or-closer content"; + let address = compute_address(content); + protocol + .put_local(&address, content) + .await + .expect("store chunk"); + let request = ChunkMessage { + request_id: 30, + body: ChunkMessageBody::GetOrCloserRequest(ChunkGetOrCloserRequest::new(address)), + }; + + let response_bytes = protocol + .try_handle_request(&request.encode().expect("encode")) + .await + .expect("handle get-or-closer") + .expect("expected response"); + let response = ChunkMessage::decode(&response_bytes).expect("decode response"); + + assert_eq!(response.request_id, 30); + let ChunkMessageBody::GetOrCloserResponse(ChunkGetOrCloserResponse::Found { + address: found, + content: found_content, + }) = response.body + else { + panic!("expected GetOrCloserResponse::Found"); + }; + assert_eq!(found, address); + assert_eq!(found_content.as_ref(), content); + } + + #[tokio::test] + async fn test_get_or_closer_without_a_routing_table_is_an_error() { + // A node always has a routing table; a protocol without an attached + // P2P node must refuse rather than claim it knows no closer peers. + let (protocol, _temp) = create_test_protocol().await; + let request = ChunkMessage { + request_id: 31, + body: ChunkMessageBody::GetOrCloserRequest(ChunkGetOrCloserRequest::new([0xCD; 32])), + }; + + let response_bytes = protocol + .try_handle_request(&request.encode().expect("encode")) + .await + .expect("handle get-or-closer") + .expect("expected response"); + let response = ChunkMessage::decode(&response_bytes).expect("decode response"); + + assert!(matches!( + response.body, + ChunkMessageBody::GetOrCloserResponse(ChunkGetOrCloserResponse::Error(_)) + )); + } + #[tokio::test] async fn test_get_telemetry_preserves_join_fields_and_response() { let (protocol, _temp) = create_test_protocol().await; diff --git a/src/storage/traffic.rs b/src/storage/traffic.rs index 35688e8f..d092236f 100644 --- a/src/storage/traffic.rs +++ b/src/storage/traffic.rs @@ -12,7 +12,9 @@ use std::sync::atomic::{AtomicU64, Ordering}; -use crate::ant_protocol::{ChunkGetResponse, ChunkMessageBody, ChunkPutResponse}; +use crate::ant_protocol::{ + ChunkGetOrCloserResponse, ChunkGetResponse, ChunkMessageBody, ChunkPutResponse, +}; use ant_protocol::chunk::{PointerGetResponse, PointerPutResponse}; /// Kind of an inbound chunk message, for the rx table. @@ -31,10 +33,12 @@ pub enum ChunkRequestKind { Other, /// Bytes that failed `ChunkMessage::decode`. DecodeError, + /// A get-or-closer request (ADR-0020). + GetOrCloser, } impl ChunkRequestKind { - const N: usize = 10; + const N: usize = 11; const fn index(self) -> usize { match self { @@ -48,6 +52,7 @@ impl ChunkRequestKind { Self::PointerPut => 7, Self::Other => 8, Self::DecodeError => 9, + Self::GetOrCloser => 10, } } } @@ -77,10 +82,13 @@ pub enum ChunkResponseKey { /// A response variant this table does not itemise (e.g. an `Other` /// outcome on a `#[non_exhaustive]` enum). Other, + GetOrCloserFound, + GetOrCloserCloser, + GetOrCloserError, } impl ChunkResponseKey { - const N: usize = 20; + const N: usize = 23; const fn index(self) -> usize { match self { @@ -104,6 +112,9 @@ impl ChunkResponseKey { Self::PointerPutPaymentRequired => 17, Self::PointerPutError => 18, Self::Other => 19, + Self::GetOrCloserFound => 20, + Self::GetOrCloserCloser => 21, + Self::GetOrCloserError => 22, } } } @@ -120,6 +131,7 @@ impl ChunkRequestKind { ChunkMessageBody::MerkleCandidateQuoteRequestV2(_) => Self::MerkleQuoteV2, ChunkMessageBody::PointerGetRequest(_) => Self::PointerGet, ChunkMessageBody::PointerPutRequest(_) => Self::PointerPut, + ChunkMessageBody::GetOrCloserRequest(_) => Self::GetOrCloser, _ => Self::Other, } } @@ -127,6 +139,15 @@ impl ChunkRequestKind { impl ChunkResponseKey { /// Classify a GET response by outcome. + pub fn of_get_or_closer(response: &ChunkGetOrCloserResponse) -> Self { + match response { + ChunkGetOrCloserResponse::Found { .. } => Self::GetOrCloserFound, + ChunkGetOrCloserResponse::Closer { .. } => Self::GetOrCloserCloser, + ChunkGetOrCloserResponse::Error(_) => Self::GetOrCloserError, + _ => Self::Other, + } + } + pub fn of_get(response: &ChunkGetResponse) -> Self { match response { ChunkGetResponse::Success { .. } => Self::GetSuccess, @@ -211,11 +232,12 @@ pub fn record_send_failed(bytes: usize) { /// Emit the cumulative chunk-RPC traffic as INFO summary lines, target /// `ant_node::storage::traffic`. /// -/// Flat snake-case keys like the replication summary. Three lines sharing the +/// Flat snake-case keys like the replication summary. Four lines sharing the /// same target and message, distinguished by `group`: rx by request kind -/// (`group = 1`), chunk and quote tx by kind × outcome (`group = 2`), and -/// pointer tx by kind × outcome (`group = 3`). Pointers take a line of their -/// own because `group = 2` is already close to `tracing`'s 32-field cap. +/// (`group = 1`), chunk and quote tx by kind × outcome (`group = 2`), pointer +/// tx by kind × outcome (`group = 3`), and get-or-closer rx and tx +/// (`group = 4`). Pointers and get-or-closer take lines of their own because +/// `group = 2` is already close to `tracing`'s 32-field cap. pub fn log_chunk_rpc_traffic_summary() { use ChunkRequestKind as Q; use ChunkResponseKey as R; @@ -286,6 +308,20 @@ pub fn log_chunk_rpc_traffic_summary() { pointer_put_error_tx_count = tc(R::PointerPutError), "chunk rpc traffic summary (cumulative)" ); + + crate::logging::info!( + target: "ant_node::storage::traffic", + group = 4, + get_or_closer_rx_bytes = rb(Q::GetOrCloser), + get_or_closer_rx_count = rc(Q::GetOrCloser), + get_or_closer_found_tx_bytes = tb(R::GetOrCloserFound), + get_or_closer_found_tx_count = tc(R::GetOrCloserFound), + get_or_closer_closer_tx_bytes = tb(R::GetOrCloserCloser), + get_or_closer_closer_tx_count = tc(R::GetOrCloserCloser), + get_or_closer_error_tx_bytes = tb(R::GetOrCloserError), + get_or_closer_error_tx_count = tc(R::GetOrCloserError), + "chunk rpc traffic summary (cumulative)" + ); } #[cfg(test)] @@ -305,6 +341,7 @@ mod tests { ChunkRequestKind::PointerPut, ChunkRequestKind::Other, ChunkRequestKind::DecodeError, + ChunkRequestKind::GetOrCloser, ]; let mut seen = std::collections::HashSet::new(); for k in req { @@ -332,6 +369,9 @@ mod tests { ChunkResponseKey::PointerPutPaymentRequired, ChunkResponseKey::PointerPutError, ChunkResponseKey::Other, + ChunkResponseKey::GetOrCloserFound, + ChunkResponseKey::GetOrCloserCloser, + ChunkResponseKey::GetOrCloserError, ]; let mut seen = std::collections::HashSet::new(); for k in resp { diff --git a/tests/e2e/data_types/chunk.rs b/tests/e2e/data_types/chunk.rs index 650b8e08..3f76f48f 100644 --- a/tests/e2e/data_types/chunk.rs +++ b/tests/e2e/data_types/chunk.rs @@ -294,6 +294,79 @@ mod tests { .expect("Failed to teardown harness"); } + /// ADR-0020: a node answers get-or-closer with the chunk when it holds + /// it, and otherwise with closer peers that the requester checks as a + /// `FIND_NODE` answer. + #[tokio::test(flavor = "multi_thread")] + #[allow(clippy::panic)] + async fn test_get_or_closer_returns_chunk_or_closer_peers() { + let harness = TestHarness::setup_minimal() + .await + .expect("Failed to setup test harness"); + let fixture = ChunkTestFixture::new(); + let requester = harness.test_node(3).expect("Node 3 should exist"); + let p2p = requester + .p2p_node + .as_ref() + .expect("Node 3 should be running"); + let peers = requester.connected_peers().await; + let target = peers.first().expect("Node 3 should have a connected peer"); + + let address = ChunkTestFixture::compute_address(&fixture.small); + harness.prepopulate_payment_cache_for_peer(target, &address); + requester + .store_chunk_on_peer(target, &fixture.small) + .await + .expect("Failed to store chunk on remote node"); + + let (found, _) = requester + .get_or_closer_from_peer(target, &address) + .await + .expect("get-or-closer should be answered"); + let ant_node::ant_protocol::ChunkGetOrCloserResponse::Found { + address: found_address, + content, + } = found + else { + panic!("expected the holder to answer Found, got {found:?}"); + }; + assert_eq!(found_address, address); + assert_eq!(content.as_ref(), fixture.small.as_slice()); + + // No node holds this address, so every node answers with peers. + let missing = [0x5a; 32]; + let (closer, transport_source) = requester + .get_or_closer_from_peer(target, &missing) + .await + .expect("get-or-closer should be answered"); + let ant_node::ant_protocol::ChunkGetOrCloserResponse::Closer { + address: closer_address, + peers, + } = closer + else { + panic!("expected a non-holder to answer Closer, got {closer:?}"); + }; + assert_eq!(closer_address, missing); + let candidates = p2p + .dht_manager() + .decode_closer_peers(target, &missing, &peers, transport_source.as_ref()) + .await + .expect("closer peers should decode as a FIND_NODE answer"); + assert!( + !candidates.is_empty(), + "the responder should know peers closer to the address" + ); + assert!( + candidates.iter().all(|node| node.peer_id != *p2p.peer_id()), + "the answer should leave out the requester" + ); + + harness + .teardown() + .await + .expect("Failed to teardown harness"); + } + /// Test 8: Reject oversized chunk (> 4MB). /// /// Chunks have a maximum size of 4MB. Attempting to store a larger diff --git a/tests/e2e/testnet.rs b/tests/e2e/testnet.rs index ca33a901..69372a9e 100644 --- a/tests/e2e/testnet.rs +++ b/tests/e2e/testnet.rs @@ -14,8 +14,9 @@ //! - chunk store persistence use ant_node::ant_protocol::{ - ChunkGetRequest, ChunkGetResponse, ChunkMessage, ChunkMessageBody, ChunkPutRequest, - ChunkPutResponse, CHUNK_PROTOCOL_ID, MAX_WIRE_MESSAGE_SIZE, + ChunkGetOrCloserRequest, ChunkGetOrCloserResponse, ChunkGetRequest, ChunkGetResponse, + ChunkMessage, ChunkMessageBody, ChunkPutRequest, ChunkPutResponse, CHUNK_PROTOCOL_ID, + MAX_WIRE_MESSAGE_SIZE, }; use ant_node::client::{send_and_await_chunk_response, DataChunk, XorName}; use ant_node::payment::{ @@ -865,6 +866,46 @@ impl TestNode { .await } + /// Send a get-or-closer request (ADR-0020) to a peer, returning its + /// answer and the transport address that delivered it. + /// + /// # Errors + /// + /// Returns an error if the node is not running, the request cannot be + /// encoded or sent, or no answer arrives in time. + pub async fn get_or_closer_from_peer( + &self, + target_peer_id: &PeerId, + address: &XorName, + ) -> Result<(ChunkGetOrCloserResponse, Option)> { + let p2p = self.p2p_node.as_ref().ok_or(TestnetError::NodeNotRunning)?; + let request_id: u64 = rand::thread_rng().gen(); + let message = ChunkMessage { + request_id, + body: ChunkMessageBody::GetOrCloserRequest(ChunkGetOrCloserRequest::new(*address)), + }; + let message_bytes = message.encode().map_err(|e| { + TestnetError::Serialization(format!("Failed to encode get-or-closer request: {e}")) + })?; + let timeout = Duration::from_secs(DEFAULT_CHUNK_OPERATION_TIMEOUT_SECS); + let response = ant_protocol::send_and_await_chunk_response_with_metadata( + p2p, + target_peer_id, + message_bytes, + request_id, + timeout, + &[], + |body| match body { + ChunkMessageBody::GetOrCloserResponse(response) => Some(Ok(response)), + _ => None, + }, + |e| TestnetError::Retrieval(format!("Failed to send get-or-closer: {e}")), + || TestnetError::Retrieval("Timeout waiting for a get-or-closer answer".to_string()), + ) + .await?; + Ok((response.result?, response.transport_source)) + } + /// Compute content address for chunk data (BLAKE3 hash). #[must_use] pub fn compute_chunk_address(data: &[u8]) -> XorName { From b8e18420f7c491b41c53551938cea1622e1004f2 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Wed, 7 Oct 2026 15:24:56 +0200 Subject: [PATCH 03/11] docs(adr): record ADR-0020's live and test validation A client built from the design read a 612 MB production file in 56 s, against 242 s for the current client, through the per-peer fallback that today's nodes take. In-process networks of upgraded nodes are read with get-or-closer requests only. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md index d4439c2a..bd7e3e52 100644 --- a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md +++ b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md @@ -187,6 +187,12 @@ still arriving. Conflicting reports about a peer are resolved with Measured with the per-peer fallback that this decision keeps for old nodes: p50 5.95 s to about 1 s, p90 13 s to about 2 s, and the whole file 3.7 times faster. +- **Today's nodes, through the per-peer fallback.** A client built from + this design downloaded the 612 MB production file in 56 s, against 242 s + for the current client, with identical bytes. Production nodes do not + announce the token yet, so every lookup step took the FIND_NODE-plus-GET + route, and all 150 chunks were found without falling back to the old + read path. - **Upgraded nodes answer each lookup step with one request** instead of FIND_NODE plus GET. The chunk travels in that same reply. - **Compatible in both directions.** Old clients never send the new variant. @@ -241,9 +247,12 @@ still arriving. Conflicting reports about a peer are resolved with - treat a hash mismatch as an error - keep a transfer alive past its lookup round's grace window - fall back to today's path when the lookup finds nothing + - In-process test networks: + - A network whose nodes all announce the token is read with + get-or-closer requests only, and no plain GET. + - The full end-to-end suite reads through the new path. - Live: a new client against today's nodes takes the legacy route for every - peer and stays within the measured latency of option 3. A local devnet of - upgraded nodes takes the `GetOrCloserRequest` route. + peer and stays within the measured latency of option 3. - **Review triggers** - Node egress for reads rises enough to matter: add the "have it" variant. - Browser reads need this request: add the portable decoder and a HELLO From 2b578aa4aed4730e198db2ffb963292449c681f1 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:10:56 +0200 Subject: [PATCH 04/11] docs(adr): ADR-0020 bounds closer peers and drops peers with bad content From review of the ADR-0020 stack: - decode_closer_peers rejects a payload over the 64 KiB FIND_NODE limit before parsing it (saorsa-core fd37775). The chunk message carrying it may be up to 5 MiB. - A peer whose content fails verification only drops out of the client's lookup (ant-client 7d9a248). Every queried peer is asked for the chunk, so ending the read on one bad answer would let any dishonest node on a lookup path fail reads. Today's read path, which runs when the lookup finds nothing, keeps its rule that bad content ends the read. - The trade-offs now list that every queried peer can send content. - Validation now covers both changes. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- .../ADR-0020-kademlia-chunk-get-or-closer.md | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md index bd7e3e52..236aeb72 100644 --- a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md +++ b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md @@ -160,8 +160,10 @@ For each peer the lookup queries, the client: the lookup's own request would need anyway. It then reads the peer's user agent. 2. **If the peer advertises `get-or-closer/1`,** sends `GetOrCloserRequest`: - - `Found`: verify the content hash, then stop the read. - - `Closer`: `decode_closer_peers` supplies the lookup candidates. + - `Found`: verify the address and the content hash, then stop the read. + - `Closer`: `decode_closer_peers` supplies the lookup candidates. It + rejects a payload over the 64 KiB FIND_NODE limit before parsing it, + although the chunk message carrying it may be up to 5 MiB. 3. **Otherwise,** sends FIND_NODE (`DhtNetworkManager::find_node_on_peer`) and a GET at the same time. FIND_NODE supplies the candidates, and a `Success` from the GET stops the read. @@ -173,9 +175,13 @@ The round's grace window therefore never cancels a chunk transfer that is still arriving. Conflicting reports about a peer are resolved with `client_routing::compute_winner`, as in both existing lookups. +- **A peer whose content fails verification only drops out of the lookup.** + Every queried peer is asked for the chunk, not just the close group. Ending + the read on one bad answer would therefore let any dishonest node on a + lookup path fail reads. - **When the lookup ends without a chunk,** or fails, the client runs today's - read path (`retrieve_progressive`) unchanged. Its absence rule and retry - round are unchanged. + read path (`retrieve_progressive`) unchanged. Its absence rule, retry round + and rule that bad content ends the read are unchanged. - **Browser clients keep today's read path.** Supporting this request there needs a portable decoder for the peer format and a HELLO capability. @@ -216,6 +222,10 @@ still arriving. Conflicting reports about a peer are resolved with connection anyway. - **A second chunk response shape** to maintain, with its own traffic accounting. +- **Every queried peer can send content,** not only peers near the chunk. + Each answer is bounded by the 5 MiB chunk message limit, as a GET answer is + today, and is verified before use. A peer whose answer fails verification + costs the client that transfer and nothing more. ### Neutral / Operational @@ -235,6 +245,8 @@ still arriving. Conflicting reports about a peer are resolved with - `encode_closer_peers` / `decode_closer_peers` return the same trusted candidates as a FIND_NODE answer for the same key. - `find_node_on_peer` and `connect_lookup_peer` behave against a live peer. + - `decode_closer_peers` rejects a payload over the FIND_NODE size limit + before parsing it, and reads one of exactly that size. - **ant-node** - The handler answers `Found` for a held chunk and `Closer`, with decodable peers, otherwise. @@ -244,7 +256,7 @@ still arriving. Conflicting reports about a peer are resolved with - stop at the first verified chunk - fall back per peer for legacy peers - handle a mixed network - - treat a hash mismatch as an error + - drop a peer whose content fails verification, and keep reading - keep a transfer alive past its lookup round's grace window - fall back to today's path when the lookup finds nothing - In-process test networks: From 7c5e78bd9012c50402b99458b4d6c489d3cdd71b Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:35:09 +0200 Subject: [PATCH 05/11] docs(adr): ADR-0020 records the measured browser variant Browsers keep today's read. The browser variant was built and A/B'd on a 60-node testnet, and was not faster: a warm browser already holds sessions with the chunk's close group there. The review trigger now names when to revisit it. Co-Authored-By: Claude Opus 5.5 --- .../adr/ADR-0020-kademlia-chunk-get-or-closer.md | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md index 236aeb72..1e03301a 100644 --- a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md +++ b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md @@ -144,7 +144,7 @@ FIND_NODE today. - **It advertises support** with the user-agent token `get-or-closer/1` (`ant_protocol::GET_OR_CLOSER_AGENT_TOKEN`), after the existing migration token. The `node/` prefix that saorsa-core gates DHT membership on is kept. -- **It does not advertise support to browsers yet.** Browsers cannot decode the +- **It does not advertise support to browsers.** Browsers cannot decode the native peer format. The WebRTC Direct listener keeps refusing the request, and HELLO does not list it. @@ -182,8 +182,14 @@ still arriving. Conflicting reports about a peer are resolved with - **When the lookup ends without a chunk,** or fails, the client runs today's read path (`retrieve_progressive`) unchanged. Its absence rule, retry round and rule that bad content ends the read are unchanged. -- **Browser clients keep today's read path.** Supporting this request there - needs a portable decoder for the peer format and a HELLO capability. +- **Browser clients keep today's read path.** A browser variant was built and + measured on a 60-node testnet (2026-10-12). Nodes took the request inside + `chunk_protocol` and listed it in HELLO, and answered a missing chunk with + their browser `find_node` reply. Against today's browser read it was 17% + slower from fra1 and 5% faster from syd1, neither significant. A warm + browser there already holds sessions with the chunk's close group, so + today's read sends its first GET to a holder, and a lookup cannot start + sooner than that. ## Consequences @@ -267,8 +273,8 @@ still arriving. Conflicting reports about a peer are resolved with peer and stays within the measured latency of option 3. - **Review triggers** - Node egress for reads rises enough to matter: add the "have it" variant. - - Browser reads need this request: add the portable decoder and a HELLO - capability. + - Browsers spend their reads finding a holder, because they no longer hold + sessions with the close group: revisit the browser variant above. - saorsa-core changes its native lookup parameters: mirror them in the client lookup. From ecaa0c67cc3e8f9bd56815875647fdd482c998ea Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:20:04 +0100 Subject: [PATCH 06/11] docs(adr): ADR-0020 spreads first reads as ADR-0023 does ADR-0023 made the ordinary read choose its first GET uniformly among a chunk's close group, so a popular chunk no longer loads one node while the rest of its close group idles. The get-or-closer read asked the nearest likely holder first, which put that load back on one node for every native read. Record that it now chooses the first likely holder uniformly among those a lookup round hands over, and that the likely holders are asked one at a time and hedged after 500 ms. The user-agent token now follows `node/` directly: ADR-0022 removed the migration token it used to follow. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- .../ADR-0020-kademlia-chunk-get-or-closer.md | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md index 1e03301a..fc6134b2 100644 --- a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md +++ b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md @@ -9,7 +9,9 @@ - **Related:** Linear V2-1358; ant-client investigation `docs/investigations/2026-10-06-kademlia-chunk-get/`; [ADR-0015](ADR-0015-direct-browser-clients-over-webrtc-direct.md) (browser - capabilities); saorsa-core, ant-protocol and ant-client branches + capabilities); + [ADR-0023](ADR-0023-spread-hot-reads-and-refuse-full-browser-sessions.md) + (spread first reads); saorsa-core, ant-protocol and ant-client branches `mickvandijke/v2-1358-kad-chunk-get` ## Context @@ -142,8 +144,8 @@ FIND_NODE today. - **Otherwise it answers `Closer`,** using `encode_closer_peers` with the authenticated sender as the requester. - **It advertises support** with the user-agent token `get-or-closer/1` - (`ant_protocol::GET_OR_CLOSER_AGENT_TOKEN`), after the existing migration - token. The `node/` prefix that saorsa-core gates DHT membership on is kept. + (`ant_protocol::GET_OR_CLOSER_AGENT_TOKEN`), after `node/`. The + `node/` prefix that saorsa-core gates DHT membership on is kept. - **It does not advertise support to browsers.** Browsers cannot decode the native peer format. The WebRTC Direct listener keeps refusing the request, and HELLO does not list it. @@ -175,6 +177,15 @@ The round's grace window therefore never cancels a chunk transfer that is still arriving. Conflicting reports about a peer are resolved with `client_routing::compute_winner`, as in both existing lookups. +- **Peers that probably hold the chunk are asked one at a time.** A peer is a + likely holder when fewer than a close group of the peers the lookup knows + of are nearer the chunk. The next likely holder is asked once the current + one answers `Closer`, or after 500 ms if it is still sending. +- **The first likely holder is chosen uniformly**, among those the lookup + round has handed over, as the ordinary read chooses its first GET + ([ADR-0023](ADR-0023-spread-hot-reads-and-refuse-full-browser-sessions.md)). + Asking the nearest holder first would put every client's read of a popular + chunk on one node of its close group. - **A peer whose content fails verification only drops out of the lookup.** Every queried peer is asked for the chunk, not just the close group. Ending the read on one bad answer would therefore let any dishonest node on a @@ -265,6 +276,8 @@ still arriving. Conflicting reports about a peer are resolved with - drop a peer whose content fails verification, and keep reading - keep a transfer alive past its lookup round's grace window - fall back to today's path when the lookup finds nothing + - choose the first likely holder among a whole lookup round, not the + first step to arrive - In-process test networks: - A network whose nodes all announce the token is read with get-or-closer requests only, and no plain GET. From 3fc87ea12b0a0197527e4e36e9c530d9235626d2 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:20:51 +0100 Subject: [PATCH 07/11] chore(deps): pin saorsa-core and ant-protocol at the rebased ADR-0020 branches saorsa-core c3c4c81 is saorsa-core#173 rebased onto 0.29.2, and ant-protocol 0c6ae27 is ant-protocol#44 rebased onto 5.0.1, the rc-2026.10.3 baseline this branch now sits on. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 8 +++----- Cargo.toml | 7 +++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 248d9970..56385dab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -895,8 +895,7 @@ dependencies = [ [[package]] name = "ant-protocol" version = "5.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d95baa1de35f7183806372d59baa34b93c2de8f28c76c664f3225008ed17317" +source = "git+https://github.com/WithAutonomi/ant-protocol?rev=0c6ae27b6261227a32e6fe0f976163c42bde960e#0c6ae27b6261227a32e6fe0f976163c42bde960e" dependencies = [ "blake3", "bytes", @@ -5278,8 +5277,7 @@ dependencies = [ [[package]] name = "saorsa-core" version = "0.29.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49de4f92b6808afd54a206d7dcb6cf754f796ee876394d6838b79b39dd1a72dd" +source = "git+https://github.com/WithAutonomi/saorsa-core?rev=c3c4c81e63db7e3048f488f7cf0fec4cff3131d4#c3c4c81e63db7e3048f488f7cf0fec4cff3131d4" dependencies = [ "anyhow", "async-trait", @@ -7055,7 +7053,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index aa249a27..355364f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -214,6 +214,13 @@ webrtc-direct = [ "saorsa-transport/webrtc-direct", ] +[patch.crates-io] +# ADR-0020 (V2-1358): saorsa-core's closer-peers encoding and +# ant-protocol's get-or-closer request, each rebased onto its main. Drop +# both once releases include them. +saorsa-core = { git = "https://github.com/WithAutonomi/saorsa-core", rev = "c3c4c81e63db7e3048f488f7cf0fec4cff3131d4" } +ant-protocol = { git = "https://github.com/WithAutonomi/ant-protocol", rev = "0c6ae27b6261227a32e6fe0f976163c42bde960e" } + [profile.release] lto = true codegen-units = 1 From c02917ce1275bc80f63bca35b2f293831fdf5503 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:02:55 +0100 Subject: [PATCH 08/11] fix(devnet): announce get-or-closer as production nodes do Devnet nodes built their core config without the node's user agent, so they never announced the get-or-closer token. A client reading from a local devnet therefore took the legacy route for every peer, and ADR-0020's own path went unexercised there. Devnet nodes serve chunks, so they now announce it, as `NodeBuilder` does for a node with storage. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- src/devnet.rs | 3 +++ src/node.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/devnet.rs b/src/devnet.rs index 9b16645f..4c836614 100644 --- a/src/devnet.rs +++ b/src/devnet.rs @@ -1011,6 +1011,9 @@ impl Devnet { .bootstrap_peers .clone_from(&node.bootstrap_addrs); core_config.diversity_config = Some(IPDiversityConfig::permissive()); + // Devnet nodes serve chunks, so they announce get-or-closer + // (ADR-0020) as a production node does. + core_config.custom_user_agent = Some(crate::node::user_agent(true)); let index = node.index; let p2p_node = P2PNode::new(core_config) diff --git a/src/node.rs b/src/node.rs index 0459da13..9f48cb4d 100644 --- a/src/node.rs +++ b/src/node.rs @@ -58,7 +58,7 @@ const PROTOCOL_DRAIN_GRACE: std::time::Duration = std::time::Duration::from_secs /// exchange no capabilities, and a node built before that request drops it without a reply, /// so this token is how a client knows it may send one. A node with storage switched off /// never answers chunk requests, so it does not announce it. -fn user_agent(serves_chunks: bool) -> String { +pub(crate) fn user_agent(serves_chunks: bool) -> String { let agent = format!("node/{}", env!("CARGO_PKG_VERSION")); if serves_chunks { format!("{agent} {GET_OR_CLOSER_AGENT_TOKEN}") From c6cabd54782aaeb978197b77807c96049e077f48 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:02:55 +0100 Subject: [PATCH 09/11] fix(storage): give get-or-closer the read telemetry a GET has A get-or-closer is a chunk read, but it logged only a bare `get_or_closer_rpc` line, at INFO for every answer, and none of the GET read event's fields: queue wait, storage read, encode and send time, and send outcome. - It now emits the same `get_rpc` event a GET does. A new `request` field tells the two apart (`get` or `get_or_closer`). A `Closer` answer has storage outcome `not_found`, as a GET for a chunk the node does not hold. - The separate `get_or_closer_rpc` line is gone, so each read logs one line, as before. - `of_get` gets its doc comment back from `of_get_or_closer`, and the new response keys are documented. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- src/storage/handler.rs | 154 +++++++++++++++++++++++++++++++---------- src/storage/traffic.rs | 5 +- 2 files changed, 122 insertions(+), 37 deletions(-) diff --git a/src/storage/handler.rs b/src/storage/handler.rs index 156dbcac..c4d1b075 100644 --- a/src/storage/handler.rs +++ b/src/storage/handler.rs @@ -123,6 +123,8 @@ pub struct HandledChunkRequest { /// event with `cancelled` outcomes so the request does not disappear silently. #[cfg_attr(not(feature = "logging"), allow(dead_code))] pub struct GetRequestTelemetry { + /// The read request: `get`, or `get_or_closer` (ADR-0020). + request: &'static str, source_peer: String, request_id: u64, chunk_address: String, @@ -143,14 +145,61 @@ impl GetRequestTelemetry { storage_read_ms: u64, response: &ChunkGetResponse, ) -> Self { - let (storage_outcome, storage_error_category) = match response { + let storage = match response { ChunkGetResponse::Success { .. } => ("success", "none"), ChunkGetResponse::NotFound { .. } => ("not_found", "none"), ChunkGetResponse::Error(ProtocolError::StorageFailed(_)) => ("error", "storage"), ChunkGetResponse::Error(_) => ("error", "protocol"), _ => ("unknown", "unknown"), }; + Self::new( + "get", + context, + request_id, + chunk_address, + storage_read_ms, + storage, + ) + } + + /// A get-or-closer read (ADR-0020). Answering with closer peers means the + /// chunk was not in storage, so its storage outcome is `not_found`. + fn from_get_or_closer_response( + context: ChunkRequestContext, + request_id: u64, + chunk_address: String, + storage_read_ms: u64, + response: &ChunkGetOrCloserResponse, + ) -> Self { + let storage = match response { + ChunkGetOrCloserResponse::Found { .. } => ("success", "none"), + ChunkGetOrCloserResponse::Closer { .. } => ("not_found", "none"), + ChunkGetOrCloserResponse::Error(ProtocolError::StorageFailed(_)) => { + ("error", "storage") + } + ChunkGetOrCloserResponse::Error(_) => ("error", "protocol"), + _ => ("unknown", "unknown"), + }; + Self::new( + "get_or_closer", + context, + request_id, + chunk_address, + storage_read_ms, + storage, + ) + } + + fn new( + request: &'static str, + context: ChunkRequestContext, + request_id: u64, + chunk_address: String, + storage_read_ms: u64, + (storage_outcome, storage_error_category): (&'static str, &'static str), + ) -> Self { Self { + request, source_peer: context.source_peer, request_id, chunk_address, @@ -201,6 +250,7 @@ impl GetRequestTelemetry { let duration_ms = duration_ms(self.received_at.elapsed()); info!( target: "ant_node::storage::rpc_latency", + request = self.request, source_peer = %self.source_peer, request_id = self.request_id, chunk_address = %self.chunk_address, @@ -268,28 +318,6 @@ fn log_pointer_get_rpc(elapsed: Duration, address: &XorName, response: &PointerG ); } -fn log_get_or_closer_rpc( - elapsed: Duration, - address: &XorName, - response: &ChunkGetOrCloserResponse, -) { - let duration_ms = duration_ms(elapsed); - let outcome: &'static str = match response { - ChunkGetOrCloserResponse::Found { .. } => "found", - ChunkGetOrCloserResponse::Closer { .. } => "closer", - ChunkGetOrCloserResponse::Error(_) => "error", - _ => "unknown", - }; - let addr = hex::encode(address); - info!( - target: "ant_node::storage::rpc_latency", - duration_ms, - outcome, - addr = %addr, - "get_or_closer_rpc" - ); -} - /// How many unversioned quote requests to receive between adoption log lines. /// /// One line per request would drown the log at production quote rates, and one @@ -694,11 +722,9 @@ impl AntProtocol { (ChunkMessageBody::PointerPutResponse(response), key) } ChunkMessageBody::GetOrCloserRequest(req) => { - let started = Instant::now(); - let address = req.address; - let requester = context.as_ref().and_then(|context| context.requester); - let response = self.handle_get_or_closer(req, requester).await; - log_get_or_closer_rpc(started.elapsed(), &address, &response); + let (response, telemetry) = + self.handle_get_or_closer(req, request_id, context).await; + get_telemetry = telemetry; let key = ChunkResponseKey::of_get_or_closer(&response); (ChunkMessageBody::GetOrCloserResponse(response), key) } @@ -966,18 +992,24 @@ impl AntProtocol { } /// Answer a get-or-closer request (ADR-0020): the chunk when this node - /// holds it, otherwise the `FIND_NODE` answer it would give `requester` for - /// the address. + /// holds it, otherwise the `FIND_NODE` answer it would give the requester + /// for the address. With a request context, also returns the read's + /// telemetry, as a GET's. async fn handle_get_or_closer( &self, request: ChunkGetOrCloserRequest, - requester: Option, - ) -> ChunkGetOrCloserResponse { + request_id: u64, + context: Option, + ) -> (ChunkGetOrCloserResponse, Option) { let address = request.address; - match self.storage.get(&address).await { - Ok(Some(content)) => ChunkGetOrCloserResponse::Found { + let requester = context.as_ref().and_then(|context| context.requester); + let storage_started = Instant::now(); + let stored = self.storage.get(&address).await; + let storage_read_ms = duration_ms(storage_started.elapsed()); + let response = match stored { + Ok(Some(chunk)) => ChunkGetOrCloserResponse::Found { address, - content: Bytes::from(content), + content: Bytes::from(chunk), }, Ok(None) => self.closer_peers(address, requester).await, Err(e) => { @@ -987,7 +1019,17 @@ impl AntProtocol { ); ChunkGetOrCloserResponse::Error(ProtocolError::StorageFailed(e.to_string())) } - } + }; + let telemetry = context.map(|context| { + GetRequestTelemetry::from_get_or_closer_response( + context, + request_id, + hex::encode(address), + storage_read_ms, + &response, + ) + }); + (response, telemetry) } /// This node's closest known peers to `address`, encoded by saorsa-core @@ -1635,6 +1677,7 @@ mod tests { .await; let telemetry = handled.get_telemetry.expect("GET telemetry"); + assert_eq!(telemetry.request, "get"); assert_eq!(telemetry.source_peer, "peer-v2-995"); assert_eq!(telemetry.request_id, request_id); assert_eq!(telemetry.chunk_address, hex::encode(address)); @@ -1657,6 +1700,45 @@ mod tests { telemetry.finish_send(Duration::from_millis(3), true); } + /// A get-or-closer is a read, so it carries the same telemetry as a GET. + #[tokio::test] + async fn test_get_or_closer_found_carries_read_telemetry() { + let (protocol, _temp) = create_test_protocol().await; + let content = b"get-or-closer telemetry"; + let address = compute_address(content); + protocol + .put_local(&address, content) + .await + .expect("store chunk"); + let request_id = 996; + let request = ChunkMessage { + request_id, + body: ChunkMessageBody::GetOrCloserRequest(ChunkGetOrCloserRequest::new(address)), + }; + + let handled = protocol + .try_handle_request_with_context( + &request.encode().expect("encode get-or-closer"), + Some(ChunkRequestContext::new( + "peer-v2-996".to_string(), + Instant::now(), + Duration::ZERO, + )), + ) + .await; + + let telemetry = handled.get_telemetry.expect("get-or-closer telemetry"); + assert_eq!(telemetry.request, "get_or_closer"); + assert_eq!(telemetry.request_id, request_id); + assert_eq!(telemetry.chunk_address, hex::encode(address)); + assert_eq!(telemetry.storage_outcome, "success"); + assert_eq!( + handled.traffic_key, + Some(ChunkResponseKey::GetOrCloserFound) + ); + telemetry.finish_send(Duration::from_millis(3), true); + } + #[tokio::test] async fn test_put_address_mismatch() { let (protocol, _temp) = create_test_protocol().await; diff --git a/src/storage/traffic.rs b/src/storage/traffic.rs index d092236f..23e1672b 100644 --- a/src/storage/traffic.rs +++ b/src/storage/traffic.rs @@ -82,7 +82,9 @@ pub enum ChunkResponseKey { /// A response variant this table does not itemise (e.g. an `Other` /// outcome on a `#[non_exhaustive]` enum). Other, + /// A get-or-closer answer (ADR-0020) carrying the chunk. GetOrCloserFound, + /// A get-or-closer answer carrying closer peers. GetOrCloserCloser, GetOrCloserError, } @@ -138,7 +140,7 @@ impl ChunkRequestKind { } impl ChunkResponseKey { - /// Classify a GET response by outcome. + /// Classify a get-or-closer response by outcome. pub fn of_get_or_closer(response: &ChunkGetOrCloserResponse) -> Self { match response { ChunkGetOrCloserResponse::Found { .. } => Self::GetOrCloserFound, @@ -148,6 +150,7 @@ impl ChunkResponseKey { } } + /// Classify a GET response by outcome. pub fn of_get(response: &ChunkGetResponse) -> Self { match response { ChunkGetResponse::Success { .. } => Self::GetSuccess, From 1189217769bb3ef03adef1b5bd8191454bae4f69 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:04:47 +0100 Subject: [PATCH 10/11] test(e2e): answer chunk requests through the node's own router The e2e harness answered chunk requests with its own router, which called `try_handle_request` without the sender. The get-or-closer test then checked that the answer left out the requester after decoding it on the requester, which drops itself from any answer it decodes, so the check could not fail. Decoded on a third node, the harness's answers named the requester. - `RunningNode::answer_chunk_request` (test-utils) answers a message as a running node's router does: with the authenticated sender, the GET telemetry and the traffic counters. The harness now routes every chunk message through it, so the e2e suite exercises the production path. - The test decodes on a third node and asks with the requester's own ID as the address, so an answer that kept the requester would name it first. As a control, the same node's answer to another peer must name the requester. The strengthened test failed against the old harness router and passes through the node's. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- src/node.rs | 22 ++++++++++ tests/e2e/data_types/chunk.rs | 78 ++++++++++++++++++++++++++++++----- tests/e2e/testnet.rs | 31 +++----------- 3 files changed, 95 insertions(+), 36 deletions(-) diff --git a/src/node.rs b/src/node.rs index 9f48cb4d..390dd84c 100644 --- a/src/node.rs +++ b/src/node.rs @@ -978,6 +978,28 @@ impl RunningNode { Ok(()) } + /// Answer one inbound chunk-protocol message from `source` as a running + /// node's router does, and send the reply. For test networks that route + /// messages themselves. + #[cfg(any(test, feature = "test-utils"))] + pub async fn answer_chunk_request( + protocol: &Arc, + p2p: &Arc, + source: &saorsa_core::identity::PeerId, + data: &[u8], + ) { + Self::answer_one_request( + protocol, + p2p, + source, + data, + "chunk", + CHUNK_PROTOCOL_ID, + Instant::now(), + ) + .await; + } + /// Handle one inbound protocol message and send whatever it produced. async fn answer_one_request( protocol: &Arc, diff --git a/tests/e2e/data_types/chunk.rs b/tests/e2e/data_types/chunk.rs index 3f76f48f..7cba5895 100644 --- a/tests/e2e/data_types/chunk.rs +++ b/tests/e2e/data_types/chunk.rs @@ -294,6 +294,25 @@ mod tests { .expect("Failed to teardown harness"); } + /// The peers a get-or-closer answer from `responder` names for `key`, + /// decoded on `decoder` as a `FIND_NODE` answer. + async fn decode_closer_peers( + decoder: &saorsa_core::P2PNode, + responder: &saorsa_core::identity::PeerId, + key: &[u8; 32], + peers: &[u8], + transport_source: Option<&saorsa_core::MultiAddr>, + ) -> Vec { + decoder + .dht_manager() + .decode_closer_peers(responder, key, peers, transport_source) + .await + .expect("closer peers should decode as a FIND_NODE answer") + .into_iter() + .map(|node| node.peer_id) + .collect() + } + /// ADR-0020: a node answers get-or-closer with the chunk when it holds /// it, and otherwise with closer peers that the requester checks as a /// `FIND_NODE` answer. @@ -333,8 +352,10 @@ mod tests { assert_eq!(found_address, address); assert_eq!(content.as_ref(), fixture.small.as_slice()); - // No node holds this address, so every node answers with peers. - let missing = [0x5a; 32]; + // No node holds this address, so every node answers with peers. It is + // the requester's own ID, so the requester is the nearest peer the + // responder could name. + let missing = *p2p.peer_id().as_bytes(); let (closer, transport_source) = requester .get_or_closer_from_peer(target, &missing) .await @@ -347,18 +368,53 @@ mod tests { panic!("expected a non-holder to answer Closer, got {closer:?}"); }; assert_eq!(closer_address, missing); - let candidates = p2p - .dht_manager() - .decode_closer_peers(target, &missing, &peers, transport_source.as_ref()) - .await - .expect("closer peers should decode as a FIND_NODE answer"); + + // A third node decodes the answers, since a node leaves itself out of + // any answer it decodes. Another node asking the same shows that the + // responder does name the requester to anyone else. + let others = (0..harness.node_count()) + .filter_map(|index| harness.test_node(index)) + .filter(|node| { + node.p2p_node.as_ref().is_some_and(|other| { + other.peer_id() != p2p.peer_id() && other.peer_id() != target + }) + }) + .collect::>(); + // The request goes out without dial addresses, so the bystander must + // already be connected to the responder. + let mut bystander = None; + for node in &others { + if node.connected_peers().await.contains(target) { + bystander = Some(*node); + break; + } + } + let bystander = bystander.expect("another node should be connected to the responder"); + let decoder = others + .iter() + .filter(|node| !std::ptr::eq(**node, bystander)) + .find_map(|node| node.p2p_node.as_ref()) + .expect("a fourth node should exist"); + let named = + decode_closer_peers(decoder, target, &missing, &peers, transport_source.as_ref()).await; + assert!(!named.is_empty(), "the responder should know other peers"); assert!( - !candidates.is_empty(), - "the responder should know peers closer to the address" + !named.contains(p2p.peer_id()), + "the answer should leave out the requester" ); + + let (control, control_source) = bystander + .get_or_closer_from_peer(target, &missing) + .await + .expect("get-or-closer should be answered"); + let ant_node::ant_protocol::ChunkGetOrCloserResponse::Closer { peers, .. } = control else { + panic!("expected a non-holder to answer Closer, got {control:?}"); + }; assert!( - candidates.iter().all(|node| node.peer_id != *p2p.peer_id()), - "the answer should leave out the requester" + decode_closer_peers(decoder, target, &missing, &peers, control_source.as_ref()) + .await + .contains(p2p.peer_id()), + "another node's answer should name the requester" ); harness diff --git a/tests/e2e/testnet.rs b/tests/e2e/testnet.rs index 69372a9e..7dddd407 100644 --- a/tests/e2e/testnet.rs +++ b/tests/e2e/testnet.rs @@ -1366,32 +1366,13 @@ impl TestNetwork { ); let protocol = Arc::clone(&protocol_clone); let p2p = Arc::clone(&p2p_clone); + // As a node's own router answers it, so the + // handler knows the authenticated sender. tokio::spawn(async move { - match protocol.try_handle_request(&data).await { - Ok(Some(response)) => { - if let Err(e) = p2p - .send_message( - &source, - CHUNK_PROTOCOL_ID, - response.to_vec(), - &[], - ) - .await - { - warn!( - "Node {node_index} failed to send chunk response to {source}: {e}" - ); - } - } - Ok(None) => { - // Response message — no reply needed - } - Err(e) => { - warn!( - "Node {node_index} chunk protocol handler error: {e}" - ); - } - } + ant_node::RunningNode::answer_chunk_request( + &protocol, &p2p, &source, &data, + ) + .await; }); } } From 6a14b538412665436996516b636ca85538cb5fdf Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:04:47 +0100 Subject: [PATCH 11/11] docs(adr): ADR-0020 routes at once and asks only likely holders Records the review fixes in ant-client#221 and this PR: - Every queried peer answers the lookup at once. Only the likely holder chosen to go next is asked for the chunk in its lookup step; other likely holders answer with FIND_NODE and wait for a GET, and far peers get FIND_NODE only. - Until the read knows two close groups of peers, every peer counts as a likely holder, so the first round is gated too. - At most two likely holders are asked at once. - The ordinary read starts from the peers the lookup ended on. - Node load stays as today's: routing never waits behind PUTs, and get-or-closer waits only as a GET does. - Validation lists the new driver tests, the legacy-node e2e test, the requester control, the read telemetry and devnet nodes' token. Part of V2-1358. Co-Authored-By: Claude Opus 5.5 --- .../ADR-0020-kademlia-chunk-get-or-closer.md | 99 +++++++++++++------ 1 file changed, 67 insertions(+), 32 deletions(-) diff --git a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md index fc6134b2..37e72bc1 100644 --- a/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md +++ b/docs/adr/ADR-0020-kademlia-chunk-get-or-closer.md @@ -156,20 +156,34 @@ A chunk read runs a client-driven iterative lookup on saorsa-core's public lookup engine (`IterativeLookup`). The parameters match today's native lookup: alpha 3, K, a 5 s grace period per round, and a 120 s deadline. +Every peer the lookup queries answers it at once, but only peers that +probably hold the chunk are asked for it. A peer is a **likely holder** when +fewer than a close group of the peers the read knows of are nearer the chunk. +Until the read knows two close groups of peers, as in its first round, it +cannot tell, and every peer counts as one. + For each peer the lookup queries, the client: 1. **Connects to it.** It uses `DhtNetworkManager::connect_lookup_peer`, which the lookup's own request would need anyway. It then reads the peer's user agent. -2. **If the peer advertises `get-or-closer/1`,** sends `GetOrCloserRequest`: - - `Found`: verify the address and the content hash, then stop the read. - - `Closer`: `decode_closer_peers` supplies the lookup candidates. It - rejects a payload over the 64 KiB FIND_NODE limit before parsing it, - although the chunk message carrying it may be up to 5 MiB. -3. **Otherwise,** sends FIND_NODE (`DhtNetworkManager::find_node_on_peer`) and - a GET at the same time. FIND_NODE supplies the candidates, and a `Success` - from the GET stops the read. -4. **If the connection fails,** marks the peer failed in the lookup, as a +2. **If the peer is the likely holder chosen to go next,** asks it for the + chunk and for closer peers in one step: + - If it advertises `get-or-closer/1`, sends `GetOrCloserRequest`: + - `Found`: verify the address and the content hash, then stop the read. + - `Closer`: `decode_closer_peers` supplies the lookup candidates. It + rejects a payload over the 64 KiB FIND_NODE limit before parsing it, + although the chunk message carrying it may be up to 5 MiB. + - Otherwise, sends FIND_NODE (`DhtNetworkManager::find_node_on_peer`) and + a GET at the same time. FIND_NODE supplies the candidates, and a + `Success` from the GET stops the read. +3. **If it is a likely holder that must wait its turn,** sends FIND_NODE now, + so the lookup never waits for the holder gate, and a GET when its turn + comes. A waiting peer that the lookup has since learnt is not among the + nearest is skipped. +4. **If it is far from the chunk,** sends FIND_NODE only, as today's lookup + does. +5. **If the connection fails,** marks the peer failed in the lookup, as a failed FIND_NODE is marked today. The read driver owns every request, not the lookup round that triggered it. @@ -177,22 +191,25 @@ The round's grace window therefore never cancels a chunk transfer that is still arriving. Conflicting reports about a peer are resolved with `client_routing::compute_winner`, as in both existing lookups. -- **Peers that probably hold the chunk are asked one at a time.** A peer is a - likely holder when fewer than a close group of the peers the lookup knows - of are nearer the chunk. The next likely holder is asked once the current - one answers `Closer`, or after 500 ms if it is still sending. +- **Likely holders are asked one at a time.** The next is asked once the + current one answers without the chunk, or after 500 ms if it is still + sending. At most two are asked at once, as today's read never runs more + than two GETs: on a link too slow to receive a chunk in 500 ms, further + copies would only slow each other. - **The first likely holder is chosen uniformly**, among those the lookup round has handed over, as the ordinary read chooses its first GET ([ADR-0023](ADR-0023-spread-hot-reads-and-refuse-full-browser-sessions.md)). Asking the nearest holder first would put every client's read of a popular chunk on one node of its close group. -- **A peer whose content fails verification only drops out of the lookup.** - Every queried peer is asked for the chunk, not just the close group. Ending - the read on one bad answer would therefore let any dishonest node on a - lookup path fail reads. +- **A peer whose content fails verification only drops out of the read.** + Likely holders are judged from what the lookup has learnt, and in its first + round any peer is one. Ending the read on one bad answer would therefore let + a dishonest node on a lookup path fail reads. - **When the lookup ends without a chunk,** or fails, the client runs today's - read path (`retrieve_progressive`) unchanged. Its absence rule, retry round - and rule that bad content ends the read are unchanged. + read path (`retrieve_progressive`). Its absence rule, retry round and rule + that bad content ends the read are unchanged. Its first round starts from + the peers the lookup ended on instead of looking them up again, so reading + a chunk that is not stored still costs one lookup. - **Browser clients keep today's read path.** A browser variant was built and measured on a 60-node testnet (2026-10-12). Nodes took the request inside `chunk_protocol` and listed it in HELLO, and answered a missing chunk with @@ -216,8 +233,13 @@ still arriving. Conflicting reports about a peer are resolved with announce the token yet, so every lookup step took the FIND_NODE-plus-GET route, and all 150 chunks were found without falling back to the old read path. -- **Upgraded nodes answer each lookup step with one request** instead of - FIND_NODE plus GET. The chunk travels in that same reply. +- **An upgraded likely holder answers its lookup step and the read in one + request,** instead of FIND_NODE plus GET. The chunk travels in that same + reply. +- **Node load stays as today's.** Peers that only route get FIND_NODE, which + the DHT layer answers, and only likely holders get a chunk request. Nodes + answer get-or-closer within the same 64-request limit as GETs and PUTs, so + under heavy uploads it waits as a GET does, but routing never waits there. - **Compatible in both directions.** Old clients never send the new variant. New clients send it only to nodes that advertise it, and treat every other peer as before. @@ -227,10 +249,11 @@ still arriving. Conflicting reports about a peer are resolved with ### Negative / Trade-offs -- **Duplicate downloads.** Up to alpha holders can return the chunk in the - same round: measured at 2.5–3.3 copies per chunk natively. That costs bytes - for the client and egress for the nodes. A "have it" or inline-flag variant - can bound this later without changing the request defined here. +- **Duplicate downloads.** Asking every queried peer for the chunk, as first + measured, downloaded 2.5–3.3 copies per chunk natively. The holder gate + bounds this to at most two copies in flight, the second only after 500 ms, + but a hedged copy can still arrive unused. A "have it" or inline-flag + variant can bound this further without changing the request defined here. - **The native client runs its own lookup**, beside the one in saorsa-core. Lookup parameters and peer policy must be kept in step. - **Capability is a user-agent token**, and is known only once a peer is @@ -239,10 +262,13 @@ still arriving. Conflicting reports about a peer are resolved with connection anyway. - **A second chunk response shape** to maintain, with its own traffic accounting. -- **Every queried peer can send content,** not only peers near the chunk. - Each answer is bounded by the 5 MiB chunk message limit, as a GET answer is - today, and is verified before use. A peer whose answer fails verification - costs the client that transfer and nothing more. +- **A likely holder can send content even when it is far from the chunk,** + because the first round cannot tell. Each answer is bounded by the 5 MiB + chunk message limit, as a GET answer is today, and is verified before use. A + peer whose answer fails verification costs the client that transfer and + nothing more. +- **A waiting likely holder costs two requests,** FIND_NODE now and a GET + later, where a get-or-closer would have been one. ### Neutral / Operational @@ -266,7 +292,10 @@ still arriving. Conflicting reports about a peer are resolved with before parsing it, and reads one of exactly that size. - **ant-node** - The handler answers `Found` for a held chunk and `Closer`, with decodable - peers, otherwise. + peers, otherwise. `Closer` leaves out the requester, which the same + node's answer to another peer names. + - A get-or-closer carries the same read telemetry as a GET. + - Devnet nodes announce the token, as production nodes do. - The user agent carries the token and keeps the `node/` prefix. - **ant-client** - The read driver is tested on a scripted network. It must: @@ -275,12 +304,18 @@ still arriving. Conflicting reports about a peer are resolved with - handle a mixed network - drop a peer whose content fails verification, and keep reading - keep a transfer alive past its lookup round's grace window - - fall back to today's path when the lookup finds nothing + - fall back to today's path when the lookup finds nothing, handing it the + peers the lookup ended on - choose the first likely holder among a whole lookup round, not the first step to arrive + - ask at most two likely holders at once + - answer the lookup at once for a likely holder that waits its turn, and + skip it if the lookup has since passed it + - ask peers far from the chunk only for closer peers - In-process test networks: - A network whose nodes all announce the token is read with - get-or-closer requests only, and no plain GET. + get-or-closer requests. One whose nodes do not is read with FIND_NODE + and GET, and is sent no get-or-closer request. - The full end-to-end suite reads through the new path. - Live: a new client against today's nodes takes the legacy route for every peer and stays within the measured latency of option 3.