Use std::time::Duration instead of u64 for expiry time

Author: sgeisler

src/de.rs

@@ -485,11 +485,11 @@ impl FromBase32 for ExpiryTime {
 	type Err = ParseError;
 
 	fn from_base32(field_data: &[u5]) -> Result<ExpiryTime, ParseError> {
-		let expiry = parse_int_be::<u64, u5>(field_data, 32);
-		if let Some(expiry) = expiry {
-			Ok(ExpiryTime{seconds: expiry})
-		} else {
-			Err(ParseError::IntegerOverflowError)
+		match parse_int_be::<u64, u5>(field_data, 32)
+			.and_then(|t| ExpiryTime::from_seconds(t).ok()) // ok, since the only error is out of bounds
+		{
+			Some(t) => Ok(t),
+			None => Err(ParseError::IntegerOverflowError),
 		}
 	}
 }
@@ -814,7 +814,7 @@ mod test {
 		use bech32::FromBase32;
 
 		let input = from_bech32("pu".as_bytes());
-		let expected = Ok(ExpiryTime{seconds: 60});
+		let expected = Ok(ExpiryTime::from_seconds(60).unwrap());
 		assert_eq!(ExpiryTime::from_base32(&input), expected);
 
 		let input_too_large = from_bech32("sqqqqqqqqqqqq".as_bytes());

src/lib.rs

@@ -22,11 +22,15 @@ mod tb;
 pub use de::{ParseError, ParseOrSemanticError};
 
 
-// TODO: fix before 2038 (see rust PR #55527)
+// TODO: fix before 2037 (see rust PR #55527)
 /// Defines the maximum UNIX timestamp that can be represented as `SystemTime`. This is checked by
 /// one of the unit tests, please run them.
 const SYSTEM_TIME_MAX_UNIX_TIMESTAMP: u64 = std::i32::MAX as u64;
 
+/// Allow the expiry time to be up to one year. Since this reduces the range of possible timestamps
+/// it should be rather low as long as we still have to support 32bit time representations
+const MAX_EXPIRY_TIME: u64 = 60 * 60 * 24 * 356;
+
 /// This function is used as a static assert for the size of `SystemTime`. If the crate fails to
 /// compile due to it this indicates that your system uses unexpected bounds for `SystemTime`. You
 /// can remove this functions and run the test `test_system_time_bounds_assumptions`. In any case,
@@ -262,10 +266,12 @@ pub struct Description(String);
 pub struct PayeePubKey(pub PublicKey);
 
 /// Positive duration that defines when (relatively to the timestamp) in the future the invoice expires
+///
+/// # Invariants
+/// The number of seconds this expiry time represents has to be in the range
+/// `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP / 2` to avoid overflows when adding it to a timestamp
 #[derive(Eq, PartialEq, Debug, Clone)]
-pub struct ExpiryTime {
-	pub seconds: u64
-}
+pub struct ExpiryTime(Duration);
 
 /// `min_final_cltv_expiry` to use for the last HTLC in the route
 #[derive(Eq, PartialEq, Debug, Clone)]
@@ -382,8 +388,8 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool> InvoiceBuilder<D, H, T> {
 	}
 
 	/// Sets the expiry time in seconds.
-	pub fn expiry_time_seconds(mut self, expiry_seconds: u64) -> Self {
-		self.tagged_fields.push(TaggedField::ExpiryTime(ExpiryTime {seconds: expiry_seconds}));
+	pub fn expiry_time_seconds(mut self, expiry_time: Duration) -> Self {
+		self.tagged_fields.push(TaggedField::ExpiryTime(ExpiryTime::from_duration(expiry_time).unwrap()));
 		self
 	}
 
@@ -490,7 +496,7 @@ impl<D: tb::Bool, H: tb::Bool> InvoiceBuilder<D, H, tb::False> {
 
 	/// Sets the timestamp to the current UNIX timestamp.
 	pub fn current_timestamp(mut self) -> InvoiceBuilder<D, H, tb::True> {
-		use std::time::{SystemTime, UNIX_EPOCH};
+		use std::time::SystemTime;
 		let now = PositiveTimestamp::from_system_time(SystemTime::now());
 		self.timestamp = Some(now.expect("for the foreseeable future this shouldn't happen"));
 		self.set_flags()
@@ -760,23 +766,23 @@ impl RawInvoice {
 impl PositiveTimestamp {
 
 	/// Create a new `PositiveTimestamp` from a unix timestamp in the Range
-	/// `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP / 2`, otherwise return a
+	/// `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME`, otherwise return a
 	/// `CreationError::TimestampOutOfBounds`.
 	pub fn from_unix_timestamp(unix_seconds: u64) -> Result<Self, CreationError> {
-		if unix_seconds > SYSTEM_TIME_MAX_UNIX_TIMESTAMP / 2 {
+		if unix_seconds > SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME {
 			Err(CreationError::TimestampOutOfBounds)
 		} else {
 			Ok(PositiveTimestamp(UNIX_EPOCH + Duration::from_secs(unix_seconds)))
 		}
 	}
 
 	/// Create a new `PositiveTimestamp` from a `SystemTime` with a corresponding unix timestamp in
-	/// the Range `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP / 2`, otherwise return a
+	/// the Range `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME`, otherwise return a
 	/// `CreationError::TimestampOutOfBounds`.
 	pub fn from_system_time(time: SystemTime) -> Result<Self, CreationError> {
 		if time
 			.duration_since(UNIX_EPOCH)
-			.map(|t| t.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP / 2) // check for consistency reasons
+			.map(|t| t.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME)
 			.unwrap_or(true)
 			{
 				Ok(PositiveTimestamp(time))
@@ -1010,6 +1016,32 @@ impl Deref for PayeePubKey {
 	}
 }
 
+impl ExpiryTime {
+	pub fn from_seconds(seconds: u64) -> Result<ExpiryTime, CreationError> {
+		if seconds <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME {
+			Ok(ExpiryTime(Duration::from_secs(seconds)))
+		} else {
+			Err(CreationError::ExpiryTimeOutOfBounds)
+		}
+	}
+
+	pub fn from_duration(duration: Duration) -> Result<ExpiryTime, CreationError> {
+		if duration.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME {
+			Ok(ExpiryTime(duration))
+		} else {
+			Err(CreationError::ExpiryTimeOutOfBounds)
+		}
+	}
+
+	pub fn as_seconds(&self) -> u64 {
+		self.0.as_secs()
+	}
+
+	pub fn as_duration(&self) -> &Duration {
+		&self.0
+	}
+}
+
 impl Route {
 	pub fn new(hops: Vec<RouteHop>) -> Result<Route, CreationError> {
 		if hops.len() <= 12 {
@@ -1065,6 +1097,9 @@ pub enum CreationError {
 
 	/// The unix timestamp of the supplied date is <0 or can't be represented as `SystemTime`
 	TimestampOutOfBounds,
+
+	/// The supplied expiry time could cause an overflow if added to a `PositiveTimestamp`
+	ExpiryTimeOutOfBounds,
 }
 
 /// Errors that may occur when converting a `RawInvoice` to an `Invoice`. They relate to the
@@ -1369,7 +1404,7 @@ mod test {
 			1234567
 		);
 		assert_eq!(invoice.payee_pub_key(), Some(&PayeePubKey(public_key)));
-		assert_eq!(invoice.expiry_time(), Some(&ExpiryTime{seconds: 54321}));
+		assert_eq!(invoice.expiry_time(), Some(&ExpiryTime::from_seconds(54321).unwrap()));
 		assert_eq!(invoice.min_final_cltv_expiry(), Some(&MinFinalCltvExpiry(144)));
 		assert_eq!(invoice.fallbacks(), vec![&Fallback::PubKeyHash([0;20])]);
 		assert_eq!(invoice.routes(), vec![&Route(route_1), &Route(route_2)]);

src/ser.rs

@@ -169,7 +169,7 @@ impl ToBase32<Vec<u5>> for PayeePubKey {
 
 impl ToBase32<Vec<u5>> for ExpiryTime {
 	fn to_base32(&self) -> Vec<u5> {
-		encode_int_be_base32(self.seconds)
+		encode_int_be_base32(self.as_seconds())
 	}
 }