X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Frouting%2Fgossip.rs;h=c700fb0e4f90fbf0c5780269e9cda759b7140a3c;hb=0bf07ab39b77d25c71fa4c42c36ad86876b05d20;hp=1706e0692ef90ea1fdfbe26e7387239cfd188d3f;hpb=fc9a4c22d195a75ad5942eed271757f285452214;p=rust-lightning diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 1706e069..c700fb0e 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); + }; + }, } } } @@ -740,7 +737,7 @@ pub struct ChannelInfo { impl ChannelInfo { /// Returns a [`DirectedChannelInfo`] for the channel directed to the given `target` from a /// returned `source`, or `None` if `target` is not one of the channel's counterparties. - pub fn as_directed_to(&self, target: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { + pub(crate) fn as_directed_to(&self, target: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> { let (direction, source) = { if target == &self.node_one { (self.two_to_one.as_ref(), &self.node_two) @@ -750,7 +747,7 @@ impl ChannelInfo { return None; } }; - Some((DirectedChannelInfo::new(self, direction), source)) + direction.map(|dir| (DirectedChannelInfo::new(self, dir), source)) } /// Returns a [`DirectedChannelInfo`] for the channel directed from the given `source` to a @@ -765,7 +762,7 @@ impl ChannelInfo { return None; } }; - Some((DirectedChannelInfo::new(self, direction), target)) + direction.map(|dir| (DirectedChannelInfo::new(self, dir), target)) } /// Returns a [`ChannelUpdateInfo`] based on the direction implied by the channel_flag. @@ -860,29 +857,23 @@ impl Readable for ChannelInfo { #[derive(Clone)] pub struct DirectedChannelInfo<'a> { channel: &'a ChannelInfo, - direction: Option<&'a ChannelUpdateInfo>, + direction: &'a ChannelUpdateInfo, htlc_maximum_msat: u64, effective_capacity: EffectiveCapacity, } impl<'a> DirectedChannelInfo<'a> { #[inline] - fn new(channel: &'a ChannelInfo, direction: Option<&'a ChannelUpdateInfo>) -> Self { - let htlc_maximum_msat = direction.map(|direction| direction.htlc_maximum_msat); + fn new(channel: &'a ChannelInfo, direction: &'a ChannelUpdateInfo) -> Self { + let mut htlc_maximum_msat = direction.htlc_maximum_msat; let capacity_msat = channel.capacity_sats.map(|capacity_sats| capacity_sats * 1000); - let (htlc_maximum_msat, effective_capacity) = match (htlc_maximum_msat, capacity_msat) { - (Some(amount_msat), Some(capacity_msat)) => { - let htlc_maximum_msat = cmp::min(amount_msat, capacity_msat); - (htlc_maximum_msat, EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_maximum_msat) }) - }, - (Some(amount_msat), None) => { - (amount_msat, EffectiveCapacity::MaximumHTLC { amount_msat }) - }, - (None, Some(capacity_msat)) => { - (capacity_msat, EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: None }) + let effective_capacity = match capacity_msat { + Some(capacity_msat) => { + htlc_maximum_msat = cmp::min(htlc_maximum_msat, capacity_msat); + EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: htlc_maximum_msat } }, - (None, None) => (EffectiveCapacity::Unknown.as_msat(), EffectiveCapacity::Unknown), + None => EffectiveCapacity::MaximumHTLC { amount_msat: htlc_maximum_msat }, }; Self { @@ -891,12 +882,11 @@ impl<'a> DirectedChannelInfo<'a> { } /// Returns information for the channel. + #[inline] pub fn channel(&self) -> &'a ChannelInfo { self.channel } - /// Returns information for the direction. - pub fn direction(&self) -> Option<&'a ChannelUpdateInfo> { self.direction } - /// Returns the maximum HTLC amount allowed over the channel in the direction. + #[inline] pub fn htlc_maximum_msat(&self) -> u64 { self.htlc_maximum_msat } @@ -910,13 +900,9 @@ impl<'a> DirectedChannelInfo<'a> { self.effective_capacity } - /// Returns `Some` if [`ChannelUpdateInfo`] is available in the direction. - pub(super) fn with_update(self) -> Option> { - match self.direction { - Some(_) => Some(DirectedChannelInfoWithUpdate { inner: self }), - None => None, - } - } + /// Returns information for the direction. + #[inline] + pub(super) fn direction(&self) -> &'a ChannelUpdateInfo { self.direction } } impl<'a> fmt::Debug for DirectedChannelInfo<'a> { @@ -927,32 +913,6 @@ impl<'a> fmt::Debug for DirectedChannelInfo<'a> { } } -/// A [`DirectedChannelInfo`] with [`ChannelUpdateInfo`] available in its direction. -#[derive(Clone)] -pub(super) struct DirectedChannelInfoWithUpdate<'a> { - inner: DirectedChannelInfo<'a>, -} - -impl<'a> DirectedChannelInfoWithUpdate<'a> { - /// Returns information for the channel. - #[inline] - pub(super) fn channel(&self) -> &'a ChannelInfo { &self.inner.channel } - - /// Returns information for the direction. - #[inline] - pub(super) fn direction(&self) -> &'a ChannelUpdateInfo { self.inner.direction.unwrap() } - - /// Returns the [`EffectiveCapacity`] of the channel in the direction. - #[inline] - pub(super) fn effective_capacity(&self) -> EffectiveCapacity { self.inner.effective_capacity() } -} - -impl<'a> fmt::Debug for DirectedChannelInfoWithUpdate<'a> { - fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - self.inner.fmt(f) - } -} - /// The effective capacity of a channel for routing purposes. /// /// While this may be smaller than the actual channel capacity, amounts greater than @@ -976,7 +936,7 @@ pub enum EffectiveCapacity { /// The funding amount denominated in millisatoshi. capacity_msat: u64, /// The maximum HTLC amount denominated in millisatoshi. - htlc_maximum_msat: Option + htlc_maximum_msat: u64 }, /// A capacity sufficient to route any payment, typically used for private channels provided by /// an invoice. @@ -1059,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)?; } }, }; @@ -1690,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. @@ -1704,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)); } } @@ -1972,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, @@ -1980,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; @@ -2424,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()); @@ -2451,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) { @@ -2476,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); @@ -2508,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); @@ -2569,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")] {