review comments [squash]

Author: joostjager

bindings/ldk_node.udl

@@ -134,9 +134,9 @@ interface Bolt11Payment {
 	[Throws=NodeError]
 	Bolt11Invoice receive_variable_amount_for_hash([ByRef]Bolt11InvoiceStringDescription description, u32 expiry_secs, PaymentHash payment_hash);
 	[Throws=NodeError]
-	Bolt11Invoice receive_via_jit_channel(u64 amount_msat, [ByRef]string description, u32 expiry_secs, u64? max_lsp_fee_limit_msat);
+	Bolt11Invoice receive_via_jit_channel(u64 amount_msat, [ByRef]Bolt11InvoiceStringDescription description, u32 expiry_secs, u64? max_lsp_fee_limit_msat);
 	[Throws=NodeError]
-	Bolt11Invoice receive_variable_amount_via_jit_channel([ByRef]string description, u32 expiry_secs, u64? max_proportional_lsp_fee_limit_ppm_msat);
+	Bolt11Invoice receive_variable_amount_via_jit_channel([ByRef]Bolt11InvoiceStringDescription description, u32 expiry_secs, u64? max_proportional_lsp_fee_limit_ppm_msat);
 };
 
 interface Bolt12Payment {

src/liquidity.rs

@@ -12,7 +12,7 @@ use crate::{Config, Error};
 use lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA;
 use lightning::ln::msgs::SocketAddress;
 use lightning::routing::router::{RouteHint, RouteHintHop};
-use lightning_invoice::{Bolt11Invoice, InvoiceBuilder, RoutingFees};
+use lightning_invoice::{Bolt11Invoice, Bolt11InvoiceDescription, InvoiceBuilder, RoutingFees};
 use lightning_liquidity::events::Event;
 use lightning_liquidity::lsps0::ser::RequestId;
 use lightning_liquidity::lsps2::event::LSPS2ClientEvent;
@@ -196,7 +196,7 @@ where
 	}
 
 	pub(crate) async fn lsps2_receive_to_jit_channel(
-		&self, amount_msat: u64, description: &str, expiry_secs: u32,
+		&self, amount_msat: u64, description: &Bolt11InvoiceDescription, expiry_secs: u32,
 		max_total_lsp_fee_limit_msat: Option<u64>,
 	) -> Result<(Bolt11Invoice, u64), Error> {
 		let fee_response = self.lsps2_request_opening_fee_params().await?;
@@ -256,7 +256,7 @@ where
 	}
 
 	pub(crate) async fn lsps2_receive_variable_amount_to_jit_channel(
-		&self, description: &str, expiry_secs: u32,
+		&self, description: &Bolt11InvoiceDescription, expiry_secs: u32,
 		max_proportional_lsp_fee_limit_ppm_msat: Option<u64>,
 	) -> Result<(Bolt11Invoice, u64), Error> {
 		let fee_response = self.lsps2_request_opening_fee_params().await?;
@@ -373,8 +373,8 @@ where
 	}
 
 	fn lsps2_create_jit_invoice(
-		&self, buy_response: LSPS2BuyResponse, amount_msat: Option<u64>, description: &str,
-		expiry_secs: u32,
+		&self, buy_response: LSPS2BuyResponse, amount_msat: Option<u64>,
+		description: &Bolt11InvoiceDescription, expiry_secs: u32,
 	) -> Result<Bolt11Invoice, Error> {
 		let lsps2_service = self.lsps2_service.as_ref().ok_or(Error::LiquiditySourceUnavailable)?;
 
@@ -404,7 +404,7 @@ where
 
 		let currency = self.config.network.into();
 		let mut invoice_builder = InvoiceBuilder::new(currency)
-			.description(description.to_string())
+			.invoice_description(description.clone())
 			.payment_hash(payment_hash)
 			.payment_secret(payment_secret)
 			.current_timestamp()

src/payment/bolt11.rs

@@ -30,13 +30,13 @@ use lightning::routing::router::{PaymentParameters, RouteParameters};
 
 use lightning_types::payment::{PaymentHash, PaymentPreimage};
 
-use lightning_invoice::{Bolt11Invoice, Bolt11InvoiceDescription, Description };
+use lightning_invoice::{Bolt11Invoice, Bolt11InvoiceDescription, Description};
 
-use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hashes::Hash;
 
-use std::sync::{Arc, RwLock};
 use std::str::FromStr;
+use std::sync::{Arc, RwLock};
 
 /// A payment handler allowing to create and pay [BOLT 11] invoices.
 ///
@@ -418,7 +418,6 @@ impl Bolt11Payment {
 		let invoice_description: Bolt11InvoiceDescription = description.try_into()?;
 		self.receive_inner(Some(amount_msat), &invoice_description, expiry_secs, None)
 	}
-	
 
 	/// Returns a payable invoice that can be used to request a payment of the amount
 	/// given for the given payment hash.
@@ -436,14 +435,16 @@ impl Bolt11Payment {
 	/// [`fail_for_hash`]: Self::fail_for_hash
 	#[cfg(not(feature = "uniffi"))]
 	pub fn receive_for_hash(
-		&self, amount_msat: u64, description: &Bolt11InvoiceDescription, expiry_secs: u32, payment_hash: PaymentHash,
+		&self, amount_msat: u64, description: &Bolt11InvoiceDescription, expiry_secs: u32,
+		payment_hash: PaymentHash,
 	) -> Result<Bolt11Invoice, Error> {
 		self.receive_inner(Some(amount_msat), description, expiry_secs, Some(payment_hash))
 	}
-	
+
 	#[cfg(feature = "uniffi")]
 	pub fn receive_for_hash(
-		&self, amount_msat: u64, description: &Bolt11InvoiceStringDescription, expiry_secs: u32, payment_hash: PaymentHash,
+		&self, amount_msat: u64, description: &Bolt11InvoiceStringDescription, expiry_secs: u32,
+		payment_hash: PaymentHash,
 	) -> Result<Bolt11Invoice, Error> {
 		let invoice_description: Bolt11InvoiceDescription = description.try_into()?;
 		self.receive_inner(Some(amount_msat), &invoice_description, expiry_secs, Some(payment_hash))
@@ -491,15 +492,16 @@ impl Bolt11Payment {
 
 	#[cfg(feature = "uniffi")]
 	pub fn receive_variable_amount_for_hash(
-		&self, description: &Bolt11InvoiceStringDescription, expiry_secs: u32, payment_hash: PaymentHash,
+		&self, description: &Bolt11InvoiceStringDescription, expiry_secs: u32,
+		payment_hash: PaymentHash,
 	) -> Result<Bolt11Invoice, Error> {
 		let invoice_description: Bolt11InvoiceDescription = description.try_into()?;
 		self.receive_inner(None, &invoice_description, expiry_secs, Some(payment_hash))
 	}
 
 	pub(crate) fn receive_inner(
-		&self, amount_msat: Option<u64>, invoice_description: &Bolt11InvoiceDescription, expiry_secs: u32,
-		manual_claim_payment_hash: Option<PaymentHash>,
+		&self, amount_msat: Option<u64>, invoice_description: &Bolt11InvoiceDescription,
+		expiry_secs: u32, manual_claim_payment_hash: Option<PaymentHash>,
 	) -> Result<Bolt11Invoice, Error> {
 		let invoice = {
 			let invoice_params = Bolt11InvoiceParameters {
@@ -564,8 +566,9 @@ impl Bolt11Payment {
 	/// channel to us. We'll use its cheapest offer otherwise.
 	///
 	/// [LSPS2]: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/main/LSPS2/README.md
+	#[cfg(not(feature = "uniffi"))]
 	pub fn receive_via_jit_channel(
-		&self, amount_msat: u64, description: &str, expiry_secs: u32,
+		&self, amount_msat: u64, description: &Bolt11InvoiceDescription, expiry_secs: u32,
 		max_total_lsp_fee_limit_msat: Option<u64>,
 	) -> Result<Bolt11Invoice, Error> {
 		self.receive_via_jit_channel_inner(
@@ -577,6 +580,21 @@ impl Bolt11Payment {
 		)
 	}
 
+	#[cfg(feature = "uniffi")]
+	pub fn receive_via_jit_channel(
+		&self, amount_msat: u64, description: &Bolt11InvoiceStringDescription, expiry_secs: u32,
+		max_total_lsp_fee_limit_msat: Option<u64>,
+	) -> Result<Bolt11Invoice, Error> {
+		let invoice_description: Bolt11InvoiceDescription = description.try_into()?;
+		self.receive_via_jit_channel_inner(
+			Some(amount_msat),
+			&invoice_description,
+			expiry_secs,
+			max_total_lsp_fee_limit_msat,
+			None,
+		)
+	}
+
 	/// Returns a payable invoice that can be used to request a variable amount payment (also known
 	/// as "zero-amount" invoice) and receive it via a newly created just-in-time (JIT) channel.
 	///
@@ -588,8 +606,9 @@ impl Bolt11Payment {
 	/// We'll use its cheapest offer otherwise.
 	///
 	/// [LSPS2]: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/main/LSPS2/README.md
+	#[cfg(not(feature = "uniffi"))]
 	pub fn receive_variable_amount_via_jit_channel(
-		&self, description: &str, expiry_secs: u32,
+		&self, description: &Bolt11InvoiceDescription, expiry_secs: u32,
 		max_proportional_lsp_fee_limit_ppm_msat: Option<u64>,
 	) -> Result<Bolt11Invoice, Error> {
 		self.receive_via_jit_channel_inner(
@@ -601,8 +620,23 @@ impl Bolt11Payment {
 		)
 	}
 
+	#[cfg(feature = "uniffi")]
+	pub fn receive_variable_amount_via_jit_channel(
+		&self, description: &Bolt11InvoiceStringDescription, expiry_secs: u32,
+		max_proportional_lsp_fee_limit_ppm_msat: Option<u64>,
+	) -> Result<Bolt11Invoice, Error> {
+		let invoice_description: Bolt11InvoiceDescription = description.try_into()?;
+		self.receive_via_jit_channel_inner(
+			None,
+			&invoice_description,
+			expiry_secs,
+			None,
+			max_proportional_lsp_fee_limit_ppm_msat,
+		)
+	}
+
 	fn receive_via_jit_channel_inner(
-		&self, amount_msat: Option<u64>, description: &str, expiry_secs: u32,
+		&self, amount_msat: Option<u64>, description: &Bolt11InvoiceDescription, expiry_secs: u32,
 		max_total_lsp_fee_limit_msat: Option<u64>,
 		max_proportional_lsp_fee_limit_ppm_msat: Option<u64>,
 	) -> Result<Bolt11Invoice, Error> {
@@ -777,30 +811,34 @@ impl Bolt11Payment {
 
 /// 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.
-pub enum Bolt11InvoiceStringDescription {  // use same name (no string)
+pub enum Bolt11InvoiceStringDescription {
+	// use same name (no string)
 	/// Contains a full description.
-	Direct{
+	Direct {
 		/// Description of what the invoice is for
-		description: String
+		description: String,
 	},
 	/// Contains a hash.
-	Hash{
+	Hash {
 		/// Hash of the description of what the invoice is for
-		hash: String
-	}
+		hash: String,
+	},
 }
 
 impl TryInto<Bolt11InvoiceDescription> for &Bolt11InvoiceStringDescription {
 	type Error = Error;
 
 	fn try_into(self) -> Result<Bolt11InvoiceDescription, Self::Error> {
 		match self {
-			Bolt11InvoiceStringDescription::Direct{description} => {
-				Description::new(description.clone()).map(Bolt11InvoiceDescription::Direct).map_err(|_| Error::InvalidInvoice)
-			},
-			Bolt11InvoiceStringDescription::Hash{hash} => {
-				Sha256::from_str(&hash).map(lightning_invoice::Sha256).map(Bolt11InvoiceDescription::Hash).map_err(|_| Error::InvalidInvoice)
+			Bolt11InvoiceStringDescription::Direct { description } => {
+				Description::new(description.clone())
+					.map(Bolt11InvoiceDescription::Direct)
+					.map_err(|_| Error::InvalidInvoice)
 			},
+			Bolt11InvoiceStringDescription::Hash { hash } => Sha256::from_str(&hash)
+				.map(lightning_invoice::Sha256)
+				.map(Bolt11InvoiceDescription::Hash)
+				.map_err(|_| Error::InvalidInvoice),
 		}
 	}
-}
+}

src/payment/mod.rs

@@ -14,13 +14,13 @@ mod spontaneous;
 pub(crate) mod store;
 mod unified_qr;
 
+pub use bolt11::Bolt11InvoiceStringDescription;
 pub use bolt11::Bolt11Payment;
 pub use bolt12::Bolt12Payment;
 pub use onchain::OnchainPayment;
 pub use spontaneous::SpontaneousPayment;
 pub use store::{LSPFeeLimits, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus};
 pub use unified_qr::{QrPaymentResult, UnifiedQrPayment};
-pub use bolt11::Bolt11InvoiceStringDescription;
 
 /// Represents information used to send a payment.
 #[derive(Clone, Debug, PartialEq)]

src/payment/unified_qr.rs

@@ -102,14 +102,18 @@ impl UnifiedQrPayment {
 		let invoice_description = Bolt11InvoiceDescription::Direct(
 			Description::new(description.to_string()).map_err(|_| Error::InvoiceCreationFailed)?,
 		);
-		let bolt11_invoice =
-			match self.bolt11_invoice.receive_inner(Some(amount_msats), &invoice_description, expiry_sec, None) {
-				Ok(invoice) => Some(invoice),
-				Err(e) => {
-					log_error!(self.logger, "Failed to create invoice {}", e);
-					return Err(Error::InvoiceCreationFailed);
-				},
-			};
+		let bolt11_invoice = match self.bolt11_invoice.receive_inner(
+			Some(amount_msats),
+			&invoice_description,
+			expiry_sec,
+			None,
+		) {
+			Ok(invoice) => Some(invoice),
+			Err(e) => {
+				log_error!(self.logger, "Failed to create invoice {}", e);
+				return Err(Error::InvoiceCreationFailed);
+			},
+		};
 
 		let extras = Extras { bolt11_invoice, bolt12_offer };
 

tests/common/mod.rs

@@ -553,8 +553,10 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>(
 
 	println!("\nB receive");
 	let invoice_amount_1_msat = 2500_000;
-	let invoice_description: Bolt11InvoiceDescription = Bolt11InvoiceDescription::Direct(Description::new(String::from("asdf")).unwrap());
-	let invoice = node_b.bolt11_payment().receive(invoice_amount_1_msat, &invoice_description, 9217).unwrap();
+	let invoice_description: Bolt11InvoiceDescription =
+		Bolt11InvoiceDescription::Direct(Description::new(String::from("asdf")).unwrap());
+	let invoice =
+		node_b.bolt11_payment().receive(invoice_amount_1_msat, &invoice_description, 9217).unwrap();
 
 	println!("\nA send");
 	let payment_id = node_a.bolt11_payment().send(&invoice, None).unwrap();
@@ -600,7 +602,8 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>(
 
 	// Test under-/overpayment
 	let invoice_amount_2_msat = 2500_000;
-	let invoice = node_b.bolt11_payment().receive(invoice_amount_2_msat, &invoice_description, 9217).unwrap();
+	let invoice =
+		node_b.bolt11_payment().receive(invoice_amount_2_msat, &invoice_description, 9217).unwrap();
 
 	let underpaid_amount = invoice_amount_2_msat - 1;
 	assert_eq!(
@@ -609,7 +612,8 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>(
 	);
 
 	println!("\nB overpaid receive");
-	let invoice = node_b.bolt11_payment().receive(invoice_amount_2_msat, &invoice_description, 9217).unwrap();
+	let invoice =
+		node_b.bolt11_payment().receive(invoice_amount_2_msat, &invoice_description, 9217).unwrap();
 	let overpaid_amount_msat = invoice_amount_2_msat + 100;
 
 	println!("\nA overpaid send");
@@ -716,7 +720,12 @@ pub(crate) fn do_channel_full_cycle<E: ElectrumApi>(
 		PaymentHash(Sha256::hash(&manual_fail_preimage.0).to_byte_array());
 	let manual_fail_invoice = node_b
 		.bolt11_payment()
-		.receive_for_hash(invoice_amount_3_msat, &invoice_description, 9217, manual_fail_payment_hash)
+		.receive_for_hash(
+			invoice_amount_3_msat,
+			&invoice_description,
+			9217,
+			manual_fail_payment_hash,
+		)
 		.unwrap();
 	let manual_fail_payment_id = node_a.bolt11_payment().send(&manual_fail_invoice, None).unwrap();
 

tests/integration_tests_rust.rs

@@ -190,7 +190,8 @@ fn multi_hop_sending() {
 		max_channel_saturation_power_of_half: Some(2),
 	};
 
-	let invoice_description = Bolt11InvoiceDescription::Direct(Description::new(String::from("asdf")).unwrap());
+	let invoice_description =
+		Bolt11InvoiceDescription::Direct(Description::new(String::from("asdf")).unwrap());
 	let invoice = nodes[4].bolt11_payment().receive(2_500_000, &invoice_description, 9217).unwrap();
 	nodes[0].bolt11_payment().send(&invoice, Some(sending_params)).unwrap();