Re-broadcast our own gossip even if its same as the last broadcast
authorMatt Corallo <git@bluematt.me>
Sat, 13 Nov 2021 01:54:54 +0000 (01:54 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 23 Nov 2021 22:18:00 +0000 (22:18 +0000)
Even if our gossip hasn't changed, we should be willing to
re-broadcast it to our peers. All our peers may have been
disconnected the last time we broadcasted it.

lightning/src/ln/msgs.rs
lightning/src/ln/peer_handler.rs
lightning/src/routing/network_graph.rs

index 87944ccae639027da172e120ea83ea8b4d06411a..9662ffa7d48078f33eef4516f26ce8ffcb7d11d9 100644 (file)
@@ -715,6 +715,10 @@ pub enum ErrorAction {
        /// The peer did something harmless that we weren't able to meaningfully process.
        /// If the error is logged, log it at the given level.
        IgnoreAndLog(logger::Level),
+       /// The peer provided us with a gossip message which we'd already seen. In most cases this
+       /// should be ignored, but it may result in the message being forwarded if it is a duplicate of
+       /// our own channel announcements.
+       IgnoreDuplicateGossip,
        /// The peer did something incorrect. Tell them.
        SendErrorMessage {
                /// The message to send.
index a30e498a62ebff7d15d2f44a516e3ba6a8517855..9157dc87dc2fc5e415de7679974bfe0de3a91f0c 100644 (file)
@@ -811,6 +811,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                                                                        log_given_level!(self.logger, level, "Error handling message{}; ignoring: {}", OptionalFromDebugger(&peer.their_node_id), e.err);
                                                                                                        continue
                                                                                                },
+                                                                                               msgs::ErrorAction::IgnoreDuplicateGossip => continue, // Don't even bother logging these
                                                                                                msgs::ErrorAction::IgnoreError => {
                                                                                                        log_debug!(self.logger, "Error handling message{}; ignoring: {}", OptionalFromDebugger(&peer.their_node_id), e.err);
                                                                                                        continue;
@@ -1351,23 +1352,31 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                        },
                                        MessageSendEvent::BroadcastChannelAnnouncement { msg, update_msg } => {
                                                log_debug!(self.logger, "Handling BroadcastChannelAnnouncement event in peer_handler for short channel id {}", msg.contents.short_channel_id);
-                                               if self.message_handler.route_handler.handle_channel_announcement(&msg).is_ok() {
-                                                       self.forward_broadcast_msg(peers, &wire::Message::ChannelAnnouncement(msg), None);
+                                               match self.message_handler.route_handler.handle_channel_announcement(&msg) {
+                                                       Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
+                                                               self.forward_broadcast_msg(peers, &wire::Message::ChannelAnnouncement(msg), None),
+                                                       _ => {},
                                                }
-                                               if self.message_handler.route_handler.handle_channel_update(&update_msg).is_ok() {
-                                                       self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(update_msg), None);
+                                               match self.message_handler.route_handler.handle_channel_update(&update_msg) {
+                                                       Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
+                                                               self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(update_msg), None),
+                                                       _ => {},
                                                }
                                        },
                                        MessageSendEvent::BroadcastNodeAnnouncement { msg } => {
                                                log_debug!(self.logger, "Handling BroadcastNodeAnnouncement event in peer_handler");
-                                               if self.message_handler.route_handler.handle_node_announcement(&msg).is_ok() {
-                                                       self.forward_broadcast_msg(peers, &wire::Message::NodeAnnouncement(msg), None);
+                                               match self.message_handler.route_handler.handle_node_announcement(&msg) {
+                                                       Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
+                                                               self.forward_broadcast_msg(peers, &wire::Message::NodeAnnouncement(msg), None),
+                                                       _ => {},
                                                }
                                        },
                                        MessageSendEvent::BroadcastChannelUpdate { msg } => {
                                                log_debug!(self.logger, "Handling BroadcastChannelUpdate event in peer_handler for short channel id {}", msg.contents.short_channel_id);
-                                               if self.message_handler.route_handler.handle_channel_update(&msg).is_ok() {
-                                                       self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(msg), None);
+                                               match self.message_handler.route_handler.handle_channel_update(&msg) {
+                                                       Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
+                                                               self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(msg), None),
+                                                       _ => {},
                                                }
                                        },
                                        MessageSendEvent::SendChannelUpdate { ref node_id, ref msg } => {
@@ -1400,6 +1409,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
                                                        msgs::ErrorAction::IgnoreAndLog(level) => {
                                                                log_given_level!(self.logger, level, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
                                                        },
+                                                       msgs::ErrorAction::IgnoreDuplicateGossip => {},
                                                        msgs::ErrorAction::IgnoreError => {
                                                                log_debug!(self.logger, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
                                                        },
index 4d33bbdee9278717525c9991805245280bc22137..2dba12b630f38c98035ebe980aec39f1e4e31adb 100644 (file)
@@ -847,8 +847,10 @@ impl NetworkGraph {
                        None => Err(LightningError{err: "No existing channels for node_announcement".to_owned(), action: ErrorAction::IgnoreError}),
                        Some(node) => {
                                if let Some(node_info) = node.announcement_info.as_ref() {
-                                       if node_info.last_update  >= msg.timestamp {
+                                       if node_info.last_update  > msg.timestamp {
                                                return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)});
+                                       } else if node_info.last_update  == msg.timestamp {
+                                               return Err(LightningError{err: "Update had the same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
                                        }
                                }
 
@@ -977,7 +979,7 @@ impl NetworkGraph {
                                        Self::remove_channel_in_nodes(&mut nodes, &entry.get(), msg.short_channel_id);
                                        *entry.get_mut() = chan_info;
                                } else {
-                                       return Err(LightningError{err: "Already have knowledge of channel".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)})
+                                       return Err(LightningError{err: "Already have knowledge of channel".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
                                }
                        },
                        BtreeEntry::Vacant(entry) => {
@@ -1082,8 +1084,10 @@ impl NetworkGraph {
                                macro_rules! maybe_update_channel_info {
                                        ( $target: expr, $src_node: expr) => {
                                                if let Some(existing_chan_info) = $target.as_ref() {
-                                                       if existing_chan_info.last_update >= msg.timestamp {
+                                                       if existing_chan_info.last_update > msg.timestamp {
                                                                return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)});
+                                                       } else if existing_chan_info.last_update == msg.timestamp {
+                                                               return Err(LightningError{err: "Update had same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
                                                        }
                                                        chan_was_enabled = existing_chan_info.enabled;
                                                } else {
@@ -1720,7 +1724,7 @@ mod tests {
 
                match net_graph_msg_handler.handle_channel_update(&valid_channel_update) {
                        Ok(_) => panic!(),
-                       Err(e) => assert_eq!(e.err, "Update older than last processed update")
+                       Err(e) => assert_eq!(e.err, "Update had same timestamp as last processed update")
                };
                unsigned_channel_update.timestamp += 500;