Merge pull request #1600 from TheBlueMatt/2022-07-explicit-avoid-retries
[rust-lightning] / lightning / src / routing / router.rs
index 17253842e9214a871b177166557a9c06fd97698f..a97e1b6ffa45db704b89505cfce7d6c58cc1f16b 100644 (file)
@@ -240,6 +240,11 @@ pub struct PaymentParameters {
        ///
        /// Default value: 1
        pub max_channel_saturation_power_of_half: u8,
+
+       /// A list of SCIDs which this payment was previously attempted over and which caused the
+       /// payment to fail. Future attempts for the same payment shouldn't be relayed through any of
+       /// these SCIDs.
+       pub previously_failed_channels: Vec<u64>,
 }
 
 impl_writeable_tlv_based!(PaymentParameters, {
@@ -250,6 +255,7 @@ impl_writeable_tlv_based!(PaymentParameters, {
        (4, route_hints, vec_type),
        (5, max_channel_saturation_power_of_half, (default_value, 1)),
        (6, expiry_time, option),
+       (7, previously_failed_channels, vec_type),
 });
 
 impl PaymentParameters {
@@ -263,6 +269,7 @@ impl PaymentParameters {
                        max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA,
                        max_path_count: DEFAULT_MAX_PATH_COUNT,
                        max_channel_saturation_power_of_half: 1,
+                       previously_failed_channels: Vec::new(),
                }
        }
 
@@ -1002,7 +1009,7 @@ where L::Target: Logger {
                                        let contributes_sufficient_value = available_value_contribution_msat >= minimal_value_contribution_msat;
                                        // Do not consider candidate hops that would exceed the maximum path length.
                                        let path_length_to_node = $next_hops_path_length + 1;
-                                       let doesnt_exceed_max_path_length = path_length_to_node <= MAX_PATH_LENGTH_ESTIMATE;
+                                       let exceeds_max_path_length = path_length_to_node > MAX_PATH_LENGTH_ESTIMATE;
 
                                        // Do not consider candidates that exceed the maximum total cltv expiry limit.
                                        // In order to already account for some of the privacy enhancing random CLTV
@@ -1013,7 +1020,7 @@ where L::Target: Logger {
                                                .unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta);
                                        let hop_total_cltv_delta = ($next_hops_cltv_delta as u32)
                                                .saturating_add($candidate.cltv_expiry_delta());
-                                       let doesnt_exceed_cltv_delta_limit = hop_total_cltv_delta <= max_total_cltv_expiry_delta;
+                                       let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta;
 
                                        let value_contribution_msat = cmp::min(available_value_contribution_msat, $next_hops_value_contribution);
                                        // Includes paying fees for the use of the following channels.
@@ -1033,15 +1040,19 @@ where L::Target: Logger {
                                                 (amount_to_transfer_over_msat < $next_hops_path_htlc_minimum_msat &&
                                                  recommended_value_msat > $next_hops_path_htlc_minimum_msat));
 
+                                       let payment_failed_on_this_channel =
+                                               payment_params.previously_failed_channels.contains(&short_channel_id);
+
                                        // If HTLC minimum is larger than the amount we're going to transfer, we shouldn't
                                        // bother considering this channel. If retrying with recommended_value_msat may
                                        // allow us to hit the HTLC minimum limit, set htlc_minimum_limit so that we go
                                        // around again with a higher amount.
-                                       if contributes_sufficient_value && doesnt_exceed_max_path_length &&
-                                               doesnt_exceed_cltv_delta_limit && may_overpay_to_meet_path_minimum_msat {
+                                       if !contributes_sufficient_value || exceeds_max_path_length ||
+                                               exceeds_cltv_delta_limit || payment_failed_on_this_channel {
+                                               // Path isn't useful, ignore it and move on.
+                                       } else if may_overpay_to_meet_path_minimum_msat {
                                                hit_minimum_limit = true;
-                                       } else if contributes_sufficient_value && doesnt_exceed_max_path_length &&
-                                               doesnt_exceed_cltv_delta_limit && over_path_minimum_msat {
+                                       } else if over_path_minimum_msat {
                                                // Note that low contribution here (limited by available_liquidity_msat)
                                                // might violate htlc_minimum_msat on the hops which are next along the
                                                // payment path (upstream to the payee). To avoid that, we recompute
@@ -1960,7 +1971,7 @@ fn build_route_from_hops_internal<L: Deref>(
 
 #[cfg(test)]
 mod tests {
-       use routing::gossip::{NetworkGraph, P2PGossipSync, NodeId};
+       use routing::gossip::{NetworkGraph, P2PGossipSync, NodeId, EffectiveCapacity};
        use routing::router::{get_route, build_route_from_hops_internal, add_random_cltv_offset, default_node_features,
                PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees,
                DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE};
@@ -1993,6 +2004,8 @@ mod tests {
        use prelude::*;
        use sync::{self, Arc};
 
+       use core::convert::TryInto;
+
        fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
                        features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
                channelmanager::ChannelDetails {
@@ -5573,6 +5586,35 @@ mod tests {
                }
        }
 
+       #[test]
+       fn avoids_recently_failed_paths() {
+               // Ensure that the router always avoids all of the `previously_failed_channels` channels by
+               // randomly inserting channels into it until we can't find a route anymore.
+               let (secp_ctx, network, _, _, logger) = build_graph();
+               let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
+               let network_graph = network.read_only();
+
+               let scorer = test_utils::TestScorer::with_penalty(0);
+               let mut payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes))
+                       .with_max_path_count(1);
+               let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
+               let random_seed_bytes = keys_manager.get_secure_random_bytes();
+
+               // We should be able to find a route initially, and then after we fail a few random
+               // channels eventually we won't be able to any longer.
+               assert!(get_route(&our_id, &payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes).is_ok());
+               loop {
+                       if let Ok(route) = get_route(&our_id, &payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                               for chan in route.paths[0].iter() {
+                                       assert!(!payment_params.previously_failed_channels.contains(&chan.short_channel_id));
+                               }
+                               let victim = (u64::from_ne_bytes(random_seed_bytes[0..8].try_into().unwrap()) as usize)
+                                       % route.paths[0].len();
+                               payment_params.previously_failed_channels.push(route.paths[0][victim].short_channel_id);
+                       } else { break; }
+               }
+       }
+
        #[test]
        fn limits_path_length() {
                let (secp_ctx, network, _, _, logger) = build_line_graph();
@@ -5722,6 +5764,50 @@ mod tests {
                }
        }
 
+       #[test]
+       fn avoids_saturating_channels() {
+               let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
+               let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
+
+               let scorer = ProbabilisticScorer::new(Default::default(), &*network_graph, Arc::clone(&logger));
+
+               // Set the fee on channel 13 to 100% to match channel 4 giving us two equivalent paths (us
+               // -> node 7 -> node2 and us -> node 1 -> node 2) which we should balance over.
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 4,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: (4 << 4) | 1,
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: OptionalField::Present(200_000_000),
+                       fee_base_msat: 0,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[7], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 13,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: (13 << 4) | 1,
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: OptionalField::Present(200_000_000),
+                       fee_base_msat: 0,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+
+               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
+               let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
+               let random_seed_bytes = keys_manager.get_secure_random_bytes();
+               // 150,000 sat is less than the available liquidity on each channel, set above.
+               let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 150_000_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
+               assert_eq!(route.paths.len(), 2);
+               assert!((route.paths[0][1].short_channel_id == 4 && route.paths[1][1].short_channel_id == 13) ||
+                       (route.paths[1][1].short_channel_id == 4 && route.paths[0][1].short_channel_id == 13));
+       }
+
        #[cfg(not(feature = "no-std"))]
        pub(super) fn random_init_seed() -> u64 {
                // Because the default HashMap in std pulls OS randomness, we can use it as a (bad) RNG.
@@ -5808,7 +5894,7 @@ mod tests {
        }
 
        #[test]
-       fn avoids_banned_nodes() {
+       fn honors_manual_penalties() {
                let (secp_ctx, network_graph, _, _, logger) = build_line_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 
@@ -5818,7 +5904,17 @@ mod tests {
                let scorer_params = ProbabilisticScoringParameters::default();
                let mut scorer = ProbabilisticScorer::new(scorer_params, Arc::clone(&network_graph), Arc::clone(&logger));
 
-               // First check we can get a route.
+               // First check set manual penalties are returned by the scorer.
+               let usage = ChannelUsage {
+                       amount_msat: 0,
+                       inflight_htlc_msat: 0,
+                       effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024_000, htlc_maximum_msat: Some(1_000) },
+               };
+               scorer.set_manual_penalty(&NodeId::from_pubkey(&nodes[3]), 123);
+               scorer.set_manual_penalty(&NodeId::from_pubkey(&nodes[4]), 456);
+               assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage), 456);
+
+               // Then check we can get a normal route
                let payment_params = PaymentParameters::from_node_id(nodes[10]);
                let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes);
                assert!(route.is_ok());