X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Frouting%2Fgossip.rs;h=947df9edae0f2b3a166e96ff704c9c5775e7da8e;hb=626c60600ef3a4ec93d5f13df8d766bf3a7cd664;hp=14df033b69971c9db3240c71f409109dec94c9cc;hpb=6ca49948c169a297144d5d1474a2cef3827237e1;p=rust-lightning diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 14df033b..947df9ed 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -29,8 +29,9 @@ use crate::ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds use crate::ln::msgs; use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, MaybeReadable}; use crate::util::logger::{Logger, Level}; -use crate::util::events::{Event, EventHandler, MessageSendEvent, MessageSendEventsProvider}; +use crate::util::events::{MessageSendEvent, MessageSendEventsProvider}; use crate::util::scid_utils::{block_from_scid, scid_from_parts, MAX_SCID_BLOCK}; +use crate::util::string::PrintableString; use crate::io; use crate::io_extras::{copy, sink}; @@ -212,9 +213,6 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate, /// This network graph is then used for routing payments. /// Provides interface to help with initial routing sync by /// serving historical announcements. -/// -/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the -/// [`NetworkGraph`]. pub struct P2PGossipSync>, C: Deref, L: Deref> where C::Target: chain::Access, L::Target: Logger { @@ -274,32 +272,31 @@ where C::Target: chain::Access, L::Target: Logger } } -impl EventHandler for NetworkGraph where L::Target: Logger { - fn handle_event(&self, event: &Event) { - if let Event::PaymentPathFailed { network_update, .. } = event { - if let Some(network_update) = network_update { - match *network_update { - NetworkUpdate::ChannelUpdateMessage { ref msg } => { - let short_channel_id = msg.contents.short_channel_id; - let is_enabled = msg.contents.flags & (1 << 1) != (1 << 1); - let status = if is_enabled { "enabled" } else { "disabled" }; - log_debug!(self.logger, "Updating channel with channel_update from a payment failure. Channel {} is {}.", short_channel_id, status); - let _ = self.update_channel(msg); - }, - NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => { - let action = if is_permanent { "Removing" } else { "Disabling" }; - log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id); - self.channel_failed(short_channel_id, is_permanent); - }, - NetworkUpdate::NodeFailure { ref node_id, is_permanent } => { - if is_permanent { - log_debug!(self.logger, - "Removed node graph entry for {} due to a payment failure.", log_pubkey!(node_id)); - self.node_failed_permanent(node_id); - }; - }, - } - } +impl NetworkGraph where L::Target: Logger { + /// Handles any network updates originating from [`Event`]s. + /// + /// [`Event`]: crate::util::events::Event + pub fn handle_network_update(&self, network_update: &NetworkUpdate) { + match *network_update { + NetworkUpdate::ChannelUpdateMessage { ref msg } => { + let short_channel_id = msg.contents.short_channel_id; + let is_enabled = msg.contents.flags & (1 << 1) != (1 << 1); + let status = if is_enabled { "enabled" } else { "disabled" }; + log_debug!(self.logger, "Updating channel with channel_update from a payment failure. Channel {} is {}.", short_channel_id, status); + let _ = self.update_channel(msg); + }, + NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => { + let action = if is_permanent { "Removing" } else { "Disabling" }; + log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id); + self.channel_failed(short_channel_id, is_permanent); + }, + NetworkUpdate::NodeFailure { ref node_id, is_permanent } => { + if is_permanent { + log_debug!(self.logger, + "Removed node graph entry for {} due to a payment failure.", log_pubkey!(node_id)); + self.node_failed_permanent(node_id); + }; + }, } } } @@ -1022,23 +1019,17 @@ pub struct NodeAlias(pub [u8; 32]); impl fmt::Display for NodeAlias { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - let control_symbol = core::char::REPLACEMENT_CHARACTER; let first_null = self.0.iter().position(|b| *b == 0).unwrap_or(self.0.len()); let bytes = self.0.split_at(first_null).0; match core::str::from_utf8(bytes) { - Ok(alias) => { - for c in alias.chars() { - let mut bytes = [0u8; 4]; - let c = if !c.is_control() { c } else { control_symbol }; - f.write_str(c.encode_utf8(&mut bytes))?; - } - }, + Ok(alias) => PrintableString(alias).fmt(f)?, Err(_) => { + use core::fmt::Write; for c in bytes.iter().map(|b| *b as char) { // Display printable ASCII characters - let mut bytes = [0u8; 4]; + let control_symbol = core::char::REPLACEMENT_CHARACTER; let c = if c >= '\x20' && c <= '\x7e' { c } else { control_symbol }; - f.write_str(c.encode_utf8(&mut bytes))?; + f.write_char(c)?; } }, }; @@ -1653,7 +1644,7 @@ impl NetworkGraph where L::Target: Logger { if info.two_to_one.is_some() && info.two_to_one.as_ref().unwrap().last_update < min_time_unix { info.two_to_one = None; } - if info.one_to_two.is_none() && info.two_to_one.is_none() { + if info.one_to_two.is_none() || info.two_to_one.is_none() { // We check the announcement_received_time here to ensure we don't drop // announcements that we just received and are just waiting for our peer to send a // channel_update for. @@ -1667,6 +1658,7 @@ impl NetworkGraph where L::Target: Logger { for scid in scids_to_remove { let info = channels.remove(&scid).expect("We just accessed this scid, it should be present"); Self::remove_channel_in_nodes(&mut nodes, &info, scid); + self.removed_channels.lock().unwrap().insert(scid, Some(current_time_unix)); } } @@ -1935,7 +1927,6 @@ mod tests { use crate::chain; use crate::ln::channelmanager; use crate::ln::chan_utils::make_funding_redeemscript; - use crate::ln::PaymentHash; use crate::ln::features::InitFeatures; use crate::routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, NodeAlias, MAX_EXCESS_BYTES_FOR_RELAY, NodeId, RoutingFees, ChannelUpdateInfo, ChannelInfo, NodeAnnouncementInfo, NodeInfo}; use crate::ln::msgs::{RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement, @@ -1943,7 +1934,7 @@ mod tests { ReplyChannelRange, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT}; use crate::util::test_utils; use crate::util::ser::{ReadableArgs, Writeable}; - use crate::util::events::{Event, EventHandler, MessageSendEvent, MessageSendEventsProvider}; + use crate::util::events::{MessageSendEvent, MessageSendEventsProvider}; use crate::util::scid_utils::scid_from_parts; use crate::routing::gossip::REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS; @@ -2387,19 +2378,8 @@ mod tests { let valid_channel_update = get_signed_channel_update(|_| {}, node_1_privkey, &secp_ctx); assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none()); - network_graph.handle_event(&Event::PaymentPathFailed { - payment_id: None, - payment_hash: PaymentHash([0; 32]), - payment_failed_permanently: false, - all_paths_failed: true, - path: vec![], - network_update: Some(NetworkUpdate::ChannelUpdateMessage { - msg: valid_channel_update, - }), - short_channel_id: None, - retry: None, - error_code: None, - error_data: None, + network_graph.handle_network_update(&NetworkUpdate::ChannelUpdateMessage { + msg: valid_channel_update, }); assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some()); @@ -2414,20 +2394,9 @@ mod tests { } }; - network_graph.handle_event(&Event::PaymentPathFailed { - payment_id: None, - payment_hash: PaymentHash([0; 32]), - payment_failed_permanently: false, - all_paths_failed: true, - path: vec![], - network_update: Some(NetworkUpdate::ChannelFailure { - short_channel_id, - is_permanent: false, - }), - short_channel_id: None, - retry: None, - error_code: None, - error_data: None, + network_graph.handle_network_update(&NetworkUpdate::ChannelFailure { + short_channel_id, + is_permanent: false, }); match network_graph.read_only().channels().get(&short_channel_id) { @@ -2439,20 +2408,9 @@ mod tests { } // Permanent closing deletes a channel - network_graph.handle_event(&Event::PaymentPathFailed { - payment_id: None, - payment_hash: PaymentHash([0; 32]), - payment_failed_permanently: false, - all_paths_failed: true, - path: vec![], - network_update: Some(NetworkUpdate::ChannelFailure { - short_channel_id, - is_permanent: true, - }), - short_channel_id: None, - retry: None, - error_code: None, - error_data: None, + network_graph.handle_network_update(&NetworkUpdate::ChannelFailure { + short_channel_id, + is_permanent: true, }); assert_eq!(network_graph.read_only().channels().len(), 0); @@ -2471,40 +2429,18 @@ mod tests { assert!(network_graph.read_only().channels().get(&short_channel_id).is_some()); // Non-permanent node failure does not delete any nodes or channels - network_graph.handle_event(&Event::PaymentPathFailed { - payment_id: None, - payment_hash: PaymentHash([0; 32]), - payment_failed_permanently: false, - all_paths_failed: true, - path: vec![], - network_update: Some(NetworkUpdate::NodeFailure { - node_id: node_2_id, - is_permanent: false, - }), - short_channel_id: None, - retry: None, - error_code: None, - error_data: None, + network_graph.handle_network_update(&NetworkUpdate::NodeFailure { + node_id: node_2_id, + is_permanent: false, }); assert!(network_graph.read_only().channels().get(&short_channel_id).is_some()); assert!(network_graph.read_only().nodes().get(&NodeId::from_pubkey(&node_2_id)).is_some()); // Permanent node failure deletes node and its channels - network_graph.handle_event(&Event::PaymentPathFailed { - payment_id: None, - payment_hash: PaymentHash([0; 32]), - payment_failed_permanently: false, - all_paths_failed: true, - path: vec![], - network_update: Some(NetworkUpdate::NodeFailure { - node_id: node_2_id, - is_permanent: true, - }), - short_channel_id: None, - retry: None, - error_code: None, - error_data: None, + network_graph.handle_network_update(&NetworkUpdate::NodeFailure { + node_id: node_2_id, + is_permanent: true, }); assert_eq!(network_graph.read_only().nodes().len(), 0); @@ -2532,32 +2468,57 @@ mod tests { assert!(network_graph.update_channel_from_announcement(&valid_channel_announcement, &chain_source).is_ok()); assert!(network_graph.read_only().channels().get(&short_channel_id).is_some()); + // Submit two channel updates for each channel direction (update.flags bit). let valid_channel_update = get_signed_channel_update(|_| {}, node_1_privkey, &secp_ctx); assert!(gossip_sync.handle_channel_update(&valid_channel_update).is_ok()); assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some()); + let valid_channel_update_2 = get_signed_channel_update(|update| {update.flags |=1;}, node_2_privkey, &secp_ctx); + gossip_sync.handle_channel_update(&valid_channel_update_2).unwrap(); + assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().two_to_one.is_some()); + network_graph.remove_stale_channels_and_tracking_with_time(100 + STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS); assert_eq!(network_graph.read_only().channels().len(), 1); assert_eq!(network_graph.read_only().nodes().len(), 2); network_graph.remove_stale_channels_and_tracking_with_time(101 + STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS); + #[cfg(not(feature = "std"))] { + // Make sure removed channels are tracked. + assert_eq!(network_graph.removed_channels.lock().unwrap().len(), 1); + } + network_graph.remove_stale_channels_and_tracking_with_time(101 + STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS + + REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS); + #[cfg(feature = "std")] { // In std mode, a further check is performed before fully removing the channel - // the channel_announcement must have been received at least two weeks ago. We - // fudge that here by indicating the time has jumped two weeks. Note that the - // directional channel information will have been removed already.. + // fudge that here by indicating the time has jumped two weeks. assert_eq!(network_graph.read_only().channels().len(), 1); assert_eq!(network_graph.read_only().nodes().len(), 2); - assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none()); + // Note that the directional channel information will have been removed already.. + // We want to check that this will work even if *one* of the channel updates is recent, + // so we should add it with a recent timestamp. + assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none()); use std::time::{SystemTime, UNIX_EPOCH}; let announcement_time = SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs(); + let valid_channel_update = get_signed_channel_update(|unsigned_channel_update| { + unsigned_channel_update.timestamp = (announcement_time + 1 + STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS) as u32; + }, node_1_privkey, &secp_ctx); + assert!(gossip_sync.handle_channel_update(&valid_channel_update).is_ok()); + assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some()); network_graph.remove_stale_channels_and_tracking_with_time(announcement_time + 1 + STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS); + // Make sure removed channels are tracked. + assert_eq!(network_graph.removed_channels.lock().unwrap().len(), 1); + // Provide a later time so that sufficient time has passed + network_graph.remove_stale_channels_and_tracking_with_time(announcement_time + 1 + STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS + + REMOVED_ENTRIES_TRACKING_AGE_LIMIT_SECS); } assert_eq!(network_graph.read_only().channels().len(), 0); assert_eq!(network_graph.read_only().nodes().len(), 0); + assert!(network_graph.removed_channels.lock().unwrap().is_empty()); #[cfg(feature = "std")] {