Deduplicate PendingHTLCsForwardable events when queueing
authorValentine Wallace <vwallace@protonmail.com>
Fri, 17 Feb 2023 22:41:21 +0000 (17:41 -0500)
committerValentine Wallace <vwallace@protonmail.com>
Fri, 17 Feb 2023 22:41:21 +0000 (17:41 -0500)
lightning/src/ln/channelmanager.rs
lightning/src/ln/outbound_payment.rs

index e7a558cb6151fad5755da98b2cfdefe99e89275c..77a335fed9910965f91fc9fd5fdc731176bd8494 100644 (file)
@@ -3737,16 +3737,19 @@ where
                // being fully configured. See the docs for `ChannelManagerReadArgs` for more.
                match source {
                        HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => {
-                               self.pending_outbound_payments.fail_htlc(source, payment_hash, onion_error, path, session_priv, payment_id, payment_params, self.probing_cookie_secret, &self.secp_ctx, &self.pending_events, &self.logger);
+                               if self.pending_outbound_payments.fail_htlc(source, payment_hash, onion_error, path,
+                                       session_priv, payment_id, payment_params, self.probing_cookie_secret, &self.secp_ctx,
+                                       &self.pending_events, &self.logger)
+                               { self.push_pending_forwards_ev(); }
                        },
                        HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => {
                                log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error);
                                let err_packet = onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret);
 
-                               let mut forward_event = None;
+                               let mut push_forward_ev = false;
                                let mut forward_htlcs = self.forward_htlcs.lock().unwrap();
                                if forward_htlcs.is_empty() {
-                                       forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS));
+                                       push_forward_ev = true;
                                }
                                match forward_htlcs.entry(*short_channel_id) {
                                        hash_map::Entry::Occupied(mut entry) => {
@@ -3757,12 +3760,8 @@ where
                                        }
                                }
                                mem::drop(forward_htlcs);
+                               if push_forward_ev { self.push_pending_forwards_ev(); }
                                let mut pending_events = self.pending_events.lock().unwrap();
-                               if let Some(time) = forward_event {
-                                       pending_events.push(events::Event::PendingHTLCsForwardable {
-                                               time_forwardable: time
-                                       });
-                               }
                                pending_events.push(events::Event::HTLCHandlingFailed {
                                        prev_channel_id: outpoint.to_channel_id(),
                                        failed_next_destination: destination,
@@ -4839,7 +4838,7 @@ where
        #[inline]
        fn forward_htlcs(&self, per_source_pending_forwards: &mut [(u64, OutPoint, u128, Vec<(PendingHTLCInfo, u64)>)]) {
                for &mut (prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, ref mut pending_forwards) in per_source_pending_forwards {
-                       let mut forward_event = None;
+                       let mut push_forward_event = false;
                        let mut new_intercept_events = Vec::new();
                        let mut failed_intercept_forwards = Vec::new();
                        if !pending_forwards.is_empty() {
@@ -4897,7 +4896,7 @@ where
                                                                // We don't want to generate a PendingHTLCsForwardable event if only intercepted
                                                                // payments are being processed.
                                                                if forward_htlcs_empty {
-                                                                       forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS));
+                                                                       push_forward_event = true;
                                                                }
                                                                entry.insert(vec!(HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
                                                                        prev_short_channel_id, prev_funding_outpoint, prev_htlc_id, prev_user_channel_id, forward_info })));
@@ -4915,16 +4914,21 @@ where
                                let mut events = self.pending_events.lock().unwrap();
                                events.append(&mut new_intercept_events);
                        }
+                       if push_forward_event { self.push_pending_forwards_ev() }
+               }
+       }
 
-                       match forward_event {
-                               Some(time) => {
-                                       let mut pending_events = self.pending_events.lock().unwrap();
-                                       pending_events.push(events::Event::PendingHTLCsForwardable {
-                                               time_forwardable: time
-                                       });
-                               }
-                               None => {},
-                       }
+       // We only want to push a PendingHTLCsForwardable event if no others are queued.
+       fn push_pending_forwards_ev(&self) {
+               let mut pending_events = self.pending_events.lock().unwrap();
+               let forward_ev_exists = pending_events.iter()
+                       .find(|ev| if let events::Event::PendingHTLCsForwardable { .. } = ev { true } else { false })
+                       .is_some();
+               if !forward_ev_exists {
+                       pending_events.push(events::Event::PendingHTLCsForwardable {
+                               time_forwardable:
+                                       Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS),
+                       });
                }
        }
 
index f196fbe734ff7624246ce1c40f66097c26aac6b3..f252d88ac364e87947ed42c31bef96b5c276375f 100644 (file)
@@ -15,7 +15,7 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey};
 
 use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient};
 use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
-use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId};
+use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId};
 use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA;
 use crate::ln::msgs::DecodeError;
 use crate::ln::onion_utils::HTLCFailReason;
@@ -30,7 +30,6 @@ use crate::util::time::tests::SinceEpoch;
 use core::cmp;
 use core::fmt::{self, Display, Formatter};
 use core::ops::Deref;
-use core::time::Duration;
 
 use crate::prelude::*;
 use crate::sync::Mutex;
@@ -1012,12 +1011,13 @@ impl OutboundPayments {
                });
        }
 
+       // Returns a bool indicating whether a PendingHTLCsForwardable event should be generated.
        pub(super) fn fail_htlc<L: Deref>(
                &self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason,
                path: &Vec<RouteHop>, session_priv: &SecretKey, payment_id: &PaymentId,
                payment_params: &Option<PaymentParameters>, probing_cookie_secret: [u8; 32],
                secp_ctx: &Secp256k1<secp256k1::All>, pending_events: &Mutex<Vec<events::Event>>, logger: &L
-       ) where L::Target: Logger {
+       ) -> bool where L::Target: Logger {
                #[cfg(test)]
                let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source);
                #[cfg(not(test))]
@@ -1044,16 +1044,16 @@ impl OutboundPayments {
 
                let mut all_paths_failed = false;
                let mut full_failure_ev = None;
-               let mut pending_retry_ev = None;
+               let mut pending_retry_ev = false;
                let mut retry = None;
                let attempts_remaining = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
                        if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) {
                                log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
-                               return
+                               return false
                        }
                        if payment.get().is_fulfilled() {
                                log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0));
-                               return
+                               return false
                        }
                        let mut is_retryable_now = payment.get().is_auto_retryable_now();
                        if let Some(scid) = short_channel_id {
@@ -1105,7 +1105,7 @@ impl OutboundPayments {
                        is_retryable_now
                } else {
                        log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
-                       return
+                       return false
                };
                core::mem::drop(outbounds);
                log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
@@ -1137,9 +1137,7 @@ impl OutboundPayments {
                                // payment will sit in our outbounds forever.
                                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),
-                                       });
+                                       pending_retry_ev = true;
                                }
                                events::Event::PaymentPathFailed {
                                        payment_id: Some(*payment_id),
@@ -1160,7 +1158,7 @@ impl OutboundPayments {
                let mut pending_events = pending_events.lock().unwrap();
                pending_events.push(path_failure);
                if let Some(ev) = full_failure_ev { pending_events.push(ev); }
-               if let Some(ev) = pending_retry_ev { pending_events.push(ev); }
+               pending_retry_ev
        }
 
        pub(super) fn abandon_payment(