Clean up/clarify channel announcement_signatures handling

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index 4fb4f47f0868a38afe9d0cafb2e19fe89f3b49c3..b59379aa74fdbab50589ad9ba424a17fab916ab4 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -2275,18 +2275,22 @@ impl Channel {
 
        /// Gets an UnsignedChannelAnnouncement, as well as a signature covering it using our
        /// bitcoin_key, if available, for this channel. The channel must be publicly announceable and
-       /// available for use (have exchanged FundingLocked messages in both directions. Should be used
+       /// available for use (have exchanged FundingLocked messages in both directions). Should be used
        /// for both loose and in response to an AnnouncementSignatures message from the remote peer.
-       /// Note that you can get an announcement for a channel which is closing, though you should
-       /// likely not announce such a thing. In case its already been announced, a channel_update
-       /// message can mark the channel disabled.
+       /// Will only fail if we're not in a state where channel_announcement may be sent (including
+       /// closing).
+       /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
+       /// 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});
                }
-               if self.channel_state & (ChannelState::ChannelFunded as u32) != (ChannelState::ChannelFunded as u32) {
+               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});
                }
+               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});
+               }
 
                let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
                let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.funding_key);

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 6275713b589043142626fd2cbd582b0a3e456cee..073ba792ac156b59d838dade0c9291948fbf7a8d 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -1030,19 +1030,22 @@ impl ChannelManager {
                }
        }
 
-       fn get_announcement_sigs(&self, chan: &Channel) -> Result<Option<msgs::AnnouncementSignatures>, HandleError> {
-               if !chan.is_usable() || !chan.should_announce() { return Ok(None) }
+       fn get_announcement_sigs(&self, chan: &Channel) -> Option<msgs::AnnouncementSignatures> {
+               if !chan.should_announce() { return None }
 
-               let (announcement, our_bitcoin_sig) = chan.get_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone())?;
+               let (announcement, our_bitcoin_sig) = match chan.get_channel_announcement(self.get_our_node_id(), self.genesis_hash.clone()) {
+                       Ok(res) => res,
+                       Err(_) => return None, // Only in case of state precondition violations eg channel is closing
+               };
                let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
                let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
 
-               Ok(Some(msgs::AnnouncementSignatures {
+               Some(msgs::AnnouncementSignatures {
                        channel_id: chan.channel_id(),
                        short_channel_id: chan.get_short_channel_id().unwrap(),
                        node_signature: our_node_sig,
                        bitcoin_signature: our_bitcoin_sig,
-               }))
+               })
        }
 
        /// Processes HTLCs which are pending waiting on random forward delay.
@@ -1379,14 +1382,7 @@ 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 = match self.get_announcement_sigs(channel) {
-                                               Ok(res) => res,
-                                               Err(e) => {
-                                                       log_error!(self, "Got error handling message: {}!", e.err);
-                                                       //TODO: push e on events and blow up the channel (it has bad keys)
-                                                       return true;
-                                               }
-                                       };
+                                       let announcement_sigs = self.get_announcement_sigs(channel);
                                        new_events.push(events::Event::SendFundingLocked {
                                                node_id: channel.get_their_node_id(),
                                                msg: funding_locked,
@@ -1626,7 +1622,7 @@ impl ChannelMessageHandler for ChannelManager {
                                        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)?);
+                               return Ok(self.get_announcement_sigs(chan));
                        },
                        None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
                };