Re-broadcast our own gossip even if its same as the last broadcast
[rust-lightning] / lightning / src / routing / router.rs
index 93d3dc8f10a311f60763549014bac7faed27a2f9..8b2e4d137fd93f83fed0904fda12f6148a1e6834 100644 (file)
@@ -17,7 +17,7 @@ use bitcoin::secp256k1::key::PublicKey;
 use ln::channelmanager::ChannelDetails;
 use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
-use routing;
+use routing::scoring::Score;
 use routing::network_graph::{NetworkGraph, NodeId, RoutingFees};
 use util::ser::{Writeable, Readable};
 use util::logger::{Level, Logger};
@@ -203,7 +203,7 @@ impl_writeable_tlv_based!(Payee, {
 
 impl Payee {
        /// Creates a payee with the node id of the given `pubkey`.
-       pub fn new(pubkey: PublicKey) -> Self {
+       pub fn from_node_id(pubkey: PublicKey) -> Self {
                Self {
                        pubkey,
                        features: None,
@@ -214,7 +214,7 @@ impl Payee {
 
        /// Creates a payee with the node id of the given `pubkey` to use for keysend payments.
        pub fn for_keysend(pubkey: PublicKey) -> Self {
-               Self::new(pubkey).with_features(InvoiceFeatures::for_keysend())
+               Self::from_node_id(pubkey).with_features(InvoiceFeatures::for_keysend())
        }
 
        /// Includes the payee's features.
@@ -232,6 +232,8 @@ impl Payee {
        }
 
        /// Includes a payment expiration in seconds relative to the UNIX epoch.
+       ///
+       /// (C-not exported) since bindings don't support move semantics
        pub fn with_expiry_time(self, expiry_time: u64) -> Self {
                Self { expiry_time: Some(expiry_time), ..self }
        }
@@ -527,7 +529,7 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 ///
 /// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels
 /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
-pub fn find_route<L: Deref, S: routing::Score>(
+pub fn find_route<L: Deref, S: Score>(
        our_node_pubkey: &PublicKey, params: &RouteParameters, network: &NetworkGraph,
        first_hops: Option<&[&ChannelDetails]>, logger: L, scorer: &S
 ) -> Result<Route, LightningError>
@@ -538,7 +540,7 @@ where L::Target: Logger {
        )
 }
 
-pub(crate) fn get_route<L: Deref, S: routing::Score>(
+pub(crate) fn get_route<L: Deref, S: Score>(
        our_node_pubkey: &PublicKey, payee: &Payee, network: &NetworkGraph,
        first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
        logger: L, scorer: &S
@@ -890,9 +892,9 @@ where L::Target: Logger {
                                                                }
                                                        }
 
-                                                       let path_penalty_msat = $next_hops_path_penalty_msat
-                                                               .checked_add(scorer.channel_penalty_msat($chan_id.clone(), &$src_node_id, &$dest_node_id))
-                                                               .unwrap_or_else(|| u64::max_value());
+                                                       let path_penalty_msat = $next_hops_path_penalty_msat.checked_add(
+                                                               scorer.channel_penalty_msat($chan_id.clone(), amount_to_transfer_over_msat, Some(*available_liquidity_msat),
+                                                                       &$src_node_id, &$dest_node_id)).unwrap_or_else(|| u64::max_value());
                                                        let new_graph_node = RouteGraphNode {
                                                                node_id: $src_node_id,
                                                                lowest_fee_to_peer_through_node: total_fee_msat,
@@ -1111,22 +1113,28 @@ where L::Target: Logger {
                                                fees: hop.fees,
                                        };
 
-                                       let reqd_channel_cap = if let Some (val) = final_value_msat.checked_add(match idx {
-                                               0 => 999,
-                                               _ => aggregate_next_hops_fee_msat.checked_add(999).unwrap_or(u64::max_value())
-                                       }) { Some( val / 1000 ) } else { break; }; // converting from msat or breaking if max ~ infinity
+                                       // We want a value of final_value_msat * ROUTE_CAPACITY_PROVISION_FACTOR but we
+                                       // need it to increment at each hop by the fee charged at later hops. Further,
+                                       // we need to ensure we round up when we divide to get satoshis.
+                                       let channel_cap_msat = final_value_msat
+                                               .checked_mul(ROUTE_CAPACITY_PROVISION_FACTOR).and_then(|v| v.checked_add(aggregate_next_hops_fee_msat))
+                                               .unwrap_or(u64::max_value());
+                                       let channel_cap_sat = match channel_cap_msat.checked_add(999) {
+                                               None => break, // We overflowed above, just ignore this route hint
+                                               Some(val) => Some(val / 1000),
+                                       };
 
                                        let src_node_id = NodeId::from_pubkey(&hop.src_node_id);
                                        let dest_node_id = NodeId::from_pubkey(&prev_hop_id);
                                        aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
-                                               .checked_add(scorer.channel_penalty_msat(hop.short_channel_id, &src_node_id, &dest_node_id))
+                                               .checked_add(scorer.channel_penalty_msat(hop.short_channel_id, final_value_msat, None, &src_node_id, &dest_node_id))
                                                .unwrap_or_else(|| u64::max_value());
 
                                        // We assume that the recipient only included route hints for routes which had
                                        // sufficient value to route `final_value_msat`. Note that in the case of "0-value"
                                        // invoices where the invoice does not specify value this may not be the case, but
                                        // better to include the hints than not.
-                                       if !add_entry!(hop.short_channel_id, src_node_id, dest_node_id, directional_info, reqd_channel_cap, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat) {
+                                       if !add_entry!(hop.short_channel_id, src_node_id, dest_node_id, directional_info, channel_cap_sat, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat) {
                                                // If this hop was not used then there is no use checking the preceding hops
                                                // in the RouteHint. We can break by just searching for a direct channel between
                                                // last checked hop and first_hop_targets
@@ -1470,10 +1478,9 @@ where L::Target: Logger {
 
 #[cfg(test)]
 mod tests {
-       use routing;
+       use routing::scoring::Score;
        use routing::network_graph::{NetworkGraph, NetGraphMsgHandler, NodeId};
        use routing::router::{get_route, Payee, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees};
-       use routing::scorer::Scorer;
        use chain::transaction::OutPoint;
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
        use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
@@ -1942,8 +1949,8 @@ mod tests {
        fn simple_route_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[2]);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Simple route to 2 via 1
 
@@ -1973,8 +1980,8 @@ mod tests {
        fn invalid_first_hop_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[2]);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Simple route to 2 via 1
 
@@ -1992,8 +1999,8 @@ mod tests {
        fn htlc_minimum_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[2]);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Simple route to 2 via 1
 
@@ -2117,8 +2124,8 @@ mod tests {
        fn htlc_minimum_overpay_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known());
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // A route to node#2 via two paths.
                // One path allows transferring 35-40 sats, another one also allows 35-40 sats.
@@ -2253,8 +2260,8 @@ mod tests {
        fn disable_channels_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[2]);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // // Disable channels 4 and 12 by flags=2
                update_channel(&net_graph_msg_handler, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
@@ -2311,8 +2318,8 @@ mod tests {
        fn disable_node_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[2]);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Disable nodes 1, 2, and 8 by requiring unknown feature bits
                let unknown_features = NodeFeatures::known().set_unknown_feature_required();
@@ -2353,10 +2360,10 @@ mod tests {
        fn our_chans_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Route to 1 via 2 and 3 because our channel to 1 is disabled
-               let payee = Payee::new(nodes[0]);
+               let payee = Payee::from_node_id(nodes[0]);
                let route = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 3);
 
@@ -2382,7 +2389,7 @@ mod tests {
                assert_eq!(route.paths[0][2].channel_features.le_flags(), &id_to_feature_flags(3));
 
                // If we specify a channel to node7, that overrides our local channel view and that gets used
-               let payee = Payee::new(nodes[2]);
+               let payee = Payee::from_node_id(nodes[2]);
                let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &payee, &network_graph, Some(&our_chans.iter().collect::<Vec<_>>()), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
@@ -2482,7 +2489,7 @@ mod tests {
        fn partial_route_hint_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Simple test across 2, 3, 5, and 4 via a last_hop channel
                // Tests the behaviour when the RouteHint contains a suboptimal hop.
@@ -2504,13 +2511,13 @@ mod tests {
                let mut invalid_last_hops = last_hops_multi_private_channels(&nodes);
                invalid_last_hops.push(invalid_last_hop);
                {
-                       let payee = Payee::new(nodes[6]).with_route_hints(invalid_last_hops);
+                       let payee = Payee::from_node_id(nodes[6]).with_route_hints(invalid_last_hops);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Route hint cannot have the payee as the source.");
                        } else { panic!(); }
                }
 
-               let payee = Payee::new(nodes[6]).with_route_hints(last_hops_multi_private_channels(&nodes));
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops_multi_private_channels(&nodes));
                let route = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 5);
 
@@ -2580,8 +2587,8 @@ mod tests {
        fn ignores_empty_last_hops_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[6]).with_route_hints(empty_last_hop(&nodes));
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(empty_last_hop(&nodes));
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Test handling of an empty RouteHint passed in Invoice.
 
@@ -2662,8 +2669,8 @@ mod tests {
        fn multi_hint_last_hops_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[6]).with_route_hints(multi_hint_last_hops(&nodes));
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(multi_hint_last_hops(&nodes));
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
                // Test through channels 2, 3, 5, 8.
                // Test shows that multiple hop hints are considered.
 
@@ -2768,8 +2775,8 @@ mod tests {
        fn last_hops_with_public_channel_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[6]).with_route_hints(last_hops_with_public_channel(&nodes));
-               let scorer = Scorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops_with_public_channel(&nodes));
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
                // This test shows that public routes can be present in the invoice
                // which would be handled in the same manner.
 
@@ -2818,12 +2825,12 @@ mod tests {
        fn our_chans_last_hop_connect_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // Simple test with outbound channel to 4 to test that last_hops and first_hops connect
                let our_chans = vec![get_channel_details(Some(42), nodes[3].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let mut last_hops = last_hops(&nodes);
-               let payee = Payee::new(nodes[6]).with_route_hints(last_hops.clone());
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops.clone());
                let route = get_route(&our_id, &payee, &network_graph, Some(&our_chans.iter().collect::<Vec<_>>()), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
 
@@ -2844,7 +2851,7 @@ mod tests {
                last_hops[0].0[0].fees.base_msat = 1000;
 
                // Revert to via 6 as the fee on 8 goes up
-               let payee = Payee::new(nodes[6]).with_route_hints(last_hops);
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops);
                let route = get_route(&our_id, &payee, &network_graph, None, 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 4);
 
@@ -2937,9 +2944,9 @@ mod tests {
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: last_hop_htlc_max,
                }]);
-               let payee = Payee::new(target_node_id).with_route_hints(vec![last_hops]);
+               let payee = Payee::from_node_id(target_node_id).with_route_hints(vec![last_hops]);
                let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)];
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
                get_route(&source_node_id, &payee, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), Some(&our_chans.iter().collect::<Vec<_>>()), route_val, 42, &test_utils::TestLogger::new(), &scorer)
        }
 
@@ -2993,8 +3000,8 @@ mod tests {
 
                let (secp_ctx, network_graph, mut net_graph_msg_handler, chain_monitor, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
 
                // We will use a simple single-path route from
                // our node to node2 via node0: channels {1, 3}.
@@ -3265,8 +3272,8 @@ mod tests {
                // one of the latter hops is limited.
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known());
 
                // Path via {node7, node2, node4} is channels {12, 13, 6, 11}.
                // {12, 13, 11} have the capacities of 100, {6} has a capacity of 50.
@@ -3388,8 +3395,8 @@ mod tests {
        fn ignore_fee_first_hop_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
 
                // Path via node0 is channels {1, 3}. Limit them to 100 and 50 sats (total limit 50).
                update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
@@ -3434,8 +3441,8 @@ mod tests {
        fn simple_mpp_route_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::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).
@@ -3565,8 +3572,8 @@ mod tests {
        fn long_mpp_route_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known());
 
                // We need a route consisting of 3 paths:
                // From our node to node3 via {node0, node2}, {node7, node2, node4} and {node7, node2}.
@@ -3727,8 +3734,8 @@ mod tests {
        fn mpp_cheaper_route_test() {
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known());
 
                // This test checks that if we have two cheaper paths and one more expensive path,
                // so that liquidity-wise any 2 of 3 combination is sufficient,
@@ -3894,8 +3901,8 @@ mod tests {
                // if the fee is not properly accounted for, the behavior is different.
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[3]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[3]).with_features(InvoiceFeatures::known());
 
                // We need a route consisting of 2 paths:
                // From our node to node3 via {node0, node2} and {node7, node2, node4}.
@@ -4054,7 +4061,105 @@ mod tests {
                        assert_eq!(total_amount_paid_msat, 200_000);
                        assert_eq!(route.get_total_fees(), 150_000);
                }
+       }
+
+       #[test]
+       fn mpp_with_last_hops() {
+               // Previously, if we tried to send an MPP payment to a destination which was only reachable
+               // via a single last-hop route hint, we'd fail to route if we first collected routes
+               // totaling close but not quite enough to fund the full payment.
+               //
+               // This was because we considered last-hop hints to have exactly the sought payment amount
+               // instead of the amount we were trying to collect, needlessly limiting our path searching
+               // at the very first hop.
+               //
+               // Specifically, this interacted with our "all paths must fund at least 5% of total target"
+               // criterion to cause us to refuse all routes at the last hop hint which would be considered
+               // to only have the remaining to-collect amount in available liquidity.
+               //
+               // This bug appeared in production in some specific channel configurations.
+               let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
+               let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(PublicKey::from_slice(&[02; 33]).unwrap()).with_features(InvoiceFeatures::known())
+                       .with_route_hints(vec![RouteHint(vec![RouteHintHop {
+                               src_node_id: nodes[2],
+                               short_channel_id: 42,
+                               fees: RoutingFees { base_msat: 0, proportional_millionths: 0 },
+                               cltv_expiry_delta: 42,
+                               htlc_minimum_msat: None,
+                               htlc_maximum_msat: None,
+                       }])]);
+
+               // Keep only two paths from us to nodes[2], both with a 99sat HTLC maximum, with one with
+               // no fee and one with a 1msat fee. Previously, trying to route 100 sats to nodes[2] here
+               // would first use the no-fee route and then fail to find a path along the second route as
+               // we think we can only send up to 1 additional sat over the last-hop but refuse to as its
+               // under 5% of our payment amount.
+               update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 1,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: u16::max_value(),
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: OptionalField::Present(99_000),
+                       fee_base_msat: u32::max_value(),
+                       fee_proportional_millionths: u32::max_value(),
+                       excess_data: Vec::new()
+               });
+               update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 2,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: u16::max_value(),
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: OptionalField::Present(99_000),
+                       fee_base_msat: u32::max_value(),
+                       fee_proportional_millionths: u32::max_value(),
+                       excess_data: Vec::new()
+               });
+               update_channel(&net_graph_msg_handler, &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 << 8) | 1,
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: OptionalField::Absent,
+                       fee_base_msat: 1,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+               update_channel(&net_graph_msg_handler, &secp_ctx, &privkeys[7], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 13,
+                       timestamp: 2,
+                       flags: 0|2, // Channel disabled
+                       cltv_expiry_delta: (13 << 8) | 1,
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: OptionalField::Absent,
+                       fee_base_msat: 0,
+                       fee_proportional_millionths: 2000000,
+                       excess_data: Vec::new()
+               });
 
+               // 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, &payee, &network_graph, None, 100_000, 42, Arc::clone(&logger), &scorer).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);
+               assert_eq!(route.get_total_fees(), 1);
+               assert_eq!(route.get_total_amount(), 100_000);
        }
 
        #[test]
@@ -4063,8 +4168,8 @@ mod tests {
                // path finding we realize that we found more capacity than we need.
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::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).
@@ -4220,8 +4325,8 @@ mod tests {
                let network_graph = Arc::new(NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()));
                let net_graph_msg_handler = NetGraphMsgHandler::new(Arc::clone(&network_graph), None, Arc::clone(&logger));
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[6]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[6]);
 
                add_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6);
                update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
@@ -4349,8 +4454,8 @@ mod tests {
                // we calculated fees on a higher value, resulting in us ignoring such paths.
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[2]);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]);
 
                // We modify the graph to set the htlc_maximum of channel 2 to below the value we wish to
                // send.
@@ -4411,8 +4516,8 @@ mod tests {
                // resulting in us thinking there is no possible path, even if other paths exist.
                let (secp_ctx, network_graph, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[2]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
 
                // We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2
                // gets an htlc_maximum_msat of 80_000 and channel 4 an htlc_minimum_msat of 90_000. We
@@ -4478,8 +4583,8 @@ mod tests {
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let logger = Arc::new(test_utils::TestLogger::new());
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
-               let scorer = Scorer::with_fixed_penalty(0);
-               let payee = Payee::new(nodes[0]).with_features(InvoiceFeatures::known());
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
+               let payee = Payee::from_node_id(nodes[0]).with_features(InvoiceFeatures::known());
 
                {
                        let route = get_route(&our_id, &payee, &network_graph, Some(&[
@@ -4516,10 +4621,10 @@ mod tests {
        fn prefers_shorter_route_with_higher_fees() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[6]).with_route_hints(last_hops(&nodes));
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes));
 
                // Without penalizing each hop 100 msats, a longer path with lower fees is chosen.
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
                let route = get_route(
                        &our_id, &payee, &network_graph, None, 100, 42,
                        Arc::clone(&logger), &scorer
@@ -4532,7 +4637,7 @@ mod tests {
 
                // Applying a 100 msat penalty to each hop results in taking channels 7 and 10 to nodes[6]
                // from nodes[2] rather than channel 6, 11, and 8, even though the longer path is cheaper.
-               let scorer = Scorer::with_fixed_penalty(100);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(100);
                let route = get_route(
                        &our_id, &payee, &network_graph, None, 100, 42,
                        Arc::clone(&logger), &scorer
@@ -4548,34 +4653,34 @@ mod tests {
                short_channel_id: u64,
        }
 
-       impl routing::Score for BadChannelScorer {
-               fn channel_penalty_msat(&self, short_channel_id: u64, _source: &NodeId, _target: &NodeId) -> u64 {
+       impl Score for BadChannelScorer {
+               fn channel_penalty_msat(&self, short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, _target: &NodeId) -> u64 {
                        if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
                }
 
-               fn payment_path_failed(&mut self, _path: &Vec<RouteHop>, _short_channel_id: u64) {}
+               fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
        }
 
        struct BadNodeScorer {
                node_id: NodeId,
        }
 
-       impl routing::Score for BadNodeScorer {
-               fn channel_penalty_msat(&self, _short_channel_id: u64, _source: &NodeId, target: &NodeId) -> u64 {
+       impl Score for BadNodeScorer {
+               fn channel_penalty_msat(&self, _short_channel_id: u64, _send_amt: u64, _chan_amt: Option<u64>, _source: &NodeId, target: &NodeId) -> u64 {
                        if *target == self.node_id { u64::max_value() } else { 0 }
                }
 
-               fn payment_path_failed(&mut self, _path: &Vec<RouteHop>, _short_channel_id: u64) {}
+               fn payment_path_failed(&mut self, _path: &[&RouteHop], _short_channel_id: u64) {}
        }
 
        #[test]
        fn avoids_routing_through_bad_channels_and_nodes() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let payee = Payee::new(nodes[6]).with_route_hints(last_hops(&nodes));
+               let payee = Payee::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes));
 
                // A path to nodes[6] exists when no penalties are applied to any channel.
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
                let route = get_route(
                        &our_id, &payee, &network_graph, None, 100, 42,
                        Arc::clone(&logger), &scorer
@@ -4704,7 +4809,7 @@ mod tests {
                        },
                };
                let graph = NetworkGraph::read(&mut d).unwrap();
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
                let mut seed = random_init_seed() as usize;
@@ -4715,7 +4820,7 @@ mod tests {
                                let src = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed = seed.overflowing_mul(0xdeadbeef).0;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               let payee = Payee::new(dst);
+                               let payee = Payee::from_node_id(dst);
                                let amt = seed as u64 % 200_000_000;
                                if get_route(src, &payee, &graph, None, amt, 42, &test_utils::TestLogger::new(), &scorer).is_ok() {
                                        continue 'load_endpoints;
@@ -4735,7 +4840,7 @@ mod tests {
                        },
                };
                let graph = NetworkGraph::read(&mut d).unwrap();
-               let scorer = Scorer::with_fixed_penalty(0);
+               let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
                let mut seed = random_init_seed() as usize;
@@ -4746,7 +4851,7 @@ mod tests {
                                let src = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed = seed.overflowing_mul(0xdeadbeef).0;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               let payee = Payee::new(dst).with_features(InvoiceFeatures::known());
+                               let payee = Payee::from_node_id(dst).with_features(InvoiceFeatures::known());
                                let amt = seed as u64 % 200_000_000;
                                if get_route(src, &payee, &graph, None, amt, 42, &test_utils::TestLogger::new(), &scorer).is_ok() {
                                        continue 'load_endpoints;
@@ -4786,7 +4891,7 @@ pub(crate) mod test_utils {
 #[cfg(all(test, feature = "unstable", not(feature = "no-std")))]
 mod benches {
        use super::*;
-       use routing::scorer::Scorer;
+       use routing::scoring::Scorer;
        use util::logger::{Logger, Record};
 
        use test::Bencher;
@@ -4812,7 +4917,7 @@ mod benches {
                                let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed *= 0xdeadbeef;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               let payee = Payee::new(dst);
+                               let payee = Payee::from_node_id(dst);
                                let amt = seed as u64 % 1_000_000;
                                if get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok() {
                                        path_endpoints.push((src, dst, amt));
@@ -4825,7 +4930,7 @@ mod benches {
                let mut idx = 0;
                bench.iter(|| {
                        let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
-                       let payee = Payee::new(dst);
+                       let payee = Payee::from_node_id(dst);
                        assert!(get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok());
                        idx += 1;
                });
@@ -4847,7 +4952,7 @@ mod benches {
                                let src = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                seed *= 0xdeadbeef;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
-                               let payee = Payee::new(dst).with_features(InvoiceFeatures::known());
+                               let payee = Payee::from_node_id(dst).with_features(InvoiceFeatures::known());
                                let amt = seed as u64 % 1_000_000;
                                if get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok() {
                                        path_endpoints.push((src, dst, amt));
@@ -4860,7 +4965,7 @@ mod benches {
                let mut idx = 0;
                bench.iter(|| {
                        let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
-                       let payee = Payee::new(dst).with_features(InvoiceFeatures::known());
+                       let payee = Payee::from_node_id(dst).with_features(InvoiceFeatures::known());
                        assert!(get_route(&src, &payee, &graph, None, amt, 42, &DummyLogger{}, &scorer).is_ok());
                        idx += 1;
                });