Add an `UnrecoverableError` variant to `ChannelMonitorUpdateStatus`
[rust-lightning] / lightning / src / ln / functional_test_utils.rs
index b8c2e085999c0dbdb978bbf2b85093580b464a37..5a1a21e2999863b9f6af7b75da6c467b4228e7f3 100644 (file)
@@ -17,9 +17,9 @@ use crate::chain::transaction::OutPoint;
 use crate::events::{ClaimedHTLC, ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason};
 use crate::events::bump_transaction::{BumpTransactionEventHandler, Wallet, WalletSource};
 use crate::ln::{ChannelId, PaymentPreimage, PaymentHash, PaymentSecret};
-use crate::ln::channelmanager::{self, AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA};
+use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA};
 use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate};
-use crate::routing::router::{self, PaymentParameters, Route};
+use crate::routing::router::{self, PaymentParameters, Route, RouteParameters};
 use crate::ln::features::InitFeatures;
 use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler,RoutingMessageHandler};
@@ -46,7 +46,7 @@ use alloc::rc::Rc;
 use crate::sync::{Arc, Mutex, LockTestExt, RwLock};
 use core::mem;
 use core::iter::repeat;
-use bitcoin::{PackedLockTime, TxMerkleNode};
+use bitcoin::{PackedLockTime, TxIn, TxMerkleNode};
 
 pub const CHAN_CONFIRM_DEPTH: u32 = 10;
 
@@ -73,6 +73,20 @@ pub fn mine_transactions<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, txn: &[&Tra
        let height = node.best_block_info().1 + 1;
        confirm_transactions_at(node, txn, height);
 }
+/// Mine a single block containing the given transaction without extra consistency checks which may
+/// impact ChannelManager state.
+pub fn mine_transaction_without_consistency_checks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &Transaction) {
+       let height = node.best_block_info().1 + 1;
+       let mut block = Block {
+               header: BlockHeader { version: 0x20000000, prev_blockhash: node.best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: height, bits: 42, nonce: 42 },
+               txdata: Vec::new(),
+       };
+       for _ in 0..*node.network_chan_count.borrow() { // Make sure we don't end up with channels at the same short id by offsetting by chan_count
+               block.txdata.push(Transaction { version: 0, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: Vec::new() });
+       }
+       block.txdata.push((*tx).clone());
+       do_connect_block_without_consistency_checks(node, block, false);
+}
 /// Mine the given transaction at the given height, mining blocks as required to build to that
 /// height
 ///
@@ -211,16 +225,16 @@ pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) ->
        assert!(depth >= 1);
        for i in 1..depth {
                let prev_blockhash = block.header.block_hash();
-               do_connect_block(node, block, skip_intermediaries);
+               do_connect_block_with_consistency_checks(node, block, skip_intermediaries);
                block = create_dummy_block(prev_blockhash, height + i, Vec::new());
        }
        let hash = block.header.block_hash();
-       do_connect_block(node, block, false);
+       do_connect_block_with_consistency_checks(node, block, false);
        hash
 }
 
 pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block) {
-       do_connect_block(node, block.clone(), false);
+       do_connect_block_with_consistency_checks(node, block.clone(), false);
 }
 
 fn call_claimable_balances<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>) {
@@ -230,8 +244,14 @@ fn call_claimable_balances<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>) {
        }
 }
 
-fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, skip_intermediaries: bool) {
+fn do_connect_block_with_consistency_checks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, skip_intermediaries: bool) {
        call_claimable_balances(node);
+       do_connect_block_without_consistency_checks(node, block, skip_intermediaries);
+       call_claimable_balances(node);
+       node.node.test_process_background_events();
+}
+
+fn do_connect_block_without_consistency_checks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, skip_intermediaries: bool) {
        let height = node.best_block_info().1 + 1;
        #[cfg(feature = "std")] {
                eprintln!("Connecting block using Block Connection Style: {:?}", *node.connect_style.borrow());
@@ -286,8 +306,6 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, sk
                        }
                }
        }
-       call_claimable_balances(node);
-       node.node.test_process_background_events();
 
        for tx in &block.txdata {
                for input in &tx.input {
@@ -404,6 +422,10 @@ pub struct Node<'chan_man, 'node_cfg: 'chan_man, 'chan_mon_cfg: 'node_cfg> {
                &'chan_mon_cfg test_utils::TestLogger,
        >,
 }
+#[cfg(feature = "std")]
+impl<'a, 'b, 'c> std::panic::UnwindSafe for Node<'a, 'b, 'c> {}
+#[cfg(feature = "std")]
+impl<'a, 'b, 'c> std::panic::RefUnwindSafe for Node<'a, 'b, 'c> {}
 impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
        pub fn best_block_hash(&self) -> BlockHash {
                self.blocks.lock().unwrap().last().unwrap().0.block_hash()
@@ -560,7 +582,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
                        let chain_source = test_utils::TestChainSource::new(Network::Testnet);
                        let chain_monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &broadcaster, &self.logger, &feeest, &persister, &self.keys_manager);
                        for deserialized_monitor in deserialized_monitors.drain(..) {
-                               if chain_monitor.watch_channel(deserialized_monitor.get_funding_txo().0, deserialized_monitor) != ChannelMonitorUpdateStatus::Completed {
+                               if chain_monitor.watch_channel(deserialized_monitor.get_funding_txo().0, deserialized_monitor) != Ok(ChannelMonitorUpdateStatus::Completed) {
                                        panic!();
                                }
                        }
@@ -811,36 +833,14 @@ macro_rules! get_channel_ref {
        }
 }
 
-#[cfg(test)]
-macro_rules! get_outbound_v1_channel_ref {
-       ($node: expr, $counterparty_node: expr, $per_peer_state_lock: ident, $peer_state_lock: ident, $channel_id: expr) => {
-               {
-                       $per_peer_state_lock = $node.node.per_peer_state.read().unwrap();
-                       $peer_state_lock = $per_peer_state_lock.get(&$counterparty_node.node.get_our_node_id()).unwrap().lock().unwrap();
-                       $peer_state_lock.outbound_v1_channel_by_id.get_mut(&$channel_id).unwrap()
-               }
-       }
-}
-
-#[cfg(test)]
-macro_rules! get_inbound_v1_channel_ref {
-       ($node: expr, $counterparty_node: expr, $per_peer_state_lock: ident, $peer_state_lock: ident, $channel_id: expr) => {
-               {
-                       $per_peer_state_lock = $node.node.per_peer_state.read().unwrap();
-                       $peer_state_lock = $per_peer_state_lock.get(&$counterparty_node.node.get_our_node_id()).unwrap().lock().unwrap();
-                       $peer_state_lock.inbound_v1_channel_by_id.get_mut(&$channel_id).unwrap()
-               }
-       }
-}
-
 #[cfg(test)]
 macro_rules! get_feerate {
        ($node: expr, $counterparty_node: expr, $channel_id: expr) => {
                {
                        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.context.get_feerate_sat_per_1000_weight()
+                       let phase = get_channel_ref!($node, $counterparty_node, per_peer_state_lock, peer_state_lock, $channel_id);
+                       phase.context().get_feerate_sat_per_1000_weight()
                }
        }
 }
@@ -852,7 +852,7 @@ macro_rules! get_channel_type_features {
                        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.context.get_channel_type().clone()
+                       chan.context().get_channel_type().clone()
                }
        }
 }
@@ -981,7 +981,7 @@ pub fn _reload_node<'a, 'b, 'c>(node: &'a Node<'a, 'b, 'c>, default_config: User
 
        for monitor in monitors_read.drain(..) {
                assert_eq!(node.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor),
-                       ChannelMonitorUpdateStatus::Completed);
+                       Ok(ChannelMonitorUpdateStatus::Completed));
                check_added_monitors!(node, 1);
        }
 
@@ -1005,7 +1005,23 @@ macro_rules! reload_node {
        };
 }
 
-pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128) -> (ChannelId, Transaction, OutPoint) {
+pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>,
+       expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128)
+ -> (ChannelId, Transaction, OutPoint)
+{
+       internal_create_funding_transaction(node, expected_counterparty_node_id, expected_chan_value, expected_user_chan_id, false)
+}
+
+pub fn create_coinbase_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>,
+       expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128)
+ -> (ChannelId, Transaction, OutPoint)
+{
+       internal_create_funding_transaction(node, expected_counterparty_node_id, expected_chan_value, expected_user_chan_id, true)
+}
+
+fn internal_create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>,
+       expected_counterparty_node_id: &PublicKey, expected_chan_value: u64, expected_user_chan_id: u128,
+       coinbase: bool) -> (ChannelId, Transaction, OutPoint) {
        let chan_id = *node.network_chan_count.borrow();
 
        let events = node.node.get_and_clear_pending_events();
@@ -1016,7 +1032,16 @@ pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_
                        assert_eq!(*channel_value_satoshis, expected_chan_value);
                        assert_eq!(user_channel_id, expected_user_chan_id);
 
-                       let tx = Transaction { version: chan_id as i32, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: vec![TxOut {
+                       let input = if coinbase {
+                               vec![TxIn {
+                                       previous_output: bitcoin::OutPoint::null(),
+                                       ..Default::default()
+                               }]
+                       } else {
+                               Vec::new()
+                       };
+
+                       let tx = Transaction { version: chan_id as i32, lock_time: PackedLockTime::ZERO, input, output: vec![TxOut {
                                value: *channel_value_satoshis, script_pubkey: output_script.clone(),
                        }]};
                        let funding_outpoint = OutPoint { txid: tx.txid(), index: 0 };
@@ -1025,6 +1050,7 @@ pub fn create_funding_transaction<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, expected_
                _ => panic!("Unexpected event"),
        }
 }
+
 pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, expected_temporary_channel_id: ChannelId) -> Transaction {
        let (temporary_channel_id, tx, funding_output) = create_funding_transaction(node_a, &node_b.node.get_our_node_id(), channel_value, 42);
        assert_eq!(temporary_channel_id, expected_temporary_channel_id);
@@ -1768,20 +1794,7 @@ pub fn do_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '
        check_added_monitors!(node_a, 1);
 
        // If this commitment signed dance was due to a claim, don't check for an RAA monitor update.
-       let got_claim = node_a.node.pending_events.lock().unwrap().iter().any(|(ev, action)| {
-               let matching_action = if let Some(channelmanager::EventCompletionAction::ReleaseRAAChannelMonitorUpdate
-                       { channel_funding_outpoint, counterparty_node_id }) = action
-               {
-                       if channel_funding_outpoint.to_channel_id() == commitment_signed.channel_id {
-                               assert_eq!(*counterparty_node_id, node_b.node.get_our_node_id());
-                               true
-                       } else { false }
-               } else { false };
-               if matching_action {
-                       if let Event::PaymentSent { .. } = ev {} else { panic!(); }
-               }
-               matching_action
-       });
+       let got_claim = node_a.node.test_raa_monitor_updates_held(node_b.node.get_our_node_id(), commitment_signed.channel_id);
        if fail_backwards { assert!(!got_claim); }
        commitment_signed_dance!(node_a, node_b, (), fail_backwards, true, false, got_claim);
 
@@ -1838,14 +1851,14 @@ macro_rules! get_payment_preimage_hash {
 }
 
 /// 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) -> Result<Route, msgs::LightningError> {
+pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> 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(),
+               &send_node.node.get_our_node_id(), route_params, &send_node.network_graph.read_only(),
                Some(&send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
-               recv_value, send_node.logger, &scorer, &(), &random_seed_bytes
+               send_node.logger, &scorer, &(), &random_seed_bytes
        )
 }
 
@@ -1854,9 +1867,10 @@ pub fn get_route(send_node: &Node, payment_params: &PaymentParameters, recv_valu
 /// 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) => {
-               $crate::ln::functional_test_utils::get_route(&$send_node, &$payment_params, $recv_value)
-       }
+       ($send_node: expr, $payment_params: expr, $recv_value: expr) => {{
+               let route_params = $crate::routing::router::RouteParameters::from_payment_params_and_value($payment_params, $recv_value);
+               $crate::ln::functional_test_utils::get_route(&$send_node, &route_params)
+       }}
 }
 
 #[cfg(test)]
@@ -1868,9 +1882,10 @@ macro_rules! get_route_and_payment_hash {
                $crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value)
        }};
        ($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr) => {{
+               let route_params = $crate::routing::router::RouteParameters::from_payment_params_and_value($payment_params, $recv_value);
                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);
+               let route = $crate::ln::functional_test_utils::get_route(&$send_node, &route_params);
                (route.unwrap(), payment_hash, payment_preimage, payment_secret)
        }}
 }
@@ -1994,29 +2009,38 @@ macro_rules! expect_payment_path_successful {
        }
 }
 
+pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
+       event: Event, node: &H, prev_node: &H, next_node: &H, expected_fee: Option<u64>,
+       upstream_force_closed: bool, downstream_force_closed: bool
+) {
+       match event {
+               Event::PaymentForwarded {
+                       fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id,
+                       outbound_amount_forwarded_msat: _
+               } => {
+                       assert_eq!(fee_earned_msat, expected_fee);
+                       if !upstream_force_closed {
+                               // Is the event prev_channel_id in one of the channels between the two nodes?
+                               assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == prev_node.node().get_our_node_id() && x.channel_id == prev_channel_id.unwrap()));
+                       }
+                       // We check for force closures since a force closed channel is removed from the
+                       // node's channel list
+                       if !downstream_force_closed {
+                               assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == next_node.node().get_our_node_id() && x.channel_id == next_channel_id.unwrap()));
+                       }
+                       assert_eq!(claim_from_onchain_tx, downstream_force_closed);
+               },
+               _ => panic!("Unexpected event"),
+       }
+}
+
 macro_rules! expect_payment_forwarded {
        ($node: expr, $prev_node: expr, $next_node: expr, $expected_fee: expr, $upstream_force_closed: expr, $downstream_force_closed: expr) => {
-               let events = $node.node.get_and_clear_pending_events();
+               let mut events = $node.node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
-               match events[0] {
-                       Event::PaymentForwarded {
-                               fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id,
-                               outbound_amount_forwarded_msat: _
-                       } => {
-                               assert_eq!(fee_earned_msat, $expected_fee);
-                               if fee_earned_msat.is_some() {
-                                       // Is the event prev_channel_id in one of the channels between the two nodes?
-                                       assert!($node.node.list_channels().iter().any(|x| x.counterparty.node_id == $prev_node.node.get_our_node_id() && x.channel_id == prev_channel_id.unwrap()));
-                               }
-                               // We check for force closures since a force closed channel is removed from the
-                               // node's channel list
-                               if !$downstream_force_closed {
-                                       assert!($node.node.list_channels().iter().any(|x| x.counterparty.node_id == $next_node.node.get_our_node_id() && x.channel_id == next_channel_id.unwrap()));
-                               }
-                               assert_eq!(claim_from_onchain_tx, $downstream_force_closed);
-                       },
-                       _ => panic!("Unexpected event"),
-               }
+               $crate::ln::functional_test_utils::expect_payment_forwarded(
+                       events.pop().unwrap(), &$node, &$prev_node, &$next_node, $expected_fee,
+                       $upstream_force_closed, $downstream_force_closed);
        }
 }
 
@@ -2386,14 +2410,14 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, '
                                                let peer_state = per_peer_state.get(&$prev_node.node.get_our_node_id())
                                                        .unwrap().lock().unwrap();
                                                let channel = peer_state.channel_by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap();
-                                               if let Some(prev_config) = channel.context.prev_config() {
+                                               if let Some(prev_config) = channel.context().prev_config() {
                                                        prev_config.forwarding_fee_base_msat
                                                } else {
-                                                       channel.context.config().forwarding_fee_base_msat
+                                                       channel.context().config().forwarding_fee_base_msat
                                                }
                                        };
                                        if $idx == 1 { fee += expected_extra_fees[i]; }
-                                       expect_payment_forwarded!($node, $next_node, $prev_node, Some(fee as u64), false, false);
+                                       expect_payment_forwarded!(*$node, $next_node, $prev_node, Some(fee as u64), false, false);
                                        expected_total_fee_msat += fee as u64;
                                        check_added_monitors!($node, 1);
                                        let new_next_msgs = if $new_msgs {
@@ -2461,10 +2485,11 @@ pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 
 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) {
+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, PaymentId) {
        let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
                .with_bolt11_features(expected_route.last().unwrap().node.invoice_features()).unwrap();
-       let route = get_route(origin_node, &payment_params, recv_value).unwrap();
+       let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
+       let route = get_route(origin_node, &route_params).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].hops.len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
@@ -2472,20 +2497,20 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
        }
 
        let res = send_along_route(origin_node, route, expected_route, recv_value);
-       (res.0, res.1, res.2)
+       (res.0, res.1, res.2, res.3)
 }
 
 pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
        let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV)
                .with_bolt11_features(expected_route.last().unwrap().node.invoice_features()).unwrap();
+       let route_params = RouteParameters::from_payment_params_and_value(payment_params, recv_value);
        let network_graph = origin_node.network_graph.read_only();
        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 = router::get_route(
-               &origin_node.node.get_our_node_id(), &payment_params, &network_graph,
-               None, recv_value, origin_node.logger, &scorer, &(), &random_seed_bytes).unwrap();
+       let route = router::get_route(&origin_node.node.get_our_node_id(), &route_params, &network_graph,
+               None, origin_node.logger, &scorer, &(), &random_seed_bytes).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].hops.len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
@@ -2499,7 +2524,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
                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) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
+pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret, PaymentId) {
        let res = route_payment(&origin, expected_route, recv_value);
        claim_payment(&origin, expected_route, res.0);
        res
@@ -2912,7 +2937,9 @@ macro_rules! get_channel_value_stat {
        ($node: expr, $counterparty_node: expr, $channel_id: expr) => {{
                let peer_state_lock = $node.node.per_peer_state.read().unwrap();
                let chan_lock = peer_state_lock.get(&$counterparty_node.node.get_our_node_id()).unwrap().lock().unwrap();
-               let chan = chan_lock.channel_by_id.get(&$channel_id).unwrap();
+               let chan = chan_lock.channel_by_id.get(&$channel_id).map(
+                       |phase| if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None }
+               ).flatten().unwrap();
                chan.get_value_stat()
        }}
 }
@@ -3021,7 +3048,11 @@ pub struct ReconnectArgs<'a, 'b, 'c, 'd> {
        pub node_a: &'a Node<'b, 'c, 'd>,
        pub node_b: &'a Node<'b, 'c, 'd>,
        pub send_channel_ready: (bool, bool),
-       pub pending_htlc_adds: (i64, i64),
+       pub pending_responding_commitment_signed: (bool, bool),
+       /// Indicates that the pending responding commitment signed will be a dup for the recipient,
+       /// and no monitor update is expected
+       pub pending_responding_commitment_signed_dup_monitor: (bool, bool),
+       pub pending_htlc_adds: (usize, usize),
        pub pending_htlc_claims: (usize, usize),
        pub pending_htlc_fails: (usize, usize),
        pub pending_cell_htlc_claims: (usize, usize),
@@ -3035,6 +3066,8 @@ impl<'a, 'b, 'c, 'd> ReconnectArgs<'a, 'b, 'c, 'd> {
                        node_a,
                        node_b,
                        send_channel_ready: (false, false),
+                       pending_responding_commitment_signed: (false, false),
+                       pending_responding_commitment_signed_dup_monitor: (false, false),
                        pending_htlc_adds: (0, 0),
                        pending_htlc_claims: (0, 0),
                        pending_htlc_fails: (0, 0),
@@ -3050,7 +3083,8 @@ impl<'a, 'b, 'c, 'd> ReconnectArgs<'a, 'b, 'c, 'd> {
 pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
        let ReconnectArgs {
                node_a, node_b, send_channel_ready, pending_htlc_adds, pending_htlc_claims, pending_htlc_fails,
-               pending_cell_htlc_claims, pending_cell_htlc_fails, pending_raa
+               pending_cell_htlc_claims, pending_cell_htlc_fails, pending_raa,
+               pending_responding_commitment_signed, pending_responding_commitment_signed_dup_monitor,
        } = args;
        node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init {
                features: node_b.node.init_features(), networks: None, remote_network_address: None
@@ -3135,13 +3169,12 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
                } else {
                        assert!(chan_msgs.1.is_none());
                }
-               if pending_htlc_adds.0 != 0 || pending_htlc_claims.0 != 0 || pending_htlc_fails.0 != 0 || pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 {
+               if pending_htlc_adds.0 != 0 || pending_htlc_claims.0 != 0 || pending_htlc_fails.0 != 0 ||
+                       pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 ||
+                       pending_responding_commitment_signed.0
+               {
                        let commitment_update = chan_msgs.2.unwrap();
-                       if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
-                               assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.0 as usize);
-                       } else {
-                               assert!(commitment_update.update_add_htlcs.is_empty());
-                       }
+                       assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.0);
                        assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.0 + pending_cell_htlc_claims.0);
                        assert_eq!(commitment_update.update_fail_htlcs.len(), pending_htlc_fails.0 + pending_cell_htlc_fails.0);
                        assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
@@ -3155,7 +3188,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
                                node_a.node.handle_update_fail_htlc(&node_b.node.get_our_node_id(), &update_fail);
                        }
 
-                       if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
+                       if !pending_responding_commitment_signed.0 {
                                commitment_signed_dance!(node_a, node_b, commitment_update.commitment_signed, false);
                        } else {
                                node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), &commitment_update.commitment_signed);
@@ -3164,7 +3197,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
                                // No commitment_signed so get_event_msg's assert(len == 1) passes
                                node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &as_revoke_and_ack);
                                assert!(node_b.node.get_and_clear_pending_msg_events().is_empty());
-                               check_added_monitors!(node_b, 1);
+                               check_added_monitors!(node_b, if pending_responding_commitment_signed_dup_monitor.0 { 0 } else { 1 });
                        }
                } else {
                        assert!(chan_msgs.2.is_none());
@@ -3194,11 +3227,12 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
                } else {
                        assert!(chan_msgs.1.is_none());
                }
-               if pending_htlc_adds.1 != 0 || pending_htlc_claims.1 != 0 || pending_htlc_fails.1 != 0 || pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 {
+               if pending_htlc_adds.1 != 0 || pending_htlc_claims.1 != 0 || pending_htlc_fails.1 != 0 ||
+                       pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 ||
+                       pending_responding_commitment_signed.1
+               {
                        let commitment_update = chan_msgs.2.unwrap();
-                       if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
-                               assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.1 as usize);
-                       }
+                       assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.1);
                        assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.1 + pending_cell_htlc_claims.1);
                        assert_eq!(commitment_update.update_fail_htlcs.len(), pending_htlc_fails.1 + pending_cell_htlc_fails.1);
                        assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
@@ -3212,7 +3246,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
                                node_b.node.handle_update_fail_htlc(&node_a.node.get_our_node_id(), &update_fail);
                        }
 
-                       if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
+                       if !pending_responding_commitment_signed.1 {
                                commitment_signed_dance!(node_b, node_a, commitment_update.commitment_signed, false);
                        } else {
                                node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &commitment_update.commitment_signed);
@@ -3221,7 +3255,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
                                // No commitment_signed so get_event_msg's assert(len == 1) passes
                                node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &bs_revoke_and_ack);
                                assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
-                               check_added_monitors!(node_a, 1);
+                               check_added_monitors!(node_a, if pending_responding_commitment_signed_dup_monitor.1 { 0 } else { 1 });
                        }
                } else {
                        assert!(chan_msgs.2.is_none());