Merge pull request #1826 from TheBlueMatt/2022-10-idempotency-err

[rust-lightning] / lightning / src / ln / channelmanager.rs

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 121c1d4c66c36cb64f21df76823f5d24ace0e97a..1446f5757bb85e4348c1ec68d04a7b917989f5e4 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1204,24 +1204,40 @@ impl ChannelDetails {
 #[derive(Clone, Debug)]
 pub enum PaymentSendFailure {
        /// A parameter which was passed to send_payment was invalid, preventing us from attempting to
-       /// send the payment at all. No channel state has been changed or messages sent to peers, and
-       /// once you've changed the parameter at error, you can freely retry the payment in full.
+       /// send the payment at all.
+       ///
+       /// You can freely resend the payment in full (with the parameter error fixed).
+       ///
+       /// Because the payment failed outright, no payment tracking is done, you do not need to call
+       /// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
+       /// for this payment.
        ParameterError(APIError),
        /// A parameter in a single path which was passed to send_payment was invalid, preventing us
-       /// from attempting to send the payment at all. No channel state has been changed or messages
-       /// sent to peers, and once you've changed the parameter at error, you can freely retry the
-       /// payment in full.
+       /// from attempting to send the payment at all.
+       ///
+       /// You can freely resend the payment in full (with the parameter error fixed).
        ///
        /// The results here are ordered the same as the paths in the route object which was passed to
        /// send_payment.
+       ///
+       /// Because the payment failed outright, no payment tracking is done, you do not need to call
+       /// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
+       /// for this payment.
        PathParameterError(Vec<Result<(), APIError>>),
        /// All paths which were attempted failed to send, with no channel state change taking place.
-       /// You can freely retry the payment in full (though you probably want to do so over different
+       /// You can freely resend the payment in full (though you probably want to do so over different
        /// paths than the ones selected).
        ///
-       /// [`ChannelManager::abandon_payment`] does *not* need to be called for this payment and
-       /// [`ChannelManager::retry_payment`] will *not* work for this payment.
-       AllFailedRetrySafe(Vec<APIError>),
+       /// Because the payment failed outright, no payment tracking is done, you do not need to call
+       /// [`ChannelManager::abandon_payment`] and [`ChannelManager::retry_payment`] will *not* work
+       /// for this payment.
+       AllFailedResendSafe(Vec<APIError>),
+       /// Indicates that a payment for the provided [`PaymentId`] is already in-flight and has not
+       /// yet completed (i.e. generated an [`Event::PaymentSent`]) or been abandoned (via
+       /// [`ChannelManager::abandon_payment`]).
+       ///
+       /// [`Event::PaymentSent`]: events::Event::PaymentSent
+       DuplicatePayment,
        /// Some paths which were attempted failed to send, though possibly not all. At least some
        /// paths have irrevocably committed to the HTLC and retrying the payment in full would result
        /// in over-/re-payment.
@@ -2632,9 +2648,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 
                let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
                match pending_outbounds.entry(payment_id) {
-                       hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::ParameterError(APIError::RouteError {
-                               err: "Payment already in progress"
-                       })),
+                       hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment),
                        hash_map::Entry::Vacant(entry) => {
                                let payment = entry.insert(PendingOutboundPayment::Retryable {
                                        session_privs: HashSet::new(),
@@ -2748,7 +2762,7 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
                        // `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
                        let removed = self.pending_outbound_payments.lock().unwrap().remove(&payment_id).is_some();
                        debug_assert!(removed, "We should always have a pending payment to remove here");
-                       Err(PaymentSendFailure::AllFailedRetrySafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
+                       Err(PaymentSendFailure::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
                } else {
                        Ok(())
                }