From 8467223a5d7ea744c2af8557ed2e6a7a20991bdd Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Sun, 3 May 2020 16:06:59 -0400 Subject: [PATCH] Make DirectionalChannelInfo optional --- lightning/src/routing/network_graph.rs | 176 +++++++++++++++---------- lightning/src/routing/router.rs | 41 +++--- 2 files changed, 131 insertions(+), 86 deletions(-) diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 7e41c025..f14ab42f 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -141,9 +141,16 @@ impl RoutingMessageHandler for NetGraphMsgHandler { while result.len() < batch_amount as usize { if let Some((_, ref chan)) = iter.next() { if chan.announcement_message.is_some() { - result.push((chan.announcement_message.clone().unwrap(), - chan.one_to_two.last_update_message.clone(), - chan.two_to_one.last_update_message.clone())); + let chan_announcement = chan.announcement_message.clone().unwrap(); + let mut one_to_two_announcement: Option = None; + let mut two_to_one_announcement: Option = None; + if let Some(one_to_two) = chan.one_to_two.as_ref() { + one_to_two_announcement = one_to_two.last_update_message.clone(); + } + if let Some(two_to_one) = chan.two_to_one.as_ref() { + two_to_one_announcement = two_to_one.last_update_message.clone(); + } + result.push((chan_announcement, one_to_two_announcement, two_to_one_announcement)); } else { // TODO: We may end up sending un-announced channel_updates if we are sending // initial sync data while receiving announce/updates for this channel. @@ -238,11 +245,9 @@ impl ReadableArgs for NetGraphMsgHandler { } } -#[derive(PartialEq)] +#[derive(PartialEq, Debug)] /// Details regarding one direction of a channel pub struct DirectionalChannelInfo { - /// A node from which the channel direction starts - pub src_node_id: PublicKey, /// When the last update to the channel direction was issued pub last_update: u32, /// Whether the channel can be currently used for payments @@ -259,13 +264,12 @@ pub struct DirectionalChannelInfo { impl std::fmt::Display for DirectionalChannelInfo { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - write!(f, "src_node_id {}, last_update {}, enabled {}, cltv_expiry_delta {}, htlc_minimum_msat {}, fees {:?}", log_pubkey!(self.src_node_id), self.last_update, self.enabled, self.cltv_expiry_delta, self.htlc_minimum_msat, self.fees)?; + write!(f, "last_update {}, enabled {}, cltv_expiry_delta {}, htlc_minimum_msat {}, fees {:?}", self.last_update, self.enabled, self.cltv_expiry_delta, self.htlc_minimum_msat, self.fees)?; Ok(()) } } impl_writeable!(DirectionalChannelInfo, 0, { - src_node_id, last_update, enabled, cltv_expiry_delta, @@ -279,10 +283,14 @@ impl_writeable!(DirectionalChannelInfo, 0, { pub struct ChannelInfo { /// Protocol features of a channel communicated during its announcement pub features: ChannelFeatures, - /// Details regarding one of the directions of a channel - pub one_to_two: DirectionalChannelInfo, - /// Details regarding another direction of a channel - pub two_to_one: DirectionalChannelInfo, + /// Source node of the first direction of a channel + pub node_one: PublicKey, + /// Details about the first direction of a channel + pub one_to_two: Option, + /// Source node of the second direction of a channel + pub node_two: PublicKey, + /// Details about the second direction of a channel + pub two_to_one: Option, /// An initial announcement of the channel //this is cached here so we can send out it later if required by initial routing sync //keep an eye on this to see if the extra memory is a problem @@ -291,14 +299,17 @@ pub struct ChannelInfo { impl std::fmt::Display for ChannelInfo { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - write!(f, "features: {}, one_to_two: {}, two_to_one: {}", log_bytes!(self.features.encode()), self.one_to_two, self.two_to_one)?; + write!(f, "features: {}, node_one: {}, one_to_two: {:?}, node_two: {}, two_to_one: {:?}", + log_bytes!(self.features.encode()), log_pubkey!(self.node_one), self.one_to_two, log_pubkey!(self.node_two), self.two_to_one)?; Ok(()) } } impl_writeable!(ChannelInfo, 0, { features, + node_one, one_to_two, + node_two, two_to_one, announcement_message }); @@ -560,30 +571,10 @@ impl NetworkGraph { let chan_info = ChannelInfo { features: msg.contents.features.clone(), - one_to_two: DirectionalChannelInfo { - src_node_id: msg.contents.node_id_1.clone(), - last_update: 0, - enabled: false, - cltv_expiry_delta: u16::max_value(), - htlc_minimum_msat: u64::max_value(), - fees: RoutingFees { - base_msat: u32::max_value(), - proportional_millionths: u32::max_value(), - }, - last_update_message: None, - }, - two_to_one: DirectionalChannelInfo { - src_node_id: msg.contents.node_id_2.clone(), - last_update: 0, - enabled: false, - cltv_expiry_delta: u16::max_value(), - htlc_minimum_msat: u64::max_value(), - fees: RoutingFees { - base_msat: u32::max_value(), - proportional_millionths: u32::max_value(), - }, - last_update_message: None, - }, + node_one: msg.contents.node_id_1.clone(), + one_to_two: None, + node_two: msg.contents.node_id_2.clone(), + two_to_one: None, announcement_message: if should_relay { Some(msg.clone()) } else { None }, }; @@ -646,8 +637,12 @@ impl NetworkGraph { } } else { if let Some(chan) = self.channels.get_mut(&short_channel_id) { - chan.one_to_two.enabled = false; - chan.two_to_one.enabled = false; + if let Some(one_to_two) = chan.one_to_two.as_mut() { + one_to_two.enabled = false; + } + if let Some(two_to_one) = chan.two_to_one.as_mut() { + two_to_one.enabled = false; + } } } } @@ -671,37 +666,50 @@ impl NetworkGraph { None => return Err(LightningError{err: "Couldn't find channel for update", action: ErrorAction::IgnoreError}), Some(channel) => { macro_rules! maybe_update_channel_info { - ( $target: expr) => { - if $target.last_update >= msg.contents.timestamp { - return Err(LightningError{err: "Update older than last processed update", action: ErrorAction::IgnoreError}); + ( $target: expr, $src_node: expr) => { + if let Some(existing_chan_info) = $target.as_ref() { + if existing_chan_info.last_update >= msg.contents.timestamp { + return Err(LightningError{err: "Update older than last processed update", action: ErrorAction::IgnoreError}); + } + chan_was_enabled = existing_chan_info.enabled; + } else { + chan_was_enabled = false; } - chan_was_enabled = $target.enabled; - $target.last_update = msg.contents.timestamp; - $target.enabled = chan_enabled; - $target.cltv_expiry_delta = msg.contents.cltv_expiry_delta; - $target.htlc_minimum_msat = msg.contents.htlc_minimum_msat; - $target.fees.base_msat = msg.contents.fee_base_msat; - $target.fees.proportional_millionths = msg.contents.fee_proportional_millionths; - $target.last_update_message = if msg.contents.excess_data.is_empty() { + + let last_update_message = if msg.contents.excess_data.is_empty() { Some(msg.clone()) } else { None }; + + let updated_channel_dir_info = DirectionalChannelInfo { + enabled: chan_enabled, + last_update: msg.contents.timestamp, + cltv_expiry_delta: msg.contents.cltv_expiry_delta, + htlc_minimum_msat: msg.contents.htlc_minimum_msat, + fees: RoutingFees { + base_msat: msg.contents.fee_base_msat, + proportional_millionths: msg.contents.fee_proportional_millionths, + }, + last_update_message + }; + $target = Some(updated_channel_dir_info); } } + let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]); if msg.contents.flags & 1 == 1 { - dest_node_id = channel.one_to_two.src_node_id.clone(); + dest_node_id = channel.node_one.clone(); if let Some(sig_verifier) = secp_ctx { - secp_verify_sig!(sig_verifier, &msg_hash, &msg.signature, &channel.two_to_one.src_node_id); + secp_verify_sig!(sig_verifier, &msg_hash, &msg.signature, &channel.node_two); } - maybe_update_channel_info!(channel.two_to_one); + maybe_update_channel_info!(channel.two_to_one, channel.node_two); } else { - dest_node_id = channel.two_to_one.src_node_id.clone(); + dest_node_id = channel.node_two.clone(); if let Some(sig_verifier) = secp_ctx { - secp_verify_sig!(sig_verifier, &msg_hash, &msg.signature, &channel.one_to_two.src_node_id); + secp_verify_sig!(sig_verifier, &msg_hash, &msg.signature, &channel.node_one); } - maybe_update_channel_info!(channel.one_to_two); + maybe_update_channel_info!(channel.one_to_two, channel.node_one); } } } @@ -727,13 +735,15 @@ impl NetworkGraph { for chan_id in node.channels.iter() { let chan = self.channels.get(chan_id).unwrap(); - if chan.one_to_two.src_node_id == dest_node_id { - lowest_inbound_channel_fee_base_msat = cmp::min(lowest_inbound_channel_fee_base_msat, chan.two_to_one.fees.base_msat); - lowest_inbound_channel_fee_proportional_millionths = cmp::min(lowest_inbound_channel_fee_proportional_millionths, chan.two_to_one.fees.proportional_millionths); + // Since direction was enabled, the channel indeed had directional info + let chan_info; + if chan.node_one == dest_node_id { + chan_info = chan.two_to_one.as_ref().unwrap(); } else { - lowest_inbound_channel_fee_base_msat = cmp::min(lowest_inbound_channel_fee_base_msat, chan.one_to_two.fees.base_msat); - lowest_inbound_channel_fee_proportional_millionths = cmp::min(lowest_inbound_channel_fee_proportional_millionths, chan.one_to_two.fees.proportional_millionths); + chan_info = chan.one_to_two.as_ref().unwrap(); } + lowest_inbound_channel_fee_base_msat = cmp::min(lowest_inbound_channel_fee_base_msat, chan_info.fees.base_msat); + lowest_inbound_channel_fee_proportional_millionths = cmp::min(lowest_inbound_channel_fee_proportional_millionths, chan_info.fees.proportional_millionths); } } @@ -765,8 +775,9 @@ impl NetworkGraph { } } } - remove_from_node!(chan.one_to_two.src_node_id); - remove_from_node!(chan.two_to_one.src_node_id); + + remove_from_node!(chan.node_one); + remove_from_node!(chan.node_two); } } @@ -1177,8 +1188,8 @@ mod tests { match network.get_channels().get(&short_channel_id) { None => panic!(), Some(channel_info) => { - assert_eq!(channel_info.one_to_two.cltv_expiry_delta, 144); - assert_eq!(channel_info.two_to_one.cltv_expiry_delta, u16::max_value()); + assert_eq!(channel_info.one_to_two.as_ref().unwrap().cltv_expiry_delta, 144); + assert!(channel_info.two_to_one.is_none()); } } } @@ -1283,6 +1294,38 @@ mod tests { Err(_) => panic!() }; + let unsigned_channel_update = UnsignedChannelUpdate { + chain_hash, + short_channel_id, + timestamp: 100, + flags: 0, + cltv_expiry_delta: 144, + htlc_minimum_msat: 1000000, + fee_base_msat: 10000, + fee_proportional_millionths: 20, + excess_data: Vec::new() + }; + let msghash = hash_to_message!(&Sha256dHash::hash(&unsigned_channel_update.encode()[..])[..]); + let valid_channel_update = ChannelUpdate { + signature: secp_ctx.sign(&msghash, node_1_privkey), + contents: unsigned_channel_update.clone() + }; + + match net_graph_msg_handler.handle_channel_update(&valid_channel_update) { + Ok(res) => assert!(res), + _ => panic!() + }; + } + + // Non-permanent closing just disables a channel + { + let network = net_graph_msg_handler.network_graph.read().unwrap(); + match network.get_channels().get(&short_channel_id) { + None => panic!(), + Some(channel_info) => { + assert!(channel_info.one_to_two.is_some()); + } + } } let channel_close_msg = HTLCFailChannelUpdate::ChannelClosed { @@ -1298,8 +1341,7 @@ mod tests { match network.get_channels().get(&short_channel_id) { None => panic!(), Some(channel_info) => { - assert!(!channel_info.one_to_two.enabled); - assert!(!channel_info.two_to_one.enabled); + assert!(!channel_info.one_to_two.as_ref().unwrap().enabled); } } } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 28d14c4d..d593e8c2 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -138,7 +138,6 @@ impl cmp::PartialOrd for RouteGraphNode { } struct DummyDirectionalChannelInfo { - src_node_id: PublicKey, cltv_expiry_delta: u32, htlc_minimum_msat: u64, fees: RoutingFees, @@ -180,7 +179,6 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan // one. let dummy_directional_info = DummyDirectionalChannelInfo { // used for first_hops routes - src_node_id: our_node_id.clone(), cltv_expiry_delta: 0, htlc_minimum_msat: 0, fees: RoutingFees { @@ -217,10 +215,10 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan } macro_rules! add_entry { - // Adds entry which goes from the node pointed to by $directional_info to - // $dest_node_id over the channel with id $chan_id with fees described in + // Adds entry which goes from $src_node_id to $dest_node_id + // over the channel with id $chan_id with fees described in // $directional_info. - ( $chan_id: expr, $dest_node_id: expr, $directional_info: expr, $chan_features: expr, $starting_fee_msat: expr ) => { + ( $chan_id: expr, $src_node_id: expr, $dest_node_id: expr, $directional_info: expr, $chan_features: expr, $starting_fee_msat: expr ) => { //TODO: Explore simply adding fee to hit htlc_minimum_msat if $starting_fee_msat as u64 + final_value_msat >= $directional_info.htlc_minimum_msat { let proportional_fee_millions = ($starting_fee_msat + final_value_msat).checked_mul($directional_info.fees.proportional_millionths as u64); @@ -228,9 +226,9 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan ($directional_info.fees.base_msat as u64).checked_add(part / 1000000) }) { let mut total_fee = $starting_fee_msat as u64; - let hm_entry = dist.entry(&$directional_info.src_node_id); + let hm_entry = dist.entry(&$src_node_id); let old_entry = hm_entry.or_insert_with(|| { - let node = network.get_nodes().get(&$directional_info.src_node_id).unwrap(); + let node = network.get_nodes().get(&$src_node_id).unwrap(); let mut fee_base_msat = u32::max_value(); let mut fee_proportional_millionths = u32::max_value(); if let Some(fees) = node.lowest_inbound_channel_fees { @@ -249,7 +247,7 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan cltv_expiry_delta: 0, }) }); - if $directional_info.src_node_id != *our_node_id { + if $src_node_id != *our_node_id { // Ignore new_fee for channel-from-us as we assume all channels-from-us // will have the same effective-fee total_fee += new_fee; @@ -261,7 +259,7 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan } } let new_graph_node = RouteGraphNode { - pubkey: $directional_info.src_node_id, + pubkey: $src_node_id, lowest_fee_to_peer_through_node: total_fee, lowest_fee_to_node: $starting_fee_msat as u64 + new_fee, }; @@ -286,7 +284,7 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan ( $node: expr, $node_id: expr, $fee_to_target_msat: expr ) => { if first_hops.is_some() { if let Some(&(ref first_hop, ref features)) = first_hop_targets.get(&$node_id) { - add_entry!(first_hop, $node_id, dummy_directional_info, features.to_context(), $fee_to_target_msat); + add_entry!(first_hop, *our_node_id, $node_id, dummy_directional_info, features.to_context(), $fee_to_target_msat); } } @@ -301,18 +299,23 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan for chan_id in $node.channels.iter() { let chan = network.get_channels().get(chan_id).unwrap(); if !chan.features.requires_unknown_bits() { - if chan.one_to_two.src_node_id == *$node_id { + if chan.node_one == *$node_id { // ie $node is one, ie next hop in A* is two, via the two_to_one channel - if first_hops.is_none() || chan.two_to_one.src_node_id != *our_node_id { - if chan.two_to_one.enabled { - add_entry!(chan_id, chan.one_to_two.src_node_id, chan.two_to_one, chan.features, $fee_to_target_msat); + if first_hops.is_none() || chan.node_two != *our_node_id { + if let Some(two_to_one) = chan.two_to_one.as_ref() { + if two_to_one.enabled { + add_entry!(chan_id, chan.node_two, chan.node_one, two_to_one, chan.features, $fee_to_target_msat); + } } } } else { - if first_hops.is_none() || chan.one_to_two.src_node_id != *our_node_id { - if chan.one_to_two.enabled { - add_entry!(chan_id, chan.two_to_one.src_node_id, chan.one_to_two, chan.features, $fee_to_target_msat); + if first_hops.is_none() || chan.node_one != *our_node_id { + if let Some(one_to_two) = chan.one_to_two.as_ref() { + if one_to_two.enabled { + add_entry!(chan_id, chan.node_one, chan.node_two, one_to_two, chan.features, $fee_to_target_msat); + } } + } } } @@ -337,12 +340,12 @@ pub fn get_route(our_node_id: &PublicKey, net_graph_msg_handler: &NetGraphMsgHan // bit lazy here. In the future, we should pull them out via our // ChannelManager, but there's no reason to waste the space until we // need them. - add_entry!(first_hop, hop.src_node_id, dummy_directional_info, features.to_context(), 0); + add_entry!(first_hop, *our_node_id , hop.src_node_id, dummy_directional_info, features.to_context(), 0); } } // BOLT 11 doesn't allow inclusion of features for the last hop hints, which // really sucks, cause we're gonna need that eventually. - add_entry!(hop.short_channel_id, target, hop, ChannelFeatures::empty(), 0); + add_entry!(hop.short_channel_id, hop.src_node_id, target, hop, ChannelFeatures::empty(), 0); } } } -- 2.30.2