Merge pull request #147 from TheBlueMatt/2018-09-channelmanager-err-macro

Refactor ChannelManager handle functions into a Channel-closing macro

diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs
index 37a67183696b13f3b2dd229af6cd2978fcdbd504..7b3b7df1ce2ec3ddcc039212e92352bddef8b246 100644 (file)
--- a/fuzz/fuzz_targets/channel_target.rs
+++ b/fuzz/fuzz_targets/channel_target.rs
@@ -230,7 +230,7 @@ pub fn do_test(data: &[u8]) {
                        Ok(chan) => chan,
                        Err(_) => return,
                };
-               chan.get_accept_channel().unwrap();
+               chan.get_accept_channel();
 
                tx.output.push(TxOut{ value: open_chan.funding_satoshis, script_pubkey: chan.get_funding_redeemscript().to_v0_p2wsh() });
                let funding_output = OutPoint::new(Sha256dHash::from_data(&serialize(&tx).unwrap()[..]), 0);

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index 0659f8261af0e52083fd7b9d5a3efceadc602c70..14ab80c90ebfb7f4241a4497e02de5885a494b47 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -525,10 +525,10 @@ impl Channel {
 
                let their_announce = if (msg.channel_flags & 1) == 1 { true } else { false };
                if require_announce && !their_announce {
-                       return Err(HandleError{err: "Peer tried to open unannounced channel, but we require public ones", action: Some(msgs::ErrorAction::IgnoreError) });
+                       return_error_message!("Peer tried to open unannounced channel, but we require public ones");
                }
                if !allow_announce && their_announce {
-                       return Err(HandleError{err: "Peer tried to open announced channel, but we require private ones", action: Some(msgs::ErrorAction::IgnoreError) });
+                       return_error_message!("Peer tried to open announced channel, but we require private ones");
                }
 
                let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
@@ -2200,7 +2200,7 @@ impl Channel {
                })
        }
 
-       pub fn get_accept_channel(&self) -> Result<msgs::AcceptChannel, HandleError> {
+       pub fn get_accept_channel(&self) -> msgs::AcceptChannel {
                if self.channel_outbound {
                        panic!("Tried to send accept_channel for an outbound channel?");
                }
@@ -2213,7 +2213,7 @@ impl Channel {
 
                let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 
-               Ok(msgs::AcceptChannel {
+               msgs::AcceptChannel {
                        temporary_channel_id: self.channel_id,
                        dust_limit_satoshis: self.our_dust_limit_satoshis,
                        max_htlc_value_in_flight_msat: Channel::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
@@ -2229,7 +2229,7 @@ impl Channel {
                        htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.htlc_base_key),
                        first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
                        shutdown_scriptpubkey: None,
-               })
+               }
        }
 
        fn get_outbound_funding_created_signature(&mut self) -> Result<(Signature, Transaction), HandleError> {
@@ -2298,13 +2298,13 @@ impl Channel {
        /// https://github.com/lightningnetwork/lightning-rfc/issues/468
        pub fn get_channel_announcement(&self, our_node_id: PublicKey, chain_hash: Sha256dHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), HandleError> {
                if !self.announce_publicly {
-                       return Err(HandleError{err: "Channel is not available for public announcements", action: None});
+                       return Err(HandleError{err: "Channel is not available for public announcements", action: Some(msgs::ErrorAction::IgnoreError)});
                }
                if self.channel_state & (ChannelState::ChannelFunded as u32) == 0 {
-                       return Err(HandleError{err: "Cannot get a ChannelAnnouncement until the channel funding has been locked", action: None});
+                       return Err(HandleError{err: "Cannot get a ChannelAnnouncement until the channel funding has been locked", action: Some(msgs::ErrorAction::IgnoreError)});
                }
                if (self.channel_state & (ChannelState::LocalShutdownSent as u32 | ChannelState::ShutdownComplete as u32)) != 0 {
-                       return Err(HandleError{err: "Cannot get a ChannelAnnouncement once the channel is closing", action: None});
+                       return Err(HandleError{err: "Cannot get a ChannelAnnouncement once the channel is closing", action: Some(msgs::ErrorAction::IgnoreError)});
                }
 
                let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 956a08c98dbb2e6b00af4e5a7f894c38d2f2be78..e365b048a205765b49c4ec8a56851ca25fa31f8b 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -120,6 +120,51 @@ enum PendingOutboundHTLC {
        }
 }
 
+struct MsgHandleErrInternal {
+       err: msgs::HandleError,
+       needs_channel_force_close: bool,
+}
+impl MsgHandleErrInternal {
+       #[inline]
+       fn send_err_msg_no_close(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: false,
+               }
+       }
+       #[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 }
+       }
+}
+
 /// We hold back HTLCs we intend to relay for a random interval in the range (this, 5*this). This
 /// provides some limited amount of privacy. Ideally this would range from somewhere like 1 second
 /// to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly. We could
@@ -189,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),
                }
        };
 }
@@ -914,7 +959,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)?;
 
@@ -1357,6 +1403,83 @@ impl ChannelManager {
        pub fn test_restore_channel_monitor(&self) {
                unimplemented!();
        }
+
+       fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<msgs::AcceptChannel, 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 chan_keys = if cfg!(feature = "fuzztarget") {
+                       ChannelKeys {
+                               funding_key:               SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0]).unwrap(),
+                               revocation_base_key:       SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0]).unwrap(),
+                               payment_base_key:          SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0]).unwrap(),
+                               delayed_payment_base_key:  SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0]).unwrap(),
+                               htlc_base_key:             SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0]).unwrap(),
+                               channel_close_key:         SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, 0]).unwrap(),
+                               channel_monitor_claim_key: SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 0]).unwrap(),
+                               commitment_seed: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
+                       }
+               } else {
+                       let mut key_seed = [0u8; 32];
+                       rng::fill_bytes(&mut key_seed);
+                       match ChannelKeys::new_from_seed(&key_seed) {
+                               Ok(key) => key,
+                               Err(_) => panic!("RNG is busted!")
+                       }
+               };
+
+               let channel = Channel::new_from_req(&*self.fee_estimator, chan_keys, their_node_id.clone(), msg, 0, false, self.announce_channels_publicly, Arc::clone(&self.logger)).map_err(|e| MsgHandleErrInternal::from_no_close(e))?;
+               let accept_msg = channel.get_accept_channel();
+               channel_state.by_id.insert(channel.channel_id(), channel);
+               Ok(accept_msg)
+       }
+
+       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 {
@@ -1483,41 +1606,42 @@ impl ChainListener for ChannelManager {
        }
 }
 
+macro_rules! handle_error {
+       ($self: ident, $internal: expr, $their_node_id: expr) => {
+               match $internal {
+                       Ok(msg) => Ok(msg),
+                       Err(MsgHandleErrInternal { err, needs_channel_force_close }) => {
+                               if needs_channel_force_close {
+                                       match &err.action {
+                                               &Some(msgs::ErrorAction::DisconnectPeer { msg: Some(ref msg) }) => {
+                                                       if msg.channel_id == [0; 32] {
+                                                               $self.peer_disconnected(&$their_node_id, true);
+                                                       } else {
+                                                               $self.force_close_channel(&msg.channel_id);
+                                                       }
+                                               },
+                                               &Some(msgs::ErrorAction::DisconnectPeer { msg: None }) => {},
+                                               &Some(msgs::ErrorAction::IgnoreError) => {},
+                                               &Some(msgs::ErrorAction::SendErrorMessage { ref msg }) => {
+                                                       if msg.channel_id == [0; 32] {
+                                                               $self.peer_disconnected(&$their_node_id, true);
+                                                       } else {
+                                                               $self.force_close_channel(&msg.channel_id);
+                                                       }
+                                               },
+                                               &None => {},
+                                       }
+                               }
+                               Err(err)
+                       },
+               }
+       }
+}
+
 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> {
-               if msg.chain_hash != self.genesis_hash {
-                       return Err(HandleError{err: "Unknown genesis block hash", action: None});
-               }
-               let mut channel_state = self.channel_state.lock().unwrap();
-               if channel_state.by_id.contains_key(&msg.temporary_channel_id) {
-                       return Err(HandleError{err: "temporary_channel_id collision!", action: None});
-               }
-
-               let chan_keys = if cfg!(feature = "fuzztarget") {
-                       ChannelKeys {
-                               funding_key:               SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0]).unwrap(),
-                               revocation_base_key:       SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0]).unwrap(),
-                               payment_base_key:          SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0]).unwrap(),
-                               delayed_payment_base_key:  SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0]).unwrap(),
-                               htlc_base_key:             SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0]).unwrap(),
-                               channel_close_key:         SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, 0]).unwrap(),
-                               channel_monitor_claim_key: SecretKey::from_slice(&self.secp_ctx, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 0]).unwrap(),
-                               commitment_seed: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
-                       }
-               } else {
-                       let mut key_seed = [0u8; 32];
-                       rng::fill_bytes(&mut key_seed);
-                       match ChannelKeys::new_from_seed(&key_seed) {
-                               Ok(key) => key,
-                               Err(_) => panic!("RNG is busted!")
-                       }
-               };
-
-               let channel = Channel::new_from_req(&*self.fee_estimator, chan_keys, their_node_id.clone(), msg, 0, false, self.announce_channels_publicly, Arc::clone(&self.logger))?;
-               let accept_msg = channel.get_accept_channel()?;
-               channel_state.by_id.insert(channel.channel_id(), channel);
-               Ok(accept_msg)
+               handle_error!(self, self.internal_open_channel(their_node_id, msg), their_node_id)
        }
 
        fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
@@ -1978,42 +2102,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) {