From: Jeffrey Czyz Date: Mon, 16 Aug 2021 23:40:19 +0000 (-0500) Subject: Add a read-only view of NetworkGraph X-Git-Tag: v0.0.101~20^2~6 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=798cf6ea8387b2365e0d45681c8cf28ed11f13e1;p=rust-lightning Add a read-only view of NetworkGraph Hide the internal locking of NetworkGraph by providing a read-only view. This way the locking order is handled internally. --- diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 2d5051166..61e94e049 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -58,6 +58,12 @@ pub struct NetworkGraph { nodes: RwLock>, } +/// A read-only view of [`NetworkGraph`]. +pub struct ReadOnlyNetworkGraph<'a> { + channels: RwLockReadGuard<'a, BTreeMap>, + nodes: RwLockReadGuard<'a, BTreeMap>, +} + /// Receives and validates network updates from peers, /// stores authentic and relevant data as a network graph. /// This network graph is then used for routing payments. @@ -171,7 +177,7 @@ impl RoutingMessageHandler for NetGraphMsgHandler wh fn get_next_channel_announcements(&self, starting_point: u64, batch_amount: u8) -> Vec<(ChannelAnnouncement, Option, Option)> { let mut result = Vec::with_capacity(batch_amount as usize); - let channels = self.network_graph.get_channels(); + let channels = self.network_graph.channels.read().unwrap(); let mut iter = channels.range(starting_point..); while result.len() < batch_amount as usize { if let Some((_, ref chan)) = iter.next() { @@ -199,7 +205,7 @@ impl RoutingMessageHandler for NetGraphMsgHandler wh fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec { let mut result = Vec::with_capacity(batch_amount as usize); - let nodes = self.network_graph.get_nodes(); + let nodes = self.network_graph.nodes.read().unwrap(); let mut iter = if let Some(pubkey) = starting_point { let mut iter = nodes.range((*pubkey)..); iter.next(); @@ -340,7 +346,8 @@ impl RoutingMessageHandler for NetGraphMsgHandler wh // (has at least one update). A peer may still want to know the channel // exists even if its not yet routable. let mut batches: Vec> = vec![Vec::with_capacity(MAX_SCIDS_PER_REPLY)]; - for (_, ref chan) in self.network_graph.get_channels().range(inclusive_start_scid.unwrap()..exclusive_end_scid.unwrap()) { + let channels = self.network_graph.channels.read().unwrap(); + for (_, ref chan) in channels.range(inclusive_start_scid.unwrap()..exclusive_end_scid.unwrap()) { if let Some(chan_announcement) = &chan.announcement_message { // Construct a new batch if last one is full if batches.last().unwrap().len() == batches.last().unwrap().capacity() { @@ -351,6 +358,7 @@ impl RoutingMessageHandler for NetGraphMsgHandler wh batch.push(chan_announcement.contents.short_channel_id); } } + drop(channels); let mut pending_events = self.pending_events.lock().unwrap(); let batch_count = batches.len(); @@ -662,34 +670,6 @@ impl PartialEq for NetworkGraph { } impl NetworkGraph { - /// Returns all known valid channels' short ids along with announced channel info. - /// - /// (C-not exported) because we have no mapping for `BTreeMap`s - pub fn get_channels(&self) -> RwLockReadGuard<'_, BTreeMap> { - self.channels.read().unwrap() - } - - /// Returns all known nodes' public keys along with announced node info. - /// - /// (C-not exported) because we have no mapping for `BTreeMap`s - pub fn get_nodes(&self) -> RwLockReadGuard<'_, BTreeMap> { - self.nodes.read().unwrap() - } - - /// Get network addresses by node id. - /// Returns None if the requested node is completely unknown, - /// or if node announcement for the node was never received. - /// - /// (C-not exported) as there is no practical way to track lifetimes of returned values. - pub fn get_addresses(&self, pubkey: &PublicKey) -> Option> { - if let Some(node) = self.nodes.read().unwrap().get(pubkey) { - if let Some(node_info) = node.announcement_info.as_ref() { - return Some(node_info.addresses.clone()) - } - } - None - } - /// Creates a new, empty, network graph. pub fn new(genesis_hash: BlockHash) -> NetworkGraph { Self { @@ -699,6 +679,16 @@ impl NetworkGraph { } } + /// Returns a read-only view of the network graph. + pub fn read_only(&'_ self) -> ReadOnlyNetworkGraph<'_> { + let channels = self.channels.read().unwrap(); + let nodes = self.nodes.read().unwrap(); + ReadOnlyNetworkGraph { + channels, + nodes, + } + } + /// For an already known node (from channel announcements), update its stored properties from a /// given node announcement. /// @@ -1064,6 +1054,36 @@ impl NetworkGraph { } } +impl ReadOnlyNetworkGraph<'_> { + /// Returns all known valid channels' short ids along with announced channel info. + /// + /// (C-not exported) because we have no mapping for `BTreeMap`s + pub fn channels(&self) -> &BTreeMap { + &*self.channels + } + + /// Returns all known nodes' public keys along with announced node info. + /// + /// (C-not exported) because we have no mapping for `BTreeMap`s + pub fn nodes(&self) -> &BTreeMap { + &*self.nodes + } + + /// Get network addresses by node id. + /// Returns None if the requested node is completely unknown, + /// or if node announcement for the node was never received. + /// + /// (C-not exported) as there is no practical way to track lifetimes of returned values. + pub fn get_addresses(&self, pubkey: &PublicKey) -> Option<&Vec> { + if let Some(node) = self.nodes.get(pubkey) { + if let Some(node_info) = node.announcement_info.as_ref() { + return Some(&node_info.addresses) + } + } + None + } +} + #[cfg(test)] mod tests { use chain; @@ -1268,7 +1288,7 @@ mod tests { { let network = &net_graph_msg_handler.network_graph; - match network.get_channels().get(&unsigned_announcement.short_channel_id) { + match network.read_only().channels().get(&unsigned_announcement.short_channel_id) { None => panic!(), Some(_) => () }; @@ -1320,7 +1340,7 @@ mod tests { { let network = &net_graph_msg_handler.network_graph; - match network.get_channels().get(&unsigned_announcement.short_channel_id) { + match network.read_only().channels().get(&unsigned_announcement.short_channel_id) { None => panic!(), Some(_) => () }; @@ -1351,7 +1371,7 @@ mod tests { }; { let network = &net_graph_msg_handler.network_graph; - match network.get_channels().get(&unsigned_announcement.short_channel_id) { + match network.read_only().channels().get(&unsigned_announcement.short_channel_id) { Some(channel_entry) => { assert_eq!(channel_entry.features, ChannelFeatures::empty()); }, @@ -1481,7 +1501,7 @@ mod tests { { let network = &net_graph_msg_handler.network_graph; - match network.get_channels().get(&short_channel_id) { + match network.read_only().channels().get(&short_channel_id) { None => panic!(), Some(channel_info) => { assert_eq!(channel_info.one_to_two.as_ref().unwrap().cltv_expiry_delta, 144); @@ -1587,7 +1607,7 @@ mod tests { { // There is no nodes in the table at the beginning. let network = &net_graph_msg_handler.network_graph; - assert_eq!(network.get_nodes().len(), 0); + assert_eq!(network.read_only().nodes().len(), 0); } { @@ -1643,7 +1663,7 @@ mod tests { // Non-permanent closing just disables a channel { let network = &net_graph_msg_handler.network_graph; - match network.get_channels().get(&short_channel_id) { + match network.read_only().channels().get(&short_channel_id) { None => panic!(), Some(channel_info) => { assert!(channel_info.one_to_two.is_some()); @@ -1661,7 +1681,7 @@ mod tests { // Non-permanent closing just disables a channel { let network = &net_graph_msg_handler.network_graph; - match network.get_channels().get(&short_channel_id) { + match network.read_only().channels().get(&short_channel_id) { None => panic!(), Some(channel_info) => { assert!(!channel_info.one_to_two.as_ref().unwrap().enabled); @@ -1679,9 +1699,9 @@ mod tests { // Permanent closing deletes a channel { let network = &net_graph_msg_handler.network_graph; - assert_eq!(network.get_channels().len(), 0); + assert_eq!(network.read_only().channels().len(), 0); // Nodes are also deleted because there are no associated channels anymore - assert_eq!(network.get_nodes().len(), 0); + assert_eq!(network.read_only().nodes().len(), 0); } // TODO: Test HTLCFailChannelUpdate::NodeFailure, which is not implemented yet. } @@ -1998,8 +2018,8 @@ mod tests { let network = &net_graph_msg_handler.network_graph; let mut w = test_utils::TestVecWriter(Vec::new()); - assert!(!network.get_nodes().is_empty()); - assert!(!network.get_channels().is_empty()); + assert!(!network.read_only().nodes().is_empty()); + assert!(!network.read_only().channels().is_empty()); network.write(&mut w).unwrap(); assert!(::read(&mut io::Cursor::new(&w.0)).unwrap() == *network); } diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 6d6d603c5..770b62d9c 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -443,8 +443,9 @@ pub fn get_route(our_node_id: &PublicKey, network: &NetworkGraph, paye // to use as the A* heuristic beyond just the cost to get one node further than the current // one. - let network_channels = network.get_channels(); - let network_nodes = network.get_nodes(); + let network_graph = network.read_only(); + let network_channels = network_graph.channels(); + let network_nodes = network_graph.nodes(); let dummy_directional_info = DummyDirectionalChannelInfo { // used for first_hops routes cltv_expiry_delta: 0, htlc_minimum_msat: 0, @@ -4213,7 +4214,7 @@ mod tests { // First, get 100 (source, destination) pairs for which route-getting actually succeeds... let mut seed = random_init_seed() as usize; - let nodes = graph.get_nodes(); + let nodes = graph.read_only().nodes().clone(); 'load_endpoints: for _ in 0..10 { loop { seed = seed.overflowing_mul(0xdeadbeef).0; @@ -4242,7 +4243,7 @@ mod tests { // First, get 100 (source, destination) pairs for which route-getting actually succeeds... let mut seed = random_init_seed() as usize; - let nodes = graph.get_nodes(); + let nodes = graph.read_only().nodes().clone(); 'load_endpoints: for _ in 0..10 { loop { seed = seed.overflowing_mul(0xdeadbeef).0; @@ -4301,7 +4302,7 @@ mod benches { fn generate_routes(bench: &mut Bencher) { let mut d = test_utils::get_route_file().unwrap(); let graph = NetworkGraph::read(&mut d).unwrap(); - let nodes = graph.get_nodes(); + let nodes = graph.read_only().nodes().clone(); // First, get 100 (source, destination) pairs for which route-getting actually succeeds... let mut path_endpoints = Vec::new(); @@ -4333,7 +4334,7 @@ mod benches { fn generate_mpp_routes(bench: &mut Bencher) { let mut d = test_utils::get_route_file().unwrap(); let graph = NetworkGraph::read(&mut d).unwrap(); - let nodes = graph.get_nodes(); + let nodes = graph.read_only().nodes().clone(); // First, get 100 (source, destination) pairs for which route-getting actually succeeds... let mut path_endpoints = Vec::new();