From 0964c7c12173f9449556d8b8dad89b0330ca725b Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 16 Oct 2025 10:02:43 +0200 Subject: [PATCH] Make `skimmed_fee_msat` optional in LSPS2's `payment_forwarded` API Recent changes made `skimmed_fee_msat` a required field of `LSPS2ServiceHandler`'s `payment_forwarded` API, which seemed reasonable given that the field is available since LDK 0.0.122. However, the field is of course only set post-0.0.122 when a fee was actually withheld, which makes forcing the user to `unwrap_or(0)` potentially confusing, especially since our internal logic was written so that it *can* handle non-intercept-SCID-forwarding cases. Here, we restore the previous idea of "just pass `PaymentForwarded` fields on, we'll handle the appropriately internally anyways. --- lightning-liquidity/src/lsps2/service.rs | 18 +++++++++--------- .../tests/lsps2_integration_tests.rs | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lightning-liquidity/src/lsps2/service.rs b/lightning-liquidity/src/lsps2/service.rs index 9bb3ded58c2..c62abc81205 100644 --- a/lightning-liquidity/src/lsps2/service.rs +++ b/lightning-liquidity/src/lsps2/service.rs @@ -434,7 +434,7 @@ impl OutboundJITChannelState { } fn payment_forwarded( - &mut self, skimmed_fee_msat: u64, + &mut self, skimmed_fee_msat: Option, ) -> Result, ChannelStateError> { match self { OutboundJITChannelState::PendingPaymentForward { @@ -442,7 +442,7 @@ impl OutboundJITChannelState { channel_id, opening_fee_msat, } => { - if skimmed_fee_msat >= *opening_fee_msat { + if skimmed_fee_msat >= Some(*opening_fee_msat) { let mut pq = core::mem::take(payment_queue); let forward_htlcs = ForwardHTLCsAction(*channel_id, pq.clear()); *self = OutboundJITChannelState::PaymentForwarded { channel_id: *channel_id }; @@ -542,7 +542,7 @@ impl OutboundJITChannel { } fn payment_forwarded( - &mut self, skimmed_fee_msat: u64, + &mut self, skimmed_fee_msat: Option, ) -> Result, LightningError> { let action = self.state.payment_forwarded(skimmed_fee_msat)?; Ok(action) @@ -1179,7 +1179,7 @@ where /// /// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded pub async fn payment_forwarded( - &self, next_channel_id: ChannelId, skimmed_fee_msat: u64, + &self, next_channel_id: ChannelId, skimmed_fee_msat: Option, ) -> Result<(), APIError> { let mut should_persist = None; if let Some(counterparty_node_id) = @@ -2239,7 +2239,7 @@ where /// /// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded pub fn payment_forwarded( - &self, next_channel_id: ChannelId, skimmed_fee_msat: u64, + &self, next_channel_id: ChannelId, skimmed_fee_msat: Option, ) -> Result<(), APIError> { let mut fut = Box::pin(self.inner.payment_forwarded(next_channel_id, skimmed_fee_msat)); @@ -2583,7 +2583,7 @@ mod tests { } // Payment completes, queued payments get forwarded. { - let action = state.payment_forwarded(100000000000).unwrap(); + let action = state.payment_forwarded(Some(100000000000)).unwrap(); assert!(matches!(state, OutboundJITChannelState::PaymentForwarded { .. })); match action { Some(ForwardHTLCsAction(channel_id, htlcs)) => { @@ -2718,7 +2718,7 @@ mod tests { } // Payment completes, queued payments get forwarded. { - let action = state.payment_forwarded(10000000000).unwrap(); + let action = state.payment_forwarded(Some(10000000000)).unwrap(); assert!(matches!(state, OutboundJITChannelState::PaymentForwarded { .. })); match action { Some(ForwardHTLCsAction(channel_id, htlcs)) => { @@ -2839,14 +2839,14 @@ mod tests { ); // Forward a payment that is not enough to cover the fees - let _ = jit_channel.payment_forwarded(min_fee_msat - 1).unwrap(); + let _ = jit_channel.payment_forwarded(Some(min_fee_msat - 1)).unwrap(); assert!( !jit_channel.should_broadcast_funding_transaction(), "Should not broadcast before all the fees are collected" ); - let _ = jit_channel.payment_forwarded(min_fee_msat).unwrap(); + let _ = jit_channel.payment_forwarded(Some(min_fee_msat)).unwrap(); let broadcast_allowed = jit_channel.should_broadcast_funding_transaction(); diff --git a/lightning-liquidity/tests/lsps2_integration_tests.rs b/lightning-liquidity/tests/lsps2_integration_tests.rs index 82f93b5990c..86eaecc99bf 100644 --- a/lightning-liquidity/tests/lsps2_integration_tests.rs +++ b/lightning-liquidity/tests/lsps2_integration_tests.rs @@ -1354,7 +1354,7 @@ fn client_trusts_lsp_end_to_end_test() { } => { assert_eq!(prev_node_id, Some(payer_node_id)); assert_eq!(next_node_id, Some(client_node_id)); - service_handler.payment_forwarded(channel_id, skimmed_fee_msat.unwrap_or(0)).unwrap(); + service_handler.payment_forwarded(channel_id, skimmed_fee_msat).unwrap(); Some(total_fee_earned_msat.unwrap() - skimmed_fee_msat.unwrap()) }, _ => panic!("Expected PaymentForwarded event, got: {:?}", service_events[0]), @@ -1782,7 +1782,7 @@ fn late_payment_forwarded_and_safe_after_force_close_does_not_broadcast() { Event::PaymentForwarded { skimmed_fee_msat, .. } => skimmed_fee_msat.unwrap_or(0), other => panic!("Expected PaymentForwarded, got {:?}", other), }; - service_handler.payment_forwarded(channel_id, skimmed).unwrap(); + service_handler.payment_forwarded(channel_id, Some(skimmed)).unwrap(); // Force-close the service->client channel service_node @@ -2325,7 +2325,7 @@ fn client_trusts_lsp_partial_fee_does_not_trigger_broadcast() { assert!(service_node.liquidity_manager.get_and_clear_pending_events().is_empty()); let partial_skim_msat = fee_base_msat - 1; // less than promised fee - service_handler.payment_forwarded(channel_id, partial_skim_msat).unwrap(); + service_handler.payment_forwarded(channel_id, Some(partial_skim_msat)).unwrap(); let broadcasted = service_node.inner.tx_broadcaster.txn_broadcasted.lock().unwrap(); assert!(broadcasted.is_empty(), "There should be no broadcasted txs yet");