remove mut from ScorerAccountingForInFlightHtlcs scorer input

Author: jbesraa

lightning/src/ln/channelmanager.rs

@@ -10391,7 +10391,7 @@ pub mod bench {
 	use bitcoin::hashes::sha256::Hash as Sha256;
 	use bitcoin::{Block, BlockHeader, PackedLockTime, Transaction, TxMerkleNode, TxOut};
 
-	use crate::sync::{Arc, Mutex};
+	use crate::sync::{Arc, Mutex, RwLock};
 
 	use criterion::Criterion;
 
@@ -10428,7 +10428,7 @@ pub mod bench {
 		let tx_broadcaster = test_utils::TestBroadcaster::new(network);
 		let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 		let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
-		let scorer = Mutex::new(test_utils::TestScorer::new());
+		let scorer = RwLock::new(test_utils::TestScorer::new());
 		let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(network, &logger_a)), &scorer);
 
 		let mut config: UserConfig = Default::default();

lightning/src/ln/functional_test_utils.rs

@@ -43,7 +43,7 @@ use crate::io;
 use crate::prelude::*;
 use core::cell::RefCell;
 use alloc::rc::Rc;
-use crate::sync::{Arc, Mutex, LockTestExt};
+use crate::sync::{Arc, Mutex, LockTestExt, RwLock};
 use core::mem;
 use core::iter::repeat;
 use bitcoin::{PackedLockTime, TxMerkleNode};
@@ -352,7 +352,7 @@ pub struct TestChanMonCfg {
 	pub persister: test_utils::TestPersister,
 	pub logger: test_utils::TestLogger,
 	pub keys_manager: test_utils::TestKeysInterface,
-	pub scorer: Mutex<test_utils::TestScorer>,
+	pub scorer: RwLock<test_utils::TestScorer>,
 }
 
 pub struct NodeCfg<'a> {
@@ -530,7 +530,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 					channel_monitors.insert(monitor.get_funding_txo().0, monitor);
 				}
 
-				let scorer = Mutex::new(test_utils::TestScorer::new());
+				let scorer = RwLock::new(test_utils::TestScorer::new());
 				let mut w = test_utils::TestVecWriter(Vec::new());
 				self.node.write(&mut w).unwrap();
 				<(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>)>::read(&mut io::Cursor::new(w.0), ChannelManagerReadArgs {
@@ -2574,7 +2574,7 @@ pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
 		let persister = test_utils::TestPersister::new();
 		let seed = [i as u8; 32];
 		let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
-		let scorer = Mutex::new(test_utils::TestScorer::new());
+		let scorer = RwLock::new(test_utils::TestScorer::new());
 
 		chan_mon_cfgs.push(TestChanMonCfg { tx_broadcaster, fee_estimator, chain_source, logger, persister, keys_manager, scorer });
 	}

lightning/src/ln/functional_tests.rs

@@ -56,7 +56,7 @@ use alloc::collections::BTreeSet;
 use core::default::Default;
 use core::iter::repeat;
 use bitcoin::hashes::Hash;
-use crate::sync::{Arc, Mutex};
+use crate::sync::{Arc, Mutex, RwLock};
 
 use crate::ln::functional_test_utils::*;
 use crate::ln::chan_utils::CommitmentTransaction;
@@ -5367,7 +5367,7 @@ fn test_key_derivation_params() {
 	let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
 	let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[0].chain_source), &chanmon_cfgs[0].tx_broadcaster, &chanmon_cfgs[0].logger, &chanmon_cfgs[0].fee_estimator, &chanmon_cfgs[0].persister, &keys_manager);
 	let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[0].logger));
-	let scorer = Mutex::new(test_utils::TestScorer::new());
+	let scorer = RwLock::new(test_utils::TestScorer::new());
 	let router = test_utils::TestRouter::new(network_graph.clone(), &scorer);
 	let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) };
 	let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs);

lightning/src/ln/outbound_payment.rs

@@ -1501,7 +1501,7 @@ mod tests {
 	use crate::ln::outbound_payment::{OutboundPayments, Retry, RetryableSendFailure};
 	use crate::routing::gossip::NetworkGraph;
 	use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters};
-	use crate::sync::{Arc, Mutex};
+	use crate::sync::{Arc, Mutex, RwLock};
 	use crate::util::errors::APIError;
 	use crate::util::test_utils;
 
@@ -1537,10 +1537,11 @@ mod tests {
 	}
 	#[cfg(feature = "std")]
 	fn do_fails_paying_after_expiration(on_retry: bool) {
+
 		let outbound_payments = OutboundPayments::new();
 		let logger = test_utils::TestLogger::new();
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
-		let scorer = Mutex::new(test_utils::TestScorer::new());
+		let scorer = RwLock::new(test_utils::TestScorer::new());
 		let router = test_utils::TestRouter::new(network_graph, &scorer);
 		let secp_ctx = Secp256k1::new();
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
@@ -1587,7 +1588,7 @@ mod tests {
 		let outbound_payments = OutboundPayments::new();
 		let logger = test_utils::TestLogger::new();
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
-		let scorer = Mutex::new(test_utils::TestScorer::new());
+		let scorer = RwLock::new(test_utils::TestScorer::new());
 		let router = test_utils::TestRouter::new(network_graph, &scorer);
 		let secp_ctx = Secp256k1::new();
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);
@@ -1629,7 +1630,7 @@ mod tests {
 		let outbound_payments = OutboundPayments::new();
 		let logger = test_utils::TestLogger::new();
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger));
-		let scorer = Mutex::new(test_utils::TestScorer::new());
+		let scorer = RwLock::new(test_utils::TestScorer::new());
 		let router = test_utils::TestRouter::new(network_graph, &scorer);
 		let secp_ctx = Secp256k1::new();
 		let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet);

lightning/src/ln/payment_tests.rs

@@ -2538,7 +2538,7 @@ fn retry_multi_path_single_failed_payment() {
 		}, Ok(route.clone()));
 
 	{
-		let scorer = chanmon_cfgs[0].scorer.lock().unwrap();
+		let scorer = chanmon_cfgs[0].scorer.read().unwrap();
 		// The initial send attempt, 2 paths
 		scorer.expect_usage(chans[0].short_channel_id.unwrap(), ChannelUsage { amount_msat: 10_000, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });
 		scorer.expect_usage(chans[1].short_channel_id.unwrap(), ChannelUsage { amount_msat: 100_000_001, inflight_htlc_msat: 0, effective_capacity: EffectiveCapacity::Unknown });

lightning/src/routing/router.rs

@@ -30,7 +30,7 @@ use crate::prelude::*;
 use crate::sync::Mutex;
 use alloc::collections::BinaryHeap;
 use core::{cmp, fmt};
-use core::ops::{Deref, DerefMut};
+use core::ops::Deref;
 
 /// A [`Router`] implemented using [`find_route`].
 pub struct DefaultRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: Score<ScoreParams = SP>> where
@@ -73,7 +73,7 @@ impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: Sco
 		};
 		find_route(
 			payer, params, &self.network_graph, first_hops, &*self.logger,
-			&ScorerAccountingForInFlightHtlcs::new(self.scorer.write_lock().deref_mut(), &inflight_htlcs),
+			&ScorerAccountingForInFlightHtlcs::new(self.scorer.deref(), &inflight_htlcs),
 			&self.score_params,
 			&random_seed_bytes
 		)
@@ -112,15 +112,14 @@ pub trait Router {
 /// [`find_route`].
 ///
 /// [`Score`]: crate::routing::scoring::Score
-pub struct ScorerAccountingForInFlightHtlcs<'a, S: Score<ScoreParams = SP>, SP: Sized> {
-	scorer: &'a mut S,
+pub struct ScorerAccountingForInFlightHtlcs<'a, SP: Sized, Sc: 'a + Score<ScoreParams = SP>, S: LockableScore<'a, Score = Sc> + ?Sized> {
+	scorer: &'a S,
 	// Maps a channel's short channel id and its direction to the liquidity used up.
 	inflight_htlcs: &'a InFlightHtlcs,
 }
-
-impl<'a, S: Score<ScoreParams = SP>, SP: Sized> ScorerAccountingForInFlightHtlcs<'a, S, SP> {
+impl<'a, SP: Sized, Sc: Score<ScoreParams = SP>, S: LockableScore<'a, Score = Sc> + ?Sized> ScorerAccountingForInFlightHtlcs<'a, SP, Sc, S> {
 	/// Initialize a new `ScorerAccountingForInFlightHtlcs`.
-	pub fn new(scorer: &'a mut S, inflight_htlcs: &'a InFlightHtlcs) -> Self {
+	pub fn new(scorer: &'a S, inflight_htlcs: &'a InFlightHtlcs) -> Self {
 		ScorerAccountingForInFlightHtlcs {
 			scorer,
 			inflight_htlcs
@@ -129,12 +128,12 @@ impl<'a, S: Score<ScoreParams = SP>, SP: Sized> ScorerAccountingForInFlightHtlcs
 }
 
 #[cfg(c_bindings)]
-impl<'a, S: Score<ScoreParams = SP>, SP: Sized> Writeable for ScorerAccountingForInFlightHtlcs<'a, S, SP> {
-	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> { self.scorer.write(writer) }
+impl<'a, SP: Sized, Sc: Score<ScoreParams = SP>, S: LockableScore<'a, Score = Sc> + ?Sized> Writeable for ScorerAccountingForInFlightHtlcs<'a, SP, Sc, S> {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> { self.scorer.write_lock().write(writer) }
 }
 
-impl<'a, S: Score<ScoreParams = SP>, SP: Sized> Score for ScorerAccountingForInFlightHtlcs<'a, S, SP>  {
-	type ScoreParams = S::ScoreParams;
+impl<'a, SP: Sized, Sc: 'a + Score<ScoreParams = SP>, S: LockableScore<'a, Score = Sc> + ?Sized> Score for ScorerAccountingForInFlightHtlcs<'a, SP, Sc, S> {
+	type ScoreParams = Sc::ScoreParams;
 	fn channel_penalty_msat(&self, short_channel_id: u64, source: &NodeId, target: &NodeId, usage: ChannelUsage, score_params: &Self::ScoreParams) -> u64 {
 		if let Some(used_liquidity) = self.inflight_htlcs.used_liquidity_msat(
 			source, target, short_channel_id
@@ -144,26 +143,26 @@ impl<'a, S: Score<ScoreParams = SP>, SP: Sized> Score for ScorerAccountingForInF
 				..usage
 			};
 
-			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
+			self.scorer.read_lock().channel_penalty_msat(short_channel_id, source, target, usage, score_params)
 		} else {
-			self.scorer.channel_penalty_msat(short_channel_id, source, target, usage, score_params)
+			self.scorer.read_lock().channel_penalty_msat(short_channel_id, source, target, usage, score_params)
 		}
 	}
 
 	fn payment_path_failed(&mut self, path: &Path, short_channel_id: u64) {
-		self.scorer.payment_path_failed(path, short_channel_id)
+		self.scorer.write_lock().payment_path_failed(path, short_channel_id)
 	}
 
 	fn payment_path_successful(&mut self, path: &Path) {
-		self.scorer.payment_path_successful(path)
+		self.scorer.write_lock().payment_path_successful(path)
 	}
 
 	fn probe_failed(&mut self, path: &Path, short_channel_id: u64) {
-		self.scorer.probe_failed(path, short_channel_id)
+		self.scorer.write_lock().probe_failed(path, short_channel_id)
 	}
 
 	fn probe_successful(&mut self, path: &Path) {
-		self.scorer.probe_successful(path)
+		self.scorer.write_lock().probe_successful(path)
 	}
 }
 

lightning/src/routing/scoring.rs

@@ -235,8 +235,6 @@ pub struct MultiThreadedLockableScore<T: Score> {
 #[cfg(c_bindings)]
 impl<'a, T: 'a + Score> LockableScore<'a> for MultiThreadedLockableScore<T> {
 	type Score = T;
-	// type ReadLocked = MultiThreadedScoreLock<'a, T>;
-	// type WriteLocked = Rw<'a, T>;
 	type WriteLocked = RwLockWriteGuard<'a, T>;
 	type ReadLocked = RwLockReadGuard<'a, T>;
 

lightning/src/util/test_utils.rs

@@ -28,6 +28,7 @@ use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId};
 use crate::routing::utxo::{UtxoLookup, UtxoLookupError, UtxoResult};
 use crate::routing::router::{find_route, InFlightHtlcs, Path, Route, RouteParameters, Router, ScorerAccountingForInFlightHtlcs};
 use crate::routing::scoring::{ChannelUsage, Score};
+use crate::sync::RwLock;
 use crate::util::config::UserConfig;
 use crate::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
 use crate::util::logger::{Logger, Level, Record};
@@ -54,7 +55,7 @@ use regex;
 use crate::io;
 use crate::prelude::*;
 use core::cell::RefCell;
-use core::ops::DerefMut;
+use core::ops::Deref;
 use core::time::Duration;
 use crate::sync::{Mutex, Arc};
 use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
@@ -95,11 +96,11 @@ impl chaininterface::FeeEstimator for TestFeeEstimator {
 pub struct TestRouter<'a> {
 	pub network_graph: Arc<NetworkGraph<&'a TestLogger>>,
 	pub next_routes: Mutex<VecDeque<(RouteParameters, Result<Route, LightningError>)>>,
-	pub scorer: &'a Mutex<TestScorer>,
+	pub scorer: &'a RwLock<TestScorer>,
 }
 
 impl<'a> TestRouter<'a> {
-	pub fn new(network_graph: Arc<NetworkGraph<&'a TestLogger>>, scorer: &'a Mutex<TestScorer>) -> Self {
+	pub fn new(network_graph: Arc<NetworkGraph<&'a TestLogger>>, scorer: &'a RwLock<TestScorer>) -> Self {
 		Self { network_graph, next_routes: Mutex::new(VecDeque::new()), scorer }
 	}
 
@@ -117,8 +118,7 @@ impl<'a> Router for TestRouter<'a> {
 		if let Some((find_route_query, find_route_res)) = self.next_routes.lock().unwrap().pop_front() {
 			assert_eq!(find_route_query, *params);
 			if let Ok(ref route) = find_route_res {
-				let mut binding = self.scorer.lock().unwrap();
-				let scorer = ScorerAccountingForInFlightHtlcs::new(binding.deref_mut(), &inflight_htlcs);
+				let scorer = ScorerAccountingForInFlightHtlcs::new(self.scorer.deref(), &inflight_htlcs);
 				for path in &route.paths {
 					let mut aggregate_msat = 0u64;
 					for (idx, hop) in path.hops.iter().rev().enumerate() {
@@ -145,7 +145,7 @@ impl<'a> Router for TestRouter<'a> {
 		let logger = TestLogger::new();
 		find_route(
 			payer, params, &self.network_graph, first_hops, &logger,
-			&ScorerAccountingForInFlightHtlcs::new(self.scorer.lock().unwrap().deref_mut(), &inflight_htlcs), &(),
+			&ScorerAccountingForInFlightHtlcs::new(self.scorer.deref(), &inflight_htlcs), &(),
 			&[42; 32]
 		)
 	}