Merge pull request #1108 from TheBlueMatt/2021-10-persist-mon-blocks
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 20 Oct 2021 00:53:26 +0000 (00:53 +0000)
committerGitHub <noreply@github.com>
Wed, 20 Oct 2021 00:53:26 +0000 (00:53 +0000)
Persist ChannelMonitors after new blocks are connected

19 files changed:
CHANGELOG.md
README.md
fuzz/src/full_stack.rs
fuzz/src/router.rs
lightning-background-processor/Cargo.toml
lightning-block-sync/Cargo.toml
lightning-invoice/Cargo.toml
lightning-invoice/src/utils.rs
lightning-net-tokio/Cargo.toml
lightning-persister/Cargo.toml
lightning/Cargo.toml
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/shutdown_tests.rs
lightning/src/routing/mod.rs
lightning/src/routing/router.rs
lightning/src/routing/scorer.rs [new file with mode: 0644]
lightning/src/util/events.rs

index 24a8c96ffaa4e94d88139a31549be0d4c079e0ec..518badd1ed21d56c6533f4cbb8dd53c649078af3 100644 (file)
@@ -1,3 +1,81 @@
+# 0.0.102 - 2021-10-18
+
+## API Updates
+ * `get_route` now takes a `Score` as an argument. `Score` is queried during
+   the route-finding process, returning the absolute amounts which you are
+   willing to pay to avoid routing over a given channel. As a default, a
+   `Scorer` is provided which returns a constant amount, with a suggested
+   default of 500 msat. This translates to a willingness to pay up to 500 msat
+   in additional fees per hop in order to avoid additional hops (#1124).
+ * `Event::PaymentPathFailed` now contains a `short_channel_id` field which may
+   be filled in with a channel that can be "blamed" for the payment failure.
+   Payment retries should likely avoid the given channel for some time (#1077).
+ * `PublicKey`s in `NetworkGraph` have been replaced with a `NodeId` struct
+   which contains only a simple `[u8; 33]`, substantially improving
+   `NetworkGraph` deserialization performance (#1107).
+ * `ChainMonitor`'s `HashMap` of `ChannelMonitor`s is now private, exposed via
+   `Chainmonitor::get_monitor` and `ChainMonitor::list_monitors` instead
+   (#1112).
+ * When an outbound channel is closed prior to the broadcasting of its funding
+   transaction, but after you call
+   `ChannelManager::funding_transaction_generated`, a new event type,
+   `Event::DiscardFunding`, is generated, informing you the transaction was not
+   broadcasted and that you can spend the same inputs again elsewhere (#1098).
+ * `ChannelManager::create_channel` now returns the temporary channel ID which
+   may later appear in `Event::ChannelClosed` or `ChannelDetails` prior to the
+   channel being funded (#1121).
+ * `Event::PaymentSent` now contains the payment hash as well as the payment
+   preimage (#1062).
+ * `ReadOnlyNetworkGraph::get_addresses` now returns owned `NetAddress` rather
+   than references. As a side-effect this method is now exposed in foreign
+   language bindings (#1115).
+ * The `Persist` and `ChannelMonitorUpdateErr` types have moved to the
+   `lightning::chain::chainmonitor` and `lightning::chain` modules,
+   respectively (#1112).
+ * `ChannelManager::send_payment` now returns a `PaymentId` which identifies a
+   payment (whether MPP or not) and can be used to retry the full payment or
+   MPP parts through `retry_payment` (#1096). Note that doing so is currently
+   *not* crash safe, and you may find yourself sending twice. It is recommended
+   that you *not* use the `retry_payment` API until the next release.
+
+## Bug Fixes
+ * Due to an earlier fix for the Lightning dust inflation vulnerability tracked
+   in CVE-2021-41591/CVE-2021-41592/CVE-2021-41593 in 0.0.100, we required
+   counterparties to accept a dust limit slightly lower than the dust limit now
+   required by other implementations. This appeared as, at least, latest lnd
+   always refusing to accept channels opened by LDK clients (#1065).
+ * If there are multiple channels available to the same counterparty,
+   `get_route` would only consider the channel listed last as available for
+   sending (#1100).
+ * `Persist` implementations returning
+   `ChannelMonitorUpdateErr::TemporaryFailure` from `watch_channel` previously
+   resulted in the `ChannelMonitor` not being stored at all, resulting in a
+   panic after monitor updating is complete (#1112).
+ * If payments are pending awaiting forwarding at startup, an
+   `Event::PendingHTLCsForwardable` event will always be provided. This ensures
+   user code calls `ChannelManager::process_pending_htlc_fowards` even if it
+   shut down while awaiting the batching timer during the previous run (#1076).
+ * If a call to `ChannelManager::send_payment` failed due to lack of
+   availability of funds locally, LDK would store the payment as pending
+   forever, with no ability to retry or fail it, leaking memory (#1109).
+
+## Serialization Compatibility
+ * All above new Events/fields are ignored by prior clients. All above new
+   Events/fields, except for `Event::PaymentSent::payment_hash` are not present
+   when reading objects serialized by prior versions of the library.
+
+In total, this release features 32 files changed, 2248 insertions, and 1483
+deletions in 51 commits from 7 authors, in alphabetical order:
+
+ * 1nF0rmed
+ * Duncan Dean
+ * Elias Rohrer
+ * Galder ZamarrenĖƒo
+ * Jeffrey Czyz
+ * Matt Corallo
+ * Valentine Wallace
+
+
 # 0.0.101 - 2021-09-23
 
 ## API Updates
index d73e71da8aaf3a10fa076ca6fc953f6949094c78..f44f59a3fb4850dbf8dfc21fed90bc2816b85479 100644 (file)
--- a/README.md
+++ b/README.md
@@ -87,7 +87,7 @@ In general, Rust-Lightning does not provide (but LDK has implementations of):
   hardware wallets.
 
 LDK's customizability was presented about at Advancing Bitcoin in February 2020:
-https://vimeo.com/showcase/7131712/video/418412286
+https://vimeo.com/showcase/8372504/video/412818125
 
 Design Goal
 -----------
index f371a7eae6d444a54e4fbb274f8e9ea57a0b1f77..84ba7bab502ea4f44ec2bca37ee83a01b1a5e404 100644 (file)
@@ -37,8 +37,9 @@ use lightning::ln::channelmanager::{ChainParameters, ChannelManager};
 use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,IgnoringMessageHandler};
 use lightning::ln::msgs::DecodeError;
 use lightning::ln::script::ShutdownScript;
-use lightning::routing::router::get_route;
 use lightning::routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
+use lightning::routing::router::get_route;
+use lightning::routing::scorer::Scorer;
 use lightning::util::config::UserConfig;
 use lightning::util::errors::APIError;
 use lightning::util::events::Event;
@@ -381,6 +382,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
        let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret());
        let network_graph = NetworkGraph::new(genesis_block(network).block_hash());
        let net_graph_msg_handler = Arc::new(NetGraphMsgHandler::new(network_graph, None, Arc::clone(&logger)));
+       let scorer = Scorer::new(0);
 
        let peers = RefCell::new([false; 256]);
        let mut loss_detector = MoneyLossDetector::new(&peers, channelmanager.clone(), monitor.clone(), PeerManager::new(MessageHandler {
@@ -436,7 +438,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                        },
                        4 => {
                                let value = slice_to_be24(get_slice!(3)) as u64;
-                               let route = match get_route(&our_id, &net_graph_msg_handler.network_graph, &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger)) {
+                               let route = match get_route(&our_id, &net_graph_msg_handler.network_graph, &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger), &scorer) {
                                        Ok(route) => route,
                                        Err(_) => return,
                                };
@@ -453,7 +455,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
                        },
                        15 => {
                                let value = slice_to_be24(get_slice!(3)) as u64;
-                               let mut route = match get_route(&our_id, &net_graph_msg_handler.network_graph, &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger)) {
+                               let mut route = match get_route(&our_id, &net_graph_msg_handler.network_graph, &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger), &scorer) {
                                        Ok(route) => route,
                                        Err(_) => return,
                                };
index 6c792916f8215f40efc9d5a8a66e403ce215eaf9..6a207254ac9992e172d0fb194d7f6b76745e8d80 100644 (file)
@@ -17,6 +17,7 @@ use lightning::ln::channelmanager::{ChannelDetails, ChannelCounterparty};
 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};
@@ -216,7 +217,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                                                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,
@@ -247,11 +248,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                                }]));
                                        }
                                }
+                               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 178ffba80403acbffc35e4e7cb127e039fbe5a43..4e45bb2a83c11d58b6528c59cfef3208d07e86ac 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.0.101"
+version = "0.0.102"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "http://github.com/rust-bitcoin/rust-lightning"
@@ -11,9 +11,9 @@ edition = "2018"
 
 [dependencies]
 bitcoin = "0.27"
-lightning = { version = "0.0.101", path = "../lightning", features = ["allow_wallclock_use"] }
-lightning-persister = { version = "0.0.101", path = "../lightning-persister" }
+lightning = { version = "0.0.102", path = "../lightning", features = ["allow_wallclock_use"] }
+lightning-persister = { version = "0.0.102", path = "../lightning-persister" }
 
 [dev-dependencies]
-lightning = { version = "0.0.101", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.102", path = "../lightning", features = ["_test_utils"] }
 
index 5e0778cf7c7b87f58c0de61462ded34342e355b6..06ca52f70cd50e786b45e0b56b4142f003f993f5 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-block-sync"
-version = "0.0.101"
+version = "0.0.102"
 authors = ["Jeffrey Czyz", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "http://github.com/rust-bitcoin/rust-lightning"
@@ -15,7 +15,7 @@ rpc-client = [ "serde", "serde_json", "chunked_transfer" ]
 
 [dependencies]
 bitcoin = "0.27"
-lightning = { version = "0.0.101", path = "../lightning" }
+lightning = { version = "0.0.102", path = "../lightning" }
 tokio = { version = "1.0", features = [ "io-util", "net", "time" ], optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }
 serde_json = { version = "1.0", optional = true }
index 1169d9283cb67ceae0635d9a66e1a6104194d774..886fbc553acae0f97c772e2fbdf9d067146e668e 100644 (file)
@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.9.0"
+version = "0.10.0"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"
@@ -10,11 +10,11 @@ readme = "README.md"
 
 [dependencies]
 bech32 = "0.8"
-lightning = { version = "0.0.101", path = "../lightning" }
+lightning = { version = "0.0.102", path = "../lightning" }
 secp256k1 = { version = "0.20", features = ["recovery"] }
 num-traits = "0.2.8"
 bitcoin_hashes = "0.10"
 
 [dev-dependencies]
 hex = "0.3"
-lightning = { version = "0.0.101", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.102", path = "../lightning", features = ["_test_utils"] }
index 409fa803cab9cac20f423e21483d0bf58d8face8..31c1ab2550e4a5a53b079355ecd90f74f111a0a0 100644 (file)
@@ -98,6 +98,7 @@ mod test {
        use lightning::ln::features::InitFeatures;
        use lightning::ln::msgs::ChannelMessageHandler;
        use lightning::routing::router;
+       use lightning::routing::scorer::Scorer;
        use lightning::util::events::MessageSendEventsProvider;
        use lightning::util::test_utils;
        #[test]
@@ -117,6 +118,7 @@ mod test {
                let last_hops = invoice.route_hints();
                let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
                let logger = test_utils::TestLogger::new();
+               let scorer = Scorer::new(0);
                let route = router::get_route(
                        &nodes[0].node.get_our_node_id(),
                        network_graph,
@@ -127,6 +129,7 @@ mod test {
                        amt_msat,
                        invoice.min_final_cltv_expiry() as u32,
                        &logger,
+                       &scorer,
                ).unwrap();
 
                let payment_event = {
index ad13c8040f24204745eceae06fa2360fd4ab7f39..3d9a31f182b83d3ed9b0f6ed5d6223290acaf016 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-net-tokio"
-version = "0.0.101"
+version = "0.0.102"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-bitcoin/rust-lightning/"
@@ -12,7 +12,7 @@ edition = "2018"
 
 [dependencies]
 bitcoin = "0.27"
-lightning = { version = "0.0.101", path = "../lightning" }
+lightning = { version = "0.0.102", path = "../lightning" }
 tokio = { version = "1.0", features = [ "io-util", "macros", "rt", "sync", "net", "time" ] }
 
 [dev-dependencies]
index ded73551e75c4254c9ef646eb075d488e8c5a9ae..9e072dce78f89a44046e6530580256e349392c0c 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.0.101"
+version = "0.0.102"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-bitcoin/rust-lightning/"
@@ -13,11 +13,11 @@ unstable = ["lightning/unstable"]
 
 [dependencies]
 bitcoin = "0.27"
-lightning = { version = "0.0.101", path = "../lightning" }
+lightning = { version = "0.0.102", path = "../lightning" }
 libc = "0.2"
 
 [target.'cfg(windows)'.dependencies]
 winapi = { version = "0.3", features = ["winbase"] }
 
 [dev-dependencies]
-lightning = { version = "0.0.101", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.102", path = "../lightning", features = ["_test_utils"] }
index 9d6217b04746f81c4ecd38fa9d5090b40feb4aff..271a3d662abcf459a5c977cfc9b03bfd19cc1ead 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.0.101"
+version = "0.0.102"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-bitcoin/rust-lightning/"
index 1f3ad5541f1ba1d4697032d668c4f39765f29cb1..f3085fe2011de21201d93d5cb84c7ab710a1d794 100644 (file)
@@ -242,7 +242,7 @@ type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource
 
 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 {
@@ -278,7 +278,7 @@ 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(),
@@ -289,7 +289,7 @@ impl MsgHandleErrInternal {
                                        },
                                },
                        },
-                       chan_id: Some(channel_id),
+                       chan_id: Some((channel_id, user_channel_id)),
                        shutdown_finish: Some((shutdown_res, channel_update)),
                }
        }
@@ -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 @@ 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 @@ 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);
@@ -945,7 +949,8 @@ macro_rules! convert_chan_err {
                                        $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 @@ 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 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
        /// 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) });
                }
@@ -1294,7 +1308,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        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) }),
                        }
@@ -1305,8 +1319,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                // 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() });
@@ -1320,7 +1335,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        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> {
@@ -1346,7 +1361,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        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(),
@@ -1393,7 +1408,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        },
                        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> {
@@ -2228,7 +2247,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                                        (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)
                                },
@@ -2570,7 +2589,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                                                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"); }
                                                                        };
@@ -5613,6 +5632,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                                        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 {
@@ -5808,6 +5828,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;
@@ -6046,13 +6067,14 @@ mod tests {
                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();
@@ -6080,7 +6102,7 @@ mod tests {
 
                // 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();
@@ -6134,9 +6156,10 @@ mod tests {
                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]);
@@ -6170,9 +6193,10 @@ mod tests {
                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]);
@@ -6233,6 +6257,7 @@ pub mod bench {
        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};
@@ -6340,8 +6365,9 @@ pub mod bench {
                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 137214f0c21e0458f5dc8c779b561c1fbc6b93bd..0f188ac38eea4d4eadc2349ceb29af16c6db01d3 100644 (file)
@@ -15,8 +15,9 @@ use chain::channelmonitor::ChannelMonitor;
 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};
@@ -526,11 +527,12 @@ pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_
 }
 
 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);
@@ -1010,11 +1012,12 @@ macro_rules! get_route_and_payment_hash {
        ($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 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
+                       &$last_hops, $recv_value, $cltv, $send_node.logger, &scorer
                ).unwrap();
                (route, payment_hash, payment_preimage, payment_secret)
        }}
@@ -1325,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()) {
@@ -1340,7 +1344,8 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 
 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 63bf52b413453360312ebfbab793c383c3e786e5..eb7d868a95d05becd024058ab40a4969aa396217 100644 (file)
@@ -25,6 +25,7 @@ use ln::{chan_utils, onion_utils};
 use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT;
 use routing::network_graph::{NetworkUpdate, RoutingFees};
 use routing::router::{Route, RouteHop, RouteHint, RouteHintHop, get_route, get_keysend_route};
+use routing::scorer::Scorer;
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
@@ -7297,7 +7298,8 @@ fn test_check_htlc_underpaying() {
        // Create some initial channels
        create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
 
-       let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 10_000, TEST_FINAL_CLTV, nodes[0].logger).unwrap();
+       let scorer = Scorer::new(0);
+       let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer).unwrap();
        let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
        let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, 0).unwrap();
        nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
@@ -7694,11 +7696,12 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
 
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000, InitFeatures::known(), InitFeatures::known());
        // Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps)
+       let scorer = Scorer::new(0);
        let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph,
-               &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 3_000_000, 50, nodes[0].logger).unwrap();
+               &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 3_000_000, 50, nodes[0].logger, &scorer).unwrap();
        let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 3_000_000).0;
        let route = get_route(&nodes[1].node.get_our_node_id(), &nodes[1].net_graph_msg_handler.network_graph,
-               &nodes[0].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 3_000_000, 50, nodes[0].logger).unwrap();
+               &nodes[0].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 3_000_000, 50, nodes[0].logger, &scorer).unwrap();
        send_along_route(&nodes[1], route, &[&nodes[0]], 3_000_000);
 
        let revoked_local_txn = get_local_commitment_txn!(nodes[1], chan.2);
@@ -9187,8 +9190,9 @@ fn test_keysend_payments_to_public_node() {
        let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
        let payer_pubkey = nodes[0].node.get_our_node_id();
        let payee_pubkey = nodes[1].node.get_our_node_id();
+       let scorer = Scorer::new(0);
        let route = get_keysend_route(
-               &payer_pubkey, &network_graph, &payee_pubkey, None, &vec![], 10000, 40, nodes[0].logger
+               &payer_pubkey, &network_graph, &payee_pubkey, None, &vec![], 10000, 40, nodes[0].logger, &scorer
        ).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);
@@ -9217,9 +9221,10 @@ fn test_keysend_payments_to_private_node() {
        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
+               &vec![], 10000, 40, nodes[0].logger, &scorer
        ).unwrap();
 
        let test_preimage = PaymentPreimage([42; 32]);
index 37e7165cf71e2aea4834694a0df55db666d5dd35..e82cdae60f2b31617f801221163b4fb581c51a63 100644 (file)
@@ -13,8 +13,9 @@ use chain::keysinterface::KeysInterface;
 use chain::transaction::OutPoint;
 use ln::{PaymentPreimage, PaymentHash};
 use ln::channelmanager::PaymentSendFailure;
-use routing::network_graph::NetworkUpdate;
 use routing::router::get_route;
+use routing::network_graph::NetworkUpdate;
+use routing::scorer::Scorer;
 use ln::features::{InitFeatures, InvoiceFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler, ErrorAction};
@@ -81,6 +82,7 @@ fn updates_shutdown_wait() {
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
        let logger = test_utils::TestLogger::new();
+       let scorer = Scorer::new(0);
 
        let (our_payment_preimage, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 100000);
 
@@ -97,8 +99,8 @@ fn updates_shutdown_wait() {
 
        let net_graph_msg_handler0 = &nodes[0].net_graph_msg_handler;
        let net_graph_msg_handler1 = &nodes[1].net_graph_msg_handler;
-       let route_1 = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler0.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
-       let route_2 = get_route(&nodes[1].node.get_our_node_id(), &net_graph_msg_handler1.network_graph, &nodes[0].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
+       let route_1 = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler0.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger, &scorer).unwrap();
+       let route_2 = get_route(&nodes[1].node.get_our_node_id(), &net_graph_msg_handler1.network_graph, &nodes[0].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger, &scorer).unwrap();
        unwrap_send_err!(nodes[0].node.send_payment(&route_1, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable {..}, {});
        unwrap_send_err!(nodes[1].node.send_payment(&route_2, payment_hash, &Some(payment_secret)), true, APIError::ChannelUnavailable {..}, {});
 
index ca7b920563839cab087eaab7b3e102e8eb268cb9..51ffd91b50483d58bc2016440bdb98d9f82886e5 100644 (file)
@@ -9,5 +9,17 @@
 
 //! Structs and impls for receiving messages about the network and storing the topology live here.
 
-pub mod router;
 pub mod network_graph;
+pub mod router;
+pub mod scorer;
+
+use routing::network_graph::NodeId;
+
+/// An interface used to score payment channels for path finding.
+///
+///    Scoring is in terms of fees willing to be paid in order to avoid routing through a channel.
+pub trait Score {
+       /// Returns the fee in msats willing to be paid to avoid routing through the given channel
+       /// in the direction from `source` to `target`.
+       fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId) -> u64;
+}
index d6adb889d5060fe7e4122ba42e733a5e6615584a..b617eebd42d834996279c9346e07a743bee7c1de 100644 (file)
@@ -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};
 
@@ -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))
        }
 }
@@ -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
@@ -352,13 +363,17 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 /// 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).
@@ -380,13 +395,15 @@ pub fn get_keysend_route<L: Deref>(our_node_pubkey: &PublicKey, network: &Networ
 /// 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});
        }
@@ -560,7 +577,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                // 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
@@ -645,11 +662,10 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                // 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
@@ -679,6 +695,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                                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,
@@ -730,12 +747,16 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                                }
                                                        }
 
+                                                       let path_penalty_msat = $next_hops_path_penalty_msat
+                                                               .checked_add(scorer.channel_penalty_msat($chan_id.clone(), &$src_node_id, &$dest_node_id))
+                                                               .unwrap_or_else(|| u64::max_value());
                                                        let 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),
@@ -754,8 +775,12 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                        // 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);
@@ -770,6 +795,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                                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;
@@ -798,7 +824,10 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                                        // 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
+                                                                       );
                                                                }
                                                        }
                                                }
@@ -816,7 +845,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
        // 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;
@@ -832,7 +861,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                        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);
                                        }
                                }
 
@@ -851,7 +880,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                                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);
                                                                                }
                                                                        }
                                                                }
@@ -859,7 +888,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                                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);
                                                                                }
                                                                        }
                                                                }
@@ -886,7 +915,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                // 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);
                        }
                }
@@ -900,7 +929,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                        // 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);
                        },
                }
 
@@ -925,6 +954,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                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
@@ -943,11 +973,17 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                _ => 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
 
+                                       let src_node_id = NodeId::from_pubkey(&hop.src_node_id);
+                                       let dest_node_id = NodeId::from_pubkey(&prev_hop_id);
+                                       aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
+                                               .checked_add(scorer.channel_penalty_msat(hop.short_channel_id, &src_node_id, &dest_node_id))
+                                               .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, src_node_id, dest_node_id, directional_info, reqd_channel_cap, &empty_channel_features, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat) {
                                                // If 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
@@ -957,7 +993,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                        // 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);
                                                }
                                        }
 
@@ -991,7 +1027,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                                                // 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);
                                                        }
                                                }
                                        }
@@ -1014,7 +1050,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                // 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.
@@ -1140,7 +1176,7 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
                        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);
                                },
                        }
                }
@@ -1288,8 +1324,10 @@ pub fn get_route<L: Deref>(our_node_pubkey: &PublicKey, network: &NetworkGraph,
 
 #[cfg(test)]
 mod tests {
+       use routing;
+       use routing::network_graph::{NetworkGraph, NetGraphMsgHandler, NodeId};
        use routing::router::{get_route, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees};
-       use routing::network_graph::{NetworkGraph, NetGraphMsgHandler};
+       use routing::scorer::Scorer;
        use chain::transaction::OutPoint;
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
        use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
@@ -1327,7 +1365,7 @@ mod tests {
                        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,
@@ -1752,14 +1790,15 @@ mod tests {
        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]);
@@ -1781,16 +1820,17 @@ mod tests {
        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);
        }
 
@@ -1798,6 +1838,7 @@ mod tests {
        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
 
@@ -1894,7 +1935,7 @@ mod tests {
                });
 
                // 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!(); }
 
@@ -1913,7 +1954,7 @@ mod tests {
                });
 
                // 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);
        }
 
@@ -1921,6 +1962,7 @@ mod tests {
        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.
@@ -1991,7 +2033,7 @@ mod tests {
                });
 
                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.
@@ -2037,7 +2079,7 @@ mod tests {
                });
 
                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);
@@ -2045,7 +2087,7 @@ mod tests {
                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);
@@ -2058,6 +2100,7 @@ mod tests {
        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 {
@@ -2086,13 +2129,13 @@ mod tests {
                });
 
                // 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]);
@@ -2114,6 +2157,7 @@ mod tests {
        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();
@@ -2122,13 +2166,13 @@ mod tests {
                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]);
@@ -2154,9 +2198,10 @@ mod tests {
        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]);
@@ -2182,7 +2227,7 @@ mod tests {
 
                // 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]);
@@ -2280,6 +2325,7 @@ mod tests {
        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.
@@ -2301,12 +2347,12 @@ mod tests {
                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]);
@@ -2375,10 +2421,11 @@ mod tests {
        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]);
@@ -2455,6 +2502,7 @@ mod tests {
        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.
 
@@ -2484,7 +2532,7 @@ mod tests {
                        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]);
@@ -2559,10 +2607,11 @@ mod tests {
        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]);
@@ -2607,11 +2656,12 @@ mod tests {
        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]);
@@ -2631,7 +2681,7 @@ mod tests {
                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]);
@@ -2665,7 +2715,7 @@ mod tests {
                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]);
@@ -2724,7 +2774,8 @@ mod tests {
                        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]
@@ -2777,6 +2828,7 @@ mod tests {
 
                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}.
@@ -2840,7 +2892,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -2848,7 +2900,7 @@ mod tests {
                {
                        // 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);
@@ -2877,7 +2929,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -2885,7 +2937,7 @@ mod tests {
                {
                        // 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);
@@ -2925,7 +2977,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -2933,7 +2985,7 @@ mod tests {
                {
                        // 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);
@@ -2996,7 +3048,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3004,7 +3056,7 @@ mod tests {
                {
                        // 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);
@@ -3029,7 +3081,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3037,7 +3089,7 @@ mod tests {
                {
                        // 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);
@@ -3052,6 +3104,7 @@ mod tests {
                // 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.
@@ -3137,7 +3190,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3145,7 +3198,7 @@ mod tests {
                {
                        // 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 {
@@ -3159,7 +3212,7 @@ mod tests {
                {
                        // 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 {
@@ -3175,6 +3228,7 @@ mod tests {
        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 {
@@ -3203,7 +3257,7 @@ mod tests {
                });
 
                {
-                       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 {
@@ -3219,6 +3273,7 @@ mod tests {
        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).
@@ -3311,7 +3366,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3320,7 +3375,7 @@ mod tests {
                        // 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 {
@@ -3334,7 +3389,7 @@ mod tests {
                {
                        // 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 {
@@ -3350,6 +3405,7 @@ mod tests {
        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}.
@@ -3485,7 +3541,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3494,7 +3550,7 @@ mod tests {
                        // 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;
@@ -3511,6 +3567,7 @@ mod tests {
        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,
@@ -3651,7 +3708,7 @@ mod tests {
                        // 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;
@@ -3677,6 +3734,7 @@ mod tests {
                // 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}.
@@ -3817,7 +3875,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3825,7 +3883,7 @@ mod tests {
                {
                        // 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;
@@ -3845,6 +3903,7 @@ mod tests {
                // 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).
@@ -3936,7 +3995,7 @@ mod tests {
                {
                        // 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!(); }
                }
@@ -3945,7 +4004,7 @@ mod tests {
                        // 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 {
@@ -3959,7 +4018,7 @@ mod tests {
                {
                        // 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 {
@@ -4002,6 +4061,7 @@ mod tests {
                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 {
@@ -4094,7 +4154,7 @@ mod tests {
 
                {
                        // 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);
 
@@ -4129,6 +4189,7 @@ mod tests {
                // 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.
@@ -4161,7 +4222,7 @@ mod tests {
                {
                        // 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);
 
@@ -4189,6 +4250,7 @@ mod tests {
                // 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
@@ -4222,7 +4284,7 @@ mod tests {
                        // 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);
 
@@ -4254,12 +4316,13 @@ mod tests {
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let logger = Arc::new(test_utils::TestLogger::new());
                let network_graph = NetworkGraph::new(genesis_block(Network::Testnet).header.block_hash());
+               let scorer = Scorer::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);
 
@@ -4271,7 +4334,7 @@ mod tests {
                        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);
@@ -4286,6 +4349,99 @@ mod tests {
                }
        }
 
+       #[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);
+
+               // Without penalizing each hop 100 msats, a longer path with lower fees is chosen.
+               let scorer = Scorer::new(0);
+               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();
+               let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
+
+               assert_eq!(route.get_total_fees(), 100);
+               assert_eq!(route.get_total_amount(), 100);
+               assert_eq!(path, vec![2, 4, 6, 11, 8]);
+
+               // 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();
+               let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
+
+               assert_eq!(route.get_total_fees(), 300);
+               assert_eq!(route.get_total_amount(), 100);
+               assert_eq!(path, vec![2, 4, 7, 10]);
+       }
+
+       struct BadChannelScorer {
+               short_channel_id: u64,
+       }
+
+       impl routing::Score for BadChannelScorer {
+               fn channel_penalty_msat(&self, short_channel_id: u64, _source: &NodeId, _target: &NodeId) -> u64 {
+                       if short_channel_id == self.short_channel_id { u64::max_value() } else { 0 }
+               }
+       }
+
+       struct BadNodeScorer {
+               node_id: NodeId,
+       }
+
+       impl routing::Score for BadNodeScorer {
+               fn channel_penalty_msat(&self, _short_channel_id: u64, _source: &NodeId, target: &NodeId) -> u64 {
+                       if *target == self.node_id { u64::max_value() } else { 0 }
+               }
+       }
+
+       #[test]
+       fn avoids_routing_through_bad_channels_and_nodes() {
+               let (secp_ctx, net_graph_msg_handler, _, logger) = build_graph();
+               let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
+
+               // A path to nodes[6] exists when no penalties are applied to any channel.
+               let scorer = Scorer::new(0);
+               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();
+               let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
+
+               assert_eq!(route.get_total_fees(), 100);
+               assert_eq!(route.get_total_amount(), 100);
+               assert_eq!(path, vec![2, 4, 6, 11, 8]);
+
+               // A different path to nodes[6] exists if channel 6 cannot be routed over.
+               let scorer = BadChannelScorer { short_channel_id: 6 };
+               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();
+               let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
+
+               assert_eq!(route.get_total_fees(), 300);
+               assert_eq!(route.get_total_amount(), 100);
+               assert_eq!(path, vec![2, 4, 7, 10]);
+
+               // A path to nodes[6] does not exist if nodes[2] cannot be routed through.
+               let scorer = BadNodeScorer { node_id: NodeId::from_pubkey(&nodes[2]) };
+               match 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
+               ) {
+                       Err(LightningError { err, .. } ) => {
+                               assert_eq!(err, "Failed to find a path to the given destination");
+                       },
+                       Ok(_) => panic!("Expected error"),
+               }
+       }
+
        #[test]
        fn total_fees_single_path() {
                let route = Route {
@@ -4377,6 +4533,7 @@ mod tests {
                        },
                };
                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;
@@ -4388,7 +4545,7 @@ mod tests {
                                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;
                                }
                        }
@@ -4406,6 +4563,7 @@ mod tests {
                        },
                };
                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;
@@ -4417,7 +4575,7 @@ mod tests {
                                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;
                                }
                        }
@@ -4455,6 +4613,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;
@@ -4469,6 +4628,7 @@ mod benches {
                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();
@@ -4480,7 +4640,7 @@ mod benches {
                                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;
                                }
@@ -4491,7 +4651,7 @@ mod benches {
                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;
                });
        }
@@ -4501,6 +4661,7 @@ mod benches {
                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();
@@ -4512,7 +4673,7 @@ mod benches {
                                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;
                                }
@@ -4523,7 +4684,7 @@ mod benches {
                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;
                });
        }
diff --git a/lightning/src/routing/scorer.rs b/lightning/src/routing/scorer.rs
new file mode 100644 (file)
index 0000000..0f43c3d
--- /dev/null
@@ -0,0 +1,81 @@
+// This file is Copyright its original authors, visible in version control
+// history.
+//
+// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
+// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
+// You may not use this file except in accordance with one or both of these
+// licenses.
+
+//! Utilities for scoring payment channels.
+//!
+//! [`Scorer`] may be given to [`get_route`] to score payment channels during path finding when a
+//! custom [`routing::Score`] implementation is not needed.
+//!
+//! # Example
+//!
+//! ```
+//! # extern crate secp256k1;
+//! #
+//! # use lightning::routing::network_graph::NetworkGraph;
+//! # use lightning::routing::router::get_route;
+//! # use lightning::routing::scorer::Scorer;
+//! # use lightning::util::logger::{Logger, Record};
+//! # use secp256k1::key::PublicKey;
+//! #
+//! # struct FakeLogger {};
+//! # impl Logger for FakeLogger {
+//! #     fn log(&self, record: &Record) { unimplemented!() }
+//! # }
+//! # fn find_scored_route(payer: PublicKey, payee: PublicKey, network_graph: NetworkGraph) {
+//! # let logger = FakeLogger {};
+//! #
+//! // Use the default channel penalty.
+//! let scorer = Scorer::default();
+//!
+//! // Or use a custom channel penalty.
+//! let scorer = Scorer::new(1_000);
+//!
+//! let route = get_route(&payer, &network_graph, &payee, None, None, &vec![], 1_000, 42, &logger, &scorer);
+//! # }
+//! ```
+//!
+//! [`get_route`]: crate::routing::router::get_route
+
+use routing;
+
+use routing::network_graph::NodeId;
+
+/// [`routing::Score`] implementation that provides reasonable default behavior.
+///
+/// Used to apply a fixed penalty to each channel, thus avoiding long paths when shorter paths with
+/// slightly higher fees are available.
+///
+/// See [module-level documentation] for usage.
+///
+/// [module-level documentation]: crate::routing::scorer
+pub struct Scorer {
+       base_penalty_msat: u64,
+}
+
+impl Scorer {
+       /// Creates a new scorer using `base_penalty_msat` as the channel penalty.
+       pub fn new(base_penalty_msat: u64) -> Self {
+               Self { base_penalty_msat }
+       }
+}
+
+impl Default for Scorer {
+       /// Creates a new scorer using 500 msat as the channel penalty.
+       fn default() -> Self {
+               Scorer::new(500)
+       }
+}
+
+impl routing::Score for Scorer {
+       fn channel_penalty_msat(
+               &self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId
+       ) -> u64 {
+               self.base_penalty_msat
+       }
+}
index 9f7f4b4e5e00130a3581c5dd397497b3be0ba8ff..7ea369f551408bed9533c520a6adc8232ebf7b44 100644 (file)
@@ -146,7 +146,10 @@ pub enum Event {
                channel_value_satoshis: u64,
                /// The script which should be used in the transaction output.
                output_script: Script,
-               /// The value passed in to ChannelManager::create_channel
+               /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or 0 for
+               /// an inbound channel.
+               ///
+               /// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
                user_channel_id: u64,
        },
        /// Indicates we've received money! Just gotta dig out that payment preimage and feed it to
@@ -262,6 +265,12 @@ pub enum Event {
                /// The channel_id of the channel which has been closed. Note that on-chain transactions
                /// resolving the channel are likely still awaiting confirmation.
                channel_id: [u8; 32],
+               /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or 0 for
+               /// an inbound channel. This will always be zero for objects serialized with LDK versions
+               /// prior to 0.0.102.
+               ///
+               /// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
+               user_channel_id: u64,
                /// The reason the channel was closed.
                reason: ClosureReason
        },
@@ -352,10 +361,11 @@ impl Writeable for Event {
                                        (2, claim_from_onchain_tx, required),
                                });
                        },
-                       &Event::ChannelClosed { ref channel_id, ref reason } => {
+                       &Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason } => {
                                9u8.write(writer)?;
                                write_tlv_fields!(writer, {
                                        (0, channel_id, required),
+                                       (1, user_channel_id, required),
                                        (2, reason, required)
                                });
                        },
@@ -492,12 +502,15 @@ impl MaybeReadable for Event {
                                let f = || {
                                        let mut channel_id = [0; 32];
                                        let mut reason = None;
+                                       let mut user_channel_id_opt = None;
                                        read_tlv_fields!(reader, {
                                                (0, channel_id, required),
+                                               (1, user_channel_id_opt, option),
                                                (2, reason, ignorable),
                                        });
                                        if reason.is_none() { return Ok(None); }
-                                       Ok(Some(Event::ChannelClosed { channel_id, reason: reason.unwrap() }))
+                                       let user_channel_id = if let Some(id) = user_channel_id_opt { id } else { 0 };
+                                       Ok(Some(Event::ChannelClosed { channel_id, user_channel_id, reason: reason.unwrap() }))
                                };
                                f()
                        },