diff --git a/src/event.rs b/src/event.rs index 657c938ed6..107d52d049 100644 --- a/src/event.rs +++ b/src/event.rs @@ -707,15 +707,24 @@ where } } - fn lsps2_max_total_opening_fee_msat(payment_metadata: &[u8], amount_msat: u64) -> Option { + fn lsps2_max_total_opening_fee_msat( + payment_metadata: &[u8], gross_amount_msat: u64, + ) -> Option { 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,6 +2814,7 @@ 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, }), }; @@ -2815,6 +2827,51 @@ mod tests { ); } + #[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). Use the wire format so this also exercises readers predating the minimum. + 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::>::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::>::lsps2_max_total_opening_fee_msat( + &metadata.encode(), + gross_amount_msat, + ), + expected + ); + } + } + #[test] fn lsps2_payment_metadata_missing_or_malformed_limit_is_rejected() { let empty_metadata = PaymentMetadata { lsps2_parameters: None }.encode(); @@ -2822,6 +2879,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(); diff --git a/src/liquidity/client/lsps2.rs b/src/liquidity/client/lsps2.rs index 8084cace78..8f98e152cb 100644 --- a/src/liquidity/client/lsps2.rs +++ b/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,6 +110,11 @@ 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), @@ -117,11 +122,6 @@ where 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, diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index e2a34828cf..b68694fc77 100644 --- a/src/payment/bolt11.rs +++ b/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) }; diff --git a/src/payment/store.rs b/src/payment/store.rs index 12d360134a..c5e77f700c 100644 --- a/src/payment/store.rs +++ b/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, + /// 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, + max_proportional_opening_fee_ppm_msat: Option, + } + + 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, preimage: Option, secret: Option, - 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), }; diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 2c94803499..2ca4e64675 100644 --- a/tests/integration_tests_rust.rs +++ b/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)]