Remove AllPathsFailed outbounds at send_payment_internal callsites instead

This makes it easier to retry payments if all paths fail on initial send, in
in which case we'll want to hold off on removing the pending payment

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index a971ea8b7f0131f8929c031c48798529f8e16fd3..c2d0a6b62c9d11285439645a7a168d410ad9d76f 100644 (file)
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -380,7 +380,9 @@ impl OutboundPayments {
                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
        {
                let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
-               self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path)
+               self.send_payment_internal(route, payment_hash, payment_secret, None, payment_id, None,
+                       onion_session_privs, node_signer, best_block_height, send_payment_along_path)
+                       .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
 
        pub(super) fn send_spontaneous_payment<ES: Deref, NS: Deref, F>(
@@ -402,7 +404,10 @@ impl OutboundPayments {
 
                match self.send_payment_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) {
                        Ok(()) => Ok(payment_hash),
-                       Err(e) => Err(e)
+                       Err(e) => {
+                               self.remove_outbound_if_all_failed(payment_id, &e);
+                               Err(e)
+                       }
                }
        }
 
@@ -501,7 +506,10 @@ impl OutboundPayments {
 
                match self.send_payment_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) {
                        Ok(()) => Ok((payment_hash, payment_id)),
-                       Err(e) => Err(e)
+                       Err(e) => {
+                               self.remove_outbound_if_all_failed(payment_id, &e);
+                               Err(e)
+                       }
                }
        }
 
@@ -648,10 +656,6 @@ impl OutboundPayments {
                                } else { None },
                        })
                } else if has_err {
-                       // If we failed to send any paths, we should remove the new PaymentId from the
-                       // `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::AllFailedResendSafe(results.drain(..).map(|r| r.unwrap_err()).collect()))
                } else {
                        Ok(())
@@ -673,6 +677,16 @@ impl OutboundPayments {
                self.send_payment_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id,
                        recv_value_msat, onion_session_privs, node_signer, best_block_height,
                        send_payment_along_path)
+                       .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
+       }
+
+       // If we failed to send any paths, we should remove the new PaymentId from the
+       // `pending_outbound_payments` map, as the user isn't expected to `abandon_payment`.
+       fn remove_outbound_if_all_failed(&self, payment_id: PaymentId, err: &PaymentSendFailure) {
+               if let &PaymentSendFailure::AllFailedResendSafe(_) = err {
+                       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");
+               }
        }
 
        pub(super) fn claim_htlc<L: Deref>(