lsps2: Match variable leases to payments

Use resolved BOLT12 amounts to reject variable leases outside the
payment range or total fee policy. Keep BOLT11 selection amountless
until payment, while recording the exact BOLT12 fee limit.

Co-Authored-By: HAL 9000

Author: tnull

src/liquidity/client/lsps2/mod.rs

@@ -225,18 +225,17 @@ where
 					},
 				)
 			},
-			JitInvoiceRequest::Variable { .. } => {
-				let (lease, proportional_fee, _, _) =
-					self.acquire_variable_lease(&connection_manager).await?;
+			JitInvoiceRequest::Variable { amount_msat, .. } => {
+				// A BOLT12 invoice request has already resolved the payment amount. Use it to avoid
+				// consuming a cached variable lease outside its advertised range, and record the exact
+				// fee for that amount so payment validation need not accept the broader node-wide cap.
+				let (lease, total_fee_msat, _, _) =
+					self.acquire_variable_lease(Some(amount_msat), &connection_manager).await?;
 				(
 					lease,
 					LSPS2Parameters {
-						max_total_opening_fee_msat: self.config.lsps2_max_total_lsp_fee_limit_msat,
-						max_proportional_opening_fee_ppm_msat: self
-							.config
-							.lsps2_max_total_lsp_fee_limit_msat
-							.is_none()
-							.then_some(proportional_fee),
+						max_total_opening_fee_msat: Some(total_fee_msat),
+						max_proportional_opening_fee_ppm_msat: None,
 					},
 				)
 			},
@@ -300,7 +299,7 @@ where
 		payment_hash: Option<PaymentHash>, connection_manager: Arc<ConnectionManager<L>>,
 	) -> Result<(Bolt11Invoice, LspConfig), Error> {
 		let (lease, proportional_fee, lsp, was_negotiated) =
-			self.acquire_variable_lease(&connection_manager).await?;
+			self.acquire_variable_lease(None, &connection_manager).await?;
 		let lsps2_parameters = LSPS2Parameters {
 			max_total_opening_fee_msat: self.config.lsps2_max_total_lsp_fee_limit_msat,
 			max_proportional_opening_fee_ppm_msat: self
@@ -459,36 +458,36 @@ where
 	}
 
 	async fn acquire_variable_lease(
-		self: &Arc<Self>, connection_manager: &Arc<ConnectionManager<L>>,
+		self: &Arc<Self>, amount_msat: Option<u64>, connection_manager: &Arc<ConnectionManager<L>>,
 	) -> Result<(PaymentLease, u64, LspConfig, bool), Error> {
-		if let Some((lease, proportional_fee, lsp)) = self.take_cached_variable_lease().await? {
+		if let Some((lease, fee, lsp)) = self.take_cached_variable_lease(amount_msat).await? {
 			self.schedule_variable_lease_refill(connection_manager);
-			return Ok((lease, proportional_fee, lsp, false));
+			return Ok((lease, fee, lsp, false));
 		}
 		let request_lock = self
 			.pending_lease_request_state
 			.lock()
 			.expect("lock")
 			.request_lock(LeaseRequestKey::Variable);
 		let _request_guard = request_lock.lock().await;
-		if let Some((lease, proportional_fee, lsp)) = self.take_cached_variable_lease().await? {
+		if let Some((lease, fee, lsp)) = self.take_cached_variable_lease(amount_msat).await? {
 			self.schedule_variable_lease_refill(connection_manager);
-			return Ok((lease, proportional_fee, lsp, false));
+			return Ok((lease, fee, lsp, false));
 		}
 
-		let (negotiated_lease, min_prop_fee_ppm_msat, cheapest_lsp) =
-			self.negotiate_variable_lease(connection_manager).await?;
+		let (negotiated_lease, fee, cheapest_lsp) =
+			self.negotiate_variable_lease(amount_msat, connection_manager).await?;
 		let lease = self.consume_lease(&negotiated_lease.id).await?;
 		self.schedule_variable_lease_refill(connection_manager);
-		Ok((lease, min_prop_fee_ppm_msat, cheapest_lsp, true))
+		Ok((lease, fee, cheapest_lsp, true))
 	}
 
 	async fn negotiate_variable_lease(
-		self: &Arc<Self>, connection_manager: &Arc<ConnectionManager<L>>,
+		self: &Arc<Self>, amount_msat: Option<u64>, connection_manager: &Arc<ConnectionManager<L>>,
 	) -> Result<(PaymentLease, u64, LspConfig), Error> {
 		let mut attempt = 1;
 		loop {
-			let result = self.negotiate_variable_lease_once(connection_manager).await;
+			let result = self.negotiate_variable_lease_once(amount_msat, connection_manager).await;
 			match result {
 				Err(error) if should_retry_lease_negotiation(error, attempt) => {
 					log_warn!(
@@ -505,7 +504,7 @@ where
 	}
 
 	async fn negotiate_variable_lease_once(
-		self: &Arc<Self>, connection_manager: &Arc<ConnectionManager<L>>,
+		self: &Arc<Self>, amount_msat: Option<u64>, connection_manager: &Arc<ConnectionManager<L>>,
 	) -> Result<(PaymentLease, u64, LspConfig), Error> {
 		let all_offers = self.gather_lsps2_offers(connection_manager).await?;
 		let mut rejected_for_fee = false;
@@ -514,20 +513,34 @@ where
 			.flat_map(|(lsp, resp)| {
 				resp.opening_fee_params_menu.into_iter().map(move |params| (lsp.clone(), params))
 			})
-			.map(|(lsp, params)| {
-				let ppm = params.proportional as u64;
-				(lsp, ppm, params)
-			})
-			.filter(|(_, _, params)| {
-				let allowed = self
+			.filter_map(|(lsp, params)| {
+				// BOLT12 supplies a resolved amount here, while a BOLT11 zero-amount invoice does not.
+				// In the former case, only negotiate parameters that can carry that exact payment and
+				// compare providers by the total fee the payment would actually incur.
+				let selection_fee = if let Some(amount_msat) = amount_msat {
+					if amount_msat < params.min_payment_size_msat
+						|| amount_msat > params.max_payment_size_msat
+					{
+						return None;
+					}
+					compute_opening_fee(
+						amount_msat,
+						params.min_fee_msat,
+						params.proportional as u64,
+					)?
+				} else {
+					params.proportional as u64
+				};
+				let fee_for_limit = amount_msat.map_or(params.min_fee_msat, |_| selection_fee);
+				let fee_allowed = self
 					.config
 					.lsps2_max_total_lsp_fee_limit_msat
-					.map_or(true, |limit| params.min_fee_msat <= limit);
-				rejected_for_fee |= !allowed;
-				allowed
+					.map_or(true, |limit| fee_for_limit <= limit);
+				rejected_for_fee |= !fee_allowed;
+				fee_allowed.then_some((lsp, selection_fee, params))
 			})
 			.collect::<Vec<_>>();
-		candidates.sort_unstable_by_key(|(_, ppm, _)| *ppm);
+		candidates.sort_unstable_by_key(|(_, fee, _)| *fee);
 		if candidates.is_empty() {
 			return Err(if rejected_for_fee {
 				log_error!(
@@ -544,20 +557,29 @@ where
 		try_lease_candidates(
 			candidates,
 			|(lsp, _, _)| lsp.node_id,
-			|(lsp, proportional_fee_ppm_msat, opening_params)| {
+			|(lsp, fee, opening_params)| {
 				let client = Arc::clone(self);
 				async move {
-					log_debug!(
-						client.logger,
-						"Choosing liquidity offer from LSP {}, will pay {}ppm msat in proportional LSP fees",
-						lsp.node_id,
-						proportional_fee_ppm_msat
-					);
+					if amount_msat.is_some() {
+						log_debug!(
+							client.logger,
+							"Choosing liquidity offer from LSP {}, will pay {}msat in total LSP fees",
+							lsp.node_id,
+							fee
+						);
+					} else {
+						log_debug!(
+							client.logger,
+							"Choosing liquidity offer from LSP {}, will pay {}ppm msat in proportional LSP fees",
+							lsp.node_id,
+							fee
+						);
+					}
 					match client
 						.lsps2_send_buy_request(None, opening_params, Some(&lsp.node_id))
 						.await
 					{
-						Ok(lease) => Ok((lease, proportional_fee_ppm_msat, lsp)),
+						Ok(lease) => Ok((lease, fee, lsp)),
 						Err(error) => {
 							log_warn!(
 								client.logger,
@@ -632,11 +654,11 @@ where
 			.lease_state
 			.lock()
 			.expect("lock")
-			.has_variable_amount(self.config.lsps2_max_total_lsp_fee_limit_msat)
+			.has_variable_amount(None, self.config.lsps2_max_total_lsp_fee_limit_msat)
 		{
 			return Ok(());
 		}
-		self.negotiate_variable_lease(connection_manager).await?;
+		self.negotiate_variable_lease(None, connection_manager).await?;
 		Ok(())
 	}
 
@@ -833,22 +855,22 @@ where
 	}
 
 	async fn take_cached_variable_lease(
-		&self,
+		&self, amount_msat: Option<u64>,
 	) -> Result<Option<(PaymentLease, u64, LspConfig)>, Error> {
 		loop {
-			let Some((lease, proportional_fee)) = self
+			let Some((lease, fee)) = self
 				.lease_state
 				.lock()
 				.expect("lock")
-				.variable_amount(self.config.lsps2_max_total_lsp_fee_limit_msat)
+				.variable_amount(amount_msat, self.config.lsps2_max_total_lsp_fee_limit_msat)
 			else {
 				return Ok(None);
 			};
 			let lease = self.consume_selected_lease(lease).await?;
 			if let Some(lsp) =
 				select_lsps_for_protocol(&self.lsp_nodes, 2, Some(&lease.id.lsp_node_id))
 			{
-				return Ok(Some((lease, proportional_fee, lsp)));
+				return Ok(Some((lease, fee, lsp)));
 			}
 		}
 	}

src/liquidity/client/lsps2/state.rs

@@ -347,27 +347,42 @@ impl LSPS2LeaseState {
 	}
 
 	pub(crate) fn variable_amount(
-		&self, max_total_fee_msat: Option<u64>,
+		&self, amount_msat: Option<u64>, max_total_fee_msat: Option<u64>,
 	) -> Option<(PaymentLease, u64)> {
-		let (id, proportional_fee) = self
+		let (id, selection_fee) = self
 			.leases
 			.iter()
 			.filter(|(_, lease)| lease.payment_size_msat.is_none())
 			.filter(|(_, lease)| is_lease_usable(lease))
-			.filter(|(_, lease)| {
-				max_total_fee_msat.map_or(true, |max| lease.params.min_fee_msat <= max)
+			.filter_map(|(id, lease)| {
+				let selection_fee = if let Some(amount_msat) = amount_msat {
+					if amount_msat < lease.params.min_payment_size_msat
+						|| amount_msat > lease.params.max_payment_size_msat
+					{
+						return None;
+					}
+					compute_opening_fee(
+						amount_msat,
+						lease.params.min_fee_msat,
+						lease.params.proportional as u64,
+					)?
+				} else {
+					lease.params.proportional as u64
+				};
+				let fee_for_limit =
+					amount_msat.map_or(lease.params.min_fee_msat, |_| selection_fee);
+				max_total_fee_msat
+					.map_or(true, |max| fee_for_limit <= max)
+					.then_some((*id, selection_fee))
 			})
-			.map(|(id, lease)| (*id, lease.params.proportional as u64))
 			.min_by_key(|(_, fee)| *fee)?;
-		self.leases.get(&id).cloned().map(|lease| (lease, proportional_fee))
+		self.leases.get(&id).cloned().map(|lease| (lease, selection_fee))
 	}
 
-	pub(crate) fn has_variable_amount(&self, max_total_fee_msat: Option<u64>) -> bool {
-		self.leases
-			.values()
-			.filter(|lease| lease.payment_size_msat.is_none())
-			.filter(|lease| is_lease_usable(lease))
-			.any(|lease| max_total_fee_msat.map_or(true, |max| lease.params.min_fee_msat <= max))
+	pub(crate) fn has_variable_amount(
+		&self, amount_msat: Option<u64>, max_total_fee_msat: Option<u64>,
+	) -> bool {
+		self.variable_amount(amount_msat, max_total_fee_msat).is_some()
 	}
 
 	pub(crate) fn prune(&mut self) {
@@ -458,8 +473,27 @@ mod tests {
 		let variable = lease(2, 46, 50, None, valid_until);
 		let state = LSPS2LeaseState::from_leases(vec![variable.clone()]);
 
-		assert!(state.variable_amount(Some(49)).is_none());
-		assert_eq!(state.variable_amount(Some(50)).unwrap().0.id, variable.id);
+		assert!(state.variable_amount(None, Some(49)).is_none());
+		assert_eq!(state.variable_amount(None, Some(50)).unwrap().0.id, variable.id);
+	}
+
+	#[test]
+	fn variable_lease_matches_resolved_payment_amount() {
+		let valid_until = now_secs() + MIN_LEASE_REMAINING_SECS + 60;
+		let mut incompatible = lease(2, 47, 1, None, valid_until);
+		incompatible.params.max_payment_size_msat = 1_000;
+		let mut compatible = lease(3, 48, 2, None, valid_until);
+		compatible.params.proportional = 500_000;
+		compatible.params.max_payment_size_msat = 3_000;
+		let state = LSPS2LeaseState::from_leases(vec![incompatible, compatible.clone()]);
+
+		let (selected, total_fee_msat) = state.variable_amount(Some(2_000), None).unwrap();
+		assert_eq!(
+			selected.id, compatible.id,
+			"selected variable lease must accept the resolved payment amount"
+		);
+		assert_eq!(total_fee_msat, 1_000);
+		assert!(state.variable_amount(Some(2_000), Some(999)).is_none());
 	}
 
 	#[test]
@@ -472,8 +506,8 @@ mod tests {
 		assert!(state.has_fixed_amount(1_000, Some(100)));
 		assert!(!state.has_fixed_amount(1_000, Some(99)));
 		assert!(!state.has_fixed_amount(2_000, None));
-		assert!(state.has_variable_amount(Some(50)));
-		assert!(!state.has_variable_amount(Some(49)));
+		assert!(state.has_variable_amount(None, Some(50)));
+		assert!(!state.has_variable_amount(None, Some(49)));
 	}
 
 	fn cache_target(