Relax the channel saturation limit if we can't find enough paths
[rust-lightning] / lightning / src / routing / router.rs
index 90e658604ad1058040643e5b6be48fd93cfe29a8..17253842e9214a871b177166557a9c06fd97698f 100644 (file)
@@ -17,7 +17,7 @@ use bitcoin::secp256k1::PublicKey;
 use ln::channelmanager::ChannelDetails;
 use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
-use routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NodeId, RoutingFees};
+use routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
 use routing::scoring::{ChannelUsage, Score};
 use util::ser::{Writeable, Readable, Writer};
 use util::logger::{Level, Logger};
@@ -176,6 +176,11 @@ impl_writeable_tlv_based!(RouteParameters, {
 /// Maximum total CTLV difference we allow for a full payment path.
 pub const DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA: u32 = 1008;
 
+/// Maximum number of paths we allow an (MPP) payment to have.
+// The default limit is currently set rather arbitrary - there aren't any real fundamental path-count
+// limits, but for now more than 10 paths likely carries too much one-path failure.
+pub const DEFAULT_MAX_PATH_COUNT: u8 = 10;
+
 // The median hop CLTV expiry delta currently seen in the network.
 const MEDIAN_HOP_CLTV_EXPIRY_DELTA: u32 = 40;
 
@@ -214,14 +219,36 @@ pub struct PaymentParameters {
        pub expiry_time: Option<u64>,
 
        /// The maximum total CLTV delta we accept for the route.
+       /// Defaults to [`DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA`].
        pub max_total_cltv_expiry_delta: u32,
+
+       /// The maximum number of paths that may be used by (MPP) payments.
+       /// Defaults to [`DEFAULT_MAX_PATH_COUNT`].
+       pub max_path_count: u8,
+
+       /// Selects the maximum share of a channel's total capacity which will be sent over a channel,
+       /// as a power of 1/2. A higher value prefers to send the payment using more MPP parts whereas
+       /// a lower value prefers to send larger MPP parts, potentially saturating channels and
+       /// increasing failure probability for those paths.
+       ///
+       /// Note that this restriction will be relaxed during pathfinding after paths which meet this
+       /// restriction have been found. While paths which meet this criteria will be searched for, it
+       /// is ultimately up to the scorer to select them over other paths.
+       ///
+       /// A value of 0 will allow payments up to and including a channel's total announced usable
+       /// capacity, a value of one will only use up to half its capacity, two 1/4, etc.
+       ///
+       /// Default value: 1
+       pub max_channel_saturation_power_of_half: u8,
 }
 
 impl_writeable_tlv_based!(PaymentParameters, {
        (0, payee_pubkey, required),
        (1, max_total_cltv_expiry_delta, (default_value, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA)),
        (2, features, option),
+       (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)),
        (4, route_hints, vec_type),
+       (5, max_channel_saturation_power_of_half, (default_value, 1)),
        (6, expiry_time, option),
 });
 
@@ -234,6 +261,8 @@ impl PaymentParameters {
                        route_hints: vec![],
                        expiry_time: None,
                        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,
                }
        }
 
@@ -269,6 +298,20 @@ impl PaymentParameters {
        pub fn with_max_total_cltv_expiry_delta(self, max_total_cltv_expiry_delta: u32) -> Self {
                Self { max_total_cltv_expiry_delta, ..self }
        }
+
+       /// Includes a limit for the maximum number of payment paths that may be used.
+       ///
+       /// (C-not exported) since bindings don't support move semantics
+       pub fn with_max_path_count(self, max_path_count: u8) -> Self {
+               Self { max_path_count, ..self }
+       }
+
+       /// Includes a limit for the maximum number of payment paths that may be used.
+       ///
+       /// (C-not exported) since bindings don't support move semantics
+       pub fn with_max_channel_saturation_power_of_half(self, max_channel_saturation_power_of_half: u8) -> Self {
+               Self { max_channel_saturation_power_of_half, ..self }
+       }
 }
 
 /// A list of hops along a payment path terminating with a channel to the recipient.
@@ -414,16 +457,6 @@ impl<'a> CandidateRouteHop<'a> {
                }
        }
 
-       fn htlc_maximum_msat(&self) -> u64 {
-               match self {
-                       CandidateRouteHop::FirstHop { details } => details.next_outbound_htlc_limit_msat,
-                       CandidateRouteHop::PublicHop { info, .. } => info.htlc_maximum_msat(),
-                       CandidateRouteHop::PrivateHop { hint } => {
-                               hint.htlc_maximum_msat.unwrap_or(u64::max_value())
-                       },
-               }
-       }
-
        fn fees(&self) -> RoutingFees {
                match self {
                        CandidateRouteHop::FirstHop { .. } => RoutingFees {
@@ -445,6 +478,33 @@ impl<'a> CandidateRouteHop<'a> {
        }
 }
 
+#[inline]
+fn max_htlc_from_capacity(capacity: EffectiveCapacity, max_channel_saturation_power_of_half: u8) -> u64 {
+       let saturation_shift: u32 = max_channel_saturation_power_of_half as u32;
+       match capacity {
+               EffectiveCapacity::ExactLiquidity { liquidity_msat } => liquidity_msat,
+               EffectiveCapacity::Infinite => u64::max_value(),
+               EffectiveCapacity::Unknown => EffectiveCapacity::Unknown.as_msat(),
+               EffectiveCapacity::MaximumHTLC { amount_msat } =>
+                       amount_msat.checked_shr(saturation_shift).unwrap_or(0),
+               EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: None } =>
+                       capacity_msat.checked_shr(saturation_shift).unwrap_or(0),
+               EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_max) } =>
+                       cmp::min(capacity_msat.checked_shr(saturation_shift).unwrap_or(0), htlc_max),
+       }
+}
+
+fn iter_equal<I1: Iterator, I2: Iterator>(mut iter_a: I1, mut iter_b: I2)
+-> bool where I1::Item: PartialEq<I2::Item> {
+       loop {
+               let a = iter_a.next();
+               let b = iter_b.next();
+               if a.is_none() && b.is_none() { return true; }
+               if a.is_none() || b.is_none() { return false; }
+               if a.unwrap().ne(&b.unwrap()) { return false; }
+       }
+}
+
 /// It's useful to keep track of the hops associated with the fees required to use them,
 /// so that we can choose cheaper paths (as per Dijkstra's algorithm).
 /// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees.
@@ -552,10 +612,9 @@ impl<'a> PaymentPath<'a> {
        // to the fees being paid not lining up with the actual limits.
        //
        // Note that this function is not aware of the available_liquidity limit, and thus does not
-       // support increasing the value being transferred.
+       // support increasing the value being transferred beyond what was selected during the initial
+       // routing passes.
        fn update_value_and_recompute_fees(&mut self, value_msat: u64) {
-               assert!(value_msat <= self.hops.last().unwrap().0.fee_msat);
-
                let mut total_fee_paid_msat = 0 as u64;
                for i in (0..self.hops.len()).rev() {
                        let last_hop = i == self.hops.len() - 1;
@@ -671,16 +730,17 @@ fn default_node_features() -> NodeFeatures {
 /// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels
 /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
 /// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
-pub fn find_route<L: Deref, S: Score>(
+pub fn find_route<L: Deref, GL: Deref, S: Score>(
        our_node_pubkey: &PublicKey, route_params: &RouteParameters,
-       network_graph: &ReadOnlyNetworkGraph, first_hops: Option<&[&ChannelDetails]>, logger: L,
+       network_graph: &NetworkGraph<GL>, first_hops: Option<&[&ChannelDetails]>, logger: L,
        scorer: &S, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
-where L::Target: Logger {
-       let mut route = get_route(our_node_pubkey, &route_params.payment_params, network_graph, first_hops,
+where L::Target: Logger, GL::Target: Logger {
+       let graph_lock = network_graph.read_only();
+       let mut route = get_route(our_node_pubkey, &route_params.payment_params, &graph_lock, first_hops,
                route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, scorer,
                random_seed_bytes)?;
-       add_random_cltv_offset(&mut route, &route_params.payment_params, network_graph, random_seed_bytes);
+       add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
        Ok(route)
 }
 
@@ -780,16 +840,23 @@ where L::Target: Logger {
        let network_channels = network_graph.channels();
        let network_nodes = network_graph.nodes();
 
+       if payment_params.max_path_count == 0 {
+               return Err(LightningError{err: "Can't find a route with no paths allowed.".to_owned(), action: ErrorAction::IgnoreError});
+       }
+
        // Allow MPP only if we have a features set from somewhere that indicates the payee supports
        // it. If the payee supports it they're supposed to include it in the invoice, so that should
        // work reliably.
-       let allow_mpp = if let Some(features) = &payment_params.features {
+       let allow_mpp = if payment_params.max_path_count == 1 {
+               false
+       } else if let Some(features) = &payment_params.features {
                features.supports_basic_mpp()
        } else if let Some(node) = network_nodes.get(&payee_node_id) {
                if let Some(node_info) = node.announcement_info.as_ref() {
                        node_info.features.supports_basic_mpp()
                } else { false }
        } else { false };
+
        log_trace!(logger, "Searching for a route from payer {} to payee {} {} MPP and {} first hops {}overriding the network graph", our_node_pubkey,
                payment_params.payee_pubkey, if allow_mpp { "with" } else { "without" },
                first_hops.map(|hops| hops.len()).unwrap_or(0), if first_hops.is_some() { "" } else { "not " });
@@ -840,6 +907,26 @@ where L::Target: Logger {
        let recommended_value_msat = final_value_msat * ROUTE_CAPACITY_PROVISION_FACTOR as u64;
        let mut path_value_msat = final_value_msat;
 
+       // Routing Fragmentation Mitigation heuristic:
+       //
+       // Routing fragmentation across many payment paths increases the overall routing
+       // fees as you have irreducible routing fees per-link used (`fee_base_msat`).
+       // Taking too many smaller paths also increases the chance of payment failure.
+       // Thus to avoid this effect, we require from our collected links to provide
+       // at least a minimal contribution to the recommended value yet-to-be-fulfilled.
+       // This requirement is currently set to be 1/max_path_count of the payment
+       // value to ensure we only ever return routes that do not violate this limit.
+       let minimal_value_contribution_msat: u64 = if allow_mpp {
+               (final_value_msat + (payment_params.max_path_count as u64 - 1)) / payment_params.max_path_count as u64
+       } else {
+               final_value_msat
+       };
+
+       // When we start collecting routes we enforce the max_channel_saturation_power_of_half
+       // requirement strictly. After we've collected enough (or if we fail to find new routes) we
+       // drop the requirement by setting this to 0.
+       let mut channel_saturation_pow_half = payment_params.max_channel_saturation_power_of_half;
+
        // Keep track of how much liquidity has been used in selected channels. Used to determine
        // if the channel can be used by additional MPP paths or to inform path finding decisions. It is
        // aware of direction *only* to ensure that the correct htlc_maximum_msat value is used. Hence,
@@ -847,11 +934,8 @@ where L::Target: Logger {
        let mut used_channel_liquidities: HashMap<(u64, bool), u64> =
                HashMap::with_capacity(network_nodes.len());
 
-       // Keeping track of how much value we already collected across other paths. Helps to decide:
-       // - how much a new path should be transferring (upper bound);
-       // - whether a channel should be disregarded because
-       //   it's available liquidity is too small comparing to how much more we need to collect;
-       // - when we want to stop looking for new paths.
+       // Keeping track of how much value we already collected across other paths. Helps to decide
+       // when we want to stop looking for new paths.
        let mut already_collected_value_msat = 0;
 
        for (_, channels) in first_hop_targets.iter_mut() {
@@ -895,7 +979,8 @@ where L::Target: Logger {
                        // - for first and last hops early in get_route
                        if $src_node_id != $dest_node_id {
                                let short_channel_id = $candidate.short_channel_id();
-                               let htlc_maximum_msat = $candidate.htlc_maximum_msat();
+                               let effective_capacity = $candidate.effective_capacity();
+                               let htlc_maximum_msat = max_htlc_from_capacity(effective_capacity, channel_saturation_pow_half);
 
                                // It is tricky to subtract $next_hops_fee_msat from available liquidity here.
                                // It may be misleading because we might later choose to reduce the value transferred
@@ -913,26 +998,6 @@ where L::Target: Logger {
                                                        *used_liquidity_msat
                                                });
 
-                                       // Routing Fragmentation Mitigation heuristic:
-                                       //
-                                       // Routing fragmentation across many payment paths increases the overall routing
-                                       // fees as you have irreducible routing fees per-link used (`fee_base_msat`).
-                                       // Taking too many smaller paths also increases the chance of payment failure.
-                                       // Thus to avoid this effect, we require from our collected links to provide
-                                       // at least a minimal contribution to the recommended value yet-to-be-fulfilled.
-                                       //
-                                       // This requirement is currently 5% of the remaining-to-be-collected value.
-                                       // This means as we successfully advance in our collection,
-                                       // the absolute liquidity contribution is lowered,
-                                       // thus increasing the number of potential channels to be selected.
-
-                                       // Derive the minimal liquidity contribution with a ratio of 20 (5%, rounded up)
-                                       // or 100% if we're not allowed to do multipath payments.
-                                       let minimal_value_contribution_msat: u64 = if allow_mpp {
-                                               (recommended_value_msat - already_collected_value_msat + 19) / 20
-                                       } else {
-                                               final_value_msat
-                                       };
                                        // Verify the liquidity offered by this channel complies to the minimal contribution.
                                        let contributes_sufficient_value = available_value_contribution_msat >= minimal_value_contribution_msat;
                                        // Do not consider candidate hops that would exceed the maximum path length.
@@ -1065,7 +1130,7 @@ where L::Target: Logger {
                                                        let channel_usage = ChannelUsage {
                                                                amount_msat: amount_to_transfer_over_msat,
                                                                inflight_htlc_msat: used_liquidity_msat,
-                                                               effective_capacity: $candidate.effective_capacity(),
+                                                               effective_capacity,
                                                        };
                                                        let channel_penalty_msat = scorer.channel_penalty_msat(
                                                                short_channel_id, &$src_node_id, &$dest_node_id, channel_usage
@@ -1486,12 +1551,14 @@ where L::Target: Logger {
                                                .entry((hop.candidate.short_channel_id(), *prev_hop < hop.node_id))
                                                .and_modify(|used_liquidity_msat| *used_liquidity_msat += spent_on_hop_msat)
                                                .or_insert(spent_on_hop_msat);
-                                       if *used_liquidity_msat == hop.candidate.htlc_maximum_msat() {
+                                       let hop_capacity = hop.candidate.effective_capacity();
+                                       let hop_max_msat = max_htlc_from_capacity(hop_capacity, channel_saturation_pow_half);
+                                       if *used_liquidity_msat == hop_max_msat {
                                                // If this path used all of this channel's available liquidity, we know
                                                // this path will not be selected again in the next loop iteration.
                                                prevented_redundant_path_selection = true;
                                        }
-                                       debug_assert!(*used_liquidity_msat <= hop.candidate.htlc_maximum_msat());
+                                       debug_assert!(*used_liquidity_msat <= hop_max_msat);
                                }
                                if !prevented_redundant_path_selection {
                                        // If we weren't capped by hitting a liquidity limit on a channel in the path,
@@ -1504,10 +1571,8 @@ where L::Target: Logger {
                                        *used_channel_liquidities.entry((victim_scid, true)).or_default() = exhausted;
                                }
 
-                               // Track the total amount all our collected paths allow to send so that we:
-                               // - know when to stop looking for more paths
-                               // - know which of the hops are useless considering how much more sats we need
-                               //   (contributes_sufficient_value)
+                               // Track the total amount all our collected paths allow to send so that we know
+                               // when to stop looking for more paths
                                already_collected_value_msat += value_contribution_msat;
 
                                payment_paths.push(payment_path);
@@ -1534,6 +1599,10 @@ where L::Target: Logger {
                }
 
                if !allow_mpp {
+                       if !found_new_path && channel_saturation_pow_half != 0 {
+                               channel_saturation_pow_half = 0;
+                               continue 'paths_collection;
+                       }
                        // If we don't support MPP, no use trying to gather more value ever.
                        break 'paths_collection;
                }
@@ -1543,7 +1612,9 @@ where L::Target: Logger {
                // iteration.
                // In the latter case, making another path finding attempt won't help,
                // because we deterministically terminated the search due to low liquidity.
-               if already_collected_value_msat >= recommended_value_msat || !found_new_path {
+               if !found_new_path && channel_saturation_pow_half != 0 {
+                       channel_saturation_pow_half = 0;
+               } else if already_collected_value_msat >= recommended_value_msat || !found_new_path {
                        log_trace!(logger, "Have now collected {} msat (seeking {} msat) in paths. Last path loop {} a new path.",
                                already_collected_value_msat, recommended_value_msat, if found_new_path { "found" } else { "did not find" });
                        break 'paths_collection;
@@ -1659,8 +1730,32 @@ where L::Target: Logger {
        // Step (9).
        // Select the best route by lowest total cost.
        drawn_routes.sort_unstable_by_key(|paths| paths.iter().map(|path| path.get_cost_msat()).sum::<u64>());
+       let selected_route = drawn_routes.first_mut().unwrap();
+
+       // Sort by the path itself and combine redundant paths.
+       // Note that we sort by SCIDs alone as its simpler but when combining we have to ensure we
+       // compare both SCIDs and NodeIds as individual nodes may use random aliases causing collisions
+       // across nodes.
+       selected_route.sort_unstable_by_key(|path| {
+               let mut key = [0u64; MAX_PATH_LENGTH_ESTIMATE as usize];
+               debug_assert!(path.hops.len() <= key.len());
+               for (scid, key) in path.hops.iter().map(|h| h.0.candidate.short_channel_id()).zip(key.iter_mut()) {
+                       *key = scid;
+               }
+               key
+       });
+       for idx in 0..(selected_route.len() - 1) {
+               if idx + 1 >= selected_route.len() { break; }
+               if iter_equal(selected_route[idx    ].hops.iter().map(|h| (h.0.candidate.short_channel_id(), h.0.node_id)),
+                             selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.short_channel_id(), h.0.node_id))) {
+                       let new_value = selected_route[idx].get_value_msat() + selected_route[idx + 1].get_value_msat();
+                       selected_route[idx].update_value_and_recompute_fees(new_value);
+                       selected_route.remove(idx + 1);
+               }
+       }
+
        let mut selected_paths = Vec::<Vec<Result<RouteHop, LightningError>>>::new();
-       for payment_path in drawn_routes.first().unwrap() {
+       for payment_path in selected_route {
                let mut path = payment_path.hops.iter().map(|(payment_hop, node_features)| {
                        Ok(RouteHop {
                                pubkey: PublicKey::from_slice(payment_hop.node_id.as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &payment_hop.node_id), action: ErrorAction::IgnoreAndLog(Level::Trace)})?,
@@ -1678,6 +1773,8 @@ where L::Target: Logger {
                });
                selected_paths.push(path);
        }
+       // Make sure we would never create a route with more paths than we allow.
+       debug_assert!(selected_paths.len() <= payment_params.max_path_count.into());
 
        if let Some(features) = &payment_params.features {
                for path in selected_paths.iter_mut() {
@@ -1787,15 +1884,16 @@ fn add_random_cltv_offset(route: &mut Route, payment_params: &PaymentParameters,
 /// exclude the payer, but include the payee). This may be useful, e.g., for probing the chosen path.
 ///
 /// Re-uses logic from `find_route`, so the restrictions described there also apply here.
-pub fn build_route_from_hops<L: Deref>(
+pub fn build_route_from_hops<L: Deref, GL: Deref>(
        our_node_pubkey: &PublicKey, hops: &[PublicKey], route_params: &RouteParameters,
-       network_graph: &ReadOnlyNetworkGraph, logger: L, random_seed_bytes: &[u8; 32]
+       network_graph: &NetworkGraph<GL>, logger: L, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
-where L::Target: Logger {
+where L::Target: Logger, GL::Target: Logger {
+       let graph_lock = network_graph.read_only();
        let mut route = build_route_from_hops_internal(
-               our_node_pubkey, hops, &route_params.payment_params, &network_graph,
+               our_node_pubkey, hops, &route_params.payment_params, &graph_lock,
                route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, random_seed_bytes)?;
-       add_random_cltv_offset(&mut route, &route_params.payment_params, &network_graph, random_seed_bytes);
+       add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
        Ok(route)
 }
 
@@ -1831,6 +1929,10 @@ fn build_route_from_hops_internal<L: Deref>(
                fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
 
                fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
+
+               fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
+
+               fn probe_successful(&mut self, _path: &[&RouteHop]) {}
        }
 
        impl<'a> Writeable for HopScorer {
@@ -1862,7 +1964,7 @@ mod tests {
        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};
-       use routing::scoring::{ChannelUsage, Score};
+       use routing::scoring::{ChannelUsage, Score, ProbabilisticScorer, ProbabilisticScoringParameters};
        use chain::transaction::OutPoint;
        use chain::keysinterface::KeysInterface;
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
@@ -1921,6 +2023,7 @@ mod tests {
                        is_usable: true, is_public: true,
                        inbound_htlc_minimum_msat: None,
                        inbound_htlc_maximum_msat: None,
+                       config: None,
                }
        }
 
@@ -3999,7 +4102,8 @@ mod tests {
                let scorer = test_utils::TestScorer::with_penalty(0);
                let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
-               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
+               let payment_params = PaymentParameters::from_node_id(nodes[2])
+                       .with_features(InvoiceFeatures::known());
 
                // We need a route consisting of 3 paths:
                // From our node to node2 via node0, node7, node1 (three paths one hop each).
@@ -4092,15 +4196,39 @@ mod tests {
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
-                                       &our_id, &payment_params, &network_graph.read_only(), None, 300_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes) {
-                               assert_eq!(err, "Failed to find a sufficient route to the given destination");
+                               &our_id, &payment_params, &network_graph.read_only(), None, 300_000, 42,
+                               Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                                       assert_eq!(err, "Failed to find a sufficient route to the given destination");
+                       } else { panic!(); }
+               }
+
+               {
+                       // Attempt to route while setting max_path_count to 0 results in a failure.
+                       let zero_payment_params = payment_params.clone().with_max_path_count(0);
+                       if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
+                               &our_id, &zero_payment_params, &network_graph.read_only(), None, 100, 42,
+                               Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                                       assert_eq!(err, "Can't find a route with no paths allowed.");
+                       } else { panic!(); }
+               }
+
+               {
+                       // Attempt to route while setting max_path_count to 3 results in a failure.
+                       // This is the case because the minimal_value_contribution_msat would require each path
+                       // to account for 1/3 of the total value, which is violated by 2 out of 3 paths.
+                       let fail_payment_params = payment_params.clone().with_max_path_count(3);
+                       if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
+                               &our_id, &fail_payment_params, &network_graph.read_only(), None, 250_000, 42,
+                               Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                                       assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
 
                {
                        // Now, attempt to route 250 sats (just a bit below the capacity).
                        // Our algorithm should provide us with these 3 paths.
-                       let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 250_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
+                       let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None,
+                               250_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4113,7 +4241,8 @@ mod tests {
 
                {
                        // Attempt to route an exact amount is also fine
-                       let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 290_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
+                       let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None,
+                               290_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4712,17 +4841,18 @@ mod tests {
 
                // Get a route for 100 sats and check that we found the MPP route no problem and didn't
                // overpay at all.
-               let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
+               let mut route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
-               // Paths are somewhat randomly ordered, but:
-               // * the first is channel 2 (1 msat fee) -> channel 4 -> channel 42
-               // * the second is channel 1 (0 fee, but 99 sat maximum) -> channel 3 -> channel 42
-               assert_eq!(route.paths[0][0].short_channel_id, 2);
-               assert_eq!(route.paths[0][0].fee_msat, 1);
-               assert_eq!(route.paths[0][2].fee_msat, 1_000);
-               assert_eq!(route.paths[1][0].short_channel_id, 1);
-               assert_eq!(route.paths[1][0].fee_msat, 0);
-               assert_eq!(route.paths[1][2].fee_msat, 99_000);
+               route.paths.sort_by_key(|path| path[0].short_channel_id);
+               // Paths are manually ordered ordered by SCID, so:
+               // * the first is channel 1 (0 fee, but 99 sat maximum) -> channel 3 -> channel 42
+               // * the second is channel 2 (1 msat fee) -> channel 4 -> channel 42
+               assert_eq!(route.paths[0][0].short_channel_id, 1);
+               assert_eq!(route.paths[0][0].fee_msat, 0);
+               assert_eq!(route.paths[0][2].fee_msat, 99_000);
+               assert_eq!(route.paths[1][0].short_channel_id, 2);
+               assert_eq!(route.paths[1][0].fee_msat, 1);
+               assert_eq!(route.paths[1][2].fee_msat, 1_000);
                assert_eq!(route.get_total_fees(), 1);
                assert_eq!(route.get_total_amount(), 100_000);
        }
@@ -4736,7 +4866,8 @@ mod tests {
                let scorer = test_utils::TestScorer::with_penalty(0);
                let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
-               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
+               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known())
+                       .with_max_channel_saturation_power_of_half(0);
 
                // We need a route consisting of 3 paths:
                // From our node to node2 via node0, node7, node1 (three paths one hop each).
@@ -5184,12 +5315,13 @@ mod tests {
                        assert_eq!(route.paths[0].len(), 1);
                        assert_eq!(route.paths[1].len(), 1);
 
+                       assert!((route.paths[0][0].short_channel_id == 3 && route.paths[1][0].short_channel_id == 2) ||
+                               (route.paths[0][0].short_channel_id == 2 && route.paths[1][0].short_channel_id == 3));
+
                        assert_eq!(route.paths[0][0].pubkey, nodes[0]);
-                       assert_eq!(route.paths[0][0].short_channel_id, 3);
                        assert_eq!(route.paths[0][0].fee_msat, 50_000);
 
                        assert_eq!(route.paths[1][0].pubkey, nodes[0]);
-                       assert_eq!(route.paths[1][0].short_channel_id, 2);
                        assert_eq!(route.paths[1][0].fee_msat, 50_000);
                }
 
@@ -5269,6 +5401,8 @@ mod tests {
 
                fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
                fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
+               fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
+               fn probe_successful(&mut self, _path: &[&RouteHop]) {}
        }
 
        struct BadNodeScorer {
@@ -5287,6 +5421,8 @@ mod tests {
 
                fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
                fn payment_path_successful(&mut self, _path: &[&RouteHop]) {}
+               fn probe_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
+               fn probe_successful(&mut self, _path: &[&RouteHop]) {}
        }
 
        #[test]
@@ -5670,6 +5806,33 @@ mod tests {
                        }
                }
        }
+
+       #[test]
+       fn avoids_banned_nodes() {
+               let (secp_ctx, network_graph, _, _, logger) = build_line_graph();
+               let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
+
+               let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
+               let random_seed_bytes = keys_manager.get_secure_random_bytes();
+
+               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.
+               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());
+
+               // Then check that we can't get a route if we ban an intermediate node.
+               scorer.add_banned(&NodeId::from_pubkey(&nodes[3]));
+               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_err());
+
+               // Finally make sure we can route again, when we remove the ban.
+               scorer.remove_banned(&NodeId::from_pubkey(&nodes[3]));
+               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());
+       }
 }
 
 #[cfg(all(test, not(feature = "no-std")))]
@@ -5764,6 +5927,7 @@ mod benches {
                        is_public: true,
                        inbound_htlc_minimum_msat: None,
                        inbound_htlc_maximum_msat: None,
+                       config: None,
                }
        }