Merge pull request #160 from ariard/channel_manager_refactor_all
[rust-lightning] / src / ln / channelmanager.rs
index ff1f6e53dae90fe27999d20d9298346252cbd4fe..600342f0b35126b02d0ee5d03d0d4c80d7c4ef82 100644 (file)
@@ -141,6 +141,25 @@ impl MsgHandleErrInternal {
                }
        }
        #[inline]
+       fn send_err_msg_close_chan(err: &'static str, channel_id: [u8; 32]) -> Self {
+               Self {
+                       err: HandleError {
+                               err,
+                               action: Some(msgs::ErrorAction::SendErrorMessage {
+                                       msg: msgs::ErrorMessage {
+                                               channel_id,
+                                               data: err.to_string()
+                                       },
+                               }),
+                       },
+                       needs_channel_force_close: true,
+               }
+       }
+       #[inline]
+       fn from_maybe_close(err: msgs::HandleError) -> Self {
+               Self { err, needs_channel_force_close: true }
+       }
+       #[inline]
        fn from_no_close(err: msgs::HandleError) -> Self {
                Self { err, needs_channel_force_close: false }
        }
@@ -215,10 +234,10 @@ pub struct ChannelManager {
 const CLTV_EXPIRY_DELTA: u16 = 6 * 24 * 2; //TODO?
 
 macro_rules! secp_call {
-       ( $res: expr, $err_msg: expr, $action: expr ) => {
+       ( $res: expr, $err: expr ) => {
                match $res {
                        Ok(key) => key,
-                       Err(_) => return Err(HandleError{err: $err_msg, action: Some($action)})
+                       Err(_) => return Err($err),
                }
        };
 }
@@ -428,6 +447,8 @@ impl ChannelManager {
                //TODO: We need to handle monitoring of pending offered HTLCs which just hit the chain and
                //may be claimed, resulting in us claiming the inbound HTLCs (and back-failing after
                //timeouts are hit and our claims confirm).
+               //TODO: In any case, we need to make sure we remove any pending htlc tracking (via
+               //fail_backwards or claim_funds) eventually for all HTLCs that were in the channel
        }
 
        /// Force closes a channel, immediately broadcasting the latest local commitment transaction to
@@ -940,7 +961,8 @@ impl ChannelManager {
 
                //TODO: This should return something other than HandleError, that's really intended for
                //p2p-returns only.
-               let onion_keys = secp_call!(ChannelManager::construct_onion_keys(&self.secp_ctx, &route, &session_priv), "Pubkey along hop was maliciously selected", msgs::ErrorAction::IgnoreError);
+               let onion_keys = secp_call!(ChannelManager::construct_onion_keys(&self.secp_ctx, &route, &session_priv),
+                               HandleError{err: "Pubkey along hop was maliciously selected", action: Some(msgs::ErrorAction::IgnoreError)});
                let (onion_payloads, htlc_msat, htlc_cltv) = ChannelManager::build_onion_payloads(&route, cur_height)?;
                let onion_packet = ChannelManager::construct_onion_packet(onion_payloads, onion_keys, &payment_hash)?;
 
@@ -1127,7 +1149,12 @@ impl ChannelManager {
                                        if !add_htlc_msgs.is_empty() {
                                                let (commitment_msg, monitor) = match forward_chan.send_commitment() {
                                                        Ok(res) => res,
-                                                       Err(_e) => {
+                                                       Err(e) => {
+                                                               if let &Some(msgs::ErrorAction::DisconnectPeer{msg: Some(ref _err_msg)}) = &e.action {
+                                                               } else if let &Some(msgs::ErrorAction::SendErrorMessage{msg: ref _err_msg}) = &e.action {
+                                                               } else {
+                                                                       panic!("Stated return value requirements in send_commitment() were not met");
+                                                               }
                                                                //TODO: Handle...this is bad!
                                                                continue;
                                                        },
@@ -1418,6 +1445,191 @@ impl ChannelManager {
                channel_state.by_id.insert(channel.channel_id(), channel);
                Ok(accept_msg)
        }
+
+       fn internal_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), MsgHandleErrInternal> {
+               let (value, output_script, user_id) = {
+                       let mut channel_state = self.channel_state.lock().unwrap();
+                       match channel_state.by_id.get_mut(&msg.temporary_channel_id) {
+                               Some(chan) => {
+                                       if chan.get_their_node_id() != *their_node_id {
+                                               //TODO: see issue #153, need a consistent behavior on obnoxious behavior from random node
+                                               return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.temporary_channel_id));
+                                       }
+                                       chan.accept_channel(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       (chan.get_value_satoshis(), chan.get_funding_redeemscript().to_v0_p2wsh(), chan.get_user_id())
+                               },
+                               //TODO: same as above
+                               None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.temporary_channel_id))
+                       }
+               };
+               let mut pending_events = self.pending_events.lock().unwrap();
+               pending_events.push(events::Event::FundingGenerationReady {
+                       temporary_channel_id: msg.temporary_channel_id,
+                       channel_value_satoshis: value,
+                       output_script: output_script,
+                       user_channel_id: user_id,
+               });
+               Ok(())
+       }
+
+       fn internal_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, 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()) {
+                               hash_map::Entry::Occupied(mut chan) => {
+                                       if chan.get().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.temporary_channel_id));
+                                       }
+                                       match chan.get_mut().funding_created(msg) {
+                                               Ok((funding_msg, monitor_update)) => {
+                                                       (chan.remove(), funding_msg, monitor_update)
+                                               },
+                                               Err(e) => {
+                                                       return Err(e).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))
+                                               }
+                                       }
+                               },
+                               hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.temporary_channel_id))
+                       }
+               }; // Release channel lock for install_watch_outpoint call,
+                  // note that this means if the remote end is misbehaving and sends a message for the same
+                  // channel back-to-back with funding_created, we'll end up thinking they sent a message
+                  // for a bogus channel.
+               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();
+               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) => {
+                               e.insert(chan);
+                       }
+               }
+               Ok(funding_msg)
+       }
+
+       fn internal_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> {
+               let (funding_txo, user_id, monitor) = {
+                       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 chan_monitor = chan.funding_signed(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       (chan.get_funding_txo().unwrap(), chan.get_user_id(), chan_monitor)
+                               },
+                               None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
+                       }
+               };
+               if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
+                       unimplemented!();
+               }
+               let mut pending_events = self.pending_events.lock().unwrap();
+               pending_events.push(events::Event::FundingBroadcastSafe {
+                       funding_txo: funding_txo,
+                       user_channel_id: user_id,
+               });
+               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();
+               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));
+                               }
+                               chan.funding_locked(&msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                               return Ok(self.get_announcement_sigs(chan));
+                       },
+                       None => return 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 (res, 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()) {
+                               hash_map::Entry::Occupied(mut chan_entry) => {
+                                       if chan_entry.get().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 = chan_entry.get_mut().shutdown(&*self.fee_estimator, &msg).map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+                                       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) }
+                               },
+                               hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
+                       }
+               };
+               for payment_hash in res.2 {
+                       // unknown_next_peer...I dunno who that is anymore....
+                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
+               }
+               if let Some(chan) = chan_option {
+                       if let Ok(update) = self.get_channel_update(&chan) {
+                               let mut events = self.pending_events.lock().unwrap();
+                               events.push(events::Event::BroadcastChannelUpdate {
+                                       msg: update
+                               });
+                       }
+               }
+               Ok((res.0, res.1))
+       }
+
+       fn internal_announcement_signatures(&self, their_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) -> Result<(), MsgHandleErrInternal> {
+               let (chan_announcement, chan_update) = {
+                       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));
+                                       }
+                                       if !chan.is_usable() {
+                                               return Err(MsgHandleErrInternal::from_no_close(HandleError{err: "Got an announcement_signatures before we were ready for it", action: Some(msgs::ErrorAction::IgnoreError)}));
+                                       }
+
+                                       let our_node_id = self.get_our_node_id();
+                                       let (announcement, our_bitcoin_sig) = chan.get_channel_announcement(our_node_id.clone(), self.genesis_hash.clone())
+                                               .map_err(|e| MsgHandleErrInternal::from_maybe_close(e))?;
+
+                                       let were_node_one = announcement.node_id_1 == our_node_id;
+                                       let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
+                                       let bad_sig_action = MsgHandleErrInternal::send_err_msg_close_chan("Bad announcement_signatures node_signature", msg.channel_id);
+                                       secp_call!(self.secp_ctx.verify(&msghash, &msg.node_signature, if were_node_one { &announcement.node_id_2 } else { &announcement.node_id_1 }), bad_sig_action);
+                                       secp_call!(self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, if were_node_one { &announcement.bitcoin_key_2 } else { &announcement.bitcoin_key_1 }), bad_sig_action);
+
+                                       let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
+
+                                       (msgs::ChannelAnnouncement {
+                                               node_signature_1: if were_node_one { our_node_sig } else { msg.node_signature },
+                                               node_signature_2: if were_node_one { msg.node_signature } else { our_node_sig },
+                                               bitcoin_signature_1: if were_node_one { our_bitcoin_sig } else { msg.bitcoin_signature },
+                                               bitcoin_signature_2: if were_node_one { msg.bitcoin_signature } else { our_bitcoin_sig },
+                                               contents: announcement,
+                                       }, self.get_channel_update(chan).unwrap()) // can only fail if we're not in a ready state
+                               },
+                               None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
+                       }
+               };
+               let mut pending_events = self.pending_events.lock().unwrap();
+               pending_events.push(events::Event::BroadcastChannelAnnouncement { msg: chan_announcement, update_msg: chan_update });
+               Ok(())
+       }
+
+
 }
 
 impl events::EventsProvider for ChannelManager {
@@ -1583,144 +1795,23 @@ impl ChannelMessageHandler for ChannelManager {
        }
 
        fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
-               let (value, output_script, user_id) = {
-                       let mut channel_state = self.channel_state.lock().unwrap();
-                       match channel_state.by_id.get_mut(&msg.temporary_channel_id) {
-                               Some(chan) => {
-                                       if chan.get_their_node_id() != *their_node_id {
-                                               return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
-                                       }
-                                       chan.accept_channel(&msg)?;
-                                       (chan.get_value_satoshis(), chan.get_funding_redeemscript().to_v0_p2wsh(), chan.get_user_id())
-                               },
-                               None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
-                       }
-               };
-               let mut pending_events = self.pending_events.lock().unwrap();
-               pending_events.push(events::Event::FundingGenerationReady {
-                       temporary_channel_id: msg.temporary_channel_id,
-                       channel_value_satoshis: value,
-                       output_script: output_script,
-                       user_channel_id: user_id,
-               });
-               Ok(())
+               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> {
-               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()) {
-                               hash_map::Entry::Occupied(mut chan) => {
-                                       if chan.get().get_their_node_id() != *their_node_id {
-                                               return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
-                                       }
-                                       match chan.get_mut().funding_created(msg) {
-                                               Ok((funding_msg, monitor_update)) => {
-                                                       (chan.remove(), funding_msg, monitor_update)
-                                               },
-                                               Err(e) => {
-                                                       //TODO: Possibly remove the channel depending on e.action
-                                                       return Err(e);
-                                               }
-                                       }
-                               },
-                               hash_map::Entry::Vacant(_) => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
-                       }
-               }; // Release channel lock for install_watch_outpoint call,
-                  // note that this means if the remote end is misbehaving and sends a message for the same
-                  // channel back-to-back with funding_created, we'll end up thinking they sent a message
-                  // for a bogus channel.
-               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();
-               match channel_state.by_id.entry(funding_msg.channel_id) {
-                       hash_map::Entry::Occupied(_) => {
-                               return Err(HandleError {
-                                       err: "Duplicate channel_id!",
-                                       action: Some(msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id: funding_msg.channel_id, data: "Already had channel with the new channel_id".to_owned() } })
-                               });
-                       },
-                       hash_map::Entry::Vacant(e) => {
-                               e.insert(chan);
-                       }
-               }
-               Ok(funding_msg)
+               handle_error!(self, self.internal_funding_created(their_node_id, msg), their_node_id)
        }
 
        fn handle_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), HandleError> {
-               let (funding_txo, user_id, monitor) = {
-                       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(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
-                                       }
-                                       let chan_monitor = chan.funding_signed(&msg)?;
-                                       (chan.get_funding_txo().unwrap(), chan.get_user_id(), chan_monitor)
-                               },
-                               None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
-                       }
-               };
-               if let Err(_e) = self.monitor.add_update_monitor(monitor.get_funding_txo().unwrap(), monitor) {
-                       unimplemented!();
-               }
-               let mut pending_events = self.pending_events.lock().unwrap();
-               pending_events.push(events::Event::FundingBroadcastSafe {
-                       funding_txo: funding_txo,
-                       user_channel_id: user_id,
-               });
-               Ok(())
+               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> {
-               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(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
-                               }
-                               chan.funding_locked(&msg)?;
-                               return Ok(self.get_announcement_sigs(chan));
-                       },
-                       None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
-               };
+               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> {
-               let (res, 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()) {
-                               hash_map::Entry::Occupied(mut chan_entry) => {
-                                       if chan_entry.get().get_their_node_id() != *their_node_id {
-                                               return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
-                                       }
-                                       let res = chan_entry.get_mut().shutdown(&*self.fee_estimator, &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) }
-                               },
-                               hash_map::Entry::Vacant(_) => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
-                       }
-               };
-               for payment_hash in res.2 {
-                       // unknown_next_peer...I dunno who that is anymore....
-                       self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), &payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() });
-               }
-               if let Some(chan) = chan_option {
-                       if let Ok(update) = self.get_channel_update(&chan) {
-                               let mut events = self.pending_events.lock().unwrap();
-                               events.push(events::Event::BroadcastChannelUpdate {
-                                       msg: update
-                               });
-                       }
-               }
-               Ok((res.0, res.1))
+               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> {
@@ -2040,42 +2131,7 @@ impl ChannelMessageHandler for ChannelManager {
        }
 
        fn handle_announcement_signatures(&self, their_node_id: &PublicKey, msg: &msgs::AnnouncementSignatures) -> Result<(), HandleError> {
-               let (chan_announcement, chan_update) = {
-                       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(HandleError{err: "Got a message for a channel from the wrong node!", action: Some(msgs::ErrorAction::IgnoreError) })
-                                       }
-                                       if !chan.is_usable() {
-                                               return Err(HandleError{err: "Got an announcement_signatures before we were ready for it", action: Some(msgs::ErrorAction::IgnoreError) });
-                                       }
-
-                                       let our_node_id = self.get_our_node_id();
-                                       let (announcement, our_bitcoin_sig) = chan.get_channel_announcement(our_node_id.clone(), self.genesis_hash.clone())?;
-
-                                       let were_node_one = announcement.node_id_1 == our_node_id;
-                                       let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
-                                       let bad_sig_action = msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id: msg.channel_id.clone(), data: "Invalid signature in announcement_signatures".to_string() } };
-                                       secp_call!(self.secp_ctx.verify(&msghash, &msg.node_signature, if were_node_one { &announcement.node_id_2 } else { &announcement.node_id_1 }), "Bad announcement_signatures node_signature", bad_sig_action);
-                                       secp_call!(self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, if were_node_one { &announcement.bitcoin_key_2 } else { &announcement.bitcoin_key_1 }), "Bad announcement_signatures bitcoin_signature", bad_sig_action);
-
-                                       let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
-
-                                       (msgs::ChannelAnnouncement {
-                                               node_signature_1: if were_node_one { our_node_sig } else { msg.node_signature },
-                                               node_signature_2: if were_node_one { msg.node_signature } else { our_node_sig },
-                                               bitcoin_signature_1: if were_node_one { our_bitcoin_sig } else { msg.bitcoin_signature },
-                                               bitcoin_signature_2: if were_node_one { msg.bitcoin_signature } else { our_bitcoin_sig },
-                                               contents: announcement,
-                                       }, self.get_channel_update(chan).unwrap()) // can only fail if we're not in a ready state
-                               },
-                               None => return Err(HandleError{err: "Failed to find corresponding channel", action: Some(msgs::ErrorAction::IgnoreError)})
-                       }
-               };
-               let mut pending_events = self.pending_events.lock().unwrap();
-               pending_events.push(events::Event::BroadcastChannelAnnouncement { msg: chan_announcement, update_msg: chan_update });
-               Ok(())
+               handle_error!(self, self.internal_announcement_signatures(their_node_id, msg), their_node_id)
        }
 
        fn peer_disconnected(&self, their_node_id: &PublicKey, no_connection_possible: bool) {
@@ -3328,9 +3384,9 @@ mod tests {
                                        chain_hash: genesis_block(Network::Testnet).header.bitcoin_hash(),
                                        short_channel_id: as_chan.get_short_channel_id().unwrap(),
                                        node_id_1: if were_node_one { as_network_key } else { bs_network_key },
-                                       node_id_2: if !were_node_one { bs_network_key } else { as_network_key },
+                                       node_id_2: if were_node_one { bs_network_key } else { as_network_key },
                                        bitcoin_key_1: if were_node_one { as_bitcoin_key } else { bs_bitcoin_key },
-                                       bitcoin_key_2: if !were_node_one { bs_bitcoin_key } else { as_bitcoin_key },
+                                       bitcoin_key_2: if were_node_one { bs_bitcoin_key } else { as_bitcoin_key },
                                        excess_data: Vec::new(),
                                };
                        }
@@ -3345,9 +3401,9 @@ mod tests {
                                let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].node.our_network_key);
                                chan_announcement = msgs::ChannelAnnouncement {
                                        node_signature_1 : if were_node_one { as_node_sig } else { bs_node_sig},
-                                       node_signature_2 : if !were_node_one { bs_node_sig } else { as_node_sig},
+                                       node_signature_2 : if were_node_one { bs_node_sig } else { as_node_sig},
                                        bitcoin_signature_1: if were_node_one { as_bitcoin_sig } else { bs_bitcoin_sig },
-                                       bitcoin_signature_2 : if !were_node_one { bs_bitcoin_sig } else { as_bitcoin_sig },
+                                       bitcoin_signature_2 : if were_node_one { bs_bitcoin_sig } else { as_bitcoin_sig },
                                        contents: $unsigned_msg
                                }
                        }