Просмотр исходного кода

openspec/darkirc-delivery-status: remove uid and keep proposal focused on ack/nack for darkirc only as ref impl

darkfi 6 дней назад
Родитель
Сommit
e6410002aa

+ 117 - 69
openspec/changes/darkirc-delivery-status/design.md

@@ -29,15 +29,23 @@ Constraints that shape the design:
 
 
 - Outcome replies for `EventPut` with zero policy in the generic layer.
 - Outcome replies for `EventPut` with zero policy in the generic layer.
 - darkirc: durable outbound tracking, rebroadcast-first, bounded
 - darkirc: durable outbound tracking, rebroadcast-first, bounded
-  recreate; survives restart; safe with RLN on or off.
+  recreate only on explicit evidence; survives restart; safe with RLN
+  on or off.
 - Stable wire format (`u8` discriminants), no new panics on untrusted
 - Stable wire format (`u8` discriminants), no new panics on untrusted
   input.
   input.
 
 
 **Non-Goals:**
 **Non-Goals:**
 
 
 - Pull-based possession verification (`EventReq` challenges) — dropped;
 - Pull-based possession verification (`EventReq` challenges) — dropped;
-  status replies are the only signal, treated as hints whose worst-case
-  lie is bounded by attempt caps and uid dedup.
+  status replies plus ancestry references are the only signals, treated
+  as hints whose worst-case lie is bounded by attempt caps.
+- `bin/app` integration — out of scope entirely (no UI, subscriptions,
+  or send-path changes there). darkirc is the reference implementation;
+  the app consumes the same generic `receipt_pub` pipe in a follow-up
+  change that copies this logic.
+- `Privmsg` payload changes (`uid`/dedup field) — deferred (see
+  Alternatives Considered). No content serialization changes in this
+  change.
 - Status for `StaticPut`, IRC-surface receipt display, read-by-recipient
 - Status for `StaticPut`, IRC-surface receipt display, read-by-recipient
   semantics, changes to strike/flood policing or the relay path.
   semantics, changes to strike/flood policing or the relay path.
 
 
@@ -65,8 +73,13 @@ enum NackReason { TooOld = 0, NotSynced = 1, Invalid = 2, Busy = 3 }
 - Name: `EventPutStatus` (not `EventPutRep`) to avoid confusion with
 - Name: `EventPutStatus` (not `EventPutRep`) to avoid confusion with
   `EventRep`, which answers `EventReq`.
   `EventRep`, which answers `EventReq`.
 - Discriminants are explicit `u8`s; encoding stays the
 - Discriminants are explicit `u8`s; encoding stays the
-  `darkfi-serial` derive (variant tag + payload). Unknown variant tags
-  must decode-fail into a warn-and-drop, not a panic and not a strike.
+  `darkfi-serial` derive (variant tag `u8` + payload, verified in
+  `derive-internal`). Payload is 34 bytes; ~70 bytes on the wire
+  including the per-message frame (magic + command + VarInt length) —
+  ~14% of an RLN-less `EventPut`, ~2-3% of an RLN-carrying one, per
+  direct edge.
+- Unknown variant tags must decode-fail into a warn-and-drop, not a
+  panic and not a strike.
 - Payload carries only `event_id` + outcome — never channel, nick, or
 - Payload carries only `event_id` + outcome — never channel, nick, or
   content (privacy: the reply leaks nothing beyond what the event itself
   content (privacy: the reply leaks nothing beyond what the event itself
   already revealed to that peer).
   already revealed to that peer).
@@ -84,16 +97,23 @@ versioned together.
 |---|---|
 |---|---|
 | `!is_synced()` skip | `Nack { NotSynced }` |
 | `!is_synced()` skip | `Nack { NotSynced }` |
 | `main_tree` duplicate | `Has { inserted: false }` |
 | `main_tree` duplicate | `Has { inserted: false }` |
-| `timestamp < genesis_ts` | `Nack { TooOld }` |
+| `timestamp < genesis_ts` | check retained slots first: present → `Has { inserted: false }`; absent → `Nack { TooOld }` |
 | `validate_new` / structural / RLN / parent-fetch failure | `Nack { Invalid }` |
 | `validate_new` / structural / RLN / parent-fetch failure | `Nack { Invalid }` |
 | internal insert error after verification | `Nack { Busy }` |
 | internal insert error after verification | `Nack { Busy }` |
 | `insert_verified_signal` success | `Has { inserted: true }` |
 | `insert_verified_signal` success | `Has { inserted: true }` |
 
 
-`Has{inserted:false}` is load-bearing: after reconnect-and-rebroadcast it
-is the dominant positive outcome ("already propagated") and the only
-thing distinguishing it from "never arrived". Strike/flood paths keep
-current behavior and send nothing. The duplicate check sits before the
-flood-window tick, so replying there is free.
+The historical-slot check is load-bearing: the current duplicate check
+only consults the current slot's `main_tree`, so without it a peer
+holding a rotated-away message nacks `TooOld` exactly like a peer that
+never saw it — collapsing "delivered but idle" into "never delivered"
+and forcing needless recreations. Checking `dag_store`'s retained slots
+(`max_dags` window) before nacking separates the two.
+
+`Has{inserted:false}` (live and historical) is the dominant positive
+outcome after reconnect-and-rebroadcast, and the only thing
+distinguishing "already propagated" from "never arrived". Strike/flood
+paths keep current behavior and send nothing. The duplicate check sits
+before the flood-window tick, so replying there is free.
 
 
 ### D3: Dumb pipe — `receipt_pub` on `EventGraph`, policy in apps
 ### D3: Dumb pipe — `receipt_pub` on `EventGraph`, policy in apps
 
 
@@ -110,33 +130,54 @@ also keeps it out of the security-sensitive blast radius.
 ### D4: darkirc outbound table
 ### D4: darkirc outbound table
 
 
 New kvdb tree `darkirc_outbound`, key = event id, value (serial):
 New kvdb tree `darkirc_outbound`, key = event id, value (serial):
-`{ uid, event_id, plaintext Privmsg fields, created_ts, state:
+`{ event_id, plaintext Privmsg fields, created_ts, state:
 Pending|Delivered|Failed, attempts: u16, last_broadcast_ts,
 Pending|Delivered|Failed, attempts: u16, last_broadcast_ts,
 superseded_by: Option<event_id> }`. Written in `publish_events` before
 superseded_by: Option<event_id> }`. Written in `publish_events` before
 `p2p.broadcast`. Plaintext-at-rest is inside the existing local-wallet
 `p2p.broadcast`. Plaintext-at-rest is inside the existing local-wallet
 trust boundary (same kvdb that holds RLN identity secrets).
 trust boundary (same kvdb that holds RLN identity secrets).
 
 
-`uid`: 16 random bytes from `OsRng` (invariant: CSPRNG, never reused
-across logical messages, always reused across recreations of one
-message).
+The `superseded_by` chain is the sender-side correlation handle: when a
+record is recreated, the new event id links back to the old, so clients
+can map statuses for any generation onto one logical message without
+any wire-level dedup field.
 
 
 ### D5: Delivery monitor (darkirc)
 ### D5: Delivery monitor (darkirc)
 
 
 One task on `IrcServer` (started alongside the client loop):
 One task on `IrcServer` (started alongside the client loop):
 
 
 - Subscribes `receipt_pub`; per (event_id, channel-address) aggregates
 - Subscribes `receipt_pub`; per (event_id, channel-address) aggregates
-  replies; any `Has` closes the record as Delivered.
+  replies; any `Has` (live or historical) closes the record as
+  Delivered.
+- Ancestry references count as positive evidence: when a new foreign
+  event is observed whose ancestry includes a tracked outbound event
+  (local walk via the existing `get_ancestors` machinery), the record
+  closes as Delivered. Zero wire cost; works among mixed-version peers.
 - Periodic sweep (30 s) over `Pending` records, gated by
 - Periodic sweep (30 s) over `Pending` records, gated by
   `is_synced() && connection_count >= K` (K = 2, both session directions
   `is_synced() && connection_count >= K` (K = 2, both session directions
   counted via the existing session APIs).
   counted via the existing session APIs).
-- No replies → rebroadcast the original `EventPut` unchanged (event and
+- No evidence → rebroadcast the original `EventPut` unchanged (event and
   blob refetched from the local DAG + `dag_blob_fetch`), backoff
   blob refetched from the local DAG + `dag_blob_fetch`), backoff
-  doubling from 30 s, at most `R_MAX` (= 5) rounds.
-- Any `TooOld`, or all-observed-replies-are-nacks, or `R_MAX` exhausted
-  → recreate (D6).
-- Records older than the current genesis ts are recreated unconditionally
-  on the next eligible sweep (local rotation knowledge; no peer reply
-  needed).
+  doubling from 30 s, at most `R_MAX` (= 5) rounds per window, then
+  keep rebroadcasting at the slow rate.
+
+**Recreate only on explicit evidence (the complete trigger set):**
+
+1. *Window closed, no holder:* the rotation window for the record's slot
+   has closed (known locally from the rotation schedule, or indicated by
+   `TooOld` nacks) and no peer answered `Has` in the rebroadcast round →
+   recreate. This covers: offline across rotation (the canonical case),
+   sent into the void while "synced" with zero peers, and clock-skew
+   `TooOld` while we believed the window open.
+2. *Explicit rejection while open:* every observed reply is a nack and
+   at least one is `Invalid` → recreate (bounded by `A_MAX`; reasons
+   surface to the user).
+
+Never recreates on: silence (open window — this is what prevents
+mass-duplication during mixed-version rollout), `NotSynced`, `Busy`,
+or any `Has`. Silence after window close, with no status reply ever
+received from any status-capable peer, still recreates — local rotation
+knowledge is explicit evidence, and the alternative (gray forever,
+possibly lost) is worse than a rare duplicate for old-version holders.
 
 
 ### D6: Recreate procedure
 ### D6: Recreate procedure
 
 
@@ -146,56 +187,64 @@ Reuse the existing send path internals: stored plaintext →
 `reserve_rln_message_id` → `BudgetExhausted` parks the record until
 `reserve_rln_message_id` → `BudgetExhausted` parks the record until
 epoch rollover; `MissingIdentity` closes it as Failed → `create_signal`
 epoch rollover; `MissingIdentity` closes it as Failed → `create_signal`
 → `insert_signal_with_blob` → broadcast → write the new record with
 → `insert_signal_with_blob` → broadcast → write the new record with
-`superseded_by` and shared `uid`; `attempts` increments per uid and caps
-at `A_MAX` (= 2), then Failed + client notice (IRC error reply; the app
-reads the record state through its subscription).
-
-Same `uid` across original and recreations is what makes the duplicate
-rendering problem disappear for uid-aware clients.
-
-### D7: `Privmsg` version bump + `uid`
-
-`Privmsg.version` exists and is currently always 0. New payloads use
-version 1 and carry `uid`. Decoder is version-matched: v0 decodes
-without uid (treated as untracked/legacy), v1 requires it. Old peers
-that cannot decode a v1 payload simply fail content-deserialization and
-skip the event (existing behavior on undecodable content) — their DAG
-still carries it. This serialization change must be sequenced/merged
-with `darkirc-mod`'s content tag byte (same struct, same wire).
-
-### D8: App integration
-
-`bin/app` `handle_send` gains the same record-writing and rebroadcast
-hooks (its path is RLN-disabled, empty blob). Message identity for
-display dedup switches from the ciphertext-hash `msg_id()` to `uid`
-(foreign v0 messages keep a synthetic hash id). A subscription task maps
-`receipt_pub` events onto per-uid state (`sending`/`delivered`/`failed`)
-and notifies the UI; relayed (foreign) statuses are ignored except
-optionally as network telemetry.
+`superseded_by` pointing at the old; `attempts` is shared across the
+chain and caps at `A_MAX` (= 2), then Failed + client notice (IRC error
+reply to the connected client).
+
+## Alternatives Considered
+
+**Reference-based confirmation only (no wire message).** Raised in
+review: a foreign event whose ancestry includes your event proves the
+author's DAG held it, and children arrive for free via sync. Adopted as
+the secondary signal in D5. Rejected as the *primary* mechanism because:
+(i) silence is not an answer — the last message in a conversation is
+never referenced, permanently gray; (ii) the rotation cliff — no event
+edge crosses rotations (`timestamp_fits_slot` + parents must exist in
+the current slot's tree), so references die exactly at the decision
+deadline; (iii) absence is unprovable — `fetch_headers_with_tips` only
+extends the requester's frontier, so "peer holds it sterile" and "peer
+never got it" are observationally identical, and resolving them needs
+`TipReq` frontier probing (truncated at 1024 tips) which is pull-based
+verification with extra steps; (iv) no rejection reasons.
+
+**`Privmsg.uid` dedup field.** Deferred. It protects against duplicate
+renders when a recreate fires while someone already holds the original.
+With D2's historical check and D5's explicit-evidence rule, that overlap
+shrinks to: mixed-version rollout windows (old peers can't answer
+`Has`), adversarial fake-nack griefing, and recreate-of-recreate — all
+rare and cosmetic. Asymmetry decides: adding a payload field later is an
+additive `Privmsg.version` bump; removing one after shipping is a wire
+break. Revisit on field evidence of annoying duplicates.
+
+**Hold-at-send (don't broadcast with zero connections).** Candidate
+follow-up: gate `publish_events` on connection count so doomed events
+aren't created at all. Not required for correctness (D5 handles them);
+deferred as a small polish item.
 
 
 ## Risks / Trade-offs
 ## Risks / Trade-offs
 
 
 - [Fake statuses: a peer can lie `Has` (suppresses recreate → silent
 - [Fake statuses: a peer can lie `Has` (suppresses recreate → silent
-  loss) or spam `Nack` (forces recreates → budget burn)] → bounded:
-  statuses only count for ids in our own outbound table (blake3 ids are
-  unguessable to non-recipients), attempts capped at `A_MAX`, duplicates
-  deduped by uid, and a peer that has the event relays it anyway. Total
-  eclipse defeats this — accepted (an eclipsed node has larger
-  problems).
-- [Reply amplification: one reply per relay edge] → tiny fixed-size
-  unicast, traffic is RLN-rate-limited anyway, and the flood window is
-  untouched (replies are not EventPuts).
+  loss) or spam `Nack` (forces recreates → budget burn + injected
+  duplicates)] → bounded: statuses only count for ids in our own
+  outbound table (blake3 ids are unguessable to non-recipients),
+  attempts capped at `A_MAX`, and a peer that has the event relays it
+  anyway. Total eclipse defeats this — accepted (an eclipsed node has
+  larger problems).
+- [Reply amplification: one reply per relay edge] → ~70 bytes on the
+  wire against events measured in hundreds of bytes to kilobytes, on
+  RLN-rate-limited volume; the flood window is untouched (replies are
+  not EventPuts).
 - [Unknown-message compatibility: peers without `EventPutStatus`
 - [Unknown-message compatibility: peers without `EventPutStatus`
   receive an unsolicited message type] → verify the channel's behavior
   receive an unsolicited message type] → verify the channel's behavior
   on unknown message ids during implementation (test with a mixed
   on unknown message ids during implementation (test with a mixed
   version pair); if unknown ids can destabilize old channels, gate
   version pair); if unknown ids can destabilize old channels, gate
   replies until a capability flag exists. First implementation task
   replies until a capability flag exists. First implementation task
   resolves this.
   resolves this.
-- [Mixed-version rollout: old clients render a recreation as a
-  duplicate message] → transitional only; documented; uid clients are
-  unaffected.
+- [Duplicate renders without uid: mixed-version transition, adversarial
+  nacks, reply loss] → accepted, rare, cosmetic; documented in the
+  proposal's non-goals; additive fix available later if needed.
 - [Clock skew: slightly-future local timestamps can earn spurious
 - [Clock skew: slightly-future local timestamps can earn spurious
-  `TooOld` nacks] → worst case is a harmless recreate (uid dedup).
+  `TooOld` nacks] → worst case is a harmless recreate.
 - [RLN interplay: recreate with a stale slot would self-slash] →
 - [RLN interplay: recreate with a stale slot would self-slash] →
   impossible by construction: recreations go through
   impossible by construction: recreations go through
   `reserve_rln_message_id`, parking on exhaustion, never reusing a
   `reserve_rln_message_id`, parking on exhaustion, never reusing a
@@ -206,17 +255,16 @@ optionally as network telemetry.
 ## Migration Plan
 ## Migration Plan
 
 
 Additive wire message first (`src/event_graph`), verified against a
 Additive wire message first (`src/event_graph`), verified against a
-mixed-version two-node test; then darkirc table+monitor; then the
-`Privmsg` version bump (coordinated with `darkirc-mod`); then app UI.
-Rollback: the message and records are inert for old code; reverting
-leaves a harmless `darkirc_outbound` tree. The `Privmsg` version bump,
-once shipped, is wire-irreversible — hence sequencing it last.
+mixed-version two-node test; then darkirc table+monitor. No content
+serialization changes, so no coordination with `darkirc-mod` is
+required. Rollback: the message and records are inert for old code;
+reverting leaves a harmless `darkirc_outbound` tree. App integration is
+a follow-up change that reuses the same `receipt_pub` pipe and copies
+the darkirc monitor logic.
 
 
 ## Open Questions
 ## Open Questions
 
 
 - Exact values of K, `R_MAX`, `A_MAX`, sweep interval — tuning consts,
 - Exact values of K, `R_MAX`, `A_MAX`, sweep interval — tuning consts,
   safe to adjust after rollout.
   safe to adjust after rollout.
-- Whether `bin/app`'s outbound records live in its app db or a dedicated
-  tree — implementation convenience, no behavioral impact.
 - Whether taud later reuses the same policy for task events — deferred,
 - Whether taud later reuses the same policy for task events — deferred,
   out of scope.
   out of scope.

+ 42 - 35
openspec/changes/darkirc-delivery-status/proposal.md

@@ -16,8 +16,10 @@ network.
 - New p2p message `EventPutStatus` in `src/event_graph/proto.rs`: a reply
 - New p2p message `EventPutStatus` in `src/event_graph/proto.rs`: a reply
   sent by the receiver of an `EventPut` describing the outcome. Enum
   sent by the receiver of an `EventPut` describing the outcome. Enum
   variants use explicit `u8` discriminants for a stable wire format:
   variants use explicit `u8` discriminants for a stable wire format:
-  - `Has { inserted: bool }` — peer has the event (freshly inserted, or
-    already known from earlier propagation)
+  - `Has { inserted: bool }` — peer has the event (freshly inserted,
+    already known, **or found in a retained older rotation slot** — the
+    historical check runs before nacking `TooOld`, so a peer still
+    holding a rotated-away message answers `Has` instead of `TooOld`)
   - `Nack { reason }` with coarse reasons: `TooOld`, `NotSynced`,
   - `Nack { reason }` with coarse reasons: `TooOld`, `NotSynced`,
     `Invalid`, `Busy`. No fine-grained validation detail (avoids turning
     `Invalid`, `Busy`. No fine-grained validation detail (avoids turning
     nacks into a validation oracle).
     nacks into a validation oracle).
@@ -28,43 +30,49 @@ network.
   The event layer is a dumb pipe; delivery policy lives in applications.
   The event layer is a dumb pipe; delivery policy lives in applications.
 - darkirc (`bin/darkirc`):
 - darkirc (`bin/darkirc`):
   - Persistent outbound table (kvdb tree, written before broadcast):
   - Persistent outbound table (kvdb tree, written before broadcast):
-    event id, `uid`, plaintext privmsg, state, attempts.
-  - Receipt aggregation keyed by (event_id, peer channel).
-  - Delivery monitor: rebroadcast-first when reconnected and synced;
-    recreate when nacked `TooOld`, when all responses are nacks, or when
-    zero responses persist after bounded retries.
-  - Recreate = new event from stored plaintext (fresh parents/timestamp,
-    fresh saltbox nonce, same `uid`), new RLN slot when RLN is enabled
-    (`BudgetExhausted` parks the attempt until the next epoch), bounded
-    attempt count.
-- `Privmsg` gains a client-generated `uid` field (constant across
-  recreates) with a version bump, so receiving clients dedup a recreated
-  message against its original. Serialization change must be sequenced
-  with the in-flight `darkirc-mod` content-tag work.
-- `app` (`bin/app`): subscribes to `receipt_pub` in-process and maps
-  `uid` to per-message delivery state (sending / delivered / failed);
-  message identity switches to `uid`-based dedup.
+    event id, plaintext privmsg, state, attempts, `superseded_by` link.
+  - Receipt aggregation keyed by (event_id, peer channel); a foreign
+    event whose DAG ancestry includes our outbound event also counts as
+    positive delivery evidence (free secondary signal, no wire change).
+  - Delivery monitor — rebroadcast-first when reconnected and synced.
+    Silence alone never triggers recreation while the rotation window
+    is open; recreate only when the window is closed (locally or via
+    `TooOld` nacks) with no holder answering, or when every observed
+    reply is an explicit rejection. `NotSynced`/`Busy`/silence mean
+    retry, not evidence.
+- Recreate = new event from stored plaintext (fresh parents/timestamp,
+  fresh saltbox nonce), new RLN slot when RLN is enabled
+  (`BudgetExhausted` parks the attempt until the next epoch), bounded
+  attempt count, `superseded_by` chain for local correlation.
 
 
 Non-goals: read receipts stored in the event graph (pollutes the DAG,
 Non-goals: read receipts stored in the event graph (pollutes the DAG,
-burns RLN budget, leaks linkability); IRC-surface receipt display (only
-the app displays them); pull-based possession verification via
-`EventReq`; `StaticPut` status (nickserv already has a deferred-broadcast
-queue); recipient-identifying receipts (nodes are anonymous; this is
-delivery-to-network evidence only).
+burns RLN budget, leaks linkability); IRC-surface receipt display;
+`bin/app` integration (delivery-state UI, subscriptions, or any other
+app-side changes — darkirc is the reference implementation here and the
+app consumes the same generic pipe in a follow-up change); pull-based
+possession verification via `EventReq`; `StaticPut` status (nickserv
+already has a deferred-broadcast queue); recipient-identifying receipts
+(nodes are anonymous; this is delivery-to-network evidence only). A
+`Privmsg` dedup field (`uid`) was considered and deliberately deferred:
+with the historical-slot check and the silence-never-recreates rule,
+recreation almost never overlaps with "someone already rendered the
+original", and adding such a field later is an additive version bump
+while removing it after shipping would be a wire break. Known accepted
+cost: rare duplicate renders during mixed-version rollout windows and
+under adversarial fake-nack griefing.
 
 
 ## Capabilities
 ## Capabilities
 
 
 ### New Capabilities
 ### New Capabilities
 
 
 - `msg-delivery-status`: outcome reporting for `EventPut` broadcast,
 - `msg-delivery-status`: outcome reporting for `EventPut` broadcast,
-  delivery-state exposure to applications, sender-side rebroadcast/recreate
-  policy for darkirc, and client-side dedup/display of delivery state in
-  the app.
+  delivery-state exposure to applications, and the sender-side
+  rebroadcast/recreate policy implemented by darkirc as the reference
+  consumer.
 
 
 ### Modified Capabilities
 ### Modified Capabilities
 
 
-(none — `chatview` is untouched; app-side rendering is covered by
-`msg-delivery-status` requirements.)
+(none — no existing specs change.)
 
 
 ## Impact
 ## Impact
 
 
@@ -73,13 +81,12 @@ delivery-to-network evidence only).
   panic-free on untrusted input, keep flood/strike policing unchanged.
   panic-free on untrusted input, keep flood/strike policing unchanged.
 - `bin/darkirc` (client.rs, server.rs, lib.rs, crypto/rln.rs interplay) —
 - `bin/darkirc` (client.rs, server.rs, lib.rs, crypto/rln.rs interplay) —
   outbound table, monitor task, recreate path.
   outbound table, monitor task, recreate path.
-- `bin/app/src/plugin/darkirc.rs` — send path wrapping, `uid` dedup,
-  receipt subscription.
-- `bin/tau/taud` — unaffected consumer; may adopt the same pipe later.
+- `bin/tau/taud` and `bin/app` — untouched consumers; they may adopt the
+  same `receipt_pub` pipe in follow-up changes.
+- No `Privmsg` or other consensus/content serialization changes in this
+  change.
 - Wire compatibility: peers without `EventPutStatus` support simply never
 - Wire compatibility: peers without `EventPutStatus` support simply never
-  reply; senders treat silence as "no response" (drives rebroadcast
-  retries, never correctness). New `Privmsg` field requires version-aware
-  decoding.
-- Sequencing: coordinate `Privmsg` serialization with `darkirc-mod`.
+  reply; senders treat silence as "retry later, never a verdict", so
+  correctness does not depend on reply availability.
 - Per repo policy, `event_graph` protocol changes require
 - Per repo policy, `event_graph` protocol changes require
   `@anon-security-review` before apply/archive.
   `@anon-security-review` before apply/archive.

+ 85 - 62
openspec/changes/darkirc-delivery-status/specs/msg-delivery-status/spec.md

@@ -6,8 +6,8 @@ Gives senders of event-graph broadcast messages evidence about whether
 peers accepted them, and defines the sender-side rebroadcast/recreate
 peers accepted them, and defines the sender-side rebroadcast/recreate
 policy that prevents messages written while offline from being silently
 policy that prevents messages written while offline from being silently
 lost to DAG rotation. Covers the reply wire format, delivery-state
 lost to DAG rotation. Covers the reply wire format, delivery-state
-exposure to applications, darkirc's outbound tracking and recreate flow,
-and client-side dedup/display of delivery state.
+exposure to applications, and darkirc's outbound tracking, rebroadcast,
+and recreate flow as the reference implementation.
 
 
 ## ADDED Requirements
 ## ADDED Requirements
 
 
@@ -18,9 +18,13 @@ A node that receives an `EventPut` SHALL send the sender an
 
 
 - `Has { inserted: true }` when the event was newly accepted into the
 - `Has { inserted: true }` when the event was newly accepted into the
   node's DAG
   node's DAG
-- `Has { inserted: false }` when the node already has the event
+- `Has { inserted: false }` when the node already has the event,
+  **including when the event is only found in a retained older rotation
+  slot** — the receiver SHALL check its retained slots before nacking
+  `TooOld`, so a peer still holding a rotated-away message answers
+  `Has` rather than `TooOld`
 - `Nack { reason: TooOld }` when the event predates the node's current
 - `Nack { reason: TooOld }` when the event predates the node's current
-  rotation window
+  rotation window and is not present in any retained slot
 - `Nack { reason: NotSynced }` when the node is still performing its
 - `Nack { reason: NotSynced }` when the node is still performing its
   initial DAG sync and skipped the event
   initial DAG sync and skipped the event
 - `Nack { reason: Invalid }` when the event fails structural or
 - `Nack { reason: Invalid }` when the event fails structural or
@@ -47,7 +51,16 @@ distinguish originator from relay.
 
 
 - **WHEN** a peer receives an `EventPut` whose event timestamp precedes
 - **WHEN** a peer receives an `EventPut` whose event timestamp precedes
   its current genesis
   its current genesis
-- **THEN** it replies `Nack { reason: TooOld }`
+- **THEN** it replies `Nack { reason: TooOld }` only if the event is not
+  present in any of its retained rotation slots
+
+#### Scenario: Event held in a retained older rotation slot
+
+- **WHEN** a peer receives an `EventPut` for an event it holds in a
+  retained older rotation slot (for example after the sender
+  disconnected and the DAG rotated)
+- **THEN** it replies `Has { inserted: false }` rather than
+  `Nack { reason: TooOld }`
 
 
 #### Scenario: Receiving node still syncing
 #### Scenario: Receiving node still syncing
 
 
@@ -109,11 +122,11 @@ or make retry decisions; that policy belongs to applications.
 ### Requirement: darkirc persists outbound records before broadcast
 ### Requirement: darkirc persists outbound records before broadcast
 
 
 When darkirc publishes a chat message event, it SHALL first persist an
 When darkirc publishes a chat message event, it SHALL first persist an
-outbound record containing the event id, the message's `uid`, the
-plaintext message fields needed to rebuild it, a state, and an attempt
-counter. Records SHALL survive restart. Records SHALL be closed
-(removable) once a positive outcome is observed or the attempt cap is
-reached.
+outbound record containing the event id, the plaintext message fields
+needed to rebuild it, a state, an attempt counter, and a link to any
+superseding replacement event. Records SHALL survive restart. Records
+SHALL be closed (removable) once a positive outcome is observed or the
+attempt cap is reached.
 
 
 #### Scenario: Restart does not lose pending sends
 #### Scenario: Restart does not lose pending sends
 
 
@@ -140,20 +153,42 @@ record as delivered.
 #### Scenario: Prior propagation is detected, not recreated
 #### Scenario: Prior propagation is detected, not recreated
 
 
 - **WHEN** a rebroadcast reaches peers that already hold the event via
 - **WHEN** a rebroadcast reaches peers that already hold the event via
-  earlier propagation
+  earlier propagation or in a retained older rotation slot
 - **THEN** their `Has { inserted: false }` replies close the record and
 - **THEN** their `Has { inserted: false }` replies close the record and
   no recreation happens
   no recreation happens
 
 
-### Requirement: Recreate on unrecoverable or rejected delivery
+### Requirement: Ancestry reference counts as delivery evidence
 
 
-darkirc SHALL create a replacement event from the stored plaintext —
-fresh timestamp and DAG parents, fresh encryption nonce, same `uid` —
-when any of:
+The delivery monitor SHALL treat a foreign event whose DAG ancestry
+includes one of its tracked outbound events as positive delivery
+evidence, closing the outbound record as delivered. This is a local
+computation over already-received events and adds no wire traffic.
+
+#### Scenario: Foreign child closes the record
 
 
-- a `Nack { reason: TooOld }` arrives (the rotation window has passed)
-- every observed reply across the retry rounds is a nack
-- zero replies are observed after the maximum number of rebroadcast
-  rounds
+- **WHEN** a new event arrives whose ancestry (walked through parent
+  references) contains a pending outbound event
+- **THEN** the outbound record is closed as delivered without waiting
+  for any explicit status reply
+
+### Requirement: Recreate only on explicit evidence
+
+darkirc SHALL create a replacement event from the stored plaintext —
+fresh timestamp and DAG parents, fresh encryption nonce — only when
+either:
+
+- the rotation window for the original event is closed (known locally
+  from the rotation schedule, or indicated by `TooOld` nacks) and no
+  peer has answered `Has` for it, or
+- every observed reply is a nack and at least one carries reason
+  `Invalid`
+
+The following observations SHALL NOT trigger recreation while the
+rotation window is open: silence, `Nack { NotSynced }`, and
+`Nack { Busy }` mean retry later. Silence SHALL NOT trigger recreation
+even after the window closes when no status reply of any kind has ever
+been received from status-capable peers. A replacement links back via
+the supersession chain so the sender can correlate attempts locally.
 
 
 Recreation SHALL consume a fresh rate-limit slot when rate limiting is
 Recreation SHALL consume a fresh rate-limit slot when rate limiting is
 enabled; if the epoch budget is exhausted the attempt SHALL be parked
 enabled; if the epoch budget is exhausted the attempt SHALL be parked
@@ -163,58 +198,46 @@ SHALL be closed as failed and the failure surfaced to the client.
 
 
 #### Scenario: Rotation makes the original undeliverable
 #### Scenario: Rotation makes the original undeliverable
 
 
-- **WHEN** a pending record's event predates the current rotation window
-  and any peer nacks `TooOld`
-- **THEN** a replacement event with the same `uid` and fresh
-  parents/timestamp is published
+- **WHEN** a pending record's rotation window has closed and no peer
+  answers `Has` after a rebroadcast round
+- **THEN** a replacement event with fresh parents/timestamp is published
+  and linked via the supersession chain
 
 
-#### Scenario: Budget exhaustion parks, not drops
+#### Scenario: Holder answers before recreation
 
 
-- **WHEN** recreation is due but the rate-limit epoch budget is spent
-- **THEN** no unproven replacement is broadcast and the record waits for
-  the next epoch
+- **WHEN** the rotation window has closed but a rebroadcast round
+  returns `Has { inserted: false }` from any peer holding the event in a
+  retained slot
+- **THEN** the record is closed as delivered and no replacement is
+  created
 
 
-#### Scenario: Attempt cap surfaces failure
+#### Scenario: Silence never recreates on its own
 
 
-- **WHEN** the configured recreation attempt cap is reached without any
-  positive outcome
-- **THEN** the record is closed as failed and the client is informed
-
-### Requirement: Message uid for recreate dedup
-
-The chat message payload SHALL carry a sender-generated `uid` that
-remains identical across recreations of the same message. Receiving
-clients SHALL treat messages with equal `uid` as one logical message for
-display purposes. Payload decoding SHALL be version-aware so peers that
-do not understand the new field still decode old payloads.
-
-#### Scenario: Recreated message does not double-render
+- **WHEN** no status replies of any kind are observed (for example all
+  peers run versions without status support)
+- **THEN** the monitor keeps rebroadcasting at a bounded rate and does
+  not create replacements
 
 
-- **WHEN** a receiving client has already displayed the original message
-  and later receives its recreation
-- **THEN** the recreation is not rendered as a second message
+#### Scenario: Transient nacks mean retry
 
 
-#### Scenario: Old payloads still decode
+- **WHEN** observed replies are `Nack { NotSynced }` or `Nack { Busy }`
+  while the rotation window is open
+- **THEN** the monitor retries later and does not recreate
 
 
-- **WHEN** a client that understands `uid` receives a payload without
-  one
-- **THEN** the payload decodes via its version and renders normally
+#### Scenario: Explicit rejection recreates
 
 
-### Requirement: App displays delivery state
+- **WHEN** every observed reply across the retry rounds is a nack and at
+  least one carries reason `Invalid`
+- **THEN** a replacement event is published (bounded by the attempt cap)
 
 
-The app SHALL subscribe to delivery statuses for its own outbound
-messages and expose per-message delivery state keyed by `uid`:
-`sending` (no replies yet), `delivered` (any `Has` reply), `failed`
-(record closed at the attempt cap). Delivery state SHALL NOT be
-presented as evidence that any particular recipient read the message.
-
-#### Scenario: Tick on first acceptance
+#### Scenario: Budget exhaustion parks, not drops
 
 
-- **WHEN** any peer replies `Has` for the app's outbound message
-- **THEN** the message's state becomes `delivered`
+- **WHEN** recreation is due but the rate-limit epoch budget is spent
+- **THEN** no unproven replacement is broadcast and the record waits for
+  the next epoch
 
 
-#### Scenario: Failure is visible
+#### Scenario: Attempt cap surfaces failure
 
 
-- **WHEN** a message's record closes as failed
-- **THEN** the app shows the message as failed rather than leaving it
-  indefinitely in `sending`
+- **WHEN** the configured recreation attempt cap is reached without any
+  positive outcome
+- **THEN** the record is closed as failed and the client is informed

+ 38 - 50
openspec/changes/darkirc-delivery-status/tasks.md

@@ -17,10 +17,12 @@
   default metering, and register dispatch/subscription in
   default metering, and register dispatch/subscription in
   `ProtocolEventGraph::init`. Verify `make` builds and clippy is clean.
   `ProtocolEventGraph::init`. Verify `make` builds and clippy is clean.
 - [ ] 2.2 Emit statuses from `handle_event_put` at every outcome per the
 - [ ] 2.2 Emit statuses from `handle_event_put` at every outcome per the
-  design table (NotSynced, Has{false} duplicate, TooOld, Invalid, Busy,
-  Has{true} inserted), unicast on the receiving channel; strike/flood
-  paths unchanged and silent. Verify each emission point with a
-  two-node test asserting the exact reply variant.
+  design table (NotSynced, Has{false} duplicate, historical-slot check
+  before TooOld, Invalid, Busy, Has{true} inserted), unicast on the
+  receiving channel; strike/flood paths unchanged and silent. Verify
+  each emission point with a two-node test asserting the exact reply
+  variant, including the retained-slot `Has{false}` case across a
+  rotation.
 - [ ] 2.3 Add `receipt_pub: Publisher<(EventPutStatus, ChannelPtr)>` and
 - [ ] 2.3 Add `receipt_pub: Publisher<(EventPutStatus, ChannelPtr)>` and
   `receipt_subscribe()` to `EventGraph`; republish every inbound status
   `receipt_subscribe()` to `EventGraph`; republish every inbound status
   unfiltered from the per-channel handler. Verify with a test that
   unfiltered from the per-channel handler. Verify with a test that
@@ -33,66 +35,52 @@
 ## 3. darkirc outbound tracking (`bin/darkirc`)
 ## 3. darkirc outbound tracking (`bin/darkirc`)
 
 
 - [ ] 3.1 Add the `darkirc_outbound` kvdb tree and serializable record
 - [ ] 3.1 Add the `darkirc_outbound` kvdb tree and serializable record
-  (`uid`, event id, plaintext Privmsg fields, state, attempts, ts,
+  (event id, plaintext Privmsg fields, state, attempts, ts,
   `superseded_by`), plus helpers to insert/close/update records. Verify
   `superseded_by`), plus helpers to insert/close/update records. Verify
   with round-trip serialization + tree unit tests in `server.rs`.
   with round-trip serialization + tree unit tests in `server.rs`.
-- [ ] 3.2 Generate `uid` (16 bytes, `OsRng`) in the send path, and write
-  the outbound record in `publish_events` before `p2p.broadcast`; close
-  as Delivered on any `Has`. Verify with a unit test that a
-  crash-restart (reopen kvdb) still finds the record Pending.
+- [ ] 3.2 Write the outbound record in `publish_events` before
+  `p2p.broadcast`; close as Delivered on any `Has`. Verify with a unit
+  test that a crash-restart (reopen kvdb) still finds the record
+  Pending.
 
 
 ## 4. darkirc delivery monitor (`bin/darkirc`)
 ## 4. darkirc delivery monitor (`bin/darkirc`)
 
 
 - [ ] 4.1 Receipt aggregation task: subscribe `receipt_pub`, filter to
 - [ ] 4.1 Receipt aggregation task: subscribe `receipt_pub`, filter to
-  tracked ids, dedupe per (event id, channel address), update records.
-  Verify with a unit test driving synthetic statuses through the
-  aggregator.
+  tracked ids, dedupe per (event id, channel address), update records;
+  treat an ancestry reference (foreign event whose parents' closure
+  contains a tracked id) as positive delivery evidence. Verify with
+  unit tests driving synthetic statuses and a synthetic child event
+  through the aggregator.
 - [ ] 4.2 Sweep + rebroadcast: periodic sweep gated by
 - [ ] 4.2 Sweep + rebroadcast: periodic sweep gated by
   `is_synced() && connection_count >= K`, rebroadcasting the original
   `is_synced() && connection_count >= K`, rebroadcasting the original
   `EventPut` (event + blob from local DAG) with backoff up to `R_MAX`
   `EventPut` (event + blob from local DAG) with backoff up to `R_MAX`
-  rounds. Verify with a multi-node harness test that a peer which
-  already has the event answers `Has{inserted:false}` and the record
-  closes without recreation.
-- [ ] 4.3 Recreate: on `TooOld`, all-nack, or `R_MAX` exhaustion, build
-  a fresh event from stored plaintext (fresh nonce, same `uid`, fresh
+  rounds, then continuing at the slow rate. Verify with a multi-node
+  harness test that a peer which already has the event answers
+  `Has{inserted:false}` and the record closes without recreation, and
+  that pure silence (status-incapable peer) never produces a
+  replacement while the window is open.
+- [ ] 4.3 Recreate triggers per design: window-closed-with-no-holder
+  (local rotation knowledge or TooOld nacks) and explicit all-nack with
+  at least one `Invalid`; `NotSynced`/`Busy`/silence mean retry. Build
+  a fresh event from stored plaintext (fresh nonce, fresh
   parents/timestamp), park on RLN `BudgetExhausted`, cap attempts at
   parents/timestamp), park on RLN `BudgetExhausted`, cap attempts at
   `A_MAX`, surface failure to the client on cap. Verify with harness
   `A_MAX`, surface failure to the client on cap. Verify with harness
-  tests: rotation-forced recreate reuses the uid; budget exhaustion
-  parks (RLN test harness); attempt cap reaches Failed.
+  tests: rotation-forced recreate writes a superseding record linked
+  via `superseded_by`; budget exhaustion parks; attempt cap reaches
+  Failed; `NotSynced`-only replies do not recreate.
 - [ ] 4.4 End-to-end darkirc scenario test: node A publishes while
 - [ ] 4.4 End-to-end darkirc scenario test: node A publishes while
   disconnected from B, A reconnects after DAG rotation, B must end up
   disconnected from B, A reconnects after DAG rotation, B must end up
-  holding a recreate-generation event with the same `uid`. Verify the
-  assertion holds in the multi-node harness.
+  holding a recreate-generation event and A's record must show the
+  `superseded_by` link; a second scenario where B already holds the
+  original in a retained slot must close A's record as Delivered with
+  no recreate. Verify both assertions in the multi-node harness.
 
 
-## 5. Privmsg uid wire change (`bin/darkirc/src/lib.rs`)
+## 5. Hardening and review gate
 
 
-- [ ] 5.1 Add `uid` behind `Privmsg.version = 1` with a version-matched
-  decoder (v0 decodes without uid). Verify with golden serialization
-  tests for both versions, including a v0 peer ignoring an undecodable
-  v1 payload without error propagation.
-- [ ] 5.2 Re-check sequencing against `darkirc-mod`'s rotating-content
-  tag byte (same struct): confirm merge order or combined encoding in
-  the change notes before merge. Verify by reviewing both deltas against
-  the final `Privmsg` wire format.
-
-## 6. App integration (`bin/app`)
-
-- [ ] 6.1 Wrap `handle_send` with uid generation, outbound records, and
-  the rebroadcast/recreate hooks (RLN-disabled path). Verify
-  `make compile-dev` in `bin/app` succeeds.
-- [ ] 6.2 Switch display dedup from ciphertext-hash `msg_id()` to `uid`
-  (synthetic id for legacy v0 messages) and add the `receipt_pub`
-  subscription mapping per-uid state to `sending`/`delivered`/`failed`
-  with UI notification. Verify with an app-side test or manual dev-run
-  showing state transitions and no double-render of a recreated message.
-
-## 7. Hardening and review gate
-
-- [ ] 7.1 Full workspace gates green: `make`, `make clippy`, `make test`
-  (proofs + contracts built first per AGENTS.md), `make fmt` in
-  `bin/app`; confirm no `unwrap`/`expect`/`panic!` on any new
-  attacker-controlled decode path and no peer addresses logged with
-  receipt state.
-- [ ] 7.2 Invoke `@anon-security-review` on the full diff; treat FAIL as
+- [ ] 5.1 Full workspace gates green: `make`, `make clippy`, `make test`
+  (proofs + contracts built first per AGENTS.md); confirm no
+  `unwrap`/`expect`/`panic!` on any new attacker-controlled decode path
+  and no peer addresses logged with receipt state.
+- [ ] 5.2 Invoke `@anon-security-review` on the full diff; treat FAIL as
   blocking and address findings before marking the change ready to
   blocking and address findings before marking the change ready to
   apply. Verify the review verdict is recorded in the change notes.
   apply. Verify the review verdict is recorded in the change notes.