Remove RwLock from around NetworkGraph

Now that NetworkGraph uses interior mutability, the RwLock used around
it in NetGraphMsgHandler is no longer needed. This allows for shared
ownership without a lock.

Author: jkczyz

fuzz/src/full_stack.rs

@@ -427,7 +427,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 			},
 			4 => {
 				let value = slice_to_be24(get_slice!(3)) as u64;
-				let route = match get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger)) {
+				let route = match get_route(&our_id, &net_graph_msg_handler.network_graph, &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger)) {
 					Ok(route) => route,
 					Err(_) => return,
 				};
@@ -444,7 +444,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 			},
 			15 => {
 				let value = slice_to_be24(get_slice!(3)) as u64;
-				let mut route = match get_route(&our_id, &net_graph_msg_handler.network_graph.read().unwrap(), &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger)) {
+				let mut route = match get_route(&our_id, &net_graph_msg_handler.network_graph, &get_pubkey!(), None, None, &Vec::new(), value, 42, Arc::clone(&logger)) {
 					Ok(route) => route,
 					Err(_) => return,
 				};

fuzz/src/router.rs

@@ -160,7 +160,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new("".to_owned(), out));
 
 	let our_pubkey = get_pubkey!();
-	let mut net_graph = NetworkGraph::new(genesis_block(Network::Bitcoin).header.block_hash());
+	let net_graph = NetworkGraph::new(genesis_block(Network::Bitcoin).header.block_hash());
 
 	let mut node_pks = HashSet::new();
 	let mut scid = 42;

lightning-invoice/src/utils.rs

@@ -115,11 +115,11 @@ mod test {
 		let amt_msat = invoice.amount_pico_btc().unwrap() / 10;
 		let first_hops = nodes[0].node.list_usable_channels();
 		let last_hops = invoice.route_hints();
-		let network_graph = nodes[0].net_graph_msg_handler.network_graph.read().unwrap();
+		let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
 		let logger = test_utils::TestLogger::new();
 		let route = router::get_route(
 			&nodes[0].node.get_our_node_id(),
-			&network_graph,
+			network_graph,
 			&invoice.recover_payee_pub_key(),
 			Some(invoice.features().unwrap().clone()),
 			Some(&first_hops.iter().collect::<Vec<_>>()),

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -5416,7 +5416,7 @@ mod tests {
 
 		// First, send a partial MPP payment.
 		let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
-		let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+		let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
 		let (payment_preimage, our_payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[1]);
 		// Use the utility function send_payment_along_path to send the payment with MPP data which
 		// indicates there are more HTLCs coming.
@@ -5523,7 +5523,7 @@ mod tests {
 		let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &expected_route, 100_000);
 
 		// Next, attempt a keysend payment and make sure it fails.
-		let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph.read().unwrap(), &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+		let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
 		nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
 		check_added_monitors!(nodes[0], 1);
 		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -5551,7 +5551,7 @@ mod tests {
 
 		// To start (2), send a keysend payment but don't claim it.
 		let payment_preimage = PaymentPreimage([42; 32]);
-		let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph.read().unwrap(), &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
+		let route = get_route(&nodes[0].node.get_our_node_id(), &nodes[0].net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap();
 		let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage)).unwrap();
 		check_added_monitors!(nodes[0], 1);
 		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
@@ -5603,9 +5603,9 @@ mod tests {
 		nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() });
 
 		let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
-		let network_graph = nodes[0].net_graph_msg_handler.network_graph.read().unwrap();
+		let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
 		let first_hops = nodes[0].node.list_usable_channels();
-		let route = get_keysend_route(&payer_pubkey, &network_graph, &payee_pubkey,
+		let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey,
                                   Some(&first_hops.iter().collect::<Vec<_>>()), &vec![], 10000, 40,
                                   nodes[0].logger).unwrap();
 
@@ -5639,9 +5639,9 @@ mod tests {
 		nodes[1].node.peer_connected(&payer_pubkey, &msgs::Init { features: InitFeatures::known() });
 
 		let _chan = create_chan_between_nodes(&nodes[0], &nodes[1], InitFeatures::known(), InitFeatures::known());
-		let network_graph = nodes[0].net_graph_msg_handler.network_graph.read().unwrap();
+		let network_graph = &nodes[0].net_graph_msg_handler.network_graph;
 		let first_hops = nodes[0].node.list_usable_channels();
-		let route = get_keysend_route(&payer_pubkey, &network_graph, &payee_pubkey,
+		let route = get_keysend_route(&payer_pubkey, network_graph, &payee_pubkey,
                                   Some(&first_hops.iter().collect::<Vec<_>>()), &vec![], 10000, 40,
                                   nodes[0].logger).unwrap();
 
@@ -5678,7 +5678,7 @@ mod tests {
 		// Marshall an MPP route.
 		let (_, payment_hash, _) = get_payment_preimage_hash!(&nodes[3]);
 		let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
-		let mut route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[3].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
+		let mut route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[3].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &[], 100000, TEST_FINAL_CLTV, &logger).unwrap();
 		let path = route.paths[0].clone();
 		route.paths.push(path);
 		route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();

lightning/src/ln/channelmanager.rs

@@ -239,10 +239,10 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 			// Check that if we serialize the Router, we can deserialize it again.
 			{
 				let mut w = test_utils::TestVecWriter(Vec::new());
-				let network_graph_ser = self.net_graph_msg_handler.network_graph.read().unwrap();
+				let network_graph_ser = &self.net_graph_msg_handler.network_graph;
 				network_graph_ser.write(&mut w).unwrap();
 				let network_graph_deser = <NetworkGraph>::read(&mut io::Cursor::new(&w.0)).unwrap();
-				assert!(network_graph_deser == *self.net_graph_msg_handler.network_graph.read().unwrap());
+				assert!(network_graph_deser == self.net_graph_msg_handler.network_graph);
 				let net_graph_msg_handler = NetGraphMsgHandler::from_net_graph(
 					Some(self.chain_source), self.logger, network_graph_deser
 				);
@@ -948,7 +948,7 @@ macro_rules! get_route_and_payment_hash {
 		let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!($recv_node);
 		let net_graph_msg_handler = &$send_node.net_graph_msg_handler;
 		let route = get_route(&$send_node.node.get_our_node_id(),
-			&net_graph_msg_handler.network_graph.read().unwrap(),
+			&net_graph_msg_handler.network_graph,
 			&$recv_node.node.get_our_node_id(), None, None, &Vec::new(), $recv_value, TEST_FINAL_CLTV, $send_node.logger).unwrap();
 		(route, payment_hash, payment_preimage, payment_secret)
 	}}
@@ -1237,7 +1237,7 @@ pub const TEST_FINAL_CLTV: u32 = 70;
 pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
 	let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
 	let logger = test_utils::TestLogger::new();
-	let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(),
+	let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph,
 		&expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()),
 		Some(&origin_node.node.list_usable_channels().iter().collect::<Vec<_>>()), &[],
 		recv_value, TEST_FINAL_CLTV, &logger).unwrap();
@@ -1253,7 +1253,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
 	let logger = test_utils::TestLogger::new();
 	let net_graph_msg_handler = &origin_node.net_graph_msg_handler;
-	let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), recv_value, TEST_FINAL_CLTV, &logger).unwrap();
+	let route = get_route(&origin_node.node.get_our_node_id(), &net_graph_msg_handler.network_graph, &expected_route.last().unwrap().node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), recv_value, TEST_FINAL_CLTV, &logger).unwrap();
 	assert_eq!(route.paths.len(), 1);
 	assert_eq!(route.paths[0].len(), expected_route.len());
 	for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {