From 25a3e041d9894c8188aff23c6a44565490394995 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 23 Apr 2021 17:30:07 -0700 Subject: [PATCH 1/4] Use constant for invoice's default expiry value --- lightning-invoice/src/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index b33b1b374d8..0e469ff1e86 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -54,6 +54,11 @@ const SYSTEM_TIME_MAX_UNIX_TIMESTAMP: u64 = std::i32::MAX as u64; /// 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; +/// Default expiry time as defined by [BOLT 11]. +/// +/// [BOLT 11]: https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md +const DEFAULT_EXPIRY_TIME: u64 = 3600; + /// 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, @@ -1041,11 +1046,11 @@ impl Invoice { self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0 } - /// Returns the invoice's expiry time if present + /// Returns the invoice's expiry time, if present, otherwise [`DEFAULT_EXPIRY_TIME`]. pub fn expiry_time(&self) -> Duration { self.signed_invoice.expiry_time() .map(|x| x.0) - .unwrap_or(Duration::from_secs(3600)) + .unwrap_or(Duration::from_secs(DEFAULT_EXPIRY_TIME)) } /// Returns the invoice's `min_cltv_expiry` time if present From e0ec0168171c1fca5c950fdc1bf7a8c930b21415 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 26 Apr 2021 11:59:56 -0700 Subject: [PATCH 2/4] Use default for invoice's min_final_cltv_expiry --- lightning-invoice/src/lib.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 0e469ff1e86..9c676e434fd 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -59,6 +59,11 @@ const MAX_EXPIRY_TIME: u64 = 60 * 60 * 24 * 356; /// [BOLT 11]: https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md const DEFAULT_EXPIRY_TIME: u64 = 3600; +/// Default minimum final CLTV expiry as defined by [BOLT 11]. +/// +/// [BOLT 11]: https://github.com/lightningnetwork/lightning-rfc/blob/master/11-payment-encoding.md +const DEFAULT_MIN_FINAL_CLTV_EXPIRY: u64 = 18; + /// 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, @@ -1053,9 +1058,12 @@ impl Invoice { .unwrap_or(Duration::from_secs(DEFAULT_EXPIRY_TIME)) } - /// Returns the invoice's `min_cltv_expiry` time if present - pub fn min_final_cltv_expiry(&self) -> Option { - self.signed_invoice.min_final_cltv_expiry().map(|x| x.0) + /// Returns the invoice's `min_final_cltv_expiry` time, if present, otherwise + /// [`DEFAULT_MIN_FINAL_CLTV_EXPIRY`]. + pub fn min_final_cltv_expiry(&self) -> u64 { + self.signed_invoice.min_final_cltv_expiry() + .map(|x| x.0) + .unwrap_or(DEFAULT_MIN_FINAL_CLTV_EXPIRY) } /// Returns a list of all fallback addresses @@ -1620,7 +1628,7 @@ mod test { ); assert_eq!(invoice.payee_pub_key(), Some(&public_key)); assert_eq!(invoice.expiry_time(), Duration::from_secs(54321)); - assert_eq!(invoice.min_final_cltv_expiry(), Some(144)); + assert_eq!(invoice.min_final_cltv_expiry(), 144); assert_eq!(invoice.fallbacks(), vec![&Fallback::PubKeyHash([0;20])]); assert_eq!(invoice.routes(), vec![&RouteHint(route_1), &RouteHint(route_2)]); assert_eq!( From 383ebc040642e34f90776aa66ac53d0b694d9e9d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 26 Apr 2021 13:08:19 -0700 Subject: [PATCH 3/4] Require min_final_cltv_expiry in invoice --- lightning-invoice/src/lib.rs | 54 +++++++++++++++++-------------- lightning-invoice/tests/ser_de.rs | 9 +++--- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 9c676e434fd..0b112a72a35 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -146,6 +146,7 @@ pub fn check_platform() { /// .description("Coins pls!".into()) /// .payment_hash(payment_hash) /// .current_timestamp() +/// .min_final_cltv_expiry(144) /// .build_signed(|hash| { /// Secp256k1::new().sign_recoverable(hash, &private_key) /// }) @@ -164,7 +165,7 @@ pub fn check_platform() { /// /// (C-not exported) as we likely need to manually select one set of boolean type parameters. #[derive(Eq, PartialEq, Debug, Clone)] -pub struct InvoiceBuilder { +pub struct InvoiceBuilder { currency: Currency, amount: Option, si_prefix: Option, @@ -175,6 +176,7 @@ pub struct InvoiceBuilder { phantom_d: std::marker::PhantomData, phantom_h: std::marker::PhantomData, phantom_t: std::marker::PhantomData, + phantom_c: std::marker::PhantomData, } /// Represents a syntactically and semantically correct lightning BOLT11 invoice. @@ -426,7 +428,7 @@ pub mod constants { pub const TAG_FEATURES: u8 = 5; } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Construct new, empty `InvoiceBuilder`. All necessary fields have to be filled first before /// `InvoiceBuilder::build(self)` becomes available. pub fn new(currrency: Currency) -> Self { @@ -441,14 +443,15 @@ impl InvoiceBuilder { phantom_d: std::marker::PhantomData, phantom_h: std::marker::PhantomData, phantom_t: std::marker::PhantomData, + phantom_c: std::marker::PhantomData, } } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Helper function to set the completeness flags. - fn set_flags(self) -> InvoiceBuilder { - InvoiceBuilder:: { + fn set_flags(self) -> InvoiceBuilder { + InvoiceBuilder:: { currency: self.currency, amount: self.amount, si_prefix: self.si_prefix, @@ -459,6 +462,7 @@ impl InvoiceBuilder { phantom_d: std::marker::PhantomData, phantom_h: std::marker::PhantomData, phantom_t: std::marker::PhantomData, + phantom_c: std::marker::PhantomData, } } @@ -494,12 +498,6 @@ impl InvoiceBuilder { self } - /// Sets `min_final_cltv_expiry`. - pub fn min_final_cltv_expiry(mut self, min_final_cltv_expiry: u64) -> Self { - self.tagged_fields.push(TaggedField::MinFinalCltvExpiry(MinFinalCltvExpiry(min_final_cltv_expiry))); - self - } - /// Adds a fallback address. pub fn fallback(mut self, fallback: Fallback) -> Self { self.tagged_fields.push(TaggedField::Fallback(fallback)); @@ -523,7 +521,7 @@ impl InvoiceBuilder { } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Builds a `RawInvoice` if no `CreationError` occurred while construction any of the fields. pub fn build_raw(self) -> Result { @@ -556,9 +554,9 @@ impl InvoiceBuilder { } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Set the description. This function is only available if no description (hash) was set. - pub fn description(mut self, description: String) -> InvoiceBuilder { + pub fn description(mut self, description: String) -> InvoiceBuilder { match Description::new(description) { Ok(d) => self.tagged_fields.push(TaggedField::Description(d)), Err(e) => self.error = Some(e), @@ -567,23 +565,23 @@ impl InvoiceBuilder { } /// Set the description hash. This function is only available if no description (hash) was set. - pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder { + pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder { self.tagged_fields.push(TaggedField::DescriptionHash(Sha256(description_hash))); self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Set the payment hash. This function is only available if no payment hash was set. - pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder { + pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder { self.tagged_fields.push(TaggedField::PaymentHash(Sha256(hash))); self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Sets the timestamp. - pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder { + 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), @@ -593,14 +591,22 @@ impl InvoiceBuilder { } /// Sets the timestamp to the current UNIX timestamp. - pub fn current_timestamp(mut self) -> InvoiceBuilder { + pub fn current_timestamp(mut self) -> InvoiceBuilder { let now = PositiveTimestamp::from_system_time(SystemTime::now()); self.timestamp = Some(now.expect("for the foreseeable future this shouldn't happen")); self.set_flags() } } -impl InvoiceBuilder { +impl InvoiceBuilder { + /// Sets `min_final_cltv_expiry`. + pub fn min_final_cltv_expiry(mut self, min_final_cltv_expiry: u64) -> InvoiceBuilder { + self.tagged_fields.push(TaggedField::MinFinalCltvExpiry(MinFinalCltvExpiry(min_final_cltv_expiry))); + self.set_flags() + } +} + +impl InvoiceBuilder { /// Builds and signs an invoice using the supplied `sign_function`. This function MAY NOT fail /// and MUST produce a recoverable signature valid for the given hash and if applicable also for /// the included payee public key. @@ -1489,7 +1495,8 @@ mod test { let builder = InvoiceBuilder::new(Currency::Bitcoin) .payment_hash(sha256::Hash::from_slice(&[0;32][..]).unwrap()) - .current_timestamp(); + .current_timestamp() + .min_final_cltv_expiry(144); let too_long_string = String::from_iter( (0..1024).map(|_| '?') @@ -1606,7 +1613,6 @@ mod test { .payee_pub_key(public_key.clone()) .expiry_time(Duration::from_secs(54321)) .min_final_cltv_expiry(144) - .min_final_cltv_expiry(143) .fallback(Fallback::PubKeyHash([0;20])) .route(route_1.clone()) .route(route_2.clone()) @@ -1618,7 +1624,7 @@ mod test { }).unwrap(); assert!(invoice.check_signature().is_ok()); - assert_eq!(invoice.tagged_fields().count(), 9); + assert_eq!(invoice.tagged_fields().count(), 8); assert_eq!(invoice.amount_pico_btc(), Some(123)); assert_eq!(invoice.currency(), Currency::BitcoinTestnet); diff --git a/lightning-invoice/tests/ser_de.rs b/lightning-invoice/tests/ser_de.rs index 403f8f1f0ee..ba3836940bc 100644 --- a/lightning-invoice/tests/ser_de.rs +++ b/lightning-invoice/tests/ser_de.rs @@ -110,13 +110,14 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { .amount_pico_btc(20000000000) .timestamp(UNIX_EPOCH + Duration::from_secs(1496314658)) .payment_secret(PaymentSecret([42; 32])) - .build_signed(|msg_hash| { + .build_raw() + .unwrap() + .sign::<_, ()>(|msg_hash| { let privkey = SecretKey::from_slice(&[41; 32]).unwrap(); let secp_ctx = Secp256k1::new(); - secp_ctx.sign_recoverable(msg_hash, &privkey) + Ok(secp_ctx.sign_recoverable(msg_hash, &privkey)) }) - .unwrap() - .into_signed_raw(), + .unwrap(), None ) ] From aafa741f291167a7f0e792ff0a361715fa816417 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 26 Apr 2021 14:24:36 -0700 Subject: [PATCH 4/4] Test default invoice field values --- lightning-invoice/src/lib.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 0b112a72a35..c178169890c 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -1646,4 +1646,28 @@ mod test { let raw_invoice = builder.build_raw().unwrap(); assert_eq!(raw_invoice, *invoice.into_signed_raw().raw_invoice()) } + + #[test] + fn test_default_values() { + use ::*; + use secp256k1::Secp256k1; + use secp256k1::key::SecretKey; + + let signed_invoice = InvoiceBuilder::new(Currency::Bitcoin) + .description("Test".into()) + .payment_hash(sha256::Hash::from_slice(&[0;32][..]).unwrap()) + .current_timestamp() + .build_raw() + .unwrap() + .sign::<_, ()>(|hash| { + let privkey = SecretKey::from_slice(&[41; 32]).unwrap(); + let secp_ctx = Secp256k1::new(); + Ok(secp_ctx.sign_recoverable(hash, &privkey)) + }) + .unwrap(); + let invoice = Invoice::from_signed(signed_invoice).unwrap(); + + assert_eq!(invoice.min_final_cltv_expiry(), DEFAULT_MIN_FINAL_CLTV_EXPIRY); + assert_eq!(invoice.expiry_time(), Duration::from_secs(DEFAULT_EXPIRY_TIME)); + } }