diff --git a/src/lib.rs b/src/lib.rs index af83669..5d38a36 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,9 @@ pub use upper::config::{DnsConfig, TunConfig}; pub use discovery::{BootstrapHandoffResult, EstablishedTraversal}; // Re-export tree types (relocated from tree:: to proto::stp) -pub use proto::stp::{CoordEntry, ParentDeclaration, TreeCoordinate, TreeError, TreeState}; +pub use proto::stp::{ + CoordEntry, CoordError, ParentDeclaration, TreeCoordinate, TreeError, TreeState, +}; // Re-export bloom filter types (relocated from bloom:: to proto::bloom) pub use proto::bloom::{BloomError, BloomFilter, BloomState}; diff --git a/src/proto/bloom/mod.rs b/src/proto/bloom/mod.rs index 2c195ff..cf4332e 100644 --- a/src/proto/bloom/mod.rs +++ b/src/proto/bloom/mod.rs @@ -26,22 +26,44 @@ mod wire; #[cfg(test)] mod tests; -use thiserror::Error; - pub use core::BloomFilter; pub use limits::{DEFAULT_FILTER_SIZE_BITS, DEFAULT_HASH_COUNT, V1_SIZE_CLASS}; pub use state::BloomState; pub use wire::FilterAnnounce; /// Errors related to Bloom filter operations. -#[derive(Debug, Error)] +#[derive(Debug)] pub enum BloomError { - #[error("invalid filter size: expected {expected} bits, got {got}")] - InvalidSize { expected: usize, got: usize }, + /// Filter bit length did not match the expected size. + InvalidSize { + /// Expected number of bits. + expected: usize, + /// Number of bits received. + got: usize, + }, - #[error("filter size must be a multiple of 8, got {0}")] + /// Filter size was not a multiple of 8 bits. SizeNotByteAligned(usize), - #[error("hash count must be positive")] + /// Hash count was zero. ZeroHashCount, } + +impl ::core::fmt::Display for BloomError { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + match self { + BloomError::InvalidSize { expected, got } => { + write!( + f, + "invalid filter size: expected {expected} bits, got {got}" + ) + } + BloomError::SizeNotByteAligned(n) => { + write!(f, "filter size must be a multiple of 8, got {n}") + } + BloomError::ZeroHashCount => write!(f, "hash count must be positive"), + } + } +} + +impl ::core::error::Error for BloomError {} diff --git a/src/proto/bloom/tests/wire.rs b/src/proto/bloom/tests/wire.rs index 8bf4f3d..91cc597 100644 --- a/src/proto/bloom/tests/wire.rs +++ b/src/proto/bloom/tests/wire.rs @@ -1,5 +1,6 @@ //! Tests for the bloom wire codec (`FilterAnnounce`). +use crate::proto::Error; use crate::proto::bloom::BloomFilter; use crate::proto::bloom::FilterAnnounce; use crate::proto::link::LinkMessageType; @@ -66,13 +67,10 @@ fn test_filter_announce_decode_rejects_bad_size_class() { encoded[10] = 5; // invalid size_class > MAX_SIZE_CLASS let result = FilterAnnounce::decode(&encoded[1..]); - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("invalid size_class") - ); + assert!(matches!( + result, + Err(Error::BadSizeClass { got: 5, max: 3 }) + )); } #[test] @@ -83,13 +81,10 @@ fn test_filter_announce_decode_rejects_non_v1_size_class() { let encoded = announce.encode().unwrap(); let result = FilterAnnounce::decode(&encoded[1..]); - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("unsupported size_class") - ); + assert!(matches!( + result, + Err(Error::BadSizeClass { got: 0, max: 1 }) + )); } #[test] diff --git a/src/proto/bloom/wire.rs b/src/proto/bloom/wire.rs index e36136b..e180982 100644 --- a/src/proto/bloom/wire.rs +++ b/src/proto/bloom/wire.rs @@ -81,9 +81,7 @@ impl FilterAnnounce { /// ``` pub fn encode(&self) -> Result, Error> { if !self.is_valid() { - return Err(Error::Malformed( - "filter size does not match size_class".into(), - )); + return Err(Error::Malformed("filter size does not match size_class")); } let filter_bytes = self.filter.as_bytes(); @@ -121,7 +119,7 @@ impl FilterAnnounce { let sequence = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad sequence".into()))?, + .map_err(|_| Error::Malformed("bad sequence"))?, ); pos += 8; @@ -135,18 +133,18 @@ impl FilterAnnounce { // Validate size_class range if size_class > Self::MAX_SIZE_CLASS { - return Err(Error::Malformed(format!( - "invalid size_class: {size_class} (max {})", - Self::MAX_SIZE_CLASS - ))); + return Err(Error::BadSizeClass { + got: size_class, + max: Self::MAX_SIZE_CLASS, + }); } // v1 compliance check if size_class != super::V1_SIZE_CLASS { - return Err(Error::Malformed(format!( - "unsupported size_class: {size_class} (v1 requires {})", - super::V1_SIZE_CLASS - ))); + return Err(Error::BadSizeClass { + got: size_class, + max: super::V1_SIZE_CLASS, + }); } // Expected filter size from size_class @@ -160,8 +158,8 @@ impl FilterAnnounce { } // Construct BloomFilter from bytes - let filter = BloomFilter::from_slice(&payload[pos..], hash_count) - .map_err(|e| Error::Malformed(format!("invalid bloom filter: {e}")))?; + let filter = + BloomFilter::from_slice(&payload[pos..], hash_count).map_err(Error::BadBloom)?; let announce = Self { filter, diff --git a/src/proto/coord.rs b/src/proto/coord.rs index 64e04cf..341bdc4 100644 --- a/src/proto/coord.rs +++ b/src/proto/coord.rs @@ -6,10 +6,23 @@ use core::fmt; use crate::NodeAddr; use crate::proto::Error; -// TEMPORARY: coord depends upward on stp's TreeError here. This inversion is -// intentional and resolved in the next pass when a no_std-clean CoordError is -// created and TreeCoordinate is cut over to it. -use crate::proto::stp::TreeError; + +/// Errors from constructing a `TreeCoordinate`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CoordError { + /// Coordinate path had zero entries. + EmptyCoordinate, +} + +impl core::fmt::Display for CoordError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + CoordError::EmptyCoordinate => write!(f, "invalid tree coordinate: empty path"), + } + } +} + +impl core::error::Error for CoordError {} /// Metadata for a single node in a tree coordinate path. /// @@ -71,9 +84,9 @@ impl TreeCoordinate { /// Create a coordinate from a path of entries (self to root). /// /// The path must be non-empty and ordered from the node to the root. - pub fn new(path: Vec) -> Result { + pub fn new(path: Vec) -> Result { if path.is_empty() { - return Err(TreeError::EmptyCoordinate); + return Err(CoordError::EmptyCoordinate); } Ok(Self(path)) } @@ -82,9 +95,9 @@ impl TreeCoordinate { /// /// Convenience constructor for cases where only routing is needed. /// Each entry gets sequence=0, timestamp=0. - pub fn from_addrs(addrs: Vec) -> Result { + pub fn from_addrs(addrs: Vec) -> Result { if addrs.is_empty() { - return Err(TreeError::EmptyCoordinate); + return Err(CoordError::EmptyCoordinate); } Ok(Self(addrs.into_iter().map(CoordEntry::addr_only).collect())) } @@ -271,7 +284,7 @@ pub(crate) fn decode_coords(data: &[u8]) -> Result<(TreeCoordinate, usize), Erro }); } if count == 0 { - return Err(Error::Malformed("coordinate with zero entries".into())); + return Err(Error::Malformed("coordinate with zero entries")); } let mut addrs = Vec::with_capacity(count); for i in 0..count { @@ -280,7 +293,7 @@ pub(crate) fn decode_coords(data: &[u8]) -> Result<(TreeCoordinate, usize), Erro bytes.copy_from_slice(&data[offset..offset + 16]); addrs.push(NodeAddr::from_bytes(bytes)); } - let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; + let coord = TreeCoordinate::from_addrs(addrs).map_err(Error::BadCoord)?; Ok((coord, needed)) } @@ -314,7 +327,7 @@ pub(crate) fn decode_optional_coords( bytes.copy_from_slice(&data[offset..offset + 16]); addrs.push(NodeAddr::from_bytes(bytes)); } - let coord = TreeCoordinate::from_addrs(addrs).map_err(|e| Error::Malformed(e.to_string()))?; + let coord = TreeCoordinate::from_addrs(addrs).map_err(Error::BadCoord)?; Ok((Some(coord), needed)) } diff --git a/src/proto/discovery/wire.rs b/src/proto/discovery/wire.rs index e61806c..c717af4 100644 --- a/src/proto/discovery/wire.rs +++ b/src/proto/discovery/wire.rs @@ -98,7 +98,7 @@ impl LookupRequest { let request_id = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad request_id".into()))?, + .map_err(|_| Error::Malformed("bad request_id"))?, ); pos += 8; @@ -118,7 +118,7 @@ impl LookupRequest { let min_mtu = u16::from_le_bytes( payload[pos..pos + 2] .try_into() - .map_err(|_| Error::Malformed("bad min_mtu".into()))?, + .map_err(|_| Error::Malformed("bad min_mtu"))?, ); pos += 2; @@ -223,7 +223,7 @@ impl LookupResponse { let request_id = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad request_id".into()))?, + .map_err(|_| Error::Malformed("bad request_id"))?, ); pos += 8; @@ -235,7 +235,7 @@ impl LookupResponse { let path_mtu = u16::from_le_bytes( payload[pos..pos + 2] .try_into() - .map_err(|_| Error::Malformed("bad path_mtu".into()))?, + .map_err(|_| Error::Malformed("bad path_mtu"))?, ); pos += 2; @@ -249,7 +249,7 @@ impl LookupResponse { }); } let proof = Signature::from_slice(&payload[pos..pos + 64]) - .map_err(|_| Error::Malformed("bad proof signature".into()))?; + .map_err(|_| Error::Malformed("bad proof signature"))?; Ok(Self { request_id, diff --git a/src/proto/error.rs b/src/proto/error.rs index 04658cb..43f2c8c 100644 --- a/src/proto/error.rs +++ b/src/proto/error.rs @@ -1,31 +1,100 @@ //! Protocol error types. -use thiserror::Error; - /// Errors related to protocol message handling. -#[derive(Debug, Error)] +#[derive(Debug)] pub enum Error { - #[error("invalid message type: 0x{0:02x}")] + /// Message type byte was not recognized. InvalidMessageType(u8), - #[error("message too short: expected at least {expected}, got {got}")] - MessageTooShort { expected: usize, got: usize }, + /// Message was shorter than the minimum expected length. + MessageTooShort { + /// Minimum number of bytes expected. + expected: usize, + /// Number of bytes actually present. + got: usize, + }, - #[error("message too long: max {max}, got {got}")] - MessageTooLong { max: usize, got: usize }, + /// Message exceeded the maximum allowed length. + MessageTooLong { + /// Maximum number of bytes allowed. + max: usize, + /// Number of bytes actually present. + got: usize, + }, - #[error("invalid signature")] + /// Signature failed to parse or verify. InvalidSignature, - #[error("unsupported protocol version: {0}")] + /// Protocol version byte was not supported. UnsupportedVersion(u8), - #[error("malformed message: {0}")] - Malformed(String), + /// Message was structurally malformed; the string names the field. + Malformed(&'static str), - #[error("hop limit exceeded")] + /// Size class byte was out of range for this message. + BadSizeClass { + /// The size class value received. + got: u8, + /// The maximum (or required) size class. + max: u8, + }, + + /// A tree coordinate failed to construct during decode. + BadCoord(crate::proto::coord::CoordError), + + /// A bloom filter failed to construct during decode. + BadBloom(crate::proto::bloom::BloomError), + + /// Hop limit was exceeded while forwarding. HopLimitExceeded, - #[error("ttl expired")] + /// Time-to-live expired while forwarding. TtlExpired, } + +impl core::fmt::Display for Error { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Error::InvalidMessageType(t) => write!(f, "invalid message type: 0x{t:02x}"), + Error::MessageTooShort { expected, got } => { + write!( + f, + "message too short: expected at least {expected}, got {got}" + ) + } + Error::MessageTooLong { max, got } => { + write!(f, "message too long: max {max}, got {got}") + } + Error::InvalidSignature => write!(f, "invalid signature"), + Error::UnsupportedVersion(v) => write!(f, "unsupported protocol version: {v}"), + Error::Malformed(m) => write!(f, "malformed message: {m}"), + Error::BadSizeClass { got, max } => write!(f, "bad size class: {got} (max {max})"), + Error::BadCoord(e) => write!(f, "bad coordinate: {e}"), + Error::BadBloom(e) => write!(f, "bad bloom filter: {e}"), + Error::HopLimitExceeded => write!(f, "hop limit exceeded"), + Error::TtlExpired => write!(f, "ttl expired"), + } + } +} + +impl core::error::Error for Error { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Error::BadCoord(e) => Some(e), + Error::BadBloom(e) => Some(e), + _ => None, + } + } +} + +impl From for Error { + fn from(e: crate::proto::coord::CoordError) -> Self { + Error::BadCoord(e) + } +} + +impl From for Error { + fn from(e: crate::proto::bloom::BloomError) -> Self { + Error::BadBloom(e) + } +} diff --git a/src/proto/stp/mod.rs b/src/proto/stp/mod.rs index 70d4af5..2c679e2 100644 --- a/src/proto/stp/mod.rs +++ b/src/proto/stp/mod.rs @@ -28,11 +28,9 @@ mod wire; #[cfg(test)] mod tests; -use thiserror::Error; - use crate::{IdentityError, NodeAddr}; -pub use crate::proto::coord::{CoordEntry, TreeCoordinate}; +pub use crate::proto::coord::{CoordEntry, CoordError, TreeCoordinate}; pub(crate) use crate::proto::coord::{ coords_wire_size, decode_coords, decode_optional_coords, encode_coords, encode_empty_coords, }; @@ -42,51 +40,118 @@ pub use state::TreeState; pub use wire::TreeAnnounce; /// Errors related to spanning tree operations. -#[derive(Debug, Error)] +#[derive(Debug)] pub enum TreeError { - #[error("invalid tree coordinate: empty path")] + /// Coordinate path had zero entries. EmptyCoordinate, - #[error("invalid ancestry: does not reach claimed root")] + /// Ancestry path does not reach the claimed root. AncestryNotToRoot, - #[error("invalid ancestry: root declaration must contain only the sender")] + /// Root declaration contained hops other than the sender. RootDeclarationMismatch, - #[error("invalid ancestry: non-root declaration must include a parent hop")] + /// Non-root declaration was missing its parent hop. AncestryTooShort, - #[error("invalid ancestry: sender {declared} does not match first path entry {ancestry}")] + /// Declared sender does not match the first ancestry entry. AncestryNodeMismatch { + /// The declared sender address. declared: NodeAddr, + /// The first ancestry path entry. ancestry: NodeAddr, }, - #[error( - "invalid ancestry: signed parent {declared} does not match first ancestry hop {ancestry}" - )] + /// Signed parent does not match the first ancestry hop. AncestryParentMismatch { + /// The signed parent address. declared: NodeAddr, + /// The first ancestry hop. ancestry: NodeAddr, }, - #[error( - "invalid ancestry: advertised root {advertised} is not the minimum path entry {minimum}" - )] + /// Advertised root is not the minimum path entry. AncestryRootNotMinimum { + /// The advertised root address. advertised: NodeAddr, + /// The minimum path entry. minimum: NodeAddr, }, - #[error("signature verification failed for node {0:?}")] + /// Signature verification failed for the given node. InvalidSignature(NodeAddr), - #[error("sequence number regression: got {got}, expected > {expected}")] - SequenceRegression { got: u64, expected: u64 }, + /// Sequence number regressed below the expected value. + SequenceRegression { + /// The received sequence number. + got: u64, + /// The value the sequence had to exceed. + expected: u64, + }, - #[error("parent not in peers: {0:?}")] + /// Declared parent was not among the known peers. ParentNotPeer(NodeAddr), - #[error("identity error: {0}")] - Identity(#[from] IdentityError), + /// An identity operation failed. + Identity(IdentityError), +} + +impl ::core::fmt::Display for TreeError { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + match self { + TreeError::EmptyCoordinate => write!(f, "invalid tree coordinate: empty path"), + TreeError::AncestryNotToRoot => { + write!(f, "invalid ancestry: does not reach claimed root") + } + TreeError::RootDeclarationMismatch => write!( + f, + "invalid ancestry: root declaration must contain only the sender" + ), + TreeError::AncestryTooShort => write!( + f, + "invalid ancestry: non-root declaration must include a parent hop" + ), + TreeError::AncestryNodeMismatch { declared, ancestry } => write!( + f, + "invalid ancestry: sender {declared} does not match first path entry {ancestry}" + ), + TreeError::AncestryParentMismatch { declared, ancestry } => write!( + f, + "invalid ancestry: signed parent {declared} does not match first ancestry hop {ancestry}" + ), + TreeError::AncestryRootNotMinimum { + advertised, + minimum, + } => write!( + f, + "invalid ancestry: advertised root {advertised} is not the minimum path entry {minimum}" + ), + TreeError::InvalidSignature(node) => { + write!(f, "signature verification failed for node {node:?}") + } + TreeError::SequenceRegression { got, expected } => { + write!( + f, + "sequence number regression: got {got}, expected > {expected}" + ) + } + TreeError::ParentNotPeer(node) => write!(f, "parent not in peers: {node:?}"), + TreeError::Identity(e) => write!(f, "identity error: {e}"), + } + } +} + +impl ::core::error::Error for TreeError { + fn source(&self) -> Option<&(dyn ::core::error::Error + 'static)> { + match self { + TreeError::Identity(e) => Some(e), + _ => None, + } + } +} + +impl From for TreeError { + fn from(e: IdentityError) -> Self { + TreeError::Identity(e) + } } diff --git a/src/proto/stp/tests/coordinate.rs b/src/proto/stp/tests/coordinate.rs index 772fda2..0ae4e7a 100644 --- a/src/proto/stp/tests/coordinate.rs +++ b/src/proto/stp/tests/coordinate.rs @@ -1,7 +1,7 @@ //! TreeCoordinate unit tests. use super::util::{make_coords, make_node_addr}; -use crate::proto::stp::{CoordEntry, TreeCoordinate, TreeError}; +use crate::proto::stp::{CoordEntry, CoordError, TreeCoordinate}; #[test] fn test_tree_coordinate_root() { @@ -33,7 +33,7 @@ fn test_tree_coordinate_path() { #[test] fn test_tree_coordinate_empty_fails() { let result = TreeCoordinate::from_addrs(vec![]); - assert!(matches!(result, Err(TreeError::EmptyCoordinate))); + assert!(matches!(result, Err(CoordError::EmptyCoordinate))); } #[test] diff --git a/src/proto/stp/wire.rs b/src/proto/stp/wire.rs index fce4797..45a9efb 100644 --- a/src/proto/stp/wire.rs +++ b/src/proto/stp/wire.rs @@ -153,7 +153,7 @@ impl TreeAnnounce { let sequence = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad sequence".into()))?, + .map_err(|_| Error::Malformed("bad sequence"))?, ); pos += 8; @@ -161,7 +161,7 @@ impl TreeAnnounce { let timestamp = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad timestamp".into()))?, + .map_err(|_| Error::Malformed("bad timestamp"))?, ); pos += 8; @@ -169,7 +169,7 @@ impl TreeAnnounce { let parent = NodeAddr::from_bytes( payload[pos..pos + 16] .try_into() - .map_err(|_| Error::Malformed("bad parent".into()))?, + .map_err(|_| Error::Malformed("bad parent"))?, ); pos += 16; @@ -177,7 +177,7 @@ impl TreeAnnounce { let ancestry_count = u16::from_le_bytes( payload[pos..pos + 2] .try_into() - .map_err(|_| Error::Malformed("bad ancestry count".into()))?, + .map_err(|_| Error::Malformed("bad ancestry count"))?, ) as usize; pos += 2; @@ -196,19 +196,19 @@ impl TreeAnnounce { let node_addr = NodeAddr::from_bytes( payload[pos..pos + 16] .try_into() - .map_err(|_| Error::Malformed("bad entry node_addr".into()))?, + .map_err(|_| Error::Malformed("bad entry node_addr"))?, ); pos += 16; let entry_seq = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad entry sequence".into()))?, + .map_err(|_| Error::Malformed("bad entry sequence"))?, ); pos += 8; let entry_ts = u64::from_le_bytes( payload[pos..pos + 8] .try_into() - .map_err(|_| Error::Malformed("bad entry timestamp".into()))?, + .map_err(|_| Error::Malformed("bad entry timestamp"))?, ); pos += 8; entries.push(CoordEntry::new(node_addr, entry_seq, entry_ts)); @@ -217,7 +217,7 @@ impl TreeAnnounce { // signature (64) let sig_bytes: [u8; 64] = payload[pos..pos + 64] .try_into() - .map_err(|_| Error::Malformed("bad signature".into()))?; + .map_err(|_| Error::Malformed("bad signature"))?; // Validate the signature parses as a well-formed schnorr signature (the // codec's only crypto touch, ยง11 w2); store the raw bytes so the in-core // declaration carries no `secp256k1` dependency. Actual verification is a @@ -226,17 +226,14 @@ impl TreeAnnounce { // The first entry's node_addr is the declaring node if entries.is_empty() { - return Err(Error::Malformed( - "ancestry must have at least one entry".into(), - )); + return Err(Error::Malformed("ancestry must have at least one entry")); } let node_addr = entries[0].node_addr; let declaration = ParentDeclaration::with_signature(node_addr, parent, sequence, timestamp, sig_bytes); - let ancestry = TreeCoordinate::new(entries) - .map_err(|e| Error::Malformed(format!("bad ancestry: {}", e)))?; + let ancestry = TreeCoordinate::new(entries).map_err(Error::BadCoord)?; Ok(Self { declaration,