Merge pull request #1027 from TheBlueMatt/2021-07-check-dust
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sat, 20 Nov 2021 03:26:24 +0000 (03:26 +0000)
committerGitHub <noreply@github.com>
Sat, 20 Nov 2021 03:26:24 +0000 (03:26 +0000)
Check all outputs meet the dust threshold in `check_spends!()`

1  2 
lightning/src/chain/onchaintx.rs
lightning/src/ln/functional_test_utils.rs

index b4f5438adfb144645aa3acff008580a7fd74a8fc,8d70c7722ca15d1ec063119c68e61596aaea2aa3..765a9c091815379104b42c05578bda5b9622f7a5
@@@ -365,14 -365,6 +365,14 @@@ impl<ChannelSigner: Sign> OnchainTxHand
                }
        }
  
 +      pub(crate) fn get_prev_holder_commitment_to_self_value(&self) -> Option<u64> {
 +              self.prev_holder_commitment.as_ref().map(|commitment| commitment.to_broadcaster_value_sat())
 +      }
 +
 +      pub(crate) fn get_cur_holder_commitment_to_self_value(&self) -> u64 {
 +              self.holder_commitment.to_broadcaster_value_sat()
 +      }
 +
        /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize coutnerparty onchain) lays on the assumption of claim transactions getting confirmed before timelock expiration
        /// (CSV or CLTV following cases). In case of high-fee spikes, claim tx may stuck in the mempool, so you need to bump its feerate quickly using Replace-By-Fee or Child-Pay-For-Parent.
        /// Panics if there are signing errors, because signing operations in reaction to on-chain events
                let new_timer = Some(cached_request.get_height_timer(cur_height));
                if cached_request.is_malleable() {
                        let predicted_weight = cached_request.package_weight(&self.destination_script);
-                       if let Some((output_value, new_feerate)) = cached_request.compute_package_output(predicted_weight, fee_estimator, logger) {
+                       if let Some((output_value, new_feerate)) =
+                                       cached_request.compute_package_output(predicted_weight, self.destination_script.dust_value().as_sat(), fee_estimator, logger) {
                                assert!(new_feerate != 0);
  
                                let transaction = cached_request.finalize_package(self, output_value, self.destination_script.clone(), logger).unwrap();
index e9e36e607ec45950dc4ebadab6a8c940c756f114,6b46f62813b0baa23f13fe1b32425d9f9720f9fd..9b435516b4aea51b47b62a24801f950176dc3edc
@@@ -14,9 -14,9 +14,9 @@@ use chain::{BestBlock, Confirm, Listen
  use chain::channelmonitor::ChannelMonitor;
  use chain::transaction::OutPoint;
  use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 -use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
 -use routing::router::{Route, get_route};
 +use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId};
  use routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
 +use routing::router::{Payee, Route, get_route};
  use ln::features::{InitFeatures, InvoiceFeatures};
  use ln::msgs;
  use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler};
@@@ -189,7 -189,6 +189,7 @@@ pub struct TestChanMonCfg 
        pub persister: test_utils::TestPersister,
        pub logger: test_utils::TestLogger,
        pub keys_manager: test_utils::TestKeysInterface,
 +      pub network_graph: NetworkGraph,
  }
  
  pub struct NodeCfg<'a> {
        pub chain_monitor: test_utils::TestChainMonitor<'a>,
        pub keys_manager: &'a test_utils::TestKeysInterface,
        pub logger: &'a test_utils::TestLogger,
 +      pub network_graph: &'a NetworkGraph,
        pub node_seed: [u8; 32],
        pub features: InitFeatures,
  }
@@@ -210,8 -208,7 +210,8 @@@ pub struct Node<'a, 'b: 'a, 'c: 'b> 
        pub chain_monitor: &'b test_utils::TestChainMonitor<'c>,
        pub keys_manager: &'b test_utils::TestKeysInterface,
        pub node: &'a ChannelManager<EnforcingSigner, &'b TestChainMonitor<'c>, &'c test_utils::TestBroadcaster, &'b test_utils::TestKeysInterface, &'c test_utils::TestFeeEstimator, &'c test_utils::TestLogger>,
 -      pub net_graph_msg_handler: NetGraphMsgHandler<&'c test_utils::TestChainSource, &'c test_utils::TestLogger>,
 +      pub network_graph: &'c NetworkGraph,
 +      pub net_graph_msg_handler: NetGraphMsgHandler<&'c NetworkGraph, &'c test_utils::TestChainSource, &'c test_utils::TestLogger>,
        pub node_seed: [u8; 32],
        pub network_payment_count: Rc<RefCell<u8>>,
        pub network_chan_count: Rc<RefCell<u32>>,
@@@ -242,11 -239,12 +242,11 @@@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 
                        // Check that if we serialize the Router, we can deserialize it again.
                        {
                                let mut w = test_utils::TestVecWriter(Vec::new());
 -                              let network_graph_ser = self.net_graph_msg_handler.network_graph.read().unwrap();
 -                              network_graph_ser.write(&mut w).unwrap();
 +                              self.network_graph.write(&mut w).unwrap();
                                let network_graph_deser = <NetworkGraph>::read(&mut io::Cursor::new(&w.0)).unwrap();
 -                              assert!(network_graph_deser == *self.net_graph_msg_handler.network_graph.read().unwrap());
 -                              let net_graph_msg_handler = NetGraphMsgHandler::from_net_graph(
 -                                      Some(self.chain_source), self.logger, network_graph_deser
 +                              assert!(network_graph_deser == *self.network_graph);
 +                              let net_graph_msg_handler = NetGraphMsgHandler::new(
 +                                      &network_graph_deser, Some(self.chain_source), self.logger
                                );
                                let mut chan_progress = 0;
                                loop {
                        let feeest = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
                        let mut deserialized_monitors = Vec::new();
                        {
 -                              let old_monitors = self.chain_monitor.chain_monitor.monitors.read().unwrap();
 -                              for (_, old_monitor) in old_monitors.iter() {
 +                              for outpoint in self.chain_monitor.chain_monitor.list_monitors() {
                                        let mut w = test_utils::TestVecWriter(Vec::new());
 -                                      old_monitor.write(&mut w).unwrap();
 +                                      self.chain_monitor.chain_monitor.get_monitor(outpoint).unwrap().write(&mut w).unwrap();
                                        let (_, deserialized_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
                                                &mut io::Cursor::new(&w.0), self.keys_manager).unwrap();
                                        deserialized_monitors.push(deserialized_monitor);
@@@ -438,35 -437,20 +438,35 @@@ macro_rules! get_feerate 
        }
  }
  
 -/// Returns any local commitment transactions for the channel.
 +/// Returns a channel monitor given a channel id, making some naive assumptions
  #[macro_export]
 -macro_rules! get_local_commitment_txn {
 +macro_rules! get_monitor {
        ($node: expr, $channel_id: expr) => {
                {
 -                      let monitors = $node.chain_monitor.chain_monitor.monitors.read().unwrap();
 -                      let mut commitment_txn = None;
 -                      for (funding_txo, monitor) in monitors.iter() {
 -                              if funding_txo.to_channel_id() == $channel_id {
 -                                      commitment_txn = Some(monitor.unsafe_get_latest_holder_commitment_txn(&$node.logger));
 +                      use bitcoin::hashes::Hash;
 +                      let mut monitor = None;
 +                      // Assume funding vout is either 0 or 1 blindly
 +                      for index in 0..2 {
 +                              if let Ok(mon) = $node.chain_monitor.chain_monitor.get_monitor(
 +                                      $crate::chain::transaction::OutPoint {
 +                                              txid: bitcoin::Txid::from_slice(&$channel_id[..]).unwrap(), index
 +                                      })
 +                              {
 +                                      monitor = Some(mon);
                                        break;
                                }
                        }
 -                      commitment_txn.unwrap()
 +                      monitor.unwrap()
 +              }
 +      }
 +}
 +
 +/// Returns any local commitment transactions for the channel.
 +#[macro_export]
 +macro_rules! get_local_commitment_txn {
 +      ($node: expr, $channel_id: expr) => {
 +              {
 +                      $crate::get_monitor!($node, $channel_id).unsafe_get_latest_holder_commitment_txn(&$node.logger)
                }
        }
  }
@@@ -483,9 -467,9 +483,9 @@@ macro_rules! unwrap_send_err 
                                        _ => panic!(),
                                }
                        },
 -                      &Err(PaymentSendFailure::PartialFailure(ref fails)) if !$all_failed => {
 -                              assert_eq!(fails.len(), 1);
 -                              match fails[0] {
 +                      &Err(PaymentSendFailure::PartialFailure { ref results, .. }) if !$all_failed => {
 +                              assert_eq!(results.len(), 1);
 +                              match results[0] {
                                        Err($type) => { $check },
                                        _ => panic!(),
                                }
@@@ -528,12 -512,11 +528,12 @@@ pub fn create_funding_transaction<'a, '
  }
  
  pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> Transaction {
 -      node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
 +      let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
        node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), a_flags, &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id()));
        node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), b_flags, &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id()));
  
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, channel_value, 42);
 +      assert_eq!(temporary_channel_id, create_chan_id);
  
        node_a.node.funding_transaction_generated(&temporary_channel_id, tx.clone()).unwrap();
        check_added_monitors!(node_a, 0);
@@@ -707,6 -690,14 +707,14 @@@ pub fn update_nodes_with_chan_announce<
  macro_rules! check_spends {
        ($tx: expr, $($spends_txn: expr),*) => {
                {
+                       $(
+                       for outp in $spends_txn.output.iter() {
+                               assert!(outp.value >= outp.script_pubkey.dust_value().as_sat(), "Input tx output didn't meet dust limit");
+                       }
+                       )*
+                       for outp in $tx.output.iter() {
+                               assert!(outp.value >= outp.script_pubkey.dust_value().as_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() {
@@@ -760,16 -751,16 +768,16 @@@ macro_rules! get_closing_signed_broadca
  #[macro_export]
  macro_rules! check_closed_broadcast {
        ($node: expr, $with_error_msg: expr) => {{
 -              let events = $node.node.get_and_clear_pending_msg_events();
 -              assert_eq!(events.len(), if $with_error_msg { 2 } else { 1 });
 -              match events[0] {
 +              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 events[1] {
 +                      match msg_events[1] {
                                MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => {
                                        // TODO: Check node_id
                                        Some(msg.clone())
        }}
  }
  
 +/// Check that a channel's closing channel events has been issued
 +#[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) => {{
 +              let events = $node.node.get_and_clear_pending_events();
 +              assert_eq!(events.len(), $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);
 +      }}
 +}
 +
  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) {
        let (node_a, broadcaster_a, struct_a) = if close_inbound_first { (&inbound_node.node, &inbound_node.tx_broadcaster, inbound_node) } else { (&outbound_node.node, &outbound_node.tx_broadcaster, outbound_node) };
        let (node_b, broadcaster_b, struct_b) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster, outbound_node) } else { (&inbound_node.node, &inbound_node.tx_broadcaster, inbound_node) };
@@@ -990,16 -955,10 +998,16 @@@ macro_rules! commitment_signed_dance 
  macro_rules! get_payment_preimage_hash {
        ($dest_node: expr) => {
                {
 -                      let payment_preimage = PaymentPreimage([*$dest_node.network_payment_count.borrow(); 32]);
 -                      *$dest_node.network_payment_count.borrow_mut() += 1;
 +                      get_payment_preimage_hash!($dest_node, None)
 +              }
 +      };
 +      ($dest_node: expr, $min_value_msat: expr) => {
 +              {
 +                      let mut payment_count = $dest_node.network_payment_count.borrow_mut();
 +                      let payment_preimage = PaymentPreimage([*payment_count; 32]);
 +                      *payment_count += 1;
                        let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner());
 -                      let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, None, 7200, 0).unwrap();
 +                      let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, 0).unwrap();
                        (payment_preimage, payment_hash, payment_secret)
                }
        }
  #[cfg(test)]
  macro_rules! get_route_and_payment_hash {
        ($send_node: expr, $recv_node: expr, $recv_value: expr) => {{
 -              let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node);
 -              let net_graph_msg_handler = &$send_node.net_graph_msg_handler;
 -              let route = get_route(&$send_node.node.get_our_node_id(),
 -                      &net_graph_msg_handler.network_graph.read().unwrap(),
 -                      &$recv_node.node.get_our_node_id(), None, None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, $send_node.logger).unwrap();
 +              get_route_and_payment_hash!($send_node, $recv_node, vec![], $recv_value, TEST_FINAL_CLTV)
 +      }};
 +      ($send_node: expr, $recv_node: expr, $last_hops: expr, $recv_value: expr, $cltv: expr) => {{
 +              let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node, Some($recv_value));
 +              let payee = $crate::routing::router::Payee::from_node_id($recv_node.node.get_our_node_id())
 +                      .with_features($crate::ln::features::InvoiceFeatures::known())
 +                      .with_route_hints($last_hops);
 +              let scorer = ::util::test_utils::TestScorer::with_fixed_penalty(0);
 +              let route = ::routing::router::get_route(
 +                      &$send_node.node.get_our_node_id(), &payee, $send_node.network_graph,
 +                      Some(&$send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
 +                      $recv_value, $cltv, $send_node.logger, &scorer
 +              ).unwrap();
                (route, payment_hash, payment_preimage, payment_secret)
        }}
  }
@@@ -1043,20 -994,6 +1051,20 @@@ macro_rules! expect_pending_htlcs_forwa
        }}
  }
  
 +#[cfg(test)]
 +macro_rules! expect_pending_htlcs_forwardable_from_events {
 +      ($node: expr, $events: expr, $ignore: expr) => {{
 +              assert_eq!($events.len(), 1);
 +              match $events[0] {
 +                      Event::PendingHTLCsForwardable { .. } => { },
 +                      _ => panic!("Unexpected event"),
 +              };
 +              if $ignore {
 +                      $node.node.process_pending_htlc_forwards();
 +              }
 +      }}
 +}
 +
  #[cfg(any(test, feature = "unstable"))]
  macro_rules! expect_payment_received {
        ($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => {
  
  macro_rules! expect_payment_sent {
        ($node: expr, $expected_payment_preimage: expr) => {
 +              expect_payment_sent!($node, $expected_payment_preimage, None::<u64>);
 +      };
 +      ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr) => {
                let events = $node.node.get_and_clear_pending_events();
 +              let expected_payment_hash = PaymentHash(Sha256::hash(&$expected_payment_preimage.0).into_inner());
                assert_eq!(events.len(), 1);
                match events[0] {
 -                      Event::PaymentSent { ref payment_preimage } => {
 +                      Event::PaymentSent { payment_id: _, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
                                assert_eq!($expected_payment_preimage, *payment_preimage);
 +                              assert_eq!(expected_payment_hash, *payment_hash);
 +                              assert!(fee_paid_msat.is_some());
 +                              if $expected_fee_msat_opt.is_some() {
 +                                      assert_eq!(*fee_paid_msat, $expected_fee_msat_opt);
 +                              }
                        },
                        _ => panic!("Unexpected event"),
                }
@@@ -1116,30 -1044,22 +1124,30 @@@ macro_rules! expect_payment_forwarded 
  }
  
  #[cfg(test)]
 -macro_rules! expect_payment_failure_chan_update {
 -      ($node: expr, $scid: expr, $chan_closed: expr) => {
 -              let events = $node.node.get_and_clear_pending_msg_events();
 +macro_rules! expect_payment_failed_with_update {
 +      ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => {
 +              let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
                match events[0] {
 -                      MessageSendEvent::PaymentFailureNetworkUpdate { ref update } => {
 -                              match update {
 -                                      &HTLCFailChannelUpdate::ChannelUpdateMessage { ref msg } if !$chan_closed => {
 +                      Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, ref path, ref retry, .. } => {
 +                              assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
 +                              assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest 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().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
 +                              assert!(error_code.is_some(), "expected error_code.is_some() = true");
 +                              assert!(error_data.is_some(), "expected error_data.is_some() = true");
 +                              match network_update {
 +                                      &Some(NetworkUpdate::ChannelUpdateMessage { ref msg }) if !$chan_closed => {
                                                assert_eq!(msg.contents.short_channel_id, $scid);
                                                assert_eq!(msg.contents.flags & 2, 0);
                                        },
 -                                      &HTLCFailChannelUpdate::ChannelClosed { short_channel_id, is_permanent } if $chan_closed => {
 +                                      &Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if $chan_closed => {
                                                assert_eq!(short_channel_id, $scid);
                                                assert!(is_permanent);
                                        },
 -                                      _ => panic!("Unexpected update type"),
 +                                      Some(_) => panic!("Unexpected update type"),
 +                                      None => panic!("Expected update"),
                                }
                        },
                        _ => panic!("Unexpected event"),
@@@ -1153,12 -1073,9 +1161,12 @@@ macro_rules! expect_payment_failed 
                let events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
                match events[0] {
 -                      Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data } => {
 +                      Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, ref path, ref retry, .. } => {
                                assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash");
                                assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest 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().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
                                assert!(error_code.is_some(), "expected error_code.is_some() = true");
                                assert!(error_data.is_some(), "expected error_data.is_some() = true");
                                $(
        }
  }
  
 -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) {
 -      origin_node.node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
 +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 {
 +      let payment_id = origin_node.node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
        check_added_monitors!(origin_node, expected_paths.len());
        pass_along_route(origin_node, expected_paths, recv_value, our_payment_hash, our_payment_secret);
 +      payment_id
  }
  
  pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_received_expected: bool, expected_preimage: Option<PaymentPreimage>) {
@@@ -1238,21 -1154,19 +1246,21 @@@ pub fn pass_along_route<'a, 'b, 'c>(ori
        }
  }
  
 -pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
 +pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret, PaymentId) {
        let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(expected_route.last().unwrap());
 -      send_along_route_with_secret(origin_node, route, &[expected_route], recv_value, our_payment_hash, our_payment_secret);
 -      (our_payment_preimage, our_payment_hash, our_payment_secret)
 +      let payment_id = send_along_route_with_secret(origin_node, route, &[expected_route], recv_value, our_payment_hash, our_payment_secret);
 +      (our_payment_preimage, our_payment_hash, our_payment_secret, payment_id)
  }
  
 -pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
 +pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) -> u64 {
        for path in expected_paths.iter() {
                assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
        }
        assert!(expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage));
        check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
  
 +      let mut expected_total_fee_msat = 0;
 +
        macro_rules! msgs_from_ev {
                ($ev: expr) => {
                        match $ev {
                                        $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
                                        let fee = $node.node.channel_state.lock().unwrap().by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap().config.forwarding_fee_base_msat;
                                        expect_payment_forwarded!($node, Some(fee as u64), false);
 +                                      expected_total_fee_msat += fee as u64;
                                        check_added_monitors!($node, 1);
                                        let new_next_msgs = if $new_msgs {
                                                let events = $node.node.get_and_clear_pending_msg_events();
  
                if !skip_last {
                        last_update_fulfill_dance!(origin_node, expected_route.first().unwrap());
 -                      expect_payment_sent!(origin_node, our_payment_preimage);
                }
        }
 +      expected_total_fee_msat
 +}
 +pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
 +      let expected_total_fee_msat = do_claim_payment_along_route(origin_node, expected_paths, skip_last, our_payment_preimage);
 +      if !skip_last {
 +              expect_payment_sent!(origin_node, our_payment_preimage, Some(expected_total_fee_msat));
 +      }
  }
  
  pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_preimage: PaymentPreimage) {
  pub const TEST_FINAL_CLTV: u32 = 70;
  
  pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
 -      let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
 -      let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(),
 -              &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()),
 -              Some(&origin_node.node.list_usable_channels().iter().collect::<Vec<_>>()), &[],
 -              recv_value, TEST_FINAL_CLTV, origin_node.logger).unwrap();
 +      let payee = Payee::from_node_id(expected_route.last().unwrap().node.get_our_node_id())
 +              .with_features(InvoiceFeatures::known());
 +      let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 +      let route = get_route(
 +              &origin_node.node.get_our_node_id(), &payee, &origin_node.network_graph,
 +              Some(&origin_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
 +              recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
                assert_eq!(hop.pubkey, node.node.get_our_node_id());
        }
  
 -      send_along_route(origin_node, route, expected_route, recv_value)
 +      let res = send_along_route(origin_node, route, expected_route, recv_value);
 +      (res.0, res.1, res.2)
  }
  
  pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
 -      let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
 -      let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), recv_value, TEST_FINAL_CLTV, origin_node.logger).unwrap();
 +      let payee = Payee::from_node_id(expected_route.last().unwrap().node.get_our_node_id())
 +              .with_features(InvoiceFeatures::known());
 +      let scorer = test_utils::TestScorer::with_fixed_penalty(0);
 +      let route = get_route(&origin_node.node.get_our_node_id(), &payee, origin_node.network_graph, None, recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
@@@ -1388,100 -1290,77 +1396,100 @@@ pub fn send_payment<'a, 'b, 'c>(origin
        claim_payment(&origin, expected_route, our_payment_preimage);
  }
  
 -pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], skip_last: bool, our_payment_hash: PaymentHash)  {
 -      assert!(expected_route.last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
 -      expect_pending_htlcs_forwardable!(expected_route.last().unwrap());
 -      check_added_monitors!(expected_route.last().unwrap(), 1);
 +pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths_slice: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_hash: PaymentHash)  {
 +      let mut expected_paths: Vec<_> = expected_paths_slice.iter().collect();
 +      for path in expected_paths.iter() {
 +              assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
 +      }
 +      assert!(expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
 +      expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap());
 +      check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
  
 -      let mut next_msgs: Option<(msgs::UpdateFailHTLC, msgs::CommitmentSigned)> = None;
 -      macro_rules! update_fail_dance {
 -              ($node: expr, $prev_node: expr, $last_node: expr) => {
 -                      {
 -                              $node.node.handle_update_fail_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
 -                              commitment_signed_dance!($node, $prev_node, next_msgs.as_ref().unwrap().1, !$last_node);
 -                              if skip_last && $last_node {
 -                                      expect_pending_htlcs_forwardable!($node);
 +      let mut per_path_msgs: Vec<((msgs::UpdateFailHTLC, msgs::CommitmentSigned), PublicKey)> = Vec::with_capacity(expected_paths.len());
 +      let events = expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events();
 +      assert_eq!(events.len(), expected_paths.len());
 +      for ev in events.iter() {
 +              let (update_fail, commitment_signed, node_id) = match ev {
 +                      &MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => {
 +                              assert!(update_add_htlcs.is_empty());
 +                              assert!(update_fulfill_htlcs.is_empty());
 +                              assert_eq!(update_fail_htlcs.len(), 1);
 +                              assert!(update_fail_malformed_htlcs.is_empty());
 +                              assert!(update_fee.is_none());
 +                              (update_fail_htlcs[0].clone(), commitment_signed.clone(), node_id.clone())
 +                      },
 +                      _ => panic!("Unexpected event"),
 +              };
 +              per_path_msgs.push(((update_fail, commitment_signed), node_id));
 +      }
 +      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()));
 +
 +      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;
 +              let mut prev_node = expected_route.last().unwrap();
 +
 +              for (idx, node) in expected_route.iter().rev().enumerate().skip(1) {
 +                      assert_eq!(expected_next_node, node.node.get_our_node_id());
 +                      let update_next_node = !skip_last || idx != expected_route.len() - 1;
 +                      if next_msgs.is_some() {
 +                              node.node.handle_update_fail_htlc(&prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
 +                              commitment_signed_dance!(node, prev_node, next_msgs.as_ref().unwrap().1, update_next_node);
 +                              if !update_next_node {
 +                                      expect_pending_htlcs_forwardable!(node);
                                }
                        }
 -              }
 -      }
 +                      let events = node.node.get_and_clear_pending_msg_events();
 +                      if update_next_node {
 +                              assert_eq!(events.len(), 1);
 +                              match events[0] {
 +                                      MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => {
 +                                              assert!(update_add_htlcs.is_empty());
 +                                              assert!(update_fulfill_htlcs.is_empty());
 +                                              assert_eq!(update_fail_htlcs.len(), 1);
 +                                              assert!(update_fail_malformed_htlcs.is_empty());
 +                                              assert!(update_fee.is_none());
 +                                              expected_next_node = node_id.clone();
 +                                              next_msgs = Some((update_fail_htlcs[0].clone(), commitment_signed.clone()));
 +                                      },
 +                                      _ => panic!("Unexpected event"),
 +                              }
 +                      } else {
 +                              assert!(events.is_empty());
 +                      }
 +                      if !skip_last && idx == expected_route.len() - 1 {
 +                              assert_eq!(expected_next_node, origin_node.node.get_our_node_id());
 +                      }
  
 -      let mut expected_next_node = expected_route.last().unwrap().node.get_our_node_id();
 -      let mut prev_node = expected_route.last().unwrap();
 -      for (idx, node) in expected_route.iter().rev().enumerate() {
 -              assert_eq!(expected_next_node, node.node.get_our_node_id());
 -              if next_msgs.is_some() {
 -                      // We may be the "last node" for the purpose of the commitment dance if we're
 -                      // skipping the last node (implying it is disconnected) and we're the
 -                      // second-to-last node!
 -                      update_fail_dance!(node, prev_node, skip_last && idx == expected_route.len() - 1);
 +                      prev_node = node;
                }
  
 -              let events = node.node.get_and_clear_pending_msg_events();
 -              if !skip_last || idx != expected_route.len() - 1 {
 +              if !skip_last {
 +                      let prev_node = expected_route.first().unwrap();
 +                      origin_node.node.handle_update_fail_htlc(&prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0);
 +                      check_added_monitors!(origin_node, 0);
 +                      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);
                        match events[0] {
 -                              MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref update_fee, ref commitment_signed } } => {
 -                                      assert!(update_add_htlcs.is_empty());
 -                                      assert!(update_fulfill_htlcs.is_empty());
 -                                      assert_eq!(update_fail_htlcs.len(), 1);
 -                                      assert!(update_fail_malformed_htlcs.is_empty());
 -                                      assert!(update_fee.is_none());
 -                                      expected_next_node = node_id.clone();
 -                                      next_msgs = Some((update_fail_htlcs[0].clone(), commitment_signed.clone()));
 +                              Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, .. } => {
 +                                      assert_eq!(payment_hash, our_payment_hash);
 +                                      assert!(rejected_by_dest);
 +                                      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);
 +                                      }
                                },
                                _ => panic!("Unexpected event"),
                        }
 -              } else {
 -                      assert!(events.is_empty());
 -              }
 -              if !skip_last && idx == expected_route.len() - 1 {
 -                      assert_eq!(expected_next_node, origin_node.node.get_our_node_id());
 -              }
 -
 -              prev_node = node;
 -      }
 -
 -      if !skip_last {
 -              update_fail_dance!(origin_node, expected_route.first().unwrap(), true);
 -
 -              let events = origin_node.node.get_and_clear_pending_events();
 -              assert_eq!(events.len(), 1);
 -              match events[0] {
 -                      Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
 -                              assert_eq!(payment_hash, our_payment_hash);
 -                              assert!(rejected_by_dest);
 -                      },
 -                      _ => panic!("Unexpected event"),
                }
        }
  }
  
 -pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_hash: PaymentHash)  {
 -      fail_payment_along_route(origin_node, expected_route, false, our_payment_hash);
 +pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], our_payment_hash: PaymentHash)  {
 +      fail_payment_along_route(origin_node, &[&expected_path[..]], false, our_payment_hash);
  }
  
  pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
                let persister = test_utils::TestPersister::new();
                let seed = [i as u8; 32];
                let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
 +              let network_graph = NetworkGraph::new(chain_source.genesis_hash);
  
 -              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, network_graph });
        }
  
        chan_mon_cfgs
@@@ -1520,7 -1398,6 +1528,7 @@@ pub fn create_node_cfgs<'a>(node_count
                        keys_manager: &chanmon_cfgs[i].keys_manager,
                        node_seed: seed,
                        features: InitFeatures::known(),
 +                      network_graph: &chanmon_cfgs[i].network_graph,
                });
        }
  
@@@ -1566,15 -1443,14 +1574,15 @@@ pub fn create_network<'a, 'b: 'a, 'c: '
        let connect_style = Rc::new(RefCell::new(ConnectStyle::FullBlockViaListen));
  
        for i in 0..node_count {
 -              let net_graph_msg_handler = NetGraphMsgHandler::new(cfgs[i].chain_source.genesis_hash, None, cfgs[i].logger);
 -              nodes.push(Node{ chain_source: cfgs[i].chain_source,
 -                               tx_broadcaster: cfgs[i].tx_broadcaster, chain_monitor: &cfgs[i].chain_monitor,
 -                               keys_manager: &cfgs[i].keys_manager, node: &chan_mgrs[i], net_graph_msg_handler,
 -                               node_seed: cfgs[i].node_seed, network_chan_count: chan_count.clone(),
 -                               network_payment_count: payment_count.clone(), logger: cfgs[i].logger,
 -                               blocks: Arc::clone(&cfgs[i].tx_broadcaster.blocks),
 -                               connect_style: Rc::clone(&connect_style),
 +              let net_graph_msg_handler = NetGraphMsgHandler::new(cfgs[i].network_graph, None, cfgs[i].logger);
 +              nodes.push(Node{
 +                      chain_source: cfgs[i].chain_source, tx_broadcaster: cfgs[i].tx_broadcaster,
 +                      chain_monitor: &cfgs[i].chain_monitor, keys_manager: &cfgs[i].keys_manager,
 +                      node: &chan_mgrs[i], network_graph: &cfgs[i].network_graph, net_graph_msg_handler,
 +                      node_seed: cfgs[i].node_seed, network_chan_count: chan_count.clone(),
 +                      network_payment_count: payment_count.clone(), logger: cfgs[i].logger,
 +                      blocks: Arc::clone(&cfgs[i].tx_broadcaster.blocks),
 +                      connect_style: Rc::clone(&connect_style),
                })
        }
  
@@@ -1737,7 -1613,7 +1745,7 @@@ pub fn handle_announce_close_broadcast_
  }
  
  pub fn get_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec<Node<'a, 'b, 'c>>, a: usize, b: usize)  {
 -      handle_announce_close_broadcast_events(nodes, a, b, false, "Commitment or closing transaction was confirmed on chain.");
 +      handle_announce_close_broadcast_events(nodes, a, b, false, "Channel closed because commitment or closing transaction was confirmed on chain.");
  }
  
  #[cfg(test)]