Public commit record
58/100 · Thin
What the developer wrote
Authored by Valentine Wallace
Require Route::route_params
This field has always been set since 0.0.117. Since we're making it required
here, routes created/serialized prior to 0.0.117 will fail to deserialize on
0.4.
✓ Descriptive subject✓ Provides detailed explanatory context
What changed, and why it matters
This commit makes a piece of routing data called route_params mandatory in the Lightning Dev Kit library. Previously it was optional, and the code would silently invent a dummy value if it was missing. Now it must always be provided, and old serialized routes written before version 0.0.117 will fail to load. This is a deliberate backwards-compatibility break, not a security fix, and it reduces the risk of inconsistent or missing routing information rather than introducing a vulnerability.
Recommended action
Treat as a normal API/compatability change. Review downstream code that constructs Route manually or deserializes old routes; update callers to always populate route_params and avoid relying on pre-0.0.117 serialized routes. No urgent security patch is indicated.
Security signals we found
01
API hardening: removes silent fallback to dummy RouteParameters
02
Backwards-compatibility break for pre-0.0.117 serialized Route objects
03
No new bounds checks, cryptographic operations, or network parsing changes
04
No mention of vulnerability, CVE, or security advisory in commit message or diff
Evidence from the diff
The commit changes Route::route_params from Option
Changed components
lightning/src/routing/router.rs (Route struct and serialization)lightning/src/ln/channelmanager.rs (send_payment_with_route)lightning/src/ln/outbound_payment.rs (route validation and probes)lightning/src/ln/payment_tests.rs and other test filesInspect captured patch +229 / −252
diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index d829157..b0703d8 100644
--- a/fuzz/src/chanmon_consistency.rs
+++ b/fuzz/src/chanmon_consistency.rs
@@ -1866,7 +1866,7 @@ impl PaymentTracker {
}],
blinded_tail: None,
}],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
let onion = RecipientOnionFields::secret_only(secret, amt);
let res = source.send_payment_with_route(route, hash, onion, id);
@@ -1946,7 +1946,7 @@ impl PaymentTracker {
],
blinded_tail: None,
}],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
let onion = RecipientOnionFields::secret_only(secret, amt);
let res = source.send_payment_with_route(route, hash, onion, id);
@@ -2028,7 +2028,7 @@ impl PaymentTracker {
PaymentParameters::from_node_id(dest.get_our_node_id(), TEST_FINAL_CLTV),
amt,
);
- let route = Route { paths, route_params: Some(route_params) };
+ let route = Route { paths, route_params };
let onion = RecipientOnionFields::secret_only(secret, amt);
let res = source.send_payment_with_route(route, hash, onion, id);
let succeeded = match res {
@@ -2132,7 +2132,7 @@ impl PaymentTracker {
PaymentParameters::from_node_id(dest.get_our_node_id(), TEST_FINAL_CLTV),
amt,
);
- let route = Route { paths, route_params: Some(route_params) };
+ let route = Route { paths, route_params };
let onion = RecipientOnionFields::secret_only(secret, amt);
let res = source.send_payment_with_route(route, hash, onion, id);
let succeeded = match res {
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index 32c0709..b4de379 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -2093,6 +2093,7 @@ fn test_trampoline_forward_payload_encoded_as_receive() {
let bob_carol_scid = nodes[1].node().list_channels().iter().find(|c| c.channel_id == chan_id_bob_carol).unwrap().short_channel_id.unwrap();
let amt_msat = 1000;
+ let carol_cltv_expiry_delta = 24 + 39;
let (payment_preimage, payment_hash, _) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None);
// We need the session priv to construct an invalid onion packet later.
@@ -2148,7 +2149,7 @@ fn test_trampoline_forward_payload_encoded_as_receive() {
short_channel_id: bob_carol_scid,
channel_features: ChannelFeatures::empty(),
fee_msat: 0,
- cltv_expiry_delta: 24 + 39,
+ cltv_expiry_delta: carol_cltv_expiry_delta,
maybe_announced_channel: false,
}
],
@@ -2159,7 +2160,7 @@ fn test_trampoline_forward_payload_encoded_as_receive() {
pubkey: carol_node_id,
node_features: Features::empty(),
fee_msat: amt_msat,
- cltv_expiry_delta: 24 + 39,
+ cltv_expiry_delta: carol_cltv_expiry_delta,
},
],
hops: carol_blinded_hops,
@@ -2168,7 +2169,10 @@ fn test_trampoline_forward_payload_encoded_as_receive() {
final_value_msat: amt_msat,
})
}],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(
+ PaymentParameters::from_node_id(carol_node_id, carol_cltv_expiry_delta),
+ amt_msat,
+ ),
};
nodes[0].node.send_payment_with_route(route.clone(), payment_hash, RecipientOnionFields::spontaneous_empty(amt_msat), PaymentId(payment_hash.0)).unwrap();
@@ -2279,6 +2283,7 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
let bob_carol_scid = nodes[1].node().list_channels().iter().find(|c| c.channel_id == chan_id_bob_carol).unwrap().short_channel_id.unwrap();
let amt_msat = 1000;
+ let carol_cltv_expiry_delta = 104 + 39;
let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None);
// Create a 1-hop blinded path for Carol.
@@ -2314,7 +2319,7 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
short_channel_id: bob_carol_scid,
channel_features: ChannelFeatures::empty(),
fee_msat: 0,
- cltv_expiry_delta: 104 + 39,
+ cltv_expiry_delta: carol_cltv_expiry_delta,
maybe_announced_channel: false,
}
],
@@ -2325,7 +2330,7 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
pubkey: carol_node_id,
node_features: Features::empty(),
fee_msat: amt_msat,
- cltv_expiry_delta: 104 + 39,
+ cltv_expiry_delta: carol_cltv_expiry_delta,
},
],
hops: blinded_path.blinded_hops().to_vec(),
@@ -2334,7 +2339,10 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
final_value_msat: amt_msat,
})
}],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(
+ PaymentParameters::from_node_id(carol_node_id, carol_cltv_expiry_delta),
+ amt_msat,
+ ),
};
nodes[0].node.send_payment_with_route(route.clone(), payment_hash, RecipientOnionFields::spontaneous_empty(amt_msat), PaymentId(payment_hash.0)).unwrap();
@@ -2618,7 +2626,13 @@ fn do_test_trampoline_relay(blinded: bool, test_case: TrampolineTestCase) {
original_amt_msat,
)),
}],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(
+ PaymentParameters::from_node_id(
+ carol_node_id,
+ original_trampoline_cltv + excess_final_cltv,
+ ),
+ original_amt_msat,
+ ),
};
nodes[0]
@@ -2753,6 +2767,7 @@ fn test_trampoline_forward_rejection() {
let bob_carol_scid = nodes[1].node().list_channels().iter().find(|c| c.channel_id == chan_id_bob_carol).unwrap().short_channel_id.unwrap();
let amt_msat = 1000;
+ let carol_cltv_expiry_delta = 24 + 24 + 39;
let (payment_preimage, payment_hash, _) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None);
let route = Route {
@@ -2776,7 +2791,7 @@ fn test_trampoline_forward_rejection() {
short_channel_id: bob_carol_scid,
channel_features: ChannelFeatures::empty(),
fee_msat: 0,
- cltv_expiry_delta: 24 + 24 + 39,
+ cltv_expiry_delta: carol_cltv_expiry_delta,
maybe_announced_channel: false,
}
],
@@ -2808,7 +2823,10 @@ fn test_trampoline_forward_rejection() {
final_value_msat: amt_msat,
})
}],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(
+ PaymentParameters::from_node_id(carol_node_id, carol_cltv_expiry_delta),
+ amt_msat,
+ ),
};
nodes[0].node.send_payment_with_route(route.clone(), payment_hash, RecipientOnionFields::spontaneous_empty(amt_msat), PaymentId(payment_hash.0)).unwrap();
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 9633800..81062ea 100644
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -2315,7 +2315,7 @@ fn test_path_paused_mpp() {
route.paths[1].hops[0].pubkey = node_c_id;
route.paths[1].hops[0].short_channel_id = chan_2_ann.contents.short_channel_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;
- route.route_params.as_mut().unwrap().final_value_msat *= 2;
+ route.route_params.final_value_msat *= 2;
// Set it so that the first monitor update (for the path 0 -> 1 -> 3) succeeds, but the second
// (for the path 0 -> 2 -> 3) fails.
@@ -4315,7 +4315,7 @@ fn do_test_partial_claim_mon_update_compl_actions(reload_a: bool, reload_b: bool
route.paths[1].hops[0].pubkey = node_c_id;
route.paths[1].hops[0].short_channel_id = chan_2_scid;
route.paths[1].hops[1].short_channel_id = chan_4_scid;
- route.route_params.as_mut().unwrap().final_value_msat *= 2;
+ route.route_params.final_value_msat *= 2;
let paths = &[&[&nodes[1], &nodes[3]][..], &[&nodes[2], &nodes[3]][..]];
send_along_route_with_secret(&nodes[0], route, paths, 200_000, payment_hash, payment_secret);
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 2b8b3e8..5eb2146 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -5632,31 +5632,12 @@ impl<
/// LDK will not automatically retry this payment, though it may be manually re-sent after an
/// [`Event::PaymentFailed`] is generated.
pub fn send_payment_with_route(
- &self, mut route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
+ &self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
payment_id: PaymentId,
) -> Result<(), RetryableSendFailure> {
let best_block_height = self.best_block.read().unwrap().height;
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
- let route_params = route.route_params.clone().unwrap_or_else(|| {
- // Create a dummy route params since they're a required parameter but unused in this case
- let (payee_node_id, cltv_delta) = route
- .paths
- .first()
- .and_then(|path| {
- path.hops.last().map(|hop| (hop.pubkey, hop.cltv_expiry_delta as u32))
- })
- .unwrap_or_else(|| {
- (PublicKey::from_slice(&[2; 33]).unwrap(), MIN_FINAL_CLTV_EXPIRY_DELTA as u32)
- });
- let dummy_payment_params = PaymentParameters::from_node_id(payee_node_id, cltv_delta);
- RouteParameters::from_payment_params_and_value(
- dummy_payment_params,
- route.get_total_amount(),
- )
- });
- if route.route_params.is_none() {
- route.route_params = Some(route_params.clone());
- }
+ let route_params = route.route_params.clone();
let router = FixedRouter::new(route);
let logger =
WithContext::for_payment(&self.logger, None, None, Some(payment_hash), payment_id);
@@ -21131,7 +21112,7 @@ mod tests {
// Next, send a keysend payment with the same payment_hash and make sure it fails.
nodes[0].node.send_spontaneous_payment(
Some(payment_preimage), RecipientOnionFields::spontaneous_empty(100_000),
- PaymentId(payment_preimage.0), route.route_params.clone().unwrap(), Retry::Attempts(0)
+ PaymentId(payment_preimage.0), route.route_params.clone(), Retry::Attempts(0)
).unwrap();
check_added_monitors(&nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -21287,7 +21268,7 @@ mod tests {
).unwrap();
let payment_hash = nodes[0].node.send_spontaneous_payment(
Some(payment_preimage), RecipientOnionFields::spontaneous_empty(100_000),
- PaymentId(payment_preimage.0), route.route_params.clone().unwrap(), Retry::Attempts(0)
+ PaymentId(payment_preimage.0), route.route_params.clone(), Retry::Attempts(0)
).unwrap();
check_added_monitors(&nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -21330,7 +21311,7 @@ mod tests {
let payment_id_1 = PaymentId([44; 32]);
let payment_hash = nodes[0].node.send_spontaneous_payment(
Some(payment_preimage), RecipientOnionFields::spontaneous_empty(100_000), payment_id_1,
- route.route_params.clone().unwrap(), Retry::Attempts(0)
+ route.route_params.clone(), Retry::Attempts(0)
).unwrap();
check_added_monitors(&nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -21448,7 +21429,7 @@ mod tests {
route.paths[1].hops[0].pubkey = nodes[2].node.get_our_node_id();
route.paths[1].hops[0].short_channel_id = chan_2_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;
- route.route_params.as_mut().unwrap().final_value_msat *= 2;
+ route.route_params.final_value_msat *= 2;
nodes[0].node.send_payment_with_route(route, payment_hash,
RecipientOnionFields::spontaneous_empty(200000), PaymentId(payment_hash.0)).unwrap();
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 82c1c61..6e855c2 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -3492,14 +3492,14 @@ pub fn send_along_route_with_secret<'a, 'b, 'c>(
recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret,
) -> PaymentId {
let payment_id = PaymentId(origin_node.keys_manager.backing.get_secure_random_bytes());
- origin_node.router.expect_find_route(route.route_params.clone().unwrap(), Ok(route.clone()));
+ origin_node.router.expect_find_route(route.route_params.clone(), Ok(route.clone()));
origin_node
.node
.send_payment(
our_payment_hash,
RecipientOnionFields::secret_only(our_payment_secret, recv_value),
payment_id,
- route.route_params.unwrap(),
+ route.route_params,
Retry::Attempts(0),
)
.unwrap();
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 52e2f2e..826b077 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -164,7 +164,7 @@ pub fn fake_network_test() {
let route_params = RouteParameters::from_payment_params_and_value(payment_params, 1000000);
let route = Route {
paths: vec![Path { hops, blinded_tail: None }],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
let path: &[_] = &[&nodes[2], &nodes[3], &nodes[1]];
let payment_preimage_1 = send_along_route(&nodes[1], route, path, 1000000).0;
@@ -202,8 +202,7 @@ pub fn fake_network_test() {
+ 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 route =
- Route { paths: vec![Path { hops, blinded_tail: None }], route_params: Some(route_params) };
+ let route = Route { paths: vec![Path { hops, blinded_tail: None }], route_params };
let path: &[_] = &[&nodes[3], &nodes[2], &nodes[1]];
let payment_hash_2 = send_along_route(&nodes[1], route, path, 1000000).1;
@@ -7214,7 +7213,7 @@ pub fn test_simple_mpp() {
route.paths[1].hops[0].pubkey = node_c_id;
route.paths[1].hops[0].short_channel_id = chan_2_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;
- route.route_params.as_mut().unwrap().final_value_msat = 200_000;
+ route.route_params.final_value_msat = 200_000;
let paths: &[&[_]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]];
send_along_route_with_secret(&nodes[0], route, paths, 200_000, payment_hash, payment_secret);
claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], paths, payment_preimage));
diff --git a/lightning/src/ln/max_payment_path_len_tests.rs b/lightning/src/ln/max_payment_path_len_tests.rs
index c066f2c..242ba80 100644
--- a/lightning/src/ln/max_payment_path_len_tests.rs
+++ b/lightning/src/ln/max_payment_path_len_tests.rs
@@ -110,7 +110,7 @@ fn large_payment_metadata() {
custom_tlvs: Vec::new(),
total_mpp_amount_msat: amt_msat,
};
- let route_params = route_0_1.route_params.clone().unwrap();
+ let route_params = route_0_1.route_params.clone();
let id = PaymentId(payment_hash.0);
nodes[0]
.node
@@ -138,14 +138,14 @@ fn large_payment_metadata() {
let (payment_hash_2, _, payment_secret_2, encrypted_metadata_2) =
get_payment_hash!(nodes[2], payment_metadata.clone());
let (mut route_0_2, ..) = get_route_and_payment_hash!(&nodes[0], &nodes[2], amt_msat);
- let mut route_params_0_2 = route_0_2.route_params.clone().unwrap();
+ let mut route_params_0_2 = route_0_2.route_params.clone();
route_params_0_2.payment_params.max_path_length = 1;
nodes[0].router.expect_find_route_query(route_params_0_2);
max_sized_onion.payment_secret = Some(payment_secret_2);
max_sized_onion.payment_metadata = Some(encrypted_metadata_2);
let id = PaymentId(payment_hash_2.0);
- let mut route_params = route_0_2.route_params.clone().unwrap();
+ let mut route_params = route_0_2.route_params.clone();
let err = nodes[0]
.node
.send_payment(payment_hash_2, max_sized_onion.clone(), id, route_params, Retry::Attempts(0))
@@ -184,7 +184,7 @@ fn large_payment_metadata() {
_ => panic!(),
}
- let route_params = route_0_1.route_params.clone().unwrap();
+ let route_params = route_0_1.route_params.clone();
let err = nodes[0]
.node
.send_payment(payment_hash_2, too_large_onion, id, route_params, Retry::Attempts(0))
@@ -202,10 +202,10 @@ fn large_payment_metadata() {
custom_tlvs: Vec::new(),
total_mpp_amount_msat: amt_msat,
};
- let mut route_params_0_2 = route_0_2.route_params.clone().unwrap();
+ let mut route_params_0_2 = route_0_2.route_params.clone();
route_params_0_2.payment_params.max_path_length = 2;
nodes[0].router.expect_find_route_query(route_params_0_2);
- let route_params = route_0_2.route_params.unwrap();
+ let route_params = route_0_2.route_params;
nodes[0]
.node
.send_payment(payment_hash_2, onion_allowing_2_hops, id, route_params, Retry::Attempts(0))
diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index 234af58..d4ee3d3 100644
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -3034,7 +3034,7 @@ mod tests {
use crate::ln::channelmanager::PaymentId;
use crate::ln::msgs::{self, UpdateFailHTLC};
use crate::ln::types::ChannelId;
- use crate::routing::router::{Path, PaymentParameters, Route, RouteHop};
+ use crate::routing::router::{Path, PaymentParameters, Route, RouteHop, RouteParameters};
use crate::types::features::{ChannelFeatures, NodeFeatures};
use crate::types::payment::PaymentHash;
use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -3142,7 +3142,10 @@ mod tests {
let secp_ctx = Secp256k1::new();
let path = build_test_path();
- let route = Route { paths: vec![path], route_params: None };
+ let payment_params = PaymentParameters::from_node_id(path.hops.last().unwrap().pubkey, 0);
+ let route_params =
+ RouteParameters::from_payment_params_and_value(payment_params, path.final_value_msat());
+ let route = Route { paths: vec![path], route_params };
let onion_keys =
super::construct_onion_keys(&secp_ctx, &route.paths[0], &get_test_session_key());
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 748dfe1..6bbe43e 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -985,7 +985,7 @@ impl OutboundPayments {
fn validate_found_route<L: Logger>(
route: &mut Route, route_params: &RouteParameters, logger: &WithContext<L>,
) -> Result<(), ()> {
- if route.route_params.as_ref() != Some(route_params) {
+ if route.route_params != *route_params {
debug_assert!(
false,
"Routers are expected to return a Route which includes the requested RouteParameters. Got {:?}, expected {route_params:?}",
@@ -996,7 +996,7 @@ fn validate_found_route<L: Logger>(
"Routers are expected to return a Route which includes the requested RouteParameters. Got {:?}, expected {route_params:?}",
route.route_params
);
- route.route_params = Some(route_params.clone());
+ route.route_params = route_params.clone();
}
route.debug_assert_route_meets_params(logger)?;
@@ -1923,7 +1923,20 @@ impl OutboundPayments {
}))
}
- let route = Route { paths: vec![path], route_params: None };
+ // `route_params` is a required field, but is unused when sending a probe along a fixed
+ // path. Construct dummy parameters from the path, leaving the fee budget unset to match
+ // the previous behavior of not tracking one for probes.
+ let route_params = {
+ let last_hop = path.hops.last().unwrap();
+ let payment_params =
+ PaymentParameters::from_node_id(last_hop.pubkey, last_hop.cltv_expiry_delta);
+ RouteParameters {
+ payment_params,
+ final_value_msat: path.final_value_msat(),
+ max_total_routing_fee_msat: None,
+ }
+ };
+ let route = Route { paths: vec![path], route_params };
let recipient_onion_fields =
RecipientOnionFields::secret_only(payment_secret, route.get_total_amount());
let onion_session_privs = self.add_new_pending_payment(payment_hash,
@@ -2036,8 +2049,7 @@ impl OutboundPayments {
starting_block_height: best_block_height,
total_msat: route.get_total_amount(),
onion_total_msat: recipient_onion.total_mpp_amount_msat,
- remaining_max_total_routing_fee_msat:
- route.route_params.as_ref().and_then(|p| p.max_total_routing_fee_msat),
+ remaining_max_total_routing_fee_msat: route.route_params.max_total_routing_fee_msat,
};
for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
@@ -2204,19 +2216,17 @@ impl OutboundPayments {
results,
payment_id,
failed_paths_retry: if has_unsent {
- if let Some(route_params) = &route.route_params {
- let mut route_params = route_params.clone();
- // We calculate the leftover fee budget we're allowed to spend by
- // subtracting the used fee from the total fee budget.
- route_params.max_total_routing_fee_msat = route_params
- .max_total_routing_fee_msat.map(|m| m.saturating_sub(total_ok_fees_msat));
-
- // We calculate the remaining target amount by subtracting the succeded
- // path values.
- route_params.final_value_msat = route_params.final_value_msat
- .saturating_sub(total_ok_amt_sent_msat);
- Some(route_params)
- } else { None }
+ let mut route_params = route.route_params.clone();
+ // We calculate the leftover fee budget we're allowed to spend by
+ // subtracting the used fee from the total fee budget.
+ route_params.max_total_routing_fee_msat = route_params
+ .max_total_routing_fee_msat.map(|m| m.saturating_sub(total_ok_fees_msat));
+
+ // We calculate the remaining target amount by subtracting the succeded
+ // path values.
+ route_params.final_value_msat = route_params.final_value_msat
+ .saturating_sub(total_ok_amt_sent_msat);
+ Some(route_params)
} else { None },
})
} else if has_err {
@@ -2954,7 +2964,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(0),
- PaymentId([0; 32]), None, &Route { paths: vec![], route_params: None },
+ PaymentId([0; 32]), None, &Route { paths: vec![], route_params: expired_route_params.clone() },
Some(Retry::Attempts(1)), Some(expired_route_params.payment_params.clone()),
&&keys_manager, 0, None).unwrap();
outbound_payments.find_route_and_send_payment(
@@ -3000,7 +3010,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(0),
- PaymentId([0; 32]), None, &Route { paths: vec![], route_params: None },
+ PaymentId([0; 32]), None, &Route { paths: vec![], route_params: route_params.clone() },
Some(Retry::Attempts(1)), Some(route_params.payment_params.clone()),
&&keys_manager, 0, None).unwrap();
outbound_payments.find_route_and_send_payment(
@@ -3048,13 +3058,13 @@ mod tests {
cltv_expiry_delta: 0,
maybe_announced_channel: true,
}], blinded_tail: None }],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
router.expect_find_route(route_params.clone(), Ok(route.clone()));
let mut route_params_w_failed_scid = route_params.clone();
route_params_w_failed_scid.payment_params.previously_failed_channels.push(failed_scid);
let mut route_w_failed_scid = route.clone();
- route_w_failed_scid.route_params = Some(route_params_w_failed_scid.clone());
+ route_w_failed_scid.route_params = route_params_w_failed_scid.clone();
router.expect_find_route(route_params_w_failed_scid, Ok(route_w_failed_scid));
router.expect_find_route(route_params.clone(), Ok(route.clone()));
router.expect_find_route(route_params.clone(), Ok(route.clone()));
@@ -3418,7 +3428,7 @@ mod tests {
blinded_tail: None,
}
],
- route_params: Some(route_params),
+ route_params,
})
);
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 7768491..e86245a 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -96,7 +96,7 @@ fn mpp_failure() {
route.paths[1].hops[0].pubkey = node_c_id;
route.paths[1].hops[0].short_channel_id = chan_2_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;
- route.route_params.as_mut().unwrap().final_value_msat *= 2;
+ route.route_params.final_value_msat *= 2;
let paths: &[&[_]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]];
send_along_route_with_secret(&nodes[0], route, paths, 200_000, payment_hash, payment_secret);
@@ -138,11 +138,11 @@ fn mpp_retry() {
route.paths[1].hops[0].pubkey = node_c_id;
route.paths[1].hops[0].short_channel_id = chan_2_update.contents.short_channel_id;
route.paths[1].hops[1].short_channel_id = chan_4_update.contents.short_channel_id;
- route.route_params.as_mut().unwrap().final_value_msat *= 2;
+ route.route_params.final_value_msat *= 2;
// Initiate the MPP payment.
let id = PaymentId(hash.0);
- let mut route_params = route.route_params.clone().unwrap();
+ let mut route_params = route.route_params.clone();
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
let onion = RecipientOnionFields::secret_only(pay_secret, amt_msat * 2);
@@ -192,7 +192,7 @@ fn mpp_retry() {
// Check the remaining max total routing fee for the second attempt is 50_000 - 1_000 msat fee
// used by the first path
route_params.max_total_routing_fee_msat = Some(max_fee - 1_000);
- route.route_params = Some(route_params.clone());
+ route.route_params = route_params.clone();
nodes[0].router.expect_find_route(route_params, Ok(route));
expect_and_process_pending_htlcs(&nodes[0], false);
check_added_monitors(&nodes[0], 1);
@@ -265,7 +265,7 @@ fn mpp_retry_overpay() {
// Initiate the payment.
let id = PaymentId(hash.0);
- let mut route_params = route.route_params.clone().unwrap();
+ let mut route_params = route.route_params.clone();
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
let onion = RecipientOnionFields::secret_only(pay_secret, amt_msat);
@@ -321,7 +321,7 @@ fn mpp_retry_overpay() {
// base fee, but not for overpaid value of the first try.
route_params.max_total_routing_fee_msat.as_mut().map(|m| *m -= 1000);
- route.route_params = Some(route_params.clone());
+ route.route_params = route_params.clone();
nodes[0].router.expect_find_route(route_params, Ok(route));
nodes[0].node.process_pending_htlc_forwards();
@@ -373,12 +373,12 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool, keysend: bool) {
route.paths[1].hops[0].pubkey = node_c_id;
route.paths[1].hops[0].short_channel_id = chan_2_update.contents.short_channel_id;
route.paths[1].hops[1].short_channel_id = chan_4_update.contents.short_channel_id;
- route.route_params.as_mut().unwrap().final_value_msat *= 2;
+ route.route_params.final_value_msat *= 2;
// Initiate the MPP payment.
let onion = RecipientOnionFields::secret_only(payment_secret, 200_000);
if keysend {
- let route_params = route.route_params.clone().unwrap();
+ let route_params = route.route_params.clone();
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
nodes[0]
.node
@@ -668,8 +668,8 @@ fn test_reject_mpp_keysend_htlc_mismatching_secret() {
route.paths[0].hops[1].short_channel_id = chan_3_id;
let payment_id_0 = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes());
- nodes[0].router.expect_find_route(route.route_params.clone().unwrap(), Ok(route.clone()));
- let params = route.route_params.clone().unwrap();
+ nodes[0].router.expect_find_route(route.route_params.clone(), Ok(route.clone()));
+ let params = route.route_params.clone();
let onion = RecipientOnionFields::spontaneous_empty(amount);
let retry = Retry::Attempts(0);
nodes[0].node.send_spontaneous_payment(preimage, onion, payment_id_0, params, retry).unwrap();
@@ -716,10 +716,10 @@ fn test_reject_mpp_keysend_htlc_mismatching_secret() {
route.paths[0].hops[1].short_channel_id = chan_4_id;
let payment_id_1 = PaymentId(nodes[0].keys_manager.backing.get_secure_random_bytes());
- nodes[0].router.expect_find_route(route.route_params.clone().unwrap(), Ok(route.clone()));
+ nodes[0].router.expect_find_route(route.route_params.clone(), Ok(route.clone()));
let onion = RecipientOnionFields::spontaneous_empty(amount);
- let params = route.route_params.clone().unwrap();
+ let params = route.route_params.clone();
let retry = Retry::Attempts(0);
nodes[0].node.send_spontaneous_payment(preimage, onion, payment_id_1, params, retry).unwrap();
check_added_monitors(&nodes[0], 1);
@@ -859,7 +859,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
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 = route.route_params.unwrap().clone();
+ let route_params = route.route_params.clone();
let onion = RecipientOnionFields::secret_only(payment_secret, amt_msat);
let id = PaymentId(payment_hash.0);
nodes[0].node.send_payment(payment_hash, onion, id, route_params, Retry::Attempts(1)).unwrap();
@@ -2138,7 +2138,7 @@ fn claimed_send_payment_idempotent() {
None,
RecipientOnionFields::spontaneous_empty(100_000),
payment_id,
- route.route_params.clone().unwrap(),
+ route.route_params.clone(),
Retry::Attempts(0),
);
match send_result {
@@ -2221,7 +2221,7 @@ fn abandoned_send_payment_idempotent() {
None,
RecipientOnionFields::spontaneous_empty(100_000),
payment_id,
- route.route_params.clone().unwrap(),
+ route.route_params.clone(),
Retry::Attempts(0),
);
match send_result {
@@ -3224,7 +3224,7 @@ fn auto_retry_partial_failure() {
blinded_tail: None,
},
],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route));
@@ -3262,7 +3262,7 @@ fn auto_retry_partial_failure() {
blinded_tail: None,
},
],
- route_params: Some(retry_1_params.clone()),
+ route_params: retry_1_params.clone(),
};
nodes[0].router.expect_find_route(retry_1_params.clone(), Ok(retry_1_route));
@@ -3286,7 +3286,7 @@ fn auto_retry_partial_failure() {
}],
blinded_tail: None,
}],
- route_params: Some(retry_2_params.clone()),
+ route_params: retry_2_params.clone(),
};
nodes[0].router.expect_find_route(retry_2_params, Ok(retry_2_route));
@@ -3449,7 +3449,7 @@ fn auto_retry_zero_attempts_send_error() {
}],
blinded_tail: None,
}],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route));
@@ -3587,18 +3587,18 @@ fn retry_multi_path_single_failed_payment() {
blinded_tail: None,
},
],
- route_params: Some(route_params.clone()),
+ route_params: 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.route_params.clone().unwrap().payment_params;
+ let mut pay_params = route.route_params.clone().payment_params;
pay_params.previously_failed_channels.push(chans[1].short_channel_id.unwrap());
let mut retry_params = RouteParameters::from_payment_params_and_value(pay_params, 100_000_000);
retry_params.max_total_routing_fee_msat = None;
- route.route_params = Some(retry_params.clone());
+ route.route_params = retry_params.clone();
nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
{
@@ -3694,7 +3694,7 @@ fn immediate_retry_on_failure() {
}],
blinded_tail: None,
}],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
// On retry, split the payment across both channels.
@@ -3705,7 +3705,7 @@ fn immediate_retry_on_failure() {
let mut pay_params = route_params.payment_params.clone();
pay_params.previously_failed_channels.push(chans[0].short_channel_id.unwrap());
let retry_params = RouteParameters::from_payment_params_and_value(pay_params, amt_msat);
- route.route_params = Some(retry_params.clone());
+ route.route_params = retry_params.clone();
nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
let onion = RecipientOnionFields::secret_only(payment_secret, amt_msat);
@@ -3829,9 +3829,9 @@ fn no_extra_retries_on_back_to_back_fail() {
blinded_tail: None,
},
],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
- route.route_params.as_mut().unwrap().max_total_routing_fee_msat = None;
+ route.route_params.max_total_routing_fee_msat = None;
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, chan_2_scid];
@@ -3841,7 +3841,7 @@ fn no_extra_retries_on_back_to_back_fail() {
let mut retry_params =
RouteParameters::from_payment_params_and_value(second_payment_params, amt_msat);
retry_params.max_total_routing_fee_msat = None;
- route.route_params = Some(retry_params.clone());
+ route.route_params = retry_params.clone();
nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
// We can't use the commitment_signed_dance macro helper because in this test we'll be sending
@@ -4074,7 +4074,7 @@ fn test_simple_partial_retry() {
blinded_tail: None,
},
],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
@@ -4086,7 +4086,7 @@ fn test_simple_partial_retry() {
let mut retry_params =
RouteParameters::from_payment_params_and_value(second_payment_params, amt_msat / 2);
retry_params.max_total_routing_fee_msat = None;
- route.route_params = Some(retry_params.clone());
+ route.route_params = retry_params.clone();
nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
// We can't use the commitment_signed_dance macro helper because in this test we'll be sending
@@ -4290,7 +4290,7 @@ fn test_threaded_payment_retries() {
blinded_tail: None,
},
],
- route_params: Some(route_params.clone()),
+ route_params: route_params.clone(),
};
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
@@ -4313,7 +4313,7 @@ fn test_threaded_payment_retries() {
// from here on out, the retry `RouteParameters` amount will be amt/1000
route_params.final_value_msat /= 1000;
- route.route_params = Some(route_params.clone());
+ route.route_params = route_params.clone();
route.paths.pop();
let end_time = Instant::now() + Duration::from_secs(1);
@@ -4367,7 +4367,7 @@ fn test_threaded_payment_retries() {
previously_failed_channels.clone();
new_route_params.max_total_routing_fee_msat.as_mut().map(|m| *m -= 100_000);
route.paths[0].hops[1].short_channel_id += 1;
- route.route_params = Some(new_route_params.clone());
+ route.route_params = new_route_params.clone();
nodes[0].router.expect_find_route(new_route_params, Ok(route.clone()));
let bs_fail_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
@@ -4773,7 +4773,7 @@ fn do_test_custom_tlvs(spontaneous: bool, even_tlvs: bool, known_tlvs: bool) {
total_mpp_amount_msat: amt_msat,
};
if spontaneous {
- let params = route.route_params.unwrap();
+ let params = route.route_params;
let retry = Retry::Attempts(0);
nodes[0].node.send_spontaneous_payment(Some(preimage), onion, id, params, retry).unwrap();
} else {
@@ -4848,7 +4848,7 @@ fn test_retry_custom_tlvs() {
// Initiate the payment
let id = PaymentId(hash.0);
- let mut route_params = route.route_params.clone().unwrap();
+ let mut route_params = route.route_params.clone();
let custom_tlvs = vec![((1 << 16) + 1, vec![0x42u8; 16])];
let onion = RecipientOnionFields::secret_only(payment_secret, amt_msat);
@@ -4888,7 +4888,7 @@ fn test_retry_custom_tlvs() {
// Retry the payment and make sure it succeeds
let chan_2_scid = chan_2_update.contents.short_channel_id;
route_params.payment_params.previously_failed_channels.push(chan_2_scid);
- route.route_params = Some(route_params.clone());
+ route.route_params = route_params.clone();
nodes[0].router.expect_find_route(route_params, Ok(route));
nodes[0].node.process_pending_htlc_forwards();
check_added_monitors(&nodes[0], 1);
@@ -5603,7 +5603,7 @@ fn remove_pending_outbounds_on_buggy_router() {
// Extend the path by itself, essentially simulating route going through same channel twice
let cloned_hops = route.paths[0].hops.clone();
route.paths[0].hops.extend_from_slice(&cloned_hops);
- let route_params = route.route_params.clone().unwrap();
+ let route_params = route.route_params.clone();
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
// Send the payment with one retry allowed, but the payment should still fail
@@ -5662,40 +5662,6 @@ fn remove_pending_outbound_probe_on_buggy_path() {
assert!(nodes[0].node.list_recent_payments().is_empty());
}
-#[test]
-fn pay_route_without_params() {
- // Make sure we can use ChannelManager::send_payment_with_route to pay a route where
- // Route::route_parameters is None.
- 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]);
- let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
- let node_b_id = nodes[1].node.get_our_node_id();
-
- create_announced_chan_between_nodes(&nodes, 0, 1);
-
- let amt_msat = 10_000;
- let payment_params = PaymentParameters::from_node_id(node_b_id, TEST_FINAL_CLTV)
- .with_bolt11_features(nodes[1].node.bolt11_invoice_features())
- .unwrap();
- let (mut route, hash, preimage, payment_secret) =
- get_route_and_payment_hash!(nodes[0], nodes[1], payment_params, amt_msat);
- route.route_params.take();
-
- let onion = RecipientOnionFields::secret_only(payment_secret, amt_msat);
- let id = PaymentId(hash.0);
- nodes[0].node.send_payment_with_route(route, hash, onion, id).unwrap();
-
- check_added_monitors(&nodes[0], 1);
- let mut events = nodes[0].node.get_and_clear_pending_msg_events();
- assert_eq!(events.len(), 1);
- let node_1_msgs = remove_first_msg_event_to_node(&node_b_id, &mut events);
- let path = &[&nodes[1]];
- pass_along_path(&nodes[0], path, amt_msat, hash, Some(payment_secret), node_1_msgs, true, None);
- claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], &[path], preimage));
-}
-
#[test]
fn max_out_mpp_path() {
// In this setup, the sender is attempting to route an MPP payment split across the two channels
@@ -5984,7 +5950,7 @@ fn bolt11_multi_node_mpp_with_retry() {
// First route for A: same path but with fee_msat=0 at C to trigger a forwarding failure
let mut first_route = route.clone();
first_route.paths[0].hops[0].fee_msat = 0;
- first_route.route_params = Some(route_params.clone());
+ first_route.route_params = route_params.clone();
nodes[0].router.expect_find_route(route_params.clone(), Ok(first_route));
// Retry route for A: the natural route with correct fees (will succeed)
@@ -5995,7 +5961,7 @@ fn bolt11_multi_node_mpp_with_retry() {
payment_params: retry_payment_params,
max_total_routing_fee_msat: route_params.max_total_routing_fee_msat,
};
- route.route_params = Some(retry_route_params.clone());
+ route.route_params = retry_route_params.clone();
nodes[0].router.expect_find_route(retry_route_params, Ok(route));
// Node A pays 60_000 msat (part of the total) with retry enabled
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 01889b2..1c7c132 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -682,10 +682,7 @@ pub struct Route {
/// The `route_params` parameter passed to [`find_route`].
///
/// This is used by `ChannelManager` to track information which may be required for retries.
- ///
- /// Will be `None` for objects serialized with LDK versions prior to 0.0.117. This field will
- /// soon move to being required and must always be set.
- pub route_params: Option<RouteParameters>,
+ pub route_params: RouteParameters,
}
impl Route {
@@ -698,8 +695,8 @@ impl Route {
/// [`htlc_minimum_msat`]: https://github.com/lightning/bolts/blob/master/07-routing-gossip.md#the-channel_update-message
#[rustfmt::skip]
pub fn get_total_fees(&self) -> u64 {
- let overpaid_value_msat = self.route_params.as_ref()
- .map_or(0, |p| self.get_total_amount().saturating_sub(p.final_value_msat));
+ let overpaid_value_msat =
+ self.get_total_amount().saturating_sub(self.route_params.final_value_msat);
overpaid_value_msat + self.paths.iter().map(|path| path.fee_msat()).sum::<u64>()
}
@@ -714,105 +711,102 @@ impl Route {
}
pub(crate) fn debug_assert_route_meets_params<L: Logger>(&self, logger: L) -> Result<(), ()> {
- if let Some(route_params) = self.route_params.as_ref() {
- // Check that we actually pay less than the max fee we set.
- if let Some(max_total_fee) = route_params.max_total_routing_fee_msat {
- let total_fee = self.get_total_fees();
- if total_fee > max_total_fee {
- let err = format!("Router returned an attempt to pay with a higher fee ({total_fee}msat) than we allowed ({max_total_fee}msat). Your router is critically buggy!");
- debug_assert!(false, "{}", err);
- log_error!(logger, "{}", err);
- return Err(());
- }
+ let route_params = &self.route_params;
+ // Check that we actually pay less than the max fee we set.
+ if let Some(max_total_fee) = route_params.max_total_routing_fee_msat {
+ let total_fee = self.get_total_fees();
+ if total_fee > max_total_fee {
+ let err = format!("Router returned an attempt to pay with a higher fee ({total_fee}msat) than we allowed ({max_total_fee}msat). Your router is critically buggy!");
+ debug_assert!(false, "{}", err);
+ log_error!(logger, "{}", err);
+ return Err(());
}
+ }
+
+ if self.paths.is_empty() {
+ let err = "Selected route had no paths. Your router is buggy!";
+ debug_assert!(false, "{}", err);
+ log_error!(logger, "{}", err);
+ return Err(());
+ }
- if self.paths.is_empty() {
- let err = "Selected route had no paths. Your router is buggy!";
+ for path in self.paths.iter() {
+ if path.hops.is_empty() {
+ let err = "Unusable path in route (path.hops.len() must be at least 1)";
debug_assert!(false, "{}", err);
log_error!(logger, "{}", err);
return Err(());
}
- for path in self.paths.iter() {
- if path.hops.is_empty() {
- let err = "Unusable path in route (path.hops.len() must be at least 1)";
+ let total_cltv_delta = path.total_cltv_expiry_delta();
+ if total_cltv_delta > route_params.payment_params.max_total_cltv_expiry_delta {
+ let err = format!(
+ "Path had a total CLTV of {total_cltv_delta} which is greater than the maximum we're allowed {}",
+ route_params.payment_params.max_total_cltv_expiry_delta,
+ );
+ debug_assert!(false, "{}", err);
+ log_error!(logger, "{}", err);
+ return Err(());
+ }
+
+ if path.hops.len() > route_params.payment_params.max_path_length.into() {
+ let err = format!(
+ "Path had a length of {}, which is greater than the maximum we're allowed ({})",
+ path.hops.len(),
+ route_params.payment_params.max_path_length,
+ );
+ #[cfg(any(test, feature = "_test_utils"))]
+ debug_assert!(false, "{}", err);
+ log_error!(logger, "{}", err);
+ // This is a bug, but there's not a material safety risk to making this
+ // payment, so we don't bother to error here.
+ }
+
+ if let Some(tail) = &path.blinded_tail {
+ let trampoline_cltv_sum: u32 =
+ tail.trampoline_hops.iter().map(|hop| hop.cltv_expiry_delta).sum();
+ let last_hop_cltv_delta = path.hops.last().unwrap().cltv_expiry_delta;
+ if !tail.trampoline_hops.is_empty() && trampoline_cltv_sum != last_hop_cltv_delta {
+ let err = format!(
+ "Path had a total trampoline CLTV of {trampoline_cltv_sum}, which is not equal to the total last-hop CLTV delta of {last_hop_cltv_delta}"
+ );
debug_assert!(false, "{}", err);
log_error!(logger, "{}", err);
- return Err(());
}
-
- let total_cltv_delta = path.total_cltv_expiry_delta();
- if total_cltv_delta > route_params.payment_params.max_total_cltv_expiry_delta {
+ let last_trampoline_cltv_opt =
+ tail.trampoline_hops.last().map(|h| h.cltv_expiry_delta);
+ let last_trampoline_cltv = last_trampoline_cltv_opt.unwrap_or(u32::MAX);
+ if tail.excess_final_cltv_expiry_delta > last_trampoline_cltv {
let err = format!(
- "Path had a total CLTV of {total_cltv_delta} which is greater than the maximum we're allowed {}",
- route_params.payment_params.max_total_cltv_expiry_delta,
+ "Last trampoline CLTV of {last_trampoline_cltv} is less than the excess blinded path cltv of {}",
+ tail.excess_final_cltv_expiry_delta
);
debug_assert!(false, "{}", err);
log_error!(logger, "{}", err);
- return Err(());
}
-
- if path.hops.len() > route_params.payment_params.max_path_length.into() {
+ if tail.excess_final_cltv_expiry_delta > last_hop_cltv_delta {
let err = format!(
- "Path had a length of {}, which is greater than the maximum we're allowed ({})",
- path.hops.len(),
- route_params.payment_params.max_path_length,
+ "Last path hop CLTV of {last_hop_cltv_delta} is less than the excess blinded path cltv of {}",
+ tail.excess_final_cltv_expiry_delta
);
- #[cfg(any(test, feature = "_test_utils"))]
debug_assert!(false, "{}", err);
log_error!(logger, "{}", err);
- // This is a bug, but there's not a material safety risk to making this
- // payment, so we don't bother to error here.
- }
-
- if let Some(tail) = &path.blinded_tail {
- let trampoline_cltv_sum: u32 =
- tail.trampoline_hops.iter().map(|hop| hop.cltv_expiry_delta).sum();
- let last_hop_cltv_delta = path.hops.last().unwrap().cltv_expiry_delta;
- if !tail.trampoline_hops.is_empty()
- && trampoline_cltv_sum != last_hop_cltv_delta
- {
- let err = format!(
- "Path had a total trampoline CLTV of {trampoline_cltv_sum}, which is not equal to the total last-hop CLTV delta of {last_hop_cltv_delta}"
- );
- debug_assert!(false, "{}", err);
- log_error!(logger, "{}", err);
- }
- let last_trampoline_cltv_opt =
- tail.trampoline_hops.last().map(|h| h.cltv_expiry_delta);
- let last_trampoline_cltv = last_trampoline_cltv_opt.unwrap_or(u32::MAX);
- if tail.excess_final_cltv_expiry_delta > last_trampoline_cltv {
- let err = format!(
- "Last trampoline CLTV of {last_trampoline_cltv} is less than the excess blinded path cltv of {}",
- tail.excess_final_cltv_expiry_delta
- );
- debug_assert!(false, "{}", err);
- log_error!(logger, "{}", err);
- }
- if tail.excess_final_cltv_expiry_delta > last_hop_cltv_delta {
- let err = format!(
- "Last path hop CLTV of {last_hop_cltv_delta} is less than the excess blinded path cltv of {}",
- tail.excess_final_cltv_expiry_delta
- );
- debug_assert!(false, "{}", err);
- log_error!(logger, "{}", err);
- }
}
}
+ }
- // Test that we don't contain any "extra" MPP parts - while we're allowed to overshoot
- // the `final_value_msat` specified in the `route_params`, we aren't allowed to have
- // any MPP parts which aren't needed to meet `route_params.final_value_msat`.
- let min_mpp_part = self.paths.iter().map(|h| h.final_value_msat()).min().unwrap_or(0);
- if self.get_total_amount() - min_mpp_part >= route_params.final_value_msat {
- let err = format!(
- "Router returned an attempt to include more MPP parts than needed. The smallest MPP part ({min_mpp_part}msat) was not needed for a payment of {}msat. Your router is critically buggy!",
- route_params.final_value_msat
- );
- debug_assert!(false, "{}", err);
- log_error!(logger, "{}", err);
- return Err(());
- }
+ // Test that we don't contain any "extra" MPP parts - while we're allowed to overshoot
+ // the `final_value_msat` specified in the `route_params`, we aren't allowed to have
+ // any MPP parts which aren't needed to meet `route_params.final_value_msat`.
+ let min_mpp_part = self.paths.iter().map(|h| h.final_value_msat()).min().unwrap_or(0);
+ if self.get_total_amount() - min_mpp_part >= route_params.final_value_msat {
+ let err = format!(
+ "Router returned an attempt to include more MPP parts than needed. The smallest MPP part ({min_mpp_part}msat) was not needed for a payment of {}msat. Your router is critically buggy!",
+ route_params.final_value_msat
+ );
+ debug_assert!(false, "{}", err);
+ log_error!(logger, "{}", err);
+ return Err(());
}
Ok(())
@@ -850,12 +844,10 @@ impl Writeable for Route {
} else if !blinded_tails.is_empty() { blinded_tails.push(None); }
}
write_tlv_fields!(writer, {
- // 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),
+ (1, self.route_params.payment_params, required),
(2, blinded_tails, optional_vec),
- (3, self.route_params.as_ref().map(|p| p.final_value_msat), option),
- (5, self.route_params.as_ref().and_then(|p| p.max_total_routing_fee_msat), option),
+ (3, self.route_params.final_value_msat, required),
+ (5, self.route_params.max_total_routing_fee_msat, option),
});
Ok(())
}
@@ -881,9 +873,9 @@ impl Readable for Route {
paths.push(Path { hops, blinded_tail: None });
}
_init_and_read_len_prefixed_tlv_fields!(reader, {
- (1, payment_params, (option: ReadableArgs, min_final_cltv_expiry_delta)),
+ (1, payment_params, (required: ReadableArgs, min_final_cltv_expiry_delta)),
(2, blinded_tails, optional_vec),
- (3, final_value_msat, option),
+ (3, final_value_msat, required),
(5, max_total_routing_fee_msat, option)
});
let blinded_tails = blinded_tails.unwrap_or(Vec::new());
@@ -894,12 +886,10 @@ impl Readable for Route {
}
}
- // If we previously wrote the corresponding fields, reconstruct RouteParameters.
- let route_params = match (payment_params, final_value_msat) {
- (Some(payment_params), Some(final_value_msat)) => {
- Some(RouteParameters { payment_params, final_value_msat, max_total_routing_fee_msat })
- }
- _ => None,
+ let route_params = RouteParameters {
+ payment_params: payment_params.0.unwrap(),
+ final_value_msat: final_value_msat.0.unwrap(),
+ max_total_routing_fee_msat,
};
Ok(Route { paths, route_params })
@@ -3908,7 +3898,7 @@ pub(crate) fn get_route<L: Logger, S: ScoreLookUp>(
}
}
- let route = Route { paths, route_params: Some(route_params.clone()) };
+ let route = Route { paths, route_params: route_params.clone() };
// Make sure we would never create a route whose total fees exceed max_total_routing_fee_msat.
if let Some(max_total_routing_fee_msat) = route_params.max_total_routing_fee_msat {
@@ -7504,7 +7494,7 @@ mod tests {
short_channel_id: 0, fee_msat: 225, cltv_expiry_delta: 0, maybe_announced_channel: true,
},
], blinded_tail: None }],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(PaymentParameters::from_node_id(ln_test_utils::pubkey(42), 0), 225),
};
assert_eq!(route.get_total_fees(), 250);
@@ -7537,7 +7527,7 @@ mod tests {
short_channel_id: 0, fee_msat: 150, cltv_expiry_delta: 0, maybe_announced_channel: true,
},
], blinded_tail: None }],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(PaymentParameters::from_node_id(ln_test_utils::pubkey(42), 0), 300),
};
assert_eq!(route.get_total_fees(), 200);
@@ -7549,7 +7539,13 @@ 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(), route_params: None };
+ let route = Route {
+ paths: Vec::new(),
+ route_params: RouteParameters::from_payment_params_and_value(
+ PaymentParameters::from_node_id(ln_test_utils::pubkey(42), 0),
+ 0,
+ ),
+ };
assert_eq!(route.get_total_fees(), 0);
assert_eq!(route.get_total_amount(), 0);
@@ -8144,7 +8140,7 @@ mod tests {
cltv_expiry_delta: 0,
maybe_announced_channel: true,
}], blinded_tail: None }],
- route_params: None,
+ route_params: RouteParameters::from_payment_params_and_value(PaymentParameters::from_node_id(ln_test_utils::pubkey(42), 0), 200),
};
let encoded_route = route.encode();
let decoded_route: Route = Readable::read(&mut Cursor::new(&encoded_route[..])).unwrap();
@@ -8340,7 +8336,7 @@ mod tests {
excess_final_cltv_expiry_delta: 0,
final_value_msat: 200,
}),
- }], route_params: None};
+ }], route_params: RouteParameters::from_payment_params_and_value(PaymentParameters::from_node_id(ln_test_utils::pubkey(42), 0), 200)};
let payment_params = PaymentParameters::from_node_id(ln_test_utils::pubkey(47), 18);
let (_, network_graph, _, _, _) = build_line_graph();
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 892c9f4..7af41c1 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -240,7 +240,7 @@ impl<'a> Router for TestRouter<'a> {
assert_eq!(find_route_query, *params);
if let Some(res) = find_route_res {
if let Ok(ref route) = res {
- assert_eq!(route.route_params, Some(find_route_query));
+ assert_eq!(route.route_params, find_route_query);
let scorer = self.scorer.read().unwrap();
let scorer = ScorerAccountingForInFlightHtlcs::new(scorer, &inflight_htlcs);
for path in &route.paths {
diff --git a/pending_changelog/route-params-required.txt b/pending_changelog/route-params-required.txt
new file mode 100644
index 0000000..ea47deb
--- /dev/null
+++ b/pending_changelog/route-params-required.txt
@@ -0,0 +1,4 @@
+# Backwards Compatibility
+ * `Route`s serialized by LDK versions prior to 0.0.117 can no longer be
+ deserialized, as parts of the now-required `Route::route_params` were not
+ written by those versions.
Risk score
Our methodology →Why this scored 22/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.