diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index cc6049f0360..29c093dec95 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -168,7 +168,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, @@ -180,6 +180,7 @@ pub struct InvoiceBuilder { phantom_h: std::marker::PhantomData, phantom_t: std::marker::PhantomData, phantom_c: std::marker::PhantomData, + phantom_s: std::marker::PhantomData, } /// Represents a syntactically and semantically correct lightning BOLT11 invoice. @@ -432,7 +433,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 { @@ -448,14 +449,15 @@ impl InvoiceBuilder { phantom_h: std::marker::PhantomData, phantom_t: std::marker::PhantomData, phantom_c: std::marker::PhantomData, + phantom_s: 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, @@ -467,6 +469,7 @@ impl InvoiceBuilder InvoiceBuilder Self { - self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret)); - self - } - /// Sets the expiry time pub fn expiry_time(mut self, expiry_time: Duration) -> Self { match ExpiryTime::from_duration(expiry_time) { @@ -516,16 +513,9 @@ impl InvoiceBuilder Self { - self.tagged_fields.push(TaggedField::Features(features)); - self - } } -impl InvoiceBuilder { +impl InvoiceBuilder { /// Builds a `RawInvoice` if no `CreationError` occurred while construction any of the fields. pub fn build_raw(self) -> Result { @@ -558,9 +548,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), @@ -569,23 +559,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), @@ -595,22 +585,48 @@ 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 { + 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 { +impl InvoiceBuilder { + /// Sets the payment secret and relevant features. + pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder { + let features = InvoiceFeatures::empty() + .set_variable_length_onion_required() + .set_payment_secret_required(); + self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret)); + self.tagged_fields.push(TaggedField::Features(features)); + self.set_flags() + } +} + +impl InvoiceBuilder { + /// Sets the `basic_mpp` feature as optional. + pub fn basic_mpp(mut self) -> Self { + self.tagged_fields = self.tagged_fields + .drain(..) + .map(|field| match field { + TaggedField::Features(f) => TaggedField::Features(f.set_basic_mpp_optional()), + _ => field, + }) + .collect(); + self + } +} + +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. @@ -649,6 +665,7 @@ impl InvoiceBuilder { }; invoice.check_field_counts().expect("should be ensured by type signature of builder"); + invoice.check_feature_bits().expect("should be ensured by type signature of builder"); Ok(invoice) } @@ -977,6 +994,41 @@ impl Invoice { Ok(()) } + /// Check that feature bits are set as required + fn check_feature_bits(&self) -> Result<(), SemanticError> { + // "If the payment_secret feature is set, MUST include exactly one s field." + let payment_secret_count = self.tagged_fields().filter(|&tf| match *tf { + TaggedField::PaymentSecret(_) => true, + _ => false, + }).count(); + if payment_secret_count > 1 { + return Err(SemanticError::MultiplePaymentSecrets); + } + + // "A writer MUST set an s field if and only if the payment_secret feature is set." + let has_payment_secret = payment_secret_count == 1; + let features = self.tagged_fields().find(|&tf| match *tf { + TaggedField::Features(_) => true, + _ => false, + }); + match features { + None if has_payment_secret => Err(SemanticError::InvalidFeatures), + None => Ok(()), + Some(TaggedField::Features(features)) => { + if features.supports_payment_secret() && has_payment_secret { + Ok(()) + } else if has_payment_secret { + Err(SemanticError::InvalidFeatures) + } else if features.supports_payment_secret() { + Err(SemanticError::InvalidFeatures) + } else { + Ok(()) + } + }, + Some(_) => unreachable!(), + } + } + /// Check that the invoice is signed correctly and that key recovery works pub fn check_signature(&self) -> Result<(), SemanticError> { match self.signed_invoice.recover_payee_pub_key() { @@ -1011,6 +1063,7 @@ impl Invoice { signed_invoice: signed_invoice, }; invoice.check_field_counts()?; + invoice.check_feature_bits()?; invoice.check_signature()?; Ok(invoice) @@ -1303,6 +1356,12 @@ pub enum SemanticError { /// The invoice contains multiple descriptions and/or description hashes which isn't allowed MultipleDescriptions, + /// The invoice contains multiple payment secrets + MultiplePaymentSecrets, + + /// The invoice's features are invalid + InvalidFeatures, + /// The recovery id doesn't fit the signature/pub key InvalidRecoveryId, @@ -1317,6 +1376,8 @@ impl Display for SemanticError { SemanticError::MultiplePaymentHashes => f.write_str("The invoice has multiple payment hashes which isn't allowed"), SemanticError::NoDescription => f.write_str("No description or description hash are part of the invoice"), SemanticError::MultipleDescriptions => f.write_str("The invoice contains multiple descriptions and/or description hashes which isn't allowed"), + SemanticError::MultiplePaymentSecrets => f.write_str("The invoice contains multiple payment secrets"), + SemanticError::InvalidFeatures => f.write_str("The invoice's features are invalid"), SemanticError::InvalidRecoveryId => f.write_str("The recovery id doesn't fit the signature/pub key"), SemanticError::InvalidSignature => f.write_str("The invoice's signature is invalid"), } @@ -1467,6 +1528,97 @@ mod test { assert!(new_signed.check_signature()); } + #[test] + fn test_check_feature_bits() { + use TaggedField::*; + use lightning::ln::features::InvoiceFeatures; + use secp256k1::Secp256k1; + use secp256k1::key::SecretKey; + use {RawInvoice, RawHrp, RawDataPart, Currency, Sha256, PositiveTimestamp, Invoice, + SemanticError}; + + let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); + let payment_secret = lightning::ln::PaymentSecret([21; 32]); + let invoice_template = RawInvoice { + hrp: RawHrp { + currency: Currency::Bitcoin, + raw_amount: None, + si_prefix: None, + }, + data: RawDataPart { + timestamp: PositiveTimestamp::from_unix_timestamp(1496314658).unwrap(), + tagged_fields: vec ! [ + PaymentHash(Sha256(sha256::Hash::from_hex( + "0001020304050607080900010203040506070809000102030405060708090102" + ).unwrap())).into(), + Description( + ::Description::new( + "Please consider supporting this project".to_owned() + ).unwrap() + ).into(), + ], + }, + }; + + // Missing features + let invoice = { + let mut invoice = invoice_template.clone(); + invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures)); + + // Missing feature bits + let invoice = { + let mut invoice = invoice_template.clone(); + invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); + invoice.data.tagged_fields.push(Features(InvoiceFeatures::empty()).into()); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures)); + + // Including payment secret and feature bits + let invoice = { + let mut invoice = invoice_template.clone(); + invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); + invoice.data.tagged_fields.push(Features(InvoiceFeatures::known()).into()); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert!(Invoice::from_signed(invoice).is_ok()); + + // No payment secret or features + let invoice = { + let invoice = invoice_template.clone(); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert!(Invoice::from_signed(invoice).is_ok()); + + // No payment secret or feature bits + let invoice = { + let mut invoice = invoice_template.clone(); + invoice.data.tagged_fields.push(Features(InvoiceFeatures::empty()).into()); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert!(Invoice::from_signed(invoice).is_ok()); + + // Missing payment secret + let invoice = { + let mut invoice = invoice_template.clone(); + invoice.data.tagged_fields.push(Features(InvoiceFeatures::known()).into()); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures)); + + // Multiple payment secrets + let invoice = { + let mut invoice = invoice_template.clone(); + invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); + invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); + invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) + }.unwrap(); + assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::MultiplePaymentSecrets)); + } + #[test] fn test_builder_amount() { use ::*; @@ -1624,14 +1776,16 @@ mod test { .route(route_1.clone()) .route(route_2.clone()) .description_hash(sha256::Hash::from_slice(&[3;32][..]).unwrap()) - .payment_hash(sha256::Hash::from_slice(&[21;32][..]).unwrap()); + .payment_hash(sha256::Hash::from_slice(&[21;32][..]).unwrap()) + .payment_secret(PaymentSecret([42; 32])) + .basic_mpp(); let invoice = builder.clone().build_signed(|hash| { secp_ctx.sign_recoverable(hash, &private_key) }).unwrap(); assert!(invoice.check_signature().is_ok()); - assert_eq!(invoice.tagged_fields().count(), 8); + assert_eq!(invoice.tagged_fields().count(), 10); assert_eq!(invoice.amount_pico_btc(), Some(123)); assert_eq!(invoice.currency(), Currency::BitcoinTestnet); @@ -1649,6 +1803,8 @@ mod test { InvoiceDescription::Hash(&Sha256(sha256::Hash::from_slice(&[3;32][..]).unwrap())) ); assert_eq!(invoice.payment_hash(), &sha256::Hash::from_slice(&[21;32][..]).unwrap()); + assert_eq!(invoice.payment_secret(), Some(&PaymentSecret([42; 32]))); + assert_eq!(invoice.features(), Some(&InvoiceFeatures::known())); let raw_invoice = builder.build_raw().unwrap(); assert_eq!(raw_invoice, *invoice.into_signed_raw().raw_invoice()) diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 20247ed321a..70df3c45531 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -6,7 +6,6 @@ use lightning::chain; use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use lightning::chain::keysinterface::{Sign, KeysInterface}; use lightning::ln::channelmanager::{ChannelManager, MIN_FINAL_CLTV_EXPIRY}; -use lightning::ln::features::InvoiceFeatures; use lightning::routing::network_graph::RoutingFees; use lightning::routing::router::RouteHintHop; use lightning::util::logger::Logger; @@ -65,7 +64,7 @@ where .payee_pub_key(our_node_pubkey) .payment_hash(Hash::from_slice(&payment_hash.0).unwrap()) .payment_secret(payment_secret) - .features(InvoiceFeatures::known()) + .basic_mpp() .min_final_cltv_expiry(MIN_FINAL_CLTV_EXPIRY.into()); if let Some(amt) = amt_msat { invoice = invoice.amount_pico_btc(amt * 10); diff --git a/lightning-invoice/tests/ser_de.rs b/lightning-invoice/tests/ser_de.rs index d641c54273c..442166e740e 100644 --- a/lightning-invoice/tests/ser_de.rs +++ b/lightning-invoice/tests/ser_de.rs @@ -103,7 +103,7 @@ fn get_test_tuples() -> Vec<(String, SignedRawInvoice, Option)> { None ), ( - "lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp59g4z52329g4z52329g4z52329g4z52329g4z52329g4z52329g4q9gkzyrw8zhfxmrcxsx7hj40yejq6lkvn75l9yjmapjv94haz8x8jy2tvmgex8rnyqkj825csd2t64fu0p4ctad2cf4tgy5gh2fns6ygp6pnc3y".to_owned(), + "lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp59g4z52329g4z52329g4z52329g4z52329g4z52329g4z52329g4q9qrsgqzfhag3vsafx4e5qssalvw4rn0phsvpp3e5h2xxyk9l8fxsutvndx9t840dqvdrlu2gqmk0q8apqrgnjy9amc07hmjl9e9yzqjks5w2gqgjnyms".to_owned(), InvoiceBuilder::new(Currency::Bitcoin) .payment_hash(sha256::Hash::from_hex( "0001020304050607080900010203040506070809000102030405060708090102" diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 02e403b46d9..99e5cd0994c 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -95,7 +95,7 @@ mod sealed { // Byte 0 , // Byte 1 - StaticRemoteKey | PaymentSecret, + VariableLengthOnion | StaticRemoteKey | PaymentSecret, // Byte 2 , // Byte 3 @@ -105,7 +105,7 @@ mod sealed { // Byte 0 DataLossProtect | InitialRoutingSync | UpfrontShutdownScript | GossipQueries, // Byte 1 - VariableLengthOnion, + , // Byte 2 BasicMPP, // Byte 3 @@ -117,7 +117,7 @@ mod sealed { // Byte 0 , // Byte 1 - StaticRemoteKey | PaymentSecret, + VariableLengthOnion | StaticRemoteKey | PaymentSecret, // Byte 2 , // Byte 3 @@ -127,7 +127,7 @@ mod sealed { // Byte 0 DataLossProtect | UpfrontShutdownScript | GossipQueries, // Byte 1 - VariableLengthOnion, + , // Byte 2 BasicMPP, // Byte 3 @@ -143,7 +143,7 @@ mod sealed { // Byte 0 , // Byte 1 - PaymentSecret, + VariableLengthOnion | PaymentSecret, // Byte 2 , ], @@ -151,7 +151,7 @@ mod sealed { // Byte 0 , // Byte 1 - VariableLengthOnion, + , // Byte 2 BasicMPP, ], @@ -646,11 +646,8 @@ impl Features { pub(crate) fn requires_payment_secret(&self) -> bool { ::requires_feature(&self.flags) } - // Note that we never need to test this since what really matters is the invoice - iff the - // invoice provides a payment_secret, we assume that we can use it (ie that the recipient - // supports payment_secret). - #[allow(dead_code)] - pub(crate) fn supports_payment_secret(&self) -> bool { + /// Returns whether the `payment_secret` feature is supported. + pub fn supports_payment_secret(&self) -> bool { ::supports_feature(&self.flags) } } @@ -730,8 +727,10 @@ mod tests { assert!(InitFeatures::known().supports_variable_length_onion()); assert!(NodeFeatures::known().supports_variable_length_onion()); - assert!(!InitFeatures::known().requires_variable_length_onion()); - assert!(!NodeFeatures::known().requires_variable_length_onion()); + assert!(InvoiceFeatures::known().supports_variable_length_onion()); + assert!(InitFeatures::known().requires_variable_length_onion()); + assert!(NodeFeatures::known().requires_variable_length_onion()); + assert!(InvoiceFeatures::known().requires_variable_length_onion()); assert!(InitFeatures::known().supports_static_remote_key()); assert!(NodeFeatures::known().supports_static_remote_key()); @@ -740,13 +739,17 @@ mod tests { assert!(InitFeatures::known().supports_payment_secret()); assert!(NodeFeatures::known().supports_payment_secret()); + assert!(InvoiceFeatures::known().supports_payment_secret()); assert!(InitFeatures::known().requires_payment_secret()); assert!(NodeFeatures::known().requires_payment_secret()); + assert!(InvoiceFeatures::known().requires_payment_secret()); assert!(InitFeatures::known().supports_basic_mpp()); assert!(NodeFeatures::known().supports_basic_mpp()); + assert!(InvoiceFeatures::known().supports_basic_mpp()); assert!(!InitFeatures::known().requires_basic_mpp()); assert!(!NodeFeatures::known().requires_basic_mpp()); + assert!(!InvoiceFeatures::known().requires_basic_mpp()); assert!(InitFeatures::known().supports_shutdown_anysegwit()); assert!(NodeFeatures::known().supports_shutdown_anysegwit()); @@ -787,12 +790,12 @@ mod tests { { // Check that the flags are as expected: // - option_data_loss_protect - // - var_onion_optin | static_remote_key (req) | payment_secret(req) + // - var_onion_optin (req) | static_remote_key (req) | payment_secret(req) // - basic_mpp // - opt_shutdown_anysegwit assert_eq!(node_features.flags.len(), 4); assert_eq!(node_features.flags[0], 0b00000010); - assert_eq!(node_features.flags[1], 0b01010010); + assert_eq!(node_features.flags[1], 0b01010001); assert_eq!(node_features.flags[2], 0b00000010); assert_eq!(node_features.flags[3], 0b00001000); }