Merge pull request #1121 from TheBlueMatt/2021-10-return-temp-id
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sat, 16 Oct 2021 02:15:07 +0000 (02:15 +0000)
committerGitHub <noreply@github.com>
Sat, 16 Oct 2021 02:15:07 +0000 (02:15 +0000)
Expose temporary channel ID and user channel ID pre-funding

1  2 
fuzz/src/router.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/routing/router.rs

diff --combined fuzz/src/router.rs
index f08d66aeb06592d22735adec69ff4758383bc8b7,b80a8284eda7e2252d6e38cd44bf7cdef2d2fcbe..6a207254ac9992e172d0fb194d7f6b76745e8d80
@@@ -17,7 -17,6 +17,7 @@@ use lightning::ln::channelmanager::{Cha
  use lightning::ln::features::InitFeatures;
  use lightning::ln::msgs;
  use lightning::routing::router::{get_route, RouteHint, RouteHintHop};
 +use lightning::routing::scorer::Scorer;
  use lightning::util::logger::Logger;
  use lightning::util::ser::Readable;
  use lightning::routing::network_graph::{NetworkGraph, RoutingFees};
@@@ -217,7 -216,7 +217,7 @@@ pub fn do_test<Out: test_logger::Output
                                                                funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
                                                                short_channel_id: Some(scid),
                                                                channel_value_satoshis: slice_to_be64(get_slice!(8)),
-                                                               user_id: 0, inbound_capacity_msat: 0,
+                                                               user_channel_id: 0, inbound_capacity_msat: 0,
                                                                unspendable_punishment_reserve: None,
                                                                confirmations_required: None,
                                                                force_close_spend_delay: None,
                                                }]));
                                        }
                                }
 +                              let scorer = Scorer::new(0);
                                for target in node_pks.iter() {
                                        let _ = get_route(&our_pubkey, &net_graph, target, None,
                                                first_hops.map(|c| c.iter().collect::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
                                                &last_hops.iter().collect::<Vec<_>>(),
 -                                              slice_to_be64(get_slice!(8)), slice_to_be32(get_slice!(4)), Arc::clone(&logger));
 +                                              slice_to_be64(get_slice!(8)), slice_to_be32(get_slice!(4)), Arc::clone(&logger), &scorer);
                                }
                        },
                }
index fa0d12284a581fdff176ff1de38c52779a9356f4,0106fe51d6aaca06754fe4a8ebc22e5c72fc512f..5b29828ebaad09042dd65923512569ce47a31ef8
@@@ -36,9 -36,9 +36,9 @@@ use bitcoin::secp256k1::ecdh::SharedSec
  use bitcoin::secp256k1;
  
  use chain;
 -use chain::{Confirm, Watch, BestBlock};
 +use chain::{Confirm, ChannelMonitorUpdateErr, Watch, BestBlock};
  use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
 -use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
 +use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
  use chain::transaction::{OutPoint, TransactionData};
  // Since this struct is returned in `list_channels` methods, expose it here in case users want to
  // construct one themselves.
@@@ -242,7 -242,7 +242,7 @@@ type ShutdownResult = (Option<(OutPoint
  
  struct MsgHandleErrInternal {
        err: msgs::LightningError,
-       chan_id: Option<[u8; 32]>, // If Some a channel of ours has been closed
+       chan_id: Option<([u8; 32], u64)>, // If Some a channel of ours has been closed
        shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
  }
  impl MsgHandleErrInternal {
                Self { err, chan_id: None, shutdown_finish: None }
        }
        #[inline]
-       fn from_finish_shutdown(err: String, channel_id: [u8; 32], shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
+       fn from_finish_shutdown(err: String, channel_id: [u8; 32], user_channel_id: u64, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
                Self {
                        err: LightningError {
                                err: err.clone(),
                                        },
                                },
                        },
-                       chan_id: Some(channel_id),
+                       chan_id: Some((channel_id, user_channel_id)),
                        shutdown_finish: Some((shutdown_res, channel_update)),
                }
        }
@@@ -776,8 -776,8 +776,8 @@@ pub struct ChannelDetails 
        ///
        /// [`outbound_capacity_msat`]: ChannelDetails::outbound_capacity_msat
        pub unspendable_punishment_reserve: Option<u64>,
-       /// The user_id passed in to create_channel, or 0 if the channel was inbound.
-       pub user_id: u64,
+       /// The `user_channel_id` passed in to create_channel, or 0 if the channel was inbound.
+       pub user_channel_id: u64,
        /// The available outbound capacity for sending HTLCs to the remote peer. This does not include
        /// any pending HTLCs which are not yet fully resolved (and, thus, who's balance is not
        /// available for inclusion in new outbound HTLCs). This further does not include any pending
@@@ -894,8 -894,11 +894,11 @@@ macro_rules! handle_error 
                                                        msg: update
                                                });
                                        }
-                                       if let Some(channel_id) = chan_id {
-                                               $self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id,  reason: ClosureReason::ProcessingError { err: err.err.clone() } });
+                                       if let Some((channel_id, user_channel_id)) = chan_id {
+                                               $self.pending_events.lock().unwrap().push(events::Event::ChannelClosed {
+                                                       channel_id, user_channel_id,
+                                                       reason: ClosureReason::ProcessingError { err: err.err.clone() }
+                                               });
                                        }
                                }
  
@@@ -937,7 -940,8 +940,8 @@@ macro_rules! convert_chan_err 
                                        $short_to_id.remove(&short_id);
                                }
                                let shutdown_res = $channel.force_shutdown(true);
-                               (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
+                               (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
+                                       shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
                        },
                        ChannelError::CloseDelayBroadcast(msg) => {
                                log_error!($self.logger, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($channel_id[..]), msg);
                                        $short_to_id.remove(&short_id);
                                }
                                let shutdown_res = $channel.force_shutdown(false);
-                               (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
+                               (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
+                                       shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
                        }
                }
        }
@@@ -1013,7 -1018,7 +1018,7 @@@ macro_rules! handle_monitor_err 
                                // splitting hairs we'd prefer to claim payments that were to us, but we haven't
                                // given up the preimage yet, so might as well just wait until the payment is
                                // retried, avoiding the on-chain fees.
-                               let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id,
+                               let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id, $chan.get_user_id(),
                                                $chan.force_shutdown(true), $self.get_channel_update_for_broadcast(&$chan).ok() ));
                                (res, true)
                        },
@@@ -1267,22 -1272,31 +1272,31 @@@ impl<Signer: Sign, M: Deref, T: Deref, 
  
        /// Creates a new outbound channel to the given remote node and with the given value.
        ///
-       /// user_id will be provided back as user_channel_id in FundingGenerationReady events to allow
-       /// tracking of which events correspond with which create_channel call. Note that the
-       /// user_channel_id defaults to 0 for inbound channels, so you may wish to avoid using 0 for
-       /// user_id here. user_id has no meaning inside of LDK, it is simply copied to events and
-       /// otherwise ignored.
-       ///
-       /// If successful, will generate a SendOpenChannel message event, so you should probably poll
-       /// PeerManager::process_events afterwards.
+       /// `user_channel_id` will be provided back as in
+       /// [`Event::FundingGenerationReady::user_channel_id`] to allow tracking of which events
+       /// correspond with which `create_channel` call. Note that the `user_channel_id` defaults to 0
+       /// for inbound channels, so you may wish to avoid using 0 for `user_channel_id` here.
+       /// `user_channel_id` has no meaning inside of LDK, it is simply copied to events and otherwise
+       /// ignored.
        ///
-       /// Raises APIError::APIMisuseError when channel_value_satoshis > 2**24 or push_msat is
-       /// greater than channel_value_satoshis * 1k or channel_value_satoshis is < 1000.
+       /// Raises [`APIError::APIMisuseError`] when `channel_value_satoshis` > 2**24 or `push_msat` is
+       /// greater than `channel_value_satoshis * 1k` or `channel_value_satoshis < 1000`.
        ///
        /// Note that we do not check if you are currently connected to the given peer. If no
        /// connection is available, the outbound `open_channel` message may fail to send, resulting in
-       /// the channel eventually being silently forgotten.
-       pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, override_config: Option<UserConfig>) -> Result<(), APIError> {
+       /// the channel eventually being silently forgotten (dropped on reload).
+       ///
+       /// Returns the new Channel's temporary `channel_id`. This ID will appear as
+       /// [`Event::FundingGenerationReady::temporary_channel_id`] and in
+       /// [`ChannelDetails::channel_id`] until after
+       /// [`ChannelManager::funding_transaction_generated`] is called, swapping the Channel's ID for
+       /// one derived from the funding transaction's TXID. If the counterparty rejects the channel
+       /// immediately, this temporary ID will appear in [`Event::ChannelClosed::channel_id`].
+       ///
+       /// [`Event::FundingGenerationReady::user_channel_id`]: events::Event::FundingGenerationReady::user_channel_id
+       /// [`Event::FundingGenerationReady::temporary_channel_id`]: events::Event::FundingGenerationReady::temporary_channel_id
+       /// [`Event::ChannelClosed::channel_id`]: events::Event::ChannelClosed::channel_id
+       pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_channel_id: u64, override_config: Option<UserConfig>) -> Result<[u8; 32], APIError> {
                if channel_value_satoshis < 1000 {
                        return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
                }
                                        let peer_state = peer_state.lock().unwrap();
                                        let their_features = &peer_state.latest_features;
                                        let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
-                                       Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_id, config)?
+                                       Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_channel_id, config)?
                                },
                                None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
                        }
                // We want to make sure the lock is actually acquired by PersistenceNotifierGuard.
                debug_assert!(&self.total_consistency_lock.try_write().is_err());
  
+               let temporary_channel_id = channel.channel_id();
                let mut channel_state = self.channel_state.lock().unwrap();
-               match channel_state.by_id.entry(channel.channel_id()) {
+               match channel_state.by_id.entry(temporary_channel_id) {
                        hash_map::Entry::Occupied(_) => {
                                if cfg!(feature = "fuzztarget") {
                                        return Err(APIError::APIMisuseError { err: "Fuzzy bad RNG".to_owned() });
                        node_id: their_network_key,
                        msg: res,
                });
-               Ok(())
+               Ok(temporary_channel_id)
        }
  
        fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<Signer>)) -> bool>(&self, f: Fn) -> Vec<ChannelDetails> {
                                        unspendable_punishment_reserve: to_self_reserve_satoshis,
                                        inbound_capacity_msat,
                                        outbound_capacity_msat,
-                                       user_id: channel.get_user_id(),
+                                       user_channel_id: channel.get_user_id(),
                                        confirmations_required: channel.minimum_depth(),
                                        force_close_spend_delay: channel.get_counterparty_selected_contest_delay(),
                                        is_outbound: channel.is_outbound(),
                        },
                        None => {},
                }
-               pending_events_lock.push(events::Event::ChannelClosed { channel_id: channel.channel_id(), reason: closure_reason });
+               pending_events_lock.push(events::Event::ChannelClosed {
+                       channel_id: channel.channel_id(),
+                       user_channel_id: channel.get_user_id(),
+                       reason: closure_reason
+               });
        }
  
        fn close_channel_internal(&self, channel_id: &[u8; 32], target_feerate_sats_per_1000_weight: Option<u32>) -> Result<(), APIError> {
  
                                        (chan.get_outbound_funding_created(funding_transaction, funding_txo, &self.logger)
                                                .map_err(|e| if let ChannelError::Close(msg) = e {
-                                                       MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.force_shutdown(true), None)
+                                                       MsgHandleErrInternal::from_finish_shutdown(msg, chan.channel_id(), chan.get_user_id(), chan.force_shutdown(true), None)
                                                } else { unreachable!(); })
                                        , chan)
                                },
                                                                                                channel_state.short_to_id.remove(&short_id);
                                                                                        }
                                                                                        // ChannelClosed event is generated by handle_error for us.
-                                                                                       Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
+                                                                                       Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
                                                                                },
                                                                                ChannelError::CloseDelayBroadcast(_) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); }
                                                                        };
@@@ -5609,6 -5628,7 +5628,7 @@@ impl<'a, Signer: Sign, M: Deref, T: Der
                                        monitor.broadcast_latest_holder_commitment_txn(&args.tx_broadcaster, &args.logger);
                                        channel_closures.push(events::Event::ChannelClosed {
                                                channel_id: channel.channel_id(),
+                                               user_channel_id: channel.get_user_id(),
                                                reason: ClosureReason::OutdatedChannelManager
                                        });
                                } else {
@@@ -5804,7 -5824,6 +5824,7 @@@ mod tests 
        use ln::msgs;
        use ln::msgs::ChannelMessageHandler;
        use routing::router::{get_keysend_route, get_route};
 +      use routing::scorer::Scorer;
        use util::errors::APIError;
        use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
        use util::test_utils;
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
 -              let logger = test_utils::TestLogger::new();
  
                // First, send a partial MPP payment.
 -              let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
 -              let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
 -              let (payment_preimage, our_payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[1]);
 +              let (route, our_payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000);
                let payment_id = PaymentId([42; 32]);
                // Use the utility function send_payment_along_path to send the payment with MPP data which
                // indicates there are more HTLCs coming.
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
                let logger = test_utils::TestLogger::new();
 +              let scorer = Scorer::new(0);
  
                // To start (1), send a regular payment but don't claim it.
                let expected_route = [&nodes[1]];
                let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &expected_route, 100_000);
  
                // Next, attempt a keysend payment and make sure it fails.
 -              let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
 +              let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger, &scorer).unwrap();
                nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
  
                // To start (2), send a keysend payment but don't claim it.
                let payment_preimage = PaymentPreimage([42; 32]);
 -              let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
 +              let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger, &scorer).unwrap();
                let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
                check_added_monitors!(nodes[0], 1);
                let mut events = nodes[0].node.get_and_clear_pending_msg_events();
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
                let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
                let first_hops = nodes[0].node.list_usable_channels();
 +              let scorer = Scorer::new(0);
                let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey,
                                    Some(&first_hops.iter().collect::<Vec<_>>()), &vec![], 10000, 40,
 -                                  nodes[0].logger).unwrap();
 +                                  nodes[0].logger, &scorer).unwrap();
  
                let test_preimage = PaymentPreimage([42; 32]);
                let mismatch_payment_hash = PaymentHash([43; 32]);
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
                let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
                let first_hops = nodes[0].node.list_usable_channels();
 +              let scorer = Scorer::new(0);
                let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey,
                                    Some(&first_hops.iter().collect::<Vec<_>>()), &vec![], 10000, 40,
 -                                  nodes[0].logger).unwrap();
 +                                  nodes[0].logger, &scorer).unwrap();
  
                let test_preimage = PaymentPreimage([42; 32]);
                let test_secret = PaymentSecret([43; 32]);
                let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 2, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
                let chan_3_id = create_announced_chan_between_nodes(&nodes, 1, 3, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
                let chan_4_id = create_announced_chan_between_nodes(&nodes, 2, 3, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
 -              let logger = test_utils::TestLogger::new();
  
                // Marshall an MPP route.
 -              let (_, payment_hash, _) = get_payment_preimage_hash!(&nodes[3]);
 -              let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
 -              let mut route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[3].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
 +              let (mut route, payment_hash, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[3], 100000);
                let path = route.paths[0].clone();
                route.paths.push(path);
                route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();
  #[cfg(all(any(test, feature = "_test_utils"), feature = "unstable"))]
  pub mod bench {
        use chain::Listen;
 -      use chain::chainmonitor::ChainMonitor;
 -      use chain::channelmonitor::Persist;
 +      use chain::chainmonitor::{ChainMonitor, Persist};
        use chain::keysinterface::{KeysManager, InMemorySigner};
        use ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, PaymentHash, PaymentPreimage};
        use ln::features::{InitFeatures, InvoiceFeatures};
        use ln::msgs::{ChannelMessageHandler, Init};
        use routing::network_graph::NetworkGraph;
        use routing::router::get_route;
 +      use routing::scorer::Scorer;
        use util::test_utils;
        use util::config::UserConfig;
        use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};
                macro_rules! send_payment {
                        ($node_a: expr, $node_b: expr) => {
                                let usable_channels = $node_a.list_usable_channels();
 +                              let scorer = Scorer::new(0);
                                let route = get_route(&$node_a.get_our_node_id(), &dummy_graph, &$node_b.get_our_node_id(), Some(InvoiceFeatures::known()),
 -                                      Some(&usable_channels.iter().map(|r| r).collect::<Vec<_>>()), &[], 10_000, TEST_FINAL_CLTV, &logger_a).unwrap();
 +                                      Some(&usable_channels.iter().map(|r| r).collect::<Vec<_>>()), &[], 10_000, TEST_FINAL_CLTV, &logger_a, &scorer).unwrap();
  
                                let mut payment_preimage = PaymentPreimage([0; 32]);
                                payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
index f2ab55030e16b853535da7260ecea3bf3e56736f,19969d0c260a00796fd13ade93465b3ace5b107c..0f188ac38eea4d4eadc2349ceb29af16c6db01d3
@@@ -15,9 -15,8 +15,9 @@@ use chain::channelmonitor::ChannelMonit
  use chain::transaction::OutPoint;
  use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
  use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
 -use routing::router::{Route, get_route};
  use routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
 +use routing::router::{Route, get_route};
 +use routing::scorer::Scorer;
  use ln::features::{InitFeatures, InvoiceFeatures};
  use ln::msgs;
  use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler};
@@@ -275,9 -274,10 +275,9 @@@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 
                        let feeest = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
                        let mut deserialized_monitors = Vec::new();
                        {
 -                              let old_monitors = self.chain_monitor.chain_monitor.monitors.read().unwrap();
 -                              for (_, old_monitor) in old_monitors.iter() {
 +                              for outpoint in self.chain_monitor.chain_monitor.list_monitors() {
                                        let mut w = test_utils::TestVecWriter(Vec::new());
 -                                      old_monitor.write(&mut w).unwrap();
 +                                      self.chain_monitor.chain_monitor.get_monitor(outpoint).unwrap().write(&mut w).unwrap();
                                        let (_, deserialized_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
                                                &mut io::Cursor::new(&w.0), self.keys_manager).unwrap();
                                        deserialized_monitors.push(deserialized_monitor);
@@@ -437,35 -437,20 +437,35 @@@ macro_rules! get_feerate 
        }
  }
  
 -/// Returns any local commitment transactions for the channel.
 +/// Returns a channel monitor given a channel id, making some naive assumptions
  #[macro_export]
 -macro_rules! get_local_commitment_txn {
 +macro_rules! get_monitor {
        ($node: expr, $channel_id: expr) => {
                {
 -                      let monitors = $node.chain_monitor.chain_monitor.monitors.read().unwrap();
 -                      let mut commitment_txn = None;
 -                      for (funding_txo, monitor) in monitors.iter() {
 -                              if funding_txo.to_channel_id() == $channel_id {
 -                                      commitment_txn = Some(monitor.unsafe_get_latest_holder_commitment_txn(&$node.logger));
 +                      use bitcoin::hashes::Hash;
 +                      let mut monitor = None;
 +                      // Assume funding vout is either 0 or 1 blindly
 +                      for index in 0..2 {
 +                              if let Ok(mon) = $node.chain_monitor.chain_monitor.get_monitor(
 +                                      $crate::chain::transaction::OutPoint {
 +                                              txid: bitcoin::Txid::from_slice(&$channel_id[..]).unwrap(), index
 +                                      })
 +                              {
 +                                      monitor = Some(mon);
                                        break;
                                }
                        }
 -                      commitment_txn.unwrap()
 +                      monitor.unwrap()
 +              }
 +      }
 +}
 +
 +/// Returns any local commitment transactions for the channel.
 +#[macro_export]
 +macro_rules! get_local_commitment_txn {
 +      ($node: expr, $channel_id: expr) => {
 +              {
 +                      $crate::get_monitor!($node, $channel_id).unsafe_get_latest_holder_commitment_txn(&$node.logger)
                }
        }
  }
@@@ -527,11 -512,12 +527,12 @@@ pub fn create_funding_transaction<'a, '
  }
  
  pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> Transaction {
-       node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
+       let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
        node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), a_flags, &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id()));
        node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), b_flags, &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id()));
  
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, channel_value, 42);
+       assert_eq!(temporary_channel_id, create_chan_id);
  
        node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(node_a, 0);
@@@ -987,17 -973,12 +988,17 @@@ macro_rules! commitment_signed_dance 
  #[macro_export]
  macro_rules! get_payment_preimage_hash {
        ($dest_node: expr) => {
 +              {
 +                      get_payment_preimage_hash!($dest_node, None)
 +              }
 +      };
 +      ($dest_node: expr, $min_value_msat: expr) => {
                {
                        let mut payment_count = $dest_node.network_payment_count.borrow_mut();
                        let payment_preimage = PaymentPreimage([*payment_count; 32]);
                        *payment_count += 1;
                        let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
 -                      let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap();
 +                      let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, 0).unwrap();
                        (payment_preimage, payment_hash, payment_secret)
                }
        }
  #[cfg(test)]
  macro_rules! get_route_and_payment_hash {
        ($send_node: expr, $recv_node: expr, $recv_value: expr) => {{
 -              let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node);
 +              get_route_and_payment_hash!($send_node, $recv_node, vec![], $recv_value, TEST_FINAL_CLTV)
 +      }};
 +      ($send_node: expr, $recv_node: expr, $last_hops: expr, $recv_value: expr, $cltv: expr) => {{
 +              let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node, Some($recv_value));
                let net_graph_msg_handler = &$send_node.net_graph_msg_handler;
 -              let route = get_route(&$send_node.node.get_our_node_id(),
 -                      &net_graph_msg_handler.network_graph,
 -                      &$recv_node.node.get_our_node_id(), None,
 -                      Some(&$send_node.node.list_usable_channels().iter().map(|a| a).collect::<Vec<_>>()),
 -                      &Vec::new(), $recv_value, TEST_FINAL_CLTV, $send_node.logger).unwrap();
 +              let scorer = ::routing::scorer::Scorer::new(0);
 +              let route = ::routing::router::get_route(
 +                      &$send_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph,
 +                      &$recv_node.node.get_our_node_id(), Some(::ln::features::InvoiceFeatures::known()),
 +                      Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
 +                      &$last_hops, $recv_value, $cltv, $send_node.logger, &scorer
 +              ).unwrap();
                (route, payment_hash, payment_preimage, payment_secret)
        }}
  }
@@@ -1327,11 -1303,10 +1328,11 @@@ pub const TEST_FINAL_CLTV: u32 = 70
  
  pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
        let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
 +      let scorer = Scorer::new(0);
        let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph,
                &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()),
                Some(&origin_node.node.list_usable_channels().iter().collect::<Vec<_>>()), &[],
 -              recv_value, TEST_FINAL_CLTV, origin_node.logger).unwrap();
 +              recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
  
  pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
        let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
 -      let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), recv_value, TEST_FINAL_CLTV, origin_node.logger).unwrap();
 +      let scorer = Scorer::new(0);
 +      let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
index bfc4afd9adfbbb36d133a6d475bf571cc8b034c0,9e02e10f94381949a86a25becd8887111a65a322..4083114dcb9d8d87ced272d97bda61782f28d78e
@@@ -17,8 -17,7 +17,8 @@@ use bitcoin::secp256k1::key::PublicKey
  use ln::channelmanager::ChannelDetails;
  use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
  use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
 -use routing::network_graph::{NetworkGraph, RoutingFees, NodeId};
 +use routing;
 +use routing::network_graph::{NetworkGraph, NodeId, RoutingFees};
  use util::ser::{Writeable, Readable};
  use util::logger::{Level, Logger};
  
@@@ -164,19 -163,12 +164,19 @@@ struct RouteGraphNode 
        /// The effective htlc_minimum_msat at this hop. If a later hop on the path had a higher HTLC
        /// minimum, we use it, plus the fees required at each earlier hop to meet it.
        path_htlc_minimum_msat: u64,
 +      /// All penalties incurred from this hop on the way to the destination, as calculated using
 +      /// channel scoring.
 +      path_penalty_msat: u64,
  }
  
  impl cmp::Ord for RouteGraphNode {
        fn cmp(&self, other: &RouteGraphNode) -> cmp::Ordering {
 -              let other_score = cmp::max(other.lowest_fee_to_peer_through_node, other.path_htlc_minimum_msat);
 -              let self_score = cmp::max(self.lowest_fee_to_peer_through_node, self.path_htlc_minimum_msat);
 +              let other_score = cmp::max(other.lowest_fee_to_peer_through_node, other.path_htlc_minimum_msat)
 +                      .checked_add(other.path_penalty_msat)
 +                      .unwrap_or_else(|| u64::max_value());
 +              let self_score = cmp::max(self.lowest_fee_to_peer_through_node, self.path_htlc_minimum_msat)
 +                      .checked_add(self.path_penalty_msat)
 +                      .unwrap_or_else(|| u64::max_value());
                other_score.cmp(&self_score).then_with(|| other.node_id.cmp(&self.node_id))
        }
  }
@@@ -229,9 -221,6 +229,9 @@@ struct PathBuildingHop<'a> 
        /// A mirror of the same field in RouteGraphNode. Note that this is only used during the graph
        /// walk and may be invalid thereafter.
        path_htlc_minimum_msat: u64,
 +      /// All penalties incurred from this channel on the way to the destination, as calculated using
 +      /// channel scoring.
 +      path_penalty_msat: u64,
        /// If we've already processed a node as the best node, we shouldn't process it again. Normally
        /// we'd just ignore it if we did as all channels would have a higher new fee, but because we
        /// may decrease the amounts in use as we walk the graph, the actual calculated fee may
@@@ -363,17 -352,13 +363,17 @@@ fn compute_fees(amount_msat: u64, chann
  /// Gets a keysend route from us (payer) to the given target node (payee). This is needed because
  /// keysend payments do not have an invoice from which to pull the payee's supported features, which
  /// makes it tricky to otherwise supply the `payee_features` parameter of `get_route`.
 -pub fn get_keysend_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph, payee:
 -                       &PublicKey, first_hops: Option<&[&ChannelDetails]>, last_hops: &[&RouteHint],
 -                       final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route,
 -                       LightningError> where L::Target: Logger {
 +pub fn get_keysend_route<L: Deref, S: routing::Score>(
 +      our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey,
 +      first_hops: Option<&[&ChannelDetails]>, last_hops: &[&RouteHint], final_value_msat: u64,
 +      final_cltv: u32, logger: L, scorer: &S
 +) -> Result<Route, LightningError>
 +where L::Target: Logger {
        let invoice_features = InvoiceFeatures::for_keysend();
 -      get_route(our_node_pubkey, network, payee, Some(invoice_features), first_hops, last_hops,
 -            final_value_msat, final_cltv, logger)
 +      get_route(
 +              our_node_pubkey, network, payee, Some(invoice_features), first_hops, last_hops,
 +              final_value_msat, final_cltv, logger, scorer
 +      )
  }
  
  /// Gets a route from us (payer) to the given target node (payee).
  /// The fees on channels from us to next-hops are ignored (as they are assumed to all be
  /// equal), however the enabled/disabled bit on such channels as well as the
  /// htlc_minimum_msat/htlc_maximum_msat *are* checked as they may change based on the receiving node.
 -pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey, payee_features: Option<InvoiceFeatures>, first_hops: Option<&[&ChannelDetails]>,
 -      last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route, LightningError> where L::Target: Logger {
 +pub fn get_route<L: Deref, S: routing::Score>(
 +      our_node_pubkey: &PublicKey, network: &NetworkGraph, payee: &PublicKey,
 +      payee_features: Option<InvoiceFeatures>, first_hops: Option<&[&ChannelDetails]>,
 +      last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L, scorer: &S
 +) -> Result<Route, LightningError>
 +where L::Target: Logger {
        let payee_node_id = NodeId::from_pubkey(&payee);
        let our_node_id = NodeId::from_pubkey(&our_node_pubkey);
  
 -      // TODO: Obviously *only* using total fee cost sucks. We should consider weighting by
 -      // uptime/success in using a node in the past.
        if payee_node_id == our_node_id {
                return Err(LightningError{err: "Cannot generate a route to ourselves".to_owned(), action: ErrorAction::IgnoreError});
        }
                // 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, $next_hops_path_penalty_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
                                                // might violate htlc_minimum_msat on the hops which are next along the
                                                // payment path (upstream to the payee). To avoid that, we recompute path
                                                // path fees knowing the final path contribution after constructing it.
 -                                              let path_htlc_minimum_msat = match compute_fees($next_hops_path_htlc_minimum_msat, $directional_info.fees)
 -                                                              .map(|fee_msat| fee_msat.checked_add($next_hops_path_htlc_minimum_msat)) {
 -                                                      Some(Some(value_msat)) => cmp::max(value_msat, $directional_info.htlc_minimum_msat),
 -                                                      _ => u64::max_value()
 -                                              };
 +                                              let path_htlc_minimum_msat = compute_fees($next_hops_path_htlc_minimum_msat, $directional_info.fees)
 +                                                      .and_then(|fee_msat| fee_msat.checked_add($next_hops_path_htlc_minimum_msat))
 +                                                      .map(|fee_msat| cmp::max(fee_msat, $directional_info.htlc_minimum_msat))
 +                                                      .unwrap_or_else(|| u64::max_value());
                                                let hm_entry = dist.entry($src_node_id);
                                                let old_entry = hm_entry.or_insert_with(|| {
                                                        // If there was previously no known way to access
                                                                total_fee_msat: u64::max_value(),
                                                                htlc_minimum_msat: $directional_info.htlc_minimum_msat,
                                                                path_htlc_minimum_msat,
 +                                                              path_penalty_msat: u64::max_value(),
                                                                was_processed: false,
                                                                #[cfg(any(test, feature = "fuzztarget"))]
                                                                value_contribution_msat,
                                                                }
                                                        }
  
 +                                                      let path_penalty_msat = $next_hops_path_penalty_msat
 +                                                              .checked_add(scorer.channel_penalty_msat($chan_id.clone()))
 +                                                              .unwrap_or_else(|| u64::max_value());
                                                        let new_graph_node = RouteGraphNode {
                                                                node_id: $src_node_id,
                                                                lowest_fee_to_peer_through_node: total_fee_msat,
                                                                lowest_fee_to_node: $next_hops_fee_msat as u64 + hop_use_fee_msat,
                                                                value_contribution_msat: value_contribution_msat,
                                                                path_htlc_minimum_msat,
 +                                                              path_penalty_msat,
                                                        };
  
                                                        // Update the way of reaching $src_node_id with the given $chan_id (from $dest_node_id),
                                                        // but it may require additional tracking - we don't want to double-count
                                                        // the fees included in $next_hops_path_htlc_minimum_msat, but also
                                                        // can't use something that may decrease on future hops.
 -                                                      let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat);
 -                                                      let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat);
 +                                                      let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat)
 +                                                              .checked_add(old_entry.path_penalty_msat)
 +                                                              .unwrap_or_else(|| u64::max_value());
 +                                                      let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat)
 +                                                              .checked_add(path_penalty_msat)
 +                                                              .unwrap_or_else(|| u64::max_value());
  
                                                        if !old_entry.was_processed && new_cost < old_cost {
                                                                targets.push(new_graph_node);
                                                                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;
 +                                                              old_entry.path_penalty_msat = path_penalty_msat;
                                                                #[cfg(any(test, feature = "fuzztarget"))]
                                                                {
                                                                        old_entry.value_contribution_msat = value_contribution_msat;
                                                                        // with a lower htlc_maximum_msat instead of the one we'd
                                                                        // already decided to use.
                                                                        debug_assert!(path_htlc_minimum_msat < old_entry.path_htlc_minimum_msat);
 -                                                                      debug_assert!(value_contribution_msat < old_entry.value_contribution_msat);
 +                                                                      debug_assert!(
 +                                                                              value_contribution_msat + path_penalty_msat <
 +                                                                              old_entry.value_contribution_msat + old_entry.path_penalty_msat
 +                                                                      );
                                                                }
                                                        }
                                                }
        // meaning how much will be paid in fees after this node (to the best of our knowledge).
        // This data can later be helpful to optimize routing (pay lower fees).
        macro_rules! add_entries_to_cheapest_to_target_node {
 -              ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr ) => {
 +              ( $node: expr, $node_id: expr, $fee_to_target_msat: expr, $next_hops_value_contribution: expr, $next_hops_path_htlc_minimum_msat: expr, $next_hops_path_penalty_msat: expr ) => {
                        let skip_node = if let Some(elem) = dist.get_mut(&$node_id) {
                                let was_processed = elem.was_processed;
                                elem.was_processed = true;
                        if !skip_node {
                                if let Some(first_channels) = first_hop_targets.get(&$node_id) {
                                        for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
 -                                              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);
 +                                              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, $next_hops_path_penalty_msat);
                                        }
                                }
  
                                                                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, $next_hops_path_penalty_msat);
                                                                                }
                                                                        }
                                                                }
                                                                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, $next_hops_path_penalty_msat);
                                                                                }
                                                                        }
                                                                }
                // place where it could be added.
                if let Some(first_channels) = first_hop_targets.get(&payee_node_id) {
                        for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
 -                              let added = add_entry!(first_hop, our_node_id, payee_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0);
 +                              let added = add_entry!(first_hop, our_node_id, payee_node_id, dummy_directional_info, Some(outbound_capacity_msat / 1000), features, 0, path_value_msat, 0, 0u64);
                                log_trace!(logger, "{} direct route to payee via SCID {}", if added { "Added" } else { "Skipped" }, first_hop);
                        }
                }
                        // If not, targets.pop() will not even let us enter the loop in step 2.
                        None => {},
                        Some(node) => {
 -                              add_entries_to_cheapest_to_target_node!(node, payee_node_id, 0, path_value_msat, 0);
 +                              add_entries_to_cheapest_to_target_node!(node, payee_node_id, 0, path_value_msat, 0, 0u64);
                        },
                }
  
                                let mut hop_used = true;
                                let mut aggregate_next_hops_fee_msat: u64 = 0;
                                let mut aggregate_next_hops_path_htlc_minimum_msat: u64 = 0;
 +                              let mut aggregate_next_hops_path_penalty_msat: u64 = 0;
  
                                for (idx, (hop, prev_hop_id)) in hop_iter.zip(prev_hop_iter).enumerate() {
                                        // BOLT 11 doesn't allow inclusion of features for the last hop hints, which
                                                _ => aggregate_next_hops_fee_msat.checked_add(999).unwrap_or(u64::max_value())
                                        }) { Some( val / 1000 ) } else { break; }; // converting from msat or breaking if max ~ infinity
  
 +                                      aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
 +                                              .checked_add(scorer.channel_penalty_msat(hop.short_channel_id))
 +                                              .unwrap_or_else(|| u64::max_value());
 +
                                        // We assume that the recipient only included route hints for routes which had
                                        // sufficient value to route `final_value_msat`. Note that in the case of "0-value"
                                        // invoices where the invoice does not specify value this may not be the case, but
                                        // better to include the hints than not.
 -                                      if !add_entry!(hop.short_channel_id, NodeId::from_pubkey(&hop.src_node_id), NodeId::from_pubkey(&prev_hop_id), directional_info, reqd_channel_cap, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat) {
 +                                      if !add_entry!(hop.short_channel_id, NodeId::from_pubkey(&hop.src_node_id), NodeId::from_pubkey(&prev_hop_id), directional_info, reqd_channel_cap, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat) {
                                                // If this hop was not used then there is no use checking the preceding hops
                                                // in the RouteHint. We can break by just searching for a direct channel between
                                                // last checked hop and first_hop_targets
                                        // Searching for a direct channel between last checked hop and first_hop_targets
                                        if let Some(first_channels) = first_hop_targets.get(&NodeId::from_pubkey(&prev_hop_id)) {
                                                for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
 -                                                      add_entry!(first_hop, our_node_id , NodeId::from_pubkey(&prev_hop_id), dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
 +                                                      add_entry!(first_hop, our_node_id , NodeId::from_pubkey(&prev_hop_id), dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat);
                                                }
                                        }
  
                                                // path.
                                                if let Some(first_channels) = first_hop_targets.get(&NodeId::from_pubkey(&hop.src_node_id)) {
                                                        for (ref first_hop, ref features, ref outbound_capacity_msat, _) in first_channels {
 -                                                              add_entry!(first_hop, our_node_id , NodeId::from_pubkey(&hop.src_node_id), dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat);
 +                                                              add_entry!(first_hop, our_node_id , NodeId::from_pubkey(&hop.src_node_id), dummy_directional_info, Some(outbound_capacity_msat / 1000), features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat);
                                                        }
                                                }
                                        }
                // Both these cases (and other cases except reaching recommended_value_msat) mean that
                // paths_collection will be stopped because found_new_path==false.
                // This is not necessarily a routing failure.
 -              'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, .. }) = targets.pop() {
 +              'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat, .. }) = targets.pop() {
  
                        // Since we're going payee-to-payer, hitting our node as a target means we should stop
                        // traversing the graph and arrange the path out of what we found.
                        match network_nodes.get(&node_id) {
                                None => {},
                                Some(node) => {
 -                                      add_entries_to_cheapest_to_target_node!(node, node_id, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat);
 +                                      add_entries_to_cheapest_to_target_node!(node, node_id, lowest_fee_to_node, value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat);
                                },
                        }
                }
  
  #[cfg(test)]
  mod tests {
 -      use routing::router::{get_route, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees};
        use routing::network_graph::{NetworkGraph, NetGraphMsgHandler};
 +      use routing::router::{get_route, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees};
 +      use routing::scorer::Scorer;
        use chain::transaction::OutPoint;
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
        use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
                        funding_txo: Some(OutPoint { txid: bitcoin::Txid::from_slice(&[0; 32]).unwrap(), index: 0 }),
                        short_channel_id,
                        channel_value_satoshis: 0,
-                       user_id: 0,
+                       user_channel_id: 0,
                        outbound_capacity_msat,
                        inbound_capacity_msat: 42,
                        unspendable_punishment_reserve: None,
        fn simple_route_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Simple route to 2 via 1
  
 -              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 0, 42, Arc::clone(&logger)) {
 +              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 0, 42, Arc::clone(&logger), &scorer) {
                        assert_eq!(err, "Cannot send a payment of 0 msat");
                } else { panic!(); }
  
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
        fn invalid_first_hop_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Simple route to 2 via 1
  
                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, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)) {
 +              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger), &scorer) {
                        assert_eq!(err, "First hop cannot have our_node_pubkey as a destination.");
                } else { panic!(); }
  
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
        }
  
        fn htlc_minimum_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Simple route to 2 via 1
  
                });
  
                // Not possible to send 199_999_999, because the minimum on channel=2 is 200_000_000.
 -              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 199_999_999, 42, Arc::clone(&logger)) {
 +              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 199_999_999, 42, Arc::clone(&logger), &scorer) {
                        assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!(); }
  
                });
  
                // A payment above the minimum should pass
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 199_999_999, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 199_999_999, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
        }
  
        fn htlc_minimum_overpay_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // A route to node#2 via two paths.
                // One path allows transferring 35-40 sats, another one also allows 35-40 sats.
                });
  
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                      Some(InvoiceFeatures::known()), None, &Vec::new(), 60_000, 42, Arc::clone(&logger)).unwrap();
 +                      Some(InvoiceFeatures::known()), None, &Vec::new(), 60_000, 42, Arc::clone(&logger), &scorer).unwrap();
                // Overpay fees to hit htlc_minimum_msat.
                let overpaid_fees = route.paths[0][0].fee_msat + route.paths[1][0].fee_msat;
                // TODO: this could be better balanced to overpay 10k and not 15k.
                });
  
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                      Some(InvoiceFeatures::known()), None, &Vec::new(), 60_000, 42, Arc::clone(&logger)).unwrap();
 +                      Some(InvoiceFeatures::known()), None, &Vec::new(), 60_000, 42, Arc::clone(&logger), &scorer).unwrap();
                // Fine to overpay for htlc_minimum_msat if it allows us to save fee.
                assert_eq!(route.paths.len(), 1);
                assert_eq!(route.paths[0][0].short_channel_id, 12);
                assert_eq!(fees, 5_000);
  
                let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                      Some(InvoiceFeatures::known()), None, &Vec::new(), 50_000, 42, Arc::clone(&logger)).unwrap();
 +                      Some(InvoiceFeatures::known()), None, &Vec::new(), 50_000, 42, Arc::clone(&logger), &scorer).unwrap();
                // Not fine to overpay for htlc_minimum_msat if it requires paying more than fee on
                // the other channel.
                assert_eq!(route.paths.len(), 1);
        fn disable_channels_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // // Disable channels 4 and 12 by flags=2
                update_channel(&net_graph_msg_handler, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
                });
  
                // If all the channels require some features we don't understand, route should fail
 -              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger)) {
 +              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger), &scorer) {
                        assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!(); }
  
                // If we specify a channel to node7, that overrides our local channel view and that gets used
                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, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()),  &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()),  &Vec::new(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[7]);
        fn disable_node_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Disable nodes 1, 2, and 8 by requiring unknown feature bits
                let unknown_features = NodeFeatures::known().set_unknown_feature_required();
                add_or_update_node(&net_graph_msg_handler, &secp_ctx, &privkeys[7], unknown_features.clone(), 1);
  
                // If all nodes require some features we don't understand, route should fail
 -              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger)) {
 +              if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 100, 42, Arc::clone(&logger), &scorer) {
                        assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!(); }
  
                // If we specify a channel to node7, that overrides our local channel view and that gets used
                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, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[7]);
        fn our_chans_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Route to 1 via 2 and 3 because our channel to 1 is disabled
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[0], None, None, &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[0], None, None, &Vec::new(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 3);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
  
                // If we specify a channel to node7, that overrides our local channel view and that gets used
                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, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[7]);
        fn partial_route_hint_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Simple test across 2, 3, 5, and 4 via a last_hop channel
                // Tests the behaviour when the RouteHint contains a suboptimal hop.
                let mut invalid_last_hops = last_hops_multi_private_channels(&nodes);
                invalid_last_hops.push(invalid_last_hop);
                {
 -                      if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &invalid_last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)) {
 +                      if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &invalid_last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Last hop cannot have a payee as a source.");
                        } else { panic!(); }
                }
  
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops_multi_private_channels(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops_multi_private_channels(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 5);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
        fn ignores_empty_last_hops_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Test handling of an empty RouteHint passed in Invoice.
  
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &empty_last_hop(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &empty_last_hop(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 5);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
        fn multi_hint_last_hops_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
                // Test through channels 2, 3, 5, 8.
                // Test shows that multiple hop hints are considered.
  
                        excess_data: Vec::new()
                });
  
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &multi_hint_last_hops(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &multi_hint_last_hops(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 4);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
        fn last_hops_with_public_channel_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
                // This test shows that public routes can be present in the invoice
                // which would be handled in the same manner.
  
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops_with_public_channel(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops_with_public_channel(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 5);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
        fn our_chans_last_hop_connect_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Simple test with outbound channel to 4 to test that last_hops and first_hops connect
                let our_chans = vec![get_channel_details(Some(42), nodes[3].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let mut last_hops = last_hops(&nodes);
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, Some(&our_chans.iter().collect::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, Some(&our_chans.iter().collect::<Vec<_>>()), &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 2);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[3]);
                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, &nodes[6], None, None, &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops.iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 4);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
                assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
  
                // ...but still use 8 for larger payments as 6 has a variable feerate
 -              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops.iter().collect::<Vec<_>>(), 2000, 42, Arc::clone(&logger)).unwrap();
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops.iter().collect::<Vec<_>>(), 2000, 42, Arc::clone(&logger), &scorer).unwrap();
                assert_eq!(route.paths[0].len(), 5);
  
                assert_eq!(route.paths[0][0].pubkey, nodes[1]);
                        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()))
 +              let scorer = Scorer::new(0);
 +              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, &test_utils::TestLogger::new(), &scorer)
        }
  
        #[test]
  
                let (secp_ctx, mut net_graph_msg_handler, chain_monitor, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We will use a simple single-path route from
                // our node to node2 via node0: channels {1, 3}.
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 250_000_001, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 250_000_001, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route an exact amount we have should be fine.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 250_000_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 250_000_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.len(), 2);
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                                      Some(InvoiceFeatures::known()), Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 200_000_001, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 200_000_001, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route an exact amount we have should be fine.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 200_000_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), Some(&our_chans.iter().collect::<Vec<_>>()), &Vec::new(), 200_000_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.len(), 2);
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 15_001, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 15_001, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route an exact amount we have should be fine.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 15_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 15_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.len(), 2);
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 15_001, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 15_001, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route an exact amount we have should be fine.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 15_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 15_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.len(), 2);
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 10_001, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 10_001, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route an exact amount we have should be fine.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 10_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 10_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.len(), 2);
                // one of the latter hops is limited.
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Path via {node7, node2, node4} is channels {12, 13, 6, 11}.
                // {12, 13, 11} have the capacities of 100, {6} has a capacity of 50.
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 60_000, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 60_000, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route 49 sats (just a bit below the capacity).
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 49_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 49_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
                {
                        // Attempt to route an exact amount is also fine
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 50_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 50_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
        fn ignore_fee_first_hop_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // Path via node0 is channels {1, 3}. Limit them to 100 and 50 sats (total limit 50).
                update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
                });
  
                {
 -                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 50_000, 42, Arc::clone(&logger)).unwrap();
 +                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 50_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
        fn simple_mpp_route_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We need a route consisting of 3 paths:
                // From our node to node2 via node0, node7, node1 (three paths one hop each).
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph,
 -                                      &nodes[2], Some(InvoiceFeatures::known()), None, &Vec::new(), 300_000, 42, Arc::clone(&logger)) {
 +                                      &nodes[2], Some(InvoiceFeatures::known()), None, &Vec::new(), 300_000, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                        // Now, attempt to route 250 sats (just a bit below the capacity).
                        // Our algorithm should provide us with these 3 paths.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 250_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 250_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
                {
                        // Attempt to route an exact amount is also fine
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 290_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 290_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
        fn long_mpp_route_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We need a route consisting of 3 paths:
                // From our node to node3 via {node0, node2}, {node7, node2, node4} and {node7, node2}.
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 350_000, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 350_000, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                        // Now, attempt to route 300 sats (exact amount we can route).
                        // Our algorithm should provide us with these 3 paths, 100 sats each.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 300_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 300_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 3);
  
                        let mut total_amount_paid_msat = 0;
        fn mpp_cheaper_route_test() {
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // This test checks that if we have two cheaper paths and one more expensive path,
                // so that liquidity-wise any 2 of 3 combination is sufficient,
                        // Now, attempt to route 180 sats.
                        // Our algorithm should provide us with these 2 paths.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 180_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 180_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 2);
  
                        let mut total_value_transferred_msat = 0;
                // if the fee is not properly accounted for, the behavior is different.
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We need a route consisting of 2 paths:
                // From our node to node3 via {node0, node2} and {node7, node2, node4}.
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 210_000, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 210_000, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                {
                        // Now, attempt to route 200 sats (exact amount we can route).
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[3],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 200_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 200_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 2);
  
                        let mut total_amount_paid_msat = 0;
                // path finding we realize that we found more capacity than we need.
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We need a route consisting of 3 paths:
                // From our node to node2 via node0, node7, node1 (three paths one hop each).
                {
                        // Attempt to route more than available results in a failure.
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 150_000, 42, Arc::clone(&logger)) {
 +                                      Some(InvoiceFeatures::known()), None, &Vec::new(), 150_000, 42, Arc::clone(&logger), &scorer) {
                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
                        // Now, attempt to route 125 sats (just a bit below the capacity of 3 channels).
                        // Our algorithm should provide us with these 3 paths.
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 125_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 125_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
                {
                        // Attempt to route without the last small cheap channel
                        let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2],
 -                              Some(InvoiceFeatures::known()), None, &Vec::new(), 90_000, 42, Arc::clone(&logger)).unwrap();
 +                              Some(InvoiceFeatures::known()), None, &Vec::new(), 90_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 2);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
                let net_graph_msg_handler = NetGraphMsgHandler::new(network_graph, None, Arc::clone(&logger));
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                add_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6);
                update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
  
                {
                        // Now ensure the route flows simply over nodes 1 and 4 to 6.
 -                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &Vec::new(), 10_000, 42, Arc::clone(&logger)).unwrap();
 +                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &Vec::new(), 10_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].len(), 3);
  
                // we calculated fees on a higher value, resulting in us ignoring such paths.
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, _, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We modify the graph to set the htlc_maximum of channel 2 to below the value we wish to
                // send.
                {
                        // Now, attempt to route 90 sats, which is exactly 90 sats at the last hop, plus the
                        // 200% fee charged channel 13 in the 1-to-2 direction.
 -                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 90_000, 42, Arc::clone(&logger)).unwrap();
 +                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], None, None, &Vec::new(), 90_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].len(), 2);
  
                // resulting in us thinking there is no possible path, even if other paths exist.
                let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
 +              let scorer = Scorer::new(0);
  
                // We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2
                // gets an htlc_maximum_msat of 80_000 and channel 4 an htlc_minimum_msat of 90_000. We
                        // Now, attempt to route 90 sats, hitting the htlc_minimum on channel 4, but
                        // overshooting the htlc_maximum on channel 2. Thus, we should pick the (absurdly
                        // expensive) channels 12-13 path.
 -                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], Some(InvoiceFeatures::known()), None, &Vec::new(), 90_000, 42, Arc::clone(&logger)).unwrap();
 +                      let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[2], Some(InvoiceFeatures::known()), None, &Vec::new(), 90_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].len(), 2);
  
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let logger = Arc::new(test_utils::TestLogger::new());
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
 +              let scorer = Scorer::new(0);
  
                {
                        let route = get_route(&our_id, &network_graph, &nodes[0], Some(InvoiceFeatures::known()), Some(&[
                                &get_channel_details(Some(3), nodes[0], InitFeatures::known(), 200_000),
                                &get_channel_details(Some(2), nodes[0], InitFeatures::known(), 10_000),
 -                      ]), &[], 100_000, 42, Arc::clone(&logger)).unwrap();
 +                      ]), &[], 100_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].len(), 1);
  
                        let route = get_route(&our_id, &network_graph, &nodes[0], Some(InvoiceFeatures::known()), Some(&[
                                &get_channel_details(Some(3), nodes[0], InitFeatures::known(), 50_000),
                                &get_channel_details(Some(2), nodes[0], InitFeatures::known(), 50_000),
 -                      ]), &[], 100_000, 42, Arc::clone(&logger)).unwrap();
 +                      ]), &[], 100_000, 42, Arc::clone(&logger), &scorer).unwrap();
                        assert_eq!(route.paths.len(), 2);
                        assert_eq!(route.paths[0].len(), 1);
                        assert_eq!(route.paths[1].len(), 1);
                }
        }
  
 +      #[test]
 +      fn prefers_shorter_route_with_higher_fees() {
 +              let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
 +              let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 +
 +              // Applying a 100 msat penalty to each hop results in taking channels 7 and 10 to nodes[6]
 +              // from nodes[2] rather than channel 6, 11, and 8, even though the longer path is cheaper.
 +              let scorer = Scorer::new(100);
 +              let route = get_route(&our_id, &net_graph_msg_handler.network_graph, &nodes[6], None, None, &last_hops(&nodes).iter().collect::<Vec<_>>(), 100, 42, Arc::clone(&logger), &scorer).unwrap();
 +              assert_eq!(route.paths[0].len(), 4);
 +
 +              assert_eq!(route.paths[0][0].pubkey, nodes[1]);
 +              assert_eq!(route.paths[0][0].short_channel_id, 2);
 +              assert_eq!(route.paths[0][0].fee_msat, 200);
 +              assert_eq!(route.paths[0][0].cltv_expiry_delta, (4 << 8) | 1);
 +              assert_eq!(route.paths[0][0].node_features.le_flags(), &id_to_feature_flags(2));
 +              assert_eq!(route.paths[0][0].channel_features.le_flags(), &id_to_feature_flags(2));
 +
 +              assert_eq!(route.paths[0][1].pubkey, nodes[2]);
 +              assert_eq!(route.paths[0][1].short_channel_id, 4);
 +              assert_eq!(route.paths[0][1].fee_msat, 100);
 +              assert_eq!(route.paths[0][1].cltv_expiry_delta, (7 << 8) | 1);
 +              assert_eq!(route.paths[0][1].node_features.le_flags(), &id_to_feature_flags(3));
 +              assert_eq!(route.paths[0][1].channel_features.le_flags(), &id_to_feature_flags(4));
 +
 +              assert_eq!(route.paths[0][2].pubkey, nodes[5]);
 +              assert_eq!(route.paths[0][2].short_channel_id, 7);
 +              assert_eq!(route.paths[0][2].fee_msat, 0);
 +              assert_eq!(route.paths[0][2].cltv_expiry_delta, (10 << 8) | 1);
 +              assert_eq!(route.paths[0][2].node_features.le_flags(), &id_to_feature_flags(6));
 +              assert_eq!(route.paths[0][2].channel_features.le_flags(), &id_to_feature_flags(7));
 +
 +              assert_eq!(route.paths[0][3].pubkey, nodes[6]);
 +              assert_eq!(route.paths[0][3].short_channel_id, 10);
 +              assert_eq!(route.paths[0][3].fee_msat, 100);
 +              assert_eq!(route.paths[0][3].cltv_expiry_delta, 42);
 +              assert_eq!(route.paths[0][3].node_features.le_flags(), &Vec::<u8>::new()); // We don't pass flags in from invoices yet
 +              assert_eq!(route.paths[0][3].channel_features.le_flags(), &Vec::<u8>::new()); // We can't learn any flags from invoices, sadly
 +
 +              assert_eq!(route.get_total_fees(), 300);
 +              assert_eq!(route.get_total_amount(), 100);
 +      }
 +
        #[test]
        fn total_fees_single_path() {
                let route = Route {
                        },
                };
                let graph = NetworkGraph::read(&mut d).unwrap();
 +              let scorer = Scorer::new(0);
  
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
                let mut seed = random_init_seed() as usize;
                                seed = seed.overflowing_mul(0xdeadbeef).0;
                                let dst = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 200_000_000;
 -                              if get_route(src, &graph, dst, None, None, &[], amt, 42, &test_utils::TestLogger::new()).is_ok() {
 +                              if get_route(src, &graph, dst, None, None, &[], amt, 42, &test_utils::TestLogger::new(), &scorer).is_ok() {
                                        continue 'load_endpoints;
                                }
                        }
                        },
                };
                let graph = NetworkGraph::read(&mut d).unwrap();
 +              let scorer = Scorer::new(0);
  
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
                let mut seed = random_init_seed() as usize;
                                seed = seed.overflowing_mul(0xdeadbeef).0;
                                let dst = &PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 200_000_000;
 -                              if get_route(src, &graph, dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &test_utils::TestLogger::new()).is_ok() {
 +                              if get_route(src, &graph, dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &test_utils::TestLogger::new(), &scorer).is_ok() {
                                        continue 'load_endpoints;
                                }
                        }
@@@ -4560,7 -4455,6 +4560,7 @@@ pub(crate) mod test_utils 
  #[cfg(all(test, feature = "unstable", not(feature = "no-std")))]
  mod benches {
        use super::*;
 +      use routing::scorer::Scorer;
        use util::logger::{Logger, Record};
  
        use test::Bencher;
                let mut d = test_utils::get_route_file().unwrap();
                let graph = NetworkGraph::read(&mut d).unwrap();
                let nodes = graph.read_only().nodes().clone();
 +              let scorer = Scorer::new(0);
  
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
                let mut path_endpoints = Vec::new();
                                seed *= 0xdeadbeef;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 1_000_000;
 -                              if get_route(&src, &graph, &dst, None, None, &[], amt, 42, &DummyLogger{}).is_ok() {
 +                              if get_route(&src, &graph, &dst, None, None, &[], amt, 42, &DummyLogger{}, &scorer).is_ok() {
                                        path_endpoints.push((src, dst, amt));
                                        continue 'load_endpoints;
                                }
                let mut idx = 0;
                bench.iter(|| {
                        let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
 -                      assert!(get_route(&src, &graph, &dst, None, None, &[], amt, 42, &DummyLogger{}).is_ok());
 +                      assert!(get_route(&src, &graph, &dst, None, None, &[], amt, 42, &DummyLogger{}, &scorer).is_ok());
                        idx += 1;
                });
        }
                let mut d = test_utils::get_route_file().unwrap();
                let graph = NetworkGraph::read(&mut d).unwrap();
                let nodes = graph.read_only().nodes().clone();
 +              let scorer = Scorer::new(0);
  
                // First, get 100 (source, destination) pairs for which route-getting actually succeeds...
                let mut path_endpoints = Vec::new();
                                seed *= 0xdeadbeef;
                                let dst = PublicKey::from_slice(nodes.keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap();
                                let amt = seed as u64 % 1_000_000;
 -                              if get_route(&src, &graph, &dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}).is_ok() {
 +                              if get_route(&src, &graph, &dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}, &scorer).is_ok() {
                                        path_endpoints.push((src, dst, amt));
                                        continue 'load_endpoints;
                                }
                let mut idx = 0;
                bench.iter(|| {
                        let (src, dst, amt) = path_endpoints[idx % path_endpoints.len()];
 -                      assert!(get_route(&src, &graph, &dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}).is_ok());
 +                      assert!(get_route(&src, &graph, &dst, Some(InvoiceFeatures::known()), None, &[], amt, 42, &DummyLogger{}, &scorer).is_ok());
                        idx += 1;
                });
        }