Merge pull request #2024 from TheBlueMatt/2023-02-6conf-pub-hints
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sun, 19 Mar 2023 23:42:15 +0000 (23:42 +0000)
committerGitHub <noreply@github.com>
Sun, 19 Mar 2023 23:42:15 +0000 (23:42 +0000)
Include a route hint for public, not-yet-announced channels

1  2 
lightning-invoice/src/utils.rs
lightning/src/ln/functional_test_utils.rs

index f837502e3e687db86bda72850ed7c2db4c801599,53e411212f2a1537fd0012a68881237dd52c8894..94a597c61658e49c99c16a0abdd3779dd64c1019
@@@ -141,7 -141,7 +141,7 @@@ wher
        L::Target: Logger,
  {
  
 -      if phantom_route_hints.len() == 0 {
 +      if phantom_route_hints.is_empty() {
                return Err(SignOrCreationError::CreationError(
                        CreationError::MissingRouteHints,
                ));
@@@ -512,8 -512,10 +512,10 @@@ fn _create_invoice_from_channelmanager_
  /// * Always select the channel with the highest inbound capacity per counterparty node
  /// * Prefer channels with capacity at least `min_inbound_capacity_msat` and where the channel
  ///   `is_usable` (i.e. the peer is connected).
- /// * If any public channel exists, the returned `RouteHint`s will be empty, and the sender will
- ///   need to find the path by looking at the public channels instead
+ /// * If any public channel exists, only public [`RouteHint`]s will be returned.
+ /// * If any public, announced, channel exists (i.e. a channel with 7+ confs, to ensure the
+ ///   announcement has had a chance to propagate), no [`RouteHint`]s will be returned, as the
+ ///   sender is expected to find the path by looking at the public channels instead.
  fn filter_channels<L: Deref>(
        channels: Vec<ChannelDetails>, min_inbound_capacity_msat: Option<u64>, logger: &L
  ) -> Vec<RouteHint> where L::Target: Logger {
        let mut min_capacity_channel_exists = false;
        let mut online_channel_exists = false;
        let mut online_min_capacity_channel_exists = false;
+       let mut has_pub_unconf_chan = false;
  
        log_trace!(logger, "Considering {} channels for invoice route hints", channels.len());
        for channel in channels.into_iter().filter(|chan| chan.is_channel_ready) {
                }
  
                if channel.is_public {
-                       // If any public channel exists, return no hints and let the sender
-                       // look at the public channels instead.
-                       log_trace!(logger, "Not including channels in invoice route hints on account of public channel {}",
-                               log_bytes!(channel.channel_id));
-                       return vec![]
+                       if channel.confirmations.is_some() && channel.confirmations < Some(7) {
+                               // If we have a public channel, but it doesn't have enough confirmations to (yet)
+                               // be in the public network graph (and have gotten a chance to propagate), include
+                               // route hints but only for public channels to protect private channel privacy.
+                               has_pub_unconf_chan = true;
+                       } else {
+                               // If any public channel exists, return no hints and let the sender
+                               // look at the public channels instead.
+                               log_trace!(logger, "Not including channels in invoice route hints on account of public channel {}",
+                                       log_bytes!(channel.channel_id));
+                               return vec![]
+                       }
                }
  
                if channel.inbound_capacity_msat >= min_inbound_capacity {
                        }
                }
  
 -              if channel.is_usable {
 -                      if !online_channel_exists {
 -                              log_trace!(logger, "Channel with connected peer exists for invoice route hints");
 -                              online_channel_exists = true;
 -                      }
 +              if channel.is_usable && !online_channel_exists {
 +                      log_trace!(logger, "Channel with connected peer exists for invoice route hints");
 +                      online_channel_exists = true;
                }
  
                match filtered_channels.entry(channel.counterparty.node_id) {
                        hash_map::Entry::Occupied(mut entry) => {
                                let current_max_capacity = entry.get().inbound_capacity_msat;
-                               if channel.inbound_capacity_msat < current_max_capacity {
+                               // If this channel is public and the previous channel is not, ensure we replace the
+                               // previous channel to avoid announcing non-public channels.
+                               let new_now_public = channel.is_public && !entry.get().is_public;
+                               // If the public-ness of the channel has not changed (in which case simply defer to
+                               // `new_now_public), and this channel has a greater capacity, prefer to announce
+                               // this channel.
+                               let new_higher_capacity = channel.is_public == entry.get().is_public &&
+                                       channel.inbound_capacity_msat > current_max_capacity;
+                               if new_now_public || new_higher_capacity {
+                                       log_trace!(logger,
+                                               "Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",
+                                               log_pubkey!(channel.counterparty.node_id),
+                                               log_bytes!(channel.channel_id), channel.short_channel_id,
+                                               channel.inbound_capacity_msat,
+                                               log_bytes!(entry.get().channel_id), entry.get().short_channel_id,
+                                               current_max_capacity);
+                                       entry.insert(channel);
+                               } else {
                                        log_trace!(logger,
-                                               "Preferring counterparty {} channel {} ({} msats) over {} ({} msats) for invoice route hints",
+                                               "Preferring counterparty {} channel {} (SCID {:?}, {} msats) over {} (SCID {:?}, {} msats) for invoice route hints",
                                                log_pubkey!(channel.counterparty.node_id),
-                                               log_bytes!(entry.get().channel_id), current_max_capacity,
-                                               log_bytes!(channel.channel_id), channel.inbound_capacity_msat);
-                                       continue;
+                                               log_bytes!(entry.get().channel_id), entry.get().short_channel_id,
+                                               current_max_capacity,
+                                               log_bytes!(channel.channel_id), channel.short_channel_id,
+                                               channel.inbound_capacity_msat);
                                }
-                               log_trace!(logger,
-                                       "Preferring counterparty {} channel {} ({} msats) over {} ({} msats) for invoice route hints",
-                                       log_pubkey!(channel.counterparty.node_id),
-                                       log_bytes!(channel.channel_id), channel.inbound_capacity_msat,
-                                       log_bytes!(entry.get().channel_id), current_max_capacity);
-                               entry.insert(channel);
                        }
                        hash_map::Entry::Vacant(entry) => {
                                entry.insert(channel);
                .map(|(_, channel)| channel)
                .filter(|channel| {
                        let has_enough_capacity = channel.inbound_capacity_msat >= min_inbound_capacity;
-                       let include_channel = if online_min_capacity_channel_exists {
+                       let include_channel = if has_pub_unconf_chan {
+                               // If we have a public channel, but it doesn't have enough confirmations to (yet)
+                               // be in the public network graph (and have gotten a chance to propagate), include
+                               // route hints but only for public channels to protect private channel privacy.
+                               channel.is_public
+                       } else if online_min_capacity_channel_exists {
                                has_enough_capacity && channel.is_usable
                        } else if min_capacity_channel_exists && online_channel_exists {
                                // If there are some online channels and some min_capacity channels, but no
                                log_trace!(logger, "Ignoring channel {} without enough capacity for invoice route hints",
                                        log_bytes!(channel.channel_id));
                        } else {
-                               debug_assert!(!channel.is_usable);
+                               debug_assert!(!channel.is_usable || (has_pub_unconf_chan && !channel.is_public));
                                log_trace!(logger, "Ignoring channel {} with disconnected peer",
                                        log_bytes!(channel.channel_id));
                        }
@@@ -658,7 -687,7 +685,7 @@@ mod test 
                create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001);
                let non_default_invoice_expiry_secs = 4200;
                let invoice = create_invoice_from_channelmanager_and_duration_since_epoch(
 -                      &nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
 +                      nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
                        Some(10_000), "test".to_string(), Duration::from_secs(1234567),
                        non_default_invoice_expiry_secs, None).unwrap();
                assert_eq!(invoice.amount_pico_btc(), Some(100_000));
                let route_params = RouteParameters {
                        payment_params,
                        final_value_msat: invoice.amount_milli_satoshis().unwrap(),
 -                      final_cltv_expiry_delta: invoice.min_final_cltv_expiry_delta() as u32,
                };
                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,
 +                      &nodes[0].node.get_our_node_id(), &route_params, network_graph,
                        Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
                ).unwrap();
  
                let payment_event = {
                        let mut payment_hash = PaymentHash([0; 32]);
                        payment_hash.0.copy_from_slice(&invoice.payment_hash().as_ref()[0..32]);
 -                      nodes[0].node.send_payment(&route, payment_hash, &Some(invoice.payment_secret().clone()), PaymentId(payment_hash.0)).unwrap();
 +                      nodes[0].node.send_payment(&route, payment_hash, &Some(*invoice.payment_secret()), PaymentId(payment_hash.0)).unwrap();
                        let mut added_monitors = nodes[0].chain_monitor.added_monitors.lock().unwrap();
                        assert_eq!(added_monitors.len(), 1);
                        added_monitors.clear();
                let custom_min_final_cltv_expiry_delta = Some(50);
  
                let invoice = crate::utils::create_invoice_from_channelmanager_and_duration_since_epoch(
 -                      &nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
 +                      nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
                        Some(10_000), "".into(), Duration::from_secs(1234567), 3600,
                        if with_custom_delta { custom_min_final_cltv_expiry_delta } else { None },
                ).unwrap();
                let custom_min_final_cltv_expiry_delta = Some(21);
  
                let invoice = crate::utils::create_invoice_from_channelmanager_and_duration_since_epoch(
 -                      &nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
 +                      nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
                        Some(10_000), "".into(), Duration::from_secs(1234567), 3600,
                        custom_min_final_cltv_expiry_delta,
                ).unwrap();
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                let description_hash = crate::Sha256(Hash::hash("Testing description_hash".as_bytes()));
                let invoice = crate::utils::create_invoice_from_channelmanager_with_description_hash_and_duration_since_epoch(
 -                      &nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
 +                      nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
                        Some(10_000), description_hash, Duration::from_secs(1234567), 3600, None,
                ).unwrap();
                assert_eq!(invoice.amount_pico_btc(), Some(100_000));
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                let payment_hash = PaymentHash([0; 32]);
                let invoice = crate::utils::create_invoice_from_channelmanager_and_duration_since_epoch_with_payment_hash(
 -                      &nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
 +                      nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
                        Some(10_000), "test".to_string(), Duration::from_secs(1234567), 3600,
                        payment_hash, None,
                ).unwrap();
                assert_eq!(invoice.payment_hash(), &sha256::Hash::from_slice(&payment_hash.0[..]).unwrap());
        }
  
+       #[test]
+       fn test_hints_has_only_public_confd_channels() {
+               let chanmon_cfgs = create_chanmon_cfgs(2);
+               let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+               let mut config = test_default_channel_config();
+               config.channel_handshake_config.minimum_depth = 1;
+               let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]);
+               let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+               // Create a private channel with lots of capacity and a lower value public channel (without
+               // confirming the funding tx yet).
+               let unannounced_scid = create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0);
+               let conf_tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 10_000, 0);
+               // Before the channel is available, we should include the unannounced_scid.
+               let mut scid_aliases = HashSet::new();
+               scid_aliases.insert(unannounced_scid.0.short_channel_id_alias.unwrap());
+               match_invoice_routes(Some(5000), &nodes[1], scid_aliases.clone());
+               // However after we mine the funding tx and exchange channel_ready messages for the public
+               // channel we'll immediately switch to including it as a route hint, even though it isn't
+               // yet announced.
+               let pub_channel_scid = mine_transaction(&nodes[0], &conf_tx);
+               let node_a_pub_channel_ready = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReady, nodes[1].node.get_our_node_id());
+               nodes[1].node.handle_channel_ready(&nodes[0].node.get_our_node_id(), &node_a_pub_channel_ready);
+               assert_eq!(mine_transaction(&nodes[1], &conf_tx), pub_channel_scid);
+               let events = nodes[1].node.get_and_clear_pending_msg_events();
+               assert_eq!(events.len(), 2);
+               if let MessageSendEvent::SendChannelReady { msg, .. } = &events[0] {
+                       nodes[0].node.handle_channel_ready(&nodes[1].node.get_our_node_id(), msg);
+               } else { panic!(); }
+               if let MessageSendEvent::SendChannelUpdate { msg, .. } = &events[1] {
+                       nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), msg);
+               } else { panic!(); }
+               nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, nodes[1].node.get_our_node_id()));
+               expect_channel_ready_event(&nodes[0], &nodes[1].node.get_our_node_id());
+               expect_channel_ready_event(&nodes[1], &nodes[0].node.get_our_node_id());
+               scid_aliases.clear();
+               scid_aliases.insert(node_a_pub_channel_ready.short_channel_id_alias.unwrap());
+               match_invoice_routes(Some(5000), &nodes[1], scid_aliases.clone());
+               // This also applies even if the amount is more than the payment amount, to ensure users
+               // dont screw up their privacy.
+               match_invoice_routes(Some(50_000_000), &nodes[1], scid_aliases.clone());
+               // The same remains true until the channel has 7 confirmations, at which point we include
+               // no hints.
+               connect_blocks(&nodes[1], 5);
+               match_invoice_routes(Some(5000), &nodes[1], scid_aliases.clone());
+               connect_blocks(&nodes[1], 1);
+               get_event_msg!(nodes[1], MessageSendEvent::SendAnnouncementSignatures, nodes[0].node.get_our_node_id());
+               match_invoice_routes(Some(5000), &nodes[1], HashSet::new());
+       }
        #[test]
        fn test_hints_includes_single_channels_to_nodes() {
                let chanmon_cfgs = create_chanmon_cfgs(3);
  
                // With only one sufficient-value peer connected we should only get its hint
                scid_aliases.remove(&chan_b.0.short_channel_id_alias.unwrap());
 -              nodes[0].node.peer_disconnected(&nodes[2].node.get_our_node_id(), false);
 +              nodes[0].node.peer_disconnected(&nodes[2].node.get_our_node_id());
                match_invoice_routes(Some(1_000_000_000), &nodes[0], scid_aliases.clone());
  
                // If we don't have any sufficient-value peers connected we should get all hints with
                // sufficient value, even though there is a connected insufficient-value peer.
                scid_aliases.insert(chan_b.0.short_channel_id_alias.unwrap());
 -              nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
 +              nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
                match_invoice_routes(Some(1_000_000_000), &nodes[0], scid_aliases);
        }
  
                mut chan_ids_to_match: HashSet<u64>
        ) {
                let invoice = create_invoice_from_channelmanager_and_duration_since_epoch(
 -                      &invoice_node.node, invoice_node.keys_manager, invoice_node.logger,
 +                      invoice_node.node, invoice_node.keys_manager, invoice_node.logger,
                        Currency::BitcoinTestnet, invoice_amt, "test".to_string(), Duration::from_secs(1234567),
                        3600, None).unwrap();
                let hints = invoice.private_routes();
        #[cfg(feature = "std")]
        fn do_test_multi_node_receive(user_generated_pmt_hash: bool) {
                let mut chanmon_cfgs = create_chanmon_cfgs(3);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
                let invoice =
                        crate::utils::create_phantom_invoice::<&test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestLogger>(
                                Some(payment_amt), payment_hash, "test".to_string(), non_default_invoice_expiry_secs,
 -                              route_hints, &nodes[1].keys_manager, &nodes[1].keys_manager, &nodes[1].logger,
 +                              route_hints, nodes[1].keys_manager, nodes[1].keys_manager, nodes[1].logger,
                                Currency::BitcoinTestnet, None, Duration::from_secs(1234567)
                        ).unwrap();
                let (payment_hash, payment_secret) = (PaymentHash(invoice.payment_hash().into_inner()), *invoice.payment_secret());
                let params = RouteParameters {
                        payment_params,
                        final_value_msat: invoice.amount_milli_satoshis().unwrap(),
 -                      final_cltv_expiry_delta: invoice.min_final_cltv_expiry_delta() as u32,
                };
                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,
 +                      &nodes[0].node.get_our_node_id(), &params, network_graph,
                        Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
                ).unwrap();
                let (payment_event, fwd_idx) = {
                        let mut payment_hash = PaymentHash([0; 32]);
                        payment_hash.0.copy_from_slice(&invoice.payment_hash().as_ref()[0..32]);
 -                      nodes[0].node.send_payment(&route, payment_hash, &Some(invoice.payment_secret().clone()), PaymentId(payment_hash.0)).unwrap();
 +                      nodes[0].node.send_payment(&route, payment_hash, &Some(*invoice.payment_secret()), PaymentId(payment_hash.0)).unwrap();
                        let mut added_monitors = nodes[0].chain_monitor.added_monitors.lock().unwrap();
                        assert_eq!(added_monitors.len(), 1);
                        added_monitors.clear();
  
                let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
                expect_payment_claimable!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, route.paths[0].last().unwrap().pubkey);
 -              do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[fwd_idx])[..]), false, payment_preimage);
 +              do_claim_payment_along_route(&nodes[0], &[&vec!(&nodes[fwd_idx])[..]], false, payment_preimage);
                let events = nodes[0].node.get_and_clear_pending_events();
                assert_eq!(events.len(), 2);
                match events[0] {
        #[cfg(feature = "std")]
        fn test_multi_node_hints_has_htlc_min_max_values() {
                let mut chanmon_cfgs = create_chanmon_cfgs(3);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
  
                let invoice = crate::utils::create_phantom_invoice::<&test_utils::TestKeysInterface,
                        &test_utils::TestKeysInterface, &test_utils::TestLogger>(Some(payment_amt), Some(payment_hash),
 -                              "test".to_string(), 3600, route_hints, &nodes[1].keys_manager, &nodes[1].keys_manager,
 -                              &nodes[1].logger, Currency::BitcoinTestnet, None, Duration::from_secs(1234567)).unwrap();
 +                              "test".to_string(), 3600, route_hints, nodes[1].keys_manager, nodes[1].keys_manager,
 +                              nodes[1].logger, Currency::BitcoinTestnet, None, Duration::from_secs(1234567)).unwrap();
  
                let chan_0_1 = &nodes[1].node.list_usable_channels()[0];
                assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan_0_1.inbound_htlc_minimum_msat);
                        &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestLogger,
                >(
                        Some(payment_amt), None, non_default_invoice_expiry_secs, description_hash,
 -                      route_hints, &nodes[1].keys_manager, &nodes[1].keys_manager, &nodes[1].logger,
 +                      route_hints, nodes[1].keys_manager, nodes[1].keys_manager, nodes[1].logger,
                        Currency::BitcoinTestnet, None, Duration::from_secs(1234567),
                )
                .unwrap();
                let duration_since_epoch = Duration::from_secs(1234567);
                let invoice = crate::utils::create_phantom_invoice::<&test_utils::TestKeysInterface,
                        &test_utils::TestKeysInterface, &test_utils::TestLogger>(Some(payment_amt), payment_hash,
 -                              "".to_string(), non_default_invoice_expiry_secs, route_hints, &nodes[1].keys_manager, &nodes[1].keys_manager,
 -                              &nodes[1].logger, Currency::BitcoinTestnet, min_final_cltv_expiry_delta, duration_since_epoch).unwrap();
 +                              "".to_string(), non_default_invoice_expiry_secs, route_hints, nodes[1].keys_manager, nodes[1].keys_manager,
 +                              nodes[1].logger, Currency::BitcoinTestnet, min_final_cltv_expiry_delta, duration_since_epoch).unwrap();
                assert_eq!(invoice.amount_pico_btc(), Some(200_000));
                assert_eq!(invoice.min_final_cltv_expiry_delta(), (min_final_cltv_expiry_delta.unwrap() + 3) as u64);
                assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into()));
        #[cfg(feature = "std")]
        fn test_multi_node_hints_includes_single_channels_to_participating_nodes() {
                let mut chanmon_cfgs = create_chanmon_cfgs(3);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
        #[cfg(feature = "std")]
        fn test_multi_node_hints_includes_one_channel_of_each_counterparty_nodes_per_participating_node() {
                let mut chanmon_cfgs = create_chanmon_cfgs(4);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[3].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
        #[cfg(feature = "std")]
        fn test_multi_node_forwarding_info_not_assigned_channel_excluded_from_hints() {
                let mut chanmon_cfgs = create_chanmon_cfgs(4);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[3].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
        #[cfg(feature = "std")]
        fn test_multi_node_with_only_public_channels_hints_includes_only_phantom_route() {
                let mut chanmon_cfgs = create_chanmon_cfgs(3);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
        #[cfg(feature = "std")]
        fn test_multi_node_with_mixed_public_and_private_channel_hints_includes_only_phantom_route() {
                let mut chanmon_cfgs = create_chanmon_cfgs(4);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
        #[cfg(feature = "std")]
        fn test_multi_node_hints_has_only_highest_inbound_capacity_channel() {
                let mut chanmon_cfgs = create_chanmon_cfgs(3);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
        #[cfg(feature = "std")]
        fn test_multi_node_channels_inbound_capacity_lower_than_invoice_amt_filtering() {
                let mut chanmon_cfgs = create_chanmon_cfgs(4);
 -              let seed_1 = [42 as u8; 32];
 -              let seed_2 = [43 as u8; 32];
 -              let cross_node_seed = [44 as u8; 32];
 +              let seed_1 = [42u8; 32];
 +              let seed_2 = [43u8; 32];
 +              let cross_node_seed = [44u8; 32];
                chanmon_cfgs[1].keys_manager.backing = PhantomKeysManager::new(&seed_1, 43, 44, &cross_node_seed);
                chanmon_cfgs[2].keys_manager.backing = PhantomKeysManager::new(&seed_2, 43, 44, &cross_node_seed);
                let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
  
                let invoice = crate::utils::create_phantom_invoice::<&test_utils::TestKeysInterface,
                        &test_utils::TestKeysInterface, &test_utils::TestLogger>(invoice_amt, None, "test".to_string(),
 -                              3600, phantom_route_hints, &invoice_node.keys_manager, &invoice_node.keys_manager,
 -                              &invoice_node.logger, Currency::BitcoinTestnet, None, Duration::from_secs(1234567)).unwrap();
 +                              3600, phantom_route_hints, invoice_node.keys_manager, invoice_node.keys_manager,
 +                              invoice_node.logger, Currency::BitcoinTestnet, None, Duration::from_secs(1234567)).unwrap();
  
                let invoice_hints = invoice.private_routes();
  
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                let result = crate::utils::create_invoice_from_channelmanager_and_duration_since_epoch(
 -                      &nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
 +                      nodes[1].node, nodes[1].keys_manager, nodes[1].logger, Currency::BitcoinTestnet,
                        Some(10_000), "Some description".into(), Duration::from_secs(1234567), 3600, Some(MIN_FINAL_CLTV_EXPIRY_DELTA - 4),
                );
                match result {
index f86c4f03dab77673db176ed5ee95c80354a9f5ef,64825e7aa6d15edb012b6096f6d84520c914153a..f48c9d099e7e3eb3b03d4cf5f92cfca1d37f4b4b
@@@ -16,16 -16,15 +16,16 @@@ use crate::chain::transaction::OutPoint
  use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
  use crate::ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId, MIN_CLTV_EXPIRY_DELTA};
  use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate};
 -use crate::routing::router::{PaymentParameters, Route, get_route};
 +use crate::routing::router::{self, PaymentParameters, Route};
  use crate::ln::features::InitFeatures;
  use crate::ln::msgs;
  use crate::ln::msgs::{ChannelMessageHandler,RoutingMessageHandler};
 +use crate::util::events::ClosureReason;
  use crate::util::enforcing_trait_impls::EnforcingSigner;
  use crate::util::scid_utils;
  use crate::util::test_utils;
 -use crate::util::test_utils::{panicking, TestChainMonitor};
 -use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose};
 +use crate::util::test_utils::{panicking, TestChainMonitor, TestScorer, TestKeysInterface};
 +use crate::util::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose};
  use crate::util::errors::APIError;
  use crate::util::config::UserConfig;
  use crate::util::ser::{ReadableArgs, Writeable};
@@@ -45,7 -44,7 +45,7 @@@ use crate::io
  use crate::prelude::*;
  use core::cell::RefCell;
  use alloc::rc::Rc;
 -use crate::sync::{Arc, Mutex};
 +use crate::sync::{Arc, Mutex, LockTestExt};
  use core::mem;
  use core::iter::repeat;
  use bitcoin::{PackedLockTime, TxMerkleNode};
@@@ -63,9 -62,12 +63,12 @@@ pub fn confirm_transaction<'a, 'b, 'c, 
        scid
  }
  /// Mine a single block containing the given transaction
- pub fn mine_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction) {
+ ///
+ /// Returns the SCID a channel confirmed in the given transaction will have, assuming the funding
+ /// output is the 1st output in the transaction.
+ pub fn mine_transaction<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction) -> u64 {
        let height = node.best_block_info().1 + 1;
-       confirm_transaction_at(node, tx, height);
+       confirm_transaction_at(node, tx, height)
  }
  /// Mine a single block containing the given transactions
  pub fn mine_transactions<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, txn: &[&Transaction]) {
@@@ -306,7 -308,6 +309,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> {
@@@ -352,19 -353,6 +355,19 @@@ impl<'a, 'b, 'c> Node<'a, 'b, 'c> 
        }
  }
  
 +/// If we need an unsafe pointer to a `Node` (ie to reference it in a thread
 +/// pre-std::thread::scope), this provides that with `Sync`. Note that accessing some of the fields
 +/// in the `Node` are not safe to use (i.e. the ones behind an `Rc`), but that's left to the caller
 +/// to figure out.
 +pub struct NodePtr(pub *const Node<'static, 'static, 'static>);
 +impl NodePtr {
 +      pub fn from_node<'a, 'b: 'a, 'c: 'b>(node: &Node<'a, 'b, 'c>) -> Self {
 +              Self((node as *const Node<'a, 'b, 'c>).cast())
 +      }
 +}
 +unsafe impl Send for NodePtr {}
 +unsafe impl Sync for NodePtr {}
 +
  impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
        fn drop(&mut self) {
                if !panicking() {
                                        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 {
                                        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,
                                        panic!();
                                }
                        }
 -                      assert_eq!(*chain_source.watched_txn.lock().unwrap(), *self.chain_source.watched_txn.lock().unwrap());
 -                      assert_eq!(*chain_source.watched_outputs.lock().unwrap(), *self.chain_source.watched_outputs.lock().unwrap());
 +                      assert_eq!(*chain_source.watched_txn.unsafe_well_ordered_double_lock_self(), *self.chain_source.watched_txn.unsafe_well_ordered_double_lock_self());
 +                      assert_eq!(*chain_source.watched_outputs.unsafe_well_ordered_double_lock_self(), *self.chain_source.watched_outputs.unsafe_well_ordered_double_lock_self());
                }
        }
  }
@@@ -483,46 -470,33 +486,46 @@@ pub fn create_chan_between_nodes_with_v
        (announcement, as_update, bs_update, channel_id, tx)
  }
  
 +/// Gets an RAA and CS which were sent in response to a commitment update
 +///
 +/// Should only be used directly when the `$node` is not actually a [`Node`].
 +macro_rules! do_get_revoke_commit_msgs {
 +      ($node: expr, $recipient: expr) => { {
 +              let events = $node.node.get_and_clear_pending_msg_events();
 +              assert_eq!(events.len(), 2);
 +              (match events[0] {
 +                      MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
 +                              assert_eq!(node_id, $recipient);
 +                              (*msg).clone()
 +                      },
 +                      _ => panic!("Unexpected event"),
 +              }, match events[1] {
 +                      MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
 +                              assert_eq!(node_id, $recipient);
 +                              assert!(updates.update_add_htlcs.is_empty());
 +                              assert!(updates.update_fulfill_htlcs.is_empty());
 +                              assert!(updates.update_fail_htlcs.is_empty());
 +                              assert!(updates.update_fail_malformed_htlcs.is_empty());
 +                              assert!(updates.update_fee.is_none());
 +                              updates.commitment_signed.clone()
 +                      },
 +                      _ => panic!("Unexpected event"),
 +              })
 +      } }
 +}
 +
 +/// Gets an RAA and CS which were sent in response to a commitment update
 +pub fn get_revoke_commit_msgs(node: &Node, recipient: &PublicKey) -> (msgs::RevokeAndACK, msgs::CommitmentSigned) {
 +      do_get_revoke_commit_msgs!(node, recipient)
 +}
 +
  #[macro_export]
  /// Gets an RAA and CS which were sent in response to a commitment update
 +///
 +/// Don't use this, use the identically-named function instead.
  macro_rules! get_revoke_commit_msgs {
        ($node: expr, $node_id: expr) => {
 -              {
 -                      use $crate::util::events::MessageSendEvent;
 -                      let events = $node.node.get_and_clear_pending_msg_events();
 -                      assert_eq!(events.len(), 2);
 -                      (match events[0] {
 -                              MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
 -                                      assert_eq!(*node_id, $node_id);
 -                                      (*msg).clone()
 -                              },
 -                              _ => panic!("Unexpected event"),
 -                      }, match events[1] {
 -                              MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
 -                                      assert_eq!(*node_id, $node_id);
 -                                      assert!(updates.update_add_htlcs.is_empty());
 -                                      assert!(updates.update_fulfill_htlcs.is_empty());
 -                                      assert!(updates.update_fail_htlcs.is_empty());
 -                                      assert!(updates.update_fail_malformed_htlcs.is_empty());
 -                                      assert!(updates.update_fee.is_none());
 -                                      updates.commitment_signed.clone()
 -                              },
 -                              _ => panic!("Unexpected event"),
 -                      })
 -              }
 +              $crate::ln::functional_test_utils::get_revoke_commit_msgs(&$node, &$node_id)
        }
  }
  
@@@ -545,17 -519,22 +548,17 @@@ macro_rules! get_event_msg 
  }
  
  /// Get an error message from the pending events queue.
 -#[macro_export]
 -macro_rules! get_err_msg {
 -      ($node: expr, $node_id: expr) => {
 -              {
 -                      let events = $node.node.get_and_clear_pending_msg_events();
 -                      assert_eq!(events.len(), 1);
 -                      match events[0] {
 -                              $crate::util::events::MessageSendEvent::HandleError {
 -                                      action: $crate::ln::msgs::ErrorAction::SendErrorMessage { ref msg }, ref node_id
 -                              } => {
 -                                      assert_eq!(*node_id, $node_id);
 -                                      (*msg).clone()
 -                              },
 -                              _ => panic!("Unexpected event"),
 -                      }
 -              }
 +pub fn get_err_msg(node: &Node, recipient: &PublicKey) -> msgs::ErrorMessage {
 +      let events = node.node.get_and_clear_pending_msg_events();
 +      assert_eq!(events.len(), 1);
 +      match events[0] {
 +              MessageSendEvent::HandleError {
 +                      action: msgs::ErrorAction::SendErrorMessage { ref msg }, ref node_id
 +              } => {
 +                      assert_eq!(node_id, recipient);
 +                      (*msg).clone()
 +              },
 +              _ => panic!("Unexpected event"),
        }
  }
  
@@@ -577,36 -556,31 +580,36 @@@ macro_rules! get_event 
        }
  }
  
 +/// Gets an UpdateHTLCs MessageSendEvent
 +pub fn get_htlc_update_msgs(node: &Node, recipient: &PublicKey) -> msgs::CommitmentUpdate {
 +      let events = node.node.get_and_clear_pending_msg_events();
 +      assert_eq!(events.len(), 1);
 +      match events[0] {
 +              MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
 +                      assert_eq!(node_id, recipient);
 +                      (*updates).clone()
 +              },
 +              _ => panic!("Unexpected event"),
 +      }
 +}
 +
  #[macro_export]
  /// Gets an UpdateHTLCs MessageSendEvent
 +///
 +/// Don't use this, use the identically-named function instead.
  macro_rules! get_htlc_update_msgs {
        ($node: expr, $node_id: expr) => {
 -              {
 -                      let events = $node.node.get_and_clear_pending_msg_events();
 -                      assert_eq!(events.len(), 1);
 -                      match events[0] {
 -                              $crate::util::events::MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
 -                                      assert_eq!(*node_id, $node_id);
 -                                      (*updates).clone()
 -                              },
 -                              _ => panic!("Unexpected event"),
 -                      }
 -              }
 +              $crate::ln::functional_test_utils::get_htlc_update_msgs(&$node, &$node_id)
        }
  }
  
  /// 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
                MessageSendEvent::BroadcastChannelUpdate { .. } => {
                        false
                },
 +              MessageSendEvent::BroadcastNodeAnnouncement { .. } => {
 +                      false
 +              },
                MessageSendEvent::SendChannelUpdate { node_id, .. } => {
                        node_id == msg_node_id
                },
                },
        }});
        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!")
        }
@@@ -697,7 -670,7 +700,7 @@@ macro_rules! get_feerate 
                        let mut per_peer_state_lock;
                        let mut peer_state_lock;
                        let chan = get_channel_ref!($node, $counterparty_node, per_peer_state_lock, peer_state_lock, $channel_id);
 -                      chan.get_feerate()
 +                      chan.get_feerate_sat_per_1000_weight()
                }
        }
  }
@@@ -772,19 -745,14 +775,19 @@@ macro_rules! unwrap_send_err 
  }
  
  /// Check whether N channel monitor(s) have been added.
 +pub fn check_added_monitors(node: &Node, count: usize) {
 +      let mut added_monitors = node.chain_monitor.added_monitors.lock().unwrap();
 +      assert_eq!(added_monitors.len(), count);
 +      added_monitors.clear();
 +}
 +
 +/// Check whether N channel monitor(s) have been added.
 +///
 +/// Don't use this, use the identically-named function instead.
  #[macro_export]
  macro_rules! check_added_monitors {
        ($node: expr, $count: expr) => {
 -              {
 -                      let mut added_monitors = $node.chain_monitor.added_monitors.lock().unwrap();
 -                      assert_eq!(added_monitors.len(), $count);
 -                      added_monitors.clear();
 -              }
 +              $crate::ln::functional_test_utils::check_added_monitors(&$node, $count);
        }
  }
  
@@@ -1045,7 -1013,7 +1048,7 @@@ pub fn create_chan_between_nodes_with_v
        assert_eq!(events_7.len(), 1);
        let (announcement, bs_update) = match events_7[0] {
                MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => {
 -                      (msg, update_msg)
 +                      (msg, update_msg.clone().unwrap())
                },
                _ => panic!("Unexpected event"),
        };
        let as_update = match events_8[0] {
                MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => {
                        assert!(*announcement == *msg);
 +                      let update_msg = update_msg.clone().unwrap();
                        assert_eq!(update_msg.contents.short_channel_id, announcement.contents.short_channel_id);
                        assert_eq!(update_msg.contents.short_channel_id, bs_update.contents.short_channel_id);
                        update_msg
        *node_a.network_chan_count.borrow_mut() += 1;
  
        expect_channel_ready_event(&node_b, &node_a.node.get_our_node_id());
 -      ((*announcement).clone(), (*as_update).clone(), (*bs_update).clone())
 +      ((*announcement).clone(), as_update, bs_update)
  }
  
  pub fn create_announced_chan_between_nodes<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
@@@ -1098,6 -1065,10 +1101,10 @@@ pub fn create_unannounced_chan_between_
        nodes[a].node.handle_funding_signed(&nodes[b].node.get_our_node_id(), &cs_funding_signed);
        check_added_monitors!(nodes[a], 1);
  
+       assert_eq!(nodes[a].tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
+       assert_eq!(nodes[a].tx_broadcaster.txn_broadcasted.lock().unwrap()[0], tx);
+       nodes[a].tx_broadcaster.txn_broadcasted.lock().unwrap().clear();
        let conf_height = core::cmp::max(nodes[a].best_block_info().1 + 1, nodes[b].best_block_info().1 + 1);
        confirm_transaction_at(&nodes[a], &tx, conf_height);
        connect_blocks(&nodes[a], CHAN_CONFIRM_DEPTH - 1);
@@@ -1150,24 -1121,6 +1157,24 @@@ pub fn update_nodes_with_chan_announce<
        }
  }
  
 +pub fn do_check_spends<F: Fn(&bitcoin::blockdata::transaction::OutPoint) -> Option<TxOut>>(tx: &Transaction, get_output: F) {
 +      for outp in tx.output.iter() {
 +              assert!(outp.value >= outp.script_pubkey.dust_value().to_sat(), "Spending tx output didn't meet dust limit");
 +      }
 +      let mut total_value_in = 0;
 +      for input in tx.input.iter() {
 +              total_value_in += get_output(&input.previous_output).unwrap().value;
 +      }
 +      let mut total_value_out = 0;
 +      for output in tx.output.iter() {
 +              total_value_out += output.value;
 +      }
 +      let min_fee = (tx.weight() as u64 + 3) / 4; // One sat per vbyte (ie per weight/4, rounded up)
 +      // Input amount - output amount = fee, so check that out + min_fee is smaller than input
 +      assert!(total_value_out + min_fee <= total_value_in);
 +      tx.verify(get_output).unwrap();
 +}
 +
  #[macro_export]
  macro_rules! check_spends {
        ($tx: expr, $($spends_txn: expr),*) => {
                                assert!(outp.value >= outp.script_pubkey.dust_value().to_sat(), "Input tx output didn't meet dust limit");
                        }
                        )*
 -                      for outp in $tx.output.iter() {
 -                              assert!(outp.value >= outp.script_pubkey.dust_value().to_sat(), "Spending tx output didn't meet dust limit");
 -                      }
                        let get_output = |out_point: &bitcoin::blockdata::transaction::OutPoint| {
                                $(
                                        if out_point.txid == $spends_txn.txid() {
                                )*
                                None
                        };
 -                      let mut total_value_in = 0;
 -                      for input in $tx.input.iter() {
 -                              total_value_in += get_output(&input.previous_output).unwrap().value;
 -                      }
 -                      let mut total_value_out = 0;
 -                      for output in $tx.output.iter() {
 -                              total_value_out += output.value;
 -                      }
 -                      let min_fee = ($tx.weight() as u64 + 3) / 4; // One sat per vbyte (ie per weight/4, rounded up)
 -                      // Input amount - output amount = fee, so check that out + min_fee is smaller than input
 -                      assert!(total_value_out + min_fee <= total_value_in);
 -                      $tx.verify(get_output).unwrap();
 +                      $crate::ln::functional_test_utils::do_check_spends(&$tx, get_output);
                }
        }
  }
@@@ -1232,67 -1199,58 +1239,67 @@@ macro_rules! check_warn_msg 
  
  /// Check that a channel's closing channel update has been broadcasted, and optionally
  /// check whether an error message event has occurred.
 +pub fn check_closed_broadcast(node: &Node, with_error_msg: bool) -> Option<msgs::ErrorMessage> {
 +      let msg_events = node.node.get_and_clear_pending_msg_events();
 +      assert_eq!(msg_events.len(), if with_error_msg { 2 } else { 1 });
 +      match msg_events[0] {
 +              MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
 +                      assert_eq!(msg.contents.flags & 2, 2);
 +              },
 +              _ => panic!("Unexpected event"),
 +      }
 +      if with_error_msg {
 +              match msg_events[1] {
 +                      MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => {
 +                              // TODO: Check node_id
 +                              Some(msg.clone())
 +                      },
 +                      _ => panic!("Unexpected event"),
 +              }
 +      } else { None }
 +}
 +
 +/// Check that a channel's closing channel update has been broadcasted, and optionally
 +/// check whether an error message event has occurred.
 +///
 +/// Don't use this, use the identically-named function instead.
  #[macro_export]
  macro_rules! check_closed_broadcast {
 -      ($node: expr, $with_error_msg: expr) => {{
 -              use $crate::util::events::MessageSendEvent;
 -              use $crate::ln::msgs::ErrorAction;
 +      ($node: expr, $with_error_msg: expr) => {
 +              $crate::ln::functional_test_utils::check_closed_broadcast(&$node, $with_error_msg)
 +      }
 +}
  
 -              let msg_events = $node.node.get_and_clear_pending_msg_events();
 -              assert_eq!(msg_events.len(), if $with_error_msg { 2 } else { 1 });
 -              match msg_events[0] {
 -                      MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
 -                              assert_eq!(msg.contents.flags & 2, 2);
 +/// Check that a channel's closing channel events has been issued
 +pub fn check_closed_event(node: &Node, events_count: usize, expected_reason: ClosureReason, is_check_discard_funding: bool) {
 +      let events = node.node.get_and_clear_pending_events();
 +      assert_eq!(events.len(), events_count, "{:?}", events);
 +      let mut issues_discard_funding = false;
 +      for event in events {
 +              match event {
 +                      Event::ChannelClosed { ref reason, .. } => {
 +                              assert_eq!(*reason, expected_reason);
                        },
 +                      Event::DiscardFunding { .. } => {
 +                              issues_discard_funding = true;
 +                      }
                        _ => panic!("Unexpected event"),
                }
 -              if $with_error_msg {
 -                      match msg_events[1] {
 -                              MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => {
 -                                      // TODO: Check node_id
 -                                      Some(msg.clone())
 -                              },
 -                              _ => panic!("Unexpected event"),
 -                      }
 -              } else { None }
 -      }}
 +      }
 +      assert_eq!(is_check_discard_funding, issues_discard_funding);
  }
  
  /// Check that a channel's closing channel events has been issued
 +///
 +/// Don't use this, use the identically-named function instead.
  #[macro_export]
  macro_rules! check_closed_event {
        ($node: expr, $events: expr, $reason: expr) => {
                check_closed_event!($node, $events, $reason, false);
        };
 -      ($node: expr, $events: expr, $reason: expr, $is_check_discard_funding: expr) => {{
 -              use $crate::util::events::Event;
 -
 -              let events = $node.node.get_and_clear_pending_events();
 -              assert_eq!(events.len(), $events, "{:?}", events);
 -              let expected_reason = $reason;
 -              let mut issues_discard_funding = false;
 -              for event in events {
 -                      match event {
 -                              Event::ChannelClosed { ref reason, .. } => {
 -                                      assert_eq!(*reason, expected_reason);
 -                              },
 -                              Event::DiscardFunding { .. } => {
 -                                      issues_discard_funding = true;
 -                              }
 -                              _ => panic!("Unexpected event"),
 -                      }
 -              }
 -              assert_eq!($is_check_discard_funding, issues_discard_funding);
 -      }}
 +      ($node: expr, $events: expr, $reason: expr, $is_check_discard_funding: expr) => {
 +              $crate::ln::functional_test_utils::check_closed_event(&$node, $events, $reason, $is_check_discard_funding);
 +      }
  }
  
  pub fn close_channel<'a, 'b, 'c>(outbound_node: &Node<'a, 'b, 'c>, inbound_node: &Node<'a, 'b, 'c>, channel_id: &[u8; 32], funding_tx: Transaction, close_inbound_first: bool) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, Transaction) {
@@@ -1394,11 -1352,22 +1401,11 @@@ impl SendEvent 
  }
  
  #[macro_export]
 +/// Don't use this, use the identically-named function instead.
  macro_rules! expect_pending_htlcs_forwardable_conditions {
 -      ($node: expr, $expected_failures: expr) => {{
 -              let expected_failures = $expected_failures;
 -              let events = $node.node.get_and_clear_pending_events();
 -              match events[0] {
 -                      $crate::util::events::Event::PendingHTLCsForwardable { .. } => { },
 -                      _ => panic!("Unexpected event {:?}", events),
 -              };
 -
 -              let count = expected_failures.len() + 1;
 -              assert_eq!(events.len(), count);
 -
 -              if expected_failures.len() > 0 {
 -                      expect_htlc_handling_failed_destinations!(events, expected_failures)
 -              }
 -      }}
 +      ($node: expr, $expected_failures: expr) => {
 +              $crate::ln::functional_test_utils::expect_pending_htlcs_forwardable_conditions($node.node.get_and_clear_pending_events(), &$expected_failures);
 +      }
  }
  
  #[macro_export]
@@@ -1416,49 -1385,27 +1423,49 @@@ macro_rules! expect_htlc_handling_faile
        }}
  }
  
 +/// Checks that an [`Event::PendingHTLCsForwardable`] is available in the given events and, if
 +/// there are any [`Event::HTLCHandlingFailed`] events their [`HTLCDestination`] is included in the
 +/// `expected_failures` set.
 +pub fn expect_pending_htlcs_forwardable_conditions(events: Vec<Event>, expected_failures: &[HTLCDestination]) {
 +      match events[0] {
 +              Event::PendingHTLCsForwardable { .. } => { },
 +              _ => panic!("Unexpected event {:?}", events),
 +      };
 +
 +      let count = expected_failures.len() + 1;
 +      assert_eq!(events.len(), count);
 +
 +      if expected_failures.len() > 0 {
 +              expect_htlc_handling_failed_destinations!(events, expected_failures)
 +      }
 +}
 +
  #[macro_export]
  /// Clears (and ignores) a PendingHTLCsForwardable event
 +///
 +/// Don't use this, call [`expect_pending_htlcs_forwardable_conditions()`] with an empty failure
 +/// set instead.
  macro_rules! expect_pending_htlcs_forwardable_ignore {
 -      ($node: expr) => {{
 -              expect_pending_htlcs_forwardable_conditions!($node, vec![]);
 -      }};
 +      ($node: expr) => {
 +              $crate::ln::functional_test_utils::expect_pending_htlcs_forwardable_conditions($node.node.get_and_clear_pending_events(), &[]);
 +      }
  }
  
  #[macro_export]
  /// Clears (and ignores) PendingHTLCsForwardable and HTLCHandlingFailed events
 +///
 +/// Don't use this, call [`expect_pending_htlcs_forwardable_conditions()`] instead.
  macro_rules! expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore {
 -      ($node: expr, $expected_failures: expr) => {{
 -              expect_pending_htlcs_forwardable_conditions!($node, $expected_failures);
 -      }};
 +      ($node: expr, $expected_failures: expr) => {
 +              $crate::ln::functional_test_utils::expect_pending_htlcs_forwardable_conditions($node.node.get_and_clear_pending_events(), &$expected_failures);
 +      }
  }
  
  #[macro_export]
  /// Handles a PendingHTLCsForwardable event
  macro_rules! expect_pending_htlcs_forwardable {
        ($node: expr) => {{
 -              expect_pending_htlcs_forwardable_ignore!($node);
 +              $crate::ln::functional_test_utils::expect_pending_htlcs_forwardable_conditions($node.node.get_and_clear_pending_events(), &[]);
                $node.node.process_pending_htlc_forwards();
  
                // Ensure process_pending_htlc_forwards is idempotent.
  /// Handles a PendingHTLCsForwardable and HTLCHandlingFailed event
  macro_rules! expect_pending_htlcs_forwardable_and_htlc_handling_failed {
        ($node: expr, $expected_failures: expr) => {{
 -              expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!($node, $expected_failures);
 +              $crate::ln::functional_test_utils::expect_pending_htlcs_forwardable_conditions($node.node.get_and_clear_pending_events(), &$expected_failures);
                $node.node.process_pending_htlc_forwards();
  
                // Ensure process_pending_htlc_forwards is idempotent.
@@@ -1507,10 -1454,10 +1514,10 @@@ macro_rules! commitment_signed_dance 
        };
        ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */, false /* return extra message */, true /* return last RAA */) => {
                {
 -                      check_added_monitors!($node_a, 0);
 +                      $crate::ln::functional_test_utils::check_added_monitors(&$node_a, 0);
                        assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
                        $node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed);
 -                      check_added_monitors!($node_a, 1);
 +                      check_added_monitors(&$node_a, 1);
                        let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards);
                        assert!(extra_msg_option.is_none());
                        bs_revoke_and_ack
                {
                        let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards);
                        $node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack);
 -                      check_added_monitors!($node_a, 1);
 +                      $crate::ln::functional_test_utils::check_added_monitors(&$node_a, 1);
                        extra_msg_option
                }
        };
@@@ -1542,9 -1489,9 +1549,9 @@@ pub fn do_main_commitment_signed_dance(
        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());
@@@ -1596,51 -1543,45 +1603,51 @@@ pub fn do_commitment_signed_dance(node_
  }
  
  /// Get a payment preimage and hash.
 +pub fn get_payment_preimage_hash(recipient: &Node, min_value_msat: Option<u64>, min_final_cltv_expiry_delta: Option<u16>) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
 +      let mut payment_count = recipient.network_payment_count.borrow_mut();
 +      let payment_preimage = PaymentPreimage([*payment_count; 32]);
 +      *payment_count += 1;
 +      let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
 +      let payment_secret = recipient.node.create_inbound_payment_for_hash(payment_hash, min_value_msat, 7200, min_final_cltv_expiry_delta).unwrap();
 +      (payment_preimage, payment_hash, payment_secret)
 +}
 +
 +/// Get a payment preimage and hash.
 +///
 +/// Don't use this, use the identically-named function instead.
  #[macro_export]
  macro_rules! get_payment_preimage_hash {
        ($dest_node: expr) => {
 -              {
 -                      get_payment_preimage_hash!($dest_node, None)
 -              }
 +              get_payment_preimage_hash!($dest_node, None)
        };
        ($dest_node: expr, $min_value_msat: expr) => {
 -              {
 -                      crate::get_payment_preimage_hash!($dest_node, $min_value_msat, None)
 -              }
 +              crate::get_payment_preimage_hash!($dest_node, $min_value_msat, None)
        };
        ($dest_node: expr, $min_value_msat: expr, $min_final_cltv_expiry_delta: expr) => {
 -              {
 -                      use bitcoin::hashes::Hash as _;
 -                      let mut payment_count = $dest_node.network_payment_count.borrow_mut();
 -                      let payment_preimage = $crate::ln::PaymentPreimage([*payment_count; 32]);
 -                      *payment_count += 1;
 -                      let payment_hash = $crate::ln::PaymentHash(
 -                              bitcoin::hashes::sha256::Hash::hash(&payment_preimage.0[..]).into_inner());
 -                      let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, $min_final_cltv_expiry_delta).unwrap();
 -                      (payment_preimage, payment_hash, payment_secret)
 -              }
 +              $crate::ln::functional_test_utils::get_payment_preimage_hash(&$dest_node, $min_value_msat, $min_final_cltv_expiry_delta)
        };
  }
  
 +/// Gets a route from the given sender to the node described in `payment_params`.
 +pub fn get_route(send_node: &Node, payment_params: &PaymentParameters, recv_value: u64, final_cltv_expiry_delta: u32) -> Result<Route, msgs::LightningError> {
 +      let scorer = TestScorer::new();
 +      let keys_manager = TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet);
 +      let random_seed_bytes = keys_manager.get_secure_random_bytes();
 +      router::get_route(
 +              &send_node.node.get_our_node_id(), payment_params, &send_node.network_graph.read_only(),
 +              Some(&send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
 +              recv_value, final_cltv_expiry_delta, send_node.logger, &scorer, &random_seed_bytes
 +      )
 +}
 +
 +/// Gets a route from the given sender to the node described in `payment_params`.
 +///
 +/// Don't use this, use the identically-named function instead.
  #[macro_export]
  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 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(
 -                      &$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(),
 -                      Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
 -                      $recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes
 -              )
 -      }}
 +      ($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {
 +              $crate::ln::functional_test_utils::get_route(&$send_node, &$payment_params, $recv_value, $cltv)
 +      }
  }
  
  #[cfg(test)]
@@@ -1652,9 -1593,8 +1659,9 @@@ macro_rules! get_route_and_payment_has
                $crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV)
        }};
        ($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{
 -              let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value));
 -              let route = $crate::get_route!($send_node, $payment_params, $recv_value, $cltv);
 +              let (payment_preimage, payment_hash, payment_secret) =
 +                      $crate::ln::functional_test_utils::get_payment_preimage_hash(&$recv_node, Some($recv_value), None);
 +              let route = $crate::ln::functional_test_utils::get_route(&$send_node, &$payment_params, $recv_value, $cltv);
                (route.unwrap(), payment_hash, payment_preimage, payment_secret)
        }}
  }
@@@ -1863,18 -1803,24 +1870,18 @@@ macro_rules! expect_payment_failed 
  }
  
  pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>(
 -      node: &'a Node<'b, 'c, 'd>, payment_failed_event: Event, expected_payment_hash: PaymentHash,
 +      payment_failed_events: Vec<Event>, expected_payment_hash: PaymentHash,
        expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e>
  ) {
 -      let expected_payment_id = match payment_failed_event {
 -              Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, network_update, short_channel_id,
 +      if conditions.expected_mpp_parts_remain { assert_eq!(payment_failed_events.len(), 1); } else { assert_eq!(payment_failed_events.len(), 2); }
 +      let expected_payment_id = match &payment_failed_events[0] {
 +              Event::PaymentPathFailed { payment_hash, payment_failed_permanently, payment_id, failure,
                        #[cfg(test)]
                        error_code,
                        #[cfg(test)]
                        error_data, .. } => {
 -                      assert_eq!(payment_hash, expected_payment_hash, "unexpected payment_hash");
 -                      assert_eq!(payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
 -                      assert!(retry.is_some(), "expected retry.is_some()");
 -                      assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
 -                      assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
 -                      if let Some(scid) = short_channel_id {
 -                              assert!(retry.as_ref().unwrap().payment_params.previously_failed_channels.contains(&scid));
 -                      }
 -
 +                      assert_eq!(*payment_hash, expected_payment_hash, "unexpected payment_hash");
 +                      assert_eq!(*payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value");
                        #[cfg(test)]
                        {
                                assert!(error_code.is_some(), "expected error_code.is_some() = true");
                        }
  
                        if let Some(chan_closed) = conditions.expected_blamed_chan_closed {
 -                              match network_update {
 -                                      Some(NetworkUpdate::ChannelUpdateMessage { ref msg }) if !chan_closed => {
 -                                              if let Some(scid) = conditions.expected_blamed_scid {
 -                                                      assert_eq!(msg.contents.short_channel_id, scid);
 -                                              }
 -                                              const CHAN_DISABLED_FLAG: u8 = 2;
 -                                              assert_eq!(msg.contents.flags & CHAN_DISABLED_FLAG, 0);
 -                                      },
 -                                      Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => {
 -                                              if let Some(scid) = conditions.expected_blamed_scid {
 -                                                      assert_eq!(short_channel_id, scid);
 -                                              }
 -                                              assert!(is_permanent);
 -                                      },
 -                                      Some(_) => panic!("Unexpected update type"),
 -                                      None => panic!("Expected update"),
 -                              }
 +                              if let PathFailure::OnPath { network_update: Some(upd) } = failure {
 +                                      match upd {
 +                                              NetworkUpdate::ChannelUpdateMessage { ref msg } if !chan_closed => {
 +                                                      if let Some(scid) = conditions.expected_blamed_scid {
 +                                                              assert_eq!(msg.contents.short_channel_id, scid);
 +                                                      }
 +                                                      const CHAN_DISABLED_FLAG: u8 = 2;
 +                                                      assert_eq!(msg.contents.flags & CHAN_DISABLED_FLAG, 0);
 +                                              },
 +                                              NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } if chan_closed => {
 +                                                      if let Some(scid) = conditions.expected_blamed_scid {
 +                                                              assert_eq!(*short_channel_id, scid);
 +                                                      }
 +                                                      assert!(is_permanent);
 +                                              },
 +                                              _ => panic!("Unexpected update type"),
 +                                      }
 +                              } else { panic!("Expected network update"); }
                        }
  
                        payment_id.unwrap()
                _ => panic!("Unexpected event"),
        };
        if !conditions.expected_mpp_parts_remain {
 -              node.node.abandon_payment(expected_payment_id);
 -              let events = node.node.get_and_clear_pending_events();
 -              assert_eq!(events.len(), 1);
 -              match events[0] {
 +              match &payment_failed_events[1] {
                        Event::PaymentFailed { ref payment_hash, ref payment_id } => {
                                assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash");
                                assert_eq!(*payment_id, expected_payment_id);
@@@ -1925,8 -1873,9 +1932,8 @@@ pub fn expect_payment_failed_conditions
        node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_payment_failed_permanently: bool,
        conditions: PaymentFailedConditions<'e>
  ) {
 -      let mut events = node.node.get_and_clear_pending_events();
 -      assert_eq!(events.len(), 1);
 -      expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_payment_failed_permanently, conditions);
 +      let events = node.node.get_and_clear_pending_events();
 +      expect_payment_failed_conditions_event(events, expected_payment_hash, expected_payment_failed_permanently, conditions);
  }
  
  pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
@@@ -1997,7 -1946,8 +2004,7 @@@ pub fn pass_along_route<'a, 'b, 'c>(ori
        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;
@@@ -2056,8 -2006,9 +2063,8 @@@ pub fn do_claim_payment_along_route<'a
        } 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;
                }
        }
  
@@@ -2161,7 -2112,7 +2168,7 @@@ pub const TEST_FINAL_CLTV: u32 = 70
  pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
        let 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 route = get_route!(origin_node, payment_params, recv_value, TEST_FINAL_CLTV).unwrap();
 +      let route = get_route(origin_node, &payment_params, recv_value, TEST_FINAL_CLTV).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
@@@ -2176,11 -2127,11 +2183,11 @@@ pub fn route_over_limit<'a, 'b, 'c>(ori
        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();
 -      let route = get_route(
 +      let route = router::get_route(
                &origin_node.node.get_our_node_id(), &payment_params, &network_graph,
                None, recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer, &random_seed_bytes).unwrap();
        assert_eq!(route.paths.len(), 1);
                assert!(err.contains("Cannot send value that would put us over the max HTLC value in flight our peer will accept")));
  }
  
 -pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
 -      let our_payment_preimage = route_payment(&origin, expected_route, recv_value).0;
 -      claim_payment(&origin, expected_route, our_payment_preimage);
 +pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
 +      let res = route_payment(&origin, expected_route, recv_value);
 +      claim_payment(&origin, expected_route, res.0);
 +      res
  }
  
  pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) {
  }
  
  pub fn pass_failed_payment_back<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) {
 -      let expected_payment_id = pass_failed_payment_back_no_abandon(origin_node, expected_paths_slice, skip_last, our_payment_hash);
 -      if !skip_last {
 -              origin_node.node.abandon_payment(expected_payment_id.unwrap());
 -              let events = origin_node.node.get_and_clear_pending_events();
 -              assert_eq!(events.len(), 1);
 -              match events[0] {
 -                      Event::PaymentFailed { ref payment_hash, ref payment_id } => {
 -                              assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
 -                              assert_eq!(*payment_id, expected_payment_id.unwrap());
 -                      }
 -                      _ => panic!("Unexpected second event"),
 -              }
 -      }
 -}
 -
 -pub fn pass_failed_payment_back_no_abandon<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash) -> Option<PaymentId> {
        let mut expected_paths: Vec<_> = expected_paths_slice.iter().collect();
        check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
  
        per_path_msgs.sort_unstable_by(|(_, node_id_a), (_, node_id_b)| node_id_a.cmp(node_id_b));
        expected_paths.sort_unstable_by(|path_a, path_b| path_a[path_a.len() - 2].node.get_our_node_id().cmp(&path_b[path_b.len() - 2].node.get_our_node_id()));
  
 -      let mut expected_payment_id = None;
 -
        for (i, (expected_route, (path_msgs, next_hop))) in expected_paths.iter().zip(per_path_msgs.drain(..)).enumerate() {
                let mut next_msgs = Some(path_msgs);
                let mut expected_next_node = next_hop;
                        assert!(origin_node.node.get_and_clear_pending_msg_events().is_empty());
                        commitment_signed_dance!(origin_node, prev_node, next_msgs.as_ref().unwrap().1, false);
                        let events = origin_node.node.get_and_clear_pending_events();
 -                      assert_eq!(events.len(), 1);
 -                      expected_payment_id = Some(match events[0] {
 -                              Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => {
 +                      if i == expected_paths.len() - 1 { assert_eq!(events.len(), 2); } else { assert_eq!(events.len(), 1); }
 +
 +                      let expected_payment_id = match events[0] {
 +                              Event::PaymentPathFailed { payment_hash, payment_failed_permanently, ref path, ref payment_id, .. } => {
                                        assert_eq!(payment_hash, our_payment_hash);
                                        assert!(payment_failed_permanently);
 -                                      assert_eq!(all_paths_failed, i == expected_paths.len() - 1);
                                        for (idx, hop) in expected_route.iter().enumerate() {
                                                assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey);
                                        }
                                        payment_id.unwrap()
                                },
                                _ => panic!("Unexpected event"),
 -                      });
 +                      };
 +                      if i == expected_paths.len() - 1 {
 +                              match events[1] {
 +                                      Event::PaymentFailed { ref payment_hash, ref payment_id } => {
 +                                              assert_eq!(*payment_hash, our_payment_hash, "unexpected second payment_hash");
 +                                              assert_eq!(*payment_id, expected_payment_id);
 +                                      }
 +                                      _ => panic!("Unexpected second event"),
 +                              }
 +                      }
                }
        }
  
        assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_events().is_empty());
        assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
        check_added_monitors!(expected_paths[0].last().unwrap(), 0);
 -
 -      expected_payment_id
  }
  
  pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], our_payment_hash: PaymentHash)  {
@@@ -2331,9 -2292,8 +2338,9 @@@ pub fn create_chanmon_cfgs(node_count: 
                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
@@@ -2344,14 -2304,14 +2351,14 @@@ pub fn create_node_cfgs<'a>(node_count
  
        for i in 0..node_count {
                let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[i].chain_source), &chanmon_cfgs[i].tx_broadcaster, &chanmon_cfgs[i].logger, &chanmon_cfgs[i].fee_estimator, &chanmon_cfgs[i].persister, &chanmon_cfgs[i].keys_manager);
 -              let network_graph = Arc::new(NetworkGraph::new(chanmon_cfgs[i].chain_source.genesis_hash, &chanmon_cfgs[i].logger));
 +              let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[i].logger));
                let seed = [i as u8; 32];
                nodes.push(NodeCfg {
                        chain_source: &chanmon_cfgs[i].chain_source,
                        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,
@@@ -2385,7 -2345,7 +2392,7 @@@ pub fn create_node_chanmgrs<'a, 'b>(nod
                let network = Network::Testnet;
                let params = ChainParameters {
                        network,
 -                      best_block: BestBlock::from_genesis(network),
 +                      best_block: BestBlock::from_network(network),
                };
                let node = ChannelManager::new(cfgs[i].fee_estimator, &cfgs[i].chain_monitor, cfgs[i].tx_broadcaster, &cfgs[i].router, cfgs[i].logger, cfgs[i].keys_manager,
                        cfgs[i].keys_manager, cfgs[i].keys_manager, if node_config[i].is_some() { node_config[i].clone().unwrap() } else { test_default_channel_config() }, params);
@@@ -2418,8 -2378,8 +2425,8 @@@ pub fn create_network<'a, 'b: 'a, 'c: '
  
        for i in 0..node_count {
                for j in (i+1)..node_count {
 -                      nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &msgs::Init { features: nodes[j].override_init_features.borrow().clone().unwrap_or_else(|| nodes[j].node.init_features()), remote_network_address: None }).unwrap();
 -                      nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &msgs::Init { features: nodes[i].override_init_features.borrow().clone().unwrap_or_else(|| nodes[i].node.init_features()), remote_network_address: None }).unwrap();
 +                      nodes[i].node.peer_connected(&nodes[j].node.get_our_node_id(), &msgs::Init { features: nodes[j].override_init_features.borrow().clone().unwrap_or_else(|| nodes[j].node.init_features()), remote_network_address: None }, true).unwrap();
 +                      nodes[j].node.peer_connected(&nodes[i].node.get_our_node_id(), &msgs::Init { features: nodes[i].override_init_features.borrow().clone().unwrap_or_else(|| nodes[i].node.init_features()), remote_network_address: None }, false).unwrap();
                }
        }
  
@@@ -2702,9 -2662,9 +2709,9 @@@ macro_rules! handle_chan_reestablish_ms
  /// pending_htlc_adds includes both the holding cell and in-flight update_add_htlcs, whereas
  /// for claims/fails they are separated out.
  pub fn reconnect_nodes<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, send_channel_ready: (bool, bool), pending_htlc_adds: (i64, i64), pending_htlc_claims: (usize, usize), pending_htlc_fails: (usize, usize), pending_cell_htlc_claims: (usize, usize), pending_cell_htlc_fails: (usize, usize), pending_raa: (bool, bool))  {
 -      node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init { features: node_b.node.init_features(), remote_network_address: None }).unwrap();
 +      node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init { features: node_b.node.init_features(), remote_network_address: None }, true).unwrap();
        let reestablish_1 = get_chan_reestablish_msgs!(node_a, node_b);
 -      node_b.node.peer_connected(&node_a.node.get_our_node_id(), &msgs::Init { features: node_a.node.init_features(), remote_network_address: None }).unwrap();
 +      node_b.node.peer_connected(&node_a.node.get_our_node_id(), &msgs::Init { features: node_a.node.init_features(), remote_network_address: None }, false).unwrap();
        let reestablish_2 = get_chan_reestablish_msgs!(node_b, node_a);
  
        if send_channel_ready.0 {