Reject LSPS2 fees above payment amounts

src/event.rs

@@ -707,15 +707,24 @@ where
 		}
 	}
 
-	fn lsps2_max_total_opening_fee_msat(payment_metadata: &[u8], amount_msat: u64) -> Option<u64> {
+	fn lsps2_max_total_opening_fee_msat(
+		payment_metadata: &[u8], gross_amount_msat: u64,
+	) -> Option<u64> {
 		let metadata = PaymentMetadata::read(&mut &payment_metadata[..]).ok()?;
 		let lsps2_parameters = metadata.lsps2_parameters?;
-		lsps2_parameters.max_total_opening_fee_msat.or_else(|| {
-			lsps2_parameters.max_proportional_opening_fee_ppm_msat.and_then(|max_prop_fee| {
-				// If it's a variable amount payment, compute the actual fee.
-				compute_opening_fee(amount_msat, 0, max_prop_fee)
+		lsps2_parameters
+			.max_total_opening_fee_msat
+			.or_else(|| {
+				lsps2_parameters.max_proportional_opening_fee_ppm_msat.and_then(|max_prop_fee| {
+					// If it's a variable amount payment, compute the actual fee.
+					compute_opening_fee(
+						gross_amount_msat,
+						lsps2_parameters.min_opening_fee_msat,
+						max_prop_fee,
+					)
+				})
 			})
-		})
+			.filter(|fee| *fee <= gross_amount_msat)
 	}
 
 	async fn resolve_inbound_payment_id(
@@ -1058,7 +1067,9 @@ where
 							.as_ref()
 							.and_then(|fields| fields.payment_metadata.as_ref())
 							.and_then(|metadata| {
-								Self::lsps2_max_total_opening_fee_msat(metadata, amount_msat)
+								let gross_amount_msat =
+									amount_msat.checked_add(counterparty_skimmed_fee_msat)?;
+								Self::lsps2_max_total_opening_fee_msat(metadata, gross_amount_msat)
 							}),
 						_ => None,
 					};
@@ -2803,16 +2814,67 @@ mod tests {
 			lsps2_parameters: Some(LSPS2Parameters {
 				max_total_opening_fee_msat: Some(42_000),
 				max_proportional_opening_fee_ppm_msat: None,
+				min_opening_fee_msat: 1_000,
 			}),
 		};
 
-		assert_eq!(
-			EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
-				&metadata.encode(),
-				100_000
-			),
-			Some(42_000)
-		);
+		for (gross_amount_msat, expected) in
+			[(41_999, None), (42_000, Some(42_000)), (100_000, Some(42_000))]
+		{
+			assert_eq!(
+				EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
+					&metadata.encode(),
+					gross_amount_msat,
+				),
+				expected,
+				"The total fee limit must not exceed the gross payment amount of {gross_amount_msat} msat"
+			);
+		}
+	}
+
+	#[test]
+	fn lsps2_payment_metadata_honors_minimum_fee() {
+		// Variable-amount metadata with 10,000 ppm (type 2) and a 1,000-msat minimum
+		// (type 3), encoded in the payment metadata TLV format.
+		let metadata =
+			crate::hex_utils::to_vec("1700151402080000000000002710030800000000000003e8").unwrap();
+		for (amount_msat, expected_fee_msat) in
+			[(50_000, 1_000), (100_000, 1_000), (100_001, 1_001), (200_000, 2_000)]
+		{
+			assert_eq!(
+				EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
+					&metadata,
+					amount_msat,
+				),
+				Some(expected_fee_msat),
+				"The fee allowance must include the negotiated minimum and round up proportional fees"
+			);
+		}
+	}
+
+	#[test]
+	fn lsps2_payment_metadata_bounds_variable_fees_by_gross_amount() {
+		for (gross_amount_msat, proportional, expected) in [
+			(100_000, 600_000, Some(60_000)),
+			(100_000, 1_000_000, Some(100_000)),
+			(100_000, 1_000_001, None),
+			(100_000, u64::MAX, None),
+		] {
+			let metadata = PaymentMetadata {
+				lsps2_parameters: Some(LSPS2Parameters {
+					max_total_opening_fee_msat: None,
+					max_proportional_opening_fee_ppm_msat: Some(proportional),
+					min_opening_fee_msat: 0,
+				}),
+			};
+			assert_eq!(
+				EventHandler::<Arc<TestLogger>>::lsps2_max_total_opening_fee_msat(
+					&metadata.encode(),
+					gross_amount_msat,
+				),
+				expected
+			);
+		}
 	}
 
 	#[test]
@@ -2822,6 +2884,7 @@ mod tests {
 			lsps2_parameters: Some(LSPS2Parameters {
 				max_total_opening_fee_msat: None,
 				max_proportional_opening_fee_ppm_msat: None,
+				min_opening_fee_msat: 1_000,
 			}),
 		}
 		.encode();

src/liquidity/client/lsps2.rs

@@ -93,14 +93,14 @@ where
 				Error::LiquidityRequestFailed
 			})?;
 
-		if let Some(max_total_lsp_fee_limit_msat) = max_total_lsp_fee_limit_msat {
-			if min_total_fee_msat > max_total_lsp_fee_limit_msat {
-				log_error!(self.logger,
-					"Failed to request inbound JIT channel as LSP's requested total opening fee of {}msat exceeds our fee limit of {}msat",
-					min_total_fee_msat, max_total_lsp_fee_limit_msat
-				);
-				return Err(Error::LiquidityFeeTooHigh);
-			}
+		let max_total_lsp_fee_limit_msat =
+			max_total_lsp_fee_limit_msat.unwrap_or(amount_msat).min(amount_msat);
+		if min_total_fee_msat > max_total_lsp_fee_limit_msat {
+			log_error!(self.logger,
+				"Failed to request inbound JIT channel as LSP's requested total opening fee of {}msat exceeds our fee limit of {}msat",
+				min_total_fee_msat, max_total_lsp_fee_limit_msat
+			);
+			return Err(Error::LiquidityFeeTooHigh);
 		}
 
 		log_debug!(
@@ -110,18 +110,18 @@ where
 			min_total_fee_msat
 		);
 
+		let lsps2_parameters = LSPS2Parameters {
+			max_total_opening_fee_msat: Some(min_total_fee_msat),
+			max_proportional_opening_fee_ppm_msat: None,
+			min_opening_fee_msat: min_opening_params.min_fee_msat,
+		};
 		let buy_response = self
 			.lsps2_send_buy_request(
 				Some(amount_msat),
 				min_opening_params,
 				Some(&cheapest_lsp.node_id),
 			)
 			.await?;
-		let lsps2_parameters = LSPS2Parameters {
-			max_total_opening_fee_msat: Some(min_total_fee_msat),
-			max_proportional_opening_fee_ppm_msat: None,
-		};
-
 		let invoice = self.lsps2_create_jit_invoice(
 			buy_response,
 			Some(amount_msat),
@@ -178,13 +178,14 @@ where
 			min_prop_fee_ppm_msat
 		);
 
-		let buy_response = self
-			.lsps2_send_buy_request(None, min_opening_params, Some(&cheapest_lsp.node_id))
-			.await?;
 		let lsps2_parameters = LSPS2Parameters {
 			max_total_opening_fee_msat: None,
 			max_proportional_opening_fee_ppm_msat: Some(min_prop_fee_ppm_msat),
+			min_opening_fee_msat: min_opening_params.min_fee_msat,
 		};
+		let buy_response = self
+			.lsps2_send_buy_request(None, min_opening_params, Some(&cheapest_lsp.node_id))
+			.await?;
 		let invoice = self.lsps2_create_jit_invoice(
 			buy_response,
 			None,

src/payment/bolt11.rs

@@ -188,6 +188,7 @@ mod tests {
 		let lsps2_parameters = LSPS2Parameters {
 			max_total_opening_fee_msat: Some(42_000),
 			max_proportional_opening_fee_ppm_msat: Some(17_000),
+			min_opening_fee_msat: 1_000,
 		};
 		let metadata = PaymentMetadata { lsps2_parameters: Some(lsps2_parameters) };
 
@@ -672,8 +673,8 @@ impl Bolt11Payment {
 	/// When the returned invoice is paid, the configured [LSPS2]-compliant LSP will open a channel
 	/// to us, supplying just-in-time inbound liquidity.
 	///
-	/// If set, `max_total_lsp_fee_limit_msat` will limit how much fee we allow the LSP to take for opening the
-	/// channel to us. We'll use its cheapest offer otherwise.
+	/// We choose the cheapest offer and return [`Error::LiquidityFeeTooHigh`] if its opening fee
+	/// exceeds `amount_msat` or the optional `max_total_lsp_fee_limit_msat`.
 	///
 	/// [LSPS2]: https://github.com/BitcoinAndLightningLayerSpecs/lsp/blob/main/LSPS2/README.md
 	pub fn receive_via_jit_channel(
@@ -701,8 +702,8 @@ impl Bolt11Payment {
 	/// When the returned invoice is paid, the configured [LSPS2]-compliant LSP will open a channel
 	/// to us, supplying just-in-time inbound liquidity.
 	///
-	/// If set, `max_total_lsp_fee_limit_msat` will limit how much fee we allow the LSP to take for opening the
-	/// channel to us. We'll use its cheapest offer otherwise.
+	/// We choose the cheapest offer and return [`Error::LiquidityFeeTooHigh`] if its opening fee
+	/// exceeds `amount_msat` or the optional `max_total_lsp_fee_limit_msat`.
 	///
 	/// The inbound payment will only be accepted if
 	/// [`Config::manually_handle_unknown_bolt11_payments`] is enabled. We will then emit a

src/payment/store.rs

@@ -689,11 +689,15 @@ pub struct LSPS2Parameters {
 	/// The maximal proportional fee, in parts-per-million millisatoshi, we allow any configured
 	/// LSP withhold from us when forwarding the payment.
 	pub max_proportional_opening_fee_ppm_msat: Option<u64>,
+	/// The minimum opening fee, in millisatoshi, from the selected LSP offer. For variable-amount
+	/// payments, the allowed fee is the greater of this minimum and the proportional fee.
+	pub min_opening_fee_msat: u64,
 }
 
 impl_writeable_tlv_based!(LSPS2Parameters, {
 	(0, max_total_opening_fee_msat, option),
 	(2, max_proportional_opening_fee_ppm_msat, option),
+	(3, min_opening_fee_msat, (default_value, 0)),
 });
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -996,13 +1000,24 @@ mod tests {
 		}
 	}
 
+	#[derive(Clone, Debug, PartialEq, Eq)]
+	struct LegacyLSPS2Parameters {
+		max_total_opening_fee_msat: Option<u64>,
+		max_proportional_opening_fee_ppm_msat: Option<u64>,
+	}
+
+	impl_writeable_tlv_based!(LegacyLSPS2Parameters, {
+		(0, max_total_opening_fee_msat, option),
+		(2, max_proportional_opening_fee_ppm_msat, option),
+	});
+
 	#[derive(Clone, Debug, PartialEq, Eq)]
 	struct LegacyBolt11JitKind {
 		hash: PaymentHash,
 		counterparty_skimmed_fee_msat: Option<u64>,
 		preimage: Option<PaymentPreimage>,
 		secret: Option<PaymentSecret>,
-		lsp_fee_limits: LSPS2Parameters,
+		lsp_fee_limits: LegacyLSPS2Parameters,
 	}
 
 	impl_writeable_tlv_based!(LegacyBolt11JitKind, {
@@ -1019,7 +1034,7 @@ mod tests {
 		let preimage = Some(PaymentPreimage([43u8; 32]));
 		let secret = Some(PaymentSecret([44u8; 32]));
 		let counterparty_skimmed_fee_msat = Some(7_777u64);
-		let lsp_fee_limits = LSPS2Parameters {
+		let lsp_fee_limits = LegacyLSPS2Parameters {
 			max_total_opening_fee_msat: Some(46_000),
 			max_proportional_opening_fee_ppm_msat: Some(47_000),
 		};

tests/integration_tests_rust.rs

@@ -5291,7 +5291,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 		max_payment_size_msat: 1_000_000_000,
 		min_payment_size_msat: 0,
 		min_channel_lifetime: 100,
-		min_channel_opening_fee_msat: 0,
+		min_channel_opening_fee_msat: 1_000,
 		max_client_to_self_delay: 1024,
 		client_trusts_lsp,
 		disable_client_reserve: false,
@@ -5332,6 +5332,26 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 	let client_node = client_builder.build(client_config.node_entropy.into()).unwrap();
 	client_node.start().unwrap();
 
+	let fee_test_description =
+		Bolt11InvoiceDescription::Direct(Description::new(String::from("fee limit")).unwrap())
+			.into();
+	// Distinguish client fee rejection from the LSP's LiquidityRequestFailed.
+	for (amount, fee_limit, expected) in [
+		(999, None, Err(NodeError::LiquidityFeeTooHigh)),
+		(999, Some(2_000), Err(NodeError::LiquidityFeeTooHigh)),
+		(1_001, Some(999), Err(NodeError::LiquidityFeeTooHigh)),
+		(1_000, None, Err(NodeError::LiquidityRequestFailed)),
+		(1_001, None, Ok(())),
+	] {
+		assert_eq!(
+			client_node
+				.bolt11_payment()
+				.receive_via_jit_channel(amount, &fee_test_description, 1024, fee_limit)
+				.map(|_| ()),
+			expected
+		);
+	}
+
 	let payer_config = random_config();
 	setup_builder!(payer_builder, payer_config.node_config);
 	payer_builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config));
@@ -5569,6 +5589,25 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 		client_node.payment(&client_payment_id).unwrap().unwrap().status,
 		PaymentStatus::Failed
 	);
+
+	// Variable-amount fees must be computed from the gross payment.
+	let variable_invoice = client_node
+		.bolt11_payment()
+		.receive_variable_amount_via_jit_channel(&invoice_description, 1024, None)
+		.unwrap();
+	let variable_amount_msat = 20_000_000;
+	let variable_fee_msat = (variable_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000;
+	let variable_payment_id = payer_node
+		.bolt11_payment()
+		.send_using_amount(&variable_invoice, variable_amount_msat, None)
+		.unwrap();
+	expect_channel_pending_event!(service_node, client_node.node_id());
+	expect_channel_ready_event!(service_node, client_node.node_id());
+	expect_channel_pending_event!(client_node, service_node.node_id());
+	expect_channel_ready_event!(client_node, service_node.node_id());
+	expect_payment_received_event!(client_node, variable_amount_msat - variable_fee_msat);
+	expect_event!(service_node, PaymentForwarded);
+	expect_payment_successful_event!(payer_node, variable_payment_id, None);
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]