node: Improve `Timestamp` conversion safety

Remove `Timestamp::from(u64)` instance, since not all `u64`s are valid
Timestamps, and add `try_from` instead.

radicle-node/src/service/gossip/store.rs

@@ -71,7 +71,7 @@ impl Store for Database {
 
         if let Some(Ok(row)) = stmt.into_iter().next() {
             return match row.try_read::<Option<i64>, _>(0)? {
-                Some(i) => Ok(Some(Timestamp::from(u64::try_from(i)?))),
+                Some(i) => Ok(Some(Timestamp::try_from(i)?)),
                 None => Ok(None),
             };
         }

radicle-node/src/service/message.rs

@@ -669,7 +669,7 @@ mod tests {
     fn test_node_announcement_validate() {
         let ann = NodeAnnouncement {
             features: node::Features::SEED,
-            timestamp: Timestamp::from(42491841),
+            timestamp: Timestamp::try_from(42491841u64).unwrap(),
             alias: Alias::new("alice"),
             addresses: BoundedVec::new(),
             nonce: 0,

radicle-node/src/wire.rs

@@ -60,6 +60,8 @@ pub enum Error {
     InvalidProtocolVersion([u8; 4]),
     #[error("invalid onion address: {0}")]
     InvalidOnionAddr(#[from] tor::OnionAddrDecodeError),
+    #[error("invalid timestamp: {0}")]
+    InvalidTimestamp(u64),
     #[error("unknown address type `{0}`")]
     UnknownAddressType(u8),
     #[error("unknown message type `{0}`")]
@@ -537,8 +539,9 @@ impl Encode for Timestamp {
 impl Decode for Timestamp {
     fn decode<R: io::Read + ?Sized>(reader: &mut R) -> Result<Self, Error> {
         let millis = u64::decode(reader)?;
+        let ts = Timestamp::try_from(millis).map_err(Error::InvalidTimestamp)?;
 
-        Ok(Timestamp::from(millis))
+        Ok(ts)
     }
 }
 

radicle/src/node/routing.rs

@@ -347,10 +347,14 @@ mod test {
             vec![(id, InsertResult::SeedAdded)]
         );
         assert_eq!(
-            db.insert([&id], node, Timestamp::from(1)).unwrap(),
+            db.insert([&id], node, Timestamp::try_from(1u64).unwrap())
+                .unwrap(),
             vec![(id, InsertResult::TimeUpdated)]
         );
-        assert_eq!(db.entry(&id, &node).unwrap(), Some(Timestamp::from(1)));
+        assert_eq!(
+            db.entry(&id, &node).unwrap(),
+            Some(Timestamp::try_from(1u64).unwrap())
+        );
     }
 
     #[test]
@@ -372,7 +376,8 @@ mod test {
             ]
         );
         assert_eq!(
-            db.insert([&id1, &id2], node, Timestamp::from(1)).unwrap(),
+            db.insert([&id1, &id2], node, Timestamp::try_from(1u64).unwrap())
+                .unwrap(),
             vec![
                 (id1, InsertResult::TimeUpdated),
                 (id2, InsertResult::TimeUpdated)
@@ -415,7 +420,8 @@ mod test {
 
         for node in &nodes {
             let time = rng.u64(..now.as_millis());
-            db.insert(&ids, *node, Timestamp::from(time)).unwrap();
+            db.insert(&ids, *node, Timestamp::try_from(time).unwrap())
+                .unwrap();
         }
 
         let ids = arbitrary::vec::<RepoId>(10);
@@ -423,7 +429,8 @@ mod test {
 
         for node in &nodes {
             let time = rng.u64(now.as_millis()..i64::MAX as u64);
-            db.insert(&ids, *node, Timestamp::from(time)).unwrap();
+            db.insert(&ids, *node, Timestamp::try_from(time).unwrap())
+                .unwrap();
         }
 
         let pruned = db.prune(now.into(), None).unwrap();

radicle/src/node/timestamp.rs

@@ -1,5 +1,6 @@
 use std::{
     fmt,
+    num::TryFromIntError,
     ops::{Add, Deref, Sub},
 };
 
@@ -15,7 +16,7 @@ impl Add<u64> for Timestamp {
     type Output = Timestamp;
 
     fn add(self, millis: u64) -> Self::Output {
-        Self(self.0 + millis)
+        Self(self.0.saturating_add(millis))
     }
 }
 
@@ -23,7 +24,7 @@ impl Sub<u64> for Timestamp {
     type Output = Timestamp;
 
     fn sub(self, millis: u64) -> Self::Output {
-        Self(self.0 - millis)
+        Self(self.0.saturating_sub(millis))
     }
 }
 
@@ -69,9 +70,23 @@ impl From<Timestamp> for LocalTime {
     }
 }
 
-impl From<u64> for Timestamp {
-    fn from(u: u64) -> Self {
-        Self(u)
+impl TryFrom<u64> for Timestamp {
+    type Error = u64;
+
+    fn try_from(u: u64) -> Result<Self, u64> {
+        if u <= *Self::MAX {
+            Ok(Self(u))
+        } else {
+            Err(u)
+        }
+    }
+}
+
+impl TryFrom<i64> for Timestamp {
+    type Error = TryFromIntError;
+
+    fn try_from(i: i64) -> Result<Self, Self::Error> {
+        i.try_into().map(Self)
     }
 }
 
@@ -84,7 +99,7 @@ impl TryFrom<&sql::Value> for Timestamp {
                 Ok(u) => Ok(Timestamp(u)),
                 Err(e) => Err(sql::Error {
                     code: None,
-                    message: Some(format!("sql: invalid integer for timestamp: {e}")),
+                    message: Some(format!("sql: invalid integer `{i}` for timestamp: {e}")),
                 }),
             },
             _ => Err(sql::Error {
@@ -101,8 +116,18 @@ impl sql::BindableWithIndex for &Timestamp {
             Ok(integer) => integer.bind(stmt, i),
             Err(e) => Err(sql::Error {
                 code: None,
-                message: Some(format!("sql: invalid timestamp: {e}")),
+                message: Some(format!("sql: invalid timestamp `{self}`: {e}")),
             }),
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_timestamp_max() {
+        assert_eq!(i64::try_from(*Timestamp::MAX), Ok(i64::MAX));
+    }
+}

radicle/src/test/arbitrary.rs

@@ -315,6 +315,6 @@ impl Arbitrary for Alias {
 
 impl Arbitrary for Timestamp {
     fn arbitrary(g: &mut qcheck::Gen) -> Self {
-        Self::from(u64::arbitrary(g))
+        Self::try_from(u64::arbitrary(g).min(*Self::MAX)).unwrap()
     }
 }