Drop a bunch of unnecessary Arc::clones

Previously, we consistently handed around `Arc` references for most
objects to avoid unnecessary refactoring work. This approach however
introduced a bunch of unnecessary allocations through `Arc::clone`.

Here we opt to rather use plain references in a bunch of places,
reducing the usage of `Arc`s.

Author: tnull

src/builder.rs

@@ -1053,7 +1053,7 @@ fn build_with_store_internal(
 
 	// Initialize the status fields.
 	let node_metrics = match runtime
-		.block_on(async { read_node_metrics(Arc::clone(&kv_store), Arc::clone(&logger)).await })
+		.block_on(async { read_node_metrics(&*kv_store, Arc::clone(&logger)).await })
 	{
 		Ok(metrics) => Arc::new(RwLock::new(metrics)),
 		Err(e) => {
@@ -1068,21 +1068,20 @@ fn build_with_store_internal(
 	let tx_broadcaster = Arc::new(TransactionBroadcaster::new(Arc::clone(&logger)));
 	let fee_estimator = Arc::new(OnchainFeeEstimator::new());
 
-	let payment_store = match runtime
-		.block_on(async { read_payments(Arc::clone(&kv_store), Arc::clone(&logger)).await })
-	{
-		Ok(payments) => Arc::new(PaymentStore::new(
-			payments,
-			PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE.to_string(),
-			PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE.to_string(),
-			Arc::clone(&kv_store),
-			Arc::clone(&logger),
-		)),
-		Err(e) => {
-			log_error!(logger, "Failed to read payment data from store: {}", e);
-			return Err(BuildError::ReadFailed);
-		},
-	};
+	let payment_store =
+		match runtime.block_on(async { read_payments(&*kv_store, Arc::clone(&logger)).await }) {
+			Ok(payments) => Arc::new(PaymentStore::new(
+				payments,
+				PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE.to_string(),
+				PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE.to_string(),
+				Arc::clone(&kv_store),
+				Arc::clone(&logger),
+			)),
+			Err(e) => {
+				log_error!(logger, "Failed to read payment data from store: {}", e);
+				return Err(BuildError::ReadFailed);
+			},
+		};
 
 	let (chain_source, chain_tip_opt) = match chain_data_source_config {
 		Some(ChainDataSourceConfig::Esplora { server_url, headers, sync_config }) => {
@@ -1298,7 +1297,7 @@ fn build_with_store_internal(
 
 	// Initialize the network graph, scorer, and router
 	let network_graph = match runtime
-		.block_on(async { read_network_graph(Arc::clone(&kv_store), Arc::clone(&logger)).await })
+		.block_on(async { read_network_graph(&*kv_store, Arc::clone(&logger)).await })
 	{
 		Ok(graph) => Arc::new(graph),
 		Err(e) => {
@@ -1312,7 +1311,7 @@ fn build_with_store_internal(
 	};
 
 	let local_scorer = match runtime.block_on(async {
-		read_scorer(Arc::clone(&kv_store), Arc::clone(&network_graph), Arc::clone(&logger)).await
+		read_scorer(&*kv_store, Arc::clone(&network_graph), Arc::clone(&logger)).await
 	}) {
 		Ok(scorer) => scorer,
 		Err(e) => {
@@ -1330,8 +1329,7 @@ fn build_with_store_internal(
 
 	// Restore external pathfinding scores from cache if possible.
 	match runtime.block_on(async {
-		read_external_pathfinding_scores_from_cache(Arc::clone(&kv_store), Arc::clone(&logger))
-			.await
+		read_external_pathfinding_scores_from_cache(&*kv_store, Arc::clone(&logger)).await
 	}) {
 		Ok(external_scores) => {
 			scorer.lock().unwrap().merge(external_scores, cur_time);
@@ -1490,15 +1488,11 @@ fn build_with_store_internal(
 			{
 				let mut locked_node_metrics = node_metrics.write().unwrap();
 				locked_node_metrics.latest_rgs_snapshot_timestamp = None;
-				write_node_metrics(
-					&*locked_node_metrics,
-					Arc::clone(&kv_store),
-					Arc::clone(&logger),
-				)
-				.map_err(|e| {
-					log_error!(logger, "Failed writing to store: {}", e);
-					BuildError::WriteFailed
-				})?;
+				write_node_metrics(&*locked_node_metrics, &*kv_store, Arc::clone(&logger))
+					.map_err(|e| {
+						log_error!(logger, "Failed writing to store: {}", e);
+						BuildError::WriteFailed
+					})?;
 			}
 			p2p_source
 		},

src/chain/bitcoind.rs

@@ -205,14 +205,10 @@ impl BitcoindChainSource {
 							unix_time_secs_opt;
 						locked_node_metrics.latest_onchain_wallet_sync_timestamp =
 							unix_time_secs_opt;
-						write_node_metrics(
-							&*locked_node_metrics,
-							Arc::clone(&self.kv_store),
-							Arc::clone(&self.logger),
-						)
-						.unwrap_or_else(|e| {
-							log_error!(self.logger, "Failed to persist node metrics: {}", e);
-						});
+						write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)
+							.unwrap_or_else(|e| {
+								log_error!(self.logger, "Failed to persist node metrics: {}", e);
+							});
 					}
 					break;
 				},
@@ -420,11 +416,11 @@ impl BitcoindChainSource {
 				*self.latest_chain_tip.write().unwrap() = Some(tip);
 
 				periodically_archive_fully_resolved_monitors(
-					Arc::clone(&channel_manager),
-					chain_monitor,
-					Arc::clone(&self.kv_store),
-					Arc::clone(&self.logger),
-					Arc::clone(&self.node_metrics),
+					&*channel_manager,
+					&*chain_monitor,
+					&*self.kv_store,
+					&*self.logger,
+					&*self.node_metrics,
 				)?;
 			},
 			Ok(_) => {},
@@ -469,11 +465,7 @@ impl BitcoindChainSource {
 		locked_node_metrics.latest_lightning_wallet_sync_timestamp = unix_time_secs_opt;
 		locked_node_metrics.latest_onchain_wallet_sync_timestamp = unix_time_secs_opt;
 
-		write_node_metrics(
-			&*locked_node_metrics,
-			Arc::clone(&self.kv_store),
-			Arc::clone(&self.logger),
-		)?;
+		write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
 
 		Ok(())
 	}
@@ -586,11 +578,7 @@ impl BitcoindChainSource {
 		{
 			let mut locked_node_metrics = self.node_metrics.write().unwrap();
 			locked_node_metrics.latest_fee_rate_cache_update_timestamp = unix_time_secs_opt;
-			write_node_metrics(
-				&*locked_node_metrics,
-				Arc::clone(&self.kv_store),
-				Arc::clone(&self.logger),
-			)?;
+			write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
 		}
 
 		Ok(())

src/chain/electrum.rs

@@ -149,8 +149,8 @@ impl ElectrumChainSource {
 								unix_time_secs_opt;
 							write_node_metrics(
 								&*locked_node_metrics,
-								Arc::clone(&self.kv_store),
-								Arc::clone(&self.logger),
+								&*self.kv_store,
+								&*self.logger,
 							)?;
 						}
 						Ok(())
@@ -239,19 +239,15 @@ impl ElectrumChainSource {
 			{
 				let mut locked_node_metrics = self.node_metrics.write().unwrap();
 				locked_node_metrics.latest_lightning_wallet_sync_timestamp = unix_time_secs_opt;
-				write_node_metrics(
-					&*locked_node_metrics,
-					Arc::clone(&self.kv_store),
-					Arc::clone(&self.logger),
-				)?;
+				write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
 			}
 
 			periodically_archive_fully_resolved_monitors(
-				Arc::clone(&channel_manager),
-				Arc::clone(&chain_monitor),
-				Arc::clone(&self.kv_store),
-				Arc::clone(&self.logger),
-				Arc::clone(&self.node_metrics),
+				&*channel_manager,
+				&*chain_monitor,
+				&*self.kv_store,
+				&*self.logger,
+				&*self.node_metrics,
 			)?;
 		}
 
@@ -284,11 +280,7 @@ impl ElectrumChainSource {
 		{
 			let mut locked_node_metrics = self.node_metrics.write().unwrap();
 			locked_node_metrics.latest_fee_rate_cache_update_timestamp = unix_time_secs_opt;
-			write_node_metrics(
-				&*locked_node_metrics,
-				Arc::clone(&self.kv_store),
-				Arc::clone(&self.logger),
-			)?;
+			write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
 		}
 
 		Ok(())

src/chain/esplora.rs

@@ -128,8 +128,8 @@ impl EsploraChainSource {
 										locked_node_metrics.latest_onchain_wallet_sync_timestamp = unix_time_secs_opt;
 										write_node_metrics(
 											&*locked_node_metrics,
-											Arc::clone(&self.kv_store),
-											Arc::clone(&self.logger)
+											&*self.kv_store,
+											&*self.logger
 										)?;
 									}
 									Ok(())
@@ -259,19 +259,15 @@ impl EsploraChainSource {
 						let mut locked_node_metrics = self.node_metrics.write().unwrap();
 						locked_node_metrics.latest_lightning_wallet_sync_timestamp =
 							unix_time_secs_opt;
-						write_node_metrics(
-							&*locked_node_metrics,
-							Arc::clone(&self.kv_store),
-							Arc::clone(&self.logger),
-						)?;
+						write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
 					}
 
 					periodically_archive_fully_resolved_monitors(
-						Arc::clone(&channel_manager),
-						Arc::clone(&chain_monitor),
-						Arc::clone(&self.kv_store),
-						Arc::clone(&self.logger),
-						Arc::clone(&self.node_metrics),
+						&*channel_manager,
+						&*chain_monitor,
+						&*self.kv_store,
+						&*self.logger,
+						&*self.node_metrics,
 					)?;
 					Ok(())
 				},
@@ -353,11 +349,7 @@ impl EsploraChainSource {
 		{
 			let mut locked_node_metrics = self.node_metrics.write().unwrap();
 			locked_node_metrics.latest_fee_rate_cache_update_timestamp = unix_time_secs_opt;
-			write_node_metrics(
-				&*locked_node_metrics,
-				Arc::clone(&self.kv_store),
-				Arc::clone(&self.logger),
-			)?;
+			write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
 		}
 
 		Ok(())

src/chain/mod.rs

@@ -488,8 +488,8 @@ impl Filter for ChainSource {
 }
 
 fn periodically_archive_fully_resolved_monitors(
-	channel_manager: Arc<ChannelManager>, chain_monitor: Arc<ChainMonitor>,
-	kv_store: Arc<DynStore>, logger: Arc<Logger>, node_metrics: Arc<RwLock<NodeMetrics>>,
+	channel_manager: &ChannelManager, chain_monitor: &ChainMonitor, kv_store: &DynStore,
+	logger: &Logger, node_metrics: &RwLock<NodeMetrics>,
 ) -> Result<(), Error> {
 	let mut locked_node_metrics = node_metrics.write().unwrap();
 	let cur_height = channel_manager.current_best_block().height;

src/io/utils.rs

@@ -88,7 +88,7 @@ pub(crate) fn read_or_generate_seed_file(
 
 /// Read a previously persisted [`NetworkGraph`] from the store.
 pub(crate) async fn read_network_graph<L: Deref + Clone>(
-	kv_store: Arc<DynStore>, logger: L,
+	kv_store: &DynStore, logger: L,
 ) -> Result<NetworkGraph<L>, std::io::Error>
 where
 	L::Target: LdkLogger,
@@ -108,7 +108,7 @@ where
 
 /// Read a previously persisted [`ProbabilisticScorer`] from the store.
 pub(crate) async fn read_scorer<G: Deref<Target = NetworkGraph<L>>, L: Deref + Clone>(
-	kv_store: Arc<DynStore>, network_graph: G, logger: L,
+	kv_store: &DynStore, network_graph: G, logger: L,
 ) -> Result<ProbabilisticScorer<G, L>, std::io::Error>
 where
 	L::Target: LdkLogger,
@@ -130,7 +130,7 @@ where
 
 /// Read previously persisted external pathfinding scores from the cache.
 pub(crate) async fn read_external_pathfinding_scores_from_cache<L: Deref>(
-	kv_store: Arc<DynStore>, logger: L,
+	kv_store: &DynStore, logger: L,
 ) -> Result<ChannelLiquidities, std::io::Error>
 where
 	L::Target: LdkLogger,
@@ -150,7 +150,7 @@ where
 
 /// Persist external pathfinding scores to the cache.
 pub(crate) async fn write_external_pathfinding_scores_to_cache<L: Deref>(
-	kv_store: Arc<DynStore>, data: &ChannelLiquidities, logger: L,
+	kv_store: &DynStore, data: &ChannelLiquidities, logger: L,
 ) -> Result<(), Error>
 where
 	L::Target: LdkLogger,
@@ -218,7 +218,7 @@ where
 
 /// Read previously persisted payments information from the store.
 pub(crate) async fn read_payments<L: Deref>(
-	kv_store: Arc<DynStore>, logger: L,
+	kv_store: &DynStore, logger: L,
 ) -> Result<Vec<PaymentDetails>, std::io::Error>
 where
 	L::Target: LdkLogger,
@@ -323,7 +323,7 @@ pub(crate) async fn read_output_sweeper(
 }
 
 pub(crate) async fn read_node_metrics<L: Deref>(
-	kv_store: Arc<DynStore>, logger: L,
+	kv_store: &DynStore, logger: L,
 ) -> Result<NodeMetrics, std::io::Error>
 where
 	L::Target: LdkLogger,
@@ -342,7 +342,7 @@ where
 }
 
 pub(crate) fn write_node_metrics<L: Deref>(
-	node_metrics: &NodeMetrics, kv_store: Arc<DynStore>, logger: L,
+	node_metrics: &NodeMetrics, kv_store: &DynStore, logger: L,
 ) -> Result<(), Error>
 where
 	L::Target: LdkLogger,
@@ -469,7 +469,7 @@ macro_rules! impl_read_write_change_set_type {
 		$key:expr
 	) => {
 		pub(crate) fn $read_name<L: Deref>(
-			kv_store: Arc<DynStore>, logger: L,
+			kv_store: &DynStore, logger: L,
 		) -> Result<Option<$change_set_type>, std::io::Error>
 		where
 			L::Target: LdkLogger,
@@ -510,7 +510,7 @@ macro_rules! impl_read_write_change_set_type {
 		}
 
 		pub(crate) fn $write_name<L: Deref>(
-			value: &$change_set_type, kv_store: Arc<DynStore>, logger: L,
+			value: &$change_set_type, kv_store: &DynStore, logger: L,
 		) -> Result<(), std::io::Error>
 		where
 			L::Target: LdkLogger,
@@ -588,41 +588,35 @@ impl_read_write_change_set_type!(
 
 // Reads the full BdkWalletChangeSet or returns default fields
 pub(crate) fn read_bdk_wallet_change_set(
-	kv_store: Arc<DynStore>, logger: Arc<Logger>,
+	kv_store: &DynStore, logger: &Logger,
 ) -> Result<Option<BdkWalletChangeSet>, std::io::Error> {
 	let mut change_set = BdkWalletChangeSet::default();
 
 	// We require a descriptor and return `None` to signal creation of a new wallet otherwise.
-	if let Some(descriptor) =
-		read_bdk_wallet_descriptor(Arc::clone(&kv_store), Arc::clone(&logger))?
-	{
+	if let Some(descriptor) = read_bdk_wallet_descriptor(kv_store, logger)? {
 		change_set.descriptor = Some(descriptor);
 	} else {
 		return Ok(None);
 	}
 
 	// We require a change_descriptor and return `None` to signal creation of a new wallet otherwise.
-	if let Some(change_descriptor) =
-		read_bdk_wallet_change_descriptor(Arc::clone(&kv_store), Arc::clone(&logger))?
-	{
+	if let Some(change_descriptor) = read_bdk_wallet_change_descriptor(kv_store, logger)? {
 		change_set.change_descriptor = Some(change_descriptor);
 	} else {
 		return Ok(None);
 	}
 
 	// We require a network and return `None` to signal creation of a new wallet otherwise.
-	if let Some(network) = read_bdk_wallet_network(Arc::clone(&kv_store), Arc::clone(&logger))? {
+	if let Some(network) = read_bdk_wallet_network(kv_store, logger)? {
 		change_set.network = Some(network);
 	} else {
 		return Ok(None);
 	}
 
-	read_bdk_wallet_local_chain(Arc::clone(&kv_store), Arc::clone(&logger))?
+	read_bdk_wallet_local_chain(&*kv_store, logger)?
 		.map(|local_chain| change_set.local_chain = local_chain);
-	read_bdk_wallet_tx_graph(Arc::clone(&kv_store), Arc::clone(&logger))?
-		.map(|tx_graph| change_set.tx_graph = tx_graph);
-	read_bdk_wallet_indexer(Arc::clone(&kv_store), Arc::clone(&logger))?
-		.map(|indexer| change_set.indexer = indexer);
+	read_bdk_wallet_tx_graph(&*kv_store, logger)?.map(|tx_graph| change_set.tx_graph = tx_graph);
+	read_bdk_wallet_indexer(&*kv_store, logger)?.map(|indexer| change_set.indexer = indexer);
 	Ok(Some(change_set))
 }