Have Route hold RouteParameters

Author: tnull

fuzz/src/chanmon_consistency.rs

@@ -376,7 +376,7 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
 			fee_msat: amt,
 			cltv_expiry_delta: 200,
 		}], blinded_tail: None }],
-		payment_params: None,
+		route_params: None,
 	}, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) {
 		check_payment_err(err, amt > max_value_sendable || amt < min_value_sendable);
 		false
@@ -409,15 +409,15 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
 			channel_features: middle.channel_features(),
 			fee_msat: first_hop_fee,
 			cltv_expiry_delta: 100,
-		},RouteHop {
+		}, RouteHop {
 			pubkey: dest.get_our_node_id(),
 			node_features: dest.node_features(),
 			short_channel_id: dest_chan_id,
 			channel_features: dest.channel_features(),
 			fee_msat: amt,
 			cltv_expiry_delta: 200,
 		}], blinded_tail: None }],
-		payment_params: None,
+		route_params: None,
 	}, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) {
 		let sent_amt = amt + first_hop_fee;
 		check_payment_err(err, sent_amt < min_value_sendable || sent_amt > max_value_sendable);

lightning/src/ln/functional_tests.rs

@@ -1049,7 +1049,9 @@ fn fake_network_test() {
 	});
 	hops[1].fee_msat = chan_4.1.contents.fee_base_msat as u64 + chan_4.1.contents.fee_proportional_millionths as u64 * hops[2].fee_msat as u64 / 1000000;
 	hops[0].fee_msat = chan_3.0.contents.fee_base_msat as u64 + chan_3.0.contents.fee_proportional_millionths as u64 * hops[1].fee_msat as u64 / 1000000;
-	let payment_preimage_1 = send_along_route(&nodes[1], Route { paths: vec![Path { hops, blinded_tail: None }], payment_params: None }, &vec!(&nodes[2], &nodes[3], &nodes[1])[..], 1000000).0;
+	let payment_preimage_1 = send_along_route(&nodes[1],
+		Route { paths: vec![Path { hops, blinded_tail: None }], route_params: None },
+			&vec!(&nodes[2], &nodes[3], &nodes[1])[..], 1000000).0;
 
 	let mut hops = Vec::with_capacity(3);
 	hops.push(RouteHop {
@@ -1078,7 +1080,9 @@ fn fake_network_test() {
 	});
 	hops[1].fee_msat = chan_2.1.contents.fee_base_msat as u64 + chan_2.1.contents.fee_proportional_millionths as u64 * hops[2].fee_msat as u64 / 1000000;
 	hops[0].fee_msat = chan_3.1.contents.fee_base_msat as u64 + chan_3.1.contents.fee_proportional_millionths as u64 * hops[1].fee_msat as u64 / 1000000;
-	let payment_hash_2 = send_along_route(&nodes[1], Route { paths: vec![Path { hops, blinded_tail: None }], payment_params: None }, &vec!(&nodes[3], &nodes[2], &nodes[1])[..], 1000000).1;
+	let payment_hash_2 = send_along_route(&nodes[1],
+		Route { paths: vec![Path { hops, blinded_tail: None }], route_params: None },
+			&vec!(&nodes[3], &nodes[2], &nodes[1])[..], 1000000).1;
 
 	// Claim the rebalances...
 	fail_payment(&nodes[1], &vec!(&nodes[3], &nodes[2], &nodes[1])[..], payment_hash_2);

lightning/src/ln/onion_utils.rs

@@ -1007,7 +1007,7 @@ mod tests {
 						short_channel_id: 0, fee_msat: 0, cltv_expiry_delta: 0 // We fill in the payloads manually instead of generating them from RouteHops.
 					},
 			], blinded_tail: None }],
-			payment_params: None,
+			route_params: None,
 		};
 
 		let onion_keys = super::construct_onion_keys(&secp_ctx, &route.paths[0], &get_test_session_key()).unwrap();

lightning/src/ln/outbound_payment.rs

@@ -976,7 +976,7 @@ impl OutboundPayments {
 			}))
 		}
 
-		let route = Route { paths: vec![path], payment_params: None };
+		let route = Route { paths: vec![path], route_params: None };
 		let onion_session_privs = self.add_new_pending_payment(payment_hash,
 			RecipientOnionFields::spontaneous_empty(), payment_id, None, &route, None, None,
 			entropy_source, best_block_height)?;
@@ -1145,9 +1145,9 @@ impl OutboundPayments {
 				results,
 				payment_id,
 				failed_paths_retry: if pending_amt_unsent != 0 {
-					if let Some(payment_params) = &route.payment_params {
+					if let Some(payment_params) = route.route_params.as_ref().map(|p| p.payment_params.clone()) {
 						Some(RouteParameters {
-							payment_params: payment_params.clone(),
+							payment_params: payment_params,
 							final_value_msat: pending_amt_unsent,
 						})
 					} else { None }
@@ -1569,7 +1569,7 @@ mod tests {
 		let pending_events = Mutex::new(VecDeque::new());
 		if on_retry {
 			outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(),
-				PaymentId([0; 32]), None, &Route { paths: vec![], payment_params: None },
+				PaymentId([0; 32]), None, &Route { paths: vec![], route_params: None },
 				Some(Retry::Attempts(1)), Some(expired_route_params.payment_params.clone()),
 				&&keys_manager, 0).unwrap();
 			outbound_payments.retry_payment_internal(
@@ -1613,7 +1613,7 @@ mod tests {
 		let pending_events = Mutex::new(VecDeque::new());
 		if on_retry {
 			outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), RecipientOnionFields::spontaneous_empty(),
-				PaymentId([0; 32]), None, &Route { paths: vec![], payment_params: None },
+				PaymentId([0; 32]), None, &Route { paths: vec![], route_params: None },
 				Some(Retry::Attempts(1)), Some(route_params.payment_params.clone()),
 				&&keys_manager, 0).unwrap();
 			outbound_payments.retry_payment_internal(
@@ -1657,7 +1657,7 @@ mod tests {
 				fee_msat: 0,
 				cltv_expiry_delta: 0,
 			}], blinded_tail: None }],
-			payment_params: Some(payment_params),
+			route_params: Some(route_params.clone()),
 		};
 		router.expect_find_route(route_params.clone(), Ok(route.clone()));
 		let mut route_params_w_failed_scid = route_params.clone();

lightning/src/ln/payment_tests.rs

@@ -94,7 +94,7 @@ fn mpp_retry() {
 
 	// Initiate the MPP payment.
 	let payment_id = PaymentId(payment_hash.0);
-	let mut route_params = RouteParameters::from_payment_params_and_value(route.payment_params.clone().unwrap(), amt_msat);
+	let mut route_params = route.route_params.clone().unwrap();
 
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
@@ -524,7 +524,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
 	let amt_msat = 1_000_000;
 	let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat);
 	let (payment_preimage_1, payment_hash_1, _, payment_id_1) = send_along_route(&nodes[0], route.clone(), &[&nodes[1], &nodes[2]], 1_000_000);
-	let route_params = RouteParameters::from_payment_params_and_value(route.payment_params.clone().unwrap(), amt_msat);
+	let route_params = route.route_params.unwrap().clone();
 	nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
 		PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
 	check_added_monitors!(nodes[0], 1);
@@ -2211,7 +2211,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None },
 		],
-		payment_params: Some(route_params.payment_params.clone()),
+		route_params: Some(route_params.clone()),
 	};
 	let retry_1_route = Route {
 		paths: vec![
@@ -2232,7 +2232,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None },
 		],
-		payment_params: Some(route_params.payment_params.clone()),
+		route_params: Some(route_params.clone()),
 	};
 	let retry_2_route = Route {
 		paths: vec![
@@ -2245,7 +2245,7 @@ fn auto_retry_partial_failure() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None },
 		],
-		payment_params: Some(route_params.payment_params.clone()),
+		route_params: Some(route_params.clone()),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route));
 	let mut payment_params = route_params.payment_params.clone();
@@ -2497,13 +2497,13 @@ fn retry_multi_path_single_failed_payment() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None },
 		],
-		payment_params: Some(payment_params),
+		route_params: Some(route_params.clone()),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	// On retry, split the payment across both channels.
 	route.paths[0].hops[0].fee_msat = 50_000_001;
 	route.paths[1].hops[0].fee_msat = 50_000_000;
-	let mut pay_params = route.payment_params.clone().unwrap();
+	let mut pay_params = route.route_params.clone().unwrap().payment_params;
 	pay_params.previously_failed_channels.push(chans[1].short_channel_id.unwrap());
 	nodes[0].router.expect_find_route(
 		// Note that the second request here requests the amount we originally failed to send,
@@ -2578,7 +2578,9 @@ fn immediate_retry_on_failure() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None },
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		route_params: Some(RouteParameters::from_payment_params_and_value(
+			PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV),
+			100_000_001)),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	// On retry, split the payment across both channels.
@@ -2684,7 +2686,9 @@ fn no_extra_retries_on_back_to_back_fail() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None }
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		route_params: Some(RouteParameters::from_payment_params_and_value(
+			PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV),
+			100_000_000)),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	let mut second_payment_params = route_params.payment_params.clone();
@@ -2882,7 +2886,9 @@ fn test_simple_partial_retry() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None }
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		route_params: Some(RouteParameters::from_payment_params_and_value(
+			PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV),
+			100_000_000)),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 	let mut second_payment_params = route_params.payment_params.clone();
@@ -3044,7 +3050,9 @@ fn test_threaded_payment_retries() {
 				cltv_expiry_delta: 100,
 			}], blinded_tail: None }
 		],
-		payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
+		route_params: Some(RouteParameters::from_payment_params_and_value(
+			PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV),
+			amt_msat - amt_msat / 1000)),
 	};
 	nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
 
@@ -3470,7 +3478,7 @@ fn test_retry_custom_tlvs() {
 
 	// Initiate the payment
 	let payment_id = PaymentId(payment_hash.0);
-	let mut route_params = RouteParameters::from_payment_params_and_value(route.payment_params.clone().unwrap(), amt_msat);
+	let mut route_params = route.route_params.clone().unwrap();
 
 	let custom_tlvs = vec![((1 << 16) + 1, vec![0x42u8; 16])];
 	let onion_fields = RecipientOnionFields::secret_only(payment_secret);

lightning/src/routing/router.rs

@@ -337,10 +337,12 @@ pub struct Route {
 	/// [`BlindedTail`]s are present, then the pubkey of the last [`RouteHop`] in each path must be
 	/// the same.
 	pub paths: Vec<Path>,
-	/// The `payment_params` parameter passed via [`RouteParameters`] to [`find_route`].
+	/// The `route_params` parameter passed to [`find_route`].
 	///
 	/// This is used by `ChannelManager` to track information which may be required for retries.
-	pub payment_params: Option<PaymentParameters>,
+	///
+	/// Will be `None` for objects serialized with LDK versions prior to 0.0.117.
+	pub route_params: Option<RouteParameters>,
 }
 
 impl Route {
@@ -383,8 +385,11 @@ impl Writeable for Route {
 			}
 		}
 		write_tlv_fields!(writer, {
-			(1, self.payment_params, option),
+			// For compatibility with LDK versions prior to 0.0.117, we take the individual
+			// RouteParameters' fields and reconstruct them on read.
+			(1, self.route_params.as_ref().map(|p| &p.payment_params), option),
 			(2, blinded_tails, optional_vec),
+			(3, self.route_params.as_ref().map(|p| p.final_value_msat), option),
 		});
 		Ok(())
 	}
@@ -411,6 +416,7 @@ impl Readable for Route {
 		_init_and_read_len_prefixed_tlv_fields!(reader, {
 			(1, payment_params, (option: ReadableArgs, min_final_cltv_expiry_delta)),
 			(2, blinded_tails, optional_vec),
+			(3, final_value_msat, option),
 		});
 		let blinded_tails = blinded_tails.unwrap_or(Vec::new());
 		if blinded_tails.len() != 0 {
@@ -419,14 +425,23 @@ impl Readable for Route {
 				path.blinded_tail = blinded_tail_opt;
 			}
 		}
-		Ok(Route { paths, payment_params })
+
+		// If we previously wrote the corresponding fields, reconstruct RouteParameters.
+		let route_params = match (payment_params, final_value_msat) {
+			(Some(payment_params), Some(final_value_msat)) => {
+				Some(RouteParameters { payment_params, final_value_msat })
+			}
+			_ => None,
+		};
+
+		Ok(Route { paths, route_params })
 	}
 }
 
 /// Parameters needed to find a [`Route`].
 ///
 /// Passed to [`find_route`] and [`build_route_from_hops`].
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct RouteParameters {
 	/// The parameters of the failed payment path.
 	pub payment_params: PaymentParameters,
@@ -2489,7 +2504,7 @@ where L::Target: Logger {
 		}
 	}
 
-	let route = Route { paths, payment_params: Some(payment_params.clone()) };
+	let route = Route { paths, route_params: Some(route_params.clone()) };
 	log_info!(logger, "Got route: {}", log_route!(route));
 	Ok(route)
 }
@@ -5953,7 +5968,7 @@ mod tests {
 					short_channel_id: 0, fee_msat: 225, cltv_expiry_delta: 0
 				},
 			], blinded_tail: None }],
-			payment_params: None,
+			route_params: None,
 		};
 
 		assert_eq!(route.get_total_fees(), 250);
@@ -5986,7 +6001,7 @@ mod tests {
 					short_channel_id: 0, fee_msat: 150, cltv_expiry_delta: 0
 				},
 			], blinded_tail: None }],
-			payment_params: None,
+			route_params: None,
 		};
 
 		assert_eq!(route.get_total_fees(), 200);
@@ -5998,7 +6013,7 @@ mod tests {
 		// In an earlier version of `Route::get_total_fees` and `Route::get_total_amount`, they
 		// would both panic if the route was completely empty. We test to ensure they return 0
 		// here, even though its somewhat nonsensical as a route.
-		let route = Route { paths: Vec::new(), payment_params: None };
+		let route = Route { paths: Vec::new(), route_params: None };
 
 		assert_eq!(route.get_total_fees(), 0);
 		assert_eq!(route.get_total_amount(), 0);
@@ -6597,7 +6612,7 @@ mod tests {
 				fee_msat: 100,
 				cltv_expiry_delta: 0,
 			}], blinded_tail: None }],
-			payment_params: None,
+			route_params: None,
 		};
 		let encoded_route = route.encode();
 		let decoded_route: Route = Readable::read(&mut Cursor::new(&encoded_route[..])).unwrap();
@@ -6691,7 +6706,7 @@ mod tests {
 				excess_final_cltv_expiry_delta: 0,
 				final_value_msat: 200,
 			}),
-		}], payment_params: None};
+		}], route_params: None};
 
 		let payment_params = PaymentParameters::from_node_id(ln_test_utils::pubkey(47), 18);
 		let (_, network_graph, _, _, _) = build_line_graph();

pending_changelog/routes_route_params.txt

@@ -0,0 +1,3 @@
+# Backwards Compatibility
+
+- `Route` objects written with LDK versions prior to 0.0.117 won't be retryable after being deserialized with LDK 0.0.117 or above.