From bf03d4ccbe78dd7d951760c0d417bb9218273a87 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 17 Feb 2023 17:35:09 -0500 Subject: [PATCH] On retryable update_fail, don't queue redundant PendingHTLCsForwardable --- lightning/src/ln/outbound_payment.rs | 17 ++++++++++++++++- lightning/src/ln/payment_tests.rs | 6 +----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 715a041d..f196fbe7 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -1027,6 +1027,21 @@ impl OutboundPayments { let mut session_priv_bytes = [0; 32]; session_priv_bytes.copy_from_slice(&session_priv[..]); let mut outbounds = self.pending_outbound_payments.lock().unwrap(); + + // If any payments already need retry, there's no need to generate a redundant + // `PendingHTLCsForwardable`. + let already_awaiting_retry = outbounds.iter().any(|(_, pmt)| { + let mut awaiting_retry = false; + if pmt.is_auto_retryable_now() { + if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, .. } = pmt { + if pending_amt_msat < total_msat { + awaiting_retry = true; + } + } + } + awaiting_retry + }); + let mut all_paths_failed = false; let mut full_failure_ev = None; let mut pending_retry_ev = None; @@ -1120,7 +1135,7 @@ impl OutboundPayments { } // If we miss abandoning the payment above, we *must* generate an event here or else the // payment will sit in our outbounds forever. - if attempts_remaining { + if attempts_remaining && !already_awaiting_retry { debug_assert!(full_failure_ev.is_none()); pending_retry_ev = Some(events::Event::PendingHTLCsForwardable { time_forwardable: Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS), diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 8e35e650..3285cf4d 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -2349,7 +2349,7 @@ fn no_extra_retries_on_back_to_back_fail() { // Because we now retry payments as a batch, we simply return a single-path route in the // second, batched, request, have that fail, ensure the payment was abandoned. let mut events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 4); + assert_eq!(events.len(), 3); match events[0] { Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => { assert_eq!(payment_hash, ev_payment_hash); @@ -2368,10 +2368,6 @@ fn no_extra_retries_on_back_to_back_fail() { }, _ => panic!("Unexpected event"), } - match events[3] { - Event::PendingHTLCsForwardable { .. } => {}, - _ => panic!("Unexpected event"), - } nodes[0].node.process_pending_htlc_forwards(); let retry_htlc_updates = SendEvent::from_node(&nodes[0]); -- 2.30.2