From: Valentine Wallace <vwallace@protonmail.com>
Date: Fri, 6 Jan 2023 23:39:51 +0000 (-0500)
Subject: Generate PendingHTLCsForwardable upon retryable payment
X-Git-Tag: v0.0.114-beta~43^2~6
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=686ef083162758f980786b3e5a85f4412cd4b847;p=rust-lightning

Generate PendingHTLCsForwardable upon retryable payment
---

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index b18ddaeba..9cf58c918 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -388,7 +388,7 @@ impl MsgHandleErrInternal {
 /// Event::PendingHTLCsForwardable for the API guidelines indicating how long should be waited).
 /// This provides some limited amount of privacy. Ideally this would range from somewhere like one
 /// second to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly.
-const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100;
+pub(super) const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100;
 
 /// For events which result in both a RevokeAndACK and a CommitmentUpdate, by default they should
 /// be sent in the order they appear in the return value, however sometimes the order needs to be
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 0a5a964b3..a971ea8b7 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::{HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId};
+use crate::ln::channelmanager::{HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId};
 use crate::ln::msgs::DecodeError;
 use crate::ln::onion_utils::HTLCFailReason;
 use crate::routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, RoutePath};
@@ -29,6 +29,7 @@ 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;
@@ -87,6 +88,11 @@ impl PendingOutboundPayment {
 		}
 		false
 	}
+	pub fn insert_previously_failed_scid(&mut self, scid: u64) {
+		if let PendingOutboundPayment::Retryable { route_params: Some(params), .. } = self {
+			params.payment_params.previously_failed_channels.push(scid);
+		}
+	}
 	pub(super) fn is_fulfilled(&self) -> bool {
 		match self {
 			PendingOutboundPayment::Fulfilled { .. } => true,
@@ -793,7 +799,8 @@ impl OutboundPayments {
 		let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 		let mut all_paths_failed = false;
 		let mut full_failure_ev = None;
-		if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) {
+		let mut pending_retry_ev = 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
@@ -802,6 +809,10 @@ impl OutboundPayments {
 				log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0));
 				return
 			}
+			let is_retryable_now = payment.get().is_retryable_now();
+			if let Some(scid) = short_channel_id {
+				payment.get_mut().insert_previously_failed_scid(scid);
+			}
 			if payment.get().remaining_parts() == 0 {
 				all_paths_failed = true;
 				if payment.get().abandoned() {
@@ -812,10 +823,11 @@ impl OutboundPayments {
 					payment.remove();
 				}
 			}
+			is_retryable_now
 		} else {
 			log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0));
 			return
-		}
+		};
 		core::mem::drop(outbounds);
 		let mut retry = if let Some(payment_params_data) = payment_params {
 			let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
@@ -850,6 +862,12 @@ impl OutboundPayments {
 				if let Some(scid) = short_channel_id {
 					retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
 				}
+				if payment_retryable && attempts_remaining && retry.is_some() {
+					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),
+					});
+				}
 				events::Event::PaymentPathFailed {
 					payment_id: Some(*payment_id),
 					payment_hash: payment_hash.clone(),
@@ -869,6 +887,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); }
 	}
 
 	pub(super) fn abandon_payment(&self, payment_id: PaymentId) -> Option<events::Event> {