Improve ChannelDetails readability significantly.
[rust-lightning] / lightning / src / routing / router.rs
index 1e7d322c995558d8651c7b63b6fd5a5e78193fa8..6c14ebebd89b70d55b1e4f59af827a9f0a6b1cde 100644 (file)
@@ -21,9 +21,10 @@ use routing::network_graph::{NetworkGraph, RoutingFees};
 use util::ser::{Writeable, Readable};
 use util::logger::Logger;
 
-use std::cmp;
-use std::collections::{HashMap, BinaryHeap};
-use std::ops::Deref;
+use prelude::*;
+use alloc::collections::BinaryHeap;
+use core::cmp;
+use core::ops::Deref;
 
 /// A hop in a route
 #[derive(Clone, PartialEq)]
@@ -47,40 +48,14 @@ pub struct RouteHop {
        pub cltv_expiry_delta: u32,
 }
 
-/// (C-not exported)
-impl Writeable for Vec<RouteHop> {
-       fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-               (self.len() as u8).write(writer)?;
-               for hop in self.iter() {
-                       hop.pubkey.write(writer)?;
-                       hop.node_features.write(writer)?;
-                       hop.short_channel_id.write(writer)?;
-                       hop.channel_features.write(writer)?;
-                       hop.fee_msat.write(writer)?;
-                       hop.cltv_expiry_delta.write(writer)?;
-               }
-               Ok(())
-       }
-}
-
-/// (C-not exported)
-impl Readable for Vec<RouteHop> {
-       fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Vec<RouteHop>, DecodeError> {
-               let hops_count: u8 = Readable::read(reader)?;
-               let mut hops = Vec::with_capacity(hops_count as usize);
-               for _ in 0..hops_count {
-                       hops.push(RouteHop {
-                               pubkey: Readable::read(reader)?,
-                               node_features: Readable::read(reader)?,
-                               short_channel_id: Readable::read(reader)?,
-                               channel_features: Readable::read(reader)?,
-                               fee_msat: Readable::read(reader)?,
-                               cltv_expiry_delta: Readable::read(reader)?,
-                       });
-               }
-               Ok(hops)
-       }
-}
+impl_writeable_tlv_based!(RouteHop, {
+       (0, pubkey, required),
+       (2, node_features, required),
+       (4, short_channel_id, required),
+       (6, channel_features, required),
+       (8, fee_msat, required),
+       (10, cltv_expiry_delta, required),
+});
 
 /// A route directs a payment from the sender (us) to the recipient. If the recipient supports MPP,
 /// it can take multiple paths. Each path is composed of one or more hops through the network.
@@ -95,30 +70,49 @@ pub struct Route {
        pub paths: Vec<Vec<RouteHop>>,
 }
 
+const SERIALIZATION_VERSION: u8 = 1;
+const MIN_SERIALIZATION_VERSION: u8 = 1;
+
 impl Writeable for Route {
        fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
+               write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
                (self.paths.len() as u64).write(writer)?;
                for hops in self.paths.iter() {
-                       hops.write(writer)?;
+                       (hops.len() as u8).write(writer)?;
+                       for hop in hops.iter() {
+                               hop.write(writer)?;
+                       }
                }
+               write_tlv_fields!(writer, {});
                Ok(())
        }
 }
 
 impl Readable for Route {
        fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Route, DecodeError> {
+               let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
                let path_count: u64 = Readable::read(reader)?;
                let mut paths = Vec::with_capacity(cmp::min(path_count, 128) as usize);
                for _ in 0..path_count {
-                       paths.push(Readable::read(reader)?);
+                       let hop_count: u8 = Readable::read(reader)?;
+                       let mut hops = Vec::with_capacity(hop_count as usize);
+                       for _ in 0..hop_count {
+                               hops.push(Readable::read(reader)?);
+                       }
+                       paths.push(hops);
                }
+               read_tlv_fields!(reader, {});
                Ok(Route { paths })
        }
 }
 
-/// A channel descriptor which provides a last-hop route to get_route
-#[derive(Clone)]
-pub struct RouteHint {
+/// A list of hops along a payment path terminating with a channel to the recipient.
+#[derive(Eq, PartialEq, Debug, Clone)]
+pub struct RouteHint(pub Vec<RouteHintHop>);
+
+/// A channel descriptor for a hop along a payment path.
+#[derive(Eq, PartialEq, Debug, Clone)]
+pub struct RouteHintHop {
        /// The node_id of the non-target end of the route
        pub src_node_id: PublicKey,
        /// The short_channel_id of this channel
@@ -174,11 +168,16 @@ struct DummyDirectionalChannelInfo {
 /// 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.
 /// These fee values are useful to choose hops as we traverse the graph "payee-to-payer".
-#[derive(Clone)]
-struct PathBuildingHop {
-       /// Hop-specific details unrelated to the path during the routing phase,
-       /// but rather relevant to the LN graph.
-       route_hop: RouteHop,
+#[derive(Clone, Debug)]
+struct PathBuildingHop<'a> {
+       // The RouteHintHop fields which will eventually be used if this hop is used in a final Route.
+       // Note that node_features is calculated separately after our initial graph walk.
+       pubkey: PublicKey,
+       short_channel_id: u64,
+       channel_features: &'a ChannelFeatures,
+       fee_msat: u64,
+       cltv_expiry_delta: u32,
+
        /// Minimal fees required to route to the source node of the current hop via any of its inbound channels.
        src_lowest_inbound_fees: RoutingFees,
        /// Fees of the channel used in this hop.
@@ -217,15 +216,14 @@ struct PathBuildingHop {
 // Instantiated with a list of hops with correct data in them collected during path finding,
 // an instance of this struct should be further modified only via given methods.
 #[derive(Clone)]
-struct PaymentPath {
-       hops: Vec<PathBuildingHop>,
+struct PaymentPath<'a> {
+       hops: Vec<(PathBuildingHop<'a>, NodeFeatures)>,
 }
 
-impl PaymentPath {
-
+impl<'a> PaymentPath<'a> {
        // TODO: Add a value_msat field to PaymentPath and use it instead of this function.
        fn get_value_msat(&self) -> u64 {
-               self.hops.last().unwrap().route_hop.fee_msat
+               self.hops.last().unwrap().0.fee_msat
        }
 
        fn get_total_fee_paid_msat(&self) -> u64 {
@@ -234,9 +232,9 @@ impl PaymentPath {
                }
                let mut result = 0;
                // Can't use next_hops_fee_msat because it gets outdated.
-               for (i, hop) in self.hops.iter().enumerate() {
+               for (i, (hop, _)) in self.hops.iter().enumerate() {
                        if i != self.hops.len() - 1 {
-                               result += hop.route_hop.fee_msat;
+                               result += hop.fee_msat;
                        }
                }
                return result;
@@ -245,16 +243,14 @@ impl PaymentPath {
        // If the amount transferred by the path is updated, the fees should be adjusted. Any other way
        // to change fees may result in an inconsistency.
        //
-       // Sometimes we call this function right after constructing a path which has inconsistent
-       // (in terms of reaching htlc_minimum_msat), so that this function puts the fees in order.
-       // In that case we call it on the "same" amount we initially allocated for this path, and which
-       // could have been reduced on the way. In that case, there is also a risk of exceeding
-       // available_liquidity inside this function, because the function is unaware of this bound.
-       // In our specific recomputation cases where we never increase the value the risk is pretty low.
-       // This function, however, does not support arbitrarily increasing the value being transferred,
-       // and the exception will be triggered.
+       // Sometimes we call this function right after constructing a path which is inconsistent in
+       // that it the value being transferred has decreased while we were doing path finding, leading
+       // 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.
        fn update_value_and_recompute_fees(&mut self, value_msat: u64) {
-               assert!(value_msat <= self.hops.last().unwrap().route_hop.fee_msat);
+               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() {
@@ -265,10 +261,10 @@ impl PaymentPath {
                        // htlc_minimum_msat of the current channel. Last hop is handled separately.
                        let mut cur_hop_fees_msat = 0;
                        if !last_hop {
-                               cur_hop_fees_msat = self.hops.get(i + 1).unwrap().hop_use_fee_msat;
+                               cur_hop_fees_msat = self.hops.get(i + 1).unwrap().0.hop_use_fee_msat;
                        }
 
-                       let mut cur_hop = self.hops.get_mut(i).unwrap();
+                       let mut cur_hop = &mut self.hops.get_mut(i).unwrap().0;
                        cur_hop.next_hops_fee_msat = total_fee_paid_msat;
                        // Overpay in fees if we can't save these funds due to htlc_minimum_msat.
                        // We try to account for htlc_minimum_msat in scoring (add_entry!), so that nodes don't
@@ -292,11 +288,11 @@ impl PaymentPath {
                        if last_hop {
                                // Final hop is a special case: it usually has just value_msat (by design), but also
                                // it still could overpay for the htlc_minimum_msat.
-                               cur_hop.route_hop.fee_msat = cur_hop_transferred_amount_msat;
+                               cur_hop.fee_msat = cur_hop_transferred_amount_msat;
                        } else {
                                // Propagate updated fees for the use of the channels to one hop back, where they
                                // will be actually paid (fee_msat). The last hop is handled above separately.
-                               cur_hop.route_hop.fee_msat = cur_hop_fees_msat;
+                               cur_hop.fee_msat = cur_hop_fees_msat;
                        }
 
                        // Fee for the use of the current hop which will be deducted on the previous hop.
@@ -336,8 +332,8 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 /// If the payee provided features in their invoice, they should be provided via payee_features.
 /// Without this, MPP will only be used if the payee's features are available in the network graph.
 ///
-/// Extra routing hops between known nodes and the target will be used if they are included in
-/// last_hops.
+/// Private routing paths between a public node and the target may be included in `last_hops`.
+/// Currently, only the last hop in each path is considered.
 ///
 /// If some channels aren't announced, it may be useful to fill in a first_hops with the
 /// results from a local ChannelManager::list_usable_channels() call. If it is filled in, our
@@ -366,7 +362,8 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                return Err(LightningError{err: "Cannot send a payment of 0 msat".to_owned(), action: ErrorAction::IgnoreError});
        }
 
-       for last_hop in last_hops {
+       let last_hops = last_hops.iter().filter_map(|hops| hops.0.last()).collect::<Vec<_>>();
+       for last_hop in last_hops.iter() {
                if last_hop.src_node_id == *payee {
                        return Err(LightningError{err: "Last hop cannot have a payee as a source.".to_owned(), action: ErrorAction::IgnoreError});
                }
@@ -442,22 +439,6 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                }
        };
 
-       let mut targets = BinaryHeap::new(); //TODO: Do we care about switching to eg Fibbonaci heap?
-       let mut dist = HashMap::with_capacity(network.get_nodes().len());
-
-       // During routing, if we ignore a path due to an htlc_minimum_msat limit, we set this,
-       // indicating that we may wish to try again with a higher value, potentially paying to meet an
-       // htlc_minimum with extra fees while still finding a cheaper path.
-       let mut hit_minimum_limit;
-
-       // When arranging a route, we select multiple paths so that we can make a multi-path payment.
-       // We start with a path_value of the exact amount we want, and if that generates a route we may
-       // return it immediately. Otherwise, we don't stop searching for paths until we have 3x the
-       // amount we want in total across paths, selecting the best subset at the end.
-       const ROUTE_CAPACITY_PROVISION_FACTOR: u64 = 3;
-       let recommended_value_msat = final_value_msat * ROUTE_CAPACITY_PROVISION_FACTOR as u64;
-       let mut path_value_msat = final_value_msat;
-
        // 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.
@@ -473,25 +454,50 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        // Prepare the data we'll use for payee-to-payer search by
        // inserting first hops suggested by the caller as targets.
        // Our search will then attempt to reach them while traversing from the payee node.
-       let mut first_hop_targets = HashMap::with_capacity(if first_hops.is_some() { first_hops.as_ref().unwrap().len() } else { 0 });
+       let mut first_hop_targets: HashMap<_, (_, ChannelFeatures, _, NodeFeatures)> =
+               HashMap::with_capacity(if first_hops.is_some() { first_hops.as_ref().unwrap().len() } else { 0 });
        if let Some(hops) = first_hops {
                for chan in hops {
                        let short_channel_id = chan.short_channel_id.expect("first_hops should be filled in with usable channels, not pending ones");
-                       if chan.remote_network_id == *our_node_id {
+                       if chan.counterparty.node_id == *our_node_id {
                                return Err(LightningError{err: "First hop cannot have our_node_id as a destination.".to_owned(), action: ErrorAction::IgnoreError});
                        }
-                       first_hop_targets.insert(chan.remote_network_id, (short_channel_id, chan.counterparty_features.clone(), chan.outbound_capacity_msat));
+                       first_hop_targets.insert(chan.counterparty.node_id, (short_channel_id, chan.counterparty.features.to_context(), chan.outbound_capacity_msat, chan.counterparty.features.to_context()));
                }
                if first_hop_targets.is_empty() {
                        return Err(LightningError{err: "Cannot route when there are no outbound routes away from us".to_owned(), action: ErrorAction::IgnoreError});
                }
        }
 
+       let empty_channel_features = ChannelFeatures::empty();
+
+       // The main heap containing all candidate next-hops sorted by their score (max(A* fee,
+       // htlc_minimum)). Ideally this would be a heap which allowed cheap score reduction instead of
+       // adding duplicate entries when we find a better path to a given node.
+       let mut targets = BinaryHeap::new();
+
+       // Map from node_id to information about the best current path to that node, including feerate
+       // information.
+       let mut dist = HashMap::with_capacity(network.get_nodes().len());
+
+       // During routing, if we ignore a path due to an htlc_minimum_msat limit, we set this,
+       // indicating that we may wish to try again with a higher value, potentially paying to meet an
+       // htlc_minimum with extra fees while still finding a cheaper path.
+       let mut hit_minimum_limit;
+
+       // When arranging a route, we select multiple paths so that we can make a multi-path payment.
+       // We start with a path_value of the exact amount we want, and if that generates a route we may
+       // return it immediately. Otherwise, we don't stop searching for paths until we have 3x the
+       // amount we want in total across paths, selecting the best subset at the end.
+       const ROUTE_CAPACITY_PROVISION_FACTOR: u64 = 3;
+       let recommended_value_msat = final_value_msat * ROUTE_CAPACITY_PROVISION_FACTOR as u64;
+       let mut path_value_msat = final_value_msat;
+
        // We don't want multiple paths (as per MPP) share liquidity of the same channels.
        // This map allows paths to be aware of the channel use by other paths in the same call.
        // This would help to make a better path finding decisions and not "overbook" channels.
        // It is unaware of the directions (except for `outbound_capacity_msat` in `first_hops`).
-       let mut bookkeeped_channels_liquidity_available_msat = HashMap::new();
+       let mut bookkeeped_channels_liquidity_available_msat = HashMap::with_capacity(network.get_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);
@@ -500,14 +506,19 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        // - when we want to stop looking for new paths.
        let mut already_collected_value_msat = 0;
 
+       log_trace!(logger, "Building path from {} (payee) to {} (us/payer) for value {} msat.", payee, our_node_id, final_value_msat);
+
        macro_rules! add_entry {
                // Adds entry which goes from $src_node_id to $dest_node_id
                // over the channel with id $chan_id with fees described in
                // $directional_info.
                // $next_hops_fee_msat represents the fees paid for using all the channel *after* this one,
                // since that value has to be transferred over this channel.
+               // Returns whether this channel caused an update to `targets`.
                ( $chan_id: expr, $src_node_id: expr, $dest_node_id: expr, $directional_info: expr, $capacity_sats: expr, $chan_features: expr, $next_hops_fee_msat: expr,
-                  $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => {
+                  $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => { {
+                       // We "return" whether we updated the path at the end, via this:
+                       let mut did_add_update_path_to_src_node = false;
                        // Channels to self should not be used. This is more of belt-and-suspenders, because in
                        // practice these cases should be caught earlier:
                        // - for regular channels at channel announcement (TODO)
@@ -609,14 +620,11 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                fee_proportional_millionths = fees.proportional_millionths;
                                                        }
                                                        PathBuildingHop {
-                                                               route_hop: RouteHop {
-                                                                       pubkey: $dest_node_id.clone(),
-                                                                       node_features: NodeFeatures::empty(),
-                                                                       short_channel_id: 0,
-                                                                       channel_features: $chan_features.clone(),
-                                                                       fee_msat: 0,
-                                                                       cltv_expiry_delta: 0,
-                                                               },
+                                                               pubkey: $dest_node_id.clone(),
+                                                               short_channel_id: 0,
+                                                               channel_features: $chan_features,
+                                                               fee_msat: 0,
+                                                               cltv_expiry_delta: 0,
                                                                src_lowest_inbound_fees: RoutingFees {
                                                                        base_msat: fee_base_msat,
                                                                        proportional_millionths: fee_proportional_millionths,
@@ -710,14 +718,11 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                old_entry.next_hops_fee_msat = $next_hops_fee_msat;
                                                                old_entry.hop_use_fee_msat = hop_use_fee_msat;
                                                                old_entry.total_fee_msat = total_fee_msat;
-                                                               old_entry.route_hop = RouteHop {
-                                                                       pubkey: $dest_node_id.clone(),
-                                                                       node_features: NodeFeatures::empty(),
-                                                                       short_channel_id: $chan_id.clone(),
-                                                                       channel_features: $chan_features.clone(),
-                                                                       fee_msat: 0, // This value will be later filled with hop_use_fee_msat of the following channel
-                                                                       cltv_expiry_delta: $directional_info.cltv_expiry_delta as u32,
-                                                               };
+                                                               old_entry.pubkey = $dest_node_id.clone();
+                                                               old_entry.short_channel_id = $chan_id.clone();
+                                                               old_entry.channel_features = $chan_features;
+                                                               old_entry.fee_msat = 0; // This value will be later filled with hop_use_fee_msat of the following channel
+                                                               old_entry.cltv_expiry_delta = $directional_info.cltv_expiry_delta as u32;
                                                                old_entry.channel_fees = $directional_info.fees;
                                                                old_entry.htlc_minimum_msat = $directional_info.htlc_minimum_msat;
                                                                old_entry.path_htlc_minimum_msat = path_htlc_minimum_msat;
@@ -725,6 +730,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                {
                                                                        old_entry.value_contribution_msat = value_contribution_msat;
                                                                }
+                                                               did_add_update_path_to_src_node = true;
                                                        } else if old_entry.was_processed && new_cost < old_cost {
                                                                #[cfg(any(test, feature = "fuzztarget"))]
                                                                {
@@ -755,9 +761,11 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                        }
                                }
                        }
-               };
+                       did_add_update_path_to_src_node
+               } }
        }
 
+       let empty_node_features = NodeFeatures::empty();
        // Find ways (channels with destination) to reach a given node and store them
        // in the corresponding data structures (routing graph etc).
        // $fee_to_target_msat represents how much it costs to reach to this node from the payee,
@@ -779,17 +787,16 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
 
                        if !skip_node {
                                if first_hops.is_some() {
-                                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&$node_id) {
-                                               add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
+                                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&$node_id) {
+                                               add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
                                        }
                                }
 
-                               let features;
-                               if let Some(node_info) = $node.announcement_info.as_ref() {
-                                       features = node_info.features.clone();
+                               let features = if let Some(node_info) = $node.announcement_info.as_ref() {
+                                       &node_info.features
                                } else {
-                                       features = NodeFeatures::empty();
-                               }
+                                       &empty_node_features
+                               };
 
                                if !features.requires_unknown_bits() {
                                        for chan_id in $node.channels.iter() {
@@ -800,7 +807,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                if first_hops.is_none() || chan.node_two != *our_node_id {
                                                                        if let Some(two_to_one) = chan.two_to_one.as_ref() {
                                                                                if two_to_one.enabled {
-                                                                                       add_entry!(chan_id, chan.node_two, chan.node_one, two_to_one, chan.capacity_sats, chan.features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
+                                                                                       add_entry!(chan_id, chan.node_two, chan.node_one, two_to_one, chan.capacity_sats, &chan.features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
                                                                                }
                                                                        }
                                                                }
@@ -808,7 +815,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                                                if first_hops.is_none() || chan.node_one != *our_node_id {
                                                                        if let Some(one_to_two) = chan.one_to_two.as_ref() {
                                                                                if one_to_two.enabled {
-                                                                                       add_entry!(chan_id, chan.node_one, chan.node_two, one_to_two, chan.capacity_sats, chan.features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
+                                                                                       add_entry!(chan_id, chan.node_one, chan.node_two, one_to_two, chan.capacity_sats, &chan.features, $fee_to_target_msat, $next_hops_value_contribution, $next_hops_path_htlc_minimum_msat);
                                                                                }
                                                                        }
                                                                }
@@ -834,8 +841,8 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                // If first hop is a private channel and the only way to reach the payee, this is the only
                // place where it could be added.
                if first_hops.is_some() {
-                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&payee) {
-                               add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, path_value_msat, 0);
+                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&payee) {
+                               add_entry!(first_hop, *our_node_id, payee, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0);
                        }
                }
 
@@ -858,22 +865,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                // it matters only if the fees are exactly the same.
                for hop in last_hops.iter() {
                        let have_hop_src_in_graph =
-                               if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat)) = first_hop_targets.get(&hop.src_node_id) {
-                                       // If this hop connects to a node with which we have a direct channel, ignore
-                                       // the network graph and add both the hop and our direct channel to
-                                       // the candidate set.
-                                       //
-                                       // Currently there are no channel-context features defined, so we are a
-                                       // bit lazy here. In the future, we should pull them out via our
-                                       // ChannelManager, but there's no reason to waste the space until we
-                                       // need them.
-                                       add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features.to_context(), 0, path_value_msat, 0);
-                                       true
-                               } else {
-                                       // In any other case, only add the hop if the source is in the regular network
-                                       // graph:
-                                       network.get_nodes().get(&hop.src_node_id).is_some()
-                               };
+                               // Only add the last hop to our candidate set if either we have a direct channel or
+                               // they are in the regular network graph.
+                               first_hop_targets.get(&hop.src_node_id).is_some() ||
+                               network.get_nodes().get(&hop.src_node_id).is_some();
                        if have_hop_src_in_graph {
                                // BOLT 11 doesn't allow inclusion of features for the last hop hints, which
                                // really sucks, cause we're gonna need that eventually.
@@ -887,10 +882,23 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                        htlc_maximum_msat: hop.htlc_maximum_msat,
                                        fees: hop.fees,
                                };
-                               add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, ChannelFeatures::empty(), 0, path_value_msat, 0);
+                               if add_entry!(hop.short_channel_id, hop.src_node_id, payee, directional_info, None::<u64>, &empty_channel_features, 0, path_value_msat, 0) {
+                                       // If this hop connects to a node with which we have a direct channel,
+                                       // ignore the network graph and, if the last hop was added, add our
+                                       // direct channel to the candidate set.
+                                       //
+                                       // Note that we *must* check if the last hop was added as `add_entry`
+                                       // always assumes that the third argument is a node to which we have a
+                                       // path.
+                                       if let Some(&(ref first_hop, ref features, ref outbound_capacity_msat, _)) = first_hop_targets.get(&hop.src_node_id) {
+                                               add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0);
+                                       }
+                               }
                        }
                }
 
+               log_trace!(logger, "Starting main path collection loop with {} nodes pre-filled from first/last hops.", targets.len());
+
                // At this point, targets are filled with the data from first and
                // last hops communicated by the caller, and the payment receiver.
                let mut found_new_path = false;
@@ -910,34 +918,34 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                        // traversing the graph and arrange the path out of what we found.
                        if pubkey == *our_node_id {
                                let mut new_entry = dist.remove(&our_node_id).unwrap();
-                               let mut ordered_hops = vec!(new_entry.clone());
+                               let mut ordered_hops = vec!((new_entry.clone(), NodeFeatures::empty()));
 
                                'path_walk: loop {
-                                       if let Some(&(_, ref features, _)) = first_hop_targets.get(&ordered_hops.last().unwrap().route_hop.pubkey) {
-                                               ordered_hops.last_mut().unwrap().route_hop.node_features = features.to_context();
-                                       } else if let Some(node) = network.get_nodes().get(&ordered_hops.last().unwrap().route_hop.pubkey) {
+                                       if let Some(&(_, _, _, ref features)) = first_hop_targets.get(&ordered_hops.last().unwrap().0.pubkey) {
+                                               ordered_hops.last_mut().unwrap().1 = features.clone();
+                                       } else if let Some(node) = network.get_nodes().get(&ordered_hops.last().unwrap().0.pubkey) {
                                                if let Some(node_info) = node.announcement_info.as_ref() {
-                                                       ordered_hops.last_mut().unwrap().route_hop.node_features = node_info.features.clone();
+                                                       ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
                                                } else {
-                                                       ordered_hops.last_mut().unwrap().route_hop.node_features = NodeFeatures::empty();
+                                                       ordered_hops.last_mut().unwrap().1 = NodeFeatures::empty();
                                                }
                                        } else {
                                                // We should be able to fill in features for everything except the last
                                                // hop, if the last hop was provided via a BOLT 11 invoice (though we
                                                // should be able to extend it further as BOLT 11 does have feature
                                                // flags for the last hop node itself).
-                                               assert!(ordered_hops.last().unwrap().route_hop.pubkey == *payee);
+                                               assert!(ordered_hops.last().unwrap().0.pubkey == *payee);
                                        }
 
                                        // Means we succesfully traversed from the payer to the payee, now
                                        // save this path for the payment route. Also, update the liquidity
                                        // remaining on the used hops, so that we take them into account
                                        // while looking for more paths.
-                                       if ordered_hops.last().unwrap().route_hop.pubkey == *payee {
+                                       if ordered_hops.last().unwrap().0.pubkey == *payee {
                                                break 'path_walk;
                                        }
 
-                                       new_entry = match dist.remove(&ordered_hops.last().unwrap().route_hop.pubkey) {
+                                       new_entry = match dist.remove(&ordered_hops.last().unwrap().0.pubkey) {
                                                Some(payment_hop) => payment_hop,
                                                // We can't arrive at None because, if we ever add an entry to targets,
                                                // we also fill in the entry in dist (see add_entry!).
@@ -946,13 +954,16 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                        // We "propagate" the fees one hop backward (topologically) here,
                                        // so that fees paid for a HTLC forwarding on the current channel are
                                        // associated with the previous channel (where they will be subtracted).
-                                       ordered_hops.last_mut().unwrap().route_hop.fee_msat = new_entry.hop_use_fee_msat;
-                                       ordered_hops.last_mut().unwrap().route_hop.cltv_expiry_delta = new_entry.route_hop.cltv_expiry_delta;
-                                       ordered_hops.push(new_entry.clone());
+                                       ordered_hops.last_mut().unwrap().0.fee_msat = new_entry.hop_use_fee_msat;
+                                       ordered_hops.last_mut().unwrap().0.cltv_expiry_delta = new_entry.cltv_expiry_delta;
+                                       ordered_hops.push((new_entry.clone(), NodeFeatures::empty()));
                                }
-                               ordered_hops.last_mut().unwrap().route_hop.fee_msat = value_contribution_msat;
-                               ordered_hops.last_mut().unwrap().hop_use_fee_msat = 0;
-                               ordered_hops.last_mut().unwrap().route_hop.cltv_expiry_delta = final_cltv;
+                               ordered_hops.last_mut().unwrap().0.fee_msat = value_contribution_msat;
+                               ordered_hops.last_mut().unwrap().0.hop_use_fee_msat = 0;
+                               ordered_hops.last_mut().unwrap().0.cltv_expiry_delta = final_cltv;
+
+                               log_trace!(logger, "Found a path back to us from the target with {} hops contributing up to {} msat: {:?}",
+                                       ordered_hops.len(), value_contribution_msat, ordered_hops);
 
                                let mut payment_path = PaymentPath {hops: ordered_hops};
 
@@ -973,16 +984,11 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                // Remember that we used these channels so that we don't rely
                                // on the same liquidity in future paths.
                                let mut prevented_redundant_path_selection = false;
-                               for payment_hop in payment_path.hops.iter() {
-                                       let channel_liquidity_available_msat = bookkeeped_channels_liquidity_available_msat.get_mut(&payment_hop.route_hop.short_channel_id).unwrap();
+                               for (payment_hop, _) in payment_path.hops.iter() {
+                                       let channel_liquidity_available_msat = bookkeeped_channels_liquidity_available_msat.get_mut(&payment_hop.short_channel_id).unwrap();
                                        let mut spent_on_hop_msat = value_contribution_msat;
                                        let next_hops_fee_msat = payment_hop.next_hops_fee_msat;
                                        spent_on_hop_msat += next_hops_fee_msat;
-                                       if *channel_liquidity_available_msat < spent_on_hop_msat {
-                                               // This should not happen because we do recompute fees right before,
-                                               // trying to avoid cases when a hop is not usable due to the fee situation.
-                                               break 'path_construction;
-                                       }
                                        if spent_on_hop_msat == *channel_liquidity_available_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.
@@ -994,8 +1000,9 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                        // If we weren't capped by hitting a liquidity limit on a channel in the path,
                                        // we'll probably end up picking the same path again on the next iteration.
                                        // Decrease the available liquidity of a hop in the middle of the path.
-                                       let victim_liquidity = bookkeeped_channels_liquidity_available_msat.get_mut(
-                                               &payment_path.hops[(payment_path.hops.len() - 1) / 2].route_hop.short_channel_id).unwrap();
+                                       let victim_scid = payment_path.hops[(payment_path.hops.len() - 1) / 2].0.short_channel_id;
+                                       log_trace!(logger, "Disabling channel {} for future path building iterations to avoid duplicates.", victim_scid);
+                                       let victim_liquidity = bookkeeped_channels_liquidity_available_msat.get_mut(&victim_scid).unwrap();
                                        *victim_liquidity = 0;
                                }
 
@@ -1037,6 +1044,8 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                // 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 {
+                       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;
                } else if found_new_path && already_collected_value_msat == final_value_msat && payment_paths.len() == 1 {
                        // Further, if this was our first walk of the graph, and we weren't limited by an
@@ -1045,8 +1054,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                        // potentially allowing us to pay fees to meet the htlc_minimum on the new path while
                        // still keeping a lower total fee than this path.
                        if !hit_minimum_limit {
+                               log_trace!(logger, "Collected exactly our payment amount on the first pass, without hitting an htlc_minimum_msat limit, exiting.");
                                break 'paths_collection;
                        }
+                       log_trace!(logger, "Collected our payment amount on the first pass, but running again to collect extra paths with a potentially higher limit.");
                        path_value_msat = recommended_value_msat;
                }
        }
@@ -1120,7 +1131,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                // Now, substract the overpaid value from the most-expensive path.
                                // TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
                                // so that the sender pays less fees overall. And also htlc_minimum_msat.
-                               cur_route.sort_by_key(|path| { path.hops.iter().map(|hop| hop.channel_fees.proportional_millionths as u64).sum::<u64>() });
+                               cur_route.sort_by_key(|path| { path.hops.iter().map(|hop| hop.0.channel_fees.proportional_millionths as u64).sum::<u64>() });
                                let expensive_payment_path = cur_route.first_mut().unwrap();
                                // We already dropped all the small channels above, meaning all the
                                // remaining channels are larger than remaining overpaid_value_msat.
@@ -1138,7 +1149,16 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        drawn_routes.sort_by_key(|paths| paths.iter().map(|path| path.get_total_fee_paid_msat()).sum::<u64>());
        let mut selected_paths = Vec::<Vec<RouteHop>>::new();
        for payment_path in drawn_routes.first().unwrap() {
-               selected_paths.push(payment_path.hops.iter().map(|payment_hop| payment_hop.route_hop.clone()).collect());
+               selected_paths.push(payment_path.hops.iter().map(|(payment_hop, node_features)| {
+                       RouteHop {
+                               pubkey: payment_hop.pubkey,
+                               node_features: node_features.clone(),
+                               short_channel_id: payment_hop.short_channel_id,
+                               channel_features: payment_hop.channel_features.clone(),
+                               fee_msat: payment_hop.fee_msat,
+                               cltv_expiry_delta: payment_hop.cltv_expiry_delta,
+                       }
+               }).collect());
        }
 
        if let Some(features) = &payee_features {
@@ -1148,14 +1168,15 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        }
 
        let route = Route { paths: selected_paths };
-       log_trace!(logger, "Got route: {}", log_route!(route));
+       log_info!(logger, "Got route to {}: {}", payee, log_route!(route));
        Ok(route)
 }
 
 #[cfg(test)]
 mod tests {
-       use routing::router::{get_route, RouteHint, RoutingFees};
+       use routing::router::{get_route, Route, RouteHint, RouteHintHop, RoutingFees};
        use routing::network_graph::{NetworkGraph, NetGraphMsgHandler};
+       use chain::transaction::OutPoint;
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
        use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
           NodeAnnouncement, UnsignedNodeAnnouncement, ChannelUpdate, UnsignedChannelUpdate};
@@ -1176,8 +1197,33 @@ mod tests {
        use bitcoin::secp256k1::key::{PublicKey,SecretKey};
        use bitcoin::secp256k1::{Secp256k1, All};
 
+       use prelude::*;
        use std::sync::Arc;
 
+       fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
+                       features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
+               channelmanager::ChannelDetails {
+                       channel_id: [0; 32],
+                       counterparty: channelmanager::ChannelCounterparty {
+                               features,
+                               node_id,
+                               unspendable_punishment_reserve: 0,
+                               forwarding_info: None,
+                       },
+                       funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
+                       short_channel_id,
+                       channel_value_satoshis: 0,
+                       user_id: 0,
+                       outbound_capacity_msat,
+                       inbound_capacity_msat: 42,
+                       unspendable_punishment_reserve: None,
+                       confirmations_required: None,
+                       force_close_spend_delay: None,
+                       is_outbound: true, is_funding_locked: true,
+                       is_usable: true, is_public: true,
+               }
+       }
+
        // Using the same keys for LN and BTC ids
        fn add_channel(net_graph_msg_handler: &NetGraphMsgHandler<Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>, secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey,
           node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64) {
@@ -1614,18 +1660,7 @@ mod tests {
 
                // Simple route to 2 via 1
 
-               let our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(2),
-                       remote_network_id: our_id,
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 100000,
-                       user_id: 0,
-                       outbound_capacity_msat: 100000,
-                       inbound_capacity_msat: 100000,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
+               let our_chans = vec![get_channel_details(Some(2), our_id, InitFeatures::from_le_bytes(vec![0b11]), 100000)];
 
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)) {
                        assert_eq!(err, "First hop cannot have our_node_id as a destination.");
@@ -1932,18 +1967,7 @@ mod tests {
                } else { panic!(); }
 
                // If we specify a channel to node7, that overrides our local channel view and that gets used
-               let our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(42),
-                       remote_network_id: nodes[7].clone(),
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 0,
-                       user_id: 0,
-                       outbound_capacity_msat: 250_000_000,
-                       inbound_capacity_msat: 0,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
+               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, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()),  &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
 
@@ -1980,18 +2004,7 @@ mod tests {
                } else { panic!(); }
 
                // If we specify a channel to node7, that overrides our local channel view and that gets used
-               let our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(42),
-                       remote_network_id: nodes[7].clone(),
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 0,
-                       user_id: 0,
-                       outbound_capacity_msat: 250_000_000,
-                       inbound_capacity_msat: 0,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
+               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, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
 
@@ -2045,18 +2058,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 our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(42),
-                       remote_network_id: nodes[7].clone(),
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 0,
-                       user_id: 0,
-                       outbound_capacity_msat: 250_000_000,
-                       inbound_capacity_msat: 0,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
+               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, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
 
@@ -2080,14 +2082,14 @@ mod tests {
                        base_msat: 0,
                        proportional_millionths: 0,
                };
-               vec!(RouteHint {
+               vec![RouteHint(vec![RouteHintHop {
                        src_node_id: nodes[3].clone(),
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
-               }, RouteHint {
+               }]), RouteHint(vec![RouteHintHop {
                        src_node_id: nodes[4].clone(),
                        short_channel_id: 9,
                        fees: RoutingFees {
@@ -2097,14 +2099,14 @@ mod tests {
                        cltv_expiry_delta: (9 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
-               }, RouteHint {
+               }]), RouteHint(vec![RouteHintHop {
                        src_node_id: nodes[5].clone(),
                        short_channel_id: 10,
                        fees: zero_fees,
                        cltv_expiry_delta: (10 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
-               })
+               }])]
        }
 
        #[test]
@@ -2114,8 +2116,8 @@ mod tests {
 
                // Simple test across 2, 3, 5, and 4 via a last_hop channel
 
-               // First check that lst hop can't have its source as the payee.
-               let invalid_last_hop = RouteHint {
+               // First check that last hop can't have its source as the payee.
+               let invalid_last_hop = RouteHint(vec![RouteHintHop {
                        src_node_id: nodes[6],
                        short_channel_id: 8,
                        fees: RoutingFees {
@@ -2125,7 +2127,7 @@ mod tests {
                        cltv_expiry_delta: (8 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
-               };
+               }]);
 
                let mut invalid_last_hops = last_hops(&nodes);
                invalid_last_hops.push(invalid_last_hop);
@@ -2182,18 +2184,7 @@ mod tests {
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 
                // Simple test with outbound channel to 4 to test that last_hops and first_hops connect
-               let our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(42),
-                       remote_network_id: nodes[3].clone(),
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 0,
-                       user_id: 0,
-                       outbound_capacity_msat: 250_000_000,
-                       inbound_capacity_msat: 0,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
+               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 route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, Some(&our_chans.iter().collect::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
                assert_eq!(route.paths[0].len(), 2);
@@ -2212,7 +2203,7 @@ mod tests {
                assert_eq!(route.paths[0][1].node_features.le_flags(), &Vec::<u8>::new()); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][1].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
 
-               last_hops[0].fees.base_msat = 1000;
+               last_hops[0].0[0].fees.base_msat = 1000;
 
                // Revert to via 6 as the fee on 8 goes up
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[6], None, None, &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
@@ -2290,58 +2281,71 @@ mod tests {
                assert_eq!(route.paths[0][4].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
        }
 
-       #[test]
-       fn unannounced_path_test() {
-               // We should be able to send a payment to a destination without any help of a routing graph
-               // if we have a channel with a common counterparty that appears in the first and last hop
-               // hints.
+       fn do_unannounced_path_test(last_hop_htlc_max: Option<u64>, last_hop_fee_prop: u32, outbound_capacity_msat: u64, route_val: u64) -> Result<Route, LightningError> {
                let source_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 41).repeat(32)).unwrap()[..]).unwrap());
                let middle_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 42).repeat(32)).unwrap()[..]).unwrap());
                let target_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 43).repeat(32)).unwrap()[..]).unwrap());
 
                // If we specify a channel to a middle hop, that overrides our local channel view and that gets used
-               let last_hops = vec![RouteHint {
+               let last_hops = RouteHint(vec![RouteHintHop {
                        src_node_id: middle_node_id,
                        short_channel_id: 8,
                        fees: RoutingFees {
                                base_msat: 1000,
-                               proportional_millionths: 0,
+                               proportional_millionths: last_hop_fee_prop,
                        },
                        cltv_expiry_delta: (8 << 8) | 1,
                        htlc_minimum_msat: None,
-                       htlc_maximum_msat: None,
-               }];
-               let our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(42),
-                       remote_network_id: middle_node_id,
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 100000,
-                       user_id: 0,
-                       outbound_capacity_msat: 100000,
-                       inbound_capacity_msat: 100000,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
-               let route = get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, None, Some(&our_chans.iter().collect::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::new(test_utils::TestLogger::new())).unwrap();
+                       htlc_maximum_msat: last_hop_htlc_max,
+               }]);
+               let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)];
+               get_route(&source_node_id, &NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash()), &target_node_id, None, Some(&our_chans.iter().collect::<Vec<_>>()), &vec![&last_hops], route_val, 42, Arc::new(test_utils::TestLogger::new()))
+       }
 
+       #[test]
+       fn unannounced_path_test() {
+               // We should be able to send a payment to a destination without any help of a routing graph
+               // if we have a channel with a common counterparty that appears in the first and last hop
+               // hints.
+               let route = do_unannounced_path_test(None, 1, 2000000, 1000000).unwrap();
+
+               let middle_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 42).repeat(32)).unwrap()[..]).unwrap());
+               let target_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 43).repeat(32)).unwrap()[..]).unwrap());
                assert_eq!(route.paths[0].len(), 2);
 
                assert_eq!(route.paths[0][0].pubkey, middle_node_id);
                assert_eq!(route.paths[0][0].short_channel_id, 42);
-               assert_eq!(route.paths[0][0].fee_msat, 1000);
+               assert_eq!(route.paths[0][0].fee_msat, 1001);
                assert_eq!(route.paths[0][0].cltv_expiry_delta, (8 << 8) | 1);
                assert_eq!(route.paths[0][0].node_features.le_flags(), &[0b11]);
                assert_eq!(route.paths[0][0].channel_features.le_flags(), &[0; 0]); // We can't learn any flags from invoices, sadly
 
                assert_eq!(route.paths[0][1].pubkey, target_node_id);
                assert_eq!(route.paths[0][1].short_channel_id, 8);
-               assert_eq!(route.paths[0][1].fee_msat, 100);
+               assert_eq!(route.paths[0][1].fee_msat, 1000000);
                assert_eq!(route.paths[0][1].cltv_expiry_delta, 42);
                assert_eq!(route.paths[0][1].node_features.le_flags(), &[0; 0]); // We dont pass flags in from invoices yet
                assert_eq!(route.paths[0][1].channel_features.le_flags(), &[0; 0]); // We can't learn any flags from invoices, sadly
        }
 
+       #[test]
+       fn overflow_unannounced_path_test_liquidity_underflow() {
+               // Previously, when we had a last-hop hint connected directly to a first-hop channel, where
+               // the last-hop had a fee which overflowed a u64, we'd panic.
+               // This was due to us adding the first-hop from us unconditionally, causing us to think
+               // we'd built a path (as our node is in the "best candidate" set), when we had not.
+               // In this test, we previously hit a subtraction underflow due to having less available
+               // liquidity at the last hop than 0.
+               assert!(do_unannounced_path_test(Some(21_000_000_0000_0000_000), 0, 21_000_000_0000_0000_000, 21_000_000_0000_0000_000).is_err());
+       }
+
+       #[test]
+       fn overflow_unannounced_path_test_feerate_overflow() {
+               // This tests for the same case as above, except instead of hitting a subtraction
+               // underflow, we hit a case where the fee charged at a hop overflowed.
+               assert!(do_unannounced_path_test(Some(21_000_000_0000_0000_000), 50000, 21_000_000_0000_0000_000, 21_000_000_0000_0000_000).is_err());
+       }
+
        #[test]
        fn available_amount_while_routing_test() {
                // Tests whether we choose the correct available channel amount while routing.
@@ -2443,18 +2447,7 @@ mod tests {
                });
 
                // Now, limit the first_hop by the outbound_capacity_msat of 200_000 sats.
-               let our_chans = vec![channelmanager::ChannelDetails {
-                       channel_id: [0; 32],
-                       short_channel_id: Some(42),
-                       remote_network_id: nodes[0].clone(),
-                       counterparty_features: InitFeatures::from_le_bytes(vec![0b11]),
-                       channel_value_satoshis: 0,
-                       user_id: 0,
-                       outbound_capacity_msat: 200_000_000,
-                       inbound_capacity_msat: 0,
-                       is_live: true,
-                       counterparty_forwarding_info: None,
-               }];
+               let our_chans = vec![get_channel_details(Some(42), nodes[0].clone(), InitFeatures::from_le_bytes(vec![0b11]), 200_000_000)];
 
                {
                        // Attempt to route more than available results in a failure.
@@ -3822,43 +3815,24 @@ mod tests {
                }
        }
 
-       use std::fs::File;
-       use util::ser::Readable;
-       /// Tries to open a network graph file, or panics with a URL to fetch it.
-       pub(super) fn get_route_file() -> Result<std::fs::File, std::io::Error> {
-               let res = File::open("net_graph-2021-02-12.bin") // By default we're run in RL/lightning
-                       .or_else(|_| File::open("lightning/net_graph-2021-02-12.bin")) // We may be run manually in RL/
-                       .or_else(|_| { // Fall back to guessing based on the binary location
-                               // path is likely something like .../rust-lightning/target/debug/deps/lightning-...
-                               let mut path = std::env::current_exe().unwrap();
-                               path.pop(); // lightning-...
-                               path.pop(); // deps
-                               path.pop(); // debug
-                               path.pop(); // target
-                               path.push("lightning");
-                               path.push("net_graph-2021-02-12.bin");
-                               eprintln!("{}", path.to_str().unwrap());
-                               File::open(path)
-                       });
-               #[cfg(require_route_graph_test)]
-               return Ok(res.expect("Didn't have route graph and was configured to require it"));
-               res
-       }
-
+       #[cfg(not(feature = "no_std"))]
        pub(super) fn random_init_seed() -> u64 {
                // Because the default HashMap in std pulls OS randomness, we can use it as a (bad) RNG.
-               use std::hash::{BuildHasher, Hasher};
+               use core::hash::{BuildHasher, Hasher};
                let seed = std::collections::hash_map::RandomState::new().build_hasher().finish();
                println!("Using seed of {}", seed);
                seed
        }
+       #[cfg(not(feature = "no_std"))]
+       use util::ser::Readable;
 
        #[test]
+       #[cfg(not(feature = "no_std"))]
        fn generate_routes() {
-               let mut d = match get_route_file() {
+               let mut d = match super::test_utils::get_route_file() {
                        Ok(f) => f,
-                       Err(_) => {
-                               eprintln!("Please fetch https://bitcoin.ninja/ldk-net_graph-879e309c128-2020-02-12.bin and place it at lightning/net_graph-2021-02-12.bin");
+                       Err(e) => {
+                               eprintln!("{}", e);
                                return;
                        },
                };
@@ -3881,11 +3855,12 @@ mod tests {
        }
 
        #[test]
+       #[cfg(not(feature = "no_std"))]
        fn generate_routes_mpp() {
-               let mut d = match get_route_file() {
+               let mut d = match super::test_utils::get_route_file() {
                        Ok(f) => f,
-                       Err(_) => {
-                               eprintln!("Please fetch https://bitcoin.ninja/ldk-net_graph-879e309c128-2020-02-12.bin and place it at lightning/net_graph-2021-02-12.bin");
+                       Err(e) => {
+                               eprintln!("{}", e);
                                return;
                        },
                };
@@ -3908,12 +3883,38 @@ mod tests {
        }
 }
 
-#[cfg(all(test, feature = "unstable"))]
+#[cfg(all(test, not(feature = "no_std")))]
+pub(crate) mod test_utils {
+       use std::fs::File;
+       /// Tries to open a network graph file, or panics with a URL to fetch it.
+       pub(crate) fn get_route_file() -> Result<std::fs::File, &'static str> {
+               let res = File::open("net_graph-2021-05-31.bin") // By default we're run in RL/lightning
+                       .or_else(|_| File::open("lightning/net_graph-2021-05-31.bin")) // We may be run manually in RL/
+                       .or_else(|_| { // Fall back to guessing based on the binary location
+                               // path is likely something like .../rust-lightning/target/debug/deps/lightning-...
+                               let mut path = std::env::current_exe().unwrap();
+                               path.pop(); // lightning-...
+                               path.pop(); // deps
+                               path.pop(); // debug
+                               path.pop(); // target
+                               path.push("lightning");
+                               path.push("net_graph-2021-05-31.bin");
+                               eprintln!("{}", path.to_str().unwrap());
+                               File::open(path)
+                       })
+               .map_err(|_| "Please fetch https://bitcoin.ninja/ldk-net_graph-v0.0.15-2021-05-31.bin and place it at lightning/net_graph-2021-05-31.bin");
+               #[cfg(require_route_graph_test)]
+               return Ok(res.unwrap());
+               #[cfg(not(require_route_graph_test))]
+               return res;
+       }
+}
+
+#[cfg(all(test, feature = "unstable", not(feature = "no_std")))]
 mod benches {
        use super::*;
        use util::logger::{Logger, Record};
 
-       use std::fs::File;
        use test::Bencher;
 
        struct DummyLogger {}
@@ -3923,8 +3924,7 @@ mod benches {
 
        #[bench]
        fn generate_routes(bench: &mut Bencher) {
-               let mut d = tests::get_route_file()
-                       .expect("Please fetch https://bitcoin.ninja/ldk-net_graph-879e309c128-2020-02-12.bin and place it at lightning/net_graph-2021-02-12.bin");
+               let mut d = test_utils::get_route_file().unwrap();
                let graph = NetworkGraph::read(&mut d).unwrap();
 
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
@@ -3955,8 +3955,7 @@ mod benches {
 
        #[bench]
        fn generate_mpp_routes(bench: &mut Bencher) {
-               let mut d = tests::get_route_file()
-                       .expect("Please fetch https://bitcoin.ninja/ldk-net_graph-879e309c128-2020-02-12.bin and place it at lightning/net_graph-2021-02-12.bin");
+               let mut d = test_utils::get_route_file().unwrap();
                let graph = NetworkGraph::read(&mut d).unwrap();
 
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...