Merge pull request #206 from TheBlueMatt/2018-10-189-rebase
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sat, 6 Oct 2018 03:49:26 +0000 (20:49 -0700)
committerGitHub <noreply@github.com>
Sat, 6 Oct 2018 03:49:26 +0000 (20:49 -0700)
#189 rebased

src/ln/channel.rs
src/ln/channelmanager.rs

index 0a923dc7eedc904da44f31d6ed9f6c74c422b6a3..1d736d41d6f799569a633e2c9de8b6895d28e34c 100644 (file)
@@ -44,6 +44,17 @@ pub struct ChannelKeys {
        pub commitment_seed: [u8; 32],
 }
 
+#[cfg(test)]
+pub struct ChannelValueStat {
+       pub value_to_self_msat: u64,
+       pub channel_value_msat: u64,
+       pub channel_reserve_msat: u64,
+       pub pending_outbound_htlcs_amount_msat: u64,
+       pub pending_inbound_htlcs_amount_msat: u64,
+       pub holding_cell_outbound_amount_msat: u64,
+       pub their_max_htlc_value_in_flight_msat: u64, // outgoing
+}
+
 impl ChannelKeys {
        pub fn new_from_seed(seed: &[u8; 32]) -> Result<ChannelKeys, secp256k1::Error> {
                let mut prk = [0; 32];
@@ -308,6 +319,13 @@ pub(super) struct Channel {
        channel_update_count: u32,
        feerate_per_kw: u64,
 
+       #[cfg(debug_assertions)]
+       /// Max to_local and to_remote outputs in a locally-generated commitment transaction
+       max_commitment_tx_output_local: ::std::sync::Mutex<(u64, u64)>,
+       #[cfg(debug_assertions)]
+       /// Max to_local and to_remote outputs in a remote-generated commitment transaction
+       max_commitment_tx_output_remote: ::std::sync::Mutex<(u64, u64)>,
+
        #[cfg(test)]
        // Used in ChannelManager's tests to send a revoked transaction
        pub last_local_commitment_txn: Vec<Transaction>,
@@ -330,6 +348,7 @@ pub(super) struct Channel {
        our_dust_limit_satoshis: u64,
        their_max_htlc_value_in_flight_msat: u64,
        //get_our_max_htlc_value_in_flight_msat(): u64,
+       /// minimum channel reserve for **self** to maintain - set by them.
        their_channel_reserve_satoshis: u64,
        //get_our_channel_reserve_satoshis(): u64,
        their_htlc_minimum_msat: u64,
@@ -402,6 +421,8 @@ impl Channel {
                channel_value_satoshis * 1000 / 10 //TODO
        }
 
+       /// Returns a minimum channel reserve value **they** need to maintain
+       ///
        /// Guaranteed to return a value no larger than channel_value_satoshis
        fn get_our_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
                let (q, _) = channel_value_satoshis.overflowing_div(100);
@@ -477,6 +498,11 @@ impl Channel {
                        next_remote_htlc_id: 0,
                        channel_update_count: 1,
 
+                       #[cfg(debug_assertions)]
+                       max_commitment_tx_output_local: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
+                       #[cfg(debug_assertions)]
+                       max_commitment_tx_output_remote: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
+
                        last_local_commitment_txn: Vec::new(),
 
                        last_sent_closing_fee: None,
@@ -627,6 +653,11 @@ impl Channel {
                        next_remote_htlc_id: 0,
                        channel_update_count: 1,
 
+                       #[cfg(debug_assertions)]
+                       max_commitment_tx_output_local: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
+                       #[cfg(debug_assertions)]
+                       max_commitment_tx_output_remote: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
+
                        last_local_commitment_txn: Vec::new(),
 
                        last_sent_closing_fee: None,
@@ -812,9 +843,32 @@ impl Channel {
                }
 
 
+               let value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
+               let value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000 - self.value_to_self_msat - remote_htlc_total_msat) as i64 - value_to_self_msat_offset;
+
+               #[cfg(debug_assertions)]
+               {
+                       // Make sure that the to_self/to_remote is always either past the appropriate
+                       // channel_reserve *or* it is making progress towards it.
+                       // TODO: This should happen after fee calculation, but we don't handle that correctly
+                       // yet!
+                       let mut max_commitment_tx_output = if generated_by_local {
+                               self.max_commitment_tx_output_local.lock().unwrap()
+                       } else {
+                               self.max_commitment_tx_output_remote.lock().unwrap()
+                       };
+                       debug_assert!(max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.their_channel_reserve_satoshis as i64);
+                       max_commitment_tx_output.0 = cmp::max(max_commitment_tx_output.0, value_to_self_msat as u64);
+                       debug_assert!(max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
+                       max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64);
+               }
+
                let total_fee: u64 = feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
-               let value_to_self: i64 = ((self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset) / 1000 - if self.channel_outbound { total_fee as i64 } else { 0 };
-               let value_to_remote: i64 = (((self.channel_value_satoshis * 1000 - self.value_to_self_msat - remote_htlc_total_msat) as i64 - value_to_self_msat_offset) / 1000) - if self.channel_outbound { 0 } else { total_fee as i64 };
+               let (value_to_self, value_to_remote) = if self.channel_outbound {
+                       (value_to_self_msat / 1000 - total_fee as i64, value_to_remote_msat / 1000)
+               } else {
+                       (value_to_self_msat / 1000, value_to_remote_msat / 1000 - total_fee as i64)
+               };
 
                let value_to_a = if local { value_to_self } else { value_to_remote };
                let value_to_b = if local { value_to_remote } else { value_to_self };
@@ -1415,42 +1469,23 @@ impl Channel {
                Ok(())
        }
 
-       /// Returns (inbound_htlc_count, outbound_htlc_count, htlc_outbound_value_msat, htlc_inbound_value_msat)
-       /// If its for a remote update check, we need to be more lax about checking against messages we
-       /// sent but they may not have received/processed before they sent this message. Further, for
-       /// our own sends, we're more conservative and even consider things they've removed against
-       /// totals, though there is little reason to outside of further avoiding any race condition
-       /// issues.
-       fn get_pending_htlc_stats(&self, for_remote_update_check: bool) -> (u32, u32, u64, u64) {
-               //TODO: Can probably split this into inbound/outbound
-               let mut inbound_htlc_count: u32 = 0;
-               let mut outbound_htlc_count: u32 = 0;
-               let mut htlc_outbound_value_msat = 0;
+       /// Returns (inbound_htlc_count, htlc_inbound_value_msat)
+       fn get_inbound_pending_htlc_stats(&self) -> (u32, u64) {
                let mut htlc_inbound_value_msat = 0;
                for ref htlc in self.pending_inbound_htlcs.iter() {
-                       match htlc.state {
-                               InboundHTLCState::RemoteAnnounced => {},
-                               InboundHTLCState::AwaitingRemoteRevokeToAnnounce => {},
-                               InboundHTLCState::AwaitingAnnouncedRemoteRevoke => {},
-                               InboundHTLCState::Committed => {},
-                               InboundHTLCState::LocalRemoved => {},
-                       }
-                       inbound_htlc_count += 1;
                        htlc_inbound_value_msat += htlc.amount_msat;
                }
+               (self.pending_inbound_htlcs.len() as u32, htlc_inbound_value_msat)
+       }
+
+       /// Returns (outbound_htlc_count, htlc_outbound_value_msat)
+       fn get_outbound_pending_htlc_stats(&self) -> (u32, u64) {
+               let mut htlc_outbound_value_msat = 0;
                for ref htlc in self.pending_outbound_htlcs.iter() {
-                       match htlc.state {
-                               OutboundHTLCState::LocalAnnounced => { if for_remote_update_check { continue; } },
-                               OutboundHTLCState::Committed => {},
-                               OutboundHTLCState::RemoteRemoved => { if for_remote_update_check { continue; } },
-                               OutboundHTLCState::AwaitingRemoteRevokeToRemove => { if for_remote_update_check { continue; } },
-                               OutboundHTLCState::AwaitingRemovedRemoteRevoke => { if for_remote_update_check { continue; } },
-                       }
-                       outbound_htlc_count += 1;
                        htlc_outbound_value_msat += htlc.amount_msat;
                }
 
-               (inbound_htlc_count, outbound_htlc_count, htlc_outbound_value_msat, htlc_inbound_value_msat)
+               (self.pending_outbound_htlcs.len() as u32, htlc_outbound_value_msat)
        }
 
        pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), HandleError> {
@@ -1467,7 +1502,7 @@ impl Channel {
                        return Err(HandleError{err: "Remote side tried to send less than our minimum HTLC value", action: None});
                }
 
-               let (inbound_htlc_count, _, htlc_outbound_value_msat, htlc_inbound_value_msat) = self.get_pending_htlc_stats(true);
+               let (inbound_htlc_count, htlc_inbound_value_msat) = self.get_inbound_pending_htlc_stats();
                if inbound_htlc_count + 1 > OUR_MAX_HTLCS as u32 {
                        return Err(HandleError{err: "Remote tried to push more than our max accepted HTLCs", action: None});
                }
@@ -1479,7 +1514,7 @@ impl Channel {
                // Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet
                // the reserve_satoshis we told them to always have as direct payment so that they lose
                // something if we punish them for broadcasting an old state).
-               if htlc_inbound_value_msat + htlc_outbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 {
+               if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 {
                        return Err(HandleError{err: "Remote HTLC add would put them over their reserve value", action: None});
                }
                if self.next_remote_htlc_id != msg.htlc_id {
@@ -2349,6 +2384,30 @@ impl Channel {
                &self.local_keys
        }
 
+       #[cfg(test)]
+       pub fn get_value_stat(&self) -> ChannelValueStat {
+               ChannelValueStat {
+                       value_to_self_msat: self.value_to_self_msat,
+                       channel_value_msat: self.channel_value_satoshis * 1000,
+                       channel_reserve_msat: self.their_channel_reserve_satoshis * 1000,
+                       pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
+                       pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
+                       holding_cell_outbound_amount_msat: {
+                               let mut res = 0;
+                               for h in self.holding_cell_htlc_updates.iter() {
+                                       match h {
+                                               &HTLCUpdateAwaitingACK::AddHTLC{amount_msat, .. } => {
+                                                       res += amount_msat;
+                                               }
+                                               _ => {}
+                                       }
+                               }
+                               res
+                       },
+                       their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat,
+               }
+       }
+
        /// Allowed in any state (including after shutdown)
        pub fn get_channel_update_count(&self) -> u32 {
                self.channel_update_count
@@ -2722,7 +2781,7 @@ impl Channel {
                        return Err(HandleError{err: "Cannot send an HTLC while disconnected", action: Some(ErrorAction::IgnoreError)});
                }
 
-               let (_, outbound_htlc_count, htlc_outbound_value_msat, htlc_inbound_value_msat) = self.get_pending_htlc_stats(false);
+               let (outbound_htlc_count, htlc_outbound_value_msat) = self.get_outbound_pending_htlc_stats();
                if outbound_htlc_count + 1 > self.their_max_accepted_htlcs as u32 {
                        return Err(HandleError{err: "Cannot push more than their max accepted HTLCs", action: None});
                }
@@ -2731,8 +2790,20 @@ impl Channel {
                if htlc_outbound_value_msat + amount_msat > self.their_max_htlc_value_in_flight_msat {
                        return Err(HandleError{err: "Cannot send value that would put us over our max HTLC value in flight", action: None});
                }
-               // Check their_channel_reserve_satoshis:
-               if htlc_inbound_value_msat + htlc_outbound_value_msat + amount_msat + (self.channel_value_satoshis * 1000 - self.value_to_self_msat) > (self.channel_value_satoshis - self.their_channel_reserve_satoshis) * 1000 {
+
+               let mut holding_cell_outbound_amount_msat = 0;
+               for holding_htlc in self.holding_cell_htlc_updates.iter() {
+                       match holding_htlc {
+                               &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, .. } => {
+                                       holding_cell_outbound_amount_msat += *amount_msat;
+                               }
+                               _ => {}
+                       }
+               }
+
+               // Check self.their_channel_reserve_satoshis (the amount we must keep as
+               // reserve for them to have something to claim if we misbehave)
+               if self.value_to_self_msat < self.their_channel_reserve_satoshis * 1000 + amount_msat + holding_cell_outbound_amount_msat + htlc_outbound_value_msat {
                        return Err(HandleError{err: "Cannot send value that would put us over our reserve value", action: None});
                }
 
index fd168de4160a0ea1717e1780791ecefb55f47e4d..9bdbc80408245d8c36366b035bd66f7a47e20e61 100644 (file)
@@ -2565,7 +2565,11 @@ mod tests {
        }
 
        fn create_chan_between_nodes(node_a: &Node, node_b: &Node) -> (msgs::ChannelAnnouncement, msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
-               node_a.node.create_channel(node_b.node.get_our_node_id(), 100000, 10001, 42).unwrap();
+               create_chan_between_nodes_with_value(node_a, node_b, 100000, 10001)
+       }
+
+       fn create_chan_between_nodes_with_value(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> (msgs::ChannelAnnouncement, msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
+               node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
 
                let events_1 = node_a.node.get_and_clear_pending_events();
                assert_eq!(events_1.len(), 1);
@@ -2587,7 +2591,7 @@ mod tests {
                assert_eq!(events_2.len(), 1);
                match events_2[0] {
                        Event::FundingGenerationReady { ref temporary_channel_id, ref channel_value_satoshis, ref output_script, user_channel_id } => {
-                               assert_eq!(*channel_value_satoshis, 100000);
+                               assert_eq!(*channel_value_satoshis, channel_value);
                                assert_eq!(user_channel_id, 42);
 
                                tx = Transaction { version: chan_id as u32, lock_time: 0, input: Vec::new(), output: vec![TxOut {
@@ -2691,7 +2695,11 @@ mod tests {
        }
 
        fn create_announced_chan_between_nodes(nodes: &Vec<Node>, a: usize, b: usize) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
-               let chan_announcement = create_chan_between_nodes(&nodes[a], &nodes[b]);
+               create_announced_chan_between_nodes_with_value(nodes, a, b, 100000, 10001)
+       }
+
+       fn create_announced_chan_between_nodes_with_value(nodes: &Vec<Node>, a: usize, b: usize, channel_value: u64, push_msat: u64) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
+               let chan_announcement = create_chan_between_nodes_with_value(&nodes[a], &nodes[b], channel_value, push_msat);
                for node in nodes {
                        assert!(node.router.handle_channel_announcement(&chan_announcement.0).unwrap());
                        node.router.handle_channel_update(&chan_announcement.1).unwrap();
@@ -3879,6 +3887,281 @@ mod tests {
                }
        }
 
+       #[test]
+       fn channel_reserve_test() {
+               use util::rng;
+               use std::sync::atomic::Ordering;
+               use ln::msgs::HandleError;
+
+               macro_rules! get_channel_value_stat {
+                       ($node: expr, $channel_id: expr) => {{
+                               let chan_lock = $node.node.channel_state.lock().unwrap();
+                               let chan = chan_lock.by_id.get(&$channel_id).unwrap();
+                               chan.get_value_stat()
+                       }}
+               }
+
+               let mut nodes = create_network(3);
+               let chan_1 = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1900, 1001);
+               let chan_2 = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1900, 1001);
+
+               let mut stat01 = get_channel_value_stat!(nodes[0], chan_1.2);
+               let mut stat11 = get_channel_value_stat!(nodes[1], chan_1.2);
+
+               let mut stat12 = get_channel_value_stat!(nodes[1], chan_2.2);
+               let mut stat22 = get_channel_value_stat!(nodes[2], chan_2.2);
+
+               macro_rules! get_route_and_payment_hash {
+                       ($recv_value: expr) => {{
+                               let route = nodes[0].router.get_route(&nodes.last().unwrap().node.get_our_node_id(), None, &Vec::new(), $recv_value, TEST_FINAL_CLTV).unwrap();
+                               let (payment_preimage, payment_hash) = get_payment_preimage_hash!(nodes[0]);
+                               (route, payment_hash, payment_preimage)
+                       }}
+               };
+
+               macro_rules! expect_pending_htlcs_forwardable {
+                       ($node: expr) => {{
+                               let events = $node.node.get_and_clear_pending_events();
+                               assert_eq!(events.len(), 1);
+                               match events[0] {
+                                       Event::PendingHTLCsForwardable { .. } => { },
+                                       _ => panic!("Unexpected event"),
+                               };
+                               $node.node.channel_state.lock().unwrap().next_forward = Instant::now();
+                               $node.node.process_pending_htlc_forwards();
+                       }}
+               };
+
+               macro_rules! expect_forward {
+                       ($node: expr) => {{
+                               let mut events = $node.node.get_and_clear_pending_events();
+                               assert_eq!(events.len(), 1);
+                               check_added_monitors!($node, 1);
+                               let payment_event = SendEvent::from_event(events.remove(0));
+                               payment_event
+                       }}
+               }
+
+               macro_rules! expect_payment_received {
+                       ($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => {
+                               let events = $node.node.get_and_clear_pending_events();
+                               assert_eq!(events.len(), 1);
+                               match events[0] {
+                                       Event::PaymentReceived { ref payment_hash, amt } => {
+                                               assert_eq!($expected_payment_hash, *payment_hash);
+                                               assert_eq!($expected_recv_value, amt);
+                                       },
+                                       _ => panic!("Unexpected event"),
+                               }
+                       }
+               };
+
+               let feemsat = 239; // somehow we know?
+               let total_fee_msat = (nodes.len() - 2) as u64 * 239;
+
+               let recv_value_0 = stat01.their_max_htlc_value_in_flight_msat - total_fee_msat;
+
+               // attempt to send amt_msat > their_max_htlc_value_in_flight_msat
+               {
+                       let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_0 + 1);
+                       assert!(route.hops.iter().rev().skip(1).all(|h| h.fee_msat == feemsat));
+                       let err = nodes[0].node.send_payment(route, our_payment_hash).err().unwrap();
+                       match err {
+                               APIError::RouteError{err} => assert_eq!(err, "Cannot send value that would put us over our max HTLC value in flight"),
+                               _ => panic!("Unknown error variants"),
+                       }
+               }
+
+               let mut htlc_id = 0;
+               // channel reserve is bigger than their_max_htlc_value_in_flight_msat so loop to deplete
+               // nodes[0]'s wealth
+               loop {
+                       let amt_msat = recv_value_0 + total_fee_msat;
+                       if stat01.value_to_self_msat - amt_msat < stat01.channel_reserve_msat {
+                               break;
+                       }
+                       send_payment(&nodes[0], &vec![&nodes[1], &nodes[2]][..], recv_value_0);
+                       htlc_id += 1;
+
+                       let (stat01_, stat11_, stat12_, stat22_) = (
+                               get_channel_value_stat!(nodes[0], chan_1.2),
+                               get_channel_value_stat!(nodes[1], chan_1.2),
+                               get_channel_value_stat!(nodes[1], chan_2.2),
+                               get_channel_value_stat!(nodes[2], chan_2.2),
+                       );
+
+                       assert_eq!(stat01_.value_to_self_msat, stat01.value_to_self_msat - amt_msat);
+                       assert_eq!(stat11_.value_to_self_msat, stat11.value_to_self_msat + amt_msat);
+                       assert_eq!(stat12_.value_to_self_msat, stat12.value_to_self_msat - (amt_msat - feemsat));
+                       assert_eq!(stat22_.value_to_self_msat, stat22.value_to_self_msat + (amt_msat - feemsat));
+                       stat01 = stat01_; stat11 = stat11_; stat12 = stat12_; stat22 = stat22_;
+               }
+
+               {
+                       let recv_value = stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat;
+                       // attempt to get channel_reserve violation
+                       let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value + 1);
+                       let err = nodes[0].node.send_payment(route.clone(), our_payment_hash).err().unwrap();
+                       match err {
+                               APIError::RouteError{err} => assert_eq!(err, "Cannot send value that would put us over our reserve value"),
+                               _ => panic!("Unknown error variants"),
+                       }
+               }
+
+               // adding pending output
+               let recv_value_1 = (stat01.value_to_self_msat - stat01.channel_reserve_msat - total_fee_msat)/2;
+               let amt_msat_1 = recv_value_1 + total_fee_msat;
+
+               let (route_1, our_payment_hash_1, our_payment_preimage_1) = get_route_and_payment_hash!(recv_value_1);
+               let payment_event_1 = {
+                       nodes[0].node.send_payment(route_1, our_payment_hash_1).unwrap();
+                       check_added_monitors!(nodes[0], 1);
+
+                       let mut events = nodes[0].node.get_and_clear_pending_events();
+                       assert_eq!(events.len(), 1);
+                       SendEvent::from_event(events.remove(0))
+               };
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event_1.msgs[0]).unwrap();
+
+               // channel reserve test with htlc pending output > 0
+               let recv_value_2 = stat01.value_to_self_msat - amt_msat_1 - stat01.channel_reserve_msat - total_fee_msat;
+               {
+                       let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1);
+                       match nodes[0].node.send_payment(route, our_payment_hash).err().unwrap() {
+                               APIError::RouteError{err} => assert_eq!(err, "Cannot send value that would put us over our reserve value"),
+                               _ => panic!("Unknown error variants"),
+                       }
+               }
+
+               {
+                       // test channel_reserve test on nodes[1] side
+                       let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_2 + 1);
+
+                       // Need to manually create update_add_htlc message to go around the channel reserve check in send_htlc()
+                       let secp_ctx = Secp256k1::new();
+                       let session_priv = SecretKey::from_slice(&secp_ctx, &{
+                               let mut session_key = [0; 32];
+                               rng::fill_bytes(&mut session_key);
+                               session_key
+                       }).expect("RNG is bad!");
+
+                       let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+                       let onion_keys = ChannelManager::construct_onion_keys(&secp_ctx, &route, &session_priv).unwrap();
+                       let (onion_payloads, htlc_msat, htlc_cltv) = ChannelManager::build_onion_payloads(&route, cur_height).unwrap();
+                       let onion_packet = ChannelManager::construct_onion_packet(onion_payloads, onion_keys, &our_payment_hash);
+                       let msg = msgs::UpdateAddHTLC {
+                               channel_id: chan_1.2,
+                               htlc_id,
+                               amount_msat: htlc_msat,
+                               payment_hash: our_payment_hash,
+                               cltv_expiry: htlc_cltv,
+                               onion_routing_packet: onion_packet,
+                       };
+
+                       let err = nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg).err().unwrap();
+                       match err {
+                               HandleError{err, .. } => assert_eq!(err, "Remote HTLC add would put them over their reserve value"),
+                       }
+               }
+
+               // split the rest to test holding cell
+               let recv_value_21 = recv_value_2/2;
+               let recv_value_22 = recv_value_2 - recv_value_21 - total_fee_msat;
+               {
+                       let stat = get_channel_value_stat!(nodes[0], chan_1.2);
+                       assert_eq!(stat.value_to_self_msat - (stat.pending_outbound_htlcs_amount_msat + recv_value_21 + recv_value_22 + total_fee_msat + total_fee_msat), stat.channel_reserve_msat);
+               }
+
+               // now see if they go through on both sides
+               let (route_21, our_payment_hash_21, our_payment_preimage_21) = get_route_and_payment_hash!(recv_value_21);
+               // but this will stuck in the holding cell
+               nodes[0].node.send_payment(route_21, our_payment_hash_21).unwrap();
+               check_added_monitors!(nodes[0], 0);
+               let events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 0);
+
+               // test with outbound holding cell amount > 0
+               {
+                       let (route, our_payment_hash, _) = get_route_and_payment_hash!(recv_value_22+1);
+                       match nodes[0].node.send_payment(route, our_payment_hash).err().unwrap() {
+                               APIError::RouteError{err} => assert_eq!(err, "Cannot send value that would put us over our reserve value"),
+                               _ => panic!("Unknown error variants"),
+                       }
+               }
+
+               let (route_22, our_payment_hash_22, our_payment_preimage_22) = get_route_and_payment_hash!(recv_value_22);
+               // this will also stuck in the holding cell
+               nodes[0].node.send_payment(route_22, our_payment_hash_22).unwrap();
+               check_added_monitors!(nodes[0], 0);
+               let events = nodes[0].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 0);
+
+               // flush the pending htlc
+               let (as_revoke_and_ack, as_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event_1.commitment_msg).unwrap();
+               check_added_monitors!(nodes[1], 1);
+
+               let commitment_update_2 = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_revoke_and_ack).unwrap().unwrap();
+               check_added_monitors!(nodes[0], 1);
+               let (bs_revoke_and_ack, bs_none) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &as_commitment_signed.unwrap()).unwrap();
+               assert!(bs_none.is_none());
+               check_added_monitors!(nodes[0], 1);
+               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
+               check_added_monitors!(nodes[1], 1);
+
+               expect_pending_htlcs_forwardable!(nodes[1]);
+
+               let ref payment_event_11 = expect_forward!(nodes[1]);
+               nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_11.msgs[0]).unwrap();
+               commitment_signed_dance!(nodes[2], nodes[1], payment_event_11.commitment_msg, false);
+
+               expect_pending_htlcs_forwardable!(nodes[2]);
+               expect_payment_received!(nodes[2], our_payment_hash_1, recv_value_1);
+
+               // flush the htlcs in the holding cell
+               assert_eq!(commitment_update_2.update_add_htlcs.len(), 2);
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &commitment_update_2.update_add_htlcs[0]).unwrap();
+               nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &commitment_update_2.update_add_htlcs[1]).unwrap();
+               commitment_signed_dance!(nodes[1], nodes[0], &commitment_update_2.commitment_signed, false);
+               expect_pending_htlcs_forwardable!(nodes[1]);
+
+               let ref payment_event_3 = expect_forward!(nodes[1]);
+               assert_eq!(payment_event_3.msgs.len(), 2);
+               nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_3.msgs[0]).unwrap();
+               nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_3.msgs[1]).unwrap();
+
+               commitment_signed_dance!(nodes[2], nodes[1], &payment_event_3.commitment_msg, false);
+               expect_pending_htlcs_forwardable!(nodes[2]);
+
+               let events = nodes[2].node.get_and_clear_pending_events();
+               assert_eq!(events.len(), 2);
+               match events[0] {
+                       Event::PaymentReceived { ref payment_hash, amt } => {
+                               assert_eq!(our_payment_hash_21, *payment_hash);
+                               assert_eq!(recv_value_21, amt);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+               match events[1] {
+                       Event::PaymentReceived { ref payment_hash, amt } => {
+                               assert_eq!(our_payment_hash_22, *payment_hash);
+                               assert_eq!(recv_value_22, amt);
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+
+               claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_1);
+               claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_21);
+               claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), our_payment_preimage_22);
+
+               let expected_value_to_self = stat01.value_to_self_msat - (recv_value_1 + total_fee_msat) - (recv_value_21 + total_fee_msat) - (recv_value_22 + total_fee_msat);
+               let stat0 = get_channel_value_stat!(nodes[0], chan_1.2);
+               assert_eq!(stat0.value_to_self_msat, expected_value_to_self);
+               assert_eq!(stat0.value_to_self_msat, stat0.channel_reserve_msat);
+
+               let stat2 = get_channel_value_stat!(nodes[2], chan_2.2);
+               assert_eq!(stat2.value_to_self_msat, stat22.value_to_self_msat + recv_value_1 + recv_value_21 + recv_value_22);
+       }
+
        #[test]
        fn channel_monitor_network_test() {
                // Simple test which builds a network of ChannelManagers, connects them to each other, and