Initial BTP-U implementation.

[ek345]

A pure-Rust implementation of the Bundle Transfer Protocol -
Unidirectional (draft-ietf-dtn-btpu) that also frames the messages of
the FEC extension (draft-ietf-dtn-btpu-fec); section references follow
-04 and -02 respectively, pinned once in the crate docs. BTP-U sits
between the Bundle Protocol and a frame-based convergence layer (UDP,
CCSDS, AOS, broadcast radio) and provides segmentation and transfer
windowing without requiring a return channel. The protocol's message
repetition and interleaving are not implemented by this sender yet, and
no FEC scheme is implemented; both are stated in the docs.

Pure protocol library: #![no_std] + alloc, no dependency on hardy-bpa,
hardy-bpv7, or any async runtime. Public modules with visibility at the
definition; module-scoped thiserror enums (Clone + PartialEq + Eq) with
a Result alias per module, so no call site carries error variants it
cannot produce. Faults split by trust boundary: processing untrusted
wire input surfaces events, never errors; Result is reserved for
trusted local operations (constructor validation, enqueue).

• codec: the wire-format layer, with header/hint/message submodules.
  decode_pdu is a lazy MessageIter over a zero-copy receive path with
  two-tier fault containment: a malformed interior is skipped via the
  header length and iteration continues, while a framing fault
  (truncated header, length past the buffer, encapsulated bundle of
  unknown extent) stops the walk, keeping everything already parsed;
  is_exhausted() distinguishes the two. decode_pdu_with takes
  DecodeOptions: fec interprets the four provisional FEC type values
  (0x70..=0x73, Private Use per Section 12.1, so off by default and
  otherwise relayed as Message::Unknown), and bundle_extent takes a
  BundleExtent hook through which a caller that parses bundle formats
  lets the decoder delimit bare bundle frames and encapsulated bundles
  (Section 7.3): with it, link padding is trimmed and iteration
  continues past the bundle; without it, a bare frame is the whole PDU
  and a mid-PDU bundle is terminal, and the padding pitfalls of that
  fallback are documented. Unknown message types relay byte-exact,
  flags nibble included; encoding refuses an Unknown carrying a defined
  or reserved type. Hint chains fold to one item per type while
  decoding (bounding a chain of repeats to 128 items), a malformed
  Bundle Length hint is carried as an unknown hint rather than failing
  its message, decode_hints slices by offset (no aliasing
  precondition), encode_header returns Result instead of truncating,
  and pad_pdu chains maximum-size Definite Padding messages so every
  target length is reachable with truthful headers. Transfer Segment
  and Transfer End share one content struct (the End is the final
  segment), and the four FEC types share two (pre-agreed vs explicit),
  so nothing is matched twice.
• transfer: TransferWindow (wraparound-safe, with reset for a known
  sender restart) and TransferNumberAllocator, sized by a WindowSize
  newtype enforcing the Section 5 range (4..=4095). The allocator keeps
  outstanding numbers in allocation order and gates on their span, not
  their count (Section 5 sender MUST). The random-restart acceptance
  hazard the draft leaves open is documented.
• sender/receiver: enqueue + segmentation + PDU packing; reassembly +
  window expiry + cancellation.

Configuration is SenderConfig / ReceiverConfig, structs of validated
newtypes (PduSize, WindowSize, MaxBundleSize, SendQueueDepth) plus
LinkFraming and the FEC switch, all defaulted, so invalid sizes are
TryFrom errors at the edge, no constructor panics, and everything that
shapes queued data is fixed before anything is queued. The newtypes
(also MaxSegments and MaxRetainedBytes on the receive side) follow the
core::num::NonZero shape, and every TryFrom and FromStr returns the
shared OutOfRange and ParseError. PduSize spans one message header (MIN)
to the 20-bit content-length ceiling (MAX), so anything enqueue accepts
can always be drained.

Receiver: receive_pdu is infallible; every framing and semantic fault is
an event (MalformedMessage/MalformedPdu for decode faults,
MessageDropped/TransferRejected/BundleRejected for dispositions), so a
fault late in a PDU never discards the events of the prefix before it;
receive_pdu_into refills a caller-owned event list instead. The core and
FEC transfer messages share one pipeline (admit to the window, check the
transfer kind, check segment indices, apply, enforce limits), and
completeness is checked on every segment insert. Benign drops
(out-of-window, repeat of a delivered/cancelled/rejected transfer,
Cancel for a transfer not in progress per Section 8.4, segment-sequence
conflicts) surface as MessageDropped data. A transfer the receiver
refuses (too large, too fragmented, FEC/core mixing, a changed FEC
configuration, empty, receiver full) is reported once as
TransferRejected with its RejectReason, and its later messages are
dropped as DropReason::Rejected with the same reason. A transfer that is
over stays over for the life of the window: delivered, cancelled, and
rejected numbers are remembered, so a Section 6 repeat never re-delivers
a bundle or re-opens a phantom transfer, and a Cancel for any in-window
number is honoured even before its segments arrive (Section 5 defines
in-progress by the window range; Section 8.4 makes the later segments
discardable). Memory is bounded by what is retained: the mandatory
MaxBundleSize (default 1 GiB) polices each transfer's bundle bytes
exactly (TooLarge, also on the Bundle Length hint, FEC transfers
included) and budgets its bookkeeping separately (TooFragmented:
SEGMENT_OVERHEAD per stored segment plus retained hint bytes, against
the cap or a 4 KiB floor), so a cap-sized bundle is delivered however it
is segmented while a flood of tiny segments or hint data is still
bounded; segments shorter than half their PDU and retained hint values
are copied out so no PDU is pinned by a fragment. An optional
MaxSegments replaces the per-segment charge with a direct segment count
(MaxSegments::for_link_pdu_size derives one for links with small PDUs),
and MaxRetainedBytes bounds the state held across all in-progress
transfers, rejecting the transfer that would exceed it as ReceiverFull;
it is never less than one transfer's full allowance, which is also its
default (2 GiB with the default cap). The BundleExtent hook is a Box
borrowed by the decoder through disjoint fields, not taken and restored,
so a panicking hook cannot leave the receiver hookless. Empty segments
and an empty Transfer End are stored, since Section 4 completes a
transfer once indices 0..=N are present, so a conforming streaming
sender always completes. An empty Bundle message is rejected (Section
8.1) and a Bundle Length hint on one is dropped (Section 9.1).
BundleReceived { data, hints } carries one hint per type, most recently
received value wins; a single-segment transfer hands back its segment as
stored, without a second copy. TransferExpired events are reported
oldest first, across the 2^32 roll-over included, and an encapsulated
bundle of undeterminable extent is reported with its first byte and its
offset in the PDU. reset() discards all state for a peer restart learned
out of band.

Sender: enqueue(data, SendOptions) rejects empty bundles (Section 8.1),
returns a BundleTransferId (the transfer number for a segmented bundle,
a wrapping counter value otherwise), and attaches caller hints
(validated at the call; any caller item of the Bundle Length type is
discarded, since that hint is always sender-derived) to the Bundle
message or first segment. A segmented transfer is one queue entry: its
boundaries are fixed at enqueue against the PduSize, but its segment
messages are cut from the enqueued buffer as PDUs are packed, so a
gibibyte costs a handle rather than 720 thousand queued messages, and a
bundle needing more segments than the 32-bit index can number is
refused. The window releases itself: a transfer's slot is freed when its
End is packed by next_pdu, since a unidirectional link offers nothing to
anchor an explicit completion call to; there is no complete().
cancel(BundleTransferId) reports whether it cancelled anything: for a
transfer it frees the slot early and queues a Transfer Cancel only if
part of the transfer was emitted; a Bundle Message or bare frame still
queued is simply removed. next_pdu returns a Pdu: Bytes sized to its
content (sharing a bare frame's buffer rather than copying it), with a
Carried entry for every bundle it holds bytes of, flagged when it holds
the last, so a CLA can report per-bundle outcomes; next_pdu_into fills a
caller-owned list instead, and nothing is allocated when idle.
LinkFraming is set per link: FixedSize (default) pads every PDU to the
PduSize; Variable leaves PDUs unpadded and may emit a fitting,
hint-free, bundle-typed bundle as a bare bundle frame. Bare frames go
through the same pending queue as everything else, so they keep arrival
order and count against SendQueueDepth, which counts queue entries and
is an admission gate applied by poll_ready and is_send_queue_full, never
by enqueue. Sender's Debug summarises configuration and queue rather
than printing queued bundle bytes.

FEC messages carry a single scheme-opaque payload, so
decode(encode(m)) == m holds with no scheme registered; the earlier
unused FecScheme trait is removed until a scheme exists to shape it.

Optional features (all default-off): serde (the config structs with
kebab-case keys and defaults, newtypes as plain integers re-validated on
deserialize), rand (try_from_rng and from_rng constructors on rand_core
0.10, from a fallible RNG such as SysRng or an infallible one),
critical-section (builds on targets without atomic compare-and-swap,
such as Cortex-M0, by moving bytes onto portable-atomic), tower
(Service with From for Sender, an infallible
Service for Receiver, and a Stream<Item = Pdu> PDU drain;
poll_ready gates on window span and send-queue depth, every parked
producer is woken so several may share one Sender through a mutex, and
the shared-mutex contract is documented and tested: poll_ready and call
under one lock hold, the lock released before parking, since admission
is not reserved between them; tower::buffer::Buffer is documented as
unsuitable).

Tests: 15 inline unit tests (receiver, sender, and transfer
internals), 245 integration tests under tests/ with all features (219
with default features; one file per subject, shared fixtures in
tests/common, whole-event-list and typed-error assertions, no tokio),
and 2 doctests (1 with default features), the README example among
them (README.md is now in the Rust CI path filter for that reason):
262 in all, 235 with default features. Fuzz targets under btpu/fuzz
drive the decoder (with re-encode and byte-exact relay checks across
the FEC and extent-hook options) and Receiver::receive_pdu. The crate
is added to the thumbv7em-none-eabihf no_std CI job, and a
thumbv6m-none-eabi job builds it with critical-section. clippy -D
warnings is clean across the feature matrix (no-default, serde, rand,
tower, critical-section, default, all) and cargo doc -D warnings is
clean with and without features.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

[ek345]

rebased to pick up some fixes for CI/CD failures. PTAL

[ricktaylor]

Erik — good stuff. I went through the lot against both drafts, and also sanity-checked the API shape against the two consumers I care about: an Ethernet CLA and, unsurprisingly, your QUBICLE unreliable service. Verdict up front: architecture and codec are right, the receiver's loss-handling path needs four fixes before merge, plus a house-style pass.

The good

• Sans-io, no_std + alloc, three clean layers, no BPA/runtime dependency — exactly the shape that lets the CLA own the I/O. It holds up on paper against both link types.
• Wire format verified against btpu-02 and the FEC draft: header, hints-before-content, transfer fields, hint type/H packing, padding rules all check out. Zero-copy Bytes discipline is consistent.
• frame_kind is a nice touch, and on Ethernet it composes beautifully: NIC min-frame zero-padding decodes as indefinite padding and just vanishes.
• 76 tests, byte-level wire assertions, reserved-range sweeps, clippy/fmt clean.
• You out-implemented the spec in one spot: the diff != 0 guard in is_new_transfer deviates from Figure 2 — correctly, because Figure 2 classifies a repeat of GREATEST as "new". That's a bug in my pseudocode, not your code; fix is queued for the next rev, at which point you're conformant as written.

Blockers — all in the receiver's loss path

1. Out-of-order completion never fires. Only process_transfer_end checks is_complete() (receiver.rs:195); End-before-late-segment — the normal recovery sequence under §6 repetition, and routine reordering under QUIC datagrams — leaves the transfer stuck until window expiry. Check completeness on every insert once final_segment_index is known. The out_of_order_segments test (receiver.rs:406) currently asserts the gap — looks like you spotted it and parked it.
2. Cancelled transfers resurrect. §4.2 MUST: a repeated segment after a Cancel re-creates the entry via entry().or_insert_with. Needs a cancelled-set bounded by the window — I suspect that's what the never-constructed Error::TransferCancelled was reaching for.
3. Cancel of an unknown transfer isn't ignored (§8.4 MUST). process_transfer_cancel (receiver.rs:266) emits TransferCancelled for never-seen numbers.
4. Unbounded segment accumulation. max_bundle_size is only enforced post-reassembly. Enforce it on accumulated bytes as segments arrive, and the BundleLength hint is sitting right there for early rejection — stored, never read.

Should fix

• MAX_WINDOW_SIZE = 4096 (transfer.rs:9); §5 says less than 2^12, so 4095.
• Validate SenderConfig: pdu_size > ~1 MiB lets enqueue exceed the 20-bit length and next_pdu panics on the .expect() at sender.rs:267.
• FEC decode never populates source_fec_payload_id/fssi (scheme-defined boundaries, fair) but the struct shape claims otherwise and decode(encode(m)) ≠ m. Collapse the decoded form to one opaque Bytes until a scheme is registered — FecScheme already has the size accessors for a scheme-aware decode later.
• Re-encoding Message::Unknown drops the H flag (codec.rs:461) — corrupts relayed unknown messages with hints. Related: hints parse before type dispatch, so a malformed hint chain in an unknown/padding message errors the whole PDU instead of being ignored (§8.5).
• value.len() as u8 (hint.rs:63) and hint_type << 1 silently truncate — error instead.
• Sender::complete() ignores its argument and cancel() releases unconditionally — bogus/duplicate calls free slots that were never allocated. Minimal fix is fine; complete() probably dies in the follow-on API work anyway.
• Dead: Error::TransferOutsideWindow, Error::TransferCancelled, the tracing dependency.
• rand feature pins rand_core 0.6; your own dev-dep rand 0.9 can't drive from_rng, and the workspace is on 0.10 (bpv7).

House style

Mechanical, but the repo is consistent about these:

• pub mod codec / transfer / sender / receiver / message instead of private modules + root re-exports; only Error gets flattened. Visibility at the definition, not via re-export plumbing.
• With modules public, split the root Error — codec vs transfer/sender (cf. tcpclv4's codec::Error). Half the variants are impossible at any given call site today.
• No // ----- section ----- banners.
• use statements in one contiguous block, no blank lines.
• Config: add rename_all = "kebab-case" next to serde(default), and put defaults in the field docs (pdu_size's 1500 is undocumented).
• README is 313 lines against a 60–100 crate norm and mostly duplicates the rustdoc — trim, keep detail in lib.rs.
• The stream-of-consciousness comments in receiver.rs:406-415 go with fix #1.

Heading

So the API nits above have context: tranche 2 on Sender/Receiver gets driven by the Ethernet and QUBICLE consumers — next_pdu(max_len) with pack-time segmentation (your datagram limit is dynamic; a fixed pdu_size with eager segmentation strands queued segments when the path MTU drops), padding policy (Fixed/min-length/none — datagrams want none), §4.1 priority interleaving, repetition as the single loss knob per §6, automatic window release on drain. None of that is this PR's problem; the codec and window layers underneath it are close to ready as-is.

Fix the four receiver items, run the style pass, and this merges. Thanks Erik.

[ek345]

🙏

Latest push has attempts at fixes for all of these.

I have yet to review the change in full myself, but let's see what GitHub warns me about here...

[sylvain-pierrot]

Hello Erik, AMAZING job!!

This review mainly focuses on Rust idioms and best practices, so most comments are suggestions rather than blockers; feel free to push back on any of them. 🙂

Additionally, I would suggest moving the tests that only use the public API out to tests/, from what I can see only a couple genuinely need private access.

Let me know if anything is unclear

[ek345]

more feedback from Rick:

Code Review — PR #529 feat: add hardy-btpu crate

Reviewed at commit db129a5d (branch base 6d6d0a31). Build, all 98 tests, clippy (--all-features --all-targets), and the --no-default-features (no_std) build are green. Findings are verified against the code and against draft-ietf-dtn-btpu and draft-ietf-dtn-btpu-fec.

A clean, well-documented, well-tested #![no_std] crate; the findings below are real defects, not style. The headline is architectural: the receiver handles each PDU all-or-nothing rather than message-by-message, and three of the correctness findings (#2, #4, #5) are symptoms of that one choice rather than independent bugs. The rest stand on their own.

Root cause — a PDU is handled all-or-nothing, not message-by-message

The receiver is PDU-at-a-time, and all-or-nothing at two stacked boundaries:

1. Decode — codec::decode_pdu (codec.rs:56) eagerly walks the whole PDU into a Vec<Message>, and any ? in that loop (a malformed hint chain, a length past the buffer, a mid-PDU reserved byte) discards the entire vec — including messages already parsed before the bad one.
2. Process — receive_pdu (receiver.rs:199) then runs for msg in messages { process_message(msg)? }; the ? aborts on the first semantic error, discarding ReceiverEvents already produced — including a BundleReceived whose transfer state has already been consumed.

This fights the wire format. BTPU's common message header exists so the stream is self-framing and walkable one message at a time; §7.3 is written in exactly those terms — skip the message, continue processing subsequent messages, and only "MUST NOT process the remainder" when the message extent genuinely can't be determined. That is a streaming, message-at-a-time contract; the code materialises the batch and fails it as a unit, turning every per-message fault into a per-PDU fault.

The layered shape the format asks for has three tiers, each owning the faults it can recover from:

• Framing (bytes → message extents): incremental; the header Length gives the next boundary. It fails only when it can't find that boundary — truncated header, length past the buffer, or an unimplemented encapsulated bundle (§7.3). Those stop the walk, but keep what already parsed rather than discarding it.
• Message decode (extent → Message): a malformed interior (e.g. a bad hint chain) is contained to that one message, because its extent is already known. Drop that message, continue.
• Semantics (Message → state + events): FecCoreMixing, oversize, etc. are per-message dispositions that say nothing about the next message's framing. They become a MessageDropped/TransferRejected event and processing continues — never a ? out of the loop.

Concretely: decode_pdu returns an iterator (impl Iterator<Item = Result<Message, …>>) the receiver drives, deciding skip-vs-stop per item and emitting as it goes. Two things fall out for free:

• The §7.3 skip-and-continue vs MUST-NOT-process-the-remainder distinction is the resyncable-vs-terminal error split — you stop modelling it by accident.
• The per-PDU Vec<Message> allocation disappears.

And the signature moves toward receive_pdu(pdu) -> Vec<ReceiverEvent> — infallible at the PDU level, because every framing/semantic fault is expressible as an event. The current Result<Vec<…>, Error> return is the batch philosophy encoded in the type: it is what lets ? throw away good events. Once faults are events there is nothing to ? on, and #2 becomes structurally impossible rather than a bug to patch.

On ambition: PDUs are bounded link-layer frames, so laziness isn't needed for memory — the load-bearing fix is per-message fault containment (decode-as-far-as-you-can, plus per-message drop events), which can be done while still returning a Vec. The full iterator/streaming layering is the cleaner expression of the same idea and hands you the allocation win; the correctness rests on the fault boundary, not the laziness.

Findings #2, #4, and #5 below are all instances of this; fixing the architecture removes them as a class. #1 and #3 are independent. This is a receive-side concern — the send path is push-in / pull-out and, being built from trusted local data, has no equivalent fault-isolation problem (see Send path — future work).

High — correctness

1. is_complete overflows u32 on a wire-supplied index → panic (receiver.rs:150)

let expected = n + 1; where n = final_segment_index comes straight off the wire (codec.rs:276, unbounded). A single TransferEnd with segment_index = 0xFFFFFFFF reaches complete_if_ready → is_complete: debug/test builds panic (arithmetic overflow) — a one-packet remote DoS; release wraps to 0 so the transfer can never complete and holds a window slot until expiry. Fix: guard the increment with n.checked_add(1) and treat None as not-complete — a segment_index of u32::MAX can't be a valid final index (it implies 2³² segments), so a bogus one is correctly rejected, and debug and release agree instead of one panicking and the other wrapping to 0.

2. receive_pdu discards already-produced events — including delivered bundles (receiver.rs:203)

Symptom of the root cause — process boundary. The ? on process_message throws away the whole events vec. Trigger: one PDU = [TransferEnd completing A][FEC message colliding with in-progress core transfer B]; process_message returns Err(FecCoreMixing) after A was already completed (removed from self.transfers, BundleReceived pushed), so bundle A is gone from state and its event is dropped — silent, unrecoverable loss. Fix per the root cause: turn a per-message fault into a MessageDropped-style event instead of ?-ing out of the loop — concretely, FecCoreMixing becomes a DropReason rather than a receiver::Error (see Error model — receive vs send).

3. Unbounded per-transfer reassembly buffer with the default config (receiver.rs:298, default at receiver.rs:41)

ReceiverConfig::default() sets max_bundle_size: None, and gate_oversize is then a no-op. An attacker sends segments with distinct segment_index values (never an End) for up to window_size transfers; InProgressTransfer.segments / received_bytes grow without bound until OOM. A wire-facing receiver's default should be bounded — pick a sane default cap, or additionally bound segment count per transfer independent of max_bundle_size.

4. Mid-PDU encapsulated bundle / reserved byte aborts the entire PDU (codec.rs:118)

Symptom of the root cause — framing boundary, terminal error handled wrongly. Draft §7.3 (lines 309–311): type 6 and 0x80..0x9F mid-PDU are a legal encapsulated bundle — an implementing receiver determines its extent and continues; a non-implementing one "MUST NOT attempt to process the remainder", i.e. stop and keep what it already parsed. The code returns Err(ReservedMessageType), discarding every message parsed before it. The rustdoc claim (codec.rs:53-55) that "a well-formed BTP-U PDU never contains those bytes mid-stream" contradicts §7.3. Fix: treat as a terminal framing error — stop, keep prior messages — rather than failing the whole PDU.

5. A malformed hint in one known message poisons the whole PDU (codec.rs:137, hint.rs:184)

Symptom of the root cause — decode boundary, resyncable error handled as terminal. A bad BundleLength size or a truncated hint chain inside a Segment/End/Bundle makes decode_hints return Err, which propagates out of decode_pdu and drops all co-packed messages, including segments of unrelated interleaved transfers. The message boundary is already known from the header Length (§7.3 line 313 treats hints as safely ignorable), so this is a resyncable error: fault (or skip) that one message only, then continue.

Medium

6. pad_pdu skips the length check that every other encoder performs (codec.rs:519)

content_len = remaining - HEADER_SIZE is written via write_header with no check_content_length. For target_len - HEADER_SIZE > MAX_CONTENT_LENGTH (~1 MB): debug builds hit the debug_assert in encode_header (header.rs:33); release builds silently truncate the 20-bit Length field, emitting a header that lies. Sender is safe (it asserts pdu_size <= MAX_PDU_SIZE), but pad_pdu is public, returns (), and is documented as a low-level entry point. Make it Result (like encode_message) or clamp.

7. encoded_message_len and encode_message disagree for IndefinitePadding (codec.rs:289 vs :314)

message_content_len returns 0, so encoded_message_len returns HEADER_SIZE (4), but encode_message writes a single byte. Both are public and documented as the low-level packing API; a caller sizing a buffer with encoded_message_len for this variant miscounts by 3. Return 1 for that variant (there's no header).

8. Duplicate/late TransferEnd can permanently stall a transfer (receiver.rs:363)

final_segment_index = Some(m.segment_index) is set unconditionally. A second End with a lower index (or a stray segment at a high index, receiver.rs:151) makes is_complete unsatisfiable forever; the transfer never delivers and never errors, occupying a slot until expiry. Guard: keep the first valid final index (reject a second End, or one below the highest segment already seen) and surface the rejection as a MessageDropped rather than silently overwriting — per the per-message model (#2), a bogus End is a droppable message, not a silent state change.

The async findings (#9, #10, #13, and the send path) in Go/C terms. Rust futures and Streams are poll-driven: nothing runs until something polls it, and a poll that returns Pending must have stored the Waker it was handed or the task is never polled again — a lost wakeup (Go's scheduler hides this; the nearest C analog is forgetting to re-arm epoll). tower builds on that: poll_ready is a Service's admission gate — backpressure, where Go would use a bounded channel — and a Stream is a pull-sequence you poll for the next item. So: #9 is an admission gate that misses one path, leaving the queue effectively unbounded for small bundles; #13 is a test that would still pass with the wakeup broken; #10 is Buffer — tower's "move the service into a task that owns it" adapter, the closest thing to spawning a goroutine — which strands the half of the dual-role Sender it doesn't own.

9. Sender tower backpressure has a hole for the common single-PDU case (service.rs:68)

poll_ready gates only on window slots (transfers_in_progress < window_size). Bundles that fit one PDU take the unsegmented path (enqueue → Ok(None), no slot allocated), so poll_ready is always Ready for them and pending grows without bound if the Stream drain side is slower. The draft (line 321) actively recommends single-message bundles, so this is the normal path, not an edge case. The documented backpressure never engages here. Fix: gate poll_ready on the actual buffered work — a bounded pending whose depth drives backpressure — not just the transfer window; that bounded send queue is the concrete first step of the send-queue model in Send path — future work.

10. The Buffer sharing recommendation deadlocks the dual-role Sender (service.rs:35, sender.rs:69, lib.rs:69)

Sender is both a Service (enqueue) and a Stream (drain) and needs complete/cancel called on it. tower::buffer::Buffer moves the Sender into a worker task and exposes only the Service half — the Stream (PDU drain) and complete/cancel become unreachable, so slots never free and PDUs never leave. The Arc<Mutex<_>> suggestion is fine; drop Buffer from the docs (or explain the caveat).

Low

11. MessageFlags::from_nibble silently drops the 3 RFU flag bits (message.rs:125)

Only bit 0x8 (hint) is decoded/re-encoded. Relaying a Message::Unknown whose flags nibble carries a future-defined bit loses it, contradicting the "re-encoding relays the message intact" doc (message.rs:167) and the forward-compat intent (the Message Flags registry is Standards Action). Preserve the raw nibble for Unknown. This is load-bearing, not cosmetic: the enum-extensibility model (#15) relies on Message::Unknown round-tripping losslessly.

12. FEC decode/length arms and the two transfer paths are near-verbatim copies (codec.rs:183-259, receiver.rs:347)

The four FEC decode arms and their four message_content_len arms differ only in the constructor; extract a decode_fec_fields mirroring decode_transfer_fields. process_transfer_end duplicates process_transfer_segment's entire admission→oversize→completion pipeline, differing only in setting final_segment_index — a shared helper keeps their gate ordering (and any future fix) in lockstep. Also hint_value_len (hint.rs:112) re-derives the size ladder that encode_bundle_length already computes.

13. Tower tests can't catch a broken drain wakeup (tests/tower.rs:13)

poll_stream_until_idle re-polls with noop_waker_ref() in a loop, so it drains regardless of whether wake_drain ever fires; the Poll::Ready(None) arm is dead. Deleting the wake_drain() calls from enqueue/cancel would leave the suite green. Only the complete → enqueue_waker path is exercised with a real flag-setting waker.

---

Extensibility / API shape — decide before 0.1.0

These are not correctness bugs — they concern the crate's extensibility surface while the API is still unreleased. BTPX (draft-taylor-dtn-btpx: Object Correlator Hint = hint type 1, Transfer Status Message = message type 7) is future work; the goal is only that this PR not foreclose it. #14 is the one change worth making before the API stabilises. #15 is a design note on the enum extensibility model and the fix it depends on.

14. The send/receive API special-cases one hint where the codec is generic — give enqueue an extensible SendOpts and make the completion event hint-capable (sender.rs:205, receiver.rs:139, receiver.rs:392)

The codec handles hints generically — HintItem::BundleLength plus HintItem::Unknown { hint_type, value } round-trip losslessly. But the application API narrows that to exactly one hint:

• Send: enqueue(data) has no hint parameter; the sender hard-codes "attach BundleLength to segment 0" (sender.rs:205, :239).
• Receive: apply_hints keeps only bundle_length_hint and discards the rest (receiver.rs:139); InProgressTransfer retains nothing else; BundleReceived(Bytes) surfaces bytes only.

So the crate can wire-encode an arbitrary hint but can neither produce nor consume one through its public API. Both sides want lifting to the codec's generality; they differ in how, and both are worth shaping while the API is unreleased:

• Send — rather than a per-hint method (enqueue_with_hints, then enqueue_with_priority, …), give enqueue an extensible options struct: enqueue(data, opts: SendOpts), SendOpts { hints, … }, Default-constructible. It does not want #[non_exhaustive], for the same reason the wire enums don't (Bump cc from 1.0.98 to 1.0.99 #15): its hints: Vec<HintItem> field is already the catch-all — anything the sender needn't specially understand rides through as HintItem::Unknown. A structured SendOpts field is only ever added for a capability the sender actively implements (a priority/class selector, a repetition policy), so adding one is a coordinated change to BTPU and its in-workspace callers together, and the compile break on the struct literal is the useful checklist. Anticipated fields: caller hints today (the Correlator when BTPX exists) and a priority/class selector for send-queue scheduling (see Send path — future work). The sender still auto-derives BundleLength, merges it with the caller's hints, and attaches them per-segment. This composes with tower — the Service request type is generic, so it becomes Service<SendRequest> (or keep Service<Bytes> as a default-opts convenience alongside it).
• Receive — the near-irreversible side. BundleReceived(Bytes) is a tuple variant and cannot gain a correlator without a breaking change to the variant or an ugly parallel BundleReceivedWithHints (adding a new event variant is cheap; reshaping the existing BundleReceived variant is breaking either way). Make the completion event hint-capable now, e.g. BundleReceived { data: Bytes, hints: Vec<HintItem> }, or have the receiver retain the transfer's unrecognised hints and hand them up; a caller reads the Correlator out of hints when BTPX exists.

This is an altitude fix — lift the sender/receiver API to the generality the codec already has — and it is the difference between a future extension being "define a hint type" versus "re-open enqueue and the event API."

15. Enum extensibility rests on Unknown staying faithful (ties to #11)

The one forward-compat mechanism the crate actually needs is the explicit Unknown catch-all on the wire input enums (Message, HintItem), which stay exhaustive (not #[non_exhaustive]):

• Decoded inputs (Message, HintItem): an unknown message/hint is a real, actionable runtime case (skip, relay), so it earns an explicit Unknown variant. Unknown gives runtime forward-compat directly; leaving the enum exhaustive means promoting a wire type to a named variant later surfaces as a compile error at every site that treated it as opaque — the desired signal for in-workspace consumers, backstopped by Unknown so the fix can be as small as folding it into the ignore path.
• The recognised-type map (MessageType): the catch-all lives one layer out in TryFrom's Err(InvalidMessageType), so its own exhaustiveness is a feature — the codec's matches can't silently forget a type.

This depends on Unknown being a faithful safety net — unrecognised wire data must survive decode→re-encode intact — which is what makes #11 (from_nibble dropping the RFU flag bits, so Message::Unknown is not lossless) load-bearing rather than cosmetic.

The other public enums do not need a catch-all, and do not need #[non_exhaustive] as a principle: DropReason, ReceiverEvent, and the Error types are produced by the crate, never decoded, so there is no unrecognised value to represent. Whether they carry #[non_exhaustive] is optional API hygiene, and today it's applied inconsistently — DropReason and ReceiverEvent have it, the Error enums do not. By the same in-workspace checklist logic used for the wire enums, a consumer that acts per-variant wants the compile error when one is added, and a consumer that only logs never matches exhaustively anyway — so #[non_exhaustive] here either mutes a useful signal or is redundant. The consistent call is to drop it from DropReason and ReceiverEvent, leaving one rule across the crate: Unknown catch-all on wire inputs, plain exhaustive everywhere else. Keep #[non_exhaustive] only where you deliberately mean "diagnostic, don't switch on this" — and then apply it uniformly (errors included) rather than on two enums by reflex.

Tradeoff this accepts: adding a recognised wire type (BTPX type 7, real FEC codepoints, a future hint) is a breaking change for downstream that matches the input enums exhaustively. For an in-workspace consumer set that compile error is a useful checklist, not a broken contract; anyone driving the low-level codec API directly is covered by Unknown without a recompile.

---

Efficiency

16. Reassembly copies where Bytes could share (receiver.rs:160)

The crate is otherwise well served by Bytes, and this is worth stating because the copy-vs-share distinction isn't the instinct you bring from Go/C: Bytes is reference-counted, so .clone() is a refcount bump and .slice(..) / .split_to(..) return views over the same allocation. Threading a Bytes around costs nothing, where copying a Vec<u8> (or a Go []byte) is O(n). The crate already leans on this correctly — decode returns slice_ref views into the received PDU, segmentation uses data.slice(..), and unsegmented Bundles pass straight through — all zero-copy.

The one copy on the receive data path is reassemble, which put_slices every segment into a fresh BytesMut. Two levels:

• Single-segment transfers (a bundle that arrives as one End at index 0) needlessly allocate and copy — return the lone Bytes (a refcount bump) instead of rebuilding it.
• Multi-segment reassembly copies once into a contiguous buffer. That is defensible if the BPA needs contiguous bytes to parse; if it can consume a Buf chain, handing up the ordered segments (a Vec<Bytes> / impl Buf) avoids the copy entirely. A copy-once-per-delivered-bundle is cheap relative to the transfer, so this is a judgement call, not a defect — flagged only so the choice is deliberate.

The general rule the crate already follows and should keep: prefer sharing a Bytes over materialising an owned copy on any data path.

---

Forward-compat note

Finding 14 is the forward-compat lever: get the SendOpts struct and the completion-event shape right while the API is unreleased, and extensions like BTPX stay purely additive. Finding 15 is why the enum model already supports that, provided #11 is fixed so Unknown stays a faithful safety net.

Error model — receive vs send

The crate handles faults differently on the two paths, and the asymmetry is deliberate — it's why #6 recommends a Result on pad_pdu even though receive faults should become events, and the two are not inconsistent:

• Receive faults come from processing untrusted, multi-message input. Per the root cause, the model is per-message containment: a fault becomes a ReceiverEvent (MessageDropped / TransferRejected) and processing continues, so receive_pdu trends toward infallible at the PDU level. receiver::Error largely dissolves — FecCoreMixing becomes a DropReason (Bump libc from 0.2.153 to 0.2.155 #2) and framing faults become terminal-stop-with-events (Bump tokio from 1.37.0 to 1.38.0 #4). These faults are data-driven and per-message, so they belong in the event stream, not a Result.
• Send faults come from trusted, local, single operations — a mis-sized pdu_size, or pad_pdu handed a length that overflows the 20-bit field (Bump getrandom from 0.2.14 to 0.2.15 #6). These are genuine "this call cannot be done" conditions returned to a caller who can fix the call, with no untrusted stream to contain faults across, so a Result/Error is the correct, conventional shape.

So Result on the send helpers and events on the receive path reflect trusted single op vs untrusted multi-message stream — not an inconsistency.

Send path — future work

The send path is push-in / pull-out, and the pull half is already right: next_pdu() (wrapped by the tower Stream) lets the link-layer rate-control by pulling a frame when it has a TX opportunity. Two things are deferred, and neither belongs in this PR:

• A frame scheduler. pending: VecDeque<Message> is a single FIFO and next_pdu is "drain FIFO, pad." The BTPU-native scheduling it doesn't yet do — interleaving the next segment of which in-flight transfer, and repetition (re-emitting an earlier message on a slack pull instead of padding) — is a per-frame decision that can only live at the frame layer. When added it should be a small pluggable scheduler (a trait, per the workspace convention), not a hardcoded FIFO.
• Prioritised send queues. Multiplexing multiple queues by priority is policy: it belongs in the BPA policy layer above the CLA (classes / fairness / CoDel — the policy subsystem already owns this), mirroring the two-layer forwarding queue model, not as a second priority system inside a #![no_std] protocol crate.

Tower doesn't change this and is a reason to keep priority out of the wrapped Sender: Service+Stream is a single-stream pipe with one poll_ready gate, not a scheduler, and offers no prioritised-multiplex primitive (Buffer is a single FIFO and, per #10, breaks the dual-role Sender). So the Sender stays single-stream — which is exactly what maps cleanly onto Service/Stream — priority resolves above it, and the tower impls remain a thin optional adaptor over push-in/pull-out. Design the inherent API for the protocol; let tower adapt to it, not the reverse.

The only part of this that touches the PR is the enqueue signature (#14); the SendOpts priority/class field is the hook the future scheduler will read.

[ek345]

now with even more feedback taken 😁

PTAL

[ricktaylor]

Reviewed at head 8a46f8c2 (rebased on 99abb484; 16 files, +5,957/−0). Gates run in a worktree at the PR head: cargo fmt --check ✅, clippy --locked --all-targets --all-features -- -D warnings ✅, 114 unit + 10 tower integration tests ✅, --no-default-features (no_std) build ✅. Wire format and MUST/SHOULD semantics verified against the unpublished working draft in ~/work/btpu/draft-ietf-dtn-btpu.md (not the published -03), plus draft-taylor-dtn-btpx for future-allocation clashes.

Verdict: one real conformance finding to fix (#1), one narrow API-invariant hole (#2), the rest nits. Everything from the 2026-07-25 review and Sylvain's idiom pass has been genuinely and thoroughly applied — this is close to mergeable.

Prior review — all 16 findings verified fixed at this head

#	Finding	Status at 8a46f8c2
1	is_complete u32 overflow	✅ compares in u64 (receiver.rs:217), test final_segment_index_of_u32_max_does_not_overflow
2	receive_pdu discards produced events	✅ infallible receive_pdu, faults are events; test malformed_message_mid_pdu_keeps_prior_events_and_continues
3	unbounded default reassembly buffer	✅ MaxBundleSize NonZero newtype, no "unlimited", 1 GiB default; empty segments dropped so the byte cap also bounds entry count
4	mid-PDU reserved byte aborts PDU	✅ terminal framing error keeping the prefix (MessageIter), rustdoc corrected, test
5	malformed hint poisons PDU	✅ interior faults contained to one message via header length; two tests
6	pad_pdu unchecked length	✅ chains max-size Definite Padding, headers always truthful; boundary test
7	encoded_message_len vs encode_message for IndefinitePadding	✅ returns 1
8	duplicate/late End wedges transfer	✅ SegmentIndexConflict predicates in process_core_message; three tests
9	poll_ready backpressure hole for unsegmented bundles	✅ SendQueueDepth gate + send_queue_full(); test with real flag waker
10	Buffer doc recommendation deadlocks	✅ replaced by explicit "do NOT use tower::buffer::Buffer" warning (sender + service + lib docs)
11	RFU flag bits dropped	✅ MessageFlags.rfu preserved verbatim; byte-exact relay tests incl. flags nibble
12	FEC/transfer decode duplication	✅ decode_fec_fields, shared process_core_message pipeline, hint_value_len sized by the encoder itself
13	tower tests can't catch broken wakeups	✅ flag_waker tests for all four wake edges; poll_stream_until_idle panics on Ready(None)
14	SendOpts / hint-capable completion event	✅ SendOpts+SendRequest, BundleReceived { data, hints }, end-to-end hint test
15	enum extensibility model	✅ #[non_exhaustive] dropped; wire inputs keep faithful Unknown, everything else plain exhaustive with doc rationale
16	reassembly copies where Bytes could share	✅ single-segment returns the lone Bytes (pointer-equality test); multi-segment copy documented as deliberate

Sylvain's suggestions all landed too: ControlFlow, codec/ module folder, PduSize/WindowSize/MaxBundleSize/SendQueueDepth newtypes replacing config structs, lazy MessageIter, Enqueued enum, impl IntoIterator<Item = u32> on expired_transfers, DefinitePadding { len }.

Draft-side confirmations: the T = GREATEST → not new guard is now in the working draft's pseudocode (the code's diff != 0 matches it exactly), and receiver-side window eviction is now spec'd as "MUST be considered cancelled", which the expiry path implements.

Findings

1. Medium — sender window gates on transfer count, not number span (transfer.rs:203, sender.rs:361)

Working draft §5 (sender side): "MUST NOT emit any Message with a Transfer number less than or equal to the latest minus the size of the Transfer Window." Because transfer numbers are sequential, this is a constraint on the range of concurrently active numbers. TransferNumberAllocator::allocate only checks in_progress < window_size — a count. Out-of-order complete()/cancel() (routine under interleaving, which is a headline feature) lets the active span exceed the window:

• window 4; enqueue segmented transfers 0,1,2,3; cancel(3) (or complete(3)); enqueue again → transfer 4 allocated while 0,1,2 are still active. Active numbers {0,1,2,4} span five values.
• Wire violation: a later cancel(0) queues a Transfer Cancel for number 0 after transfer 4's messages — 0 ≤ 4 − 4, exactly what the MUST forbids.
• Bundle loss on reordering links: the PR body names UDP as a target CL. If the PDU carrying transfer 0's End is reordered behind transfer 4's first message, the receiver has already been forced to expire transfer 0 (4 − 4 = 0 is outside its window) and the late End is dropped OutsideWindow — the bundle is lost. A span-conforming sender keeps every active number within [greatest − window + 1, greatest], so late messages for any active transfer still land in-window. That protection is the point of the MUST, and the count gate silently forfeits it.

Fix: gate allocation on the span — refuse while next.wrapping_sub(oldest_active) >= window_size. The allocator needs to know the oldest outstanding number (TCP SND.UNA-style); the simplest shape is tracking active numbers in allocation order (a VecDeque front, wraparound-safe) rather than the current count, with complete/cancel removing and only the front advancing the window base. Test to pin: window 4, allocate 0–3, complete 3, assert allocate fails while 0 is still active; complete 0, assert allocate yields 4.

2. Low — PduSize has no lower bound; pdu_size < 4 + empty bundle livelocks the drain (sender.rs:78, sender.rs:453)

PduSize::try_from(0..=3) succeeds. enqueue(Bytes::new(), _) then takes the Bundle path (0 ≤ max_single_bundle_content = 0) and queues a 4-byte Bundle message that can never fit any PDU: next_pdu breaks before popping, pads, and returns Some(pure-padding PDU) with pending never draining — while sender.has_pending() { … } (the crate's own documented drive pattern) spins forever. The newtype exists to make invalid sizes unrepresentable; give it MIN = HEADER_SIZE (4), which restores the invariant "anything enqueued can eventually drain".

Related, same area: the draft says a Bundle Message's content "MUST be a valid Bundle" and a zero-length one "SHOULD NOT be used" — consider rejecting data.is_empty() at enqueue too, so the sender can't be driven into emitting either.

3. Nits

• Two wrong section cites against the current draft numbering: codec/mod.rs:173 and the test comment at receiver.rs:1106 cite "Section 8.5" for unknown-message skipping — that is §7.3 (Unrecognized Messages); §8.5 is Definite Padding. codec/mod.rs:186 cites "(Section 8.6)" for Definite Padding content — that's §8.5 (8.6 is Indefinite).
• Draft revision pins are stale/inconsistent: lib.rs:6 and transfer.rs:81 cite draft-ietf-dtn-btpu-02, the PR body says -03, and the working draft is newer still. Cite the unversioned draft name (as the README does) or pin the target rev in one place only.
• Glob imports: use self::message::* (codec/mod.rs:3), use crate::codec::message::* (receiver.rs:3, sender.rs:4) — the house style forbids globs outside super::* in leaf/test modules, and this is a new crate so it should comply. Same pass: the use blocks are not in the std/core/alloc → third-party → local order the style guide requires (local imports lead in every file).
• No CHANGELOG.md: every other publishable crate in the workspace carries one; add it before merge. Also Cargo.toml exclude = ["docs/"] references a directory that doesn't exist — either drop the exclude or add the per-crate design doc the workspace convention expects.
• Oversized Bundle message is dropped with zero events (receiver.rs:304): every other disposition in the receiver is observable; this one silently returns an empty vec (understandably — MessageDropped wants a transfer number a Bundle message doesn't have). Worth a dedicated event/variant so a CLA counting drops doesn't have a blind spot.
• Hint dedup asymmetry: the transfer path dedups hints by type (latest wins, documented); the Bundle-message path forwards the decoded Vec verbatim, duplicates included. Harmless, but the completion-event contract ("one item per hint type") is only true for one of the two paths.
• Test placement: the codec round-trip tests are pure public-API tests living in inline mod tests; the repo test guide places those in tests/. The receiver/sender inline tests genuinely peek at private state and are correctly placed.
• tests/tower.rs:177 hand-rolls an unsafe RawWaker vtable (~30 lines of unsafe) for a flag waker; std::task::Wake on an Arc<Flag> does the same with zero unsafe.

4. Spec-side observation (for the draft author, not the PR)

The working draft's new registry policy marks message types 0x70..0x7F Private Use, and the crate's four FEC messages (from fec-01) sit at 0x70–0x73. If the FEC extension is to stay an IETF-stream document its codepoints presumably need to move into the Standards Action range (BTPX already takes type 7 / hint 1; no clash there). Not a code defect today — the crate faithfully implements fec-01 — but the enum values will churn when the FEC draft rev lands, which is exactly the compile-break checklist the exhaustive MessageType was designed for.

Quality notes

• The receiver architecture is now precisely the shape the 07-25 review asked for: lazy framing iterator, resyncable-vs-terminal split surfaced as is_exhausted(), infallible receive_pdu, faults-as-events with the receive/send trust-boundary split documented and consistently applied. The process_core_message shared pipeline with per-message-type conflict predicates is a clean way to keep the Segment/End gate ordering in lockstep.
• Wire format re-verified against the working draft: header, flags nibble (H + 3 RFU preserved), hint TLV (7-bit type + H chain bit, 8-bit length), Bundle Length sizes 1/2/4/8 smallest-fit, padding forms, cancel MUST-ignore, cancelled-transfer MUST-NOT-resurrect, empty-segment SHOULD NOT (dropped, with the index-flood rationale documented), window formula including the T = GREATEST guard. All check out.
• 32-bit safety is respected throughout (u64 comparisons for wire-supplied values; the only as usize casts are from values bounded by the 20-bit length field).
• Test discipline is strong: byte-level wire assertions, adversarial cases (hostile End index, index floods, resurrection, reserved-byte sweeps), pointer-equality for the zero-copy claim, and real flag-waker coverage of all four wake edges. No sleeps, no timing margins.

Disposition requested

Fix #1 (with the pinning test) and #2, sweep the nits in one pass (cites, imports, CHANGELOG); #4 is Rick's call on the draft side. After that this is an approve.

[ek345]

Thank you!

I think I've processed each of these, though (4) I think didn't have anything actionable here?

[ek345]

Per Monday's video chat, having the naked bundle send path be outside the BTP-U CL sending code means they side step any prioritization the BTP-U sender might be applying. This is always possible, but it seems like a Good Idea (tm) to have a naked bundle send path within the BTP-U sending layer, so that prioritization can be applied (in future).

That change is now made in this PR.