Account for Path::blinded_tail in channelmanager+outbound_payment

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -45,7 +45,7 @@ use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, No
 #[cfg(any(feature = "_test_utils", test))]
 use crate::ln::features::InvoiceFeatures;
 use crate::routing::gossip::NetworkGraph;
-use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router};
+use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters, Router};
 use crate::routing::scoring::ProbabilisticScorer;
 use crate::ln::msgs;
 use crate::ln::onion_utils;
@@ -2531,7 +2531,7 @@ where
 		// The top-level caller should hold the total_consistency_lock read lock.
 		debug_assert!(self.total_consistency_lock.try_write().is_err());
 
-		log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.hops.first().unwrap().short_channel_id);
+		log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first_hop_scid());
 		let prng_seed = self.entropy_source.get_secure_random_bytes();
 		let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");
 
@@ -2544,7 +2544,7 @@ where
 		let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
 
 		let err: Result<(), _> = loop {
-			let (counterparty_node_id, id) = match self.short_to_chan_info.read().unwrap().get(&path.hops.first().unwrap().short_channel_id) {
+			let (counterparty_node_id, id) = match self.short_to_chan_info.read().unwrap().get(&path.first_hop_scid()) {
 				None => return Err(APIError::ChannelUnavailable{err: "No channel available with first hop!".to_owned()}),
 				Some((cp_id, chan_id)) => (cp_id.clone(), chan_id.clone()),
 			};
@@ -2595,7 +2595,7 @@ where
 			return Ok(());
 		};
 
-		match handle_error!(self, err, path.hops.first().unwrap().pubkey) {
+		match handle_error!(self, err, path.first_hop_node_id()) {
 			Ok(_) => unreachable!(),
 			Err(e) => {
 				Err(APIError::ChannelUnavailable { err: e.err })
@@ -7588,12 +7588,12 @@ where
 				if id_to_peer.get(&monitor.get_funding_txo().0.to_channel_id()).is_none() {
 					for (htlc_source, (htlc, _)) in monitor.get_pending_or_resolved_outbound_htlcs() {
 						if let HTLCSource::OutboundRoute { payment_id, session_priv, path, .. } = htlc_source {
-							if path.hops.is_empty() {
+							if path.len() == 0 {
 								log_error!(args.logger, "Got an empty path for a pending payment");
 								return Err(DecodeError::InvalidValue);
 							}
 
-							let path_amt = path.hops.last().unwrap().fee_msat;
+							let path_amt = path.final_value_msat();
 							let mut session_priv_bytes = [0; 32];
 							session_priv_bytes[..].copy_from_slice(&session_priv[..]);
 							match pending_outbounds.pending_outbound_payments.lock().unwrap().entry(payment_id) {
@@ -7603,7 +7603,7 @@ where
 										if newly_added { "Added" } else { "Had" }, path_amt, log_bytes!(session_priv_bytes), log_bytes!(htlc.payment_hash.0));
 								},
 								hash_map::Entry::Vacant(entry) => {
-									let path_fee = path.hops.get_path_fees();
+									let path_fee = path.fee_msat();
 									entry.insert(PendingOutboundPayment::Retryable {
 										retry_strategy: None,
 										attempts: PaymentAttempts::new(),

lightning/src/ln/outbound_payment.rs

@@ -18,7 +18,7 @@ use crate::events;
 use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
 use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId};
 use crate::ln::onion_utils::HTLCFailReason;
-use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, RoutePath, Router};
+use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router};
 use crate::util::errors::APIError;
 use crate::util::logger::Logger;
 use crate::util::time::Time;
@@ -174,10 +174,9 @@ impl PendingOutboundPayment {
 		if remove_res {
 			if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
 				let path = path.expect("Fulfilling a payment should always come with a path");
-				let path_last_hop = path.hops.last().expect("Outbound payments must have had a valid path");
-				*pending_amt_msat -= path_last_hop.fee_msat;
+				*pending_amt_msat -= path.final_value_msat();
 				if let Some(fee_msat) = pending_fee_msat.as_mut() {
-					*fee_msat -= path.hops.get_path_fees();
+					*fee_msat -= path.fee_msat();
 				}
 			}
 		}
@@ -195,10 +194,9 @@ impl PendingOutboundPayment {
 		};
 		if insert_res {
 			if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
-				let path_last_hop = path.hops.last().expect("Outbound payments must have had a valid path");
-				*pending_amt_msat += path_last_hop.fee_msat;
+				*pending_amt_msat += path.final_value_msat();
 				if let Some(fee_msat) = pending_fee_msat.as_mut() {
-					*fee_msat += path.hops.get_path_fees();
+					*fee_msat += path.fee_msat();
 				}
 			}
 		}
@@ -688,7 +686,7 @@ impl OutboundPayments {
 			}
 		};
 		for path in route.paths.iter() {
-			if path.hops.len() == 0 {
+			if path.len() == 0 {
 				log_error!(logger, "length-0 path in route");
 				self.abandon_payment(payment_id, pending_events);
 				return
@@ -720,7 +718,7 @@ impl OutboundPayments {
 						PendingOutboundPayment::Retryable {
 							total_msat, keysend_preimage, payment_secret, pending_amt_msat, ..
 						} => {
-							let retry_amt_msat: u64 = route.paths.iter().map(|path| path.hops.last().unwrap().fee_msat).sum();
+							let retry_amt_msat = route.get_total_amount();
 							if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 {
 								log_error!(logger, "retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat);
 								abandon_with_entry!(payment);
@@ -824,7 +822,7 @@ impl OutboundPayments {
 				if let APIError::MonitorUpdateInProgress = e { continue }
 				let mut failed_scid = None;
 				if let APIError::ChannelUnavailable { .. } = e {
-					let scid = path.hops[0].short_channel_id;
+					let scid = path.first_hop_scid();
 					failed_scid = Some(scid);
 					route_params.payment_params.previously_failed_channels.push(scid);
 				}
@@ -858,7 +856,7 @@ impl OutboundPayments {
 
 		let payment_hash = probing_cookie_from_id(&payment_id, probing_cookie_secret);
 
-		if path.hops.len() < 2 {
+		if path.len() < 2 {
 			return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
 				err: "No need probing a path with less than two hops".to_string()
 			}))
@@ -946,17 +944,20 @@ impl OutboundPayments {
 		let our_node_id = node_signer.get_node_id(Recipient::Node).unwrap(); // TODO no unwrap
 		let mut path_errs = Vec::with_capacity(route.paths.len());
 		'path_check: for path in route.paths.iter() {
-			if path.hops.len() < 1 || path.hops.len() > 20 {
+			if path.len() < 1 || path.len() > 20 {
 				path_errs.push(Err(APIError::InvalidRoute{err: "Path didn't go anywhere/had bogus size".to_owned()}));
 				continue 'path_check;
 			}
-			for (idx, hop) in path.hops.iter().enumerate() {
-				if idx != path.hops.len() - 1 && hop.pubkey == our_node_id {
-					path_errs.push(Err(APIError::InvalidRoute{err: "Path went through us but wasn't a simple rebalance loop to us".to_owned()}));
-					continue 'path_check;
-				}
+			let we_are_intermed_hop = path.blinded_tail.as_ref().map_or_else(
+				|| path.hops.split_last().map_or(false, |(_, path_prefix)| path_prefix.iter().any(|hop| hop.pubkey == our_node_id)),
+				|tail|
+					(tail.path.introduction_node_id == our_node_id && tail.path.blinded_hops.len() > 1)
+					|| path.hops.iter().any(|hop| hop.pubkey == our_node_id));
+			if we_are_intermed_hop {
+				path_errs.push(Err(APIError::InvalidRoute{err: "Path went through us but wasn't a simple rebalance loop to us".to_owned()}));
+				continue 'path_check;
 			}
-			total_value += path.hops.last().unwrap().fee_msat;
+			total_value += path.final_value_msat();
 			path_errs.push(Ok(()));
 		}
 		if path_errs.iter().any(|e| e.is_err()) {
@@ -1004,7 +1005,7 @@ impl OutboundPayments {
 				has_err = true;
 				has_ok = true;
 			} else if res.is_err() {
-				pending_amt_unsent += path.hops.last().unwrap().fee_msat;
+				pending_amt_unsent += path.final_value_msat();
 			}
 		}
 		if has_err && has_ok {

lightning/src/routing/router.rs

@@ -304,6 +304,16 @@ pub struct Path {
 }
 
 impl Path {
+	/// Returns the total amount of fees paid on this [`Path`].
+	pub fn fee_msat(&self) -> u64 {
+		match &self.blinded_tail {
+			Some(tail) => self.hops.iter().map(|hop| hop.fee_msat).sum::<u64>() + tail.fee_msat,
+			None =>
+				self.hops.split_last().map_or(0,
+					|(_, path_prefix)| path_prefix.iter().map(|hop| hop.fee_msat).sum())
+			}
+	}
+
 	/// Returns the total amount paid on this [`Path`], excluding the fees.
 	pub fn final_value_msat(&self) -> u64 {
 		match &self.blinded_tail {
@@ -317,6 +327,18 @@ impl Path {
 		self.hops.len() + self.blinded_tail.as_ref().map_or(0, |tail| tail.path.blinded_hops.len())
 	}
 
+	pub(crate) fn first_hop_node_id(&self) -> PublicKey {
+		self.hops.first().map_or_else(
+			|| self.blinded_tail.as_ref().expect("Path had 0 hops").path.introduction_node_id,
+			|hop| hop.pubkey)
+	}
+
+	pub(crate) fn first_hop_scid(&self) -> u64 {
+		self.hops.first().map_or_else(
+			|| self.blinded_tail.as_ref().expect("Path had 0 hops").intro_node_scid,
+			|hop| hop.short_channel_id)
+	}
+
 	pub(super) fn node_id_scid_iter(&self) -> impl Iterator<Item=(PublicKey, u64)> + '_ {
 		self.hops.iter().map(|hop| (hop.pubkey, hop.short_channel_id))
 			.chain(self.blinded_tail.as_ref().map(|tail| (tail.path.introduction_node_id, tail.intro_node_scid)))
@@ -339,34 +361,19 @@ pub struct Route {
 	pub payment_params: Option<PaymentParameters>,
 }
 
-pub(crate) trait RoutePath {
-	/// Gets the fees for a given path, excluding any excess paid to the recipient.
-	fn get_path_fees(&self) -> u64;
-}
-impl RoutePath for Vec<RouteHop> {
-	fn get_path_fees(&self) -> u64 {
-		// Do not count last hop of each path since that's the full value of the payment
-		self.split_last().map(|(_, path_prefix)| path_prefix).unwrap_or(&[])
-			.iter().map(|hop| &hop.fee_msat)
-			.sum()
-	}
-}
-
 impl Route {
 	/// Returns the total amount of fees paid on this [`Route`].
 	///
 	/// This doesn't include any extra payment made to the recipient, which can happen in excess of
 	/// the amount passed to [`find_route`]'s `params.final_value_msat`.
 	pub fn get_total_fees(&self) -> u64 {
-		self.paths.iter().map(|path| path.hops.get_path_fees()).sum()
+		self.paths.iter().map(|path| path.fee_msat()).sum()
 	}
 
 	/// Returns the total amount paid on this [`Route`], excluding the fees. Might be more than
 	/// requested if we had to reach htlc_minimum_msat.
 	pub fn get_total_amount(&self) -> u64 {
-		return self.paths.iter()
-			.map(|path| path.hops.split_last().map(|(hop, _)| hop.fee_msat).unwrap_or(0))
-			.sum();
+		self.paths.iter().map(|path| path.final_value_msat()).sum()
 	}
 }