Relax the channel saturation limit if we can't find enough paths
[rust-lightning] / lightning / src / routing / router.rs
index 634282d6c4b1f202fd0867312fc3dd83fc9b57fb..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,10 +176,10 @@ 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.
+/// 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_MPP_PATH_COUNT: u8 = 10;
+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;
@@ -222,17 +222,33 @@ pub struct PaymentParameters {
        /// 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_MPP_PATH_COUNT`].
-       pub max_mpp_path_count: u8,
+       /// 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_mpp_path_count, (default_value, DEFAULT_MAX_MPP_PATH_COUNT)),
+       (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),
 });
 
@@ -245,7 +261,8 @@ impl PaymentParameters {
                        route_hints: vec![],
                        expiry_time: None,
                        max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA,
-                       max_mpp_path_count: DEFAULT_MAX_MPP_PATH_COUNT,
+                       max_path_count: DEFAULT_MAX_PATH_COUNT,
+                       max_channel_saturation_power_of_half: 1,
                }
        }
 
@@ -282,11 +299,18 @@ impl PaymentParameters {
                Self { max_total_cltv_expiry_delta, ..self }
        }
 
-       /// Includes a limit for the maximum number of payment paths that may be used by MPP.
+       /// 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_mpp_path_count(self, max_mpp_path_count: u8) -> Self {
-               Self { max_mpp_path_count, ..self }
+       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 }
        }
 }
 
@@ -433,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 {
@@ -464,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.
@@ -571,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;
@@ -690,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)
 }
 
@@ -799,10 +840,16 @@ 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() {
@@ -810,10 +857,6 @@ where L::Target: Logger {
                } else { false }
        } else { false };
 
-       if allow_mpp && payment_params.max_mpp_path_count == 0 {
-               return Err(LightningError{err: "Can't find an MPP route with no paths allowed.".to_owned(), action: ErrorAction::IgnoreError});
-       }
-
        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 " });
@@ -871,14 +914,19 @@ where L::Target: Logger {
        // 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_mpp_path_count of the payment
+       // 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_mpp_path_count as u64 - 1)) / payment_params.max_mpp_path_count as u64
+               (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,
@@ -931,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
@@ -1081,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
@@ -1502,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,
@@ -1548,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;
                }
@@ -1557,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;
@@ -1673,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)})?,
@@ -1693,7 +1774,7 @@ 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_mpp_path_count.into());
+       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() {
@@ -1803,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)
 }
 
@@ -1847,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 {
@@ -1878,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};
@@ -4117,20 +4203,20 @@ mod tests {
                }
 
                {
-                       // Attempt to route while setting max_mpp_path_count to 0 results in a failure.
-                       let zero_payment_params = payment_params.clone().with_max_mpp_path_count(0);
+                       // 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 an MPP route with no paths allowed.");
+                                       assert_eq!(err, "Can't find a route with no paths allowed.");
                        } else { panic!(); }
                }
 
                {
-                       // Attempt to route while setting max_mpp_path_count to 3 results in a failure.
+                       // 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_mpp_path_count(3);
+                       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) {
@@ -4755,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);
        }
@@ -4779,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).
@@ -5227,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);
                }
 
@@ -5312,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 {
@@ -5330,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]
@@ -5713,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")))]