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.

Author: jkczyz

lightning-invoice/src/lib.rs

@@ -233,25 +233,45 @@ 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),
 
 	/// Reference to the description's hash included in the invoice
 	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<H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool, M: tb::Bool> InvoiceBui
 	pub fn invoice_description(self, description: Bolt11InvoiceDescription) -> InvoiceBuilder<tb::True, H, T, C, S, M> {
 		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]));

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::<ES, NS, L>(
 		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::<ES, NS, L>(
-		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]