Merge pull request #2664 from TheBlueMatt/2023-10-ci-disk-usage
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 16 Oct 2023 14:56:34 +0000 (14:56 +0000)
committerGitHub <noreply@github.com>
Mon, 16 Oct 2023 14:56:34 +0000 (14:56 +0000)
Reduce disk usage in CI

lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs
lightning/src/routing/router.rs
lightning/src/util/test_utils.rs

index b806b138a59092721d10b8e9289d9f0d28df23a0..d6d9f7aaca02f7fb3ae7296467a91fa6414dba29 100644 (file)
@@ -901,12 +901,10 @@ 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;
-                       }
+               if route.route_params.as_ref() != Some(&route_params) {
+                       debug_assert!(false,
+                               "Routers are expected to return a Route which includes the requested RouteParameters");
+                       route.route_params = Some(route_params.clone());
                }
 
                let onion_session_privs = self.add_new_pending_payment(payment_hash,
@@ -963,12 +961,10 @@ impl OutboundPayments {
                        }
                };
 
-               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;
-                       }
+               if route.route_params.as_ref() != Some(&route_params) {
+                       debug_assert!(false,
+                               "Routers are expected to return a Route which includes the requested RouteParameters");
+                       route.route_params = Some(route_params.clone());
                }
 
                for path in route.paths.iter() {
@@ -1958,7 +1954,9 @@ mod tests {
                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);
-               router.expect_find_route(route_params_w_failed_scid, Ok(route.clone()));
+               let mut route_w_failed_scid = route.clone();
+               route_w_failed_scid.route_params = Some(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()));
 
index 616a1d4ce00a3a429c64697100f109c144caa855..9f6f233861b57a01f5bb6b6fc3a8b2ff843a9b5a 100644 (file)
@@ -147,6 +147,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_total_routing_fee_msat - 1_000);
+       route.route_params = Some(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);
@@ -253,12 +254,12 @@ 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
        // 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());
        nodes[0].router.expect_find_route(route_params, Ok(route));
        nodes[0].node.process_pending_htlc_forwards();
 
@@ -2738,7 +2739,7 @@ fn retry_multi_path_single_failed_payment() {
 
        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.as_mut().unwrap().final_value_msat = 100_000_000;
+       route.route_params = Some(retry_params.clone());
        nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
 
        {
@@ -2809,9 +2810,7 @@ fn immediate_retry_on_failure() {
                                maybe_announced_channel: true,
                        }], blinded_tail: None },
                ],
-               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)),
+               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.
@@ -2821,9 +2820,9 @@ fn immediate_retry_on_failure() {
        route.paths[1].hops[0].fee_msat = 50_000_001;
        let mut pay_params = route_params.payment_params.clone();
        pay_params.previously_failed_channels.push(chans[0].short_channel_id.unwrap());
-       nodes[0].router.expect_find_route(
-               RouteParameters::from_payment_params_and_value(pay_params, amt_msat),
-               Ok(route.clone()));
+       let retry_params = RouteParameters::from_payment_params_and_value(pay_params, amt_msat);
+       route.route_params = Some(retry_params.clone());
+       nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
 
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
                PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
@@ -2933,6 +2932,7 @@ fn no_extra_retries_on_back_to_back_fail() {
        route.paths[0].hops[1].fee_msat = amt_msat;
        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());
        nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
 
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
@@ -3137,7 +3137,7 @@ fn test_simple_partial_retry() {
        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;
+       route.route_params = Some(retry_params.clone());
        nodes[0].router.expect_find_route(retry_params, Ok(route.clone()));
 
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
@@ -3316,7 +3316,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.route_params = Some(route_params.clone());
        route.paths.pop();
 
        let end_time = Instant::now() + Duration::from_secs(1);
@@ -3358,6 +3358,7 @@ fn test_threaded_payment_retries() {
                new_route_params.payment_params.previously_failed_channels = previously_failed_channels.clone();
                new_route_params.max_total_routing_fee_msat.as_mut().map(|m| *m -= 100_000);
                route.paths[0].hops[1].short_channel_id += 1;
+               route.route_params = Some(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], nodes[0].node.get_our_node_id());
@@ -3720,7 +3721,7 @@ fn test_retry_custom_tlvs() {
        send_payment(&nodes[2], &vec!(&nodes[1])[..], 1_500_000);
 
        let amt_msat = 1_000_000;
-       let (route, payment_hash, payment_preimage, payment_secret) =
+       let (mut route, payment_hash, payment_preimage, payment_secret) =
                get_route_and_payment_hash!(nodes[0], nodes[2], amt_msat);
 
        // Initiate the payment
@@ -3772,6 +3773,7 @@ fn test_retry_custom_tlvs() {
 
        // Retry the payment and make sure it succeeds
        route_params.payment_params.previously_failed_channels.push(chan_2_update.contents.short_channel_id);
+       route.route_params = Some(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);
index 28d452c56b616afde8e1aea886e5a0bfe65f8eb0..faf414cc10651fe63fb9a00887554136ccea699e 100644 (file)
@@ -7691,6 +7691,203 @@ mod tests {
                assert_eq!(route.paths.len(), 1);
                assert_eq!(route.get_total_amount(), amt_msat);
        }
+
+       #[test]
+       fn first_hop_preferred_over_hint() {
+               // Check that if we have a first hop to a peer we'd always prefer that over a route hint
+               // they gave us, but we'd still consider all subsequent hints if they are more attractive.
+               let secp_ctx = Secp256k1::new();
+               let logger = Arc::new(ln_test_utils::TestLogger::new());
+               let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
+               let gossip_sync = P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger));
+               let scorer = ln_test_utils::TestScorer::new();
+               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 amt_msat = 1_000_000;
+               let (our_privkey, our_node_id, privkeys, nodes) = get_nodes(&secp_ctx);
+
+               add_channel(&gossip_sync, &secp_ctx, &our_privkey, &privkeys[0],
+                       ChannelFeatures::from_le_bytes(id_to_feature_flags(1)), 1);
+               update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 1,
+                       timestamp: 1,
+                       flags: 0,
+                       cltv_expiry_delta: 42,
+                       htlc_minimum_msat: 1_000,
+                       htlc_maximum_msat: 10_000_000,
+                       fee_base_msat: 800,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[0], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 1,
+                       timestamp: 1,
+                       flags: 1,
+                       cltv_expiry_delta: 42,
+                       htlc_minimum_msat: 1_000,
+                       htlc_maximum_msat: 10_000_000,
+                       fee_base_msat: 800,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+
+               add_channel(&gossip_sync, &secp_ctx, &privkeys[0], &privkeys[1],
+                       ChannelFeatures::from_le_bytes(id_to_feature_flags(1)), 2);
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[0], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 2,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: 42,
+                       htlc_minimum_msat: 1_000,
+                       htlc_maximum_msat: 10_000_000,
+                       fee_base_msat: 800,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 2,
+                       timestamp: 2,
+                       flags: 1,
+                       cltv_expiry_delta: 42,
+                       htlc_minimum_msat: 1_000,
+                       htlc_maximum_msat: 10_000_000,
+                       fee_base_msat: 800,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+
+               let dest_node_id = nodes[2];
+
+               let route_hint = RouteHint(vec![RouteHintHop {
+                       src_node_id: our_node_id,
+                       short_channel_id: 44,
+                       fees: RoutingFees {
+                               base_msat: 234,
+                               proportional_millionths: 0,
+                       },
+                       cltv_expiry_delta: 10,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: Some(5_000_000),
+               },
+               RouteHintHop {
+                       src_node_id: nodes[0],
+                       short_channel_id: 45,
+                       fees: RoutingFees {
+                               base_msat: 123,
+                               proportional_millionths: 0,
+                       },
+                       cltv_expiry_delta: 10,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: None,
+               }]);
+
+               let payment_params = PaymentParameters::from_node_id(dest_node_id, 42)
+                       .with_route_hints(vec![route_hint]).unwrap()
+                       .with_bolt11_features(channelmanager::provided_invoice_features(&config)).unwrap();
+               let route_params = RouteParameters::from_payment_params_and_value(
+                       payment_params, amt_msat);
+
+               // First create an insufficient first hop for channel with SCID 1 and check we'd use the
+               // route hint.
+               let first_hop = get_channel_details(Some(1), nodes[0],
+                       channelmanager::provided_init_features(&config), 999_999);
+               let first_hops = vec![first_hop];
+
+               let route = get_route(&our_node_id, &route_params.clone(), &network_graph.read_only(),
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
+               assert_eq!(route.paths.len(), 1);
+               assert_eq!(route.get_total_amount(), amt_msat);
+               assert_eq!(route.paths[0].hops.len(), 2);
+               assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
+               assert_eq!(route.paths[0].hops[1].short_channel_id, 45);
+               assert_eq!(route.get_total_fees(), 123);
+
+               // Now check we would trust our first hop info, i.e., fail if we detect the route hint is
+               // for a first hop channel.
+               let mut first_hop = get_channel_details(Some(1), nodes[0], channelmanager::provided_init_features(&config), 999_999);
+               first_hop.outbound_scid_alias = Some(44);
+               let first_hops = vec![first_hop];
+
+               let route_res = get_route(&our_node_id, &route_params.clone(), &network_graph.read_only(),
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes);
+               assert!(route_res.is_err());
+
+               // Finally check we'd use the first hop if has sufficient outbound capacity. But we'd stil
+               // use the cheaper second hop of the route hint.
+               let mut first_hop = get_channel_details(Some(1), nodes[0],
+                       channelmanager::provided_init_features(&config), 10_000_000);
+               first_hop.outbound_scid_alias = Some(44);
+               let first_hops = vec![first_hop];
+
+               let route = get_route(&our_node_id, &route_params.clone(), &network_graph.read_only(),
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
+               assert_eq!(route.paths.len(), 1);
+               assert_eq!(route.get_total_amount(), amt_msat);
+               assert_eq!(route.paths[0].hops.len(), 2);
+               assert_eq!(route.paths[0].hops[0].short_channel_id, 1);
+               assert_eq!(route.paths[0].hops[1].short_channel_id, 45);
+               assert_eq!(route.get_total_fees(), 123);
+       }
+
+       #[test]
+       fn allow_us_being_first_hint() {
+               // Check that we consider a route hint even if we are the src of the first hop.
+               let secp_ctx = Secp256k1::new();
+               let logger = Arc::new(ln_test_utils::TestLogger::new());
+               let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, Arc::clone(&logger)));
+               let scorer = ln_test_utils::TestScorer::new();
+               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 (_, our_node_id, _, nodes) = get_nodes(&secp_ctx);
+
+               let amt_msat = 1_000_000;
+               let dest_node_id = nodes[1];
+
+               let first_hop = get_channel_details(Some(1), nodes[0], channelmanager::provided_init_features(&config), 10_000_000);
+               let first_hops = vec![first_hop];
+
+               let route_hint = RouteHint(vec![RouteHintHop {
+                       src_node_id: our_node_id,
+                       short_channel_id: 44,
+                       fees: RoutingFees {
+                               base_msat: 123,
+                               proportional_millionths: 0,
+                       },
+                       cltv_expiry_delta: 10,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: None,
+               }]);
+
+               let payment_params = PaymentParameters::from_node_id(dest_node_id, 42)
+                       .with_route_hints(vec![route_hint]).unwrap()
+                       .with_bolt11_features(channelmanager::provided_invoice_features(&config)).unwrap();
+
+               let route_params = RouteParameters::from_payment_params_and_value(
+                       payment_params, amt_msat);
+
+
+               let route = get_route(&our_node_id, &route_params, &network_graph.read_only(),
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
+
+               assert_eq!(route.paths.len(), 1);
+               assert_eq!(route.get_total_amount(), amt_msat);
+               assert_eq!(route.get_total_fees(), 0);
+               assert_eq!(route.paths[0].hops.len(), 1);
+
+               assert_eq!(route.paths[0].hops[0].short_channel_id, 44);
+       }
 }
 
 #[cfg(all(any(test, ldk_bench), not(feature = "no-std")))]
index bd66e4cae3e56fa70195a4627bf38451bac37d9c..8a988b629079647e8cc85c166e3b874986ec1b3e 100644 (file)
@@ -124,7 +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);
+                               assert_eq!(route.route_params, Some(find_route_query));
                                let scorer = self.scorer.read().unwrap();
                                let scorer = ScorerAccountingForInFlightHtlcs::new(scorer, &inflight_htlcs);
                                for path in &route.paths {