From ff0b1f66c126d7511b1ee1a5f0f5787920292341 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 21 Sep 2026 19:16:21 +0200 Subject: [PATCH 01/24] fix(replication): bound encoded fresh offers waiting behind send permits Under sustained client writes on a real network, nodes grew to 1-2 GiB within an hour. Heap profiles on the testnet attribute ~80% of live memory to encoded FreshReplicationOffer messages queued in the fresh-write drainer: every accepted PUT was encoded immediately (chunk plus proof, ~4-5 MiB) and its per-peer send tasks then waited for one of MAX_CONCURRENT_REPLICATION_SENDS (3) permits while pinning that buffer. When WAN sends hold permits longer than writes arrive, nothing bounded the backlog, so the number of encoded offers kept growing. - FreshWriteEvent no longer carries the chunk bytes; the chunk is on disk already and the drainer reads it back when it is ready to send. - The drainer acquires a pending-offer permit (MAX_PENDING_FRESH_OFFERS) before reading and encoding, and the permit lives with the encoded buffer until the last per-peer send drops it. A backlog now waits as small queued events instead of chunk-sized buffers. - The direct ReplicationEngine::replicate_fresh entry point takes the same permit so tests and callers share the bound. Co-Authored-By: Claude Fable 5.1 --- src/replication/config.rs | 11 ++++++ src/replication/fresh.rs | 42 ++++++++++++++------- src/replication/mod.rs | 79 ++++++++++++++++++++++++++++----------- src/storage/handler.rs | 9 ++--- 4 files changed, 100 insertions(+), 41 deletions(-) diff --git a/src/replication/config.rs b/src/replication/config.rs index 276ba1bb..8bd8666a 100644 --- a/src/replication/config.rs +++ b/src/replication/config.rs @@ -170,6 +170,17 @@ pub const SELF_LOOKUP_INTERVAL_MAX: Duration = Duration::from_secs(SELF_LOOKUP_I /// at most ~12 MB queued for the upload link at any instant. pub const MAX_CONCURRENT_REPLICATION_SENDS: usize = 3; +/// Maximum number of encoded fresh-replication offers held in memory. +/// +/// Each accepted write is encoded once (chunk plus proof, up to ~4 MB) and +/// that buffer stays alive until the last of its per-peer sends completes. +/// With only `MAX_CONCURRENT_REPLICATION_SENDS` transfers in flight, a write +/// rate above the network's send rate would otherwise queue an unbounded +/// number of encoded offers behind the send permits. The fresh-write drainer +/// takes one of these permits before it reads and encodes a chunk, so the +/// backlog waits as small queued events instead of chunk-sized buffers. +pub const MAX_PENDING_FRESH_OFFERS: usize = 8; + /// Maximum number of concurrent in-flight audit-responder tasks. /// /// The LIGHT audit-responder handlers — responsible-chunk audits and subtree diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index 80e9766e..d41e283d 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -11,7 +11,7 @@ use crate::logging::{debug, warn}; use rand::Rng; use saorsa_core::identity::PeerId; use saorsa_core::P2PNode; -use tokio::sync::Semaphore; +use tokio::sync::{OwnedSemaphorePermit, Semaphore}; use crate::ant_protocol::XorName; use crate::replication::config::{ @@ -26,16 +26,27 @@ use crate::replication::protocol::{ /// /// Sent from the chunk PUT handler to the replication engine via an /// unbounded channel so that the PUT response is not blocked by -/// replication fan-out. +/// replication fan-out. The event deliberately carries no chunk bytes: the +/// chunk is already on disk, and the drainer reads it back only once it +/// holds a pending-offer permit, so a replication backlog queues as small +/// events rather than chunk-sized buffers. pub struct FreshWriteEvent { /// Content-address of the stored chunk. pub key: XorName, - /// The chunk data. - pub data: Vec, /// Serialized proof-of-payment. pub payment_proof: Vec, } +/// An encoded fresh offer shared by the per-peer send tasks. +/// +/// The pending-offer permit is released together with the buffer, once the +/// last send task drops its reference, which caps how many encoded offers +/// can wait behind the send permits at `MAX_PENDING_FRESH_OFFERS`. +struct EncodedOffer { + bytes: Vec, + _pending: OwnedSemaphorePermit, +} + /// Execute fresh replication for a newly accepted record. /// /// Sends fresh offers to close group members (with bounded delivery retries, @@ -45,7 +56,10 @@ pub struct FreshWriteEvent { /// /// The `send_semaphore` limits how many outbound chunk transfers can be /// in-flight concurrently across the entire replication engine, preventing -/// bandwidth saturation on home broadband connections. +/// bandwidth saturation on home broadband connections. `pending_offer` is the +/// caller's permit from the pending-offer semaphore; it is held with the +/// encoded offer until the last per-peer send finishes. +#[allow(clippy::too_many_arguments)] pub async fn replicate_fresh( key: &XorName, data: &[u8], @@ -54,6 +68,7 @@ pub async fn replicate_fresh( paid_list: &Arc, config: &ReplicationConfig, send_semaphore: &Arc, + pending_offer: OwnedSemaphorePermit, ) -> Vec { let self_id = *p2p_node.peer_id(); @@ -95,11 +110,15 @@ pub async fn replicate_fresh( }; // Share one encoded copy across the per-peer send tasks so a retry only // re-materialises the buffer for the (consuming) send call, keeping the - // common single-attempt path at one clone per peer. - let encoded = Arc::new(encoded); + // common single-attempt path at one clone per peer. The pending-offer + // permit travels with the buffer. + let encoded = Arc::new(EncodedOffer { + bytes: encoded, + _pending: pending_offer, + }); for peer in &target_peers { let p2p = Arc::clone(p2p_node); - let data = Arc::clone(&encoded); + let offer = Arc::clone(&encoded); let peer_id = *peer; let sem = Arc::clone(send_semaphore); tokio::spawn(async move { @@ -117,12 +136,7 @@ pub async fn replicate_fresh( let mut attempt = 0u32; loop { match p2p - .send_message( - &peer_id, - REPLICATION_PROTOCOL_ID, - data.as_ref().clone(), - &[], - ) + .send_message(&peer_id, REPLICATION_PROTOCOL_ID, offer.bytes.clone(), &[]) .await { Ok(()) => break, diff --git a/src/replication/mod.rs b/src/replication/mod.rs index 73d91f45..e916a665 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -81,7 +81,7 @@ use crate::replication::commitment_state::{ use crate::replication::config::{ max_parallel_fetch, storage_admission_width, ReplicationConfig, MAX_AUDIT_RESPONSES_PER_PEER, MAX_CONCURRENT_AUDIT_RESPONSES, MAX_CONCURRENT_REPLICATION_SENDS, - MAX_DIGEST_AUDIT_RESPONSES_PER_PEER, MAX_INCOMING_VERIFICATION_KEYS, + MAX_DIGEST_AUDIT_RESPONSES_PER_PEER, MAX_INCOMING_VERIFICATION_KEYS, MAX_PENDING_FRESH_OFFERS, MAX_SUBTREE_ROUND1_PER_PEER, MAX_SUBTREE_SESSIONS, MAX_VERIFICATION_KEYS_PER_CYCLE, REPLICATION_PROTOCOL_ID, SUBTREE_AUDIT_PROTOCOL_ID, SUBTREE_ROUND1_WORK_BURST_BYTES, SUBTREE_ROUND1_WORK_REFILL_BYTES_PER_SEC, SUBTREE_SESSION_TTL, @@ -1748,6 +1748,9 @@ pub struct ReplicationEngine { /// Limits concurrent outbound replication sends to prevent bandwidth /// saturation on home broadband connections. send_semaphore: Arc, + /// Bounds how many encoded fresh offers can wait behind `send_semaphore`; + /// see [`MAX_PENDING_FRESH_OFFERS`]. + pending_offer_semaphore: Arc, /// Bounds concurrent IN-FLIGHT LIGHT audit-responder tasks (responsible-chunk /// audits + subtree slice round 2). The heavy subtree round 1 has its own /// tighter pool ([`SubtreeRound1Limiter`]). Those are spawned off the serial @@ -1917,6 +1920,7 @@ impl ReplicationEngine { recent_provers: Arc::new(RwLock::new(RecentProvers::new())), sig_verify_attempts: Arc::new(RwLock::new(HashMap::new())), send_semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_REPLICATION_SENDS)), + pending_offer_semaphore: Arc::new(Semaphore::new(MAX_PENDING_FRESH_OFFERS)), audit_responder_semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_AUDIT_RESPONSES)), audit_responder_inflight: Arc::new(RwLock::new(HashMap::new())), audit_responder_metrics: Arc::new(AuditResponderMetrics::default()), @@ -2414,6 +2418,13 @@ impl ReplicationEngine { /// drainer; this direct entry point schedules here so callers (and tests) /// that drive replication directly still get the possession check. pub async fn replicate_fresh(&self, key: &XorName, data: &[u8], proof_of_payment: &[u8]) { + // The semaphore is never closed, so this only fails at shutdown. + let Ok(pending_offer) = Arc::clone(&self.pending_offer_semaphore) + .acquire_owned() + .await + else { + return; + }; let peers = fresh::replicate_fresh( key, data, @@ -2422,6 +2433,7 @@ impl ReplicationEngine { &self.paid_list, &self.config, &self.send_semaphore, + pending_offer, ) .await; if !peers.is_empty() { @@ -2443,37 +2455,62 @@ impl ReplicationEngine { }; let p2p = Arc::clone(&self.p2p_node); let paid_list = Arc::clone(&self.paid_list); + let storage = Arc::clone(&self.storage); let config = Arc::clone(&self.config); let send_semaphore = Arc::clone(&self.send_semaphore); + let pending_offer_semaphore = Arc::clone(&self.pending_offer_semaphore); let possession_tx = self.possession_check_tx.clone(); let shutdown = self.shutdown.clone(); let handle = tokio::spawn(async move { loop { - tokio::select! { + let event = tokio::select! { () = shutdown.cancelled() => break, event = rx.recv() => { let Some(event) = event else { break }; - let peers = fresh::replicate_fresh( - &event.key, - &event.data, - &event.payment_proof, - &p2p, - &paid_list, - &config, - &send_semaphore, - ) - .await; - // Schedule the delayed possession check (ADR-0003) for - // the responsible close-group peers. A closed receiver - // (engine shutting down) is ignored. - if !peers.is_empty() { - let _ = possession_tx.send(possession::PossessionCheckEvent { - key: event.key, - peers, - }); - } + event + } + }; + // Wait for a pending-offer permit before touching the chunk so a + // send backlog holds queued events, not encoded chunk buffers. + let pending_offer = tokio::select! { + () = shutdown.cancelled() => break, + permit = Arc::clone(&pending_offer_semaphore).acquire_owned() => { + let Ok(permit) = permit else { break }; + permit + } + }; + let key_hex = hex::encode(event.key); + let data = match storage.get(&event.key).await { + Ok(Some(data)) => data, + Ok(None) => { + debug!("Chunk {key_hex} no longer stored, skipping fresh replication"); + continue; } + Err(e) => { + warn!("Failed to read chunk {key_hex} for fresh replication: {e}"); + continue; + } + }; + let peers = fresh::replicate_fresh( + &event.key, + &data, + &event.payment_proof, + &p2p, + &paid_list, + &config, + &send_semaphore, + pending_offer, + ) + .await; + // Schedule the delayed possession check (ADR-0003) for + // the responsible close-group peers. A closed receiver + // (engine shutting down) is ignored. + if !peers.is_empty() { + let _ = possession_tx.send(possession::PossessionCheckEvent { + key: event.key, + peers, + }); } } debug!("Fresh-write drainer shut down"); diff --git a/src/storage/handler.rs b/src/storage/handler.rs index 840a5ef9..8474478f 100644 --- a/src/storage/handler.rs +++ b/src/storage/handler.rs @@ -871,14 +871,11 @@ impl AntProtocol { // fall back to the original proof rather than dropping the // replication entirely. let proof = Self::strip_commitment_sidecars(proof); - // `request.content` is now `bytes::Bytes`; FreshWriteEvent - // still carries the chunk as `Vec` for compatibility - // with the replication wire format, so materialise once - // here. Done only on the success path, where storage has - // already accepted the chunk. + // Storage has already accepted the chunk on this path, so + // the event carries only the key; the replication drainer + // reads the chunk back when it is ready to send it. let event = FreshWriteEvent { key: address, - data: request.content.to_vec(), payment_proof: proof, }; if tx.send(event).is_err() { From 1e2e23398bcd5fccab4b63236de2e099f9a4980a Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 22 Sep 2026 08:58:28 +0200 Subject: [PATCH 02/24] perf(replication): cut copies of fresh offers on the send path Two of the copies each queued fresh offer carried were avoidable inside this crate: - The chunk read from storage now moves into FreshReplicationOffer instead of being copied, and the offer is dropped as soon as it has been encoded, so only the encoded bytes stay alive while sends queue. - ReplicationMessage::encode serializes into a buffer sized from postcard's serialized_size. A doubling Vec left chunk-sized messages with up to twice their length in capacity, retained by every queued offer for as long as it waited for a send permit. The remaining copies per in-flight send live in saorsa-core (payload clone per channel attempt, signing re-serialization, wire frame) and saorsa-transport (stream buffer copy in SendStream::write). Co-Authored-By: Claude Fable 5.1 --- src/replication/fresh.rs | 13 +++++++++---- src/replication/mod.rs | 4 ++-- src/replication/protocol.rs | 26 +++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index d41e283d..9d05cbeb 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -58,11 +58,12 @@ struct EncodedOffer { /// in-flight concurrently across the entire replication engine, preventing /// bandwidth saturation on home broadband connections. `pending_offer` is the /// caller's permit from the pending-offer semaphore; it is held with the -/// encoded offer until the last per-peer send finishes. +/// encoded offer until the last per-peer send finishes. `data` is taken by +/// value so the chunk moves into the offer instead of being copied. #[allow(clippy::too_many_arguments)] pub async fn replicate_fresh( key: &XorName, - data: &[u8], + data: Vec, proof_of_payment: &[u8], p2p_node: &Arc, paid_list: &Arc, @@ -92,7 +93,7 @@ pub async fn replicate_fresh( let offer = FreshReplicationOffer { key: *key, - data: data.to_vec(), + data, proof_of_payment: proof_of_payment.to_vec(), }; let request_id = rand::thread_rng().gen::(); @@ -101,7 +102,11 @@ pub async fn replicate_fresh( body: ReplicationMessageBody::FreshReplicationOffer(offer), }; - let Ok(encoded) = offer_msg.encode() else { + let encoded = offer_msg.encode(); + // Only the encoded bytes are needed from here on; release the chunk now + // rather than holding it alongside the encoding while sends are queued. + drop(offer_msg); + let Ok(encoded) = encoded else { warn!( "Failed to encode FreshReplicationOffer for {}", hex::encode(key), diff --git a/src/replication/mod.rs b/src/replication/mod.rs index e916a665..baeb5ba0 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2427,7 +2427,7 @@ impl ReplicationEngine { }; let peers = fresh::replicate_fresh( key, - data, + data.to_vec(), proof_of_payment, &self.p2p_node, &self.paid_list, @@ -2494,7 +2494,7 @@ impl ReplicationEngine { }; let peers = fresh::replicate_fresh( &event.key, - &data, + data, &event.payment_proof, &p2p, &paid_list, diff --git a/src/replication/protocol.rs b/src/replication/protocol.rs index b2ca7962..0ddfe9c9 100644 --- a/src/replication/protocol.rs +++ b/src/replication/protocol.rs @@ -46,7 +46,13 @@ impl ReplicationMessage { /// Returns [`ReplicationProtocolError::SerializationFailed`] if postcard /// serialization fails. pub fn encode(&self) -> Result, ReplicationProtocolError> { - let bytes = postcard::to_stdvec(self) + // Size the buffer exactly up front. Chunk-carrying bodies run to + // several MiB, and a growing `Vec` would otherwise end up with up to + // twice the needed capacity, retained for as long as the encoded + // message is queued for sending. + let size = postcard::experimental::serialized_size(self) + .map_err(|e| ReplicationProtocolError::SerializationFailed(e.to_string()))?; + let bytes = postcard::to_extend(self, Vec::with_capacity(size)) .map_err(|e| ReplicationProtocolError::SerializationFailed(e.to_string()))?; // The same family ceiling the decoder applies, from the same table and @@ -2595,6 +2601,24 @@ mod tests { assert_eq!(decoded.request_id, 7); } + #[test] + fn encode_allocates_exactly_the_serialized_size() { + // A chunk-sized offer must not carry growth slack: the encoded buffer is + // shared by every per-peer send task for as long as it is queued. + let msg = ReplicationMessage { + request_id: 7, + body: ReplicationMessageBody::FreshReplicationOffer(FreshReplicationOffer { + key: [3; 32], + data: vec![0xAB; 3 * 1024 * 1024 + 123], + proof_of_payment: vec![1, 2, 3], + }), + }; + let encoded = msg.encode().unwrap(); + assert_eq!(encoded.capacity(), encoded.len()); + let decoded = ReplicationMessage::decode(&encoded).unwrap(); + assert_eq!(decoded.request_id, 7); + } + #[test] fn encode_rejects_oversized_message() { // Build a message whose serialized form exceeds the limit. From 408eea048e521a925badba7f13a469b0f4a0da03 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:35:07 +0200 Subject: [PATCH 03/24] perf(replication): share fresh offers with the transport as Bytes With saorsa-core accepting `impl Into` on `send_message`, the encoded fresh offer is now held as `Bytes` and each per-peer send attempt hands out a reference-counted handle instead of cloning the multi-MiB buffer. Together with the exactly-sized frame and the transport's owned-buffer write, an in-flight send now costs one frame instead of the previous four copies. Adds ADR-0017 describing the bounded fresh-offer backlog and the copy-free send path across ant-node, saorsa-core and saorsa-transport. Pins: saorsa-core 0da3260c, saorsa-transport c2b9f3b0 (both perf/replication-send-path). Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 6 +- Cargo.toml | 7 + ...ounded-fresh-offers-and-copy-free-sends.md | 128 ++++++++++++++++++ docs/adr/README.md | 1 + src/replication/fresh.rs | 16 ++- 5 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md diff --git a/Cargo.lock b/Cargo.lock index 7e146a36..12b07181 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5305,8 +5305,7 @@ dependencies = [ [[package]] name = "saorsa-core" version = "0.28.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "84923dc4955c42dfad8e06cfc34ef75b8e89d4d2fed048b18b81dc9ebadf790b" +source = "git+https://github.com/WithAutonomi/saorsa-core?rev=0da3260cd731cf706fd76f2d2589a8725022438b#0da3260cd731cf706fd76f2d2589a8725022438b" dependencies = [ "anyhow", "async-trait", @@ -5374,8 +5373,7 @@ dependencies = [ [[package]] name = "saorsa-transport" version = "0.37.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "261ed20ee1ed8555594fef412b428fa20c21d2cbeebfe3f11868c7df9489a410" +source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=c2b9f3b0b0d1ed9df1be595058974259254254e6#c2b9f3b0b0d1ed9df1be595058974259254254e6" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index a129dd58..01f2d9d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -228,6 +228,13 @@ webrtc-direct = [ "dep:self_encryption", ] +[patch.crates-io] +# Copy-free sends (ADR-0017): saorsa-core's `send_message` takes +# `impl Into`, and saorsa-transport writes owned buffers to QUIC +# streams without copying. Drop both once releases include them. +saorsa-core = { git = "https://github.com/WithAutonomi/saorsa-core", rev = "7f815141ff8d5e07f443c25c99607586fa7099cf" } +saorsa-transport = { git = "https://github.com/WithAutonomi/saorsa-transport", rev = "6a772bd3cf806e381cf5706cfe34492bede3141e" } + [profile.release] lto = true codegen-units = 1 diff --git a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md new file mode 100644 index 00000000..f43a5c3f --- /dev/null +++ b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md @@ -0,0 +1,128 @@ +# ADR-0017: Bounded fresh-replication offers and copy-free message sends + +- **Status:** Proposed +- **Date:** 2026-09-22 +- **Decision owners:** +- **Reviewers:** +- **Supersedes:** none +- **Superseded by:** none +- **Related:** [ADR-0003](ADR-0003-full-node-detection-and-eviction.md) + (best-effort fresh delivery and possession checks), + [ADR-0005](ADR-0005-replication-repair-hardening.md), + ant-node `perf/replication-send-path`, saorsa-core `perf/replication-send-path`, + saorsa-transport `perf/replication-send-path`, + `ant-testnet/state/comparisons/web-support-memory-diag-0921/` (heap profiles + and per-minute live/RSS reports from the diagnosis) + +## Context + +Under sustained client uploads on a 60-node DigitalOcean testnet, individual +nodes grew from ~100 MiB to 1–2 GiB of resident memory within an hour and +kept growing for as long as writes continued. Heap profiles taken on the +running nodes attributed roughly 80% of live memory to one site: encoded +`FreshReplicationOffer` messages queued by the fresh-write drainer. + +The mechanism was structural rather than a leak: + +- Every accepted PUT was pushed to the drainer with the full chunk, and + `replicate_fresh` encoded the offer (chunk plus proof, up to ~4–5 MiB) + immediately, before any send permit was held. +- One send task per close-group peer then waited for one of + `MAX_CONCURRENT_REPLICATION_SENDS` (3) permits while pinning that encoded + buffer. Nothing bounded how many chunks could be waiting in that state. +- On a real network each send holds its permit for seconds (QUIC delivery + acknowledgement, retries, unreachable NAT peers), so a write rate above + the send rate grew the queue without limit. Loopback devnets never showed + it because sends complete instantly. + +Once the backlog was bounded, the profile showed the remaining cost per +in-flight send: the same frame existed as the caller's serialized message +*and* as the QUIC stream's copy for the whole transfer, plus transient +copies made while framing (payload clone per channel attempt, owned wire +message for signing, and a doubling `Vec` that left chunk-sized frames with +up to twice their length in capacity). + +## Decision Drivers + +- Node memory must stay bounded under any client write rate; replication + may be delayed by backpressure but must not be dropped. +- The change must not alter the wire format, storage format or payment + logic, so it can ship as a behavioural fix. +- Existing callers of the send APIs in saorsa-core and saorsa-transport must + keep compiling and behaving the same. + +## Considered Options + +1. Bound the fresh-write channel and drop or block PUT handling when full. + Rejected: either silently loses replication or blocks client responses + on network conditions. +2. Raise `MAX_CONCURRENT_REPLICATION_SENDS`. Rejected: only moves the + knee of the curve and increases bandwidth pressure on home links; the + queue behind the permits would still be unbounded. +3. Keep events small and take a bounded permit before materialising an + offer; separately remove the avoidable copies on the send path. Chosen. + +## Decision + +We will bound the number of encoded fresh offers that can exist at once and +make the send path hand a single owned buffer down to the QUIC stream: + +- `FreshWriteEvent` carries only the key and the payment proof. The drainer + acquires a `MAX_PENDING_FRESH_OFFERS` (8) permit before it reads the chunk + back from storage and encodes it; the permit lives with the encoded offer + until the last per-peer send drops it. A backlog therefore waits as small + queued events, and at most ~40 MiB of encoded offers exist per node. +- The chunk moves into the offer rather than being copied, and + `ReplicationMessage::encode` serializes into an exactly-sized buffer. +- The encoded offer is shared as `Bytes`; saorsa-core's `send_message` + accepts `impl Into`, frames the payload through a borrowing + `WireMessageRef` (byte-identical to `WireMessage` on the wire) into an + exactly-sized frame, and passes that frame as `Bytes` to + saorsa-transport's new `send_bytes`, where the QUIC stream takes ownership + via `write_chunks` instead of copying it. + +## Consequences + +### Positive + +- Memory under write load is bounded by configuration: pending offers plus + the three in-flight sends, each held once, instead of growing with the + backlog. On the diagnostic fleets peak live memory fell from 1068 MiB to + 298 MiB (mimalloc build) and from 674 MiB to 262 MiB (jemalloc build) + after the backpressure change alone. +- Every large send node-wide (chunk GET responses included) stops paying + for a second copy of its frame during the transfer. +- No wire, storage or API break: `send(&[u8])` remains and copies once as + before; `Vec` callers of `send_message` convert without copying. + +### Negative / Trade-offs + +- Replication of a burst of writes is spread out in time rather than + encoded eagerly; the delayed possession check is scheduled after each + offer's sends are dispatched, so it shifts by the same amount. +- The drainer re-reads each chunk from disk when its permit arrives, one + extra read per accepted write. + +### Neutral / Operational + +- `MAX_PENDING_FRESH_OFFERS` and `MAX_CONCURRENT_REPLICATION_SENDS` are + the two knobs; raising the first trades memory for burst absorption. +- The signing step still serializes the payload once to produce the signed + bytes; changing that would alter the signature input and is out of scope. + +## Validation + +- Unit tests: exact-capacity encoding of chunk-sized offers (ant-node) and + byte-for-byte equivalence of `WireMessageRef` with `WireMessage` + (saorsa-core); replication unit and e2e fresh-replication scenarios pass. +- Testnet evidence (2026-09-21): with the backpressure change, the node + that had reached 1051 MiB live memory stayed flat at 0.0 MiB/min with a + 150 MiB peak, and the worst bootstrap's queued offers dropped from 101 + (463 MiB) to 6 (17.7 MiB) in heap profiles. +- Review trigger: any change to fresh replication fan-out, send permits, or + the wire-message framing must re-run the memory diagnostics under + sustained uploads and confirm live memory stays bounded. + +## 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 5061984e..cb17f2f3 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -36,3 +36,4 @@ See [`TOOLING.md`](./TOOLING.md) for `adrs`, `adr-kit`, and AI harness setup. - [ADR-0013: Settlement version and pre-payment compatibility](./ADR-0013-settlement-version-and-pre-payment-compatibility.md) - [ADR-0015: Direct browser clients over WebRTC Direct](./ADR-0015-direct-browser-clients-over-webrtc-direct.md) - [ADR-0016: Pointers — paid mutable references with an immutable owner](./ADR-0016-pointers-immutable-owner.md) +- [ADR-0017: Bounded fresh-replication offers and copy-free message sends](./ADR-0017-bounded-fresh-offers-and-copy-free-sends.md) diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index 9d05cbeb..8fd3cdc6 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use crate::logging::{debug, warn}; +use bytes::Bytes; use rand::Rng; use saorsa_core::identity::PeerId; use saorsa_core::P2PNode; @@ -41,9 +42,11 @@ pub struct FreshWriteEvent { /// /// The pending-offer permit is released together with the buffer, once the /// last send task drops its reference, which caps how many encoded offers -/// can wait behind the send permits at `MAX_PENDING_FRESH_OFFERS`. +/// can wait behind the send permits at `MAX_PENDING_FRESH_OFFERS`. The bytes +/// are shared with the transport as well: each send attempt hands out a +/// reference-counted handle rather than a copy. struct EncodedOffer { - bytes: Vec, + bytes: Bytes, _pending: OwnedSemaphorePermit, } @@ -113,12 +116,11 @@ pub async fn replicate_fresh( ); return Vec::new(); }; - // Share one encoded copy across the per-peer send tasks so a retry only - // re-materialises the buffer for the (consuming) send call, keeping the - // common single-attempt path at one clone per peer. The pending-offer - // permit travels with the buffer. + // One encoded copy serves every per-peer send task and every retry; the + // transport borrows it through `Bytes` instead of taking a copy. The + // pending-offer permit travels with the buffer. let encoded = Arc::new(EncodedOffer { - bytes: encoded, + bytes: Bytes::from(encoded), _pending: pending_offer, }); for peer in &target_peers { From 1663d70791a2bcb4b63ee76b0315f37c1c1d8b85 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:09:20 +0200 Subject: [PATCH 04/24] fix(replication): send PaidNotify before waiting for an offer permit PaidNotify carries the paid-list evidence the paid close group needs to repair a key later. Since the pending-offer permit was introduced it was sent from replicate_fresh, i.e. only after the drainer had waited for a permit, so a chunk backlog also delayed the evidence. Send it as soon as a write is dequeued (and from the direct replicate_fresh entry point), before any permit wait; only the bulk chunk offers are back-pressured. Nothing is dropped by the permit: the fresh-write queue is unbounded and FIFO, permits are released whenever a send terminates, and each offer keeps the same fan-out, retries and delayed possession check. ADR-0017 now says so explicitly and records the measured download-latency cost. Co-Authored-By: Claude Fable 5.1 --- ...ounded-fresh-offers-and-copy-free-sends.md | 20 ++++++++++++---- src/replication/fresh.rs | 23 ++++++++++--------- src/replication/mod.rs | 5 ++++ 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md index f43a5c3f..9e1113cd 100644 --- a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md +++ b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md @@ -68,10 +68,15 @@ We will bound the number of encoded fresh offers that can exist at once and make the send path hand a single owned buffer down to the QUIC stream: - `FreshWriteEvent` carries only the key and the payment proof. The drainer - acquires a `MAX_PENDING_FRESH_OFFERS` (8) permit before it reads the chunk - back from storage and encodes it; the permit lives with the encoded offer - until the last per-peer send drops it. A backlog therefore waits as small - queued events, and at most ~40 MiB of encoded offers exist per node. + sends `PaidNotify` to the paid close group as soon as it dequeues a write, + so the paid-list evidence that later repair depends on is never delayed by + chunk back-pressure. It then acquires a `MAX_PENDING_FRESH_OFFERS` (8) + permit before it reads the chunk back from storage and encodes it; the + permit lives with the encoded offer until the last per-peer send drops it. + A backlog therefore waits as small queued events, and at most ~40 MiB of + encoded offers exist per node. Nothing is dropped: the queue is unbounded + and FIFO, and every offer is still dispatched with the same fan-out, + retries and delayed possession check. - The chunk moves into the offer rather than being copied, and `ReplicationMessage::encode` serializes into an exactly-sized buffer. - The encoded offer is shared as `Bytes`; saorsa-core's `send_message` @@ -99,7 +104,12 @@ make the send path hand a single owned buffer down to the QUIC stream: - Replication of a burst of writes is spread out in time rather than encoded eagerly; the delayed possession check is scheduled after each - offer's sends are dispatched, so it shifts by the same amount. + offer's sends are dispatched, so it shifts by the same amount. A chunk + fetched seconds after its upload can therefore have fewer replicas than + before (the 2026-09-22 comparison measured downloads of just-uploaded + files 8% slower). Paid-list evidence is not affected, and the previous + unbounded fan-out lost that evidence outright under load (2,795 + "paid notify dropped at admission" in one hour on the baseline fleet). - The drainer re-reads each chunk from disk when its permit arrives, one extra read per accepted write. diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index 8fd3cdc6..9414a98f 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -53,9 +53,12 @@ struct EncodedOffer { /// Execute fresh replication for a newly accepted record. /// /// Sends fresh offers to close group members (with bounded delivery retries, -/// ADR-0003) and `PaidNotify` to `PaidCloseGroup`. Returns the close-group -/// peers responsible for the key (excluding self) so the caller can schedule -/// the delayed possession check; `PaidNotify` remains fire-and-forget. +/// ADR-0003). Returns the close-group peers responsible for the key +/// (excluding self) so the caller can schedule the delayed possession check. +/// `PaidNotify` is deliberately not sent here: it carries the paid-list +/// evidence peers need to repair the key later, so callers send it with +/// [`send_paid_notify`] as soon as the write is accepted, before waiting for +/// a pending-offer permit. /// /// The `send_semaphore` limits how many outbound chunk transfers can be /// in-flight concurrently across the entire replication engine, preventing @@ -166,13 +169,8 @@ pub async fn replicate_fresh( }); } - // Rule 7-8: Send PaidNotify to every member of PaidCloseGroup(K). - // PaidNotify messages are small metadata (no chunk data), so they don't - // need semaphore gating. - send_paid_notify(key, proof_of_payment, p2p_node, config).await; - debug!( - "Fresh replication initiated for {} to {} peers + PaidNotify", + "Fresh replication initiated for {} to {} peers", hex::encode(key), target_peers.len() ); @@ -182,8 +180,11 @@ pub async fn replicate_fresh( /// Send `PaidNotify(K)` to every peer in `PaidCloseGroup(K)` (fire-and-forget). /// -/// Per Invariant 16: sender MUST attempt delivery to every member. -async fn send_paid_notify( +/// Per Invariant 16: sender MUST attempt delivery to every member. The +/// message is small metadata (no chunk data), so it is neither gated by the +/// send semaphore nor by the pending-offer permit: rules 7-8 run as soon as +/// the write is accepted, even when chunk offers are backed up. +pub(crate) async fn send_paid_notify( key: &XorName, proof_of_payment: &[u8], p2p_node: &Arc, diff --git a/src/replication/mod.rs b/src/replication/mod.rs index baeb5ba0..f3658bc9 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2418,6 +2418,7 @@ impl ReplicationEngine { /// drainer; this direct entry point schedules here so callers (and tests) /// that drive replication directly still get the possession check. pub async fn replicate_fresh(&self, key: &XorName, data: &[u8], proof_of_payment: &[u8]) { + fresh::send_paid_notify(key, proof_of_payment, &self.p2p_node, &self.config).await; // The semaphore is never closed, so this only fails at shutdown. let Ok(pending_offer) = Arc::clone(&self.pending_offer_semaphore) .acquire_owned() @@ -2471,6 +2472,10 @@ impl ReplicationEngine { event } }; + // Paid-list evidence goes out immediately: it is what lets the + // paid close group repair the key later, so it must never wait + // behind chunk offers. + fresh::send_paid_notify(&event.key, &event.payment_proof, &p2p, &config).await; // Wait for a pending-offer permit before touching the chunk so a // send backlog holds queued events, not encoded chunk buffers. let pending_offer = tokio::select! { From 0b3e43eeb0a1412d91a792dd15601bedbd3019cc Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 22 Sep 2026 17:27:02 +0200 Subject: [PATCH 05/24] fix(replication): two-stage fresh replication with un-gated paid-list evidence Review follow-up. Sending PaidNotify before the permit wait only helped the head-of-line event: the drainer was one serial loop, so every write queued behind a blocked permit still had its PaidNotify and PaidForList insert delayed by chunk back-pressure. - The fresh-write drainer now never waits for a permit. For every event, at arrival rate, it records PaidForList(self) and sends PaidNotify, then forwards the event to a new offer dispatcher, the only stage that takes a pending-offer permit. - The dispatcher reads the chunk back with `get_raw` (it was content-checked when stored), retries a failed read up to MAX_FRESH_READ_ATTEMPTS times with the permit released in between, and skips only a chunk that is no longer stored. - The offer pipeline is one function (`dispatch_fresh_offer`) shared by the dispatcher and the direct `replicate_fresh` entry point; the 8-argument helper and its clippy allow are gone. - Chunk-carrying protocol fields are encoded as byte strings (`serde_bytes`), which has the same postcard layout as a u8 sequence (unit-tested) but sizes and serializes in one memcpy pass; an oversized body is now refused before anything is allocated. - PaidNotify shares one `Bytes` buffer across its recipients. - A new e2e test drives the PUT pipeline through the real channel with a missing-chunk event queued ahead of a real one; the harness keeps the fresh-write sender so the drainer stays alive in tests. Pins: saorsa-core c5aae116, saorsa-transport 4e0fffc5 (both perf/replication-send-path). Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 5 +- Cargo.toml | 3 + ...ounded-fresh-offers-and-copy-free-sends.md | 26 +-- src/replication/config.rs | 11 ++ src/replication/fresh.rs | 120 ++++++++----- src/replication/mod.rs | 158 +++++++++++++----- src/replication/protocol.rs | 71 +++++++- src/storage/handler.rs | 7 + tests/e2e/replication.rs | 77 +++++++++ tests/e2e/testnet.rs | 9 +- 10 files changed, 382 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12b07181..35561ae4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -862,6 +862,7 @@ dependencies = [ "self_encryption", "semver 1.0.28", "serde", + "serde_bytes", "serde_json", "serial_test", "sha2", @@ -5305,7 +5306,7 @@ dependencies = [ [[package]] name = "saorsa-core" version = "0.28.0" -source = "git+https://github.com/WithAutonomi/saorsa-core?rev=0da3260cd731cf706fd76f2d2589a8725022438b#0da3260cd731cf706fd76f2d2589a8725022438b" +source = "git+https://github.com/WithAutonomi/saorsa-core?rev=c5aae1164927c71f7b06a5efe2c3d687eb3bede2#c5aae1164927c71f7b06a5efe2c3d687eb3bede2" dependencies = [ "anyhow", "async-trait", @@ -5373,7 +5374,7 @@ dependencies = [ [[package]] name = "saorsa-transport" version = "0.37.0" -source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=c2b9f3b0b0d1ed9df1be595058974259254254e6#c2b9f3b0b0d1ed9df1be595058974259254254e6" +source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=4e0fffc5beb6833c71a61326a552ae72bf1aca76#4e0fffc5beb6833c71a61326a552ae72bf1aca76" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 01f2d9d3..84a73314 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,6 +107,9 @@ page_size = "0.6" # Protocol serialization postcard = { version = "1.1.3", features = ["use-std"] } +# Byte-string encoding for chunk payloads in replication messages (same postcard +# wire layout as a u8 sequence, but serialized and sized in one memcpy pass). +serde_bytes = "0.11" bao = "0.13.1" # Shared portable browser profile. The native listener is enabled separately diff --git a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md index 9e1113cd..05cb18ab 100644 --- a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md +++ b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md @@ -67,16 +67,22 @@ up to twice their length in capacity). We will bound the number of encoded fresh offers that can exist at once and make the send path hand a single owned buffer down to the QUIC stream: -- `FreshWriteEvent` carries only the key and the payment proof. The drainer - sends `PaidNotify` to the paid close group as soon as it dequeues a write, - so the paid-list evidence that later repair depends on is never delayed by - chunk back-pressure. It then acquires a `MAX_PENDING_FRESH_OFFERS` (8) - permit before it reads the chunk back from storage and encodes it; the - permit lives with the encoded offer until the last per-peer send drops it. - A backlog therefore waits as small queued events, and at most ~40 MiB of - encoded offers exist per node. Nothing is dropped: the queue is unbounded - and FIFO, and every offer is still dispatched with the same fan-out, - retries and delayed possession check. +- `FreshWriteEvent` carries only the key and the payment proof, and fresh + replication runs as two stages. The fresh-write drainer never waits for + chunk back-pressure: for every event, at arrival rate, it records the key + in `PaidForList(self)` and sends `PaidNotify` to the paid close group — + the evidence later repair depends on — then forwards the event to the + offer dispatcher. The dispatcher is the only permit-gated stage: it + acquires a `MAX_PENDING_FRESH_OFFERS` (8) permit before it reads the + chunk back from storage (`get_raw`; the chunk was content-checked when + stored) and encodes it; the permit lives with the encoded offer until the + last per-peer send drops it. A backlog therefore waits as small queued + events, and at most ~40 MiB of encoded offers exist per node. Nothing is + dropped by back-pressure: both queues are unbounded and FIFO, every offer + is dispatched with the same fan-out, retries and delayed possession check, + and a failed read-back is retried `MAX_FRESH_READ_ATTEMPTS` times with the + permit released in between; only a chunk that is no longer stored is + skipped. - The chunk moves into the offer rather than being copied, and `ReplicationMessage::encode` serializes into an exactly-sized buffer. - The encoded offer is shared as `Bytes`; saorsa-core's `send_message` diff --git a/src/replication/config.rs b/src/replication/config.rs index 8bd8666a..0d6898ba 100644 --- a/src/replication/config.rs +++ b/src/replication/config.rs @@ -181,6 +181,17 @@ pub const MAX_CONCURRENT_REPLICATION_SENDS: usize = 3; /// backlog waits as small queued events instead of chunk-sized buffers. pub const MAX_PENDING_FRESH_OFFERS: usize = 8; +/// How many times the offer dispatcher tries to read an accepted chunk back +/// from storage before giving up on its fresh offers. +/// +/// The chunk was stored moments earlier, so a failed read is a transient +/// fault (exhausted descriptors, an I/O hiccup) far more often than a lost +/// chunk; a lost chunk reports `None` and is skipped without retry. +pub const MAX_FRESH_READ_ATTEMPTS: u32 = 3; + +/// Pause before retrying a failed chunk read-back in the offer dispatcher. +pub const FRESH_READ_RETRY_DELAY: Duration = Duration::from_secs(1); + /// Maximum number of concurrent in-flight audit-responder tasks. /// /// The LIGHT audit-responder handlers — responsible-chunk audits and subtree diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index 9414a98f..1b3b156d 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -2,8 +2,11 @@ //! //! When a node accepts a newly written record with valid `PoP`: //! 1. Store locally (already done by chunk handler). -//! 2. Send fresh offers to `CLOSE_GROUP_SIZE` nearest peers (excluding self). -//! 3. Send `PaidNotify` to all peers in `PaidCloseGroup(K)`. +//! 2. Record the key in `PaidForList(self)` and send `PaidNotify` to every +//! peer in `PaidCloseGroup(K)` — immediately, never behind back-pressure. +//! 3. Send fresh offers to `CLOSE_GROUP_SIZE` nearest peers (excluding self), +//! bounded by the pending-offer permits so a write burst cannot pile up +//! chunk-sized buffers. use std::sync::Arc; @@ -12,13 +15,14 @@ use bytes::Bytes; use rand::Rng; use saorsa_core::identity::PeerId; use saorsa_core::P2PNode; -use tokio::sync::{OwnedSemaphorePermit, Semaphore}; +use tokio::sync::{mpsc, OwnedSemaphorePermit, Semaphore}; use crate::ant_protocol::XorName; use crate::replication::config::{ ReplicationConfig, FRESH_REPLICATION_DELIVERY_MAX_RETRIES, REPLICATION_PROTOCOL_ID, }; use crate::replication::paid_list::PaidList; +use crate::replication::possession::PossessionCheckEvent; use crate::replication::protocol::{ FreshReplicationOffer, PaidNotify, ReplicationMessage, ReplicationMessageBody, }; @@ -28,9 +32,9 @@ use crate::replication::protocol::{ /// Sent from the chunk PUT handler to the replication engine via an /// unbounded channel so that the PUT response is not blocked by /// replication fan-out. The event deliberately carries no chunk bytes: the -/// chunk is already on disk, and the drainer reads it back only once it -/// holds a pending-offer permit, so a replication backlog queues as small -/// events rather than chunk-sized buffers. +/// chunk is already on disk, and the offer dispatcher reads it back only +/// once it holds a pending-offer permit, so a replication backlog queues as +/// small events rather than chunk-sized buffers. pub struct FreshWriteEvent { /// Content-address of the stored chunk. pub key: XorName, @@ -38,6 +42,28 @@ pub struct FreshWriteEvent { pub payment_proof: Vec, } +/// A write whose paid-list evidence has been announced and whose chunk offer +/// is waiting for a pending-offer permit. Carries no chunk bytes. +pub(crate) struct FreshOfferEvent { + pub(crate) key: XorName, + pub(crate) payment_proof: Vec, + /// Storage read-backs attempted so far; see `MAX_FRESH_READ_ATTEMPTS`. + pub(crate) read_attempts: u32, +} + +/// Handles shared by everything that dispatches fresh offers, so the offer +/// dispatcher task and the direct entry point run one pipeline. +#[derive(Clone)] +pub(crate) struct FreshOfferContext { + pub(crate) p2p_node: Arc, + pub(crate) config: Arc, + /// Limits concurrent outbound chunk transfers across the engine. + pub(crate) send_semaphore: Arc, + /// Delayed possession checks (ADR-0003) are scheduled here once an + /// offer's sends are dispatched. + pub(crate) possession_check_tx: mpsc::UnboundedSender, +} + /// An encoded fresh offer shared by the per-peer send tasks. /// /// The pending-offer permit is released together with the buffer, once the @@ -50,46 +76,49 @@ struct EncodedOffer { _pending: OwnedSemaphorePermit, } -/// Execute fresh replication for a newly accepted record. -/// -/// Sends fresh offers to close group members (with bounded delivery retries, -/// ADR-0003). Returns the close-group peers responsible for the key -/// (excluding self) so the caller can schedule the delayed possession check. -/// `PaidNotify` is deliberately not sent here: it carries the paid-list -/// evidence peers need to repair the key later, so callers send it with -/// [`send_paid_notify`] as soon as the write is accepted, before waiting for -/// a pending-offer permit. +/// Rules 6-8: record the paid key locally and announce it to +/// `PaidCloseGroup(K)`. /// -/// The `send_semaphore` limits how many outbound chunk transfers can be -/// in-flight concurrently across the entire replication engine, preventing -/// bandwidth saturation on home broadband connections. `pending_offer` is the -/// caller's permit from the pending-offer semaphore; it is held with the -/// encoded offer until the last per-peer send finishes. `data` is taken by -/// value so the chunk moves into the offer instead of being copied. -#[allow(clippy::too_many_arguments)] -pub async fn replicate_fresh( +/// This is the evidence peers need to repair the key later, so it runs the +/// moment a write is accepted and is never gated by the pending-offer permit +/// or the send semaphore; both messages are small metadata. +pub(crate) async fn announce_paid_write( key: &XorName, - data: Vec, proof_of_payment: &[u8], + paid_list: &PaidList, p2p_node: &Arc, - paid_list: &Arc, config: &ReplicationConfig, - send_semaphore: &Arc, - pending_offer: OwnedSemaphorePermit, -) -> Vec { - let self_id = *p2p_node.peer_id(); - +) { // Rule 6: Node that validates PoP adds K to PaidForList(self). if let Err(e) = paid_list.insert(key).await { warn!("Failed to add key {} to PaidForList: {e}", hex::encode(key)); } + // Rules 7-8: PaidNotify to every member of PaidCloseGroup(K). + send_paid_notify(key, proof_of_payment, p2p_node, config).await; +} - // Rule 2-3: Send fresh offers to CLOSE_GROUP_SIZE nearest peers - // (excluding self). Use self-inclusive query to get the true close group, - // then filter self out. - let closest = p2p_node +/// Rules 2-3: send fresh offers to the close group and schedule the delayed +/// possession check (ADR-0003) for the responsible peers. +/// +/// `pending_offer` is the caller's permit from the pending-offer semaphore; +/// it is held with the encoded offer until the last per-peer send finishes. +/// `data` is taken by value so the chunk moves into the offer instead of +/// being copied. +pub(crate) async fn dispatch_fresh_offer( + ctx: &FreshOfferContext, + key: &XorName, + data: Vec, + proof_of_payment: &[u8], + pending_offer: OwnedSemaphorePermit, +) { + let self_id = *ctx.p2p_node.peer_id(); + + // Use the self-inclusive query to get the true close group, then filter + // self out. + let closest = ctx + .p2p_node .dht_manager() - .find_closest_nodes_local_with_self(key, config.close_group_size) + .find_closest_nodes_local_with_self(key, ctx.config.close_group_size) .await; let target_peers: Vec = closest .iter() @@ -117,7 +146,7 @@ pub async fn replicate_fresh( "Failed to encode FreshReplicationOffer for {}", hex::encode(key), ); - return Vec::new(); + return; }; // One encoded copy serves every per-peer send task and every retry; the // transport borrows it through `Bytes` instead of taking a copy. The @@ -127,10 +156,10 @@ pub async fn replicate_fresh( _pending: pending_offer, }); for peer in &target_peers { - let p2p = Arc::clone(p2p_node); + let p2p = Arc::clone(&ctx.p2p_node); let offer = Arc::clone(&encoded); let peer_id = *peer; - let sem = Arc::clone(send_semaphore); + let sem = Arc::clone(&ctx.send_semaphore); tokio::spawn(async move { // Acquire a permit before sending — this caps the number of // concurrent outbound replication transfers across the engine. @@ -175,15 +204,21 @@ pub async fn replicate_fresh( target_peers.len() ); - target_peers + // Schedule the delayed possession check (ADR-0003) for the responsible + // close-group peers. A closed receiver (engine shutting down) is ignored. + if !target_peers.is_empty() { + let _ = ctx.possession_check_tx.send(PossessionCheckEvent { + key: *key, + peers: target_peers, + }); + } } /// Send `PaidNotify(K)` to every peer in `PaidCloseGroup(K)` (fire-and-forget). /// /// Per Invariant 16: sender MUST attempt delivery to every member. The /// message is small metadata (no chunk data), so it is neither gated by the -/// send semaphore nor by the pending-offer permit: rules 7-8 run as soon as -/// the write is accepted, even when chunk offers are backed up. +/// send semaphore nor by the pending-offer permit. pub(crate) async fn send_paid_notify( key: &XorName, proof_of_payment: &[u8], @@ -210,7 +245,8 @@ pub(crate) async fn send_paid_notify( warn!("Failed to encode PaidNotify for {}", hex::encode(key)); return; }; - + // One buffer for every recipient; the sends only take handles. + let encoded = Bytes::from(encoded); for node in &paid_group { if node.peer_id == self_id { continue; diff --git a/src/replication/mod.rs b/src/replication/mod.rs index f3658bc9..1fd609af 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -79,12 +79,12 @@ use crate::replication::commitment_state::{ PeerCommitmentRecord, PersistedRetention, ResponderCommitmentState, GOSSIP_ANSWERABILITY_TTL, }; use crate::replication::config::{ - max_parallel_fetch, storage_admission_width, ReplicationConfig, MAX_AUDIT_RESPONSES_PER_PEER, - MAX_CONCURRENT_AUDIT_RESPONSES, MAX_CONCURRENT_REPLICATION_SENDS, - MAX_DIGEST_AUDIT_RESPONSES_PER_PEER, MAX_INCOMING_VERIFICATION_KEYS, MAX_PENDING_FRESH_OFFERS, - MAX_SUBTREE_ROUND1_PER_PEER, MAX_SUBTREE_SESSIONS, MAX_VERIFICATION_KEYS_PER_CYCLE, - REPLICATION_PROTOCOL_ID, SUBTREE_AUDIT_PROTOCOL_ID, SUBTREE_ROUND1_WORK_BURST_BYTES, - SUBTREE_ROUND1_WORK_REFILL_BYTES_PER_SEC, SUBTREE_SESSION_TTL, + max_parallel_fetch, storage_admission_width, ReplicationConfig, FRESH_READ_RETRY_DELAY, + MAX_AUDIT_RESPONSES_PER_PEER, MAX_CONCURRENT_AUDIT_RESPONSES, MAX_CONCURRENT_REPLICATION_SENDS, + MAX_DIGEST_AUDIT_RESPONSES_PER_PEER, MAX_FRESH_READ_ATTEMPTS, MAX_INCOMING_VERIFICATION_KEYS, + MAX_PENDING_FRESH_OFFERS, MAX_SUBTREE_ROUND1_PER_PEER, MAX_SUBTREE_SESSIONS, + MAX_VERIFICATION_KEYS_PER_CYCLE, REPLICATION_PROTOCOL_ID, SUBTREE_AUDIT_PROTOCOL_ID, + SUBTREE_ROUND1_WORK_BURST_BYTES, SUBTREE_ROUND1_WORK_REFILL_BYTES_PER_SEC, SUBTREE_SESSION_TTL, }; use crate::replication::paid_list::PaidList; use crate::replication::protocol::{ @@ -1817,13 +1817,19 @@ pub struct ReplicationEngine { subtree_round1: SubtreeRound1Limiter, /// Receiver for fresh-write events from the chunk PUT handler. /// - /// When present, `start()` spawns a drainer task that calls - /// `replicate_fresh` for each event. + /// When present, `start()` spawns the fresh-write drainer, which records + /// paid-list evidence for each event immediately and forwards it to the + /// offer dispatcher. fresh_write_rx: Option>, /// Pointer replication (ADR-0016), when this node stores pointers. pointers: Option>, /// Receiver for fresh pointer writes, taken by `start()`. pointer_fresh_rx: Option>, + /// Hand-off from the fresh-write drainer to the offer dispatcher, the only + /// permit-gated stage. Unbounded and FIFO, holding key + proof only. + fresh_offer_tx: mpsc::UnboundedSender, + /// Receiver paired with `fresh_offer_tx`; taken by the dispatcher task. + fresh_offer_rx: Option>, /// Sender for delayed possession-check events (ADR-0003). The fresh-write /// drainer pushes the responsible close-group peers here after each fresh /// replication; the possession-check scheduler drains the paired receiver. @@ -1887,6 +1893,7 @@ impl ReplicationEngine { let initial_neighbors = NeighborSyncState::new_cycle(Vec::new()); let config = Arc::new(config); let (possession_check_tx, possession_check_rx) = mpsc::unbounded_channel(); + let (fresh_offer_tx, fresh_offer_rx) = mpsc::unbounded_channel(); // ADR-0004: monetized-pin channel (verifier -> first-audit drainer). // Bounded (Amendment 2): every stage of the first-audit pipeline is @@ -1960,6 +1967,8 @@ impl ReplicationEngine { fresh_write_rx: Some(fresh_write_rx), pointers: None, pointer_fresh_rx: None, + fresh_offer_tx, + fresh_offer_rx: Some(fresh_offer_rx), possession_check_tx, possession_check_rx: Some(possession_check_rx), monetized_pin_tx, @@ -2274,6 +2283,7 @@ impl ReplicationEngine { self.start_verification_worker(); self.start_bootstrap_sync(dht_events); self.start_fresh_write_drainer(); + self.start_fresh_offer_dispatcher(); self.start_possession_check_scheduler(); if let Some(pointers) = &self.pointers { self.task_handles.push(pointers.start_verification_loop()); @@ -2418,7 +2428,14 @@ impl ReplicationEngine { /// drainer; this direct entry point schedules here so callers (and tests) /// that drive replication directly still get the possession check. pub async fn replicate_fresh(&self, key: &XorName, data: &[u8], proof_of_payment: &[u8]) { - fresh::send_paid_notify(key, proof_of_payment, &self.p2p_node, &self.config).await; + fresh::announce_paid_write( + key, + proof_of_payment, + &self.paid_list, + &self.p2p_node, + &self.config, + ) + .await; // The semaphore is never closed, so this only fails at shutdown. let Ok(pending_offer) = Arc::clone(&self.pending_offer_semaphore) .acquire_owned() @@ -2426,21 +2443,23 @@ impl ReplicationEngine { else { return; }; - let peers = fresh::replicate_fresh( + fresh::dispatch_fresh_offer( + &self.fresh_offer_context(), key, data.to_vec(), proof_of_payment, - &self.p2p_node, - &self.paid_list, - &self.config, - &self.send_semaphore, pending_offer, ) .await; - if !peers.is_empty() { - let _ = self - .possession_check_tx - .send(possession::PossessionCheckEvent { key: *key, peers }); + } + + /// Handles the offer dispatcher and the direct entry point share. + fn fresh_offer_context(&self) -> fresh::FreshOfferContext { + fresh::FreshOfferContext { + p2p_node: Arc::clone(&self.p2p_node), + config: Arc::clone(&self.config), + send_semaphore: Arc::clone(&self.send_semaphore), + possession_check_tx: self.possession_check_tx.clone(), } } @@ -2456,11 +2475,62 @@ impl ReplicationEngine { }; let p2p = Arc::clone(&self.p2p_node); let paid_list = Arc::clone(&self.paid_list); - let storage = Arc::clone(&self.storage); let config = Arc::clone(&self.config); - let send_semaphore = Arc::clone(&self.send_semaphore); + let offer_tx = self.fresh_offer_tx.clone(); + let shutdown = self.shutdown.clone(); + + let handle = tokio::spawn(async move { + loop { + let event = tokio::select! { + () = shutdown.cancelled() => break, + event = rx.recv() => { + let Some(event) = event else { break }; + event + } + }; + // Stage one never waits for chunk back-pressure: the paid-list + // entry and PaidNotify are what let peers repair the key later, + // so every queued write gets them at arrival rate. + fresh::announce_paid_write( + &event.key, + &event.payment_proof, + &paid_list, + &p2p, + &config, + ) + .await; + if offer_tx + .send(fresh::FreshOfferEvent { + key: event.key, + payment_proof: event.payment_proof, + read_attempts: 0, + }) + .is_err() + { + break; + } + } + debug!("Fresh-write drainer shut down"); + }); + self.task_handles.push(handle); + } + + /// Spawn the fresh-offer dispatcher: the only stage of fresh replication + /// that waits for a pending-offer permit. + /// + /// For each forwarded write it acquires a permit, reads the chunk back + /// from storage and dispatches the offers. A missing chunk is skipped; a + /// failed read is retried up to `MAX_FRESH_READ_ATTEMPTS` times with the + /// permit released in between, so a transient I/O fault does not lose the + /// write's replication. + fn start_fresh_offer_dispatcher(&mut self) { + let Some(mut rx) = self.fresh_offer_rx.take() else { + return; + }; + let storage = Arc::clone(&self.storage); let pending_offer_semaphore = Arc::clone(&self.pending_offer_semaphore); - let possession_tx = self.possession_check_tx.clone(); + let offer_tx = self.fresh_offer_tx.clone(); + let ctx = self.fresh_offer_context(); let shutdown = self.shutdown.clone(); let handle = tokio::spawn(async move { @@ -2472,10 +2542,6 @@ impl ReplicationEngine { event } }; - // Paid-list evidence goes out immediately: it is what lets the - // paid close group repair the key later, so it must never wait - // behind chunk offers. - fresh::send_paid_notify(&event.key, &event.payment_proof, &p2p, &config).await; // Wait for a pending-offer permit before touching the chunk so a // send backlog holds queued events, not encoded chunk buffers. let pending_offer = tokio::select! { @@ -2486,39 +2552,47 @@ impl ReplicationEngine { } }; let key_hex = hex::encode(event.key); - let data = match storage.get(&event.key).await { + // The chunk was content-checked when it was stored; the + // receiver validates the offer against its address anyway. + let data = match storage.get_raw(&event.key).await { Ok(Some(data)) => data, Ok(None) => { debug!("Chunk {key_hex} no longer stored, skipping fresh replication"); continue; } Err(e) => { - warn!("Failed to read chunk {key_hex} for fresh replication: {e}"); + drop(pending_offer); + let attempts = event.read_attempts + 1; + if attempts >= MAX_FRESH_READ_ATTEMPTS { + warn!( + "Giving up fresh replication of {key_hex} after {attempts} failed reads: {e}" + ); + continue; + } + warn!( + "Failed to read chunk {key_hex} for fresh replication (attempt {attempts}): {e}" + ); + tokio::select! { + () = shutdown.cancelled() => break, + () = tokio::time::sleep(FRESH_READ_RETRY_DELAY) => {} + } + let _ = offer_tx.send(fresh::FreshOfferEvent { + read_attempts: attempts, + ..event + }); continue; } }; - let peers = fresh::replicate_fresh( + fresh::dispatch_fresh_offer( + &ctx, &event.key, data, &event.payment_proof, - &p2p, - &paid_list, - &config, - &send_semaphore, pending_offer, ) .await; - // Schedule the delayed possession check (ADR-0003) for - // the responsible close-group peers. A closed receiver - // (engine shutting down) is ignored. - if !peers.is_empty() { - let _ = possession_tx.send(possession::PossessionCheckEvent { - key: event.key, - peers, - }); - } } - debug!("Fresh-write drainer shut down"); + debug!("Fresh-offer dispatcher shut down"); }); self.task_handles.push(handle); } diff --git a/src/replication/protocol.rs b/src/replication/protocol.rs index 0ddfe9c9..d8da6c17 100644 --- a/src/replication/protocol.rs +++ b/src/replication/protocol.rs @@ -52,6 +52,12 @@ impl ReplicationMessage { // message is queued for sending. let size = postcard::experimental::serialized_size(self) .map_err(|e| ReplicationProtocolError::SerializationFailed(e.to_string()))?; + // The size is known before anything is allocated, so an oversized body + // is refused without serializing it first. + let max_size = ceiling_for(family_of_variant(self.body.variant_index())); + if size > max_size { + return Err(ReplicationProtocolError::MessageTooLarge { size, max_size }); + } let bytes = postcard::to_extend(self, Vec::with_capacity(size)) .map_err(|e| ReplicationProtocolError::SerializationFailed(e.to_string()))?; @@ -72,13 +78,6 @@ impl ReplicationMessage { // the largest is a round-1 proof at the commitment // key-count cap, pinned under it with headroom by // `max_round1_proof_fits_the_audit_family_ceiling`. - let max_size = ceiling_for(family_of_variant(self.body.variant_index())); - if bytes.len() > max_size { - return Err(ReplicationProtocolError::MessageTooLarge { - size: bytes.len(), - max_size, - }); - } // V2-623: cumulative per-variant tx accounting. Every replication send // funnels through here, so this is the single tx choke point. @@ -828,9 +827,13 @@ pub(crate) fn log_served_peers_summary() { pub struct FreshReplicationOffer { /// The record key. pub key: XorName, - /// The record data. + /// The record data. Encoded as a byte string, which postcard lays out + /// exactly like a `u8` sequence, so the wire format is unchanged while + /// serialization and sizing copy the payload in one pass. + #[serde(with = "serde_bytes")] pub data: Vec, /// Proof of Payment (required, validated by receiver). + #[serde(with = "serde_bytes")] pub proof_of_payment: Vec, } @@ -860,6 +863,7 @@ pub struct PaidNotify { /// The record key. pub key: XorName, /// Proof of Payment for receiver-side verification. + #[serde(with = "serde_bytes")] pub proof_of_payment: Vec, } @@ -2601,6 +2605,57 @@ mod tests { assert_eq!(decoded.request_id, 7); } + /// `serde_bytes` must not change the wire layout: postcard encodes a byte + /// string and a `u8` sequence identically (varint length + raw bytes). + #[test] + fn byte_string_fields_encode_like_u8_sequences() { + #[derive(Serialize)] + struct PlainOffer { + key: XorName, + data: Vec, + proof_of_payment: Vec, + } + #[derive(Serialize)] + struct PlainNotify { + key: XorName, + proof_of_payment: Vec, + } + let data: Vec = (0..=255u8).cycle().take(70_000).collect(); + let proof = vec![9u8; 300]; + + let offer = FreshReplicationOffer { + key: [5; 32], + data: data.clone(), + proof_of_payment: proof.clone(), + }; + let plain_offer = PlainOffer { + key: [5; 32], + data: data.clone(), + proof_of_payment: proof.clone(), + }; + assert_eq!( + postcard::to_stdvec(&offer).unwrap(), + postcard::to_stdvec(&plain_offer).unwrap() + ); + let decoded: FreshReplicationOffer = + postcard::from_bytes(&postcard::to_stdvec(&plain_offer).unwrap()).unwrap(); + assert_eq!(decoded.data, data); + assert_eq!(decoded.proof_of_payment, proof); + + let notify = PaidNotify { + key: [6; 32], + proof_of_payment: proof.clone(), + }; + let plain_notify = PlainNotify { + key: [6; 32], + proof_of_payment: proof, + }; + assert_eq!( + postcard::to_stdvec(¬ify).unwrap(), + postcard::to_stdvec(&plain_notify).unwrap() + ); + } + #[test] fn encode_allocates_exactly_the_serialized_size() { // A chunk-sized offer must not carry growth slack: the encoded buffer is diff --git a/src/storage/handler.rs b/src/storage/handler.rs index 8474478f..422cd25b 100644 --- a/src/storage/handler.rs +++ b/src/storage/handler.rs @@ -430,6 +430,13 @@ impl AntProtocol { self.fresh_write_tx = Some(tx); } + /// The fresh-write sender, if one was set. Lets tests drive the + /// replication engine's fresh-write pipeline exactly as a PUT would. + #[cfg(any(test, feature = "test-utils"))] + pub fn fresh_write_sender(&self) -> Option> { + self.fresh_write_tx.clone() + } + /// Get the protocol identifier. #[must_use] pub fn protocol_id(&self) -> &'static str { diff --git a/tests/e2e/replication.rs b/tests/e2e/replication.rs index abf5f92b..08cffaad 100644 --- a/tests/e2e/replication.rs +++ b/tests/e2e/replication.rs @@ -13,6 +13,7 @@ use ant_node::replication::commitment_state::{BuiltCommitment, ResponderCommitme use ant_node::replication::config::{ storage_admission_width, K_BUCKET_SIZE, REPLICATION_PROTOCOL_ID, }; +use ant_node::replication::fresh::FreshWriteEvent; use ant_node::replication::protocol::{ compute_audit_digest, AuditChallenge, AuditResponse, FetchRequest, FetchResponse, FreshReplicationOffer, FreshReplicationResponse, NeighborSyncRequest, ReplicationMessage, @@ -256,6 +257,82 @@ async fn test_fresh_replication_propagates_to_close_group() { harness.teardown().await.expect("teardown"); } +/// The PUT-driven pipeline (fresh-write drainer → offer dispatcher) replicates +/// a queued write, and a write whose chunk is no longer stored is skipped +/// without stalling the pipeline or leaking a pending-offer permit. +#[tokio::test] +async fn fresh_write_pipeline_replicates_queued_writes_and_skips_missing_chunks() { + let harness = TestHarness::setup_minimal().await.expect("setup"); + harness.warmup_dht().await.expect("warmup"); + + let source_idx = 3; // first regular node + let source = harness.test_node(source_idx).expect("source node"); + let source_protocol = source.ant_protocol.as_ref().expect("protocol"); + let fresh_tx = source + .fresh_write_tx + .clone() + .expect("fresh-write sender wired by the harness"); + + let content = b"queued write through the fresh-write pipeline"; + let address = compute_address(content); + source_protocol + .storage() + .put(&address, content) + .await + .expect("put"); + for i in 0..harness.node_count() { + if let Some(node) = harness.test_node(i) { + if let Some(protocol) = &node.ant_protocol { + protocol.payment_verifier().cache_insert(address); + } + } + } + + let dummy_pop = vec![0x01u8; 64]; + // A write whose chunk was never stored goes first: the dispatcher must + // skip it and carry on with the next event. + let missing = compute_address(b"never stored anywhere"); + fresh_tx + .send(FreshWriteEvent { + key: missing, + payment_proof: dummy_pop.clone(), + }) + .expect("queue missing write"); + fresh_tx + .send(FreshWriteEvent { + key: address, + payment_proof: dummy_pop, + }) + .expect("queue write"); + + let deadline = tokio::time::Instant::now() + PROPAGATION_TIMEOUT; + let mut found_on_other = false; + while tokio::time::Instant::now() < deadline { + for i in 0..harness.node_count() { + if i == source_idx { + continue; + } + if let Some(node) = harness.test_node(i) { + if let Some(protocol) = &node.ant_protocol { + if protocol.storage().exists(&address).unwrap_or(false) { + found_on_other = true; + } + } + } + } + if found_on_other { + break; + } + tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; + } + assert!( + found_on_other, + "queued write should have replicated through the fresh-write pipeline" + ); + + harness.teardown().await.expect("teardown"); +} + /// ADR-0003: the delayed possession check penalises a responsible peer that /// does NOT hold the chunk, and leaves a peer that DOES hold it unpenalised. /// diff --git a/tests/e2e/testnet.rs b/tests/e2e/testnet.rs index 6e878709..e9cbb5bf 100644 --- a/tests/e2e/testnet.rs +++ b/tests/e2e/testnet.rs @@ -23,6 +23,7 @@ use ant_node::payment::{ QuotingMetricsTracker, }; use ant_node::replication::config::MAX_REPLICATION_MESSAGE_SIZE; +use ant_node::replication::fresh::FreshWriteEvent; use ant_node::storage::{AntProtocol, ChunkStore, ChunkStoreConfig}; use ant_node::{ReplicationConfig, ReplicationEngine}; use bytes::Bytes; @@ -439,6 +440,10 @@ pub struct TestNode { /// Shutdown token for the replication engine. pub replication_shutdown: Option, + + /// Sender feeding the replication engine's fresh-write pipeline, kept so + /// tests can queue writes exactly as the PUT handler does. + pub fresh_write_tx: Option>, } impl TestNode { @@ -1104,6 +1109,7 @@ impl TestNetwork { protocol_task: None, replication_engine: None, replication_shutdown: None, + fresh_write_tx: None, }) } @@ -1356,7 +1362,8 @@ impl TestNetwork { { let shutdown = CancellationToken::new(); let repl_config = self.config.replication_config.clone().unwrap_or_default(); - let (_fresh_tx, fresh_rx) = tokio::sync::mpsc::unbounded_channel(); + let (fresh_tx, fresh_rx) = tokio::sync::mpsc::unbounded_channel(); + node.fresh_write_tx = Some(fresh_tx); let node_identity = Arc::clone(id); match ReplicationEngine::new( repl_config, From bfaef647873bda743979592005784369b0efc96d Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Wed, 23 Sep 2026 09:43:28 +0200 Subject: [PATCH 06/24] test(replication): saturation and read-retry coverage for the fresh-write pipeline Review "patch first" follow-up for the two-stage fresh replication. Two e2e tests over the real harness: - `fresh_write_pipeline_drains_a_burst_larger_than_the_offer_budget` queues 3 x MAX_PENDING_FRESH_OFFERS writes in one go, asserts every one replicates to another node, and asserts all pending-offer permits are back afterwards, so a burst neither loses a write to back-pressure nor leaks a permit. - `fresh_write_pipeline_retries_a_transient_read_failure` makes the chunk file unreadable on disk, waits until the dispatcher's failed read has marked the chunk suspect (observable through `exists`), restores it inside FRESH_READ_RETRY_DELAY, and asserts the write replicates and the suspect mark clears on the source. Adds the test-utils accessor `ReplicationEngine::pending_offer_permits_available` and shares the store/poll helpers with the existing pipeline test. Pins: saorsa-transport 5da61737 (legacy `send` copies only on the QUIC path, `send_bytes` regression tests), saorsa-core b18e3754 (pin bump only). Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 4 +- src/replication/mod.rs | 10 ++ tests/e2e/replication.rs | 318 ++++++++++++++++++++++++++++++++++----- 3 files changed, 290 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35561ae4..c41fcea7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5306,7 +5306,7 @@ dependencies = [ [[package]] name = "saorsa-core" version = "0.28.0" -source = "git+https://github.com/WithAutonomi/saorsa-core?rev=c5aae1164927c71f7b06a5efe2c3d687eb3bede2#c5aae1164927c71f7b06a5efe2c3d687eb3bede2" +source = "git+https://github.com/WithAutonomi/saorsa-core?rev=b18e3754fc4626977079e57c5f2bc449edb52680#b18e3754fc4626977079e57c5f2bc449edb52680" dependencies = [ "anyhow", "async-trait", @@ -5374,7 +5374,7 @@ dependencies = [ [[package]] name = "saorsa-transport" version = "0.37.0" -source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=4e0fffc5beb6833c71a61326a552ae72bf1aca76#4e0fffc5beb6833c71a61326a552ae72bf1aca76" +source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=5da617375d5e3949277b1355c69012c0303f866c#5da617375d5e3949277b1355c69012c0303f866c" dependencies = [ "anyhow", "async-trait", diff --git a/src/replication/mod.rs b/src/replication/mod.rs index 1fd609af..91732a7b 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2257,6 +2257,16 @@ impl ReplicationEngine { self.pointers.as_ref() } + /// Test-only: pending-offer permits not currently held by an encoded + /// fresh offer. Equals [`MAX_PENDING_FRESH_OFFERS`] when no fresh + /// replication is in flight, which is how tests prove a burst of writes + /// drained without leaking a permit. + #[cfg(any(test, feature = "test-utils"))] + #[must_use] + pub fn pending_offer_permits_available(&self) -> usize { + self.pending_offer_semaphore.available_permits() + } + /// Start all background tasks. /// /// `dht_events` must be subscribed **before** `P2PNode::start()` so that diff --git a/tests/e2e/replication.rs b/tests/e2e/replication.rs index 08cffaad..814931bf 100644 --- a/tests/e2e/replication.rs +++ b/tests/e2e/replication.rs @@ -11,7 +11,8 @@ use ant_node::client::compute_address; use ant_node::replication::audit_coordinator::AuditChallengeCoordinator; use ant_node::replication::commitment_state::{BuiltCommitment, ResponderCommitmentState}; use ant_node::replication::config::{ - storage_admission_width, K_BUCKET_SIZE, REPLICATION_PROTOCOL_ID, + storage_admission_width, FRESH_READ_RETRY_DELAY, K_BUCKET_SIZE, MAX_PENDING_FRESH_OFFERS, + REPLICATION_PROTOCOL_ID, }; use ant_node::replication::fresh::FreshWriteEvent; use ant_node::replication::protocol::{ @@ -22,11 +23,17 @@ use ant_node::replication::protocol::{ use ant_node::replication::pruning; use ant_node::replication::scheduling::ReplicationQueues; use ant_node::replication::types::{NeighborSyncState, RepairProofs}; +use ant_node::storage::file_store::CHUNKS_DIR_NAME; +use ant_node::storage::XorName; use ant_node::ReplicationConfig; use saorsa_core::identity::PeerId; use saorsa_core::{P2PNode, TrustEvent}; use serial_test::serial; use std::collections::HashSet; +use std::fs; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; use tokio::sync::RwLock; @@ -49,6 +56,17 @@ const FULL_NODE_SHUN_POSSESSION_DELAY_MAX: Duration = Duration::from_millis(500) const DUMMY_PAYMENT_PROOF_LEN: usize = 64; /// Dummy proof byte used when a test only needs to reach pre-payment gates. const DUMMY_PAYMENT_PROOF_BYTE: u8 = 0x01; +/// First regular (non-bootstrap) node of the minimal harness; source of the +/// fresh-write pipeline tests. +const FRESH_PIPELINE_SOURCE_INDEX: usize = 3; +/// Writes queued at once by the saturation test: three times the pending-offer +/// budget, so the dispatcher must block on and recycle permits to drain it. +const FRESH_BURST_WRITES: usize = 3 * MAX_PENDING_FRESH_OFFERS; +/// Wait budget for the whole burst to replicate and release its permits. +const FRESH_BURST_TIMEOUT: Duration = Duration::from_secs(45); +/// File mode that makes a chunk unreadable, injecting a transient read fault. +#[cfg(unix)] +const UNREADABLE_FILE_MODE: u32 = 0o000; /// Minimal paid-list repair close group used by the deterministic repair e2e. const PAID_REPAIR_GROUP_SIZE: usize = 5; /// Storage threshold configured above majority so one holder is below quorum. @@ -265,74 +283,294 @@ async fn fresh_write_pipeline_replicates_queued_writes_and_skips_missing_chunks( let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); - let source_idx = 3; // first regular node - let source = harness.test_node(source_idx).expect("source node"); - let source_protocol = source.ant_protocol.as_ref().expect("protocol"); + let source = harness + .test_node(FRESH_PIPELINE_SOURCE_INDEX) + .expect("source node"); let fresh_tx = source .fresh_write_tx .clone() .expect("fresh-write sender wired by the harness"); + let address = store_paid_chunk( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + b"queued write through the fresh-write pipeline", + ) + .await; - let content = b"queued write through the fresh-write pipeline"; - let address = compute_address(content); - source_protocol - .storage() - .put(&address, content) - .await - .expect("put"); - for i in 0..harness.node_count() { - if let Some(node) = harness.test_node(i) { - if let Some(protocol) = &node.ant_protocol { - protocol.payment_verifier().cache_insert(address); - } - } - } - - let dummy_pop = vec![0x01u8; 64]; // A write whose chunk was never stored goes first: the dispatcher must // skip it and carry on with the next event. let missing = compute_address(b"never stored anywhere"); fresh_tx .send(FreshWriteEvent { key: missing, - payment_proof: dummy_pop.clone(), + payment_proof: dummy_payment_proof(), }) .expect("queue missing write"); fresh_tx .send(FreshWriteEvent { key: address, - payment_proof: dummy_pop, + payment_proof: dummy_payment_proof(), }) .expect("queue write"); - let deadline = tokio::time::Instant::now() + PROPAGATION_TIMEOUT; - let mut found_on_other = false; - while tokio::time::Instant::now() < deadline { - for i in 0..harness.node_count() { - if i == source_idx { - continue; - } - if let Some(node) = harness.test_node(i) { - if let Some(protocol) = &node.ant_protocol { - if protocol.storage().exists(&address).unwrap_or(false) { - found_on_other = true; - } - } + assert!( + wait_until_replicated( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + &address, + PROPAGATION_TIMEOUT + ) + .await, + "queued write should have replicated through the fresh-write pipeline" + ); + + harness.teardown().await.expect("teardown"); +} + +/// Saturation: a burst of writes three times larger than the pending-offer +/// budget, queued in one go, all replicate. The dispatcher has to block on the +/// `MAX_PENDING_FRESH_OFFERS` semaphore and recycle permits to get through it, +/// and once the burst has drained every permit is back — no write was lost to +/// back-pressure and no permit leaked. +#[tokio::test] +async fn fresh_write_pipeline_drains_a_burst_larger_than_the_offer_budget() { + let harness = TestHarness::setup_minimal().await.expect("setup"); + harness.warmup_dht().await.expect("warmup"); + + let source = harness + .test_node(FRESH_PIPELINE_SOURCE_INDEX) + .expect("source node"); + let engine = source + .replication_engine + .as_ref() + .expect("replication engine"); + let fresh_tx = source + .fresh_write_tx + .clone() + .expect("fresh-write sender wired by the harness"); + + let mut addresses = Vec::with_capacity(FRESH_BURST_WRITES); + for i in 0..FRESH_BURST_WRITES { + let content = format!("fresh-write burst chunk {i}"); + addresses.push( + store_paid_chunk(&harness, FRESH_PIPELINE_SOURCE_INDEX, content.as_bytes()).await, + ); + } + assert_eq!( + engine.pending_offer_permits_available(), + MAX_PENDING_FRESH_OFFERS, + "all permits must be free before the burst" + ); + + for &address in &addresses { + fresh_tx + .send(FreshWriteEvent { + key: address, + payment_proof: dummy_payment_proof(), + }) + .expect("queue write"); + } + + let deadline = tokio::time::Instant::now() + FRESH_BURST_TIMEOUT; + let mut replicated: HashSet = HashSet::new(); + while tokio::time::Instant::now() < deadline && replicated.len() < addresses.len() { + for address in &addresses { + if !replicated.contains(address) + && stored_on_another_node(&harness, FRESH_PIPELINE_SOURCE_INDEX, address) + { + replicated.insert(*address); } } - if found_on_other { - break; - } tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; } + assert_eq!( + replicated.len(), + addresses.len(), + "only {} of {} burst writes replicated within {FRESH_BURST_TIMEOUT:?}", + replicated.len(), + addresses.len() + ); + + // A permit is released when the last per-peer send of its offer finishes, + // which can trail the chunk landing on a peer by a moment. + while tokio::time::Instant::now() < deadline + && engine.pending_offer_permits_available() < MAX_PENDING_FRESH_OFFERS + { + tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; + } + assert_eq!( + engine.pending_offer_permits_available(), + MAX_PENDING_FRESH_OFFERS, + "the burst leaked a pending-offer permit" + ); + + harness.teardown().await.expect("teardown"); +} + +/// Retry: a chunk whose file cannot be read when its offer permit arrives is +/// not dropped. The dispatcher releases the permit, waits +/// `FRESH_READ_RETRY_DELAY` and reads again; once the fault has cleared the +/// write replicates. The fault is injected by making the chunk file +/// unreadable on disk and restored inside the retry window. +#[cfg(unix)] +#[tokio::test] +async fn fresh_write_pipeline_retries_a_transient_read_failure() { + let harness = TestHarness::setup_minimal().await.expect("setup"); + harness.warmup_dht().await.expect("warmup"); + + let source = harness + .test_node(FRESH_PIPELINE_SOURCE_INDEX) + .expect("source node"); + let storage = source.ant_protocol.as_ref().expect("protocol").storage(); + let fresh_tx = source + .fresh_write_tx + .clone() + .expect("fresh-write sender wired by the harness"); + let address = store_paid_chunk( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + b"chunk whose first read-back fails", + ) + .await; + + let chunk_file = chunk_file_path(storage.root_dir(), &address).expect("chunk file on disk"); + let readable = fs::metadata(&chunk_file) + .expect("chunk metadata") + .permissions(); + fs::set_permissions( + &chunk_file, + fs::Permissions::from_mode(UNREADABLE_FILE_MODE), + ) + .expect("make chunk unreadable"); + if fs::File::open(&chunk_file).is_ok() { + // File modes are not enforced for this user (root): the fault cannot + // be injected, so there is nothing to test here. + eprintln!("skipping: file modes are not enforced for this user"); + fs::set_permissions(&chunk_file, readable).expect("restore chunk mode"); + harness.teardown().await.expect("teardown"); + return; + } assert!( - found_on_other, - "queued write should have replicated through the fresh-write pipeline" + storage.exists(&address).unwrap_or(false), + "chunk is indexed before the fault is hit" + ); + + fresh_tx + .send(FreshWriteEvent { + key: address, + payment_proof: dummy_payment_proof(), + }) + .expect("queue write"); + + // A failed read marks the chunk suspect, which hides it from `exists`: + // that is the observable proof the dispatcher's first attempt hit the + // fault. Only then clear it, inside the retry delay. + assert!( + wait_until( + || !storage.exists(&address).unwrap_or(true), + PROPAGATION_TIMEOUT + ) + .await, + "dispatcher never attempted the faulty read" + ); + fs::set_permissions(&chunk_file, readable).expect("restore chunk mode"); + + assert!( + wait_until_replicated( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + &address, + FRESH_READ_RETRY_DELAY + PROPAGATION_TIMEOUT + ) + .await, + "write should have replicated on the retried read" + ); + assert!( + storage.exists(&address).unwrap_or(false), + "the successful retry clears the suspect mark on the source" ); harness.teardown().await.expect("teardown"); } +/// Proof bytes for tests that only need to reach the pre-payment gates. +fn dummy_payment_proof() -> Vec { + vec![DUMMY_PAYMENT_PROOF_BYTE; DUMMY_PAYMENT_PROOF_LEN] +} + +/// Store `content` on `source_idx` and mark it paid on every node, so a fresh +/// offer for it is accepted wherever it lands. Returns the chunk's address. +async fn store_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u8]) -> XorName { + let address = compute_address(content); + harness + .test_node(source_idx) + .expect("source node") + .ant_protocol + .as_ref() + .expect("protocol") + .storage() + .put(&address, content) + .await + .expect("put"); + for i in 0..harness.node_count() { + if let Some(protocol) = harness + .test_node(i) + .and_then(|node| node.ant_protocol.as_ref()) + { + protocol.payment_verifier().cache_insert(address); + } + } + address +} + +/// Whether any node other than `source_idx` currently stores `address`. +fn stored_on_another_node(harness: &TestHarness, source_idx: usize, address: &XorName) -> bool { + (0..harness.node_count()) + .filter(|&i| i != source_idx) + .filter_map(|i| harness.test_node(i)) + .filter_map(|node| node.ant_protocol.as_ref()) + .any(|protocol| protocol.storage().exists(address).unwrap_or(false)) +} + +/// Poll until `address` is stored on a node other than `source_idx`, or +/// `budget` runs out. +async fn wait_until_replicated( + harness: &TestHarness, + source_idx: usize, + address: &XorName, + budget: Duration, +) -> bool { + wait_until( + || stored_on_another_node(harness, source_idx, address), + budget, + ) + .await +} + +/// Poll `condition` every `PROPAGATION_POLL_INTERVAL` until it holds or +/// `budget` runs out. +async fn wait_until(mut condition: impl FnMut() -> bool, budget: Duration) -> bool { + let deadline = tokio::time::Instant::now() + budget; + while tokio::time::Instant::now() < deadline { + if condition() { + return true; + } + tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; + } + false +} + +/// On-disk file of a stored chunk: the store shards `root/chunks/` into +/// subdirectories, so look through them for the file named after the address. +fn chunk_file_path(root_dir: &Path, address: &XorName) -> Option { + let file_name = hex::encode(address); + fs::read_dir(root_dir.join(CHUNKS_DIR_NAME)) + .ok()? + .filter_map(Result::ok) + .map(|shard| shard.path().join(&file_name)) + .find(|candidate| candidate.is_file()) +} + /// ADR-0003: the delayed possession check penalises a responsible peer that /// does NOT hold the chunk, and leaves a peer that DOES hold it unpenalised. /// From dcb9c58edc4bf5ad795097eb9c53d818501230a9 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:48:09 +0200 Subject: [PATCH 07/24] fix(replication): read fresh offers back through the verified store path The offer dispatcher read the chunk back with `get_raw`, which skips the content-address check every other serve path applies. The bytes were checked when the PUT stored them, but under a backlog they come off disk much later, and a chunk that no longer hashes to its key would be offered to the whole close group: every receiver rejects it and charges the sender an application failure, and the store's own view that the chunk is wrong (`known_wrong`) was never consulted. The dispatcher now reads with `ChunkStore::get`, the read the fetch path serves from. A chunk that fails verification is quarantined by that read, so the node stops advertising it and ordinary repair replaces it; its read-back retry finds it gone and skips it. Verification follows the store's `verify_on_read` setting, like every serve. Review follow-up (point 1). Co-Authored-By: Claude Opus 5.5 --- src/replication/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/replication/mod.rs b/src/replication/mod.rs index 91732a7b..c1ed19ca 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2562,9 +2562,13 @@ impl ReplicationEngine { } }; let key_hex = hex::encode(event.key); - // The chunk was content-checked when it was stored; the - // receiver validates the offer against its address anyway. - let data = match storage.get_raw(&event.key).await { + // The same verified read the fetch path serves from. The bytes + // were content-checked when they were stored, but they come + // off disk now, possibly long after, and every receiver + // charges the sender for an offer that does not hash to its + // key. A chunk that fails verification is quarantined by this + // read, so its retry finds it gone and skips it. + let data = match storage.get(&event.key).await { Ok(Some(data)) => data, Ok(None) => { debug!("Chunk {key_hex} no longer stored, skipping fresh replication"); From e5657fb404005a6fed0975116c8a81cd662ace4e Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:48:10 +0200 Subject: [PATCH 08/24] fix(replication): retry a failed read-back without stalling the dispatcher The offer dispatcher slept `FRESH_READ_RETRY_DELAY` inline before re-queueing a write whose chunk could not be read. It is the only dispatcher, so releasing the permit first did not help: nothing was drained while it slept. The expected cause of a failed read, exhausted descriptors, is shared by the reads that follow it, so a fault turned into one delay per queued write, twice over, with every healthy write held behind them. The delay now runs on a tracked task of its own that re-queues the write when it expires (or does nothing at shutdown), and the dispatcher moves straight on to the next event. Review follow-up (point 2). Co-Authored-By: Claude Opus 5.5 --- src/replication/mod.rs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/replication/mod.rs b/src/replication/mod.rs index c1ed19ca..dbb08f65 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2532,7 +2532,8 @@ impl ReplicationEngine { /// from storage and dispatches the offers. A missing chunk is skipped; a /// failed read is retried up to `MAX_FRESH_READ_ATTEMPTS` times with the /// permit released in between, so a transient I/O fault does not lose the - /// write's replication. + /// write's replication. The retry waits on its own task, never on the + /// dispatcher. fn start_fresh_offer_dispatcher(&mut self) { let Some(mut rx) = self.fresh_offer_rx.take() else { return; @@ -2542,6 +2543,7 @@ impl ReplicationEngine { let offer_tx = self.fresh_offer_tx.clone(); let ctx = self.fresh_offer_context(); let shutdown = self.shutdown.clone(); + let retries = self.detached_task_tracker.clone(); let handle = tokio::spawn(async move { loop { @@ -2586,13 +2588,22 @@ impl ReplicationEngine { warn!( "Failed to read chunk {key_hex} for fresh replication (attempt {attempts}): {e}" ); - tokio::select! { - () = shutdown.cancelled() => break, - () = tokio::time::sleep(FRESH_READ_RETRY_DELAY) => {} - } - let _ = offer_tx.send(fresh::FreshOfferEvent { - read_attempts: attempts, - ..event + // Wait on a task of its own: a read fault is often + // shared (exhausted descriptors), and sleeping here + // would hold every healthy write queued behind this + // one for the whole delay, once per failed read. + let retry_tx = offer_tx.clone(); + let retry_shutdown = shutdown.clone(); + retries.spawn(async move { + tokio::select! { + () = retry_shutdown.cancelled() => {} + () = tokio::time::sleep(FRESH_READ_RETRY_DELAY) => { + let _ = retry_tx.send(fresh::FreshOfferEvent { + read_attempts: attempts, + ..event + }); + } + } }); continue; } From 551c6c83a7224d8555a7dcfdb6e4600577152c23 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:48:10 +0200 Subject: [PATCH 09/24] perf(replication): encode fetched chunks and subtree slices as byte strings `FetchResponse::Success::data` (a whole chunk) and `SubtreeSliceItem::Present::bao_slice` were plain `Vec` fields, which serde walks one byte at a time. The size-then-write `encode` compiles the sizing pass down to a length in optimised builds, so it did not make these more expensive, but the per-byte write pass is still the bulk of encoding a fetch response, and decoding one grows the buffer from serde's cautious size hint, leaving a 3 MiB chunk in a 4 MiB allocation. Both are now byte strings (`serde_bytes`), like the fresh-offer fields: postcard lays them out exactly like a `u8` sequence, so the wire is unchanged, and each direction is one copy. For a 4 MiB fetch response, encoding drops from ~1.6 ms to ~0.07 ms and decoding from ~2.6 ms to ~0.05 ms (postcard 1.1, release build). The wire-equivalence test now checks every annotated field against its pre-annotation layout in both directions and on either side of the varint length boundaries, and a new test pins exact-capacity decoding of a fetched chunk. Review follow-up (point 3). Co-Authored-By: Claude Opus 5.5 --- src/replication/protocol.rs | 164 ++++++++++++++++++++++++++++-------- 1 file changed, 127 insertions(+), 37 deletions(-) diff --git a/src/replication/protocol.rs b/src/replication/protocol.rs index d8da6c17..d71811b8 100644 --- a/src/replication/protocol.rs +++ b/src/replication/protocol.rs @@ -967,7 +967,10 @@ pub enum FetchResponse { Success { /// The record key. key: XorName, - /// The record data. + /// The record data. Encoded as a byte string, which postcard lays out + /// exactly like a `u8` sequence: one copy of the chunk each way instead + /// of a per-byte loop, and an exactly-sized buffer on decode. + #[serde(with = "serde_bytes")] data: Vec, }, /// Record not found on this peer. @@ -1360,7 +1363,9 @@ pub enum SubtreeSliceItem { block_index: u32, /// Bao verified slice: the block bytes plus the BLAKE3 parent hashes that /// authenticate them against the chunk address. The auditor decodes this - /// to recover the verified block bytes. + /// to recover the verified block bytes. Encoded as a byte string, laid + /// out exactly like a `u8` sequence on the wire. + #[serde(with = "serde_bytes")] bao_slice: Vec, /// Sibling hashes on the path from this block up to the committed /// `nonced_root`, bottom-up. The auditor folds the block leaf with these @@ -1520,6 +1525,17 @@ impl std::error::Error for ReplicationProtocolError {} mod tests { use super::*; + /// Payload lengths on either side of postcard's one- and two-byte varint + /// length boundaries, plus one past 64 KiB. + const BYTE_STRING_WIRE_LENGTHS: [usize; 6] = [0, 127, 128, 16_383, 16_384, 70_000]; + /// Proof length used alongside each payload in the byte-string wire checks. + const BYTE_STRING_WIRE_PROOF_LEN: usize = 300; + /// Block index carried by the subtree-slice item in the byte-string checks. + const BYTE_STRING_WIRE_BLOCK_INDEX: u32 = 7; + /// A payload that is not a power of two, so a doubling buffer would show + /// slack: three MiB and change. + const CHUNK_SIZED_TEST_PAYLOAD_LEN: usize = 3 * 1024 * 1024 + 123; + // === Audit outcome counters === /// No other test mutates the audit outcome counters, so per-slot deltas are @@ -2605,55 +2621,129 @@ mod tests { assert_eq!(decoded.request_id, 7); } + /// Two types that must be interchangeable on the wire: identical bytes, and + /// each decodes what the other encodes. + fn assert_same_wire(annotated: &A, plain: &P) + where + A: Serialize + serde::de::DeserializeOwned, + P: Serialize + serde::de::DeserializeOwned, + { + let wire = postcard::to_stdvec(annotated).unwrap(); + assert_eq!(wire, postcard::to_stdvec(plain).unwrap()); + let as_plain: P = postcard::from_bytes(&wire).unwrap(); + assert_eq!(postcard::to_stdvec(&as_plain).unwrap(), wire); + let as_annotated: A = postcard::from_bytes(&wire).unwrap(); + assert_eq!(postcard::to_stdvec(&as_annotated).unwrap(), wire); + } + /// `serde_bytes` must not change the wire layout: postcard encodes a byte - /// string and a `u8` sequence identically (varint length + raw bytes). + /// string and a `u8` sequence identically (varint length + raw bytes). Each + /// annotated field is checked against its pre-annotation layout in both + /// directions, on either side of the varint length boundaries. #[test] fn byte_string_fields_encode_like_u8_sequences() { - #[derive(Serialize)] + #[derive(Serialize, Deserialize)] struct PlainOffer { key: XorName, data: Vec, proof_of_payment: Vec, } - #[derive(Serialize)] + #[derive(Serialize, Deserialize)] struct PlainNotify { key: XorName, proof_of_payment: Vec, } - let data: Vec = (0..=255u8).cycle().take(70_000).collect(); - let proof = vec![9u8; 300]; + // Only the first variant is mirrored: postcard tags it 0 in both. + #[derive(Serialize, Deserialize)] + enum PlainFetchResponse { + Success { key: XorName, data: Vec }, + } + #[derive(Serialize, Deserialize)] + enum PlainSliceItem { + Present { + key: XorName, + block_index: u32, + bao_slice: Vec, + nonced_siblings: Vec<[u8; 32]>, + }, + } - let offer = FreshReplicationOffer { - key: [5; 32], - data: data.clone(), - proof_of_payment: proof.clone(), - }; - let plain_offer = PlainOffer { - key: [5; 32], - data: data.clone(), - proof_of_payment: proof.clone(), - }; - assert_eq!( - postcard::to_stdvec(&offer).unwrap(), - postcard::to_stdvec(&plain_offer).unwrap() - ); - let decoded: FreshReplicationOffer = - postcard::from_bytes(&postcard::to_stdvec(&plain_offer).unwrap()).unwrap(); - assert_eq!(decoded.data, data); - assert_eq!(decoded.proof_of_payment, proof); - - let notify = PaidNotify { - key: [6; 32], - proof_of_payment: proof.clone(), + let key = [5; 32]; + let proof = vec![9u8; BYTE_STRING_WIRE_PROOF_LEN]; + let siblings = vec![[1u8; 32]]; + for len in BYTE_STRING_WIRE_LENGTHS { + let data: Vec = (0..=u8::MAX).cycle().take(len).collect(); + assert_same_wire( + &FreshReplicationOffer { + key, + data: data.clone(), + proof_of_payment: proof.clone(), + }, + &PlainOffer { + key, + data: data.clone(), + proof_of_payment: proof.clone(), + }, + ); + assert_same_wire( + &PaidNotify { + key, + proof_of_payment: data.clone(), + }, + &PlainNotify { + key, + proof_of_payment: data.clone(), + }, + ); + assert_same_wire( + &FetchResponse::Success { + key, + data: data.clone(), + }, + &PlainFetchResponse::Success { + key, + data: data.clone(), + }, + ); + assert_same_wire( + &SubtreeSliceItem::Present { + key, + block_index: BYTE_STRING_WIRE_BLOCK_INDEX, + bao_slice: data.clone(), + nonced_siblings: siblings.clone(), + }, + &PlainSliceItem::Present { + key, + block_index: BYTE_STRING_WIRE_BLOCK_INDEX, + bao_slice: data, + nonced_siblings: siblings.clone(), + }, + ); + } + } + + #[test] + fn fetch_response_decodes_into_an_exactly_sized_buffer() { + // A fetched chunk is held by the receiver until it is stored; a + // byte-string field decodes in one copy with no growth slack. + let data = vec![0xCD; CHUNK_SIZED_TEST_PAYLOAD_LEN]; + let msg = ReplicationMessage { + request_id: 11, + body: ReplicationMessageBody::FetchResponse(FetchResponse::Success { + key: [4; 32], + data, + }), }; - let plain_notify = PlainNotify { - key: [6; 32], - proof_of_payment: proof, + let encoded = msg.encode().unwrap(); + assert_eq!(encoded.capacity(), encoded.len()); + let decoded = ReplicationMessage::decode(&encoded).unwrap(); + let ReplicationMessageBody::FetchResponse(FetchResponse::Success { data, .. }) = + decoded.body + else { + panic!("decoded a different body"); }; - assert_eq!( - postcard::to_stdvec(¬ify).unwrap(), - postcard::to_stdvec(&plain_notify).unwrap() - ); + assert_eq!(data.len(), CHUNK_SIZED_TEST_PAYLOAD_LEN); + assert_eq!(data.capacity(), CHUNK_SIZED_TEST_PAYLOAD_LEN); } #[test] @@ -2664,7 +2754,7 @@ mod tests { request_id: 7, body: ReplicationMessageBody::FreshReplicationOffer(FreshReplicationOffer { key: [3; 32], - data: vec![0xAB; 3 * 1024 * 1024 + 123], + data: vec![0xAB; CHUNK_SIZED_TEST_PAYLOAD_LEN], proof_of_payment: vec![1, 2, 3], }), }; From 060ddd2631a25367b6fb14708435f25b47f6f110 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:48:22 +0200 Subject: [PATCH 10/24] test(replication): drive the fresh-write pipeline through the PUT handler and hold the offer budget The pipeline tests did not prove what they claimed (review point 4): - The harness never set the fresh-write sender on the `AntProtocol`, so the handler's emit path was not exercised; the tests posted events by hand. `create_node` now wires the sender into the protocol as a node does and parks the receiver for the engine `start_node` creates, so every PUT through a test node's handler feeds its pipeline. - The burst test wrote tiny chunks and never observed the cap binding, and it was flaky: a one-source burst of small chunks overruns the receivers' per-source fresh-offer admission cap (12), and when all four receivers refused the same key the write never landed (7 failures in 10 isolated runs on the previous head). It now holds the send stage (`ReplicationEngine::hold_replication_sends`), shows the dispatcher encodes exactly `MAX_PENDING_FRESH_OFFERS` offers and stops, then that every write is offered once sends resume and every permit comes back. Offers are counted at the sender (`fresh_offers_dispatched`), so receiver admission cannot make the test flaky. - The missing-chunk test now PUTs through the handler, and checks the skipped write was not offered and its permit came back. - The retry test injects the fault with a directory where the chunk file should be, which the store refuses to read for any user on any platform, so it no longer skips silently as root. It checks the failed read released its permit, and that a healthy write queued behind it is offered within half the retry delay, which a dispatcher sleeping out the delay cannot do. - A new test corrupts a stored chunk on disk and checks it is quarantined and never offered. - Each wait has its own deadline. Co-Authored-By: Claude Opus 5.5 --- src/replication/fresh.rs | 5 + src/replication/mod.rs | 26 +++ tests/e2e/replication.rs | 420 ++++++++++++++++++++++++++------------- tests/e2e/testnet.rs | 23 ++- 4 files changed, 326 insertions(+), 148 deletions(-) diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index 1b3b156d..e4b87ab8 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -8,6 +8,7 @@ //! bounded by the pending-offer permits so a write burst cannot pile up //! chunk-sized buffers. +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use crate::logging::{debug, warn}; @@ -62,6 +63,9 @@ pub(crate) struct FreshOfferContext { /// Delayed possession checks (ADR-0003) are scheduled here once an /// offer's sends are dispatched. pub(crate) possession_check_tx: mpsc::UnboundedSender, + /// Offers encoded and handed to at least one per-peer send, so tests can + /// tell a write that went out from one lost to back-pressure. + pub(crate) dispatched: Arc, } /// An encoded fresh offer shared by the per-peer send tasks. @@ -207,6 +211,7 @@ pub(crate) async fn dispatch_fresh_offer( // Schedule the delayed possession check (ADR-0003) for the responsible // close-group peers. A closed receiver (engine shutting down) is ignored. if !target_peers.is_empty() { + ctx.dispatched.fetch_add(1, Ordering::Relaxed); let _ = ctx.possession_check_tx.send(PossessionCheckEvent { key: *key, peers: target_peers, diff --git a/src/replication/mod.rs b/src/replication/mod.rs index dbb08f65..8daba7c7 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -1751,6 +1751,8 @@ pub struct ReplicationEngine { /// Bounds how many encoded fresh offers can wait behind `send_semaphore`; /// see [`MAX_PENDING_FRESH_OFFERS`]. pending_offer_semaphore: Arc, + /// Fresh offers encoded and handed to their per-peer sends. + fresh_offers_dispatched: Arc, /// Bounds concurrent IN-FLIGHT LIGHT audit-responder tasks (responsible-chunk /// audits + subtree slice round 2). The heavy subtree round 1 has its own /// tighter pool ([`SubtreeRound1Limiter`]). Those are spawned off the serial @@ -1928,6 +1930,7 @@ impl ReplicationEngine { sig_verify_attempts: Arc::new(RwLock::new(HashMap::new())), send_semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_REPLICATION_SENDS)), pending_offer_semaphore: Arc::new(Semaphore::new(MAX_PENDING_FRESH_OFFERS)), + fresh_offers_dispatched: Arc::new(AtomicU64::new(0)), audit_responder_semaphore: Arc::new(Semaphore::new(MAX_CONCURRENT_AUDIT_RESPONSES)), audit_responder_inflight: Arc::new(RwLock::new(HashMap::new())), audit_responder_metrics: Arc::new(AuditResponderMetrics::default()), @@ -2267,6 +2270,28 @@ impl ReplicationEngine { self.pending_offer_semaphore.available_permits() } + /// Test-only: fresh offers encoded and handed to their per-peer sends + /// since the engine was created. A write skipped because its chunk is gone + /// is not counted. + #[cfg(any(test, feature = "test-utils"))] + #[must_use] + pub fn fresh_offers_dispatched(&self) -> u64 { + self.fresh_offers_dispatched.load(Ordering::Relaxed) + } + + /// Test-only: take every outbound replication send permit. Until the + /// returned permit is dropped, encoded fresh offers wait behind the send + /// stage, so a test can fill the pending-offer budget deterministically. + /// `None` only if the engine is shutting down. + #[cfg(any(test, feature = "test-utils"))] + pub async fn hold_replication_sends(&self) -> Option { + let all = u32::try_from(MAX_CONCURRENT_REPLICATION_SENDS).ok()?; + Arc::clone(&self.send_semaphore) + .acquire_many_owned(all) + .await + .ok() + } + /// Start all background tasks. /// /// `dht_events` must be subscribed **before** `P2PNode::start()` so that @@ -2470,6 +2495,7 @@ impl ReplicationEngine { config: Arc::clone(&self.config), send_semaphore: Arc::clone(&self.send_semaphore), possession_check_tx: self.possession_check_tx.clone(), + dispatched: Arc::clone(&self.fresh_offers_dispatched), } } diff --git a/tests/e2e/replication.rs b/tests/e2e/replication.rs index 814931bf..ed39a97c 100644 --- a/tests/e2e/replication.rs +++ b/tests/e2e/replication.rs @@ -7,12 +7,13 @@ use super::testnet::TestNetworkConfig; use super::TestHarness; +use ant_node::ant_protocol::{ChunkMessage, ChunkMessageBody, ChunkPutRequest, ChunkPutResponse}; use ant_node::client::compute_address; use ant_node::replication::audit_coordinator::AuditChallengeCoordinator; use ant_node::replication::commitment_state::{BuiltCommitment, ResponderCommitmentState}; use ant_node::replication::config::{ - storage_admission_width, FRESH_READ_RETRY_DELAY, K_BUCKET_SIZE, MAX_PENDING_FRESH_OFFERS, - REPLICATION_PROTOCOL_ID, + storage_admission_width, FRESH_READ_RETRY_DELAY, K_BUCKET_SIZE, MAX_FRESH_READ_ATTEMPTS, + MAX_PENDING_FRESH_OFFERS, REPLICATION_PROTOCOL_ID, }; use ant_node::replication::fresh::FreshWriteEvent; use ant_node::replication::protocol::{ @@ -26,13 +27,12 @@ use ant_node::replication::types::{NeighborSyncState, RepairProofs}; use ant_node::storage::file_store::CHUNKS_DIR_NAME; use ant_node::storage::XorName; use ant_node::ReplicationConfig; +use bytes::Bytes; use saorsa_core::identity::PeerId; use saorsa_core::{P2PNode, TrustEvent}; use serial_test::serial; use std::collections::HashSet; use std::fs; -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -62,11 +62,20 @@ const FRESH_PIPELINE_SOURCE_INDEX: usize = 3; /// Writes queued at once by the saturation test: three times the pending-offer /// budget, so the dispatcher must block on and recycle permits to drain it. const FRESH_BURST_WRITES: usize = 3 * MAX_PENDING_FRESH_OFFERS; -/// Wait budget for the whole burst to replicate and release its permits. -const FRESH_BURST_TIMEOUT: Duration = Duration::from_secs(45); -/// File mode that makes a chunk unreadable, injecting a transient read fault. -#[cfg(unix)] -const UNREADABLE_FILE_MODE: u32 = 0o000; +/// How long the saturation test watches a full budget for offers encoded past +/// it. The dispatcher encodes a small chunk in well under a millisecond. +const FRESH_BURST_SETTLE: Duration = Duration::from_millis(500); +/// Wait budget for every burst write to be offered once sends resume. +const FRESH_BURST_DRAIN_TIMEOUT: Duration = Duration::from_secs(30); +/// Wait budget for pending-offer permits to come back once the offers holding +/// them have finished sending. +const PERMIT_RELEASE_TIMEOUT: Duration = Duration::from_secs(10); +/// Poll interval for timing the dispatcher against the retry delay: fine +/// enough that the poll cannot hide a stall of that length. +const DISPATCH_POLL_INTERVAL: Duration = Duration::from_millis(10); +/// Extension a chunk file is moved aside under while a directory stands in +/// for it, injecting a read fault. +const FAULT_SET_ASIDE_EXTENSION: &str = "set-aside"; /// Minimal paid-list repair close group used by the deterministic repair e2e. const PAID_REPAIR_GROUP_SIZE: usize = 5; /// Storage threshold configured above majority so one holder is below quorum. @@ -275,30 +284,27 @@ async fn test_fresh_replication_propagates_to_close_group() { harness.teardown().await.expect("teardown"); } -/// The PUT-driven pipeline (fresh-write drainer → offer dispatcher) replicates -/// a queued write, and a write whose chunk is no longer stored is skipped -/// without stalling the pipeline or leaking a pending-offer permit. +/// The whole PUT path: a chunk PUT through the handler emits the fresh-write +/// event itself, and the drainer → dispatcher pipeline replicates it. A write +/// whose chunk is no longer stored, queued ahead of it, is skipped without +/// being offered, without stalling the pipeline and without keeping its +/// pending-offer permit. #[tokio::test] -async fn fresh_write_pipeline_replicates_queued_writes_and_skips_missing_chunks() { +async fn fresh_write_pipeline_replicates_a_put_and_skips_missing_chunks() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); let source = harness .test_node(FRESH_PIPELINE_SOURCE_INDEX) .expect("source node"); - let fresh_tx = source - .fresh_write_tx - .clone() - .expect("fresh-write sender wired by the harness"); - let address = store_paid_chunk( - &harness, - FRESH_PIPELINE_SOURCE_INDEX, - b"queued write through the fresh-write pipeline", - ) - .await; + let engine = source + .replication_engine + .as_ref() + .expect("replication engine"); + let fresh_tx = fresh_write_sender(&harness, FRESH_PIPELINE_SOURCE_INDEX); // A write whose chunk was never stored goes first: the dispatcher must - // skip it and carry on with the next event. + // skip it and carry on with the PUT queued behind it. let missing = compute_address(b"never stored anywhere"); fresh_tx .send(FreshWriteEvent { @@ -306,12 +312,12 @@ async fn fresh_write_pipeline_replicates_queued_writes_and_skips_missing_chunks( payment_proof: dummy_payment_proof(), }) .expect("queue missing write"); - fresh_tx - .send(FreshWriteEvent { - key: address, - payment_proof: dummy_payment_proof(), - }) - .expect("queue write"); + let address = put_paid_chunk( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + b"chunk PUT through the handler", + ) + .await; assert!( wait_until_replicated( @@ -321,19 +327,32 @@ async fn fresh_write_pipeline_replicates_queued_writes_and_skips_missing_chunks( PROPAGATION_TIMEOUT ) .await, - "queued write should have replicated through the fresh-write pipeline" + "the PUT should have replicated through the fresh-write pipeline" + ); + assert!( + wait_until( + || engine.pending_offer_permits_available() == MAX_PENDING_FRESH_OFFERS, + PERMIT_RELEASE_TIMEOUT + ) + .await, + "a pending-offer permit was not released" + ); + assert_eq!( + engine.fresh_offers_dispatched(), + 1, + "only the stored chunk may be offered; the missing one is skipped" ); harness.teardown().await.expect("teardown"); } -/// Saturation: a burst of writes three times larger than the pending-offer -/// budget, queued in one go, all replicate. The dispatcher has to block on the -/// `MAX_PENDING_FRESH_OFFERS` semaphore and recycle permits to get through it, -/// and once the burst has drained every permit is back — no write was lost to -/// back-pressure and no permit leaked. +/// Saturation: with the send stage held, a burst three times the pending-offer +/// budget encodes exactly `MAX_PENDING_FRESH_OFFERS` offers and the dispatcher +/// then waits for a permit instead of encoding more. Once sends resume, every +/// write is offered — none is lost to back-pressure — and every permit comes +/// back. #[tokio::test] -async fn fresh_write_pipeline_drains_a_burst_larger_than_the_offer_budget() { +async fn fresh_write_pipeline_holds_a_burst_at_the_offer_budget() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); @@ -344,150 +363,214 @@ async fn fresh_write_pipeline_drains_a_burst_larger_than_the_offer_budget() { .replication_engine .as_ref() .expect("replication engine"); - let fresh_tx = source - .fresh_write_tx - .clone() - .expect("fresh-write sender wired by the harness"); + let budget = u64::try_from(MAX_PENDING_FRESH_OFFERS).expect("budget fits u64"); + let burst = u64::try_from(FRESH_BURST_WRITES).expect("burst fits u64"); - let mut addresses = Vec::with_capacity(FRESH_BURST_WRITES); + let held_sends = engine + .hold_replication_sends() + .await + .expect("replication send permits"); for i in 0..FRESH_BURST_WRITES { let content = format!("fresh-write burst chunk {i}"); - addresses.push( - store_paid_chunk(&harness, FRESH_PIPELINE_SOURCE_INDEX, content.as_bytes()).await, - ); + put_paid_chunk(&harness, FRESH_PIPELINE_SOURCE_INDEX, content.as_bytes()).await; } - assert_eq!( - engine.pending_offer_permits_available(), - MAX_PENDING_FRESH_OFFERS, - "all permits must be free before the burst" - ); - for &address in &addresses { - fresh_tx - .send(FreshWriteEvent { - key: address, - payment_proof: dummy_payment_proof(), - }) - .expect("queue write"); - } - - let deadline = tokio::time::Instant::now() + FRESH_BURST_TIMEOUT; - let mut replicated: HashSet = HashSet::new(); - while tokio::time::Instant::now() < deadline && replicated.len() < addresses.len() { - for address in &addresses { - if !replicated.contains(address) - && stored_on_another_node(&harness, FRESH_PIPELINE_SOURCE_INDEX, address) - { - replicated.insert(*address); - } - } - tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; - } + assert!( + wait_until( + || engine.fresh_offers_dispatched() >= budget, + PROPAGATION_TIMEOUT + ) + .await, + "the burst never filled the pending-offer budget ({} offers encoded)", + engine.fresh_offers_dispatched() + ); + // Give a dispatcher that ignored the budget time to overshoot it. + tokio::time::sleep(FRESH_BURST_SETTLE).await; assert_eq!( - replicated.len(), - addresses.len(), - "only {} of {} burst writes replicated within {FRESH_BURST_TIMEOUT:?}", - replicated.len(), - addresses.len() + engine.fresh_offers_dispatched(), + budget, + "offers were encoded past the pending-offer budget" ); + assert_eq!(engine.pending_offer_permits_available(), 0); - // A permit is released when the last per-peer send of its offer finishes, - // which can trail the chunk landing on a peer by a moment. - while tokio::time::Instant::now() < deadline - && engine.pending_offer_permits_available() < MAX_PENDING_FRESH_OFFERS - { - tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; - } - assert_eq!( - engine.pending_offer_permits_available(), - MAX_PENDING_FRESH_OFFERS, + drop(held_sends); + assert!( + wait_until( + || engine.fresh_offers_dispatched() == burst, + FRESH_BURST_DRAIN_TIMEOUT + ) + .await, + "only {} of {burst} burst writes were offered", + engine.fresh_offers_dispatched() + ); + assert!( + wait_until( + || engine.pending_offer_permits_available() == MAX_PENDING_FRESH_OFFERS, + PERMIT_RELEASE_TIMEOUT + ) + .await, "the burst leaked a pending-offer permit" ); harness.teardown().await.expect("teardown"); } -/// Retry: a chunk whose file cannot be read when its offer permit arrives is -/// not dropped. The dispatcher releases the permit, waits -/// `FRESH_READ_RETRY_DELAY` and reads again; once the fault has cleared the -/// write replicates. The fault is injected by making the chunk file -/// unreadable on disk and restored inside the retry window. -#[cfg(unix)] +/// Retry: a chunk whose read-back fails is retried after +/// `FRESH_READ_RETRY_DELAY` without holding its permit or the dispatcher. A +/// healthy write queued behind it is offered well inside the delay, and once +/// the fault clears the failed write is offered too. The fault is a directory +/// standing where the chunk file should be, which the store refuses to read +/// whatever the platform or user. #[tokio::test] -async fn fresh_write_pipeline_retries_a_transient_read_failure() { +async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); let source = harness .test_node(FRESH_PIPELINE_SOURCE_INDEX) .expect("source node"); + let engine = source + .replication_engine + .as_ref() + .expect("replication engine"); let storage = source.ant_protocol.as_ref().expect("protocol").storage(); - let fresh_tx = source - .fresh_write_tx - .clone() - .expect("fresh-write sender wired by the harness"); - let address = store_paid_chunk( + let fresh_tx = fresh_write_sender(&harness, FRESH_PIPELINE_SOURCE_INDEX); + let faulty = store_paid_chunk( &harness, FRESH_PIPELINE_SOURCE_INDEX, b"chunk whose first read-back fails", ) .await; - let chunk_file = chunk_file_path(storage.root_dir(), &address).expect("chunk file on disk"); - let readable = fs::metadata(&chunk_file) - .expect("chunk metadata") - .permissions(); - fs::set_permissions( - &chunk_file, - fs::Permissions::from_mode(UNREADABLE_FILE_MODE), - ) - .expect("make chunk unreadable"); - if fs::File::open(&chunk_file).is_ok() { - // File modes are not enforced for this user (root): the fault cannot - // be injected, so there is nothing to test here. - eprintln!("skipping: file modes are not enforced for this user"); - fs::set_permissions(&chunk_file, readable).expect("restore chunk mode"); - harness.teardown().await.expect("teardown"); - return; - } - assert!( - storage.exists(&address).unwrap_or(false), - "chunk is indexed before the fault is hit" - ); + let chunk_file = chunk_file_path(storage.root_dir(), &faulty).expect("chunk file on disk"); + let set_aside = chunk_file.with_extension(FAULT_SET_ASIDE_EXTENSION); + fs::rename(&chunk_file, &set_aside).expect("move the chunk file aside"); + fs::create_dir(&chunk_file).expect("put a directory in its place"); fresh_tx .send(FreshWriteEvent { - key: address, + key: faulty, payment_proof: dummy_payment_proof(), }) .expect("queue write"); - // A failed read marks the chunk suspect, which hides it from `exists`: - // that is the observable proof the dispatcher's first attempt hit the - // fault. Only then clear it, inside the retry delay. + // the observable proof that the dispatcher's first attempt hit the fault. assert!( wait_until( - || !storage.exists(&address).unwrap_or(true), + || !storage.exists(&faulty).unwrap_or(true), PROPAGATION_TIMEOUT ) .await, - "dispatcher never attempted the faulty read" + "the dispatcher never attempted the faulty read" + ); + assert!( + wait_until( + || engine.pending_offer_permits_available() == MAX_PENDING_FRESH_OFFERS, + PERMIT_RELEASE_TIMEOUT + ) + .await, + "the failed read kept its pending-offer permit while waiting to retry" ); - fs::set_permissions(&chunk_file, readable).expect("restore chunk mode"); + // A dispatcher sleeping out the retry delay would hold this write until + // the delay ended; one that is not offers it straight away. + put_paid_chunk( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + b"healthy write queued behind a failed read", + ) + .await; + let offered_in_time = tokio::time::timeout(FRESH_READ_RETRY_DELAY / 2, async { + while engine.fresh_offers_dispatched() == 0 { + tokio::time::sleep(DISPATCH_POLL_INTERVAL).await; + } + }) + .await + .is_ok(); + assert!( + offered_in_time, + "a healthy write waited behind the failed read's retry delay" + ); + + fs::remove_dir(&chunk_file).expect("remove the directory"); + fs::rename(&set_aside, &chunk_file).expect("restore the chunk file"); + assert!( + wait_until( + || engine.fresh_offers_dispatched() == 2, + FRESH_READ_RETRY_DELAY * MAX_FRESH_READ_ATTEMPTS + PROPAGATION_TIMEOUT + ) + .await, + "the failed write was not offered once its read succeeded" + ); + assert!( + storage.exists(&faulty).unwrap_or(false), + "the successful retry clears the suspect mark on the source" + ); assert!( wait_until_replicated( &harness, FRESH_PIPELINE_SOURCE_INDEX, - &address, - FRESH_READ_RETRY_DELAY + PROPAGATION_TIMEOUT + &faulty, + PROPAGATION_TIMEOUT ) .await, - "write should have replicated on the retried read" + "the retried write should have replicated" ); + + harness.teardown().await.expect("teardown"); +} + +/// A chunk whose bytes rotted on disk after it was stored is never offered: +/// every receiver would reject it and charge the sender. The read-back +/// verifies the chunk, quarantines it on the mismatch, and the retry finds it +/// gone and skips it. +#[tokio::test] +async fn fresh_write_pipeline_never_offers_a_corrupt_chunk() { + let harness = TestHarness::setup_minimal().await.expect("setup"); + harness.warmup_dht().await.expect("warmup"); + + let source = harness + .test_node(FRESH_PIPELINE_SOURCE_INDEX) + .expect("source node"); + let engine = source + .replication_engine + .as_ref() + .expect("replication engine"); + let storage = source.ant_protocol.as_ref().expect("protocol").storage(); + let fresh_tx = fresh_write_sender(&harness, FRESH_PIPELINE_SOURCE_INDEX); + let content = b"chunk that rots on disk before its offer"; + let address = store_paid_chunk(&harness, FRESH_PIPELINE_SOURCE_INDEX, content).await; + + let chunk_file = chunk_file_path(storage.root_dir(), &address).expect("chunk file on disk"); + let mut rotted = content.to_vec(); + rotted.reverse(); + fs::write(&chunk_file, &rotted).expect("corrupt the chunk file"); + + fresh_tx + .send(FreshWriteEvent { + key: address, + payment_proof: dummy_payment_proof(), + }) + .expect("queue write"); assert!( - storage.exists(&address).unwrap_or(false), - "the successful retry clears the suspect mark on the source" + wait_until( + || chunk_file_path(storage.root_dir(), &address).is_none(), + PROPAGATION_TIMEOUT + ) + .await, + "the read-back never quarantined the corrupt chunk" + ); + // Long enough for the retry to have run and skipped the vanished chunk. + tokio::time::sleep(FRESH_READ_RETRY_DELAY + FRESH_BURST_SETTLE).await; + assert_eq!( + engine.fresh_offers_dispatched(), + 0, + "a chunk that does not match its address was offered" + ); + assert!(!storage.exists(&address).unwrap_or(true)); + assert_eq!( + engine.pending_offer_permits_available(), + MAX_PENDING_FRESH_OFFERS ); harness.teardown().await.expect("teardown"); @@ -498,8 +581,21 @@ fn dummy_payment_proof() -> Vec { vec![DUMMY_PAYMENT_PROOF_BYTE; DUMMY_PAYMENT_PROOF_LEN] } -/// Store `content` on `source_idx` and mark it paid on every node, so a fresh -/// offer for it is accepted wherever it lands. Returns the chunk's address. +/// Pre-populate the payment cache on every node, so the source's handler and +/// the receivers of its offers accept a dummy proof for `address`. +fn cache_payment_everywhere(harness: &TestHarness, address: &XorName) { + for i in 0..harness.node_count() { + if let Some(protocol) = harness + .test_node(i) + .and_then(|node| node.ant_protocol.as_ref()) + { + protocol.payment_verifier().cache_insert(*address); + } + } +} + +/// Store a chunk on the source directly, bypassing the handler, so no +/// fresh-write event is emitted for it. async fn store_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u8]) -> XorName { let address = compute_address(content); harness @@ -512,17 +608,59 @@ async fn store_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u .put(&address, content) .await .expect("put"); - for i in 0..harness.node_count() { - if let Some(protocol) = harness - .test_node(i) - .and_then(|node| node.ant_protocol.as_ref()) - { - protocol.payment_verifier().cache_insert(address); - } - } + cache_payment_everywhere(harness, &address); address } +/// PUT a chunk through the source node's handler, as a client would. The +/// handler stores it and emits the fresh-write event itself, carrying the +/// dummy proof it was paid with. +async fn put_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u8]) -> XorName { + let address = compute_address(content); + cache_payment_everywhere(harness, &address); + let request = ChunkMessage { + request_id: rand::random(), + body: ChunkMessageBody::PutRequest(ChunkPutRequest::with_payment( + address, + Bytes::copy_from_slice(content), + dummy_payment_proof(), + )), + }; + let response = harness + .test_node(source_idx) + .expect("source node") + .ant_protocol + .as_ref() + .expect("protocol") + .try_handle_request(&request.encode().expect("encode PUT")) + .await + .expect("handle PUT") + .expect("PUT response"); + match ChunkMessage::decode(&response) + .expect("decode PUT response") + .body + { + ChunkMessageBody::PutResponse(ChunkPutResponse::Success { .. }) => address, + other => panic!("PUT through the handler failed: {other:?}"), + } +} + +/// The sender the source's PUT handler feeds its fresh-write pipeline from, +/// for tests that queue a write the handler would not emit. +fn fresh_write_sender( + harness: &TestHarness, + source_idx: usize, +) -> tokio::sync::mpsc::UnboundedSender { + harness + .test_node(source_idx) + .expect("source node") + .ant_protocol + .as_ref() + .expect("protocol") + .fresh_write_sender() + .expect("fresh-write sender wired by the harness") +} + /// Whether any node other than `source_idx` currently stores `address`. fn stored_on_another_node(harness: &TestHarness, source_idx: usize, address: &XorName) -> bool { (0..harness.node_count()) diff --git a/tests/e2e/testnet.rs b/tests/e2e/testnet.rs index e9cbb5bf..073029db 100644 --- a/tests/e2e/testnet.rs +++ b/tests/e2e/testnet.rs @@ -441,9 +441,10 @@ pub struct TestNode { /// Shutdown token for the replication engine. pub replication_shutdown: Option, - /// Sender feeding the replication engine's fresh-write pipeline, kept so - /// tests can queue writes exactly as the PUT handler does. - pub fresh_write_tx: Option>, + /// Fresh-write events from this node's PUT handler, waiting for the + /// replication engine that `start_node` creates to take them. The sender + /// half lives in `ant_protocol`, as it does in a real node. + pub fresh_write_rx: Option>, } impl TestNode { @@ -1087,13 +1088,17 @@ impl TestNetwork { .get(&index) .copied() .unwrap_or_default(); - let ant_protocol = Self::create_ant_protocol_with_disk_reserve( + let mut ant_protocol = Self::create_ant_protocol_with_disk_reserve( &data_dir, self.config.evm_network.clone(), storage_disk_reserve, &identity, ) .await?; + // Wired as a real node wires it, so a PUT through the handler feeds + // the replication engine's fresh-write pipeline. + let (fresh_write_tx, fresh_write_rx) = tokio::sync::mpsc::unbounded_channel(); + ant_protocol.set_fresh_write_sender(fresh_write_tx); Ok(TestNode { index, @@ -1109,7 +1114,7 @@ impl TestNetwork { protocol_task: None, replication_engine: None, replication_shutdown: None, - fresh_write_tx: None, + fresh_write_rx: Some(fresh_write_rx), }) } @@ -1362,8 +1367,12 @@ impl TestNetwork { { let shutdown = CancellationToken::new(); let repl_config = self.config.replication_config.clone().unwrap_or_default(); - let (fresh_tx, fresh_rx) = tokio::sync::mpsc::unbounded_channel(); - node.fresh_write_tx = Some(fresh_tx); + // `create_node` fills this and each node is started once, so it + // is always there; a closed channel would only idle the drainer. + let fresh_rx = node + .fresh_write_rx + .take() + .unwrap_or_else(|| tokio::sync::mpsc::unbounded_channel().1); let node_identity = Arc::clone(id); match ReplicationEngine::new( repl_config, From 7ad7bef85bf0905c3f6c0cd567ad42e536775aa6 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:48:22 +0200 Subject: [PATCH 11/24] docs(adr-0017): verified read-back, retries off the dispatcher, and what is bounded - The read-back is the verified serve path, and why. - Read-back retries wait off the dispatcher; what happens after the last one, and what brings an unadvertised key back. - The fetch-response and subtree-slice payloads are byte strings too. - Soften "bounded": chunk buffers are bounded, the queues ahead of them grow by one stripped proof per write under sustained overload. - The pending-offer budget bounds the sender, not receiver admission. - Validation lists the new tests. Review follow-up. Co-Authored-By: Claude Opus 5.5 --- ...ounded-fresh-offers-and-copy-free-sends.md | 92 ++++++++++++++----- 1 file changed, 71 insertions(+), 21 deletions(-) diff --git a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md index 05cb18ab..5e52bdb9 100644 --- a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md +++ b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md @@ -44,8 +44,9 @@ up to twice their length in capacity). ## Decision Drivers -- Node memory must stay bounded under any client write rate; replication - may be delayed by backpressure but must not be dropped. +- The chunk-sized buffers held for replication must stay bounded under any + client write rate, and what queues ahead of them must be small; + replication may be delayed by backpressure but must not be dropped by it. - The change must not alter the wire format, storage format or payment logic, so it can ship as a behavioural fix. - Existing callers of the send APIs in saorsa-core and saorsa-transport must @@ -74,17 +75,33 @@ make the send path hand a single owned buffer down to the QUIC stream: the evidence later repair depends on — then forwards the event to the offer dispatcher. The dispatcher is the only permit-gated stage: it acquires a `MAX_PENDING_FRESH_OFFERS` (8) permit before it reads the - chunk back from storage (`get_raw`; the chunk was content-checked when - stored) and encodes it; the permit lives with the encoded offer until the - last per-peer send drops it. A backlog therefore waits as small queued - events, and at most ~40 MiB of encoded offers exist per node. Nothing is - dropped by back-pressure: both queues are unbounded and FIFO, every offer - is dispatched with the same fan-out, retries and delayed possession check, - and a failed read-back is retried `MAX_FRESH_READ_ATTEMPTS` times with the - permit released in between; only a chunk that is no longer stored is - skipped. + chunk back from storage and encodes it; the permit lives with the encoded + offer until the last per-peer send drops it. A backlog therefore waits as + small queued events, and at most ~40 MiB of encoded offers exist per node. + Nothing is dropped by back-pressure: both queues are unbounded and FIFO, + and every offer is dispatched with the same fan-out, retries and delayed + possession check. +- The read-back is the same verified read the fetch path serves from + (`ChunkStore::get`), not a raw one. The bytes were content-checked when + they were stored, but they now come off disk, possibly long after, and + every receiver rejects an offer that does not hash to its key and charges + the sender for it. A chunk that fails verification is quarantined by that + read, as on any serve, so the node stops advertising it and ordinary + repair replaces it; it is never offered. Like every serve, verification + follows the store's `verify_on_read` setting (on by default). +- A failed read-back is retried up to `MAX_FRESH_READ_ATTEMPTS` times, + `FRESH_READ_RETRY_DELAY` apart, with the permit released in between. The + delay runs on a task of its own, never on the dispatcher: read faults tend + to be shared (exhausted descriptors), and a dispatcher that slept them out + would hold every healthy write queued behind them. Only a chunk that is no + longer stored is skipped without retry. - The chunk moves into the offer rather than being copied, and - `ReplicationMessage::encode` serializes into an exactly-sized buffer. + `ReplicationMessage::encode` serializes into an exactly-sized buffer. The + chunk-carrying fields — the offer's data and proof, `PaidNotify`'s proof, + `FetchResponse::Success::data` and a subtree slice's `bao_slice` — are + byte strings (`serde_bytes`), which postcard lays out exactly like a `u8` + sequence: one copy each way instead of a per-byte loop, and an + exactly-sized buffer on decode. - The encoded offer is shared as `Bytes`; saorsa-core's `send_message` accepts `impl Into`, frames the payload through a borrowing `WireMessageRef` (byte-identical to `WireMessage` on the wire) into an @@ -96,13 +113,14 @@ make the send path hand a single owned buffer down to the QUIC stream: ### Positive -- Memory under write load is bounded by configuration: pending offers plus - the three in-flight sends, each held once, instead of growing with the - backlog. On the diagnostic fleets peak live memory fell from 1068 MiB to +- Chunk memory under write load is bounded by configuration: pending offers + plus the three in-flight sends, each held once, instead of growing with + the backlog. On the diagnostic fleets peak live memory fell from 1068 MiB to 298 MiB (mimalloc build) and from 674 MiB to 262 MiB (jemalloc build) after the backpressure change alone. - Every large send node-wide (chunk GET responses included) stops paying - for a second copy of its frame during the transfer. + for a second copy of its frame during the transfer, and a fetched chunk is + encoded and decoded in one copy each. - No wire, storage or API break: `send(&[u8])` remains and copies once as before; `Vec` callers of `send_message` convert without copying. @@ -116,21 +134,53 @@ make the send path hand a single owned buffer down to the QUIC stream: files 8% slower). Paid-list evidence is not affected, and the previous unbounded fan-out lost that evidence outright under load (2,795 "paid notify dropped at admission" in one hour on the baseline fleet). -- The drainer re-reads each chunk from disk when its permit arrives, one - extra read per accepted write. +- It is not a hard memory bound. The queues ahead of the permit hold a key + and a stripped payment proof per write (about 40 KB for a single-node + proof, about 130 KB for a merkle proof, at most 512 KiB), so a sustained + write rate above the send rate still grows memory, roughly a hundred times + more slowly than one encoded chunk per write did. Capping those queues + would mean dropping offers, which is left to a later decision if testnets + show a sustained backlog. +- The dispatcher re-reads and re-verifies each chunk when its permit + arrives: one extra read and one BLAKE3 pass per accepted write, the same + as serving it once. +- The pending-offer budget bounds the sender's memory, not what receivers + admit. Offers are one-way and the sender does not read the answer, so a + burst of small chunks from one sender can exceed a receiver's per-source + fresh-offer admission cap, and that receiver refuses the excess. Chunk- + sized sends are slow enough that this is rare in practice, and neighbor + sync fills the gap, as it does for any refused offer. ### Neutral / Operational - `MAX_PENDING_FRESH_OFFERS` and `MAX_CONCURRENT_REPLICATION_SENDS` are the two knobs; raising the first trades memory for burst absorption. +- A write whose read-back fails `MAX_FRESH_READ_ATTEMPTS` times is not + offered, and the failed read leaves the key marked suspect, so the node + stops advertising it. That is self-correcting: the store clears the mark + on the next read that succeeds (a peer's fetch, or a duplicate PUT or + offer checking what it holds), and on a repair or re-put. The paid-list evidence + went out before the read was attempted, and the client stored the chunk + directly on a majority of the close group, each of which fans it out, so + one node's lost offers cost a replica for a while, not the data. - The signing step still serializes the payload once to produce the signed bytes; changing that would alter the signature input and is out of scope. ## Validation -- Unit tests: exact-capacity encoding of chunk-sized offers (ant-node) and - byte-for-byte equivalence of `WireMessageRef` with `WireMessage` - (saorsa-core); replication unit and e2e fresh-replication scenarios pass. +- Unit tests: exact-capacity encoding of chunk-sized offers and + exact-capacity decoding of fetched chunks; wire equivalence of every + byte-string field with its `u8`-sequence layout, both directions, across + the varint length boundaries (ant-node); byte-for-byte equivalence of + `WireMessageRef` with `WireMessage` (saorsa-core). +- E2E tests over the real harness, whose nodes wire the PUT handler to the + fresh-write pipeline as a node does: a PUT through the handler replicates + and a missing chunk queued ahead of it is skipped; with the send stage + held, a burst three times the budget encodes exactly + `MAX_PENDING_FRESH_OFFERS` offers, then all of them once sends resume, + and returns every permit; a failed read-back releases its permit, does + not delay a healthy write behind it, and is offered once the fault + clears; a chunk corrupted on disk is quarantined and never offered. - Testnet evidence (2026-09-21): with the backpressure change, the node that had reached 1051 MiB live memory stayed flat at 0.0 MiB/min with a 150 MiB peak, and the worst bootstrap's queued offers dropped from 101 From afe5cbcf14a248599e0a5c0d892b07ca9f158fdf Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:45:39 +0200 Subject: [PATCH 12/24] test(replication): run the fresh-write pipeline tests serially The four pipeline tests were the only harness tests in the file without `#[serial]`, so serial_test let them overlap the serial ones. The burst test drives 24 offers per receiver past the per-source admission cap, and those refusals land in the process-wide counters that `normal_upload_never_reaches_fresh_offer_capacity` asserts stay at zero: run together with two test threads, the capacity test failed 3 of 3 times. CI runs e2e with one thread, which hid it. Co-Authored-By: Claude Opus 5.5 --- tests/e2e/replication.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/e2e/replication.rs b/tests/e2e/replication.rs index ed39a97c..818d2d2a 100644 --- a/tests/e2e/replication.rs +++ b/tests/e2e/replication.rs @@ -290,6 +290,7 @@ async fn test_fresh_replication_propagates_to_close_group() { /// being offered, without stalling the pipeline and without keeping its /// pending-offer permit. #[tokio::test] +#[serial] async fn fresh_write_pipeline_replicates_a_put_and_skips_missing_chunks() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); @@ -352,6 +353,7 @@ async fn fresh_write_pipeline_replicates_a_put_and_skips_missing_chunks() { /// write is offered — none is lost to back-pressure — and every permit comes /// back. #[tokio::test] +#[serial] async fn fresh_write_pipeline_holds_a_burst_at_the_offer_budget() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); @@ -422,6 +424,7 @@ async fn fresh_write_pipeline_holds_a_burst_at_the_offer_budget() { /// standing where the chunk file should be, which the store refuses to read /// whatever the platform or user. #[tokio::test] +#[serial] async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); @@ -525,6 +528,7 @@ async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { /// verifies the chunk, quarantines it on the mismatch, and the retry finds it /// gone and skips it. #[tokio::test] +#[serial] async fn fresh_write_pipeline_never_offers_a_corrupt_chunk() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); From d5f400e2474f415114d7c278b581a4aeb4cf5c54 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:45:39 +0200 Subject: [PATCH 13/24] docs(replication): the pending-offer and send semaphores are never closed `replicate_fresh` said its permit wait "only fails at shutdown" and `hold_replication_sends` said it returns `None` only when shutting down. Neither semaphore is ever closed, so neither can fail; say so. Co-Authored-By: Claude Opus 5.5 --- src/replication/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/replication/mod.rs b/src/replication/mod.rs index 8daba7c7..ea672bc5 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2282,7 +2282,7 @@ impl ReplicationEngine { /// Test-only: take every outbound replication send permit. Until the /// returned permit is dropped, encoded fresh offers wait behind the send /// stage, so a test can fill the pending-offer budget deterministically. - /// `None` only if the engine is shutting down. + /// The send semaphore is never closed, so this returns `Some`. #[cfg(any(test, feature = "test-utils"))] pub async fn hold_replication_sends(&self) -> Option { let all = u32::try_from(MAX_CONCURRENT_REPLICATION_SENDS).ok()?; @@ -2471,7 +2471,7 @@ impl ReplicationEngine { &self.config, ) .await; - // The semaphore is never closed, so this only fails at shutdown. + // Never closed, so this cannot fail; the arm keeps the call panic-free. let Ok(pending_offer) = Arc::clone(&self.pending_offer_semaphore) .acquire_owned() .await From 338eb3e579693f9f30cc2ccb0389d2e4b33c74cd Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:47:41 +0200 Subject: [PATCH 14/24] fix(replication): back off read-back retries so a store-wide fault spares the backlog Read-back retries were a fixed three attempts one second apart. A read fault is usually store-wide (exhausted descriptors), and then every queued write fails together; since each failure frees its permit for the next write at once, the whole backlog spent its attempts within about two seconds and lost its fresh offers and possession checks, with every key left marked suspect. origin/main was immune (the bytes travelled in the event), and the earlier inline sleep paced it at about one attempt a second at the cost of stalling healthy writes. Retries now back off: seven attempts, the pause doubling from one second (1, 2, 4, 8, 16, 32 s), still on their own tasks so a single unreadable chunk holds nothing behind it. A store-wide fault shorter than about a minute now costs no offers. Only the first failure and the final give-up log at WARN, so a fault across a large backlog does not flood the log. ADR-0017 describes the window and the whole-backlog case, and lists the readers that actually clear a suspect mark. Deep-review follow-up (K1). Co-Authored-By: Claude Opus 5.5 --- ...ounded-fresh-offers-and-copy-free-sends.md | 31 +++++++----- src/replication/config.rs | 49 +++++++++++++++++-- src/replication/mod.rs | 31 ++++++++---- 3 files changed, 86 insertions(+), 25 deletions(-) diff --git a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md index 5e52bdb9..c44fa8df 100644 --- a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md +++ b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md @@ -89,12 +89,17 @@ make the send path hand a single owned buffer down to the QUIC stream: read, as on any serve, so the node stops advertising it and ordinary repair replaces it; it is never offered. Like every serve, verification follows the store's `verify_on_read` setting (on by default). -- A failed read-back is retried up to `MAX_FRESH_READ_ATTEMPTS` times, - `FRESH_READ_RETRY_DELAY` apart, with the permit released in between. The - delay runs on a task of its own, never on the dispatcher: read faults tend - to be shared (exhausted descriptors), and a dispatcher that slept them out - would hold every healthy write queued behind them. Only a chunk that is no - longer stored is skipped without retry. +- A failed read-back is retried up to `MAX_FRESH_READ_ATTEMPTS` (7) times + with the permit released in between, the pause doubling from + `FRESH_READ_RETRY_DELAY` (1, 2, 4, 8, 16 and 32 s). The delay runs on a + task of its own, never on the dispatcher, so a chunk that alone cannot be + read does not hold the healthy writes queued behind it. Read faults tend + to be store-wide (exhausted descriptors), though, and then every queued + write fails together: each failure frees its permit for the next write at + once, so a fixed short retry window would spend the whole backlog's + attempts within seconds. The backoff spreads them over about a minute, and + a store-wide fault shorter than that costs no offers. Only a chunk that is + no longer stored is skipped without retry. - The chunk moves into the offer rather than being copied, and `ReplicationMessage::encode` serializes into an exactly-sized buffer. The chunk-carrying fields — the offer's data and proof, `PaidNotify`'s proof, @@ -156,11 +161,15 @@ make the send path hand a single owned buffer down to the QUIC stream: - `MAX_PENDING_FRESH_OFFERS` and `MAX_CONCURRENT_REPLICATION_SENDS` are the two knobs; raising the first trades memory for burst absorption. - A write whose read-back fails `MAX_FRESH_READ_ATTEMPTS` times is not - offered, and the failed read leaves the key marked suspect, so the node - stops advertising it. That is self-correcting: the store clears the mark - on the next read that succeeds (a peer's fetch, or a duplicate PUT or - offer checking what it holds), and on a repair or re-put. The paid-list evidence - went out before the read was attempted, and the client stored the chunk + offered, and neither is its possession check scheduled. A store-wide + fault lasting longer than the retry window does this to every write + queued at the time. Each failed read also leaves the key marked suspect, + so the node stops advertising it. That is self-correcting: the store + clears the mark on the next read that succeeds — another holder's + possession probe, a late duplicate offer or client PUT checking what it + holds, a client GET, or this node's own neighbor sync re-fetching a key it + no longer claims — and on a repair or re-put. The paid-list evidence went + out before the read was attempted, and the client stored the chunk directly on a majority of the close group, each of which fans it out, so one node's lost offers cost a replica for a while, not the data. - The signing step still serializes the payload once to produce the signed diff --git a/src/replication/config.rs b/src/replication/config.rs index 0d6898ba..dd89bfbb 100644 --- a/src/replication/config.rs +++ b/src/replication/config.rs @@ -186,12 +186,31 @@ pub const MAX_PENDING_FRESH_OFFERS: usize = 8; /// /// The chunk was stored moments earlier, so a failed read is a transient /// fault (exhausted descriptors, an I/O hiccup) far more often than a lost -/// chunk; a lost chunk reports `None` and is skipped without retry. -pub const MAX_FRESH_READ_ATTEMPTS: u32 = 3; - -/// Pause before retrying a failed chunk read-back in the offer dispatcher. +/// chunk, and such a fault is usually store-wide: every queued write fails +/// at once. The retries back off (see [`fresh_read_retry_delay`]), so these +/// attempts span about a minute and a fault shorter than that costs no +/// offers. A lost chunk reports `None` and is skipped without retry. +pub const MAX_FRESH_READ_ATTEMPTS: u32 = 7; + +/// Pause before the first retry of a failed chunk read-back in the offer +/// dispatcher; each later retry waits [`FRESH_READ_RETRY_BACKOFF_FACTOR`] +/// times longer than the one before. pub const FRESH_READ_RETRY_DELAY: Duration = Duration::from_secs(1); +/// Growth of the pause between successive read-back retries. +pub const FRESH_READ_RETRY_BACKOFF_FACTOR: u32 = 2; + +/// Pause before retrying a read-back that has failed `failed_attempts` times. +/// +/// [`FRESH_READ_RETRY_DELAY`], grown by [`FRESH_READ_RETRY_BACKOFF_FACTOR`] +/// per earlier failure. With [`MAX_FRESH_READ_ATTEMPTS`] the retries come 1, +/// 2, 4, 8, 16 and 32 seconds apart. +#[must_use] +pub fn fresh_read_retry_delay(failed_attempts: u32) -> Duration { + let growth = FRESH_READ_RETRY_BACKOFF_FACTOR.saturating_pow(failed_attempts.saturating_sub(1)); + FRESH_READ_RETRY_DELAY.saturating_mul(growth) +} + /// Maximum number of concurrent in-flight audit-responder tasks. /// /// The LIGHT audit-responder handlers — responsible-chunk audits and subtree @@ -2101,4 +2120,26 @@ mod tests { "a capped retry must still get several looks inside one entry lifetime" ); } + + #[test] + fn fresh_read_retries_back_off_over_about_a_minute() { + let delays: Vec = (1..MAX_FRESH_READ_ATTEMPTS) + .map(fresh_read_retry_delay) + .collect(); + let expected: Vec = [1, 2, 4, 8, 16, 32] + .into_iter() + .map(Duration::from_secs) + .collect(); + assert_eq!(delays, expected); + assert_eq!( + delays.iter().sum::(), + Duration::from_secs(63), + "a store-wide read fault shorter than this costs no fresh offers" + ); + // Out-of-range inputs saturate rather than panic. + assert_eq!(fresh_read_retry_delay(0), FRESH_READ_RETRY_DELAY); + assert!( + fresh_read_retry_delay(u32::MAX) >= fresh_read_retry_delay(MAX_FRESH_READ_ATTEMPTS) + ); + } } diff --git a/src/replication/mod.rs b/src/replication/mod.rs index ea672bc5..38abec22 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -79,7 +79,7 @@ use crate::replication::commitment_state::{ PeerCommitmentRecord, PersistedRetention, ResponderCommitmentState, GOSSIP_ANSWERABILITY_TTL, }; use crate::replication::config::{ - max_parallel_fetch, storage_admission_width, ReplicationConfig, FRESH_READ_RETRY_DELAY, + fresh_read_retry_delay, max_parallel_fetch, storage_admission_width, ReplicationConfig, MAX_AUDIT_RESPONSES_PER_PEER, MAX_CONCURRENT_AUDIT_RESPONSES, MAX_CONCURRENT_REPLICATION_SENDS, MAX_DIGEST_AUDIT_RESPONSES_PER_PEER, MAX_FRESH_READ_ATTEMPTS, MAX_INCOMING_VERIFICATION_KEYS, MAX_PENDING_FRESH_OFFERS, MAX_SUBTREE_ROUND1_PER_PEER, MAX_SUBTREE_SESSIONS, @@ -2556,10 +2556,10 @@ impl ReplicationEngine { /// /// For each forwarded write it acquires a permit, reads the chunk back /// from storage and dispatches the offers. A missing chunk is skipped; a - /// failed read is retried up to `MAX_FRESH_READ_ATTEMPTS` times with the - /// permit released in between, so a transient I/O fault does not lose the - /// write's replication. The retry waits on its own task, never on the - /// dispatcher. + /// failed read is retried up to `MAX_FRESH_READ_ATTEMPTS` times, backing + /// off over about a minute with the permit released in between, so a + /// transient I/O fault does not lose the write's replication. The retry + /// waits on its own task, never on the dispatcher. fn start_fresh_offer_dispatcher(&mut self) { let Some(mut rx) = self.fresh_offer_rx.take() else { return; @@ -2611,19 +2611,30 @@ impl ReplicationEngine { ); continue; } - warn!( - "Failed to read chunk {key_hex} for fresh replication (attempt {attempts}): {e}" - ); + // One warning per write: a store-wide fault fails the + // whole backlog, once per retry. + if attempts == 1 { + warn!( + "Failed to read chunk {key_hex} for fresh replication; will retry: {e}" + ); + } else { + debug!( + "Failed to read chunk {key_hex} for fresh replication (attempt {attempts}): {e}" + ); + } // Wait on a task of its own: a read fault is often // shared (exhausted descriptors), and sleeping here // would hold every healthy write queued behind this - // one for the whole delay, once per failed read. + // one for the whole delay, once per failed read. The + // delay grows, so a store-wide fault spends a write's + // attempts over about a minute rather than seconds. let retry_tx = offer_tx.clone(); let retry_shutdown = shutdown.clone(); + let delay = fresh_read_retry_delay(attempts); retries.spawn(async move { tokio::select! { () = retry_shutdown.cancelled() => {} - () = tokio::time::sleep(FRESH_READ_RETRY_DELAY) => { + () = tokio::time::sleep(delay) => { let _ = retry_tx.send(fresh::FreshOfferEvent { read_attempts: attempts, ..event From 52bfed088561031c59a0f6491bfa4e81d251ada4 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:49:09 +0200 Subject: [PATCH 15/24] test(replication): drive the capacity test through the fresh-write channel The capacity driver looped over `ReplicationEngine::replicate_fresh`, which since the two-stage split waits for a pending-offer permit after announcing each write. On fast storage that paced the loop: 33-39 of the 48 calls blocked, spreading PaidNotify over ~0.8 s where the production drainer sends it at arrival rate (~0.2 s). With receiver PaidNotify handling slowed to 30 ms the production path dropped 77-79 notifies at the per-peer cap while this test still reported zero, so it no longer guarded the pressure it says it applies. It now stores each chunk and queues a `FreshWriteEvent` on the channel the harness wires into the PUT handler, the path production takes. It still sees no refusals with the current caps. Deep-review follow-up (K3). Co-Authored-By: Claude Opus 5.5 --- tests/e2e/fresh_offer_capacity.rs | 32 +++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/tests/e2e/fresh_offer_capacity.rs b/tests/e2e/fresh_offer_capacity.rs index 5774c2a0..c540ae7a 100644 --- a/tests/e2e/fresh_offer_capacity.rs +++ b/tests/e2e/fresh_offer_capacity.rs @@ -26,6 +26,7 @@ use super::TestHarness; use ant_node::client::compute_address; +use ant_node::replication::fresh::FreshWriteEvent; use ant_node::replication::{ fresh_offer_admission_refusals, fresh_offer_refusals_global_pool, fresh_offer_refusals_per_peer_share, paid_notify_admission_refusals, @@ -102,15 +103,11 @@ async fn normal_upload_never_reaches_fresh_offer_capacity() { } let source = harness.test_node(UPLOAD_SOURCE_INDEX).expect("source node"); - let source_storage = source - .ant_protocol - .as_ref() - .expect("source protocol") - .storage(); - let engine = source - .replication_engine - .as_ref() - .expect("source replication engine"); + let source_protocol = source.ant_protocol.as_ref().expect("source protocol"); + let source_storage = source_protocol.storage(); + let fresh_writes = source_protocol + .fresh_write_sender() + .expect("fresh-write sender wired by the harness"); // Counters are process-global and cumulative, and other tests share this // binary, so measure a delta rather than an absolute. @@ -119,14 +116,21 @@ async fn normal_upload_never_reaches_fresh_offer_capacity() { let global_before = fresh_offer_refusals_global_pool(); let share_before = fresh_offer_refusals_per_peer_share(); - // Drive the upload the way the fresh-write drainer does: store, then hand - // off to replication, moving to the next chunk immediately. No pacing — - // pacing here would be the test quietly avoiding the very pressure it - // exists to apply. + // Drive the upload through the fresh-write channel, exactly as the PUT + // handler does: store, then hand the write to the drainer, moving to the + // next chunk immediately. The drainer announces every write at arrival + // rate and only the offers wait for pending-offer permits, as in + // production. No pacing here — pacing would be the test quietly avoiding + // the very pressure it exists to apply. let started = Instant::now(); for (content, address) in &chunks { source_storage.put(address, content).await.expect("put"); - engine.replicate_fresh(address, content, &DUMMY_POP).await; + fresh_writes + .send(FreshWriteEvent { + key: *address, + payment_proof: DUMMY_POP.to_vec(), + }) + .expect("queue fresh write"); } let dispatch_elapsed = started.elapsed(); From b0584c65666ae3d885e2674c9347d6aeee120053 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:53:42 +0200 Subject: [PATCH 16/24] test(replication): time the retry test from the fault and bound the handler PUT - The retry test's half-delay window opened only after a handler PUT returned, and the fault was detected on a 200 ms poll, so on a slow runner a dispatcher that slept the delay out inline could still pass. The healthy chunk is now stored up front and queued straight after the fault is seen, the fault is detected on a 10 ms poll, and the window is counted from the fault. The restore now lands well before the first retry, too. - The corrupt-chunk test no longer claims to observe the retry; it checks nothing is offered by the time the retry is due. - `wait_until_every` takes the poll interval, so the retry test does not hand-roll its own loop. - The handler PUT helper is bounded by the harness's chunk-operation timeout (now public) instead of hanging CI if the handler stalls. Deep-review follow-up (K6, K8, K25, K27). Co-Authored-By: Claude Opus 5.5 --- tests/e2e/replication.rs | 95 +++++++++++++++++++++++++--------------- tests/e2e/testnet.rs | 2 +- 2 files changed, 61 insertions(+), 36 deletions(-) diff --git a/tests/e2e/replication.rs b/tests/e2e/replication.rs index 818d2d2a..9264d97e 100644 --- a/tests/e2e/replication.rs +++ b/tests/e2e/replication.rs @@ -5,15 +5,15 @@ #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] -use super::testnet::TestNetworkConfig; +use super::testnet::{TestNetworkConfig, DEFAULT_CHUNK_OPERATION_TIMEOUT_SECS}; use super::TestHarness; use ant_node::ant_protocol::{ChunkMessage, ChunkMessageBody, ChunkPutRequest, ChunkPutResponse}; use ant_node::client::compute_address; use ant_node::replication::audit_coordinator::AuditChallengeCoordinator; use ant_node::replication::commitment_state::{BuiltCommitment, ResponderCommitmentState}; use ant_node::replication::config::{ - storage_admission_width, FRESH_READ_RETRY_DELAY, K_BUCKET_SIZE, MAX_FRESH_READ_ATTEMPTS, - MAX_PENDING_FRESH_OFFERS, REPLICATION_PROTOCOL_ID, + storage_admission_width, FRESH_READ_RETRY_DELAY, K_BUCKET_SIZE, MAX_PENDING_FRESH_OFFERS, + REPLICATION_PROTOCOL_ID, }; use ant_node::replication::fresh::FreshWriteEvent; use ant_node::replication::protocol::{ @@ -419,7 +419,8 @@ async fn fresh_write_pipeline_holds_a_burst_at_the_offer_budget() { /// Retry: a chunk whose read-back fails is retried after /// `FRESH_READ_RETRY_DELAY` without holding its permit or the dispatcher. A -/// healthy write queued behind it is offered well inside the delay, and once +/// healthy write queued behind it is offered within half the delay of the +/// failed read, which a dispatcher sleeping the delay out cannot do, and once /// the fault clears the failed write is offered too. The fault is a directory /// standing where the chunk file should be, which the store refuses to read /// whatever the platform or user. @@ -444,6 +445,13 @@ async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { b"chunk whose first read-back fails", ) .await; + // Stored up front, so queueing it later takes no time out of the window. + let healthy = store_paid_chunk( + &harness, + FRESH_PIPELINE_SOURCE_INDEX, + b"healthy write queued behind a failed read", + ) + .await; let chunk_file = chunk_file_path(storage.root_dir(), &faulty).expect("chunk file on disk"); let set_aside = chunk_file.with_extension(FAULT_SET_ASIDE_EXTENSION); @@ -458,40 +466,44 @@ async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { .expect("queue write"); // A failed read marks the chunk suspect, which hides it from `exists`: // the observable proof that the dispatcher's first attempt hit the fault. + // Polled finely, so the window below starts within a poll of the fault. assert!( - wait_until( + wait_until_every( || !storage.exists(&faulty).unwrap_or(true), - PROPAGATION_TIMEOUT + PROPAGATION_TIMEOUT, + DISPATCH_POLL_INTERVAL ) .await, "the dispatcher never attempted the faulty read" ); + let fault_seen = tokio::time::Instant::now(); assert!( - wait_until( + wait_until_every( || engine.pending_offer_permits_available() == MAX_PENDING_FRESH_OFFERS, - PERMIT_RELEASE_TIMEOUT + PERMIT_RELEASE_TIMEOUT, + DISPATCH_POLL_INTERVAL ) .await, "the failed read kept its pending-offer permit while waiting to retry" ); // A dispatcher sleeping out the retry delay would hold this write until - // the delay ended; one that is not offers it straight away. - put_paid_chunk( - &harness, - FRESH_PIPELINE_SOURCE_INDEX, - b"healthy write queued behind a failed read", - ) - .await; - let offered_in_time = tokio::time::timeout(FRESH_READ_RETRY_DELAY / 2, async { - while engine.fresh_offers_dispatched() == 0 { - tokio::time::sleep(DISPATCH_POLL_INTERVAL).await; - } - }) - .await - .is_ok(); + // the delay ended; one that is not offers it straight away. The window is + // half the delay, counted from the failed read. + fresh_tx + .send(FreshWriteEvent { + key: healthy, + payment_proof: dummy_payment_proof(), + }) + .expect("queue healthy write"); + let window = (FRESH_READ_RETRY_DELAY / 2).saturating_sub(fault_seen.elapsed()); assert!( - offered_in_time, + wait_until_every( + || engine.fresh_offers_dispatched() > 0, + window, + DISPATCH_POLL_INTERVAL + ) + .await, "a healthy write waited behind the failed read's retry delay" ); @@ -500,7 +512,7 @@ async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { assert!( wait_until( || engine.fresh_offers_dispatched() == 2, - FRESH_READ_RETRY_DELAY * MAX_FRESH_READ_ATTEMPTS + PROPAGATION_TIMEOUT + FRESH_READ_RETRY_DELAY + PROPAGATION_TIMEOUT ) .await, "the failed write was not offered once its read succeeded" @@ -525,8 +537,8 @@ async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { /// A chunk whose bytes rotted on disk after it was stored is never offered: /// every receiver would reject it and charge the sender. The read-back -/// verifies the chunk, quarantines it on the mismatch, and the retry finds it -/// gone and skips it. +/// verifies the chunk and quarantines it on the mismatch, and nothing is +/// offered for it by the time its first retry is due. #[tokio::test] #[serial] async fn fresh_write_pipeline_never_offers_a_corrupt_chunk() { @@ -564,7 +576,7 @@ async fn fresh_write_pipeline_never_offers_a_corrupt_chunk() { .await, "the read-back never quarantined the corrupt chunk" ); - // Long enough for the retry to have run and skipped the vanished chunk. + // Past the first retry, which must not offer anything either. tokio::time::sleep(FRESH_READ_RETRY_DELAY + FRESH_BURST_SETTLE).await; assert_eq!( engine.fresh_offers_dispatched(), @@ -630,16 +642,20 @@ async fn put_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u8] dummy_payment_proof(), )), }; - let response = harness + let protocol = harness .test_node(source_idx) .expect("source node") .ant_protocol .as_ref() - .expect("protocol") - .try_handle_request(&request.encode().expect("encode PUT")) - .await - .expect("handle PUT") - .expect("PUT response"); + .expect("protocol"); + let response = tokio::time::timeout( + Duration::from_secs(DEFAULT_CHUNK_OPERATION_TIMEOUT_SECS), + protocol.try_handle_request(&request.encode().expect("encode PUT")), + ) + .await + .expect("PUT through the handler timed out") + .expect("handle PUT") + .expect("PUT response"); match ChunkMessage::decode(&response) .expect("decode PUT response") .body @@ -691,13 +707,22 @@ async fn wait_until_replicated( /// Poll `condition` every `PROPAGATION_POLL_INTERVAL` until it holds or /// `budget` runs out. -async fn wait_until(mut condition: impl FnMut() -> bool, budget: Duration) -> bool { +async fn wait_until(condition: impl FnMut() -> bool, budget: Duration) -> bool { + wait_until_every(condition, budget, PROPAGATION_POLL_INTERVAL).await +} + +/// Poll `condition` every `interval` until it holds or `budget` runs out. +async fn wait_until_every( + mut condition: impl FnMut() -> bool, + budget: Duration, + interval: Duration, +) -> bool { let deadline = tokio::time::Instant::now() + budget; while tokio::time::Instant::now() < deadline { if condition() { return true; } - tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; + tokio::time::sleep(interval).await; } false } diff --git a/tests/e2e/testnet.rs b/tests/e2e/testnet.rs index 073029db..eca97c84 100644 --- a/tests/e2e/testnet.rs +++ b/tests/e2e/testnet.rs @@ -102,7 +102,7 @@ const SMALL_STABILIZATION_TIMEOUT_SECS: u64 = 60; /// conservative; the happy path completes in well under a second on /// loopback, so the larger budget only shows up on flakes. Test-only — /// no production code path reads this constant. -const DEFAULT_CHUNK_OPERATION_TIMEOUT_SECS: u64 = 90; +pub const DEFAULT_CHUNK_OPERATION_TIMEOUT_SECS: u64 = 90; /// Short node-level network timeout for E2E test harness. /// From 5d50836da4190abec590e381c34b72f7ea40f97d Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:55:56 +0200 Subject: [PATCH 17/24] perf(storage): read a chunk into a buffer sized from its file `read_bounded` read into `Vec::new()` through `Take`, which gets no size hint and grows by doubling: a full 4 MiB chunk ended in an 8 MiB allocation, a 3 MiB one in 4 MiB. Every GET serve, audit read and now every fresh-offer read-back went through it, and a fetch response holds that buffer while it is sent. The buffer is now sized from the file's length, capped at the chunk ceiling so a planted oversized file still cannot make it allocate more than a chunk up front. A test pins exact capacity for a full chunk, verified and raw. Deep-review follow-up (K15). Co-Authored-By: Claude Opus 5.5 --- src/storage/file_store.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/storage/file_store.rs b/src/storage/file_store.rs index 3172bdda..dafb6a59 100644 --- a/src/storage/file_store.rs +++ b/src/storage/file_store.rs @@ -2462,7 +2462,12 @@ fn open_regular(path: &Path) -> Result> { /// during an ordinary GET or an audit response. fn read_bounded(file: File, path: &Path) -> Result> { let ceiling = MAX_CHUNK_SIZE as u64; - let mut buf = Vec::new(); + // Sized from the file, so a chunk lands in an exactly-sized buffer: grown from + // empty, a full chunk ended in twice its size, held for as long as the bytes are + // served or offered. Capped at the ceiling, so a planted oversized file still + // cannot make this allocate more than a chunk up front. + let expected = file.metadata().map_or(0, |m| m.len()).min(ceiling); + let mut buf = Vec::with_capacity(usize::try_from(expected).unwrap_or(0)); let read = file.take(ceiling + 1).read_to_end(&mut buf).map_err(|e| { Error::Storage(format!("Failed to read chunk file {}: {e}", path.display())) })?; @@ -3189,6 +3194,21 @@ mod tests { assert_eq!(raw, b"tampered"); } + /// A full-size chunk reads into an exactly-sized buffer, verified or raw. + #[tokio::test] + async fn reads_allocate_exactly_the_chunk() { + let (store, _dir) = test_store().await; + let content: Vec = (0..=u8::MAX).cycle().take(MAX_CHUNK_SIZE).collect(); + let addr = crate::client::compute_address(&content); + store.put(&addr, &content).await.expect("put"); + + let verified = store.get(&addr).await.expect("get").expect("bytes"); + assert_eq!(verified, content); + assert_eq!(verified.capacity(), MAX_CHUNK_SIZE); + let raw = store.get_raw(&addr).await.expect("get_raw").expect("bytes"); + assert_eq!(raw.capacity(), MAX_CHUNK_SIZE); + } + /// A put whose caller goes away does not admit a key on bytes nothing has read. /// /// The blocking half of a put outlives the future that started it, deliberately, so From b4e0d765ce346b4b55a30ff5f17ea42309ced749 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:55:56 +0200 Subject: [PATCH 18/24] perf(node): hand chunk GET responses to the transport without copying The client response path copied each encoded response with `to_vec()` before `send_message`, although it is already `Bytes` and `send_message` now takes `impl Into`. A GET response carries up to a whole chunk, so that was a full extra copy per GET, held for the send. The length used for traffic accounting is read before the hand-off. Deep-review follow-up (K12). Co-Authored-By: Claude Opus 5.5 --- src/node.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/node.rs b/src/node.rs index 56813244..b926fd85 100644 --- a/src/node.rs +++ b/src/node.rs @@ -1108,9 +1108,12 @@ impl RunningNode { let traffic_key = handled.traffic_key; match handled.response { Ok(Some(response)) => { + let response_len = response.len(); let send_started = Instant::now(); + // Handed over as the `Bytes` it already is: a chunk GET + // response is up to a whole chunk, and `to_vec` copied it. let send_result = p2p - .send_message(source, response_topic, response.to_vec(), &[]) + .send_message(source, response_topic, response, &[]) .await; if let Some(telemetry) = telemetry { telemetry.finish_send(send_started.elapsed(), send_result.is_ok()); @@ -1118,14 +1121,14 @@ impl RunningNode { // V2-834: attribute response bytes only once the send is // confirmed; failed sends are itemised separately. match (&send_result, traffic_key) { - (Ok(()), Some(key)) => storage_traffic::record_tx(key, response.len()), + (Ok(()), Some(key)) => storage_traffic::record_tx(key, response_len), (Ok(()), None) => { storage_traffic::record_tx( storage_traffic::ChunkResponseKey::Other, - response.len(), + response_len, ); } - (Err(_), _) => storage_traffic::record_send_failed(response.len()), + (Err(_), _) => storage_traffic::record_send_failed(response_len), } if let Err(e) = send_result { warn!("Failed to send {data_type} protocol response to {source}: {e}"); From c48090876dec2ec503889397f2bb54be625d166d Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:57:48 +0200 Subject: [PATCH 19/24] refactor(replication): tidy the fresh-offer sender - Tie the pending-offer permit to every handle of the encoded offer: `EncodedOffer` now owns the buffer and the permit and is wrapped with `Bytes::from_owner`, so the per-peer sends and the transport share one reference count and the permit is released with the last handle anywhere, not the last send task's `Arc`. With today's saorsa-core, which frames into its own buffer, nothing changes; a transport that queued the `Bytes` itself would otherwise have loosened the bound. `bytes` needs 1.10.1 for this (from_owner without its `to_vec` leak). - Move the payment proof into the offer instead of copying it (up to 512 KiB); the dispatcher owns it and dropped it right after. - Rename the sender-side `fresh::dispatch_fresh_offer` to `fresh::send_fresh_offers`, which no longer shares a name with the receiver-side admission function `dispatch_fresh_offer`. - Drop the explicit `drop(offer_msg)`: no await follows it any more, so it freed nothing earlier and its comment claimed otherwise. - `send_paid_notify` is private again and `FreshOfferContext` loses an unused `Clone`; both were left from intermediate commits. Deep-review follow-up (K16, K20, K21, K22, K32). Co-Authored-By: Claude Opus 5.5 --- Cargo.toml | 3 +- src/replication/fresh.rs | 76 +++++++++++++++++++++++++++------------- src/replication/mod.rs | 8 ++--- 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 84a73314..11b23ac7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,7 +85,8 @@ rmp-serde = "1" hex = "0.4" # Utilities -bytes = "1" +# 1.10.1: `Bytes::from_owner` (1.9) without its `to_vec` leak (fixed in 1.10.1). +bytes = "1.10.1" chrono = { version = "0.4", features = ["serde"] } tempfile = "3" rand = "0.8" diff --git a/src/replication/fresh.rs b/src/replication/fresh.rs index e4b87ab8..af656cb2 100644 --- a/src/replication/fresh.rs +++ b/src/replication/fresh.rs @@ -52,9 +52,8 @@ pub(crate) struct FreshOfferEvent { pub(crate) read_attempts: u32, } -/// Handles shared by everything that dispatches fresh offers, so the offer +/// Handles shared by everything that sends fresh offers, so the offer /// dispatcher task and the direct entry point run one pipeline. -#[derive(Clone)] pub(crate) struct FreshOfferContext { pub(crate) p2p_node: Arc, pub(crate) config: Arc, @@ -68,18 +67,23 @@ pub(crate) struct FreshOfferContext { pub(crate) dispatched: Arc, } -/// An encoded fresh offer shared by the per-peer send tasks. +/// An encoded fresh offer and the pending-offer permit it holds. /// -/// The pending-offer permit is released together with the buffer, once the -/// last send task drops its reference, which caps how many encoded offers -/// can wait behind the send permits at `MAX_PENDING_FRESH_OFFERS`. The bytes -/// are shared with the transport as well: each send attempt hands out a -/// reference-counted handle rather than a copy. +/// Wrapped with [`Bytes::from_owner`], so the per-peer send tasks and the +/// transport share the one buffer through reference-counted handles, and the +/// permit is released only when the last handle anywhere is dropped. That is +/// what caps the encoded offers alive at once at `MAX_PENDING_FRESH_OFFERS`. struct EncodedOffer { - bytes: Bytes, + bytes: Vec, _pending: OwnedSemaphorePermit, } +impl AsRef<[u8]> for EncodedOffer { + fn as_ref(&self) -> &[u8] { + &self.bytes + } +} + /// Rules 6-8: record the paid key locally and announce it to /// `PaidCloseGroup(K)`. /// @@ -105,14 +109,14 @@ pub(crate) async fn announce_paid_write( /// possession check (ADR-0003) for the responsible peers. /// /// `pending_offer` is the caller's permit from the pending-offer semaphore; -/// it is held with the encoded offer until the last per-peer send finishes. -/// `data` is taken by value so the chunk moves into the offer instead of -/// being copied. -pub(crate) async fn dispatch_fresh_offer( +/// it is held with the encoded offer until the last handle to it is dropped. +/// `data` and `proof_of_payment` are taken by value so they move into the +/// offer instead of being copied. +pub(crate) async fn send_fresh_offers( ctx: &FreshOfferContext, key: &XorName, data: Vec, - proof_of_payment: &[u8], + proof_of_payment: Vec, pending_offer: OwnedSemaphorePermit, ) { let self_id = *ctx.p2p_node.peer_id(); @@ -133,7 +137,7 @@ pub(crate) async fn dispatch_fresh_offer( let offer = FreshReplicationOffer { key: *key, data, - proof_of_payment: proof_of_payment.to_vec(), + proof_of_payment, }; let request_id = rand::thread_rng().gen::(); let offer_msg = ReplicationMessage { @@ -141,11 +145,7 @@ pub(crate) async fn dispatch_fresh_offer( body: ReplicationMessageBody::FreshReplicationOffer(offer), }; - let encoded = offer_msg.encode(); - // Only the encoded bytes are needed from here on; release the chunk now - // rather than holding it alongside the encoding while sends are queued. - drop(offer_msg); - let Ok(encoded) = encoded else { + let Ok(encoded) = offer_msg.encode() else { warn!( "Failed to encode FreshReplicationOffer for {}", hex::encode(key), @@ -155,13 +155,13 @@ pub(crate) async fn dispatch_fresh_offer( // One encoded copy serves every per-peer send task and every retry; the // transport borrows it through `Bytes` instead of taking a copy. The // pending-offer permit travels with the buffer. - let encoded = Arc::new(EncodedOffer { - bytes: Bytes::from(encoded), + let encoded = Bytes::from_owner(EncodedOffer { + bytes: encoded, _pending: pending_offer, }); for peer in &target_peers { let p2p = Arc::clone(&ctx.p2p_node); - let offer = Arc::clone(&encoded); + let offer = encoded.clone(); let peer_id = *peer; let sem = Arc::clone(&ctx.send_semaphore); tokio::spawn(async move { @@ -179,7 +179,7 @@ pub(crate) async fn dispatch_fresh_offer( let mut attempt = 0u32; loop { match p2p - .send_message(&peer_id, REPLICATION_PROTOCOL_ID, offer.bytes.clone(), &[]) + .send_message(&peer_id, REPLICATION_PROTOCOL_ID, offer.clone(), &[]) .await { Ok(()) => break, @@ -224,7 +224,7 @@ pub(crate) async fn dispatch_fresh_offer( /// Per Invariant 16: sender MUST attempt delivery to every member. The /// message is small metadata (no chunk data), so it is neither gated by the /// send semaphore nor by the pending-offer permit. -pub(crate) async fn send_paid_notify( +async fn send_paid_notify( key: &XorName, proof_of_payment: &[u8], p2p_node: &Arc, @@ -269,3 +269,29 @@ pub(crate) async fn send_paid_notify( }); } } + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + + /// The permit is released with the last handle to the encoded offer, not + /// the first: a slice the transport still holds keeps the offer counted. + #[test] + fn the_pending_offer_permit_outlives_every_handle() { + let permits = Arc::new(Semaphore::new(1)); + let permit = Arc::clone(&permits) + .try_acquire_owned() + .expect("free permit"); + let offer = Bytes::from_owner(EncodedOffer { + bytes: vec![7; 16], + _pending: permit, + }); + let held_elsewhere = offer.slice(4..8); + + drop(offer); + assert_eq!(permits.available_permits(), 0); + drop(held_elsewhere); + assert_eq!(permits.available_permits(), 1); + } +} diff --git a/src/replication/mod.rs b/src/replication/mod.rs index 38abec22..a01ab640 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -2478,11 +2478,11 @@ impl ReplicationEngine { else { return; }; - fresh::dispatch_fresh_offer( + fresh::send_fresh_offers( &self.fresh_offer_context(), key, data.to_vec(), - proof_of_payment, + proof_of_payment.to_vec(), pending_offer, ) .await; @@ -2645,11 +2645,11 @@ impl ReplicationEngine { continue; } }; - fresh::dispatch_fresh_offer( + fresh::send_fresh_offers( &ctx, &event.key, data, - &event.payment_proof, + event.payment_proof, pending_offer, ) .await; From 21eb07f0f0234b16d9e7ac3d7d1be3436057e920 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:00:50 +0200 Subject: [PATCH 20/24] refactor: share one exactly-sized postcard encoder `ReplicationMessage::encode` and `web_rtc::encode_binary_response` each sized a message, refused it over a limit and then encoded it into exactly that space. Both now call `codec::encode_exact`, which keeps the one use of `postcard::experimental::serialized_size` in a single place, and the WebRTC path stops zero-filling a buffer of up to a chunk before encoding into it. In `encode`, the family-ceiling rationale had been left below the encode after the check moved above it; it sits with the check again. Deep-review follow-up (K10, K19, K30). Co-Authored-By: Claude Opus 5.5 --- src/codec.rs | 57 +++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/replication/protocol.rs | 32 +++++++++++---------- src/web_rtc.rs | 17 +++++------ 4 files changed, 82 insertions(+), 25 deletions(-) create mode 100644 src/codec.rs diff --git a/src/codec.rs b/src/codec.rs new file mode 100644 index 00000000..3f401ee0 --- /dev/null +++ b/src/codec.rs @@ -0,0 +1,57 @@ +//! Exactly-sized postcard encoding shared by the node's wire formats. + +use serde::Serialize; + +/// Why [`encode_exact`] produced no bytes. +#[derive(Debug)] +pub enum ExactEncodeError { + /// Postcard could not serialize the value. + Serialize(postcard::Error), + /// The encoding would be `size` bytes, over `limit`; nothing was allocated. + TooLarge { size: usize, limit: usize }, +} + +/// Encode `value` into a buffer of exactly its serialized size, refusing +/// before anything is allocated if that size is over `limit`. +/// +/// A growing `Vec` would otherwise keep up to twice the needed capacity for +/// as long as the bytes are held, and chunk-carrying messages are held while +/// they are sent. Sizing first costs next to nothing: postcard's sizing pass +/// adds a byte string's length in one step, and compiles a plain `u8` +/// sequence's per-byte count down to a length too. +pub fn encode_exact( + value: &T, + limit: usize, +) -> Result, ExactEncodeError> { + let size = + postcard::experimental::serialized_size(value).map_err(ExactEncodeError::Serialize)?; + if size > limit { + return Err(ExactEncodeError::TooLarge { size, limit }); + } + postcard::to_extend(value, Vec::with_capacity(size)).map_err(ExactEncodeError::Serialize) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + + #[test] + fn encodes_into_exactly_the_serialized_size() { + let value = vec![0xABu8; 70_000]; + let bytes = encode_exact(&value, usize::MAX).expect("encode"); + assert_eq!(bytes, postcard::to_stdvec(&value).expect("reference")); + assert_eq!(bytes.capacity(), bytes.len()); + } + + #[test] + fn refuses_an_encoding_over_the_limit() { + let value = vec![1u8; 100]; + let size = postcard::to_stdvec(&value).expect("reference").len(); + assert!(encode_exact(&value, size).is_ok()); + assert!(matches!( + encode_exact(&value, size - 1), + Err(ExactEncodeError::TooLarge { size: s, limit }) if s == size && limit == size - 1 + )); + } +} diff --git a/src/lib.rs b/src/lib.rs index 852c837a..3adbaace 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,6 +47,7 @@ pub mod ant_protocol; pub mod browser; pub mod client; +mod codec; pub mod config; pub mod devnet; pub mod error; diff --git a/src/replication/protocol.rs b/src/replication/protocol.rs index d71811b8..9d617340 100644 --- a/src/replication/protocol.rs +++ b/src/replication/protocol.rs @@ -10,6 +10,7 @@ use saorsa_core::identity::PeerId; use serde::{Deserialize, Serialize}; use crate::ant_protocol::XorName; +use crate::codec::{encode_exact, ExactEncodeError}; use super::types::AuditFailureReason; @@ -46,21 +47,6 @@ impl ReplicationMessage { /// Returns [`ReplicationProtocolError::SerializationFailed`] if postcard /// serialization fails. pub fn encode(&self) -> Result, ReplicationProtocolError> { - // Size the buffer exactly up front. Chunk-carrying bodies run to - // several MiB, and a growing `Vec` would otherwise end up with up to - // twice the needed capacity, retained for as long as the encoded - // message is queued for sending. - let size = postcard::experimental::serialized_size(self) - .map_err(|e| ReplicationProtocolError::SerializationFailed(e.to_string()))?; - // The size is known before anything is allocated, so an oversized body - // is refused without serializing it first. - let max_size = ceiling_for(family_of_variant(self.body.variant_index())); - if size > max_size { - return Err(ReplicationProtocolError::MessageTooLarge { size, max_size }); - } - let bytes = postcard::to_extend(self, Vec::with_capacity(size)) - .map_err(|e| ReplicationProtocolError::SerializationFailed(e.to_string()))?; - // The same family ceiling the decoder applies, from the same table and // with the same arms, including the unclassified case. Every receiver // drops a subtree-audit body over that ceiling before decoding it, so @@ -78,6 +64,22 @@ impl ReplicationMessage { // the largest is a round-1 proof at the commitment // key-count cap, pinned under it with headroom by // `max_round1_proof_fits_the_audit_family_ceiling`. + // + // The buffer is sized exactly, and an oversized body is refused + // before anything is allocated. Chunk-carrying bodies run to several + // MiB and are held while they are sent. + let max_size = ceiling_for(family_of_variant(self.body.variant_index())); + let bytes = encode_exact(self, max_size).map_err(|e| match e { + ExactEncodeError::Serialize(e) => { + ReplicationProtocolError::SerializationFailed(e.to_string()) + } + ExactEncodeError::TooLarge { size, limit } => { + ReplicationProtocolError::MessageTooLarge { + size, + max_size: limit, + } + } + })?; // V2-623: cumulative per-variant tx accounting. Every replication send // funnels through here, so this is the single tx choke point. diff --git a/src/web_rtc.rs b/src/web_rtc.rs index 6bb3e8e0..b682aba1 100644 --- a/src/web_rtc.rs +++ b/src/web_rtc.rs @@ -13,6 +13,7 @@ use crate::ant_protocol::{ ChunkQuoteResponse, MAX_CHUNK_SIZE, }; use crate::browser::{browser_payment_network, BrowserEndpoint, BrowserPaymentNetwork}; +use crate::codec::{encode_exact, ExactEncodeError}; use crate::config::WebRtcDirectConfig; use crate::error::{Error, Result}; use crate::logging::{debug, info, warn}; @@ -1762,16 +1763,12 @@ fn binary_response_limit(body: &ChunkMessageBody) -> ServerResult { fn encode_binary_response(message: &ChunkMessage, limit: usize) -> ServerResult> { // Size without allocating, then encode into exactly the charged space. - // This also prevents Vec growth from retaining an oversized capacity. - let length = postcard::experimental::serialized_size(message) - .map_err(|error| public_error("invalid_response", error))?; - if length > limit { - return Err("chunk protocol response exceeds operation limit".to_string()); - } - let mut bytes = vec![0; length]; - postcard::to_slice(message, &mut bytes) - .map_err(|error| public_error("invalid_response", error))?; - Ok(bytes) + encode_exact(message, limit).map_err(|error| match error { + ExactEncodeError::Serialize(error) => public_error("invalid_response", error), + ExactEncodeError::TooLarge { .. } => { + "chunk protocol response exceeds operation limit".to_string() + } + }) } fn hello_response(request_id: u64, state: &ServerState) -> Response { From 5208c9e1bde6be8d6184a2a39f59d6ac15ef7814 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:01:42 +0200 Subject: [PATCH 21/24] refactor(replication): name the right fresh-replication stage in comments; scope the offer channel to start() - Comments in config.rs, handler.rs and mod.rs still said the fresh-write drainer takes the pending-offer permit, reads the chunk back and schedules possession checks, and that the direct entry point schedules them itself. The offer dispatcher and `fresh::send_fresh_offers` do all of that; the drainer never waits by design. The `refuse_fresh_offer` doc named the deleted `fresh::replicate_fresh`. `replicate_fresh`'s doc now says how it differs from the PUT path (it waits for a permit and offers the caller's bytes). - The drainer-to-dispatcher channel was kept as two engine fields used only by the launchers `start()` calls; it is now created in `start_fresh_replication` and handed to both stages. - The dispatcher's explicit `drop(pending_offer)` did nothing the end of the iteration does not; a comment says when the permit goes instead. Deep-review follow-up (K18, K19, K20, K23). Co-Authored-By: Claude Opus 5.5 --- src/replication/config.rs | 2 +- src/replication/mod.rs | 67 +++++++++++++++++++++------------------ src/storage/handler.rs | 2 +- 3 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/replication/config.rs b/src/replication/config.rs index dd89bfbb..7f35d2fe 100644 --- a/src/replication/config.rs +++ b/src/replication/config.rs @@ -176,7 +176,7 @@ pub const MAX_CONCURRENT_REPLICATION_SENDS: usize = 3; /// that buffer stays alive until the last of its per-peer sends completes. /// With only `MAX_CONCURRENT_REPLICATION_SENDS` transfers in flight, a write /// rate above the network's send rate would otherwise queue an unbounded -/// number of encoded offers behind the send permits. The fresh-write drainer +/// number of encoded offers behind the send permits. The offer dispatcher /// takes one of these permits before it reads and encodes a chunk, so the /// backlog waits as small queued events instead of chunk-sized buffers. pub const MAX_PENDING_FRESH_OFFERS: usize = 8; diff --git a/src/replication/mod.rs b/src/replication/mod.rs index a01ab640..1beaa8c8 100644 --- a/src/replication/mod.rs +++ b/src/replication/mod.rs @@ -1827,14 +1827,10 @@ pub struct ReplicationEngine { pointers: Option>, /// Receiver for fresh pointer writes, taken by `start()`. pointer_fresh_rx: Option>, - /// Hand-off from the fresh-write drainer to the offer dispatcher, the only - /// permit-gated stage. Unbounded and FIFO, holding key + proof only. - fresh_offer_tx: mpsc::UnboundedSender, - /// Receiver paired with `fresh_offer_tx`; taken by the dispatcher task. - fresh_offer_rx: Option>, - /// Sender for delayed possession-check events (ADR-0003). The fresh-write - /// drainer pushes the responsible close-group peers here after each fresh - /// replication; the possession-check scheduler drains the paired receiver. + /// Sender for delayed possession-check events (ADR-0003). Sending a write's + /// fresh offers (`fresh::send_fresh_offers`) pushes its responsible + /// close-group peers here; the possession-check scheduler drains the paired + /// receiver. possession_check_tx: mpsc::UnboundedSender, /// Receiver paired with `possession_check_tx`; taken by the scheduler task. possession_check_rx: Option>, @@ -1895,7 +1891,6 @@ impl ReplicationEngine { let initial_neighbors = NeighborSyncState::new_cycle(Vec::new()); let config = Arc::new(config); let (possession_check_tx, possession_check_rx) = mpsc::unbounded_channel(); - let (fresh_offer_tx, fresh_offer_rx) = mpsc::unbounded_channel(); // ADR-0004: monetized-pin channel (verifier -> first-audit drainer). // Bounded (Amendment 2): every stage of the first-audit pipeline is @@ -1970,8 +1965,6 @@ impl ReplicationEngine { fresh_write_rx: Some(fresh_write_rx), pointers: None, pointer_fresh_rx: None, - fresh_offer_tx, - fresh_offer_rx: Some(fresh_offer_rx), possession_check_tx, possession_check_rx: Some(possession_check_rx), monetized_pin_tx, @@ -2317,8 +2310,7 @@ impl ReplicationEngine { self.start_fetch_worker(); self.start_verification_worker(); self.start_bootstrap_sync(dht_events); - self.start_fresh_write_drainer(); - self.start_fresh_offer_dispatcher(); + self.start_fresh_replication(); self.start_possession_check_scheduler(); if let Some(pointers) = &self.pointers { self.task_handles.push(pointers.start_verification_loop()); @@ -2457,11 +2449,13 @@ impl ReplicationEngine { self.sync_trigger.notify_one(); } - /// Execute fresh replication for a newly stored record, then schedule the - /// delayed possession check for the responsible close-group peers - /// (ADR-0003). The production PUT path schedules via the fresh-write - /// drainer; this direct entry point schedules here so callers (and tests) - /// that drive replication directly still get the possession check. + /// Execute fresh replication for a newly stored record: announce it, then + /// send its offers and schedule the delayed possession check for the + /// responsible close-group peers (ADR-0003), as the PUT path does. + /// + /// Unlike the PUT path, this waits for a pending-offer permit before + /// returning, and it offers the caller's bytes rather than reading them + /// back from storage. pub async fn replicate_fresh(&self, key: &XorName, data: &[u8], proof_of_payment: &[u8]) { fresh::announce_paid_write( key, @@ -2503,16 +2497,28 @@ impl ReplicationEngine { // Background task launchers // ======================================================================= - /// Spawn a task that drains the fresh-write channel and triggers - /// replication for each newly-stored chunk. - fn start_fresh_write_drainer(&mut self) { - let Some(mut rx) = self.fresh_write_rx.take() else { + /// Spawn fresh replication's two stages, joined by an unbounded FIFO of + /// key + proof events: the drainer, which announces every write at arrival + /// rate, and the offer dispatcher, the only permit-gated stage. + fn start_fresh_replication(&mut self) { + let Some(writes) = self.fresh_write_rx.take() else { return; }; + let (offer_tx, offer_rx) = mpsc::unbounded_channel(); + self.start_fresh_write_drainer(writes, offer_tx.clone()); + self.start_fresh_offer_dispatcher(offer_rx, offer_tx); + } + + /// Spawn a task that drains the fresh-write channel: it announces each + /// newly-stored chunk and hands it to the offer dispatcher. + fn start_fresh_write_drainer( + &mut self, + mut rx: mpsc::UnboundedReceiver, + offer_tx: mpsc::UnboundedSender, + ) { let p2p = Arc::clone(&self.p2p_node); let paid_list = Arc::clone(&self.paid_list); let config = Arc::clone(&self.config); - let offer_tx = self.fresh_offer_tx.clone(); let shutdown = self.shutdown.clone(); let handle = tokio::spawn(async move { @@ -2560,13 +2566,13 @@ impl ReplicationEngine { /// off over about a minute with the permit released in between, so a /// transient I/O fault does not lose the write's replication. The retry /// waits on its own task, never on the dispatcher. - fn start_fresh_offer_dispatcher(&mut self) { - let Some(mut rx) = self.fresh_offer_rx.take() else { - return; - }; + fn start_fresh_offer_dispatcher( + &mut self, + mut rx: mpsc::UnboundedReceiver, + offer_tx: mpsc::UnboundedSender, + ) { let storage = Arc::clone(&self.storage); let pending_offer_semaphore = Arc::clone(&self.pending_offer_semaphore); - let offer_tx = self.fresh_offer_tx.clone(); let ctx = self.fresh_offer_context(); let shutdown = self.shutdown.clone(); let retries = self.detached_task_tracker.clone(); @@ -2603,7 +2609,8 @@ impl ReplicationEngine { continue; } Err(e) => { - drop(pending_offer); + // The permit is released as this iteration ends, well + // before any retry is due. let attempts = event.read_attempts + 1; if attempts >= MAX_FRESH_READ_ATTEMPTS { warn!( @@ -5913,7 +5920,7 @@ fn fresh_offer_structural_rejection( /// Tell `source` its offer for `key` was not taken. /// -/// Note the sender does not currently read this: `fresh::replicate_fresh` uses +/// Note the sender does not currently read this: `fresh::send_fresh_offers` uses /// one-way `send_message`, so the refusal is observed only as a later absence by /// the delayed possession check. Recorded in ADR-0005 as a known gap. async fn refuse_fresh_offer( diff --git a/src/storage/handler.rs b/src/storage/handler.rs index 422cd25b..a712bd5a 100644 --- a/src/storage/handler.rs +++ b/src/storage/handler.rs @@ -879,7 +879,7 @@ impl AntProtocol { // replication entirely. let proof = Self::strip_commitment_sidecars(proof); // Storage has already accepted the chunk on this path, so - // the event carries only the key; the replication drainer + // the event carries only the key; the offer dispatcher // reads the chunk back when it is ready to send it. let event = FreshWriteEvent { key: address, From b779d290d7ba9cd1b1c67aed004a2d3bd489731c Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:03:45 +0200 Subject: [PATCH 22/24] test(e2e): reuse the payment, proof, replication and chunk-path helpers - `TestHarness::prepopulate_payment_cache_everywhere` replaces the all-nodes `cache_insert` loop copied through replication.rs and the capacity test. - The inline dummy-proof literals use `dummy_payment_proof()`, and `test_fresh_replication_propagates_to_close_group` uses `store_paid_chunk` and `wait_until_replicated` instead of its own copies of them. - `ChunkStore::test_chunk_path` (test-utils) exposes the store's own path for a chunk, so the tests that damage or remove a chunk file no longer re-derive the on-disk layout. - The fresh-replication source index is documented as a regular node, not the first one (the minimal harness has two bootstraps). - `start_node` no longer invents a closed channel when a node has no fresh-write receiver, which would silently end the drainer; it skips the engine with a warning, like a node without an identity, so tests that need replication fail on the missing engine. Deep-review follow-up (K24, K26, K28, K29, K31). Co-Authored-By: Claude Opus 5.5 --- src/storage/chunk_store.rs | 8 ++ src/storage/file_store.rs | 2 +- tests/e2e/fresh_offer_capacity.rs | 8 +- tests/e2e/harness.rs | 13 +++ tests/e2e/replication.rs | 161 ++++++------------------------ tests/e2e/testnet.rs | 31 +++--- 6 files changed, 74 insertions(+), 149 deletions(-) diff --git a/src/storage/chunk_store.rs b/src/storage/chunk_store.rs index fe297705..e6da0c4f 100644 --- a/src/storage/chunk_store.rs +++ b/src/storage/chunk_store.rs @@ -1104,6 +1104,14 @@ impl ChunkStore { std::fs::metadata(self.legacy_env_dir.join(LEGACY_DATA_FILE)).map_or(0, |m| m.len()) } + /// Test-only path of the file a chunk is (or would be) stored in, so tests + /// can damage or remove it without re-deriving the store's layout. + #[cfg(any(test, feature = "test-utils"))] + #[must_use] + pub fn test_chunk_path(&self, address: &XorName) -> PathBuf { + self.files.chunk_path(address) + } + /// Test-only handle to the file store's put gate. #[cfg(any(test, feature = "test-utils"))] #[must_use] diff --git a/src/storage/file_store.rs b/src/storage/file_store.rs index dafb6a59..b4b087d7 100644 --- a/src/storage/file_store.rs +++ b/src/storage/file_store.rs @@ -1622,7 +1622,7 @@ impl FileStore { } /// Absolute path of a chunk file. - fn chunk_path(&self, address: &XorName) -> PathBuf { + pub(crate) fn chunk_path(&self, address: &XorName) -> PathBuf { self.chunks_dir .join(shard_name(address)) .join(hex::encode(address)) diff --git a/tests/e2e/fresh_offer_capacity.rs b/tests/e2e/fresh_offer_capacity.rs index c540ae7a..66db3358 100644 --- a/tests/e2e/fresh_offer_capacity.rs +++ b/tests/e2e/fresh_offer_capacity.rs @@ -93,13 +93,7 @@ async fn normal_upload_never_reaches_fresh_offer_capacity() { // Every node must accept the offers without an on-chain proof; Anvil is not // running in this suite. for (_, address) in &chunks { - for index in 0..harness.node_count() { - if let Some(node) = harness.test_node(index) { - if let Some(protocol) = node.ant_protocol.as_ref() { - protocol.payment_verifier().cache_insert(*address); - } - } - } + harness.prepopulate_payment_cache_everywhere(address); } let source = harness.test_node(UPLOAD_SOURCE_INDEX).expect("source node"); diff --git a/tests/e2e/harness.rs b/tests/e2e/harness.rs index eaa560f9..786b18c5 100644 --- a/tests/e2e/harness.rs +++ b/tests/e2e/harness.rs @@ -379,6 +379,19 @@ impl TestHarness { self.network.node_mut(index) } + /// Pre-populate the payment cache on every node. + /// + /// The source of a write and every receiver of its fresh offers and + /// `PaidNotify` then accept a dummy proof for `address`, as if it had + /// been paid for on chain (no Anvil runs in most suites). + pub fn prepopulate_payment_cache_everywhere(&self, address: &XorName) { + for node in self.network.nodes() { + if let Some(ref protocol) = node.ant_protocol { + protocol.payment_verifier().cache_insert(*address); + } + } + } + /// Pre-populate the payment cache on the node matching `peer_id`. /// /// Inserts `address` into the target node's payment verifier cache so diff --git a/tests/e2e/replication.rs b/tests/e2e/replication.rs index 9264d97e..876e42c9 100644 --- a/tests/e2e/replication.rs +++ b/tests/e2e/replication.rs @@ -24,7 +24,6 @@ use ant_node::replication::protocol::{ use ant_node::replication::pruning; use ant_node::replication::scheduling::ReplicationQueues; use ant_node::replication::types::{NeighborSyncState, RepairProofs}; -use ant_node::storage::file_store::CHUNKS_DIR_NAME; use ant_node::storage::XorName; use ant_node::ReplicationConfig; use bytes::Bytes; @@ -33,7 +32,6 @@ use saorsa_core::{P2PNode, TrustEvent}; use serial_test::serial; use std::collections::HashSet; use std::fs; -use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; use tokio::sync::RwLock; @@ -56,8 +54,8 @@ const FULL_NODE_SHUN_POSSESSION_DELAY_MAX: Duration = Duration::from_millis(500) const DUMMY_PAYMENT_PROOF_LEN: usize = 64; /// Dummy proof byte used when a test only needs to reach pre-payment gates. const DUMMY_PAYMENT_PROOF_BYTE: u8 = 0x01; -/// First regular (non-bootstrap) node of the minimal harness; source of the -/// fresh-write pipeline tests. +/// A regular (non-bootstrap) node of the minimal harness; source of the fresh +/// replication tests. const FRESH_PIPELINE_SOURCE_INDEX: usize = 3; /// Writes queued at once by the saturation test: three times the pending-offer /// budget, so the dispatcher must block on and recycle permits to drain it. @@ -228,56 +226,19 @@ async fn test_fresh_replication_propagates_to_close_group() { let harness = TestHarness::setup_minimal().await.expect("setup"); harness.warmup_dht().await.expect("warmup"); - // Pick a non-bootstrap node with replication engine - let source_idx = 3; // first regular node - let source = harness.test_node(source_idx).expect("source node"); - let source_protocol = source.ant_protocol.as_ref().expect("protocol"); - let source_storage = source_protocol.storage(); - - // Create and store a chunk + let source_idx = FRESH_PIPELINE_SOURCE_INDEX; let content = b"hello replication world"; - let address = compute_address(content); - source_storage.put(&address, content).await.expect("put"); - - // Pre-populate payment cache on ALL nodes so receivers accept the offer - // (bypasses EVM verification, which is unavailable without Anvil). - for i in 0..harness.node_count() { - if let Some(node) = harness.test_node(i) { - if let Some(protocol) = &node.ant_protocol { - protocol.payment_verifier().cache_insert(address); - } - } - } - - // Trigger fresh replication with a dummy PoP - let dummy_pop = [0x01u8; 64]; - if let Some(ref engine) = source.replication_engine { - engine.replicate_fresh(&address, content, &dummy_pop).await; - } + // Paid on every node, so receivers accept the offer without Anvil. + let address = store_paid_chunk(&harness, source_idx, content).await; + harness + .test_node(source_idx) + .and_then(|node| node.replication_engine.as_ref()) + .expect("source replication engine") + .replicate_fresh(&address, content, &dummy_payment_proof()) + .await; - // Poll until replication propagates (or timeout). - let deadline = tokio::time::Instant::now() + PROPAGATION_TIMEOUT; - let mut found_on_other = false; - while tokio::time::Instant::now() < deadline { - for i in 0..harness.node_count() { - if i == source_idx { - continue; - } - if let Some(node) = harness.test_node(i) { - if let Some(protocol) = &node.ant_protocol { - if protocol.storage().exists(&address).unwrap_or(false) { - found_on_other = true; - } - } - } - } - if found_on_other { - break; - } - tokio::time::sleep(PROPAGATION_POLL_INTERVAL).await; - } assert!( - found_on_other, + wait_until_replicated(&harness, source_idx, &address, PROPAGATION_TIMEOUT).await, "Chunk should have replicated to at least one other node" ); @@ -453,7 +414,8 @@ async fn fresh_write_pipeline_retries_a_failed_read_off_the_dispatcher() { ) .await; - let chunk_file = chunk_file_path(storage.root_dir(), &faulty).expect("chunk file on disk"); + let chunk_file = storage.test_chunk_path(&faulty); + assert!(chunk_file.is_file(), "chunk file on disk"); let set_aside = chunk_file.with_extension(FAULT_SET_ASIDE_EXTENSION); fs::rename(&chunk_file, &set_aside).expect("move the chunk file aside"); fs::create_dir(&chunk_file).expect("put a directory in its place"); @@ -557,7 +519,8 @@ async fn fresh_write_pipeline_never_offers_a_corrupt_chunk() { let content = b"chunk that rots on disk before its offer"; let address = store_paid_chunk(&harness, FRESH_PIPELINE_SOURCE_INDEX, content).await; - let chunk_file = chunk_file_path(storage.root_dir(), &address).expect("chunk file on disk"); + let chunk_file = storage.test_chunk_path(&address); + assert!(chunk_file.is_file(), "chunk file on disk"); let mut rotted = content.to_vec(); rotted.reverse(); fs::write(&chunk_file, &rotted).expect("corrupt the chunk file"); @@ -569,11 +532,7 @@ async fn fresh_write_pipeline_never_offers_a_corrupt_chunk() { }) .expect("queue write"); assert!( - wait_until( - || chunk_file_path(storage.root_dir(), &address).is_none(), - PROPAGATION_TIMEOUT - ) - .await, + wait_until(|| !chunk_file.is_file(), PROPAGATION_TIMEOUT).await, "the read-back never quarantined the corrupt chunk" ); // Past the first retry, which must not offer anything either. @@ -597,19 +556,6 @@ fn dummy_payment_proof() -> Vec { vec![DUMMY_PAYMENT_PROOF_BYTE; DUMMY_PAYMENT_PROOF_LEN] } -/// Pre-populate the payment cache on every node, so the source's handler and -/// the receivers of its offers accept a dummy proof for `address`. -fn cache_payment_everywhere(harness: &TestHarness, address: &XorName) { - for i in 0..harness.node_count() { - if let Some(protocol) = harness - .test_node(i) - .and_then(|node| node.ant_protocol.as_ref()) - { - protocol.payment_verifier().cache_insert(*address); - } - } -} - /// Store a chunk on the source directly, bypassing the handler, so no /// fresh-write event is emitted for it. async fn store_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u8]) -> XorName { @@ -624,7 +570,7 @@ async fn store_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u .put(&address, content) .await .expect("put"); - cache_payment_everywhere(harness, &address); + harness.prepopulate_payment_cache_everywhere(&address); address } @@ -633,7 +579,7 @@ async fn store_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u /// dummy proof it was paid with. async fn put_paid_chunk(harness: &TestHarness, source_idx: usize, content: &[u8]) -> XorName { let address = compute_address(content); - cache_payment_everywhere(harness, &address); + harness.prepopulate_payment_cache_everywhere(&address); let request = ChunkMessage { request_id: rand::random(), body: ChunkMessageBody::PutRequest(ChunkPutRequest::with_payment( @@ -727,17 +673,6 @@ async fn wait_until_every( false } -/// On-disk file of a stored chunk: the store shards `root/chunks/` into -/// subdirectories, so look through them for the file named after the address. -fn chunk_file_path(root_dir: &Path, address: &XorName) -> Option { - let file_name = hex::encode(address); - fs::read_dir(root_dir.join(CHUNKS_DIR_NAME)) - .ok()? - .filter_map(Result::ok) - .map(|shard| shard.path().join(&file_name)) - .find(|candidate| candidate.is_file()) -} - /// ADR-0003: the delayed possession check penalises a responsible peer that /// does NOT hold the chunk, and leaves a peer that DOES hold it unpenalised. /// @@ -916,7 +851,7 @@ async fn possession_scheduler_penalises_absent_close_peer_after_delay() { // Trigger fresh replication; the engine enqueues the possession check, which // fires ~200-500 ms later and penalises the absent close peers. - let dummy_pop = [0x01u8; 64]; + let dummy_pop = dummy_payment_proof(); engine_a .replicate_fresh(&address, content, &dummy_pop) .await; @@ -1008,20 +943,13 @@ async fn full_close_group_node_rejects_replica_and_is_penalised_as_absent() { let (content, address) = candidate.expect("find key where full node is a responsible close-group peer"); - for idx in 0..harness.node_count() { - if let Some(protocol) = harness - .test_node(idx) - .and_then(|node| node.ant_protocol.as_ref()) - { - protocol.payment_verifier().cache_insert(address); - } - } + harness.prepopulate_payment_cache_everywhere(&address); - let dummy_payment_proof = vec![DUMMY_PAYMENT_PROOF_BYTE; DUMMY_PAYMENT_PROOF_LEN]; + let dummy_proof = dummy_payment_proof(); let offer = FreshReplicationOffer { key: address, data: content.clone(), - proof_of_payment: dummy_payment_proof.clone(), + proof_of_payment: dummy_proof.clone(), }; let response = send_replication_request( checker_p2p, @@ -1077,7 +1005,7 @@ async fn full_close_group_node_rejects_replica_and_is_penalised_as_absent() { let trust_before = checker_p2p.peer_trust(&full_peer); checker_engine - .replicate_fresh(&address, &content, &dummy_payment_proof) + .replicate_fresh(&address, &content, &dummy_proof) .await; let deadline = tokio::time::Instant::now() + PROPAGATION_TIMEOUT; @@ -2289,7 +2217,7 @@ async fn test_fresh_offer_with_mismatched_content_address_rejected() { let offer = FreshReplicationOffer { key: wrong_address, data: content.to_vec(), - proof_of_payment: vec![0x01; 64], + proof_of_payment: dummy_payment_proof(), }; let msg = ReplicationMessage { request_id: 1001, @@ -2515,16 +2443,10 @@ async fn scenario_1_and_24_fresh_replication_stores_and_propagates_paid_list() { // Pre-populate payment cache on ALL nodes so receivers accept the offer // (bypasses EVM verification, which is unavailable without Anvil). - for i in 0..harness.node_count() { - if let Some(node) = harness.test_node(i) { - if let Some(p) = &node.ant_protocol { - p.payment_verifier().cache_insert(address); - } - } - } + harness.prepopulate_payment_cache_everywhere(&address); // Trigger fresh replication (sends FreshReplicationOffer + PaidNotify) - let dummy_pop = [0x01u8; 64]; + let dummy_pop = dummy_payment_proof(); if let Some(ref engine) = source.replication_engine { engine.replicate_fresh(&address, content, &dummy_pop).await; } @@ -3077,16 +2999,10 @@ async fn scenario_24_fresh_replication_propagates_paid_notify() { // Pre-populate payment cache on ALL nodes so receivers accept the offer // and PaidNotify (bypasses EVM verification, unavailable without Anvil). - for i in 0..harness.node_count() { - if let Some(node) = harness.test_node(i) { - if let Some(p) = &node.ant_protocol { - p.payment_verifier().cache_insert(address); - } - } - } + harness.prepopulate_payment_cache_everywhere(&address); // Trigger fresh replication (includes PaidNotify to PaidCloseGroup) - let dummy_pop = [0x01u8; 64]; + let dummy_pop = dummy_payment_proof(); if let Some(ref engine) = source.replication_engine { engine.replicate_fresh(&address, content, &dummy_pop).await; } @@ -3265,14 +3181,7 @@ async fn scenario_26_paid_list_majority_repairs_missing_replica_below_storage_qu .await .expect("put source record"); - for idx in 0..harness.node_count() { - if let Some(protocol) = harness - .test_node(idx) - .and_then(|node| node.ant_protocol.as_ref()) - { - protocol.payment_verifier().cache_insert(address); - } - } + harness.prepopulate_payment_cache_everywhere(&address); for idx in 0..PAID_REPAIR_CONFIRMING_NODES { let engine = harness @@ -3511,17 +3420,11 @@ async fn test_late_joiner_replicates_responsible_chunks() { } for (address, _) in &chunks { - for i in 0..harness.node_count() { - if let Some(node) = harness.test_node(i) { - if let Some(protocol) = &node.ant_protocol { - protocol.payment_verifier().cache_insert(*address); - } - } - } + harness.prepopulate_payment_cache_everywhere(address); } // Trigger fresh replication for each chunk so they spread to close groups. - let dummy_pop = [0x01u8; 64]; + let dummy_pop = dummy_payment_proof(); { let source = harness.test_node(source_idx).expect("source node"); if let Some(ref engine) = source.replication_engine { diff --git a/tests/e2e/testnet.rs b/tests/e2e/testnet.rs index eca97c84..b97a6c0c 100644 --- a/tests/e2e/testnet.rs +++ b/tests/e2e/testnet.rs @@ -1359,20 +1359,21 @@ impl TestNetwork { } // Start replication engine for this node. A node without an identity - // skips ONLY the engine (no early return — the node must still be - // tracked in `self.nodes` below, or its already-started P2P/protocol - // tasks would keep running untracked by the harness). - if let (Some(ref p2p), Some(ref protocol), Some(ref id)) = - (&node.p2p_node, &node.ant_protocol, &node.node_identity) - { + // or a fresh-write channel skips ONLY the engine (no early return — + // the node must still be tracked in `self.nodes` below, or its + // already-started P2P/protocol tasks would keep running untracked by + // the harness). `create_node` fills the channel and each node is + // started once, so it is missing only if that invariant breaks. + let fresh_write_rx = node.fresh_write_rx.take(); + let has_fresh_writes = fresh_write_rx.is_some(); + if let (Some(ref p2p), Some(ref protocol), Some(ref id), Some(fresh_rx)) = ( + &node.p2p_node, + &node.ant_protocol, + &node.node_identity, + fresh_write_rx, + ) { let shutdown = CancellationToken::new(); let repl_config = self.config.replication_config.clone().unwrap_or_default(); - // `create_node` fills this and each node is started once, so it - // is always there; a closed channel would only idle the drainer. - let fresh_rx = node - .fresh_write_rx - .take() - .unwrap_or_else(|| tokio::sync::mpsc::unbounded_channel().1); let node_identity = Arc::clone(id); match ReplicationEngine::new( repl_config, @@ -1411,6 +1412,12 @@ impl TestNetwork { "Node {} has no identity; skipping replication engine", node.index ); + } else if !has_fresh_writes { + warn!( + "Node {} has no fresh-write channel (started twice?); skipping \ + replication engine", + node.index + ); } debug!("Node {} started successfully", node.index); From 5149d1142be2988e9d18215e8a058a18583836bb Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:04:11 +0200 Subject: [PATCH 23/24] docs(adr-0017): what the envelope still copies, the permit's lifetime, and the new tests - "A fetched chunk is encoded and decoded in one copy each" overclaimed: that holds for the replication message, but a fetch answered over request/response is wrapped in saorsa-core's envelope, which still serializes and decodes the payload per byte. Say so, and that the wire-identical fix belongs in saorsa-core. - The pending-offer permit is released with the last handle to the encoded bytes (`Bytes::from_owner`), the proof moves into the offer, chunk reads are exactly sized, the exact encoder is shared, and client GET responses are not copied before the send. - Validation lists the new unit tests, the fault-timed retry test and the capacity driver's switch to the production channel. Deep-review follow-up (K11). Co-Authored-By: Claude Opus 5.5 --- ...ounded-fresh-offers-and-copy-free-sends.md | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md index c44fa8df..ee76da8a 100644 --- a/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md +++ b/docs/adr/ADR-0017-bounded-fresh-offers-and-copy-free-sends.md @@ -76,7 +76,8 @@ make the send path hand a single owned buffer down to the QUIC stream: offer dispatcher. The dispatcher is the only permit-gated stage: it acquires a `MAX_PENDING_FRESH_OFFERS` (8) permit before it reads the chunk back from storage and encodes it; the permit lives with the encoded - offer until the last per-peer send drops it. A backlog therefore waits as + offer until the last handle to its bytes is dropped, whether a per-peer + send's or the transport's. A backlog therefore waits as small queued events, and at most ~40 MiB of encoded offers exist per node. Nothing is dropped by back-pressure: both queues are unbounded and FIFO, and every offer is dispatched with the same fan-out, retries and delayed @@ -100,14 +101,18 @@ make the send path hand a single owned buffer down to the QUIC stream: attempts within seconds. The backoff spreads them over about a minute, and a store-wide fault shorter than that costs no offers. Only a chunk that is no longer stored is skipped without retry. -- The chunk moves into the offer rather than being copied, and - `ReplicationMessage::encode` serializes into an exactly-sized buffer. The +- The chunk and its proof move into the offer rather than being copied, and + `ReplicationMessage::encode` serializes into an exactly-sized buffer + (shared with the WebRTC browser path as `codec::encode_exact`). A chunk is + read from disk into a buffer sized from its file, not grown to twice its + size. The chunk-carrying fields — the offer's data and proof, `PaidNotify`'s proof, `FetchResponse::Success::data` and a subtree slice's `bao_slice` — are byte strings (`serde_bytes`), which postcard lays out exactly like a `u8` sequence: one copy each way instead of a per-byte loop, and an exactly-sized buffer on decode. -- The encoded offer is shared as `Bytes`; saorsa-core's `send_message` +- The encoded offer is shared as `Bytes` (`Bytes::from_owner`, owning the + buffer and the permit); saorsa-core's `send_message` accepts `impl Into`, frames the payload through a borrowing `WireMessageRef` (byte-identical to `WireMessage` on the wire) into an exactly-sized frame, and passes that frame as `Bytes` to @@ -124,8 +129,13 @@ make the send path hand a single owned buffer down to the QUIC stream: 298 MiB (mimalloc build) and from 674 MiB to 262 MiB (jemalloc build) after the backpressure change alone. - Every large send node-wide (chunk GET responses included) stops paying - for a second copy of its frame during the transfer, and a fetched chunk is - encoded and decoded in one copy each. + for a second copy of its frame during the transfer, and a client GET + response is handed to the transport without being copied first. A + replication fetch response's own encoding and decoding are one copy each; + when it is answered over request/response, saorsa-core's envelope around + it still serializes the payload per byte into a growing buffer and + decodes it the same way (a `serde_bytes` payload there would be + wire-identical, and is left for saorsa-core). - No wire, storage or API break: `send(&[u8])` remains and copies once as before; `Vec` callers of `send_message` convert without copying. @@ -178,18 +188,23 @@ make the send path hand a single owned buffer down to the QUIC stream: ## Validation - Unit tests: exact-capacity encoding of chunk-sized offers and - exact-capacity decoding of fetched chunks; wire equivalence of every - byte-string field with its `u8`-sequence layout, both directions, across - the varint length boundaries (ant-node); byte-for-byte equivalence of - `WireMessageRef` with `WireMessage` (saorsa-core). + exact-capacity decoding of fetched chunks; exact-capacity chunk reads; + the read-back retry schedule; the pending-offer permit outliving every + handle to the offer; wire equivalence of every byte-string field with its + `u8`-sequence layout, both directions, across the varint length + boundaries (ant-node); byte-for-byte equivalence of `WireMessageRef` with + `WireMessage` (saorsa-core). - E2E tests over the real harness, whose nodes wire the PUT handler to the fresh-write pipeline as a node does: a PUT through the handler replicates and a missing chunk queued ahead of it is skipped; with the send stage held, a burst three times the budget encodes exactly `MAX_PENDING_FRESH_OFFERS` offers, then all of them once sends resume, and returns every permit; a failed read-back releases its permit, does - not delay a healthy write behind it, and is offered once the fault - clears; a chunk corrupted on disk is quarantined and never offered. + not delay a healthy write queued behind it (timed from the fault), and is + offered once the fault clears; a chunk corrupted on disk is quarantined + and never offered. The capacity driver feeds the same fresh-write channel, + so it measures receiver admission under the production drainer's pacing. + Each pipeline test fails against a build with its fix reverted. - Testnet evidence (2026-09-21): with the backpressure change, the node that had reached 1051 MiB live memory stayed flat at 0.0 MiB/min with a 150 MiB peak, and the worst bootstrap's queued offers dropped from 101 From 289b89d22791ab0980bdd47905a300982e38d3ca Mon Sep 17 00:00:00 2001 From: Chris O'Neil Date: Thu, 1 Oct 2026 22:17:21 +0100 Subject: [PATCH 24/24] chore: refresh Cargo.lock for the rebased send-path pins The rebase onto main moved the saorsa-core and saorsa-transport revs this branch patches; the lock now names the rebased commits. ant-protocol stays on the published 3.1.0 that main took as its release baseline. Co-Authored-By: Claude Opus 5 --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c41fcea7..2c85fc5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5306,7 +5306,7 @@ dependencies = [ [[package]] name = "saorsa-core" version = "0.28.0" -source = "git+https://github.com/WithAutonomi/saorsa-core?rev=b18e3754fc4626977079e57c5f2bc449edb52680#b18e3754fc4626977079e57c5f2bc449edb52680" +source = "git+https://github.com/WithAutonomi/saorsa-core?rev=7f815141ff8d5e07f443c25c99607586fa7099cf#7f815141ff8d5e07f443c25c99607586fa7099cf" dependencies = [ "anyhow", "async-trait", @@ -5374,7 +5374,7 @@ dependencies = [ [[package]] name = "saorsa-transport" version = "0.37.0" -source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=5da617375d5e3949277b1355c69012c0303f866c#5da617375d5e3949277b1355c69012c0303f866c" +source = "git+https://github.com/WithAutonomi/saorsa-transport?rev=6a772bd3cf806e381cf5706cfe34492bede3141e#6a772bd3cf806e381cf5706cfe34492bede3141e" dependencies = [ "anyhow", "async-trait",