Implement Score's FeeParams as a passed-in parameter on RouteFinding functions

henghonglee · henghonglee · commit c3e37649aa5d · 2023-05-08T17:49:36.000-07:00
This PR aims to create a "stateless" scorer. Instead of passing in fee params
at construction-time, we want to parametrize the scorer with an associated "parameter"
type, which is then passed to the router function itself, and allows passing different
parameters per route-finding call.
diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
@@ -48,7 +48,7 @@ use lightning::util::config::UserConfig;
 use lightning::util::errors::APIError;
 use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
 use lightning::util::logger::Logger;
-use lightning::util::ser::{Readable, ReadableArgs, Writeable};
+use lightning::util::ser::{ReadableArgs, Writeable};
 
 use crate::utils::test_logger;
 use crate::utils::test_persister::TestPersister;
diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
@@ -17,7 +17,7 @@ use lightning::ln::msgs;
 use lightning::routing::gossip::{NetworkGraph, RoutingFees};
 use lightning::routing::utxo::{UtxoFuture, UtxoLookup, UtxoLookupError, UtxoResult};
 use lightning::routing::router::{find_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters};
-use lightning::routing::scoring::ProbabilisticScorer;
+use lightning::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters};
 use lightning::util::config::UserConfig;
 use lightning::util::ser::Readable;
 
@@ -293,7 +293,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 						}]));
 					}
 				}
-				let scorer = ProbabilisticScorer::new(Default::default(), &net_graph, &logger);
+				let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &net_graph, &logger);
 				let random_seed_bytes: [u8; 32] = [get_slice!(1)[0]; 32];
 				for target in node_pks.iter() {
 					let final_value_msat = slice_to_be64(get_slice!(8));
@@ -305,7 +305,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 					};
 					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);
+						&logger, &scorer, &ProbabilisticScoringFeeParameters::default(), &random_seed_bytes);
 				}
 			},
 		}
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
@@ -866,7 +866,7 @@ mod tests {
 		fn disconnect_socket(&mut self) {}
 	}
 
-	type ChannelManager = channelmanager::ChannelManager<Arc<ChainMonitor>, Arc<test_utils::TestBroadcaster>, Arc<KeysManager>, Arc<KeysManager>, Arc<KeysManager>, Arc<test_utils::TestFeeEstimator>, Arc<DefaultRouter< Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestLogger>, Arc<Mutex<TestScorer>>>>, Arc<test_utils::TestLogger>>;
+	type ChannelManager = channelmanager::ChannelManager<Arc<ChainMonitor>, Arc<test_utils::TestBroadcaster>, Arc<KeysManager>, Arc<KeysManager>, Arc<KeysManager>, Arc<test_utils::TestFeeEstimator>, Arc<DefaultRouter<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestLogger>, Arc<Mutex<TestScorer>>, (), TestScorer>>, Arc<test_utils::TestLogger>>;
 
 	type ChainMonitor = chainmonitor::ChainMonitor<InMemorySigner, Arc<test_utils::TestChainSource>, Arc<test_utils::TestBroadcaster>, Arc<test_utils::TestFeeEstimator>, Arc<test_utils::TestLogger>, Arc<FilesystemPersister>>;
 
@@ -1000,8 +1000,9 @@ mod tests {
 	}
 
 	impl Score for TestScorer {
+		type ScoreParams = ();
 		fn channel_penalty_msat(
-			&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage
+			&self, _short_channel_id: u64, _source: &NodeId, _target: &NodeId, _usage: ChannelUsage, _score_params: &Self::ScoreParams
 		) -> u64 { unimplemented!(); }
 
 		fn payment_path_failed(&mut self, actual_path: &Path, actual_short_channel_id: u64) {
@@ -1114,7 +1115,7 @@ mod tests {
 			let network_graph = Arc::new(NetworkGraph::new(network, logger.clone()));
 			let scorer = Arc::new(Mutex::new(TestScorer::new()));
 			let seed = [i as u8; 32];
-			let router = Arc::new(DefaultRouter::new(network_graph.clone(), logger.clone(), seed, scorer.clone()));
+			let router = Arc::new(DefaultRouter::new(network_graph.clone(), logger.clone(), seed, scorer.clone(), ()));
 			let chain_source = Arc::new(test_utils::TestChainSource::new(Network::Testnet));
 			let persister = Arc::new(FilesystemPersister::new(format!("{}_persister_{}", &persist_dir, i)));
 			let now = Duration::from_secs(genesis_block.header.time as u64);
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -46,7 +46,7 @@ use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, No
 use crate::ln::features::InvoiceFeatures;
 use crate::routing::gossip::NetworkGraph;
 use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters, Router};
-use crate::routing::scoring::ProbabilisticScorer;
+use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters};
 use crate::ln::msgs;
 use crate::ln::onion_utils;
 use crate::ln::onion_utils::HTLCFailReason;
@@ -623,7 +623,9 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
 	Arc<DefaultRouter<
 		Arc<NetworkGraph<Arc<L>>>,
 		Arc<L>,
-		Arc<Mutex<ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>>>
+		Arc<Mutex<ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>>>,
+		ProbabilisticScoringFeeParameters,
+		ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>,
 	>>,
 	Arc<L>
 >;
@@ -639,7 +641,7 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
 /// of [`KeysManager`] and [`DefaultRouter`].
 ///
 /// This is not exported to bindings users as Arcs don't make sense in bindings
-pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = ChannelManager<&'a M, &'b T, &'c KeysManager, &'c KeysManager, &'c KeysManager, &'d F, &'e DefaultRouter<&'f NetworkGraph<&'g L>, &'g L, &'h Mutex<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>>, &'g L>;
+pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> = ChannelManager<&'a M, &'b T, &'c KeysManager, &'c KeysManager, &'c KeysManager, &'d F, &'e DefaultRouter<&'f NetworkGraph<&'g L>, &'g L, &'h Mutex<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>, ProbabilisticScoringFeeParameters, ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>, &'g L>;
 
 /// A trivial trait which describes any [`ChannelManager`] used in testing.
 #[cfg(any(test, feature = "_test_utils"))]
@@ -8586,7 +8588,7 @@ mod tests {
 		};
 		let route = find_route(
 			&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
-			None, nodes[0].logger, &scorer, &random_seed_bytes
+			None, nodes[0].logger, &scorer, &(), &random_seed_bytes
 		).unwrap();
 		nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
 			RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
@@ -8620,7 +8622,7 @@ mod tests {
 		let payment_preimage = PaymentPreimage([42; 32]);
 		let route = find_route(
 			&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
-			None, nodes[0].logger, &scorer, &random_seed_bytes
+			None, nodes[0].logger, &scorer, &(), &random_seed_bytes
 		).unwrap();
 		let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
 			RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
@@ -8683,7 +8685,7 @@ mod tests {
 		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 		let route = find_route(
 			&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
-			nodes[0].logger, &scorer, &random_seed_bytes
+			nodes[0].logger, &scorer, &(), &random_seed_bytes
 		).unwrap();
 
 		let test_preimage = PaymentPreimage([42; 32]);
@@ -8727,7 +8729,7 @@ mod tests {
 		let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 		let route = find_route(
 			&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
-			nodes[0].logger, &scorer, &random_seed_bytes
+			nodes[0].logger, &scorer, &(), &random_seed_bytes
 		).unwrap();
 
 		let test_preimage = PaymentPreimage([42; 32]);
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
@@ -1738,7 +1738,7 @@ pub fn get_route(send_node: &Node, payment_params: &PaymentParameters, recv_valu
 	router::get_route(
 		&send_node.node.get_our_node_id(), payment_params, &send_node.network_graph.read_only(),
 		Some(&send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
-		recv_value, send_node.logger, &scorer, &random_seed_bytes
+		recv_value, send_node.logger, &scorer, &(), &random_seed_bytes
 	)
 }
 
@@ -2328,7 +2328,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
 	let random_seed_bytes = keys_manager.get_secure_random_bytes();
 	let route = router::get_route(
 		&origin_node.node.get_our_node_id(), &payment_params, &network_graph,
-		None, recv_value, origin_node.logger, &scorer, &random_seed_bytes).unwrap();
+		None, recv_value, origin_node.logger, &scorer, &(), &random_seed_bytes).unwrap();
 	assert_eq!(route.paths.len(), 1);
 	assert_eq!(route.paths[0].hops.len(), expected_route.len());
 	for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
@@ -7044,7 +7044,7 @@ fn test_check_htlc_underpaying() {
 	let scorer = test_utils::TestScorer::new();
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features());
-	let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 10_000, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
+	let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 10_000, nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
 	let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
 	let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, None).unwrap();
 	nodes[0].node.send_payment_with_route(&route, our_payment_hash,
@@ -7298,11 +7298,11 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
 	let scorer = test_utils::TestScorer::new();
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 	let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None,
-		3_000_000, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
+		3_000_000, nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
 	let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 3_000_000).0;
 	let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), 50).with_features(nodes[0].node.invoice_features());
 	let route = get_route(&nodes[1].node.get_our_node_id(), &payment_params, &nodes[1].network_graph.read_only(), None,
-		3_000_000, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
+		3_000_000, nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
 	send_along_route(&nodes[1], route, &[&nodes[0]], 3_000_000);
 
 	let revoked_local_txn = get_local_commitment_txn!(nodes[1], chan.2);
@@ -9520,7 +9520,7 @@ fn test_keysend_payments_to_public_node() {
 	};
 	let scorer = test_utils::TestScorer::new();
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
-	let route = find_route(&payer_pubkey, &route_params, &network_graph, 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),
@@ -9555,7 +9555,7 @@ fn test_keysend_payments_to_private_node() {
 	let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 	let route = find_route(
 		&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
-		nodes[0].logger, &scorer, &random_seed_bytes
+		nodes[0].logger, &scorer, &(), &random_seed_bytes
 	).unwrap();
 
 	let test_preimage = PaymentPreimage([42; 32]);
diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
@@ -993,7 +993,7 @@ macro_rules! get_phantom_route {
 		(get_route(
 			&$nodes[0].node.get_our_node_id(), &payment_params, &network_graph,
 			Some(&$nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
-			$amt, $nodes[0].logger, &scorer, &[0u8; 32]
+			$amt, $nodes[0].logger, &scorer, &(), &[0u8; 32]
 		).unwrap(), phantom_route_hint.phantom_scid)
 	}
 }}
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
@@ -864,7 +864,7 @@ fn get_ldk_payment_preimage() {
 	let route = get_route(
 		&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(),
 		Some(&nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
-		amt_msat, nodes[0].logger, &scorer, &random_seed_bytes).unwrap();
+		amt_msat, nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
 	nodes[0].node.send_payment_with_route(&route, payment_hash,
 		RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
 	check_added_monitors!(nodes[0], 1);
@@ -1418,7 +1418,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
 	let route = get_route(
 		&nodes[0].node.get_our_node_id(), &route_params.payment_params,
 		&nodes[0].network_graph.read_only(), None, route_params.final_value_msat,
-		nodes[0].logger, &scorer, &random_seed_bytes,
+		nodes[0].logger, &scorer, &(), &random_seed_bytes,
 	).unwrap();
 
 	let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60, None).unwrap();
diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs
@@ -95,9 +95,9 @@ fn updates_shutdown_wait() {
 	let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[0]);
 
 	let payment_params_1 = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features());
-	let route_1 = get_route(&nodes[0].node.get_our_node_id(), &payment_params_1, &nodes[0].network_graph.read_only(), None, 100000, &logger, &scorer, &random_seed_bytes).unwrap();
+	let route_1 = get_route(&nodes[0].node.get_our_node_id(), &payment_params_1, &nodes[0].network_graph.read_only(), None, 100000, &logger, &scorer, &(), &random_seed_bytes).unwrap();
 	let payment_params_2 = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[0].node.invoice_features());
-	let route_2 = get_route(&nodes[1].node.get_our_node_id(), &payment_params_2, &nodes[1].network_graph.read_only(), None, 100000, &logger, &scorer, &random_seed_bytes).unwrap();
+	let route_2 = get_route(&nodes[1].node.get_our_node_id(), &payment_params_2, &nodes[1].network_graph.read_only(), None, 100000, &logger, &scorer, &(), &random_seed_bytes).unwrap();
 	unwrap_send_err!(nodes[0].node.send_payment_with_route(&route_1, payment_hash,
 			RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
 		), true, APIError::ChannelUnavailable {..}, {});
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ use lightning::ln::msgs;`
`17`	`17`	`use lightning::routing::gossip::{NetworkGraph, RoutingFees};`
`18`	`18`	`use lightning::routing::utxo::{UtxoFuture, UtxoLookup, UtxoLookupError, UtxoResult};`
`19`	`19`	`use lightning::routing::router::{find_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters};`
`20`		`-use lightning::routing::scoring::ProbabilisticScorer;`
	`20`	`+use lightning::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters};`
`21`	`21`	`use lightning::util::config::UserConfig;`
`22`	`22`	`use lightning::util::ser::Readable;`
`23`	`23`
`@@ -293,7 +293,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {`
`293`	`293`	`}]));`
`294`	`294`	`}`
`295`	`295`	`}`
`296`		`- let scorer = ProbabilisticScorer::new(Default::default(), &net_graph, &logger);`
	`296`	`+ let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &net_graph, &logger);`
`297`	`297`	`let random_seed_bytes: [u8; 32] = [get_slice!(1)[0]; 32];`
`298`	`298`	`for target in node_pks.iter() {`
`299`	`299`	`let final_value_msat = slice_to_be64(get_slice!(8));`
`@@ -305,7 +305,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {`
`305`	`305`	`};`
`306`	`306`	`let _ = find_route(&our_pubkey, &route_params, &net_graph,`
`307`	`307`	`first_hops.map(\|c\| c.iter().collect::<Vec<_>>()).as_ref().map(\|a\| a.as_slice()),`
`308`		`- &logger, &scorer, &random_seed_bytes);`
	`308`	`+ &logger, &scorer, &ProbabilisticScoringFeeParameters::default(), &random_seed_bytes);`
`309`	`309`	`}`
`310`	`310`	`},`
`311`	`311`	`}`
Original file line number	Diff line number	Diff line change
`@@ -993,7 +993,7 @@ macro_rules! get_phantom_route {`
`993`	`993`	`(get_route(`
`994`	`994`	`&$nodes[0].node.get_our_node_id(), &payment_params, &network_graph,`
`995`	`995`	`Some(&$nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),`
`996`		`- $amt, $nodes[0].logger, &scorer, &[0u8; 32]`
	`996`	`+ $amt, $nodes[0].logger, &scorer, &(), &[0u8; 32]`
`997`	`997`	`).unwrap(), phantom_route_hint.phantom_scid)`
`998`	`998`	`}`
`999`	`999`	`}}`