From 0e1723dc74b5261604f43e19eaf89590ab8539d4 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 28 Oct 2024 14:49:21 -0500 Subject: [PATCH] Owned and ref versions of Bolt11InvoiceDescription Split Bolt11InvoiceDescription into a version used with references to the description or description hash in the invoice and an owned version of these for when constructing an invoice. The latter is useful as it removes an unnecessary clone and can be used in a future change specifying either a description or description hash in larger set of invoice parameters. Since it doesn't use a reference, it can be exposed in bindings as well. --- lightning-invoice/src/lib.rs | 38 +++++++++++++++++++++++-------- lightning/src/ln/invoice_utils.rs | 28 +++++++++++------------ 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 8baedbe26..9f049deb7 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -233,13 +233,33 @@ pub struct Bolt11Invoice { signed_invoice: SignedRawBolt11Invoice, } +/// Represents the description of an invoice which has to be either a directly included string or +/// a hash of a description provided out of band. +#[derive(Eq, PartialEq, Debug, Clone, Ord, PartialOrd)] +pub enum Bolt11InvoiceDescription { + /// Description of what the invoice is for + Direct(Description), + + /// Hash of the description of what the invoice is for + Hash(Sha256), +} + +impl Display for Bolt11InvoiceDescription { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + Bolt11InvoiceDescription::Direct(desc) => write!(f, "{}", desc.0), + Bolt11InvoiceDescription::Hash(hash) => write!(f, "{}", hash.0), + } + } +} + /// Represents the description of an invoice which has to be either a directly included string or /// a hash of a description provided out of band. /// /// This is not exported to bindings users as we don't have a good way to map the reference lifetimes making this /// practically impossible to use safely in languages like C. #[derive(Eq, PartialEq, Debug, Clone, Ord, PartialOrd)] -pub enum Bolt11InvoiceDescription<'f> { +pub enum Bolt11InvoiceDescriptionRef<'f> { /// Reference to the directly supplied description in the invoice Direct(&'f Description), @@ -247,11 +267,11 @@ pub enum Bolt11InvoiceDescription<'f> { Hash(&'f Sha256), } -impl<'f> Display for Bolt11InvoiceDescription<'f> { +impl<'f> Display for Bolt11InvoiceDescriptionRef<'f> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { - Bolt11InvoiceDescription::Direct(desc) => write!(f, "{}", desc.0), - Bolt11InvoiceDescription::Hash(hash) => write!(f, "{}", hash.0), + Bolt11InvoiceDescriptionRef::Direct(desc) => write!(f, "{}", desc.0), + Bolt11InvoiceDescriptionRef::Hash(hash) => write!(f, "{}", hash.0), } } } @@ -708,7 +728,7 @@ impl InvoiceBui pub fn invoice_description(self, description: Bolt11InvoiceDescription) -> InvoiceBuilder { match description { Bolt11InvoiceDescription::Direct(desc) => { - self.description(desc.clone().into_inner().0) + self.description(desc.0.0) } Bolt11InvoiceDescription::Hash(hash) => { self.description_hash(hash.0) @@ -1374,11 +1394,11 @@ impl Bolt11Invoice { /// Return the description or a hash of it for longer ones /// /// This is not exported to bindings users because we don't yet export Bolt11InvoiceDescription - pub fn description(&self) -> Bolt11InvoiceDescription { + pub fn description(&self) -> Bolt11InvoiceDescriptionRef { if let Some(direct) = self.signed_invoice.description() { - return Bolt11InvoiceDescription::Direct(direct); + return Bolt11InvoiceDescriptionRef::Direct(direct); } else if let Some(hash) = self.signed_invoice.description_hash() { - return Bolt11InvoiceDescription::Hash(hash); + return Bolt11InvoiceDescriptionRef::Hash(hash); } unreachable!("ensured by constructor"); } @@ -2211,7 +2231,7 @@ mod test { assert_eq!(invoice.private_routes(), vec![&PrivateRoute(route_1), &PrivateRoute(route_2)]); assert_eq!( invoice.description(), - Bolt11InvoiceDescription::Hash(&Sha256(sha256::Hash::from_slice(&[3;32][..]).unwrap())) + Bolt11InvoiceDescriptionRef::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(), &PaymentSecret([42; 32])); diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs index 611e3c053..06f8a76e0 100644 --- a/lightning/src/ln/invoice_utils.rs +++ b/lightning/src/ln/invoice_utils.rs @@ -75,7 +75,7 @@ where L::Target: Logger, { let description = Description::new(description).map_err(SignOrCreationError::CreationError)?; - let description = Bolt11InvoiceDescription::Direct(&description,); + let description = Bolt11InvoiceDescription::Direct(description); _create_phantom_invoice::( amt_msat, payment_hash, description, invoice_expiry_delta_secs, phantom_route_hints, entropy_source, node_signer, logger, network, min_final_cltv_expiry_delta, duration_since_epoch, @@ -130,7 +130,7 @@ where L::Target: Logger, { _create_phantom_invoice::( - amt_msat, payment_hash, Bolt11InvoiceDescription::Hash(&description_hash), + amt_msat, payment_hash, Bolt11InvoiceDescription::Hash(description_hash), invoice_expiry_delta_secs, phantom_route_hints, entropy_source, node_signer, logger, network, min_final_cltv_expiry_delta, duration_since_epoch, ) @@ -161,7 +161,7 @@ where let invoice = match description { Bolt11InvoiceDescription::Direct(description) => { - InvoiceBuilder::new(network).description(description.as_inner().0.clone()) + InvoiceBuilder::new(network).description(description.into_inner().0) } Bolt11InvoiceDescription::Hash(hash) => InvoiceBuilder::new(network).description_hash(hash.0), }; @@ -424,7 +424,7 @@ where { _create_invoice_from_channelmanager_and_duration_since_epoch( channelmanager, node_signer, logger, network, amt_msat, - Bolt11InvoiceDescription::Hash(&description_hash), + Bolt11InvoiceDescription::Hash(description_hash), duration_since_epoch, invoice_expiry_delta_secs, min_final_cltv_expiry_delta, ) } @@ -454,7 +454,7 @@ where _create_invoice_from_channelmanager_and_duration_since_epoch( channelmanager, node_signer, logger, network, amt_msat, Bolt11InvoiceDescription::Direct( - &Description::new(description).map_err(SignOrCreationError::CreationError)?, + Description::new(description).map_err(SignOrCreationError::CreationError)?, ), duration_since_epoch, invoice_expiry_delta_secs, min_final_cltv_expiry_delta, ) @@ -518,7 +518,7 @@ where .map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?; _create_invoice_from_channelmanager_and_duration_since_epoch_with_payment_hash( channelmanager, node_signer, logger, network, amt_msat, - Bolt11InvoiceDescription::Hash(&description_hash), + Bolt11InvoiceDescription::Hash(description_hash), duration_since_epoch, invoice_expiry_delta_secs, payment_hash, payment_secret, min_final_cltv_expiry_delta, ) @@ -551,7 +551,7 @@ where _create_invoice_from_channelmanager_and_duration_since_epoch_with_payment_hash( channelmanager, node_signer, logger, network, amt_msat, Bolt11InvoiceDescription::Direct( - &Description::new(description).map_err(SignOrCreationError::CreationError)?, + Description::new(description).map_err(SignOrCreationError::CreationError)?, ), duration_since_epoch, invoice_expiry_delta_secs, payment_hash, payment_secret, min_final_cltv_expiry_delta, @@ -586,7 +586,7 @@ where let invoice = match description { Bolt11InvoiceDescription::Direct(description) => { - InvoiceBuilder::new(network).description(description.as_inner().0.clone()) + InvoiceBuilder::new(network).description(description.into_inner().0) } Bolt11InvoiceDescription::Hash(hash) => InvoiceBuilder::new(network).description_hash(hash.0), }; @@ -864,7 +864,7 @@ impl<'a, 'b, L: Deref> WithChannelDetails<'a, 'b, L> where L::Target: Logger { mod test { use super::*; use core::time::Duration; - use lightning_invoice::{Currency, Description, Bolt11InvoiceDescription, SignOrCreationError, CreationError}; + use lightning_invoice::{Currency, Description, Bolt11InvoiceDescriptionRef, SignOrCreationError, CreationError}; use bitcoin::hashes::{Hash, sha256}; use bitcoin::hashes::sha256::Hash as Sha256; use crate::sign::PhantomKeysManager; @@ -921,7 +921,7 @@ mod test { assert_eq!(invoice.amount_milli_satoshis(), Some(10_000)); // If no `min_final_cltv_expiry_delta` is specified, then it should be `MIN_FINAL_CLTV_EXPIRY_DELTA`. assert_eq!(invoice.min_final_cltv_expiry_delta(), MIN_FINAL_CLTV_EXPIRY_DELTA as u64); - assert_eq!(invoice.description(), Bolt11InvoiceDescription::Direct(&Description::new("test".to_string()).unwrap())); + assert_eq!(invoice.description(), Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())); assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into())); // Invoice SCIDs should always use inbound SCID aliases over the real channel ID, if one is @@ -1009,7 +1009,7 @@ mod test { ).unwrap(); assert_eq!(invoice.amount_milli_satoshis(), Some(10_000)); assert_eq!(invoice.min_final_cltv_expiry_delta(), MIN_FINAL_CLTV_EXPIRY_DELTA as u64); - assert_eq!(invoice.description(), Bolt11InvoiceDescription::Hash(&Sha256(Sha256::hash("Testing description_hash".as_bytes())))); + assert_eq!(invoice.description(), Bolt11InvoiceDescriptionRef::Hash(&Sha256(Sha256::hash("Testing description_hash".as_bytes())))); } #[test] @@ -1026,7 +1026,7 @@ mod test { ).unwrap(); assert_eq!(invoice.amount_milli_satoshis(), Some(10_000)); assert_eq!(invoice.min_final_cltv_expiry_delta(), MIN_FINAL_CLTV_EXPIRY_DELTA as u64); - assert_eq!(invoice.description(), Bolt11InvoiceDescription::Direct(&Description::new("test".to_string()).unwrap())); + assert_eq!(invoice.description(), Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())); assert_eq!(invoice.payment_hash(), &sha256::Hash::from_slice(&payment_hash.0[..]).unwrap()); } @@ -1379,7 +1379,7 @@ mod test { }; assert_eq!(invoice.min_final_cltv_expiry_delta(), MIN_FINAL_CLTV_EXPIRY_DELTA as u64); - assert_eq!(invoice.description(), Bolt11InvoiceDescription::Direct(&Description::new("test".to_string()).unwrap())); + assert_eq!(invoice.description(), Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())); assert_eq!(invoice.route_hints().len(), 2); assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into())); assert!(!invoice.features().unwrap().supports_basic_mpp()); @@ -1498,7 +1498,7 @@ mod test { assert_eq!(invoice.amount_milli_satoshis(), Some(20_000)); assert_eq!(invoice.min_final_cltv_expiry_delta(), MIN_FINAL_CLTV_EXPIRY_DELTA as u64); assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into())); - assert_eq!(invoice.description(), Bolt11InvoiceDescription::Hash(&Sha256(Sha256::hash("Description hash phantom invoice".as_bytes())))); + assert_eq!(invoice.description(), Bolt11InvoiceDescriptionRef::Hash(&Sha256(Sha256::hash("Description hash phantom invoice".as_bytes())))); } #[test] -- 2.39.5