Add a separate PaymentSendFailure for idempotency violation

When a user attempts to send a payment but it fails due to
idempotency key violation, they need to know that this was the
reason as they need to handle the error programmatically
differently from other errors.

Here we simply add a new `PaymentSendFailure` enum variant for
`DuplicatePayment` to allow for that.

diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index aa6a3faf023fc00ed6688530cd465274bed9ec27..783bbc06aa9fe58e4de90e5ce13650aeb0458b9d 100644 (file)
--- a/fuzz/src/chanmon_consistency.rs
+++ b/fuzz/src/chanmon_consistency.rs
@@ -289,6 +289,7 @@ fn check_payment_err(send_err: PaymentSendFailure) {
                PaymentSendFailure::PartialFailure { results, .. } => {
                        for res in results { if let Err(api_err) = res { check_api_err(api_err); } }
                },
+               PaymentSendFailure::DuplicatePayment => panic!(),
        }
 }
 

diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs
index 118ad5da6ef19438df754c2765f8450559ab601d..0d1279710be27f293dfb58781dbfbba721295c89 100644 (file)
--- a/lightning-invoice/src/payment.rs
+++ b/lightning-invoice/src/payment.rs
@@ -543,6 +543,7 @@ where
                        Err(e) => match e {
                                PaymentSendFailure::ParameterError(_) => Err(e),
                                PaymentSendFailure::PathParameterError(_) => Err(e),
+                               PaymentSendFailure::DuplicatePayment => Err(e),
                                PaymentSendFailure::AllFailedResendSafe(_) => {
                                        let mut payment_cache = self.payment_cache.lock().unwrap();
                                        let payment_info = payment_cache.get_mut(&payment_hash).unwrap();
@@ -658,6 +659,10 @@ where
                        Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
                                self.retry_payment(payment_id, payment_hash, params)
                        },
+                       Err(PaymentSendFailure::DuplicatePayment) => {
+                               log_error!(self.logger, "Got a DuplicatePayment error when attempting to retry a payment, this shouldn't happen.");
+                               Err(())
+                       }
                        Err(PaymentSendFailure::PartialFailure { failed_paths_retry, results, .. }) => {
                                // If a `PartialFailure` error contains a result that is an `Ok()`, it means that
                                // part of our payment is retried. When we receive `MonitorUpdateInProgress`, it

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7de2da89c59a92c6e5dcffb14b9b7d3ae0493203..7884ff0da3a1f81a18db5df9644f6235627ac968 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -1188,16 +1188,25 @@ 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 resend the payment in full (though you probably want to do so over different
@@ -1207,6 +1216,12 @@ pub enum PaymentSendFailure {
        /// [`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.
@@ -2611,9 +2626,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(),

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index a65da368709a77aa84147b86dc11e55ae367a558..d3146a6e68667f1426e2edf55f22f28c3ea782e3 100644 (file)
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -1275,7 +1275,7 @@ fn claimed_send_payment_idempotent() {
                        // payment_id, it should be rejected.
                        let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
                        match send_result {
-                               Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
+                               Err(PaymentSendFailure::DuplicatePayment) => {},
                                _ => panic!("Unexpected send result: {:?}", send_result),
                        }
 
@@ -1283,7 +1283,7 @@ fn claimed_send_payment_idempotent() {
                        // also be rejected.
                        let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
                        match send_result {
-                               Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
+                               Err(PaymentSendFailure::DuplicatePayment) => {},
                                _ => panic!("Unexpected send result: {:?}", send_result),
                        }
                }
@@ -1347,7 +1347,7 @@ fn abandoned_send_payment_idempotent() {
                        // payment_id, it should be rejected.
                        let send_result = nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), payment_id);
                        match send_result {
-                               Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
+                               Err(PaymentSendFailure::DuplicatePayment) => {},
                                _ => panic!("Unexpected send result: {:?}", send_result),
                        }
 
@@ -1355,7 +1355,7 @@ fn abandoned_send_payment_idempotent() {
                        // also be rejected.
                        let send_result = nodes[0].node.send_spontaneous_payment(&route, None, payment_id);
                        match send_result {
-                               Err(PaymentSendFailure::ParameterError(APIError::RouteError { err: "Payment already in progress" })) => {},
+                               Err(PaymentSendFailure::DuplicatePayment) => {},
                                _ => panic!("Unexpected send result: {:?}", send_result),
                        }
                }