From 72a7da8d5175f656d6e4b73b7ab7f5b2ee9a89f5 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Sun, 18 Dec 2022 23:29:45 -0500 Subject: [PATCH] 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 --- lightning/src/ln/outbound_payment.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a971ea8b..c2d0a6b6 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -380,7 +380,9 @@ impl OutboundPayments { u32, PaymentId, &Option, [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( @@ -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( -- 2.30.2