Parameterize NetworkGraph with Logger

P2PGossipSync logs before delegating to NetworkGraph in its
EventHandler. In order to share this handling with RapidGossipSync,
NetworkGraph needs to take a logger so that it can implement
EventHandler instead.

Author: jkczyz

fuzz/src/full_stack.rs

@@ -163,7 +163,7 @@ type ChannelMan = ChannelManager<
 	EnforcingSigner,
 	Arc<chainmonitor::ChainMonitor<EnforcingSigner, Arc<dyn chain::Filter>, Arc<TestBroadcaster>, Arc<FuzzEstimator>, Arc<dyn Logger>, Arc<TestPersister>>>,
 	Arc<TestBroadcaster>, Arc<KeyProvider>, Arc<FuzzEstimator>, Arc<dyn Logger>>;
-type PeerMan<'a> = PeerManager<Peer<'a>, Arc<ChannelMan>, Arc<P2PGossipSync<Arc<NetworkGraph>, Arc<dyn chain::Access>, Arc<dyn Logger>>>, Arc<dyn Logger>, IgnoringMessageHandler>;
+type PeerMan<'a> = PeerManager<Peer<'a>, Arc<ChannelMan>, Arc<P2PGossipSync<Arc<NetworkGraph<Arc<dyn Logger>>>, Arc<dyn chain::Access>, Arc<dyn Logger>>>, Arc<dyn Logger>, IgnoringMessageHandler>;
 
 struct MoneyLossDetector<'a> {
 	manager: Arc<ChannelMan>,
@@ -395,7 +395,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 	// it's easier to just increment the counter here so the keys don't change.
 	keys_manager.counter.fetch_sub(1, Ordering::AcqRel);
 	let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap());
-	let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash()));
+	let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash(), Arc::clone(&logger)));
 	let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger)));
 	let scorer = FixedPenaltyScorer::with_penalty(0);
 
@@ -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, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+				let route = match find_route(&our_id, &params, &network_graph.read_only(), 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, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+				let mut route = match find_route(&our_id, &params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
 					Ok(route) => route,
 					Err(_) => return,
 				};

fuzz/src/process_network_graph.rs

@@ -1,22 +1,27 @@
 // Imports that need to be added manually
+use lightning::util::logger::Logger;
 use lightning_rapid_gossip_sync::RapidGossipSync;
+
 use utils::test_logger;
 
+use std::sync::Arc;
+
 /// Actual fuzz test, method signature and name are fixed
-fn do_test(data: &[u8]) {
+fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let block_hash = bitcoin::BlockHash::default();
-	let network_graph = lightning::routing::gossip::NetworkGraph::new(block_hash);
+	let logger: Arc<dyn Logger> = Arc::new(test_logger::TestLogger::new("".to_owned(), out));
+	let network_graph = lightning::routing::gossip::NetworkGraph::new(block_hash, logger);
 	let rapid_sync = RapidGossipSync::new(&network_graph);
 	let _ = rapid_sync.update_network_graph(data);
 }
 
 /// Method that needs to be added manually, {name}_test
-pub fn process_network_graph_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	do_test(data);
+pub fn process_network_graph_test<Out: test_logger::Output>(data: &[u8], out: Out) {
+	do_test(data, out);
 }
 
 /// Method that needs to be added manually, {name}_run
 #[no_mangle]
 pub extern "C" fn process_network_graph_run(data: *const u8, datalen: usize) {
-	do_test(unsafe { std::slice::from_raw_parts(data, datalen) });
+	do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, test_logger::DevNull {});
 }

fuzz/src/router.rs

@@ -162,7 +162,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 net_graph = NetworkGraph::new(genesis_block(Network::Bitcoin).header.block_hash());
+	let net_graph = NetworkGraph::new(genesis_block(Network::Bitcoin).header.block_hash(), Arc::clone(&logger));
 
 	let mut node_pks = HashSet::new();
 	let mut scid = 42;
@@ -267,7 +267,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,
+					let _ = find_route(&our_pubkey, &route_params, &net_graph.read_only(),
 						first_hops.map(|c| c.iter().collect::<Vec<_>>()).as_ref().map(|a| a.as_slice()),
 						Arc::clone(&logger), &scorer, &random_seed_bytes);
 				}

lightning-background-processor/src/lib.rs

@@ -94,7 +94,7 @@ const FIRST_NETWORK_PRUNE_TIMER: u64 = 1;
 struct DecoratingEventHandler<
 	E: EventHandler,
 	P: Deref<Target = P2PGossipSync<G, A, L>>,
-	G: Deref<Target = NetworkGraph>,
+	G: Deref<Target = NetworkGraph<L>>,
 	A: Deref,
 	L: Deref,
 >
@@ -106,7 +106,7 @@ where A::Target: chain::Access, L::Target: Logger {
 impl<
 	E: EventHandler,
 	P: Deref<Target = P2PGossipSync<G, A, L>>,
-	G: Deref<Target = NetworkGraph>,
+	G: Deref<Target = NetworkGraph<L>>,
 	A: Deref,
 	L: Deref,
 > EventHandler for DecoratingEventHandler<E, P, G, A, L>
@@ -173,7 +173,7 @@ impl BackgroundProcessor {
 		T: 'static + Deref + Send + Sync,
 		K: 'static + Deref + Send + Sync,
 		F: 'static + Deref + Send + Sync,
-		G: 'static + Deref<Target = NetworkGraph> + Send + Sync,
+		G: 'static + Deref<Target = NetworkGraph<L>> + Send + Sync,
 		L: 'static + Deref + Send + Sync,
 		P: 'static + Deref + Send + Sync,
 		Descriptor: 'static + SocketDescriptor + Send + Sync,
@@ -188,7 +188,7 @@ impl BackgroundProcessor {
 		PM: 'static + Deref<Target = PeerManager<Descriptor, CMH, RMH, L, UMH>> + Send + Sync,
 		S: 'static + Deref<Target = SC> + Send + Sync,
 		SC: WriteableScore<'a>,
-		RGS: 'static + Deref<Target = RapidGossipSync<G>> + Send
+		RGS: 'static + Deref<Target = RapidGossipSync<G, L>> + Send
 	>(
 		persister: PS, event_handler: EH, chain_monitor: M, channel_manager: CM,
 		p2p_gossip_sync: Option<PGS>, peer_manager: PM, logger: L, scorer: Option<S>,
@@ -442,16 +442,16 @@ mod tests {
 
 	struct Node {
 		node: Arc<SimpleArcChannelManager<ChainMonitor, test_utils::TestBroadcaster, test_utils::TestFeeEstimator, test_utils::TestLogger>>,
-		p2p_gossip_sync: Option<Arc<P2PGossipSync<Arc<NetworkGraph>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>>>,
+		p2p_gossip_sync: Option<Arc<P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>>>,
 		peer_manager: Arc<PeerManager<TestDescriptor, Arc<test_utils::TestChannelMessageHandler>, Arc<test_utils::TestRoutingMessageHandler>, Arc<test_utils::TestLogger>, IgnoringMessageHandler>>,
 		chain_monitor: Arc<ChainMonitor>,
 		persister: Arc<FilesystemPersister>,
 		tx_broadcaster: Arc<test_utils::TestBroadcaster>,
-		network_graph: Arc<NetworkGraph>,
+		network_graph: Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
 		logger: Arc<test_utils::TestLogger>,
 		best_block: BestBlock,
 		scorer: Arc<Mutex<FixedPenaltyScorer>>,
-		rapid_gossip_sync: Option<Arc<RapidGossipSync<Arc<NetworkGraph>>>>
+		rapid_gossip_sync: Option<Arc<RapidGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestLogger>>>>,
 	}
 
 	impl Drop for Node {
@@ -546,7 +546,7 @@ mod tests {
 			let best_block = BestBlock::from_genesis(network);
 			let params = ChainParameters { network, best_block };
 			let manager = Arc::new(ChannelManager::new(fee_estimator.clone(), chain_monitor.clone(), tx_broadcaster.clone(), logger.clone(), keys_manager.clone(), UserConfig::default(), params));
-			let network_graph = Arc::new(NetworkGraph::new(genesis_block.header.block_hash()));
+			let network_graph = Arc::new(NetworkGraph::new(genesis_block.header.block_hash(), logger.clone()));
 			let p2p_gossip_sync = Some(Arc::new(P2PGossipSync::new(network_graph.clone(), Some(chain_source.clone()), logger.clone())));
 			let msg_handler = MessageHandler { chan_handler: Arc::new(test_utils::TestChannelMessageHandler::new()), route_handler: Arc::new(test_utils::TestRoutingMessageHandler::new() )};
 			let peer_manager = Arc::new(PeerManager::new(msg_handler, keys_manager.get_node_secret(Recipient::Node).unwrap(), &seed, logger.clone(), IgnoringMessageHandler{}));

lightning-invoice/src/utils.rs

@@ -440,13 +440,13 @@ fn filter_channels(channels: Vec<ChannelDetails>, min_inbound_capacity_msat: Opt
 }
 
 /// A [`Router`] implemented using [`find_route`].
-pub struct DefaultRouter<G: Deref<Target = NetworkGraph>, L: Deref> where L::Target: Logger {
+pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref> where L::Target: Logger {
 	network_graph: G,
 	logger: L,
 	random_seed_bytes: Mutex<[u8; 32]>,
 }
 
-impl<G: Deref<Target = NetworkGraph>, L: Deref> DefaultRouter<G, L> where L::Target: Logger {
+impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> DefaultRouter<G, L> where L::Target: Logger {
 	/// Creates a new router using the given [`NetworkGraph`], a [`Logger`], and a randomness source
 	/// `random_seed_bytes`.
 	pub fn new(network_graph: G, logger: L, random_seed_bytes: [u8; 32]) -> Self {
@@ -455,18 +455,19 @@ impl<G: Deref<Target = NetworkGraph>, L: Deref> DefaultRouter<G, L> where L::Tar
 	}
 }
 
-impl<G: Deref<Target = NetworkGraph>, L: Deref, S: Score> Router<S> for DefaultRouter<G, L>
+impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Score> Router<S> for DefaultRouter<G, L>
 where L::Target: Logger {
 	fn find_route(
 		&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, &*self.network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
+		find_route(payer, params, &network_graph, first_hops, &*self.logger, scorer, &random_seed_bytes)
 	}
 }
 
@@ -566,12 +567,12 @@ mod test {
 			final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32,
 		};
 		let first_hops = nodes[0].node.list_usable_channels();
-		let network_graph = node_cfgs[0].network_graph;
+		let network_graph = &node_cfgs[0].network_graph;
 		let logger = test_utils::TestLogger::new();
 		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,
+			&nodes[0].node.get_our_node_id(), &route_params, &network_graph.read_only(),
 			Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
 		).unwrap();
 
@@ -842,12 +843,12 @@ mod test {
 			final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32,
 		};
 		let first_hops = nodes[0].node.list_usable_channels();
-		let network_graph = node_cfgs[0].network_graph;
+		let network_graph = &node_cfgs[0].network_graph;
 		let logger = test_utils::TestLogger::new();
 		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,
+			&nodes[0].node.get_our_node_id(), &params, &network_graph.read_only(),
 			Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
 		).unwrap();
 		let (payment_event, fwd_idx) = {

lightning-rapid-gossip-sync/src/lib.rs

@@ -30,10 +30,12 @@
 //! use bitcoin::blockdata::constants::genesis_block;
 //! use bitcoin::Network;
 //! use lightning::routing::gossip::NetworkGraph;
+//! use lightning::util::test_utils::TestLogger;
 //! use lightning_rapid_gossip_sync::RapidGossipSync;
 //!
 //! let block_hash = genesis_block(Network::Bitcoin).header.block_hash();
-//! let network_graph = NetworkGraph::new(block_hash);
+//! let logger = TestLogger::new();
+//! let network_graph = NetworkGraph::new(block_hash, &logger);
 //! let rapid_sync = RapidGossipSync::new(&network_graph);
 //! let new_last_sync_timestamp_result = rapid_sync.sync_network_graph_with_file_path("./rapid_sync.lngossip");
 //! ```
@@ -63,6 +65,7 @@ use std::ops::Deref;
 use std::sync::atomic::{AtomicBool, Ordering};
 
 use lightning::routing::gossip::NetworkGraph;
+use lightning::util::logger::Logger;
 
 use crate::error::GraphSyncError;
 
@@ -76,12 +79,13 @@ pub mod processing;
 /// See [crate-level documentation] for usage.
 ///
 /// [crate-level documentation]: crate
-pub struct RapidGossipSync<NG: Deref<Target=NetworkGraph>> {
+pub struct RapidGossipSync<NG: Deref<Target=NetworkGraph<L>>, L: Deref>
+where L::Target: Logger {
 	network_graph: NG,
 	is_initial_sync_complete: AtomicBool
 }
 
-impl<NG: Deref<Target=NetworkGraph>> RapidGossipSync<NG> {
+impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L::Target: Logger {
 	/// Instantiate a new [`RapidGossipSync`] instance
 	pub fn new(network_graph: NG) -> Self {
 		Self {
@@ -128,6 +132,7 @@ mod tests {
 
 	use lightning::ln::msgs::DecodeError;
 	use lightning::routing::gossip::NetworkGraph;
+	use lightning::util::test_utils::TestLogger;
 	use crate::RapidGossipSync;
 
 	#[test]
@@ -187,7 +192,8 @@ mod tests {
 		let graph_sync_test_file = sync_test.get_test_file_path();
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
@@ -219,7 +225,8 @@ mod tests {
 	#[test]
 	fn measure_native_read_from_file() {
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
@@ -254,12 +261,14 @@ pub mod bench {
 
 	use lightning::ln::msgs::DecodeError;
 	use lightning::routing::gossip::NetworkGraph;
+	use lightning::util::test_utils::TestLogger;
 
 	use crate::RapidGossipSync;
 
 	#[bench]
 	fn bench_reading_full_graph_from_file(b: &mut Bencher) {
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
+		let logger = TestLogger::new();
 		b.iter(|| {
 			let network_graph = NetworkGraph::new(block_hash);
 			let rapid_sync = RapidGossipSync::new(&network_graph);

lightning-rapid-gossip-sync/src/processing.rs

@@ -11,6 +11,7 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, OptionalField, UnsignedChannelUpdate,
 };
 use lightning::routing::gossip::NetworkGraph;
+use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, Readable};
 
 use crate::error::GraphSyncError;
@@ -26,7 +27,7 @@ const GOSSIP_PREFIX: [u8; 4] = [76, 68, 75, 1];
 /// avoid malicious updates being able to trigger excessive memory allocation.
 const MAX_INITIAL_NODE_ID_VECTOR_CAPACITY: u32 = 50_000;
 
-impl<NG: Deref<Target=NetworkGraph>> RapidGossipSync<NG> {
+impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L::Target: Logger {
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
@@ -236,14 +237,16 @@ mod tests {
 
 	use lightning::ln::msgs::DecodeError;
 	use lightning::routing::gossip::NetworkGraph;
+	use lightning::util::test_utils::TestLogger;
 
 	use crate::error::GraphSyncError;
 	use crate::RapidGossipSync;
 
 	#[test]
 	fn network_graph_fails_to_update_from_clipped_input() {
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		let example_input = vec![
 			76, 68, 75, 1, 111, 226, 140, 10, 182, 241, 179, 114, 193, 166, 162, 70, 174, 99, 247,
@@ -282,7 +285,8 @@ mod tests {
 		];
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
@@ -315,7 +319,8 @@ mod tests {
 		];
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
@@ -351,7 +356,8 @@ mod tests {
 		];
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
@@ -417,7 +423,8 @@ mod tests {
 		];
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);
 
@@ -476,7 +483,8 @@ mod tests {
 		];
 
 		let block_hash = genesis_block(Network::Bitcoin).block_hash();
-		let network_graph = NetworkGraph::new(block_hash);
+		let logger = TestLogger::new();
+		let network_graph = NetworkGraph::new(block_hash, &logger);
 
 		assert_eq!(network_graph.read_only().channels().len(), 0);