Improve ChannelDetails readability significantly.
[rust-lightning] / lightning / src / routing / router.rs
index 5e36a281fce18b4379634b35aed7572289cda16a..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,9 +168,9 @@ 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)]
+#[derive(Clone, Debug)]
 struct PathBuildingHop<'a> {
-       // The RouteHint fields which will eventually be used if this hop is used in a final Route.
+       // 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,
@@ -338,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
@@ -368,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});
                }
@@ -464,10 +459,10 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
        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.to_context(), chan.outbound_capacity_msat, chan.counterparty_features.to_context()));
+                       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});
@@ -511,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)
@@ -730,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"))]
                                                                {
@@ -760,7 +761,8 @@ 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();
@@ -863,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, 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.
@@ -892,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>, &empty_channel_features, 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;
@@ -959,6 +962,9 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
                                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};
 
                                // We could have possibly constructed a slightly inconsistent path: since we reduce
@@ -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].0.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;
                }
        }
@@ -1157,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};
@@ -1185,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) {
@@ -1623,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.");
@@ -1941,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);
 
@@ -1989,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);
 
@@ -2054,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);
 
@@ -2089,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 {
@@ -2106,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]
@@ -2123,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 {
@@ -2134,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);
@@ -2191,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);
@@ -2221,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();
@@ -2299,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.
@@ -2452,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.
@@ -3831,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;
                        },
                };
@@ -3890,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;
                        },
                };
@@ -3917,7 +3883,34 @@ 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};
@@ -3931,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...
@@ -3963,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...