Merge pull request #1832 from jkczyz/2022-11-composite-handler
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 15 Feb 2023 17:45:39 +0000 (17:45 +0000)
committerGitHub <noreply@github.com>
Wed, 15 Feb 2023 17:45:39 +0000 (17:45 +0000)
Macro for composing custom message handlers

18 files changed:
SECURITY.md
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
lightning-invoice/src/utils.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/reload_tests.rs
lightning/src/ln/reorg_tests.rs
lightning/src/ln/shutdown_tests.rs
lightning/src/routing/gossip.rs
lightning/src/routing/router.rs
lightning/src/routing/scoring.rs
lightning/src/util/test_utils.rs

index d4b33ec3609019497ac93bfee2c6e6579f70644e..3f6b25fe8b993da805c3a8656527e74a50568fb8 100644 (file)
@@ -13,7 +13,7 @@ your own public key as an attachment or inline for replies.
 
  * 07DF3E57A548CCFB7530709189BBB8663E2E65CE (Matt Corallo)
  * 5DBC576CCCF546CA72AB06CE912EF12EA67705F5 (Jeffrey Czyz)
- * 729E9D9D92C75A5FBFEEE057B5DD717BEF7CA5B1 (Wilmer Paulino)
+ * 0A156842CF60B58BD826ABDD808FC696767C6147 (Wilmer Paulino)
  * BD6EED4D339EDBF7E7CE7F8836153082BDF676FD (Elias Rohrer)
  * 6E0287D8849AE741E47CC586FD3E106A2CE099B4 (Valentine Wallace)
  * 69CFEA635D0E6E6F13FD9D9136D932FCAC0305F0 (Arik Sosman)
index 0caee0801aa1d747ec6f564a2cc86f9658172f78..a088f64f4d1cbad35a7dfd8fb55146a6e07951eb 100644 (file)
@@ -97,10 +97,6 @@ impl Router for FuzzRouter {
                        action: msgs::ErrorAction::IgnoreError
                })
        }
-       fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
-       fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
-       fn notify_payment_probe_successful(&self, _path: &[&RouteHop]) {}
-       fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
 }
 
 pub struct TestBroadcaster {}
index 57c665a2ef1f51f97a31c0a69f00f3ec18d92ed5..9b3b76c2b3a7d4e60f9d7354406411b2b42a294f 100644 (file)
@@ -140,10 +140,6 @@ impl Router for FuzzRouter {
                        action: msgs::ErrorAction::IgnoreError
                })
        }
-       fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
-       fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
-       fn notify_payment_probe_successful(&self, _path: &[&RouteHop]) {}
-       fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
 }
 
 struct TestBroadcaster {
index 83f72e461455d3e3d06001121b7fa2bf8c6414f5..f6cc87fa72af2d0645b3b1538199b28f4265d5a3 100644 (file)
@@ -691,7 +691,7 @@ mod test {
                let first_hops = nodes[0].node.list_usable_channels();
                let network_graph = &node_cfgs[0].network_graph;
                let logger = test_utils::TestLogger::new();
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &nodes[0].node.get_our_node_id(), &route_params, &network_graph,
@@ -1055,7 +1055,7 @@ mod test {
                let first_hops = nodes[0].node.list_usable_channels();
                let network_graph = &node_cfgs[0].network_graph;
                let logger = test_utils::TestLogger::new();
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &nodes[0].node.get_our_node_id(), &params, &network_graph,
index 385f5b5353ca5f697880c441061de805cfbc2d28..e1ea256a661c663e5e00606517c441b56223fe61 100644 (file)
@@ -935,7 +935,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
 
        // Note that the ordering of the events for different nodes is non-prescriptive, though the
        // ordering of the two events that both go to nodes[2] have to stay in the same order.
-       let (nodes_0_event, events_3) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events_3);
+       let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events_3);
        let messages_a = match nodes_0_event {
                MessageSendEvent::UpdateHTLCs { node_id, mut updates } => {
                        assert_eq!(node_id, nodes[0].node.get_our_node_id());
@@ -949,12 +949,12 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
                _ => panic!("Unexpected event type!"),
        };
 
-       let (nodes_2_event, events_3) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events_3);
+       let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events_3);
        let send_event_b = SendEvent::from_event(nodes_2_event);
        assert_eq!(send_event_b.node_id, nodes[2].node.get_our_node_id());
 
        let raa = if test_ignore_second_cs {
-               let (nodes_2_event, _events_3) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events_3);
+               let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events_3);
                match nodes_2_event {
                        MessageSendEvent::SendRevokeAndACK { node_id, msg } => {
                                assert_eq!(node_id, nodes[2].node.get_our_node_id());
index 6ab5149f4c548267aad161d2dfc660c98078990b..a1148b692b102b15754386bfc620a116f4d5fb8d 100644 (file)
@@ -487,6 +487,22 @@ pub(super) struct PeerState<Signer: ChannelSigner> {
        /// Messages to send to the peer - pushed to in the same lock that they are generated in (except
        /// for broadcast messages, where ordering isn't as strict).
        pub(super) pending_msg_events: Vec<MessageSendEvent>,
+       /// The peer is currently connected (i.e. we've seen a
+       /// [`ChannelMessageHandler::peer_connected`] and no corresponding
+       /// [`ChannelMessageHandler::peer_disconnected`].
+       is_connected: bool,
+}
+
+impl <Signer: ChannelSigner> PeerState<Signer> {
+       /// Indicates that a peer meets the criteria where we're ok to remove it from our storage.
+       /// If true is passed for `require_disconnected`, the function will return false if we haven't
+       /// disconnected from the node already, ie. `PeerState::is_connected` is set to `true`.
+       fn ok_to_remove(&self, require_disconnected: bool) -> bool {
+               if require_disconnected && self.is_connected {
+                       return false
+               }
+               self.channel_by_id.len() == 0
+       }
 }
 
 /// Stores a PaymentSecret and any other data we may need to validate an inbound payment is
@@ -763,9 +779,8 @@ where
        /// very far in the past, and can only ever be up to two hours in the future.
        highest_seen_timestamp: AtomicUsize,
 
-       /// The bulk of our storage will eventually be here (message queues and the like). Currently
-       /// the `per_peer_state` stores our channels on a per-peer basis, as well as the peer's latest
-       /// features.
+       /// The bulk of our storage. Currently the `per_peer_state` stores our channels on a per-peer
+       /// basis, as well as the peer's latest features.
        ///
        /// If we are connected to a peer we always at least have an entry here, even if no channels
        /// are currently open with that peer.
@@ -1243,26 +1258,6 @@ macro_rules! handle_error {
                                                let mut peer_state = peer_state_mutex.lock().unwrap();
                                                peer_state.pending_msg_events.append(&mut msg_events);
                                        }
-                                       #[cfg(any(feature = "_test_utils", test))]
-                                       {
-                                               if let None = per_peer_state.get(&$counterparty_node_id) {
-                                                       // This shouldn't occour in tests unless an unkown counterparty_node_id
-                                                       // has been passed to our message handling functions.
-                                                       let expected_error_str = format!("Can't find a peer matching the passed counterparty node_id {}", $counterparty_node_id);
-                                                       match err.action {
-                                                               msgs::ErrorAction::SendErrorMessage {
-                                                                       msg: msgs::ErrorMessage { ref channel_id, ref data }
-                                                               }
-                                                               => {
-                                                                       assert_eq!(*data, expected_error_str);
-                                                                       if let Some((err_channel_id, _user_channel_id)) = chan_id {
-                                                                               debug_assert_eq!(*channel_id, err_channel_id);
-                                                                       }
-                                                               }
-                                                               _ => debug_assert!(false, "Unexpected event"),
-                                                       }
-                                               }
-                                       }
                                }
 
                                // Return error in case higher-API need one
@@ -1592,12 +1587,10 @@ where
 
                let per_peer_state = self.per_peer_state.read().unwrap();
 
-               let peer_state_mutex_opt = per_peer_state.get(&their_network_key);
-               if let None = peer_state_mutex_opt {
-                       return Err(APIError::APIMisuseError { err: format!("Not connected to node: {}", their_network_key) });
-               }
+               let peer_state_mutex = per_peer_state.get(&their_network_key)
+                       .ok_or_else(|| APIError::APIMisuseError{ err: format!("Not connected to node: {}", their_network_key) })?;
 
-               let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let mut peer_state = peer_state_mutex.lock().unwrap();
                let channel = {
                        let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
                        let their_features = &peer_state.latest_features;
@@ -1635,14 +1628,13 @@ where
        }
 
        fn list_channels_with_filter<Fn: FnMut(&(&[u8; 32], &Channel<<SP::Target as SignerProvider>::Signer>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
-               let mut res = Vec::new();
                // Allocate our best estimate of the number of channels we have in the `res`
                // Vec. Sadly the `short_to_chan_info` map doesn't cover channels without
                // a scid or a scid alias, and the `id_to_peer` shouldn't be used outside
                // of the ChannelMonitor handling. Therefore reallocations may still occur, but is
                // unlikely as the `short_to_chan_info` map often contains 2 entries for
                // the same channel.
-               res.reserve(self.short_to_chan_info.read().unwrap().len());
+               let mut res = Vec::with_capacity(self.short_to_chan_info.read().unwrap().len());
                {
                        let best_block_height = self.best_block.read().unwrap().height();
                        let per_peer_state = self.per_peer_state.read().unwrap();
@@ -1772,12 +1764,10 @@ where
                let result: Result<(), _> = loop {
                        let per_peer_state = self.per_peer_state.read().unwrap();
 
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(APIError::APIMisuseError { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) });
-                       }
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
 
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(channel_id.clone()) {
                                hash_map::Entry::Occupied(mut chan_entry) => {
@@ -1893,12 +1883,10 @@ where
        fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: &PublicKey, peer_msg: Option<&String>, broadcast: bool)
        -> Result<PublicKey, APIError> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(peer_node_id);
+               let peer_state_mutex = per_peer_state.get(peer_node_id)
+                       .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", peer_node_id) })?;
                let mut chan = {
-                       if let None = peer_state_mutex_opt {
-                               return Err(APIError::APIMisuseError{ err: format!("Can't find a peer matching the passed counterparty node_id {}", peer_node_id) });
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        if let hash_map::Entry::Occupied(chan) = peer_state.channel_by_id.entry(channel_id.clone()) {
                                if let Some(peer_msg) = peer_msg {
@@ -1914,7 +1902,7 @@ where
                log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..]));
                self.finish_force_close_channel(chan.force_shutdown(broadcast));
                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
-                       let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let mut peer_state = peer_state_mutex.lock().unwrap();
                        peer_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                msg: update
                        });
@@ -2201,7 +2189,7 @@ where
                                        let chan_update_opt = if let Some((counterparty_node_id, forwarding_id)) = forwarding_chan_info_opt {
                                                let per_peer_state = self.per_peer_state.read().unwrap();
                                                let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
-                                               if let None = peer_state_mutex_opt {
+                                               if peer_state_mutex_opt.is_none() {
                                                        break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
                                                }
                                                let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
@@ -2315,7 +2303,9 @@ where
        /// public, and thus should be called whenever the result is going to be passed out in a
        /// [`MessageSendEvent::BroadcastChannelUpdate`] event.
        ///
-       /// May be called with peer_state already locked!
+       /// Note that in `internal_closing_signed`, this function is called without the `peer_state`
+       /// corresponding to the channel's counterparty locked, as the channel been removed from the
+       /// storage and the `peer_state` lock has been dropped.
        fn get_channel_update_for_broadcast(&self, chan: &Channel<<SP::Target as SignerProvider>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
                if !chan.should_announce() {
                        return Err(LightningError {
@@ -2334,7 +2324,10 @@ where
        /// is public (only returning an Err if the channel does not yet have an assigned short_id),
        /// and thus MUST NOT be called unless the recipient of the resulting message has already
        /// provided evidence that they know about the existence of the channel.
-       /// May be called with peer_state already locked!
+       ///
+       /// Note that through `internal_closing_signed`, this function is called without the
+       /// `peer_state`  corresponding to the channel's counterparty locked, as the channel been
+       /// removed from the storage and the `peer_state` lock has been dropped.
        fn get_channel_update_for_unicast(&self, chan: &Channel<<SP::Target as SignerProvider>::Signer>) -> Result<msgs::ChannelUpdate, LightningError> {
                log_trace!(self.logger, "Attempting to generate channel update for channel {}", log_bytes!(chan.channel_id()));
                let short_channel_id = match chan.get_short_channel_id().or(chan.latest_inbound_scid_alias()) {
@@ -2395,11 +2388,9 @@ where
                        };
 
                        let per_peer_state = self.per_peer_state.read().unwrap();
-                       let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(APIError::InvalidRoute{err: "No peer matching the path's first hop found!" });
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let peer_state_mutex = per_peer_state.get(&counterparty_node_id)
+                               .ok_or_else(|| APIError::InvalidRoute{err: "No peer matching the path's first hop found!" })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        if let hash_map::Entry::Occupied(mut chan) = peer_state.channel_by_id.entry(id) {
                                match {
@@ -2546,7 +2537,7 @@ where
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments
                        .send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params,
-                               &self.router, self.list_usable_channels(), self.compute_inflight_htlcs(),
+                               &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
                                &self.entropy_source, &self.node_signer, best_block_height, &self.logger,
                                |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
                                self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
@@ -2646,7 +2637,7 @@ where
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
                        retry_strategy, route_params, &self.router, self.list_usable_channels(),
-                       self.compute_inflight_htlcs(),  &self.entropy_source, &self.node_signer, best_block_height,
+                       || self.compute_inflight_htlcs(),  &self.entropy_source, &self.node_signer, best_block_height,
                        &self.logger,
                        |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv|
                        self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv))
@@ -2675,12 +2666,10 @@ where
                &self, temporary_channel_id: &[u8; 32], counterparty_node_id: &PublicKey, funding_transaction: Transaction, find_funding_output: FundingOutput
        ) -> Result<(), APIError> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })
-               }
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
 
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                let (chan, msg) = {
                        let (res, chan) = {
@@ -2846,11 +2835,9 @@ where
                        &self.total_consistency_lock, &self.persistence_notifier,
                );
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(APIError::APIMisuseError{ err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) });
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                for channel_id in channel_ids {
                        if !peer_state.channel_by_id.contains_key(channel_id) {
@@ -2903,24 +2890,22 @@ where
 
                let next_hop_scid = {
                        let peer_state_lock = self.per_peer_state.read().unwrap();
-                       if let Some(peer_state_mutex) = peer_state_lock.get(&next_node_id) {
-                               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
-                               let peer_state = &mut *peer_state_lock;
-                               match peer_state.channel_by_id.get(next_hop_channel_id) {
-                                       Some(chan) => {
-                                               if !chan.is_usable() {
-                                                       return Err(APIError::ChannelUnavailable {
-                                                               err: format!("Channel with id {} not fully established", log_bytes!(*next_hop_channel_id))
-                                                       })
-                                               }
-                                               chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias())
-                                       },
-                                       None => return Err(APIError::ChannelUnavailable {
-                                               err: format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(*next_hop_channel_id), next_node_id)
-                                       })
-                               }
-                       } else {
-                               return Err(APIError::APIMisuseError{ err: format!("Can't find a peer matching the passed counterparty node_id {}", next_node_id) });
+                       let peer_state_mutex = peer_state_lock.get(&next_node_id)
+                               .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", next_node_id) })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
+                       let peer_state = &mut *peer_state_lock;
+                       match peer_state.channel_by_id.get(next_hop_channel_id) {
+                               Some(chan) => {
+                                       if !chan.is_usable() {
+                                               return Err(APIError::ChannelUnavailable {
+                                                       err: format!("Channel with id {} not fully established", log_bytes!(*next_hop_channel_id))
+                                               })
+                                       }
+                                       chan.get_short_channel_id().unwrap_or(chan.outbound_scid_alias())
+                               },
+                               None => return Err(APIError::ChannelUnavailable {
+                                       err: format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(*next_hop_channel_id), next_node_id)
+                               })
                        }
                };
 
@@ -3100,7 +3085,7 @@ where
                                        };
                                        let per_peer_state = self.per_peer_state.read().unwrap();
                                        let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
-                                       if let None = peer_state_mutex_opt {
+                                       if peer_state_mutex_opt.is_none() {
                                                forwarding_channel_not_found!();
                                                continue;
                                        }
@@ -3492,6 +3477,7 @@ where
        ///    the channel.
        ///  * Expiring a channel's previous `ChannelConfig` if necessary to only allow forwarding HTLCs
        ///    with the current `ChannelConfig`.
+       ///  * Removing peers which have disconnected but and no longer have any channels.
        ///
        /// Note that this may cause reentrancy through `chain::Watch::update_channel` calls or feerate
        /// estimate fetches.
@@ -3504,19 +3490,21 @@ where
 
                        let mut handle_errors: Vec<(Result<(), _>, _)> = Vec::new();
                        let mut timed_out_mpp_htlcs = Vec::new();
+                       let mut pending_peers_awaiting_removal = Vec::new();
                        {
                                let per_peer_state = self.per_peer_state.read().unwrap();
                                for (counterparty_node_id, peer_state_mutex) in per_peer_state.iter() {
                                        let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                                        let peer_state = &mut *peer_state_lock;
                                        let pending_msg_events = &mut peer_state.pending_msg_events;
+                                       let counterparty_node_id = *counterparty_node_id;
                                        peer_state.channel_by_id.retain(|chan_id, chan| {
                                                let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate);
                                                if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; }
 
                                                if let Err(e) = chan.timer_check_closing_negotiation_progress() {
                                                        let (needs_close, err) = convert_chan_err!(self, e, chan, chan_id);
-                                                       handle_errors.push((Err(err), *counterparty_node_id));
+                                                       handle_errors.push((Err(err), counterparty_node_id));
                                                        if needs_close { return false; }
                                                }
 
@@ -3550,6 +3538,36 @@ where
 
                                                true
                                        });
+                                       if peer_state.ok_to_remove(true) {
+                                               pending_peers_awaiting_removal.push(counterparty_node_id);
+                                       }
+                               }
+                       }
+
+                       // When a peer disconnects but still has channels, the peer's `peer_state` entry in the
+                       // `per_peer_state` is not removed by the `peer_disconnected` function. If the channels
+                       // of to that peer is later closed while still being disconnected (i.e. force closed),
+                       // we therefore need to remove the peer from `peer_state` separately.
+                       // To avoid having to take the `per_peer_state` `write` lock once the channels are
+                       // closed, we instead remove such peers awaiting removal here on a timer, to limit the
+                       // negative effects on parallelism as much as possible.
+                       if pending_peers_awaiting_removal.len() > 0 {
+                               let mut per_peer_state = self.per_peer_state.write().unwrap();
+                               for counterparty_node_id in pending_peers_awaiting_removal {
+                                       match per_peer_state.entry(counterparty_node_id) {
+                                               hash_map::Entry::Occupied(entry) => {
+                                                       // Remove the entry if the peer is still disconnected and we still
+                                                       // have no channels to the peer.
+                                                       let remove_entry = {
+                                                               let peer_state = entry.get().lock().unwrap();
+                                                               peer_state.ok_to_remove(true)
+                                                       };
+                                                       if remove_entry {
+                                                               entry.remove_entry();
+                                                       }
+                                               },
+                                               hash_map::Entry::Vacant(_) => { /* The PeerState has already been removed */ }
+                                       }
                                }
                        }
 
@@ -3727,7 +3745,7 @@ where
        fn fail_htlc_backwards_internal(&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, destination: HTLCDestination) {
                #[cfg(any(feature = "_test_utils", test))]
                {
-                       // Ensure that no peer state channel storage lock is not held when calling this
+                       // Ensure that the peer state channel storage lock is not held when calling this
                        // function.
                        // This ensures that future code doesn't introduce a lock_order requirement for
                        // `forward_htlcs` to be locked after the `per_peer_state` peer locks, which calling
@@ -3850,7 +3868,7 @@ where
                let mut expected_amt_msat = None;
                let mut valid_mpp = true;
                let mut errs = Vec::new();
-               let mut per_peer_state = Some(self.per_peer_state.read().unwrap());
+               let per_peer_state = self.per_peer_state.read().unwrap();
                for htlc in sources.iter() {
                        let (counterparty_node_id, chan_id) = match self.short_to_chan_info.read().unwrap().get(&htlc.prev_hop.short_channel_id) {
                                Some((cp_id, chan_id)) => (cp_id.clone(), chan_id.clone()),
@@ -3860,16 +3878,16 @@ where
                                }
                        };
 
-                       if let None = per_peer_state.as_ref().unwrap().get(&counterparty_node_id) {
+                       let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
+                       if peer_state_mutex_opt.is_none() {
                                valid_mpp = false;
                                break;
                        }
 
-                       let peer_state_mutex = per_peer_state.as_ref().unwrap().get(&counterparty_node_id).unwrap();
-                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
+                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
 
-                       if let None = peer_state.channel_by_id.get(&chan_id) {
+                       if peer_state.channel_by_id.get(&chan_id).is_none() {
                                valid_mpp = false;
                                break;
                        }
@@ -3895,14 +3913,13 @@ where
 
                        claimable_amt_msat += htlc.value;
                }
+               mem::drop(per_peer_state);
                if sources.is_empty() || expected_amt_msat.is_none() {
-                       mem::drop(per_peer_state);
                        self.claimable_payments.lock().unwrap().pending_claiming_payments.remove(&payment_hash);
                        log_info!(self.logger, "Attempted to claim an incomplete payment which no longer had any available HTLCs!");
                        return;
                }
                if claimable_amt_msat != expected_amt_msat.unwrap() {
-                       mem::drop(per_peer_state);
                        self.claimable_payments.lock().unwrap().pending_claiming_payments.remove(&payment_hash);
                        log_info!(self.logger, "Attempted to claim an incomplete payment, expected {} msat, had {} available to claim.",
                                expected_amt_msat.unwrap(), claimable_amt_msat);
@@ -3910,8 +3927,7 @@ where
                }
                if valid_mpp {
                        for htlc in sources.drain(..) {
-                               if per_peer_state.is_none() { per_peer_state = Some(self.per_peer_state.read().unwrap()); }
-                               if let Err((pk, err)) = self.claim_funds_from_hop(per_peer_state.take().unwrap(),
+                               if let Err((pk, err)) = self.claim_funds_from_hop(
                                        htlc.prev_hop, payment_preimage,
                                        |_| Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash }))
                                {
@@ -3923,7 +3939,6 @@ where
                                }
                        }
                }
-               mem::drop(per_peer_state);
                if !valid_mpp {
                        for htlc in sources.drain(..) {
                                let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec();
@@ -3944,11 +3959,11 @@ where
        }
 
        fn claim_funds_from_hop<ComplFunc: FnOnce(Option<u64>) -> Option<MonitorUpdateCompletionAction>>(&self,
-               per_peer_state_lock: RwLockReadGuard<HashMap<PublicKey, Mutex<PeerState<<SP::Target as SignerProvider>::Signer>>>>,
                prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage, completion_action: ComplFunc)
        -> Result<(), (PublicKey, MsgHandleErrInternal)> {
                //TODO: Delay the claimed_funds relaying just like we do outbound relay!
 
+               let per_peer_state = self.per_peer_state.read().unwrap();
                let chan_id = prev_hop.outpoint.to_channel_id();
 
                let counterparty_node_id_opt = match self.short_to_chan_info.read().unwrap().get(&prev_hop.short_channel_id) {
@@ -3956,83 +3971,76 @@ where
                        None => None
                };
 
-               let (found_channel, mut peer_state_opt) = if counterparty_node_id_opt.is_some() && per_peer_state_lock.get(&counterparty_node_id_opt.unwrap()).is_some() {
-                       let peer_mutex = per_peer_state_lock.get(&counterparty_node_id_opt.unwrap()).unwrap();
-                       let peer_state = peer_mutex.lock().unwrap();
-                       let found_channel = peer_state.channel_by_id.contains_key(&chan_id);
-                       (found_channel, Some(peer_state))
-               }  else { (false, None) };
-
-               if found_channel {
-                       let peer_state = &mut *peer_state_opt.as_mut().unwrap();
-                       if let hash_map::Entry::Occupied(mut chan) = peer_state.channel_by_id.entry(chan_id) {
-                               let counterparty_node_id = chan.get().get_counterparty_node_id();
-                               match chan.get_mut().get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, &self.logger) {
-                                       Ok(msgs_monitor_option) => {
-                                               if let UpdateFulfillCommitFetch::NewClaim { msgs, htlc_value_msat, monitor_update } = msgs_monitor_option {
-                                                       match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update) {
-                                                               ChannelMonitorUpdateStatus::Completed => {},
-                                                               e => {
-                                                                       log_given_level!(self.logger, if e == ChannelMonitorUpdateStatus::PermanentFailure { Level::Error } else { Level::Debug },
-                                                                               "Failed to update channel monitor with preimage {:?}: {:?}",
-                                                                               payment_preimage, e);
-                                                                       let err = handle_monitor_update_res!(self, e, chan, RAACommitmentOrder::CommitmentFirst, false, msgs.is_some()).unwrap_err();
-                                                                       mem::drop(peer_state_opt);
-                                                                       mem::drop(per_peer_state_lock);
-                                                                       self.handle_monitor_update_completion_actions(completion_action(Some(htlc_value_msat)));
-                                                                       return Err((counterparty_node_id, err));
-                                                               }
-                                                       }
-                                                       if let Some((msg, commitment_signed)) = msgs {
-                                                               log_debug!(self.logger, "Claiming funds for HTLC with preimage {} resulted in a commitment_signed for channel {}",
-                                                                       log_bytes!(payment_preimage.0), log_bytes!(chan.get().channel_id()));
-                                                               peer_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
-                                                                       node_id: counterparty_node_id,
-                                                                       updates: msgs::CommitmentUpdate {
-                                                                               update_add_htlcs: Vec::new(),
-                                                                               update_fulfill_htlcs: vec![msg],
-                                                                               update_fail_htlcs: Vec::new(),
-                                                                               update_fail_malformed_htlcs: Vec::new(),
-                                                                               update_fee: None,
-                                                                               commitment_signed,
-                                                                       }
-                                                               });
-                                                       }
-                                                       mem::drop(peer_state_opt);
-                                                       mem::drop(per_peer_state_lock);
-                                                       self.handle_monitor_update_completion_actions(completion_action(Some(htlc_value_msat)));
-                                                       Ok(())
-                                               } else {
-                                                       Ok(())
-                                               }
-                                       },
-                                       Err((e, monitor_update)) => {
+               let mut peer_state_opt = counterparty_node_id_opt.as_ref().map(
+                       |counterparty_node_id| per_peer_state.get(counterparty_node_id).map(
+                               |peer_mutex| peer_mutex.lock().unwrap()
+                       )
+               ).unwrap_or(None);
+
+               if let Some(hash_map::Entry::Occupied(mut chan)) = peer_state_opt.as_mut().map(|peer_state| peer_state.channel_by_id.entry(chan_id))
+               {
+                       let counterparty_node_id = chan.get().get_counterparty_node_id();
+                       match chan.get_mut().get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, &self.logger) {
+                               Ok(msgs_monitor_option) => {
+                                       if let UpdateFulfillCommitFetch::NewClaim { msgs, htlc_value_msat, monitor_update } = msgs_monitor_option {
                                                match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update) {
                                                        ChannelMonitorUpdateStatus::Completed => {},
                                                        e => {
-                                                               // TODO: This needs to be handled somehow - if we receive a monitor update
-                                                               // with a preimage we *must* somehow manage to propagate it to the upstream
-                                                               // channel, or we must have an ability to receive the same update and try
-                                                               // again on restart.
-                                                               log_given_level!(self.logger, if e == ChannelMonitorUpdateStatus::PermanentFailure { Level::Error } else { Level::Info },
-                                                                       "Failed to update channel monitor with preimage {:?} immediately prior to force-close: {:?}",
+                                                               log_given_level!(self.logger, if e == ChannelMonitorUpdateStatus::PermanentFailure { Level::Error } else { Level::Debug },
+                                                                       "Failed to update channel monitor with preimage {:?}: {:?}",
                                                                        payment_preimage, e);
-                                                       },
+                                                               let err = handle_monitor_update_res!(self, e, chan, RAACommitmentOrder::CommitmentFirst, false, msgs.is_some()).unwrap_err();
+                                                               mem::drop(peer_state_opt);
+                                                               mem::drop(per_peer_state);
+                                                               self.handle_monitor_update_completion_actions(completion_action(Some(htlc_value_msat)));
+                                                               return Err((counterparty_node_id, err));
+                                                       }
                                                }
-                                               let (drop, res) = convert_chan_err!(self, e, chan.get_mut(), &chan_id);
-                                               if drop {
-                                                       chan.remove_entry();
+                                               if let Some((msg, commitment_signed)) = msgs {
+                                                       log_debug!(self.logger, "Claiming funds for HTLC with preimage {} resulted in a commitment_signed for channel {}",
+                                                               log_bytes!(payment_preimage.0), log_bytes!(chan.get().channel_id()));
+                                                       peer_state_opt.as_mut().unwrap().pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
+                                                               node_id: counterparty_node_id,
+                                                               updates: msgs::CommitmentUpdate {
+                                                                       update_add_htlcs: Vec::new(),
+                                                                       update_fulfill_htlcs: vec![msg],
+                                                                       update_fail_htlcs: Vec::new(),
+                                                                       update_fail_malformed_htlcs: Vec::new(),
+                                                                       update_fee: None,
+                                                                       commitment_signed,
+                                                               }
+                                                       });
                                                }
                                                mem::drop(peer_state_opt);
-                                               mem::drop(per_peer_state_lock);
-                                               self.handle_monitor_update_completion_actions(completion_action(None));
-                                               Err((counterparty_node_id, res))
-                                       },
-                               }
-                       } else {
-                               // We've held the peer_state mutex since finding the channel and setting
-                               // found_channel to true, so the channel can't have been dropped.
-                               unreachable!()
+                                               mem::drop(per_peer_state);
+                                               self.handle_monitor_update_completion_actions(completion_action(Some(htlc_value_msat)));
+                                               Ok(())
+                                       } else {
+                                               Ok(())
+                                       }
+                               },
+                               Err((e, monitor_update)) => {
+                                       match self.chain_monitor.update_channel(chan.get().get_funding_txo().unwrap(), &monitor_update) {
+                                               ChannelMonitorUpdateStatus::Completed => {},
+                                               e => {
+                                                       // TODO: This needs to be handled somehow - if we receive a monitor update
+                                                       // with a preimage we *must* somehow manage to propagate it to the upstream
+                                                       // channel, or we must have an ability to receive the same update and try
+                                                       // again on restart.
+                                                       log_given_level!(self.logger, if e == ChannelMonitorUpdateStatus::PermanentFailure { Level::Error } else { Level::Info },
+                                                               "Failed to update channel monitor with preimage {:?} immediately prior to force-close: {:?}",
+                                                               payment_preimage, e);
+                                               },
+                                       }
+                                       let (drop, res) = convert_chan_err!(self, e, chan.get_mut(), &chan_id);
+                                       if drop {
+                                               chan.remove_entry();
+                                       }
+                                       mem::drop(peer_state_opt);
+                                       mem::drop(per_peer_state);
+                                       self.handle_monitor_update_completion_actions(completion_action(None));
+                                       Err((counterparty_node_id, res))
+                               },
                        }
                } else {
                        let preimage_update = ChannelMonitorUpdate {
@@ -4053,7 +4061,7 @@ where
                                        payment_preimage, update_res);
                        }
                        mem::drop(peer_state_opt);
-                       mem::drop(per_peer_state_lock);
+                       mem::drop(per_peer_state);
                        // Note that we do process the completion action here. This totally could be a
                        // duplicate claim, but we have no way of knowing without interrogating the
                        // `ChannelMonitor` we've provided the above update to. Instead, note that `Event`s are
@@ -4075,7 +4083,7 @@ where
                        },
                        HTLCSource::PreviousHopData(hop_data) => {
                                let prev_outpoint = hop_data.outpoint;
-                               let res = self.claim_funds_from_hop(self.per_peer_state.read().unwrap(), hop_data, payment_preimage,
+                               let res = self.claim_funds_from_hop(hop_data, payment_preimage,
                                        |htlc_claim_value_msat| {
                                                if let Some(forwarded_htlc_value) = forwarded_htlc_value_msat {
                                                        let fee_earned_msat = if let Some(claimed_htlc_value) = htlc_claim_value_msat {
@@ -4207,7 +4215,7 @@ where
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let mut peer_state_lock;
                        let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
-                       if let None = peer_state_mutex_opt { return }
+                       if peer_state_mutex_opt.is_none() { return }
                        peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        let mut channel = {
@@ -4297,11 +4305,9 @@ where
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(APIError::APIMisuseError { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) });
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(temporary_channel_id.clone()) {
                        hash_map::Entry::Occupied(mut channel) => {
@@ -4349,11 +4355,12 @@ where
 
                let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id.clone()))
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                   .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id.clone())
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                let mut channel = match Channel::new_from_req(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
                        counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features, msg, user_channel_id, &self.default_configuration,
@@ -4401,11 +4408,12 @@ where
        fn internal_accept_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
                let (value, output_script, user_id) = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id))
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| {
+                                       debug_assert!(false);
+                                       MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id)
+                               })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.temporary_channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
@@ -4428,13 +4436,14 @@ where
 
        fn internal_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id))
-               }
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id)
+                       })?;
                let ((funding_msg, monitor, mut channel_ready), mut chan) = {
                        let best_block = *self.best_block.read().unwrap();
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.temporary_channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
@@ -4471,7 +4480,7 @@ where
                // It's safe to unwrap as we've held the `per_peer_state` read lock since checking that the
                // peer exists, despite the inner PeerState potentially having no channels after removing
                // the channel above.
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(funding_msg.channel_id) {
                        hash_map::Entry::Occupied(_) => {
@@ -4506,12 +4515,13 @@ where
                let funding_tx = {
                        let best_block = *self.best_block.read().unwrap();
                        let per_peer_state = self.per_peer_state.read().unwrap();
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-                       }
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| {
+                                       debug_assert!(false);
+                                       MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                               })?;
 
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
@@ -4549,11 +4559,12 @@ where
 
        fn internal_channel_ready(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReady) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id));
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -4592,11 +4603,12 @@ where
                let mut dropped_htlcs: Vec<(HTLCSource, PaymentHash)>;
                let result: Result<(), _> = loop {
                        let per_peer_state = self.per_peer_state.read().unwrap();
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| {
+                                       debug_assert!(false);
+                                       MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                               })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id.clone()) {
                                hash_map::Entry::Occupied(mut chan_entry) => {
@@ -4645,12 +4657,13 @@ where
 
        fn internal_closing_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-               }
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
                let (tx, chan_option) = {
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id.clone()) {
                                hash_map::Entry::Occupied(mut chan_entry) => {
@@ -4679,7 +4692,7 @@ where
                }
                if let Some(chan) = chan_option {
                        if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
-                               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                                let peer_state = &mut *peer_state_lock;
                                peer_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                        msg: update
@@ -4702,11 +4715,12 @@ where
 
                let pending_forward_info = self.decode_update_add_htlc_onion(msg);
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -4743,11 +4757,12 @@ where
        fn internal_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), MsgHandleErrInternal> {
                let (htlc_source, forwarded_htlc_value) = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id));
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| {
+                                       debug_assert!(false);
+                                       MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                               })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
@@ -4762,11 +4777,12 @@ where
 
        fn internal_update_fail_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id));
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -4779,11 +4795,12 @@ where
 
        fn internal_update_fail_malformed_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFailMalformedHTLC) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -4800,11 +4817,12 @@ where
 
        fn internal_commitment_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -4943,11 +4961,12 @@ where
                let mut htlcs_to_fail = Vec::new();
                let res = loop {
                        let per_peer_state = self.per_peer_state.read().unwrap();
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               break Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id))
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| {
+                                       debug_assert!(false);
+                                       MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                               })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
@@ -5008,11 +5027,12 @@ where
 
        fn internal_update_fee(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFee) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id));
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -5025,11 +5045,12 @@ where
 
        fn internal_announcement_signatures(&self, counterparty_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) -> Result<(), MsgHandleErrInternal> {
                let per_peer_state = self.per_peer_state.read().unwrap();
-               let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-               if let None = peer_state_mutex_opt {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id));
-               }
-               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+               let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                       .ok_or_else(|| {
+                               debug_assert!(false);
+                               MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                       })?;
+               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
@@ -5063,7 +5084,7 @@ where
                };
                let per_peer_state = self.per_peer_state.read().unwrap();
                let peer_state_mutex_opt = per_peer_state.get(&chan_counterparty_node_id);
-               if let None = peer_state_mutex_opt {
+               if peer_state_mutex_opt.is_none() {
                        return Ok(NotifyOption::SkipPersist)
                }
                let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
@@ -5098,11 +5119,12 @@ where
                let need_lnd_workaround = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
 
-                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                       if let None = peer_state_mutex_opt {
-                               return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id));
-                       }
-                       let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                       let peer_state_mutex = per_peer_state.get(counterparty_node_id)
+                               .ok_or_else(|| {
+                                       debug_assert!(false);
+                                       MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id)
+                               })?;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        match peer_state.channel_by_id.entry(msg.channel_id) {
                                hash_map::Entry::Occupied(mut chan) => {
@@ -5720,9 +5742,7 @@ where
                                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                                let peer_state = &mut *peer_state_lock;
                                if peer_state.pending_msg_events.len() > 0 {
-                                       let mut peer_pending_events = Vec::new();
-                                       mem::swap(&mut peer_pending_events, &mut peer_state.pending_msg_events);
-                                       pending_events.append(&mut peer_pending_events);
+                                       pending_events.append(&mut peer_state.pending_msg_events);
                                }
                        }
 
@@ -6253,9 +6273,8 @@ where
        fn peer_disconnected(&self, counterparty_node_id: &PublicKey, no_connection_possible: bool) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
                let mut failed_channels = Vec::new();
-               let mut no_channels_remain = true;
                let mut per_peer_state = self.per_peer_state.write().unwrap();
-               {
+               let remove_peer = {
                        log_debug!(self.logger, "Marking channels with {} disconnected and generating channel_updates. We believe we {} make future connections to this peer.",
                                log_pubkey!(counterparty_node_id), if no_connection_possible { "cannot" } else { "can" });
                        if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
@@ -6268,8 +6287,6 @@ where
                                                update_maps_on_chan_removal!(self, chan);
                                                self.issue_channel_close_events(chan, ClosureReason::DisconnectedPeer);
                                                return false;
-                                       } else {
-                                               no_channels_remain = false;
                                        }
                                        true
                                });
@@ -6298,9 +6315,12 @@ where
                                                &events::MessageSendEvent::SendGossipTimestampFilter { .. } => false,
                                        }
                                });
-                       }
-               }
-               if no_channels_remain {
+                               debug_assert!(peer_state.is_connected, "A disconnected peer cannot disconnect");
+                               peer_state.is_connected = false;
+                               peer_state.ok_to_remove(true)
+                       } else { true }
+               };
+               if remove_peer {
                        per_peer_state.remove(counterparty_node_id);
                }
                mem::drop(per_peer_state);
@@ -6328,10 +6348,14 @@ where
                                                channel_by_id: HashMap::new(),
                                                latest_features: init_msg.features.clone(),
                                                pending_msg_events: Vec::new(),
+                                               is_connected: true,
                                        }));
                                },
                                hash_map::Entry::Occupied(e) => {
-                                       e.get().lock().unwrap().latest_features = init_msg.features.clone();
+                                       let mut peer_state = e.get().lock().unwrap();
+                                       peer_state.latest_features = init_msg.features.clone();
+                                       debug_assert!(!peer_state.is_connected, "A peer shouldn't be connected twice");
+                                       peer_state.is_connected = true;
                                },
                        }
                }
@@ -6382,7 +6406,7 @@ where
                        let channel_ids: Vec<[u8; 32]> = {
                                let per_peer_state = self.per_peer_state.read().unwrap();
                                let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                               if let None = peer_state_mutex_opt { return; }
+                               if peer_state_mutex_opt.is_none() { return; }
                                let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
                                let peer_state = &mut *peer_state_lock;
                                peer_state.channel_by_id.keys().cloned().collect()
@@ -6396,7 +6420,7 @@ where
                                // First check if we can advance the channel type and try again.
                                let per_peer_state = self.per_peer_state.read().unwrap();
                                let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                               if let None = peer_state_mutex_opt { return; }
+                               if peer_state_mutex_opt.is_none() { return; }
                                let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
                                let peer_state = &mut *peer_state_lock;
                                if let Some(chan) = peer_state.channel_by_id.get_mut(&msg.channel_id) {
@@ -6890,6 +6914,7 @@ where
                        best_block.block_hash().write(writer)?;
                }
 
+               let mut serializable_peer_count: u64 = 0;
                {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let mut unfunded_channels = 0;
@@ -6897,6 +6922,9 @@ where
                        for (_, peer_state_mutex) in per_peer_state.iter() {
                                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                                let peer_state = &mut *peer_state_lock;
+                               if !peer_state.ok_to_remove(false) {
+                                       serializable_peer_count += 1;
+                               }
                                number_of_channels += peer_state.channel_by_id.len();
                                for (_, channel) in peer_state.channel_by_id.iter() {
                                        if !channel.is_funding_initiated() {
@@ -6947,11 +6975,18 @@ where
                        htlc_purposes.push(purpose);
                }
 
-               (per_peer_state.len() as u64).write(writer)?;
+               (serializable_peer_count).write(writer)?;
                for (peer_pubkey, peer_state_mutex) in per_peer_state.iter() {
-                       peer_pubkey.write(writer)?;
-                       let peer_state = peer_state_mutex.lock().unwrap();
-                       peer_state.latest_features.write(writer)?;
+                       let peer_state_lock = peer_state_mutex.lock().unwrap();
+                       let peer_state = &*peer_state_lock;
+                       // Peers which we have no channels to should be dropped once disconnected. As we
+                       // disconnect all peers when shutting down and serializing the ChannelManager, we
+                       // consider all peers as disconnected here. There's therefore no need write peers with
+                       // no channels.
+                       if !peer_state.ok_to_remove(false) {
+                               peer_pubkey.write(writer)?;
+                               peer_state.latest_features.write(writer)?;
+                       }
                }
 
                let events = self.pending_events.lock().unwrap();
@@ -7350,6 +7385,7 @@ where
                                channel_by_id: peer_channels.remove(&peer_pubkey).unwrap_or(HashMap::new()),
                                latest_features: Readable::read(reader)?,
                                pending_msg_events: Vec::new(),
+                               is_connected: false,
                        };
                        per_peer_state.insert(peer_pubkey, Mutex::new(peer_state));
                }
@@ -7737,19 +7773,14 @@ where
 mod tests {
        use bitcoin::hashes::Hash;
        use bitcoin::hashes::sha256::Hash as Sha256;
-       use bitcoin::hashes::hex::FromHex;
        use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
-       use bitcoin::secp256k1::ecdsa::Signature;
-       use bitcoin::secp256k1::ffi::Signature as FFISignature;
-       use bitcoin::blockdata::script::Script;
-       use bitcoin::Txid;
        use core::time::Duration;
        use core::sync::atomic::Ordering;
        use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
        use crate::ln::channelmanager::{inbound_payment, PaymentId, PaymentSendFailure, InterceptId};
        use crate::ln::functional_test_utils::*;
        use crate::ln::msgs;
-       use crate::ln::msgs::{ChannelMessageHandler, OptionalField};
+       use crate::ln::msgs::ChannelMessageHandler;
        use crate::routing::router::{PaymentParameters, RouteParameters, find_route};
        use crate::util::errors::APIError;
        use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
@@ -7966,7 +7997,7 @@ mod tests {
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1);
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 
                // To start (1), send a regular payment but don't claim it.
@@ -8063,8 +8094,6 @@ mod tests {
 
                let payer_pubkey = nodes[0].node.get_our_node_id();
                let payee_pubkey = nodes[1].node.get_our_node_id();
-               nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }).unwrap();
-               nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }).unwrap();
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
                let route_params = RouteParameters {
@@ -8074,7 +8103,7 @@ mod tests {
                };
                let network_graph = nodes[0].network_graph.clone();
                let first_hops = nodes[0].node.list_usable_channels();
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -8108,8 +8137,6 @@ mod tests {
 
                let payer_pubkey = nodes[0].node.get_our_node_id();
                let payee_pubkey = nodes[1].node.get_our_node_id();
-               nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }).unwrap();
-               nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }).unwrap();
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
                let route_params = RouteParameters {
@@ -8119,7 +8146,7 @@ mod tests {
                };
                let network_graph = nodes[0].network_graph.clone();
                let first_hops = nodes[0].node.list_usable_channels();
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -8174,6 +8201,40 @@ mod tests {
                }
        }
 
+       #[test]
+       fn test_drop_disconnected_peers_when_removing_channels() {
+               let chanmon_cfgs = create_chanmon_cfgs(2);
+               let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+               let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+               let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
+
+               nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+               nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
+
+               nodes[0].node.force_close_broadcasting_latest_txn(&chan.2, &nodes[1].node.get_our_node_id()).unwrap();
+               check_closed_broadcast!(nodes[0], true);
+               check_added_monitors!(nodes[0], 1);
+               check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
+
+               {
+                       // Assert that nodes[1] is awaiting removal for nodes[0] once nodes[1] has been
+                       // disconnected and the channel between has been force closed.
+                       let nodes_0_per_peer_state = nodes[0].node.per_peer_state.read().unwrap();
+                       // Assert that nodes[1] isn't removed before `timer_tick_occurred` has been executed.
+                       assert_eq!(nodes_0_per_peer_state.len(), 1);
+                       assert!(nodes_0_per_peer_state.get(&nodes[1].node.get_our_node_id()).is_some());
+               }
+
+               nodes[0].node.timer_tick_occurred();
+
+               {
+                       // Assert that nodes[1] has now been removed.
+                       assert_eq!(nodes[0].node.per_peer_state.read().unwrap().len(), 0);
+               }
+       }
+
        #[test]
        fn bad_inbound_payment_hash() {
                // Add coverage for checking that a user-provided payment hash matches the payment secret.
@@ -8318,19 +8379,22 @@ mod tests {
 
        fn check_not_connected_to_peer_error<T>(res_err: Result<T, APIError>, expected_public_key: PublicKey) {
                let expected_message = format!("Not connected to node: {}", expected_public_key);
-               check_api_misuse_error_message(expected_message, res_err)
+               check_api_error_message(expected_message, res_err)
        }
 
        fn check_unkown_peer_error<T>(res_err: Result<T, APIError>, expected_public_key: PublicKey) {
                let expected_message = format!("Can't find a peer matching the passed counterparty node_id {}", expected_public_key);
-               check_api_misuse_error_message(expected_message, res_err)
+               check_api_error_message(expected_message, res_err)
        }
 
-       fn check_api_misuse_error_message<T>(expected_err_message: String, res_err: Result<T, APIError>) {
+       fn check_api_error_message<T>(expected_err_message: String, res_err: Result<T, APIError>) {
                match res_err {
                        Err(APIError::APIMisuseError { err }) => {
                                assert_eq!(err, expected_err_message);
                        },
+                       Err(APIError::ChannelUnavailable { err }) => {
+                               assert_eq!(err, expected_err_message);
+                       },
                        Ok(_) => panic!("Unexpected Ok"),
                        Err(_) => panic!("Unexpected Error"),
                }
@@ -8338,140 +8402,23 @@ mod tests {
 
        #[test]
        fn test_api_calls_with_unkown_counterparty_node() {
-               // Tests that our API functions and message handlers that expects a `counterparty_node_id`
-               // as input, behaves as expected if the `counterparty_node_id` is an unkown peer in the
+               // Tests that our API functions that expects a `counterparty_node_id` as input, behaves as
+               // expected if the `counterparty_node_id` is an unkown peer in the
                // `ChannelManager::per_peer_state` map.
                let chanmon_cfg = create_chanmon_cfgs(2);
                let node_cfg = create_node_cfgs(2, &chanmon_cfg);
                let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[None, None]);
                let nodes = create_network(2, &node_cfg, &node_chanmgr);
 
-               // Boilerplate code to produce `open_channel` and `accept_channel` msgs more densly than
-               // creating dummy ones.
-               nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 500_000_000, 42, None).unwrap();
-               let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
-               nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
-               let accept_channel_msg = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
-
                // Dummy values
                let channel_id = [4; 32];
-               let signature = Signature::from(unsafe { FFISignature::new() });
                let unkown_public_key = PublicKey::from_secret_key(&Secp256k1::signing_only(), &SecretKey::from_slice(&[42; 32]).unwrap());
                let intercept_id = InterceptId([0; 32]);
 
-               // Dummy msgs
-               let funding_created_msg = msgs::FundingCreated {
-                       temporary_channel_id: open_channel_msg.temporary_channel_id,
-                       funding_txid: Txid::from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(),
-                       funding_output_index: 0,
-                       signature: signature,
-               };
-
-               let funding_signed_msg = msgs::FundingSigned {
-                       channel_id: channel_id,
-                       signature: signature,
-               };
-
-               let channel_ready_msg = msgs::ChannelReady {
-                       channel_id: channel_id,
-                       next_per_commitment_point: unkown_public_key,
-                       short_channel_id_alias: None,
-               };
-
-               let announcement_signatures_msg = msgs::AnnouncementSignatures {
-                       channel_id: channel_id,
-                       short_channel_id: 0,
-                       node_signature: signature,
-                       bitcoin_signature: signature,
-               };
-
-               let channel_reestablish_msg = msgs::ChannelReestablish {
-                       channel_id: channel_id,
-                       next_local_commitment_number: 0,
-                       next_remote_commitment_number: 0,
-                       data_loss_protect: OptionalField::Absent,
-               };
-
-               let closing_signed_msg = msgs::ClosingSigned {
-                       channel_id: channel_id,
-                       fee_satoshis: 1000,
-                       signature: signature,
-                       fee_range: None,
-               };
-
-               let shutdown_msg = msgs::Shutdown {
-                       channel_id: channel_id,
-                       scriptpubkey: Script::new(),
-               };
-
-               let onion_routing_packet = msgs::OnionPacket {
-                       version: 255,
-                       public_key: Ok(unkown_public_key),
-                       hop_data: [1; 20*65],
-                       hmac: [2; 32]
-               };
-
-               let update_add_htlc_msg = msgs::UpdateAddHTLC {
-                       channel_id: channel_id,
-                       htlc_id: 0,
-                       amount_msat: 1000000,
-                       payment_hash: PaymentHash([1; 32]),
-                       cltv_expiry: 821716,
-                       onion_routing_packet
-               };
-
-               let commitment_signed_msg = msgs::CommitmentSigned {
-                       channel_id: channel_id,
-                       signature: signature,
-                       htlc_signatures: Vec::new(),
-               };
-
-               let update_fee_msg = msgs::UpdateFee {
-                       channel_id: channel_id,
-                       feerate_per_kw: 1000,
-               };
-
-               let malformed_update_msg = msgs::UpdateFailMalformedHTLC{
-                       channel_id: channel_id,
-                       htlc_id: 0,
-                       sha256_of_onion: [1; 32],
-                       failure_code: 0x8000,
-               };
-
-               let fulfill_update_msg = msgs::UpdateFulfillHTLC{
-                       channel_id: channel_id,
-                       htlc_id: 0,
-                       payment_preimage: PaymentPreimage([1; 32]),
-               };
-
-               let fail_update_msg = msgs::UpdateFailHTLC{
-                       channel_id: channel_id,
-                       htlc_id: 0,
-                       reason: msgs::OnionErrorPacket { data: Vec::new()},
-               };
-
-               let revoke_and_ack_msg = msgs::RevokeAndACK {
-                       channel_id: channel_id,
-                       per_commitment_secret: [1; 32],
-                       next_per_commitment_point: unkown_public_key,
-               };
-
-               // Test the API functions and message handlers.
+               // Test the API functions.
                check_not_connected_to_peer_error(nodes[0].node.create_channel(unkown_public_key, 1_000_000, 500_000_000, 42, None), unkown_public_key);
 
-               nodes[1].node.handle_open_channel(&unkown_public_key, &open_channel_msg);
-
-               nodes[0].node.handle_accept_channel(&unkown_public_key, &accept_channel_msg);
-
-               check_unkown_peer_error(nodes[0].node.accept_inbound_channel(&open_channel_msg.temporary_channel_id, &unkown_public_key, 42), unkown_public_key);
-
-               nodes[1].node.handle_funding_created(&unkown_public_key, &funding_created_msg);
-
-               nodes[0].node.handle_funding_signed(&unkown_public_key, &funding_signed_msg);
-
-               nodes[0].node.handle_channel_ready(&unkown_public_key, &channel_ready_msg);
-
-               nodes[1].node.handle_announcement_signatures(&unkown_public_key, &announcement_signatures_msg);
+               check_unkown_peer_error(nodes[0].node.accept_inbound_channel(&channel_id, &unkown_public_key, 42), unkown_public_key);
 
                check_unkown_peer_error(nodes[0].node.close_channel(&channel_id, &unkown_public_key), unkown_public_key);
 
@@ -8482,26 +8429,6 @@ mod tests {
                check_unkown_peer_error(nodes[0].node.forward_intercepted_htlc(intercept_id, &channel_id, unkown_public_key, 1_000_000), unkown_public_key);
 
                check_unkown_peer_error(nodes[0].node.update_channel_config(&unkown_public_key, &[channel_id], &ChannelConfig::default()), unkown_public_key);
-
-               nodes[0].node.handle_shutdown(&unkown_public_key, &shutdown_msg);
-
-               nodes[1].node.handle_closing_signed(&unkown_public_key, &closing_signed_msg);
-
-               nodes[0].node.handle_channel_reestablish(&unkown_public_key, &channel_reestablish_msg);
-
-               nodes[1].node.handle_update_add_htlc(&unkown_public_key, &update_add_htlc_msg);
-
-               nodes[1].node.handle_commitment_signed(&unkown_public_key, &commitment_signed_msg);
-
-               nodes[1].node.handle_update_fail_malformed_htlc(&unkown_public_key, &malformed_update_msg);
-
-               nodes[1].node.handle_update_fail_htlc(&unkown_public_key, &fail_update_msg);
-
-               nodes[1].node.handle_update_fulfill_htlc(&unkown_public_key, &fulfill_update_msg);
-
-               nodes[1].node.handle_revoke_and_ack(&unkown_public_key, &revoke_and_ack_msg);
-
-               nodes[1].node.handle_update_fee(&unkown_public_key, &update_fee_msg);
        }
 
        #[cfg(anchors)]
@@ -8589,7 +8516,8 @@ pub mod bench {
                let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
                let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
                let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
-               let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)));
+               let scorer = Mutex::new(test_utils::TestScorer::new());
+               let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)), &scorer);
 
                let mut config: UserConfig = Default::default();
                config.channel_handshake_config.minimum_depth = 1;
@@ -8680,7 +8608,7 @@ pub mod bench {
                                let usable_channels = $node_a.list_usable_channels();
                                let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id(), TEST_FINAL_CLTV)
                                        .with_features($node_b.invoice_features());
-                               let scorer = test_utils::TestScorer::with_penalty(0);
+                               let scorer = test_utils::TestScorer::new();
                                let seed = [3u8; 32];
                                let keys_manager = KeysManager::new(&seed, 42, 42);
                                let random_seed_bytes = keys_manager.get_secure_random_bytes();
index 7bf51df5c7a28bb553dd67fd44de672e0f98142d..9f888848fe91db7d2e158ca1bdf922af2dfa729d 100644 (file)
@@ -305,6 +305,7 @@ pub struct TestChanMonCfg {
        pub persister: test_utils::TestPersister,
        pub logger: test_utils::TestLogger,
        pub keys_manager: test_utils::TestKeysInterface,
+       pub scorer: Mutex<test_utils::TestScorer>,
 }
 
 pub struct NodeCfg<'a> {
@@ -427,6 +428,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
                                        channel_monitors.insert(monitor.get_funding_txo().0, monitor);
                                }
 
+                               let scorer = Mutex::new(test_utils::TestScorer::new());
                                let mut w = test_utils::TestVecWriter(Vec::new());
                                self.node.write(&mut w).unwrap();
                                <(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>)>::read(&mut io::Cursor::new(w.0), ChannelManagerReadArgs {
@@ -435,7 +437,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
                                        node_signer: self.keys_manager,
                                        signer_provider: self.keys_manager,
                                        fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
-                                       router: &test_utils::TestRouter::new(Arc::new(network_graph)),
+                                       router: &test_utils::TestRouter::new(Arc::new(network_graph), &scorer),
                                        chain_monitor: self.chain_monitor,
                                        tx_broadcaster: &broadcaster,
                                        logger: &self.logger,
@@ -572,12 +574,12 @@ macro_rules! get_htlc_update_msgs {
 }
 
 /// Fetches the first `msg_event` to the passed `node_id` in the passed `msg_events` vec.
-/// Returns the `msg_event`, along with an updated `msg_events` vec with the message removed.
+/// Returns the `msg_event`.
 ///
 /// Note that even though `BroadcastChannelAnnouncement` and `BroadcastChannelUpdate`
 /// `msg_events` are stored under specific peers, this function does not fetch such `msg_events` as
 /// such messages are intended to all peers.
-pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &Vec<MessageSendEvent>) -> (MessageSendEvent, Vec<MessageSendEvent>) {
+pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &mut Vec<MessageSendEvent>) -> MessageSendEvent {
        let ev_index = msg_events.iter().position(|e| { match e {
                MessageSendEvent::SendAcceptChannel { node_id, .. } => {
                        node_id == msg_node_id
@@ -644,9 +646,7 @@ pub fn remove_first_msg_event_to_node(msg_node_id: &PublicKey, msg_events: &Vec<
                },
        }});
        if ev_index.is_some() {
-               let mut updated_msg_events = msg_events.to_vec();
-               let ev = updated_msg_events.remove(ev_index.unwrap());
-               (ev, updated_msg_events)
+               msg_events.remove(ev_index.unwrap())
        } else {
                panic!("Couldn't find any MessageSendEvent to the node!")
        }
@@ -1486,9 +1486,9 @@ pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<
        check_added_monitors!(node_b, 1);
        node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &as_commitment_signed);
        let (bs_revoke_and_ack, extra_msg_option) = {
-               let events = node_b.node.get_and_clear_pending_msg_events();
+               let mut events = node_b.node.get_and_clear_pending_msg_events();
                assert!(events.len() <= 2);
-               let (node_a_event, events) = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &events);
+               let node_a_event = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &mut events);
                (match node_a_event {
                        MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
                                assert_eq!(*node_id, node_a.node.get_our_node_id());
@@ -1570,7 +1570,7 @@ macro_rules! get_payment_preimage_hash {
 macro_rules! get_route {
        ($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
                use $crate::chain::keysinterface::EntropySource;
-               let scorer = $crate::util::test_utils::TestScorer::with_penalty(0);
+               let scorer = $crate::util::test_utils::TestScorer::new();
                let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                $crate::routing::router::get_route(
@@ -1943,8 +1943,7 @@ pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
        let mut events = origin_node.node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), expected_route.len());
        for (path_idx, expected_path) in expected_route.iter().enumerate() {
-               let (ev, updated_events) = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &events);
-               events = updated_events;
+               let ev = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &mut events);
                // Once we've gotten through all the HTLCs, the last one should result in a
                // PaymentClaimable (but each previous one should not!), .
                let expect_payment = path_idx == expected_route.len() - 1;
@@ -2003,9 +2002,8 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>,
        } else {
                for expected_path in expected_paths.iter() {
                        // For MPP payments, we always want the message to the first node in the path.
-                       let (ev, updated_events) = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &events);
+                       let ev = remove_first_msg_event_to_node(&expected_path[0].node.get_our_node_id(), &mut events);
                        per_path_msgs.push(msgs_from_ev!(&ev));
-                       events = updated_events;
                }
        }
 
@@ -2124,7 +2122,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
        let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
                .with_features(expected_route.last().unwrap().node.invoice_features());
        let network_graph = origin_node.network_graph.read_only();
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let seed = [0u8; 32];
        let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
        let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2289,8 +2287,9 @@ pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
                let persister = test_utils::TestPersister::new();
                let seed = [i as u8; 32];
                let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
+               let scorer = Mutex::new(test_utils::TestScorer::new());
 
-               chan_mon_cfgs.push(TestChanMonCfg { tx_broadcaster, fee_estimator, chain_source, logger, persister, keys_manager });
+               chan_mon_cfgs.push(TestChanMonCfg { tx_broadcaster, fee_estimator, chain_source, logger, persister, keys_manager, scorer });
        }
 
        chan_mon_cfgs
@@ -2308,7 +2307,7 @@ pub fn create_node_cfgs<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMon
                        logger: &chanmon_cfgs[i].logger,
                        tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster,
                        fee_estimator: &chanmon_cfgs[i].fee_estimator,
-                       router: test_utils::TestRouter::new(network_graph.clone()),
+                       router: test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[i].scorer),
                        chain_monitor,
                        keys_manager: &chanmon_cfgs[i].keys_manager,
                        node_seed: seed,
index bf0c3396cedb81e746d8bcdbc648ccd9139a218a..9055d4e76fe58fb8ecbccfa386f75bb604298430 100644 (file)
@@ -2747,7 +2747,7 @@ fn test_htlc_on_chain_success() {
                },
                _ => panic!()
        }
-       let events = nodes[1].node.get_and_clear_pending_msg_events();
+       let mut events = nodes[1].node.get_and_clear_pending_msg_events();
        {
                let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap();
                assert_eq!(added_monitors.len(), 2);
@@ -2757,8 +2757,8 @@ fn test_htlc_on_chain_success() {
        }
        assert_eq!(events.len(), 3);
 
-       let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
-       let (nodes_0_event, events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events);
+       let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
+       let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events);
 
        match nodes_2_event {
                MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
@@ -3196,14 +3196,15 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                        _ => panic!("Unexpected event"),
                }
        }
+
        nodes[1].node.process_pending_htlc_forwards();
        check_added_monitors!(nodes[1], 1);
 
        let mut events = nodes[1].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), if deliver_bs_raa { 4 } else { 3 });
 
-       let events = if deliver_bs_raa {
-               let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
+       if deliver_bs_raa {
+               let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
                match nodes_2_event {
                        MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, .. } } => {
                                assert_eq!(nodes[2].node.get_our_node_id(), *node_id);
@@ -3214,10 +3215,9 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                        },
                        _ => panic!("Unexpected event"),
                }
-               events
-       } else { events };
+       }
 
-       let (nodes_2_event, events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
+       let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
        match nodes_2_event {
                MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => {
                        assert_eq!(channel_id, chan_2.2);
@@ -3226,7 +3226,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
                _ => panic!("Unexpected event"),
        }
 
-       let (nodes_0_event, events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &events);
+       let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events);
        match nodes_0_event {
                MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fail_htlcs, ref update_fulfill_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed, .. } } => {
                        assert!(update_add_htlcs.is_empty());
@@ -3839,9 +3839,10 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
        if messages_delivered == 1 || messages_delivered == 2 {
                expect_payment_path_successful!(nodes[0]);
        }
-
-       nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
-       nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
+       if messages_delivered <= 5 {
+               nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
+               nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
+       }
        reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
 
        if messages_delivered > 2 {
@@ -4641,10 +4642,10 @@ fn test_onchain_to_onchain_claim() {
                _ => panic!("Unexpected event"),
        }
        check_added_monitors!(nodes[1], 1);
-       let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+       let mut msg_events = nodes[1].node.get_and_clear_pending_msg_events();
        assert_eq!(msg_events.len(), 3);
-       let (nodes_2_event, msg_events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &msg_events);
-       let (nodes_0_event, msg_events) = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &msg_events);
+       let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut msg_events);
+       let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut msg_events);
 
        match nodes_2_event {
                MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
@@ -5257,7 +5258,8 @@ fn test_key_derivation_params() {
        let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
        let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[0].chain_source), &chanmon_cfgs[0].tx_broadcaster, &chanmon_cfgs[0].logger, &chanmon_cfgs[0].fee_estimator, &chanmon_cfgs[0].persister, &keys_manager);
        let network_graph = Arc::new(NetworkGraph::new(chanmon_cfgs[0].chain_source.genesis_hash, &chanmon_cfgs[0].logger));
-       let router = test_utils::TestRouter::new(network_graph.clone());
+       let scorer = Mutex::new(test_utils::TestScorer::new());
+       let router = test_utils::TestRouter::new(network_graph.clone(), &scorer);
        let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) };
        let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
        node_cfgs.remove(0);
@@ -6925,7 +6927,7 @@ fn test_check_htlc_underpaying() {
        // Create some initial channels
        create_announced_chan_between_nodes(&nodes, 0, 1);
 
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
        let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features());
        let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
@@ -7175,7 +7177,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000);
        // Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps)
        let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 50).with_features(nodes[1].node.invoice_features());
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
        let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None,
                3_000_000, 50, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
@@ -9186,7 +9188,7 @@ fn test_keysend_payments_to_public_node() {
                final_value_msat: 10000,
                final_cltv_expiry_delta: 40,
        };
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
        let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
 
@@ -9210,8 +9212,6 @@ fn test_keysend_payments_to_private_node() {
 
        let payer_pubkey = nodes[0].node.get_our_node_id();
        let payee_pubkey = nodes[1].node.get_our_node_id();
-       nodes[0].node.peer_connected(&payee_pubkey, &msgs::Init { features: nodes[1].node.init_features(), remote_network_address: None }).unwrap();
-       nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: nodes[0].node.init_features(), remote_network_address: None }).unwrap();
 
        let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
        let route_params = RouteParameters {
@@ -9221,7 +9221,7 @@ fn test_keysend_payments_to_private_node() {
        };
        let network_graph = nodes[0].network_graph.clone();
        let first_hops = nodes[0].node.list_usable_channels();
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
        let route = find_route(
                &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -9285,7 +9285,7 @@ fn test_double_partial_claim() {
 
        let mut events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(events.len(), 2);
-       let (node_1_msgs, _events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events);
+       let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
        pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, false, None);
 
        // At this point nodes[3] has received one half of the payment, and the user goes to handle
index 1ed10a4a61dc784b291d8b514b84a290c4ff6fa7..fbc8e1c9e4ec22cfe805154e04f2af7546ff89f9 100644 (file)
@@ -928,7 +928,7 @@ macro_rules! get_phantom_route {
                                                htlc_maximum_msat: None,
                                        }
                ])]);
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let network_graph = $nodes[0].network_graph.read_only();
                (get_route(
                        &$nodes[0].node.get_our_node_id(), &payment_params, &network_graph,
index a9ced49f647a75e13a1a2a535c38fa86ef29a1c9..31ba9cb371f9c2c3036a5f4779cc9f24c429467e 100644 (file)
@@ -402,22 +402,23 @@ impl OutboundPayments {
                }
        }
 
-       pub(super) fn send_payment<R: Deref, ES: Deref, NS: Deref, F, L: Deref>(
+       pub(super) fn send_payment<R: Deref, ES: Deref, NS: Deref, IH, SP, L: Deref>(
                &self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId,
                retry_strategy: Retry, route_params: RouteParameters, router: &R,
-               first_hops: Vec<ChannelDetails>, inflight_htlcs: InFlightHtlcs, entropy_source: &ES,
-               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F,
+               first_hops: Vec<ChannelDetails>, compute_inflight_htlcs: IH, entropy_source: &ES,
+               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: SP,
        ) -> Result<(), PaymentSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
                NS::Target: NodeSigner,
                L::Target: Logger,
-               F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                         u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
        {
                self.pay_internal(payment_id, Some((payment_hash, payment_secret, None, retry_strategy)),
-                       route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
+                       route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer,
                        best_block_height, logger, &send_payment_along_path)
                        .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
@@ -439,25 +440,26 @@ impl OutboundPayments {
                        .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
        }
 
-       pub(super) fn send_spontaneous_payment<R: Deref, ES: Deref, NS: Deref, F, L: Deref>(
+       pub(super) fn send_spontaneous_payment<R: Deref, ES: Deref, NS: Deref, IH, SP, L: Deref>(
                &self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId,
                retry_strategy: Retry, route_params: RouteParameters, router: &R,
-               first_hops: Vec<ChannelDetails>, inflight_htlcs: InFlightHtlcs, entropy_source: &ES,
-               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F
+               first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES,
+               node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: SP
        ) -> Result<PaymentHash, PaymentSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
                NS::Target: NodeSigner,
                L::Target: Logger,
-               F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                         u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>,
        {
                let preimage = payment_preimage
                        .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes()));
                let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
                self.pay_internal(payment_id, Some((payment_hash, &None, Some(preimage), retry_strategy)),
-                       route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer,
+                       route_params, router, first_hops, &inflight_htlcs, entropy_source, node_signer,
                        best_block_height, logger, &send_payment_along_path)
                        .map(|()| payment_hash)
                        .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
@@ -525,26 +527,27 @@ impl OutboundPayments {
                        }
                        if let Some((payment_id, route_params)) = retry_id_route_params {
                                core::mem::drop(outbounds);
-                               if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, logger, &send_payment_along_path) {
+                               if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, &send_payment_along_path) {
                                        log_info!(logger, "Errored retrying payment: {:?}", e);
                                }
                        } else { break }
                }
        }
 
-       fn pay_internal<R: Deref, NS: Deref, ES: Deref, F, L: Deref>(
+       fn pay_internal<R: Deref, NS: Deref, ES: Deref, IH, SP, L: Deref>(
                &self, payment_id: PaymentId,
                initial_send_info: Option<(PaymentHash, &Option<PaymentSecret>, Option<PaymentPreimage>, Retry)>,
                route_params: RouteParameters, router: &R, first_hops: Vec<ChannelDetails>,
-               inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
-               logger: &L, send_payment_along_path: &F,
+               inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32,
+               logger: &L, send_payment_along_path: &SP,
        ) -> Result<(), PaymentSendFailure>
        where
                R::Target: Router,
                ES::Target: EntropySource,
                NS::Target: NodeSigner,
                L::Target: Logger,
-               F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
+               IH: Fn() -> InFlightHtlcs,
+               SP: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
                   u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
        {
                #[cfg(feature = "std")] {
@@ -557,7 +560,7 @@ impl OutboundPayments {
 
                let route = router.find_route(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
-                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs
+                       Some(&first_hops.iter().collect::<Vec<_>>()), &inflight_htlcs(),
                ).map_err(|e| PaymentSendFailure::ParameterError(APIError::APIMisuseError {
                        err: format!("Failed to find a route for payment {}: {:?}", log_bytes!(payment_id.0), e), // TODO: add APIError::RouteNotFound
                }))?;
@@ -1202,7 +1205,7 @@ mod tests {
        use crate::ln::outbound_payment::{OutboundPayments, Retry};
        use crate::routing::gossip::NetworkGraph;
        use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters};
-       use crate::sync::Arc;
+       use crate::sync::{Arc, Mutex};
        use crate::util::errors::APIError;
        use crate::util::test_utils;
 
@@ -1218,7 +1221,8 @@ mod tests {
                let logger = test_utils::TestLogger::new();
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
                let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger));
-               let router = test_utils::TestRouter::new(network_graph);
+               let scorer = Mutex::new(test_utils::TestScorer::new());
+               let router = test_utils::TestRouter::new(network_graph, &scorer);
                let secp_ctx = Secp256k1::new();
                let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
 
@@ -1234,12 +1238,12 @@ mod tests {
                };
                let err = if on_retry {
                        outbound_payments.pay_internal(
-                               PaymentId([0; 32]), None, expired_route_params, &&router, vec![], InFlightHtlcs::new(),
+                               PaymentId([0; 32]), None, expired_route_params, &&router, vec![], &|| InFlightHtlcs::new(),
                                &&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                } else {
                        outbound_payments.send_payment(
                                PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), expired_route_params,
-                               &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                               &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
                                |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                };
                if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
@@ -1257,7 +1261,8 @@ mod tests {
                let logger = test_utils::TestLogger::new();
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
                let network_graph = Arc::new(NetworkGraph::new(genesis_hash, &logger));
-               let router = test_utils::TestRouter::new(network_graph);
+               let scorer = Mutex::new(test_utils::TestScorer::new());
+               let router = test_utils::TestRouter::new(network_graph, &scorer);
                let secp_ctx = Secp256k1::new();
                let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
 
@@ -1276,12 +1281,12 @@ mod tests {
                                &Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)),
                                Some(route_params.payment_params.clone()), &&keys_manager, 0).unwrap();
                        outbound_payments.pay_internal(
-                               PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
+                               PaymentId([0; 32]), None, route_params, &&router, vec![], &|| InFlightHtlcs::new(),
                                &&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                } else {
                        outbound_payments.send_payment(
                                PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params,
-                               &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
+                               &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger,
                                |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err()
                };
                if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err {
index c6b170364d6f8eb1dceba3b3e9ef2ff7adf4ad4d..b4cb379c4adf17c32911a92e584cb040cbc4fe63 100644 (file)
@@ -21,8 +21,9 @@ use crate::ln::features::InvoiceFeatures;
 use crate::ln::msgs;
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::ln::outbound_payment::Retry;
-use crate::routing::gossip::RoutingFees;
+use crate::routing::gossip::{EffectiveCapacity, RoutingFees};
 use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RouteParameters};
+use crate::routing::scoring::ChannelUsage;
 use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
 use crate::util::test_utils;
 use crate::util::errors::APIError;
@@ -153,11 +154,11 @@ fn mpp_retry() {
        assert_eq!(events.len(), 2);
 
        // Pass half of the payment along the success path.
-       let (success_path_msgs, mut events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events);
+       let success_path_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
        pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 2_000_000, payment_hash, Some(payment_secret), success_path_msgs, false, None);
 
        // Add the HTLC along the first hop.
-       let (fail_path_msgs_1, _events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
+       let fail_path_msgs_1 = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
        let (update_add, commitment_signed) = match fail_path_msgs_1 {
                MessageSendEvent::UpdateHTLCs { node_id: _, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => {
                        assert_eq!(update_add_htlcs.len(), 1);
@@ -237,7 +238,7 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
        assert_eq!(events.len(), 2);
 
        // Pass half of the payment along the first path.
-       let (node_1_msgs, mut events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &events);
+       let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
        pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 200_000, payment_hash, Some(payment_secret), node_1_msgs, false, None);
 
        if send_partial_mpp {
@@ -265,7 +266,7 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
                expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain().expected_htlc_error_data(23, &[][..]));
        } else {
                // Pass half of the payment along the second path.
-               let (node_2_msgs, _events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &events);
+               let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
                pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash, Some(payment_secret), node_2_msgs, true, None);
 
                // Even after MPP_TIMEOUT_TICKS we should not timeout the MPP if we have all the parts
@@ -921,7 +922,7 @@ fn get_ldk_payment_preimage() {
 
        let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)
                .with_features(nodes[1].node.invoice_features());
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
        let random_seed_bytes = keys_manager.get_secure_random_bytes();
        let route = get_route(
@@ -1442,7 +1443,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
        let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(intercept_forwards_config), Some(zero_conf_chan_config)]);
 
        let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let random_seed_bytes = chanmon_cfgs[0].keys_manager.get_secure_random_bytes();
 
        let _ = create_announced_chan_between_nodes(&nodes, 0, 1).2;
@@ -2175,6 +2176,16 @@ fn retry_multi_path_single_failed_payment() {
                        final_value_msat: 100_000_001, final_cltv_expiry_delta: TEST_FINAL_CLTV
                }, Ok(route.clone()));
 
+       {
+               let scorer = chanmon_cfgs[0].scorer.lock().unwrap();
+               // The initial send attempt, 2 paths
+               scorer.expect_usage(chans[0].short_channel_id.unwrap(), ChannelUsage { amount_msat: 10_000, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
+               scorer.expect_usage(chans[1].short_channel_id.unwrap(), ChannelUsage { amount_msat: 100_000_001, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
+               // The retry, 2 paths. Ensure that the in-flight HTLC amount is factored in.
+               scorer.expect_usage(chans[0].short_channel_id.unwrap(), ChannelUsage { amount_msat: 50_000_001, inflight_htlc_msat: 10_000, effective_capacity: EffectiveCapacity::Unknown });
+               scorer.expect_usage(chans[1].short_channel_id.unwrap(), ChannelUsage { amount_msat: 50_000_000, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
+       }
+
        nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap();
        let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(htlc_msgs.len(), 2);
index 427c4001b6d83b36eefbcb7bf3067ccd65fd4b64..8e5056dc68e45750e4bc354df973b86d403720ae 100644 (file)
@@ -701,8 +701,8 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
        // Send the payment through to nodes[3] *without* clearing the PaymentClaimable event
        let mut send_events = nodes[0].node.get_and_clear_pending_msg_events();
        assert_eq!(send_events.len(), 2);
-       let (node_1_msgs, mut send_events) = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &send_events);
-       let (node_2_msgs, _send_events) = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &send_events);
+       let node_1_msgs = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut send_events);
+       let node_2_msgs = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut send_events);
        do_pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_1_msgs, true, false, None);
        do_pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, payment_hash, Some(payment_secret), node_2_msgs, true, false, None);
 
index 55c1341c3d0b9824e9e9abb2aa3667550ee9eecd..ce1f3b64750bafbd7343ff66f47e8b16677d30a4 100644 (file)
@@ -13,8 +13,8 @@ use crate::chain::channelmonitor::ANTI_REORG_DELAY;
 use crate::chain::transaction::OutPoint;
 use crate::chain::Confirm;
 use crate::ln::channelmanager::ChannelManager;
-use crate::ln::msgs::ChannelMessageHandler;
-use crate::util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination};
+use crate::ln::msgs::{ChannelMessageHandler, Init};
+use crate::util::events::{Event, MessageSendEventsProvider, ClosureReason, HTLCDestination};
 use crate::util::test_utils;
 use crate::util::ser::Writeable;
 
@@ -374,6 +374,12 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_
        nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
 
        // Now check that we can create a new channel
+       if reload_node && nodes[0].node.per_peer_state.read().unwrap().len() == 0 {
+               // If we dropped the channel before reloading the node, nodes[1] was also dropped from
+               // nodes[0] storage, and hence not connected again on startup. We therefore need to
+               // reconnect to the node before attempting to create a new channel.
+               nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &Init { features: nodes[1].node.init_features(), remote_network_address: None }).unwrap();
+       }
        create_announced_chan_between_nodes(&nodes, 0, 1);
        send_payment(&nodes[0], &[&nodes[1]], 8000000);
 }
index c1fe23a62a65406180e3dd5356a6fef06340411e..5a5f054846f8d4e66d720aaa3bc77a29b1b7da6e 100644 (file)
@@ -76,7 +76,7 @@ fn updates_shutdown_wait() {
        let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
        let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2);
        let logger = test_utils::TestLogger::new();
-       let scorer = test_utils::TestScorer::with_penalty(0);
+       let scorer = test_utils::TestScorer::new();
        let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
        let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
index a499532e6a94a8c46016afb3d367ceab4461f633..6c2d70bd6f309ed429af45876666673fc6421a35 100644 (file)
@@ -968,7 +968,7 @@ impl<'a> fmt::Debug for DirectedChannelInfo<'a> {
 ///
 /// While this may be smaller than the actual channel capacity, amounts greater than
 /// [`Self::as_msat`] should not be routed through the channel.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub enum EffectiveCapacity {
        /// The available liquidity in the channel known from being a channel counterparty, and thus a
        /// direct hop.
index 0e51b2bf86b9d366386b736c5dacc28e82740592..cfee77a67a2e89ece33616bc5a285a721182bdae 100644 (file)
@@ -72,22 +72,6 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref> Router for DefaultR
                        &random_seed_bytes
                )
        }
-
-       fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
-               self.scorer.lock().payment_path_failed(path, short_channel_id);
-       }
-
-       fn notify_payment_path_successful(&self, path: &[&RouteHop]) {
-               self.scorer.lock().payment_path_successful(path);
-       }
-
-       fn notify_payment_probe_successful(&self, path: &[&RouteHop]) {
-               self.scorer.lock().probe_successful(path);
-       }
-
-       fn notify_payment_probe_failed(&self, path: &[&RouteHop], short_channel_id: u64) {
-               self.scorer.lock().probe_failed(path, short_channel_id);
-       }
 }
 
 /// A trait defining behavior for routing a payment.
@@ -106,14 +90,6 @@ pub trait Router {
        ) -> Result<Route, LightningError> {
                self.find_route(payer, route_params, first_hops, inflight_htlcs)
        }
-       /// Lets the router know that payment through a specific path has failed.
-       fn notify_payment_path_failed(&self, path: &[&RouteHop], short_channel_id: u64);
-       /// Lets the router know that payment through a specific path was successful.
-       fn notify_payment_path_successful(&self, path: &[&RouteHop]);
-       /// Lets the router know that a payment probe was successful.
-       fn notify_payment_probe_successful(&self, path: &[&RouteHop]);
-       /// Lets the router know that a payment probe failed.
-       fn notify_payment_probe_failed(&self, path: &[&RouteHop], short_channel_id: u64);
 }
 
 /// [`Score`] implementation that factors in in-flight HTLC liquidity.
@@ -2116,7 +2092,7 @@ mod tests {
        use crate::routing::router::{get_route, build_route_from_hops_internal, add_random_cltv_offset, default_node_features,
                PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees,
                DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE};
-       use crate::routing::scoring::{ChannelUsage, Score, ProbabilisticScorer, ProbabilisticScoringParameters};
+       use crate::routing::scoring::{ChannelUsage, FixedPenaltyScorer, Score, ProbabilisticScorer, ProbabilisticScoringParameters};
        use crate::routing::test_utils::{add_channel, add_or_update_node, build_graph, build_line_graph, id_to_feature_flags, get_nodes, update_channel};
        use crate::chain::transaction::OutPoint;
        use crate::chain::keysinterface::EntropySource;
@@ -2186,7 +2162,7 @@ mod tests {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2219,7 +2195,7 @@ mod tests {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2241,7 +2217,7 @@ mod tests {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2369,7 +2345,7 @@ mod tests {
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
                let config = UserConfig::default();
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config));
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2507,7 +2483,7 @@ mod tests {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2567,7 +2543,7 @@ mod tests {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2611,7 +2587,7 @@ mod tests {
        fn our_chans_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2742,7 +2718,7 @@ mod tests {
        fn partial_route_hint_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2843,7 +2819,7 @@ mod tests {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(empty_last_hop(&nodes));
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -2923,7 +2899,7 @@ mod tests {
                let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
                let last_hops = multi_hop_last_hops_hint([nodes[2], nodes[3]]);
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone());
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                // Test through channels 2, 3, 0xff00, 0xff01.
@@ -2997,7 +2973,7 @@ mod tests {
 
                let last_hops = multi_hop_last_hops_hint([nodes[2], non_announced_pubkey]);
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone());
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                // Test through channels 2, 3, 0xff00, 0xff01.
                // Test shows that multiple hop hints are considered.
 
@@ -3103,7 +3079,7 @@ mod tests {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops_with_public_channel(&nodes));
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                // This test shows that public routes can be present in the invoice
@@ -3154,7 +3130,7 @@ mod tests {
        fn our_chans_last_hop_connect_test() {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -3277,7 +3253,7 @@ mod tests {
                }]);
                let payment_params = PaymentParameters::from_node_id(target_node_id, 42).with_route_hints(vec![last_hops]);
                let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)];
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
@@ -3338,7 +3314,7 @@ mod tests {
 
                let (secp_ctx, network_graph, mut gossip_sync, chain_monitor, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -3614,7 +3590,7 @@ mod tests {
                // one of the latter hops is limited.
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -3740,7 +3716,7 @@ mod tests {
        fn ignore_fee_first_hop_test() {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
@@ -3788,7 +3764,7 @@ mod tests {
        fn simple_mpp_route_test() {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -3948,7 +3924,7 @@ mod tests {
        fn long_mpp_route_test() {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -4113,7 +4089,7 @@ mod tests {
        fn mpp_cheaper_route_test() {
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -4283,7 +4259,7 @@ mod tests {
                // if the fee is not properly accounted for, the behavior is different.
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -4465,7 +4441,7 @@ mod tests {
                // This bug appeared in production in some specific channel configurations.
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -4557,7 +4533,7 @@ mod tests {
                // path finding we realize that we found more capacity than we need.
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -4719,7 +4695,7 @@ mod tests {
                let network = Arc::new(NetworkGraph::new(genesis_hash, Arc::clone(&logger)));
                let gossip_sync = P2PGossipSync::new(Arc::clone(&network), None, Arc::clone(&logger));
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42);
@@ -4850,7 +4826,7 @@ mod tests {
                // we calculated fees on a higher value, resulting in us ignoring such paths.
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, _, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let payment_params = PaymentParameters::from_node_id(nodes[2], 42);
@@ -4914,7 +4890,7 @@ mod tests {
                // resulting in us thinking there is no possible path, even if other paths exist.
                let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
                let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let config = UserConfig::default();
@@ -4985,7 +4961,7 @@ mod tests {
                let genesis_hash = genesis_block(Network::Testnet).header.block_hash();
                let logger = Arc::new(ln_test_utils::TestLogger::new());
                let network_graph = NetworkGraph::new(genesis_hash, Arc::clone(&logger));
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let config = UserConfig::default();
                let payment_params = PaymentParameters::from_node_id(nodes[0], 42).with_features(channelmanager::provided_invoice_features(&config));
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
@@ -5056,7 +5032,7 @@ mod tests {
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes));
 
                // Without penalizing each hop 100 msats, a longer path with lower fees is chosen.
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let route = get_route(
@@ -5071,7 +5047,7 @@ mod tests {
 
                // Applying a 100 msat penalty to each hop results in taking channels 7 and 10 to nodes[6]
                // from nodes[2] rather than channel 6, 11, and 8, even though the longer path is cheaper.
-               let scorer = ln_test_utils::TestScorer::with_penalty(100);
+               let scorer = FixedPenaltyScorer::with_penalty(100);
                let route = get_route(
                        &our_id, &payment_params, &network_graph.read_only(), None, 100, 42,
                        Arc::clone(&logger), &scorer, &random_seed_bytes
@@ -5130,7 +5106,7 @@ mod tests {
                let network_graph = network.read_only();
 
                // A path to nodes[6] exists when no penalties are applied to any channel.
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
                let route = get_route(
@@ -5245,7 +5221,7 @@ mod tests {
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let network_graph = network.read_only();
 
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
 
                // Make sure that generally there is at least one route available
                let feasible_max_total_cltv_delta = 1008;
@@ -5278,7 +5254,7 @@ mod tests {
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let network_graph = network.read_only();
 
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let mut payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes))
                        .with_max_path_count(1);
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
@@ -5305,7 +5281,7 @@ mod tests {
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
                let network_graph = network.read_only();
 
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
 
@@ -5333,7 +5309,7 @@ mod tests {
                let (secp_ctx, network_graph, _, _, logger) = build_graph();
                let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
 
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
 
                let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes));
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
@@ -5367,7 +5343,7 @@ mod tests {
                let network_graph = network.read_only();
                let network_nodes = network_graph.nodes();
                let network_channels = network_graph.channels();
-               let scorer = ln_test_utils::TestScorer::with_penalty(0);
+               let scorer = ln_test_utils::TestScorer::new();
                let payment_params = PaymentParameters::from_node_id(nodes[3], 0);
                let keys_manager = ln_test_utils::TestKeysInterface::new(&[4u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
index f16dd2d5b401e521c7299cfa3e2b647cb9d105ab..7504e0840d2683125cf75144fd445b070c6f08da 100644 (file)
@@ -260,7 +260,7 @@ impl<'a, S: Writeable> Writeable for MutexGuard<'a, S> {
 }
 
 /// Proposed use of a channel passed as a parameter to [`Score::channel_penalty_msat`].
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub struct ChannelUsage {
        /// The amount to send through the channel, denominated in millisatoshis.
        pub amount_msat: u64,
index 83647a733853029b2501606bc41dc8cec266f1c0..1c2cfe00df0a5379a8f30e9924a50125df35693e 100644 (file)
@@ -22,10 +22,10 @@ use crate::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
 use crate::ln::{msgs, wire};
 use crate::ln::msgs::LightningError;
 use crate::ln::script::ShutdownScript;
-use crate::routing::gossip::{NetworkGraph, NodeId};
+use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId};
 use crate::routing::utxo::{UtxoLookup, UtxoLookupError, UtxoResult};
 use crate::routing::router::{find_route, InFlightHtlcs, Route, RouteHop, RouteParameters, Router, ScorerAccountingForInFlightHtlcs};
-use crate::routing::scoring::FixedPenaltyScorer;
+use crate::routing::scoring::{ChannelUsage, Score};
 use crate::util::config::UserConfig;
 use crate::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
 use crate::util::events;
@@ -48,6 +48,7 @@ use regex;
 
 use crate::io;
 use crate::prelude::*;
+use core::cell::RefCell;
 use core::time::Duration;
 use crate::sync::{Mutex, Arc};
 use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
@@ -79,11 +80,12 @@ impl chaininterface::FeeEstimator for TestFeeEstimator {
 pub struct TestRouter<'a> {
        pub network_graph: Arc<NetworkGraph<&'a TestLogger>>,
        pub next_routes: Mutex<VecDeque<(RouteParameters, Result<Route, LightningError>)>>,
+       pub scorer: &'a Mutex<TestScorer>,
 }
 
 impl<'a> TestRouter<'a> {
-       pub fn new(network_graph: Arc<NetworkGraph<&'a TestLogger>>) -> Self {
-               Self { network_graph, next_routes: Mutex::new(VecDeque::new()), }
+       pub fn new(network_graph: Arc<NetworkGraph<&'a TestLogger>>, scorer: &'a Mutex<TestScorer>) -> Self {
+               Self { network_graph, next_routes: Mutex::new(VecDeque::new()), scorer }
        }
 
        pub fn expect_find_route(&self, query: RouteParameters, result: Result<Route, LightningError>) {
@@ -99,19 +101,40 @@ impl<'a> Router for TestRouter<'a> {
        ) -> Result<Route, msgs::LightningError> {
                if let Some((find_route_query, find_route_res)) = self.next_routes.lock().unwrap().pop_front() {
                        assert_eq!(find_route_query, *params);
+                       if let Ok(ref route) = find_route_res {
+                               let locked_scorer = self.scorer.lock().unwrap();
+                               let scorer = ScorerAccountingForInFlightHtlcs::new(locked_scorer, inflight_htlcs);
+                               for path in &route.paths {
+                                       let mut aggregate_msat = 0u64;
+                                       for (idx, hop) in path.iter().rev().enumerate() {
+                                               aggregate_msat += hop.fee_msat;
+                                               let usage = ChannelUsage {
+                                                       amount_msat: aggregate_msat,
+                                                       inflight_htlc_msat: 0,
+                                                       effective_capacity: EffectiveCapacity::Unknown,
+                                               };
+
+                                               // Since the path is reversed, the last element in our iteration is the first
+                                               // hop.
+                                               if idx == path.len() - 1 {
+                                                       scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(payer), &NodeId::from_pubkey(&hop.pubkey), usage);
+                                               } else {
+                                                       let curr_hop_path_idx = path.len() - 1 - idx;
+                                                       scorer.channel_penalty_msat(hop.short_channel_id, &NodeId::from_pubkey(&path[curr_hop_path_idx - 1].pubkey), &NodeId::from_pubkey(&hop.pubkey), usage);
+                                               }
+                                       }
+                               }
+                       }
                        return find_route_res;
                }
                let logger = TestLogger::new();
+               let scorer = self.scorer.lock().unwrap();
                find_route(
                        payer, params, &self.network_graph, first_hops, &logger,
-                       &ScorerAccountingForInFlightHtlcs::new(TestScorer::with_penalty(0), &inflight_htlcs),
+                       &ScorerAccountingForInFlightHtlcs::new(scorer, &inflight_htlcs),
                        &[42; 32]
                )
        }
-       fn notify_payment_path_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
-       fn notify_payment_path_successful(&self, _path: &[&RouteHop]) {}
-       fn notify_payment_probe_successful(&self, _path: &[&RouteHop]) {}
-       fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
 }
 
 #[cfg(feature = "std")] // If we put this on the `if`, we get "attributes are not yet allowed on `if` expressions" on 1.41.1
@@ -889,5 +912,65 @@ impl Drop for TestChainSource {
        }
 }
 
-/// A scorer useful in testing, when the passage of time isn't a concern.
-pub type TestScorer = FixedPenaltyScorer;
+pub struct TestScorer {
+       /// Stores a tuple of (scid, ChannelUsage)
+       scorer_expectations: RefCell<Option<VecDeque<(u64, ChannelUsage)>>>,
+}
+
+impl TestScorer {
+       pub fn new() -> Self {
+               Self {
+                       scorer_expectations: RefCell::new(None),
+               }
+       }
+
+       pub fn expect_usage(&self, scid: u64, expectation: ChannelUsage) {
+               self.scorer_expectations.borrow_mut().get_or_insert_with(|| VecDeque::new()).push_back((scid, expectation));
+       }
+}
+
+#[cfg(c_bindings)]
+impl crate::util::ser::Writeable for TestScorer {
+       fn write<W: crate::util::ser::Writer>(&self, _: &mut W) -> Result<(), crate::io::Error> { unreachable!(); }
+}
+
+impl Score for TestScorer {
+       fn channel_penalty_msat(
+               &self, short_channel_id: u64, _source: &NodeId, _target: &NodeId, usage: ChannelUsage
+       ) -> u64 {
+               if let Some(scorer_expectations) = self.scorer_expectations.borrow_mut().as_mut() {
+                       match scorer_expectations.pop_front() {
+                               Some((scid, expectation)) => {
+                                       assert_eq!(expectation, usage);
+                                       assert_eq!(scid, short_channel_id);
+                               },
+                               None => {},
+                       }
+               }
+               0
+       }
+
+       fn payment_path_failed(&mut self, _actual_path: &[&RouteHop], _actual_short_channel_id: u64) {}
+
+       fn payment_path_successful(&mut self, _actual_path: &[&RouteHop]) {}
+
+       fn probe_failed(&mut self, _actual_path: &[&RouteHop], _: u64) {}
+
+       fn probe_successful(&mut self, _actual_path: &[&RouteHop]) {}
+}
+
+impl Drop for TestScorer {
+       fn drop(&mut self) {
+               #[cfg(feature = "std")] {
+                       if std::thread::panicking() {
+                               return;
+                       }
+               }
+
+               if let Some(scorer_expectations) = self.scorer_expectations.borrow().as_ref() {
+                       if !scorer_expectations.is_empty() {
+                               panic!("Unsatisfied scorer expectations: {:?}", scorer_expectations)
+                       }
+               }
+       }
+}