Send warning messages when appropriate in gossip handling pipeline

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 95f1e86b9c3ad8e67c8c3b2d0ad6e4bc6e22120b..e9af5b1868193094ed2e9c4be9a1b53696c398ff 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -613,8 +613,14 @@ impl MsgHandleErrInternal {
                Self {
                        err: match err {
                                ChannelError::Warn(msg) =>  LightningError {
-                                       err: msg,
-                                       action: msgs::ErrorAction::IgnoreError,
+                                       err: msg.clone(),
+                                       action: msgs::ErrorAction::SendWarningMessage {
+                                               msg: msgs::WarningMessage {
+                                                       channel_id,
+                                                       data: msg
+                                               },
+                                               log_level: Level::Warn,
+                                       },
                                },
                                ChannelError::Ignore(msg) => LightningError {
                                        err: msg,
@@ -1373,9 +1379,7 @@ macro_rules! convert_chan_err {
        ($self: ident, $err: expr, $short_to_id: expr, $channel: expr, $channel_id: expr) => {
                match $err {
                        ChannelError::Warn(msg) => {
-                               //TODO: Once warning messages are merged, we should send a `warning` message to our
-                               //peer here.
-                               (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), $channel_id.clone()))
+                               (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Warn(msg), $channel_id.clone()))
                        },
                        ChannelError::Ignore(msg) => {
                                (false, MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), $channel_id.clone()))

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 8ff793ed08b723280c3f089eb2ce74cb7bd2ad53..6012b54644aa7e4a0831e9065c3641f4f7d6fd7c 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -780,6 +780,22 @@ macro_rules! get_closing_signed_broadcast {
        }
 }
 
+#[cfg(test)]
+macro_rules! check_warn_msg {
+       ($node: expr, $recipient_node_id: expr, $chan_id: expr) => {{
+               let msg_events = $node.node.get_and_clear_pending_msg_events();
+               assert_eq!(msg_events.len(), 1);
+               match msg_events[0] {
+                       MessageSendEvent::HandleError { action: ErrorAction::SendWarningMessage { ref msg, log_level: _ }, node_id } => {
+                               assert_eq!(node_id, $recipient_node_id);
+                               assert_eq!(msg.channel_id, $chan_id);
+                               msg.data.clone()
+                       },
+                       _ => panic!("Unexpected event"),
+               }
+       }}
+}
+
 /// Check that a channel's closing channel update has been broadcasted, and optionally
 /// check whether an error message event has occurred.
 #[macro_export]

diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 6607d986affd062eee04fb987256d571cec88bb4..f6369c62f70837ec0d0341552b6ca5cee33ba9d2 100644 (file)
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -918,7 +918,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                                                                        msgs::DecodeError::BadLengthDescriptor => return Err(PeerHandleError { no_connection_possible: false }),
                                                                                                        msgs::DecodeError::Io(_) => return Err(PeerHandleError { no_connection_possible: false }),
                                                                                                        msgs::DecodeError::UnsupportedCompression => {
-                                                                                                               log_gossip!(self.logger, "We don't support zlib-compressed message fields, ignoring message");
+                                                                                                               log_gossip!(self.logger, "We don't support zlib-compressed message fields, sending a warning and ignoring message");
+                                                                                                               self.enqueue_message(peer, &msgs::WarningMessage { channel_id: [0; 32], data: "Unsupported message compression: zlib".to_owned() });
                                                                                                                continue;
                                                                                                        }
                                                                                                }

diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs
index 791e87377f09c8335f181c13fb9962b47ed74a45..014672598758b48f30eac3104dec2a934b3feff3 100644 (file)
--- a/lightning/src/ln/shutdown_tests.rs
+++ b/lightning/src/ln/shutdown_tests.rs
@@ -762,10 +762,8 @@ fn do_test_closing_signed_reinit_timeout(timeout_step: TimeoutStep) {
 
        if timeout_step != TimeoutStep::AfterShutdown {
                nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed);
-               // At this point nodes[1] should send back a warning message indicating it disagrees with the
-               // given channel-closing fee. Currently we do not implement warning messages so instead we
-               // remain silent here.
-               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+               assert!(check_warn_msg!(nodes[1], nodes[0].node.get_our_node_id(), chan_id)
+                       .starts_with("Unable to come to consensus about closing feerate"));
 
                // Now deliver a mutated closing_signed indicating a higher acceptable fee range, which
                // nodes[1] should happily accept and respond to.

diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs
index ae597d30364f5ca039cac07c99d5d317179a750e..6f431d940af9c2059c48a16da64ef631ff8fe84e 100644 (file)
--- a/lightning/src/routing/network_graph.rs
+++ b/lightning/src/routing/network_graph.rs
@@ -294,10 +294,21 @@ where C::Target: chain::Access, L::Target: Logger
 }
 
 macro_rules! secp_verify_sig {
-       ( $secp_ctx: expr, $msg: expr, $sig: expr, $pubkey: expr ) => {
+       ( $secp_ctx: expr, $msg: expr, $sig: expr, $pubkey: expr, $msg_type: expr ) => {
                match $secp_ctx.verify($msg, $sig, $pubkey) {
                        Ok(_) => {},
-                       Err(_) => return Err(LightningError{err: "Invalid signature from remote node".to_owned(), action: ErrorAction::IgnoreError}),
+                       Err(_) => {
+                               return Err(LightningError {
+                                       err: format!("Invalid signature on {} message", $msg_type),
+                                       action: ErrorAction::SendWarningMessage {
+                                               msg: msgs::WarningMessage {
+                                                       channel_id: [0; 32],
+                                                       data: format!("Invalid signature on {} message", $msg_type),
+                                               },
+                                               log_level: Level::Trace,
+                                       },
+                               });
+                       },
                }
        };
 }
@@ -850,7 +861,7 @@ impl NetworkGraph {
        /// routing messages from a source using a protocol other than the lightning P2P protocol.
        pub fn update_node_from_announcement<T: secp256k1::Verification>(&self, msg: &msgs::NodeAnnouncement, secp_ctx: &Secp256k1<T>) -> Result<(), LightningError> {
                let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]);
-               secp_verify_sig!(secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id);
+               secp_verify_sig!(secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id, "node_announcement");
                self.update_node_from_announcement_intern(&msg.contents, Some(&msg))
        }
 
@@ -910,10 +921,10 @@ impl NetworkGraph {
                C::Target: chain::Access,
        {
                let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]);
-               secp_verify_sig!(secp_ctx, &msg_hash, &msg.node_signature_1, &msg.contents.node_id_1);
-               secp_verify_sig!(secp_ctx, &msg_hash, &msg.node_signature_2, &msg.contents.node_id_2);
-               secp_verify_sig!(secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1);
-               secp_verify_sig!(secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2);
+               secp_verify_sig!(secp_ctx, &msg_hash, &msg.node_signature_1, &msg.contents.node_id_1, "channel_announcement");
+               secp_verify_sig!(secp_ctx, &msg_hash, &msg.node_signature_2, &msg.contents.node_id_2, "channel_announcement");
+               secp_verify_sig!(secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1, "channel_announcement");
+               secp_verify_sig!(secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2, "channel_announcement");
                self.update_channel_from_unsigned_announcement_intern(&msg.contents, Some(msg), chain_access)
        }
 
@@ -1239,7 +1250,7 @@ impl NetworkGraph {
                                                secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_two.as_slice()).map_err(|_| LightningError{
                                                        err: "Couldn't parse source node pubkey".to_owned(),
                                                        action: ErrorAction::IgnoreAndLog(Level::Debug)
-                                               })?);
+                                               })?, "channel_update");
                                        }
                                        maybe_update_channel_info!(channel.two_to_one, channel.node_two);
                                } else {
@@ -1248,7 +1259,7 @@ impl NetworkGraph {
                                                secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_one.as_slice()).map_err(|_| LightningError{
                                                        err: "Couldn't parse destination node pubkey".to_owned(),
                                                        action: ErrorAction::IgnoreAndLog(Level::Debug)
-                                               })?);
+                                               })?, "channel_update");
                                        }
                                        maybe_update_channel_info!(channel.one_to_two, channel.node_one);
                                }
@@ -1522,7 +1533,7 @@ mod tests {
                                contents: valid_announcement.contents.clone()
                }) {
                        Ok(_) => panic!(),
-                       Err(e) => assert_eq!(e.err, "Invalid signature from remote node")
+                       Err(e) => assert_eq!(e.err, "Invalid signature on node_announcement message")
                };
 
                let announcement_with_data = get_signed_node_announcement(|unsigned_announcement| {
@@ -1651,7 +1662,7 @@ mod tests {
                invalid_sig_announcement.contents.excess_data = Vec::new();
                match net_graph_msg_handler.handle_channel_announcement(&invalid_sig_announcement) {
                        Ok(_) => panic!(),
-                       Err(e) => assert_eq!(e.err, "Invalid signature from remote node")
+                       Err(e) => assert_eq!(e.err, "Invalid signature on channel_announcement message")
                };
 
                let channel_to_itself_announcement = get_signed_channel_announcement(|_| {}, node_1_privkey, node_1_privkey, &secp_ctx);
@@ -1760,7 +1771,7 @@ mod tests {
                invalid_sig_channel_update.signature = secp_ctx.sign(&fake_msghash, node_1_privkey);
                match net_graph_msg_handler.handle_channel_update(&invalid_sig_channel_update) {
                        Ok(_) => panic!(),
-                       Err(e) => assert_eq!(e.err, "Invalid signature from remote node")
+                       Err(e) => assert_eq!(e.err, "Invalid signature on channel_update message")
                };
        }