Have find_route take a NetworkGraph instead of a ReadOnly one

Because downstream languages are often garbage-collected, having
the user directly allocate a `ReadOnlyNetworkGraph` and pass a
reference to it to `find_route` often results in holding a read
lock long in excess of the `find_route` call. Worse, some languages
(like JavaScript) tend to only garbage collect when other code is
not running, possibly leading to deadlocks.

Author: TheBlueMatt

fuzz/src/full_stack.rs

@@ -460,7 +460,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 					final_cltv_expiry_delta: 42,
 				};
 				let random_seed_bytes: [u8; 32] = keys_manager.get_secure_random_bytes();
-				let route = match find_route(&our_id, &params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+				let route = match find_route(&our_id, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
 					Ok(route) => route,
 					Err(_) => return,
 				};
@@ -484,7 +484,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 					final_cltv_expiry_delta: 42,
 				};
 				let random_seed_bytes: [u8; 32] = keys_manager.get_secure_random_bytes();
-				let mut route = match find_route(&our_id, &params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+				let mut route = match find_route(&our_id, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
 					Ok(route) => route,
 					Err(_) => return,
 				};

fuzz/src/router.rs

@@ -268,7 +268,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 						final_value_msat: slice_to_be64(get_slice!(8)),
 						final_cltv_expiry_delta: slice_to_be32(get_slice!(4)),
 					};
-					let _ = find_route(&our_pubkey, &route_params, &net_graph.read_only(),
+					let _ = find_route(&our_pubkey, &route_params, &net_graph,
 						first_hops.map(|c| c.iter().collect::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
 						&logger, &scorer, &random_seed_bytes);
 				}

lightning-invoice/src/utils.rs

@@ -461,13 +461,12 @@ where L::Target: Logger {
 		&self, payer: &PublicKey, params: &RouteParameters, _payment_hash: &PaymentHash,
 		first_hops: Option<&[&ChannelDetails]>, scorer: &S
 	) -> Result<Route, LightningError> {
-		let network_graph = self.network_graph.read_only();
 		let random_seed_bytes = {
 			let mut locked_random_seed_bytes = self.random_seed_bytes.lock().unwrap();
 			*locked_random_seed_bytes = sha256::Hash::hash(&*locked_random_seed_bytes).into_inner();
 			*locked_random_seed_bytes
 		};
-		find_route(payer, params, &network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
+		find_route(payer, params, &self.network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
 	}
 }
 
@@ -572,7 +571,7 @@ mod test {
 		let scorer = test_utils::TestScorer::with_penalty(0);
 		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 		let route = find_route(
-			&nodes[0].node.get_our_node_id(), &route_params, &network_graph.read_only(),
+			&nodes[0].node.get_our_node_id(), &route_params, &network_graph,
 			Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
 		).unwrap();
 
@@ -848,7 +847,7 @@ mod test {
 		let scorer = test_utils::TestScorer::with_penalty(0);
 		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 		let route = find_route(
-			&nodes[0].node.get_our_node_id(), &params, &network_graph.read_only(),
+			&nodes[0].node.get_our_node_id(), &params, &network_graph,
 			Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
 		).unwrap();
 		let (payment_event, fwd_idx) = {

lightning/src/ln/channelmanager.rs

@@ -7460,7 +7460,7 @@ mod tests {
 			final_cltv_expiry_delta: TEST_FINAL_CLTV,
 		};
 		let route = find_route(
-			&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(),
+			&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
 			None, nodes[0].logger, &scorer, &random_seed_bytes
 		).unwrap();
 		nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
@@ -7491,7 +7491,7 @@ mod tests {
 		// To start (2), send a keysend payment but don't claim it.
 		let payment_preimage = PaymentPreimage([42; 32]);
 		let route = find_route(
-			&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(),
+			&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
 			None, nodes[0].logger, &scorer, &random_seed_bytes
 		).unwrap();
 		let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
@@ -7555,9 +7555,8 @@ mod tests {
 		let scorer = test_utils::TestScorer::with_penalty(0);
 		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 		let route = find_route(
-			&payer_pubkey, &route_params, &network_graph.read_only(),
-			Some(&first_hops.iter().collect::<Vec<_>>()), nodes[0].logger, &scorer,
-			&random_seed_bytes
+			&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+			nodes[0].logger, &scorer, &random_seed_bytes
 		).unwrap();
 
 		let test_preimage = PaymentPreimage([42; 32]);
@@ -7600,9 +7599,8 @@ mod tests {
 		let scorer = test_utils::TestScorer::with_penalty(0);
 		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 		let route = find_route(
-			&payer_pubkey, &route_params, &network_graph.read_only(),
-			Some(&first_hops.iter().collect::<Vec<_>>()), nodes[0].logger, &scorer,
-			&random_seed_bytes
+			&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+			nodes[0].logger, &scorer, &random_seed_bytes
 		).unwrap();
 
 		let test_preimage = PaymentPreimage([42; 32]);

lightning/src/ln/functional_tests.rs

@@ -9874,7 +9874,7 @@ fn test_keysend_payments_to_public_node() {
 	};
 	let scorer = test_utils::TestScorer::with_penalty(0);
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
-	let route = find_route(&payer_pubkey, &route_params, &network_graph.read_only(), None, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
+	let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
 
 	let test_preimage = PaymentPreimage([42; 32]);
 	let (payment_hash, _) = nodes[0].node.send_spontaneous_payment(&route, Some(test_preimage)).unwrap();
@@ -9910,8 +9910,8 @@ fn test_keysend_payments_to_private_node() {
 	let scorer = test_utils::TestScorer::with_penalty(0);
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 	let route = find_route(
-		&payer_pubkey, &route_params, &network_graph.read_only(),
-		Some(&first_hops.iter().collect::<Vec<_>>()), nodes[0].logger, &scorer, &random_seed_bytes
+		&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
+		nodes[0].logger, &scorer, &random_seed_bytes
 	).unwrap();
 
 	let test_preimage = PaymentPreimage([42; 32]);

lightning/src/routing/router.rs

@@ -17,7 +17,7 @@ use bitcoin::secp256k1::PublicKey;
 use ln::channelmanager::ChannelDetails;
 use ln::features::{ChannelFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
-use routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NodeId, RoutingFees};
+use routing::gossip::{DirectedChannelInfoWithUpdate, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
 use routing::scoring::{ChannelUsage, Score};
 use util::ser::{Writeable, Readable, Writer};
 use util::logger::{Level, Logger};
@@ -690,16 +690,17 @@ fn default_node_features() -> NodeFeatures {
 /// [`ChannelManager::list_usable_channels`]: crate::ln::channelmanager::ChannelManager::list_usable_channels
 /// [`Event::PaymentPathFailed`]: crate::util::events::Event::PaymentPathFailed
 /// [`NetworkGraph`]: crate::routing::gossip::NetworkGraph
-pub fn find_route<L: Deref, S: Score>(
+pub fn find_route<L: Deref, GL: Deref, S: Score>(
 	our_node_pubkey: &PublicKey, route_params: &RouteParameters,
-	network_graph: &ReadOnlyNetworkGraph, first_hops: Option<&[&ChannelDetails]>, logger: L,
+	network_graph: &NetworkGraph<GL>, first_hops: Option<&[&ChannelDetails]>, logger: L,
 	scorer: &S, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
-where L::Target: Logger {
-	let mut route = get_route(our_node_pubkey, &route_params.payment_params, network_graph, first_hops,
+where L::Target: Logger, GL::Target: Logger {
+	let graph_lock = network_graph.read_only();
+	let mut route = get_route(our_node_pubkey, &route_params.payment_params, &graph_lock, first_hops,
 		route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, scorer,
 		random_seed_bytes)?;
-	add_random_cltv_offset(&mut route, &route_params.payment_params, network_graph, random_seed_bytes);
+	add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
 	Ok(route)
 }
 
@@ -1803,15 +1804,16 @@ fn add_random_cltv_offset(route: &mut Route, payment_params: &PaymentParameters,
 /// exclude the payer, but include the payee). This may be useful, e.g., for probing the chosen path.
 ///
 /// Re-uses logic from `find_route`, so the restrictions described there also apply here.
-pub fn build_route_from_hops<L: Deref>(
+pub fn build_route_from_hops<L: Deref, GL: Deref>(
 	our_node_pubkey: &PublicKey, hops: &[PublicKey], route_params: &RouteParameters,
-	network_graph: &ReadOnlyNetworkGraph, logger: L, random_seed_bytes: &[u8; 32]
+	network_graph: &NetworkGraph<GL>, logger: L, random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
-where L::Target: Logger {
+where L::Target: Logger, GL::Target: Logger {
+	let graph_lock = network_graph.read_only();
 	let mut route = build_route_from_hops_internal(
-		our_node_pubkey, hops, &route_params.payment_params, &network_graph,
+		our_node_pubkey, hops, &route_params.payment_params, &graph_lock,
 		route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, random_seed_bytes)?;
-	add_random_cltv_offset(&mut route, &route_params.payment_params, &network_graph, random_seed_bytes);
+	add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes);
 	Ok(route)
 }
 

lightning/src/routing/scoring.rs

@@ -43,7 +43,7 @@
 //! let scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
 //! # let random_seed_bytes = [42u8; 32];
 //!
-//! let route = find_route(&payer, &route_params, &network_graph.read_only(), None, &logger, &scorer, &random_seed_bytes);
+//! let route = find_route(&payer, &route_params, &network_graph, None, &logger, &scorer, &random_seed_bytes);
 //! # }
 //! ```
 //!