Skip to content

Commit 391fbfb

Browse files
committed
Re-broadcast our own gossip even if its same as the last broadcast
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.
1 parent 8f89371 commit 391fbfb

File tree

3 files changed

+30
-12
lines changed

3 files changed

+30
-12
lines changed

lightning/src/ln/msgs.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,10 @@ pub enum ErrorAction {
715715
/// The peer did something harmless that we weren't able to meaningfully process.
716716
/// If the error is logged, log it at the given level.
717717
IgnoreAndLog(logger::Level),
718+
/// The peer provided us with a gossip message which we'd already seen. In most cases this
719+
/// should be ignored, but it may result in the message being forwarded if it is a duplicate of
720+
/// our own channel announcements.
721+
IgnoreDuplicateGossip,
718722
/// The peer did something incorrect. Tell them.
719723
SendErrorMessage {
720724
/// The message to send.

lightning/src/ln/peer_handler.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
811811
log_given_level!(self.logger, level, "Error handling message{}; ignoring: {}", OptionalFromDebugger(&peer.their_node_id), e.err);
812812
continue
813813
},
814+
msgs::ErrorAction::IgnoreDuplicateGossip => continue, // Don't even bother logging these
814815
msgs::ErrorAction::IgnoreError => {
815816
log_debug!(self.logger, "Error handling message{}; ignoring: {}", OptionalFromDebugger(&peer.their_node_id), e.err);
816817
continue;
@@ -1351,23 +1352,31 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
13511352
},
13521353
MessageSendEvent::BroadcastChannelAnnouncement { msg, update_msg } => {
13531354
log_debug!(self.logger, "Handling BroadcastChannelAnnouncement event in peer_handler for short channel id {}", msg.contents.short_channel_id);
1354-
if self.message_handler.route_handler.handle_channel_announcement(&msg).is_ok() {
1355-
self.forward_broadcast_msg(peers, &wire::Message::ChannelAnnouncement(msg), None);
1355+
match self.message_handler.route_handler.handle_channel_announcement(&msg) {
1356+
Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
1357+
self.forward_broadcast_msg(peers, &wire::Message::ChannelAnnouncement(msg), None),
1358+
_ => {},
13561359
}
1357-
if self.message_handler.route_handler.handle_channel_update(&update_msg).is_ok() {
1358-
self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(update_msg), None);
1360+
match self.message_handler.route_handler.handle_channel_update(&update_msg) {
1361+
Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
1362+
self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(update_msg), None),
1363+
_ => {},
13591364
}
13601365
},
13611366
MessageSendEvent::BroadcastNodeAnnouncement { msg } => {
13621367
log_debug!(self.logger, "Handling BroadcastNodeAnnouncement event in peer_handler");
1363-
if self.message_handler.route_handler.handle_node_announcement(&msg).is_ok() {
1364-
self.forward_broadcast_msg(peers, &wire::Message::NodeAnnouncement(msg), None);
1368+
match self.message_handler.route_handler.handle_node_announcement(&msg) {
1369+
Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
1370+
self.forward_broadcast_msg(peers, &wire::Message::NodeAnnouncement(msg), None),
1371+
_ => {},
13651372
}
13661373
},
13671374
MessageSendEvent::BroadcastChannelUpdate { msg } => {
13681375
log_debug!(self.logger, "Handling BroadcastChannelUpdate event in peer_handler for short channel id {}", msg.contents.short_channel_id);
1369-
if self.message_handler.route_handler.handle_channel_update(&msg).is_ok() {
1370-
self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(msg), None);
1376+
match self.message_handler.route_handler.handle_channel_update(&msg) {
1377+
Ok(_) | Err(LightningError { action: msgs::ErrorAction::IgnoreDuplicateGossip, .. }) =>
1378+
self.forward_broadcast_msg(peers, &wire::Message::ChannelUpdate(msg), None),
1379+
_ => {},
13711380
}
13721381
},
13731382
MessageSendEvent::SendChannelUpdate { ref node_id, ref msg } => {
@@ -1400,6 +1409,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref, CMH: Deref> P
14001409
msgs::ErrorAction::IgnoreAndLog(level) => {
14011410
log_given_level!(self.logger, level, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
14021411
},
1412+
msgs::ErrorAction::IgnoreDuplicateGossip => {},
14031413
msgs::ErrorAction::IgnoreError => {
14041414
log_debug!(self.logger, "Received a HandleError event to be ignored for node {}", log_pubkey!(node_id));
14051415
},

lightning/src/routing/network_graph.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -847,8 +847,10 @@ impl NetworkGraph {
847847
None => Err(LightningError{err: "No existing channels for node_announcement".to_owned(), action: ErrorAction::IgnoreError}),
848848
Some(node) => {
849849
if let Some(node_info) = node.announcement_info.as_ref() {
850-
if node_info.last_update >= msg.timestamp {
850+
if node_info.last_update > msg.timestamp {
851851
return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)});
852+
} else if node_info.last_update == msg.timestamp {
853+
return Err(LightningError{err: "Update had the same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
852854
}
853855
}
854856

@@ -977,7 +979,7 @@ impl NetworkGraph {
977979
Self::remove_channel_in_nodes(&mut nodes, &entry.get(), msg.short_channel_id);
978980
*entry.get_mut() = chan_info;
979981
} else {
980-
return Err(LightningError{err: "Already have knowledge of channel".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)})
982+
return Err(LightningError{err: "Already have knowledge of channel".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
981983
}
982984
},
983985
BtreeEntry::Vacant(entry) => {
@@ -1082,8 +1084,10 @@ impl NetworkGraph {
10821084
macro_rules! maybe_update_channel_info {
10831085
( $target: expr, $src_node: expr) => {
10841086
if let Some(existing_chan_info) = $target.as_ref() {
1085-
if existing_chan_info.last_update >= msg.timestamp {
1087+
if existing_chan_info.last_update > msg.timestamp {
10861088
return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreAndLog(Level::Gossip)});
1089+
} else if existing_chan_info.last_update == msg.timestamp {
1090+
return Err(LightningError{err: "Update had same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
10871091
}
10881092
chan_was_enabled = existing_chan_info.enabled;
10891093
} else {
@@ -1720,7 +1724,7 @@ mod tests {
17201724

17211725
match net_graph_msg_handler.handle_channel_update(&valid_channel_update) {
17221726
Ok(_) => panic!(),
1723-
Err(e) => assert_eq!(e.err, "Update older than last processed update")
1727+
Err(e) => assert_eq!(e.err, "Update had same timestamp as last processed update")
17241728
};
17251729
unsigned_channel_update.timestamp += 500;
17261730

0 commit comments

Comments
 (0)