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); + } }