From d7e2ff62203ca5f09cc074bd8b55ca2e09706e03 Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Fri, 14 Jul 2023 11:47:22 +0200
Subject: [PATCH 1/8] Introduce `RouteParameters::max_total_routing_fee_msat`

Currently, users have no means to upper-bound the total fees accruing
when finding a route. Here, we add a corresponding field to
`RouteParameters` which will be used to limit the candidate set during
path finding in the following commits.
---
 lightning-invoice/src/payment.rs          |  9 +--
 lightning/src/ln/blinded_payment_tests.rs | 16 ++---
 lightning/src/ln/channelmanager.rs        |  3 +-
 lightning/src/ln/outbound_payment.rs      | 77 ++++++++++++++++-------
 lightning/src/ln/payment_tests.rs         |  2 +-
 lightning/src/routing/router.rs           | 17 ++++-
 6 files changed, 85 insertions(+), 39 deletions(-)

diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs
index f5b20d87fa1..08be3d54ba5 100644
--- a/lightning-invoice/src/payment.rs
+++ b/lightning-invoice/src/payment.rs
@@ -116,10 +116,7 @@ fn pay_invoice_using_amount<P: Deref>(
 	if let Some(features) = invoice.features() {
 		payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
 	}
-	let route_params = RouteParameters {
-		payment_params,
-		final_value_msat: amount_msats,
-	};
+	let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msats);
 
 	payer.send_payment(payment_hash, recipient_onion, payment_id, route_params, retry_strategy)
 }
@@ -148,7 +145,7 @@ pub fn preflight_probe_invoice<C: AChannelManager>(
 	if let Some(features) = invoice.features() {
 		payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
 	}
-	let route_params = RouteParameters { payment_params, final_value_msat: amount_msat };
+	let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
 	channelmanager.get_cm().send_preflight_probes(route_params, liquidity_limit_multiplier)
 		.map_err(ProbingError::Sending)
@@ -178,7 +175,7 @@ pub fn preflight_probe_zero_value_invoice<C: AChannelManager>(
 	if let Some(features) = invoice.features() {
 		payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
 	}
-	let route_params = RouteParameters { payment_params, final_value_msat: amount_msat };
+	let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
 	channelmanager.get_cm().send_preflight_probes(route_params, liquidity_limit_multiplier)
 		.map_err(ProbingError::Sending)
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index 826eaa86f48..1c9563db0fa 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -47,10 +47,10 @@ fn do_one_hop_blinded_path(success: bool) {
 		nodes[1].node.get_our_node_id(), payee_tlvs, &chanmon_cfgs[1].keys_manager, &secp_ctx
 	).unwrap();
 
-	let route_params = RouteParameters {
-		payment_params: PaymentParameters::blinded(vec![blinded_path]),
-		final_value_msat: amt_msat
-	};
+	let route_params = RouteParameters::from_payment_params_and_value(
+		PaymentParameters::blinded(vec![blinded_path]),
+		amt_msat,
+	);
 	nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(),
 	PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
 	check_added_monitors(&nodes[0], 1);
@@ -90,11 +90,11 @@ fn mpp_to_one_hop_blinded_path() {
 
 	let bolt12_features: Bolt12InvoiceFeatures =
 		channelmanager::provided_invoice_features(&UserConfig::default()).to_context();
-	let route_params = RouteParameters {
-		payment_params: PaymentParameters::blinded(vec![blinded_path])
+	let route_params = RouteParameters::from_payment_params_and_value(
+		PaymentParameters::blinded(vec![blinded_path])
 			.with_bolt12_features(bolt12_features).unwrap(),
-		final_value_msat: amt_msat,
-	};
+		amt_msat,
+	);
 	nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
 	check_added_monitors(&nodes[0], 2);
 
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 0cadbd41a29..c76ca58f59f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3570,7 +3570,7 @@ where
 		let payment_params =
 			PaymentParameters::from_node_id(node_id, final_cltv_expiry_delta);
 
-		let route_params = RouteParameters { payment_params, final_value_msat: amount_msat };
+		let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
 		self.send_preflight_probes(route_params, liquidity_limit_multiplier)
 	}
@@ -9559,6 +9559,7 @@ where
 										pending_fee_msat: Some(path_fee),
 										total_msat: path_amt,
 										starting_block_height: best_block_height,
+										remaining_max_total_routing_fee_msat: None, // only used for retries, and we'll never retry on startup
 									});
 									log_info!(args.logger, "Added a pending payment for {} msat with payment hash {} for path with session priv {}",
 										path_amt, &htlc.payment_hash,  log_bytes!(session_priv_bytes));
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 023412e1afb..2f1e3b0dd0e 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -54,10 +54,14 @@ pub(crate) enum PendingOutboundPayment {
 	AwaitingInvoice {
 		timer_ticks_without_response: u8,
 		retry_strategy: Retry,
+		max_total_routing_fee_msat: Option<u64>,
 	},
 	InvoiceReceived {
 		payment_hash: PaymentHash,
 		retry_strategy: Retry,
+		// Note this field is currently just replicated from AwaitingInvoice but not actually
+		// used anywhere.
+		max_total_routing_fee_msat: Option<u64>,
 	},
 	Retryable {
 		retry_strategy: Option<Retry>,
@@ -76,6 +80,7 @@ pub(crate) enum PendingOutboundPayment {
 		total_msat: u64,
 		/// Our best known block height at the time this payment was initiated.
 		starting_block_height: u32,
+		remaining_max_total_routing_fee_msat: Option<u64>,
 	},
 	/// When a pending payment is fulfilled, we continue tracking it until all pending HTLCs have
 	/// been resolved. This ensures we don't look up pending payments in ChannelMonitors on restart
@@ -731,12 +736,15 @@ impl OutboundPayments {
 		SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
 	{
 		let payment_hash = invoice.payment_hash();
+		let mut max_total_routing_fee_msat = None;
 		match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
 			hash_map::Entry::Occupied(entry) => match entry.get() {
-				PendingOutboundPayment::AwaitingInvoice { retry_strategy, .. } => {
+				PendingOutboundPayment::AwaitingInvoice { retry_strategy, max_total_routing_fee_msat: max_total_fee, .. } => {
+					max_total_routing_fee_msat = *max_total_fee;
 					*entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
 						payment_hash,
 						retry_strategy: *retry_strategy,
+						max_total_routing_fee_msat,
 					};
 				},
 				_ => return Err(Bolt12PaymentError::DuplicateInvoice),
@@ -747,6 +755,7 @@ impl OutboundPayments {
 		let route_params = RouteParameters {
 			payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
 			final_value_msat: invoice.amount_msats(),
+			max_total_routing_fee_msat,
 		};
 
 		self.find_route_and_send_payment(
@@ -779,11 +788,12 @@ impl OutboundPayments {
 			let mut retry_id_route_params = None;
 			for (pmt_id, pmt) in outbounds.iter_mut() {
 				if pmt.is_auto_retryable_now() {
-					if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, payment_params: Some(params), payment_hash, .. } = pmt {
+					if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, payment_params: Some(params), payment_hash, remaining_max_total_routing_fee_msat, .. } = pmt {
 						if pending_amt_msat < total_msat {
 							retry_id_route_params = Some((*payment_hash, *pmt_id, RouteParameters {
 								final_value_msat: *total_msat - *pending_amt_msat,
 								payment_params: params.clone(),
+								max_total_routing_fee_msat: *remaining_max_total_routing_fee_msat,
 							}));
 							break
 						}
@@ -987,7 +997,7 @@ impl OutboundPayments {
 							log_error!(logger, "Payment not yet sent");
 							return
 						},
-						PendingOutboundPayment::InvoiceReceived { payment_hash, retry_strategy } => {
+						PendingOutboundPayment::InvoiceReceived { payment_hash, retry_strategy, .. } => {
 							let total_amount = route_params.final_value_msat;
 							let recipient_onion = RecipientOnionFields {
 								payment_secret: None,
@@ -1207,6 +1217,8 @@ impl OutboundPayments {
 			custom_tlvs: recipient_onion.custom_tlvs,
 			starting_block_height: best_block_height,
 			total_msat: route.get_total_amount(),
+			remaining_max_total_routing_fee_msat:
+				route.route_params.as_ref().and_then(|p| p.max_total_routing_fee_msat),
 		};
 
 		for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
@@ -1218,7 +1230,7 @@ impl OutboundPayments {
 
 	#[allow(unused)]
 	pub(super) fn add_new_awaiting_invoice(
-		&self, payment_id: PaymentId, retry_strategy: Retry
+		&self, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option<u64>
 	) -> Result<(), ()> {
 		let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
 		match pending_outbounds.entry(payment_id) {
@@ -1227,6 +1239,7 @@ impl OutboundPayments {
 				entry.insert(PendingOutboundPayment::AwaitingInvoice {
 					timer_ticks_without_response: 0,
 					retry_strategy,
+					max_total_routing_fee_msat,
 				});
 
 				Ok(())
@@ -1328,8 +1341,9 @@ impl OutboundPayments {
 				failed_paths_retry: if pending_amt_unsent != 0 {
 					if let Some(payment_params) = route.route_params.as_ref().map(|p| p.payment_params.clone()) {
 						Some(RouteParameters {
-							payment_params: payment_params,
+							payment_params,
 							final_value_msat: pending_amt_unsent,
+							max_total_routing_fee_msat: None,
 						})
 					} else { None }
 				} else { None },
@@ -1689,6 +1703,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(8, pending_amt_msat, required),
 		(9, custom_tlvs, optional_vec),
 		(10, starting_block_height, required),
+		(11, remaining_max_total_routing_fee_msat, option),
 		(not_written, retry_strategy, (static_value, None)),
 		(not_written, attempts, (static_value, PaymentAttempts::new())),
 	},
@@ -1700,10 +1715,12 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 	(5, AwaitingInvoice) => {
 		(0, timer_ticks_without_response, required),
 		(2, retry_strategy, required),
+		(4, max_total_routing_fee_msat, option),
 	},
 	(7, InvoiceReceived) => {
 		(0, payment_hash, required),
 		(2, retry_strategy, required),
+		(4, max_total_routing_fee_msat, option),
 	},
 );
 
@@ -1926,7 +1943,9 @@ mod tests {
 		let payment_id = PaymentId([0; 32]);
 
 		assert!(!outbound_payments.has_pending_payments());
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
 		for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS {
@@ -1944,10 +1963,15 @@ mod tests {
 		);
 		assert!(pending_events.lock().unwrap().is_empty());
 
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_err());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None)
+				.is_err()
+		);
 	}
 
 	#[test]
@@ -1957,7 +1981,9 @@ mod tests {
 		let payment_id = PaymentId([0; 32]);
 
 		assert!(!outbound_payments.has_pending_payments());
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
 		outbound_payments.abandon_payment(
@@ -1985,7 +2011,9 @@ mod tests {
 		let outbound_payments = OutboundPayments::new();
 		let payment_id = PaymentId([0; 32]);
 
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
 		let created_at = now() - DEFAULT_RELATIVE_EXPIRY;
@@ -2031,7 +2059,9 @@ mod tests {
 		let outbound_payments = OutboundPayments::new();
 		let payment_id = PaymentId([0; 32]);
 
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
 		let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
@@ -2045,10 +2075,10 @@ mod tests {
 			.sign(recipient_sign).unwrap();
 
 		router.expect_find_route(
-			RouteParameters {
-				payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
-				final_value_msat: invoice.amount_msats(),
-			},
+			RouteParameters::from_payment_params_and_value(
+				PaymentParameters::from_bolt12_invoice(&invoice),
+				invoice.amount_msats(),
+			),
 			Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }),
 		);
 
@@ -2084,7 +2114,9 @@ mod tests {
 		let outbound_payments = OutboundPayments::new();
 		let payment_id = PaymentId([0; 32]);
 
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
 		let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
@@ -2097,10 +2129,10 @@ mod tests {
 			.build().unwrap()
 			.sign(recipient_sign).unwrap();
 
-		let route_params = RouteParameters {
-			payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
-			final_value_msat: invoice.amount_msats(),
-		};
+		let route_params = RouteParameters::from_payment_params_and_value(
+			PaymentParameters::from_bolt12_invoice(&invoice),
+			invoice.amount_msats(),
+		);
 		router.expect_find_route(
 			route_params.clone(), Ok(Route { paths: vec![], route_params: Some(route_params) })
 		);
@@ -2150,6 +2182,7 @@ mod tests {
 		let route_params = RouteParameters {
 			payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
 			final_value_msat: invoice.amount_msats(),
+			max_total_routing_fee_msat: Some(1234),
 		};
 		router.expect_find_route(
 			route_params.clone(),
@@ -2185,7 +2218,9 @@ mod tests {
 		assert!(!outbound_payments.has_pending_payments());
 		assert!(pending_events.lock().unwrap().is_empty());
 
-		assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+		assert!(
+			outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), Some(1234)).is_ok()
+		);
 		assert!(outbound_payments.has_pending_payments());
 
 		assert_eq!(
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 3def4e3629b..f74fab255e6 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -1337,7 +1337,7 @@ fn preflight_probes_yield_event_and_skip() {
 	let mut payment_params = PaymentParameters::from_node_id(nodes[4].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_bolt11_features(invoice_features).unwrap();
 
-	let route_params = RouteParameters { payment_params, final_value_msat: 80_000_000 };
+	let route_params = RouteParameters::from_payment_params_and_value(payment_params, 80_000_000);
 	let res = nodes[0].node.send_preflight_probes(route_params, None).unwrap();
 
 	// We check that only one probe was sent, the other one was skipped due to limited liquidity.
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 1758cbbf9c3..dacc137ae8f 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -409,6 +409,7 @@ impl Writeable for Route {
 			(1, self.route_params.as_ref().map(|p| &p.payment_params), option),
 			(2, blinded_tails, optional_vec),
 			(3, self.route_params.as_ref().map(|p| p.final_value_msat), option),
+			(5, self.route_params.as_ref().map(|p| p.max_total_routing_fee_msat), option),
 		});
 		Ok(())
 	}
@@ -436,6 +437,7 @@ impl Readable for Route {
 			(1, payment_params, (option: ReadableArgs, min_final_cltv_expiry_delta)),
 			(2, blinded_tails, optional_vec),
 			(3, final_value_msat, option),
+			(5, max_total_routing_fee_msat, option)
 		});
 		let blinded_tails = blinded_tails.unwrap_or(Vec::new());
 		if blinded_tails.len() != 0 {
@@ -448,7 +450,7 @@ impl Readable for Route {
 		// If we previously wrote the corresponding fields, reconstruct RouteParameters.
 		let route_params = match (payment_params, final_value_msat) {
 			(Some(payment_params), Some(final_value_msat)) => {
-				Some(RouteParameters { payment_params, final_value_msat })
+				Some(RouteParameters { payment_params, final_value_msat, max_total_routing_fee_msat })
 			}
 			_ => None,
 		};
@@ -467,12 +469,20 @@ pub struct RouteParameters {
 
 	/// The amount in msats sent on the failed payment path.
 	pub final_value_msat: u64,
+
+	/// The maximum total fees, in millisatoshi, that may accrue during route finding.
+	///
+	/// This limit also applies to the total fees that may arise while retrying failed payment
+	/// paths.
+	///
+	/// Default value: `None`
+	pub max_total_routing_fee_msat: Option<u64>,
 }
 
 impl RouteParameters {
 	/// Constructs [`RouteParameters`] from the given [`PaymentParameters`] and a payment amount.
 	pub fn from_payment_params_and_value(payment_params: PaymentParameters, final_value_msat: u64) -> Self {
-		Self { payment_params, final_value_msat }
+		Self { payment_params, final_value_msat, max_total_routing_fee_msat: None }
 	}
 }
 
@@ -480,6 +490,7 @@ impl Writeable for RouteParameters {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
 		write_tlv_fields!(writer, {
 			(0, self.payment_params, required),
+			(1, self.max_total_routing_fee_msat, option),
 			(2, self.final_value_msat, required),
 			// LDK versions prior to 0.0.114 had the `final_cltv_expiry_delta` parameter in
 			// `RouteParameters` directly. For compatibility, we write it here.
@@ -493,6 +504,7 @@ impl Readable for RouteParameters {
 	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
 		_init_and_read_len_prefixed_tlv_fields!(reader, {
 			(0, payment_params, (required: ReadableArgs, 0)),
+			(1, max_total_routing_fee_msat, option),
 			(2, final_value_msat, required),
 			(4, final_cltv_delta, option),
 		});
@@ -505,6 +517,7 @@ impl Readable for RouteParameters {
 		Ok(Self {
 			payment_params,
 			final_value_msat: final_value_msat.0.unwrap(),
+			max_total_routing_fee_msat,
 		})
 	}
 }

From ca40d8a6fb5b220db443748fa90dfc1bd8e7d99b Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Fri, 14 Jul 2023 13:25:33 +0200
Subject: [PATCH 2/8] Consider `RouteParameters::max_total_routing_fee_msat` in
 `get_route`

We exclude any candidate hops if we find that using them would let the
aggregated path routing fees exceed `max_total_routing_fee_msat`.

Moreover, we return an error if the aggregated fees over all paths of
the selected route would surpass `max_total_routing_fee_msat`.
---
 lightning/src/routing/router.rs | 186 +++++++++++++++++---------------
 1 file changed, 102 insertions(+), 84 deletions(-)

diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index dacc137ae8f..340d5e5c107 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -1693,6 +1693,7 @@ where L::Target: Logger {
 	let mut num_ignored_path_length_limit = 0;
 	let mut num_ignored_cltv_delta_limit = 0;
 	let mut num_ignored_previously_failed = 0;
+	let mut num_ignored_total_fee_limit = 0;
 
 	macro_rules! add_entry {
 		// Adds entry which goes from $src_node_id to $dest_node_id over the $candidate hop.
@@ -1854,89 +1855,98 @@ where L::Target: Logger {
 								total_fee_msat = total_fee_msat.saturating_add(hop_use_fee_msat);
 							}
 
-							let channel_usage = ChannelUsage {
-								amount_msat: amount_to_transfer_over_msat,
-								inflight_htlc_msat: used_liquidity_msat,
-								effective_capacity,
-							};
-							let channel_penalty_msat = scid_opt.map_or(0,
-								|scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
-									channel_usage, score_params));
-							let path_penalty_msat = $next_hops_path_penalty_msat
-								.saturating_add(channel_penalty_msat);
-							let new_graph_node = RouteGraphNode {
-								node_id: $src_node_id,
-								lowest_fee_to_node: total_fee_msat,
-								total_cltv_delta: hop_total_cltv_delta,
-								value_contribution_msat,
-								path_htlc_minimum_msat,
-								path_penalty_msat,
-								path_length_to_node,
-							};
-
-							// Update the way of reaching $src_node_id with the given short_channel_id (from $dest_node_id),
-							// if this way is cheaper than the already known
-							// (considering the cost to "reach" this channel from the route destination,
-							// the cost of using this channel,
-							// and the cost of routing to the source node of this channel).
-							// Also, consider that htlc_minimum_msat_difference, because we might end up
-							// paying it. Consider the following exploit:
-							// we use 2 paths to transfer 1.5 BTC. One of them is 0-fee normal 1 BTC path,
-							// and for the other one we picked a 1sat-fee path with htlc_minimum_msat of
-							// 1 BTC. Now, since the latter is more expensive, we gonna try to cut it
-							// by 0.5 BTC, but then match htlc_minimum_msat by paying a fee of 0.5 BTC
-							// to this channel.
-							// Ideally the scoring could be smarter (e.g. 0.5*htlc_minimum_msat here),
-							// but it may require additional tracking - we don't want to double-count
-							// the fees included in $next_hops_path_htlc_minimum_msat, but also
-							// can't use something that may decrease on future hops.
-							let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat)
-								.saturating_add(old_entry.path_penalty_msat);
-							let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat)
-								.saturating_add(path_penalty_msat);
-
-							if !old_entry.was_processed && new_cost < old_cost {
-								targets.push(new_graph_node);
-								old_entry.next_hops_fee_msat = $next_hops_fee_msat;
-								old_entry.hop_use_fee_msat = hop_use_fee_msat;
-								old_entry.total_fee_msat = total_fee_msat;
-								old_entry.node_id = $dest_node_id.clone();
-								old_entry.candidate = $candidate.clone();
-								old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel
-								old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat;
-								old_entry.path_penalty_msat = path_penalty_msat;
-								#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
-								{
-									old_entry.value_contribution_msat = value_contribution_msat;
+							// Ignore hops if augmenting the current path to them would put us over `max_total_routing_fee_msat`
+							let max_total_routing_fee_msat = route_params.max_total_routing_fee_msat.unwrap_or(u64::max_value());
+							if total_fee_msat > max_total_routing_fee_msat {
+								if should_log_candidate {
+									log_trace!(logger, "Ignoring {} due to exceeding max total routing fee limit.", LoggedCandidateHop(&$candidate));
 								}
-								did_add_update_path_to_src_node = Some(value_contribution_msat);
-							} else if old_entry.was_processed && new_cost < old_cost {
-								#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
-								{
-									// If we're skipping processing a node which was previously
-									// processed even though we found another path to it with a
-									// cheaper fee, check that it was because the second path we
-									// found (which we are processing now) has a lower value
-									// contribution due to an HTLC minimum limit.
-									//
-									// e.g. take a graph with two paths from node 1 to node 2, one
-									// through channel A, and one through channel B. Channel A and
-									// B are both in the to-process heap, with their scores set by
-									// a higher htlc_minimum than fee.
-									// Channel A is processed first, and the channels onwards from
-									// node 1 are added to the to-process heap. Thereafter, we pop
-									// Channel B off of the heap, note that it has a much more
-									// restrictive htlc_maximum_msat, and recalculate the fees for
-									// all of node 1's channels using the new, reduced, amount.
-									//
-									// This would be bogus - we'd be selecting a higher-fee path
-									// with a lower htlc_maximum_msat instead of the one we'd
-									// already decided to use.
-									debug_assert!(path_htlc_minimum_msat < old_entry.path_htlc_minimum_msat);
-									debug_assert!(
-										value_contribution_msat + path_penalty_msat <
-										old_entry.value_contribution_msat + old_entry.path_penalty_msat
-									);
+								num_ignored_total_fee_limit += 1;
+							} else {
+								let channel_usage = ChannelUsage {
+									amount_msat: amount_to_transfer_over_msat,
+									inflight_htlc_msat: used_liquidity_msat,
+									effective_capacity,
+								};
+								let channel_penalty_msat = scid_opt.map_or(0,
+									|scid| scorer.channel_penalty_msat(scid, &$src_node_id, &$dest_node_id,
+										channel_usage, score_params));
+								let path_penalty_msat = $next_hops_path_penalty_msat
+									.saturating_add(channel_penalty_msat);
+								let new_graph_node = RouteGraphNode {
+									node_id: $src_node_id,
+									lowest_fee_to_node: total_fee_msat,
+									total_cltv_delta: hop_total_cltv_delta,
+									value_contribution_msat,
+									path_htlc_minimum_msat,
+									path_penalty_msat,
+									path_length_to_node,
+								};
+
+								// Update the way of reaching $src_node_id with the given short_channel_id (from $dest_node_id),
+								// if this way is cheaper than the already known
+								// (considering the cost to "reach" this channel from the route destination,
+								// the cost of using this channel,
+								// and the cost of routing to the source node of this channel).
+								// Also, consider that htlc_minimum_msat_difference, because we might end up
+								// paying it. Consider the following exploit:
+								// we use 2 paths to transfer 1.5 BTC. One of them is 0-fee normal 1 BTC path,
+								// and for the other one we picked a 1sat-fee path with htlc_minimum_msat of
+								// 1 BTC. Now, since the latter is more expensive, we gonna try to cut it
+								// by 0.5 BTC, but then match htlc_minimum_msat by paying a fee of 0.5 BTC
+								// to this channel.
+								// Ideally the scoring could be smarter (e.g. 0.5*htlc_minimum_msat here),
+								// but it may require additional tracking - we don't want to double-count
+								// the fees included in $next_hops_path_htlc_minimum_msat, but also
+								// can't use something that may decrease on future hops.
+								let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat)
+									.saturating_add(old_entry.path_penalty_msat);
+								let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat)
+									.saturating_add(path_penalty_msat);
+
+								if !old_entry.was_processed && new_cost < old_cost {
+									targets.push(new_graph_node);
+									old_entry.next_hops_fee_msat = $next_hops_fee_msat;
+									old_entry.hop_use_fee_msat = hop_use_fee_msat;
+									old_entry.total_fee_msat = total_fee_msat;
+									old_entry.node_id = $dest_node_id.clone();
+									old_entry.candidate = $candidate.clone();
+									old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel
+									old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat;
+									old_entry.path_penalty_msat = path_penalty_msat;
+									#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
+									{
+										old_entry.value_contribution_msat = value_contribution_msat;
+									}
+									did_add_update_path_to_src_node = Some(value_contribution_msat);
+								} else if old_entry.was_processed && new_cost < old_cost {
+									#[cfg(all(not(ldk_bench), any(test, fuzzing)))]
+									{
+										// If we're skipping processing a node which was previously
+										// processed even though we found another path to it with a
+										// cheaper fee, check that it was because the second path we
+										// found (which we are processing now) has a lower value
+										// contribution due to an HTLC minimum limit.
+										//
+										// e.g. take a graph with two paths from node 1 to node 2, one
+										// through channel A, and one through channel B. Channel A and
+										// B are both in the to-process heap, with their scores set by
+										// a higher htlc_minimum than fee.
+										// Channel A is processed first, and the channels onwards from
+										// node 1 are added to the to-process heap. Thereafter, we pop
+										// Channel B off of the heap, note that it has a much more
+										// restrictive htlc_maximum_msat, and recalculate the fees for
+										// all of node 1's channels using the new, reduced, amount.
+										//
+										// This would be bogus - we'd be selecting a higher-fee path
+										// with a lower htlc_maximum_msat instead of the one we'd
+										// already decided to use.
+										debug_assert!(path_htlc_minimum_msat < old_entry.path_htlc_minimum_msat);
+										debug_assert!(
+											value_contribution_msat + path_penalty_msat <
+											old_entry.value_contribution_msat + old_entry.path_penalty_msat
+										);
+									}
 								}
 							}
 						}
@@ -2407,9 +2417,9 @@ where L::Target: Logger {
 	}
 
 	let num_ignored_total = num_ignored_value_contribution + num_ignored_path_length_limit +
-		num_ignored_cltv_delta_limit + num_ignored_previously_failed;
+		num_ignored_cltv_delta_limit + num_ignored_previously_failed + num_ignored_total_fee_limit;
 	if num_ignored_total > 0 {
-		log_trace!(logger, "Ignored {} candidate hops due to insufficient value contribution, {} due to path length limit, {} due to CLTV delta limit, {} due to previous payment failure. Total: {} ignored candidates.", num_ignored_value_contribution, num_ignored_path_length_limit, num_ignored_cltv_delta_limit, num_ignored_previously_failed, num_ignored_total);
+		log_trace!(logger, "Ignored {} candidate hops due to insufficient value contribution, {} due to path length limit, {} due to CLTV delta limit, {} due to previous payment failure, {} due to maximum total fee limit. Total: {} ignored candidates.", num_ignored_value_contribution, num_ignored_path_length_limit, num_ignored_cltv_delta_limit, num_ignored_previously_failed, num_ignored_total_fee_limit, num_ignored_total);
 	}
 
 	// Step (5).
@@ -2551,6 +2561,14 @@ where L::Target: Logger {
 	// Make sure we would never create a route with more paths than we allow.
 	debug_assert!(paths.len() <= payment_params.max_path_count.into());
 
+	// Make sure we would never create a route whose total fees exceed max_total_routing_fee_msat.
+	if let Some(max_total_routing_fee_msat) = route_params.max_total_routing_fee_msat {
+		if paths.iter().map(|p| p.fee_msat()).sum::<u64>() > max_total_routing_fee_msat {
+			return Err(LightningError{err: format!("Failed to find route that adheres to the maximum total fee limit of {}msat",
+				max_total_routing_fee_msat), action: ErrorAction::IgnoreError});
+		}
+	}
+
 	if let Some(node_features) = payment_params.payee.node_features() {
 		for path in paths.iter_mut() {
 			path.hops.last_mut().unwrap().node_features = node_features.clone();

From b1a878fe81e6abe129c47f1aaea103278cf207a3 Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Fri, 14 Jul 2023 16:46:52 +0200
Subject: [PATCH 3/8] Test we adhere to `max_total_routing_fee_msat`

---
 lightning/src/routing/router.rs | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 340d5e5c107..c680d57a6dc 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -5269,9 +5269,21 @@ mod tests {
 			} else { panic!(); }
 		}
 
+		{
+			// Attempt to route while setting max_total_routing_fee_msat to 149_999 results in a failure.
+			let route_params = RouteParameters { payment_params: payment_params.clone(), final_value_msat: 200_000,
+				max_total_routing_fee_msat: Some(149_999) };
+			if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
+				&our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
+				&scorer, &(), &random_seed_bytes) {
+					assert_eq!(err, "Failed to find a sufficient route to the given destination");
+			} else { panic!(); }
+		}
+
 		{
 			// Now, attempt to route 200 sats (exact amount we can route).
-			let route_params = RouteParameters::from_payment_params_and_value(payment_params, 200_000);
+			let route_params = RouteParameters { payment_params: payment_params.clone(), final_value_msat: 200_000,
+				max_total_routing_fee_msat: Some(150_000) };
 			let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
 				Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
 			assert_eq!(route.paths.len(), 2);

From ac57163895b93bb2c0999aed50aca56a15fe4e33 Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Tue, 18 Jul 2023 15:46:06 +0200
Subject: [PATCH 4/8] Account for leftover fee budget when retrying
 `PartialFailure`s

---
 lightning/src/ln/outbound_payment.rs | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 2f1e3b0dd0e..e06ba8d8539 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1322,14 +1322,19 @@ impl OutboundPayments {
 		let mut has_ok = false;
 		let mut has_err = false;
 		let mut pending_amt_unsent = 0;
+		let mut total_ok_fees_msat = 0;
 		for (res, path) in results.iter().zip(route.paths.iter()) {
-			if res.is_ok() { has_ok = true; }
+			if res.is_ok() {
+				has_ok = true;
+				total_ok_fees_msat += path.fee_msat();
+			}
 			if res.is_err() { has_err = true; }
 			if let &Err(APIError::MonitorUpdateInProgress) = res {
 				// MonitorUpdateInProgress is inherently unsafe to retry, so we call it a
 				// PartialFailure.
 				has_err = true;
 				has_ok = true;
+				total_ok_fees_msat += path.fee_msat();
 			} else if res.is_err() {
 				pending_amt_unsent += path.final_value_msat();
 			}
@@ -1339,12 +1344,15 @@ impl OutboundPayments {
 				results,
 				payment_id,
 				failed_paths_retry: if pending_amt_unsent != 0 {
-					if let Some(payment_params) = route.route_params.as_ref().map(|p| p.payment_params.clone()) {
-						Some(RouteParameters {
-							payment_params,
-							final_value_msat: pending_amt_unsent,
-							max_total_routing_fee_msat: None,
-						})
+					if let Some(route_params) = &route.route_params {
+						let mut route_params = route_params.clone();
+						// We calculate the leftover fee budget we're allowed to spend by
+						// subtracting the used fee from the total fee budget.
+						route_params.max_total_routing_fee_msat = route_params
+							.max_total_routing_fee_msat.map(|m| m.saturating_sub(total_ok_fees_msat));
+						route_params.final_value_msat = pending_amt_unsent;
+
+						Some(route_params)
 					} else { None }
 				} else { None },
 			})

From 649144d25fb9e5015969914caaa5362b64c51bfb Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Mon, 4 Sep 2023 14:51:42 +0200
Subject: [PATCH 5/8] Account for leftover fee budget when retrying via
 `check_retry_payment`

---
 lightning/src/ln/outbound_payment.rs | 34 ++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index e06ba8d8539..5859cbe54aa 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -215,11 +215,19 @@ impl PendingOutboundPayment {
 				PendingOutboundPayment::InvoiceReceived { .. } => { debug_assert!(false); false },
 		};
 		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");
+			if let PendingOutboundPayment::Retryable {
+				ref mut pending_amt_msat, ref mut pending_fee_msat,
+				ref mut remaining_max_total_routing_fee_msat, ..
+			} = self {
+				let path = path.expect("Removing a failed payment should always come with a path");
 				*pending_amt_msat -= path.final_value_msat();
+				let path_fee_msat = path.fee_msat();
 				if let Some(fee_msat) = pending_fee_msat.as_mut() {
-					*fee_msat -= path.fee_msat();
+					*fee_msat -= path_fee_msat;
+				}
+
+				if let Some(max_total_routing_fee_msat) = remaining_max_total_routing_fee_msat.as_mut() {
+					*max_total_routing_fee_msat = max_total_routing_fee_msat.saturating_add(path_fee_msat);
 				}
 			}
 		}
@@ -238,11 +246,19 @@ impl PendingOutboundPayment {
 			PendingOutboundPayment::Abandoned { .. } => false,
 		};
 		if insert_res {
-			if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self {
-				*pending_amt_msat += path.final_value_msat();
-				if let Some(fee_msat) = pending_fee_msat.as_mut() {
-					*fee_msat += path.fee_msat();
-				}
+			if let PendingOutboundPayment::Retryable {
+				ref mut pending_amt_msat, ref mut pending_fee_msat,
+				ref mut remaining_max_total_routing_fee_msat, .. 
+			} = self {
+					*pending_amt_msat += path.final_value_msat();
+					let path_fee_msat = path.fee_msat();
+					if let Some(fee_msat) = pending_fee_msat.as_mut() {
+						*fee_msat += path_fee_msat;
+					}
+
+					if let Some(max_total_routing_fee_msat) = remaining_max_total_routing_fee_msat.as_mut() {
+						*max_total_routing_fee_msat = max_total_routing_fee_msat.saturating_sub(path_fee_msat);
+					}
 			}
 		}
 		insert_res
@@ -1573,7 +1589,7 @@ impl OutboundPayments {
 				is_retryable_now = false;
 			}
 			if payment.get().remaining_parts() == 0 {
-				if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. }= payment.get() {
+				if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = payment.get() {
 					if !payment_is_probe {
 						full_failure_ev = Some(events::Event::PaymentFailed {
 							payment_id: *payment_id,

From 26b515c13cccd1d027e67d0c65d69321d235ce40 Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Wed, 20 Sep 2023 15:32:37 +0200
Subject: [PATCH 6/8] Check `max_total_routing_fee` is reduced in `mpp_retry`
 test

We check that the `RouteParameters::max_total_routing_fee` field is reduced accordingly
to our previously used fees.
---
 lightning/src/ln/functional_test_utils.rs | 6 +++++-
 lightning/src/ln/payment_tests.rs         | 9 ++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index f6684485dba..8f27cf1ad53 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1878,7 +1878,11 @@ macro_rules! get_route_and_payment_hash {
 		$crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value)
 	}};
 	($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr) => {{
-		let route_params = $crate::routing::router::RouteParameters::from_payment_params_and_value($payment_params, $recv_value);
+		$crate::get_route_and_payment_hash!($send_node, $recv_node, $payment_params, $recv_value, None)
+	}};
+	($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $max_total_routing_fee_msat: expr) => {{
+		let mut route_params = $crate::routing::router::RouteParameters::from_payment_params_and_value($payment_params, $recv_value);
+		route_params.max_total_routing_fee_msat = $max_total_routing_fee_msat;
 		let (payment_preimage, payment_hash, payment_secret) =
 			$crate::ln::functional_test_utils::get_payment_preimage_hash(&$recv_node, Some($recv_value), None);
 		let route = $crate::ln::functional_test_utils::get_route(&$send_node, &route_params);
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index f74fab255e6..1bd4a8fa91f 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -83,7 +83,11 @@ fn mpp_retry() {
 	send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000);
 
 	let amt_msat = 1_000_000;
-	let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[3], amt_msat);
+	let max_total_routing_fee_msat = 50_000;
+	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV)
+		.with_bolt11_features(nodes[3].node.invoice_features()).unwrap();
+	let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(
+		nodes[0], nodes[3], payment_params, amt_msat, Some(max_total_routing_fee_msat));
 	let path = route.paths[0].clone();
 	route.paths.push(path);
 	route.paths[0].hops[0].pubkey = nodes[1].node.get_our_node_id();
@@ -150,6 +154,9 @@ fn mpp_retry() {
 	route.paths.remove(0);
 	route_params.final_value_msat = 1_000_000;
 	route_params.payment_params.previously_failed_channels.push(chan_4_update.contents.short_channel_id);
+	// Check the remaining max total routing fee for the second attempt is 50_000 - 1_000 msat fee
+	// used by the first path
+	route_params.max_total_routing_fee_msat = Some(max_total_routing_fee_msat - 1_000);
 	nodes[0].router.expect_find_route(route_params, Ok(route));
 	nodes[0].node.process_pending_htlc_forwards();
 	check_added_monitors!(nodes[0], 1);

From 78959439537266a0f6aa478b8274bb11e36282db Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Tue, 26 Sep 2023 10:48:33 +0200
Subject: [PATCH 7/8] Check `max_total_routing_fee` is accounted for in
 `test_threaded_payment_retries`

---
 lightning/src/ln/payment_tests.rs | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 1bd4a8fa91f..2833ba8e809 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -3135,7 +3135,9 @@ fn test_threaded_payment_retries() {
 	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_expiry_time(payment_expiry_secs as u64)
 		.with_bolt11_features(invoice_features).unwrap();
-	let mut route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat);
+	let mut route_params = RouteParameters {
+		payment_params, final_value_msat: amt_msat, max_total_routing_fee_msat: Some(500_000),
+	};
 
 	let mut route = Route {
 		paths: vec![
@@ -3174,9 +3176,11 @@ fn test_threaded_payment_retries() {
 				maybe_announced_channel: true,
 			}], blinded_tail: None }
 		],
-		route_params: Some(RouteParameters::from_payment_params_and_value(
-			PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV),
-			amt_msat - amt_msat / 1000)),
+		route_params: Some(RouteParameters {
+			payment_params: PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV),
+			final_value_msat: amt_msat - amt_msat / 1000,
+			max_total_routing_fee_msat: Some(500_000),
+		}),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 
@@ -3234,6 +3238,7 @@ fn test_threaded_payment_retries() {
 		let mut new_route_params = route_params.clone();
 		previously_failed_channels.push(route.paths[0].hops[1].short_channel_id);
 		new_route_params.payment_params.previously_failed_channels = previously_failed_channels.clone();
+		new_route_params.max_total_routing_fee_msat.as_mut().map(|m| *m -= 100_000);
 		route.paths[0].hops[1].short_channel_id += 1;
 		nodes[0].router.expect_find_route(new_route_params, Ok(route.clone()));
 

From 6765767cbc2b8e4a8fbe04c4a2b2e982809f5604 Mon Sep 17 00:00:00 2001
From: Elias Rohrer <dev@tnull.de>
Date: Tue, 26 Sep 2023 15:30:39 +0200
Subject: [PATCH 8/8] Test `max_total_routing_fee_msat` handling when retrying
 overpaid paths

We setup an MPP scenario with two paths in which we need to overpay to
reach `htlc_minimum_msat`. We then fail the overpaid path and check that
on retry our `max_total_routing_fee_msat` only accounts for the path
fees, but not for the fees overpaid in the first attempt.
---
 lightning/src/ln/payment_tests.rs | 136 ++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 2833ba8e809..94c3bf668f7 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -166,6 +166,142 @@ fn mpp_retry() {
 	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
 }
 
+#[test]
+fn mpp_retry_overpay() {
+	// We create an MPP scenario with two paths in which we need to overpay to reach
+	// htlc_minimum_msat. We then fail the overpaid path and check that on retry our
+	// max_total_routing_fee_msat only accounts for the path's fees, but not for the fees overpaid
+	// in the first attempt.
+	let chanmon_cfgs = create_chanmon_cfgs(4);
+	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+	let mut user_config = test_default_channel_config();
+	user_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
+	let mut limited_config_1 = user_config.clone();
+	limited_config_1.channel_handshake_config.our_htlc_minimum_msat = 35_000_000;
+	let mut limited_config_2 = user_config.clone();
+	limited_config_2.channel_handshake_config.our_htlc_minimum_msat = 34_500_000;
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs,
+		&[Some(user_config), Some(limited_config_1), Some(limited_config_2), Some(user_config)]);
+	let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+	let (chan_1_update, _, _, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 40_000, 0);
+	let (chan_2_update, _, _, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 40_000, 0);
+	let (_chan_3_update, _, _, _) = create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 40_000, 0);
+	let (chan_4_update, _, chan_4_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 3, 2, 40_000, 0);
+
+	let amt_msat = 70_000_000;
+	let max_total_routing_fee_msat = Some(1_000_000);
+
+	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV)
+		.with_bolt11_features(nodes[3].node.invoice_features()).unwrap();
+	let (mut route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(
+		nodes[0], nodes[3], payment_params, amt_msat, max_total_routing_fee_msat);
+
+	// Check we overpay on the second path which we're about to fail.
+	assert_eq!(chan_1_update.contents.fee_proportional_millionths, 0);
+	let overpaid_amount_1 = route.paths[0].fee_msat() as u32 - chan_1_update.contents.fee_base_msat;
+	assert_eq!(overpaid_amount_1, 0);
+
+	assert_eq!(chan_2_update.contents.fee_proportional_millionths, 0);
+	let overpaid_amount_2 = route.paths[1].fee_msat() as u32 - chan_2_update.contents.fee_base_msat;
+
+	let total_overpaid_amount = overpaid_amount_1 + overpaid_amount_2;
+
+	// Initiate the payment.
+	let payment_id = PaymentId(payment_hash.0);
+	let mut route_params = route.route_params.clone().unwrap();
+
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
+	nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
+		payment_id, route_params.clone(), Retry::Attempts(1)).unwrap();
+	check_added_monitors!(nodes[0], 2); // one monitor per path
+	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(events.len(), 2);
+
+	// Pass half of the payment along the success path.
+	let success_path_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
+	pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], amt_msat, payment_hash,
+		Some(payment_secret), success_path_msgs, false, None);
+
+	// Add the HTLC along the first hop.
+	let fail_path_msgs_1 = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
+	let (update_add, commitment_signed) = match fail_path_msgs_1 {
+		MessageSendEvent::UpdateHTLCs {
+			node_id: _,
+			updates: msgs::CommitmentUpdate {
+					ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs,
+					ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed
+			}
+		} => {
+			assert_eq!(update_add_htlcs.len(), 1);
+			assert!(update_fail_htlcs.is_empty());
+			assert!(update_fulfill_htlcs.is_empty());
+			assert!(update_fail_malformed_htlcs.is_empty());
+			assert!(update_fee.is_none());
+			(update_add_htlcs[0].clone(), commitment_signed.clone())
+		},
+		_ => panic!("Unexpected event"),
+	};
+	nodes[2].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &update_add);
+	commitment_signed_dance!(nodes[2], nodes[0], commitment_signed, false);
+
+	// Attempt to forward the payment and complete the 2nd path's failure.
+	expect_pending_htlcs_forwardable!(&nodes[2]);
+	expect_pending_htlcs_forwardable_and_htlc_handling_failed!(&nodes[2],
+		vec![HTLCDestination::NextHopChannel {
+			node_id: Some(nodes[3].node.get_our_node_id()), channel_id: chan_4_id
+		}]
+	);
+	let htlc_updates = get_htlc_update_msgs!(nodes[2], nodes[0].node.get_our_node_id());
+	assert!(htlc_updates.update_add_htlcs.is_empty());
+	assert_eq!(htlc_updates.update_fail_htlcs.len(), 1);
+	assert!(htlc_updates.update_fulfill_htlcs.is_empty());
+	assert!(htlc_updates.update_fail_malformed_htlcs.is_empty());
+	check_added_monitors!(nodes[2], 1);
+	nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(),
+		&htlc_updates.update_fail_htlcs[0]);
+	commitment_signed_dance!(nodes[0], nodes[2], htlc_updates.commitment_signed, false);
+	let mut events = nodes[0].node.get_and_clear_pending_events();
+	match events[1] {
+		Event::PendingHTLCsForwardable { .. } => {},
+		_ => panic!("Unexpected event")
+	}
+	events.remove(1);
+	expect_payment_failed_conditions_event(events, payment_hash, false,
+		PaymentFailedConditions::new().mpp_parts_remain());
+
+	// Rebalance the channel so the second half of the payment can succeed.
+	send_payment(&nodes[3], &vec!(&nodes[2])[..], 38_000_000);
+
+	// Retry the second half of the payment and make sure it succeeds.
+	let first_path_value = route.paths[0].final_value_msat();
+	assert_eq!(first_path_value, 36_000_000);
+
+	route.paths.remove(0);
+	route_params.final_value_msat -= first_path_value;
+	route_params.payment_params.previously_failed_channels.push(chan_4_update.contents.short_channel_id);
+
+	// Check the remaining max total routing fee for the second attempt accounts only for 1_000 msat
+	// base fee, but not for overpaid value of the first try.
+	route_params.max_total_routing_fee_msat.as_mut().map(|m| *m -= 1000);
+	nodes[0].router.expect_find_route(route_params, Ok(route));
+	nodes[0].node.process_pending_htlc_forwards();
+
+	check_added_monitors!(nodes[0], 1);
+	let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(events.len(), 1);
+	pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], amt_msat, payment_hash,
+		Some(payment_secret), events.pop().unwrap(), true, None);
+
+	// Can't use claim_payment_along_route as it doesn't support overpayment, so we break out the
+	// individual steps here.
+	let extra_fees = vec![0, total_overpaid_amount];
+	let expected_total_fee_msat = do_claim_payment_along_route_with_extra_penultimate_hop_fees(
+		&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], &extra_fees[..], false,
+		payment_preimage);
+	expect_payment_sent!(&nodes[0], payment_preimage, Some(expected_total_fee_msat));
+}
+
 fn do_mpp_receive_timeout(send_partial_mpp: bool) {
 	let chanmon_cfgs = create_chanmon_cfgs(4);
 	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);