From c98f80dfaef155cab35989af85678505e756ac4f Mon Sep 17 00:00:00 2001 From: jurvis Date: Sun, 4 Dec 2022 17:24:08 -0800 Subject: [PATCH] Expose pending payments through ChannelManager Adds a new method, `list_recent_payments ` to `ChannelManager` that returns an array of `RecentPaymentDetails` containing the payment status (Fulfilled/Retryable/Abandoned) and its total amount across all paths. --- lightning/src/ln/channelmanager.rs | 94 ++++++++++++++++++++++++++---- lightning/src/ln/payment_tests.rs | 30 +++++++++- 2 files changed, 109 insertions(+), 15 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f2ee0860e..83db9d58b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1154,6 +1154,36 @@ impl ChannelDetails { } } +/// Used by [`ChannelManager::list_recent_payments`] to express the status of recent payments. +/// These include payments that have yet to find a successful path, or have unresolved HTLCs. +#[derive(Debug, PartialEq)] +pub enum RecentPaymentDetails { + /// When a payment is still being sent and awaiting successful delivery. + Pending { + /// Hash of the payment that is currently being sent but has yet to be fulfilled or + /// abandoned. + payment_hash: PaymentHash, + /// Total amount (in msat, excluding fees) across all paths for this payment, + /// not just the amount currently inflight. + total_msat: u64, + }, + /// When a pending payment is fulfilled, we continue tracking it until all pending HTLCs have + /// been resolved. Upon receiving [`Event::PaymentSent`], we delay for a few minutes before the + /// payment is removed from tracking. + Fulfilled { + /// Hash of the payment that was claimed. `None` for serializations of [`ChannelManager`] + /// made before LDK version 0.0.104. + payment_hash: Option, + }, + /// After a payment is explicitly abandoned by calling [`ChannelManager::abandon_payment`], it + /// is marked as abandoned until an [`Event::PaymentFailed`] is generated. A payment could also + /// be marked as abandoned if pathfinding fails repeatedly or retries have been exhausted. + Abandoned { + /// Hash of the payment that we have given up trying to send. + payment_hash: PaymentHash, + }, +} + /// Route hints used in constructing invoices for [phantom node payents]. /// /// [phantom node payments]: crate::chain::keysinterface::PhantomKeysManager @@ -1691,6 +1721,34 @@ where self.list_channels_with_filter(|&(_, ref channel)| channel.is_live()) } + /// Returns in an undefined order recent payments that -- if not fulfilled -- have yet to find a + /// successful path, or have unresolved HTLCs. + /// + /// This can be useful for payments that may have been prepared, but ultimately not sent, as a + /// result of a crash. If such a payment exists, is not listed here, and an + /// [`Event::PaymentSent`] has not been received, you may consider retrying the payment. + /// + /// [`Event::PaymentSent`]: events::Event::PaymentSent + pub fn list_recent_payments(&self) -> Vec { + self.pending_outbound_payments.pending_outbound_payments.lock().unwrap().iter() + .filter_map(|(_, pending_outbound_payment)| match pending_outbound_payment { + PendingOutboundPayment::Retryable { payment_hash, total_msat, .. } => { + Some(RecentPaymentDetails::Pending { + payment_hash: *payment_hash, + total_msat: *total_msat, + }) + }, + PendingOutboundPayment::Abandoned { payment_hash, .. } => { + Some(RecentPaymentDetails::Abandoned { payment_hash: *payment_hash }) + }, + PendingOutboundPayment::Fulfilled { payment_hash, .. } => { + Some(RecentPaymentDetails::Fulfilled { payment_hash: *payment_hash }) + }, + PendingOutboundPayment::Legacy { .. } => None + }) + .collect() + } + /// Helper function that issues the channel close events fn issue_channel_close_events(&self, channel: &Channel<::Signer>, closure_reason: ClosureReason) { let mut pending_events_lock = self.pending_events.lock().unwrap(); @@ -2415,9 +2473,14 @@ where /// Sends a payment along a given route. /// - /// Value parameters are provided via the last hop in route, see documentation for RouteHop + /// Value parameters are provided via the last hop in route, see documentation for [`RouteHop`] /// fields for more info. /// + /// May generate SendHTLCs message(s) event on success, which should be relayed (e.g. via + /// [`PeerManager::process_events`]). + /// + /// # Avoiding Duplicate Payments + /// /// If a pending payment is currently in-flight with the same [`PaymentId`] provided, this /// method will error with an [`APIError::InvalidRoute`]. Note, however, that once a payment /// is no longer pending (either via [`ChannelManager::abandon_payment`], or handling of an @@ -2430,12 +2493,16 @@ where /// consider using the [`PaymentHash`] as the key for tracking payments. In that case, the /// [`PaymentId`] should be a copy of the [`PaymentHash`] bytes. /// - /// May generate SendHTLCs message(s) event on success, which should be relayed (e.g. via - /// [`PeerManager::process_events`]). + /// Additionally, in the scenario where we begin the process of sending a payment, but crash + /// before `send_payment` returns (or prior to [`ChannelMonitorUpdate`] persistence if you're + /// using [`ChannelMonitorUpdateStatus::InProgress`]), the payment may be lost on restart. See + /// [`ChannelManager::list_recent_payments`] for more information. + /// + /// # Possible Error States on [`PaymentSendFailure`] /// /// Each path may have a different return value, and PaymentSendValue may return a Vec with /// each entry matching the corresponding-index entry in the route paths, see - /// PaymentSendFailure for more info. + /// [`PaymentSendFailure`] for more info. /// /// In general, a path may raise: /// * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee, @@ -2450,18 +2517,21 @@ where /// irrevocably committed to on our end. In such a case, do NOT retry the payment with a /// different route unless you intend to pay twice! /// - /// payment_secret is unrelated to payment_hash (or PaymentPreimage) and exists to authenticate - /// the sender to the recipient and prevent payment-probing (deanonymization) attacks. For - /// newer nodes, it will be provided to you in the invoice. If you do not have one, the Route - /// must not contain multiple paths as multi-path payments require a recipient-provided - /// payment_secret. + /// # A caution on `payment_secret` + /// + /// `payment_secret` is unrelated to `payment_hash` (or [`PaymentPreimage`]) and exists to + /// authenticate the sender to the recipient and prevent payment-probing (deanonymization) + /// attacks. For newer nodes, it will be provided to you in the invoice. If you do not have one, + /// the [`Route`] must not contain multiple paths as multi-path payments require a + /// recipient-provided `payment_secret`. /// - /// If a payment_secret *is* provided, we assume that the invoice had the payment_secret feature - /// bit set (either as required or as available). If multiple paths are present in the Route, - /// we assume the invoice had the basic_mpp feature set. + /// If a `payment_secret` *is* provided, we assume that the invoice had the payment_secret + /// feature bit set (either as required or as available). If multiple paths are present in the + /// [`Route`], we assume the invoice had the basic_mpp feature set. /// /// [`Event::PaymentSent`]: events::Event::PaymentSent /// [`PeerManager::process_events`]: crate::ln::peer_handler::PeerManager::process_events + /// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option, payment_id: PaymentId) -> Result<(), PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); self.pending_outbound_payments diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 68074f1b5..774f1a050 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -16,7 +16,7 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS use crate::chain::keysinterface::EntropySource; use crate::chain::transaction::OutPoint; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; -use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS}; +use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, ChannelManager, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails}; use crate::ln::features::InvoiceFeatures; use crate::ln::msgs; use crate::ln::msgs::ChannelMessageHandler; @@ -1274,7 +1274,11 @@ fn test_trivial_inflight_htlc_tracking(){ let (_, _, chan_2_id, _) = create_announced_chan_between_nodes(&nodes, 1, 2); // Send and claim the payment. Inflight HTLCs should be empty. - send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 500000); + let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 500000); + nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)).unwrap(); + check_added_monitors!(nodes[0], 1); + pass_along_route(&nodes[0], &[&vec!(&nodes[1], &nodes[2])[..]], 500000, payment_hash, payment_secret); + claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], payment_preimage); { let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs(); @@ -1299,9 +1303,17 @@ fn test_trivial_inflight_htlc_tracking(){ assert_eq!(chan_1_used_liquidity, None); assert_eq!(chan_2_used_liquidity, None); } + let pending_payments = nodes[0].node.list_recent_payments(); + assert_eq!(pending_payments.len(), 1); + assert_eq!(pending_payments[0], RecentPaymentDetails::Fulfilled { payment_hash: Some(payment_hash) }); + + // Remove fulfilled payment + for _ in 0..=IDEMPOTENCY_TIMEOUT_TICKS { + nodes[0].node.timer_tick_occurred(); + } // Send the payment, but do not claim it. Our inflight HTLCs should contain the pending payment. - let (payment_preimage, _, _) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 500000); + let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 500000); { let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs(); @@ -1327,9 +1339,18 @@ fn test_trivial_inflight_htlc_tracking(){ assert_eq!(chan_1_used_liquidity, Some(501000)); assert_eq!(chan_2_used_liquidity, Some(500000)); } + let pending_payments = nodes[0].node.list_recent_payments(); + assert_eq!(pending_payments.len(), 1); + assert_eq!(pending_payments[0], RecentPaymentDetails::Pending { payment_hash, total_msat: 500000 }); // Now, let's claim the payment. This should result in the used liquidity to return `None`. claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); + + // Remove fulfilled payment + for _ in 0..=IDEMPOTENCY_TIMEOUT_TICKS { + nodes[0].node.timer_tick_occurred(); + } + { let inflight_htlcs = node_chanmgrs[0].compute_inflight_htlcs(); @@ -1354,6 +1375,9 @@ fn test_trivial_inflight_htlc_tracking(){ assert_eq!(chan_1_used_liquidity, None); assert_eq!(chan_2_used_liquidity, None); } + + let pending_payments = nodes[0].node.list_recent_payments(); + assert_eq!(pending_payments.len(), 0); } #[test] -- 2.39.5