Respond to channel_reestablish out-of-band for ordered delivery
[rust-lightning] / src / ln / channelmanager.rs
index bd663c9241235e7dee611f709e71541b9cf3ceac..7d11fe987f2c67d07a8a3fbc2cbe43bb3df1a7e0 100644 (file)
@@ -26,7 +26,7 @@ use ln::channel::{Channel, ChannelError};
 use ln::channelmonitor::{ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS};
 use ln::router::{Route,RouteHop};
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, HandleError, RAACommitmentOrder};
+use ln::msgs::{ChannelMessageHandler, HandleError};
 use chain::keysinterface::KeysInterface;
 use util::{byte_utils, events, internal_traits, rng};
 use util::sha2::Sha256;
@@ -244,6 +244,18 @@ struct HTLCForwardInfo {
        forward_info: PendingForwardHTLCInfo,
 }
 
+/// For events which result in both a RevokeAndACK and a CommitmentUpdate, by default they should
+/// be sent in the order they appear in the return value, however sometimes the order needs to be
+/// variable at runtime (eg Channel::channel_reestablish needs to re-send messages in the order
+/// they were originally sent). In those cases, this enum is also returned.
+#[derive(Clone, PartialEq)]
+pub(super) enum RAACommitmentOrder {
+       /// Send the CommitmentUpdate messages first
+       CommitmentFirst,
+       /// Send the RevokeAndACK message first
+       RevokeAndACKFirst,
+}
+
 struct ChannelHolder {
        by_id: HashMap<[u8; 32], Channel>,
        short_to_id: HashMap<u64, [u8; 32]>,
@@ -1617,20 +1629,26 @@ impl ChannelManager {
                }
        }
 
-       fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<msgs::AcceptChannel, MsgHandleErrInternal> {
+       fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
                if msg.chain_hash != self.genesis_hash {
                        return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash", msg.temporary_channel_id.clone()));
                }
-               let mut channel_state = self.channel_state.lock().unwrap();
-               if channel_state.by_id.contains_key(&msg.temporary_channel_id) {
-                       return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision!", msg.temporary_channel_id.clone()));
-               }
 
                let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), msg, 0, false, self.announce_channels_publicly, Arc::clone(&self.logger))
                        .map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
-               let accept_msg = channel.get_accept_channel();
-               channel_state.by_id.insert(channel.channel_id(), channel);
-               Ok(accept_msg)
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
+               match channel_state.by_id.entry(channel.channel_id()) {
+                       hash_map::Entry::Occupied(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision!", msg.temporary_channel_id.clone())),
+                       hash_map::Entry::Vacant(entry) => {
+                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel {
+                                       node_id: their_node_id.clone(),
+                                       msg: channel.get_accept_channel(),
+                               });
+                               entry.insert(channel);
+                       }
+               }
+               Ok(())
        }
 
        fn internal_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
@@ -1660,7 +1678,7 @@ impl ChannelManager {
                Ok(())
        }
 
-       fn internal_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, MsgHandleErrInternal> {
+       fn internal_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), MsgHandleErrInternal> {
                let (chan, funding_msg, monitor_update) = {
                        let mut channel_state = self.channel_state.lock().unwrap();
                        match channel_state.by_id.entry(msg.temporary_channel_id.clone()) {
@@ -1686,16 +1704,21 @@ impl ChannelManager {
                if let Err(_e) = self.monitor.add_update_monitor(monitor_update.get_funding_txo().unwrap(), monitor_update) {
                        unimplemented!();
                }
-               let mut channel_state = self.channel_state.lock().unwrap();
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
                match channel_state.by_id.entry(funding_msg.channel_id) {
                        hash_map::Entry::Occupied(_) => {
                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Already had channel with the new channel_id", funding_msg.channel_id))
                        },
                        hash_map::Entry::Vacant(e) => {
+                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
+                                       node_id: their_node_id.clone(),
+                                       msg: funding_msg,
+                               });
                                e.insert(chan);
                        }
                }
-               Ok(funding_msg)
+               Ok(())
        }
 
        fn internal_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> {
@@ -1724,8 +1747,9 @@ impl ChannelManager {
                Ok(())
        }
 
-       fn internal_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) -> Result<Option<msgs::AnnouncementSignatures>, MsgHandleErrInternal> {
-               let mut channel_state = self.channel_state.lock().unwrap();
+       fn internal_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) -> Result<(), MsgHandleErrInternal> {
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
                match channel_state.by_id.get_mut(&msg.channel_id) {
                        Some(chan) => {
                                if chan.get_their_node_id() != *their_node_id {
@@ -1734,14 +1758,20 @@ impl ChannelManager {
                                }
                                chan.funding_locked(&msg)
                                        .map_err(|e| MsgHandleErrInternal::from_chan_maybe_close(e, msg.channel_id))?;
-                               return Ok(self.get_announcement_sigs(chan));
+                               if let Some(announcement_sigs) = self.get_announcement_sigs(chan) {
+                                       channel_state.pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+                                               node_id: their_node_id.clone(),
+                                               msg: announcement_sigs,
+                                       });
+                               }
+                               Ok(())
                        },
-                       None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
-               };
+                       None => Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
+               }
        }
 
-       fn internal_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>), MsgHandleErrInternal> {
-               let (mut res, chan_option) = {
+       fn internal_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), MsgHandleErrInternal> {
+               let (mut dropped_htlcs, chan_option) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = channel_state_lock.borrow_parts();
 
@@ -1751,18 +1781,30 @@ impl ChannelManager {
                                                //TODO: here and below MsgHandleErrInternal, #153 case
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                        }
-                                       let res = chan_entry.get_mut().shutdown(&*self.fee_estimator, &msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       let (shutdown, closing_signed, dropped_htlcs) = chan_entry.get_mut().shutdown(&*self.fee_estimator, &msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       if let Some(msg) = shutdown {
+                                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
+                                                       node_id: their_node_id.clone(),
+                                                       msg,
+                                               });
+                                       }
+                                       if let Some(msg) = closing_signed {
+                                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendClosingSigned {
+                                                       node_id: their_node_id.clone(),
+                                                       msg,
+                                               });
+                                       }
                                        if chan_entry.get().is_shutdown() {
                                                if let Some(short_id) = chan_entry.get().get_short_channel_id() {
                                                        channel_state.short_to_id.remove(&short_id);
                                                }
-                                               (res, Some(chan_entry.remove_entry().1))
-                                       } else { (res, None) }
+                                               (dropped_htlcs, Some(chan_entry.remove_entry().1))
+                                       } else { (dropped_htlcs, None) }
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                        }
                };
-               for htlc_source in res.2.drain(..) {
+               for htlc_source in dropped_htlcs.drain(..) {
                        // unknown_next_peer...I dunno who that is anymore....
                        self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
                }
@@ -1774,11 +1816,11 @@ impl ChannelManager {
                                });
                        }
                }
-               Ok((res.0, res.1))
+               Ok(())
        }
 
-       fn internal_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<Option<msgs::ClosingSigned>, MsgHandleErrInternal> {
-               let (res, chan_option) = {
+       fn internal_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), MsgHandleErrInternal> {
+               let (tx, chan_option) = {
                        let mut channel_state_lock = self.channel_state.lock().unwrap();
                        let channel_state = channel_state_lock.borrow_parts();
                        match channel_state.by_id.entry(msg.channel_id.clone()) {
@@ -1787,8 +1829,14 @@ impl ChannelManager {
                                                //TODO: here and below MsgHandleErrInternal, #153 case
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                        }
-                                       let res = chan_entry.get_mut().closing_signed(&*self.fee_estimator, &msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
-                                       if res.1.is_some() {
+                                       let (closing_signed, tx) = chan_entry.get_mut().closing_signed(&*self.fee_estimator, &msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       if let Some(msg) = closing_signed {
+                                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendClosingSigned {
+                                                       node_id: their_node_id.clone(),
+                                                       msg,
+                                               });
+                                       }
+                                       if tx.is_some() {
                                                // We're done with this channel, we've got a signed closing transaction and
                                                // will send the closing_signed back to the remote peer upon return. This
                                                // also implies there are no pending HTLCs left on the channel, so we can
@@ -1797,13 +1845,13 @@ impl ChannelManager {
                                                if let Some(short_id) = chan_entry.get().get_short_channel_id() {
                                                        channel_state.short_to_id.remove(&short_id);
                                                }
-                                               (res, Some(chan_entry.remove_entry().1))
-                                       } else { (res, None) }
+                                               (tx, Some(chan_entry.remove_entry().1))
+                                       } else { (tx, None) }
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                        }
                };
-               if let Some(broadcast_tx) = res.1 {
+               if let Some(broadcast_tx) = tx {
                        self.tx_broadcaster.broadcast_transaction(&broadcast_tx);
                }
                if let Some(chan) = chan_option {
@@ -1814,7 +1862,7 @@ impl ChannelManager {
                                });
                        }
                }
-               Ok(res.0)
+               Ok(())
        }
 
        fn internal_update_add_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateAddHTLC) -> Result<(), MsgHandleErrInternal> {
@@ -2094,25 +2142,40 @@ impl ChannelManager {
                }
        }
 
-       fn internal_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>), MsgHandleErrInternal> {
-               let (revoke_and_ack, commitment_signed) = {
-                       let mut channel_state = self.channel_state.lock().unwrap();
-                       match channel_state.by_id.get_mut(&msg.channel_id) {
-                               Some(chan) => {
-                                       if chan.get_their_node_id() != *their_node_id {
-                                               //TODO: here and below MsgHandleErrInternal, #153 case
-                                               return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
-                                       }
-                                       let (revoke_and_ack, commitment_signed, chan_monitor) = chan.commitment_signed(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
-                                       if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
-                                               unimplemented!();
-                                       }
-                                       (revoke_and_ack, commitment_signed)
-                               },
-                               None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
-                       }
-               };
-               Ok((revoke_and_ack, commitment_signed))
+       fn internal_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(), MsgHandleErrInternal> {
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
+               match channel_state.by_id.get_mut(&msg.channel_id) {
+                       Some(chan) => {
+                               if chan.get_their_node_id() != *their_node_id {
+                                       //TODO: here and below MsgHandleErrInternal, #153 case
+                                       return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
+                               }
+                               let (revoke_and_ack, commitment_signed, chan_monitor) = chan.commitment_signed(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                               if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
+                                       unimplemented!();
+                               }
+                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendRevokeAndACK {
+                                       node_id: their_node_id.clone(),
+                                       msg: revoke_and_ack,
+                               });
+                               if let Some(msg) = commitment_signed {
+                                       channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
+                                               node_id: their_node_id.clone(),
+                                               updates: msgs::CommitmentUpdate {
+                                                       update_add_htlcs: Vec::new(),
+                                                       update_fulfill_htlcs: Vec::new(),
+                                                       update_fail_htlcs: Vec::new(),
+                                                       update_fail_malformed_htlcs: Vec::new(),
+                                                       update_fee: None,
+                                                       commitment_signed: msg,
+                                               },
+                                       });
+                               }
+                               Ok(())
+                       },
+                       None => Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
+               }
        }
 
        #[inline]
@@ -2148,20 +2211,27 @@ impl ChannelManager {
                }
        }
 
-       fn internal_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<Option<msgs::CommitmentUpdate>, MsgHandleErrInternal> {
-               let ((res, pending_forwards, mut pending_failures), short_channel_id) = {
-                       let mut channel_state = self.channel_state.lock().unwrap();
+       fn internal_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<(), MsgHandleErrInternal> {
+               let (pending_forwards, mut pending_failures, short_channel_id) = {
+                       let mut channel_state_lock = self.channel_state.lock().unwrap();
+                       let channel_state = channel_state_lock.borrow_parts();
                        match channel_state.by_id.get_mut(&msg.channel_id) {
                                Some(chan) => {
                                        if chan.get_their_node_id() != *their_node_id {
                                                //TODO: here and below MsgHandleErrInternal, #153 case
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
                                        }
-                                       let (res, pending_forwards, pending_failures, chan_monitor) = chan.revoke_and_ack(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       let (commitment_update, pending_forwards, pending_failures, chan_monitor) = chan.revoke_and_ack(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
                                        if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) {
                                                unimplemented!();
                                        }
-                                       ((res, pending_forwards, pending_failures), chan.get_short_channel_id().expect("RAA should only work on a short-id-available channel"))
+                                       if let Some(updates) = commitment_update {
+                                               channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
+                                                       node_id: their_node_id.clone(),
+                                                       updates,
+                                               });
+                                       }
+                                       (pending_forwards, pending_failures, chan.get_short_channel_id().expect("RAA should only work on a short-id-available channel"))
                                },
                                None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
                        }
@@ -2171,7 +2241,7 @@ impl ChannelManager {
                }
                self.forward_htlcs(&mut [(short_channel_id, pending_forwards)]);
 
-               Ok(res)
+               Ok(())
        }
 
        fn internal_update_fee(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFee) -> Result<(), MsgHandleErrInternal> {
@@ -2229,28 +2299,58 @@ impl ChannelManager {
                Ok(())
        }
 
-       fn internal_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder), MsgHandleErrInternal> {
-               let res = {
-                       let mut channel_state = self.channel_state.lock().unwrap();
-                       match channel_state.by_id.get_mut(&msg.channel_id) {
-                               Some(chan) => {
-                                       if chan.get_their_node_id() != *their_node_id {
-                                               return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
+       fn internal_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), MsgHandleErrInternal> {
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
+
+               match channel_state.by_id.get_mut(&msg.channel_id) {
+                       Some(chan) => {
+                               if chan.get_their_node_id() != *their_node_id {
+                                       return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id));
+                               }
+                               let (funding_locked, revoke_and_ack, commitment_update, channel_monitor, order) = chan.channel_reestablish(msg)
+                                       .map_err(|e| MsgHandleErrInternal::from_chan_maybe_close(e, msg.channel_id))?;
+                               if let Some(monitor) = channel_monitor {
+                                       if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
+                                               unimplemented!();
                                        }
-                                       let (funding_locked, revoke_and_ack, commitment_update, channel_monitor, order) = chan.channel_reestablish(msg)
-                                               .map_err(|e| MsgHandleErrInternal::from_chan_maybe_close(e, msg.channel_id))?;
-                                       if let Some(monitor) = channel_monitor {
-                                               if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
-                                                       unimplemented!();
-                                               }
+                               }
+                               if let Some(msg) = funding_locked {
+                                       channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
+                                               node_id: their_node_id.clone(),
+                                               msg
+                                       });
+                               }
+                               macro_rules! send_raa { () => {
+                                       if let Some(msg) = revoke_and_ack {
+                                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendRevokeAndACK {
+                                                       node_id: their_node_id.clone(),
+                                                       msg
+                                               });
                                        }
-                                       Ok((funding_locked, revoke_and_ack, commitment_update, order))
-                               },
-                               None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
-                       }
-               };
-
-               res
+                               } }
+                               macro_rules! send_cu { () => {
+                                       if let Some(updates) = commitment_update {
+                                               channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
+                                                       node_id: their_node_id.clone(),
+                                                       updates
+                                               });
+                                       }
+                               } }
+                               match order {
+                                       RAACommitmentOrder::RevokeAndACKFirst => {
+                                               send_raa!();
+                                               send_cu!();
+                                       },
+                                       RAACommitmentOrder::CommitmentFirst => {
+                                               send_cu!();
+                                               send_raa!();
+                                       },
+                               }
+                               Ok(())
+                       },
+                       None => Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
+               }
        }
 
        /// Begin Update fee process. Allowed only on an outbound channel.
@@ -2325,12 +2425,16 @@ impl ChainListener for ChannelManager {
                        channel_state.by_id.retain(|_, channel| {
                                let chan_res = channel.block_connected(header, height, txn_matched, indexes_of_txn_matched);
                                if let Ok(Some(funding_locked)) = chan_res {
-                                       let announcement_sigs = self.get_announcement_sigs(channel);
                                        pending_msg_events.push(events::MessageSendEvent::SendFundingLocked {
                                                node_id: channel.get_their_node_id(),
                                                msg: funding_locked,
-                                               announcement_sigs: announcement_sigs
                                        });
+                                       if let Some(announcement_sigs) = self.get_announcement_sigs(channel) {
+                                               pending_msg_events.push(events::MessageSendEvent::SendAnnouncementSignatures {
+                                                       node_id: channel.get_their_node_id(),
+                                                       msg: announcement_sigs,
+                                               });
+                                       }
                                        short_to_id.insert(channel.get_short_channel_id().unwrap(), channel.channel_id());
                                } else if let Err(e) = chan_res {
                                        pending_msg_events.push(events::MessageSendEvent::HandleError {
@@ -2453,7 +2557,7 @@ macro_rules! handle_error {
 
 impl ChannelMessageHandler for ChannelManager {
        //TODO: Handle errors and close channel (or so)
-       fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<msgs::AcceptChannel, HandleError> {
+       fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), HandleError> {
                handle_error!(self, self.internal_open_channel(their_node_id, msg), their_node_id)
        }
 
@@ -2461,7 +2565,7 @@ impl ChannelMessageHandler for ChannelManager {
                handle_error!(self, self.internal_accept_channel(their_node_id, msg), their_node_id)
        }
 
-       fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, HandleError> {
+       fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), HandleError> {
                handle_error!(self, self.internal_funding_created(their_node_id, msg), their_node_id)
        }
 
@@ -2469,15 +2573,15 @@ impl ChannelMessageHandler for ChannelManager {
                handle_error!(self, self.internal_funding_signed(their_node_id, msg), their_node_id)
        }
 
-       fn handle_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) -> Result<Option<msgs::AnnouncementSignatures>, HandleError> {
+       fn handle_funding_locked(&self, their_node_id: &PublicKey, msg: &msgs::FundingLocked) -> Result<(), HandleError> {
                handle_error!(self, self.internal_funding_locked(their_node_id, msg), their_node_id)
        }
 
-       fn handle_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>), HandleError> {
+       fn handle_shutdown(&self, their_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), HandleError> {
                handle_error!(self, self.internal_shutdown(their_node_id, msg), their_node_id)
        }
 
-       fn handle_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<Option<msgs::ClosingSigned>, HandleError> {
+       fn handle_closing_signed(&self, their_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), HandleError> {
                handle_error!(self, self.internal_closing_signed(their_node_id, msg), their_node_id)
        }
 
@@ -2497,11 +2601,11 @@ impl ChannelMessageHandler for ChannelManager {
                handle_error!(self, self.internal_update_fail_malformed_htlc(their_node_id, msg), their_node_id)
        }
 
-       fn handle_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(msgs::RevokeAndACK, Option<msgs::CommitmentSigned>), HandleError> {
+       fn handle_commitment_signed(&self, their_node_id: &PublicKey, msg: &msgs::CommitmentSigned) -> Result<(), HandleError> {
                handle_error!(self, self.internal_commitment_signed(their_node_id, msg), their_node_id)
        }
 
-       fn handle_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<Option<msgs::CommitmentUpdate>, HandleError> {
+       fn handle_revoke_and_ack(&self, their_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<(), HandleError> {
                handle_error!(self, self.internal_revoke_and_ack(their_node_id, msg), their_node_id)
        }
 
@@ -2513,7 +2617,7 @@ impl ChannelMessageHandler for ChannelManager {
                handle_error!(self, self.internal_announcement_signatures(their_node_id, msg), their_node_id)
        }
 
-       fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, RAACommitmentOrder), HandleError> {
+       fn handle_channel_reestablish(&self, their_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), HandleError> {
                handle_error!(self, self.internal_channel_reestablish(their_node_id, msg), their_node_id)
        }
 
@@ -2613,7 +2717,7 @@ mod tests {
        use chain::chaininterface::ChainListener;
        use chain::keysinterface::KeysInterface;
        use chain::keysinterface;
-       use ln::channelmanager::{ChannelManager,OnionKeys,PaymentFailReason};
+       use ln::channelmanager::{ChannelManager,OnionKeys,PaymentFailReason,RAACommitmentOrder};
        use ln::channelmonitor::{ChannelMonitorUpdateErr, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS};
        use ln::router::{Route, RouteHop, Router};
        use ln::msgs;
@@ -2841,20 +2945,69 @@ mod tests {
                (announcement, as_update, bs_update, channel_id, tx)
        }
 
-       fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> Transaction {
-               node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
+       macro_rules! get_revoke_commit_msgs {
+               ($node: expr, $node_id: expr) => {
+                       {
+                               let events = $node.node.get_and_clear_pending_msg_events();
+                               assert_eq!(events.len(), 2);
+                               (match events[0] {
+                                       MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
+                                               assert_eq!(*node_id, $node_id);
+                                               (*msg).clone()
+                                       },
+                                       _ => panic!("Unexpected event"),
+                               }, match events[1] {
+                                       MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
+                                               assert_eq!(*node_id, $node_id);
+                                               assert!(updates.update_add_htlcs.is_empty());
+                                               assert!(updates.update_fulfill_htlcs.is_empty());
+                                               assert!(updates.update_fail_htlcs.is_empty());
+                                               assert!(updates.update_fail_malformed_htlcs.is_empty());
+                                               assert!(updates.update_fee.is_none());
+                                               updates.commitment_signed.clone()
+                                       },
+                                       _ => panic!("Unexpected event"),
+                               })
+                       }
+               }
+       }
 
-               let events_1 = node_a.node.get_and_clear_pending_msg_events();
-               assert_eq!(events_1.len(), 1);
-               let accept_chan = match events_1[0] {
-                       MessageSendEvent::SendOpenChannel { ref node_id, ref msg } => {
-                               assert_eq!(*node_id, node_b.node.get_our_node_id());
-                               node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), msg).unwrap()
-                       },
-                       _ => panic!("Unexpected event"),
-               };
+       macro_rules! get_event_msg {
+               ($node: expr, $event_type: path, $node_id: expr) => {
+                       {
+                               let events = $node.node.get_and_clear_pending_msg_events();
+                               assert_eq!(events.len(), 1);
+                               match events[0] {
+                                       $event_type { ref node_id, ref msg } => {
+                                               assert_eq!(*node_id, $node_id);
+                                               (*msg).clone()
+                                       },
+                                       _ => panic!("Unexpected event"),
+                               }
+                       }
+               }
+       }
 
-               node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &accept_chan).unwrap();
+       macro_rules! get_htlc_update_msgs {
+               ($node: expr, $node_id: expr) => {
+                       {
+                               let events = $node.node.get_and_clear_pending_msg_events();
+                               assert_eq!(events.len(), 1);
+                               match events[0] {
+                                       MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
+                                               assert_eq!(*node_id, $node_id);
+                                               (*updates).clone()
+                                       },
+                                       _ => panic!("Unexpected event"),
+                               }
+                       }
+               }
+       }
+
+       fn create_chan_between_nodes_with_value_init(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> Transaction {
+               node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42).unwrap();
+               node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id())).unwrap();
+               node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id())).unwrap();
 
                let chan_id = *node_a.network_chan_count.borrow();
                let tx;
@@ -2881,22 +3034,15 @@ mod tests {
                        _ => panic!("Unexpected event"),
                }
 
-               let events_3 = node_a.node.get_and_clear_pending_msg_events();
-               assert_eq!(events_3.len(), 1);
-               let funding_signed = match events_3[0] {
-                       MessageSendEvent::SendFundingCreated { ref node_id, ref msg } => {
-                               assert_eq!(*node_id, node_b.node.get_our_node_id());
-                               let res = node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), msg).unwrap();
-                               let mut added_monitors = node_b.chan_monitor.added_monitors.lock().unwrap();
-                               assert_eq!(added_monitors.len(), 1);
-                               assert_eq!(added_monitors[0].0, funding_output);
-                               added_monitors.clear();
-                               res
-                       },
-                       _ => panic!("Unexpected event"),
-               };
+               node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id())).unwrap();
+               {
+                       let mut added_monitors = node_b.chan_monitor.added_monitors.lock().unwrap();
+                       assert_eq!(added_monitors.len(), 1);
+                       assert_eq!(added_monitors[0].0, funding_output);
+                       added_monitors.clear();
+               }
 
-               node_a.node.handle_funding_signed(&node_b.node.get_our_node_id(), &funding_signed).unwrap();
+               node_a.node.handle_funding_signed(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a.node.get_our_node_id())).unwrap();
                {
                        let mut added_monitors = node_a.chan_monitor.added_monitors.lock().unwrap();
                        assert_eq!(added_monitors.len(), 1);
@@ -2919,30 +3065,27 @@ mod tests {
 
        fn create_chan_between_nodes_with_value_confirm(node_a: &Node, node_b: &Node, tx: &Transaction) -> ((msgs::FundingLocked, msgs::AnnouncementSignatures), [u8; 32]) {
                confirm_transaction(&node_b.chain_monitor, &tx, tx.version);
-               let events_5 = node_b.node.get_and_clear_pending_msg_events();
-               assert_eq!(events_5.len(), 1);
-               match events_5[0] {
-                       MessageSendEvent::SendFundingLocked { ref node_id, ref msg, ref announcement_sigs } => {
-                               assert_eq!(*node_id, node_a.node.get_our_node_id());
-                               assert!(announcement_sigs.is_none());
-                               node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), msg).unwrap()
-                       },
-                       _ => panic!("Unexpected event"),
-               };
+               node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendFundingLocked, node_a.node.get_our_node_id())).unwrap();
 
                let channel_id;
 
                confirm_transaction(&node_a.chain_monitor, &tx, tx.version);
                let events_6 = node_a.node.get_and_clear_pending_msg_events();
-               assert_eq!(events_6.len(), 1);
-               (match events_6[0] {
-                       MessageSendEvent::SendFundingLocked { ref node_id, ref msg, ref announcement_sigs } => {
+               assert_eq!(events_6.len(), 2);
+               ((match events_6[0] {
+                       MessageSendEvent::SendFundingLocked { ref node_id, ref msg } => {
                                channel_id = msg.channel_id.clone();
                                assert_eq!(*node_id, node_b.node.get_our_node_id());
-                               (msg.clone(), announcement_sigs.clone().unwrap())
+                               msg.clone()
                        },
                        _ => panic!("Unexpected event"),
-               }, channel_id)
+               }, match events_6[1] {
+                       MessageSendEvent::SendAnnouncementSignatures { ref node_id, ref msg } => {
+                               assert_eq!(*node_id, node_b.node.get_our_node_id());
+                               msg.clone()
+                       },
+                       _ => panic!("Unexpected event"),
+               }), channel_id)
        }
 
        fn create_chan_between_nodes_with_value_a(node_a: &Node, node_b: &Node, channel_value: u64, push_msat: u64) -> ((msgs::FundingLocked, msgs::AnnouncementSignatures), [u8; 32], Transaction) {
@@ -2952,11 +3095,9 @@ mod tests {
        }
 
        fn create_chan_between_nodes_with_value_b(node_a: &Node, node_b: &Node, as_funding_msgs: &(msgs::FundingLocked, msgs::AnnouncementSignatures)) -> (msgs::ChannelAnnouncement, msgs::ChannelUpdate, msgs::ChannelUpdate) {
-               let bs_announcement_sigs = {
-                       let bs_announcement_sigs = node_b.node.handle_funding_locked(&node_a.node.get_our_node_id(), &as_funding_msgs.0).unwrap().unwrap();
-                       node_b.node.handle_announcement_signatures(&node_a.node.get_our_node_id(), &as_funding_msgs.1).unwrap();
-                       bs_announcement_sigs
-               };
+               node_b.node.handle_funding_locked(&node_a.node.get_our_node_id(), &as_funding_msgs.0).unwrap();
+               let bs_announcement_sigs = get_event_msg!(node_b, MessageSendEvent::SendAnnouncementSignatures, node_a.node.get_our_node_id());
+               node_b.node.handle_announcement_signatures(&node_a.node.get_our_node_id(), &as_funding_msgs.1).unwrap();
 
                let events_7 = node_b.node.get_and_clear_pending_msg_events();
                assert_eq!(events_7.len(), 1);
@@ -3009,68 +3150,91 @@ mod tests {
        }
 
        fn close_channel(outbound_node: &Node, inbound_node: &Node, channel_id: &[u8; 32], funding_tx: Transaction, close_inbound_first: bool) -> (msgs::ChannelUpdate, msgs::ChannelUpdate) {
-               let (node_a, broadcaster_a) = if close_inbound_first { (&inbound_node.node, &inbound_node.tx_broadcaster) } else { (&outbound_node.node, &outbound_node.tx_broadcaster) };
+               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) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster) } else { (&inbound_node.node, &inbound_node.tx_broadcaster) };
                let (tx_a, tx_b);
 
                node_a.close_channel(channel_id).unwrap();
-               let events_1 = node_a.get_and_clear_pending_msg_events();
-               assert_eq!(events_1.len(), 1);
-               let shutdown_a = match events_1[0] {
+               node_b.handle_shutdown(&node_a.get_our_node_id(), &get_event_msg!(struct_a, MessageSendEvent::SendShutdown, node_b.get_our_node_id())).unwrap();
+
+               let events_1 = node_b.get_and_clear_pending_msg_events();
+               assert!(events_1.len() >= 1);
+               let shutdown_b = match events_1[0] {
                        MessageSendEvent::SendShutdown { ref node_id, ref msg } => {
-                               assert_eq!(node_id, &node_b.get_our_node_id());
+                               assert_eq!(node_id, &node_a.get_our_node_id());
                                msg.clone()
                        },
                        _ => panic!("Unexpected event"),
                };
 
-               let (shutdown_b, mut closing_signed_b) = node_b.handle_shutdown(&node_a.get_our_node_id(), &shutdown_a).unwrap();
-               if !close_inbound_first {
-                       assert!(closing_signed_b.is_none());
+               let closing_signed_b = if !close_inbound_first {
+                       assert_eq!(events_1.len(), 1);
+                       None
+               } else {
+                       Some(match events_1[1] {
+                               MessageSendEvent::SendClosingSigned { ref node_id, ref msg } => {
+                                       assert_eq!(node_id, &node_a.get_our_node_id());
+                                       msg.clone()
+                               },
+                               _ => panic!("Unexpected event"),
+                       })
+               };
+
+               macro_rules! get_closing_signed_broadcast {
+                       ($node: expr, $dest_pubkey: expr) => {
+                               {
+                                       let events = $node.get_and_clear_pending_msg_events();
+                                       assert!(events.len() == 1 || events.len() == 2);
+                                       (match events[events.len() - 1] {
+                                               MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
+                                                       msg.clone()
+                                               },
+                                               _ => panic!("Unexpected event"),
+                                       }, if events.len() == 2 {
+                                               match events[0] {
+                                                       MessageSendEvent::SendClosingSigned { ref node_id, ref msg } => {
+                                                               assert_eq!(*node_id, $dest_pubkey);
+                                                               Some(msg.clone())
+                                                       },
+                                                       _ => panic!("Unexpected event"),
+                                               }
+                                       } else { None })
+                               }
+                       }
                }
-               let (empty_a, mut closing_signed_a) = node_a.handle_shutdown(&node_b.get_our_node_id(), &shutdown_b.unwrap()).unwrap();
-               assert!(empty_a.is_none());
-               if close_inbound_first {
-                       assert!(closing_signed_a.is_none());
-                       closing_signed_a = node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap()).unwrap();
+
+               node_a.handle_shutdown(&node_b.get_our_node_id(), &shutdown_b).unwrap();
+               let (as_update, bs_update) = if close_inbound_first {
+                       assert!(node_a.get_and_clear_pending_msg_events().is_empty());
+                       node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap()).unwrap();
                        assert_eq!(broadcaster_a.txn_broadcasted.lock().unwrap().len(), 1);
                        tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
+                       let (as_update, closing_signed_a) = get_closing_signed_broadcast!(node_a, node_b.get_our_node_id());
 
-                       let empty_b = node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a.unwrap()).unwrap();
-                       assert!(empty_b.is_none());
+                       node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a.unwrap()).unwrap();
+                       let (bs_update, none_b) = get_closing_signed_broadcast!(node_b, node_a.get_our_node_id());
+                       assert!(none_b.is_none());
                        assert_eq!(broadcaster_b.txn_broadcasted.lock().unwrap().len(), 1);
                        tx_b = broadcaster_b.txn_broadcasted.lock().unwrap().remove(0);
+                       (as_update, bs_update)
                } else {
-                       closing_signed_b = node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a.unwrap()).unwrap();
+                       let closing_signed_a = get_event_msg!(struct_a, MessageSendEvent::SendClosingSigned, node_b.get_our_node_id());
+
+                       node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a).unwrap();
                        assert_eq!(broadcaster_b.txn_broadcasted.lock().unwrap().len(), 1);
                        tx_b = broadcaster_b.txn_broadcasted.lock().unwrap().remove(0);
+                       let (bs_update, closing_signed_b) = get_closing_signed_broadcast!(node_b, node_a.get_our_node_id());
 
-                       let empty_a2 = node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap()).unwrap();
-                       assert!(empty_a2.is_none());
+                       node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap()).unwrap();
+                       let (as_update, none_a) = get_closing_signed_broadcast!(node_a, node_b.get_our_node_id());
+                       assert!(none_a.is_none());
                        assert_eq!(broadcaster_a.txn_broadcasted.lock().unwrap().len(), 1);
                        tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
-               }
+                       (as_update, bs_update)
+               };
                assert_eq!(tx_a, tx_b);
                check_spends!(tx_a, funding_tx);
 
-               let events_2 = node_a.get_and_clear_pending_msg_events();
-               assert_eq!(events_2.len(), 1);
-               let as_update = match events_2[0] {
-                       MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
-                               msg.clone()
-                       },
-                       _ => panic!("Unexpected event"),
-               };
-
-               let events_3 = node_b.get_and_clear_pending_msg_events();
-               assert_eq!(events_3.len(), 1);
-               let bs_update = match events_3[0] {
-                       MessageSendEvent::BroadcastChannelUpdate { ref msg } => {
-                               msg.clone()
-                       },
-                       _ => panic!("Unexpected event"),
-               };
-
                (as_update, bs_update)
        }
 
@@ -3110,19 +3274,32 @@ mod tests {
                ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => {
                        {
                                check_added_monitors!($node_a, 0);
-                               let (as_revoke_and_ack, as_commitment_signed) = $node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed).unwrap();
+                               assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
+                               $node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed).unwrap();
+                               let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!($node_a, $node_b.node.get_our_node_id());
                                check_added_monitors!($node_a, 1);
                                check_added_monitors!($node_b, 0);
-                               assert!($node_b.node.handle_revoke_and_ack(&$node_a.node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
+                               assert!($node_b.node.get_and_clear_pending_msg_events().is_empty());
+                               $node_b.node.handle_revoke_and_ack(&$node_a.node.get_our_node_id(), &as_revoke_and_ack).unwrap();
+                               assert!($node_b.node.get_and_clear_pending_msg_events().is_empty());
                                check_added_monitors!($node_b, 1);
-                               let (bs_revoke_and_ack, bs_none) = $node_b.node.handle_commitment_signed(&$node_a.node.get_our_node_id(), &as_commitment_signed.unwrap()).unwrap();
-                               assert!(bs_none.is_none());
+                               $node_b.node.handle_commitment_signed(&$node_a.node.get_our_node_id(), &as_commitment_signed).unwrap();
+                               let bs_revoke_and_ack = get_event_msg!($node_b, MessageSendEvent::SendRevokeAndACK, $node_a.node.get_our_node_id());
                                check_added_monitors!($node_b, 1);
                                if $fail_backwards {
                                        assert!($node_a.node.get_and_clear_pending_events().is_empty());
                                        assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
                                }
-                               assert!($node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
+                               $node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+                               if $fail_backwards {
+                                       let channel_state = $node_a.node.channel_state.lock().unwrap();
+                                       assert_eq!(channel_state.pending_msg_events.len(), 1);
+                                       if let MessageSendEvent::UpdateHTLCs { ref node_id, .. } = channel_state.pending_msg_events[0] {
+                                               assert_ne!(*node_id, $node_b.node.get_our_node_id());
+                                       } else { panic!("Unexpected event"); }
+                               } else {
+                                       assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
+                               }
                                {
                                        let mut added_monitors = $node_a.chan_monitor.added_monitors.lock().unwrap();
                                        if $fail_backwards {
@@ -3210,45 +3387,65 @@ mod tests {
                check_added_monitors!(expected_route.last().unwrap(), 1);
 
                let mut next_msgs: Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)> = None;
-               macro_rules! update_fulfill_dance {
-                       ($node: expr, $prev_node: expr, $last_node: expr) => {
+               let mut expected_next_node = expected_route.last().unwrap().node.get_our_node_id();
+               macro_rules! get_next_msgs {
+                       ($node: expr) => {
+                               {
+                                       let events = $node.node.get_and_clear_pending_msg_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_eq!(update_fulfill_htlcs.len(), 1);
+                                                       assert!(update_fail_htlcs.is_empty());
+                                                       assert!(update_fail_malformed_htlcs.is_empty());
+                                                       assert!(update_fee.is_none());
+                                                       expected_next_node = node_id.clone();
+                                                       Some((update_fulfill_htlcs[0].clone(), commitment_signed.clone()))
+                                               },
+                                               _ => panic!("Unexpected event"),
+                                       }
+                               }
+                       }
+               }
+
+               macro_rules! last_update_fulfill_dance {
+                       ($node: expr, $prev_node: expr) => {
+                               {
+                                       $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0).unwrap();
+                                       check_added_monitors!($node, 0);
+                                       assert!($node.node.get_and_clear_pending_msg_events().is_empty());
+                                       commitment_signed_dance!($node, $prev_node, next_msgs.as_ref().unwrap().1, false);
+                               }
+                       }
+               }
+               macro_rules! mid_update_fulfill_dance {
+                       ($node: expr, $prev_node: expr, $new_msgs: expr) => {
                                {
                                        $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0).unwrap();
-                                       if $last_node {
-                                               check_added_monitors!($node, 0);
+                                       check_added_monitors!($node, 1);
+                                       let new_next_msgs = if $new_msgs {
+                                               get_next_msgs!($node)
                                        } else {
-                                               check_added_monitors!($node, 1);
-                                       }
+                                               assert!($node.node.get_and_clear_pending_msg_events().is_empty());
+                                               None
+                                       };
                                        commitment_signed_dance!($node, $prev_node, next_msgs.as_ref().unwrap().1, false);
+                                       next_msgs = new_next_msgs;
                                }
                        }
                }
 
-               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());
+                       let update_next_msgs = !skip_last || idx != expected_route.len() - 1;
                        if next_msgs.is_some() {
-                               update_fulfill_dance!(node, prev_node, false);
-                       }
-
-                       let events = node.node.get_and_clear_pending_msg_events();
-                       if !skip_last || idx != expected_route.len() - 1 {
-                               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_eq!(update_fulfill_htlcs.len(), 1);
-                                               assert!(update_fail_htlcs.is_empty());
-                                               assert!(update_fail_malformed_htlcs.is_empty());
-                                               assert!(update_fee.is_none());
-                                               expected_next_node = node_id.clone();
-                                               next_msgs = Some((update_fulfill_htlcs[0].clone(), commitment_signed.clone()));
-                                       },
-                                       _ => panic!("Unexpected event"),
-                               }
+                               mid_update_fulfill_dance!(node, prev_node, update_next_msgs);
+                       } else if update_next_msgs {
+                               next_msgs = get_next_msgs!(node);
                        } else {
-                               assert!(events.is_empty());
+                               assert!(node.node.get_and_clear_pending_msg_events().is_empty());
                        }
                        if !skip_last && idx == expected_route.len() - 1 {
                                assert_eq!(expected_next_node, origin_node.node.get_our_node_id());
@@ -3258,7 +3455,7 @@ mod tests {
                }
 
                if !skip_last {
-                       update_fulfill_dance!(origin_node, expected_route.first().unwrap(), true);
+                       last_update_fulfill_dance!(origin_node, expected_route.first().unwrap());
                        let events = origin_node.node.get_and_clear_pending_events();
                        assert_eq!(events.len(), 1);
                        match events[0] {
@@ -3468,40 +3665,45 @@ mod tests {
 
                // ...now when the messages get delivered everyone should be happy
                nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
-               let (as_revoke_msg, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event.commitment_msg).unwrap(); // (2)
-               assert!(as_commitment_signed.is_none()); // nodes[0] is awaiting nodes[1] revoke_and_ack
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event.commitment_msg).unwrap(); // (2)
+               let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // nodes[0] is awaiting nodes[1] revoke_and_ack so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
                // deliver(1), generate (3):
-               let (bs_revoke_msg, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
-               assert!(bs_commitment_signed.is_none()); // nodes[1] is awaiting nodes[0] revoke_and_ack
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
+               let bs_revoke_and_ack = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // nodes[1] is awaiting nodes[0] revoke_and_ack so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[1], 1);
 
-               let bs_update = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap(); // deliver (2)
-               assert!(bs_update.as_ref().unwrap().update_add_htlcs.is_empty()); // (4)
-               assert!(bs_update.as_ref().unwrap().update_fulfill_htlcs.is_empty()); // (4)
-               assert!(bs_update.as_ref().unwrap().update_fail_htlcs.is_empty()); // (4)
-               assert!(bs_update.as_ref().unwrap().update_fail_malformed_htlcs.is_empty()); // (4)
-               assert!(bs_update.as_ref().unwrap().update_fee.is_none()); // (4)
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap(); // deliver (2)
+               let bs_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               assert!(bs_update.update_add_htlcs.is_empty()); // (4)
+               assert!(bs_update.update_fulfill_htlcs.is_empty()); // (4)
+               assert!(bs_update.update_fail_htlcs.is_empty()); // (4)
+               assert!(bs_update.update_fail_malformed_htlcs.is_empty()); // (4)
+               assert!(bs_update.update_fee.is_none()); // (4)
                check_added_monitors!(nodes[1], 1);
 
-               let as_update = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_msg).unwrap(); // deliver (3)
-               assert!(as_update.as_ref().unwrap().update_add_htlcs.is_empty()); // (5)
-               assert!(as_update.as_ref().unwrap().update_fulfill_htlcs.is_empty()); // (5)
-               assert!(as_update.as_ref().unwrap().update_fail_htlcs.is_empty()); // (5)
-               assert!(as_update.as_ref().unwrap().update_fail_malformed_htlcs.is_empty()); // (5)
-               assert!(as_update.as_ref().unwrap().update_fee.is_none()); // (5)
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap(); // deliver (3)
+               let as_update = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+               assert!(as_update.update_add_htlcs.is_empty()); // (5)
+               assert!(as_update.update_fulfill_htlcs.is_empty()); // (5)
+               assert!(as_update.update_fail_htlcs.is_empty()); // (5)
+               assert!(as_update.update_fail_malformed_htlcs.is_empty()); // (5)
+               assert!(as_update.update_fee.is_none()); // (5)
                check_added_monitors!(nodes[0], 1);
 
-               let (as_second_revoke, as_second_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_update.unwrap().commitment_signed).unwrap(); // deliver (4)
-               assert!(as_second_commitment_signed.is_none()); // only (6)
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_update.commitment_signed).unwrap(); // deliver (4)
+               let as_second_revoke = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // only (6) so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
-               let (bs_second_revoke, bs_second_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_update.unwrap().commitment_signed).unwrap(); // deliver (5)
-               assert!(bs_second_commitment_signed.is_none());
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_update.commitment_signed).unwrap(); // deliver (5)
+               let bs_second_revoke = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
                check_added_monitors!(nodes[1], 1);
 
-               assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke).unwrap().is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke).unwrap();
                check_added_monitors!(nodes[0], 1);
 
                let events_2 = nodes[0].node.get_and_clear_pending_events();
@@ -3511,7 +3713,7 @@ mod tests {
                        _ => panic!("Unexpected event"),
                }
 
-               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_revoke).unwrap().is_none()); // deliver (6)
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_revoke).unwrap(); // deliver (6)
                check_added_monitors!(nodes[1], 1);
        }
 
@@ -3564,11 +3766,12 @@ mod tests {
 
                // ...now when the messages get delivered everyone should be happy
                nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
-               let (as_revoke_msg, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event.commitment_msg).unwrap(); // (2)
-               assert!(as_commitment_signed.is_none()); // nodes[0] is awaiting nodes[1] revoke_and_ack
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event.commitment_msg).unwrap(); // (2)
+               let as_revoke_msg = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // nodes[0] is awaiting nodes[1] revoke_and_ack so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
-               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap().is_none()); // deliver (2)
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap(); // deliver (2)
                check_added_monitors!(nodes[1], 1);
 
                // We can't continue, sadly, because our (1) now has a bogus signature
@@ -3623,7 +3826,8 @@ mod tests {
 
                // Deliver first update_fee/commitment_signed pair, generating (1) and (2):
                nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg_1).unwrap();
-               let (bs_revoke_msg, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed_1).unwrap();
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed_1).unwrap();
+               let (bs_revoke_msg, bs_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                check_added_monitors!(nodes[1], 1);
 
                // nodes[0] is awaiting a revoke from nodes[1] before it will create a new commitment
@@ -3645,36 +3849,43 @@ mod tests {
                nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update_msg_2).unwrap();
 
                // Deliver (1), generating (3) and (4)
-               let as_second_update = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_msg).unwrap();
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_msg).unwrap();
+               let as_second_update = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
                check_added_monitors!(nodes[0], 1);
-               assert!(as_second_update.as_ref().unwrap().update_add_htlcs.is_empty());
-               assert!(as_second_update.as_ref().unwrap().update_fulfill_htlcs.is_empty());
-               assert!(as_second_update.as_ref().unwrap().update_fail_htlcs.is_empty());
-               assert!(as_second_update.as_ref().unwrap().update_fail_malformed_htlcs.is_empty());
+               assert!(as_second_update.update_add_htlcs.is_empty());
+               assert!(as_second_update.update_fulfill_htlcs.is_empty());
+               assert!(as_second_update.update_fail_htlcs.is_empty());
+               assert!(as_second_update.update_fail_malformed_htlcs.is_empty());
                // Check that the update_fee newly generated matches what we delivered:
-               assert_eq!(as_second_update.as_ref().unwrap().update_fee.as_ref().unwrap().channel_id, update_msg_2.channel_id);
-               assert_eq!(as_second_update.as_ref().unwrap().update_fee.as_ref().unwrap().feerate_per_kw, update_msg_2.feerate_per_kw);
+               assert_eq!(as_second_update.update_fee.as_ref().unwrap().channel_id, update_msg_2.channel_id);
+               assert_eq!(as_second_update.update_fee.as_ref().unwrap().feerate_per_kw, update_msg_2.feerate_per_kw);
 
                // Deliver (2) commitment_signed
-               let (as_revoke_msg, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), bs_commitment_signed.as_ref().unwrap()).unwrap();
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed).unwrap();
+               let as_revoke_msg = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
                check_added_monitors!(nodes[0], 1);
-               assert!(as_commitment_signed.is_none());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
 
-               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap().is_none());
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap();
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[1], 1);
 
                // Delever (4)
-               let (bs_second_revoke, bs_second_commitment) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_update.unwrap().commitment_signed).unwrap();
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_update.commitment_signed).unwrap();
+               let (bs_second_revoke, bs_second_commitment) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                check_added_monitors!(nodes[1], 1);
 
-               assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke).unwrap().is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
-               let (as_second_revoke, as_second_commitment) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment.unwrap()).unwrap();
-               assert!(as_second_commitment.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment).unwrap();
+               let as_second_revoke = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
-               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_revoke).unwrap().is_none());
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_revoke).unwrap();
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[1], 1);
        }
 
@@ -3694,6 +3905,7 @@ mod tests {
 
                let feerate = get_feerate!(nodes[0]);
                nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
+               check_added_monitors!(nodes[0], 1);
 
                let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events_0.len(), 1);
@@ -3705,21 +3917,21 @@ mod tests {
                };
                nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
 
-               let (revoke_msg, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
-               let commitment_signed = commitment_signed.unwrap();
-               check_added_monitors!(nodes[0], 1);
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
+               let (revoke_msg, commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                check_added_monitors!(nodes[1], 1);
 
-               let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
-               assert!(resp_option.is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
-               let (revoke_msg, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
-               assert!(commitment_signed.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
+               let revoke_msg = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
-               let resp_option = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg).unwrap();
-               assert!(resp_option.is_none());
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg).unwrap();
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[1], 1);
        }
 
@@ -3742,6 +3954,7 @@ mod tests {
 
                let feerate = get_feerate!(nodes[0]);
                nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
+               check_added_monitors!(nodes[0], 1);
 
                let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events_0.len(), 1);
@@ -3752,9 +3965,8 @@ mod tests {
                        _ => panic!("Unexpected event"),
                };
                nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
-               check_added_monitors!(nodes[0], 1);
-               let (revoke_msg, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
-               let commitment_signed = commitment_signed.unwrap();
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
+               let (revoke_msg, commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                check_added_monitors!(nodes[1], 1);
 
                let route = nodes[1].router.get_route(&nodes[0].node.get_our_node_id(), None, &Vec::new(), 800000, TEST_FINAL_CLTV).unwrap();
@@ -3772,17 +3984,19 @@ mod tests {
                assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                // node[1] has nothing to do
 
-               let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
-               assert!(resp_option.is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
-               let (revoke_msg, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
-               assert!(commitment_signed.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
+               let revoke_msg = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
-               let resp_option = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg).unwrap();
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg).unwrap();
+               check_added_monitors!(nodes[1], 1);
                // AwaitingRemoteRevoke ends here
 
-               let commitment_update = resp_option.unwrap();
+               let commitment_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                assert_eq!(commitment_update.update_add_htlcs.len(), 1);
                assert_eq!(commitment_update.update_fulfill_htlcs.len(), 0);
                assert_eq!(commitment_update.update_fail_htlcs.len(), 0);
@@ -3790,20 +4004,22 @@ mod tests {
                assert_eq!(commitment_update.update_fee.is_none(), true);
 
                nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &commitment_update.update_add_htlcs[0]).unwrap();
-               let (revoke, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
                check_added_monitors!(nodes[0], 1);
+               let (revoke, commitment_signed) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke).unwrap();
                check_added_monitors!(nodes[1], 1);
-               let commitment_signed = commitment_signed.unwrap();
-               let resp_option = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke).unwrap();
-               check_added_monitors!(nodes[1], 1);
-               assert!(resp_option.is_none());
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
 
-               let (revoke, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commitment_signed).unwrap();
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commitment_signed).unwrap();
                check_added_monitors!(nodes[1], 1);
-               assert!(commitment_signed.is_none());
-               let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke).unwrap();
+               let revoke = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
+
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke).unwrap();
                check_added_monitors!(nodes[0], 1);
-               assert!(resp_option.is_none());
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
                let events = nodes[0].node.get_and_clear_pending_events();
                assert_eq!(events.len(), 1);
@@ -3859,6 +4075,7 @@ mod tests {
                // Create and deliver (1)...
                let feerate = get_feerate!(nodes[0]);
                nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
+               check_added_monitors!(nodes[0], 1);
 
                let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events_0.len(), 1);
@@ -3871,18 +4088,18 @@ mod tests {
                nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
 
                // Generate (2) and (3):
-               let (revoke_msg, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
-               let commitment_signed_0 = commitment_signed.unwrap();
-               check_added_monitors!(nodes[0], 1);
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
+               let (revoke_msg, commitment_signed_0) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                check_added_monitors!(nodes[1], 1);
 
                // Deliver (2):
-               let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
-               assert!(resp_option.is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
                // Create and deliver (4)...
                nodes[0].node.update_fee(channel_id, feerate+30).unwrap();
+               check_added_monitors!(nodes[0], 1);
                let events_0 = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events_0.len(), 1);
                let (update_msg, commitment_signed) = match events_0[0] {
@@ -3891,36 +4108,44 @@ mod tests {
                        },
                        _ => panic!("Unexpected event"),
                };
-               nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
 
-               let (revoke_msg, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
-               // ... creating (5)
-               assert!(commitment_signed.is_none());
-               check_added_monitors!(nodes[0], 1);
+               nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
                check_added_monitors!(nodes[1], 1);
+               // ... creating (5)
+               let revoke_msg = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
 
                // Handle (3), creating (6):
-               let (revoke_msg_0, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed_0).unwrap();
-               assert!(commitment_signed.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed_0).unwrap();
                check_added_monitors!(nodes[0], 1);
+               let revoke_msg_0 = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
 
                // Deliver (5):
-               let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
-               assert!(resp_option.is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
                // Deliver (6), creating (7):
-               let resp_option = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg_0).unwrap();
-               let commitment_signed = resp_option.unwrap().commitment_signed;
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg_0).unwrap();
+               let commitment_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+               assert!(commitment_update.update_add_htlcs.is_empty());
+               assert!(commitment_update.update_fulfill_htlcs.is_empty());
+               assert!(commitment_update.update_fail_htlcs.is_empty());
+               assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
+               assert!(commitment_update.update_fee.is_none());
                check_added_monitors!(nodes[1], 1);
 
                // Deliver (7)
-               let (revoke_msg, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
-               assert!(commitment_signed.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
                check_added_monitors!(nodes[0], 1);
-               let resp_option = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg).unwrap();
-               assert!(resp_option.is_none());
+               let revoke_msg = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
+
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg).unwrap();
                check_added_monitors!(nodes[1], 1);
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
 
                assert_eq!(get_feerate!(nodes[0]), feerate + 30);
                assert_eq!(get_feerate!(nodes[1]), feerate + 30);
@@ -4391,15 +4616,21 @@ mod tests {
                assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
                // 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();
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event_1.commitment_msg).unwrap();
+               let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                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();
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_revoke_and_ack).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());
+               let commitment_update_2 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &as_commitment_signed).unwrap();
+               let bs_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                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());
+
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[1], 1);
 
                expect_pending_htlcs_forwardable!(nodes[1]);
@@ -4875,6 +5106,7 @@ mod tests {
                nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
                nodes[2].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
                check_added_monitors!(nodes[2], 1);
+               let (_, _) = get_revoke_commit_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 
                // nodes[2] now has the latest commitment transaction, but hasn't revoked its previous
                // state or updated nodes[1]' state. Now force-close and broadcast that commitment/HTLC
@@ -4965,6 +5197,61 @@ mod tests {
                assert_eq!(channel_state.short_to_id.len(), 0);
        }
 
+       macro_rules! handle_chan_reestablish_msgs {
+               ($src_node: expr, $dst_node: expr) => {
+                       {
+                               let msg_events = $src_node.node.get_and_clear_pending_msg_events();
+                               let mut idx = 0;
+                               let funding_locked = if let Some(&MessageSendEvent::SendFundingLocked { ref node_id, ref msg }) = msg_events.get(0) {
+                                       idx += 1;
+                                       assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                                       Some(msg.clone())
+                               } else {
+                                       None
+                               };
+
+                               let mut revoke_and_ack = None;
+                               let mut commitment_update = None;
+                               let order = if let Some(ev) = msg_events.get(idx) {
+                                       idx += 1;
+                                       match ev {
+                                               &MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
+                                                       assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                                                       revoke_and_ack = Some(msg.clone());
+                                                       RAACommitmentOrder::RevokeAndACKFirst
+                                               },
+                                               &MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
+                                                       assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                                                       commitment_update = Some(updates.clone());
+                                                       RAACommitmentOrder::CommitmentFirst
+                                               },
+                                               _ => panic!("Unexpected event"),
+                                       }
+                               } else {
+                                       RAACommitmentOrder::CommitmentFirst
+                               };
+
+                               if let Some(ev) = msg_events.get(idx) {
+                                       match ev {
+                                               &MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => {
+                                                       assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                                                       assert!(revoke_and_ack.is_none());
+                                                       revoke_and_ack = Some(msg.clone());
+                                               },
+                                               &MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => {
+                                                       assert_eq!(*node_id, $dst_node.node.get_our_node_id());
+                                                       assert!(commitment_update.is_none());
+                                                       commitment_update = Some(updates.clone());
+                                               },
+                                               _ => panic!("Unexpected event"),
+                                       }
+                               }
+
+                               (funding_locked, revoke_and_ack, commitment_update, order)
+                       }
+               }
+       }
+
        /// pending_htlc_adds includes both the holding cell and in-flight update_add_htlcs, whereas
        /// for claims/fails they are separated out.
        fn reconnect_nodes(node_a: &Node, node_b: &Node, pre_all_htlcs: bool, pending_htlc_adds: (i64, i64), pending_htlc_claims: (usize, usize), pending_cell_htlc_claims: (usize, usize), pending_cell_htlc_fails: (usize, usize), pending_raa: (bool, bool)) {
@@ -4973,7 +5260,8 @@ mod tests {
 
                let mut resp_1 = Vec::new();
                for msg in reestablish_1 {
-                       resp_1.push(node_b.node.handle_channel_reestablish(&node_a.node.get_our_node_id(), &msg).unwrap());
+                       node_b.node.handle_channel_reestablish(&node_a.node.get_our_node_id(), &msg).unwrap();
+                       resp_1.push(handle_chan_reestablish_msgs!(node_b, node_a));
                }
                if pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 {
                        check_added_monitors!(node_b, 1);
@@ -4983,7 +5271,8 @@ mod tests {
 
                let mut resp_2 = Vec::new();
                for msg in reestablish_2 {
-                       resp_2.push(node_a.node.handle_channel_reestablish(&node_b.node.get_our_node_id(), &msg).unwrap());
+                       node_a.node.handle_channel_reestablish(&node_b.node.get_our_node_id(), &msg).unwrap();
+                       resp_2.push(handle_chan_reestablish_msgs!(node_a, node_b));
                }
                if pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 {
                        check_added_monitors!(node_a, 1);
@@ -4997,15 +5286,21 @@ mod tests {
 
                for chan_msgs in resp_1.drain(..) {
                        if pre_all_htlcs {
-                               let a = node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), &chan_msgs.0.unwrap());
-                               let _announcement_sigs_opt = a.unwrap();
-                               //TODO: Test announcement_sigs re-sending when we've implemented it
+                               node_a.node.handle_funding_locked(&node_b.node.get_our_node_id(), &chan_msgs.0.unwrap()).unwrap();
+                               let announcement_event = node_a.node.get_and_clear_pending_msg_events();
+                               if !announcement_event.is_empty() {
+                                       assert_eq!(announcement_event.len(), 1);
+                                       if let MessageSendEvent::SendAnnouncementSignatures { .. } = announcement_event[0] {
+                                               //TODO: Test announcement_sigs re-sending
+                                       } else { panic!("Unexpected event!"); }
+                               }
                        } else {
                                assert!(chan_msgs.0.is_none());
                        }
                        if pending_raa.0 {
-                               assert!(chan_msgs.3 == msgs::RAACommitmentOrder::RevokeAndACKFirst);
-                               assert!(node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &chan_msgs.1.unwrap()).unwrap().is_none());
+                               assert!(chan_msgs.3 == RAACommitmentOrder::RevokeAndACKFirst);
+                               node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &chan_msgs.1.unwrap()).unwrap();
+                               assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
                                check_added_monitors!(node_a, 1);
                        } else {
                                assert!(chan_msgs.1.is_none());
@@ -5033,10 +5328,12 @@ mod tests {
                                if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
                                        commitment_signed_dance!(node_a, node_b, commitment_update.commitment_signed, false);
                                } else {
-                                       let (as_revoke_and_ack, as_commitment_signed) = node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
+                                       node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
                                        check_added_monitors!(node_a, 1);
-                                       assert!(as_commitment_signed.is_none());
-                                       assert!(node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
+                                       let as_revoke_and_ack = get_event_msg!(node_a, MessageSendEvent::SendRevokeAndACK, node_b.node.get_our_node_id());
+                                       // 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).unwrap();
+                                       assert!(node_b.node.get_and_clear_pending_msg_events().is_empty());
                                        check_added_monitors!(node_b, 1);
                                }
                        } else {
@@ -5046,14 +5343,21 @@ mod tests {
 
                for chan_msgs in resp_2.drain(..) {
                        if pre_all_htlcs {
-                               let _announcement_sigs_opt = node_b.node.handle_funding_locked(&node_a.node.get_our_node_id(), &chan_msgs.0.unwrap()).unwrap();
-                               //TODO: Test announcement_sigs re-sending when we've implemented it
+                               node_b.node.handle_funding_locked(&node_a.node.get_our_node_id(), &chan_msgs.0.unwrap()).unwrap();
+                               let announcement_event = node_b.node.get_and_clear_pending_msg_events();
+                               if !announcement_event.is_empty() {
+                                       assert_eq!(announcement_event.len(), 1);
+                                       if let MessageSendEvent::SendAnnouncementSignatures { .. } = announcement_event[0] {
+                                               //TODO: Test announcement_sigs re-sending
+                                       } else { panic!("Unexpected event!"); }
+                               }
                        } else {
                                assert!(chan_msgs.0.is_none());
                        }
                        if pending_raa.1 {
-                               assert!(chan_msgs.3 == msgs::RAACommitmentOrder::RevokeAndACKFirst);
-                               assert!(node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &chan_msgs.1.unwrap()).unwrap().is_none());
+                               assert!(chan_msgs.3 == RAACommitmentOrder::RevokeAndACKFirst);
+                               node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &chan_msgs.1.unwrap()).unwrap();
+                               assert!(node_b.node.get_and_clear_pending_msg_events().is_empty());
                                check_added_monitors!(node_b, 1);
                        } else {
                                assert!(chan_msgs.1.is_none());
@@ -5079,10 +5383,12 @@ mod tests {
                                if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
                                        commitment_signed_dance!(node_b, node_a, commitment_update.commitment_signed, false);
                                } else {
-                                       let (bs_revoke_and_ack, bs_commitment_signed) = node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
+                                       node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &commitment_update.commitment_signed).unwrap();
                                        check_added_monitors!(node_b, 1);
-                                       assert!(bs_commitment_signed.is_none());
-                                       assert!(node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
+                                       let bs_revoke_and_ack = get_event_msg!(node_b, MessageSendEvent::SendRevokeAndACK, node_a.node.get_our_node_id());
+                                       // 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).unwrap();
+                                       assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
                                        check_added_monitors!(node_a, 1);
                                }
                        } else {
@@ -5172,20 +5478,24 @@ mod tests {
                        // Drop the payment_event messages, and let them get re-generated in reconnect_nodes!
                } else {
                        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
-                       let (bs_revoke_and_ack, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
+                       nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
                        check_added_monitors!(nodes[1], 1);
+                       let (bs_revoke_and_ack, bs_commitment_signed) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
 
                        if messages_delivered >= 3 {
-                               assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
+                               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+                               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                                check_added_monitors!(nodes[0], 1);
 
                                if messages_delivered >= 4 {
-                                       let (as_revoke_and_ack, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed.unwrap()).unwrap();
-                                       assert!(as_commitment_signed.is_none());
+                                       nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_commitment_signed).unwrap();
+                                       let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+                                       // No commitment_signed so get_event_msg's assert(len == 1) passes
                                        check_added_monitors!(nodes[0], 1);
 
                                        if messages_delivered >= 5 {
-                                               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
+                                               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap();
+                                               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                                                check_added_monitors!(nodes[1], 1);
                                        }
                                }
@@ -5267,20 +5577,24 @@ mod tests {
                        }
 
                        if messages_delivered >= 2 {
-                               let (as_revoke_and_ack, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
+                               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
                                check_added_monitors!(nodes[0], 1);
+                               let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
 
                                if messages_delivered >= 3 {
-                                       assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
+                                       nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap();
+                                       assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                                        check_added_monitors!(nodes[1], 1);
 
                                        if messages_delivered >= 4 {
-                                               let (bs_revoke_and_ack, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_signed.unwrap()).unwrap();
-                                               assert!(bs_commitment_signed.is_none());
+                                               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_signed).unwrap();
+                                               let bs_revoke_and_ack = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+                                               // No commitment_signed so get_event_msg's assert(len == 1) passes
                                                check_added_monitors!(nodes[1], 1);
 
                                                if messages_delivered >= 5 {
-                                                       assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().is_none());
+                                                       nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+                                                       assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                                                        check_added_monitors!(nodes[0], 1);
                                                }
                                        }
@@ -5353,9 +5667,8 @@ mod tests {
                let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(events_1.len(), 1);
                match events_1[0] {
-                       MessageSendEvent::SendFundingLocked { ref node_id, msg: _, ref announcement_sigs } => {
+                       MessageSendEvent::SendFundingLocked { ref node_id, msg: _ } => {
                                assert_eq!(*node_id, nodes[1].node.get_our_node_id());
-                               assert!(announcement_sigs.is_none());
                        },
                        _ => panic!("Unexpected event"),
                }
@@ -5364,9 +5677,8 @@ mod tests {
                let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
                assert_eq!(events_2.len(), 1);
                match events_2[0] {
-                       MessageSendEvent::SendFundingLocked { ref node_id, msg: _, ref announcement_sigs } => {
+                       MessageSendEvent::SendFundingLocked { ref node_id, msg: _ } => {
                                assert_eq!(*node_id, nodes[0].node.get_our_node_id());
-                               assert!(announcement_sigs.is_none());
                        },
                        _ => panic!("Unexpected event"),
                }
@@ -5431,8 +5743,9 @@ mod tests {
                                        _ => panic!("Unexpected event"),
                                }
 
-                               let (_, commitment_update) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), commitment_signed).unwrap();
-                               assert!(commitment_update.is_none());
+                               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), commitment_signed).unwrap();
+                               let _ = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+                               // No commitment_signed so get_event_msg's assert(len == 1) passes
                                check_added_monitors!(nodes[0], 1);
                        },
                        _ => panic!("Unexpected event"),
@@ -5446,8 +5759,10 @@ mod tests {
                let reestablish_2 = nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
                assert_eq!(reestablish_2.len(), 1);
 
-               let as_resp = nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
-               let bs_resp = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+               nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
+               let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+               nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+               let bs_resp = handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
                assert!(as_resp.0.is_none());
                assert!(bs_resp.0.is_none());
@@ -5455,7 +5770,7 @@ mod tests {
                assert!(bs_resp.1.is_none());
                assert!(bs_resp.2.is_none());
 
-               assert!(as_resp.3 == msgs::RAACommitmentOrder::CommitmentFirst);
+               assert!(as_resp.3 == RAACommitmentOrder::CommitmentFirst);
 
                assert_eq!(as_resp.2.as_ref().unwrap().update_add_htlcs.len(), 1);
                assert!(as_resp.2.as_ref().unwrap().update_fulfill_htlcs.is_empty());
@@ -5463,11 +5778,13 @@ mod tests {
                assert!(as_resp.2.as_ref().unwrap().update_fail_malformed_htlcs.is_empty());
                assert!(as_resp.2.as_ref().unwrap().update_fee.is_none());
                nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_resp.2.as_ref().unwrap().update_add_htlcs[0]).unwrap();
-               let (bs_revoke_and_ack, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_resp.2.as_ref().unwrap().commitment_signed).unwrap();
-               assert!(bs_commitment_signed.is_none());
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_resp.2.as_ref().unwrap().commitment_signed).unwrap();
+               let bs_revoke_and_ack = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[1], 1);
 
-               let bs_second_commitment_signed = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), as_resp.1.as_ref().unwrap()).unwrap().unwrap();
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), as_resp.1.as_ref().unwrap()).unwrap();
+               let bs_second_commitment_signed = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                assert!(bs_second_commitment_signed.update_add_htlcs.is_empty());
                assert!(bs_second_commitment_signed.update_fulfill_htlcs.is_empty());
                assert!(bs_second_commitment_signed.update_fail_htlcs.is_empty());
@@ -5475,7 +5792,8 @@ mod tests {
                assert!(bs_second_commitment_signed.update_fee.is_none());
                check_added_monitors!(nodes[1], 1);
 
-               let as_commitment_signed = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().unwrap();
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+               let as_commitment_signed = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
                assert!(as_commitment_signed.update_add_htlcs.is_empty());
                assert!(as_commitment_signed.update_fulfill_htlcs.is_empty());
                assert!(as_commitment_signed.update_fail_htlcs.is_empty());
@@ -5483,15 +5801,18 @@ mod tests {
                assert!(as_commitment_signed.update_fee.is_none());
                check_added_monitors!(nodes[0], 1);
 
-               let (as_revoke_and_ack, as_second_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment_signed.commitment_signed).unwrap();
-               assert!(as_second_commitment_signed.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment_signed.commitment_signed).unwrap();
+               let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
-               let (bs_second_revoke_and_ack, bs_third_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_signed.commitment_signed).unwrap();
-               assert!(bs_third_commitment_signed.is_none());
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_signed.commitment_signed).unwrap();
+               let bs_second_revoke_and_ack = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[1], 1);
 
-               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap();
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[1], 1);
 
                let events_4 = nodes[1].node.get_and_clear_pending_events();
@@ -5513,7 +5834,8 @@ mod tests {
                        _ => panic!("Unexpected event"),
                }
 
-               assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke_and_ack).unwrap().is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke_and_ack).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
                claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
@@ -5725,8 +6047,10 @@ mod tests {
                        let reestablish_2 = nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
                        assert_eq!(reestablish_2.len(), 1);
 
-                       let as_resp = nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
-                       let bs_resp = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+                       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
+                       let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+                       nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+                       let bs_resp = handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
                        assert!(as_resp.0.is_none());
                        assert!(bs_resp.0.is_none());
@@ -5743,10 +6067,12 @@ mod tests {
                        let reestablish_2 = nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id());
                        assert_eq!(reestablish_2.len(), 1);
 
-                       let mut as_resp = nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
+                       nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]).unwrap();
                        check_added_monitors!(nodes[0], 0);
-                       let mut bs_resp = nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
+                       let mut as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+                       nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]).unwrap();
                        check_added_monitors!(nodes[1], 0);
+                       let mut bs_resp = handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
 
                        assert!(as_resp.0.is_none());
                        assert!(bs_resp.0.is_none());
@@ -5757,7 +6083,7 @@ mod tests {
 
                                assert!(as_resp.1.is_some());
                                assert!(as_resp.2.is_some());
-                               assert!(as_resp.3 == msgs::RAACommitmentOrder::CommitmentFirst);
+                               assert!(as_resp.3 == RAACommitmentOrder::CommitmentFirst);
                        } else {
                                assert!(bs_resp.2.as_ref().unwrap().update_add_htlcs.is_empty());
                                assert!(bs_resp.2.as_ref().unwrap().update_fail_htlcs.is_empty());
@@ -5778,8 +6104,9 @@ mod tests {
                                        _ => panic!("Unexpected event"),
                                }
 
-                               let (as_resp_raa, as_resp_cu) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_resp.2.as_ref().unwrap().commitment_signed).unwrap();
-                               assert!(as_resp_cu.is_none());
+                               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_resp.2.as_ref().unwrap().commitment_signed).unwrap();
+                               let as_resp_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+                               // No commitment_signed so get_event_msg's assert(len == 1) passes
                                check_added_monitors!(nodes[0], 1);
 
                                as_resp.1 = Some(as_resp_raa);
@@ -5813,8 +6140,9 @@ mod tests {
                assert_eq!(payment_event.node_id, nodes[1].node.get_our_node_id());
 
                nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
-               let (bs_revoke_and_ack, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
-               assert!(bs_commitment_signed.is_none()); // nodes[1] is awaiting an RAA from nodes[0] still
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg).unwrap();
+               let bs_revoke_and_ack = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // nodes[1] is awaiting an RAA from nodes[0] still so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[1], 1);
 
                if disconnect_count & !disconnect_flags > 2 {
@@ -5831,7 +6159,8 @@ mod tests {
                let bs_second_commitment_update;
 
                macro_rules! handle_bs_raa { () => {
-                       as_commitment_update = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap().unwrap();
+                       nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_and_ack).unwrap();
+                       as_commitment_update = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
                        assert!(as_commitment_update.update_add_htlcs.is_empty());
                        assert!(as_commitment_update.update_fulfill_htlcs.is_empty());
                        assert!(as_commitment_update.update_fail_htlcs.is_empty());
@@ -5841,7 +6170,8 @@ mod tests {
                } }
 
                macro_rules! handle_initial_raa { () => {
-                       bs_second_commitment_update = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &initial_revoke_and_ack).unwrap().unwrap();
+                       nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &initial_revoke_and_ack).unwrap();
+                       bs_second_commitment_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
                        assert!(bs_second_commitment_update.update_add_htlcs.is_empty());
                        assert!(bs_second_commitment_update.update_fulfill_htlcs.is_empty());
                        assert!(bs_second_commitment_update.update_fail_htlcs.is_empty());
@@ -5862,7 +6192,7 @@ mod tests {
                                assert!(as_resp.2.unwrap() == as_commitment_update);
                                assert!(bs_resp.2.is_none());
 
-                               assert!(as_resp.3 == msgs::RAACommitmentOrder::RevokeAndACKFirst);
+                               assert!(as_resp.3 == RAACommitmentOrder::RevokeAndACKFirst);
                        }
 
                        handle_initial_raa!();
@@ -5888,7 +6218,7 @@ mod tests {
                                assert!(as_resp.2.is_none());
                                assert!(bs_resp.2.unwrap() == bs_second_commitment_update);
 
-                               assert!(bs_resp.3 == msgs::RAACommitmentOrder::RevokeAndACKFirst);
+                               assert!(bs_resp.3 == RAACommitmentOrder::RevokeAndACKFirst);
                        }
 
                        handle_bs_raa!();
@@ -5904,18 +6234,22 @@ mod tests {
                        }
                }
 
-               let (as_revoke_and_ack, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment_update.commitment_signed).unwrap();
-               assert!(as_commitment_signed.is_none());
+               nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment_update.commitment_signed).unwrap();
+               let as_revoke_and_ack = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[0], 1);
 
-               let (bs_second_revoke_and_ack, bs_third_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_update.commitment_signed).unwrap();
-               assert!(bs_third_commitment_signed.is_none());
+               nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_commitment_update.commitment_signed).unwrap();
+               let bs_second_revoke_and_ack = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+               // No commitment_signed so get_event_msg's assert(len == 1) passes
                check_added_monitors!(nodes[1], 1);
 
-               assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap().is_none());
+               nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_and_ack).unwrap();
+               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[1], 1);
 
-               assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke_and_ack).unwrap().is_none());
+               nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke_and_ack).unwrap();
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
                check_added_monitors!(nodes[0], 1);
 
                expect_pending_htlcs_forwardable!(nodes[1]);