From 8c99e34b9321db8ba22eb6b1836b436bbb652271 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 28 Sep 2023 10:04:35 +0200 Subject: [PATCH] Assert query's and route's `final_value_msat` are equal --- lightning/src/ln/outbound_payment.rs | 21 ++++++++++- lightning/src/ln/payment_tests.rs | 56 ++++++++++++++-------------- lightning/src/util/test_utils.rs | 1 + 3 files changed, 49 insertions(+), 29 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 025a19734..4cc8d85ad 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -875,7 +875,7 @@ impl OutboundPayments { } } - let route = router.find_route_with_id( + let mut route = router.find_route_with_id( &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params, Some(&first_hops.iter().collect::>()), inflight_htlcs(), payment_hash, payment_id, @@ -885,6 +885,14 @@ impl OutboundPayments { RetryableSendFailure::RouteNotFound })?; + if let Some(route_route_params) = route.route_params.as_mut() { + if route_route_params.final_value_msat != route_params.final_value_msat { + debug_assert!(false, + "Routers are expected to return a route which includes the requested final_value_msat"); + route_route_params.final_value_msat = route_params.final_value_msat; + } + } + let onion_session_privs = self.add_new_pending_payment(payment_hash, recipient_onion.clone(), payment_id, keysend_preimage, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height) @@ -926,7 +934,7 @@ impl OutboundPayments { } } - let route = match router.find_route_with_id( + let mut route = match router.find_route_with_id( &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params, Some(&first_hops.iter().collect::>()), inflight_htlcs(), payment_hash, payment_id, @@ -938,6 +946,15 @@ impl OutboundPayments { return } }; + + if let Some(route_route_params) = route.route_params.as_mut() { + if route_route_params.final_value_msat != route_params.final_value_msat { + debug_assert!(false, + "Routers are expected to return a route which includes the requested final_value_msat"); + route_route_params.final_value_msat = route_params.final_value_msat; + } + } + for path in route.paths.iter() { if path.hops.len() == 0 { log_error!(logger, "Unusable path in route (path.hops.len() must be at least 1"); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 0af1d69f2..e48ae7bd8 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -253,6 +253,7 @@ fn mpp_retry_overpay() { route.paths.remove(0); route_params.final_value_msat -= first_path_value; + route.route_params.as_mut().map(|p| p.final_value_msat -= first_path_value); route_params.payment_params.previously_failed_channels.push(chan_4_update.contents.short_channel_id); // Check the remaining max total routing fee for the second attempt accounts only for 1_000 msat @@ -2395,10 +2396,11 @@ fn auto_retry_partial_failure() { let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_expiry_time(payment_expiry_secs as u64) .with_bolt11_features(invoice_features).unwrap(); + + // Configure the initial send path let mut route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat); route_params.max_total_routing_fee_msat = None; - // Configure the initial send, retry1 and retry2's paths. let send_route = Route { paths: vec![ Path { hops: vec![RouteHop { @@ -2422,6 +2424,14 @@ fn auto_retry_partial_failure() { ], route_params: Some(route_params.clone()), }; + nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route)); + + // Configure the retry1 paths + let mut payment_params = route_params.payment_params.clone(); + payment_params.previously_failed_channels.push(chan_2_id); + let mut retry_1_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat / 2); + retry_1_params.max_total_routing_fee_msat = None; + let retry_1_route = Route { paths: vec![ Path { hops: vec![RouteHop { @@ -2443,8 +2453,16 @@ fn auto_retry_partial_failure() { maybe_announced_channel: true, }], blinded_tail: None }, ], - route_params: Some(route_params.clone()), + route_params: Some(retry_1_params.clone()), }; + nodes[0].router.expect_find_route(retry_1_params.clone(), Ok(retry_1_route)); + + // Configure the retry2 path + let mut payment_params = retry_1_params.payment_params.clone(); + payment_params.previously_failed_channels.push(chan_3_id); + let mut retry_2_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat / 4); + retry_2_params.max_total_routing_fee_msat = None; + let retry_2_route = Route { paths: vec![ Path { hops: vec![RouteHop { @@ -2457,20 +2475,8 @@ fn auto_retry_partial_failure() { maybe_announced_channel: true, }], blinded_tail: None }, ], - route_params: Some(route_params.clone()), + route_params: Some(retry_2_params.clone()), }; - nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route)); - let mut payment_params = route_params.payment_params.clone(); - payment_params.previously_failed_channels.push(chan_2_id); - - let mut retry_1_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat / 2); - retry_1_params.max_total_routing_fee_msat = None; - nodes[0].router.expect_find_route(retry_1_params, Ok(retry_1_route)); - - let mut payment_params = route_params.payment_params.clone(); - payment_params.previously_failed_channels.push(chan_3_id); - let mut retry_2_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat / 4); - retry_2_params.max_total_routing_fee_msat = None; nodes[0].router.expect_find_route(retry_2_params, Ok(retry_2_route)); // Send a payment that will partially fail on send, then partially fail on retry, then succeed. @@ -2734,6 +2740,7 @@ fn retry_multi_path_single_failed_payment() { // not the amount remaining on the full payment, which should be changed. let mut retry_params = RouteParameters::from_payment_params_and_value(pay_params, 100_000_001); retry_params.max_total_routing_fee_msat = None; + route.route_params.as_mut().unwrap().final_value_msat = 100_000_000; nodes[0].router.expect_find_route(retry_params, Ok(route.clone())); { @@ -2917,9 +2924,7 @@ fn no_extra_retries_on_back_to_back_fail() { maybe_announced_channel: true, }], blinded_tail: None } ], - route_params: Some(RouteParameters::from_payment_params_and_value( - PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV), - 100_000_000)), + route_params: Some(route_params.clone()), }; route.route_params.as_mut().unwrap().max_total_routing_fee_msat = None; nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); @@ -3123,18 +3128,18 @@ fn test_simple_partial_retry() { maybe_announced_channel: true, }], blinded_tail: None } ], - route_params: Some(RouteParameters::from_payment_params_and_value( - PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV), - 100_000_000)), + route_params: Some(route_params.clone()), }; - route.route_params.as_mut().unwrap().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]; // On retry, we'll only be asked for one path (or 100k sats) route.paths.remove(0); 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.as_mut().unwrap().final_value_msat = amt_msat / 2; nodes[0].router.expect_find_route(retry_params, Ok(route.clone())); nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret), @@ -3294,11 +3299,7 @@ fn test_threaded_payment_retries() { maybe_announced_channel: true, }], blinded_tail: None } ], - route_params: Some(RouteParameters { - payment_params: PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV), - final_value_msat: amt_msat - amt_msat / 1000, - max_total_routing_fee_msat: Some(500_000), - }), + route_params: Some(route_params.clone()), }; nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); @@ -3317,6 +3318,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.as_mut().unwrap().final_value_msat /= 1000; route.paths.pop(); let end_time = Instant::now() + Duration::from_secs(1); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index d53cd39b1..50467a173 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -124,6 +124,7 @@ impl<'a> Router for TestRouter<'a> { if let Some((find_route_query, find_route_res)) = self.next_routes.lock().unwrap().pop_front() { assert_eq!(find_route_query, *params); if let Ok(ref route) = find_route_res { + assert_eq!(route.route_params.as_ref().unwrap().final_value_msat, find_route_query.final_value_msat); let scorer = self.scorer.read().unwrap(); let scorer = ScorerAccountingForInFlightHtlcs::new(scorer, &inflight_htlcs); for path in &route.paths { -- 2.39.5