From 989ae65fc5114425327c82c97d44879bea10afc8 Mon Sep 17 00:00:00 2001
From: Johnathan Corgan <johnathan@corganlabs.com>
Date: Sat, 25 Jul 2026 17:05:20 +0000
Subject: [PATCH] Align the datagram hop-limit helpers with the forwarder's
 semantics

The forwarding path was corrected to full IP semantics: local delivery is not
hop-limit gated, and forwarding decrements first and drops at zero. Two helpers
on SessionDatagram were left implementing the old rule. decrement_ttl still
checked before decrementing, and can_forward still answered true at a hop limit
of one, where the decrement leaves zero and the datagram is dropped. Neither is
called anywhere today, which is precisely why they are worth correcting rather
than leaving as an invitation to reintroduce the defect.

The TtlExhausted reject doc described behaviour that no longer exists. The
reject is charged for a transit arrival at one as well as at zero, and is never
charged for a datagram addressed to this node, whose delivery is decided ahead
of the test.

Both helpers are pinned by tests at the boundary, which fail against the
previous implementations.
---
 CHANGELOG.md         |  5 ++++
 src/node/reject.rs   |  6 +++-
 src/protocol/link.rs | 66 ++++++++++++++++++++++++++++++++++++++------
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b686088..3e3b502 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `SessionDatagram::decrement_ttl` and `SessionDatagram::can_forward` now match
+  the forwarder's IP hop-limit semantics: `decrement_ttl` decrements first and
+  reports false when the result is zero, and `can_forward` is true only at a
+  TTL of 2 or more.
+
 ### Fixed
 
 - Nostr NAT traversal no longer breaks after the host suspends. The traversal
diff --git a/src/node/reject.rs b/src/node/reject.rs
index cac5ad0..d2a44cf 100644
--- a/src/node/reject.rs
+++ b/src/node/reject.rs
@@ -232,7 +232,11 @@ pub enum ForwardingReject {
     /// `SessionDatagramRef::decode` returned an error. Tracked via
     /// [`ForwardingStats::decode_error_packets`](crate::node::stats::ForwardingStats).
     DecodeError,
-    /// Datagram arrived with TTL=0 — already exhausted, no forward.
+    /// Transit datagram whose TTL would reach zero on this hop, so it is
+    /// dropped rather than forwarded. Charged for an arrival at TTL 1 as
+    /// well as an already-exhausted arrival at TTL 0. Never charged for a
+    /// datagram addressed to this node, whose delivery is not TTL-gated and
+    /// is decided ahead of this test.
     /// Tracked via
     /// [`ForwardingStats::ttl_exhausted_packets`](crate::node::stats::ForwardingStats).
     TtlExhausted,
diff --git a/src/protocol/link.rs b/src/protocol/link.rs
index 102f814..0dd652b 100644
--- a/src/protocol/link.rs
+++ b/src/protocol/link.rs
@@ -337,19 +337,27 @@ impl SessionDatagram {
         self
     }
 
-    /// Decrement TTL, returning false if exhausted.
+    /// Decrement the TTL for a transit hop, returning whether the result may
+    /// still be transmitted.
+    ///
+    /// Follows IP semantics: the decrement happens first, and a datagram that
+    /// would leave with a TTL of zero is not transmitted. `saturating_sub`
+    /// folds an already-exhausted arrival (TTL 0) into the same outcome as a
+    /// last-hop arrival (TTL 1); both leave `ttl` at 0 and return false.
+    ///
+    /// This governs forwarding only. Delivery to the addressed node is not
+    /// TTL-gated and must not consult this method.
     pub fn decrement_ttl(&mut self) -> bool {
-        if self.ttl > 0 {
-            self.ttl -= 1;
-            true
-        } else {
-            false
-        }
+        self.ttl = self.ttl.saturating_sub(1);
+        self.ttl > 0
     }
 
-    /// Check if the datagram can be forwarded.
+    /// Check whether this datagram would survive a transit hop.
+    ///
+    /// True only at TTL 2 or more: at TTL 1 the decrement leaves zero, so the
+    /// datagram is dropped rather than forwarded.
     pub fn can_forward(&self) -> bool {
-        self.ttl > 0
+        self.ttl > 1
     }
 
     /// Encode as link-layer message (msg_type + ttl + path_mtu + src_addr + dest_addr + payload).
@@ -675,4 +683,44 @@ mod tests {
             assert_eq!(decoded.ttl, hop);
         }
     }
+
+    #[test]
+    fn test_session_datagram_can_forward() {
+        let dg = SessionDatagram::new(make_node_addr(1), make_node_addr(2), vec![0x42]);
+
+        assert!(!dg.clone().with_ttl(0).can_forward());
+        assert!(
+            !dg.clone().with_ttl(1).can_forward(),
+            "ttl=1 would leave at zero, so it is not forwardable"
+        );
+        assert!(
+            dg.clone().with_ttl(2).can_forward(),
+            "ttl=2 leaves at one, so it is forwardable"
+        );
+        assert!(dg.with_ttl(255).can_forward());
+    }
+
+    #[test]
+    fn test_session_datagram_decrement_ttl() {
+        let base = SessionDatagram::new(make_node_addr(1), make_node_addr(2), vec![0x42]);
+
+        let mut dg = base.clone().with_ttl(0);
+        assert!(!dg.decrement_ttl(), "ttl=0 is already exhausted");
+        assert_eq!(dg.ttl, 0, "decrement must saturate rather than wrap");
+
+        let mut dg = base.clone().with_ttl(1);
+        assert!(
+            !dg.decrement_ttl(),
+            "ttl=1 leaves at zero, so it is dropped"
+        );
+        assert_eq!(dg.ttl, 0);
+
+        let mut dg = base.clone().with_ttl(2);
+        assert!(dg.decrement_ttl());
+        assert_eq!(dg.ttl, 1);
+
+        let mut dg = base.with_ttl(64);
+        assert!(dg.decrement_ttl());
+        assert_eq!(dg.ttl, 63);
+    }
 }