From 9363ee8e24ec9e6fe70baec58e9fc24f2895b13c Mon Sep 17 00:00:00 2001 From: Sebastian Geisler Date: Tue, 30 Oct 2018 00:32:52 -0700 Subject: [PATCH 1/4] Use std::time::SystemTime instead of u64 for timestamps --- README.md | 3 + src/de.rs | 29 +++++++-- src/lib.rs | 154 ++++++++++++++++++++++++++++++++++++++++++------ src/ser.rs | 9 ++- tests/ser_de.rs | 6 +- 5 files changed, 175 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 477564a1..ecb4b54f 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,9 @@ This repo provides datastructures for BOLT 11 lightning invoices. It provides functions to parse and serialize invoices from and to bech32. +**Please be sure to run the test suite since we need to check assumptions +regarding `SystemTime`'s bounds on your platform.** + ## Contributing * same coding style standard as [rust-bitcoin/rust-lightning](https://github.com/rust-bitcoin/rust-lightning) * use tabs and spaces (appropriately) diff --git a/src/de.rs b/src/de.rs index e0b5ba2e..44694397 100644 --- a/src/de.rs +++ b/src/de.rs @@ -311,8 +311,7 @@ impl FromBase32 for RawDataPart { return Err(ParseError::TooShortDataPart); } - let timestamp: u64 = parse_int_be(&data[0..7], 32) - .expect("7*5bit < 64bit, no overflow possible"); + let timestamp = PositiveTimestamp::from_base32(&data[0..7])?; let tagged = parse_tagged_parts(&data[7..])?; Ok(RawDataPart { @@ -322,6 +321,23 @@ impl FromBase32 for RawDataPart { } } +impl FromBase32 for PositiveTimestamp { + type Err = ParseError; + + fn from_base32(b32: &[u5]) -> Result { + if b32.len() != 7 { + return Err(ParseError::InvalidSliceLength("PositiveTimestamp::from_base32()".into())); + } + let timestamp: u64 = parse_int_be(b32, 32) + .expect("7*5bit < 64bit, no overflow possible"); + match PositiveTimestamp::from_unix_timestamp(timestamp) { + Ok(t) => Ok(t), + Err(CreationError::TimestampOutOfBounds) => Err(ParseError::TimestampOverflow), + Err(_) => unreachable!(), + } + } +} + impl FromBase32 for Signature { type Err = ParseError; fn from_base32(signature: &[u5]) -> Result { @@ -589,7 +605,8 @@ pub enum ParseError { InvalidScriptHashLength, InvalidRecoveryId, InvalidSliceLength(String), - Skip + Skip, + TimestampOverflow, } #[derive(PartialEq, Debug, Clone)] @@ -648,6 +665,7 @@ impl error::Error for ParseError { InvalidRecoveryId => "recovery id is out of range (should be in [0,3])", InvalidSliceLength(_) => "some slice had the wrong length", Skip => "the tagged field has to be skipped because of an unexpected, but allowed property", + TimestampOverflow => "the invoice's timestamp could not be represented as SystemTime", } } } @@ -924,7 +942,8 @@ mod test { fn test_raw_signed_invoice_deserialization() { use TaggedField::*; use secp256k1::{RecoveryId, RecoverableSignature, Secp256k1}; - use {SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256}; + use {SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, + PositiveTimestamp}; assert_eq!( "lnbc1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdpl2pkx2ctnv5sxxmmw\ @@ -938,7 +957,7 @@ mod test { si_prefix: None, }, data: RawDataPart { - timestamp: 1496314658, + timestamp: PositiveTimestamp::from_unix_timestamp(1496314658).unwrap(), tagged_fields: vec ! [ PaymentHash(Sha256(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" diff --git a/src/lib.rs b/src/lib.rs index 3aeeec92..8c8098a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,7 @@ use std::ops::Deref; use std::iter::FilterMap; use std::slice::Iter; +use std::time::{SystemTime, Duration, UNIX_EPOCH}; mod de; mod ser; @@ -20,6 +21,24 @@ mod tb; pub use de::{ParseError, ParseOrSemanticError}; + +// TODO: fix before 2038 (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; + +/// 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, +/// please open an issue. If all tests pass you should be able to use this library safely by just +/// removing this function till we patch it accordingly. +fn __system_time_size_check() { + /// Use 2 * sizeof(u64) as expected size since the expected underlying implementation is storing + /// a `Duration` since `SystemTime::UNIX_EPOCH`. + unsafe { std::mem::transmute::(UNIX_EPOCH); } +} + + /// Builder for `Invoice`s. It's the most convenient and advised way to use this library. It ensures /// that only a semantically and syntactically correct Invoice can be built using it. /// @@ -72,7 +91,7 @@ pub struct InvoiceBuilder { currency: Currency, amount: Option, si_prefix: Option, - timestamp: Option, + timestamp: Option, tagged_fields: Vec, error: Option, @@ -149,14 +168,22 @@ pub struct RawHrp { /// Data of the `RawInvoice` that is encoded in the data part #[derive(Eq, PartialEq, Debug, Clone)] pub struct RawDataPart { - // TODO: find better fitting type that only allows positive timestamps to avoid checks for negative timestamps when encoding - /// generation time of the invoice as UNIX timestamp - pub timestamp: u64, + /// generation time of the invoice + pub timestamp: PositiveTimestamp, /// tagged fields of the payment request pub tagged_fields: Vec, } +/// A timestamp that refers to a date after 1 January 1970 which means its representation as UNIX +/// timestamp is positive. +/// +/// # Invariants +/// The UNIX timestamp representing the stored time has to be positive and small enough so that +/// a `EpiryTime` can be added to it without an overflow. +#[derive(Eq, PartialEq, Debug, Clone)] +pub struct PositiveTimestamp(SystemTime); + /// SI prefixes for the human readable part #[derive(Eq, PartialEq, Debug, Clone, Copy)] pub enum SiPrefix { @@ -442,17 +469,30 @@ impl InvoiceBuilder { impl InvoiceBuilder { /// Sets the timestamp. `time` is a UNIX timestamp. - pub fn timestamp(mut self, time: u64) -> InvoiceBuilder { - self.timestamp = Some(time); + pub fn timestamp_raw(mut self, time: u64) -> InvoiceBuilder { + match PositiveTimestamp::from_unix_timestamp(time) { + Ok(t) => self.timestamp = Some(t), + Err(e) => self.error = Some(e), + } + + self.set_flags() + } + + /// Sets the timestamp. + pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder { + match PositiveTimestamp::from_system_time(time) { + Ok(t) => self.timestamp = Some(t), + Err(e) => self.error = Some(e), + } + self.set_flags() } /// Sets the timestamp to the current UNIX timestamp. pub fn current_timestamp(mut self) -> InvoiceBuilder { use std::time::{SystemTime, UNIX_EPOCH}; - let now = SystemTime::now(); - let since_unix_epoch = now.duration_since(UNIX_EPOCH).expect("it won't be 1970 ever again"); - self.timestamp = Some(since_unix_epoch.as_secs() as u64); + let now = PositiveTimestamp::from_system_time(SystemTime::now()); + self.timestamp = Some(now.expect("for the foreseeable future this shouldn't happen")); self.set_flags() } } @@ -717,6 +757,60 @@ impl RawInvoice { } } +impl PositiveTimestamp { + /// Create a new `PositiveTimestamp` from a unix timestamp in the Range + /// `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP`, otherwise return a + /// `CreationError::TimestampOutOfBounds`. + pub fn from_unix_timestamp(unix_seconds: u64) -> Result { + if unix_seconds > SYSTEM_TIME_MAX_UNIX_TIMESTAMP { + 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`, otherwise return a + /// `CreationError::TimestampOutOfBounds`. + pub fn from_system_time(time: SystemTime) -> Result { + if time + .duration_since(UNIX_EPOCH) + .map(|t| t.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP) // check for consistency reasons + .unwrap_or(true) + { + Ok(PositiveTimestamp(time)) + } else { + Err(CreationError::TimestampOutOfBounds) + } + } + + /// Returns the UNIX timestamp representing the stored time + pub fn as_unix_timestamp(&self) -> u64 { + self.0.duration_since(UNIX_EPOCH) + .expect("ensured by type contract/constructors") + .as_secs() + } + + /// Returns a reference to the internal `SystemTime` time representation + pub fn as_time(&self) -> &SystemTime { + &self.0 + } +} + +impl Into for PositiveTimestamp { + fn into(self) -> SystemTime { + self.0 + } +} + +impl Deref for PositiveTimestamp { + type Target = SystemTime; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + impl Invoice { fn into_signed_raw(self) -> SignedRawInvoice { self.signed_invoice @@ -788,8 +882,8 @@ impl Invoice { Ok(invoice) } - pub fn timestamp(&self) -> u64 { - self.signed_invoice.raw_invoice.data.timestamp + pub fn timestamp(&self) -> &SystemTime { + self.signed_invoice.raw_invoice().data.timestamp.as_time() } /// Returns an iterator over all tagged fields of this Invoice. @@ -967,6 +1061,9 @@ pub enum CreationError { /// The specified route has too many hops and can't be encoded RouteTooLong, + + /// The unix timestamp of the supplied date is <0 or can't be represented as `SystemTime` + TimestampOutOfBounds, } /// Errors that may occur when converting a `RawInvoice` to an `Invoice`. They relate to the @@ -997,9 +1094,27 @@ mod test { use bitcoin_hashes::hex::FromHex; use bitcoin_hashes::sha256::Sha256Hash; + #[test] + fn test_system_time_bounds_assumptions() { + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + + // The upper and lower bounds of `SystemTime` are not part of its public contract and are + // platform specific. That's why we have to test if our assumptions regarding these bounds + // hold on the target platform. + // + // If this test fails on your platform, please don't use the library and open an issue + // instead so we can resolve the situation. Currently this library is tested on: + // * Linux (64bit) + let fail_date = UNIX_EPOCH + Duration::from_secs(::SYSTEM_TIME_MAX_UNIX_TIMESTAMP); + let year = Duration::from_secs(60 * 60 * 24 * 365); + + // Make sure that the library will keep working for another year + assert!(fail_date.duration_since(SystemTime::now()).unwrap() > year) + } + #[test] fn test_calc_invoice_hash() { - use ::{RawInvoice, RawHrp, RawDataPart, Currency}; + use ::{RawInvoice, RawHrp, RawDataPart, Currency, PositiveTimestamp}; use secp256k1::*; use ::TaggedField::*; @@ -1010,7 +1125,7 @@ mod test { si_prefix: None, }, data: RawDataPart { - timestamp: 1496314658, + timestamp: PositiveTimestamp::from_unix_timestamp(1496314658).unwrap(), tagged_fields: vec![ PaymentHash(::Sha256(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" @@ -1036,7 +1151,8 @@ mod test { use TaggedField::*; use secp256k1::{RecoveryId, RecoverableSignature, Secp256k1}; use secp256k1::key::{SecretKey, PublicKey}; - use {SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256}; + use {SignedRawInvoice, Signature, RawInvoice, RawHrp, RawDataPart, Currency, Sha256, + PositiveTimestamp}; let mut invoice = SignedRawInvoice { raw_invoice: RawInvoice { @@ -1046,7 +1162,7 @@ mod test { si_prefix: None, }, data: RawDataPart { - timestamp: 1496314658, + timestamp: PositiveTimestamp::from_unix_timestamp(1496314658).unwrap(), tagged_fields: vec ! [ PaymentHash(Sha256(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" @@ -1181,6 +1297,7 @@ mod test { use ::*; use secp256k1::Secp256k1; use secp256k1::key::{SecretKey, PublicKey}; + use std::time::UNIX_EPOCH; let secp_ctx = Secp256k1::new(); @@ -1230,7 +1347,7 @@ mod test { let builder = InvoiceBuilder::new(Currency::BitcoinTestnet) .amount_pico_btc(123) - .timestamp(1234567) + .timestamp_raw(1234567) .payee_pub_key(public_key.clone()) .expiry_time_seconds(54321) .min_final_cltv_expiry(144) @@ -1250,7 +1367,10 @@ mod test { assert_eq!(invoice.amount_pico_btc(), Some(123)); assert_eq!(invoice.currency(), Currency::BitcoinTestnet); - assert_eq!(invoice.timestamp(), 1234567); + assert_eq!( + invoice.timestamp().duration_since(UNIX_EPOCH).unwrap().as_secs(), + 1234567 + ); assert_eq!(invoice.payee_pub_key(), Some(&PayeePubKey(public_key))); assert_eq!(invoice.expiry_time(), Some(&ExpiryTime{seconds: 54321})); assert_eq!(invoice.min_final_cltv_expiry(), Some(&MinFinalCltvExpiry(144))); diff --git a/src/ser.rs b/src/ser.rs index a2a90176..784c532b 100644 --- a/src/ser.rs +++ b/src/ser.rs @@ -119,7 +119,7 @@ impl ToBase32> for RawDataPart { let mut encoded = Vec::::new(); // encode timestamp - encoded.extend(&encode_int_be_base32(self.timestamp)); + encoded.extend(self.timestamp.to_base32()); // encode tagged fields for tagged_field in self.tagged_fields.iter() { @@ -130,6 +130,13 @@ impl ToBase32> for RawDataPart { } } +impl ToBase32> for PositiveTimestamp { + fn to_base32(&self) -> Vec { + try_stretch(encode_int_be_base32(self.as_unix_timestamp()), 7) + .expect("Can't be longer due than 7 u5s due to timestamp bounds") + } +} + impl ToBase32> for RawTaggedField { fn to_base32(&self) -> Vec { match *self { diff --git a/tests/ser_de.rs b/tests/ser_de.rs index 0dced6ce..057cbe46 100644 --- a/tests/ser_de.rs +++ b/tests/ser_de.rs @@ -16,7 +16,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { wd5kgetjypeh2ursdae8g6twvus8g6rfwvs8qun0dfjkxaq8rkx3yf5tcsyz3d73gafnh3cax9rn449d9p5uxz9\ ezhhypd0elx87sjle52x86fux2ypatgddc6k63n7erqz25le42c4u4ecky03ylcqca784w".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) - .timestamp(1496314658) + .timestamp_raw(1496314658) .payment_hash(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) @@ -45,7 +45,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { 9zw97j25emudupq63nyw24cg27h2rspfj9srp".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) .amount_pico_btc(2500000000) - .timestamp(1496314658) + .timestamp_raw(1496314658) .payment_hash(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) @@ -75,7 +75,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { hhr8wpald05e92xw006sq94mg8v2ndf4sefvf9sygkshp5zfem29trqq2yxxz7".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) .amount_pico_btc(20000000000) - .timestamp(1496314658) + .timestamp_raw(1496314658) .payment_hash(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) From 8bfd6df914dc9a9a1ffa628ddfe5577295c3c8a1 Mon Sep 17 00:00:00 2001 From: Sebastian Geisler Date: Fri, 2 Nov 2018 17:03:51 -0700 Subject: [PATCH 2/4] Use std::time::Duration instead of u64 for expiry time --- src/de.rs | 12 +++---- src/lib.rs | 94 ++++++++++++++++++++++++++++++++++++++++--------- src/ser.rs | 2 +- tests/ser_de.rs | 3 +- 4 files changed, 86 insertions(+), 25 deletions(-) diff --git a/src/de.rs b/src/de.rs index 44694397..48a05149 100644 --- a/src/de.rs +++ b/src/de.rs @@ -485,11 +485,11 @@ impl FromBase32 for ExpiryTime { type Err = ParseError; fn from_base32(field_data: &[u5]) -> Result { - let expiry = parse_int_be::(field_data, 32); - if let Some(expiry) = expiry { - Ok(ExpiryTime{seconds: expiry}) - } else { - Err(ParseError::IntegerOverflowError) + match parse_int_be::(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()); diff --git a/src/lib.rs b/src/lib.rs index 8c8098a2..71c5a93c 100644 --- a/src/lib.rs +++ b/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, @@ -261,11 +265,15 @@ pub struct Description(String); #[derive(Eq, PartialEq, Debug, Clone)] pub struct PayeePubKey(pub PublicKey); -/// Positive duration that defines when (relatively to the timestamp) in the future the invoice expires +/// 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 - MAX_EXPIRY_TIME)` 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)] @@ -381,9 +389,12 @@ impl InvoiceBuilder { self } - /// 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})); + /// Sets the expiry time + pub fn expiry_time(mut self, expiry_time: Duration) -> Self { + match ExpiryTime::from_duration(expiry_time) { + Ok(t) => self.tagged_fields.push(TaggedField::ExpiryTime(t)), + Err(e) => self.error = Some(e), + }; self } @@ -490,7 +501,7 @@ impl InvoiceBuilder { /// Sets the timestamp to the current UNIX timestamp. pub fn current_timestamp(mut self) -> InvoiceBuilder { - 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() @@ -759,10 +770,10 @@ impl RawInvoice { impl PositiveTimestamp { /// Create a new `PositiveTimestamp` from a unix timestamp in the Range - /// `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP`, 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 { - if unix_seconds > SYSTEM_TIME_MAX_UNIX_TIMESTAMP { + if unix_seconds > SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME { Err(CreationError::TimestampOutOfBounds) } else { Ok(PositiveTimestamp(UNIX_EPOCH + Duration::from_secs(unix_seconds))) @@ -770,12 +781,12 @@ impl PositiveTimestamp { } /// Create a new `PositiveTimestamp` from a `SystemTime` with a corresponding unix timestamp in - /// the Range `0...SYSTEM_TIME_MAX_UNIX_TIMESTAMP`, 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 { if time .duration_since(UNIX_EPOCH) - .map(|t| t.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP) // check for consistency reasons + .map(|t| t.as_secs() <= SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME) .unwrap_or(true) { Ok(PositiveTimestamp(time)) @@ -1009,6 +1020,32 @@ impl Deref for PayeePubKey { } } +impl ExpiryTime { + pub fn from_seconds(seconds: u64) -> Result { + if seconds <= MAX_EXPIRY_TIME { + Ok(ExpiryTime(Duration::from_secs(seconds))) + } else { + Err(CreationError::ExpiryTimeOutOfBounds) + } + } + + pub fn from_duration(duration: Duration) -> Result { + if duration.as_secs() <= 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) -> Result { if hops.len() <= 12 { @@ -1064,6 +1101,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 @@ -1109,7 +1149,27 @@ mod test { let year = Duration::from_secs(60 * 60 * 24 * 365); // Make sure that the library will keep working for another year - assert!(fail_date.duration_since(SystemTime::now()).unwrap() > year) + assert!(fail_date.duration_since(SystemTime::now()).unwrap() > year); + + let max_ts = ::PositiveTimestamp::from_unix_timestamp( + ::SYSTEM_TIME_MAX_UNIX_TIMESTAMP - ::MAX_EXPIRY_TIME + ).unwrap(); + let max_exp = ::ExpiryTime::from_seconds(::MAX_EXPIRY_TIME).unwrap(); + + assert_eq!( + (*max_ts.as_time() + *max_exp.as_duration()).duration_since(UNIX_EPOCH).unwrap().as_secs(), + ::SYSTEM_TIME_MAX_UNIX_TIMESTAMP + ); + + assert_eq!( + ::PositiveTimestamp::from_unix_timestamp(::SYSTEM_TIME_MAX_UNIX_TIMESTAMP + 1), + Err(::CreationError::TimestampOutOfBounds) + ); + + assert_eq!( + ::ExpiryTime::from_seconds(::MAX_EXPIRY_TIME + 1), + Err(::CreationError::ExpiryTimeOutOfBounds) + ); } #[test] @@ -1297,7 +1357,7 @@ mod test { use ::*; use secp256k1::Secp256k1; use secp256k1::key::{SecretKey, PublicKey}; - use std::time::UNIX_EPOCH; + use std::time::{UNIX_EPOCH, Duration}; let secp_ctx = Secp256k1::new(); @@ -1349,7 +1409,7 @@ mod test { .amount_pico_btc(123) .timestamp_raw(1234567) .payee_pub_key(public_key.clone()) - .expiry_time_seconds(54321) + .expiry_time(Duration::from_secs(54321)) .min_final_cltv_expiry(144) .min_final_cltv_expiry(143) .fallback(Fallback::PubKeyHash([0;20])) @@ -1372,7 +1432,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)]); diff --git a/src/ser.rs b/src/ser.rs index 784c532b..6e9f9d8d 100644 --- a/src/ser.rs +++ b/src/ser.rs @@ -170,7 +170,7 @@ impl ToBase32> for PayeePubKey { impl ToBase32> for ExpiryTime { fn to_base32(&self) -> Vec { - encode_int_be_base32(self.seconds) + encode_int_be_base32(self.as_seconds()) } } diff --git a/tests/ser_de.rs b/tests/ser_de.rs index 057cbe46..6c2e9293 100644 --- a/tests/ser_de.rs +++ b/tests/ser_de.rs @@ -6,6 +6,7 @@ use bitcoin_hashes::hex::FromHex; use bitcoin_hashes::sha256::Sha256Hash; use lightning_invoice::*; use secp256k1::{Secp256k1, RecoverableSignature, RecoveryId}; +use std::time::Duration; // TODO: add more of the examples from BOLT11 and generate ones causing SemanticErrors @@ -50,7 +51,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) .description("1 cup coffee".to_owned()) - .expiry_time_seconds(60) + .expiry_time(Duration::from_secs(60)) .build_raw() .unwrap() .sign(|_| { From ef9440a6284136acd1348816362e2c71f3cdd293 Mon Sep 17 00:00:00 2001 From: Sebastian Geisler Date: Tue, 13 Nov 2018 17:03:46 -0800 Subject: [PATCH 3/4] remove timestamp_raw function from builder --- src/lib.rs | 12 +----------- tests/ser_de.rs | 8 ++++---- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 71c5a93c..f729924b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -479,16 +479,6 @@ impl InvoiceBuilder { } impl InvoiceBuilder { - /// Sets the timestamp. `time` is a UNIX timestamp. - pub fn timestamp_raw(mut self, time: u64) -> InvoiceBuilder { - match PositiveTimestamp::from_unix_timestamp(time) { - Ok(t) => self.timestamp = Some(t), - Err(e) => self.error = Some(e), - } - - self.set_flags() - } - /// Sets the timestamp. pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder { match PositiveTimestamp::from_system_time(time) { @@ -1407,7 +1397,7 @@ mod test { let builder = InvoiceBuilder::new(Currency::BitcoinTestnet) .amount_pico_btc(123) - .timestamp_raw(1234567) + .timestamp(UNIX_EPOCH + Duration::from_secs(1234567)) .payee_pub_key(public_key.clone()) .expiry_time(Duration::from_secs(54321)) .min_final_cltv_expiry(144) diff --git a/tests/ser_de.rs b/tests/ser_de.rs index 6c2e9293..f48a306e 100644 --- a/tests/ser_de.rs +++ b/tests/ser_de.rs @@ -6,7 +6,7 @@ use bitcoin_hashes::hex::FromHex; use bitcoin_hashes::sha256::Sha256Hash; use lightning_invoice::*; use secp256k1::{Secp256k1, RecoverableSignature, RecoveryId}; -use std::time::Duration; +use std::time::{Duration, UNIX_EPOCH}; // TODO: add more of the examples from BOLT11 and generate ones causing SemanticErrors @@ -17,7 +17,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { wd5kgetjypeh2ursdae8g6twvus8g6rfwvs8qun0dfjkxaq8rkx3yf5tcsyz3d73gafnh3cax9rn449d9p5uxz9\ ezhhypd0elx87sjle52x86fux2ypatgddc6k63n7erqz25le42c4u4ecky03ylcqca784w".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) - .timestamp_raw(1496314658) + .timestamp(UNIX_EPOCH + Duration::from_secs(1496314658)) .payment_hash(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) @@ -46,7 +46,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { 9zw97j25emudupq63nyw24cg27h2rspfj9srp".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) .amount_pico_btc(2500000000) - .timestamp_raw(1496314658) + .timestamp(UNIX_EPOCH + Duration::from_secs(1496314658)) .payment_hash(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) @@ -76,7 +76,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { hhr8wpald05e92xw006sq94mg8v2ndf4sefvf9sygkshp5zfem29trqq2yxxz7".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) .amount_pico_btc(20000000000) - .timestamp_raw(1496314658) + .timestamp(UNIX_EPOCH + Duration::from_secs(1496314658)) .payment_hash(Sha256Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" ).unwrap()) From b2f9b12810b416229b4e1a50237dd3d39c51e994 Mon Sep 17 00:00:00 2001 From: Sebastian Geisler Date: Tue, 13 Nov 2018 18:40:07 -0800 Subject: [PATCH 4/4] add check_platform function --- README.md | 3 ++- src/lib.rs | 66 ++++++++++++++++++++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index ecb4b54f..465e2db1 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,8 @@ This repo provides datastructures for BOLT 11 lightning invoices. It provides functions to parse and serialize invoices from and to bech32. **Please be sure to run the test suite since we need to check assumptions -regarding `SystemTime`'s bounds on your platform.** +regarding `SystemTime`'s bounds on your platform. You can also call `check_platform` +on startup or in your test suite to do so.** ## Contributing * same coding style standard as [rust-bitcoin/rust-lightning](https://github.com/rust-bitcoin/rust-lightning) diff --git a/src/lib.rs b/src/lib.rs index f729924b..4de7f6ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,6 +43,47 @@ fn __system_time_size_check() { } +/// **Call this function on startup to ensure that all assumptions about the platform are valid.** +/// +/// Unfortunately we have to make assumptions about the upper bounds of the `SystemTime` type on +/// your platform which we can't fully verify at compile time and which isn't part of it's contract. +/// To our best knowledge our assumptions hold for all platforms officially supported by rust, but +/// since this check is fast we recommend to do it anyway. +/// +/// If this function fails this is considered a bug. Please open an issue describing your +/// platform and stating your current system time. +/// +/// # Panics +/// If the check fails this function panics. By calling this function on startup you ensure that +/// this wont happen at an arbitrary later point in time. +pub fn check_platform() { + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + + // The upper and lower bounds of `SystemTime` are not part of its public contract and are + // platform specific. That's why we have to test if our assumptions regarding these bounds + // hold on the target platform. + // + // If this test fails on your platform, please don't use the library and open an issue + // instead so we can resolve the situation. Currently this library is tested on: + // * Linux (64bit) + let fail_date = UNIX_EPOCH + Duration::from_secs(SYSTEM_TIME_MAX_UNIX_TIMESTAMP); + let year = Duration::from_secs(60 * 60 * 24 * 365); + + // Make sure that the library will keep working for another year + assert!(fail_date.duration_since(SystemTime::now()).unwrap() > year); + + let max_ts = PositiveTimestamp::from_unix_timestamp( + SYSTEM_TIME_MAX_UNIX_TIMESTAMP - MAX_EXPIRY_TIME + ).unwrap(); + let max_exp = ::ExpiryTime::from_seconds(MAX_EXPIRY_TIME).unwrap(); + + assert_eq!( + (*max_ts.as_time() + *max_exp.as_duration()).duration_since(UNIX_EPOCH).unwrap().as_secs(), + SYSTEM_TIME_MAX_UNIX_TIMESTAMP + ); +} + + /// Builder for `Invoice`s. It's the most convenient and advised way to use this library. It ensures /// that only a semantically and syntactically correct Invoice can be built using it. /// @@ -1126,30 +1167,7 @@ mod test { #[test] fn test_system_time_bounds_assumptions() { - use std::time::{Duration, SystemTime, UNIX_EPOCH}; - - // The upper and lower bounds of `SystemTime` are not part of its public contract and are - // platform specific. That's why we have to test if our assumptions regarding these bounds - // hold on the target platform. - // - // If this test fails on your platform, please don't use the library and open an issue - // instead so we can resolve the situation. Currently this library is tested on: - // * Linux (64bit) - let fail_date = UNIX_EPOCH + Duration::from_secs(::SYSTEM_TIME_MAX_UNIX_TIMESTAMP); - let year = Duration::from_secs(60 * 60 * 24 * 365); - - // Make sure that the library will keep working for another year - assert!(fail_date.duration_since(SystemTime::now()).unwrap() > year); - - let max_ts = ::PositiveTimestamp::from_unix_timestamp( - ::SYSTEM_TIME_MAX_UNIX_TIMESTAMP - ::MAX_EXPIRY_TIME - ).unwrap(); - let max_exp = ::ExpiryTime::from_seconds(::MAX_EXPIRY_TIME).unwrap(); - - assert_eq!( - (*max_ts.as_time() + *max_exp.as_duration()).duration_since(UNIX_EPOCH).unwrap().as_secs(), - ::SYSTEM_TIME_MAX_UNIX_TIMESTAMP - ); + ::check_platform(); assert_eq!( ::PositiveTimestamp::from_unix_timestamp(::SYSTEM_TIME_MAX_UNIX_TIMESTAMP + 1),