On retryable update_fail, don't queue redundant PendingHTLCsForwardable
authorValentine Wallace <vwallace@protonmail.com>
Fri, 17 Feb 2023 22:35:09 +0000 (17:35 -0500)
committerValentine Wallace <vwallace@protonmail.com>
Fri, 17 Feb 2023 22:35:09 +0000 (17:35 -0500)
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs

index 715a041dcf3512b0fdfb83244d7418b148b3cc80..f196fbe734ff7624246ce1c40f66097c26aac6b3 100644 (file)
@@ -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),
index 8e35e650b7020495687c5e635856c50789dc3d7d..3285cf4d7a5b41de63766f1c92776bd19f01d364 100644 (file)
@@ -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]);