From fbc08477e8dcdd8f3f2ada8ca77388b6185febe2 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Fri, 27 Jan 2023 19:24:52 +0000
Subject: [PATCH 1/7] Move the final CLTV delta to `PaymentParameters` from
 `RouteParams`

`PaymentParams` is all about the parameters for a payment, i.e. the
parameters which are static across all the paths of a paymet.
`RouteParameters` is about the information specific to a given
`Route` (i.e. a set of paths, among multiple potential sets of
paths for a payment). The CLTV delta thus doesn't belong in
`RouterParameters` but instead in `PaymentParameters`.

Worse, because `RouteParameters` is built from the information in
the last hops of a `Route`, when we deliberately inflate the CLTV
delta in path-finding, retries of the payment will have the final
CLTV delta double-inflated as it inflates starting from the final
CLTV delta used in the last attempt.

By moving the CLTV delta to `PaymentParameters` we avoid this
issue, leaving only the sought amount in the `RouteParameters`.
---
 fuzz/src/full_stack.rs                    |   4 +-
 fuzz/src/router.rs                        |   9 +-
 lightning-invoice/src/payment.rs          |  16 +--
 lightning-invoice/src/utils.rs            |   6 +-
 lightning/src/ln/channelmanager.rs        |   8 +-
 lightning/src/ln/functional_test_utils.rs |   6 +-
 lightning/src/ln/functional_tests.rs      |  26 ++---
 lightning/src/ln/onion_route_tests.rs     |   6 +-
 lightning/src/ln/outbound_payment.rs      |   7 +-
 lightning/src/ln/payment_tests.rs         |  34 +++---
 lightning/src/ln/priv_short_conf_tests.rs |  10 +-
 lightning/src/ln/shutdown_tests.rs        |   4 +-
 lightning/src/routing/router.rs           | 120 +++++++++++++---------
 13 files changed, 143 insertions(+), 113 deletions(-)

diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index fb2740c1de9..145b7c8350d 100644
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -512,7 +512,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 			},
 			4 => {
 				let final_value_msat = slice_to_be24(get_slice!(3)) as u64;
-				let payment_params = PaymentParameters::from_node_id(get_pubkey!());
+				let payment_params = PaymentParameters::from_node_id(get_pubkey!(), 42);
 				let params = RouteParameters {
 					payment_params,
 					final_value_msat,
@@ -536,7 +536,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 			},
 			15 => {
 				let final_value_msat = slice_to_be24(get_slice!(3)) as u64;
-				let payment_params = PaymentParameters::from_node_id(get_pubkey!());
+				let payment_params = PaymentParameters::from_node_id(get_pubkey!(), 42);
 				let params = RouteParameters {
 					payment_params,
 					final_value_msat,
diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index 3e51d0c6a6d..86538bbb662 100644
--- a/fuzz/src/router.rs
+++ b/fuzz/src/router.rs
@@ -263,10 +263,13 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 				let scorer = FixedPenaltyScorer::with_penalty(0);
 				let random_seed_bytes: [u8; 32] = [get_slice!(1)[0]; 32];
 				for target in node_pks.iter() {
+					let final_value_msat = slice_to_be64(get_slice!(8));
+					let final_cltv_expiry_delta = slice_to_be32(get_slice!(4));
 					let route_params = RouteParameters {
-						payment_params: PaymentParameters::from_node_id(*target).with_route_hints(last_hops.clone()),
-						final_value_msat: slice_to_be64(get_slice!(8)),
-						final_cltv_expiry_delta: slice_to_be32(get_slice!(4)),
+						payment_params: PaymentParameters::from_node_id(*target, final_cltv_expiry_delta)
+							.with_route_hints(last_hops.clone()),
+						final_value_msat,
+						final_cltv_expiry_delta,
 					};
 					let _ = find_route(&our_pubkey, &route_params, &net_graph,
 						first_hops.map(|c| c.iter().collect::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs
index a3517264b4a..89ff242a268 100644
--- a/lightning-invoice/src/payment.rs
+++ b/lightning-invoice/src/payment.rs
@@ -422,7 +422,8 @@ where
 		};
 
 		let payment_secret = Some(invoice.payment_secret().clone());
-		let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
+		let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
+				invoice.min_final_cltv_expiry_delta() as u32)
 			.with_expiry_time(expiry_time_from_unix_epoch(&invoice).as_secs())
 			.with_route_hints(invoice.route_hints());
 		if let Some(features) = invoice.features() {
@@ -486,7 +487,7 @@ where
 		};
 
 		let route_params = RouteParameters {
-			payment_params: PaymentParameters::for_keysend(pubkey),
+			payment_params: PaymentParameters::for_keysend(pubkey, final_cltv_expiry_delta),
 			final_value_msat: amount_msats,
 			final_cltv_expiry_delta,
 		};
@@ -1377,7 +1378,7 @@ mod tests {
 		assert_eq!(*payer.attempts.borrow(), 1);
 
 		let retry = RouteParameters {
-			payment_params: PaymentParameters::for_keysend(pubkey),
+			payment_params: PaymentParameters::for_keysend(pubkey, final_cltv_expiry_delta),
 			final_value_msat,
 			final_cltv_expiry_delta,
 		};
@@ -1655,7 +1656,8 @@ mod tests {
 		}
 
 		fn retry_for_invoice(invoice: &Invoice) -> RouteParameters {
-			let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
+			let mut payment_params = PaymentParameters::from_node_id(
+					invoice.recover_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32)
 				.with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
 				.with_route_hints(invoice.route_hints());
 			if let Some(features) = invoice.features() {
@@ -2071,7 +2073,7 @@ mod tests {
 					cltv_expiry_delta: 100,
 				}],
 			],
-			payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+			payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 100)),
 		};
 		let router = ManualRouter(RefCell::new(VecDeque::new()));
 		router.expect_find_route(Ok(route.clone()));
@@ -2114,7 +2116,7 @@ mod tests {
 					cltv_expiry_delta: 100,
 				}],
 			],
-			payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+			payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 100)),
 		};
 		let router = ManualRouter(RefCell::new(VecDeque::new()));
 		router.expect_find_route(Ok(route.clone()));
@@ -2194,7 +2196,7 @@ mod tests {
 					cltv_expiry_delta: 100,
 				}]
 			],
-			payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())),
+			payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 100)),
 		};
 		let router = ManualRouter(RefCell::new(VecDeque::new()));
 		router.expect_find_route(Ok(route.clone()));
diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs
index 0d6a1f11667..cee1a6b7a88 100644
--- a/lightning-invoice/src/utils.rs
+++ b/lightning-invoice/src/utils.rs
@@ -724,7 +724,8 @@ mod test {
 		assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan.inbound_htlc_minimum_msat);
 		assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan.inbound_htlc_maximum_msat);
 
-		let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
+		let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
+				invoice.min_final_cltv_expiry_delta() as u32)
 			.with_features(invoice.features().unwrap().clone())
 			.with_route_hints(invoice.route_hints());
 		let route_params = RouteParameters {
@@ -1087,7 +1088,8 @@ mod test {
 		assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into()));
 		assert!(!invoice.features().unwrap().supports_basic_mpp());
 
-		let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
+		let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
+				invoice.min_final_cltv_expiry_delta() as u32)
 			.with_features(invoice.features().unwrap().clone())
 			.with_route_hints(invoice.route_hints());
 		let params = RouteParameters {
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index b2a23c7c1d4..64c3ee6f671 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -7871,7 +7871,7 @@ mod tests {
 
 		// Next, attempt a keysend payment and make sure it fails.
 		let route_params = RouteParameters {
-			payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id()),
+			payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV),
 			final_value_msat: 100_000,
 			final_cltv_expiry_delta: TEST_FINAL_CLTV,
 		};
@@ -7964,7 +7964,7 @@ mod tests {
 
 		let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
 		let route_params = RouteParameters {
-			payment_params: PaymentParameters::for_keysend(payee_pubkey),
+			payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
 			final_value_msat: 10_000,
 			final_cltv_expiry_delta: 40,
 		};
@@ -8009,7 +8009,7 @@ mod tests {
 
 		let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
 		let route_params = RouteParameters {
-			payment_params: PaymentParameters::for_keysend(payee_pubkey),
+			payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
 			final_value_msat: 10_000,
 			final_cltv_expiry_delta: 40,
 		};
@@ -8574,7 +8574,7 @@ pub mod bench {
 		macro_rules! send_payment {
 			($node_a: expr, $node_b: expr) => {
 				let usable_channels = $node_a.list_usable_channels();
-				let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id())
+				let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id(), TEST_FINAL_CLTV)
 					.with_features($node_b.invoice_features());
 				let scorer = test_utils::TestScorer::with_penalty(0);
 				let seed = [3u8; 32];
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 868b38e3fda..dc9522d3071 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1484,7 +1484,7 @@ macro_rules! get_route {
 #[macro_export]
 macro_rules! get_route_and_payment_hash {
 	($send_node: expr, $recv_node: expr, $recv_value: expr) => {{
-		let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id())
+		let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id(), TEST_FINAL_CLTV)
 			.with_features($recv_node.node.invoice_features());
 		$crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV)
 	}};
@@ -2096,7 +2096,7 @@ pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 pub const TEST_FINAL_CLTV: u32 = 70;
 
 pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
-	let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(expected_route.last().unwrap().node.invoice_features());
 	let route = get_route!(origin_node, payment_params, recv_value, TEST_FINAL_CLTV).unwrap();
 	assert_eq!(route.paths.len(), 1);
@@ -2110,7 +2110,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 }
 
 pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
-	let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(expected_route.last().unwrap().node.invoice_features());
 	let network_graph = origin_node.network_graph.read_only();
 	let scorer = test_utils::TestScorer::with_penalty(0);
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 371fcfa7764..bf0c3396ced 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1802,7 +1802,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
 
 	// attempt to send amt_msat > their_max_htlc_value_in_flight_msat
 	{
-		let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+		let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
 			.with_features(nodes[2].node.invoice_features()).with_max_channel_saturation_power_of_half(0);
 		let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, recv_value_0, TEST_FINAL_CLTV);
 		route.paths[0].last_mut().unwrap().fee_msat += 1;
@@ -1827,7 +1827,7 @@ fn test_channel_reserve_holding_cell_htlcs() {
 			break;
 		}
 
-		let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+		let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
 			.with_features(nodes[2].node.invoice_features()).with_max_channel_saturation_power_of_half(0);
 		let route = get_route!(nodes[0], payment_params, recv_value_0, TEST_FINAL_CLTV).unwrap();
 		let (payment_preimage, ..) = send_along_route(&nodes[0], route, &[&nodes[1], &nodes[2]], recv_value_0);
@@ -4716,7 +4716,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 	// We reduce the final CLTV here by a somewhat arbitrary constant to keep it under the one-byte
 	// script push size limit so that the below script length checks match
 	// ACCEPTED_HTLC_SCRIPT_WEIGHT.
-	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV - 40)
 		.with_features(nodes[3].node.invoice_features());
 	let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, 800_000, TEST_FINAL_CLTV - 40);
 	send_along_route_with_secret(&nodes[0], route, &[&[&nodes[1], &nodes[2], &nodes[3]]], 800_000, duplicate_payment_hash, payment_secret);
@@ -6009,7 +6009,7 @@ fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() {
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 	let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 0);
 
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 0)
 		.with_features(nodes[1].node.invoice_features());
 	let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], payment_params, 100000000, 0);
 	route.paths[0].last_mut().unwrap().cltv_expiry_delta = 500000001;
@@ -6927,7 +6927,7 @@ fn test_check_htlc_underpaying() {
 
 	let scorer = test_utils::TestScorer::with_penalty(0);
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()).with_features(nodes[1].node.invoice_features());
+	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features());
 	let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
 	let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
 	let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, None).unwrap();
@@ -7068,7 +7068,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() {
 	let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000);
 
 	let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
-	let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), 30)
 		.with_features(nodes[0].node.invoice_features());
 	let (route,_, _, _) = get_route_and_payment_hash!(nodes[1], nodes[0], payment_params, 3000000, 30);
 	send_along_route(&nodes[1], route, &vec!(&nodes[0])[..], 3000000);
@@ -7174,13 +7174,13 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
 
 	let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000);
 	// Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps)
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()).with_features(nodes[1].node.invoice_features());
+	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 50).with_features(nodes[1].node.invoice_features());
 	let scorer = test_utils::TestScorer::with_penalty(0);
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 	let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None,
 		3_000_000, 50, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
 	let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 3_000_000).0;
-	let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id()).with_features(nodes[0].node.invoice_features());
+	let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), 50).with_features(nodes[0].node.invoice_features());
 	let route = get_route(&nodes[1].node.get_our_node_id(), &payment_params, &nodes[1].network_graph.read_only(), None,
 		3_000_000, 50, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
 	send_along_route(&nodes[1], route, &[&nodes[0]], 3_000_000);
@@ -8976,7 +8976,7 @@ fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) {
 
 	let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001);
 
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(nodes[1].node.invoice_features());
 	let route = get_route!(nodes[0], payment_params, 10_000, TEST_FINAL_CLTV).unwrap();
 
@@ -9081,7 +9081,7 @@ fn test_inconsistent_mpp_params() {
 	create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0);
 	let chan_2_3 =create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0);
 
-	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(nodes[3].node.invoice_features());
 	let mut route = get_route!(nodes[0], payment_params, 15_000_000, TEST_FINAL_CLTV).unwrap();
 	assert_eq!(route.paths.len(), 2);
@@ -9182,7 +9182,7 @@ fn test_keysend_payments_to_public_node() {
 	let payer_pubkey = nodes[0].node.get_our_node_id();
 	let payee_pubkey = nodes[1].node.get_our_node_id();
 	let route_params = RouteParameters {
-		payment_params: PaymentParameters::for_keysend(payee_pubkey),
+		payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
 		final_value_msat: 10000,
 		final_cltv_expiry_delta: 40,
 	};
@@ -9215,7 +9215,7 @@ fn test_keysend_payments_to_private_node() {
 
 	let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
 	let route_params = RouteParameters {
-		payment_params: PaymentParameters::for_keysend(payee_pubkey),
+		payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
 		final_value_msat: 10000,
 		final_cltv_expiry_delta: 40,
 	};
@@ -9603,7 +9603,7 @@ fn do_payment_with_custom_min_final_cltv_expiry(valid_delta: bool, use_user_hash
 
 	create_chan_between_nodes(&nodes[0], &nodes[1]);
 
-	let payment_parameters = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id());
+	let payment_parameters = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), final_cltv_expiry_delta as u32);
 	let (payment_hash, payment_preimage, payment_secret) = if use_user_hash {
 		let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1],
 			Some(recv_value), Some(min_final_cltv_expiry_delta));
diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index 1ecdb5a87fc..1ed10a4a61d 100644
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -655,7 +655,7 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
 			htlc_maximum_msat: None,
 			htlc_minimum_msat: None,
 		}])];
-		let payment_params = PaymentParameters::from_node_id(*channel_to_update_counterparty)
+		let payment_params = PaymentParameters::from_node_id(*channel_to_update_counterparty, TEST_FINAL_CLTV)
 			.with_features(nodes[2].node.invoice_features())
 			.with_route_hints(hop_hints);
 		get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, PAYMENT_AMT, TEST_FINAL_CLTV)
@@ -802,7 +802,7 @@ fn test_always_create_tlv_format_onion_payloads() {
 	create_announced_chan_between_nodes(&nodes, 0, 1);
 	create_announced_chan_between_nodes(&nodes, 1, 2);
 
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(InvoiceFeatures::empty());
 	let (route, _payment_hash, _payment_preimage, _payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 40000, TEST_FINAL_CLTV);
 
@@ -902,7 +902,7 @@ macro_rules! get_phantom_route {
 	($nodes: expr, $amt: expr, $channel: expr) => {{
 		let phantom_pubkey = $nodes[1].keys_manager.get_node_id(Recipient::PhantomNode).unwrap();
 		let phantom_route_hint = $nodes[1].node.get_phantom_route_hints();
-		let payment_params = PaymentParameters::from_node_id(phantom_pubkey)
+		let payment_params = PaymentParameters::from_node_id(phantom_pubkey, TEST_FINAL_CLTV)
 			.with_features($nodes[1].node.invoice_features())
 			.with_route_hints(vec![RouteHint(vec![
 					RouteHintHop {
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 8867b2e96ae..c91829393e4 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1149,8 +1149,9 @@ mod tests {
 
 		let past_expiry_time = std::time::SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() - 2;
 		let payment_params = PaymentParameters::from_node_id(
-			PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()))
-			.with_expiry_time(past_expiry_time);
+				PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()),
+				0
+			).with_expiry_time(past_expiry_time);
 		let expired_route_params = RouteParameters {
 			payment_params,
 			final_value_msat: 0,
@@ -1187,7 +1188,7 @@ mod tests {
 		router.expect_find_route(Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }));
 
 		let payment_params = PaymentParameters::from_node_id(
-			PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()));
+			PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0);
 		let route_params = RouteParameters {
 			payment_params,
 			final_value_msat: 0,
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 726c5b89e54..5897d11be03 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -919,7 +919,7 @@ fn get_ldk_payment_preimage() {
 	let expiry_secs = 60 * 60;
 	let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(amt_msat), expiry_secs, None).unwrap();
 
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(nodes[1].node.invoice_features());
 	let scorer = test_utils::TestScorer::with_penalty(0);
 	let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
@@ -1033,7 +1033,7 @@ fn failed_probe_yields_event() {
 	create_announced_chan_between_nodes(&nodes, 0, 1);
 	create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000);
 
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42);
 
 	let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_998_000, 42);
 
@@ -1080,7 +1080,7 @@ fn onchain_failed_probe_yields_event() {
 	let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
 	create_announced_chan_between_nodes(&nodes, 1, 2);
 
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42);
 
 	// Send a dust HTLC, which will be treated as if it timed out once the channel hits the chain.
 	let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 1_000, 42);
@@ -1425,7 +1425,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
 
 	let amt_msat = 100_000;
 	let intercept_scid = nodes[1].node.get_intercept_scid();
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_route_hints(vec![
 			RouteHint(vec![RouteHintHop {
 				src_node_id: nodes[1].node.get_our_node_id(),
@@ -1623,7 +1623,7 @@ fn do_automatic_retries(test: AutoRetry) {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_expiry_time(payment_expiry_secs as u64)
 		.with_features(invoice_features);
 	let route_params = RouteParameters {
@@ -1805,7 +1805,7 @@ fn auto_retry_partial_failure() {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	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_features(invoice_features);
 	let route_params = RouteParameters {
@@ -1844,7 +1844,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	let retry_1_route = Route {
 		paths: vec![
@@ -1865,7 +1865,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	let retry_2_route = Route {
 		paths: vec![
@@ -1878,7 +1878,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	nodes[0].router.expect_find_route(Ok(send_route));
 	nodes[0].router.expect_find_route(Ok(retry_1_route));
@@ -2001,7 +2001,7 @@ fn auto_retry_zero_attempts_send_error() {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	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_features(invoice_features);
 	let route_params = RouteParameters {
@@ -2039,7 +2039,7 @@ fn fails_paying_after_rejected_by_payee() {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	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_features(invoice_features);
 	let route_params = RouteParameters {
@@ -2094,7 +2094,7 @@ fn retry_multi_path_single_failed_payment() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	nodes[0].router.expect_find_route(Ok(route.clone()));
 	// On retry, split the payment across both channels.
@@ -2112,7 +2112,7 @@ fn retry_multi_path_single_failed_payment() {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	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_features(invoice_features);
 	let route_params = RouteParameters {
@@ -2149,7 +2149,7 @@ fn immediate_retry_on_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	nodes[0].router.expect_find_route(Ok(route.clone()));
 	// On retry, split the payment across both channels.
@@ -2169,7 +2169,7 @@ fn immediate_retry_on_failure() {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	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_features(invoice_features);
 	let route_params = RouteParameters {
@@ -2241,7 +2241,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 				cltv_expiry_delta: 100,
 			}]
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())),
+		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	nodes[0].router.expect_find_route(Ok(route.clone()));
 	// On retry, we'll only be asked for one path
@@ -2258,7 +2258,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 	invoice_features.set_variable_length_onion_required();
 	invoice_features.set_payment_secret_required();
 	invoice_features.set_basic_mpp_optional();
-	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+	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_features(invoice_features);
 	let route_params = RouteParameters {
diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs
index aca781b9fd6..a6fd7c1dc09 100644
--- a/lightning/src/ln/priv_short_conf_tests.rs
+++ b/lightning/src/ln/priv_short_conf_tests.rs
@@ -66,7 +66,7 @@ fn test_priv_forwarding_rejection() {
 		htlc_maximum_msat: None,
 	}]);
 	let last_hops = vec![route_hint];
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)
 		.with_features(nodes[2].node.invoice_features())
 		.with_route_hints(last_hops);
 	let (route, our_payment_hash, our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 10_000, TEST_FINAL_CLTV);
@@ -233,7 +233,7 @@ fn test_routed_scid_alias() {
 		htlc_maximum_msat: None,
 		htlc_minimum_msat: None,
 	}])];
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42)
 		.with_features(nodes[2].node.invoice_features())
 		.with_route_hints(hop_hints);
 	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 100_000, 42);
@@ -388,7 +388,7 @@ fn test_inbound_scid_privacy() {
 		htlc_maximum_msat: None,
 		htlc_minimum_msat: None,
 	}])];
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42)
 		.with_features(nodes[2].node.invoice_features())
 		.with_route_hints(hop_hints.clone());
 	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 100_000, 42);
@@ -403,7 +403,7 @@ fn test_inbound_scid_privacy() {
 	// what channel we're talking about.
 	hop_hints[0].0[0].short_channel_id = last_hop[0].short_channel_id.unwrap();
 
-	let payment_params_2 = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params_2 = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42)
 		.with_features(nodes[2].node.invoice_features())
 		.with_route_hints(hop_hints);
 	let (route_2, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params_2, 100_000, 42);
@@ -454,7 +454,7 @@ fn test_scid_alias_returned() {
 		htlc_maximum_msat: None,
 		htlc_minimum_msat: None,
 	}])];
-	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+	let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42)
 		.with_features(nodes[2].node.invoice_features())
 		.with_route_hints(hop_hints);
 	let (mut route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 10_000, 42);
diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs
index 2e703600f55..c1fe23a62a6 100644
--- a/lightning/src/ln/shutdown_tests.rs
+++ b/lightning/src/ln/shutdown_tests.rs
@@ -93,9 +93,9 @@ fn updates_shutdown_wait() {
 
 	let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[0]);
 
-	let payment_params_1 = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()).with_features(nodes[1].node.invoice_features());
+	let payment_params_1 = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features());
 	let route_1 = get_route(&nodes[0].node.get_our_node_id(), &payment_params_1, &nodes[0].network_graph.read_only(), None, 100000, TEST_FINAL_CLTV, &logger, &scorer, &random_seed_bytes).unwrap();
-	let payment_params_2 = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id()).with_features(nodes[0].node.invoice_features());
+	let payment_params_2 = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[0].node.invoice_features());
 	let route_2 = get_route(&nodes[1].node.get_our_node_id(), &payment_params_2, &nodes[1].network_graph.read_only(), None, 100000, TEST_FINAL_CLTV, &logger, &scorer, &random_seed_bytes).unwrap();
 	unwrap_send_err!(nodes[0].node.send_payment(&route_1, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)), true, APIError::ChannelUnavailable {..}, {});
 	unwrap_send_err!(nodes[1].node.send_payment(&route_2, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)), true, APIError::ChannelUnavailable {..}, {});
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 1e2ca10c0bf..950384d957e 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -372,6 +372,9 @@ pub struct RouteParameters {
 	pub final_value_msat: u64,
 
 	/// The CLTV on the final hop of the failed payment path.
+	///
+	/// This field is deprecated, [`PaymentParameters::final_cltv_expiry_delta`] should be used
+	/// instead, if available.
 	pub final_cltv_expiry_delta: u32,
 }
 
@@ -452,6 +455,11 @@ pub struct PaymentParameters {
 	/// payment to fail. Future attempts for the same payment shouldn't be relayed through any of
 	/// these SCIDs.
 	pub previously_failed_channels: Vec<u64>,
+
+	/// The minimum CLTV delta at the end of the route.
+	///
+	/// This field should always be set to `Some` and may be required in a future release.
+	pub final_cltv_expiry_delta: Option<u32>,
 }
 
 impl_writeable_tlv_based!(PaymentParameters, {
@@ -463,11 +471,15 @@ impl_writeable_tlv_based!(PaymentParameters, {
 	(5, max_channel_saturation_power_of_half, (default_value, 2)),
 	(6, expiry_time, option),
 	(7, previously_failed_channels, vec_type),
+	(9, final_cltv_expiry_delta, option),
 });
 
 impl PaymentParameters {
 	/// Creates a payee with the node id of the given `pubkey`.
-	pub fn from_node_id(payee_pubkey: PublicKey) -> Self {
+	///
+	/// The `final_cltv_expiry_delta` should match the expected final CLTV delta the recipient has
+	/// provided.
+	pub fn from_node_id(payee_pubkey: PublicKey, final_cltv_expiry_delta: u32) -> Self {
 		Self {
 			payee_pubkey,
 			features: None,
@@ -477,12 +489,16 @@ impl PaymentParameters {
 			max_path_count: DEFAULT_MAX_PATH_COUNT,
 			max_channel_saturation_power_of_half: 2,
 			previously_failed_channels: Vec::new(),
+			final_cltv_expiry_delta: Some(final_cltv_expiry_delta),
 		}
 	}
 
 	/// Creates a payee with the node id of the given `pubkey` to use for keysend payments.
-	pub fn for_keysend(payee_pubkey: PublicKey) -> Self {
-		Self::from_node_id(payee_pubkey).with_features(InvoiceFeatures::for_keysend())
+	///
+	/// The `final_cltv_expiry_delta` should match the expected final CLTV delta the recipient has
+	/// provided.
+	pub fn for_keysend(payee_pubkey: PublicKey, final_cltv_expiry_delta: u32) -> Self {
+		Self::from_node_id(payee_pubkey, final_cltv_expiry_delta).with_features(InvoiceFeatures::for_keysend())
 	}
 
 	/// Includes the payee's features.
@@ -948,8 +964,11 @@ pub fn find_route<L: Deref, GL: Deref, S: Score>(
 ) -> Result<Route, LightningError>
 where L::Target: Logger, GL::Target: Logger {
 	let graph_lock = network_graph.read_only();
+	let final_cltv_expiry_delta =
+		if let Some(delta) = route_params.payment_params.final_cltv_expiry_delta { delta }
+		else { route_params.final_cltv_expiry_delta };
 	let mut route = get_route(our_node_pubkey, &route_params.payment_params, &graph_lock, first_hops,
-		route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, scorer,
+		route_params.final_value_msat, final_cltv_expiry_delta, logger, scorer,
 		random_seed_bytes)?;
 	add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
 	Ok(route)
@@ -986,6 +1005,9 @@ where L::Target: Logger {
 	if payment_params.max_total_cltv_expiry_delta <= final_cltv_expiry_delta {
 		return Err(LightningError{err: "Can't find a route where the maximum total CLTV expiry delta is below the final CLTV expiry.".to_owned(), action: ErrorAction::IgnoreError});
 	}
+	if let Some(delta) = payment_params.final_cltv_expiry_delta {
+		debug_assert_eq!(delta, final_cltv_expiry_delta);
+	}
 
 	// The general routing idea is the following:
 	// 1. Fill first/last hops communicated by the caller.
@@ -2165,7 +2187,7 @@ mod tests {
 	fn simple_route_test() {
 		let (secp_ctx, network_graph, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2198,7 +2220,7 @@ mod tests {
 	fn invalid_first_hop_test() {
 		let (secp_ctx, network_graph, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2220,7 +2242,7 @@ mod tests {
 	fn htlc_minimum_test() {
 		let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
 		let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2348,7 +2370,7 @@ mod tests {
 		let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
 		let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config));
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2486,7 +2508,7 @@ mod tests {
 	fn disable_channels_test() {
 		let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
 		let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2546,7 +2568,7 @@ mod tests {
 	fn disable_node_test() {
 		let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
 		let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2596,7 +2618,7 @@ mod tests {
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
 		// Route to 1 via 2 and 3 because our channel to 1 is disabled
-		let payment_params = PaymentParameters::from_node_id(nodes[0]);
+		let payment_params = PaymentParameters::from_node_id(nodes[0], 42);
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].len(), 3);
 
@@ -2622,7 +2644,7 @@ mod tests {
 		assert_eq!(route.paths[0][2].channel_features.le_flags(), &id_to_feature_flags(3));
 
 		// If we specify a channel to node7, that overrides our local channel view and that gets used
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 		let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), Some(&our_chans.iter().collect::<Vec<_>>()), 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].len(), 2);
@@ -2746,13 +2768,13 @@ mod tests {
 		let mut invalid_last_hops = last_hops_multi_private_channels(&nodes);
 		invalid_last_hops.push(invalid_last_hop);
 		{
-			let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(invalid_last_hops);
+			let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(invalid_last_hops);
 			if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes) {
 				assert_eq!(err, "Route hint cannot have the payee as the source.");
 			} else { panic!(); }
 		}
 
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops_multi_private_channels(&nodes));
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops_multi_private_channels(&nodes));
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].len(), 5);
 
@@ -2822,7 +2844,7 @@ mod tests {
 	fn ignores_empty_last_hops_test() {
 		let (secp_ctx, network_graph, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(empty_last_hop(&nodes));
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(empty_last_hop(&nodes));
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2902,7 +2924,7 @@ mod tests {
 		let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
 		let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 		let last_hops = multi_hop_last_hops_hint([nodes[2], nodes[3]]);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops.clone());
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone());
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2976,7 +2998,7 @@ mod tests {
 		let non_announced_pubkey = PublicKey::from_secret_key(&secp_ctx, &non_announced_privkey);
 
 		let last_hops = multi_hop_last_hops_hint([nodes[2], non_announced_pubkey]);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops.clone());
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone());
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		// Test through channels 2, 3, 0xff00, 0xff01.
 		// Test shows that multiple hop hints are considered.
@@ -3082,7 +3104,7 @@ mod tests {
 	fn last_hops_with_public_channel_test() {
 		let (secp_ctx, network_graph, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops_with_public_channel(&nodes));
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops_with_public_channel(&nodes));
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -3141,7 +3163,7 @@ mod tests {
 		// Simple test with outbound channel to 4 to test that last_hops and first_hops connect
 		let our_chans = vec![get_channel_details(Some(42), nodes[3].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
 		let mut last_hops = last_hops(&nodes);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops.clone());
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone());
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), Some(&our_chans.iter().collect::<Vec<_>>()), 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].len(), 2);
 
@@ -3162,7 +3184,7 @@ mod tests {
 		last_hops[0].0[0].fees.base_msat = 1000;
 
 		// Revert to via 6 as the fee on 8 goes up
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops);
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops);
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
 		assert_eq!(route.paths[0].len(), 4);
 
@@ -3255,7 +3277,7 @@ mod tests {
 			htlc_minimum_msat: None,
 			htlc_maximum_msat: last_hop_htlc_max,
 		}]);
-		let payment_params = PaymentParameters::from_node_id(target_node_id).with_route_hints(vec![last_hops]);
+		let payment_params = PaymentParameters::from_node_id(target_node_id, 42).with_route_hints(vec![last_hops]);
 		let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)];
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
@@ -3322,7 +3344,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config));
 
 		// We will use a simple single-path route from
 		// our node to node2 via node0: channels {1, 3}.
@@ -3597,7 +3619,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config));
 
 		// Path via {node7, node2, node4} is channels {12, 13, 6, 11}.
 		// {12, 13, 11} have the capacities of 100, {6} has a capacity of 50.
@@ -3722,7 +3744,7 @@ mod tests {
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 
 		// Path via node0 is channels {1, 3}. Limit them to 100 and 50 sats (total limit 50).
 		update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
@@ -3771,7 +3793,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[2])
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42)
 			.with_features(channelmanager::provided_invoice_features(&config));
 
 		// We need a route consisting of 3 paths:
@@ -3931,7 +3953,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config));
 
 		// We need a route consisting of 3 paths:
 		// From our node to node3 via {node0, node2}, {node7, node2, node4} and {node7, node2}.
@@ -4096,7 +4118,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config));
 
 		// This test checks that if we have two cheaper paths and one more expensive path,
 		// so that liquidity-wise any 2 of 3 combination is sufficient,
@@ -4266,7 +4288,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config));
 
 		// We need a route consisting of 2 paths:
 		// From our node to node3 via {node0, node2} and {node7, node2, node4}.
@@ -4448,7 +4470,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(PublicKey::from_slice(&[02; 33]).unwrap()).with_features(channelmanager::provided_invoice_features(&config))
+		let payment_params = PaymentParameters::from_node_id(PublicKey::from_slice(&[02; 33]).unwrap(), 42).with_features(channelmanager::provided_invoice_features(&config))
 			.with_route_hints(vec![RouteHint(vec![RouteHintHop {
 				src_node_id: nodes[2],
 				short_channel_id: 42,
@@ -4540,7 +4562,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config))
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config))
 			.with_max_channel_saturation_power_of_half(0);
 
 		// We need a route consisting of 3 paths:
@@ -4701,7 +4723,7 @@ mod tests {
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
-		let payment_params = PaymentParameters::from_node_id(nodes[6]);
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42);
 
 		add_channel(&gossip_sync, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6);
 		update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
@@ -4832,7 +4854,7 @@ mod tests {
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]);
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
 
 		// We modify the graph to set the htlc_maximum of channel 2 to below the value we wish to
 		// send.
@@ -4897,7 +4919,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config));
 
 		// We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2
 		// gets an htlc_maximum_msat of 80_000 and channel 4 an htlc_minimum_msat of 90_000. We
@@ -4966,7 +4988,7 @@ mod tests {
 		let network_graph = NetworkGraph::new(genesis_hash, Arc::clone(&logger));
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[0]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[0], 42).with_features(channelmanager::provided_invoice_features(&config));
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -5032,7 +5054,7 @@ mod tests {
 	fn prefers_shorter_route_with_higher_fees() {
 		let (secp_ctx, network_graph, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes));
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes));
 
 		// Without penalizing each hop 100 msats, a longer path with lower fees is chosen.
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
@@ -5105,7 +5127,7 @@ mod tests {
 	fn avoids_routing_through_bad_channels_and_nodes() {
 		let (secp_ctx, network, _, _, logger) = build_graph();
 		let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes));
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes));
 		let network_graph = network.read_only();
 
 		// A path to nodes[6] exists when no penalties are applied to any channel.
@@ -5228,7 +5250,7 @@ mod tests {
 
 		// Make sure that generally there is at least one route available
 		let feasible_max_total_cltv_delta = 1008;
-		let feasible_payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes))
+		let feasible_payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes))
 			.with_max_total_cltv_expiry_delta(feasible_max_total_cltv_delta);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -5238,7 +5260,7 @@ mod tests {
 
 		// But not if we exclude all paths on the basis of their accumulated CLTV delta
 		let fail_max_total_cltv_delta = 23;
-		let fail_payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes))
+		let fail_payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes))
 			.with_max_total_cltv_expiry_delta(fail_max_total_cltv_delta);
 		match get_route(&our_id, &fail_payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes)
 		{
@@ -5258,7 +5280,7 @@ mod tests {
 		let network_graph = network.read_only();
 
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
-		let mut payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes))
+		let mut payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes))
 			.with_max_path_count(1);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -5289,14 +5311,14 @@ mod tests {
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
 		// First check we can actually create a long route on this graph.
-		let feasible_payment_params = PaymentParameters::from_node_id(nodes[18]);
+		let feasible_payment_params = PaymentParameters::from_node_id(nodes[18], 0);
 		let route = get_route(&our_id, &feasible_payment_params, &network_graph, None, 100, 0,
 			Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
 		let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
 		assert!(path.len() == MAX_PATH_LENGTH_ESTIMATE.into());
 
 		// But we can't create a path surpassing the MAX_PATH_LENGTH_ESTIMATE limit.
-		let fail_payment_params = PaymentParameters::from_node_id(nodes[19]);
+		let fail_payment_params = PaymentParameters::from_node_id(nodes[19], 0);
 		match get_route(&our_id, &fail_payment_params, &network_graph, None, 100, 0,
 			Arc::clone(&logger), &scorer, &random_seed_bytes)
 		{
@@ -5314,7 +5336,7 @@ mod tests {
 
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
 
-		let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes));
+		let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes));
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
@@ -5347,7 +5369,7 @@ mod tests {
 		let network_nodes = network_graph.nodes();
 		let network_channels = network_graph.channels();
 		let scorer = ln_test_utils::TestScorer::with_penalty(0);
-		let payment_params = PaymentParameters::from_node_id(nodes[3]);
+		let payment_params = PaymentParameters::from_node_id(nodes[3], 0);
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[4u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -5414,7 +5436,7 @@ mod tests {
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
-		let payment_params = PaymentParameters::from_node_id(nodes[3]);
+		let payment_params = PaymentParameters::from_node_id(nodes[3], 0);
 		let hops = [nodes[1], nodes[2], nodes[4], nodes[3]];
 		let route = build_route_from_hops_internal(&our_id, &hops, &payment_params,
 			 &network_graph, 100, 0, Arc::clone(&logger), &random_seed_bytes).unwrap();
@@ -5460,7 +5482,7 @@ mod tests {
 		});
 
 		let config = UserConfig::default();
-		let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config));
+		let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config));
 		let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 		let random_seed_bytes = keys_manager.get_secure_random_bytes();
 		// 100,000 sats is less than the available liquidity on each channel, set above.
@@ -5507,7 +5529,7 @@ mod tests {
 				let src = &PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
 				seed = seed.overflowing_mul(0xdeadbeef).0;
 				let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-				let payment_params = PaymentParameters::from_node_id(dst);
+				let payment_params = PaymentParameters::from_node_id(dst, 42);
 				let amt = seed as u64 % 200_000_000;
 				let params = ProbabilisticScoringParameters::default();
 				let scorer = ProbabilisticScorer::new(params, &graph, &logger);
@@ -5545,7 +5567,7 @@ mod tests {
 				let src = &PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
 				seed = seed.overflowing_mul(0xdeadbeef).0;
 				let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-				let payment_params = PaymentParameters::from_node_id(dst).with_features(channelmanager::provided_invoice_features(&config));
+				let payment_params = PaymentParameters::from_node_id(dst, 42).with_features(channelmanager::provided_invoice_features(&config));
 				let amt = seed as u64 % 200_000_000;
 				let params = ProbabilisticScoringParameters::default();
 				let scorer = ProbabilisticScorer::new(params, &graph, &logger);
@@ -5578,7 +5600,7 @@ mod tests {
 		assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage), 456);
 
 		// Then check we can get a normal route
-		let payment_params = PaymentParameters::from_node_id(nodes[10]);
+		let payment_params = PaymentParameters::from_node_id(nodes[10], 42);
 		let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes);
 		assert!(route.is_ok());
 
@@ -5745,7 +5767,7 @@ mod benches {
 				let src = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
 				seed *= 0xdeadbeef;
 				let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-				let params = PaymentParameters::from_node_id(dst).with_features(features.clone());
+				let params = PaymentParameters::from_node_id(dst, 42).with_features(features.clone());
 				let first_hop = first_hop(src);
 				let amt = seed as u64 % 1_000_000;
 				if let Ok(route) = get_route(&payer, &params, &graph.read_only(), Some(&[&first_hop]), amt, 42, &DummyLogger{}, &scorer, &random_seed_bytes) {

From 2b135578e8e88d3e8a16463be7a8ef4458abd33e Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Fri, 27 Jan 2023 20:28:20 +0000
Subject: [PATCH 2/7] Use the `PaymentParameter` final CLTV delta over
 `RouteParameters`

`PaymentParams` is all about the parameters for a payment, i.e. the
parameters which are static across all the paths of a paymet.
`RouteParameters` is about the information specific to a given
`Route` (i.e. a set of paths, among multiple potential sets of
paths for a payment). The CLTV delta thus doesn't belong in
`RouterParameters` but instead in `PaymentParameters`.

Worse, because `RouteParameters` is built from the information in
the last hops of a `Route`, when we deliberately inflate the CLTV
delta in path-finding, retries of the payment will have the final
CLTV delta double-inflated as it inflates starting from the final
CLTV delta used in the last attempt.

When we calculate the `final_cltv_expiry_delta` to put in the
`RouteParameters` returned via events after a payment failure, we
should re-use the new one in the `PaymentParameters`, rather than
the one that was in the route itself.
---
 lightning/src/ln/channelmanager.rs   | 4 ++++
 lightning/src/ln/outbound_payment.rs | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 64c3ee6f671..8620c3cc15f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -245,6 +245,10 @@ pub(crate) enum HTLCSource {
 		first_hop_htlc_msat: u64,
 		payment_id: PaymentId,
 		payment_secret: Option<PaymentSecret>,
+		/// Note that this is now "deprecated" - we write it for forwards (and read it for
+		/// backwards) compatibility reasons, but prefer to use the data in the
+		/// [`super::outbound_payment`] module, which stores per-payment data once instead of in
+		/// each HTLC.
 		payment_params: Option<PaymentParameters>,
 	},
 }
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index c91829393e4..a8acb9e80ba 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -789,7 +789,9 @@ impl OutboundPayments {
 						Some(RouteParameters {
 							payment_params: payment_params.clone(),
 							final_value_msat: pending_amt_unsent,
-							final_cltv_expiry_delta: max_unsent_cltv_delta,
+							final_cltv_expiry_delta:
+								if let Some(delta) = payment_params.final_cltv_expiry_delta { delta }
+								else { max_unsent_cltv_delta },
 						})
 					} else { None }
 				} else { None },
@@ -987,7 +989,9 @@ impl OutboundPayments {
 			Some(RouteParameters {
 				payment_params: payment_params_data.clone(),
 				final_value_msat: path_last_hop.fee_msat,
-				final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta,
+				final_cltv_expiry_delta:
+					if let Some(delta) = payment_params_data.final_cltv_expiry_delta { delta }
+					else { path_last_hop.cltv_expiry_delta },
 			})
 		} else { None };
 		log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));

From 5ce4bfc1f6369cde16a88f2d2af416fc32f71082 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Fri, 27 Jan 2023 20:31:10 +0000
Subject: [PATCH 3/7] Test the `RouteParameters` passed to `TestRouter`

`TestRouter` allows us to simply select the `Route` that will be
returned in the next `find_route` call, but it does so without any
checking of what was *requested* for the call. This makes it a
somewhat dubious test utility as it very helpfully ensures we
ignore errors in the routes we're looking for.

Instead, we require users of `TestRouter` pass a `RouteParameters`
to `expect_find_route`, which we compare against the requested
parameters passed to `find_route`.
---
 lightning/src/ln/outbound_payment.rs |   5 +-
 lightning/src/ln/payment_tests.rs    | 158 +++++++++++++++------------
 lightning/src/util/test_utils.rs     |  11 +-
 3 files changed, 99 insertions(+), 75 deletions(-)

diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index a8acb9e80ba..d6e32ed2519 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -1189,8 +1189,6 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
 
-		router.expect_find_route(Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }));
-
 		let payment_params = PaymentParameters::from_node_id(
 			PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0);
 		let route_params = RouteParameters {
@@ -1198,6 +1196,9 @@ mod tests {
 			final_value_msat: 0,
 			final_cltv_expiry_delta: 0,
 		};
+		router.expect_find_route(route_params.clone(),
+			Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }));
+
 		let err = if on_retry {
 			outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]),
 			&Route { paths: vec![], payment_params: None }, Retry::Attempts(1), Some(route_params.clone()),
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 5897d11be03..c7c8198f6c4 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -1844,7 +1844,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		payment_params: Some(route_params.payment_params.clone()),
 	};
 	let retry_1_route = Route {
 		paths: vec![
@@ -1865,7 +1865,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		payment_params: Some(route_params.payment_params.clone()),
 	};
 	let retry_2_route = Route {
 		paths: vec![
@@ -1878,11 +1878,17 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		payment_params: Some(route_params.payment_params.clone()),
 	};
-	nodes[0].router.expect_find_route(Ok(send_route));
-	nodes[0].router.expect_find_route(Ok(retry_1_route));
-	nodes[0].router.expect_find_route(Ok(retry_2_route));
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route));
+	nodes[0].router.expect_find_route(RouteParameters {
+			payment_params: route_params.payment_params.clone(),
+			final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV
+		}, Ok(retry_1_route));
+	nodes[0].router.expect_find_route(RouteParameters {
+			payment_params: route_params.payment_params.clone(),
+			final_value_msat: amt_msat / 4, final_cltv_expiry_delta: TEST_FINAL_CLTV
+		}, Ok(retry_2_route));
 
 	// Send a payment that will partially fail on send, then partially fail on retry, then succeed.
 	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(3)).unwrap();
@@ -2074,6 +2080,27 @@ fn retry_multi_path_single_failed_payment() {
 
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+
+	let amt_msat = 100_010_000;
+
+	let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
+	#[cfg(feature = "std")]
+	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
+	#[cfg(not(feature = "std"))]
+	let payment_expiry_secs = 60 * 60;
+	let mut invoice_features = InvoiceFeatures::empty();
+	invoice_features.set_variable_length_onion_required();
+	invoice_features.set_payment_secret_required();
+	invoice_features.set_basic_mpp_optional();
+	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_features(invoice_features);
+	let route_params = RouteParameters {
+		payment_params: payment_params.clone(),
+		final_value_msat: amt_msat,
+		final_cltv_expiry_delta: TEST_FINAL_CLTV,
+	};
+
 	let chans = nodes[0].node.list_usable_channels();
 	let mut route = Route {
 		paths: vec![
@@ -2094,15 +2121,37 @@ fn retry_multi_path_single_failed_payment() {
 				cltv_expiry_delta: 100,
 			}],
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		payment_params: Some(payment_params),
 	};
-	nodes[0].router.expect_find_route(Ok(route.clone()));
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	// On retry, split the payment across both channels.
 	route.paths[0][0].fee_msat = 50_000_001;
 	route.paths[1][0].fee_msat = 50_000_000;
-	nodes[0].router.expect_find_route(Ok(route.clone()));
+	nodes[0].router.expect_find_route(RouteParameters {
+			payment_params: route.payment_params.clone().unwrap(),
+			// Note that the second request here requests the amount we originally failed to send,
+			// not the amount remaining on the full payment, which should be changed.
+			final_value_msat: 100_000_001, final_cltv_expiry_delta: TEST_FINAL_CLTV
+		}, Ok(route.clone()));
 
-	let amt_msat = 100_010_000;
+	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
+	let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(htlc_msgs.len(), 2);
+	check_added_monitors!(nodes[0], 2);
+}
+
+#[test]
+fn immediate_retry_on_failure() {
+	// Tests that we can/will retry immediately after a failure
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
+
+	let amt_msat = 100_000_001;
 	let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
 	#[cfg(feature = "std")]
 	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
@@ -2121,22 +2170,6 @@ fn retry_multi_path_single_failed_payment() {
 		final_cltv_expiry_delta: TEST_FINAL_CLTV,
 	};
 
-	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
-	let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
-	assert_eq!(htlc_msgs.len(), 2);
-	check_added_monitors!(nodes[0], 2);
-}
-
-#[test]
-fn immediate_retry_on_failure() {
-	// Tests that we can/will retry immediately after a failure
-	let chanmon_cfgs = create_chanmon_cfgs(2);
-	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]);
-	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
-	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
-	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
 	let chans = nodes[0].node.list_usable_channels();
 	let mut route = Route {
 		paths: vec![
@@ -2151,32 +2184,16 @@ fn immediate_retry_on_failure() {
 		],
 		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
-	nodes[0].router.expect_find_route(Ok(route.clone()));
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	// On retry, split the payment across both channels.
 	route.paths.push(route.paths[0].clone());
 	route.paths[0][0].short_channel_id = chans[1].short_channel_id.unwrap();
 	route.paths[0][0].fee_msat = 50_000_000;
 	route.paths[1][0].fee_msat = 50_000_001;
-	nodes[0].router.expect_find_route(Ok(route.clone()));
-
-	let amt_msat = 100_010_000;
-	let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
-	#[cfg(feature = "std")]
-	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
-	#[cfg(not(feature = "std"))]
-	let payment_expiry_secs = 60 * 60;
-	let mut invoice_features = InvoiceFeatures::empty();
-	invoice_features.set_variable_length_onion_required();
-	invoice_features.set_payment_secret_required();
-	invoice_features.set_basic_mpp_optional();
-	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_features(invoice_features);
-	let route_params = RouteParameters {
-		payment_params,
-		final_value_msat: amt_msat,
-		final_cltv_expiry_delta: TEST_FINAL_CLTV,
-	};
+	nodes[0].router.expect_find_route(RouteParameters {
+			payment_params: route_params.payment_params.clone(),
+			final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV
+		}, Ok(route.clone()));
 
 	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
 	let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
@@ -2208,6 +2225,25 @@ fn no_extra_retries_on_back_to_back_fail() {
 	let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0).0.contents.short_channel_id;
 	let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0).0.contents.short_channel_id;
 
+	let amt_msat = 200_000_000;
+	let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
+	#[cfg(feature = "std")]
+	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
+	#[cfg(not(feature = "std"))]
+	let payment_expiry_secs = 60 * 60;
+	let mut invoice_features = InvoiceFeatures::empty();
+	invoice_features.set_variable_length_onion_required();
+	invoice_features.set_payment_secret_required();
+	invoice_features.set_basic_mpp_optional();
+	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_features(invoice_features);
+	let route_params = RouteParameters {
+		payment_params,
+		final_value_msat: amt_msat,
+		final_cltv_expiry_delta: TEST_FINAL_CLTV,
+	};
+
 	let mut route = Route {
 		paths: vec![
 			vec![RouteHop {
@@ -2243,29 +2279,15 @@ fn no_extra_retries_on_back_to_back_fail() {
 		],
 		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
-	nodes[0].router.expect_find_route(Ok(route.clone()));
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	// On retry, we'll only be asked for one path
 	route.paths.remove(1);
-	nodes[0].router.expect_find_route(Ok(route.clone()));
-
-	let amt_msat = 100_010_000;
-	let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
-	#[cfg(feature = "std")]
-	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
-	#[cfg(not(feature = "std"))]
-	let payment_expiry_secs = 60 * 60;
-	let mut invoice_features = InvoiceFeatures::empty();
-	invoice_features.set_variable_length_onion_required();
-	invoice_features.set_payment_secret_required();
-	invoice_features.set_basic_mpp_optional();
-	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_features(invoice_features);
-	let route_params = RouteParameters {
-		payment_params,
-		final_value_msat: amt_msat,
-		final_cltv_expiry_delta: TEST_FINAL_CLTV,
-	};
+	let mut second_payment_params = route_params.payment_params.clone();
+	second_payment_params.previously_failed_channels = vec![chan_2_scid, chan_2_scid];
+	nodes[0].router.expect_find_route(RouteParameters {
+			payment_params: second_payment_params,
+			final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV,
+		}, Ok(route.clone()));
 
 	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
 	let htlc_updates = SendEvent::from_node(&nodes[0]);
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 1dae61ab3ef..f9a2eafb82d 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -77,7 +77,7 @@ impl chaininterface::FeeEstimator for TestFeeEstimator {
 
 pub struct TestRouter<'a> {
 	pub network_graph: Arc<NetworkGraph<&'a TestLogger>>,
-	pub next_routes: Mutex<VecDeque<Result<Route, LightningError>>>,
+	pub next_routes: Mutex<VecDeque<(RouteParameters, Result<Route, LightningError>)>>,
 }
 
 impl<'a> TestRouter<'a> {
@@ -85,9 +85,9 @@ impl<'a> TestRouter<'a> {
 		Self { network_graph, next_routes: Mutex::new(VecDeque::new()), }
 	}
 
-	pub fn expect_find_route(&self, result: Result<Route, LightningError>) {
+	pub fn expect_find_route(&self, query: RouteParameters, result: Result<Route, LightningError>) {
 		let mut expected_routes = self.next_routes.lock().unwrap();
-		expected_routes.push_back(result);
+		expected_routes.push_back((query, result));
 	}
 }
 
@@ -96,8 +96,9 @@ impl<'a> Router for TestRouter<'a> {
 		&self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&channelmanager::ChannelDetails]>,
 		inflight_htlcs: &InFlightHtlcs
 	) -> Result<Route, msgs::LightningError> {
-		if let Some(find_route_res) = self.next_routes.lock().unwrap().pop_front() {
-			return find_route_res
+		if let Some((find_route_query, find_route_res)) = self.next_routes.lock().unwrap().pop_front() {
+			assert_eq!(find_route_query, *params);
+			return find_route_res;
 		}
 		let logger = TestLogger::new();
 		find_route(

From ddde63ee12c8b10c0597c50940006d4cd53cfbc3 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Fri, 27 Jan 2023 22:59:28 +0000
Subject: [PATCH 4/7] Log more information when retrying a payment attempt
 fails

---
 lightning/src/ln/channelmanager.rs   |  2 +-
 lightning/src/ln/outbound_payment.rs | 33 +++++++++++++++++-----------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 8620c3cc15f..3b58610487f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2477,7 +2477,7 @@ where
 		self.pending_outbound_payments
 			.send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params,
 				&self.router, self.list_usable_channels(), self.compute_inflight_htlcs(),
-				&self.entropy_source, &self.node_signer, best_block_height,
+				&self.entropy_source, &self.node_signer, best_block_height, &self.logger,
 				|path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
 				self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
 	}
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index d6e32ed2519..0ff7a33e9d8 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -378,22 +378,23 @@ impl OutboundPayments {
 		}
 	}
 
-	pub(super) fn send_payment<R: Deref, ES: Deref, NS: Deref, F>(
+	pub(super) fn send_payment<R: Deref, ES: Deref, NS: Deref, F, L: Deref>(
 		&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId,
 		retry_strategy: Retry, route_params: RouteParameters, router: &R,
 		first_hops: Vec<ChannelDetails>, inflight_htlcs: InFlightHtlcs, entropy_source: &ES,
-		node_signer: &NS, best_block_height: u32, send_payment_along_path: F
+		node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F,
 	) -> Result<(), PaymentSendFailure>
 	where
 		R::Target: Router,
 		ES::Target: EntropySource,
 		NS::Target: NodeSigner,
+		L::Target: Logger,
 		F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
 			 u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
 	{
 		self.pay_internal(payment_id, Some((payment_hash, payment_secret, retry_strategy)),
 			route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
-			best_block_height, &send_payment_along_path)
+			best_block_height, logger, &send_payment_along_path)
 			.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
 	}
 
@@ -470,24 +471,25 @@ impl OutboundPayments {
 			}
 			if let Some((payment_id, route_params)) = retry_id_route_params {
 				core::mem::drop(outbounds);
-				if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, &send_payment_along_path) {
-					log_trace!(logger, "Errored retrying payment: {:?}", e);
+				if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, logger, &send_payment_along_path) {
+					log_info!(logger, "Errored retrying payment: {:?}", e);
 				}
 			} else { break }
 		}
 	}
 
-	fn pay_internal<R: Deref, NS: Deref, ES: Deref, F>(
+	fn pay_internal<R: Deref, NS: Deref, ES: Deref, F, L: Deref>(
 		&self, payment_id: PaymentId,
 		initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Retry)>,
 		route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
 		inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
-		send_payment_along_path: &F
+		logger: &L, send_payment_along_path: &F,
 	) -> Result<(), PaymentSendFailure>
 	where
 		R::Target: Router,
 		ES::Target: EntropySource,
 		NS::Target: NodeSigner,
+		L::Target: Logger,
 		F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
 		   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
 	{
@@ -522,7 +524,9 @@ impl OutboundPayments {
 					} else { return res }
 				} else { return res }
 				core::mem::drop(outbounds);
-				self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path)
+				let retry_res = self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, send_payment_along_path);
+				log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res);
+				retry_res
 			},
 			Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. }) => {
 				let mut outbounds = self.pending_outbound_payments.lock().unwrap();
@@ -537,7 +541,8 @@ impl OutboundPayments {
 				// Some paths were sent, even if we failed to send the full MPP value our recipient may
 				// misbehave and claim the funds, at which point we have to consider the payment sent, so
 				// return `Ok()` here, ignoring any retry errors.
-				let _ = self.pay_internal(payment_id, None, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path);
+				let retry_res = self.pay_internal(payment_id, None, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, send_payment_along_path);
+				log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res);
 				Ok(())
 			},
 			Err(PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. }) => {
@@ -1164,11 +1169,12 @@ mod tests {
 		let err = if on_retry {
 			outbound_payments.pay_internal(
 				PaymentId([0; 32]), None, expired_route_params, &&router, vec![], InFlightHtlcs::new(),
-				&&keys_manager, &&keys_manager, 0, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
+				&&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
 		} else {
 			outbound_payments.send_payment(
 				PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), expired_route_params,
-				&&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
+				&&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+				|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
 		};
 		if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
 			assert!(err.contains("Invoice expired"));
@@ -1205,11 +1211,12 @@ mod tests {
 			&&keys_manager, 0).unwrap();
 			outbound_payments.pay_internal(
 				PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
-				&&keys_manager, &&keys_manager, 0, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
+				&&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
 		} else {
 			outbound_payments.send_payment(
 				PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params,
-				&&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
+				&&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+				|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
 		};
 		if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
 			assert!(err.contains("Failed to find a route"));

From 8af05e0172ab8176d1abe139521fda9d6efbed9a Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Fri, 27 Jan 2023 23:01:39 +0000
Subject: [PATCH 5/7] Move retry-limiting to `retry_payment_with_route`

The documentation for `Retry` is very clear that it counts the
number of failed paths, not discrete retries. When we added
retries internally in `ChannelManager`, we switched to counting
the number if discrete retries, even if multiple paths failed and
were replace with multiple MPP HTLCs.

Because we are now rewriting retries, we take this opportunity to
reduce the places where retries are analyzed, specifically a good
chunk of code is removed from `pay_internal`.

Because we now retry multiple failed paths with one single retry,
we keep the new behavior, updating the docs on `Retry` to describe
the new behavior.
---
 lightning/src/ln/channelmanager.rs   |  4 +-
 lightning/src/ln/outbound_payment.rs | 77 +++++++++++++++-------------
 lightning/src/ln/payment_tests.rs    | 23 ++++++---
 3 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 3b58610487f..59d1dbfc93f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2493,7 +2493,7 @@ where
 	#[cfg(test)]
 	pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId, route: &Route) -> Result<Vec<[u8; 32]>, PaymentSendFailure> {
 		let best_block_height = self.best_block.read().unwrap().height();
-		self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, Retry::Attempts(0), &self.entropy_source, best_block_height)
+		self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, None, &self.entropy_source, best_block_height)
 	}
 
 
@@ -7361,7 +7361,7 @@ where
 								hash_map::Entry::Vacant(entry) => {
 									let path_fee = path.get_path_fees();
 									entry.insert(PendingOutboundPayment::Retryable {
-										retry_strategy: Retry::Attempts(0),
+										retry_strategy: None,
 										attempts: PaymentAttempts::new(),
 										route_params: None,
 										session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 0ff7a33e9d8..08f600b2d8f 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -41,7 +41,7 @@ pub(crate) enum PendingOutboundPayment {
 		session_privs: HashSet<[u8; 32]>,
 	},
 	Retryable {
-		retry_strategy: Retry,
+		retry_strategy: Option<Retry>,
 		attempts: PaymentAttempts,
 		route_params: Option<RouteParameters>,
 		session_privs: HashSet<[u8; 32]>,
@@ -82,11 +82,25 @@ impl PendingOutboundPayment {
 			attempts.count += 1;
 		}
 	}
+	fn is_auto_retryable_now(&self) -> bool {
+		match self {
+			PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => {
+				strategy.is_retryable_now(&attempts)
+			},
+			_ => false,
+		}
+	}
 	fn is_retryable_now(&self) -> bool {
-		if let PendingOutboundPayment::Retryable { retry_strategy, attempts, .. } = self {
-			return retry_strategy.is_retryable_now(&attempts)
+		match self {
+			PendingOutboundPayment::Retryable { retry_strategy: None, .. } => {
+				// We're handling retries manually, we can always retry.
+				true
+			},
+			PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => {
+				strategy.is_retryable_now(&attempts)
+			},
+			_ => false,
 		}
-		false
 	}
 	pub fn insert_previously_failed_scid(&mut self, scid: u64) {
 		if let PendingOutboundPayment::Retryable { route_params: Some(params), .. } = self {
@@ -212,9 +226,9 @@ impl PendingOutboundPayment {
 pub enum Retry {
 	/// Max number of attempts to retry payment.
 	///
-	/// Note that this is the number of *path* failures, not full payment retries. For multi-path
-	/// payments, if this is less than the total number of paths, we will never even retry all of the
-	/// payment's paths.
+	/// Each attempt may be multiple HTLCs along multiple paths if the router decides to split up a
+	/// retry, and may retry multiple failed HTLCs at once if they failed around the same time and
+	/// were retried along a route from a single call to [`Router::find_route`].
 	Attempts(usize),
 	#[cfg(not(feature = "no-std"))]
 	/// Time elapsed before abandoning retries for a payment.
@@ -409,7 +423,7 @@ impl OutboundPayments {
 		F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
 		   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
 	{
-		let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
+		let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, None, None, entropy_source, best_block_height)?;
 		self.pay_route_internal(route, payment_hash, payment_secret, None, payment_id, None,
 			onion_session_privs, node_signer, best_block_height, &send_payment_along_path)
 			.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
@@ -430,7 +444,7 @@ impl OutboundPayments {
 			None => PaymentPreimage(entropy_source.get_secure_random_bytes()),
 		};
 		let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
-		let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
+		let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?;
 
 		match self.pay_route_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) {
 			Ok(()) => Ok(payment_hash),
@@ -459,11 +473,10 @@ impl OutboundPayments {
 			let mut outbounds = self.pending_outbound_payments.lock().unwrap();
 			let mut retry_id_route_params = None;
 			for (pmt_id, pmt) in outbounds.iter_mut() {
-				if pmt.is_retryable_now() {
+				if pmt.is_auto_retryable_now() {
 					if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, route_params: Some(params), .. } = pmt {
 						if pending_amt_msat < total_msat {
 							retry_id_route_params = Some((*pmt_id, params.clone()));
-							pmt.increment_attempts();
 							break
 						}
 					}
@@ -509,35 +522,21 @@ impl OutboundPayments {
 		}))?;
 
 		let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info {
-			let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, retry_strategy, Some(route_params.clone()), entropy_source, best_block_height)?;
+			let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, Some(retry_strategy), Some(route_params.clone()), entropy_source, best_block_height)?;
 			self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path)
 		} else {
 			self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path)
 		};
 		match res {
 			Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
-				let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-				if let Some(payment) = outbounds.get_mut(&payment_id) {
-					let retryable = payment.is_retryable_now();
-					if retryable {
-						payment.increment_attempts();
-					} else { return res }
-				} else { return res }
-				core::mem::drop(outbounds);
 				let retry_res = self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, send_payment_along_path);
 				log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res);
+				if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = &retry_res {
+					if err.starts_with("Retries exhausted ") { return res; }
+				}
 				retry_res
 			},
-			Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. }) => {
-				let mut outbounds = self.pending_outbound_payments.lock().unwrap();
-				if let Some(payment) = outbounds.get_mut(&payment_id) {
-					let retryable = payment.is_retryable_now();
-					if retryable {
-						payment.increment_attempts();
-					} else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) }
-				} else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) }
-				core::mem::drop(outbounds);
-
+			Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), .. }) => {
 				// Some paths were sent, even if we failed to send the full MPP value our recipient may
 				// misbehave and claim the funds, at which point we have to consider the payment sent, so
 				// return `Ok()` here, ignoring any retry errors.
@@ -611,6 +610,12 @@ impl OutboundPayments {
 							}));
 						},
 					};
+					if !payment.is_retryable_now() {
+						return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+							err: format!("Retries exhausted for payment id {}", log_bytes!(payment_id.0)),
+						}))
+					}
+					payment.increment_attempts();
 					for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
 						assert!(payment.insert(*session_priv_bytes, path));
 					}
@@ -646,7 +651,7 @@ impl OutboundPayments {
 		}
 
 		let route = Route { paths: vec![hops], payment_params: None };
-		let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
+		let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?;
 
 		match self.pay_route_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) {
 			Ok(()) => Ok((payment_hash, payment_id)),
@@ -660,14 +665,14 @@ impl OutboundPayments {
 	#[cfg(test)]
 	pub(super) fn test_add_new_pending_payment<ES: Deref>(
 		&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
-		route: &Route, retry_strategy: Retry, entropy_source: &ES, best_block_height: u32
+		route: &Route, retry_strategy: Option<Retry>, entropy_source: &ES, best_block_height: u32
 	) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
 		self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, retry_strategy, None, entropy_source, best_block_height)
 	}
 
 	pub(super) fn add_new_pending_payment<ES: Deref>(
 		&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
-		route: &Route, retry_strategy: Retry, route_params: Option<RouteParameters>,
+		route: &Route, retry_strategy: Option<Retry>, route_params: Option<RouteParameters>,
 		entropy_source: &ES, best_block_height: u32
 	) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
 		let mut onion_session_privs = Vec::with_capacity(route.paths.len());
@@ -969,7 +974,7 @@ 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();
+			let is_retryable_now = payment.get().is_auto_retryable_now();
 			if let Some(scid) = short_channel_id {
 				payment.get_mut().insert_previously_failed_scid(scid);
 			}
@@ -1110,7 +1115,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(0, session_privs, required),
 		(1, pending_fee_msat, option),
 		(2, payment_hash, required),
-		(not_written, retry_strategy, (static_value, Retry::Attempts(0))),
+		(not_written, retry_strategy, (static_value, None)),
 		(4, payment_secret, option),
 		(not_written, attempts, (static_value, PaymentAttempts::new())),
 		(6, total_msat, required),
@@ -1207,7 +1212,7 @@ mod tests {
 
 		let err = if on_retry {
 			outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]),
-			&Route { paths: vec![], payment_params: None }, Retry::Attempts(1), Some(route_params.clone()),
+			&Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)), Some(route_params.clone()),
 			&&keys_manager, 0).unwrap();
 			outbound_payments.pay_internal(
 				PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index c7c8198f6c4..b3acfc3209d 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -2251,7 +2251,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 				node_features: nodes[1].node.node_features(),
 				short_channel_id: chan_1_scid,
 				channel_features: nodes[1].node.channel_features(),
-				fee_msat: 0,
+				fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee
 				cltv_expiry_delta: 100,
 			}, RouteHop {
 				pubkey: nodes[2].node.get_our_node_id(),
@@ -2266,7 +2266,7 @@ fn no_extra_retries_on_back_to_back_fail() {
 				node_features: nodes[1].node.node_features(),
 				short_channel_id: chan_1_scid,
 				channel_features: nodes[1].node.channel_features(),
-				fee_msat: 0,
+				fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee
 				cltv_expiry_delta: 100,
 			}, RouteHop {
 				pubkey: nodes[2].node.get_our_node_id(),
@@ -2280,10 +2280,11 @@ fn no_extra_retries_on_back_to_back_fail() {
 		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
-	// On retry, we'll only be asked for one path
-	route.paths.remove(1);
 	let mut second_payment_params = route_params.payment_params.clone();
 	second_payment_params.previously_failed_channels = vec![chan_2_scid, chan_2_scid];
+	// On retry, we'll only return one path
+	route.paths.remove(1);
+	route.paths[0][1].fee_msat = amt_msat;
 	nodes[0].router.expect_find_route(RouteParameters {
 			payment_params: second_payment_params,
 			final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV,
@@ -2351,10 +2352,16 @@ fn no_extra_retries_on_back_to_back_fail() {
 
 	// At this point A has sent two HTLCs which both failed due to lack of fee. It now has two
 	// pending `PaymentPathFailed` events, one with `all_paths_failed` unset, and the second
-	// with it set. The first event will use up the only retry we are allowed, with the second
-	// `PaymentPathFailed` being passed up to the user (us, in this case). Previously, we'd
-	// treated this as "HTLC complete" and dropped the retry counter, causing us to retry again
-	// if the final HTLC failed.
+	// with it set.
+	//
+	// Previously, we retried payments in an event consumer, which would retry each
+	// `PaymentPathFailed` individually. In that setup, we had retried the payment in response to
+	// the first `PaymentPathFailed`, then seen the second `PaymentPathFailed` with
+	// `all_paths_failed` set and assumed the payment was completely failed. We ultimately fixed it
+	// by adding the `PaymentFailed` event.
+	//
+	// Because we now retry payments as a batch, we simply return a single-path route in the
+	// second, batched, request, have that fail, then complete the payment via `abandon_payment`.
 	let mut events = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events.len(), 4);
 	match events[0] {

From 071297234ac3490bcbdc30caa7f6768ecea9eca7 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Sat, 28 Jan 2023 02:14:26 +0000
Subject: [PATCH 6/7] Use only the failed amount when retrying payments, not
 the full amt

When we landed the initial in-`ChannelManager` payment retries, we
stored the `RouteParameters` in the payment info, and then re-use
it as-is for new payments. `RouteParameters` is intended to store
the information specific to the *route*, `PaymentParameters` exists
to store information specific to a payment.

Worse, because we don't recalculate the amount stored in the
`RouteParameters` before fetching a new route with it, we end up
attempting to retry the full payment amount, rather than only the
failed part.

This issue brought to you by having redundant data in
datastructures, part 5,001.
---
 lightning/src/ln/channelmanager.rs   |   2 +-
 lightning/src/ln/outbound_payment.rs |  80 ++++++++++----
 lightning/src/ln/payment_tests.rs    | 158 +++++++++++++++++++++++++++
 3 files changed, 217 insertions(+), 23 deletions(-)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 59d1dbfc93f..e9b7fb94930 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -7363,7 +7363,7 @@ where
 									entry.insert(PendingOutboundPayment::Retryable {
 										retry_strategy: None,
 										attempts: PaymentAttempts::new(),
-										route_params: None,
+										payment_params: None,
 										session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
 										payment_hash: htlc.payment_hash,
 										payment_secret,
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 08f600b2d8f..f745363036b 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -16,6 +16,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::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA;
 use crate::ln::msgs::DecodeError;
 use crate::ln::onion_utils::HTLCFailReason;
 use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router};
@@ -43,7 +44,7 @@ pub(crate) enum PendingOutboundPayment {
 	Retryable {
 		retry_strategy: Option<Retry>,
 		attempts: PaymentAttempts,
-		route_params: Option<RouteParameters>,
+		payment_params: Option<PaymentParameters>,
 		session_privs: HashSet<[u8; 32]>,
 		payment_hash: PaymentHash,
 		payment_secret: Option<PaymentSecret>,
@@ -102,9 +103,17 @@ impl PendingOutboundPayment {
 			_ => false,
 		}
 	}
+	fn payment_parameters(&mut self) -> Option<&mut PaymentParameters> {
+		match self {
+			PendingOutboundPayment::Retryable { payment_params: Some(ref mut params), .. } => {
+				Some(params)
+			},
+			_ => None,
+		}
+	}
 	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);
+		if let PendingOutboundPayment::Retryable { payment_params: Some(params), .. } = self {
+			params.previously_failed_channels.push(scid);
 		}
 	}
 	pub(super) fn is_fulfilled(&self) -> bool {
@@ -474,9 +483,18 @@ 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, route_params: Some(params), .. } = pmt {
+					if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, payment_params: Some(params), .. } = pmt {
 						if pending_amt_msat < total_msat {
-							retry_id_route_params = Some((*pmt_id, params.clone()));
+							retry_id_route_params = Some((*pmt_id, RouteParameters {
+								final_value_msat: *total_msat - *pending_amt_msat,
+								final_cltv_expiry_delta:
+									if let Some(delta) = params.final_cltv_expiry_delta { delta }
+									else {
+										debug_assert!(false, "We always set the final_cltv_expiry_delta when a path fails");
+										LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA.into()
+									},
+								payment_params: params.clone(),
+							}));
 							break
 						}
 					}
@@ -522,7 +540,7 @@ impl OutboundPayments {
 		}))?;
 
 		let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info {
-			let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, Some(retry_strategy), Some(route_params.clone()), entropy_source, best_block_height)?;
+			let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height)?;
 			self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path)
 		} else {
 			self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path)
@@ -672,7 +690,7 @@ impl OutboundPayments {
 
 	pub(super) fn add_new_pending_payment<ES: Deref>(
 		&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
-		route: &Route, retry_strategy: Option<Retry>, route_params: Option<RouteParameters>,
+		route: &Route, retry_strategy: Option<Retry>, payment_params: Option<PaymentParameters>,
 		entropy_source: &ES, best_block_height: u32
 	) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
 		let mut onion_session_privs = Vec::with_capacity(route.paths.len());
@@ -687,7 +705,7 @@ impl OutboundPayments {
 				let payment = entry.insert(PendingOutboundPayment::Retryable {
 					retry_strategy,
 					attempts: PaymentAttempts::new(),
-					route_params,
+					payment_params,
 					session_privs: HashSet::new(),
 					pending_amt_msat: 0,
 					pending_fee_msat: Some(0),
@@ -965,6 +983,7 @@ impl OutboundPayments {
 		let mut all_paths_failed = false;
 		let mut full_failure_ev = None;
 		let mut pending_retry_ev = None;
+		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));
@@ -978,6 +997,33 @@ impl OutboundPayments {
 			if let Some(scid) = short_channel_id {
 				payment.get_mut().insert_previously_failed_scid(scid);
 			}
+
+			// We want to move towards only using the `PaymentParameters` in the outbound payments
+			// map. However, for backwards-compatibility, we still need to support passing the
+			// `PaymentParameters` data that was shoved in the HTLC (and given to us via
+			// `payment_params`) back to the user.
+			let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
+			if let Some(params) = payment.get_mut().payment_parameters() {
+				if params.final_cltv_expiry_delta.is_none() {
+					// This should be rare, but a user could provide None for the payment data, and
+					// we need it when we go to retry the payment, so fill it in.
+					params.final_cltv_expiry_delta = Some(path_last_hop.cltv_expiry_delta);
+				}
+				retry = Some(RouteParameters {
+					payment_params: params.clone(),
+					final_value_msat: path_last_hop.fee_msat,
+					final_cltv_expiry_delta: params.final_cltv_expiry_delta.unwrap(),
+				});
+			} else if let Some(params) = payment_params {
+				retry = Some(RouteParameters {
+					payment_params: params.clone(),
+					final_value_msat: path_last_hop.fee_msat,
+					final_cltv_expiry_delta:
+						if let Some(delta) = params.final_cltv_expiry_delta { delta }
+						else { path_last_hop.cltv_expiry_delta },
+				});
+			}
+
 			if payment.get().remaining_parts() == 0 {
 				all_paths_failed = true;
 				if payment.get().abandoned() {
@@ -994,16 +1040,6 @@ impl OutboundPayments {
 			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");
-			Some(RouteParameters {
-				payment_params: payment_params_data.clone(),
-				final_value_msat: path_last_hop.fee_msat,
-				final_cltv_expiry_delta:
-					if let Some(delta) = payment_params_data.final_cltv_expiry_delta { delta }
-					else { path_last_hop.cltv_expiry_delta },
-			})
-		} else { None };
 		log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0));
 
 		let path_failure = {
@@ -1115,13 +1151,13 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
 		(0, session_privs, required),
 		(1, pending_fee_msat, option),
 		(2, payment_hash, required),
-		(not_written, retry_strategy, (static_value, None)),
+		(3, payment_params, option),
 		(4, payment_secret, option),
-		(not_written, attempts, (static_value, PaymentAttempts::new())),
 		(6, total_msat, required),
-		(not_written, route_params, (static_value, None)),
 		(8, pending_amt_msat, required),
 		(10, starting_block_height, required),
+		(not_written, retry_strategy, (static_value, None)),
+		(not_written, attempts, (static_value, PaymentAttempts::new())),
 	},
 	(3, Abandoned) => {
 		(0, session_privs, required),
@@ -1212,7 +1248,7 @@ mod tests {
 
 		let err = if on_retry {
 			outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]),
-			&Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)), Some(route_params.clone()),
+			&Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)), Some(route_params.payment_params.clone()),
 			&&keys_manager, 0).unwrap();
 			outbound_payments.pay_internal(
 				PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index b3acfc3209d..68074f1b59b 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -2417,3 +2417,161 @@ fn no_extra_retries_on_back_to_back_fail() {
 		_ => panic!("Unexpected event"),
 	}
 }
+
+#[test]
+fn test_simple_partial_retry() {
+	// In the first version of the in-`ChannelManager` payment retries, retries were sent for the
+	// full amount of the payment, rather than only the missing amount. Here we simply test for
+	// this by sending a payment with two parts, failing one, and retrying the second. Note that
+	// `TestRouter` will check that the `RouteParameters` (which contain the amount) matches the
+	// request.
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+	let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+	let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0).0.contents.short_channel_id;
+	let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0).0.contents.short_channel_id;
+
+	let amt_msat = 200_000_000;
+	let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[2], amt_msat);
+	#[cfg(feature = "std")]
+	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
+	#[cfg(not(feature = "std"))]
+	let payment_expiry_secs = 60 * 60;
+	let mut invoice_features = InvoiceFeatures::empty();
+	invoice_features.set_variable_length_onion_required();
+	invoice_features.set_payment_secret_required();
+	invoice_features.set_basic_mpp_optional();
+	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_features(invoice_features);
+	let route_params = RouteParameters {
+		payment_params,
+		final_value_msat: amt_msat,
+		final_cltv_expiry_delta: TEST_FINAL_CLTV,
+	};
+
+	let mut route = Route {
+		paths: vec![
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: nodes[1].node.node_features(),
+				short_channel_id: chan_1_scid,
+				channel_features: nodes[1].node.channel_features(),
+				fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee
+				cltv_expiry_delta: 100,
+			}, RouteHop {
+				pubkey: nodes[2].node.get_our_node_id(),
+				node_features: nodes[2].node.node_features(),
+				short_channel_id: chan_2_scid,
+				channel_features: nodes[2].node.channel_features(),
+				fee_msat: 100_000_000,
+				cltv_expiry_delta: 100,
+			}],
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: nodes[1].node.node_features(),
+				short_channel_id: chan_1_scid,
+				channel_features: nodes[1].node.channel_features(),
+				fee_msat: 100_000,
+				cltv_expiry_delta: 100,
+			}, RouteHop {
+				pubkey: nodes[2].node.get_our_node_id(),
+				node_features: nodes[2].node.node_features(),
+				short_channel_id: chan_2_scid,
+				channel_features: nodes[2].node.channel_features(),
+				fee_msat: 100_000_000,
+				cltv_expiry_delta: 100,
+			}]
+		],
+		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
+	};
+	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
+	let mut second_payment_params = route_params.payment_params.clone();
+	second_payment_params.previously_failed_channels = vec![chan_2_scid];
+	// On retry, we'll only be asked for one path (or 100k sats)
+	route.paths.remove(0);
+	nodes[0].router.expect_find_route(RouteParameters {
+			payment_params: second_payment_params,
+			final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV,
+		}, Ok(route.clone()));
+
+	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
+	let htlc_updates = SendEvent::from_node(&nodes[0]);
+	check_added_monitors!(nodes[0], 1);
+	assert_eq!(htlc_updates.msgs.len(), 1);
+
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &htlc_updates.msgs[0]);
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &htlc_updates.commitment_msg);
+	check_added_monitors!(nodes[1], 1);
+	let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa);
+	check_added_monitors!(nodes[0], 1);
+	let second_htlc_updates = SendEvent::from_node(&nodes[0]);
+
+	nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs);
+	check_added_monitors!(nodes[0], 1);
+	let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &second_htlc_updates.msgs[0]);
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &second_htlc_updates.commitment_msg);
+	check_added_monitors!(nodes[1], 1);
+	let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa);
+	check_added_monitors!(nodes[1], 1);
+	let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa);
+	check_added_monitors!(nodes[0], 1);
+
+	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]);
+	nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_fail_update.commitment_signed);
+	check_added_monitors!(nodes[0], 1);
+	let (as_second_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa);
+	check_added_monitors!(nodes[1], 1);
+
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs);
+	check_added_monitors!(nodes[1], 1);
+
+	let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
+	check_added_monitors!(nodes[0], 1);
+
+	let mut events = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 2);
+	match events[0] {
+		Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, ..  } => {
+			assert_eq!(payment_hash, ev_payment_hash);
+			assert_eq!(payment_failed_permanently, false);
+		},
+		_ => panic!("Unexpected event"),
+	}
+	match events[1] {
+		Event::PendingHTLCsForwardable { .. } => {},
+		_ => panic!("Unexpected event"),
+	}
+
+	nodes[0].node.process_pending_htlc_forwards();
+	let retry_htlc_updates = SendEvent::from_node(&nodes[0]);
+	check_added_monitors!(nodes[0], 1);
+
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &retry_htlc_updates.msgs[0]);
+	commitment_signed_dance!(nodes[1], nodes[0], &retry_htlc_updates.commitment_msg, false, true);
+
+	expect_pending_htlcs_forwardable!(nodes[1]);
+	check_added_monitors!(nodes[1], 1);
+
+	let bs_forward_update = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
+	nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &bs_forward_update.update_add_htlcs[0]);
+	nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &bs_forward_update.update_add_htlcs[1]);
+	commitment_signed_dance!(nodes[2], nodes[1], &bs_forward_update.commitment_signed, false);
+
+	expect_pending_htlcs_forwardable!(nodes[2]);
+	expect_payment_claimable!(nodes[2], payment_hash, payment_secret, amt_msat);
+}

From 076560062ba30ddf2604ebe10450ad934cf84bc8 Mon Sep 17 00:00:00 2001
From: Matt Corallo <git@bluematt.me>
Date: Sat, 28 Jan 2023 05:25:19 +0000
Subject: [PATCH 7/7] Reduce the code in the commitment_signed_dance macro

This should marginally reduce compile times for the tests by
reducing the total volume of code across the tests in the lightning
crate.
---
 lightning/src/ln/functional_test_utils.rs | 309 +++++++++++-----------
 1 file changed, 158 insertions(+), 151 deletions(-)

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index dc9522d3071..c2ce158f897 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1344,157 +1344,6 @@ impl SendEvent {
 	}
 }
 
-#[macro_export]
-/// Performs the "commitment signed dance" - the series of message exchanges which occur after a
-/// commitment update.
-macro_rules! commitment_signed_dance {
-	($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */) => {
-		{
-			check_added_monitors!($node_a, 0);
-			assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
-			$node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed);
-			check_added_monitors!($node_a, 1);
-			commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, false);
-		}
-	};
-	($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */, true /* return last RAA */) => {
-		{
-			let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!($node_a, $node_b.node.get_our_node_id());
-			check_added_monitors!($node_b, 0);
-			assert!($node_b.node.get_and_clear_pending_msg_events().is_empty());
-			$node_b.node.handle_revoke_and_ack(&$node_a.node.get_our_node_id(), &as_revoke_and_ack);
-			assert!($node_b.node.get_and_clear_pending_msg_events().is_empty());
-			check_added_monitors!($node_b, 1);
-			$node_b.node.handle_commitment_signed(&$node_a.node.get_our_node_id(), &as_commitment_signed);
-			let (bs_revoke_and_ack, extra_msg_option) = {
-				let events = $node_b.node.get_and_clear_pending_msg_events();
-				assert!(events.len() <= 2);
-				let (node_a_event, events) = remove_first_msg_event_to_node(&$node_a.node.get_our_node_id(), &events);
-				(match node_a_event {
-					MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
-						assert_eq!(*node_id, $node_a.node.get_our_node_id());
-						(*msg).clone()
-					},
-					_ => panic!("Unexpected event"),
-				}, events.get(0).map(|e| e.clone()))
-			};
-			check_added_monitors!($node_b, 1);
-			if $fail_backwards {
-				assert!($node_a.node.get_and_clear_pending_events().is_empty());
-				assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
-			}
-			(extra_msg_option, bs_revoke_and_ack)
-		}
-	};
-	($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */, false /* return extra message */, true /* return last RAA */) => {
-		{
-			check_added_monitors!($node_a, 0);
-			assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
-			$node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed);
-			check_added_monitors!($node_a, 1);
-			let (extra_msg_option, bs_revoke_and_ack) = commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true, true);
-			assert!(extra_msg_option.is_none());
-			bs_revoke_and_ack
-		}
-	};
-	($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */) => {
-		{
-			let (extra_msg_option, bs_revoke_and_ack) = commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true, true);
-			$node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack);
-			check_added_monitors!($node_a, 1);
-			extra_msg_option
-		}
-	};
-	($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, false /* no extra message */) => {
-		{
-			assert!(commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true).is_none());
-		}
-	};
-	($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => {
-		{
-			commitment_signed_dance!($node_a, $node_b, $commitment_signed, $fail_backwards, true);
-			if $fail_backwards {
-				expect_pending_htlcs_forwardable_and_htlc_handling_failed!($node_a, vec![$crate::util::events::HTLCDestination::NextHopChannel{ node_id: Some($node_b.node.get_our_node_id()), channel_id: $commitment_signed.channel_id }]);
-				check_added_monitors!($node_a, 1);
-
-				let node_a_per_peer_state = $node_a.node.per_peer_state.read().unwrap();
-				let mut number_of_msg_events = 0;
-				for (cp_id, peer_state_mutex) in node_a_per_peer_state.iter() {
-					let peer_state = peer_state_mutex.lock().unwrap();
-					let cp_pending_msg_events = &peer_state.pending_msg_events;
-					number_of_msg_events += cp_pending_msg_events.len();
-					if cp_pending_msg_events.len() == 1 {
-						if let MessageSendEvent::UpdateHTLCs { .. } = cp_pending_msg_events[0] {
-							assert_ne!(*cp_id, $node_b.node.get_our_node_id());
-						} else { panic!("Unexpected event"); }
-					}
-				}
-				// Expecting the failure backwards event to the previous hop (not `node_b`)
-				assert_eq!(number_of_msg_events, 1);
-			} else {
-				assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
-			}
-		}
-	}
-}
-
-/// Get a payment preimage and hash.
-#[macro_export]
-macro_rules! get_payment_preimage_hash {
-	($dest_node: expr) => {
-		{
-			get_payment_preimage_hash!($dest_node, None)
-		}
-	};
-	($dest_node: expr, $min_value_msat: expr) => {
-		{
-			crate::get_payment_preimage_hash!($dest_node, $min_value_msat, None)
-		}
-	};
-	($dest_node: expr, $min_value_msat: expr, $min_final_cltv_expiry_delta: expr) => {
-		{
-			use bitcoin::hashes::Hash as _;
-			let mut payment_count = $dest_node.network_payment_count.borrow_mut();
-			let payment_preimage = $crate::ln::PaymentPreimage([*payment_count; 32]);
-			*payment_count += 1;
-			let payment_hash = $crate::ln::PaymentHash(
-				bitcoin::hashes::sha256::Hash::hash(&payment_preimage.0[..]).into_inner());
-			let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, $min_final_cltv_expiry_delta).unwrap();
-			(payment_preimage, payment_hash, payment_secret)
-		}
-	};
-}
-
-#[macro_export]
-macro_rules! get_route {
-	($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
-		use $crate::chain::keysinterface::EntropySource;
-		let scorer = $crate::util::test_utils::TestScorer::with_penalty(0);
-		let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
-		let random_seed_bytes = keys_manager.get_secure_random_bytes();
-		$crate::routing::router::get_route(
-			&$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(),
-			Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
-			$recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes
-		)
-	}}
-}
-
-#[cfg(test)]
-#[macro_export]
-macro_rules! get_route_and_payment_hash {
-	($send_node: expr, $recv_node: expr, $recv_value: expr) => {{
-		let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id(), TEST_FINAL_CLTV)
-			.with_features($recv_node.node.invoice_features());
-		$crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV)
-	}};
-	($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
-		let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value));
-		let route = $crate::get_route!($send_node, $payment_params, $recv_value, $cltv);
-		(route.unwrap(), payment_hash, payment_preimage, payment_secret)
-	}}
-}
-
 #[macro_export]
 macro_rules! expect_pending_htlcs_forwardable_conditions {
 	($node: expr, $expected_failures: expr) => {{
@@ -1585,6 +1434,164 @@ macro_rules! expect_pending_htlcs_forwardable_from_events {
 		}
 	}}
 }
+
+#[macro_export]
+/// Performs the "commitment signed dance" - the series of message exchanges which occur after a
+/// commitment update.
+macro_rules! commitment_signed_dance {
+	($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */) => {
+		$crate::ln::functional_test_utils::do_commitment_signed_dance(&$node_a, &$node_b, &$commitment_signed, $fail_backwards, true);
+	};
+	($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */, true /* return last RAA */) => {
+		$crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards)
+	};
+	($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */, false /* return extra message */, true /* return last RAA */) => {
+		{
+			check_added_monitors!($node_a, 0);
+			assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
+			$node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed);
+			check_added_monitors!($node_a, 1);
+			let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards);
+			assert!(extra_msg_option.is_none());
+			bs_revoke_and_ack
+		}
+	};
+	($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */) => {
+		{
+			let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards);
+			$node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack);
+			check_added_monitors!($node_a, 1);
+			extra_msg_option
+		}
+	};
+	($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, false /* no extra message */) => {
+		assert!(commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true).is_none());
+	};
+	($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => {
+		$crate::ln::functional_test_utils::do_commitment_signed_dance(&$node_a, &$node_b, &$commitment_signed, $fail_backwards, false);
+	}
+}
+
+
+pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, fail_backwards: bool) -> (Option<MessageSendEvent>, msgs::RevokeAndACK) {
+	let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(node_a, node_b.node.get_our_node_id());
+	check_added_monitors!(node_b, 0);
+	assert!(node_b.node.get_and_clear_pending_msg_events().is_empty());
+	node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &as_revoke_and_ack);
+	assert!(node_b.node.get_and_clear_pending_msg_events().is_empty());
+	check_added_monitors!(node_b, 1);
+	node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &as_commitment_signed);
+	let (bs_revoke_and_ack, extra_msg_option) = {
+		let events = node_b.node.get_and_clear_pending_msg_events();
+		assert!(events.len() <= 2);
+		let (node_a_event, events) = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &events);
+		(match node_a_event {
+			MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
+				assert_eq!(*node_id, node_a.node.get_our_node_id());
+				(*msg).clone()
+			},
+			_ => panic!("Unexpected event"),
+		}, events.get(0).map(|e| e.clone()))
+	};
+	check_added_monitors!(node_b, 1);
+	if fail_backwards {
+		assert!(node_a.node.get_and_clear_pending_events().is_empty());
+		assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
+	}
+	(extra_msg_option, bs_revoke_and_ack)
+}
+
+pub fn do_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, commitment_signed: &msgs::CommitmentSigned, fail_backwards: bool, skip_last_step: bool) {
+	check_added_monitors!(node_a, 0);
+	assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
+	node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), commitment_signed);
+	check_added_monitors!(node_a, 1);
+
+	commitment_signed_dance!(node_a, node_b, (), fail_backwards, true, false);
+
+	if skip_last_step { return; }
+
+	if fail_backwards {
+		expect_pending_htlcs_forwardable_and_htlc_handling_failed!(node_a,
+			vec![crate::util::events::HTLCDestination::NextHopChannel{ node_id: Some(node_b.node.get_our_node_id()), channel_id: commitment_signed.channel_id }]);
+		check_added_monitors!(node_a, 1);
+
+		let node_a_per_peer_state = node_a.node.per_peer_state.read().unwrap();
+		let mut number_of_msg_events = 0;
+		for (cp_id, peer_state_mutex) in node_a_per_peer_state.iter() {
+			let peer_state = peer_state_mutex.lock().unwrap();
+			let cp_pending_msg_events = &peer_state.pending_msg_events;
+			number_of_msg_events += cp_pending_msg_events.len();
+			if cp_pending_msg_events.len() == 1 {
+				if let MessageSendEvent::UpdateHTLCs { .. } = cp_pending_msg_events[0] {
+					assert_ne!(*cp_id, node_b.node.get_our_node_id());
+				} else { panic!("Unexpected event"); }
+			}
+		}
+		// Expecting the failure backwards event to the previous hop (not `node_b`)
+		assert_eq!(number_of_msg_events, 1);
+	} else {
+		assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
+	}
+}
+
+/// Get a payment preimage and hash.
+#[macro_export]
+macro_rules! get_payment_preimage_hash {
+	($dest_node: expr) => {
+		{
+			get_payment_preimage_hash!($dest_node, None)
+		}
+	};
+	($dest_node: expr, $min_value_msat: expr) => {
+		{
+			crate::get_payment_preimage_hash!($dest_node, $min_value_msat, None)
+		}
+	};
+	($dest_node: expr, $min_value_msat: expr, $min_final_cltv_expiry_delta: expr) => {
+		{
+			use bitcoin::hashes::Hash as _;
+			let mut payment_count = $dest_node.network_payment_count.borrow_mut();
+			let payment_preimage = $crate::ln::PaymentPreimage([*payment_count; 32]);
+			*payment_count += 1;
+			let payment_hash = $crate::ln::PaymentHash(
+				bitcoin::hashes::sha256::Hash::hash(&payment_preimage.0[..]).into_inner());
+			let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, $min_final_cltv_expiry_delta).unwrap();
+			(payment_preimage, payment_hash, payment_secret)
+		}
+	};
+}
+
+#[macro_export]
+macro_rules! get_route {
+	($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
+		use $crate::chain::keysinterface::EntropySource;
+		let scorer = $crate::util::test_utils::TestScorer::with_penalty(0);
+		let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
+		let random_seed_bytes = keys_manager.get_secure_random_bytes();
+		$crate::routing::router::get_route(
+			&$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(),
+			Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
+			$recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes
+		)
+	}}
+}
+
+#[cfg(test)]
+#[macro_export]
+macro_rules! get_route_and_payment_hash {
+	($send_node: expr, $recv_node: expr, $recv_value: expr) => {{
+		let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id(), TEST_FINAL_CLTV)
+			.with_features($recv_node.node.invoice_features());
+		$crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV)
+	}};
+	($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
+		let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value));
+		let route = $crate::get_route!($send_node, $payment_params, $recv_value, $cltv);
+		(route.unwrap(), payment_hash, payment_preimage, payment_secret)
+	}}
+}
+
 #[macro_export]
 #[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
 macro_rules! expect_payment_claimable {