From a2489b126ff3156fc5c2a7e222dab4a17ac1e64b Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 17 Feb 2023 17:41:21 -0500 Subject: [PATCH] Deduplicate PendingHTLCsForwardable events when queueing --- lightning/src/ln/channelmanager.rs | 42 +++++++++++++++------------- lightning/src/ln/outbound_payment.rs | 20 ++++++------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e7a558cb6..77a335fed 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -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), + }); } } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index f196fbe73..f252d88ac 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -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( &self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, path: &Vec, session_priv: &SecretKey, payment_id: &PaymentId, payment_params: &Option, probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1, pending_events: &Mutex>, 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( -- 2.39.5